Skip to content

Commit

Permalink
Data Layer: Track data request meta information (#17263)
Browse files Browse the repository at this point in the history
* Data Layer: Track data request meta information

We often have a legitimate need in a React component to know if data has
been requested yet or if requests for that data have failed (though not
as often as you might think!)

In this patch we're adding in a new system into the `dispatchRequest`
wrapper whereby requests are tracked and that meta information is
retrievable.

Currently we track two properties: what is the current state of a
request and when did we last receive an update in Calypso?

This tracking is based off of the assumption that we have a
correspondance between the `_REQUEST` Redux action and an associated
`HTTP_REQUEST` to get it _and_ that we pass in the original action to
the call to `http()`. However, if those conditions are met then we can
track the request successfully and key it by the original action.

That is, suppose we issue `{ type: SITE_REQUEST, siteId }`: Calypso UI
code only cares about declaring that the data is needed. How does it
know the answers to the following questions?

 - _Is the data in state?_ - just check state
 - _Have we attempted to get the data?_ - `getRequestMeta( { type:
   SITE_REQUEST, siteId } ).status !== 'null'`
 - _When was our last update?_ - `getRequestMeta( ... ).lastUpdate`
 - _Was it a failed request?_ - `getRequestMeta( ... ).status ===
   'failure'`
 - _Is a request for this data going on right now?_ - `getRequestMeta(
   ... ).status === 'pending'`

This is an attempt to provide wanted information without polluting the
Redux state tree. We need to ascertain if the assumptions it makes are
safe enough to use; I suspect they are.

Expose data request metadata to window in dev environment

By exporting the request metadata to `window` in development
environments we can interact with it and manually change certain values
for testing purposes.

For example, we may want to fake a failed request. We can do this by
opening the developer console and using `dataRequests.set()`

```js
dataRequests.set("'type'='PLANS_REQUEST'", {lastUpdated: Date.now(),
status: 'failure'})
```

Track requests with a unique id

Previously we were losing updates to requests when the returned actions
were different than the first one which originated the request.

Now an additional meta property is added to the actions so that they
will carry through and we can track via mapping to the original request.

We could have also injected a custom `dispatch()` to guarantee the
tracking but this change is simpler and may be safe enough as most
cases so far don't use different actions for the HTTP request
responders.
  • Loading branch information
dmsnell committed Nov 17, 2017
1 parent 41e4be8 commit d87983f
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 84 deletions.
140 changes: 110 additions & 30 deletions client/state/data-layer/wpcom-http/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
/**
* External dependencies
*/

import deterministicStringify from 'json-stable-stringify';
import schemaValidator from 'is-my-json-valid';
import { get, identity, noop } from 'lodash';
import { get, identity, merge, noop } from 'lodash';

/**
* Internal dependencies
*/
import { keyedReducer } from 'state/utils';
import warn from 'lib/warn';

/**
Expand Down Expand Up @@ -95,10 +96,83 @@ export const makeParser = ( schema, schemaOptions = {}, transformer = identity )
// the actual parser
return data => transform( validate( data ) );
};
const getRequestStatus = action => {
if ( undefined !== getError( action ) ) {
return 'failure';
}

if ( undefined !== getData( action ) ) {
return 'success';
}

return 'pending';
};

export const getRequestKey = fullAction => {
const { meta, ...action } = fullAction; // eslint-disable-line no-unused-vars
const requestKey = get( meta, 'dataLayer.requestKey' );

return requestKey ? requestKey : deterministicStringify( action );
};

export const getRequest = ( state, key ) => state.dataRequests[ key ] || {};

export const requestsReducerItem = (
state = null,
{ meta: { dataLayer: { lastUpdated, pendingSince, status } = {} } = {} }
) => Object.assign( { status }, lastUpdated && { lastUpdated }, pendingSince && { pendingSince } );

export const reducer = keyedReducer( 'meta.dataLayer.requestKey', requestsReducerItem );

export const isRequestLoading = ( state, action ) =>
getRequest( state, getRequestKey( action ) ).status === 'pending';

export const hasRequestLoaded = ( state, action ) =>
getRequest( state, getRequestKey( action ) ).lastUpdated > -Infinity;

/**
* Tracks the state of network activity for a given request type
*
* When we issue _REQUEST type actions they usually create some
* associated network activity by means of an HTTP request.
* We may want to know what the status of those requests are, if
* they have completed or if they have failed.
*
* This tracker stores the meta data for those requests which
* can then be independently polled by React components which
* need to know about those data requests.
*
* Note that this is meta data about remote data requests and
* _not_ about network activity, which is why this is code is
* here operating on the _REQUEST actions and not in the HTTP
* pipeline as a processor on HTTP_REQUEST actions.
*
* @param {Function} next next link in HTTP middleware chain
* @returns {Function} middleware function to track requests
*/
export const trackRequests = next => ( store, action ) => {
// progress events don't affect
// any tracking meta at the moment
if ( true !== get( action, 'meta.dataLayer.trackRequest' ) || getProgress( action ) ) {
return next( store, action );
}

const requestKey = getRequestKey( action );
const status = getRequestStatus( action );
const dataLayer = Object.assign(
{ requestKey, status },
status === 'pending' ? { pendingSince: Date.now() } : { lastUpdated: Date.now() }
);

const dispatch = response => store.dispatch( merge( response, { meta: { dataLayer } } ) );

next( { ...store, dispatch }, action );
};

/**
* @type Object default dispatchRequest options
* @property {Function} fromApi validates and transforms API response data
* @property {Function} middleware chain of functions to process before dispatch
* @property {Function} onProgress called on progress events
*/
const defaultOptions = {
Expand Down Expand Up @@ -131,42 +205,46 @@ const defaultOptions = {
* onProgress :: ReduxStore -> Action -> Dispatcher -> ProgressData
* fromApi :: ResponseData -> [ Boolean, Data ]
*
* @param {Function} middleware intercepts requests moving through the system
* @param {Function} initiator called if action lacks response meta; should create HTTP request
* @param {Function} onSuccess called if the action meta includes response data
* @param {Function} onError called if the action meta includes error data
* @param {Object} options configures additional dispatching behaviors
+ @param {Function} [options.fromApi] maps between API data and Calypso data
+ @param {Function} [options.onProgress] called on progress events when uploading
* @param {Function} [options.middleware] runs before the dispatch itself
* @param {Function} [options.onProgress] called on progress events when uploading
* @returns {?*} please ignore return values, they are undefined
*/
export const dispatchRequest = ( initiator, onSuccess, onError, options = {} ) => (
store,
action
) => {
export const requestDispatcher = middleware => ( initiator, onSuccess, onError, options = {} ) => {
const { fromApi, onProgress } = { ...defaultOptions, ...options };

const error = getError( action );
if ( undefined !== error ) {
return onError( store, action, error );
}
return middleware( ( store, action ) => {
const error = getError( action );
if ( undefined !== error ) {
return onError( store, action, error );
}

const data = getData( action );
if ( undefined !== data ) {
try {
return onSuccess( store, action, fromApi( data ) );
} catch ( err ) {
return onError( store, action, err );
const data = getData( action );
if ( undefined !== data ) {
try {
return onSuccess( store, action, fromApi( data ) );
} catch ( err ) {
return onError( store, action, err );
}
}
}

const progress = getProgress( action );
if ( undefined !== progress ) {
return onProgress( store, action, progress );
}
const progress = getProgress( action );
if ( undefined !== progress ) {
return onProgress( store, action, progress );
}

return initiator( store, action );
return initiator( store, action );
} );
};

export const dispatchRequest = requestDispatcher( trackRequests );

/**
* Dispatches to appropriate function based on HTTP request meta
*
Expand Down Expand Up @@ -195,15 +273,16 @@ export const dispatchRequest = ( initiator, onSuccess, onError, options = {} ) =
* onProgress :: Action -> ProgressData -> Action
* fromApi :: ResponseData -> TransformedData throws TransformerError|SchemaError
*
* @param {Function} middleware intercepts requests moving through the system
* @param {Object} options object with named parameters:
* @param {Function} fetch called if action lacks response meta; should create HTTP request
* @param {Function} onSuccess called if the action meta includes response data
* @param {Function} onError called if the action meta includes error data
* @param {Function} onProgress called on progress events when uploading
* @param {Function} fromApi maps between API data and Calypso data
* @param {Function} options.fetch called if action lacks response meta; should create HTTP request
* @param {Function} options.onSuccess called if the action meta includes response data
* @param {Function} options.onError called if the action meta includes error data
* @param {Function} options.onProgress called on progress events when uploading
* @param {Function} options.fromApi maps between API data and Calypso data
* @returns {Action} action or action thunk to be executed in response to HTTP event
*/
export const dispatchRequestEx = options => {
export const exRequestDispatcher = middleware => options => {
if ( ! options.fetch ) {
warn( 'fetch handler is not defined: no request will ever be issued' );
}
Expand All @@ -216,7 +295,7 @@ export const dispatchRequestEx = options => {
warn( 'onError handler is not defined: error during the request is being ignored' );
}

return ( store, action ) => {
return middleware( ( store, action ) => {
// create the low-level action we want to dispatch
const requestAction = createRequestAction( options, action );

Expand All @@ -230,8 +309,9 @@ export const dispatchRequestEx = options => {
}

return store.dispatch( requestAction );
};
} );
};
export const dispatchRequestEx = exRequestDispatcher( trackRequests );

/*
* Converts an application-level Calypso action that's handled by the data-layer middleware
Expand Down
40 changes: 19 additions & 21 deletions client/state/data-layer/wpcom/plans/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
/**
* Internal dependencies
*/

import { http } from 'state/data-layer/wpcom-http/actions';
import { dispatchRequest } from 'state/data-layer/wpcom-http/utils';
import { dispatchRequestEx } from 'state/data-layer/wpcom-http/utils';
import { PLANS_REQUEST } from 'state/action-types';
import {
plansReceiveAction,
Expand All @@ -20,47 +19,46 @@ import {
/**
* Dispatches a request to fetch all available WordPress.com plans
*
* @param {Function} dispatch Redux dispatcher
* @param {Object} action Redux action
* @returns {Object} original action
*/
export const requestPlans = ( { dispatch }, action ) =>
dispatch(
http( {
export const requestPlans = action =>
http(
{
apiVersion: '1.4',
method: 'GET',
path: '/plans',
onSuccess: action,
onFailure: action,
} )
},
action
);

/**
* Dispatches returned WordPress.com plan data
*
* @param {Function} dispatch Redux dispatcher
* @param {Object} action Redux action
* @param {Array} plans raw data from plans API
* @returns {Array<Object>} Redux actions
*/
export const receivePlans = ( { dispatch }, action, plans ) => {
dispatch( plansRequestSuccessAction() );
dispatch( plansReceiveAction( plans ) );
};
export const receivePlans = ( action, plans ) => [
plansRequestSuccessAction(),
plansReceiveAction( plans ),
];

/**
* Dispatches returned error from plans request
*
* @param {Function} dispatch Redux dispatcher
* @param {Object} action Redux action
* @param {Object} rawError raw error from HTTP request
* @returns {Object} Redux action
*/
export const receiveError = ( { dispatch }, action, rawError ) => {
const error = rawError instanceof Error ? rawError.message : rawError;

dispatch( plansRequestFailureAction( error ) );
};
export const receiveError = ( action, rawError ) =>
plansRequestFailureAction( rawError instanceof Error ? rawError.message : rawError );

export const dispatchPlansRequest = dispatchRequest( requestPlans, receivePlans, receiveError );
export const dispatchPlansRequest = dispatchRequestEx( {
fetch: requestPlans,
onSuccess: receivePlans,
onError: receiveError,
} );

export default {
[ PLANS_REQUEST ]: [ dispatchPlansRequest ],
Expand Down
50 changes: 17 additions & 33 deletions client/state/data-layer/wpcom/plans/test/index.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
/** @format */

/**
* External dependencies
*/
import { expect } from 'chai';
import { spy } from 'sinon';

/**
* Internal dependencies
*/
Expand All @@ -21,49 +14,40 @@ import { WPCOM_RESPONSE } from 'state/plans/test/fixture';
describe( 'wpcom-api', () => {
describe( 'plans request', () => {
describe( '#requestPlans', () => {
test( 'should dispatch HTTP request to plans endpoint', () => {
test( 'should return HTTP request to plans endpoint', () => {
const action = { type: 'DUMMY' };
const dispatch = spy();

requestPlans( { dispatch }, action );

expect( dispatch ).to.have.been.calledOnce;
expect( dispatch ).to.have.been.calledWith(
http( {
apiVersion: '1.4',
method: 'GET',
path: '/plans',
onSuccess: action,
onFailure: action,
} )
expect( requestPlans( action ) ).toEqual(
http(
{
apiVersion: '1.4',
method: 'GET',
path: '/plans',
},
action
)
);
} );
} );

describe( '#receivePlans', () => {
test( 'should dispatch plan updates', () => {
test( 'should return plan updates', () => {
const plans = WPCOM_RESPONSE;
const action = plansReceiveAction( plans );
const dispatch = spy();

receivePlans( { dispatch }, action, plans );

expect( dispatch ).to.have.been.calledTwice;
expect( dispatch ).to.have.been.calledWith( plansRequestSuccessAction() );
expect( dispatch ).to.have.been.calledWith( plansReceiveAction( plans ) );
expect( receivePlans( action, plans ) ).toEqual( [
plansRequestSuccessAction(),
plansReceiveAction( plans ),
] );
} );
} );

describe( '#receiveError', () => {
test( 'should dispatch error', () => {
test( 'should return error', () => {
const error = 'could not find plans';
const action = plansRequestFailureAction( error );
const dispatch = spy();

receiveError( { dispatch }, action, error );

expect( dispatch ).to.have.been.calledOnce;
expect( dispatch ).to.have.been.calledWith( plansRequestFailureAction( error ) );
expect( receiveError( action, error ) ).toEqual( plansRequestFailureAction( error ) );
} );
} );
} );
Expand Down
2 changes: 2 additions & 0 deletions client/state/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import consoleDispatcher from './console-dispatch';
import countries from './countries/reducer';
import countryStates from './country-states/reducer';
import currentUser from './current-user/reducer';
import { reducer as dataRequests } from './data-layer/wpcom-http/utils';
import documentHead from './document-head/reducer';
import domains from './domains/reducer';
import geo from './geo/reducer';
Expand Down Expand Up @@ -99,6 +100,7 @@ const reducers = {
countries,
countryStates,
currentUser,
dataRequests,
documentHead,
domains,
extensions,
Expand Down

0 comments on commit d87983f

Please sign in to comment.