Merge pull request FasterXML#255 from nbauernfeind/Fix#231

Fix Multiple JsonCreator Issue.
nbauernfeind · Apr 26, 2016 · 1029739 · 1029739
2 parents ca5a47f + e59164e
commit 1029739
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 16 deletions.
diff --git a/...ala/com/fasterxml/jackson/module/scala/introspect/ScalaAnnotationIntrospectorModule.scala b/...ala/com/fasterxml/jackson/module/scala/introspect/ScalaAnnotationIntrospectorModule.scala
@@ -1,15 +1,14 @@
-package com.fasterxml.jackson
-package module
-package scala
-package introspect
+package com.fasterxml.jackson.module.scala.introspect
 
-import annotation.JsonCreator
-import databind.`type`.ClassKey
-import databind.introspect._
-import databind.util.LRUMap
-import paranamer.ParanamerAnnotationIntrospector
+import com.fasterxml.jackson.annotation.JsonCreator
+import com.fasterxml.jackson.databind.`type`.ClassKey
+import com.fasterxml.jackson.databind.introspect._
+import com.fasterxml.jackson.databind.util.LRUMap
+import com.fasterxml.jackson.module.paranamer.ParanamerAnnotationIntrospector
+import com.fasterxml.jackson.module.scala.JacksonModule
+import com.fasterxml.jackson.module.scala.util.Implicits._
 
-import util.Implicits._
+import java.lang.annotation.Annotation
 
 object ScalaAnnotationIntrospector extends NopAnnotationIntrospector
 {
@@ -83,10 +82,26 @@ object ScalaAnnotationIntrospector extends NopAnnotationIntrospector
   }
 
   override def hasCreatorAnnotation(a: Annotated): Boolean = {
+    val jsonCreators = PartialFunction[Annotation, JsonCreator]({ case jc: JsonCreator => jc })
+
     a match {
       case ac: AnnotatedConstructor =>
-        isScala(ac) && _descriptorFor(ac.getDeclaringClass).
-          properties.view.flatMap(_.param).exists(_.constructor == ac.getAnnotated)
+        if (!isScala(ac)) return false
+        val annotatedFound = _descriptorFor(ac.getDeclaringClass)
+          .properties
+          .flatMap(_.param)
+          .exists(_.constructor == ac.getAnnotated)
+
+        // Ignore this annotation if there is another annotation that is actually annotated with @JsonCreator.
+        val annotatedConstructor = {
+          for (constructor <- ac.getDeclaringClass.getDeclaredConstructors;
+               annotation: JsonCreator <- constructor.getAnnotations.collect(jsonCreators) if annotation.mode() != JsonCreator.Mode.DISABLED) yield constructor
+        }.headOption
+
+        // Ignore this annotation if it is Mode.DISABLED.
+        val isDisabled = ac.getAnnotated.getAnnotations.collect(jsonCreators).exists(_.mode() == JsonCreator.Mode.DISABLED)
+
+        annotatedFound && annotatedConstructor.forall(_ == ac.getAnnotated) && !isDisabled
       case _ => false
     }
   }

diff --git a/src/test/scala/com/fasterxml/jackson/module/scala/deser/CreatorTest.scala b/src/test/scala/com/fasterxml/jackson/module/scala/deser/CreatorTest.scala
@@ -1,12 +1,9 @@
 package com.fasterxml.jackson.module.scala.deser
 
-import java.util.concurrent.TimeUnit
-
-import com.fasterxml.jackson.annotation.JsonCreator
+import com.fasterxml.jackson.annotation.{JsonCreator, JsonIgnore}
 import com.fasterxml.jackson.databind.ObjectMapper
 import org.junit.runner.RunWith
 import org.scalatest.junit.JUnitRunner
-import org.scalatest.matchers.ShouldMatchers
 
 object CreatorTest
 {
@@ -18,6 +15,29 @@ object CreatorTest
 
   class CreatorModeBean @JsonCreator(mode=JsonCreator.Mode.DELEGATING)(val s : String)
   case class CreatorModeWrapper (a: CreatorModeBean)
+
+  class AlternativeConstructor(val script: String, dummy: Int) {
+    @JsonCreator
+    def this(script: String) = {
+      this(script, 0)
+    }
+    override def equals(o: Any): Boolean = o match {
+      case ac: AlternativeConstructor => script == ac.script
+      case _ => false
+    }
+  }
+
+  case class MultipleConstructors(script: String, dummy: Int) {
+    def this(script: String) = {
+      this(script, 0)
+    }
+  }
+
+  case class MultipleConstructorsAnn @JsonCreator()(script: String, dummy: Int) {
+    def this(script: String) = {
+      this(script, 0)
+    }
+  }
 }
 
 @RunWith(classOf[JUnitRunner])
@@ -60,4 +80,28 @@ class CreatorTest extends DeserializationFixture {
     val v2 = f.readValue[ValueHolder](json)
     v2.internalValue shouldEqual 2L
   }
+
+  it should "use secondary constructor annotated with JsonCreator" in { f =>
+    val orig = new AlternativeConstructor("abc", 42)
+    val bean = f.writeValueAsString(orig)
+    bean shouldBe """{"script":"abc"}"""
+    val roundTrip = f.readValue[AlternativeConstructor](bean)
+    roundTrip shouldEqual orig
+  }
+
+  it should "use primary constructor if no JsonCreator annotation" in { f =>
+    val orig = MultipleConstructors("abc", 42)
+    val bean = f.writeValueAsString(orig)
+    bean shouldBe """{"script":"abc","dummy":42}"""
+    val roundTrip = f.readValue[MultipleConstructors](bean)
+    roundTrip shouldEqual orig
+  }
+
+  it should "use primary constructor if primary is JsonCreator annotated" in { f =>
+    val orig = MultipleConstructorsAnn("abc", 42)
+    val bean = f.writeValueAsString(orig)
+    bean shouldBe """{"script":"abc","dummy":42}"""
+    val roundTrip = f.readValue[MultipleConstructorsAnn](bean)
+    roundTrip shouldEqual orig
+  }
 }