graphile / crystal

šŸ”® Graphile's Crystal Monorepo; home to Grafast, PostGraphile, pg-introspection, pg-sql2 and much more!
https://graphile.org/
Other
12.62k stars 571 forks source link

Is there any plugin to prefix the default types such as Query, Node, Cursor, PageInfo? #1505

Closed codef0rmerz closed 3 years ago

codef0rmerz commented 3 years ago

I tried to hook up a postgraphile GQL service into Apollo Federation Gateway using @graphile/federation plugin. This worked as expected. However, when use more than one postgraphile GQL service, I get a schema validation error such as GraphQLSchemaValidationError: There can be only one type named "PageInfo".

Is there a way to prefix those default types so that each postgraphile GQL service will have unique type names for the Apollo Gateway to run smoothly?

benjie commented 3 years ago

Many of these use the builtin inflector which has the default implementation of str => str but you can customise that via https://www.graphile.org/postgraphile/make-add-inflectors-plugin/

those that do not use any inflector Iā€™d welcome a PR to use the builtin inflector for šŸ‘

codef0rmerz commented 3 years ago

Thanks @benjie. My plugin worked just fine except Cursor which I think does not come under builtin. I also tried looking into all the inflectors but could not find it anywhere.

const MyPlugin = makeAddInflectorsPlugin((inflectors) => {
  const { builtin: oldEnumName } = inflectors;

  return {
    builtin(value) {
      if (value == 'Query' || value == 'Node' || value == 'PageInfo') {
        return oldEnumName.call(this, `postgraphile1${value}`);
      } else {
        return value;
      }
    },
  };
}, true);

However, I ran into another issue. When I combine MyPlugin with FederationPlugin, the gateway just refuses to start.

// Federated Postgraphql service
import FederationPlugin from '@graphile/federation';

app.use(
  postgraphile('<db-string-goes-here>', 'public', {
    watchPg: true,
    graphiql: true,
    enhanceGraphiql: true,
    appendPlugins: [FederationPlugin, MyPlugin]
  })
);
// Apollo Gateway
const serviceList = [
  {
    name: 'postgraphile-apollo1',
    url: `http://localhost:5001/graphql`,
  },
  {
    name: 'postgraphile-apollo2',
    url: `http://localhost:5002/graphql`,
  }
];

const app = express();

const gateway = new ApolloGateway({
  serviceList,
  buildService({ url }) {
    return new RemoteGraphQLDataSource({
      url,
      willSendRequest({ request, context }) {
        request.http.headers.set('Authorization', context.Authorization);
      },
    });
  },
});

