refactor(publish): remove 'entity' and 'env' fields from records and update validation logic

Author: m-houston

packages/ddb-publisher/src/__integration__/publish-action.integration.test.ts

@@ -21,46 +21,34 @@ const bundlePath = resolve(
 );
 const tableName = "supplier-config-it";
 const expectedRecords = [
-  {
-    pk: "ENTITY#channel",
-    sk: "ID#letter",
-    entity: "channel",
-    id: "letter",
-  },
   {
     pk: "ENTITY#volume-group",
     sk: "ID#vg-1",
-    entity: "volume-group",
     id: "vg-1",
   },
   {
     pk: "ENTITY#letter-variant",
     sk: "ID#lv-1",
-    entity: "letter-variant",
     id: "lv-1",
   },
   {
     pk: "ENTITY#pack-specification",
     sk: "ID#pack-spec-1",
-    entity: "pack-specification",
     id: "pack-spec-1",
   },
   {
     pk: "ENTITY#supplier",
     sk: "ID#sup-1",
-    entity: "supplier",
     id: "sup-1",
   },
   {
     pk: "ENTITY#supplier-allocation",
     sk: "ID#alloc-1",
-    entity: "supplier-allocation",
     id: "alloc-1",
   },
   {
     pk: "ENTITY#supplier-pack",
     sk: "ID#sp-1",
-    entity: "supplier-pack",
     id: "sp-1",
   },
 ] as const;
@@ -125,8 +113,6 @@ function expectExpectedRecords(
     );
 
     expect(actual).toBeDefined();
-    expect(actual?.env?.S).toBe("draft");
-    expect(actual?.entity?.S).toBe(expectedRecord.entity);
     expect(actual?.id?.S).toBe(expectedRecord.id);
   }
 }
@@ -187,9 +173,7 @@ describe("publish action integration (DynamoDB Local)", () => {
         Item: {
           pk: { S: "ENTITY#supplier" },
           sk: { S: "ID#stale-supplier" },
-          entity: { S: "supplier" },
           id: { S: "stale-supplier" },
-          env: { S: "draft" },
           status: { S: "DRAFT" },
           name: { S: "Stale Supplier" },
           channelType: { S: "LETTER" },

packages/ddb-publisher/src/__tests__/publish.test.ts

@@ -7,7 +7,7 @@ function makeRecords(count: number): ConfigRecord[] {
     entity: "supplier",
     sourceFilePath: `/tmp/supplier/${i}.json`,
     id: String(i),
-    data: { ok: true },
+    data: { id: String(i), ok: true },
   }));
 }
 
@@ -42,7 +42,6 @@ describe("publishRecords", () => {
     await publishRecords({
       ddb: { send } as any,
       tableName: "tbl",
-      env: "draft",
       records: [],
     });
 
@@ -55,7 +54,6 @@ describe("publishRecords", () => {
     await publishRecords({
       ddb: ddb as any,
       tableName: "tbl",
-      env: "draft",
       records: makeRecords(26),
     });
 
@@ -70,13 +68,13 @@ describe("publishRecords", () => {
     await publishRecords({
       ddb: ddb as any,
       tableName: "tbl",
-      env: "draft",
       records: [
         {
           entity: "supplier",
           sourceFilePath: "/tmp/supplier/sup-1.json",
           id: "sup-1",
           data: {
+            id: "sup-1",
             name: "Example Supplier",
             status: "DRAFT",
             dailyCapacity: 100,
@@ -91,9 +89,7 @@ describe("publishRecords", () => {
           Item: {
             pk: "ENTITY#supplier",
             sk: "ID#sup-1",
-            entity: "supplier",
             id: "sup-1",
-            env: "draft",
             name: "Example Supplier",
             status: "DRAFT",
             dailyCapacity: 100,
@@ -125,7 +121,6 @@ describe("publishRecords", () => {
     await publishRecords({
       ddb: ddb as any,
       tableName: "tbl",
-      env: "draft",
       records: makeRecords(1),
     });
 
@@ -147,9 +142,27 @@ describe("publishRecords", () => {
       publishRecords({
         ddb: ddb as any,
         tableName: "tbl",
-        env: "draft",
         records: makeRecords(1),
       }),
     ).rejects.toThrow("Failed to write");
   });
+
+  it("should reject records that are not objects with an id string", async () => {
+    const { ddb } = fakeDdb();
+
+    await expect(
+      publishRecords({
+        ddb: ddb as any,
+        tableName: "tbl",
+        records: [
+          {
+            entity: "supplier",
+            sourceFilePath: "/tmp/supplier/sup-1.json",
+            id: "sup-1",
+            data: "not-an-object",
+          },
+        ],
+      }),
+    ).rejects.toThrow();
+  });
 });

packages/ddb-publisher/src/__tests__/run.test.ts

@@ -192,8 +192,8 @@ describe("runPublisher", () => {
           data: {},
         },
         {
-          entity: "channel",
-          sourceFilePath: "/tmp/channel/1.json",
+          entity: "supplier-pack",
+          sourceFilePath: "/tmp/supplier-pack/1.json",
           id: "1",
           data: {},
         },
@@ -218,7 +218,7 @@ describe("runPublisher", () => {
     });
 
     const logOutput = writeSpy.mock.calls.map(([msg]) => String(msg)).join("\n");
-    expect(logOutput).toContain("Loaded 3 records (channel=1, supplier=2).\n");
+    expect(logOutput).toContain("Loaded 3 records (supplier=2, supplier-pack=1).\n");
 
     writeSpy.mockRestore();
   });

packages/ddb-publisher/src/ddb/audit.ts

@@ -1,5 +1,5 @@
 import type { AttributeValue } from "@aws-sdk/client-dynamodb";
-import { DynamoDBDocumentClient, ScanCommand } from "@aws-sdk/lib-dynamodb";
+import {DynamoDBDocumentClient, ScanCommand, ScanCommandOutput} from "@aws-sdk/lib-dynamodb";
 import { z } from "zod";
 
 import type { ConfigRecord } from "@supplier-config/file-store";
@@ -45,7 +45,7 @@ async function auditBeforeLoad(params: {
 
   // Scan all records in the table (projecting only keys used for validation)
   do {
-    const page = await params.ddb.send(
+    const page: ScanCommandOutput = await params.ddb.send(
       new ScanCommand({
         TableName: params.tableName,
         ExclusiveStartKey: lastEvaluatedKey ?? undefined,

packages/ddb-publisher/src/ddb/publish.ts

@@ -1,26 +1,27 @@
 import {BatchWriteCommand, DynamoDBDocumentClient} from "@aws-sdk/lib-dynamodb";
+import {z} from "zod";
 
 import type {ConfigRecord} from "@supplier-config/file-store";
 
 import {pkForEntity, skForId} from "./keys";
 
-function itemForRecord(record: ConfigRecord, env: string): Record<string, unknown> {
-  const base = record.data as Record<string, unknown>;
+const persistedRecordShapeSchema = z.looseObject({
+  id: z.string(),
+});
+
+function itemForRecord(record: ConfigRecord): Record<string, unknown> {
+  const base = persistedRecordShapeSchema.parse(record.data);
 
   return {
     pk: pkForEntity(record.entity),
     sk: skForId(record.id),
-    entity: record.entity,
-    id: record.id,
-    env,
     ...base,
   };
 }
 
 export async function publishRecords(params: {
   ddb: DynamoDBDocumentClient;
   tableName: string;
-  env: string;
   records: ConfigRecord[];
 }): Promise<void> {
   const chunks: ConfigRecord[][] = [];
@@ -31,7 +32,7 @@ export async function publishRecords(params: {
   for (const chunk of chunks) {
     let unprocessed = chunk.map((r) => ({
       PutRequest: {
-        Item: itemForRecord(r, params.env),
+        Item: itemForRecord(r),
       },
     }));
 

packages/ddb-publisher/src/run.ts

@@ -145,7 +145,6 @@ async function runPublisher(plan: LoadPlan): Promise<void> {
   await publishRecords({
     ddb,
     tableName: plan.tableName,
-    env: plan.env,
     records: store.records,
   });
 

packages/file-store/src/__tests__/config-store-loader.test.ts

@@ -18,9 +18,8 @@ describe("loadConfigStore", () => {
     const store = await loadConfigStore(exampleConfigStoreRoot);
 
     expect(store.rootPath).toBe(exampleConfigStoreRoot);
-    expect(store.records).toHaveLength(7);
+    expect(store.records).toHaveLength(6);
     expect(store.records.map((record) => `${record.entity}:${record.id}`).sort()).toEqual([
-      "channel:letter",
       "letter-variant:lv-1",
       "pack-specification:pack-spec-1",
       "supplier-allocation:alloc-1",
@@ -30,11 +29,12 @@ describe("loadConfigStore", () => {
     ]);
   });
 
-  it("should ignore non-json files and unexpected directories", async () => {
+  it("should ignore non-json files and non-persisted directories", async () => {
     const root = await createTempConfigStore();
 
     try {
       await mkdir(join(root, "supplier"), { recursive: true });
+      await mkdir(join(root, "channel"), { recursive: true });
       await mkdir(join(root, "unexpected-folder"), { recursive: true });
       await writeFile(
         join(root, "supplier", "sup-1.json"),
@@ -47,6 +47,11 @@ describe("loadConfigStore", () => {
         JSON.stringify({ ignored: true }),
         "utf8",
       );
+      await writeFile(
+        join(root, "channel", "letter.json"),
+        JSON.stringify("LETTER"),
+        "utf8",
+      );
 
       const store = await loadConfigStore(root);
 

packages/file-store/src/__tests__/config-store-validator.test.ts

@@ -8,7 +8,6 @@ import {
 
 jest.mock("@nhsdigital/nhs-notify-event-schemas-supplier-config", () => {
   return {
-    $ChannelType: z.literal("LETTER"),
     $VolumeGroup: z.object({
       id: z.string(),
       name: z.string(),
@@ -57,7 +56,6 @@ describe("normalizeIssuePath", () => {
 describe("validateConfigStore", () => {
   it("should return ok=true with no issues for a larger mixed valid dataset", () => {
     const validRecords = [
-      makeRecord("channel", "channel-1", "LETTER"),
       makeRecord("volume-group", "vg-1", {
         id: "vg-1",
         name: "VG 1",
@@ -177,4 +175,24 @@ describe("validateConfigStore", () => {
     expect(result.ok).toBe(true);
     expect(result.issues).toHaveLength(0);
   });
+
+  it("should report an issue when the filename id does not match the parsed object id", () => {
+    const result = validateConfigStore({
+      rootPath: "/tmp",
+      records: [
+        makeRecord("supplier", "sup-1", {
+          id: "different-id",
+          dailyCapacity: 100,
+        }),
+      ],
+    });
+
+    expect(result.ok).toBe(false);
+    expect(result.issues).toContainEqual({
+      entity: "supplier",
+      sourceFilePath: "/tmp/supplier/sup-1.json",
+      message: "Record id 'different-id' does not match filename id 'sup-1'.",
+      path: ["id"],
+    });
+  });
 });

packages/file-store/src/loader/config-store-loader.ts

@@ -7,8 +7,7 @@ import type {
   LoadedConfigStore,
 } from "../types.js";
 
-const entityDirectoryAllowList: readonly DomainEntityName[] = [
-  "channel",
+const persistedEntityDirectoryAllowList: readonly DomainEntityName[] = [
   "volume-group",
   "letter-variant",
   "pack-specification",
@@ -17,8 +16,8 @@ const entityDirectoryAllowList: readonly DomainEntityName[] = [
   "supplier-pack",
 ];
 
-function isEntityDirName(name: string): name is DomainEntityName {
-  return (entityDirectoryAllowList as readonly string[]).includes(name);
+function isPersistedEntityDirName(name: string): name is DomainEntityName {
+  return (persistedEntityDirectoryAllowList as readonly string[]).includes(name);
 }
 
 /**
@@ -44,7 +43,7 @@ export async function loadConfigStore(
   const records: ConfigRecord[] = [];
 
   for (const entry of dirents) {
-    if (!isEntityDirName(entry)) continue;
+    if (!isPersistedEntityDirName(entry)) continue;
 
     const entityDir = join(resolvedRoot, entry);
     const files = await readdir(entityDir);

packages/file-store/src/types.ts

@@ -1,7 +1,6 @@
 import type { z } from 'zod';
 
 export type DomainEntityName =
-  | 'channel'
   | 'volume-group'
   | 'letter-variant'
   | 'pack-specification'