nestjs / graphql

GraphQL (TypeScript) module for Nest framework (node.js) ๐Ÿท
https://docs.nestjs.com/graphql/quick-start
MIT License
1.46k stars 396 forks source link

Federation issue when pushing with `apollo-cli` #1074

Closed StevenLangbroek closed 3 years ago

StevenLangbroek commented 4 years ago

I'm submitting a...


[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

We've started implementing code-first federation. I've got everything working locally, but we want to use Apollo Graph Manager (Studio). Apollo recommends using the apollo cli to push a microservice's schema to the manager:

$ yarn apollo service:push graph=our-graph-name key=$APOLLO_KEY localSchemaFile=./schema.gql

schema.gql is the schema generated by nestjs/graphl using autoSchemaFile. It looks like this for us:

# ------------------------------------------------------
# THIS FILE WAS AUTOMATICALLY GENERATED (DO NOT MODIFY)
# ------------------------------------------------------

directive @key(fields: String!) on OBJECT | INTERFACE

directive @extends on OBJECT | INTERFACE

directive @external on OBJECT | FIELD_DEFINITION

directive @requires(fields: String!) on FIELD_DEFINITION

directive @provides(fields: String!) on FIELD_DEFINITION

type Info {
  status: String!
}

type HealthInfo {
  heap: Info!
}

type Health {
  status: String!
  info: HealthInfo!
}

type Query {
  cnpHealth: Health!
}

This results in the following error on our CI:

Fetching info from federated service
  โœ” Loading Apollo Project
  โœ– Uploading service to Apollo Graph Manager

    GraphQLSchemaValidationError: Directive "@key" already exists in the 
    schema. It cannot be redefined.

    Directive "@extends" already exists in the schema. It cannot be redefined.

    Directive "@external" already exists in the schema. It cannot be 
    redefined.

    Directive "@requires" already exists in the schema. It cannot be 
    redefined.

    Directive "@provides" already exists in the schema. It cannot be 
    redefined.

Am I missing some configuration to drop these definitions from our schema?

Expected behavior

These types I think should be dropped for federation? Or maybe I'm missing a build step here? I'm not sure to be honest, I think they're typically provided by the gateway, not a microservice itself...

Minimal reproduction of the problem with instructions

If you have an Apollo Studio account, I suppose you could use the e2e testing repro I created earlier and try to push the schema: https://github.com/StevenLangbroek/nestjs-e2e-federation-repro

What is the motivation / use case for changing the behavior?

Apollo's approach is referred to in the documentation, so I would expect having this work is desirable.

Environment


Nest version: 7.3.2

โ”œโ”€ @nestjs/cli@7.4.1
โ”œโ”€ @nestjs/common@7.3.2
โ”œโ”€ @nestjs/core@7.3.2
โ”œโ”€ @nestjs/graphql@7.5.5
โ”œโ”€ @nestjs/mapped-types@0.0.5
โ”œโ”€ @nestjs/platform-express@7.3.2
โ”œโ”€ @nestjs/schematics@7.0.1
โ”œโ”€ @nestjs/sequelize@0.1.0
โ”œโ”€ @nestjs/terminus@7.0.1
โ””โ”€ @nestjs/testing@7.3.1



For Tooling issues:
- Node version: 12.18.0
- Platform: CircleCI, OSX

Others:

StevenLangbroek commented 4 years ago

I guess what I'm trying to understand is; the generated schema looks very different from what the schema-first approach in the Federation documentation looks like:

Code-first:

# ------------------------------------------------------
# THIS FILE WAS AUTOMATICALLY GENERATED (DO NOT MODIFY)
# ------------------------------------------------------

directive @key(fields: String!) on OBJECT | INTERFACE

directive @extends on OBJECT | INTERFACE

directive @external on OBJECT | FIELD_DEFINITION

directive @requires(fields: String!) on FIELD_DEFINITION

directive @provides(fields: String!) on FIELD_DEFINITION

type Query {
  cnpHealth: String!
}

Schema-first:

type User @key(fields: "id") {
  id: ID!
  name: String!
}

extend type User @key(fields: "id") {
  id: ID! @external
  posts: [Post]
}

extend type Query {
  getUser(id: ID!): User
}

The extend keyword is missing, and the code-first approach has a bunch of directives that are not in the schema-first approach. Is this ok?

I'm not sure that I have enough information to effectively run a schema visitor before printing a schema specifically for the CLI...

StevenLangbroek commented 4 years ago

I've tried switching to schema-first, and the "combined" schema has the same directives...

edit: nevermind, I see the _sdl query now.

ThisIsMissEm commented 4 years ago

Okay, so, I've looked into this a bit further, and the big difference seems to be that the schema-builder code doesn't actually know about the correct AST Node kind in order to generate:

extend type Query {
  getUser(id: ID!): User
}

The schema builder will need to generate an ObjectTypeExtension AST Node, instead of an ObjectTypeDefinition one. To me, that means the @Query, @Mutation, @Subscription and @ObjectType decorators need to be modified to support knowing whether they're an extension or a definition, as currently there's not enough information in the TypeMetadataStorage in order to determine which to generate, hence only ObjectTypeDefinition is supported.

When using SDL, this works fine because the schema builder and TypeMetadataStorage parts are completely* bypassed.

You also wouldn't be able to take the currently generated schema and use transformSchema to convert various AST nodes into the respective *Extension types, without separately maintaining a full list of types that need to be converted, which would kinda defeat the point of the schema builder.

StevenLangbroek commented 4 years ago

Ok, so I've dug a bit further. It seems the apollo-cli expects the output of the query { _service { sdl } } when you try to provide it with a file. There's enough information in that to be able to add the extend keyword etc (it's based on the @extend directive). The way I've hacked around it is kind of clunky though; after the server is done starting and the schema has been generated, I run a side-script that queries the server for the sdl query, and then runs it through normalizeTypeDefs from @apollo/federation. I'm thinking maybe this can be a new feature:

GraphQLFederationModule.forRoot({
  autoFederatedSchemaFile: path.join(process.cwd(), 'federated-schema.graphql')
});

Internally this could just run the result of whatever generated query { _service { sdl } } through normalizeTypeDefs from @apollo/federation (a dependency we already have anyway). What do you think?

StevenLangbroek commented 4 years ago

@ThisIsMissEm so, @apollo/federation can deduce the right ObjectTypeExtensions based on the @extends directive. I think nestjs/graphql has all the information it needs to generate an appropriate federated schema.

thunderball1 commented 4 years ago

@StevenLangbroek were you able to make it work?

StevenLangbroek commented 4 years ago

@thunderball1 yeah sorta, it's kinda hacky but it works. I've added this to main.ts:

  if (process.env.NODE_ENV === 'development') {
    generatedFederatedSchema(process.env.PORT);
  }

Where generateFederatedSchema is implemented as:

import { printWithComments } from 'graphql-tools';
import { parse } from 'graphql';
import { normalizeTypeDefs } from '@apollo/federation';
import { promises as fs } from 'fs';
import { request } from 'graphql-request';

const sdlQuery = `
  query {
    _service {
      sdl
    }
  }
`;

export async function generatedFederatedSchema(port: number): Promise<void> {
  const sdlResult = await request(`http://localhost:${port}/graphql`, sdlQuery);

  const typeDefs = parse(sdlResult._service.sdl);

  const federatedSchema = `# ------------------------------------------------------
# THIS FILE WAS AUTOMATICALLY GENERATED (DO NOT MODIFY)
# ------------------------------------------------------

${printWithComments(normalizeTypeDefs(typeDefs))}`;

  await fs.writeFile('federated-schema.gql', federatedSchema, {
    encoding: 'utf8',
  });
}
nidomiro commented 3 years ago

I spent some time debugging through the process and found out if you execute this line the schema prints nearly correctly without a server startup: https://github.com/nestjs/graphql/blob/829b4eaa40dcddbdcfc812c32f96d32193212716/lib/federation/graphql-federation.module.ts#L163

The following print is the result of executing the printSchema-method while debugging this test: https://github.com/nestjs/graphql/blob/829b4eaa40dcddbdcfc812c32f96d32193212716/tests/e2e/code-first-federation.spec.ts#L22

The result:

"Search result description"
union FederationSearchResultUnion = Post | User

type Post @key(fields: "id") {
  id: ID!
  title: String!
  authorId: Int!
}

type Query {
  _entities(representations: [_Any!]!): [_Entity]!
  _service: _Service!
  findPost(id: Float!): Post!
  getPosts: [Post!]!
  search: [FederationSearchResultUnion!]! @deprecated(reason: "test")
}

type User @extends @key(fields: "id") {
  id: ID! @external
  posts: [Post!]!
}

It needs to be redirected into a file somehow.

adikari commented 3 years ago

I have used printSchema to generate the schema and saved it in a schema.gql file before pushing it using apollo:push. However, this throws an error.

Error: Unknown type: "_Entity"
LeonYanghaha commented 3 years ago

I also encountered this problem. I used the code-first. What should I do next

ThisIsMissEm commented 3 years ago

You need both a schema.gql and a federated-schema.gql file.

StevenLangbroek commented 3 years ago

@LeonYanghaha you can use this script for now until there's a native solution: https://github.com/nestjs/graphql/issues/1074#issuecomment-704454212

StevenLangbroek commented 3 years ago

@kamilmysliwiec can i ask, wdyt about this suggestion? https://github.com/nestjs/graphql/issues/1074#issuecomment-669015034

andrewl913 commented 3 years ago

@adikari that issue happens as well using the introspection method via --endpoint flag

anlauren commented 3 years ago

Would there be a way to execute the sdl query on the app directly, without having to start a server? ๐Ÿค” i'm trying to read the source code to see if there's any way to call the resolvers directly but no luck yet

leo6104 commented 3 years ago

@anlauren Here is solution. I got an idea from test code (https://github.com/nestjs/graphql/blob/53296420f2bcfea334459ff66fc2e157bd765f24/tests/e2e/code-first-federation.spec.ts)

I create Testing module with AppModule and create Apollo client through test code way.

  const module = await Test.createTestingModule({
    imports: [AppModule],
  }).compile();

Here is my full code generate.ts Use ts-node generate.ts command.

import { AppModule } from './src/app.module';
import { Test, TestingModule } from '@nestjs/testing';
import { ApolloServerTestClient, createTestClient } from 'apollo-server-testing';
import { getApolloServer } from '@nestjs/graphql';
import { gql } from 'apollo-server-express';
import { parse } from 'graphql';
import { normalizeTypeDefs } from '@apollo/federation';
import { promises as fs } from 'fs';
import { printWithComments } from 'graphql-tools';

export function createClient(
  testingModule: TestingModule,
): ApolloServerTestClient {
  const apolloServer: any = getApolloServer(testingModule);
  return createTestClient(apolloServer);
}

async function generate() {
  const module = await Test.createTestingModule({
    imports: [AppModule],
  }).compile();
  const app = module.createNestApplication();
  await app.init();
  const apolloClient = createClient(module);
  const sdlResult = await apolloClient.query({
      query: gql`
          {
              _service {
                  sdl
              }
          }
      `,
  });

  const typeDefs = parse(sdlResult.data._service.sdl);

  const federatedSchema = `# ------------------------------------------------------
# THIS FILE WAS AUTOMATICALLY GENERATED (DO NOT MODIFY)
# ------------------------------------------------------

${printWithComments(normalizeTypeDefs(typeDefs))}`;
  await fs.writeFile('federated-schema.gql', federatedSchema, {
    encoding: 'utf8',
  });
  await app.close();
}
generate();
kamilmysliwiec commented 3 years ago

There are no plans to implement this feature in the foreseeable future as we currently don't have enough time to look into that ๐Ÿ˜ž PRs are more than welcome though!