From 57c49a0045e6aa5e6812269109a1a928c59f7a60 Mon Sep 17 00:00:00 2001 From: asalem Date: Thu, 18 Jun 2020 13:35:11 -0700 Subject: [PATCH] chore(PR-suggestions): made suggested PR changes by Adding context to where the hashing function was found Renaming the data src to queryCache Renaming the imported function in TimeSeries Renaming some of the input parameters to be a little more relevant --- .../dashboards/components/DashboardPage.tsx | 2 +- ui/src/data/actions/index.ts | 18 ---------- ui/src/queryCache/actions/index.ts | 33 +++++++++++++++++++ ui/src/{data => queryCache}/reducers/index.ts | 18 +++++----- ui/src/shared/components/TimeSeries.tsx | 20 ++++++++--- ui/src/store/configureStore.ts | 4 +-- ui/src/types/stores.ts | 4 +-- ui/src/views/actions/thunks.ts | 5 ++- 8 files changed, 63 insertions(+), 41 deletions(-) delete mode 100644 ui/src/data/actions/index.ts create mode 100644 ui/src/queryCache/actions/index.ts rename ui/src/{data => queryCache}/reducers/index.ts (54%) diff --git a/ui/src/dashboards/components/DashboardPage.tsx b/ui/src/dashboards/components/DashboardPage.tsx index ab52c98a657..103a2f3a0e7 100644 --- a/ui/src/dashboards/components/DashboardPage.tsx +++ b/ui/src/dashboards/components/DashboardPage.tsx @@ -17,7 +17,7 @@ import RateLimitAlert from 'src/cloud/components/RateLimitAlert' import {pageTitleSuffixer} from 'src/shared/utils/pageTitles' // Selectors & Actions -import {resetCachedQueryResults} from 'src/data/actions' +import {resetCachedQueryResults} from 'src/queryCache/actions' import {getByID} from 'src/resources/selectors' // Types diff --git a/ui/src/data/actions/index.ts b/ui/src/data/actions/index.ts deleted file mode 100644 index c50a5425741..00000000000 --- a/ui/src/data/actions/index.ts +++ /dev/null @@ -1,18 +0,0 @@ -export type Action = - | ReturnType - | ReturnType - -export const hashCode = s => - s.split('').reduce((a, b) => ((a << 5) - a + b.charCodeAt(0)) | 0, 0) - -export const setQueryResultsByQueryID = (queryID: string, files: string[]) => - ({ - type: 'SET_QUERY_RESULTS_BY_QUERY', - queryID: hashCode(queryID), - files, - } as const) - -export const resetCachedQueryResults = () => - ({ - type: 'RESET_CACHED_QUERY_RESULTS', - } as const) diff --git a/ui/src/queryCache/actions/index.ts b/ui/src/queryCache/actions/index.ts new file mode 100644 index 00000000000..3778644a27a --- /dev/null +++ b/ui/src/queryCache/actions/index.ts @@ -0,0 +1,33 @@ +export type Action = + | ReturnType + | ReturnType + +// Hashing function found here: +// https://jsperf.com/hashcodelordvlad +// Through this thread: +// https://stackoverflow.com/questions/7616461/generate-a-hash-from-string-in-javascript +export const hashCode = (queryText: string): string => { + let hash = 0, + char + if (!queryText) { + return `${hash}` + } + for (let i = 0; i < queryText.length; i++) { + char = queryText.charCodeAt(i) + hash = (hash << 5) - hash + char + hash |= 0 // Convert to 32bit integer + } + return `${hash}` +} + +export const setQueryResultsByQueryID = (queryID: string, files: string[]) => + ({ + type: 'SET_QUERY_RESULTS_BY_QUERY', + queryID: hashCode(queryID), + files, + } as const) + +export const resetCachedQueryResults = () => + ({ + type: 'RESET_CACHED_QUERY_RESULTS', + } as const) diff --git a/ui/src/data/reducers/index.ts b/ui/src/queryCache/reducers/index.ts similarity index 54% rename from ui/src/data/reducers/index.ts rename to ui/src/queryCache/reducers/index.ts index d1a5c7fc87d..ea9b78d3e4e 100644 --- a/ui/src/data/reducers/index.ts +++ b/ui/src/queryCache/reducers/index.ts @@ -2,32 +2,30 @@ import {produce} from 'immer' // Actions -import {Action} from 'src/data/actions' +import {Action} from 'src/queryCache/actions' -export interface DataState { +export interface QueryCacheState { queryResultsByQueryID: {[queryID: string]: string[]} } -export const initialState: DataState = { +export const initialState: QueryCacheState = { queryResultsByQueryID: {}, } -export const dataReducer = ( - state: DataState = initialState, +export const queryCacheReducer = ( + state: QueryCacheState = initialState, action: Action -): DataState => { +): QueryCacheState => { switch (action.type) { case 'SET_QUERY_RESULTS_BY_QUERY': { return produce(state, draftState => { const {queryID, files} = action - if (queryID && files.length) { - draftState.queryResultsByQueryID[queryID] = files - } + draftState.queryResultsByQueryID[queryID] = files }) } case 'RESET_CACHED_QUERY_RESULTS': { - return initialState + return {queryResultsByQueryID: {}} } } diff --git a/ui/src/shared/components/TimeSeries.tsx b/ui/src/shared/components/TimeSeries.tsx index 5c2a63bb4c1..e85ef77027c 100644 --- a/ui/src/shared/components/TimeSeries.tsx +++ b/ui/src/shared/components/TimeSeries.tsx @@ -32,6 +32,7 @@ import { isDemoDataAvailabilityError, demoDataError, } from 'src/cloud/utils/demoDataErrors' +import {hashCode} from 'src/queryCache/actions' // Constants import { @@ -43,7 +44,7 @@ import {TIME_RANGE_START, TIME_RANGE_STOP} from 'src/variables/constants' // Actions import {notify as notifyAction} from 'src/shared/actions/notifications' -import {setQueryResultsByQueryID} from 'src/data/actions' +import {setQueryResultsByQueryID} from 'src/queryCache/actions' // Types import { @@ -88,7 +89,7 @@ interface OwnProps { interface DispatchProps { notify: typeof notifyAction - setQueryResults: typeof setQueryResultsByQueryID + onSetQueryResultsByQueryID: typeof setQueryResultsByQueryID } type Props = StateProps & OwnProps & DispatchProps @@ -183,7 +184,13 @@ class TimeSeries extends Component { } private reload = async () => { - const {variables, setQueryResults, notify, check, buckets} = this.props + const { + buckets, + check, + notify, + onSetQueryResultsByQueryID, + variables, + } = this.props const queries = this.props.queries.filter(({text}) => !!text.trim()) if (!queries.length) { @@ -274,7 +281,10 @@ class TimeSeries extends Component { this.pendingReload = false // set the results per the view const queryText = queries[0].text - setQueryResults(queryText, files) + const queryID = hashCode(queryText) + if (queryID && files.length) { + onSetQueryResultsByQueryID(queryID, files) + } this.setState({ giraffeResult, @@ -349,7 +359,7 @@ const mstp = (state: AppState, props: OwnProps): StateProps => { const mdtp: DispatchProps = { notify: notifyAction, - setQueryResults: setQueryResultsByQueryID, + onSetQueryResultsByQueryID: setQueryResultsByQueryID, } export default connect( diff --git a/ui/src/store/configureStore.ts b/ui/src/store/configureStore.ts index d688389e858..a3f4ce3c0a4 100644 --- a/ui/src/store/configureStore.ts +++ b/ui/src/store/configureStore.ts @@ -55,7 +55,7 @@ import alertBuilderReducer from 'src/alerting/reducers/alertBuilder' // Types import {AppState, LocalStorage} from 'src/types' -import {dataReducer} from 'src/data/reducers' +import {queryCacheReducer} from 'src/queryCache/reducers' type ReducerState = Pick> @@ -74,7 +74,6 @@ export const rootReducer = combineReducers({ }), currentPage: currentPageReducer, currentDashboard: currentDashboardReducer, - data: dataReducer, dataLoading: dataLoadingReducer, me: meReducer, flags: flagReducer, @@ -83,6 +82,7 @@ export const rootReducer = combineReducers({ overlays: overlaysReducer, plugins: pluginsResourceReducer, predicates: predicatesReducer, + queryCache: queryCacheReducer, ranges: rangesReducer, resources: combineReducers({ buckets: bucketsReducer, diff --git a/ui/src/types/stores.ts b/ui/src/types/stores.ts index 4205223c456..5d2773b7447 100644 --- a/ui/src/types/stores.ts +++ b/ui/src/types/stores.ts @@ -30,7 +30,7 @@ import {AlertBuilderState} from 'src/alerting/reducers/alertBuilder' import {CurrentPage} from 'src/shared/reducers/currentPage' import {DemoDataState} from 'src/cloud/reducers/demodata' import {OrgSettingsState} from 'src/cloud/reducers/orgsettings' -import {DataState} from 'src/data/reducers' +import {QueryCacheState} from 'src/queryCache/reducers' export interface AppState { alertBuilder: AlertBuilderState @@ -43,7 +43,7 @@ export interface AppState { } currentPage: CurrentPage currentDashboard: CurrentDashboardState - data: DataState + queryCache: QueryCacheState dataLoading: DataLoadingState links: Links me: MeState diff --git a/ui/src/views/actions/thunks.ts b/ui/src/views/actions/thunks.ts index b22e48baa54..2f37ecc8f8f 100644 --- a/ui/src/views/actions/thunks.ts +++ b/ui/src/views/actions/thunks.ts @@ -16,7 +16,7 @@ import {notify} from 'src/shared/actions/notifications' import {setActiveTimeMachine} from 'src/timeMachine/actions' import {executeQueries} from 'src/timeMachine/actions/queries' import {setView, Action} from 'src/views/actions/creators' -import {hashCode} from 'src/data/actions' +import {hashCode} from 'src/queryCache/actions' import {getActiveTimeMachine} from 'src/timeMachine/selectors' import {setQueryResults} from 'src/timeMachine/actions/queries' @@ -128,7 +128,7 @@ export const setQueryResultsByQueryID = (queryID: string) => ( ): Promise => { try { const state = getState() - const files = state.data.queryResultsByQueryID[hashCode(queryID)] + const files = state.queryCache.queryResultsByQueryID[hashCode(queryID)] if (files) { dispatch(setQueryResults(RemoteDataState.Done, files, null, null)) return @@ -136,7 +136,6 @@ export const setQueryResultsByQueryID = (queryID: string) => ( dispatch(executeQueries()) } catch (error) { // if the files don't exist in the cache, we want to execute the query - console.error(error) dispatch(executeQueries()) } }