From a668a6f4af096137b31a0ec42ed5cefacbd27e78 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 23 Oct 2018 12:50:35 -0400 Subject: [PATCH 1/4] Throw when querying non-scalar objects without a selection set. This would have helped diagnose the cause of #4025, which is that query fields with non-scalar types (or list-of-non-scalar types) must have a selection set. In other words, given a schema like type Query { selectedMessageList: [SelectedMessage] } a query like query getSelectedMessageList { selectedMessageList @client } is invalid since it doesn't specify which fields of the SelectedMessage objects should be included in the list. --- .../src/readFromStore.ts | 54 +++++++++++++------ packages/apollo-utilities/src/storeUtils.ts | 4 +- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/packages/apollo-cache-inmemory/src/readFromStore.ts b/packages/apollo-cache-inmemory/src/readFromStore.ts index 5d92e094063..ef3afd950cb 100644 --- a/packages/apollo-cache-inmemory/src/readFromStore.ts +++ b/packages/apollo-cache-inmemory/src/readFromStore.ts @@ -401,18 +401,6 @@ export class StoreReader { info, ); - // Handle all scalar types here - if (!field.selectionSet) { - return readStoreResult; - } - - // From here down, the field has a selection set, which means it's trying to - // query a GraphQLObjectType - if (readStoreResult.result == null) { - // Basically any field in a GraphQL response can be null, or missing - return readStoreResult; - } - function handleMissing(res: ExecResult): ExecResult { let missing: ExecResultMissingField[] = null; @@ -440,6 +428,19 @@ export class StoreReader { )); } + // Handle all scalar types here + if (!field.selectionSet) { + assertSelectionSetForIdValue(field, readStoreResult.result); + return readStoreResult; + } + + // From here down, the field has a selection set, which means it's trying to + // query a GraphQLObjectType + if (readStoreResult.result == null) { + // Basically any field in a GraphQL response can be null, or missing + return readStoreResult; + } + // Returned value is an object, and the query has a sub-selection. Recurse. return handleMissing(this.executeSelectionSet({ selectionSet: field.selectionSet, @@ -476,17 +477,36 @@ export class StoreReader { } // This is an object, run the selection set on it - return handleMissing(this.executeSelectionSet({ - selectionSet: field.selectionSet, - rootValue: item, - execContext, - })); + if (field.selectionSet) { + return handleMissing(this.executeSelectionSet({ + selectionSet: field.selectionSet, + rootValue: item, + execContext, + })); + } + + assertSelectionSetForIdValue(field, item); + + return item; }); return { result, missing }; } } +function assertSelectionSetForIdValue( + field: FieldNode, + value: any, +) { + if (!field.selectionSet && isIdValue(value)) { + throw new Error( + `Missing selection set for object of type ${ + value.typename + } returned for query field ${field.name.value}` + ); + } +} + function defaultFragmentMatcher() { return true; } diff --git a/packages/apollo-utilities/src/storeUtils.ts b/packages/apollo-utilities/src/storeUtils.ts index bce295909aa..31e81c97665 100644 --- a/packages/apollo-utilities/src/storeUtils.ts +++ b/packages/apollo-utilities/src/storeUtils.ts @@ -271,7 +271,9 @@ export function isInlineFragment( } export function isIdValue(idObject: StoreValue): idObject is IdValue { - return idObject && (idObject as IdValue | JsonValue).type === 'id'; + return idObject && + (idObject as IdValue | JsonValue).type === 'id' && + typeof (idObject as IdValue).generated === 'boolean'; } export type IdConfig = { From 74c555cec8eb387e0771c5a5b60636cd7ed2bb6b Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 23 Oct 2018 13:37:23 -0400 Subject: [PATCH 2/4] Avoid declaring handleMissing function in execField method. --- .../src/readFromStore.ts | 61 ++++++++++--------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/packages/apollo-cache-inmemory/src/readFromStore.ts b/packages/apollo-cache-inmemory/src/readFromStore.ts index ef3afd950cb..8a244589675 100644 --- a/packages/apollo-cache-inmemory/src/readFromStore.ts +++ b/packages/apollo-cache-inmemory/src/readFromStore.ts @@ -401,31 +401,15 @@ export class StoreReader { info, ); - function handleMissing(res: ExecResult): ExecResult { - let missing: ExecResultMissingField[] = null; - - if (readStoreResult.missing) { - missing = missing || []; - missing.push(...readStoreResult.missing); - } - - if (res.missing) { - missing = missing || []; - missing.push(...res.missing); - } - - return { - result: res.result, - missing, - }; - } - if (Array.isArray(readStoreResult.result)) { - return handleMissing(this.executeSubSelectedArray( - field, - readStoreResult.result, - execContext, - )); + return this.combineExecResults( + readStoreResult, + this.executeSubSelectedArray( + field, + readStoreResult.result, + execContext, + ), + ); } // Handle all scalar types here @@ -442,11 +426,30 @@ export class StoreReader { } // Returned value is an object, and the query has a sub-selection. Recurse. - return handleMissing(this.executeSelectionSet({ - selectionSet: field.selectionSet, - rootValue: readStoreResult.result, - execContext, - })); + return this.combineExecResults( + readStoreResult, + this.executeSelectionSet({ + selectionSet: field.selectionSet, + rootValue: readStoreResult.result, + execContext, + }), + ); + } + + private combineExecResults( + ...execResults: ExecResult[] + ): ExecResult { + let missing: ExecResultMissingField[] = null; + execResults.forEach(execResult => { + if (execResult.missing) { + missing = missing || []; + missing.push(...execResult.missing); + } + }); + return { + result: execResults.pop().result, + missing, + }; } private executeSubSelectedArray( From 9ad87d47b9eb8f2ec2556700a5db23c27cbb101b Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 26 Oct 2018 13:20:03 -0400 Subject: [PATCH 3/4] Convert src/__tests__/diffQueryAgainstStore.ts from \r\n to \n endings. --- .../src/__tests__/diffAgainstStore.ts | 1946 ++++++++--------- 1 file changed, 973 insertions(+), 973 deletions(-) diff --git a/packages/apollo-cache-inmemory/src/__tests__/diffAgainstStore.ts b/packages/apollo-cache-inmemory/src/__tests__/diffAgainstStore.ts index 382ee3172fe..3c2be35cea8 100644 --- a/packages/apollo-cache-inmemory/src/__tests__/diffAgainstStore.ts +++ b/packages/apollo-cache-inmemory/src/__tests__/diffAgainstStore.ts @@ -1,973 +1,973 @@ -import gql, { disableFragmentWarnings } from 'graphql-tag'; -import { toIdValue } from 'apollo-utilities'; - -import { defaultNormalizedCacheFactory } from '../objectCache'; -import { StoreReader } from '../readFromStore'; -import { StoreWriter } from '../writeToStore'; -import { HeuristicFragmentMatcher } from '../fragmentMatcher'; -import { defaultDataIdFromObject } from '../inMemoryCache'; - -const fragmentMatcherFunction = new HeuristicFragmentMatcher().match; - -disableFragmentWarnings(); -export function withError(func: Function, regex: RegExp) { - let message: string = null as never; - const oldError = console.error; - - console.error = (m: string) => (message = m); - - try { - const result = func(); - expect(message).toMatch(regex); - return result; - } finally { - console.error = oldError; - } -} - -describe('diffing queries against the store', () => { - const reader = new StoreReader(); - const writer = new StoreWriter(); - - it( - 'expects named fragments to return complete as true when diffd against ' + - 'the store', - () => { - const store = defaultNormalizedCacheFactory({}); - - const queryResult = reader.diffQueryAgainstStore({ - store, - query: gql` - query foo { - ...root - } - - fragment root on Query { - nestedObj { - innerArray { - id - someField - } - } - } - `, - fragmentMatcherFunction, - config: { - dataIdFromObject: defaultDataIdFromObject, - }, - }); - - expect(queryResult.complete).toEqual(false); - }, - ); - - it( - 'expects inline fragments to return complete as true when diffd against ' + - 'the store', - () => { - const store = defaultNormalizedCacheFactory(); - - const queryResult = reader.diffQueryAgainstStore({ - store, - query: gql` - { - ... on DummyQuery { - nestedObj { - innerArray { - id - otherField - } - } - } - ... on Query { - nestedObj { - innerArray { - id - someField - } - } - } - ... on DummyQuery2 { - nestedObj { - innerArray { - id - otherField2 - } - } - } - } - `, - fragmentMatcherFunction, - config: { - dataIdFromObject: defaultDataIdFromObject, - }, - }); - - expect(queryResult.complete).toEqual(false); - }, - ); - - it('returns nothing when the store is enough', () => { - const query = gql` - { - people_one(id: "1") { - name - } - } - `; - - const result = { - people_one: { - name: 'Luke Skywalker', - }, - }; - - const store = writer.writeQueryToStore({ - result, - query, - }); - - expect( - reader.diffQueryAgainstStore({ - store, - query, - }).complete, - ).toBeTruthy(); - }); - - it('caches root queries both under the ID of the node and the query name', () => { - const firstQuery = gql` - { - people_one(id: "1") { - __typename - id - name - } - } - `; - - const result = { - people_one: { - __typename: 'Person', - id: '1', - name: 'Luke Skywalker', - }, - }; - - const getIdField = ({ id }: { id: string }) => id; - - const store = writer.writeQueryToStore({ - result, - query: firstQuery, - dataIdFromObject: getIdField, - }); - - const secondQuery = gql` - { - people_one(id: "1") { - __typename - id - name - } - } - `; - - const { complete } = reader.diffQueryAgainstStore({ - store, - query: secondQuery, - }); - - expect(complete).toBeTruthy(); - expect(store.get('1')).toEqual(result.people_one); - }); - - it('does not swallow errors other than field errors', () => { - const firstQuery = gql` - query { - person { - powers - } - } - `; - const firstResult = { - person: { - powers: 'the force', - }, - }; - const store = writer.writeQueryToStore({ - result: firstResult, - query: firstQuery, - }); - const unionQuery = gql` - query { - ...notARealFragment - } - `; - return expect(() => { - reader.diffQueryAgainstStore({ - store, - query: unionQuery, - }); - }).toThrowError(/No fragment/); - }); - - it('does not error on a correct query with union typed fragments', () => { - return withError(() => { - const firstQuery = gql` - query { - person { - __typename - firstName - lastName - } - } - `; - const firstResult = { - person: { - __typename: 'Author', - firstName: 'John', - lastName: 'Smith', - }, - }; - const store = writer.writeQueryToStore({ - result: firstResult, - query: firstQuery, - }); - const unionQuery = gql` - query { - person { - __typename - ... on Author { - firstName - lastName - } - ... on Jedi { - powers - } - } - } - `; - const { complete } = reader.diffQueryAgainstStore({ - store, - query: unionQuery, - returnPartialData: false, - fragmentMatcherFunction, - }); - - expect(complete).toBe(false); - }, /IntrospectionFragmentMatcher/); - }); - - it('does not error on a query with fields missing from all but one named fragment', () => { - const firstQuery = gql` - query { - person { - __typename - firstName - lastName - } - } - `; - const firstResult = { - person: { - __typename: 'Author', - firstName: 'John', - lastName: 'Smith', - }, - }; - const store = writer.writeQueryToStore({ - result: firstResult, - query: firstQuery, - }); - const unionQuery = gql` - query { - person { - __typename - ...authorInfo - ...jediInfo - } - } - - fragment authorInfo on Author { - firstName - } - - fragment jediInfo on Jedi { - powers - } - `; - - const { complete } = reader.diffQueryAgainstStore({ - store, - query: unionQuery, - }); - - expect(complete).toBe(false); - }); - - it('throws an error on a query with fields missing from matching named fragments', () => { - const firstQuery = gql` - query { - person { - __typename - firstName - lastName - } - } - `; - const firstResult = { - person: { - __typename: 'Author', - firstName: 'John', - lastName: 'Smith', - }, - }; - const store = writer.writeQueryToStore({ - result: firstResult, - query: firstQuery, - }); - const unionQuery = gql` - query { - person { - __typename - ...authorInfo2 - ...jediInfo2 - } - } - - fragment authorInfo2 on Author { - firstName - address - } - - fragment jediInfo2 on Jedi { - jedi - } - `; - expect(() => { - reader.diffQueryAgainstStore({ - store, - query: unionQuery, - returnPartialData: false, - }); - }).toThrow(); - }); - - it('returns available fields if returnPartialData is true', () => { - const firstQuery = gql` - { - people_one(id: "1") { - __typename - id - name - } - } - `; - - const firstResult = { - people_one: { - __typename: 'Person', - id: 'lukeId', - name: 'Luke Skywalker', - }, - }; - - const store = writer.writeQueryToStore({ - result: firstResult, - query: firstQuery, - }); - - // Variants on a simple query with a missing field. - - const simpleQuery = gql` - { - people_one(id: "1") { - name - age - } - } - `; - - const inlineFragmentQuery = gql` - { - people_one(id: "1") { - ... on Person { - name - age - } - } - } - `; - - const namedFragmentQuery = gql` - query { - people_one(id: "1") { - ...personInfo - } - } - - fragment personInfo on Person { - name - age - } - `; - - const simpleDiff = reader.diffQueryAgainstStore({ - store, - query: simpleQuery, - }); - - expect(simpleDiff.result).toEqual({ - people_one: { - name: 'Luke Skywalker', - }, - }); - - const inlineDiff = reader.diffQueryAgainstStore({ - store, - query: inlineFragmentQuery, - }); - - expect(inlineDiff.result).toEqual({ - people_one: { - name: 'Luke Skywalker', - }, - }); - - const namedDiff = reader.diffQueryAgainstStore({ - store, - query: namedFragmentQuery, - }); - - expect(namedDiff.result).toEqual({ - people_one: { - name: 'Luke Skywalker', - }, - }); - - expect(function() { - reader.diffQueryAgainstStore({ - store, - query: simpleQuery, - returnPartialData: false, - }); - }).toThrow(); - }); - - it('will add a private id property', () => { - const query = gql` - query { - a { - id - b - } - c { - d - e { - id - f - } - g { - h - } - } - } - `; - - const queryResult = { - a: [{ id: 'a:1', b: 1.1 }, { id: 'a:2', b: 1.2 }, { id: 'a:3', b: 1.3 }], - c: { - d: 2, - e: [ - { id: 'e:1', f: 3.1 }, - { id: 'e:2', f: 3.2 }, - { id: 'e:3', f: 3.3 }, - { id: 'e:4', f: 3.4 }, - { id: 'e:5', f: 3.5 }, - ], - g: { h: 4 }, - }, - }; - - function dataIdFromObject({ id }: { id: string}) { - return id; - } - - const store = writer.writeQueryToStore({ - query, - result: queryResult, - dataIdFromObject, - }); - - const { result } = reader.diffQueryAgainstStore({ - store, - query, - }); - - expect(result).toEqual(queryResult); - expect(dataIdFromObject(result.a[0])).toBe('a:1'); - expect(dataIdFromObject(result.a[1])).toBe('a:2'); - expect(dataIdFromObject(result.a[2])).toBe('a:3'); - expect(dataIdFromObject(result.c.e[0])).toBe('e:1'); - expect(dataIdFromObject(result.c.e[1])).toBe('e:2'); - expect(dataIdFromObject(result.c.e[2])).toBe('e:3'); - expect(dataIdFromObject(result.c.e[3])).toBe('e:4'); - expect(dataIdFromObject(result.c.e[4])).toBe('e:5'); - }); - - describe('referential equality preservation', () => { - it('will return the previous result if there are no changes', () => { - const query = gql` - query { - a { - b - } - c { - d - e { - f - } - } - } - `; - - const queryResult = { - a: { b: 1 }, - c: { d: 2, e: { f: 3 } }, - }; - - const store = writer.writeQueryToStore({ - query, - result: queryResult, - }); - - const previousResult = { - a: { b: 1 }, - c: { d: 2, e: { f: 3 } }, - }; - - const { result } = reader.diffQueryAgainstStore({ - store, - query, - previousResult, - }); - - expect(result).toEqual(queryResult); - expect(result).toEqual(previousResult); - }); - - it('will return parts of the previous result that changed', () => { - const query = gql` - query { - a { - b - } - c { - d - e { - f - } - } - } - `; - - const queryResult = { - a: { b: 1 }, - c: { d: 2, e: { f: 3 } }, - }; - - const store = writer.writeQueryToStore({ - query, - result: queryResult, - }); - - const previousResult = { - a: { b: 1 }, - c: { d: 20, e: { f: 3 } }, - }; - - const { result } = reader.diffQueryAgainstStore({ - store, - query, - previousResult, - }); - - expect(result).toEqual(queryResult); - expect(result).not.toEqual(previousResult); - expect(result.a).toEqual(previousResult.a); - expect(result.c).not.toEqual(previousResult.c); - expect(result.c.e).toEqual(previousResult.c.e); - }); - - it('will return the previous result if there are no changes in child arrays', () => { - const query = gql` - query { - a { - b - } - c { - d - e { - f - } - } - } - `; - - const queryResult = { - a: [{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }], - c: { - d: 2, - e: [{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }, { f: 3.4 }, { f: 3.5 }], - }, - }; - - const store = writer.writeQueryToStore({ - query, - result: queryResult, - }); - - const previousResult = { - a: [{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }], - c: { - d: 2, - e: [{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }, { f: 3.4 }, { f: 3.5 }], - }, - }; - - const { result } = reader.diffQueryAgainstStore({ - store, - query, - previousResult, - }); - - expect(result).toEqual(queryResult); - expect(result).toEqual(previousResult); - }); - - it('will not add zombie items when previousResult starts with the same items', () => { - const query = gql` - query { - a { - b - } - } - `; - - const queryResult = { - a: [{ b: 1.1 }, { b: 1.2 }], - }; - - const store = writer.writeQueryToStore({ - query, - result: queryResult, - }); - - const previousResult = { - a: [{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }], - }; - - const { result } = reader.diffQueryAgainstStore({ - store, - query, - previousResult, - }); - - expect(result).toEqual(queryResult); - expect(result.a[0]).toEqual(previousResult.a[0]); - expect(result.a[1]).toEqual(previousResult.a[1]); - }); - - it('will return the previous result if there are no changes in nested child arrays', () => { - const query = gql` - query { - a { - b - } - c { - d - e { - f - } - } - } - `; - - const queryResult = { - a: [[[[[{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }]]]]], - c: { - d: 2, - e: [[{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }], [{ f: 3.4 }, { f: 3.5 }]], - }, - }; - - const store = writer.writeQueryToStore({ - query, - result: queryResult, - }); - - const previousResult = { - a: [[[[[{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }]]]]], - c: { - d: 2, - e: [[{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }], [{ f: 3.4 }, { f: 3.5 }]], - }, - }; - - const { result } = reader.diffQueryAgainstStore({ - store, - query, - previousResult, - }); - - expect(result).toEqual(queryResult); - expect(result).toEqual(previousResult); - }); - - it('will return parts of the previous result if there are changes in child arrays', () => { - const query = gql` - query { - a { - b - } - c { - d - e { - f - } - } - } - `; - - const queryResult = { - a: [{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }], - c: { - d: 2, - e: [{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }, { f: 3.4 }, { f: 3.5 }], - }, - }; - - const store = writer.writeQueryToStore({ - query, - result: queryResult, - }); - - const previousResult = { - a: [{ b: 1.1 }, { b: -1.2 }, { b: 1.3 }], - c: { - d: 20, - e: [{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }, { f: 3.4 }, { f: 3.5 }], - }, - }; - - const { result } = reader.diffQueryAgainstStore({ - store, - query, - previousResult, - }); - - expect(result).toEqual(queryResult); - expect(result).not.toEqual(previousResult); - expect(result.a).not.toEqual(previousResult.a); - expect(result.a[0]).toEqual(previousResult.a[0]); - expect(result.a[1]).not.toEqual(previousResult.a[1]); - expect(result.a[2]).toEqual(previousResult.a[2]); - expect(result.c).not.toEqual(previousResult.c); - expect(result.c.e).toEqual(previousResult.c.e); - expect(result.c.e[0]).toEqual(previousResult.c.e[0]); - expect(result.c.e[1]).toEqual(previousResult.c.e[1]); - expect(result.c.e[2]).toEqual(previousResult.c.e[2]); - expect(result.c.e[3]).toEqual(previousResult.c.e[3]); - expect(result.c.e[4]).toEqual(previousResult.c.e[4]); - }); - - it('will return the same items in a different order with `dataIdFromObject`', () => { - const query = gql` - query { - a { - id - b - } - c { - d - e { - id - f - } - g { - h - } - } - } - `; - - const queryResult = { - a: [ - { id: 'a:1', b: 1.1 }, - { id: 'a:2', b: 1.2 }, - { id: 'a:3', b: 1.3 }, - ], - c: { - d: 2, - e: [ - { id: 'e:1', f: 3.1 }, - { id: 'e:2', f: 3.2 }, - { id: 'e:3', f: 3.3 }, - { id: 'e:4', f: 3.4 }, - { id: 'e:5', f: 3.5 }, - ], - g: { h: 4 }, - }, - }; - - const store = writer.writeQueryToStore({ - query, - result: queryResult, - dataIdFromObject: ({ id }: { id: string }) => id, - }); - - const previousResult = { - a: [ - { id: 'a:3', b: 1.3 }, - { id: 'a:2', b: 1.2 }, - { id: 'a:1', b: 1.1 }, - ], - c: { - d: 2, - e: [ - { id: 'e:4', f: 3.4 }, - { id: 'e:2', f: 3.2 }, - { id: 'e:5', f: 3.5 }, - { id: 'e:3', f: 3.3 }, - { id: 'e:1', f: 3.1 }, - ], - g: { h: 4 }, - }, - }; - - const { result } = reader.diffQueryAgainstStore({ - store, - query, - previousResult, - }); - - expect(result).toEqual(queryResult); - expect(result).not.toEqual(previousResult); - expect(result.a).not.toEqual(previousResult.a); - expect(result.a[0]).toEqual(previousResult.a[2]); - expect(result.a[1]).toEqual(previousResult.a[1]); - expect(result.a[2]).toEqual(previousResult.a[0]); - expect(result.c).not.toEqual(previousResult.c); - expect(result.c.e).not.toEqual(previousResult.c.e); - expect(result.c.e[0]).toEqual(previousResult.c.e[4]); - expect(result.c.e[1]).toEqual(previousResult.c.e[1]); - expect(result.c.e[2]).toEqual(previousResult.c.e[3]); - expect(result.c.e[3]).toEqual(previousResult.c.e[0]); - expect(result.c.e[4]).toEqual(previousResult.c.e[2]); - expect(result.c.g).toEqual(previousResult.c.g); - }); - - it('will return the same JSON scalar field object', () => { - const query = gql` - { - a { - b - c - } - d { - e - f - } - } - `; - - const queryResult = { - a: { b: 1, c: { x: 2, y: 3, z: 4 } }, - d: { e: 5, f: { x: 6, y: 7, z: 8 } }, - }; - - const store = writer.writeQueryToStore({ - query, - result: queryResult, - }); - - const previousResult = { - a: { b: 1, c: { x: 2, y: 3, z: 4 } }, - d: { e: 50, f: { x: 6, y: 7, z: 8 } }, - }; - - const { result } = reader.diffQueryAgainstStore({ - store, - query, - previousResult, - }); - - expect(result).toEqual(queryResult); - expect(result).not.toEqual(previousResult); - expect(result.a).toEqual(previousResult.a); - expect(result.d).not.toEqual(previousResult.d); - expect(result.d.f).toEqual(previousResult.d.f); - }); - it('will preserve equality with custom resolvers', () => { - const listQuery = gql` - { - people { - id - name - __typename - } - } - `; - - const listResult = { - people: [ - { - id: '4', - name: 'Luke Skywalker', - __typename: 'Person', - }, - ], - }; - - const itemQuery = gql` - { - person(id: 4) { - id - name - __typename - } - } - `; - - const dataIdFromObject = (obj: any) => obj.id; - - const store = writer.writeQueryToStore({ - query: listQuery, - result: listResult, - dataIdFromObject, - }); - - const previousResult = { - person: listResult.people[0], - }; - - const cacheRedirects = { - Query: { - person: (_: any, args: any) => - toIdValue({ id: args['id'], typename: 'Person' }), - }, - }; - - const config = { dataIdFromObject, cacheRedirects }; - - const { result } = reader.diffQueryAgainstStore({ - store, - query: itemQuery, - previousResult, - config, - }); - - expect(result).toEqual(previousResult); - }); - }); -}); +import gql, { disableFragmentWarnings } from 'graphql-tag'; +import { toIdValue } from 'apollo-utilities'; + +import { defaultNormalizedCacheFactory } from '../objectCache'; +import { StoreReader } from '../readFromStore'; +import { StoreWriter } from '../writeToStore'; +import { HeuristicFragmentMatcher } from '../fragmentMatcher'; +import { defaultDataIdFromObject } from '../inMemoryCache'; + +const fragmentMatcherFunction = new HeuristicFragmentMatcher().match; + +disableFragmentWarnings(); +export function withError(func: Function, regex: RegExp) { + let message: string = null as never; + const oldError = console.error; + + console.error = (m: string) => (message = m); + + try { + const result = func(); + expect(message).toMatch(regex); + return result; + } finally { + console.error = oldError; + } +} + +describe('diffing queries against the store', () => { + const reader = new StoreReader(); + const writer = new StoreWriter(); + + it( + 'expects named fragments to return complete as true when diffd against ' + + 'the store', + () => { + const store = defaultNormalizedCacheFactory({}); + + const queryResult = reader.diffQueryAgainstStore({ + store, + query: gql` + query foo { + ...root + } + + fragment root on Query { + nestedObj { + innerArray { + id + someField + } + } + } + `, + fragmentMatcherFunction, + config: { + dataIdFromObject: defaultDataIdFromObject, + }, + }); + + expect(queryResult.complete).toEqual(false); + }, + ); + + it( + 'expects inline fragments to return complete as true when diffd against ' + + 'the store', + () => { + const store = defaultNormalizedCacheFactory(); + + const queryResult = reader.diffQueryAgainstStore({ + store, + query: gql` + { + ... on DummyQuery { + nestedObj { + innerArray { + id + otherField + } + } + } + ... on Query { + nestedObj { + innerArray { + id + someField + } + } + } + ... on DummyQuery2 { + nestedObj { + innerArray { + id + otherField2 + } + } + } + } + `, + fragmentMatcherFunction, + config: { + dataIdFromObject: defaultDataIdFromObject, + }, + }); + + expect(queryResult.complete).toEqual(false); + }, + ); + + it('returns nothing when the store is enough', () => { + const query = gql` + { + people_one(id: "1") { + name + } + } + `; + + const result = { + people_one: { + name: 'Luke Skywalker', + }, + }; + + const store = writer.writeQueryToStore({ + result, + query, + }); + + expect( + reader.diffQueryAgainstStore({ + store, + query, + }).complete, + ).toBeTruthy(); + }); + + it('caches root queries both under the ID of the node and the query name', () => { + const firstQuery = gql` + { + people_one(id: "1") { + __typename + id + name + } + } + `; + + const result = { + people_one: { + __typename: 'Person', + id: '1', + name: 'Luke Skywalker', + }, + }; + + const getIdField = ({ id }: { id: string }) => id; + + const store = writer.writeQueryToStore({ + result, + query: firstQuery, + dataIdFromObject: getIdField, + }); + + const secondQuery = gql` + { + people_one(id: "1") { + __typename + id + name + } + } + `; + + const { complete } = reader.diffQueryAgainstStore({ + store, + query: secondQuery, + }); + + expect(complete).toBeTruthy(); + expect(store.get('1')).toEqual(result.people_one); + }); + + it('does not swallow errors other than field errors', () => { + const firstQuery = gql` + query { + person { + powers + } + } + `; + const firstResult = { + person: { + powers: 'the force', + }, + }; + const store = writer.writeQueryToStore({ + result: firstResult, + query: firstQuery, + }); + const unionQuery = gql` + query { + ...notARealFragment + } + `; + return expect(() => { + reader.diffQueryAgainstStore({ + store, + query: unionQuery, + }); + }).toThrowError(/No fragment/); + }); + + it('does not error on a correct query with union typed fragments', () => { + return withError(() => { + const firstQuery = gql` + query { + person { + __typename + firstName + lastName + } + } + `; + const firstResult = { + person: { + __typename: 'Author', + firstName: 'John', + lastName: 'Smith', + }, + }; + const store = writer.writeQueryToStore({ + result: firstResult, + query: firstQuery, + }); + const unionQuery = gql` + query { + person { + __typename + ... on Author { + firstName + lastName + } + ... on Jedi { + powers + } + } + } + `; + const { complete } = reader.diffQueryAgainstStore({ + store, + query: unionQuery, + returnPartialData: false, + fragmentMatcherFunction, + }); + + expect(complete).toBe(false); + }, /IntrospectionFragmentMatcher/); + }); + + it('does not error on a query with fields missing from all but one named fragment', () => { + const firstQuery = gql` + query { + person { + __typename + firstName + lastName + } + } + `; + const firstResult = { + person: { + __typename: 'Author', + firstName: 'John', + lastName: 'Smith', + }, + }; + const store = writer.writeQueryToStore({ + result: firstResult, + query: firstQuery, + }); + const unionQuery = gql` + query { + person { + __typename + ...authorInfo + ...jediInfo + } + } + + fragment authorInfo on Author { + firstName + } + + fragment jediInfo on Jedi { + powers + } + `; + + const { complete } = reader.diffQueryAgainstStore({ + store, + query: unionQuery, + }); + + expect(complete).toBe(false); + }); + + it('throws an error on a query with fields missing from matching named fragments', () => { + const firstQuery = gql` + query { + person { + __typename + firstName + lastName + } + } + `; + const firstResult = { + person: { + __typename: 'Author', + firstName: 'John', + lastName: 'Smith', + }, + }; + const store = writer.writeQueryToStore({ + result: firstResult, + query: firstQuery, + }); + const unionQuery = gql` + query { + person { + __typename + ...authorInfo2 + ...jediInfo2 + } + } + + fragment authorInfo2 on Author { + firstName + address + } + + fragment jediInfo2 on Jedi { + jedi + } + `; + expect(() => { + reader.diffQueryAgainstStore({ + store, + query: unionQuery, + returnPartialData: false, + }); + }).toThrow(); + }); + + it('returns available fields if returnPartialData is true', () => { + const firstQuery = gql` + { + people_one(id: "1") { + __typename + id + name + } + } + `; + + const firstResult = { + people_one: { + __typename: 'Person', + id: 'lukeId', + name: 'Luke Skywalker', + }, + }; + + const store = writer.writeQueryToStore({ + result: firstResult, + query: firstQuery, + }); + + // Variants on a simple query with a missing field. + + const simpleQuery = gql` + { + people_one(id: "1") { + name + age + } + } + `; + + const inlineFragmentQuery = gql` + { + people_one(id: "1") { + ... on Person { + name + age + } + } + } + `; + + const namedFragmentQuery = gql` + query { + people_one(id: "1") { + ...personInfo + } + } + + fragment personInfo on Person { + name + age + } + `; + + const simpleDiff = reader.diffQueryAgainstStore({ + store, + query: simpleQuery, + }); + + expect(simpleDiff.result).toEqual({ + people_one: { + name: 'Luke Skywalker', + }, + }); + + const inlineDiff = reader.diffQueryAgainstStore({ + store, + query: inlineFragmentQuery, + }); + + expect(inlineDiff.result).toEqual({ + people_one: { + name: 'Luke Skywalker', + }, + }); + + const namedDiff = reader.diffQueryAgainstStore({ + store, + query: namedFragmentQuery, + }); + + expect(namedDiff.result).toEqual({ + people_one: { + name: 'Luke Skywalker', + }, + }); + + expect(function() { + reader.diffQueryAgainstStore({ + store, + query: simpleQuery, + returnPartialData: false, + }); + }).toThrow(); + }); + + it('will add a private id property', () => { + const query = gql` + query { + a { + id + b + } + c { + d + e { + id + f + } + g { + h + } + } + } + `; + + const queryResult = { + a: [{ id: 'a:1', b: 1.1 }, { id: 'a:2', b: 1.2 }, { id: 'a:3', b: 1.3 }], + c: { + d: 2, + e: [ + { id: 'e:1', f: 3.1 }, + { id: 'e:2', f: 3.2 }, + { id: 'e:3', f: 3.3 }, + { id: 'e:4', f: 3.4 }, + { id: 'e:5', f: 3.5 }, + ], + g: { h: 4 }, + }, + }; + + function dataIdFromObject({ id }: { id: string}) { + return id; + } + + const store = writer.writeQueryToStore({ + query, + result: queryResult, + dataIdFromObject, + }); + + const { result } = reader.diffQueryAgainstStore({ + store, + query, + }); + + expect(result).toEqual(queryResult); + expect(dataIdFromObject(result.a[0])).toBe('a:1'); + expect(dataIdFromObject(result.a[1])).toBe('a:2'); + expect(dataIdFromObject(result.a[2])).toBe('a:3'); + expect(dataIdFromObject(result.c.e[0])).toBe('e:1'); + expect(dataIdFromObject(result.c.e[1])).toBe('e:2'); + expect(dataIdFromObject(result.c.e[2])).toBe('e:3'); + expect(dataIdFromObject(result.c.e[3])).toBe('e:4'); + expect(dataIdFromObject(result.c.e[4])).toBe('e:5'); + }); + + describe('referential equality preservation', () => { + it('will return the previous result if there are no changes', () => { + const query = gql` + query { + a { + b + } + c { + d + e { + f + } + } + } + `; + + const queryResult = { + a: { b: 1 }, + c: { d: 2, e: { f: 3 } }, + }; + + const store = writer.writeQueryToStore({ + query, + result: queryResult, + }); + + const previousResult = { + a: { b: 1 }, + c: { d: 2, e: { f: 3 } }, + }; + + const { result } = reader.diffQueryAgainstStore({ + store, + query, + previousResult, + }); + + expect(result).toEqual(queryResult); + expect(result).toEqual(previousResult); + }); + + it('will return parts of the previous result that changed', () => { + const query = gql` + query { + a { + b + } + c { + d + e { + f + } + } + } + `; + + const queryResult = { + a: { b: 1 }, + c: { d: 2, e: { f: 3 } }, + }; + + const store = writer.writeQueryToStore({ + query, + result: queryResult, + }); + + const previousResult = { + a: { b: 1 }, + c: { d: 20, e: { f: 3 } }, + }; + + const { result } = reader.diffQueryAgainstStore({ + store, + query, + previousResult, + }); + + expect(result).toEqual(queryResult); + expect(result).not.toEqual(previousResult); + expect(result.a).toEqual(previousResult.a); + expect(result.c).not.toEqual(previousResult.c); + expect(result.c.e).toEqual(previousResult.c.e); + }); + + it('will return the previous result if there are no changes in child arrays', () => { + const query = gql` + query { + a { + b + } + c { + d + e { + f + } + } + } + `; + + const queryResult = { + a: [{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }], + c: { + d: 2, + e: [{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }, { f: 3.4 }, { f: 3.5 }], + }, + }; + + const store = writer.writeQueryToStore({ + query, + result: queryResult, + }); + + const previousResult = { + a: [{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }], + c: { + d: 2, + e: [{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }, { f: 3.4 }, { f: 3.5 }], + }, + }; + + const { result } = reader.diffQueryAgainstStore({ + store, + query, + previousResult, + }); + + expect(result).toEqual(queryResult); + expect(result).toEqual(previousResult); + }); + + it('will not add zombie items when previousResult starts with the same items', () => { + const query = gql` + query { + a { + b + } + } + `; + + const queryResult = { + a: [{ b: 1.1 }, { b: 1.2 }], + }; + + const store = writer.writeQueryToStore({ + query, + result: queryResult, + }); + + const previousResult = { + a: [{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }], + }; + + const { result } = reader.diffQueryAgainstStore({ + store, + query, + previousResult, + }); + + expect(result).toEqual(queryResult); + expect(result.a[0]).toEqual(previousResult.a[0]); + expect(result.a[1]).toEqual(previousResult.a[1]); + }); + + it('will return the previous result if there are no changes in nested child arrays', () => { + const query = gql` + query { + a { + b + } + c { + d + e { + f + } + } + } + `; + + const queryResult = { + a: [[[[[{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }]]]]], + c: { + d: 2, + e: [[{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }], [{ f: 3.4 }, { f: 3.5 }]], + }, + }; + + const store = writer.writeQueryToStore({ + query, + result: queryResult, + }); + + const previousResult = { + a: [[[[[{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }]]]]], + c: { + d: 2, + e: [[{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }], [{ f: 3.4 }, { f: 3.5 }]], + }, + }; + + const { result } = reader.diffQueryAgainstStore({ + store, + query, + previousResult, + }); + + expect(result).toEqual(queryResult); + expect(result).toEqual(previousResult); + }); + + it('will return parts of the previous result if there are changes in child arrays', () => { + const query = gql` + query { + a { + b + } + c { + d + e { + f + } + } + } + `; + + const queryResult = { + a: [{ b: 1.1 }, { b: 1.2 }, { b: 1.3 }], + c: { + d: 2, + e: [{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }, { f: 3.4 }, { f: 3.5 }], + }, + }; + + const store = writer.writeQueryToStore({ + query, + result: queryResult, + }); + + const previousResult = { + a: [{ b: 1.1 }, { b: -1.2 }, { b: 1.3 }], + c: { + d: 20, + e: [{ f: 3.1 }, { f: 3.2 }, { f: 3.3 }, { f: 3.4 }, { f: 3.5 }], + }, + }; + + const { result } = reader.diffQueryAgainstStore({ + store, + query, + previousResult, + }); + + expect(result).toEqual(queryResult); + expect(result).not.toEqual(previousResult); + expect(result.a).not.toEqual(previousResult.a); + expect(result.a[0]).toEqual(previousResult.a[0]); + expect(result.a[1]).not.toEqual(previousResult.a[1]); + expect(result.a[2]).toEqual(previousResult.a[2]); + expect(result.c).not.toEqual(previousResult.c); + expect(result.c.e).toEqual(previousResult.c.e); + expect(result.c.e[0]).toEqual(previousResult.c.e[0]); + expect(result.c.e[1]).toEqual(previousResult.c.e[1]); + expect(result.c.e[2]).toEqual(previousResult.c.e[2]); + expect(result.c.e[3]).toEqual(previousResult.c.e[3]); + expect(result.c.e[4]).toEqual(previousResult.c.e[4]); + }); + + it('will return the same items in a different order with `dataIdFromObject`', () => { + const query = gql` + query { + a { + id + b + } + c { + d + e { + id + f + } + g { + h + } + } + } + `; + + const queryResult = { + a: [ + { id: 'a:1', b: 1.1 }, + { id: 'a:2', b: 1.2 }, + { id: 'a:3', b: 1.3 }, + ], + c: { + d: 2, + e: [ + { id: 'e:1', f: 3.1 }, + { id: 'e:2', f: 3.2 }, + { id: 'e:3', f: 3.3 }, + { id: 'e:4', f: 3.4 }, + { id: 'e:5', f: 3.5 }, + ], + g: { h: 4 }, + }, + }; + + const store = writer.writeQueryToStore({ + query, + result: queryResult, + dataIdFromObject: ({ id }: { id: string }) => id, + }); + + const previousResult = { + a: [ + { id: 'a:3', b: 1.3 }, + { id: 'a:2', b: 1.2 }, + { id: 'a:1', b: 1.1 }, + ], + c: { + d: 2, + e: [ + { id: 'e:4', f: 3.4 }, + { id: 'e:2', f: 3.2 }, + { id: 'e:5', f: 3.5 }, + { id: 'e:3', f: 3.3 }, + { id: 'e:1', f: 3.1 }, + ], + g: { h: 4 }, + }, + }; + + const { result } = reader.diffQueryAgainstStore({ + store, + query, + previousResult, + }); + + expect(result).toEqual(queryResult); + expect(result).not.toEqual(previousResult); + expect(result.a).not.toEqual(previousResult.a); + expect(result.a[0]).toEqual(previousResult.a[2]); + expect(result.a[1]).toEqual(previousResult.a[1]); + expect(result.a[2]).toEqual(previousResult.a[0]); + expect(result.c).not.toEqual(previousResult.c); + expect(result.c.e).not.toEqual(previousResult.c.e); + expect(result.c.e[0]).toEqual(previousResult.c.e[4]); + expect(result.c.e[1]).toEqual(previousResult.c.e[1]); + expect(result.c.e[2]).toEqual(previousResult.c.e[3]); + expect(result.c.e[3]).toEqual(previousResult.c.e[0]); + expect(result.c.e[4]).toEqual(previousResult.c.e[2]); + expect(result.c.g).toEqual(previousResult.c.g); + }); + + it('will return the same JSON scalar field object', () => { + const query = gql` + { + a { + b + c + } + d { + e + f + } + } + `; + + const queryResult = { + a: { b: 1, c: { x: 2, y: 3, z: 4 } }, + d: { e: 5, f: { x: 6, y: 7, z: 8 } }, + }; + + const store = writer.writeQueryToStore({ + query, + result: queryResult, + }); + + const previousResult = { + a: { b: 1, c: { x: 2, y: 3, z: 4 } }, + d: { e: 50, f: { x: 6, y: 7, z: 8 } }, + }; + + const { result } = reader.diffQueryAgainstStore({ + store, + query, + previousResult, + }); + + expect(result).toEqual(queryResult); + expect(result).not.toEqual(previousResult); + expect(result.a).toEqual(previousResult.a); + expect(result.d).not.toEqual(previousResult.d); + expect(result.d.f).toEqual(previousResult.d.f); + }); + it('will preserve equality with custom resolvers', () => { + const listQuery = gql` + { + people { + id + name + __typename + } + } + `; + + const listResult = { + people: [ + { + id: '4', + name: 'Luke Skywalker', + __typename: 'Person', + }, + ], + }; + + const itemQuery = gql` + { + person(id: 4) { + id + name + __typename + } + } + `; + + const dataIdFromObject = (obj: any) => obj.id; + + const store = writer.writeQueryToStore({ + query: listQuery, + result: listResult, + dataIdFromObject, + }); + + const previousResult = { + person: listResult.people[0], + }; + + const cacheRedirects = { + Query: { + person: (_: any, args: any) => + toIdValue({ id: args['id'], typename: 'Person' }), + }, + }; + + const config = { dataIdFromObject, cacheRedirects }; + + const { result } = reader.diffQueryAgainstStore({ + store, + query: itemQuery, + previousResult, + config, + }); + + expect(result).toEqual(previousResult); + }); + }); +}); From 2e70c99e1a4d019c061afa9624266376a2cbd600 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 26 Oct 2018 13:20:51 -0400 Subject: [PATCH 4/4] Add a regression test for issue #4025 (fixed by PR #4038). As requested by @jbaxleyiii: https://github.com/apollographql/apollo-client/pull/4038#issuecomment-433055662 --- .../src/__tests__/diffAgainstStore.ts | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/packages/apollo-cache-inmemory/src/__tests__/diffAgainstStore.ts b/packages/apollo-cache-inmemory/src/__tests__/diffAgainstStore.ts index 3c2be35cea8..642d052a104 100644 --- a/packages/apollo-cache-inmemory/src/__tests__/diffAgainstStore.ts +++ b/packages/apollo-cache-inmemory/src/__tests__/diffAgainstStore.ts @@ -970,4 +970,59 @@ describe('diffing queries against the store', () => { expect(result).toEqual(previousResult); }); }); + + describe('malformed queries', () => { + it('throws for non-scalar query fields without selection sets', () => { + // Issue #4025, fixed by PR #4038. + + const validQuery = gql` + query getMessageList { + messageList { + id + __typename + message + } + } + `; + + const invalidQuery = gql` + query getMessageList { + # This field needs a selection set because its value is an array + # of non-scalar objects. + messageList + } + `; + + const store = writer.writeQueryToStore({ + query: validQuery, + result: { + messageList: [{ + id: 1, + __typename: "Message", + message: "hi" + }, { + id: 2, + __typename: "Message", + message: "hello" + }, { + id: 3, + __typename: "Message", + message: "hey" + }] + } + }); + + try { + reader.diffQueryAgainstStore({ + store, + query: invalidQuery, + }); + throw new Error('should have thrown'); + } catch (e) { + expect(e.message).toEqual( + 'Missing selection set for object of type Message returned for query field messageList' + ); + } + }); + }); });