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

Support field access if visibility rules permit #557

Closed
gunnarmorling opened this Issue May 22, 2015 · 14 comments

Comments

Projects
None yet
6 participants
@gunnarmorling
Member

gunnarmorling commented May 22, 2015

If e.g. the target object is a plain "struct" with public fields, it should supported to write to these using direct field access. This should only be supported if the visibility rules permit that access, i.e. no reflection-based work-around otherwise.

@GilraenBurland

This comment has been minimized.

Show comment
Hide comment
@GilraenBurland

GilraenBurland commented Jun 2, 2015

+1 for this.

@cardosso

This comment has been minimized.

Show comment
Hide comment
@cardosso

cardosso Nov 16, 2015

Is this being considered?

For me this is the current deal breaker with MapStruct and the only reason I'm still using Orika. I don't want to fill my Hibernate domain objects with unnecessary getters and setters just to be able to use a mapping framework. It's a pitty this feature is missing because otherwise this is a great library.

This should only be done on public accessible fields and could even be made configurable (ie disabled by default).

cardosso commented Nov 16, 2015

Is this being considered?

For me this is the current deal breaker with MapStruct and the only reason I'm still using Orika. I don't want to fill my Hibernate domain objects with unnecessary getters and setters just to be able to use a mapping framework. It's a pitty this feature is missing because otherwise this is a great library.

This should only be done on public accessible fields and could even be made configurable (ie disabled by default).

@gunnarmorling

This comment has been minimized.

Show comment
Hide comment
@gunnarmorling

gunnarmorling Nov 16, 2015

Member

Hey @cardosso, yes, it is considered for sure. We just did not yet find the time to implement it. If you are interested in helping out, let me know and I can help you to get started with hacking on MapStruct.

I will schedule this for the 1.1 release.

Member

gunnarmorling commented Nov 16, 2015

Hey @cardosso, yes, it is considered for sure. We just did not yet find the time to implement it. If you are interested in helping out, let me know and I can help you to get started with hacking on MapStruct.

I will schedule this for the 1.1 release.

@gunnarmorling gunnarmorling added this to the 1.1-next milestone Nov 16, 2015

@cardosso

This comment has been minimized.

Show comment
Hide comment
@cardosso

cardosso Nov 17, 2015

That's great news thanks.
I never looked into the MapStruct source code, I guess I could check it out and see if I could be of some help implementing it if you could give me some quick guidelines. Not sure if I will have much free time during the next days though.

cardosso commented Nov 17, 2015

That's great news thanks.
I never looked into the MapStruct source code, I guess I could check it out and see if I could be of some help implementing it if you could give me some quick guidelines. Not sure if I will have much free time during the next days though.

@gunnarmorling

This comment has been minimized.

Show comment
Hide comment
@gunnarmorling

gunnarmorling Nov 21, 2015

Member

Hi, any help would be more than welcome. If you are interested, you may check out the resources given here for getting started. The change would have to include adaptions the logic for identifying mappable properties as well as the templates for generating code working with properties.

Member

gunnarmorling commented Nov 21, 2015

Hi, any help would be more than welcome. If you are interested, you may check out the resources given here for getting started. The change would have to include adaptions the logic for identifying mappable properties as well as the templates for generating code working with properties.

@xiemeilong

This comment has been minimized.

Show comment
Hide comment
@xiemeilong

xiemeilong commented Jul 15, 2016

+1

@filiphr

This comment has been minimized.

Show comment
Hide comment
@filiphr

filiphr Oct 8, 2016

Member

@gunnarmorling I had a look at this issue. And I think I can start working on it. I had a deeper look at the code and the inner workings. I think that in order to do this, we would need to add our own Accessor class which would wrap the ExecutableElemement(s) and the VariableElements. In our own Accessor we would add the methods which we need and get them from the underlying Element.
The Type methods getPropertyPresenceCheckers, getPropertyReadAccessors and getPropertyWriteAccessors will need to be adjusted to return our wrapper.

