address comments

Signed-off-by: Su <[email protected]>

CHANGELOG.md

@@ -67,7 +67,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Multiple DataSource] Refactor test connection to support SigV4 auth type ([#3456](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3456))
 - [Darwin] Add support for Darwin for running OpenSearch snapshots with `yarn opensearch snapshot` ([#3537](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3537))
 - [Vis Builder] Add metric to metric, bucket to bucket aggregation persistence ([#3495](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3495))
-- [Multiple DataSource] Prepend data source name to index pattern title in UI to distinguish across data sources ([#3571](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3571))
+- [Multiple DataSource] Allow create and distinguish index pattern with same name but from different datasources ([#3571](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3571))
 
 ### 🐛 Bug Fixes
 

src/plugins/data/common/index_patterns/fields/utils.test.ts

@@ -0,0 +1,71 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Any modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { isFilterable } from '..';
+import { IFieldType } from '.';
+
+const mockField = {
+  name: 'foo',
+  scripted: false,
+  searchable: true,
+  type: 'string',
+} as IFieldType;
+
+describe('isFilterable', () => {
+  describe('types', () => {
+    it('should return true for filterable types', () => {
+      ['string', 'number', 'date', 'ip', 'boolean'].forEach((type) => {
+        expect(isFilterable({ ...mockField, type })).toBe(true);
+      });
+    });
+
+    it('should return false for filterable types if the field is not searchable', () => {
+      ['string', 'number', 'date', 'ip', 'boolean'].forEach((type) => {
+        expect(isFilterable({ ...mockField, type, searchable: false })).toBe(false);
+      });
+    });
+
+    it('should return false for un-filterable types', () => {
+      ['geo_point', 'geo_shape', 'attachment', 'murmur3', '_source', 'unknown', 'conflict'].forEach(
+        (type) => {
+          expect(isFilterable({ ...mockField, type })).toBe(false);
+        }
+      );
+    });
+  });
+
+  it('should return true for scripted fields', () => {
+    expect(isFilterable({ ...mockField, scripted: true, searchable: false })).toBe(true);
+  });
+
+  it('should return true for the _id field', () => {
+    expect(isFilterable({ ...mockField, name: '_id' })).toBe(true);
+  });
+});

src/plugins/data/common/index_patterns/utils.test.ts

@@ -9,63 +9,84 @@
  * GitHub history for details.
  */
 
-/*
- * Licensed to Elasticsearch B.V. under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch B.V. licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-import { isFilterable } from '.';
-import { IFieldType } from './fields';
+import { AuthType, DataSourceAttributes } from 'src/plugins/data_source/common/data_sources';
+import { IndexPatternSavedObjectAttrs } from './index_patterns';
+import { SavedObject, SavedObjectReference } from './types';
+import { getIndexPatternTitle, validateDataSourceReference } from './utils';
 
-const mockField = {
-  name: 'foo',
-  scripted: false,
-  searchable: true,
-  type: 'string',
-} as IFieldType;
+describe('test validateDataSourceReference', () => {
+  const getIndexPatternSavedObjectMock = (mockedFields: any = {}) =>
+    ({ ...mockedFields } as SavedObject<IndexPatternSavedObjectAttrs>);
+  let indexPatternSavedObjectMock;
+  const dataSourceId = 'fakeDataSourceId';
 
-describe('isFilterable', () => {
-  describe('types', () => {
-    it('should return true for filterable types', () => {
-      ['string', 'number', 'date', 'ip', 'boolean'].forEach((type) => {
-        expect(isFilterable({ ...mockField, type })).toBe(true);
-      });
+  test('ivalidateDataSourceReference should return false when datasource reference does not exist in index pattern', () => {
+    indexPatternSavedObjectMock = getIndexPatternSavedObjectMock({
+      references: [{ name: 'someReference' }],
     });
 
-    it('should return false for filterable types if the field is not searchable', () => {
-      ['string', 'number', 'date', 'ip', 'boolean'].forEach((type) => {
-        expect(isFilterable({ ...mockField, type, searchable: false })).toBe(false);
-      });
-    });
+    expect(validateDataSourceReference(indexPatternSavedObjectMock)).toBe(false);
+    expect(validateDataSourceReference(indexPatternSavedObjectMock, dataSourceId)).toBe(false);
+  });
 
-    it('should return false for un-filterable types', () => {
-      ['geo_point', 'geo_shape', 'attachment', 'murmur3', '_source', 'unknown', 'conflict'].forEach(
-        (type) => {
-          expect(isFilterable({ ...mockField, type })).toBe(false);
-        }
-      );
+  test('ivalidateDataSourceReference should return true when datasource reference exists in index pattern, and datasource id matches', () => {
+    indexPatternSavedObjectMock = getIndexPatternSavedObjectMock({
+      references: [{ type: 'data-source', id: dataSourceId }],
     });
+
+    expect(validateDataSourceReference(indexPatternSavedObjectMock)).toBe(false);
+    expect(validateDataSourceReference(indexPatternSavedObjectMock, dataSourceId)).toBe(true);
+  });
+});
+
+describe('test getIndexPatternTitle', () => {
+  const dataSourceMock: SavedObject<DataSourceAttributes> = {
+    id: 'dataSourceId',
+    type: 'data-source',
+    attributes: {
+      title: 'dataSourceMockTitle',
+      endpoint: 'https://fakeendpoint.com',
+      auth: {
+        type: AuthType.NoAuth,
+        credentials: undefined,
+      },
+    },
+    references: [],
+  };
+  const indexPatternMockTitle = 'indexPatternMockTitle';
+  const referencesMock: SavedObjectReference[] = [{ type: 'data-source', id: 'dataSourceId' }];
+
+  let getDataSourceMock: jest.Mock<any, any>;
+
+  beforeEach(() => {
+    getDataSourceMock = jest.fn().mockResolvedValue(dataSourceMock);
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  test('getIndexPatternTitle should concat datasource title with index pattern title', async () => {
+    const res = await getIndexPatternTitle(
+      indexPatternMockTitle,
+      referencesMock,
+      getDataSourceMock
+    );
+    expect(res).toEqual('dataSourceMockTitle.indexPatternMockTitle');
   });
 
-  it('should return true for scripted fields', () => {
-    expect(isFilterable({ ...mockField, scripted: true, searchable: false })).toBe(true);
+  test('getIndexPatternTitle should return index pattern title, when index-pattern is not referenced to any datasource', async () => {
+    const res = await getIndexPatternTitle(indexPatternMockTitle, [], getDataSourceMock);
+    expect(res).toEqual('indexPatternMockTitle');
   });
 
-  it('should return true for the _id field', () => {
-    expect(isFilterable({ ...mockField, name: '_id' })).toBe(true);
+  test('getIndexPatternTitle should return index pattern title, when failing to fetch datasource info', async () => {
+    getDataSourceMock = jest.fn().mockRejectedValue(new Error('error'));
+    const res = await getIndexPatternTitle(
+      indexPatternMockTitle,
+      referencesMock,
+      getDataSourceMock
+    );
+    expect(res).toEqual('dataSourceId.indexPatternMockTitle');
   });
 });

src/plugins/data/common/index_patterns/utils.ts

@@ -64,7 +64,7 @@ export async function findByTitle(
 
 // This is used to validate datasource reference of index pattern
 export const validateDataSourceReference = (
-  indexPattern: SavedObject<any>,
+  indexPattern: SavedObject<IndexPatternSavedObjectAttrs>,
   dataSourceId?: string
 ) => {
   const references = indexPattern.references;
@@ -82,22 +82,25 @@ export const getIndexPatternTitle = async (
   references: SavedObjectReference[],
   getDataSource: (id: string) => Promise<SavedObject<DataSourceAttributes>>
 ): Promise<string> => {
-  const DELIMITER = '.';
+  const DATA_SOURCE_INDEX_PATTERN_DELIMITER = '.';
   let dataSourceTitle;
+  const dataSourceReference = references.find((ref) => ref.type === 'data-source');
+
   // If an index-pattern references datasource, prepend data source name with index pattern name for display purpose
-  if (Array.isArray(references) && references[0] && references[0].type === 'data-source') {
-    const dataSourceId = references[0].id;
+  if (dataSourceReference) {
+    const dataSourceId = dataSourceReference.id;
     try {
       const {
         attributes: { title },
         error,
       } = await getDataSource(dataSourceId);
       dataSourceTitle = error ? dataSourceId : title;
     } catch (e) {
+      // use datasource id as title when failing to fetch datasource
       dataSourceTitle = dataSourceId;
     }
 
-    return dataSourceTitle.concat(DELIMITER).concat(indexPatternTitle);
+    return dataSourceTitle.concat(DATA_SOURCE_INDEX_PATTERN_DELIMITER).concat(indexPatternTitle);
   } else {
     // if index pattern doesn't reference datasource, return as it is.
     return indexPatternTitle;

src/plugins/discover/public/application/components/sidebar/discover_index_pattern.test.tsx

@@ -47,14 +47,14 @@ const indexPattern = {
 const indexPattern1 = {
   id: 'test1',
   attributes: {
-    title: 'test1 title',
+    title: 'test1 titleToDisplay',
   },
 } as SavedObject<any>;
 
 const indexPattern2 = {
   id: 'test2',
   attributes: {
-    title: 'test2 title',
+    title: 'test2 titleToDisplay',
   },
 } as SavedObject<any>;
 
@@ -97,15 +97,15 @@ describe('DiscoverIndexPattern', () => {
     const instance = shallow(<DiscoverIndexPattern {...defaultProps} />);
 
     expect(getIndexPatternPickerOptions(instance)!.map((option: any) => option.label)).toEqual([
-      'test1 title',
-      'test2 title',
+      'test1 titleToDisplay',
+      'test2 titleToDisplay',
     ]);
   });
 
   test('should switch data panel to target index pattern', () => {
     const instance = shallow(<DiscoverIndexPattern {...defaultProps} />);
 
-    selectIndexPatternPickerOption(instance, 'test2 title');
+    selectIndexPatternPickerOption(instance, 'test2 titleToDisplay');
     expect(defaultProps.setIndexPattern).toHaveBeenCalledWith('test2');
   });
 });