From fc0657362bfe58a12c34dd4a70b73b58b9f10aed Mon Sep 17 00:00:00 2001 From: fcaps Date: Sat, 18 Nov 2023 21:56:57 +0100 Subject: [PATCH] renaming Lock to Mutex and use appConfig --- express.js | 7 --- lib/LeaderboardService.js | 10 ++-- lib/LeaderboardServiceFactory.js | 6 +-- lib/{LockService.js => MutexService.js} | 12 ++--- routes/views/leaderboardRouter.js | 8 ++-- tests/LeaderboardService.test.js | 8 ++-- tests/LockService.test.js | 64 ------------------------- tests/MutexService.test.js | 61 +++++++++++++++++++++++ 8 files changed, 84 insertions(+), 92 deletions(-) rename lib/{LockService.js => MutexService.js} (83%) delete mode 100644 tests/LockService.test.js create mode 100644 tests/MutexService.test.js diff --git a/express.js b/express.js index 3def79ca..b5a92800 100644 --- a/express.js +++ b/express.js @@ -71,13 +71,6 @@ function loggedIn(req, res, next) { } } -//Start and listen on port - - - -// --- R O U T E S --- -// when the website is asked to render "/pageName" it will come here and see what are the "instructions" to render said page. If the page isn't here, then the website won't render it properly. - app.use('/', authRouter) app.use('/', staticMarkdownRouter) app.use('/news', newsRouter) diff --git a/lib/LeaderboardService.js b/lib/LeaderboardService.js index 4447c999..89e24c43 100644 --- a/lib/LeaderboardService.js +++ b/lib/LeaderboardService.js @@ -1,8 +1,8 @@ class LeaderboardService { - constructor(cacheService, lockService, leaderboardRepository, lockTimeout = 3000) { + constructor(cacheService, mutexService, leaderboardRepository, lockTimeout = 3000) { this.lockTimeout = lockTimeout this.cacheService = cacheService - this.lockService = lockService + this.mutexService = mutexService this.leaderboardRepository = leaderboardRepository } @@ -18,13 +18,13 @@ class LeaderboardService { return this.cacheService.get(cacheKey) } - if (this.lockService.locked) { - await this.lockService.lock(() => { + if (this.mutexService.locked) { + await this.mutexService.acquire(() => { }, this.lockTimeout) return this.getLeaderboard(id) } - await this.lockService.lock(async () => { + await this.mutexService.acquire(async () => { const result = await this.leaderboardRepository.fetchLeaderboard(id) this.cacheService.set(cacheKey, result); }) diff --git a/lib/LeaderboardServiceFactory.js b/lib/LeaderboardServiceFactory.js index d614de44..0f8cef30 100644 --- a/lib/LeaderboardServiceFactory.js +++ b/lib/LeaderboardServiceFactory.js @@ -1,10 +1,10 @@ const LeaderboardService = require("./LeaderboardService"); const LeaderboardRepository = require("./LeaderboardRepository"); -const {LockService} = require("./LockService"); +const {MutexService} = require("./MutexService"); const NodeCache = require("node-cache"); const {Axios} = require("axios"); -const leaderboardLock = new LockService() +const leaderboardMutex = new MutexService() const cacheService = new NodeCache( { stdTTL: 300, // use 5 min for all caches if not changed with ttl @@ -19,5 +19,5 @@ module.exports = (javaApiBaseURL, token) => { }; const javaApiClient = new Axios(config) - return new LeaderboardService(cacheService, leaderboardLock, new LeaderboardRepository(javaApiClient)) + return new LeaderboardService(cacheService, leaderboardMutex, new LeaderboardRepository(javaApiClient)) } diff --git a/lib/LockService.js b/lib/MutexService.js similarity index 83% rename from lib/LockService.js rename to lib/MutexService.js index 76a5e5c8..a31c9f66 100644 --- a/lib/LockService.js +++ b/lib/MutexService.js @@ -1,13 +1,13 @@ -class LockoutTimeoutError extends Error { +class AcquireTimeoutError extends Error { } -class LockService { +class MutexService { constructor() { this.queue = []; this.locked = false; } - async lock(callback, timeLimitMS = 500) { + async acquire(callback, timeLimitMS = 500) { let timeoutHandle; const lockHandler = {} @@ -16,7 +16,7 @@ class LockService { lockHandler.reject = reject timeoutHandle = setTimeout( - () => reject(new LockoutTimeoutError('LockService timeout reached')), + () => reject(new AcquireTimeoutError('MutexService timeout reached')), timeLimitMS ); }); @@ -66,5 +66,5 @@ class LockService { } } -module.exports.LockService = LockService -module.exports.LockoutTimeoutError = LockoutTimeoutError +module.exports.MutexService = MutexService +module.exports.AcquireTimeoutError = AcquireTimeoutError diff --git a/routes/views/leaderboardRouter.js b/routes/views/leaderboardRouter.js index 2282a847..c2f5c2bf 100644 --- a/routes/views/leaderboardRouter.js +++ b/routes/views/leaderboardRouter.js @@ -1,7 +1,9 @@ const express = require('express'); const router = express.Router(); const LeaderboardServiceFactory = require('../../lib/LeaderboardServiceFactory') -const {LockoutTimeoutError} = require("../../lib/LockService"); +const {AcquireTimeoutError} = require('../../lib/MutexService'); +const appConfig = require('../../config/app') + const getLeaderboardId = (leaderboardName) => { const mapping = { @@ -39,11 +41,11 @@ router.get('/:leaderboard.json', async (req, res) => { } const token = req.user.data.attributes.token - const leaderboardService = LeaderboardServiceFactory(process.env.API_URL, token) + const leaderboardService = LeaderboardServiceFactory(appConfig.apiUrl, token) return res.json(await leaderboardService.getLeaderboard(leaderboardId)) } catch (e) { - if (e instanceof LockoutTimeoutError) { + if (e instanceof AcquireTimeoutError) { return res.status(503).json({error: 'timeout reached'}) } diff --git a/tests/LeaderboardService.test.js b/tests/LeaderboardService.test.js index 95a90f65..410158af 100644 --- a/tests/LeaderboardService.test.js +++ b/tests/LeaderboardService.test.js @@ -1,6 +1,6 @@ const LeaderboardService = require("../lib/LeaderboardService") const LeaderboardRepository = require("../lib/LeaderboardRepository") -const {LockService} = require("../lib/LockService") +const {MutexService} = require("../lib/MutexService") const NodeCache = require("node-cache") const {Axios} = require("axios"); @@ -34,7 +34,7 @@ beforeEach(() => { new NodeCache( { stdTTL: 300, checkperiod: 600 } ), - new LockService(), + new MutexService(), new LeaderboardRepository(axios) ) }) @@ -97,9 +97,9 @@ test('timeout for cache creation throws an error', async () => { expect.assertions(1); axios.get.mockImplementationOnce(() => Promise.resolve({ status: 200, data: fakeEntry })) - leaderboardService.lockService.locked = true + leaderboardService.mutexService.locked = true leaderboardService.getLeaderboard(0).then(() => {}).catch((e) => { - expect(e.toString()).toBe('Error: LockService timeout reached') + expect(e.toString()).toBe('Error: MutexService timeout reached') }) jest.runOnlyPendingTimers() diff --git a/tests/LockService.test.js b/tests/LockService.test.js deleted file mode 100644 index 216f1d7f..00000000 --- a/tests/LockService.test.js +++ /dev/null @@ -1,64 +0,0 @@ -const {LockoutTimeoutError, LockService} = require('../lib/LockService') -function Sleep(milliseconds) { - return new Promise(resolve => setTimeout(resolve, milliseconds)); -} -test('release will unlock the queue', async () => { - const lockService = new LockService() - expect(lockService.locked).toBe(false) - - await lockService.lock(() => { - expect(lockService.locked).toBe(true) - }) - - expect(lockService.locked).toBe(false) -}) - -test('call lock twice will fill the queue', async () => { - let oneCalled = false - let twoCalled = false - const lockService = new LockService() - const one = lockService.lock(() => oneCalled = true) - const two = lockService.lock(() => twoCalled = true) - - expect(lockService.queue).toHaveLength(1) - expect(lockService.locked).toBe(true) - - await one - await two - expect(oneCalled).toBe(true) - expect(twoCalled).toBe(true) - expect(lockService.queue).toHaveLength(0) - expect(lockService.locked).toBe(false) -}) - -test('lock timeout will trow an error if locked by another "process" for too long', async () => { - expect.assertions(1); - - const lockService = new LockService() - - await lockService.lock(async () => { - try { - await lockService.lock(() => {}, 1) - } catch (e) { - expect(e).toBeInstanceOf(LockoutTimeoutError) - } - }) -}); - -test('lock timeout will remove it from queue', async () => { - expect.assertions(2); - - const lockService = new LockService() - - await lockService.lock(async () => { - try { - await lockService.lock(() => { - }, 1) - } catch (e) { - expect(e).toBeInstanceOf(LockoutTimeoutError) - expect(lockService.queue).toHaveLength(0); - } - - }) -}) - diff --git a/tests/MutexService.test.js b/tests/MutexService.test.js new file mode 100644 index 00000000..47d389c2 --- /dev/null +++ b/tests/MutexService.test.js @@ -0,0 +1,61 @@ +const {AcquireTimeoutError, MutexService} = require('../lib/MutexService') +test('release will unlock the queue', async () => { + const mutexService = new MutexService() + expect(mutexService.locked).toBe(false) + + await mutexService.acquire(() => { + expect(mutexService.locked).toBe(true) + }) + + expect(mutexService.locked).toBe(false) +}) + +test('call lock twice will fill the queue', async () => { + let oneCalled = false + let twoCalled = false + const mutexService = new MutexService() + const one = mutexService.acquire(() => oneCalled = true) + const two = mutexService.acquire(() => twoCalled = true) + + expect(mutexService.queue).toHaveLength(1) + expect(mutexService.locked).toBe(true) + + await one + await two + expect(oneCalled).toBe(true) + expect(twoCalled).toBe(true) + expect(mutexService.queue).toHaveLength(0) + expect(mutexService.locked).toBe(false) +}) + +test('lock timeout will trow an error if locked by another "process" for too long', async () => { + expect.assertions(1); + + const mutexService = new MutexService() + + await mutexService.acquire(async () => { + try { + await mutexService.acquire(() => {}, 1) + } catch (e) { + expect(e).toBeInstanceOf(AcquireTimeoutError) + } + }) +}); + +test('lock timeout will remove it from queue', async () => { + expect.assertions(2); + + const mutexService = new MutexService() + + await mutexService.acquire(async () => { + try { + await mutexService.acquire(() => { + }, 1) + } catch (e) { + expect(e).toBeInstanceOf(AcquireTimeoutError) + expect(mutexService.queue).toHaveLength(0); + } + + }) +}) +