Skip to content

Commit

Permalink
Run Policies's merge before cache merge
Browse files Browse the repository at this point in the history
adds test

fix formatting
  • Loading branch information
gastonmorixe committed Mar 8, 2022
1 parent 6ca525a commit 045a9d5
Show file tree
Hide file tree
Showing 8 changed files with 251 additions and 6 deletions.
58 changes: 58 additions & 0 deletions src/cache/inmemory/__tests__/__snapshots__/writeToStore.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,64 @@ Object {
}
`;
exports[`writing to the store root type policy merge is called before cache deep merge 1`] = `
Array [
Array [
Object {
"existing": Object {
"__ref": "Person:123",
},
"incoming": Object {
"__typename": "Person",
"age": 28,
"id": 123,
"name": "Gaston",
"status": "ACTIVE",
"updatedAt": 100000,
},
"times": 1,
},
],
Array [
Object {
"existing": undefined,
"incoming": Object {
"__ref": "Person:123",
},
"times": 2,
},
],
Array [
Object {
"existing": Object {
"__ref": "Person:123",
},
"incoming": Object {
"__typename": "Person",
"id": 123,
"status": "DISABLED",
"updatedAt": 50,
},
"times": 3,
},
],
Array [
Object {
"existing": Object {
"__ref": "Person:123",
},
"incoming": Object {
"__typename": "Person",
"id": 123,
"status": "PENDING",
"updatedAt": 100001,
},
"times": 4,
},
],
]
`;
exports[`writing to the store should not keep reference when type of mixed inlined field changes to non-inlined field 1`] = `
[MockFunction] {
"calls": Array [
Expand Down
165 changes: 165 additions & 0 deletions src/cache/inmemory/__tests__/writeToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2496,6 +2496,20 @@ describe('writing to the store', () => {
},
},

{
existing: {
__ref: "Account:12345",
},
incoming: {
__typename: "Account",
contact: "support@example.com",
id: 12345,
},
merged: {
__ref: "Account:12345",
},
},

{
existing: {
__typename: "Account",
Expand Down Expand Up @@ -2923,6 +2937,157 @@ describe('writing to the store', () => {
});
});

it("root type policy merge is called before cache deep merge", () => {
const personMergeMock = jest.fn();

let times = 0;
const cache = new InMemoryCache({
typePolicies: {
Person: {
merge(existing, incoming, tools) {
times++;

personMergeMock({
times,
existing,
incoming,
});

if (tools.isReference(existing) && !tools.isReference(incoming)) {
const cachedData = tools.cache.data.lookup(existing.__ref);
const existingUpdatedAt = cachedData?.["updatedAt"];
const incomingUpdatedAt = incoming?.["updatedAt"];
if (
typeof existingUpdatedAt === "number" &&
typeof incomingUpdatedAt === "number" &&
existingUpdatedAt > incomingUpdatedAt
) {
return existing;
}
}

return tools.mergeObjects(existing, incoming);
},
},
},
});

expect(times).toEqual(0);

const query = gql`
query Person($offset: Int, $limit: Int) {
person {
id
name
age
status
updatedAt
}
}
`

expect(times).toEqual(0);

cache.writeQuery({
query,
data: {
person: {
__typename: "Person",
id: 123,
name: "Gaston",
age: 28,
status: "ACTIVE",
updatedAt: 100000,
},
},
variables: {},
});

expect(times).toEqual(2); // TODO: ideally this should only be called once

expect(cache.extract()).toEqual({
ROOT_QUERY: {
__typename: "Query",
person: {
__ref: "Person:123",
},
},
"Person:123": {
__typename: "Person",
id: 123,
name: "Gaston",
age: 28,
status: "ACTIVE",
updatedAt: 100000,
},
});

cache.writeQuery({
query,
data: {
person: {
__typename: "Person",
id: 123,
status: "DISABLED",
updatedAt: 50,
},
},
variables: {},
});

expect(times).toEqual(3);

expect(cache.extract()).toEqual({
ROOT_QUERY: {
__typename: "Query",
person: {
__ref: "Person:123",
},
},
"Person:123": {
__typename: "Person",
id: 123,
name: "Gaston",
age: 28,
status: "ACTIVE",
updatedAt: 100000,
},
});

cache.writeQuery({
query,
data: {
person: {
__typename: "Person",
id: 123,
status: "PENDING",
updatedAt: 100001,
},
},
variables: {},
});

expect(personMergeMock.mock.calls).toMatchSnapshot();
expect(times).toEqual(4);

expect(cache.extract()).toEqual({
ROOT_QUERY: {
__typename: "Query",
person: {
__ref: "Person:123",
},
},
"Person:123": {
__typename: "Person",
id: 123,
name: "Gaston",
age: 28,
status: "PENDING",
updatedAt: 100001,
},
});
});

describe("StoreWriter", () => {
const writer = new StoreWriter(new InMemoryCache());

Expand Down
2 changes: 1 addition & 1 deletion src/cache/inmemory/entityStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export abstract class EntityStore implements NormalizedCache {
}
}

protected lookup(dataId: string, dependOnExistence?: boolean): StoreObject | undefined {
public lookup(dataId: string, dependOnExistence?: boolean): StoreObject | undefined {
// The has method (above) calls lookup with dependOnExistence = true, so
// that it can later be invalidated when we add or remove a StoreObject for
// this dataId. Any consumer who cares about the contents of the StoreObject
Expand Down
2 changes: 1 addition & 1 deletion src/cache/inmemory/inMemoryCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type BroadcastOptions = Pick<
>

export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
private data: EntityStore;
public data: EntityStore;
private optimisticData: EntityStore;

protected config: InMemoryCacheConfig;
Expand Down
7 changes: 5 additions & 2 deletions src/cache/inmemory/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ export interface FieldFunctionOptions<
// helper function can be used to merge objects in a way that respects any
// custom merge functions defined for their fields.
mergeObjects: MergeObjectsFunction;

context: ReadMergeModifyContext | undefined;
}

type MergeObjectsFunction = <T extends StoreObject | Reference>(
Expand Down Expand Up @@ -542,7 +544,7 @@ export class Policies {
});
}

private getTypePolicy(typename: string): Policies["typePolicies"][string] {
public getTypePolicy(typename: string): Policies["typePolicies"][string] {
if (!hasOwn.call(this.typePolicies, typename)) {
const policy: Policies["typePolicies"][string] =
this.typePolicies[typename] = Object.create(null);
Expand Down Expand Up @@ -878,7 +880,7 @@ export class Policies {
// that need to deduplicate child objects and references.
void 0,
{ typename,
fieldName: field.name.value,
fieldName: field?.name.value || "ROOT",
field,
variables: context.variables },
context,
Expand Down Expand Up @@ -917,6 +919,7 @@ function makeFieldFunctionOptions(
);
},
mergeObjects: makeMergeObjectsFunction(context.store),
context
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/cache/inmemory/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export interface InMemoryCacheConfig extends ApolloReducerConfig {
}

export interface MergeInfo {
field: FieldNode;
field: FieldNode | undefined;
typename: string | undefined;
merge: FieldMergeFunction;
};
Expand Down
20 changes: 19 additions & 1 deletion src/cache/inmemory/writeToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export class StoreWriter {
context.incomingById.forEach(({ storeObject, mergeTree, fieldNodeSet }, dataId) => {
const entityRef = makeReference(dataId);

if (mergeTree && mergeTree.map.size) {
if (mergeTree && (mergeTree.map.size || mergeTree.info)) {
const applied = this.applyMerges(mergeTree, entityRef, storeObject, context);
if (isReference(applied)) {
// Assume References returned by applyMerges have already been merged
Expand Down Expand Up @@ -421,6 +421,21 @@ export class StoreWriter {
previous.mergeTree = mergeMergeTrees(previous.mergeTree, mergeTree);
fieldNodeSet.forEach(field => previous.fieldNodeSet.add(field));
} else {
// Add the policy type's merge function for individual upcoming payloads
if(typename && mergeTreeIsEmpty(mergeTree)) {
const typePolicy = policies.getTypePolicy(
typename,
);
const merge = typePolicy.merge;
if (merge) {
mergeTree.info = {
field: undefined as any,
typename,
merge,
};
}
}

context.incomingById.set(dataId, {
storeObject: incoming,
// Save a reference to mergeTree only if it is not empty, because
Expand Down Expand Up @@ -660,6 +675,9 @@ export class StoreWriter {
}

if (mergeTree.info) {
if(isReference(existing) && isReference(incoming)){
return incoming;
}
return this.cache.policies.runMergeFunction(
existing,
incoming,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ describe('relayStylePagination', () => {
readField: () => undefined,
canRead: () => false,
mergeObjects: (existing, _incoming) => existing,
context: undefined,
};

it('should maintain endCursor and startCursor with empty edges', () => {
Expand Down

0 comments on commit 045a9d5

Please sign in to comment.