diff --git a/.changeset/21419-cube-measure-aggregate-field-type-refused.md b/.changeset/21419-cube-measure-aggregate-field-type-refused.md new file mode 100644 index 00000000000..d87f2a2d48c --- /dev/null +++ b/.changeset/21419-cube-measure-aggregate-field-type-refused.md @@ -0,0 +1,17 @@ +--- +"@objectstack/lint": minor +--- + +fix(lint)!: `os validate`, `os build` and `os lint` refuse an `analyticsCubes` measure whose aggregate the aggregate × field-type table refuses for its column, which the analytics door already refuses at query time + +Clause-②: no (narrowing) + + + +**BREAKING**: metadata that passed `os validate`, `os build` and `os lint` can now fail. A cube measure's `type` is its aggregate, and the analytics door judges every cube measure against `AGGREGATE_FIELD_TYPE_COMPATIBILITY` before any SQL is built: a pair the table refuses is answered `400 INVALID_FIELD`. The authoring check judged only a cube's `count_distinct` measures, so a cube `sum` over a `text` column, for one, passed every command and was refused on its first query. The `measure-aggregate-field-type-refused` id (gating, `error`) now judges every cube measure exactly as it judges a dataset measure. It ships as `minor` under the launch-window convention for accept-set narrowings. No export is added or removed. + +**What is refused.** On a cube whose `sql` names an object the stack defines: a `measures` entry of `type` `sum`, `avg`, `min` or `max` whose `sql` column is declared with a type outside that aggregate's row of `AGGREGATE_FIELD_TYPE_COMPATIBILITY` (`@objectstack/spec/data`). The four rows accept the numeric and boolean types, `min` and `max` the temporal types too, and `sum` does not accept `percent`; so a text, option, reference, file or structured-JSON column, among others, is refused under all four, and a `date`, `datetime` or `time` column under `sum` and `avg`. The column is the measure's `sql`: a column of the cube's object, or a relationship path read on the object its last hop reaches (the join the cube declares for that hop, else the lookup field's `reference`). + +**What an author sees now.** The finding names the cube, the measure, the column, the object that declares it and its type, the types the aggregate accepts and the aggregates the column's type accepts, and says the analytics door refuses the pair with `400 INVALID_FIELD`. It is located at `analyticsCubes[N].measures.KEY.type`, where `KEY` is the measure's key. A quantity that must be added up, averaged or ordered has to be stored as a numeric or temporal field and aggregated as one; `count` accepts every column. + +**Unchanged.** Every dataset finding and every cube dimension finding; a cube `count` measure over any column; a cube `count_distinct` measure, judged as before; a measure of an expression type (`number`, `string`, `boolean`); the row wildcard `'*'`; a measure whose column does not resolve or declares no type; a cube whose `sql` names no object this stack defines. The runtime metadata write door: no authoring rule is dispatched for an `analytics_cube` save. diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index 7cf19620713..f6b4ccfb7f2 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -236,13 +236,18 @@ Both checks also judge the members of an analytics cube (`analyticsCubes`), because an authored cube is queried through the same analytics door. A cube dimension whose `sql` column is declared with a structured-JSON type or as a multi-value field is refused (`dimension-json-stored-field-refused`) at -`analyticsCubes[N].dimensions..sql`, and so is a `count_distinct` measure -over such a column (`measure-aggregate-field-type-refused`, at -`analyticsCubes[N].measures..type`). The column is read where the door +`analyticsCubes[N].dimensions..sql`. A cube measure is judged exactly as a +dataset measure is, its `type` being the aggregate: a pair outside the table is +refused (`measure-aggregate-field-type-refused`, at +`analyticsCubes[N].measures..type`). That covers `sum`, `avg`, `min` or +`max` over a column whose type their row does not accept (a `sum` over a `text` +column, say), and `count_distinct` over a JSON-stored column; the analytics +door refuses each of them with `400 INVALID_FIELD` when a query names the +measure. `count` is accepted over every type. The column is read where the door reads it: on the object the cube's `sql` names, or, for a relationship path, on the object the last hop reaches — the join the cube declares for that hop, -else the lookup's `reference`. A cube's other measure types and the row -wildcard `'*'` are not judged by this check. +else the lookup's `reference`. A measure of an expression type (`number`, +`string`, `boolean`) and the row wildcard `'*'` are not judged by this check. ### 7. Navigation exposing objects nobody can read diff --git a/packages/lint/src/validate-dataset-measure-aggregates.test.ts b/packages/lint/src/validate-dataset-measure-aggregates.test.ts index fa4dc6c5da2..1a8e0417dfb 100644 --- a/packages/lint/src/validate-dataset-measure-aggregates.test.ts +++ b/packages/lint/src/validate-dataset-measure-aggregates.test.ts @@ -15,6 +15,7 @@ import { describe, expect, it } from 'vitest'; import { AGGREGATE_FIELD_TYPE_COMPATIBILITY, + AggregationMetricType, BOOLEAN_VALUE_TYPES, FieldType, MULTI_CAPABLE_TYPES, @@ -656,12 +657,14 @@ const cubeStack = ( { name: 'fx_account', sharingModel: 'private', - fields: { name: { type: 'text' }, hq: { type: 'json' }, region: { type: 'text' } }, + // [#21419] `revenue` is `number` here and `text` on `fx_branch`, so the + // two hop tiers give a cube `sum` opposite verdicts. + fields: { name: { type: 'text' }, hq: { type: 'json' }, region: { type: 'text' }, revenue: { type: 'number' } }, }, { name: 'fx_branch', sharingModel: 'private', - fields: { name: { type: 'text' }, hq: { type: 'text' }, region: { type: 'json' } }, + fields: { name: { type: 'text' }, hq: { type: 'text' }, region: { type: 'json' }, revenue: { type: 'text' } }, }, ], analyticsCubes: [ @@ -741,9 +744,16 @@ describe('the cube leg — an analyticsCubes member the analytics door refuses i expect(validateDatasetMeasureAggregates(stack)).toEqual([]); }); - it('judges only count_distinct among the cube measures: count reads no value, and the other rows are not this leg\'s', () => { - for (const type of ['count', 'sum', 'avg', 'min', 'max']) { - expect(findings(cubeStack({ measures: { m: measure(type, 'meta') } })), type).toEqual([]); + // [#21419] Every row of the table is judged on a cube measure (the sweep is + // in the next block): over the same json column `count` reads no value and + // is accepted, and every other row refuses it. + it('judges every row of the table on a cube measure: count reads no value, and the other rows refuse a json column', () => { + expect(findings(cubeStack({ measures: { m: measure('count', 'meta') } }))).toEqual([]); + for (const type of ['sum', 'avg', 'min', 'max']) { + expect( + findings(cubeStack({ measures: { m: measure(type, 'meta') } })).map((f) => f.path), + type, + ).toEqual(['analyticsCubes[0].measures.m.type']); } }); @@ -859,3 +869,187 @@ describe('the cube leg — an analyticsCubes member the analytics door refuses i ).toEqual([]); }); }); + +// ─────────────────────────────────────────────────────────────────────────── +// [#21419] The cube leg judges EVERY cube measure through `acceptsDeclaration` +// — the dataset measure's own verdict — on the column the door reads. The +// doors are `service-analytics`' `cube-measure-field-type-door.ts` for the +// `count` / `sum` / `avg` / `min` / `max` rows (the declared TYPE against the +// table; the `multiple` flag is not read) and `structured-json-dimension-door.ts` +// for `count_distinct` (the row AND `isMultiValueField`). Until this leg a cube +// `sum` over a `text` column passed `os validate` (measured: +// `oos-cube-sum-text`, exit 0). +// ─────────────────────────────────────────────────────────────────────────── + +/** + * The cube doors' verdict on a measure of `type` over a column declared as + * `shape`, as the two door modules state their rules — read off the spec's + * table and predicates, never a retyped list: + * + * - a `type` that is no row of the table (the expression metric types) is + * judged by neither door; + * - `count_distinct` — `structured-json-dimension-door.ts`: refused when the + * row refuses the type OR the declaration is multi-value; + * - every other row — `cube-measure-field-type-door.ts`: refused when the row + * refuses the declared type, whatever the `multiple` flag says. + */ +const cubeDoorRefuses = (type: string, shape: { type: string; multiple: boolean }): boolean => { + if (!Object.prototype.hasOwnProperty.call(AGGREGATE_FIELD_TYPE_COMPATIBILITY, type)) return false; + if (type === 'count_distinct') { + return !isAggregateCompatibleWithFieldType(type, shape.type) || isMultiValueField(shape); + } + return !isAggregateCompatibleWithFieldType(type, shape.type); +}; + +describe('the cube leg — every cube measure is judged by the aggregate × field-type table, as the cube door judges it', () => { + // ⭐ The card's measured instance and its control, one pair. + it('refuses oos-cube-sum-text — a cube sum over a text column — and accepts sum over a number one', () => { + const found = validateDatasetMeasureAggregates(cubeStack({ measures: { sum_name: measure('sum', 'name') } })); + expect(found).toHaveLength(1); + const issue = found[0]; + expect(issue.severity).toBe('error'); + expect(issue.rule).toBe(RULE); + expect(issue.path).toBe('analyticsCubes[0].measures.sum_name.type'); + expect(issue.where).toBe('cube "fx_cube" › measure "sum_name"'); + expect(issue.message).toContain('aggregate "sum" to field "name"'); + expect(issue.message).toContain('object "fx_ledger" declares as `text`'); + expect(issue.message).toContain(`"sum" accepts: ${AGGREGATE_FIELD_TYPE_COMPATIBILITY.sum.join(', ')}.`); + // The door that refuses it later is the cube's, never the dataset compile leg. + expect(issue.hint).toContain('on the cube with `400 INVALID_FIELD`'); + expect(issue.hint).not.toContain('DATASET_INVALID'); + + expect( + validateDatasetMeasureAggregates( + cubeStack({ measures: { sum_amount: measure('sum', 'amount') } }, {}, { amount: { type: 'number' } }), + ), + ).toEqual([]); + }); + + it('refuses avg / min / max over that text column, and sum / avg over a datetime one; accepts min / max over the datetime', () => { + const datetime = { opened_at: { type: 'datetime' } }; + for (const [type, column] of [['avg', 'name'], ['min', 'name'], ['max', 'name'], ['sum', 'opened_at'], ['avg', 'opened_at']]) { + const [issue] = findings(cubeStack({ measures: { m: measure(type, column) } }, {}, datetime)); + expect(issue?.path, `${type}(${column})`).toBe('analyticsCubes[0].measures.m.type'); + expect(issue?.message, `${type}(${column})`).toContain(`aggregate "${type}" to field "${column}"`); + } + for (const type of ['min', 'max', 'count', 'count_distinct']) { + expect(findings(cubeStack({ measures: { m: measure(type, 'opened_at') } }, {}, datetime)), type).toEqual([]); + } + }); + + // ⭐ The enumeration pin: the whole surface, against the doors' rules. + it('agrees with the cube doors on every cube measure type × every declared FieldType, flagged and not', () => { + const rows = AGGREGATES; + const metricTypes: readonly string[] = AggregationMetricType.options; + // Every row of the table is a `type` a cube measure can write, so the sweep + // below reaches each row through a real cube measure, not a fiction. + for (const row of rows) expect(metricTypes, `row ${row}`).toContain(row); + // The population is every cube measure `type` AND every row: a row or a + // metric type added later is swept here the day it lands. + const types = [...new Set([...metricTypes, ...rows])]; + + const refusedBy = new Map(); + const acceptedBy = new Map(); + for (const type of types) { + for (const fieldType of FieldType.options) { + for (const multiple of [false, true]) { + const def = multiple ? { type: fieldType, multiple: true } : { type: fieldType }; + const stack = cubeStack({ measures: { probe_measure: measure(type, 'probe') } }, {}, { probe: def }); + const fires = findings(stack).length > 0; + expect(fires, `cube ${type}(${fieldType}${multiple ? ', multiple' : ''})`).toBe( + cubeDoorRefuses(type, { type: fieldType, multiple }), + ); + const tally = fires ? refusedBy : acceptedBy; + tally.set(type, (tally.get(type) ?? 0) + 1); + } + } + } + + // Floors per type, so no row can agree vacuously: `count` refuses nothing, + // every other row both refuses and accepts, and a type outside the table + // is never judged. + const perType = FieldType.options.length * 2; + for (const type of types) { + const refused = refusedBy.get(type) ?? 0; + const accepted = acceptedBy.get(type) ?? 0; + expect(refused + accepted, type).toBe(perType); + if (!rows.includes(type) || type === 'count') { + expect(refused, `${type} refuses nothing`).toBe(0); + } else { + expect(refused, `${type} refuses`).toBeGreaterThan(10); + expect(accepted, `${type} accepts`).toBeGreaterThan(5); + } + } + // The expression metric types are in the population and outside the table. + expect(types.filter((t) => !rows.includes(t)).sort()).toEqual(['boolean', 'number', 'string']); + }); + + it('reads a relationship path on the object the last hop reaches — the lookup\'s reference, or the join the cube declares for it', () => { + // No declared join: the lookup's `reference`, `fx_account`, where `name` is text and `revenue` a number. + const [byReference] = findings(cubeStack({ measures: { s: measure('sum', 'account.name') } })); + expect(byReference?.path).toBe('analyticsCubes[0].measures.s.type'); + expect(byReference?.message).toContain('object "fx_account" (reached through this cube\'s join chain)'); + expect(findings(cubeStack({ measures: { s: measure('sum', 'account.revenue') } }))).toEqual([]); + + // A declared join wins over the reference, as at the door: keyed `account`, + // reaching `fx_branch`, where `revenue` is text. + const joined = { joins: { account: { name: 'fx_branch' } } }; + const [byJoin] = findings(cubeStack({ measures: { s: measure('sum', 'account.revenue') } }, joined)); + expect(byJoin?.message).toContain('object "fx_branch"'); + expect(byJoin?.message).toContain('declares as `text`'); + }); + + it('never hands the predicate a guess: no type, a type outside the table, the row wildcard, an unresolved column', () => { + const silent = (measures: Record, ledger: Record = {}) => + expect(validateDatasetMeasureAggregates(cubeStack({ measures }, {}, ledger))).toEqual([]); + // The expression metric types are outside the table's vocabulary (skip 5). + silent({ n: measure('number', 'name'), s: measure('string', 'name'), b: measure('boolean', 'name') }); + // A prototype key is not a row. + silent({ p: measure('toString', 'name') }); + // No `type`, or a non-string one — the schema's to refuse. + silent({ x: { label: 'X', sql: 'name' }, y: { label: 'Y', type: ['sum'], sql: 'name' } }); + // The row wildcard names no column: whether `'*'` belongs on a non-count + // measure is a question of its own, not this leg's. + silent({ s: measure('sum', '*'), a: measure('avg', '*'), lo: measure('min', '*'), hi: measure('max', '*') }); + // A column that does not resolve, a hop the graph cannot follow, an untyped column. + silent({ s: measure('sum', 'nope'), t: measure('sum', 'ghost.name'), u: measure('sum', 'untyped') }, { + untyped: { label: 'U' }, + }); + // A cube whose `sql` names no object this stack defines. + expect( + validateDatasetMeasureAggregates(cubeStack({ measures: { s: measure('sum', 'name') } }, { sql: 'not_here' })), + ).toEqual([]); + }); + + it('reports each refused measure once, in the cube\'s key order, after its dimensions', () => { + const stack = cubeStack({ + dimensions: { meta: dim('meta') }, + measures: { + sum_name: measure('sum', 'name'), + count_name: measure('count', 'name'), + distinct_meta: measure('count_distinct', 'meta'), + max_stage: measure('max', 'stage'), + }, + }); + expect(validateDatasetMeasureAggregates(stack).map((f) => [f.rule, f.path])).toEqual([ + [DIMENSION_RULE, 'analyticsCubes[0].dimensions.meta.sql'], + [RULE, 'analyticsCubes[0].measures.sum_name.type'], + [RULE, 'analyticsCubes[0].measures.distinct_meta.type'], + [RULE, 'analyticsCubes[0].measures.max_stage.type'], + ]); + }); + + it('fires through runAuthoringRules on all three commands, and only on the refused measure', () => { + const stack = cubeStack({ measures: { sum_name: measure('sum', 'name') } }); + for (const command of ['validate', 'build', 'lint'] as const) { + const found = runAuthoringRules(command, { normalized: stack, parsed: stack }).filter((f) => f.rule === RULE); + expect(found.map((f) => [f.path, f.severity]), command).toEqual([ + ['analyticsCubes[0].measures.sum_name.type', 'error'], + ]); + } + const control = cubeStack({ measures: { count_name: measure('count', 'name') } }); + expect( + runAuthoringRules('lint', { normalized: control, parsed: control }).filter((f) => f.rule === RULE), + ).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-dataset-measure-aggregates.ts b/packages/lint/src/validate-dataset-measure-aggregates.ts index 4a7f91da8f7..0d56d082134 100644 --- a/packages/lint/src/validate-dataset-measure-aggregates.ts +++ b/packages/lint/src/validate-dataset-measure-aggregates.ts @@ -137,23 +137,31 @@ * * ## [#21082] The cube leg — `analyticsCubes` members, under the same two ids * - * An authored cube (`defineStack({ analyticsCubes })`) reaches the SAME door - * the dataset compiles to: `structured-json-dimension-door.ts` judges every - * cube's grouped members and `count_distinct` measures, whether the cube was - * authored or compiled from a dataset. Until this leg no rule in this package - * walked `analyticsCubes`, so a cube dimension over a `json` field passed - * `os validate` and met its first refusal at query time. The leg refuses the - * three shapes that door refuses, with the verdicts this file already holds — - * ⛔ no second account of either: + * An authored cube (`defineStack({ analyticsCubes })`) reaches the SAME doors + * the dataset compiles to, whether the cube was authored or compiled from a + * dataset: `structured-json-dimension-door.ts` judges every cube's grouped + * members and `count_distinct` measures, and [#21419] + * `cube-measure-field-type-door.ts` (#21044) judges every other measure whose + * `type` is a row of the table. Until this leg no rule in this package walked + * `analyticsCubes`, so a cube dimension over a `json` field, or a `sum` over a + * `text` one, passed `os validate` and met its first refusal at query time. + * The leg refuses what those doors refuse, with the verdicts this file already + * holds — ⛔ no second account of either: * * - a dimension whose column is structured-JSON or multi-value — * {@link groupKeyClassOf}, under `dimension-json-stored-field-refused`; - * - a `count_distinct` measure whose column is JSON-stored — - * {@link acceptsDeclaration}, under `measure-aggregate-field-type-refused`. - * - * The door's other cube judgment (#21044, `cube-measure-field-type-door.ts`: - * `sum` / `avg` / `min` / `max` against the table) is not this leg's; a cube - * measure of any other `type` is left unjudged here. + * - [#21419] a measure whose `type` is a row of the table and whose column's + * declaration that row does not accept — {@link acceptsDeclaration}, the + * dataset measure's own verdict, under `measure-aggregate-field-type-refused`. + * That is every row, as on a dataset: `count_distinct` over a JSON-stored + * column (the `structured-json-dimension-door.ts` half), `sum` / `avg` / + * `min` / `max` over a type its row refuses (the + * `cube-measure-field-type-door.ts` half, which reads the TYPE alone — the + * `multiple` flag moves only `count_distinct`, see above), and never + * `count`, which reads no value and accepts every type. A `type` outside + * the table's vocabulary — the expression metric types `number` / + * `string` / `boolean` — is skip 5, as on a dataset, and neither door + * judges it. * * How a cube names its column is read the way the door reads it * (`analytics-service.ts`, `hop-object.ts`): @@ -174,7 +182,8 @@ * skipped, on a dimension and a measure alike (whether `'*'` belongs on * either is a question of its own, not this leg's). * - * The same skips 1, 3 and 4 hold, plus a member that writes no `sql`. + * The same skips 1, 3 and 4 hold, plus a member that writes no `sql`; and on + * a measure, skip 5 and a measure that writes no `type`. * * Both legs ride the one registry entry: gating, on all three commands. At the * runtime write door that entry is dispatched for a `dataset` write, whose @@ -532,11 +541,13 @@ function cubeMemberFindings(cube: AnyRec, cubePath: string, graph: ObjectGraph): } for (const { rec: measure, path } of collectionEntries(cube.measures, `${cubePath}.measures`)) { - // Only `count_distinct` is this leg's: it is the cube measure the door's - // JSON-stored judgment covers. Every other `type` is skipped, judged or not - // elsewhere — see the module note. + // [#21419] Every row of the table is this leg's, as on a dataset: the + // `type` IS the aggregate, judged by the one verdict — see the module note. + // A measure that writes no `type` has nothing to judge; a non-string there + // is the schema's refusal to give, not this rule's. const aggregate = strName(measure.type); - if (aggregate !== 'count_distinct') continue; + if (!aggregate) continue; + // ── Skip 5: the `type` is outside the table's vocabulary (`number` / `string` / `boolean`) ── const accepted = ACCEPTED_TYPES_BY_AGGREGATE.get(aggregate); if (!accepted) continue; const sql = strName(measure.sql); @@ -570,8 +581,9 @@ function cubeMemberFindings(cube: AnyRec, cubePath: string, graph: ObjectGraph): /** * Refuse every dataset measure whose `aggregate` the field's declaration * cannot carry, and every dataset dimension whose field is JSON-stored; then - * [#21082] every cube dimension whose `sql` column is JSON-stored, and every - * cube `count_distinct` measure whose column is. Returns findings (empty = + * [#21082] every cube dimension whose `sql` column is JSON-stored, and + * [#21419] every cube measure whose `type` the column's declaration cannot + * carry. Returns findings (empty = * clean): each dataset's dimensions before its measures, then each cube's. * Pure `(stack) => Finding[]` (ADR-0019): no I/O, and safe on both the * schema-parsed stack and the raw config the `os lint` path carries.