Merge pull request #10916 from playframework/mergify/bp/2.8.x/pr-10840

Fix tail-recursive deserializer (Lagom's #3241) (backport #10840)
playframework · Jul 9, 2021 · 60ec676 · 60ec676
2 parents 039a02d + 0770dbc
commit 60ec676
Show file tree

Hide file tree

Showing 5 changed files with 202 additions and 7 deletions.
diff --git a/core/play/src/main/scala/play/utils/JsonNodeDeserializer.scala b/core/play/src/main/scala/play/utils/JsonNodeDeserializer.scala
@@ -196,16 +196,15 @@ private class JsonNodeDeserializer extends JsonDeserializer[JsonNode] {
       case JsonTokenId.ID_EMBEDDED_OBJECT => (Some(fromEmbedded(jp, ctxt)), parserContext)
     }
 
-    // Read ahead
-    jp.nextToken()
-
     maybeValue match {
       case Some(v) if nextContext.isEmpty =>
         // done, no more tokens and got a value!
         // note: jp.getCurrentToken == null happens when using treeToValue (we're not parsing tokens)
         v
 
       case maybeValue =>
+        // Read ahead
+        jp.nextToken()
         val toPass = maybeValue
           .map { v =>
             val previous :: stack = nextContext

diff --git a/core/play/src/test/java/play/utils/Child.java b/core/play/src/test/java/play/utils/Child.java
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) Lightbend Inc. <https://www.lightbend.com>
+ */
+
+package play.utils;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import org.checkerframework.checker.nullness.qual.NonNull;
+
+import java.util.Objects;
+
+// refs https://github.com/lagom/lagom/issues/3241
+@JsonDeserialize(using = ChildDeserializer.class)
+public class Child {
+
+  private final @NonNull Long updatedAt;
+  private final @NonNull String updatedBy;
+
+  @JsonCreator
+  public Child(@NonNull Long updatedAt, @NonNull String updatedBy) {
+    this.updatedAt = updatedAt;
+    this.updatedBy = updatedBy;
+  }
+
+  public Long getUpdatedAt() {
+    return updatedAt;
+  }
+
+  public String getUpdatedBy() {
+    return updatedBy;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) return true;
+    if (o == null || getClass() != o.getClass()) return false;
+    Child child = (Child) o;
+    return updatedAt.equals(child.updatedAt) && updatedBy.equals(child.updatedBy);
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(updatedAt, updatedBy);
+  }
+
+  @Override
+  public String toString() {
+    return "Child{" + "updatedAt=" + updatedAt + ", updatedBy='" + updatedBy + '\'' + '}';
+  }
+}
diff --git a/core/play/src/test/java/play/utils/ChildDeserializer.java b/core/play/src/test/java/play/utils/ChildDeserializer.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) Lightbend Inc. <https://www.lightbend.com>
+ */
+
+package play.utils;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
+
+import java.io.IOException;
+
+// refs https://github.com/lagom/lagom/issues/3241
+@SuppressWarnings("WeakerAccess")
+public class ChildDeserializer extends StdDeserializer<Child> {
+
+  public ChildDeserializer() {
+    this(null);
+  }
+
+  public ChildDeserializer(Class<?> vc) {
+    super(vc);
+  }
+
+  @Override
+  public Child deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException {
+    JsonNode node = jp.readValueAsTree();
+    String updatedBy = node.get("updatedBy").asText();
+    Long updatedAt = node.get("updatedAt").asLong();
+
+    return new Child(updatedAt, updatedBy);
+  }
+}
diff --git a/core/play/src/test/java/play/utils/Parent.java b/core/play/src/test/java/play/utils/Parent.java
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) Lightbend Inc. <https://www.lightbend.com>
+ */
+
+package play.utils;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.checkerframework.checker.nullness.qual.NonNull;
+
+import java.util.Objects;
+
+// refs https://github.com/lagom/lagom/issues/3241
+public class Parent {
+
+  private final @NonNull Long createdAt;
+  private final Child child;
+  private final @NonNull Long updatedAt;
+  private final @NonNull String updatedBy;
+
+  @JsonCreator
+  public Parent(
+      @JsonProperty("createdAt") @NonNull Long createdAt,
+      @JsonProperty("child") Child child,
+      @JsonProperty("updatedAt") @NonNull Long updatedAt,
+      @JsonProperty("updatedBy") @NonNull String updatedBy) {
+    this.createdAt = createdAt;
+    this.child = child;
+    this.updatedAt = updatedAt;
+    this.updatedBy = updatedBy;
+  }
+
+  public Long getCreatedAt() {
+    return createdAt;
+  }
+
+  public Child getChild() {
+    return child;
+  }
+
+  public Long getUpdatedAt() {
+    return updatedAt;
+  }
+
+  public String getUpdatedBy() {
+    return updatedBy;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) return true;
+    if (o == null || getClass() != o.getClass()) return false;
+    Parent parent = (Parent) o;
+    return createdAt.equals(parent.createdAt)
+        && child.equals(parent.child)
+        && updatedAt.equals(parent.updatedAt)
+        && updatedBy.equals(parent.updatedBy);
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(createdAt, child, updatedAt, updatedBy);
+  }
+
+  @Override
+  public String toString() {
+    return "Parent{"
+        + "createdAt="
+        + createdAt
+        + ", child="
+        + child
+        + ", updatedAt="
+        + updatedAt
+        + ", updatedBy='"
+        + updatedBy
+        + '\''
+        + '}';
+  }
+}
diff --git a/core/play/src/test/scala/play/utils/JsonNodeDeserializerSpec.scala b/core/play/src/test/scala/play/utils/JsonNodeDeserializerSpec.scala
@@ -4,14 +4,11 @@
 
 package play.utils
 
