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

JsonScalaEnumeration annotation not picked up when using a Mixin #399

Closed
seanrohead opened this issue Feb 22, 2019 · 12 comments
Closed

JsonScalaEnumeration annotation not picked up when using a Mixin #399

seanrohead opened this issue Feb 22, 2019 · 12 comments
Assignees
Milestone

Comments

@seanrohead
Copy link

I'm using version 2.9.8 and found that the JsonScalaEnumeration annotation does not get picked up when using a Mixin. Here is code that demonstrates the behavior.

object TestEnum extends Enumeration {
  type TestEnum = Value
  val Value1, Value2 = Value
}

class TestEnumClass extends TypeReference[TestEnum.type]

case class TestObject1(@JsonScalaEnumeration(classOf[TestEnumClass]) field: TestEnum)

case class TestObject2(field: TestEnum)

abstract class TestObject2Mixin {
  @JsonScalaEnumeration(classOf[TestEnumClass]) val field: TestEnum = null
}

object JacksonBug extends App {
  val mapper = new ObjectMapper() with ScalaObjectMapper
  mapper.registerModule(DefaultScalaModule)
  mapper.addMixIn(classOf[TestObject2], classOf[TestObject2Mixin])

  val json = """{"field": "Value1"}"""

  val obj1 = mapper.readValue[TestObject1](json)
  println(s"With annotation: $obj1")

  val obj2 = mapper.readValue[TestObject2](json)
  println(s"With mixin: $obj2")
}

OUTPUT

With annotation: TestObject1(Value1)
Exception in thread "main" com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot deserialize instance of `scala.Enumeration$Value` out of VALUE_STRING token
 at [Source: (String)"{"field": "Value1"}"; line: 1, column: 11] (through reference chain: TestObject2["field"])
	at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63)
	at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1343)
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1139)
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1093)
	at com.fasterxml.jackson.module.scala.deser.EnumerationDeserializer.deserialize(EnumerationDeserializerModule.scala:24)
	at com.fasterxml.jackson.module.scala.deser.EnumerationDeserializer.deserialize(EnumerationDeserializerModule.scala:21)
	at com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:530)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeWithErrorWrapping(BeanDeserializer.java:528)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:417)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1287)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:326)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:159)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4013)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3042)
	at com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper.readValue(ScalaObjectMapper.scala:191)
	at com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper.readValue$(ScalaObjectMapper.scala:190)
	at JacksonBug$$anon$1.readValue(JacksonBug.scala:23)
	at JacksonBug$.delayedEndpoint$JacksonBug$1(JacksonBug.scala:32)
	at JacksonBug$delayedInit$body.apply(JacksonBug.scala:22)
	at scala.Function0.apply$mcV$sp(Function0.scala:34)
	at scala.Function0.apply$mcV$sp$(Function0.scala:34)
	at scala.runtime.AbstractFunction0.apply$mcV$sp(AbstractFunction0.scala:12)
	at scala.App.$anonfun$main$1$adapted(App.scala:76)
	at scala.collection.immutable.List.foreach(List.scala:388)
	at scala.App.main(App.scala:76)
	at scala.App.main$(App.scala:74)
	at JacksonBug$.main(JacksonBug.scala:22)
	at JacksonBug.main(JacksonBug.scala)

@pjfanning
Copy link
Member

@cowtowncoder I've confirmed that this is still an issue. Mixins do work for scala classes. Is it possible that this issue could be because JsonScalaEnumeration is a jackson-module-scala annotation but that the mixin code is in jackson-databind and therefore only supports the core annotations?

https://github.com/FasterXML/jackson-docs/wiki/JacksonMixInAnnotations#usage-notes seems to say that only the core jackson annotations and jaxb annotations are supported

@cowtowncoder
Copy link
Member

Hmmh. I don't think this is true (i.e. I think all annotations are actually merged), but adding @JacksonAnnotation on all Scala annotation definitions would make sense if that has not yet been done. This is what is used to recognize Jackson annotations.
But JAXB annotations do not have this meta-annotation and I don't recall reports that mix-ins wouldn't work for them.

