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

[Maps] fix term join agg key collision #63324

Merged
merged 10 commits into from
Apr 17, 2020
140 changes: 140 additions & 0 deletions x-pack/legacy/plugins/maps/common/migrations/join_agg_key.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { LAYER_TYPE } from '../constants';
import { migrateJoinAggKey } from './join_agg_key';

describe('migrateJoinAggKey', () => {
const joins = [
{
leftField: 'machine.os',
right: {
id: '9055b4aa-136a-4b6d-90ab-9f94ccfe5eb5',
indexPatternTitle: 'kibana_sample_data_logs',
term: 'machine.os.keyword',
metrics: [
{
type: 'avg',
field: 'bytes',
},
{
type: 'count',
},
],
whereQuery: {
query: 'bytes > 10000',
language: 'kuery',
},
indexPatternRefName: 'layer_1_join_0_index_pattern',
},
},
{
leftField: 'machine.os',
right: {
id: '9a7f4e71-9500-4512-82f1-b7eaee3d87ff',
indexPatternTitle: 'kibana_sample_data_logs',
term: 'machine.os.keyword',
whereQuery: {
query: 'bytes < 10000',
language: 'kuery',
},
metrics: [
{
type: 'avg',
field: 'bytes',
},
],
indexPatternRefName: 'layer_1_join_1_index_pattern',
},
},
];

test('Should handle missing layerListJSON attribute', () => {
const attributes = {
title: 'my map',
};
expect(migrateJoinAggKey({ attributes })).toEqual({
title: 'my map',
});
});

test('Should migrate vector styles from legacy join agg key to new join agg key', () => {
const layerListJSON = JSON.stringify([
{
type: LAYER_TYPE.VECTOR,
joins,
style: {
properties: {
fillColor: {
type: 'DYNAMIC',
options: {
color: 'Blues',
colorCategory: 'palette_0',
field: {
name:
'__kbnjoin__avg_of_bytes_groupby_kibana_sample_data_logs.machine.os.keyword',
origin: 'join',
},
fieldMetaOptions: {
isEnabled: true,
sigma: 3,
},
type: 'ORDINAL',
},
},
lineColor: {
type: 'DYNAMIC',
options: {
color: 'Blues',
colorCategory: 'palette_0',
field: {
name: '__kbnjoin__count_groupby_kibana_sample_data_logs.machine.os.keyword',
origin: 'join',
},
fieldMetaOptions: {
isEnabled: true,
sigma: 3,
},
type: 'ORDINAL',
},
},
lineWidth: {
type: 'DYNAMIC',
options: {
color: 'Blues',
colorCategory: 'palette_0',
field: {
name: 'mySourceField',
origin: 'source',
},
fieldMetaOptions: {
isEnabled: true,
sigma: 3,
},
type: 'ORDINAL',
},
},
},
},
},
]);
const attributes = {
title: 'my map',
layerListJSON,
};
const { layerListJSON: migratedLayerListJSON } = migrateJoinAggKey({ attributes });
const migratedLayerList = JSON.parse(migratedLayerListJSON!);
expect(migratedLayerList[0].style.properties.fillColor.options.field.name).toBe(
'__kbnjoin__avg_of_bytes__9055b4aa-136a-4b6d-90ab-9f94ccfe5eb5'
);
expect(migratedLayerList[0].style.properties.lineColor.options.field.name).toBe(
'__kbnjoin__count__9055b4aa-136a-4b6d-90ab-9f94ccfe5eb5'
);
expect(migratedLayerList[0].style.properties.lineWidth.options.field.name).toBe(
'mySourceField'
);
});
});
109 changes: 109 additions & 0 deletions x-pack/legacy/plugins/maps/common/migrations/join_agg_key.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import _ from 'lodash';
import {
AGG_DELIMITER,
AGG_TYPE,
FIELD_ORIGIN,
getJoinAggKey,
JOIN_FIELD_NAME_PREFIX,
LAYER_TYPE,
VECTOR_STYLES,
} from '../constants';
import { AggDescriptor, JoinDescriptor, LayerDescriptor } from '../descriptor_types';
import { MapSavedObjectAttributes } from '../../../../../plugins/maps/common/map_saved_object_type';

const GROUP_BY_DELIMITER = '_groupby_';