-import java.math.MathContext
-
-import com.fasterxml.jackson.core.JsonParser
 import com.fasterxml.jackson.core.json.JsonReadFeature
 import com.fasterxml.jackson.databind.DeserializationFeature
-import com.fasterxml.jackson.databind.JsonNode
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.fasterxml.jackson.databind.json.JsonMapper
+import com.fasterxml.jackson.databind.module.SimpleModule
 import org.specs2.mutable.Specification
 
 class JsonNodeDeserializerSpec extends BaseJacksonDeserializer("JsonNodeDeserializer") {
@@ -162,6 +159,40 @@ abstract class BaseJacksonDeserializer(val implementationName: String) extends S
       val json = s"""{ "value" : ${Long.MaxValue}0000 }"""
       readNode(baseMapper(), json).isBigInteger must beTrue
     }
+
+    "not advance the cursor excessively when re/entering tine Deserializer from a custom Deserializer on the Child of a Parent/Child class hierarchy" >> {
+      // https://github.com/lagom/lagom/issues/3241
+      val json = {
+        """
+          |{
+          |  "createdAt": 1234,
+          |  "child": {
+          |    "updatedAt": 555,
+          |    "updatedBy": "another-user"
+          |  },
+          |  "updatedBy": "some-user",
+          |  "updatedAt": 5678
+          |}
+          |""".stripMargin
+      }
+
+      val mapper   = baseMapper()
+      val jsonNode = mapper.readTree(json)
+      jsonNode.get("createdAt").asLong() must equalTo(1234L)
+      jsonNode.get("child").get("updatedAt").asLong() must equalTo(555)
+      jsonNode.get("child").get("updatedBy").asText() must equalTo("another-user")
+      jsonNode.get("updatedAt").asLong() must equalTo(5678L)
+      jsonNode.get("updatedBy").asText() must equalTo("some-user")
+
+      val actual   = mapper.readValue(json, classOf[Parent]);
+      val expected = new Parent(1234, new Child(555, "another-user"), 5678, "some-user")
+      actual.getCreatedAt must equalTo(expected.getCreatedAt)
+      actual.getChild.getUpdatedAt must equalTo(expected.getChild.getUpdatedAt)
+      actual.getChild.getUpdatedBy must equalTo(expected.getChild.getUpdatedBy)
+      actual.getUpdatedAt must equalTo(expected.getUpdatedAt)
+      actual.getUpdatedBy must equalTo(expected.getUpdatedBy)
+    }
+
   }
 
 }