Merge pull request #3604 from benjiwheeler/refactor-fetch-session

Refactor session fetching to reduce code duplication, make more testable
This commit is contained in:
Benjamin Wheeler 2019-12-19 17:23:52 -05:00 committed by GitHub
commit 29019115f3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 231 additions and 81 deletions

View file

@ -2,7 +2,45 @@ const api = require('./api');
module.exports = {};
module.exports.requestSessionWithRetry = (count, resolve, reject) => {
/*
requestSessionWithRetry()
Retries the session api call until it has either reached the limit of number of
retries, or received a successful response with that contains a 'user' field in
its body.
Each time it retries, it will double its previous waiting time, and subtract
that time from the totalDelayMS parameter.
example of what this might look like:
1st call:
receives params: retriesLeft=3 and totalDelayMS=3500
performs api call
delay until next call: 3500 / (2^3 - 1) = 500ms
next call params: retriesLeft=2, totalDelayMS=3000
2nd call:
receives params: retriesLeft=2 and totalDelayMS=3000
performs api call
delay until next call: 3000 / (2^2 - 1) = 1000ms
next call: retriesLeft=1, totalDelayMS=2000
3rd call:
receives params: retriesLeft=1, totalDelayMS=2000
performs api call
delay until next call: 2000 / (2^1 - 1) = 2000ms
next call: retriesLeft=0, totalDelayMS=0
4th call:
receives params: retriesLeft=0, totalDelayMS=0
performs api call
returns the response, even if it is undefined or empty
total api calls: 4
total delay time: 3500ms
*/
module.exports.requestSessionWithRetry = (resolve, reject, retriesLeft, totalDelayMS) => {
api({
host: '',
uri: '/session/'
@ -10,14 +48,22 @@ module.exports.requestSessionWithRetry = (count, resolve, reject) => {
if (err || (response && response.statusCode === 404)) {
return reject(err);
}
if (typeof body === 'undefined' || !body.user) {
// Retry after 500ms, 1.5s, 3.5s, 7.5s and then stop.
if (count > 4) {
if (typeof body === 'undefined' || body === null || !body.user) {
if (retriesLeft < 1) {
return resolve(body);
}
return setTimeout(module.exports.requestSessionWithRetry.bind(null, count + 1, resolve, reject),
250 * Math.pow(2, count));
const nextTimeout = totalDelayMS / (Math.pow(2, retriesLeft) - 1);
return setTimeout(
module.exports.requestSessionWithRetry.bind(
null, resolve, reject, retriesLeft - 1, totalDelayMS - nextTimeout
),
nextTimeout
);
}
return resolve(body);
});
};
module.exports.requestSession = (resolve, reject) => (
module.exports.requestSessionWithRetry(resolve, reject, 0, 0)
);

View file

@ -1,8 +1,7 @@
const keyMirror = require('keymirror');
const defaults = require('lodash.defaults');
const api = require('../lib/api');
const sessionLib = require('../lib/session');
const {requestSession, requestSessionWithRetry} = require('../lib/session');
const messageCountActions = require('./message-count.js');
const permissionsActions = require('./permissions.js');
@ -62,87 +61,59 @@ module.exports.setStatus = status => ({
status: status
});
const handleSessionResponse = (dispatch, body) => {
if (typeof body === 'undefined') return dispatch(module.exports.setSessionError('No session content'));
if (
body.user &&
body.user.banned &&
banWhitelistPaths.indexOf(window.location.pathname) === -1
) {
window.location = '/accounts/banned-response/';
return;
} else if (
body.flags &&
body.flags.must_complete_registration &&
window.location.pathname !== '/classes/complete_registration'
) {
window.location = '/classes/complete_registration';
return;
} else if (
body.flags &&
body.flags.must_reset_password &&
!body.flags.must_complete_registration &&
window.location.pathname !== '/classes/student_password_reset/'
) {
window.location = '/classes/student_password_reset/';
return;
}
dispatch(module.exports.setSession(body));
dispatch(module.exports.setStatus(module.exports.Status.FETCHED));
// get the permissions from the updated session
dispatch(permissionsActions.storePermissions(body.permissions));
if (typeof body.user !== 'undefined') {
dispatch(messageCountActions.getCount(body.user.username));
}
return;
};
module.exports.refreshSession = () => (dispatch => {
dispatch(module.exports.setStatus(module.exports.Status.FETCHING));
api({
host: '',
uri: '/session/'
}, (err, body) => {
if (err) return dispatch(module.exports.setSessionError(err));
if (typeof body === 'undefined') return dispatch(module.exports.setSessionError('No session content'));
if (
body.user &&
body.user.banned &&
banWhitelistPaths.indexOf(window.location.pathname) === -1
) {
window.location = '/accounts/banned-response/';
return;
} else if (
body.flags &&
body.flags.must_complete_registration &&
window.location.pathname !== '/classes/complete_registration'
) {
window.location = '/classes/complete_registration';
return;
} else if (
body.flags &&
body.flags.must_reset_password &&
!body.flags.must_complete_registration &&
window.location.pathname !== '/classes/student_password_reset/'
) {
window.location = '/classes/student_password_reset/';
return;
}
dispatch(module.exports.setSession(body));
dispatch(module.exports.setStatus(module.exports.Status.FETCHED));
// get the permissions from the updated session
dispatch(permissionsActions.storePermissions(body.permissions));
if (typeof body.user !== 'undefined') {
dispatch(messageCountActions.getCount(body.user.username));
}
return;
});
return new Promise((resolve, reject) => (
requestSession(resolve, reject).then(body => {
handleSessionResponse(dispatch, body);
}, err => {
dispatch(module.exports.setSessionError(err));
})
));
});
module.exports.refreshSessionWithRetry = () => (dispatch => {
dispatch(module.exports.setStatus(module.exports.Status.FETCHING));
return new Promise((resolve, reject) => (
sessionLib.requestSessionWithRetry(1, resolve, reject)
requestSessionWithRetry(resolve, reject, 4, 7500)
)).then(body => {
if (typeof body === 'undefined') return dispatch(module.exports.setSessionError('No session content'));
if (
body.user &&
body.user.banned &&
banWhitelistPaths.indexOf(window.location.pathname) === -1
) {
window.location = '/accounts/banned-response/';
return;
} else if (
body.flags &&
body.flags.must_complete_registration &&
window.location.pathname !== '/classes/complete_registration'
) {
window.location = '/classes/complete_registration';
return;
} else if (
body.flags &&
body.flags.must_reset_password &&
!body.flags.must_complete_registration &&
window.location.pathname !== '/classes/student_password_reset/'
) {
window.location = '/classes/student_password_reset/';
return;
}
dispatch(module.exports.setSession(body));
dispatch(module.exports.setStatus(module.exports.Status.FETCHED));
// get the permissions from the updated session
dispatch(permissionsActions.storePermissions(body.permissions));
if (typeof body.user !== 'undefined') {
dispatch(messageCountActions.getCount(body.user.username));
}
return;
handleSessionResponse(dispatch, body);
}, err => {
dispatch(module.exports.setSessionError(err));
});

View file

@ -0,0 +1,133 @@
describe('session library', () => {
// respond to session requests with empty session object
let sessionNoUser = jest.fn((opts, callback) => {
callback(null, {}, {statusCode: 200});
});
// respond to session requests with session object that indicates
// successfully logged-in user
let sessionYesUser = jest.fn((opts, callback) => {
callback(null, {user: {username: 'test_username'}}, {statusCode: 200});
});
// respond to first two requests with empty session object; after that,
// respond with user in object
let sessionNoThenYes = jest.fn((opts, callback) => {
if (sessionNoThenYes.mock.calls.length <= 2) {
callback(null, {}, {statusCode: 200});
} else {
callback(null, {user: {username: 'test_username'}}, {statusCode: 200});
}
});
// respond to session requests with response code 404, indicating no session
// found for that user
let sessionNotFound = jest.fn((opts, callback) => {
callback(null, null, {statusCode: 404});
});
// respond to session requests with response code 503, indicating connection failure
let sessionConnectFailure = jest.fn((opts, callback) => {
callback(null, null, {statusCode: 503});
});
// by changing whichMockAPIRequest, we can simulate different api responses
let whichMockAPIRequest = null;
let mockAPIRequest = (opts, callback) => {
whichMockAPIRequest(opts, callback);
};
// mock lib/api.js, and include our mocked version in lib/session.js
jest.mock('../../../src/lib/api', () => {
return mockAPIRequest;
});
const sessionLib = require('../../../src/lib/session'); // eslint-disable-line global-require
afterEach(() => {
jest.clearAllMocks();
});
test('requestSession can call api 1 time, when session found', done => {
whichMockAPIRequest = sessionYesUser;
new Promise((resolve, reject) => { // eslint-disable-line no-undef
sessionLib.requestSession(resolve, reject);
}).then(body => {
expect(sessionYesUser).toHaveBeenCalledTimes(1);
expect(body).toEqual({user: {username: 'test_username'}});
done();
});
});
test('requestSession can call api 1 time, when session not found', done => {
whichMockAPIRequest = sessionNoUser;
new Promise((resolve, reject) => { // eslint-disable-line no-undef
sessionLib.requestSession(resolve, reject);
}).then(body => {
expect(sessionNoUser).toHaveBeenCalledTimes(1);
expect(body).toEqual({});
done();
});
});
test('requestSessionWithRetry can call api once', done => {
whichMockAPIRequest = sessionNoUser;
new Promise((resolve, reject) => { // eslint-disable-line no-undef
sessionLib.requestSessionWithRetry(resolve, reject, 0, 0);
}).then(() => {
expect(sessionNoUser).toHaveBeenCalledTimes(1);
done();
});
});
test('requestSessionWithRetry can call api multiple times', done => {
whichMockAPIRequest = sessionNoUser;
new Promise((resolve, reject) => { // eslint-disable-line no-undef
sessionLib.requestSessionWithRetry(resolve, reject, 2, 0);
}).then(() => {
expect(sessionNoUser).toHaveBeenCalledTimes(3);
done();
});
});
test('requestSessionWithRetry respects total delay time param within a reasonable tolerance', done => {
whichMockAPIRequest = sessionNoUser;
const startTime = new Date().getTime();
new Promise((resolve, reject) => { // eslint-disable-line no-undef
sessionLib.requestSessionWithRetry(resolve, reject, 2, 2500);
}).then(() => {
const endTime = new Date().getTime();
expect(endTime - startTime > 2000).toBeTruthy();
expect(endTime - startTime < 3000).toBeTruthy();
done();
});
});
test('requestSessionWithRetry will retry if no user found, then stop when user is found', done => {
whichMockAPIRequest = sessionNoThenYes;
new Promise((resolve, reject) => { // eslint-disable-line no-undef
sessionLib.requestSessionWithRetry(resolve, reject, 4, 3000);
}).then(body => {
expect(body).toEqual({user: {username: 'test_username'}});
expect(sessionNoThenYes).toHaveBeenCalledTimes(3);
done();
});
});
test('requestSessionWithRetry handles session not found as immediate error', done => {
whichMockAPIRequest = sessionNotFound;
new Promise((resolve, reject) => { // eslint-disable-line no-undef
sessionLib.requestSessionWithRetry(resolve, reject, 2, 0);
}).then(() => {}, err => {
expect(err).toBeFalsy();
done();
});
});
test('requestSessionWithRetry handles connection failure by retrying', done => {
whichMockAPIRequest = sessionConnectFailure;
new Promise((resolve, reject) => { // eslint-disable-line no-undef
sessionLib.requestSessionWithRetry(resolve, reject, 2, 0);
}).then(body => {
expect(sessionConnectFailure).toHaveBeenCalledTimes(3);
expect(body).toBeFalsy();
done();
});
});
});