Refactor session to avoid duplicate calls to api

Author: cesnietor

portal-ui/src/ProtectedRoutes.tsx

@@ -14,25 +14,18 @@
 // You should have received a copy of the GNU Affero General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import React, { useEffect, useState } from "react";
+import { useEffect, useState } from "react";
 import { Navigate, useLocation } from "react-router-dom";
 import useApi from "./screens/Console/Common/Hooks/useApi";
 import { ErrorResponseHandler } from "./common/types";
 import { ReplicationSite } from "./screens/Console/Configurations/SiteReplication/SiteReplication";
 import { useSelector } from "react-redux";
-import {
-  globalSetDistributedSetup,
-  setAnonymousMode,
-  setOverrideStyles,
-  setSiteReplicationInfo,
-  userLogged,
-} from "./systemSlice";
 import { SRInfoStateType } from "./types";
 import { AppState, useAppDispatch } from "./store";
-import { saveSessionResponse } from "./screens/Console/consoleSlice";
-import { getOverrideColorVariants } from "./utils/stylesUtils";
 import LoadingComponent from "./common/LoadingComponent";
-import { api } from "api";
+import { fetchSession } from "./screens/LoginPage/sessionThunk";
+import { setSiteReplicationInfo } from "./systemSlice";
+import { SessionCallStates } from "./screens/Console/consoleSlice";
 
 interface ProtectedRouteProps {
   Component: any;
@@ -41,8 +34,14 @@ interface ProtectedRouteProps {
 const ProtectedRoute = ({ Component }: ProtectedRouteProps) => {
   const dispatch = useAppDispatch();
 
-  const [sessionLoading, setSessionLoading] = useState<boolean>(true);
   const userLoggedIn = useSelector((state: AppState) => state.system.loggedIn);
+  // sessionLoading is used to control rendering this page, if we only use console.sessionLoadingState,
+  // since the state chages it triggers the reload of this component multiple times, causing duplicate queries.
+  const [sessionLoading, setSessionLoading] = useState<boolean>(true);
+  const sessionLoadingState = useSelector(
+    (state: AppState) => state.console.sessionLoadingState
+  );
+
   const anonymousMode = useSelector(
     (state: AppState) => state.system.anonymousMode
   );
@@ -53,56 +52,16 @@ const ProtectedRoute = ({ Component }: ProtectedRouteProps) => {
     return <Navigate to={{ pathname: `login` }} />;
   };
 
-  const pathnameParts = pathname.split("/");
-  const screen = pathnameParts.length > 2 ? pathnameParts[1] : "";
+  // update location path and store it
+  useEffect(() => {
+    dispatch(fetchSession(pathname));
+  }, [dispatch, pathname]);
 
   useEffect(() => {
-    api.session
-      .sessionCheck()
-      .then((res) => {
-        dispatch(saveSessionResponse(res.data));
-        dispatch(userLogged(true));
-        setSessionLoading(false);
-        dispatch(globalSetDistributedSetup(res.data?.distributedMode || false));
-
-        if (res.data.customStyles && res.data.customStyles !== "") {
-          const overrideColorVariants = getOverrideColorVariants(
-            res.data.customStyles
-          );
-
-          if (overrideColorVariants !== false) {
-            dispatch(setOverrideStyles(overrideColorVariants));
-          }
-        }
-      })
-      .catch(() => {
-        // if we are trying to browse, probe access to the requested prefix
-        if (screen === "browser") {
-          const bucket = pathnameParts.length >= 3 ? pathnameParts[2] : "";
-          // no bucket, no business
-          if (bucket === "") {
-            setSessionLoading(false);
-            return;
-          }
-          // before marking the session as done, let's check if the bucket is publicly accessible
-          api.buckets
-            .listObjects(
-              bucket,
-              { limit: 1 },
-              { headers: { "X-Anonymous": "1" } }
-            )
-            .then(() => {
-              dispatch(setAnonymousMode());
-              setSessionLoading(false);
-            })
-            .catch(() => {
-              setSessionLoading(false);
-            });
-        } else {
-          setSessionLoading(false);
-        }
-      });
-  }, [dispatch, screen, pathnameParts]);
+    if (sessionLoadingState === SessionCallStates.Done) {
+      setSessionLoading(false);
+    }
+  }, [dispatch, sessionLoadingState]);
 
   const [, invokeSRInfoApi] = useApi(
     (res: any) => {
@@ -142,6 +101,7 @@ const ProtectedRoute = ({ Component }: ProtectedRouteProps) => {
   if (sessionLoading) {
     return <LoadingComponent />;
   }
+
   // redirect user to the right page based on session status
   return userLoggedIn ? <Component /> : <StorePathAndRedirect />;
 };

portal-ui/src/screens/Console/CommandBar.tsx

@@ -152,9 +152,9 @@ const CommandBar = () => {
   }, []);
 
   const initialActions: Action[] = routesAsKbarActions(
-    features,
     buckets,
-    navigate
+    navigate,
+    features
   );
 
   useRegisterActions(initialActions, [buckets, features]);

portal-ui/src/screens/Console/consoleSlice.ts

@@ -15,40 +15,56 @@
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import { createSlice, PayloadAction } from "@reduxjs/toolkit";
+import { SessionResponse } from "../../api/consoleApi";
 import { AppState } from "../../store";
-import { SessionResponse } from "api/consoleApi";
+import { fetchSession } from "../../screens/LoginPage/sessionThunk";
+
+export enum SessionCallStates {
+  Idle = "idle",
+  Loading = "loading",
+  Done = "done",
+}
 
 export interface ConsoleState {
   session: SessionResponse;
+  sessionLoadingState: SessionCallStates;
 }
 
 const initialState: ConsoleState = {
-  session: {
-    status: undefined,
-    features: [],
-    distributedMode: false,
-    permissions: {},
-    allowResources: undefined,
-    customStyles: undefined,
-    envConstants: undefined,
-    serverEndPoint: "",
-  },
+  session: {},
+  sessionLoadingState: SessionCallStates.Idle,
 };
 
 export const consoleSlice = createSlice({
   name: "console",
   initialState,
   reducers: {
+    setSessionLoadingState: (
+      state,
+      action: PayloadAction<SessionCallStates>
+    ) => {
+      state.sessionLoadingState = action.payload;
+    },
     saveSessionResponse: (state, action: PayloadAction<SessionResponse>) => {
       state.session = action.payload;
     },
     resetSession: (state) => {
       state.session = initialState.session;
     },
   },
+  extraReducers: (builder) => {
+    builder
+      .addCase(fetchSession.pending, (state, action) => {
+        state.sessionLoadingState = SessionCallStates.Loading;
+      })
+      .addCase(fetchSession.fulfilled, (state, action) => {
+        state.sessionLoadingState = SessionCallStates.Done;
+      });
+  },
 });
 
-export const { saveSessionResponse, resetSession } = consoleSlice.actions;
+export const { saveSessionResponse, resetSession, setSessionLoadingState } =
+  consoleSlice.actions;
 export const selSession = (state: AppState) => state.console.session;
 export const selFeatures = (state: AppState) =>
   state.console.session ? state.console.session.features : [];

portal-ui/src/screens/Console/kbar-actions.tsx

@@ -21,9 +21,9 @@ import { IAM_PAGES } from "../../common/SecureComponent/permissions";
 import { Bucket } from "../../api/consoleApi";
 
 export const routesAsKbarActions = (
-  features: string[] | undefined,
   buckets: Bucket[],
-  navigate: (url: string) => void
+  navigate: (url: string) => void,
+  features?: string[]
 ) => {
   const initialActions: Action[] = [];
   const allowedMenuItems = validRoutes(features);

portal-ui/src/screens/LoginPage/Login.tsx

@@ -36,7 +36,8 @@ export interface LoginStrategyPayload {
 }
 
 export const getTargetPath = () => {
-  let targetPath = "/";
+  // default to browser view to avoid redirecting first to "/" and then to "/browser" on login
+  let targetPath = "/browser";
   if (
     localStorage.getItem("redirect-path") &&
     localStorage.getItem("redirect-path") !== ""

portal-ui/src/screens/LoginPage/sessionThunk.ts

@@ -0,0 +1,95 @@
+// This file is part of MinIO Console Server
+// Copyright (c) 2023 MinIO, Inc.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import { createAsyncThunk } from "@reduxjs/toolkit";
+import { setErrorSnackMessage } from "../../systemSlice";
+import { api } from "api";
+import {
+  Error,
+  HttpResponse,
+  SessionResponse,
+  ListObjectsResponse,
+} from "api/consoleApi";
+import { errorToHandler } from "api/errors";
+import {
+  saveSessionResponse,
+  setSessionLoadingState,
+  SessionCallStates,
+} from "../Console/consoleSlice";
+import {
+  globalSetDistributedSetup,
+  setOverrideStyles,
+  setAnonymousMode,
+} from "../../../src/systemSlice";
+import { getOverrideColorVariants } from "../../utils/stylesUtils";
+import { userLogged } from "../../../src/systemSlice";
+
+export const fetchSession = createAsyncThunk(
+  "session/fetchSession",
+  async (location: string, { dispatch, rejectWithValue }) => {
+    const pathnameParts = location.split("/");
+    const screen = pathnameParts.length > 2 ? pathnameParts[1] : "";
+
+    return api.session
+      .sessionCheck()
+      .then((res: HttpResponse<SessionResponse, Error>) => {
+        dispatch(userLogged(true));
+        dispatch(saveSessionResponse(res.data));
+        dispatch(globalSetDistributedSetup(res.data.distributedMode || false));
+
+        if (res.data.customStyles && res.data.customStyles !== "") {
+          const overrideColorVariants = getOverrideColorVariants(
+            res.data.customStyles
+          );
+
+          if (overrideColorVariants !== false) {
+            dispatch(setOverrideStyles(overrideColorVariants));
+          }
+        }
+      })
+      .catch(async (res: HttpResponse<SessionResponse, Error>) => {
+        if (screen === "browser") {
+          const bucket = pathnameParts.length >= 3 ? pathnameParts[2] : "";
+          // no bucket, no business
+          if (bucket === "") {
+            return;
+          }
+          // before marking the session as done, let's check if the bucket is publicly accessible (anonymous)
+          api.buckets
+            .listObjects(
+              bucket,
+              { limit: 1 },
+              { headers: { "X-Anonymous": "1" } }
+            )
+            .then(() => {
+              dispatch(setAnonymousMode());
+            })
+            .catch((res: HttpResponse<ListObjectsResponse, Error>) => {
+              dispatch(setErrorSnackMessage(errorToHandler(res.error)));
+            })
+            .finally(() => {
+              // TODO: we probably need a thunk for this api since setting the state here is hacky,
+              // we can use a state to let the ProtectedRoutes know when to render the elements
+              dispatch(setSessionLoadingState(SessionCallStates.Done));
+            });
+        } else {
+          dispatch(setSessionLoadingState(SessionCallStates.Done));
+          dispatch(setErrorSnackMessage(errorToHandler(res.error)));
+        }
+        return rejectWithValue(res.error);
+      });
+  }
+);

portal-ui/src/store.ts

@@ -23,7 +23,7 @@ import logReducer from "./screens/Console/Logs/logsSlice";
 import healthInfoReducer from "./screens/Console/HealthInfo/healthInfoSlice";
 import watchReducer from "./screens/Console/Watch/watchSlice";
 import consoleReducer from "./screens/Console/consoleSlice";
-import bucketsReducer from "./screens/Console/Buckets/ListBuckets/AddBucket/addBucketsSlice";
+import addBucketsReducer from "./screens/Console/Buckets/ListBuckets/AddBucket/addBucketsSlice";
 import bucketDetailsReducer from "./screens/Console/Buckets/BucketDetails/bucketDetailsSlice";
 import objectBrowserReducer from "./screens/Console/ObjectBrowser/objectBrowserSlice";
 import dashboardReducer from "./screens/Console/Dashboard/dashboardSlice";
@@ -39,7 +39,7 @@ const rootReducer = combineReducers({
   logs: logReducer,
   watch: watchReducer,
   console: consoleReducer,
-  addBucket: bucketsReducer,
+  addBucket: addBucketsReducer,
   bucketDetails: bucketDetailsReducer,
   objectBrowser: objectBrowserReducer,
   healthInfo: healthInfoReducer,

portal-ui/src/systemSlice.ts

@@ -29,7 +29,6 @@ export interface SystemState {
   loggedIn: boolean;
   showMarketplace: boolean;
   sidebarOpen: boolean;
-  session: string;
   userName: string;
   serverNeedsRestart: boolean;
   serverIsLoading: boolean;
@@ -51,7 +50,6 @@ const initialState: SystemState = {
   value: 0,
   loggedIn: false,
   showMarketplace: false,
-  session: "",
   userName: "",
   sidebarOpen: initSideBarOpen,
   siteReplicationInfo: { siteName: "", curSite: false, enabled: false },
@@ -155,7 +153,6 @@ export const systemSlice = createSlice({
       state.licenseInfo = action.payload;
     },
     setHelpName: (state, action: PayloadAction<string>) => {
-      console.log("setting helpName: ", action.payload);
       state.helpName = action.payload;
     },
     setHelpTabName: (state, action: PayloadAction<string>) => {