refactor(migrate-ts): post-review simplifications in referential-actions

- Extract isRequired, findField, readIdentityFields to referential-actions.ts
  and remove character-for-character duplicates from expected-schema.ts
- Collapse makeDoc in referential-actions.test.ts to delegate to the
  existing weekDoc factory (same document structure, one fewer function)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: dmealing

server/typescript/packages/migrate-ts/src/expected-schema.ts

@@ -1,13 +1,9 @@
-import type { MetaData, MetaObject, MetaReferenceIdentity, MetaRoot } from "@metaobjectsdev/metadata";
+import type { MetaData, MetaObject, MetaRoot } from "@metaobjectsdev/metadata";
 import {
   TYPE_OBJECT,
-  TYPE_VALIDATOR,
   MetaSource,
-  VALIDATOR_SUBTYPE_REQUIRED,
-  IDENTITY_ATTR_FIELDS,
   IDENTITY_ATTR_GENERATION,
   IDENTITY_ATTR_UNIQUE,
-  FIELD_ATTR_REQUIRED,
   FIELD_ATTR_DEFAULT,
   FIELD_ATTR_UNIQUE,
   FIELD_SUBTYPE_STRING,
@@ -34,7 +30,13 @@ import type { SqlType } from "./sql-type.js";
 import type {
   SchemaSnapshot, TableDescriptor, ColumnDescriptor, IndexDescriptor, FkDescriptor,
 } from "./types.js";
-import { resolveReferentialActions, validateSetNullNullability } from "./referential-actions.js";
+import {
+  resolveReferentialActions,
+  validateSetNullNullability,
+  readIdentityFields,
+  findField,
+  isRequired,
+} from "./referential-actions.js";
 
 export interface BuildExpectedSchemaOptions {
   /**
@@ -297,19 +299,14 @@ function buildColumn(
   isPk: boolean,
   pkGeneration: string | undefined,
 ): ColumnDescriptor {
-  const requiredAttr = field.ownAttr(FIELD_ATTR_REQUIRED);
-  // BaseEntity-style metadata expresses required via a validator.required child;
-  // direct attr form is the alternative. Either signals NOT NULL.
-  const hasRequiredValidator = field.ownChildren().some(
-    (c) => c.type === TYPE_VALIDATOR && c.subType === VALIDATOR_SUBTYPE_REQUIRED,
-  );
-  const isRequired = requiredAttr === true || requiredAttr === "true" || hasRequiredValidator;
+  // Both the @required attr and the validator.required child signal NOT NULL.
+  const fieldIsRequired = isRequired(field);
   const defaultRaw = field.ownAttr(FIELD_ATTR_DEFAULT);
 
   const col: ColumnDescriptor = {
     name: resolveColumnName(field),
     sqlType: subtypeToSqlType(field.subType),
-    nullable: !isPk && !isRequired,
+    nullable: !isPk && !fieldIsRequired,
   };
 
   if (typeof defaultRaw === "string" && defaultRaw.length > 0) {
@@ -347,25 +344,6 @@ function subtypeToSqlType(subType: string): SqlType {
   }
 }
 
-function readIdentityFields(identity: MetaData): string[] {
-  const raw = identity.ownAttr(IDENTITY_ATTR_FIELDS);
-  if (Array.isArray(raw)) {
-    return raw.map(String).filter((s) => s.length > 0);
-  }
-  if (typeof raw === "string") {
-    // Fallback: comma-separated string form (defensive; canonical form is array)
-    return raw.split(",").map((s) => s.trim()).filter((s) => s.length > 0);
-  }
-  return [];
-}
-
-function findField(entity: MetaObject, name: string): MetaData | undefined {
-  for (const field of entity.fields()) {
-    if (field.name === name) return field;
-  }
-  return undefined;
-}
-
 function toSnake(s: string): string {
   return s
     .replace(/([A-Z]+)([A-Z][a-z])/g, "$1_$2")

server/typescript/packages/migrate-ts/src/referential-actions.ts

@@ -12,6 +12,33 @@ import {
 import type { FkAction } from "./types.js";
 import { SetNullNotNullableError } from "./errors.js";
 
+// ---------------------------------------------------------------------------
+// Shared field helpers — exported for use by expected-schema.ts
+// ---------------------------------------------------------------------------
+
+export function readIdentityFields(identity: MetaData): string[] {
+  const raw = identity.ownAttr(IDENTITY_ATTR_FIELDS);
+  if (Array.isArray(raw)) return raw.map(String).filter((s) => s.length > 0);
+  // Fallback: comma-separated string form (defensive; canonical form is array)
+  if (typeof raw === "string") return raw.split(",").map((s) => s.trim()).filter((s) => s.length > 0);
+  return [];
+}
+
+export function findField(entity: MetaObject, name: string): MetaData | undefined {
+  for (const field of entity.fields()) {
+    if (field.name === name) return field;
+  }
+  return undefined;
+}
+
+export function isRequired(field: MetaData): boolean {
+  const attr = field.ownAttr(FIELD_ATTR_REQUIRED);
+  if (attr === true || attr === "true") return true;
+  return field.ownChildren().some(
+    (c) => c.type === TYPE_VALIDATOR && c.subType === VALIDATOR_SUBTYPE_REQUIRED,
+  );
+}
+
 /**
  * Resolve the referential actions for a foreign key inferred from an
  * identity.reference, by correlating it with a sibling relationship on the
@@ -92,7 +119,7 @@ export function validateSetNullNullability(
 ): void {
   if (onDelete !== "set-null") return;
 
-  const fkFieldJsNames = readRefFields(ref);
+  const fkFieldJsNames = readIdentityFields(ref);
   const offending: string[] = [];
   for (const jsName of fkFieldJsNames) {
     const field = findField(entity, jsName);
@@ -105,25 +132,3 @@ export function validateSetNullNullability(
     throw new SetNullNotNullableError(entity.name, constraintName, offending);
   }
 }
-
-function readRefFields(ref: MetaReferenceIdentity): string[] {
-  const raw = ref.ownAttr(IDENTITY_ATTR_FIELDS);
-  if (Array.isArray(raw)) return raw.map(String).filter((s) => s.length > 0);
-  if (typeof raw === "string") return raw.split(",").map((s) => s.trim()).filter((s) => s.length > 0);
-  return [];
-}
-
-function findField(entity: MetaObject, name: string): MetaData | undefined {
-  for (const field of entity.fields()) {
-    if (field.name === name) return field;
-  }
-  return undefined;
-}
-
-function isRequired(field: MetaData): boolean {
-  const attr = field.ownAttr(FIELD_ATTR_REQUIRED);
-  if (attr === true || attr === "true") return true;
-  return field.ownChildren().some(
-    (c) => c.type === TYPE_VALIDATOR && c.subType === VALIDATOR_SUBTYPE_REQUIRED,
-  );
-}

server/typescript/packages/migrate-ts/test/unit/referential-actions.test.ts

@@ -91,31 +91,8 @@ describe("resolveReferentialActions", () => {
 
 const EMPTY_SCHEMA = { tables: [], views: [] };
 
-function makeDoc(rel: Record<string, unknown>) {
-  return {
-    "metadata.root": {
-      package: "acme",
-      children: [
-        { "object.entity": { name: "Program", children: [
-          { "field.long": { name: "id" } },
-          { "identity.primary": { "@fields": "id" } },
-        ] } },
-        { "object.entity": { name: "Week", children: [
-          { "field.long": { name: "id" } },
-          { "field.long": { name: "programId" } },
-          rel,
-          { "identity.reference": { name: "ref_program", "@fields": ["programId"], "@references": "Program" } },
-          { "identity.primary": { "@fields": "id" } },
-        ] } },
-      ],
-    },
-  };
-}
-
 async function buildSnapshotForRel(rel: Record<string, unknown>) {
-  const { root, errors } = await new MetaDataLoader().load([
-    new InMemorySource(JSON.stringify(makeDoc(rel))),
-  ]);
+  const { root, errors } = await loadDoc(weekDoc(rel));
   expect(errors).toHaveLength(0);
   return buildExpectedSchema(root);
 }