vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
618 stars 167 forks source link

JsonSerializer: add support for dependency loops inside the serialized objects #2085

Open gilberto-torrezan opened 7 years ago

gilberto-torrezan commented 7 years ago

Example object:

public class A {
    private B child;

    // getter and setter
}

public class B {
    private A parent;

    // getter and setter
}

A a = new A();
B b = new B();
a.setChild(b);
b.setParent(a);
JsonSerializer.toJson(a);

Currently the serialization enters in an infinite loop that causes a StackOverflow when trying to convert the object to json.

Legioth commented 7 years ago

Is there any real-world case when this happens, or is it only a theoretical issue?

No3x commented 5 years ago

I also run into this issue when using a bidirectional JPA relationship. In the following example with collections, but it's the same issue.

Recipe (1) <-----> (*) RecipeItem Stacktrace

```java @Entity @Table(name = "recipe") public class Recipe implements Serializable { private static final long serialVersionUID = 1L; @Id @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; @NotNull @Column(name = "name", nullable = false, unique = true) private String name; @OneToMany(mappedBy = "recipe") private Set recipeItems = new HashSet<>(); public Long getId() { return id; } @Override public Integer getTotalPrice() { return 42; } public void setId(Long id) { this.id = id; } public String getName() { return name; } public Recipe name(String name) { this.name = name; return this; } public void setName(String name) { this.name = name; } public Set getRecipeItems() { return recipeItems; } public Recipe recipeItems(Set recipeItems) { this.recipeItems = recipeItems; return this; } public Recipe addRecipeItems(RecipeItem recipeItem) { this.recipeItems.add(recipeItem); recipeItem.setRecipe(this); return this; } public Recipe removeRecipeItems(RecipeItem recipeItem) { this.recipeItems.remove(recipeItem); recipeItem.setRecipe(null); return this; } public void setRecipeItems(Set recipeItems) { this.recipeItems = recipeItems; } @Override public boolean equals(Object o) { if (this == o) { return true; } if (o == null || getClass() != o.getClass()) { return false; } Recipe recipe = (Recipe) o; if (recipe.getId() == null || getId() == null) { return false; } return Objects.equals(getId(), recipe.getId()); } @Override public int hashCode() { return Objects.hashCode(getId()); } @Override public String toString() { return "Recipe{" + "id=" + getId() + ", name='" + getName() + "'" + "}"; } } ``` ```java @Entity @Table(name = "recipe_item") public class RecipeItem implements Serializable { private static final long serialVersionUID = 1L; @Id @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; @ManyToOne @JsonIgnoreProperties(value = {"recipeItems"}) private Recipe recipe; public Long getId() { return id; } public void setId(Long id) { this.id = id; } public Recipe getRecipe() { return recipe; } public RecipeItem recipe(Recipe recipe) { this.recipe = recipe; return this; } public void setRecipe(Recipe recipe) { this.recipe = recipe; } public Ingredient getIngredient() { return ingredient; } public RecipeItem ingredient(Ingredient ingredient) { this.ingredient = ingredient; return this; } public void setIngredient(Ingredient ingredient) { this.ingredient = ingredient; } @Override public boolean equals(Object o) { if (this == o) { return true; } if (o == null || getClass() != o.getClass()) { return false; } RecipeItem recipeItem = (RecipeItem) o; if (recipeItem.getId() == null || getId() == null) { return false; } return Objects.equals(getId(), recipeItem.getId()); } @Override public int hashCode() { return Objects.hashCode(getId()); } @Override public String toString() { return "RecipeItem{" + "id=" + getId() + "}"; } } ```

A simple test to show the error:

public class SerializeTest {

    @Test
    public void shouldSerialize() {
        Recipe r = new Recipe();
        RecipeItem ri = new RecipeItem();
        ri.setRecipe(r);
        r.setRecipeItems(Sets.newHashSet(ri));
        ri.setRecipe(r);
        r.addRecipeItems(ri);

        String json = JsonSerializer.toJson(r).toJson();  // StackOverflowError exception
        System.out.println(json);
        assertTrue(true);
    }
}
Stacktrace

