@JsonProperty is ignored, sometimes

[brefsdal]

I've found a case where the JsonProperty annotation is ignored by the serializer. Sometimes. Unfortunately, I haven't found out why this occurs, seemly at random. Here is a example schema

case class MyBase(
  @JsonProperty("my_value"): myValue: Int
)
case class MyClass(
  @JsonProperty("my_name") myName: String,
  @JsonUnwrapped base: MyBase
)
case class App(
  name: String,
  @JsonUnwrapped class: MyClass
)

Most of the time I'll see 'my_name' in the output JSON string. Sometimes a machine will output myName. Once the machine starts outputting camelcase like this, it continues to do so until I restart the JVM. Here is the configuration of the ObjectMapper below

import com.fasterxml.jackson.annotation.JsonInclude
import com.fasterxml.jackson.databind.{DeserializationFeature, ObjectMapper, SerializationFeature}
import com.fasterxml.jackson.module.scala.DefaultScalaModule
import com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper

class ObjectMapperWithScalaMixin extends ObjectMapper with ScalaObjectMapper
object JacksonConfig {
  val apiObjectMapper = {
    val objectMapper = new ObjectMapperWithScalaMixin
    objectMapper.registerModule(DefaultScalaModule)
    objectMapper.setSerializationInclusion(JsonInclude.Include.NON_NULL)
    objectMapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
    objectMapper
  }

JacksonConfig.objectMapper.writeValueAsString(instanceOfApp)

[brefsdal]

I am using version 2.4.4

[brefsdal]

java version "1.7.0_65"
OpenJDK Runtime Environment (IcedTea 2.5.3) (7u71-2.5.3-0ubuntu0.14.04.1)
OpenJDK 64-Bit Server VM (build 24.65-b04, mixed mode)

[cowtowncoder]

While it is not guaranteed to change things, I would suggest latest patch release, 2.4.5, since jackson-databind has certain fixes to caches which could theoretically affect things here. But mostly just to rule out that possibility.

[brefsdal]

I've tried version 2.4.5 to no avail. I will have to remove these annotations in order to avoid this issue.

[christophercurrie]

Thanks for reporting this. I know it's terribly inconvenient. This might be a lot to ask, but are you able to reproduce this problem with an Oracle JDK? We've had other reports (#189) of issues when using OpenJDK, and I suspect this may be related.

[brefsdal]

@christophercurrie This is occurring on Oracle Hotspot JDK as well.

[christophercurrie]

Cool, thanks for checking. I'll see if I can reproduce this. You might also try Jackson 2.5.2, for which the Scala module has improved certain aspects of property detection.

[brefsdal]

Upon upgrading to Jackson 2.5.2 I find compilation errors that seem to require properties to be unwrapped require the @JsonProperty annotation as well. In referencing my example above.
@JsonUnwrapped class: MyClass
should be
@JsonUnwrapped @JsonProperty("myClass") class: MyClass

Is this expected? Even though there will never be a JSON field named "myClass".

[christophercurrie]

Not expected, and I especially wouldn't expect compilation errors. What errors is the compiler giving you?

[brefsdal]

@christophercurrie Sorry not a compilation error, but a runtime exception
com.fasterxml.jackson.databind.JsonMappingException: Could not find creator property with name 'camelcaseVariable' (in class foo.bar.Response)

[brefsdal]

Upgrading to Jackson 2.5.2 has exacerbated the problem. Even more instances of ignored @JsonProperty annotations.

[jendap]

The description here is only one of the manifestations of the problem. The exact class hierarchy and their fields do not matter. We see it happening randomly on any class in our class hierarchy. It is happening during startup - first request and stays for the process lifetime. Jackson properly walk the class and find all the fields. However it sometimes does not process @JsonProperty annotations. Are those two separate steps? Can there be a race condition between them? Or can multiple ObjectMappers hurt?

@christophercurrie would you have any tip where to look? It looks like some race condition.

[christophercurrie]

I suppose a race condition is possible, but I am more likely to suspect the fact that Java reflection, upon which Jackson is based, stopped providing consistent ordering of methods, fields, and annotations, as of Java 7. A good test of this theory would be to run the relevant code under a Java 6 JVM which, despite Oracle's discontinuation of support, is still the only JVM version upon which any guarantees are made for Scala (and which the library must still support).

Which is not to say that this issue shouldn't be fixed, just that it is both hard to reproduce and diagnose. If the issue appears in Java 6, then it's not due to reflection ordering, and will be relatively simple to locate and fix. If not, then it's almost certainly an ordering issue, and diagnosing it will likely require repeated traces with a debugger. This issue is definitely on my list to address, but I have not had the time to investigate.

[cowtowncoder]

@jendap Multiple ObjectMappers should not hurt: no state is shared between mappers, at least if constructed from scratch. If using mapper.copy() there should not be coupling either, but there would be theoretical possibility of unintentional sharing.
So the problem should not be worse with multiple mappers; if anything, use of a single mapper could possibly lead to false caching. But even then there should be something else to trigger problems: one candidate would be use of mix-ins (which could be problematic in 2.4.4/2.4.5 and 2.5.0/2.5.1, when used with Maps with POJO values).

[jendap]

I will find it. It is top priority for us. We are loosing money on it. Unfortunately I can't even after two days replicate it locally. (which is also why I suspect race condition). But we have a production heap dump. We can see anything including the data from java reflection. Comparing the BeanSerializer...
The good one:
_props[]._name is correct from the annotation
_props[]._member._annotations has the annotation
The bad one:
_props[]._name is wrong from field name
_props[]._member._annotations is empty!
... I will try to dig deeper how can the annotations be sometimes empty.

Do you think adding the JsonProperty annotation on methods as well as on fields can make difference?

We can try java6 on a few nodes. The problem is that we need G1 garbage collector to keep our SLA. And if we put it on a few nodes and restart a couple of times it is hard to say if it is not happening for sure. Currently it is probably something like 1 in 20 restarts.

[christophercurrie]

Re annotation location: without looking at your specific code is is hard to say, but maybe. If you're using case classes, and the annotations are on the constructor parameters, it might be useful to try change @JsonProperty(...) to @(JsonProperty @param @field @getter)(...); similarly with @JsonUnwrapped, if you're using it.

[jendap]

Yes, that case class with annotated fields is exactly what we use and I'm going to change it to what you have described. We will see next week after a couple of deployments if it will work. Thanks!

[jendap]

Ok, forcing the annotation to all the places - param, field, getter - works well. But it is rather ugly hack and even worse it will bite a lot more people in very bad way. We have lost a lot of money on this.

Can you tell point me to where this listing is happening for scala case class? I would like to look at it how to fix it (and replicate). Thanks!

BTW: This is what we use now. We import from our.package.JacksonAnnotations._ instead of com.faster.xml.jackson.annotations._. And our JacksonAnnotations is this:

import scala.annotation.meta.field
import scala.annotation.meta.getter
import scala.annotation.meta.param

object JacksonAnnotations {
  type JsonSerialize = com.fasterxml.jackson.databind.annotation.JsonSerialize @param @field @getter
  type JsonDeserialize = com.fasterxml.jackson.databind.annotation.JsonDeserialize @param @field @getter
  type JsonIgnore = com.fasterxml.jackson.annotation.JsonIgnore @param @field @getter
  type JsonIgnoreProperties = com.fasterxml.jackson.annotation.JsonIgnoreProperties @param @field @getter
  type JsonInclude = com.fasterxml.jackson.annotation.JsonInclude @param @field @getter
  type JsonProperty = com.fasterxml.jackson.annotation.JsonProperty @param @field @getter
  type JsonTypeInfo = com.fasterxml.jackson.annotation.JsonTypeInfo @param @field @getter
  type JsonUnwrapped = com.fasterxml.jackson.annotation.JsonUnwrapped @param @field @getter
}

[christophercurrie]

The relevant class is ScalaAnnotationIntrospector. For each field and method on a class (as reported by JVM reflection) core Jackson calls findNameForSerialization to ask it what the serialized name of the property should be. This method tries to locate the constructor parameter for the field or method, and delegates the lookup to the base implementation, which has all the information about what annotations to look for.

As far as I can tell, the triggering issue is that Java 7+ reflection will provide the methods and classes to Jackson in different orders each time the JVM is created (though it's likely that the order is stable during a particular JVM run). Simulating this failure would involve asking Jackson to generate the Annotated objects it uses internally, and then creating a test case that calls the annotation introspector with different orderings of the fields and methods, to find which ordering causes the failure condition. This might look something like:

// NB: off the cuff, might not compile

val appType = mapper.constructType(classOf[App])
val serialzationConfig = mapper.getSerializationConfig()
val ai = serializationConfig.getAnnotationIntrospector()
val beanDesc = serializationConfig.introspectDirectClassAnnotations(appType)
val annotatedClass = beanDesc.getClassInfo()
// play around with annotatedClass.fields() and annotatedClass.memberMethods()
// to find the broken order, e.g.:
for (method <- annotatedClass.memberMethods().asScala) {
  val name = ai.findNameForSerialization(method);
}

I don't really know what you might find though; I would not expect this ordering to matter, but it is the only common thread between bug reports of this nature, that they are using Java 7 or 8. That said, looking at findNameForSerialization, there might be a couple of things that could be done better:

1. Only the constructor parameter for a property is consulted. The code should probably consult the parameter, the field, the getter, and the beanGetter, if any, in that order, and use the first that it finds.
2. If no "property" is found (using the detection logic that's in BeanIntrospector), the method should delegate the base implementation. The code currently only looks for a constructor parameter (not a property more generally), and if not found, returns null, signifying failure.

I don't really know how the deficiencies above would actually result in failures, largely because looking at the constructor parameter is generally the right thing to do. It's possible that there are circumstances where the property detection logic can't find the constructor parameter when it should. The change above might be something to try if you're able to reproduce the problem. If I have time, I will also take another look at this issue.

[cowtowncoder]

For what it is worth, AnnotationIntrospector is meant to only look at annotations on specific member it is given; separate call is made for constructor parameters, fields and methods. Logic to find members and combine information into logical property is in classes POJOPropertiesCollector and POJOPropertyBuilder. However, there is some additional stitching done later on by BeanDeserializerFactory, necessary to combine information from separate creator properties (constructor parameters) into logical Creator (constructor / factory method) construction.

I am also puzzled by the problem, since ordering of methods, constructors should not any significance. But it sounds like it may well be the thing that triggers whatever the actual problem is.

[christophercurrie]

@cowtowncoder, the difficulty with what AnnotationIntrospector is "meant" to do is that it doesn't completely cover the bases for Scala. For example, as far as I am aware, an annotation on a constructor parameter is only used for deserialization, not for serialization.

This falls down in Scala because a single entry in the code generates many different JVM artifacts.

class Foo(@JsonProperty("bar") var foo: String)

This definition creates four different JVM artifacts: the constructor parameter, the field, the getter, and the setter. However, by default, the annotation appears only on the constructor parameter, resulting in incorrect serialization. Similar behavior happens for fields.

The JacksonAnnotations object that @jendap created is a mechanism for instructing the annotation to appear in multiple places. This works, but it's awkward, somewhat inflexible, and potentially surprising. The behavior as I described is an attempt to allow the annotation to appear in its default location but still work more or less "as expected." I'm open to different heuristics, if you have ideas on how it can be improved.

[cowtowncoder]

@christophercurrie Actually constructor parameter should by now be visible to serialization purposes as well; 1.9 was the version that unified handling of annotations. Before 1.9 this was not the case.
The only (?) difference between ser/deser is precedence of annotations, where serialization gives priority for getters, deserialization setters (and so forth). If annotations are not combined in this way, that is a bug.
Combination is handled by POJOPropertiesCollector / POJOPropertyBuilder, and ends up chaining annotations to be accessible through fields, setters/getters, creator parameters.

There being a bug is not completely out of the question, partly because detection and use of creator parameters is much more complicated than that of simple fields or setters/getters.

But the mechanism should combine annotations from different "accessors" (term used to denote members like fields, getters and setters, and creator parameters that act as virtual setters).

Another module that formerly did its own unification is JAXB annotations module. It may still try some of that (to try to support JAXB package level annotations), but for the most part has been simplified to do less work.

[christophercurrie]

I'll be ecstatic if that's the case, but I'm not quite sure how property naming via constructor parameter annotation could have been supported prior to Java 8 parameter name detection. A Java-based example:

class Foo {

  private String foo;

  private Foo(@JsonProperty("bar") String foo) { this.foo = foo; }

  private String getFoo() { return foo; };

  private void setFoo(String foo) { this.foo = foo; }

}

This is the Java equivalent of what the Scala code would produce. Without implicit parameter names, I would expect this produce {"foo": "value"}, which is not what is intended. With implicit parameter names, I can see that the unification might work now, so I'll have to try it out. But I don't think it would have worked before then, unless I'm missing a connecting detail.

[cowtowncoder]

Correct, this would not have worked without implicit names. Whether it works with implicit names is an interesting question, but I think it should, considering that similar approach with other accessors would work.

In above case, making things click would have required one additional annotation on either field, setter or getter, to declare renaming as "bar". If so, all the other annotations would have been unified.
Another way to put this is that naming annotations are indeed somewhat special (as well as implicit name detection) as they establish grouping of accessors. But then the remaining annotations should follow along.

[christophercurrie]

I'm beginning to believe that I may be on the wrong track; the reason is that this problem exists in 2.4.4, and the ScalaAnnotationIntrospector is 2.5+ code only. I have another theory, but I need to confirm a couple of things first. I'll get back to you asap.

[jendap]

Thank you for working on this guys!

The thing I still don't understand is that it is random with every restart. Same machine, same jvm, exactly the same jar file and classpath gives random outcome. The patch does not fix anything like that. Could it be some sort of race condition? Java 7/8 have multi-threaded class loading enabled by default. But I can't see it.

Anyway, if jackson double check the annotations on constructor parameters every time it should do the trick.

Is it just that tiny commit in jackson-databind? Or do you think more is needed? I can test it in our production.

[christophercurrie]

I'm fairly confident there is more to do, but I haven't yet found the root cause. The patch addresses separate issues that will improve annotation handling for 2.6, but I don't think that's what's at work here.

I'm starting to suspect something in the caching that happens internally to either the Scala module or core jackson. The trouble is, despite lots of examples, I have yet to observe the issue locally. It seems there might be some other environment factors influencing the behavior.

[jnadler]

Not sure this is super helpful, but I'm having the same problem and thought I'd chime in. Our auto integration tests fail intermittently when a @JsonProperty is ignored. We're using:

-version 2.5.2
-scala 2.11
-Oracle JDK 8 on MacOS

The workaround proposed by @jendap above is doing the job for us, thanks for that! Still, I obviously look forward to removing the hack when this is resolved.

[cowtowncoder]

Another note that may or may not help, but version 2.5.3 of Scala module is now available as well.

[christophercurrie]

That said, I don't know of any changes in 2.5.3 that will solve the root cause. I apologize that this continues to be an issue.

[jnadler]

No apology needed, obviously this is a hairy one or it would be fixed. Thanks :)

No luck with 2.5.3, I still need the @jendap workaround.

[jnadler]

So actually 2.5.3 seems to introduce a different issue, also intermittent, but not resolved by the workaround:

com.fasterxml.jackson.databind.JsonMappingException: Argument #1 of constructor [constructor for com.zzyzx.indexstore.Item, annotations: [null]] has no property name annotation; must have name when multiple-parameter constructor annotated as Creator

Reverted to 2.5.2 and all is good again, using the workaround.

[jendap]

The issue with Creator in 2.5.3 is likely something else.

@jnadler are you able to reproduce it on a small codebase you could share? I have spend a day or more of work trying to replicate it outside of our full production system but unsuccessfully. It should not be that hard to fix it once we can replicate it.

[jnadler]

Reproduce which? The original 2.5.2 problem or the 2.5.3 Creator issue?

I can try to boil it down to a simple project.

[jendap]

Ideally both of them! :-)

[christophercurrie]

Sorry for the very late follow up. I have reason to believe that the original issue here is resolved by FasterXML/jackson-databind#868, which is fixed in Jackson 2.6.0 and later. The Scala module is currently behind in the release cycle, having only a 2.6.1 release, while core Jackson is at 2.6.3. Please let me know if upgrading to 2.6.1 or later resolves this issue for you.

[samcoer]

do we have any update on this, we are using 2.6.2 version and getting this issue.

[coryfklein]

This has been resolved for me when upgrading to 2.7.4.

[jendap]

Neither me nor @brefsdal work in the environment where we could replicate it. So we can't confirm for sure....

But it looks like it was fixed in 2.6.1. I think we can close this. Thank you @christophercurrie!

[cowtowncoder]

Assuming this is fixed: if not, would make sense to re-file, I think, with reproduction, info on version failing.