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

Customize scalar type serialization #413

Open
benneq opened this issue Jun 13, 2022 · 4 comments
Open

Customize scalar type serialization #413

benneq opened this issue Jun 13, 2022 · 4 comments
Assignees
Labels
status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged
Milestone

Comments

@benneq
Copy link

benneq commented Jun 13, 2022

Currently it's very hard to use custom Value Objects with ID Scalars. Or in general: to provide custom serialization for any kind of generics / class inheritance.

Schema:

type Query {
    bookById(id: ID): ID
}

Controller:

@Controller
public class BookController {

    @QueryMapping
    Mono<BookId> bookById(@Argument BookId id) {
        return Mono.just(id);
    }

    // this is called for `@Argument BookId` and works as expected
    @Bean
    Converter<String, BookId> stringToBookIdConverter() {
        return new Converter<String, BookId>() {
            @Override
            public BookId convert(String source) {
                return new BookId(source);
            }
        };
    }

    // this is not working
    @Bean
    Converter<BookId, String> bookIdToStringConverter() {
        return new Converter<BookId, String>() {
            @Override
            public String convert(BookId source) {
                return source.value(); // this is not called
            }
        };
    }
}
public record BookId(String value) {
    @Override
    public String toString() {
        return "don't use this for serialization"; // instead this is called by GraphqlIDCoercing.serialize
    }
}

As a workaround I could implement a custom BookId scalar, with a matching scalar coercing, but this will lead to new problems:

interface Node {
  id: ID
}

type Book implements Node {
  id: BookId # ERROR, because the Node interface wants the ID scalar
}

Another workaround would be to use RuntimeWiringConfigurer and override the default ID scalar and provide a custom serialize implementation that doesn't use toString(). But that's hard to use, especially if you want to dynamically provide new classes with custom serialization.

The code in graphql.execution.ExecutionStrategy does not seem to provide a way to customize the serialization. Or maybe I have missed something in the code or the docs.

Is there already a (easy) way to do this?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 13, 2022
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jun 17, 2022

@benneq, GraphQL Java has support for scalar types. Have you tried it? This is also mentioned in the Spring for GraphQL reference docs.

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Jun 17, 2022
@benneq
Copy link
Author

benneq commented Jun 17, 2022

@rstoyanchev Thanks for the response 😸 (Btw: Great docs and Spring YouTube Series 👍 )
Sure I'm using scalar type coercing. At the moment I use this code to (de)serialize the ID scalar to multiple custom Java Classes:

@QueryMapping
Mono<BookId> bookById(@Argument BookId id) {
  return Mono.just(id);
}

// this will convert the returned MyCustomId (from ID Scalar Corercing parseValue and parseLiteral) to BookId
// so it can be used as @Argument in @QueryMapping
@Bean
Converter<MyCustomId, BookId> myConverter() {
  return new Converter<MyCustomId, BookId>() {
    @Override
    public BookId convert(MyCustomId source) {
      return new BookId(/* convert from MyCustomId */);
    }
  };
}

// Scalar ID Coercing serialize method is used to convert BookId to String
@Bean
RuntimeWiringConfigurer configurer() {
  return builder -> builder.scalar(GraphQLScalarType.newScalar().name("ID").coercing(new Coercing<>() {
    @Override
    public String serialize(Object dataFetcherResult) {
      if (dataFetcherResult instanceof BookId bookId) {
        // custom BookId serialization code
        return "Book:" + bookId.value();
      }
      throw new CoercingSerializeException();
    }

    @Override
    public MyCustomId parseValue(Object input) {
      if (input instanceof String) {
        return new MyCustomId(/* parse input */);
      }
      throw new CoercingParseValueException();
    }

    @Override
    public MyCustomId parseLiteral(Object input) {
      if (input instanceof StringValue stringValue) {
        // parse input to intermediate object, because at this point
        // we have no information about the @Argument type
        return new MyCustomId(/* parse input */);
      }
      throw new CoercingParseLiteralException();
    }
  };
}

It's working fine. My only "issue" is that it's a lot of code and feels a bit strange to have the (de)serialization logic in two completely different places.

If this is the intended way to do this, you can close this issue.

@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 Jun 17, 2022
@rstoyanchev rstoyanchev self-assigned this Jun 23, 2022
@rstoyanchev
Copy link
Contributor

@benneq it's not very clear why you need to have both a Spring Converter in addition to the GraphQL scalar type coercion. Aside from that, the Spring code is quite minimal, and most of the rest is the actual logic to do the coercion. I'm not quite sure what you're suggesting that could change, and also whether that would be a change here or in GraphQL Java.

@rstoyanchev rstoyanchev added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jun 23, 2022
@rstoyanchev rstoyanchev added this to the Triage Queue milestone Jun 23, 2022
@benneq
Copy link
Author

benneq commented Jun 23, 2022

it's not very clear why you need to have both a Spring Converter in addition to the GraphQL scalar type coercion.

@rstoyanchev I'll try to explain it in more detail:

In my GraphQL schema I only have a single ID scalar, that is used for many different types:

interface Node {
  id: ID!
}

type Book implements Node {
  id: ID!
}

type Author implements Node {
  id: ID!
}

type Query {
  node(id: ID!): Node
  book(id: ID!): Book
  author(id: ID!): Author
}

Those ID scalars have different business meaning, depending on the type they are used in: One is a BookId and the other one is a AuthorId. Though I'd want to use seperate BookId and AuthorId value classes in my Java Code for the single ID scalar.

I have a @SchemaMapping with @Argument BookId like here:

@Controller
public class BookController {
    @QueryMapping
    Mono<Book> book(@Argument BookId id) {
        return Mono.just(new Book(...));
    }
}

The provided default ID scalar impl (GraphqlIDCoercing) will simply transform any given value into a String, but my @Argument expects a BookId. Though this will result in a Failed to convert value of type java.lang.String to required type BookId; no matching editors or conversion strategy found. This can be fixed by providing a Converter that can create BookId objects.

Within the scalar coercing code this can not be done, because there (in the parseLiteral and parseValue methods) I have no information about the target type of the @Argument . That's why I use a Converter, because the Converter runs after the coercing and knows the target type and can construct the matching object.

And the next issue is the ID scalar serialization:

@Controller
public class BookController {
    @QueryMapping
    Mono<BookId> someBookId() {
        return Mono.just(new BookId(...));
    }
}

The provided default ID scalar impl (GraphqlIDCoercing) will simply call toString on any object. Though I provide my own ID scalar serialize implementation to run custom serialization, which behaves differently for BookId and AuthorId and other classes.

This conversion can not be done using Spring Converter, because the scalar's coercing serialize method accepts an arbitrary Object and only after the execution of the method the resulting type is known. (I tried Converter<BookId, String> and Converter<BookId, graphql.language.StringValue>)

That's why I have used both, a custom scalar impl and a Converter. The Converter is used for custom @Argument types. And the custom scalar serialize method is needed, to be able to have custom serialization for different classes. Else it would just use toString on any object, which is not desired.

@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 Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

No branches or pull requests

3 participants