Fix circular reference bug

Language-Research-Technology · Dec 14, 2023 · d9051c9 · d9051c9
1 parent 3729110
commit d9051c9
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 25 deletions.
diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml
@@ -19,7 +19,7 @@ jobs:
         with:
           node-version: 18
           registry-url: 'https://registry.npmjs.org'
-      - uses: ButlerLogic/[email protected].1
+      - uses: ButlerLogic/[email protected].2
         with:
           GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
           tag_prefix: "v"

diff --git a/lib/rocrate.js b/lib/rocrate.js
@@ -174,11 +174,17 @@ class ROCrate {
    * @param {boolean} [opt.merge] If false and if node already exists, remove all existing properties not in the specified data
    * @param {boolean} [opt.recurse] Process nested objects recursively
    * @param {boolean} [opt.add] If true, create an entity even if the data is empty
+   * @param {WeakSet} [opt.seen] A set to keep track of cyclic reference in the input
    * @returns {boolean} Return true if node is changed
    */
-  __updateNode(node, data, { replace = this.config.replace, merge = this.config.merge, recurse, add }) {
+  __updateNode(node, data, { replace = this.config.replace, merge = this.config.merge, recurse, add, seen }) {
     var keys = Object.keys(data).filter(prop => prop !== '@id' && prop !== '@reverse');
     if (!add && !keys.length) return false;
+    if (recurse) {
+      if (!seen) seen = new WeakSet();
+      if (seen.has(data)) return false;
+      seen.add(data);
+    }
     //console.log('node[$size]', node[$size]);
     if (node[$size]) {
       //console.log('replace', replace);
@@ -196,7 +202,7 @@ class ROCrate {
       node[$size] = 1;
     }
     for (const prop of keys) {
-      this.__setProperty(node, prop, data[prop], { merge, replace, recurse });
+      this.__setProperty(node, prop, data[prop], { merge, replace, recurse, seen });
     }
     if (!node['@type']) {
       this.__setProperty(node, '@type', 'Thing', {});
@@ -210,8 +216,20 @@ class ROCrate {
     return true;
   }
 
-  __addValues(ref, prop, oldValues_, values, { duplicate = this.config.duplicate, replace = this.config.replace, merge = this.config.merge, recurse = false }) {
+  __addValues(ref, prop, oldValues_, values, { duplicate = this.config.duplicate, replace = this.config.replace, merge = this.config.merge, recurse = false, seen }) {
     let oldValues = Utils.asArrayRef(oldValues_);
+    const exists = (() => {
+      if (duplicate) {
+        return (() => false);
+      } else {
+        const oldVals = new Set(oldValues.map(v => typeof v === 'object' ? Symbol.for(v['@id']) : v));
+        return function(v) {
+          let key = v;
+          if (typeof v === 'object' && v['@id']) key = Symbol.for(v['@id']);
+          return oldVals.has(key);
+        };
+      }
+    })();
     mapValue(values, v => {
       if (duplicate || !Utils.exists(oldValues, v)) {
         let nv = v;
@@ -222,7 +240,7 @@ class ROCrate {
           if (!node) {
             //let id = v['@id'] || this.uniqueId('#_blank-');
             node = this.__getNode(v['@id']);
-            if (recurse) this.__updateNode(node, v, { replace, merge, recurse });
+            if (recurse) this.__updateNode(node, v, { replace, merge, recurse, seen });
           }
           nv = node[$noderef];
           addReverse(ref, prop, nv);
@@ -243,9 +261,10 @@ class ROCrate {
    * @param {boolean} [opt.replace]
    * @param {boolean} [opt.recurse]
    * @param {boolean} [opt.merge]
+   * @param {WeakSet} [opt.seen]
    */
   // TODO: return false if no change in value
-  __setProperty(entity, prop, values, { duplicate, replace, recurse, merge }) {
+  __setProperty(entity, prop, values, { duplicate, replace, recurse, merge, seen }) {
     //if (values == null) return this.deleteProperty(entity, prop);
     let ref = entity[Symbols.$noderef];
     if (!(prop in entity)) entity[$size]++;
@@ -255,7 +274,7 @@ class ROCrate {
         let newIds = new Set(mapValue(values, v => v['@id']));
         mapValue(entity[prop], v => !newIds.has(v['@id']) && removeReverse(ref, prop, v));
       }
-      entity[prop] = this.__addValues(ref, prop, [], values, { duplicate, replace, recurse, merge }) ?? [];
+      entity[prop] = this.__addValues(ref, prop, [], values, { duplicate, replace, recurse, merge, seen }) ?? [];
     } else if (prop in entity) {
       this.deleteProperty(entity, prop);
     }

diff --git a/lib/utils.js b/lib/utils.js
@@ -161,7 +161,7 @@ function isDeepEqual(a, b, objectEquals) {
       if (Array.isArray(a) && Array.isArray(b)) {
         if (a.length === b.length) {
           for (let i = 0; i < a.length; ++i) {
-            if (!isDeepEqual(a[i], b[i])) return false;
+            if (!isDeepEqual(a[i], b[i], objectEquals)) return false;
           }
           return true;
         }
@@ -182,7 +182,7 @@ function isDeepEqual(a, b, objectEquals) {
         if (typeof ia === 'function' && typeof ib === 'function') {
           let ra = ia.next(), rb = ib.next();
           while (ra && rb && (!ra.done || !rb.done)) {
-            if (!isDeepEqual(ra.value, rb.value)) return false;
+            if (!isDeepEqual(ra.value, rb.value, objectEquals)) return false;
             ra = ia.next(), rb = ib.next();
           }
         }

diff --git a/test/rocrate.new.spec.js b/test/rocrate.new.spec.js
@@ -102,7 +102,7 @@ describe("entities", function () {
   });
   it("can iterate filtered entities", function () {
     let crate = new ROCrate();
-    let result = Array.from(crate.entities({filter: {'@type': /^Dataset$/}}));
+    let result = Array.from(crate.entities({ filter: { '@type': /^Dataset$/ } }));
     assert.equal(result.length, 1);
     assert.equal(result[0], crate.rootDataset);
   });
@@ -143,7 +143,7 @@ describe("addEntity", function () {
     let e = crate.getEntity('abc');
     assert(e);
     assert.strictEqual(e.name, 'abc');
-    crate.addEntity({ '@id': 'abc' }, {replace: true});
+    crate.addEntity({ '@id': 'abc' }, { replace: true });
     assert(!e.name);
   });
   it("can set default @type", function () {
@@ -200,10 +200,10 @@ describe("updateEntity", function () {
         '@id': '[email protected]', contactType: 'general'
       }
     };
-    crate.updateEntity(e, {merge:true});
+    crate.updateEntity(e, { merge: true });
     assert.strictEqual(crate.getEntity('https://orcid.org/0000')?.name, 'test0');
     assert.strictEqual(crate.getEntity('[email protected]')?.contactType, 'support');
-    crate.updateEntity(e, {merge:true, recurse:true});
+    crate.updateEntity(e, { merge: true, recurse: true });
     assert.strictEqual(crate.getEntity('[email protected]')?.contactType, 'general');
   });
 });
@@ -272,14 +272,14 @@ describe("addValues", function () {
     let crate = new ROCrate(testData);
     let e = crate.getEntity('./');
     assert.strictEqual(e.author['@id'], "https://orcid.org/0000");
-    crate.addValues(e, "author", { "@id": "https://orcid.org/0000" }, {duplicate:true});
+    crate.addValues(e, "author", { "@id": "https://orcid.org/0000" }, { duplicate: true });
     assert.strictEqual(e.author.length, 2);
     assert.strictEqual(e.author[0]['@id'], "https://orcid.org/0000");
     assert.strictEqual(e.author[1]['@id'], "https://orcid.org/0000");
-    crate.addValues(e, "keywords", "Test", {duplicate:true});
+    crate.addValues(e, "keywords", "Test", { duplicate: true });
     assert.strictEqual(e.keywords[1], "Test");
-    crate.addValues(e, "keywords", ["Test"], {duplicate:true});
-    crate.addValues(e, "keywords", ["Test", "Test"], {duplicate:true});
+    crate.addValues(e, "keywords", ["Test"], { duplicate: true });
+    crate.addValues(e, "keywords", ["Test", "Test"], { duplicate: true });
     assert.strictEqual(e.keywords.length, 5);
     assert.strictEqual(e.keywords[2], "Test");
     assert.strictEqual(e.keywords[3], "Test");
@@ -344,7 +344,7 @@ describe("addValues", function () {
     var author2 = {
       '@id': '#john',
       name: 'john',
-      contactPoint: {'@id': '[email protected]'}
+      contactPoint: { '@id': '[email protected]' }
     };
     crate.addValues(root, 'author', author2);
     assert.equal(crate.getEntity('[email protected]')?.email, newAuthor.contactPoint.email);
@@ -398,12 +398,32 @@ describe("addValues", function () {
     assert(!crate.getEntity('https://specs.frictionlessdata.io/table-schema/'));
     assert.equal(crate.graphSize, count + 1);
   });
+  it("can handle circular references in the input", function () {
+    const crate = new ROCrate(testData, {link: true, array: true});
+    const root = crate.rootDataset.toJSON();
+    const file = {
+      "@id": "BCNT_anon/elan/Bcnt_AEF_032_Camila.eaf",
+      "speaker": "Camila",
+      "@type": [
+        "File",
+        "Annotation"
+      ],
+      "partOf": root,
+      "annotationOf": {
+        "@id": "Bcnt_AEF_032_Camila.wav"
+      },
+      "encodingFormat": []
+    };
+    root.hasPart = [file];
+    crate.addValues('./', 'hasPart', file);
+  });
+
   //TODO: it can add an entity already exist in the values and update the nested data
 });
 
 describe("getProperty", function () {
   it("can update array", function () {
-    let crate = new ROCrate(testData, {array: true});
+    let crate = new ROCrate(testData, { array: true });
     let root = crate.rootDataset;
     let keywords = root.keywords;
     assert.strictEqual(keywords[0], "Test");
@@ -495,7 +515,7 @@ describe("setProperty", function () {
     r.license.push('test licence 2');
     assert.strictEqual(r.license[1], 'test licence');
     assert.strictEqual(r.license[2], 'test licence 2');
-  
+
   });
   it("can remove duplicates from the array", function () {
     let crate = new ROCrate(testData, { array: true });
@@ -511,21 +531,21 @@ describe("setProperty", function () {
     assert.strictEqual(r.license.length, 2);
     r.license.push('test licence 2');
     assert.strictEqual(r.license.length, 2);
-  
+
   });
-  it("can replace existing entities", function() {
+  it("can replace existing entities", function () {
     let crate = new ROCrate(testData, { link: true, replace: true });
     let e = crate.getEntity('https://orcid.org/0000');
     assert.ok(e);
     assert.strictEqual(e.contactPoint.email, "[email protected]");
     // ref only, don't replace 
-    crate.rootDataset.author = {'@id': 'https://orcid.org/0000'}
+    crate.rootDataset.author = { '@id': 'https://orcid.org/0000' }
     let auth = crate.getEntity('https://orcid.org/0000');
     assert.strictEqual(auth.name, "John Doe");
     assert.strictEqual(auth.contactPoint.email, "[email protected]");
     // replace here
     crate.rootDataset.author = {
-      '@id': 'https://orcid.org/0000', 
+      '@id': 'https://orcid.org/0000',
       '@type': 'Person',
       name: 'Jane Doe'
     };
@@ -643,7 +663,7 @@ describe("getTerm", function () {
     const crate = new ROCrate();
     await crate.resolveContext();
     assert.equal(crate.getTerm('http://schema.org/Place'), "Place");
-  })  
+  })
 })
 describe("resolveTerm", function () {
   const crate = new ROCrate();