spring-cloud / spring-cloud-function

Apache License 2.0
1.03k stars 613 forks source link

Custom ObjectMapper is not used #1159

Open sonallux opened 1 month ago

sonallux commented 1 month ago

Describe the bug With the latest spring-cloud-function 4.1.3 release a custom ObjectMapper bean is no longer respected when creating the FunctionCatalog in the ContextFunctionCatalogAutoConfiguration.java.

We need this behaviour, because must configure the ObjectMapper to use the snake-case naming strategy.

This regression is introduced by commit 8b66fd296e72e8c0f1ed5df0711da5ea75527cb8 which fixed issue #1148.

Sample If you really want an example I can provide some.

Possible Solution I would like to propose the following solution to fix this issue. Adding a @ConditionalOnMissingBean on the bean definition of the JsonMapper here. With this change one can easily provide a custom JsonMapper Bean wrapping a custom ObjectMapper. This could also fix issue #1059.

mmaeller commented 1 month ago

We also stumbled over that issue. 😞 You can mitigate it annotating a custom provided bean with @Primary.

  @Bean
  @Primary
  public JacksonMapper jacksonMapper(final ObjectMapper objectMapper) {
    return new JacksonMapper(objectMapper);
  }
lbilger commented 1 month ago

If I understood the issue correctly, the reason for 8b66fd2 was that you need to configure some things on the ObjectMapper and don't want this to affect the original one from the context. Wouldn't it make sense then to just copy() the context ObjectMapper before modifying it? This way, you would get all the configuration from the context ObjectMapper like before, but would not change the original configuration.

I will submit a PR. This should restore the behavior from before the GH-1148 change without any additional configuration required. I still think #1160 is a valuable change as it would allow you to further configure the ObjectMapper to be used.

pgehl commented 1 month ago

Same here. We declare several Jackson Modules to add support for some classes serialization and deserialization. Since upgrading to version 4.13 out tests fail. I was able to trace the changes between 4.12 and 4.13 in ContextFunctionCatalogAutoConfiguration

4.12: our modules and jackson settings are used

private JsonMapper jackson(ApplicationContext context) {
    ObjectMapper mapper;
    try {
        mapper = context.getBean(ObjectMapper.class);
    }
    catch (Exception e) {
        mapper = new ObjectMapper();
    }
    mapper.configure(SerializationFeature.FAIL_ON_EMPTY_BEANS, false);
    mapper.configure(DeserializationFeature.FAIL_ON_TRAILING_TOKENS, true);
    return new JacksonMapper(mapper);
}

4.13: our modules and jackson settings are no long used

private JsonMapper jackson(ApplicationContext context) {
    ObjectMapper mapper = new ObjectMapper();
    mapper.registerModule(new JavaTimeModule());
    mapper.configure(SerializationFeature.FAIL_ON_EMPTY_BEANS, false);
    mapper.configure(DeserializationFeature.FAIL_ON_TRAILING_TOKENS, true);
    return new JacksonMapper(mapper);
}
ilkengin commented 1 month ago

We suffer from the same issue as well. As a workaround, we are overriding the behavior of the ObjectMapper used inside the JacksonMapper by calling the jacksonMapper.configureObjectMapper method. Hope this solution helps someone out there.

@Configuration
public class CustomJacksonMapperConfig {

    @Autowired
    public void customJacksonMapperConfig(JacksonMapper jacksonMapper) {
        jacksonMapper.configureObjectMapper(objectMapper -> {
            objectMapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
        });
    }
}
ivan-sirosh commented 2 weeks ago

We also stumbled over that issue. 😞 You can mitigate it annotating a custom provided bean with @Primary.

  @Bean
  @Primary
  public JacksonMapper jacksonMapper(final ObjectMapper objectMapper) {
    return new JacksonMapper(objectMapper);
  }

it does not work, fails to create a bean.

The bean 'jsonMapper', defined in class path resource [org/springframework/cloud/function/context/config/ContextFunctionCatalogAutoConfiguration$JsonMapperConfiguration.class], could not be registered. A bean with that name has already been defined in com.foo.bar.config.ObjectMapperConfiguration
anand188 commented 1 week ago

any updates on this issue?

marcos-fernandez commented 1 week ago

I've been also hit by this issue today 😅 . I think could be a nice fix the solution given by @sonallux

Thanks