Merge pull request #65 from metaobjectsdev/fix/migrate-identity-reference-ondelete

fix(migrate): honor @onDelete/@onUpdate declared on identity.reference

Author: dmealing

server/typescript/packages/metadata/src/core/identity/identity-constants.ts

@@ -27,6 +27,10 @@ export const IDENTITY_ATTR_UNIQUE = "unique";
 export const IDENTITY_REFERENCE_ATTR_REFERENCES = "references";
 /** Identity-reference attr: physical-enforcement flag. Default true → hard FK constraint; false → logical-only reference. */
 export const IDENTITY_REFERENCE_ATTR_ENFORCE = "enforce";
+/** Identity-reference attr: referential action on parent delete (cascade/set-null/restrict/no-action). */
+export const IDENTITY_REFERENCE_ATTR_ON_DELETE = "onDelete";
+/** Identity-reference attr: referential action on key update (cascade/set-null/restrict/no-action). */
+export const IDENTITY_REFERENCE_ATTR_ON_UPDATE = "onUpdate";
 
 // NOTE: physical RDB-only identity attrs (@constraintName on identity.reference;
 // @orders / @where on identity.secondary) are NOT core — they are contributed by

server/typescript/packages/metadata/src/core/identity/meta-identity.ts

@@ -13,6 +13,8 @@ import {
   IDENTITY_ATTR_UNIQUE,
   IDENTITY_REFERENCE_ATTR_REFERENCES,
   IDENTITY_REFERENCE_ATTR_ENFORCE,
+  IDENTITY_REFERENCE_ATTR_ON_DELETE,
+  IDENTITY_REFERENCE_ATTR_ON_UPDATE,
 } from "./identity-constants.js";
 import type { MetaRoot } from "../../shared/meta-root.js";
 
@@ -104,6 +106,24 @@ export class MetaReferenceIdentity extends MetaIdentity {
     return this.ownAttr(IDENTITY_REFERENCE_ATTR_ENFORCE) !== false;
   }
 
