Skip to content
This repository has been archived by the owner on Jan 22, 2019. It is now read-only.

@XmlElement type and @XmlJavaTypeAdapter annotations #26

Open
barelnir opened this issue Nov 21, 2013 · 9 comments
Open

@XmlElement type and @XmlJavaTypeAdapter annotations #26

barelnir opened this issue Nov 21, 2013 · 9 comments

Comments

@barelnir
Copy link

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:

FasterXML/jackson-jaxrs-providers#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
@TuomasKiviaho
Copy link

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.

@cowtowncoder
Copy link
Member

@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
Copy link
Member

@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
Copy link
Member

@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.

@TuomasKiviaho
Copy link

@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.

@barelnir
Copy link
Author

@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?

@cowtowncoder
Copy link
Member

@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).

@TuomasKiviaho
Copy link

@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;
    }

@cowtowncoder
Copy link
Member

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants