Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GraphQlTester should go by the schema when serializing variables #569

Open
david-kubecka opened this issue Dec 7, 2022 · 11 comments
Open
Labels
type: enhancement A general enhancement
Milestone

Comments

@david-kubecka
Copy link

Motivation

My output types in the schema are the same as my input types except the latter ones don't have id field. For conciseness reasons, I use the same underlying DTO with nullable id for both types. This works well in the main code flow because id is set to null on input while it has some value on the output.

In the tests, though, I want to use my DTOs as input variables, e.g.

graphQlTester
            .document(SOME_QUERY)
            .variable("var", SomeDtoWithIdField)
            .execute()

Now I obviously don't want the variable var to contain the id field because it's not defined in the input type corresponding to SomeDtoWithIdField. I was thinking about how to drop the id from the serialized var and thought that the following code surely wouldn't work because "GraphQL does not use Jackson annotations to drive JSON serialization/deserialization".

class SomeDtoWithIdField {
    @get:JsonIgnore
    var id: Long? = null
}

But my colleague tried that anyway and to my surprise, it worked :-) That is, in my test case the id is not serialized into var while in the main flow the id is serialized to the output type. My hypothesis is that while in the normal flow spring-grqphql uses some custom serializer, GraphQlTester uses Jackson.

Questions

  • Is this expected behavior? Shouldn't rather GraphQlTester use the same serializer as the main flow?
  • If both flows should indeed behave consistently, then how could I achieve my use case? Is it somehow possible to hook into the serialization process?

(This question can be read as a sort of reverse of #521)

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Dec 8, 2022

Not sure what you mean by normal flow because serialization happens on the client side and it could be any client, any language, and this is completely independent of anything GraphQL Java does on the server side which involves only deserialization for request input.

GraphQlTester is a client, and it delegates to GraphQlTransport, just like GraphQlClient does, and it passes a GraphQlRequest to it with maps of data. If the transport is WebTestClientTransport then will be serialized to JSON via Jackson. This is all expected.

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Dec 8, 2022
@david-kubecka
Copy link
Author

serialization happens on the client side

I don't understand this. GraphQL Java must somehow convert/serialize the return types of the queries/mutations to JSON and custom scalar converters must be taken into account, i.e. generic mappers like Jackson cannot be used.

OTOH I understand that WebTestClientTransport uses Jackson to serialize the variable objects. Is there any way how to use the "native" GraphQL serializer in tests? If yes how would I achieve my use case of ignoring the id field?

My motivation basically is that while the approach with @get:JsonIgnore works for me that's only because I don't need any custom scalar converters in this particular case, so I would like to know what the generic solution would be.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 8, 2022
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Dec 8, 2022

No, GraphQL Java doesn't serialize anything to JSON. It goes field by field and assembles the output map.

I'm not really sure what's going on from this level of detail. Something less than obvious perhaps. The only other suggestion I have is that maybe @JsonInclude(Include.NON_NULL) could work better for what you're describing.

@david-kubecka
Copy link
Author

It goes field by field and assembles the output map

Is this method also used if the transport is WebTestClientTransport? If yes then I got something wrong in my configuration as the JsonIgnore shouldn't actually work. If not is there any way how to obtain this output map also in the testing use case?

(Perhaps there's just a misunderstanding of what serialization really means in this case. Creating the map out of the object is IMO also part of the serialization process...)

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Dec 9, 2022

Is this method also used if the transport is WebTestClientTransport?

No, that's apples and oranges.

What I talked about is GraphQL Java on the server side returning an ExecutionResult with a map of data and errors assembled field by field. This is then written out to JSON by our server HTTP transport, but it's just writing the Map to JSON, there are no higher level Objects involved.

What you're asking about is WebTestClientTransport which is on the client side, and that has nothing to do with what GraphQL Java does on the server side.

@rstoyanchev
Copy link
Contributor

Is it possible to provide a small sample that demonstrates a concrete issue?

@rstoyanchev
Copy link
Contributor

Re-reading the description again, I think I'm beginning to understand.

I think your expectation is that GraphQlTester will compare SomeDtoWithIdField to the corresponding GraphQL schema input type and prepare maps of data that contain only the fields that exist in the schema type, and therefore the id field should not be in the resulting output.

GraphQL Java indeed does something similar, but that's on the server side and it's for responses, and it's slightly different because it's checking against the field selection set rather than directly against the schema types. In any case, it's not something that can be re-used on the client side.

It's a valid potential enhancement, assuming I've got it right, for GraphQlClient and GraphQlTester to turn variables into maps of data, by looking at the corresponding schema input and taking only what's needed, before passing that down to the transport to serialize to JSON.

@rstoyanchev
Copy link
Contributor

I've improved that documentation note in 089c754, based on this conversation.

@david-kubecka
Copy link
Author

I think your expectation is

Exactly! I think, though, the problem has two parts:

  • an ability to turn variables into maps of data on the client side according to the schema
  • ignoring some keys in the map based on the required input type
    These could be treated separately (or rather incrementally) if that would simplify things.

Anyway, I'm glad that you consider this as an enhancement. This would greatly improve code reuse in tests because typically you have very similar fixtures for asserting server output as for providing server input (on the client side).

@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Jan 11, 2023
@rstoyanchev rstoyanchev added this to the 1.2 Backlog milestone Jan 11, 2023
@bclozel bclozel changed the title GraphQlTester should use native GraphQL serialization for variables GraphQlTester should go by the schema when serializing variables Jan 12, 2023
@david-kubecka
Copy link
Author

I don't know of a more appropriate place so I will try to ask a related question here.

Is it possible to use a different ObjectMapper for GraphQlTester client than for the server in a single SpringBootTest? I need the serialize test data differently than deserialize them on the server.

@rstoyanchev rstoyanchev modified the milestones: 1.2 Backlog, 1.x Backlog Apr 14, 2023
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Apr 18, 2023

It depends on the types of tests and GraphQlTester variant but for each it's possible to customize. For HTTP, it's connected to the codecs in the underlying WebTestClient, for WebSocket you can provide a CodecConfigurer through the WebSocketGraphQlClientTester builder, or for ExecutionGraphQlServiceTester you can provide an Encoder and Decoder on the builder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants