Skip to content

Annotations should be validated (like reporting error for using @JsonWrapped on array) #41

Closed
cowwoc opened this Issue Nov 14, 2012 · 13 comments

2 participants

@cowwoc
cowwoc commented Nov 14, 2012

I'd like to transform:

{
  "singleKey":
  [
    "value1",
    "value2"
  ]
}

into:

[
    "value1",
    "value2"
]

I tried using @JsonUnwrapped on the List in question but Jackson ignores it silently. I assume this is what is meant by the Javadoc: "can not unwrap JSON arrays using this mechanism". I have three questions:

  1. Is this what is meant by the Javadoc?
  2. If so, can we add this feature?
  3. Is there a workaround I can use?
@cowwoc
cowwoc commented Nov 14, 2012

In all honesty I'm just using this feature to work around a Jersey/Java shortcoming.

I am declaring a class to denote the HTTP entity because Jersey's client API doesn't play nice with generics. It's easier to read: client.get(MyEntity.class) than client.get(new GenericType<List<URI>>(){});

Can you think of an easier/better way to do this?

@cowwoc
cowwoc commented Nov 14, 2012

I am using the following workaround for now:

private static final GenericType<List<URI>> listOfUri = new GenericType<List<URI>>()
{
};

[...]
List<URI> = client.get(listOfUri);

It's less ideal than what I had in mind because the user no longer gets a compiler error if I change the entity representation in the future, but at least it's easy to read.

@cowtowncoder
FasterXML, LLC member

