Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/unreleased/issue-15957.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type = "fixed"
message = "Fix timerange validation not running when loading a saved search."

issues = ["15957"]
pulls = ["25585"]
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
import org.graylog.plugins.views.search.Search;
import org.graylog.plugins.views.search.SearchType;
import org.graylog.plugins.views.search.engine.SearchConfig;
import org.graylog.plugins.views.search.errors.QueryError;
import org.graylog.plugins.views.search.errors.QueryTimeRangeLimitError;
import org.graylog.plugins.views.search.errors.SearchError;
import org.graylog.plugins.views.search.errors.SearchTypeError;
import org.graylog.plugins.views.search.errors.SearchTypeTimeRangeLimitError;
import org.graylog.plugins.views.search.permissions.SearchUser;
import org.graylog.plugins.views.search.searchtypes.DataLakeSearchType;
import org.graylog2.plugin.indexer.searches.timeranges.TimeRange;
Expand All @@ -49,7 +49,7 @@ private Stream<SearchError> validateQueryTimeRange(Query query, SearchConfig con
.flatMap(timeRangeLimit -> Optional.ofNullable(query.timerange())
.filter(tr -> tr.getFrom() != null && tr.getTo() != null) // TODO: is this check necessary?
.filter(tr -> isOutOfLimit(tr, timeRangeLimit)))
.map(tr -> new QueryError(query, "Search out of allowed time range limit", true));
.map(tr -> new QueryTimeRangeLimitError(query, "Search out of allowed time range limit", true));

final Stream<SearchError> searchTypeErrors = query.searchTypes()
.stream()
Expand All @@ -58,12 +58,17 @@ private Stream<SearchError> validateQueryTimeRange(Query query, SearchConfig con
return Stream.concat(queryError.map(Stream::of).orElseGet(Stream::empty), searchTypeErrors);
}

private Optional<SearchTypeError> validateSearchType(Query query, SearchType searchType, SearchConfig searchConfig) {
private Optional<SearchTypeTimeRangeLimitError> validateSearchType(Query query, SearchType searchType, SearchConfig searchConfig) {
return searchConfig.getQueryTimeRangeLimit()
.flatMap(configuredTimeLimit -> searchType.timerange() // TODO: what if there is no timerange for the type but there is a global limit?
.map(tr -> tr.effectiveTimeRange(query, searchType))
.filter(tr -> isOutOfLimit(tr, configuredTimeLimit))
.map(tr -> new SearchTypeError(query, searchType.id(), "Search type '" + searchType.type() + "' out of allowed time range limit", true)));
.map(tr -> new SearchTypeTimeRangeLimitError(
query,
searchType.id(),
"Search type '" + searchType.type() + "' out of allowed time range limit",
true
)));
}

boolean isOutOfLimit(TimeRange timeRange, Period limit) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (C) 2020 Graylog, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the Server Side Public License, version 1,
* as published by MongoDB, Inc.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* Server Side Public License for more details.
*
* You should have received a copy of the Server Side Public License
* along with this program. If not, see
* <http://www.mongodb.com/licensing/server-side-public-license>.
*/
package org.graylog.plugins.views.search.errors;

import org.graylog.plugins.views.search.Query;

import javax.annotation.Nonnull;

public class QueryTimeRangeLimitError extends QueryError {
public QueryTimeRangeLimitError(@Nonnull Query query, @Nonnull String description, boolean fatal) {
super(query, description, fatal);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@

@JsonSubTypes({
@JsonSubTypes.Type(name = "query", value = QueryError.class),
@JsonSubTypes.Type(name = "query_time_range_limit", value = QueryTimeRangeLimitError.class),
@JsonSubTypes.Type(name = "search_type", value = SearchTypeError.class),
@JsonSubTypes.Type(name = "search_type_time_range_limit", value = SearchTypeTimeRangeLimitError.class),
@JsonSubTypes.Type(name = "unbound_parameter", value = UnboundParameterError.class),
@JsonSubTypes.Type(name = "result_window_limit", value = ResultWindowLimitError.class),
@JsonSubTypes.Type(name = "search_type_aborted", value = SearchTypeAbortedError.class),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (C) 2020 Graylog, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the Server Side Public License, version 1,
* as published by MongoDB, Inc.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* Server Side Public License for more details.
*
* You should have received a copy of the Server Side Public License
* along with this program. If not, see
* <http://www.mongodb.com/licensing/server-side-public-license>.
*/
package org.graylog.plugins.views.search.errors;

import org.graylog.plugins.views.search.Query;

import javax.annotation.Nonnull;

public class SearchTypeTimeRangeLimitError extends SearchTypeError {
public SearchTypeTimeRangeLimitError(@Nonnull Query query, @Nonnull String searchTypeId, @Nonnull String description, boolean fatal) {
super(query, searchTypeId, description, fatal);
}
}
2 changes: 2 additions & 0 deletions graylog2-web-interface/src/views/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,5 @@ export const keySeparator = '\u2E31';
export const humanSeparator = '-';
export const thresholdsSupportedVisualizations = ['bar', 'area', 'line', 'scatter'];
export const multipleValuesActionsSupportedVisualizations = ['bar', 'area', 'line', 'scatter'];
export const QUERY_TIME_RANGE_LIMIT_ERROR_TYPE = 'query_time_range_limit';
export const SEARCH_TYPE_RANGE_LIMIT_ERROR_TYPE = 'search_type_time_range_limit';
43 changes: 43 additions & 0 deletions graylog2-web-interface/src/views/components/SearchBar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import useViewsPlugin from 'views/test/testViewsPlugin';
import TestStoreProvider from 'views/test/TestStoreProvider';
import useViewsDispatch from 'views/stores/useViewsDispatch';
import useSearchConfiguration from 'hooks/useSearchConfiguration';
import useSearchResultTimeRangeErrorCheck from 'views/hooks/useSearchResultTimeRangeErrorCheck';

import OriginalSearchBar from './SearchBar';

Expand Down Expand Up @@ -58,6 +59,7 @@ jest.mock('views/logic/debounceWithPromise', () => (fn: any) => fn);
jest.mock('views/logic/queries/useCurrentQuery');
jest.mock('views/stores/useViewsDispatch');
jest.mock('views/hooks/useAutoRefresh');
jest.mock('views/hooks/useSearchResultTimeRangeErrorCheck');

const query = MockQuery.builder()
.timerange({ type: 'relative', from: 300 })
Expand All @@ -81,6 +83,7 @@ describe('SearchBar', () => {
isInitialLoading: false,
});
asMock(useCurrentQuery).mockReturnValue(query);
asMock(useSearchResultTimeRangeErrorCheck).mockReturnValue(() => false);
});

it('should render the SearchBar', async () => {
Expand Down Expand Up @@ -174,4 +177,44 @@ describe('SearchBar', () => {

await waitFor(() => expect(validateQuery).toHaveBeenCalled());
});

it('shows warning icon on timerange button when search result timerange check returns true', async () => {
asMock(useSearchResultTimeRangeErrorCheck).mockReturnValue(() => true);

render(<SearchBar />);

const timeRangePickerButton = await screen.findByLabelText('Open Time Range Selector');

await waitFor(() => expect(within(timeRangePickerButton).getByText('warning')).toBeInTheDocument());
});

it('disables the search button when search result timerange check returns true', async () => {
asMock(useSearchResultTimeRangeErrorCheck).mockReturnValue(() => true);

render(<SearchBar />);

const searchButton = await screen.findByRole('button', { name: /perform search/i });

await waitFor(() => expect(searchButton.classList).toContain('disabled'));
});

it('does not show warning icon on timerange button when search result timerange check returns false', async () => {
asMock(useSearchResultTimeRangeErrorCheck).mockReturnValue(() => false);

render(<SearchBar />);

const timeRangePickerButton = await screen.findByLabelText('Open Time Range Selector');

expect(within(timeRangePickerButton).queryByText('warning')).not.toBeInTheDocument();
});

it('does not disable the search button when search result timerange check returns false', async () => {
asMock(useSearchResultTimeRangeErrorCheck).mockReturnValue(() => false);

render(<SearchBar />);

const searchButton = await screen.findByRole('button', { name: /perform search/i });

await waitFor(() => expect(searchButton.classList).not.toContain('disabled'));
});
});
10 changes: 8 additions & 2 deletions graylog2-web-interface/src/views/components/SearchBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
newFiltersForQuery,
} from 'views/logic/queries/Query';
import type { SearchBarFormValues } from 'views/Constants';
import { QUERY_TIME_RANGE_LIMIT_ERROR_TYPE } from 'views/Constants';
import WidgetFocusContext from 'views/components/contexts/WidgetFocusContext';
import FormWarningsContext from 'contexts/FormWarningsContext';
import FormWarningsProvider from 'contexts/FormWarningsProvider';
Expand Down Expand Up @@ -77,6 +78,7 @@ import StreamCategoryFilter from 'views/components/searchbar/StreamCategoryFilte
import useAutoRefresh from 'views/hooks/useAutoRefresh';
import useViewsSelector from 'views/stores/useViewsSelector';
import { selectCurrentQueryResults } from 'views/logic/slices/viewSelectors';
import useSearchResultTimeRangeErrorCheck from 'views/hooks/useSearchResultTimeRangeErrorCheck';

import SearchBarForm from './searchbar/SearchBarForm';

Expand Down Expand Up @@ -211,6 +213,8 @@ const SearchBar = ({ onSubmit = defaultProps.onSubmit, scrollContainer }: Props)
const handlerContext = useHandlerContext();
const isLoadingExecution = useIsLoading();

const searchResultTimeRangeErrorCheck = useSearchResultTimeRangeErrorCheck(QUERY_TIME_RANGE_LIMIT_ERROR_TYPE);

if (!currentQuery || !config) {
return <Spinner />;
}
Expand Down Expand Up @@ -240,7 +244,9 @@ const SearchBar = ({ onSubmit = defaultProps.onSubmit, scrollContainer }: Props)
setFieldValue,
validateForm,
}) => {
const disableSearchSubmit = isSubmitting || isValidating || !isValid || isLoadingExecution;
const showTimeRangeErrorFromResults = searchResultTimeRangeErrorCheck(values?.timerange);
const disableSearchSubmit =
isSubmitting || isValidating || !isValid || isLoadingExecution || showTimeRangeErrorFromResults;

return (
<>
Expand All @@ -256,7 +262,7 @@ const SearchBar = ({ onSubmit = defaultProps.onSubmit, scrollContainer }: Props)
limitDuration={limitDuration}
onChange={(nextTimeRange) => setFieldValue('timerange', nextTimeRange)}
value={values?.timerange}
hasErrorOnMount={!!errors.timerange}
hasErrorOnMount={!!errors.timerange || showTimeRangeErrorFromResults}
moveRangeProps={{
effectiveTimerange: results?.effectiveTimerange,
initialTimerange: currentQuery.timerange,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* <http://www.mongodb.com/licensing/server-side-public-license>.
*/
import * as React from 'react';
import { render, waitFor, screen } from 'wrappedTestingLibrary';
import { render, waitFor, screen, within } from 'wrappedTestingLibrary';
import userEvent from '@testing-library/user-event';

import GlobalOverride from 'views/logic/search/GlobalOverride';
Expand All @@ -27,6 +27,7 @@ import { asMock } from 'helpers/mocking';
import useGlobalOverride from 'views/hooks/useGlobalOverride';
import { setGlobalOverrideTimerange, setGlobalOverrideQuery } from 'views/logic/slices/searchExecutionSlice';
import { executeActiveQuery } from 'views/logic/slices/viewSlice';
import useSearchResultTimeRangeErrorCheck from 'views/hooks/useSearchResultTimeRangeErrorCheck';

import WidgetQueryControls from './WidgetQueryControls';
import WidgetContext from './contexts/WidgetContext';
Expand All @@ -38,6 +39,7 @@ jest.mock('views/components/searchbar/queryinput/QueryInput');
jest.mock('views/components/searchbar/queryinput/BasicQueryInput');
jest.mock('views/logic/fieldtypes/useFieldTypes');
jest.mock('views/hooks/useGlobalOverride');
jest.mock('views/hooks/useSearchResultTimeRangeErrorCheck');

jest.mock('views/logic/slices/searchExecutionSlice', () => ({
...jest.requireActual('views/logic/slices/searchExecutionSlice'),
Expand All @@ -54,6 +56,7 @@ describe('WidgetQueryControls', () => {
beforeEach(() => {
jest.clearAllMocks();
asMock(useGlobalOverride).mockReturnValue(GlobalOverride.empty());
asMock(useSearchResultTimeRangeErrorCheck).mockReturnValue(() => false);
});

useViewsPlugin();
Expand Down Expand Up @@ -199,4 +202,44 @@ describe('WidgetQueryControls', () => {
expect(screen.queryByText(timeRangeOverrideInfo)).toBeNull();
});
});

it('shows warning icon on timerange button when search result timerange check returns true', async () => {
asMock(useSearchResultTimeRangeErrorCheck).mockReturnValue(() => true);

renderSUT();

const timeRangePickerButton = await screen.findByLabelText('Open Time Range Selector');

await waitFor(() => expect(within(timeRangePickerButton).getByText('warning')).toBeInTheDocument());
});

it('disables the search button when search result timerange check returns true', async () => {
asMock(useSearchResultTimeRangeErrorCheck).mockReturnValue(() => true);

renderSUT();

const searchButton = await screen.findByRole('button', { name: /perform search/i });

await waitFor(() => expect(searchButton.classList).toContain('disabled'));
});

it('does not show warning icon on timerange button when search result timerange check returns false', async () => {
asMock(useSearchResultTimeRangeErrorCheck).mockReturnValue(() => false);

renderSUT();

const timeRangePickerButton = await screen.findByLabelText('Open Time Range Selector');

expect(within(timeRangePickerButton).queryByText('warning')).not.toBeInTheDocument();
});

it('does not disable the search button when search result timerange check returns false', async () => {
asMock(useSearchResultTimeRangeErrorCheck).mockReturnValue(() => false);

renderSUT();

const searchButton = await screen.findByRole('button', { name: /perform search/i });

await waitFor(() => expect(searchButton.classList).not.toContain('disabled'));
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import connect from 'stores/connect';
import { createElasticsearchQueryString } from 'views/logic/queries/Query';
import type Widget from 'views/logic/widgets/Widget';
import type { SearchBarFormValues } from 'views/Constants';
import { DEFAULT_TIMERANGE } from 'views/Constants';
import { SEARCH_TYPE_RANGE_LIMIT_ERROR_TYPE, DEFAULT_TIMERANGE } from 'views/Constants';
import type GlobalOverride from 'views/logic/search/GlobalOverride';
import WidgetContext from 'views/components/contexts/WidgetContext';
import { PropagateDisableSubmissionState } from 'views/components/aggregationwizard';
Expand Down Expand Up @@ -64,6 +64,7 @@ import useSearchConfiguration from 'hooks/useSearchConfiguration';
import { defaultCompare } from 'logic/DefaultCompare';
import StreamCategoryFilter from 'views/components/searchbar/StreamCategoryFilter';
import { executeActiveQuery } from 'views/logic/slices/viewSlice';
import useSearchResultTimeRangeErrorCheck from 'views/hooks/useSearchResultTimeRangeErrorCheck';

import TimeRangeOverrideInfo from './searchbar/WidgetTimeRangeOverride';
import TimeRangeFilter from './searchbar/time-range-filter';
Expand Down Expand Up @@ -238,6 +239,8 @@ const WidgetQueryControls = ({ availableStreams }: Props) => {

useBindApplySearchControlsChanges(formRef);

const searchResultTimeRangeErrorCheck = useSearchResultTimeRangeErrorCheck(SEARCH_TYPE_RANGE_LIMIT_ERROR_TYPE);

return (
<FormWarningsProvider>
<SearchBarForm
Expand All @@ -247,7 +250,8 @@ const WidgetQueryControls = ({ availableStreams }: Props) => {
onSubmit={_onSubmit}
validateQueryString={validate}>
{({ dirty, errors, isValid, isSubmitting, handleSubmit, values, setFieldValue, validateForm }) => {
const disableSearchSubmit = isSubmitting || isValidatingQuery || !isValid;
const showTimeRangeErrorFromResults = searchResultTimeRangeErrorCheck(values?.timerange);
const disableSearchSubmit = isSubmitting || isValidatingQuery || !isValid || showTimeRangeErrorFromResults;

return (
<Container>
Expand All @@ -263,7 +267,7 @@ const WidgetQueryControls = ({ availableStreams }: Props) => {
limitDuration={limitDuration}
onChange={(nextTimeRange) => setFieldValue('timerange', nextTimeRange)}
value={values?.timerange}
hasErrorOnMount={!!errors.timerange}
hasErrorOnMount={!!errors.timerange || showTimeRangeErrorFromResults}
/>
)}
{hasTimeRangeOverride && (
Expand Down
Loading
Loading