+  /**
+   * Referential action on parent delete, declared directly on the FK-defining
+   * reference (cascade / set-null / restrict / no-action). Undefined when not
+   * set — callers fall back to a correlated relationship's @onDelete, then the
+   * relationship-subtype default. The FK is declared here, so the action may be
+   * declared here too rather than only on a sibling relationship node.
+   */
+  get onDelete(): string | undefined {
+    const v = this.ownAttr(IDENTITY_REFERENCE_ATTR_ON_DELETE);
+    return typeof v === "string" && v !== "" ? v : undefined;
+  }
+
+  /** Referential action on key update, declared directly on the reference. Undefined when not set. */
+  get onUpdate(): string | undefined {
+    const v = this.ownAttr(IDENTITY_REFERENCE_ATTR_ON_UPDATE);
+    return typeof v === "string" && v !== "" ? v : undefined;
+  }
+
   /**
    * Target field names. Empty array means "use the target's primary identity"
    * (the bare-entity form). For dotted forms, returns the field(s) after the

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

@@ -41,13 +41,18 @@ export function isRequired(field: MetaData): boolean {
 
 /**
  * Resolve the referential actions for a foreign key inferred from an
- * identity.reference, by correlating it with a sibling relationship on the
- * same entity (matched on target-entity name).
+ * identity.reference.
  *
- * - No correlated relationship → both undefined (no ON DELETE / ON UPDATE clause).
- * - With a relationship: onDelete defaults from the relationship subtype
- *   (composition→cascade, aggregation→set-null, association→restrict);
- *   onUpdate defaults to "cascade". Explicit @onDelete / @onUpdate override.
+ * Precedence (highest first):
+ *   1. @onDelete / @onUpdate declared DIRECTLY on the identity.reference — the
+ *      reference IS the FK, so the action may be declared right where the FK is.
+ *   2. A correlated sibling relationship on the same entity (matched on
+ *      target-entity name): its explicit @onDelete, else its subtype default
+ *      (composition→cascade, aggregation→set-null, association→restrict);
+ *      onUpdate defaults to "cascade".
+ *   3. Neither → undefined (no ON DELETE / ON UPDATE clause).
+ *
+ * - "setnull" is accepted as an alias for the canonical "set-null".
  * - Resolved "no-action" → undefined: introspection in introspect/{postgres,sqlite}.ts
  *   omits actions when the DB value is "no-action", so the expected side does the same
  *   to keep round-trip diffs clean.
@@ -75,20 +80,35 @@ export function resolveReferentialActions(
   // resolve to undefined (no clause emitted) — surfacing the mismatch as a
   // silent loss of intent rather than a wrong action. Cross-language ports
   // should match the same correlation rule.
+  // (1) Actions declared directly on the FK-defining reference win.
+  const refOnDelete = ref.onDelete;
+  const refOnUpdate = ref.onUpdate;
+
+  // (2) Otherwise correlate with a sibling relationship and use its action /
+  //     subtype default. onUpdate's "cascade" default only applies when a
+  //     relationship is present, so a reference-only FK with no explicit
+  //     @onUpdate emits no ON UPDATE clause.
   const rel = entity.relationships().find((r) => r.objectRef === target);
-  if (rel === undefined) return { onDelete: undefined, onUpdate: undefined };
 
-  const onDeleteRaw = rel.onDelete ?? ON_DELETE_DEFAULT_BY_SUBTYPE[rel.subType];
-  const onUpdateRaw = rel.onUpdate ?? ON_UPDATE_DEFAULT;
+  const onDeleteRaw =
+    refOnDelete ??
+    (rel ? (rel.onDelete ?? ON_DELETE_DEFAULT_BY_SUBTYPE[rel.subType]) : undefined);
+  const onUpdateRaw =
+    refOnUpdate ??
+    (rel ? (rel.onUpdate ?? ON_UPDATE_DEFAULT) : undefined);
+
   return {
     onDelete: normalize(onDeleteRaw),
     onUpdate: normalize(onUpdateRaw),
   };
 }
 
 function normalize(a: string | undefined): FkAction | undefined {
-  if (a === undefined || a === "no-action") return undefined;
-  return a as FkAction;
+  if (a === undefined) return undefined;
+  // Accept the hyphen-less spelling as an alias for the canonical kebab-case form.
+  const canonical = a === "setnull" ? "set-null" : a;
+  if (canonical === "no-action") return undefined;
+  return canonical as FkAction;
 }
 
 // ---------------------------------------------------------------------------

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

@@ -142,3 +142,70 @@ describe("end-to-end FK actions in emitted DDL", () => {
     expect(up).toContain('FOREIGN KEY ("program_id") REFERENCES "programs" ("id") ON DELETE SET NULL ON UPDATE CASCADE');
   });
 });
+
+// ---------------------------------------------------------------------------
+// Referential actions declared DIRECTLY on the identity.reference (the FK).
+//
+// Regression: an @onDelete / @onUpdate authored on the identity.reference was
+// previously ignored — only a correlated relationship node supplied actions —
+// so FKs declared with @onDelete: cascade emitted a bare FK (no ON DELETE),
+// silently dropping cascade intent.
+// ---------------------------------------------------------------------------
+
+function weekDocRef(refAttrs: Record<string, unknown>, rel?: Record<string, unknown>) {
+  return { "metadata.root": { package: "acme", children: [
+    { "object.entity": { name: "Program", children: [
+      { "field.long": { name: "id" } },
+      { "identity.primary": { "name": "id", "@fields": "id" } },
+    ] } },
+    { "object.entity": { name: "Week", children: [
+      { "field.long": { name: "id" } },
+      { "field.long": { name: "programId" } },
+      ...(rel ? [rel] : []),
+      { "identity.reference": { name: "ref_program", "@fields": ["programId"], "@references": "Program", ...refAttrs } },
+      { "identity.primary": { "name": "id", "@fields": "id" } },
+    ] } },
+  ] } };
+}
+
+async function loadWeekRef(refAttrs: Record<string, unknown>, rel?: Record<string, unknown>) {
+  const { root, errors } = await loadDoc(weekDocRef(refAttrs, rel));
+  expect(errors).toHaveLength(0);
+  const week = root.objects().find((o) => o.name === "Week")!;
+  const ref = week.referenceIdentities()[0]!;
+  return { root, week, ref };
+}
+
+describe("reference-level @onDelete / @onUpdate (declared on identity.reference)", () => {
+  test("reference @onDelete wins with no relationship; onUpdate stays absent", async () => {
+    const { week, ref } = await loadWeekRef({ "@onDelete": "cascade" });
+    expect(resolveReferentialActions(week, ref)).toEqual({ onDelete: "cascade", onUpdate: undefined });
+  });
+
+  test("reference @onUpdate is honored independently", async () => {
+    const { week, ref } = await loadWeekRef({ "@onDelete": "restrict", "@onUpdate": "cascade" });
+    expect(resolveReferentialActions(week, ref)).toEqual({ onDelete: "restrict", onUpdate: "cascade" });
+  });
+
+  test("'setnull' is accepted as an alias for the canonical 'set-null'", async () => {
+    const { week, ref } = await loadWeekRef({ "@onDelete": "setnull" });
+    expect(resolveReferentialActions(week, ref)).toEqual({ onDelete: "set-null", onUpdate: undefined });
+  });
+
+  test("reference @onDelete overrides a correlated relationship's action", async () => {
+    const { week, ref } = await loadWeekRef(
+      { "@onDelete": "restrict" },
+      { "relationship.composition": { name: "program", "@objectRef": "Program", "@cardinality": "one" } },
+    );
+    expect(resolveReferentialActions(week, ref)).toEqual({ onDelete: "restrict", onUpdate: "cascade" });
+  });
+
+  test("end-to-end: reference @onDelete: cascade → ON DELETE CASCADE (Postgres), no relationship node", async () => {
+    const { root } = await loadWeekRef({ "@onDelete": "cascade" });
+    const snapshot = buildExpectedSchema(root);
+    const { changes } = await diff(snapshot, EMPTY_SCHEMA);
+    const { up } = emit(changes, { dialect: "postgres" });
+    expect(up).toContain('ADD CONSTRAINT "weeks_program_id_fk"');
+    expect(up).toContain('REFERENCES "programs" ("id") ON DELETE CASCADE');
+  });
+});