Skip to content

Commit 420ccff

Browse files
nreeseelasticmachine
nreese
and
elasticmachine
authored
[Maps] fix term join agg key collision (#63324)
* [Maps] fix term join agg key collision * fix tslint and jest errors * fix join functional test * revert LayerDescriptor union and cast to VectorLayerDescriptor instead * move getJoinKey out of constants and into its own file Co-authored-by: Elastic Machine <[email protected]>
1 parent e3eed0b commit 420ccff

File tree

13 files changed

+325
-33
lines changed

13 files changed

+325
-33
lines changed

‎x-pack/legacy/plugins/maps/common/get_join_key.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
8+
export * from '../../../../plugins/maps/common/get_join_key';

‎x-pack/legacy/plugins/maps/common/migrations/join_agg_key.test.ts‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { LAYER_TYPE } from '../constants';
8+
import { migrateJoinAggKey } from './join_agg_key';
9+
10+
describe('migrateJoinAggKey', () => {
11+
const joins = [
12+
{
13+
leftField: 'machine.os',
14+
right: {
15+
id: '9055b4aa-136a-4b6d-90ab-9f94ccfe5eb5',
16+
indexPatternTitle: 'kibana_sample_data_logs',
17+
term: 'machine.os.keyword',
18+
metrics: [
19+
{
20+
type: 'avg',
21+
field: 'bytes',
22+
},
23+
{
24+
type: 'count',
25+
},
26+
],
27+
whereQuery: {
28+
query: 'bytes > 10000',
29+
language: 'kuery',
30+
},
31+
indexPatternRefName: 'layer_1_join_0_index_pattern',
32+
},
33+
},
34+
{
35+
leftField: 'machine.os',
36+
right: {
37+
id: '9a7f4e71-9500-4512-82f1-b7eaee3d87ff',
38+
indexPatternTitle: 'kibana_sample_data_logs',
39+
term: 'machine.os.keyword',
40+
whereQuery: {
41+
query: 'bytes < 10000',
42+
language: 'kuery',
43+
},
44+
metrics: [
45+
{
46+
type: 'avg',
47+
field: 'bytes',
48+
},
49+
],
50+
indexPatternRefName: 'layer_1_join_1_index_pattern',
51+
},
52+
},
53+
];
54+
55+
test('Should handle missing layerListJSON attribute', () => {
56+
const attributes = {
57+
title: 'my map',
58+
};
59+
expect(migrateJoinAggKey({ attributes })).toEqual({
60+
title: 'my map',
61+
});
62+
});
63+
64+
test('Should migrate vector styles from legacy join agg key to new join agg key', () => {
65+
const layerListJSON = JSON.stringify([
66+
{
67+
type: LAYER_TYPE.VECTOR,
68+
joins,
69+
style: {
70+
properties: {
71+
fillColor: {
72+
type: 'DYNAMIC',
73+
options: {
74+
color: 'Blues',
75+
colorCategory: 'palette_0',
76+
field: {
77+
name:
78+
'__kbnjoin__avg_of_bytes_groupby_kibana_sample_data_logs.machine.os.keyword',
79+
origin: 'join',
80+
},
81+
fieldMetaOptions: {
82+
isEnabled: true,
83+
sigma: 3,
84+
},
85+
type: 'ORDINAL',
86+
},
87+
},
88+
lineColor: {
89+
type: 'DYNAMIC',
90+
options: {
91+
color: 'Blues',
92+
colorCategory: 'palette_0',
93+
field: {
94+
name: '__kbnjoin__count_groupby_kibana_sample_data_logs.machine.os.keyword',
95+
origin: 'join',
96+
},
97+
fieldMetaOptions: {
98+
isEnabled: true,
99+
sigma: 3,
100+
},
101+
type: 'ORDINAL',
102+
},
103+
},
104+
lineWidth: {
105+
type: 'DYNAMIC',
106+
options: {
107+
color: 'Blues',
108+
colorCategory: 'palette_0',
109+
field: {
110+
name: 'mySourceField',
111+
origin: 'source',
112+
},
113+
fieldMetaOptions: {
114+
isEnabled: true,
115+
sigma: 3,
116+
},
117+
type: 'ORDINAL',
118+
},
119+
},
120+
},
121+
},
122+
},
123+
]);
124+
const attributes = {
125+
title: 'my map',
126+
layerListJSON,
127+
};
128+
const { layerListJSON: migratedLayerListJSON } = migrateJoinAggKey({ attributes });
129+
const migratedLayerList = JSON.parse(migratedLayerListJSON!);
130+
expect(migratedLayerList[0].style.properties.fillColor.options.field.name).toBe(
131+
'__kbnjoin__avg_of_bytes__9055b4aa-136a-4b6d-90ab-9f94ccfe5eb5'
132+
);
133+
expect(migratedLayerList[0].style.properties.lineColor.options.field.name).toBe(
134+
'__kbnjoin__count__9055b4aa-136a-4b6d-90ab-9f94ccfe5eb5'
135+
);
136+
expect(migratedLayerList[0].style.properties.lineWidth.options.field.name).toBe(
137+
'mySourceField'
138+
);
139+
});
140+
});

‎x-pack/legacy/plugins/maps/common/migrations/join_agg_key.ts‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import _ from 'lodash';
8+
import {
9+
AGG_DELIMITER,
10+
AGG_TYPE,
11+
FIELD_ORIGIN,
12+
JOIN_FIELD_NAME_PREFIX,
13+
LAYER_TYPE,
14+
VECTOR_STYLES,
15+
} from '../constants';
16+
import { getJoinAggKey } from '../get_join_key';
17+
import {
18+
AggDescriptor,
19+
JoinDescriptor,
20+
LayerDescriptor,
21+
VectorLayerDescriptor,
22+
} from '../descriptor_types';
23+
import { MapSavedObjectAttributes } from '../../../../../plugins/maps/common/map_saved_object_type';
24+
25+
const GROUP_BY_DELIMITER = '_groupby_';
26+
27+
function getLegacyAggKey({
28+
aggType,
29+
aggFieldName,
30+
indexPatternTitle,
31+
termFieldName,
32+
}: {
33+
aggType: AGG_TYPE;
34+
aggFieldName?: string;
35+
indexPatternTitle: string;
36+
termFieldName: string;
37+
}): string {
38+
const metricKey =
39+
aggType !== AGG_TYPE.COUNT ? `${aggType}${AGG_DELIMITER}${aggFieldName}` : aggType;
40+
return `${JOIN_FIELD_NAME_PREFIX}${metricKey}${GROUP_BY_DELIMITER}${indexPatternTitle}.${termFieldName}`;
41+
}
42+
43+
function parseLegacyAggKey(legacyAggKey: string): { aggType: AGG_TYPE; aggFieldName?: string } {
44+
const groupBySplit = legacyAggKey
45+
.substring(JOIN_FIELD_NAME_PREFIX.length)
46+
.split(GROUP_BY_DELIMITER);
47+
const metricKey = groupBySplit[0];
48+
const metricKeySplit = metricKey.split(AGG_DELIMITER);
49+
return {
50+
aggType: metricKeySplit[0] as AGG_TYPE,
51+
aggFieldName: metricKeySplit.length === 2 ? metricKeySplit[1] : undefined,
52+
};
53+
}
54+
55+
export function migrateJoinAggKey({
56+
attributes,
57+
}: {
58+
attributes: MapSavedObjectAttributes;
59+
}): MapSavedObjectAttributes {
60+
if (!attributes || !attributes.layerListJSON) {
61+
return attributes;
62+
}
63+
64+
const layerList: LayerDescriptor[] = JSON.parse(attributes.layerListJSON);
65+
layerList.forEach((layerDescriptor: LayerDescriptor) => {
66+
if (
67+
layerDescriptor.type === LAYER_TYPE.VECTOR ||
68+
layerDescriptor.type === LAYER_TYPE.BLENDED_VECTOR
69+
) {
70+
const vectorLayerDescriptor = layerDescriptor as VectorLayerDescriptor;
71+
72+
if (
73+
!vectorLayerDescriptor.style ||
74+
!vectorLayerDescriptor.joins ||
75+
vectorLayerDescriptor.joins.length === 0
76+
) {
77+
return;
78+
}
79+
80+
const legacyJoinFields = new Map<string, JoinDescriptor>();
81+
vectorLayerDescriptor.joins.forEach((joinDescriptor: JoinDescriptor) => {
82+
_.get(joinDescriptor, 'right.metrics', []).forEach((aggDescriptor: AggDescriptor) => {
83+
const legacyAggKey = getLegacyAggKey({
84+
aggType: aggDescriptor.type,
85+
aggFieldName: aggDescriptor.field,
86+
indexPatternTitle: _.get(joinDescriptor, 'right.indexPatternTitle', ''),
87+
termFieldName: _.get(joinDescriptor, 'right.term', ''),
88+
});
89+
// The legacy getAggKey implemenation has a naming collision bug where
90+
// aggType, aggFieldName, indexPatternTitle, and termFieldName would result in the identical aggKey.
91+
// The VectorStyle implemenation used the first matching join descriptor
92+
// so, in the event of a name collision, the first join descriptor will be used here as well.
93+
if (!legacyJoinFields.has(legacyAggKey)) {
94+
legacyJoinFields.set(legacyAggKey, joinDescriptor);
95+
}
96+
});
97+
});
98+
99+
Object.keys(vectorLayerDescriptor.style.properties).forEach(key => {
100+
const style: any = vectorLayerDescriptor.style!.properties[key as VECTOR_STYLES];
101+
if (_.get(style, 'options.field.origin') === FIELD_ORIGIN.JOIN) {
102+
const joinDescriptor = legacyJoinFields.get(style.options.field.name);
103+
if (joinDescriptor) {
104+
const { aggType, aggFieldName } = parseLegacyAggKey(style.options.field.name);
105+
// Update legacy join agg key to new join agg key
106+
style.options.field.name = getJoinAggKey({
107+
aggType,
108+
aggFieldName,
109+
rightSourceId: joinDescriptor.right.id!,
110+
});
111+
}
112+
}
113+
});
114+
}
115+
});
116+
117+
return {
118+
...attributes,
119+
layerListJSON: JSON.stringify(layerList),
120+
};
121+
}

‎x-pack/legacy/plugins/maps/migrations.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { moveApplyGlobalQueryToSources } from './common/migrations/move_apply_gl
1111
import { addFieldMetaOptions } from './common/migrations/add_field_meta_options';
1212
import { migrateSymbolStyleDescriptor } from './common/migrations/migrate_symbol_style_descriptor';
1313
import { migrateUseTopHitsToScalingType } from './common/migrations/scaling_type';
14+
import { migrateJoinAggKey } from './common/migrations/join_agg_key';
1415

1516
export const migrations = {
1617
map: {
@@ -57,5 +58,13 @@ export const migrations = {
5758
attributes: attributesPhase2,
5859
};
5960
},
61+
'7.8.0': doc => {
62+
const attributes = migrateJoinAggKey(doc);
63+
64+
return {
65+
...doc,
66+
attributes,
67+
};
68+
},
6069
},
6170
};

