smallrye-graphql
smallrye-graphql copied to clipboard
Don't repeat `Input`
If an input parameter type already ends with Input
, e.g. SuperHeroInput
, the generated schema shouldn't repeat it to become SuperHeroInputInput
.
I agree with this. Is this in the spec though ? If not we might want to add that rule there too.
I hadn't expected this to be ambiguous, but after re reading: yes, the spec should be clarified, too.
Just created https://github.com/eclipse/microprofile-graphql/issues/285
@t1 - Ok, So if this object is also use for an output, should we remove the (already there) Input ? If not we will have an error as both in- and output will have the same name ....
I assume so, yes. If the application has only a SuperHeroInput
class, it should either become SuperHero
when used as an output, or it should be reported as an error... maybe the latter would be less confusing?
@phillip-kruger : Did you have time to think about this?
Maybe in this case, the user should just use @Name
to correct the name ?
Okay, that would work. It's just not really intuitive to write @Name("FooInput") class FooInput {...}
, is it? OTOH it would be even stranger to have a class TeamInput
turn into a return type Team
.
So the rules would have to be:
- If a class is used as a return value, the GraphQL
type
is exactly the simple class name. - If a class is used as parameter as well as a return type, the GraphQL
type
is exactly the simple class name and the GraphQLinput
is the simple class name plusInput
. - If a class is used only as a return type, and it ends with
Input
, the GraphQLinput
is exactly the simple class name. - If a class is used only as a return type, and it doesn't end with
Input
, the GraphQLinput
is the simple class name plusInput
.
The only downside I can see is this situation: you start with TeamInput
as parameter only; so the schema contains an input TeamInput
. Then you also return it somewhere; so the schema gets a type TeamInput
, which is okay; but your input suddenly changes to input TeamInputInput
.
BTW: I checked the GraphQL tutorial as well as the GraphQL spec: maybe I'm just blind, but I didn't see either to mention that an input
and a type
can't use the same name! So it should be possible to use TeamInput
as a type
as well as an input
. But the graphql.schema.GraphQLTypeCollectingVisitor
complains if I try. If this was true, this would be an issue for graphql-java
, wouldn't it?
Why would you use ***Input as a classname ? We currently would control what is input and what is output via annotations on a normal class.
My examples weren't clear enough, I guess.
FooInput
was meant to be an example where the Foo
class is used only for the output, and a different class FooInput
for the input.
I used TeamInput
as an example of a class where Input
is not an indicator that this is an input type, but rather the input of a team. This can be output as well and TeamInputInput
would make perfect sense.
Is this a duplicate of #650?
Just stumbled upon this.
I have a complex entity Foo
:
@Entity
public class Foo {
// ...
}
And a dedicated Input
for it as a record:
public record FooInput(
// ...
) {}
The Foo
is nowhere used as input of @Mutation
queries and yet a FooInput
is generated for Foo
and FooInputInput
is generated for FooInput
.
How do I disable generation of FooInput
for Foo
? How do I keep the input name of FooInput
the same as the class/record? Shouldn't GraphQL be smart enough to detect that there already exist a FooInput
type (detected in @Mutation
queries) and skip the generation of the one for Foo
?
Using GraphQL version associated with Quarkus 2.13.3.Final.
Primary reason for having dedicated input is mainly for updates, so that the user isn't forced to re-fill out all properties which are marked @NotNull
in complex entity even though they are not modified. I.e. only the ones actually present in GraphQL arguments will be mapped. Another reason is being able to specify just the ID of the nested entity instead of the whole nested entity. Yet another reason is that the entity superclass can't necessarily be modified (e.g. 3rd party lib) to add @Ignore
to e.g. id
, version
, created
, lastModified
properties because we absolutely don't want them to be updatable by GraphQL input.
@phillip-kruger any thoughts on the above?
I'm not sure, but it's worth a try to annotate your FooInput
as @Input("Foo")
.
The problem is that we can't just look at the type and not duplicate the Input
suffix. E.g. a UserInput
type could be the input for a User
class, or it could be the input that a user has generated, so it's feasible to have a UserInputInput
.
@t1 can you think of a reason why his non-input Foo
would be generating a FooInput
? Should that happen if it's not used in a @Mutation
?
This has nothing to do with @Mutation
vs. @Query
. If it's a parameter (also in a nested method, BTW), then it's an Input. If it's being returned somewhere, it's (also) a Type.
If it's a parameter (also in a nested method, BTW), then it's an Input.
These entities are indeed used as @Source
parameter in @Query
methods. But that should still not make them a Input
.
@t1
I'm not sure, but it's worth a try to annotate your
FooInput
as@Input("Foo")
.
This throws the following exception:
Caused by: graphql.AssertException: All types within a GraphQL schema must have unique names. No two provided types may have the same name.
No provided type may have a name which conflicts with any built in types (including Scalar and Introspection types).
You have redefined the type 'Foo' from being a 'GraphQLObjectType' to a 'GraphQLInputObjectType'
at graphql.schema.impl.GraphQLTypeCollectingVisitor.assertUniqueTypeObjects(GraphQLTypeCollectingVisitor.java:158)
at graphql.schema.impl.GraphQLTypeCollectingVisitor.assertTypeUniqueness(GraphQLTypeCollectingVisitor.java:150)
at graphql.schema.impl.GraphQLTypeCollectingVisitor.visitGraphQLInputObjectType(GraphQLTypeCollectingVisitor.java:71)
at graphql.schema.impl.MultiReadOnlyGraphQLTypeVisitor.lambda$visitGraphQLInputObjectType$9(MultiReadOnlyGraphQLTypeVisitor.java:106)
at java.base/java.util.Arrays$ArrayList.forEach(Arrays.java:4204)
at graphql.schema.impl.MultiReadOnlyGraphQLTypeVisitor.visitGraphQLInputObjectType(MultiReadOnlyGraphQLTypeVisitor.java:106)
at graphql.schema.GraphQLInputObjectType.accept(GraphQLInputObjectType.java:162)
at graphql.schema.SchemaTraverser$TraverserDelegateVisitor.enter(SchemaTraverser.java:109)
at graphql.util.Traverser.traverse(Traverser.java:144)
at graphql.schema.SchemaTraverser.doTraverse(SchemaTraverser.java:96)
at graphql.schema.SchemaTraverser.depthFirst(SchemaTraverser.java:86)
at graphql.schema.SchemaTraverser.depthFirst(SchemaTraverser.java:79)
at graphql.schema.impl.SchemaUtil.visitPartiallySchema(SchemaUtil.java:61)
at graphql.schema.GraphQLSchema$Builder.buildImpl(GraphQLSchema.java:917)
at graphql.schema.GraphQLSchema$Builder.build(GraphQLSchema.java:894)
at io.smallrye.graphql.bootstrap.Bootstrap.generateGraphQLSchema(Bootstrap.java:186)
Interestingly it already considered Foo
a GraphQLObjectType
in the beginning?
Sorry, my fault. It should be @Input("FooInput")
.
And you're right: using something as a @Source
parameter shouldn't make it an Input.
Sorry, my fault. It should be
@Input("FooInput")
.
I've tried this before. It throws the following exception:
Caused by: io.smallrye.graphql.schema.SchemaBuilderException: Classes com.example.model.input.FooInput and com.example.model.Foo map to the same GraphQL type 'FooInput', consider using the @Name annotation or a different naming strategy to distinguish between them
at io.smallrye.graphql.schema.creator.ReferenceCreator.putIfAbsent(ReferenceCreator.java:472)
at io.smallrye.graphql.schema.creator.ReferenceCreator.createReference(ReferenceCreator.java:276)
at io.smallrye.graphql.schema.creator.ReferenceCreator.getReference(ReferenceCreator.java:372)
at io.smallrye.graphql.schema.creator.ReferenceCreator.createReferenceForPojoField(ReferenceCreator.java:182)
at io.smallrye.graphql.schema.creator.FieldCreator.createFieldForPojo(FieldCreator.java:93)
at io.smallrye.graphql.schema.creator.type.InputTypeCreator.addFields(InputTypeCreator.java:158)
at io.smallrye.graphql.schema.creator.type.InputTypeCreator.create(InputTypeCreator.java:71)
at io.smallrye.graphql.schema.creator.type.InputTypeCreator.create(InputTypeCreator.java:36)
at io.smallrye.graphql.schema.SchemaBuilder.createAndAddToSchema(SchemaBuilder.java:266)
at io.smallrye.graphql.schema.SchemaBuilder.addTypesToSchema(SchemaBuilder.java:160)
at io.smallrye.graphql.schema.SchemaBuilder.generateSchema(SchemaBuilder.java:125)
at io.smallrye.graphql.schema.SchemaBuilder.build(SchemaBuilder.java:89)
at io.quarkus.smallrye.graployment.SmallRyeGraphQLProcessor.buildExecutionService(SmallRyeGraphQLProcessor.java:267)
Moreover, ideally the schema.graphql
should also NOT contain the generated input for Foo
. Currently it contains both FooInput
of Foo
and FooInputInput
of FooInput
.
It's difficult to investigate this without seeing the code. Do you happen to have a reproducer you can share?
Okay, I'll have to prep one first.
^^
While prepping the reproducer I discovered that having a @Source
is indeed the cause of observable issue. See the README at https://github.com/BalusC/smallrye-graphql-issue-360
It's because the method with the Source parameter also contains a query annotation, so we will tread that is a query and a source. If you only want to attach that as a field, you can remove the query annotation and that should then not create the input
That worked. Thank you very much :)
Pleasure :)