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

Duplicate key detection does not work for java.util.Map #50

Closed
inorick opened this issue Apr 14, 2017 · 9 comments
Closed

Duplicate key detection does not work for java.util.Map #50

inorick opened this issue Apr 14, 2017 · 9 comments
Milestone

Comments

@inorick
Copy link

inorick commented Apr 14, 2017

The feature flag FAIL_ON_DUPLICATE_MAP_KEYS does not seem to work. As I understand the parser configuration, the following code should throw an exception but doesnt:

String duplicateKeyString = "{\"a\":1,\"b\":2,\"b\":3,\"c\":4}";`
Map<Object, Object> json = JSON.std
.with(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS)
.mapFrom(duplicateKeyString);

The same is true for this similar call:

JSON.std.with(new JacksonJrsTreeCodec())
.with(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS)
.treeFrom(duplicateKeyString);
@cowtowncoder
Copy link
Member

Definitely sounds like a bug yes. Which version?

@inorick
Copy link
Author

inorick commented Apr 14, 2017

I tested versions 2.8.8 and 2.9.0.pr2

@cowtowncoder
Copy link
Member

@inorick thank you! Just wanted to verify; I occasionally get reports against versions so old that problem has been fixed.

@cowtowncoder
Copy link
Member

Lol. Feature added originally, never implemented. Obviously also not tested. Should do both.

@inorick
Copy link
Author

inorick commented Apr 14, 2017

Hehe, thanks. I really like the 'jr' flavour of jackson and the whole idea behind it. This is literally the last thing I miss :)

@cowtowncoder cowtowncoder modified the milestones: 2.3.1, 2.8.9 Apr 14, 2017
@cowtowncoder
Copy link
Member

@inorick Thank you for reporting this; fixed. Please let me know if and when you find other edge cases; as I said testing coverage is bit lighter than it should be.

Fix will be in 2.8.9 / 2.9.0.pr3.

Also: appreciate feedback; it is hard to know which things are useful -- seemed to me too that light-weight variant like jr would make lots of sense for many use cases. So I'm happy others find it useful too!

@inorick
Copy link
Author

inorick commented Apr 14, 2017

Thanks, that was quick!
Sure, I plan to use jr more anyway and will report back if I encounter anything.

@inorick
Copy link
Author

inorick commented Apr 15, 2017

@cowtowncoder I have to reopen the issue: The function JSON.treeFrom() still ignores the feature.
The following test case fails in git master:

public void testFailOnDupMapKeys() throws Exception
    {
        JSON j = JSON.std.with(new JacksonJrsTreeCodec()).with(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS);
        assertNotNull(j.getTreeCodec());
        assertTrue(j.isEnabled(JSON.Feature.FAIL_ON_DUPLICATE_MAP_KEYS));
        
        final String json = "{\"a\":1,\"b\":2,\"b\":3,\"c\":4}";
        try {
            /*TreeNode tree =*/ j.treeFrom(json);
            fail("Should not pass");
        } catch (JSONObjectException e) {
            verifyException(e, "Duplicate key");
        }
    }

@cowtowncoder
Copy link
Member

@inorick Technically this is out of scope of setting, as it relates to java.util.Map keys, not tree keys ("Object" node). But I guess it would be reasonable to extend the definition. I'll open a new issue for that part.
(also: while a new feature could be added, probably no need as cases do not overlap).

@cowtowncoder cowtowncoder changed the title Duplicate key detection does not work Duplicate key detection does not work for java.util.Map Apr 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants