Skip to content
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

Copy constructor does not preserve _injectableValues #696

Closed
drcrallen opened this issue Feb 5, 2015 · 3 comments
Closed

Copy constructor does not preserve _injectableValues #696

drcrallen opened this issue Feb 5, 2015 · 3 comments

Comments

@drcrallen
Copy link
Contributor

Currently the copy constructor makes no effort to copy injectable values. I'm not quite sure what the proper behavior of this would be, whether the ObjectMapper copy should retain a copy of the values at the time of the copy, or if the copy and original should just point to the same instance of the injectable values. Or if it just needs to be noted in the injectables documentation that the injectables are not attempted to be preserved across copies.

@drcrallen
Copy link
Contributor Author

It is worth noting that currently there is no way to recover _injectableValues without resorting to reflection or independently retaining the original value passed to setInjectableValues.

drcrallen added a commit to metamx/druid that referenced this issue Feb 5, 2015
@cowtowncoder
Copy link
Member

Right. I had to think for a bit, to figure out whether this is a bug or feature.
I think it is a bug, and that settings should be copied. And as (or, more) importantly, ensure same happens with ObjectReader, which is the preferred way of interacting with configuration these days.

As to whether these should accessible or not... so far I haven't thought they need to be made visible, hence no accessor added. I can add one; they are not state secret either. :)

cowtowncoder added a commit that referenced this issue Feb 6, 2015
@cowtowncoder cowtowncoder added this to the 1.9.13 milestone Feb 6, 2015
@cowtowncoder
Copy link
Member

Fixed; 2.5.1 will copy injectables as expected, and 2.6 has additional accessors.

Thank you again for reporting this!

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

No branches or pull requests

2 participants