no-mistakes(review): Read naming.collections in TS, keep it in probes, add tests

Author: dmealing

server/csharp/MetaObjects.Codegen/CodegenCli.cs

@@ -815,7 +815,7 @@ private static IEnumerable<PhysicalNaming> NamingCandidates(PhysicalNaming used)
         var tables = NamingStrategies.Tables;
         foreach (var t in tables.OrderBy(t => t == used.Tables ? 0 : 1))
             foreach (var c in columns.OrderBy(c => c == used.Columns ? 0 : 1))
-                if (new PhysicalNaming(c, t) is var candidate && candidate != used)
+                if (new PhysicalNaming(c, t) { Collections = used.Collections } is var candidate && candidate != used)
                     yield return candidate;
     }
 

server/csharp/MetaObjects.Conformance.Tests/CollectionNameVocabularyTests.cs

@@ -3,8 +3,10 @@
 // type-checks it. It loads under the strict
 // loader on object.entity / projection / report.
 
+using System.Text.Json;
 using MetaObjects;
 using MetaObjects.Loader;
+using MetaObjects.Meta;
 using Xunit;
 
 namespace MetaObjects.Conformance.Tests;
@@ -46,4 +48,39 @@ public void It_is_shared_vocabulary_marked_by_implementing_port()
         var attr = registry.Find("object", "entity")!.Attributes.Single(a => a.Name == "collectionName");
         Assert.Equal(new[] { "csharp" }, attr.ImplementedIn);
     }
+    [Fact]
+    public void The_standard_pluralizer_matches_the_shared_naming_conformance_corpus()
+    {
+        string repoRoot = Path.GetDirectoryName(Path.GetDirectoryName(CorpusRoot.Path)!)!;
+        using var doc = JsonDocument.Parse(File.ReadAllText(
+            Path.Combine(repoRoot, "fixtures", "naming-conformance", "already-plural-pluralize.json")));
+        var cases = doc.RootElement.GetProperty("cases").EnumerateArray().ToList();
+        Assert.NotEmpty(cases);
+        foreach (var c in cases)
+        {
+            string name = c.GetProperty("name").GetString()!;
+            Assert.Equal((name, c.GetProperty("apiPlural").GetString()), (name, CollectionNames.StandardPluralize(name)));
+        }
+    }
+
+    [Theory]
+    [InlineData(null, "CaseHistories")]
+    [InlineData("standard", "CaseHistories")]
+    [InlineData("none", "CaseHistory")]
+    public void An_object_without_collectionName_takes_the_project_pluralizer(string? collections, string expected)
+    {
+        const string model = """
+            { "metadata.root": { "package": "acme", "children": [
+              { "object.entity": { "name": "CaseHistory", "children": [ { "field.long": { "name": "id" } } ] } },
+              { "object.entity": { "name": "Diagnosis", "@collectionName": "DiagnosisRows", "children": [ { "field.long": { "name": "id" } } ] } }
+            ] } }
+            """;
+        var result = new MetaDataLoader(Provider.ComposeRegistry(CoreTypes.LibraryProviders), strict: true)
+            .Load([new InMemoryStringSource(model, id: "meta.json")]);
+        Assert.Empty(result.Errors.Select(e => e.Code + ": " + e.Message));
+        result.Root.PhysicalNaming = PhysicalNaming.Default with { Collections = collections };
+        var objects = result.Root.Objects().ToDictionary(o => o.Name);
+        Assert.Equal(expected, CollectionNames.Of(objects["CaseHistory"]));
+        Assert.Equal("DiagnosisRows", CollectionNames.Of(objects["Diagnosis"]));
+    }
 }

server/java/metadata/src/test/java/com/metaobjects/database/CollectionNamingTest.java

@@ -0,0 +1,104 @@
+package com.metaobjects.database;
+
+import com.google.gson.JsonArray;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonParser;
+import com.metaobjects.MetaDataException;
+import com.metaobjects.config.NeutralConfig;
+import org.junit.After;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * {@code naming.collections} names the pluralizer an object without {@code @collectionName}
+ * takes its collection name from. Java generates nothing from it yet, but a project shared with
+ * a port that does carries the key, so Java reads it, refuses an unknown name like
+ * {@code columns} and {@code tables}, and keeps the selected name on the loaded naming.
+ */
+public class CollectionNamingTest {
+
+    @After
+    public void reset() {
+        NamingStrategies.resetForTesting();
+    }
+
+    private static PhysicalNaming readNaming(String naming) throws IOException {
+        Path dir = Files.createTempDirectory("mo-collection-naming-");
+        try {
+            Files.createDirectory(dir.resolve(".metaobjects"));
+            Files.writeString(dir.resolve(".metaobjects").resolve("config.json"),
+                "{ \"schema_version\": 1, \"naming\": " + naming + " }");
+            return NeutralConfig.read(dir).orElseThrow().getPhysicalNaming();
+        } finally {
+            try (Stream<Path> paths = Files.walk(dir)) {
+                paths.sorted(Comparator.reverseOrder()).forEach(p -> p.toFile().delete());
+            }
+        }
+    }
+
+    @Test
+    public void theBuiltInsPluralizeOrKeepTheName() {
+        assertEquals(List.of("standard", "none"), NamingStrategies.BUILT_IN_PLURALIZERS);
+        assertEquals("standard", NamingStrategies.DEFAULT_PLURALIZER);
+        assertEquals("CaseHistories", NamingStrategies.pluralizer("standard").apply("CaseHistory"));
+        assertEquals("CaseHistory", NamingStrategies.pluralizer("none").apply("CaseHistory"));
+    }
+
+    @Test
+    public void standardMatchesTheSharedNamingConformanceCorpus() throws IOException {
+        Path repoRoot = Path.of(System.getProperty("user.dir")).resolve("../../..").normalize();
+        Path fixture = repoRoot.resolve("fixtures/naming-conformance/already-plural-pluralize.json");
+        JsonArray cases = JsonParser.parseString(Files.readString(fixture)).getAsJsonObject().getAsJsonArray("cases");
+        assertFalse(cases.isEmpty());
+        for (int i = 0; i < cases.size(); i++) {
+            JsonObject c = cases.get(i).getAsJsonObject();
+            String name = c.get("name").getAsString();
+            assertEquals(name, c.get("apiPlural").getAsString(), NamingStrategies.standardPluralize(name));
+        }
+    }
+
+    @Test
+    public void theConfigReadsABuiltIn() throws IOException {
+        assertEquals("none", readNaming("{ \"collections\": \"none\" }").collections());
+    }
+
+    @Test
+    public void absentNamesNoPluralizer() throws IOException {
+        assertNull(readNaming("{ \"columns\": \"snake_case\" }").collections());
+    }
+
+    @Test
+    public void anUnknownPluralizerIsRefusedNamingTheKind() {
+        MetaDataException ex = assertThrows(MetaDataException.class,
+            () -> readNaming("{ \"collections\": \"latin\" }"));
+        assertTrue(ex.getMessage(), ex.getMessage().contains("\"naming.collections\" names \"latin\""));
+        assertTrue(ex.getMessage(), ex.getMessage().contains("Java port"));
+    }
+
+    @Test
+    public void anApplicationPluralizerIsSelectable() throws IOException {
+        NamingStrategies.registerPluralizer("java-test-shout", n -> n.toUpperCase() + "S");
+        assertTrue(NamingStrategies.registeredPluralizers().contains("java-test-shout"));
+        assertEquals("java-test-shout", readNaming("{ \"collections\": \"java-test-shout\" }").collections());
+        NamingStrategies.resetForTesting();
+        assertFalse(NamingStrategies.isPluralizer("java-test-shout"));
+    }
+
+    @Test
+    public void aBuiltInPluralizerCannotBeReplaced() {
+        assertThrows(IllegalArgumentException.class, () -> NamingStrategies.registerPluralizer("none", n -> n));
+        assertThrows(IllegalArgumentException.class, () -> NamingStrategies.registerPluralizer("standard", n -> n));
+    }
+}

server/python/src/metaobjects/cli.py

@@ -2320,7 +2320,7 @@ def naming_candidates(used: NamingRules) -> list[NamingRules]:
         candidate
         for t in tables
         for c in columns
-        if (candidate := NamingRules(columns=c, tables=t)) != used
+        if (candidate := NamingRules(columns=c, tables=t, collections=used.collections)) != used
     ]
 
 

server/typescript/packages/sdk/src/config.ts

