Skip to content

Commit

Permalink
fix: unknown filter should return empty set
Browse files Browse the repository at this point in the history
  • Loading branch information
@kptdobe
kptdobe committed Oct 4, 2024
1 parent 07b9c00 commit b569f5e
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 73 deletions.
38 changes: 27 additions & 11 deletions tools/rum/cruncher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,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 @@ -654,15 +659,21 @@ 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 combiner = combinerExtractorFn(attributeName, this);
return desiredValues[combiner]((value) => 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 combiner = combinerExtractorFn(attributeName, this);
return desiredValues[combiner]((value) => actualValues.includes(value));
}));
} catch (error) {
// eslint-disable-next-line no-console
console.warn(`Error while applying filter: ${error.message}`);
return [];
}
}

/**
Expand All @@ -677,7 +688,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
154 changes: 92 additions & 62 deletions tools/rum/test/cruncher.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,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 @@ -673,6 +679,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 @@ -703,69 +710,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 @@ -783,4 +791,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);
});
});

0 comments on commit b569f5e

Please sign in to comment.