More customizable TokenFilter inclusion (using Tokenfilter.Inclusion)

[jhaber]

Related to #572

There is a new enum that makes the behavior of FilteringGeneratorDelegate more customizable. I first called this enum PathWriteMode, since it replaces the _includePath flag. The options were NONE (equivalent to _includePath=false), LAZY (equivalent to _includePath=true), or EAGER (write tokens immediately unless a null TokenFilter is returned). However, LAZY/EAGER are very tied to how FilteringGeneratorDelegate is implemented and may not make intuitive sense unless you're familiar with the implementation.

I realized that an equivalent way to think about this is in terms of how TokenFilter return values should be interpreted. In particular, what does it mean to return a filter that is neither null nor INCLUDE_ALL? It seems like the options are:

• this means don't include the token (equivalent to _includePath=false)
• this means include the token, but only if a child token returns INCLUDE_ALL (equivalent to _includePath=true)
• this means always include the token (the new behavior I'm looking for)

So I made the enum TokenFilter.Inclusion inside of TokenFilter with options of ONLY_INCLUDE_ALL, INCLUDE_ALL_AND_PATH, or INCLUDE_NON_NULL. I'm also realizing that this enum could apply equally to FilteringParserDelegate , so these changes should probably support both reading and writing. I can work on that next, just wanted to make sure everything makes sense so far

[jhaber]

Ok updated to support parsing as well. One of the tests I added for parsing is failing, but it appears to also fail on master so I don't think it's related to my changes. Here's the failing test you can add to BasicParserFilteringTest:

    public void testExcludeArrays() throws Exception
    {
        class NoArraysFilter extends TokenFilter
        {
            @Override
            public TokenFilter filterStartArray() {
                return null;
            }
        }

        String jsonString = aposToQuotes("{'a':123,'array':[1,2]}");
        JsonParser p0 = JSON_F.createParser(jsonString);
        FilteringParserDelegate p = new FilteringParserDelegate(p0,
            new NoArraysFilter(),
            true, // includePath
            true // multipleMatches
        );
        String result = readAndWrite(JSON_F, p);
        assertEquals(aposToQuotes("{'a':123}"), result);
        assertEquals(1, p.getMatchCount());
    }

When I run it I get:

junit.framework.ComparisonFailure: 
Expected :{"a":123}
Actual   :{"a"}

So it's generating invalid JSON. I'm not seeing anything wrong with the logic of the test, and the same test works on the FilteringGeneratorDelegate. I can try to get to the bottom of this, but not sure if you want me to put those changes in a separate PR since it appears orthogonal

[cowtowncoder]

@jhaber apologies for slow follow up. I added this on my short (?) list of things to do (https://github.com/FasterXML/jackson-future-ideas/wiki/Jackson-Work-in-Progress) and will try to review soon.

There is also minor (I hope) merge conflict, related to fix for #582.

[jhaber]

No worries, I'll get that merge conflict fixed in the meantime

[jhaber]

Merge conflict addressed

[cowtowncoder]

@jhaber I seem to have dropped the ball here... and so this can't go in 2.11 any more (since that is in patch mode).
But now that I am trying to finalize 2.12, I could get this merged in 2.12 instead, if you had time to rebase against 2.12?

[jhaber]

Sounds good, I can try to rebase before end of week

[cowtowncoder]

Excellent!

[jhaber]

Updated this PR to target 2.12 branch. Luckily there are no conflicts so that's all there was to it

[cowtowncoder]

Hmmh. Looks like there are 2 new test failures; one for parser filtering, another for generator; filtering generator produces invalid output ({ "a"})

[jhaber]

Sorry about that, I'll take a look over the weekend. I mentioned above about a similar quirk I noticed, but it failed the same way without my changes so I didn't dig into it too deeply

[cowtowncoder]

@jhaber np. I'll leave changes in 2.12 branch, will see if you can spot what might be going on. Not sure why PR build didn't catch this on Travis (but they seem to have had issues).

[jhaber]

Opened #650 to fix these tests