From 18d3143e0ff1399bd87990998ac89caf009426c9 Mon Sep 17 00:00:00 2001 From: starptech Date: Sun, 16 May 2021 19:45:52 +0200 Subject: [PATCH] fix #20 --- README.md | 2 +- docs/api.md | 6 +- package-lock.json | 60 ++++++++++++ package.json | 1 + src/core/federation.ts | 2 +- src/core/graphql-utils.ts | 8 +- .../20210504193054_initial_schema.ts | 2 +- .../compose-schema-versions.test.ts | 8 +- .../federation/compose-schema.test.ts | 8 +- .../federation/register-schema.test.ts | 98 ++++++++++++++++--- src/registry/federation/register-schema.ts | 2 + 11 files changed, 169 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 3410aa8..2021b1f 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ State: Experimental - Serves schema for GraphQL gateway based on provided services & their versions - Validates new schema to be compatible with other running services - Validates that all client operations are supported by your schema -- Produce a diff between your proposed schema and the current registry state +- Produce a diff between your proposed schema and the current registry state to detect breaking changes and more - Lightweight authorization concept based on JWT. [**Read more**](https://principledgraphql.com/integrity#3-track-the-schema-in-a-registry) diff --git a/docs/api.md b/docs/api.md index 3d078b6..d3dd57d 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1,6 +1,6 @@ # Composition stability -Whenever a schema is pushed or fetched, Graph-Registry ensures that the schema is valid. You can't fetch or push an invalid schema. +Whenever a schema is pushed or fetched, Graph-Registry ensures that the schema is valid. You can't fetch or produce an invalid registry state. # Terminology @@ -35,6 +35,8 @@ POST - `/schema/push` Creates a new graph and schema for a service. If you omit **Notice:** A schema is normalized before it's stored in the database. Whitespaces are stipped and graphql elements are sorted lexicographically. +**Notice:** The schema isn't validated for breaking-changes. Use [schema diff](#produce-a-diff-from-your-schema) to understand the implications of your update. +
Example Request

@@ -93,7 +95,7 @@ PUT - `/schema/deactivate` Deactivates a schema by id. The schema will no longer ### Produce a diff from your schema -POST - `/schema/diff` Returns the schema report of all services and the provided new schema. +POST - `/schema/diff` Returns the schema report between provided and latest schemas.

