From 1149bd525e65835fda4e43dc0c46ec5d0a81942d Mon Sep 17 00:00:00 2001 From: Barret Rennie Date: Mon, 13 Feb 2023 18:42:36 +0000 Subject: [PATCH] Bug 1813597 - Support reach experiments in the FxMS schemas r=omc-reviewers,emcminn Differential Revision: https://phabricator.services.mozilla.com/D168322 --- .gitignore | 6 + .hgignore | 6 + ...kgroundTaskMessagingExperiment.schema.json | 62 +-- .../asrouter/schemas/FxMSCommon.schema.json | 10 +- .../schemas/MessagingExperiment.schema.json | 354 +++++++++--------- .../corpus/ReachExperiments.messages.json | 17 + .../asrouter/schemas/make-schemas.py | 88 ++--- .../test/xpcshell/test_reach_experiments.js | 96 +++++ .../newtab/test/xpcshell/xpcshell.ini | 1 + 9 files changed, 376 insertions(+), 264 deletions(-) create mode 100644 browser/components/newtab/content-src/asrouter/schemas/corpus/ReachExperiments.messages.json create mode 100644 browser/components/newtab/test/xpcshell/test_reach_experiments.js diff --git a/.gitignore b/.gitignore index bf75a460a4a9..0b03d95f2fc0 100644 --- a/.gitignore +++ b/.gitignore @@ -58,6 +58,12 @@ security/manager/.nss.checkout # Ignore newtab component build assets browser/components/newtab/logs/ +# Ignore ASRouter generated test files +browser/components/newtab/content-src/asrouter/schemas/corpus/CFRMessageProvider.messages.json +browser/components/newtab/content-src/asrouter/schemas/corpus/OnboardingMessageProvider.messages.json +browser/components/newtab/content-src/asrouter/schemas/corpus/PanelTestProvider.messages.json +browser/components/newtab/content-src/asrouter/schemas/corpus/PanelTestProvider_toast_notification.messages.json + # Ignore Pocket component build and dev assets browser/components/pocket/content/panels/css/main.compiled.css.map diff --git a/.hgignore b/.hgignore index 005a541b6dc0..ea0ba48f3bc0 100644 --- a/.hgignore +++ b/.hgignore @@ -56,6 +56,12 @@ compile_commands\.json # Ignore newtab component build assets ^browser/components/newtab/logs/ +# Ignore ASRouter generated test files +^browser/components/newtab/content-src/asrouter/schemas/corpus/CFRMessageProvider.messages.json +^browser/components/newtab/content-src/asrouter/schemas/corpus/OnboardingMessageProvider.messages.json +^browser/components/newtab/content-src/asrouter/schemas/corpus/PanelTestProvider.messages.json +^browser/components/newtab/content-src/asrouter/schemas/corpus/PanelTestProvider_toast_notification.messages.json + # Ignore Pocket component build and dev assets browser/components/pocket/content/panels/css/main.compiled.css.map diff --git a/browser/components/newtab/content-src/asrouter/schemas/BackgroundTaskMessagingExperiment.schema.json b/browser/components/newtab/content-src/asrouter/schemas/BackgroundTaskMessagingExperiment.schema.json index 9b5e2991d2e6..c1a10afe782a 100644 --- a/browser/components/newtab/content-src/asrouter/schemas/BackgroundTaskMessagingExperiment.schema.json +++ b/browser/components/newtab/content-src/asrouter/schemas/BackgroundTaskMessagingExperiment.schema.json @@ -3,37 +3,28 @@ "$id": "resource://activity-stream/schemas/BackgroundTaskMessagingExperiment.schema.json", "title": "Messaging Experiment", "description": "A Firefox Messaging System message.", - "oneOf": [ + "allOf": [ { - "description": "An empty FxMS message.", - "type": "object", - "additionalProperties": false + "$ref": "resource://activity-stream/schemas/BackgroundTaskMessagingExperiment.schema.json#/$defs/Message" }, { - "allOf": [ - { - "$ref": "resource://activity-stream/schemas/BackgroundTaskMessagingExperiment.schema.json#/$defs/Message" - }, - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": [ - "toast_notification" - ] - } - }, - "required": [ - "template" + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": [ + "toast_notification" ] - }, - "then": { - "$ref": "resource://activity-stream/schemas/BackgroundTaskMessagingExperiment.schema.json#/$defs/ToastNotification" } - } - ] + }, + "required": [ + "template" + ] + }, + "then": { + "$ref": "resource://activity-stream/schemas/BackgroundTaskMessagingExperiment.schema.json#/$defs/ToastNotification" + } } ], "$defs": { @@ -240,11 +231,20 @@ } }, "additionalProperties": true, - "required": [ - "id", - "content", - "template" - ] + "dependentRequired": { + "id": [ + "content", + "template" + ], + "content": [ + "id", + "template" + ], + "template": [ + "id", + "content" + ] + } }, "localizedText": { "type": "object", diff --git a/browser/components/newtab/content-src/asrouter/schemas/FxMSCommon.schema.json b/browser/components/newtab/content-src/asrouter/schemas/FxMSCommon.schema.json index 0351f0bd9845..11d9aef09aac 100644 --- a/browser/components/newtab/content-src/asrouter/schemas/FxMSCommon.schema.json +++ b/browser/components/newtab/content-src/asrouter/schemas/FxMSCommon.schema.json @@ -102,11 +102,11 @@ } }, "additionalProperties": true, - "required": [ - "id", - "content", - "template" - ] + "dependentRequired": { + "id": ["content", "template"], + "content": ["id", "template"], + "template": ["id", "content"] + } }, "localizedText": { "type": "object", diff --git a/browser/components/newtab/content-src/asrouter/schemas/MessagingExperiment.schema.json b/browser/components/newtab/content-src/asrouter/schemas/MessagingExperiment.schema.json index b73be07e422d..92a2d40d65e1 100644 --- a/browser/components/newtab/content-src/asrouter/schemas/MessagingExperiment.schema.json +++ b/browser/components/newtab/content-src/asrouter/schemas/MessagingExperiment.schema.json @@ -3,210 +3,201 @@ "$id": "resource://activity-stream/schemas/MessagingExperiment.schema.json", "title": "Messaging Experiment", "description": "A Firefox Messaging System message.", - "oneOf": [ + "allOf": [ { - "description": "An empty FxMS message.", - "type": "object", - "additionalProperties": false + "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/Message" }, { - "allOf": [ - { - "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/Message" - }, - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": [ - "cfr_urlbar_chiclet" - ] - } - }, - "required": [ - "template" + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": [ + "cfr_urlbar_chiclet" ] - }, - "then": { - "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/CFRUrlbarChiclet" } }, - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": [ - "cfr_doorhanger", - "milestone_message" - ] - } - }, - "required": [ - "template" + "required": [ + "template" + ] + }, + "then": { + "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/CFRUrlbarChiclet" + } + }, + { + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": [ + "cfr_doorhanger", + "milestone_message" ] - }, - "then": { - "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/ExtensionDoorhanger" } }, - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": [ - "infobar" - ] - } - }, - "required": [ - "template" + "required": [ + "template" + ] + }, + "then": { + "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/ExtensionDoorhanger" + } + }, + { + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": [ + "infobar" ] - }, - "then": { - "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/InfoBar" } }, - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": [ - "pb_newtab" - ] - } - }, - "required": [ - "template" + "required": [ + "template" + ] + }, + "then": { + "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/InfoBar" + } + }, + { + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": [ + "pb_newtab" ] - }, - "then": { - "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/NewtabPromoMessage" } }, - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": [ - "protections_panel" - ] - } - }, - "required": [ - "template" + "required": [ + "template" + ] + }, + "then": { + "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/NewtabPromoMessage" + } + }, + { + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": [ + "protections_panel" ] - }, - "then": { - "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/ProtectionsPanelMessage" } }, - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": [ - "spotlight", - "feature_callout" - ] - } - }, - "required": [ - "template" + "required": [ + "template" + ] + }, + "then": { + "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/ProtectionsPanelMessage" + } + }, + { + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": [ + "spotlight", + "feature_callout" ] - }, - "then": { - "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/Spotlight" } }, - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": [ - "toast_notification" - ] - } - }, - "required": [ - "template" + "required": [ + "template" + ] + }, + "then": { + "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/Spotlight" + } + }, + { + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": [ + "toast_notification" ] - }, - "then": { - "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/ToastNotification" } }, - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": [ - "toolbar_badge" - ] - } - }, - "required": [ - "template" + "required": [ + "template" + ] + }, + "then": { + "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/ToastNotification" + } + }, + { + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": [ + "toolbar_badge" ] - }, - "then": { - "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/ToolbarBadgeMessage" } }, - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": [ - "update_action" - ] - } - }, - "required": [ - "template" + "required": [ + "template" + ] + }, + "then": { + "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/ToolbarBadgeMessage" + } + }, + { + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": [ + "update_action" ] - }, - "then": { - "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/UpdateAction" } }, - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": [ - "whatsnew_panel_message" - ] - } - }, - "required": [ - "template" + "required": [ + "template" + ] + }, + "then": { + "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/UpdateAction" + } + }, + { + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": [ + "whatsnew_panel_message" ] - }, - "then": { - "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/WhatsNewMessage" } - } - ] + }, + "required": [ + "template" + ] + }, + "then": { + "$ref": "resource://activity-stream/schemas/MessagingExperiment.schema.json#/$defs/WhatsNewMessage" + } } ], "$defs": { @@ -1567,11 +1558,20 @@ } }, "additionalProperties": true, - "required": [ - "id", - "content", - "template" - ] + "dependentRequired": { + "id": [ + "content", + "template" + ], + "content": [ + "id", + "template" + ], + "template": [ + "id", + "content" + ] + } }, "localizedText": { "type": "object", diff --git a/browser/components/newtab/content-src/asrouter/schemas/corpus/ReachExperiments.messages.json b/browser/components/newtab/content-src/asrouter/schemas/corpus/ReachExperiments.messages.json new file mode 100644 index 000000000000..b9feae455519 --- /dev/null +++ b/browser/components/newtab/content-src/asrouter/schemas/corpus/ReachExperiments.messages.json @@ -0,0 +1,17 @@ +[ + { + "trigger": { + "id": "defaultBrowserCheck" + }, + "targeting": "source == 'startup' && !isMajorUpgrade && !activeNotifications && totalBookmarksCount == 5" + }, + { + "groups": [ + "eco" + ], + "trigger": { + "id": "defaultBrowserCheck" + }, + "targeting": "source == 'startup' && !isMajorUpgrade && !activeNotifications && totalBookmarksCount == 5" + } +] diff --git a/browser/components/newtab/content-src/asrouter/schemas/make-schemas.py b/browser/components/newtab/content-src/asrouter/schemas/make-schemas.py index 91e6c2a9dbe7..764371245e00 100755 --- a/browser/components/newtab/content-src/asrouter/schemas/make-schemas.py +++ b/browser/components/newtab/content-src/asrouter/schemas/make-schemas.py @@ -91,8 +91,9 @@ SCHEMAS = [ ), }, bundle_common=True, - # These are generated via extract-test-corpus.js test_corpus={ + "ReachExperiments": Path("corpus", "ReachExperiments.messages.json"), + # These are generated via extract-test-corpus.js "CFRMessageProvider": Path("corpus", "CFRMessageProvider.messages.json"), "OnboardingMessageProvider": Path( "corpus", "OnboardingMessageProvider.messages.json" @@ -316,57 +317,42 @@ def bundle_schema(schema_def: SchemaDefinition): "$id": schema_def.schema_id, "title": "Messaging Experiment", "description": "A Firefox Messaging System message.", - # A message must be one of - # - an empty message (i.e., a completely empty object), which is the - # equivalent of an experiment branch not providing a message; or - # - An object that contains a template field - "oneOf": [ - { - "description": "An empty FxMS message.", - "type": "object", - "additionalProperties": False, - }, - { - "allOf": [ - # Ensure each message has all the fields defined in the base - # Message type. - # - # This is slightly redundant because each message should - # already inherit from this message type, but it is easier - # to add this requirement here than to verify that each - # message's schema is properly inheriting. - {"$ref": f"{schema_def.schema_id}#/$defs/Message"}, - # For each message type, create a subschema that says if the - # template field matches a value for a message type defined - # in MESSAGE_TYPES, then the message must also match the - # schema for that message type. - # - # This is done using `allOf: [{ if, then }]` instead of `oneOf: []` - # because it provides better error messages. Using `if-then` - # will only show validation errors for the sub-schema that - # matches template, whereas using `oneOf` will show - # validation errors for *all* sub-schemas, which makes - # debugging messages much harder. - *( - { - "if": { - "type": "object", - "properties": { - "template": { - "type": "string", - "enum": templates[message_type], - }, - }, - "required": ["template"], + "allOf": [ + # Ensure each message has all the fields defined in the base + # Message type. + # + # This is slightly redundant because each message should + # already inherit from this message type, but it is easier + # to add this requirement here than to verify that each + # message's schema is properly inheriting. + {"$ref": f"{schema_def.schema_id}#/$defs/Message"}, + # For each message type, create a subschema that says if the + # template field matches a value for a message type defined + # in MESSAGE_TYPES, then the message must also match the + # schema for that message type. + # + # This is done using `allOf: [{ if, then }]` instead of `oneOf: []` + # because it provides better error messages. Using `if-then` + # will only show validation errors for the sub-schema that + # matches template, whereas using `oneOf` will show + # validation errors for *all* sub-schemas, which makes + # debugging messages much harder. + *( + { + "if": { + "type": "object", + "properties": { + "template": { + "type": "string", + "enum": templates[message_type], }, - "then": { - "$ref": f"{schema_def.schema_id}#/$defs/{message_type}" - }, - } - for message_type in schema_def.message_types - ), - ], - }, + }, + "required": ["template"], + }, + "then": {"$ref": f"{schema_def.schema_id}#/$defs/{message_type}"}, + } + for message_type in schema_def.message_types + ), ], "$defs": defs, } diff --git a/browser/components/newtab/test/xpcshell/test_reach_experiments.js b/browser/components/newtab/test/xpcshell/test_reach_experiments.js new file mode 100644 index 000000000000..b5e055eab645 --- /dev/null +++ b/browser/components/newtab/test/xpcshell/test_reach_experiments.js @@ -0,0 +1,96 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +const { ObjectUtils } = ChromeUtils.import( + "resource://gre/modules/ObjectUtils.jsm" +); + +const MESSAGES = [ + { + trigger: { id: "defaultBrowserCheck" }, + targeting: + "source == 'startup' && !isMajorUpgrade && !activeNotifications && totalBookmarksCount == 5", + }, + { + groups: ["eco"], + trigger: { + id: "defaultBrowserCheck", + }, + targeting: + "source == 'startup' && !isMajorUpgrade && !activeNotifications && totalBookmarksCount == 5", + }, +]; + +let EXPERIMENT_VALIDATOR; + +add_setup(async function setup() { + EXPERIMENT_VALIDATOR = await schemaValidatorFor( + "resource://activity-stream/schemas/MessagingExperiment.schema.json" + ); +}); + +add_task(function test_reach_experiments_validation() { + for (const [index, message] of MESSAGES.entries()) { + assertValidates( + EXPERIMENT_VALIDATOR, + message, + `Message ${index} validates as a MessagingExperiment` + ); + } +}); + +function depError(has, missing) { + return { + instanceLocation: "#", + keyword: "dependentRequired", + keywordLocation: "#/oneOf/1/allOf/0/$ref/dependantRequired", + error: `Instance has "${has}" but does not have "${missing}".`, + }; +} + +function assertContains(haystack, needle) { + Assert.ok( + haystack.find(item => ObjectUtils.deepEqual(item, needle)) !== null + ); +} + +add_task(function test_reach_experiment_dependentRequired() { + info("Testing that if id is present then content and template are required"); + + { + const message = { + ...MESSAGES[0], + id: "message-id", + }; + + const result = EXPERIMENT_VALIDATOR.validate(message); + Assert.ok(!result.valid, "message should not validate"); + + assertContains(result.errors, depError("id", "content")); + assertContains(result.errors, depError("id", "template")); + } + + { + const message = { + ...MESSAGES[0], + content: {}, + }; + + const result = EXPERIMENT_VALIDATOR.validate(message); + Assert.ok(!result.valid, "message should not validate"); + assertContains(result.errors, depError("content", "id")); + assertContains(result.errors, depError("content", "template")); + } + + { + const message = { + ...MESSAGES[0], + template: "cfr", + }; + + const result = EXPERIMENT_VALIDATOR.validate(message); + Assert.ok(!result.valid, "message should not validate"); + assertContains(result.errors, depError("template", "content")); + assertContains(result.errors, depError("template", "id")); + } +}); diff --git a/browser/components/newtab/test/xpcshell/xpcshell.ini b/browser/components/newtab/test/xpcshell/xpcshell.ini index fa3aaa6c5294..597ca743405d 100644 --- a/browser/components/newtab/test/xpcshell/xpcshell.ini +++ b/browser/components/newtab/test/xpcshell/xpcshell.ini @@ -26,3 +26,4 @@ skip-if = [test_InflightAssetsMessageProvider.js] [test_OnboardingMessageProvider.js] [test_PanelTestProvider.js] +[test_reach_experiments.js]