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

Annotated returns RawType in place of Generic Type in 2.7.x #1248

Closed
apjoseph opened this issue May 22, 2016 · 13 comments
Closed

Annotated returns RawType in place of Generic Type in 2.7.x #1248

apjoseph opened this issue May 22, 2016 · 13 comments
Milestone

Comments

@apjoseph
Copy link

When calling getGenericType() on com.fasterxml.jackson.databind.introspect.Annotated, 2.7.x returns the raw type instead!. I am aware that this method has been deprecated, but the current functionality results in very hard to debug issues -like a LinkedHashMap being substituted for a List<String>, resulting in an error when the getter is called.

This method should either be removed from 2.7 -or should function correctly until it is removed. Treating the type as List is obviously not going to do anything remotely similar to what the user expects and will wreak havoc on any libraries that are maintaining backwards compatibility with older Jackson versions.

@cowtowncoder
Copy link
Member

Agreed. PR would be welcome; either way, should be fixed for 2.7.5

@cowtowncoder cowtowncoder added this to the 2.7.5 milestone May 23, 2016
@cowtowncoder
Copy link
Member

Added overrides for AnnotatedMethod and AnnotatedField which do have generic type information available; other impls do not (AnnotatedParameter theoretically could perhaps be improved to find it too).

@apjoseph
Copy link
Author

Unfortunately this is still failing on AnnotatedParameter which is where I encountered the issue. This previously functioned in 2.6.4 so existing libs utilizing that functionality will still be broken.

@cowtowncoder
Copy link
Member

@apjoseph yes, some libraries may use this method, but I don't know how widely it is used. Doesn't really help if one you are using is of course.
I can try to see how easy it'd be to wire AnnotatedParameter case; this depends on a few things.

@cowtowncoder cowtowncoder reopened this May 23, 2016
cowtowncoder added a commit that referenced this issue May 25, 2016
@cowtowncoder
Copy link
Member

One more change, should now have generic type available for AnnotatedParameter.

@apjoseph
Copy link
Author

@cowtowncoder Looks like it is still broken with 1e5d349. Below is a simple test case that demonstrates the problem. In case you are wondering why the example seems so contrived, the actual issue occurs in Rosetta - a JDBI module that uses Jackson to automatically convert JDBC ResultSets into POJOs and bind POJOS to SQL statements, which requires a special introspector and corresponding annotation for cases when the DB column is actually a stored as json in the database.

