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

Rearranging of props when property-based generator is in use leads to incorrect output #2759

Closed
oshatrk opened this issue Jun 12, 2020 · 6 comments
Milestone

Comments

@oshatrk
Copy link

oshatrk commented Jun 12, 2020

When these annotations:

@JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class, property = "id")
@JsonIdentityReference(alwaysAsId = true)

are used on a link to a parent inside a child, and id is not the first field of the parent class,
then JSON output is incorrect.

With example code I got this:

{"name":"main hive","bees":[{"id":1,"hiveId":100500}],"bees":[{"id":1,"hiveId":100500}]}

Expected this:

{"name":"main hive","bees":[{"id":1,"hiveId":100500}],"id":100500}

Notice that private Long id; is not the first field of the Hive class.

A possible bug is in BeanSerializerBase.createContextual(), near this line:

// Let's force it to be the first property to output

Example:

import java.util.ArrayList;
import java.util.List;

import com.fasterxml.jackson.annotation.JsonIdentityInfo;
import com.fasterxml.jackson.annotation.JsonIdentityReference;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.ObjectIdGenerators;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

public class Main {

    public static class Hive {

        private String name;

        private List<Bee> bees = new ArrayList<>();

        private Long id;

        public Hive() {
        }

        public Hive(Long id, String name) {
            this.id = id;
            this.name = name;
        }

        public void addBee(Bee bee) {
            bees.add(bee);
        }

        public Long getId() {
            return id;
        }

        public void setId(Long id) {
            this.id = id;
        }

        public String getName() {
            return name;
        }

        public void setName(String name) {
            this.name = name;
        }

        public List<Bee> getBees() {
            return bees;
        }

        public void setBees(List<Bee> bees) {
            this.bees = bees;
        }
    }

    public static class Bee {

        private Long id;

        @JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class, property = "id")
        @JsonIdentityReference(alwaysAsId = true)
        @JsonProperty("hiveId")
        private Hive hive;

        public Bee() {
        }

        public Bee(Long id, Hive hive) {
            this.id = id;
            this.hive = hive;
        }

        public Long getId() {
            return id;
        }

        public void setId(Long id) {
            this.id = id;
        }

        public Hive getHive() {
            return hive;
        }

        public void setHive(Hive hive) {
            this.hive = hive;
        }
    }

    public static void main(String[] args) throws JsonProcessingException {

        Hive hive = new Hive(100500L, "main hive");
        hive.addBee(new Bee(1L, hive));

        ObjectMapper mapper = new ObjectMapper();
        System.out.println(mapper.writeValueAsString(hive));
    }

}
@cowtowncoder
Copy link
Member

Just to make sure: so the observed problem is duplication of a property? That would be wrong, yes. Thank you for providing a reproduction.

One quick question: which version(s) is this with? 2.11.0 or something else?

@cowtowncoder
Copy link
Member

I can reproduce this with 2.11(.0) and 2.12.0-SNAPSHOT.
Thank you for reporting the issue: I am guessing internally there is duplication due to problems during reordering.

@oshatrk
Copy link
Author

oshatrk commented Jun 15, 2020

I feel it is not just a "duplication", but a sort of "shifting" (try put id into the middle of many fields).

The described problem can also arise if @JsonProperty is used on id field, that is if in the example code you'll write something like this:

...
    public static class Hive {

        @JsonProperty("id")
        private Long id;

        private String name;

        private List<Bee> bees = new ArrayList<>();
...

@oshatrk
Copy link
Author

oshatrk commented Jun 15, 2020

I accidentally found the similar logic in another class (BeanSerializerFactory.constructObjectIdHandler()).
Maybe it should be reviewed also (sorry I don't have a test for this one).

cowtowncoder added a commit that referenced this issue Jun 16, 2020
@cowtowncoder
Copy link
Member

cowtowncoder commented Jun 17, 2020

@oshatrk Ah. Lol. Hmmh. That code looked wrong, wrt reordering but... I'll have to a closer look. Thought I spotted a bug but not necessarily.

@cowtowncoder
Copy link
Member

cowtowncoder commented Jun 17, 2020

Hmmh. I bet it's not reordering that was wrong but rather that mutability is wrong: createContextual MUST create copies...

Yes. That's very likely it: arrays of properties ends up being shared between original (parent), copy (child), and that's likely where problem comes up. Now I need to figure out how to change createContextual to avoid this.

Quick and dirty check on forcing copy (after making _props non-final) does prevent the problem.

@cowtowncoder cowtowncoder added this to the 1.9.13 milestone Jun 17, 2020
@cowtowncoder cowtowncoder modified the milestones: 1.9.13, 2.11.1 Jun 17, 2020
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