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

bug: Registering customer mapper bean leads to exception #1775

Closed
frct1 opened this issue Jan 13, 2024 · 7 comments
Closed

bug: Registering customer mapper bean leads to exception #1775

frct1 opened this issue Jan 13, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@frct1
Copy link

frct1 commented Jan 13, 2024

Please read our contributor guide before
creating an issue.

Expected behavior

Mapper that used to convert camelCase field to snake_case:

Actual behavior

Invalid query - Cannot construct instance of `com.netflix.graphql.dgs.mvc.DgsRestController$Companion$InputQuery` (no Creators, like default constructor, exist): cannot deserialize from Object value (no delegate- or property-based Creator)
 at [Source: (org.apache.catalina.connector.CoyoteInputStream); line: 1, column: 2
].

Steps to reproduce

Latest DGS framework with Spring (3.2.1) and bean with mapper

  @Bean
  @Qualifier("dgsObjectMapper")
  public ObjectMapper dgsObjectMapper() {
    ObjectMapper customMapper = new ObjectMapper();
    customMapper.registerModule(new JavaTimeModule());
    return customMapper;
  }

DTO have properties in camelCase, for example

ZonedDateTime createdAt;

that needs to be format as created_at, that's why custom mapper required, but ut leads to graphql misconfiguration

Note: A test case would be highly appreciated, but we understand that's not always possible

@frct1 frct1 added the bug Something isn't working label Jan 13, 2024
@tinnou
Copy link
Collaborator

tinnou commented Jan 16, 2024

Thanks for the report. Would you be able to setup a minimal reproducible example to showcase your issue? Specifically I'm having trouble understanding if the issue is happening when you execute a GraphQL query or at startup.

@frct1
Copy link
Author

frct1 commented Jan 17, 2024

Thanks for the report. Would you be able to setup a minimal reproducible example to showcase your issue? Specifically I'm having trouble understanding if the issue is happening when you execute a GraphQL query or at startup.

Hi, here it is https://github.com/baikov-ilia/dgs-custom-mapper-issue
Query is:

query mapper{
    mapper {
        snake_case_property
    }
}

Exception is

Invalid query - Cannot construct instance of `com.netflix.graphql.dgs.mvc.DgsRestController$Companion$InputQuery` (no Creators, like default constructor, exist): cannot deserialize from Object value (no delegate- or property-based Creator)
 at [Source: (org.apache.catalina.connector.CoyoteInputStream); line: 1, column: 2
].

@tinnou
Copy link
Collaborator

tinnou commented Jan 17, 2024

Thank you for sending. I was able to get the error to go away but indeed there is a regression, the dgsObjectMapper no longer allows to customize the serialization of the response as mentioned in the documentation, it only allows to control the deserialization of the GraphQL request.

The dgsObjectMapper expects to have the KotlinModule installed because it deserializes the GraphQL request (InputQuery) represented by a Kotlin class:

  @Bean
  @Qualifier("dgsObjectMapper")
  public ObjectMapper dgsObjectMapper() {
    ObjectMapper customMapper = jacksonObjectMapper(); // installs KotlinModule
//    ObjectMapper customMapper = new ObjectMapper();
    customMapper.registerModule(new JavaTimeModule()); // copy pasted
    return customMapper;
  }

We are going to file a ticket for this.

A few other things to note for what you are trying to do:

  • the data fetcher must be placed in a class annotated with @DgsComponent for it to be picked up
  • note that customizing object mappers only affect serialization/deserialization, in the example you linked, the GraphQL field snake_case_property must match the property returned by your data fetcher. This is the GraphQL data fetching execution and can not be customized with object mappers. So if your GraphQL schema has a field named snake_case_property the returned map from your data fetcher must contain a property of the same name.
HashMap<String, String> data = new HashMap<>(){{
  put("snake_case_property", "hello world");
}};

If you want to control the serialization of the response, you can override the Spring default object mapper with your own:

  @Bean
  @Primary
  public ObjectMapper objectMapper() {
    JavaTimeModule module = new JavaTimeModule();
    return new ObjectMapper()
            .setSerializationInclusion(JsonInclude.Include.NON_NULL)
            .setPropertyNamingStrategy(SnakeCaseStrategy.INSTANCE)
            .registerModule(module);
  }

Note that .setPropertyNamingStrategy(SnakeCaseStrategy.INSTANCE) only works for POJOs and the DGS framework will return the GraphQL response as a map of maps. You can check jackson docs on how to write a custom serializer module that will serialize map keys the way you want.

@frct1
Copy link
Author

frct1 commented Jan 17, 2024

I glad that it has been confirmed. So im not sure how to properly return response within snake case like in raw spring boot. Map has been used to wrap up a quick sample for issue repo.

For rest api i'm using the next way of serializing from camelCase to snake_case:

@Value
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
public class RegionDto implements Serializable {

  Long id;
  String name;
  String i18nName;
  String datacenterI18nName;
  String countryCode;
  String slug;
  String continent;
  String description;
  String[] tags;
  String disabledMessage;
  Boolean isActive;
  OffsetDateTime createdAt;
  OffsetDateTime updatedAt;
}
...
List<RegionDto> regionsDto = regionMapper.toDto(regions.get()); // mapstruct from Hibernate entity to POJO
return regionsDto;

GraphQL schema which i currently use for playground

type InstanceCreateOptionsRegion {
    id: Int
    name: String
    created_at: DateTime
    updated_at: DateTime
}

but seems like it's not working with DGS this way.

@kilink
Copy link
Member

kilink commented Jan 17, 2024

I don't think this is a Jackson issue at all, if I'm understanding correctly based on your example repo; you have a DgsQuery-annotated method that returns an object with camel case fields, but your GraphQL schema is all snake case. The issue here is in the graphql-java layer, by default it's going to look for getters / fields that exactly match the field name. One solution is to install data fetchers that do what you want:

@DgsComponent
public class MapperFetcher {
    @DgsQuery(field = "mapper")
    public Dto index(){
        return new Dto();
    }

    public static class Dto {

        public String getSnakeCaseProperty() {
            return "hello world";
        }
    }

    @DgsRuntimeWiring
    public RuntimeWiring.Builder runtimeWiringCustomizer(RuntimeWiring.Builder runtimeWiring) {
        return runtimeWiring.type("Mapper", builder -> builder.dataFetcher("snake_case_property", PropertyDataFetcher.fetching("snakeCaseProperty")));
    }
}

@kilink
Copy link
Member

kilink commented Jan 17, 2024

Also, the above is just an example solution, I realize it may be tedious to register it for each field / getter, but that can easily be handled with reflection:

    @DgsRuntimeWiring
    public RuntimeWiring.Builder runtimeWiringCustomizer(RuntimeWiring.Builder runtimeWiring) {
        return runtimeWiring.type("InstanceCreateOptionsRegion", builder -> {
            for (Field field : RegionDto.class.getDeclaredFields()) {
                String snakeCaseName = PropertyNamingStrategies.SnakeCaseStrategy.INSTANCE.translate(field.getName());
                if (!snakeCaseName.equals(field.getName())) {
                    builder.dataFetcher(snakeCaseName, PropertyDataFetcher.fetching(field.getName()));
                }
            }
            return builder;
        });
    }

@kilink
Copy link
Member

kilink commented Jan 23, 2024

I'm going to resolve this, please open a new issue if you encounter a problem, but based on the details in this thread, the issue was with the default DataFetchers, not with JSON mapping. The above workaround was provided as well which should fix the problem in this specific case.

@kilink kilink closed this as completed Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants