Resolve File Browser race condition (#1069)

* fixed

* fix search redirects

* resolve race condition

* remove stray import

* Apply suggestions from code review

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>

* make eslint :D

Co-authored-by: tanmoy12 <tanmoy3399@gmail.com>
Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>

Author: 3 people

packages/files-ui/src/Components/Modules/FileBrowsers/BinFileBrowser.tsx

@@ -7,7 +7,7 @@ import { t } from "@lingui/macro"
 import { CONTENT_TYPES } from "../../../Utils/Constants"
 import { IFilesTableBrowserProps } from "../../Modules/FileBrowsers/types"
 import { useHistory, useLocation, useToaster } from "@chainsafe/common-components"
-import { extractDrivePath, getPathWithFile } from "../../../Utils/pathUtils"
+import { extractFileBrowserPathFromURL, getArrayOfPaths, getPathWithFile, getURISafePathFromArray } from "../../../Utils/pathUtils"
 import { ROUTE_LINKS } from "../../FilesRoutes"
 import { FileBrowserContext } from "../../../Contexts/FileBrowserContext"
 import { useFilesApi } from "@chainsafe/common-contexts"
@@ -20,7 +20,10 @@ const BinFileBrowser: React.FC<IFileBrowserModuleProps> = ({ controls = false }:
   const [loadingCurrentPath, setLoadingCurrentPath] = useState(false)
   const [pathContents, setPathContents] = useState<FileSystemItem[]>([])
   const { pathname } = useLocation()
-  const [currentPath, setCurrentPath] = useState(extractDrivePath(pathname.split("/").slice(1).join("/")))
+  const currentPath = useMemo(() =>
+    extractFileBrowserPathFromURL(pathname, ROUTE_LINKS.Bin("")),
+  [pathname]
+  )
   const { redirect } = useHistory()
 
   const bucket = useMemo(() => buckets.find(b => b.type === "trash"), [buckets])
@@ -52,22 +55,6 @@ const BinFileBrowser: React.FC<IFileBrowserModuleProps> = ({ controls = false }:
     refreshContents(true)
   }, [bucket, refreshContents])
 
-  useEffect(() => {
-    let binPath = extractDrivePath(pathname)
-    if (binPath[0] !== "/") {
-      binPath = "/" + binPath
-    }
-    if (binPath !== currentPath) {
-      if (binPath === "/") {
-        setCurrentPath(binPath)
-      } else {
-        setCurrentPath(decodeURIComponent(binPath.slice(0, -1)))
-      }
-      refreshContents(true)
-    }
-  }, [refreshContents, pathname, currentPath])
-
-
   const deleteFile = useCallback(async (cid: string) => {
     const itemToDelete = pathContents.find((i) => i.cid === cid)
 
@@ -153,10 +140,8 @@ const BinFileBrowser: React.FC<IFileBrowserModuleProps> = ({ controls = false }:
   const viewFolder = useCallback((cid: string) => {
     const fileSystemItem = pathContents.find(f => f.cid === cid)
     if (fileSystemItem && fileSystemItem.content_type === CONTENT_TYPES.Directory) {
-      let urlSafePath
-      if (currentPath !== "/") {
-        urlSafePath = `/${currentPath.slice(1).split("/").map(encodeURIComponent).join("/")}`
-      } else {
+      let urlSafePath =  getURISafePathFromArray(getArrayOfPaths(currentPath))
+      if (urlSafePath === "/") {
         urlSafePath = ""
       }
       redirect(ROUTE_LINKS.Bin(`${urlSafePath}/${encodeURIComponent(`${fileSystemItem.name}`)}`))

packages/files-ui/src/Components/Modules/FileBrowsers/CSFFileBrowser.tsx

@@ -1,7 +1,7 @@
 import React, { useCallback, useEffect, useMemo, useState } from "react"
 import { Crumb, useToaster, useHistory, useLocation } from "@chainsafe/common-components"
 import { useFiles, FileSystemItem } from "../../../Contexts/FilesContext"
-import { extractDrivePath, getArrayOfPaths, getURISafePathFromArray, getPathWithFile } from "../../../Utils/pathUtils"
+import { getArrayOfPaths, getURISafePathFromArray, getPathWithFile, extractFileBrowserPathFromURL } from "../../../Utils/pathUtils"
 import { IBulkOperations, IFileBrowserModuleProps, IFilesTableBrowserProps } from "./types"
 import FilesList from "./views/FilesList"
 import { CONTENT_TYPES } from "../../../Utils/Constants"
@@ -29,8 +29,10 @@ const CSFFileBrowser: React.FC<IFileBrowserModuleProps> = () => {
   const { redirect } = useHistory()
 
   const { pathname } = useLocation()
-  const [currentPath, setCurrentPath] = useState(extractDrivePath(pathname.split("/").slice(1).join("/")))
-
+  const currentPath = useMemo(() =>
+    extractFileBrowserPathFromURL(pathname, ROUTE_LINKS.Drive("")),
+  [pathname]
+  )
   const bucket = useMemo(() => buckets.find(b => b.type === "csf"), [buckets])
 
   const refreshContents = useCallback((showLoading?: boolean) => {
@@ -71,22 +73,6 @@ const CSFFileBrowser: React.FC<IFileBrowserModuleProps> = () => {
     refreshContents(true)
   }, [bucket, refreshContents])
 
-  useEffect(() => {
-    let drivePath = extractDrivePath(pathname)
-    if (drivePath[0] !== "/") {
-      drivePath = "/" + drivePath
-    }
-    if (drivePath !== currentPath) {
-      if (drivePath === "/") {
-        setCurrentPath(drivePath)
-      } else {
-        setCurrentPath(decodeURIComponent(drivePath.slice(0, -1)))
-      }
-
-      refreshContents(true)
-    }
-  }, [refreshContents, pathname, currentPath])
-
   const moveItemsToBin = useCallback(async (cids: string[]) => {
     if (!bucket) return
     await Promise.all(
@@ -162,7 +148,6 @@ const CSFFileBrowser: React.FC<IFileBrowserModuleProps> = () => {
   const crumbs: Crumb[] = useMemo(() => arrayOfPaths.map((path, index) => ({
     text: decodeURIComponent(path),
     onClick: () => {
-
       redirect(
         ROUTE_LINKS.Drive(getURISafePathFromArray(arrayOfPaths.slice(0, index + 1)))
       )
@@ -191,10 +176,8 @@ const CSFFileBrowser: React.FC<IFileBrowserModuleProps> = () => {
   const viewFolder = useCallback((cid: string) => {
     const fileSystemItem = pathContents.find(f => f.cid === cid)
     if (fileSystemItem && fileSystemItem.content_type === CONTENT_TYPES.Directory) {
-      let urlSafePath
-      if (currentPath !== "/") {
-        urlSafePath = `/${currentPath.slice(1).split("/").map(encodeURIComponent).join("/")}`
-      } else {
+      let urlSafePath =  getURISafePathFromArray(getArrayOfPaths(currentPath))
+      if (urlSafePath === "/") {
         urlSafePath = ""
       }
       redirect(ROUTE_LINKS.Drive(`${urlSafePath}/${encodeURIComponent(`${fileSystemItem.name}`)}`))
@@ -221,7 +204,7 @@ const CSFFileBrowser: React.FC<IFileBrowserModuleProps> = () => {
       bucket,
       bulkOperations,
       crumbs,
-      moduleRootPath: ROUTE_LINKS.Drive(""),
+      moduleRootPath: ROUTE_LINKS.Drive("/"),
       currentPath,
       refreshContents,
       deleteItems: moveItemsToBin,

packages/files-ui/src/Components/Modules/FileBrowsers/SearchFileBrowser.tsx

@@ -15,7 +15,7 @@ const SearchFileBrowser: React.FC<IFileBrowserModuleProps> = ({ controls = false
   const { pathname } = useLocation()
   const { filesApiClient } = useFilesApi()
   const { buckets } = useFiles()
-  const [searchTerm, setSearchTerm] = useState(pathname.split("/").slice(2)[0])
+  const searchTerm = useMemo(() => decodeURIComponent(pathname.split("/").slice(2)[0]), [pathname])
   const { redirect } = useHistory()
 
   const { addToastMessage } = useToaster()
@@ -38,11 +38,10 @@ const SearchFileBrowser: React.FC<IFileBrowserModuleProps> = ({ controls = false
   }, [addToastMessage, bucket, filesApiClient])
 
   useEffect(() => {
-    const searchPath = pathname.split("/").slice(2)[0]
-
-    setSearchTerm(decodeURI(searchPath))
-    getSearchResults(decodeURI(searchPath))
-  }, [getSearchResults, pathname])
+    getSearchResults(searchTerm)
+      .then(setSearchResults)
+      .catch(console.error)
+  }, [searchTerm, getSearchResults])
 
   const [loadingSearchResults, setLoadingSearchResults] = useState(true)
   const [searchResults, setSearchResults] = useState<SearchEntry[]>([])

packages/files-ui/src/Components/Modules/SearchModule.tsx

@@ -23,7 +23,6 @@ import { getArrayOfPaths, getParentPathFromFilePath, getURISafePathFromArray } f
 import { t, Trans } from "@lingui/macro"
 import { CSFTheme } from "../../Themes/types"
 import { useFilesApi } from "@chainsafe/common-contexts"
-import { trimChar } from "../../Utils/pathUtils"
 
 export interface SearchParams {
   bucketType: BucketType
@@ -209,7 +208,7 @@ const SearchModule: React.FC<ISearchModule> = ({
   const onSearchSubmit = (e: React.FormEvent<HTMLFormElement>) => {
     e.preventDefault()
     setSearchActive(false)
-    redirect(ROUTE_LINKS.Search(searchQuery))
+    redirect(ROUTE_LINKS.Search(encodeURIComponent(searchQuery)))
   }
 
   const searchResultsFiles = searchResults?.results.filter(

packages/files-ui/src/Utils/pathUtils.ts

@@ -25,11 +25,13 @@ export function getArrayOfPaths(path: string): string[] {
 export function getURISafePathFromArray(arrayOfPaths: string[]): string {
   if (!arrayOfPaths.length) return "/"
   else {
-    return "/" + arrayOfPaths.map(encodeURIComponent).join("/")
+    return `/${arrayOfPaths.map(encodeURIComponent).join("/")}`
   }
 }
 
-// get path and file
+// Safe append path to file name
+// /path/to/somewhere + 1.txt -> /path/to/somewhere/1.txt
+// /path/to/somewhere/ + 1.txt -> /path/to/somewhere/1.txt
 export function getPathWithFile(path: string, fileName: string) {
   return path === "/"
     ? `/${fileName}`
@@ -38,13 +40,14 @@ export function getPathWithFile(path: string, fileName: string) {
       : `${path}/${fileName}`
 }
 
-// get path and file
+// /path/to/somewhere/1.txt -> /path/to/somewhere
 export function getParentPathFromFilePath(filePath: string) {
   const parentPath = filePath.substring(0, filePath.lastIndexOf("/"))
   if (!parentPath) return "/"
   return parentPath
 }
 
-export function extractDrivePath(pathname: string) {
-  return pathname.split("/").slice(2).join("/").concat("/")
-}
+// /drive/path/to/somewhere -> /path/to/somewhere
+export function extractFileBrowserPathFromURL(browserUrl: string, modulePath: string) {
+  return browserUrl.replace(modulePath, "").split("/").map(decodeURIComponent).join("/")
+}