Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(rum-oversight): unknown filter should return empty set #9

Merged
merged 1 commit into from
Oct 14, 2024
Merged

fix(rum-oversight): unknown filter should return empty set #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 50 additions & 34 deletions distiller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,12 @@ export class DataChunks {
* @param {string[]} skipped facets to skip
*/
filterBundles(bundles, filterSpec, skipped = []) {
const existenceFilterFn = ([facetName]) => this.facetFns[facetName];
const existenceFilterFn = ([facetName]) => {
if (!this.facetFns[facetName]) {
throw new Error(`Unknown "${facetName}" facet in filter`);
}
return this.facetFns[facetName];
};
const skipFilterFn = ([facetName]) => !skipped.includes(facetName);
const valuesExtractorFn = (attributeName, bundle, parent) => {
const facetValue = parent.facetFns[attributeName](bundle);
Expand All @@ -526,38 +531,44 @@ export class DataChunks {
*/
// eslint-disable-next-line max-len
applyFilter(bundles, filterSpec, skipFilterFn, existenceFilterFn, valuesExtractorFn, combinerExtractorFn) {
const filterBy = Object.entries(filterSpec)
.filter(skipFilterFn)
.filter(([, desiredValues]) => desiredValues.length)
.filter(existenceFilterFn);
return bundles.filter((bundle) => filterBy.every(([attributeName, desiredValues]) => {
const actualValues = valuesExtractorFn(attributeName, bundle, this);

const combiners = {
// if some elements match, then return true (partial inclusion)
some: 'some',
// if some elements do not match, then return true (partial exclusion)
none: 'some',
// if every element matches, then return true (full inclusion)
every: 'every',
// if every element does not match, then return true (full exclusion)
never: 'every',
};

const negators = {
some: (value) => value,
every: (value) => value,
none: (value) => !value,
never: (value) => !value,
};
// this can be some, every, or none
const combinerprefence = combinerExtractorFn(attributeName, this);

const combiner = combiners[combinerprefence];
const negator = negators[combinerprefence];

return desiredValues[combiner]((value) => negator(actualValues.includes(value)));
}));
try {
const filterBy = Object.entries(filterSpec)
.filter(skipFilterFn)
.filter(([, desiredValues]) => desiredValues.length)
.filter(existenceFilterFn);
return bundles.filter((bundle) => filterBy.every(([attributeName, desiredValues]) => {
const actualValues = valuesExtractorFn(attributeName, bundle, this);

const combiners = {
// if some elements match, then return true (partial inclusion)
some: 'some',
// if some elements do not match, then return true (partial exclusion)
none: 'some',
// if every element matches, then return true (full inclusion)
every: 'every',
// if every element does not match, then return true (full exclusion)
never: 'every',
};

const negators = {
some: (value) => value,
every: (value) => value,
none: (value) => !value,
never: (value) => !value,
};
// this can be some, every, or none
const combinerprefence = combinerExtractorFn(attributeName, this);

const combiner = combiners[combinerprefence];
const negator = negators[combinerprefence];

return desiredValues[combiner]((value) => negator(actualValues.includes(value)));
}));
} catch (error) {
// eslint-disable-next-line no-console
console.warn(`Error while applying filter: ${error.message}`);
return [];
}
}

/**
Expand All @@ -572,7 +583,12 @@ export class DataChunks {
* @returns {boolean} the result of the check.
*/
hasConversion(aBundle, filterSpec, combiner) {
const existenceFilterFn = ([facetName]) => this.facetFns[facetName];
const existenceFilterFn = ([facetName]) => {
if (!this.facetFns[facetName]) {
throw new Error(`Unknown "${facetName}" facet in filter`);
}
return this.facetFns[facetName];
};
const skipFilterFn = () => true;
const valuesExtractorFn = (attributeName, bundle, parent) => {
const facetValue = parent.facetFns[attributeName](bundle);
Expand Down
155 changes: 93 additions & 62 deletions test/distiller.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,12 @@ describe('DataChunks', () => {
id: ['one'],
};
assert.equal(d.filtered.length, 1);

d.filter = {
// trying to filter with an undefined facet
unknown: ['unknown'],
};
assert.equal(d.filtered.length, 0);
});

it('DataChunk.group()', () => {
Expand Down Expand Up @@ -686,6 +692,7 @@ describe('DataChunks', () => {
d.load(chunks1);

// define facet functions
d.addFacet('host', (bundle) => bundle.host);
d.addFacet('userAgent', (bundle) => {
const parts = bundle.userAgent.split(':');
return parts.reduce((acc, _, i) => {
Expand Down Expand Up @@ -782,6 +789,7 @@ describe('DataChunks', () => {
d.load(chunks1);

// define facet functions
d.addFacet('host', (bundle) => bundle.host);
d.addFacet(
// as a convention, we use ! to indicate negation. When used in URL parameters, this
// looks like a nice filter: ?userAgent!=desktop
Expand Down Expand Up @@ -809,69 +817,70 @@ describe('DataChunks', () => {
});
});
describe('DataChunks.hasConversion', () => {
const chunks = [
{
date: '2024-05-06',
rumBundles: [
{
id: 'one',
host: 'www.aem.live',
time: '2024-05-06T00:00:04.444Z',
timeSlot: '2024-05-06T00:00:00.000Z',
url: 'https://www.aem.live/developer/tutorial',
userAgent: 'desktop:windows',
weight: 100,
events: [
{
checkpoint: 'top',
target: 'visible',
timeDelta: 100,
},
],
},
{
id: 'two',
host: 'www.aem.live',
time: '2024-05-06T00:00:04.444Z',
timeSlot: '2024-05-06T00:00:00.000Z',
url: 'https://www.aem.live/home',
userAgent: 'desktop',
weight: 100,
events: [
{
checkpoint: 'top',
target: 'hidden',
timeDelta: 200,
},
{
checkpoint: 'click',
},
],
},
{
id: 'three',
host: 'www.aem.live',
time: '2024-05-06T00:00:04.444Z',
timeSlot: '2024-05-06T00:00:00.000Z',
url: 'https://www.aem.live/home',
userAgent: 'mobile:ios',
weight: 100,
events: [
{
checkpoint: 'top',
target: 'visible',
timeDelta: 200,
},
{
checkpoint: 'viewmedia',
target: 'some_image.png',
},
],
},
],
},
];

it('will tag bundles with convert and not-convert based on a filter spec', () => {
const chunks = [
{
date: '2024-05-06',
rumBundles: [
{
id: 'one',
host: 'www.aem.live',
time: '2024-05-06T00:00:04.444Z',
timeSlot: '2024-05-06T00:00:00.000Z',
url: 'https://www.aem.live/developer/tutorial',
userAgent: 'desktop:windows',
weight: 100,
events: [
{
checkpoint: 'top',
target: 'visible',
timeDelta: 100,
},
],
},
{
id: 'two',
host: 'www.aem.live',
time: '2024-05-06T00:00:04.444Z',
timeSlot: '2024-05-06T00:00:00.000Z',
url: 'https://www.aem.live/home',
userAgent: 'desktop',
weight: 100,
events: [
{
checkpoint: 'top',
target: 'hidden',
timeDelta: 200,
},
{
checkpoint: 'click',
},
],
},
{
id: 'three',
host: 'www.aem.live',
time: '2024-05-06T00:00:04.444Z',
timeSlot: '2024-05-06T00:00:00.000Z',
url: 'https://www.aem.live/home',
userAgent: 'mobile:ios',
weight: 100,
events: [
{
checkpoint: 'top',
target: 'visible',
timeDelta: 200,
},
{
checkpoint: 'viewmedia',
target: 'some_image.png',
},
],
},
],
},
];
const d = new DataChunks();
d.load(chunks);

Expand All @@ -889,4 +898,26 @@ describe('DataChunks.hasConversion', () => {
const notConverted = facets.find((f) => f.value === 'not-converted');
assert.equal(notConverted?.count, 2);
});

it('unknown facet in filter spec', () => {
const d = new DataChunks();
d.load(chunks);

const spec = {
facetOne: ['top'],
facetTwo: ['hidden'],
// trying to filter with an undefined facet
unknowFacet: ['unknown'],
};
d.addFacet('facetOne', (bundle) => bundle.events.map((e) => e.checkpoint));
d.addFacet('facetTwo', (bundle) => bundle.events.map((e) => e.target));
const facetValueFn = (bundle) => (d.hasConversion(bundle, spec) ? 'converted' : 'not-converted');
d.addFacet('conversion', facetValueFn);

const facets = d.facets.conversion;
const converted = facets.find((f) => f.value === 'converted');
assert.equal(converted, undefined);
const notConverted = facets.find((f) => f.value === 'not-converted');
assert.equal(notConverted?.count, 3);
});
});
Loading