From 173ef97b483933477078927cba82f2b3d2644099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 14 Apr 2026 13:59:39 +0200 Subject: [PATCH] Address second round of PR review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Close async iterators in ModelHolder after reading first value to prevent resource leaks - Catch exceptions from validateMetaSchema in Zod refine predicate so validation errors flow through Zod's normal issue reporting - Warn on duplicate catalog model layer IDs instead of silently dropping later entries - Replace `as any` with `as JsonObject` for schema import in scaffolder template model layer Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Fredrik Adelöw --- packages/catalog-model/report-alpha.api.md | 2 +- .../catalog-model/src/model/jsonSchema/zod.ts | 15 +++++++---- .../src/model/sources/CatalogModelSources.ts | 19 ++++++++++--- .../catalog-backend/src/model/ModelHolder.ts | 27 ++++++++++--------- plugins/scaffolder-common/src/catalogModel.ts | 3 ++- 5 files changed, 42 insertions(+), 24 deletions(-) diff --git a/packages/catalog-model/report-alpha.api.md b/packages/catalog-model/report-alpha.api.md index 0150301adc..903f0770a6 100644 --- a/packages/catalog-model/report-alpha.api.md +++ b/packages/catalog-model/report-alpha.api.md @@ -395,7 +395,7 @@ export function createCatalogModelLayerBuilder(options: { build(): CatalogModelLayer; }; -// @alpha (undocumented) +// @alpha export const defaultCatalogEntityModel: CatalogModelLayer; // @alpha diff --git a/packages/catalog-model/src/model/jsonSchema/zod.ts b/packages/catalog-model/src/model/jsonSchema/zod.ts index c4f7ff3e78..ea82263cac 100644 --- a/packages/catalog-model/src/model/jsonSchema/zod.ts +++ b/packages/catalog-model/src/model/jsonSchema/zod.ts @@ -25,8 +25,13 @@ export const jsonObjectSchema = z message: 'Invalid JSON schema', }); -export const jsonSchemaSchema = z - .record(z.string(), z.unknown()) - .refine((x): x is JsonObject => validateMetaSchema(x), { - message: 'Invalid JSON schema', - }); +export const jsonSchemaSchema = z.record(z.string(), z.unknown()).refine( + (x): x is JsonObject => { + try { + return validateMetaSchema(x); + } catch { + return false; + } + }, + { message: 'Invalid JSON schema' }, +); diff --git a/packages/catalog-model/src/model/sources/CatalogModelSources.ts b/packages/catalog-model/src/model/sources/CatalogModelSources.ts index 9ef035c336..6c021be5e4 100644 --- a/packages/catalog-model/src/model/sources/CatalogModelSources.ts +++ b/packages/catalog-model/src/model/sources/CatalogModelSources.ts @@ -19,7 +19,6 @@ import { defaultCatalogEntityModel } from '../defaultCatalogEntityModel'; import { StaticCatalogModelSource } from './StaticCatalogModelSource'; import { CatalogModelSource } from './types'; import { CatalogModelLayer } from '../types'; -import uniqBy from 'lodash/uniqBy'; /** * A helper for creating common catalog model sources. @@ -39,9 +38,21 @@ export class CatalogModelSources { * included automatically). */ static static(layers: CatalogModelLayer[]): CatalogModelSource { - return new StaticCatalogModelSource( - uniqBy([...layers, defaultCatalogEntityModel], 'layerId'), - ); + const allLayers = [...layers, defaultCatalogEntityModel]; + const seen = new Set(); + const deduped: CatalogModelLayer[] = []; + for (const layer of allLayers) { + if (seen.has(layer.layerId)) { + // eslint-disable-next-line no-console + console.warn( + `Duplicate catalog model layer ID "${layer.layerId}" detected; only the first occurrence will be used`, + ); + } else { + seen.add(layer.layerId); + deduped.push(layer); + } + } + return new StaticCatalogModelSource(deduped); } private constructor() { diff --git a/plugins/catalog-backend/src/model/ModelHolder.ts b/plugins/catalog-backend/src/model/ModelHolder.ts index e12b6fc4a7..146ef59f85 100644 --- a/plugins/catalog-backend/src/model/ModelHolder.ts +++ b/plugins/catalog-backend/src/model/ModelHolder.ts @@ -57,19 +57,20 @@ export class ModelHolder { // model source events during the lifetime of the plugin. try { const layers = await Promise.all( - sources.map(source => - source - .read({ signal: shutdownController.signal }) - .next() - .then(result => { - readyCount += 1; - const ls = result.value?.layers ?? []; - for (const layer of ls) { - logger.info(`Loaded catalog model layer: ${layer.layerId}`); - } - return ls; - }), - ), + sources.map(async source => { + const iter = source.read({ signal: shutdownController.signal }); + try { + const result = await iter.next(); + readyCount += 1; + const ls = result.value?.layers ?? []; + for (const layer of ls) { + logger.info(`Loaded catalog model layer: ${layer.layerId}`); + } + return ls; + } finally { + await iter.return(undefined as void); + } + }), ); return new ModelHolder(compileCatalogModel(layers.flat())); } finally { diff --git a/plugins/scaffolder-common/src/catalogModel.ts b/plugins/scaffolder-common/src/catalogModel.ts index 3428d88f6a..4ee69b1dd7 100644 --- a/plugins/scaffolder-common/src/catalogModel.ts +++ b/plugins/scaffolder-common/src/catalogModel.ts @@ -15,6 +15,7 @@ */ import { createCatalogModelLayer } from '@backstage/catalog-model/alpha'; +import { JsonObject } from '@backstage/types'; import schema from './Template.v1beta3.schema.json'; /** @@ -47,7 +48,7 @@ export const templateModelLayer = createCatalogModelLayer({ }, ], schema: { - jsonSchema: schema as any, + jsonSchema: schema as JsonObject, }, }, ],