Federated _service Query not honouring schema-altering directives

[wabrit]

Summary

If a subgraph schema uses a custom directive that alters the schema element to which it is applied, this is not detected using the _service { sdl } query. This causes an incorrect gateway schema to be generated via rover.

Environment

I'm using @apollo/subgraph 0.1.4.

Description

My subgraph schema uses a custom directive to "expand" enums on apollo startup, but querying for _service { sdl } does not take into account their action.

For example, the following gql

enum Foo @dynamicEnum(...) {
    PLACEHOLDER @description(value: "Placeholder")
}

will use my custom @dynamicEnum directive on startup to replace the PLACEHOLDER value with actual values, so that executing the following GQL query:

  __type(name: "Foo") {
    name
    enumValues(includeDeprecated:true) {
      name
      description
      isDeprecated
      deprecationReason
    }
  }
}

Returns the response

{
  "data": {
    "__type": {
      "name": "Foo",
      "enumValues": [
        {
          "name": "LIVE_VALUE_A",
          "description": "Live value A",
          "isDeprecated": false,
          "deprecationReason": null
        },
        {
          "name": "LIVE_VALUE_B",
          "description": "Live value B",
          "isDeprecated": false,
          "deprecationReason": null
        }
      ]
    }
  }
}

This is also the response I get if I issue a rover graph introspect call against my service.

However if I include the subgraph in my federated gateway then the same query issued against that gateway gives me:

{
  "data": {
    "__type": {
      "name": "FOO",
      "enumValues": [
        {
          "name": "PLACEHOLDER",
          "description": "Placeholder",
          "isDeprecated": false,
          "deprecationReason": null
        }
      ]
    }
  }
}

which I believe is because rover supergraph compose does not honour the directive, which in turn is because the _service { sdl } query does not honour it.

[wabrit]

I think the issue is that the buildSubgraphSchema method provided by the @apollo/subgraph library doesn't allow for the caller to pass in some directive functions that can be visited after the schema is built but before printSubgraphSchema is called.

What I ended up doing to workaround this for now is to recalculate the SDL after using a SchemaDirectiveVisitor and then plugging that into the _service resolver e.g.

// Build subgraph schema
const schema = buildSubgraphSchema([{  typeDefs, resolvers }]);

// Allow the directives to visit the schema
SchemaDirectiveVisitor.visitSchemaDirectives(schema, mySchemaDirectives);

// Recalculate the SDL now the directives have done their work
const sdl = (0, printSubgraphSchema)(schema);

// Plug in the revised SDL to the _service query resolver
schema.getQueryType().getFields()['_service'].resolve = () => ({ sdl });

[wabrit]

Note that the above only works for older versions of apollo-server; for 3.5.x the SchemaDirectiveVisitor class no longer exists, and the new directives pattern causes issues when applied to a built subgraph schema e.g.

• The mapSchema() call removes resolverReference functions from GraphQLObjectType
• The mapSchema() call moves fields from extensionASTNodes to astNode

I think both of the above are a consequence of the way mapSchema() deconstructs a schema and rebuilds it.

In summary; I think buildSubgraphSchema() needs some way of being able to pass in directives that need to be evaluated before the _service resolver is constructed, unless there is a way to first construct a schema and evaluate the directive, then pass the schema to buildSubgraphSchema().

Would really appreciate some feedback on this if possible.

[pcmanus]

Note that the above only works for older versions of apollo-server

Out of curiosity, how do you apply your custom directive logic on the schema in general (that is, leaving aside _service)? Are you relying on SchemaDirectiveVisitor? Essentially, trying to understand if mapSchema is only problematic in the context of your workaround for _service or if it more generally doesn't work with subgraphs at all (sounds more like the latter, just trying to make sure).

unless there is a way to first construct a schema and evaluate the directive, then pass the schema to buildSubgraphSchema()

