Customizing InputArgument parsing

[beikov]

Hi,

I saw that the input argument handling was made completely static with this change: 2fe40a9#diff-0d8ecc929924d083759c48f18c0bda7a0671208191ccdd30ac743aaedd264b4e

and before it was only possible to "hack" into DgsSchemaProvider to change the internal ObjectMapper for custom deserialization logic.

Can you please provide a hook for replacing the parsing of InputArgument?

My main use case is that I want to accept an interface as input argument for a mutation. Apart from that, my desired deserializer also controls how objects are constructed.

[paulbakker]

This issue contains the reasoning for the custom implementation and not using Jackson: #549

In short, the input we're getting is not JSON, so handling it as JSON seems to work in many scenarios, but its fundamentally broken. This means that replacing the implementation would be far more involved than just using something like Jackson, and will most likely introduce other issues. Because it's such as hairy topic, I'm thinking it's probably not a good idea to make this customizable, because it's going to cause more issues than it solves.

What we could consider is adding support for the specific use cases you find missing in the implementation.

[beikov]

I don't care how the string is parsed. In fact, I don't even want to know about the parsing :) I can imagine though that there are some parsing requirements with respect to support various basic types e.g. timestamp etc., but that's a different topic.
All I need is control over how objects are constructed and how attributes are set on the objects, so if you could introduce an abstraction for that, this would be enough for my purposes. Something as simple as this for example:

interface ObjectMapper {
  <T> T fromAttributes(Map<String, Object> map, java.lang.reflect.Type t);
  default <T> T fromAttributes(Map<String, Object> map, Class<T> clazz) {
    return (T) formAttributes(map, (Type) clazz);
  }
}

Where the value for a map entry referring to a nested object would be a Map<String, Object> again.

[paulbakker]

Isn't that the same as using `DataFetchingEnvironment.getArgument(...)` directly?

…

On Fri, Dec 10, 2021 at 2:21 AM Christian Beikov ***@***.***> wrote: I don't care how the string is parsed. In fact, I don't even want to know about the parsing :) I can imagine though that there are some parsing requirements with respect to support various basic types e.g. timestamp etc., but that's a different topic. All I need is control over how objects are constructed and how attributes are set on the objects, so if you could introduce an abstraction for that, this would be enough for my purposes. Something as simple as this for example: interface ObjectMapper { <T> T fromAttributes(Map<String, Object> map, java.lang.reflect.Type t); default <T> T fromAttributes(Map<String, Object> map, Class<T> clazz) { return (T) formAttributes(map, (Type) clazz); } } Where the value for a map entry referring to a nested object would be a Map<String, Object> again. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#775 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA2XLHFEMVISUGERRS4FRLUQHIBTANCNFSM5JVKSPXQ> .

[beikov]

I want users to be able to write code like this:

@DgsMutation
public Long createCat(@InputArgument(name = "cat") CatCreateDto dto) {
...
}

and for this to work, a contract is needed that handles the construction of argument objects. I don't want to force users to bind to some implementation for the parsing/construction of the object. That's just boilerplate that the declarative model of DGS should get rid off, no?

[EugenMayer]

@beikov hey :) Just to understand the issue a little better and be part of the discussion, could you make it a little more clear if this is a conveniens effort to make the input argument binding easier with BP is used, or is this a critical requirement to get mutations going with blaze persistence?

@paulbakker as a side note, we are heavy users of blaze-persistence which @beikov is the author of, and we worked with him in the last months to get a DGS integration up and running, which we are currently already using in production (you might know, we had a couple of discussions/bugs we looked at with you together). This said we are yet only using fetchers, mutations are on our list. Regarding how deeply integrated BP is in our persistence model it would be a huge bummer if mutations are not working with DGS - this would mean we would need to remove DGS and replace it with something else (which we actually do not want to). We would love to use DGS and BP together, it is actually a great match!

[paulbakker]

A value in the map of inputs can be of type List. For an array of objects, the items in the list would be another Map<String, Object>. For "simple" values, they are the converted scalar values.

For further context, this is the code that converts Lists in our InputObjectMapper: https://github.com/Netflix/dgs-framework/blob/master/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/InputObjectMapper.kt#L171
Unfortunately its quite complex due to generics.
Do you need another method to hook into in interface I added for lists?

[beikov]

Well, I was just thinking about how I will need to construct the whole object graph now from within that single method and was curious about collections, but it sounds to me like this should work out. My original suggestion to use java.lang.reflect.Type and KType could be used to implement a recursive approach per field, but I guess this works good enough as well.

[EugenMayer]

Just that you guys know, i still follow, but i cannot really add much - @beikov does all the work here and has about 1 gazillion more insight, so i just keep quiet and follow.

Super happy though that this issue is moving forward, thank you so much for the effort!

[paulbakker]

@beikov Also, if this wasn't clear yet, for each level of object in the graph your custom mapper will be called again: https://github.com/Netflix/dgs-framework/pull/842/files#diff-674a7494b4ba6668d6f6e9af0b0ad17bc3c672e13f0fdc65cee6a62a94998138R81.

The tests in the PR demonstrate this as well. This means you don't have to construct the whole graph from the root, you can make modifications at each child object and call back into the default mapper.

[beikov]

Yeah, I guess this should work for me. Thanks!

[EugenMayer]

@beikov @paulbakker we are now at at stage were we finally start with mutators and now hit this thing.

I did not remember and find this issue directly, thus search around custom scalars and things like that, leading to nothing when going down the road.

As far as i can see @beikov this is a similar take then we alware have with the spring-webmvc "jackson" integration (EntityViewAwareMappingJackson2HttpMessageConverter) which works as an converter registered with BlazePersistenceWebConfiguration` in the WebMVC framework.

Currently, seeing the discussion around InputObjectMapper which do around the same job as EntityViewAwareMappingJackson2HttpMessageConverter i ask myself:

• How is an customize the InputObjectMapper implementation - i see there is DefaultInputObjectMapper and is created as a bean via DgsInputArgumentConfiguration - so is the intention to to decorate / inherit DefaultInputObjectMapper, expose the new DefaultInputObjectMapper as primary and the add the needed handling for custom / other input types?
• the discussion aged a bit and we are not looking at 5.x / 6.x - did anything change in the way this can be done that needs a refresher?

Thank you for the effort!

[beikov]

The idea is simple, an integration in Blaze-Persistence provides a bean for the InputObjectMapper contract which is then used to implement deserialization based on entity view logic, and falls back to the DefaultInputObjectMapper for non entity view types.