From 34ee3fe20925c6216055b0f3ba19750e2fe8f1b1 Mon Sep 17 00:00:00 2001 From: Shalnark <65479699+Shounaks@users.noreply.github.com> Date: Wed, 28 Feb 2024 22:54:42 +0530 Subject: [PATCH 1/6] Adding Duplicate key check in JacksonJrsTreeCodec. --- .../jackson/jr/stree/JacksonJrsTreeCodec.java | 11 ++++++++--- .../stree/{failing => }/DupFieldNameInTree51Test.java | 4 +--- 2 files changed, 9 insertions(+), 6 deletions(-) rename jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/{failing => }/DupFieldNameInTree51Test.java (85%) diff --git a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java index 070f39df..5e70443a 100644 --- a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java +++ b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java @@ -4,6 +4,8 @@ import java.util.*; import com.fasterxml.jackson.core.*; +import com.fasterxml.jackson.jr.ob.JSON; +import com.fasterxml.jackson.jr.ob.JSONObjectException; /** * {@link TreeCodec} implementation that can build "simple", immutable @@ -60,6 +62,9 @@ private JrsValue nodeFrom(JsonParser p) throws IOException Map values = _map(); while (p.nextToken() != JsonToken.END_OBJECT) { final String currentName = p.currentName(); + if(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS.enabledByDefault() && values.containsKey(currentName)){ + throw new JSONObjectException("Duplicate key (key '"+currentName+"')"); + } p.nextToken(); values.put(currentName, nodeFrom(p)); } @@ -108,7 +113,7 @@ public JrsValue nullNode() { @Override public JsonParser treeAsTokens(TreeNode node) { - return ((JrsValue) node).traverse(_objectCodec); + return node.traverse(_objectCodec); } /* @@ -169,10 +174,10 @@ public JrsNumber numberNode(Number nr) { */ protected List _list() { - return new ArrayList(); + return new ArrayList<>(); } protected Map _map() { - return new LinkedHashMap(); + return new LinkedHashMap<>(); } } diff --git a/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/failing/DupFieldNameInTree51Test.java b/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/DupFieldNameInTree51Test.java similarity index 85% rename from jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/failing/DupFieldNameInTree51Test.java rename to jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/DupFieldNameInTree51Test.java index de6599a8..9e5e15fa 100644 --- a/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/failing/DupFieldNameInTree51Test.java +++ b/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/DupFieldNameInTree51Test.java @@ -1,8 +1,7 @@ -package com.fasterxml.jackson.jr.stree.failing; +package com.fasterxml.jackson.jr.stree; import com.fasterxml.jackson.jr.ob.JSON; import com.fasterxml.jackson.jr.ob.JSONObjectException; -import com.fasterxml.jackson.jr.stree.JacksonJrTreeTestBase; /** * Tests for reading content using {@link JSON} with proper @@ -22,7 +21,6 @@ public void testFailOnDupMapKeys() throws Exception final String json = "{\"a\":1,\"b\":2,\"b\":3,\"c\":4}"; try { /*TreeNode node =*/ treeJSON.treeFrom(json); - fail("Should not pass"); } catch (JSONObjectException e) { verifyException(e, "Duplicate key"); } From 8c8588a4a58a90c6c544d769aa44385590cca1d4 Mon Sep 17 00:00:00 2001 From: Shalnark <65479699+Shounaks@users.noreply.github.com> Date: Tue, 5 Mar 2024 09:18:03 +0530 Subject: [PATCH 2/6] Adding Config to extension --- .../jackson/jr/stree/JacksonJrsTreeCodec.java | 80 +++++++++---------- .../jr/stree/JrSimpleTreeExtension.java | 5 ++ .../jr/stree/DupFieldNameInTree51Test.java | 25 ++++-- .../jr/stree/JacksonJrTreeTestBase.java | 12 ++- 4 files changed, 72 insertions(+), 50 deletions(-) diff --git a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java index 5e70443a..8a1404fa 100644 --- a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java +++ b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java @@ -12,20 +12,23 @@ * (read-only) trees out of JSON: these are represented as subtypes * of {@link JrsValue} ("Jrs" from "jackson JR Simple"). */ -public class JacksonJrsTreeCodec extends TreeCodec -{ - public static JrsMissing MISSING = JrsMissing.instance; - +public class JacksonJrsTreeCodec extends TreeCodec { public static final JacksonJrsTreeCodec SINGLETON = new JacksonJrsTreeCodec(); - + public static JrsMissing MISSING = JrsMissing.instance; + public final JSON _config; protected ObjectCodec _objectCodec; public JacksonJrsTreeCodec() { - this(null); + this(null, null); } - public JacksonJrsTreeCodec(ObjectCodec codec) { + public JacksonJrsTreeCodec(JSON config) { + this(null, config); + } + + public JacksonJrsTreeCodec(ObjectCodec codec, JSON config) { _objectCodec = codec; + _config = config; } @SuppressWarnings("unchecked") @@ -34,52 +37,49 @@ public T readTree(JsonParser p) throws IOException { return (T) nodeFrom(p); } - private JrsValue nodeFrom(JsonParser p) throws IOException - { + private JrsValue nodeFrom(JsonParser p) throws IOException { int tokenId = p.hasCurrentToken() ? p.currentTokenId() : p.nextToken().id(); - + switch (tokenId) { - case JsonTokenId.ID_TRUE: - return JrsBoolean.TRUE; - case JsonTokenId.ID_FALSE: - return JrsBoolean.FALSE; - case JsonTokenId.ID_NUMBER_INT: - case JsonTokenId.ID_NUMBER_FLOAT: - return new JrsNumber(p.getNumberValue()); - case JsonTokenId.ID_STRING: - return new JrsString(p.getText()); - case JsonTokenId.ID_START_ARRAY: - { + case JsonTokenId.ID_TRUE: + return JrsBoolean.TRUE; + case JsonTokenId.ID_FALSE: + return JrsBoolean.FALSE; + case JsonTokenId.ID_NUMBER_INT: + case JsonTokenId.ID_NUMBER_FLOAT: + return new JrsNumber(p.getNumberValue()); + case JsonTokenId.ID_STRING: + return new JrsString(p.getText()); + case JsonTokenId.ID_START_ARRAY: { List values = _list(); while (p.nextToken() != JsonToken.END_ARRAY) { values.add(nodeFrom(p)); } return new JrsArray(values); } - case JsonTokenId.ID_START_OBJECT: - { + case JsonTokenId.ID_START_OBJECT: { Map values = _map(); while (p.nextToken() != JsonToken.END_OBJECT) { final String currentName = p.currentName(); - if(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS.enabledByDefault() && values.containsKey(currentName)){ - throw new JSONObjectException("Duplicate key (key '"+currentName+"')"); + if (_config.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS) && values.containsKey(currentName)) { + throw new JSONObjectException("Duplicate key (key '" + currentName + "')"); } p.nextToken(); values.put(currentName, nodeFrom(p)); } return new JrsObject(values); } - case JsonTokenId.ID_EMBEDDED_OBJECT: - // 07-Jan-2016, tatu: won't happen with JSON, but other types like Smile - // may produce binary data or such - return new JrsEmbeddedObject(p.getEmbeddedObject()); - - case JsonTokenId.ID_NULL: - return JrsNull.instance; - default: + case JsonTokenId.ID_EMBEDDED_OBJECT: + // 07-Jan-2016, tatu: won't happen with JSON, but other types like Smile + // may produce binary data or such + return new JrsEmbeddedObject(p.getEmbeddedObject()); + + case JsonTokenId.ID_NULL: + return JrsNull.instance; + default: } - throw new UnsupportedOperationException("Unsupported token id "+tokenId+" ("+p.currentToken()+")"); + throw new UnsupportedOperationException("Unsupported token id " + tokenId + " (" + p.currentToken() + ")"); } @Override @@ -126,22 +126,18 @@ public JsonParser treeAsTokens(TreeNode node) { * Factory method for constructing node to represent Boolean values. * * @param state Whether to create {@code Boolean.TRUE} or {@code Boolean.FALSE} node - * * @return Node instance for given boolean value - * * @since 2.8 */ public JrsBoolean booleanNode(boolean state) { - return state? JrsBoolean.TRUE : JrsBoolean.FALSE; + return state ? JrsBoolean.TRUE : JrsBoolean.FALSE; } /** * Factory method for constructing node to represent String values. * * @param text String value for constructed node to contain - * * @return Node instance for given text value - * * @since 2.8 */ public JrsString stringNode(String text) { @@ -155,9 +151,7 @@ public JrsString stringNode(String text) { * Factory method for constructing node to represent String values. * * @param nr Numeric value for constructed node to contain - * * @return Node instance for given numeric value - * * @since 2.8 */ public JrsNumber numberNode(Number nr) { @@ -172,12 +166,12 @@ public JrsNumber numberNode(Number nr) { /* Internal methods /********************************************************************** */ - + protected List _list() { return new ArrayList<>(); } - protected Map _map() { + protected Map _map() { return new LinkedHashMap<>(); } } diff --git a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JrSimpleTreeExtension.java b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JrSimpleTreeExtension.java index 9d8954f2..3048fc34 100644 --- a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JrSimpleTreeExtension.java +++ b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JrSimpleTreeExtension.java @@ -1,5 +1,6 @@ package com.fasterxml.jackson.jr.stree; +import com.fasterxml.jackson.jr.ob.JSON; import com.fasterxml.jackson.jr.ob.JacksonJrExtension; import com.fasterxml.jackson.jr.ob.api.ExtensionContext; @@ -20,6 +21,10 @@ public JrSimpleTreeExtension() { this(new JacksonJrsTreeCodec()); } + public JrSimpleTreeExtension(JSON config) { + this(new JacksonJrsTreeCodec(config)); + } + public JrSimpleTreeExtension(JacksonJrsTreeCodec tc) { _codec = tc; } diff --git a/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/DupFieldNameInTree51Test.java b/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/DupFieldNameInTree51Test.java index 9e5e15fa..42097a8a 100644 --- a/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/DupFieldNameInTree51Test.java +++ b/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/DupFieldNameInTree51Test.java @@ -7,16 +7,31 @@ * Tests for reading content using {@link JSON} with proper * codec registration */ -public class DupFieldNameInTree51Test extends JacksonJrTreeTestBase -{ - private final JSON treeJSON = jsonWithTreeCodec(); +public class DupFieldNameInTree51Test extends JacksonJrTreeTestBase { // [jackson-jr#51]: test dup keys for trees too - public void testFailOnDupMapKeys() throws Exception - { + public void testFailOnDupMapKeys() throws Exception { JSON j = JSON.builder() .enable(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS) .build(); + + final JSON treeJSON = jsonWithTreeCodec(j); + + assertTrue(j.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS)); + final String json = "{\"a\":1,\"b\":2,\"b\":3,\"c\":4}"; + try { + /*TreeNode node =*/ treeJSON.treeFrom(json); + } catch (JSONObjectException e) { + verifyException(e, "Duplicate key"); + } + } + + public void testFailOnDupMapKeys2() throws Exception { + //missing flag - Enabled by default! + JSON j = JSON.builder().build(); + + final JSON treeJSON = jsonWithTreeCodec(j); + assertTrue(j.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS)); final String json = "{\"a\":1,\"b\":2,\"b\":3,\"c\":4}"; try { diff --git a/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/JacksonJrTreeTestBase.java b/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/JacksonJrTreeTestBase.java index 4485dfa5..8ca333ca 100644 --- a/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/JacksonJrTreeTestBase.java +++ b/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/JacksonJrTreeTestBase.java @@ -29,7 +29,7 @@ protected void verifyException(Throwable e, String... matches) String lmsg = (msg == null) ? "" : msg.toLowerCase(); for (String match : matches) { String lmatch = match.toLowerCase(); - if (lmsg.indexOf(lmatch) >= 0) { + if (lmsg.contains(lmatch)) { return; } } @@ -44,11 +44,19 @@ protected String a2q(String json) { return json.replace("'", "\""); } + protected JSON jsonWithTreeCodec(JSON config) { + return JSON.builder() + // 13-Feb-2020, tatu: There are 2 different ways actually.. +// .treeCodec(new JacksonJrsTreeCodec()) + .register(new JrSimpleTreeExtension(config)) + .build(); + } + protected JSON jsonWithTreeCodec() { return JSON.builder() // 13-Feb-2020, tatu: There are 2 different ways actually.. // .treeCodec(new JacksonJrsTreeCodec()) .register(new JrSimpleTreeExtension()) - .build(); + .build(); } } From 8cfe0111eb76441c31fa3a31ce5ab57c61178d7b Mon Sep 17 00:00:00 2001 From: Shalnark <65479699+Shounaks@users.noreply.github.com> Date: Tue, 5 Mar 2024 09:22:12 +0530 Subject: [PATCH 3/6] Fixing duplicate check to add null check --- .../com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java index 8a1404fa..03f5cff9 100644 --- a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java +++ b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java @@ -62,7 +62,7 @@ private JrsValue nodeFrom(JsonParser p) throws IOException { Map values = _map(); while (p.nextToken() != JsonToken.END_OBJECT) { final String currentName = p.currentName(); - if (_config.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS) && values.containsKey(currentName)) { + if (duplicateCheck(values, currentName)) { throw new JSONObjectException("Duplicate key (key '" + currentName + "')"); } p.nextToken(); @@ -82,6 +82,10 @@ private JrsValue nodeFrom(JsonParser p) throws IOException { throw new UnsupportedOperationException("Unsupported token id " + tokenId + " (" + p.currentToken() + ")"); } + private boolean duplicateCheck(Map values, String currentName) { + return _config != null && _config.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS) && values.containsKey(currentName); + } + @Override public void writeTree(JsonGenerator g, TreeNode treeNode) throws IOException { if (treeNode == null) { From 75694dcd9be8a92e8522da7eca01e2eb8111c9b6 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sun, 10 Mar 2024 17:46:45 -0700 Subject: [PATCH 4/6] Undo some style changes to reduce diff --- .../jackson/jr/stree/JacksonJrsTreeCodec.java | 76 ++++++++++--------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java index 2ebeaafb..94550cf5 100644 --- a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java +++ b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java @@ -4,7 +4,6 @@ import java.util.*; import com.fasterxml.jackson.core.*; -import com.fasterxml.jackson.jr.ob.JSON; import com.fasterxml.jackson.jr.ob.JSONObjectException; /** @@ -12,7 +11,8 @@ * (read-only) trees out of JSON: these are represented as subtypes * of {@link JrsValue} ("Jrs" from "jackson JR Simple"). */ -public class JacksonJrsTreeCodec extends TreeCodec { +public class JacksonJrsTreeCodec extends TreeCodec +{ public static JrsMissing MISSING = JrsMissing.instance; protected final ObjectCodec _objectCodec; @@ -38,47 +38,48 @@ public T readTree(JsonParser p) throws IOException { return (T) nodeFrom(p); } - private JrsValue nodeFrom(JsonParser p) throws IOException { + private JrsValue nodeFrom(JsonParser p) throws IOException + { int tokenId = p.hasCurrentToken() ? p.currentTokenId() : p.nextToken().id(); switch (tokenId) { - case JsonTokenId.ID_TRUE: - return JrsBoolean.TRUE; - case JsonTokenId.ID_FALSE: - return JrsBoolean.FALSE; - case JsonTokenId.ID_NUMBER_INT: - case JsonTokenId.ID_NUMBER_FLOAT: - return new JrsNumber(p.getNumberValue()); - case JsonTokenId.ID_STRING: - return new JrsString(p.getText()); - case JsonTokenId.ID_START_ARRAY: { - List values = _list(); - while (p.nextToken() != JsonToken.END_ARRAY) { - values.add(nodeFrom(p)); - } - return new JrsArray(values); + case JsonTokenId.ID_TRUE: + return JrsBoolean.TRUE; + case JsonTokenId.ID_FALSE: + return JrsBoolean.FALSE; + case JsonTokenId.ID_NUMBER_INT: + case JsonTokenId.ID_NUMBER_FLOAT: + return new JrsNumber(p.getNumberValue()); + case JsonTokenId.ID_STRING: + return new JrsString(p.getText()); + case JsonTokenId.ID_START_ARRAY: { + List values = _list(); + while (p.nextToken() != JsonToken.END_ARRAY) { + values.add(nodeFrom(p)); } - case JsonTokenId.ID_START_OBJECT: { - Map values = _map(); - while (p.nextToken() != JsonToken.END_OBJECT) { - final String currentName = p.currentName(); - if (duplicateCheck(values, currentName)) { - throw new JSONObjectException("Duplicate key (key '" + currentName + "')"); - } - p.nextToken(); - values.put(currentName, nodeFrom(p)); + return new JrsArray(values); + } + case JsonTokenId.ID_START_OBJECT: { + Map values = _map(); + while (p.nextToken() != JsonToken.END_OBJECT) { + final String currentName = p.currentName(); + if (duplicateCheck(values, currentName)) { + throw new JSONObjectException("Duplicate key (key '" + currentName + "')"); } - return new JrsObject(values); + p.nextToken(); + values.put(currentName, nodeFrom(p)); } - case JsonTokenId.ID_EMBEDDED_OBJECT: - // 07-Jan-2016, tatu: won't happen with JSON, but other types like Smile - // may produce binary data or such - return new JrsEmbeddedObject(p.getEmbeddedObject()); - - case JsonTokenId.ID_NULL: - return JrsNull.instance; - default: + return new JrsObject(values); + } + case JsonTokenId.ID_EMBEDDED_OBJECT: + // 07-Jan-2016, tatu: won't happen with JSON, but other types like Smile + // may produce binary data or such + return new JrsEmbeddedObject(p.getEmbeddedObject()); + + case JsonTokenId.ID_NULL: + return JrsNull.instance; + default: } throw new UnsupportedOperationException("Unsupported token id " + tokenId + " (" + p.currentToken() + ")"); } @@ -132,6 +133,7 @@ public JsonParser treeAsTokens(TreeNode node) { * * @param state Whether to create {@code Boolean.TRUE} or {@code Boolean.FALSE} node * @return Node instance for given boolean value + * * @since 2.8 */ public JrsBoolean booleanNode(boolean state) { @@ -143,6 +145,7 @@ public JrsBoolean booleanNode(boolean state) { * * @param text String value for constructed node to contain * @return Node instance for given text value + * * @since 2.8 */ public JrsString stringNode(String text) { @@ -157,6 +160,7 @@ public JrsString stringNode(String text) { * * @param nr Numeric value for constructed node to contain * @return Node instance for given numeric value + * * @since 2.8 */ public JrsNumber numberNode(Number nr) { From 21476dea83dbd143c5d3dd64a56228cc6a7b4991 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sun, 10 Mar 2024 17:48:21 -0700 Subject: [PATCH 5/6] Update release notes --- release-notes/CREDITS-2.x | 2 ++ release-notes/VERSION-2.x | 2 ++ 2 files changed, 4 insertions(+) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 0e0c3d07..fa9294e0 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -52,6 +52,8 @@ Julian Honnen (@jhonnen) (2.17.0) * Contributed PoC of #25: Add support single-int Constructors (2.17.0) +* Contributed #51: Duplicate key detection does not work for (simple) Trees + (2.17.0) * Contributed fix for #93: Skip serialization of `groovy.lang.MetaClass` values to avoid `StackOverflowError` (2.17.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index c2560687..1dea6504 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -15,6 +15,8 @@ Not yet released #7: Support deserialization of `int[]` (contributed by @Shounaks) +#51: Duplicate key detection does not work for (simple) Trees + (contributed by @Shounaks) #131: Add mechanism for `JacksonJrExtension`s to access state of `JSON.Feature`s 2.17.0-rc1 (26-Feb-2024) From 2862feb6bce97ce546b1c49fc13c2e9e1c565f74 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sun, 10 Mar 2024 17:59:02 -0700 Subject: [PATCH 6/6] Final touches before merge --- .../jackson/jr/stree/JacksonJrsTreeCodec.java | 10 ++--- .../jr/stree/DupFieldNameInTree51Test.java | 45 +++++++++---------- .../jr/stree/JacksonJrTreeTestBase.java | 8 ---- 3 files changed, 23 insertions(+), 40 deletions(-) diff --git a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java index 94550cf5..6e14afe5 100644 --- a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java +++ b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java @@ -64,11 +64,11 @@ private JrsValue nodeFrom(JsonParser p) throws IOException Map values = _map(); while (p.nextToken() != JsonToken.END_OBJECT) { final String currentName = p.currentName(); - if (duplicateCheck(values, currentName)) { + p.nextToken(); + JrsValue prev = values.put(currentName, nodeFrom(p)); + if (_failOnDuplicateKeys && (prev != null)) { throw new JSONObjectException("Duplicate key (key '" + currentName + "')"); } - p.nextToken(); - values.put(currentName, nodeFrom(p)); } return new JrsObject(values); } @@ -84,10 +84,6 @@ private JrsValue nodeFrom(JsonParser p) throws IOException throw new UnsupportedOperationException("Unsupported token id " + tokenId + " (" + p.currentToken() + ")"); } - private boolean duplicateCheck(Map values, String currentName) { - return _failOnDuplicateKeys && values.containsKey(currentName); - } - @Override public void writeTree(JsonGenerator g, TreeNode treeNode) throws IOException { if (treeNode == null) { diff --git a/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/DupFieldNameInTree51Test.java b/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/DupFieldNameInTree51Test.java index 42097a8a..0869bfdf 100644 --- a/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/DupFieldNameInTree51Test.java +++ b/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/DupFieldNameInTree51Test.java @@ -7,37 +7,32 @@ * Tests for reading content using {@link JSON} with proper * codec registration */ -public class DupFieldNameInTree51Test extends JacksonJrTreeTestBase { - +public class DupFieldNameInTree51Test extends JacksonJrTreeTestBase +{ + private final JSON NO_DUPS_JSON = JSON.builder() + .enable(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS) + .register(new JrSimpleTreeExtension()) + .build(); + + private final JSON DUPS_OK_JSON = JSON.builder() + .disable(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS) + .register(new JrSimpleTreeExtension()) + .build(); + // [jackson-jr#51]: test dup keys for trees too - public void testFailOnDupMapKeys() throws Exception { - JSON j = JSON.builder() - .enable(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS) - .build(); - - final JSON treeJSON = jsonWithTreeCodec(j); - - assertTrue(j.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS)); + public void testFailOnDupMapKeys() throws Exception + { + assertTrue(NO_DUPS_JSON.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS)); final String json = "{\"a\":1,\"b\":2,\"b\":3,\"c\":4}"; try { - /*TreeNode node =*/ treeJSON.treeFrom(json); + /*TreeNode node =*/ NO_DUPS_JSON.treeFrom(json); + fail("Should not pass"); } catch (JSONObjectException e) { verifyException(e, "Duplicate key"); } - } - - public void testFailOnDupMapKeys2() throws Exception { - //missing flag - Enabled by default! - JSON j = JSON.builder().build(); - - final JSON treeJSON = jsonWithTreeCodec(j); - assertTrue(j.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS)); - final String json = "{\"a\":1,\"b\":2,\"b\":3,\"c\":4}"; - try { - /*TreeNode node =*/ treeJSON.treeFrom(json); - } catch (JSONObjectException e) { - verifyException(e, "Duplicate key"); - } + assertFalse(DUPS_OK_JSON.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS)); + // But should pass fine without setting + assertNotNull(DUPS_OK_JSON.treeFrom(json)); } } diff --git a/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/JacksonJrTreeTestBase.java b/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/JacksonJrTreeTestBase.java index 9f00883e..1422c079 100644 --- a/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/JacksonJrTreeTestBase.java +++ b/jr-stree/src/test/java/com/fasterxml/jackson/jr/stree/JacksonJrTreeTestBase.java @@ -44,14 +44,6 @@ protected String a2q(String json) { return json.replace("'", "\""); } - protected JSON jsonWithTreeCodec(JSON config) { - return JSON.builder() - // 13-Feb-2020, tatu: There are 2 different ways actually.. -// .treeCodec(new JacksonJrsTreeCodec()) - .register(new JrSimpleTreeExtension()) - .build(); - } - protected JSON jsonWithTreeCodec() { return JSON.builder() // 13-Feb-2020, tatu: There are 2 different ways actually..