Merge pull request #12089 from timvandermeij/refsetcache

Convert `RefSetCache` to a proper class and to use a `Map` internally
mozilla · Jul 17, 2020 · c55122c · c55122c
2 parents a604973 + b19a179
commit c55122c
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 32 deletions.
diff --git a/src/core/primitives.js b/src/core/primitives.js
@@ -239,46 +239,41 @@ class RefSet {
   }
 }
 
-var RefSetCache = (function RefSetCacheClosure() {
-  // eslint-disable-next-line no-shadow
-  function RefSetCache() {
-    this.dict = Object.create(null);
+class RefSetCache {
+  constructor() {
+    this._map = new Map();
   }
 
-  RefSetCache.prototype = {
-    get size() {
-      return Object.keys(this.dict).length;
-    },
-
-    get: function RefSetCache_get(ref) {
-      return this.dict[ref.toString()];
-    },
+  get size() {
+    return this._map.size;
+  }
 
-    has: function RefSetCache_has(ref) {
-      return ref.toString() in this.dict;
-    },
+  get(ref) {
+    return this._map.get(ref.toString());
+  }
 
-    put: function RefSetCache_put(ref, obj) {
-      this.dict[ref.toString()] = obj;
-    },
+  has(ref) {
+    return this._map.has(ref.toString());
+  }
 
-    putAlias: function RefSetCache_putAlias(ref, aliasRef) {
-      this.dict[ref.toString()] = this.get(aliasRef);
-    },
+  put(ref, obj) {
+    this._map.set(ref.toString(), obj);
+  }
 
-    forEach: function RefSetCache_forEach(callback) {
-      for (const i in this.dict) {
-        callback(this.dict[i]);
-      }
-    },
+  putAlias(ref, aliasRef) {
+    this._map.set(ref.toString(), this.get(aliasRef));
+  }
 
-    clear: function RefSetCache_clear() {
-      this.dict = Object.create(null);
-    },
-  };
+  forEach(callback) {
+    for (const value of this._map.values()) {
+      callback(value);
+    }
+  }
 
-  return RefSetCache;
-})();
+  clear() {
+    this._map.clear();
+  }
+}
 
 function isEOF(v) {
   return v === EOF;

diff --git a/test/unit/primitives_spec.js b/test/unit/primitives_spec.js
@@ -27,6 +27,7 @@ import {
   Name,
   Ref,
   RefSet,
+  RefSetCache,
 } from "../../src/core/primitives.js";
 import { StringStream } from "../../src/core/stream.js";
 import { XRefMock } from "./test_utils.js";
@@ -336,6 +337,64 @@ describe("primitives", function () {
     });
   });
 
+  describe("RefSetCache", function () {
+    const ref1 = Ref.get(4, 2);
+    const ref2 = Ref.get(5, 2);
+    const obj1 = Name.get("foo");
+    const obj2 = Name.get("bar");
+    let cache;
+
+    beforeEach(function (done) {
+      cache = new RefSetCache();
+      done();
+    });
+
+    afterEach(function () {
+      cache = null;
+    });
+
+    it("should put, have and get a value", function () {
+      cache.put(ref1, obj1);
+      expect(cache.has(ref1)).toBeTruthy();
+      expect(cache.has(ref2)).toBeFalsy();
+      expect(cache.get(ref1)).toBe(obj1);
+    });
+
+    it("should put, have and get a value by alias", function () {
+      cache.put(ref1, obj1);
+      cache.putAlias(ref2, ref1);
+      expect(cache.has(ref1)).toBeTruthy();
+      expect(cache.has(ref2)).toBeTruthy();
+      expect(cache.get(ref1)).toBe(obj1);
+      expect(cache.get(ref2)).toBe(obj1);
+    });
+
+    it("should report the size of the cache", function () {
+      cache.put(ref1, obj1);
+      expect(cache.size).toEqual(1);
+      cache.put(ref2, obj2);
+      expect(cache.size).toEqual(2);
+    });
+
+    it("should clear the cache", function () {
+      cache.put(ref1, obj1);
+      expect(cache.size).toEqual(1);
+      cache.clear();
+      expect(cache.size).toEqual(0);
+    });
+
+    it("should support iteration", function () {
+      cache.put(ref1, obj1);
+      cache.put(ref2, obj2);
+
+      const values = [];
+      cache.forEach(function (value) {
+        values.push(value);
+      });
+      expect(values).toEqual([obj1, obj2]);
+    });
+  });
+
   describe("isEOF", function () {
     it("handles non-EOF", function () {
       const nonEOF = "foo";