Note the cycle ```java java.lang.StackOverflowError at sun.reflect.misc.ReflectUtil.checkPackageAccess(ReflectUtil.java:164) at sun.reflect.misc.ReflectUtil.isPackageAccessible(ReflectUtil.java:195) at java.beans.Introspector.getBeanInfo(Introspector.java:164) at com.vaadin.flow.internal.JsonSerializer.toJson(JsonSerializer.java:83) at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193) at java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1556) at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481) at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471) at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151) at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174) at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.util.stream.ReferencePipeline.forEachOrdered(ReferencePipeline.java:423) at com.vaadin.flow.internal.JsonSerializer.toJson(JsonSerializer.java:120) at com.vaadin.flow.internal.JsonSerializer.toJson(JsonSerializer.java:67) at com.vaadin.flow.internal.JsonSerializer.toJson(JsonSerializer.java:90) at com.vaadin.flow.internal.JsonSerializer.toJson(JsonSerializer.java:90) at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193) at java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1556) at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481) at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471) at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151) at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174) at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.util.stream.ReferencePipeline.forEachOrdered(ReferencePipeline.java:423) at com.vaadin.flow.internal.JsonSerializer.toJson(JsonSerializer.java:120) at com.vaadin.flow.internal.JsonSerializer.toJson(JsonSerializer.java:67) at com.vaadin.flow.internal.JsonSerializer.toJson(JsonSerializer.java:90) at com.vaadin.flow.internal.JsonSerializer.toJson(JsonSerializer.java:90) at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193) at java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1556) at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481) at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471) ```

Jackson for example provides JsonIgnoreProperties to break the cycle.

I can't believe the issue is still present. This would mean there is nobody using bidirectional associations in any of their objects that go to the UI or it would mean they all map to DTO before.

Legioth commented 5 years ago

What would you expect the JSON to look like?

No3x commented 5 years ago

I used the following test with jackson mapper to produce the json I would expect.

public class SerializeTest {

    @Test
    public void shouldSerialize() throws JsonProcessingException {
        Recipe r = new Recipe();
        r.setId(42L);
        r.setName("My Recipe");

        RecipeItem ri = new RecipeItem();
        ri.setId(1L);
        ri.setRecipe(r);
        r.setRecipeItems(Sets.newHashSet(ri));
        r.addRecipeItems(ri);

        final ObjectMapper objectMapper = new ObjectMapper();
        String jsonJackson = objectMapper.writeValueAsString(r);
        System.out.println(jsonJackson); // {"id":42,"name":"My Recipe","recipeItems":[{"id":1,"recipe":{"id":42,"name":"My Recipe"}}]}

        String json = JsonSerializer.toJson(r).toJson();  // StackOverflowError exception
        System.out.println(json);

        assertTrue(true);
    }
}
{
  "id": 42,
  "name": "My Recipe",
  "recipeItems": [
    {
      "id": 1,
      "recipe": {
        "id": 42,
        "name": "My Recipe"
      }
    }
  ]
}
Legioth commented 5 years ago

I don't understand the logic for that JSON output. Why is the the item with id 42 duplicated but not the item with id 1?

No3x commented 5 years ago

That's because of the @JsonIgnoreProperties annotation in RecipeItem - otherwise it would produce a cycle.

@JsonIgnoreProperties(value = {"recipeItems"})
private Recipe recipe;

It tells jackson to omit the recipeItems-property when serializing the recipe.

If you omit the annotation jackson fails with an exception that explains the cycle:

com.fasterxml.jackson.databind.JsonMappingException: Infinite recursion (StackOverflowError) 
(through reference chain: RecipeItem["recipe"]->Recipe["recipeItems"]->java.util.HashSet[0]->RecipeItem["recipe"]->Recipe["recipeItems"]->java.util.HashSet[0]->RecipeItem["recipe"] ...

I hope this helps.