Fix visualization error generated by using "other bucket" in terms aggregations. (#40855) (#41501)

This commit is contained in:
Luke Elmers 2019-07-18 14:23:02 -06:00 committed by GitHub
parent 49209781fb
commit f1d6c05ec0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 70 additions and 23 deletions

View file

@ -111,6 +111,22 @@ const nestedTermResponse = {
}, 'status': 200
};
const nestedTermResponseNoResults = {
'took': 10,
'timed_out': false,
'_shards': {
'total': 1, 'successful': 1, 'skipped': 0, 'failed': 0
}, 'hits': {
'total': 0, 'max_score': null, 'hits': []
}, 'aggregations': {
'1': {
'doc_count_error_upper_bound': 0,
'sum_other_doc_count': 0,
'buckets': []
}
}, 'status': 200
};
const singleOtherResponse = {
'took': 3,
'timed_out': false,
@ -232,6 +248,12 @@ describe('Terms Agg Other bucket helper', () => {
expect(agg).to.eql(expectedResponse);
});
it('returns false when nested terms agg has no buckets', () => {
init(visConfigNestedTerm);
const agg = buildOtherBucketAgg(vis.aggs, vis.aggs[1], nestedTermResponseNoResults);
expect(agg).to.eql(false);
});
});
describe('mergeOtherBucketAggResponse', () => {

View file

@ -26,8 +26,16 @@ import { buildExistsFilter, buildPhrasesFilter, buildQueryFromFilters } from '@k
* @param startFromId: id of an aggregation from where we want to get the nested DSL
*/
const getNestedAggDSL = (aggNestedDsl, startFromAggId) => {
if (aggNestedDsl[startFromAggId]) return aggNestedDsl[startFromAggId];
return getNestedAggDSL(_.values(aggNestedDsl)[0].aggs, startFromAggId);
if (aggNestedDsl[startFromAggId]) {
return aggNestedDsl[startFromAggId];
}
const nestedAggs = _.values(aggNestedDsl);
let aggs;
for (let i = 0; i < nestedAggs.length; i++) {
if (nestedAggs[i].aggs && (aggs = getNestedAggDSL(nestedAggs[i].aggs, startFromAggId))) {
return aggs;
}
}
};
/**
@ -42,19 +50,27 @@ const getAggResultBuckets = (aggConfigs, response, aggWithOtherBucket, key) => {
let responseAgg = response;
for (const i in keyParts) {
if (keyParts[i]) {
const agg = _.values(responseAgg)[0];
const aggKey = _.keys(responseAgg)[0];
const aggConfig = _.find(aggConfigs, agg => agg.id === aggKey);
const bucket = _.find(agg.buckets, (bucket, bucketObjKey) => {
const bucketKey = aggConfig.getKey(bucket, Number.isInteger(bucketObjKey) ? null : bucketObjKey).toString();
return bucketKey === keyParts[i];
});
if (bucket) {
responseAgg = bucket;
const responseAggs = _.values(responseAgg);
// If you have multi aggs, we cannot just assume the first one is the `other` bucket,
// so we need to loop over each agg until we find it.
for (let aggId = 0; aggId < responseAggs.length; aggId++) {
const agg = responseAggs[aggId];
const aggKey = _.keys(responseAgg)[aggId];
const aggConfig = _.find(aggConfigs, agg => agg.id === aggKey);
const bucket = _.find(agg.buckets, (bucket, bucketObjKey) => {
const bucketKey = aggConfig.getKey(bucket, Number.isInteger(bucketObjKey) ? null : bucketObjKey).toString();
return bucketKey === keyParts[i];
});
if (bucket) {
responseAgg = bucket;
break;
}
}
}
}
if (responseAgg[aggWithOtherBucket.id]) return responseAgg[aggWithOtherBucket.id].buckets;
if (responseAgg[aggWithOtherBucket.id]) {
return responseAgg[aggWithOtherBucket.id].buckets;
}
return [];
};
@ -99,8 +115,7 @@ const getOtherAggTerms = (requestAgg, key, otherAgg) => {
);
};
const buildOtherBucketAgg = (aggConfigs, aggWithOtherBucket, response) => {
export const buildOtherBucketAgg = (aggConfigs, aggWithOtherBucket, response) => {
const bucketAggs = aggConfigs.filter(agg => agg.type.type === 'buckets');
const index = bucketAggs.findIndex(agg => agg.id === aggWithOtherBucket.id);
const aggs = aggConfigs.toDsl();
@ -120,8 +135,16 @@ const buildOtherBucketAgg = (aggConfigs, aggWithOtherBucket, response) => {
filters: filterAgg.toDsl(),
};
// create filters for all parent aggregation buckets
let noAggBucketResults = false;
// recursively create filters for all parent aggregation buckets
const walkBucketTree = (aggIndex, aggs, aggId, filters, key) => {
// make sure there are actually results for the buckets
if (aggs[aggId].buckets.length < 1) {
noAggBucketResults = true;
return;
}
const agg = aggs[aggId];
const newAggIndex = aggIndex + 1;
const newAgg = bucketAggs[newAggIndex];
@ -154,6 +177,11 @@ const buildOtherBucketAgg = (aggConfigs, aggWithOtherBucket, response) => {
};
walkBucketTree(0, response.aggregations, bucketAggs[0].id, [], '');
// bail if there were no bucket results
if (noAggBucketResults) {
return false;
}
return () => {
return {
'other-filter': resultAgg
@ -161,7 +189,7 @@ const buildOtherBucketAgg = (aggConfigs, aggWithOtherBucket, response) => {
};
};
const mergeOtherBucketAggResponse = (aggsConfig, response, otherResponse, otherAgg, requestAgg) => {
export const mergeOtherBucketAggResponse = (aggsConfig, response, otherResponse, otherAgg, requestAgg) => {
const updatedResponse = _.cloneDeep(response);
_.each(otherResponse.aggregations['other-filter'].buckets, (bucket, key) => {
if (!bucket.doc_count) return;
@ -182,7 +210,7 @@ const mergeOtherBucketAggResponse = (aggsConfig, response, otherResponse, otherA
return updatedResponse;
};
const updateMissingBucket = (response, aggConfigs, agg) => {
export const updateMissingBucket = (response, aggConfigs, agg) => {
const updatedResponse = _.cloneDeep(response);
const aggResultBuckets = getAggConfigResultMissingBuckets(updatedResponse.aggregations, agg.id);
aggResultBuckets.forEach(bucket => {
@ -190,9 +218,3 @@ const updateMissingBucket = (response, aggConfigs, agg) => {
});
return updatedResponse;
};
export {
buildOtherBucketAgg,
mergeOtherBucketAggResponse,
updateMissingBucket,
};

View file

@ -77,9 +77,12 @@ export const termsBucketAgg = new BucketAggType({
},
createFilter: createFilterTerms,
postFlightRequest: async (resp, aggConfigs, aggConfig, searchSource, inspectorAdapters) => {
if (!resp.aggregations) return resp;
const nestedSearchSource = searchSource.createChild();
if (aggConfig.params.otherBucket) {
const filterAgg = buildOtherBucketAgg(aggConfigs, aggConfig, resp);
if (!filterAgg) return resp;
nestedSearchSource.setField('aggs', filterAgg);
const request = inspectorAdapters.requests.start(