Skip to content
This repository was archived by the owner on Mar 27, 2024. It is now read-only.

Commit 4566911

Browse files
authored
fix: malformed definition breaking flow, add test (#181)
1 parent d0a1f0e commit 4566911

File tree

2 files changed

+66
-37
lines changed

2 files changed

+66
-37
lines changed

src/engine/evaluate.ts

+41-37
Original file line numberDiff line numberDiff line change
@@ -15,45 +15,49 @@ export const evaluateCondition = (
1515
condition: Readonly<EngineCondition<AudienceDefinitionFilter>>,
1616
pageViews: Readonly<PageView[]>
1717
): boolean => {
18-
const { filter, rules } = condition;
18+
try {
19+
const { filter, rules } = condition;
1920

20-
// if no queries, do not match at all
21-
if (pageViews.length === 0 || filter.queries.length === 0) {
22-
return false;
23-
}
21+
// if no queries, do not match at all
22+
if (pageViews.length === 0 || filter.queries.length === 0) {
23+
return false;
24+
}
2425

25-
/* Checks each pageView once for any matching queries
26-
* and returns the filtered array containing the matched
27-
* pageViews
28-
*
29-
* TODO Here, also, we have an opportunity to implement the
30-
* internal AND logic, swapping some for every
31-
* according to the value of filter.any
32-
*
33-
* NOTE: there is actually a semantic problem here.
34-
* The filter definition specifies an 'any' switch,
35-
* which defaults to false. However, the current
36-
* filtering uses `some` to check the query conditions.
37-
* The complete implementation here would be:
38-
* ```
39-
* filter.queries[filter.any ? 'some' : 'every']((query) => ...
40-
* ```
41-
* Yet, there is too much (testing) code depending on
42-
* this defaulting to `some` and the refactor would take
43-
* some time.
44-
*/
45-
const filteredPageViews = pageViews.filter((pageView) =>
46-
filter.queries.some((query) => {
47-
const pageFeatures = pageView.features[query.queryProperty];
48-
return queryMatches(query, pageFeatures);
49-
})
50-
);
26+
/* Checks each pageView once for any matching queries
27+
* and returns the filtered array containing the matched
28+
* pageViews
29+
*
30+
* TODO Here, also, we have an opportunity to implement the
31+
* internal AND logic, swapping some for every
32+
* according to the value of filter.any
33+
*
34+
* NOTE: there is actually a semantic problem here.
35+
* The filter definition specifies an 'any' switch,
36+
* which defaults to false. However, the current
37+
* filtering uses `some` to check the query conditions.
38+
* The complete implementation here would be:
39+
* ```
40+
* filter.queries[filter.any ? 'some' : 'every']((query) => ...
41+
* ```
42+
* Yet, there is too much (testing) code depending on
43+
* this defaulting to `some` and the refactor would take
44+
* some time.
45+
*/
46+
const filteredPageViews = pageViews.filter((pageView) =>
47+
filter.queries.some((query) => {
48+
const pageFeatures = pageView.features[query.queryProperty];
49+
return queryMatches(query, pageFeatures);
50+
})
51+
);
5152

52-
return rules.every((rule) => {
53-
const reducer = reducers[rule.reducer.name]();
54-
const value = reducer(filteredPageViews);
55-
const matches = matchers[rule.matcher.name](rule.matcher.args);
53+
return rules.every((rule) => {
54+
const reducer = reducers[rule.reducer.name]();
55+
const value = reducer(filteredPageViews);
56+
const matches = matchers[rule.matcher.name](rule.matcher.args);
5657

57-
return matches(value);
58-
});
58+
return matches(value);
59+
});
60+
} catch (error) {
61+
return false;
62+
}
5963
};

test/unit/run.test.ts

+25
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,31 @@ describe('edkt run method', () => {
117117

118118
expect(edkt.getMatchedAudiences()).toHaveLength(0);
119119
});
120+
121+
it('does not break execution if there is a malformed audience definition', async () => {
122+
setUpLocalStorage(TWO_PAGE_VIEW);
123+
124+
const malformedAudience = {
125+
id: 'test_id',
126+
version: 7,
127+
ttl: 1209600,
128+
lookBack: 0,
129+
occurrences: 0,
130+
};
131+
132+
await edkt.run({
133+
pageFeatures,
134+
audienceDefinitions: [
135+
audienceDefinition,
136+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
137+
// @ts-ignore
138+
malformedAudience,
139+
],
140+
omitGdprConsent: true,
141+
});
142+
143+
expect(edkt.getMatchedAudiences()).toHaveLength(1);
144+
});
120145
});
121146

122147
describe('look back edkt run behaviour', () => {

0 commit comments

Comments
 (0)