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

feat: APQ between subgraph and gateway #313

Merged
merged 5 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .changeset/wise-seahorses-rhyme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
'@graphql-tools/executor-http': minor
'@graphql-mesh/transport-http': patch
---

Automatic Persisted Queries support for upstream requests

For HTTP Executor;
```ts
buildHTTPExecutor({
// ...
apq: true,
})
```

For Gateway Configuration;
```ts
export const gatewayConfig = defineConfig({
transportEntries: {
'*': {
options: {
apq: true
}
}
},
})
```
86 changes: 86 additions & 0 deletions e2e/apq-subgraphs/apq-subgraphs.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { createTenv } from '@internal/e2e';
import { stripIgnoredCharacters } from 'graphql';
import { describe, expect, it } from 'vitest';
import { hashSHA256 } from '../../packages/executors/http/src/utils';

const { service, gateway } = createTenv(__dirname);

describe('APQ to the upstream', () => {
it('works', async () => {
await using gw = await gateway({
supergraph: {
with: 'mesh',
services: [await service('greetings')],
},
});
const query = stripIgnoredCharacters(/* GraphQL */ `
{
__typename
hello
}
`);
const sha256Hash = await hashSHA256(query);
await expect(gw.execute({ query })).resolves.toEqual({
data: {
__typename: 'Query',
hello: 'world',
},
});
// First it sends the request with query
expect(gw.getStd('both')).toContain(
`fetch 1 ${JSON.stringify({
extensions: {
persistedQuery: {
version: 1,
sha256Hash,
},
},
})}`,
);
// Then it sends the query with the hash
// In the following requests the query won't be needed
expect(gw.getStd('both')).toContain(
`fetch 2 ${JSON.stringify({
query,
extensions: {
persistedQuery: {
version: 1,
sha256Hash,
},
},
})}`,
);

await expect(gw.execute({ query })).resolves.toEqual({
data: {
__typename: 'Query',
hello: 'world',
},
});

// The query is not sent again
expect(gw.getStd('both')).toContain(
`fetch 3 ${JSON.stringify({
extensions: {
persistedQuery: {
version: 1,
sha256Hash,
},
},
})}`,
);

// The query is not sent again
expect(gw.getStd('both')).not.toContain(
`fetch 4 ${JSON.stringify({
query,
extensions: {
persistedQuery: {
version: 1,
sha256Hash,
},
},
})}`,
);
});
});
20 changes: 20 additions & 0 deletions e2e/apq-subgraphs/gateway.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { defineConfig } from '@graphql-hive/gateway';

let fetchCnt = 0;
export const gatewayConfig = defineConfig({
transportEntries: {
greetings: {
options: {
apq: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Transport entries is marked as an advanced feature today. Do we want to hide this behind this setting ?

Perhaps we can have a top level option instead ?

Copy link
Member Author

@ardatan ardatan Dec 12, 2024

Choose a reason for hiding this comment

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

They are not advanced. It is a way to configure the transport.
I don't think a top level option is a good idea because this feature is specific to HTTP transport.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it's advanced, but that's what we are advertising in the documentation ^^'

And the way transport entries work is never actually explain. It's shortly explained in the Subscriptions page because it is needed to configure websocket/http callback subscription transports

Copy link
Member Author

Choose a reason for hiding this comment

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

We should fix it then yes. I agree it is a bit low level but i think we should cover each feature individial rather than grouping them into one single transportEntries documentation. What do you think?

},
},
},
plugins: () => [
{
onFetch({ options }) {
fetchCnt++;
process.stdout.write(`fetch ${fetchCnt} ${options.body}\n`);
},
},
],
});
17 changes: 17 additions & 0 deletions e2e/apq-subgraphs/mesh.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import {
defineConfig,
loadGraphQLHTTPSubgraph,
} from '@graphql-mesh/compose-cli';
import { Opts } from '@internal/testing';

const opts = Opts(process.argv);