You can only unwrap properties of a JSON Object, to be contained in another JSON Object. In your case the problem is (I am guessing, since there's no class definition) that you want sort of double unwrapping.

As to Jersey client, I don't know -- Jersey (and RESTeasy, SpringMVC) have problem that there is no JDK type that can be used to pass enough generic type information (with context) to make root-level generic type values work correctly. Jackson could allow that with JavaType (which does provide all the information needed), but none of frameworks uses it.

Your workaround looks valid to me, and I can't think of better ways; although you can use TypeFactory to programmatically construct these types instead of GenericType. Benefit is that this is not compile-time static, but fully dynamic, that is, can compose nested generic types using type-erased Class instances.
Not sure if that would help here, but it would allow avoiding generating all permutations.

@cowtowncoder
FasterXML, LLC member

Reading through SO question, I realized that you may be looking for @JsonValue here: it does replace a JSON Object with value of one of its properties. And for reverse, @JsonCreator works when used as delegate (i.e. single argument, without @JsonProperty).

@cowwoc
cowwoc commented Nov 15, 2012

Tatu,

I have a bunch of related questions:

  1. Shouldn't Jackson always throw an exception when it runs across an annotation it cannot apply? I don't think it should ever silently ignore an annotation.

  2. How do you use JavaType? Like this? TypeFactory.fromType(List.class, TypeFactory.fromType(URI.class)) Does this result in Class<List<URI>>? If so, couldn't you improve the API by having JavaType implement Type and then replace fromType(Type type, JavaType context) by fromType(Type type, Type... typeParameters)? Now users can simply write: TypeFactory.fromType(List.class, URI.class). This also allows multiple type parameters and each type parameter may be simple or compound. What do you think?

  3. I don't think you can use TypeFactory with Jersey. If I use TypeFactory to construct a Class and pass it into Jersey, will it be able to get at the generic type parameters?

@cowtowncoder
FasterXML, LLC member
  1. I agree in that it would be good to throw an exception, but due to the way this annotation is used this would pretty much require all serializers/deserializers to check for it, for the sole purpose of throwing exception. There is no central place where it would be handled. So it may not be practical to try full coverage.
    But if you find a suitable place to add the check I would definitely prefer exception over quiet ignoral.

  2. Usage can be like that, although there is also a convenience method for 'fromType(List.class, URI.class)' which should do what you want. java.lang.reflect.Type itself is not (alas!) suitable general replacement for JavaType (see http://www.cowtowncoder.com/blog/archives/2010/12/entry_436.html for longer explanation), so I try to minimize its use. It's one of poorly designed things in JDK, which is very unfortunate given how important type detection is...

  3. No, Jersey does not recognize it, and JAX-RS really does not have good story on how types should be passed. Beyond hard to use api, java.lang.reflect.Type is inadequate due to lack of context; and JAX-RS API does not pass the required context info (like Method or Class that type is used in, required to resolve type variables). This is unfortunately similarly missing piece from all Java REST frameworks, AFAIK. It is also one of few areas where a future JSR could improve things.

Now, as to JavaType implementing java.lang.reflect.Type... actually, maybe it is not a bad idea actually. Why? Because even thought it would not (IMO) make that much sense to try to fully implement it for access itself (given flaws of its API), what it COULD be useful for is simple "hide JavaType as java.lang.reflect.Type", so that it could be passed via Jersey.

And given this, Jackson JAX-RS provider could indeed check if given Type might be JavaType... and if so, just cast it, and get access.

... so, your suggestion is actually good. :-D
This requires a new iseeu for databind etc, but is doable.

@cowwoc
cowwoc commented Nov 16, 2012
  1. Perhaps you could expose a validateAnnotations() method once in some super-class and all serializers/deserializers would invoke it. This method would apply to all annotations (not just @JsonUnwrapped) so you cost/benefit woudl improve.

  2. I just took a look at ClassMate's TypeResolver. My point was that by offering a method that takes Type... arguments you allow users to mix Class and ResolvedType arguments which leads to the best of both worlds from a usability point of view. If they have a simple type they just pass in the Class. If they have a complex type, they pass in the resolved type. Is that something you could add to ClassMate?

  3. Did you or anyone else file a bug report against JAX-RS to that affect? Is this something we can overcome with Jersey-specific extensions until JAX-RS 3.0? I ask because the door is quickly closing for Jersey 2.0 changes. There is a good chance we can push this change in if we talk to the right people.

  4. Regarding the idea of tunneling JavaType behind a Type, good idea. I just meant the point I mentioned in #2 but this works too! :)

@cowtowncoder
FasterXML, LLC member
  1. I am all for unifying things if possible -- I just remember that annotation processing is very much scattered, due to improvements to createContextual(). But we could improve it for at least some set of types, like arrays/collections. A issue for improving validation of annotations would be good. It also goes with the related question of how to do more centralized POJO property introspection, handling; POJOPropertyCollector is aiming to do more of that, to help.

  2. Long-term, replacing existing type handling with ClassMate functionality would be great. And iff JavaType (or for Classmate, ResolvedType) would implement java.lang.reflect.Type, we could then accept j.l.r.t.

  3. I have not filed a bug, since I hadn't thought of a good incremental improvement to tackle it. Paul Sandoz (ex-lead of Jersey) was aware of issue (he left some comments on my blog article), but I don't know if Jersey folks otherwise know this. It's a good point wrt 2.0.

  4. Yeah, I think use as tag interface would at least allow "masquerading" JavaType/ResolvedType to be passed through interfaces. There are problems for non-Jackson libraries (since this is "unknown" Type implementation), but at least it could work for Jackson.

@cowwoc
cowwoc commented Nov 16, 2012

Okay. Did you file an issue for each of these items, or should I? :) I want to make sure there is a follow-up.

@cowtowncoder
FasterXML, LLC member

Nope. I generally let ones with itch do this. :)

@cowwoc
cowwoc commented Nov 16, 2012

For point 3 please see and comment on http://java.net/jira/browse/JERSEY-1581

I believe point 4 is covered by the new issues I filed above.

@cowtowncoder
FasterXML, LLC member

Ok, this issue has morphed in multiple directions; but since construction of JavaType is handled under others, I edited title a bit. Can leave this open for general wish to do stricter validation of annotations, at least for now.

@cowtowncoder
FasterXML, LLC member
@cowwoc cowwoc referenced this issue in FasterXML/jackson-databind Apr 17, 2013
Closed

Centralize annotation validation #209

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.