jackson-module-jaxb-annotations icon indicating copy to clipboard operation
jackson-module-jaxb-annotations copied to clipboard

@XmlElement type and @XmlJavaTypeAdapter annotations

Open barelnir opened this issue 12 years ago • 9 comments

Hi.

On one of my classes I got this annotation for JAXB:

@XmlElement(required = true, type = String.class, nillable = true)
public CustomObject getName()

in the package-info.java I defined an adapter to convert string to CustomObject and CustomObject to string

@javax.xml.bind.annotation.adapters.XmlJavaTypeAdapters({ @javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter(type = CustomObject.class, value = CustomObjectAdapter.class) })

When i tried to get JSON string from object using:

objectMapper.writeValueAsString(myObject);

I get an exception: Caused by: java.lang.IllegalArgumentException: Illegal concrete-type annotation for method 'getName': class java.lang.String not a super-type of (declared) class com.copmany.CustomObject

for more information:

https://github.com/FasterXML/jackson-jaxrs-providers/issues/36

I found in method: public Class<?> findSerializationType(Annotated a) In file: JaxbAnnotationInspector.java that you dont look for @XmlJavaTypeAdapter at the package level at all... but this isn't right....

the @XmlJavaTypeAdapter in the package level always need to be checked because it very important for custom objects that returns many times. ( like custom UUIDs class)

Full stack trace:

com.fasterxml.jackson.databind.JsonMappingException: Illegal concrete-type annotation for method 'getName': class java.lang.String not a super-type of (declared) class com.copmany.CustomObject
at com.fasterxml.jackson.databind.SerializerProvider.createAndCacheUntypedSerializer(SerializerProvider.java:1042)
at com.fasterxml.jackson.databind.SerializerProvider.findValueSerializer(SerializerProvider.java:445)
at com.fasterxml.jackson.databind.SerializerProvider.findTypedValueSerializer(SerializerProvider.java:599)
at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.serializeValue(DefaultSerializerProvider.java:93)
at com.fasterxml.jackson.databind.ObjectMapper.configAndWriteValue(ObjectMapper.java:2811)
at com.fasterxml.jackson.databind.ObjectMapper.writeValueAsString(ObjectMapper.java:2268)
at com.copmany.EntryPoint.getString(EntryPoint.java:217)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:88)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:55)
at java.lang.reflect.Method.invoke(Method.java:613)
at org.apache.cxf.service.invoker.AbstractInvoker.performInvocation(AbstractInvoker.java:180)
at org.apache.cxf.service.invoker.AbstractInvoker.invoke(AbstractInvoker.java:96)
at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:194)
at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:102)
at org.apache.cxf.interceptor.ServiceInvokerInterceptor$1.run(ServiceInvokerInterceptor.java:58)
at org.apache.cxf.interceptor.ServiceInvokerInterceptor.handleMessage(ServiceInvokerInterceptor.java:94)
at org.apache.cxf.phase.PhaseInterceptorChain.doIntercept(PhaseInterceptorChain.java:271)
at org.apache.cxf.transport.ChainInitiationObserver.onMessage(ChainInitiationObserver.java:121)
at org.apache.cxf.transport.http.AbstractHTTPDestination.invoke(AbstractHTTPDestination.java:239)
at org.apache.cxf.transport.servlet.ServletController.invokeDestination(ServletController.java:223)
at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:203)
at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:137)
at org.apache.cxf.transport.servlet.CXFNonSpringServlet.invoke(CXFNonSpringServlet.java:158)
at org.apache.cxf.transport.servlet.AbstractHTTPServlet.handleRequest(AbstractHTTPServlet.java:243)
at org.apache.cxf.transport.servlet.AbstractHTTPServlet.doPost(AbstractHTTPServlet.java:163)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:755)
at org.apache.cxf.transport.servlet.AbstractHTTPServlet.service(AbstractHTTPServlet.java:219)
at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:681)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1452)
at org.eclipse.jetty.servlets.CrossOriginFilter.handle(CrossOriginFilter.java:248)
at org.eclipse.jetty.servlets.CrossOriginFilter.doFilter(CrossOriginFilter.java:211)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1423)
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:450)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:138)
at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:540)
at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:213)
at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1083)
at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:379)
at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:175)
at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1017)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:136)
at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:258)
at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:109)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
at org.eclipse.jetty.rewrite.handler.RewriteHandler.handle(RewriteHandler.java:317)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
at org.eclipse.jetty.server.Server.handle(Server.java:445)
at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:260)
at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:225)
at org.eclipse.jetty.io.AbstractConnection$ReadCallback.run(AbstractConnection.java:358)
at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:596)
at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:527)
at java.lang.Thread.run(Thread.java:780)
Caused by: java.lang.IllegalArgumentException: Illegal concrete-type annotation for method 'getName': class java.lang.String not a super-type of (declared) class com.copmany.CustomObject
at com.fasterxml.jackson.databind.ser.PropertyBuilder.findSerializationType(PropertyBuilder.java:187)
at com.fasterxml.jackson.databind.ser.PropertyBuilder.buildWriter(PropertyBuilder.java:80)
at com.fasterxml.jackson.databind.ser.BeanSerializerFactory.constructWriter(BeanSerializerFactory.java:758)
at com.fasterxml.jackson.databind.ser.BeanSerializerFactory.findBeanProperties(BeanSerializerFactory.java:590)
at com.fasterxml.jackson.databind.ser.BeanSerializerFactory.constructBeanSerializer(BeanSerializerFactory.java:377)
at com.fasterxml.jackson.databind.ser.BeanSerializerFactory.findBeanSerializer(BeanSerializerFactory.java:272)
at com.fasterxml.jackson.databind.ser.BeanSerializerFactory.createSerializer2(BeanSerializerFactory.java:217)
at com.fasterxml.jackson.databind.ser.BeanSerializerFactory.createSerializer(BeanSerializerFactory.java:152)
at com.fasterxml.jackson.databind.SerializerProvider.createUntypedSerializer(SerializerProvider.java:1077)
at com.fasterxml.jackson.databind.SerializerProvider.createAndCacheUntypedSerializer(SerializerProvider.java:1037)
... 53 more

