From b5c54795047f90cf407e92c59e560705c94b0ae4 Mon Sep 17 00:00:00 2001 From: Joon Park Date: Tue, 16 Nov 2021 13:06:31 +0000 Subject: [PATCH] Fix tests Signed-off-by: Joon Park --- plugins/catalog-backend/src/catalog/index.ts | 2 -- .../legacy/database/CommonDatabase.test.ts | 28 +------------------ .../src/legacy/database/CommonDatabase.ts | 25 ++++++++--------- .../src/legacy/service/router.test.ts | 6 ++-- .../src/service/NextEntitiesCatalog.ts | 1 - .../src/service/NextRouter.test.ts | 6 ++-- .../request/parseEntityFilterParams.test.ts | 24 ++++++++-------- 7 files changed, 30 insertions(+), 62 deletions(-) diff --git a/plugins/catalog-backend/src/catalog/index.ts b/plugins/catalog-backend/src/catalog/index.ts index 460aefddba..885a2f5389 100644 --- a/plugins/catalog-backend/src/catalog/index.ts +++ b/plugins/catalog-backend/src/catalog/index.ts @@ -23,8 +23,6 @@ export type { EntityUpsertResponse, PageInfo, EntitiesSearchFilter, - EntitiesKeyFilter, - EntitiesValuesFilter, EntityFilter, EntityPagination, } from './types'; diff --git a/plugins/catalog-backend/src/legacy/database/CommonDatabase.test.ts b/plugins/catalog-backend/src/legacy/database/CommonDatabase.test.ts index ca2e6c2934..8b72b470a6 100644 --- a/plugins/catalog-backend/src/legacy/database/CommonDatabase.test.ts +++ b/plugins/catalog-backend/src/legacy/database/CommonDatabase.test.ts @@ -536,9 +536,7 @@ describe('CommonDatabase', () => { filter: { anyOf: [ { - allOf: [ - { key: 'metadata.annotations.foo', matchValueExists: true }, - ], + allOf: [{ key: 'metadata.annotations.foo' }], }, ], }, @@ -558,30 +556,6 @@ describe('CommonDatabase', () => { }, ]), ); - - const nonExistRows = await db.transaction(async tx => - db.entities(tx, { - filter: { - anyOf: [ - { - allOf: [ - { key: 'metadata.annotations.foo', matchValueExists: false }, - ], - }, - ], - }, - }), - ); - - expect(nonExistRows.entities.length).toEqual(1); - expect(nonExistRows.entities).toEqual( - expect.arrayContaining([ - { - locationId: undefined, - entity: expect.objectContaining({ kind: 'k3' }), - }, - ]), - ); }); }); diff --git a/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts b/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts index 6a150f7583..3ea8696437 100644 --- a/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts +++ b/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts @@ -224,7 +224,8 @@ export class CommonDatabase implements Database { if ( request?.filter && (request.filter.hasOwnProperty('key') || - request.filter.hasOwnProperty('allOf')) + request.filter.hasOwnProperty('allOf') || + request.filter.hasOwnProperty('not')) ) { throw new Error( 'Filters for the legacy CommonDatabase must obey the { anyOf: [{ allOf: [] }] } format.', @@ -236,13 +237,14 @@ export class CommonDatabase implements Database { for (const filter of singleFilter.allOf) { if ( filter.hasOwnProperty('anyOf') || - filter.hasOwnProperty('allOf') + filter.hasOwnProperty('allOf') || + filter.hasOwnProperty('not') ) { throw new Error( 'Nested filters are not supported in the legacy CommonDatabase', ); } - const { key, matchValueIn, matchValueExists } = filter; + const { key, values } = filter; // NOTE(freben): This used to be a set of OUTER JOIN, which may seem to // make a lot of sense. However, it had abysmal performance on sqlite // when datasets grew large, so we're using IN instead. @@ -250,24 +252,19 @@ export class CommonDatabase implements Database { .select('entity_id') .where(function keyFilter() { this.andWhere({ key: key.toLowerCase() }); - if (matchValueExists !== false && matchValueIn) { - if (matchValueIn.length === 1) { - this.andWhere({ value: matchValueIn[0].toLowerCase() }); - } else if (matchValueIn.length > 1) { + if (values) { + if (values.length === 1) { + this.andWhere({ value: values[0].toLowerCase() }); + } else if (values.length > 1) { this.andWhere( 'value', 'in', - matchValueIn.map(v => v.toLowerCase()), + values.map(v => v.toLowerCase()), ); } } }); - // Explicitly evaluate matchValueExists as a boolean since it may be undefined - this.andWhere( - 'id', - matchValueExists === false ? 'not in' : 'in', - matchQuery, - ); + this.andWhere('id', 'in', matchQuery); } }); } diff --git a/plugins/catalog-backend/src/legacy/service/router.test.ts b/plugins/catalog-backend/src/legacy/service/router.test.ts index edc7d852ac..0ff5a44ed0 100644 --- a/plugins/catalog-backend/src/legacy/service/router.test.ts +++ b/plugins/catalog-backend/src/legacy/service/router.test.ts @@ -115,11 +115,11 @@ describe('createRouter readonly disabled', () => { anyOf: [ { allOf: [ - { key: 'a', matchValueIn: ['1', '2'] }, - { key: 'b', matchValueIn: ['3'] }, + { key: 'a', values: ['1', '2'] }, + { key: 'b', values: ['3'] }, ], }, - { allOf: [{ key: 'c', matchValueIn: ['4'] }] }, + { allOf: [{ key: 'c', values: ['4'] }] }, ], }, }); diff --git a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts index 0f2fb9494c..8c2de9d7c1 100644 --- a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts @@ -100,7 +100,6 @@ function addCondition( } } }); - // Explicitly evaluate matchValueExists as a boolean since it may be undefined queryBuilder.andWhere('entity_id', negate ? 'not in' : 'in', matchQuery); } diff --git a/plugins/catalog-backend/src/service/NextRouter.test.ts b/plugins/catalog-backend/src/service/NextRouter.test.ts index 57911c74fb..6bc4481425 100644 --- a/plugins/catalog-backend/src/service/NextRouter.test.ts +++ b/plugins/catalog-backend/src/service/NextRouter.test.ts @@ -104,11 +104,11 @@ describe('createNextRouter readonly disabled', () => { anyOf: [ { allOf: [ - { key: 'a', matchValueIn: ['1', '2'] }, - { key: 'b', matchValueIn: ['3'] }, + { key: 'a', values: ['1', '2'] }, + { key: 'b', values: ['3'] }, ], }, - { allOf: [{ key: 'c', matchValueIn: ['4'] }] }, + { allOf: [{ key: 'c', values: ['4'] }] }, ], }, }); diff --git a/plugins/catalog-backend/src/service/request/parseEntityFilterParams.test.ts b/plugins/catalog-backend/src/service/request/parseEntityFilterParams.test.ts index fc22a638de..f9fc596cb9 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityFilterParams.test.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityFilterParams.test.ts @@ -28,7 +28,7 @@ describe('parseEntityFilterParams', () => { it('supports single-string format', () => { const result = parseEntityFilterParams({ filter: 'a=1' })!; expect(result).toEqual({ - anyOf: [{ allOf: [{ key: 'a', matchValueIn: ['1'] }] }], + anyOf: [{ allOf: [{ key: 'a', values: ['1'] }] }], }); }); @@ -38,8 +38,8 @@ describe('parseEntityFilterParams', () => { }); expect(result).toEqual({ anyOf: [ - { allOf: [{ key: 'a', matchValueIn: ['1'] }] }, - { allOf: [{ key: 'b', matchValueIn: ['2'] }] }, + { allOf: [{ key: 'a', values: ['1'] }] }, + { allOf: [{ key: 'b', values: ['2'] }] }, ], }); }); @@ -50,11 +50,11 @@ describe('parseEntityFilterParams', () => { }); expect(result).toEqual({ anyOf: [ - { allOf: [{ key: 'a', matchValueIn: ['1'] }] }, + { allOf: [{ key: 'a', values: ['1'] }] }, { allOf: [ - { key: 'b', matchValueIn: ['2', '3'] }, - { key: 'c', matchValueIn: ['4'] }, + { key: 'b', values: ['2', '3'] }, + { key: 'c', values: ['4'] }, ], }, ], @@ -70,17 +70,17 @@ describe('parseEntityFilterString', () => { it('works for the happy path', () => { expect(parseEntityFilterString('')).toBeUndefined(); expect(parseEntityFilterString('a=1,b=2,a=3,c,d=')).toEqual([ - { key: 'a', matchValueIn: ['1', '3'] }, - { key: 'b', matchValueIn: ['2'] }, - { key: 'c', matchValueExists: true }, - { key: 'd', matchValueIn: [''] }, + { key: 'a', values: ['1', '3'] }, + { key: 'b', values: ['2'] }, + { key: 'c' }, + { key: 'd', values: [''] }, ]); }); it('trims values', () => { expect(parseEntityFilterString(' a = 1 , b = 2 , a = 3 ')).toEqual([ - { key: 'a', matchValueIn: ['1', '3'] }, - { key: 'b', matchValueIn: ['2'] }, + { key: 'a', values: ['1', '3'] }, + { key: 'b', values: ['2'] }, ]); });