graphql-java-codegen icon indicating copy to clipboard operation
graphql-java-codegen copied to clipboard

spring-graphql compatible code generation

Open msl-at-fcb opened this issue 2 years ago • 11 comments

Now that spring-graphql is 1.0 and reasonably stable, it would be good to provide first class support for their structures.

Currently @QueryMapping, @MutationMapping and @SubscriptionMapping can be shoe-horned in via customAnnotationsMapping with something like:

  "customAnnotationsMapping": {
    "^Query\\.\\w+$": ["org.springframework.graphql.data.method.annotation.QueryMapping"],
    "^Mutation\\.\\w+$": ["org.springframework.graphql.data.method.annotation.MutationMapping"],
    "^Subscription\\.\\w+$": ["org.springframework.graphql.data.method.annotation.SubscriptionMapping"]
  }

But that falls short on parameterised resolvers, or resolvers for extensions which are generated separately which rely on spring's @SchemaMapping(typeName = "MyGraphQLType") (and so need to be manually added)

Likewise use of @Argument for all input parameters can be manually added to the implementing classes, but would be convenient to have on the generated interfaces.

There are other parameter types that can be provided (see https://docs.spring.io/spring-graphql/docs/current/reference/html/#controllers). Adding these with similar configuration as exists today for DataFetchingEnvironment would also be useful and would provide reasonably complete with what spring-graphql offers.

Also note that there is a feature request in spring-graphql for code generation: https://github.com/spring-projects/spring-graphql/issues/159 which would effectively be solved with this functionality.

msl-at-fcb avatar Jun 22 '22 12:06 msl-at-fcb

The custom annotations mapping you provided works well for me too.

The main issue for me is not being able to annotate the arguments with @Argument.

Maybe a customResolverArgumentAnnotations option would be good? It could just take a list of annotations to be applied to all arguments. But not the DataFetchingEnvironment etc

JamesPeters98 avatar Jul 05 '22 16:07 JamesPeters98

We recently adopted Spring for GraphQL and set up graphql-java-codegen today.

Thanks to the comment of @msl-at-fcb we've got up and running relatively easy.

In addition to using customAnnotationsMapping for the root types Query, Mutation and Subscription, we also added @SchemaMapping for every field declared in fieldsWithResolvers.

tasks {
    named<GraphQLCodegenGradleTask>("graphqlCodegen") {
        // ...

        fieldsWithResolvers = setOf("Foo.bar")
        val customResolverMappings = fieldsWithResolvers.associateWith { field ->
             val type = field.substringBefore('.')
             listOf("""org.springframework.graphql.data.method.annotation.SchemaMapping(typeName = "$type")""")
        }
        customAnnotationsMapping.addAll(customResolverMappings)
    }
}

This works fine but involves quite some boilerplate which is likely to to be duplicated for other projects as well. Thus I would love to see explicit support for Spring for GraphQL being part of graphql-java-codegen.

Beside the aforementioned annotations I would like to see support for @BatchMapping. This however we did not get to work as I don't see any way to tell graphql-java-codegen to generate a mapping of e.g. List<T> to List<R> (or one of the other supported types) instead of T to R. If that's possible today I would love to hear how.


Disclaimer: There might be some typos in the code above, as I'm writing this on my phone right now. But I assume one gets the idea.

EndzeitBegins avatar Jul 08 '22 19:07 EndzeitBegins

Added support of 2 configs: resolverArgumentAnnotations and parametrizedResolverAnnotations which should solve abovementioned problems:

resolverArgumentAnnotations

  "resolverArgumentAnnotations": ["org.springframework.graphql.data.method.annotation.Argument"]

This will add the specified annotation to every argument in every method in each Query/Mutation/Subscription resolver:

public interface QueryResolver {
    String version(graphql.schema.DataFetchingEnvironment env) throws Exception;

    java.util.List<Event> eventsByCategoryAndStatus(@javax.validation.constraints.NotNull @org.springframework.graphql.data.method.annotation.Argument String categoryId, @org.springframework.graphql.data.method.annotation.Argument EventStatus status, graphql.schema.DataFetchingEnvironment env) throws Exception;

    Event eventById(@javax.validation.constraints.NotNull @org.springframework.graphql.data.method.annotation.Argument String id, graphql.schema.DataFetchingEnvironment env) throws Exception;
}

parametrizedResolverAnnotations

  "parametrizedResolverAnnotations": ["org.springframework.graphql.data.method.annotation.SchemaMapping(typeName="{{TYPE_NAME}}")"]

This will add the specified annotation to every field resolver method:

public interface EventPropertyResolver {
    @org.springframework.graphql.data.method.annotation.SchemaMapping(typeName="EventProperty")
    int intVal(EventPropertyTO eventProperty) throws Exception;

    @org.springframework.graphql.data.method.annotation.SchemaMapping(typeName="EventProperty")
    java.util.List<EventPropertyTO> child(EventPropertyTO eventProperty, Integer first, Integer last) throws Exception;
}

Let me know what you think.

kobylynskyi avatar Jul 31 '22 20:07 kobylynskyi

Very nice, thanks for adding this @kobylynskyi! We'll migrate as soon as it's available.

Any chance for adding @BatchMapping support somehow? That would complete the fundamental Spring support from my point of view.

EndzeitBegins avatar Jul 31 '22 21:07 EndzeitBegins

@EndzeitBegins the problem with supporting @BatchMapping is that the method signature is completely off from the field resolver that we already generate today:

FieldType field(Type type); // that's what we generate today
Mono<Map<Type, FieldType>> field(List<Type> types); // that's what should be generated to support @BatchMapping

kobylynskyi avatar Jul 31 '22 21:07 kobylynskyi

This looks perfect, will also give it a try as soon as it's available, thanks!

JamesPeters98 avatar Aug 01 '22 08:08 JamesPeters98

@EndzeitBegins the problem with supporting @BatchMapping is that the method signature is completely off from the field resolver that we already generate today:

FieldType field(Type type); // that's what we generate today
Mono<Map<Type, FieldType>> field(List<Type> types); // that's what should be generated to support @BatchMapping

I'm aware of that. Actually, there are multiple type signatures supported by Spring for GraphQL, e.g. List<Type> -> List<FieldType>. I was just hoping one might be able to configure which fields should be resolved using a batch mapping, similar to the existing fieldsWithResolvers, e.g. fieldsWithBatchResolvers. Additionally being able to configure which type signature to generate would be nice.

But I can understand if that's a lot more difficult to support than just adding additional annotations.

EndzeitBegins avatar Aug 01 '22 15:08 EndzeitBegins

Given @kobylynskyi's other helpful commits for the other annotations, is it implied that for @QueryMapping, @MutationMapping, and @SubscriptionMapping we will be doing it the way @msl-at-fcb described, i.e. customAnnotationsMapping?

Is that configuration expressible with the Maven plugin?

  "customAnnotationsMapping": {
    "^Query\\.\\w+$": ["org.springframework.graphql.data.method.annotation.QueryMapping"],
    "^Mutation\\.\\w+$": ["org.springframework.graphql.data.method.annotation.MutationMapping"],
    "^Subscription\\.\\w+$": ["org.springframework.graphql.data.method.annotation.SubscriptionMapping"]
  }

My understanding was this would have to map to something like:

<customAnnotationsMapping>
  <^Query\\.\\w+$>
    <annotation>org.springframework.graphql.data.method.annotation.QueryMapping</annotation>
  </^Query\\.\\w+$>
</customAnnotationsMapping>

Which I don't believe is expressible in xml unless I'm missing some way of escaping it.

If I understand correctly it is these lines which allow the key to be a pattern, which would appear to explain how the configuration@msl-at-fcb provided works, I'm just not sure how to express it in maven.

https://github.com/kobylynskyi/graphql-java-codegen/blob/9a64c33bedbff81ce849f253a7a7644eab00cc9d/src/main/java/com/kobylynskyi/graphql/codegen/mapper/AnnotationsMapper.java#L134-L139

If it's not currently expressible as-is, I wonder if it can be passed as a tag attribute instead? e.g.

<Query pattern="^Query\\.\\w+$">

I'm not sure if that's possible with Maven though since it would appear that the code already receives the tag as a string, but I admit I know nothing about maven plugins:

https://github.com/kobylynskyi/graphql-java-codegen/blob/0198f41aacfd8dc19e5322f8436d4d923d4b7737/plugins/maven/graphql-java-codegen-maven-plugin/src/main/java/io/github/kobylynskyi/graphql/codegen/GraphQLCodegenMojo.java#L671

blaenk avatar Aug 05 '22 08:08 blaenk

@blaenk Yeah I'm not sure it is possible directly in your pom but you can just use the .json config instead so your json config would include this:

"customAnnotationsMapping": {
    "^Query\\.\\w+$" : ["org.springframework.graphql.data.method.annotation.QueryMapping"],
    "^Mutation\\.\\w+$" : ["org.springframework.graphql.data.method.annotation.MutationMapping"],
    "^Subscription\\.\\w+$" : ["org.springframework.graphql.data.method.annotation.SubscriptionMapping"]
  }

And in your pom:

           <configuration>
              <configurationFiles>
                <configurationFile>${project.basedir}/codegenConfig.json</configurationFile>
              </configurationFiles>
              <!-- all config options:
              https://github.com/kobylynskyi/graphql-java-codegen/blob/master/docs/codegen-options.md
              -->
              <graphqlSchemas>
                <rootDir>${project.basedir}/src/main/resources/graphql</rootDir>
              </graphqlSchemas>
              <outputDir>${project.build.directory}/generated-sources/graphql</outputDir>
            </configuration>

Using the json config makes your pom way less cluttered too

JamesPeters98 avatar Aug 05 '22 09:08 JamesPeters98

Thank you @JamesPeters98 I completely missed that a JSON config was even possible. It did seem like that's what that code snippet was (JSON) but I thought for sure there couldn't be a JSON config mechanism and assumed it to be Groovy/Gradle!

Thanks again I appreciate it!

blaenk avatar Aug 05 '22 09:08 blaenk

A bit off topic, but...

Using the json config makes your pom way less cluttered too

Yep, that's pretty much our setup. Anything that needs to know about maven goes in pom.xml (eg <rootDir>${project.basedir}...</rootDir> but then the rest goes into dedicated configuration files (graphql-codegen.json) - try to keep the two as separate as we can.

msl-at-fcb avatar Aug 05 '22 09:08 msl-at-fcb

🚀 Released version 5.5.0 today which contains new configs: resolverArgumentAnnotations and parametrizedResolverAnnotations For usage please refer to my previous comment: https://github.com/kobylynskyi/graphql-java-codegen/issues/983#issuecomment-1200496176

Support for @BatchMapping will be addressed in future releases once we come up with a way of defining the desired method signature.

kobylynskyi avatar Sep 11 '22 14:09 kobylynskyi

@kobylynskyi , can we also add a configuration to add arbitrary parameters in generated resolvers? With this, we could provide a list of additional parameter names/types and also do not need generateDataFetchingEnvironmentArgumentInApis anymore.

Best, Philipp

PhilippS93 avatar Oct 12 '22 07:10 PhilippS93