From 245eb94c80edd6433c22ad4a1528c6f3eeb83d74 Mon Sep 17 00:00:00 2001 From: Andrew Paseltiner Date: Wed, 17 Jan 2024 10:39:10 -0500 Subject: [PATCH] Replace single-spec with top-level trigger data --- EVENT.md | 41 +++---- flexible_event_config.md | 4 - index.bs | 88 ++++++++------ ts/src/header-validator/source.test.ts | 139 +++++++++-------------- ts/src/header-validator/validate-json.ts | 125 ++++++++++++-------- 5 files changed, 201 insertions(+), 196 deletions(-) diff --git a/EVENT.md b/EVENT.md index 1f4771c96e..18459f3dd2 100644 --- a/EVENT.md +++ b/EVENT.md @@ -346,24 +346,21 @@ cardinality): ```jsonc { // Specifies how the 64-bit unsigned trigger_data from the trigger is matched - // against the source's trigger_specs trigger_data, which is 32-bit. Defaults - // to "modulus". + // against the source's trigger_data, which is 32-bit. Defaults to "modulus". // // If "exact", the trigger_data must exactly match a value contained in the - // source's trigger_specs; if there is no such match, no event-level + // source's trigger_data; if there is no such match, no event-level // attribution takes place. // - // If "modulus", the set of all trigger_data values across all trigger_specs - // for the source must be a contiguous sequence of integers starting at 0. - // The trigger's trigger_data is taken modulus the cardinality of this - // sequence and then matched against the trigger specs. See below for an - // example. It is an error to use "modulus" if the trigger specs do not - // contain such a sequence. + // If "modulus", the source's trigger_data must form a contiguous sequence of + // integers starting at 0. The trigger's trigger_data is taken modulus the + // cardinality of this sequence and then matched against the trigger data. + // See below for an example. It is an error to use "modulus" if the trigger + // data does not form such a sequence. "trigger_data_matching": , - "trigger_specs": { - "trigger_data": [<32-bit unsigned integer>, ...] - } + // Size must be in the range [0, 32], inclusive. + "trigger_data": [<32-bit unsigned integer>, ...] } ``` @@ -374,12 +371,10 @@ navigation source (by limiting the number of distinct trigger data values) or ```jsonc { ..., - "trigger_specs": { - // The effective cardinality is 5, so the trigger's trigger_data will be - // taken modulus 5. Likewise, noise will be applied using the effective - // cardinality of 5, instead of the default for the source type. - "trigger_data": [0, 1, 2, 3, 4] - } + // The effective cardinality is 5, so the trigger's trigger_data will be taken + // modulus 5. Likewise, noise will be applied using the effective cardinality + // of 5, instead of the default for the source type. + "trigger_data": [0, 1, 2, 3, 4] } ``` @@ -390,12 +385,10 @@ contain the exact value, e.g. ```jsonc { "trigger_data_matching": "exact", - "trigger_specs": { - // If this list does not contain the trigger's trigger_data value, - // attribution will fail and no report will be generated. - // Noise will be applied using an effective cardinality of 2. - "trigger_data": [123, 456] - } + // If this list does not contain the trigger's trigger_data value, attribution + // will fail and no report will be generated. Noise will be applied using an + // effective cardinality of 2. + "trigger_data": [123, 456] } ``` diff --git a/flexible_event_config.md b/flexible_event_config.md index 9c75214dfa..09076900d6 100644 --- a/flexible_event_config.md +++ b/flexible_event_config.md @@ -56,10 +56,6 @@ In addition to the parameters that were added in Phase 1, we will add one additi // triggers within the specified event_report_window. // There will be a limit on the number of specs possible to define for a source. // MAX_UINT32 is 2^32 - 1 (4294967295). - // - // Lists containing a single spec dictionary may be replaced with the - // dictionary itself for brevity: `{"trigger_specs": [{...}]}` is equivalent - // to `{"trigger_specs": {...}}`. "trigger_specs": [{ // This spec will only apply to registrations that set one of the given // trigger data values (integers in the range [0, MAX_UINT32]) in the list. diff --git a/index.bs b/index.bs index 3398051e19..0f971c6c6b 100644 --- a/index.bs +++ b/index.bs @@ -1965,6 +1965,22 @@ To parse summary buckets given a [=map=] |map| and an integer |maxEve 1. Set |prev| to |item|. 1. Return |summaryBuckets|. +To parse trigger data into a trigger spec map given a +|triggerDataList|, a [=trigger spec=] |spec|, and a [=trigger spec map=] +|specs|: + +1. If |triggerDataList| is not a [=list=], [=list/is empty=], or its + [=list/size=] is greater than [=max distinct trigger data per source=], + return false. +1. [=list/iterate|For each=] |triggerData| of |triggerDataList|: + 1. If |triggerData| is not an integer or cannot be represented by an + unsigned 32-bit integer, or |specs|[|triggerData|] [=map/exists=], + return false. + 1. [=map/Set=] |specs|[|triggerData|] to |spec|. + 1. If |specs|'s [=map/size=] is greater than + [=max distinct trigger data per source=], return false. +1. Return true. + To parse trigger specs given a [=map=] |map|, a [=moment=] |sourceTime|, a [=source type=] |sourceType|, a [=duration=] |expiry|, and a [=trigger-data matching mode=] |matchingMode|: @@ -1973,44 +1989,44 @@ To parse trigger specs given a [=map=] |map|, a [=moment=] [=parsing top-level report windows=] with |map|, |sourceTime|, |sourceType|, and |expiry|. 1. If |defaultReportWindows| is an error, return an error. -1. Let |specs| be a new [=map=]. -1. If |map|["`trigger_specs`"] does not [=map/exist=]: - 1. Let |spec| be a new [=trigger spec=] with the following items: - : [=trigger spec/event-level report windows=] - :: |defaultReportWindows| - 1. [=set/iterate|For each=] integer |triggerData| of [=the exclusive range|the range=] 0 to - [=default trigger data cardinality=][|sourceType|], exclusive: - 1. [=map/Set=] |specs|[|triggerData|] to |spec|. - 1. Return |specs|. -1. Let |val| be |map|["`trigger_specs`"]. -1. If [=experimental Flexible Event support=] is false and |val| is not a - [=map=], return an error. -1. If |val| is a [=map=], set |val| to « |val| ». -1. If |val| is not a [=list=] or its - [=list/size=] is greater than [=max distinct trigger data per source=], - return an error. -1. [=list/iterate|For each=] |item| of |val|: - 1. If |item| is not a [=map=], return an error. +1. Let |specs| be a new [=trigger spec map=]. +1. If [=experimental Flexible Event support=] is true and + |map|["`trigger_specs`"] [=map/exists=]: + 1. If |map|["`trigger_data`"] [=map/exists=], return an error. + 1. If |map|["`trigger_specs`"] is not a [=list=] or its + [=list/size=] is greater than [=max distinct trigger data per source=], + return an error. + 1. [=list/iterate|For each=] |item| of |map|["`trigger_specs`"]: + 1. If |item| is not a [=map=], return an error. + 1. Let |spec| be a new [=trigger spec=] with the following items: + : [=trigger spec/event-level report windows=] + :: |defaultReportWindows| + 1. If |item|["`event_report_windows`"] [=map/exists=]: + 1. Let |reportWindows| be the result of + [=parsing report windows=] with |item|["`event_report_windows`"], + |sourceTime|, and |expiry|. + 1. If |reportWindows| is an error, return it. + 1. Set |spec|'s [=trigger spec/event-level report windows=] to + |reportWindows|. + 1. If |item|["`trigger_data`"] does not [=map/exist=], return an error. + 1. If the result of running + [=parse trigger data into a trigger spec map=] with + |item|["`trigger_data`"], |spec|, and |specs| is false, return an + error. +1. Otherwise: 1. Let |spec| be a new [=trigger spec=] with the following items: : [=trigger spec/event-level report windows=] :: |defaultReportWindows| - 1. If [=experimental Flexible Event support=] is true and |item|["`event_report_windows`"] [=map/exists=]: - 1. Let |reportWindows| be the result of - [=parsing report windows=] with |item|["`event_report_windows`"], - |sourceTime|, and |expiry|. - 1. If |reportWindows| is an error, return it. - 1. Set |spec|'s [=trigger spec/event-level report windows=] to - |reportWindows|. - 1. If |item|["`trigger_data`"] does not [=map/exist=], is not a [=list=], - [=list/is empty=], or its [=list/size=] is greater than - [=max distinct trigger data per source=], return an error. - 1. [=list/iterate|For each=] |triggerData| of |item|["`trigger_data`"]: - 1. If |triggerData| is not an integer or cannot be represented by an - unsigned 32-bit integer, or |specs|[|triggerData|] [=map/exists=], - return an error. - 1. [=map/Set=] |specs|[|triggerData|] to |spec|. - 1. If |specs|'s [=map/size=] is greater than - [=max distinct trigger data per source=], return an error. + 1. If |map|["`trigger_data`"] [=map/exists=]: + 1. If the result of running + [=parse trigger data into a trigger spec map=] with + |map|["`trigger_data`"], |spec|, and |specs| is false, return an + error. + 1. Otherwise: + 1. [=set/iterate|For each=] integer |triggerData| of + [=the exclusive range|the range=] 0 to + [=default trigger data cardinality=][|sourceType|], exclusive: + 1. [=map/Set=] |specs|[|triggerData|] to |spec|. 1. If |matchingMode| is "[=trigger-data matching mode/modulus=]": 1. Let |i| be 0. 1. [=map/iterate|For each=] |triggerData| of |specs|'s [=map/get the keys|keys=]: @@ -2019,7 +2035,7 @@ To parse trigger specs given a [=map=] |map|, a [=moment=] 1. Return |specs|. Issue: Invoke [=parse summary buckets=] and [=parse summary window operator=] -from this algorithm when [=experimental Flexible Event support=] is true. +from this algorithm. To parse source-registration JSON given a [=byte sequence=] |json|, a [=suitable origin=] |sourceOrigin|, a [=suitable origin=] |reportingOrigin|, a diff --git a/ts/src/header-validator/source.test.ts b/ts/src/header-validator/source.test.ts index d9696cf69c..beb838518b 100644 --- a/ts/src/header-validator/source.test.ts +++ b/ts/src/header-validator/source.test.ts @@ -1310,52 +1310,13 @@ const testCases: TestCase[] = [ name: 'trigger-specs-wrong-type', json: `{ "destination": "https://a.test", - "trigger_specs": [] - }`, - expectedErrors: [ - { - path: ['trigger_specs'], - msg: 'must be an object', - }, - ], - }, - { - name: 'trigger-specs-unknown-fields', - json: `{ - "destination": "https://a.test", - "trigger_specs": { - "trigger_data": [0], - "event_report_windows": {}, - "summary_buckets": [], - "summary_window_operator": "count" - } - }`, - expectedWarnings: [ - { - path: ['trigger_specs', 'event_report_windows'], - msg: 'unknown field', - }, - { - path: ['trigger_specs', 'summary_buckets'], - msg: 'unknown field', - }, - { - path: ['trigger_specs', 'summary_window_operator'], - msg: 'unknown field', - }, - ], - }, - { - name: 'trigger-specs-wrong-type-ff', - json: `{ - "destination": "https://a.test", - "trigger_specs": 123 + "trigger_specs": {} }`, parseFullFlex: true, expectedErrors: [ { path: ['trigger_specs'], - msg: 'must be an object or a list', + msg: 'must be a list', }, ], }, @@ -1389,15 +1350,46 @@ const testCases: TestCase[] = [ }, ], }, + { + name: 'top-level-trigger-data-and-trigger-specs', + json: `{ + "destination": "https://a.test", + "trigger_data": [], + "trigger_specs": [] + }`, + parseFullFlex: true, + expectedErrors: [ + { + path: [], + msg: 'mutually exclusive fields: trigger_data, trigger_specs', + }, + ], + }, + { + name: 'top-level-trigger-data-and-trigger-specs-ignored', + json: `{ + "destination": "https://a.test", + "max_event_level_reports": 0, + "trigger_data": [], + "trigger_specs": [] + }`, + expectedWarnings: [ + { + path: ['trigger_specs'], + msg: 'unknown field', + }, + ], + }, { name: 'trigger-data-missing', json: `{ "destination": "https://a.test", - "trigger_specs": {} + "trigger_specs": [{}] }`, + parseFullFlex: true, expectedErrors: [ { - path: ['trigger_specs', 'trigger_data'], + path: ['trigger_specs', 0, 'trigger_data'], msg: 'required', }, ], @@ -1406,12 +1398,11 @@ const testCases: TestCase[] = [ name: 'trigger-data-wrong-type', json: `{ "destination": "https://a.test", - "trigger_specs": {"trigger_data": 1} + "trigger_data": 1 }`, - parseFullFlex: true, expectedErrors: [ { - path: ['trigger_specs', 'trigger_data'], + path: ['trigger_data'], msg: 'must be a list', }, ], @@ -1420,11 +1411,11 @@ const testCases: TestCase[] = [ name: 'trigger-data-value-wrong-type', json: `{ "destination": "https://a.test", - "trigger_specs": {"trigger_data": ["1"]} + "trigger_data": ["1"] }`, expectedErrors: [ { - path: ['trigger_specs', 'trigger_data', 0], + path: ['trigger_data', 0], msg: 'must be a number', }, ], @@ -1433,11 +1424,11 @@ const testCases: TestCase[] = [ name: 'trigger-data-value-not-integer', json: `{ "destination": "https://a.test", - "trigger_specs": {"trigger_data": [1.5]} + "trigger_data": [1.5] }`, expectedErrors: [ { - path: ['trigger_specs', 'trigger_data', 0], + path: ['trigger_data', 0], msg: 'must be an integer', }, ], @@ -1446,11 +1437,11 @@ const testCases: TestCase[] = [ name: 'trigger-data-value-negative', json: `{ "destination": "https://a.test", - "trigger_specs": {"trigger_data": [-1]} + "trigger_data": [-1] }`, expectedErrors: [ { - path: ['trigger_specs', 'trigger_data', 0], + path: ['trigger_data', 0], msg: 'must be in the range [0, 4294967295]', }, ], @@ -1459,11 +1450,11 @@ const testCases: TestCase[] = [ name: 'trigger-data-value-exceeds-max', json: `{ "destination": "https://a.test", - "trigger_specs": {"trigger_data": [4294967296]} + "trigger_data": [4294967296] }`, expectedErrors: [ { - path: ['trigger_specs', 'trigger_data', 0], + path: ['trigger_data', 0], msg: 'must be in the range [0, 4294967295]', }, ], @@ -1472,11 +1463,11 @@ const testCases: TestCase[] = [ name: 'trigger-data-duplicated-within', json: `{ "destination": "https://a.test", - "trigger_specs": {"trigger_data": [1, 2, 1]} + "trigger_data": [1, 2, 1] }`, expectedErrors: [ { - path: ['trigger_specs', 'trigger_data', 2], + path: ['trigger_data', 2], msg: 'duplicate value 1', }, ], @@ -1499,31 +1490,15 @@ const testCases: TestCase[] = [ ], }, { - name: 'trigger-data-empty', + name: 'trigger-spec-trigger-data-empty', json: `{ "destination": "https://a.test", - "trigger_specs": {"trigger_data": []} + "trigger_specs": [{"trigger_data": []}] }`, + parseFullFlex: true, expectedErrors: [ { - path: ['trigger_specs', 'trigger_data'], - msg: 'length must be in the range [1, 32]', - }, - ], - }, - { - name: 'trigger-data-too-many-within', - json: JSON.stringify({ - destination: 'https://a.test', - trigger_specs: { - trigger_data: Array(33) - .fill(0) - .map((_, i) => i), - }, - }), - expectedErrors: [ - { - path: ['trigger_specs', 'trigger_data'], + path: ['trigger_specs', 0, 'trigger_data'], msg: 'length must be in the range [1, 32]', }, ], @@ -1792,7 +1767,7 @@ const testCases: TestCase[] = [ json: `{ "destination": "https://a.test", "trigger_data_matching": "modulus", - "trigger_specs": {"trigger_data": [1]} + "trigger_data": [1] }`, expectedErrors: [ { @@ -1824,7 +1799,7 @@ const testCases: TestCase[] = [ json: `{ "destination": "https://a.test", "trigger_data_matching": "modulus", - "trigger_specs": {"trigger_data": [0, 1, 3]} + "trigger_data": [0, 1, 3] }`, expectedErrors: [ { @@ -1851,7 +1826,7 @@ const testCases: TestCase[] = [ json: `{ "destination": "https://a.test", "trigger_data_matching": "modulus", - "trigger_specs": {"trigger_data": [1, 0, 2, 3]} + "trigger_data": [1, 0, 2, 3] }`, }, @@ -1860,9 +1835,8 @@ const testCases: TestCase[] = [ json: `{ "destination": "https://a.test", "max_event_level_reports": 0, - "trigger_specs": [{"trigger_data": [1]}] + "trigger_data": [1] }`, - parseFullFlex: true, expectedWarnings: [ { path: [], @@ -1875,9 +1849,8 @@ const testCases: TestCase[] = [ json: `{ "destination": "https://a.test", "max_event_level_reports": 1, - "trigger_specs": [] + "trigger_data": [] }`, - parseFullFlex: true, expectedWarnings: [ { path: [], diff --git a/ts/src/header-validator/validate-json.ts b/ts/src/header-validator/validate-json.ts index eeeb73c8da..cd1ad1e96c 100644 --- a/ts/src/header-validator/validate-json.ts +++ b/ts/src/header-validator/validate-json.ts @@ -908,9 +908,13 @@ function fullFlexTriggerDatum(ctx: Context, j: Json): Maybe { .filter((n) => isInRange(ctx, n, 0, UINT32_MAX)) } -function triggerDataSet(ctx: Context, j: Json): Maybe> { +function triggerDataSet( + ctx: Context, + j: Json, + allowEmpty: boolean = false +): Maybe> { return set(ctx, j, fullFlexTriggerDatum, { - minLength: 1, + minLength: allowEmpty ? 0 : 1, maxLength: constants.maxTriggerDataPerSource, requireDistinct: true, }) @@ -922,39 +926,37 @@ type TriggerSpecDeps = { maxEventLevelReports: Maybe } +function makeDefaultSummaryBuckets(maxEventLevelReports: number): number[] { + return Array.from({ length: maxEventLevelReports }, (_, i) => i + 1) +} + function triggerSpec( ctx: SourceContext, j: Json, deps: TriggerSpecDeps ): Maybe { - const defaultSummaryBuckets = deps.maxEventLevelReports.map((n) => - Array.from({ length: n }, (_, i) => i + 1) + const defaultSummaryBuckets = deps.maxEventLevelReports.map( + makeDefaultSummaryBuckets ) return struct(ctx, j, { - eventReportWindows: ctx.parseFullFlex - ? field( - 'event_report_windows', - (ctx, j) => eventReportWindows(ctx, j, deps.expiry), - deps.eventReportWindows - ) - : () => deps.eventReportWindows, - - summaryBuckets: ctx.parseFullFlex - ? field( - 'summary_buckets', - (ctx, j) => summaryBuckets(ctx, j, deps.maxEventLevelReports), - defaultSummaryBuckets - ) - : () => defaultSummaryBuckets, - - summaryWindowOperator: ctx.parseFullFlex - ? field( - 'summary_window_operator', - (ctx, j) => enumerated(ctx, j, SummaryWindowOperator), - SummaryWindowOperator.count - ) - : () => some(SummaryWindowOperator.count), + eventReportWindows: field( + 'event_report_windows', + (ctx, j) => eventReportWindows(ctx, j, deps.expiry), + deps.eventReportWindows + ), + + summaryBuckets: field( + 'summary_buckets', + (ctx, j) => summaryBuckets(ctx, j, deps.maxEventLevelReports), + defaultSummaryBuckets + ), + + summaryWindowOperator: field( + 'summary_window_operator', + (ctx, j) => enumerated(ctx, j, SummaryWindowOperator), + SummaryWindowOperator.count + ), triggerData: field('trigger_data', triggerDataSet), }) @@ -965,20 +967,9 @@ function triggerSpecs( j: Json, deps: TriggerSpecDeps ): Maybe { - const singularSpec = (ctx: SourceContext, j: Json) => - triggerSpec(ctx, j, deps).map((spec) => [spec]) - - return ( - ctx.parseFullFlex - ? typeSwitch(ctx, j, { - object: singularSpec, - list: (ctx, j) => - array(ctx, j, (ctx, j) => triggerSpec(ctx, j, deps), { - maxLength: constants.maxTriggerDataPerSource, - }), - }) - : singularSpec(ctx, j) - ).filter((specs) => { + return array(ctx, j, (ctx, j) => triggerSpec(ctx, j, deps), { + maxLength: constants.maxTriggerDataPerSource, + }).filter((specs) => { const triggerData = new Set() const dups = new Set() for (const spec of specs) { @@ -1008,6 +999,33 @@ function triggerSpecs( }) } +function triggerSpecsFromTriggerData( + ctx: Context, + j: Json, + deps: TriggerSpecDeps +): Maybe { + return triggerDataSet(ctx, j, /*allowEmpty=*/ true).map((triggerData) => { + if ( + triggerData.size === 0 || + deps.eventReportWindows.value === undefined || + deps.maxEventLevelReports.value === undefined + ) { + return [] + } + + return [ + { + eventReportWindows: deps.eventReportWindows.value, + summaryBuckets: makeDefaultSummaryBuckets( + deps.maxEventLevelReports.value + ), + summaryWindowOperator: SummaryWindowOperator.count, + triggerData: triggerData, + }, + ] + }) +} + function defaultTriggerSpecs( ctx: SourceContext, eventReportWindows: Maybe, @@ -1138,14 +1156,23 @@ function source(ctx: SourceContext, j: Json): Maybe { maxEventLevelReportsVal ) - const triggerSpecsVal = field( - 'trigger_specs', - (ctx: SourceContext, j) => - triggerSpecs(ctx, j, { - expiry: expiryVal, - eventReportWindows: eventReportWindowsVal, - maxEventLevelReports: maxEventLevelReportsVal, - }), + const triggerSpecsDeps = { + expiry: expiryVal, + eventReportWindows: eventReportWindowsVal, + maxEventLevelReports: maxEventLevelReportsVal, + } + + const triggerSpecsVal = exclusive( + { + trigger_data: (ctx, j) => + triggerSpecsFromTriggerData(ctx, j, triggerSpecsDeps), + ...(ctx.parseFullFlex + ? { + trigger_specs: (ctx: SourceContext, j) => + triggerSpecs(ctx, j, triggerSpecsDeps), + } + : {}), + }, defaultTriggerSpecsVal )(ctx, j)