fix: correctly detect truly circular paths in JS objects, rather than…

… objects with multiple references

libraries/o-tracking/src/javascript/core/send.js

@@ -1,5 +1,5 @@
 import {get as getSetting} from './settings.js';
-import {broadcast, is, findCircularPathsIn, containsCircularPaths, merge, addEvent, log} from '../utils.js';
+import {broadcast, is, findCircularPaths, merge, addEvent, log} from '../utils.js';
 import {Queue} from './queue.js';
 import {get as getTransport} from './transports/index.js';
 
@@ -68,15 +68,23 @@ function sendRequest(request, callback) {
 	log('user_callback', user_callback);
 	log('PreSend', request);
 
-	if (containsCircularPaths(request)) {
-		const errorMessage = "o-tracking does not support circular references in the analytics data.\n" +
-		"Please remove the circular references in the data.\n" +
-		"Here are the paths in the data which are circular:\n" +
-		JSON.stringify(findCircularPathsIn(request), undefined, 4);
-		throw new Error(errorMessage);
-	}
+	let stringifiedData;
+
+	try {
+		stringifiedData = JSON.stringify(request);
+	} catch (error) {
+		const circularPaths = findCircularPaths(request);
 
-	const stringifiedData = JSON.stringify(request);
+		if (circularPaths) {
+			const errorMessage = "o-tracking does not support circular references in the analytics data.\n" +
+			"Please remove the circular references in the data.\n" +
+			"Here are the paths in the data which are circular:\n" +
+			JSON.stringify(circularPaths, undefined, 4);
+			throw new Error(errorMessage);
+		}
+
+		throw error;
+	}
 
 	transport.complete(function (error) {
 		if (is(user_callback, 'function')) {

libraries/o-tracking/src/javascript/core/store.js

@@ -1,4 +1,4 @@
-import {broadcast, containsCircularPaths, decode, encode, findCircularPathsIn, is} from '../utils.js';
+import {broadcast, findCircularPaths, decode, encode, is} from '../utils.js';
 
 /**
  * Class for storing data
@@ -172,14 +172,21 @@ Store.prototype.write = function (data) {
 	if (typeof this.data === 'string') {
 		value = this.data;
 	} else {
-		if (containsCircularPaths(this.data)) {
-			const errorMessage = "o-tracking does not support circular references in the analytics data.\n" +
-			"Please remove the circular references in the data.\n" +
-			"Here are the paths in the data which are circular:\n" +
-			JSON.stringify(findCircularPathsIn(this.data), undefined, 4);
-			throw new Error(errorMessage);
+		try {
+			value = JSON.stringify(this.data);
+		} catch (error) {
+			const circularPaths = findCircularPaths(request);
+
+			if (circularPaths) {
+				const errorMessage = "o-tracking does not support circular references in the analytics data.\n" +
+				"Please remove the circular references in the data.\n" +
+				"Here are the paths in the data which are circular:\n" +
+				JSON.stringify(circularPaths, undefined, 4);
+				throw new Error(errorMessage);
+			}
+
+			throw error;
 		}
-		value = JSON.stringify(this.data);
 	}
 
 	this.storage.save(this.storageKey, value);

libraries/o-tracking/src/javascript/utils.js

@@ -225,96 +225,66 @@ function assignIfUndefined (subject, target) {
  * Used to find out all the paths which contain a circular reference.
  *
  * @param {*} rootObject The object we want to search within for circular references
- * @returns {string[]} Returns an array of strings, the strings are the full paths to the circular references within the rootObject
+ * @returns {null || string[]} Where there are no circular paths, returns null. Where there are circular paths, returns an array of strings, the strings are the full paths to the circular references within the rootObject
  */
-function findCircularPathsIn(rootObject) {
-	const traversedValues = new WeakSet();
+function findCircularPaths(object) {
+	const traversedValues = new WeakMap();
 	const circularPaths = [];
+	let hasCircularPath = false;
 
-	function _findCircularPathsIn(currentObject, path) {
-		// If we already saw this object
-		// the rootObject contains a circular reference
-		// and we can stop looking any further into this currentObj
-		if (traversedValues.has(currentObject)) {
-			circularPaths.push(path);
-			return;
-		}
+	function jsonReplacer(value, path = "") {
 
-		// Only Objects and things which inherit from Objects can contain circular references
-		// I.E. string/number/boolean/template literals can not contain circular references
-		if (currentObject instanceof Object) {
-			traversedValues.add(currentObject);
-
-			// Loop through all the values of the current object and search those for circular references
-			for (const [key, value] of Object.entries(currentObject)) {
-				// No need to recurse on every value because only things which inherit
-				// from Objects can contain circular references
-				if (value instanceof Object) {
-					const parentObjectIsAnArray = Array.isArray(currentObject);
-					if (parentObjectIsAnArray) {
-					// Store path in bracket notation when value is an array
-						_findCircularPathsIn(value, `${path}[${key}]`);
-					} else {
-					// Store path in dot-notation when value is an object
-						_findCircularPathsIn(value, `${path}.${key}`);
-					}
-				}
-			}
-		}
-	}
+		let oldPath;
+		let modifiedValue;
 
-	_findCircularPathsIn(rootObject, "");
-	return circularPaths;
-}
+		if (
+			typeof value === "object"
+			&& value !== null
+			&& !(value instanceof Boolean)
+			&& !(value instanceof Date)
+			&& !(value instanceof Number)
+			&& !(value instanceof RegExp)
+			&& !(value instanceof String)
+		) {
 
-/**
- * Used to find out whether an object contains a circular reference.
- *
- * @param {object} rootObject The object we want to search within for circular references
- * @returns {boolean} Returns true if a circular reference was found, otherwise returns false
- */
-function containsCircularPaths(rootObject) {
-	// Used to keep track of all the values the rootObject contains
-	const traversedValues = new WeakSet();
-
-	/**
-	 *
-	 * @param {*} currentObject The current object we want to search within for circular references
-	 * @returns {boolean|undefined} Returns true if a circular reference was found, otherwise returns undefined
-	 */
-	function _containsCircularPaths(currentObject) {
-		// If we already saw this object
-		// the rootObject contains a circular reference
-		// and we can stop looking any further
-		if (traversedValues.has(currentObject)) {
-			return true;
-		}
+			oldPath = traversedValues.get(value);
 
-		// Only Objects and things which inherit from Objects can contain circular references
-		// I.E. string/number/boolean/template literals can not contain circular references
-		if (currentObject instanceof Object) {
-			traversedValues.add(currentObject);
-			// Loop through all the values of the current object and search those for circular references
-			for (const value of Object.values(currentObject)) {
-				// No need to recurse on every value because only things which inherit
-				// from Objects can contain circular references
-				if (value instanceof Object) {
-					if (_containsCircularPaths(value)) {
-						return true;
-					}
-				}
+			if (oldPath !== undefined) {
+				circularPaths.push(path);
+				hasCircularPath = true;
+				return { $ref: oldPath };
 			}
+
+			traversedValues.set(value, path);
+
+			if (Array.isArray(value)) {
+				modifiedValue = [];
+				value.forEach(function (element, i) {
+					modifiedValue[i] = jsonReplacer(element, path + "[" + i + "]");
+				});
+			} else {
+				modifiedValue = {};
+				Object.keys(value).forEach(function (name) {
+					modifiedValue[name] = jsonReplacer(
+						value[name],
+						path + "." + name
+					);
+				});
+			}
+			return modifiedValue;
 		}
+
+		return value;
 	}
 
-	// _containsCircularPaths returns true or undefined.
-	// By using Boolean we convert the undefined into false.
-	return Boolean(
-		_containsCircularPaths(
-			rootObject
-		)
-	);
-}
+	jsonReplacer(object);
+
+	if (hasCircularPath) {
+		return circularPaths;
+	}
+
+	return null;
+};
 
 /**
  * Find out whether two objects are deeply equal to each other.
@@ -403,7 +373,6 @@ export {
 	sanitise,
 	assignIfUndefined,
 	filterProperties,
-	findCircularPathsIn,
-	containsCircularPaths,
+	findCircularPaths,
 	isDeepEqual
 };

libraries/o-tracking/test/events/custom.test.js

@@ -129,4 +129,37 @@ Here are the paths in the data which are circular:
 			}
 		}
 	});
+
+	it('should not report a circular reference when multiple references are made to the same object', function () {
+
+		const callback = sinon.spy();
+
+		const contextPartOne = {key: 'part one'};
+		const contextPartTwo = {key: 'part two'};
+
+		const sharedReference = {shared: 'shared'};
+		contextPartOne.sharedReference = sharedReference;
+		contextPartTwo.sharedReference = sharedReference;
+
+		const context = {
+			contextPartOne,
+			contextPartTwo
+		};
+
+		trackEvent(
+			new CustomEvent("oTracking.event", {
+				detail: {
+					category: "slideshow",
+					action: "slide_viewed",
+					slide_number: 5,
+					component_id: "123456",
+					component_name: "custom-o-tracking",
+					context,
+				},
+			}),
+			callback
+		);
+
+		proclaim.ok("Expected function to not to throw an error");
+	});
 });

libraries/o-tracking/test/utils.test.js

@@ -14,8 +14,7 @@ import {
 	sanitise,
 	assignIfUndefined,
 	filterProperties,
-	findCircularPathsIn,
-	containsCircularPaths,
+	findCircularPaths,
 	guid
 } from '../src/javascript/utils.js';
 
@@ -122,25 +121,25 @@ describe('Utils', function () {
 		});
 	});
 
-	describe('findCircularPathsIn', function() {
-		it('should return an empty array if object contains no circular references', function() {
-			proclaim.deepStrictEqual(findCircularPathsIn({ a: 1, b: 2 }), []);
+	describe('findCircularPaths', function() {
+		it('should return null if object contains no circular references', function() {
+			proclaim.isNull(findCircularPaths({ a: 1, b: 2 }));
 		});
 
-		it('should return an empty array if given a string literal', function() {
-			proclaim.deepStrictEqual(findCircularPathsIn(''), []);
+		it('should return null if given a string literal', function() {
+			proclaim.isNull(findCircularPaths(''));
 		});
 
-		it('should return an empty array if given a number literal', function() {
-			proclaim.deepStrictEqual(findCircularPathsIn(137), []);
+		it('should return null if given a number literal', function() {
+			proclaim.isNull(findCircularPaths(137));
 		});
 
-		it('should return an empty array if given a boolean literal', function() {
-			proclaim.deepStrictEqual(findCircularPathsIn(true), []);
+		it('should return null if given a boolean literal', function() {
+			proclaim.isNull(findCircularPaths(true));
 		});
 
-		it('should return an empty array if given null', function() {
-			proclaim.deepStrictEqual(findCircularPathsIn(null), []);
+		it('should return null if given null', function() {
+			proclaim.isNull(findCircularPaths(null));
 		});
 
 		it('should return an array containing all the paths which are circular', function() {
@@ -152,46 +151,12 @@ describe('Utils', function () {
 			rootObject.e = true;
 			rootObject.f = [rootObject.a, 'carrot', rootObject];
 			rootObject.f.push(rootObject.f);
-			proclaim.deepStrictEqual(findCircularPathsIn(rootObject), [
+			proclaim.deepStrictEqual(findCircularPaths(rootObject), [
 				'.a',
 				'.f[0]',
 				'.f[2]',
 				'.f[3]'
 			]);
 		});
-	});
-
-	describe('containsCircularPaths', function() {
-		it('should return false if object contains no circular references', function() {
-			proclaim.isFalse(containsCircularPaths({ a: 1, b: 2 }));
-		});
-
-		it('should return false if given a string literal', function() {
-			proclaim.isFalse(containsCircularPaths(''));
-		});
-
-		it('should return false if given a number literal', function() {
-			proclaim.isFalse(containsCircularPaths(137));
-		});
-
-		it('should return false if given a boolean literal', function() {
-			proclaim.isFalse(containsCircularPaths(true));
-		});
-
-		it('should return false if given null', function() {
-			proclaim.isFalse(containsCircularPaths(null));
-		});
-
-		it('should return true if given an object which contains circular references', function() {
-			const rootObject = {};
-			rootObject.a = rootObject;
-			rootObject.b = 2;
-			rootObject.c = '';
-			rootObject.d = null;
-			rootObject.e = true;
-			rootObject.f = [rootObject.a, 'carrot', rootObject];
-			rootObject.f.push(rootObject.f);
-			proclaim.isTrue(containsCircularPaths(rootObject));
-		});
-	});
+	})
 });