From 371957ac27dbc529838ec0ed935e4294b31f538d Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Sun, 30 Aug 2026 16:41:21 -0400 Subject: [PATCH 1/8] feat(dice-storage): Drivine-backed MetamodelVersionStore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Persist MetamodelVersion stamps in Neo4j: idempotent MERGE on the (schemaName, contentHash) natural key, findVersion by hash, and history ordered by a persisted per-schema sequence rather than wall-clock time — a MetamodelVersion(schemaName, sequence) uniqueness constraint makes a duplicate position unstorable, so write-order corruption is loud and retryable. Property signatures serialize as deterministic sorted JSON. Introduce the shared Neo4jTestContainer and adopt it across the module's Spring Boot ITs. Refs #45; stacks on feat/metamodel-versioning. --- CHANGELOG.md | 22 + dice-storage/pom.xml | 6 + .../storage/DrivineMetamodelVersionStore.kt | 198 ++++++ .../dice/storage/MetamodelRowMappers.kt | 219 ++++++ ...ivineCollectorTraceStoreIntegrationTest.kt | 15 +- .../DrivineGraphQueryParityIntegrationTest.kt | 11 + ...rivineLineageRecordStoreIntegrationTest.kt | 11 + ...ineMetamodelVersionStoreIntegrationTest.kt | 673 ++++++++++++++++++ ...PropositionStoreContractIntegrationTest.kt | 11 + .../DrivinePropositionStoreIntegrationTest.kt | 17 +- .../dice/storage/MetamodelRowMapperTest.kt | 166 +++++ .../dice/storage/Neo4jTestContainer.kt | 61 ++ .../embabel/dice/storage/TestApplication.kt | 60 ++ docs/design/architecture.md | 15 +- docs/design/metamodel-versioning.md | 27 + 15 files changed, 1500 insertions(+), 12 deletions(-) create mode 100644 dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt create mode 100644 dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt create mode 100644 dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt create mode 100644 dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt create mode 100644 dice-storage/src/test/kotlin/com/embabel/dice/storage/Neo4jTestContainer.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index cd5fa989..0ae2349d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,3 +45,25 @@ and the consumer PRs that deliver it). metadata key is removed; lineage answers per-proposition attribution. **Compatibility: breaking.** The key is no longer available; code holding it must migrate to extraction-run queries. +- `DiceMetadataKeys.METAMODEL_VERSION` metadata key and stamping contract. + Propositions can carry the declared schema version hash under this key to + record which schema governed their extraction. The key is defined here with + its contract; production wiring that stamps propositions at persistence time + lands in a follow-up slice after the extraction-run stack merges. + **Compatibility: additive.** New metadata key only; no existing API or code + touched. +- Drivine/Neo4j-backed `MetamodelVersionStore` in `dice-storage` + (`DrivineMetamodelVersionStore`): stamps persist as `(:MetamodelVersion)` nodes, + MERGEd on the natural key `(schemaName, contentHash)` so a re-stamp updates in + place instead of duplicating. `latestVersion`, `versionHistory` and `findVersion` + all resolve in Cypher. History is ordered by a persisted per-schema sequence, taken + off a `(:MetamodelSchemaCounter)` node in the same statement that creates the version, + so ordering is true logical write order rather than a wall clock that ties within a + millisecond and can run backwards under NTP correction or failover; `savedAt` and + `savedAtEpochMillis` remain as informational metadata. An idempotent re-save neither + bumps the counter nor reassigns a sequence. Concurrent saves of one version leave + exactly one node. Hosts must declare three uniqueness constraints: + `MetamodelVersion(schemaName, contentHash)`, `MetamodelSchemaCounter(schemaName)`, and + `MetamodelVersion(schemaName, sequence)`. + **Compatibility: additive.** New class and a new `dice-storage` → `dice-metamodel` + module dependency; no existing API touched. diff --git a/dice-storage/pom.xml b/dice-storage/pom.xml index 46bc43e0..c461dc10 100644 --- a/dice-storage/pom.xml +++ b/dice-storage/pom.xml @@ -31,6 +31,12 @@ dice + + + com.embabel.dice + dice-metamodel + + com.embabel.agent diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt new file mode 100644 index 00000000..ce82189a --- /dev/null +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt @@ -0,0 +1,198 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.storage + +import com.embabel.dice.metamodel.MetamodelVersion +import com.embabel.dice.metamodel.MetamodelVersionStore +import org.drivine.manager.PersistenceManager +import org.drivine.query.QuerySpecification +import org.slf4j.LoggerFactory +import org.springframework.transaction.annotation.Transactional +import java.time.Clock + +/** + * Drivine / Neo4j implementation of [MetamodelVersionStore]: keeps every schema stamp as a + * `(:MetamodelVersion)` node. + * + * The write MERGEs on the natural key `(schemaName, contentHash)`, so a retry or a re-stamp of an + * unchanged schema updates the node that's already there instead of adding a duplicate. That's only + * race-free under a uniqueness constraint on the same pair of properties — without one, concurrent + * MERGEs all miss, all take the CREATE branch, and history fills with copies of one version. + * + * Every statement is parameterized — nothing user-derived is ever interpolated into Cypher. Ordering + * and the keyed lookup both run in the database rather than over an in-memory list. + * + * **Order comes from a counter, not from a clock.** The contract says "most recent" means logical + * write order, and a wall clock can't express that: two saves can land in the same millisecond, and + * an NTP correction or a failover to a differently-skewed node can make the clock run *backwards* + * between them. Either way the newest stamp stops being the one that comes back. So each schema owns + * a `(:MetamodelSchemaCounter)` node, and a version gets the next value off it when — and only + * when — its node is first created. That sequence is what every ordered read sorts on. `savedAt` and + * `savedAtEpochMillis` are still written, but they are now informational metadata: useful when you + * are staring at a node wondering when it landed, and sorted on by nothing. + * + * The counter is bumped in the same statement, and so the same transaction, as the MERGE that + * creates the version — there is no window in which a version node exists without its place in the + * order. A re-save of a version that already exists neither bumps the counter nor reassigns the + * sequence, which is what keeps an idempotent write idempotent and stops an old stamp jumping to the + * head of the history. That mirrors the in-memory reference implementation, where a re-save keeps + * its original position in the list. + * + * **Three constraints are required**, and all are the host's job to declare in a `SchemaCatalog` + * bean (the module's `TestApplication` shows the shape): + * - `UniquenessConstraintSpec("MetamodelVersion", listOf("schemaName", "contentHash"))` — makes the + * version MERGE race-free, as above. + * - `UniquenessConstraintSpec("MetamodelSchemaCounter", "schemaName")` — makes the counter MERGE + * race-free, so a schema can't end up with two counters handing out the same numbers. + * - `UniquenessConstraintSpec("MetamodelVersion", listOf("schemaName", "sequence"))` — makes two + * versions sharing a position in the order unstorable, so a lost counter update fails loudly and + * retryably instead of quietly making "newest first" arbitrary again. + * + * @param persistenceManager Drivine's handle on the `neo` datasource. + * @param clock supplies the instant a version is stamped as saved at. Injectable so a test can place + * two saves at instants it chooses rather than at whatever the wall clock happened to say. + */ +open class DrivineMetamodelVersionStore( + private val persistenceManager: PersistenceManager, + private val clock: Clock = Clock.systemUTC(), +) : MetamodelVersionStore { + + private val logger = LoggerFactory.getLogger(DrivineMetamodelVersionStore::class.java) + + private companion object { + + /** + * Upsert the version node and, if this is the first time we've seen it, give it the next + * number off its schema's counter. One statement, so one transaction: a version node never + * exists without its place in the write order. + * + * The two halves are separated by `WITH n WHERE n.sequence IS NULL`. On a re-save that + * filters the row away, so the counter is never bumped and the existing sequence is never + * reassigned — the content is refreshed and the version keeps the position it has always + * had. + * + * `SET c.lockedBy = $contentHash` writes a property nobody reads, to take the exclusive lock + * on the counter before the increment below reads it. On its own, + * `SET c.sequence = coalesce(c.sequence, 0) + 1` is a read-modify-write, and the textbook + * failure is two concurrent saves both reading 5, both writing 6, and two versions claiming + * one position. This is the documented Neo4j idiom for avoiding that. + * + * Being honest about how well that's established: at 12 and at 48 concurrent savers this + * store's own tests could not tell the locked and unlocked versions apart, so Neo4j appears + * to serialise the increment on its own here. The line is kept as cheap insurance, not + * because a failing test demanded it — don't read it as load-bearing. + * + * What *is* load-bearing is the uniqueness constraint on `(schemaName, sequence)`. It makes + * a duplicate position impossible to store rather than merely unlikely: if the increment + * ever did lose an update — a Neo4j version with different locking, a cluster, contention + * beyond what's been tried — the second writer fails loudly with a constraint violation and + * the caller retries, instead of silently corrupting the order. Correctness rests on that, + * not on a lock this code can't verify. + */ + private val SAVE_VERSION = """ + MERGE (n:MetamodelVersion {schemaName: ${'$'}schemaName, contentHash: ${'$'}contentHash}) + ON CREATE SET n.savedAt = ${'$'}savedAt, + n.savedAtEpochMillis = ${'$'}savedAtEpochMillis + SET n.entityTypeNames = ${'$'}entityTypeNames, + n.entityTypeLabels = ${'$'}entityTypeLabels, + n.entityTypeProperties = ${'$'}entityTypeProperties, + n.relationshipNames = ${'$'}relationshipNames + WITH n + WHERE n.sequence IS NULL + MERGE (c:MetamodelSchemaCounter {schemaName: ${'$'}schemaName}) + SET c.lockedBy = ${'$'}contentHash + WITH n, c + SET c.sequence = coalesce(c.sequence, 0) + 1 + WITH n, c + SET n.sequence = c.sequence + """.trimIndent() + + /** + * Every stamp for one schema, newest first. + * + * `coalesce(n.sequence, -1)` rather than a bare `n.sequence` because Neo4j sorts null as the + * *largest* value, so a node that somehow has no sequence would sort to the front of a DESC + * order and be handed back as the newest. A node with no sequence never took a place in the + * write order at all, so last is the honest position for it. + */ + private val VERSIONS_NEWEST_FIRST = """ + MATCH (n:MetamodelVersion {schemaName: ${'$'}schemaName}) + RETURN n + ORDER BY coalesce(n.sequence, -1) DESC + """.trimIndent() + } + + @Transactional + override fun saveVersion(version: MetamodelVersion) { + logger.debug( + "Saving metamodel version schemaName={} contentHash={}", + version.schemaName, + version.contentHash.take(8), + ) + persistenceManager.execute( + QuerySpecification.withStatement(SAVE_VERSION) + .bind(MetamodelVersionRowMapper.bindMap(version, clock.instant())), + ) + } + + @Transactional(readOnly = true) + override fun latestVersion(schemaName: String): MetamodelVersion? = + readVersions(VERSIONS_NEWEST_FIRST, mapOf("schemaName" to schemaName)).firstOrNull() + + @Transactional(readOnly = true) + override fun versionHistory(schemaName: String): List = + readVersions(VERSIONS_NEWEST_FIRST, mapOf("schemaName" to schemaName)) + + /** + * Overridden so resolving a recorded hash is a single keyed `MATCH` rather than a read of the + * schema's whole history followed by an in-memory filter. Both halves of the natural key are in + * the pattern, which is exactly what the uniqueness constraint indexes. + */ + @Transactional(readOnly = true) + override fun findVersion(schemaName: String, contentHash: String): MetamodelVersion? = readVersions( + """ + MATCH (n:MetamodelVersion {schemaName: ${'$'}schemaName, contentHash: ${'$'}contentHash}) + RETURN n + LIMIT 1 + """.trimIndent(), + mapOf("schemaName" to schemaName, "contentHash" to contentHash), + ).firstOrNull() + + /** + * Run one of the version queries and turn its rows into stamps, dropping any row that won't + * deserialize. + * + * A single corrupt or tampered node shouldn't take down a whole history read, so it's warned + * about and skipped — see [MetamodelVersionRowMapper], which throws rather than inventing + * defaults precisely so this can happen. The warning names the property or the failed integrity + * check, which is what an operator needs to go find the node. + * + * Note what [latestVersion] does *not* do: push a `LIMIT 1` into Cypher. If the newest node were + * the corrupt one, that would read it, drop it, and answer "this schema has no versions" — + * hiding the perfectly good history behind it, and disagreeing with [versionHistory], whose + * first element is meant to be the same stamp. It sorts in the database and takes the first + * survivor instead, so a bad node hides only itself. + */ + private fun readVersions(statement: String, bindings: Map): List { + @Suppress("UNCHECKED_CAST") + val spec = QuerySpecification.withStatement(statement).bind(bindings) as QuerySpecification + return persistenceManager.query(spec).filterIsInstance>().mapNotNull { row -> + runCatching { MetamodelVersionRowMapper.fromRow(row) } + .onFailure { logger.warn("Skipping unreadable MetamodelVersion row: {}", it.message) } + .getOrNull() + } + } +} diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt new file mode 100644 index 00000000..f9e483c3 --- /dev/null +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt @@ -0,0 +1,219 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.storage + +import com.embabel.agent.core.Cardinality +import com.embabel.dice.metamodel.MetamodelVersion +import com.embabel.dice.metamodel.PropertySignature +import com.fasterxml.jackson.databind.ObjectMapper +import java.time.Instant + +private val objectMapper = ObjectMapper() + +/** + * Translate metamodel versions to and from the property maps the Neo4j graph store reads and + * writes. + * + * Neo4j properties are scalars and flat arrays, and a version's content is neither — it is lists, + * a map of label sets, and a map of property *signature* sets. So all four structural fields are + * serialized to JSON strings. JSON also handles names containing pipes, tabs, newlines and quotes, + * which matters because these names come out of LLM extraction and routinely do. + * + * **The timestamp is informational, and is written twice anyway.** `savedAt` is the ISO-8601 string, + * which is what you want when you're looking at a node and wondering when it landed. `savedAtEpochMillis` + * is the same instant as a number, which is what you want when you're filtering or grouping by time + * in an ad-hoc query — the string is no use for that, since `Instant.toString()` drops the fraction + * entirely at a whole second and `'Z'` sorts above `'.'`, making `"…T00:00:00Z"` compare greater than + * `"…T00:00:00.500Z"`. + * + * Neither one orders the history. That's the `sequence` property's job — see + * [DrivineMetamodelVersionStore], which explains why a clock can't express write order. Nothing here + * writes or reads `sequence`: it's assigned by Cypher off a per-schema counter, and it is storage + * bookkeeping rather than part of the stamp, so it stays out of the strict round-trip below. + * + * **Reads are strict.** A property this mapper wrote must be present when it is read again. A node + * missing one is corrupt — not a version with a blank name — so the accessor throws and the store's + * surrounding guard skips the row with a warning instead of quietly materializing junk. + */ +object MetamodelVersionRowMapper { + + /** + * Bind values for a write — the natural key is (schemaName, contentHash). + * + * [savedAt] is passed in rather than read from the wall clock here, so this stays a pure + * function of its arguments and a test can pin the instant a version was stored at. + */ + fun bindMap(version: MetamodelVersion, savedAt: Instant): Map = mapOf( + "schemaName" to version.schemaName, + "contentHash" to version.contentHash, + "entityTypeNames" to serializeList(version.entityTypeNames), + "entityTypeLabels" to serializeMapOfLabelSets(version.entityTypeLabels), + "entityTypeProperties" to serializeMapOfSignatureSets(version.entityTypeProperties), + "relationshipNames" to serializeList(version.relationshipNames), + "savedAt" to savedAt.toString(), + "savedAtEpochMillis" to savedAt.toEpochMilli(), + ) + + /** + * Rebuild a [MetamodelVersion] from a returned node's property map, and check its integrity + * on the way. + * + * A version's content hash is derived from its structural fields, not stored alongside them as + * an independent value, so the reconstructed object computes its own hash. The `contentHash` + * property on the node is therefore a checksum rather than data: recomputing it and finding a + * different answer means the node was written by an older hash format, hand-edited, or + * corrupted. Either way it is not the version it claims to be, so this throws and the caller + * skips it. Note the stored hash is also half the natural key, so a mismatch would additionally + * mean a re-save of the same content lands on a *different* node. + */ + fun fromRow(row: Map<*, *>): MetamodelVersion { + val storedHash = row.str("contentHash") + val version = MetamodelVersion( + schemaName = row.str("schemaName"), + entityTypeNames = deserializeList(row.str("entityTypeNames")), + entityTypeLabels = deserializeMapOfLabelSets(row.str("entityTypeLabels")), + entityTypeProperties = deserializeMapOfSignatureSets(row.str("entityTypeProperties")), + relationshipNames = deserializeList(row.str("relationshipNames")), + ) + require(version.contentHash == storedHash) { + "MetamodelVersion '${version.schemaName}' fails its integrity check: stored contentHash " + + "$storedHash, but the persisted structural fields hash to ${version.contentHash}" + } + return version + } +} + +// Serialization helpers: JSON for escape-safe round-trip encoding. + +/** Serialize a list to a JSON string. */ +private fun serializeList(items: List): String = + objectMapper.writeValueAsString(items) + +/** Deserialize a JSON string back to a list. */ +private fun deserializeList(serialized: String): List = + if (serialized.isEmpty()) emptyList() + else objectMapper.readValue( + serialized, + objectMapper.typeFactory.constructCollectionType(List::class.java, String::class.java) + ) + +/** + * Serialize the per-type label sets as `{"Person": ["Agent", "Entity"], ...}`. + * + * Sets have no order, so they're written sorted. Nothing reads the order back — the sets go into a + * `Set` again — but a deterministic encoding means re-saving the same version writes byte-identical + * JSON, which keeps an idempotent MERGE genuinely a no-op and makes a stored node diffable by hand. + */ +private fun serializeMapOfLabelSets(map: Map>): String = + objectMapper.writeValueAsString(map.toSortedMap().mapValues { (_, labels) -> labels.sorted() }) + +/** Inverse of [serializeMapOfLabelSets]. */ +private fun deserializeMapOfLabelSets(serialized: String): Map> { + if (serialized.isEmpty()) return emptyMap() + @Suppress("UNCHECKED_CAST") + val mapOfLists = objectMapper.readValue( + serialized, + objectMapper.typeFactory.constructMapType(Map::class.java, String::class.java, List::class.java), + ) as Map> + return mapOfLists.mapValues { (_, labels) -> labels.toSet() } +} + +/** + * Serialize the per-type property signatures as a JSON object of arrays of four-field objects: + * + * ```json + * {"Person": [{"name": "age", "kind": "VALUE", "type": "integer", "cardinality": "ONE"}]} + * ``` + * + * Written out field by field rather than handed to Jackson's bean serializer. The shape on disk is + * a persisted format — it feeds the version's own content hash on the way back in — so it is spelled + * out here where you can see it, and doesn't move because someone renames a Kotlin property or + * because `jackson-module-kotlin` is or isn't on the classpath. + * + * Enums are stored by `name`, not ordinal. An ordinal would silently re-point at a different + * constant the moment someone inserts a value into [Cardinality] or [PropertySignature.Kind]. + */ +private fun serializeMapOfSignatureSets(map: Map>): String = + objectMapper.writeValueAsString( + map.toSortedMap().mapValues { (_, signatures) -> + signatures.sorted().map { signature -> + // A LinkedHashMap, so the keys land in this order in the JSON and the encoding is + // fully determined by the content. + linkedMapOf( + "name" to signature.name, + "kind" to signature.kind.name, + "type" to signature.type, + "cardinality" to signature.cardinality.name, + ) + } + } + ) + +/** + * Inverse of [serializeMapOfSignatureSets], and strict about it: a signature object missing a field, + * or naming an enum constant this build doesn't have, throws rather than being patched up with a + * default. A guessed default would change the structural content, and the version's integrity check + * would then reject the whole row anyway — with a confusing message about a hash mismatch instead of + * the real problem. + */ +private fun deserializeMapOfSignatureSets(serialized: String): Map> { + if (serialized.isEmpty()) return emptyMap() + @Suppress("UNCHECKED_CAST") + val mapOfLists = objectMapper.readValue( + serialized, + objectMapper.typeFactory.constructMapType(Map::class.java, String::class.java, List::class.java), + ) as Map> + return mapOfLists.mapValues { (typeName, encoded) -> + encoded.map { element -> + val fields = element as? Map<*, *> + ?: throw IllegalArgumentException( + "entityTypeProperties for '$typeName' holds ${element?.javaClass?.simpleName ?: "null"} " + + "where a property signature object was expected" + ) + PropertySignature( + name = fields.signatureField(typeName, "name"), + kind = enumConstant(fields.signatureField(typeName, "kind"), typeName, "kind"), + type = fields.signatureField(typeName, "type"), + cardinality = enumConstant(fields.signatureField(typeName, "cardinality"), typeName, "cardinality"), + ) + }.toSet() + } +} + +/** Read one field of a stored property signature, blowing up by name if it isn't there. */ +private fun Map<*, *>.signatureField(typeName: String, field: String): String = + this[field]?.toString() ?: throw IllegalArgumentException( + "a property signature for '$typeName' is missing its '$field' field" + ) + +/** Turn a stored enum constant name back into the constant, naming what failed if it's unknown. */ +private inline fun > enumConstant(stored: String, typeName: String, field: String): E = + enumValues().firstOrNull { it.name == stored } ?: throw IllegalArgumentException( + "a property signature for '$typeName' has '$field' = '$stored', which is not a known " + + "${E::class.simpleName} — the node was written by a different version of the schema model" + ) + +/** + * Read a property that must be there, and blow up if it isn't. + * + * Returning `""` for an absent property would be the friendlier-looking choice and is precisely + * the wrong one: a node missing `schemaName` would come back as a real-looking version named `""`, + * indistinguishable from data, and the caller's "skip the unreadable row" guard would never fire + * for the most likely kind of corruption there is. Throwing is what makes that guard mean + * something. + */ +private fun Map<*, *>.str(key: String): String = + this[key]?.toString() ?: throw IllegalArgumentException("required property '$key' is missing from the stored node") diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineCollectorTraceStoreIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineCollectorTraceStoreIntegrationTest.kt index bdcc8fda..7cb03cab 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineCollectorTraceStoreIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineCollectorTraceStoreIntegrationTest.kt @@ -40,14 +40,25 @@ import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest +import org.springframework.test.context.DynamicPropertyRegistry +import org.springframework.test.context.DynamicPropertySource /** - * Integration tests for [DrivineCollectorTraceStore] against a Neo4j testcontainer (provided by - * Drivine's test support). Each test starts from an empty graph via [cleanUp]. + * Integration tests for [DrivineCollectorTraceStore] against a Neo4j testcontainer. Each test + * starts from an empty graph via [cleanUp]. + * + * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class + * for why. */ @SpringBootTest(classes = [TestApplication::class]) class DrivineCollectorTraceStoreIntegrationTest { + companion object { + @JvmStatic + @DynamicPropertySource + fun neo4jProperties(registry: DynamicPropertyRegistry) = Neo4jTestContainer.registerProperties(registry) + } + @Autowired private lateinit var traceStore: DrivineCollectorTraceStore diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineGraphQueryParityIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineGraphQueryParityIntegrationTest.kt index 59a82aaf..6f195dcc 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineGraphQueryParityIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineGraphQueryParityIntegrationTest.kt @@ -40,6 +40,8 @@ import org.junit.jupiter.api.Test import org.drivine.manager.PersistenceManager import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest +import org.springframework.test.context.DynamicPropertyRegistry +import org.springframework.test.context.DynamicPropertySource import java.time.Instant /** @@ -52,10 +54,19 @@ import java.time.Instant * exists (which propositions land on a `via` or on a shortest path when parallel edges / ties are * present), we assert each returned edge is *valid* rather than object-identical — both engines are * free to pick a different but correct edge. + * + * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class + * for why. */ @SpringBootTest(classes = [TestApplication::class]) class DrivineGraphQueryParityIntegrationTest { + companion object { + @JvmStatic + @DynamicPropertySource + fun neo4jProperties(registry: DynamicPropertyRegistry) = Neo4jTestContainer.registerProperties(registry) + } + @Autowired private lateinit var repository: DrivinePropositionRepository diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineLineageRecordStoreIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineLineageRecordStoreIntegrationTest.kt index 9906a7eb..0f6802b3 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineLineageRecordStoreIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineLineageRecordStoreIntegrationTest.kt @@ -31,15 +31,26 @@ import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest +import org.springframework.test.context.DynamicPropertyRegistry +import org.springframework.test.context.DynamicPropertySource import java.time.Instant /** * Integration tests for the durable lineage stores against a Neo4j testcontainer (provided by * Drivine's test support). Each test starts from an empty graph via [cleanUp]. + * + * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class + * for why. */ @SpringBootTest(classes = [TestApplication::class]) class DrivineLineageRecordStoreIntegrationTest { + companion object { + @JvmStatic + @DynamicPropertySource + fun neo4jProperties(registry: DynamicPropertyRegistry) = Neo4jTestContainer.registerProperties(registry) + } + @Autowired private lateinit var projectionStore: DrivineProjectionRecordStore diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt new file mode 100644 index 00000000..eb7deace --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt @@ -0,0 +1,673 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.storage + +import ch.qos.logback.classic.Level +import ch.qos.logback.classic.Logger +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.core.read.ListAppender +import com.embabel.agent.core.Cardinality +import com.embabel.dice.metamodel.MetamodelVersion +import com.embabel.dice.metamodel.PropertySignature +import com.embabel.dice.metamodel.PropertySignature.Kind +import org.drivine.manager.PersistenceManager +import org.drivine.query.QuerySpecification +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNotEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.slf4j.LoggerFactory +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.test.context.DynamicPropertyRegistry +import org.springframework.test.context.DynamicPropertySource +import java.time.Instant +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger + +/** + * Integration tests for [DrivineMetamodelVersionStore] against a Neo4j testcontainer. Each test + * starts from an empty graph via [cleanUp]. + * + * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class + * for why. + * + * Note what's *absent* from every `MetamodelVersion` built here: a content hash. It's derived from + * the structural fields, so two versions can't claim the same identity while describing different + * schemas -- which is why the tests that need two distinct versions of one schema give them + * genuinely different content rather than different hand-written hash strings. + */ +@SpringBootTest(classes = [TestApplication::class]) +class DrivineMetamodelVersionStoreIntegrationTest { + + companion object { + @JvmStatic + @DynamicPropertySource + fun neo4jProperties(registry: DynamicPropertyRegistry) = Neo4jTestContainer.registerProperties(registry) + } + + @Autowired + private lateinit var store: DrivineMetamodelVersionStore + + @Autowired + private lateinit var persistenceManager: PersistenceManager + + @Autowired + private lateinit var clock: PinnableClock + + @AfterEach + fun cleanUp() { + clock.unpin() + listOf("MetamodelVersion", "MetamodelSchemaCounter").forEach { label -> + persistenceManager.execute(QuerySpecification.withStatement("MATCH (n:$label) DETACH DELETE n")) + } + } + + // ---- CRUD ---- + + @Test + fun `a version persists and reads back every field`() { + val version = MetamodelVersion( + schemaName = "test-schema", + entityTypeNames = listOf("Person", "Company", "Location"), + entityTypeLabels = mapOf( + "Person" to setOf("Agent", "Entity"), + "Company" to setOf("Organization", "Entity"), + ), + entityTypeProperties = mapOf( + "Person" to setOf( + PropertySignature("name", Kind.VALUE, "string", Cardinality.ONE), + PropertySignature("age", Kind.VALUE, "integer", Cardinality.OPTIONAL), + ), + "Company" to setOf( + PropertySignature("name", Kind.VALUE, "string", Cardinality.ONE), + PropertySignature("employs", Kind.REFERENCE, "Person", Cardinality.SET), + ), + ), + relationshipNames = listOf("WORKS_FOR", "LOCATED_IN"), + ) + + store.saveVersion(version) + + val reloaded = store.latestVersion("test-schema") + assertEquals(version, reloaded) + assertEquals(version.contentHash, reloaded!!.contentHash) + } + + @Test + fun `version history returns empty for unknown schema`() { + assertEquals(emptyList(), store.versionHistory("unknown-schema")) + assertNull(store.latestVersion("unknown-schema")) + } + + @Test + fun `each schema sees only its own versions`() { + val schemaA = MetamodelVersion("schema-a", listOf("TypeA"), emptyMap(), emptyMap(), emptyList()) + val schemaB = MetamodelVersion("schema-b", listOf("TypeB"), emptyMap(), emptyMap(), emptyList()) + + store.saveVersion(schemaA) + store.saveVersion(schemaB) + + assertEquals(schemaA, store.latestVersion("schema-a")) + assertEquals(schemaB, store.latestVersion("schema-b")) + assertEquals(listOf(schemaA), store.versionHistory("schema-a")) + } + + // ---- Property signatures round-trip ---- + + @Test + fun `a property signature round-trips its kind, type and cardinality, not just its name`() { + // The whole reason entityTypeProperties holds signatures rather than bare names: turning a + // single `age` string into a list of integers is a real schema change. If the store dropped + // kind/type/cardinality on the way to disk, the reloaded stamp would hash differently from + // the one that was saved -- and the mapper's integrity check would reject its own write. + val everyShape = setOf( + PropertySignature("optionalString", Kind.VALUE, "string", Cardinality.OPTIONAL), + PropertySignature("oneInteger", Kind.VALUE, "integer", Cardinality.ONE), + PropertySignature("listOfDates", Kind.VALUE, "date", Cardinality.LIST), + PropertySignature("setOfCompanies", Kind.REFERENCE, "Company", Cardinality.SET), + PropertySignature("mystery", Kind.UNKNOWN, "", Cardinality.ONE), + ) + val version = MetamodelVersion( + schemaName = "signature-schema", + entityTypeNames = listOf("Person"), + entityTypeLabels = emptyMap(), + entityTypeProperties = mapOf("Person" to everyShape), + relationshipNames = emptyList(), + ) + + store.saveVersion(version) + + val reloaded = store.latestVersion("signature-schema")!! + assertEquals(everyShape, reloaded.entityTypeProperties["Person"]) + assertEquals(version.contentHash, reloaded.contentHash) + } + + @Test + fun `two versions differing only in one property's cardinality are two stored versions`() { + // Same type name, same property name -- the change is invisible to any encoding that stores + // property *names*. It has to survive as two nodes with two hashes, or the store has + // silently lost a schema change. + val schemaName = "cardinality-change-schema" + fun withCardinality(cardinality: Cardinality) = MetamodelVersion( + schemaName = schemaName, + entityTypeNames = listOf("Person"), + entityTypeLabels = emptyMap(), + entityTypeProperties = mapOf("Person" to setOf(PropertySignature("nickname", Kind.VALUE, "string", cardinality))), + relationshipNames = emptyList(), + ) + val one = withCardinality(Cardinality.ONE) + val many = withCardinality(Cardinality.LIST) + assertNotEquals(one.contentHash, many.contentHash, "precondition: the two stamps must differ") + + store.saveVersion(one) + store.saveVersion(many) + + assertEquals(setOf(one, many), store.versionHistory(schemaName).toSet()) + assertEquals(Cardinality.ONE, store.findVersion(schemaName, one.contentHash)!!.entityTypeProperties["Person"]!!.single().cardinality) + assertEquals(Cardinality.LIST, store.findVersion(schemaName, many.contentHash)!!.entityTypeProperties["Person"]!!.single().cardinality) + } + + // ---- findVersion ---- + + @Test + fun `findVersion resolves a recorded hash back to the stamp it named`() { + val schemaName = "find-schema" + val v1 = MetamodelVersion(schemaName, listOf("Type1"), emptyMap(), emptyMap(), emptyList()) + val v2 = MetamodelVersion(schemaName, listOf("Type2"), emptyMap(), emptyMap(), emptyList()) + store.saveVersion(v1) + store.saveVersion(v2) + + assertEquals(v1, store.findVersion(schemaName, v1.contentHash)) + assertEquals(v2, store.findVersion(schemaName, v2.contentHash)) + } + + @Test + fun `findVersion is null for an unknown hash, and keyed on the schema name too`() { + val v = MetamodelVersion("find-null-schema", listOf("Type1"), emptyMap(), emptyMap(), emptyList()) + store.saveVersion(v) + + assertNull(store.findVersion("find-null-schema", "no-such-hash")) + // The hash is real, but it belongs to another schema: the natural key is the pair. + assertNull(store.findVersion("some-other-schema", v.contentHash)) + } + + @Test + fun `findVersion applies the same integrity check as the history read`() { + val schemaName = "find-tampered-schema" + val version = MetamodelVersion(schemaName, listOf("Original"), emptyMap(), emptyMap(), emptyList()) + store.saveVersion(version) + tamperWithEntityTypeNames(schemaName, """["SwappedInBehindTheHash"]""") + + val (found, logged) = capturingStoreWarnings { store.findVersion(schemaName, version.contentHash) } + + assertNull(found, "a keyed lookup must not hand back a node that fails its own checksum") + assertTrue(logged.any { it.contains("fails its integrity check") }, "warnings were: $logged") + } + + // ---- Natural-key idempotency ---- + + @Test + fun `saving the same version twice leaves one node, not two`() { + val version = MetamodelVersion( + schemaName = "idempotent-schema", + entityTypeNames = listOf("TypeA"), + entityTypeLabels = mapOf("TypeA" to setOf("LabelA")), + entityTypeProperties = mapOf("TypeA" to setOf(PropertySignature("prop", Kind.VALUE, "string", Cardinality.ONE))), + relationshipNames = listOf("REL"), + ) + + store.saveVersion(version) + store.saveVersion(version) + + val history = store.versionHistory("idempotent-schema") + assertEquals(1, history.size) + assertEquals(version, history.single()) + } + + @Test + fun `re-saving an old version neither bumps the counter nor moves it in the history`() { + val schemaName = "history-order-schema" + val v1 = MetamodelVersion(schemaName, listOf("Type1"), emptyMap(), emptyMap(), emptyList()) + val v2 = MetamodelVersion(schemaName, listOf("Type2"), emptyMap(), emptyMap(), emptyList()) + + clock.pin(Instant.parse("2026-01-01T00:00:00Z")) + store.saveVersion(v1) + clock.pin(Instant.parse("2026-01-02T00:00:00Z")) + store.saveVersion(v2) + assertEquals(v2, store.latestVersion(schemaName)) + assertEquals(1L, storedSequence(schemaName, v1)) + assertEquals(2L, storedSequence(schemaName, v2)) + + // Re-stamp the old one much later. This is the idempotent path: it must refresh v1's + // content and nothing else -- not its sequence, and not the counter, or the next genuinely + // new version would skip a number. + clock.pin(Instant.parse("2026-06-01T00:00:00Z")) + store.saveVersion(v1) + + assertEquals(v2, store.latestVersion(schemaName), "re-saving v1 must not make it the latest") + assertEquals(listOf(v2, v1), store.versionHistory(schemaName)) + assertEquals(1L, storedSequence(schemaName, v1), "v1 must keep the position it has always had") + assertEquals(2L, counterValue(schemaName), "an idempotent re-save must not consume a sequence number") + } + + @Test + fun `many threads saving the identical version leave exactly one node`() { + // The write is a MERGE, which is only race-free under a uniqueness constraint on the key it + // merges on -- see TestApplication.metamodelSchema. Without one, concurrent MERGEs all miss, + // all take the CREATE branch, and the "history" fills with duplicates of one version. + val threads = 12 + val version = MetamodelVersion( + schemaName = "concurrent-schema", + entityTypeNames = listOf("Contended"), + entityTypeLabels = mapOf("Contended" to setOf("LabelC")), + entityTypeProperties = mapOf("Contended" to setOf(PropertySignature("prop", Kind.VALUE, "string", Cardinality.ONE))), + relationshipNames = listOf("REL"), + ) + + val startTogether = CountDownLatch(1) + val succeeded = AtomicInteger() + val failures = mutableListOf() + val pool = Executors.newFixedThreadPool(threads) + try { + repeat(threads) { + pool.submit { + startTogether.await() + // A loser in a MERGE race can surface a constraint violation or a lock timeout. + // That's tolerable -- a caller retries. Two surviving nodes are not. + runCatching { store.saveVersion(version) } + .onSuccess { succeeded.incrementAndGet() } + .onFailure { t -> synchronized(failures) { failures += t } } + } + } + startTogether.countDown() + pool.shutdown() + assertTrue(pool.awaitTermination(60, TimeUnit.SECONDS), "concurrent saves did not finish in time") + } finally { + pool.shutdownNow() + } + + assertTrue(succeeded.get() > 0, "every concurrent save failed: ${failures.firstOrNull()}") + val history = store.versionHistory("concurrent-schema") + assertEquals( + 1, + history.size, + "$threads concurrent saves of one version must leave one node, not ${history.size} " + + "(${succeeded.get()} succeeded, ${failures.size} failed)", + ) + assertEquals(version, history.single()) + // The sequence is assigned in the same transaction as the MERGE that creates the node, and + // only on create -- so the losing threads, which matched an existing node, took no number. + assertEquals(1L, storedSequence("concurrent-schema", version), "the one node must hold the first sequence") + assertEquals(1L, counterValue("concurrent-schema"), "only the creating save may consume a number") + } + + @Test + fun `concurrent saves of distinct versions each get their own place in the order`() { + // The lost-update test. Every thread here creates a *different* version of one schema, so + // all of them hit the counter at the same moment. If the increment lost an update, two + // versions would claim one position and "newest first" would be arbitrary again for the + // pair -- the very bug the sequence was introduced to fix. + val schemaName = "concurrent-distinct-schema" + val threads = 12 + val versions = (1..threads).map { + MetamodelVersion(schemaName, listOf("Type$it"), emptyMap(), emptyMap(), emptyList()) + } + + val startTogether = CountDownLatch(1) + val failures = mutableListOf() + val pool = Executors.newFixedThreadPool(threads) + try { + versions.forEach { version -> + pool.submit { + startTogether.await() + runCatching { store.saveVersion(version) } + .onFailure { t -> synchronized(failures) { failures += t } } + } + } + startTogether.countDown() + pool.shutdown() + assertTrue(pool.awaitTermination(60, TimeUnit.SECONDS), "concurrent saves did not finish in time") + } finally { + pool.shutdownNow() + } + assertTrue(failures.isEmpty(), "distinct versions must not contend for the same node: ${failures.firstOrNull()}") + + val sequences = versions.map { storedSequence(schemaName, it) } + assertEquals( + (1L..threads.toLong()).toSet(), + sequences.toSet(), + "each version must hold its own sequence; got $sequences", + ) + // And the order the store reports has to be a total order over all of them, not a heap with + // ties in it. + assertEquals(threads, store.versionHistory(schemaName).size) + assertEquals(threads.toLong(), counterValue(schemaName)) + } + + @Test + fun `two versions of one schema cannot be stored at the same position`() { + // The safety net under the sequence. The counter increment appears to serialise correctly on + // its own -- removing the lock from the save statement doesn't make the test above fail, even + // at four times the contention -- so "the increment is atomic" is an observation, not a proof. + // This constraint is the proof: whatever the counter does, the database will not hold two + // versions of one schema claiming one place in the write order. A lost update becomes a + // retryable failure rather than a silently scrambled history. + val schemaName = "position-constraint-schema" + val first = MetamodelVersion(schemaName, listOf("First"), emptyMap(), emptyMap(), emptyList()) + store.saveVersion(first) + assertEquals(1L, storedSequence(schemaName, first)) + + val collision = runCatching { + persistenceManager.execute( + QuerySpecification.withStatement( + """ + CREATE (n:MetamodelVersion { + schemaName: ${'$'}schemaName, contentHash: 'a-different-hash', sequence: 1 + }) + """.trimIndent(), + ).bind(mapOf("schemaName" to schemaName)), + ) + } + + assertTrue( + collision.isFailure, + "the database must refuse a second version at position 1; a lost counter update has to be loud", + ) + assertEquals(listOf(first), store.versionHistory(schemaName), "and the history is untouched") + } + + // ---- Chronological ordering ---- + + @Test + fun `version history is newest-first`() { + val schemaName = "ordering-schema" + val v1 = MetamodelVersion(schemaName, listOf("Type1"), emptyMap(), emptyMap(), emptyList()) + val v2 = MetamodelVersion(schemaName, listOf("Type2"), emptyMap(), emptyMap(), emptyList()) + + store.saveVersion(v1) + store.saveVersion(v2) + + assertEquals(listOf(v2, v1), store.versionHistory(schemaName)) + assertEquals(v2, store.latestVersion(schemaName)) + } + + @Test + fun `two versions saved in the very same millisecond still order by write order`() { + // No sleep, and the clock is pinned to one instant for both saves, so every timestamp on + // both nodes is byte-identical. A clock -- at any precision -- simply cannot separate these + // two; only a counter can. This is the ordinary case, not an exotic one: back-to-back saves + // land in the same millisecond routinely. + val schemaName = "same-millisecond-schema" + val first = MetamodelVersion(schemaName, listOf("First"), emptyMap(), emptyMap(), emptyList()) + val second = MetamodelVersion(schemaName, listOf("Second"), emptyMap(), emptyMap(), emptyList()) + + clock.pin(Instant.parse("2026-01-01T00:00:00Z")) + store.saveVersion(first) + store.saveVersion(second) + + assertEquals( + listOf(second, first), + store.versionHistory(schemaName), + "identical timestamps must not make the order arbitrary", + ) + assertEquals(second, store.latestVersion(schemaName)) + assertEquals(listOf(1L, 2L), listOf(storedSequence(schemaName, first), storedSequence(schemaName, second))) + } + + @Test + fun `write order survives a clock that runs backwards`() { + // An NTP correction, or a failover to a node with a different skew, can move the wall clock + // *backwards* between two saves. Ordering on any timestamp then reports the older stamp as + // the newest -- a silent wrong answer, which is worse than a slow one. The sequence is + // monotonic regardless of what the clock is doing. + val schemaName = "clock-skew-schema" + val earlier = MetamodelVersion(schemaName, listOf("WrittenFirst"), emptyMap(), emptyMap(), emptyList()) + val later = MetamodelVersion(schemaName, listOf("WrittenSecond"), emptyMap(), emptyMap(), emptyList()) + + clock.pin(Instant.parse("2026-01-01T12:00:00Z")) + store.saveVersion(earlier) + clock.pin(Instant.parse("2026-01-01T11:00:00Z")) // an hour backwards + store.saveVersion(later) + + assertEquals(later, store.latestVersion(schemaName), "the last write must be the latest, whatever the clock says") + assertEquals(listOf(later, earlier), store.versionHistory(schemaName)) + } + + // ---- Corrupt rows are skipped, not materialized ---- + + @Test + fun `a version node missing a required property is skipped and warned about, not read as a blank version`() { + // Written straight through Cypher, so the node exists exactly as a partially-failed write or + // a hand-edit would leave it: one required property simply absent. `entityTypeNames` rather + // than `schemaName` because every read MATCHes on schemaName -- a node without one is + // filtered out by the query and never reaches the mapper at all. + val schemaName = "corrupt-row-schema" + val good = MetamodelVersion(schemaName, listOf("Sound"), emptyMap(), emptyMap(), emptyList()) + store.saveVersion(good) + // Spelled out property by property rather than copied from the good node, so what's wrong + // with it is visible: every property the mapper writes except `entityTypeNames`. + val brokenSavedAt = Instant.parse("2026-01-01T00:00:00Z") + persistenceManager.execute( + QuerySpecification.withStatement( + """ + CREATE (broken:MetamodelVersion { + schemaName: ${'$'}schemaName, + contentHash: ${'$'}contentHash, + entityTypeLabels: '{}', + entityTypeProperties: '{}', + relationshipNames: '[]', + savedAt: ${'$'}savedAt, + savedAtEpochMillis: ${'$'}savedAtEpochMillis + }) + """.trimIndent(), + ).bind( + mapOf( + "schemaName" to schemaName, + // Distinct from the good node's, so the (schemaName, contentHash) uniqueness + // constraint lets both nodes exist. + "contentHash" to "a-different-hash-so-the-natural-key-does-not-collide", + "savedAt" to brokenSavedAt.toString(), + "savedAtEpochMillis" to brokenSavedAt.toEpochMilli(), + ), + ), + ) + assertEquals(2, rawNodeCount(), "the corrupt node must really be in the graph") + + val (history, logged) = capturingStoreWarnings { store.versionHistory(schemaName) } + + assertEquals(listOf(good), history, "the readable version survives; the corrupt one is dropped") + assertTrue( + logged.any { it.contains("Skipping unreadable MetamodelVersion row") && it.contains("entityTypeNames") }, + "the skip must be warned about and name the missing property; warnings were: $logged", + ) + } + + @Test + fun `a stored property signature missing a field is skipped and warned about by name`() { + // The signature encoding is a persisted format of its own, so a node can be structurally + // fine and still hold a half-written signature -- an older writer, a hand-edit. Guessing a + // default cardinality would change the content and surface later as a baffling hash + // mismatch, so the mapper names the missing field instead. + val schemaName = "corrupt-signature-schema" + val version = MetamodelVersion( + schemaName = schemaName, + entityTypeNames = listOf("Person"), + entityTypeLabels = emptyMap(), + entityTypeProperties = mapOf("Person" to setOf(PropertySignature("age", Kind.VALUE, "integer", Cardinality.ONE))), + relationshipNames = emptyList(), + ) + store.saveVersion(version) + persistenceManager.execute( + QuerySpecification.withStatement( + """ + MATCH (n:MetamodelVersion {schemaName: ${'$'}schemaName}) + SET n.entityTypeProperties = ${'$'}halfWritten + """.trimIndent(), + ).bind( + mapOf( + "schemaName" to schemaName, + "halfWritten" to """{"Person":[{"name":"age","kind":"VALUE","type":"integer"}]}""", + ), + ), + ) + + val (history, logged) = capturingStoreWarnings { store.versionHistory(schemaName) } + + assertEquals(emptyList(), history) + assertTrue( + logged.any { it.contains("missing its 'cardinality' field") }, + "the warning must name the missing signature field; warnings were: $logged", + ) + } + + @Test + fun `a version node whose stored hash disagrees with its stored fields is skipped and warned about`() { + // The content hash is derived from the structural fields, so the copy on the node is a + // checksum rather than data. Disagreement means the node was written by an older hash format + // or tampered with -- either way it is not the version it claims to be. + val schemaName = "tampered-hash-schema" + val version = MetamodelVersion(schemaName, listOf("Original"), emptyMap(), emptyMap(), emptyList()) + store.saveVersion(version) + tamperWithEntityTypeNames(schemaName, """["SwappedInBehindTheHash"]""") + + val (history, logged) = capturingStoreWarnings { store.versionHistory(schemaName) } + + assertEquals(emptyList(), history) + assertTrue(logged.any { it.contains("fails its integrity check") }, "warnings were: $logged") + assertNull(store.latestVersion(schemaName), "and it must not come back as the latest version either") + } + + @Test + fun `a corrupt newest node hides only itself, not the readable version behind it`() { + // latestVersion deliberately sorts in Cypher but takes the first *readable* row rather than + // pushing LIMIT 1 down. With the limit in the query, a corrupt newest node would make the + // store answer "no versions at all" while versionHistory still returned the older one -- + // two reads disagreeing about the same graph. + val schemaName = "corrupt-head-schema" + val readable = MetamodelVersion(schemaName, listOf("Readable"), emptyMap(), emptyMap(), emptyList()) + val doomed = MetamodelVersion(schemaName, listOf("Doomed"), emptyMap(), emptyMap(), emptyList()) + + clock.pin(Instant.parse("2026-01-01T00:00:00Z")) + store.saveVersion(readable) + clock.pin(Instant.parse("2026-01-02T00:00:00Z")) + store.saveVersion(doomed) + // Tamper with the newer node only, keyed on its own hash. + persistenceManager.execute( + QuerySpecification.withStatement( + """ + MATCH (n:MetamodelVersion {schemaName: ${'$'}schemaName, contentHash: ${'$'}contentHash}) + SET n.entityTypeNames = '["NotWhatTheHashSays"]' + """.trimIndent(), + ).bind(mapOf("schemaName" to schemaName, "contentHash" to doomed.contentHash)), + ) + + assertEquals(readable, store.latestVersion(schemaName)) + assertEquals(listOf(readable), store.versionHistory(schemaName)) + } + + // ---- Adversarial serialization: names carrying delimiter characters ---- + + @Test + fun `names containing delimiter characters survive the round-trip intact`() { + // The old encoding joined on delimiters; this one is JSON, and these are the characters that + // would break a joined one. Entity type names, labels, property names, property types and + // relationship names all go through it, so all five carry a delimiter here. + listOf("|" to "pipe", "\t" to "tab", "\n" to "newline", "\"" to "quote", "\\" to "backslash") + .forEach { (delimiter, label) -> + val schemaName = "delimiter-$label" + val typeName = "Type${delimiter}WithIt" + val version = MetamodelVersion( + schemaName = schemaName, + entityTypeNames = listOf(typeName, "Normal"), + entityTypeLabels = mapOf(typeName to setOf("Label${delimiter}1", "Label2")), + entityTypeProperties = mapOf( + typeName to setOf( + PropertySignature("prop${delimiter}1", Kind.VALUE, "string${delimiter}ish", Cardinality.ONE), + ), + ), + relationshipNames = listOf("REL${delimiter}WITH${delimiter}IT"), + ) + + store.saveVersion(version) + + assertEquals(version, store.latestVersion(schemaName), "'$label' did not survive the round-trip") + } + } + + // ---- helpers ---- + + /** Rewrite a version node's serialized entity type names, leaving its stored hash untouched. */ + private fun tamperWithEntityTypeNames(schemaName: String, serializedNames: String) { + persistenceManager.execute( + QuerySpecification.withStatement( + """ + MATCH (n:MetamodelVersion {schemaName: ${'$'}schemaName}) + SET n.entityTypeNames = ${'$'}serializedNames + """.trimIndent(), + ).bind(mapOf("schemaName" to schemaName, "serializedNames" to serializedNames)), + ) + } + + /** + * The `sequence` a version node actually holds. Read straight out of the graph because the + * sequence is storage bookkeeping — it is deliberately not on [MetamodelVersion], so the only + * honest way to assert about it is to go and look. + */ + private fun storedSequence(schemaName: String, version: MetamodelVersion): Long? = + persistenceManager.maybeGetOne( + QuerySpecification.withStatement( + """ + MATCH (n:MetamodelVersion {schemaName: ${'$'}schemaName, contentHash: ${'$'}contentHash}) + RETURN n.sequence AS sequence + """.trimIndent(), + ).bind(mapOf("schemaName" to schemaName, "contentHash" to version.contentHash)) + .transform(Long::class.java), + ) + + /** How far the schema's counter has been advanced. */ + private fun counterValue(schemaName: String): Long? = persistenceManager.maybeGetOne( + QuerySpecification.withStatement( + "MATCH (c:MetamodelSchemaCounter {schemaName: ${'$'}schemaName}) RETURN c.sequence AS sequence", + ).bind(mapOf("schemaName" to schemaName)).transform(Long::class.java), + ) + + /** Count version nodes without going through the store's mapper. */ + private fun rawNodeCount(): Int = persistenceManager.maybeGetOne( + QuerySpecification.withStatement("MATCH (n:MetamodelVersion) RETURN count(n) AS c").transform(Long::class.java), + )?.toInt() ?: 0 + + /** + * Run [block] with a listener attached to the store's logger, and hand back both its result and + * every WARN message the store emitted. Needed because "skips the row" and "skips the row *and + * says so*" are different behaviours, and only the second is any use to an operator. + */ + private fun capturingStoreWarnings(block: () -> T): Pair> { + val logger = LoggerFactory.getLogger(DrivineMetamodelVersionStore::class.java) as Logger + val appender = ListAppender().apply { start() } + logger.addAppender(appender) + return try { + block() to appender.list.filter { it.level == Level.WARN }.map { it.formattedMessage } + } finally { + logger.detachAppender(appender) + appender.stop() + } + } +} diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreContractIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreContractIntegrationTest.kt index ec2b17bf..3ef7d04e 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreContractIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreContractIntegrationTest.kt @@ -21,15 +21,26 @@ import org.drivine.query.QuerySpecification import org.junit.jupiter.api.AfterEach import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest +import org.springframework.test.context.DynamicPropertyRegistry +import org.springframework.test.context.DynamicPropertySource /** * Runs the [AbstractPropositionStoreContractTest] suite against the Neo4j-backed * [DrivinePropositionRepository] (testcontainer). This is the half that catches a graph backend * silently disagreeing with the in-memory contract — substitutability enforced, not assumed. + * + * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class + * for why. */ @SpringBootTest(classes = [TestApplication::class]) class DrivinePropositionStoreContractIntegrationTest : AbstractPropositionStoreContractTest() { + companion object { + @JvmStatic + @DynamicPropertySource + fun neo4jProperties(registry: DynamicPropertyRegistry) = Neo4jTestContainer.registerProperties(registry) + } + @Autowired private lateinit var repository: DrivinePropositionRepository diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreIntegrationTest.kt index 4b89a448..79d3f4c8 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreIntegrationTest.kt @@ -43,18 +43,29 @@ import org.drivine.manager.PersistenceManager import org.drivine.query.QuerySpecification import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest +import org.springframework.test.context.DynamicPropertyRegistry +import org.springframework.test.context.DynamicPropertySource import org.springframework.transaction.PlatformTransactionManager import java.time.Duration import java.time.Instant /** - * Integration tests for the graph storage stack against a Neo4j testcontainer (provided by Drivine's - * test support). Not `@Transactional`: dedup commits via its own [org.springframework.transaction.support.TransactionTemplate], - * so isolation is by explicit `clearAll()` per test rather than rollback. + * Integration tests for the graph storage stack against a Neo4j testcontainer. Not `@Transactional`: + * dedup commits via its own [org.springframework.transaction.support.TransactionTemplate], so + * isolation is by explicit `clearAll()` per test rather than rollback. + * + * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class + * for why. */ @SpringBootTest(classes = [TestApplication::class]) class DrivinePropositionStoreIntegrationTest { + companion object { + @JvmStatic + @DynamicPropertySource + fun neo4jProperties(registry: DynamicPropertyRegistry) = Neo4jTestContainer.registerProperties(registry) + } + @Autowired private lateinit var repository: DrivinePropositionRepository diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt new file mode 100644 index 00000000..930c0fe7 --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt @@ -0,0 +1,166 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.storage + +import com.embabel.agent.core.Cardinality +import com.embabel.dice.metamodel.MetamodelVersion +import com.embabel.dice.metamodel.PropertySignature +import com.embabel.dice.metamodel.PropertySignature.Kind +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import java.time.Instant + +/** + * Unit tests for [MetamodelVersionRowMapper] — no database, just the property map it produces and + * consumes. + * + * The point of most of these is that reading is *strict*. A stored node missing a property the + * mapper wrote is corrupt, and the mapper has to say so rather than quietly substituting a blank: + * the store wraps every read in "skip the unreadable row and warn", and that guard is only worth + * anything if unreadable rows actually throw. `DrivineMetamodelVersionStoreIntegrationTest` shows + * the guard firing end to end; these pin the mapper's half of the contract, including the cases the + * store's own `MATCH` filters out before they can reach it. + */ +class MetamodelRowMapperTest { + + private val version = MetamodelVersion( + schemaName = "test-schema", + entityTypeNames = listOf("Person", "Company"), + entityTypeLabels = mapOf("Person" to setOf("Agent"), "Company" to setOf("Org")), + entityTypeProperties = mapOf( + "Person" to setOf( + PropertySignature("name", Kind.VALUE, "string", Cardinality.ONE), + PropertySignature("age", Kind.VALUE, "integer", Cardinality.OPTIONAL), + ), + "Company" to setOf(PropertySignature("employs", Kind.REFERENCE, "Person", Cardinality.SET)), + ), + relationshipNames = listOf("WORKS_FOR"), + ) + + private val savedAt = Instant.parse("2026-01-01T00:00:00.500Z") + + private fun row(savedAtInstant: Instant = savedAt): MutableMap = + MetamodelVersionRowMapper.bindMap(version, savedAtInstant).toMutableMap() + + @Test + fun `a version round-trips through its own property map`() { + assertEquals(version, MetamodelVersionRowMapper.fromRow(row())) + } + + @Test + fun `property signatures are written as explicit named fields, enums by name, in a fixed order`() { + // The encoding on disk feeds the content hash on the way back in, so it is a persisted + // format and this is where its shape is pinned. Three things at once: + // - enum *names*, never ordinals -- an ordinal would silently re-point the moment someone + // inserts a constant into Cardinality; + // - map keys sorted (Company before Person, though Person was declared first); + // - signatures within a type sorted (age before name). + // The sorting is why re-saving an unchanged version writes byte-identical JSON. Without it + // the order would come from `java.util.Set.copyOf`, whose iteration order is deliberately + // randomised per JVM -- so the same stamp would encode differently after every restart. + assertEquals( + """{"Company":[{"name":"employs","kind":"REFERENCE","type":"Person","cardinality":"SET"}],""" + + """"Person":[{"name":"age","kind":"VALUE","type":"integer","cardinality":"OPTIONAL"},""" + + """{"name":"name","kind":"VALUE","type":"string","cardinality":"ONE"}]}""", + row()["entityTypeProperties"], + ) + assertEquals("""["Company","Person"]""", row()["entityTypeNames"]) + assertEquals("""{"Company":["Org"],"Person":["Agent"]}""", row()["entityTypeLabels"]) + } + + @Test + fun `bindMap stamps the instant it is given, not the wall clock`() { + assertEquals(savedAt.toString(), row()["savedAt"]) + assertEquals(savedAt.toEpochMilli(), row()["savedAtEpochMillis"]) + } + + @Test + fun `every timestamp gets a sortable numeric twin, because the ISO string is not sortable`() { + // Half a second apart, and the *older* one is the one with no fractional part. As strings + // the older sorts higher, because 'Z' outranks '.'; as numbers it doesn't. Anything that + // orders on the string will hand back the wrong row, which is why nothing does. + val older = row(Instant.parse("2026-01-01T00:00:00Z")) + val newer = row(Instant.parse("2026-01-01T00:00:00.500Z")) + + assertTrue(older["savedAt"].toString() > newer["savedAt"].toString(), "the string order is backwards") + assertTrue( + (older["savedAtEpochMillis"] as Long) < (newer["savedAtEpochMillis"] as Long), + "the numeric order is the true one", + ) + } + + @Test + fun `every property the mapper writes is required when reading it back`() { + // schemaName is in the list on purpose. That corruption can't reach the store's readers -- + // they all MATCH on schemaName, so a node without one is filtered out upstream -- but the + // mapper is the contract's home and this is where it's pinned. + listOf("schemaName", "contentHash", "entityTypeNames", "entityTypeLabels", "entityTypeProperties", "relationshipNames") + .forEach { property -> + val corrupt = row().apply { remove(property) } + val thrown = assertThrows("removing '$property' must fail the read") { + MetamodelVersionRowMapper.fromRow(corrupt) + } + assertTrue(thrown.message!!.contains(property), "the failure must name '$property': ${thrown.message}") + } + } + + @Test + fun `a property signature missing a field fails the read, naming the field and the type`() { + val corrupt = row().apply { + put("entityTypeProperties", """{"Person":[{"name":"name","kind":"VALUE","type":"string"}]}""") + } + + val thrown = assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } + assertTrue(thrown.message!!.contains("cardinality"), thrown.message) + assertTrue(thrown.message!!.contains("Person"), thrown.message) + } + + @Test + fun `a property signature naming an enum constant this build does not have fails the read`() { + // A node written by a build whose Cardinality had a constant ours doesn't. Substituting a + // default would change the content and then fail the integrity check with a confusing + // message about hashes; failing here says what actually happened. + val corrupt = row().apply { + put( + "entityTypeProperties", + """{"Person":[{"name":"name","kind":"VALUE","type":"string","cardinality":"MANY_ISH"}]}""", + ) + } + + val thrown = assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } + assertTrue(thrown.message!!.contains("MANY_ISH"), thrown.message) + assertTrue(thrown.message!!.contains("Cardinality"), thrown.message) + } + + @Test + fun `a stored hash that disagrees with the stored fields fails the integrity check`() { + // contentHash is derived from the structural fields, so the copy on the node is a checksum. + // Rewriting the fields underneath it -- an old hash format, a hand-edit -- must be caught. + val corrupt = row().apply { put("relationshipNames", """["SOMETHING_ELSE"]""") } + + val thrown = assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } + assertTrue(thrown.message!!.contains("integrity check"), "the failure must say what went wrong: ${thrown.message}") + } + + @Test + fun `an empty schema round-trips as empty, not as null`() { + val empty = MetamodelVersion("empty-schema", emptyList(), emptyMap(), emptyMap(), emptyList()) + + assertEquals(empty, MetamodelVersionRowMapper.fromRow(MetamodelVersionRowMapper.bindMap(empty, savedAt))) + } +} diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/Neo4jTestContainer.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/Neo4jTestContainer.kt new file mode 100644 index 00000000..dc3106c8 --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/Neo4jTestContainer.kt @@ -0,0 +1,61 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.storage + +import org.springframework.test.context.DynamicPropertyRegistry +import org.testcontainers.containers.Neo4jContainer +import org.testcontainers.utility.DockerImageName + +/** + * One self-managed Neo4j testcontainer, shared by every IT in this module's test JVM, pinned to a + * version we choose instead of the `neo4j:5.26.1-community` `@EnableDrivineTestConfig` hardcodes + * (no override hook of its own, and 0.0.58 is drivine4j's newest release). `5.26.1-community` has + * a confirmed upstream bug (https://github.com/neo4j/neo4j/issues/13597): a dynamic + * relationship-type parameter gets baked into the query-plan cache on first execution and + * silently reused for every later execution of the same query text with a different value. + * Confirmed fixed on `neo4j:2026.05-community`, which is what this starts. + * + * Rather than let Drivine start its own container, each test class wires this one in via + * `test.neo4j.use-local=true` -- the officially-supported switch that tells + * `DrivineTestConfiguration` to use whatever's in the Spring `Environment` for the `neo` datasource + * as-is, instead of overriding host/port/password with its own testcontainer. A `@DynamicPropertySource` + * method in each test class (see [registerProperties]) supplies those values from *this* container, + * started on first use and reused (via Kotlin `object`/`by lazy`) for the rest of the test JVM. + * + * Duplicated (not shared) with `dice-storage-autoconfigure`'s copy -- each module's tests run in + * their own forked JVM/classpath. + */ +object Neo4jTestContainer { + + const val PASSWORD = "test-password" + + val instance: Neo4jContainer<*> by lazy { + Neo4jContainer(DockerImageName.parse("neo4j:2026.05-community").asCompatibleSubstituteFor(DockerImageName.parse("neo4j"))) + .withAdminPassword(PASSWORD) + .withPlugins("apoc") + .withNeo4jConfig("dbms.security.procedures.unrestricted", "apoc.*") + .withNeo4jConfig("dbms.security.procedures.allowlist", "apoc.*") + .also { it.start() } + } + + /** Points Drivine's `neo` datasource at [instance] instead of its own testcontainer. */ + fun registerProperties(registry: DynamicPropertyRegistry) { + registry.add("test.neo4j.use-local") { "true" } + registry.add("database.datasources.neo.host") { instance.host } + registry.add("database.datasources.neo.port") { instance.getMappedPort(7687) } + registry.add("database.datasources.neo.password") { PASSWORD } + } +} diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt index 37776c52..186257fd 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt @@ -31,6 +31,10 @@ import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration import org.springframework.context.annotation.EnableAspectJAutoProxy import org.springframework.transaction.PlatformTransactionManager +import java.time.Clock +import java.time.Instant +import java.time.ZoneId +import java.time.ZoneOffset import kotlin.random.Random /** @@ -49,6 +53,35 @@ class FakeEmbeddingService(override val dimensions: Int = 16) : EmbeddingService override fun embed(texts: List): List = texts.map(::embed) } +/** + * A clock a test can pin to an instant of its choosing. Left alone it just reads the system clock, + * so anything that doesn't care about the exact save instant behaves exactly as it would in + * production. Pin it and [DrivineMetamodelVersionStore.saveVersion] stamps the version at that + * instant instead — which is the only way to place two saves at deliberately chosen timestamps + * rather than at whatever the wall clock said, and so the only way to test that ordering is + * chronological. + */ +class PinnableClock : Clock() { + + @Volatile + private var pinned: Instant? = null + + fun pin(instant: Instant) { + pinned = instant + } + + fun unpin() { + pinned = null + } + + override fun instant(): Instant = pinned ?: Instant.now() + + override fun getZone(): ZoneId = ZoneOffset.UTC + + /** There is only one of these and its zone never matters — it only ever produces instants. */ + override fun withZone(zone: ZoneId): Clock = this +} + /** * Test wiring: Drivine's test support spins a Neo4j testcontainer and transaction management; * we add the graph stores and a fake embedding service. [SchemaCatalog] beans are ensured on @@ -126,4 +159,31 @@ open class TestApplication { repository: DrivinePropositionRepository, persistenceManager: PersistenceManager, ): GraphDecayManager = GraphDecayManager(repository, persistenceManager) + + /** + * Both MERGEs the version store performs need their key to be unique, because a MERGE is only + * race-free when it is. Without the first, concurrent saves of one version all miss the match, + * all create, and the history fills with duplicates. Without the second, a schema can end up + * with two counter nodes handing out the same sequence numbers. + * + * The third is the safety net under the sequence itself: it makes two versions of one schema + * sharing a position impossible to store, so if the counter increment ever lost an update the + * write would fail loudly rather than quietly scrambling the order. + * `DrivineMetamodelVersionStoreIntegrationTest` pins all three. + */ + @Bean + open fun metamodelSchema(): SchemaCatalog = SchemaCatalog.of( + UniquenessConstraintSpec(label = "MetamodelVersion", properties = listOf("schemaName", "contentHash")), + UniquenessConstraintSpec(label = "MetamodelSchemaCounter", property = "schemaName"), + UniquenessConstraintSpec(label = "MetamodelVersion", properties = listOf("schemaName", "sequence")), + ) + + @Bean + open fun metamodelClock(): PinnableClock = PinnableClock() + + @Bean + open fun metamodelVersionStore( + persistenceManager: PersistenceManager, + clock: PinnableClock, + ): DrivineMetamodelVersionStore = DrivineMetamodelVersionStore(persistenceManager, clock) } diff --git a/docs/design/architecture.md b/docs/design/architecture.md index 6e2570cb..0bd50e7e 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -12,11 +12,11 @@ DICE is a multi-module Maven build. Each module's intent, and what it's allowed | Module | Intent | |---|---| | `dice` | The core: proposition model, pipeline, gates, projection interfaces, query facades, agent tools, REST controllers. In-memory implementations only — no database driver. | -| `dice-storage` | The durable Neo4j backend: `Drivine`-based repository, graph/Prolog/lineage projectors, schema and index bootstrap. Depends on `dice`. | +| `dice-storage` | The durable Neo4j backend: `Drivine`-based repository, graph/Prolog/lineage projectors, schema and index bootstrap, `MetamodelVersionStore` persistence. Depends on `dice` and `dice-metamodel`. | | `dice-storage-autoconfigure` | Spring Boot autoconfiguration that wires `dice-storage`'s beans (repository, projectors, trust scorer) into a host application. Depends on `dice-storage`. | | `dice-ingestion` | Content-hash dedup ledger and source adapters that sit in front of `PropositionPipeline`, so the same artifact is never extracted twice concurrently. Depends on `dice`. | | `dice-report` | Rationale and structured report generation over propositions and their lineage. Depends on `dice`. | -| `dice-metamodel` | Schema versioning: content-hash stamps over the governed part of a `DataDictionary`, the declared-schema seam, and the version store contract. Pure JVM. Depends on no other DICE module. | +| `dice-metamodel` | Schema versioning: content-hash stamps over the governed part of a `DataDictionary`, the declared-schema seam, and the version store contract. Pure JVM. Depends on no other DICE module. `dice-storage` implements its store contract. | | `dice-integration-tests` | End-to-end tests exercising the real Neo4j backend and full pipeline across module boundaries. Depends on `dice`, `dice-ingestion`, `dice-report` (and transitively `dice-storage`). Not shipped. | ```mermaid @@ -30,6 +30,7 @@ flowchart TB itest["dice-integration-tests"] storage --> dice + storage --> metamodel autoconf --> storage ingestion --> dice report --> dice @@ -39,11 +40,11 @@ flowchart TB ``` `dice` never depends on any other DICE module — it's the leaf of the graph, so every other module -can be added or removed without touching core logic. `dice-metamodel` is a second leaf with no -edges: it stamps a schema, and depends only on Embabel's agent core types. -`dice-storage-autoconfigure` is the only module that knows about Spring -Boot autoconfiguration; plain `dice-storage` stays framework-neutral so it can be wired by hand -outside Spring Boot. +can be added or removed without touching core logic. `dice-metamodel` stamps a schema, and depends +only on Embabel's agent core types. One DICE module depends on it: `dice-storage`, which implements +its `MetamodelVersionStore` against Neo4j. `dice-storage-autoconfigure` is the only module that +knows about Spring Boot autoconfiguration; plain `dice-storage` stays framework-neutral so it can +be wired by hand outside Spring Boot. ### Subsystem design docs diff --git a/docs/design/metamodel-versioning.md b/docs/design/metamodel-versioning.md index b5ff7b86..c3b41767 100644 --- a/docs/design/metamodel-versioning.md +++ b/docs/design/metamodel-versioning.md @@ -336,6 +336,33 @@ keyed question; a backend that can push the lookup down to the database should o module ships no implementation. Storage is a separate concern, and a stamp is useful in memory before anything durable exists. +The durable implementation lives in `dice-storage`. `DrivineMetamodelVersionStore` keeps each stamp +as a `(:MetamodelVersion)` node and MERGEs on `(schemaName, contentHash)`, so re-stamping an +unchanged schema updates the node already there. Three things govern how it behaves: + +- **It needs three uniqueness constraints**, declared in a `SchemaCatalog` bean. A MERGE is + race-free only when what it merges on is unique, so `MetamodelVersion(schemaName, contentHash)` + and `MetamodelSchemaCounter(schemaName)` are both required. Without the first, concurrent saves of + one version all miss the match, all create, and history fills with copies. The third, + `MetamodelVersion(schemaName, sequence)`, guards the ordering described below. +- **Ordered reads sort on a per-schema counter.** "Most recent" here means logical write order, + which no timestamp can express: two saves land in the same millisecond routinely, and an NTP + correction or a failover can move the clock backwards between them. Each schema owns a + `(:MetamodelSchemaCounter)` node, and a version takes the next number off it in the same statement + that creates the version node. `savedAt` and `savedAtEpochMillis` are informational; nothing sorts + on them. Because `(schemaName, sequence)` is unique, a lost counter update surfaces as a retryable + failure. +- **A re-save updates content only.** Sequence, counter, and `savedAt` keep their existing values, + so an old stamp stays at its original position in the history. The in-memory reference + implementation behaves the same way. + +The structural fields are stored as JSON strings, since Neo4j properties are scalars and flat +arrays. Property signatures get explicit named fields with enums by name +(`{"name": "age", "kind": "VALUE", "type": "integer", "cardinality": "ONE"}`); an ordinal would +re-point the day someone inserts a constant into `Cardinality`. The content hash is derived, so the +`contentHash` on a node is a checksum: the store recomputes it on read and skips a node that +disagrees with itself, logging a warning. + ## Plain classes, not data classes `MetamodelVersion`, `DeclaredSchema` and `SchemaAliases` each write their own `equals`, `hashCode` From aafaf6025b8d348ef8cce90ad75327b4203e6fb3 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Sun, 30 Aug 2026 20:36:23 -0400 Subject: [PATCH 2/8] docs(dice-storage): voice pass on version-store docs and KDoc Comment and doc text only; no code change. --- CHANGELOG.md | 22 ++- .../storage/DrivineMetamodelVersionStore.kt | 125 ++++++++---------- .../dice/storage/MetamodelRowMappers.kt | 94 ++++++------- ...ivineCollectorTraceStoreIntegrationTest.kt | 4 +- .../DrivineGraphQueryParityIntegrationTest.kt | 4 +- ...rivineLineageRecordStoreIntegrationTest.kt | 4 +- ...ineMetamodelVersionStoreIntegrationTest.kt | 125 +++++++++--------- ...PropositionStoreContractIntegrationTest.kt | 4 +- .../DrivinePropositionStoreIntegrationTest.kt | 4 +- .../dice/storage/MetamodelRowMapperTest.kt | 42 +++--- .../dice/storage/Neo4jTestContainer.kt | 31 +++-- .../embabel/dice/storage/TestApplication.kt | 23 ++-- 12 files changed, 226 insertions(+), 256 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ae2349d..677af53d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,16 +54,14 @@ and the consumer PRs that deliver it). touched. - Drivine/Neo4j-backed `MetamodelVersionStore` in `dice-storage` (`DrivineMetamodelVersionStore`): stamps persist as `(:MetamodelVersion)` nodes, - MERGEd on the natural key `(schemaName, contentHash)` so a re-stamp updates in - place instead of duplicating. `latestVersion`, `versionHistory` and `findVersion` - all resolve in Cypher. History is ordered by a persisted per-schema sequence, taken - off a `(:MetamodelSchemaCounter)` node in the same statement that creates the version, - so ordering is true logical write order rather than a wall clock that ties within a - millisecond and can run backwards under NTP correction or failover; `savedAt` and - `savedAtEpochMillis` remain as informational metadata. An idempotent re-save neither - bumps the counter nor reassigns a sequence. Concurrent saves of one version leave - exactly one node. Hosts must declare three uniqueness constraints: - `MetamodelVersion(schemaName, contentHash)`, `MetamodelSchemaCounter(schemaName)`, and - `MetamodelVersion(schemaName, sequence)`. - **Compatibility: additive.** New class and a new `dice-storage` → `dice-metamodel` + MERGEd on the natural key `(schemaName, contentHash)`, so a re-stamp updates in + place. `latestVersion`, `versionHistory` and `findVersion` all resolve in Cypher. + History is ordered by a persisted per-schema sequence, taken off a + `(:MetamodelSchemaCounter)` node in the same statement that creates the version; + `savedAt` and `savedAtEpochMillis` are informational, and nothing sorts on them. + An idempotent re-save leaves the counter and the sequence alone. Concurrent saves + of one version leave one node. Hosts must declare three uniqueness constraints: + `MetamodelVersion(schemaName, contentHash)`, `MetamodelSchemaCounter(schemaName)`, + and `MetamodelVersion(schemaName, sequence)`. + **Compatibility: additive.** New class, and a new `dice-storage` → `dice-metamodel` module dependency; no existing API touched. diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt index ce82189a..51f388cf 100644 --- a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt @@ -24,46 +24,43 @@ import org.springframework.transaction.annotation.Transactional import java.time.Clock /** - * Drivine / Neo4j implementation of [MetamodelVersionStore]: keeps every schema stamp as a + * Drivine/Neo4j implementation of [MetamodelVersionStore]. Every schema stamp is a * `(:MetamodelVersion)` node. * * The write MERGEs on the natural key `(schemaName, contentHash)`, so a retry or a re-stamp of an - * unchanged schema updates the node that's already there instead of adding a duplicate. That's only - * race-free under a uniqueness constraint on the same pair of properties — without one, concurrent - * MERGEs all miss, all take the CREATE branch, and history fills with copies of one version. + * unchanged schema updates the node that's already there. That is race-free only under a uniqueness + * constraint on the same pair of properties: without one, concurrent MERGEs all miss, all take the + * CREATE branch, and history fills with copies of one version. * - * Every statement is parameterized — nothing user-derived is ever interpolated into Cypher. Ordering - * and the keyed lookup both run in the database rather than over an in-memory list. + * Every statement is parameterized; nothing user-derived is interpolated into Cypher. Ordering and + * the keyed lookup both run in the database. * - * **Order comes from a counter, not from a clock.** The contract says "most recent" means logical - * write order, and a wall clock can't express that: two saves can land in the same millisecond, and - * an NTP correction or a failover to a differently-skewed node can make the clock run *backwards* - * between them. Either way the newest stamp stops being the one that comes back. So each schema owns - * a `(:MetamodelSchemaCounter)` node, and a version gets the next value off it when — and only - * when — its node is first created. That sequence is what every ordered read sorts on. `savedAt` and - * `savedAtEpochMillis` are still written, but they are now informational metadata: useful when you - * are staring at a node wondering when it landed, and sorted on by nothing. + * Ordered reads sort on a per-schema counter. "Most recent" in the contract means logical write + * order, which a wall clock can't express: two saves can land in the same millisecond, and an NTP + * correction or a failover to a differently-skewed node can make the clock run backwards between + * them. Each schema owns a `(:MetamodelSchemaCounter)` node, and a version takes the next value off + * it when its node is first created. `savedAt` and `savedAtEpochMillis` are informational; nothing + * sorts on them. * * The counter is bumped in the same statement, and so the same transaction, as the MERGE that - * creates the version — there is no window in which a version node exists without its place in the - * order. A re-save of a version that already exists neither bumps the counter nor reassigns the - * sequence, which is what keeps an idempotent write idempotent and stops an old stamp jumping to the - * head of the history. That mirrors the in-memory reference implementation, where a re-save keeps - * its original position in the list. + * creates the version, so a version node always carries its place in the order. A re-save of an + * existing version leaves the counter and the sequence alone, which keeps the write idempotent and + * holds an old stamp at its original position. The in-memory reference implementation behaves the + * same way. * - * **Three constraints are required**, and all are the host's job to declare in a `SchemaCatalog` - * bean (the module's `TestApplication` shows the shape): - * - `UniquenessConstraintSpec("MetamodelVersion", listOf("schemaName", "contentHash"))` — makes the + * Three uniqueness constraints are required, and the host declares them in a `SchemaCatalog` bean + * (the module's `TestApplication` shows the shape): + * - `UniquenessConstraintSpec("MetamodelVersion", listOf("schemaName", "contentHash"))` makes the * version MERGE race-free, as above. - * - `UniquenessConstraintSpec("MetamodelSchemaCounter", "schemaName")` — makes the counter MERGE - * race-free, so a schema can't end up with two counters handing out the same numbers. - * - `UniquenessConstraintSpec("MetamodelVersion", listOf("schemaName", "sequence"))` — makes two - * versions sharing a position in the order unstorable, so a lost counter update fails loudly and - * retryably instead of quietly making "newest first" arbitrary again. + * - `UniquenessConstraintSpec("MetamodelSchemaCounter", "schemaName")` makes the counter MERGE + * race-free, so one schema can only have one counter handing out numbers. + * - `UniquenessConstraintSpec("MetamodelVersion", listOf("schemaName", "sequence"))` makes two + * versions sharing a position in the order unstorable, so a lost counter update fails with a + * constraint violation the caller can retry. * * @param persistenceManager Drivine's handle on the `neo` datasource. - * @param clock supplies the instant a version is stamped as saved at. Injectable so a test can place - * two saves at instants it chooses rather than at whatever the wall clock happened to say. + * @param clock supplies the instant a version is stamped as saved at. Injectable so a test can pin + * the instants of two saves. */ open class DrivineMetamodelVersionStore( private val persistenceManager: PersistenceManager, @@ -75,32 +72,26 @@ open class DrivineMetamodelVersionStore( private companion object { /** - * Upsert the version node and, if this is the first time we've seen it, give it the next - * number off its schema's counter. One statement, so one transaction: a version node never - * exists without its place in the write order. + * Upsert the version node and, on first insert, give it the next number off its schema's + * counter. One statement, so one transaction: a version node always carries its place in + * the write order. * - * The two halves are separated by `WITH n WHERE n.sequence IS NULL`. On a re-save that - * filters the row away, so the counter is never bumped and the existing sequence is never - * reassigned — the content is refreshed and the version keeps the position it has always - * had. + * `WITH n WHERE n.sequence IS NULL` separates the two halves. A re-save filters the row + * away, so the counter stays put and the existing sequence is kept; only the content is + * refreshed. * - * `SET c.lockedBy = $contentHash` writes a property nobody reads, to take the exclusive lock - * on the counter before the increment below reads it. On its own, - * `SET c.sequence = coalesce(c.sequence, 0) + 1` is a read-modify-write, and the textbook - * failure is two concurrent saves both reading 5, both writing 6, and two versions claiming - * one position. This is the documented Neo4j idiom for avoiding that. + * `SET c.lockedBy = $contentHash` writes a property nobody reads. It takes the exclusive + * lock on the counter before `SET c.sequence = coalesce(c.sequence, 0) + 1` reads it. That + * increment is a read-modify-write, whose textbook failure is two concurrent saves both + * reading 5, both writing 6, and two versions claiming one position; the lock write is the + * documented Neo4j idiom for avoiding it. This store's own tests at 12 and at 48 concurrent + * savers could not tell the locked and unlocked statements apart, so Neo4j appears to + * serialise the increment here anyway, and the line is cheap insurance. * - * Being honest about how well that's established: at 12 and at 48 concurrent savers this - * store's own tests could not tell the locked and unlocked versions apart, so Neo4j appears - * to serialise the increment on its own here. The line is kept as cheap insurance, not - * because a failing test demanded it — don't read it as load-bearing. - * - * What *is* load-bearing is the uniqueness constraint on `(schemaName, sequence)`. It makes - * a duplicate position impossible to store rather than merely unlikely: if the increment - * ever did lose an update — a Neo4j version with different locking, a cluster, contention - * beyond what's been tried — the second writer fails loudly with a constraint violation and - * the caller retries, instead of silently corrupting the order. Correctness rests on that, - * not on a lock this code can't verify. + * Correctness rests on the uniqueness constraint on `(schemaName, sequence)`, which makes a + * duplicate position impossible to store. If the increment ever did lose an update (a Neo4j + * version with different locking, a cluster, contention beyond what has been tried) the + * second writer fails with a constraint violation and the caller retries. */ private val SAVE_VERSION = """ MERGE (n:MetamodelVersion {schemaName: ${'$'}schemaName, contentHash: ${'$'}contentHash}) @@ -123,10 +114,10 @@ open class DrivineMetamodelVersionStore( /** * Every stamp for one schema, newest first. * - * `coalesce(n.sequence, -1)` rather than a bare `n.sequence` because Neo4j sorts null as the - * *largest* value, so a node that somehow has no sequence would sort to the front of a DESC - * order and be handed back as the newest. A node with no sequence never took a place in the - * write order at all, so last is the honest position for it. + * The sort key is `coalesce(n.sequence, -1)`: Neo4j sorts null as the largest value, so a + * node that somehow has no sequence would sort to the front of a DESC order and be handed + * back as the newest. A node with no sequence never took a place in the write order, so it + * belongs last. */ private val VERSIONS_NEWEST_FIRST = """ MATCH (n:MetamodelVersion {schemaName: ${'$'}schemaName}) @@ -157,9 +148,9 @@ open class DrivineMetamodelVersionStore( readVersions(VERSIONS_NEWEST_FIRST, mapOf("schemaName" to schemaName)) /** - * Overridden so resolving a recorded hash is a single keyed `MATCH` rather than a read of the - * schema's whole history followed by an in-memory filter. Both halves of the natural key are in - * the pattern, which is exactly what the uniqueness constraint indexes. + * Overridden to resolve a recorded hash with a single keyed `MATCH`; the interface default + * reads the schema's whole history and filters it in memory. Both halves of the natural key are + * in the pattern, which is what the uniqueness constraint indexes. */ @Transactional(readOnly = true) override fun findVersion(schemaName: String, contentHash: String): MetamodelVersion? = readVersions( @@ -175,16 +166,16 @@ open class DrivineMetamodelVersionStore( * Run one of the version queries and turn its rows into stamps, dropping any row that won't * deserialize. * - * A single corrupt or tampered node shouldn't take down a whole history read, so it's warned - * about and skipped — see [MetamodelVersionRowMapper], which throws rather than inventing - * defaults precisely so this can happen. The warning names the property or the failed integrity - * check, which is what an operator needs to go find the node. + * A single corrupt or tampered node shouldn't take down a whole history read, so the row is + * logged at warn and skipped. [MetamodelVersionRowMapper] throws on bad data so that this can + * happen; the warning names the missing property or the failed integrity check, which is what + * an operator needs to go find the node. * - * Note what [latestVersion] does *not* do: push a `LIMIT 1` into Cypher. If the newest node were - * the corrupt one, that would read it, drop it, and answer "this schema has no versions" — - * hiding the perfectly good history behind it, and disagreeing with [versionHistory], whose + * [latestVersion] deliberately keeps `LIMIT 1` out of the Cypher. If the newest node were the + * corrupt one, a database-side limit would read it, drop it, and answer "this schema has no + * versions", hiding the good history behind it and disagreeing with [versionHistory], whose * first element is meant to be the same stamp. It sorts in the database and takes the first - * survivor instead, so a bad node hides only itself. + * survivor here. */ private fun readVersions(statement: String, bindings: Map): List { @Suppress("UNCHECKED_CAST") diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt index f9e483c3..d132fd53 100644 --- a/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt @@ -27,34 +27,33 @@ private val objectMapper = ObjectMapper() * Translate metamodel versions to and from the property maps the Neo4j graph store reads and * writes. * - * Neo4j properties are scalars and flat arrays, and a version's content is neither — it is lists, - * a map of label sets, and a map of property *signature* sets. So all four structural fields are - * serialized to JSON strings. JSON also handles names containing pipes, tabs, newlines and quotes, - * which matters because these names come out of LLM extraction and routinely do. + * Neo4j properties are scalars and flat arrays, while a version's content is lists, a map of label + * sets, and a map of property signature sets, so all four structural fields are serialized to JSON + * strings. JSON also handles names containing pipes, tabs, newlines and quotes, which these names + * routinely do: they come out of LLM extraction. * - * **The timestamp is informational, and is written twice anyway.** `savedAt` is the ISO-8601 string, - * which is what you want when you're looking at a node and wondering when it landed. `savedAtEpochMillis` - * is the same instant as a number, which is what you want when you're filtering or grouping by time - * in an ad-hoc query — the string is no use for that, since `Instant.toString()` drops the fraction - * entirely at a whole second and `'Z'` sorts above `'.'`, making `"…T00:00:00Z"` compare greater than - * `"…T00:00:00.500Z"`. + * The save instant is informational, and is written twice. `savedAt` is the ISO-8601 string, which + * is what you want when you're looking at a node and wondering when it landed. `savedAtEpochMillis` + * is the same instant as a number, for filtering or grouping by time in an ad-hoc query; the string + * is no use for that, since `Instant.toString()` drops the fraction entirely at a whole second and + * `'Z'` sorts above `'.'`, making `"…T00:00:00Z"` compare greater than `"…T00:00:00.500Z"`. * - * Neither one orders the history. That's the `sequence` property's job — see - * [DrivineMetamodelVersionStore], which explains why a clock can't express write order. Nothing here - * writes or reads `sequence`: it's assigned by Cypher off a per-schema counter, and it is storage - * bookkeeping rather than part of the stamp, so it stays out of the strict round-trip below. + * Neither field orders the history. The `sequence` property does that; see + * [DrivineMetamodelVersionStore] for why a clock can't express write order. Nothing here writes or + * reads `sequence`: Cypher assigns it off a per-schema counter, and it is storage bookkeeping, so + * it stays out of the strict round-trip below. * - * **Reads are strict.** A property this mapper wrote must be present when it is read again. A node - * missing one is corrupt — not a version with a blank name — so the accessor throws and the store's - * surrounding guard skips the row with a warning instead of quietly materializing junk. + * Reads are strict. A property this mapper wrote must be present when it is read again; a node + * missing one is corrupt, so the accessor throws and the store's surrounding guard skips the row + * with a warning. */ object MetamodelVersionRowMapper { /** - * Bind values for a write — the natural key is (schemaName, contentHash). + * Bind values for a write. The natural key is (schemaName, contentHash). * - * [savedAt] is passed in rather than read from the wall clock here, so this stays a pure - * function of its arguments and a test can pin the instant a version was stored at. + * [savedAt] is a parameter, so this stays a pure function of its arguments and a test can pin + * the instant a version was stored at. */ fun bindMap(version: MetamodelVersion, savedAt: Instant): Map = mapOf( "schemaName" to version.schemaName, @@ -71,13 +70,12 @@ object MetamodelVersionRowMapper { * Rebuild a [MetamodelVersion] from a returned node's property map, and check its integrity * on the way. * - * A version's content hash is derived from its structural fields, not stored alongside them as - * an independent value, so the reconstructed object computes its own hash. The `contentHash` - * property on the node is therefore a checksum rather than data: recomputing it and finding a - * different answer means the node was written by an older hash format, hand-edited, or - * corrupted. Either way it is not the version it claims to be, so this throws and the caller - * skips it. Note the stored hash is also half the natural key, so a mismatch would additionally - * mean a re-save of the same content lands on a *different* node. + * A version's content hash is derived from its structural fields, so the reconstructed object + * computes its own hash and the `contentHash` property on the node acts as a checksum. + * Recomputing it and getting a different answer means the node was written by an older hash + * format, hand-edited, or corrupted, so this throws and the caller skips it. The stored hash is + * also half the natural key, so a mismatch also means a re-save of the same content lands on a + * different node. */ fun fromRow(row: Map<*, *>): MetamodelVersion { val storedHash = row.str("contentHash") @@ -96,13 +94,11 @@ object MetamodelVersionRowMapper { } } -// Serialization helpers: JSON for escape-safe round-trip encoding. +// Serialization helpers: JSON, for escape-safe round-trip encoding. -/** Serialize a list to a JSON string. */ private fun serializeList(items: List): String = objectMapper.writeValueAsString(items) -/** Deserialize a JSON string back to a list. */ private fun deserializeList(serialized: String): List = if (serialized.isEmpty()) emptyList() else objectMapper.readValue( @@ -113,9 +109,9 @@ private fun deserializeList(serialized: String): List = /** * Serialize the per-type label sets as `{"Person": ["Agent", "Entity"], ...}`. * - * Sets have no order, so they're written sorted. Nothing reads the order back — the sets go into a - * `Set` again — but a deterministic encoding means re-saving the same version writes byte-identical - * JSON, which keeps an idempotent MERGE genuinely a no-op and makes a stored node diffable by hand. + * Sets have no order, so they're written sorted. Nothing reads the order back, but a deterministic + * encoding means re-saving the same version writes byte-identical JSON, which keeps an idempotent + * MERGE a no-op and makes a stored node diffable by hand. */ private fun serializeMapOfLabelSets(map: Map>): String = objectMapper.writeValueAsString(map.toSortedMap().mapValues { (_, labels) -> labels.sorted() }) @@ -138,19 +134,19 @@ private fun deserializeMapOfLabelSets(serialized: String): Map>): String = objectMapper.writeValueAsString( map.toSortedMap().mapValues { (_, signatures) -> signatures.sorted().map { signature -> - // A LinkedHashMap, so the keys land in this order in the JSON and the encoding is + // A LinkedHashMap, so the keys land in the JSON in this order and the encoding is // fully determined by the content. linkedMapOf( "name" to signature.name, @@ -163,11 +159,10 @@ private fun serializeMapOfSignatureSets(map: Map> ) /** - * Inverse of [serializeMapOfSignatureSets], and strict about it: a signature object missing a field, - * or naming an enum constant this build doesn't have, throws rather than being patched up with a - * default. A guessed default would change the structural content, and the version's integrity check - * would then reject the whole row anyway — with a confusing message about a hash mismatch instead of - * the real problem. + * Inverse of [serializeMapOfSignatureSets], and strict about it: a signature object missing a + * field, or naming an enum constant this build doesn't have, throws. Patching it up with a default + * would change the structural content, and the version's integrity check would then reject the + * whole row with a message about a hash mismatch that hides the real problem. */ private fun deserializeMapOfSignatureSets(serialized: String): Map> { if (serialized.isEmpty()) return emptyMap() @@ -209,11 +204,10 @@ private inline fun > enumConstant(stored: String, typeName: /** * Read a property that must be there, and blow up if it isn't. * - * Returning `""` for an absent property would be the friendlier-looking choice and is precisely - * the wrong one: a node missing `schemaName` would come back as a real-looking version named `""`, - * indistinguishable from data, and the caller's "skip the unreadable row" guard would never fire - * for the most likely kind of corruption there is. Throwing is what makes that guard mean - * something. + * Returning `""` for an absent property would let a node missing `schemaName` come back as a + * real-looking version named `""`, indistinguishable from data, and the caller's "skip the + * unreadable row" guard would never fire for the most likely kind of corruption there is. Throwing + * is what gives that guard something to catch. */ private fun Map<*, *>.str(key: String): String = this[key]?.toString() ?: throw IllegalArgumentException("required property '$key' is missing from the stored node") diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineCollectorTraceStoreIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineCollectorTraceStoreIntegrationTest.kt index 7cb03cab..1c6ffa26 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineCollectorTraceStoreIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineCollectorTraceStoreIntegrationTest.kt @@ -47,8 +47,8 @@ import org.springframework.test.context.DynamicPropertySource * Integration tests for [DrivineCollectorTraceStore] against a Neo4j testcontainer. Each test * starts from an empty graph via [cleanUp]. * - * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class - * for why. + * Uses the shared [Neo4jTestContainer]; see that class for why Drivine's built-in testcontainer + * is bypassed. */ @SpringBootTest(classes = [TestApplication::class]) class DrivineCollectorTraceStoreIntegrationTest { diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineGraphQueryParityIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineGraphQueryParityIntegrationTest.kt index 6f195dcc..423df71c 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineGraphQueryParityIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineGraphQueryParityIntegrationTest.kt @@ -55,8 +55,8 @@ import java.time.Instant * present), we assert each returned edge is *valid* rather than object-identical — both engines are * free to pick a different but correct edge. * - * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class - * for why. + * Uses the shared [Neo4jTestContainer]; see that class for why Drivine's built-in testcontainer + * is bypassed. */ @SpringBootTest(classes = [TestApplication::class]) class DrivineGraphQueryParityIntegrationTest { diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineLineageRecordStoreIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineLineageRecordStoreIntegrationTest.kt index 0f6802b3..290a8701 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineLineageRecordStoreIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineLineageRecordStoreIntegrationTest.kt @@ -39,8 +39,8 @@ import java.time.Instant * Integration tests for the durable lineage stores against a Neo4j testcontainer (provided by * Drivine's test support). Each test starts from an empty graph via [cleanUp]. * - * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class - * for why. + * Uses the shared [Neo4jTestContainer]; see that class for why Drivine's built-in testcontainer + * is bypassed. */ @SpringBootTest(classes = [TestApplication::class]) class DrivineLineageRecordStoreIntegrationTest { diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt index eb7deace..88ae1525 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt @@ -46,13 +46,12 @@ import java.util.concurrent.atomic.AtomicInteger * Integration tests for [DrivineMetamodelVersionStore] against a Neo4j testcontainer. Each test * starts from an empty graph via [cleanUp]. * - * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class - * for why. + * Uses the shared [Neo4jTestContainer]; see that class for why Drivine's built-in testcontainer + * is bypassed. * - * Note what's *absent* from every `MetamodelVersion` built here: a content hash. It's derived from - * the structural fields, so two versions can't claim the same identity while describing different - * schemas -- which is why the tests that need two distinct versions of one schema give them - * genuinely different content rather than different hand-written hash strings. + * None of the `MetamodelVersion`s built here carries a hand-written content hash: the hash is + * derived from the structural fields. Tests that need two distinct versions of one schema give them + * genuinely different content. */ @SpringBootTest(classes = [TestApplication::class]) class DrivineMetamodelVersionStoreIntegrationTest { @@ -134,10 +133,10 @@ class DrivineMetamodelVersionStoreIntegrationTest { @Test fun `a property signature round-trips its kind, type and cardinality, not just its name`() { - // The whole reason entityTypeProperties holds signatures rather than bare names: turning a - // single `age` string into a list of integers is a real schema change. If the store dropped - // kind/type/cardinality on the way to disk, the reloaded stamp would hash differently from - // the one that was saved -- and the mapper's integrity check would reject its own write. + // entityTypeProperties holds signatures, so turning a single `age` string into a list of + // integers registers as the schema change it is. If the store dropped kind/type/cardinality + // on the way to disk, the reloaded stamp would hash differently from the saved one, and the + // mapper's integrity check would reject its own write. val everyShape = setOf( PropertySignature("optionalString", Kind.VALUE, "string", Cardinality.OPTIONAL), PropertySignature("oneInteger", Kind.VALUE, "integer", Cardinality.ONE), @@ -162,9 +161,8 @@ class DrivineMetamodelVersionStoreIntegrationTest { @Test fun `two versions differing only in one property's cardinality are two stored versions`() { - // Same type name, same property name -- the change is invisible to any encoding that stores - // property *names*. It has to survive as two nodes with two hashes, or the store has - // silently lost a schema change. + // Same type name, same property name: an encoding that stored only property names would + // miss this change. It has to survive as two nodes with two hashes. val schemaName = "cardinality-change-schema" fun withCardinality(cardinality: Cardinality) = MetamodelVersion( schemaName = schemaName, @@ -256,9 +254,9 @@ class DrivineMetamodelVersionStoreIntegrationTest { assertEquals(1L, storedSequence(schemaName, v1)) assertEquals(2L, storedSequence(schemaName, v2)) - // Re-stamp the old one much later. This is the idempotent path: it must refresh v1's - // content and nothing else -- not its sequence, and not the counter, or the next genuinely - // new version would skip a number. + // Re-stamp the old one much later. The idempotent path refreshes v1's content and leaves + // its sequence and the counter alone; bumping the counter would make the next new version + // skip a number. clock.pin(Instant.parse("2026-06-01T00:00:00Z")) store.saveVersion(v1) @@ -270,9 +268,9 @@ class DrivineMetamodelVersionStoreIntegrationTest { @Test fun `many threads saving the identical version leave exactly one node`() { - // The write is a MERGE, which is only race-free under a uniqueness constraint on the key it - // merges on -- see TestApplication.metamodelSchema. Without one, concurrent MERGEs all miss, - // all take the CREATE branch, and the "history" fills with duplicates of one version. + // The write is a MERGE, race-free only under a uniqueness constraint on the key it merges + // on; see TestApplication.metamodelSchema. Without one, concurrent MERGEs all miss, all take + // the CREATE branch, and the history fills with duplicates of one version. val threads = 12 val version = MetamodelVersion( schemaName = "concurrent-schema", @@ -290,8 +288,8 @@ class DrivineMetamodelVersionStoreIntegrationTest { repeat(threads) { pool.submit { startTogether.await() - // A loser in a MERGE race can surface a constraint violation or a lock timeout. - // That's tolerable -- a caller retries. Two surviving nodes are not. + // A loser in a MERGE race can surface a constraint violation or a lock timeout, + // which a caller retries. The surviving node count is what's asserted below. runCatching { store.saveVersion(version) } .onSuccess { succeeded.incrementAndGet() } .onFailure { t -> synchronized(failures) { failures += t } } @@ -313,18 +311,17 @@ class DrivineMetamodelVersionStoreIntegrationTest { "(${succeeded.get()} succeeded, ${failures.size} failed)", ) assertEquals(version, history.single()) - // The sequence is assigned in the same transaction as the MERGE that creates the node, and - // only on create -- so the losing threads, which matched an existing node, took no number. + // The sequence is assigned on create only, in the same transaction as the MERGE, so the + // losing threads matched the existing node and took no number. assertEquals(1L, storedSequence("concurrent-schema", version), "the one node must hold the first sequence") assertEquals(1L, counterValue("concurrent-schema"), "only the creating save may consume a number") } @Test fun `concurrent saves of distinct versions each get their own place in the order`() { - // The lost-update test. Every thread here creates a *different* version of one schema, so - // all of them hit the counter at the same moment. If the increment lost an update, two - // versions would claim one position and "newest first" would be arbitrary again for the - // pair -- the very bug the sequence was introduced to fix. + // The lost-update test. Every thread creates a different version of one schema, so all of + // them hit the counter at the same moment. If the increment lost an update, two versions + // would claim one position, and the order between that pair would be arbitrary. val schemaName = "concurrent-distinct-schema" val threads = 12 val versions = (1..threads).map { @@ -356,20 +353,19 @@ class DrivineMetamodelVersionStoreIntegrationTest { sequences.toSet(), "each version must hold its own sequence; got $sequences", ) - // And the order the store reports has to be a total order over all of them, not a heap with - // ties in it. + // The history the store reports has to hold all of them, with no shared positions. assertEquals(threads, store.versionHistory(schemaName).size) assertEquals(threads.toLong(), counterValue(schemaName)) } @Test fun `two versions of one schema cannot be stored at the same position`() { - // The safety net under the sequence. The counter increment appears to serialise correctly on - // its own -- removing the lock from the save statement doesn't make the test above fail, even - // at four times the contention -- so "the increment is atomic" is an observation, not a proof. - // This constraint is the proof: whatever the counter does, the database will not hold two - // versions of one schema claiming one place in the write order. A lost update becomes a - // retryable failure rather than a silently scrambled history. + // The safety net under the sequence. The counter increment appears to serialise on its own + // (removing the lock from the save statement doesn't fail the test above, even at four times + // the contention), so its atomicity is an observation rather than a proof. The guarantee + // rests on this constraint: whatever the counter does, the database will not hold two + // versions of one schema at one place in the write order, so a lost update becomes a + // retryable failure. val schemaName = "position-constraint-schema" val first = MetamodelVersion(schemaName, listOf("First"), emptyMap(), emptyMap(), emptyList()) store.saveVersion(first) @@ -412,9 +408,8 @@ class DrivineMetamodelVersionStoreIntegrationTest { @Test fun `two versions saved in the very same millisecond still order by write order`() { // No sleep, and the clock is pinned to one instant for both saves, so every timestamp on - // both nodes is byte-identical. A clock -- at any precision -- simply cannot separate these - // two; only a counter can. This is the ordinary case, not an exotic one: back-to-back saves - // land in the same millisecond routinely. + // both nodes is byte-identical. No clock, at any precision, can separate the two; the + // counter can. Back-to-back saves land in the same millisecond routinely. val schemaName = "same-millisecond-schema" val first = MetamodelVersion(schemaName, listOf("First"), emptyMap(), emptyMap(), emptyList()) val second = MetamodelVersion(schemaName, listOf("Second"), emptyMap(), emptyMap(), emptyList()) @@ -435,9 +430,8 @@ class DrivineMetamodelVersionStoreIntegrationTest { @Test fun `write order survives a clock that runs backwards`() { // An NTP correction, or a failover to a node with a different skew, can move the wall clock - // *backwards* between two saves. Ordering on any timestamp then reports the older stamp as - // the newest -- a silent wrong answer, which is worse than a slow one. The sequence is - // monotonic regardless of what the clock is doing. + // backwards between two saves. Ordering on any timestamp then reports the older stamp as the + // newest. The sequence is monotonic whatever the clock does. val schemaName = "clock-skew-schema" val earlier = MetamodelVersion(schemaName, listOf("WrittenFirst"), emptyMap(), emptyMap(), emptyList()) val later = MetamodelVersion(schemaName, listOf("WrittenSecond"), emptyMap(), emptyMap(), emptyList()) @@ -451,19 +445,19 @@ class DrivineMetamodelVersionStoreIntegrationTest { assertEquals(listOf(later, earlier), store.versionHistory(schemaName)) } - // ---- Corrupt rows are skipped, not materialized ---- + // ---- Corrupt rows are skipped ---- @Test fun `a version node missing a required property is skipped and warned about, not read as a blank version`() { - // Written straight through Cypher, so the node exists exactly as a partially-failed write or - // a hand-edit would leave it: one required property simply absent. `entityTypeNames` rather - // than `schemaName` because every read MATCHes on schemaName -- a node without one is - // filtered out by the query and never reaches the mapper at all. + // Written straight through Cypher, so the node looks the way a partially-failed write or a + // hand-edit would leave it: one required property absent. The absent property is + // `entityTypeNames`, because every read MATCHes on schemaName, and a node without that is + // filtered out by the query before the mapper sees it. val schemaName = "corrupt-row-schema" val good = MetamodelVersion(schemaName, listOf("Sound"), emptyMap(), emptyMap(), emptyList()) store.saveVersion(good) - // Spelled out property by property rather than copied from the good node, so what's wrong - // with it is visible: every property the mapper writes except `entityTypeNames`. + // Spelled out property by property so the defect is visible here: every property the mapper + // writes except `entityTypeNames`. val brokenSavedAt = Instant.parse("2026-01-01T00:00:00Z") persistenceManager.execute( QuerySpecification.withStatement( @@ -503,9 +497,9 @@ class DrivineMetamodelVersionStoreIntegrationTest { @Test fun `a stored property signature missing a field is skipped and warned about by name`() { // The signature encoding is a persisted format of its own, so a node can be structurally - // fine and still hold a half-written signature -- an older writer, a hand-edit. Guessing a - // default cardinality would change the content and surface later as a baffling hash - // mismatch, so the mapper names the missing field instead. + // fine and still hold a half-written signature (an older writer, a hand-edit). Guessing a + // default cardinality would change the content and surface later as a hash mismatch, so the + // mapper names the missing field. val schemaName = "corrupt-signature-schema" val version = MetamodelVersion( schemaName = schemaName, @@ -541,8 +535,8 @@ class DrivineMetamodelVersionStoreIntegrationTest { @Test fun `a version node whose stored hash disagrees with its stored fields is skipped and warned about`() { // The content hash is derived from the structural fields, so the copy on the node is a - // checksum rather than data. Disagreement means the node was written by an older hash format - // or tampered with -- either way it is not the version it claims to be. + // checksum. Disagreement means the node was written by an older hash format or tampered + // with. val schemaName = "tampered-hash-schema" val version = MetamodelVersion(schemaName, listOf("Original"), emptyMap(), emptyMap(), emptyList()) store.saveVersion(version) @@ -557,10 +551,10 @@ class DrivineMetamodelVersionStoreIntegrationTest { @Test fun `a corrupt newest node hides only itself, not the readable version behind it`() { - // latestVersion deliberately sorts in Cypher but takes the first *readable* row rather than - // pushing LIMIT 1 down. With the limit in the query, a corrupt newest node would make the - // store answer "no versions at all" while versionHistory still returned the older one -- - // two reads disagreeing about the same graph. + // latestVersion sorts in Cypher and takes the first readable row, keeping LIMIT 1 out of the + // query. With the limit in the query, a corrupt newest node would make the store answer "no + // versions at all" while versionHistory still returned the older one: two reads disagreeing + // about the same graph. val schemaName = "corrupt-head-schema" val readable = MetamodelVersion(schemaName, listOf("Readable"), emptyMap(), emptyMap(), emptyList()) val doomed = MetamodelVersion(schemaName, listOf("Doomed"), emptyMap(), emptyMap(), emptyList()) @@ -587,9 +581,9 @@ class DrivineMetamodelVersionStoreIntegrationTest { @Test fun `names containing delimiter characters survive the round-trip intact`() { - // The old encoding joined on delimiters; this one is JSON, and these are the characters that - // would break a joined one. Entity type names, labels, property names, property types and - // relationship names all go through it, so all five carry a delimiter here. + // These are the characters that would break a delimiter-joined encoding; this one is JSON. + // Entity type names, labels, property names, property types and relationship names all go + // through it, so all five carry one here. listOf("|" to "pipe", "\t" to "tab", "\n" to "newline", "\"" to "quote", "\\" to "backslash") .forEach { (delimiter, label) -> val schemaName = "delimiter-$label" @@ -627,9 +621,8 @@ class DrivineMetamodelVersionStoreIntegrationTest { } /** - * The `sequence` a version node actually holds. Read straight out of the graph because the - * sequence is storage bookkeeping — it is deliberately not on [MetamodelVersion], so the only - * honest way to assert about it is to go and look. + * The `sequence` a version node holds. Read straight out of the graph, since the sequence is + * storage bookkeeping and [MetamodelVersion] doesn't carry it. */ private fun storedSequence(schemaName: String, version: MetamodelVersion): Long? = persistenceManager.maybeGetOne( @@ -655,9 +648,9 @@ class DrivineMetamodelVersionStoreIntegrationTest { )?.toInt() ?: 0 /** - * Run [block] with a listener attached to the store's logger, and hand back both its result and - * every WARN message the store emitted. Needed because "skips the row" and "skips the row *and - * says so*" are different behaviours, and only the second is any use to an operator. + * Run [block] with a listener attached to the store's logger, and hand back its result along + * with every WARN message the store emitted. The tests assert on the warning text as well as the + * skip, because an operator needs the message to find the bad node. */ private fun capturingStoreWarnings(block: () -> T): Pair> { val logger = LoggerFactory.getLogger(DrivineMetamodelVersionStore::class.java) as Logger diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreContractIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreContractIntegrationTest.kt index 3ef7d04e..eba396e5 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreContractIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreContractIntegrationTest.kt @@ -29,8 +29,8 @@ import org.springframework.test.context.DynamicPropertySource * [DrivinePropositionRepository] (testcontainer). This is the half that catches a graph backend * silently disagreeing with the in-memory contract — substitutability enforced, not assumed. * - * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class - * for why. + * Uses the shared [Neo4jTestContainer]; see that class for why Drivine's built-in testcontainer + * is bypassed. */ @SpringBootTest(classes = [TestApplication::class]) class DrivinePropositionStoreContractIntegrationTest : AbstractPropositionStoreContractTest() { diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreIntegrationTest.kt index 79d3f4c8..07702dcd 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionStoreIntegrationTest.kt @@ -54,8 +54,8 @@ import java.time.Instant * dedup commits via its own [org.springframework.transaction.support.TransactionTemplate], so * isolation is by explicit `clearAll()` per test rather than rollback. * - * Runs against [Neo4jTestContainer], not Drivine's own built-in testcontainer -- see that class - * for why. + * Uses the shared [Neo4jTestContainer]; see that class for why Drivine's built-in testcontainer + * is bypassed. */ @SpringBootTest(classes = [TestApplication::class]) class DrivinePropositionStoreIntegrationTest { diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt index 930c0fe7..1da5e80e 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt @@ -26,15 +26,14 @@ import org.junit.jupiter.api.assertThrows import java.time.Instant /** - * Unit tests for [MetamodelVersionRowMapper] — no database, just the property map it produces and + * Unit tests for [MetamodelVersionRowMapper]: no database, just the property map it produces and * consumes. * - * The point of most of these is that reading is *strict*. A stored node missing a property the - * mapper wrote is corrupt, and the mapper has to say so rather than quietly substituting a blank: - * the store wraps every read in "skip the unreadable row and warn", and that guard is only worth - * anything if unreadable rows actually throw. `DrivineMetamodelVersionStoreIntegrationTest` shows - * the guard firing end to end; these pin the mapper's half of the contract, including the cases the - * store's own `MATCH` filters out before they can reach it. + * Most of these pin strict reads. A stored node missing a property the mapper wrote is corrupt, and + * the mapper has to throw: the store wraps every read in "skip the unreadable row and warn", and + * that guard needs unreadable rows to throw. `DrivineMetamodelVersionStoreIntegrationTest` shows + * the guard firing end to end; these cover the mapper's half of the contract, including the cases + * the store's own `MATCH` filters out before they can reach it. */ class MetamodelRowMapperTest { @@ -65,14 +64,13 @@ class MetamodelRowMapperTest { @Test fun `property signatures are written as explicit named fields, enums by name, in a fixed order`() { // The encoding on disk feeds the content hash on the way back in, so it is a persisted - // format and this is where its shape is pinned. Three things at once: - // - enum *names*, never ordinals -- an ordinal would silently re-point the moment someone - // inserts a constant into Cardinality; + // format, and this is where its shape is pinned: + // - enum names, so inserting a constant into Cardinality can't re-point a stored ordinal; // - map keys sorted (Company before Person, though Person was declared first); // - signatures within a type sorted (age before name). - // The sorting is why re-saving an unchanged version writes byte-identical JSON. Without it - // the order would come from `java.util.Set.copyOf`, whose iteration order is deliberately - // randomised per JVM -- so the same stamp would encode differently after every restart. + // The sorting is why re-saving an unchanged version writes byte-identical JSON. Left + // unsorted, the order would come from `java.util.Set.copyOf`, whose iteration order is + // randomised per JVM, so the same stamp would encode differently after every restart. assertEquals( """{"Company":[{"name":"employs","kind":"REFERENCE","type":"Person","cardinality":"SET"}],""" + """"Person":[{"name":"age","kind":"VALUE","type":"integer","cardinality":"OPTIONAL"},""" + @@ -91,9 +89,9 @@ class MetamodelRowMapperTest { @Test fun `every timestamp gets a sortable numeric twin, because the ISO string is not sortable`() { - // Half a second apart, and the *older* one is the one with no fractional part. As strings - // the older sorts higher, because 'Z' outranks '.'; as numbers it doesn't. Anything that - // orders on the string will hand back the wrong row, which is why nothing does. + // Half a second apart, and the older one has no fractional part. As strings the older sorts + // higher, because 'Z' outranks '.'; as numbers it sorts lower. Anything ordering on the + // string hands back the wrong row. val older = row(Instant.parse("2026-01-01T00:00:00Z")) val newer = row(Instant.parse("2026-01-01T00:00:00.500Z")) @@ -106,9 +104,9 @@ class MetamodelRowMapperTest { @Test fun `every property the mapper writes is required when reading it back`() { - // schemaName is in the list on purpose. That corruption can't reach the store's readers -- - // they all MATCH on schemaName, so a node without one is filtered out upstream -- but the - // mapper is the contract's home and this is where it's pinned. + // schemaName is in the list on purpose. The store's readers all MATCH on schemaName, so a + // node without one is filtered out upstream and never reaches the mapper; the mapper is + // where this contract lives, so it is pinned here. listOf("schemaName", "contentHash", "entityTypeNames", "entityTypeLabels", "entityTypeProperties", "relationshipNames") .forEach { property -> val corrupt = row().apply { remove(property) } @@ -133,8 +131,8 @@ class MetamodelRowMapperTest { @Test fun `a property signature naming an enum constant this build does not have fails the read`() { // A node written by a build whose Cardinality had a constant ours doesn't. Substituting a - // default would change the content and then fail the integrity check with a confusing - // message about hashes; failing here says what actually happened. + // default would change the content and then fail the integrity check with a message about + // hashes; failing here says what actually happened. val corrupt = row().apply { put( "entityTypeProperties", @@ -150,7 +148,7 @@ class MetamodelRowMapperTest { @Test fun `a stored hash that disagrees with the stored fields fails the integrity check`() { // contentHash is derived from the structural fields, so the copy on the node is a checksum. - // Rewriting the fields underneath it -- an old hash format, a hand-edit -- must be caught. + // Rewriting the fields underneath it (an old hash format, a hand-edit) has to be caught. val corrupt = row().apply { put("relationshipNames", """["SOMETHING_ELSE"]""") } val thrown = assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/Neo4jTestContainer.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/Neo4jTestContainer.kt index dc3106c8..704c7356 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/Neo4jTestContainer.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/Neo4jTestContainer.kt @@ -20,23 +20,22 @@ import org.testcontainers.containers.Neo4jContainer import org.testcontainers.utility.DockerImageName /** - * One self-managed Neo4j testcontainer, shared by every IT in this module's test JVM, pinned to a - * version we choose instead of the `neo4j:5.26.1-community` `@EnableDrivineTestConfig` hardcodes - * (no override hook of its own, and 0.0.58 is drivine4j's newest release). `5.26.1-community` has - * a confirmed upstream bug (https://github.com/neo4j/neo4j/issues/13597): a dynamic - * relationship-type parameter gets baked into the query-plan cache on first execution and - * silently reused for every later execution of the same query text with a different value. - * Confirmed fixed on `neo4j:2026.05-community`, which is what this starts. + * One self-managed Neo4j testcontainer, shared by every IT in this module's test JVM, on a version + * we pick. `@EnableDrivineTestConfig` hardcodes `neo4j:5.26.1-community` with no override hook, and + * 0.0.58 is drivine4j's newest release. `5.26.1-community` has a confirmed upstream bug + * (https://github.com/neo4j/neo4j/issues/13597): a dynamic relationship-type parameter gets baked + * into the query-plan cache on first execution and silently reused for every later execution of the + * same query text with a different value. Confirmed fixed on `neo4j:2026.05-community`, which is + * what this starts. * - * Rather than let Drivine start its own container, each test class wires this one in via - * `test.neo4j.use-local=true` -- the officially-supported switch that tells - * `DrivineTestConfiguration` to use whatever's in the Spring `Environment` for the `neo` datasource - * as-is, instead of overriding host/port/password with its own testcontainer. A `@DynamicPropertySource` - * method in each test class (see [registerProperties]) supplies those values from *this* container, - * started on first use and reused (via Kotlin `object`/`by lazy`) for the rest of the test JVM. + * Each test class wires this container in with `test.neo4j.use-local=true`, the supported switch + * that tells `DrivineTestConfiguration` to take the `neo` datasource's host, port and password from + * the Spring `Environment` as they are. A `@DynamicPropertySource` method in each test class (see + * [registerProperties]) supplies those values from this container, started on first use and reused + * (via Kotlin `object`/`by lazy`) for the rest of the test JVM. * - * Duplicated (not shared) with `dice-storage-autoconfigure`'s copy -- each module's tests run in - * their own forked JVM/classpath. + * `dice-storage-autoconfigure` keeps its own copy; each module's tests run in a forked JVM with its + * own classpath. */ object Neo4jTestContainer { @@ -51,7 +50,7 @@ object Neo4jTestContainer { .also { it.start() } } - /** Points Drivine's `neo` datasource at [instance] instead of its own testcontainer. */ + /** Points Drivine's `neo` datasource at [instance]. */ fun registerProperties(registry: DynamicPropertyRegistry) { registry.add("test.neo4j.use-local") { "true" } registry.add("database.datasources.neo.host") { instance.host } diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt index 186257fd..1abdcec8 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt @@ -54,12 +54,10 @@ class FakeEmbeddingService(override val dimensions: Int = 16) : EmbeddingService } /** - * A clock a test can pin to an instant of its choosing. Left alone it just reads the system clock, - * so anything that doesn't care about the exact save instant behaves exactly as it would in - * production. Pin it and [DrivineMetamodelVersionStore.saveVersion] stamps the version at that - * instant instead — which is the only way to place two saves at deliberately chosen timestamps - * rather than at whatever the wall clock said, and so the only way to test that ordering is - * chronological. + * A clock a test can pin to an instant of its choosing. Left alone it reads the system clock, so + * anything that doesn't care about the exact save instant behaves as it would in production. Pin it + * and [DrivineMetamodelVersionStore.saveVersion] stamps the version at that instant, which is how a + * test places two saves at timestamps it chooses. */ class PinnableClock : Clock() { @@ -78,7 +76,7 @@ class PinnableClock : Clock() { override fun getZone(): ZoneId = ZoneOffset.UTC - /** There is only one of these and its zone never matters — it only ever produces instants. */ + /** There is only one of these, and its zone never matters: it only produces instants. */ override fun withZone(zone: ZoneId): Clock = this } @@ -161,15 +159,14 @@ open class TestApplication { ): GraphDecayManager = GraphDecayManager(repository, persistenceManager) /** - * Both MERGEs the version store performs need their key to be unique, because a MERGE is only - * race-free when it is. Without the first, concurrent saves of one version all miss the match, + * Both MERGEs the version store performs need their key to be unique, because a MERGE is + * race-free only then. Without the first, concurrent saves of one version all miss the match, * all create, and the history fills with duplicates. Without the second, a schema can end up * with two counter nodes handing out the same sequence numbers. * - * The third is the safety net under the sequence itself: it makes two versions of one schema - * sharing a position impossible to store, so if the counter increment ever lost an update the - * write would fail loudly rather than quietly scrambling the order. - * `DrivineMetamodelVersionStoreIntegrationTest` pins all three. + * The third backs the sequence itself: it makes two versions of one schema sharing a position + * impossible to store, so a lost counter update fails with a constraint violation the caller + * can retry. `DrivineMetamodelVersionStoreIntegrationTest` pins all three. */ @Bean open fun metamodelSchema(): SchemaCatalog = SchemaCatalog.of( From e1c9387e4ebd3fe33e34fd2f8411a8fff38a9f49 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Mon, 31 Aug 2026 01:20:19 -0400 Subject: [PATCH 3/8] Persist stamp provenance and declared aliases in the Drivine version store Origin is taken by the first save that carries one and never moved; lastStamped moves only on a non-null incoming value; both rules are coalesce expressions inside the existing MERGE, so a routine re-stamp with no provenance rewrites neither. Alias fields serialize only when non-empty and decode absent-as-empty, so an alias-free stamp writes byte-identical node properties to the pre-alias writer and every old row reads back through the strict hash recompute. The in-memory reference store implements the same contract, proven by one shared suite against both backends. --- CHANGELOG.md | 36 +- .../InMemoryMetamodelVersionStore.kt | 68 ++++ .../dice/metamodel/MetamodelVersionStore.kt | 18 + .../metamodel/MetamodelVersionStoreTest.kt | 33 +- .../storage/DrivineMetamodelVersionStore.kt | 33 +- .../dice/storage/MetamodelRowMappers.kt | 127 ++++++- ...stractMetamodelVersionStoreContractTest.kt | 219 +++++++++++++ ...odelVersionStoreContractIntegrationTest.kt | 59 ++++ ...ineMetamodelVersionStoreIntegrationTest.kt | 310 ++++++++++++++++++ ...MemoryMetamodelVersionStoreContractTest.kt | 28 ++ .../dice/storage/MetamodelRowMapperTest.kt | 185 +++++++++++ docs/design/metamodel-versioning.md | 56 +++- 12 files changed, 1137 insertions(+), 35 deletions(-) create mode 100644 dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt create mode 100644 dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt create mode 100644 dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreContractIntegrationTest.kt create mode 100644 dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryMetamodelVersionStoreContractTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 677af53d..773ade0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,5 +63,37 @@ and the consumer PRs that deliver it). of one version leave one node. Hosts must declare three uniqueness constraints: `MetamodelVersion(schemaName, contentHash)`, `MetamodelSchemaCounter(schemaName)`, and `MetamodelVersion(schemaName, sequence)`. - **Compatibility: additive.** New class, and a new `dice-storage` → `dice-metamodel` - module dependency; no existing API touched. + Declared aliases persist at both levels: the version-level `entityTypeAliases` map + as its own node property, and a property signature's former names as a fifth + `aliases` field inside the stored signature. Both are written only when they hold + something, so an alias-free stamp writes exactly the properties this mapper wrote + before aliases existed, and a node from that older build reads back as a stamp + declaring none. Aliases feed `contentHash`, and the mapper recomputes the hash from + the persisted fields, so a stamp that failed to store them would be unreadable for + good — pinned by an integration test that writes a row in the old four-field shape + through raw Cypher and reads it back, one that round-trips a stamp carrying both + alias kinds, and one that removes the stored alias map and asserts the integrity + check rejects the row. + Stamp provenance persists too, and is the one part of a stamp a re-save does not + overwrite — **EXPERIMENTAL** (shape may change before 1.0): `origin` is + first-write-wins, set only when the stored row has none, and `lastStamped` moves + only when the incoming value is non-null. A re-stamp carrying no provenance leaves + both alone, which is what a scheduled drift check does on every pass, so a routine + check can neither erase the recorded cause nor replace it with its own identity. + Both rules are `coalesce` expressions inside the MERGE, so they hold under + concurrency without a read followed by a write. `savedAt` and `savedAtEpochMillis` + keep their existing behavior: set on create, untouched by a re-save. `origin` and + `lastStamped` are not hashed, so neither rule can move a stamp off its natural key. + Each is stored as a JSON object, which keeps a `StampProvenance()` with both fields + unset distinguishable from no provenance at all. `StampProvenance`'s 256-character + cap needs no column sizing here, since a Neo4j string property has no declared + width; a byte-sized backend still needs room for the up-to-1024 UTF-8 bytes. + `MetamodelVersionStore.saveVersion`'s KDoc now states both rules as contract, and + `dice-metamodel` gains `InMemoryMetamodelVersionStore`, the reference + implementation that applies them, promoted from a private class in that module's + own tests. `AbstractMetamodelVersionStoreContractTest` runs one suite against both + stores. + **Compatibility: additive.** New classes, and a new `dice-storage` → `dice-metamodel` + module dependency; no existing API touched. Stored nodes stay readable: every + property that existed before keeps its name, meaning, and encoding, and the four + new ones are absent when nothing declares them. diff --git a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt new file mode 100644 index 00000000..1db76dbf --- /dev/null +++ b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt @@ -0,0 +1,68 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.metamodel + +/** + * Reference [MetamodelVersionStore] that keeps stamps in a list. + * + * It is the executable statement of what the contract means, so a durable backend can be held to + * the same suite of tests. It also lets a host stamp and compare schemas before it has a database, + * which is most of what the first tier of versioning is for. + * + * Nothing here survives the JVM, and two instances know nothing about each other. + */ +class InMemoryMetamodelVersionStore : MetamodelVersionStore { + + private val saved = mutableListOf() + + /** + * Upsert on `(schemaName, contentHash)`, applying the contract's two provenance rules: `origin` + * is kept once the stored stamp has one, and `lastStamped` moves only when [version] carries a + * value. A stamp that is already there keeps its place in the write order, so re-saving an old + * version doesn't make it the latest. + * + * Everything runs under the list's own lock, so two threads re-stamping one version can't + * interleave the read of the stored provenance with the write that replaces it. + */ + override fun saveVersion(version: MetamodelVersion) { + synchronized(saved) { + val at = saved.indexOfFirst { + it.schemaName == version.schemaName && it.contentHash == version.contentHash + } + if (at < 0) { + saved += version + return + } + val stored = saved[at] + saved[at] = MetamodelVersion( + schemaName = version.schemaName, + entityTypeNames = version.entityTypeNames, + entityTypeLabels = version.entityTypeLabels, + entityTypeProperties = version.entityTypeProperties, + relationshipNames = version.relationshipNames, + entityTypeAliases = version.entityTypeAliases, + origin = stored.origin ?: version.origin, + lastStamped = version.lastStamped ?: stored.lastStamped, + ) + } + } + + override fun latestVersion(schemaName: String): MetamodelVersion? = + versionHistory(schemaName).firstOrNull() + + override fun versionHistory(schemaName: String): List = + synchronized(saved) { saved.filter { it.schemaName == schemaName }.reversed() } +} diff --git a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt index 62e06587..a14b17fe 100644 --- a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt +++ b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt @@ -49,6 +49,24 @@ interface MetamodelVersionStore { * Save a version stamp, keyed on `(schemaName, contentHash)`. Saving the same version twice * leaves one stored version. * + * **Provenance survives routine re-saves.** [MetamodelVersion.origin] and + * [MetamodelVersion.lastStamped] are the two fields a re-save does not simply overwrite: + * + * - `origin` is first-write-wins. An implementation sets it only when the stored stamp has + * none, so the cause of the first stamp stands however many times the schema is re-stamped. + * - `lastStamped` moves only when the incoming stamp carries a value. + * - A re-save whose provenance is null on both fields leaves both stored fields untouched. + * That is what a scheduled drift check does on every pass, and the rule is what stops it + * erasing the recorded cause or replacing it with its own identity. + * + * Neither field is hashed, so neither rule can move a stamp off its natural key. Everything + * else about a stored stamp is content the key already determines, so a re-save overwrites it + * with an identical value. + * + * Whatever an implementation records as the moment of the save keeps its existing value on a + * re-save, along with the stamp's place in the write order: a re-saved old stamp does not + * become the latest. + * * @param version The version to save. */ fun saveVersion(version: MetamodelVersion) diff --git a/dice-metamodel/src/test/kotlin/com/embabel/dice/metamodel/MetamodelVersionStoreTest.kt b/dice-metamodel/src/test/kotlin/com/embabel/dice/metamodel/MetamodelVersionStoreTest.kt index e83a0f88..bb141e05 100644 --- a/dice-metamodel/src/test/kotlin/com/embabel/dice/metamodel/MetamodelVersionStoreTest.kt +++ b/dice-metamodel/src/test/kotlin/com/embabel/dice/metamodel/MetamodelVersionStoreTest.kt @@ -24,28 +24,13 @@ import org.junit.jupiter.api.Test * Covers the one piece of behaviour the contract itself ships: the default [findVersion], which a * backend is free to override with a keyed lookup. A store implementation gets its own tests * wherever it lives. + * + * The provenance rules [MetamodelVersionStore.saveVersion] states are checked by + * `AbstractMetamodelVersionStoreContractTest`, which runs the same suite against + * [InMemoryMetamodelVersionStore] and the graph-backed store. */ class MetamodelVersionStoreTest { - /** Minimal store honouring the contract: upsert on (schemaName, contentHash), newest first. */ - private class InMemoryVersionStore : MetamodelVersionStore { - - private val saved = mutableListOf() - - override fun saveVersion(version: MetamodelVersion) { - // Idempotent: re-saving an existing version keeps its original position in write order. - if (saved.none { it.schemaName == version.schemaName && it.contentHash == version.contentHash }) { - saved.add(version) - } - } - - override fun latestVersion(schemaName: String): MetamodelVersion? = - versionHistory(schemaName).firstOrNull() - - override fun versionHistory(schemaName: String): List = - saved.filter { it.schemaName == schemaName }.reversed() - } - private fun version(schemaName: String, vararg typeNames: String): MetamodelVersion = MetamodelVersion.from( DataDictionary.fromDomainTypes(schemaName, typeNames.map { DynamicType(name = it) }), @@ -53,7 +38,7 @@ class MetamodelVersionStoreTest { @Test fun `findVersion returns the stamp with that hash`() { - val store = InMemoryVersionStore() + val store = InMemoryMetamodelVersionStore() val first = version("app", "Person") val second = version("app", "Person", "Company") store.saveVersion(first) @@ -67,7 +52,7 @@ class MetamodelVersionStoreTest { fun `findVersion is scoped to the schema name`() { // Two schemas can hold structurally identical versions, because the hash excludes the // name, so the lookup has to match on both halves of the key. - val store = InMemoryVersionStore() + val store = InMemoryMetamodelVersionStore() val mine = version("mine", "Person") store.saveVersion(mine) @@ -77,7 +62,7 @@ class MetamodelVersionStoreTest { @Test fun `findVersion returns null for an unknown hash`() { - val store = InMemoryVersionStore() + val store = InMemoryMetamodelVersionStore() store.saveVersion(version("app", "Person")) assertNull(store.findVersion("app", "not-a-hash")) @@ -85,7 +70,7 @@ class MetamodelVersionStoreTest { @Test fun `re-saving a version leaves one record, not two`() { - val store = InMemoryVersionStore() + val store = InMemoryMetamodelVersionStore() val v = version("app", "Person") store.saveVersion(v) store.saveVersion(v) @@ -96,7 +81,7 @@ class MetamodelVersionStoreTest { @Test fun `an empty store has no latest version and an empty history`() { - val store = InMemoryVersionStore() + val store = InMemoryMetamodelVersionStore() assertNull(store.latestVersion("app")) assertEquals(emptyList(), store.versionHistory("app")) diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt index 51f388cf..d27bb631 100644 --- a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt @@ -48,6 +48,19 @@ import java.time.Clock * holds an old stamp at its original position. The in-memory reference implementation behaves the * same way. * + * Stamp provenance is the one part of a version a re-save does not overwrite. `origin` is + * first-write-wins: the stored value stands, and the incoming one is taken only when the node has + * none. `lastStamped` moves only when the incoming stamp carries a value. A re-stamp supplying no + * provenance therefore leaves both alone, which is what every routine drift-check re-stamp does, so + * a scheduled check can neither erase the recorded cause nor replace it with its own identity. + * `MetamodelVersion.origin` and `lastStamped` are not hashed, so neither rule can move a version + * off its natural key. + * + * Both rules are conditional expressions inside the MERGE, so they hold under concurrency: nothing + * reads the stored provenance in one statement and writes it back in another. + * `InMemoryMetamodelVersionStore` applies the same two rules, and + * `AbstractMetamodelVersionStoreContractTest` runs the same suite against both. + * * Three uniqueness constraints are required, and the host declares them in a `SchemaCatalog` bean * (the module's `TestApplication` shows the shape): * - `UniquenessConstraintSpec("MetamodelVersion", listOf("schemaName", "contentHash"))` makes the @@ -80,6 +93,21 @@ open class DrivineMetamodelVersionStore( * away, so the counter stays put and the existing sequence is kept; only the content is * refreshed. * + * The two provenance properties are set through `coalesce`, which is where the store's + * first-write-wins and update-only-when-supplied rules live: + * - `n.origin = coalesce(n.origin, $origin)` keeps whatever the node already holds. On a + * create there is nothing to keep, so the incoming value lands; a `$origin` of null on a + * node that has none is a set-to-null, which leaves no property behind. + * - `n.lastStamped = coalesce($lastStamped, n.lastStamped)` prefers the incoming value and + * falls back to the stored one, so a null incoming value keeps what is there. + * + * Both are read and written inside one statement, so two concurrent saves can't interleave + * a read of the stored value with the write that replaces it. + * + * `entityTypeAliases` binds null when the version declares no former names. Setting a + * property to null removes it, so an alias-free stamp leaves a node with no such property, + * which is what a writer from before aliases existed left. + * * `SET c.lockedBy = $contentHash` writes a property nobody reads. It takes the exclusive * lock on the counter before `SET c.sequence = coalesce(c.sequence, 0) + 1` reads it. That * increment is a read-modify-write, whose textbook failure is two concurrent saves both @@ -100,7 +128,10 @@ open class DrivineMetamodelVersionStore( SET n.entityTypeNames = ${'$'}entityTypeNames, n.entityTypeLabels = ${'$'}entityTypeLabels, n.entityTypeProperties = ${'$'}entityTypeProperties, - n.relationshipNames = ${'$'}relationshipNames + n.relationshipNames = ${'$'}relationshipNames, + n.entityTypeAliases = ${'$'}entityTypeAliases, + n.origin = coalesce(n.origin, ${'$'}origin), + n.lastStamped = coalesce(${'$'}lastStamped, n.lastStamped) WITH n WHERE n.sequence IS NULL MERGE (c:MetamodelSchemaCounter {schemaName: ${'$'}schemaName}) diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt index d132fd53..f674c52c 100644 --- a/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt @@ -18,6 +18,7 @@ package com.embabel.dice.storage import com.embabel.agent.core.Cardinality import com.embabel.dice.metamodel.MetamodelVersion import com.embabel.dice.metamodel.PropertySignature +import com.embabel.dice.metamodel.StampProvenance import com.fasterxml.jackson.databind.ObjectMapper import java.time.Instant @@ -46,6 +47,14 @@ private val objectMapper = ObjectMapper() * Reads are strict. A property this mapper wrote must be present when it is read again; a node * missing one is corrupt, so the accessor throws and the store's surrounding guard skips the row * with a warning. + * + * Four things are optional, and absent means "none of these were declared": the version-level + * `entityTypeAliases` property, the `aliases` field inside a stored property signature, and the + * `origin` and `lastStamped` properties. Writing them only when they hold something means an + * alias-free stamp with no provenance stores exactly the properties this mapper stored before any + * of the four existed, and a node written by that older build reads back here as a stamp declaring + * none of them. Aliases feed the content hash, so a stamp carrying them and failing to store them + * would fail its own integrity check on the way back in and be unreadable for good. */ object MetamodelVersionRowMapper { @@ -54,6 +63,10 @@ object MetamodelVersionRowMapper { * * [savedAt] is a parameter, so this stays a pure function of its arguments and a test can pin * the instant a version was stored at. + * + * `entityTypeAliases`, `origin` and `lastStamped` bind `null` when the version declares none. + * A Cypher `SET` of `null` leaves no property behind, which is the encoding the read side + * expects and the shape an older writer left. */ fun bindMap(version: MetamodelVersion, savedAt: Instant): Map = mapOf( "schemaName" to version.schemaName, @@ -62,6 +75,9 @@ object MetamodelVersionRowMapper { "entityTypeLabels" to serializeMapOfLabelSets(version.entityTypeLabels), "entityTypeProperties" to serializeMapOfSignatureSets(version.entityTypeProperties), "relationshipNames" to serializeList(version.relationshipNames), + "entityTypeAliases" to serializeAliasMap(version.entityTypeAliases), + "origin" to serializeProvenance(version.origin), + "lastStamped" to serializeProvenance(version.lastStamped), "savedAt" to savedAt.toString(), "savedAtEpochMillis" to savedAt.toEpochMilli(), ) @@ -76,6 +92,11 @@ object MetamodelVersionRowMapper { * format, hand-edited, or corrupted, so this throws and the caller skips it. The stored hash is * also half the natural key, so a mismatch also means a re-save of the same content lands on a * different node. + * + * Aliases are part of that derivation, at both levels, so a node that dropped either alias + * field fails here rather than reading back as an alias-free stamp with the wrong hash. + * Provenance is not, so a node's `origin` and `lastStamped` are carried through untouched by + * the check. */ fun fromRow(row: Map<*, *>): MetamodelVersion { val storedHash = row.str("contentHash") @@ -85,6 +106,9 @@ object MetamodelVersionRowMapper { entityTypeLabels = deserializeMapOfLabelSets(row.str("entityTypeLabels")), entityTypeProperties = deserializeMapOfSignatureSets(row.str("entityTypeProperties")), relationshipNames = deserializeList(row.str("relationshipNames")), + entityTypeAliases = deserializeAliasMap(row.optionalStr("entityTypeAliases")), + origin = deserializeProvenance(row.optionalStr("origin"), "origin"), + lastStamped = deserializeProvenance(row.optionalStr("lastStamped"), "lastStamped"), ) require(version.contentHash == storedHash) { "MetamodelVersion '${version.schemaName}' fails its integrity check: stored contentHash " + @@ -116,6 +140,71 @@ private fun deserializeList(serialized: String): List = private fun serializeMapOfLabelSets(map: Map>): String = objectMapper.writeValueAsString(map.toSortedMap().mapValues { (_, labels) -> labels.sorted() }) +/** + * Serialize the former names each entity type goes by, in the same shape as the label sets, and + * write nothing at all when no type declares any. + * + * The empty case has to leave no property behind. This map feeds the content hash, and a stamp that + * declares no aliases hashes to the same digest it did before aliases existed, so its node must + * also look the way the older writer left it — otherwise the two spellings of one schema are two + * different-looking nodes on the same key. + */ +private fun serializeAliasMap(aliases: Map>): String? = + if (aliases.isEmpty()) null else serializeMapOfLabelSets(aliases) + +/** Inverse of [serializeAliasMap]; an absent property means no type declared a former name. */ +private fun deserializeAliasMap(serialized: String?): Map> = + if (serialized.isNullOrEmpty()) emptyMap() else deserializeMapOfLabelSets(serialized) + +/** + * Serialize a [StampProvenance] as `{"actor": ..., "trigger": ...}`, and write nothing when the + * stamp carries none. + * + * A JSON object rather than two scalar properties, because `StampProvenance()` with both fields + * unset is a real provenance and has to stay distinguishable from no provenance at all; two scalar + * properties would encode both as two absent values. + * + * Nothing here sizes the value. [StampProvenance] caps `actor` and `trigger` at 256 characters, and + * a Neo4j string property has no declared width, so there is no column to size. A backend that + * stores them in a byte-sized column needs room for the up-to-1024 UTF-8 bytes 256 characters can + * take. + */ +private fun serializeProvenance(provenance: StampProvenance?): String? = provenance?.let { + objectMapper.writeValueAsString(linkedMapOf("actor" to it.actor, "trigger" to it.trigger)) +} + +/** + * Inverse of [serializeProvenance]. An absent property means the stamp carried no provenance, which + * is what a node written before provenance existed looks like. + * + * Malformed content throws, naming the [property] it came from. The character cap is re-applied by + * [StampProvenance]'s own constructor, so a hand-edit that pushes `actor` past it makes the node + * unreadable and the store skips it with a warning rather than handing back a value the model says + * is impossible. + */ +private fun deserializeProvenance(serialized: String?, property: String): StampProvenance? { + if (serialized.isNullOrEmpty()) return null + val parsed = objectMapper.readValue(serialized, Any::class.java) + val fields = parsed as? Map<*, *> ?: throw IllegalArgumentException( + "the stored '$property' is a ${parsed?.javaClass?.simpleName ?: "null"} where a provenance " + + "object with 'actor' and 'trigger' was expected" + ) + return StampProvenance( + actor = fields.provenanceField(property, "actor"), + trigger = fields.provenanceField(property, "trigger"), + ) +} + +/** Read one nullable field of a stored provenance object, refusing anything that isn't a string. */ +private fun Map<*, *>.provenanceField(property: String, field: String): String? = + when (val value = this[field]) { + null -> null + is String -> value + else -> throw IllegalArgumentException( + "the '$field' of the stored '$property' is a ${value.javaClass.simpleName} where a string was expected" + ) + } + /** Inverse of [serializeMapOfLabelSets]. */ private fun deserializeMapOfLabelSets(serialized: String): Map> { if (serialized.isEmpty()) return emptyMap() @@ -139,6 +228,11 @@ private fun deserializeMapOfLabelSets(serialized: String): Map> signatures.sorted().map { signature -> // A LinkedHashMap, so the keys land in the JSON in this order and the encoding is // fully determined by the content. - linkedMapOf( + linkedMapOf( "name" to signature.name, "kind" to signature.kind.name, "type" to signature.type, "cardinality" to signature.cardinality.name, - ) + ).apply { + if (signature.aliases.isNotEmpty()) put("aliases", signature.aliases.sorted()) + } } } ) @@ -183,6 +279,7 @@ private fun deserializeMapOfSignatureSets(serialized: String): Map.signatureField(typeName: String, field: String): String = "a property signature for '$typeName' is missing its '$field' field" ) +/** + * Read a stored signature's former names. An absent `aliases` field means none were declared, which + * is every signature written before aliases existed. Anything present but not a list of names + * throws: aliases are part of the signature and feed the content hash, so quietly dropping a + * malformed one would surface later as a hash mismatch instead. + */ +private fun Map<*, *>.signatureAliases(typeName: String): Set { + val encoded = this["aliases"] ?: return emptySet() + val names = encoded as? List<*> ?: throw IllegalArgumentException( + "a property signature for '$typeName' has an 'aliases' field holding a " + + "${encoded.javaClass.simpleName} where a list of former names was expected" + ) + return names.map { name -> + name?.toString() ?: throw IllegalArgumentException( + "a property signature for '$typeName' has a null entry in its 'aliases' field" + ) + }.toSet() +} + /** Turn a stored enum constant name back into the constant, naming what failed if it's unknown. */ private inline fun > enumConstant(stored: String, typeName: String, field: String): E = enumValues().firstOrNull { it.name == stored } ?: throw IllegalArgumentException( @@ -211,3 +327,10 @@ private inline fun > enumConstant(stored: String, typeName: */ private fun Map<*, *>.str(key: String): String = this[key]?.toString() ?: throw IllegalArgumentException("required property '$key' is missing from the stored node") + +/** + * Read a property that may legitimately not be there, where absent means the stamp declared nothing + * to put in it. Only the alias map and the two provenance fields are read this way; everything else + * goes through [str]. + */ +private fun Map<*, *>.optionalStr(key: String): String? = this[key]?.toString() diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt new file mode 100644 index 00000000..0db50f53 --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt @@ -0,0 +1,219 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.storage + +import com.embabel.dice.metamodel.MetamodelVersion +import com.embabel.dice.metamodel.MetamodelVersionStore +import com.embabel.dice.metamodel.StampProvenance +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Test + +/** + * Cross-backend contract for [MetamodelVersionStore.saveVersion]'s upsert, and for the two + * provenance rules it states. Each subclass supplies a store and inherits the whole suite, so a + * backend that disagrees with the in-memory reference fails at authoring time. + * + * The provenance rules exist because the drift check re-stamps its schema on every pass and carries + * no provenance when it does. A store that overwrote both fields on every save would blank the + * recorded cause within a deploy cycle, and one that stamped its own identity into `lastStamped` + * would launder it, so both halves get a test here. + */ +abstract class AbstractMetamodelVersionStoreContractTest { + + /** A store holding nothing for the schema names below. */ + protected abstract fun store(): MetamodelVersionStore + + /** + * A stamp of one entity type. Provenance is never hashed, so two calls differing only in + * [origin] or [lastStamped] land on the same natural key, which is what a re-stamp is. + */ + private fun version( + schemaName: String, + typeName: String = "Person", + origin: StampProvenance? = null, + lastStamped: StampProvenance? = null, + ): MetamodelVersion = MetamodelVersion( + schemaName = schemaName, + entityTypeNames = listOf(typeName), + entityTypeLabels = emptyMap(), + entityTypeProperties = emptyMap(), + relationshipNames = emptyList(), + entityTypeAliases = emptyMap(), + origin = origin, + lastStamped = lastStamped, + ) + + // ---- the upsert the provenance rules sit on ---- + + @Test + fun `re-saving a version leaves one record`() { + val store = store() + val schemaName = "contract-idempotent" + val stamp = version(schemaName) + + store.saveVersion(stamp) + store.saveVersion(stamp) + + assertEquals(listOf(stamp), store.versionHistory(schemaName)) + assertEquals(stamp, store.latestVersion(schemaName)) + } + + // ---- provenance is written and read back ---- + + @Test + fun `a first save keeps the provenance it was given`() { + val store = store() + val schemaName = "contract-provenance-first" + val cause = StampProvenance("deploy-pipeline", "release-42") + + store.saveVersion(version(schemaName, origin = cause, lastStamped = cause)) + + val reloaded = store.latestVersion(schemaName)!! + assertEquals(cause, reloaded.origin) + assertEquals(cause, reloaded.lastStamped) + } + + @Test + fun `a provenance field the host left unset stays unset`() { + val store = store() + val schemaName = "contract-provenance-partial" + + store.saveVersion(version(schemaName, origin = StampProvenance(actor = "operator"))) + + val reloaded = store.latestVersion(schemaName)!! + assertEquals("operator", reloaded.origin!!.actor) + assertNull(reloaded.origin!!.trigger) + assertNull(reloaded.lastStamped) + } + + @Test + fun `a provenance with neither field set is still a provenance`() { + // StampProvenance() says "the host recorded a cause and named nothing in it", which is not + // the same as recording no cause at all. A store that flattened the two would answer the + // question "was this stamp taken by something that reports provenance" wrongly. + val store = store() + val schemaName = "contract-provenance-empty" + + store.saveVersion(version(schemaName, origin = StampProvenance())) + + val reloaded = store.latestVersion(schemaName)!! + assertNotNull(reloaded.origin, "an empty provenance must not read back as no provenance") + assertNull(reloaded.origin!!.actor) + assertNull(reloaded.origin!!.trigger) + } + + // ---- the two rules ---- + + @Test + fun `a re-stamp carrying no provenance leaves both fields alone`() { + // Every routine drift-check re-stamp arrives like this. + val store = store() + val schemaName = "contract-provenance-null-restamp" + val cause = StampProvenance("operator", "first-stamp") + store.saveVersion(version(schemaName, origin = cause, lastStamped = cause)) + + store.saveVersion(version(schemaName)) + + val reloaded = store.latestVersion(schemaName)!! + assertEquals(cause, reloaded.origin, "a null re-stamp must not erase the original cause") + assertEquals(cause, reloaded.lastStamped, "nor the most recent one") + } + + @Test + fun `a re-stamp carrying provenance keeps origin and moves lastStamped`() { + val store = store() + val schemaName = "contract-provenance-restamp" + val first = StampProvenance("bootstrap", "first-boot") + val second = StampProvenance("operator", "manual-restamp") + store.saveVersion(version(schemaName, origin = first, lastStamped = first)) + + store.saveVersion(version(schemaName, origin = second, lastStamped = second)) + + val reloaded = store.latestVersion(schemaName)!! + assertEquals(first, reloaded.origin, "origin is first-write-wins") + assertEquals(second, reloaded.lastStamped, "lastStamped follows the newest save that names one") + } + + @Test + fun `origin is taken by the first save that carries one and not moved after`() { + val store = store() + val schemaName = "contract-provenance-first-write-wins" + + store.saveVersion(version(schemaName)) + assertNull(store.latestVersion(schemaName)!!.origin, "nothing was supplied, so nothing is recorded") + + val backfilled = StampProvenance("operator", "backfill") + store.saveVersion(version(schemaName, origin = backfilled)) + assertEquals(backfilled, store.latestVersion(schemaName)!!.origin, "a stamp with no origin takes one") + + store.saveVersion(version(schemaName, origin = StampProvenance("someone-else", "later"))) + assertEquals(backfilled, store.latestVersion(schemaName)!!.origin, "and never gives it up again") + } + + @Test + fun `lastStamped moves without an origin ever being supplied`() { + val store = store() + val schemaName = "contract-provenance-last-only" + store.saveVersion(version(schemaName, lastStamped = StampProvenance("first", "run-1"))) + + store.saveVersion(version(schemaName, lastStamped = StampProvenance("second", "run-2"))) + + val reloaded = store.latestVersion(schemaName)!! + assertNull(reloaded.origin) + assertEquals(StampProvenance("second", "run-2"), reloaded.lastStamped) + } + + // ---- provenance belongs to the stamp ---- + + @Test + fun `each stamp of a schema carries its own provenance`() { + val store = store() + val schemaName = "contract-provenance-per-stamp" + val first = version(schemaName, "First", origin = StampProvenance("first-cause")) + val second = version(schemaName, "Second", origin = StampProvenance("second-cause")) + + store.saveVersion(first) + store.saveVersion(second) + + assertEquals(StampProvenance("first-cause"), store.findVersion(schemaName, first.contentHash)!!.origin) + assertEquals(StampProvenance("second-cause"), store.findVersion(schemaName, second.contentHash)!!.origin) + } + + @Test + fun `a re-stamp for provenance leaves the stamp where it was in the history`() { + // Provenance is not hashed, so this write lands on an existing key. It has to behave like + // any other re-save: content refreshed, position in the write order untouched. + val store = store() + val schemaName = "contract-provenance-order" + val first = version(schemaName, "First", origin = StampProvenance("original")) + val second = version(schemaName, "Second") + store.saveVersion(first) + store.saveVersion(second) + + store.saveVersion(version(schemaName, "First", lastStamped = StampProvenance("re-stamped"))) + + assertEquals( + listOf("Second", "First"), + store.versionHistory(schemaName).map { it.entityTypeNames.single() }, + "a provenance re-stamp must not make an old version the latest", + ) + val reloadedFirst = store.findVersion(schemaName, first.contentHash)!! + assertEquals(StampProvenance("original"), reloadedFirst.origin) + assertEquals(StampProvenance("re-stamped"), reloadedFirst.lastStamped) + } +} diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreContractIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreContractIntegrationTest.kt new file mode 100644 index 00000000..e0eca4ba --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreContractIntegrationTest.kt @@ -0,0 +1,59 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.storage + +import com.embabel.dice.metamodel.MetamodelVersionStore +import org.drivine.manager.PersistenceManager +import org.drivine.query.QuerySpecification +import org.junit.jupiter.api.AfterEach +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.test.context.DynamicPropertyRegistry +import org.springframework.test.context.DynamicPropertySource + +/** + * Runs the [AbstractMetamodelVersionStoreContractTest] suite against the Neo4j-backed + * [DrivineMetamodelVersionStore] (testcontainer). This is the half that catches the graph backend + * disagreeing with the in-memory reference on the provenance rules, which is easy to do: they live + * in a Cypher `coalesce` there and in Kotlin here. + * + * Uses the shared [Neo4jTestContainer]; see that class for why Drivine's built-in testcontainer + * is bypassed. + */ +@SpringBootTest(classes = [TestApplication::class]) +class DrivineMetamodelVersionStoreContractIntegrationTest : AbstractMetamodelVersionStoreContractTest() { + + companion object { + @JvmStatic + @DynamicPropertySource + fun neo4jProperties(registry: DynamicPropertyRegistry) = Neo4jTestContainer.registerProperties(registry) + } + + @Autowired + private lateinit var graphStore: DrivineMetamodelVersionStore + + @Autowired + private lateinit var persistenceManager: PersistenceManager + + override fun store(): MetamodelVersionStore = graphStore + + @AfterEach + fun cleanUp() { + listOf("MetamodelVersion", "MetamodelSchemaCounter").forEach { label -> + persistenceManager.execute(QuerySpecification.withStatement("MATCH (n:$label) DETACH DELETE n")) + } + } +} diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt index 88ae1525..63952d5a 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt @@ -23,11 +23,13 @@ import com.embabel.agent.core.Cardinality import com.embabel.dice.metamodel.MetamodelVersion import com.embabel.dice.metamodel.PropertySignature import com.embabel.dice.metamodel.PropertySignature.Kind +import com.embabel.dice.metamodel.StampProvenance import org.drivine.manager.PersistenceManager import org.drivine.query.QuerySpecification import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertNotEquals +import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test @@ -606,8 +608,316 @@ class DrivineMetamodelVersionStoreIntegrationTest { } } + // ---- Stamp provenance on the node ---- + + @Test + fun `provenance persists and reads back`() { + val schemaName = "provenance-schema" + val cause = StampProvenance("deploy-pipeline", "release-42") + + store.saveVersion(stampedBy(schemaName, "Person", origin = cause, lastStamped = cause)) + + val reloaded = store.latestVersion(schemaName)!! + assertEquals(cause, reloaded.origin) + assertEquals(cause, reloaded.lastStamped) + } + + @Test + fun `a re-stamp carrying no provenance rewrites neither stored property`() { + // The routine case: DefaultDriftCheckRunner re-stamps on every pass and supplies nothing. + // Asserted on the raw properties as well as the reloaded stamp, because a store that wrote + // null over both would still read back as "no provenance" and look plausible. + val schemaName = "provenance-untouched-schema" + val cause = StampProvenance("operator", "first-stamp") + val stamp = stampedBy(schemaName, "Person", origin = cause, lastStamped = cause) + store.saveVersion(stamp) + val before = storedProvenanceProperties(schemaName, stamp) + assertEquals( + """{"actor":"operator","trigger":"first-stamp"}|{"actor":"operator","trigger":"first-stamp"}""", + before, + "precondition: both properties are on the node", + ) + + store.saveVersion(stampedBy(schemaName, "Person")) + + assertEquals(before, storedProvenanceProperties(schemaName, stamp), "neither property may be rewritten") + val reloaded = store.latestVersion(schemaName)!! + assertEquals(cause, reloaded.origin) + assertEquals(cause, reloaded.lastStamped) + } + + @Test + fun `a re-stamp carrying provenance keeps the stored origin and moves lastStamped`() { + val schemaName = "provenance-restamp-schema" + val first = StampProvenance("bootstrap", "first-boot") + val second = StampProvenance("operator", "manual-restamp") + val stamp = stampedBy(schemaName, "Person", origin = first, lastStamped = first) + store.saveVersion(stamp) + + store.saveVersion(stampedBy(schemaName, "Person", origin = second, lastStamped = second)) + + assertEquals( + """{"actor":"bootstrap","trigger":"first-boot"}|{"actor":"operator","trigger":"manual-restamp"}""", + storedProvenanceProperties(schemaName, stamp), + ) + val reloaded = store.latestVersion(schemaName)!! + assertEquals(first, reloaded.origin) + assertEquals(second, reloaded.lastStamped) + } + + @Test + fun `a stamp with no provenance leaves no provenance properties on the node`() { + // What a node written before provenance existed looks like, produced by the current writer. + val schemaName = "provenance-absent-schema" + val stamp = stampedBy(schemaName, "Person") + + store.saveVersion(stamp) + + assertEquals("|", storedProvenanceProperties(schemaName, stamp)) + val reloaded = store.latestVersion(schemaName)!! + assertNull(reloaded.origin) + assertNull(reloaded.lastStamped) + } + + @Test + fun `a provenance whose fields are both unset survives as a provenance`() { + // Two scalar properties could not tell this apart from the test above; the JSON object can. + val schemaName = "provenance-empty-schema" + + store.saveVersion(stampedBy(schemaName, "Person", origin = StampProvenance())) + + val reloaded = store.latestVersion(schemaName)!! + assertNotNull(reloaded.origin, "an empty provenance must not read back as no provenance") + assertNull(reloaded.origin!!.actor) + assertNull(reloaded.origin!!.trigger) + } + + // ---- Aliases, and rows written before they existed ---- + + @Test + fun `a version carrying both kinds of alias round-trips with its integrity check passing`() { + val schemaName = "alias-round-trip-schema" + val version = MetamodelVersion( + schemaName = schemaName, + entityTypeNames = listOf("Organisation"), + entityTypeLabels = mapOf("Organisation" to setOf("Entity")), + entityTypeProperties = mapOf( + "Organisation" to setOf( + PropertySignature("legalName", Kind.VALUE, "string", Cardinality.ONE, setOf("companyName", "name")), + PropertySignature("staff", Kind.REFERENCE, "Person", Cardinality.SET), + ), + ), + relationshipNames = listOf("Organisation-[EMPLOYS]->Person"), + entityTypeAliases = mapOf("Organisation" to setOf("Company", "Firm")), + ) + + store.saveVersion(version) + + // The mapper recomputes the hash from the persisted fields and throws on a mismatch, so a + // reloaded stamp at all is already proof that both alias kinds reached the node. + val reloaded = store.latestVersion(schemaName)!! + assertEquals(version, reloaded) + assertEquals(version.contentHash, reloaded.contentHash) + assertEquals(mapOf("Organisation" to setOf("Company", "Firm")), reloaded.entityTypeAliases) + assertEquals( + setOf("companyName", "name"), + reloaded.entityTypeProperties["Organisation"]!!.single { it.name == "legalName" }.aliases, + ) + assertEquals( + emptySet(), + reloaded.entityTypeProperties["Organisation"]!!.single { it.name == "staff" }.aliases, + ) + } + + @Test + fun `a type-aliased version whose alias map is missing from the node cannot be read back`() { + // Why the map has to be stored: it feeds the content hash, so a writer that dropped it + // would produce nodes that fail their own checksum and are unreadable for good. Removing + // the property is exactly what that bug would leave behind. + val schemaName = "alias-map-dropped-schema" + val version = MetamodelVersion( + schemaName = schemaName, + entityTypeNames = listOf("Organisation"), + entityTypeLabels = emptyMap(), + entityTypeProperties = emptyMap(), + relationshipNames = emptyList(), + entityTypeAliases = mapOf("Organisation" to setOf("Company")), + ) + store.saveVersion(version) + assertEquals(version, store.latestVersion(schemaName), "precondition: it reads back while the map is stored") + + persistenceManager.execute( + QuerySpecification.withStatement( + """ + MATCH (n:MetamodelVersion {schemaName: ${'$'}schemaName}) + REMOVE n.entityTypeAliases + """.trimIndent(), + ).bind(mapOf("schemaName" to schemaName)), + ) + + val (history, logged) = capturingStoreWarnings { store.versionHistory(schemaName) } + + assertEquals(emptyList(), history) + assertTrue(logged.any { it.contains("fails its integrity check") }, "warnings were: $logged") + } + + @Test + fun `an alias-free stamp leaves no entityTypeAliases property on the node`() { + // The other half of the same rule. A stamp declaring no former names has to store the + // properties the writer stored before aliases existed, so the two spellings of one schema + // are one node rather than two shapes on one key. + val schemaName = "alias-free-schema" + val version = MetamodelVersion(schemaName, listOf("Person"), emptyMap(), emptyMap(), emptyList()) + + store.saveVersion(version) + + assertEquals("", storedProperty(schemaName, version, StoredProperty.ENTITY_TYPE_ALIASES)) + assertEquals(emptyMap>(), store.latestVersion(schemaName)!!.entityTypeAliases) + } + + @Test + fun `a node written in the old four-field shape reads back through the new mapper`() { + // A stamp saved before aliases and provenance existed: property signatures with exactly + // four fields, and no entityTypeAliases, origin or lastStamped. Written straight through + // Cypher, so nothing in the current writer can quietly supply the missing properties. + val schemaName = "old-shape-schema" + val expected = MetamodelVersion( + schemaName = schemaName, + entityTypeNames = listOf("Person"), + entityTypeLabels = mapOf("Person" to setOf("Agent")), + entityTypeProperties = mapOf( + "Person" to setOf( + PropertySignature("age", Kind.VALUE, "integer", Cardinality.OPTIONAL), + PropertySignature("name", Kind.VALUE, "string", Cardinality.ONE), + ), + ), + relationshipNames = listOf("Person-[WORKS_FOR]->Company"), + ) + persistenceManager.execute( + QuerySpecification.withStatement( + """ + CREATE (n:MetamodelVersion { + schemaName: ${'$'}schemaName, + contentHash: ${'$'}contentHash, + entityTypeNames: '["Person"]', + entityTypeLabels: '{"Person":["Agent"]}', + entityTypeProperties: ${'$'}entityTypeProperties, + relationshipNames: '["Person-[WORKS_FOR]->Company"]', + savedAt: '2026-01-01T00:00:00Z', + savedAtEpochMillis: 1767225600000, + sequence: 1 + }) + """.trimIndent(), + ).bind( + mapOf( + "schemaName" to schemaName, + "contentHash" to expected.contentHash, + "entityTypeProperties" to + """{"Person":[{"name":"age","kind":"VALUE","type":"integer","cardinality":"OPTIONAL"},""" + + """{"name":"name","kind":"VALUE","type":"string","cardinality":"ONE"}]}""", + ), + ), + ) + + val reloaded = store.latestVersion(schemaName) + + assertEquals(expected, reloaded, "an old-shape node must still read, and pass its integrity check") + assertEquals(expected.contentHash, reloaded!!.contentHash) + assertEquals(emptyMap>(), reloaded.entityTypeAliases) + assertNull(reloaded.origin) + assertNull(reloaded.lastStamped) + assertTrue( + reloaded.entityTypeProperties["Person"]!!.all { it.aliases.isEmpty() }, + "four-field signatures carry no former names", + ) + } + + @Test + fun `re-saving an old-shape node through the current writer leaves it in the old shape`() { + // The upgrade path: an application that boots against a graph written by an older build + // re-stamps its unchanged schema. The write lands on the existing node, and because the + // stamp declares no aliases and no provenance, none of the four new fields appears. + val schemaName = "old-shape-restamp-schema" + val version = MetamodelVersion( + schemaName = schemaName, + entityTypeNames = listOf("Person"), + entityTypeLabels = emptyMap(), + entityTypeProperties = mapOf( + "Person" to setOf(PropertySignature("name", Kind.VALUE, "string", Cardinality.ONE)), + ), + relationshipNames = emptyList(), + ) + store.saveVersion(version) + + store.saveVersion(version) + + assertEquals(1, rawNodeCount()) + assertEquals("", storedProperty(schemaName, version, StoredProperty.ENTITY_TYPE_ALIASES)) + assertEquals("|", storedProvenanceProperties(schemaName, version)) + assertEquals( + """{"Person":[{"name":"name","kind":"VALUE","type":"string","cardinality":"ONE"}]}""", + storedProperty(schemaName, version, StoredProperty.ENTITY_TYPE_PROPERTIES), + "a signature with no former names keeps its four fields", + ) + } + // ---- helpers ---- + /** A stamp of one entity type, carrying whatever provenance the test wants to record on it. */ + private fun stampedBy( + schemaName: String, + typeName: String, + origin: StampProvenance? = null, + lastStamped: StampProvenance? = null, + ): MetamodelVersion = MetamodelVersion( + schemaName = schemaName, + entityTypeNames = listOf(typeName), + entityTypeLabels = emptyMap(), + entityTypeProperties = emptyMap(), + relationshipNames = emptyList(), + entityTypeAliases = emptyMap(), + origin = origin, + lastStamped = lastStamped, + ) + + /** The stored properties a test can read back raw, each with the Cypher that returns it. */ + private enum class StoredProperty(val returnExpression: String) { + ENTITY_TYPE_ALIASES("coalesce(n.entityTypeAliases, '')"), + ENTITY_TYPE_PROPERTIES("n.entityTypeProperties"), + } + + /** + * Read one property straight off a version node, bypassing the mapper. `` stands for a + * property that isn't on the node, which is what a Cypher `SET` of null leaves behind and what + * these tests are checking for. + */ + private fun storedProperty(schemaName: String, version: MetamodelVersion, property: StoredProperty): String? = + persistenceManager.maybeGetOne( + QuerySpecification.withStatement( + """ + MATCH (n:MetamodelVersion {schemaName: ${'$'}schemaName, contentHash: ${'$'}contentHash}) + RETURN ${property.returnExpression} AS value + """.trimIndent(), + ).bind(mapOf("schemaName" to schemaName, "contentHash" to version.contentHash)) + .transform(String::class.java), + ) + + /** + * Both provenance properties as one `origin|lastStamped` string, so a test can assert that a + * re-save left the stored bytes exactly as they were. `` stands for a property that + * isn't on the node. + */ + private fun storedProvenanceProperties(schemaName: String, version: MetamodelVersion): String? = + persistenceManager.maybeGetOne( + QuerySpecification.withStatement( + """ + MATCH (n:MetamodelVersion {schemaName: ${'$'}schemaName, contentHash: ${'$'}contentHash}) + RETURN coalesce(n.origin, '') + '|' + coalesce(n.lastStamped, '') AS value + """.trimIndent(), + ).bind(mapOf("schemaName" to schemaName, "contentHash" to version.contentHash)) + .transform(String::class.java), + ) + /** Rewrite a version node's serialized entity type names, leaving its stored hash untouched. */ private fun tamperWithEntityTypeNames(schemaName: String, serializedNames: String) { persistenceManager.execute( diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryMetamodelVersionStoreContractTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryMetamodelVersionStoreContractTest.kt new file mode 100644 index 00000000..2afca2f1 --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryMetamodelVersionStoreContractTest.kt @@ -0,0 +1,28 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.storage + +import com.embabel.dice.metamodel.InMemoryMetamodelVersionStore +import com.embabel.dice.metamodel.MetamodelVersionStore + +/** + * Runs the [AbstractMetamodelVersionStoreContractTest] suite against the in-memory reference store. + * No Docker, so it runs in the normal test phase — the always-on half of the cross-backend check + * the graph IT completes. + */ +class InMemoryMetamodelVersionStoreContractTest : AbstractMetamodelVersionStoreContractTest() { + override fun store(): MetamodelVersionStore = InMemoryMetamodelVersionStore() +} diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt index 1da5e80e..6ef63dd6 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt @@ -19,7 +19,9 @@ import com.embabel.agent.core.Cardinality import com.embabel.dice.metamodel.MetamodelVersion import com.embabel.dice.metamodel.PropertySignature import com.embabel.dice.metamodel.PropertySignature.Kind +import com.embabel.dice.metamodel.StampProvenance import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows @@ -51,11 +53,39 @@ class MetamodelRowMapperTest { relationshipNames = listOf("WORKS_FOR"), ) + /** The same schema with both kinds of alias declared on it. */ + private val aliased = MetamodelVersion( + schemaName = "aliased-schema", + entityTypeNames = listOf("Organisation"), + entityTypeLabels = mapOf("Organisation" to setOf("Entity")), + entityTypeProperties = mapOf( + "Organisation" to setOf( + PropertySignature("legalName", Kind.VALUE, "string", Cardinality.ONE, setOf("companyName", "name")), + PropertySignature("staff", Kind.REFERENCE, "Person", Cardinality.SET), + ), + ), + relationshipNames = listOf("Organisation-[EMPLOYS]->Person"), + entityTypeAliases = mapOf("Organisation" to setOf("Company", "Firm")), + ) + private val savedAt = Instant.parse("2026-01-01T00:00:00.500Z") private fun row(savedAtInstant: Instant = savedAt): MutableMap = MetamodelVersionRowMapper.bindMap(version, savedAtInstant).toMutableMap() + /** [version] again, carrying provenance. The two are the same stamp: provenance isn't hashed. */ + private fun withProvenance(origin: StampProvenance?, lastStamped: StampProvenance?): MetamodelVersion = + MetamodelVersion( + schemaName = version.schemaName, + entityTypeNames = version.entityTypeNames, + entityTypeLabels = version.entityTypeLabels, + entityTypeProperties = version.entityTypeProperties, + relationshipNames = version.relationshipNames, + entityTypeAliases = version.entityTypeAliases, + origin = origin, + lastStamped = lastStamped, + ) + @Test fun `a version round-trips through its own property map`() { assertEquals(version, MetamodelVersionRowMapper.fromRow(row())) @@ -161,4 +191,159 @@ class MetamodelRowMapperTest { assertEquals(empty, MetamodelVersionRowMapper.fromRow(MetamodelVersionRowMapper.bindMap(empty, savedAt))) } + + // ---- Aliases: written only when declared, absent read as none ---- + + @Test + fun `a version declaring no aliases binds neither alias field`() { + // The four-field signature encoding and the absent alias map are what a writer from before + // aliases existed produced. Keeping the empty case byte-identical is what lets old nodes + // and new ones share a natural key. + assertNull(row()["entityTypeAliases"], "an empty alias map must leave no property behind") + assertEquals( + """{"Company":[{"name":"employs","kind":"REFERENCE","type":"Person","cardinality":"SET"}],""" + + """"Person":[{"name":"age","kind":"VALUE","type":"integer","cardinality":"OPTIONAL"},""" + + """{"name":"name","kind":"VALUE","type":"string","cardinality":"ONE"}]}""", + row()["entityTypeProperties"], + ) + } + + @Test + fun `both kinds of alias are written when declared, sorted, and round-trip`() { + val bound = MetamodelVersionRowMapper.bindMap(aliased, savedAt) + + assertEquals("""{"Organisation":["Company","Firm"]}""", bound["entityTypeAliases"]) + assertEquals( + """{"Organisation":[{"name":"legalName","kind":"VALUE","type":"string","cardinality":"ONE",""" + + """"aliases":["companyName","name"]},""" + + """{"name":"staff","kind":"REFERENCE","type":"Person","cardinality":"SET"}]}""", + bound["entityTypeProperties"], + "a signature with no former names keeps exactly four fields", + ) + + val reloaded = MetamodelVersionRowMapper.fromRow(bound) + assertEquals(aliased, reloaded) + assertEquals(aliased.contentHash, reloaded.contentHash) + assertEquals(mapOf("Organisation" to setOf("Company", "Firm")), reloaded.entityTypeAliases) + assertEquals( + setOf("companyName", "name"), + reloaded.entityTypeProperties["Organisation"]!!.single { it.name == "legalName" }.aliases, + ) + } + + @Test + fun `dropping the stored alias map fails the integrity check`() { + // Aliases feed the content hash, so this is the failure mode a mapper that forgot to write + // the map would produce on every read: the stamp is unreadable, not silently alias-free. + val corrupt = MetamodelVersionRowMapper.bindMap(aliased, savedAt).toMutableMap() + .apply { remove("entityTypeAliases") } + + val thrown = assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } + assertTrue(thrown.message!!.contains("integrity check"), thrown.message) + } + + @Test + fun `dropping a signature's stored aliases fails the integrity check`() { + val corrupt = MetamodelVersionRowMapper.bindMap(aliased, savedAt).toMutableMap().apply { + put( + "entityTypeProperties", + """{"Organisation":[{"name":"legalName","kind":"VALUE","type":"string","cardinality":"ONE"},""" + + """{"name":"staff","kind":"REFERENCE","type":"Person","cardinality":"SET"}]}""", + ) + } + + val thrown = assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } + assertTrue(thrown.message!!.contains("integrity check"), thrown.message) + } + + @Test + fun `a stored aliases field that is not a list of names fails the read, naming the type`() { + val corrupt = row().apply { + put( + "entityTypeProperties", + """{"Person":[{"name":"name","kind":"VALUE","type":"string","cardinality":"ONE","aliases":"nickname"}]}""", + ) + } + + val thrown = assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } + assertTrue(thrown.message!!.contains("aliases"), thrown.message) + assertTrue(thrown.message!!.contains("Person"), thrown.message) + } + + // ---- Provenance ---- + + @Test + fun `provenance round-trips, and a version carrying none binds neither property`() { + assertNull(row()["origin"]) + assertNull(row()["lastStamped"]) + assertNull(MetamodelVersionRowMapper.fromRow(row()).origin) + assertNull(MetamodelVersionRowMapper.fromRow(row()).lastStamped) + + val stamped = withProvenance(StampProvenance("deploy", "release-1"), StampProvenance("operator", null)) + val bound = MetamodelVersionRowMapper.bindMap(stamped, savedAt) + + assertEquals("""{"actor":"deploy","trigger":"release-1"}""", bound["origin"]) + assertEquals("""{"actor":"operator","trigger":null}""", bound["lastStamped"]) + + val reloaded = MetamodelVersionRowMapper.fromRow(bound) + assertEquals(StampProvenance("deploy", "release-1"), reloaded.origin) + assertEquals(StampProvenance("operator", null), reloaded.lastStamped) + } + + @Test + fun `a provenance with neither field set stays distinguishable from no provenance`() { + // The reason provenance is stored as an object rather than as two scalar properties: two + // scalars would encode StampProvenance() and null identically. + val bound = MetamodelVersionRowMapper.bindMap(withProvenance(StampProvenance(), null), savedAt) + + assertEquals("""{"actor":null,"trigger":null}""", bound["origin"]) + assertEquals(StampProvenance(), MetamodelVersionRowMapper.fromRow(bound).origin) + assertNull(MetamodelVersionRowMapper.fromRow(bound).lastStamped) + } + + @Test + fun `provenance stays out of the content hash, so a re-stamp lands on the same key`() { + val plain = MetamodelVersionRowMapper.bindMap(version, savedAt) + val stamped = MetamodelVersionRowMapper.bindMap( + withProvenance(StampProvenance("a", "b"), StampProvenance("c", "d")), + savedAt, + ) + + assertEquals(plain["contentHash"], stamped["contentHash"]) + } + + @Test + fun `a stored provenance that is not an object fails the read, naming the property`() { + val corrupt = row().apply { put("lastStamped", """"just-a-string"""") } + + val thrown = assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } + assertTrue(thrown.message!!.contains("lastStamped"), thrown.message) + } + + @Test + fun `a stored provenance field that is not a string fails the read, naming both`() { + val corrupt = row().apply { put("origin", """{"actor":42,"trigger":null}""") } + + val thrown = assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } + assertTrue(thrown.message!!.contains("actor"), thrown.message) + assertTrue(thrown.message!!.contains("origin"), thrown.message) + } + + @Test + fun `a row with no alias or provenance properties at all reads back as a stamp declaring none`() { + // A node written before any of the four existed. Removing the keys is the same thing the + // graph does when a property was never set. + val old = row().apply { + remove("entityTypeAliases") + remove("origin") + remove("lastStamped") + } + + val reloaded = MetamodelVersionRowMapper.fromRow(old) + + assertEquals(version, reloaded) + assertEquals(emptyMap>(), reloaded.entityTypeAliases) + assertNull(reloaded.origin) + assertNull(reloaded.lastStamped) + } } diff --git a/docs/design/metamodel-versioning.md b/docs/design/metamodel-versioning.md index c3b41767..a30bfce5 100644 --- a/docs/design/metamodel-versioning.md +++ b/docs/design/metamodel-versioning.md @@ -14,7 +14,8 @@ stamp against a live graph, comes later — see [the tiers ahead](#the-tiers-ahe The types live in `dice-metamodel`, a small pure-JVM module: `MetamodelVersion`, `GovernedTypeSelector`, `DeclaredSchema`/`DeclaredSchemaSource`, `SchemaAliases`, and the -`MetamodelVersionStore` contract. It depends on Embabel's agent core types and nothing else. +`MetamodelVersionStore` contract with its `InMemoryMetamodelVersionStore` reference +implementation. It depends on Embabel's agent core types and nothing else. `SchemaAliases` and the alias fields on `PropertySignature` and `MetamodelVersion` are experimental; their shape may change before 1.0. @@ -332,9 +333,11 @@ built for. `findVersion` resolves a recorded hash back to the schema shape it named. The default scans `versionHistory`, which is correct for any implementation but reads the whole history to answer a -keyed question; a backend that can push the lookup down to the database should override it. This -module ships no implementation. Storage is a separate concern, and a stamp is useful in memory -before anything durable exists. +keyed question; a backend that can push the lookup down to the database should override it. + +The only implementation this module ships is `InMemoryMetamodelVersionStore`, which keeps stamps in +a list. It exists so a host can stamp and compare schemas before it has a database, and so the +contract has an executable statement of what its rules mean; durable storage is a separate concern. The durable implementation lives in `dice-storage`. `DrivineMetamodelVersionStore` keeps each stamp as a `(:MetamodelVersion)` node and MERGEs on `(schemaName, contentHash)`, so re-stamping an @@ -353,8 +356,10 @@ unchanged schema updates the node already there. Three things govern how it beha on them. Because `(schemaName, sequence)` is unique, a lost counter update surfaces as a retryable failure. - **A re-save updates content only.** Sequence, counter, and `savedAt` keep their existing values, - so an old stamp stays at its original position in the history. The in-memory reference - implementation behaves the same way. + so an old stamp stays at its original position in the history. `InMemoryMetamodelVersionStore`, + the reference implementation `dice-metamodel` ships, behaves the same way. +- **Provenance survives a re-save.** `origin` and `lastStamped` are the two fields a re-save does + not overwrite; the rules are below. The structural fields are stored as JSON strings, since Neo4j properties are scalars and flat arrays. Property signatures get explicit named fields with enums by name @@ -363,6 +368,45 @@ re-point the day someone inserts a constant into `Cardinality`. The content hash `contentHash` on a node is a checksum: the store recomputes it on read and skips a node that disagrees with itself, logging a warning. +### Aliases and provenance in storage + +Declared aliases land in two places on the node: the version-level `entityTypeAliases` map as its +own property, and a property's former names as a fifth `aliases` field inside its stored signature. +Both are written only when they hold something, so a stamp that declares no former names writes +exactly the properties the store wrote before aliases existed, and a node from that older build +reads back as a stamp declaring none. + +Getting that wrong is unrecoverable. Aliases feed `contentHash`, and the read side recomputes the +hash from the persisted fields, so a writer that dropped the alias map would produce nodes that +fail their own checksum on every read and can never be read back. Three tests pin it: one writes a +row in the old four-field shape through raw Cypher and reads it back, one round-trips a stamp +carrying both alias kinds, and one deletes the stored alias map and asserts the integrity check +rejects the row. + +The two provenance pairs follow the rules the `MetamodelVersionStore` contract states: + +- `origin` is first-write-wins. It is set only when the stored node has none, so the cause of the + first stamp stands however many times the schema is re-stamped. +- `lastStamped` moves only when the incoming stamp carries a value. +- A re-stamp whose provenance is null on both fields leaves both stored fields alone. That is every + pass of the drift check, and the rule is what stops a scheduled job erasing the recorded cause or + writing its own identity over it. + +Both are `coalesce` expressions inside the MERGE — `n.origin = coalesce(n.origin, $origin)` and +`n.lastStamped = coalesce($lastStamped, n.lastStamped)` — so the rules hold under concurrency +without reading the stored value in one statement and writing it back in another. Neither field is +hashed, so neither rule can move a stamp off its natural key. + +Each pair is stored as a JSON object, which keeps a `StampProvenance()` with both fields unset +distinguishable from no provenance at all; two scalar properties would encode the two identically. +The 256-character cap needs no column sizing here, since a Neo4j string property has no declared +width. A backend that stores provenance in a byte-sized column still needs room for the up-to-1024 +UTF-8 bytes those characters can take. + +`AbstractMetamodelVersionStoreContractTest` runs one suite against the graph store and the in-memory +reference, so the two can't drift apart on rules that live in Cypher on one side and Kotlin on the +other. + ## Plain classes, not data classes `MetamodelVersion`, `DeclaredSchema` and `SchemaAliases` each write their own `equals`, `hashCode` From f54bb3275547f1934ef28add3c604222d0979dd1 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Mon, 31 Aug 2026 14:43:08 -0400 Subject: [PATCH 4/8] Remove stamp provenance persistence with the type --- CHANGELOG.md | 28 +-- .../InMemoryMetamodelVersionStore.kt | 27 +-- .../dice/metamodel/MetamodelVersionStore.kt | 15 +- .../metamodel/MetamodelVersionStoreTest.kt | 2 +- .../storage/DrivineMetamodelVersionStore.kt | 28 +-- .../dice/storage/MetamodelRowMappers.kt | 78 ++------ ...stractMetamodelVersionStoreContractTest.kt | 166 ++---------------- ...odelVersionStoreContractIntegrationTest.kt | 4 +- ...ineMetamodelVersionStoreIntegrationTest.kt | 130 +------------- .../dice/storage/MetamodelRowMapperTest.kt | 87 +-------- docs/design/metamodel-versioning.md | 24 +-- 11 files changed, 54 insertions(+), 535 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 773ade0b..7b5496b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,26 +74,12 @@ and the consumer PRs that deliver it). through raw Cypher and reads it back, one that round-trips a stamp carrying both alias kinds, and one that removes the stored alias map and asserts the integrity check rejects the row. - Stamp provenance persists too, and is the one part of a stamp a re-save does not - overwrite — **EXPERIMENTAL** (shape may change before 1.0): `origin` is - first-write-wins, set only when the stored row has none, and `lastStamped` moves - only when the incoming value is non-null. A re-stamp carrying no provenance leaves - both alone, which is what a scheduled drift check does on every pass, so a routine - check can neither erase the recorded cause nor replace it with its own identity. - Both rules are `coalesce` expressions inside the MERGE, so they hold under - concurrency without a read followed by a write. `savedAt` and `savedAtEpochMillis` - keep their existing behavior: set on create, untouched by a re-save. `origin` and - `lastStamped` are not hashed, so neither rule can move a stamp off its natural key. - Each is stored as a JSON object, which keeps a `StampProvenance()` with both fields - unset distinguishable from no provenance at all. `StampProvenance`'s 256-character - cap needs no column sizing here, since a Neo4j string property has no declared - width; a byte-sized backend still needs room for the up-to-1024 UTF-8 bytes. - `MetamodelVersionStore.saveVersion`'s KDoc now states both rules as contract, and - `dice-metamodel` gains `InMemoryMetamodelVersionStore`, the reference - implementation that applies them, promoted from a private class in that module's - own tests. `AbstractMetamodelVersionStoreContractTest` runs one suite against both - stores. + `savedAt` and `savedAtEpochMillis` keep their existing behavior: set on create, + untouched by a re-save. `dice-metamodel` gains `InMemoryMetamodelVersionStore`, the + reference implementation of the store contract, promoted from a private class in + that module's own tests. `AbstractMetamodelVersionStoreContractTest` runs one suite + against both stores. **Compatibility: additive.** New classes, and a new `dice-storage` → `dice-metamodel` module dependency; no existing API touched. Stored nodes stay readable: every - property that existed before keeps its name, meaning, and encoding, and the four - new ones are absent when nothing declares them. + property that existed before keeps its name, meaning, and encoding, and the two + new alias fields are absent when nothing declares them. diff --git a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt index 1db76dbf..48cd4215 100644 --- a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt +++ b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt @@ -29,34 +29,19 @@ class InMemoryMetamodelVersionStore : MetamodelVersionStore { private val saved = mutableListOf() /** - * Upsert on `(schemaName, contentHash)`, applying the contract's two provenance rules: `origin` - * is kept once the stored stamp has one, and `lastStamped` moves only when [version] carries a - * value. A stamp that is already there keeps its place in the write order, so re-saving an old - * version doesn't make it the latest. + * Upsert on `(schemaName, contentHash)`. A stamp that is already there keeps its place in the + * write order, so re-saving an old version doesn't make it the latest; the incoming stamp + * replaces it in place. * - * Everything runs under the list's own lock, so two threads re-stamping one version can't - * interleave the read of the stored provenance with the write that replaces it. + * Everything runs under the list's own lock, so two threads saving the same version can't + * interleave the search for an existing stamp with the write that lands the new one. */ override fun saveVersion(version: MetamodelVersion) { synchronized(saved) { val at = saved.indexOfFirst { it.schemaName == version.schemaName && it.contentHash == version.contentHash } - if (at < 0) { - saved += version - return - } - val stored = saved[at] - saved[at] = MetamodelVersion( - schemaName = version.schemaName, - entityTypeNames = version.entityTypeNames, - entityTypeLabels = version.entityTypeLabels, - entityTypeProperties = version.entityTypeProperties, - relationshipNames = version.relationshipNames, - entityTypeAliases = version.entityTypeAliases, - origin = stored.origin ?: version.origin, - lastStamped = version.lastStamped ?: stored.lastStamped, - ) + if (at < 0) saved += version else saved[at] = version } } diff --git a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt index a14b17fe..be763fdc 100644 --- a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt +++ b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt @@ -49,19 +49,8 @@ interface MetamodelVersionStore { * Save a version stamp, keyed on `(schemaName, contentHash)`. Saving the same version twice * leaves one stored version. * - * **Provenance survives routine re-saves.** [MetamodelVersion.origin] and - * [MetamodelVersion.lastStamped] are the two fields a re-save does not simply overwrite: - * - * - `origin` is first-write-wins. An implementation sets it only when the stored stamp has - * none, so the cause of the first stamp stands however many times the schema is re-stamped. - * - `lastStamped` moves only when the incoming stamp carries a value. - * - A re-save whose provenance is null on both fields leaves both stored fields untouched. - * That is what a scheduled drift check does on every pass, and the rule is what stops it - * erasing the recorded cause or replacing it with its own identity. - * - * Neither field is hashed, so neither rule can move a stamp off its natural key. Everything - * else about a stored stamp is content the key already determines, so a re-save overwrites it - * with an identical value. + * Everything about a stored stamp is content the key already determines, so a re-save + * overwrites it with an identical value. * * Whatever an implementation records as the moment of the save keeps its existing value on a * re-save, along with the stamp's place in the write order: a re-saved old stamp does not diff --git a/dice-metamodel/src/test/kotlin/com/embabel/dice/metamodel/MetamodelVersionStoreTest.kt b/dice-metamodel/src/test/kotlin/com/embabel/dice/metamodel/MetamodelVersionStoreTest.kt index bb141e05..15cf86ce 100644 --- a/dice-metamodel/src/test/kotlin/com/embabel/dice/metamodel/MetamodelVersionStoreTest.kt +++ b/dice-metamodel/src/test/kotlin/com/embabel/dice/metamodel/MetamodelVersionStoreTest.kt @@ -25,7 +25,7 @@ import org.junit.jupiter.api.Test * backend is free to override with a keyed lookup. A store implementation gets its own tests * wherever it lives. * - * The provenance rules [MetamodelVersionStore.saveVersion] states are checked by + * The upsert rules [MetamodelVersionStore.saveVersion] states are checked by * `AbstractMetamodelVersionStoreContractTest`, which runs the same suite against * [InMemoryMetamodelVersionStore] and the graph-backed store. */ diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt index d27bb631..05bf83ff 100644 --- a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt @@ -48,19 +48,6 @@ import java.time.Clock * holds an old stamp at its original position. The in-memory reference implementation behaves the * same way. * - * Stamp provenance is the one part of a version a re-save does not overwrite. `origin` is - * first-write-wins: the stored value stands, and the incoming one is taken only when the node has - * none. `lastStamped` moves only when the incoming stamp carries a value. A re-stamp supplying no - * provenance therefore leaves both alone, which is what every routine drift-check re-stamp does, so - * a scheduled check can neither erase the recorded cause nor replace it with its own identity. - * `MetamodelVersion.origin` and `lastStamped` are not hashed, so neither rule can move a version - * off its natural key. - * - * Both rules are conditional expressions inside the MERGE, so they hold under concurrency: nothing - * reads the stored provenance in one statement and writes it back in another. - * `InMemoryMetamodelVersionStore` applies the same two rules, and - * `AbstractMetamodelVersionStoreContractTest` runs the same suite against both. - * * Three uniqueness constraints are required, and the host declares them in a `SchemaCatalog` bean * (the module's `TestApplication` shows the shape): * - `UniquenessConstraintSpec("MetamodelVersion", listOf("schemaName", "contentHash"))` makes the @@ -93,17 +80,6 @@ open class DrivineMetamodelVersionStore( * away, so the counter stays put and the existing sequence is kept; only the content is * refreshed. * - * The two provenance properties are set through `coalesce`, which is where the store's - * first-write-wins and update-only-when-supplied rules live: - * - `n.origin = coalesce(n.origin, $origin)` keeps whatever the node already holds. On a - * create there is nothing to keep, so the incoming value lands; a `$origin` of null on a - * node that has none is a set-to-null, which leaves no property behind. - * - `n.lastStamped = coalesce($lastStamped, n.lastStamped)` prefers the incoming value and - * falls back to the stored one, so a null incoming value keeps what is there. - * - * Both are read and written inside one statement, so two concurrent saves can't interleave - * a read of the stored value with the write that replaces it. - * * `entityTypeAliases` binds null when the version declares no former names. Setting a * property to null removes it, so an alias-free stamp leaves a node with no such property, * which is what a writer from before aliases existed left. @@ -129,9 +105,7 @@ open class DrivineMetamodelVersionStore( n.entityTypeLabels = ${'$'}entityTypeLabels, n.entityTypeProperties = ${'$'}entityTypeProperties, n.relationshipNames = ${'$'}relationshipNames, - n.entityTypeAliases = ${'$'}entityTypeAliases, - n.origin = coalesce(n.origin, ${'$'}origin), - n.lastStamped = coalesce(${'$'}lastStamped, n.lastStamped) + n.entityTypeAliases = ${'$'}entityTypeAliases WITH n WHERE n.sequence IS NULL MERGE (c:MetamodelSchemaCounter {schemaName: ${'$'}schemaName}) diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt index f674c52c..cb95a657 100644 --- a/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt @@ -18,7 +18,6 @@ package com.embabel.dice.storage import com.embabel.agent.core.Cardinality import com.embabel.dice.metamodel.MetamodelVersion import com.embabel.dice.metamodel.PropertySignature -import com.embabel.dice.metamodel.StampProvenance import com.fasterxml.jackson.databind.ObjectMapper import java.time.Instant @@ -48,13 +47,12 @@ private val objectMapper = ObjectMapper() * missing one is corrupt, so the accessor throws and the store's surrounding guard skips the row * with a warning. * - * Four things are optional, and absent means "none of these were declared": the version-level - * `entityTypeAliases` property, the `aliases` field inside a stored property signature, and the - * `origin` and `lastStamped` properties. Writing them only when they hold something means an - * alias-free stamp with no provenance stores exactly the properties this mapper stored before any - * of the four existed, and a node written by that older build reads back here as a stamp declaring - * none of them. Aliases feed the content hash, so a stamp carrying them and failing to store them - * would fail its own integrity check on the way back in and be unreadable for good. + * Two things are optional, and absent means "no former names were declared": the version-level + * `entityTypeAliases` property, and the `aliases` field inside a stored property signature. Writing + * them only when they hold something means an alias-free stamp stores exactly the properties this + * mapper stored before either existed, and a node written by that older build reads back here as a + * stamp declaring neither. Aliases feed the content hash, so a stamp carrying them and failing to + * store them would fail its own integrity check on the way back in and be unreadable for good. */ object MetamodelVersionRowMapper { @@ -64,9 +62,9 @@ object MetamodelVersionRowMapper { * [savedAt] is a parameter, so this stays a pure function of its arguments and a test can pin * the instant a version was stored at. * - * `entityTypeAliases`, `origin` and `lastStamped` bind `null` when the version declares none. - * A Cypher `SET` of `null` leaves no property behind, which is the encoding the read side - * expects and the shape an older writer left. + * `entityTypeAliases` binds `null` when the version declares no former names. A Cypher `SET` of + * `null` leaves no property behind, which is the encoding the read side expects and the shape an + * older writer left. */ fun bindMap(version: MetamodelVersion, savedAt: Instant): Map = mapOf( "schemaName" to version.schemaName, @@ -76,8 +74,6 @@ object MetamodelVersionRowMapper { "entityTypeProperties" to serializeMapOfSignatureSets(version.entityTypeProperties), "relationshipNames" to serializeList(version.relationshipNames), "entityTypeAliases" to serializeAliasMap(version.entityTypeAliases), - "origin" to serializeProvenance(version.origin), - "lastStamped" to serializeProvenance(version.lastStamped), "savedAt" to savedAt.toString(), "savedAtEpochMillis" to savedAt.toEpochMilli(), ) @@ -95,8 +91,6 @@ object MetamodelVersionRowMapper { * * Aliases are part of that derivation, at both levels, so a node that dropped either alias * field fails here rather than reading back as an alias-free stamp with the wrong hash. - * Provenance is not, so a node's `origin` and `lastStamped` are carried through untouched by - * the check. */ fun fromRow(row: Map<*, *>): MetamodelVersion { val storedHash = row.str("contentHash") @@ -107,8 +101,6 @@ object MetamodelVersionRowMapper { entityTypeProperties = deserializeMapOfSignatureSets(row.str("entityTypeProperties")), relationshipNames = deserializeList(row.str("relationshipNames")), entityTypeAliases = deserializeAliasMap(row.optionalStr("entityTypeAliases")), - origin = deserializeProvenance(row.optionalStr("origin"), "origin"), - lastStamped = deserializeProvenance(row.optionalStr("lastStamped"), "lastStamped"), ) require(version.contentHash == storedHash) { "MetamodelVersion '${version.schemaName}' fails its integrity check: stored contentHash " + @@ -156,55 +148,6 @@ private fun serializeAliasMap(aliases: Map>): String? = private fun deserializeAliasMap(serialized: String?): Map> = if (serialized.isNullOrEmpty()) emptyMap() else deserializeMapOfLabelSets(serialized) -/** - * Serialize a [StampProvenance] as `{"actor": ..., "trigger": ...}`, and write nothing when the - * stamp carries none. - * - * A JSON object rather than two scalar properties, because `StampProvenance()` with both fields - * unset is a real provenance and has to stay distinguishable from no provenance at all; two scalar - * properties would encode both as two absent values. - * - * Nothing here sizes the value. [StampProvenance] caps `actor` and `trigger` at 256 characters, and - * a Neo4j string property has no declared width, so there is no column to size. A backend that - * stores them in a byte-sized column needs room for the up-to-1024 UTF-8 bytes 256 characters can - * take. - */ -private fun serializeProvenance(provenance: StampProvenance?): String? = provenance?.let { - objectMapper.writeValueAsString(linkedMapOf("actor" to it.actor, "trigger" to it.trigger)) -} - -/** - * Inverse of [serializeProvenance]. An absent property means the stamp carried no provenance, which - * is what a node written before provenance existed looks like. - * - * Malformed content throws, naming the [property] it came from. The character cap is re-applied by - * [StampProvenance]'s own constructor, so a hand-edit that pushes `actor` past it makes the node - * unreadable and the store skips it with a warning rather than handing back a value the model says - * is impossible. - */ -private fun deserializeProvenance(serialized: String?, property: String): StampProvenance? { - if (serialized.isNullOrEmpty()) return null - val parsed = objectMapper.readValue(serialized, Any::class.java) - val fields = parsed as? Map<*, *> ?: throw IllegalArgumentException( - "the stored '$property' is a ${parsed?.javaClass?.simpleName ?: "null"} where a provenance " + - "object with 'actor' and 'trigger' was expected" - ) - return StampProvenance( - actor = fields.provenanceField(property, "actor"), - trigger = fields.provenanceField(property, "trigger"), - ) -} - -/** Read one nullable field of a stored provenance object, refusing anything that isn't a string. */ -private fun Map<*, *>.provenanceField(property: String, field: String): String? = - when (val value = this[field]) { - null -> null - is String -> value - else -> throw IllegalArgumentException( - "the '$field' of the stored '$property' is a ${value.javaClass.simpleName} where a string was expected" - ) - } - /** Inverse of [serializeMapOfLabelSets]. */ private fun deserializeMapOfLabelSets(serialized: String): Map> { if (serialized.isEmpty()) return emptyMap() @@ -330,7 +273,6 @@ private fun Map<*, *>.str(key: String): String = /** * Read a property that may legitimately not be there, where absent means the stamp declared nothing - * to put in it. Only the alias map and the two provenance fields are read this way; everything else - * goes through [str]. + * to put in it. Only the alias map is read this way; everything else goes through [str]. */ private fun Map<*, *>.optionalStr(key: String): String? = this[key]?.toString() diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt index 0db50f53..41a8da40 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt @@ -17,36 +17,27 @@ package com.embabel.dice.storage import com.embabel.dice.metamodel.MetamodelVersion import com.embabel.dice.metamodel.MetamodelVersionStore -import com.embabel.dice.metamodel.StampProvenance import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Assertions.assertNotNull -import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Test /** - * Cross-backend contract for [MetamodelVersionStore.saveVersion]'s upsert, and for the two - * provenance rules it states. Each subclass supplies a store and inherits the whole suite, so a - * backend that disagrees with the in-memory reference fails at authoring time. + * Cross-backend contract for [MetamodelVersionStore.saveVersion]'s upsert. Each subclass supplies a + * store and inherits the whole suite, so a backend that disagrees with the in-memory reference fails + * at authoring time. * - * The provenance rules exist because the drift check re-stamps its schema on every pass and carries - * no provenance when it does. A store that overwrote both fields on every save would blank the - * recorded cause within a deploy cycle, and one that stamped its own identity into `lastStamped` - * would launder it, so both halves get a test here. + * The rules here matter because the drift check re-stamps its schema on every pass. A store that + * treated each of those re-stamps as a new record would fill the history with copies of one version, + * and one that moved a re-stamped version to the front would report the wrong stamp as the latest. */ abstract class AbstractMetamodelVersionStoreContractTest { /** A store holding nothing for the schema names below. */ protected abstract fun store(): MetamodelVersionStore - /** - * A stamp of one entity type. Provenance is never hashed, so two calls differing only in - * [origin] or [lastStamped] land on the same natural key, which is what a re-stamp is. - */ + /** A stamp of one entity type. */ private fun version( schemaName: String, typeName: String = "Person", - origin: StampProvenance? = null, - lastStamped: StampProvenance? = null, ): MetamodelVersion = MetamodelVersion( schemaName = schemaName, entityTypeNames = listOf(typeName), @@ -54,11 +45,9 @@ abstract class AbstractMetamodelVersionStoreContractTest { entityTypeProperties = emptyMap(), relationshipNames = emptyList(), entityTypeAliases = emptyMap(), - origin = origin, - lastStamped = lastStamped, ) - // ---- the upsert the provenance rules sit on ---- + // ---- the upsert ---- @Test fun `re-saving a version leaves one record`() { @@ -73,147 +62,24 @@ abstract class AbstractMetamodelVersionStoreContractTest { assertEquals(stamp, store.latestVersion(schemaName)) } - // ---- provenance is written and read back ---- - - @Test - fun `a first save keeps the provenance it was given`() { - val store = store() - val schemaName = "contract-provenance-first" - val cause = StampProvenance("deploy-pipeline", "release-42") - - store.saveVersion(version(schemaName, origin = cause, lastStamped = cause)) - - val reloaded = store.latestVersion(schemaName)!! - assertEquals(cause, reloaded.origin) - assertEquals(cause, reloaded.lastStamped) - } - - @Test - fun `a provenance field the host left unset stays unset`() { - val store = store() - val schemaName = "contract-provenance-partial" - - store.saveVersion(version(schemaName, origin = StampProvenance(actor = "operator"))) - - val reloaded = store.latestVersion(schemaName)!! - assertEquals("operator", reloaded.origin!!.actor) - assertNull(reloaded.origin!!.trigger) - assertNull(reloaded.lastStamped) - } - - @Test - fun `a provenance with neither field set is still a provenance`() { - // StampProvenance() says "the host recorded a cause and named nothing in it", which is not - // the same as recording no cause at all. A store that flattened the two would answer the - // question "was this stamp taken by something that reports provenance" wrongly. - val store = store() - val schemaName = "contract-provenance-empty" - - store.saveVersion(version(schemaName, origin = StampProvenance())) - - val reloaded = store.latestVersion(schemaName)!! - assertNotNull(reloaded.origin, "an empty provenance must not read back as no provenance") - assertNull(reloaded.origin!!.actor) - assertNull(reloaded.origin!!.trigger) - } - - // ---- the two rules ---- - - @Test - fun `a re-stamp carrying no provenance leaves both fields alone`() { - // Every routine drift-check re-stamp arrives like this. - val store = store() - val schemaName = "contract-provenance-null-restamp" - val cause = StampProvenance("operator", "first-stamp") - store.saveVersion(version(schemaName, origin = cause, lastStamped = cause)) - - store.saveVersion(version(schemaName)) - - val reloaded = store.latestVersion(schemaName)!! - assertEquals(cause, reloaded.origin, "a null re-stamp must not erase the original cause") - assertEquals(cause, reloaded.lastStamped, "nor the most recent one") - } - - @Test - fun `a re-stamp carrying provenance keeps origin and moves lastStamped`() { - val store = store() - val schemaName = "contract-provenance-restamp" - val first = StampProvenance("bootstrap", "first-boot") - val second = StampProvenance("operator", "manual-restamp") - store.saveVersion(version(schemaName, origin = first, lastStamped = first)) - - store.saveVersion(version(schemaName, origin = second, lastStamped = second)) - - val reloaded = store.latestVersion(schemaName)!! - assertEquals(first, reloaded.origin, "origin is first-write-wins") - assertEquals(second, reloaded.lastStamped, "lastStamped follows the newest save that names one") - } - - @Test - fun `origin is taken by the first save that carries one and not moved after`() { - val store = store() - val schemaName = "contract-provenance-first-write-wins" - - store.saveVersion(version(schemaName)) - assertNull(store.latestVersion(schemaName)!!.origin, "nothing was supplied, so nothing is recorded") - - val backfilled = StampProvenance("operator", "backfill") - store.saveVersion(version(schemaName, origin = backfilled)) - assertEquals(backfilled, store.latestVersion(schemaName)!!.origin, "a stamp with no origin takes one") - - store.saveVersion(version(schemaName, origin = StampProvenance("someone-else", "later"))) - assertEquals(backfilled, store.latestVersion(schemaName)!!.origin, "and never gives it up again") - } - - @Test - fun `lastStamped moves without an origin ever being supplied`() { - val store = store() - val schemaName = "contract-provenance-last-only" - store.saveVersion(version(schemaName, lastStamped = StampProvenance("first", "run-1"))) - - store.saveVersion(version(schemaName, lastStamped = StampProvenance("second", "run-2"))) - - val reloaded = store.latestVersion(schemaName)!! - assertNull(reloaded.origin) - assertEquals(StampProvenance("second", "run-2"), reloaded.lastStamped) - } - - // ---- provenance belongs to the stamp ---- - - @Test - fun `each stamp of a schema carries its own provenance`() { - val store = store() - val schemaName = "contract-provenance-per-stamp" - val first = version(schemaName, "First", origin = StampProvenance("first-cause")) - val second = version(schemaName, "Second", origin = StampProvenance("second-cause")) - - store.saveVersion(first) - store.saveVersion(second) - - assertEquals(StampProvenance("first-cause"), store.findVersion(schemaName, first.contentHash)!!.origin) - assertEquals(StampProvenance("second-cause"), store.findVersion(schemaName, second.contentHash)!!.origin) - } - @Test - fun `a re-stamp for provenance leaves the stamp where it was in the history`() { - // Provenance is not hashed, so this write lands on an existing key. It has to behave like - // any other re-save: content refreshed, position in the write order untouched. + fun `a re-save leaves the stamp where it was in the history`() { + // The write lands on an existing key, so it has to behave like any other re-save: content + // refreshed, position in the write order untouched. val store = store() - val schemaName = "contract-provenance-order" - val first = version(schemaName, "First", origin = StampProvenance("original")) + val schemaName = "contract-order" + val first = version(schemaName, "First") val second = version(schemaName, "Second") store.saveVersion(first) store.saveVersion(second) - store.saveVersion(version(schemaName, "First", lastStamped = StampProvenance("re-stamped"))) + store.saveVersion(version(schemaName, "First")) assertEquals( listOf("Second", "First"), store.versionHistory(schemaName).map { it.entityTypeNames.single() }, - "a provenance re-stamp must not make an old version the latest", + "a re-save must not make an old version the latest", ) - val reloadedFirst = store.findVersion(schemaName, first.contentHash)!! - assertEquals(StampProvenance("original"), reloadedFirst.origin) - assertEquals(StampProvenance("re-stamped"), reloadedFirst.lastStamped) + assertEquals(first, store.findVersion(schemaName, first.contentHash)) } } diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreContractIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreContractIntegrationTest.kt index e0eca4ba..a693d4f3 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreContractIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreContractIntegrationTest.kt @@ -27,8 +27,8 @@ import org.springframework.test.context.DynamicPropertySource /** * Runs the [AbstractMetamodelVersionStoreContractTest] suite against the Neo4j-backed * [DrivineMetamodelVersionStore] (testcontainer). This is the half that catches the graph backend - * disagreeing with the in-memory reference on the provenance rules, which is easy to do: they live - * in a Cypher `coalesce` there and in Kotlin here. + * disagreeing with the in-memory reference on the upsert rules, which is easy to do: they live in a + * Cypher `MERGE` there and in a list index here. * * Uses the shared [Neo4jTestContainer]; see that class for why Drivine's built-in testcontainer * is bypassed. diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt index 63952d5a..f52d54e2 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStoreIntegrationTest.kt @@ -23,13 +23,11 @@ import com.embabel.agent.core.Cardinality import com.embabel.dice.metamodel.MetamodelVersion import com.embabel.dice.metamodel.PropertySignature import com.embabel.dice.metamodel.PropertySignature.Kind -import com.embabel.dice.metamodel.StampProvenance import org.drivine.manager.PersistenceManager import org.drivine.query.QuerySpecification import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertNotEquals -import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test @@ -608,90 +606,6 @@ class DrivineMetamodelVersionStoreIntegrationTest { } } - // ---- Stamp provenance on the node ---- - - @Test - fun `provenance persists and reads back`() { - val schemaName = "provenance-schema" - val cause = StampProvenance("deploy-pipeline", "release-42") - - store.saveVersion(stampedBy(schemaName, "Person", origin = cause, lastStamped = cause)) - - val reloaded = store.latestVersion(schemaName)!! - assertEquals(cause, reloaded.origin) - assertEquals(cause, reloaded.lastStamped) - } - - @Test - fun `a re-stamp carrying no provenance rewrites neither stored property`() { - // The routine case: DefaultDriftCheckRunner re-stamps on every pass and supplies nothing. - // Asserted on the raw properties as well as the reloaded stamp, because a store that wrote - // null over both would still read back as "no provenance" and look plausible. - val schemaName = "provenance-untouched-schema" - val cause = StampProvenance("operator", "first-stamp") - val stamp = stampedBy(schemaName, "Person", origin = cause, lastStamped = cause) - store.saveVersion(stamp) - val before = storedProvenanceProperties(schemaName, stamp) - assertEquals( - """{"actor":"operator","trigger":"first-stamp"}|{"actor":"operator","trigger":"first-stamp"}""", - before, - "precondition: both properties are on the node", - ) - - store.saveVersion(stampedBy(schemaName, "Person")) - - assertEquals(before, storedProvenanceProperties(schemaName, stamp), "neither property may be rewritten") - val reloaded = store.latestVersion(schemaName)!! - assertEquals(cause, reloaded.origin) - assertEquals(cause, reloaded.lastStamped) - } - - @Test - fun `a re-stamp carrying provenance keeps the stored origin and moves lastStamped`() { - val schemaName = "provenance-restamp-schema" - val first = StampProvenance("bootstrap", "first-boot") - val second = StampProvenance("operator", "manual-restamp") - val stamp = stampedBy(schemaName, "Person", origin = first, lastStamped = first) - store.saveVersion(stamp) - - store.saveVersion(stampedBy(schemaName, "Person", origin = second, lastStamped = second)) - - assertEquals( - """{"actor":"bootstrap","trigger":"first-boot"}|{"actor":"operator","trigger":"manual-restamp"}""", - storedProvenanceProperties(schemaName, stamp), - ) - val reloaded = store.latestVersion(schemaName)!! - assertEquals(first, reloaded.origin) - assertEquals(second, reloaded.lastStamped) - } - - @Test - fun `a stamp with no provenance leaves no provenance properties on the node`() { - // What a node written before provenance existed looks like, produced by the current writer. - val schemaName = "provenance-absent-schema" - val stamp = stampedBy(schemaName, "Person") - - store.saveVersion(stamp) - - assertEquals("|", storedProvenanceProperties(schemaName, stamp)) - val reloaded = store.latestVersion(schemaName)!! - assertNull(reloaded.origin) - assertNull(reloaded.lastStamped) - } - - @Test - fun `a provenance whose fields are both unset survives as a provenance`() { - // Two scalar properties could not tell this apart from the test above; the JSON object can. - val schemaName = "provenance-empty-schema" - - store.saveVersion(stampedBy(schemaName, "Person", origin = StampProvenance())) - - val reloaded = store.latestVersion(schemaName)!! - assertNotNull(reloaded.origin, "an empty provenance must not read back as no provenance") - assertNull(reloaded.origin!!.actor) - assertNull(reloaded.origin!!.trigger) - } - // ---- Aliases, and rows written before they existed ---- @Test @@ -777,9 +691,9 @@ class DrivineMetamodelVersionStoreIntegrationTest { @Test fun `a node written in the old four-field shape reads back through the new mapper`() { - // A stamp saved before aliases and provenance existed: property signatures with exactly - // four fields, and no entityTypeAliases, origin or lastStamped. Written straight through - // Cypher, so nothing in the current writer can quietly supply the missing properties. + // A stamp saved before aliases existed: property signatures with exactly four fields, and + // no entityTypeAliases. Written straight through Cypher, so nothing in the current writer + // can quietly supply the missing properties. val schemaName = "old-shape-schema" val expected = MetamodelVersion( schemaName = schemaName, @@ -824,8 +738,6 @@ class DrivineMetamodelVersionStoreIntegrationTest { assertEquals(expected, reloaded, "an old-shape node must still read, and pass its integrity check") assertEquals(expected.contentHash, reloaded!!.contentHash) assertEquals(emptyMap>(), reloaded.entityTypeAliases) - assertNull(reloaded.origin) - assertNull(reloaded.lastStamped) assertTrue( reloaded.entityTypeProperties["Person"]!!.all { it.aliases.isEmpty() }, "four-field signatures carry no former names", @@ -836,7 +748,7 @@ class DrivineMetamodelVersionStoreIntegrationTest { fun `re-saving an old-shape node through the current writer leaves it in the old shape`() { // The upgrade path: an application that boots against a graph written by an older build // re-stamps its unchanged schema. The write lands on the existing node, and because the - // stamp declares no aliases and no provenance, none of the four new fields appears. + // stamp declares no aliases, neither of the two new fields appears. val schemaName = "old-shape-restamp-schema" val version = MetamodelVersion( schemaName = schemaName, @@ -853,7 +765,6 @@ class DrivineMetamodelVersionStoreIntegrationTest { assertEquals(1, rawNodeCount()) assertEquals("", storedProperty(schemaName, version, StoredProperty.ENTITY_TYPE_ALIASES)) - assertEquals("|", storedProvenanceProperties(schemaName, version)) assertEquals( """{"Person":[{"name":"name","kind":"VALUE","type":"string","cardinality":"ONE"}]}""", storedProperty(schemaName, version, StoredProperty.ENTITY_TYPE_PROPERTIES), @@ -863,23 +774,6 @@ class DrivineMetamodelVersionStoreIntegrationTest { // ---- helpers ---- - /** A stamp of one entity type, carrying whatever provenance the test wants to record on it. */ - private fun stampedBy( - schemaName: String, - typeName: String, - origin: StampProvenance? = null, - lastStamped: StampProvenance? = null, - ): MetamodelVersion = MetamodelVersion( - schemaName = schemaName, - entityTypeNames = listOf(typeName), - entityTypeLabels = emptyMap(), - entityTypeProperties = emptyMap(), - relationshipNames = emptyList(), - entityTypeAliases = emptyMap(), - origin = origin, - lastStamped = lastStamped, - ) - /** The stored properties a test can read back raw, each with the Cypher that returns it. */ private enum class StoredProperty(val returnExpression: String) { ENTITY_TYPE_ALIASES("coalesce(n.entityTypeAliases, '')"), @@ -902,22 +796,6 @@ class DrivineMetamodelVersionStoreIntegrationTest { .transform(String::class.java), ) - /** - * Both provenance properties as one `origin|lastStamped` string, so a test can assert that a - * re-save left the stored bytes exactly as they were. `` stands for a property that - * isn't on the node. - */ - private fun storedProvenanceProperties(schemaName: String, version: MetamodelVersion): String? = - persistenceManager.maybeGetOne( - QuerySpecification.withStatement( - """ - MATCH (n:MetamodelVersion {schemaName: ${'$'}schemaName, contentHash: ${'$'}contentHash}) - RETURN coalesce(n.origin, '') + '|' + coalesce(n.lastStamped, '') AS value - """.trimIndent(), - ).bind(mapOf("schemaName" to schemaName, "contentHash" to version.contentHash)) - .transform(String::class.java), - ) - /** Rewrite a version node's serialized entity type names, leaving its stored hash untouched. */ private fun tamperWithEntityTypeNames(schemaName: String, serializedNames: String) { persistenceManager.execute( diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt index 6ef63dd6..175397e4 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt @@ -19,7 +19,6 @@ import com.embabel.agent.core.Cardinality import com.embabel.dice.metamodel.MetamodelVersion import com.embabel.dice.metamodel.PropertySignature import com.embabel.dice.metamodel.PropertySignature.Kind -import com.embabel.dice.metamodel.StampProvenance import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue @@ -73,19 +72,6 @@ class MetamodelRowMapperTest { private fun row(savedAtInstant: Instant = savedAt): MutableMap = MetamodelVersionRowMapper.bindMap(version, savedAtInstant).toMutableMap() - /** [version] again, carrying provenance. The two are the same stamp: provenance isn't hashed. */ - private fun withProvenance(origin: StampProvenance?, lastStamped: StampProvenance?): MetamodelVersion = - MetamodelVersion( - schemaName = version.schemaName, - entityTypeNames = version.entityTypeNames, - entityTypeLabels = version.entityTypeLabels, - entityTypeProperties = version.entityTypeProperties, - relationshipNames = version.relationshipNames, - entityTypeAliases = version.entityTypeAliases, - origin = origin, - lastStamped = lastStamped, - ) - @Test fun `a version round-trips through its own property map`() { assertEquals(version, MetamodelVersionRowMapper.fromRow(row())) @@ -270,80 +256,15 @@ class MetamodelRowMapperTest { assertTrue(thrown.message!!.contains("Person"), thrown.message) } - // ---- Provenance ---- - @Test - fun `provenance round-trips, and a version carrying none binds neither property`() { - assertNull(row()["origin"]) - assertNull(row()["lastStamped"]) - assertNull(MetamodelVersionRowMapper.fromRow(row()).origin) - assertNull(MetamodelVersionRowMapper.fromRow(row()).lastStamped) - - val stamped = withProvenance(StampProvenance("deploy", "release-1"), StampProvenance("operator", null)) - val bound = MetamodelVersionRowMapper.bindMap(stamped, savedAt) - - assertEquals("""{"actor":"deploy","trigger":"release-1"}""", bound["origin"]) - assertEquals("""{"actor":"operator","trigger":null}""", bound["lastStamped"]) - - val reloaded = MetamodelVersionRowMapper.fromRow(bound) - assertEquals(StampProvenance("deploy", "release-1"), reloaded.origin) - assertEquals(StampProvenance("operator", null), reloaded.lastStamped) - } - - @Test - fun `a provenance with neither field set stays distinguishable from no provenance`() { - // The reason provenance is stored as an object rather than as two scalar properties: two - // scalars would encode StampProvenance() and null identically. - val bound = MetamodelVersionRowMapper.bindMap(withProvenance(StampProvenance(), null), savedAt) - - assertEquals("""{"actor":null,"trigger":null}""", bound["origin"]) - assertEquals(StampProvenance(), MetamodelVersionRowMapper.fromRow(bound).origin) - assertNull(MetamodelVersionRowMapper.fromRow(bound).lastStamped) - } - - @Test - fun `provenance stays out of the content hash, so a re-stamp lands on the same key`() { - val plain = MetamodelVersionRowMapper.bindMap(version, savedAt) - val stamped = MetamodelVersionRowMapper.bindMap( - withProvenance(StampProvenance("a", "b"), StampProvenance("c", "d")), - savedAt, - ) - - assertEquals(plain["contentHash"], stamped["contentHash"]) - } - - @Test - fun `a stored provenance that is not an object fails the read, naming the property`() { - val corrupt = row().apply { put("lastStamped", """"just-a-string"""") } - - val thrown = assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } - assertTrue(thrown.message!!.contains("lastStamped"), thrown.message) - } - - @Test - fun `a stored provenance field that is not a string fails the read, naming both`() { - val corrupt = row().apply { put("origin", """{"actor":42,"trigger":null}""") } - - val thrown = assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } - assertTrue(thrown.message!!.contains("actor"), thrown.message) - assertTrue(thrown.message!!.contains("origin"), thrown.message) - } - - @Test - fun `a row with no alias or provenance properties at all reads back as a stamp declaring none`() { - // A node written before any of the four existed. Removing the keys is the same thing the - // graph does when a property was never set. - val old = row().apply { - remove("entityTypeAliases") - remove("origin") - remove("lastStamped") - } + fun `a row with no alias property at all reads back as a stamp declaring none`() { + // A node written before aliases existed. Removing the key is the same thing the graph does + // when a property was never set. + val old = row().apply { remove("entityTypeAliases") } val reloaded = MetamodelVersionRowMapper.fromRow(old) assertEquals(version, reloaded) assertEquals(emptyMap>(), reloaded.entityTypeAliases) - assertNull(reloaded.origin) - assertNull(reloaded.lastStamped) } } diff --git a/docs/design/metamodel-versioning.md b/docs/design/metamodel-versioning.md index a30bfce5..48f8c697 100644 --- a/docs/design/metamodel-versioning.md +++ b/docs/design/metamodel-versioning.md @@ -358,8 +358,6 @@ unchanged schema updates the node already there. Three things govern how it beha - **A re-save updates content only.** Sequence, counter, and `savedAt` keep their existing values, so an old stamp stays at its original position in the history. `InMemoryMetamodelVersionStore`, the reference implementation `dice-metamodel` ships, behaves the same way. -- **Provenance survives a re-save.** `origin` and `lastStamped` are the two fields a re-save does - not overwrite; the rules are below. The structural fields are stored as JSON strings, since Neo4j properties are scalars and flat arrays. Property signatures get explicit named fields with enums by name @@ -368,7 +366,7 @@ re-point the day someone inserts a constant into `Cardinality`. The content hash `contentHash` on a node is a checksum: the store recomputes it on read and skips a node that disagrees with itself, logging a warning. -### Aliases and provenance in storage +### Aliases in storage Declared aliases land in two places on the node: the version-level `entityTypeAliases` map as its own property, and a property's former names as a fifth `aliases` field inside its stored signature. @@ -383,26 +381,6 @@ row in the old four-field shape through raw Cypher and reads it back, one round- carrying both alias kinds, and one deletes the stored alias map and asserts the integrity check rejects the row. -The two provenance pairs follow the rules the `MetamodelVersionStore` contract states: - -- `origin` is first-write-wins. It is set only when the stored node has none, so the cause of the - first stamp stands however many times the schema is re-stamped. -- `lastStamped` moves only when the incoming stamp carries a value. -- A re-stamp whose provenance is null on both fields leaves both stored fields alone. That is every - pass of the drift check, and the rule is what stops a scheduled job erasing the recorded cause or - writing its own identity over it. - -Both are `coalesce` expressions inside the MERGE — `n.origin = coalesce(n.origin, $origin)` and -`n.lastStamped = coalesce($lastStamped, n.lastStamped)` — so the rules hold under concurrency -without reading the stored value in one statement and writing it back in another. Neither field is -hashed, so neither rule can move a stamp off its natural key. - -Each pair is stored as a JSON object, which keeps a `StampProvenance()` with both fields unset -distinguishable from no provenance at all; two scalar properties would encode the two identically. -The 256-character cap needs no column sizing here, since a Neo4j string property has no declared -width. A backend that stores provenance in a byte-sized column still needs room for the up-to-1024 -UTF-8 bytes those characters can take. - `AbstractMetamodelVersionStoreContractTest` runs one suite against the graph store and the in-memory reference, so the two can't drift apart on rules that live in Cypher on one side and Kotlin on the other. From b13141179484bc52ee71e1eefa030a8e28c7166e Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Wed, 2 Sep 2026 09:33:44 -0400 Subject: [PATCH 5/8] Drop the superseded stamping-contract changelog entry The attribution rework removed the metadata key and recorded that as breaking; the entry describing the key as added survived a rebase two lines below it. One record remains: the key is gone and run lineage answers attribution. --- CHANGELOG.md | 7 ------- 1 file changed, 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b5496b4..6524fd6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,13 +45,6 @@ and the consumer PRs that deliver it). metadata key is removed; lineage answers per-proposition attribution. **Compatibility: breaking.** The key is no longer available; code holding it must migrate to extraction-run queries. -- `DiceMetadataKeys.METAMODEL_VERSION` metadata key and stamping contract. - Propositions can carry the declared schema version hash under this key to - record which schema governed their extraction. The key is defined here with - its contract; production wiring that stamps propositions at persistence time - lands in a follow-up slice after the extraction-run stack merges. - **Compatibility: additive.** New metadata key only; no existing API or code - touched. - Drivine/Neo4j-backed `MetamodelVersionStore` in `dice-storage` (`DrivineMetamodelVersionStore`): stamps persist as `(:MetamodelVersion)` nodes, MERGEd on the natural key `(schemaName, contentHash)`, so a re-stamp updates in From c2209ffcbdccecbeae2bd974a239e6542f2e31d3 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Thu, 3 Sep 2026 17:00:59 -0400 Subject: [PATCH 6/8] Answer the code review on the version store Name the upsert index for what it is. Use require and requireNotNull where the row mapper was throwing IllegalArgumentException by hand; same exception, same messages. --- .../InMemoryMetamodelVersionStore.kt | 4 +-- .../dice/storage/MetamodelRowMappers.kt | 29 ++++++++++--------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt index 48cd4215..bf1f966c 100644 --- a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt +++ b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt @@ -38,10 +38,10 @@ class InMemoryMetamodelVersionStore : MetamodelVersionStore { */ override fun saveVersion(version: MetamodelVersion) { synchronized(saved) { - val at = saved.indexOfFirst { + val existingIndex = saved.indexOfFirst { it.schemaName == version.schemaName && it.contentHash == version.contentHash } - if (at < 0) saved += version else saved[at] = version + if (existingIndex < 0) saved += version else saved[existingIndex] = version } } diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt index cb95a657..3b9f5ecd 100644 --- a/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt @@ -213,10 +213,10 @@ private fun deserializeMapOfSignatureSets(serialized: String): Map encoded.map { element -> val fields = element as? Map<*, *> - ?: throw IllegalArgumentException( - "entityTypeProperties for '$typeName' holds ${element?.javaClass?.simpleName ?: "null"} " + - "where a property signature object was expected" - ) + require(fields != null) { + "entityTypeProperties for '$typeName' holds ${element?.javaClass?.simpleName ?: "null"} " + + "where a property signature object was expected" + } PropertySignature( name = fields.signatureField(typeName, "name"), kind = enumConstant(fields.signatureField(typeName, "kind"), typeName, "kind"), @@ -230,9 +230,9 @@ private fun deserializeMapOfSignatureSets(serialized: String): Map.signatureField(typeName: String, field: String): String = - this[field]?.toString() ?: throw IllegalArgumentException( + requireNotNull(this[field]) { "a property signature for '$typeName' is missing its '$field' field" - ) + }.toString() /** * Read a stored signature's former names. An absent `aliases` field means none were declared, which @@ -242,23 +242,24 @@ private fun Map<*, *>.signatureField(typeName: String, field: String): String = */ private fun Map<*, *>.signatureAliases(typeName: String): Set { val encoded = this["aliases"] ?: return emptySet() - val names = encoded as? List<*> ?: throw IllegalArgumentException( + val names = encoded as? List<*> + require(names != null) { "a property signature for '$typeName' has an 'aliases' field holding a " + "${encoded.javaClass.simpleName} where a list of former names was expected" - ) + } return names.map { name -> - name?.toString() ?: throw IllegalArgumentException( + requireNotNull(name) { "a property signature for '$typeName' has a null entry in its 'aliases' field" - ) + }.toString() }.toSet() } /** Turn a stored enum constant name back into the constant, naming what failed if it's unknown. */ private inline fun > enumConstant(stored: String, typeName: String, field: String): E = - enumValues().firstOrNull { it.name == stored } ?: throw IllegalArgumentException( + requireNotNull(enumValues().firstOrNull { it.name == stored }) { "a property signature for '$typeName' has '$field' = '$stored', which is not a known " + - "${E::class.simpleName} — the node was written by a different version of the schema model" - ) + "${E::class.simpleName}. The node was written by a different version of the schema model" + } /** * Read a property that must be there, and blow up if it isn't. @@ -269,7 +270,7 @@ private inline fun > enumConstant(stored: String, typeName: * is what gives that guard something to catch. */ private fun Map<*, *>.str(key: String): String = - this[key]?.toString() ?: throw IllegalArgumentException("required property '$key' is missing from the stored node") + requireNotNull(this[key]) { "required property '$key' is missing from the stored node" }.toString() /** * Read a property that may legitimately not be there, where absent means the stamp declared nothing From 980d0d248cf30c54b4387df08247855d730f2310 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Thu, 3 Sep 2026 23:39:44 -0400 Subject: [PATCH 7/8] Answer the agent findings on the version store Drop the redundant open modifier, since the module already applies the allopen spring plugin. Log and skip a row that is not a map in place of dropping it silently. Document the retry contract for a lost counter update on both the interface and the Drivine store. Read JSON maps through TypeReference so the unchecked casts go, and let an empty stored string fail the read as the corruption it is. Answer latestVersion in one pass under the lock. Grow the contract suite to cover a missed lookup, newest-first ordering, schema isolation, and an empty schema. --- .../InMemoryMetamodelVersionStore.kt | 2 +- .../dice/metamodel/MetamodelVersionStore.kt | 4 ++ .../storage/DrivineMetamodelVersionStore.kt | 24 +++++++- .../dice/storage/MetamodelRowMappers.kt | 21 +++---- ...stractMetamodelVersionStoreContractTest.kt | 61 ++++++++++++++++++- .../dice/storage/MetamodelRowMapperTest.kt | 10 +++ 6 files changed, 103 insertions(+), 19 deletions(-) diff --git a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt index bf1f966c..c758d728 100644 --- a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt +++ b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/InMemoryMetamodelVersionStore.kt @@ -46,7 +46,7 @@ class InMemoryMetamodelVersionStore : MetamodelVersionStore { } override fun latestVersion(schemaName: String): MetamodelVersion? = - versionHistory(schemaName).firstOrNull() + synchronized(saved) { saved.lastOrNull { it.schemaName == schemaName } } override fun versionHistory(schemaName: String): List = synchronized(saved) { saved.filter { it.schemaName == schemaName }.reversed() } diff --git a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt index be763fdc..6e2a0190 100644 --- a/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt +++ b/dice-metamodel/src/main/kotlin/com/embabel/dice/metamodel/MetamodelVersionStore.kt @@ -56,6 +56,10 @@ interface MetamodelVersionStore { * re-save, along with the stamp's place in the write order: a re-saved old stamp does not * become the latest. * + * An implementation may fail a save with its backend's own concurrency exception when two + * writers race to save the same schema at once. Since the write is idempotent, the caller can + * simply retry it. + * * @param version The version to save. */ fun saveVersion(version: MetamodelVersion) diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt index 05bf83ff..0e598dad 100644 --- a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt @@ -58,11 +58,20 @@ import java.time.Clock * versions sharing a position in the order unstorable, so a lost counter update fails with a * constraint violation the caller can retry. * + * **Retrying a failed save.** If the counter's read-modify-write ever does lose an update, the + * second writer fails with a uniqueness-constraint violation on `(schemaName, sequence)`. + * [saveVersion] just lets that exception propagate; it doesn't retry internally, because the + * failure has already ended the surrounding Neo4j transaction, and a retry needs a new + * transaction, which only the caller can open. That's safe to do: the write is an idempotent + * upsert, so retrying a failed save never produces a duplicate or a wrong result. The drift check + * re-stamps its schema on every pass anyway, so for that caller the next pass already is the + * retry. + * * @param persistenceManager Drivine's handle on the `neo` datasource. * @param clock supplies the instant a version is stamped as saved at. Injectable so a test can pin * the instants of two saves. */ -open class DrivineMetamodelVersionStore( +class DrivineMetamodelVersionStore( private val persistenceManager: PersistenceManager, private val clock: Clock = Clock.systemUTC(), ) : MetamodelVersionStore { @@ -174,7 +183,9 @@ open class DrivineMetamodelVersionStore( * A single corrupt or tampered node shouldn't take down a whole history read, so the row is * logged at warn and skipped. [MetamodelVersionRowMapper] throws on bad data so that this can * happen; the warning names the missing property or the failed integrity check, which is what - * an operator needs to go find the node. + * an operator needs to go find the node. A row that isn't even a `Map` is logged and skipped + * the same way, naming its runtime class, so a count mismatch against what was expected still + * shows up in the log. * * [latestVersion] deliberately keeps `LIMIT 1` out of the Cypher. If the newest node were the * corrupt one, a database-side limit would read it, drop it, and answer "this schema has no @@ -185,7 +196,14 @@ open class DrivineMetamodelVersionStore( private fun readVersions(statement: String, bindings: Map): List { @Suppress("UNCHECKED_CAST") val spec = QuerySpecification.withStatement(statement).bind(bindings) as QuerySpecification - return persistenceManager.query(spec).filterIsInstance>().mapNotNull { row -> + return persistenceManager.query(spec).mapNotNull { row -> + if (row !is Map<*, *>) { + logger.warn( + "Skipping MetamodelVersion row: expected a Map, got {}", + row?.javaClass?.name ?: "null", + ) + return@mapNotNull null + } runCatching { MetamodelVersionRowMapper.fromRow(row) } .onFailure { logger.warn("Skipping unreadable MetamodelVersion row: {}", it.message) } .getOrNull() diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt index 3b9f5ecd..001e6de9 100644 --- a/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/MetamodelRowMappers.kt @@ -18,6 +18,7 @@ package com.embabel.dice.storage import com.embabel.agent.core.Cardinality import com.embabel.dice.metamodel.MetamodelVersion import com.embabel.dice.metamodel.PropertySignature +import com.fasterxml.jackson.core.type.TypeReference import com.fasterxml.jackson.databind.ObjectMapper import java.time.Instant @@ -45,7 +46,8 @@ private val objectMapper = ObjectMapper() * * Reads are strict. A property this mapper wrote must be present when it is read again; a node * missing one is corrupt, so the accessor throws and the store's surrounding guard skips the row - * with a warning. + * with a warning. An empty string is corrupt too: an empty collection is written as `[]` or + * `{}`, so `""` never comes from this mapper, and it fails the read like any other bad JSON. * * Two things are optional, and absent means "no former names were declared": the version-level * `entityTypeAliases` property, and the `aliases` field inside a stored property signature. Writing @@ -116,8 +118,7 @@ private fun serializeList(items: List): String = objectMapper.writeValueAsString(items) private fun deserializeList(serialized: String): List = - if (serialized.isEmpty()) emptyList() - else objectMapper.readValue( + objectMapper.readValue( serialized, objectMapper.typeFactory.constructCollectionType(List::class.java, String::class.java) ) @@ -146,16 +147,14 @@ private fun serializeAliasMap(aliases: Map>): String? = /** Inverse of [serializeAliasMap]; an absent property means no type declared a former name. */ private fun deserializeAliasMap(serialized: String?): Map> = - if (serialized.isNullOrEmpty()) emptyMap() else deserializeMapOfLabelSets(serialized) + if (serialized == null) emptyMap() else deserializeMapOfLabelSets(serialized) /** Inverse of [serializeMapOfLabelSets]. */ private fun deserializeMapOfLabelSets(serialized: String): Map> { - if (serialized.isEmpty()) return emptyMap() - @Suppress("UNCHECKED_CAST") val mapOfLists = objectMapper.readValue( serialized, - objectMapper.typeFactory.constructMapType(Map::class.java, String::class.java, List::class.java), - ) as Map> + object : TypeReference>>() {}, + ) return mapOfLists.mapValues { (_, labels) -> labels.toSet() } } @@ -204,12 +203,10 @@ private fun serializeMapOfSignatureSets(map: Map> * whole row with a message about a hash mismatch that hides the real problem. */ private fun deserializeMapOfSignatureSets(serialized: String): Map> { - if (serialized.isEmpty()) return emptyMap() - @Suppress("UNCHECKED_CAST") val mapOfLists = objectMapper.readValue( serialized, - objectMapper.typeFactory.constructMapType(Map::class.java, String::class.java, List::class.java), - ) as Map> + object : TypeReference>>() {}, + ) return mapOfLists.mapValues { (typeName, encoded) -> encoded.map { element -> val fields = element as? Map<*, *> diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt index 41a8da40..f865dcc7 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractMetamodelVersionStoreContractTest.kt @@ -18,12 +18,13 @@ package com.embabel.dice.storage import com.embabel.dice.metamodel.MetamodelVersion import com.embabel.dice.metamodel.MetamodelVersionStore import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Test /** - * Cross-backend contract for [MetamodelVersionStore.saveVersion]'s upsert. Each subclass supplies a - * store and inherits the whole suite, so a backend that disagrees with the in-memory reference fails - * at authoring time. + * Cross-backend contract for [MetamodelVersionStore]: the upsert, history ordering, keyed lookup, + * and schema isolation. Each subclass supplies a store and inherits the whole suite, so a backend + * that disagrees with the in-memory reference fails at authoring time. * * The rules here matter because the drift check re-stamps its schema on every pass. A store that * treated each of those re-stamps as a new record would fill the history with copies of one version, @@ -82,4 +83,58 @@ abstract class AbstractMetamodelVersionStoreContractTest { ) assertEquals(first, store.findVersion(schemaName, first.contentHash)) } + + // ---- keyed lookup ---- + + @Test + fun `findVersion returns null for a hash the schema has never stored`() { + val store = store() + val schemaName = "contract-find-miss" + val stamp = version(schemaName) + store.saveVersion(stamp) + + assertNull(store.findVersion(schemaName, "not-a-real-hash")) + assertNull(store.findVersion("contract-find-miss-other-schema", stamp.contentHash)) + } + + // ---- ordering ---- + + @Test + fun `versionHistory is newest first`() { + val store = store() + val schemaName = "contract-newest-first" + + store.saveVersion(version(schemaName, "First")) + store.saveVersion(version(schemaName, "Second")) + store.saveVersion(version(schemaName, "Third")) + + assertEquals( + listOf("Third", "Second", "First"), + store.versionHistory(schemaName).map { it.entityTypeNames.single() }, + ) + assertEquals("Third", store.latestVersion(schemaName)?.entityTypeNames?.single()) + } + + // ---- schema isolation ---- + + @Test + fun `one schema's writes are invisible to another`() { + val store = store() + val schemaA = "contract-isolation-a" + val schemaB = "contract-isolation-b" + + store.saveVersion(version(schemaA)) + + assertEquals(emptyList(), store.versionHistory(schemaB)) + assertNull(store.latestVersion(schemaB)) + + store.saveVersion(version(schemaB)) + + assertEquals(1, store.versionHistory(schemaA).size, "schema B's save must not touch schema A's history") + } + + @Test + fun `latestVersion is null for a schema with no versions`() { + assertNull(store().latestVersion("contract-never-saved")) + } } diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt index 175397e4..4a8efd80 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/MetamodelRowMapperTest.kt @@ -178,6 +178,16 @@ class MetamodelRowMapperTest { assertEquals(empty, MetamodelVersionRowMapper.fromRow(MetamodelVersionRowMapper.bindMap(empty, savedAt))) } + @Test + fun `a required JSON field holding the empty string fails the read`() { + // The writer never produces "" for one of these fields: an empty list or map is written as + // "[]" or "{}". A bare empty string only shows up through corruption, and it has to fail + // loudly, not read back as an empty collection that hides the problem. + val corrupt = row().apply { put("entityTypeNames", "") } + + assertThrows { MetamodelVersionRowMapper.fromRow(corrupt) } + } + // ---- Aliases: written only when declared, absent read as none ---- @Test From 842203f70c57129b8ace75d2f3e7c0604ec76ef3 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Thu, 3 Sep 2026 23:46:13 -0400 Subject: [PATCH 8/8] Mark the version store transactional at class level The allopen spring preset only opens a class that carries a Spring annotation itself, and this store annotated its methods alone, which is why it needed a hand-written open. Put @Transactional on the class like the other Drivine stores and let the reads keep their readOnly override. --- .../com/embabel/dice/storage/DrivineMetamodelVersionStore.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt index 0e598dad..02f7105e 100644 --- a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivineMetamodelVersionStore.kt @@ -71,6 +71,7 @@ import java.time.Clock * @param clock supplies the instant a version is stamped as saved at. Injectable so a test can pin * the instants of two saves. */ +@Transactional class DrivineMetamodelVersionStore( private val persistenceManager: PersistenceManager, private val clock: Clock = Clock.systemUTC(), @@ -140,7 +141,6 @@ class DrivineMetamodelVersionStore( """.trimIndent() } - @Transactional override fun saveVersion(version: MetamodelVersion) { logger.debug( "Saving metamodel version schemaName={} contentHash={}",