Skip to content

Commit

Permalink
Merge 3482d93 into 59cba3f
Browse files Browse the repository at this point in the history
  • Loading branch information
@shawnmcknight
shawnmcknight committed Feb 21, 2020
2 parents 59cba3f + 3482d93 commit 65cbcfe
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 29 deletions.
96 changes: 71 additions & 25 deletions src/service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ module.exports = function(mixinOptions) {
*/
createActionResolver(actionName, def = {}) {
const {
dataLoader = false,
dataLoader: useDataLoader = false,
nullIfError = false,
params: staticParams = {},
rootParams = {},
Expand All @@ -138,44 +138,63 @@ module.exports = function(mixinOptions) {

return async (root, args, context) => {
try {
if (dataLoader) {
if (useDataLoader) {
const dataLoaderMapKey = this.getDataLoaderMapKey(
actionName,
staticParams,
args
);
const dataLoaderRootKey = rootKeys[0]; // for dataloader, use the first root key only
// if a dataLoader batching parameter is specified, then all root params can be data loaded;
// otherwise use only the primary rootParam
const primaryDataLoaderRootKey = rootKeys[0]; // for dataloader, use the first root key only
const dataLoaderBatchParam = this.dataLoaderBatchParams.get(actionName);
const dataLoaderUseAllRootKeys = dataLoaderBatchParam != null;

// check to see if the DataLoader has already been added to the GraphQL context; if not then add it for subsequent use
let dataLoader;
if (context.dataLoaders.has(dataLoaderMapKey)) {
dataLoader = context.dataLoaders.get(dataLoaderMapKey);
} else {
const batchedParamKey = rootParams[dataLoaderRootKey];
const batchedParamKey =
dataLoaderBatchParam || rootParams[primaryDataLoaderRootKey];

dataLoader = this.buildDataLoader(
context.ctx,
actionName,
batchedParamKey,
staticParams,
args
args,
{ hashCacheKey: dataLoaderUseAllRootKeys } // must hash the cache key if not loading scalar
);
context.dataLoaders.set(dataLoaderMapKey, dataLoader);
}

const rootValue = root && _.get(root, dataLoaderRootKey);
if (rootValue == null) {
let dataLoaderKey;
if (dataLoaderUseAllRootKeys) {
if (root && rootKeys) {
dataLoaderKey = {};

rootKeys.forEach(key => {
_.set(dataLoaderKey, rootParams[key], _.get(root, key));
});
}
} else {
dataLoaderKey = root && _.get(root, primaryDataLoaderRootKey);
}

if (dataLoaderKey == null) {
return null;
}

return Array.isArray(rootValue)
? await dataLoader.loadMany(rootValue)
: await dataLoader.load(rootValue);
return Array.isArray(dataLoaderKey)
? await dataLoader.loadMany(dataLoaderKey)
: await dataLoader.load(dataLoaderKey);
} else {
const params = {};
if (root && rootKeys) {
rootKeys.forEach(key =>
_.set(params, rootParams[key], _.get(root, key))
);
rootKeys.forEach(key => {
_.set(params, rootParams[key], _.get(root, key));
});
}

return await context.ctx.call(
Expand Down Expand Up @@ -223,21 +242,31 @@ module.exports = function(mixinOptions) {
* @param {string} batchedParamKey - Parameter key to use for loaded values
* @param {Object} staticParams - Static parameters to use in dataloader
* @param {Object} args - Arguments passed to GraphQL child resolver
* @param {Object} [options={}] - Optional arguments
* @param {Boolean} [options.hashCacheKey=false] - Use a hash for the cacheKeyFn
* @returns {DataLoader} Dataloader instance
*/
buildDataLoader(ctx, actionName, batchedParamKey, staticParams, args) {
buildDataLoader(
ctx,
actionName,
batchedParamKey,
staticParams,
args,
{ hashCacheKey = false } = {}
) {
const batchLoadFn = keys => {
const rootParams = { [batchedParamKey]: keys };
return ctx.call(actionName, _.defaultsDeep({}, args, rootParams, staticParams));
};

if (this.dataLoaderOptions.has(actionName)) {
// use any specified options assigned to this action
const options = this.dataLoaderOptions.get(actionName);
return new DataLoader(batchLoadFn, options);
}
const dataLoaderOptions = this.dataLoaderOptions.get(actionName) || {};
const cacheKeyFn = hashCacheKey && (key => hash(key));
const options = {
...(cacheKeyFn && { cacheKeyFn }),
...dataLoaderOptions,
};

return new DataLoader(batchLoadFn);
return new DataLoader(batchLoadFn, options);
},

/**
Expand Down Expand Up @@ -577,23 +606,39 @@ module.exports = function(mixinOptions) {
*
* @param {Object[]} services
* @modifies {this.dataLoaderOptions}
* @modifies {this.dataLoaderBatchParams}
*/
buildLoaderOptionMap(services) {
this.dataLoaderOptions.clear(); // clear map before rebuilding
this.dataLoaderBatchParams.clear(); // clear map before rebuilding

services.forEach(service => {
Object.values(service.actions).forEach(action => {
const { graphql: graphqlDefinition, name: actionName } = action;
if (graphqlDefinition && graphqlDefinition.dataLoaderOptions) {
if (
graphqlDefinition &&
(graphqlDefinition.dataLoaderOptions ||
graphqlDefinition.dataLoaderBatchParam)
) {
const serviceName = this.getServiceName(service);
const fullActionName = this.getResolverActionName(
serviceName,
actionName
);
this.dataLoaderOptions.set(
fullActionName,
graphqlDefinition.dataLoaderOptions
);

if (graphqlDefinition.dataLoaderOptions) {
this.dataLoaderOptions.set(
fullActionName,
graphqlDefinition.dataLoaderOptions
);
}

if (graphqlDefinition.dataLoaderBatchParam) {
this.dataLoaderBatchParams.set(
fullActionName,
graphqlDefinition.dataLoaderBatchParam
);
}
}
});
});
Expand All @@ -607,6 +652,7 @@ module.exports = function(mixinOptions) {
this.pubsub = null;
this.shouldUpdateGraphqlSchema = true;
this.dataLoaderOptions = new Map();
this.dataLoaderBatchParams = new Map();

const route = _.defaultsDeep(mixinOptions.routeOptions, {
aliases: {
Expand Down
95 changes: 91 additions & 4 deletions test/unit/service.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ describe("Test Service", () => {

it("should not invalidate schema when autoUpdateSchema is false", async () => {
const { broker, svc, stop } = await startService({
autoUpdateSchema: false
autoUpdateSchema: false,
});
svc.invalidateGraphQLSchema = jest.fn();

Expand All @@ -223,7 +223,7 @@ describe("Test Service", () => {

it("should not invalidate schema when autoUpdateSchema is true", async () => {
const { broker, svc, stop } = await startService({
autoUpdateSchema: true
autoUpdateSchema: true,
});
svc.invalidateGraphQLSchema = jest.fn();

Expand Down Expand Up @@ -548,6 +548,7 @@ describe("Test Service", () => {

beforeEach(() => {
svc.dataLoaderOptions.clear();
svc.dataLoaderBatchParams.clear();
});

it("should return null if no rootValue", async () => {
Expand Down Expand Up @@ -637,6 +638,79 @@ describe("Test Service", () => {
expect(ctx.call).toHaveBeenNthCalledWith(2, "users.resolve", { a: 5, id: [5] });
});

it("should call the action via the loader using all root params", async () => {
svc.dataLoaderBatchParams.set("users.resolve", "testBatchParam");
const resolver = svc.createActionResolver("users.resolve", {
rootParams: {
authorId: "authorIdParam",
testId: "testIdParam",
},

dataLoader: true,
});

const ctx = new Context(broker);
ctx.call = jest.fn().mockResolvedValue(["res1", "res2", "res3"]);

const fakeRoot1 = { authorId: 1, testId: "foo" };
const fakeRoot2 = { authorId: 2, testId: "bar" };
const fakeRoot3 = { authorId: 5, testId: "baz" };

const fakeContext = { ctx, dataLoaders: new Map() };
const res = await Promise.all([
resolver(fakeRoot1, {}, fakeContext),
resolver(fakeRoot2, {}, fakeContext),
resolver(fakeRoot3, {}, fakeContext),
]);

expect(res).toEqual(["res1", "res2", "res3"]);

expect(ctx.call).toHaveBeenCalledTimes(1);
expect(ctx.call).toHaveBeenNthCalledWith(1, "users.resolve", {
testBatchParam: [
{ authorIdParam: 1, testIdParam: "foo" },
{ authorIdParam: 2, testIdParam: "bar" },
{ authorIdParam: 5, testIdParam: "baz" },
],
});
});

it("should call the action via the loader using all root params while leveraging cache", async () => {
svc.dataLoaderBatchParams.set("users.resolve", "testBatchParam");
const resolver = svc.createActionResolver("users.resolve", {
rootParams: {
authorId: "authorIdParam",
testId: "testIdParam",
},

dataLoader: true,
});

const ctx = new Context(broker);
ctx.call = jest.fn().mockResolvedValue(["res1", "res2"]);

const fakeRoot1 = { authorId: 1, testId: "foo" };
const fakeRoot2 = { authorId: 2, testId: "bar" };
const fakeRoot3 = { authorId: 1, testId: "foo" }; // same as fakeRoot1

const fakeContext = { ctx, dataLoaders: new Map() };
const res = await Promise.all([
resolver(fakeRoot1, {}, fakeContext),
resolver(fakeRoot2, {}, fakeContext),
resolver(fakeRoot3, {}, fakeContext),
]);

expect(res).toEqual(["res1", "res2", "res1"]);

expect(ctx.call).toHaveBeenCalledTimes(1);
expect(ctx.call).toHaveBeenNthCalledWith(1, "users.resolve", {
testBatchParam: [
{ authorIdParam: 1, testIdParam: "foo" },
{ authorIdParam: 2, testIdParam: "bar" },
],
});
});

it("should reuse the loader for multiple calls with the same context", async () => {
const resolver = svc.createActionResolver("users.resolve", {
rootParams: {
Expand Down Expand Up @@ -1171,7 +1245,10 @@ describe("Test Service", () => {
actions: [
{
name: "test-action-1",
graphql: { dataLoaderOptions: { option1: "option-value-1" } },
graphql: {
dataLoaderOptions: { option1: "option-value-1" },
dataLoaderBatchParam: "batch-param-1",
},
},
{ name: "test-action-2" },
],
Expand All @@ -1182,7 +1259,10 @@ describe("Test Service", () => {
actions: [
{
name: "test-action-3",
graphql: { dataLoaderOptions: { option2: "option-value-2" } },
graphql: {
dataLoaderOptions: { option2: "option-value-2" },
dataLoaderBatchParam: "batch-param-2",
},
},
{ name: "test-action-4" },
],
Expand Down Expand Up @@ -1250,6 +1330,13 @@ describe("Test Service", () => {
])
);

expect(svc.dataLoaderBatchParams).toEqual(
new Map([
["test-svc-1.test-action-1", "batch-param-1"],
["v1.test-svc-2.test-action-3", "batch-param-2"],
])
);

// Test `context` method
const contextFn = ApolloServer.mock.calls[0][0].context;

Expand Down

0 comments on commit 65cbcfe

Please sign in to comment.