Add parsing for InternalStats

[cbuescher]

Similar to #24085 this adds parsing for InternalStats. This will be needed for the high level rest client.
PR is against the current feature branch for aggregation parsing.

[cbuescher]

This is one caveat I found in the current InternalStats implementation. We dont have a COUNT_AS_STRING field in the xContent output, but the java API provides formatted String values of the "long" count via getCountAsString(). Possible solutions I can think of for now:

• add COUNT_AS_STRING to xContent (breaking ?)
• change getCountAsString() in InternalStats to return Long.toString(count) (breaking Java?)
• remove getCountAsString() from the Stats interface (after all we don't render a formatted version)
• ignore this as a rare edge case and exclude it from testing like I've done it now

Wdyt?

[tlrx]

It looks like it has been forgotten, I think we can add it in master/v6 and ignore it/not add it in 5.x

[cbuescher]

I opened #24291 for this and Colin agreed that we should remove getCountAsString() on master. I rebased this PR on that change.

[tlrx]

LGTM, I left some comments though

[tlrx]

We could rename it to parseDouble() and add some Javadoc.

[tlrx]

final?

[tlrx]

I find this more readable:

        if (count != 0) {
            builder.field(Fields.MIN, min);
            builder.field(Fields.MAX, max);
            builder.field(Fields.AVG, avg);
            builder.field(Fields.SUM, sum);
            if (valueAsString.get(Fields.MIN_AS_STRING) != null) {
                builder.field(Fields.MIN_AS_STRING, getMinAsString());
                builder.field(Fields.MAX_AS_STRING, getMaxAsString());
                builder.field(Fields.AVG_AS_STRING, getAvgAsString());
                builder.field(Fields.SUM_AS_STRING, getSumAsString());
            }
        } else {
            builder.nullField(Fields.MIN);
            builder.nullField(Fields.MAX);
            builder.nullField(Fields.AVG);
            builder.nullField(Fields.SUM);       
        }

or maybe store count != 0 in a boolean?

[cbuescher]

Good point, should I also change this in InternalStats then to make it more readable there?

[cbuescher]

I changed this here in ParsedStats and left the other version in InternalStats. I don't know really which one I like better, probably the "old" one? Lets see if @javanna has an opinion when comparing both.

[javanna]

I like it, we could align also InternalStats.

[cbuescher]

Okay, I also changed InternalStats#doXContentBody(), do you think I should do this on master as well do have a smaller diff with the feature branch when we merge or is it enough to just do it on the branch? It shouldn't change any behavior...

[javanna]

I think it's fine to keep this on the branch, but if you prefer to move it to master I am good with that too.

[tlrx]

typo

[cbuescher]

Good catch, that is in there in InternalStats too. Will also change that.

[tlrx]

I think there's a type in core that can be fixed too :)

[tlrx]

It looks like it has been forgotten, I think we can add it in master/v6 and ignore it/not add it in 5.x

[javanna]

LGTM