smallrye-graphql icon indicating copy to clipboard operation
smallrye-graphql copied to clipboard

Problems with annotations like nonnull and lists

Open robp94 opened this issue 3 years ago • 9 comments

I found more problems with annotations and lists.

Java

Code like this

    @Query
    public String testNull(List<String> list)

    @Query
    public String test1(@NonNull List<String> list)

    @Query
    public String test2(List<@NonNull String> list)

    @Query
    public String test3(@NonNull List<@NonNull String> list)

results in a schema like this:

type Query {
  test1(list: [String]!): String
  test2(list: [String!]!): String
  test3(list: [String!]!): String
  testNull(list: [String]): String
}

which seems not correct, even so I don't know if anybody would need

test2(list: [String!]): String

instead of the actual result.

The problem is that the typesafe client seems to handle it correctly.

With an api like:

    @Query
    public String test2(List<@NonNull String> list);

which calls test2 from above, the result is

Variable type '[String!]' doesn't match expected type '[String!]!' @ 'test2', locations=[{line=1, column=45}], path=null, extensions={classification=ValidationError}})

Kotlin

There seems to be another discrepancy between client and server

    @Query
    fun test2(list: List<@NonNull String>): String

    @Query
    fun test3(list: @NonNull List<@NonNull String>): String

result in schema

  test2(list: [String!]!): String
  test3(list: [String!]!): String

But both test fail with

Variable type '[String!]' doesn't match expected type '[String!]!

I think that both server and client should fit together.

Furthermore, test1 is wrong in both client and server, but could be some kotlin problem.

    @Query
    fun test1(list: @NonNull List<String>): String

Graphql:

test1(list: [String]): String

Note kotlin is a bit special with annotations https://youtrack.jetbrains.com/issue/KT-35843 we need an extra flag.

Similar apis for client and server in java and kotlin.

code-with-quarkus-nonnull.zip code-with-quarkus-nonnull-kotlin.zip

robp94 avatar Apr 21 '22 15:04 robp94

I think test2(list: [String!]): String means that the list itself can be null, but if not null, the elements in the list can not be null. Something like that.

I am trying to follow the issue, but I can not figure out exactly what is wrong. Are you say the client is wrong ? Or the server ? Or both ?

phillip-kruger avatar Apr 22 '22 00:04 phillip-kruger

Both.

Okay, taking a step back: The server and client produce different schema for the same code here. The schema I'd like to produce is

type Query {
  testNull(list: [String]): String
  test1(list: [String]!): String
  test2(list: [String!]): String
  test3(list: [String!]!): String
}

Java Code for client and server:

    @Query
    public String testNull(List<String> list)
    @Query
    public String test1(@NonNull List<String> list)
    @Query
    public String test2(List<@NonNull String> list)
    @Query
    public String test3(@NonNull List<@NonNull String> list)

Schema Server:

type Query {
  testNull(list: [String]): String
  test1(list: [String]!): String
  test2(list: [String!]!): String
  test3(list: [String!]!): String
}

Schema client:

type Query {
  testNull(list: [String]): String
  test1(list: [String]!): String
  test2(list: [String!]): String
  test3(list: [String!]!): String
}

so, test2 fails.

Using Kotlin there seems to be a different problem: Kotlin Code

    @Query
    fun test1(list: @NonNull List<String>): String
    @Query
    fun test2(list: List<@NonNull String>): String
    @Query
    fun test3(list: @NonNull List<@NonNull String>): String

Server schema:

  test1(list: [String]): String  
  test2(list: [String!]!): String
  test3(list: [String!]!): String

Client schema

  test1(list: [String]): String  
  test2(list: [String!]): String
  test3(list: [String!]): String

and both are different from the schema I'd like to have (the one from the first block above)

robp94 avatar Apr 22 '22 07:04 robp94

Ok, so test 2 generate wrong on the server it seems ?

phillip-kruger avatar Apr 26 '22 00:04 phillip-kruger

And test 3 generates wrong for the client ?

phillip-kruger avatar Apr 26 '22 00:04 phillip-kruger

The client seems to be correct (in Java, let's ignore Kotlin for a moment, as that seems to be a different issue). So only test2 in the server is wrong.

t1 avatar Apr 27 '22 05:04 t1

Is there any progress on this issue? Especially case test2 in the kotlin world is right now a show stopper for me in Quarkus.

pschyma avatar Oct 14 '22 05:10 pschyma

No progress. But we accept Pull requests :)

phillip-kruger avatar Jan 06 '23 02:01 phillip-kruger

@jmartisk - do you have capacity to look at this ?

phillip-kruger avatar May 01 '23 06:05 phillip-kruger

I've spent many hours trying to fix all the nullability scanning, but especially generics (and Kotlin) make it really hard. I can try to look again perhaps next week

jmartisk avatar May 02 '23 06:05 jmartisk