Filter path refactoring

[tlrx]

Jackson has been updated to version 2.6 in #13344, we can now reimplement the filter_path feature using the new Jackson streaming support.

No more custom hand-made logic (made in #10980), we can now implement a Jackson's TokenFilter and have a more readable and more maintainable code.

Bonus points:

• less code
• better performance
• closes add escaping for JSON response body filtering #11560 since JSON fields that contains dots (like .marvel-es-XX) are now correctly filtered
• allows to filter _source in search response hits / get document response / multi get document response

[clintongormley]

@tlrx note that fields can no longer contain embedded dots

[tlrx]

@clintongormley Yes I know, but indices still can have dot in names and the response filtering must be able to filter them correctly

[danielmitterdorfer]

Do yo need the parameter filters? The only call of evaluatejust uses the field filters.

[tlrx]

You're right. I'll need this field in an upcoming pull request where filters will be removed in favour of includes and excludes filters so I'd like to keep it :)

[danielmitterdorfer]

I left a few comments but apart from that LGTM. I am not sure whether somebody else wants to check so I am leaving the 'review' label for now. @tlrx Feel free to update it.

[tlrx]

@danielmitterdorfer Thanks for your review! I updated the code according to your comments about static/final. I'd like to keep the null check for now if that's fine for you.

I'll look for someone else to review it.

[jpountz]

I'm not too happy with those two methods. Could the constructor take those sub generators explicitly instead? Or maybe the constructor could take JsonGenerator jsonGenerator, String... filters and create them in the constructor?

[tlrx]

Could the constructor take those sub generators explicitly instead?

GeneratorBase is only used internally within this class and having a constructor that takes all the sub generators would exposes it to the outside.

Or maybe the constructor could take JsonGenerator jsonGenerator, String... filters and create them in the constructor?

I'd like to keep the filters out of this class as much as possible and let the *XContent classes instanciate everything is needed... As an alternative what do you think of:

    public JsonXContentGenerator(JsonGenerator generator) {
        this.generator = generator;

        if (generator instanceof FilteringGeneratorDelegate) {
            this.filter = (FilteringGeneratorDelegate) generator;
        } else {
            this.filter = null;
        }

        if (generator instanceof GeneratorBase) {
            this.base = (GeneratorBase) generator;
        } else if (filter != null && filter.getDelegate() instanceof GeneratorBase) {
            this.base = (GeneratorBase) filter.getDelegate();
        } else {
            this.base = null;
        }
    }

[jpountz]

I'm still unhappy about this: if for some reason you need to add another wrapper, it could hide the FilteringGeneratorDelegate and JsonXContentGenerator wouldn't know anymore that the content is filtered. I'd rather like something that does not depend on "instanceof".

[tlrx]

Oh I see, now I understand your point... It makes sense and I'm OK with both of your suggestions.

Would this be OK for you?

public JsonXContentGenerator(JsonGenerator jsonGenerator, String... filters) {
        if (jsonGenerator instanceof GeneratorBase) {
            this.base = (GeneratorBase) jsonGenerator;
        } else {
            this.base = null;
        }

        if (CollectionUtils.isEmpty(filters)) {
            this.filter = null;
            this.generator = jsonGenerator;
        } else {
            this.filter = new FilteringGeneratorDelegate(jsonGenerator, new FilterPathBasedFilter(filters), true, true);
            this.generator = this.filter;
        }
    }

[jpountz]

++

[jpountz]

I left some comments but I like the change in general, eg. how it handles raw fields.

[tlrx]

@jpountz I updated the code following your comments, can you give it another look please?

[jpountz]

LGTM

[tlrx]

@jpountz thanks!

[reinaldoluckman]

At this point in ES 2.1.0, there is some way to make a search like

http://localhost:9200/twitter/tweet/_search?q=message:elastic

And receive an array of _source's without any metadata like

[{
"user" : "yoyo",
"postDate" : "2009-11-15T14:12:12",
"message" : "Elastic is fun"
}, {
"user" : "bulgogi",
"postDate" : "2009-11-15T14:12:12",
"message" : "Elastic Search is cool"
}, {
"user" : "kimchy",
"postDate" : "2009-11-15T14:12:12",
"message" : "trying out Elastic Search"
}]

[tlrx]

For now, a GET /_search?filter_path=hits.hits._source will return only the _source of your document, containing all the fields and without metadata fields like _id, _index etc. Note that the structure of the JSON will be unchanged.