Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spring-graphql compatible code generation #983

Closed
msl-at-fcb opened this issue Jun 22, 2022 · 13 comments
Closed

spring-graphql compatible code generation #983

msl-at-fcb opened this issue Jun 22, 2022 · 13 comments
Assignees
Labels
enhancement New feature or request help-wanted Extra attention is needed
Milestone

Comments

@msl-at-fcb
Copy link
Contributor

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: spring-projects/spring-graphql#159 which would effectively be solved with this functionality.

@JamesPeters98
Copy link
Contributor

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

@EndzeitBegins
Copy link

EndzeitBegins commented Jul 8, 2022

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.

@kobylynskyi
Copy link
Owner

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 kobylynskyi added this to the 5.5.0 milestone Jul 31, 2022
@EndzeitBegins
Copy link

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.

@kobylynskyi
Copy link
Owner

@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

@JamesPeters98
Copy link
Contributor

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

@EndzeitBegins
Copy link

@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.

@blaenk
Copy link

blaenk commented Aug 5, 2022

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.

// try finding if there's a RegEx present for this type
for (Map.Entry<String, List<String>> entry : customAnnotationsMapping.entrySet()) {
if (key.matches(entry.getKey()) && !Utils.isEmpty(entry.getValue())) {
result.addAll(entry.getValue());
}
}

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:

@JamesPeters98
Copy link
Contributor

@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

@blaenk
Copy link

blaenk commented Aug 5, 2022

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!

@msl-at-fcb
Copy link
Contributor Author

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.

@kobylynskyi
Copy link
Owner

🚀 Released version 5.5.0 today which contains new configs: resolverArgumentAnnotations and parametrizedResolverAnnotations
For usage please refer to my previous comment: #983 (comment)

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

@PhilippS93
Copy link

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help-wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants