From 9fcc93457f4ac382cbeb40777de369fe50c73eed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez?= Date: Wed, 11 Dec 2019 13:47:37 +0100 Subject: [PATCH 1/4] [Logs + Metrics UI] Add missing headers in Logs & metrics (#52405) * Fix broken aria references `EuiDescribedFormGroup` needs an actual header in its `title` for it to make a correct `aria-labelledby`. * Fix `aria-labelledby` references in settings page Co-authored-by: Elastic Machine --- .../fields_configuration_panel.tsx | 50 +++++++++++-------- .../indices_configuration_panel.tsx | 22 ++++---- .../name_configuration_panel.tsx | 4 +- .../analysis_setup_indices_form.tsx | 10 ++-- .../analysis_setup_timerange_form.tsx | 10 ++-- 5 files changed, 58 insertions(+), 38 deletions(-) diff --git a/x-pack/legacy/plugins/infra/public/components/source_configuration/fields_configuration_panel.tsx b/x-pack/legacy/plugins/infra/public/components/source_configuration/fields_configuration_panel.tsx index 771285e8ccee4..5f3d1a63e72eb 100644 --- a/x-pack/legacy/plugins/infra/public/components/source_configuration/fields_configuration_panel.tsx +++ b/x-pack/legacy/plugins/infra/public/components/source_configuration/fields_configuration_panel.tsx @@ -50,10 +50,12 @@ export const FieldsConfigurationPanel = ({ +

+ +

} description={ +

+ +

} description={ +

+ +

} description={ +

+ +

} description={ +

+ +

} description={ +

+ +

} description={ +

+ +

} description={ +

+ +

} description={ +

+ +

} description={ +

+ +

} description={ Date: Wed, 11 Dec 2019 13:48:40 +0100 Subject: [PATCH 2/4] [Logs + Metrics UI] Remove eslint exceptions (#50979) This removes the two eslint exceptions specific to the `infra` plugin introduced in #49244. fixes #49563 --- .eslintrc.js | 7 -- .../public/components/formatted_time.tsx | 1 - .../log_entry_actions_menu.tsx | 2 +- .../logging/log_highlights_menu.tsx | 24 +++++-- .../logging/log_text_stream/text_styles.tsx | 2 +- .../components/metrics_explorer/metrics.tsx | 33 ++++----- .../components/saved_views/create_modal.tsx | 2 +- .../add_log_column_popover.tsx | 2 +- .../source_configuration_form_state.tsx | 2 +- .../waffle/waffle_inventory_switcher.tsx | 27 ++++--- .../log_analysis_capabilities.tsx | 2 +- .../logs/log_analysis/log_analysis_module.tsx | 10 +-- .../log_analysis/log_analysis_setup_state.tsx | 2 +- .../log_highlights/log_entry_highlights.tsx | 2 +- .../log_highlights/log_summary_highlights.ts | 10 ++- .../logs/log_highlights/next_and_previous.tsx | 2 +- .../logs/log_highlights/redux_bridges.tsx | 6 +- .../containers/logs/with_stream_items.ts | 2 +- .../use_metrics_explorer_data.ts | 3 + .../use_metrics_explorer_options.ts | 2 +- .../infra/public/hooks/use_saved_view.ts | 61 ++++++++-------- .../infra/public/hooks/use_track_metric.tsx | 3 + .../public/pages/infrastructure/index.tsx | 15 ++-- .../infrastructure/metrics_explorer/index.tsx | 7 +- .../use_metric_explorer_state.ts | 12 ++-- .../logs/log_entry_rate/page_content.tsx | 2 +- .../sections/anomalies/table.tsx | 2 +- .../analysis_setup_indices_form.tsx | 2 +- .../use_log_entry_rate_module.tsx | 2 +- .../use_log_entry_rate_results_url_state.tsx | 11 +-- .../metrics/components/chart_section_vis.tsx | 18 ++--- .../metrics/components/node_details_page.tsx | 10 +-- .../pages/metrics/components/section.tsx | 71 ++++++++++--------- .../pages/metrics/components/sub_section.tsx | 36 +++++----- .../metrics/containers/with_metrics_time.tsx | 11 ++- .../infra/public/pages/metrics/index.tsx | 42 +++++------ .../infra/public/utils/cancellable_effect.ts | 3 + .../public/utils/use_kibana_ui_setting.ts | 7 +- .../infra/public/utils/use_tracked_promise.ts | 2 + .../infra/public/utils/use_url_state.ts | 38 +++++++--- .../public/utils/use_visibility_state.ts | 2 +- 41 files changed, 269 insertions(+), 231 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index e01632815bc68..367ac892107ab 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -170,13 +170,6 @@ module.exports = { 'react-hooks/rules-of-hooks': 'off', }, }, - { - files: ['x-pack/legacy/plugins/infra/**/*.{js,ts,tsx}'], - rules: { - 'react-hooks/exhaustive-deps': 'off', - 'react-hooks/rules-of-hooks': 'off', - }, - }, { files: ['x-pack/legacy/plugins/lens/**/*.{js,ts,tsx}'], rules: { diff --git a/x-pack/legacy/plugins/infra/public/components/formatted_time.tsx b/x-pack/legacy/plugins/infra/public/components/formatted_time.tsx index 78255c55df124..46b505d4fab52 100644 --- a/x-pack/legacy/plugins/infra/public/components/formatted_time.tsx +++ b/x-pack/legacy/plugins/infra/public/components/formatted_time.tsx @@ -37,7 +37,6 @@ export const useFormattedTime = ( const dateFormat = formatMap[format]; const formattedTime = useMemo(() => getFormattedTime(time, dateFormat, fallbackFormat), [ - getFormattedTime, time, dateFormat, fallbackFormat, diff --git a/x-pack/legacy/plugins/infra/public/components/logging/log_entry_flyout/log_entry_actions_menu.tsx b/x-pack/legacy/plugins/infra/public/components/logging/log_entry_flyout/log_entry_actions_menu.tsx index 92c6ddd193609..d018b3a0f38ff 100644 --- a/x-pack/legacy/plugins/infra/public/components/logging/log_entry_flyout/log_entry_actions_menu.tsx +++ b/x-pack/legacy/plugins/infra/public/components/logging/log_entry_flyout/log_entry_actions_menu.tsx @@ -51,7 +51,7 @@ export const LogEntryActionsMenu: React.FunctionComponent<{ /> , ], - [uptimeLink] + [apmLink, uptimeLink] ); const hasMenuItems = useMemo(() => menuItems.length > 0, [menuItems]); diff --git a/x-pack/legacy/plugins/infra/public/components/logging/log_highlights_menu.tsx b/x-pack/legacy/plugins/infra/public/components/logging/log_highlights_menu.tsx index 24a5e8bacb4f9..d13ccde7466cd 100644 --- a/x-pack/legacy/plugins/infra/public/components/logging/log_highlights_menu.tsx +++ b/x-pack/legacy/plugins/infra/public/components/logging/log_highlights_menu.tsx @@ -16,7 +16,7 @@ import { import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { debounce } from 'lodash'; -import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import React, { useCallback, useMemo, useState } from 'react'; import euiStyled from '../../../../../common/eui_styled_components'; import { useVisibilityState } from '../../utils/use_visibility_state'; @@ -47,8 +47,25 @@ export const LogHighlightsMenu: React.FC = ({ } = useVisibilityState(false); // Input field state - const [highlightTerm, setHighlightTerm] = useState(''); + const [highlightTerm, _setHighlightTerm] = useState(''); + const debouncedOnChange = useMemo(() => debounce(onChange, 275), [onChange]); + const setHighlightTerm = useCallback( + valueOrUpdater => + _setHighlightTerm(previousHighlightTerm => { + const newHighlightTerm = + typeof valueOrUpdater === 'function' + ? valueOrUpdater(previousHighlightTerm) + : valueOrUpdater; + + if (newHighlightTerm !== previousHighlightTerm) { + debouncedOnChange([newHighlightTerm]); + } + + return newHighlightTerm; + }), + [debouncedOnChange] + ); const changeHighlightTerm = useCallback( e => { const value = e.target.value; @@ -57,9 +74,6 @@ export const LogHighlightsMenu: React.FC = ({ [setHighlightTerm] ); const clearHighlightTerm = useCallback(() => setHighlightTerm(''), [setHighlightTerm]); - useEffect(() => { - debouncedOnChange([highlightTerm]); - }, [highlightTerm]); const button = ( diff --git a/x-pack/legacy/plugins/infra/public/components/logging/log_text_stream/text_styles.tsx b/x-pack/legacy/plugins/infra/public/components/logging/log_text_stream/text_styles.tsx index 1d40c88f5d1d0..e95ac6aa7923b 100644 --- a/x-pack/legacy/plugins/infra/public/components/logging/log_text_stream/text_styles.tsx +++ b/x-pack/legacy/plugins/infra/public/components/logging/log_text_stream/text_styles.tsx @@ -63,7 +63,7 @@ export const useMeasuredCharacterDimensions = (scale: TextScale) => { X ), - [scale] + [measureElement, scale] ); return { diff --git a/x-pack/legacy/plugins/infra/public/components/metrics_explorer/metrics.tsx b/x-pack/legacy/plugins/infra/public/components/metrics_explorer/metrics.tsx index d59e709d9a19a..42df7c6915a0d 100644 --- a/x-pack/legacy/plugins/infra/public/components/metrics_explorer/metrics.tsx +++ b/x-pack/legacy/plugins/infra/public/components/metrics_explorer/metrics.tsx @@ -7,7 +7,7 @@ import { EuiComboBox } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import React, { useCallback, useState, useEffect } from 'react'; +import React, { useCallback, useState } from 'react'; import { FieldType } from 'ui/index_patterns'; import { colorTransformer, MetricsExplorerColor } from '../../../common/color_palette'; import { @@ -31,24 +31,19 @@ interface SelectedOption { export const MetricsExplorerMetrics = ({ options, onChange, fields, autoFocus = false }: Props) => { const colors = Object.keys(MetricsExplorerColor) as MetricsExplorerColor[]; - const [inputRef, setInputRef] = useState(null); - const [focusOnce, setFocusState] = useState(false); + const [shouldFocus, setShouldFocus] = useState(autoFocus); - useEffect(() => { - if (inputRef && autoFocus && !focusOnce) { - inputRef.focus(); - setFocusState(true); - } - }, [inputRef]); + // the EuiCombobox forwards the ref to an input element + const autoFocusInputElement = useCallback( + (inputElement: HTMLInputElement | null) => { + if (inputElement && shouldFocus) { + inputElement.focus(); + setShouldFocus(false); + } + }, + [shouldFocus] + ); - // I tried to use useRef originally but the EUIComboBox component's type definition - // would only accept an actual input element or a callback function (with the same type). - // This effectivly does the same thing but is compatible with EuiComboBox. - const handleInputRef = (ref: HTMLInputElement) => { - if (ref) { - setInputRef(ref); - } - }; const handleChange = useCallback( selectedOptions => { onChange( @@ -59,7 +54,7 @@ export const MetricsExplorerMetrics = ({ options, onChange, fields, autoFocus = })) ); }, - [options, onChange] + [onChange, options.aggregation, colors] ); const comboOptions = fields @@ -86,7 +81,7 @@ export const MetricsExplorerMetrics = ({ options, onChange, fields, autoFocus = selectedOptions={selectedOptions} onChange={handleChange} isClearable={true} - inputRef={handleInputRef} + inputRef={autoFocusInputElement} /> ); }; diff --git a/x-pack/legacy/plugins/infra/public/components/saved_views/create_modal.tsx b/x-pack/legacy/plugins/infra/public/components/saved_views/create_modal.tsx index 8df479f36e2f9..9b8907a1ff9e1 100644 --- a/x-pack/legacy/plugins/infra/public/components/saved_views/create_modal.tsx +++ b/x-pack/legacy/plugins/infra/public/components/saved_views/create_modal.tsx @@ -36,7 +36,7 @@ export const SavedViewCreateModal = ({ close, save, isInvalid }: Props) => { const saveView = useCallback(() => { save(viewName, includeTime); - }, [viewName, includeTime]); + }, [includeTime, save, viewName]); return ( diff --git a/x-pack/legacy/plugins/infra/public/components/source_configuration/add_log_column_popover.tsx b/x-pack/legacy/plugins/infra/public/components/source_configuration/add_log_column_popover.tsx index 9b83f62e7856b..fc8407c5298e6 100644 --- a/x-pack/legacy/plugins/infra/public/components/source_configuration/add_log_column_popover.tsx +++ b/x-pack/legacy/plugins/infra/public/components/source_configuration/add_log_column_popover.tsx @@ -94,7 +94,7 @@ export const AddLogColumnButtonAndPopover: React.FunctionComponent<{ addLogColumn(selectedOption.columnConfiguration); }, - [addLogColumn, availableColumnOptions] + [addLogColumn, availableColumnOptions, closePopover] ); return ( diff --git a/x-pack/legacy/plugins/infra/public/components/source_configuration/source_configuration_form_state.tsx b/x-pack/legacy/plugins/infra/public/components/source_configuration/source_configuration_form_state.tsx index 3614a88c1e99e..262649e20709b 100644 --- a/x-pack/legacy/plugins/infra/public/components/source_configuration/source_configuration_form_state.tsx +++ b/x-pack/legacy/plugins/infra/public/components/source_configuration/source_configuration_form_state.tsx @@ -52,7 +52,7 @@ export const useSourceConfigurationFormState = (configuration?: SourceConfigurat const resetForm = useCallback(() => { indicesConfigurationFormState.resetForm(); logColumnsConfigurationFormState.resetForm(); - }, [indicesConfigurationFormState.resetForm, logColumnsConfigurationFormState.formState]); + }, [indicesConfigurationFormState, logColumnsConfigurationFormState]); const isFormDirty = useMemo( () => indicesConfigurationFormState.isFormDirty || logColumnsConfigurationFormState.isFormDirty, diff --git a/x-pack/legacy/plugins/infra/public/components/waffle/waffle_inventory_switcher.tsx b/x-pack/legacy/plugins/infra/public/components/waffle/waffle_inventory_switcher.tsx index 38e87038b7c4f..c8f03cef4d6ac 100644 --- a/x-pack/legacy/plugins/infra/public/components/waffle/waffle_inventory_switcher.tsx +++ b/x-pack/legacy/plugins/infra/public/components/waffle/waffle_inventory_switcher.tsx @@ -17,28 +17,33 @@ import { } from '../../graphql/types'; import { findInventoryModel } from '../../../common/inventory_models'; -interface Props { +interface WaffleInventorySwitcherProps { nodeType: InfraNodeType; changeNodeType: (nodeType: InfraNodeType) => void; changeGroupBy: (groupBy: InfraSnapshotGroupbyInput[]) => void; changeMetric: (metric: InfraSnapshotMetricInput) => void; } -export const WaffleInventorySwitcher = (props: Props) => { +export const WaffleInventorySwitcher: React.FC = ({ + changeNodeType, + changeGroupBy, + changeMetric, + nodeType, +}) => { const [isOpen, setIsOpen] = useState(false); const closePopover = useCallback(() => setIsOpen(false), []); const openPopover = useCallback(() => setIsOpen(true), []); const goToNodeType = useCallback( - (nodeType: InfraNodeType) => { + (targetNodeType: InfraNodeType) => { closePopover(); - props.changeNodeType(nodeType); - props.changeGroupBy([]); - const inventoryModel = findInventoryModel(nodeType); - props.changeMetric({ + changeNodeType(targetNodeType); + changeGroupBy([]); + const inventoryModel = findInventoryModel(targetNodeType); + changeMetric({ type: inventoryModel.metrics.defaultSnapshot as InfraSnapshotMetricType, }); }, - [props.changeGroupBy, props.changeNodeType, props.changeMetric] + [closePopover, changeNodeType, changeGroupBy, changeMetric] ); const goToHost = useCallback(() => goToNodeType('host' as InfraNodeType), [goToNodeType]); const goToK8 = useCallback(() => goToNodeType('pod' as InfraNodeType), [goToNodeType]); @@ -68,10 +73,10 @@ export const WaffleInventorySwitcher = (props: Props) => { ], }, ], - [] + [goToDocker, goToHost, goToK8] ); const selectedText = useMemo(() => { - switch (props.nodeType) { + switch (nodeType) { case InfraNodeType.host: return i18n.translate('xpack.infra.waffle.nodeTypeSwitcher.hostsLabel', { defaultMessage: 'Hosts', @@ -81,7 +86,7 @@ export const WaffleInventorySwitcher = (props: Props) => { case InfraNodeType.container: return 'Docker'; } - }, [props.nodeType]); + }, [nodeType]); return ( diff --git a/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_capabilities.tsx b/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_capabilities.tsx index 35a3ac737ada3..bb01043b0db6e 100644 --- a/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_capabilities.tsx +++ b/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_capabilities.tsx @@ -46,7 +46,7 @@ export const useLogAnalysisCapabilities = () => { useEffect(() => { fetchMlCapabilities(); - }, []); + }, [fetchMlCapabilities]); const isLoading = useMemo(() => fetchMlCapabilitiesRequest.state === 'pending', [ fetchMlCapabilitiesRequest.state, diff --git a/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_module.tsx b/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_module.tsx index 189b58d7923f8..d7d0ecb6f2c8d 100644 --- a/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_module.tsx +++ b/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_module.tsx @@ -125,23 +125,23 @@ export const useLogAnalysisModule = ({ dispatchModuleStatus({ type: 'failedSetup' }); }); }, - [cleanUpModule, setUpModule] + [cleanUpModule, dispatchModuleStatus, setUpModule] ); const viewSetupForReconfiguration = useCallback(() => { dispatchModuleStatus({ type: 'requestedJobConfigurationUpdate' }); - }, []); + }, [dispatchModuleStatus]); const viewSetupForUpdate = useCallback(() => { dispatchModuleStatus({ type: 'requestedJobDefinitionUpdate' }); - }, []); + }, [dispatchModuleStatus]); const viewResults = useCallback(() => { dispatchModuleStatus({ type: 'viewedResults' }); - }, []); + }, [dispatchModuleStatus]); const jobIds = useMemo(() => moduleDescriptor.getJobIds(spaceId, sourceId), [ - moduleDescriptor.getJobIds, + moduleDescriptor, spaceId, sourceId, ]); diff --git a/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_setup_state.tsx b/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_setup_state.tsx index 275c0194be3b2..74dbb3c7a8062 100644 --- a/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_setup_state.tsx +++ b/x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_setup_state.tsx @@ -140,7 +140,7 @@ export const useAnalysisSetupState = ({ ? [...errors, ...index.errors] : errors; }, []); - }, [selectedIndexNames, validatedIndices, validateIndicesRequest.state]); + }, [isValidating, validateIndicesRequest.state, selectedIndexNames, validatedIndices]); return { cleanupAndSetup, diff --git a/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/log_entry_highlights.tsx b/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/log_entry_highlights.tsx index 6ead866fb960a..2b19958a9b1a1 100644 --- a/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/log_entry_highlights.tsx +++ b/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/log_entry_highlights.tsx @@ -78,7 +78,7 @@ export const useLogEntryHighlights = ( } else { setLogEntryHighlights([]); } - }, [highlightTerms, startKey, endKey, filterQuery, sourceVersion]); + }, [endKey, filterQuery, highlightTerms, loadLogEntryHighlights, sourceVersion, startKey]); const logEntryHighlightsById = useMemo( () => diff --git a/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/log_summary_highlights.ts b/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/log_summary_highlights.ts index 34c66afda010e..874c70e016496 100644 --- a/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/log_summary_highlights.ts +++ b/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/log_summary_highlights.ts @@ -74,7 +74,15 @@ export const useLogSummaryHighlights = ( } else { setLogSummaryHighlights([]); } - }, [highlightTerms, start, end, bucketSize, filterQuery, sourceVersion]); + }, [ + bucketSize, + debouncedLoadSummaryHighlights, + end, + filterQuery, + highlightTerms, + sourceVersion, + start, + ]); return { logSummaryHighlights, diff --git a/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/next_and_previous.tsx b/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/next_and_previous.tsx index 95ead50119eb4..62a43a5412825 100644 --- a/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/next_and_previous.tsx +++ b/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/next_and_previous.tsx @@ -53,7 +53,7 @@ export const useNextAndPrevious = ({ const initialTimeKey = getUniqueLogEntryKey(entries[initialIndex]); setCurrentTimeKey(initialTimeKey); } - }, [currentTimeKey, entries, setCurrentTimeKey]); + }, [currentTimeKey, entries, setCurrentTimeKey, visibleMidpoint]); const indexOfCurrentTimeKey = useMemo(() => { if (currentTimeKey && entries.length > 0) { diff --git a/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/redux_bridges.tsx b/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/redux_bridges.tsx index 2b60c6edd97aa..9ea8987d4f326 100644 --- a/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/redux_bridges.tsx +++ b/x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/redux_bridges.tsx @@ -25,11 +25,11 @@ export const LogHighlightsPositionBridge = withLogPosition( const { setJumpToTarget, setVisibleMidpoint } = useContext(LogHighlightsState.Context); useEffect(() => { setVisibleMidpoint(visibleMidpoint); - }, [visibleMidpoint]); + }, [setVisibleMidpoint, visibleMidpoint]); useEffect(() => { setJumpToTarget(() => jumpToTargetPosition); - }, [jumpToTargetPosition]); + }, [jumpToTargetPosition, setJumpToTarget]); return null; } @@ -41,7 +41,7 @@ export const LogHighlightsFilterQueryBridge = withLogFilter( useEffect(() => { setFilterQuery(serializedFilterQuery); - }, [serializedFilterQuery]); + }, [serializedFilterQuery, setFilterQuery]); return null; } diff --git a/x-pack/legacy/plugins/infra/public/containers/logs/with_stream_items.ts b/x-pack/legacy/plugins/infra/public/containers/logs/with_stream_items.ts index da468b4391e4e..9b20676486af2 100644 --- a/x-pack/legacy/plugins/infra/public/containers/logs/with_stream_items.ts +++ b/x-pack/legacy/plugins/infra/public/containers/logs/with_stream_items.ts @@ -35,7 +35,7 @@ export const WithStreamItems: React.FunctionComponent<{ createLogEntryStreamItem(logEntry, logEntryHighlightsById[logEntry.gid] || []) ), - [logEntries.entries, logEntryHighlightsById] + [isAutoReloading, logEntries.entries, logEntries.isReloading, logEntryHighlightsById] ); return children({ diff --git a/x-pack/legacy/plugins/infra/public/containers/metrics_explorer/use_metrics_explorer_data.ts b/x-pack/legacy/plugins/infra/public/containers/metrics_explorer/use_metrics_explorer_data.ts index 1418d6aef67ac..c2a599ea1ae78 100644 --- a/x-pack/legacy/plugins/infra/public/containers/metrics_explorer/use_metrics_explorer_data.ts +++ b/x-pack/legacy/plugins/infra/public/containers/metrics_explorer/use_metrics_explorer_data.ts @@ -96,6 +96,9 @@ export function useMetricsExplorerData( } setLoading(false); })(); + + // TODO: fix this dependency list while preserving the semantics + // eslint-disable-next-line react-hooks/exhaustive-deps }, [options, source, timerange, signal, afterKey]); return { error, loading, data }; } diff --git a/x-pack/legacy/plugins/infra/public/containers/metrics_explorer/use_metrics_explorer_options.ts b/x-pack/legacy/plugins/infra/public/containers/metrics_explorer/use_metrics_explorer_options.ts index 278f3e0a9c17d..de7a8d5805ecc 100644 --- a/x-pack/legacy/plugins/infra/public/containers/metrics_explorer/use_metrics_explorer_options.ts +++ b/x-pack/legacy/plugins/infra/public/containers/metrics_explorer/use_metrics_explorer_options.ts @@ -102,7 +102,7 @@ function useStateWithLocalStorage( const [state, setState] = useState(parseJsonOrDefault(storageState, defaultState)); useEffect(() => { localStorage.setItem(key, JSON.stringify(state)); - }, [state]); + }, [key, state]); return [state, setState]; } diff --git a/x-pack/legacy/plugins/infra/public/hooks/use_saved_view.ts b/x-pack/legacy/plugins/infra/public/hooks/use_saved_view.ts index 8db0ed28d9b21..4b12b6c51ea0e 100644 --- a/x-pack/legacy/plugins/infra/public/hooks/use_saved_view.ts +++ b/x-pack/legacy/plugins/infra/public/hooks/use_saved_view.ts @@ -26,29 +26,32 @@ export const useSavedView = (defaultViewState: ViewState, viewType: s >(viewType); const { create, error: errorOnCreate, createdId } = useCreateSavedObject(viewType); const { deleteObject, deletedId } = useDeleteSavedObject(viewType); - const deleteView = useCallback((id: string) => deleteObject(id), []); + const deleteView = useCallback((id: string) => deleteObject(id), [deleteObject]); const [createError, setCreateError] = useState(null); - useEffect(() => setCreateError(createError), [errorOnCreate, setCreateError]); + useEffect(() => setCreateError(errorOnCreate), [errorOnCreate]); - const saveView = useCallback((d: { [p: string]: any }) => { - const doSave = async () => { - const exists = await hasView(d.name); - if (exists) { - setCreateError( - i18n.translate('xpack.infra.savedView.errorOnCreate.duplicateViewName', { - defaultMessage: `A view with that name already exists.`, - }) - ); - return; - } - create(d); - }; - setCreateError(null); - doSave(); - }, []); + const saveView = useCallback( + (d: { [p: string]: any }) => { + const doSave = async () => { + const exists = await hasView(d.name); + if (exists) { + setCreateError( + i18n.translate('xpack.infra.savedView.errorOnCreate.duplicateViewName', { + defaultMessage: `A view with that name already exists.`, + }) + ); + return; + } + create(d); + }; + setCreateError(null); + doSave(); + }, + [create, hasView] + ); - const savedObjects = data ? data.savedObjects : []; + const savedObjects = useMemo(() => (data ? data.savedObjects : []), [data]); const views = useMemo(() => { const items: Array> = [ { @@ -61,19 +64,17 @@ export const useSavedView = (defaultViewState: ViewState, viewType: s }, ]; - if (data) { - data.savedObjects.forEach( - o => - o.type === viewType && - items.push({ - ...o.attributes, - id: o.id, - }) - ); - } + savedObjects.forEach( + o => + o.type === viewType && + items.push({ + ...o.attributes, + id: o.id, + }) + ); return items; - }, [savedObjects, defaultViewState]); + }, [defaultViewState, savedObjects, viewType]); return { views, diff --git a/x-pack/legacy/plugins/infra/public/hooks/use_track_metric.tsx b/x-pack/legacy/plugins/infra/public/hooks/use_track_metric.tsx index 379b3af3f1063..c5945ab808202 100644 --- a/x-pack/legacy/plugins/infra/public/hooks/use_track_metric.tsx +++ b/x-pack/legacy/plugins/infra/public/hooks/use_track_metric.tsx @@ -57,6 +57,9 @@ export function useTrackMetric( const trackUiMetric = getTrackerForApp(app); const id = setTimeout(() => trackUiMetric(metricType, decoratedMetric), Math.max(delay, 0)); return () => clearTimeout(id); + + // the dependencies are managed externally + // eslint-disable-next-line react-hooks/exhaustive-deps }, effectDependencies); } diff --git a/x-pack/legacy/plugins/infra/public/pages/infrastructure/index.tsx b/x-pack/legacy/plugins/infra/public/pages/infrastructure/index.tsx index fe48fcc62f77d..9efbbe790abc1 100644 --- a/x-pack/legacy/plugins/infra/public/pages/infrastructure/index.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/infrastructure/index.tsx @@ -24,6 +24,7 @@ import { MetricsExplorerPage } from './metrics_explorer'; import { SnapshotPage } from './snapshot'; import { SettingsPage } from '../shared/settings'; import { AppNavigation } from '../../components/navigation/app_navigation'; +import { SourceLoadingPage } from '../../components/source_loading_page'; interface InfrastructurePageProps extends RouteComponentProps { uiCapabilities: UICapabilities; @@ -95,11 +96,15 @@ export const InfrastructurePage = injectUICapabilities( {({ configuration, createDerivedIndexPattern }) => ( - + {configuration ? ( + + ) : ( + + )} )} diff --git a/x-pack/legacy/plugins/infra/public/pages/infrastructure/metrics_explorer/index.tsx b/x-pack/legacy/plugins/infra/public/pages/infrastructure/metrics_explorer/index.tsx index 63f5a81967618..4db4319b91d3c 100644 --- a/x-pack/legacy/plugins/infra/public/pages/infrastructure/metrics_explorer/index.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/infrastructure/metrics_explorer/index.tsx @@ -11,22 +11,17 @@ import { IIndexPattern } from 'src/plugins/data/public'; import { DocumentTitle } from '../../../components/document_title'; import { MetricsExplorerCharts } from '../../../components/metrics_explorer/charts'; import { MetricsExplorerToolbar } from '../../../components/metrics_explorer/toolbar'; -import { SourceLoadingPage } from '../../../components/source_loading_page'; import { SourceQuery } from '../../../../common/graphql/types'; import { NoData } from '../../../components/empty_states'; import { useMetricsExplorerState } from './use_metric_explorer_state'; import { useTrackPageview } from '../../../hooks/use_track_metric'; interface MetricsExplorerPageProps { - source: SourceQuery.Query['source']['configuration'] | undefined; + source: SourceQuery.Query['source']['configuration']; derivedIndexPattern: IIndexPattern; } export const MetricsExplorerPage = ({ source, derivedIndexPattern }: MetricsExplorerPageProps) => { - if (!source) { - return ; - } - const { loading, error, diff --git a/x-pack/legacy/plugins/infra/public/pages/infrastructure/metrics_explorer/use_metric_explorer_state.ts b/x-pack/legacy/plugins/infra/public/pages/infrastructure/metrics_explorer/use_metric_explorer_state.ts index 415a6ae89a8b1..57ea886169701 100644 --- a/x-pack/legacy/plugins/infra/public/pages/infrastructure/metrics_explorer/use_metric_explorer_state.ts +++ b/x-pack/legacy/plugins/infra/public/pages/infrastructure/metrics_explorer/use_metric_explorer_state.ts @@ -59,7 +59,7 @@ export const useMetricsExplorerState = ( setAfterKey(null); setTimeRange({ ...currentTimerange, from: start, to: end }); }, - [currentTimerange] + [currentTimerange, setTimeRange] ); const handleGroupByChange = useCallback( @@ -70,7 +70,7 @@ export const useMetricsExplorerState = ( groupBy: groupBy || void 0, }); }, - [options] + [options, setOptions] ); const handleFilterQuerySubmit = useCallback( @@ -81,7 +81,7 @@ export const useMetricsExplorerState = ( filterQuery: query, }); }, - [options] + [options, setOptions] ); const handleMetricsChange = useCallback( @@ -92,7 +92,7 @@ export const useMetricsExplorerState = ( metrics, }); }, - [options] + [options, setOptions] ); const handleAggregationChange = useCallback( @@ -109,7 +109,7 @@ export const useMetricsExplorerState = ( })); setOptions({ ...options, aggregation, metrics }); }, - [options] + [options, setOptions] ); const onViewStateChange = useCallback( @@ -124,7 +124,7 @@ export const useMetricsExplorerState = ( setOptions(vs.options); } }, - [setChartOptions, setTimeRange, setTimeRange] + [setChartOptions, setOptions, setTimeRange] ); return { diff --git a/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/page_content.tsx b/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/page_content.tsx index e62164cb17b2c..e71985f73fbb8 100644 --- a/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/page_content.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/page_content.tsx @@ -36,7 +36,7 @@ export const LogEntryRatePageContent = () => { useEffect(() => { fetchModuleDefinition(); fetchJobStatus(); - }, []); + }, [fetchJobStatus, fetchModuleDefinition]); if (!hasLogAnalysisCapabilites) { return ; diff --git a/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/table.tsx b/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/table.tsx index 2057d75f72354..86760cf2da7d6 100644 --- a/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/table.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/table.tsx @@ -124,7 +124,7 @@ export const AnomaliesTable: React.FunctionComponent<{ setItemIdToExpandedRowMap(newItemIdToExpandedRowMap); } }, - [results, setTimeRange, timeRange, itemIdToExpandedRowMap, setItemIdToExpandedRowMap] + [itemIdToExpandedRowMap, jobId, results, setTimeRange, timeRange] ); const columns = [ diff --git a/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/setup/initial_configuration_step/analysis_setup_indices_form.tsx b/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/setup/initial_configuration_step/analysis_setup_indices_form.tsx index 35cad040323a6..5a4c21670191e 100644 --- a/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/setup/initial_configuration_step/analysis_setup_indices_form.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/setup/initial_configuration_step/analysis_setup_indices_form.tsx @@ -54,7 +54,7 @@ export const AnalysisSetupIndicesForm: React.FunctionComponent<{ ); }), - [indices] + [handleCheckboxChange, indices] ); return ( diff --git a/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/use_log_entry_rate_module.tsx b/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/use_log_entry_rate_module.tsx index ab6a6578601bf..d1efedb176aba 100644 --- a/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/use_log_entry_rate_module.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/use_log_entry_rate_module.tsx @@ -31,7 +31,7 @@ export const useLogEntryRateModule = ({ spaceId, timestampField, }), - [indexPattern] + [indexPattern, sourceId, spaceId, timestampField] ); return useLogAnalysisModule({ diff --git a/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/use_log_entry_rate_results_url_state.tsx b/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/use_log_entry_rate_results_url_state.tsx index 017be6be49e16..6d4495c8d9e0f 100644 --- a/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/use_log_entry_rate_results_url_state.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/use_log_entry_rate_results_url_state.tsx @@ -8,7 +8,6 @@ import { fold } from 'fp-ts/lib/Either'; import { constant, identity } from 'fp-ts/lib/function'; import { pipe } from 'fp-ts/lib/pipeable'; import * as rt from 'io-ts'; -import { useEffect } from 'react'; import { useUrlState } from '../../../utils/use_url_state'; @@ -41,12 +40,9 @@ export const useLogAnalysisResultsUrlState = () => { pipe(urlTimeRangeRT.decode(value), fold(constant(undefined), identity)), encodeUrlState: urlTimeRangeRT.encode, urlStateKey: TIME_RANGE_URL_STATE_KEY, + writeDefaultState: true, }); - useEffect(() => { - setTimeRange(timeRange); - }, []); - const [autoRefresh, setAutoRefresh] = useUrlState({ defaultState: { isPaused: false, @@ -56,12 +52,9 @@ export const useLogAnalysisResultsUrlState = () => { pipe(autoRefreshRT.decode(value), fold(constant(undefined), identity)), encodeUrlState: autoRefreshRT.encode, urlStateKey: AUTOREFRESH_URL_STATE_KEY, + writeDefaultState: true, }); - useEffect(() => { - setAutoRefresh(autoRefresh); - }, []); - return { timeRange, setTimeRange, diff --git a/x-pack/legacy/plugins/infra/public/pages/metrics/components/chart_section_vis.tsx b/x-pack/legacy/plugins/infra/public/pages/metrics/components/chart_section_vis.tsx index 425b5a43f793f..309961cc39025 100644 --- a/x-pack/legacy/plugins/infra/public/pages/metrics/components/chart_section_vis.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/metrics/components/chart_section_vis.tsx @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import React, { useCallback } from 'react'; +import React, { useCallback, useMemo } from 'react'; import moment from 'moment'; import { i18n } from '@kbn/i18n'; import { @@ -42,15 +42,15 @@ export const ChartSectionVis = ({ seriesOverrides, type, }: VisSectionProps) => { - if (!metric || !id) { - return null; - } const [dateFormat] = useKibanaUiSetting('dateFormat'); const valueFormatter = useCallback(getFormatter(formatter, formatterTemplate), [ formatter, formatterTemplate, ]); - const dateFormatter = useCallback(niceTimeFormatter(getMaxMinTimestamp(metric)), [metric]); + const dateFormatter = useMemo( + () => (metric != null ? niceTimeFormatter(getMaxMinTimestamp(metric)) : undefined), + [metric] + ); const handleTimeChange = useCallback( (from: number, to: number) => { if (onChangeRangeTime) { @@ -73,7 +73,9 @@ export const ChartSectionVis = ({ ), }; - if (!metric) { + if (!id) { + return null; + } else if (!metric) { return ( ); - } - - if (metric.series.some(seriesHasLessThen2DataPoints)) { + } else if (metric.series.some(seriesHasLessThen2DataPoints)) { return ( { - if (!props.metadata) { - return null; - } - const { parsedTimeRange } = props; const { metrics, loading, makeRequest, error } = useNodeDetails( props.requiredMetrics, @@ -65,11 +61,11 @@ export const NodeDetailsPage = (props: Props) => { const refetch = useCallback(() => { makeRequest(); - }, []); + }, [makeRequest]); useEffect(() => { makeRequest(); - }, [parsedTimeRange]); + }, [makeRequest, parsedTimeRange]); if (error) { return ; diff --git a/x-pack/legacy/plugins/infra/public/pages/metrics/components/section.tsx b/x-pack/legacy/plugins/infra/public/pages/metrics/components/section.tsx index 32d2e2eff8ab9..2f9ed9f54df82 100644 --- a/x-pack/legacy/plugins/infra/public/pages/metrics/components/section.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/metrics/components/section.tsx @@ -4,15 +4,15 @@ * you may not use this file except in compliance with the Elastic License. */ +import { EuiTitle } from '@elastic/eui'; import React, { - useContext, Children, - isValidElement, cloneElement, FunctionComponent, - useMemo, + isValidElement, + useContext, } from 'react'; -import { EuiTitle } from '@elastic/eui'; + import { SideNavContext, SubNavItem } from '../lib/side_nav_context'; import { LayoutProps } from '../types'; @@ -31,35 +31,42 @@ export const Section: FunctionComponent = ({ stopLiveStreaming, }) => { const { addNavItem } = useContext(SideNavContext); - const subNavItems: SubNavItem[] = []; - const childrenWithProps = useMemo( - () => - Children.map(children, child => { - if (isValidElement(child)) { - const metric = (metrics && metrics.find(m => m.id === child.props.id)) || null; - if (metric) { - subNavItems.push({ - id: child.props.id, - name: child.props.label, - onClick: () => { - const el = document.getElementById(child.props.id); - if (el) { - el.scrollIntoView(); - } - }, - }); - } - return cloneElement(child, { - metrics, - onChangeRangeTime, - isLiveStreaming, - stopLiveStreaming, - }); - } - return null; - }), - [children, metrics, onChangeRangeTime, isLiveStreaming, stopLiveStreaming] + const subNavItems = Children.toArray(children).reduce( + (accumulatedChildren, child) => { + if (!isValidElement(child)) { + return accumulatedChildren; + } + const metric = metrics?.find(m => m.id === child.props.id) ?? null; + if (metric === null) { + return accumulatedChildren; + } + return [ + ...accumulatedChildren, + { + id: child.props.id, + name: child.props.label, + onClick: () => { + const el = document.getElementById(child.props.id); + if (el) { + el.scrollIntoView(); + } + }, + }, + ]; + }, + [] + ); + + const childrenWithProps = Children.map(children, child => + isValidElement(child) + ? cloneElement(child, { + metrics, + onChangeRangeTime, + isLiveStreaming, + stopLiveStreaming, + }) + : null ); if (metrics && subNavItems.length) { diff --git a/x-pack/legacy/plugins/infra/public/pages/metrics/components/sub_section.tsx b/x-pack/legacy/plugins/infra/public/pages/metrics/components/sub_section.tsx index f3db3b1670199..325d510293135 100644 --- a/x-pack/legacy/plugins/infra/public/pages/metrics/components/sub_section.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/metrics/components/sub_section.tsx @@ -23,29 +23,25 @@ export const SubSection: FunctionComponent = ({ isLiveStreaming, stopLiveStreaming, }) => { - if (!children || !metrics) { + const metric = useMemo(() => metrics?.find(m => m.id === id), [id, metrics]); + + if (!children || !metric) { return null; } - const metric = metrics.find(m => m.id === id); - if (!metric) { + + const childrenWithProps = Children.map(children, child => { + if (isValidElement(child)) { + return cloneElement(child, { + metric, + id, + onChangeRangeTime, + isLiveStreaming, + stopLiveStreaming, + }); + } return null; - } - const childrenWithProps = useMemo( - () => - Children.map(children, child => { - if (isValidElement(child)) { - return cloneElement(child, { - metric, - id, - onChangeRangeTime, - isLiveStreaming, - stopLiveStreaming, - }); - } - return null; - }), - [children, metric, id, onChangeRangeTime, isLiveStreaming, stopLiveStreaming] - ); + }); + return (
{label ? ( diff --git a/x-pack/legacy/plugins/infra/public/pages/metrics/containers/with_metrics_time.tsx b/x-pack/legacy/plugins/infra/public/pages/metrics/containers/with_metrics_time.tsx index 432725b6f62b0..64d2ddb67139d 100644 --- a/x-pack/legacy/plugins/infra/public/pages/metrics/containers/with_metrics_time.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/metrics/containers/with_metrics_time.tsx @@ -59,13 +59,10 @@ export const useMetricsTime = () => { const [parsedTimeRange, setParsedTimeRange] = useState(parseRange(defaultRange)); - const updateTimeRange = useCallback( - (range: MetricsTimeInput) => { - setTimeRange(range); - setParsedTimeRange(parseRange(range)); - }, - [setParsedTimeRange] - ); + const updateTimeRange = useCallback((range: MetricsTimeInput) => { + setTimeRange(range); + setParsedTimeRange(parseRange(range)); + }, []); return { timeRange, diff --git a/x-pack/legacy/plugins/infra/public/pages/metrics/index.tsx b/x-pack/legacy/plugins/infra/public/pages/metrics/index.tsx index 93253406aec2d..b330ad02f1022 100644 --- a/x-pack/legacy/plugins/infra/public/pages/metrics/index.tsx +++ b/x-pack/legacy/plugins/infra/public/pages/metrics/index.tsx @@ -112,26 +112,28 @@ export const MetricDetail = withMetricPageProviders( })} /> - + {metadata ? ( + + ) : null} )} diff --git a/x-pack/legacy/plugins/infra/public/utils/cancellable_effect.ts b/x-pack/legacy/plugins/infra/public/utils/cancellable_effect.ts index bb7d253ea1557..a986af07f0c9a 100644 --- a/x-pack/legacy/plugins/infra/public/utils/cancellable_effect.ts +++ b/x-pack/legacy/plugins/infra/public/utils/cancellable_effect.ts @@ -27,5 +27,8 @@ export const useCancellableEffect = ( effect(() => cancellationSignal.isCancelled); return cancellationSignal.cancel; + + // the dependencies are managed externally + // eslint-disable-next-line react-hooks/exhaustive-deps }, deps); }; diff --git a/x-pack/legacy/plugins/infra/public/utils/use_kibana_ui_setting.ts b/x-pack/legacy/plugins/infra/public/utils/use_kibana_ui_setting.ts index c48f95a6521cf..1b08fb4231243 100644 --- a/x-pack/legacy/plugins/infra/public/utils/use_kibana_ui_setting.ts +++ b/x-pack/legacy/plugins/infra/public/utils/use_kibana_ui_setting.ts @@ -28,10 +28,15 @@ import { useObservable } from './use_observable'; export const useKibanaUiSetting = (key: string, defaultValue?: any) => { const uiSettingsClient = npSetup.core.uiSettings; - const uiSetting$ = useMemo(() => uiSettingsClient.get$(key, defaultValue), [uiSettingsClient]); + const uiSetting$ = useMemo(() => uiSettingsClient.get$(key, defaultValue), [ + defaultValue, + key, + uiSettingsClient, + ]); const uiSetting = useObservable(uiSetting$); const setUiSetting = useCallback((value: any) => uiSettingsClient.set(key, value), [ + key, uiSettingsClient, ]); diff --git a/x-pack/legacy/plugins/infra/public/utils/use_tracked_promise.ts b/x-pack/legacy/plugins/infra/public/utils/use_tracked_promise.ts index 366caf0dfb156..c23bab7026aaa 100644 --- a/x-pack/legacy/plugins/infra/public/utils/use_tracked_promise.ts +++ b/x-pack/legacy/plugins/infra/public/utils/use_tracked_promise.ts @@ -190,6 +190,8 @@ export const useTrackedPromise = ( return newPendingPromise.promise; }, + // the dependencies are managed by the caller + // eslint-disable-next-line react-hooks/exhaustive-deps dependencies ); diff --git a/x-pack/legacy/plugins/infra/public/utils/use_url_state.ts b/x-pack/legacy/plugins/infra/public/utils/use_url_state.ts index d03a5aaa9d697..79a5d552bcd78 100644 --- a/x-pack/legacy/plugins/infra/public/utils/use_url_state.ts +++ b/x-pack/legacy/plugins/infra/public/utils/use_url_state.ts @@ -5,10 +5,10 @@ */ import { Location } from 'history'; -import { useMemo, useCallback } from 'react'; +import { useCallback, useEffect, useMemo, useState } from 'react'; import { decode, encode, RisonValue } from 'rison-node'; - import { QueryString } from 'ui/utils/query_string'; + import { useHistory } from './history_context'; export const useUrlState = ({ @@ -16,21 +16,26 @@ export const useUrlState = ({ decodeUrlState, encodeUrlState, urlStateKey, + writeDefaultState = false, }: { defaultState: State; decodeUrlState: (value: RisonValue | undefined) => State | undefined; encodeUrlState: (value: State) => RisonValue | undefined; urlStateKey: string; + writeDefaultState?: boolean; }) => { const history = useHistory(); + // history.location is mutable so we can't reliably use useMemo + const queryString = history?.location ? getQueryStringFromLocation(history.location) : ''; + const urlStateString = useMemo(() => { - if (!history) { + if (!queryString) { return; } - return getParamFromQueryString(getQueryStringFromLocation(history.location), urlStateKey); - }, [history && history.location, urlStateKey]); + return getParamFromQueryString(queryString, urlStateKey); + }, [queryString, urlStateKey]); const decodedState = useMemo(() => decodeUrlState(decodeRisonUrlState(urlStateString)), [ decodeUrlState, @@ -44,27 +49,38 @@ export const useUrlState = ({ const setState = useCallback( (newState: State | undefined) => { - if (!history) { + if (!history || !history.location) { return; } - const location = history.location; + const currentLocation = history.location; const newLocation = replaceQueryStringInLocation( - location, + currentLocation, replaceStateKeyInQueryString( urlStateKey, typeof newState !== 'undefined' ? encodeUrlState(newState) : undefined - )(getQueryStringFromLocation(location)) + )(getQueryStringFromLocation(currentLocation)) ); - if (newLocation !== location) { + if (newLocation !== currentLocation) { history.replace(newLocation); } }, - [encodeUrlState, history, history && history.location, urlStateKey] + [encodeUrlState, history, urlStateKey] ); + const [shouldInitialize, setShouldInitialize] = useState( + writeDefaultState && typeof decodedState === 'undefined' + ); + + useEffect(() => { + if (shouldInitialize) { + setShouldInitialize(false); + setState(defaultState); + } + }, [shouldInitialize, setState, defaultState]); + return [state, setState] as [typeof state, typeof setState]; }; diff --git a/x-pack/legacy/plugins/infra/public/utils/use_visibility_state.ts b/x-pack/legacy/plugins/infra/public/utils/use_visibility_state.ts index 5763834b1cc2a..f4d8b572e4f7f 100644 --- a/x-pack/legacy/plugins/infra/public/utils/use_visibility_state.ts +++ b/x-pack/legacy/plugins/infra/public/utils/use_visibility_state.ts @@ -20,6 +20,6 @@ export const useVisibilityState = (initialState: boolean) => { show, toggle, }), - [isVisible, show, hide] + [hide, isVisible, show, toggle] ); }; From 489b39cfe7cc88dfc2ba77d2f8af93fdf08c36db Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 11 Dec 2019 14:14:25 +0100 Subject: [PATCH 3/4] Re-enable datemath in from/to canvas timelion args (#52159) --- .../canvas/public/functions/timelion.ts | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/x-pack/legacy/plugins/canvas/public/functions/timelion.ts b/x-pack/legacy/plugins/canvas/public/functions/timelion.ts index ee7dd981009d9..4377f2cb4d53b 100644 --- a/x-pack/legacy/plugins/canvas/public/functions/timelion.ts +++ b/x-pack/legacy/plugins/canvas/public/functions/timelion.ts @@ -5,7 +5,10 @@ */ import { flatten } from 'lodash'; +import moment from 'moment-timezone'; import chrome from 'ui/chrome'; +import { npStart } from 'ui/new_platform'; +import { TimeRange } from 'src/plugins/data/common'; import { ExpressionFunction, DatatableRow } from 'src/plugins/expressions/public'; import { fetch } from '../../common/lib/fetch'; // @ts-ignore untyped local @@ -21,6 +24,26 @@ interface Arguments { timezone: string; } +/** + * This function parses a given time range containing date math + * and returns ISO dates. Parsing is done respecting the given time zone. + * @param timeRange time range to parse + * @param timeZone time zone to do the parsing in + */ +function parseDateMath(timeRange: TimeRange, timeZone: string) { + // the datemath plugin always parses dates by using the current default moment time zone. + // to use the configured time zone, we are switching just for the bounds calculation. + const defaultTimezone = moment().zoneName(); + moment.tz.setDefault(timeZone); + + const parsedRange = npStart.plugins.data.query.timefilter.timefilter.calculateBounds(timeRange); + + // reset default moment timezone + moment.tz.setDefault(defaultTimezone); + + return parsedRange; +} + export function timelion(): ExpressionFunction<'timelion', Filter, Arguments, Promise> { const { help, args: argHelp } = getFunctionHelp().timelion; @@ -64,8 +87,8 @@ export function timelion(): ExpressionFunction<'timelion', Filter, Arguments, Pr // workpad, if it exists. Otherwise fall back on the function args. const timeFilter = context.and.find(and => and.type === 'time'); const range = timeFilter - ? { from: timeFilter.from, to: timeFilter.to } - : { from: args.from, to: args.to }; + ? { min: timeFilter.from, max: timeFilter.to } + : parseDateMath({ from: args.from, to: args.to }, args.timezone); const body = { extended: { @@ -79,8 +102,8 @@ export function timelion(): ExpressionFunction<'timelion', Filter, Arguments, Pr }, sheet: [args.query], time: { - from: range.from, - to: range.to, + from: range.min, + to: range.max, interval: args.interval, timezone: args.timezone, }, From b6ea6990c0b679aa890ff87b161f78485f3b7200 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 11 Dec 2019 14:19:28 +0100 Subject: [PATCH 4/4] Migrate url shortener service (#50896) --- .github/CODEOWNERS | 4 +- .../url_shortening/routes/create_routes.js | 2 - .../server/url_shortening/routes/goto.js | 8 +- .../routes/lib/short_url_lookup.js | 24 ---- .../routes/lib/short_url_lookup.test.js | 37 ------ src/plugins/share/kibana.json | 2 +- .../share/server/index.ts} | 20 +-- src/plugins/share/server/plugin.ts | 37 ++++++ .../share/server/routes/create_routes.ts | 32 +++++ src/plugins/share/server/routes/goto.ts | 64 +++++++++ .../routes/lib/short_url_assert_valid.test.ts | 63 +++++++++ .../routes/lib/short_url_assert_valid.ts | 41 ++++++ .../routes/lib/short_url_lookup.test.ts | 125 ++++++++++++++++++ .../server/routes/lib/short_url_lookup.ts | 84 ++++++++++++ .../share/server/routes/shorten_url.ts | 48 +++++++ .../apis/short_urls/feature_controls.ts | 2 +- 16 files changed, 504 insertions(+), 89 deletions(-) rename src/{legacy/server/url_shortening/routes/shorten_url.js => plugins/share/server/index.ts} (60%) create mode 100644 src/plugins/share/server/plugin.ts create mode 100644 src/plugins/share/server/routes/create_routes.ts create mode 100644 src/plugins/share/server/routes/goto.ts create mode 100644 src/plugins/share/server/routes/lib/short_url_assert_valid.test.ts create mode 100644 src/plugins/share/server/routes/lib/short_url_assert_valid.ts create mode 100644 src/plugins/share/server/routes/lib/short_url_lookup.test.ts create mode 100644 src/plugins/share/server/routes/lib/short_url_lookup.ts create mode 100644 src/plugins/share/server/routes/shorten_url.ts diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index c5e6768c17d46..338fbf2e359b7 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -5,6 +5,8 @@ # App /x-pack/legacy/plugins/lens/ @elastic/kibana-app /x-pack/legacy/plugins/graph/ @elastic/kibana-app +/src/plugins/share/ @elastic/kibana-app +/src/legacy/server/url_shortening/ @elastic/kibana-app /src/legacy/server/sample_data/ @elastic/kibana-app # App Architecture @@ -14,7 +16,6 @@ /src/plugins/kibana_react/ @elastic/kibana-app-arch /src/plugins/kibana_utils/ @elastic/kibana-app-arch /src/plugins/navigation/ @elastic/kibana-app-arch -/src/plugins/share/ @elastic/kibana-app-arch /src/plugins/ui_actions/ @elastic/kibana-app-arch /src/plugins/visualizations/ @elastic/kibana-app-arch /x-pack/plugins/advanced_ui_actions/ @elastic/kibana-app-arch @@ -28,7 +29,6 @@ /src/legacy/core_plugins/kibana/server/routes/api/suggestions/ @elastic/kibana-app-arch /src/legacy/core_plugins/visualizations/ @elastic/kibana-app-arch /src/legacy/server/index_patterns/ @elastic/kibana-app-arch -/src/legacy/server/url_shortening/ @elastic/kibana-app-arch # APM /x-pack/legacy/plugins/apm/ @elastic/apm-ui diff --git a/src/legacy/server/url_shortening/routes/create_routes.js b/src/legacy/server/url_shortening/routes/create_routes.js index 091eabcf47c1f..c6347ace873f7 100644 --- a/src/legacy/server/url_shortening/routes/create_routes.js +++ b/src/legacy/server/url_shortening/routes/create_routes.js @@ -19,12 +19,10 @@ import { shortUrlLookupProvider } from './lib/short_url_lookup'; import { createGotoRoute } from './goto'; -import { createShortenUrlRoute } from './shorten_url'; export function createRoutes(server) { const shortUrlLookup = shortUrlLookupProvider(server); server.route(createGotoRoute({ server, shortUrlLookup })); - server.route(createShortenUrlRoute({ shortUrlLookup })); } diff --git a/src/legacy/server/url_shortening/routes/goto.js b/src/legacy/server/url_shortening/routes/goto.js index 675bc5df50670..60a34499dd2d5 100644 --- a/src/legacy/server/url_shortening/routes/goto.js +++ b/src/legacy/server/url_shortening/routes/goto.js @@ -22,18 +22,12 @@ import { shortUrlAssertValid } from './lib/short_url_assert_valid'; export const createGotoRoute = ({ server, shortUrlLookup }) => ({ method: 'GET', - path: '/goto/{urlId}', + path: '/goto_LP/{urlId}', handler: async function (request, h) { try { const url = await shortUrlLookup.getUrl(request.params.urlId, request); shortUrlAssertValid(url); - const uiSettings = request.getUiSettingsService(); - const stateStoreInSessionStorage = await uiSettings.get('state:storeInSessionStorage'); - if (!stateStoreInSessionStorage) { - return h.redirect(request.getBasePath() + url); - } - const app = server.getHiddenUiAppById('stateSessionStorageRedirect'); return h.renderApp(app, { redirectUrl: url, diff --git a/src/legacy/server/url_shortening/routes/lib/short_url_lookup.js b/src/legacy/server/url_shortening/routes/lib/short_url_lookup.js index c4f6af03d7d93..3a4b96c802c58 100644 --- a/src/legacy/server/url_shortening/routes/lib/short_url_lookup.js +++ b/src/legacy/server/url_shortening/routes/lib/short_url_lookup.js @@ -17,7 +17,6 @@ * under the License. */ -import crypto from 'crypto'; import { get } from 'lodash'; export function shortUrlLookupProvider(server) { @@ -34,29 +33,6 @@ export function shortUrlLookupProvider(server) { } return { - async generateUrlId(url, req) { - const id = crypto.createHash('md5').update(url).digest('hex'); - const savedObjectsClient = req.getSavedObjectsClient(); - const { isConflictError } = savedObjectsClient.errors; - - try { - const doc = await savedObjectsClient.create('url', { - url, - accessCount: 0, - createDate: new Date(), - accessDate: new Date() - }, { id }); - - return doc.id; - } catch (error) { - if (isConflictError(error)) { - return id; - } - - throw error; - } - }, - async getUrl(id, req) { const doc = await req.getSavedObjectsClient().get('url', id); updateMetadata(doc, req); diff --git a/src/legacy/server/url_shortening/routes/lib/short_url_lookup.test.js b/src/legacy/server/url_shortening/routes/lib/short_url_lookup.test.js index 033aeb92926a5..7303682c63e0b 100644 --- a/src/legacy/server/url_shortening/routes/lib/short_url_lookup.test.js +++ b/src/legacy/server/url_shortening/routes/lib/short_url_lookup.test.js @@ -48,43 +48,6 @@ describe('shortUrlLookupProvider', () => { sandbox.restore(); }); - describe('generateUrlId', () => { - it('returns the document id', async () => { - const id = await shortUrl.generateUrlId(URL, req); - expect(id).toEqual(ID); - }); - - it('provides correct arguments to savedObjectsClient', async () => { - await shortUrl.generateUrlId(URL, req); - - sinon.assert.calledOnce(savedObjectsClient.create); - const [type, attributes, options] = savedObjectsClient.create.getCall(0).args; - - expect(type).toEqual(TYPE); - expect(Object.keys(attributes).sort()).toEqual(['accessCount', 'accessDate', 'createDate', 'url']); - expect(attributes.url).toEqual(URL); - expect(options.id).toEqual(ID); - }); - - it('passes persists attributes', async () => { - await shortUrl.generateUrlId(URL, req); - - sinon.assert.calledOnce(savedObjectsClient.create); - const [type, attributes] = savedObjectsClient.create.getCall(0).args; - - expect(type).toEqual(TYPE); - expect(Object.keys(attributes).sort()).toEqual(['accessCount', 'accessDate', 'createDate', 'url']); - expect(attributes.url).toEqual(URL); - }); - - it('gracefully handles version conflict', async () => { - const error = savedObjectsClient.errors.decorateConflictError(new Error()); - savedObjectsClient.create.throws(error); - const id = await shortUrl.generateUrlId(URL, req); - expect(id).toEqual(ID); - }); - }); - describe('getUrl', () => { beforeEach(() => { const attributes = { accessCount: 2, url: URL }; diff --git a/src/plugins/share/kibana.json b/src/plugins/share/kibana.json index bbe393a76c5da..dce2ac9281aba 100644 --- a/src/plugins/share/kibana.json +++ b/src/plugins/share/kibana.json @@ -1,6 +1,6 @@ { "id": "share", "version": "kibana", - "server": false, + "server": true, "ui": true } diff --git a/src/legacy/server/url_shortening/routes/shorten_url.js b/src/plugins/share/server/index.ts similarity index 60% rename from src/legacy/server/url_shortening/routes/shorten_url.js rename to src/plugins/share/server/index.ts index 0203e9373384a..9e574314f8000 100644 --- a/src/legacy/server/url_shortening/routes/shorten_url.js +++ b/src/plugins/share/server/index.ts @@ -17,19 +17,9 @@ * under the License. */ -import { handleShortUrlError } from './lib/short_url_error'; -import { shortUrlAssertValid } from './lib/short_url_assert_valid'; +import { PluginInitializerContext } from '../../../core/server'; +import { SharePlugin } from './plugin'; -export const createShortenUrlRoute = ({ shortUrlLookup }) => ({ - method: 'POST', - path: '/api/shorten_url', - handler: async function (request) { - try { - shortUrlAssertValid(request.payload.url); - const urlId = await shortUrlLookup.generateUrlId(request.payload.url, request); - return { urlId }; - } catch (err) { - throw handleShortUrlError(err); - } - } -}); +export function plugin(initializerContext: PluginInitializerContext) { + return new SharePlugin(initializerContext); +} diff --git a/src/plugins/share/server/plugin.ts b/src/plugins/share/server/plugin.ts new file mode 100644 index 0000000000000..bcb681a50652a --- /dev/null +++ b/src/plugins/share/server/plugin.ts @@ -0,0 +1,37 @@ +/* + * 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 { CoreSetup, Plugin, PluginInitializerContext } from 'kibana/server'; +import { createRoutes } from './routes/create_routes'; + +export class SharePlugin implements Plugin { + constructor(private readonly initializerContext: PluginInitializerContext) {} + + public async setup(core: CoreSetup) { + createRoutes(core, this.initializerContext.logger.get()); + } + + public start() { + this.initializerContext.logger.get().debug('Starting plugin'); + } + + public stop() { + this.initializerContext.logger.get().debug('Stopping plugin'); + } +} diff --git a/src/plugins/share/server/routes/create_routes.ts b/src/plugins/share/server/routes/create_routes.ts new file mode 100644 index 0000000000000..bd4b6fdb08791 --- /dev/null +++ b/src/plugins/share/server/routes/create_routes.ts @@ -0,0 +1,32 @@ +/* + * 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 { CoreSetup, Logger } from 'kibana/server'; + +import { shortUrlLookupProvider } from './lib/short_url_lookup'; +import { createGotoRoute } from './goto'; +import { createShortenUrlRoute } from './shorten_url'; + +export function createRoutes({ http }: CoreSetup, logger: Logger) { + const shortUrlLookup = shortUrlLookupProvider({ logger }); + const router = http.createRouter(); + + createGotoRoute({ router, shortUrlLookup, http }); + createShortenUrlRoute({ router, shortUrlLookup }); +} diff --git a/src/plugins/share/server/routes/goto.ts b/src/plugins/share/server/routes/goto.ts new file mode 100644 index 0000000000000..7343dc1bd34a2 --- /dev/null +++ b/src/plugins/share/server/routes/goto.ts @@ -0,0 +1,64 @@ +/* + * 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 { CoreSetup, IRouter } from 'kibana/server'; +import { schema } from '@kbn/config-schema'; + +import { shortUrlAssertValid } from './lib/short_url_assert_valid'; +import { ShortUrlLookupService } from './lib/short_url_lookup'; + +export const createGotoRoute = ({ + router, + shortUrlLookup, + http, +}: { + router: IRouter; + shortUrlLookup: ShortUrlLookupService; + http: CoreSetup['http']; +}) => { + router.get( + { + path: '/goto/{urlId}', + validate: { + params: schema.object({ urlId: schema.string() }), + }, + }, + router.handleLegacyErrors(async function(context, request, response) { + const url = await shortUrlLookup.getUrl(request.params.urlId, { + savedObjects: context.core.savedObjects.client, + }); + shortUrlAssertValid(url); + + const uiSettings = context.core.uiSettings.client; + const stateStoreInSessionStorage = await uiSettings.get('state:storeInSessionStorage'); + if (!stateStoreInSessionStorage) { + return response.redirected({ + headers: { + location: http.basePath.prepend(url), + }, + }); + } + return response.redirected({ + headers: { + location: http.basePath.prepend('/goto_LP/' + request.params.urlId), + }, + }); + }) + ); +}; diff --git a/src/plugins/share/server/routes/lib/short_url_assert_valid.test.ts b/src/plugins/share/server/routes/lib/short_url_assert_valid.test.ts new file mode 100644 index 0000000000000..f83073e6aefe9 --- /dev/null +++ b/src/plugins/share/server/routes/lib/short_url_assert_valid.test.ts @@ -0,0 +1,63 @@ +/* + * 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 { shortUrlAssertValid } from './short_url_assert_valid'; + +describe('shortUrlAssertValid()', () => { + const invalid = [ + ['protocol', 'http://localhost:5601/app/kibana'], + ['protocol', 'https://localhost:5601/app/kibana'], + ['protocol', 'mailto:foo@bar.net'], + ['protocol', 'javascript:alert("hi")'], // eslint-disable-line no-script-url + ['hostname', 'localhost/app/kibana'], + ['hostname and port', 'local.host:5601/app/kibana'], + ['hostname and auth', 'user:pass@localhost.net/app/kibana'], + ['path traversal', '/app/../../not-kibana'], + ['deep path', '/app/kibana/foo'], + ['deep path', '/app/kibana/foo/bar'], + ['base path', '/base/app/kibana'], + ]; + + invalid.forEach(([desc, url]) => { + it(`fails when url has ${desc}`, () => { + try { + shortUrlAssertValid(url); + throw new Error(`expected assertion to throw`); + } catch (err) { + if (!err || !err.isBoom) { + throw err; + } + } + }); + }); + + const valid = [ + '/app/kibana', + '/app/monitoring#angular/route', + '/app/text#document-id', + '/app/some?with=query', + '/app/some?with=query#and-a-hash', + ]; + + valid.forEach(url => { + it(`allows ${url}`, () => { + shortUrlAssertValid(url); + }); + }); +}); diff --git a/src/plugins/share/server/routes/lib/short_url_assert_valid.ts b/src/plugins/share/server/routes/lib/short_url_assert_valid.ts new file mode 100644 index 0000000000000..2f120bbc03cd7 --- /dev/null +++ b/src/plugins/share/server/routes/lib/short_url_assert_valid.ts @@ -0,0 +1,41 @@ +/* + * 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 { parse } from 'url'; +import { trim } from 'lodash'; +import Boom from 'boom'; + +export function shortUrlAssertValid(url: string) { + const { protocol, hostname, pathname } = parse(url); + + if (protocol) { + throw Boom.notAcceptable(`Short url targets cannot have a protocol, found "${protocol}"`); + } + + if (hostname) { + throw Boom.notAcceptable(`Short url targets cannot have a hostname, found "${hostname}"`); + } + + const pathnameParts = trim(pathname, '/').split('/'); + if (pathnameParts.length !== 2) { + throw Boom.notAcceptable( + `Short url target path must be in the format "/app/{{appId}}", found "${pathname}"` + ); + } +} diff --git a/src/plugins/share/server/routes/lib/short_url_lookup.test.ts b/src/plugins/share/server/routes/lib/short_url_lookup.test.ts new file mode 100644 index 0000000000000..87e2b7b726e59 --- /dev/null +++ b/src/plugins/share/server/routes/lib/short_url_lookup.test.ts @@ -0,0 +1,125 @@ +/* + * 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 { shortUrlLookupProvider, ShortUrlLookupService } from './short_url_lookup'; +import { SavedObjectsClientContract, Logger } from 'kibana/server'; +import { SavedObjectsClient } from '../../../../../core/server'; + +describe('shortUrlLookupProvider', () => { + const ID = 'bf00ad16941fc51420f91a93428b27a0'; + const TYPE = 'url'; + const URL = 'http://elastic.co'; + + let savedObjects: jest.Mocked; + let deps: { savedObjects: SavedObjectsClientContract }; + let shortUrl: ShortUrlLookupService; + + beforeEach(() => { + savedObjects = ({ + get: jest.fn(), + create: jest.fn(() => Promise.resolve({ id: ID })), + update: jest.fn(), + errors: SavedObjectsClient.errors, + } as unknown) as jest.Mocked; + + deps = { savedObjects }; + shortUrl = shortUrlLookupProvider({ logger: ({ warn: () => {} } as unknown) as Logger }); + }); + + describe('generateUrlId', () => { + it('returns the document id', async () => { + const id = await shortUrl.generateUrlId(URL, deps); + expect(id).toEqual(ID); + }); + + it('provides correct arguments to savedObjectsClient', async () => { + await shortUrl.generateUrlId(URL, { savedObjects }); + + expect(savedObjects.create).toHaveBeenCalledTimes(1); + const [type, attributes, options] = savedObjects.create.mock.calls[0]; + + expect(type).toEqual(TYPE); + expect(Object.keys(attributes).sort()).toEqual([ + 'accessCount', + 'accessDate', + 'createDate', + 'url', + ]); + expect(attributes.url).toEqual(URL); + expect(options!.id).toEqual(ID); + }); + + it('passes persists attributes', async () => { + await shortUrl.generateUrlId(URL, deps); + + expect(savedObjects.create).toHaveBeenCalledTimes(1); + const [type, attributes] = savedObjects.create.mock.calls[0]; + + expect(type).toEqual(TYPE); + expect(Object.keys(attributes).sort()).toEqual([ + 'accessCount', + 'accessDate', + 'createDate', + 'url', + ]); + expect(attributes.url).toEqual(URL); + }); + + it('gracefully handles version conflict', async () => { + const error = savedObjects.errors.decorateConflictError(new Error()); + savedObjects.create.mockImplementation(() => { + throw error; + }); + const id = await shortUrl.generateUrlId(URL, deps); + expect(id).toEqual(ID); + }); + }); + + describe('getUrl', () => { + beforeEach(() => { + const attributes = { accessCount: 2, url: URL }; + savedObjects.get.mockResolvedValue({ id: ID, attributes, type: 'url', references: [] }); + }); + + it('provides the ID to savedObjectsClient', async () => { + await shortUrl.getUrl(ID, { savedObjects }); + + expect(savedObjects.get).toHaveBeenCalledTimes(1); + expect(savedObjects.get).toHaveBeenCalledWith(TYPE, ID); + }); + + it('returns the url', async () => { + const response = await shortUrl.getUrl(ID, deps); + expect(response).toEqual(URL); + }); + + it('increments accessCount', async () => { + await shortUrl.getUrl(ID, { savedObjects }); + + expect(savedObjects.update).toHaveBeenCalledTimes(1); + + const [type, id, attributes] = savedObjects.update.mock.calls[0]; + + expect(type).toEqual(TYPE); + expect(id).toEqual(ID); + expect(Object.keys(attributes).sort()).toEqual(['accessCount', 'accessDate']); + expect(attributes.accessCount).toEqual(3); + }); + }); +}); diff --git a/src/plugins/share/server/routes/lib/short_url_lookup.ts b/src/plugins/share/server/routes/lib/short_url_lookup.ts new file mode 100644 index 0000000000000..0d8a9c86621de --- /dev/null +++ b/src/plugins/share/server/routes/lib/short_url_lookup.ts @@ -0,0 +1,84 @@ +/* + * 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 crypto from 'crypto'; +import { get } from 'lodash'; + +import { Logger, SavedObject, SavedObjectsClientContract } from 'kibana/server'; + +export interface ShortUrlLookupService { + generateUrlId(url: string, deps: { savedObjects: SavedObjectsClientContract }): Promise; + getUrl(url: string, deps: { savedObjects: SavedObjectsClientContract }): Promise; +} + +export function shortUrlLookupProvider({ logger }: { logger: Logger }): ShortUrlLookupService { + async function updateMetadata( + doc: SavedObject, + { savedObjects }: { savedObjects: SavedObjectsClientContract } + ) { + try { + await savedObjects.update('url', doc.id, { + accessDate: new Date().valueOf(), + accessCount: get(doc, 'attributes.accessCount', 0) + 1, + }); + } catch (error) { + logger.warn('Warning: Error updating url metadata'); + logger.warn(error); + // swallow errors. It isn't critical if there is no update. + } + } + + return { + async generateUrlId(url, { savedObjects }) { + const id = crypto + .createHash('md5') + .update(url) + .digest('hex'); + const { isConflictError } = savedObjects.errors; + + try { + const doc = await savedObjects.create( + 'url', + { + url, + accessCount: 0, + createDate: new Date().valueOf(), + accessDate: new Date().valueOf(), + }, + { id } + ); + + return doc.id; + } catch (error) { + if (isConflictError(error)) { + return id; + } + + throw error; + } + }, + + async getUrl(id, { savedObjects }) { + const doc = await savedObjects.get('url', id); + updateMetadata(doc, { savedObjects }); + + return doc.attributes.url; + }, + }; +} diff --git a/src/plugins/share/server/routes/shorten_url.ts b/src/plugins/share/server/routes/shorten_url.ts new file mode 100644 index 0000000000000..116b90c6971c5 --- /dev/null +++ b/src/plugins/share/server/routes/shorten_url.ts @@ -0,0 +1,48 @@ +/* + * 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 { IRouter } from 'kibana/server'; +import { schema } from '@kbn/config-schema'; + +import { shortUrlAssertValid } from './lib/short_url_assert_valid'; +import { ShortUrlLookupService } from './lib/short_url_lookup'; + +export const createShortenUrlRoute = ({ + shortUrlLookup, + router, +}: { + shortUrlLookup: ShortUrlLookupService; + router: IRouter; +}) => { + router.post( + { + path: '/api/shorten_url', + validate: { + body: schema.object({ url: schema.string() }), + }, + }, + router.handleLegacyErrors(async function(context, request, response) { + shortUrlAssertValid(request.body.url); + const urlId = await shortUrlLookup.generateUrlId(request.body.url, { + savedObjects: context.core.savedObjects.client, + }); + return response.ok({ body: { urlId } }); + }) + ); +}; diff --git a/x-pack/test/api_integration/apis/short_urls/feature_controls.ts b/x-pack/test/api_integration/apis/short_urls/feature_controls.ts index 06fd971399ea3..db5e11ef367ad 100644 --- a/x-pack/test/api_integration/apis/short_urls/feature_controls.ts +++ b/x-pack/test/api_integration/apis/short_urls/feature_controls.ts @@ -107,7 +107,7 @@ export default function featureControlsTests({ getService }: FtrProviderContext) expect(resp.status).to.eql(302); expect(resp.headers.location).to.eql('/app/kibana#foo/bar/baz'); } else { - expect(resp.status).to.eql(500); + expect(resp.status).to.eql(403); expect(resp.headers.location).to.eql(undefined); } });