From c8c10da4661e8a9342457e07cfb8be601ff2b5c5 Mon Sep 17 00:00:00 2001
From: Kiko Beats <josefrancisco.verdu@gmail.com>
Date: Fri, 12 Jan 2024 12:25:55 +0100
Subject: [PATCH] fix: strict typecheck

---
 src/keys.js   | 16 ++++++++-----
 src/plans.js  | 16 ++++++++-----
 src/util.js   |  2 +-
 test/keys.js  | 66 +++++++++++++++++++++++++++++++++++++++------------
 test/plans.js | 33 +++++++++++++++++++-------
 5 files changed, 96 insertions(+), 37 deletions(-)

diff --git a/src/keys.js b/src/keys.js
index c738f42..5037d6d 100644
--- a/src/keys.js
+++ b/src/keys.js
@@ -39,14 +39,16 @@ export default ({ serialize, deserialize, plans, redis } = {}) => {
    * @param {boolean} [options.validate=true] - Validate if the plan id is valid.
    * @param {boolean} [options.throwError=false] - Throw an error if the plan does not exist.
    *
-   * @returns {Object} The key.
+   * @returns {Object|null} The key object, null if it doesn't exist.
    */
   const retrieve = async (
     keyId,
     { throwError = false, validate = true } = {}
   ) => {
     const key = await redis.get(getKey(keyId, { validate }))
-    if (key === null && throwError) { throw new TypeError(`The key \`${keyId}\` does not exist.`) }
+    if (key === null && throwError) {
+      throw new TypeError(`The key \`${keyId}\` does not exist.`)
+    }
     return deserialize(key)
   }
 
@@ -60,8 +62,11 @@ export default ({ serialize, deserialize, plans, redis } = {}) => {
   const del = async keyId => {
     const key = await retrieve(keyId, { verify: true })
 
-    if (key !== null && key.plan) {
-      const plan = await plans.retrieve(key.plan, { throwError: true, validate: false })
+    if (key !== null && typeof key.plan === 'string') {
+      const plan = await plans.retrieve(key.plan, {
+        throwError: true,
+        validate: false
+      })
       if (plan !== null) {
         throw new TypeError(
           `The key \`${keyId}\` is associated with the plan \`${getKey.plan}\``
@@ -69,7 +74,7 @@ export default ({ serialize, deserialize, plans, redis } = {}) => {
       }
     }
 
-    const isDeleted = Boolean(await redis.del(getKey(keyId, { verify: true })))
+    const isDeleted = (await redis.del(getKey(keyId, { verify: true }))) === 1
     if (!isDeleted) throw new TypeError(`The key \`${keyId}\` does not exist.`)
     return isDeleted
   }
@@ -89,7 +94,6 @@ export default ({ serialize, deserialize, plans, redis } = {}) => {
    */
   const update = async (keyId, opts) => {
     const currentKey = await retrieve(keyId, { throwError: true })
-    if (!currentKey) throw new TypeError(`The key \`${keyId}\` does not exist.`)
     const metadata = Object.assign({}, currentKey.metadata, opts.metadata)
     const key = Object.assign(currentKey, pick(opts, KEY_FIELDS), {
       updatedAt: Date.now()
diff --git a/src/plans.js b/src/plans.js
index 6838875..2be5c6a 100644
--- a/src/plans.js
+++ b/src/plans.js
@@ -1,5 +1,3 @@
-'use strict'
-
 import { pick, uid, validateKey } from './util.js'
 
 export const PLAN_PREFIX = 'plan_'
@@ -22,7 +20,7 @@ export default ({ serialize, deserialize, redis } = {}) => {
    * @param {number} [options.throttle.rateLimit] - The rate limit of the plan.
    * @param {Object} [options.metadata] - Any extra information can be attached here.
    *
-   * @returns {Object} The created plan.
+   * @returns {Object|null} The plan object, null if it doesn't exist.
    */
   const create = async (opts = {}) => {
     if (!opts.name) throw TypeError('The argument `name` is required.')
@@ -58,7 +56,9 @@ export default ({ serialize, deserialize, redis } = {}) => {
     { throwError = false, validate = true } = {}
   ) => {
     const plan = await redis.get(getKey(planId, { validate }))
-    if (plan === null && throwError) { throw new TypeError(`The plan \`${planId}\` does not exist.`) }
+    if (plan === null && throwError) {
+      throw new TypeError(`The plan \`${planId}\` does not exist.`)
+    }
     return deserialize(plan)
   }
 
@@ -71,8 +71,12 @@ export default ({ serialize, deserialize, redis } = {}) => {
    * @returns {boolean} Whether the plan was deleted or not.
    */
   const del = async planId => {
-    const isDeleted = Boolean(await redis.del(getKey(planId, { validate: true })))
-    if (!isDeleted) throw new TypeError(`The plan \`${planId}\` does not exist.`)
+    const isDeleted = Boolean(
+      await redis.del(getKey(planId, { validate: true }))
+    )
+    if (!isDeleted) {
+      throw new TypeError(`The plan \`${planId}\` does not exist.`)
+    }
     return isDeleted
   }
 
diff --git a/src/util.js b/src/util.js
index 0f11b40..9789c23 100644
--- a/src/util.js
+++ b/src/util.js
@@ -25,6 +25,6 @@ export const validateKey =
   ({ prefix }) =>
     (id, { validate = true } = {}) => {
       if (!validate) return id
-      if (!id.startsWith(prefix)) throw new TypeError(`The id \`${id}\` must to start with \`${prefix}\`.`)
+      if (!String(id).startsWith(prefix)) { throw new TypeError(`The id \`${id}\` must to start with \`${prefix}\`.`) }
       return id
     }
diff --git a/test/keys.js b/test/keys.js
index d41e460..71d2315 100644
--- a/test/keys.js
+++ b/test/keys.js
@@ -1,5 +1,3 @@
-'use strict'
-
 import { setTimeout } from 'timers/promises'
 import openkey from 'openkey'
 import Redis from 'ioredis'
@@ -24,8 +22,19 @@ test('.create # `name` is required', async t => {
   t.is(error.name, 'TypeError')
 })
 
+test('.create # error if plan is invalid', async t => {
+  const error = await t.throwsAsync(
+    keys.create({ name: 'hello@microlink.io', plan: 123 })
+  )
+
+  t.is(error.message, 'The id `123` must to start with `plan_`.')
+  t.is(error.name, 'TypeError')
+})
+
 test('.create # error if plan does not exist', async t => {
-  const error = await t.throwsAsync(keys.create({ name: 'hello@microlink.io', plan: 'plan_123' }))
+  const error = await t.throwsAsync(
+    keys.create({ name: 'hello@microlink.io', plan: 'plan_123' })
+  )
 
   t.is(error.message, 'The plan `plan_123` does not exist.')
   t.is(error.name, 'TypeError')
@@ -55,7 +64,9 @@ test('.retrieve', async t => {
 })
 
 test('.update', async t => {
-  const { id, value, createdAt } = await keys.create({ name: 'hello@microlink.io' })
+  const { id, value, createdAt } = await keys.create({
+    name: 'hello@microlink.io'
+  })
 
   await setTimeout(0) // ensure time move forward
 
@@ -77,16 +88,31 @@ test('.update', async t => {
   t.deepEqual(await keys.retrieve(id), { ...key, updatedAt })
 })
 
-test('.update # error if plan does not exist', async t => {
+test('.update # error if plan is invalid', async t => {
   const { id } = await keys.create({ name: 'hello@microlink.io' })
 
-  await setTimeout(0) // ensure time move forward
+  const error = await t.throwsAsync(
+    keys.update(id, {
+      description: 'new description',
+      enabled: false,
+      plan: 123
+    })
+  )
 
-  const error = await t.throwsAsync(keys.update(id, {
-    description: 'new description',
-    enabled: false,
-    plan: 'plan_123'
-  }))
+  t.is(error.message, 'The id `123` must to start with `plan_`.')
+  t.is(error.name, 'TypeError')
+})
+
+test('.update # error if plan does not exist', async t => {
+  const { id } = await keys.create({ name: 'hello@microlink.io' })
+
+  const error = await t.throwsAsync(
+    keys.update(id, {
+      description: 'new description',
+      enabled: false,
+      plan: 'plan_123'
+    })
+  )
 
   t.is(error.message, 'The plan `plan_123` does not exist.')
   t.is(error.name, 'TypeError')
@@ -142,10 +168,17 @@ test.serial('.list', async t => {
 })
 
 test('.del', async t => {
-  const { id } = await keys.create({ name: 'hello@microlink.io' })
+  {
+    const { id } = await keys.create({ name: 'hello@microlink.io' })
 
-  t.true(await keys.del(id))
-  t.is(await keys.retrieve(id), null)
+    t.true(await keys.del(id))
+    t.is(await keys.retrieve(id), null)
+  }
+  {
+    const { id } = await keys.create({ name: 'hello@microlink.io', plan: null })
+    t.true(await keys.del(id))
+    t.is(await keys.retrieve(id), null)
+  }
 })
 
 test('.del # error if key does not exist', async t => {
@@ -161,7 +194,10 @@ test('.del # error plan associated exist', async t => {
     quota: { limit: 3000, period: 'day' }
   })
 
-  const { id } = await keys.create({ name: 'hello@microlink.io', plan: plan.id })
+  const { id } = await keys.create({
+    name: 'hello@microlink.io',
+    plan: plan.id
+  })
   const error = await t.throwsAsync(keys.del(id))
 
   t.true(error.message.includes('is associated with the plan'))
diff --git a/test/plans.js b/test/plans.js
index 73f1e8f..c40c311 100644
--- a/test/plans.js
+++ b/test/plans.js
@@ -1,5 +1,3 @@
-'use strict'
-
 import { setTimeout } from 'timers/promises'
 import openkey from 'openkey'
 import Redis from 'ioredis'
@@ -25,26 +23,43 @@ test('.create # `name` is required', async t => {
 test('.create # `quota` is required', async t => {
   {
     const error = await t.throwsAsync(plans.create({ name: 'free tier' }))
-    t.is(error.message, 'The argument `quota.period` must be `day` or `week` or `month`.')
+    t.is(
+      error.message,
+      'The argument `quota.period` must be `day` or `week` or `month`.'
+    )
     t.is(error.name, 'TypeError')
   }
   {
-    const error = await t.throwsAsync(plans.create({ name: 'free tier', quota: {} }))
-    t.is(error.message, 'The argument `quota.period` must be `day` or `week` or `month`.')
+    const error = await t.throwsAsync(
+      plans.create({ name: 'free tier', quota: {} })
+    )
+    t.is(
+      error.message,
+      'The argument `quota.period` must be `day` or `week` or `month`.'
+    )
     t.is(error.name, 'TypeError')
   }
   {
-    const error = await t.throwsAsync(plans.create({ name: 'free tier', quota: { period: 'today' } }))
-    t.is(error.message, 'The argument `quota.period` must be `day` or `week` or `month`.')
+    const error = await t.throwsAsync(
+      plans.create({ name: 'free tier', quota: { period: 'today' } })
+    )
+    t.is(
+      error.message,
+      'The argument `quota.period` must be `day` or `week` or `month`.'
+    )
     t.is(error.name, 'TypeError')
   }
   {
-    const error = await t.throwsAsync(plans.create({ name: 'free tier', quota: { period: 'week' } }))
+    const error = await t.throwsAsync(
+      plans.create({ name: 'free tier', quota: { period: 'week' } })
+    )
     t.is(error.message, 'The argument `quota.limit` must be a positive number.')
     t.is(error.name, 'TypeError')
   }
   {
-    const error = await t.throwsAsync(plans.create({ name: 'free tier', quota: { period: 'week', limit: 0 } }))
+    const error = await t.throwsAsync(
+      plans.create({ name: 'free tier', quota: { period: 'week', limit: 0 } })
+    )
     t.is(error.message, 'The argument `quota.limit` must be a positive number.')
     t.is(error.name, 'TypeError')
   }