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

Deserialization result error when two properties return the same collection object #4312

Closed
SWQXDBA opened this issue Jan 9, 2024 · 9 comments

Comments

@SWQXDBA
Copy link

SWQXDBA commented Jan 9, 2024

When two properties return the same object, deserialization results in double the number of elements in the collection being populated.
This is an example of minimal reproduction

public class Main2 {
    @lombok.Data
    public static class Data{
        List<Long> idList;

        public List<Long> getMyIdList() {
            return idList;
        }
    }

    public static void main(String[] args) throws JsonProcessingException {
        ObjectMapper objectMapper = new ObjectMapper();
        Data data = new Data();
        data.idList = Arrays.asList(1L,2L,3L,4L,5L);
        String jsonStr = objectMapper.writeValueAsString(data);
        System.out.println(jsonStr);//{"idList":[1,2,3,4,5],"myIdList":[1,2,3,4,5]}
        
        Data data1 = objectMapper.readValue(jsonStr, Data.class);
        assert data1.getIdList().size() != data.getIdList().size();
        System.out.println(data1);//Main2.Data(idList=[1, 2, 3, 4, 5, 1, 2, 3, 4, 5]) 
    }
}

More seriously, if such an object is subjected to the serialization and deserialization process over and over again, after a few times, it will cause a memory overflow.

@SWQXDBA
Copy link
Author

SWQXDBA commented Jan 9, 2024

the version is 2.13.2

@cowtowncoder
Copy link
Member

In future, please do not report against old versions: should be reproducible with 2.16(.1).

Second: Lombok does bytecode manipulation so we would need to see actual class being used, not Lombok definition (since it is different from declaration, adding possible setters/getters/fields).

Beyond this it'd be necessary to see the actual class: this may be due to non-obvious feature where "getter" can be used to get collection for deserialization purpose (if no setter or field found).
This can be disabled via MapperFeature.USE_GETTERS_AS_SETTERS (defaults to enabled) -- that would probably stop deserialization via getter-returned List.
But it seems like may be something sub-optimal in POJO definition as Jackson sees 2 logical properties whereas I assume you only want one?

@cowtowncoder
Copy link
Member

(wrong repo, moving to jackson-databind)

@cowtowncoder cowtowncoder transferred this issue from FasterXML/jackson-core Jan 9, 2024
@SWQXDBA
Copy link
Author

SWQXDBA commented Jan 10, 2024

I try the version of 2.16.1, it exists too.
the codes without lombok:

    public static class Data{

        List<Long> idList = new ArrayList<>();

        public Data setIdList(List<Long> idList) {
            this.idList = idList;
            return this;
        }

        public List<Long> getIdList() {
            return idList;
        }

        public  List<Long> getMyIdList(){
            return idList;
        }

    }

    public static void main(String[] args) throws JsonProcessingException {
        ObjectMapper objectMapper = new ObjectMapper();
        Data data = new Data();
        data.setIdList( Arrays.asList(1L,2L,3L,4L,5L));
        String jsonStr = objectMapper.writeValueAsString(data);
        System.out.println(jsonStr);//{"idList":[1,2,3,4,5],"myIdList":[1,2,3,4,5]}

        Data data1 = objectMapper.readValue(jsonStr, Data.class);
        assert data1.getIdList().size() != data.getIdList().size();
        String jsonStr2 = objectMapper.writeValueAsString(data1);
       //{"idList":[1,2,3,4,5,1,2,3,4,5],"myIdList":[1,2,3,4,5,1,2,3,4,5]}
        System.out.println(jsonStr2);
    }

