Address second round of PR review comments

- 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) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@spotify.com>
This commit is contained in:
Fredrik Adelöw
2026-04-14 13:59:39 +02:00
parent 445aefd4b9
commit 173ef97b48
5 changed files with 42 additions and 24 deletions
+1 -1
View File
@@ -395,7 +395,7 @@ export function createCatalogModelLayerBuilder(options: {
build(): CatalogModelLayer;
};
// @alpha (undocumented)
// @alpha
export const defaultCatalogEntityModel: CatalogModelLayer;
// @alpha
@@ -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' },
);
@@ -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<string>();
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() {
@@ -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 {
@@ -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,
},
},
],