Skip to content

Commit 4dc156c

Browse files
authored
fix: explicitly document behavior when cursor entity no longer exists (#453)
# Why The one downside of using id-based cursor pagination and subqueries (added in #422, #431) is that it risks the row referenced by the cursor being deleted between pagination requests. It's an edge case though to be clear. Encoding the full cursor alleviates this, but has it's own downsides since trigram similarity isn't encodable and neither are things like Date objects. It is still preferable to use ID-based cursors for all pagination. But it becomes the library's responsibility to explicitly document what the behavior is when a row referenced by the cursor is no longer present. This PR does this. # How When the cursor row is no longer present, the tuple evaluates to `NULL`, which produces a empty page. The alternative to this behavior is to run an id check query ahead of the pagination query, and throw an error if it doesn't exist, or even return the first page of data if it doesn't exist. Both of these are less optimal since they cause unexpected behavior during results consumption. So we keep the behavior as is and explicitly document it. # Test Plan Run new tests.
1 parent cc78029 commit 4dc156c

2 files changed

Lines changed: 58 additions & 29 deletions

File tree

packages/entity-database-adapter-knex/src/__integration-tests__/PostgresEntityIntegration-test.ts

Lines changed: 44 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,8 +1618,6 @@ describe('postgres entity integration', () => {
16181618
);
16191619

16201620
// Create test data with names that sort in a specific order
1621-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
1622-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
16231621

16241622
const entities = [];
16251623
for (let i = 1; i <= 5; i++) {
@@ -1673,8 +1671,6 @@ describe('postgres entity integration', () => {
16731671
);
16741672

16751673
// Create entities with duplicate values to test stability
1676-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
1677-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
16781674

16791675
const entities = [];
16801676
for (let i = 1; i <= 6; i++) {
@@ -1777,9 +1773,6 @@ describe('postgres entity integration', () => {
17771773
createKnexIntegrationTestEntityCompanionProvider(knexInstance),
17781774
);
17791775

1780-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
1781-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
1782-
17831776
// Create entities with different names
17841777
const names = ['Alice', 'Bob', 'Charlie', 'David', 'Eve', 'Frank'];
17851778
for (const name of names) {
@@ -1841,9 +1834,6 @@ describe('postgres entity integration', () => {
18411834
createKnexIntegrationTestEntityCompanionProvider(knexInstance),
18421835
);
18431836

1844-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
1845-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
1846-
18471837
// Create exactly 6 entities
18481838
for (let i = 1; i <= 6; i++) {
18491839
await PostgresTestEntity.creator(vc).setField('name', `Entity${i}`).createAsync();
@@ -1879,8 +1869,6 @@ describe('postgres entity integration', () => {
18791869
const vc = new ViewerContext(
18801870
createKnexIntegrationTestEntityCompanionProvider(knexInstance),
18811871
);
1882-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
1883-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
18841872

18851873
// Create test data with specific order
18861874
await PostgresTestEntity.creator(vc).setField('name', 'Charlie').createAsync();
@@ -1910,8 +1898,6 @@ describe('postgres entity integration', () => {
19101898
const vc = new ViewerContext(
19111899
createKnexIntegrationTestEntityCompanionProvider(knexInstance),
19121900
);
1913-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
1914-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
19151901

19161902
// Enable pg_trgm extension for trigram similarity
19171903
await knexInstance.raw('CREATE EXTENSION IF NOT EXISTS pg_trgm');
@@ -2091,15 +2077,56 @@ describe('postgres entity integration', () => {
20912077
});
20922078
});
20932079

2080+
it('returns empty page when cursor entity no longer exists', async () => {
2081+
const vc = new ViewerContext(createKnexIntegrationTestEntityCompanionProvider(knexInstance));
2082+
2083+
// Create test entities
2084+
const names = ['Alice', 'Bob', 'Charlie', 'David', 'Eve'];
2085+
for (const name of names) {
2086+
await PostgresTestEntity.creator(vc).setField('name', name).createAsync();
2087+
}
2088+
2089+
// Get first page and capture cursor pointing to a specific entity
2090+
const firstPage = await PostgresTestEntity.knexLoader(vc).loadPageAsync({
2091+
first: 2,
2092+
pagination: {
2093+
strategy: PaginationStrategy.STANDARD,
2094+
orderBy: [{ fieldName: 'name', order: OrderByOrdering.ASCENDING }],
2095+
},
2096+
});
2097+
2098+
expect(firstPage.edges).toHaveLength(2);
2099+
const cursorEntityNode = firstPage.edges[1]!.node; // 'Bob'
2100+
const cursor = firstPage.pageInfo.endCursor!;
2101+
2102+
// Delete the entity that the cursor refers to
2103+
await PostgresTestEntity.deleter(cursorEntityNode).deleteAsync();
2104+
2105+
// Paginate using the cursor of the now-deleted entity
2106+
const result = await PostgresTestEntity.knexLoader(vc).loadPageAsync({
2107+
first: 10,
2108+
after: cursor,
2109+
pagination: {
2110+
strategy: PaginationStrategy.STANDARD,
2111+
orderBy: [{ fieldName: 'name', order: OrderByOrdering.ASCENDING }],
2112+
},
2113+
});
2114+
2115+
expect(result.edges).toEqual([]);
2116+
expect(result.pageInfo).toEqual({
2117+
hasNextPage: false,
2118+
hasPreviousPage: false,
2119+
startCursor: null,
2120+
endCursor: null,
2121+
});
2122+
});
2123+
20942124
describe(PaginationStrategy.ILIKE_SEARCH, () => {
20952125
it('supports search with ILIKE strategy', async () => {
20962126
const vc = new ViewerContext(
20972127
createKnexIntegrationTestEntityCompanionProvider(knexInstance),
20982128
);
20992129

2100-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
2101-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
2102-
21032130
// Create test data with searchable names
21042131
const names = [
21052132
'Alice Johnson',
@@ -2196,8 +2223,6 @@ describe('postgres entity integration', () => {
21962223
const vc = new ViewerContext(
21972224
createKnexIntegrationTestEntityCompanionProvider(knexInstance),
21982225
);
2199-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
2200-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
22012226

22022227
// Create test data
22032228
const names = ['Apple', 'Application', 'Apply', 'Banana', 'Cherry', 'Pineapple'];
@@ -2388,9 +2413,6 @@ describe('postgres entity integration', () => {
23882413
createKnexIntegrationTestEntityCompanionProvider(knexInstance),
23892414
);
23902415

2391-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
2392-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
2393-
23942416
// Enable pg_trgm extension for trigram similarity
23952417
await knexInstance.raw('CREATE EXTENSION IF NOT EXISTS pg_trgm');
23962418

@@ -2448,8 +2470,6 @@ describe('postgres entity integration', () => {
24482470
const vc = new ViewerContext(
24492471
createKnexIntegrationTestEntityCompanionProvider(knexInstance),
24502472
);
2451-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
2452-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
24532473

24542474
// Enable pg_trgm extension for trigram similarity
24552475
await knexInstance.raw('CREATE EXTENSION IF NOT EXISTS pg_trgm');
@@ -2552,8 +2572,6 @@ describe('postgres entity integration', () => {
25522572
const vc = new ViewerContext(
25532573
createKnexIntegrationTestEntityCompanionProvider(knexInstance),
25542574
);
2555-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
2556-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
25572575

25582576
// Enable pg_trgm extension for trigram similarity
25592577
await knexInstance.raw('CREATE EXTENSION IF NOT EXISTS pg_trgm');
@@ -2721,8 +2739,6 @@ describe('postgres entity integration', () => {
27212739
const vc = new ViewerContext(
27222740
createKnexIntegrationTestEntityCompanionProvider(knexInstance),
27232741
);
2724-
await PostgresTestEntity.dropPostgresTableAsync(knexInstance);
2725-
await PostgresTestEntity.createOrTruncatePostgresTableAsync(knexInstance);
27262742

27272743
// Enable pg_trgm extension for trigram similarity
27282744
await knexInstance.raw('CREATE EXTENSION IF NOT EXISTS pg_trgm');

packages/entity-database-adapter-knex/src/internal/EntityKnexDataManager.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,15 @@ export class EntityKnexDataManager<
218218
/**
219219
* Load a page of objects using cursor-based pagination with unified pagination specification.
220220
*
221+
* @remarks
222+
*
223+
* This method implements cursor-based pagination using the seek method for efficient pagination even on large datasets
224+
* given appropriate indexes. Cursors are opaque and encode the necessary information to fetch the next page based on the
225+
* specified pagination strategy (standard, ILIKE search, or trigram search). For this implementation in particular,
226+
* the cursor encodes the ID of the last entity in the page to ensure correct pagination for all strategies, even in cases
227+
* where multiple rows have the same value for all fields other than the ID. If the entity referenced by a cursor has been
228+
* deleted, the load will return an empty page with `hasNextPage: false`.
229+
*
221230
* @param queryContext - query context in which to perform the load
222231
* @param args - pagination arguments including pagination and first/after or last/before
223232
* @returns connection with edges containing field objects and page info
@@ -477,6 +486,8 @@ export class EntityKnexDataManager<
477486
// We build a tuple comparison for fieldsToUseInPostgresTupleCursor fields of the
478487
// entity identified by the external cursor to ensure correct pagination behavior
479488
// even in cases where multiple rows have the same value all fields other than id.
489+
// If the cursor entity has been deleted, the subquery returns no rows and the
490+
// comparison evaluates to NULL, filtering out all results (empty page).
480491
const operator = direction === PaginationDirection.FORWARD ? '>' : '<';
481492

482493
const idField = getDatabaseFieldForEntityField(
@@ -555,7 +566,9 @@ export class EntityKnexDataManager<
555566
decodedExternalCursorEntityID: TFields[TIDField],
556567
direction: PaginationDirection,
557568
): SQLFragment {
558-
// For TRIGRAM search, we compute the similarity values using a subquery, similar to normal cursor
569+
// For TRIGRAM search, we compute the similarity values using a subquery, similar to normal cursor.
570+
// If the cursor entity has been deleted, the subquery returns no rows and the
571+
// comparison evaluates to NULL, filtering out all results (empty page).
559572
const operator = direction === PaginationDirection.FORWARD ? '<' : '>';
560573
const idField = getDatabaseFieldForEntityField(
561574
this.entityConfiguration,

0 commit comments

Comments
 (0)