Skip to content

add ignored annotation issues #1958#3788

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

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

Conversation

@xumk

@xumk xumk commented Dec 8, 2024

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown

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

xumk commented Dec 18, 2024

Copy link
Copy Markdown
Contributor Author

I'll try to see it on the weekend.

@xumk

xumk commented Dec 18, 2024

Copy link
Copy Markdown
Contributor Author

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

Comment thread core/src/main/java/org/mapstruct/Ignoreds.java Outdated
@diversit

Copy link
Copy Markdown

Awesome!

@filiphr

filiphr commented Dec 19, 2024

Copy link
Copy Markdown
Member

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

xumk commented Dec 20, 2024

Copy link
Copy Markdown
Contributor Author

@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 left a comment

Copy link
Copy Markdown
Member

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

Comment thread core/src/main/java/org/mapstruct/Ignored.java Outdated
Comment thread core/src/main/java/org/mapstruct/Ignored.java Outdated
@xumk xumk force-pushed the issues/1958 branch 2 times, most recently from 6d80e09 to 0f8780e Compare May 11, 2025 15:54
@xumk

xumk commented May 11, 2025

Copy link
Copy Markdown
Contributor Author

@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

filiphr commented May 11, 2025

Copy link
Copy Markdown
Member

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

xumk commented May 11, 2025

Copy link
Copy Markdown
Contributor Author

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

@xumk

xumk commented May 12, 2025

Copy link
Copy Markdown
Contributor Author

@filiphr I added a check

@filiphr filiphr left a comment

Copy link
Copy Markdown
Member

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

Allow ignoring multiple target properties

3 participants