GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add support for custom name in Spring stereotype annotations by jccampanero · Pull Request #3019 · mapstruct/mapstruct

Conversation

@jccampanero

@jccampanero jccampanero commented Sep 11, 2022 •
edited by filiphr
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
jccampanero added 2 commits September 11, 2022 00:25
@jccampanero
mapstruct#1427 Prevent the inclusion of @component if Mapper is alrea…
fd7edb7
…dy annotated as Spring stereotype
@jccampanero
Initial testing of decorators when using AnnotateWith and Spring
c7056ab
@Zegveld

Zegveld commented Sep 19, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jccampanero

jccampanero commented Sep 19, 2022

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jccampanero added 2 commits September 27, 2022 00:15
@jccampanero
mapstruct#1427 Improve @component annotation processing.
0994e91
Check the existence of the @component annotation across the full annotation hierarchy.
@jccampanero
Merge branch 'main' into 1427_custom_name_spring_annotations
d329755
@jccampanero

jccampanero commented Sep 26, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

filiphr
filiphr requested changes Sep 28, 2022
View reviewed changes

@filiphr filiphr left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread ...t/injectionstrategy/spring/annotateWith/decorator/SpringAnnotateWithDecoratorMapperTest.java Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread ...ct/ap/test/injectionstrategy/spring/annotateWith/qualified/SpringAnnotateWithMapperTest.java Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread ...ct/ap/test/injectionstrategy/spring/annotateWith/qualified/SpringAnnotateWithMapperTest.java Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread ...ct/ap/test/injectionstrategy/spring/annotateWith/qualified/SpringAnnotateWithMapperTest.java Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread ...ct/ap/test/injectionstrategy/spring/annotateWith/qualified/SpringAnnotateWithMapperTest.java Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread ...ct/ap/test/injectionstrategy/spring/annotateWith/qualified/SpringAnnotateWithMapperTest.java Resolved

Uh oh!

There was an error while loading. Please reload this page.

jccampanero added 2 commits September 30, 2022 00:31
@jccampanero
mapstruct#1427 Test only the generated source code, without Spring co…
db24200
…ntext.

Remove unnecessary line endings.
@jccampanero
mapstruct#1427 Test related Mappers code simplification.
658aa6d
@jccampanero

jccampanero commented Sep 29, 2022

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jccampanero added 2 commits September 30, 2022 20:54
@jccampanero
mapstruct#1427 Remove Decorator test related stuff. Restore original …
cd2923d
…test package.
@jccampanero
Merge branch 'main' into 1427_custom_name_spring_annotations
8a45bbb
@filiphr
filiphr merged commit 90a487a into mapstruct:main Oct 1, 2022
@filiphr

filiphr commented Oct 1, 2022

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jccampanero

jccampanero commented Oct 1, 2022

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@filiphr filiphr mentioned this pull request Oct 1, 2022
#1427 Add additional configuration for Jsr330 and Spring component models #1437
Closed
@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

Reviewers

@filiphr filiphr filiphr requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@jccampanero @Zegveld @filiphr
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.