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

Object ID handling tries (unnecessarily) to set id value on a record #4729

Open
1 task done
hjohn opened this issue Oct 2, 2024 · 2 comments
Open
1 task done

Object ID handling tries (unnecessarily) to set id value on a record #4729

hjohn opened this issue Oct 2, 2024 · 2 comments
Labels
Record Issue related to JDK17 java.lang.Record support to-evaluate Issue that has been received but not yet evaluated

Comments

@hjohn
Copy link

hjohn commented Oct 2, 2024

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

When using JsonIdentityInfo on a record, Jackson tries to set the id on the record despite it already being correct. This is odd to say the least, as the correct record is already found, which has the correct id (otherwise it wouldn't be found), and then after that it tries to set the id. Here's the code that's causing the problem in PropertyValueBuffer:

public Object handleIdValue(final DeserializationContext ctxt, Object bean) throws IOException
{
    if (_objectIdReader != null) {
        if (_idValue != null) {
            ReadableObjectId roid = ctxt.findObjectId(_idValue, _objectIdReader.generator, _objectIdReader.resolver);
            roid.bindItem(bean);
            // also: may need to set a property value as well
            SettableBeanProperty idProp = _objectIdReader.idProperty;
            if (idProp != null) {
                return idProp.setAndReturn(bean, _idValue);
            }
        } else {
            // 07-Jun-2016, tatu: Trying to improve error messaging here...
            ctxt.reportUnresolvedObjectId(_objectIdReader, bean);
        }
    }
    return bean;
}

Just before it calls idProp.setAndReturn everything is already correct. The bean variable holds the correct record Device[id=Arris]. The _idValue holds Arris. The idProp is [creator property, name 'id'; inject id 'null']. roId holds [ObjectId: key=Arris, type=com.fasterxml.jackson.databind.deser.impl.PropertyBasedObjectIdGenerator, scope=java.lang.Object].

In other words, it found the correct bean, which already has the correct id, but Jackson wants to still set the id value ... to the same value...

Remvoing the line return idProp.setAndReturn(bean, _idValue); makes the deserialization work correctly.

Version Information

2.18.0

Reproduction

Run below snippet. It will throw an exception:

 No fallback setter/field defined for creator property 'id' (of `org.int4.nexus.core.JacksonBug$Device`) (through reference chain: org.int4.nexus.core.JacksonBug$Configuration["devices"]->java.util.ArrayList[0])
import com.fasterxml.jackson.annotation.JsonIdentityInfo;
import com.fasterxml.jackson.annotation.JsonIdentityReference;
import com.fasterxml.jackson.annotation.ObjectIdGenerators;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import java.util.List;

public class JacksonBug {

  static String input = """
    {
      "devices": [
        {
          "id": "Arris"
        }
      ],
      "activities": [
        {
          "id": "TV",
          "participants": ["Arris"]
        }
      ]
    }
  """;

  public static void main(String[] args) throws JsonMappingException, JsonProcessingException {
    ObjectMapper mapper = new ObjectMapper();

    Configuration c = mapper.readValue(input, Configuration.class);

  }

  @JsonIdentityInfo(property = "id", generator = ObjectIdGenerators.PropertyGenerator.class)
  record Device(String id) {}

  record Configuration(List<Device> devices, List<Activity> activities) {}

  record Activity(String id, @JsonIdentityReference(alwaysAsId = true) List<Device> participants) {}
}

Expected behavior

I'd expect this to just work out of the box. It shouldn't be trying to call setters on a record under any circumstances...

Additional context

No response

@hjohn hjohn added the to-evaluate Issue that has been received but not yet evaluated label Oct 2, 2024
@hjohn
Copy link
Author

hjohn commented Oct 2, 2024

I was looking into a better fix for this, and realized that idProp probably should just be null for records. When I use the ObjectIdGenerators.StringIdGenerator (even though I really don't want to generate id's) it does work. Using ObjectIdGenerators.None (which I tried initially) doesn't work either though, as I get a different exception then:

Cannot construct instance of `org.int4.nexus.core.JacksonBug$Device` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('Arris')

I'm able to proceed though using the StringIdGenerator for now.

It's unclear to me which of the ObjectIdGenerators should and should not work in combination with records :)

@JooHyukKim
Copy link
Member

Thank you for reporting this.
There has been some rewrites regarding property introspection in 2.18, but no related change has been made to PropertyValueBuffer since 2021 (according to Git history).

realized that idProp probably should just be null for records

And yes, I agree. Seems like it.

@cowtowncoder cowtowncoder added the Record Issue related to JDK17 java.lang.Record support label Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Record Issue related to JDK17 java.lang.Record support to-evaluate Issue that has been received but not yet evaluated
Projects
None yet
Development

No branches or pull requests

3 participants