Example Request diff --git a/package-lock.json b/package-lock.json index 63a8298..caa99c9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,6 +13,21 @@ "lodash.xorby": "^4.7.0" } }, + "@ardatan/aggregate-error": { + "version": "0.0.6", + "resolved": "https://registry.npmjs.org/@ardatan/aggregate-error/-/aggregate-error-0.0.6.tgz", + "integrity": "sha512-vyrkEHG1jrukmzTPtyWB4NLPauUw5bQeg4uhn8f+1SSynmrOcyvlb1GKQjjgoBzElLdfXCRYX8UnBlhklOHYRQ==", + "requires": { + "tslib": "~2.0.1" + }, + "dependencies": { + "tslib": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.0.3.tgz", + "integrity": "sha512-uZtkfKblCEQtZKBF6EBXVZeQNl82yqtDQdv+eck8u7tdPxjLu2/lp5/uPW+um2tpuxINHWy3GhiccY7QgEaVHQ==" + } + } + }, "@ava/typescript": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/@ava/typescript/-/typescript-2.0.0.tgz", @@ -384,6 +399,16 @@ "tslib": "^2.0.0" } }, + "@graphql-tools/utils": { + "version": "7.10.0", + "resolved": "https://registry.npmjs.org/@graphql-tools/utils/-/utils-7.10.0.tgz", + "integrity": "sha512-d334r6bo9mxdSqZW6zWboEnnOOFRrAPVQJ7LkU8/6grglrbcu6WhwCLzHb90E94JI3TD3ricC3YGbUqIi9Xg0w==", + "requires": { + "@ardatan/aggregate-error": "0.0.6", + "camel-case": "4.1.2", + "tslib": "~2.2.0" + } + }, "@hapi/bourne": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/@hapi/bourne/-/bourne-2.0.0.tgz", @@ -1215,6 +1240,15 @@ "integrity": "sha512-P8BjAsXvZS+VIDUI11hHCQEv74YT67YUi5JJFNWIqL235sBmjX4+qx9Muvls5ivyNENctx46xQLQ3aTuE7ssaQ==", "dev": true }, + "camel-case": { + "version": "4.1.2", + "resolved": "https://registry.npmjs.org/camel-case/-/camel-case-4.1.2.tgz", + "integrity": "sha512-gxGWBrTT1JuMx6R+o5PTXMmUnhnVzLQ9SNutD4YqKtI6ap897t3tKECYla6gCWEkplXnlNybEkZg9GEGxKFCgw==", + "requires": { + "pascal-case": "^3.1.2", + "tslib": "^2.0.3" + } + }, "camelcase": { "version": "6.2.0", "resolved": "https://registry.npmjs.org/camelcase/-/camelcase-6.2.0.tgz", @@ -3144,6 +3178,14 @@ "is-unicode-supported": "^0.1.0" } }, + "lower-case": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/lower-case/-/lower-case-2.0.2.tgz", + "integrity": "sha512-7fm3l3NAF9WfN6W3JOmf5drwpVqX78JtoGJ3A6W0a6ZnldM41w2fV5D490psKFTpMds8TJse/eHLFFsNHHjHgg==", + "requires": { + "tslib": "^2.0.3" + } + }, "lowercase-keys": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/lowercase-keys/-/lowercase-keys-1.0.1.tgz", @@ -3320,6 +3362,15 @@ "integrity": "sha512-nnbWWOkoWyUsTjKrhgD0dcz22mdkSnpYqbEjIm2nhwhuxlSkpywJmBo8h0ZqJdkp73mb90SssHkN4rsRaBAfAA==", "dev": true }, + "no-case": { + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/no-case/-/no-case-3.0.4.tgz", + "integrity": "sha512-fgAN3jGAh+RoxUGZHTSOLJIqUc2wmoBwGR4tbpNAKmmovFoWq0OdRkb0VkldReO2a2iBT/OEulG9XSUc10r3zg==", + "requires": { + "lower-case": "^2.0.2", + "tslib": "^2.0.3" + } + }, "node-fetch": { "version": "2.6.1", "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.1.tgz", @@ -3900,6 +3951,15 @@ } } }, + "pascal-case": { + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/pascal-case/-/pascal-case-3.1.2.tgz", + "integrity": "sha512-uWlGT3YSnK9x3BQJaOdcZwrnV6hPpd8jFH1/ucpiLRPh/2zCVJKS19E4GvYHvaCcACn3foXZ0cLB9Wrx1KGe5g==", + "requires": { + "no-case": "^3.0.4", + "tslib": "^2.0.3" + } + }, "path-exists": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-4.0.0.tgz", diff --git a/package.json b/package.json index 5372530..e626965 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,7 @@ "dependencies": { "@apollo/federation": "^0.25.0", "@graphql-inspector/core": "2.5.0", + "@graphql-tools/utils": "^7.10.0", "ajv": "6.12.6", "env-schema": "^3.0.1", "fastify": "^3.15.1", diff --git a/src/core/federation.ts b/src/core/federation.ts index 59e4c2a..69bb4ec 100644 --- a/src/core/federation.ts +++ b/src/core/federation.ts @@ -19,7 +19,7 @@ export function composeAndValidateSchema(servicesSchemaMap: ServiceSchema[]) { return { name: schema.name, - url: '', + url: schema.url, typeDefs, } }) diff --git a/src/core/graphql-utils.ts b/src/core/graphql-utils.ts index 9ee6569..cfe54d2 100644 --- a/src/core/graphql-utils.ts +++ b/src/core/graphql-utils.ts @@ -2,15 +2,17 @@ import { buildSchema, GraphQLSchema, lexicographicSortSchema, - printSchema, stripIgnoredCharacters, } from 'graphql' +import { printSchemaWithDirectives } from '@graphql-tools/utils' export function normalize(typeDefs: string): string { - const schema = buildSchema(typeDefs) + const schema = buildSchema(typeDefs, { assumeValidSDL: true }) return stripIgnoredCharacters(printSorted(schema)) } export function printSorted(schema: GraphQLSchema): string { - return printSchema(lexicographicSortSchema(schema)) + const sorted = lexicographicSortSchema(schema) + // without this the default implementation of "printSchema" would remove metadata like "directives" + return printSchemaWithDirectives(sorted) } diff --git a/src/migrations/20210504193054_initial_schema.ts b/src/migrations/20210504193054_initial_schema.ts index e5793e8..96ba487 100644 --- a/src/migrations/20210504193054_initial_schema.ts +++ b/src/migrations/20210504193054_initial_schema.ts @@ -50,7 +50,7 @@ export async function up(knex: Knex): Promise { .createTable(SchemaDBModel.table, (table) => { table.increments(SchemaDBModel.field('id')).primary() - table.string(SchemaDBModel.field('typeDefs')) + table.text(SchemaDBModel.field('typeDefs')) table.boolean(SchemaDBModel.field('isActive')).notNullable().defaultTo(true) table .timestamp(SchemaDBModel.field('createdAt'), { useTz: true }) diff --git a/src/registry/federation/compose-schema-versions.test.ts b/src/registry/federation/compose-schema-versions.test.ts index 841e619..c1a00d8 100644 --- a/src/registry/federation/compose-schema-versions.test.ts +++ b/src/registry/federation/compose-schema-versions.test.ts @@ -90,14 +90,14 @@ test('Should return schema of two services', async (t) => { t.truthy(response.data[0].lastUpdatedAt) t.like(response.data[0], { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: 'type Query{hello:String}', + typeDefs: 'schema{query:Query}type Query{hello:String}', version: '2', }) t.truthy(response.data[1].lastUpdatedAt) t.like(response.data[1], { serviceName: `${t.context.testPrefix}_bar`, - typeDefs: 'type Query{world:String}', + typeDefs: 'schema{query:Query}type Query{world:String}', version: '2', }) }) @@ -327,7 +327,7 @@ test('Version "current" should always return the latest (not versioned) register t.like(response.data[0], { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{world:String}`, + typeDefs: `schema{query:Query}type Query{world:String}`, version: CURRENT_VERSION, }) }) @@ -374,7 +374,7 @@ test('Should include "routingUrl" of the service', async (t) => { t.like(response.data[0], { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: 'type Query{hello:String}', + typeDefs: 'schema{query:Query}type Query{hello:String}', routingUrl: 'http://localhost:3000/api/graphql', version: '1', }) diff --git a/src/registry/federation/compose-schema.test.ts b/src/registry/federation/compose-schema.test.ts index 963e367..6985aea 100644 --- a/src/registry/federation/compose-schema.test.ts +++ b/src/registry/federation/compose-schema.test.ts @@ -55,14 +55,14 @@ test('Should return schema of two services', async (t) => { t.truthy(response.data[0].lastUpdatedAt) t.like(response.data[0], { serviceName: `${t.context.testPrefix}_bar`, - typeDefs: 'type Query{world:String}', + typeDefs: 'schema{query:Query}type Query{world:String}', version: '2', }) t.truthy(response.data[1].lastUpdatedAt) t.like(response.data[1], { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{hello:String}`, + typeDefs: `schema{query:Query}type Query{hello:String}`, version: '1', }) }) @@ -136,7 +136,7 @@ test('Version "current" has no precedence over the last updated', async (t) => { t.like(response.data[0], { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: 'type Query{world:String}', + typeDefs: 'schema{query:Query}type Query{world:String}', version: '2', }) }) @@ -177,7 +177,7 @@ test('Should include "routingUrl" of the service', async (t) => { t.like(response.data[0], { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: 'type Query{hello:String}', + typeDefs: 'schema{query:Query}type Query{hello:String}', routingUrl: 'http://localhost:3000/api/graphql', version: '1', }) diff --git a/src/registry/federation/register-schema.test.ts b/src/registry/federation/register-schema.test.ts index a6663a6..3d8b649 100644 --- a/src/registry/federation/register-schema.test.ts +++ b/src/registry/federation/register-schema.test.ts @@ -40,7 +40,7 @@ test('Should register new schema', async (t) => { { data: { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{hello:String}`, + typeDefs: `schema{query:Query}type Query{hello:String}`, version: '1', }, success: true, @@ -73,7 +73,7 @@ test('Should register new schema with service "routingUrl"', async (t) => { { data: { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{hello:String}`, + typeDefs: `schema{query:Query}type Query{hello:String}`, routingUrl: 'http://localhost:3000/api/graphql', version: '1', }, @@ -83,6 +83,47 @@ test('Should register new schema with service "routingUrl"', async (t) => { ) }) +test('Should keep metdata like graphql directives', async (t) => { + const app = build({ + databaseConnectionUrl: t.context.connectionUrl, + }) + t.teardown(() => app.close()) + + const res = await app.inject({ + method: 'POST', + url: '/schema/push', + payload: { + typeDefs: `schema { query:Query } + type Query { me: User} + type User @key(fields:"id") { + id:ID! + username:String @deprecated(reason:"Use \`newField\`.") + }`, + version: '1', + serviceName: `${t.context.testPrefix}_foo`, + graphName: `${t.context.graphName}`, + }, + }) + + const response = res.json() + + t.is(res.statusCode, 200) + t.truthy(response.data.lastUpdatedAt) + t.like( + response, + { + data: { + serviceName: `${t.context.testPrefix}_foo`, + typeDefs: + 'schema{query:Query}type Query{me:User}type User@key(fields:"id"){id:ID!username:String@deprecated(reason:"Use `newField`.")}', + version: '1', + }, + success: true, + }, + 'response payload match', + ) +}) + test('Should return 400 when routingUrl is not valid URI', async (t) => { const app = build({ databaseConnectionUrl: t.context.connectionUrl, @@ -168,7 +209,7 @@ test('Should use version "current" when no version was specified', async (t) => { data: { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{hello:String}`, + typeDefs: `schema{query:Query}type Query{hello:String}`, version: CURRENT_VERSION, }, success: true, @@ -226,7 +267,7 @@ test('Should not create multiple schemas when client and typeDefs does not chang t.like(response.data[0], { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{hello:String}`, + typeDefs: `schema{query:Query}type Query{hello:String}`, version: '2', }) }) @@ -279,13 +320,13 @@ test('Should be able to register schemas from multiple clients', async (t) => { t.like(response.data[0], { serviceName: `${t.context.testPrefix}_bar`, - typeDefs: `type Query{world:String}`, + typeDefs: `schema{query:Query}type Query{world:String}`, version: '2', }) t.like(response.data[1], { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{hello:String}`, + typeDefs: `schema{query:Query}type Query{hello:String}`, version: '1', }) }) @@ -300,7 +341,7 @@ test('Should not be able to push invalid schema', async (t) => { method: 'POST', url: '/schema/push', payload: { - typeDefs: `foo`, + typeDefs: `foo`, // invalid GraphQL SDL version: '1', serviceName: `${t.context.testPrefix}_foo`, graphName: t.context.graphName, @@ -359,7 +400,7 @@ test('Should be able to store multiple versions of the same schema with the same t.like(response.data[0], { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{world:String}`, + typeDefs: `schema{query:Query}type Query{world:String}`, version: '2', }) }) @@ -398,6 +439,39 @@ test('Should reject schema because it is not compatible with registry state', as t.true(res.json().error.includes('Field "Query.hello" can only be defined once.')) }) +test('Should not reject schema because breaking changes can be applied', async (t) => { + const app = build({ + databaseConnectionUrl: t.context.connectionUrl, + }) + t.teardown(() => app.close()) + + let res = await app.inject({ + method: 'POST', + url: '/schema/push', + payload: { + typeDefs: `type Query { hello: String world: String }`, + version: '1', + serviceName: `${t.context.testPrefix}_foo`, + graphName: t.context.graphName, + }, + }) + + t.is(res.statusCode, 200) + + res = await app.inject({ + method: 'POST', + url: '/schema/push', + payload: { + typeDefs: `type Query { hello: String }`, + version: '1', + serviceName: `${t.context.testPrefix}_foo`, + graphName: t.context.graphName, + }, + }) + + t.is(res.statusCode, 200) +}) + test('Should return correct latest service schema with multiple graphs', async (t) => { const app = build({ databaseConnectionUrl: t.context.connectionUrl, @@ -447,7 +521,7 @@ test('Should return correct latest service schema with multiple graphs', async ( t.like(response.data[0], { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{hello:String}`, + typeDefs: `schema{query:Query}type Query{hello:String}`, version: '1', }) @@ -468,7 +542,7 @@ test('Should return correct latest service schema with multiple graphs', async ( t.like(response.data[0], { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{hello:String}`, + typeDefs: `schema{query:Query}type Query{hello:String}`, version: '2', }) }) @@ -553,7 +627,7 @@ test('Should be able to register a schema with a valid JWT', async (t) => { { data: { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{hello:String}`, + typeDefs: `schema{query:Query}type Query{hello:String}`, version: '1', }, success: true, @@ -592,7 +666,7 @@ test('Should be able to register a schema in name of another service', async (t) { data: { serviceName: `${t.context.testPrefix}_foo`, - typeDefs: `type Query{hello:String}`, + typeDefs: `schema{query:Query}type Query{hello:String}`, version: '1', }, success: true, diff --git a/src/registry/federation/register-schema.ts b/src/registry/federation/register-schema.ts index e071af7..12d8895 100644 --- a/src/registry/federation/register-schema.ts +++ b/src/registry/federation/register-schema.ts @@ -92,11 +92,13 @@ export default function registerSchema(fastify: FastifyInstance) { const serviceSchemas = schemas.map((s) => ({ name: s.serviceName, typeDefs: s.typeDefs, + url: s.routingUrl, })) // Add the new schema to validate it against the current registry state before creating. serviceSchemas.push({ name: req.body.serviceName, typeDefs: req.body.typeDefs, + url: req.body.routingUrl, }) const { error: schemaError } = composeAndValidateSchema(serviceSchemas)