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

Final implementation specific collection is overridden on deserialization #966

Closed
reda-alaoui opened this issue Oct 11, 2015 · 7 comments
Closed

Comments

@reda-alaoui
Copy link
Contributor

Hello,

I am using Jackson 2.5.1, Hibernate and the Jackson Hibernate Module.

Sometimes I need to merge a JSON String into an object returned by hibernate.
When the hibernate provided object contains an attribute of type Set (or another collection subtype), the attribute value is often a specific Hibernate collection implementation, PersistentSet in our example.

Merging a JSON into the hibernate provided object looks ok, except the collection implementation is changed by Jackson. In our example, PersistentSet is turned into HashSet.
This behaviour leads later to an exception on save because Hibernate detect this inconsistency.

The problem seems located in ManagedReferenceProperty.java:

@Override
    public Object setAndReturn(Object instance, Object value) throws IOException
    {
        /* 04-Feb-2014, tatu: As per [#390], it may be necessary to switch the
         *   ordering of forward/backward references, and start with back ref.
         */
        if (value != null) {
            if (_isContainer) { // ok, this gets ugly... but has to do for now
                if (value instanceof Object[]) {
                    for (Object ob : (Object[]) value) {
                        if (ob != null) { _backProperty.set(ob, instance); }
                    }
                } else if (value instanceof Collection<?>) {
                    for (Object ob : (Collection<?>) value) {
                        if (ob != null) { _backProperty.set(ob, instance); }
                    }
                } else if (value instanceof Map<?,?>) {
                    for (Object ob : ((Map<?,?>) value).values()) {
                        if (ob != null) { _backProperty.set(ob, instance); }
                    }
                } else {
                    throw new IllegalStateException("Unsupported container type ("+value.getClass().getName()
                            +") when resolving reference '"+_referenceName+"'");
                }
            } else {
                _backProperty.set(value, instance);
            }
        }
        // and then the forward reference itself
        return _managedProperty.setAndReturn(instance, value);
    }

The last method line replace the existing PersistentSet value with the new HashSet value.

When the collection is already initiliazed in the target object, would it be possible to keep it, clear it and populate it with the Jackson generated collection?

@reda-alaoui reda-alaoui changed the title Final implementation specific collection is overrided on deserialization Final implementation specific collection is overridden on deserialization Oct 11, 2015
@cowtowncoder
Copy link
Member

There is no functionality currently to do sort of merge with possibly existing Collection/Map valued property, although there are plans/hopes to allow this on per-property basis.
Functionality for doing this does actually sort-of exist: in case there is no setter or field available for a property, it is possible to use "getter-as-setter". However, this is not the default handling, and for various reasons (from backwards compatibility to efficiency reasons -- it is costly to use reflection, and always checking for getter first would be costly for common case where value is null) it can not be made the default.

So, as things are things can be done.

To further complicate things, while allowing merge would work for regular properties (once implemented), it is not certain how feasible it will be for parent/child properties; they go through additional processing.

@reda-alaoui
Copy link
Contributor Author

Thanks for your answer.

Here is my proposition to start with a global configuration.

First:

  • Add a DeserializationFeature flag CAN_OVERRIDE_FINAL_COLLECTION_MAP_INSTANCE, enabled by default
  • If enabled SetterlessProperty will be preferred to SettableProperty in case of final collection field

Then:

  • Add a DeserializationFeature flag CLEAR_EXISTING_COLLECTION_MAP_BEFORE_DESERIALIZATION, disabled by default
  • If enabled, collections/map will be cleared before any modification

I can make a pull request if you confirm the design.

@reda-alaoui
Copy link
Contributor Author

I opened a pull request implementing the described behaviour.
Openjdk 6 build is failing on classes that i have not changed.

@cowtowncoder
Copy link
Member

Thank you for PR. It may take me some time to check it out -- changes are delightfully small (yay!), but since the change itself is potentially wide-ranging I want to make sure I understand consequences.

@cowtowncoder
Copy link
Member

I suspect #1399 might solve much of this issue, or perhaps all of it. Leaving open for now.

@cowtowncoder cowtowncoder removed the 2.9 label Oct 28, 2016
@reda-alaoui
Copy link
Contributor Author

#1399 is enough for us.
Closing this issue ;)

@cowtowncoder
Copy link
Member

@reda-alaoui perfect, thank you for following up on 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