‎x-pack/plugins/maps/common/constants.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,3 @@
1-
/*
2-
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3-
* or more contributor license agreements. Licensed under the Elastic License;
4-
* you may not use this file except in compliance with the Elastic License.
5-
*/
6-
71
/*
82
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
93
* or more contributor license agreements. Licensed under the Elastic License;
@@ -75,6 +69,7 @@ export enum FIELD_ORIGIN {
7569
SOURCE = 'source',
7670
JOIN = 'join',
7771
}
72+
export const JOIN_FIELD_NAME_PREFIX = '__kbnjoin__';
7873

7974
export const SOURCE_DATA_ID_ORIGIN = 'source';
8075
export const META_ID_ORIGIN_SUFFIX = 'meta';
@@ -132,6 +127,7 @@ export enum DRAW_TYPE {
132127
POLYGON = 'POLYGON',
133128
}
134129

130+
export const AGG_DELIMITER = '_of_';
135131
export enum AGG_TYPE {
136132
AVG = 'avg',
137133
COUNT = 'count',

‎x-pack/plugins/maps/common/get_join_key.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { AGG_DELIMITER, AGG_TYPE, JOIN_FIELD_NAME_PREFIX } from './constants';
8+
9+
// function in common since its needed by migration
10+
export function getJoinAggKey({
11+
aggType,
12+
aggFieldName,
13+
rightSourceId,
14+
}: {
15+
aggType: AGG_TYPE;
16+
aggFieldName?: string;
17+
rightSourceId: string;
18+
}) {
19+
const metricKey =
20+
aggType !== AGG_TYPE.COUNT ? `${aggType}${AGG_DELIMITER}${aggFieldName}` : aggType;
21+
return `${JOIN_FIELD_NAME_PREFIX}${metricKey}__${rightSourceId}`;
22+
}

‎x-pack/plugins/maps/public/layers/joins/inner_join.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const leftJoin = new InnerJoin(
3535
},
3636
mockSource
3737
);
38-
const COUNT_PROPERTY_NAME = '__kbnjoin__count_groupby_kibana_sample_data_logs.geo.dest';
38+
const COUNT_PROPERTY_NAME = '__kbnjoin__count__d3625663-5b34-4d50-a784-0d743f676a0c';
3939

4040
describe('joinPropertiesToFeature', () => {
4141
it('Should add join property to features in feature collection', () => {

‎x-pack/plugins/maps/public/layers/sources/es_agg_source/es_agg_source.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,14 @@
77
import { i18n } from '@kbn/i18n';
88
import { AbstractESSource } from '../es_source';
99
import { esAggFieldsFactory } from '../../fields/es_agg_field';
10-
1110
import {
11+
AGG_DELIMITER,
1212
AGG_TYPE,
1313
COUNT_PROP_LABEL,
1414
COUNT_PROP_NAME,
1515
FIELD_ORIGIN,
1616
} from '../../../../common/constants';
1717

18-
export const AGG_DELIMITER = '_of_';
19-
2018
export class AbstractESAggSource extends AbstractESSource {
2119
constructor(descriptor, inspectorAdapters) {
2220
super(descriptor, inspectorAdapters);

0 commit comments

Comments
 (0)