spring-example: Upgrade graphql-spring-boot-starter to 11.0.0

[sachindshinde]

Our Spring Boot example was using a very old version of graphql-spring-boot-starter (5.10.0), as noted in #101 . This PR upgrades the example to 11.0.0. More specifically, this PR:

• Bumps all graphql-spring-boot-* artifacts to 11.0.0.
• Bumps all spring-boot-starter-* artifacts to 2.3.6.RELEASE (the same version used by graphql-spring-boot-* 11.0.0).
• Bumps JUnit Jupiter to 5.7.1 and switches to using the aggregator artifact junit-jupiter, as the Maven Surefire Plugin was encountering NoClassDefFoundErrors without it.
• Refactors the Spring Boot example to be simpler and to mimic Apollo Server's __resolveReference() pattern.
• Adds some more checks to the test, specifically to ensure there are no GraphQL errors and to ensure that federated tracing is enabled.
  • Note that the previous Spring Boot example had a bug where the Instrumentation bean wasn't getting created due to the lack of @Import(App.class) on the test class; this has been fixed in this PR.
• Disables logging in the test via logback.xml, as spring-boot-starter-* added logback to their dependency tree at some point and it's causing tests to print to stdout, which the Maven Surefire Plugin complains about since it also uses stdout to communicate between forked processes.
• Adds an example of a graphql-java-tools microservice (along with tests), set up under a non-default Spring profile.

[setchy]

@sachindshinde - this is great. I really like the simpler logic around resolveReference

I have a few questions about whether it could be simplified even further for Graph Builders.

Some suggestions:

• could the string references for any federated types (ie: "Product") be replaced by something powered by parsing any SDL types with the @extends directive
• could the string references for key fields (ie: "upc") be replaced by something powered by parsing any SDL types with the @key(fields: "...") directive
• could there be a base class that provides a default implementation of the resolveReference which each type object could extend from. In most cases it would be creating a new object, setting the @external fields with the matching @key fields and returning that new object. The type or field level resolvers could then add the necessary service layer call to populate the additional data (ie: "quantity", "inStock")

[sachindshinde]

@setchy Thanks for the suggestions! I do think it would be nice for users to:

• Have more automation around defining _entities fetchers and _Entity type resolvers.
• Have some sane defaults they can activate through some simple mechanisms (e.g. inheritance).

With regards to your specific suggestions:

• could the string references for any federated types (ie: "Product") be replaced by something powered by parsing any SDL types with the @extends directive
• could the string references for key fields (ie: "upc") be replaced by something powered by parsing any SDL types with the @key(fields: "...") directive

For some clarifying context, a GraphQL type is an entity type if it has an @key directive, or if it implements an interface with an @key directive. Since we're given the GraphQLSchema in Federation#transform(), we can indeed use this to determine:

• The GraphQL type names for all entity types.
• The possible key fields for each entity type.
  • Note that an entity type can have multiple @key usages, and that each @key usage may specify multiple fields (additionally with nesting).
  • Note that a federation reference passed to the _entities resolver may additionally contain extra fields as needed by @requires.
  • Parsing/validating fields is going to be a bit trickier here, as the grammar for fields is basically a SelectionSet without braces with some additional limitations/validation rules, and I haven't looked into how difficult it would be for graphql-java to parse this particular symbol with a separate set of validation rules.

For the switch statements in the _entities fetcher and the _Entity type resolver, we could automate that if we let users register wirings for each entity type, e.g. looking like

public interface EntityWiring {
    // The GraphQL type name of the entity.
    public String getName();

    // Resolves a federation reference to an object representing the entity type.
    public Object resolveReference(Map<String, Object> reference);

    // Determines whether an object represents this specific entity type.
    public boolean isType(Object object);
}

We could then validate that they provide wirings for every entity type present in the schema. We could probably infer some of these automatically if the user's using a library like graphql-java-annotations where we have a well-defined correspondence between Java types and GraphQL types.

Regarding the automation of parsing of the federation reference, we could eliminate at least some boilerplate and provide type safety using something similar to JSON object mapping, where the user provides POJO classes and we validate them against the parsed @key and @requires fields. Their resolveReference() function could then take in their own class instead of having to walk Map<String, Object>. (If we can leverage code-generation tools, we could additionally generate the POJO classes for the user.)