I suspect this could be more due to member declaration of mix-in not matching that of target for some reason (Scala compiler might add some decorations perhaps?).

@pjfanning
Copy link
Member

I had another look and the scala annotation is being picked up. The issue happens in com.fasterxml.jackson.module.scala.deser.EnumerationDeserializer.

It seems that in mixin case that the parser events are not appearing as expected. EnumerationDeserializer works ok normally.

In the mixin case, EnumerationDeserializer gets the VALUE_STRING token when it is expecting a START_OBJECT token.

@cowtowncoder
Copy link
Member

That is odd as Enums typically would be JSON Strings (or perhaps integers)? Assuming Scala Enumeration is similar to Java Enums. Mixins also should not have effect on token streams.

@pjfanning
Copy link
Member

https://github.com/FasterXML/jackson-module-scala/blob/master/src/main/scala/com/fasterxml/jackson/module/scala/deser/EnumerationDeserializerModule.scala#L13

The bean-property is based on the target class and not on the mixin class. This leads to the code in L13 returning this instead of the AnnotatedEnumerationDeserializer (which does handle the VALUE_STRING token correctly).

pjfanning pushed a commit that referenced this issue Sep 10, 2019
@pjfanning
Copy link
Member

I suspect the issue is in https://github.com/FasterXML/jackson-module-scala/blob/master/src/main/scala/com/fasterxml/jackson/module/scala/introspect/ScalaAnnotationIntrospectorModule.scala#L59 - I have a test case and the JsonScalaEnumeration annotation makes it to this point but the code seems to go back to the raw class at this point (instead of using the AnnotatedMember)

@cowtowncoder
Copy link
Member

Uh. That code is... all kinds of wrong, if I understand it correctly. For some odd reason it starts with accessor, but instead of using it like it should, then goes and introspects declaring class (which is not necessarily class we are introspecting; may be super-class/-interface) and does something from there. I wish I knew what is the point there... I assume there may have been a reason at some point, but there are code comments to help.

@pjfanning
Copy link
Member

I got the mixin tests to pass (https://github.com/FasterXML/jackson-module-scala/blob/master/src/test/scala/com/fasterxml/jackson/module/scala/experimental/EnumMixinTest.scala) - hopefully this fix should be in 2.10.0 release.

@pjfanning pjfanning self-assigned this Sep 11, 2019
@cowtowncoder
Copy link
Member

Excellent! We still have time until 2.10.0 closes -- my best estimate is that it'd be weekend after next (Sep 21, 2019) at earliest, and I will try to communicate this over jackson-dev to try to see what can go in, esp. regarding API changes (bug fixes are easy in patches, API changes not).

@cowtowncoder
Copy link
Member

@pjfanning One related thing: it would be great to have release-notes for Scala module, too. I know there is release-notes/VERSION.md, but it doesn't seem to be used. That could be revived, or something else created: most other modules use release-notes/VERSION and release-notes/VERSION-2.x. Author(s) of module can decide what makes most sense.

In addition, there are global release notes, and for 2.10:

https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.10

and I can fill that from per-repo one if necessary. But it is probably best if module author(s) can update that wiki: it should be as simple as just copy-paste comments from repo release notes; which in turn should usually be copying Github issue title next to issue id.

The main benefit of combined release notes is, I think, that

  1. Users have a place for quick look at "what's in 2.10", and
  2. Authors have similar source for overviews: it makes my writing of upcoming "what's new in Jackson 2.10" much easier too :)

@pjfanning
Copy link
Member

@cowtowncoder I merged some notes into https://github.com/FasterXML/jackson-module-scala/blob/master/release-notes/VERSION.md - I'll see if I can cobble together some of the missing release notes over time

@cowtowncoder
Copy link
Member

Excellent, thank you!

@cowtowncoder cowtowncoder added this to the 2.9.10 milestone Sep 27, 2019
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

3 participants