[BUG] Allow workspace-specific datasets with same title by using UUID (#10899)

* [BUG] Allow workspace-specific datasets with same title by using UUID generation

Signed-off-by: Anan Zhuang <[email protected]>

* fix PR comment

Signed-off-by: Anan Zhuang <[email protected]>

* Changeset file for PR #10899 created/updated

---------

Signed-off-by: Anan Zhuang <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

Author: ananzh

changelogs/fragments/10899.yml

@@ -0,0 +1,2 @@
+fix:
+- [BUG] Allow workspace-specific datasets with same title by using UUID ([#10899](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/10899))

src/plugins/data/public/query/query_string/dataset_service/dataset_service.test.ts

@@ -207,7 +207,7 @@ describe('DatasetService', () => {
     );
   });
 
-  test('saveDataset creates and saves a new dataset', async () => {
+  test('saveDataset creates and saves a new dataset without data source', async () => {
     const mockDataset = {
       id: 'test-dataset',
       title: 'Test Dataset',
@@ -216,10 +216,15 @@ describe('DatasetService', () => {
       type: mockType.id,
       timeFieldName: 'timestamp',
       schemaMappings: { otelLogs: { spanId: 'span.id' } },
+      // No dataSource property
     } as Dataset;
 
+    const mockCreatedDataView = {
+      id: 'generated-uuid-1234',
+    };
+
     const mockDataViews = {
-      createAndSave: jest.fn().mockResolvedValue({}),
+      createAndSave: jest.fn().mockResolvedValue(mockCreatedDataView),
     };
 
     const servicesWithDataViews = {
@@ -233,19 +238,84 @@ describe('DatasetService', () => {
     service.registerType(mockType);
     await service.saveDataset(mockDataset, servicesWithDataViews, 'metrics');
 
+    // Should call createAndSave without the ID to allow UUID generation
     expect(mockDataViews.createAndSave).toHaveBeenCalledWith(
       expect.objectContaining({
-        id: 'test-dataset',
+        id: undefined, // Should be undefined for datasets without data source
+        title: 'Test Dataset',
+        displayName: 'My Dataset',
+        description: 'Test description',
+        timeFieldName: 'timestamp',
+        signalType: 'metrics',
+        schemaMappings: { otelLogs: { spanId: 'span.id' } },
+      }),
+      undefined,
+      false
+    );
+
+    // Should update the dataset with the generated UUID
+    expect(mockDataset.id).toBe('generated-uuid-1234');
+  });
+
+  test('saveDataset creates and saves a new dataset with data source', async () => {
+    const mockDataset = {
+      id: 'test-dataset',
+      title: 'Test Dataset',
+      displayName: 'My Dataset',
+      description: 'Test description',
+      type: mockType.id,
+      timeFieldName: 'timestamp',
+      schemaMappings: { otelLogs: { spanId: 'span.id' } },
+      dataSource: {
+        id: 'data-source-123',
+        title: 'My Data Source',
+        type: 'OpenSearch',
+        version: '1.0',
+      },
+    } as Dataset;
+
+    const mockCreatedDataView = {
+      id: 'data-source-123::generated-uuid-5678',
+    };
+
+    const mockDataViews = {
+      createAndSave: jest.fn().mockResolvedValue(mockCreatedDataView),
+    };
+
+    const servicesWithDataViews = {
+      ...mockDataPluginServices,
+      data: {
+        ...dataPluginMock.createStartContract(),
+        dataViews: mockDataViews as any,
+      },
+    };
+
+    service.registerType(mockType);
+    await service.saveDataset(mockDataset, servicesWithDataViews, 'metrics');
+
+    // Should call createAndSave with data source prefixed ID
+    expect(mockDataViews.createAndSave).toHaveBeenCalledWith(
+      expect.objectContaining({
+        id: expect.stringMatching(/^data-source-123::[0-9a-f-]{36}$/), // Should match data-source-id::uuid pattern
         title: 'Test Dataset',
         displayName: 'My Dataset',
         description: 'Test description',
         timeFieldName: 'timestamp',
         signalType: 'metrics',
         schemaMappings: { otelLogs: { spanId: 'span.id' } },
+        dataSourceRef: {
+          id: 'data-source-123',
+          name: 'My Data Source',
+          type: 'OpenSearch',
+          version: '1.0',
+        },
       }),
       undefined,
       false
     );
+
+    // Should update the dataset with the generated UUID
+    expect(mockDataset.id).toBe('data-source-123::generated-uuid-5678');
   });
 
   test('saveDataset does not save index pattern datasets', async () => {
@@ -489,5 +559,59 @@ describe('DatasetService', () => {
         'Failed to save dataset'
       );
     });
+
+    test('does not update dataset ID when createAndSave returns undefined', async () => {
+      const originalId = 'test-dataset';
+      const mockDataset = {
+        id: originalId,
+        title: 'Test Dataset',
+        type: mockType.id,
+      } as Dataset;
+
+      const mockDataViews = {
+        createAndSave: jest.fn().mockResolvedValue(undefined),
+      };
+
+      const servicesWithDataViews = {
+        ...mockDataPluginServices,
+        data: {
+          ...dataPluginMock.createStartContract(),
+          dataViews: mockDataViews as any,
+        },
+      };
+
+      service.registerType(mockType);
+      await service.saveDataset(mockDataset, servicesWithDataViews);
+
+      // Dataset ID should remain unchanged when no ID is returned
+      expect(mockDataset.id).toBe(originalId);
+    });
+
+    test('does not update dataset ID when createAndSave returns object without ID', async () => {
+      const originalId = 'test-dataset';
+      const mockDataset = {
+        id: originalId,
+        title: 'Test Dataset',
+        type: mockType.id,
+      } as Dataset;
+
+      const mockDataViews = {
+        createAndSave: jest.fn().mockResolvedValue({}),
+      };
+
+      const servicesWithDataViews = {
+        ...mockDataPluginServices,
+        data: {
+          ...dataPluginMock.createStartContract(),
+          dataViews: mockDataViews as any,
+        },
+      };
+
+      service.registerType(mockType);
+      await service.saveDataset(mockDataset, servicesWithDataViews);
+
+      // Dataset ID should remain unchanged when no ID is returned
+      expect(mockDataset.id).toBe(originalId);
+    });
   });
 });

src/plugins/data/public/query/query_string/dataset_service/dataset_service.ts

@@ -3,6 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { v4 as uuidv4 } from 'uuid';
 import LRUCache from 'lru-cache';
 import { CoreStart } from 'opensearch-dashboards/public';
 import {
@@ -188,7 +189,8 @@ export class DatasetService {
           ? ({} as IndexPatternFieldMap)
           : await type?.fetchFields(dataset, services);
         const spec = {
-          id: dataset.id,
+          // Generate ID with data source prefix if data source exists, otherwise allow UUID generation
+          id: dataset.dataSource?.id ? `${dataset.dataSource.id}::${uuidv4()}` : undefined,
           displayName: dataset.displayName,
           title: dataset.title,
           timeFieldName: dataset.timeFieldName,
@@ -214,7 +216,16 @@ export class DatasetService {
         // Consider fetching fields after createAndSave and updating the saved object:
         //   const dataView = await createAndSave(...);
         //   if (asyncType) { await type.fetchFields(...); await dataViews.updateSavedObject(dataView); }
-        await services.data?.dataViews.createAndSave(spec, undefined, asyncType);
+        const createdDataView = await services.data?.dataViews.createAndSave(
+          spec,
+          undefined,
+          asyncType
+        );
+
+        // Update the dataset with the new UUID generated during save
+        if (createdDataView?.id) {
+          dataset.id = createdDataView.id;
+        }
       }
     } catch (error) {
       // Re-throw DuplicateDataViewError without wrapping to preserve error type