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

move deprecation description key mapping into filterByVisibility

Browse Source 
Signed-off-by: Colton Padden <colton.padden@fastmail.com>
This commit is contained in:
Colton Padden
2022-01-04 16:24:06 -05:00
parent 1136ed83e3
commit 63fa8f8371
9 changed files with 105 additions and 117 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/config-loader/src/lib/schema/collect.ts
+1 -1
View File
@@ -182,7 +182,7 @@ function compileTsSchemas(paths: string[]) {
program,
// All schemas should export a `Config` symbol
'Config',
// This enables usage of @visibility and @deprecated is doc comments
// This enables usage of @visibility and @deprecated in doc comments
{
required: true,
validationKeywords: ['visibility', 'deprecated'],
packages/config-loader/src/lib/schema/compile.test.ts
+6 -6
View File
@@ -40,7 +40,7 @@ describe('compileConfigSchemas', () => {
],
visibilityByDataPath: new Map(),
visibilityBySchemaPath: new Map(),
deprecationBySchemaPath: new Map(),
deprecationByDataPath: new Map(),
});
expect(validate([{ data: { b: 'b' }, context: 'test' }])).toEqual({
errors: [
@@ -54,7 +54,7 @@ describe('compileConfigSchemas', () => {
],
visibilityByDataPath: new Map(),
visibilityBySchemaPath: new Map(),
deprecationBySchemaPath: new Map(),
deprecationByDataPath: new Map(),
});
});
@@ -114,7 +114,7 @@ describe('compileConfigSchemas', () => {
'/properties/d/items': 'frontend',
}),
),
deprecationBySchemaPath: new Map(),
deprecationByDataPath: new Map(),
});
});
@@ -160,10 +160,10 @@ describe('compileConfigSchemas', () => {
{ data: { a: 'a', b: 'b', c: 'c', d: ['d'] }, context: 'test' },
]),
).toEqual({
deprecationBySchemaPath: new Map(
deprecationByDataPath: new Map(
Object.entries({
'/properties/a': 'deprecation reason for a',
'/properties/b': 'deprecation reason for b',
'/a': 'deprecation reason for a',
'/b': 'deprecation reason for b',
}),
),
visibilityByDataPath: new Map(),
packages/config-loader/src/lib/schema/compile.ts
+43 -27
View File
@@ -40,6 +40,7 @@ export function compileConfigSchemas(
// output during validation. We work around this by having this extra piece
// of state that we reset before each validation.
const visibilityByDataPath = new Map<string, ConfigVisibility>();
const deprecationByDataPath = new Map<string, string>();
const ajv = new Ajv({
allErrors: true,
@@ -47,28 +48,48 @@ export function compileConfigSchemas(
schemas: {
'https://backstage.io/schema/config-v1': true,
},
}).addKeyword({
keyword: 'visibility',
metaSchema: {
type: 'string',
enum: CONFIG_VISIBILITIES,
},
compile(visibility: ConfigVisibility) {
return (_data, context) => {
if (context?.dataPath === undefined) {
return false;
}
if (visibility && visibility !== 'backend') {
})
.addKeyword({
keyword: 'visibility',
metaSchema: {
type: 'string',
enum: CONFIG_VISIBILITIES,
},
compile(visibility: ConfigVisibility) {
return (_data, context) => {
if (context?.dataPath === undefined) {
return false;
}
if (visibility && visibility !== 'backend') {
const normalizedPath = context.dataPath.replace(
/\['?(.*?)'?\]/g,
(_, segment) => `/${segment}`,
);
visibilityByDataPath.set(normalizedPath, visibility);
}
return true;
};
},
})
.removeKeyword('deprecated') // remove `deprecated` keyword so that we can implement our own compiler
.addKeyword({
keyword: 'deprecated',
metaSchema: { type: 'string' },
compile(deprecationDescription: string) {
return (_data, context) => {
if (context?.dataPath === undefined) {
return false;
}
const normalizedPath = context.dataPath.replace(
/\['?(.*?)'?\]/g,
(_, segment) => `/${segment}`,
);
visibilityByDataPath.set(normalizedPath, visibility);
}
return true;
};
},
});
// create mapping of deprecation description and data path of property
deprecationByDataPath.set(normalizedPath, deprecationDescription);
return true;
};
},
});
for (const schema of schemas) {
try {
@@ -79,14 +100,8 @@ export function compileConfigSchemas(
}
const merged = mergeConfigSchemas(schemas.map(_ => _.value));
const validate = ajv.compile(merged);
const deprecationBySchemaPath = new Map<string, string>();
traverse(merged, (schema, path) => {
if ('deprecated' in schema) {
deprecationBySchemaPath.set(path, schema.deprecated);
}
});
const validate = ajv.compile(merged);
const visibilityBySchemaPath = new Map<string, ConfigVisibility>();
traverse(merged, (schema, path) => {
@@ -101,19 +116,20 @@ export function compileConfigSchemas(
visibilityByDataPath.clear();
const valid = validate(config);
if (!valid) {
return {
errors: validate.errors ?? [],
visibilityByDataPath: new Map(visibilityByDataPath),
visibilityBySchemaPath,
deprecationBySchemaPath,
deprecationByDataPath,
};
}
return {
visibilityByDataPath: new Map(visibilityByDataPath),
visibilityBySchemaPath,
deprecationBySchemaPath,
deprecationByDataPath,
};
};
}
packages/config-loader/src/lib/schema/filtering.test.ts
+29 -22
View File
@@ -16,11 +16,7 @@
import { JsonObject } from '@backstage/types';
import { ConfigVisibility } from './types';
import {
filterByVisibility,
filterErrorsByVisibility,
filterByDeprecated,
} from './filtering';
import { filterByVisibility, filterErrorsByVisibility } from './filtering';
const data = {
arr: ['f', 'b', 's'],
@@ -70,8 +66,8 @@ const visibility = new Map<string, ConfigVisibility>(
const deprecations = new Map<string, string>(
Object.entries({
'/properties/arr': 'deprecated array',
'/properties/objB/properties/never': 'deprecated nested property',
'/arr': 'deprecated array',
'/objB/never': 'deprecated nested property',
}),
);
@@ -196,9 +192,34 @@ describe('filterByVisibility', () => {
[['frontend', 'backend', 'secret'], { data, filteredKeys: [] }],
])('should filter correctly with %p', (filter, expected) => {
expect(
filterByVisibility(data, filter, visibility, undefined, true),
filterByVisibility(
data,
filter,
visibility,
deprecations,
undefined,
true,
false,
),
).toEqual(expected);
});
it('should include deprecated keys regardless of visibility', () => {
expect(
filterByVisibility(
data,
[],
visibility,
deprecations,
undefined,
true,
true,
).deprecatedKeys,
).toEqual([
{ key: 'arr', description: 'deprecated array' },
{ key: 'objB.never', description: 'deprecated nested property' },
]);
});
});
describe('filterErrorsByVisibility', () => {
@@ -392,17 +413,3 @@ describe('filterErrorsByVisibility', () => {
]);
});
});
describe('filterByDeprecated', () => {
it('should allow empty list of deprecations', () => {
expect(filterByDeprecated(data, new Map())).toEqual({ deprecatedKeys: [] });
});
it('should return a list of deprecated properties', () => {
expect(filterByDeprecated(data, deprecations)).toEqual({
deprecatedKeys: [
{ key: 'arr', description: 'deprecated array' },
{ key: 'objB.never', description: 'deprecated nested property' },
],
});
});
});
packages/config-loader/src/lib/schema/filtering.ts
+15 -31
View File
@@ -23,36 +23,6 @@ import {
ValidationError,
} from './types';
/**
* This filters data by deprecation status to only include those which have been deprecated
*
* @param data - configuration data
* @param deprecationBySchemaPath - mapping of schema path to deprecation description
* @returns deprecated configuration keys with their deprecation description
*/
export function filterByDeprecated(
data: JsonObject,
deprecationBySchemaPath: Map<string, string>,
withDeprecatedKeys: boolean = true,
): { deprecatedKeys: { key: string; description: string }[] } {
const deprecatedKeys = new Array<{ key: string; description: string }>();
if (!withDeprecatedKeys) {
return { deprecatedKeys };
}
deprecationBySchemaPath.forEach((desc, schemaPath) => {
// convert schema path to object path (/properties/techdocs/properties/storageUrl -> techdocs.storageUrl)
const propertyPath = schemaPath
.replace(/^\/properties\//, '') // remove leading `/properties/` entirely
.replace(/\/properties\//g, '.'); // replace all other `/properties/` with a `.`
if (has(data, propertyPath)) {
deprecatedKeys.push({ key: propertyPath, description: desc });
}
});
return { deprecatedKeys };
}
/**
* This filters data by visibility by discovering the visibility of each
* value, and then only keeping the ones that are specified in `includeVisibilities`.
@@ -61,10 +31,17 @@ export function filterByVisibility(
data: JsonObject,
includeVisibilities: ConfigVisibility[],
visibilityByDataPath: Map<string, ConfigVisibility>,
deprecationByDataPath: Map<string, string>,
transformFunc?: TransformFunc<number | string | boolean>,
withFilteredKeys?: boolean,
): { data: JsonObject; filteredKeys?: string[] } {
withDeprecatedKeys?: boolean,
): {
data: JsonObject;
filteredKeys?: string[];
deprecatedKeys?: { key: string; description: string }[];
} {
const filteredKeys = new Array<string>();
const deprecatedKeys = new Array<{ key: string; description: string }>();
function transform(
jsonVal: JsonValue,
@@ -75,6 +52,12 @@ export function filterByVisibility(
visibilityByDataPath.get(visibilityPath) ?? DEFAULT_CONFIG_VISIBILITY;
const isVisible = includeVisibilities.includes(visibility);
// deprecated keys are added regardless of visibility indicator
const deprecation = deprecationByDataPath.get(visibilityPath);
if (deprecation) {
deprecatedKeys.push({ key: filterPath, description: deprecation });
}
if (typeof jsonVal !== 'object') {
if (isVisible) {
if (transformFunc) {
@@ -140,6 +123,7 @@ export function filterByVisibility(
return {
filteredKeys: withFilteredKeys ? filteredKeys : undefined,
deprecatedKeys: withDeprecatedKeys ? deprecatedKeys : undefined,
data: (transform(data, '', '') as JsonObject) ?? {},
};
}
packages/config-loader/src/lib/schema/load.test.ts
-8
View File
@@ -65,7 +65,6 @@ describe('loadConfigSchema', () => {
{
data: { key1: 'a' },
context: 'test',
deprecatedKeys: [],
},
]);
expect(
@@ -79,7 +78,6 @@ describe('loadConfigSchema', () => {
data: { key1: 'X' },
context: 'test',
filteredKeys: ['key2'],
deprecatedKeys: [],
},
]);
expect(
@@ -92,7 +90,6 @@ describe('loadConfigSchema', () => {
data: { key1: 'X', key2: 'X' },
context: 'test',
filteredKeys: [],
deprecatedKeys: [],
},
]);
@@ -103,7 +100,6 @@ describe('loadConfigSchema', () => {
{
data: { key1: 'a' },
context: 'test',
deprecatedKeys: [],
},
]);
expect(() =>
@@ -152,7 +148,6 @@ describe('loadConfigSchema', () => {
{
data: { key1: 'a' },
context: 'test',
deprecatedKeys: [],
},
]);
expect(() => schema.process(configs, { visibility: ['secret'] })).toThrow(
@@ -205,7 +200,6 @@ describe('loadConfigSchema', () => {
{
data: { nested: [{}] },
context: 'test',
deprecatedKeys: [],
},
]);
expect(() => schema.process(mkConfig({ y: 1 }))).toThrow(
@@ -222,7 +216,6 @@ describe('loadConfigSchema', () => {
{
data: { nested: [{}] },
context: 'test',
deprecatedKeys: [],
},
]);
expect(
@@ -231,7 +224,6 @@ describe('loadConfigSchema', () => {
{
data: { nested: [{ y: 'aaa' }] },
context: 'test',
deprecatedKeys: [],
},
]);
expect(() =>
packages/config-loader/src/lib/schema/load.ts
+2 -8
View File
@@ -109,12 +109,9 @@ export async function loadConfigSchema(
data,
visibility,
result.visibilityByDataPath,
result.deprecationByDataPath,
valueTransform,
withFilteredKeys,
),
...filterByDeprecated(
data,
result.deprecationBySchemaPath,
withDeprecatedKeys,
),
}));
@@ -125,12 +122,9 @@ export async function loadConfigSchema(
data,
Array.from(CONFIG_VISIBILITIES),
result.visibilityByDataPath,
result.deprecationByDataPath,
valueTransform,
withFilteredKeys,
),
...filterByDeprecated(
data,
result.deprecationBySchemaPath,
withDeprecatedKeys,
),
}));
packages/config-loader/src/lib/schema/types.ts
+2 -2
View File
@@ -85,9 +85,9 @@ type ValidationResult = {
/**
* The deprecated options that were discovered during validation.
*
* The path in the key uses the form `/properties/<key>/items/additionalProperties/<leaf-key>`
* The path in the key uses the form `/<key>/<sub-key>/<array-index>/<leaf-key>`
*/
deprecationBySchemaPath: Map<string, string>;
deprecationByDataPath: Map<string, string>;
};
/**
packages/config/src/reader.ts
+7 -12
View File
@@ -87,20 +87,15 @@ export class ConfigReader implements Config {
const reader = new ConfigReader(data, context, previousReader);
reader.filteredKeys = filteredKeys;
// TODO(cmpadden) introduce `deprecatedKeys` and `notifiedDeprecatedKeys` & use logger
// TODO(cmpadden) `withDeprecatedKeys` is defaulting to false on app start
if (deprecatedKeys) {
for (const { key, description } of deprecatedKeys) {
if (description) {
// eslint-disable-next-line no-console
console.warn(
`The configuration key '${key}' is deprecated and may be removed soon. (reason: ${description})`,
);
} else {
// eslint-disable-next-line no-console
console.warn(
`The configuration key '${key}' is deprecated and may be removed soon.`,
);
}
// eslint-disable-next-line no-console
console.warn(
`The configuration key '${key}' of ${context} is deprecated and may be removed soon. ${
description || ''
}`,
);
}
}