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

@XmlElementWrapper and MapperFeature.USE_WRAPPER_NAME_AS_PROPERTY_NAME #22

Closed
mattbroekhuis opened this issue Jul 24, 2013 · 15 comments
Closed

Comments

@mattbroekhuis
Copy link

I have an object that has three List containers. They are marked up with @XmlElementWrapper annotations. I have added the configuration to include the wrapper name as the property (see first code block below). After digging through source, it's clear to me that jackson-module-jaxb-annotations is not aware of these properties.

That is fine, however in this case you cannot have multiple containers like this in a row. This module looks at the @xmlelement name instead of the wrapper, and the field list then creates one entry for all 3 of the below list containers since they contain the same abstract class. When POJOPropertitesCollector in the jackson-databind goes to rename the properties (as a result of having USE_WRAPPER_NAME_AS_PROPERTY_NAME enabled) it comes upon this entry and can't figure out what's going on. You get a JSONException with the message "Multiple value properties defined (xxx,xxx...".

There's explicit code commented out for this. I don't know where this responsibility lies, but I was able to fix this by just uncommenting the code, and moving the block about the wrapper above the @xmlelement block (as that is my preference). Would be nice if there were flags for the different ways this can be done. Also, that method is static, which makes it pretty hard to override to handle how a particular individual might want to.

 ObjectMapper jacksonObjectMapper = new ObjectMapper();
        jacksonObjectMapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
        jacksonObjectMapper.enable(MapperFeature.USE_WRAPPER_NAME_AS_PROPERTY_NAME);
        jacksonObjectMapper.enable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);

Annotated Pojo

    @XmlElementWrapper(name = "topics", required = true, namespace = "http://optimus.com/view/topic")
    @XmlElement(name = "export", namespace = "http://company.com/view/topic")
    protected List<TopicSearchExportView> topics;
    @XmlElementWrapper(name = "partners", required = true, namespace = "http://optimus.com/view/partner")
    @XmlElement(name = "export", namespace = "http://company.com/view/partner")
    protected List<PartnerSearchExportView> partners;
    @XmlElementWrapper(name = "speakers", required = true, namespace = "http://optimus.com/view/speaker")
    @XmlElement(name = "export", namespace = "http://company.com/view/speaker")
    protected List<SpeakerSearchExportView> speakers;

Method in question

private static PropertyName findJaxbPropertyName(Annotated ae, Class<?> aeType, String defaultName)
    {
        XmlAttribute attribute = ae.getAnnotation(XmlAttribute.class);
        if (attribute != null) {
            return _combineNames(attribute.name(), attribute.namespace(), defaultName);
        }
        XmlElement element = ae.getAnnotation(XmlElement.class);
        if (element != null) {
            return _combineNames(element.name(), element.namespace(), defaultName);
        }
        /* 11-Sep-2012, tatu: Wrappers should not be automatically used for renaming.
         *   At least not here (databinding core can do it if feature enabled).
         */
        /*
        XmlElementWrapper elementWrapper = ae.getAnnotation(XmlElementWrapper.class);
        if (elementWrapper != null) {
            return _combineNames(elementWrapper.name(), elementWrapper.namespace(), defaultName);
        }
        */
        XmlElementRef elementRef = ae.getAnnotation(XmlElementRef.class);
        if (elementRef != null) {
            if (!MARKER_FOR_DEFAULT.equals(elementRef.name())) {
                return _combineNames(elementRef.name(), elementRef.namespace(), defaultName);
            }
            if (aeType != null) {
                XmlRootElement rootElement = (XmlRootElement) aeType.getAnnotation(XmlRootElement.class);
                if (rootElement != null) {
                    String name = rootElement.name();
                    if (!MARKER_FOR_DEFAULT.equals(name)) {
                        return _combineNames(name, rootElement.namespace(), defaultName);
                    }
                    // Is there a namespace there to use? Probably not?
                    return new PropertyName(Introspector.decapitalize(aeType.getSimpleName()));
                }
            }
        }
        XmlValue valueInfo = ae.getAnnotation(XmlValue.class);
        if (valueInfo != null) {
            return new PropertyName("value");
        }

        return null;
    }
@cowtowncoder
Copy link
Member

Please note that although JAXB annotations module exposes metadata derived from annotations, it is really used by main jackson-databind module, as well as XML module (jackson-dataformat-xml). Wrappers are bit nasty to handle since natural bindings differ between XML and JSON: in JSON, wrappers are rarely needed (so adding them for every @XmlElementWrapper would produce sub-optimal output), whereas XML more or less requires them for Collections and Maps.

So to know whether changes fully work, you need to run unit tests for databind and XML modules -- that will likely show cases where this would not work.

@mattbroekhuis
Copy link
Author

