8000 add ignored annotation issues #1958 by xumk · Pull Request #3788 · mapstruct/mapstruct · GitHub
[go: up one dir, main page]
Skip to content

add ignored annotation issues #1958#3788

Merged
filiphr merged 2 commits intomapstruct:mainfrom
xumk:issues/1958
May 25, 2025
Merged

add ignored annotation issues #1958#3788
filiphr merged 2 commits intomapstruct:mainfrom
xumk:issues/1958

Conversation

@xumk
Copy link
Contributor
@xumk xumk commented Dec 8, 2024 •
edited by filiphr
Loading

add new annotation for ignored more than one property

Fixes #1958

@xumk xumk changed the title add ignored annotation issues 1958 add ignored annotation issues #1958 Dec 9, 2024
@diversit
8000 Copy link
diversit commented Dec 17, 2024

Would be nice if this would also support ignoring properties based on a prefix, like mentioned in this comment related to generated 'withXX' methods.

@xumk
Copy link
Contributor Author
xumk commented Dec 18, 2024

I'll try to see it on the weekend.

@xumk
Copy link
Contributor Author
xumk commented Dec 18, 2024

@diversit i'm add prefix for Ignored annotation for this case

diversit
diversit reviewed Dec 18, 2024
View reviewed changes
@diversit
Copy link
diversit commented Dec 18, 2024

Awesome!

@filiphr
Copy link
Member
filiphr commented Dec 19, 2024

Would be nice if this would also support ignoring properties based on a prefix, like mentioned in #1958 (comment) related to generated 'withXX' methods.

@diversit we do not want to support pattern ignores. We've said this few times already.

@xumk your idea with the prefix is OK I think, it basically transforms customer.firstName, customer.lastName to something more simpler. Unless, I am not seeing it properly

@xumk
Copy link
Contributor Author
xumk commented Dec 20, 2024 •
edited
Loading

@filiphr
I assumed exactly this use
@Ignored( prefix = "animal", targets = { "publicAge", "size", "publicColor", "age", "color" } )
just added a prefix with dot to the properties. It is more convenient than writing targets = { "animal.publicAge", "animal.size", "animal.publicColor"...etc
but nothing prevents you from writing it down according to the standard

filiphr
filiphr requested changes May 10, 2025
View reviewed changes
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.

Thanks @xumk, sorry that it take some time, but I've been a bit busy.

I've left some comments.

Something additional. I'd like to have some tests for the following options:

@Mapping(target = "name", ignore = true)
@Ignored(targets = "name")

and

@Mapping(target = "name", source = "firstName")
@Ignored(targets = "name")

What is happening in those scenarios

@xumk xumk force-pushed the issues/1958 branch 2 times, most recently from 6d80e09 to 0f8780e Compare May 11, 2025 15:54
@xumk
Copy link
Contributor Author
xumk commented May 11, 2025 •
edited
Loading

@filiphr thanks. i fixed remarks and checked you case

@Mapping(target = "name", ignore = true)
@Ignored(targets = "name")

it test ok. property is ignored.

@Mapping(target = "name", source = "firstName")
@Ignored(targets =  "name")

it not ignored property. name have value firstName. Is it good?

i added two test

@filiphr
Copy link
Member
filiphr commented May 11, 2025

Thanks for checking that @xumk. My main point was the fact that a combination of @Mapping and @Ignored for the same property should not be allowed, we should have an error if such a thing happens. The same as if we are using the same target in @Mapping

@xumk
Copy link
Contributor Author
xumk commented May 11, 2025

@filiphr ok, I will try to do it soon.

@xumk
Copy link
Contributor Author
xumk commented May 12, 2025

@filiphr I added a check

filiphr
filiphr approved these changes May 25, 2025
View reviewed changes
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.

After some polishing this LGTM. Thanks @xumk

@filiphr filiphr merged commit 6b6600c into mapstruct:main May 25, 2025
6 checks passed
@thunderhook thunderhook mentioned this pull request Jul 20, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@filiphr filiphr filiphr approved these changes

+1 more reviewer

@diversit diversit diversit left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow ignoring multiple target properties

3 participants

@xumk @diversit @filiphr
0