From 29097461476ac14e24032707ba2d8fb3a6eaa393 Mon Sep 17 00:00:00 2001 From: Mark David Avery Date: Wed, 27 Apr 2022 11:02:40 -0700 Subject: [PATCH 1/7] fix: Updated entity query param transform to handle keys with '.' in them. Signed-off-by: Mark David Avery --- .changeset/shiny-apes-design.md | 5 +++++ .../parseEntityTransformParams.test.ts | 22 ++++++++++++++++++- .../request/parseEntityTransformParams.ts | 22 +++++++++++++++++-- 3 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 .changeset/shiny-apes-design.md diff --git a/.changeset/shiny-apes-design.md b/.changeset/shiny-apes-design.md new file mode 100644 index 0000000000..fe3d7d84f2 --- /dev/null +++ b/.changeset/shiny-apes-design.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Updated entity query param transform to handle keys with '.' in them. This will allow for querying based of annotations such as 'backstage.io/orgin-location' for instance diff --git a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts index 1d5d41fc03..8ecae1655f 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts @@ -24,6 +24,9 @@ describe('parseEntityTransformParams', () => { metadata: { name: 'n', tags: ['t1', 't2'], + annotations: { + 'test.com/url-like': 'ul1', + }, }, spec: { type: 't', @@ -61,7 +64,24 @@ describe('parseEntityTransformParams', () => { parseEntityTransformParams({ fields: 'kind,metadata.name' })!(entity), ).toEqual({ kind: 'k', metadata: { name: 'n' } }); expect(parseEntityTransformParams({ fields: 'metadata' })!(entity)).toEqual( - { metadata: { name: 'n', tags: ['t1', 't2'] } }, + { + metadata: { + name: 'n', + tags: ['t1', 't2'], + annotations: { 'test.com/url-like': 'ul1' }, + }, + }, ); }); + + it('supports dot notated feilds properly', () => { + expect( + parseEntityTransformParams({ + fields: 'kind,metadata.annotations.test.com/url-like', + })!(entity), + ).toEqual({ + kind: 'k', + metadata: { annotations: { 'test.com/url-like': 'ul1' } }, + }); + }); }); diff --git a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts index 9935da07a8..adb7b8a133 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts @@ -20,6 +20,22 @@ import lodash from 'lodash'; import { RecursivePartial } from '../../util/RecursivePartial'; import { parseStringsParam } from './common'; +function getPathArray(input: Entity, field: string) { + const pathArray = []; + let currentPathPart = ''; + + for (const pathPart of field.split('.')) { + currentPathPart += pathPart; + + if (lodash.has(input, pathArray.concat(currentPathPart))) { + pathArray.push(currentPathPart); + currentPathPart = ''; + } + } + + return pathArray; +} + export function parseEntityTransformParams( params: Record, ): ((entity: Entity) => Entity) | undefined { @@ -46,9 +62,11 @@ export function parseEntityTransformParams( const output: RecursivePartial = {}; for (const field of fields) { - const value = lodash.get(input, field); + const pathArray = getPathArray(input, field); + + const value = lodash.get(input, pathArray); if (value !== undefined) { - lodash.set(output, field, value); + lodash.set(output, pathArray, value); } } From 61eed179c7fe8b396e980ec833d3f678bda2bd2a Mon Sep 17 00:00:00 2001 From: Mark David Avery Date: Tue, 3 May 2022 17:43:11 -0700 Subject: [PATCH 2/7] fix: updated parseEntityTransformParams dot in key notation to properly handle multiple dot notated keys Also added additional tests to handle varried scenarios. Signed-off-by: Mark David Avery --- .changeset/shiny-apes-design.md | 2 +- .../parseEntityTransformParams.test.ts | 55 +++++++++++++++++-- .../request/parseEntityTransformParams.ts | 2 + 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/.changeset/shiny-apes-design.md b/.changeset/shiny-apes-design.md index fe3d7d84f2..69dfbf57ab 100644 --- a/.changeset/shiny-apes-design.md +++ b/.changeset/shiny-apes-design.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog-backend': patch --- -Updated entity query param transform to handle keys with '.' in them. This will allow for querying based of annotations such as 'backstage.io/orgin-location' for instance +Updated parseEntityTransformParams to handle keys with '.' in them. This will allow for querying of entities based off annotations such as 'backstage.io/orgin-location' or other entity field keys that have '.' in them. diff --git a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts index 8ecae1655f..65f85639e1 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts @@ -25,7 +25,9 @@ describe('parseEntityTransformParams', () => { name: 'n', tags: ['t1', 't2'], annotations: { - 'test.com/url-like': 'ul1', + 'example.test/url-like-key': 'ul1', + 'example.com/other-url-like-key': 'ul2', + 'other-example.test/next-url-like-key': 'ul3', }, }, spec: { @@ -68,20 +70,63 @@ describe('parseEntityTransformParams', () => { metadata: { name: 'n', tags: ['t1', 't2'], - annotations: { 'test.com/url-like': 'ul1' }, + annotations: { + 'example.test/url-like-key': 'ul1', + 'example.com/other-url-like-key': 'ul2', + 'other-example.test/next-url-like-key': 'ul3', + }, }, }, ); }); - it('supports dot notated feilds properly', () => { + it('supports dot notated fields properly', () => { expect( parseEntityTransformParams({ - fields: 'kind,metadata.annotations.test.com/url-like', + fields: 'kind,metadata.annotations.example.com/other-url-like-key', })!(entity), ).toEqual({ kind: 'k', - metadata: { annotations: { 'test.com/url-like': 'ul1' } }, + metadata: { annotations: { 'example.com/other-url-like-key': 'ul2' } }, + }); + }); + + it('supports nested dot notated fields properly', () => { + entity.spec = { + ...entity.spec, + 'field-with.dot': 'fd1', + 'other-field-with.dot-also': { + subItem: 'fd2.sub', + 'subite.with/dot': 'fd2.sub.dot', + }, + 'third-field-with.dot-again': 'fd3', + type: 't', + }; + + expect( + parseEntityTransformParams({ + fields: 'kind,spec.other-field-with.dot-also', + })!(entity), + ).toEqual({ + kind: 'k', + spec: { + 'other-field-with.dot-also': { + subItem: 'fd2.sub', + 'subite.with/dot': 'fd2.sub.dot', + }, + }, + }); + expect( + parseEntityTransformParams({ + fields: 'kind,spec.other-field-with.dot-also.subite.with/dot', + })!(entity), + ).toEqual({ + kind: 'k', + spec: { + 'other-field-with.dot-also': { + 'subite.with/dot': 'fd2.sub.dot', + }, + }, }); }); }); diff --git a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts index adb7b8a133..e09289d5c0 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts @@ -30,6 +30,8 @@ function getPathArray(input: Entity, field: string) { if (lodash.has(input, pathArray.concat(currentPathPart))) { pathArray.push(currentPathPart); currentPathPart = ''; + } else { + currentPathPart += '.'; } } From 69bf1a83edb1f747d20e36f8416b2bd3b66a87b0 Mon Sep 17 00:00:00 2001 From: Mark David Avery Date: Wed, 4 May 2022 14:51:14 -0700 Subject: [PATCH 3/7] refactor: switched to a reduce, and now returning the value as well as the path array Signed-off-by: Mark David Avery --- .../request/parseEntityTransformParams.ts | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts index e09289d5c0..9bbe9e7861 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts @@ -20,22 +20,20 @@ import lodash from 'lodash'; import { RecursivePartial } from '../../util/RecursivePartial'; import { parseStringsParam } from './common'; -function getPathArray(input: Entity, field: string) { - const pathArray = []; - let currentPathPart = ''; +function getPathArrayAndValue(input: Entity, field: string) { + return field.split('.').reduce( + ([pathArray, inputSubset], pathPart, index, fieldParts) => { + if (Object.hasOwn(inputSubset, pathPart)) { + return [pathArray.concat(pathPart), inputSubset[pathPart]]; + } else if (fieldParts[index + 1] !== undefined) { + fieldParts[index + 1] = `${pathPart}.${fieldParts[index + 1]}`; + return [pathArray, inputSubset]; + } - for (const pathPart of field.split('.')) { - currentPathPart += pathPart; - - if (lodash.has(input, pathArray.concat(currentPathPart))) { - pathArray.push(currentPathPart); - currentPathPart = ''; - } else { - currentPathPart += '.'; - } - } - - return pathArray; + return [pathArray, undefined]; + }, + [[] as unknown as string, input as any], + ); } export function parseEntityTransformParams( @@ -64,9 +62,8 @@ export function parseEntityTransformParams( const output: RecursivePartial = {}; for (const field of fields) { - const pathArray = getPathArray(input, field); + const [pathArray, value] = getPathArrayAndValue(input, field); - const value = lodash.get(input, pathArray); if (value !== undefined) { lodash.set(output, pathArray, value); } From 7bc5db06c8412a7dc2a5ff99cbe089fe90be88ed Mon Sep 17 00:00:00 2001 From: Mark David Avery Date: Wed, 4 May 2022 14:54:26 -0700 Subject: [PATCH 4/7] fix: had set the wrong type for the return value Signed-off-by: Mark David Avery --- .../src/service/request/parseEntityTransformParams.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts index 9bbe9e7861..a7ee3cad9b 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts @@ -32,7 +32,7 @@ function getPathArrayAndValue(input: Entity, field: string) { return [pathArray, undefined]; }, - [[] as unknown as string, input as any], + [[] as string[], input as any], ); } From 74c218a54f0451211235250787bffaa9b5554eaf Mon Sep 17 00:00:00 2001 From: Mark David Avery Date: Thu, 5 May 2022 07:11:32 -0700 Subject: [PATCH 5/7] fix: node 14 is still supported and does not have hasOwn Signed-off-by: Mark David Avery --- .../src/service/request/parseEntityTransformParams.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts index a7ee3cad9b..5d11ea84b8 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts @@ -23,7 +23,7 @@ import { parseStringsParam } from './common'; function getPathArrayAndValue(input: Entity, field: string) { return field.split('.').reduce( ([pathArray, inputSubset], pathPart, index, fieldParts) => { - if (Object.hasOwn(inputSubset, pathPart)) { + if (inputSubset.hasOwnProperty(pathPart)) { return [pathArray.concat(pathPart), inputSubset[pathPart]]; } else if (fieldParts[index + 1] !== undefined) { fieldParts[index + 1] = `${pathPart}.${fieldParts[index + 1]}`; From acfd217142613b52efa02f807dfa9f3b9804072e Mon Sep 17 00:00:00 2001 From: Mark David Avery Date: Fri, 6 May 2022 08:26:45 -0700 Subject: [PATCH 6/7] spec: added additional tests to account for potential lookups of keys under values Signed-off-by: Mark David Avery --- .../parseEntityTransformParams.test.ts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts index 65f85639e1..ae1c8855ea 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts @@ -100,7 +100,6 @@ describe('parseEntityTransformParams', () => { 'subite.with/dot': 'fd2.sub.dot', }, 'third-field-with.dot-again': 'fd3', - type: 't', }; expect( @@ -129,4 +128,23 @@ describe('parseEntityTransformParams', () => { }, }); }); + + it('does not return a sub key if an incorrect longer key is requested', () => { + entity.spec = { + ...entity.spec, + 'field-with.dot': 'fd1', + plain: true, + numLike: 4, + arrayLike: [4, 5], + }; + + expect( + parseEntityTransformParams({ + fields: + 'kind,spec.field-withdot.other.item,kind.other.other,spec.plain.other,spec.numLike.other,spec.arrayLike.other', + })!(entity), + ).toEqual({ + kind: 'k', + }); + }); }); From fe2d99e471e9cca7a97cb06d1769006eb65dc378 Mon Sep 17 00:00:00 2001 From: Mark David Avery Date: Fri, 6 May 2022 09:04:25 -0700 Subject: [PATCH 7/7] fix: cleaned up the tests and added in a lowdash 'hasOwn' alternative Signed-off-by: Mark David Avery --- .../parseEntityTransformParams.test.ts | 50 +++++++++++++++---- .../request/parseEntityTransformParams.ts | 2 +- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts index ae1c8855ea..3dce579ae5 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.test.ts @@ -132,19 +132,49 @@ describe('parseEntityTransformParams', () => { it('does not return a sub key if an incorrect longer key is requested', () => { entity.spec = { ...entity.spec, + strValue: 'st1', + boolValue: true, + numValue: 4, + arrValue: [4, 5], + nullValue: null, + undefValue: undefined, 'field-with.dot': 'fd1', - plain: true, - numLike: 4, - arrayLike: [4, 5], }; expect( - parseEntityTransformParams({ - fields: - 'kind,spec.field-withdot.other.item,kind.other.other,spec.plain.other,spec.numLike.other,spec.arrayLike.other', - })!(entity), - ).toEqual({ - kind: 'k', - }); + parseEntityTransformParams({ fields: 'kind,spec.strValue.other' })!( + entity, + ), + ).toEqual({ kind: 'k' }); + expect( + parseEntityTransformParams({ fields: 'kind,spec.boolValue.other' })!( + entity, + ), + ).toEqual({ kind: 'k' }); + expect( + parseEntityTransformParams({ fields: 'kind,spec.numValue.other' })!( + entity, + ), + ).toEqual({ kind: 'k' }); + expect( + parseEntityTransformParams({ fields: 'kind,spec.arrValue.other' })!( + entity, + ), + ).toEqual({ kind: 'k' }); + expect( + parseEntityTransformParams({ fields: 'kind,spec.nullValue.other' })!( + entity, + ), + ).toEqual({ kind: 'k' }); + expect( + parseEntityTransformParams({ fields: 'kind,spec.undefValue.other' })!( + entity, + ), + ).toEqual({ kind: 'k' }); + expect( + parseEntityTransformParams({ fields: 'kind,spec.field-with.dot.other' })!( + entity, + ), + ).toEqual({ kind: 'k' }); }); }); diff --git a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts index 5d11ea84b8..cef6e5ef64 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityTransformParams.ts @@ -23,7 +23,7 @@ import { parseStringsParam } from './common'; function getPathArrayAndValue(input: Entity, field: string) { return field.split('.').reduce( ([pathArray, inputSubset], pathPart, index, fieldParts) => { - if (inputSubset.hasOwnProperty(pathPart)) { + if (lodash.hasIn(inputSubset, pathPart)) { return [pathArray.concat(pathPart), inputSubset[pathPart]]; } else if (fieldParts[index + 1] !== undefined) { fieldParts[index + 1] = `${pathPart}.${fieldParts[index + 1]}`;