do you have any recommendations on how to fix it? I have been stuck on this problem for quite a while now, and this seems to have resolved it, but I don't want to cause even more problems.

@mattbroekhuis
Copy link
Author

I'm wondering -- if something is a Collection, and there's a wrapper name present, why would you ever not want to use the name of the wrapper for the eventual JSON array attribute hash name? Or am I missing a bigger picture here?

@cowtowncoder
Copy link
Member

I fixed something that may or may not be related (issue #25). But one comment on fix: it can not be fixed there, since it would cause problems elsewhere: handling of property name, and wrapper name need to stay separate, to allow for combining different annotation sources (Jackson XML module has separate annotation for this, for example).

As to why not use wrapper for JSON: to me wrappers are XML construct, unrelated to JSON. One possible interpretation there would be to completely ignore any wrapper annotations, and use property name. Another is to make wrapper completely override the name. I don't know what makes most sense, and suspect it depends on how model itself came about (was XML added as an after-thought, or was that the starting point).
But most practical view at this point is that since the way things work with 2.x is established by 3 versions (2.0, 2.1, 2.2), any functional changes to this behavior must be very well argued; in absence of this, the way it works should be retained.

@cowtowncoder
Copy link
Member

Minor request: could you edit the title to indicate actual problem? As is, it is only explained in longer description. I understand the issue itself, but given large number of issues over dozens of Jackson projects, I rely on going through lits of titles, and it'd be easier if title was more descriptive.

As to problem itself: this is very difficult to fix. I dont have any timeline for the fix. Ideally someone contributes a patch that fixes the issue without breaking existing unit tests for this project, jackson-databind or jackson-dataformat-xml.

@bleshik
Copy link

bleshik commented Nov 7, 2013

I got the same problem. It looks like the problem is that you have several XmlElement annotations with the same name value. The thing is that the overriding names using wrappers is done AFTER the collecting of jackson properties and it fails because of the same name value.
Use the following configuration as a workaround

enable(MapperFeature.USE_WRAPPER_NAME_AS_PROPERTY_NAME);
setAnnotationIntrospector(new AnnotationIntrospectorPair(new JacksonAnnotationIntrospector(),
     new JaxbAnnotationIntrospector(TypeFactory.defaultInstance())
));

And use JsonProperty annotation in such cases.

@cowtowncoder
Copy link
Member

@diorcety I don't think that'll work -- you can not change AnnotationIntrospector to pass configuration that way.

@diorcety
Copy link

i'm not saying that the best way. i tested it and it works

@cowtowncoder
Copy link
Member

@diorcety I wasn't commenting on goodness of the fix in general, but just that due to compatibility & design constraints, such changes will unfortunately not be merged in Jackson code base.
But maybe something along similar lines could work.

@TuomasKiviaho
Copy link

@cowtowncoder Property naming patch that I introduced in #360 for XML also makes it possible to mimic JAXB on JSON side as well. By using the custom JAXB strategy that spares the original explicit name though patched renaming process I'm able to produce wrapper_name: {element_name:{...}} by borrowing the approach from XML side. Naturally this change would require to be put behind a feature flag and it certainly requires that the naming strategy patch would be taken into account in a form or another.

BeanPropertyWriter

   @Override
    public void serializeAsField(Object bean, JsonGenerator gen, SerializerProvider prov) throws Exception
    {
    ...
//        gen.writeFieldName(_name);
//        if (_typeSerializer == null) {
//            ser.serialize(value, gen, prov);
//        } else {
//            ser.serializeWithType(value, gen, prov, _typeSerializer);
//        }
        // Ok then; addition we want to do is to add wrapper element, and that's what happens here
        // 19-Aug-2013, tatu: ... except for those nasty 'convertValue()' calls...
        if (_wrapperName != null) {
            gen.writeFieldName(_wrapperName.getSimpleName());
            gen.writeStartObject();
        }
        gen.writeFieldName(_name);
        if (_typeSerializer == null) {
            ser.serialize(value, gen, prov);
        } else {
            ser.serializeWithType(value, gen, prov, _typeSerializer);
        }
        if (_wrapperName != null) {
            gen.writeEndObject();
        }
    }

@cowtowncoder
Copy link
Member

True. The whole question of wrapping is complicated one. So far it is only enabled for root element (for non-XML), and collections (XML). But there is certainly hope that wrapping would be allowed elsewhere too, perhaps via @JsonWrapped (see FasterXML/jackson-databind#512).
Things do get rather complex, as witnessed by existing @JsonUnwrapped functionality, although additional wrapping should be slightly simplers as there's still 1-to-1 mapping, instead of 1-to-N of @JsonUnwrapped.

@TuomasKiviaho
Copy link

This is bit off-topic, but here's one way to use @XmlElements with/without @XmlElementWrapper when utilizing the patch I mentioned above.

Currently either the @type parameter is returned or you are forced to accept a wrapper with implicit property name or name from @XmlType. My aim was to get the name from @XmlElement itself and serialize it as name/value wrapper object or just use the value or as-is when wrapping is not wanted.

Here's the scenario where wrapper object is not requested.

@XmlElements({@XmlElement(name="foo", type=Foo.class), @XmlElement(name="bar", type=Bar.class)})
Collection<?> fooAndOrBar;

@XmlElements({@XmlElement(name="foo2", type=Foo.class), @XmlElement(name="bar2", type=Bar.class)})
Object fooOrBar;
{
    "foo": {},
    "bar": {},
    "foo2": {}
}

And here the wrapper object is explicitly requested.

@XmlWrapper(name="foobar")
@XmlElements({@XmlElement(name="foo", type=Foo.class), @XmlElement(name="bar", type=Bar.class)})
Collection<?> fooAndOrBar;

@XmlWrapper(name="foo_bar")
@XmlElements({@XmlElement(name="foo", type=Foo.class), @XmlElement(name="bar", type=Bar.class)})
Object fooOrBar;
    "foobar": [  
        {  
            "foo": {}
        }, {  
            "bar": {}
        }
    ],
    "foo_bar": {  
        "foo": {}
    }

These serialization suggestions combined with the patch mentioned above would allow depending solely on the @XmlElement name while not having to expose the @XmlType names a bit like qualified names versus unqualified names in XML.

JaxbAnnotationIntrospector

    @Override
    public TypeResolverBuilder<?> findPropertyTypeResolver(MapperConfig<?> config,
            AnnotatedMember am, JavaType baseType)
    {
        /* First: @XmlElements and @XmlElementRefs only applies type for immediate property, if it
         * is NOT a structured type.
         */
        // if (baseType.isContainerType()) return null;
        TypeResolverBuilder<?> b = _typeResolverFromXmlElements(am);
        if (b != null && baseType.isContainerType()) {
            b = b.init(JsonTypeInfo.Id.NONE, null);
            XmlElementWrapper wrapper = findAnnotation(XmlElementWrapper.class, am, false, false, false);
            if (wrapper != null) {
                b.inclusion(JsonTypeInfo.As.WRAPPER_ARRAY);
            }
        }
        return b;
    }

    @Override
    public TypeResolverBuilder<?> findPropertyContentTypeResolver(MapperConfig<?> config,
            AnnotatedMember am, JavaType containerType)
    {
        /* First: let's ensure property is a container type: caller should have
         * verified but just to be sure
         */
        if (!containerType.isContainerType()) {
            throw new IllegalArgumentException("Must call method with a container type (got "+containerType+")");
        }
        TypeResolverBuilder<?> b = _typeResolverFromXmlElements(am);
        if (b != null) {
            XmlElementWrapper wrapper = findAnnotation(XmlElementWrapper.class, am, false, false, false);
            if (wrapper != null) {
                // and let's consider WRAPPER_OBJECT to be canonical inclusion method
                b = b.inclusion(JsonTypeInfo.As.WRAPPER_OBJECT);
            }
        }
        return b;
    }

    protected TypeResolverBuilder<?> _typeResolverFromXmlElements(AnnotatedMember am)
    {
        /* If simple type, @XmlElements and @XmlElementRefs are applicable.
         * Note: @XmlElement and @XmlElementRef are NOT handled here, since they
         * are handled specifically as non-polymorphic indication
         * of the actual type
         */
        XmlElements elems = findAnnotation(XmlElements.class, am, false, false, false);
        XmlElementRefs elemRefs = findAnnotation(XmlElementRefs.class, am, false, false, false);
        if (elems == null && elemRefs == null) {
            return null;
        }

        TypeResolverBuilder<?> b = new JaxbTypeResolverBuilder();
        // JAXB always uses type name as id
        b = b.init(JsonTypeInfo.Id.NAME, null);
        return b;        
    }

I am using following TypeSerializer myself and utilize the includeAs from above to know when to wrap either into array/object or not. I guess that with some adjustment, some of the already available ones might be able to do the same thing.

public class JaxbTypeResolverBuilder extends StdTypeResolverBuilder
{

    public JaxbTypeResolverBuilder()
    {
        super();
    }

    @Override
    public TypeSerializer buildTypeSerializer(SerializationConfig config,
        JavaType baseType, Collection<NamedType> subtypes)
    {
        TypeIdResolver idRes = TypeNameIdResolver.construct(config, baseType, subtypes,
            true, false);
        return new JaxbTypeSerializer(idRes, _includeAs);
    }

    @Override
    public TypeDeserializer buildTypeDeserializer(DeserializationConfig config,
        JavaType baseType, Collection<NamedType> subtypes)
    {
        TypeIdResolver idRes = TypeNameIdResolver.construct(config, baseType, subtypes,
            false, true);
        return new JaxbTypeDeserializer(baseType, idRes, _typeProperty, _typeIdVisible,
            _defaultImpl);
    }

}

public class JaxbTypeSerializer extends AsWrapperTypeSerializer
{

    private JsonTypeInfo.As _includeAs;

    public JaxbTypeSerializer(TypeIdResolver idRes, JsonTypeInfo.As includeAs)
    {
        super(idRes, null);
        this._includeAs = includeAs;
    }

    @Override
    public void writeTypePrefixForObject(Object value, JsonGenerator jgen)
        throws IOException
    {
        String typeId = idFromValue(value);
        writeCustomTypePrefixForObject(value, jgen, typeId);
    }

    @Override
    public void writeCustomTypePrefixForObject(Object value, JsonGenerator jgen,
        String typeId) throws IOException
    {
        if (jgen.canWriteTypeId())
        {
            if (typeId != null)
            {
                jgen.writeTypeId(typeId);
            }
            jgen.writeStartObject();
        }
        else if (JsonTypeInfo.As.WRAPPER_OBJECT.equals(_includeAs))
        {
            jgen.writeStartObject();
            jgen.writeObjectFieldStart(_validTypeId(typeId));
        }
        else
        {
            jgen.writeFieldName(_validTypeId(typeId));
            jgen.writeStartObject();
        }
    }

    @Override
    public void writeTypeSuffixForObject(Object value, JsonGenerator jgen)
        throws IOException
    {
        jgen.writeEndObject();
        if (!jgen.canWriteTypeId() && JsonTypeInfo.As.WRAPPER_OBJECT.equals(_includeAs))
        {
            // and then wrapper
            jgen.writeEndObject();
        }
    }

    @Override
    public void writeTypePrefixForArray(Object value, JsonGenerator jgen)
        throws IOException
    {
        String typeId = idFromValue(value);
        writeCustomTypePrefixForArray(value, jgen, typeId);
    }

    @Override
    public void writeCustomTypePrefixForArray(Object value, JsonGenerator jgen,
        String typeId) throws IOException
    {
        if (jgen.canWriteTypeId()) {
            if (typeId != null) {
                jgen.writeTypeId(typeId);
            }
            jgen.writeStartArray();
        } else if (JsonTypeInfo.As.WRAPPER_OBJECT.equals(_includeAs))
        {
            jgen.writeStartObject();
            jgen.writeArrayFieldStart(_validTypeId(typeId));
        }
    }

    @Override
    public void writeCustomTypeSuffixForObject(Object value, JsonGenerator jgen,
        String typeId) throws IOException
    {
        writeTypeSuffixForObject(value, jgen);
    }

    @Override
    public void writeTypeSuffixForArray(Object value, JsonGenerator jgen)
        throws IOException
    {
        if (JsonTypeInfo.As.WRAPPER_OBJECT.equals(_includeAs)) {
            jgen.writeEndArray();
            if (!jgen.canWriteTypeId()) {
                // and then wrapper
                jgen.writeEndObject();
            }
        }
    }

    @Override
    public void writeCustomTypeSuffixForArray(Object value, JsonGenerator jgen,
        String typeId) throws IOException
    {
        writeTypeSuffixForArray(value, jgen);
    }

}

@epabst
Copy link

epabst commented Jun 19, 2017

I ran into this same issue:

  @XmlElement
  public Inventory getInventory() {
    return inventory;
  }

  public void setInventory(Inventory inventory) {
    this.inventory = inventory;
  }

  @XmlElementWrapper(name = "assignableInventories")
  @XmlElement(
    name="inventory"
  )
  public Collection<Inventory> getAssignableInventories() {
    return assignableInventories;
  }

  public void setAssignableInventories(Collection<Inventory> assignableInventories) {
    this.assignableInventories = assignableInventories;
  }

Error: com.fasterxml.jackson.databind.JsonMappingException: Conflicting setter definitions for property "inventory": Foo#setInventory(1 params) vs Foo#setAssignableInventories(1 params)

@XmlElementWrapper is meant to be a plural noun, while an associated @xmlelement should be a singular noun so that the XML makes sense. JSON doesn't need a singular name since it can simply use a JSON array. The plural noun makes the most sense for JSON, IMO. I imagine that anyone using Jackson for JAXB with code that has @XmlElementWrapper are not pleased with the serialized form.

@cowtowncoder
Copy link
Member

Needs to be refiled at

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

if still an issue.

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

6 participants