Issue 7285 - Abstract caching logic

lib/caching.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 11 matching lines @@
  */
 class Cache
 {
   /**
    * Creates a cache.
    * @param {number} capacity The maximum number of entries that can exist in
    *   the cache.
    */
   constructor(capacity)
   {
-    if (isNaN(capacity))
-      throw new Error("capacity must be a number.");
+    // Note: This check works for non-number values.
+    if (!(capacity >= 1))
+      throw new Error("capacity must be a positive number.");
 
     this._capacity = capacity;
     this._storage = new Map();
   }
 
   /**
    * Reads an entry from the cache.
    * @param {?*} key The key for the entry.
    * @returns {?*} The value of the entry, or <code>undefined</code> if the
    *   entry doesn't exist in the cache.
    */
-  read(key)

[hub, 2019/02/14 20:43:00]

I think I would still have names these methods `get` and `set`.

[Manish Jethani, 2019/02/16 12:17:17]

Done.

+  get(key)
   {
     return this._storage.get(key);
   }
 
   /**
-   * Saves an entry into the cache. If the cache has reached the specified
+   * Writes an entry to the cache. If the cache has reached the specified
    * maximum number of entries, all the old entries are cleared first.
    * @param {?*} key The key for the entry.
    * @param {?*} value The value of the entry.
    */
-  save(key, value)
+  set(key, value)
   {
     // To prevent logical errors, neither key nor value is allowed to be
     // undefined.
     if (typeof key == "undefined")
       throw new Error("key must not be undefined.");
 
     if (typeof value == "undefined")
       throw new Error("value must not be undefined.");
 
-    if (this._storage.size >= this._capacity)
+    if (this._storage.size == this._capacity && !this._storage.has(key))

[Manish Jethani, 2019/02/16 12:17:17]

This is slightly less efficient but more correct (i.e. an existing entry should
not allow the cache to get cleared). In practice it would not matter, but since
we are writing tests and making the object reusable in other places, it should
have good semantics.

[hub, 2019/02/18 13:36:51]

Acknowledged.

       this._storage.clear();
 
     this._storage.set(key, value);
   }
 
   /**
    * Clears the cache.
    */
   clear()
   {
     this._storage.clear();
   }
 }
 
 exports.Cache = Cache;