Hmm hashmap is better for runtime. Test should be fix rather than src/main.
Please revert or proove me im wrong Le 5 nov. 2014 20:10, "Hendrik Dev" <[email protected]> a écrit : > java 6,7,8 build is green -> > https://travis-ci.org/salyh/incubator-johnzon/builds/40102683 > > On Wed, Nov 5, 2014 at 8:53 PM, Hendrik Dev <[email protected]> > wrote: > > 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 > > > > -- > Hendrik Saly (salyh, hendrikdev22) > @hendrikdev22 > PGP: 0x22D7F6EC >