And then,when I return a List with a new Object like new ArrayList(idList) that is ok;

   public static class Data{

        List<Long> idList = new ArrayList<>();

        public Data setIdList(List<Long> idList) {
            this.idList = idList;
            return this;
        }

        public List<Long> getIdList() {
            return idList;
        }

        public  List<Long> getMyIdList(){
            return new ArrayList<>(idList);
        }

    }

    public static void main(String[] args) throws JsonProcessingException {
        ObjectMapper objectMapper = new ObjectMapper();
        Data data = new Data();
        data.setIdList( Arrays.asList(1L,2L,3L,4L,5L));
        String jsonStr = objectMapper.writeValueAsString(data);
        System.out.println(jsonStr);//{"idList":[1,2,3,4,5],"myIdList":[1,2,3,4,5]}

        Data data1 = objectMapper.readValue(jsonStr, Data.class);
        assert data1.getIdList().size() != data.getIdList().size();
        String jsonStr2 = objectMapper.writeValueAsString(data1);
        //{"idList":[1,2,3,4,5],"myIdList":[1,2,3,4,5]}
        System.out.println(jsonStr2);
    }

All in all, I think it's probably a good idea to do a clear on a collection that's been serialized using a getter.

@SWQXDBA
Copy link
Author

SWQXDBA commented Jan 10, 2024

Or, if you need to deserialize a collection obtained using a getter, you need to check for the existence of a setter method (even if you don't use a setter method, you need to make sure the setter exists). It makes no sense to deserialize a logical property. Logical properties only need to be serialized, not deserialized.

@cowtowncoder
Copy link
Member

My problem here is that while I understand what Jackson does and why (despite possibly surprising behavior of MapperFeature.USE_GETTERS_AS_SETTERS which was added years ago for JAXB compatibility), I do not quite
understand what your object tries to achieve -- why are you exposing same List as 2 separate logical properties? (getters with different implied property name).
In most cases one would serialize just one; and if that is what you want, it is easy enough to achieve by f.ex @JsonIgnoreing one (getMyIdList() I'd guess).

I do not think there is anything that needs changing on Jackson side (USE_GETTERS_AS_SETTERS will be disabled by default for Jackson 3.0 as I don't think it is a good feature in general). But I would like to help you make things work the way they should, if possible.

@SWQXDBA
Copy link
Author

SWQXDBA commented Jan 10, 2024

My problem here is that while I understand what Jackson does and why (despite possibly surprising behavior of MapperFeature.USE_GETTERS_AS_SETTERS which was added years ago for JAXB compatibility), I do not quite understand what your object tries to achieve -- why are you exposing same List as 2 separate logical properties? (getters with different implied property name). In most cases one would serialize just one; and if that is what you want, it is easy enough to achieve by f.ex @JsonIgnoreing one (getMyIdList() I'd guess).

I do not think there is anything that needs changing on Jackson side (USE_GETTERS_AS_SETTERS will be disabled by default for Jackson 3.0 as I don't think it is a good feature in general). But I would like to help you make things work the way they should, if possible.

This is just a simple example. Real business scenarios can be much more complex.

For example, to determine the value returned based on some conditions

    public class Data {

        List<Long> listA;

        List<Long> listB;

        Boolean predicate;

        public List<Long> getViewList(){
            if(predicate==null){
                return new ArrayList<>();
            }
            if(predicate){
                return listA;
            }else{
                return listB;
            }
        }
        //...other getter setters
    }

It's similar to a view property, except that under certain conditions it actually returns the object of another property.

@cowtowncoder
Copy link
Member

Yes, that's fine -- user needs to be aware of linking between actual values and not create problematic data structures.

Jackson does not, and should not try to track physical references across values: it operates on logical property values, based on possible annotations.
If code mixes and matches values, possible with cycles, that is problem with data model. Jackson does not do (and cannot really do) arbitrary graph traversal.
It just follows what it introspects to be the model via properties detected.

@cowtowncoder
Copy link
Member

At this point I am ready to close the issue, leaving open for a day or two for additional information on concrete problem to solve.

One last thing I'll mention is that if actual Object identity needs to be retained (to avoid duplicate serialization as JSON, as well as re-link values on deserialization, there is a way to do that, using @JsonIdentityInfo.
That is the mechanism that allows handling of cyclic data structures (to some degree).

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