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

Adding Duplicate key check in JacksonJrsTreeCodec #127

Merged
merged 14 commits into from
Mar 11, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,31 @@
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
* (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")
Expand All @@ -32,49 +37,53 @@ public <T extends TreeNode> 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<JrsValue> 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: {
Shounaks marked this conversation as resolved.
Show resolved Hide resolved
Map<String, JrsValue> 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 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() + ")");
}

private boolean duplicateCheck(Map<String, JrsValue> values, String currentName) {
return _config != null && _config.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS) && values.containsKey(currentName);
}

@Override
Expand Down Expand Up @@ -121,22 +130,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) {
Expand All @@ -150,9 +155,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) {
Expand All @@ -167,12 +170,12 @@ public JrsNumber numberNode(Number nr) {
/* Internal methods
/**********************************************************************
*/

protected List<JrsValue> _list() {
return new ArrayList<>();
}

protected Map<String,JrsValue> _map() {
protected Map<String, JrsValue> _map() {
return new LinkedHashMap<>();
}
}
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -20,6 +21,10 @@ public JrSimpleTreeExtension() {
this(new JacksonJrsTreeCodec());
}

public JrSimpleTreeExtension(JSON config) {
this(new JacksonJrsTreeCodec(config));
}

public JrSimpleTreeExtension(JacksonJrsTreeCodec tc) {
_codec = tc;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,28 +1,41 @@
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
* 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 {
/*TreeNode node =*/ treeJSON.treeFrom(json);
fail("Should not pass");
} catch (JSONObjectException e) {
verifyException(e, "Duplicate key");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand All @@ -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))
Copy link
Member

@cowtowncoder cowtowncoder Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is backwards incompatible change, breaking all existing users. I don't think that is
doable.

But there are other ways to achieve this, suggested below.

.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();
}
}