smallrye / smallrye-graphql

Implementation for MicroProfile GraphQL
Apache License 2.0
160 stars 92 forks source link

Add getInstance as a way to do mapping. #642

Closed patope closed 3 years ago

patope commented 3 years ago

JsonB is on static field on execution service. I'd like to use quarkus customized jsonb bean instance instead.

https://github.com/smallrye/smallrye-graphql/blob/master/server/implementation/src/main/java/io/smallrye/graphql/execution/ExecutionService.java#L55

phillip-kruger commented 3 years ago

Hi @patope . We can look into how to make this plugable. Maybe you can provide more detail on your use case ? What are you trying to do ?

patope commented 3 years ago

Hi! I'm trying to use java.util.Currency type on input type. By default jsonb cannot deserialize this type. I tried to define JsonbCustomizer in quarkus, but this does not seem to have any effect

@Dependent
public class JsonbConfigCustomizer implements io.quarkus.jsonb.JsonbConfigCustomizer {

  @Override
  public void customize(JsonbConfig jsonbConfig) {
    jsonbConfig.withAdapters(CurrencyStringJsonbAdapter.INSTANCE);
  }

  public static class CurrencyStringJsonbAdapter implements JsonbAdapter<Currency,String> {

    private static final CurrencyStringJsonbAdapter INSTANCE = new CurrencyStringJsonbAdapter();

    @Override
    public String adaptToJson(Currency currency) {
      return currency.getCurrencyCode();
    }

    @Override
    public Currency adaptFromJson(String str) {
      return Currency.getInstance(str);
    }
  }
}
phillip-kruger commented 3 years ago

Can you share your GraphQLApi code as well ?

Maybe we can use @ToScalar since that is what seems to happen here ? You are trying to map between a String and a complex type. Right ?

See https://quarkus.io/blog/experimental_graphql/#transformation-and-mapping

patope commented 3 years ago

Yes. Currency accepts ISO 4217 currency code as string. I want to map these back and forth.

@GraphQLApi
public class CurrencyResource {

  @Mutation
  @Description("Add new Data")
  @Transactional
  public Uni<Data> createData(Data data) {
    data.id = (long) (Math.random() * 10000);
    return Uni.createFrom().item(data);
  }

  @Input
  public static class Data {
    public Long id;

    public String name;

    public Currency currency;

  }
}
patope commented 3 years ago

Query example

mutation c {

  createData(data:{
    name: "Hello"
    currency: "EUR"
  }) {
    id
        currency
    name
  }
}
phillip-kruger commented 3 years ago

What happens if you @ToScalar on the Currency field ?

phillip-kruger commented 3 years ago

OK, Currency has a getInstance method. With a small change, we can support that for @ToScalar. I'll add that to SmallRye.

You data class will then look like this:

@Input
  public static class Data {
    public Long id;

    public String name;

    @ToScalar(Scalar.String.class)
    public Currency currency;

  }
phillip-kruger commented 3 years ago

Will that work for you ?

patope commented 3 years ago
@ToScalar(Scalar.String.class)
public Currency currency;

gives

{
  "errors": [
    {
      "message": "Validation error of type WrongType: argument 'data' with value 'StringValue{value='\n{\n    \"name\": \"Hello\",\n    \"currency\": \"EUR\"\n}'}' is not a valid 'Unknown Scalar Type [io.resys.ri.application.gql.CurrencyResource$Data]' @ 'createData'",
      "locations": [
        {
          "line": 3,
          "column": 14
        }
      ],
      "extensions": {
        "description": "argument 'data' with value 'StringValue{value='\n{\n    \"name\": \"Hello\",\n    \"currency\": \"EUR\"\n}'}' is not a valid 'Unknown Scalar Type [io.resys.ri.application.gql.CurrencyResource$Data]'",
        "validationErrorType": "WrongType",
        "queryPath": [
          "createData"
        ],
        "classification": "ValidationError"
      }
    }
  ],
  "data": {
    "createData": null
  }
}
patope commented 3 years ago

Will that work for you ?

yes. Thanks!

phillip-kruger commented 3 years ago

Hi @patope

So the fix was not as simple as initially thought, mostly because java.util.Currency is typically not indexed in Jandex and also do not have a default constructor. So once the above PR is merged (and released and available in Quarkus) you should be able to do something like this in your Data class:

        @ToScalar(value = Scalar.String.class, deserializeMethod = "getInstance")
        @JsonbTypeAdapter(CurrencyAdapter.class)
        public Currency currency;

Unfortunately you are going to need both annotations, but that does work. The @ToScalar creates a String type in the schema, and (because it's not indexed) will use getInstance to create a instance of Currency (if index that can be auto-detected). The JsonB allows JsonB binding when the default constructor is not available.

This is the CurrencyAdapter class:

public class CurrencyAdapter implements JsonbAdapter<Currency, JsonObject> {

    @Override
    public JsonObject adaptToJson(Currency currency) {
        return Json.createObjectBuilder()
                .add("currencyCode", currency.getCurrencyCode())
                .build();
    }

    @Override
    public Currency adaptFromJson(JsonObject json) {
        return Currency.getInstance(json.getString("currencyCode"));
    }
}

I hope this will work for you. I will play a bit more to see if I can make this simpler somehow, but so far this is the easiest.

Let me know.

Cheers

patope commented 3 years ago

@phillip-kruger This will work for me. Thanks!

phillip-kruger commented 3 years ago

Great. Changes should make it into Quarkus 1.12.1.Final.