From 7cc4995abfd9746124363bf16a628096dfd1a3a3 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Tue, 25 Mar 2025 09:13:09 +1100 Subject: [PATCH 1/4] Fix for duplicate results in queryEntities when using an orderField Signed-off-by: Tyler Davis --- .changeset/famous-tips-raise.md | 5 +++++ .../catalog-backend/src/service/DefaultEntitiesCatalog.ts | 1 + 2 files changed, 6 insertions(+) create mode 100644 .changeset/famous-tips-raise.md diff --git a/.changeset/famous-tips-raise.md b/.changeset/famous-tips-raise.md new file mode 100644 index 0000000000..89fc8a6563 --- /dev/null +++ b/.changeset/famous-tips-raise.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Fix for duplicate results in queryEntities when using an orderField diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts index f0e0667bef..a7b8fdc6c9 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts @@ -285,6 +285,7 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { if (sortField) { inner + .distinct() .leftOuterJoin('search', qb => qb .on('search.entity_id', 'final_entities.entity_id') From b8b327dcdbcd9ce737af02e885018afb1a73dd19 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Tue, 25 Mar 2025 11:36:45 +1100 Subject: [PATCH 2/4] changeset improvements Signed-off-by: Tyler Davis --- .changeset/famous-tips-raise.md | 2 +- .../src/service/DefaultEntitiesCatalog.ts | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/.changeset/famous-tips-raise.md b/.changeset/famous-tips-raise.md index 89fc8a6563..1a44e19608 100644 --- a/.changeset/famous-tips-raise.md +++ b/.changeset/famous-tips-raise.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog-backend': patch --- -Fix for duplicate results in queryEntities when using an orderField +Fix for duplicate results in `queryEntities` when providing an `orderField` parameter diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts index a7b8fdc6c9..8e71e61bc5 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts @@ -273,6 +273,24 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { const sortField = cursor.orderFields.at(0); + if (sortField) { + const duplicateCheck = await this.database('search') + .select('entity_id', 'key', 'value') + .count('* as count') + .where('key', '=', sortField.field) + .groupBy('entity_id', 'key', 'value') + .having(this.database.raw('count(*) > 1')) + .limit(5); + + if (duplicateCheck.length > 0) { + this.logger.warn( + `Found duplicate search entries for field ${ + sortField.field + }: ${JSON.stringify(duplicateCheck)}`, + ); + } + } + // The first part of the query builder is a subquery that applies all of the // filtering. const dbQuery = this.database.with( From 5fc743a3fd32dd5538eb459ea8c244a314539119 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Tue, 25 Mar 2025 11:37:33 +1100 Subject: [PATCH 3/4] remove debugging code Signed-off-by: Tyler Davis --- .../src/service/DefaultEntitiesCatalog.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts index 8e71e61bc5..a7b8fdc6c9 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts @@ -273,24 +273,6 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { const sortField = cursor.orderFields.at(0); - if (sortField) { - const duplicateCheck = await this.database('search') - .select('entity_id', 'key', 'value') - .count('* as count') - .where('key', '=', sortField.field) - .groupBy('entity_id', 'key', 'value') - .having(this.database.raw('count(*) > 1')) - .limit(5); - - if (duplicateCheck.length > 0) { - this.logger.warn( - `Found duplicate search entries for field ${ - sortField.field - }: ${JSON.stringify(duplicateCheck)}`, - ); - } - } - // The first part of the query builder is a subquery that applies all of the // filtering. const dbQuery = this.database.with( From 0cf9d74a5fea9f5965814ac064c3e1290efba114 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Tue, 25 Mar 2025 12:39:33 +1100 Subject: [PATCH 4/4] Add test to replicate the duplicate issue Signed-off-by: Tyler Davis --- .../service/DefaultEntitiesCatalog.test.ts | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts index 070b93ab2c..aad09d7de1 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts @@ -1970,6 +1970,87 @@ describe('DefaultEntitiesCatalog', () => { ).resolves.toEqual(['BB']); }, ); + + it.each(databases.eachSupportedId())( + 'should not return duplicate entities when using orderField, %p', + async databaseId => { + await createDatabase(databaseId); + + // Create a few test entities with different names to sort by + const entities = [ + { + apiVersion: 'a', + kind: 'k', + metadata: { + name: 'a-entity', + title: 'A Test Entity', + uid: 'uid-a', + }, + spec: {}, + }, + { + apiVersion: 'a', + kind: 'k', + metadata: { + name: 'b-entity', + title: 'B Test Entity', + uid: 'uid-b', + }, + spec: {}, + }, + { + apiVersion: 'a', + kind: 'k', + metadata: { + name: 'c-entity', + title: 'C Test Entity', + uid: 'uid-c', + }, + spec: {}, + }, + ]; + + await Promise.all(entities.map(e => addEntityToSearch(e))); + + // Manually insert duplicate search entries for the same entities + // I'm not sure exactly how this happens but I have seen it in the real world + await knex('search').insert([ + { + entity_id: 'uid-a', + key: 'metadata.title', + value: 'a test entity', + original_value: 'A Test Entity', + }, + { + entity_id: 'uid-b', + key: 'metadata.title', + value: 'b test entity', + original_value: 'B Test Entity', + }, + ]); + + const catalog = new DefaultEntitiesCatalog({ + database: knex, + logger: mockServices.logger.mock(), + stitcher, + }); + + // Query with orderField + const response = await catalog.queryEntities({ + orderFields: [{ field: 'metadata.title', order: 'asc' }], + credentials: mockCredentials.none(), + }); + + const resultEntities = entitiesResponseToObjects(response.items); + + // Ensure we get exactly 3 entities back, sorted, with no duplicates + expect(resultEntities.map(e => e!.metadata.name)).toEqual([ + 'a-entity', + 'b-entity', + 'c-entity', + ]); + }, + ); }); describe('removeEntityByUid', () => {