SYSTEST-9586 - Enhanced $ref Replacement and Self-Reference Check in …

…Schema Dereferencing (#147)

* Modify dereference logic to loop to a maximum depth of 3

* Alter de-referencing logic

* Update unit tests

* remove maxDereferenceDepth

* v1.0.2

CHANGELOG.md

@@ -2,6 +2,28 @@
 
 All notable changes to this project will be documented in this file.
 
+## [1.0.2] - 2023-11-10
+
+### Added
+
+* New unit tests to ensure the reliability of schema dereferencing functions in `fireboltOpenRpcDereferencing.mjs`.
+
+### Changed
+
+* Refined `selfReferenceSchemaCheck()` to return a boolean consistently, enhancing readability and predictability of self-referencing detection logic.
+
+* Overhauled `replaceRefs()` for improved efficiency:
+
+    * Implemented a set to track replaced $refs, thereby preventing infinite recursion during ref replacement.
+
+    * Expanded the function to handle nested objects and arrays more effectively.
+
+* Modernized various parts of `fireboltOpenRpcDereferencing.mjs` with arrow functions for a more modern code syntax.
+
+### Fixed
+
+* Addressed potential issues with recursive calls and infinite loops in `replaceRefs()` by marking self-referenced schemas as replaced and removing redundant $ref keys.
+
 ## [1.0.1] - 2023-10-18
 
 ### Added

cli/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@firebolt-js/mock-firebolt-cli",
-  "version": "1.0.1",
+  "version": "1.0.2",
   "description": "Command-line interface for controlling the Mock Firebolt server",
   "main": "./src/cli.mjs",
   "scripts": {},

conduit/package.json

@@ -1,7 +1,7 @@
 {
   "name": "conduit",
   "description": "A TV app that works in concert with Mock Firebolt in Reverse Proxy mode",
-  "version": "1.0.1",
+  "version": "1.0.2",
   "private": true,
   "license": "Apache 2.0",
   "author": "Mike Fine <[email protected]>",

functional/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@firebolt-js/mock-firebolt-functional-tests",
-  "version": "1.0.1",
+  "version": "1.0.2",
   "description": "Command-line interface for controlling the Mock Firebolt server",
   "scripts": {
     "test": "NODE_ENV=test npx jest --config=jest.config.js --silent -i",

server/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@firebolt-js/mock-firebolt",
-  "version": "1.0.1",
+  "version": "1.0.2",
   "description": "Controllable mock Firebolt server",
   "main": "./build/index.mjs",
   "scripts": {

server/src/fireboltOpenRpcDereferencing.mjs

@@ -49,80 +49,101 @@ function replaceRefArr(arrWithItemWithRef, posInArrWithRef, lookedUpSchema) {
   arrWithItemWithRef[posInArrWithRef] = lookedUpSchema;
 }
 
-// to check for self-referencing schema objects that can otherwise lead to recursive loops causing maximum call stack size to exceed
+/**
+ * Recursively searches through the provided schema object to detect self-referencing schemas by comparing $ref values against a provided path.
+ * 
+ * @param {object} schemaObj - The schema object to be checked for self-referencing.
+ * @param {string} path - The reference path to compare against for self-referencing.
+ * @returns {boolean} Returns true if a self-reference is detected, false otherwise.
+ */
 function selfReferenceSchemaCheck(schemaObj, path) {
   if (typeof schemaObj !== 'object' || schemaObj === null) {
-    return null
+    return false;
   }
   // check if self-refernce is present in current schema object
   if ('$ref' in schemaObj && schemaObj['$ref'] == path) {
-    return true
+    return true;
   }
   // check if self-reference is present in any nested objects
   for (const key in schemaObj) {
     const refValue = selfReferenceSchemaCheck(schemaObj[key], path);
-    if (refValue !== null) {
-      return true
+    if (refValue === true) {
+      return true;
     }
   }
-  return null
+  return false;
 }
 
-// NOTE: Doesn't handle arrays of arrays
-function replaceRefs(metaForSdk, thing, key) {
-  let xSchema, lookedUpSchema, selfReference = false
+/**
+ * Recursively replaces $ref keys in an object with their corresponding schemas to resolve references.
+ * It avoids infinite recursion by tracking replaced references using a set. This function handles nested
+ * objects and arrays but does not handle arrays of arrays. It also accounts for schemas defined under
+ * "x-schemas" and "components" in the OpenRPC.
+ * 
+ * @param {object} metaForSdk - The metadata object which may contain the schemas for replacement.
+ * @param {object|array} thing - The object or array where the replacement should occur.
+ * @param {string|number} key - The key in the 'thing' object that needs to be checked and potentially replaced.
+ * @param {Set} [replacedRefs] - A set to keep track of replaced references to prevent recursion. Defaults to a new Set.
+ * @returns {void} This function does not return a value. It mutates the 'thing' object by reference.
+ */
+function replaceRefs(metaForSdk, thing, key, replacedRefs = new Set()) {
+  let xSchema, lookedUpSchema;
+
   if (isObject(thing[key])) {
-    if ('$ref' in thing[key]) {
-      // If schema resides under x-schemas object in openRPC
-      if (thing[key]['$ref'].includes("x-schemas")) {
-        let xSchemaArray = thing[key]['$ref'].split("/");
-        xSchema = xSchemaArray.filter(element => element !== "#" && element !== "x-schemas")[0]
-        lookedUpSchema = lookupSchema(metaForSdk, ref2schemaName(thing[key]['$ref']), xSchema);
+    const refKey = thing[key]['$ref'];
+
+    if (refKey) {
+      // Check if this reference was already replaced to prevent recursion.
+      if (replacedRefs.has(refKey)) {
+        // If already replaced, remove the $ref to avoid recursion.
+        delete thing[key]['$ref'];
       } else {
-        // else if schema resides under components object in openRPC
-        lookedUpSchema = lookupSchema(metaForSdk, ref2schemaName(thing[key]['$ref']));
-      }
-      if (lookedUpSchema) {
-        if (selfReferenceSchemaCheck(lookedUpSchema, thing[key]['$ref']) == true) {
-          selfReference = true
+        // If schema resides under x-schemas object in openRPC.
+        if (refKey.includes("x-schemas")) {
+          let xSchemaArray = refKey.split("/");
+          xSchema = xSchemaArray.filter(element => element !== "#" && element !== "x-schemas")[0];
+          lookedUpSchema = lookupSchema(metaForSdk, ref2schemaName(refKey), xSchema);
+        } else {
+          // Else if schema resides under components object in openRPC.
+          lookedUpSchema = lookupSchema(metaForSdk, ref2schemaName(refKey));
         }
-      }
-      // replace reference path with the corresponding schema object
-      replaceRefObj(thing, key, lookedUpSchema);
-    } if (selfReference !== true) {
-      // if no self-referencing detected, recursively replace references in nested schema objects, else skip dereferencing the schema object further to avoid infinite loop
-      for (const key2 in thing[key]) {
-        replaceRefs(metaForSdk, thing[key], key2);
+
+        if (lookedUpSchema && selfReferenceSchemaCheck(lookedUpSchema, refKey)) {
+          // If it's a self-reference, mark it as replaced.
+          replacedRefs.add(refKey);
+        }
+
+        // Replace the reference with the actual schema.
+        thing[key] = lookedUpSchema || thing[key];
       }
     }
+
+    // Recursively call replaceRefs on nested objects, passing the replacedRefs set forward.
+    Object.keys(thing[key]).forEach((nestedKey) => {
+      replaceRefs(metaForSdk, thing[key], nestedKey, replacedRefs);
+    });
   } else if (isArray(thing[key])) {
-    for (let idx = 0; ii < thing[key].length; idx += 1) {
-      if (isObject(thing[key][idx])) {
-        if ('$ref' in thing[idx]) {
-          lookedUpSchema = lookupSchema(metaForSdk, ref2schemaName(thing[key][idx]['$ref']));
-          replaceRefArr(thing[key], idx, lookedUpSchema);
-        }
+    thing[key].forEach((item, idx) => {
+      if (isObject(item)) {
+        replaceRefs(metaForSdk, thing[key], idx, replacedRefs);
       }
-    }
+    });
   }
 }
 
 function dereferenceSchemas(metaForSdk, methodName) {
-  let newSchema;
   const methods = metaForSdk.methods;
-  const matchMethods = methods.filter(function(method) { return method.name === methodName; });
+  const matchMethods = methods.filter((method) => method.name === methodName);
   const matchMethod = matchMethods[0];
-  const result = matchMethod.result;
-  //replaceRefs(metaForSdk, result, 'schema');
+
   replaceRefs(metaForSdk, matchMethod, 'result');
   replaceRefs(metaForSdk, matchMethod, 'params');
   replaceRefs(metaForSdk, matchMethod, 'tags');
-
 }
 
 function dereferenceMeta(_meta) {
   const meta = JSON.parse(JSON.stringify(_meta)); // Deep copy
-  config.dotConfig.supportedOpenRPCs.forEach(function(oSdk) {
+  config.dotConfig.supportedOpenRPCs.forEach((oSdk) => {
     const sdkName = oSdk.name;
     if ( sdkName in meta ) {
       const metaForSdk = meta[sdkName];
@@ -137,11 +158,16 @@ function dereferenceMeta(_meta) {
   return meta;
 }
 
-
 // --- Exports ---
-
 export const testExports = {
-  replaceRefArr
+  replaceRefArr,
+  replaceRefObj,
+  isObject,
+  isArray,
+  ref2schemaName,
+  lookupSchema,
+  selfReferenceSchemaCheck,
+  replaceRefs
 };
 
 export {