From fb24060299fd980e90a4a879bb898104b8e10107 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Fri, 7 Mar 2025 09:07:34 +0100 Subject: [PATCH] chore: small refactor to keep things simpler, and remove lodash Signed-off-by: benjdlambert --- .../src/ScaffolderPlugin.ts | 11 +- .../actions/builtin/fetch/template.ts | 4 +- .../actions/builtin/fetch/templateFile.ts | 4 +- .../tasks/NunjucksWorkflowRunner.ts | 4 +- .../scaffolder-backend/src/service/router.ts | 13 +- .../src/util/templating.test.ts | 30 +-- .../scaffolder-backend/src/util/templating.ts | 243 ++++++++++++------ 7 files changed, 198 insertions(+), 111 deletions(-) diff --git a/plugins/scaffolder-backend/src/ScaffolderPlugin.ts b/plugins/scaffolder-backend/src/ScaffolderPlugin.ts index e8f6630a85..4882b3cb2e 100644 --- a/plugins/scaffolder-backend/src/ScaffolderPlugin.ts +++ b/plugins/scaffolder-backend/src/ScaffolderPlugin.ts @@ -52,7 +52,10 @@ import { } from './scaffolder'; import { createRouter } from './service/router'; import { loggerToWinstonLogger } from './util/loggerToWinstonLogger'; -import { templateFilterImpls, templateGlobals } from './util/templating'; +import { + convertFiltersToRecord, + convertGlobalsToRecord, +} from './util/templating'; /** * Scaffolder plugin @@ -157,10 +160,12 @@ export const scaffolderPlugin = createBackendPlugin({ const integrations = ScmIntegrations.fromConfig(config); const templateExtensions = { - additionalTemplateFilters: templateFilterImpls( + additionalTemplateFilters: convertFiltersToRecord( additionalTemplateFilters, ), - additionalTemplateGlobals: templateGlobals(additionalTemplateGlobals), + additionalTemplateGlobals: convertGlobalsToRecord( + additionalTemplateGlobals, + ), }; const actions = [ // actions provided from other modules diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts index 7b11474d18..543c25ab3e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -31,7 +31,7 @@ import { isBinaryFile } from 'isbinaryfile'; import { SecureTemplater } from '../../../../lib/templating/SecureTemplater'; import createDefaultFilters from '../../../../lib/templating/filters'; import { examples } from './template.examples'; -import { templateFilterImpls } from '../../../../util/templating'; +import { convertFiltersToRecord } from '../../../../util/templating'; /** * Downloads a skeleton, templates variables into file and directory names and content. @@ -53,7 +53,7 @@ export function createFetchTemplateAction(options: { additionalTemplateGlobals, } = options; - const defaultTemplateFilters = templateFilterImpls( + const defaultTemplateFilters = convertFiltersToRecord( createDefaultFilters({ integrations }), ); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFile.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFile.ts index 8a07896dbf..f3b98d148a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFile.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFile.ts @@ -28,7 +28,7 @@ import { SecureTemplater } from '../../../../lib/templating/SecureTemplater'; import createDefaultFilters from '../../../../lib/templating/filters'; import path from 'path'; import fs from 'fs-extra'; -import { templateFilterImpls } from '../../../../util/templating'; +import { convertFiltersToRecord } from '../../../../util/templating'; /** * Downloads a single file and templates variables into file. @@ -49,7 +49,7 @@ export function createFetchTemplateFileAction(options: { additionalTemplateGlobals, } = options; - const defaultTemplateFilters = templateFilterImpls( + const defaultTemplateFilters = convertFiltersToRecord( createDefaultFilters({ integrations }), ); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index f2979cea43..b771c5ded1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -60,7 +60,7 @@ import { scaffolderActionRules } from '../../service/rules'; import { createCounterMetric, createHistogramMetric } from '../../util/metrics'; import { BackstageLoggerTransport, WinstonLogger } from './logger'; import { loggerToWinstonLogger } from '../../util/loggerToWinstonLogger'; -import { templateFilterImpls } from '../../util/templating'; +import { convertFiltersToRecord } from '../../util/templating'; type NunjucksWorkflowRunnerOptions = { workingDirectory: string; @@ -152,7 +152,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { private readonly defaultTemplateFilters: Record; constructor(private readonly options: NunjucksWorkflowRunnerOptions) { - this.defaultTemplateFilters = templateFilterImpls( + this.defaultTemplateFilters = convertFiltersToRecord( createDefaultFilters({ integrations: this.options.integrations, }), diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 3c6db3dd03..35782ea441 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -110,7 +110,10 @@ import { } from './helpers'; import { scaffolderActionRules, scaffolderTemplateRules } from './rules'; import { HostDiscovery } from '@backstage/backend-defaults/discovery'; -import { templateFilterImpls, templateGlobals } from '../util/templating'; +import { + convertFiltersToRecord, + convertGlobalsToRecord, +} from '../util/templating'; /** * @@ -369,8 +372,12 @@ export async function createRouter( const actionRegistry = new TemplateActionRegistry(); const templateExtensions = { - additionalTemplateFilters: templateFilterImpls(additionalTemplateFilters), - additionalTemplateGlobals: templateGlobals(additionalTemplateGlobals), + additionalTemplateFilters: convertFiltersToRecord( + additionalTemplateFilters, + ), + additionalTemplateGlobals: convertGlobalsToRecord( + additionalTemplateGlobals, + ), }; const workers: TaskWorker[] = []; diff --git a/plugins/scaffolder-backend/src/util/templating.test.ts b/plugins/scaffolder-backend/src/util/templating.test.ts index f5d6431f28..5ac6db601b 100644 --- a/plugins/scaffolder-backend/src/util/templating.test.ts +++ b/plugins/scaffolder-backend/src/util/templating.test.ts @@ -21,11 +21,11 @@ import { createTemplateGlobalValue, } from '@backstage/plugin-scaffolder-node/alpha'; import { - templateFilterImpls, - templateFilterMetadata, - templateGlobalFunctionMetadata, - templateGlobals, - templateGlobalValueMetadata, + convertFiltersToRecord, + extractFilterMetadata, + extractGlobalFunctionMetadata, + convertGlobalsToRecord, + extractGlobalValueMetadata, } from './templating'; import { JsonValue } from '@backstage/types'; import builtInFilters from '../lib/templating/filters'; @@ -37,8 +37,8 @@ describe('templating utilities', () => { const integrations = ScmIntegrations.fromConfig(new ConfigReader({})); const filters = builtInFilters({ integrations }); it('generates equivalent filter metadata', () => { - const metadata = templateFilterMetadata(filters); - expect(metadata).toMatchObject(templateFilterMetadata(filters)); + const metadata = extractFilterMetadata(filters); + expect(metadata).toMatchObject(extractFilterMetadata(filters)); }); }); describe('api filters', () => { @@ -71,7 +71,7 @@ describe('templating utilities', () => { }), ]; it('extracts filter implementations', () => { - const impls = templateFilterImpls(filters); + const impls = convertFiltersToRecord(filters); expect(impls).toHaveProperty('nop'); expect(impls.nop(42)).toBe(42); expect(impls).toHaveProperty('nul'); @@ -81,7 +81,7 @@ describe('templating utilities', () => { expect(impls.repeat('foo', 3, '-')).toBe('foo-foo-foo'); }); it('extracts filter metadata', () => { - const metadata = templateFilterMetadata(filters); + const metadata = extractFilterMetadata(filters); expect(metadata).toHaveProperty('nop'); expect(metadata.nop).toHaveProperty('description', 'nop'); expect(metadata.nop).toHaveProperty( @@ -121,14 +121,14 @@ describe('templating utilities', () => { nul: _ => null, } as Record; it('extracts filter implementations', () => { - const impls = templateFilterImpls(filters); + const impls = convertFiltersToRecord(filters); expect(impls).toHaveProperty('nop'); expect(impls.nop(42)).toBe(42); expect(impls).toHaveProperty('nul'); expect(impls.nul('anything')).toBeNull(); }); it('extracts filter metadata', () => { - const metadata = templateFilterMetadata(filters); + const metadata = extractFilterMetadata(filters); expect(metadata).toHaveProperty('nop', {}); expect(metadata).toHaveProperty('nul', {}); }); @@ -154,7 +154,7 @@ describe('templating utilities', () => { }), ]; it('extracts global function metadata', () => { - const metadata = templateGlobalFunctionMetadata(documentedGlobals); + const metadata = extractGlobalFunctionMetadata(documentedGlobals); expect(metadata).toHaveProperty('foo'); expect(metadata.foo).toHaveProperty('description', 'foo something'); expect(metadata).not.toHaveProperty('bar'); @@ -171,13 +171,13 @@ describe('templating utilities', () => { ); }); it('extracts global value metadata', () => { - const metadata = templateGlobalValueMetadata(documentedGlobals); + const metadata = extractGlobalValueMetadata(documentedGlobals); expect(metadata).not.toHaveProperty('foo'); expect(metadata).toHaveProperty('bar'); expect(metadata.bar).toHaveProperty('description', 'bar value'); expect(metadata).not.toHaveProperty('respond'); }); - const globals = templateGlobals(documentedGlobals); + const globals = convertGlobalsToRecord(documentedGlobals); it('extracts documented globals', () => { expect(globals).toHaveProperty('foo'); expect((globals.foo as (x: any) => string)('something')).toBe( @@ -189,6 +189,6 @@ describe('templating utilities', () => { expect((globals.respond as Function)('knock knock')).toBe("who's there?"); }); it('extracts backward-compatible/undocumented globals', () => { - expect(templateGlobals(globals)).toBe(globals); + expect(convertGlobalsToRecord(globals)).toBe(globals); }); }); diff --git a/plugins/scaffolder-backend/src/util/templating.ts b/plugins/scaffolder-backend/src/util/templating.ts index 71780ec104..1f97e98b83 100644 --- a/plugins/scaffolder-backend/src/util/templating.ts +++ b/plugins/scaffolder-backend/src/util/templating.ts @@ -26,30 +26,27 @@ import { } from '@backstage/plugin-scaffolder-node/alpha'; import { JsonValue } from '@backstage/types'; import { Schema } from 'jsonschema'; -import { - filter, - fromPairs, - isEmpty, - keyBy, - mapValues, - negate, - pick, - pickBy, - toPairs, - wrap, -} from 'lodash'; -import { z, ZodType } from 'zod'; +import { ZodType, z } from 'zod'; import zodToJsonSchema from 'zod-to-json-schema'; -export function templateFilterImpls( +/** + * Converts template filters to a record of filter functions + */ +export function convertFiltersToRecord( filters?: Record | CreatedTemplateFilter[], ): Record { if (!filters) { return {}; } + if (Array.isArray(filters)) { - return mapValues(keyBy(filters, 'id'), f => f.filter as TemplateFilter); + const result: Record = {}; + for (const filter of filters) { + result[filter.id] = filter.filter as TemplateFilter; + } + return result; } + return filters; } @@ -62,29 +59,52 @@ type ExportFilterSchema = { input?: Schema; } & ExportFunctionSchema; -function zodFunctionToJsonSchema( +/** + * Converts a Zod function schema to JSON schema + */ +function convertZodFunctionToJsonSchema( t: ReturnType>, ): ExportFunctionSchema { const args = (t.parameters().items as ZodType[]).map( zt => zodToJsonSchema(zt) as Schema, ); - const output = wrap(t.returnType(), rt => - rt._unknown ? undefined : (zodToJsonSchema(rt) as Schema), - )(); - return pickBy({ output, arguments: args }, negate(isEmpty)); + + let output: Schema | undefined = undefined; + const returnType = t.returnType(); + if (!returnType._unknown) { + output = zodToJsonSchema(returnType) as Schema; + } + + const result: ExportFunctionSchema = {}; + if (args.length > 0) { + result.arguments = args; + } + if (output) { + result.output = output; + } + + return result; } -function toExportFilterSchema( +/** + * Converts a function schema to a filter schema + */ +function convertToFilterSchema( fnSchema: ExportFunctionSchema, ): ExportFilterSchema { if (fnSchema.arguments?.length) { const [input, ...rest] = fnSchema.arguments; - const { output } = fnSchema; - return { - input, - ...(rest.length ? { arguments: rest } : {}), - ...(output ? { output } : {}), - }; + const result: ExportFilterSchema = { input }; + + if (rest.length > 0) { + result.arguments = rest; + } + + if (fnSchema.output) { + result.output = fnSchema.output; + } + + return result; } return fnSchema; } @@ -96,30 +116,56 @@ type ExportFilter = Pick< schema?: ExportFilterSchema; }; -export function templateFilterMetadata( +/** + * Extracts metadata from template filters + */ +export function extractFilterMetadata( filters?: Record | CreatedTemplateFilter[], ): Record { if (!filters) { return {}; } + if (Array.isArray(filters)) { - return mapValues( - keyBy(filters, 'id'), - >(f: F): ExportFilter => { - const schema = f.schema - ? toExportFilterSchema(zodFunctionToJsonSchema(f.schema(z))) - : undefined; - return { - ...pick(f, 'description', 'examples'), - ...pickBy({ schema }), - }; - }, - ); + const result: Record = {}; + + for (const filter of filters) { + const metadata: ExportFilter = {}; + + if (filter.description) { + metadata.description = filter.description; + } + + if (filter.examples) { + metadata.examples = filter.examples; + } + + if (filter.schema) { + metadata.schema = convertToFilterSchema( + convertZodFunctionToJsonSchema(filter.schema(z)), + ); + } + + result[filter.id] = metadata; + } + + return result; } - return mapValues(filters, _ => ({})); + + // For non-array filters, return empty metadata + const result: Record = {}; + for (const key in filters) { + if (filters.hasOwnProperty(key)) { + result[key] = {}; + } + } + return result; } -function isGlobalFunctionInfo( +/** + * Checks if a global is a function + */ +function isGlobalFunction( global: CreatedTemplateGlobal, ): global is CreatedTemplateGlobalFunction< TemplateGlobalFunctionSchema | undefined, @@ -128,9 +174,10 @@ function isGlobalFunctionInfo( return 'fn' in global; } -type GlobalRecordRow = [string, TemplateGlobal]; - -export function templateGlobalFunctionMetadata( +/** + * Extracts metadata from template global functions + */ +export function extractGlobalFunctionMetadata( globals?: Record | CreatedTemplateGlobal[], ): Record< string, @@ -141,64 +188,92 @@ export function templateGlobalFunctionMetadata( if (!globals) { return {}; } - if (Array.isArray(globals)) { - return mapValues(keyBy(filter(globals, isGlobalFunctionInfo), 'id'), v => { - const schema = v.schema - ? zodFunctionToJsonSchema(v.schema(z)) - : undefined; - return { - ...pick(v, 'description', 'examples'), - ...pickBy({ schema }), - }; - }); + if (Array.isArray(globals)) { + const result: Record = {}; + + for (const global of globals) { + if (isGlobalFunction(global)) { + const metadata: any = { + description: global.description, + examples: global.examples, + }; + + if (global.schema) { + metadata.schema = convertZodFunctionToJsonSchema(global.schema(z)); + } + + result[global.id] = metadata; + } + } + + return result; } - const rows = toPairs(globals) as GlobalRecordRow[]; - const fns = rows.filter(([_, g]) => typeof g === 'function') as [ - string, - Exclude, - ][]; - return fromPairs(fns.map(([k, _]) => [k, {}])); + + // For non-array globals, extract function metadata + const result: Record = {}; + for (const key in globals) { + if (typeof globals[key] === 'function') { + result[key] = {}; + } + } + return result; } -export function templateGlobalValueMetadata( +/** + * Extracts metadata from template global values + */ +export function extractGlobalValueMetadata( globals?: Record | CreatedTemplateGlobal[], ): Record> { if (!globals) { return {}; } + if (Array.isArray(globals)) { - return mapValues( - keyBy( - filter( - globals, - negate(isGlobalFunctionInfo), - ) as CreatedTemplateGlobalValue[], - 'id', - ), - v => pick(v, 'value', 'description'), - ); + const result: Record> = {}; + + for (const global of globals) { + if (!isGlobalFunction(global)) { + result[global.id] = { + value: (global as CreatedTemplateGlobalValue).value, + description: global.description, + }; + } + } + + return result; } - const rows = toPairs(globals) as GlobalRecordRow[]; - const vals = rows.filter(([_, g]) => typeof g !== 'function') as [ - string, - JsonValue, - ][]; - return fromPairs(vals.map(([k, value]) => [k, { value }])); + + // For non-array globals, extract value metadata + const result: Record> = {}; + for (const key in globals) { + if (typeof globals[key] !== 'function') { + result[key] = { value: globals[key] as JsonValue }; + } + } + return result; } -export function templateGlobals( +/** + * Converts template globals to a record of global values and functions + */ +export function convertGlobalsToRecord( globals?: Record | CreatedTemplateGlobal[], ): Record { if (!globals) { return {}; } + if (!Array.isArray(globals)) { return globals; } - return mapValues(keyBy(globals, 'id'), v => - isGlobalFunctionInfo(v) - ? (v.fn as TemplateGlobal) - : (v as CreatedTemplateGlobalValue).value, - ); + + const result: Record = {}; + for (const global of globals) { + result[global.id] = isGlobalFunction(global) + ? (global.fn as TemplateGlobal) + : (global as CreatedTemplateGlobalValue).value; + } + return result; }