RelayConnection wrapped by a Union causes issues

[starJammer]

tl dr: Is the code in the sample project a good or bad solution for handling RelayConnections wrapped by a union? It uses a solution similar to the existing ConnectionFieldTypeVisitor.

Problem Description

Relay Pagination is supported in Spring Boot GraphQL so that in the following schema the integers field will work correctly, but the integersUnion field will not.

You can get a sample project here here: https://github.com/starJammer/spring-graphql-unions-pagination
The project's README.md file contains the everything I've written below as well.
The sample project includes the proposed solution below so it can be looked at in code.

You can start the sample project and see that no results are returned. (sample query and results below). You can also run the unit tests to check.

type Query {
    """ returns a list of integers from 1 to 1000 """
    integers(first: Int, after: String): IntegerConnection!
    """ Same as above except the connection object is wrapped in a union """
    integersUnion(first: Int, after: String): IntegerResult!
}

union IntegerResult = IntegerConnection | NoIntegersFound

type IntegerConnection {
    edges: [IntegerEdge]
    pageInfo: PageInfo
}

type NoIntegersFound {
    message: String!   
}

type IntegerEdge {
    node: Int
    cursor: String   
}

type PageInfo {
    startCursor: String
    endCursor: String
    hasNextPage: Boolean
    hasPreviousPage: Boolean
}

# Sample query
query Integers {
	integersUnion {
		__typename
		... on IntegerConnection {
			pageInfo {
				startCursor
				endCursor
				hasNextPage
				hasPreviousPage
			}
			edges {
				node
			}
		}
		... on NoIntegersFound {
			message
		}
	}
}

There is no error but the service doesn't return any data. Instead it returns this:

{
	"data": {
		"integersUnion": {
			"__typename": "IntegerConnection",
			"pageInfo": null,
			"edges": null
		}
	}
}

The unit test fails with this at the high level and I included the detailed error below:

IntegersGraphQlControllerTest > generateUnionIssue() FAILED
    java.lang.AssertionError at IntegersGraphQlControllerTest.kt:67
        Caused by: java.lang.AssertionError at IntegersGraphQlControllerTest.kt:67

java.lang.AssertionError: No value at JSON path "$.data.integersUnion.edges"
Request: document='query Integers {
    integersUnion {
        __typename
        ... on IntegerConnection {
            pageInfo {
                startCursor
                endCursor
                hasNextPage
                hasPreviousPage
            }
            edges {
                node
            }
        }
        ... on NoIntegersFound {
            message
        }
    }
}'
.....

This is the expected result (notice that the edges array is not null):

{
	"data": {
		"integersUnion": {
			"__typename": "IntegerConnection",
			"pageInfo": {
				"startCursor": "S19bImphdmEudXRpbC5Db2xsZWN0aW9ucyRVbm1vZGlmaWFibGVNYXAiLHsic3RhcnQiOjF9XQ==",
				"endCursor": "S19bImphdmEudXRpbC5Db2xsZWN0aW9ucyRVbm1vZGlmaWFibGVNYXAiLHsic3RhcnQiOjEwfV0=",
				"hasNextPage": true,
				"hasPreviousPage": false
			},
			"edges": [
				{
					"node": 1
				},
				{
					"node": 2
				},
				{
					"node": 3
				},
				{
					"node": 4
				},
				{
					"node": 5
				},
				{
					"node": 6
				},
				{
					"node": 7
				},
				{
					"node": 8
				},
				{
					"node": 9
				},
				{
					"node": 10
				}
			]
		}
	}
}

Solution Description

Open the GraphQLUnionConfiguration.kt file in the project and go to line 23 and uncomment it so the bean gets created. (make sure to uncomment or add the related import too at the top of the file).

Run the unit tests again you'll see they succeed. You can also run the service and attempt the query again and notice that it works. (or it should the last time I tested)

If you pass in a negative first value you will see that the NoIntegersFound object is returned instead of an IntegerConnection.

GraphQLUnionConfiguration will register the ConnectionUnderUnionTypeVisitor I created in the project.

Read comments on the class for more details but in short it will:

1. Look for union types being returned and in a decorator data fetcher.
2. The decorator data fetcher will execute the original data fetcher and if the return value
   is a SliceWrapper or WindowWrapper it will adapt the return value to a Connection object
   the same way that ConnectionFieldTypeVisitor does. It also adds a type resolver for the union
   so it resolves the connection object to the Connection type.

Limitations:

1. Does not support multiple connection objects under a union.
2. Not sure if this is a good approach or if there is a better one.

[bclozel]

Hello @starJammer

We recently changed the generated schema for pagination/cursor connection support in 2.0.0-M3 with #1296.
It should be now possible to return a null value and you shouldn't get the "NullValueInNonNullableField" error anymore.

I tried upgrading your application but unfortunately there is a lot going on there between spring-graphql changes, graphql-java changes (removals and nullability changes that affect Kotlin), and quite a few unrelated plugins and features.

Can you give it a try and let us know? Don't hesitate to reach out if you have questions about the Spring Boot upgrade.
Thanks!

[starJammer]

Hey @bclozel I'm glad you took the time to respond and try it out!

What did you try upgrading exactly?

The example project runs on Spring boot version "3.4.1". The change you mentioned in how the schema is generated seems unrelated. Were you trying to upgrade to a later spring boot version?

I can update my project to whatever spring version you were attempting and change the schema so it doesn't use the ! for the edge list. I think that the issue I'm trying to get at is unrelated to the nullability of the list.

The main focus of this issue is that it's not possible to put a Connection type in a union because it won't be resolved by the existing ConnectionFieldTypeVisitor.

[bclozel]

I was trying to upgrade to Spring Boot 4.0.0-SNAPSHOT because the issue I linked to is changing the connection types we are generating at runtime. The edge type is now made nullable, and this is the main symptom in the error message that you are sharing.

I was merely trying to check whether this change unlocks your setup.

If the main issue is about supporting your own connection type, I think the expected solution would be to define the connection types in your schema so that spring graphql backs off and you can take things into your own hands.

[starJammer]

Ahhh!! OK, that explains it.

I took some time to upgrade this but currently I set the project to Spring Boot 4.0.0-M3 and to Spring GraphQL 2.0.0-M3.

Can you take a look again @bclozel ?
I can still reproduce the error. I'm not sure if gradle was an issue for you but I set it up with a unit test so you don't have to run the service or query it.

The unit tests can be run with ./gradlew clean build and you'll see the failure.
Then if you uncomment line GraphQLUnionConfiguration:23 to add the bean back the unit test will pass due to the functionality in from the ConnectionUnderUnionTypeVisitor that I created to solve this.

The ConnectionUnderUnionTypeVisitor I created is HIGHLY based on the existing class org.springframework.graphql.data.pagination.ConnectionFieldTypeVisitor and if you look at them side by side (despite the kotlin/java difference), you'll see the similarities and some of the extra functionality I wrote to support the Connection type under a union.

[bclozel]

Thanks for the help @starJammer

I had a deeper look and discussed it with @rstoyanchev earlier today.

Looking at the sample application, it is in full control with the ConnectionUnderUnionTypeVisitor and that's a valid choice if you consider that spring-graphql doesn't work for your case. From my perspective, spring-graphql does not support union types for connections and a custom type visitor is required to make it work.

I guess that behind the title of this issue, you are also asking whether the team considers this a valid approach and how we could make it easier for applications.

Our opinion is that allowing union types for Connections is not something that we would consider in Spring for GraphQL. If you're trying to add information to the response about errors, I think that enriching the existing Connection type is a better option and mentioned in the specification. We do not support this case right now but this is something we could consider if you can share a more detailed use case and if such an approach would solve your problem.