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

Improvement: Customize class name and bean name for ConverterRegistrationConfiguration #119

Closed
simonovdenis opened this issue Jan 21, 2025 · 7 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@simonovdenis
Copy link
Contributor

simonovdenis commented Jan 21, 2025

As of now, Mapstruct spring extension(more specifically ConverterRegistrationConfigurationGenerator) generates configuration class with name ConverterRegistrationConfiguration, name of this bean/component is not set, ie defaults to converterRegistrationConfiguration:

@Generated(
    value = "org.mapstruct.extensions.spring.converter.ConverterRegistrationConfigurationGenerator",
    date = "2025-01-21T08:53:44.800409700Z"
)
@Configuration
class ConverterRegistrationConfiguration {
  private final ConfigurableConversionService conversionService;

  private final List<Converter<?, ?>> converters;

  ConverterRegistrationConfiguration(
      @Qualifier("conversionService") final ConfigurableConversionService conversionService,
      final List<Converter<?, ?>> converters) {
    this.conversionService = conversionService;
    this.converters = converters;
  }

  @PostConstruct
  void registerConverters() {
    converters.forEach(conversionService::addConverter);
  }
}

in my project Mapstruct is used in multiple places: a different set of mappers is implemented for each of them. So 2 different mapstruct related spring configs are defined with pretty much identical content:

package some.package.name1;
...
@Configuration
@ComponentScan("some.package.name1")
@MapperConfig(componentModel = "spring", uses = ConversionServiceAdapter.class)
@SpringMapperConfig(conversionServiceAdapterPackage ="some.package.name1.adapter",
        conversionServiceAdapterClassName ="ConversionServiceAdapter",
        conversionServiceBeanName = "conversionService",
        generateConverterScan = true)
public class MapperSpringConfig1 {
}

and

package some.package.name2;
...
@Configuration
@ComponentScan("some.package.name2")
@MapperConfig(componentModel = "spring", uses = ConversionServiceAdapter.class)
@SpringMapperConfig(conversionServiceAdapterPackage ="some.package.name2.adapter",
        conversionServiceAdapterClassName ="ConversionServiceAdapter",
        conversionServiceBeanName = "conversionService",
        generateConverterScan = true)
public class MapperSpringConfig2 {
}

when application is started I see a conflict between them due to same bean name used in both cases for ConverterRegistrationConfiguration instance:

ERROR o.s.boot.SpringApplication - Application run failed
org.springframework.context.annotation.ConflictingBeanDefinitionException: Annotation-specified bean name 'converterRegistrationConfiguration' for bean class [some.package.name1.adapter.ConverterRegistrationConfiguration] conflicts with existing, non-compatible bean definition of same name and class [some.package.name2.adapter.ConverterRegistrationConfiguration]

So I want to propose to provide ability to configure it's class name and bean name(just like it is done for adapter class - conversionServiceAdapterClassName in org.mapstruct.extensions.spring.SpringMapperConfig with default value "ConversionServiceAdapter"), so smth like:
converterRegistrationConfigurationClassName in org.mapstruct.extensions.spring.SpringMapperConfig with default value "ConverterRegistrationConfiguration"

@Chessray Chessray added enhancement New feature or request good first issue Good for newcomers labels Jan 22, 2025
@simonovdenis
Copy link
Contributor Author

Hello,
I wanted to propose PR with needed changes, but seems I do not have write access to do it. Can you grant me rights to present my changes?

@Chessray
Copy link
Collaborator

That's quite odd. We've definitely received PRs from others without having to add them to the organisation or repo beforehand. @filiphr Have you got an idea what could be wrong here?

@simonovdenis
Copy link
Contributor Author

On my side I'm receiving error 403, when trying to push my feature branch to remote repository.
As well I don't see in Github web UI green button to create new branch, like on other projects, where I do have access.
mapstruct:
Image
Other project:
Image
Maybe I'm doing something wrong and you have some proper way to create/push branches, create PRs. If so - please share such info with me, please.

@Chessray
Copy link
Collaborator

Non-members can follow the basic GitHub flow:

  1. Fork the project into your own space.
  2. Work on the main branch in your own space.
  3. Open the PR back to the organisation.

@simonovdenis
Copy link
Contributor Author

@Chessray Thanks for providing an instruction. Opened PR: #121. Please review.

@simonovdenis
Copy link
Contributor Author

@Chessray Do I have to add something else to PR for its successful approval/merge?

@simonovdenis
Copy link
Contributor Author

PR: #121 merged to main. So feature will be available in next release.
BTW is there some info on release schedule?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants