Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple rootParams to be used with Dataloader child resolution #65

Merged
merged 5 commits into from
Feb 27, 2020

Conversation

shawnmcknight
Copy link
Member

The purpose of this PR is to allow specification of multiple rootParams properties to be provided for child resolution while using Dataloader.

A fundamental premise of Dataloader is that only a single "key" can be loaded. This key can be any type, including objects, provided that the Dataloader can properly cache those keys. In the current implementation of Dataloader, only a single property from rootParams can be loaded, since there was no effective way to declare how an object should be loaded on to the called action params.

This PR allows for specification of a new property in the action schema's graphql definition named dataLoaderBatchParam. If this property is specified, then the data which is pulled from the parent (root) will be loaded into an object with the specified property name, rather than passed as a parameter with that name. For instance, without Dataloader, a child resolution structure like:

Item: {
  group: {
    action: 'group.get',
    rootParams: {
      parentId: 'groupParentId',
      childId: 'groupChildId',
    }
  }
}

would call an action with signature ctx.call('group.get', { groupParentId: ROOT_PARENTID, groupChildId: ROOT_CHILDID });

However, with Dataloader enabled, that same structure would call an action with signature ctx.call('group.get', { groupParentId: [ROOT_PARENTID] }); with the data being batched and loaded into an array and the second specified parameter being dropped.

With this change, if the group.get action has, in its action schema, graphql: { dataLoaderBatchParam: 'myBatchParam' }, that will instead get called with signature ctx.call('group.get', { myBatchParam: [{ groupParentId: ROOT_PARENTID, groupChildId: ROOT_CHILDID }] });. If multiple "keys" are data loaded, the myBatchParam array will have multiple objects passed to it.

To facilitate proper caching of these objects, a custom cacheKeyFn has been passed to the Dataloader constructor which will hash the objects to provide a unique ID of the value of the objects since Dataloader would otherwise use an equality check which would only match the objects by reference.

The rationale for defining this parameter name in the action schema instead of the child resolution definition is that it would be highly unusual for different resolutions calling the same action to use a different batching parameter, thus, all resolutions calling the same action would use the same batching parameter. Rather than defining that every time you wanted to child resolve via that action, the parameter only needs to be defined once in the action definition.

Copy link
Member

@icebob icebob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!

@icebob
Copy link
Member

icebob commented Feb 25, 2020

@shawnmcknight you can merge it.

@shawnmcknight
Copy link
Member Author

Thank you! Merging 🤖

@shawnmcknight shawnmcknight merged commit 46889c0 into moleculerjs:master Feb 27, 2020
@shawnmcknight shawnmcknight deleted the dataloader-objects branch May 2, 2020 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants