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

JsonFormat.Shape.OBJECT ignored when class implements Map.Entry #865

Closed
DaveGrahamCA opened this issue Jul 15, 2015 · 9 comments
Closed
Milestone

Comments

@DaveGrahamCA
Copy link

The change #565 has overridden the default behavior that is defined by annotations when using JsonFormat.Shape.OBJECT. This is problematic if the Map.Entry is only one aspect of the object. Below is example code that exposes this defect. It represents an attempt to implement the pre Jackson-2.5 behavior for a Map.Entry by forcing the jackson to deal with object as a POJO. But instead of reading the annotations as if it were a POJO it appears to serialize as by using some default Map.Entry algorithm. The behavior is not as expected after annotating the class. This code use to work with Jackson 2.4.

Example files
...
package example;

import com.fasterxml.jackson.annotation.JsonFormat;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;

import java.util.Arrays;
import java.util.List;
import java.util.Map;

@JsonFormat(shape = JsonFormat.Shape.OBJECT)
public class VariableSubstitutionEntry implements Map.Entry<String, List<String>> {

    private static final String JSON_VALUE_FIELD = "value";
    private static final String JSON_KEY_FIELD = "key";
    private final SubstitutionList value = new SubstitutionList();
    private String name;

    public VariableSubstitutionEntry() {
        super();
    }

    public VariableSubstitutionEntry(String variableName, String... valueArray) {
        super();
        add(valueArray);
        name = variableName;
    }

    @JsonIgnore
    @Override
    public SubstitutionList getValue() {
        return value;
    }

    @JsonProperty(JSON_VALUE_FIELD)
    public SubstitutionList getSerializationValue() {
        return value;
    }

    @JsonIgnore
    @Override
    public List<String> setValue(List<String> value) {
        getValue().clear();
        getValue().addAll(value);
        return getValue();
    }

    @JsonIgnore
    public final void add(String... valueArray) {
        value.addAll(Arrays.asList(valueArray));
    }

    @JsonIgnore
    boolean isEmpty() {
        return value.isEmpty();
    }

    @Override
    public String toString() {
        return value.toString();
    }

    @Override
    public int hashCode() {
        return value.hashCode();
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == null) {
            return false;
        }
        if (getClass() != obj.getClass()) {
            return false;
        }
        final VariableSubstitutionEntry other = (VariableSubstitutionEntry) obj;
        if (getValue() != other.getValue() && (getValue() == null || !getValue().equals(other.getValue()))) {
            return false;
        }
        return true;
    }

    /**
     * Get the name of the variable to substitute.
     *
     * @return the name of the variable to substitute
     */
    @JsonIgnore
    @Override
    public String getKey() {
        return name;
    }

    /**
     * Set the name of the variable to substitute.
     *
     * @param name the name of the variable to substitute
     */
    @JsonIgnore
    public void setKey(String name) {
        this.name = name;
    }

    /**
     * Get the name of the variable to substitute.
     *
     * @return the name of the variable to substitute
     */
    @JsonProperty(JSON_KEY_FIELD)
    public String getSerializationName() {
        return name;
    }

    /**
     * Set the name of the variable to substitute.
     *
     * @param name the name of the variable to substitute
     */
    @JsonProperty(JSON_KEY_FIELD)
    public void setSerializationName(String name) {
        this.name = name;
    }
}

package example;

import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import java.util.ArrayList;
import java.util.Arrays;

/**
 * Implementation of
 * <code>List</code> containing variable substitution values.
 */
@JsonDeserialize(contentAs = String.class)
public class SubstitutionList extends ArrayList<String> {

    private static final long serialVersionUID = 1L;

    /**
     * Construct a new instance of
     * <code>SubstitutionList</code>.
     */
    public SubstitutionList() {
        super();
    }

    /**
     * Construct a new instance of
     * <code>SubstitutionList</code> from the given strings.
     *
     * @param strings one or more strings to add to the list
     */
    public SubstitutionList(String... strings) {
        super(Arrays.asList(strings));
    }

    /**
     * {@inheritDoc}
     */
    @Override
    public int hashCode() {
        return toString().hashCode();
    }

    /**
     * {@inheritDoc}
     */
    @Override
    public String toString() {
        StringBuilder sb = new StringBuilder("[\"");
        boolean first = true;
        for (String s : this) {
            if (first) {
                first = false;
            } else {
                sb.append("\",\"");
            }
            sb.append(s);
        }
        sb.append("\"]");
        return sb.toString();
    }

    /**
     * {@inheritDoc}
     */
    @Override
    public boolean equals(Object obj) {
        if (obj == null || getClass() != obj.getClass()) {
            return false;
        }
        final SubstitutionList other = (SubstitutionList) obj;
        return toString().equals(other.toString());
    }
}

...

@cowtowncoder
Copy link
Member

This is the problem with various add-on interfaces, including java.util.Iterator: it is difficult to tell the difference between POJOs and serialize-as-default.

In this case serializer does not look at @JsonFormat information; but even if it does, there is the problem that format refers to actual JSON representation, and not Java handling or semantics.
Since both POJO aspect and Map.Entry get serialized as JSON Object, it is not clear if the annotation could be used: what value would be used to indicate serialize usingMap.Entryinterface?

There are other possible approaches to take, to try to figure out whether what we have is "pure" Map.Entry. Perhaps checking whether any properties beside key and value may be found, and if so, use standard POJO serialization.

I am open to suggestions for handling this, but unfortunately I am not sure use of @JsonFormat works here, due to semantic fuzziness. Checking for additional properties (or, rather, that it differs from exactly two, key, value, to allow ignoring of one?) would seem like possible route.

@DaveGrahamCA
Copy link
Author

Basically in 2.4 I had the @JsonProperty tags on the actual getValue(), getKey() and setKey(String) methods. This was to fix the deserialization problem found #565. I was building the Map.Entry objects from a BUS data packet, and serializing them out for later use in a List. It was convenient when I reloaded them and into a custom map. But looking at the code, I really do not need to implement Map.Entry to do that. I can workaround this problem.

I think I can workaround, but because Map.Entry is an interface, it can be used to designate only one aspect of a multifaceted object. You need to choose whether to simply announce that use of Map.Entry as an API on a custom Serialized POJO is not supported.

Alternatively, you may need to have the system do some annotation checking. There may be people who want the new behavior for their custom Map.Entry. Perhaps they could directly specify that with JSONFormat.shape.MAP_ENTRY. This would have to be the assumed default for any Map.Entry object that is not annotated with a JSONFormat.

Just some thoughts.

@cowtowncoder
Copy link
Member

@DaveGrahamCA Yes. Apologies for this breakage, I did not quite realize that Map.Entry may be used in ways that would break with the change. And there needs to be a way to allow choosing between behaviors. The original reason for (thought to be...) improved serialization was usage that I had seen multiple times by people, of using something like List<Map.Entry<Key,Value>>, as sort of alternative to basic java.util.Map; use case for which POJO style serialization adds unwieldy extra 'key' and 'value' properties.

As to solutions, @JsonFormat would be natural thing to use in some way or form, but it does become challenging to find intuitive form to use, without adding dozens of special-purpose one-off properties.

@cowtowncoder
Copy link
Member

Thinking about this a bit more, I think that use of JsonFormat.Shape.OBJECT would actually make sense, to indicate "serialize as POJO". This could still be prevented/overloaded by using value of JsonFormat.Shape.ANY to indicate "use default", that is, more compact representation.

I may also file an issue for annotations to indicate a new choice (maybe Shape.NATURAL or Shape.NATIVE), which could be usable here as well; but until then, ANY seems like a reasonable override.

@cbutterfield
Copy link

I just got burned by this change too. My class that Implements Map.Entry is now (as of 2.5.0) serialized without the attribute names, BUT the Jackson deserializer still wants the names.

Are there any workarounds (the work with the 2.5.0 version, not a later one) that will allow:

  1. Force the serializer to use the OLD format, i.e. {'key':'key1', 'value':'value1'} ???
  2. Force the deserializer to use the NEW format, i.e. {'key1':'value1'}

Any help would be appreciated.

@cowtowncoder
Copy link
Member

Unfortunately there is no functionality to change the behavior at this point, except by using custom serializer/deserializer for Map.Entry.

Further, for 2.5 only bug fixes are allowed, no functional changes, so adding new features (or enum values, like Features) is unfortunately not an option.

@cbutterfield
Copy link

Okay. For the sake of others, here is a workaround I used to force deserialization to use the OLD format (for my class named MapEntry):

// WA-1: force old serialization format via custom serializer
@JsonSerialize(using = MapEntry.MySerializer.class)
public class MapEntry extends ReflectiveHashAndEquals implements
        Map.Entry<String, String> {

... existing implementation code omitted ...

    /**
     *  Serializer using the OLD (pre Jackson 2.5.0) serialization  format for any
     * object implementing @{link Map.Entry}.
     *
     * <pre>
     * OLD Format: {"key":"some-key","value":"some-value"}
     * NEW Format: {"some-key":"some-value"}
     * <pre/>
     */
    public static class MySerializer extends JsonSerializer<MapEntry> {
        @Override
        public void serialize(MapEntry value, JsonGenerator gen,
                SerializerProvider serializers) throws IOException,
                JsonProcessingException {
            gen.writeStartObject();
            gen.writeStringField("key", value.getKey());
            gen.writeStringField("value", value.getValue());
            gen.writeEndObject();
        }
    }
}

@cbutterfield
Copy link

And this an alternate workaround that forces deserialization to use the NEW format (and NOT the OLD) for my class named MapEntry:

public class MapEntry extends ReflectiveHashAndEquals implements
        Map.Entry<String, String> {

... existing implementation code omitted ...

   DELETE THE FOLLOWING OLD format deserializing constructor. (I wanted to show it
   in strikeout text but couldn't figure out how to do that)
    @JsonCreator
    public MapEntry(@JsonProperty("key") String key,  @JsonProperty("value") String value) {
        verifyNotNull(key, "key");
        verifyNotNull(value, "value");
        mKey = key;
        mValue = value;
    }

    // WA-2: support new serialization format via more focused JsonCreator
    // NOTE: keep this disabled while using WA-1, they are incompatible
    // @JsonCreator
    public MapEntry(Map.Entry<String, String> entry) {
        verifyNotNull(entry.getKey(), "key");
        verifyNotNull(entry.getValue(), "value");
        mKey = entry.getKey();
        mValue = entry.getValue();
    }
}

cowtowncoder added a commit that referenced this issue Oct 17, 2016
@cowtowncoder
Copy link
Member

Support for class annotation has been added for 2.9, as requested.

One thing that does not work yet, however, is use on property. I hope to get that working, although that is much more difficult to support since it would require somehow constructing BeanSerializer dynamically.
I will close this issue and create a new one for remaining work.

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

No branches or pull requests

3 participants