Skip to content

Commit

Permalink
Add join type awareness to models and data stores (#982)
Browse files Browse the repository at this point in the history
## Problem this Pull Request solves
As @tn3rb has been working on implementing the CRUD work from the models/data-stores into the REM work he is doing, he's encountering difficulties with:

- efficient network queries (so getting all the tickets for all the datetimes attached to an event in as few queries as possible - as opposed to doing a getRelatedTickets request for each datetime).
- using the existing selectors for the `eventespress/core` in an expected manner despite the efficient queries.

The difficulty lies in the fact that retrieving and setting up all the relations in the store state for entities retrieved through efficient queries is a extensive process that's easy to get "wrong" because of the opinionated way relation data is kept in the state.

The purpose of this pull then is to introduce some changes so that:

- `BaseEntity` models currently _are aware_ of the possible relations, but are _not_ aware of the relation types.  We need to expose this in the models so efficient queries can be built.
- Alternatively (and what might be the better option) is `eventespresso/schema` can be used for deriving the relation types and relation models from the schema stored in its state.  Then there could simply be selectors that get the needed data for building the appropriate queries and knowing what dispatches to execute to get the correct relations setup in `eventespresso/core`.
- `eventespresso/core` will have a selector (and resolver) for getting all the related entities for a collection of entity ids.  So for example `getRelatedEntitiesForIds( datetimeIds, 'ticket' )`.  This would handle all the necessary dispatches etc for correctly adding the relations to the store state as well as using the new selectors in `eventespresso/schema` to know if relations have to be joined via a join table query.
- I'd like to also explore if the schema (and the REST API behaviour) provides enough information to programmatically derive the most efficient endpoint to do the query on.  For instance, since the `include` parameter allows for requesting related model entities to be returned in the response, it might be possible to get all the entities and their relations in one go via the join table (or base model). If that is the case, then the number of requests can be reduced more and it might even make the relation dispatches easier.

## Changes in this pull

Along with the purpose outlined in the original post, I created a test app in #988 based off of this branch to do some implementation testing of things to ensure it worked as expected.  While testing, there were a number of other fixes that had to be done.  

Along with all the below, unit-tests were updated, new tests added, and additional developer documentation was created as well.

### New features

* added `resolveRelationRecordForRelation` action generator (`eventespresso/core`)
* added `getRelatedEntitiesForIds` selector and resolver. (`eventespresso/core`)
* added new `receiveRelationSchema` action and related reducer to `eventespresso/schema` store.
* added new `hasJoinTableRelation` selector and resolver (`eventespresso/schema`).
* added new `getRelationType` selector and resolver (`eventespresso/schema`).
* added new `getRelationSchema` selector and resolver (`eventespresso/schema`).
* added a new `clone` getter to the `BaseEntity` class.  Can be used to clone a given entity instance.
* added a new `forClone` property to the `BaseEntity` class.  Can be used to export a plain object representing all the fields and values on a given entity instance that can immediately be used in instantiating a new entity instance.


### Fixes

* Fix incorrect argument list in the `receiveEntityAndResolve` dispatch control invoked in the `createRelation` action. ( `eventespresso/core`)
* Add normalization for model and relation names in the `createRelations` action. (`eventespresso/core`)
* Ensure the the `getEntityById` and `getRelationEntities` selectors are resolved for the new relations created in the `createRelations` action. (`eventespresso/core`)
* add some fixes to the `eventespresso/core` reducer for relations: make sure id is always normalized.
* make sure model name is normalized for `getSchemaForModel` resolver in `eventespresso/schema`
  • Loading branch information
nerrad committed Mar 5, 2019
1 parent f6461dc commit 70e538a
Show file tree
Hide file tree
Showing 37 changed files with 2,916 additions and 428 deletions.
6 changes: 3 additions & 3 deletions assets/dist/build-manifest.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"components.css": "ee-components.997177945b17ee98a639.dist.css",
"components.js": "ee-components.242af0d6fa489fcb996c.dist.js",
"data-stores.js": "ee-data-stores.abbba76c609fdf3ba024.dist.js",
"components.js": "ee-components.cf0aaacfef0cb46b08ec.dist.js",
"data-stores.js": "ee-data-stores.df8edcafb88ccc1687d7.dist.js",
"editor-hocs.js": "ee-editor-hocs.15a813aac7a51b5473cb.dist.js",
"eejs.js": "ee-eejs.4bd48b02f85a875e3121.dist.js",
"eventespresso-core-blocks-frontend.css": "ee-eventespresso-core-blocks-frontend.f0ea9ad96d720bc8dc9b.dist.css",
Expand All @@ -11,7 +11,7 @@
"helpers.js": "ee-helpers.68fc060c389a6c79fa44.dist.js",
"hocs.js": "ee-hocs.2c2966004969557c1cf6.dist.js",
"manifest.js": "ee-manifest.45b8faa573682047153a.dist.js",
"model.js": "ee-model.41c0ae94a0949b96ad54.dist.js",
"model.js": "ee-model.721c82f777d5ba478afe.dist.js",
"validators.js": "ee-validators.a528908a7b9fe1495f98.dist.js",
"valueObjects.js": "ee-valueObjects.85086c9d7f47d85da410.dist.js",
"vendor.js": "ee-vendor.bd38b3513999b11ec414.dist.js",
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { getIdsFromBaseEntityArray } from '@eventespresso/helpers';
/**
* Internal imports
*/
import { dispatch } from '../../base-controls';
import { dispatch, select, resolveSelect } from '../../base-controls';
import { REDUCER_KEY } from '../constants';

/**
Expand Down Expand Up @@ -46,7 +46,6 @@ function* createRelation(
yield dispatch(
REDUCER_KEY,
'receiveEntityAndResolve',
singularRelationName,
relationEntity
);
yield dispatch(
Expand Down Expand Up @@ -84,6 +83,7 @@ function* createRelations(
) {
relationName = pluralModelName( relationName );
const singularRelationName = singularModelName( relationName );
const pluralRelationName = pluralModelName( relationName );

try {
assertArrayHasEntitiesForModel( relationEntities, singularRelationName );
Expand All @@ -97,7 +97,7 @@ function* createRelations(
);
return;
}
let relationIds = getIdsFromBaseEntityArray( relationEntities );
const relationIds = getIdsFromBaseEntityArray( relationEntities );
yield dispatch(
REDUCER_KEY,
'receiveEntitiesAndResolve',
Expand All @@ -112,17 +112,111 @@ function* createRelations(
relationName,
relationIds,
);
relationIds = [ ...relationIds ];
while ( relationIds.length > 0 ) {
const modelEntity = yield resolveSelect(
REDUCER_KEY,
'getEntityById',
modelName,
entityId,
);
yield dispatch(
'core/data',
'finishResolution',
REDUCER_KEY,
'getRelatedEntities',
[ modelEntity, pluralRelationName ]
);
const relationsToResolve = [ ...relationEntities ];
while ( relationsToResolve.length > 0 ) {
const relationEntity = relationsToResolve.pop();
yield dispatch(
REDUCER_KEY,
'receiveDirtyRelationAddition',
relationName,
relationIds.pop(),
relationEntity.id,
modelName,
entityId,
);
yield dispatch(
'core/data',
'finishResolution',
REDUCER_KEY,
'getRelatedEntities',
[ relationEntity, pluralModelName( modelName ) ]
);
}
}

/**
* This action is used to ensure a relation Entity related to the given
* model entity id is both added to the state and various selectors for these
* are resolved so no additional resolution happens for these.
*
* The purpose for this action is to allow for doing more efficient batch
* queries of entities from an api request and then triggering the resolution of
* any more granular selectors that have resolvers. This basically allows one
* to hydrate the `eventespresso/core` state with more efficient queries.
*
* @param {BaseEntity} relationEntity
* @param {string} modelName
* @param {number|string} modelId
*/
function* resolveRelationRecordForRelation(
relationEntity,
modelName,
modelId
) {
const singularRelationName = singularModelName( relationEntity.modelName );
const pluralRelationName = pluralModelName( relationEntity.modelName );
const hasEntity = yield select(
'core/data',
'hasFinishedResolution',
REDUCER_KEY,
'getEntityById',
[ singularRelationName, relationEntity.id ]
);
relationEntity = hasEntity ?
yield select(
REDUCER_KEY,
'getEntityById',
singularRelationName,
relationEntity.id
) :
relationEntity;
if ( ! hasEntity ) {
yield dispatch(
REDUCER_KEY,
'receiveEntityAndResolve',
relationEntity
);
}
yield dispatch(
REDUCER_KEY,
'receiveRelatedEntities',
modelName,
modelId,
pluralRelationName,
[ relationEntity.id ]
);
const modelEntity = yield resolveSelect(
REDUCER_KEY,
'getEntityById',
modelName,
modelId
);
yield dispatch(
'core/data',
'finishResolution',
REDUCER_KEY,
'getRelatedEntities',
[ modelEntity, pluralRelationName ]
);
yield dispatch(
'core/data',
'finishResolution',
REDUCER_KEY,
'getRelatedEntities',
[ relationEntity, pluralModelName( modelName ) ]
);
}

/**
Expand All @@ -142,4 +236,4 @@ const assertArrayHasEntitiesForModel = ( entities, relationModelName ) => {
}
};

export { createRelation, createRelations };
export { createRelation, createRelations, resolveRelationRecordForRelation };
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
import {
createRelation,
createRelations,
resolveRelationRecordForRelation,
} from '../create-relations-generators';
import { dispatch } from '../../../base-controls';
import { dispatch, resolveSelect, select } from '../../../base-controls';
import { REDUCER_KEY } from '../../constants';
import { EventEntities } from '../../../test/fixtures/base';
import { EventEntities, DateTimeEntities } from '../../../test/fixtures/base';

describe( 'createRelation()', () => {
describe( 'yields with expected response', () => {
Expand All @@ -34,7 +35,6 @@ describe( 'createRelation()', () => {
dispatch(
REDUCER_KEY,
'receiveEntityAndResolve',
'event',
TestEvent
)
);
Expand Down Expand Up @@ -117,12 +117,36 @@ describe( 'createRelations()', () => {
)
);
} );
it( 'yields resolveSelect control for getting the entity by id', () => {
const { value } = fulfillment.next();
expect( value ).toEqual(
resolveSelect(
REDUCER_KEY,
'getEntityById',
'datetime',
40,
)
);
} );
it( 'yields dispatch control action for finishing the resolution on ' +
'getRelatedEntities', () => {
const { value } = fulfillment.next( DateTimeEntities.a );
expect( value ).toEqual(
dispatch(
'core/data',
'finishResolution',
REDUCER_KEY,
'getRelatedEntities',
[ DateTimeEntities.a, 'events' ]
)
);
} );
describe( 'yields dispatch actions for receiving dirty relations (for all ' +
'expected ids)', () => {
[ 30, 20 ].forEach( ( relationId ) => {
it( 'yields dispatch action for id:' + relationId, () => {
[ EventEntities.c, EventEntities.b ].forEach( ( relationEntity ) => {
it( 'yields dispatch action for id:' + relationEntity.id, () => {
const { value } = fulfillment.next();
const args = [ 'events', relationId, 'datetime', 40 ];
const args = [ 'events', relationEntity.id, 'datetime', 40 ];
expect( value ).toEqual(
dispatch(
REDUCER_KEY,
Expand All @@ -131,6 +155,118 @@ describe( 'createRelations()', () => {
)
);
} );
it( 'yields dispatch action for finishing the resolution for ' +
'get relatedEntities for id: ' + relationEntity.id, () => {
const { value } = fulfillment.next();
expect( value ).toEqual(
dispatch(
'core/data',
'finishResolution',
REDUCER_KEY,
'getRelatedEntities',
[ relationEntity, 'datetimes' ]
)
);
} );
} );
} );
} );

describe( 'resolveRelationRecordForRelation', () => {
let fulfillment;
const reset = () => fulfillment = resolveRelationRecordForRelation(
DateTimeEntities.a,
'event',
20
);
it( 'yields select control action for whether the resolution has ' +
'finished for the getEntityById selector for the incoming relation ' +
'id', () => {
reset();
const { value } = fulfillment.next();
expect( value ).toEqual(
select(
'core/data',
'hasFinishedResolution',
REDUCER_KEY,
'getEntityById',
[ 'datetime', DateTimeEntities.a.id ]
)
);
} );
it( 'if entity exists in the state, then yields select action for ' +
'getting that entity from the state (it serves as authority)', () => {
const { value } = fulfillment.next( true );
expect( value ).toEqual(
select(
REDUCER_KEY,
'getEntityById',
'datetime',
DateTimeEntities.a.id
)
);
} );
it( 'if entity does not exist in the state, then yields dispatch control ' +
'for receiveEntityAndResolve', () => {
reset();
fulfillment.next();
const { value } = fulfillment.next( false );
expect( value ).toEqual(
dispatch(
REDUCER_KEY,
'receiveEntityAndResolve',
DateTimeEntities.a
)
);
} );
it( 'yields dispatch control action for receiveRelatedEntities', () => {
const { value } = fulfillment.next();
expect( value ).toEqual(
dispatch(
REDUCER_KEY,
'receiveRelatedEntities',
'event',
20,
'datetimes',
[ DateTimeEntities.a.id ]
)
);
} );
it( 'yields resolveSelect control action for getEntityById', () => {
const { value } = fulfillment.next();
expect( value ).toEqual(
resolveSelect(
REDUCER_KEY,
'getEntityById',
'event',
20,
)
);
} );
it( 'yields dispatch control action for finishing the resolution on the ' +
'getRelatedEntities selector for the relation on the model', () => {
const { value } = fulfillment.next( EventEntities.a );
expect( value ).toEqual(
dispatch(
'core/data',
'finishResolution',
REDUCER_KEY,
'getRelatedEntities',
[ EventEntities.a, 'datetimes' ]
)
);
} );
it( 'yields dispatch control action for finishing the resolution on the ' +
'getRelatedEntities selector for the model on the relation', () => {
const { value } = fulfillment.next();
expect( value ).toEqual(
dispatch(
'core/data',
'finishResolution',
REDUCER_KEY,
'getRelatedEntities',
[ DateTimeEntities.a, 'events' ]
)
);
} );
} );
7 changes: 4 additions & 3 deletions assets/src/data/eventespresso/core/reducers/relations.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@ const normalizedReceiveAndRemoveRelations = ( state, action ) => {
...action,
modelName: singularModelName( action.modelName ),
relationName: pluralModelName( action.relationName ),
entityId: normalizeEntityId( action.entityId ),
};
const {
modelName,
relationName,
entityId,
relatedEntityIds,
entityId,
} = action;
// if modelName exists, then we just process as is.
if ( state.hasIn( [ 'entityMap', modelName ] ) ) {
Expand All @@ -57,7 +58,7 @@ const normalizedReceiveAndRemoveRelations = ( state, action ) => {
};
// loop through each existing relation id and get the state for each
while ( relatedEntityIds.length > 0 ) {
newAction.entityId = relatedEntityIds.pop();
newAction.entityId = normalizeEntityId( relatedEntityIds.pop() );
state = receiveAndRemoveRelations( state, newAction );
}
return state;
Expand Down Expand Up @@ -113,7 +114,7 @@ function receiveAndRemoveRelations( state, action ) {
const relationEntityIds = Set( action.relatedEntityIds );
const path = [ 'entityMap', modelName, entityId, relationName ];

const existingIds = state.getIn( path ) || Set();
const existingIds = state.getIn( path, Set() );

switch ( type ) {
case types.RECEIVE_RELATED_ENTITY_IDS:
Expand Down
Loading

0 comments on commit 70e538a

Please sign in to comment.