No ConnectionAdapter IllegalArgumentException for existing Java Connection type #709
Comments
spring-projects-issues
added
the
status: waiting-for-triage
|
If anyone is curious, for now, the interconnectedness of the spring ecosystem made it a bit tricky to only revert this library (should be doable but don't have the time for that), so to keep things simple I ended up pinning spring boot to 3.0.7 away from 3.1.0 (including the spring-boot-parent etc.). |
The only thing that Generally, the idea is it shouldn't be necessary to opt out explicitly. At runtime we also check if what the |
rstoyanchev
added
type: enhancement
rstoyanchev
changed the title
|
Sure thing, sorry for the sparse details. So to be clear, this is code/schema that predates pagination support, where Maybe it becomes a conscious decision to be more convention-over-configuration on this aspect, deciding to implicitly treat any type ending in Here is a portion of the schema related to type PuzzleEdge {
puzzle: Puzzle
cursor: String!
}
type PuzzlePageInfo {
total: Int!
hasPreviousPage: Boolean!
startCursor: String
hasNextPage: Boolean!
endCursor: String
}
type PuzzleConnection {
pageInfo: PuzzlePageInfo!
puzzles: [PuzzleEdge]
}Here is one example stacktrace emitted during an integration test (so there is no sensitive data): To conclude, I am interested in seeing how I can adapt to using spring-graphql's own support for pagination/connections. It seems the way to go would be to implement No stress or pressure on this by the way! I am truly grateful for all of the work you all put into this project, and my use of it is for a side-project anyway. My only aim with this ticket was (1) to make you all aware of this situation in case you weren't already and (2) to learn from you all what you believe to be the best course of action, whether on my end and/or on changes necessary in the project itself (e.g. opt-out annotations or more intelligent implicit behavior). Thanks again! |
|
@blaenk, thanks for the extra details. The issue is scheduled for 1.2.1, and goal is to find a fix.
I have some suggestions, but let's make the existing code work first. |
ConnectionFieldTypeVisitor now checks the complete structure of Connection, Edge, and PageInfo schema types before decorating a DataFetcher. See gh-709
ConnectionFieldTypeVisitor now also checks if the container type ends on "Connection", and if so it lets it pass through. See gh-709
|
I've made a couple of updates, 1) to check the complete structure of a Connection schema type before deciding to apply a
It could be opt-in or opt-out, but it makes sense to strengthen checks first, and exhaust our options. In the end, my sense is that we will be able to do without that. |
|
Sure I'm happy to help try it out! I can confirm that the issue appears to be resolved with everything else on 3.1.0, and an explicit dependency of: <dependency>
<groupId>org.springframework.graphql</groupId>
<artifactId>spring-graphql</artifactId>
<version>1.2.1-SNAPSHOT</version>
</dependency>I appreciate your prompt response! Let me know if there's anything else you need. As for adapting my code, it seems like I would need to implement Once I do that, then spring-graphql will detect that it is of type |
|
Thanks for confirming the fix!
The idea with our pagination support is that you don't have to implement these types. Instead, you would return the actual result items however they may be represented in your underlying service/data layer, and then configure a For example if using Spring Data repositories, you would naturally have This does mean that you would need to change your schema to match the GraphQL Cursor Connection spec, which might not be an option, and this is why we make sure your exiting pagination can continue to work as it did before. This is just to give you an idea, but have a look around the documentation for more details. |
|
Thanks again @rstoyanchev. No rush or anything but any idea more or less when to expect 1.2.1 to be released? |
|
@rstoyanchev if I understand correctly, the fix planned for 1.2.1 only applies to connection types that do not follow the spec structure. In our application we're following the spec structure, but we don't want to update all our connection implementations all at once. |
|
@koenpunt, it should work if your connection implementation is based on |
|
@blaenk I've set the release date for 1.2.1 for June 20. We could decide to release sooner depending on the issues that come in, but it's good to allow some additional time for feedback, and would be timed with the Boot 3.1.1 release on June 22. In the mean time, looking at the Boot auto-configuration that installs the |
|
Our connections currently aren't based on the I'm not sure what you mean with "it should work", because we don't have an I believe that if this part of the autoconfiguration would be excluded the error would disappear, but haven't tried that yet; |
|
I have to add that I haven't tried the snapshot yet, so will do that first and report back. |
There is no suggestion that you should have to change anything.
You don't need to have a Please give it a try, and create a new issue if needed. |
|
The snapshot seems to work without changes to our current connection implementations 👍 |
@rstoyanchev thank you for the clarification. I'm just curious if it also works for reactive annotated controllers? For example: Thanks! |
|
@mzvankovich yes that is supported. |
I had been following the development of pagination support in #103.
In my setup, I already had types like
PuzzleConnection. Upon updating to the release of spring-graphql that added pagination support, I am now getting exceptions like:I looked through the reference and even a bit of the code (the starter/autoconfigure-er) but couldn't see anything about opting out either entirely or on a case-by-case basis via graphql annotations, or even implicitly opting out such as when it detects that the
EdgeandConnectiontypes already exist (which might be something worthwhile to add?).Should I just pin to the prior version until something is implemented? Do you recommend that I do something to prevent
ConnectionTypeDefinitionConfigurerfrom being registered?Thanks for all your work on spring-graphql!
The text was updated successfully, but these errors were encountered: