From 44b404c445b4f8ffd390ef84951893dfacd8f504 Mon Sep 17 00:00:00 2001 From: Tim Leslie Date: Tue, 7 Jul 2020 15:14:54 +1000 Subject: [PATCH] Apply cache hints to mutation resolvers. --- .changeset/funny-students-smash.md | 6 ++ .../src/types/Relationship/Implementation.js | 3 +- .../types/Relationship/nested-mutations.js | 17 ++-- packages/keystone/lib/ListTypes/list.js | 85 +++++++++---------- packages/keystone/lib/providers/listAuth.js | 3 +- packages/keystone/tests/List.test.js | 26 +++--- 6 files changed, 76 insertions(+), 64 deletions(-) create mode 100644 .changeset/funny-students-smash.md diff --git a/.changeset/funny-students-smash.md b/.changeset/funny-students-smash.md new file mode 100644 index 00000000000..aabae083a14 --- /dev/null +++ b/.changeset/funny-students-smash.md @@ -0,0 +1,6 @@ +--- +'@keystonejs/fields': minor +'@keystonejs/keystone': minor +--- + +Applied cache hints to all Mutation resolvers. diff --git a/packages/fields/src/types/Relationship/Implementation.js b/packages/fields/src/types/Relationship/Implementation.js index 1457043e8ec..ef71cc34eb1 100644 --- a/packages/fields/src/types/Relationship/Implementation.js +++ b/packages/fields/src/types/Relationship/Implementation.js @@ -166,7 +166,7 @@ export class Relationship extends Implementation { * previous stored values, which means indecies may not match those passed in * `operations`. */ - async resolveNestedOperations(operations, item, context, getItem, mutationState) { + async resolveNestedOperations(operations, item, context, getItem, mutationState, info) { const { refList, refField } = this.tryResolveRefList(); const listInfo = { local: { list: this.getListByKey(this.listKey), field: this }, @@ -222,6 +222,7 @@ export class Relationship extends Implementation { many: this.many, context, mutationState, + info, }); return { create, connect, disconnect, currentValue }; diff --git a/packages/fields/src/types/Relationship/nested-mutations.js b/packages/fields/src/types/Relationship/nested-mutations.js index ba0624d4c0b..9191911dbf3 100644 --- a/packages/fields/src/types/Relationship/nested-mutations.js +++ b/packages/fields/src/types/Relationship/nested-mutations.js @@ -76,6 +76,7 @@ async function resolveNestedMany({ localField, target, mutationState, + info, }) { // Disconnections let disconnectIds = []; @@ -95,7 +96,7 @@ async function resolveNestedMany({ // This will resolve access control, etc for us. // In the future, when WhereUniqueInput accepts more than just an id, // this will also resolve those queries for us too. - const action = where => refList.itemQuery(where, context, refList.gqlNames.itemQueryName); + const action = where => refList.itemQuery(where, context, refList.gqlNames.itemQueryName, info); // We don't throw if any fail; we're only interested in the ones this user has // access to read (and hence remove from the list) const disconnectItems = (await pSettle((withoutId || []).map(action))) @@ -114,7 +115,7 @@ async function resolveNestedMany({ // In the future, when WhereUniqueInput accepts more than just an id, // this will also resolve those queries for us too. const [connectedItems, connectErrors] = await _runActions( - where => refList.itemQuery({ where }, context, refList.gqlNames.itemQueryName), + where => refList.itemQuery({ where }, context, refList.gqlNames.itemQueryName, info), input.connect, ['connect'] ); @@ -123,7 +124,7 @@ async function resolveNestedMany({ // NOTE: We don't check for read access control on the returned ids as the // user will not have seen it, so it's ok to return it directly here. const [createdItems, createErrors] = await _runActions( - data => refList.createMutation(data, context, mutationState), + data => refList.createMutation(data, context, info, mutationState), input.create, ['create'] ); @@ -162,6 +163,7 @@ async function resolveNestedSingle({ context, target, mutationState, + info, }) { let result_ = {}; if ((input.disconnect || input.disconnectAll) && currentValue) { @@ -177,7 +179,8 @@ async function resolveNestedSingle({ await refList.itemQuery( { where: input.disconnect }, context, - refList.gqlNames.itemQueryName + refList.gqlNames.itemQueryName, + info ) ).id.toString(); } catch (error) { @@ -199,10 +202,10 @@ async function resolveNestedSingle({ if (input.connect) { operation = 'connect'; method = () => - refList.itemQuery({ where: input.connect }, context, refList.gqlNames.itemQueryName); + refList.itemQuery({ where: input.connect }, context, refList.gqlNames.itemQueryName, info); } else if (input.create) { operation = 'create'; - method = () => refList.createMutation(input.create, context, mutationState); + method = () => refList.createMutation(input.create, context, info, mutationState); } if (operation) { @@ -237,6 +240,7 @@ export async function resolveNested({ listInfo, context, mutationState, + info, }) { const localList = listInfo.local.list; const localField = listInfo.local.field; @@ -250,6 +254,7 @@ export async function resolveNested({ localField, target, mutationState, + info, }; return await (many ? resolveNestedMany(args) : resolveNestedSingle(args)); } diff --git a/packages/keystone/lib/ListTypes/list.js b/packages/keystone/lib/ListTypes/list.js index 101f517391c..bf9834f1a3c 100644 --- a/packages/keystone/lib/ListTypes/list.js +++ b/packages/keystone/lib/ListTypes/list.js @@ -419,7 +419,7 @@ module.exports = class List { return item; } - async getAccessControlledItems(ids, access, { context, info } = {}) { + async getAccessControlledItems(ids, access, { context, info }) { if (ids.length === 0) { return []; } @@ -503,13 +503,7 @@ module.exports = class List { }; } - async itemQuery( - // prettier-ignore - { where: { id } }, - context, - gqlName, - info - ) { + async itemQuery({ where: { id } }, context, gqlName, info) { const operation = 'read'; graphqlLogger.debug({ id, operation, type: opToType[operation], gqlName }, 'Start query'); @@ -529,7 +523,7 @@ module.exports = class List { return result; } - async _itemsQuery(args, extra) { + async _itemsQuery(args, { info, ...extra }) { // This is private because it doesn't handle access control const { maxResults } = this.queryLimits; @@ -578,15 +572,15 @@ module.exports = class List { } } - if (extra && extra.info && extra.info.cacheControl) { + if (info && info.cacheControl) { switch (typeof this.cacheHint) { case 'object': - extra.info.cacheControl.setCacheHint(this.cacheHint); + info.cacheControl.setCacheHint(this.cacheHint); break; case 'function': - const operationName = extra.info.operation.name && extra.info.operation.name.value; - extra.info.cacheControl.setCacheHint( + const operationName = info.operation.name && info.operation.name.value; + info.cacheControl.setCacheHint( this.cacheHint({ results, operationName, meta: !!extra.meta }) ); break; @@ -606,7 +600,7 @@ module.exports = class List { .filter(field => field); } - async _resolveRelationship(data, existingItem, context, getItem, mutationState) { + async _resolveRelationship(data, existingItem, context, getItem, mutationState, info) { const fields = this._fieldsFromObject(data).filter(field => field.isRelationship); const resolvedRelationships = await mapToFields(fields, async field => { const { create, connect, disconnect, currentValue } = await field.resolveNestedOperations( @@ -614,7 +608,8 @@ module.exports = class List { existingItem, context, getItem, - mutationState + mutationState, + info ); // This code codifies the order of operations for nested mutations: // 1. disconnectAll @@ -690,7 +685,7 @@ module.exports = class List { return result; } - async createMutation(data, context, mutationState) { + async createMutation(data, context, info, mutationState) { const operation = 'create'; const gqlName = this.gqlNames.createMutationName; @@ -702,10 +697,10 @@ module.exports = class List { await this.checkFieldAccess(operation, itemsToUpdate, context, { gqlName }); - return await this._createSingle(data, existingItem, context, mutationState); + return await this._createSingle(data, existingItem, context, info, mutationState); } - async createManyMutation(data, context, mutationState) { + async createManyMutation(data, context, info, mutationState) { const operation = 'create'; const gqlName = this.gqlNames.createManyMutationName; @@ -716,11 +711,11 @@ module.exports = class List { await this.checkFieldAccess(operation, itemsToUpdate, context, { gqlName }); return Promise.all( - data.map(d => this._createSingle(d.data, undefined, context, mutationState)) + data.map(d => this._createSingle(d.data, undefined, context, info, mutationState)) ); } - async _createSingle(originalInput, existingItem, context, mutationState) { + async _createSingle(originalInput, existingItem, context, info, mutationState) { const operation = 'create'; return await this._nestedMutation(mutationState, context, async mutationState => { const defaultedItem = await this._resolveDefaults({ context, originalInput }); @@ -735,7 +730,8 @@ module.exports = class List { existingItem, context, createdPromise.promise, - mutationState + mutationState, + info ); resolvedData = await this.hookManager.resolveInput({ @@ -792,7 +788,7 @@ module.exports = class List { }); } - async updateMutation(id, data, context, mutationState) { + async updateMutation(id, data, context, info, mutationState) { const operation = 'update'; const gqlName = this.gqlNames.updateMutationName; const extraData = { gqlName, itemId: id }; @@ -803,16 +799,17 @@ module.exports = class List { context, operation, gqlName, + info, }); const itemsToUpdate = [{ existingItem, data }]; await this.checkFieldAccess(operation, itemsToUpdate, context, extraData); - return await this._updateSingle(id, data, existingItem, context, mutationState); + return await this._updateSingle(id, data, existingItem, context, info, mutationState); } - async updateManyMutation(data, context, mutationState) { + async updateManyMutation(data, context, info, mutationState) { const operation = 'update'; const gqlName = this.gqlNames.updateManyMutationName; const ids = data.map(d => d.id); @@ -820,7 +817,7 @@ module.exports = class List { const access = await this.checkListAccess(context, data, operation, extraData); - const existingItems = await this.getAccessControlledItems(ids, access); + const existingItems = await this.getAccessControlledItems(ids, access, { context, info }); const existingItemsById = arrayToObject(existingItems, 'id'); const itemsToUpdate = zipObj({ @@ -834,12 +831,12 @@ module.exports = class List { return Promise.all( itemsToUpdate.map(({ existingItem, id, data }) => - this._updateSingle(id, data, existingItem, context, mutationState) + this._updateSingle(id, data, existingItem, context, info, mutationState) ) ); } - async _updateSingle(id, originalInput, existingItem, context, mutationState) { + async _updateSingle(id, originalInput, existingItem, context, info, mutationState) { const operation = 'update'; return await this._nestedMutation(mutationState, context, async mutationState => { let resolvedData = await this._resolveRelationship( @@ -847,7 +844,8 @@ module.exports = class List { existingItem, context, undefined, - mutationState + mutationState, + info ); resolvedData = await this.hookManager.resolveInput({ @@ -890,7 +888,7 @@ module.exports = class List { }); } - async deleteMutation(id, context, mutationState) { + async deleteMutation(id, context, info, mutationState) { const operation = 'delete'; const gqlName = this.gqlNames.deleteMutationName; @@ -903,12 +901,13 @@ module.exports = class List { context, operation, gqlName, + info, }); return this._deleteSingle(existingItem, context, mutationState); } - async deleteManyMutation(ids, context, mutationState) { + async deleteManyMutation(ids, context, info, mutationState) { const operation = 'delete'; const gqlName = this.gqlNames.deleteManyMutationName; @@ -917,7 +916,7 @@ module.exports = class List { itemIds: ids, }); - const existingItems = await this.getAccessControlledItems(ids, access); + const existingItems = await this.getAccessControlledItems(ids, access, { context, info }); return Promise.all( existingItems.map(existingItem => this._deleteSingle(existingItem, context, mutationState)) @@ -1289,28 +1288,28 @@ module.exports = class List { const createFields = this.getFieldsWithAccess({ schemaName, access: 'create' }); if (schemaAccess.create && createFields.length) { - mutationResolvers[this.gqlNames.createMutationName] = (_, { data }, context) => - this.createMutation(data, context); + mutationResolvers[this.gqlNames.createMutationName] = (_, { data }, context, info) => + this.createMutation(data, context, info); - mutationResolvers[this.gqlNames.createManyMutationName] = (_, { data }, context) => - this.createManyMutation(data, context); + mutationResolvers[this.gqlNames.createManyMutationName] = (_, { data }, context, info) => + this.createManyMutation(data, context, info); } const updateFields = this.getFieldsWithAccess({ schemaName, access: 'update' }); if (schemaAccess.update && updateFields.length) { - mutationResolvers[this.gqlNames.updateMutationName] = (_, { id, data }, context) => - this.updateMutation(id, data, context); + mutationResolvers[this.gqlNames.updateMutationName] = (_, { id, data }, context, info) => + this.updateMutation(id, data, context, info); - mutationResolvers[this.gqlNames.updateManyMutationName] = (_, { data }, context) => - this.updateManyMutation(data, context); + mutationResolvers[this.gqlNames.updateManyMutationName] = (_, { data }, context, info) => + this.updateManyMutation(data, context, info); } if (schemaAccess.delete) { - mutationResolvers[this.gqlNames.deleteMutationName] = (_, { id }, context) => - this.deleteMutation(id, context); + mutationResolvers[this.gqlNames.deleteMutationName] = (_, { id }, context, info) => + this.deleteMutation(id, context, info); - mutationResolvers[this.gqlNames.deleteManyMutationName] = (_, { ids }, context) => - this.deleteManyMutation(ids, context); + mutationResolvers[this.gqlNames.deleteManyMutationName] = (_, { ids }, context, info) => + this.deleteManyMutation(ids, context, info); } return mutationResolvers; diff --git a/packages/keystone/lib/providers/listAuth.js b/packages/keystone/lib/providers/listAuth.js index 3b0867a8673..e2effff7514 100644 --- a/packages/keystone/lib/providers/listAuth.js +++ b/packages/keystone/lib/providers/listAuth.js @@ -118,7 +118,8 @@ class ListAuthProvider { return this.list.itemQuery( mergeWhereClause({ where: { id: context.authedItem.id } }, access), context, - this.gqlNames.authenticatedQueryName + this.gqlNames.authenticatedQueryName, + info ); } diff --git a/packages/keystone/tests/List.test.js b/packages/keystone/tests/List.test.js index bd4f7890cd0..12421203c24 100644 --- a/packages/keystone/tests/List.test.js +++ b/packages/keystone/tests/List.test.js @@ -888,43 +888,43 @@ test('getAccessControlledItem', async () => { test('getAccessControlledItems', async () => { const list = setup(); - expect(await list.getAccessControlledItems([], true)).toEqual([]); - expect(await list.getAccessControlledItems([1, 2], true)).toEqual([ + expect(await list.getAccessControlledItems([], true, {})).toEqual([]); + expect(await list.getAccessControlledItems([1, 2], true, {})).toEqual([ { name: 'b', email: 'b@example.com', index: 1 }, { name: 'c', email: 'c@example.com', index: 2 }, ]); - expect(await list.getAccessControlledItems([1, 2, 1, 2], true)).toEqual([ + expect(await list.getAccessControlledItems([1, 2, 1, 2], true, {})).toEqual([ { name: 'b', email: 'b@example.com', index: 1 }, { name: 'c', email: 'c@example.com', index: 2 }, ]); - expect(await list.getAccessControlledItems([1, 2], { id: 1 })).toEqual([ + expect(await list.getAccessControlledItems([1, 2], { id: 1 }, {})).toEqual([ { name: 'b', email: 'b@example.com', index: 1 }, ]); - expect(await list.getAccessControlledItems([1, 2], { id: 3 })).toEqual([]); + expect(await list.getAccessControlledItems([1, 2], { id: 3 }, {})).toEqual([]); - expect(await list.getAccessControlledItems([1, 2], { id_in: [1, 2, 3] })).toEqual([ + expect(await list.getAccessControlledItems([1, 2], { id_in: [1, 2, 3] }, {})).toEqual([ { name: 'b', email: 'b@example.com', index: 1 }, { name: 'c', email: 'c@example.com', index: 2 }, ]); - expect(await list.getAccessControlledItems([1, 2], { id_in: [2, 3] })).toEqual([ + expect(await list.getAccessControlledItems([1, 2], { id_in: [2, 3] }, {})).toEqual([ { name: 'c', email: 'c@example.com', index: 2 }, ]); - expect(await list.getAccessControlledItems([1, 2], { id_in: [3, 4] })).toEqual([]); + expect(await list.getAccessControlledItems([1, 2], { id_in: [3, 4] }, {})).toEqual([]); - expect(await list.getAccessControlledItems([1, 2], { id_not: 2 })).toEqual([ + expect(await list.getAccessControlledItems([1, 2], { id_not: 2 }, {})).toEqual([ { name: 'b', email: 'b@example.com', index: 1 }, ]); - expect(await list.getAccessControlledItems([1, 2], { id_not: 3 })).toEqual([ + expect(await list.getAccessControlledItems([1, 2], { id_not: 3 }, {})).toEqual([ { name: 'b', email: 'b@example.com', index: 1 }, { name: 'c', email: 'c@example.com', index: 2 }, ]); - expect(await list.getAccessControlledItems([1, 2], { id_not_in: [1, 2, 3] })).toEqual([]); - expect(await list.getAccessControlledItems([1, 2], { id_not_in: [2, 3] })).toEqual([ + expect(await list.getAccessControlledItems([1, 2], { id_not_in: [1, 2, 3] }, {})).toEqual([]); + expect(await list.getAccessControlledItems([1, 2], { id_not_in: [2, 3] }, {})).toEqual([ { name: 'b', email: 'b@example.com', index: 1 }, ]); - expect(await list.getAccessControlledItems([1, 2], { id_not_in: [3, 4] })).toEqual([ + expect(await list.getAccessControlledItems([1, 2], { id_not_in: [3, 4] }, {})).toEqual([ { name: 'b', email: 'b@example.com', index: 1 }, { name: 'c', email: 'c@example.com', index: 2 }, ]);