diff --git a/.changeset/21828-metadata-loader-one-content-hash.md b/.changeset/21828-metadata-loader-one-content-hash.md new file mode 100644 index 00000000000..aa1a17a4378 --- /dev/null +++ b/.changeset/21828-metadata-loader-one-content-hash.md @@ -0,0 +1,11 @@ +--- +'@objectstack/metadata': patch +--- + +`DatabaseLoader` now persists a `register` whose only change is the order of an object's `fields` (#21828). Before, the loader's checksum sorted every map, so a field reorder hashed equal to the stored row and was never written. The running process still saw the new order, but the persisted `sys_metadata` row kept the old one. + +Clause-②: no + +- **One hash vocabulary in `sys_metadata.checksum`.** The loader now stamps the hash `SysMetadataRepository` stamps on the same column: `hashSpec(body, type)` from `@objectstack/metadata-core`, written as `sha256:` + 64 hex. It is hashed as the item's metadata type, so a reorder of an object's `fields` is a change and every other map is still key-order independent. Its history rows carry the same value as the row they record. Before, the loader wrote bare hex from `calculateChecksum`. That function is unchanged and still exported, but nothing writes the column with it. +- **Rows stamped before this release.** Whether a `register` is a no-op is now decided by re-hashing the stored body, not by comparing the stored checksum. A row with an unchanged body is not rewritten: no version bump, no history row, and it keeps its old checksum until its content next changes. The first real change rewrites it with the new stamp. A reorder into sorted key order is written too, even against a stored checksum that sorted every map. +- **ETags.** `load()` and `stat()` report the row's checksum as `etag`, so a row the loader writes from this release on reports a `sha256:` value. diff --git a/packages/metadata/src/loaders/database-loader-21828-one-content-hash.test.ts b/packages/metadata/src/loaders/database-loader-21828-one-content-hash.test.ts new file mode 100644 index 00000000000..690162a9893 --- /dev/null +++ b/packages/metadata/src/loaders/database-loader-21828-one-content-hash.test.ts @@ -0,0 +1,201 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21828] `DatabaseLoader` stamps and compares the content hash + * `SysMetadataRepository` stamps on the same `sys_metadata` column, + * `hashSpec(body, type)`, so the column carries one vocabulary and a + * field-reorder-only `register` is persisted. + * + * Before this card the loader stamped `calculateChecksum` (bare hex, every map + * sorted) and skipped its write whenever the new stamp equalled the stored + * one. An `object` whose only change was the order of its `fields` hashed + * equal, so `MetadataManager.register` refreshed the in-process cache with + * the new order and the persisted row kept the old one. + * + * Driven through the door the card names, `MetadataManager.register`, onto + * real SQLite (`driver-sqlite-wasm`, already a devDependency), and read back + * from the rows, never from the loader's cache. + * + * Pins, per triage's grade on the card: + * §A a field-reorder-only `register` persists the new order; + * §B an unordered map's key swap still compares equal (no write, no history row); + * §C the upgrade: a row stamped under an older rule with an unchanged body + * is not rewritten, and a reorder against such a stamp is still persisted. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { hashSpec } from '@objectstack/metadata-core'; +import { DatabaseLoader } from './database-loader.js'; +import { MetadataManager } from '../metadata-manager.js'; +import { calculateChecksum } from '../utils/metadata-history-utils.js'; + +type Row = Record; + +const TITLE = { type: 'text', label: 'Title' }; +const AMOUNT = { type: 'number', label: 'Amount' }; +const DUE = { type: 'date', label: 'Due' }; + +/** An object whose `fields` are in the order given, and nothing else differs. */ +function invoice(order: ReadonlyArray<'title' | 'amount' | 'due'>): Row { + const all = { title: TITLE, amount: AMOUNT, due: DUE }; + const fields: Row = {}; + for (const key of order) fields[key] = { ...all[key] }; + return { name: 'invoice', label: 'Invoice', fields }; +} + +describe('[#21828] DatabaseLoader stamps and compares hashSpec(body, type)', () => { + let driver: SqliteWasmDriver; + let manager: MetadataManager; + + beforeEach(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.connect(); + const loader = new DatabaseLoader({ driver, trackHistory: true, cache: { enabled: false } }); + manager = new MetadataManager({ datasource: 'sqlite', loaders: [loader] }); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + async function row(type: string, name: string): Promise { + const found = await driver.findOne('sys_metadata', { where: { type, name } }); + expect(found, `${type}/${name} has a sys_metadata row`).toBeTruthy(); + return found as Row; + } + + async function history(type: string, name: string): Promise { + return (await driver.find('sys_metadata_history', { where: { type, name } })) as Row[]; + } + + function storedFieldOrder(r: Row): string[] { + return Object.keys((JSON.parse(r.metadata as string) as { fields: Row }).fields); + } + + /** Overwrite the stored stamp, as a row written under an older rule carries it. */ + async function restamp(r: Row, checksum: string): Promise { + await driver.update('sys_metadata', r.id as string, { checksum }); + } + + describe('§A a field-reorder-only register is persisted', () => { + it('stores the new order, bumps the version and appends one history row', async () => { + await manager.register('object', 'invoice', invoice(['title', 'amount', 'due'])); + await manager.register('object', 'invoice', invoice(['due', 'title', 'amount'])); + + const r = await row('object', 'invoice'); + expect(storedFieldOrder(r)).toEqual(['due', 'title', 'amount']); + expect(r.version).toBe(2); + const h = await history('object', 'invoice'); + expect(h.map((x) => x.operation_type).sort()).toEqual(['create', 'update']); + }); + + it('stamps the row and its history row with the repository vocabulary, one value', async () => { + await manager.register('object', 'invoice', invoice(['title', 'amount', 'due'])); + const first = await row('object', 'invoice'); + await manager.register('object', 'invoice', invoice(['due', 'title', 'amount'])); + const r = await row('object', 'invoice'); + + expect(r.checksum).toBe(hashSpec(invoice(['due', 'title', 'amount']), 'object')); + expect(r.checksum).not.toBe(first.checksum); + const update = (await history('object', 'invoice')).find((x) => x.operation_type === 'update')!; + expect(update.checksum).toBe(r.checksum); + expect(update.previous_checksum).toBe(first.checksum); + }); + + it('a rollback restores the earlier order and stamps it in the same vocabulary', async () => { + await manager.register('object', 'invoice', invoice(['title', 'amount', 'due'])); + await manager.register('object', 'invoice', invoice(['due', 'title', 'amount'])); + await manager.rollback('object', 'invoice', 1); + + const r = await row('object', 'invoice'); + expect(storedFieldOrder(r)).toEqual(['title', 'amount', 'due']); + expect(r.checksum).toBe(hashSpec(invoice(['title', 'amount', 'due']), 'object')); + }); + }); + + describe('§B a key swap in an unordered map still compares equal', () => { + it('object: swapping the keys around `fields` and inside a field writes nothing', async () => { + await manager.register('object', 'invoice', { + name: 'invoice', + label: 'Invoice', + fields: { title: { type: 'text', label: 'Title' } }, + }); + const before = await row('object', 'invoice'); + await manager.register('object', 'invoice', { + fields: { title: { label: 'Title', type: 'text' } }, + label: 'Invoice', + name: 'invoice', + }); + + const after = await row('object', 'invoice'); + expect(after.version).toBe(1); + expect(after.checksum).toBe(before.checksum); + expect(await history('object', 'invoice')).toHaveLength(1); + }); + + it('view: a type with no ordered map stays order-blind in every map', async () => { + await manager.register('view', 'invoice_grid', { + name: 'invoice_grid', + label: 'Invoices', + options: { density: 'compact', striped: true }, + }); + await manager.register('view', 'invoice_grid', { + options: { striped: true, density: 'compact' }, + label: 'Invoices', + name: 'invoice_grid', + }); + + const r = await row('view', 'invoice_grid'); + expect(r.version).toBe(1); + expect(await history('view', 'invoice_grid')).toHaveLength(1); + }); + }); + + describe('§C the upgrade: content, not a stamp from an older rule, decides "unchanged"', () => { + it('a row stamped by the old calculateChecksum with an unchanged body is not rewritten', async () => { + const body = invoice(['title', 'amount', 'due']); + await manager.register('object', 'invoice', body); + const legacy = await calculateChecksum(body); + await restamp(await row('object', 'invoice'), legacy); + + await manager.register('object', 'invoice', invoice(['title', 'amount', 'due'])); + + const r = await row('object', 'invoice'); + expect(r.version).toBe(1); + expect(r.checksum).toBe(legacy); + expect(await history('object', 'invoice')).toHaveLength(1); + }); + + it('the same holds for a type with no ordered map', async () => { + const body = { name: 'invoice_grid', label: 'Invoices' }; + await manager.register('view', 'invoice_grid', body); + const legacy = await calculateChecksum(body); + await restamp(await row('view', 'invoice_grid'), legacy); + + await manager.register('view', 'invoice_grid', { ...body }); + + const r = await row('view', 'invoice_grid'); + expect(r.version).toBe(1); + expect(r.checksum).toBe(legacy); + expect(await history('view', 'invoice_grid')).toHaveLength(1); + }); + + it('a reorder INTO sorted key order against an order-blind stamp is persisted', async () => { + // `hashSpec(body)` with no type is the order-blind `sha256:` stamp a row + // carries when `SysMetadataRepository` wrote it before #21790. It IS the + // hash of the sorted order, so a stamp comparison calls this reorder + // "unchanged". + await manager.register('object', 'invoice', invoice(['title', 'due', 'amount'])); + await restamp(await row('object', 'invoice'), hashSpec(invoice(['title', 'due', 'amount']))); + + await manager.register('object', 'invoice', invoice(['amount', 'due', 'title'])); + + const r = await row('object', 'invoice'); + expect(storedFieldOrder(r)).toEqual(['amount', 'due', 'title']); + expect(r.version).toBe(2); + expect(r.checksum).toBe(hashSpec(invoice(['amount', 'due', 'title']), 'object')); + expect(await history('object', 'invoice')).toHaveLength(2); + }); + }); +}); diff --git a/packages/metadata/src/loaders/database-loader.ts b/packages/metadata/src/loaders/database-loader.ts index 6a472096e20..4da1f0736df 100644 --- a/packages/metadata/src/loaders/database-loader.ts +++ b/packages/metadata/src/loaders/database-loader.ts @@ -19,13 +19,12 @@ import type { MetadataRecord, MetadataHistoryRecord, } from '@objectstack/spec/system'; -import { SysMetadataObject, SysMetadataHistoryObject } from '@objectstack/metadata-core'; +import { SysMetadataObject, SysMetadataHistoryObject, hashSpec } from '@objectstack/metadata-core'; import { applyConversionsToStoredItem } from '@objectstack/spec'; import { lowerFilterCondition, type FilterLoweringOptions } from '@objectstack/spec/data'; import { PLURAL_TO_SINGULAR } from '@objectstack/spec/shared'; import type { IDataDriver, IDataEngine, DriverQuery } from '@objectstack/spec/contracts'; import type { MetadataLoader, MetadataKeyedItem } from './loader-interface.js'; -import { calculateChecksum } from '../utils/metadata-history-utils.js'; import { LRUCache } from '../utils/lru-cache.js'; // [commit 6a180e42d] Both predicates moved to `@objectstack/types` — see its // `driver-error-classification.ts` `## Home` section. The verdicts are @@ -698,12 +697,23 @@ export class DatabaseLoader implements MetadataLoader { /** * Create a history record for a metadata change. * + * [#21828] The caller has already decided this IS a change and hands in the + * stamp it wrote on the parent row ({@link contentHash}), so the history row + * carries the same value. ⛔ No "unchanged?" comparison here: the only + * `'update'` caller, `save`, reaches this line after comparing CONTENT, and a + * second comparison of the new stamp with the previous STORED stamp is never + * the right arbiter. A stamp written under an older rule can equal the new + * one while the content differs (a reorder of an object's `fields` into + * sorted key order, against a stamp that sorted every map), which would + * record a real change with no history row. + * * @param type - Metadata type * @param name - Metadata name * @param version - Version number * @param metadata - The metadata payload * @param operationType - Type of operation - * @param previousChecksum - Checksum of previous version (if any) + * @param checksum - The parent row's new stamp, `contentHash` of the stored body + * @param previousChecksum - The parent row's stamp before this write (if any) * @param changeNote - Optional change description * @param recordedBy - Optional user who made the change */ @@ -713,6 +723,7 @@ export class DatabaseLoader implements MetadataLoader { version: number, metadata: unknown, operationType: 'create' | 'update' | 'publish' | 'revert' | 'delete', + checksum: string, previousChecksum?: string, changeNote?: string, recordedBy?: string @@ -722,12 +733,6 @@ export class DatabaseLoader implements MetadataLoader { await this.ensureHistorySchema(); const now = new Date().toISOString(); - const checksum = await calculateChecksum(metadata); - - // Skip if checksum matches previous version (no actual change) - if (previousChecksum && checksum === previousChecksum && operationType === 'update') { - return; - } const historyId = generateId(); const metadataJson = JSON.stringify(metadata); @@ -1353,7 +1358,8 @@ export class DatabaseLoader implements MetadataLoader { const now = new Date().toISOString(); const metadataJson = JSON.stringify(restoredData); - const newChecksum = await calculateChecksum(restoredData); + // [#21828] The one stamp vocabulary of `sys_metadata` — see {@link contentHash}. + const newChecksum = contentHash(metadataJson, type); const existing = await this._findOne(this.tableName, { where: this.baseFilter(type, name), @@ -1383,6 +1389,7 @@ export class DatabaseLoader implements MetadataLoader { newVersion, restoredData, 'revert', + newChecksum, previousChecksum, changeNote ?? `Rolled back to version ${targetVersion}`, recordedBy @@ -1401,17 +1408,19 @@ export class DatabaseLoader implements MetadataLoader { const now = new Date().toISOString(); const metadataJson = JSON.stringify(data); - const newChecksum = await calculateChecksum(data); try { + // [#21828] The one stamp vocabulary of `sys_metadata` — see {@link contentHash}. + const newChecksum = contentHash(metadataJson, type); const existing = await this._findOne(this.tableName, { where: this.baseFilter(type, name), }); if (existing) { - // Skip update if the content is identical (prevents phantom version bumps) + // Skip the write when the stored CONTENT is identical (prevents phantom + // version bumps). [#21828] Content, not stamps: see `storedBodyUnchanged`. const previousChecksum = existing.checksum as string | undefined; - if (newChecksum === previousChecksum) { + if (storedBodyUnchanged(existing, type, newChecksum)) { // No DB write, but make sure the cached payload reflects the latest // call (prior cached `null` would otherwise mask a freshly-saved // record). @@ -1444,6 +1453,7 @@ export class DatabaseLoader implements MetadataLoader { version, data, 'update', + newChecksum, previousChecksum ); @@ -1481,7 +1491,8 @@ export class DatabaseLoader implements MetadataLoader { name, 1, data, - 'create' + 'create', + newChecksum ); return { @@ -1523,6 +1534,62 @@ export class DatabaseLoader implements MetadataLoader { } } +/** + * [#21828] The content hash this loader stamps on a `sys_metadata` row and on + * its history row: `hashSpec(body, type)` from `@objectstack/metadata-core`, + * the hash `SysMetadataRepository` stamps on the same column. One column, one + * vocabulary (`sha256:` + 64 hex). ⛔ Not `calculateChecksum`: it sorts every + * map, an object's `fields` included, and writes bare hex, so a field-reorder- + * only `register` hashed equal and was never persisted. + * + * Hashed AS ITS TYPE (canonicalize guarantee 8): a map the spec declares + * ordered (an `object`'s `fields`) keeps its key order, so a reorder of it is a + * change, and every other map stays key-order independent. + * + * It hashes the bytes the loader stores (the parse of the JSON it writes), not + * the in-memory value it was handed. Canonicalize guarantee 7 makes the two one + * hash for every value JSON can carry. For a value it cannot (a function or an + * `undefined` property, which the stored JSON drops), the stamp still names + * the stored bytes, and `hashSpec` is never handed what it refuses. + */ +function contentHash(storedJson: string, type: string): string { + return hashSpec(JSON.parse(storedJson), type); +} + +/** + * [#21828] Does `row` already store the body whose {@link contentHash} is + * `hash`? `save`'s no-op question, answered about CONTENT: the stored bytes + * are re-hashed under the current rule, the comparison + * `SysMetadataRepository.put` makes for an ordered-map type + * (`storedBodyUnchanged` there). + * + * The stored stamp cannot answer it, because a row's stamp may come from an + * older rule. Before #21828 this loader stamped `calculateChecksum` (bare hex, + * every map sorted), and a row `SysMetadataRepository` stamped before #21790 + * carries an order-blind `sha256:` hash. Trusting the stamp misfires both ways: + * - an identical body looks changed against a stamp no current hash equals, + * so every such row would be rewritten with a version bump and a history + * row recording no change, on the first `register` after an upgrade; + * - a reorder of an object's `fields` INTO sorted key order looks unchanged + * against an order-blind stamp, the drop #21828 fixes. + * The stamp of a skipped row is left as written: it stays the row's version + * token until its content next changes. + * + * Stored bytes that are absent or do not parse are not the incoming body, so + * the answer is "changed" and the write goes ahead, which also repairs the row. + */ +function storedBodyUnchanged(row: Record, type: string, hash: string): boolean { + const stored = row.metadata; + if (stored === null || stored === undefined) return false; + let storedBody: unknown; + try { + storedBody = typeof stored === 'string' ? JSON.parse(stored) : stored; + } catch { + return false; + } + return hashSpec(storedBody, type) === hash; +} + /** * Generate a simple unique ID for metadata records. * Uses crypto.randomUUID when available, falls back to timestamp-based ID. diff --git a/packages/metadata/src/utils/metadata-history-utils.ts b/packages/metadata/src/utils/metadata-history-utils.ts index b9661a1fbe9..c20971d6f6f 100644 --- a/packages/metadata/src/utils/metadata-history-utils.ts +++ b/packages/metadata/src/utils/metadata-history-utils.ts @@ -12,6 +12,12 @@ * Normalizes the JSON by sorting keys and removing whitespace * to ensure consistent checksums across identical content. * + * ⚠️ Not the `sys_metadata.checksum` stamp (#21828). That column carries + * `hashSpec(body, type)` from `@objectstack/metadata-core`, which every writer + * of it stamps (`DatabaseLoader`, `SysMetadataRepository`). This function sorts + * EVERY map, an object's `fields` included, whose key order the spec declares + * to be the field order, so it cannot tell a field reorder from no change. + * * @param metadata - The metadata object to checksum * @returns SHA-256 hex string */