Bypass storage for functions whose TTL is 0 (#32)

* Bypass storage for functions whose TTL is 0

Functions whose TTL is set to 0 will try to retrieve the cached
value from storage, even when the value is never stored in the first
place. We should bypass storage completely, to avoid the performance hit
of the additional lookup.

* Ensure ttl is a positive integer

The async-cache-dedupe library assumes that TTLs are positive integers
greater than 0. If the Cache or function definitions use fractional TTLs
(e.g. `0 < ttl < 1`), it can cause odd behaviors. So this change
introduces additional validation to ensure the values are rejected.

src/cache.js

@@ -19,7 +19,7 @@ class Cache {
       throw new Error('storage is required')
     }
 
-    if (options.ttl && (typeof options.ttl !== 'number' || options.ttl < 0)) {
+    if (options.ttl && (typeof options.ttl !== 'number' || options.ttl < 0 || !Number.isInteger(options.ttl))) {
       throw new Error('ttl must be a positive integer greater than 0')
     }
 
@@ -92,6 +92,10 @@ class Cache {
       throw new TypeError('references must be a function')
     }
 
+    if (opts.ttl && (typeof opts.ttl !== 'number' || opts.ttl < 0 || !Number.isInteger(opts.ttl))) {
+      throw new Error('ttl must be a positive integer greater than 0')
+    }
+
     let storage
     if (opts.storage) {
       storage = createStorage(opts.storage.type, opts.storage.options)
@@ -231,15 +235,17 @@ class Wrapper {
    */
   async wrapFunction (args, key) {
     const storageKey = this.getStorageKey(key)
-    let data = this.storage.get(storageKey)
-    if (data && typeof data.then === 'function') { data = await data }
+    if (this.ttl > 0) {
+      let data = this.storage.get(storageKey)
+      if (data && typeof data.then === 'function') { data = await data }
 
-    if (data !== undefined) {
-      this.onHit(key)
-      return data
-    }
+      if (data !== undefined) {
+        this.onHit(key)
+        return data
+      }
 
-    this.onMiss(key)
+      this.onMiss(key)
+    }
 
     const result = await this.func(args, key)
 

test/cache.test.js

@@ -3,7 +3,7 @@
 const t = require('tap')
 const { Cache } = require('../src/cache')
 const createStorage = require('../src/storage')
-const { kStorages } = require('../src/symbol')
+const { kStorage, kStorages } = require('../src/symbol')
 
 const { test } = t
 
@@ -51,6 +51,24 @@ test('Cache', async (t) => {
         t.equal(err.message, 'fiiii is not defined in the cache')
       })
     })
+
+    test('should bypass storage when ttl is 0', async (t) => {
+      t.plan(1)
+      const cache = new Cache({ storage: createStorage() })
+      cache[kStorage].get = () => {
+        t.fail('should bypass storage')
+      }
+      cache[kStorage].set = () => {
+        t.fail('should bypass storage')
+      }
+      cache.define('f', { ttl: 0 }, async (k) => {
+        t.equal(k, 'foo')
+
+        return { k }
+      })
+
+      await cache.f('foo')
+    })
   })
 
   test('set', async (t) => {

test/ttl.test.js

@@ -32,6 +32,28 @@ test('ttl', async (t) => {
   t.same(await cache.fetchSomething(42), { k: 42 })
 })
 
+test('global ttl is a positive integer', async (t) => {
+  t.plan(1)
+
+  try {
+    // eslint-disable-next-line no-new
+    new Cache({ ttl: 3.14, storage: createStorage() })
+  } catch (err) {
+    t.equal(err.message, 'ttl must be a positive integer greater than 0')
+  }
+})
+
+test('function ttl is a positive integer', async (t) => {
+  t.plan(1)
+
+  const cache = new Cache({ storage: createStorage() })
+  try {
+    cache.define('fetchSomething', { ttl: 3.14 }, async (k) => ({ k }))
+  } catch (err) {
+    t.equal(err.message, 'ttl must be a positive integer greater than 0')
+  }
+})
+
 test('ttl expires', async (t) => {
   t.plan(5)