Merge pull request #658 from sharetribe/improve-api-data-handling

Improve API response data handling
This commit is contained in:
Kimmo Puputti 2018-01-22 10:07:41 +02:00 committed by GitHub
commit cfa119b1d3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 159 additions and 125 deletions

View file

@ -1,5 +1,5 @@
import { pick } from 'lodash';
import { updatedEntities, denormalisedEntities } from '../../util/data';
import { denormalisedResponseEntities } from '../../util/data';
import { storableError } from '../../util/errors';
import { TRANSITION_REQUEST } from '../../util/types';
import * as log from '../../util/log';
@ -176,10 +176,11 @@ export const speculateTransaction = (listingId, bookingStart, bookingEnd) => (
return sdk.transactions
.initiateSpeculative(bodyParams, queryParams)
.then(response => {
const transactionId = response.data.data.id;
const entities = updatedEntities({}, response.data);
const denormalised = denormalisedEntities(entities, 'transaction', [transactionId]);
const tx = denormalised[0];
const entities = denormalisedResponseEntities(response);
if (entities.length !== 1) {
throw new Error('Expected a resource in the sdk.transactions.initiateSpeculative response');
}
const tx = entities[0];
dispatch(speculateTransactionSuccess(tx));
})
.catch(e => {

View file

@ -2,7 +2,7 @@ import { pick } from 'lodash';
import { types as sdkTypes } from '../../util/sdkLoader';
import { storableError } from '../../util/errors';
import { addMarketplaceEntities } from '../../ducks/marketplaceData.duck';
import { updatedEntities, denormalisedEntities } from '../../util/data';
import { denormalisedResponseEntities } from '../../util/data';
import { TRANSITION_ENQUIRE } from '../../util/types';
import { fetchCurrentUser } from '../../ducks/user.duck';
@ -117,10 +117,8 @@ export const fetchReviews = listingId => (dispatch, getState, sdk) => {
return sdk.reviews
.query({ listing_id: listingId, state: 'public', include: ['author', 'author.profileImage'] })
.then(response => {
const entities = updatedEntities({}, response.data);
const reviewIds = response.data.data.map(d => d.id);
const denormalized = denormalisedEntities(entities, 'review', reviewIds);
dispatch(fetchReviewsSuccess(denormalized));
const reviews = denormalisedResponseEntities(response);
dispatch(fetchReviewsSuccess(reviews));
})
.catch(e => {
dispatch(fetchReviewsError(storableError(e)));

View file

@ -34,10 +34,11 @@ const initialState = {
const resultIds = data => data.data.map(l => l.id);
const merge = (state, apiResponse) => {
const merge = (state, sdkResponse) => {
const apiResponse = sdkResponse.data;
return {
...state,
ownEntities: updatedEntities({ ...state.ownEntities }, apiResponse.data),
ownEntities: updatedEntities({ ...state.ownEntities }, apiResponse),
};
};
@ -143,35 +144,14 @@ export default manageListingsPageReducer;
* @param {Object} state the full Redux store
* @param {Array<UUID>} listingIds listing IDs to select from the store
*/
export const getListingsById = (state, listingIds) => {
export const getOwnListingsById = (state, listingIds) => {
const { ownEntities } = state.ManageListingsPage;
try {
return denormalisedEntities(ownEntities, 'ownListing', listingIds);
} catch (e) {
return [];
}
};
/**
* Get the denormalised own entities from the given entity references.
*
* @param {Object} state the full Redux store
*
* @param {Array<{ id, type }} entityRefs References to entities that
* we want to query from the data. Currently we expect that all the
* entities have the same type.
*
* @return {Array<Object>} denormalised entities
*/
export const getOwnEntities = (state, entityRefs) => {
const { ownEntities } = state.ManageListingsPage;
const type = entityRefs.length > 0 ? entityRefs[0].type : null;
const ids = entityRefs.map(ref => ref.id);
try {
return denormalisedEntities(ownEntities, type, ids);
} catch (e) {
return [];
}
const resources = listingIds.map(id => ({
id,
type: 'ownListing',
}));
const throwIfNotFound = false;
return denormalisedEntities(ownEntities, resources, throwIfNotFound);
};
// ================ Action creators ================ //
@ -179,9 +159,9 @@ export const getOwnEntities = (state, entityRefs) => {
// This works the same way as addMarketplaceEntities,
// but we don't want to mix own listings with searched listings
// (own listings data contains different info - e.g. exact location etc.)
export const addOwnEntities = apiResponse => ({
export const addOwnEntities = sdkResponse => ({
type: ADD_OWN_ENTITIES,
payload: apiResponse,
payload: sdkResponse,
});
export const openListingRequest = listingId => ({

View file

@ -22,7 +22,7 @@ import { TopbarContainer } from '../../containers';
import {
closeListing,
openListing,
getListingsById,
getOwnListingsById,
queryOwnListings,
} from './ManageListingsPage.duck';
import css from './ManageListingsPage.css';
@ -201,7 +201,7 @@ const mapStateToProps = state => {
closingListing,
closingListingError,
} = state.ManageListingsPage;
const listings = getListingsById(state, currentPageResultIds);
const listings = getOwnListingsById(state, currentPageResultIds);
return {
currentPageResultIds,
listings,

View file

@ -1,6 +1,6 @@
import { addMarketplaceEntities } from '../../ducks/marketplaceData.duck';
import { fetchCurrentUser } from '../../ducks/user.duck';
import { updatedEntities, denormalisedEntities } from '../../util/data';
import { denormalisedResponseEntities } from '../../util/data';
import { storableError } from '../../util/errors';
// ================ Action types ================ //
@ -139,10 +139,8 @@ export const queryUserReviews = userId => (dispatch, getState, sdk) => {
sdk.reviews
.query({ subject_id: userId, state: 'public', include: ['author', 'author_profile_image'] })
.then(response => {
const entities = updatedEntities({}, response.data);
const reviewIds = response.data.data.map(d => d.id);
const denormalized = denormalisedEntities(entities, 'review', reviewIds);
dispatch(queryReviewsSuccess(denormalized));
const reviews = denormalisedResponseEntities(response);
dispatch(queryReviewsSuccess(reviews));
})
.catch(e => dispatch(queryReviewsError(e)));
};

View file

@ -1,4 +1,4 @@
import { updatedEntities, denormalisedEntities } from '../../util/data';
import { denormalisedResponseEntities } from '../../util/data';
import { storableError } from '../../util/errors';
import { currentUserShowSuccess } from '../../ducks/user.duck';
@ -134,11 +134,11 @@ export const updateProfile = actionPayload => {
.then(response => {
dispatch(updateProfileSuccess(response));
// Include profile image to denormalized user entity
const currentUserId = response.data.data.id;
const entities = updatedEntities({}, response.data);
const denormalised = denormalisedEntities(entities, 'currentUser', [currentUserId]);
const currentUser = denormalised[0];
const entities = denormalisedResponseEntities(response);
if (entities.length !== 1) {
throw new Error('Expected a resource in the sdk.currentUser.updateProfile response');
}
const currentUser = entities[0];
// Update current user in state.user.currentUser through user.duck.js
dispatch(currentUserShowSuccess(currentUser));

View file

@ -10,7 +10,11 @@ import {
TRANSITION_REVIEW_2_BY_PROVIDER,
} from '../../util/types';
import * as log from '../../util/log';
import { updatedEntities, denormalisedEntities } from '../../util/data';
import {
updatedEntities,
denormalisedEntities,
denormalisedResponseEntities,
} from '../../util/data';
import { addMarketplaceEntities } from '../../ducks/marketplaceData.duck';
import { fetchCurrentUserNotifications } from '../../ducks/user.duck';
@ -221,7 +225,8 @@ export const fetchTransaction = id => (dispatch, getState, sdk) => {
txResponse = response;
const listingId = listingRelationship(response).id;
const entities = updatedEntities({}, response.data);
const denormalised = denormalisedEntities(entities, 'listing', [listingId]);
const listingRef = { id: listingId, type: 'listing' };
const denormalised = denormalisedEntities(entities, [listingRef]);
const listing = denormalised[0];
const canFetchListing = listing && listing.attributes && !listing.attributes.deleted;
@ -302,15 +307,13 @@ const fetchMessages = (txId, page) => (dispatch, getState, sdk) => {
return sdk.messages
.query({ transaction_id: txId, include: ['sender', 'sender.profileImage'], ...paging })
.then(response => {
const entities = updatedEntities({}, response.data);
const messageIds = response.data.data.map(d => d.id);
const denormalizedMessages = denormalisedEntities(entities, 'message', messageIds);
const messages = denormalisedResponseEntities(response);
const { totalItems, totalPages, page: fetchedPage } = response.data.meta;
const pagination = { totalItems, totalPages, page: fetchedPage };
const totalMessages = getState().TransactionPage.totalMessages;
// Original fetchMessages call succeeded
dispatch(fetchMessagesSuccess(denormalizedMessages, pagination));
dispatch(fetchMessagesSuccess(messages, pagination));
// Check if totalItems has changed between fetched pagination pages
// if totalItems has changed, fetch first page again to include new incoming messages.

View file

@ -11,10 +11,11 @@ const initialState = {
entities: {},
};
const merge = (state, apiResponse) => {
const merge = (state, sdkResponse) => {
const apiResponse = sdkResponse.data;
return {
...state,
entities: updatedEntities({ ...state.entities }, apiResponse.data),
entities: updatedEntities({ ...state.entities }, apiResponse),
};
};
@ -39,11 +40,12 @@ export default function marketplaceDataReducer(state = initialState, action = {}
*/
export const getListingsById = (state, listingIds) => {
const { entities } = state.marketplaceData;
try {
return denormalisedEntities(entities, 'listing', listingIds);
} catch (e) {
return [];
}
const resources = listingIds.map(id => ({
id,
type: 'listing',
}));
const throwIfNotFound = false;
return denormalisedEntities(entities, resources, throwIfNotFound);
};
/**
@ -59,18 +61,13 @@ export const getListingsById = (state, listingIds) => {
*/
export const getMarketplaceEntities = (state, entityRefs) => {
const { entities } = state.marketplaceData;
const type = entityRefs.length > 0 ? entityRefs[0].type : null;
const ids = entityRefs.map(ref => ref.id);
try {
return denormalisedEntities(entities, type, ids);
} catch (e) {
return [];
}
const throwIfNotFound = false;
return denormalisedEntities(entities, entityRefs, throwIfNotFound);
};
// ================ Action creators ================ //
export const addMarketplaceEntities = apiResponse => ({
export const addMarketplaceEntities = sdkResponse => ({
type: ADD_MARKETPLACE_ENTITIES,
payload: apiResponse,
payload: sdkResponse,
});

View file

@ -1,4 +1,4 @@
import { updatedEntities, denormalisedEntities } from '../util/data';
import { denormalisedResponseEntities } from '../util/data';
import { storableError } from '../util/errors';
import { TRANSITION_REQUEST, TRANSITION_REQUEST_AFTER_ENQUIRY } from '../util/types';
import * as log from '../util/log';
@ -335,11 +335,12 @@ export const fetchCurrentUser = () => (dispatch, getState, sdk) => {
return sdk.currentUser
.show({ include: ['profileImage'] })
.then(response => {
// Include profile image to denormalized user entity
const currentUserId = response.data.data.id;
const entities = updatedEntities({}, response.data);
const denormalised = denormalisedEntities(entities, 'currentUser', [currentUserId]);
const currentUser = denormalised[0];
const entities = denormalisedResponseEntities(response);
if (entities.length !== 1) {
throw new Error('Expected a resource in the sdk.currentUser.show response');
}
const currentUser = entities[0];
// set current user id to the logger
log.setUserId(currentUser.id.uuid);
dispatch(currentUserShowSuccess(currentUser));

View file

@ -39,7 +39,6 @@ export const updatedEntities = (oldEntities, apiResponse) => {
const { data, included = [] } = apiResponse;
const objects = (Array.isArray(data) ? data : [data]).concat(included);
/* eslint-disable no-param-reassign */
const newEntities = objects.reduce((entities, curr) => {
const { id, type } = curr;
entities[type] = entities[type] || {};
@ -47,30 +46,36 @@ export const updatedEntities = (oldEntities, apiResponse) => {
entities[type][id.uuid] = entity ? combinedResourceObjects(entity, curr) : curr;
return entities;
}, oldEntities);
/* eslint-enable no-param-reassign */
return newEntities;
};
/**
* Denormalise the entities with the given IDs from the entities object
* Denormalise the entities with the resources from the entities object
*
* This function calculates the dernormalised tree structure from the
* normalised entities object with all the relationships joined in.
*
* @param {Object} entities entities object in the SDK Redux store
* @param {String} type entity type of the given IDs
* @param {Array<UUID>} ids IDs to pick from the entities
* @param {Array<{ id, type }} resources array of objects
* with id and type
* @param {Boolean} throwIfNotFound wheather to skip a resource that
* is not found (false), or to throw an Error (true)
*
* @return {Array} the given resource objects denormalised that were
* found in the entities
*/
export const denormalisedEntities = (entities, type, ids) => {
if (!entities[type] && ids.length > 0) {
throw new Error(`No entities of type ${type}`);
}
return ids.map(id => {
const entity = entities[type][id.uuid];
if (!entity) {
throw new Error(`Entity ${type} with id ${id.uuid} not found`);
export const denormalisedEntities = (entities, resources, throwIfNotFound = true) => {
const denormalised = resources.map(res => {
const { id, type } = res;
const entityFound = entities[type] && id && entities[type][id.uuid];
if (!entityFound) {
if (throwIfNotFound) {
throw new Error(`Entity with type "${type}" and id "${id ? id.uuid : id}" not found`);
}
return null;
}
const entity = entities[type][id.uuid];
const { relationships, ...entityData } = entity;
if (relationships) {
@ -84,15 +89,13 @@ export const denormalisedEntities = (entities, type, ids) => {
const hasMultipleRefs = Array.isArray(relRef.data);
const multipleRefsEmpty = hasMultipleRefs && relRef.data.length === 0;
if (!relRef.data || multipleRefsEmpty) {
// eslint-disable-next-line no-param-reassign
ent[relName] = hasMultipleRefs ? [] : null;
} else {
const refs = hasMultipleRefs ? relRef.data : [relRef.data];
const relIds = refs.map(ref => ref.id);
const relType = refs[0].type;
const rels = denormalisedEntities(entities, relType, relIds);
// eslint-disable-next-line no-param-reassign
// If a relationship is not found, an Error should be thrown
const rels = denormalisedEntities(entities, refs, true);
ent[relName] = hasMultipleRefs ? rels : rels[0];
}
return ent;
@ -102,6 +105,28 @@ export const denormalisedEntities = (entities, type, ids) => {
}
return entityData;
});
return denormalised.filter(e => !!e);
};
/**
* Denormalise the data from the given SDK response
*
* @param {Object} sdkResponse response object from an SDK call
*
* @return {Array} entities in the response with relationships
* denormalised from the included data
*/
export const denormalisedResponseEntities = sdkResponse => {
const apiResponse = sdkResponse.data;
const data = apiResponse.data;
const resources = Array.isArray(data) ? data : [data];
if (!data || resources.length === 0) {
return [];
}
const entities = updatedEntities({}, apiResponse);
return denormalisedEntities(entities, resources);
};
/**

View file

@ -187,38 +187,43 @@ describe('data utils', () => {
attributes: { url: `https://example.com/${id}.jpg`, width: 300, height: 200 },
});
it('return no results with empty ids', () => {
it('returns no results with empty resources', () => {
const entities = { listing: { listing1: createListing('listing1') } };
expect(denormalisedEntities(entities, 'listing', [])).toEqual([]);
expect(denormalisedEntities(entities, [])).toEqual([]);
});
it('return no results with empty entities and empty ids', () => {
it('returns no results with empty entities and empty ids', () => {
const entities = {};
expect(denormalisedEntities(entities, 'listing', [])).toEqual([]);
expect(denormalisedEntities(entities, [])).toEqual([]);
});
it('throws when selecting a nonexistent type', () => {
it('handles selecting a nonexistent resource of different type', () => {
const entities = { listing: { listing1: createListing('listing1') } };
expect(() => denormalisedEntities(entities, 'user', [new UUID('user1')])).toThrow(
'No entities of type user'
const user = createUser('user1');
expect(() => denormalisedEntities(entities, [user])).toThrow(
'Entity with type "user" and id "user1" not found'
);
// Empty results when error throwIfNotFound=false
expect(denormalisedEntities(entities, [user], false)).toEqual([]);
});
it('throws when selecting a nonexistent id', () => {
it('handles selecting a nonexistent resource of same type', () => {
const entities = { listing: { listing1: createListing('listing1') } };
const ids = [new UUID('listing2')];
expect(() => denormalisedEntities(entities, 'listing', ids)).toThrow(
'Entity listing with id listing2 not found'
const listing2 = createListing('listing2');
expect(() => denormalisedEntities(entities, [listing2])).toThrow(
'Entity with type "listing" and id "listing2" not found'
);
// Empty results when error throwIfNotFound=false
expect(denormalisedEntities(entities, [listing2], false)).toEqual([]);
});
it('throws if a related entity is not found', () => {
const listing1 = createListing('listing1');
listing1.relationships = { author: { data: createUser('user1') } };
const entities = { listing: { listing1 } };
const ids = [listing1.id];
expect(() => denormalisedEntities(entities, 'listing', ids)).toThrow(
'No entities of type user'
const resources = [listing1];
expect(() => denormalisedEntities(entities, resources)).toThrow(
'Entity with type "user" and id "user1" not found'
);
});
@ -228,8 +233,39 @@ describe('data utils', () => {
const listing3 = createListing('listing3');
const entities = { listing: { listing1, listing2, listing3 } };
const ids = [listing1.id, listing3.id];
expect(denormalisedEntities(entities, 'listing', ids)).toEqual([listing1, listing3]);
const resources = [listing1, listing3];
expect(denormalisedEntities(entities, resources)).toEqual([listing1, listing3]);
});
it('returns entities of different types', () => {
const user1 = createUser('user1');
const listing1 = createListing('listing1');
const entities = {
listing: { listing1 },
user: { user1 },
};
expect(denormalisedEntities(entities, [listing1, user1])).toEqual([listing1, user1]);
});
it('throws with circular relationships', () => {
// Currently the data handling code cannot handle circular
// relationships. These are possible in practice with certain
// include params in different API queries, but fixing this is
// not trivial. The denormalisation cannot skip entities that
// are already added since that would remove the fundamental
// feature of updating entities with new data.
const user1 = createUser('user1');
const listing1 = createListing('listing1');
const listing1Relationships = { author: { data: user1 } };
const listing1WithRelationships = { ...listing1, relationships: listing1Relationships };
const user1Relationships = { topListing: { data: listing1 } };
const user1WithRelationships = { ...user1, relationships: user1Relationships };
const entities = {
listing: { listing1: listing1WithRelationships },
user: { user1: user1WithRelationships },
};
expect(() => denormalisedEntities(entities, [listing1])).toThrow();
});
it('denormalises simple relationships', () => {
@ -242,10 +278,7 @@ describe('data utils', () => {
listing: { listing1: listing1WithRelationships, listing2 },
user: { user1 },
};
const ids = [listing1.id];
expect(denormalisedEntities(entities, 'listing', ids)).toEqual([
{ ...listing1, author: user1 },
]);
expect(denormalisedEntities(entities, [listing1])).toEqual([{ ...listing1, author: user1 }]);
});
it('denormalises multiple relationships', () => {
@ -265,9 +298,8 @@ describe('data utils', () => {
user: { user1 },
image: { image1, image2 },
};
const ids = [listing1.id, listing2.id];
expect(denormalisedEntities(entities, 'listing', ids)).toEqual([
expect(denormalisedEntities(entities, [listing1, listing2])).toEqual([
{ ...listing1, author: user1, images: [image1, image2] },
listing2,
]);
@ -287,9 +319,8 @@ describe('data utils', () => {
listing: { listing1: listing1WithRelationships, listing2, listing3 },
user: { user1 },
};
const ids = [listing1.id, listing2.id];
expect(denormalisedEntities(entities, 'listing', ids)).toEqual([
expect(denormalisedEntities(entities, [listing1, listing2])).toEqual([
{ ...listing1, author: null, images: [] },
listing2,
]);