Skip to content

Commit 7625370

Browse files
committed
Merge main (PR #3112 landed) into the RelatedList sort consolidation branch
2 parents e7e0e31 + 24e0e0a commit 7625370

8 files changed

Lines changed: 510 additions & 45 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
"@object-ui/core": patch
3+
---
4+
5+
chore(core): deprecate `ObjectValidationEngine` — rule enforcement stays single-implementation on the server (#3110)
6+
7+
`ObjectValidationEngine` / `defaultObjectValidationEngine` / `validateRecord` are
8+
now `@deprecated`. **Nothing is removed and no behaviour changes** — existing
9+
callers keep working exactly as they did after #3103.
10+
11+
**Why.** Object-level validation rules are *enforcement*, and enforcement is
12+
single-implementation on the server (`objectql`'s rule-validator). objectui
13+
already draws that line for the predicates it *does* evaluate client-side:
14+
`evaluator/fieldRules.ts` handles the presentation predicates (`visibleWhen` /
15+
`readonlyWhen` / `requiredWhen`) by delegating to the canonical
16+
`ExpressionEngine`, "rather than re-implementing a parallel evaluator"
17+
(ADR-0036). This engine was that parallel evaluator, on the enforcement side.
18+
19+
#3103 converged its semantics onto the server rule-for-rule, with eight
20+
mutation-tested gates — and still left a known divergence: the server carries
21+
ADR-0113's legacy-violation exemption (reject only when the merged state violates
22+
*and* this write makes it worse), which this engine does not implement. Editing
23+
an unrelated field on a legacy row would be blocked here and accepted there. One
24+
careful pass still left a gap, which is the argument: mirroring cross-repo
25+
behaviour is structurally unreliable, not unreliable-this-time.
26+
27+
**What to use instead.** Let the write fail and render the server's rejection —
28+
it is already structured (`field` / `code` / `message`, plus a label since
29+
objectstack#3957). For pre-submit feedback, the answer is a validate-only
30+
(dry-run) write on the server: identical UX, zero parity risk, and it covers the
31+
two rule kinds a client can never check — `unique` (needs the database) and
32+
`json_schema` (ajv lives server-side).
33+
34+
**The decision is a mechanism, not a comment.** What #3103 removed was a doc
35+
comment claiming spec canonicity that had been false for fifteen majors;
36+
shipping its successor as another comment would repeat the mistake one level up.
37+
`validation-engine-stays-unwired.test.ts` scans `packages/*/src` and fails if a
38+
production module starts referencing the engine, naming the file and the issue to
39+
reverse first. Barrels still re-export it — publishing a deprecated API is not
40+
wiring — and host applications are free to keep importing it.
41+
42+
The five spec-derived rule TYPES in `@object-ui/types` are unaffected: they are
43+
the anchor for objectstack#4115's ledger and are independent of whether objectui
44+
ships an engine.
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@object-ui/data-objectstack": patch
3+
---
4+
5+
fix(data-objectstack): a string `$orderby` reaches the server as a sort instead of a list of character indices — #3106
6+
7+
`QueryParams['$orderby']` declares four shapes — `string`, `string[]`,
8+
`SortNode[]`, `Record<field, direction>`. Both of this adapter's `find()` routes
9+
(`convertQueryParams` for a plain read, `rawFindWithPopulate` for one carrying
10+
`$expand`/`$search`) carried their own copy of the fold that serializes it, and
11+
both copies handled the same three. The bare string fell through to the
12+
`Record` branch, where `Object.entries('name asc')` enumerates the string's
13+
character indices — so the request went out as `sort=0,1,2,3,4,5,6,7`.
14+
15+
Since `objectstack#4226` the server refuses a sort it cannot read
16+
(`400 INVALID_SORT`) rather than dropping it silently, so this was not a
17+
degraded ordering but a list that failed to load outright — and `"${field}
18+
${order}"` is exactly the shape `ObjectGrid` builds from its view metadata's
19+
`sort`, making every standalone grid with a configured sort a broken one.
20+
21+
Both routes now share one exported `serializeOrderBy`, for the same reason the
22+
filter path already shares one: two copies of a fold can only agree by
23+
inspection, and these two did not.
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* `ObjectValidationEngine` stays unwired — the mechanism behind #3110.
11+
*
12+
* The decision was: validation rules are ENFORCEMENT, enforcement is
13+
* single-implementation on the server, and objectui renders the server's
14+
* rejection rather than predicting it. `evaluator/fieldRules.ts` draws the same
15+
* line for the presentation predicates it DOES evaluate client-side — it
16+
* delegates to the canonical `ExpressionEngine` "rather than re-implementing a
17+
* parallel evaluator" (ADR-0036). This engine is that parallel evaluator.
18+
*
19+
* #3103 converged it onto the server rule-for-rule and still left a known
20+
* divergence (ADR-0113's legacy-violation exemption), which is the whole
21+
* argument: mirroring cross-repo behaviour is structurally unreliable.
22+
*
23+
* Why a test and not just the `@deprecated` tag: the thing this replaced was a
24+
* doc comment claiming spec canonicity that had been false for fifteen majors
25+
* (#3103). Shipping the successor decision as another comment would repeat the
26+
* mistake one level up — a rule that is not a check is not a rule (#3017,
27+
* objectstack#4115). So the ban is compiled here: wire the engine into a
28+
* production module and this file names the file, the line, and the issue to
29+
* reverse first.
30+
*
31+
* This is deliberately a REACHABILITY ban, not a mention ban. Nothing stops a
32+
* host application from importing the deprecated engine — it is still exported,
33+
* still functional. What is banned is objectui itself growing a client-side
34+
* enforcement path again.
35+
*/
36+
37+
import { describe, it, expect } from 'vitest';
38+
import { readdirSync, readFileSync, statSync } from 'node:fs';
39+
import { join, resolve, relative } from 'node:path';
40+
41+
/** `packages/` — three levels up from `packages/core/src/validation/__tests__`. */
42+
const PACKAGES_DIR = resolve(__dirname, '../../../..');
43+
44+
/**
45+
* The engine's own module, its barrels, and its tests. A barrel re-export is not
46+
* a wiring: it publishes the deprecated API for host applications, which is
47+
* exactly what "deprecated but not removed" means.
48+
*/
49+
const ALLOWED = [
50+
'core/src/validation/validators/object-validation-engine.ts',
51+
'core/src/validation/validators/index.ts',
52+
'core/src/validation/index.ts',
53+
];
54+
55+
/** The engine's module path, and the entry points a caller would name. */
56+
const ENGINE_REFERENCE =
57+
/object-validation-engine|ObjectValidationEngine|defaultObjectValidationEngine/;
58+
59+
/**
60+
* Strip comments and string bodies before looking for a reference.
61+
*
62+
* Prose is not a wiring: `@object-ui/types` names the engine in a doc comment
63+
* explaining which defaults it applies, and that must not read as a production
64+
* import. Scanning the CODE (rather than the import statements alone) still
65+
* catches the indirect route — `import * as core` followed by
66+
* `core.ObjectValidationEngine` — which an import-list regex would miss.
67+
*
68+
* String bodies go too, so a URL or an error message quoting the name is prose
69+
* as well. Template-literal substitutions are kept, since that is real code.
70+
*/
71+
function stripCommentsAndStrings(src: string): string {
72+
let out = '';
73+
let i = 0;
74+
while (i < src.length) {
75+
const two = src.slice(i, i + 2);
76+
if (two === '//') {
77+
while (i < src.length && src[i] !== '\n') i++;
78+
continue;
79+
}
80+
if (two === '/*') {
81+
i += 2;
82+
while (i < src.length && src.slice(i, i + 2) !== '*/') i++;
83+
i += 2;
84+
continue;
85+
}
86+
const ch = src[i];
87+
if (ch === '"' || ch === "'" || ch === '`') {
88+
const quote = ch;
89+
i++;
90+
while (i < src.length) {
91+
if (src[i] === '\\') {
92+
i += 2;
93+
continue;
94+
}
95+
// Keep `${…}` substitutions — they are code, not prose.
96+
if (quote === '`' && src.slice(i, i + 2) === '${') {
97+
let depth = 1;
98+
i += 2;
99+
const start = i;
100+
while (i < src.length && depth > 0) {
101+
if (src[i] === '{') depth++;
102+
else if (src[i] === '}') depth--;
103+
if (depth > 0) i++;
104+
}
105+
out += src.slice(start, i);
106+
i++;
107+
continue;
108+
}
109+
if (src[i] === quote) break;
110+
i++;
111+
}
112+
i++;
113+
// A module specifier is a string, so preserve the ones that name the
114+
// engine's module — an `import … from '…/object-validation-engine'` must
115+
// still be caught.
116+
continue;
117+
}
118+
out += ch;
119+
i++;
120+
}
121+
return out;
122+
}
123+
124+
/** Module specifiers are strings, so they are checked separately. */
125+
const IMPORT_SPECIFIER = /(?:from|import|require)\s*\(?\s*['"]([^'"]+)['"]/g;
126+
127+
function referencesEngine(src: string): boolean {
128+
if (ENGINE_REFERENCE.test(stripCommentsAndStrings(src))) return true;
129+
for (const [, specifier] of src.matchAll(IMPORT_SPECIFIER)) {
130+
if (specifier.includes('object-validation-engine')) return true;
131+
}
132+
return false;
133+
}
134+
135+
function sourceFiles(): string[] {
136+
const out: string[] = [];
137+
const walk = (dir: string) => {
138+
for (const entry of readdirSync(dir, { withFileTypes: true })) {
139+
const full = join(dir, entry.name);
140+
if (entry.isDirectory()) {
141+
// Tests are not a production path, and `dist` is build output.
142+
if (['node_modules', 'dist', '__tests__'].includes(entry.name)) continue;
143+
walk(full);
144+
continue;
145+
}
146+
if (!/\.tsx?$/.test(entry.name) || /\.d\.ts$/.test(entry.name)) continue;
147+
if (/\.(test|stories)\.tsx?$/.test(entry.name)) continue;
148+
out.push(full);
149+
}
150+
};
151+
152+
for (const pkg of readdirSync(PACKAGES_DIR)) {
153+
const src = join(PACKAGES_DIR, pkg, 'src');
154+
try {
155+
if (!statSync(src).isDirectory()) continue;
156+
} catch {
157+
continue;
158+
}
159+
walk(src);
160+
}
161+
return out;
162+
}
163+
164+
describe('the deprecated ObjectValidationEngine stays out of production paths (#3110)', () => {
165+
it('scans a plausible source tree — so a passing result means something', () => {
166+
// Without this, a broken path silently scans zero files and the ban above
167+
// reports success forever. That failure mode is the one objectstack#4115
168+
// was filed about, so the guard guards itself.
169+
const files = sourceFiles();
170+
expect(files.length).toBeGreaterThan(500);
171+
expect(
172+
files.some((f) => f.endsWith('core/src/validation/validators/object-validation-engine.ts'))
173+
).toBe(true);
174+
});
175+
176+
it('is referenced only by its own module and the barrels that publish it', () => {
177+
const offenders = sourceFiles()
178+
.filter((file) => referencesEngine(readFileSync(file, 'utf8')))
179+
.map((file) => relative(PACKAGES_DIR, file))
180+
.filter((rel) => !ALLOWED.includes(rel))
181+
.sort();
182+
183+
expect(
184+
offenders,
185+
offenders.length === 0
186+
? ''
187+
: `A production module now references the deprecated ObjectValidationEngine:\n` +
188+
offenders.map((o) => ` - packages/${o}`).join('\n') +
189+
`\n\n` +
190+
`#3110 decided objectui does NOT run object-level validation rules on the\n` +
191+
`client: enforcement is single-implementation on the server, and mirroring\n` +
192+
`it drifts (see ADR-0113's legacy-violation exemption, still divergent).\n\n` +
193+
`For pre-submit feedback, use a validate-only (dry-run) write instead — it\n` +
194+
`also covers \`unique\` and \`json_schema\`, which a client cannot check.\n\n` +
195+
`If you are deliberately reversing the decision (e.g. offline writes),\n` +
196+
`reverse it in #3110 first, land the spec conformance fixtures it names,\n` +
197+
`then update ALLOWED here with the reason.`
198+
).toEqual([]);
199+
});
200+
});

packages/core/src/validation/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,15 @@
22
* @object-ui/core - Validation Module
33
*
44
* Phase 3.5: Validation engine
5+
*
56
* Object-level validation. The rule vocabulary is owned by
67
* `@objectstack/spec/data` and derived in `@object-ui/types`; canonicity is
78
* carried by that derivation and its parity gate, not by this comment.
9+
*
10+
* The object-level rule ENGINE (`validators/`) is @deprecated (#3110) — the
11+
* server is the single implementation of rule enforcement. Field/record SHAPE
12+
* validation (`schema-validator.js`, `validation-engine.js`) is unaffected: it
13+
* is client-side form validation, not a mirror of a server rule.
814
*/
915

1016
export * from './validation-engine.js';

packages/core/src/validation/validators/index.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,15 @@
88

99
/**
1010
* @object-ui/core - Validators
11-
*
12-
* Validators for the spec-owned object-level rule vocabulary.
13-
*
11+
*
12+
* Publishes the @deprecated `ObjectValidationEngine` (#3110). Object-level
13+
* validation rules are enforcement, and enforcement is single-implementation on
14+
* the server (`objectql`'s rule-validator); objectui renders the server's
15+
* rejection rather than predicting it, and pre-submit feedback belongs in a
16+
* validate-only (dry-run) write. This barrel keeps the API exported for host
17+
* applications that already depend on it — re-exporting is not wiring, and
18+
* `validation-engine-stays-unwired.test.ts` enforces that distinction.
19+
*
1420
* @module validators
1521
* @packageDocumentation
1622
*/

packages/core/src/validation/validators/object-validation-engine.ts

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,55 @@
77
*/
88

99
/**
10-
* @object-ui/core - Object-Level Validation Engine
10+
* @object-ui/core - Object-Level Validation Engine — **DEPRECATED** (#3110)
1111
*
12-
* A CLIENT-SIDE PRE-CHECK of the rules an object declares in
12+
* A client-side pre-check of the rules an object declares in
1313
* `ObjectSchema.validations`. The authority is the server
1414
* (`objectql/src/validation/rule-validator.ts`), which evaluates the same rules
15-
* on the write path; this engine exists to surface a violation before the round
16-
* trip, never to invent one. That framing sets every semantic below — a
17-
* pre-check that disagrees with the server is worse than no pre-check, because
18-
* it either blocks writes the server would accept or greenlights ones it
19-
* rejects.
15+
* on the write path.
16+
*
17+
* ## Why this is deprecated rather than finished
18+
*
19+
* Validation rules are ENFORCEMENT, and enforcement is single-implementation on
20+
* the server by design. `evaluator/fieldRules.ts` draws that line explicitly:
21+
* PRESENTATION predicates (`visibleWhen` / `readonlyWhen` / `requiredWhen`) are
22+
* evaluated client-side by delegating to the canonical `ExpressionEngine`,
23+
* "rather than re-implementing a parallel evaluator" (ADR-0036). This module is
24+
* that parallel evaluator, on the enforcement side of the line.
25+
*
26+
* #3103 converged its semantics onto the server rule-for-rule, with eight
27+
* mutation-tested gates — and it STILL left a known divergence: the server
28+
* carries ADR-0113's legacy-violation exemption (reject only when the merged
29+
* state violates AND this write makes it worse, so a pre-existing violation on
30+
* an old row passes), and this engine does not. Editing an unrelated field on a
31+
* legacy row would be blocked here and accepted there. That is the argument in
32+
* one line: mirroring cross-repo behaviour is structurally unreliable, not
33+
* unreliable-this-time.
34+
*
35+
* ## What to use instead
36+
*
37+
* Let the write fail and render the server's rejection — it is already
38+
* structured (`field` / `code` / `message`, plus a label since objectstack#3957).
39+
* If pre-submit feedback is wanted, the answer is a **validate-only (dry-run)
40+
* write** on the server: identical UX, zero parity risk, and it also covers the
41+
* two rule kinds a client can never check — `unique` (needs the database) and
42+
* `json_schema` (ajv lives server-side).
43+
*
44+
* Nothing here has been removed or changed in behaviour; existing callers keep
45+
* working. `validation-engine-stays-unwired.test.ts` fails if a production
46+
* module starts importing this one, so the decision is a mechanism rather than
47+
* this comment (#3017).
48+
*
49+
* ## When to revisit
50+
*
51+
* Offline writes (`OfflineConfig`). With no server to ask, a client evaluator
52+
* stops being redundant and becomes necessary. The precondition then is
53+
* conformance fixtures shipped by `@objectstack/spec` — golden
54+
* (rule, record, verdict) triples both repos run in CI — because that is the
55+
* only thing that turns cross-repo parity into a mechanism instead of a habit.
56+
* Reverse the decision in #3110 before reversing it here.
57+
*
58+
* ## Semantics (unchanged, for as long as this exists)
2059
*
2160
* The rule types come from `@object-ui/types`, which derives them from
2261
* `@objectstack/spec/data` (objectstack#4115). Semantics mirrored from the
@@ -340,6 +379,13 @@ class SimpleExpressionEvaluator implements ValidationExpressionEvaluator {
340379
/**
341380
* Object-Level Validation Engine — the client pre-check described at the top of
342381
* this module.
382+
*
383+
* @deprecated (#3110) Validation rules are enforcement, and enforcement is
384+
* single-implementation on the server (`objectql`'s rule-validator). Render the
385+
* server's rejection instead; for pre-submit feedback, use a validate-only
386+
* (dry-run) write. Still functional and still correct as of #3103 — but it
387+
* cannot stay in step with the server without a conformance-fixture mechanism
388+
* that does not exist yet. See the module header.
343389
*/
344390
export class ObjectValidationEngine {
345391
private expressionEvaluator: ValidationExpressionEvaluator;
@@ -872,11 +918,15 @@ function matchesNamedFormat(format: FormatValidation['format'], str: string): bo
872918

873919
/**
874920
* Default instance
921+
*
922+
* @deprecated (#3110) See `ObjectValidationEngine`.
875923
*/
876924
export const defaultObjectValidationEngine = new ObjectValidationEngine();
877925

878926
/**
879927
* Convenience function to validate a record
928+
*
929+
* @deprecated (#3110) See `ObjectValidationEngine`.
880930
*/
881931
export async function validateRecord(
882932
rules: ObjectValidationRule[],

0 commit comments

Comments
 (0)