mapstruct / mapstruct-spring-extensions

Helpful additions to MapStruct when using the Spring Framework.
Apache License 2.0
140 stars 33 forks source link

Not auto register converters after manually create a ConversionService bean. #105

Closed jaggerwang closed 1 week ago

jaggerwang commented 4 months ago

My spring boot application is a none web application, and spring boot will not auto create the ConversionService bean (which class is WebConversionService), so I need to manually create one.


    @Bean
    public ConversionService conversionService() {
        var conversionService = new DefaultFormattingConversionService();
        return conversionService;
    }

Here is my converter and tests:

@Mapper(config = MapStructConfig.class)
public interface UserMapper extends Converter<UserBO, UserDTO> {

    UserDTO convert(UserBO userBO);

    @InheritInverseConfiguration
    @DelegatingConverter
    UserBO invertConvert(UserDTO userDTO);

}
@MapperConfig(componentModel = "spring")
@SpringMapperConfig(
        externalConversions = @ExternalConversion(sourceType = String.class, targetType = Locale.class)
)
public interface MapStructConfig {
}
@SpringBootTest(classes = Application.class)
public class UserMapperTest {

    @Autowired
    private ConversionService conversionService;

    @Test
    public void shouldMapBoToDto() {
        var userBo = UserBO.builder()
                .id(1L)
                .username("jaggerwang")
                .build();
        var userDto = conversionService.convert(userBo, UserDTO.class);
        assertThat(userDto).isNotNull();
        assertThat(userDto.getId()).isEqualTo(userBo.getId());
        assertThat(userDto.getUsername()).isEqualTo(userBo.getUsername());
    }

    @Test
    public void shouldMapDtoToBo() {
        var userDto = UserDTO.builder()
                .id(1L)
                .username("jaggerwang")
                .build();
        var userBo = conversionService.convert(userDto, UserBO.class);
        assertThat(userBo).isNotNull();
        assertThat(userBo.getId()).isEqualTo(userDto.getId());
        assertThat(userBo.getUsername()).isEqualTo(userDto.getUsername());
    }

}

The tests will pass when using the auto created ConversionService bean, but when run with the manually created one, it will fail with error "org.springframework.core.convert.ConverterNotFoundException: No converter found capable of converting from type [ai.basic.basicai.client.dto.user.UserDTO] to type [ai.basic.basicai.user.entity.UserBO]".

org.springframework.core.convert.ConverterNotFoundException: No converter found capable of converting from type [ai.basic.basicai.user.entity.UserBO] to type [ai.basic.basicai.client.dto.user.UserDTO]

    at org.springframework.core.convert.support.GenericConversionService.handleConverterNotFound(GenericConversionService.java:321)
    at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:195)
    at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:175)
    at ai.basic.basicai.user.adapter.converter.dto.UserMapperTest.shouldMapBoToDto(UserMapperTest.java:25)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

And there is another error when using the manually created ConversionService bean, the @Value annotaion cannot convert my custom enum type again.

public enum RunningMode {

    CLOUD_GLOBAL("cloud-global"),
    CLOUD_CHINA("cloud-china"),
    @Deprecated
    ONPREM("onprem");

    private final String name;

    RunningMode(String name) {
        this.name = name;
    }

    public String getName() {
        return name;
    }

    public static RunningMode convert(String value) {
        for (var mode : values()) {
            if (mode.name.equals(value)) {
                return mode;
            }
        }
        return null;
    }

}

    @Value("${runningMode}")
    protected RunningMode runningMode;
runningMode: cloud-global
Caused by: java.lang.IllegalArgumentException: No enum constant ai.basic.basicai.client.enums.RunningMode.cloud-global
    at java.base/java.lang.Enum.valueOf(Enum.java:273) ~[na:na]
    at org.springframework.core.convert.support.StringToEnumConverterFactory$StringToEnum.convert(StringToEnumConverterFactory.java:54) ~[spring-core-6.0.14.jar:6.0.14]
    at org.springframework.core.convert.support.StringToEnumConverterFactory$StringToEnum.convert(StringToEnumConverterFactory.java:39) ~[spring-core-6.0.14.jar:6.0.14]
    at org.springframework.core.convert.support.GenericConversionService$ConverterFactoryAdapter.convert(GenericConversionService.java:436) ~[spring-core-6.0.14.jar:6.0.14]
    at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:41) ~[spring-core-6.0.14.jar:6.0.14]
    ... 173 common frames omitted

If I change cloud-global to CLOUD_GLOBAL in property file, it became ok again.

jaggerwang commented 4 months ago

If I change the class of manually created ConversionService bean from DefaultFormattingConversionService to ApplicationConversionService, the enum type convert error dissapeared, but the custom mappers still not be auto detected.

Chessray commented 4 months ago

Spring's ConversionService initialization doesn't follow standard procedures. You can try the generated ConverterScan annotations. This feature wasn't designed for your case, but may cover it nonetheless.

jaggerwang commented 4 months ago

