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.

Sign up for GitHub

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

Adding Duplicate key check in JacksonJrsTreeCodec #127

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
34ee3fe
Adding Duplicate key check in JacksonJrsTreeCodec.
Shounaks Feb 28, 2024
b606684
Merge branch '2.17' into 2.17-duplicateKey
Shounaks Feb 28, 2024
093e5a8
Merge branch '2.17' into 2.17-duplicateKey
cowtowncoder Feb 28, 2024
6c732e6
Merge branch '2.17' into 2.17-duplicateKey
Shounaks Mar 1, 2024
8c8588a
Adding Config to extension
Shounaks Mar 5, 2024
8cfe011
Fixing duplicate check to add null check
Shounaks Mar 5, 2024
a6a2b58
Merge branch '2.17' into 2.17-duplicateKey
Shounaks Mar 5, 2024
f2f8ea8
Merge branch '2.17' into 2.17-duplicateKey
Shounaks Mar 5, 2024
065b64a
Merge branch '2.17' into 2.17-duplicateKey
cowtowncoder Mar 8, 2024
f4a9e9a
Merge branch '2.17' into 2.17-duplicateKey
cowtowncoder Mar 8, 2024
6e05f32
Merge branch '2.17' into 2.17-duplicateKey
Shounaks Mar 10, 2024
75694dc
Undo some style changes to reduce diff
cowtowncoder Mar 11, 2024
21476de
Update release notes
cowtowncoder Mar 11, 2024
2862feb
Final touches before merge
cowtowncoder Mar 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/JacksonJrsTreeCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -60,6 +62,9 @@ private JrsValue nodeFrom(JsonParser p) throws IOException
Map<String, JrsValue> 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)){
Copy link
Member

Choose a reason for hiding this comment

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

Did you read my notes on #51? This does not work, it only considers default setting for feature but not its actual current state.

So without solving that problem this issue cannot really be solved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for pointing that out. As you said, TreeCodec doesn't have JSON as a parameter, so we cannot bring the JSON value of the current state up to this point. and its not a part of this library. (and also it doesn't make sense to change a library API just for a single line of code in another API. How should we tackle this problem... ☹️

Copy link
Member

Choose a reason for hiding this comment

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

Ideally configuration would be passed -- maybe there is something I overlooked earlier, as I did not spend tons of time on it? I am open to suggestions, as always.

throw new JSONObjectException("Duplicate key (key '"+currentName+"')");
}
p.nextToken();
values.put(currentName, nodeFrom(p));
}
Expand Down Expand Up @@ -108,7 +113,7 @@ public JrsValue nullNode() {

@Override
public JsonParser treeAsTokens(TreeNode node) {
return ((JrsValue) node).traverse(_objectCodec);
return node.traverse(_objectCodec);
}

/*
Expand Down Expand Up @@ -169,10 +174,10 @@ public JrsNumber numberNode(Number nr) {
*/

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

protected Map<String,JrsValue> _map() {
return new LinkedHashMap<String,JrsValue>();
return new LinkedHashMap<>();
}
}
4 changes: 1 addition & 3 deletions ...ree/failing/DupFieldNameInTree51Test.java → ...on/jr/stree/DupFieldNameInTree51Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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");
}
Expand Down