From 47b46e489fe5f593092ff48f011a8dfd1cdab8c2 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 14 Apr 2017 14:02:19 -0700 Subject: [PATCH] Fix #50 --- .../com/fasterxml/jackson/jr/ob/JSON.java | 4 +- .../jackson/jr/ob/impl/AnyReader.java | 40 +++++++++++------ .../jackson/jr/ob/impl/MapBuilder.java | 24 +++++++--- .../jackson/jr/ob/impl/MapReader.java | 44 +++++++++++-------- .../jackson/jr/ob/ReadFeaturesTest.java | 13 ++++++ release-notes/VERSION | 5 +++ 6 files changed, 89 insertions(+), 41 deletions(-) diff --git a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/JSON.java b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/JSON.java index 42fb03af..60166072 100644 --- a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/JSON.java +++ b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/JSON.java @@ -326,8 +326,8 @@ public final boolean isDisabled(int flags) { public final boolean isEnabled(int flags) { return (flags & _mask) != 0; } - } - + } + // Important: has to come before 'std' instance, since it refers to it private final static int DEFAULT_FEATURES = Feature.defaults(); diff --git a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/AnyReader.java b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/AnyReader.java index cd3dc095..c81c6b98 100644 --- a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/AnyReader.java +++ b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/AnyReader.java @@ -100,10 +100,14 @@ public Map readFromObject(JSONReader r, JsonParser p, MapBuilder } // but then it's loop-de-loop - b = b.start().put(key, value); - do { - b = b.put(fromKey(p.getCurrentName()), read(r, p)); - } while (p.nextValue() != JsonToken.END_OBJECT); + try { + b = b.start().put(key, value); + do { + b = b.put(fromKey(p.getCurrentName()), read(r, p)); + } while (p.nextValue() != JsonToken.END_OBJECT); + } catch (IllegalArgumentException e) { + throw JSONObjectException.from(p, e.getMessage()); + } return b.build(); } @@ -117,11 +121,15 @@ public Object[] readArrayFromArray(JSONReader r, JsonParser p, CollectionBuilder if (p.nextToken() == JsonToken.END_ARRAY) { return b.singletonArray(value); } - b = b.start().add(value); - do { - b = b.add(read(r, p)); - } while (p.nextToken() != JsonToken.END_ARRAY); - return b.buildArray(); + try { + b = b.start().add(value); + do { + b = b.add(read(r, p)); + } while (p.nextToken() != JsonToken.END_ARRAY); + return b.buildArray(); + } catch (IllegalArgumentException e) { + throw JSONObjectException.from(p, e.getMessage()); + } } public Collection readCollectionFromArray(JSONReader r, JsonParser p, CollectionBuilder b) throws IOException @@ -133,11 +141,15 @@ public Collection readCollectionFromArray(JSONReader r, JsonParser p, Co if (p.nextToken() == JsonToken.END_ARRAY) { return b.singletonCollection(value); } - b = b.start().add(value); - do { - b = b.add(read(r, p)); - } while (p.nextToken() != JsonToken.END_ARRAY); - return b.buildCollection(); + try { + b = b.start().add(value); + do { + b = b.add(read(r, p)); + } while (p.nextToken() != JsonToken.END_ARRAY); + return b.buildCollection(); + } catch (IllegalArgumentException e) { + throw JSONObjectException.from(p, e.getMessage()); + } } /* diff --git a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/MapBuilder.java b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/MapBuilder.java index 2043de69..3a5d7cd6 100644 --- a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/MapBuilder.java +++ b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/MapBuilder.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.jr.ob.JSON; import com.fasterxml.jackson.jr.ob.JSON.Feature; +import com.fasterxml.jackson.jr.ob.JSONObjectException; /** * Helper class that is used for constructing {@link java.util.Map}s @@ -18,7 +19,8 @@ public abstract class MapBuilder { protected final int _features; - + protected final boolean _checkDups; + /** * Optional {@link Map} implementation class, used when specific * implementation is desired. @@ -27,6 +29,7 @@ public abstract class MapBuilder protected MapBuilder(int features, Class type) { _features = features; + _checkDups = JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS.isEnabled(features); _mapType = type; } @@ -67,7 +70,7 @@ public final boolean isEnabled(JSON.Feature f) { * * which assumes that a builder has been constructed with {@link #newBuilder} */ - public Map emptyMap() { + public Map emptyMap() throws JSONObjectException { return start().build(); } @@ -81,7 +84,7 @@ public Map emptyMap() { * start().put(key, value).build(); * */ - public Map singletonMap(Object key, Object value) { + public Map singletonMap(Object key, Object value) throws JSONObjectException { return start().put(key, value).build(); } @@ -102,7 +105,7 @@ public Map singletonMap(Object key, Object value) { public static class Default extends MapBuilder { protected Map _current; - + protected Default(int features, Class type) { super(features, type); } @@ -116,7 +119,7 @@ public MapBuilder newBuilder(int features) { public MapBuilder newBuilder(Class mapImpl) { return new Default(_features, mapImpl); } - + @Override public MapBuilder start() { // If this builder is "busy", create a new one... @@ -126,7 +129,7 @@ public MapBuilder start() { _current = _map(12); return this; } - + @Override public Map build() { Map result = _current; @@ -136,10 +139,17 @@ public Map build() { @Override public MapBuilder put(Object key, Object value) { + if (_checkDups) { + if (_current.containsKey(key)) { + // 14-Apr-2017, tatu: Note that choice of `IllegalArgumentException` is arbitrary + // but not random: caller catches and re-packages it to give context + throw new IllegalArgumentException("Duplicate key (key '"+key+"')"); + } + } _current.put(key, value); return this; } - + @Override public Map emptyMap() { if ((_mapType == null) && isEnabled(Feature.READ_ONLY)) { diff --git a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/MapReader.java b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/MapReader.java index 3675ffb0..d6b3c527 100644 --- a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/MapReader.java +++ b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/MapReader.java @@ -46,19 +46,23 @@ public Object readNext(JSONReader r, JsonParser p) throws IOException { } throw _reportProblem(p); } - b = b.start().put(propName0, value); - while (true) { - b = b.put(propName, _valueReader.readNext(r, p)); - propName = p.nextFieldName(); - if (propName == null) { - if (p.hasToken(JsonToken.END_OBJECT)) { - return b.build(); + try { + b = b.start().put(propName0, value); + while (true) { + b = b.put(propName, _valueReader.readNext(r, p)); + propName = p.nextFieldName(); + if (propName == null) { + if (p.hasToken(JsonToken.END_OBJECT)) { + return b.build(); + } + throw _reportProblem(p); } - throw _reportProblem(p); } + } catch (IllegalArgumentException e) { + throw JSONObjectException.from(p, e.getMessage()); } } - + @Override public Object read(JSONReader r, JsonParser p) throws IOException { MapBuilder b = r._mapBuilder(_mapType); @@ -77,20 +81,24 @@ public Object read(JSONReader r, JsonParser p) throws IOException { } throw _reportProblem(p); } - b = b.start().put(propName0, value); - while (true) { - b = b.put(propName, _valueReader.readNext(r, p)); - propName = p.nextFieldName(); - if (propName == null) { - if (p.hasToken(JsonToken.END_OBJECT)) { - return b.build(); + try { + b = b.start().put(propName0, value); + while (true) { + b = b.put(propName, _valueReader.readNext(r, p)); + propName = p.nextFieldName(); + if (propName == null) { + if (p.hasToken(JsonToken.END_OBJECT)) { + return b.build(); + } + throw _reportProblem(p); } - throw _reportProblem(p); } + } catch (IllegalArgumentException e) { + throw JSONObjectException.from(p, e.getMessage()); } } - protected IOException _reportProblem(JsonParser p) { + protected JSONObjectException _reportProblem(JsonParser p) { return JSONObjectException.from(p, "Unexpected token "+p.getCurrentToken()+"; should get FIELD_NAME or END_OBJECT"); } } diff --git a/jr-objects/src/test/java/com/fasterxml/jackson/jr/ob/ReadFeaturesTest.java b/jr-objects/src/test/java/com/fasterxml/jackson/jr/ob/ReadFeaturesTest.java index af94624b..3f0d9dab 100644 --- a/jr-objects/src/test/java/com/fasterxml/jackson/jr/ob/ReadFeaturesTest.java +++ b/jr-objects/src/test/java/com/fasterxml/jackson/jr/ob/ReadFeaturesTest.java @@ -32,4 +32,17 @@ public void testPojoWithIsGetter() throws Exception .asString(new IsBean()); assertEquals(aposToQuotes("{'value':42}"), json); } + + public void testFailOnDupMapKeys() throws Exception + { + JSON j = JSON.std.with(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS); + assertTrue(j.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS)); + final String json = "{\"a\":1,\"b\":2,\"b\":3,\"c\":4}"; + try { + /*Map map =*/ j.mapFrom(json); + fail("Should not pass"); + } catch (JSONObjectException e) { + verifyException(e, "Duplicate key"); + } + } } diff --git a/release-notes/VERSION b/release-notes/VERSION index 9459ce04..31bc1a66 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -4,6 +4,11 @@ Project: jackson-jr === Releases === ------------------------------------------------------------------------ +2.8.9 (not yet released) + +#50: Duplicate key detection does not work + (reported by inorick@github) + 2.8.8 (05-Apr-2017) 2.8.7 (21-Feb-2017) 2.8.6 (12-Jan-2017)