[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 <elasticmachine@users.noreply.github.com>
elastic · Apr 17, 2020 · 420ccff · 420ccff
1 parent e3eed0b
commit 420ccff
Show file tree

Hide file tree

Showing 13 changed files with 325 additions and 33 deletions.
diff --git a/x-pack/legacy/plugins/maps/common/get_join_key.ts b/x-pack/legacy/plugins/maps/common/get_join_key.ts
@@ -0,0 +1,8 @@
+/*
+ * 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.
+ */
+
+// eslint-disable-next-line @kbn/eslint/no-restricted-paths
+export * from '../../../../plugins/maps/common/get_join_key';
diff --git a/x-pack/legacy/plugins/maps/common/migrations/join_agg_key.test.ts b/x-pack/legacy/plugins/maps/common/migrations/join_agg_key.test.ts
@@ -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'
+    );
+  });
+});
diff --git a/x-pack/legacy/plugins/maps/common/migrations/join_agg_key.ts b/x-pack/legacy/plugins/maps/common/migrations/join_agg_key.ts
@@ -0,0 +1,121 @@
+/*
+ * 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,
+  JOIN_FIELD_NAME_PREFIX,
+  LAYER_TYPE,
+  VECTOR_STYLES,
+} from '../constants';
+import { getJoinAggKey } from '../get_join_key';
+import {
+  AggDescriptor,
+  JoinDescriptor,
+  LayerDescriptor,
+  VectorLayerDescriptor,
+} 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
+    ) {
+      const vectorLayerDescriptor = layerDescriptor as VectorLayerDescriptor;
+
+      if (
+        !vectorLayerDescriptor.style ||
+        !vectorLayerDescriptor.joins ||
+        vectorLayerDescriptor.joins.length === 0
+      ) {
+        return;
+      }
+
+      const legacyJoinFields = new Map<string, JoinDescriptor>();
+      vectorLayerDescriptor.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(vectorLayerDescriptor.style.properties).forEach(key => {
+        const style: any = vectorLayerDescriptor.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),
+  };
+}
diff --git a/x-pack/legacy/plugins/maps/migrations.js b/x-pack/legacy/plugins/maps/migrations.js
@@ -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: {
@@ -57,5 +58,13 @@ export const migrations = {
         attributes: attributesPhase2,
       };
     },
+    '7.8.0': doc => {
+      const attributes = migrateJoinAggKey(doc);
+
+      return {
+        ...doc,
+        attributes,
+      };
+    },
   },
 };
diff --git a/x-pack/plugins/maps/common/constants.ts b/x-pack/plugins/maps/common/constants.ts
@@ -1,9 +1,3 @@
-/*
- * 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.
- */
-
 /*
  * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
  * or more contributor license agreements. Licensed under the Elastic License;
@@ -75,6 +69,7 @@ export enum FIELD_ORIGIN {
   SOURCE = 'source',
   JOIN = 'join',
 }
+export const JOIN_FIELD_NAME_PREFIX = '__kbnjoin__';
 
 export const SOURCE_DATA_ID_ORIGIN = 'source';
 export const META_ID_ORIGIN_SUFFIX = 'meta';
@@ -132,6 +127,7 @@ export enum DRAW_TYPE {
   POLYGON = 'POLYGON',
 }
 
+export const AGG_DELIMITER = '_of_';
 export enum AGG_TYPE {
   AVG = 'avg',
   COUNT = 'count',

diff --git a/x-pack/plugins/maps/common/get_join_key.ts b/x-pack/plugins/maps/common/get_join_key.ts
@@ -0,0 +1,22 @@
+/*
+ * 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 { AGG_DELIMITER, AGG_TYPE, JOIN_FIELD_NAME_PREFIX } from './constants';
+
+// function in common since its needed by migration
+export function getJoinAggKey({
+  aggType,
+  aggFieldName,
+  rightSourceId,
+}: {
+  aggType: AGG_TYPE;
+  aggFieldName?: string;
+  rightSourceId: string;
+}) {
+  const metricKey =
+    aggType !== AGG_TYPE.COUNT ? `${aggType}${AGG_DELIMITER}${aggFieldName}` : aggType;
+  return `${JOIN_FIELD_NAME_PREFIX}${metricKey}__${rightSourceId}`;
+}
diff --git a/x-pack/plugins/maps/public/layers/joins/inner_join.test.js b/x-pack/plugins/maps/public/layers/joins/inner_join.test.js
@@ -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', () => {

diff --git a/x-pack/plugins/maps/public/layers/sources/es_agg_source/es_agg_source.js b/x-pack/plugins/maps/public/layers/sources/es_agg_source/es_agg_source.js
@@ -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);