diff --git a/common/constants.ts b/common/constants.ts index d1efcbe8..765265a4 100644 --- a/common/constants.ts +++ b/common/constants.ts @@ -18,10 +18,7 @@ export const GROUP_BY = 'Group by'; export const AVERAGE_LATENCY = 'Average Latency'; export const AVERAGE_CPU_TIME = 'Average CPU Time'; export const AVERAGE_MEMORY_USAGE = 'Average Memory Usage'; -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ + export const MetricType = { LATENCY: 'latency', CPU: 'cpu', @@ -69,3 +66,18 @@ export const DEFAULT_TIME_UNIT = TIME_UNIT.MINUTES; export const DEFAULT_GROUP_BY = 'none'; export const DEFAULT_EXPORTER_TYPE = EXPORTER_TYPE.localIndex; export const DEFAULT_DELETE_AFTER_DAYS = '7'; +// Validation constants +export const VALIDATION_LIMITS = { + TOP_N_SIZE: { + MIN: 1, + MAX: 100, + }, + WINDOW_SIZE_HOURS: { + MIN: 1, + MAX: 24, + }, + DELETE_AFTER_DAYS: { + MIN: 1, + MAX: 180, + }, +}; diff --git a/public/pages/Configuration/Configuration.test.tsx b/public/pages/Configuration/Configuration.test.tsx index d795e8f9..59337d46 100644 --- a/public/pages/Configuration/Configuration.test.tsx +++ b/public/pages/Configuration/Configuration.test.tsx @@ -9,6 +9,13 @@ import '@testing-library/jest-dom/extend-expect'; import { MemoryRouter } from 'react-router-dom'; import Configuration from './Configuration'; import { DataSourceContext } from '../TopNQueries/TopNQueries'; +import { TIME_UNITS_TEXT, EXPORTER_TYPE } from '../../../common/constants'; +import { + validateTopNSize, + validateWindowSize, + validateDeleteAfterDays, + validateConfiguration, +} from './configurationValidation'; const mockConfigInfo = jest.fn(); const mockCoreStart = { @@ -145,4 +152,183 @@ describe('Configuration Component', () => { fireEvent.click(screen.getByText('Cancel')); expect(getTopNSizeConfiguration()[0]).toHaveValue(5); // Resets to initial value }); + + describe('Validation Logic Tests', () => { + describe('TopN Size Validation', () => { + it('should hide Save button when topN size is less than 1', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '0' } }); + expect(screen.queryByText('Save')).not.toBeInTheDocument(); + }); + + it('should hide Save button when topN size is greater than 100', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '101' } }); + expect(screen.queryByText('Save')).not.toBeInTheDocument(); + }); + + it('should show Save button when topN size is within valid range (1-100)', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '50' } }); + expect(screen.getByText('Save')).toBeInTheDocument(); + }); + }); + + describe('Window Size Validation - Minutes', () => { + it('should hide Save button when window size is empty for minutes', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } }); + fireEvent.change(getWindowSizeConfigurations()[2], { target: { value: 'MINUTES' } }); + fireEvent.change(getWindowSizeConfigurations()[1], { target: { value: '' } }); + expect(screen.queryByText('Save')).not.toBeInTheDocument(); + }); + + it('should hide Save button when window size is NaN for minutes', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } }); + fireEvent.change(getWindowSizeConfigurations()[2], { target: { value: 'MINUTES' } }); + fireEvent.change(getWindowSizeConfigurations()[1], { target: { value: 'invalid' } }); + expect(screen.queryByText('Save')).not.toBeInTheDocument(); + }); + + it('should show Save button when window size is valid for minutes', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } }); + fireEvent.change(getWindowSizeConfigurations()[2], { target: { value: 'MINUTES' } }); + fireEvent.change(getWindowSizeConfigurations()[1], { target: { value: '5' } }); + expect(screen.getByText('Save')).toBeInTheDocument(); + }); + }); + + describe('Window Size Validation - Hours', () => { + it('should show Save button when window size is within valid range (1-24) for hours', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } }); + fireEvent.change(getWindowSizeConfigurations()[2], { target: { value: 'HOURS' } }); + fireEvent.change(getWindowSizeConfigurations()[1], { target: { value: '12' } }); + expect(screen.getByText('Save')).toBeInTheDocument(); + }); + }); + + describe('Delete After Days Validation', () => { + it('should hide Save button when delete after days is less than 1 for local index', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } }); + const deleteAfterField = getTopNSizeConfiguration()[1]; + fireEvent.change(deleteAfterField, { target: { value: '0' } }); + expect(screen.queryByText('Save')).not.toBeInTheDocument(); + }); + + it('should hide Save button when delete after days is greater than 180 for local index', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } }); + const deleteAfterField = getTopNSizeConfiguration()[1]; + fireEvent.change(deleteAfterField, { target: { value: '181' } }); + expect(screen.queryByText('Save')).not.toBeInTheDocument(); + }); + + it('should show Save button when delete after days is within valid range (1-180) for local index', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } }); + const deleteAfterField = getTopNSizeConfiguration()[1]; + fireEvent.change(deleteAfterField, { target: { value: '90' } }); + expect(screen.getByText('Save')).toBeInTheDocument(); + }); + + it('should show Save button when exporter is changed to none', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '6' } }); + fireEvent.change(screen.getByDisplayValue('Local Index'), { target: { value: 'none' } }); + expect(screen.getByText('Save')).toBeInTheDocument(); + }); + }); + + describe('Combined Validation Scenarios', () => { + it('should hide Save button when multiple validation rules fail', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '101' } }); + expect(screen.queryByText('Save')).not.toBeInTheDocument(); + }); + + it('should show Save button when all validation rules pass', () => { + renderConfiguration(); + fireEvent.change(getTopNSizeConfiguration()[0], { target: { value: '25' } }); + expect(screen.getByText('Save')).toBeInTheDocument(); + }); + }); + }); + + describe('Validation Utility Functions', () => { + describe('validateTopNSize', () => { + it('should return false for values less than 1', () => { + expect(validateTopNSize('0')).toBe(false); + expect(validateTopNSize('-1')).toBe(false); + }); + + it('should return false for values greater than 100', () => { + expect(validateTopNSize('101')).toBe(false); + expect(validateTopNSize('200')).toBe(false); + }); + + it('should return true for valid values (1-100)', () => { + expect(validateTopNSize('1')).toBe(true); + expect(validateTopNSize('50')).toBe(true); + expect(validateTopNSize('100')).toBe(true); + }); + + it('should return false for non-numeric strings', () => { + expect(validateTopNSize('abc')).toBe(false); + expect(validateTopNSize('')).toBe(false); + expect(validateTopNSize('10.5')).toBe(false); + }); + }); + + describe('validateWindowSize', () => { + it('should validate minutes correctly', () => { + const minutesUnit = TIME_UNITS_TEXT[0].value; + expect(validateWindowSize('', minutesUnit)).toBe(false); + expect(validateWindowSize('abc', minutesUnit)).toBe(false); + expect(validateWindowSize('1', minutesUnit)).toBe(true); + expect(validateWindowSize('30', minutesUnit)).toBe(true); + }); + + it('should validate hours correctly', () => { + const hoursUnit = TIME_UNITS_TEXT[1].value; + expect(validateWindowSize('0', hoursUnit)).toBe(false); + expect(validateWindowSize('25', hoursUnit)).toBe(false); + expect(validateWindowSize('1', hoursUnit)).toBe(true); + expect(validateWindowSize('24', hoursUnit)).toBe(true); + }); + }); + + describe('validateDeleteAfterDays', () => { + it('should validate local index correctly', () => { + const localIndexType = EXPORTER_TYPE.localIndex; + expect(validateDeleteAfterDays('0', localIndexType)).toBe(false); + expect(validateDeleteAfterDays('181', localIndexType)).toBe(false); + expect(validateDeleteAfterDays('1', localIndexType)).toBe(true); + expect(validateDeleteAfterDays('180', localIndexType)).toBe(true); + }); + + it('should return true for non-local index', () => { + const noneType = EXPORTER_TYPE.none; + expect(validateDeleteAfterDays('0', noneType)).toBe(true); + expect(validateDeleteAfterDays('abc', noneType)).toBe(true); + }); + }); + + describe('validateConfiguration', () => { + it('should return false when any validation fails', () => { + expect(validateConfiguration('101', '5', 'MINUTES', '30', EXPORTER_TYPE.localIndex)).toBe( + false + ); + expect(validateConfiguration('50', '', 'MINUTES', '30', EXPORTER_TYPE.localIndex)).toBe( + false + ); + expect(validateConfiguration('50', '5', 'MINUTES', '181', EXPORTER_TYPE.localIndex)).toBe( + false + ); + }); + }); + }); }); diff --git a/public/pages/Configuration/Configuration.tsx b/public/pages/Configuration/Configuration.tsx index 79f9079e..108b827c 100644 --- a/public/pages/Configuration/Configuration.tsx +++ b/public/pages/Configuration/Configuration.tsx @@ -43,6 +43,7 @@ import { } from '../../../common/constants'; import { QueryInsightsDataSourceMenu } from '../../components/DataSourcePicker'; import { QueryInsightsDashboardsPluginStartDependencies } from '../../types'; +import { validateConfiguration } from './configurationValidation'; const Configuration = ({ latencySettings, @@ -198,6 +199,9 @@ const Configuration = ({ ); const WindowChoice = time === TIME_UNITS_TEXT[0].value ? MinutesBox : HoursBox; + const isLocalIndex = exporterType === EXPORTER_TYPE.localIndex; + const parsedDeleteAfter = parseInt(deleteAfterDays, 10); + const isDeleteAfterValid = !isLocalIndex || (parsedDeleteAfter >= 1 && parsedDeleteAfter <= 180); const isChanged = isEnabled !== metricSettingsMap[metric].isEnabled || @@ -208,13 +212,7 @@ const Configuration = ({ exporterType !== dataRetentionSettingMap.dataRetention.exporterType || deleteAfterDays !== dataRetentionSettingMap.dataRetention.deleteAfterDays; - const isValid = (() => { - const nVal = parseInt(topNSize, 10); - if (nVal < 1 || nVal > 100) return false; - if (time === TIME_UNITS_TEXT[0].value) return true; - const windowVal = parseInt(windowSize, 10); - return windowVal >= 1 && windowVal <= 24; - })(); + const isValid = validateConfiguration(topNSize, windowSize, time, deleteAfterDays, exporterType); const formRowPadding = { padding: '0px 0px 20px' }; const enabledSymb = Enabled; @@ -505,13 +503,25 @@ const Configuration = ({ /> - + diff --git a/public/pages/Configuration/__snapshots__/Configuration.test.tsx.snap b/public/pages/Configuration/__snapshots__/Configuration.test.tsx.snap index 9d484aeb..e2e640f8 100644 --- a/public/pages/Configuration/__snapshots__/Configuration.test.tsx.snap +++ b/public/pages/Configuration/__snapshots__/Configuration.test.tsx.snap @@ -962,6 +962,7 @@ exports[`Configuration Component renders with default settings: should match def class="euiFormControlLayout__childrenWrapper" > +
+ Max allowed limit 180. +
@@ -1991,6 +1998,7 @@ exports[`Configuration Component updates state when toggling metrics and enables class="euiFormControlLayout__childrenWrapper" > +
+ Max allowed limit 180. +
diff --git a/public/pages/Configuration/configurationValidation.ts b/public/pages/Configuration/configurationValidation.ts new file mode 100644 index 00000000..ac0f8ccd --- /dev/null +++ b/public/pages/Configuration/configurationValidation.ts @@ -0,0 +1,55 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { TIME_UNITS_TEXT, EXPORTER_TYPE, VALIDATION_LIMITS } from '../../../common/constants'; + +export const validateTopNSize = (topNSize: string): boolean => { + const nVal = parseInt(topNSize, 10); + if (topNSize !== nVal.toString() || Number.isNaN(nVal)) return false; + return nVal >= VALIDATION_LIMITS.TOP_N_SIZE.MIN && nVal <= VALIDATION_LIMITS.TOP_N_SIZE.MAX; +}; + +export const validateWindowSize = (windowSize: string, timeUnit: string): boolean => { + if (timeUnit === TIME_UNITS_TEXT[0].value) { + // MINUTES + const windowVal = parseInt(windowSize, 10); + return windowSize !== '' && !Number.isNaN(windowVal) && windowSize === windowVal.toString(); + } else { + // HOURS + const windowVal = parseInt(windowSize, 10); + if (windowSize !== windowVal.toString() || Number.isNaN(windowVal)) return false; + return ( + windowVal >= VALIDATION_LIMITS.WINDOW_SIZE_HOURS.MIN && + windowVal <= VALIDATION_LIMITS.WINDOW_SIZE_HOURS.MAX + ); + } +}; + +export const validateDeleteAfterDays = (deleteAfterDays: string, exporterType: string): boolean => { + const isLocalIndex = exporterType === EXPORTER_TYPE.localIndex; + if (!isLocalIndex) return true; + + const parsedDeleteAfter = parseInt(deleteAfterDays, 10); + if (deleteAfterDays !== parsedDeleteAfter.toString() || Number.isNaN(parsedDeleteAfter)) + return false; + return ( + parsedDeleteAfter >= VALIDATION_LIMITS.DELETE_AFTER_DAYS.MIN && + parsedDeleteAfter <= VALIDATION_LIMITS.DELETE_AFTER_DAYS.MAX + ); +}; + +export const validateConfiguration = ( + topNSize: string, + windowSize: string, + timeUnit: string, + deleteAfterDays: string, + exporterType: string +): boolean => { + return ( + validateTopNSize(topNSize) && + validateWindowSize(windowSize, timeUnit) && + validateDeleteAfterDays(deleteAfterDays, exporterType) + ); +}; diff --git a/public/pages/TopNQueries/TopNQueries.test.tsx b/public/pages/TopNQueries/TopNQueries.test.tsx index 37734171..b10e69e6 100644 --- a/public/pages/TopNQueries/TopNQueries.test.tsx +++ b/public/pages/TopNQueries/TopNQueries.test.tsx @@ -224,6 +224,120 @@ describe('TopNQueries Component', () => { }); }); + describe('Group by settings extraction', () => { + it('should extract group_by from persistent settings', async () => { + const mockSettingsResponse = { + response: { + persistent: { + search: { + insights: { + top_queries: { + latency: { enabled: 'true', top_n_size: '10', window_size: '1h' }, + grouping: { group_by: 'similarity' }, + }, + }, + }, + }, + }, + }; + (mockCore.http.get as jest.Mock).mockResolvedValueOnce(mockSettingsResponse); + + renderTopNQueries(CONFIGURATION); + + await waitFor(() => { + expect(mockCore.http.get).toHaveBeenCalledWith('/api/settings', { + query: { dataSourceId: undefined }, + }); + }); + }); + + it('should extract group_by from transient settings when both exist', async () => { + const mockSettingsResponse = { + response: { + persistent: { + search: { + insights: { + top_queries: { + latency: { enabled: 'true', top_n_size: '10', window_size: '1h' }, + grouping: { group_by: 'similarity' }, + }, + }, + }, + }, + transient: { + search: { + insights: { + top_queries: { + grouping: { group_by: 'none' }, + }, + }, + }, + }, + }, + }; + (mockCore.http.get as jest.Mock).mockResolvedValueOnce(mockSettingsResponse); + + renderTopNQueries(CONFIGURATION); + + await waitFor(() => { + expect(mockCore.http.get).toHaveBeenCalledWith('/api/settings', { + query: { dataSourceId: undefined }, + }); + }); + }); + + it('should use default group_by when neither persistent nor transient settings exist', async () => { + const mockSettingsResponse = { + response: { + persistent: { + search: { + insights: { + top_queries: { + latency: { enabled: 'true', top_n_size: '10', window_size: '1h' }, + }, + }, + }, + }, + }, + }; + (mockCore.http.get as jest.Mock).mockResolvedValueOnce(mockSettingsResponse); + + renderTopNQueries(CONFIGURATION); + + await waitFor(() => { + expect(mockCore.http.get).toHaveBeenCalledWith('/api/settings', { + query: { dataSourceId: undefined }, + }); + }); + }); + + it('should handle missing grouping object gracefully', async () => { + const mockSettingsResponse = { + response: { + persistent: { + search: { + insights: { + top_queries: { + latency: { enabled: 'true', top_n_size: '10', window_size: '1h' }, + // No grouping object + }, + }, + }, + }, + }, + }; + (mockCore.http.get as jest.Mock).mockResolvedValueOnce(mockSettingsResponse); + + renderTopNQueries(CONFIGURATION); + + await waitFor(() => { + expect(mockCore.http.get).toHaveBeenCalledWith('/api/settings', { + query: { dataSourceId: undefined }, + }); + }); + }); + }); + describe('Query deduplication', () => { it('should deduplicate queries by ID from multiple metric endpoints', async () => { // Setup: Create queries with duplicate IDs but different measurements