Support more options to create target objects in `@Argument` binding

[david-kubecka]

I would like to be able to customize how Spring for GraphQL instantiates objects from query/mutation arguments. Example schema:

input type Parent1 {
  child: Child
}

input type Parent2 {
  child: Child
}

input type Child {
  prop1: Int
  prop2: Int
}

type Query {
  createParent1(parent: Parent1): Parent1
  createParent2(parent: Parent2): Parent2
}

I have the corresponding controller methods which follow the standard naming convention and whose arguments are classes Parent1 and Parent2, respectively. These parent classes' structure is the same as the input types but Child class has a different structure than the corresponding input type (e.g. input prop1 is used as an argument to a function that populates target prop3). I would like to be able to define a custom deserializer of the Child class from the internal argument submap.

Currently, I'm achieving this manually, i.e. my controller method looks like this:

@MutationMapping
fun createParent1(@Argument parent: Map<String, Any>) {
  val parentTransformed = parent.findAndTransformNestedChild()
  return mapper.convertValue(parentTransformed, Parent1::class.java)
}

This works but must be repeated for each mutation/parent combination.

Is there a better way how to achieve my use case? I've initially also explored a way to define an input object directive but I couldn't figure out whether/how an input data transformer can be set there.

[rstoyanchev]

Have you tried registering a Converter? You can do that by providing a FormatterRegistrar to AnnotatedControllerConfigurer.

[david-kubecka]

Thanks for pointing out the general direction. Currently, I have this skeleton:

class CustomFormatterRegistrar : FormatterRegistrar {
    override fun registerFormatters(registry: FormatterRegistry) {
        registry.addConverter(CustomConverter())
    }
}

class CustomConverter : Converter<String, Child> {
    override fun convert(input: String): Child {
        ...
    }
}

@Configuration
class GraphQlConfig(val objectMapper: ObjectMapper, controllerConfigurer: AnnotatedControllerConfigurer) {
  init {
        controllerConfigurer.addFormatterRegistrar(CustomFormatterRegistrar())
  }
}

I hope I've done the wiring right... Anyway, I'm struggling to figure out what the converter source type should be. I've tried both String and Map<String, Any> but neither triggers my converter.

[rstoyanchev]

There is some detail missing from the description, so if you want us to take a closer look and give you specific advice, please provide a small sample.

[david-kubecka]

Hi. I've created a sample app that demonstrates the problem: https://github.com/david-kubecka/graphql-custom-converter/tree/main. I believe I just didn't get how the custom converter should be wired into GraphQlConfig and/or what should be the source type of the Converter.

Thank you for looking into this!

[rstoyanchev]

Thanks for the sample.

This case requires use of a static factory method to create the target type, and that's not supported. Currently, we support using a single, public, constructor with arguments, or a default constructor and then binding via properties. The ConversionService is used in addition, mainly to assist with converting scalar values where needed. We could consider an enhancement for what to do when there is no suitable constructor for the target object.

One option is to expand use of the ConversionService to check if it supports conversion from Map to the target type, but the converter would then have to deal with converting sclara values, and potentially with nested ones too. None of these are much of an issue in this case, but not ideal as a general solution.

Another option is to check for static factory methods, but there may be several of them and it could get tricky to select the right one with overloaded methods and/or nested input. Furthermore, such factory methods might not even be on the target type, like the case here where Money.of is used to create MonetaryAmount. Ideally, we just need to be pointed to the factory method to use, and we can handle the rest.

Whatever mechanism we choose could also be useful for selecting among multiple data constructors.

[rstoyanchev]

As for a workaround, if you have flexibility to change the target class, you could create a class with a data constructor that matches the input. That target class would take that as its input, and then convert internally. For example in this case:

data class MoneyValue(val amount: Float, val currency: String)

class Transaction(val value: MoneyValue, val transactionType: TransactionType) {

    val monetaryAmount: MonetaryAmount

    init {
        monetaryAmount = Money.of(value.amount, value.currency)
    }
}

This could work if the target class is specifically for GraphQL input, and not a domain entity class,

[david-kubecka]

expand use of the ConversionService to check if it supports conversion from Map to the target type

That's what I would envision to be a good general solution. The concerns you mention (nested types or scalars conversion) are valid but IMO if one resorts to a custom conversion he might not expect that the generic conversion mechanism would plug in automagically. (OTOH it would certainly be good if it was possible to hook onto the generic mechanism explicitly! I.e. in my example I might want to define a custom converter for Transaction but still would like to reuse built-in converter for the TransactionType enum or custom scalars.)

The ConversionService is used in addition, mainly to assist with converting scalar values where needed

Does this mean that the S source type in Converter<S, T> can't in fact be anything else than String? Also, do I understand it correctly that spring-graphl calls convert only for the scalar input/leaf types? (That would explain why might converter was not used at all.)

As for a workaround

I could use that but it suffers from the same issues I mentioned in my original post, i.e. I would need to employ this little bit cumbersome strategy whenever I need the Money type. Moreover, in my actual code I don't use Money.of at all - that was just for illustration purposes. Instead, I register a specific jackson Module which would ideally facilitate the conversion, e.g.

