From 3e2b3e99e4893acf309db0d949e4b97a55225857 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 15 Oct 2021 14:20:12 +0200 Subject: [PATCH] config-loader: unit tests for error visibility filtering + fixes Signed-off-by: Patrik Oldsberg --- .../src/lib/schema/filtering.test.ts | 194 +++++++++++++++++- .../config-loader/src/lib/schema/filtering.ts | 8 +- 2 files changed, 197 insertions(+), 5 deletions(-) diff --git a/packages/config-loader/src/lib/schema/filtering.test.ts b/packages/config-loader/src/lib/schema/filtering.test.ts index d4b3cd55b6..3b9e548026 100644 --- a/packages/config-loader/src/lib/schema/filtering.test.ts +++ b/packages/config-loader/src/lib/schema/filtering.test.ts @@ -16,7 +16,7 @@ import { JsonObject } from '@backstage/config'; import { ConfigVisibility } from './types'; -import { filterByVisibility } from './filtering'; +import { filterByVisibility, filterErrorsByVisibility } from './filtering'; const data = { arr: ['f', 'b', 's'], @@ -175,3 +175,195 @@ describe('filterByVisibility', () => { ).toEqual(expected); }); }); + +describe('filterErrorsByVisibility', () => { + it('should allow empty input', () => { + expect( + filterErrorsByVisibility(undefined, ['frontend'], new Map(), new Map()), + ).toEqual([]); + expect( + filterErrorsByVisibility( + ['my-error' as any], + undefined, + new Map(), + new Map(), + ), + ).toEqual(['my-error']); + expect( + filterErrorsByVisibility([], ['frontend'], new Map(), new Map()), + ).toEqual([]); + }); + + it('should filter generic errors', () => { + const errors = [ + { + keyword: 'something', + dataPath: '/a', + schemaPath: '#/properties/a/something', + params: {}, + message: 'a', + }, + { + keyword: 'something', + dataPath: '/b', + schemaPath: '#/properties/b/something', + params: {}, + message: 'b', + }, + { + keyword: 'something', + dataPath: '/c', + schemaPath: '#/properties/c/something', + params: {}, + message: 'c', + }, + ]; + const visibilityByDataPath = new Map([ + ['/a', 'frontend'], + ['/c', 'secret'], + ]); + + expect( + filterErrorsByVisibility( + errors, + undefined, + visibilityByDataPath, + new Map(), + ), + ).toEqual([ + expect.objectContaining({ message: 'a' }), + expect.objectContaining({ message: 'b' }), + expect.objectContaining({ message: 'c' }), + ]); + expect( + filterErrorsByVisibility( + errors, + ['frontend'], + visibilityByDataPath, + new Map(), + ), + ).toEqual([expect.objectContaining({ message: 'a' })]); + expect( + filterErrorsByVisibility( + errors, + ['backend'], + visibilityByDataPath, + new Map(), + ), + ).toEqual([expect.objectContaining({ message: 'b' })]); + expect( + filterErrorsByVisibility( + errors, + ['secret'], + visibilityByDataPath, + new Map(), + ), + ).toEqual([expect.objectContaining({ message: 'c' })]); + expect( + filterErrorsByVisibility(errors, [], visibilityByDataPath, new Map()), + ).toEqual([]); + }); + + it('should always forward structural type errors', () => { + const errors = [ + { + keyword: 'type', + dataPath: '/a', + schemaPath: '#/properties/a/type', + params: { type: 'number' }, + message: 'a', + }, + { + keyword: 'type', + dataPath: '/b', + schemaPath: '#/properties/b/type', + params: { type: 'string' }, + message: 'b', + }, + { + keyword: 'type', + dataPath: '/c', + schemaPath: '#/properties/c/type', + params: { type: 'array' }, + message: 'c', + }, + { + keyword: 'type', + dataPath: '/c', + schemaPath: '#/properties/c/type', + params: { type: 'object' }, + message: 'd', + }, + { + keyword: 'type', + dataPath: '/c', + schemaPath: '#/properties/c/type', + params: { type: 'null' }, + message: 'e', + }, + ]; + const visibilityByDataPath = new Map([ + ['/a', 'secret'], + ['/b', 'secret'], + ['/c', 'secret'], + ['/d', 'secret'], + ['/e', 'secret'], + ]); + + expect( + filterErrorsByVisibility( + errors, + ['frontend'], + visibilityByDataPath, + new Map(), + ), + ).toEqual([ + expect.objectContaining({ message: 'c' }), + expect.objectContaining({ message: 'd' }), + ]); + }); + + it('should filter requirement errors based on schema path', () => { + const errors = [ + { + keyword: 'required', + dataPath: '/a', + schemaPath: '#/properties/o/required', + params: { missingProperty: 'a' }, + message: 'a', + }, + { + keyword: 'required', + dataPath: '/b', + schemaPath: '#/properties/o/required', + params: { missingProperty: 'b' }, + message: 'b', + }, + { + keyword: 'required', + dataPath: '/c', + schemaPath: '#/properties/o/required', + params: { missingProperty: 'c' }, + message: 'c', + }, + ]; + const visibilityBySchemaPath = new Map([ + ['/properties/o', 'secret'], + ['/properties/o/properties/a', 'frontend'], + ['/properties/o/properties/b', 'secret'], + ['/properties/o/properties/c/properties/x', 'frontend'], + ]); + + expect( + filterErrorsByVisibility( + errors, + ['frontend'], + new Map(), + visibilityBySchemaPath, + ), + ).toEqual([ + expect.objectContaining({ message: 'a' }), + expect.objectContaining({ message: 'c' }), + ]); + }); +}); diff --git a/packages/config-loader/src/lib/schema/filtering.ts b/packages/config-loader/src/lib/schema/filtering.ts index 7cb122ee90..2b13f7ac3f 100644 --- a/packages/config-loader/src/lib/schema/filtering.ts +++ b/packages/config-loader/src/lib/schema/filtering.ts @@ -144,16 +144,16 @@ export function filterErrorsByVisibility( // and doesn't require us to properly trim the schema path. if (error.keyword === 'required') { const trimmedPath = error.schemaPath.slice(1, -'/required'.length); + const fullPath = `${trimmedPath}/properties/${error.params.missingProperty}`; if ( - visibleSchemaPaths.some(visiblePath => - visiblePath.startsWith(trimmedPath), - ) + visibleSchemaPaths.some(visiblePath => visiblePath.startsWith(fullPath)) ) { return true; } } - const vis = visibilityByDataPath.get(error.dataPath); + const vis = + visibilityByDataPath.get(error.dataPath) ?? DEFAULT_CONFIG_VISIBILITY; return vis && includeVisibilities.includes(vis); }); }