export const composeConfig = defineConfig({
subgraphs: [
{
sourceHandler: loadGraphQLHTTPSubgraph('greetings', {
endpoint: `http://localhost:${opts.getServicePort('greetings')}/graphql`,
}),
},
],
});
10 changes: 10 additions & 0 deletions e2e/apq-subgraphs/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "@e2e/apq-subgraphs",
"private": true,
"devDependencies": {
"@apollo/server": "^4.11.2",
"@graphql-mesh/compose-cli": "^1.2.13",
"graphql": "^16.9.0",
"tslib": "^2.8.1"
}
}
25 changes: 25 additions & 0 deletions e2e/apq-subgraphs/services/greetings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { ApolloServer } from '@apollo/server';
import { startStandaloneServer } from '@apollo/server/standalone';
import { Opts } from '@internal/testing';

const opts = Opts(process.argv);

const apolloServer = new ApolloServer({
typeDefs: /* GraphQL */ `
type Query {
hello: String
}
`,
resolvers: {
Query: {
hello: () => 'world',
},
},
});

startStandaloneServer(apolloServer, {
listen: { port: opts.getServicePort('greetings') },
}).catch((e) => {
console.error(e);
process.exit(1);
});
4 changes: 3 additions & 1 deletion internal/e2e/src/tenv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,9 @@ export function createTenv(cwd: string): Tenv {
subgraph = await handleDockerHostName(subgraph, volumes);
}

for (const configfile of await glob('gateway.config.*', { cwd })) {
for (const configfile of await glob('gateway.config.*', {
cwd,
})) {
volumes.push({
host: configfile,
container: `/gateway/${path.basename(configfile)}`,
Expand Down
1 change: 1 addition & 0 deletions packages/executors/http/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"value-or-promise": "^1.0.12"
},
"devDependencies": {
"@apollo/server": "^4.11.2",
"@types/extract-files": "8.1.3",
"@whatwg-node/disposablestack": "^0.0.5",
"graphql": "^16.9.0",
Expand Down
35 changes: 9 additions & 26 deletions packages/executors/http/src/createFormDataFromVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
FormData as DefaultFormData,
} from '@whatwg-node/fetch';
import { extractFiles, isExtractableFile } from 'extract-files';
import { SerializedRequest } from './index.js';
import { isGraphQLUpload } from './isGraphQLUpload.js';

function collectAsyncIterableValues<T>(
Expand All @@ -30,18 +31,8 @@ function collectAsyncIterableValues<T>(
return iterate();
}

export function createFormDataFromVariables<TVariables>(
{
query,
variables,
operationName,
extensions,
}: {
query: string;
variables: TVariables;
operationName?: string;
extensions?: any;
},
export function createFormDataFromVariables(
body: SerializedRequest,
{
File: FileCtor = DefaultFile,
FormData: FormDataCtor = DefaultFormData,
Expand All @@ -50,7 +41,10 @@ export function createFormDataFromVariables<TVariables>(
FormData?: typeof DefaultFormData;
},
) {
const vars = Object.assign({}, variables);
if (!body.variables) {
return JSON.stringify(body);
}
const vars = Object.assign({}, body.variables);
const { clone, files } = extractFiles(
vars,
'variables',
Expand All @@ -62,16 +56,7 @@ export function createFormDataFromVariables<TVariables>(
typeof v?.arrayBuffer === 'function') as any,
);
if (files.size === 0) {
return JSON.stringify(
{
query,
variables,
operationName,
extensions,
},
null,
2,
);
return JSON.stringify(body);
}
const map: Record<number, string[]> = {};
const uploads: any[] = [];
Expand All @@ -85,10 +70,8 @@ export function createFormDataFromVariables<TVariables>(
form.append(
'operations',
JSON.stringify({
query,
...body,
variables: clone,
operationName,
extensions,
}),
);
form.append('map', JSON.stringify(map));
Expand Down
Loading
Loading