objectMapper.convertValue(input as Map<*, *>, Money::class.java)

With your suggestion, I would somehow need to acquire objectMapper which is not very practical.

Additionally, I don't think it would be a good idea to support static factory methods (for similar reasons you mentioned). Instead, I would rather like to be able to convert from a Map as this seems to be the most flexible option to me (AFAIK). Would you consider changing the scope of the issue, then? Or do you see yet another option?

[rstoyanchev]

Thanks for the additional feedback.

A Converter is really meant for Java type conversion, and that means any Java type, including maps, but turning a Map into a an Object is something more involved, in effect object mapping. That can be simple if the target Object is simple, but becomes challenging quickly as the target object structure becomes more complex.

So, while using a Converter is an option, it's not the right contract. A Converter implementation can use an existing object mapper like Jackson's to do the heavy lifing, nothing wrong there. I'm just not sure about putting it behind Converter. An implementation from scratch would not make much sense. You would also need only one implementation since Jackson can convert Map to any target Object. It does not make sense to present this as a converter.

Originally, we did use Jackson for GraphQL argument binding, but moved away from it because it can interfere with custom scalar values, see #122. We now have GraphQlArgumentBinder to do the work of object mapping, by navigating the input Map and creating the target Object structure recursively. This works well generally, and supports both constructor and property setter initialization. However, clearly there are cases where there is no suitable constructor.

One option is for us to provide a way for the application to suggest how to create a specific object, e.g. by pointing to a static factory method. We can do the rest of the work from there, matching and converting map values, creating nested objects, etc.

Another option would be to allow use of Jackson, or another object mapper, for specific parts of the input map, but this would be a little contradictory with the changes in #122, and I'd like to understand the use case very well.

You mentioned that you need to use Jackson, but I'm wondering if that's really necessary?

[rstoyanchev]

After a team discussion, we've decided this.

We'll continue to enhance GraphQlArgumentBinder to support more options for how to create an object when there is neither a single, public, data constructor, nor a default constructor to use along with property setters. Rather than giving up, we can try more things. Support for static factory methods is a natural choice. We could look for a single factory method, but also provide some way to register it.

If none of this works, we could provide a hook as a final fallback to convert from a Map<String, Object> to a target Object. This could be based on Converter, or a subtype with more specific generic types, but either way, it would likely be provided as a single instance, and directly to GraphQlArgumentBinder, rather than registered in the ConversionService.

We may also combine these concepts into one, dedicated contract that gives a range of options, from selecting a static factory method or a data constructor among several, to a function to take over the conversion of a Map to the target object.

The goal is to make it feasible to leave as much as possible to GraphQlArgumentBinder to do most of the work, but also leaving a hook in case you really need to take over.

[david-kubecka]

Thank you for the detailed explanation. It's much appreciated! Now I understand why you originally steered more toward the static factory methods support and it seems to make sense.

One additional question: Theoretically, one could do whatever they want in the factory method, including using an object mapper. So perhaps natively supporting conversion from Map<String, Object> might not be needed at all.

[rstoyanchev]

Good point, will keep that in mind.

[david-kubecka]

Another related question: Is GraphQlArgumentBinder used only for binding the input objects or also for result/output objects? If it's the former what is the native GraphQL serializer of output objects and how can I get access to it e.g. in tests (using HttpGraphQlTester)?

[david-kubecka]

I have reformulated that ^ to a separate issue #569.

[david-kubecka]

After reading the source code, I wonder whether a simple first step for this couldn't be to allow extending the list of argumentResolvers. It wouldn't probably solve my original problem but it would solve another situation I also encounter quite often when my input type class doesn't have a suitable constructor. Currently, I have this repeating pattern

fun someMutation(@Argument inputMap: Map<String, Any>): Output {
   val someTypeInput = mapper.convertValue(inputMap["someType"], ClassWithoutSuitableConstructor::class.java)
   ..
}

That could easily be abstracted away to a custom resolver, i.e. based on a custom argument annotation. I can file a separate issue if this makes sense to you.

[david-kubecka]

Hmm, thinking about it more I wonder why spring-graphql itself doesn't use this approach. The inputMap contains the input data properly formatted (e.g. with custom scalar conversion) so the resulting object should always be correct. Perhaps it's just for performance reasons? If that's so would you consider applying this "map -> convertValue" strategy as a fallback if there's no suitable constructor?

[bostandyksoft]

Hi. I've read this thread and it still is not clear for me.

for example, i have several entites based on one:

abstract class VehicleInput { ... }

class CarInput extends VehicleInput { ... }
class BoatInput extends VehicleInput { ... }

and i have a mutation

mutation { saveVehicle(input: VehicleInput): Vehicle }

for WebMVC and jackson i had described annotation-based contract witj @JsonTypeInfo and @JsonSubTypes. And it worked perfectly. But now i got error

org.springframework.beans.BeanInstantiationException: Failed to instantiate [com.test.VehicleInput]: Is it an abstract class?

because there is not used jackson. Is there correct way to map all of my classed?