paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 53 Packages Projects Releases Wiki Activity

catalog-backend: handle clashing items on pagination

Browse Source 
Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
This commit is contained in:
Vincenzo Scamporlino
2022-12-29 22:56:07 -06:00
parent f277673534
commit 77a08f4ffc
2 changed files with 168 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts
+116 -18
View File
@@ -85,8 +85,7 @@ describe('DefaultEntitiesCatalog', () => {
return id;
}
async function addEntityToSearch(knex: Knex, entity: Entity) {
const id = uuid();
async function addEntityToSearch(knex: Knex, entity: Entity, id = uuid()) {
const entityRef = stringifyEntityRef(entity);
const entityJson = JSON.stringify(entity);
@@ -746,7 +745,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response2.entities).toEqual([entityFrom('C'), entityFrom('D')]);
expect(response2.nextCursor).toBeDefined();
expect(response2.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response2.totalItems).toBe(names.length);
// third request (forward)
const request3: PaginatedEntitiesCursorRequest = {
@@ -757,7 +756,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response3.entities).toEqual([entityFrom('E'), entityFrom('F')]);
expect(response3.nextCursor).toBeDefined();
expect(response3.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response3.totalItems).toBe(names.length);
// fourth request (backwards)
const request4: PaginatedEntitiesCursorRequest = {
@@ -768,7 +767,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response4.entities).toEqual([entityFrom('C'), entityFrom('D')]);
expect(response4.nextCursor).toBeDefined();
expect(response4.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response4.totalItems).toBe(names.length);
// fifth request (backwards)
const request5: PaginatedEntitiesCursorRequest = {
@@ -779,7 +778,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response5.entities).toEqual([entityFrom('A'), entityFrom('B')]);
expect(response5.nextCursor).toBeDefined();
expect(response5.prevCursor).toBeUndefined();
expect(response1.totalItems).toBe(names.length);
expect(response5.totalItems).toBe(names.length);
// sixth request (forward)
const request6: PaginatedEntitiesCursorRequest = {
@@ -790,7 +789,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response6.entities).toEqual([entityFrom('C'), entityFrom('D')]);
expect(response6.nextCursor).toBeDefined();
expect(response6.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response6.totalItems).toBe(names.length);
// seventh request (forward)
const request7: PaginatedEntitiesCursorRequest = {
@@ -801,7 +800,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response7.entities).toEqual([entityFrom('E'), entityFrom('F')]);
expect(response7.nextCursor).toBeDefined();
expect(response7.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response7.totalItems).toBe(names.length);
// seventh.2 request (forward with a different limit)
const request7bis: PaginatedEntitiesCursorRequest = {
@@ -816,7 +815,7 @@ describe('DefaultEntitiesCatalog', () => {
]);
expect(response7bis.nextCursor).toBeUndefined();
expect(response7bis.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response7bis.totalItems).toBe(names.length);
// last request (forward)
const request8: PaginatedEntitiesCursorRequest = {
@@ -827,7 +826,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response8.entities).toEqual([entityFrom('G')]);
expect(response8.nextCursor).toBeUndefined();
expect(response8.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response8.totalItems).toBe(names.length);
},
);
@@ -898,7 +897,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response2.entities).toEqual([entityFrom('E'), entityFrom('D')]);
expect(response2.nextCursor).toBeDefined();
expect(response2.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response2.totalItems).toBe(names.length);
// third request (forward)
const request3: PaginatedEntitiesCursorRequest = {
@@ -909,7 +908,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response3.entities).toEqual([entityFrom('C'), entityFrom('B')]);
expect(response3.nextCursor).toBeDefined();
expect(response3.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response3.totalItems).toBe(names.length);
// fourth request (backwards)
const request4: PaginatedEntitiesCursorRequest = {
@@ -921,7 +920,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response4.entities).toEqual([entityFrom('E'), entityFrom('D')]);
expect(response4.nextCursor).toBeDefined();
expect(response4.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response4.totalItems).toBe(names.length);
// fifth request (backwards)
const request5: PaginatedEntitiesCursorRequest = {
@@ -932,7 +931,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response5.entities).toEqual([entityFrom('G'), entityFrom('F')]);
expect(response5.nextCursor).toBeDefined();
expect(response5.prevCursor).toBeUndefined();
expect(response1.totalItems).toBe(names.length);
expect(response5.totalItems).toBe(names.length);
// sixth request (forward)
const request6: PaginatedEntitiesCursorRequest = {
@@ -943,7 +942,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response6.entities).toEqual([entityFrom('E'), entityFrom('D')]);
expect(response6.nextCursor).toBeDefined();
expect(response6.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response6.totalItems).toBe(names.length);
// seventh request (forward)
const request7: PaginatedEntitiesCursorRequest = {
@@ -954,7 +953,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response7.entities).toEqual([entityFrom('C'), entityFrom('B')]);
expect(response7.nextCursor).toBeDefined();
expect(response7.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response7.totalItems).toBe(names.length);
// seventh.2 request (forward with a different limit)
const request7bis: PaginatedEntitiesCursorRequest = {
@@ -969,7 +968,7 @@ describe('DefaultEntitiesCatalog', () => {
]);
expect(response7bis.nextCursor).toBeUndefined();
expect(response7bis.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response7bis.totalItems).toBe(names.length);
// last request (forward)
const request8: PaginatedEntitiesCursorRequest = {
@@ -980,7 +979,7 @@ describe('DefaultEntitiesCatalog', () => {
expect(response8.entities).toEqual([entityFrom('A')]);
expect(response8.nextCursor).toBeUndefined();
expect(response8.prevCursor).toBeDefined();
expect(response1.totalItems).toBe(names.length);
expect(response8.totalItems).toBe(names.length);
},
);
@@ -1073,6 +1072,105 @@ describe('DefaultEntitiesCatalog', () => {
expect(response).toEqual({ totalItems: 20, entities: [] });
},
);
it.each(databases.eachSupportedId())(
'should paginate results accordingly in case of clashing items, %p',
async databaseId => {
const { knex } = await createDatabase(databaseId);
function entityFrom(name: string, namespace?: string) {
return {
apiVersion: 'a',
kind: 'k',
metadata: { name, ...(!!namespace && { namespace }) },
spec: { should_include_this: 'yes' },
};
}
await Promise.all([
addEntityToSearch(knex, entityFrom('AA')),
addEntityToSearch(knex, entityFrom('AA', 'namespace2')),
addEntityToSearch(knex, entityFrom('AA', 'namespace3')),
addEntityToSearch(knex, entityFrom('AA', 'namespace4')),
addEntityToSearch(knex, entityFrom('CC')),
addEntityToSearch(knex, entityFrom('DD')),
]);
const catalog = new DefaultEntitiesCatalog(knex);
const limit = 2;
// initial request
const request1: PaginatedEntitiesInitialRequest = {
limit,
sortFields: [{ field: 'metadata.name' }],
};
const response1 = await catalog.paginatedEntities(request1);
expect(response1.entities).toMatchObject([
entityFrom('AA'),
entityFrom('AA'),
]);
expect(response1.nextCursor).toBeDefined();
expect(response1.prevCursor).toBeUndefined();
expect(response1.totalItems).toBe(6);
// second request (forward)
const request2: PaginatedEntitiesCursorRequest = {
cursor: response1.nextCursor!,
limit,
};
const response2 = await catalog.paginatedEntities(request2);
expect(response2.entities).toMatchObject([
entityFrom('AA'),
entityFrom('AA'),
]);
expect(response2.nextCursor).toBeDefined();
expect(response2.prevCursor).toBeDefined();
expect(response2.totalItems).toBe(6);
// third request (forward)
const request3: PaginatedEntitiesCursorRequest = {
cursor: response2.nextCursor!,
limit,
};
const response3 = await catalog.paginatedEntities(request3);
expect(response3.entities).toEqual([
entityFrom('CC'),
entityFrom('DD'),
]);
expect(response3.nextCursor).toBeUndefined();
expect(response3.prevCursor).toBeDefined();
expect(response3.totalItems).toBe(6);
// forth request (backward)
const request4: PaginatedEntitiesCursorRequest = {
cursor: response3.prevCursor!,
limit,
};
const response4 = await catalog.paginatedEntities(request4);
expect(response4.entities).toMatchObject([
entityFrom('AA'),
entityFrom('AA'),
]);
expect(response4.nextCursor).toBeDefined();
expect(response4.prevCursor).toBeDefined();
expect(response4.totalItems).toBe(6);
// fifth request (backward)
const request5: PaginatedEntitiesCursorRequest = {
cursor: response4.prevCursor!,
limit,
};
const response5 = await catalog.paginatedEntities(request5);
expect(response5.entities).toMatchObject([
entityFrom('AA'),
entityFrom('AA'),
]);
expect(response5.nextCursor).toBeDefined();
expect(response5.prevCursor).toBeUndefined();
expect(response5.totalItems).toBe(6);
},
);
});
describe('removeEntityByUid', () => {
plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts
+52 -21
View File
@@ -20,8 +20,8 @@ import {
stringifyEntityRef,
} from '@backstage/catalog-model';
import { InputError, NotFoundError } from '@backstage/errors';
import lodash from 'lodash';
import { Knex } from 'knex';
import { isEqual } from 'lodash';
import {
EntitiesBatchRequest,
EntitiesBatchResponse,
@@ -331,8 +331,9 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog {
const db = this.database;
const limit = request?.limit ?? 20;
const cursor: Omit<Cursor, 'sortFieldIds'> & { sortFieldIds?: string[] } = {
firstFieldId: '',
const cursor: Omit<Cursor, 'sortFieldValues'> & {
sortFieldValues?: string[];
} = {
sortFields: [defaultSortField],
isPrevious: false,
...parseCursorFromRequest(request),
@@ -346,11 +347,11 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog {
...cursor.sortFields[0],
};
const sortFieldId = cursor.sortFieldIds?.[0];
const [sortFieldId, metadataSortFieldId] = cursor.sortFieldValues || [];
const dbQuery = db('search')
.join('final_entities', 'search.entity_id', 'final_entities.entity_id')
.where('key', sortField.field);
.where('search.key', sortField.field);
if (cursor.filter) {
parseFilter(cursor.filter, dbQuery, db, false, 'search.entity_id');
@@ -368,18 +369,36 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog {
const isOrderingDescending = sortField.order === 'desc';
if (sortFieldId) {
dbQuery.andWhere(
'value',
isFetchingBackwards !== isOrderingDescending ? '<' : '>',
sortFieldId,
);
dbQuery
.andWhere(
'value',
isFetchingBackwards !== isOrderingDescending ? '<' : '>',
sortFieldId,
)
.orWhere(function nested() {
this.where('value', '=', sortFieldId).andWhere(
'search.entity_id',
isFetchingBackwards !== isOrderingDescending ? '<' : '>',
metadataSortFieldId,
);
});
}
dbQuery
.orderBy(
'value',
isFetchingBackwards ? invertOrder(sortField.order) : sortField.order,
)
.orderBy([
{
column: 'value',
order: isFetchingBackwards
? invertOrder(sortField.order)
: sortField.order,
},
{
column: 'search.entity_id',
order: isFetchingBackwards
? invertOrder(sortField.order)
: sortField.order,
},
])
// fetch an extra item to check if there are more items.
.limit(isFetchingBackwards ? limit : limit + 1);
@@ -409,15 +428,22 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog {
rows.length -= 1;
}
const isInitialRequest = cursor.firstFieldId === '';
const isInitialRequest = cursor.firstSortFieldValues === undefined;
const firstFieldId = cursor.firstFieldId || rows[0]?.value;
const firstSortFieldValues = cursor.firstSortFieldValues || [
rows[0]?.value,
rows[0]?.entity_id,
];
const nextCursor = hasMoreResults
? encodeCursor({
...cursor,
sortFieldIds: [rows[rows.length - 1].value],
firstFieldId,
sortFieldValues: [
// TODO generalize
rows[rows.length - 1].value,
rows[rows.length - 1].entity_id,
],
firstSortFieldValues,
isPrevious: false,
totalItems,
})
@@ -426,11 +452,12 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog {
const prevCursor =
!isInitialRequest &&
rows.length > 0 &&
rows[0].value !== cursor.firstFieldId
!isEqual(sortFieldsFromRow(rows[0]), cursor.firstSortFieldValues)
? encodeCursor({
...cursor,
sortFieldIds: [rows[0].value],
firstFieldId: cursor.firstFieldId,
// TODO generalize
sortFieldValues: [rows[0].value, rows[0].entity_id],
firstSortFieldValues: cursor.firstSortFieldValues,
isPrevious: true,
totalItems,
})
@@ -652,3 +679,7 @@ function parseCursorFromRequest(
function invertOrder(order: EntitySortField['order']) {
return order === 'asc' ? 'desc' : 'asc';
}
function sortFieldsFromRow(row: DbSearchRow) {
return [row.value, row.entity_id];
}