From 4691f2068a5c48bceffbe1fd4ca86a160786c924 Mon Sep 17 00:00:00 2001 From: Marco Prontera Date: Thu, 3 Aug 2023 09:23:10 +0200 Subject: [PATCH 1/4] perf: switch from BST to a simple Set The BST isn't good to satisfy perf requirement --- .../src/model/PurposeRestrictionVector.ts | 95 ++++++++----------- 1 file changed, 39 insertions(+), 56 deletions(-) diff --git a/modules/core/src/model/PurposeRestrictionVector.ts b/modules/core/src/model/PurposeRestrictionVector.ts index 276ab5e0..36fae8dd 100644 --- a/modules/core/src/model/PurposeRestrictionVector.ts +++ b/modules/core/src/model/PurposeRestrictionVector.ts @@ -1,5 +1,4 @@ import {PurposeRestriction} from './PurposeRestriction.js'; -import {BinarySearchTree} from './BinarySearchTree.js'; import {RestrictionType} from './RestrictionType.js'; import {GVL} from '../GVL.js'; import {Vendor} from './gvl/index.js'; @@ -16,10 +15,8 @@ export class PurposeRestrictionVector extends Cloneable = new Map(); + private map: Map> = new Map>(); private gvl_: GVL; private has(hash: string): boolean { @@ -113,7 +110,7 @@ export class PurposeRestrictionVector extends Cloneable(); - this.map.forEach((bst: BinarySearchTree): void => { + this.map.forEach((vendorIds: Set): void => { - bst.get().forEach((vendorId: number): void => { + Array.from(vendorIds).forEach((vendorId: number): void => { vendorSet.add(vendorId); @@ -222,7 +204,7 @@ export class PurposeRestrictionVector extends Cloneable a - b); } @@ -280,41 +262,42 @@ export class PurposeRestrictionVector extends Cloneable { + this.map.forEach((purposeRestrictionVendorIds: Set): void => { - retr = Math.max(bst.max(), retr); + const vendorIds = Array.from(purposeRestrictionVendorIds); + result = Math.max(vendorIds[vendorIds.length - 1], result); }); - return retr; + return result; } public getRestrictions(vendorId?: number): PurposeRestriction[] { - const retr: PurposeRestriction[] = []; + const result: PurposeRestriction[] = []; - this.map.forEach((bst: BinarySearchTree, hash: string): void => { + this.map.forEach((vendorIds: Set, hash: string): void => { if (vendorId) { - if (bst.contains(vendorId)) { + if (vendorIds.has(vendorId)) { - retr.push(PurposeRestriction.unHash(hash)); + result.push(PurposeRestriction.unHash(hash)); } } else { - retr.push(PurposeRestriction.unHash(hash)); + result.push(PurposeRestriction.unHash(hash)); } }); - return retr; + return result; } @@ -322,7 +305,7 @@ export class PurposeRestrictionVector extends Cloneable(); - this.map.forEach((bst: BinarySearchTree, hash: string): void => { + this.map.forEach((vendorIds: Set, hash: string): void => { purposeIds.add(PurposeRestriction.unHash(hash).purposeId); @@ -342,14 +325,14 @@ export class PurposeRestrictionVector extends Cloneable | undefined = this.map.get(hash); - if (bst) { + if (vendorIds) { - bst.remove(vendorId); + vendorIds.delete(vendorId); // if it's empty let's delete the key so it doesn't show up empty - if (bst.isEmpty()) { + if (vendorIds.size == 0) { this.map.delete(hash); this.bitLength = 0; @@ -377,16 +360,16 @@ export class PurposeRestrictionVector extends Cloneable { + this.map.forEach((vendorIds: Set, hash: string): void => { const purposeRestriction: PurposeRestriction = PurposeRestriction.unHash(hash); - const vendors: number[] = bst.get(); + const vendors: number[] = Array.from(vendorIds); vendors.forEach((vendorId: number): void => { if (!this.isOkToHave(purposeRestriction.restrictionType, purposeRestriction.purposeId, vendorId)) { - bst.remove(vendorId); + vendorIds.delete(vendorId); } From 7817e7b2127c912be4ac5175bf6456d033959ccd Mon Sep 17 00:00:00 2001 From: Marco Prontera Date: Thu, 3 Aug 2023 09:29:10 +0200 Subject: [PATCH 2/4] perf: improve performance and readability of encoding and decoding --- .../field/PurposeRestrictionVectorEncoder.ts | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/modules/core/src/encoder/field/PurposeRestrictionVectorEncoder.ts b/modules/core/src/encoder/field/PurposeRestrictionVectorEncoder.ts index 76dbb63d..d87c96ad 100644 --- a/modules/core/src/encoder/field/PurposeRestrictionVectorEncoder.ts +++ b/modules/core/src/encoder/field/PurposeRestrictionVectorEncoder.ts @@ -14,6 +14,19 @@ export class PurposeRestrictionVectorEncoder { // if the vector is empty we'll just return a string with just the numRestricitons being 0 if (!prVector.isEmpty()) { + const gvlVendorIds = Array.from(prVector.gvl.vendorIds); + + const gvlHasVendorBetween = (vendorId: number, nextVendorId: number): boolean => { + + const firstIndex = gvlVendorIds.indexOf(vendorId); + const nextIndex = gvlVendorIds.indexOf(nextVendorId); + + const res = nextIndex - firstIndex; + + return res > 1; + + }; + // create each restriction group prVector.getRestrictions().forEach((purpRestriction: PurposeRestriction): void => { @@ -44,23 +57,10 @@ export class PurposeRestrictionVectorEncoder { } - // we know that `len` is greater than zero because we entered the loop - const lastVendorId = vendors[len - 1]; - const gvlVendorIds = prVector.gvl.vendorIds; - - const nextGvlVendor = (vendorId: number): number => { - - while (++vendorId <= lastVendorId && !gvlVendorIds.has(vendorId)) { - } - - return vendorId; - - }; - /** * either end of the loop or there are GVL vendor IDs before the next one */ - if (i === len - 1 || vendors[i + 1] > nextGvlVendor(vendorId)) { + if (i === len - 1 || gvlHasVendorBetween(vendorId, vendors[i + 1])) { /** * it's a range entry if we've got something other than the start @@ -141,15 +141,13 @@ export class PurposeRestrictionVectorEncoder { } - for ( let k: number = startOrOnlyVendorId; k <= endVendorId; k++) { - - vector.add(k, purposeRestriction); - - } + // required to preserve the default behavior (includes also vendors ids that doesn't exist) + const vendorIds = Array.from({length: endVendorId - startOrOnlyVendorId + 1}, (_, index) => startOrOnlyVendorId + index); + vector.restrictPurposeToLegalBasis(purposeRestriction, vendorIds); } else { - vector.add(startOrOnlyVendorId, purposeRestriction); + vector.restrictPurposeToLegalBasis(purposeRestriction, [startOrOnlyVendorId]); } From bca7f735ba04dd438a06d4bcf978119b1cd8e2eb Mon Sep 17 00:00:00 2001 From: Marco Prontera Date: Thu, 3 Aug 2023 09:31:47 +0200 Subject: [PATCH 3/4] refactor: delete BinarySearchTree since unused --- modules/core/src/model/BinarySearchTree.ts | 410 ------------------ modules/core/src/model/index.ts | 1 - modules/core/test/Cloneable.test.ts | 19 - .../core/test/model/BinarySearchTree.test.ts | 183 -------- 4 files changed, 613 deletions(-) delete mode 100644 modules/core/src/model/BinarySearchTree.ts delete mode 100644 modules/core/test/Cloneable.test.ts delete mode 100644 modules/core/test/model/BinarySearchTree.test.ts diff --git a/modules/core/src/model/BinarySearchTree.ts b/modules/core/src/model/BinarySearchTree.ts deleted file mode 100644 index 0bb367b7..00000000 --- a/modules/core/src/model/BinarySearchTree.ts +++ /dev/null @@ -1,410 +0,0 @@ -import {Cloneable} from '../Cloneable.js'; - -interface TreeNode { - value: number; - right: TreeNode | null; - left: TreeNode | null; -} - -type TreeNodeMaybe = TreeNode | null; - -export class BinarySearchTree extends Cloneable { - - private root: TreeNodeMaybe = null; - - public getRoot(): TreeNodeMaybe { - - return this.root; - - } - - public isEmpty(): boolean { - - // if root is undefined or null then by definition this is empty - return !(this.root); - - } - - public add(value: number): void { - - // create new node object - const node: TreeNode = { - value: value, - left: null, - right: null, - }; - - let current; - - // first item? - if (this.isEmpty()) { - - this.root = node; - - } else { - - // start at the root - current = this.root; - - // infinite loop, figure out where to put it - while (true) { - - // if the value is less than current value; go left - if (value < current.value) { - - // if it's empty, we can insert - if (current.left === null) { - - // insert on the left - current.left = node; - - // our work is done here - break; - - } else { - - /** - * if there's something there already, we'll reset the pointer and - * wait for the next loop to do something ie. keep traversing - */ - current = current.left; - - } - - } else if (value > current.value) { - - // if the value is greater than our current value; go right - if (current.right === null) { - - // there's nothing to the right, so put it here - current.right = node; - break; - - } else { - - /** - * if there's something there already, we'll reset the pointer and - * wait for the next loop to do something ie. keep traversing - */ - - current = current.right; - - } - - } else { - - /** - * If it's neither greater than the right or less than the right then - * it is equal to the current nodes value. In that case we won't do - * anything with it because we will only insert unique values. - */ - - break; - - } - - } - - } - - } - - /** - * performs Morris in-order traversal - * @return {number[]} sorted array - */ - public get(): number[] { - - const retr: number[] = []; - let current: TreeNodeMaybe = this.root; - - while (current) { - - if (!current.left) { - - retr.push(current.value); // if there is no left child, visit current node - current = current.right; // then we go the right branch - - } else { - - // find the right most leaf of root.left node. - let pre: TreeNodeMaybe = current.left; - - // when pre.right == null, it means we go to the right most leaf - // when pre.right == current, it means the right most leaf has been visited in the last round - while (pre.right && pre.right != current) { - - pre = pre.right; - - } - - // this means the pre.right has been set, it's time to go to current node - if (pre.right == current) { - - pre.right = null; - - // means the current node is pointed by left right most child - // the left branch has been visited, it's time to push the current node - retr.push(current.value); - current = current.right; - - } else { - - // the fist time to visit the pre node, make its right child point to current node - pre.right = current; - current = current.left; - - } - - } - - } - - return retr; - - } - - public contains(value: number): boolean { - - let retr = false; - let current: TreeNodeMaybe = this.root; - - while (current) { - - if (current.value === value) { - - retr = true; - break; - - } else if (value > current.value) { - - current = current.right; - - } else if (value < current.value) { - - current = current.left; - - } - - } - - return retr; - - } - - public min(current: TreeNodeMaybe = this.root): number { - - let retr; - - while (current) { - - if (current.left) { - - current =current.left; - - } else { - - retr = current.value; - current = null; - - } - - } - - return retr; - - } - - public max(current: TreeNodeMaybe = this.root): number { - - let retr; - - while (current) { - - if (current.right) { - - current =current.right; - - } else { - - retr = current.value; - current = null; - - } - - } - - return retr; - - } - - public remove(value: number, current: TreeNodeMaybe = this.root ): void { - - // we start at the root, so the parent is null - let parent: TreeNodeMaybe = null; - let parentSide = 'left'; - - while (current) { - - if (value < current.value) { - - // set our parent to the current value - parent = current; - - // value is less than current value, so go left - current = current.left; - parentSide = 'left'; - - } else if (value > current.value) { - - // set our parent to the current value - parent = current; - - // value is greater than current value, so go right - current = current.right; - parentSide = 'right'; - - } else { - - /** - * if it's neither greater than or less than, then it's equal so BINGO! - * we've found it - * - * If we have children, we've got to figure out what to do with - * them once we are no longer around... Woah, code is like real - * life... - * - * There are three cases we care about when it comes to this removal - * process: - * - * 1. No children -- If not children we just delete an do nothing - * else, no harm no foul. - * - * 2. One child -- Just link the parent's link to current to the - * child. - * - * 3. Two children -- Find the minimum value from the right subtree - * replace us with the minimum value and of course remove that - * minimum value from the right stubtree - */ - - if (!current.left && !current.right) { - - // case 1 there are no children easy peasy lemon squeezy - if (parent) { - - parent[parentSide] = null; - - } else { - - this.root = null; - - } - - } else if (!current.left) { - - // no left side only right, so link right - if (parent) { - - parent[parentSide] = current.right; - - } else { - - this.root = current.right; - - } - - } else if (!current.right) { - - // no right side only left, so link left - if (parent) { - - parent[parentSide] = current.left; - - } else { - - this.root = current.left; - - } - - } else { - - /** - * case 3 just like real life, if you delete a parent the more kids - * that parent has the more complicated things get... in this case we - * have two children. We're gonna have to figure out who goes where. - */ - - const minVal = this.min(current.right); - - // little bit of recursion... - this.remove(minVal, current.right); - current.value = minVal; - - } - - current = null; - - } - - } - - } - - /** - * Build Binary Search Tree from the ordered number array. - * The depth of the tree will be the `log2` of the array length. - * @param {number[]} values number array in ascending order - * @return {BinarySearchTree} Binary Search Tree - */ - static build(values?: number[]): BinarySearchTree | null { - - if (!values || values.length === 0) { - - return null; - - } else if (values.length === 1) { - - const tree = new BinarySearchTree(); - - tree.add(values[0]); - - return tree; - - } else { - - const rootIndex = values.length >> 1; - - const tree = new BinarySearchTree(); - - tree.add(values[rootIndex]); - - const root = tree.getRoot(); - - if (root) { - - if (rootIndex + 1 < values.length) { - - const rightTree = BinarySearchTree.build(values.slice(rootIndex + 1)); - - root.right = rightTree ? rightTree.getRoot() : null; - - } - - if (rootIndex - 1 > 0 ) { - - const leftTree = BinarySearchTree.build(values.slice(0, rootIndex - 1)); - - root.left = leftTree ? leftTree.getRoot(): null; - - } - - } - - return tree; - - } - - } - -} diff --git a/modules/core/src/model/index.ts b/modules/core/src/model/index.ts index 617a2179..a0da8379 100644 --- a/modules/core/src/model/index.ts +++ b/modules/core/src/model/index.ts @@ -1,4 +1,3 @@ -export * from './BinarySearchTree.js'; export * from './ConsentLanguages.js'; export * from './Fields.js'; export * from './IntMap.js'; diff --git a/modules/core/test/Cloneable.test.ts b/modules/core/test/Cloneable.test.ts deleted file mode 100644 index b56cf3c9..00000000 --- a/modules/core/test/Cloneable.test.ts +++ /dev/null @@ -1,19 +0,0 @@ -// import {Cloneable} from '../src/Cloneable'; -import {BinarySearchTree} from '../../core/src/model/BinarySearchTree'; -import {expect} from 'chai'; - -describe('Cloneable', (): void => { - - it('Clone BinarySearchTree', (done: () => void): void => { - - const lastEntry = 4057; - const values = [...Array(lastEntry).keys()].map( (i) => i + 1); - const tree = BinarySearchTree.build(values); - expect(tree).to.be.a('object'); - - expect(tree?.clone()).to.be.a('object'); - done(); - - }); - -}); diff --git a/modules/core/test/model/BinarySearchTree.test.ts b/modules/core/test/model/BinarySearchTree.test.ts deleted file mode 100644 index 69914e69..00000000 --- a/modules/core/test/model/BinarySearchTree.test.ts +++ /dev/null @@ -1,183 +0,0 @@ -import {expect} from 'chai'; -import {BinarySearchTree} from '../../src/model/BinarySearchTree'; - -export function run(): void { - - describe('BinarySearchTree', (): void => { - - const getOrderedArray = (len: number, startNum = 1): number[] => { - - const ar: number[] = []; - - // generate ordered array - for (let i = startNum; i < startNum + len; i++) { - - ar.push(i); - - } - - return ar; - - }; - - const getRandomArray = (len: number, startNum = 1): number[] => { - - const ar: number[] = getOrderedArray(len, startNum); - - // fisher-yates shuffle - for (let i = len - 1; i >= 0; i--) { - - const swapWith: number = Math.round(Math.random() * i); - const temp: number = ar[swapWith]; - - ar[swapWith] = ar[i]; - ar[i] = temp; - - } - - return ar; - - }; - - it('should create an empty tree on instantiation', (): void => { - - const bst = new BinarySearchTree(); - - expect(bst.isEmpty()).to.be.true; - - }); - - it('should build tree from ordered array using static build method', (): void => { - - const numItems = 40; - const values = getOrderedArray(numItems); - const bst = BinarySearchTree.build(values); - - const result = bst?.get(); - - expect(result).to.deep.equal(values); - - }); - - it('should get() sorted array', (): void => { - - const numItems = 40; - const ar: number[] = getRandomArray(numItems); - const bst = new BinarySearchTree(); - - for (let i =0; i < numItems; i++) { - - bst.add(ar[i]); - - } - - const result: number[] = bst.get(); - - // expect the bst get method to return a sorted - expect(result).to.deep.equal(getOrderedArray(numItems)); - - }); - - it('should handle non sequential numbers', (): void => { - - const bst: BinarySearchTree = new BinarySearchTree(); - const randAr: number[] = [5, 9, 7, 2, 3]; - - randAr.forEach((num: number): void => { - - bst.add(num); - - }); - - const expected: number[] = randAr.sort(); - const actual: number[] = bst.get(); - - expect(actual).to.deep.equal(expected); - - }); - - it('should not insert duplicates', (): void => { - - const bst: BinarySearchTree = new BinarySearchTree(); - const randAr: number[] = [1, 2, 2, 3]; - - randAr.forEach((num: number): void => { - - bst.add(num); - - }); - - // de-dupe the array - const arSet: Set = new Set(randAr); - // convert back to array and sort it - const expected: number[] = Array.from(arSet).sort(); - const actual: number[] = bst.get(); - - expect(actual).to.deep.equal(expected); - - }); - - for (let i = 0; i < 12; i++) { - - const numItems = Math.pow(2, i); - const startNum = 1 + Math.round(Math.random() * numItems); - const maxVal = startNum + numItems - 1; - const ar: number[] = getRandomArray(numItems, startNum); - const randomNum: number = ar[Math.floor(Math.random()*ar.length)]; - - const makeTree = (): BinarySearchTree => { - - const bst = new BinarySearchTree(); - - for (let i =0; i < numItems; i++) { - - bst.add(ar[i]); - - } - - return bst; - - }; - - describe(`${numItems} item${numItems === 1 ? '' : 's'}: ${startNum} to ${maxVal}`, (): void => { - - it(`should remove ${randomNum}`, (): void => { - - const bst = makeTree(); - - bst.remove(randomNum); - expect(bst.get()).to.not.include(randomNum); - - }); - - it(`should have a min value of ${startNum}`, (): void => { - - const bst = makeTree(); - - expect(bst.min()).to.equal(startNum); - - }); - - it(`should have a max value of ${maxVal}`, (): void => { - - const bst = makeTree(); - - expect(bst.max()).to.equal(maxVal); - - }); - - it(`should contain ${randomNum}`, (): void => { - - const bst = makeTree(); - - expect(bst.contains(randomNum)).to.be.true; - - }); - - }); - - } - - }); - -} From 0624bc7b6fbef5221426863f6c6763aa550cd504 Mon Sep 17 00:00:00 2001 From: Sergei Sevriugin Date: Mon, 4 Sep 2023 11:27:01 +0200 Subject: [PATCH 4/4] update module version to 1.5.8 for core and cmpapi --- modules/cli/package.json | 2 +- modules/cmpapi/package.json | 4 ++-- modules/core/package.json | 2 +- modules/testing/package.json | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/cli/package.json b/modules/cli/package.json index f5c26fc2..564f1c52 100644 --- a/modules/cli/package.json +++ b/modules/cli/package.json @@ -27,7 +27,7 @@ "lint": "eslint `find src -name '*.ts'`" }, "dependencies": { - "@didomi/iabtcf-core": "1.5.6" + "@didomi/iabtcf-core": "1.5.8" }, "devDependencies": { "@types/node": "^17.0.18", diff --git a/modules/cmpapi/package.json b/modules/cmpapi/package.json index 28e8a651..c9506988 100644 --- a/modules/cmpapi/package.json +++ b/modules/cmpapi/package.json @@ -1,6 +1,6 @@ { "name": "@didomi/iabtcf-cmpapi", - "version": "1.5.6", + "version": "1.5.8", "description": "Ensures other in-page digital marketing technologies have access to CMP transparency and consent information for the iab. Transparency and Consent Framework (TCF).", "author": "Chris Paterson ", "homepage": "https://iabtcf.com/", @@ -28,7 +28,7 @@ "test-cov": "rm -rf coverage; nyc --reporter=html mocha" }, "peerDependencies": { - "@didomi/iabtcf-core": ">=1.0.0" + "@didomi/iabtcf-core": ">=1.5.8" }, "devDependencies": { "@didomi/iabtcf-stub": "1.5.6", diff --git a/modules/core/package.json b/modules/core/package.json index a797c887..ea67894a 100644 --- a/modules/core/package.json +++ b/modules/core/package.json @@ -1,6 +1,6 @@ { "name": "@didomi/iabtcf-core", - "version": "1.5.7", + "version": "1.5.8", "description": "Ensures consistent encoding and decoding of TC Signals for the iab. Transparency and Consent Framework (TCF).", "author": "Chris Paterson ", "homepage": "https://iabtcf.com/", diff --git a/modules/testing/package.json b/modules/testing/package.json index b6923ac3..4cca649d 100644 --- a/modules/testing/package.json +++ b/modules/testing/package.json @@ -26,7 +26,7 @@ "lint": "eslint `find src -name '*.ts'`" }, "peerDependencies": { - "@didomi/iabtcf-core": ">=1.0.0" + "@didomi/iabtcf-core": ">=1.5.8" }, "dependencies": { "@types/sinon": "^10.0.11",