• could their be a base class that provides a default implementation of the resolveReference which each type object could extend from. In most cases it would be creating a new object, setting the @external fields with the matching @key fields and returning that new object. The type or field level resolvers could then add the necessary service layer call to populate the additional data (ie: "quantity", "inStock")

I'm not sure if we could do this exactly as written, as resolveReference() would be static and statics can't be overridden via inheritance. This also becomes trickier for the cases of multiple @keys, nested fields in @keys, and @requires fields. A potential solution that comes to mind is one where the user provides constructors that take in their POJO class, and we use reflection or annotations to detect those constructors to set up resolveReference() in an EntityWiring.

Circling back to this PR, while I believe it would be nice to provide automation and reduce boilerplate, I wouldn't say it's in scope for this PR. If you're interested in contributing to federation-jvm, you can file a GitHub issue referencing this thread and we can work through developing a more concrete proposal. 🙂

[setchy]

Thanks @sachindshinde - appreciate the thorough reply - lots of good suggestions / ideas in there. I agree, those suggestions are definitely not in the scope of this card. I'll raise a new issue shortly so we can continue the discussion and together find a nice path forward.

Back to the scope of this card, I created an sample spring-boot-federation-example last weekend (which was cause for raising the original ticket #101). There are two main gotchas/quirks that are not covered in this spring-example. I'll do my best to explain them below, based on my current understanding...

To illustrate, I've made a fork of your branch and made a few changes to bring it more in-line with typical graphql-java-kickstart services (use of GraphQLQueryResolver and GraphQLResolver<> interfaces)

Quirk 1 - Cannot have an empty query root node in graphql-java.

This is documented in Federation.java.

graphql-java-kickstart appears to enforce this at start-up the moment you implement either of the above *Resolver interfaces. If you do not define a Query type and resolver, you'll be faced with
Caused by: graphql.kickstart.tools.SchemaClassScannerError: Type definition for root query type 'Query' not found! at startup

You can use Federation.transform(final GraphQLSchema schema, final boolean queryTypeShouldBeEmpty) and pass queryTypeShouldBeEmpty as true, while defining a Query type with a dummy query to work around this. I'm using the SchemaParser to get back an executable GraphQLSchema object, but this in turn causes issue 2 below

Note: If you use the Federation.transform(final File sdl) and pass the **/*graphqls file in (as the example is today) the server will start, but the dummy Query field will be visible, as there are not an equivalent queryTypeShouldBeEmpty argument.

Quirk 2. graphql-java-kickstart by default will remove any unused types from the parsed SDL.

Because unusued / unreferenced types are removed from the executable schema when doing schemaParser.makeExecutableSchema, the _entities query does not get created.

One workaround to this is to change the the dummy query from above to return the federated type (ie: Product) and set the queryTypeShouldBeEmpty to true. Though this works in a non-trivial example there is likely to be legitimate queries that need to co-exist.

I haven't found an elegant solution to this yet, but I suspect the answer lies in either

• adding a new Federation.transform(final File sdl, final boolean queryTypeShouldBeEmpty) would allow developers to use the raw SDL to create the `entities query, have a dummy query to make graphql-java happy, and discard it in the final product
• using the existing Federation.transform(GraphQLSchema schema, boolean queryTypeShouldBeEmpty) but getting a non-executable schema / schema that has the unusued types (not sure exactly how well this would work - would likely require creating a separate schemaparser and setting some of the non-default SchemaParserOptions in order to allow unusuedTypes)
• something more obvious that I can't see 😅

[sachindshinde]

@setchy I've looked into the graphql-java-tools library a bit more, and added to this PR a graphql-java-tools example to the federation-spring-example artifact under a new Spring profile, which should hopefully help. See the updated README.md for how to run it.

Regarding the quirks you've noticed:

Quirk 1 - Cannot have an empty query root node in graphql-java.

This is documented in Federation.java.

graphql-java-kickstart appears to enforce this at start-up the moment you implement either of the above *Resolver interfaces. If you do not define a Query type and resolver, you'll be faced with
Caused by: graphql.kickstart.tools.SchemaClassScannerError: Type definition for root query type 'Query' not found! at startup

While it's true that you do need to specify a GraphQLQueryResolver, you can still leave that class as empty. Similarly, the SDL you give graphql-java-tools cannot omit a query type, but it can specify an empty query type of type Query. The example in this PR does this.

You can use Federation.transform(final GraphQLSchema schema, final boolean queryTypeShouldBeEmpty) and pass queryTypeShouldBeEmpty as true, while defining a Query type with a dummy query to work around this. I'm using the SchemaParser to get back an executable GraphQLSchema object, but this in turn causes issue 2 below

If you take a look at GraphQLJavaToolsConfiguration#schemaTransformer() in this PR, it looks at the SchemaObjects registered in SchemaParser and amends the query type to include a dummy field if needed. Some of this logic could be moved into federation-graphql-java-support if we create a Federation.transform(final GraphQLSchema.Builder builder) overload, but unfortunately the GraphQLSchema.Builder class doesn't have any public getters, so we can't copy any data from it without building (which will throw).

Note: If you use the Federation.transform(final File sdl) and pass the **/*graphqls file in (as the example is today) the server will start, but the dummy Query field will be visible, as there are not an equivalent queryTypeShouldBeEmpty argument.

I think there's been a bit of a misunderstanding about Federation#transform(final File sdl). Specifically, overloads of Federation#transform() other than the one that accepts a GraphQLSchema will handle schemas without query types (or with empty query types) just fine, so you can hand Federation#transform(final File sdl) a schema without a query type and it should succeed. I think the docstring around this was confusing, and I've accordingly updated it in #104 . You're definitely right in that the specific overload you mentioned does not have a way to hide dummy query fields, but the schemas you pass to such overloads shouldn't need to declare dummy query fields.

Quirk 2. graphql-java-kickstart by default will remove any unused types from the parsed SDL.

Because unusued / unreferenced types are removed from the executable schema when doing schemaParser.makeExecutableSchema, the _entities query does not get created.

One workaround to this is to change the the dummy query from above to return the federated type (ie: Product) and set the queryTypeShouldBeEmpty to true. Though this works in a non-trivial example there is likely to be legitimate queries that need to co-exist.

I haven't found an elegant solution to this yet, but I suspect the answer lies in either

• adding a new Federation.transform(final File sdl, final boolean queryTypeShouldBeEmpty) would allow developers to use the raw SDL to create the _entities query, have a dummy query to make graphql-java happy, and discard it in the final product
• using the existing Federation.transform(GraphQLSchema schema, boolean queryTypeShouldBeEmpty) but getting a non-executable schema / schema that has the unusued types (not sure exactly how well this would work - would likely require creating a separate schemaparser and setting some of the non-default SchemaParserOptions in order to allow unusuedTypes)
• something more obvious that I can't see 😅

There ends up being a solution here which isn't too painful, which is namely both:

• Setting SchemaParserOptions.Builder.includeUnusedTypes to true.
• Adding any types that are only referenced by _entities to a SchemaParserDictionary bean.

The code under GraphQLJavaToolsConfiguration in this PR does this.

Note that SchemaParserOptions.Builder does not use standard JavaBeans API naming conventions of getFoo()/setFoo(), so you can't set includeUnusedTypes via properties.xml/yml (given that they've used @ConfigurationProperties in their code, this is likely a bug in graphql-java-tools). You'll instead need to either specify the entire bean for SchemaParserOptions.Builder in code, or to use BeanPostProcessor to customize the auto-configuration bean (the code in this PR opts for the latter).

[setchy]

That is excellent @sachindshinde 🎉

I've reached out to the team behind graphql-java-tools yesterday to discuss the possibility of either:

• updating SchemaParserOptions.Builder to use standard JavaBeans API so that includeUnusedTypes could be configured via the properties.xml/yml, or,
• consider any type that has the @extends federation directive as used and therefore keep it in the executable schema.

[sachindshinde]

@setchy Thanks for reaching out to the graphql-java-tools team here! 🙇

One slight correction regarding:

• consider any type that has the @extends federation directive as used and therefore keep it in the executable schema.

You'd want to keep entity types in the executable schema, and accordingly you'd want to consider any type with the @key directive here. The @extends directive is used on an entity type/method when that type/method is not owned by the implementing service. Whether an implementing service owns an entity type is unrelated to whether it can only be reached from the _entities field: an implementing service that doesn't own a type can still return that type directly in its schema, and an implementing service that owns a type should be okay if that type is only returned by the _entities field.