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
228 changes: 228 additions & 0 deletions src/__tests__/dataMasking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1076,6 +1076,234 @@ 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` for '%s' fragment, 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.",
"UserFields"
);

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` for '%s' fragment, 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.",
"UserFields"
);

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
16 changes: 15 additions & 1 deletion src/cache/core/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
Observable,
cacheSizes,
defaultCacheSizes,
getFragmentDefinition,
getFragmentQueryDocument,
mergeDeepArray,
} from "../../utilities/index.js";
Expand Down Expand Up @@ -245,10 +246,23 @@ 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 (__DEV__) {
const actuaFragmentName =
fragmentName || getFragmentDefinition(fragment).name.value;

if (!id) {
invariant.warn(
"Could not identify object passed to `from` for '%s' fragment, 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.",
actuaFragmentName
);
}
}

const diffOptions: Cache.DiffOptions<TData, TVars> = {
returnPartialData: true,
id: typeof from === "string" ? from : this.identify(from),
id,
query,
optimistic,
};
Expand Down
40 changes: 40 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,46 @@ 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` for '%s' fragment, 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.",
"UserFields"
);
Comment on lines +1587 to +1590
Copy link
Member

Choose a reason for hiding this comment

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

Interesting question: do we want to auto-unmask key fields?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to avoid that since it might be surprising behavior and instead just recommend that someone selects from the parent entity instead. Definitely something we can keep an eye on and consider down the road!


{
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
Loading