barelnir avatar Nov 21 '13 09:11 barelnir

To summarize the problem JaxbAnnotationIntrospector.findAdapter supports @XmlJavaTypeAdapter only for members and not for classes. I commented out findAdapterForClass section from the beginning of the method and it started working for me.

PS. I noticed also that instead of using best match approach the code that handles @XmlJavaTypeAdapters uses first match approach which might result to false positives when there are more than one matching adapters in the list.

TuomasKiviaho avatar Dec 29 '14 08:12 TuomasKiviaho

@TuomasKiviaho Thank you for digging into this!

You are probably wrt first match. I can have a look to see if it'd be easy enough to try to find a better match. I'll also try to figure out how adapter registration is incomplete.

cowtowncoder avatar Dec 29 '14 18:12 cowtowncoder

@dn2k-dev I guess I do not quite follow what type property here means, and why it would actually be included. It'd seem that since there is an adapter declared for its nominal type, such additional information would not be useful.

cowtowncoder avatar Dec 29 '14 21:12 cowtowncoder

@TuomasKiviaho Actually, I am not quite sure what you mean there. @XmlJavaTypeAdapter is handled both for classes (AnnotatedClass) and properties, at least from perspective of annotation introspector.

cowtowncoder avatar Dec 29 '14 21:12 cowtowncoder

@cowtowncoder Oops. I meant to say @XmlJavaTypeAdapter declaration AT PACKAGE LEVEL is only supported for members and not for classes because findAdapterForClass only considers annotations that have been declared directly on the class itself. @XmlJavaTypeAdapters is not supported even that much.

findAdapterForClass seems to me as a remnant of previous refactoring(s) because findAnnotation does the same thing but more thoroughly.

TuomasKiviaho avatar Dec 30 '14 07:12 TuomasKiviaho

@cowtowncoder you are right the type is redundant or not needed here at all. but still I don't understand if @TuomasKiviaho found an issue or explaining that it by design?

barelnir avatar Dec 30 '14 07:12 barelnir

@dn2k-dev I am sure there are remaining issues, and handling of package-level annotations is bit of a pain as it is something Jackson was never designed to support (but it is something JAXB heavily promotes).

Having said all that, it seems to me that it should be possible to support all of this within JaxbAnnotationIntrospector, by rewriting logic for finding adapters, as well as resolving issues with type handling. I think my understanding of type property was incorrect here, and perhaps JAXB actually defines that to be used as intermediate type to use with adapeters. At least in case of multiple annotations.

