fix(server): converge app-sync isUnique diff for single-column unique constraints (#22592)
## Context Fixes #22550. On app sync (`twenty dev`), a field backed by a single-column unique constraint produces a **permanent, non-converging `[isUnique] changed` field-metadata diff**: the change is reported, the apply succeeds, and the identical change is reported again on the very next sync. ## Root cause Since the uniqueness-source-of-truth moved from a `FieldMetadata.isUnique` column to `IndexMetadata` (@FelixMalfait's #20846 / #20883), `field.isUnique` is a **derived** property. The two sides of the app-sync diff derive it differently: - **"from" side** (workspace cache, `WorkspaceFlatFieldMetadataMapCacheService`) derives `isUnique` from indexes via `computeUniqueFieldMetadataIdsFromIndexes`, which counted **any** single-column `UNIQUE` index. - **"to" side** (`from-field-manifest-to-universal-flat-field-metadata.util.ts`) sets `isUnique` from the field-level manifest flag (`fieldManifest.isUnique ?? false`). For a field whose uniqueness is declared with `defineIndex({ isUnique: true, fields: [oneField] })` (no field-level flag): - "from" derives `true` (the custom unique index exists), - "to" is `false` (no field-level flag), so the diff emits a `fieldMetadata … [isUnique] changed` update forever. The field-update runner drops `isUnique` before the SQL `UPDATE` (it has no column), and the custom index persists, so the derived value never changes — the loop cannot converge. ## Fix Restrict the derivation in `computeUniqueFieldMetadataIdsFromIndexes` to the field's **engine-owned backing constraint** — a `UNIQUE` index with `isSystemSideEffect: true` — rather than any single-column unique index. This makes `field.isUnique` mean the same thing on both sides: | declaration | backing index (`isSystemSideEffect`) | "from" derived | "to" flag | converges | |---|---|---|---|---| | field-level `isUnique: true` | side-effect handler generates it → `true` | `true` | `true` | ✅ | | `defineIndex({ isUnique: true, fields:[x] })` | custom index → `false` | `false` | `false` | ✅ (index converges on its own) | Standard objects and the create/update side-effect backing indexes are all `isSystemSideEffect: true` (`create-standard-index-flat-metadata.util.ts`, `generate-deterministic-index-for-flat-field-metadata-or-throw.util.ts`), so their fields keep `isUnique = true`. Only a user-declared custom `defineIndex` unique index (`isSystemSideEffect: false`) is now excluded — which is also what stops the create/update side-effect from generating a **second, duplicate** backing index for a field the custom index already covers (which would otherwise trip `DUPLICATE_UNIQUE_INDEX` on first apply). ## Why this location (and not the manifest "to" side) An earlier attempt derived `isUnique` on the manifest "to" side from the built indexes. That breaks after #22295: `field.isUnique === true` is the **trigger** for the `fieldUniqueBackingIndexOnCreate/Update` side-effect handlers, so forcing it to derive from the compute-service maps (which don't yet contain the not-yet-generated backing index) would suppress the backing index for field-level unique fields. Narrowing the shared "from" derivation keeps the side-effect trigger intact and makes both sides symmetric in one place. ## For review — @FelixMalfait This touches the uniqueness model you own in #20883 ("make IndexMetadata the source of truth for uniqueness"), and interacts with the side-effect engine from #22295. The semantic change is: **`field.isUnique` now reflects only the field's backing constraint, not an arbitrary user-declared single-column unique index.** A `defineIndex`-declared single-column unique field now surfaces `isUnique: false` on the field (the constraint is still enforced by the index). If instead you'd want `defineIndex` single-column uniqueness to surface as `field.isUnique: true`, the fix would need to live in the side-effect engine (dedupe the backing index against the declared one) rather than the derivation — happy to take it that direction. Flagging for your call before this leaves draft. ## Related - Issue #22550 - @FelixMalfait #20883, #20846 (IndexMetadata as source of truth for uniqueness) - #22295 (centralized side-effect engine — unique field backing index) - #21383 (adjacent field-`isUnique` handling) https://claude.ai/code/session_01T1Cqvt5tHS6tZ1FeQQWyRo
This commit is contained in:
+90
@@ -0,0 +1,90 @@
|
||||
import { computeUniqueFieldMetadataIdsFromIndexes } from 'src/engine/metadata-modules/index-metadata/utils/compute-unique-field-metadata-ids-from-indexes.util';
|
||||
|
||||
const FIELD_A = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa';
|
||||
const FIELD_B = 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb';
|
||||
|
||||
describe('computeUniqueFieldMetadataIdsFromIndexes', () => {
|
||||
it('includes a field backed by a single-column system-side-effect unique index', () => {
|
||||
const result = computeUniqueFieldMetadataIdsFromIndexes([
|
||||
{
|
||||
isUnique: true,
|
||||
isSystemSideEffect: true,
|
||||
flatIndexFieldMetadatas: [
|
||||
{ fieldMetadataId: FIELD_A, subFieldName: null },
|
||||
],
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.has(FIELD_A)).toBe(true);
|
||||
});
|
||||
|
||||
it('excludes a single-column unique index that is not a system side effect (e.g. a user defineIndex)', () => {
|
||||
const result = computeUniqueFieldMetadataIdsFromIndexes([
|
||||
{
|
||||
isUnique: true,
|
||||
isSystemSideEffect: false,
|
||||
flatIndexFieldMetadatas: [
|
||||
{ fieldMetadataId: FIELD_A, subFieldName: null },
|
||||
],
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.has(FIELD_A)).toBe(false);
|
||||
});
|
||||
|
||||
it('excludes a non-unique index', () => {
|
||||
const result = computeUniqueFieldMetadataIdsFromIndexes([
|
||||
{
|
||||
isUnique: false,
|
||||
isSystemSideEffect: true,
|
||||
flatIndexFieldMetadatas: [
|
||||
{ fieldMetadataId: FIELD_A, subFieldName: null },
|
||||
],
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.has(FIELD_A)).toBe(false);
|
||||
});
|
||||
|
||||
it('excludes fields that are part of a multi-column unique index', () => {
|
||||
const result = computeUniqueFieldMetadataIdsFromIndexes([
|
||||
{
|
||||
isUnique: true,
|
||||
isSystemSideEffect: true,
|
||||
flatIndexFieldMetadatas: [
|
||||
{ fieldMetadataId: FIELD_A, subFieldName: null },
|
||||
{ fieldMetadataId: FIELD_B, subFieldName: null },
|
||||
],
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.has(FIELD_A)).toBe(false);
|
||||
expect(result.has(FIELD_B)).toBe(false);
|
||||
});
|
||||
|
||||
it('excludes a unique index on a composite sub-field', () => {
|
||||
const result = computeUniqueFieldMetadataIdsFromIndexes([
|
||||
{
|
||||
isUnique: true,
|
||||
isSystemSideEffect: true,
|
||||
flatIndexFieldMetadatas: [
|
||||
{ fieldMetadataId: FIELD_A, subFieldName: 'primaryLinkUrl' },
|
||||
],
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.has(FIELD_A)).toBe(false);
|
||||
});
|
||||
|
||||
it('supports the entity-shaped indexFieldMetadatas key', () => {
|
||||
const result = computeUniqueFieldMetadataIdsFromIndexes([
|
||||
{
|
||||
isUnique: true,
|
||||
isSystemSideEffect: true,
|
||||
indexFieldMetadatas: [{ fieldMetadataId: FIELD_A, subFieldName: null }],
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.has(FIELD_A)).toBe(true);
|
||||
});
|
||||
});
|
||||
+20
-3
@@ -1,5 +1,6 @@
|
||||
type IndexLike = {
|
||||
isUnique: boolean;
|
||||
isSystemSideEffect: boolean;
|
||||
flatIndexFieldMetadatas?: Array<{
|
||||
fieldMetadataId: string;
|
||||
subFieldName: string | null;
|
||||
@@ -10,9 +11,24 @@ type IndexLike = {
|
||||
}>;
|
||||
};
|
||||
|
||||
// A field is "unique" iff there exists a UNIQUE IndexMetadata whose single
|
||||
// member is exactly that field (no composite sub-field). Centralized so the
|
||||
// cache builder, REST controller, and any future consumer agree on the rule.
|
||||
// A field is "unique" iff a UNIQUE IndexMetadata that the engine owns as the
|
||||
// field's backing constraint (isSystemSideEffect) has exactly that field as its
|
||||
// single member (no composite sub-field).
|
||||
//
|
||||
// The isSystemSideEffect guard is what keeps `field.isUnique` symmetric between
|
||||
// the app-sync "from" side (this derivation, over the workspace cache) and the
|
||||
// "to" side (the field-level flag on the manifest). A user-declared custom
|
||||
// UNIQUE index — e.g. `defineIndex({ isUnique: true, fields: [oneField] })` —
|
||||
// is isSystemSideEffect: false: it enforces uniqueness at the DB level but is
|
||||
// not the field's backing constraint, so it must NOT flip `field.isUnique`.
|
||||
// Counting it would make the cache report `true` while the manifest field
|
||||
// reports `false`, producing a phantom `[isUnique] changed` diff that reapplies
|
||||
// on every sync and never converges. Excluding it also stops the create/update
|
||||
// side-effect from generating a second, duplicate backing index for a field
|
||||
// whose uniqueness the custom index already provides.
|
||||
//
|
||||
// Centralized so the cache builder, REST controller, and any future consumer
|
||||
// agree on the rule.
|
||||
export const computeUniqueFieldMetadataIdsFromIndexes = (
|
||||
indexes: ReadonlyArray<IndexLike>,
|
||||
): Set<string> => {
|
||||
@@ -20,6 +36,7 @@ export const computeUniqueFieldMetadataIdsFromIndexes = (
|
||||
|
||||
for (const index of indexes) {
|
||||
if (!index.isUnique) continue;
|
||||
if (!index.isSystemSideEffect) continue;
|
||||
|
||||
const fields = index.flatIndexFieldMetadatas ?? index.indexFieldMetadatas;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user