function getLegacyAggKey({
aggType,
aggFieldName,
indexPatternTitle,
termFieldName,
}: {
aggType: AGG_TYPE;
aggFieldName?: string;
indexPatternTitle: string;
termFieldName: string;
}): string {
const metricKey =
aggType !== AGG_TYPE.COUNT ? `${aggType}${AGG_DELIMITER}${aggFieldName}` : aggType;
return `${JOIN_FIELD_NAME_PREFIX}${metricKey}${GROUP_BY_DELIMITER}${indexPatternTitle}.${termFieldName}`;
}

function parseLegacyAggKey(legacyAggKey: string): { aggType: AGG_TYPE; aggFieldName?: string } {
const groupBySplit = legacyAggKey
.substring(JOIN_FIELD_NAME_PREFIX.length)
.split(GROUP_BY_DELIMITER);
const metricKey = groupBySplit[0];
const metricKeySplit = metricKey.split(AGG_DELIMITER);
return {
aggType: metricKeySplit[0] as AGG_TYPE,
aggFieldName: metricKeySplit.length === 2 ? metricKeySplit[1] : undefined,
};
}

export function migrateJoinAggKey({
attributes,
}: {
attributes: MapSavedObjectAttributes;
}): MapSavedObjectAttributes {
if (!attributes || !attributes.layerListJSON) {
return attributes;
}

const layerList: LayerDescriptor[] = JSON.parse(attributes.layerListJSON);
layerList.forEach((layerDescriptor: LayerDescriptor) => {
if (
(layerDescriptor.type === LAYER_TYPE.VECTOR ||
layerDescriptor.type === LAYER_TYPE.BLENDED_VECTOR) &&
layerDescriptor.style &&
layerDescriptor.joins &&
layerDescriptor.joins.length
) {
const legacyJoinFields = new Map<string, JoinDescriptor>();
layerDescriptor.joins.forEach((joinDescriptor: JoinDescriptor) => {
_.get(joinDescriptor, 'right.metrics', []).forEach((aggDescriptor: AggDescriptor) => {
const legacyAggKey = getLegacyAggKey({
aggType: aggDescriptor.type,
aggFieldName: aggDescriptor.field,
indexPatternTitle: _.get(joinDescriptor, 'right.indexPatternTitle', ''),
termFieldName: _.get(joinDescriptor, 'right.term', ''),
});
// The legacy getAggKey implemenation has a naming collision bug where
// aggType, aggFieldName, indexPatternTitle, and termFieldName would result in the identical aggKey.
// The VectorStyle implemenation used the first matching join descriptor
// so, in the event of a name collision, the first join descriptor will be used here as well.
if (!legacyJoinFields.has(legacyAggKey)) {
legacyJoinFields.set(legacyAggKey, joinDescriptor);
}
});
});

Object.keys(layerDescriptor.style.properties).forEach(key => {
const style: any = layerDescriptor.style!.properties[key as VECTOR_STYLES];
if (_.get(style, 'options.field.origin') === FIELD_ORIGIN.JOIN) {
const joinDescriptor = legacyJoinFields.get(style.options.field.name);
if (joinDescriptor) {
const { aggType, aggFieldName } = parseLegacyAggKey(style.options.field.name);
// Update legacy join agg key to new join agg key
style.options.field.name = getJoinAggKey({
aggType,
aggFieldName,
rightSourceId: joinDescriptor.right.id!,
});
}
}
});
}
});

return {
...attributes,
layerListJSON: JSON.stringify(layerList),
};
}
9 changes: 9 additions & 0 deletions x-pack/legacy/plugins/maps/migrations.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { moveApplyGlobalQueryToSources } from './common/migrations/move_apply_gl
import { addFieldMetaOptions } from './common/migrations/add_field_meta_options';
import { migrateSymbolStyleDescriptor } from './common/migrations/migrate_symbol_style_descriptor';
import { migrateUseTopHitsToScalingType } from './common/migrations/scaling_type';
import { migrateJoinAggKey } from './common/migrations/join_agg_key';

export const migrations = {
map: {
Expand Down Expand Up @@ -57,5 +58,13 @@ export const migrations = {
attributes: attributesPhase2,
};
},
'7.8.0': doc => {
const attributes = migrateJoinAggKey(doc);

return {
...doc,
attributes,
};
},
},
};
17 changes: 17 additions & 0 deletions x-pack/plugins/maps/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,22 @@ export enum FIELD_ORIGIN {
SOURCE = 'source',
JOIN = 'join',
}
export const JOIN_FIELD_NAME_PREFIX = '__kbnjoin__';

