From 807b956de4eabe056a0d7fa90f4f6bcdf8bc40b6 Mon Sep 17 00:00:00 2001 From: Kingston Date: Fri, 20 Mar 2026 02:09:34 -0700 Subject: [PATCH 1/8] feat: Support renames in reference expressions: when fields, relations, or roles are renamed, authorizer expressions are automatically updated to use the new names --- .changeset/support-expression-renames.md | 7 + .../src/references/expression-types.ts | 33 +- .../src/references/fix-definition-refs.ts | 50 +++ .../src/references/fix-expression-renames.ts | 118 ++++++ .../fix-expression-renames.unit.test.ts | 260 ++++++++++++ .../src/references/index.ts | 2 + .../authorizer-expression-parser.ts | 273 +++++++++++-- .../authorizer-expression-rename.unit.test.ts | 383 ++++++++++++++++++ .../expression-stub-parser.test-helper.ts | 14 +- .../expression-warning-parser.test-helper.ts | 12 +- .../definition-test-fixtures.test-helper.ts | 1 + .../definition/draft-lifecycle.int.test.ts | 1 + .../src/actions/definition/draft-session.ts | 5 + .../definition/stage-update-entity.action.ts | 11 +- .../src/actions/definition/validate-draft.ts | 18 +- .../project-definition-provider.tsx | 11 +- 16 files changed, 1104 insertions(+), 95 deletions(-) create mode 100644 .changeset/support-expression-renames.md create mode 100644 packages/project-builder-lib/src/references/fix-definition-refs.ts create mode 100644 packages/project-builder-lib/src/references/fix-expression-renames.ts create mode 100644 packages/project-builder-lib/src/references/fix-expression-renames.unit.test.ts create mode 100644 packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-rename.unit.test.ts diff --git a/.changeset/support-expression-renames.md b/.changeset/support-expression-renames.md new file mode 100644 index 000000000..f1bdd436a --- /dev/null +++ b/.changeset/support-expression-renames.md @@ -0,0 +1,7 @@ +--- +'@baseplate-dev/project-builder-lib': patch +'@baseplate-dev/project-builder-server': patch +'@baseplate-dev/project-builder-web': patch +--- + +Support renames in reference expressions: when fields, relations, or roles are renamed, authorizer expressions are automatically updated to use the new names diff --git a/packages/project-builder-lib/src/references/expression-types.ts b/packages/project-builder-lib/src/references/expression-types.ts index fd402449f..a55bbe2d1 100644 --- a/packages/project-builder-lib/src/references/expression-types.ts +++ b/packages/project-builder-lib/src/references/expression-types.ts @@ -97,8 +97,7 @@ export type ResolvedExpressionSlots< * readonly name = 'stub'; * parse(): undefined { return undefined; } * getWarnings(): [] { return []; } - * getDependencies(): [] { return []; } - * updateForRename(value: string): string { return value; } + * getReferencedEntities(): [] { return []; } * } * * // A parser that requires a model slot @@ -159,31 +158,25 @@ export abstract class RefExpressionParser< ): RefExpressionWarning[]; /** - * Get entity/field dependencies from the expression. - * Used for tracking what the expression references for rename handling. + * Get entity references from the expression with their positions. * - * @param value - The raw expression value - * @param parseResult - The cached parse result - * @returns Array of dependencies - */ - abstract getDependencies( - value: TValue, - parseResult: RefExpressionParseResult, - ): RefExpressionDependency[]; - - /** - * Update the expression when dependencies are renamed. + * Used by the generic rename system to detect and apply renames. + * Each returned dependency identifies an entity by ID and marks the + * position in the expression text that should be replaced with the + * entity's new name if it is renamed. * * @param value - The raw expression value * @param parseResult - The cached parse result - * @param renames - Map of old entity ID to new name - * @returns The updated expression value + * @param definition - The project definition (typed as unknown to avoid circular reference) + * @param resolvedSlots - The resolved slot paths for this expression + * @returns Array of entity references with positions for rename */ - abstract updateForRename( + abstract getReferencedEntities( value: TValue, parseResult: RefExpressionParseResult, - renames: Map, - ): TValue; + definition: unknown, + resolvedSlots: ResolvedExpressionSlots, + ): RefExpressionDependency[]; /** * Convenience method that combines parse() and getWarnings() into a single call. diff --git a/packages/project-builder-lib/src/references/fix-definition-refs.ts b/packages/project-builder-lib/src/references/fix-definition-refs.ts new file mode 100644 index 000000000..1e672b2a8 --- /dev/null +++ b/packages/project-builder-lib/src/references/fix-definition-refs.ts @@ -0,0 +1,50 @@ +import type { z } from 'zod'; + +import type { FixRefDeletionResult } from './fix-ref-deletions.js'; +import type { ResolvedZodRefPayload } from './types.js'; + +import { applyExpressionRenames } from './fix-expression-renames.js'; +import { fixRefDeletions } from './fix-ref-deletions.js'; +import { parseSchemaWithTransformedReferences } from './parse-schema-with-references.js'; + +export interface FixDefinitionRefsOptions { + /** Ref payload from the previous definition version, for detecting expression renames. */ + oldRefPayload?: ResolvedZodRefPayload; +} + +/** + * Fixes expression renames and dangling references in a single pass. + * + * Expression renames use the OLD definition (via `oldRefPayload`) to resolve + * entity references — expressions still contain old names like `model.title`, + * which can only be resolved against the definition where those names exist. + * The new entity names are then used to detect what was renamed. + * + * @param schema - The project definition Zod schema + * @param value - The definition after auto-fixes + * @param options - Optional old ref payload for rename detection + * @returns The fixed definition with ref payload + */ +export function fixDefinitionRefs( + schema: T, + value: unknown, + options?: FixDefinitionRefsOptions, +): FixRefDeletionResult> { + if (!options?.oldRefPayload) { + return fixRefDeletions(schema, value); + } + + // Parse the new definition to get new entities (for rename comparison) + const newRefPayload = parseSchemaWithTransformedReferences(schema, value, { + allowInvalidReferences: true, + }); + + const { value: renamedValue, modified } = applyExpressionRenames( + newRefPayload.data, + newRefPayload.entities, + options.oldRefPayload, + ); + + // Run fixRefDeletions on the (possibly renamed) definition + return fixRefDeletions(schema, modified ? renamedValue : newRefPayload.data); +} diff --git a/packages/project-builder-lib/src/references/fix-expression-renames.ts b/packages/project-builder-lib/src/references/fix-expression-renames.ts new file mode 100644 index 000000000..952044bc8 --- /dev/null +++ b/packages/project-builder-lib/src/references/fix-expression-renames.ts @@ -0,0 +1,118 @@ +import { set } from 'es-toolkit/compat'; + +import type { + DefinitionEntity, + ReferencePath, + ResolvedZodRefPayload, +} from './types.js'; + +export interface ApplyExpressionRenamesResult { + value: T; + modified: boolean; +} + +/** + * Detects renamed entities and updates expression strings accordingly. + * + * Uses the OLD definition/refPayload to resolve entity references (since + * expressions still contain old names like `model.title`), then compares + * old vs new entity names to detect renames and apply position-based + * string replacements on the NEW definition. + * + * @param newDefinition - The new definition value (where expressions will be updated) + * @param newEntities - Entities from the new definition (with new names) + * @param oldRefPayload - The ref payload from the old definition (has old expressions, entities, and definition) + * @returns The (possibly modified) definition and whether any renames were applied + */ +export function applyExpressionRenames( + newDefinition: T, + newEntities: ReadonlyArray, + oldRefPayload: ResolvedZodRefPayload, +): ApplyExpressionRenamesResult { + const { expressions: oldExpressions, entities: oldEntities } = oldRefPayload; + + if (oldExpressions.length === 0) { + return { value: newDefinition, modified: false }; + } + + // Detect renames: compare old vs new entity names by ID + const renames = new Map(); // entityId → newName + const newNameById = new Map(); + for (const entity of newEntities) { + newNameById.set(entity.id, entity.name); + } + for (const entity of oldEntities) { + const newName = newNameById.get(entity.id); + if (newName !== undefined && newName !== entity.name) { + renames.set(entity.id, newName); + } + } + + if (renames.size === 0) { + return { value: newDefinition, modified: false }; + } + + let modified = false; + const updates: { path: ReferencePath; value: string }[] = []; + + for (const expression of oldExpressions) { + // Parse and resolve entities against the OLD definition where old names still exist + const parseResult = expression.parser.parse( + expression.value, + oldRefPayload.data, + ); + if (!parseResult.success) { + // Don't touch broken expressions + continue; + } + + const refs = expression.parser.getReferencedEntities( + expression.value, + parseResult, + oldRefPayload.data, + expression.resolvedSlots, + ); + + // Build replacements for renamed entities that have positions + const replacements: { start: number; end: number; newValue: string }[] = []; + for (const ref of refs) { + if (ref.start == null || ref.end == null) { + continue; + } + const newName = renames.get(ref.entityId); + if (newName !== undefined) { + replacements.push({ + start: ref.start, + end: ref.end, + newValue: newName, + }); + } + } + + if (replacements.length === 0) { + continue; + } + + // Sort by position descending so earlier replacements don't shift later positions + replacements.sort((a, b) => b.start - a.start); + + let updated = expression.value as string; + for (const { start, end, newValue } of replacements) { + updated = updated.slice(0, start) + newValue + updated.slice(end); + } + + modified = true; + // Update at the same path in the NEW definition + updates.push({ path: expression.path, value: updated }); + } + + if (!modified) { + return { value: newDefinition, modified: false }; + } + + const result = structuredClone(newDefinition) as object; + for (const { path, value } of updates) { + set(result, path, value); + } + return { value: result as T, modified: true }; +} diff --git a/packages/project-builder-lib/src/references/fix-expression-renames.unit.test.ts b/packages/project-builder-lib/src/references/fix-expression-renames.unit.test.ts new file mode 100644 index 000000000..8fbaa111e --- /dev/null +++ b/packages/project-builder-lib/src/references/fix-expression-renames.unit.test.ts @@ -0,0 +1,260 @@ +import { describe, expect, it } from 'vitest'; +import { z } from 'zod'; + +import type { + DefinitionExpression, + RefExpressionDependency, + RefExpressionParseResult, +} from './expression-types.js'; +import type { DefinitionEntity, ResolvedZodRefPayload } from './types.js'; + +import { RefExpressionParser } from './expression-types.js'; +import { applyExpressionRenames } from './fix-expression-renames.js'; +import { createEntityType } from './types.js'; + +const testEntityType = createEntityType('test-entity'); + +/** + * A fake parser that returns configurable dependencies from getReferencedEntities. + */ +class FakeParser extends RefExpressionParser { + readonly name = 'fake'; + private readonly deps: RefExpressionDependency[]; + private readonly shouldFailParse: boolean; + + constructor( + deps: RefExpressionDependency[] = [], + options?: { shouldFailParse?: boolean }, + ) { + super(); + this.deps = deps; + this.shouldFailParse = options?.shouldFailParse ?? false; + } + + createSchema(): z.ZodType { + return z.string(); + } + + parse(value: string): RefExpressionParseResult { + if (this.shouldFailParse) { + return { success: false, error: 'Parse failed' }; + } + return { success: true, value }; + } + + getWarnings(): [] { + return []; + } + + getReferencedEntities(): RefExpressionDependency[] { + return this.deps; + } +} + +function makeEntity(id: string, name: string): DefinitionEntity { + return { + id, + name, + type: testEntityType, + path: [], + idPath: ['id'], + }; +} + +function makeExpression( + value: string, + parser: FakeParser, + path: (string | number)[] = ['expressions', 0], +): DefinitionExpression { + return { + value, + parser, + path, + resolvedSlots: {}, + }; +} + +function makeOldPayload( + data: unknown, + entities: DefinitionEntity[], + expressions: DefinitionExpression[], +): ResolvedZodRefPayload { + return { data, entities, references: [], expressions }; +} + +describe('applyExpressionRenames', () => { + it('should apply a single rename', () => { + const parser = new FakeParser([ + { entityType: testEntityType, entityId: 'e:1', start: 0, end: 5 }, + ]); + + const result = applyExpressionRenames( + { expressions: ['hello world'] }, // new definition (expressions not yet updated) + [makeEntity('e:1', 'world')], // new entities (renamed) + makeOldPayload( + { expressions: ['hello world'] }, + [makeEntity('e:1', 'hello')], // old entity name was 'hello' + [makeExpression('hello world', parser, ['expressions', 0])], + ), + ); + + expect(result.modified).toBe(true); + expect(result.value).toEqual({ expressions: ['world world'] }); + }); + + it('should apply multiple renames in descending position order', () => { + const parser = new FakeParser([ + { entityType: testEntityType, entityId: 'e:1', start: 0, end: 3 }, + { entityType: testEntityType, entityId: 'e:2', start: 4, end: 7 }, + ]); + + const result = applyExpressionRenames( + { expr: 'aaa bbb' }, + [makeEntity('e:1', 'xxx'), makeEntity('e:2', 'yyy')], + makeOldPayload( + { expr: 'aaa bbb' }, + [makeEntity('e:1', 'aaa'), makeEntity('e:2', 'bbb')], + [makeExpression('aaa bbb', parser, ['expr'])], + ), + ); + + expect(result.modified).toBe(true); + expect(result.value).toEqual({ expr: 'xxx yyy' }); + }); + + it('should return modified: false when no entities were renamed', () => { + const parser = new FakeParser([ + { entityType: testEntityType, entityId: 'e:1', start: 0, end: 5 }, + ]); + + const result = applyExpressionRenames( + { expr: 'hello' }, + [makeEntity('e:1', 'hello')], + makeOldPayload( + { expr: 'hello' }, + [makeEntity('e:1', 'hello')], // same name — no rename + [makeExpression('hello', parser, ['expr'])], + ), + ); + + expect(result.modified).toBe(false); + expect(result.value).toEqual({ expr: 'hello' }); + }); + + it('should return modified: false when no expressions exist', () => { + const result = applyExpressionRenames( + { expr: 'hello' }, + [makeEntity('e:1', 'world')], + makeOldPayload( + { expr: 'hello' }, + [makeEntity('e:1', 'hello')], + [], // no expressions + ), + ); + + expect(result.modified).toBe(false); + }); + + it('should skip expressions where parse fails', () => { + const failingParser = new FakeParser( + [{ entityType: testEntityType, entityId: 'e:1', start: 0, end: 5 }], + { shouldFailParse: true }, + ); + + const result = applyExpressionRenames( + { expr: 'hello' }, + [makeEntity('e:1', 'world')], + makeOldPayload( + { expr: 'hello' }, + [makeEntity('e:1', 'hello')], + [makeExpression('hello', failingParser, ['expr'])], + ), + ); + + expect(result.modified).toBe(false); + expect(result.value).toEqual({ expr: 'hello' }); + }); + + it('should skip dependencies without positions', () => { + const parser = new FakeParser([ + { entityType: testEntityType, entityId: 'e:1' }, // no start/end + ]); + + const result = applyExpressionRenames( + { expr: 'hello' }, + [makeEntity('e:1', 'world')], + makeOldPayload( + { expr: 'hello' }, + [makeEntity('e:1', 'hello')], + [makeExpression('hello', parser, ['expr'])], + ), + ); + + expect(result.modified).toBe(false); + }); + + it('should skip dependencies whose entity ID was not renamed', () => { + const parser = new FakeParser([ + { entityType: testEntityType, entityId: 'e:2', start: 0, end: 5 }, + ]); + + const result = applyExpressionRenames( + { expr: 'hello' }, + [makeEntity('e:1', 'world'), makeEntity('e:2', 'hello')], + makeOldPayload( + { expr: 'hello' }, + [makeEntity('e:1', 'old'), makeEntity('e:2', 'hello')], // e:1 renamed, but dep is on e:2 + [makeExpression('hello', parser, ['expr'])], + ), + ); + + expect(result.modified).toBe(false); + }); + + it('should handle multiple expressions in the same definition', () => { + const parser1 = new FakeParser([ + { entityType: testEntityType, entityId: 'e:1', start: 0, end: 3 }, + ]); + const parser2 = new FakeParser([ + { entityType: testEntityType, entityId: 'e:1', start: 0, end: 3 }, + ]); + + const result = applyExpressionRenames( + { items: [{ expr: 'foo' }, { expr: 'foo bar' }] }, + [makeEntity('e:1', 'baz')], + makeOldPayload( + { items: [{ expr: 'foo' }, { expr: 'foo bar' }] }, + [makeEntity('e:1', 'foo')], + [ + makeExpression('foo', parser1, ['items', 0, 'expr']), + makeExpression('foo bar', parser2, ['items', 1, 'expr']), + ], + ), + ); + + expect(result.modified).toBe(true); + expect(result.value).toEqual({ + items: [{ expr: 'baz' }, { expr: 'baz bar' }], + }); + }); + + it('should handle renames that change string length', () => { + const parser = new FakeParser([ + { entityType: testEntityType, entityId: 'e:1', start: 0, end: 2 }, + { entityType: testEntityType, entityId: 'e:2', start: 3, end: 4 }, + ]); + + const result = applyExpressionRenames( + { expr: 'ab c' }, + [makeEntity('e:1', 'xxxx'), makeEntity('e:2', 'yyyyy')], + makeOldPayload( + { expr: 'ab c' }, + [makeEntity('e:1', 'ab'), makeEntity('e:2', 'c')], + [makeExpression('ab c', parser, ['expr'])], + ), + ); + + expect(result.modified).toBe(true); + expect(result.value).toEqual({ expr: 'xxxx yyyyy' }); + }); +}); diff --git a/packages/project-builder-lib/src/references/index.ts b/packages/project-builder-lib/src/references/index.ts index 2478ac26a..d8aa931ca 100644 --- a/packages/project-builder-lib/src/references/index.ts +++ b/packages/project-builder-lib/src/references/index.ts @@ -4,6 +4,8 @@ export * from './deserialize-schema.js'; export * from './expression-types.js'; export { withEnt, withRef } from './extend-parser-context-with-refs.js'; export * from './extract-definition-refs.js'; +export * from './fix-definition-refs.js'; +export * from './fix-expression-renames.js'; export * from './fix-ref-deletions.js'; export * from './ref-context-slot.js'; export * from './ref-schema-visitor.js'; diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-parser.ts b/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-parser.ts index c641b659c..c55f05adf 100644 --- a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-parser.ts +++ b/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-parser.ts @@ -13,13 +13,20 @@ import { RefExpressionParser } from '#src/references/expression-types.js'; import type { modelEntityType } from '../types.js'; import type { AuthorizerExpressionInfo } from './authorizer-expression-ast.js'; import type { ModelValidationContext } from './authorizer-expression-validator.js'; +import type { AuthorizerExpressionVisitor } from './authorizer-expression-visitor.js'; +import { + modelLocalRelationEntityType, + modelScalarFieldEntityType, +} from '../types.js'; import { parseAuthorizerExpression } from './authorizer-expression-acorn-parser.js'; import { AuthorizerExpressionParseError } from './authorizer-expression-ast.js'; import { buildModelExpressionContext, validateAuthorizerExpression, } from './authorizer-expression-validator.js'; +import { visitAuthorizerExpression } from './authorizer-expression-visitor.js'; +import { modelAuthorizerRoleEntityType } from './types.js'; /** * Shape of a raw model in the project definition JSON. @@ -29,8 +36,9 @@ interface RawModelDefinition { id?: string; name?: string; model?: { - fields?: { name: string; type?: string }[]; + fields?: { id?: string; name: string; type?: string }[]; relations?: { + id?: string; name: string; modelRef: string; foreignRelationName?: string; @@ -38,7 +46,7 @@ interface RawModelDefinition { }[]; }; authorizer?: { - roles?: { name: string }[]; + roles?: { id?: string; name: string }[]; }; } @@ -106,15 +114,11 @@ export class AuthorizerExpressionParser extends RefExpressionParser< context: ExpressionValidationContext, resolvedSlots: ResolvedExpressionSlots<{ model: typeof modelEntityType }>, ): RefExpressionWarning[] { - // Get model context from resolved slots + // Get model context from resolved slots (throws if model not found) const modelContext = this.getModelContext( context.definition, resolvedSlots, ); - if (!modelContext) { - // Can't validate without model context - return []; - } // Validate the expression against model fields and roles return validateAuthorizerExpression( @@ -126,55 +130,242 @@ export class AuthorizerExpressionParser extends RefExpressionParser< } /** - * Get entity/field dependencies from the expression. + * Get entity references from the expression with their positions. * - * Currently returns empty array as we don't yet track - * entity-level dependencies (just field names). - * Future: could track model field entity references for renames. + * Walks the AST and resolves each name reference (field, relation, role) + * to its entity ID by navigating the model definition from the resolved slots. + * Returns positions marking exactly which text to replace when an entity is renamed. */ - getDependencies(): RefExpressionDependency[] { - // TODO: Track model field entities for rename support - return []; - } + getReferencedEntities( + _value: string, + parseResult: RefExpressionParseResult, + definition: unknown, + resolvedSlots: ResolvedExpressionSlots<{ model: typeof modelEntityType }>, + ): RefExpressionDependency[] { + if (!parseResult.success) { + return []; + } - /** - * Update the expression when dependencies are renamed. - * - * Currently returns value unchanged as we don't yet - * support field renames in expressions. - */ - updateForRename(value: string): string { - // TODO: Implement rename support using AST position info - return value; + const model = this.getRawModel(definition, resolvedSlots); + + const allModels = ( + (definition as { models?: RawModelDefinition[] }).models ?? [] + ).filter( + (m): m is RawModelDefinition & { name: string } => + typeof m.name === 'string', + ); + + // Build lookup maps + const fieldByName = new Map(); + for (const field of model.model?.fields ?? []) { + if (field.id) { + fieldByName.set(field.name, { id: field.id }); + } + } + + const relationByName = new Map(); + for (const relation of model.model?.relations ?? []) { + if (relation.id) { + relationByName.set(relation.name, { + id: relation.id, + modelRef: relation.modelRef, + }); + } + } + + const modelById = new Map(); + for (const m of allModels) { + if (m.id) { + modelById.set(m.id, m); + } + } + + const deps: RefExpressionDependency[] = []; + + const visitor: AuthorizerExpressionVisitor = { + fieldComparison(node) { + for (const side of [node.left, node.right]) { + if (side.type === 'fieldRef' && side.source === 'model') { + const field = fieldByName.get(side.field); + if (field) { + deps.push({ + entityType: modelScalarFieldEntityType, + entityId: field.id, + start: side.end - side.field.length, + end: side.end, + }); + } + } + } + }, + hasRole() { + // Global auth roles are defined by plugins, not navigable from + // the raw model definition. Skip — auth role renames are rare + // and would require traversing plugin-specific config. + }, + hasSomeRole() { + // Same as hasRole — skip global auth role references + }, + nestedHasRole(node) { + const relation = relationByName.get(node.relationName); + if (relation) { + deps.push({ + entityType: modelLocalRelationEntityType, + entityId: relation.id, + start: node.relationEnd - node.relationName.length, + end: node.relationEnd, + }); + // Foreign authorizer role + const foreignModel = modelById.get(relation.modelRef); + const foreignRole = foreignModel?.authorizer?.roles?.find( + (r) => r.name === node.role, + ); + if (foreignRole?.id) { + deps.push({ + entityType: modelAuthorizerRoleEntityType, + entityId: foreignRole.id, + start: node.roleStart + 1, + end: node.roleEnd - 1, + }); + } + } + }, + nestedHasSomeRole(node) { + const relation = relationByName.get(node.relationName); + if (relation) { + deps.push({ + entityType: modelLocalRelationEntityType, + entityId: relation.id, + start: node.relationEnd - node.relationName.length, + end: node.relationEnd, + }); + const foreignModel = modelById.get(relation.modelRef); + if (foreignModel?.authorizer?.roles) { + const foreignRoleByName = new Map( + foreignModel.authorizer.roles + .filter((r) => r.id) + .map((r) => [r.name, r]), + ); + for (let i = 0; i < node.roles.length; i++) { + const foreignRole = foreignRoleByName.get(node.roles[i]); + if (foreignRole?.id) { + deps.push({ + entityType: modelAuthorizerRoleEntityType, + entityId: foreignRole.id, + start: node.rolesStart[i] + 1, + end: node.rolesEnd[i] - 1, + }); + } + } + } + } + }, + relationFilter(node) { + const relation = relationByName.get(node.relationName); + if (relation) { + deps.push({ + entityType: modelLocalRelationEntityType, + entityId: relation.id, + start: node.relationEnd - node.relationName.length, + end: node.relationEnd, + }); + // Foreign model fields referenced in conditions + const foreignModel = modelById.get(relation.modelRef); + const foreignFieldByName = new Map(); + for (const f of foreignModel?.model?.fields ?? []) { + if (f.id) { + foreignFieldByName.set(f.name, { id: f.id }); + } + } + for (const condition of node.conditions) { + // Condition key references a field on the foreign model + const foreignField = foreignFieldByName.get(condition.field); + if (foreignField) { + deps.push({ + entityType: modelScalarFieldEntityType, + entityId: foreignField.id, + start: condition.fieldStart, + end: condition.fieldEnd, + }); + } + // Condition value may be a model field ref + if ( + condition.value.type === 'fieldRef' && + condition.value.source === 'model' + ) { + const field = fieldByName.get(condition.value.field); + if (field) { + deps.push({ + entityType: modelScalarFieldEntityType, + entityId: field.id, + start: condition.value.end - condition.value.field.length, + end: condition.value.end, + }); + } + } + } + } + }, + isAuthenticated() { + // No entity references + }, + binaryLogical(_node, _ctx, visit) { + visit(_node.left); + visit(_node.right); + }, + }; + + visitAuthorizerExpression(parseResult.value.ast, visitor); + + return deps; } /** - * Extract model context from the project definition using resolved slots. + * Navigate to the raw model object from the definition using resolved slots. + * + * Resolved slot paths point to the entity's ID field (e.g., `['models', 2, 'id']`), + * so we walk parent paths until we find an object with a string `name` property. */ - private getModelContext( + private getRawModel( definition: unknown, resolvedSlots: ResolvedExpressionSlots<{ model: typeof modelEntityType }>, - ): ModelValidationContext | undefined { + ): RawModelDefinition & { name: string } { const modelPath = resolvedSlots.model; - if (modelPath.length === 0) { - return undefined; - } - // Navigate to the model in the project definition - // The path is like ['models', 0] for models[0] - let current: unknown = definition; - for (const segment of modelPath) { - if (current === null || current === undefined) { - return undefined; + // Walk progressively shorter paths to find the model object. + // Slot paths include the idPath suffix (e.g., ['models', 2, 'id']), + // so we try the full path first, then strip segments until we find + // an object with a name property. + for (let len = modelPath.length; len > 0; len--) { + let current: unknown = definition; + for (let i = 0; i < len; i++) { + if (current === null || current === undefined) { + break; + } + current = (current as Record)[modelPath[i]]; + } + if ( + current !== null && + current !== undefined && + typeof current === 'object' && + 'name' in current && + typeof (current as Record).name === 'string' + ) { + return current as RawModelDefinition & { name: string }; } - current = (current as Record)[segment]; } - const model = current as RawModelDefinition | null; + throw new Error(`Could not resolve model at path ${modelPath.join('.')}`); + } - if (!model || typeof model.name !== 'string') { - return undefined; - } + /** + * Extract model context from the project definition using resolved slots. + */ + private getModelContext( + definition: unknown, + resolvedSlots: ResolvedExpressionSlots<{ model: typeof modelEntityType }>, + ): ModelValidationContext { + const model = this.getRawModel(definition, resolvedSlots); const allModels = ( (definition as { models?: RawModelDefinition[] }).models ?? [] diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-rename.unit.test.ts b/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-rename.unit.test.ts new file mode 100644 index 000000000..264e5cbb1 --- /dev/null +++ b/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-rename.unit.test.ts @@ -0,0 +1,383 @@ +import { describe, expect, it } from 'vitest'; + +import type { RefExpressionDependency } from '#src/references/expression-types.js'; + +import { modelScalarFieldEntityType } from '../types.js'; +import { parseAuthorizerExpression } from './authorizer-expression-acorn-parser.js'; +import { AuthorizerExpressionParser } from './authorizer-expression-parser.js'; +import { modelAuthorizerRoleEntityType } from './types.js'; + +/** + * Build a minimal definition with one model for testing getReferencedEntities. + */ +function buildDefinition( + model: { + fields?: { id: string; name: string }[]; + relations?: { + id: string; + name: string; + modelRef: string; + }[]; + }, + otherModels?: { + id: string; + name: string; + fields?: { id: string; name: string }[]; + authorizer?: { roles?: { id: string; name: string }[] }; + }[], +): { definition: unknown; resolvedSlots: { model: (string | number)[] } } { + const mainModel = { + id: 'model:main', + name: 'Main', + model: { + fields: model.fields ?? [], + relations: model.relations ?? [], + }, + authorizer: { roles: [] }, + }; + + const models = [ + mainModel, + ...(otherModels ?? []).map((m) => ({ + id: m.id, + name: m.name, + model: { fields: m.fields ?? [], relations: [] }, + authorizer: m.authorizer ?? { roles: [] }, + })), + ]; + + return { + definition: { models }, + resolvedSlots: { model: ['models', 0] }, + }; +} + +function getReferencedEntities( + expression: string, + model: Parameters[0], + otherModels?: Parameters[1], +): RefExpressionDependency[] { + const parser = new AuthorizerExpressionParser(); + const info = parseAuthorizerExpression(expression); + const { definition, resolvedSlots } = buildDefinition(model, otherModels); + return parser.getReferencedEntities( + expression, + { success: true, value: info }, + definition, + resolvedSlots, + ); +} + +/** + * Helper to apply renames using getReferencedEntities output. + * Mimics the generic orchestrator logic. + */ +function applyRenames( + expression: string, + deps: RefExpressionDependency[], + renames: Map, +): string { + const replacements = deps + .filter( + (ref) => + ref.start != null && ref.end != null && renames.has(ref.entityId), + ) + .map((ref) => ({ + start: ref.start!, + end: ref.end!, + newValue: renames.get(ref.entityId)!, + })) + .toSorted((a, b) => b.start - a.start); + + let result = expression; + for (const { start, end, newValue } of replacements) { + result = result.slice(0, start) + newValue + result.slice(end); + } + return result; +} + +describe('AuthorizerExpressionParser.getReferencedEntities', () => { + describe('model field references', () => { + it('should resolve model.field to field entity ID', () => { + const deps = getReferencedEntities('model.title === userId', { + fields: [{ id: 'field:title', name: 'title' }], + }); + expect(deps).toEqual([ + { + entityType: modelScalarFieldEntityType, + entityId: 'field:title', + start: 6, + end: 11, + }, + ]); + }); + + it('should resolve fields on both sides of a comparison', () => { + const deps = getReferencedEntities('model.authorId === model.creatorId', { + fields: [ + { id: 'field:author', name: 'authorId' }, + { id: 'field:creator', name: 'creatorId' }, + ], + }); + expect(deps).toHaveLength(2); + expect(deps[0].entityId).toBe('field:author'); + expect(deps[1].entityId).toBe('field:creator'); + }); + + it('should skip unknown fields', () => { + const deps = getReferencedEntities('model.unknown === userId', { + fields: [{ id: 'field:title', name: 'title' }], + }); + expect(deps).toEqual([]); + }); + }); + + describe('relation references', () => { + it('should resolve relation in nested hasRole', () => { + const deps = getReferencedEntities( + "hasRole(model.todoList, 'owner')", + { + relations: [ + { + id: 'rel:todoList', + name: 'todoList', + modelRef: 'model:todo', + }, + ], + }, + [ + { + id: 'model:todo', + name: 'Todo', + authorizer: { + roles: [{ id: 'auth-role:owner', name: 'owner' }], + }, + }, + ], + ); + // Should have both the relation ref and the foreign role ref + expect(deps).toHaveLength(2); + expect(deps[0].entityId).toBe('rel:todoList'); + expect(deps[1].entityId).toBe('auth-role:owner'); + expect(deps[1].entityType).toBe(modelAuthorizerRoleEntityType); + }); + + it('should resolve relation in exists filter', () => { + const deps = getReferencedEntities( + 'exists(model.members, { memberId: userId })', + { + relations: [ + { + id: 'rel:members', + name: 'members', + modelRef: 'model:member', + }, + ], + }, + [ + { + id: 'model:member', + name: 'Member', + fields: [{ id: 'field:memberId', name: 'memberId' }], + }, + ], + ); + // Relation + foreign field in condition + expect(deps).toHaveLength(2); + expect(deps[0].entityId).toBe('rel:members'); + expect(deps[1].entityId).toBe('field:memberId'); + }); + }); + + describe('end-to-end rename via generic orchestrator', () => { + it('should rename a model field', () => { + const expression = 'model.title === userId'; + const deps = getReferencedEntities(expression, { + fields: [{ id: 'field:title', name: 'title' }], + }); + const result = applyRenames( + expression, + deps, + new Map([['field:title', 'heading']]), + ); + expect(result).toBe('model.heading === userId'); + }); + + it('should rename a relation in nested hasRole', () => { + const expression = "hasRole(model.todoList, 'owner')"; + const deps = getReferencedEntities( + expression, + { + relations: [ + { + id: 'rel:todoList', + name: 'todoList', + modelRef: 'model:todo', + }, + ], + }, + [ + { + id: 'model:todo', + name: 'Todo', + authorizer: { + roles: [{ id: 'auth-role:owner', name: 'owner' }], + }, + }, + ], + ); + const result = applyRenames( + expression, + deps, + new Map([['rel:todoList', 'list']]), + ); + expect(result).toBe("hasRole(model.list, 'owner')"); + }); + + it('should rename a foreign authorizer role', () => { + const expression = "hasRole(model.todoList, 'owner')"; + const deps = getReferencedEntities( + expression, + { + relations: [ + { + id: 'rel:todoList', + name: 'todoList', + modelRef: 'model:todo', + }, + ], + }, + [ + { + id: 'model:todo', + name: 'Todo', + authorizer: { + roles: [{ id: 'auth-role:owner', name: 'owner' }], + }, + }, + ], + ); + const result = applyRenames( + expression, + deps, + new Map([['auth-role:owner', 'admin']]), + ); + expect(result).toBe("hasRole(model.todoList, 'admin')"); + }); + + it('should rename relation and foreign role together', () => { + const expression = "hasRole(model.todoList, 'owner')"; + const deps = getReferencedEntities( + expression, + { + relations: [ + { + id: 'rel:todoList', + name: 'todoList', + modelRef: 'model:todo', + }, + ], + }, + [ + { + id: 'model:todo', + name: 'Todo', + authorizer: { + roles: [{ id: 'auth-role:owner', name: 'owner' }], + }, + }, + ], + ); + const result = applyRenames( + expression, + deps, + new Map([ + ['rel:todoList', 'list'], + ['auth-role:owner', 'admin'], + ]), + ); + expect(result).toBe("hasRole(model.list, 'admin')"); + }); + + it('should rename foreign field in exists condition', () => { + const expression = 'exists(model.members, { userName: userId })'; + const deps = getReferencedEntities( + expression, + { + relations: [ + { + id: 'rel:members', + name: 'members', + modelRef: 'model:member', + }, + ], + }, + [ + { + id: 'model:member', + name: 'Member', + fields: [{ id: 'field:userName', name: 'userName' }], + }, + ], + ); + const result = applyRenames( + expression, + deps, + new Map([['field:userName', 'memberName']]), + ); + expect(result).toBe('exists(model.members, { memberName: userId })'); + }); + + it('should handle complex expression with multiple renames', () => { + const expression = + "model.authorId === userId && hasRole(model.todoList, 'owner')"; + const deps = getReferencedEntities( + expression, + { + fields: [{ id: 'field:authorId', name: 'authorId' }], + relations: [ + { + id: 'rel:todoList', + name: 'todoList', + modelRef: 'model:todo', + }, + ], + }, + [ + { + id: 'model:todo', + name: 'Todo', + authorizer: { + roles: [{ id: 'auth-role:owner', name: 'owner' }], + }, + }, + ], + ); + const result = applyRenames( + expression, + deps, + new Map([ + ['field:authorId', 'ownerId'], + ['rel:todoList', 'list'], + ['auth-role:owner', 'admin'], + ]), + ); + expect(result).toBe( + "model.ownerId === userId && hasRole(model.list, 'admin')", + ); + }); + + it('should not rename when no entities match', () => { + const expression = 'model.id === userId'; + const deps = getReferencedEntities(expression, { + fields: [{ id: 'field:id', name: 'id' }], + }); + const result = applyRenames( + expression, + deps, + new Map([['field:other', 'something']]), + ); + expect(result).toBe(expression); + }); + }); +}); diff --git a/packages/project-builder-lib/src/testing/expression-stub-parser.test-helper.ts b/packages/project-builder-lib/src/testing/expression-stub-parser.test-helper.ts index 38b150a8f..2e6bbd2b2 100644 --- a/packages/project-builder-lib/src/testing/expression-stub-parser.test-helper.ts +++ b/packages/project-builder-lib/src/testing/expression-stub-parser.test-helper.ts @@ -36,15 +36,9 @@ class StubParser extends RefExpressionParser { return []; } - getDependencies(): [] { - // No dependencies tracked by stub parser + getReferencedEntities(): [] { return []; } - - updateForRename(value: string): string { - // No rename handling - return value unchanged - return value; - } } /** @@ -86,11 +80,7 @@ export class StubParserWithSlots< return []; } - getDependencies(): [] { + getReferencedEntities(): [] { return []; } - - updateForRename(value: string): string { - return value; - } } diff --git a/packages/project-builder-lib/src/testing/expression-warning-parser.test-helper.ts b/packages/project-builder-lib/src/testing/expression-warning-parser.test-helper.ts index 35622f2d9..dac625415 100644 --- a/packages/project-builder-lib/src/testing/expression-warning-parser.test-helper.ts +++ b/packages/project-builder-lib/src/testing/expression-warning-parser.test-helper.ts @@ -44,13 +44,9 @@ export class WarningParser extends RefExpressionParser { return this.warningsToReturn; } - getDependencies(): [] { + getReferencedEntities(): [] { return []; } - - updateForRename(value: string): string { - return value; - } } /** @@ -74,11 +70,7 @@ export class FailingParser extends RefExpressionParser { return []; } - getDependencies(): [] { + getReferencedEntities(): [] { return []; } - - updateForRename(value: string): string { - return value; - } } diff --git a/packages/project-builder-server/src/actions/definition/definition-test-fixtures.test-helper.ts b/packages/project-builder-server/src/actions/definition/definition-test-fixtures.test-helper.ts index 2fe02dc2b..67938597f 100644 --- a/packages/project-builder-server/src/actions/definition/definition-test-fixtures.test-helper.ts +++ b/packages/project-builder-server/src/actions/definition/definition-test-fixtures.test-helper.ts @@ -102,6 +102,7 @@ export const test = baseTest.extend<{ testEntityServiceContext.entityContext.serializedDefinition, }, entityContext: testEntityServiceContext.entityContext, + oldRefPayload: testEntityServiceContext.container.refPayload, parserContext: testEntityServiceContext.parserContext, projectDirectory: '/test-project', }; diff --git a/packages/project-builder-server/src/actions/definition/draft-lifecycle.int.test.ts b/packages/project-builder-server/src/actions/definition/draft-lifecycle.int.test.ts index 9bd66f522..0e33cb282 100644 --- a/packages/project-builder-server/src/actions/definition/draft-lifecycle.int.test.ts +++ b/packages/project-builder-server/src/actions/definition/draft-lifecycle.int.test.ts @@ -164,6 +164,7 @@ describe('draft lifecycle', () => { draftDefinition: mismatchContext.entityContext.serializedDefinition, }, entityContext: mismatchContext.entityContext, + oldRefPayload: mismatchContext.container.refPayload, parserContext: mismatchContext.parserContext, projectDirectory: '/test-project', }; diff --git a/packages/project-builder-server/src/actions/definition/draft-session.ts b/packages/project-builder-server/src/actions/definition/draft-session.ts index 72ffccf97..43f402c70 100644 --- a/packages/project-builder-server/src/actions/definition/draft-session.ts +++ b/packages/project-builder-server/src/actions/definition/draft-session.ts @@ -2,6 +2,7 @@ import type { EntityServiceContext, SchemaParserContext, } from '@baseplate-dev/project-builder-lib'; +import type { ResolvedZodRefPayload } from '@baseplate-dev/project-builder-lib'; import { ProjectDefinitionContainer } from '@baseplate-dev/project-builder-lib'; import { hashWithSHA256, stringifyPrettyStable } from '@baseplate-dev/utils'; @@ -112,6 +113,8 @@ export async function deleteDraftSession( export interface DraftSessionContext { session: DraftSession; entityContext: EntityServiceContext; + /** Ref payload from the current draft (before the pending edit), used for rename detection. */ + oldRefPayload: ResolvedZodRefPayload; parserContext: SchemaParserContext; projectDirectory: string; } @@ -167,6 +170,7 @@ export async function getOrCreateDraftSession( return { session: existingDraft, entityContext, + oldRefPayload: draftContainer.refPayload, parserContext, projectDirectory: project.directory, }; @@ -187,6 +191,7 @@ export async function getOrCreateDraftSession( return { session, entityContext, + oldRefPayload: container.refPayload, parserContext, projectDirectory: project.directory, }; diff --git a/packages/project-builder-server/src/actions/definition/stage-update-entity.action.ts b/packages/project-builder-server/src/actions/definition/stage-update-entity.action.ts index 5a5d7b9c3..7c39cf3be 100644 --- a/packages/project-builder-server/src/actions/definition/stage-update-entity.action.ts +++ b/packages/project-builder-server/src/actions/definition/stage-update-entity.action.ts @@ -43,8 +43,13 @@ export const stageUpdateEntityAction = createServiceAction({ handler: async (input, context) => { assertEntityTypeNotBlacklisted(input.entityTypeName); - const { session, entityContext, parserContext, projectDirectory } = - await getOrCreateDraftSession(input.project, context); + const { + session, + entityContext, + oldRefPayload, + parserContext, + projectDirectory, + } = await getOrCreateDraftSession(input.project, context); const newDefinition = updateEntity( { @@ -60,6 +65,8 @@ export const stageUpdateEntityAction = createServiceAction({ parserContext, session, projectDirectory, + undefined, + oldRefPayload, ); return { diff --git a/packages/project-builder-server/src/actions/definition/validate-draft.ts b/packages/project-builder-server/src/actions/definition/validate-draft.ts index 6c4995efe..60fbbb63e 100644 --- a/packages/project-builder-server/src/actions/definition/validate-draft.ts +++ b/packages/project-builder-server/src/actions/definition/validate-draft.ts @@ -1,12 +1,13 @@ import type { DefinitionIssue, + ResolvedZodRefPayload, SchemaParserContext, } from '@baseplate-dev/project-builder-lib'; import { applyDefinitionFixes, collectDefinitionIssues, - fixRefDeletions, + fixDefinitionRefs, partitionIssuesBySeverity, ProjectDefinitionContainer, } from '@baseplate-dev/project-builder-lib'; @@ -90,16 +91,18 @@ export interface FixAndValidateResult { } /** - * Applies auto-fixes, fixes dangling references, then validates the definition. + * Applies auto-fixes, fixes dangling references and expression renames, + * then validates the definition. * * Mirrors the web UI save pipeline: * 1. applyDefinitionFixes — clears disabled services, etc. - * 2. fixRefDeletions — cascades reference deletions + * 2. fixDefinitionRefs — cascades reference deletions + updates expressions for renames * 3. collectDefinitionIssues — partitions into errors/warnings */ export function fixAndValidateDraftDefinition( draftDefinition: Record, parserContext: SchemaParserContext, + oldRefPayload?: ResolvedZodRefPayload, ): FixAndValidateResult { const container = ProjectDefinitionContainer.fromSerializedConfig( draftDefinition, @@ -112,8 +115,10 @@ export function fixAndValidateDraftDefinition( container.definition, ); - // Step 2: Fix dangling references - const refResult = fixRefDeletions(container.schema, fixedDefinition); + // Step 2: Fix dangling references and expression renames + const refResult = fixDefinitionRefs(container.schema, fixedDefinition, { + oldRefPayload, + }); if (refResult.type === 'failure') { // RESTRICT issues — report as errors @@ -195,9 +200,10 @@ export async function validateAndSaveDraft( session: DraftSession, projectDirectory: string, errorPrefix = 'Staging blocked by definition errors', + oldRefPayload?: ResolvedZodRefPayload, ): Promise { const { fixedSerializedDefinition, errors, warnings } = - fixAndValidateDraftDefinition(definition, parserContext); + fixAndValidateDraftDefinition(definition, parserContext, oldRefPayload); if (errors.length > 0) { const messages = errors.map((e) => e.message).join('; '); diff --git a/packages/project-builder-web/src/app/project-definition-provider/project-definition-provider.tsx b/packages/project-builder-web/src/app/project-definition-provider/project-definition-provider.tsx index a353f8538..1d700d8b5 100644 --- a/packages/project-builder-web/src/app/project-definition-provider/project-definition-provider.tsx +++ b/packages/project-builder-web/src/app/project-definition-provider/project-definition-provider.tsx @@ -15,7 +15,7 @@ import { createDefinitionSchemaParserContext, createPluginSpecStore, createProjectDefinitionSchema, - fixRefDeletions, + fixDefinitionRefs, partitionIssuesBySeverity, ProjectDefinitionContainer, } from '@baseplate-dev/project-builder-lib'; @@ -94,7 +94,7 @@ export function ProjectDefinitionProvider({ const result: UseProjectDefinitionResult | undefined = useMemo(() => { if (!projectDefinitionContainer || !schemaParserContext) return; - const { definition } = projectDefinitionContainer; + const { definition, refPayload } = projectDefinitionContainer; const parserContext = schemaParserContext; async function saveDefinition( @@ -126,12 +126,15 @@ export function ProjectDefinitionProvider({ createDefinitionSchemaParserContext(schemaCreatorOptions); const defSchema = createProjectDefinitionSchema(defContext); const parsedProjectDefinition = defSchema.parse(rawProjectDefinition); - const newProjectDefinition = applyDefinitionFixes( + const autoFixedDefinition = applyDefinitionFixes( defSchema, parsedProjectDefinition, ); - const result = fixRefDeletions(defSchema, newProjectDefinition); + // Fix dangling references and update expressions for renames + const result = fixDefinitionRefs(defSchema, autoFixedDefinition, { + oldRefPayload: refPayload, + }); if (result.type === 'failure') { throw new RefDeleteError(result.issues); } From a12220ef273e1fe50e431c40253bc78be9b5370d Mon Sep 17 00:00:00 2001 From: Kingston Date: Fri, 20 Mar 2026 03:23:05 -0700 Subject: [PATCH 2/8] Refactor plugin parsers into separate folder --- .claude/settings.json | 3 +- .../src/parser/collect-definition-issues.ts | 8 +-- .../collect-definition-issues.unit.test.ts | 9 ++- .../src/parser/collect-expression-issues.ts | 41 +++++++----- .../collect-expression-issues.unit.test.ts | 65 ++++++++++++------- .../authorizer-expression-acorn-parser.ts | 0 ...rizer-expression-acorn-parser.unit.test.ts | 0 .../authorizer/authorizer-expression-ast.ts | 0 .../authorizer-expression-parser.ts | 12 ++-- .../authorizer-expression-rename.unit.test.ts | 5 +- .../authorizer-expression-validator.ts | 0 ...thorizer-expression-validator.unit.test.ts | 0 .../authorizer-expression-visitor.ts | 0 .../expression-parsers/authorizer/index.ts | 5 ++ .../schema/models/authorizer/authorizer.ts | 2 +- .../src/schema/models/authorizer/index.ts | 6 +- 16 files changed, 96 insertions(+), 60 deletions(-) rename packages/project-builder-lib/src/{schema/models => references/expression-parsers}/authorizer/authorizer-expression-acorn-parser.ts (100%) rename packages/project-builder-lib/src/{schema/models => references/expression-parsers}/authorizer/authorizer-expression-acorn-parser.unit.test.ts (100%) rename packages/project-builder-lib/src/{schema/models => references/expression-parsers}/authorizer/authorizer-expression-ast.ts (100%) rename packages/project-builder-lib/src/{schema/models => references/expression-parsers}/authorizer/authorizer-expression-parser.ts (98%) rename packages/project-builder-lib/src/{schema/models => references/expression-parsers}/authorizer/authorizer-expression-rename.unit.test.ts (98%) rename packages/project-builder-lib/src/{schema/models => references/expression-parsers}/authorizer/authorizer-expression-validator.ts (100%) rename packages/project-builder-lib/src/{schema/models => references/expression-parsers}/authorizer/authorizer-expression-validator.unit.test.ts (100%) rename packages/project-builder-lib/src/{schema/models => references/expression-parsers}/authorizer/authorizer-expression-visitor.ts (100%) create mode 100644 packages/project-builder-lib/src/references/expression-parsers/authorizer/index.ts diff --git a/.claude/settings.json b/.claude/settings.json index 62468ef18..cca897817 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -57,7 +57,8 @@ "Bash(git fetch *)", "Bash(git checkout -b *)", "Bash(echo $?)", - "Bash(jq *)" + "Bash(jq *)", + "Bash(git mv:*)" ], "deny": [ "Edit(**/baseplate/generated/**)", diff --git a/packages/project-builder-lib/src/parser/collect-definition-issues.ts b/packages/project-builder-lib/src/parser/collect-definition-issues.ts index b4fa679a8..b02ae4bd1 100644 --- a/packages/project-builder-lib/src/parser/collect-definition-issues.ts +++ b/packages/project-builder-lib/src/parser/collect-definition-issues.ts @@ -118,12 +118,12 @@ export function collectDefinitionIssues( issues.push(...result); } - // Collect expression validation issues - const expressionIssues = collectExpressionIssues( - schema, + // Collect expression validation issues (uses container's pre-resolved expressions) + const expressionIssues = collectExpressionIssues({ definition, pluginStore, - ); + expressions: container.refPayload.expressions, + }); issues.push(...expressionIssues); return issues; diff --git a/packages/project-builder-lib/src/parser/collect-definition-issues.unit.test.ts b/packages/project-builder-lib/src/parser/collect-definition-issues.unit.test.ts index 2661afd5a..eff18e49b 100644 --- a/packages/project-builder-lib/src/parser/collect-definition-issues.unit.test.ts +++ b/packages/project-builder-lib/src/parser/collect-definition-issues.unit.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { z } from 'zod'; import { PluginSpecStore } from '#src/plugins/index.js'; +import { extractDefinitionRefs } from '#src/references/extract-definition-refs.js'; import { definitionFieldIssueRegistry, withIssueChecker, @@ -272,7 +273,13 @@ describe('collectExpressionIssues', () => { ); const data = { name: 'test', condition: 'model.badField === auth.userId' }; - const issues = collectExpressionIssues(schema, data, pluginStore); + const parsed = schema.parse(data); + const refPayload = extractDefinitionRefs(schema, parsed); + const issues = collectExpressionIssues({ + definition: parsed, + pluginStore, + expressions: refPayload.expressions, + }); const expressionIssues = issues.filter( (i) => i.message === 'Invalid field reference', diff --git a/packages/project-builder-lib/src/parser/collect-expression-issues.ts b/packages/project-builder-lib/src/parser/collect-expression-issues.ts index fd3c5c2ee..0d27de706 100644 --- a/packages/project-builder-lib/src/parser/collect-expression-issues.ts +++ b/packages/project-builder-lib/src/parser/collect-expression-issues.ts @@ -1,41 +1,46 @@ -import type { z } from 'zod'; - import type { PluginSpecStore } from '#src/plugins/index.js'; -import type { ExpressionValidationContext } from '#src/references/expression-types.js'; +import type { + DefinitionExpression, + ExpressionValidationContext, +} from '#src/references/expression-types.js'; import type { DefinitionIssue } from '#src/schema/creator/definition-issue-types.js'; -import { extractDefinitionRefs } from '#src/references/extract-definition-refs.js'; +/** + * Input for expression issue collection. + * Satisfied by ProjectDefinitionContainer and by lightweight test fixtures. + */ +export interface CollectExpressionIssuesInput { + definition: unknown; + pluginStore: PluginSpecStore; + expressions: ReadonlyArray; +} /** - * Collects validation issues from expression parsers registered on the schema. + * Collects validation issues from expression parsers in the definition. * - * Walks the schema+data to find expression annotations, resolves their slots, - * then calls each parser's `validate()` method. Warnings are mapped to - * `DefinitionIssue` objects with warning severity. + * Uses pre-resolved expressions to avoid redundant schema walks. + * Each parser's `validate()` method is called with the expression value and + * resolved slots. Warnings are mapped to `DefinitionIssue` objects. * - * @param schema - The Zod schema to walk - * @param data - The parsed definition data - * @param pluginStore - The plugin spec store for validation context + * @param input - The definition, plugin store, and pre-resolved expressions * @returns Array of definition issues from expression validation */ export function collectExpressionIssues( - schema: z.ZodType, - data: unknown, - pluginStore: PluginSpecStore, + input: CollectExpressionIssuesInput, ): DefinitionIssue[] { - const refPayload = extractDefinitionRefs(schema, data); + const { definition, pluginStore, expressions } = input; const context: ExpressionValidationContext = { - definition: data, + definition, pluginStore, }; const issues: DefinitionIssue[] = []; - for (const expression of refPayload.expressions) { + for (const expression of expressions) { const warnings = expression.parser.validate( expression.value, - data, + definition, context, expression.resolvedSlots, ); diff --git a/packages/project-builder-lib/src/parser/collect-expression-issues.unit.test.ts b/packages/project-builder-lib/src/parser/collect-expression-issues.unit.test.ts index eb5c4ccc8..3810cd689 100644 --- a/packages/project-builder-lib/src/parser/collect-expression-issues.unit.test.ts +++ b/packages/project-builder-lib/src/parser/collect-expression-issues.unit.test.ts @@ -4,6 +4,7 @@ import { z } from 'zod'; import type { RefExpressionParser } from '#src/references/expression-types.js'; import { PluginSpecStore } from '#src/plugins/index.js'; +import { extractDefinitionRefs } from '#src/references/extract-definition-refs.js'; import { createDefinitionSchemaParserContext, definitionSchema, @@ -33,21 +34,39 @@ describe('collectExpressionIssues', () => { ); } + function buildInput( + schema: z.ZodType, + data: unknown, + ): { + definition: unknown; + pluginStore: PluginSpecStore; + expressions: ReturnType['expressions']; + } { + const parsed = schema.parse(data); + const refPayload = extractDefinitionRefs(schema, parsed); + return { + definition: parsed, + pluginStore, + expressions: refPayload.expressions, + }; + } + it('returns empty array when schema has no expressions', () => { const schema = z.object({ name: z.string() }); const issues = collectExpressionIssues( - schema, - { name: 'test' }, - pluginStore, + buildInput(schema, { name: 'test' }), ); expect(issues).toEqual([]); }); it('returns empty array when expression parser produces no warnings', () => { const schema = createSchemaWithExpression(stubParser); - const data = { name: 'test', condition: 'model.id === auth.userId' }; - - const issues = collectExpressionIssues(schema, data, pluginStore); + const issues = collectExpressionIssues( + buildInput(schema, { + name: 'test', + condition: 'model.id === auth.userId', + }), + ); expect(issues).toEqual([]); }); @@ -57,9 +76,10 @@ describe('collectExpressionIssues', () => { { message: 'Role not defined', start: 10, end: 20 }, ]); const schema = createSchemaWithExpression(warningParser); - const data = { name: 'test', condition: 'some expression' }; - const issues = collectExpressionIssues(schema, data, pluginStore); + const issues = collectExpressionIssues( + buildInput(schema, { name: 'test', condition: 'some expression' }), + ); expect(issues).toHaveLength(2); expect(issues[0]).toEqual({ @@ -89,14 +109,15 @@ describe('collectExpressionIssues', () => { const schema = schemaCreator( createDefinitionSchemaParserContext({ plugins: pluginStore }), ); - const data = { - rules: [ - { name: 'rule1', condition: 'expr1' }, - { name: 'rule2', condition: 'expr2' }, - ], - }; - const issues = collectExpressionIssues(schema, data, pluginStore); + const issues = collectExpressionIssues( + buildInput(schema, { + rules: [ + { name: 'rule1', condition: 'expr1' }, + { name: 'rule2', condition: 'expr2' }, + ], + }), + ); expect(issues).toHaveLength(2); expect(issues[0]?.path).toEqual(['rules', 0, 'condition']); @@ -106,9 +127,10 @@ describe('collectExpressionIssues', () => { it('returns parse error as warning when parse fails', () => { const failingParser = new FailingParser(); const schema = createSchemaWithExpression(failingParser); - const data = { name: 'test', condition: 'bad expression' }; - const issues = collectExpressionIssues(schema, data, pluginStore); + const issues = collectExpressionIssues( + buildInput(schema, { name: 'test', condition: 'bad expression' }), + ); expect(issues).toHaveLength(1); expect(issues[0]).toEqual({ @@ -119,12 +141,11 @@ describe('collectExpressionIssues', () => { }); it('returns empty array when schema has no expression annotations', () => { - const schema = z.object({ name: z.string() }); - const issues = collectExpressionIssues( - schema, - 'not an object', + const issues = collectExpressionIssues({ + definition: 'not an object', pluginStore, - ); + expressions: [], + }); expect(issues).toEqual([]); }); }); diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-acorn-parser.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-acorn-parser.ts similarity index 100% rename from packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-acorn-parser.ts rename to packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-acorn-parser.ts diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-acorn-parser.unit.test.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-acorn-parser.unit.test.ts similarity index 100% rename from packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-acorn-parser.unit.test.ts rename to packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-acorn-parser.unit.test.ts diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-ast.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-ast.ts similarity index 100% rename from packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-ast.ts rename to packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-ast.ts diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-parser.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts similarity index 98% rename from packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-parser.ts rename to packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts index c55f05adf..0091e35be 100644 --- a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-parser.ts +++ b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts @@ -7,18 +7,19 @@ import type { RefExpressionWarning, ResolvedExpressionSlots, } from '#src/references/expression-types.js'; +import type { modelEntityType } from '#src/schema/models/types.js'; import { RefExpressionParser } from '#src/references/expression-types.js'; +import { modelAuthorizerRoleEntityType } from '#src/schema/models/authorizer/types.js'; +import { + modelLocalRelationEntityType, + modelScalarFieldEntityType, +} from '#src/schema/models/types.js'; -import type { modelEntityType } from '../types.js'; import type { AuthorizerExpressionInfo } from './authorizer-expression-ast.js'; import type { ModelValidationContext } from './authorizer-expression-validator.js'; import type { AuthorizerExpressionVisitor } from './authorizer-expression-visitor.js'; -import { - modelLocalRelationEntityType, - modelScalarFieldEntityType, -} from '../types.js'; import { parseAuthorizerExpression } from './authorizer-expression-acorn-parser.js'; import { AuthorizerExpressionParseError } from './authorizer-expression-ast.js'; import { @@ -26,7 +27,6 @@ import { validateAuthorizerExpression, } from './authorizer-expression-validator.js'; import { visitAuthorizerExpression } from './authorizer-expression-visitor.js'; -import { modelAuthorizerRoleEntityType } from './types.js'; /** * Shape of a raw model in the project definition JSON. diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-rename.unit.test.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts similarity index 98% rename from packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-rename.unit.test.ts rename to packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts index 264e5cbb1..6dfd293d9 100644 --- a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-rename.unit.test.ts +++ b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts @@ -2,10 +2,11 @@ import { describe, expect, it } from 'vitest'; import type { RefExpressionDependency } from '#src/references/expression-types.js'; -import { modelScalarFieldEntityType } from '../types.js'; +import { modelAuthorizerRoleEntityType } from '#src/schema/models/authorizer/types.js'; +import { modelScalarFieldEntityType } from '#src/schema/models/types.js'; + import { parseAuthorizerExpression } from './authorizer-expression-acorn-parser.js'; import { AuthorizerExpressionParser } from './authorizer-expression-parser.js'; -import { modelAuthorizerRoleEntityType } from './types.js'; /** * Build a minimal definition with one model for testing getReferencedEntities. diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-validator.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-validator.ts similarity index 100% rename from packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-validator.ts rename to packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-validator.ts diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-validator.unit.test.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-validator.unit.test.ts similarity index 100% rename from packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-validator.unit.test.ts rename to packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-validator.unit.test.ts diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-visitor.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-visitor.ts similarity index 100% rename from packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-visitor.ts rename to packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-visitor.ts diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/index.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/index.ts new file mode 100644 index 000000000..ab085e929 --- /dev/null +++ b/packages/project-builder-lib/src/references/expression-parsers/authorizer/index.ts @@ -0,0 +1,5 @@ +export * from './authorizer-expression-acorn-parser.js'; +export * from './authorizer-expression-ast.js'; +export * from './authorizer-expression-parser.js'; +export * from './authorizer-expression-validator.js'; +export * from './authorizer-expression-visitor.js'; diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer.ts b/packages/project-builder-lib/src/schema/models/authorizer/authorizer.ts index 43a39dd4a..e21b53b4c 100644 --- a/packages/project-builder-lib/src/schema/models/authorizer/authorizer.ts +++ b/packages/project-builder-lib/src/schema/models/authorizer/authorizer.ts @@ -2,11 +2,11 @@ import { z } from 'zod'; import type { def } from '#src/schema/creator/index.js'; +import { authorizerExpressionParser } from '#src/references/expression-parsers/authorizer/authorizer-expression-parser.js'; import { definitionSchemaWithSlots } from '#src/schema/creator/schema-creator.js'; import { VALIDATORS } from '#src/schema/utils/validation.js'; import { modelEntityType } from '../types.js'; -import { authorizerExpressionParser } from './authorizer-expression-parser.js'; import { modelAuthorizerRoleEntityType } from './types.js'; /** diff --git a/packages/project-builder-lib/src/schema/models/authorizer/index.ts b/packages/project-builder-lib/src/schema/models/authorizer/index.ts index 894879d1e..758e1ebcd 100644 --- a/packages/project-builder-lib/src/schema/models/authorizer/index.ts +++ b/packages/project-builder-lib/src/schema/models/authorizer/index.ts @@ -1,7 +1,3 @@ -export * from './authorizer-expression-acorn-parser.js'; -export * from './authorizer-expression-ast.js'; -export * from './authorizer-expression-parser.js'; -export * from './authorizer-expression-validator.js'; -export * from './authorizer-expression-visitor.js'; +export * from '#src/references/expression-parsers/authorizer/index.js'; export * from './authorizer.js'; export * from './types.js'; From 627581d1838f71f1f2a3d1b65ddffacfd3735368 Mon Sep 17 00:00:00 2001 From: Kingston Date: Fri, 20 Mar 2026 07:11:57 -0700 Subject: [PATCH 3/8] Implement expression parser refs --- packages/project-builder-lib/src/index.ts | 1 + .../src/parser/collect-expression-issues.ts | 2 +- .../src/references/expression-parser-ref.ts | 67 ++++++++++++++ .../src/references/expression-parser-spec.ts | 39 ++++++++ .../authorizer-expression-rename.unit.test.ts | 11 +-- .../register-core-module.ts | 35 ++++++++ .../src/references/expression-types.ts | 7 +- .../extend-parser-context-with-refs.ts | 89 +++++++++++++------ .../src/references/extract-definition-refs.ts | 2 +- .../src/references/fix-expression-renames.ts | 7 +- .../fix-expression-renames.unit.test.ts | 18 ---- .../src/references/index.ts | 2 + .../parse-schema-with-references.ts | 5 +- .../src/schema/creator/schema-creator.ts | 2 +- .../authorizer/authorizer-expression-ref.ts | 16 ++++ .../schema/models/authorizer/authorizer.ts | 4 +- .../src/testing/parser-context.test-helper.ts | 3 +- ...roject-definition-container.test-helper.ts | 3 +- .../src/core-modules/index.ts | 25 +++--- .../src/core-modules/index.ts | 27 +++--- 20 files changed, 277 insertions(+), 88 deletions(-) create mode 100644 packages/project-builder-lib/src/references/expression-parser-ref.ts create mode 100644 packages/project-builder-lib/src/references/expression-parser-spec.ts create mode 100644 packages/project-builder-lib/src/references/expression-parsers/register-core-module.ts create mode 100644 packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-ref.ts diff --git a/packages/project-builder-lib/src/index.ts b/packages/project-builder-lib/src/index.ts index 23668cadb..357a50eff 100644 --- a/packages/project-builder-lib/src/index.ts +++ b/packages/project-builder-lib/src/index.ts @@ -6,6 +6,7 @@ export * from './migrations/index.js'; export * from './parser/index.js'; export * from './plugins/index.js'; export * from './references/index.js'; +export { expressionParserCoreModule } from './references/expression-parsers/register-core-module.js'; export * from './schema/index.js'; export * from './specs/index.js'; export * from './tools/index.js'; diff --git a/packages/project-builder-lib/src/parser/collect-expression-issues.ts b/packages/project-builder-lib/src/parser/collect-expression-issues.ts index 0d27de706..36ada838d 100644 --- a/packages/project-builder-lib/src/parser/collect-expression-issues.ts +++ b/packages/project-builder-lib/src/parser/collect-expression-issues.ts @@ -12,7 +12,7 @@ import type { DefinitionIssue } from '#src/schema/creator/definition-issue-types export interface CollectExpressionIssuesInput { definition: unknown; pluginStore: PluginSpecStore; - expressions: ReadonlyArray; + expressions: readonly DefinitionExpression[]; } /** diff --git a/packages/project-builder-lib/src/references/expression-parser-ref.ts b/packages/project-builder-lib/src/references/expression-parser-ref.ts new file mode 100644 index 000000000..4a0c19156 --- /dev/null +++ b/packages/project-builder-lib/src/references/expression-parser-ref.ts @@ -0,0 +1,67 @@ +import type { z } from 'zod'; + +import type { DefinitionEntityType } from './types.js'; + +/** + * A lightweight reference to an expression parser registered in the + * `expressionParserSpec`. + * + * Used in schema definitions instead of importing the full parser class + * directly. The actual parser is resolved at runtime during schema+data + * walking via the plugin spec store. + * + * Phantom type parameters enforce slot requirements at compile time + * without requiring the parser implementation to be imported. + * + * @typeParam TValue - The type of the raw expression value (e.g., string) + * @typeParam TRequiredSlots - Record of required slot names to entity types + * + * @example + * ```typescript + * const authorizerExpressionRef = createExpressionParserRef< + * string, + * { model: typeof modelEntityType } + * >('authorizer-expression', z.string().min(1, 'Expression is required')); + * ``` + */ +export interface ExpressionParserRef< + TValue = unknown, + TRequiredSlots extends Record = Record< + string, + never + >, +> { + /** Unique name matching the parser registered in expressionParserSpec */ + readonly name: string; + /** + * Creates a fresh Zod schema instance for basic validation. + * Must return a new instance per call to avoid shared metadata conflicts + * when the same ref is used at multiple schema sites. + */ + readonly createSchema: () => z.ZodType; + /** @internal Phantom type for slot enforcement */ + readonly _slots?: TRequiredSlots; +} + +/** + * Creates a typed reference to an expression parser. + * + * The ref carries a parser name and a schema factory for basic value validation. + * The actual parser implementation is looked up from `expressionParserSpec` + * at runtime. + * + * @param name - Unique name matching the registered parser + * @param createSchema - Factory that returns a fresh Zod schema per call site + */ +export function createExpressionParserRef< + TValue, + TRequiredSlots extends Record = Record< + string, + never + >, +>( + name: string, + createSchema: () => z.ZodType, +): ExpressionParserRef { + return { name, createSchema }; +} diff --git a/packages/project-builder-lib/src/references/expression-parser-spec.ts b/packages/project-builder-lib/src/references/expression-parser-spec.ts new file mode 100644 index 000000000..a0f33ca12 --- /dev/null +++ b/packages/project-builder-lib/src/references/expression-parser-spec.ts @@ -0,0 +1,39 @@ +import { createFieldMapSpec } from '#src/plugins/utils/create-field-map-spec.js'; + +import type { RefExpressionParser } from './expression-types.js'; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type AnyExpressionParser = RefExpressionParser; + +/** + * Plugin spec for registering expression parsers. + * + * Expression parsers handle parsing, validation, and rename detection + * for expression fields in the project definition (e.g., authorizer + * role expressions). + * + * Built-in parsers (authorizer expressions) are registered by core modules. + * Plugins can register additional parsers during initialization. + * + * @example + * ```typescript + * createPluginModule({ + * dependencies: { expressionParsers: expressionParserSpec }, + * initialize: ({ expressionParsers }) => { + * expressionParsers.parsers.set('my-expression', myParser); + * }, + * }); + * ``` + */ +export const expressionParserSpec = createFieldMapSpec( + 'core/expression-parsers', + (t) => ({ + parsers: t.map(), + }), + { + use: (values) => ({ + getParser: (name: string): AnyExpressionParser | undefined => + values.parsers.get(name), + }), + }, +); diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts index 6dfd293d9..a0734b030 100644 --- a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts +++ b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts @@ -79,14 +79,11 @@ function applyRenames( renames: Map, ): string { const replacements = deps - .filter( - (ref) => - ref.start != null && ref.end != null && renames.has(ref.entityId), - ) + .filter((ref) => renames.has(ref.entityId)) .map((ref) => ({ - start: ref.start!, - end: ref.end!, - newValue: renames.get(ref.entityId)!, + start: ref.start, + end: ref.end, + newValue: renames.get(ref.entityId) ?? '', })) .toSorted((a, b) => b.start - a.start); diff --git a/packages/project-builder-lib/src/references/expression-parsers/register-core-module.ts b/packages/project-builder-lib/src/references/expression-parsers/register-core-module.ts new file mode 100644 index 000000000..018254ab3 --- /dev/null +++ b/packages/project-builder-lib/src/references/expression-parsers/register-core-module.ts @@ -0,0 +1,35 @@ +import type { PluginModuleWithKey } from '#src/plugins/imports/types.js'; + +import { createPluginModule } from '#src/plugins/imports/types.js'; + +import { expressionParserSpec } from '../expression-parser-spec.js'; +import { authorizerExpressionParser } from './authorizer/authorizer-expression-parser.js'; + +/** + * Core module that registers built-in expression parsers. + * + * This module is included in every consumer's coreModules array + * (server, web, CLI, tests) to ensure the authorizer expression parser + * is available for schemas that use `withExpression(authorizerExpressionRef, ...)`. + */ +const registerExpressionParsersModule = createPluginModule({ + name: 'register-expression-parsers', + dependencies: { + expressionParsers: expressionParserSpec, + }, + initialize: ({ expressionParsers }) => { + expressionParsers.parsers.set( + authorizerExpressionParser.name, + authorizerExpressionParser, + ); + }, +}); + +/** + * Core module with key for inclusion in PluginStore.coreModules. + */ +export const expressionParserCoreModule: PluginModuleWithKey = { + key: 'core/lib/expression-parsers', + pluginKey: 'core', + module: registerExpressionParsersModule, +}; diff --git a/packages/project-builder-lib/src/references/expression-types.ts b/packages/project-builder-lib/src/references/expression-types.ts index bf51dd2ad..a02ca93e6 100644 --- a/packages/project-builder-lib/src/references/expression-types.ts +++ b/packages/project-builder-lib/src/references/expression-types.ts @@ -50,9 +50,10 @@ export interface RefExpressionDependency { entityType: DefinitionEntityType; /** The ID of the entity being referenced */ entityId: string; - /** Position in the expression for rename updates */ - start?: number; - end?: number; + /** Start position in the expression text to replace on rename */ + start: number; + /** End position in the expression text to replace on rename */ + end: number; } /** diff --git a/packages/project-builder-lib/src/references/extend-parser-context-with-refs.ts b/packages/project-builder-lib/src/references/extend-parser-context-with-refs.ts index b42cdf3d9..417585408 100644 --- a/packages/project-builder-lib/src/references/extend-parser-context-with-refs.ts +++ b/packages/project-builder-lib/src/references/extend-parser-context-with-refs.ts @@ -3,15 +3,14 @@ import type { TuplePaths } from '@baseplate-dev/utils'; import { z } from 'zod'; import type { DefinitionEntityType } from '#src/index.js'; +import type { PluginSpecStore } from '#src/plugins/index.js'; import type { DefinitionEntityInput, DefinitionReferenceInput, } from './definition-ref-builder.js'; -import type { - ExpressionSlotMap, - RefExpressionParser, -} from './expression-types.js'; +import type { ExpressionParserRef } from './expression-parser-ref.js'; +import type { ExpressionSlotMap } from './expression-types.js'; import type { RefContextSlot, RefContextSlotDefinition, @@ -19,6 +18,8 @@ import type { } from './ref-context-slot.js'; import { definitionRefRegistry } from './definition-ref-registry.js'; +import { expressionParserSpec } from './expression-parser-spec.js'; +import { RefExpressionParser } from './expression-types.js'; import { createRefContextSlotMap } from './ref-context-slot.js'; type ZodTypeWithOptional = T extends z.ZodOptional @@ -62,12 +63,12 @@ export type RefContextType = < * provided as the second argument. TypeScript enforces this at compile time. */ export interface WithExpressionType { - // Overload for parsers with no required slots + // Overload for direct parser with no required slots ( parser: RefExpressionParser, ): z.ZodType; - // Overload for parsers with required slots + // Overload for direct parser with required slots < TValue, TParseResult, @@ -76,6 +77,15 @@ export interface WithExpressionType { parser: RefExpressionParser, slots: ExpressionSlotMap, ): z.ZodType; + + // Overload for parser ref with no required slots + (parserRef: ExpressionParserRef): z.ZodType; + + // Overload for parser ref with required slots + >( + parserRef: ExpressionParserRef, + slots: ExpressionSlotMap, + ): z.ZodType; } /** @@ -201,28 +211,55 @@ function refContext< * ); * ``` */ -function withExpression< - TValue, - TParseResult, - TRequiredSlots extends Record, ->( - parser: RefExpressionParser, - slots?: ExpressionSlotMap, -): z.ZodType { - // createSchema() returns a fresh instance per call, allowing per-call - // metadata to be registered without conflicting across call sites. - const schema = parser.createSchema() as z.ZodType; - - definitionRefRegistry.add(schema, { - kind: 'expression', +/** + * Creates a `withExpression` function that resolves parser refs eagerly + * from the plugin spec store at schema construction time. + */ +function createWithExpression(plugins?: PluginSpecStore): WithExpressionType { + // Implementation + function withExpression( + parserOrRef: // eslint-disable-next-line @typescript-eslint/no-explicit-any + | RefExpressionParser + // eslint-disable-next-line @typescript-eslint/no-explicit-any + | ExpressionParserRef, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + slots?: ExpressionSlotMap, + ): z.ZodType { + // oxlint-disable-next-line typescript/no-explicit-any + let parser: RefExpressionParser; + if (parserOrRef instanceof RefExpressionParser) { + parser = parserOrRef; + } else { + // Resolve parser ref from plugin spec store + if (!plugins) { + throw new Error( + `PluginSpecStore is required to resolve expression parser ref "${parserOrRef.name}". ` + + `Ensure plugins are provided to createDefinitionSchemaParserContext.`, + ); + } + const specUse = plugins.use(expressionParserSpec); + const resolved = specUse.getParser(parserOrRef.name); + if (!resolved) { + throw new Error( + `Expression parser "${parserOrRef.name}" not found in expressionParserSpec. ` + + `Ensure it is registered via a core module or plugin.`, + ); + } + parser = resolved; + } - parser, - slots, - }); - return schema; + const schema = parser.createSchema(); + definitionRefRegistry.add(schema, { + kind: 'expression', + parser, + slots, + }); + return schema; + } + return withExpression as WithExpressionType; } -export function extendParserContextWithRefs(): { +export function extendParserContextWithRefs(plugins?: PluginSpecStore): { withRef: WithRefType; withEnt: WithEntType; refContext: RefContextType; @@ -246,6 +283,6 @@ export function extendParserContextWithRefs(): { ); }, refContext, - withExpression, + withExpression: createWithExpression(plugins), }; } diff --git a/packages/project-builder-lib/src/references/extract-definition-refs.ts b/packages/project-builder-lib/src/references/extract-definition-refs.ts index 37ca3f8a0..6e78a6650 100644 --- a/packages/project-builder-lib/src/references/extract-definition-refs.ts +++ b/packages/project-builder-lib/src/references/extract-definition-refs.ts @@ -128,7 +128,7 @@ export function extractDefinitionRefs( }, }); - // Expression collector: records expression annotations + // Expression collector: records expression annotations (direct parsers) const expressionCollector = createRefSchemaCollector({ kind: 'expression', visit(meta, data, context): void { diff --git a/packages/project-builder-lib/src/references/fix-expression-renames.ts b/packages/project-builder-lib/src/references/fix-expression-renames.ts index 952044bc8..11baddbc6 100644 --- a/packages/project-builder-lib/src/references/fix-expression-renames.ts +++ b/packages/project-builder-lib/src/references/fix-expression-renames.ts @@ -26,7 +26,7 @@ export interface ApplyExpressionRenamesResult { */ export function applyExpressionRenames( newDefinition: T, - newEntities: ReadonlyArray, + newEntities: readonly DefinitionEntity[], oldRefPayload: ResolvedZodRefPayload, ): ApplyExpressionRenamesResult { const { expressions: oldExpressions, entities: oldEntities } = oldRefPayload; @@ -73,12 +73,9 @@ export function applyExpressionRenames( expression.resolvedSlots, ); - // Build replacements for renamed entities that have positions + // Build replacements for renamed entities const replacements: { start: number; end: number; newValue: string }[] = []; for (const ref of refs) { - if (ref.start == null || ref.end == null) { - continue; - } const newName = renames.get(ref.entityId); if (newName !== undefined) { replacements.push({ diff --git a/packages/project-builder-lib/src/references/fix-expression-renames.unit.test.ts b/packages/project-builder-lib/src/references/fix-expression-renames.unit.test.ts index 8fbaa111e..ef9f887cd 100644 --- a/packages/project-builder-lib/src/references/fix-expression-renames.unit.test.ts +++ b/packages/project-builder-lib/src/references/fix-expression-renames.unit.test.ts @@ -175,24 +175,6 @@ describe('applyExpressionRenames', () => { expect(result.value).toEqual({ expr: 'hello' }); }); - it('should skip dependencies without positions', () => { - const parser = new FakeParser([ - { entityType: testEntityType, entityId: 'e:1' }, // no start/end - ]); - - const result = applyExpressionRenames( - { expr: 'hello' }, - [makeEntity('e:1', 'world')], - makeOldPayload( - { expr: 'hello' }, - [makeEntity('e:1', 'hello')], - [makeExpression('hello', parser, ['expr'])], - ), - ); - - expect(result.modified).toBe(false); - }); - it('should skip dependencies whose entity ID was not renamed', () => { const parser = new FakeParser([ { entityType: testEntityType, entityId: 'e:2', start: 0, end: 5 }, diff --git a/packages/project-builder-lib/src/references/index.ts b/packages/project-builder-lib/src/references/index.ts index d8aa931ca..7d69555df 100644 --- a/packages/project-builder-lib/src/references/index.ts +++ b/packages/project-builder-lib/src/references/index.ts @@ -1,6 +1,8 @@ export * from './definition-ref-builder.js'; export * from './definition-ref-registry.js'; export * from './deserialize-schema.js'; +export * from './expression-parser-ref.js'; +export * from './expression-parser-spec.js'; export * from './expression-types.js'; export { withEnt, withRef } from './extend-parser-context-with-refs.js'; export * from './extract-definition-refs.js'; diff --git a/packages/project-builder-lib/src/references/parse-schema-with-references.ts b/packages/project-builder-lib/src/references/parse-schema-with-references.ts index 36e0e68e4..526b68dc7 100644 --- a/packages/project-builder-lib/src/references/parse-schema-with-references.ts +++ b/packages/project-builder-lib/src/references/parse-schema-with-references.ts @@ -6,6 +6,9 @@ import type { ResolvedZodRefPayload } from './types.js'; import { extractDefinitionRefs } from './extract-definition-refs.js'; import { resolveZodRefPayloadNames } from './resolve-zod-ref-payload-names.js'; +export type ParseSchemaWithTransformedReferencesOptions = + ResolveZodRefPayloadNamesOptions; + /** * Parses a schema with references. * @@ -20,7 +23,7 @@ import { resolveZodRefPayloadNames } from './resolve-zod-ref-payload-names.js'; export function parseSchemaWithTransformedReferences( schema: T, input: unknown, - options?: ResolveZodRefPayloadNamesOptions, + options?: ParseSchemaWithTransformedReferencesOptions, ): ResolvedZodRefPayload> { // Step 1: Validate with Zod const value = schema.parse(input); diff --git a/packages/project-builder-lib/src/schema/creator/schema-creator.ts b/packages/project-builder-lib/src/schema/creator/schema-creator.ts index 5301b04d2..5bdaed719 100644 --- a/packages/project-builder-lib/src/schema/creator/schema-creator.ts +++ b/packages/project-builder-lib/src/schema/creator/schema-creator.ts @@ -31,7 +31,7 @@ export function createDefinitionSchemaParserContext( const context: DefinitionSchemaParserContext = { ...options, - ...extendParserContextWithRefs(), + ...extendParserContextWithRefs(options.plugins), ...extendParserContextWithDefaults(), }; contextCache.set(options.plugins, context); diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-ref.ts b/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-ref.ts new file mode 100644 index 000000000..78fd7bbe4 --- /dev/null +++ b/packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-ref.ts @@ -0,0 +1,16 @@ +import { z } from 'zod'; + +import { createExpressionParserRef } from '#src/references/expression-parser-ref.js'; + +import type { modelEntityType } from '../types.js'; + +/** + * Typed reference to the authorizer expression parser. + * + * Used in schema definitions instead of importing the full parser class. + * The actual parser is resolved at runtime from `expressionParserSpec`. + */ +export const authorizerExpressionRef = createExpressionParserRef< + string, + { model: typeof modelEntityType } +>('authorizer-expression', () => z.string().min(1, 'Expression is required')); diff --git a/packages/project-builder-lib/src/schema/models/authorizer/authorizer.ts b/packages/project-builder-lib/src/schema/models/authorizer/authorizer.ts index e21b53b4c..cbb569c48 100644 --- a/packages/project-builder-lib/src/schema/models/authorizer/authorizer.ts +++ b/packages/project-builder-lib/src/schema/models/authorizer/authorizer.ts @@ -2,11 +2,11 @@ import { z } from 'zod'; import type { def } from '#src/schema/creator/index.js'; -import { authorizerExpressionParser } from '#src/references/expression-parsers/authorizer/authorizer-expression-parser.js'; import { definitionSchemaWithSlots } from '#src/schema/creator/schema-creator.js'; import { VALIDATORS } from '#src/schema/utils/validation.js'; import { modelEntityType } from '../types.js'; +import { authorizerExpressionRef } from './authorizer-expression-ref.js'; import { modelAuthorizerRoleEntityType } from './types.js'; /** @@ -40,7 +40,7 @@ export const createAuthorizerRoleSchema = definitionSchemaWithSlots( * @example 'hasRole("admin")' * @example 'model.authorId === userId || hasRole("admin")' */ - expression: ctx.withExpression(authorizerExpressionParser, { + expression: ctx.withExpression(authorizerExpressionRef, { model: modelSlot, }), }), diff --git a/packages/project-builder-lib/src/testing/parser-context.test-helper.ts b/packages/project-builder-lib/src/testing/parser-context.test-helper.ts index 4dcc6a814..19848961e 100644 --- a/packages/project-builder-lib/src/testing/parser-context.test-helper.ts +++ b/packages/project-builder-lib/src/testing/parser-context.test-helper.ts @@ -1,11 +1,12 @@ import type { DefinitionSchemaParserContext } from '../schema/creator/types.js'; import { createPluginSpecStore } from '../parser/parser.js'; +import { expressionParserCoreModule } from '../references/expression-parsers/register-core-module.js'; import { createDefinitionSchemaParserContext } from '../schema/creator/schema-creator.js'; const emptyPluginStore = { availablePlugins: [], - coreModules: [], + coreModules: [expressionParserCoreModule], }; /** diff --git a/packages/project-builder-lib/src/testing/project-definition-container.test-helper.ts b/packages/project-builder-lib/src/testing/project-definition-container.test-helper.ts index f70ef9e08..3ea12b5fb 100644 --- a/packages/project-builder-lib/src/testing/project-definition-container.test-helper.ts +++ b/packages/project-builder-lib/src/testing/project-definition-container.test-helper.ts @@ -9,6 +9,7 @@ import { ProjectDefinitionContainer } from '#src/definition/project-definition-c import { getLatestMigrationVersion } from '#src/migrations/index.js'; import { createPluginSpecStore } from '#src/parser/parser.js'; import { deserializeSchemaWithTransformedReferences } from '#src/references/deserialize-schema.js'; +import { expressionParserCoreModule } from '#src/references/expression-parsers/register-core-module.js'; import { createDefinitionSchemaParserContext } from '#src/schema/index.js'; import { createProjectDefinitionSchema } from '#src/schema/project-definition.js'; @@ -48,7 +49,7 @@ export function createTestProjectDefinitionContainer( ): ProjectDefinitionContainer { const pluginStore: PluginStore = { availablePlugins: [], - coreModules: [], + coreModules: [expressionParserCoreModule], }; const pluginSpecStore = createPluginSpecStore(pluginStore, { plugins: [], diff --git a/packages/project-builder-server/src/core-modules/index.ts b/packages/project-builder-server/src/core-modules/index.ts index d6a295d0f..f296f514f 100644 --- a/packages/project-builder-server/src/core-modules/index.ts +++ b/packages/project-builder-server/src/core-modules/index.ts @@ -1,5 +1,7 @@ import type { PluginModuleWithKey } from '@baseplate-dev/project-builder-lib'; +import { expressionParserCoreModule } from '@baseplate-dev/project-builder-lib'; + import { adminCrudActionCoreModule } from './admin-crud-action-compiler.js'; import { adminCrudColumnCoreModule } from './admin-crud-column-compiler.js'; import { adminCrudInputCoreModule } from './admin-crud-input-compiler.js'; @@ -7,13 +9,16 @@ import { libraryTypeCoreModule } from './library-type-spec.js'; import { modelTransformerCoreModule } from './model-transformer-compiler.js'; export const SERVER_CORE_MODULES: PluginModuleWithKey[] = [ - adminCrudActionCoreModule, - modelTransformerCoreModule, - adminCrudColumnCoreModule, - adminCrudInputCoreModule, - libraryTypeCoreModule, -].map((module) => ({ - key: `core/server/${module.name}`, - pluginKey: 'core', - module, -})); + expressionParserCoreModule, + ...[ + adminCrudActionCoreModule, + modelTransformerCoreModule, + adminCrudColumnCoreModule, + adminCrudInputCoreModule, + libraryTypeCoreModule, + ].map((module) => ({ + key: `core/server/${module.name}`, + pluginKey: 'core', + module, + })), +]; diff --git a/packages/project-builder-web/src/core-modules/index.ts b/packages/project-builder-web/src/core-modules/index.ts index 8b3f6b7cc..502669299 100644 --- a/packages/project-builder-web/src/core-modules/index.ts +++ b/packages/project-builder-web/src/core-modules/index.ts @@ -1,5 +1,7 @@ import type { PluginModuleWithKey } from '@baseplate-dev/project-builder-lib'; +import { expressionParserCoreModule } from '@baseplate-dev/project-builder-lib'; + import { actionWebConfigsCoreModule } from './action-web-configs.js'; import { columnWebConfigsCoreModule } from './column-web-configs.js'; import { entityTypeUrlsCoreModule } from './entity-type-urls-core-module.js'; @@ -8,14 +10,17 @@ import { libraryTypeWebConfigsCoreModule } from './library-type-web-configs.js'; import { transformerWebConfigsCoreModule } from './transformer-web-configs.js'; export const WEB_CORE_MODULES: PluginModuleWithKey[] = [ - columnWebConfigsCoreModule, - actionWebConfigsCoreModule, - transformerWebConfigsCoreModule, - inputWebConfigsCoreModule, - libraryTypeWebConfigsCoreModule, - entityTypeUrlsCoreModule, -].map((mod) => ({ - key: `core/web/${mod.name}`, - pluginKey: 'core', - module: mod, -})); + expressionParserCoreModule, + ...[ + columnWebConfigsCoreModule, + actionWebConfigsCoreModule, + transformerWebConfigsCoreModule, + inputWebConfigsCoreModule, + libraryTypeWebConfigsCoreModule, + entityTypeUrlsCoreModule, + ].map((mod) => ({ + key: `core/web/${mod.name}`, + pluginKey: 'core', + module: mod, + })), +]; From f00ee14e1348092eea47d2738d80c37b302240bc Mon Sep 17 00:00:00 2001 From: Kingston Date: Fri, 20 Mar 2026 07:19:25 -0700 Subject: [PATCH 4/8] Clean up types --- .../authorizer-expression-parser.ts | 44 ++++--------------- 1 file changed, 9 insertions(+), 35 deletions(-) diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts index 0091e35be..18dff3f90 100644 --- a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts +++ b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts @@ -7,7 +7,9 @@ import type { RefExpressionWarning, ResolvedExpressionSlots, } from '#src/references/expression-types.js'; +import type { ModelConfig } from '#src/schema/models/models.js'; import type { modelEntityType } from '#src/schema/models/types.js'; +import type { ProjectDefinition } from '#src/schema/project-definition.js'; import { RefExpressionParser } from '#src/references/expression-types.js'; import { modelAuthorizerRoleEntityType } from '#src/schema/models/authorizer/types.js'; @@ -28,28 +30,6 @@ import { } from './authorizer-expression-validator.js'; import { visitAuthorizerExpression } from './authorizer-expression-visitor.js'; -/** - * Shape of a raw model in the project definition JSON. - * Used for navigating the untyped definition to extract relation and authorizer info. - */ -interface RawModelDefinition { - id?: string; - name?: string; - model?: { - fields?: { id?: string; name: string; type?: string }[]; - relations?: { - id?: string; - name: string; - modelRef: string; - foreignRelationName?: string; - references?: { localRef: string; foreignRef: string }[]; - }[]; - }; - authorizer?: { - roles?: { id?: string; name: string }[]; - }; -} - /** * Expression parser for model authorizer role expressions. * @@ -148,11 +128,8 @@ export class AuthorizerExpressionParser extends RefExpressionParser< const model = this.getRawModel(definition, resolvedSlots); - const allModels = ( - (definition as { models?: RawModelDefinition[] }).models ?? [] - ).filter( - (m): m is RawModelDefinition & { name: string } => - typeof m.name === 'string', + const allModels = ((definition as ProjectDefinition).models ?? []).filter( + (m): m is ModelConfig => typeof m.name === 'string', ); // Build lookup maps @@ -173,7 +150,7 @@ export class AuthorizerExpressionParser extends RefExpressionParser< } } - const modelById = new Map(); + const modelById = new Map(); for (const m of allModels) { if (m.id) { modelById.set(m.id, m); @@ -329,7 +306,7 @@ export class AuthorizerExpressionParser extends RefExpressionParser< private getRawModel( definition: unknown, resolvedSlots: ResolvedExpressionSlots<{ model: typeof modelEntityType }>, - ): RawModelDefinition & { name: string } { + ): ModelConfig { const modelPath = resolvedSlots.model; // Walk progressively shorter paths to find the model object. @@ -351,7 +328,7 @@ export class AuthorizerExpressionParser extends RefExpressionParser< 'name' in current && typeof (current as Record).name === 'string' ) { - return current as RawModelDefinition & { name: string }; + return current as ModelConfig; } } @@ -367,11 +344,8 @@ export class AuthorizerExpressionParser extends RefExpressionParser< ): ModelValidationContext { const model = this.getRawModel(definition, resolvedSlots); - const allModels = ( - (definition as { models?: RawModelDefinition[] }).models ?? [] - ).filter( - (m): m is RawModelDefinition & { name: string } => - typeof m.name === 'string', + const allModels = ((definition as ProjectDefinition).models ?? []).filter( + (m): m is ModelConfig => typeof m.name === 'string', ); return buildModelExpressionContext( From 2e8657ec0ce32b8c21ec92f9542833c5527f8432 Mon Sep 17 00:00:00 2001 From: Kingston Date: Fri, 20 Mar 2026 07:54:28 -0700 Subject: [PATCH 5/8] Use ProjectDefinition types in authorizers --- .../collect-definition-issues.unit.test.ts | 4 +- .../src/parser/collect-expression-issues.ts | 3 +- .../collect-expression-issues.unit.test.ts | 14 +- .../authorizer-expression-parser.ts | 10 +- .../authorizer-expression-rename.unit.test.ts | 286 ++++++++++-------- .../src/references/expression-types.ts | 15 +- .../src/references/fix-expression-renames.ts | 8 +- 7 files changed, 189 insertions(+), 151 deletions(-) diff --git a/packages/project-builder-lib/src/parser/collect-definition-issues.unit.test.ts b/packages/project-builder-lib/src/parser/collect-definition-issues.unit.test.ts index eff18e49b..bf459f7c9 100644 --- a/packages/project-builder-lib/src/parser/collect-definition-issues.unit.test.ts +++ b/packages/project-builder-lib/src/parser/collect-definition-issues.unit.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from 'vitest'; import { z } from 'zod'; +import type { ProjectDefinition } from '#src/schema/project-definition.js'; + import { PluginSpecStore } from '#src/plugins/index.js'; import { extractDefinitionRefs } from '#src/references/extract-definition-refs.js'; import { @@ -276,7 +278,7 @@ describe('collectExpressionIssues', () => { const parsed = schema.parse(data); const refPayload = extractDefinitionRefs(schema, parsed); const issues = collectExpressionIssues({ - definition: parsed, + definition: parsed as unknown as ProjectDefinition, pluginStore, expressions: refPayload.expressions, }); diff --git a/packages/project-builder-lib/src/parser/collect-expression-issues.ts b/packages/project-builder-lib/src/parser/collect-expression-issues.ts index 36ada838d..e2ba84ad4 100644 --- a/packages/project-builder-lib/src/parser/collect-expression-issues.ts +++ b/packages/project-builder-lib/src/parser/collect-expression-issues.ts @@ -4,13 +4,14 @@ import type { ExpressionValidationContext, } from '#src/references/expression-types.js'; import type { DefinitionIssue } from '#src/schema/creator/definition-issue-types.js'; +import type { ProjectDefinition } from '#src/schema/project-definition.js'; /** * Input for expression issue collection. * Satisfied by ProjectDefinitionContainer and by lightweight test fixtures. */ export interface CollectExpressionIssuesInput { - definition: unknown; + definition: ProjectDefinition; pluginStore: PluginSpecStore; expressions: readonly DefinitionExpression[]; } diff --git a/packages/project-builder-lib/src/parser/collect-expression-issues.unit.test.ts b/packages/project-builder-lib/src/parser/collect-expression-issues.unit.test.ts index 3810cd689..d89dd599c 100644 --- a/packages/project-builder-lib/src/parser/collect-expression-issues.unit.test.ts +++ b/packages/project-builder-lib/src/parser/collect-expression-issues.unit.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { z } from 'zod'; import type { RefExpressionParser } from '#src/references/expression-types.js'; +import type { ProjectDefinition } from '#src/schema/project-definition.js'; import { PluginSpecStore } from '#src/plugins/index.js'; import { extractDefinitionRefs } from '#src/references/extract-definition-refs.js'; @@ -15,6 +16,8 @@ import { WarningParser, } from '#src/testing/expression-warning-parser.test-helper.js'; +import type { CollectExpressionIssuesInput } from './collect-expression-issues.js'; + import { collectExpressionIssues } from './collect-expression-issues.js'; describe('collectExpressionIssues', () => { @@ -37,15 +40,12 @@ describe('collectExpressionIssues', () => { function buildInput( schema: z.ZodType, data: unknown, - ): { - definition: unknown; - pluginStore: PluginSpecStore; - expressions: ReturnType['expressions']; - } { + ): CollectExpressionIssuesInput { const parsed = schema.parse(data); const refPayload = extractDefinitionRefs(schema, parsed); return { - definition: parsed, + // Cast: test schemas produce mock data, not real ProjectDefinition + definition: parsed as ProjectDefinition, pluginStore, expressions: refPayload.expressions, }; @@ -142,7 +142,7 @@ describe('collectExpressionIssues', () => { it('returns empty array when schema has no expression annotations', () => { const issues = collectExpressionIssues({ - definition: 'not an object', + definition: 'not an object' as unknown as ProjectDefinition, pluginStore, expressions: [], }); diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts index 18dff3f90..ca068c8ac 100644 --- a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts +++ b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts @@ -119,7 +119,7 @@ export class AuthorizerExpressionParser extends RefExpressionParser< getReferencedEntities( _value: string, parseResult: RefExpressionParseResult, - definition: unknown, + definition: ProjectDefinition, resolvedSlots: ResolvedExpressionSlots<{ model: typeof modelEntityType }>, ): RefExpressionDependency[] { if (!parseResult.success) { @@ -128,7 +128,7 @@ export class AuthorizerExpressionParser extends RefExpressionParser< const model = this.getRawModel(definition, resolvedSlots); - const allModels = ((definition as ProjectDefinition).models ?? []).filter( + const allModels = (definition.models ?? []).filter( (m): m is ModelConfig => typeof m.name === 'string', ); @@ -304,7 +304,7 @@ export class AuthorizerExpressionParser extends RefExpressionParser< * so we walk parent paths until we find an object with a string `name` property. */ private getRawModel( - definition: unknown, + definition: ProjectDefinition, resolvedSlots: ResolvedExpressionSlots<{ model: typeof modelEntityType }>, ): ModelConfig { const modelPath = resolvedSlots.model; @@ -339,12 +339,12 @@ export class AuthorizerExpressionParser extends RefExpressionParser< * Extract model context from the project definition using resolved slots. */ private getModelContext( - definition: unknown, + definition: ProjectDefinition, resolvedSlots: ResolvedExpressionSlots<{ model: typeof modelEntityType }>, ): ModelValidationContext { const model = this.getRawModel(definition, resolvedSlots); - const allModels = ((definition as ProjectDefinition).models ?? []).filter( + const allModels = (definition.models ?? []).filter( (m): m is ModelConfig => typeof m.name === 'string', ); diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts index a0734b030..24488a448 100644 --- a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts +++ b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts @@ -1,66 +1,62 @@ import { describe, expect, it } from 'vitest'; import type { RefExpressionDependency } from '#src/references/expression-types.js'; +import type { ModelConfigInput } from '#src/schema/models/models.js'; +import type { ProjectDefinition } from '#src/schema/project-definition.js'; import { modelAuthorizerRoleEntityType } from '#src/schema/models/authorizer/types.js'; -import { modelScalarFieldEntityType } from '#src/schema/models/types.js'; +import { + modelLocalRelationEntityType, + modelScalarFieldEntityType, +} from '#src/schema/models/types.js'; +import { createTestModel } from '#src/testing/definition-helpers.test-helper.js'; +import { createTestProjectDefinition } from '#src/testing/project-definition-container.test-helper.js'; import { parseAuthorizerExpression } from './authorizer-expression-acorn-parser.js'; import { AuthorizerExpressionParser } from './authorizer-expression-parser.js'; /** - * Build a minimal definition with one model for testing getReferencedEntities. + * Build a definition with models for testing getReferencedEntities. + * Returns properly-typed objects from createTestModel/createTestProjectDefinition. */ function buildDefinition( - model: { - fields?: { id: string; name: string }[]; - relations?: { - id: string; - name: string; - modelRef: string; - }[]; - }, - otherModels?: { - id: string; - name: string; - fields?: { id: string; name: string }[]; - authorizer?: { roles?: { id: string; name: string }[] }; - }[], -): { definition: unknown; resolvedSlots: { model: (string | number)[] } } { - const mainModel = { + mainModelInput: TestModelOverrides, + otherModelInputs: TestModelOverrides[] = [], +): { + definition: ProjectDefinition; + resolvedSlots: { model: (string | number)[] }; +} { + const mainModel = createTestModel({ id: 'model:main', name: 'Main', - model: { - fields: model.fields ?? [], - relations: model.relations ?? [], - }, - authorizer: { roles: [] }, - }; + ...mainModelInput, + } as Partial); - const models = [ - mainModel, - ...(otherModels ?? []).map((m) => ({ - id: m.id, - name: m.name, - model: { fields: m.fields ?? [], relations: [] }, - authorizer: m.authorizer ?? { roles: [] }, - })), - ]; + const otherModels = otherModelInputs.map((input) => + createTestModel(input as Partial), + ); + + const definition = createTestProjectDefinition({ + models: [mainModel, ...otherModels], + }); return { - definition: { models }, + definition, resolvedSlots: { model: ['models', 0] }, }; } function getReferencedEntities( expression: string, - model: Parameters[0], - otherModels?: Parameters[1], + mainModelInput: TestModelOverrides, + otherModelInputs?: TestModelOverrides[], ): RefExpressionDependency[] { const parser = new AuthorizerExpressionParser(); const info = parseAuthorizerExpression(expression); - const { definition, resolvedSlots } = buildDefinition(model, otherModels); + const { definition, resolvedSlots } = buildDefinition( + mainModelInput, + otherModelInputs, + ); return parser.getReferencedEntities( expression, { success: true, value: info }, @@ -69,6 +65,49 @@ function getReferencedEntities( ); } +/** + * Test-only model input that allows partial model sub-fields. + * createTestModel fills in all required defaults at runtime. + */ +// oxlint-disable-next-line typescript/no-explicit-any +type TestModelInput = Record; + +/** Loose override type for test model creation — cast to ModelConfigInput at call site. */ +interface TestModelOverrides { + id?: string; + name?: string; + model?: TestModelInput; + authorizer?: TestModelInput; + [key: string]: unknown; +} + +/** Helper: create model input with extra fields (default id field is kept by createTestModel). */ +function modelWithFields( + extraFields: { id: string; name: string }[], +): TestModelInput { + return { + fields: extraFields.map((f) => ({ + ...f, + type: 'string' as const, + isOptional: false, + options: { default: '' }, + })), + }; +} + +/** Helper: create model input with relations. */ +function modelWithRelations( + relations: { id: string; name: string; modelRef: string }[], +): TestModelInput { + return { + relations: relations.map((r) => ({ + ...r, + foreignRelationName: 'backRef', + references: [], + })), + }; +} + /** * Helper to apply renames using getReferencedEntities output. * Mimics the generic orchestrator logic. @@ -98,12 +137,14 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { describe('model field references', () => { it('should resolve model.field to field entity ID', () => { const deps = getReferencedEntities('model.title === userId', { - fields: [{ id: 'field:title', name: 'title' }], + model: modelWithFields([ + { id: 'model-scalar-field:title', name: 'title' }, + ]), }); expect(deps).toEqual([ { entityType: modelScalarFieldEntityType, - entityId: 'field:title', + entityId: 'model-scalar-field:title', start: 6, end: 11, }, @@ -112,20 +153,18 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { it('should resolve fields on both sides of a comparison', () => { const deps = getReferencedEntities('model.authorId === model.creatorId', { - fields: [ - { id: 'field:author', name: 'authorId' }, - { id: 'field:creator', name: 'creatorId' }, - ], + model: modelWithFields([ + { id: 'model-scalar-field:author', name: 'authorId' }, + { id: 'model-scalar-field:creator', name: 'creatorId' }, + ]), }); expect(deps).toHaveLength(2); - expect(deps[0].entityId).toBe('field:author'); - expect(deps[1].entityId).toBe('field:creator'); + expect(deps[0].entityId).toBe('model-scalar-field:author'); + expect(deps[1].entityId).toBe('model-scalar-field:creator'); }); it('should skip unknown fields', () => { - const deps = getReferencedEntities('model.unknown === userId', { - fields: [{ id: 'field:title', name: 'title' }], - }); + const deps = getReferencedEntities('model.unknown === userId', {}); expect(deps).toEqual([]); }); }); @@ -135,28 +174,34 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { const deps = getReferencedEntities( "hasRole(model.todoList, 'owner')", { - relations: [ + model: modelWithRelations([ { - id: 'rel:todoList', + id: 'model-local-relation:todoList', name: 'todoList', modelRef: 'model:todo', }, - ], + ]), }, [ { id: 'model:todo', name: 'Todo', authorizer: { - roles: [{ id: 'auth-role:owner', name: 'owner' }], + roles: [ + { + id: 'model-authorizer-role:owner', + name: 'owner', + expression: 'model.id === userId', + }, + ], }, }, ], ); - // Should have both the relation ref and the foreign role ref expect(deps).toHaveLength(2); - expect(deps[0].entityId).toBe('rel:todoList'); - expect(deps[1].entityId).toBe('auth-role:owner'); + expect(deps[0].entityId).toBe('model-local-relation:todoList'); + expect(deps[0].entityType).toBe(modelLocalRelationEntityType); + expect(deps[1].entityId).toBe('model-authorizer-role:owner'); expect(deps[1].entityType).toBe(modelAuthorizerRoleEntityType); }); @@ -164,26 +209,27 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { const deps = getReferencedEntities( 'exists(model.members, { memberId: userId })', { - relations: [ + model: modelWithRelations([ { - id: 'rel:members', + id: 'model-local-relation:members', name: 'members', modelRef: 'model:member', }, - ], + ]), }, [ { id: 'model:member', name: 'Member', - fields: [{ id: 'field:memberId', name: 'memberId' }], + model: modelWithFields([ + { id: 'model-scalar-field:memberId', name: 'memberId' }, + ]), }, ], ); - // Relation + foreign field in condition expect(deps).toHaveLength(2); - expect(deps[0].entityId).toBe('rel:members'); - expect(deps[1].entityId).toBe('field:memberId'); + expect(deps[0].entityId).toBe('model-local-relation:members'); + expect(deps[1].entityId).toBe('model-scalar-field:memberId'); }); }); @@ -191,12 +237,14 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { it('should rename a model field', () => { const expression = 'model.title === userId'; const deps = getReferencedEntities(expression, { - fields: [{ id: 'field:title', name: 'title' }], + model: modelWithFields([ + { id: 'model-scalar-field:title', name: 'title' }, + ]), }); const result = applyRenames( expression, deps, - new Map([['field:title', 'heading']]), + new Map([['model-scalar-field:title', 'heading']]), ); expect(result).toBe('model.heading === userId'); }); @@ -206,20 +254,26 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { const deps = getReferencedEntities( expression, { - relations: [ + model: modelWithRelations([ { - id: 'rel:todoList', + id: 'model-local-relation:todoList', name: 'todoList', modelRef: 'model:todo', }, - ], + ]), }, [ { id: 'model:todo', name: 'Todo', authorizer: { - roles: [{ id: 'auth-role:owner', name: 'owner' }], + roles: [ + { + id: 'model-authorizer-role:owner', + name: 'owner', + expression: 'model.id === userId', + }, + ], }, }, ], @@ -227,7 +281,7 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { const result = applyRenames( expression, deps, - new Map([['rel:todoList', 'list']]), + new Map([['model-local-relation:todoList', 'list']]), ); expect(result).toBe("hasRole(model.list, 'owner')"); }); @@ -237,20 +291,26 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { const deps = getReferencedEntities( expression, { - relations: [ + model: modelWithRelations([ { - id: 'rel:todoList', + id: 'model-local-relation:todoList', name: 'todoList', modelRef: 'model:todo', }, - ], + ]), }, [ { id: 'model:todo', name: 'Todo', authorizer: { - roles: [{ id: 'auth-role:owner', name: 'owner' }], + roles: [ + { + id: 'model-authorizer-role:owner', + name: 'owner', + expression: 'model.id === userId', + }, + ], }, }, ], @@ -258,7 +318,7 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { const result = applyRenames( expression, deps, - new Map([['auth-role:owner', 'admin']]), + new Map([['model-authorizer-role:owner', 'admin']]), ); expect(result).toBe("hasRole(model.todoList, 'admin')"); }); @@ -268,20 +328,26 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { const deps = getReferencedEntities( expression, { - relations: [ + model: modelWithRelations([ { - id: 'rel:todoList', + id: 'model-local-relation:todoList', name: 'todoList', modelRef: 'model:todo', }, - ], + ]), }, [ { id: 'model:todo', name: 'Todo', authorizer: { - roles: [{ id: 'auth-role:owner', name: 'owner' }], + roles: [ + { + id: 'model-authorizer-role:owner', + name: 'owner', + expression: 'model.id === userId', + }, + ], }, }, ], @@ -290,8 +356,8 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { expression, deps, new Map([ - ['rel:todoList', 'list'], - ['auth-role:owner', 'admin'], + ['model-local-relation:todoList', 'list'], + ['model-authorizer-role:owner', 'admin'], ]), ); expect(result).toBe("hasRole(model.list, 'admin')"); @@ -302,78 +368,42 @@ describe('AuthorizerExpressionParser.getReferencedEntities', () => { const deps = getReferencedEntities( expression, { - relations: [ + model: modelWithRelations([ { - id: 'rel:members', + id: 'model-local-relation:members', name: 'members', modelRef: 'model:member', }, - ], + ]), }, [ { id: 'model:member', name: 'Member', - fields: [{ id: 'field:userName', name: 'userName' }], + model: modelWithFields([ + { id: 'model-scalar-field:userName', name: 'userName' }, + ]), }, ], ); const result = applyRenames( expression, deps, - new Map([['field:userName', 'memberName']]), + new Map([['model-scalar-field:userName', 'memberName']]), ); expect(result).toBe('exists(model.members, { memberName: userId })'); }); - it('should handle complex expression with multiple renames', () => { - const expression = - "model.authorId === userId && hasRole(model.todoList, 'owner')"; - const deps = getReferencedEntities( - expression, - { - fields: [{ id: 'field:authorId', name: 'authorId' }], - relations: [ - { - id: 'rel:todoList', - name: 'todoList', - modelRef: 'model:todo', - }, - ], - }, - [ - { - id: 'model:todo', - name: 'Todo', - authorizer: { - roles: [{ id: 'auth-role:owner', name: 'owner' }], - }, - }, - ], - ); - const result = applyRenames( - expression, - deps, - new Map([ - ['field:authorId', 'ownerId'], - ['rel:todoList', 'list'], - ['auth-role:owner', 'admin'], - ]), - ); - expect(result).toBe( - "model.ownerId === userId && hasRole(model.list, 'admin')", - ); - }); - it('should not rename when no entities match', () => { const expression = 'model.id === userId'; - const deps = getReferencedEntities(expression, { - fields: [{ id: 'field:id', name: 'id' }], - }); + const deps = getReferencedEntities(expression, {}); + // The default id field from createTestModel should be resolved + expect(deps).toHaveLength(1); + const result = applyRenames( expression, deps, - new Map([['field:other', 'something']]), + new Map([['model-scalar-field:other', 'something']]), ); expect(result).toBe(expression); }); diff --git a/packages/project-builder-lib/src/references/expression-types.ts b/packages/project-builder-lib/src/references/expression-types.ts index a02ca93e6..04cc7cdc0 100644 --- a/packages/project-builder-lib/src/references/expression-types.ts +++ b/packages/project-builder-lib/src/references/expression-types.ts @@ -1,6 +1,7 @@ import type { z } from 'zod'; import type { PluginSpecStore } from '#src/plugins/index.js'; +import type { ProjectDefinition } from '#src/schema/project-definition.js'; import type { RefContextSlot } from './ref-context-slot.js'; import type { DefinitionEntityType, ReferencePath } from './types.js'; @@ -11,8 +12,8 @@ import type { DefinitionEntityType, ReferencePath } from './types.js'; * validating expressions against model fields, roles, etc. */ export interface ExpressionValidationContext { - /** The raw project definition data */ - readonly definition: unknown; + /** The project definition data */ + readonly definition: ProjectDefinition; /** The plugin spec store for accessing plugin-registered configuration */ readonly pluginStore: PluginSpecStore; } @@ -135,12 +136,12 @@ export abstract class RefExpressionParser< * The result is cached on the marker for subsequent operations. * * @param value - The raw expression value - * @param projectDef - The project definition for context (typed as unknown to avoid circular reference) + * @param projectDef - The project definition for context * @returns Success with parsed value, or failure with error message */ abstract parse( value: TValue, - projectDef: unknown, + projectDef: ProjectDefinition, ): RefExpressionParseResult; /** @@ -168,14 +169,14 @@ export abstract class RefExpressionParser< * * @param value - The raw expression value * @param parseResult - The cached parse result - * @param definition - The project definition (typed as unknown to avoid circular reference) + * @param definition - The project definition * @param resolvedSlots - The resolved slot paths for this expression * @returns Array of entity references with positions for rename */ abstract getReferencedEntities( value: TValue, parseResult: RefExpressionParseResult, - definition: unknown, + definition: ProjectDefinition, resolvedSlots: ResolvedExpressionSlots, ): RefExpressionDependency[]; @@ -191,7 +192,7 @@ export abstract class RefExpressionParser< */ validate( value: TValue, - projectDef: unknown, + projectDef: ProjectDefinition, context: ExpressionValidationContext, resolvedSlots: ResolvedExpressionSlots, ): RefExpressionWarning[] { diff --git a/packages/project-builder-lib/src/references/fix-expression-renames.ts b/packages/project-builder-lib/src/references/fix-expression-renames.ts index 11baddbc6..93adb664e 100644 --- a/packages/project-builder-lib/src/references/fix-expression-renames.ts +++ b/packages/project-builder-lib/src/references/fix-expression-renames.ts @@ -1,5 +1,7 @@ import { set } from 'es-toolkit/compat'; +import type { ProjectDefinition } from '#src/schema/project-definition.js'; + import type { DefinitionEntity, ReferencePath, @@ -55,11 +57,13 @@ export function applyExpressionRenames( let modified = false; const updates: { path: ReferencePath; value: string }[] = []; + const oldDefinition = oldRefPayload.data as ProjectDefinition; + for (const expression of oldExpressions) { // Parse and resolve entities against the OLD definition where old names still exist const parseResult = expression.parser.parse( expression.value, - oldRefPayload.data, + oldDefinition, ); if (!parseResult.success) { // Don't touch broken expressions @@ -69,7 +73,7 @@ export function applyExpressionRenames( const refs = expression.parser.getReferencedEntities( expression.value, parseResult, - oldRefPayload.data, + oldDefinition, expression.resolvedSlots, ); From 120b4517479ed9c8a08f8ff8600a627b3349d947 Mon Sep 17 00:00:00 2001 From: Kingston Date: Fri, 20 Mar 2026 13:17:56 -0700 Subject: [PATCH 6/8] Add support for foreign relations in models --- .../authorizer-expression-parser.ts | 95 +++++++++++-------- 1 file changed, 55 insertions(+), 40 deletions(-) diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts index ca068c8ac..cc7a56266 100644 --- a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts +++ b/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts @@ -7,6 +7,7 @@ import type { RefExpressionWarning, ResolvedExpressionSlots, } from '#src/references/expression-types.js'; +import type { DefinitionEntityType } from '#src/references/types.js'; import type { ModelConfig } from '#src/schema/models/models.js'; import type { modelEntityType } from '#src/schema/models/types.js'; import type { ProjectDefinition } from '#src/schema/project-definition.js'; @@ -14,6 +15,7 @@ import type { ProjectDefinition } from '#src/schema/project-definition.js'; import { RefExpressionParser } from '#src/references/expression-types.js'; import { modelAuthorizerRoleEntityType } from '#src/schema/models/authorizer/types.js'; import { + modelForeignRelationEntityType, modelLocalRelationEntityType, modelScalarFieldEntityType, } from '#src/schema/models/types.js'; @@ -128,32 +130,45 @@ export class AuthorizerExpressionParser extends RefExpressionParser< const model = this.getRawModel(definition, resolvedSlots); - const allModels = (definition.models ?? []).filter( + const allModels = definition.models.filter( (m): m is ModelConfig => typeof m.name === 'string', ); // Build lookup maps const fieldByName = new Map(); - for (const field of model.model?.fields ?? []) { - if (field.id) { - fieldByName.set(field.name, { id: field.id }); - } + for (const field of model.model.fields) { + fieldByName.set(field.name, { id: field.id }); } - const relationByName = new Map(); - for (const relation of model.model?.relations ?? []) { - if (relation.id) { - relationByName.set(relation.name, { - id: relation.id, - modelRef: relation.modelRef, - }); - } + const relationByName = new Map< + string, + { id: string; modelRef: string; entityType: DefinitionEntityType } + >(); + // Local relations (defined on this model) + for (const relation of model.model.relations) { + relationByName.set(relation.name, { + id: relation.id, + modelRef: relation.modelRef, + entityType: modelLocalRelationEntityType, + }); } const modelById = new Map(); for (const m of allModels) { - if (m.id) { - modelById.set(m.id, m); + modelById.set(m.id, m); + } + + // Foreign relations (defined on other models pointing to this model via foreignRelationName) + for (const m of allModels) { + for (const relation of m.model.relations) { + if (relation.foreignRelationName && relation.modelRef === model.id) { + relationByName.set(relation.foreignRelationName, { + id: relation.foreignId, + // Foreign relation points back to the model that defines it + modelRef: m.id, + entityType: modelForeignRelationEntityType, + }); + } } } @@ -187,23 +202,25 @@ export class AuthorizerExpressionParser extends RefExpressionParser< const relation = relationByName.get(node.relationName); if (relation) { deps.push({ - entityType: modelLocalRelationEntityType, + entityType: relation.entityType, entityId: relation.id, start: node.relationEnd - node.relationName.length, end: node.relationEnd, }); // Foreign authorizer role const foreignModel = modelById.get(relation.modelRef); - const foreignRole = foreignModel?.authorizer?.roles?.find( - (r) => r.name === node.role, - ); - if (foreignRole?.id) { - deps.push({ - entityType: modelAuthorizerRoleEntityType, - entityId: foreignRole.id, - start: node.roleStart + 1, - end: node.roleEnd - 1, - }); + if (foreignModel) { + const foreignRole = foreignModel.authorizer.roles.find( + (r) => r.name === node.role, + ); + if (foreignRole) { + deps.push({ + entityType: modelAuthorizerRoleEntityType, + entityId: foreignRole.id, + start: node.roleStart + 1, + end: node.roleEnd - 1, + }); + } } } }, @@ -211,21 +228,19 @@ export class AuthorizerExpressionParser extends RefExpressionParser< const relation = relationByName.get(node.relationName); if (relation) { deps.push({ - entityType: modelLocalRelationEntityType, + entityType: relation.entityType, entityId: relation.id, start: node.relationEnd - node.relationName.length, end: node.relationEnd, }); const foreignModel = modelById.get(relation.modelRef); - if (foreignModel?.authorizer?.roles) { + if (foreignModel) { const foreignRoleByName = new Map( - foreignModel.authorizer.roles - .filter((r) => r.id) - .map((r) => [r.name, r]), + foreignModel.authorizer.roles.map((r) => [r.name, r]), ); for (let i = 0; i < node.roles.length; i++) { const foreignRole = foreignRoleByName.get(node.roles[i]); - if (foreignRole?.id) { + if (foreignRole) { deps.push({ entityType: modelAuthorizerRoleEntityType, entityId: foreignRole.id, @@ -241,7 +256,7 @@ export class AuthorizerExpressionParser extends RefExpressionParser< const relation = relationByName.get(node.relationName); if (relation) { deps.push({ - entityType: modelLocalRelationEntityType, + entityType: relation.entityType, entityId: relation.id, start: node.relationEnd - node.relationName.length, end: node.relationEnd, @@ -249,8 +264,8 @@ export class AuthorizerExpressionParser extends RefExpressionParser< // Foreign model fields referenced in conditions const foreignModel = modelById.get(relation.modelRef); const foreignFieldByName = new Map(); - for (const f of foreignModel?.model?.fields ?? []) { - if (f.id) { + if (foreignModel) { + for (const f of foreignModel.model.fields) { foreignFieldByName.set(f.name, { id: f.id }); } } @@ -344,7 +359,7 @@ export class AuthorizerExpressionParser extends RefExpressionParser< ): ModelValidationContext { const model = this.getRawModel(definition, resolvedSlots); - const allModels = (definition.models ?? []).filter( + const allModels = definition.models.filter( (m): m is ModelConfig => typeof m.name === 'string', ); @@ -352,15 +367,15 @@ export class AuthorizerExpressionParser extends RefExpressionParser< { id: model.id, name: model.name, - fields: model.model?.fields, - model: { relations: model.model?.relations }, + fields: model.model.fields, + model: { relations: model.model.relations }, }, allModels.map((m) => ({ id: m.id, name: m.name, authorizer: m.authorizer, - fields: m.model?.fields, - model: { relations: m.model?.relations }, + fields: m.model.fields, + model: { relations: m.model.relations }, })), ); } From d37f5ca92b0bf148a6bc8c9d62cacdbe30854475 Mon Sep 17 00:00:00 2001 From: Kingston Date: Sat, 21 Mar 2026 15:36:08 -0700 Subject: [PATCH 7/8] PR feedback --- .../authorizer/authorizer-expression-acorn-parser.ts | 0 .../authorizer-expression-acorn-parser.unit.test.ts | 0 .../authorizer/authorizer-expression-ast.ts | 0 .../authorizer/authorizer-expression-parser.ts | 5 ----- .../authorizer-expression-rename.unit.test.ts | 0 .../authorizer/authorizer-expression-validator.ts | 0 .../authorizer-expression-validator.unit.test.ts | 0 .../authorizer/authorizer-expression-visitor.ts | 0 .../expression-parsers/authorizer/index.ts | 0 .../expression-parsers/register-core-module.ts | 10 ++++------ packages/project-builder-lib/src/index.ts | 2 +- .../src/references/expression-parser-spec.ts | 2 +- .../src/references/extend-parser-context-with-refs.ts | 6 +++--- .../src/schema/models/authorizer/index.ts | 2 +- .../src/testing/parser-context.test-helper.ts | 2 +- .../project-definition-container.test-helper.ts | 2 +- .../src/stripe/core/schema/schema-issue-checker.ts | 7 +++---- 17 files changed, 15 insertions(+), 23 deletions(-) rename packages/project-builder-lib/src/{references => }/expression-parsers/authorizer/authorizer-expression-acorn-parser.ts (100%) rename packages/project-builder-lib/src/{references => }/expression-parsers/authorizer/authorizer-expression-acorn-parser.unit.test.ts (100%) rename packages/project-builder-lib/src/{references => }/expression-parsers/authorizer/authorizer-expression-ast.ts (100%) rename packages/project-builder-lib/src/{references => }/expression-parsers/authorizer/authorizer-expression-parser.ts (98%) rename packages/project-builder-lib/src/{references => }/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts (100%) rename packages/project-builder-lib/src/{references => }/expression-parsers/authorizer/authorizer-expression-validator.ts (100%) rename packages/project-builder-lib/src/{references => }/expression-parsers/authorizer/authorizer-expression-validator.unit.test.ts (100%) rename packages/project-builder-lib/src/{references => }/expression-parsers/authorizer/authorizer-expression-visitor.ts (100%) rename packages/project-builder-lib/src/{references => }/expression-parsers/authorizer/index.ts (100%) rename packages/project-builder-lib/src/{references => }/expression-parsers/register-core-module.ts (78%) diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-acorn-parser.ts b/packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-acorn-parser.ts similarity index 100% rename from packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-acorn-parser.ts rename to packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-acorn-parser.ts diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-acorn-parser.unit.test.ts b/packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-acorn-parser.unit.test.ts similarity index 100% rename from packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-acorn-parser.unit.test.ts rename to packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-acorn-parser.unit.test.ts diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-ast.ts b/packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-ast.ts similarity index 100% rename from packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-ast.ts rename to packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-ast.ts diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts b/packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-parser.ts similarity index 98% rename from packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts rename to packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-parser.ts index cc7a56266..f6c576984 100644 --- a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-parser.ts +++ b/packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-parser.ts @@ -380,8 +380,3 @@ export class AuthorizerExpressionParser extends RefExpressionParser< ); } } - -/** - * Singleton instance of AuthorizerExpressionParser. - */ -export const authorizerExpressionParser = new AuthorizerExpressionParser(); diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts b/packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts similarity index 100% rename from packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts rename to packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-rename.unit.test.ts diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-validator.ts b/packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-validator.ts similarity index 100% rename from packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-validator.ts rename to packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-validator.ts diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-validator.unit.test.ts b/packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-validator.unit.test.ts similarity index 100% rename from packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-validator.unit.test.ts rename to packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-validator.unit.test.ts diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-visitor.ts b/packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-visitor.ts similarity index 100% rename from packages/project-builder-lib/src/references/expression-parsers/authorizer/authorizer-expression-visitor.ts rename to packages/project-builder-lib/src/expression-parsers/authorizer/authorizer-expression-visitor.ts diff --git a/packages/project-builder-lib/src/references/expression-parsers/authorizer/index.ts b/packages/project-builder-lib/src/expression-parsers/authorizer/index.ts similarity index 100% rename from packages/project-builder-lib/src/references/expression-parsers/authorizer/index.ts rename to packages/project-builder-lib/src/expression-parsers/authorizer/index.ts diff --git a/packages/project-builder-lib/src/references/expression-parsers/register-core-module.ts b/packages/project-builder-lib/src/expression-parsers/register-core-module.ts similarity index 78% rename from packages/project-builder-lib/src/references/expression-parsers/register-core-module.ts rename to packages/project-builder-lib/src/expression-parsers/register-core-module.ts index 018254ab3..0a18604bc 100644 --- a/packages/project-builder-lib/src/references/expression-parsers/register-core-module.ts +++ b/packages/project-builder-lib/src/expression-parsers/register-core-module.ts @@ -2,8 +2,8 @@ import type { PluginModuleWithKey } from '#src/plugins/imports/types.js'; import { createPluginModule } from '#src/plugins/imports/types.js'; -import { expressionParserSpec } from '../expression-parser-spec.js'; -import { authorizerExpressionParser } from './authorizer/authorizer-expression-parser.js'; +import { expressionParserSpec } from '../references/expression-parser-spec.js'; +import { AuthorizerExpressionParser } from './authorizer/authorizer-expression-parser.js'; /** * Core module that registers built-in expression parsers. @@ -18,10 +18,8 @@ const registerExpressionParsersModule = createPluginModule({ expressionParsers: expressionParserSpec, }, initialize: ({ expressionParsers }) => { - expressionParsers.parsers.set( - authorizerExpressionParser.name, - authorizerExpressionParser, - ); + const parser = new AuthorizerExpressionParser(); + expressionParsers.parsers.set(parser.name, parser); }, }); diff --git a/packages/project-builder-lib/src/index.ts b/packages/project-builder-lib/src/index.ts index 357a50eff..cf5752a68 100644 --- a/packages/project-builder-lib/src/index.ts +++ b/packages/project-builder-lib/src/index.ts @@ -1,12 +1,12 @@ export * from './compiler/index.js'; export * from './constants/index.js'; export * from './definition/index.js'; +export { expressionParserCoreModule } from './expression-parsers/register-core-module.js'; export * from './feature-flags/index.js'; export * from './migrations/index.js'; export * from './parser/index.js'; export * from './plugins/index.js'; export * from './references/index.js'; -export { expressionParserCoreModule } from './references/expression-parsers/register-core-module.js'; export * from './schema/index.js'; export * from './specs/index.js'; export * from './tools/index.js'; diff --git a/packages/project-builder-lib/src/references/expression-parser-spec.ts b/packages/project-builder-lib/src/references/expression-parser-spec.ts index a0f33ca12..0916f76fe 100644 --- a/packages/project-builder-lib/src/references/expression-parser-spec.ts +++ b/packages/project-builder-lib/src/references/expression-parser-spec.ts @@ -2,7 +2,7 @@ import { createFieldMapSpec } from '#src/plugins/utils/create-field-map-spec.js' import type { RefExpressionParser } from './expression-types.js'; -// eslint-disable-next-line @typescript-eslint/no-explicit-any +// oxlint-disable-next-line typescript/no-explicit-any type AnyExpressionParser = RefExpressionParser; /** diff --git a/packages/project-builder-lib/src/references/extend-parser-context-with-refs.ts b/packages/project-builder-lib/src/references/extend-parser-context-with-refs.ts index 417585408..bda3d6fed 100644 --- a/packages/project-builder-lib/src/references/extend-parser-context-with-refs.ts +++ b/packages/project-builder-lib/src/references/extend-parser-context-with-refs.ts @@ -218,11 +218,11 @@ function refContext< function createWithExpression(plugins?: PluginSpecStore): WithExpressionType { // Implementation function withExpression( - parserOrRef: // eslint-disable-next-line @typescript-eslint/no-explicit-any + parserOrRef: // oxlint-disable-next-line typescript/no-explicit-any | RefExpressionParser - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // oxlint-disable-next-line typescript/no-explicit-any | ExpressionParserRef, - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // oxlint-disable-next-line typescript/no-explicit-any slots?: ExpressionSlotMap, ): z.ZodType { // oxlint-disable-next-line typescript/no-explicit-any diff --git a/packages/project-builder-lib/src/schema/models/authorizer/index.ts b/packages/project-builder-lib/src/schema/models/authorizer/index.ts index 758e1ebcd..a4fc3929b 100644 --- a/packages/project-builder-lib/src/schema/models/authorizer/index.ts +++ b/packages/project-builder-lib/src/schema/models/authorizer/index.ts @@ -1,3 +1,3 @@ -export * from '#src/references/expression-parsers/authorizer/index.js'; export * from './authorizer.js'; export * from './types.js'; +export * from '#src/expression-parsers/authorizer/index.js'; diff --git a/packages/project-builder-lib/src/testing/parser-context.test-helper.ts b/packages/project-builder-lib/src/testing/parser-context.test-helper.ts index 19848961e..3c2e778ca 100644 --- a/packages/project-builder-lib/src/testing/parser-context.test-helper.ts +++ b/packages/project-builder-lib/src/testing/parser-context.test-helper.ts @@ -1,7 +1,7 @@ import type { DefinitionSchemaParserContext } from '../schema/creator/types.js'; +import { expressionParserCoreModule } from '../expression-parsers/register-core-module.js'; import { createPluginSpecStore } from '../parser/parser.js'; -import { expressionParserCoreModule } from '../references/expression-parsers/register-core-module.js'; import { createDefinitionSchemaParserContext } from '../schema/creator/schema-creator.js'; const emptyPluginStore = { diff --git a/packages/project-builder-lib/src/testing/project-definition-container.test-helper.ts b/packages/project-builder-lib/src/testing/project-definition-container.test-helper.ts index 3ea12b5fb..4400ca4f4 100644 --- a/packages/project-builder-lib/src/testing/project-definition-container.test-helper.ts +++ b/packages/project-builder-lib/src/testing/project-definition-container.test-helper.ts @@ -6,10 +6,10 @@ import type { import type { EntityServiceContext } from '#src/tools/entity-service/types.js'; import { ProjectDefinitionContainer } from '#src/definition/project-definition-container.js'; +import { expressionParserCoreModule } from '#src/expression-parsers/register-core-module.js'; import { getLatestMigrationVersion } from '#src/migrations/index.js'; import { createPluginSpecStore } from '#src/parser/parser.js'; import { deserializeSchemaWithTransformedReferences } from '#src/references/deserialize-schema.js'; -import { expressionParserCoreModule } from '#src/references/expression-parsers/register-core-module.js'; import { createDefinitionSchemaParserContext } from '#src/schema/index.js'; import { createProjectDefinitionSchema } from '#src/schema/project-definition.js'; diff --git a/plugins/plugin-payments/src/stripe/core/schema/schema-issue-checker.ts b/plugins/plugin-payments/src/stripe/core/schema/schema-issue-checker.ts index a0d4092bd..780abb6e2 100644 --- a/plugins/plugin-payments/src/stripe/core/schema/schema-issue-checker.ts +++ b/plugins/plugin-payments/src/stripe/core/schema/schema-issue-checker.ts @@ -18,10 +18,9 @@ export function createStripeSchemaChecker( pluginKey, pluginLabel: 'Stripe', buildPartialDef: (container) => { - const config = PluginUtils.configByKey( - container.definition, - pluginKey, - ) as StripePluginDefinition | undefined; + const config = PluginUtils.configByKey(container.definition, pluginKey) as + | StripePluginDefinition + | undefined; if (!config?.billing.enabled || !config.billing.featureRef) { return undefined; From a656ca80a6ed8fbd2faed5d2b700b62f63754fe9 Mon Sep 17 00:00:00 2001 From: Kingston Date: Sun, 22 Mar 2026 00:43:02 -0700 Subject: [PATCH 8/8] PR feedback --- .claude/settings.json | 2 +- .../src/parser/collect-expression-issues.ts | 24 ++++++++++++------- .../src/references/expression-parser-ref.ts | 5 +++- .../src/references/expression-types.ts | 4 ++-- .../src/references/fix-expression-renames.ts | 10 +++++++- 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index c45443b2b..fd826690f 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -58,7 +58,7 @@ "Bash(git checkout -b *)", "Bash(echo $?)", "Bash(jq *)", - "Bash(git mv:*)", + "Bash(git mv *)", "WebFetch(domain:ui.shadcn.com)" ], "deny": ["Edit(**/baseplate/generated/**)"] diff --git a/packages/project-builder-lib/src/parser/collect-expression-issues.ts b/packages/project-builder-lib/src/parser/collect-expression-issues.ts index e2ba84ad4..749dd78cf 100644 --- a/packages/project-builder-lib/src/parser/collect-expression-issues.ts +++ b/packages/project-builder-lib/src/parser/collect-expression-issues.ts @@ -39,16 +39,24 @@ export function collectExpressionIssues( const issues: DefinitionIssue[] = []; for (const expression of expressions) { - const warnings = expression.parser.validate( - expression.value, - definition, - context, - expression.resolvedSlots, - ); + try { + const warnings = expression.parser.validate( + expression.value, + definition, + context, + expression.resolvedSlots, + ); - for (const warning of warnings) { + for (const warning of warnings) { + issues.push({ + message: warning.message, + path: expression.path, + severity: 'warning', + }); + } + } catch (error) { issues.push({ - message: warning.message, + message: `Expression parser "${expression.parser.name}" threw an error: ${error instanceof Error ? error.message : String(error)}`, path: expression.path, severity: 'warning', }); diff --git a/packages/project-builder-lib/src/references/expression-parser-ref.ts b/packages/project-builder-lib/src/references/expression-parser-ref.ts index 4a0c19156..930229b0d 100644 --- a/packages/project-builder-lib/src/references/expression-parser-ref.ts +++ b/packages/project-builder-lib/src/references/expression-parser-ref.ts @@ -21,7 +21,10 @@ import type { DefinitionEntityType } from './types.js'; * const authorizerExpressionRef = createExpressionParserRef< * string, * { model: typeof modelEntityType } - * >('authorizer-expression', z.string().min(1, 'Expression is required')); + * >( + * 'authorizer-expression', + * () => z.string().min(1, 'Expression is required'), + * ); * ``` */ export interface ExpressionParserRef< diff --git a/packages/project-builder-lib/src/references/expression-types.ts b/packages/project-builder-lib/src/references/expression-types.ts index 04cc7cdc0..7f301cbbc 100644 --- a/packages/project-builder-lib/src/references/expression-types.ts +++ b/packages/project-builder-lib/src/references/expression-types.ts @@ -51,9 +51,9 @@ export interface RefExpressionDependency { entityType: DefinitionEntityType; /** The ID of the entity being referenced */ entityId: string; - /** Start position in the expression text to replace on rename */ + /** Start position (inclusive) in the expression text to replace on rename */ start: number; - /** End position in the expression text to replace on rename */ + /** End position (exclusive) in the expression text to replace on rename */ end: number; } diff --git a/packages/project-builder-lib/src/references/fix-expression-renames.ts b/packages/project-builder-lib/src/references/fix-expression-renames.ts index 93adb664e..93bbe2e61 100644 --- a/packages/project-builder-lib/src/references/fix-expression-renames.ts +++ b/packages/project-builder-lib/src/references/fix-expression-renames.ts @@ -1,4 +1,4 @@ -import { set } from 'es-toolkit/compat'; +import { get, set } from 'es-toolkit/compat'; import type { ProjectDefinition } from '#src/schema/project-definition.js'; @@ -60,6 +60,14 @@ export function applyExpressionRenames( const oldDefinition = oldRefPayload.data as ProjectDefinition; for (const expression of oldExpressions) { + // Verify the expression still exists at the same path in the new definition. + // If the expression was removed or its shape changed, skip it to avoid + // resurrecting deleted nodes via set(). + const currentValue: unknown = get(newDefinition as object, expression.path); + if (typeof currentValue !== 'string') { + continue; + } + // Parse and resolve entities against the OLD definition where old names still exist const parseResult = expression.parser.parse( expression.value,