Skip to content

Commit

Permalink
Merge branch 'feature/queue-semaphore' into develop
Browse files Browse the repository at this point in the history
  • Loading branch information
emanueleDiVizio committed Oct 3, 2019
2 parents 9943f2a + d556be9 commit 59244e6
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/components/NetworkConnectivity.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function validateProps(props: Props) {
throw new Error('httpMethod parameter should be either HEAD or OPTIONS');
}
}

/* eslint-disable react/default-props-match-prop-types */
class NetworkConnectivity extends React.PureComponent<Props, State> {
static defaultProps = {
onConnectivityChange: () => undefined,
Expand Down
3 changes: 3 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ module.exports = {
get offlineActionTypes() {
return require('./redux/actionTypes').default;
},
get offlineActionCreators() {
return require('./redux/actionCreators').default;
},
get networkSaga() {
return require('./redux/sagas').default;
},
Expand Down
12 changes: 12 additions & 0 deletions src/redux/actionCreators.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
FluxActionWithPreviousIntent,
FluxActionForRemoval,
FluxActionForDismissal,
FluxActionForQueueSemaphoreChange,
} from '../types';

type EnqueuedAction = FluxAction | Function;
Expand Down Expand Up @@ -53,3 +54,14 @@ export const dismissActionsFromQueue = (
type: actionTypes.DISMISS_ACTIONS_FROM_QUEUE,
payload: actionTrigger,
});

export const queueSemaphoreChange = (
shouldHaltQueue: boolean,
): FluxActionForQueueSemaphoreChange => ({
type: actionTypes.QUEUE_SEMAPHORE_CHANGE,
payload: shouldHaltQueue,
});

export default {
queueSemaphoreChange,
};
2 changes: 2 additions & 0 deletions src/redux/actionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ type ActionTypes = {|
FETCH_OFFLINE_MODE: '@@network-connectivity/FETCH_OFFLINE_MODE',
REMOVE_FROM_ACTION_QUEUE: '@@network-connectivity/REMOVE_FROM_ACTION_QUEUE',
DISMISS_ACTIONS_FROM_QUEUE: '@@network-connectivity/DISMISS_ACTIONS_FROM_QUEUE',
QUEUE_SEMAPHORE_CHANGE: '@@network-connectivity/QUEUE_SEMAPHORE_CHANGE',
|};

const actionTypes: ActionTypes = {
Expand All @@ -13,6 +14,7 @@ const actionTypes: ActionTypes = {
REMOVE_FROM_ACTION_QUEUE: '@@network-connectivity/REMOVE_FROM_ACTION_QUEUE',
DISMISS_ACTIONS_FROM_QUEUE:
'@@network-connectivity/DISMISS_ACTIONS_FROM_QUEUE',
QUEUE_SEMAPHORE_CHANGE: '@@network-connectivity/QUEUE_SEMAPHORE_CHANGE',
};

export default actionTypes;
23 changes: 16 additions & 7 deletions src/redux/createNetworkMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,19 @@ function didComeBackOnline(action, wasConnected) {
);
}

export const createReleaseQueue = (getState, next, delay, shouldDequeueSelector) => async queue => {
function didQueueResume(action, wasQueueHalted) {
return (
action.type === networkActionTypes.QUEUE_SEMAPHORE_CHANGE &&
wasQueueHalted &&
action.payload === false
);
}

export const createReleaseQueue = (getState, next, delay) => async queue => {
// eslint-disable-next-line
for (const action of queue) {
const { isConnected } = getState().network;
if (isConnected &&
shouldDequeueSelector(getState())) {
const { isConnected, hasQueueBeenHalted } = getState().network;
if (isConnected && !hasQueueBeenHalted) {
next(removeActionFromQueue(action));
next(action);
// eslint-disable-next-line
Expand All @@ -96,7 +103,7 @@ function createNetworkMiddleware({
return ({ getState }: MiddlewareAPI<State>) => (
next: (action: any) => void,
) => (action: any) => {
const { isConnected, actionQueue } = getState().network;
const { isConnected, actionQueue, hasQueueBeenHalted } = getState().network;

This comment has been minimized.

Copy link
@MakhouT

MakhouT Oct 8, 2019

I think isQueuePaused is a better name

This comment has been minimized.

Copy link
@emanueleDiVizio

emanueleDiVizio Oct 8, 2019

Author Owner

Good point, thanks!

const releaseQueue = createReleaseQueue(
getState,
next,
Expand All @@ -118,8 +125,10 @@ function createNetworkMiddleware({
}

const isBackOnline = didComeBackOnline(action, isConnected);
let shouldDequeue = (isConnected || isBackOnline) && actionQueue.length > 0 && shouldDequeueSelector(getState());
if (shouldDequeue) {
const hasQueueBeenResumed = didQueueResume(action, hasQueueBeenHalted);

let shouldDequeue = (isConnected || isBackOnline || hasQueueBeenResumed) && actionQueue.length > 0 && shouldDequeueSelector(getState());
if (isBackOnline || hasQueueBeenResumed) {
// Dispatching queued actions in order of arrival (if we have any)
next(action);
return releaseQueue(actionQueue);
Expand Down
23 changes: 21 additions & 2 deletions src/redux/createReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {
export const initialState = {
isConnected: true,
actionQueue: [],
hasQueueBeenHalted: false,
};

function handleOfflineAction(
Expand Down Expand Up @@ -81,8 +82,24 @@ function handleDismissActionsFromQueue(
};
}

export default (comparisonFn: Function = getSimilarActionInQueue) => (
state: NetworkState = initialState,
function handleQueueSemaphoreChange(
state: NetworkState,
hasQueueBeenHalted: boolean,
): NetworkState {
return {
...state,
hasQueueBeenHalted,
};
}

export default (
comparisonFn: Function = getSimilarActionInQueue,
shouldQueueStartAutomatically: boolean = true,
) => (
state: NetworkState = {
...initialState,
hasQueueBeenHalted: !shouldQueueStartAutomatically,
},
action: *,
): NetworkState => {
switch (action.type) {
Expand All @@ -97,6 +114,8 @@ export default (comparisonFn: Function = getSimilarActionInQueue) => (
return handleRemoveActionFromQueue(state, action.payload);
case actionTypes.DISMISS_ACTIONS_FROM_QUEUE:
return handleDismissActionsFromQueue(state, action.payload);
case actionTypes.QUEUE_SEMAPHORE_CHANGE:
return handleQueueSemaphoreChange(state, action.payload);
default:
return state;
}
Expand Down
6 changes: 6 additions & 0 deletions src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,15 @@ export type FluxActionForDismissal = {
payload: string,
};

export type FluxActionForQueueSemaphoreChange = {
type: string,
payload: boolean,
};

export type NetworkState = {
isConnected: boolean,
actionQueue: Array<*>,
hasQueueBeenHalted: boolean,
};

export type HTTPMethod = 'HEAD' | 'OPTIONS';
28 changes: 24 additions & 4 deletions test/createNetworkMiddleware.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,21 +378,20 @@ describe('createReleaseQueue', () => {
const mockGetState = jest.fn().mockImplementation(() => ({
network: {
isConnected: true,
hasQueueBeenHalted: false,
},
}));
const mockDelay = 50;
afterEach(() => {
mockDispatch.mockClear();
mockGetState.mockClear();
});

it('empties the queue if we are online and dequeue selector returns true', async () => {
const mockDequeueSelector = () => true;

it('empties the queue if we are online and queue is not halted', async () => {
const releaseQueue = createReleaseQueue(
mockGetState,
mockDispatch,
mockDelay,
mockDequeueSelector,
);
const actionQueue = ['foo', 'bar'];
await releaseQueue(actionQueue);
Expand Down Expand Up @@ -457,6 +456,27 @@ describe('createReleaseQueue', () => {
);
expect(mockDispatch).toHaveBeenNthCalledWith(2, 'foo');
});

it('should stop dispatching if queue has been halted', async () => {
const haltQueue = () =>
new Promise(async resolve => {
await wait(30);
mockGetState.mockImplementation(() => ({
network: {
hasQueueBeenHalted: true,
},
}));
resolve();
});
const releaseQueue = createReleaseQueue(
mockGetState,
mockDispatch,
mockDelay,
);
const actionQueue = ['foo', 'bar'];
await Promise.all([releaseQueue(actionQueue), haltQueue()]);
expect(mockDispatch).toHaveBeenCalledTimes(0);
});
});

describe('createNetworkMiddleware with wrong type params', () => {
Expand Down
32 changes: 32 additions & 0 deletions test/reducer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const networkReducer = createReducer();
const getState = (isConnected = false, ...actionQueue) => ({
isConnected,
actionQueue,
hasQueueBeenHalted: false,
});

/** Actions used from now on to test different scenarios */
Expand Down Expand Up @@ -63,6 +64,7 @@ describe('CONNECTION_CHANGE action type', () => {
expect(networkReducer(initialState, mockAction)).toEqual({
isConnected: false,
actionQueue: [],
hasQueueBeenHalted: false,
});
});
});
Expand Down Expand Up @@ -106,6 +108,7 @@ describe('OFFLINE_ACTION action type', () => {
expect(nextState).toEqual({
isConnected: false,
actionQueue: [prevActionToRetry1],
hasQueueBeenHalted: false,
});

const action2 = actionCreators.fetchOfflineMode(prevActionToRetry2);
Expand Down Expand Up @@ -236,6 +239,17 @@ describe('REMOVE_ACTION_FROM_QUEUE action type', () => {
});
});

describe('QUEUE_SEMAPHORE_CHANGE action type', () => {
it('Assigns the correct value to hasQueueBeenHalted', () => {
expect(
networkReducer(undefined, actionCreators.queueSemaphoreChange(true)),
).toEqual({
...initialState,
hasQueueBeenHalted: true,
});
});
});

describe('thunks', () => {
function fetchThunk(dispatch) {
dispatch({ type: 'FETCH_DATA_REQUEST' });
Expand Down Expand Up @@ -383,3 +397,21 @@ describe('networkSelector', () => {
});
});
});

describe('network reducer config', () => {
it('has hasQueueBeenHalted set to false by default', () => {
expect(networkReducer(undefined, { type: 'ACTION_I_DONT_CARE' })).toEqual(
initialState,
);
});

it('has hasQueueBeenHalted set to true if shouldQueueStartAutomatically is false', () => {
const networkReducerWithConfig = createReducer(undefined, false);
expect(
networkReducerWithConfig(undefined, { type: 'ACTION_I_DONT_CARE' }),
).toEqual({
...initialState,
hasQueueBeenHalted: true,
});
});
});
33 changes: 20 additions & 13 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -800,8 +800,8 @@
"@types/istanbul-lib-report" "*"

"@types/node@*":
version "12.7.9"
resolved "https://registry.yarnpkg.com/@types/node/-/node-12.7.9.tgz#da0210f91096aa67138cf5afd04c4d629f8a406a"
version "12.7.8"
resolved "https://registry.yarnpkg.com/@types/node/-/node-12.7.8.tgz#cb1bf6800238898bc2ff6ffa5702c3cadd350708"

"@types/stack-utils@^1.0.1":
version "1.0.1"
Expand Down Expand Up @@ -2609,18 +2609,18 @@ eslint-plugin-prettier@^3.0.0:
prettier-linter-helpers "^1.0.0"

eslint-plugin-react@^7.11.1:
version "7.15.1"
resolved "https://registry.yarnpkg.com/eslint-plugin-react/-/eslint-plugin-react-7.15.1.tgz#db5f8ed66c6ba46922518f08e1df9dac52ccaa49"
version "7.14.3"
resolved "https://registry.yarnpkg.com/eslint-plugin-react/-/eslint-plugin-react-7.14.3.tgz#911030dd7e98ba49e1b2208599571846a66bdf13"
dependencies:
array-includes "^3.0.3"
doctrine "^2.1.0"
has "^1.0.3"
jsx-ast-utils "^2.2.1"
jsx-ast-utils "^2.1.0"
object.entries "^1.1.0"
object.fromentries "^2.0.0"
object.values "^1.1.0"
prop-types "^15.7.2"
resolve "^1.12.0"
resolve "^1.10.1"

eslint-scope@^4.0.3:
version "4.0.3"
Expand Down Expand Up @@ -4367,7 +4367,7 @@ jsprim@^1.2.2:
json-schema "0.2.3"
verror "1.10.0"

jsx-ast-utils@^2.2.1:
jsx-ast-utils@^2.1.0, jsx-ast-utils@^2.2.1:
version "2.2.1"
resolved "https://registry.yarnpkg.com/jsx-ast-utils/-/jsx-ast-utils-2.2.1.tgz#4d4973ebf8b9d2837ee91a8208cc66f3a2776cfb"
dependencies:
Expand Down Expand Up @@ -4969,16 +4969,23 @@ minimist@~0.0.1:
version "0.0.10"
resolved "https://registry.yarnpkg.com/minimist/-/minimist-0.0.10.tgz#de3f98543dbf96082be48ad1a0c7cda836301dcf"

minipass@^2.6.0, minipass@^2.8.6, minipass@^2.9.0:
minipass@^2.6.0, minipass@^2.8.6:
version "2.8.6"
resolved "https://registry.yarnpkg.com/minipass/-/minipass-2.8.6.tgz#620d889ace26356391d010ecb9458749df9b6db5"
dependencies:
safe-buffer "^5.1.2"
yallist "^3.0.0"

minipass@^2.9.0:
version "2.9.0"
resolved "https://registry.yarnpkg.com/minipass/-/minipass-2.9.0.tgz#e713762e7d3e32fed803115cf93e04bca9fcc9a6"
dependencies:
safe-buffer "^5.1.2"
yallist "^3.0.0"

minizlib@^1.2.1:
version "1.3.3"
resolved "https://registry.yarnpkg.com/minizlib/-/minizlib-1.3.3.tgz#2290de96818a34c29551c8a8d301216bd65a861d"
version "1.3.2"
resolved "https://registry.yarnpkg.com/minizlib/-/minizlib-1.3.2.tgz#5d24764998f98112586f7e566bd4c0999769dad4"
dependencies:
minipass "^2.9.0"

Expand Down Expand Up @@ -6116,7 +6123,7 @@ resolve@1.1.7:
version "1.1.7"
resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.1.7.tgz#203114d82ad2c5ed9e8e0411b3932875e889e97b"

resolve@^1.10.0, resolve@^1.11.0, resolve@^1.12.0, resolve@^1.3.2, resolve@^1.5.0, resolve@^1.8.1:
resolve@^1.10.0, resolve@^1.10.1, resolve@^1.11.0, resolve@^1.12.0, resolve@^1.3.2, resolve@^1.5.0, resolve@^1.8.1:
version "1.12.0"
resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.12.0.tgz#3fc644a35c84a48554609ff26ec52b66fa577df6"
dependencies:
Expand Down Expand Up @@ -7193,8 +7200,8 @@ yallist@^2.1.2:
resolved "https://registry.yarnpkg.com/yallist/-/yallist-2.1.2.tgz#1c11f9218f076089a47dd512f93c6699a6a81d52"

yallist@^3.0.0, yallist@^3.0.3:
version "3.1.1"
resolved "https://registry.yarnpkg.com/yallist/-/yallist-3.1.1.tgz#dbb7daf9bfd8bac9ab45ebf602b8cbad0d5d08fd"
version "3.1.0"
resolved "https://registry.yarnpkg.com/yallist/-/yallist-3.1.0.tgz#906cc2100972dc2625ae78f566a2577230a1d6f7"

yargs-parser@^11.1.1:
version "11.1.1"
Expand Down

0 comments on commit 59244e6

Please sign in to comment.