package com.immeseration.intropection;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target({ElementType.FIELD, ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)
public @interface GenericDeserialization {
}
package com.immeseration.intropection;

import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.introspect.Annotated;
import com.fasterxml.jackson.databind.introspect.AnnotatedMethod;
import com.fasterxml.jackson.databind.introspect.NopAnnotationIntrospector;


public class GenericAnnotationIntrospector extends NopAnnotationIntrospector {
  private static final long serialVersionUID = 1L;

  @Override
  @SuppressWarnings("unchecked")
  public JsonDeserializer<?> findDeserializer(Annotated a) {
    GenericDeserialization storedAsJson = a.getAnnotation(GenericDeserialization.class);
    if (storedAsJson == null) {
      return null;
    }

    else {

      if (a instanceof AnnotatedMethod) {
        a = ((AnnotatedMethod) a).getParameter(0);
      }

      return new GenericDeserializer<>(a.getRawType(),a.getGenericType());

    }
  }

}
package com.immeseration.intropection;

import com.fasterxml.jackson.core.Base64Variants;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.deser.std.StdScalarDeserializer;

import java.io.IOException;
import java.lang.reflect.Type;
import java.nio.charset.StandardCharsets;

public class GenericDeserializer<T> extends StdScalarDeserializer<T> {
  private static final long serialVersionUID = 1L;

  private final Type type;

  public GenericDeserializer(Class<T> vc, Type type) {
    super(vc);
    this.type = type;
  }

  @Override
  public T deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException {
    JavaType javaType = ctxt.getTypeFactory().constructType(type);
    ObjectMapper mapper = (ObjectMapper) jp.getCodec();

    if (jp.getCurrentToken() == JsonToken.VALUE_STRING) {
      return deserialize(mapper, jp.getText(), javaType);
    } else if (jp.getCurrentToken() == JsonToken.VALUE_EMBEDDED_OBJECT) {
      String json = new String(jp.getBinaryValue(Base64Variants.getDefaultVariant()), StandardCharsets.UTF_8);
      return deserialize(mapper, json, javaType);
    } else if(jp.getCurrentToken() == JsonToken.START_OBJECT || jp.getCurrentToken() == JsonToken.START_ARRAY) {
      return mapper.readValue(jp, javaType);
    } else {
      throw ctxt.mappingException("Expected JSON String");
    }
  }

  private T deserialize(ObjectMapper mapper, String json, JavaType javaType) throws IOException {
    if (json == null || json.isEmpty()) {
      return null;
    }

    else {
      return mapper.readValue(json, javaType);
    }

  }
}
package com.immeseration.intropection;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.module.SimpleModule;
import org.junit.Test;

import java.io.IOException;

public class TestIntrospection {

    @Test
    public void testIntrospection() throws IOException {

        ObjectMapper om = new ObjectMapper().registerModule(new SimpleModule() {

            @Override
            public void setupModule(SetupContext context) {
                super.setupModule(context);
                context.insertAnnotationIntrospector(new GenericAnnotationIntrospector());
            }
        });

        String testStr = "{\"innocuousList\" : [{\"integer\": 1},{\"integer\": 2},{\"integer\": 3}]}";
        ListContainer lc = om.readValue(testStr,ListContainer.class);


        for (BasicObject basicObject : lc.getInnocuousList()) {
            basicObject.toString();
        }

    }
}
package com.immeseration.intropection;

import java.util.List;

public class ListContainer {

    @GenericDeserialization
    public List<BasicObject> innocuousList;

    public List<BasicObject> getInnocuousList() {
        return innocuousList;
    }

    public void setInnocuousList(List<BasicObject> innocuousList) {
        this.innocuousList = innocuousList;
    }
}
package com.immeseration.intropection;

public class BasicObject {

    private Integer integer;

    public Integer getInteger() {
        return integer;
    }

    public void setInteger(Integer integer) {
        this.integer = integer;
    }
}

The test errors out with a nasty

java.lang.ClassCastException: java.util.LinkedHashMap cannot be cast to com.immeseration.intropection.BasicObject

in 2.7.x versions, but works fine in all earlier versions

@cowtowncoder
Copy link
Member

Ok, at this point I will need a simpler reproduction, and perhaps a fix. I unfortunately do not have time to work on this.

apjoseph added a commit to apjoseph/jackson-databind that referenced this issue May 25, 2016
apjoseph added a commit to apjoseph/jackson-databind that referenced this issue May 25, 2016
@apjoseph
Copy link
Author

This turned out to be a relatively simple fix after further investigation. AnnotatedWithParams was returning the raw type instead of the JavaType. Fixed the issue in #1250

@jhaber
Copy link

jhaber commented Jul 19, 2016

Thanks for the fix, I'm the maintainer of Rosetta and I'll try to get it off any methods that are now deprecated before the next release

@cowtowncoder
Copy link
Member

@jhaber Great, thank you. Type resolution changes ended up much more disruptive than I hoped (although risk was there, it is a fundamental big change). Deprecated methods still exist and work in 2.8, to give bit more time, and there may not be hurry to remove them. But their behavior isn't necessarily as correct as new versions (by necessity).

@jhaber
Copy link

jhaber commented Jul 19, 2016

Out of curiosity, it seems like the deprecated method has signature:
public Type getGenericType();
and the new method has a narrower signature:
public JavaType getType();

Since the new return type is narrower, couldn't getGenericType just delegate to getType and preserve behavioral compatibility?

@cowtowncoder
Copy link
Member

@jhaber That is a possibility in theory, but the problem is that java.lang.reflect.Type is kind of useless marker/tag type, and all real usage has to cast it to actual type like Class<?> or such. So any existing code is likely to check what kind of type it is, and since JavaType is not returned by JDK, and was never returned by this method, it is likely that code wouldn't work, even when signature didn't change.

@jhaber
Copy link

jhaber commented Jul 19, 2016

I see, thanks for that context

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

Successfully merging a pull request may close this issue.

3 participants