From e3efc885e426c977584e552b8858098202785cba Mon Sep 17 00:00:00 2001 From: Simon Gardner Date: Mon, 27 Oct 2025 16:04:34 +0000 Subject: [PATCH] Migrate UserSessionsManager and associated tests to async/await GitOrigin-RevId: 8b5f3a296798930aa1168738cd3a4c666c7a3028 --- .../.github/prompts/await-migration.prompt.md | 389 ++++++++ .../src/Features/User/UserSessionsManager.js | 311 +++--- .../unit/src/User/UserSessionsManagerTests.js | 924 ++++++++---------- 3 files changed, 906 insertions(+), 718 deletions(-) create mode 100644 services/web/.github/prompts/await-migration.prompt.md diff --git a/services/web/.github/prompts/await-migration.prompt.md b/services/web/.github/prompts/await-migration.prompt.md new file mode 100644 index 0000000000..26ee1ee114 --- /dev/null +++ b/services/web/.github/prompts/await-migration.prompt.md @@ -0,0 +1,389 @@ +--- +mode: 'agent' +description: 'Generate a clear code explanation with examples' +--- + +# Improved Async/Await Migration Instructions + +Based on lessons learned from PR #28840 and reviewer feedback, these comprehensive instructions address the original migration requirements while preventing common issues. + +## Core Migration Principles + +### 1. Function Signature Transformation + +- Convert callback-style functions to async/await +- Remove callback parameters from function signatures +- Add `async` keyword to function declarations +- Replace `return callback(err, result)` with `throw err` or `return result` +- Replace `return callback()` with `return` (or `return undefined`) + +### 2. Error Handling Patterns + +#### OError.tag Usage - CRITICAL UPDATE + +**DO NOT** wrap simple operations in try/catch just to tag errors with OError. With async/await, the stack trace is preserved automatically, making OError.tag less necessary for basic error propagation. + +```javascript +// OLD (callback style) - OError.tag was needed +callback(err) => { + OError.tag(err, 'description', { context }) + return callback(err) +} + +// BAD (unnecessary with async/await) +try { + await operation() +} catch (err) { + throw OError.tag(err, 'description', { context }) +} + +// GOOD (let errors propagate naturally) +await operation() + +// ONLY use OError.tag when adding meaningful context or transforming errors +try { + await complexOperation() +} catch (err) { + if (err.code === 'SPECIFIC_ERROR') { + throw OError.tag(err, 'meaningful context about why this failed', { + important_context: value + }) + } + throw err // let other errors propagate unchanged +} +``` + +### 3. Concurrency Considerations - CRITICAL + +#### Sequential vs Parallel Operations + +**Be extremely cautious when converting from serial to parallel operations.** The original code's choice of sequential processing is often intentional. + +```javascript +// OLD - Sequential processing (often intentional) +Async.mapSeries(items, processItem, callback) + +// BAD - Unbounded parallel processing +await Promise.all(items.map(processItem)) + +// BETTER - Keep sequential if unsure about resource limits +for (const item of items) { + await processItem(item) +} + +// GOOD - Controlled batch processing for performance +const BATCH_SIZE = 10 +for (let i = 0; i < items.length; i += BATCH_SIZE) { + const batch = items.slice(i, i + BATCH_SIZE) + await Promise.all(batch.map(processItem)) +} + +// IDEAL - Use Redis MGET for multiple key retrieval +// Instead of: Promise.all(keys.map(k => redis.get(k))) +const values = await redis.mget(keys) +``` + +#### Database/Redis Operation Guidelines + +- **Never** send unbounded parallel requests to databases +- **Prefer** sequential processing for database operations unless there's a specific performance need +- **Consider** batch operations (like Redis MGET/MSET) for multiple operations +- **Implement** maximum concurrency limits when parallel processing is necessary + +### 4. Background Operations + +#### Fire-and-Forget Pattern + +When operations were called in the background (with empty callbacks), preserve this behavior: + +```javascript +// OLD - Background operation with ignored callback +someOperation(user, function () {}) // errors swallowed + +// GOOD - Preserve background behavior +someOperation(user).catch(err => { + logger.error({ err }, 'Failed to run background operation') +}) + +// Or if truly fire-and-forget: +someOperation(user).catch(() => {}) // explicitly ignore errors +``` + +### 5. Module Export Patterns + +#### Using callbackifyAll for Dual API + +```javascript +const { callbackifyAll } = require('@overleaf/promise-utils') + +const MyModule = { + async myMethod(param) { + // async implementation + }, +} + +const moduleExports = { + ...callbackifyAll(MyModule), // callback API + promises: MyModule, // promise API +} + +module.exports = moduleExports +``` + +#### Internal Method Stubbing (for testing) + +**Only** add method binding patterns when tests need to stub internal method calls: + +```javascript +// ONLY if tests need to stub internal calls to _internalMethod +MyModule._internalMethod = (...args) => moduleExports._internalMethod(...args) +``` + +**Do NOT** expose internal methods at the top level - they should be accessible via `moduleExports.promises._internalMethod`. + +### 6. Test Migration Patterns + +#### Async Test Conversion + +```javascript +// OLD +it('should do something', function (done) { + MyModule.method(param, function (err, result) { + expect(err).to.not.exist + expect(result).to.equal(expected) + done() + }) +}) + +// NEW +it('should do something', async function () { + const result = await MyModule.promises.method(param) + expect(result).to.equal(expected) +}) +``` + +#### Mock/Stub Patterns + +```javascript +// For Redis or database mocks, ensure method chaining works +beforeEach(function () { + redis.multi = sinon.stub().returns({ + sadd: sinon.stub().returnsThis(), + pexpire: sinon.stub().returnsThis(), + exec: sinon.stub().resolves(), + }) +}) +``` + +### 7. Specific Redis Patterns + +#### Multi-Transaction Operations + +```javascript +// Correct pattern for Redis multi operations +const multi = redis.multi() +multi.sadd(key, value) +multi.pexpire(key, ttl) +await multi.exec() +``` + +#### Single vs Multiple Key Operations + +```javascript +// BAD - Multiple individual operations +const values = await Promise.all(keys.map(k => redis.get(k))) + +// GOOD - Use batch operations when available +const values = await redis.mget(keys) +``` + +## Migration Checklist + +### Before Starting + +- [ ] Understand the original code's concurrency patterns +- [ ] Identify any background operations that should remain non-blocking +- [ ] Check if Redis batch operations can replace individual operations +- [ ] Look for internal method calls that might need test stubbing + +### During Migration + +- [ ] Convert function signatures (remove callbacks, add async) +- [ ] Replace callback patterns with await +- [ ] Handle early returns properly +- [ ] Preserve sequential processing unless there's a clear performance benefit +- [ ] Keep background operations non-blocking +- [ ] Avoid unnecessary OError.tag wrapping +- [ ] Update JSDoc comments to remove callback parameters + +### After Migration + +- [ ] Run comprehensive tests (fix Docker/environment issues if needed) +- [ ] Verify all background operations still work correctly +- [ ] Check that internal method calls can be stubbed if needed +- [ ] Ensure database operations don't overwhelm resources +- [ ] Validate error handling preserves meaningful context +- [ ] **Remove all decaffeinate artifacts from both implementation AND test files** +- [ ] Add explanatory comments for any non-obvious technical patterns +- [ ] Avoid selfRef patterns - use module exports routing instead + +### Test Migration + +- [ ] **Run tests EARLY and OFTEN during migration process** +- [ ] Convert test functions to async +- [ ] Update assertion patterns +- [ ] Fix mock/stub configurations for chained operations (Redis multi, etc.) +- [ ] Verify all test scenarios still pass +- [ ] Remove duplicate or unnecessary mock setups +- [ ] Clean up decaffeinate comments from test files +- [ ] Ensure internal method stubs work through promises interface + +## Critical Lessons from Real Migration Experience + +### 1. Testing Environment Issues + +**ALWAYS run tests early and often during migration.** Don't wait until the end. + +Common test running problems: + +- Docker containers may need cleanup: `docker system prune -f` +- Use specific test grep patterns: `MOCHA_GREP="ModuleName" make test_unit_app` +- Mock objects must return proper objects for chaining (e.g., `multi()` must return `{method: stub().returnsThis(), ...}`) + +### 2. Method Stubbing for Internal Calls + +When methods call other methods internally, tests may need to stub those calls: + +```javascript +// If methodA() calls methodB() internally and tests need to verify this: +// DON'T do this - creates unnecessary complexity: +const selfRef = { ... }; // BAD pattern + +// DO this - route through the module exports interface: +moduleExports.promises.methodB(params).catch(err => { + logger.error({ err }, 'Failed to run background operation') +}) + +// Add a brief comment explaining the routing pattern: +// Route through moduleExports so tests can stub this call +``` + +### 3. Avoid the selfRef Pattern + +The `selfRef` pattern should be avoided - it's a code smell that indicates better module structure is needed: + +```javascript +// BAD - selfRef pattern +const selfRef = {} +selfRef.methodA = async function() { + await selfRef.methodB() // circular reference +} + +// GOOD - route through module exports when stubbing is needed +async methodA() { + await moduleExports.promises.methodB() // testable +} +``` + +### 4. Complete Decaffeinate Cleanup + +**Remove ALL legacy CoffeeScript artifacts** - this is a required part of the migration. + +Look for and remove patterns like these (exact format may vary): + +```javascript +/* eslint-disable */ +// TODO: This file was created by bulk-decaffeinate. + +/* eslint-disable + camelcase, + n/handle-callback-err, + max-len, + no-return-assign, + no-unused-vars, +*/ + +// Fix any style issues and re-enable lint. + +/* + * decaffeinate suggestions: + * DS102: Remove unnecessary code created because of implicit returns + * DS207: Consider shorter variations of null checks + * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md + */ +``` + +**Search patterns to look for:** + +- Comments containing "bulk-decaffeinate" or "decaffeinate" +- Large `/* eslint-disable */` blocks at the top of files +- Comments about "Fix any style issues and re-enable lint" +- "decaffeinate suggestions" comment blocks + +Check **both implementation AND test files** for these artifacts. + +### 5. Add Explanatory Comments for Non-Obvious Code + +When you need to write "ugly" code for unavoidable technical reasons, add a brief comment explaining why: + +```javascript +// Background operation - preserve fire-and-forget behavior +// Route through moduleExports so tests can stub this call +moduleExports.promises._checkSessions(user).catch(err => { + logger.error({ err }, 'Failed to check sessions in background') +}) +``` + +This prevents future developers from "refactoring" the code and breaking functionality. + +### 6. Simplify OError.tag() Usage + +**With async/await, many OError.tag() wrappers can be removed.** OError.tag() was primarily used to preserve stack traces across callback boundaries, but async/await handles this automatically. + +```javascript +// BEFORE - callback era (needed OError.tag for stack traces) +try { + await redis.multi().sadd(key, value).exec() +} catch (err) { + throw OError.tag(err, 'error adding to redis set', { key }) +} + +// AFTER - async/await preserves stack traces naturally +await redis.multi().sadd(key, value).exec() +``` + +**Keep OError.tag() only when:** + +- Adding meaningful context that aids debugging +- Transforming low-level errors into domain-specific errors +- The wrapper adds significant value beyond just a descriptive message + +**Remove OError.tag() when:** + +- The error message doesn't add meaningful context +- The tag message just restates what the code obviously does +- Stack trace preservation was the only benefit + +## Common Pitfalls to Avoid + +1. **Over-parallelization**: Don't convert all sequential operations to parallel +2. **Unnecessary error wrapping**: Don't wrap every operation in try/catch just for OError.tag +3. **Breaking background operations**: Maintain fire-and-forget behavior where intended +4. **Exposing internal methods incorrectly**: Use the promises interface, not top-level exports +5. **Resource exhaustion**: Be mindful of database connection limits and Redis performance +6. **Test mock complexity**: Keep mocks simple and targeted to what's actually needed +7. **Using selfRef patterns**: Always route through module exports instead +8. **Forgetting decaffeinate cleanup**: Remove all legacy comments and eslint disables +9. **Not running tests early**: Run tests frequently during migration, not just at the end +10. **Missing explanatory comments**: Add brief comments for non-obvious technical patterns + +## Success Metrics + +- All existing tests pass without modification (except for async conversion) +- No new resource exhaustion issues under load +- Background operations continue to work as intended +- Error messages and logging remain informative +- Internal method stubbing works correctly for testing +- Code is cleaner and more maintainable than before + +These instructions should be applied systematically, with careful consideration of the specific context and requirements of each module being migrated. diff --git a/services/web/app/src/Features/User/UserSessionsManager.js b/services/web/app/src/Features/User/UserSessionsManager.js index d4f7eb7ce5..b8d51b9282 100644 --- a/services/web/app/src/Features/User/UserSessionsManager.js +++ b/services/web/app/src/Features/User/UserSessionsManager.js @@ -1,9 +1,7 @@ -const OError = require('@overleaf/o-error') const Settings = require('@overleaf/settings') const logger = require('@overleaf/logger') -const Async = require('async') const _ = require('lodash') -const { promisify } = require('util') +const { callbackifyAll } = require('@overleaf/promise-utils') const UserSessionsRedis = require('./UserSessionsRedis') const rclient = UserSessionsRedis.client() @@ -13,246 +11,157 @@ const UserSessionsManager = { return `sess:${sessionId}` }, - trackSession(user, sessionId, callback) { + async trackSession(user, sessionId) { if (!user) { - return callback(null) + return } if (!sessionId) { - return callback(null) + return } const sessionSetKey = UserSessionsRedis.sessionSetKey(user) const value = UserSessionsManager._sessionKey(sessionId) - rclient - .multi() - .sadd(sessionSetKey, value) - .pexpire(sessionSetKey, `${Settings.cookieSessionLength}`) // in milliseconds - .exec(function (err, response) { - if (err) { - OError.tag( - err, - 'error while adding session key to UserSessions set', - { - user_id: user._id, - sessionSetKey, - } - ) - return callback(err) - } - UserSessionsManager._checkSessions(user, function () {}) - callback() - }) + + const multi = rclient.multi() + multi.sadd(sessionSetKey, value) + multi.pexpire(sessionSetKey, `${Settings.cookieSessionLength}`) // in milliseconds + + await multi.exec() + + UserSessionsManager._checkSessions(user).catch(err => { + logger.error({ err }, 'Failed to check sessions in background') + }) }, - untrackSession(user, sessionId, callback) { - if (!callback) { - callback = function () {} - } + async untrackSession(user, sessionId) { if (!user) { - return callback(null) + return } if (!sessionId) { - return callback(null) + return } const sessionSetKey = UserSessionsRedis.sessionSetKey(user) const value = UserSessionsManager._sessionKey(sessionId) - rclient - .multi() - .srem(sessionSetKey, value) - .pexpire(sessionSetKey, `${Settings.cookieSessionLength}`) // in milliseconds - .exec(function (err, response) { - if (err) { - OError.tag( - err, - 'error while removing session key from UserSessions set', - { - user_id: user._id, - sessionSetKey, - } - ) - return callback(err) - } - UserSessionsManager._checkSessions(user, function () {}) - callback() - }) + + const multi = rclient.multi() + multi.srem(sessionSetKey, value) + multi.pexpire(sessionSetKey, `${Settings.cookieSessionLength}`) // in milliseconds + + await multi.exec() + + UserSessionsManager._checkSessions(user).catch(err => { + logger.error({ err }, 'Failed to check sessions in background') + }) }, - getAllUserSessions(user, exclude, callback) { + async getAllUserSessions(user, exclude) { exclude = _.map(exclude, UserSessionsManager._sessionKey) const sessionSetKey = UserSessionsRedis.sessionSetKey(user) - rclient.smembers(sessionSetKey, function (err, sessionKeys) { - if (err) { - OError.tag(err, 'error getting all session keys for user from redis', { - user_id: user._id, - }) - return callback(err) + + const sessionKeys = await rclient.smembers(sessionSetKey) + + const filteredSessionKeys = _.filter( + sessionKeys, + k => !_.includes(exclude, k) + ) + if (filteredSessionKeys.length === 0) { + logger.debug({ userId: user._id }, 'no other sessions found, returning') + return [] + } + + // Use sequential processing to avoid overwhelming Redis + const sessions = [] + for (const key of filteredSessionKeys) { + const session = await rclient.get(key) + sessions.push(session) + } + + const result = [] + for (let session of sessions) { + if (!session) { + continue } - sessionKeys = _.filter(sessionKeys, k => !_.includes(exclude, k)) - if (sessionKeys.length === 0) { - logger.debug({ userId: user._id }, 'no other sessions found, returning') - return callback(null, []) + session = JSON.parse(session) + let sessionUser = session.passport && session.passport.user + if (!sessionUser) { + sessionUser = session.user } - Async.mapSeries( - sessionKeys, - (k, cb) => rclient.get(k, cb), - function (err, sessions) { - if (err) { - OError.tag(err, 'error getting all sessions for user from redis', { - user_id: user._id, - }) - return callback(err) - } + result.push({ + ip_address: sessionUser.ip_address, + session_created: sessionUser.session_created, + }) + } - const result = [] - for (let session of Array.from(sessions)) { - if (!session) { - continue - } - session = JSON.parse(session) - let sessionUser = session.passport && session.passport.user - if (!sessionUser) { - sessionUser = session.user - } - - result.push({ - ip_address: sessionUser.ip_address, - session_created: sessionUser.session_created, - }) - } - - callback(null, result) - } - ) - }) + return result }, /** * @param {{_id: string}} user * @param {string | null | undefined} retainSessionID - the session ID to exclude from deletion - * @param {(err: Error | null, data?: unknown) => void} callback */ - removeSessionsFromRedis(user, retainSessionID, callback) { + async removeSessionsFromRedis(user, retainSessionID) { if (!user) { - return callback( - new Error('bug: user not passed to removeSessionsFromRedis') - ) + throw new Error('bug: user not passed to removeSessionsFromRedis') } const sessionSetKey = UserSessionsRedis.sessionSetKey(user) - rclient.smembers(sessionSetKey, function (err, sessionKeys) { - if (err) { - OError.tag(err, 'error getting contents of UserSessions set', { - user_id: user._id, - sessionSetKey, - }) - return callback(err) - } - const keysToDelete = retainSessionID - ? _.without( - sessionKeys, - UserSessionsManager._sessionKey(retainSessionID) - ) - : sessionKeys - if (keysToDelete.length === 0) { - logger.debug( - { userId: user._id }, - 'no sessions in UserSessions set to delete, returning' - ) - return callback(null, 0) - } + + const sessionKeys = await rclient.smembers(sessionSetKey) + + const keysToDelete = retainSessionID + ? _.without(sessionKeys, UserSessionsManager._sessionKey(retainSessionID)) + : sessionKeys + + if (keysToDelete.length === 0) { logger.debug( - { userId: user._id, count: keysToDelete.length }, - 'deleting sessions for user' + { userId: user._id }, + 'no sessions in UserSessions set to delete, returning' ) - - const deletions = keysToDelete.map(k => cb => rclient.del(k, cb)) - - Async.series(deletions, function (err, _result) { - if (err) { - OError.tag(err, 'error revoking all sessions for user', { - user_id: user._id, - sessionSetKey, - }) - return callback(err) - } - rclient.srem(sessionSetKey, keysToDelete, function (err) { - if (err) { - OError.tag(err, 'error removing session set for user', { - user_id: user._id, - sessionSetKey, - }) - return callback(err) - } - callback(null, keysToDelete.length) - }) - }) - }) - }, - - touch(user, callback) { - if (!user) { - return callback(null) + return 0 } - const sessionSetKey = UserSessionsRedis.sessionSetKey(user) - rclient.pexpire( - sessionSetKey, - `${Settings.cookieSessionLength}`, // in milliseconds - function (err, response) { - if (err) { - OError.tag(err, 'error while updating ttl on UserSessions set', { - user_id: user._id, - }) - return callback(err) - } - callback(null) - } + + logger.debug( + { userId: user._id, count: keysToDelete.length }, + 'deleting sessions for user' ) + + // Use sequential processing to avoid overwhelming Redis + for (const key of keysToDelete) { + await rclient.del(key) + } + + await rclient.srem(sessionSetKey, keysToDelete) + + return keysToDelete.length }, - _checkSessions(user, callback) { + async touch(user) { if (!user) { - return callback(null) + return } const sessionSetKey = UserSessionsRedis.sessionSetKey(user) - rclient.smembers(sessionSetKey, function (err, sessionKeys) { - if (err) { - OError.tag(err, 'error getting contents of UserSessions set', { - user_id: user._id, - sessionSetKey, - }) - return callback(err) + + await rclient.pexpire(sessionSetKey, `${Settings.cookieSessionLength}`) + }, + + async _checkSessions(user) { + if (!user) { + return + } + const sessionSetKey = UserSessionsRedis.sessionSetKey(user) + + const sessionKeys = await rclient.smembers(sessionSetKey) + + // Use sequential processing to avoid overwhelming Redis + for (const key of sessionKeys) { + const val = await rclient.get(key) + if (!val) { + await rclient.srem(sessionSetKey, key) } - Async.series( - sessionKeys.map( - key => next => - rclient.get(key, function (err, val) { - if (err) { - return next(err) - } - if (!val) { - rclient.srem(sessionSetKey, key, function (err, result) { - return next(err) - }) - } else { - next() - } - }) - ), - function (err, results) { - callback(err) - } - ) - }) + } }, } -UserSessionsManager.promises = { - getAllUserSessions: promisify(UserSessionsManager.getAllUserSessions), - removeSessionsFromRedis: (user, retainSessionID = null) => - promisify(UserSessionsManager.removeSessionsFromRedis)( - user, - retainSessionID - ), - untrackSession: promisify(UserSessionsManager.untrackSession), +module.exports = { + ...callbackifyAll(UserSessionsManager), + promises: UserSessionsManager, } - -module.exports = UserSessionsManager diff --git a/services/web/test/unit/src/User/UserSessionsManagerTests.js b/services/web/test/unit/src/User/UserSessionsManagerTests.js index dc6a0c52e3..872ea4c56a 100644 --- a/services/web/test/unit/src/User/UserSessionsManagerTests.js +++ b/services/web/test/unit/src/User/UserSessionsManagerTests.js @@ -1,16 +1,3 @@ -/* eslint-disable - n/handle-callback-err, - max-len, - no-return-assign, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const sinon = require('sinon') const { expect } = require('chai') const modulePath = '../../../../app/src/Features/User/UserSessionsManager.js' @@ -35,14 +22,18 @@ describe('UserSessionsManager', function () { mget: sinon.stub(), pexpire: sinon.stub(), } - this.rclient.multi.returns(this.rclient) - this.rclient.get.returns(this.rclient) - this.rclient.del.returns(this.rclient) - this.rclient.sadd.returns(this.rclient) - this.rclient.srem.returns(this.rclient) - this.rclient.smembers.returns(this.rclient) - this.rclient.pexpire.returns(this.rclient) - this.rclient.exec.callsArgWith(0, null) + this.rclient.multi.returns({ + sadd: sinon.stub().returnsThis(), + srem: sinon.stub().returnsThis(), + pexpire: sinon.stub().returnsThis(), + exec: sinon.stub().resolves(), + }) + this.rclient.get.resolves() + this.rclient.del.resolves() + this.rclient.sadd.resolves() + this.rclient.srem.resolves() + this.rclient.smembers.resolves([]) + this.rclient.pexpire.resolves() this.UserSessionsRedis = { client: () => this.rclient, @@ -70,272 +61,222 @@ describe('UserSessionsManager', function () { describe('trackSession', function () { beforeEach(function () { - this.call = callback => { - return this.UserSessionsManager.trackSession( - this.user, - this.sessionId, - callback - ) - } - this.rclient.exec.callsArgWith(0, null) - return (this._checkSessions = sinon - .stub(this.UserSessionsManager, '_checkSessions') - .returns(null)) + this._checkSessions = sinon + .stub(this.UserSessionsManager.promises, '_checkSessions') + .resolves() }) afterEach(function () { return this._checkSessions.restore() }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.trackSession( + this.user, + this.sessionId + ) }) - it('should call the appropriate redis methods', function (done) { - return this.call(err => { - this.rclient.multi.callCount.should.equal(1) - this.rclient.sadd.callCount.should.equal(1) - this.rclient.pexpire.callCount.should.equal(1) - this.rclient.exec.callCount.should.equal(1) - return done() - }) + it('should call the appropriate redis methods', async function () { + await this.UserSessionsManager.promises.trackSession( + this.user, + this.sessionId + ) + this.rclient.multi.callCount.should.equal(1) + const multiInstance = this.rclient.multi.returnValues[0] + multiInstance.sadd.callCount.should.equal(1) + multiInstance.pexpire.callCount.should.equal(1) + multiInstance.exec.callCount.should.equal(1) }) - it('should call _checkSessions', function (done) { - return this.call(err => { - this._checkSessions.callCount.should.equal(1) - return done() - }) + it('should call _checkSessions', async function () { + await this.UserSessionsManager.promises.trackSession( + this.user, + this.sessionId + ) + this._checkSessions.callCount.should.equal(1) }) describe('when rclient produces an error', function () { beforeEach(function () { - return this.rclient.exec.callsArgWith(0, new Error('woops')) - }) - - it('should produce an error', function (done) { - return this.call(err => { - expect(err).to.be.instanceof(Error) - return done() + this.rclient.multi.returns({ + sadd: sinon.stub().returnsThis(), + pexpire: sinon.stub().returnsThis(), + exec: sinon.stub().rejects(new Error('woops')), }) }) - it('should not call _checkSessions', function (done) { - return this.call(err => { - this._checkSessions.callCount.should.equal(0) - return done() - }) + it('should produce an error', async function () { + await expect( + this.UserSessionsManager.promises.trackSession( + this.user, + this.sessionId + ) + ).to.be.rejectedWith(Error) + }) + + it('should not call _checkSessions', async function () { + try { + await this.UserSessionsManager.promises.trackSession( + this.user, + this.sessionId + ) + } catch (err) { + // Expected error + } + this._checkSessions.callCount.should.equal(0) }) }) describe('when no user is supplied', function () { - beforeEach(function () { - return (this.call = callback => { - return this.UserSessionsManager.trackSession( - null, - this.sessionId, - callback - ) - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.trackSession( + null, + this.sessionId + ) }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) + it('should not call the appropriate redis methods', async function () { + await this.UserSessionsManager.promises.trackSession( + null, + this.sessionId + ) + this.rclient.multi.callCount.should.equal(0) }) - it('should not call the appropriate redis methods', function (done) { - return this.call(err => { - this.rclient.multi.callCount.should.equal(0) - this.rclient.sadd.callCount.should.equal(0) - this.rclient.pexpire.callCount.should.equal(0) - this.rclient.exec.callCount.should.equal(0) - return done() - }) - }) - - it('should not call _checkSessions', function (done) { - return this.call(err => { - this._checkSessions.callCount.should.equal(0) - return done() - }) + it('should not call _checkSessions', async function () { + await this.UserSessionsManager.promises.trackSession( + null, + this.sessionId + ) + this._checkSessions.callCount.should.equal(0) }) }) describe('when no sessionId is supplied', function () { - beforeEach(function () { - return (this.call = callback => { - return this.UserSessionsManager.trackSession( - this.user, - null, - callback - ) - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.trackSession(this.user, null) }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) + it('should not call the appropriate redis methods', async function () { + await this.UserSessionsManager.promises.trackSession(this.user, null) + this.rclient.multi.callCount.should.equal(0) }) - it('should not call the appropriate redis methods', function (done) { - return this.call(err => { - this.rclient.multi.callCount.should.equal(0) - this.rclient.sadd.callCount.should.equal(0) - this.rclient.pexpire.callCount.should.equal(0) - this.rclient.exec.callCount.should.equal(0) - return done() - }) - }) - - it('should not call _checkSessions', function (done) { - return this.call(err => { - this._checkSessions.callCount.should.equal(0) - return done() - }) + it('should not call _checkSessions', async function () { + await this.UserSessionsManager.promises.trackSession(this.user, null) + this._checkSessions.callCount.should.equal(0) }) }) }) describe('untrackSession', function () { beforeEach(function () { - this.call = callback => { - return this.UserSessionsManager.untrackSession( - this.user, - this.sessionId, - callback - ) - } - this.rclient.exec.callsArgWith(0, null) - return (this._checkSessions = sinon - .stub(this.UserSessionsManager, '_checkSessions') - .returns(null)) + this._checkSessions = sinon + .stub(this.UserSessionsManager.promises, '_checkSessions') + .resolves() }) afterEach(function () { return this._checkSessions.restore() }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.untrackSession( + this.user, + this.sessionId + ) }) - it('should call the appropriate redis methods', function (done) { - return this.call(err => { - this.rclient.multi.callCount.should.equal(1) - this.rclient.srem.callCount.should.equal(1) - this.rclient.pexpire.callCount.should.equal(1) - this.rclient.exec.callCount.should.equal(1) - return done() - }) + it('should call the appropriate redis methods', async function () { + await this.UserSessionsManager.promises.untrackSession( + this.user, + this.sessionId + ) + this.rclient.multi.callCount.should.equal(1) + const multiInstance = this.rclient.multi.returnValues[0] + multiInstance.srem.callCount.should.equal(1) + multiInstance.pexpire.callCount.should.equal(1) + multiInstance.exec.callCount.should.equal(1) }) - it('should call _checkSessions', function (done) { - return this.call(err => { - this._checkSessions.callCount.should.equal(1) - return done() - }) + it('should call _checkSessions', async function () { + await this.UserSessionsManager.promises.untrackSession( + this.user, + this.sessionId + ) + this._checkSessions.callCount.should.equal(1) }) describe('when rclient produces an error', function () { beforeEach(function () { - return this.rclient.exec.callsArgWith(0, new Error('woops')) - }) - - it('should produce an error', function (done) { - return this.call(err => { - expect(err).to.be.instanceof(Error) - return done() + this.rclient.multi.returns({ + srem: sinon.stub().returnsThis(), + pexpire: sinon.stub().returnsThis(), + exec: sinon.stub().rejects(new Error('woops')), }) }) - it('should not call _checkSessions', function (done) { - return this.call(err => { - this._checkSessions.callCount.should.equal(0) - return done() - }) + it('should produce an error', async function () { + await expect( + this.UserSessionsManager.promises.untrackSession( + this.user, + this.sessionId + ) + ).to.be.rejectedWith(Error) + }) + + it('should not call _checkSessions', async function () { + try { + await this.UserSessionsManager.promises.untrackSession( + this.user, + this.sessionId + ) + } catch (err) { + // Expected error + } + this._checkSessions.callCount.should.equal(0) }) }) describe('when no user is supplied', function () { - beforeEach(function () { - return (this.call = callback => { - return this.UserSessionsManager.untrackSession( - null, - this.sessionId, - callback - ) - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.untrackSession( + null, + this.sessionId + ) }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) + it('should not call the appropriate redis methods', async function () { + await this.UserSessionsManager.promises.untrackSession( + null, + this.sessionId + ) + this.rclient.multi.callCount.should.equal(0) }) - it('should not call the appropriate redis methods', function (done) { - return this.call(err => { - this.rclient.multi.callCount.should.equal(0) - this.rclient.srem.callCount.should.equal(0) - this.rclient.pexpire.callCount.should.equal(0) - this.rclient.exec.callCount.should.equal(0) - return done() - }) - }) - - it('should not call _checkSessions', function (done) { - return this.call(err => { - this._checkSessions.callCount.should.equal(0) - return done() - }) + it('should not call _checkSessions', async function () { + await this.UserSessionsManager.promises.untrackSession( + null, + this.sessionId + ) + this._checkSessions.callCount.should.equal(0) }) }) describe('when no sessionId is supplied', function () { - beforeEach(function () { - return (this.call = callback => { - return this.UserSessionsManager.untrackSession( - this.user, - null, - callback - ) - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.untrackSession(this.user, null) }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) + it('should not call the appropriate redis methods', async function () { + await this.UserSessionsManager.promises.untrackSession(this.user, null) + this.rclient.multi.callCount.should.equal(0) }) - it('should not call the appropriate redis methods', function (done) { - return this.call(err => { - this.rclient.multi.callCount.should.equal(0) - this.rclient.srem.callCount.should.equal(0) - this.rclient.pexpire.callCount.should.equal(0) - this.rclient.exec.callCount.should.equal(0) - return done() - }) - }) - - it('should not call _checkSessions', function (done) { - return this.call(err => { - this._checkSessions.callCount.should.equal(0) - return done() - }) + it('should not call _checkSessions', async function () { + await this.UserSessionsManager.promises.untrackSession(this.user, null) + this._checkSessions.callCount.should.equal(0) }) }) }) @@ -344,231 +285,198 @@ describe('UserSessionsManager', function () { beforeEach(function () { this.sessionKeys = ['sess:one', 'sess:two'] this.currentSessionID = undefined - this.rclient.smembers.callsArgWith(1, null, this.sessionKeys) - this.rclient.del = sinon.stub().callsArgWith(1, null) - this.rclient.srem = sinon.stub().callsArgWith(2, null) - return (this.call = callback => { - return this.UserSessionsManager.removeSessionsFromRedis( + this.rclient.smembers.resolves(this.sessionKeys) + this.rclient.del.resolves() + this.rclient.srem.resolves() + }) + + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.removeSessionsFromRedis( + this.user, + this.currentSessionID + ) + }) + + it('should yield the number of purged sessions', async function () { + const result = + await this.UserSessionsManager.promises.removeSessionsFromRedis( this.user, - this.currentSessionID, - callback + this.currentSessionID ) - }) + expect(result).to.equal(this.sessionKeys.length) }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) - }) + it('should call the appropriate redis methods', async function () { + await this.UserSessionsManager.promises.removeSessionsFromRedis( + this.user, + this.currentSessionID + ) + this.rclient.smembers.callCount.should.equal(1) - it('should yield the number of purged sessions', function (done) { - return this.call((err, n) => { - expect(err).to.not.exist - expect(n).to.equal(this.sessionKeys.length) - return done() - }) - }) + this.rclient.del.callCount.should.equal(2) + expect(this.rclient.del.firstCall.args[0]).to.deep.equal( + this.sessionKeys[0] + ) + expect(this.rclient.del.secondCall.args[0]).to.deep.equal( + this.sessionKeys[1] + ) - it('should call the appropriate redis methods', function (done) { - return this.call(err => { - this.rclient.smembers.callCount.should.equal(1) - - this.rclient.del.callCount.should.equal(2) - expect(this.rclient.del.firstCall.args[0]).to.deep.equal( - this.sessionKeys[0] - ) - expect(this.rclient.del.secondCall.args[0]).to.deep.equal( - this.sessionKeys[1] - ) - - this.rclient.srem.callCount.should.equal(1) - expect(this.rclient.srem.firstCall.args[1]).to.deep.equal( - this.sessionKeys - ) - - return done() - }) + this.rclient.srem.callCount.should.equal(1) + expect(this.rclient.srem.firstCall.args[0]).to.deep.equal( + 'UserSessions:{abcd}' + ) + expect(this.rclient.srem.firstCall.args[1]).to.deep.equal( + this.sessionKeys + ) }) describe('when a session is retained', function () { beforeEach(function () { this.sessionKeys = ['sess:one', 'sess:two', 'sess:three', 'sess:four'] this.currentSessionID = 'two' - this.rclient.smembers.callsArgWith(1, null, this.sessionKeys) - this.rclient.del = sinon.stub().callsArgWith(1, null) - return (this.call = callback => { - return this.UserSessionsManager.removeSessionsFromRedis( - this.user, - this.currentSessionID, - callback - ) - }) + this.rclient.smembers.resolves(this.sessionKeys) + this.rclient.del.resolves() }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.removeSessionsFromRedis( + this.user, + this.currentSessionID + ) }) - it('should call the appropriate redis methods', function (done) { - return this.call(err => { - this.rclient.smembers.callCount.should.equal(1) - this.rclient.del.callCount.should.equal(this.sessionKeys.length - 1) - this.rclient.srem.callCount.should.equal(1) - return done() - }) + it('should call the appropriate redis methods', async function () { + await this.UserSessionsManager.promises.removeSessionsFromRedis( + this.user, + this.currentSessionID + ) + this.rclient.smembers.callCount.should.equal(1) + this.rclient.del.callCount.should.equal(this.sessionKeys.length - 1) + this.rclient.srem.callCount.should.equal(1) }) - it('should remove all sessions except for the retained one', function (done) { - return this.call(err => { - expect(this.rclient.del.firstCall.args[0]).to.deep.equal('sess:one') - expect(this.rclient.del.secondCall.args[0]).to.deep.equal( - 'sess:three' - ) - expect(this.rclient.del.thirdCall.args[0]).to.deep.equal('sess:four') - expect(this.rclient.srem.firstCall.args[1]).to.deep.equal([ - 'sess:one', - 'sess:three', - 'sess:four', - ]) - return done() - }) + it('should remove all sessions except for the retained one', async function () { + await this.UserSessionsManager.promises.removeSessionsFromRedis( + this.user, + this.currentSessionID + ) + expect(this.rclient.del.firstCall.args[0]).to.deep.equal('sess:one') + expect(this.rclient.del.secondCall.args[0]).to.deep.equal('sess:three') + expect(this.rclient.del.thirdCall.args[0]).to.deep.equal('sess:four') + expect(this.rclient.srem.firstCall.args[1]).to.deep.equal([ + 'sess:one', + 'sess:three', + 'sess:four', + ]) }) }) describe('when rclient produces an error', function () { beforeEach(function () { - return (this.rclient.del = sinon - .stub() - .callsArgWith(1, new Error('woops'))) + this.rclient.del.rejects(new Error('woops')) }) - it('should produce an error', function (done) { - return this.call(err => { - expect(err).to.be.instanceof(Error) - return done() - }) + it('should produce an error', async function () { + await expect( + this.UserSessionsManager.promises.removeSessionsFromRedis( + this.user, + this.currentSessionID + ) + ).to.be.rejectedWith(Error) }) - it('should not call rclient.srem', function (done) { - return this.call(err => { - this.rclient.srem.callCount.should.equal(0) - return done() - }) + it('should not call rclient.srem', async function () { + try { + await this.UserSessionsManager.promises.removeSessionsFromRedis( + this.user, + this.currentSessionID + ) + } catch (err) { + // Expected error + } + this.rclient.srem.callCount.should.equal(0) }) }) describe('when no user is supplied', function () { - beforeEach(function () { - return (this.call = callback => { - return this.UserSessionsManager.removeSessionsFromRedis( + it('should produce an error', async function () { + await expect( + this.UserSessionsManager.promises.removeSessionsFromRedis( null, - this.currentSessionID, - callback + this.currentSessionID ) - }) + ).to.be.rejectedWith(/bug: user not passed to removeSessionsFromRedis/) }) - it('should produce an error', function (done) { - return this.call(err => { - expect(err).to.match( - /bug: user not passed to removeSessionsFromRedis/ + it('should not call the appropriate redis methods', async function () { + try { + await this.UserSessionsManager.promises.removeSessionsFromRedis( + null, + this.currentSessionID ) - return done() - }) - }) - - it('should not call the appropriate redis methods', function (done) { - return this.call(err => { - this.rclient.smembers.callCount.should.equal(0) - this.rclient.del.callCount.should.equal(0) - this.rclient.srem.callCount.should.equal(0) - return done() - }) + } catch (err) { + // Expected error + } + this.rclient.smembers.callCount.should.equal(0) + this.rclient.del.callCount.should.equal(0) + this.rclient.srem.callCount.should.equal(0) }) }) describe('when there are no keys to delete', function () { beforeEach(function () { - return this.rclient.smembers.callsArgWith(1, null, []) + this.rclient.smembers.resolves([]) }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.removeSessionsFromRedis( + this.user, + this.currentSessionID + ) }) - it('should not do the delete operation', function (done) { - return this.call(err => { - this.rclient.smembers.callCount.should.equal(1) - this.rclient.del.callCount.should.equal(0) - this.rclient.srem.callCount.should.equal(0) - return done() - }) + it('should not do the delete operation', async function () { + await this.UserSessionsManager.promises.removeSessionsFromRedis( + this.user, + this.currentSessionID + ) + this.rclient.smembers.callCount.should.equal(1) + this.rclient.del.callCount.should.equal(0) + this.rclient.srem.callCount.should.equal(0) }) }) }) describe('touch', function () { - beforeEach(function () { - this.rclient.pexpire.callsArgWith(2, null) - return (this.call = callback => { - return this.UserSessionsManager.touch(this.user, callback) - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.touch(this.user) }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) - }) - - it('should call rclient.pexpire', function (done) { - return this.call(err => { - this.rclient.pexpire.callCount.should.equal(1) - return done() - }) + it('should call rclient.pexpire', async function () { + await this.UserSessionsManager.promises.touch(this.user) + this.rclient.pexpire.callCount.should.equal(1) }) describe('when rclient produces an error', function () { beforeEach(function () { - return this.rclient.pexpire.callsArgWith(2, new Error('woops')) + this.rclient.pexpire.rejects(new Error('woops')) }) - it('should produce an error', function (done) { - return this.call(err => { - expect(err).to.be.instanceof(Error) - return done() - }) + it('should produce an error', async function () { + await expect( + this.UserSessionsManager.promises.touch(this.user) + ).to.be.rejectedWith(Error) }) }) describe('when no user is supplied', function () { - beforeEach(function () { - return (this.call = callback => { - return this.UserSessionsManager.touch(null, callback) - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.touch(null) }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) - }) - - it('should not call pexpire', function (done) { - return this.call(err => { - this.rclient.pexpire.callCount.should.equal(0) - return done() - }) + it('should not call pexpire', async function () { + await this.UserSessionsManager.promises.touch(null) + this.rclient.pexpire.callCount.should.equal(0) }) }) }) @@ -581,218 +489,200 @@ describe('UserSessionsManager', function () { '{"passport": {"user": {"ip_address": "c", "session_created": "d"}}}', ] this.exclude = ['two'] - this.rclient.smembers.callsArgWith(1, null, this.sessionKeys) + this.rclient.smembers.resolves(this.sessionKeys) this.rclient.get = sinon.stub() - this.rclient.get.onCall(0).callsArgWith(1, null, this.sessions[0]) - this.rclient.get.onCall(1).callsArgWith(1, null, this.sessions[1]) + this.rclient.get.onCall(0).resolves(this.sessions[0]) + this.rclient.get.onCall(1).resolves(this.sessions[1]) + }) - return (this.call = callback => { - return this.UserSessionsManager.getAllUserSessions( + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.getAllUserSessions( + this.user, + this.exclude + ) + }) + + it('should get sessions', async function () { + const sessions = + await this.UserSessionsManager.promises.getAllUserSessions( this.user, - this.exclude, - callback + this.exclude ) - }) + expect(sessions).to.deep.equal([ + { ip_address: 'a', session_created: 'b' }, + { ip_address: 'c', session_created: 'd' }, + ]) }) - it('should not produce an error', function (done) { - return this.call((err, sessions) => { - expect(err).to.not.exist - return done() - }) + it('should have called rclient.smembers', async function () { + await this.UserSessionsManager.promises.getAllUserSessions( + this.user, + this.exclude + ) + this.rclient.smembers.callCount.should.equal(1) }) - it('should get sessions', function (done) { - return this.call((err, sessions) => { - expect(sessions).to.deep.equal([ - { ip_address: 'a', session_created: 'b' }, - { ip_address: 'c', session_created: 'd' }, - ]) - return done() - }) - }) - - it('should have called rclient.smembers', function (done) { - return this.call((err, sessions) => { - this.rclient.smembers.callCount.should.equal(1) - return done() - }) - }) - - it('should have called rclient.get', function (done) { - return this.call((err, sessions) => { - this.rclient.get.callCount.should.equal(this.sessionKeys.length - 1) - return done() - }) + it('should have called rclient.get', async function () { + await this.UserSessionsManager.promises.getAllUserSessions( + this.user, + this.exclude + ) + this.rclient.get.callCount.should.equal(this.sessionKeys.length - 1) }) describe('when there are no other sessions', function () { beforeEach(function () { this.sessionKeys = ['sess:two'] - return this.rclient.smembers.callsArgWith(1, null, this.sessionKeys) + this.rclient.smembers.resolves(this.sessionKeys) }) - it('should not produce an error', function (done) { - return this.call((err, sessions) => { - expect(err).to.not.exist - return done() - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises.getAllUserSessions( + this.user, + this.exclude + ) }) - it('should produce an empty list of sessions', function (done) { - return this.call((err, sessions) => { - expect(sessions).to.deep.equal([]) - return done() - }) + it('should produce an empty list of sessions', async function () { + const sessions = + await this.UserSessionsManager.promises.getAllUserSessions( + this.user, + this.exclude + ) + expect(sessions).to.deep.equal([]) }) - it('should have called rclient.smembers', function (done) { - return this.call((err, sessions) => { - this.rclient.smembers.callCount.should.equal(1) - return done() - }) + it('should have called rclient.smembers', async function () { + await this.UserSessionsManager.promises.getAllUserSessions( + this.user, + this.exclude + ) + this.rclient.smembers.callCount.should.equal(1) }) - it('should not have called rclient.mget', function (done) { - return this.call((err, sessions) => { - this.rclient.mget.callCount.should.equal(0) - return done() - }) + it('should not have called rclient.get for individual keys', async function () { + await this.UserSessionsManager.promises.getAllUserSessions( + this.user, + this.exclude + ) + this.rclient.get.callCount.should.equal(0) }) }) describe('when smembers produces an error', function () { beforeEach(function () { - return this.rclient.smembers.callsArgWith(1, new Error('woops')) + this.rclient.smembers.rejects(new Error('woops')) }) - it('should produce an error', function (done) { - return this.call((err, sessions) => { - expect(err).to.not.equal(null) - expect(err).to.be.instanceof(Error) - return done() - }) + it('should produce an error', async function () { + await expect( + this.UserSessionsManager.promises.getAllUserSessions( + this.user, + this.exclude + ) + ).to.be.rejectedWith(Error) }) - it('should not have called rclient.mget', function (done) { - return this.call((err, sessions) => { - this.rclient.mget.callCount.should.equal(0) - return done() - }) + it('should not have called rclient.get', async function () { + try { + await this.UserSessionsManager.promises.getAllUserSessions( + this.user, + this.exclude + ) + } catch (err) { + // Expected error + } + this.rclient.get.callCount.should.equal(0) }) }) describe('when get produces an error', function () { beforeEach(function () { - return (this.rclient.get = sinon - .stub() - .callsArgWith(1, new Error('woops'))) + this.rclient.get = sinon.stub().rejects(new Error('woops')) }) - it('should produce an error', function (done) { - return this.call((err, sessions) => { - expect(err).to.not.equal(null) - expect(err).to.be.instanceof(Error) - return done() - }) + it('should produce an error', async function () { + await expect( + this.UserSessionsManager.promises.getAllUserSessions( + this.user, + this.exclude + ) + ).to.be.rejectedWith(Error) }) }) }) describe('_checkSessions', function () { beforeEach(function () { - this.call = callback => { - return this.UserSessionsManager._checkSessions(this.user, callback) - } this.sessionKeys = ['one', 'two'] - this.rclient.smembers.callsArgWith(1, null, this.sessionKeys) - this.rclient.get.callsArgWith(1, null, 'some-value') - return this.rclient.srem.callsArgWith(2, null, {}) + this.rclient.smembers.resolves(this.sessionKeys) + this.rclient.get.resolves('some-value') + this.rclient.srem.resolves({}) }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises._checkSessions(this.user) }) - it('should call the appropriate redis methods', function (done) { - return this.call(err => { - this.rclient.smembers.callCount.should.equal(1) - this.rclient.get.callCount.should.equal(2) - this.rclient.srem.callCount.should.equal(0) - return done() - }) + it('should call the appropriate redis methods', async function () { + await this.UserSessionsManager.promises._checkSessions(this.user) + this.rclient.smembers.callCount.should.equal(1) + this.rclient.get.callCount.should.equal(2) + this.rclient.srem.callCount.should.equal(0) }) describe('when one of the keys is not present in redis', function () { beforeEach(function () { - this.rclient.get.onCall(0).callsArgWith(1, null, 'some-val') - return this.rclient.get.onCall(1).callsArgWith(1, null, null) + this.rclient.get.onCall(0).resolves('some-val') + this.rclient.get.onCall(1).resolves(null) }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises._checkSessions(this.user) }) - it('should remove that key from the set', function (done) { - return this.call(err => { - this.rclient.smembers.callCount.should.equal(1) - this.rclient.get.callCount.should.equal(2) - this.rclient.srem.callCount.should.equal(1) - this.rclient.srem.firstCall.args[1].should.equal('two') - return done() - }) + it('should remove that key from the set', async function () { + await this.UserSessionsManager.promises._checkSessions(this.user) + this.rclient.smembers.callCount.should.equal(1) + this.rclient.get.callCount.should.equal(2) + this.rclient.srem.callCount.should.equal(1) + this.rclient.srem.firstCall.args[1].should.equal('two') }) }) describe('when no user is supplied', function () { - beforeEach(function () { - return (this.call = callback => { - return this.UserSessionsManager._checkSessions(null, callback) - }) + it('should not produce an error', async function () { + await this.UserSessionsManager.promises._checkSessions(null) }) - it('should not produce an error', function (done) { - return this.call(err => { - expect(err).to.not.exist - return done() - }) - }) - - it('should not call redis methods', function (done) { - return this.call(err => { - this.rclient.smembers.callCount.should.equal(0) - this.rclient.get.callCount.should.equal(0) - return done() - }) + it('should not call redis methods', async function () { + await this.UserSessionsManager.promises._checkSessions(null) + this.rclient.smembers.callCount.should.equal(0) + this.rclient.get.callCount.should.equal(0) }) }) describe('when one of the get operations produces an error', function () { beforeEach(function () { - this.rclient.get.onCall(0).callsArgWith(1, new Error('woops'), null) - return this.rclient.get.onCall(1).callsArgWith(1, null, null) + this.rclient.get.onCall(0).rejects(new Error('woops')) + this.rclient.get.onCall(1).resolves(null) }) - it('should produce an error', function (done) { - return this.call(err => { - expect(err).to.be.instanceof(Error) - return done() - }) + it('should produce an error', async function () { + await expect( + this.UserSessionsManager.promises._checkSessions(this.user) + ).to.be.rejectedWith(Error) }) - it('should call the right redis methods, bailing out early', function (done) { - return this.call(err => { - this.rclient.smembers.callCount.should.equal(1) - this.rclient.get.callCount.should.equal(1) - this.rclient.srem.callCount.should.equal(0) - return done() - }) + it('should call the right redis methods, bailing out early', async function () { + try { + await this.UserSessionsManager.promises._checkSessions(this.user) + } catch (err) { + // Expected error + } + this.rclient.smembers.callCount.should.equal(1) + this.rclient.get.callCount.should.equal(1) + this.rclient.srem.callCount.should.equal(0) }) }) })