This is only the first step, I will have to investigate a bit more, about how to proceed once we have the fields for mapping.

What do you think? Am I on the right path?

Member

filiphr commented Oct 8, 2016

@gunnarmorling I had a look at this issue. And I think I can start working on it. I had a deeper look at the code and the inner workings. I think that in order to do this, we would need to add our own Accessor class which would wrap the ExecutableElemement(s) and the VariableElements. In our own Accessor we would add the methods which we need and get them from the underlying Element.
The Type methods getPropertyPresenceCheckers, getPropertyReadAccessors and getPropertyWriteAccessors will need to be adjusted to return our wrapper.

This is only the first step, I will have to investigate a bit more, about how to proceed once we have the fields for mapping.

What do you think? Am I on the right path?

@filiphr

This comment has been minimized.

Show comment
Hide comment
@filiphr

filiphr Oct 8, 2016

Member

Another thing, we are using the AccessorNamingStrategy with the ExecutableElement(s). I guess this is public and should not be modified?

Member

filiphr commented Oct 8, 2016

Another thing, we are using the AccessorNamingStrategy with the ExecutableElement(s). I guess this is public and should not be modified?

@gunnarmorling

This comment has been minimized.

Show comment
Hide comment
@gunnarmorling

gunnarmorling Oct 9, 2016

Member

@filiphr Thanks for picking up this one!

What do you think? Am I on the right path?

Yes, I think building such common abstraction would the right way to go.

we are using the AccessorNamingStrategy with the ExecutableElement(s)

Good question, I reckon that it doesn't need to be changed. A field named "foo" really can only represent a property named "foo" and it cannot be a "presence checker" or any of the other method types.

Member

gunnarmorling commented Oct 9, 2016

@filiphr Thanks for picking up this one!

What do you think? Am I on the right path?

Yes, I think building such common abstraction would the right way to go.

we are using the AccessorNamingStrategy with the ExecutableElement(s)

Good question, I reckon that it doesn't need to be changed. A field named "foo" really can only represent a property named "foo" and it cannot be a "presence checker" or any of the other method types.

@filiphr

This comment has been minimized.

Show comment
Hide comment
@filiphr

filiphr Dec 3, 2016

Member

Official support for direct field mappings in MapStruct is possible since #928. You can try it out with the latest SNAPSHOT version. Once the documentation PR #982 is merged the ticket will be closed.

Member

filiphr commented Dec 3, 2016

Official support for direct field mappings in MapStruct is possible since #928. You can try it out with the latest SNAPSHOT version. Once the documentation PR #982 is merged the ticket will be closed.

@filiphr

This comment has been minimized.

Show comment
Hide comment
@filiphr

filiphr Dec 4, 2016

Member

With the merged documentation from #982 this new feature is completely implemented

Member

filiphr commented Dec 4, 2016

With the merged documentation from #982 this new feature is completely implemented

@filiphr filiphr closed this Dec 4, 2016

filiphr added a commit to filiphr/mapstruct that referenced this issue Dec 17, 2016

filiphr added a commit that referenced this issue Dec 19, 2016

@cpoissonnier

This comment has been minimized.

Show comment
Hide comment
@cpoissonnier

cpoissonnier Feb 20, 2017

Can't wait this feature to be available in the next version (I need this to use Mapstruct in my Playframework projects). Do you have any idea when 1.2.0 will be released?

cpoissonnier commented Feb 20, 2017

Can't wait this feature to be available in the next version (I need this to use Mapstruct in my Playframework projects). Do you have any idea when 1.2.0 will be released?

@gunnarmorling

This comment has been minimized.

Show comment
Hide comment
@gunnarmorling

gunnarmorling Feb 20, 2017

Member
Member

gunnarmorling commented Feb 20, 2017

@cpoissonnier

This comment has been minimized.

Show comment
Hide comment
@cpoissonnier

cpoissonnier Feb 20, 2017

Amazing timing, thanks !

cpoissonnier commented Feb 20, 2017

Amazing timing, thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment