fixed the ordering here: https://github.com/apache/incubator-johnzon/commit/c0ebe9d2acbd8515a6169e7c1c80ecbe3ae67988#diff-2f240336715f1d8e5f8d4223ee4a6e19R207
(line 207 in Mappings.java -> change HashMap to LinkedHashMap, this maker iterator() ordering reliable) will clean up the testcase .. On Tue, Nov 4, 2014 at 11:59 PM, Romain Manni-Bucau <[email protected]> wrote: > Maybe too late but is the commit in MapperTest intended? to avoid > ordering issue it was using try/catch pattern, now it seems this is > broken > > > Romain Manni-Bucau > @rmannibucau > http://www.tomitribe.com > http://rmannibucau.wordpress.com > https://github.com/rmannibucau > > > > ---------- Forwarded message ---------- > From: <[email protected]> > Date: 2014-11-04 22:53 GMT+00:00 > Subject: [2/2] git commit: JOHNZON-21 (renamed setter/getter to > method), implemented basic null and empty array handling (allow to > have nulls in the serialization, allow to have/skip empty arrays in > the serialization) > To: [email protected] > > > JOHNZON-21 (renamed setter/getter to method), implemented basic null > and empty array handling (allow to have nulls in the serialization, > allow to have/skip empty arrays in the serialization) > > > Project: http://git-wip-us.apache.org/repos/asf/incubator-johnzon/repo > Commit: > http://git-wip-us.apache.org/repos/asf/incubator-johnzon/commit/c0ebe9d2 > Tree: http://git-wip-us.apache.org/repos/asf/incubator-johnzon/tree/c0ebe9d2 > Diff: http://git-wip-us.apache.org/repos/asf/incubator-johnzon/diff/c0ebe9d2 > > Branch: refs/heads/master > Commit: c0ebe9d2acbd8515a6169e7c1c80ecbe3ae67988 > Parents: 7176fde > Author: Hendrik Saly <[email protected]> > Authored: Tue Nov 4 23:53:29 2014 +0100 > Committer: Hendrik Saly <[email protected]> > Committed: Tue Nov 4 23:53:29 2014 +0100 > > ---------------------------------------------------------------------- > .../java/org/apache/johnzon/mapper/Mapper.java | 37 +++- > .../apache/johnzon/mapper/MapperBuilder.java | 14 +- > .../johnzon/mapper/reflection/Mappings.java | 18 +- > .../org/apache/johnzon/mapper/MapperTest.java | 2 +- > .../org/apache/johnzon/mapper/NullTest.java | 195 +++++++++++++++++++ > 5 files changed, 248 insertions(+), 18 deletions(-) > ---------------------------------------------------------------------- > > > http://git-wip-us.apache.org/repos/asf/incubator-johnzon/blob/c0ebe9d2/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/Mapper.java > ---------------------------------------------------------------------- > diff --git > a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/Mapper.java > b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/Mapper.java > index eadaac9..77dd96c 100644 > --- a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/Mapper.java > +++ b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/Mapper.java > @@ -74,16 +74,20 @@ public class Mapper { > protected final boolean close; > protected final ConcurrentMap<Type, Converter<?>> converters; > protected final int version; > + protected boolean skipNull; > + protected boolean skipEmptyArray; > > public Mapper(final JsonReaderFactory readerFactory, final > JsonGeneratorFactory generatorFactory, > final boolean doClose, final Map<Class<?>, > Converter<?>> converters, > - final int version, final Comparator<String> > attributeOrder) { > + final int version, final Comparator<String> > attributeOrder, boolean skipNull, boolean skipEmptyArray) { > this.readerFactory = readerFactory; > this.generatorFactory = generatorFactory; > this.close = doClose; > this.converters = new ConcurrentHashMap<Type, > Converter<?>>(converters); > this.version = version; > this.mappings = new Mappings(attributeOrder); > + this.skipNull = skipNull; > + this.skipEmptyArray = skipEmptyArray; > } > > private static JsonGenerator writePrimitives(final JsonGenerator > generator, final Object value) { > @@ -301,11 +305,20 @@ public class Mapper { > JsonGenerator generator = gen; > for (final Map.Entry<String, Mappings.Getter> getterEntry : > classMapping.getters.entrySet()) { > final Mappings.Getter getter = getterEntry.getValue(); > - final Object value = getter.setter.invoke(object); > - if (value == null || (getter.version >= 0 && version >= > getter.version)) { > + final Object value = getter.method.invoke(object); > + if (getter.version >= 0 && version >= getter.version) { > continue; > } > > + if (value == null) { > + if(skipNull) { > + continue; > + } else { > + gen.writeNull(getterEntry.getKey()); > + continue; > + } > + } > + > generator = writeValue(generator, value.getClass(), > getter.primitive, getter.array, > getter.collection, getter.map, > @@ -319,11 +332,17 @@ public class Mapper { > JsonGenerator generator = gen; > for (final Map.Entry<?, ?> entry : ((Map<?, ?>) object).entrySet()) { > final Object value = entry.getValue(); > + final Object key = entry.getKey(); > + > if (value == null) { > - continue; > + if(skipNull) { > + continue; > + } else { > + gen.writeNull(key == null ? "null" : key.toString()); > + continue; > + } > } > > - final Object key = entry.getKey(); > final Class<?> valueClass = value.getClass(); > final boolean primitive = Mappings.isPrimitive(valueClass); > final boolean clazz = mappings.getClassMapping(valueClass) != > null; > @@ -342,8 +361,12 @@ public class Mapper { > final boolean collection, final > boolean map, > final String key, final Object > value) throws InvocationTargetException, IllegalAccessException { > if (array) { > - JsonGenerator gen = generator.writeStartArray(key); > final int length = Array.getLength(value); > + if(length == 0 && skipEmptyArray) { > + return generator; > + } > + > + JsonGenerator gen = generator.writeStartArray(key); > for (int i = 0; i < length; i++) { > gen = writeItem(gen, Array.get(value, i)); > } > @@ -511,7 +534,7 @@ public class Mapper { > for (final Map.Entry<String, Mappings.Setter> setter : > classMapping.setters.entrySet()) { > final JsonValue jsonValue = object.get(setter.getKey()); > final Mappings.Setter value = setter.getValue(); > - final Method setterMethod = value.setter; > + final Method setterMethod = value.method; > final Object convertedValue = value.converter == null? > toObject(jsonValue, value.paramType) : > jsonValue.getValueType() == ValueType.STRING ? > > value.converter.fromString(JsonString.class.cast(jsonValue).getString()): > > http://git-wip-us.apache.org/repos/asf/incubator-johnzon/blob/c0ebe9d2/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MapperBuilder.java > ---------------------------------------------------------------------- > diff --git > a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MapperBuilder.java > b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MapperBuilder.java > index e31aab6..b20fdbd 100644 > --- > a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MapperBuilder.java > +++ > b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MapperBuilder.java > @@ -78,6 +78,8 @@ public class MapperBuilder { > private boolean doCloseOnStreams = false; > private int version = -1; > private Comparator<String> attributeOrder = null; > + private boolean skipNull = true; > + private boolean skipEmptyArray = false; > private final Map<Class<?>, Converter<?>> converters = new > HashMap<Class<?>, Converter<?>>(DEFAULT_CONVERTERS); > > public Mapper build() { > @@ -92,7 +94,7 @@ public class MapperBuilder { > } > } > > - return new Mapper(readerFactory, generatorFactory, > doCloseOnStreams, converters, version, attributeOrder); > + return new Mapper(readerFactory, generatorFactory, > doCloseOnStreams, converters, version, attributeOrder, skipNull, > skipEmptyArray); > } > > public MapperBuilder setAttributeOrder(final Comparator<String> > attributeOrder) { > @@ -124,4 +126,14 @@ public class MapperBuilder { > this.version = version; > return this; > } > + > + public MapperBuilder setSkipNull(final boolean skipNull) { > + this.skipNull = skipNull; > + return this; > + } > + > + public MapperBuilder setSkipEmptyArray(final boolean skipEmptyArray) { > + this.skipEmptyArray = skipEmptyArray; > + return this; > + } > } > > http://git-wip-us.apache.org/repos/asf/incubator-johnzon/blob/c0ebe9d2/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/reflection/Mappings.java > ---------------------------------------------------------------------- > diff --git > a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/reflection/Mappings.java > b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/reflection/Mappings.java > index 5baae6b..d0e2d07 100644 > --- > a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/reflection/Mappings.java > +++ > b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/reflection/Mappings.java > @@ -33,7 +33,7 @@ import java.math.BigDecimal; > import java.math.BigInteger; > import java.util.Collection; > import java.util.Comparator; > -import java.util.HashMap; > +import java.util.LinkedHashMap; > import java.util.List; > import java.util.Map; > import java.util.Queue; > @@ -70,7 +70,7 @@ public class Mappings { > } > > public static class Getter { > - public final Method setter; > + public final Method method; > public final int version; > public final Converter<Object> converter; > public final boolean primitive; > @@ -78,11 +78,11 @@ public class Mappings { > public final boolean map; > public final boolean collection; > > - public Getter(final Method setter, > + public Getter(final Method method, > final boolean primitive, final boolean array, > final boolean collection, final boolean map, > final Converter<Object> converter, final int version) { > - this.setter = setter; > + this.method = method; > this.converter = converter; > this.version = version; > this.array = array; > @@ -93,14 +93,14 @@ public class Mappings { > } > > public static class Setter { > - public final Method setter; > + public final Method method; > public final int version; > public final Type paramType; > public final Converter<?> converter; > public final boolean primitive; > > - public Setter(final Method setter, final boolean primitive, > final Type paramType, final Converter<?> converter, final int version) > { > - this.setter = setter; > + public Setter(final Method method, final boolean primitive, > final Type paramType, final Converter<?> converter, final int version) > { > + this.method = method; > this.paramType = paramType; > this.converter = converter; > this.version = version; > @@ -204,9 +204,9 @@ public class Mappings { > private ClassMapping createClassMapping(final Class<?> clazz) { > try { > final Map<String, Getter> getters = fieldOrdering != null ? > - new TreeMap<String, Getter>(fieldOrdering) : new > HashMap<String, Getter>(); > + new TreeMap<String, Getter>(fieldOrdering) : new > LinkedHashMap<String, Getter>(); > final Map<String, Setter> setters = fieldOrdering != null ? > - new TreeMap<String, Setter>(fieldOrdering) : new > HashMap<String, Setter>(); > + new TreeMap<String, Setter>(fieldOrdering) : new > LinkedHashMap<String, Setter>(); > > final PropertyDescriptor[] propertyDescriptors = > Introspector.getBeanInfo(clazz).getPropertyDescriptors(); > for (final PropertyDescriptor descriptor : propertyDescriptors) { > > http://git-wip-us.apache.org/repos/asf/incubator-johnzon/blob/c0ebe9d2/johnzon-mapper/src/test/java/org/apache/johnzon/mapper/MapperTest.java > ---------------------------------------------------------------------- > diff --git > a/johnzon-mapper/src/test/java/org/apache/johnzon/mapper/MapperTest.java > b/johnzon-mapper/src/test/java/org/apache/johnzon/mapper/MapperTest.java > index 59711d8..26ff435 100644 > --- a/johnzon-mapper/src/test/java/org/apache/johnzon/mapper/MapperTest.java > +++ b/johnzon-mapper/src/test/java/org/apache/johnzon/mapper/MapperTest.java > @@ -169,7 +169,7 @@ public class MapperTest { > final ByteArrayOutputStream baos = new ByteArrayOutputStream(); > new MapperBuilder().build().writeArray(new Pair[] { new > Pair(1, "a"), new Pair(2, "b") }, baos); > try { > - > assertEquals("[{\"s\":\"a\",\"i\":1},{\"s\":\"b\",\"i\":2}]", new > String(baos.toByteArray())); > + > assertEquals("[{\"i\":1,\"s\":\"a\"},{\"i\":2,\"s\":\"b\"}]", new > String(baos.toByteArray())); > } catch (final AssertionFailedError afe) { // a bit lazy > but to avoid field ordering on java > 6 > > assertEquals("[{\"i\":1,\"s\":\"a\"},{\"i\":2,\"s\":\"b\"}]", new > String(baos.toByteArray())); > } > > http://git-wip-us.apache.org/repos/asf/incubator-johnzon/blob/c0ebe9d2/johnzon-mapper/src/test/java/org/apache/johnzon/mapper/NullTest.java > ---------------------------------------------------------------------- > diff --git > a/johnzon-mapper/src/test/java/org/apache/johnzon/mapper/NullTest.java > b/johnzon-mapper/src/test/java/org/apache/johnzon/mapper/NullTest.java > new file mode 100644 > index 0000000..54354a2 > --- /dev/null > +++ b/johnzon-mapper/src/test/java/org/apache/johnzon/mapper/NullTest.java > @@ -0,0 +1,195 @@ > +/* > + * Licensed to the Apache Software Foundation (ASF) under one > + * or more contributor license agreements. See the NOTICE file > + * distributed with this work for additional information > + * regarding copyright ownership. The ASF licenses this file > + * to you under the Apache License, Version 2.0 (the > + * "License"); you may not use this file except in compliance > + * with the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, > + * software distributed under the License is distributed on an > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > + * KIND, either express or implied. See the License for the > + * specific language governing permissions and limitations > + * under the License. > + */ > +package org.apache.johnzon.mapper; > + > +import static org.junit.Assert.assertEquals; > + > +import java.io.StringWriter; > +import java.util.Comparator; > +import java.util.LinkedHashMap; > +import java.util.Map; > + > +import org.junit.Test; > + > +public class NullTest { > + > + @Test > + public void writeNullObjectDefault() { > + final StringWriter writer = new StringWriter(); > + new MapperBuilder().build().writeObject(new NullObject(), writer); > + assertEquals("{\"emptyArray\":[]}", writer.toString()); > + } > + > + @Test > + public void writeNullObjectDefaultMap() { > + final StringWriter writer = new StringWriter(); > + > + final String expectedJson = > "{\"map\":{\"key1\":\"val1\",\"null\":\"val3\"}}"; > + > + final Comparator<String> attributeOrder = new Comparator<String>() { > + @Override > + public int compare(String o1, String o2) { > + > + if(o1 == null) { > + o1 = "null"; > + } > + > + if(o2 == null) { > + o2 = "null"; > + } > + > + return expectedJson.indexOf(o1) - expectedJson.indexOf(o2); > + } > + }; > + > + new > MapperBuilder().setAttributeOrder(attributeOrder).build().writeObject(new > NullObjectWithMap(), writer); > + assertEquals(expectedJson, writer.toString()); > + } > + > + @Test > + public void writeNullObjectDefaultMapAllowNull() { > + final StringWriter writer = new StringWriter(); > + > + final String expectedJson = > "{\"map\":{\"key1\":\"val1\",\"key2\":null,\"null\":\"val3\"}}"; > + > + final Comparator<String> attributeOrder = new Comparator<String>() { > + @Override > + public int compare(String o1, String o2) { > + > + if(o1 == null) { > + o1 = "null"; > + } > + > + if(o2 == null) { > + o2 = "null"; > + } > + > + return expectedJson.indexOf(o1) - expectedJson.indexOf(o2); > + } > + }; > + > + new > MapperBuilder().setSkipNull(false).setAttributeOrder(attributeOrder).build().writeObject(new > NullObjectWithMap(), writer); > + assertEquals(expectedJson, writer.toString()); > + } > + > + @Test > + public void writeNullObjectAllowNull() { > + final StringWriter writer = new StringWriter(); > + > + final String expectedJson = > "{\"stringIsnull\":null,\"integerIsnull\":null,\"nullArray\":null,\"emptyArray\":[]}"; > + > + final Comparator<String> attributeOrder = new Comparator<String>() { > + @Override > + public int compare(final String o1, final String o2) { > + return expectedJson.indexOf(o1) - expectedJson.indexOf(o2); > + } > + }; > + > + new > MapperBuilder().setSkipNull(false).setAttributeOrder(attributeOrder).build().writeObject(new > NullObject(), writer); > + assertEquals(expectedJson, writer.toString()); > + } > + > + @Test > + public void writeNullObjectAllowNullSkipEmptyArray() { > + final StringWriter writer = new StringWriter(); > + > + final String expectedJson = > "{\"stringIsnull\":null,\"integerIsnull\":null,\"nullArray\":null}"; > + > + final Comparator<String> attributeOrder = new Comparator<String>() { > + @Override > + public int compare(final String o1, final String o2) { > + return expectedJson.indexOf(o1) - expectedJson.indexOf(o2); > + } > + }; > + > + new > MapperBuilder().setSkipNull(false).setSkipEmptyArray(true).setAttributeOrder(attributeOrder).build() > + .writeObject(new NullObject(), writer); > + assertEquals(expectedJson, writer.toString()); > + } > + > + @Test > + public void writeNullObjectSkipAll() { > + final StringWriter writer = new StringWriter(); > + new > MapperBuilder().setSkipNull(true).setSkipEmptyArray(true).build().writeObject(new > NullObject(), writer); > + assertEquals("{}", writer.toString()); > + } > + > + public static class NullObjectWithMap { > + > + private Map map = new LinkedHashMap(); > + > + NullObjectWithMap() { > + super(); > + map.put("key1", "val1"); > + map.put("key2", null); > + map.put(null, "val3"); > + } > + > + public Map getMap() { > + return map; > + } > + > + public void setMap(final Map map) { > + this.map = map; > + } > + > + } > + > + public static class NullObject { > + > + private String stringIsnull; > + private Integer integerIsnull; > + private String[] nullArray; > + private String[] emptyArray = new String[0]; > + > + public String[] getNullArray() { > + return nullArray; > + } > + > + public void setNullArray(final String[] nullArray) { > + this.nullArray = nullArray; > + } > + > + public String[] getEmptyArray() { > + return emptyArray; > + } > + > + public void setEmptyArray(final String[] emptyArray) { > + this.emptyArray = emptyArray; > + } > + > + public String getStringIsnull() { > + return stringIsnull; > + } > + > + public void setStringIsnull(final String stringIsnull) { > + this.stringIsnull = stringIsnull; > + } > + > + public Integer getIntegerIsnull() { > + return integerIsnull; > + } > + > + public void setIntegerIsnull(final Integer integerIsnull) { > + this.integerIsnull = integerIsnull; > + } > + > + } > + > +} -- Hendrik Saly (salyh, hendrikdev22) @hendrikdev22 PGP: 0x22D7F6EC
