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

Add getInstance as a way to do mapping. #642

Closed
patope opened this issue Feb 19, 2021 · 13 comments
Closed

Add getInstance as a way to do mapping. #642

patope opened this issue Feb 19, 2021 · 13 comments
Assignees
Milestone

Comments

@patope
Copy link

patope commented Feb 19, 2021

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
Copy link
Member

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
Copy link
Author

patope commented Feb 19, 2021

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
Copy link
Member

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
Copy link
Author

patope commented Feb 19, 2021

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
Copy link
Author

patope commented Feb 19, 2021

Query example

mutation c {
  
  createData(data:{
    name: "Hello"
    currency: "EUR"
  }) {
    id
		currency
    name
  }
}

@phillip-kruger
Copy link
Member

What happens if you @ToScalar on the Currency field ?

@phillip-kruger
Copy link
Member

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
Copy link
Member

Will that work for you ?

@patope
Copy link
Author

patope commented Feb 19, 2021

@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
Copy link
Author

patope commented Feb 19, 2021

Will that work for you ?

yes. Thanks!

@phillip-kruger phillip-kruger changed the title How to use own jsonb in execution service? Add getInstance as a way to do mapping. Feb 19, 2021
@phillip-kruger phillip-kruger self-assigned this Feb 19, 2021
@phillip-kruger phillip-kruger added this to the 1.0.22 milestone Feb 19, 2021
@phillip-kruger
Copy link
Member

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
Copy link
Author

patope commented Feb 23, 2021

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

@phillip-kruger
Copy link
Member

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

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

No branches or pull requests

2 participants