From 7f0f3684b9e197738c39cb894cc4397ca3bd74b4 Mon Sep 17 00:00:00 2001 From: Christoph Jerolimov Date: Tue, 12 Jan 2021 11:39:56 +0100 Subject: [PATCH 1/4] Use localstorage fallback when running a local bridge --- cmd/bridge/main.go | 2 ++ contrib/oc-environment.sh | 3 +++ frontend/@types/console/declarations.d.ts | 1 + .../src/hooks/useUserSettings.ts | 27 +++++++++++++------ pkg/server/server.go | 3 +++ pkg/serverconfig/validate.go | 5 ++++ 6 files changed, 33 insertions(+), 8 deletions(-) diff --git a/cmd/bridge/main.go b/cmd/bridge/main.go index 7a3b9a8d4de..0245303c1fc 100644 --- a/cmd/bridge/main.go +++ b/cmd/bridge/main.go @@ -123,6 +123,7 @@ func main() { fLoadTestFactor := fs.Int("load-test-factor", 0, "DEV ONLY. The factor used to multiply k8s API list responses for load testing purposes.") fDevCatalogCategories := fs.String("developer-catalog-categories", "", "Allow catalog categories customization. (JSON as string)") + fUserSettingsLocation := fs.String("user-settings-location", "configmap", "DEV ONLY. Define where the user settings should be stored. (configmap | localstorage).") if err := serverconfig.Parse(fs, os.Args[1:], "BRIDGE"); err != nil { fmt.Fprintln(os.Stderr, err.Error()) @@ -229,6 +230,7 @@ func main() { LoadTestFactor: *fLoadTestFactor, InactivityTimeout: *fInactivityTimeout, DevCatalogCategories: *fDevCatalogCategories, + UserSettingsLocation: *fUserSettingsLocation, } // if !in-cluster (dev) we should not pass these values to the frontend diff --git a/contrib/oc-environment.sh b/contrib/oc-environment.sh index ba0d45f975d..b6212a8c084 100644 --- a/contrib/oc-environment.sh +++ b/contrib/oc-environment.sh @@ -43,4 +43,7 @@ export BRIDGE_K8S_AUTH BRIDGE_K8S_AUTH_BEARER_TOKEN=$(oc whoami --show-token) export BRIDGE_K8S_AUTH_BEARER_TOKEN +BRIDGE_USER_SETTINGS_LOCATION="localstorage" +export BRIDGE_USER_SETTINGS_LOCATION + echo "Using $BRIDGE_K8S_MODE_OFF_CLUSTER_ENDPOINT" diff --git a/frontend/@types/console/declarations.d.ts b/frontend/@types/console/declarations.d.ts index b514dce1491..6695289d924 100644 --- a/frontend/@types/console/declarations.d.ts +++ b/frontend/@types/console/declarations.d.ts @@ -41,6 +41,7 @@ declare interface Window { GOOS: string; graphqlBaseURL: string; developerCatalogCategories: string; + userSettingsLocation: string; }; windowError?: string; __REDUX_DEVTOOLS_EXTENSION_COMPOSE__?: Function; diff --git a/frontend/packages/console-shared/src/hooks/useUserSettings.ts b/frontend/packages/console-shared/src/hooks/useUserSettings.ts index d5d8f9933f5..44101e28e5b 100644 --- a/frontend/packages/console-shared/src/hooks/useUserSettings.ts +++ b/frontend/packages/console-shared/src/hooks/useUserSettings.ts @@ -16,6 +16,12 @@ import { USER_SETTING_CONFIGMAP_NAMESPACE, } from '../utils/user-settings'; +const alwaysUseFallbackLocalStorage = window.SERVER_FLAGS.userSettingsLocation === 'localstorage'; +if (alwaysUseFallbackLocalStorage) { + // eslint-disable-next-line no-console + console.info('user-settings will be stored in localstorage instead of configmap.'); +} + const useCounterRef = (initialValue: number = 0): [boolean, () => void, () => void] => { const counterRef = React.useRef(initialValue); const increment = React.useCallback(() => { @@ -32,19 +38,22 @@ export const useUserSettings = ( defaultValue?: T, sync: boolean = false, ): [T, React.Dispatch>, boolean] => { - const defaultValueRef = React.useRef(defaultValue); const keyRef = React.useRef(key); + const defaultValueRef = React.useRef(defaultValue); const [isRequestPending, increaseRequest, decreaseRequest] = useCounterRef(); const userUid = useSelector( (state: RootState) => state.UI.get('user')?.metadata?.uid ?? 'kubeadmin', ); const configMapResource = React.useMemo( - () => ({ - kind: ConfigMapModel.kind, - namespace: USER_SETTING_CONFIGMAP_NAMESPACE, - isList: false, - name: `user-settings-${userUid}`, - }), + () => + alwaysUseFallbackLocalStorage + ? null + : { + kind: ConfigMapModel.kind, + namespace: USER_SETTING_CONFIGMAP_NAMESPACE, + isList: false, + name: `user-settings-${userUid}`, + }, [userUid], ); const [cfData, cfLoaded, cfLoadError] = useK8sWatchResource(configMapResource); @@ -53,7 +62,9 @@ export const useUserSettings = ( settingsRef.current = settings; const [loaded, setLoaded] = React.useState(false); - const [fallbackLocalStorage, setFallbackLocalStorage] = React.useState(false); + const [fallbackLocalStorage, setFallbackLocalStorage] = React.useState( + alwaysUseFallbackLocalStorage, + ); const [lsData, setLsDataCallback] = useUserSettingsLocalStorage( keyRef.current, defaultValueRef.current, diff --git a/pkg/server/server.go b/pkg/server/server.go index c1d9201f4f0..9ea79ec0796 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -90,6 +90,7 @@ type jsGlobals struct { GOOS string `json:"GOOS"` GraphQLBaseURL string `json:"graphqlBaseURL"` DevCatalogCategories string `json:"developerCatalogCategories"` + UserSettingsLocation string `json:"userSettingsLocation"` } type Server struct { @@ -134,6 +135,7 @@ type Server struct { PrometheusPublicURL *url.URL ThanosPublicURL *url.URL DevCatalogCategories string + UserSettingsLocation string } func (s *Server) authDisabled() bool { @@ -489,6 +491,7 @@ func (s *Server) indexHandler(w http.ResponseWriter, r *http.Request) { LoadTestFactor: s.LoadTestFactor, GraphQLBaseURL: proxy.SingleJoiningSlash(s.BaseURL.Path, graphQLEndpoint), DevCatalogCategories: s.DevCatalogCategories, + UserSettingsLocation: s.UserSettingsLocation, } if !s.authDisabled() { diff --git a/pkg/serverconfig/validate.go b/pkg/serverconfig/validate.go index 25bb4421114..dc98e1a5373 100644 --- a/pkg/serverconfig/validate.go +++ b/pkg/serverconfig/validate.go @@ -5,12 +5,17 @@ import ( "flag" "fmt" "strings" + + "github.com/openshift/console/pkg/bridge" ) func Validate(fs *flag.FlagSet) error { if _, err := validateDeveloperCatalogCategories(fs.Lookup("developer-catalog-categories").Value.String()); err != nil { return err } + + bridge.ValidateFlagIs("user-settings-location", fs.Lookup("user-settings-location").Value.String(), "configmap", "localstorage") + return nil } From 450d77fdb159978462b318d89c94778b2871fb55 Mon Sep 17 00:00:00 2001 From: Christoph Jerolimov Date: Thu, 17 Dec 2020 11:34:33 +0100 Subject: [PATCH 2/4] Align userSettingsLocalStorage sync option to usersettings hook --- .../src/hooks/useUserSettings.ts | 2 +- .../src/hooks/useUserSettingsLocalStorage.ts | 23 ++++++++----------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/frontend/packages/console-shared/src/hooks/useUserSettings.ts b/frontend/packages/console-shared/src/hooks/useUserSettings.ts index 44101e28e5b..5eb750467f0 100644 --- a/frontend/packages/console-shared/src/hooks/useUserSettings.ts +++ b/frontend/packages/console-shared/src/hooks/useUserSettings.ts @@ -68,7 +68,7 @@ export const useUserSettings = ( const [lsData, setLsDataCallback] = useUserSettingsLocalStorage( keyRef.current, defaultValueRef.current, - fallbackLocalStorage, + fallbackLocalStorage && sync, ); React.useEffect(() => { diff --git a/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts b/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts index dc233cd9f54..162443cb82c 100644 --- a/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts +++ b/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts @@ -11,7 +11,7 @@ const CONFIGMAP_LS_KEY = 'console-user-settings'; export const useUserSettingsLocalStorage = ( key: string, defaultValue: T, - watch: boolean = true, + sync: boolean = false, ): [T, React.Dispatch>] => { const keyRef = React.useRef(key); const defaultValueRef = React.useRef(defaultValue); @@ -32,30 +32,27 @@ export const useUserSettingsLocalStorage = ( lsDataRef.current = lsData; const localStorageUpdated = React.useCallback( - (ev: StorageEvent) => { - if (ev.key === storageConfigNameRef.current) { - const lsConfigMapData = deseralizeData(localStorage.getItem(storageConfigNameRef.current)); - if ( - lsData !== undefined && - lsConfigMapData?.[keyRef.current] && - seralizeData(lsConfigMapData[keyRef.current]) !== seralizeData(lsData) - ) { - setLsData(lsConfigMapData[keyRef.current]); + (event: StorageEvent) => { + if (event.key === storageConfigNameRef.current) { + const lsConfigMapData = deseralizeData(event.newValue); + const newData = lsConfigMapData?.[keyRef.current]; + if (newData && seralizeData(newData) !== seralizeData(lsData)) { + setTimeout(() => setLsData(newData), 100); } } }, [lsData], ); React.useEffect(() => { - if (watch) { + if (sync) { window.addEventListener('storage', localStorageUpdated); } return () => { - if (watch) { + if (sync) { window.removeEventListener('storage', localStorageUpdated); } }; - }, [localStorageUpdated, watch]); + }, [localStorageUpdated, sync]); const updateLsData = React.useCallback>>( (action: React.SetStateAction) => { From 6826f290c7bc8f9a42c3b441b269c62bded3c3e7 Mon Sep 17 00:00:00 2001 From: Christian Vogt Date: Thu, 17 Dec 2020 17:02:06 -0500 Subject: [PATCH 3/4] dispatch storage event to sync current window --- .../src/hooks/useUserSettingsLocalStorage.ts | 38 +++++++++++++------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts b/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts index 162443cb82c..82ee709a9c3 100644 --- a/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts +++ b/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts @@ -31,18 +31,16 @@ export const useUserSettingsLocalStorage = ( const lsDataRef = React.useRef(lsData); lsDataRef.current = lsData; - const localStorageUpdated = React.useCallback( - (event: StorageEvent) => { - if (event.key === storageConfigNameRef.current) { - const lsConfigMapData = deseralizeData(event.newValue); - const newData = lsConfigMapData?.[keyRef.current]; - if (newData && seralizeData(newData) !== seralizeData(lsData)) { - setTimeout(() => setLsData(newData), 100); - } + const localStorageUpdated = React.useCallback((event: StorageEvent) => { + if (event.key === storageConfigNameRef.current) { + const lsConfigMapData = deseralizeData(event.newValue); + const newData = lsConfigMapData?.[keyRef.current]; + if (newData && seralizeData(newData) !== seralizeData(lsDataRef.current)) { + setLsData(newData); } - }, - [lsData], - ); + } + }, []); + React.useEffect(() => { if (sync) { window.addEventListener('storage', localStorageUpdated); @@ -72,7 +70,23 @@ export const useUserSettingsLocalStorage = ( [keyRef.current]: data, }, }; - localStorage.setItem(storageConfigNameRef.current, seralizeData(dataToUpdate)); + const newValue = seralizeData(dataToUpdate); + + // create a storage event to dispatch locally since browser windows do not fire the + // storage event if the change originated from the current window + const event = new StorageEvent('storage', { + storageArea: localStorage, + key: storageConfigNameRef.current, + newValue, + oldValue: localStorage.getItem(storageConfigNameRef.current), + url: window.location.toString(), + }); + + // update local storage + localStorage.setItem(storageConfigNameRef.current, newValue); + + // dispatch local event + window.dispatchEvent(event); } }, [], From 6d71d3cd176a38d54184b3dc7103e34771fb2067 Mon Sep 17 00:00:00 2001 From: Christoph Jerolimov Date: Tue, 12 Jan 2021 14:49:46 +0100 Subject: [PATCH 4/4] Skip user suffix for local developments in localStorage and skip second use of redux useSelector in inner useUserSettingsLocalStorage --- .../src/hooks/useUserSettings.ts | 1 + .../src/hooks/useUserSettingsLocalStorage.ts | 51 ++++++++----------- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/frontend/packages/console-shared/src/hooks/useUserSettings.ts b/frontend/packages/console-shared/src/hooks/useUserSettings.ts index 5eb750467f0..421ba2eb3f4 100644 --- a/frontend/packages/console-shared/src/hooks/useUserSettings.ts +++ b/frontend/packages/console-shared/src/hooks/useUserSettings.ts @@ -66,6 +66,7 @@ export const useUserSettings = ( alwaysUseFallbackLocalStorage, ); const [lsData, setLsDataCallback] = useUserSettingsLocalStorage( + alwaysUseFallbackLocalStorage ? 'console-user-settings' : `console-user-settings-${userUid}`, keyRef.current, defaultValueRef.current, fallbackLocalStorage && sync, diff --git a/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts b/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts index 82ee709a9c3..e08eee315d1 100644 --- a/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts +++ b/frontend/packages/console-shared/src/hooks/useUserSettingsLocalStorage.ts @@ -1,28 +1,18 @@ import * as React from 'react'; -// FIXME upgrading redux types is causing many errors at this time -// eslint-disable-next-line @typescript-eslint/ban-ts-ignore -// @ts-ignore -import { useSelector } from 'react-redux'; -import { RootState } from '@console/internal/redux'; import { deseralizeData, seralizeData } from '../utils/user-settings'; -const CONFIGMAP_LS_KEY = 'console-user-settings'; - export const useUserSettingsLocalStorage = ( - key: string, + localStorageKey: string, + userSettingsKey: string, defaultValue: T, sync: boolean = false, ): [T, React.Dispatch>] => { - const keyRef = React.useRef(key); + const keyRef = React.useRef(userSettingsKey); const defaultValueRef = React.useRef(defaultValue); - const userUid = useSelector( - (state: RootState) => state.UI.get('user')?.metadata?.uid ?? 'kubeadmin', - ); - const storageConfigNameRef = React.useRef(`${CONFIGMAP_LS_KEY}-${userUid}`); const [lsData, setLsData] = React.useState(() => { const valueInLocalStorage = - localStorage.getItem(storageConfigNameRef.current) !== null && - deseralizeData(localStorage.getItem(storageConfigNameRef.current)); + localStorage.getItem(localStorageKey) !== null && + deseralizeData(localStorage.getItem(localStorageKey)); return valueInLocalStorage?.hasOwnProperty(keyRef.current) && valueInLocalStorage[keyRef.current] ? valueInLocalStorage[keyRef.current] @@ -31,15 +21,19 @@ export const useUserSettingsLocalStorage = ( const lsDataRef = React.useRef(lsData); lsDataRef.current = lsData; - const localStorageUpdated = React.useCallback((event: StorageEvent) => { - if (event.key === storageConfigNameRef.current) { - const lsConfigMapData = deseralizeData(event.newValue); - const newData = lsConfigMapData?.[keyRef.current]; - if (newData && seralizeData(newData) !== seralizeData(lsDataRef.current)) { - setLsData(newData); + const localStorageUpdated = React.useCallback( + (event: StorageEvent) => { + if (event.key === localStorageKey) { + const lsConfigMapData = deseralizeData(event.newValue); + const newData = lsConfigMapData?.[keyRef.current]; + + if (newData && seralizeData(newData) !== seralizeData(lsDataRef.current)) { + setLsData(newData); + } } - } - }, []); + }, + [localStorageKey], + ); React.useEffect(() => { if (sync) { @@ -57,8 +51,7 @@ export const useUserSettingsLocalStorage = ( const previousData = lsDataRef.current; const data = typeof action === 'function' ? (action as (prevState: T) => T)(previousData) : action; - const lsConfigMapData = - deseralizeData(localStorage.getItem(storageConfigNameRef.current)) ?? {}; + const lsConfigMapData = deseralizeData(localStorage.getItem(localStorageKey)) ?? {}; if ( data !== undefined && seralizeData(data) !== seralizeData(lsConfigMapData?.[keyRef.current]) @@ -76,20 +69,20 @@ export const useUserSettingsLocalStorage = ( // storage event if the change originated from the current window const event = new StorageEvent('storage', { storageArea: localStorage, - key: storageConfigNameRef.current, + key: localStorageKey, newValue, - oldValue: localStorage.getItem(storageConfigNameRef.current), + oldValue: localStorage.getItem(localStorageKey), url: window.location.toString(), }); // update local storage - localStorage.setItem(storageConfigNameRef.current, newValue); + localStorage.setItem(localStorageKey, newValue); // dispatch local event window.dispatchEvent(event); } }, - [], + [localStorageKey], ); return [lsData, updateLsData];