Netflix / dgs-framework

GraphQL for Java with Spring Boot made easy.
https://netflix.github.io/dgs
Apache License 2.0
3.03k stars 286 forks source link

bug: Registering customer mapper bean leads to exception #1775

Closed frct1 closed 5 months ago

frct1 commented 5 months ago

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

tinnou commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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:

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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.