@Chessray Thanks for your suggestion! It seems ConverterScan is just for test only, I tried the following way but the problem is still there.


    @Bean
    public ConversionService conversionService() {
        var conversionService = ApplicationConversionService.getSharedInstance();
        return conversionService;
    }
@MapperConfig(componentModel = "spring")
@SpringMapperConfig(conversionServiceBeanName = "conversionService", generateConverterScan = true)
public interface MapStructConfig {
}

The custom converter UserMapper still cannot be found in the converters field of the conversionService bean.

jaggerwang commented 4 months ago

Temporary resolve this problem by manually add converters to ApplicationConversionService, but it's very inconvenient.


    @Bean
    public ConversionService conversionService() {
        var conversionService = new ApplicationConversionService();
        conversionService.addConverter(new UserDtoMapperImpl());
        conversionService.addConverter(new UserModelMapperImpl());
        return conversionService;
    }
jaggerwang commented 4 months ago

Does it because the ConversionService bean created after custom converter beans, so the converters cannot be auto registered, as there is no ConversionService bean. But how to make the generated mapper implementation to depend on the ConversionService bean?

Chessray commented 4 months ago

The generated ConverterScan (as opposed to the one provided in test-extensions) is intended for production use. Is there any way for you to share your project so I can help a bit more? In the meantime, you might wish to compare it to the example project.

Can you confirm whether your generated code includes the @ConverterScan annotation?

jaggerwang commented 4 months ago

Here is the testing project.

ai.basic.basicai.user.adapter.config.CommonConfig

@Configuration(proxyBeanMethods = false)
public class CommonConfig {

    @Bean
    public ConversionService conversionService() {
        var conversionService = new ApplicationConversionService();
        // If comment this line, the application will start failed with error "Caused by: org.springframework.core.convert.ConverterNotFoundException: No converter found capable of converting from type [ai.basic.basicai.user.entity.UserBO] to type [ai.basic.basicai.user.adapter.dto.UserDTO]"
        conversionService.addConverter(new UserDtoMapperImpl());
        return conversionService;
    }

}
filiphr commented 4 months ago

@jaggerwang why don't you do something like:

@Configuration(proxyBeanMethods = false)
public class CommonConfig {

    @Bean
    public ConversionService conversionService(ObjectProvider<Converter<?, ?>> converters) {
        var conversionService = new ApplicationConversionService();
        converters.orderedStream().forEach(converter -> conversionService.addConverter(converter));
        return conversionService;
    }

}

Something like this is going to register all the beans that are implementing Converter in the ApplicationConversionService.

jaggerwang commented 4 months ago

@filiphr Thanks for your suggestion, it really works! But I still think it's the responsibility of MapStruct Spring Extension to add all converters generated by MapStruct to the global ConversionService, even it was created by myself.

filiphr commented 4 months ago

But I still think it's the responsibility of MapStruct Spring Extension to add all converters generated by MapStruct to the global ConversionService, even it was created by myself.

I disagree with this @jaggerwang. When you are using Spring Boot the WebMvcAutoConfiguration with its WebMvcAutoConfigurationAdapter#addFormatter is responsible for adding the required converters for the Spring MVC FormatterRegistry.

From what I've learned reading from the comments from @Chessray and his work on this is that the ConversionService is a special type of bean in the Spring application context. Therefore, I think it is the responsibility of the user to handle this appropriately.

I'll leave the decision whether something should be done here to @Chessray.

Chessray commented 3 months ago

Your test project works if you add the generated ConverterScan:

Add this dependency:

 <dependency>
     <groupId>javax.annotation</groupId>
     <artifactId>jsr250-api</artifactId>
     <version>1.0</version>
</dependency>

Add this line to MapStructConfig:

@SpringMapperConfig(generateConverterScan = true, conversionServiceBeanName = "conversionService")

There's no need for any manual adds after doing this.

The one thing this highlighted though is that we should probably update the generated @PostConstruct import so that it uses the newer package jakarta.annotation instead of javax.annotation.

jaggerwang commented 3 months ago

👍So is there any plan to fix this problem, and is there any posibility to not require user to include dependency jakarta.annotation-api

Got compile error "package javax.annotation not exist" right now.

image
Chessray commented 3 months ago

You need to include the older dependency jsr250 as outlined above. jakarta-annotation comes with Spring Boot 3 anyway. I'll have to look into how we can add this in a nice fashion. Also, remember this is an open source project. You're welcome to submit a Fix yourself. 🙂

jaggerwang commented 3 months ago

OK,I got it!I'll try if I can:)

stickfigure commented 1 month ago

PR here: https://github.com/mapstruct/mapstruct-spring-extensions/pull/110

stickfigure commented 1 month ago

If this issue is going to track compatibility with Spring Boot 3, maybe retitle it?

stickfigure commented 1 month ago

Also worth mentioning - it's not harmless to add javax.annotation-api to a Spring Boot 3 project. All it takes is one misclick to import the wrong annotation and you have a potentially serious bug that will only be caught by integration tests (if you have them).

Chessray commented 1 week ago

Closed by #110.