Fix atomicity of counter and test for it

Author: mxstbr

src/index.js

@@ -27,12 +27,13 @@ module.exports = async function (req, res) {
   }
   const shouldIncrement = query.inc !== 'false' && query.inc !== false
   try {
+    const currentViews = db.has(pathname) ? db.get(pathname).views.length : 0
     // Add a view and send the total views back to the client
     if (shouldIncrement) {
-      await pushView(pathname, { time: Date.now() })
+      pushView(pathname, { time: Date.now() })
     }
     if (req.method === 'GET') {
-      send(res, 200, { views: db.has(pathname) ? db.get(pathname).views.length : 0 })
+      send(res, 200, { views: shouldIncrement ? currentViews + 1 : currentViews })
     } else {
       send(res, 200)
     }

src/utils.js

@@ -4,9 +4,9 @@ const db = require('./db')
 // db messing up.
 const pushView = async (key, view) => {
   const locks = {}
-  await push()
+  push()
 
-  async function push() {
+  function push() {
     if (locks[key]) return setImmediate(push)
     locks[key] = true
 
@@ -18,9 +18,13 @@ const pushView = async (key, view) => {
     }
 
     try {
-      await db.put(key, { views: views.concat([view]) })
-      delete locks[key]
+      db.put(key, { views: views.concat([view]) })
+        .then(() => {
+          delete locks[key]
+        })
+        .catch(err => { throw err })
     } catch (err) {
+      delete locks[key]
       throw err
     }
   }

tests/atomicity.test.js

@@ -0,0 +1,28 @@
+const request = require('request-promise')
+const { listen, mockDb } = require('./utils')
+
+jest.mock('../src/db', () => mockDb)
+const service = require('../src')
+let url
+
+beforeEach(async () => {
+  url = await listen(service)
+  mockDb._setDelay(10)
+})
+
+afterEach(async () => {
+  mockDb._reset()
+  mockDb._setDelay()
+})
+
+it('should atomically set two views coming in at the same time', async () => {
+  // Request twice at the same time
+  // NOTE: These two requests will return a wrong view count, they'll both say it's 1
+  request(`${url}/path`)
+  request(`${url}/path`)
+  // After the data is persisted (in the mocked case after 10ms) the path shows the right view count
+  setTimeout(async () => {
+    const body = JSON.parse(await request(`${url}/path`))
+    expect(body.views).toEqual(3)
+  }, 10)
+})

tests/items.test.js

@@ -6,11 +6,8 @@ const service = require('../src')
 let url
 
 beforeEach(async () => {
-  url = await listen(service)
-})
-
-afterEach(async () => {
   mockDb._reset()
+  url = await listen(service)
 })
 
 describe('single', () => {

tests/utils.js

@@ -17,7 +17,7 @@ const DB = () => {
     keys: () => Object.keys(data),
     // Custom methods used in tests
     _reset: () => { data = {} },
-    _setDelay: (ms) => { DELAY = ms }
+    _setDelay: (ms) => { DELAY = ms || 1 }
   }
 }