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

[Data masking] Warn when passing object to useFragment/watchFragment from that is not identifiable #12004

Merged
merged 7 commits into from
Aug 16, 2024
226 changes: 226 additions & 0 deletions src/__tests__/dataMasking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1076,6 +1076,232 @@ test("warns when accessing a unmasked field while using @unmask with mode: 'migr
}
});

test("reads fragment by passing parent object to `from`", async () => {
interface Query {
currentUser: {
__typename: "User";
id: number;
name: string;
};
}

interface Fragment {
age: number;
}

const fragment: TypedDocumentNode<Fragment, never> = gql`
fragment UserFields on User {
age
}
`;

const query: TypedDocumentNode<Query, never> = gql`
query MaskedQuery {
currentUser {
id
name
...UserFields
}
}

${fragment}
`;

const mocks = [
{
request: { query },
result: {
data: {
currentUser: {
__typename: "User",
id: 1,
name: "Test User",
age: 30,
},
},
},
},
];

const client = new ApolloClient({
dataMasking: true,
cache: new InMemoryCache(),
link: new MockLink(mocks),
});

const queryStream = new ObservableStream(client.watchQuery({ query }));

const { data } = await queryStream.takeNext();
const fragmentObservable = client.watchFragment({
fragment,
from: data.currentUser,
});

const fragmentStream = new ObservableStream(fragmentObservable);

{
const { data, complete } = await fragmentStream.takeNext();

expect(complete).toBe(true);
expect(data).toEqual({ __typename: "User", age: 30 });
}
});

test("warns when passing parent object to `from` when id is masked", async () => {
using _ = spyOnConsole("warn");

interface Query {
currentUser: {
__typename: "User";
id: number;
name: string;
};
}

interface Fragment {
age: number;
}

const fragment: TypedDocumentNode<Fragment, never> = gql`
fragment UserFields on User {
id
age
}
`;

const query: TypedDocumentNode<Query, never> = gql`
query MaskedQuery {
currentUser {
name
...UserFields
}
}

${fragment}
`;

const mocks = [
{
request: { query },
result: {
data: {
currentUser: {
__typename: "User",
id: 1,
name: "Test User",
age: 30,
},
},
},
},
];

const client = new ApolloClient({
dataMasking: true,
cache: new InMemoryCache(),
link: new MockLink(mocks),
});

const queryStream = new ObservableStream(client.watchQuery({ query }));

const { data } = await queryStream.takeNext();
const fragmentObservable = client.watchFragment({
fragment,
from: data.currentUser,
});

expect(console.warn).toHaveBeenCalledTimes(1);
expect(console.warn).toHaveBeenCalledWith(
"Could not identify object passed to `from` either because the object is non-normalized or the key fields are missing. If you are masking this object, please ensure the key fields are requested by the parent object."
);

const fragmentStream = new ObservableStream(fragmentObservable);

{
const { data, complete } = await fragmentStream.takeNext();

expect(data).toEqual({});
// TODO: Update when https://github.com/apollographql/apollo-client/issues/12003 is fixed
expect(complete).toBe(true);
}
});

test("warns when passing parent object to `from` that is non-normalized", async () => {
using _ = spyOnConsole("warn");

interface Query {
currentUser: {
__typename: "User";
name: string;
};
}

interface Fragment {
age: number;
}

const fragment: TypedDocumentNode<Fragment, never> = gql`
fragment UserFields on User {
age
}
`;

const query: TypedDocumentNode<Query, never> = gql`
query MaskedQuery {
currentUser {
name
...UserFields
}
}

${fragment}
`;

const mocks = [
{
request: { query },
result: {
data: {
currentUser: {
__typename: "User",
name: "Test User",
age: 30,
},
},
},
},
];

const client = new ApolloClient({
dataMasking: true,
cache: new InMemoryCache(),
link: new MockLink(mocks),
});

const queryStream = new ObservableStream(client.watchQuery({ query }));

const { data } = await queryStream.takeNext();
const fragmentObservable = client.watchFragment({
fragment,
from: data.currentUser,
});

expect(console.warn).toHaveBeenCalledTimes(1);
expect(console.warn).toHaveBeenCalledWith(
"Could not identify object passed to `from` either because the object is non-normalized or the key fields are missing. If you are masking this object, please ensure the key fields are requested by the parent object."
);

const fragmentStream = new ObservableStream(fragmentObservable);

{
const { data, complete } = await fragmentStream.takeNext();

expect(data).toEqual({});
// TODO: Update when https://github.com/apollographql/apollo-client/issues/12003 is fixed
expect(complete).toBe(true);
}
});

class TestCache extends ApolloCache<unknown> {
public diff<T>(query: Cache.DiffOptions): DataProxy.DiffResult<T> {
return {};
Expand Down
9 changes: 8 additions & 1 deletion src/cache/core/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,17 @@ export abstract class ApolloCache<TSerialized> implements DataProxy {
): Observable<WatchFragmentResult<TData>> {
const { fragment, fragmentName, from, optimistic = true } = options;
const query = this.getFragmentDoc(fragment, fragmentName);
const id = typeof from === "string" ? from : this.identify(from);

if (!id) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This might make sense as a dev-only warning

Copy link
Member Author

Choose a reason for hiding this comment

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

Dev-only warning added in 885be47

invariant.warn(
"Could not identify object passed to `from` either because the object is non-normalized or the key fields are missing. If you are masking this object, please ensure the key fields are requested by the parent object."
Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at this, this might be a bit too generic and be hard to find what is triggering this warning. Perhaps we add the fragment name in here as well?

Should we also try and differentiate between watchFragment and useFragment, or would adding in the fragment name be enough?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fragment name added in fd66d42

);
}

const diffOptions: Cache.DiffOptions<TData, TVars> = {
returnPartialData: true,
id: typeof from === "string" ? from : this.identify(from),
id,
query,
optimistic,
};
Expand Down
39 changes: 39 additions & 0 deletions src/react/hooks/__tests__/useFragment.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1558,6 +1558,45 @@ describe("useFragment", () => {
await expect(ProfiledHook).not.toRerender();
});

it("warns when passing parent object to `from` when key fields are missing", async () => {
using _ = spyOnConsole("warn");

interface Fragment {
age: number;
}

const fragment: TypedDocumentNode<Fragment, never> = gql`
fragment UserFields on User {
age
}
`;

const client = new ApolloClient({ cache: new InMemoryCache() });

const ProfiledHook = profileHook(() =>
useFragment({ fragment, from: { __typename: "User" } })
);

render(<ProfiledHook />, {
wrapper: ({ children }) => (
<ApolloProvider client={client}>{children}</ApolloProvider>
),
});

expect(console.warn).toHaveBeenCalledTimes(1);
expect(console.warn).toHaveBeenCalledWith(
"Could not identify object passed to `from` either because the object is non-normalized or the key fields are missing. If you are masking this object, please ensure the key fields are requested by the parent object."
);

{
const { data, complete } = await ProfiledHook.takeSnapshot();

expect(data).toEqual({});
// TODO: Update when https://github.com/apollographql/apollo-client/issues/12003 is fixed
expect(complete).toBe(true);
}
});

describe("tests with incomplete data", () => {
let cache: InMemoryCache, wrapper: React.FunctionComponent;
const ItemFragment = gql`
Expand Down