Well, a hacky way to do this might be to build the transformed GraphQLSchema (where you've evaluated the directive) and then print/re-parse that to pass it to buildSubgraphSchema. So something along the lines of:

const rawSchema = buildASTSchema(typeDefs);
const expandedSchema = SchemaDirectiveVisitor.visitSchemaDirectives(rawSchema, mySchemaDirectives);
const schema = buildSubgraphSchema([{ typeDefs: parse(printSchema(expandedSchema)), resolvers }]);

Clearly not suggesting this is ideal, but mentioning it in case it helps somehow (and while the print/re-parse is a tad inefficient, this might not matter so much in this context?).

Overall, agreed this should be supported more cleanly. At a minimum, it feels that having buildSubgraphSchema accept a GraphQLSchema directly shouldn't be too hard and would at least provide a somewhat "cleaner" option here. @trevor-scheer: opinions on this?

Fwiw, more generally, "better support for custom directives in federation" is definitively something we're thinking about, but that's a bigger topic because while here the custom directive is completely local to the subgraph, we're trying to also think about custom directives that may propagate to the supergraph.

[wabrit]

Hi @pcmanus thanks for getting back to me - some answers (and thanks for the suggested hack - I will try that out).

I started off with the SchemaDirectiveVisitor but am now on later versions of apollo. The relevant dependencies are:

    "@graphql-tools/schema": "^8.3.1",
    "@graphql-tools/utils": "^8.5.3",
    "apollo-server": "^3.5.0",
    "apollo-server-errors": "^3.3.0",
    "apollo-server-express": "^3.5.0",
    "graphql": "^15.5.3",

So I'm expressing directives in the "modern way" via mapSchema e.g.

function reloadEnumValuesDirectiveTransformer(schema) {
  return  mapSchema(schema, {
    [MapperKind.ENUM_TYPE]: (enumTypeConfig) => { ... }

and then in effect doing something like

const schema = reloadEnumValuesDirectiveTransformer(buildSubgraphSchema([{ typeDefs, resolvers }]));
...
const server = new ApolloServer({
    schema,
    ...

From stepping through the debugger, it seems to me that mapSchema() at some point "unpacks" the schema config and reassembles it; in the course of that it loses what presumably are some specific embellishments created by buildSubgraphSchema (for example the __resolveReference functions go missing).

For what its worth I had to surround the application of the directive with some "repair" logic to get things running which, while hokey and certainly very much not the way I would want to leave it, might help in illuminating what is going wrong:

        const typeMap = schema._typeMap;

        // Applying directives destroys resolverReference functions so we need to keep hold of them
        const resolverReferences = Object.keys(typeMap)
          .filter(t => typeof typeMap[t]['resolveReference'] === 'function')
          .map(t => ({ type: t, resolveReference: typeMap[t]['resolveReference'] }));

        // Applying directives moves fields from extensionASTNodes to astNodes, so we need to remember which
        // types are affected
        const extendedTypes = Object.keys(typeMap)
          .filter(t => typeMap[t].extensionASTNodes && typeMap[t].extensionASTNodes[0]?.fields !== undefined)
          .map(t => ({ type: t, fields: typeMap[t].extensionASTNodes[0].fields }));

       let revisedSchema = schema;
        Object.keys(directives).forEach(directive => {
            revisedSchema = directives[directive](revisedSchema);
        });

        // Plug back in the resolveReference functions
        resolverReferences.forEach(({
                                        type,
                                        resolveReference
                                    }) => (revisedSchema._typeMap[type]).resolveReference = resolveReference);

        // Switch the extension fields back where they belong
        extendedTypes.forEach(({ type, fields }) => {
            const typeToRevise = revisedSchema._typeMap[type];
            typeToRevise.extensionASTNodes[0].fields = fields;
            typeToRevise.astNode.fields = undefined;
        });

        // Re-evaluate the _service resolver against the revised schema
        const sdl = (0, printSubgraphSchema)(revisedSchema);
        const serviceQuery = revisedSchema.getQueryType().getFields()['_service'];
        serviceQuery.resolve = () => ({ sdl });

[wabrit]

Unfortunately your suggested workaround didn't work; despite passing assumeValid: true as options to buildASTSchema so that it doesn't have to worry about federation directives, it stumbles over any extend type ... declarations where the type is in another subgraph.

[pcmanus]

it stumbles over any extend type ... declarations where the type is in another subgraph.

Hum, forgot about that, my bad. I believe (haven't tested, sorry) you could work-around that by replacing buildASTSchema with the buildSchemaFromSDL method from https://www.npmjs.com/package/apollo-graphql (it's a dependency of @apollo/subgraphs and in fact what buildSubgraphSchema uses internally). You might also have to replace printSchema in my suggestion by printSubgraphSchema (exposed by @apollo/subgraphs).

But again, I fully agree this is way too complex and should be better. And it does mean having buildSubgraphSchema take a GraphQLSchema wouldn't help. I suppose buildSubgraphSchema could accept an optional (s: GraphQLSchema) => GraphQLSchema callback that is applied before we generate _service ...

For context, I'll remark here that part of the problem is that subgraphs are not valid graphQL schema, and the main culprit is type extensions. On that front, the upcoming federation 2 might help a bit as it make unnecessary to use type extension and thus make it possible to write subgraphs that are valid graphQL. At which point the work-around I suggested would at least have worked (and you wouldn't have to work-around extensions with mapSchema). Of course, federation 2 is currently only in alpha and not generally available so it's not a readily useful remark. But the more you know ... :)

[wabrit]

Yep a little foreknowledge is good :-) I'm working with federation 2 (at least on the gateway) so as to pick up the latest and greatest.

Agree that a callback would be the most flexible approach; also it would be nice to be able to use some of the makeExecutableSchema options in the new world such as inheritResolversFromInterfaces/allowUndefinedInResolve when building subgraphs. Porting over nonfederated resolvers currently means duplicating what used to be interface-level resolvers to each of their implementing types (unless I've missed something).

[wabrit]

P.S. buildSchemaFromSDL() fails because it can't recognise federation directives, and in this case the method doesn't seem to have a way of disabling validation.

[rubnogueira]

I'm implementing a Federation 2 solution, and I have directives working on the subgraph side. To sum up, I created a folder with individual directives per file, where it's run against the schema.

Directive example:

const { mapSchema, getDirective, MapperKind } = require('@graphql-tools/utils');
const { GraphQLString } = require('graphql');
const moment = require('moment');

const directive = schema => {
  const directiveName = 'date';

  return mapSchema(schema, {

    // Executes once for each object field in the schema
    [MapperKind.OBJECT_FIELD]: fieldConfig => {
      const directiveFound = getDirective(schema, fieldConfig, directiveName)?.[0];
      if (directiveFound) {
        fieldConfig.args.format = { type: GraphQLString };

        fieldConfig.resolve = async (source, args) => {
          if (args.format) {
            return moment(source).format(args.format);
          }

          return source;
        };

        return fieldConfig;
      }
    }
  });
};

module.exports = directive;

Recursive search:

const { loadFilesSync } = require('@graphql-tools/load-files');

const injectDirectives = (schema) => {
  const directivesSettings = {
    extensions: ['js'],
    recursive: true
  };

  const directives = loadFilesSync(folderPath, directivesSettings);

  // Schema is passed as reference and updated as well, injecting the directives
  directives.forEach(directive => {
    // Only consider directives that are executable
    if (typeof (directive) === 'function') {
      schema = directive(schema);
    }
  });

  return schema;
};

How it's used:

const { buildSubgraphSchema } = require('@apollo/subgraph');
const { ApolloServer } = require('apollo-server');
const preSchema = buildSubgraphSchema({
    typeDefs,
    resolvers
  });

// Final schema with the directives injected into the schema
const schema = injectDirectives(preSchema);

const server = new ApolloServer({
    schema
})

It's working well on the subgraph side, however when I run rover fed2 supergraph compose --config ./supergraph.yml > subgraph.gql, the graphql schema output doesn't have any custom directive. My plan is to include a list of custom directives (the same) in all subgraphs and have them available in the gateway. Does it make sense?

Any idea on how to generate a supergraph schema for fed2 retaining those directives? Thanks