@TuomasKiviaho I think that solving the problem with non-optimal adapter would be relatively easy to solve, if there was a simple test case to reproduce it. I had a look at code itself, and resolution is relatively compact, but does indeed stop at first match. Plus I wasn't 100% sure whether test used was even correct (isAssignableFrom() is easy to get wrong way around... esp. since ser/deser have different requirements).

cowtowncoder avatar Dec 30 '14 17:12 cowtowncoder

@cowtowncoder Adapter consideration is missing also from findSubTypes. Here are proposals for both cases that I currently utilize. AnnotatedClass is build without mixin and forSerialization is forced due to non-ideal parameterization of the method.

StdSubtypeResolver is the only place where the method is used so it might be wise to provide MapperConfig and change the name FindSerializationSubTypes. NamedType could return AnnotatedClass instead of raw type because next steps repeat the creation again.

JaxbAnnotationIntrospector

    private XmlAdapter<Object,Object> findAdapter(Annotated am, boolean forSerialization,
            Class<?> type)
    {
        // First of all, are we looking for annotations for class?
//        if (am instanceof AnnotatedClass) {
//            return findAdapterForClass((AnnotatedClass) am, forSerialization);
//        }
        ...
    }

    @Override
    public List<NamedType> findSubtypes(Annotated a)
    {
        // No package/superclass defaulting (only used with fields, methods)
        XmlElements elems = findAnnotation(XmlElements.class, a, false, false, false);
        ArrayList<NamedType> result = null;
        if (elems != null) {
            result = new ArrayList<NamedType>();
            for (XmlElement elem : elems.value()) {
                String name = elem.name();
                if (MARKER_FOR_DEFAULT.equals(name)) name = null;
                Class<?> type = elem.type();
                AnnotatedClass ac = AnnotatedClass.constructWithoutSuperTypes(type, this, null);
                XmlAdapter<Object, Object> adapter = findAdapter(ac, true, type);
                if (adapter != null) {
                    type = findAdapterBoundType(adapter);
                }
                result.add(new NamedType(type, name));
            }
        } else {
            XmlElementRefs elemRefs = findAnnotation(XmlElementRefs.class, a, false, false, false);
            if (elemRefs != null) {
                result = new ArrayList<NamedType>();
                for (XmlElementRef elemRef : elemRefs.value()) {
                    Class<?> refType = elemRef.type();
                    // only good for types other than JAXBElement (which is XML based)
                    if (!JAXBElement.class.isAssignableFrom(refType)) {
                        AnnotatedClass ac = AnnotatedClass.constructWithoutSuperTypes(refType, this, null);
                        XmlAdapter<Object, Object> adapter = findAdapter(ac, true, refType);
                        if (adapter != null) {
                            refType = findAdapterBoundType(adapter);
                        }
                        // [JACKSON-253] first consider explicit name declaration
                        String name = elemRef.name();
                        if (name == null || MARKER_FOR_DEFAULT.equals(name)) {
                            XmlRootElement rootElement = (XmlRootElement) refType.getAnnotation(XmlRootElement.class);
                            if (rootElement != null) {
                                name = rootElement.name();
                            }
                        }
                        if (name == null || MARKER_FOR_DEFAULT.equals(name)) {
                            name = Introspector.decapitalize(refType.getSimpleName());
                        }
                        result.add(new NamedType(refType, name));
                    }
                }
            }
        }

        // [Issue#1] check @XmlSeeAlso as well.
        /* 17-Aug-2012, tatu:  But wait! For structured type, what we really is
         *    value (content) type!
         *    If code below does not make full (or any) sense, do not despair -- it
         *    is wrong. Yet it works. The call sequence before we get here is mangled,
         *    its logic twisted... but as Dire Straits put it: "That ain't working --
         *    that's The Way You Do It!"
         */
        XmlSeeAlso ann = a.getAnnotation(XmlSeeAlso.class);
        if (ann != null) {
            if (result == null) {
                result = new ArrayList<NamedType>();
            }
            for (Class<?> cls : ann.value()) {
                result.add(new NamedType(cls));
            }
        }
        return result;
    }

TuomasKiviaho avatar Jun 12 '15 10:06 TuomasKiviaho

@TuomasKiviaho since projected has moved, this would need to move to:

https://github.com/FasterXML/jackson-modules-base/issues

Do you think you could re-file it over there? The reason I am asking you is that I think you have better understanding of the situation and could give better up-to-date description than I can.

cowtowncoder avatar May 19 '18 17:05 cowtowncoder