@@ -7,6 +7,7 @@ import {
   DEFAULT_PHYSICAL_NAMING,
   isRegisteredColumnNamingStrategy,
   isRegisteredTableNamingRule,
+  isRegisteredPluralizer,
   unknownNamingStrategyMessage,
   DEFAULT_WIRE_FORMAT,
   DEFAULT_ENUM_DEFAULTS,
@@ -227,6 +228,8 @@ export const ConfigSchema = z.object({
    * the PROJECT, read here by every port, because every port reads and writes the one database
    * `meta migrate` created: codegen, migrate, the runtime and verify all resolve through it.
    * Absent means `snake_case` for both, which is what `meta migrate` has always created.
+   * `collections` names the pluralizer (`standard`, `none` or one registered with
+   * `registerPluralizer`) an object without `@collectionName` takes its collection name from.
    *
    * Each value is the NAME of a registered strategy (ADR-0058 Amendment 1): a built-in
    * (`columns`: `snake_case`, `literal`, `kebab-case`, `pascal`; `tables`: `snake_case`,
@@ -247,6 +250,11 @@ export const ConfigSchema = z.object({
           if (!isRegisteredTableNamingRule(n)) ctx.addIssue({ code: "custom", message: unknownNamingStrategyMessage("tables", n) });
         })
         .optional(),
+      collections: z.string()
+        .superRefine((n, ctx) => {
+          if (!isRegisteredPluralizer(n)) ctx.addIssue({ code: "custom", message: unknownNamingStrategyMessage("collections", n) });
+        })
+        .optional(),
     })
     .strict()
     .optional(),
@@ -316,6 +324,7 @@ export function physicalNamingFromConfig(cfg: Pick<Config, "naming">): PhysicalN
   return {
     columns: cfg.naming?.columns ?? DEFAULT_PHYSICAL_NAMING.columns,
     tables: cfg.naming?.tables ?? DEFAULT_PHYSICAL_NAMING.tables,
+    ...(cfg.naming?.collections !== undefined ? { collections: cfg.naming.collections } : {}),
   };
 }
 

server/typescript/packages/sdk/test/collection-naming-config.test.ts

@@ -0,0 +1,90 @@
+// `naming.collections` names the pluralizer an object without `@collectionName` takes its
+// collection name from. The config reader accepts a registered pluralizer, refuses an unknown
+// one when the file is read, and the loaded root carries the selection to `collectionNameOf`.
+
+import { afterEach, describe, expect, test } from "bun:test";
+import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join, resolve } from "node:path";
+import {
+  InMemoryStringSource,
+  MetaDataLoader,
+  collectionNameOf,
+  isMetaRoot,
+  pluralizer,
+  registerPluralizer,
+  resetNamingStrategiesForTesting,
+  type MetaRoot,
+} from "@metaobjectsdev/metadata";
+import { loadConfig, physicalNamingFromConfig } from "../src/config.js";
+
+const PLURALS = resolve(import.meta.dir, "../../../../../fixtures/naming-conformance/already-plural-pluralize.json");
+
+afterEach(resetNamingStrategiesForTesting);
+
+async function readConfigWith(naming: unknown) {
+  const dir = mkdtempSync(join(tmpdir(), "mo-collection-naming-"));
+  try {
+    writeFileSync(join(dir, "config.json"), JSON.stringify({ schema_version: 1, naming }));
+    return await loadConfig(dir);
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+}
+
+async function loadModel(): Promise<MetaRoot> {
+  const json = JSON.stringify({
+    "metadata.root": {
+      package: "acme",
+      children: [
+        { "object.entity": { name: "CaseHistory", children: [{ "field.long": { name: "id" } }] } },
+        { "object.entity": { name: "Diagnosis", "@collectionName": "DiagnosisRows", children: [{ "field.long": { name: "id" } }] } },
+      ],
+    },
+  });
+  const result = await new MetaDataLoader().load([new InMemoryStringSource(json)]);
+  expect(result.errors).toEqual([]);
+  if (!isMetaRoot(result.root)) throw new Error("not a root");
+  return result.root;
+}
+
+describe("naming.collections", () => {
+  test("the standard pluralizer matches the shared naming-conformance corpus", () => {
+    const corpus = JSON.parse(readFileSync(PLURALS, "utf8")) as { cases: { name: string; apiPlural: string }[] };
+    expect(corpus.cases.length).toBeGreaterThan(0);
+    for (const c of corpus.cases) expect([c.name, pluralizer("standard")(c.name)]).toEqual([c.name, c.apiPlural]);
+  });
+
+  test("the config reads a built-in and the root carries it to collectionNameOf", async () => {
+    const config = await readConfigWith({ collections: "none" });
+    expect(config.naming?.collections).toBe("none");
+    const root = await loadModel();
+    root.physicalNaming = physicalNamingFromConfig(config);
+    expect(collectionNameOf(root.findObject("CaseHistory")!)).toBe("CaseHistory");
+    expect(collectionNameOf(root.findObject("Diagnosis")!)).toBe("DiagnosisRows");
+  });
+
+  test("absent, an object takes the standard pluralizer", async () => {
+    const config = await readConfigWith({ columns: "snake_case" });
+    expect(physicalNamingFromConfig(config).collections).toBeUndefined();
+    const root = await loadModel();
+    root.physicalNaming = physicalNamingFromConfig(config);
+    expect(collectionNameOf(root.findObject("CaseHistory")!)).toBe("CaseHistories");
+  });
+
+  test("an unknown pluralizer is refused when the config is read", async () => {
+    await expect(readConfigWith({ collections: "latin" })).rejects.toThrow(/"naming\.collections" names "latin"/);
+  });
+
+  test("an application pluralizer is selectable by name", async () => {
+    registerPluralizer("ts-test-shout", (n) => n.toUpperCase() + "S");
+    const config = await readConfigWith({ collections: "ts-test-shout" });
+    const root = await loadModel();
+    root.physicalNaming = physicalNamingFromConfig(config);
+    expect(collectionNameOf(root.findObject("CaseHistory")!)).toBe("CASEHISTORYS");
+  });
+
+  test("a built-in pluralizer cannot be replaced", () => {
+    expect(() => registerPluralizer("none", (n) => n)).toThrow();
+  });
+});