spring-projects / spring-graphql

Spring Integration for GraphQL
https://spring.io/projects/spring-graphql
Apache License 2.0
1.54k stars 306 forks source link

Add `@GraphQlType` Annotation #1089

Closed Plygon closed 5 days ago

Plygon commented 6 days ago

A Java class name might not perfectly correspond to the corresponding GraphQL type name. In such cases, the default ClassNameTypeResolver cannot determine the GraphQL type.

This change introduces an annotation, @GraphQlType, which allows you to override the name picked up by the default classNameExtractor in ClassNameTypeResolver.

pivotal-cla commented 6 days ago

@Plygon Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

bclozel commented 4 days ago

We didn't have time to discuss your contribution @Plygon - I guess you closed this because of the CLA? Even if we don't integrate this @GraphQlType annotation, is there something we can do to make this easier on your side maybe?

rstoyanchev commented 4 days ago

If there is a common classname convention, you can set a classNameExtractor function on ClassNameTypeResolver to deal with that. The same can also look at an annotation like the one proposed here, so it should be relatively straight forward to achieve this. For us it would be less ideal to introduce such an annotation in the framework if its only purpose to customize the name of union member or interface implementation types.

Plygon commented 4 days ago

Thanks @bclozel and @rstoyanchev. I closed the PR because I need to make sure I am following my company's policies before signing the CLA. Waiting to hear back from them.

I am overriding the classNameExtractor already, but this seemed like it could be a good out-of-the-box feature. In my eyes, it is much more direct and convenient to be able to specify the GaphQL interface/union type on the data class itself, rather than adding each interface type resolver in the config file. But I also see that this is a bit of an edge-case.