-
Notifications
You must be signed in to change notification settings - Fork 917
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
[MDS] Support for Timeline #6385
Changes from all commits
fbbffef
faa2863
ba471b5
140a1c6
4e13978
28edd7f
b4411b3
a274600
e8a41b4
6146194
b3ee293
cff410a
541cbdd
503e77b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,152 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import { savedObjectsClientMock } from '../../../../core/server/mocks'; | ||
import { fetchDataSourceIdByName } from './fetch_data_source_id'; | ||
import { OpenSearchFunctionConfig } from '../types'; | ||
|
||
jest.mock('./services', () => ({ | ||
getDataSourceEnabled: jest | ||
.fn() | ||
.mockReturnValueOnce({ enabled: false }) | ||
.mockReturnValue({ enabled: true }), | ||
})); | ||
|
||
describe('fetchDataSourceIdByName()', () => { | ||
const validId = 'some-valid-id'; | ||
const config: OpenSearchFunctionConfig = { | ||
q: null, | ||
metric: null, | ||
split: null, | ||
index: null, | ||
timefield: null, | ||
kibana: null, | ||
opensearchDashboards: null, | ||
interval: null, | ||
}; | ||
const client = savedObjectsClientMock.create(); | ||
client.find = jest.fn().mockImplementation((props) => { | ||
if (props.search === '"No Results With Filter"') { | ||
return Promise.resolve({ | ||
saved_objects: [ | ||
{ | ||
id: 'some-non-matching-id', | ||
attributes: { | ||
title: 'No Results With Filter Some Suffix', | ||
}, | ||
}, | ||
], | ||
}); | ||
} | ||
if (props.search === '"Duplicate Title"') { | ||
return Promise.resolve({ | ||
saved_objects: [ | ||
{ | ||
id: 'duplicate-id-1', | ||
attributes: { | ||
title: 'Duplicate Title', | ||
}, | ||
}, | ||
{ | ||
id: 'duplicate-id-2', | ||
attributes: { | ||
title: 'Duplicate Title', | ||
}, | ||
}, | ||
], | ||
}); | ||
} | ||
if (props.search === '"Some Data Source"') { | ||
return Promise.resolve({ | ||
saved_objects: [ | ||
{ | ||
id: validId, | ||
attributes: { | ||
title: 'Some Data Source', | ||
}, | ||
}, | ||
], | ||
}); | ||
} | ||
if (props.search === '"Some Prefix"') { | ||
return Promise.resolve({ | ||
saved_objects: [ | ||
{ | ||
id: 'some-id-2', | ||
attributes: { | ||
title: 'Some Prefix B', | ||
}, | ||
}, | ||
{ | ||
id: validId, | ||
attributes: { | ||
title: 'Some Prefix', | ||
}, | ||
}, | ||
], | ||
}); | ||
} | ||
|
||
return Promise.resolve({ saved_objects: [] }); | ||
}); | ||
|
||
it('should return undefined if data_source_name is not present', async () => { | ||
expect(await fetchDataSourceIdByName(config, client)).toBe(undefined); | ||
}); | ||
|
||
it('should return undefined if data_source_name is an empty string', async () => { | ||
expect(await fetchDataSourceIdByName({ ...config, data_source_name: '' }, client)).toBe( | ||
undefined | ||
); | ||
}); | ||
|
||
it('should throw errors when MDS is disabled', async () => { | ||
await expect( | ||
fetchDataSourceIdByName({ ...config, data_source_name: 'Some Data Source' }, client) | ||
).rejects.toThrowError( | ||
'To query from multiple data sources, first enable the data source feature' | ||
); | ||
}); | ||
|
||
it.each([ | ||
{ | ||
dataSourceName: 'Non-existent Data Source', | ||
expectedResultCount: 0, | ||
}, | ||
{ | ||
dataSourceName: 'No Results With Filter', | ||
expectedResultCount: 0, | ||
}, | ||
{ | ||
dataSourceName: 'Duplicate Title', | ||
expectedResultCount: 2, | ||
}, | ||
])( | ||
'should throw errors when non-existent or duplicate data_source_name is provided', | ||
async ({ dataSourceName, expectedResultCount }) => { | ||
await expect( | ||
fetchDataSourceIdByName({ ...config, data_source_name: dataSourceName }, client) | ||
).rejects.toThrowError( | ||
`Expected exactly 1 result for data_source_name "${dataSourceName}" but got ${expectedResultCount} results` | ||
); | ||
} | ||
); | ||
|
||
it.each([ | ||
{ | ||
dataSourceName: 'Some Data Source', | ||
}, | ||
{ | ||
dataSourceName: 'Some Prefix', | ||
}, | ||
])( | ||
'should return valid id when data_source_name exists and is unique', | ||
async ({ dataSourceName }) => { | ||
expect( | ||
await fetchDataSourceIdByName({ ...config, data_source_name: dataSourceName }, client) | ||
).toBe(validId); | ||
} | ||
); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import { SavedObjectsClientContract } from 'src/core/server'; | ||
import { DataSourceAttributes } from 'src/plugins/data_source/common/data_sources'; | ||
import { getDataSourceEnabled } from './services'; | ||
import { OpenSearchFunctionConfig } from '../types'; | ||
|
||
export const fetchDataSourceIdByName = async ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't we define similar function for vega? In vega, we are fetching data source id by name, right? Correct me if I'm missing something. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes but Vega fetches data source id client-side while Timeline fetches data source id information server-side. I agree that this is something that needs to be refactored into one utility function in data source plugin and I cut a new issue #6417 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there an ETA to refactor this function? Just asking because if we don't actively track and then we will keep using these two separate function when they are performing same functionality. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if this is a huge priority at the moment. Most plugins are not blocked by this refactor, but once the UI components are finalized (these are blocking issues), I will pick up this task. This should be started within this sprint or at the most, next sprint. |
||
config: OpenSearchFunctionConfig, | ||
client: SavedObjectsClientContract | ||
) => { | ||
if (!config.data_source_name) { | ||
return undefined; | ||
} | ||
|
||
if (!getDataSourceEnabled().enabled) { | ||
throw new Error('To query from multiple data sources, first enable the data source feature'); | ||
} | ||
|
||
const dataSources = await client.find<DataSourceAttributes>({ | ||
type: 'data-source', | ||
perPage: 100, | ||
search: `"${config.data_source_name}"`, | ||
searchFields: ['title'], | ||
fields: ['id', 'title'], | ||
}); | ||
|
||
const possibleDataSourceIds = dataSources.saved_objects.filter( | ||
(obj) => obj.attributes.title === config.data_source_name | ||
); | ||
|
||
if (possibleDataSourceIds.length !== 1) { | ||
throw new Error( | ||
`Expected exactly 1 result for data_source_name "${config.data_source_name}" but got ${possibleDataSourceIds.length} results` | ||
); | ||
} | ||
|
||
return possibleDataSourceIds.pop()?.id; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import { createGetterSetter } from '../../../opensearch_dashboards_utils/common'; | ||
|
||
export const [getDataSourceEnabled, setDataSourceEnabled] = createGetterSetter<{ | ||
enabled: boolean; | ||
}>('DataSource'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does the name field matter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes: server-side error messages are propagated to the client and the toast error message will show (this is existing behavior and UX is aligned). This is needed to make the toast more readable (stack information is kept hidden from client).