Skip to content

Conversation

@jccampanero
Copy link
Contributor

@jccampanero jccampanero commented Sep 11, 2022

Adds support for defining a custom name for the Spring stereotype annotations (@Component, @Service) in the generated Mapper when using the Spring component model.

Fixes #1427

@Zegveld
Copy link
Contributor

Zegveld commented Sep 19, 2022

This solution only covers one out of many options for the @Component annotation replacements.
you have @Service covered, but not for example @Controller, or user defined meta annotations that have @Component included.

Try creating a check that walks the meta-annotation tree instead of something that only checks the top level annotation.
Take a look at RepeatableAnnotations for how this is possible, that class handles meta-annotations for the @Mapping and @SubclassMapping annotations.

@jccampanero
Copy link
Contributor Author

Thank you very much for the feedback @Zegveld.

I absolutely agree with you: I initially thought of including checks for @Repository and @Controller, but they seem like a less obvious use case to me. But you are completely right, the way to go should be traversing the different annotation tree looking for the occurrence of @Component within it, directly or through the annotation hierarchy. I will review RepeatableAnnotations as you suggested looking for reference.

Again, thank you very much for the feedback, and sorry for this first baby step.

@jccampanero
Copy link
Contributor Author

jccampanero commented Sep 26, 2022

@Zegveld Sorry for the late reply.

I have pushed some new changes.

Per your suggestion, I tried to process the Mapper instance by checking for the existence of the @Component annotation through the entire annotation hierarchy.

Please let me know what additional work needs to be done.

Thank you very much.

Copy link
Member

@filiphr filiphr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation looks good @jccampanero. I only have some points regarding the tests.

I do not think that we should complicate them that much. Do we really need to test this entirely using this in the Spring context. I think that it is enough to what we are doing in SpringAnnotateWithMapperTest without the spring context testing. We could also simplify the mappers by not doing Mapper#uses and without extending from other mappers. We can have empty mappers for the tests.

@jccampanero
Copy link
Contributor Author

Thank you very much for the code review @filiphr, I appreciate it a lot.

I updated the code with the changes you requested.

Please let me know if any additional changes need to be made.

@filiphr filiphr merged commit 90a487a into mapstruct:main Oct 1, 2022
@filiphr
Copy link
Member

filiphr commented Oct 1, 2022

Thanks a lot for your patience @jccampanero. We've integrated this into main.

@jccampanero
Copy link
Contributor Author

Thank you very much @filiphr. Please, on the contrary, thank you very much for your time and for allowing me to contribute to this wonderful library.

@jccampanero jccampanero deleted the 1427_custom_name_spring_annotations branch October 1, 2022 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@Mapper(componentModel = "spring") doesn't support custom name for Spring Service Annotation

3 participants