(async () => {
  const { schema, executor } = await gateway.load();

With MyPlugin alongside FederationPlugin, it yells node:84837) UnhandledPromiseRejectionWarning: Error: Couldn't load service definitions for "postgraphile-apollo1" at http://localhost:5001/graphql: 400: Bad Request at node_modules/@apollo/gateway/src/loadServicesFromRemoteEndpoint.ts:73:15

codef0rmerz commented 3 years ago

@benjie Turns out that I should not rename Query type. The above error has gone after I changed the if-cond to if (value == 'Node' || value == 'PageInfo'). However, Apollo is now throwing schema validation error as expected:

UnhandledPromiseRejectionWarning: Error: A valid schema couldn't be composed. The following composition errors were found:
        Field "Query.query" can only be defined once.
        Field "Query.nodeId" can only be defined once.
        Field "Query.node" can only be defined once.

How else do I rename the root type Query?

benjie commented 3 years ago

Delete those fields: https://www.graphile.org/postgraphile/extending-raw/#removing-things-from-the-schema

codef0rmerz commented 3 years ago

@benjie I managed to rename those fields except node using a following plugin.

const MyPlugin2 = function (builder) {
  builder.hook('GraphQLObjectType:fields', (fields) => {
    if (fields.query) {
      fields.queryPostgraphile1 = fields.query;
      delete fields.query;
    } else if (fields.node) {
      fields.nodePostgraphile1 = fields.node;
      delete fields.node;
    }
    return fields;
  });
};

However, that only renamed node field under SecuritiesEdge and not root Query even after trying all the hooks How else do I target node() thingy under root Query to rename it?

type Query implements Postgraphile1Node {
  """
  The root query type must be a `Node` to work well with Relay 1 mutations. This just resolves to `query`.
  """
  nodeIdPostgraphile1: ID!

  """Fetches an object given its globally unique `ID`."""
  node(
    """The globally unique `ID`."""
    nodeIdPostgraphile1: ID!
  ): Postgraphile1Node
}
type SecuritiesEdge {
  """A cursor for use in pagination."""
  cursor: Cursor

  """The `Security` at the end of the edge."""
  nodePostgraphile1: Security
}
benjie commented 3 years ago

You have elseif; maybe it should just be a separate if statement because you want to perform both actions?

codef0rmerz commented 3 years ago

@benjie No, that wont work because the following node (looks like an interface to me) is not captured by any hook I tried with. The fields.node only targeted the node field under SecuritiesEdge. That means below node is not a field.

Do you know what it's called?

node(
    """The globally unique `ID`."""
    nodeIdPostgraphile1: ID!
 ): Postgraphile1Node
benjie commented 3 years ago

I think you're mistaken.

const MyPlugin2 = function (builder) {
  builder.hook('GraphQLObjectType:fields', (fields) => {
    if (fields.query) {
      fields.queryPostgraphile1 = fields.query;
      delete fields.query;
    }

    // The `else` here prevented `Query.node` being renamed because `Query.query` exists
    // so only the above block executed, not the block below. Remove the `else`.

    if (fields.node) {
      fields.nodePostgraphile1 = fields.node;
      delete fields.node;
    }
    return fields;
  });
};
codef0rmerz commented 3 years ago

When I tried logging fields.node, I thought the hook runs for each field. My bad šŸ‘

codef0rmerz commented 3 years ago

I feel bad for asking too many questions but things are not working as expected as per the documentation.

Here I'm trying to extend the existing typeDef schema using makeExtendSchemaPlugin plugin but it's not working. Neither there is any error. What am I missing?

const MyPlugin3 = makeExtendSchemaPlugin((build) => {
  return {
    typeDefs: gql`
      extend type Securityinfo @key(fields: "id") {
        id: Int! @external
        securityinfoById: [Security!]!
      }
    `
  };
});
benjie commented 3 years ago

makeExtendSchemaPlugin doesn't currently produce errors; if you try and extend something that doesn't exist then that just doesn't do anything (effectively the extension sets up a hook for when that type is declared; but the type is never declared so the hook never fires).

Are you sure Securityinfo exists within your GraphQL schema, and are you sure the case is right (e.g. it's not SecurityInfo?).

codef0rmerz commented 3 years ago

Thanks @benjie. Figured that out, now grokking graphile/federation plugin to improve my plugins.

benjie commented 3 years ago

[semi-automated message] To keep things manageable I'm going to close this issue as I think it's solved; but if not or you require further help please re-open it.

codef0rmerz commented 3 years ago

@benjie Is it possible to add directive @key(fields: "id") on a type?

For example,

type Security implements Postgraphile1Node @key(fields: "id") {
...
}

I tried the approach mentioned here https://github.com/graphile/postgraphile/issues/745 but that just adding a directive on top of the schema i.e. directive @key(fields: "id") on Security prior to defining Security and hence failing.

codef0rmerz commented 3 years ago

makeExtendSchemaPlugin doesn't currently produce errors; if you try and extend something that doesn't exist then that just doesn't do anything (effectively the extension sets up a hook for when that type is declared; but the type is never declared so the hook never fires).

Are you sure Securityinfo exists within your GraphQL schema, and are you sure the case is right (e.g. it's not SecurityInfo?).

It looks like it's not injecting the type which does not exist in the schema I'm injecting into. However, that's the requirement for Apollo Federation Entity to query data over subgraphs. Alternatively, I've also tried to generate schema using createPostGraphileSchema and then manipulating it by hand in order to feed back to postgraphile(DB_string, schema) over express but seems not to be working either :(

codef0rmerz commented 3 years ago

@benjie I tried to add a directive by referring to AddKeyPlugin from postgraphile/federation however I'm not seeing it in the schema generated when printed šŸ¤”

builder.hook('GraphQLObjectType:interfaces', (interfaces, build, context) => {
    const { GraphQLObjectType: spec, Self } = context;
    if (spec.name == 'Security') {
      const astNode = { ...ObjectTypeDefinition(spec) };
      astNode.directives.push(Directive('key', { fields: StringValue('id') }));
      Self.astNode = astNode;
    }

    return interfaces;
  });
benjie commented 3 years ago

No schema when printed with graphql.printSchema will show directives other than @deprecated after fields. Apollo are doing non-standard stuff in Federation and you need a custom printer and custom directive processing to make it work.

codef0rmerz commented 3 years ago

@benjie you mean @key directives are added correctly using the above hook but it wont be shown in the schema printed, right?

Also, I know that when we use postgraphile(DB_string, schema) as an Express middleware, it under the hood generate typeDef and resolvers for queries to run. So can we feed the custom schema from an external file?

benjie commented 3 years ago

You mean @key directives are added correctly using the above hook but it wont be shown in the schema printed, right?

Effectively what that hook is doing is pretending that the schema was defined via SDL and then pretending that the specific node you're hooking had a directive as part of it's definition. Keep in mind that GraphQL doesn't really have a concept of a type/field having a directive, more that you can apply a directive to a type/field - again, this is Apollo doing non-standard stuff. Try printing it with the federation schema printer?

So can we feed the custom schema from an external file?

It's just JavaScript, so if you can read it from a file and wrangle it into the right format then yes.

codef0rmerz commented 3 years ago

@benjie Thanks, I'll investigate how to print Federated Schema to see all the non-standard directives.

Btw, I've cracked on how to consume two or more PostgraphQL federated services in Apollo. Without my fix, Apollo would not start because of clashing Types such as Query/Node/Cursor/PageInfo/etc.

So do you want a separate plugin for the same or should it be a part of graphile/federation?

benjie commented 3 years ago

If you can raise a PR against graphile/federation we can decide where to go from there. We could just do a new major version for example.

codef0rmerz commented 3 years ago

@benjie Is there a way to rename nodeIdFieldName via CLI arguments because I did not find it here https://www.graphile.org/postgraphile/usage-cli/

benjie commented 3 years ago

You can set it in a .postgraphilerc.js via module.exports = {options:{graphileBuildOptions:{nodeIdFieldName: "..."}}}

codef0rmerz commented 3 years ago

Thanks. Btw, I've raised a PR https://github.com/graphile/federation/pull/25 @benjie

codef0rmerz commented 3 years ago

So can we feed the custom schema from an external file?

It's just JavaScript, so if you can read it from a file and wrangle it into the right format then yes.

I was looking to pass a schema from a filesystem to postgraphile method that being passed to express as follows. But seems to be not working šŸ¤”

const app = express();
app.use(
  postgraphile(
    'postgres://codef0rmer@localhost:5432/postgraphile',
    [fs.readFileSync('graphql.schema', { encoding: 'utf-8' })],
    {
      graphiql: true,
      enhanceGraphiql: true
    }
  )
);
app.listen(5001);

P.S. FTR, graphql.schema was generated by createPostGraphQLSchema

benjie commented 3 years ago

You're passing a GraphQL Schema description (note: it does not have any resolvers so it is not an executable schema) into the list of PostgreSQL schema names; these are two completely unrelated things.

You want to use makeExtendSchemaPlugin or similar and you need to find a way to take that SDL from the filesystem and have makeExtendSchemaPlugin parse it, and then you need to write resolvers for all the fields.

I somewhat suspect this isn't actually what you're intending to do.

codef0rmerz commented 3 years ago

@benjie I am aware of resolvers part but was looking for ways to pass the external schema file to postgraphile() while it generates resolvers on the fly based on DB path or pgPool. But looks like it's not possible.

Plus, I'm still struggling to make this work https://github.com/graphile/postgraphile/issues/1505#issuecomment-870383087 because Securityinfo is not the type defined in the schema - it's for external entities to work over Apollo Gateway. I have also noticed that extend type <typeName> works fine if the <typeName> already exists in the schema. I also tried to use build.graphql.GraphQLObjectType with build.newWithHooks but looks like it does not have support for extend so it just adds type <typeName> {...} instead of extend type <typeName> {...} šŸ¤¦

benjie commented 3 years ago

@benjie I am aware of resolvers part but was looking for ways to pass the external schema file to postgraphile() while it generates resolvers on the fly based on DB path or pgPool. But looks like it's not possible.

That's indeed not how PostGraphile works.

Plus, I'm still struggling to make this work #1505 (comment) because Securityinfo is not the type defined in the schema - it's for external entities to work over Apollo Gateway. I have also noticed that extend type works fine if the already exists in the schema.

Correct, you can only extend types that already exist; if the type doesn't exist then the extension will never "fire".

I also tried to use build.graphql.GraphQLObjectType with build.newWithHooks but looks like it does not have support for extend so it just adds type {...} instead of extend type {...}

You're trying to do Apollo Federation specific things with something that is designed to build a regular GraphQL schema, not an Apollo Federation augmented GraphQL schema.