// function in common since its needed by migration
export function getJoinAggKey({
nreese marked this conversation as resolved.
Show resolved Hide resolved
aggType,
aggFieldName,
rightSourceId,
}: {
aggType: AGG_TYPE;
aggFieldName?: string;
rightSourceId: string;
nreese marked this conversation as resolved.
Show resolved Hide resolved
}) {
const metricKey =
aggType !== AGG_TYPE.COUNT ? `${aggType}${AGG_DELIMITER}${aggFieldName}` : aggType;
return `${JOIN_FIELD_NAME_PREFIX}${metricKey}__${rightSourceId}`;
}

export const SOURCE_DATA_ID_ORIGIN = 'source';
export const META_ID_ORIGIN_SUFFIX = 'meta';
Expand Down Expand Up @@ -130,6 +146,7 @@ export enum DRAW_TYPE {
POLYGON = 'POLYGON',
}

export const AGG_DELIMITER = '_of_';
export enum AGG_TYPE {
AVG = 'avg',
COUNT = 'count',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export type SourceDescriptor =
| ESGeoGridSourceDescriptor
| EMSFileSourceDescriptor;

export type LayerDescriptor = {
export type BaseLayerDescriptor = {
__dataRequests?: DataRequestDescriptor[];
__isInErrorState?: boolean;
__errorMessage?: string;
Expand All @@ -123,11 +123,13 @@ export type LayerDescriptor = {
visible?: boolean;
};

export type VectorLayerDescriptor = LayerDescriptor & {
export type VectorLayerDescriptor = BaseLayerDescriptor & {
joins?: JoinDescriptor[];
style?: VectorStyleDescriptor;
};

export type LayerDescriptor = VectorLayerDescriptor;
nreese marked this conversation as resolved.
Show resolved Hide resolved

export type RangeFieldMeta = {
min: number;
max: number;
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/maps/public/layers/joins/inner_join.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const leftJoin = new InnerJoin(
},
mockSource
);
const COUNT_PROPERTY_NAME = '__kbnjoin__count_groupby_kibana_sample_data_logs.geo.dest';
const COUNT_PROPERTY_NAME = '__kbnjoin__count__d3625663-5b34-4d50-a784-0d743f676a0c';

describe('joinPropertiesToFeature', () => {
it('Should add join property to features in feature collection', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,14 @@
import { i18n } from '@kbn/i18n';
import { AbstractESSource } from '../es_source';
import { esAggFieldsFactory } from '../../fields/es_agg_field';

import {
AGG_DELIMITER,
AGG_TYPE,
COUNT_PROP_LABEL,
COUNT_PROP_NAME,
FIELD_ORIGIN,
} from '../../../../common/constants';

export const AGG_DELIMITER = '_of_';

export class AbstractESAggSource extends AbstractESSource {
constructor(descriptor, inspectorAdapters) {
super(descriptor, inspectorAdapters);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,18 @@
import _ from 'lodash';

import { i18n } from '@kbn/i18n';
import { DEFAULT_MAX_BUCKETS_LIMIT, FIELD_ORIGIN, AGG_TYPE } from '../../../../common/constants';
import {
AGG_TYPE,
DEFAULT_MAX_BUCKETS_LIMIT,
FIELD_ORIGIN,
getJoinAggKey,
} from '../../../../common/constants';
import { ESDocField } from '../../fields/es_doc_field';
import { AbstractESAggSource, AGG_DELIMITER } from '../es_agg_source';
import { AbstractESAggSource } from '../es_agg_source';
import { getField, addFieldToDSL, extractPropertiesFromBucket } from '../../util/es_agg_utils';

const TERMS_AGG_NAME = 'join';

const FIELD_NAME_PREFIX = '__kbnjoin__';
const GROUP_BY_DELIMITER = '_groupby_';
const TERMS_BUCKET_KEYS_TO_IGNORE = ['key', 'doc_count'];

export function extractPropertiesMap(rawEsData, countPropertyName) {
Expand Down Expand Up @@ -64,11 +67,11 @@ export class ESTermSource extends AbstractESAggSource {
}

getAggKey(aggType, fieldName) {
const metricKey =
aggType !== AGG_TYPE.COUNT ? `${aggType}${AGG_DELIMITER}${fieldName}` : aggType;
return `${FIELD_NAME_PREFIX}${metricKey}${GROUP_BY_DELIMITER}${
this._descriptor.indexPatternTitle
}.${this._termField.getName()}`;
return getJoinAggKey({
aggType,
aggFieldName: fieldName,
rightSourceId: this._descriptor.id,
});
}

getAggLabel(aggType, fieldName) {
Expand Down
Loading