mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-10-19 00:05:36 +00:00
Bug 1644743 - Consolidate Targeting evaluation across Normandy, Experiment Manager and ASRouter r=k88hudson,mythmon,janerik
Differential Revision: https://phabricator.services.mozilla.com/D79967
This commit is contained in:
parent
15fb12a409
commit
08df665966
@ -1482,3 +1482,23 @@ Unlike other Activity Stream pings, this is a Firefox Events telemetry event, an
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Experiment attribute errors
|
||||
|
||||
This records whether issues were encountered with any of the targeting attributes used in the experiment enrollment or message targeting.
|
||||
Two different types of events are sent: `attribute_error` and `attribute_timeout` along with the attribute that caused it.
|
||||
|
||||
```js
|
||||
[
|
||||
"messaging_experiments",
|
||||
"targeting",
|
||||
"attribute_error", // event
|
||||
"foo" // attribute
|
||||
],
|
||||
[
|
||||
"messaging_experiments",
|
||||
"targeting",
|
||||
"attribute_timeout", // event
|
||||
"bar" // attribute
|
||||
]
|
||||
```
|
||||
|
@ -34,6 +34,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
|
||||
ExperimentAPI: "resource://messaging-system/experiments/ExperimentAPI.jsm",
|
||||
SpecialMessageActions:
|
||||
"resource://messaging-system/lib/SpecialMessageActions.jsm",
|
||||
TargetingContext: "resource://messaging-system/targeting/Targeting.jsm",
|
||||
});
|
||||
XPCOMUtils.defineLazyServiceGetters(this, {
|
||||
BrowserHandler: ["@mozilla.org/browser/clh;1", "nsIBrowserHandler"],
|
||||
@ -581,12 +582,13 @@ class _ASRouter {
|
||||
// Notify all tabs of messages that have become invalid after pref change
|
||||
const invalidMessages = [];
|
||||
const context = this._getMessagesContext();
|
||||
const targetingContext = new TargetingContext(context);
|
||||
|
||||
for (const msg of this.state.messages.filter(this.isUnblockedMessage)) {
|
||||
if (!msg.targeting) {
|
||||
continue;
|
||||
}
|
||||
const isMatch = await ASRouterTargeting.isMatch(msg.targeting, context);
|
||||
const isMatch = await targetingContext.evalWithDefault(msg.targeting);
|
||||
if (!isMatch) {
|
||||
invalidMessages.push(msg.id);
|
||||
}
|
||||
@ -1105,7 +1107,7 @@ class _ASRouter {
|
||||
});
|
||||
}
|
||||
|
||||
_handleTargetingError(type, error, message) {
|
||||
_handleTargetingError(error, message) {
|
||||
Cu.reportError(error);
|
||||
if (this.dispatchToAS) {
|
||||
this.dispatchToAS(
|
||||
@ -1113,7 +1115,6 @@ class _ASRouter {
|
||||
message_id: message.id,
|
||||
action: "asrouter_undesired_event",
|
||||
event: "TARGETING_EXPRESSION_ERROR",
|
||||
event_context: type,
|
||||
})
|
||||
);
|
||||
}
|
||||
@ -1147,10 +1148,11 @@ class _ASRouter {
|
||||
|
||||
async evaluateExpression(target, { expression, context }) {
|
||||
const channel = target || this.messageChannel;
|
||||
const targetingContext = new TargetingContext(context);
|
||||
let evaluationStatus;
|
||||
try {
|
||||
evaluationStatus = {
|
||||
result: await ASRouterTargeting.isMatch(expression, context),
|
||||
result: await targetingContext.evalWithDefault(expression),
|
||||
success: true,
|
||||
};
|
||||
} catch (e) {
|
||||
|
@ -21,8 +21,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
|
||||
TelemetryEnvironment: "resource://gre/modules/TelemetryEnvironment.jsm",
|
||||
AppConstants: "resource://gre/modules/AppConstants.jsm",
|
||||
AttributionCode: "resource:///modules/AttributionCode.jsm",
|
||||
FilterExpressions:
|
||||
"resource://gre/modules/components-utils/FilterExpressions.jsm",
|
||||
TargetingContext: "resource://messaging-system/targeting/Targeting.jsm",
|
||||
fxAccounts: "resource://gre/modules/FxAccounts.jsm",
|
||||
Region: "resource://gre/modules/Region.jsm",
|
||||
});
|
||||
@ -101,7 +100,6 @@ XPCOMUtils.defineLazyServiceGetter(
|
||||
);
|
||||
|
||||
const FXA_USERNAME_PREF = "services.sync.username";
|
||||
const MOZ_JEXL_FILEPATH = "mozjexl";
|
||||
|
||||
const { activityStreamProvider: asProvider } = NewTabUtils;
|
||||
|
||||
@ -566,37 +564,6 @@ const TargetingGetters = {
|
||||
this.ASRouterTargeting = {
|
||||
Environment: TargetingGetters,
|
||||
|
||||
ERROR_TYPES: {
|
||||
MALFORMED_EXPRESSION: "MALFORMED_EXPRESSION",
|
||||
ATTRIBUTE_ERROR: "JEXL_ATTRIBUTE_GETTER_ERROR",
|
||||
OTHER_ERROR: "OTHER_ERROR",
|
||||
},
|
||||
|
||||
// Combines the getter properties of two objects without evaluating them
|
||||
combineContexts(contextA = {}, contextB = {}, onError) {
|
||||
return {
|
||||
get: (obj, prop) => {
|
||||
try {
|
||||
return contextA[prop] || contextB[prop];
|
||||
} catch (error) {
|
||||
onError(this.ERROR_TYPES.ATTRIBUTE_ERROR, error, prop);
|
||||
}
|
||||
|
||||
return null;
|
||||
},
|
||||
};
|
||||
},
|
||||
|
||||
isMatch(filterExpression, customContext, onError) {
|
||||
return FilterExpressions.eval(
|
||||
filterExpression,
|
||||
new Proxy(
|
||||
{},
|
||||
this.combineContexts(customContext, this.Environment, onError)
|
||||
)
|
||||
);
|
||||
},
|
||||
|
||||
isTriggerMatch(trigger = {}, candidateMessageTrigger = {}) {
|
||||
if (trigger.id !== candidateMessageTrigger.id) {
|
||||
return false;
|
||||
@ -650,12 +617,12 @@ this.ASRouterTargeting = {
|
||||
* checkMessageTargeting - Checks is a message's targeting parameters are satisfied
|
||||
*
|
||||
* @param {*} message An AS router message
|
||||
* @param {obj} context A FilterExpression context
|
||||
* @param {obj} targetingContext a TargetingContext instance complete with eval environment
|
||||
* @param {func} onError A function to handle errors (takes two params; error, message)
|
||||
* @param {boolean} shouldCache Should the JEXL evaluations be cached and reused.
|
||||
* @returns
|
||||
*/
|
||||
async checkMessageTargeting(message, context, onError, shouldCache) {
|
||||
async checkMessageTargeting(message, targetingContext, onError, shouldCache) {
|
||||
// If no targeting is specified,
|
||||
if (!message.targeting) {
|
||||
return true;
|
||||
@ -668,7 +635,7 @@ this.ASRouterTargeting = {
|
||||
return result.value;
|
||||
}
|
||||
}
|
||||
result = await this.isMatch(message.targeting, context, onError);
|
||||
result = await targetingContext.evalWithDefault(message.targeting);
|
||||
if (shouldCache) {
|
||||
jexlEvaluationCache.set(message.targeting, {
|
||||
timestamp: Date.now(),
|
||||
@ -676,19 +643,22 @@ this.ASRouterTargeting = {
|
||||
});
|
||||
}
|
||||
} catch (error) {
|
||||
Cu.reportError(error);
|
||||
if (onError) {
|
||||
const type = error.fileName.includes(MOZ_JEXL_FILEPATH)
|
||||
? this.ERROR_TYPES.MALFORMED_EXPRESSION
|
||||
: this.ERROR_TYPES.OTHER_ERROR;
|
||||
onError(type, error, message);
|
||||
onError(error, message);
|
||||
}
|
||||
Cu.reportError(error);
|
||||
result = false;
|
||||
}
|
||||
return result;
|
||||
},
|
||||
|
||||
_isMessageMatch(message, trigger, context, onError, shouldCache = false) {
|
||||
_isMessageMatch(
|
||||
message,
|
||||
trigger,
|
||||
targetingContext,
|
||||
onError,
|
||||
shouldCache = false
|
||||
) {
|
||||
return (
|
||||
message &&
|
||||
(trigger
|
||||
@ -696,7 +666,12 @@ this.ASRouterTargeting = {
|
||||
: !message.trigger) &&
|
||||
// If a trigger expression was passed to this function, the message should match it.
|
||||
// Otherwise, we should choose a message with no trigger property (i.e. a message that can show up at any time)
|
||||
this.checkMessageTargeting(message, context, onError, shouldCache)
|
||||
this.checkMessageTargeting(
|
||||
message,
|
||||
targetingContext,
|
||||
onError,
|
||||
shouldCache
|
||||
)
|
||||
);
|
||||
},
|
||||
|
||||
@ -715,25 +690,28 @@ this.ASRouterTargeting = {
|
||||
*/
|
||||
async findMatchingMessage({
|
||||
messages,
|
||||
trigger,
|
||||
context,
|
||||
trigger = {},
|
||||
context = {},
|
||||
onError,
|
||||
ordered = false,
|
||||
shouldCache = false,
|
||||
returnAll = false,
|
||||
}) {
|
||||
const sortedMessages = getSortedMessages(messages, { ordered });
|
||||
const combinedContext = new Proxy(
|
||||
{},
|
||||
this.combineContexts(trigger && trigger.context, context, onError)
|
||||
);
|
||||
const matching = returnAll ? [] : null;
|
||||
const targetingContext = new TargetingContext(
|
||||
TargetingContext.combineContexts(
|
||||
context,
|
||||
this.Environment,
|
||||
trigger.context || {}
|
||||
)
|
||||
);
|
||||
|
||||
const isMatch = candidate =>
|
||||
this._isMessageMatch(
|
||||
candidate,
|
||||
trigger,
|
||||
combinedContext,
|
||||
targetingContext,
|
||||
onError,
|
||||
shouldCache
|
||||
);
|
||||
|
@ -48,33 +48,6 @@ ChromeUtils.defineModuleGetter(
|
||||
"resource://gre/modules/Region.jsm"
|
||||
);
|
||||
|
||||
// ASRouterTargeting.isMatch
|
||||
add_task(async function should_do_correct_targeting() {
|
||||
is(
|
||||
await ASRouterTargeting.isMatch("FOO", { FOO: true }),
|
||||
true,
|
||||
"should return true for a matching value"
|
||||
);
|
||||
is(
|
||||
await ASRouterTargeting.isMatch("!FOO", { FOO: true }),
|
||||
false,
|
||||
"should return false for a non-matching value"
|
||||
);
|
||||
});
|
||||
|
||||
add_task(async function should_handle_async_getters() {
|
||||
const context = {
|
||||
get FOO() {
|
||||
return Promise.resolve(true);
|
||||
},
|
||||
};
|
||||
is(
|
||||
await ASRouterTargeting.isMatch("FOO", context),
|
||||
true,
|
||||
"should return true for a matching async value"
|
||||
);
|
||||
});
|
||||
|
||||
// ASRouterTargeting.findMatchingMessage
|
||||
add_task(async function find_matching_message() {
|
||||
const messages = [
|
||||
@ -107,38 +80,10 @@ add_task(async function return_nothing_for_no_matching_message() {
|
||||
);
|
||||
});
|
||||
|
||||
add_task(async function check_syntax_error_handling() {
|
||||
let result;
|
||||
function onError(...args) {
|
||||
result = args;
|
||||
}
|
||||
|
||||
const messages = [{ id: "foo", targeting: "foo === 0" }];
|
||||
const match = await ASRouterTargeting.findMatchingMessage({
|
||||
messages,
|
||||
onError,
|
||||
});
|
||||
|
||||
is(
|
||||
match,
|
||||
undefined,
|
||||
"should return nothing since no valid matching message exists"
|
||||
);
|
||||
// Note that in order for the following test to pass, we are expecting a particular filepath for mozjexl.
|
||||
// If the location of this file has changed, the MOZ_JEXL_FILEPATH constant should be updated om ASRouterTargeting.jsm
|
||||
is(
|
||||
result[0],
|
||||
ASRouterTargeting.ERROR_TYPES.MALFORMED_EXPRESSION,
|
||||
"should recognize the error as coming from mozjexl and call onError with the MALFORMED_EXPRESSION error type"
|
||||
);
|
||||
ok(result[1].message, "should call onError with the error from mozjexl");
|
||||
is(result[2], messages[0], "should call onError with the invalid message");
|
||||
});
|
||||
|
||||
add_task(async function check_other_error_handling() {
|
||||
let result;
|
||||
let called = false;
|
||||
function onError(...args) {
|
||||
result = args;
|
||||
called = true;
|
||||
}
|
||||
|
||||
const messages = [{ id: "foo", targeting: "foo" }];
|
||||
@ -158,19 +103,8 @@ add_task(async function check_other_error_handling() {
|
||||
undefined,
|
||||
"should return nothing since no valid matching message exists"
|
||||
);
|
||||
// Note that in order for the following test to pass, we are expecting a particular filepath for mozjexl.
|
||||
// If the location of this file has changed, the MOZ_JEXL_FILEPATH constant should be updated om ASRouterTargeting.jsm
|
||||
is(
|
||||
result[0],
|
||||
ASRouterTargeting.ERROR_TYPES.ATTRIBUTE_ERROR,
|
||||
"should not recognize the error as being an attribute error."
|
||||
);
|
||||
is(
|
||||
result[1].message,
|
||||
"test error",
|
||||
"should call onError with the error thrown in the context"
|
||||
);
|
||||
is(result[2], "foo", "should call onError with the invalid attribute");
|
||||
|
||||
Assert.ok(called, "Attribute error caught");
|
||||
});
|
||||
|
||||
// ASRouterTargeting.Environment
|
||||
|
@ -71,6 +71,7 @@ describe("ASRouter", () => {
|
||||
let FakeToolbarPanelHub;
|
||||
let FakeMomentsPageHub;
|
||||
let personalizedCfrScores;
|
||||
let fakeTargetingContext;
|
||||
|
||||
function createFakeStorage() {
|
||||
const getStub = sandbox.stub();
|
||||
@ -168,6 +169,10 @@ describe("ASRouter", () => {
|
||||
uninit: sandbox.stub(),
|
||||
executeAction: sandbox.stub(),
|
||||
};
|
||||
fakeTargetingContext = {
|
||||
combineContexts: sandbox.stub(),
|
||||
evalWithDefault: sandbox.stub().resolves(),
|
||||
};
|
||||
globals.set({
|
||||
// Testing framework doesn't know how to `defineLazyModuleGetter` so we're
|
||||
// importing these modules into the global scope ourselves.
|
||||
@ -210,6 +215,15 @@ describe("ASRouter", () => {
|
||||
SpecialMessageActions: {
|
||||
handleAction: sandbox.stub(),
|
||||
},
|
||||
TargetingContext: class {
|
||||
static combineContexts(...args) {
|
||||
return fakeTargetingContext.combineContexts.apply(sandbox, args);
|
||||
}
|
||||
|
||||
evalWithDefault(expr) {
|
||||
return fakeTargetingContext.evalWithDefault(expr);
|
||||
}
|
||||
},
|
||||
});
|
||||
await createRouterAndInit();
|
||||
});
|
||||
@ -472,7 +486,7 @@ describe("ASRouter", () => {
|
||||
messages: [messageTargeted, messageNotTargeted],
|
||||
providers: [{ id: "snippets" }],
|
||||
});
|
||||
sandbox.stub(ASRouterTargeting, "isMatch").resolves(false);
|
||||
fakeTargetingContext.evalWithDefault.resolves(false);
|
||||
|
||||
await Router.onPrefChange("services.sync.username");
|
||||
|
||||
@ -552,7 +566,7 @@ describe("ASRouter", () => {
|
||||
beforeEach(async () => {
|
||||
stub = sandbox.stub();
|
||||
stub.resolves("foo");
|
||||
sandbox.stub(ASRouterTargeting, "isMatch").callsFake(stub);
|
||||
fakeTargetingContext.evalWithDefault.callsFake(stub);
|
||||
});
|
||||
afterEach(() => {
|
||||
sandbox.restore();
|
||||
@ -3488,9 +3502,7 @@ describe("ASRouter", () => {
|
||||
sandbox.stub(Router, "loadMessagesFromAllProviders");
|
||||
});
|
||||
it("should dispatch an event when a targeting expression throws an error", async () => {
|
||||
sandbox
|
||||
.stub(global.FilterExpressions, "eval")
|
||||
.returns(Promise.reject(new Error("fake error")));
|
||||
fakeTargetingContext.evalWithDefault.rejects("unit test error");
|
||||
await Router.setState({
|
||||
messages: [
|
||||
{
|
||||
|
@ -17,6 +17,7 @@ describe("#CachedTargetingGetter", () => {
|
||||
let clock;
|
||||
let frecentStub;
|
||||
let topsitesCache;
|
||||
let globals;
|
||||
beforeEach(() => {
|
||||
sandbox = sinon.createSandbox();
|
||||
clock = sinon.useFakeTimers();
|
||||
@ -26,11 +27,25 @@ describe("#CachedTargetingGetter", () => {
|
||||
);
|
||||
sandbox.stub(global.Cu, "reportError");
|
||||
topsitesCache = new CachedTargetingGetter("getTopFrecentSites");
|
||||
globals = new GlobalOverrider();
|
||||
globals.set(
|
||||
"TargetingContext",
|
||||
class {
|
||||
static combineContexts(...args) {
|
||||
return sinon.stub();
|
||||
}
|
||||
|
||||
evalWithDefault(expr) {
|
||||
return sinon.stub();
|
||||
}
|
||||
}
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
sandbox.restore();
|
||||
clock.restore();
|
||||
globals.restore();
|
||||
});
|
||||
|
||||
it("should only make a request every 6 hours", async () => {
|
||||
@ -188,26 +203,6 @@ describe("#CachedTargetingGetter", () => {
|
||||
assert.equal(arg_m3.id, m2.id);
|
||||
});
|
||||
});
|
||||
describe("combineContexts", () => {
|
||||
it("should combine the properties of the two objects", () => {
|
||||
const joined = ASRouterTargeting.combineContexts(
|
||||
{
|
||||
get foo() {
|
||||
return "foo";
|
||||
},
|
||||
},
|
||||
{
|
||||
get bar() {
|
||||
return "bar";
|
||||
},
|
||||
}
|
||||
);
|
||||
const proxy = new Proxy({}, joined);
|
||||
|
||||
assert.equal(proxy.foo, "foo");
|
||||
assert.equal(proxy.bar, "bar");
|
||||
});
|
||||
});
|
||||
});
|
||||
describe("#CacheListAttachedOAuthClients", () => {
|
||||
const twoHours = 2 * 60 * 60 * 1000;
|
||||
@ -262,34 +257,55 @@ describe("ASRouterTargeting", () => {
|
||||
let evalStub;
|
||||
let sandbox;
|
||||
let clock;
|
||||
let globals;
|
||||
let fakeTargetingContext;
|
||||
beforeEach(() => {
|
||||
sandbox = sinon.createSandbox();
|
||||
evalStub = sandbox.stub(global.FilterExpressions, "eval");
|
||||
sandbox.replace(ASRouterTargeting, "Environment", {});
|
||||
clock = sinon.useFakeTimers();
|
||||
fakeTargetingContext = {
|
||||
combineContexts: sandbox.stub(),
|
||||
evalWithDefault: sandbox.stub().resolves(),
|
||||
};
|
||||
globals = new GlobalOverrider();
|
||||
globals.set(
|
||||
"TargetingContext",
|
||||
class {
|
||||
static combineContexts(...args) {
|
||||
return fakeTargetingContext.combineContexts.apply(sandbox, args);
|
||||
}
|
||||
|
||||
evalWithDefault(expr) {
|
||||
return fakeTargetingContext.evalWithDefault(expr);
|
||||
}
|
||||
}
|
||||
);
|
||||
evalStub = fakeTargetingContext.evalWithDefault;
|
||||
});
|
||||
afterEach(() => {
|
||||
clock.restore();
|
||||
sandbox.restore();
|
||||
globals.restore();
|
||||
});
|
||||
it("should cache evaluation result", async () => {
|
||||
evalStub.resolves(true);
|
||||
let targetingContext = new global.TargetingContext();
|
||||
|
||||
await ASRouterTargeting.checkMessageTargeting(
|
||||
{ targeting: "jexl1" },
|
||||
{},
|
||||
targetingContext,
|
||||
sandbox.stub(),
|
||||
true
|
||||
);
|
||||
await ASRouterTargeting.checkMessageTargeting(
|
||||
{ targeting: "jexl2" },
|
||||
{},
|
||||
targetingContext,
|
||||
sandbox.stub(),
|
||||
true
|
||||
);
|
||||
await ASRouterTargeting.checkMessageTargeting(
|
||||
{ targeting: "jexl1" },
|
||||
{},
|
||||
targetingContext,
|
||||
sandbox.stub(),
|
||||
true
|
||||
);
|
||||
@ -298,22 +314,23 @@ describe("ASRouterTargeting", () => {
|
||||
});
|
||||
it("should not cache evaluation result", async () => {
|
||||
evalStub.resolves(true);
|
||||
let targetingContext = new global.TargetingContext();
|
||||
|
||||
await ASRouterTargeting.checkMessageTargeting(
|
||||
{ targeting: "jexl" },
|
||||
{},
|
||||
targetingContext,
|
||||
sandbox.stub(),
|
||||
false
|
||||
);
|
||||
await ASRouterTargeting.checkMessageTargeting(
|
||||
{ targeting: "jexl" },
|
||||
{},
|
||||
targetingContext,
|
||||
sandbox.stub(),
|
||||
false
|
||||
);
|
||||
await ASRouterTargeting.checkMessageTargeting(
|
||||
{ targeting: "jexl" },
|
||||
{},
|
||||
targetingContext,
|
||||
sandbox.stub(),
|
||||
false
|
||||
);
|
||||
@ -322,23 +339,24 @@ describe("ASRouterTargeting", () => {
|
||||
});
|
||||
it("should expire cache entries", async () => {
|
||||
evalStub.resolves(true);
|
||||
let targetingContext = new global.TargetingContext();
|
||||
|
||||
await ASRouterTargeting.checkMessageTargeting(
|
||||
{ targeting: "jexl" },
|
||||
{},
|
||||
targetingContext,
|
||||
sandbox.stub(),
|
||||
true
|
||||
);
|
||||
await ASRouterTargeting.checkMessageTargeting(
|
||||
{ targeting: "jexl" },
|
||||
{},
|
||||
targetingContext,
|
||||
sandbox.stub(),
|
||||
true
|
||||
);
|
||||
clock.tick(5 * 60 * 1000 + 1);
|
||||
await ASRouterTargeting.checkMessageTargeting(
|
||||
{ targeting: "jexl" },
|
||||
{},
|
||||
targetingContext,
|
||||
sandbox.stub(),
|
||||
true
|
||||
);
|
||||
|
@ -6,3 +6,4 @@ toolkit.jar:
|
||||
% resource messaging-system %res/messaging-system/
|
||||
res/messaging-system/experiments/ (./experiments/*)
|
||||
res/messaging-system/lib/ (./lib/*)
|
||||
res/messaging-system/targeting/Targeting.jsm (./targeting/Targeting.jsm)
|
||||
|
@ -20,6 +20,7 @@ const { XPCOMUtils } = ChromeUtils.import(
|
||||
|
||||
XPCOMUtils.defineLazyModuleGetters(this, {
|
||||
ASRouterTargeting: "resource://activity-stream/lib/ASRouterTargeting.jsm",
|
||||
TargetingContext: "resource://messaging-system/targeting/Targeting.jsm",
|
||||
ExperimentManager:
|
||||
"resource://messaging-system/experiments/ExperimentManager.jsm",
|
||||
RemoteSettings: "resource://services-settings/remote-settings.js",
|
||||
@ -106,20 +107,24 @@ class _RemoteSettingsExperimentLoader {
|
||||
* @returns {Promise<boolean>} Should we process the recipe?
|
||||
*/
|
||||
async checkTargeting(recipe, customContext = {}) {
|
||||
const context = TargetingContext.combineContexts(
|
||||
customContext,
|
||||
ASRouterTargeting.Environment
|
||||
);
|
||||
const { targeting } = recipe;
|
||||
if (!targeting) {
|
||||
log.debug("No targeting for recipe, so it matches automatically");
|
||||
return true;
|
||||
}
|
||||
log.debug("Testing targeting expression:", targeting);
|
||||
const result = await ASRouterTargeting.isMatch(
|
||||
targeting,
|
||||
customContext,
|
||||
err => {
|
||||
log.debug("Targeting failed because of an error");
|
||||
Cu.reportError(err);
|
||||
}
|
||||
);
|
||||
const targetingContext = new TargetingContext(context);
|
||||
let result = false;
|
||||
try {
|
||||
result = await targetingContext.evalWithDefault(targeting);
|
||||
} catch (e) {
|
||||
log.debug("Targeting failed because of an error");
|
||||
Cu.reportError(e);
|
||||
}
|
||||
return Boolean(result);
|
||||
}
|
||||
|
||||
|
@ -15,6 +15,7 @@ BROWSER_CHROME_MANIFESTS += [
|
||||
SPHINX_TREES['docs'] = 'schemas'
|
||||
|
||||
XPCSHELL_TESTS_MANIFESTS += ['test/unit/xpcshell.ini']
|
||||
XPCSHELL_TESTS_MANIFESTS += ['targeting/test/unit/xpcshell.ini']
|
||||
|
||||
TESTING_JS_MODULES += [
|
||||
'schemas/SpecialMessageActionSchemas/SpecialMessageActionSchemas.json',
|
||||
|
203
toolkit/components/messaging-system/targeting/Targeting.jsm
Normal file
203
toolkit/components/messaging-system/targeting/Targeting.jsm
Normal file
@ -0,0 +1,203 @@
|
||||
/* This Source Code Form is subject to the terms of the Mozilla Public
|
||||
* License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
|
||||
|
||||
"use strict";
|
||||
|
||||
const { XPCOMUtils } = ChromeUtils.import(
|
||||
"resource://gre/modules/XPCOMUtils.jsm"
|
||||
);
|
||||
XPCOMUtils.defineLazyModuleGetters(this, {
|
||||
Services: "resource://gre/modules/Services.jsm",
|
||||
clearTimeout: "resource://gre/modules/Timer.jsm",
|
||||
setTimeout: "resource://gre/modules/Timer.jsm",
|
||||
ASRouterTargeting: "resource://activity-stream/lib/ASRouterTargeting.jsm",
|
||||
FilterExpressions:
|
||||
"resource://gre/modules/components-utils/FilterExpressions.jsm",
|
||||
ClientEnvironment: "resource://normandy/lib/ClientEnvironment.jsm",
|
||||
});
|
||||
|
||||
var EXPORTED_SYMBOLS = ["TargetingContext"];
|
||||
|
||||
const TARGETING_EVENT_CATEGORY = "messaging_experiments";
|
||||
const TARGETING_EVENT_METHOD = "targeting";
|
||||
const DEFAULT_TIMEOUT = 3000;
|
||||
const ERROR_TYPES = {
|
||||
ATTRIBUTE_ERROR: "attribute_error",
|
||||
TIMEOUT: "attribute_timeout",
|
||||
};
|
||||
|
||||
const TargetingEnvironment = {
|
||||
get locale() {
|
||||
return ASRouterTargeting.Environment.locale;
|
||||
},
|
||||
|
||||
get localeLanguageCode() {
|
||||
return ASRouterTargeting.Environment.localeLanguageCode;
|
||||
},
|
||||
|
||||
get region() {
|
||||
return ASRouterTargeting.Environment.region;
|
||||
},
|
||||
|
||||
get userId() {
|
||||
return ClientEnvironment.userId;
|
||||
},
|
||||
};
|
||||
|
||||
class TargetingContext {
|
||||
constructor(customContext) {
|
||||
if (customContext) {
|
||||
this.ctx = new Proxy(customContext, {
|
||||
get: (customCtx, prop) => {
|
||||
if (prop in TargetingEnvironment) {
|
||||
return TargetingEnvironment[prop];
|
||||
}
|
||||
return customCtx[prop];
|
||||
},
|
||||
});
|
||||
} else {
|
||||
this.ctx = TargetingEnvironment;
|
||||
}
|
||||
|
||||
// Enable event recording
|
||||
Services.telemetry.setEventRecordingEnabled(TARGETING_EVENT_CATEGORY, true);
|
||||
}
|
||||
|
||||
_sendUndesiredEvent(eventData) {
|
||||
Services.telemetry.recordEvent(
|
||||
TARGETING_EVENT_CATEGORY,
|
||||
TARGETING_EVENT_METHOD,
|
||||
eventData.event,
|
||||
eventData.value
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Wrap each property of context[key] with a Proxy that captures errors and
|
||||
* timeouts
|
||||
*
|
||||
* @param {Object.<string, TargetingGetters> | TargetingGetters} context
|
||||
* @param {string} key Namespace value found in `context` param
|
||||
* @returns {TargetingGetters} Wrapped context where getter report errors and timeouts
|
||||
*/
|
||||
createContextWithTimeout(context, key = null) {
|
||||
const timeoutDuration = key ? context[key].timeout : context.timeout;
|
||||
const logUndesiredEvent = (event, key, prop) => {
|
||||
const value = key ? `${key}.${prop}` : prop;
|
||||
this._sendUndesiredEvent({ event, value });
|
||||
Cu.reportError(`${event}: ${value}`);
|
||||
};
|
||||
|
||||
return new Proxy(context, {
|
||||
get(target, prop) {
|
||||
// eslint-disable-next-line no-async-promise-executor
|
||||
return new Promise(async (resolve, reject) => {
|
||||
// Create timeout cb to record attribute resolution taking too long.
|
||||
let timeout = setTimeout(() => {
|
||||
logUndesiredEvent(ERROR_TYPES.TIMEOUT, key, prop);
|
||||
reject(
|
||||
new Error(
|
||||
`${prop} targeting getter timed out after ${timeoutDuration ||
|
||||
DEFAULT_TIMEOUT}ms`
|
||||
)
|
||||
);
|
||||
}, timeoutDuration || DEFAULT_TIMEOUT);
|
||||
|
||||
try {
|
||||
resolve(await (key ? target[key][prop] : target[prop]));
|
||||
} catch (error) {
|
||||
logUndesiredEvent(ERROR_TYPES.ATTRIBUTE_ERROR, key, prop);
|
||||
reject(error);
|
||||
Cu.reportError(error);
|
||||
} finally {
|
||||
clearTimeout(timeout);
|
||||
}
|
||||
});
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge all evaluation contexts and wrap the getters with timeouts
|
||||
*
|
||||
* @param {Object.<string, TargetingGetters>[]} contexts
|
||||
* @returns {Object.<string, TargetingGetters>} Object that follows the pattern of `namespace: getters`
|
||||
*/
|
||||
mergeEvaluationContexts(contexts) {
|
||||
let context = {};
|
||||
for (let c of contexts) {
|
||||
for (let envNamespace of Object.keys(c)) {
|
||||
// Take the provided context apart, replace it with a proxy
|
||||
context[envNamespace] = this.createContextWithTimeout(c, envNamespace);
|
||||
}
|
||||
}
|
||||
|
||||
return context;
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge multiple TargetingGetters objects without accidentally evaluating
|
||||
*
|
||||
* @param {TargetingGetters[]} ...contexts
|
||||
* @returns {Proxy<TargetingGetters>}
|
||||
*/
|
||||
static combineContexts(...contexts) {
|
||||
return new Proxy(
|
||||
{},
|
||||
{
|
||||
get(target, prop) {
|
||||
for (let context of contexts) {
|
||||
if (prop in context) {
|
||||
return context[prop];
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
},
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Evaluate JEXL expressions with default `TargetingEnvironment` and custom
|
||||
* provided targeting contexts
|
||||
*
|
||||
* @example
|
||||
* eval(
|
||||
* "ctx.locale == 'en-US' && customCtx.foo == 42",
|
||||
* { customCtx: { foo: 42 } }
|
||||
* ); // true
|
||||
*
|
||||
* @param {string} expression JEXL expression
|
||||
* @param {Object.<string, TargetingGetters>[]} ...contexts Additional custom context
|
||||
* objects where the keys act as namespaces for the different getters
|
||||
*
|
||||
* @returns {promise} Evaluation result
|
||||
*/
|
||||
eval(expression, ...contexts) {
|
||||
return FilterExpressions.eval(
|
||||
expression,
|
||||
this.mergeEvaluationContexts([{ ctx: this.ctx }, ...contexts])
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Evaluate JEXL expressions with default provided targeting context
|
||||
*
|
||||
* @example
|
||||
* new TargetingContext({ bar: 42 });
|
||||
* evalWithDefault(
|
||||
* "bar == 42",
|
||||
* ); // true
|
||||
*
|
||||
* @param {string} expression JEXL expression
|
||||
* @returns {promise} Evaluation result
|
||||
*/
|
||||
evalWithDefault(expression) {
|
||||
return FilterExpressions.eval(
|
||||
expression,
|
||||
this.createContextWithTimeout(this.ctx)
|
||||
);
|
||||
}
|
||||
}
|
@ -0,0 +1,5 @@
|
||||
"use strict";
|
||||
// Globals
|
||||
|
||||
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
|
||||
const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm");
|
@ -0,0 +1,206 @@
|
||||
const { TargetingContext } = ChromeUtils.import(
|
||||
"resource://messaging-system/targeting/Targeting.jsm"
|
||||
);
|
||||
const { TelemetryTestUtils } = ChromeUtils.import(
|
||||
"resource://testing-common/TelemetryTestUtils.jsm"
|
||||
);
|
||||
|
||||
add_task(async function instance_with_default() {
|
||||
let targeting = new TargetingContext();
|
||||
|
||||
let res = await targeting.eval(
|
||||
`ctx.locale == '${Services.locale.appLocaleAsBCP47}'`
|
||||
);
|
||||
|
||||
Assert.ok(res, "Has local context");
|
||||
});
|
||||
|
||||
add_task(async function instance_with_context() {
|
||||
let targeting = new TargetingContext({ bar: 42 });
|
||||
|
||||
let res = await targeting.eval("ctx.bar == 42");
|
||||
|
||||
Assert.ok(res, "Merge provided context with default");
|
||||
});
|
||||
|
||||
add_task(async function eval_1_context() {
|
||||
let targeting = new TargetingContext();
|
||||
|
||||
let res = await targeting.eval("custom1.bar == 42", { custom1: { bar: 42 } });
|
||||
|
||||
Assert.ok(res, "Eval uses provided context");
|
||||
});
|
||||
|
||||
add_task(async function eval_2_context() {
|
||||
let targeting = new TargetingContext();
|
||||
|
||||
let res = await targeting.eval("custom1.bar == 42 && custom2.foo == 42", {
|
||||
custom1: { bar: 42 },
|
||||
custom2: { foo: 42 },
|
||||
});
|
||||
|
||||
Assert.ok(res, "Eval uses provided context");
|
||||
});
|
||||
|
||||
add_task(async function eval_multiple_context() {
|
||||
let targeting = new TargetingContext();
|
||||
|
||||
let res = await targeting.eval(
|
||||
"custom1.bar == 42 && custom2.foo == 42 && custom3.baz == 42",
|
||||
{ custom1: { bar: 42 }, custom2: { foo: 42 } },
|
||||
{ custom3: { baz: 42 } }
|
||||
);
|
||||
|
||||
Assert.ok(res, "Eval uses provided context");
|
||||
});
|
||||
|
||||
add_task(async function eval_multiple_context_precedence() {
|
||||
let targeting = new TargetingContext();
|
||||
|
||||
let res = await targeting.eval(
|
||||
"custom1.bar == 42 && custom2.foo == 42",
|
||||
{ custom1: { bar: 24 }, custom2: { foo: 24 } },
|
||||
{ custom1: { bar: 42 }, custom2: { foo: 42 } }
|
||||
);
|
||||
|
||||
Assert.ok(res, "Last provided context overrides previously defined ones.");
|
||||
});
|
||||
|
||||
add_task(async function eval_evalWithDefault() {
|
||||
let targeting = new TargetingContext({ foo: 42 });
|
||||
|
||||
let res = await targeting.evalWithDefault("foo == 42");
|
||||
|
||||
Assert.ok(res, "Eval uses provided context");
|
||||
});
|
||||
|
||||
add_task(async function log_targeting_error_events() {
|
||||
let ctx = {
|
||||
get foo() {
|
||||
throw new Error("unit test");
|
||||
},
|
||||
};
|
||||
let targeting = new TargetingContext(ctx);
|
||||
let stub = sinon.stub(targeting, "_sendUndesiredEvent");
|
||||
|
||||
await Assert.rejects(
|
||||
targeting.evalWithDefault("foo == 42", ctx),
|
||||
/unit test/,
|
||||
"Getter should throw"
|
||||
);
|
||||
|
||||
Assert.equal(stub.callCount, 1, "Error event was logged");
|
||||
let {
|
||||
args: [{ event, value }],
|
||||
} = stub.firstCall;
|
||||
Assert.equal(event, "attribute_error", "Correct error message");
|
||||
Assert.equal(value, "foo", "Correct attribute name");
|
||||
});
|
||||
|
||||
add_task(async function eval_evalWithDefault_precedence() {
|
||||
let targeting = new TargetingContext({ region: "space" });
|
||||
let res = await targeting.evalWithDefault("region != 'space'");
|
||||
|
||||
Assert.ok(res, "Custom context does not override TargetingEnvironment");
|
||||
});
|
||||
|
||||
add_task(async function eval_evalWithDefault_combineContexts() {
|
||||
let combinedCtxs = TargetingContext.combineContexts({ foo: 1 }, { foo: 2 });
|
||||
let targeting = new TargetingContext(combinedCtxs);
|
||||
let res = await targeting.evalWithDefault("foo == 1");
|
||||
|
||||
Assert.ok(res, "First match is returned for combineContexts");
|
||||
});
|
||||
|
||||
add_task(async function log_targeting_error_events_in_namespace() {
|
||||
let ctx = {
|
||||
get foo() {
|
||||
throw new Error("unit test");
|
||||
},
|
||||
};
|
||||
let targeting = new TargetingContext(ctx);
|
||||
let stub = sinon.stub(targeting, "_sendUndesiredEvent");
|
||||
let catchStub = sinon.stub();
|
||||
|
||||
try {
|
||||
await targeting.eval("ctx.foo == 42");
|
||||
} catch (e) {
|
||||
catchStub();
|
||||
}
|
||||
|
||||
Assert.equal(stub.callCount, 1, "Error event was logged");
|
||||
let {
|
||||
args: [{ event, value }],
|
||||
} = stub.firstCall;
|
||||
Assert.equal(event, "attribute_error", "Correct error message");
|
||||
Assert.equal(value, "ctx.foo", "Correct attribute name");
|
||||
Assert.ok(catchStub.calledOnce, "eval throws errors");
|
||||
});
|
||||
|
||||
add_task(async function log_timeout_errors() {
|
||||
let ctx = {
|
||||
timeout: 1,
|
||||
get foo() {
|
||||
return new Promise(() => {});
|
||||
},
|
||||
};
|
||||
|
||||
let targeting = new TargetingContext(ctx);
|
||||
let stub = sinon.stub(targeting, "_sendUndesiredEvent");
|
||||
let catchStub = sinon.stub();
|
||||
|
||||
try {
|
||||
await targeting.eval("ctx.foo");
|
||||
} catch (e) {
|
||||
catchStub();
|
||||
}
|
||||
|
||||
Assert.equal(catchStub.callCount, 1, "Timeout error throws");
|
||||
Assert.equal(stub.callCount, 1, "Timeout event was logged");
|
||||
let {
|
||||
args: [{ event, value }],
|
||||
} = stub.firstCall;
|
||||
Assert.equal(event, "attribute_timeout", "Correct error message");
|
||||
Assert.equal(value, "ctx.foo", "Correct attribute name");
|
||||
});
|
||||
|
||||
add_task(async function test_telemetry_event_timeout() {
|
||||
Services.telemetry.clearEvents();
|
||||
let ctx = {
|
||||
timeout: 1,
|
||||
get foo() {
|
||||
return new Promise(() => {});
|
||||
},
|
||||
};
|
||||
let expectedEvents = [
|
||||
["messaging_experiments", "targeting", "attribute_timeout", "ctx.foo"],
|
||||
];
|
||||
let targeting = new TargetingContext(ctx);
|
||||
|
||||
try {
|
||||
await targeting.eval("ctx.foo");
|
||||
} catch (e) {}
|
||||
|
||||
TelemetryTestUtils.assertEvents(expectedEvents);
|
||||
Services.telemetry.clearEvents();
|
||||
});
|
||||
|
||||
add_task(async function test_telemetry_event_error() {
|
||||
Services.telemetry.clearEvents();
|
||||
let ctx = {
|
||||
get bar() {
|
||||
throw new Error("unit test");
|
||||
},
|
||||
};
|
||||
let expectedEvents = [
|
||||
["messaging_experiments", "targeting", "attribute_error", "ctx.bar"],
|
||||
];
|
||||
let targeting = new TargetingContext(ctx);
|
||||
|
||||
try {
|
||||
await targeting.eval("ctx.bar");
|
||||
} catch (e) {}
|
||||
|
||||
TelemetryTestUtils.assertEvents(expectedEvents);
|
||||
Services.telemetry.clearEvents();
|
||||
});
|
@ -0,0 +1,6 @@
|
||||
[DEFAULT]
|
||||
head = head.js
|
||||
tags = messaging-system
|
||||
firefox-appdir = browser
|
||||
|
||||
[test_targeting.js]
|
@ -24,6 +24,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
|
||||
Storage: "resource://normandy/lib/Storage.jsm",
|
||||
FilterExpressions:
|
||||
"resource://gre/modules/components-utils/FilterExpressions.jsm",
|
||||
TargetingContext: "resource://messaging-system/targeting/Targeting.jsm",
|
||||
NormandyApi: "resource://normandy/lib/NormandyApi.jsm",
|
||||
ClientEnvironment: "resource://normandy/lib/ClientEnvironment.jsm",
|
||||
CleanupManager: "resource://normandy/lib/CleanupManager.jsm",
|
||||
@ -539,8 +540,9 @@ var RecipeRunner = {
|
||||
}
|
||||
|
||||
const context = this.getFilterContext(recipe);
|
||||
const targetingContext = new TargetingContext();
|
||||
try {
|
||||
if (await FilterExpressions.eval(recipe.filter_expression, context)) {
|
||||
if (await targetingContext.eval(recipe.filter_expression, context)) {
|
||||
yield BaseAction.suitability.FILTER_MATCH;
|
||||
} else {
|
||||
yield BaseAction.suitability.FILTER_MISMATCH;
|
||||
|
@ -922,6 +922,26 @@ messaging_experiments:
|
||||
branches: >
|
||||
A semicolon separated string for all the qualified branch ID(s).
|
||||
e.g. "control;variant_01;treatment_02".
|
||||
targeting:
|
||||
objects:
|
||||
- attribute_error
|
||||
- attribute_timeout
|
||||
methods:
|
||||
- targeting
|
||||
release_channel_collection: opt-out
|
||||
products:
|
||||
- firefox
|
||||
record_in_processes:
|
||||
- main
|
||||
description: >
|
||||
Record generic JEXL errors that result from issues with experiment or
|
||||
message targeting expressions. The value field contains the namespace and
|
||||
attribute name that caused the error.
|
||||
bug_numbers:
|
||||
- 1644743
|
||||
notification_emails:
|
||||
- ujet@mozilla.com
|
||||
expiry_version: never
|
||||
|
||||
# This category contains event entries used for Telemetry tests.
|
||||
# They will not be sent out with any pings.
|
||||
|
Loading…
Reference in New Issue
Block a user