diff --git a/fileglancer/apps/core.py b/fileglancer/apps/core.py index 9edc87201..4d9a2cde4 100644 --- a/fileglancer/apps/core.py +++ b/fileglancer/apps/core.py @@ -515,6 +515,13 @@ def _validate_parameter_value(param: AppParameter, value, session=None) -> str: if param.type in ("file", "directory"): str_val = str_val.replace("\\", "/") + # Expand ~ so the path works inside shlex.quote() single quotes, + # where the shell would not perform tilde expansion. + # Use euid so this works when the server runs as root with seteuid. + if str_val.startswith("~/") or str_val == "~": + import pwd + home = pwd.getpwuid(os.geteuid()).pw_dir + str_val = home + str_val[1:] if session is not None: error = validate_path_in_filestore(str_val, session) else: diff --git a/fileglancer/database.py b/fileglancer/database.py index f1f885923..987c44c4a 100644 --- a/fileglancer/database.py +++ b/fileglancer/database.py @@ -526,6 +526,53 @@ def _clear_sharing_key_cache(): logger.debug(f"Cleared entire sharing key cache, removed {old_size} entries") +def _find_best_fsp_match( + fsps: list[FileSharePath], + normalized_input: str, + get_candidates: callable, + separator: str = "/", +) -> Optional[tuple[FileSharePath, str]]: + """Find the FSP whose candidate path is the longest prefix of *normalized_input*. + + Used by ``find_fsp_from_absolute_path`` to check filesystem-resolved mount paths. + + Args: + fsps: All file share paths to search. + normalized_input: The input path, already normalised by the caller. + get_candidates: ``fn(fsp) -> list[str | None]`` returning the candidate + prefix strings to test for each FSP. + separator: The path separator used for the boundary check (``/`` or + ``os.sep``). + + Returns: + ``(best_fsp, subpath)`` for the longest match, or *None*. + """ + best_fsp: Optional[FileSharePath] = None + best_len = 0 + + for fsp in fsps: + for candidate in get_candidates(fsp): + if not candidate: + continue + if ( + normalized_input.startswith(candidate) + and len(candidate) > best_len + ): + rest = normalized_input[len(candidate):] + if rest == "" or rest.startswith(separator): + best_fsp = fsp + best_len = len(candidate) + + if best_fsp is None: + return None + + subpath = normalized_input[best_len:] + if subpath.startswith(separator): + subpath = subpath.lstrip(separator) + + return (best_fsp, subpath) + + def find_fsp_from_absolute_path(session: Session, absolute_path: str) -> Optional[tuple[FileSharePath, str]]: """ Find the file share path that exactly matches the given absolute path. @@ -546,27 +593,20 @@ def find_fsp_from_absolute_path(session: Session, absolute_path: str) -> Optiona # Get all file share paths paths = get_file_share_paths(session) + # Pre-compute expanded mount paths so the helper can use them + expanded_mounts: dict[str, str] = {} for fsp in paths: - # Expand ~ to user's home directory and resolve symlinks to match Filestore behavior - expanded_mount_path = os.path.expanduser(fsp.mount_path) - expanded_mount_path = os.path.realpath(expanded_mount_path) - - # Check if the normalized path starts with this mount path - if normalized_path.startswith(expanded_mount_path): - # Calculate the relative subpath - if normalized_path == expanded_mount_path: - subpath = "" - logger.trace(f"Found exact match for path: {absolute_path} in fsp: {fsp.name} with subpath: {subpath}") - return (fsp, subpath) - else: - # Ensure we're matching on a directory boundary - remainder = normalized_path[len(expanded_mount_path):] - if remainder.startswith(os.sep): - subpath = remainder.lstrip(os.sep) - logger.trace(f"Found exact match for path: {absolute_path} in fsp: {fsp.name} with subpath: {subpath}") - return (fsp, subpath) - - return None + expanded = os.path.expanduser(fsp.mount_path) + expanded_mounts[fsp.name] = os.path.realpath(expanded) + + def _expanded_mount(fsp: FileSharePath): + return [expanded_mounts[fsp.name]] + + result = _find_best_fsp_match(paths, normalized_path, _expanded_mount, separator=os.sep) + if result is not None: + fsp, subpath = result + logger.trace(f"Found exact match for path: {absolute_path} in fsp: {fsp.name} with subpath: {subpath}") + return result def _validate_proxied_path(session: Session, fsp_name: str, path: str) -> None: diff --git a/frontend/src/__tests__/componentTests/FileSelector.test.tsx b/frontend/src/__tests__/componentTests/FileSelector.test.tsx new file mode 100644 index 000000000..9a809fc75 --- /dev/null +++ b/frontend/src/__tests__/componentTests/FileSelector.test.tsx @@ -0,0 +1,271 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import userEvent from '@testing-library/user-event'; +import { http, HttpResponse } from 'msw'; + +import FileSelectorButton from '@/components/ui/FileSelector/FileSelectorButton'; +import { render, screen, waitFor } from '@/__tests__/test-utils'; +import { server } from '@/__tests__/mocks/node'; + +describe('FileSelector', () => { + const onSelect = vi.fn(); + + describe('Smoke tests', () => { + it('opens dialog when button is clicked', async () => { + const user = userEvent.setup(); + render(, { + initialEntries: ['/browse'] + }); + + await user.click(screen.getByRole('button', { name: /browse/i })); + + await waitFor(() => { + expect(screen.getByText('Select File or Folder')).toBeInTheDocument(); + }); + }); + + it('displays zones at top level', async () => { + const user = userEvent.setup(); + render(, { + initialEntries: ['/browse'] + }); + + await user.click(screen.getByRole('button', { name: /browse/i })); + + await waitFor(() => { + expect(screen.getByText('Zone1')).toBeInTheDocument(); + }); + expect(screen.getByText('Zone2')).toBeInTheDocument(); + }); + + it('closes dialog when Cancel is clicked', async () => { + const user = userEvent.setup(); + render(, { + initialEntries: ['/browse'] + }); + + await user.click(screen.getByRole('button', { name: /browse/i })); + + await waitFor(() => { + expect(screen.getByText('Select File or Folder')).toBeInTheDocument(); + }); + + await user.click(screen.getByRole('button', { name: /cancel/i })); + + await waitFor(() => { + expect( + screen.queryByText('Select File or Folder') + ).not.toBeInTheDocument(); + }); + }); + }); + + describe('Search functionality', () => { + beforeEach(async () => { + const user = userEvent.setup(); + render(, { + initialEntries: ['/browse'] + }); + + await user.click(screen.getByRole('button', { name: /browse/i })); + + await waitFor(() => { + expect(screen.getByText('Zone1')).toBeInTheDocument(); + }); + }); + + it('shows search input at zones level', () => { + expect(screen.getByPlaceholderText('Type to filter')).toBeInTheDocument(); + }); + + it('filters displayed zones by name when typing in search', async () => { + const user = userEvent.setup(); + + const searchInput = screen.getByPlaceholderText('Type to filter'); + await user.click(searchInput); + await user.keyboard('1'); + + expect(screen.getByText('Zone1')).toBeInTheDocument(); + expect(screen.queryByText('Zone2')).not.toBeInTheDocument(); + }); + + it('restores full list when search is cleared', async () => { + const user = userEvent.setup(); + + const searchInput = screen.getByPlaceholderText('Type to filter'); + await user.click(searchInput); + await user.keyboard('1'); + + expect(screen.queryByText('Zone2')).not.toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: /clear search/i })); + + await waitFor(() => { + expect(screen.getByText('Zone1')).toBeInTheDocument(); + expect(screen.getByText('Zone2')).toBeInTheDocument(); + }); + }); + + it('clears search when navigating into a zone', async () => { + const user = userEvent.setup(); + + const searchInput = screen.getByPlaceholderText('Type to filter'); + await user.click(searchInput); + await user.keyboard('Zone'); + + expect(searchInput).toHaveValue('Zone'); + + // Double-click Zone1 to navigate into it + await user.dblClick(screen.getByText('Zone1')); + + await waitFor(() => { + const input = screen.getByPlaceholderText('Type to filter'); + expect(input).toHaveValue(''); + }); + }); + }); + + describe('FSP-level search', () => { + it('filters FSPs by name when typing in search at zone level', async () => { + // Add a second FSP in Zone1 so we can test filtering + server.use( + http.get('/api/file-share-paths', () => { + return HttpResponse.json({ + paths: [ + { + name: 'test_fsp', + zone: 'Zone1', + group: 'group1', + storage: 'primary', + mount_path: '/test/fsp', + mac_path: 'smb://test/fsp', + windows_path: '\\\\test\\fsp', + linux_path: '/test/fsp' + }, + { + name: 'alpha_fsp', + zone: 'Zone1', + group: 'group1', + storage: 'primary', + mount_path: '/alpha/fsp', + mac_path: 'smb://alpha/fsp', + windows_path: '\\\\alpha\\fsp', + linux_path: '/alpha/fsp' + } + ] + }); + }) + ); + + const user = userEvent.setup(); + render(, { + initialEntries: ['/browse'] + }); + + await user.click(screen.getByRole('button', { name: /browse/i })); + + await waitFor(() => { + expect(screen.getByText('Zone1')).toBeInTheDocument(); + }); + + // Navigate into Zone1 + await user.dblClick(screen.getByText('Zone1')); + + await waitFor(() => { + expect(screen.getByText('/test/fsp')).toBeInTheDocument(); + expect(screen.getByText('/alpha/fsp')).toBeInTheDocument(); + }); + + // Filter by typing + const searchInput = screen.getByPlaceholderText('Type to filter'); + await user.click(searchInput); + await user.keyboard('alpha'); + + expect(screen.getByText('/alpha/fsp')).toBeInTheDocument(); + expect(screen.queryByText('/test/fsp')).not.toBeInTheDocument(); + }); + }); + + describe('Group filter status', () => { + it('hides status message when user has no groups', async () => { + const user = userEvent.setup(); + render(, { + initialEntries: ['/browse'] + }); + + await user.click(screen.getByRole('button', { name: /browse/i })); + + await waitFor(() => { + expect(screen.getByText('Zone1')).toBeInTheDocument(); + }); + + expect( + screen.queryByText('Viewing zones for your groups only') + ).not.toBeInTheDocument(); + expect(screen.queryByText('Viewing all zones')).not.toBeInTheDocument(); + }); + + it('shows "Viewing zones for your groups only" when user has groups and isFilteredByGroups is true', async () => { + server.use( + http.get('/api/profile', () => { + return HttpResponse.json({ + username: 'testuser', + groups: ['group1'] + }); + }) + ); + + const user = userEvent.setup(); + render(, { + initialEntries: ['/browse'] + }); + + await user.click(screen.getByRole('button', { name: /browse/i })); + + await waitFor(() => { + expect( + screen.getByText('Viewing zones for your groups only') + ).toBeInTheDocument(); + }); + }); + + it('hides status message at filesystem level', async () => { + server.use( + http.get('/api/profile', () => { + return HttpResponse.json({ + username: 'testuser', + groups: ['group1'] + }); + }) + ); + + const user = userEvent.setup(); + render(, { + initialEntries: ['/browse'] + }); + + await user.click(screen.getByRole('button', { name: /browse/i })); + + await waitFor(() => { + expect(screen.getByText('Zone1')).toBeInTheDocument(); + }); + + // Navigate into Zone1 + await user.dblClick(screen.getByText('Zone1')); + + // At zone level, FSPs are displayed using their preferred path format + await waitFor(() => { + expect(screen.getByText('/test/fsp')).toBeInTheDocument(); + }); + + // Navigate into FSP + await user.dblClick(screen.getByText('/test/fsp')); + + await waitFor(() => { + expect( + screen.queryByText('Viewing zones for your groups only') + ).not.toBeInTheDocument(); + expect(screen.queryByText('Viewing all zones')).not.toBeInTheDocument(); + }); + }); + }); +}); diff --git a/frontend/src/__tests__/unitTests/pathHandling.test.ts b/frontend/src/__tests__/unitTests/pathHandling.test.ts index 11e33afdf..a2fff14fb 100644 --- a/frontend/src/__tests__/unitTests/pathHandling.test.ts +++ b/frontend/src/__tests__/unitTests/pathHandling.test.ts @@ -12,7 +12,8 @@ import { convertBackToForwardSlash, escapePathForUrl, normalizePosixStylePath, - removeTrailingSlashes + removeTrailingSlashes, + resolvePathToFsp } from '@/utils/pathHandling'; import type { FileSharePath } from '@/shared.types'; @@ -245,6 +246,151 @@ describe('removeLastSegmentFromPath', () => { }); }); +describe('resolvePathToFsp', () => { + const fspA = { + zone: 'Zone1', + name: 'fsp_a', + group: 'group1', + storage: 'primary', + mount_path: '/mount/a', + linux_path: '/linux/a', + mac_path: 'smb://mac/a', + windows_path: '\\\\win\\a' + } as FileSharePath; + + const fspB = { + zone: 'Zone1', + name: 'fsp_b', + group: 'group1', + storage: 'primary', + mount_path: '/mount/b', + linux_path: '/linux/b', + mac_path: 'smb://mac/b', + windows_path: '\\\\win\\b' + } as FileSharePath; + + const zonesAndFspData: Record = { + zone_Zone1: { name: 'Zone1', fileSharePaths: [fspA, fspB] }, + fsp_fsp_a: fspA, + fsp_fsp_b: fspB + }; + + test('matches a Linux-style path', () => { + const result = resolvePathToFsp('/linux/a/sub/folder', zonesAndFspData); + expect(result).not.toBeNull(); + expect(result!.fsp.name).toBe('fsp_a'); + expect(result!.subpath).toBe('sub/folder'); + }); + + test('matches a Mac-style path', () => { + const result = resolvePathToFsp('smb://mac/b/deep/path', zonesAndFspData); + expect(result).not.toBeNull(); + expect(result!.fsp.name).toBe('fsp_b'); + expect(result!.subpath).toBe('deep/path'); + }); + + test('matches a Windows-style path with backslashes', () => { + const result = resolvePathToFsp('\\\\win\\a\\sub\\folder', zonesAndFspData); + expect(result).not.toBeNull(); + expect(result!.fsp.name).toBe('fsp_a'); + expect(result!.subpath).toBe('sub/folder'); + }); + + test('matches a mount_path', () => { + const result = resolvePathToFsp('/mount/b/file.txt', zonesAndFspData); + expect(result).not.toBeNull(); + expect(result!.fsp.name).toBe('fsp_b'); + expect(result!.subpath).toBe('file.txt'); + }); + + test('returns null when no FSP matches', () => { + const result = resolvePathToFsp('/unknown/path', zonesAndFspData); + expect(result).toBeNull(); + }); + + test('returns empty subpath when path matches exactly', () => { + const result = resolvePathToFsp('/linux/a', zonesAndFspData); + expect(result).not.toBeNull(); + expect(result!.fsp.name).toBe('fsp_a'); + expect(result!.subpath).toBe(''); + }); + + test('strips leading slash from subpath', () => { + const result = resolvePathToFsp('/linux/a/child', zonesAndFspData); + expect(result).not.toBeNull(); + expect(result!.subpath).toBe('child'); + }); + + test('trims whitespace from input', () => { + const result = resolvePathToFsp(' /linux/a/child ', zonesAndFspData); + expect(result).not.toBeNull(); + expect(result!.fsp.name).toBe('fsp_a'); + expect(result!.subpath).toBe('child'); + }); + + test('picks the longest (most specific) match', () => { + const fspShort = { + zone: 'Zone1', + name: 'fsp_short', + group: 'g', + storage: 'primary', + mount_path: '/data', + linux_path: '/data', + mac_path: null, + windows_path: null + } as FileSharePath; + + const fspLong = { + zone: 'Zone1', + name: 'fsp_long', + group: 'g', + storage: 'primary', + mount_path: '/data/science', + linux_path: '/data/science', + mac_path: null, + windows_path: null + } as FileSharePath; + + const data: Record = { + fsp_fsp_short: fspShort, + fsp_fsp_long: fspLong + }; + + const result = resolvePathToFsp('/data/science/images', data); + expect(result).not.toBeNull(); + expect(result!.fsp.name).toBe('fsp_long'); + expect(result!.subpath).toBe('images'); + }); + + test('does not match partial path segments', () => { + // '/linux/abc' should not match fsp_a whose linux_path is '/linux/a' + const result = resolvePathToFsp('/linux/abc', zonesAndFspData); + expect(result).toBeNull(); + }); + + test('handles FSPs with null path fields', () => { + const fspNulls = { + zone: 'Zone1', + name: 'fsp_nulls', + group: 'g', + storage: 'primary', + mount_path: '/only/mount', + linux_path: null, + mac_path: null, + windows_path: null + } as FileSharePath; + + const data: Record = { + fsp_fsp_nulls: fspNulls + }; + + const result = resolvePathToFsp('/only/mount/sub', data); + expect(result).not.toBeNull(); + expect(result!.fsp.name).toBe('fsp_nulls'); + expect(result!.subpath).toBe('sub'); + }); +}); + describe('getPreferredPathForDisplay', () => { const mockFsp = { zone: 'test_zone', diff --git a/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx b/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx index ec945e53c..cba8a4553 100644 --- a/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx +++ b/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx @@ -9,12 +9,16 @@ import { HiOutlineTrash } from 'react-icons/hi'; -import FileSelectorButton from '@/components/ui/BrowsePage/FileSelector/FileSelectorButton'; +import FileSelectorButton from '@/components/ui/FileSelector/FileSelectorButton'; import FgSwitch from '@/components/ui/widgets/FgSwitch'; import { usePreferencesContext } from '@/contexts/PreferencesContext'; +import { useZoneAndFspMapContext } from '@/contexts/ZonesAndFspMapContext'; import { validatePaths } from '@/queries/appsQueries'; import { useClusterDefaultsQuery } from '@/queries/jobsQueries'; -import { convertBackToForwardSlash } from '@/utils/pathHandling'; +import { + convertBackToForwardSlash, + resolvePathToFsp +} from '@/utils/pathHandling'; import { flattenParameters, isParameterSection } from '@/shared.types'; import type { AppEntryPoint, @@ -61,6 +65,13 @@ function ParameterField({ readonly value: unknown; readonly onChange: (value: unknown) => void; }) { + // For file/directory fields, track a display-formatted path separately from + // the server-formatted value used for submission. When the user selects a + // path via the file selector, the display path uses their OS preference + // (e.g. Windows backslashes) while the stored value stays in server format. + // Manual edits clear the override so the input shows exactly what was typed. + const [fileDisplayPath, setFileDisplayPath] = useState(null); + const baseInputClass = 'w-full p-2 text-foreground border rounded-sm focus:outline-none bg-background border-primary-light focus:border-primary'; @@ -126,10 +137,16 @@ function ParameterField({
onChange(e.target.value)} + onChange={e => { + setFileDisplayPath(null); + onChange(e.target.value); + }} placeholder={`Select a ${param.type}...`} type="text" - value={value !== undefined && value !== null ? String(value) : ''} + value={ + fileDisplayPath ?? + (value !== undefined && value !== null ? String(value) : '') + } /> onChange(path)} + onSelect={(serverPath, displayPath) => { + onChange(serverPath); + setFileDisplayPath(displayPath); + }} useServerPath />
@@ -672,6 +692,7 @@ export default function AppLaunchForm({ initialContainerArgs }: AppLaunchFormProps) { const { defaultExtraArgs } = usePreferencesContext(); + const { zonesAndFspQuery } = useZoneAndFspMapContext(); const clusterDefaultsQuery = useClusterDefaultsQuery(); const allParams = flattenParameters([ ...entryPoint.parameters, @@ -760,6 +781,24 @@ export default function AppLaunchForm({ 'submitOptions' ]); + /** + * Resolve a path in any OS format (Mac smb://, Windows UNC, Linux) to + * the server's mount_path + subpath. Returns the original value if FSP + * data isn't loaded or no match is found. + */ + const resolveToServerPath = (val: string): string => { + const fspData = zonesAndFspQuery.data; + if (!fspData) { + return val; + } + const result = resolvePathToFsp(val, fspData); + if (!result) { + return val; + } + const { fsp, subpath } = result; + return subpath ? `${fsp.mount_path}/${subpath}` : fsp.mount_path; + }; + const handleChange = (paramId: string, value: unknown) => { setValues(prev => ({ ...prev, [paramId]: value })); // Clear error on change @@ -814,7 +853,9 @@ export default function AppLaunchForm({ (param.type === 'file' || param.type === 'directory') && typeof val === 'string' ) { - const normalized = convertBackToForwardSlash(val); + // Resolve Mac/Windows/alternate-Linux paths to server mount_path + const resolved = resolveToServerPath(val); + const normalized = convertBackToForwardSlash(resolved); if ( !normalized.startsWith('s3://') && !normalized.startsWith('gs://') && @@ -881,7 +922,9 @@ export default function AppLaunchForm({ (paramDef.type === 'file' || paramDef.type === 'directory') && typeof val === 'string' ) { - const normalized = convertBackToForwardSlash(val); + // Resolve Mac/Windows/alternate-Linux paths to server mount_path + const resolved = resolveToServerPath(val); + const normalized = convertBackToForwardSlash(resolved); params[key] = normalized; // Skip server-side path validation for URI schemes (e.g. s3://) if ( diff --git a/frontend/src/components/ui/BrowsePage/Crumbs.tsx b/frontend/src/components/ui/BrowsePage/Crumbs.tsx index bfc53c58f..cf16fac3b 100644 --- a/frontend/src/components/ui/BrowsePage/Crumbs.tsx +++ b/frontend/src/components/ui/BrowsePage/Crumbs.tsx @@ -3,7 +3,7 @@ import toast from 'react-hot-toast'; import { HiChevronRight, HiOutlineDuplicate } from 'react-icons/hi'; import { HiOutlineSquares2X2 } from 'react-icons/hi2'; -import BreadcrumbSegment from './BreadcrumbSegment'; +import BreadcrumbSegment from '@/components/ui/widgets/BreadcrumbSegment'; import { useFileBrowserContext } from '@/contexts/FileBrowserContext'; import { usePreferencesContext } from '@/contexts/PreferencesContext'; import { useZoneAndFspMapContext } from '@/contexts/ZonesAndFspMapContext'; diff --git a/frontend/src/components/ui/Dialogs/ConvertFile.tsx b/frontend/src/components/ui/Dialogs/ConvertFile.tsx index f00fa1622..68de2feed 100644 --- a/frontend/src/components/ui/Dialogs/ConvertFile.tsx +++ b/frontend/src/components/ui/Dialogs/ConvertFile.tsx @@ -10,7 +10,7 @@ import { usePreferencesContext } from '@/contexts/PreferencesContext'; import { useFileBrowserContext } from '@/contexts/FileBrowserContext'; import { useTicketContext } from '@/contexts/TicketsContext'; import { getPreferredPathForDisplay } from '@/utils/pathHandling'; -import FileSelectorButton from '@/components/ui/BrowsePage/FileSelector/FileSelectorButton'; +import FileSelectorButton from '@/components/ui/FileSelector/FileSelectorButton'; type ItemNamingDialogProps = { readonly showConvertFileDialog: boolean; diff --git a/frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorBreadcrumbs.tsx b/frontend/src/components/ui/FileSelector/FileSelectorBreadcrumbs.tsx similarity index 97% rename from frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorBreadcrumbs.tsx rename to frontend/src/components/ui/FileSelector/FileSelectorBreadcrumbs.tsx index 32fb24015..93d58defb 100644 --- a/frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorBreadcrumbs.tsx +++ b/frontend/src/components/ui/FileSelector/FileSelectorBreadcrumbs.tsx @@ -1,7 +1,7 @@ import { Breadcrumb } from '@material-tailwind/react'; import { HiChevronRight, HiOutlineSquares2X2 } from 'react-icons/hi2'; -import BreadcrumbSegment from '@/components/ui/BrowsePage/BreadcrumbSegment'; +import BreadcrumbSegment from '@/components/ui/widgets/BreadcrumbSegment'; import { usePreferencesContext } from '@/contexts/PreferencesContext'; import { makePathSegmentArray, joinPaths } from '@/utils/pathHandling'; import { makeMapKey, getPreferredPathForDisplay } from '@/utils'; diff --git a/frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorButton.tsx b/frontend/src/components/ui/FileSelector/FileSelectorButton.tsx similarity index 74% rename from frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorButton.tsx rename to frontend/src/components/ui/FileSelector/FileSelectorButton.tsx index 6ac84033a..9963bc565 100644 --- a/frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorButton.tsx +++ b/frontend/src/components/ui/FileSelector/FileSelectorButton.tsx @@ -1,7 +1,7 @@ import { useState, useEffect } from 'react'; import type { MouseEvent } from 'react'; -import { Button, Typography } from '@material-tailwind/react'; -import { HiOutlineFolder } from 'react-icons/hi2'; +import { Button, Input, Typography } from '@material-tailwind/react'; +import { HiOutlineFolder, HiOutlineFunnel, HiXMark } from 'react-icons/hi2'; import FgDialog from '@/components/ui/Dialogs/FgDialog'; import FileSelectorBreadcrumbs from './FileSelectorBreadcrumbs'; @@ -17,14 +17,14 @@ import type { let lastSelectedParentPath: string | null = null; function getParentPath(fullPath: string): string { - // Strip trailing slash, then take everything up to the last slash - const trimmed = fullPath.endsWith('/') ? fullPath.slice(0, -1) : fullPath; - const lastSlash = trimmed.lastIndexOf('/'); - return lastSlash > 0 ? trimmed.slice(0, lastSlash) : trimmed; + // Strip trailing slash, then take everything up to the last separator + const trimmed = fullPath.replace(/[\\/]+$/, ''); + const lastSep = Math.max(trimmed.lastIndexOf('/'), trimmed.lastIndexOf('\\')); + return lastSep > 0 ? trimmed.slice(0, lastSep) : trimmed; } type FileSelectorButtonProps = { - readonly onSelect: (path: string) => void; + readonly onSelect: (path: string, displayPath: string) => void; readonly triggerClasses?: string; readonly label?: string; readonly initialLocation?: FileSelectorInitialLocation; @@ -56,7 +56,12 @@ export default function FileSelectorButton({ navigateToLocation, selectItem, handleItemDoubleClick, - reset + reset, + searchQuery, + handleSearchChange, + clearSearch, + isFilteredByGroups, + userHasGroups } = useFileSelector({ initialLocation, initialPath: showDialog ? effectiveInitialPath : undefined, @@ -78,8 +83,8 @@ export default function FileSelectorButton({ const handleSelect = () => { if (state.selectedItem) { - lastSelectedParentPath = getParentPath(state.selectedItem.fullPath); - onSelect(state.selectedItem.fullPath); + lastSelectedParentPath = getParentPath(state.selectedItem.displayPath); + onSelect(state.selectedItem.fullPath, state.selectedItem.displayPath); onClose(); } }; @@ -135,6 +140,31 @@ export default function FileSelectorButton({ zonesData={zonesQuery.data} /> + {/* Search input */} +
+ + + + + + {searchQuery ? ( + + ) : null} +
+ {/* Table with loading/error states */}
{zonesQuery.isPending ? ( @@ -177,6 +207,17 @@ export default function FileSelectorButton({ )}
+ {/* Group filter status */} + {state.currentLocation.type === 'zones' && userHasGroups ? ( +
+ + {isFilteredByGroups + ? 'Viewing zones for your groups only' + : 'Viewing all zones'} + +
+ ) : null} + {/* Selected path display */}
@@ -185,7 +226,7 @@ export default function FileSelectorButton({ {state.selectedItem ? ( - {state.selectedItem.fullPath} + {state.selectedItem.displayPath} ) : (
diff --git a/frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorTable.tsx b/frontend/src/components/ui/FileSelector/FileSelectorTable.tsx similarity index 94% rename from frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorTable.tsx rename to frontend/src/components/ui/FileSelector/FileSelectorTable.tsx index c7ca972d5..a97ff98f6 100644 --- a/frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorTable.tsx +++ b/frontend/src/components/ui/FileSelector/FileSelectorTable.tsx @@ -9,7 +9,7 @@ import { } from '@tanstack/react-table'; import { Typography } from '@material-tailwind/react'; import { - HiOutlineFolder, + HiFolder, HiOutlineSquares2X2, HiOutlineRectangleStack } from 'react-icons/hi2'; @@ -34,6 +34,7 @@ type FileSelectorTableProps = { name: string; isDir: boolean; fullPath: string; + displayPath: string; } | null; readonly zonesData: Record | undefined; readonly onItemClick: (item: FileOrFolder) => void; @@ -95,7 +96,7 @@ export default function FileSelectorTable({ } else { // At filesystem level: show folder or file icon icon = file.is_dir ? ( - + ) : ( ); @@ -182,14 +183,14 @@ export default function FileSelectorTable({ const isSelected = selectedItem?.name === row.original.name; return ( onItemClick(row.original)} onDoubleClick={() => onItemDoubleClick(row.original)} > {row.getVisibleCells().map(cell => ( diff --git a/frontend/src/components/ui/BrowsePage/BreadcrumbSegment.tsx b/frontend/src/components/ui/widgets/BreadcrumbSegment.tsx similarity index 100% rename from frontend/src/components/ui/BrowsePage/BreadcrumbSegment.tsx rename to frontend/src/components/ui/widgets/BreadcrumbSegment.tsx diff --git a/frontend/src/hooks/useFileSelector.ts b/frontend/src/hooks/useFileSelector.ts index b79867e95..b95a2a4e4 100644 --- a/frontend/src/hooks/useFileSelector.ts +++ b/frontend/src/hooks/useFileSelector.ts @@ -1,10 +1,14 @@ import { useState, useCallback, useEffect, useMemo, useRef } from 'react'; +import type { ChangeEvent } from 'react'; import { useZoneAndFspMapContext } from '@/contexts/ZonesAndFspMapContext'; import { usePreferencesContext } from '@/contexts/PreferencesContext'; import { useProfileContext } from '@/contexts/ProfileContext'; import useFileQuery from '@/queries/fileQueries'; -import { getPreferredPathForDisplay } from '@/utils/pathHandling'; +import { + getPreferredPathForDisplay, + resolvePathToFsp +} from '@/utils/pathHandling'; import { makeMapKey } from '@/utils'; import { filterFspsByGroupMembership } from '@/utils/groupFiltering'; import type { FileOrFolder, FileSharePath, Zone } from '@/shared.types'; @@ -19,7 +23,8 @@ type FileSelectorState = { selectedItem: { name: string; isDir: boolean; - fullPath: string; // Full filesystem path in preferred format + fullPath: string; // Path in effective format (may be overridden for server use) + displayPath: string; // Path in user's preferred format for display } | null; }; @@ -62,6 +67,22 @@ export default function useFileSelector(options?: FileSelectorOptions) { selectedItem: null }); + const [searchQuery, setSearchQuery] = useState(''); + const normalizedQuery = searchQuery.trim().toLowerCase(); + + const handleSearchChange = useCallback( + (event: ChangeEvent) => { + setSearchQuery(event.target.value); + }, + [] + ); + + const clearSearch = useCallback(() => { + setSearchQuery(''); + }, []); + + const userHasGroups = (profile?.groups?.length ?? 0) > 0; + // Resolve initialPath (raw filesystem path) to FSP + relative path const lastResolvedPath = useRef(undefined); useEffect(() => { @@ -75,35 +96,10 @@ export default function useFileSelector(options?: FileSelectorOptions) { } lastResolvedPath.current = initialPath; - // Find the FSP whose mount_path is the longest prefix of initialPath - let bestFsp: FileSharePath | null = null; - let bestMountPath = ''; - for (const [key, value] of Object.entries(zonesAndFspQuery.data)) { - if (!key.startsWith('fsp_')) { - continue; - } - const fsp = value as FileSharePath; - const mountPath = fsp.mount_path; - if ( - mountPath && - initialPath.startsWith(mountPath) && - mountPath.length > bestMountPath.length - ) { - // Ensure it's a proper prefix (matches at a path boundary) - const rest = initialPath.slice(mountPath.length); - if (rest === '' || rest.startsWith('/')) { - bestFsp = fsp; - bestMountPath = mountPath; - } - } - } + const resolved = resolvePathToFsp(initialPath, zonesAndFspQuery.data); - if (bestFsp) { - let subPath = initialPath.slice(bestMountPath.length); - // Remove leading slash from subpath - if (subPath.startsWith('/')) { - subPath = subPath.slice(1); - } + if (resolved) { + let subPath = resolved.subpath; // For file mode, navigate to the parent directory if (subPath && mode !== 'directory') { const lastSlash = subPath.lastIndexOf('/'); @@ -117,7 +113,7 @@ export default function useFileSelector(options?: FileSelectorOptions) { setState({ currentLocation: { type: 'filesystem', - fspName: bestFsp.name, + fspName: resolved.fsp.name, path: subPath || '.' }, selectedItem: null @@ -185,6 +181,13 @@ export default function useFileSelector(options?: FileSelectorOptions) { } }); + // Filter zones by search query + if (normalizedQuery) { + return items.filter(item => + item.name.toLowerCase().includes(normalizedQuery) + ); + } + return items; } else if (state.currentLocation.type === 'zone') { // Show FSPs in the selected zone @@ -209,8 +212,15 @@ export default function useFileSelector(options?: FileSelectorOptions) { isFilteredByGroups ); + // Filter FSPs by search query + const searchFilteredFsps = normalizedQuery + ? accessibleFsps.filter(fsp => + fsp.name.toLowerCase().includes(normalizedQuery) + ) + : accessibleFsps; + // Convert to FileOrFolder items to display in file selector table - const items: FileOrFolder[] = accessibleFsps.map(fsp => ({ + const items: FileOrFolder[] = searchFilteredFsps.map(fsp => ({ name: fsp.name, path: fsp.name, is_dir: true, @@ -224,7 +234,13 @@ export default function useFileSelector(options?: FileSelectorOptions) { return items; } else { // In filesystem mode, return files from query - return fileQuery.data?.files || []; + const files = fileQuery.data?.files || []; + if (normalizedQuery) { + return files.filter(item => + item.name.toLowerCase().includes(normalizedQuery) + ); + } + return files; } }, [ state.currentLocation, @@ -232,20 +248,23 @@ export default function useFileSelector(options?: FileSelectorOptions) { zonesAndFspQuery.isPending, fileQuery.data, isFilteredByGroups, - profile + profile, + normalizedQuery ]); // Navigation methods - const navigateToLocation = (location: FileSelectorLocation) => { + const navigateToLocation = useCallback((location: FileSelectorLocation) => { + setSearchQuery(''); setState({ currentLocation: location, selectedItem: null }); - }; + }, []); // Reset to initial state (for when dialog is closed/cancelled) const reset = useCallback(() => { lastResolvedPath.current = undefined; + setSearchQuery(''); setState({ currentLocation: initialLocation ? { @@ -263,6 +282,7 @@ export default function useFileSelector(options?: FileSelectorOptions) { const selectItem = useCallback( (item?: FileOrFolder) => { let fullPath = ''; + let displayPath = ''; let name = ''; let isDir = true; @@ -273,10 +293,17 @@ export default function useFileSelector(options?: FileSelectorOptions) { return; } + const subPath = + state.currentLocation.path === '.' ? '' : state.currentLocation.path; fullPath = getPreferredPathForDisplay( effectivePathPreference, currentFsp, - state.currentLocation.path === '.' ? '' : state.currentLocation.path + subPath + ); + displayPath = getPreferredPathForDisplay( + pathPreference, + currentFsp, + subPath ); // Get the folder name from the path @@ -303,6 +330,7 @@ export default function useFileSelector(options?: FileSelectorOptions) { const fsp = zonesAndFspQuery.data?.[fspKey] as FileSharePath; if (fsp) { fullPath = getPreferredPathForDisplay(effectivePathPreference, fsp); + displayPath = getPreferredPathForDisplay(pathPreference, fsp); } } else if (currentFsp) { // In filesystem mode, generate path from current FSP + item path @@ -311,6 +339,11 @@ export default function useFileSelector(options?: FileSelectorOptions) { currentFsp, item.path ); + displayPath = getPreferredPathForDisplay( + pathPreference, + currentFsp, + item.path + ); } name = item.name; @@ -324,7 +357,8 @@ export default function useFileSelector(options?: FileSelectorOptions) { selectedItem: { name, isDir, - fullPath + fullPath, + displayPath } })); } @@ -333,6 +367,7 @@ export default function useFileSelector(options?: FileSelectorOptions) { state.currentLocation, currentFsp, effectivePathPreference, + pathPreference, mode, zonesAndFspQuery.data ] @@ -342,39 +377,26 @@ export default function useFileSelector(options?: FileSelectorOptions) { const handleItemDoubleClick = useCallback( (item: FileOrFolder) => { if (!item.is_dir) { - // Can't navigate into files return; } if (state.currentLocation.type === 'zones') { - // Navigate to zone - setState({ - currentLocation: { type: 'zone', zoneId: item.name }, - selectedItem: null - }); + navigateToLocation({ type: 'zone', zoneId: item.name }); } else if (state.currentLocation.type === 'zone') { - // Navigate to FSP - setState({ - currentLocation: { - type: 'filesystem', - fspName: item.name, - path: '.' - }, - selectedItem: null + navigateToLocation({ + type: 'filesystem', + fspName: item.name, + path: '.' }); } else if (state.currentLocation.type === 'filesystem') { - // Navigate to folder - setState({ - currentLocation: { - type: 'filesystem', - fspName: state.currentLocation.fspName, - path: item.path - }, - selectedItem: null + navigateToLocation({ + type: 'filesystem', + fspName: state.currentLocation.fspName, + path: item.path }); } }, - [state.currentLocation] + [state.currentLocation, navigateToLocation] ); return { @@ -385,6 +407,11 @@ export default function useFileSelector(options?: FileSelectorOptions) { navigateToLocation, selectItem, handleItemDoubleClick, - reset + reset, + searchQuery, + handleSearchChange, + clearSearch, + isFilteredByGroups, + userHasGroups }; } diff --git a/frontend/src/hooks/useNavigationInput.ts b/frontend/src/hooks/useNavigationInput.ts index ff664c090..eea2d13ff 100644 --- a/frontend/src/hooks/useNavigationInput.ts +++ b/frontend/src/hooks/useNavigationInput.ts @@ -3,11 +3,8 @@ import type { ChangeEvent } from 'react'; import { useNavigate } from 'react-router'; import { useZoneAndFspMapContext } from '@/contexts/ZonesAndFspMapContext'; -import { FileSharePath, Result } from '@/shared.types'; -import { - convertBackToForwardSlash, - makeBrowseLink -} from '@/utils/pathHandling'; +import type { Result } from '@/shared.types'; +import { makeBrowseLink, resolvePathToFsp } from '@/utils/pathHandling'; import { createSuccess, handleError } from '@/utils/errorHandling'; export default function useNavigationInput(initialValue: string = '') { @@ -41,64 +38,11 @@ export default function useNavigationInput(initialValue: string = '') { } try { - // Trim white space and, if necessary, convert backslashes to forward slashes - const normalizedInput = convertBackToForwardSlash(inputValue.trim()); + const resolved = resolvePathToFsp(inputValue, zonesAndFspQuery.data); - // Track best match - let bestMatch: { - fspObject: FileSharePath; - matchedPath: string; - subpath: string; - } | null = null; - - const keys = Object.keys(zonesAndFspQuery.data); - for (const key of keys) { - // Iterate through only the objects in zonesAndFileSharePathsMap that have a key that start with "fsp_" - if (key.startsWith('fsp_')) { - const fspObject = zonesAndFspQuery.data[key] as FileSharePath; - const linuxPath = fspObject.linux_path || ''; - const macPath = fspObject.mac_path || ''; - const windowsPath = convertBackToForwardSlash(fspObject.windows_path); - - let matchedPath: string | null = null; - let subpath = ''; - // Check if the normalized input starts with any of the mount paths - // If a match is found, extract the subpath - // Collect all potential matches - if (normalizedInput.startsWith(linuxPath)) { - matchedPath = linuxPath; - subpath = normalizedInput.replace(linuxPath, ''); - } else if (normalizedInput.startsWith(macPath)) { - matchedPath = macPath; - subpath = normalizedInput.replace(macPath, ''); - } else if (normalizedInput.startsWith(windowsPath)) { - matchedPath = windowsPath; - subpath = normalizedInput.replace(windowsPath, ''); - } - - if (matchedPath) { - // The best match is the one with the longest matched path (most specific) - if ( - !bestMatch || - matchedPath.length > bestMatch.matchedPath.length - ) { - bestMatch = { - fspObject, - matchedPath, - subpath - }; - } - } - } - } - - if (bestMatch) { - const browseLink = makeBrowseLink( - bestMatch.fspObject.name, - bestMatch.subpath - ); + if (resolved) { + const browseLink = makeBrowseLink(resolved.fsp.name, resolved.subpath); navigate(browseLink); - // Clear the inputValue setInputValue(''); return createSuccess(undefined); } else { diff --git a/frontend/src/utils/groupFiltering.ts b/frontend/src/utils/groupFiltering.ts index 95c2e387d..b03b70683 100644 --- a/frontend/src/utils/groupFiltering.ts +++ b/frontend/src/utils/groupFiltering.ts @@ -9,7 +9,11 @@ import type { FileSharePath } from '@/shared.types'; * @returns true if the user has access, false otherwise */ function shouldDisplayFsp(fsp: FileSharePath, userGroups: string[]): boolean { - return userGroups.includes(fsp.group) || fsp.group === 'public'; + return ( + userGroups.includes(fsp.group) || + fsp.group === 'public' || + fsp.group === 'local' + ); } /** diff --git a/frontend/src/utils/pathHandling.ts b/frontend/src/utils/pathHandling.ts index 3891bce23..710c66af9 100644 --- a/frontend/src/utils/pathHandling.ts +++ b/frontend/src/utils/pathHandling.ts @@ -202,6 +202,61 @@ function makeBrowseLink( return `/browse/${escapedFspName}`; } +/** + * Resolves a raw filesystem path (in any format: Linux, Mac, or Windows) to the + * best-matching FSP and remaining subpath. Used by both the navigation input and + * the file selector to accept pasted paths in any OS format. + * + * Returns null if no FSP matches the given path. + */ +function resolvePathToFsp( + rawPath: string, + zonesAndFspData: Record +): { fsp: FileSharePath; subpath: string } | null { + const normalizedInput = convertBackToForwardSlash(rawPath.trim()); + + let bestFsp: FileSharePath | null = null; + let bestMountPath = ''; + + for (const [key, value] of Object.entries(zonesAndFspData)) { + if (!key.startsWith('fsp_')) { + continue; + } + const fsp = value as FileSharePath; + + const candidatePaths = [ + fsp.mount_path, + fsp.linux_path, + fsp.mac_path, + fsp.windows_path ? convertBackToForwardSlash(fsp.windows_path) : null + ]; + + for (const candidatePath of candidatePaths) { + if ( + candidatePath && + normalizedInput.startsWith(candidatePath) && + candidatePath.length > bestMountPath.length + ) { + const rest = normalizedInput.slice(candidatePath.length); + if (rest === '' || rest.startsWith('/')) { + bestFsp = fsp; + bestMountPath = candidatePath; + } + } + } + } + + if (!bestFsp) { + return null; + } + + let subpath = normalizedInput.slice(bestMountPath.length); + if (subpath.startsWith('/')) { + subpath = subpath.slice(1); + } + return { fsp: bestFsp, subpath }; +} + export { convertBackToForwardSlash, escapePathForUrl, @@ -213,5 +268,6 @@ export { makePathSegmentArray, normalizePosixStylePath, removeLastSegmentFromPath, - removeTrailingSlashes + removeTrailingSlashes, + resolvePathToFsp }; diff --git a/tests/test_apps.py b/tests/test_apps.py index 9da0dd7d9..35e99f367 100644 --- a/tests/test_apps.py +++ b/tests/test_apps.py @@ -13,6 +13,7 @@ verify_requirements, _container_sif_name, _build_container_script, + build_command, ) @@ -525,3 +526,45 @@ def test_syntax_error_short_circuits(self): error = validate_path_in_filestore("/data;bad", mock_session) assert error is not None assert "invalid characters" in error + + + +class TestBuildCommandTildeExpansion: + """build_command expands ~ in file/directory params so shlex quoting works.""" + + @pytest.fixture() + def entry_point(self): + return AppEntryPoint( + id="test", + name="test", + command="test_cmd", + parameters=[ + { + "key": "output_dir", + "name": "Output Directory", + "type": "directory", + "flag": "--output_dir", + } + ], + ) + + def test_tilde_expanded_in_directory_param(self, entry_point): + import os + cmd = build_command(entry_point, {"output_dir": "~/data/output"}) + home = os.path.expanduser("~") + expected = f"{home}/data/output" + assert expected in cmd + assert "~" not in cmd + + def test_bare_tilde_expanded(self, entry_point): + import os + cmd = build_command(entry_point, {"output_dir": "~"}) + home = os.path.expanduser("~") + assert home in cmd + assert "~" not in cmd + + def test_absolute_path_unchanged(self, entry_point): + cmd = build_command(entry_point, {"output_dir": "/data/output"}) + assert "/data/output" in cmd + + diff --git a/tests/test_database.py b/tests/test_database.py index ac6a8a178..93ae91d54 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -2,12 +2,15 @@ import os import shutil from datetime import datetime +from unittest.mock import patch, MagicMock import pytest import pandas as pd from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker from fileglancer.database import * +from fileglancer.database import _find_best_fsp_match +from fileglancer.model import FileSharePath from fileglancer.utils import slugify_path def create_file_share_path_dicts(df): @@ -419,3 +422,86 @@ def test_find_fsp_from_absolute_path_with_symlink_resolution(db_session, temp_di finally: shutil.rmtree(symlink_container) + +# --- _find_best_fsp_match tests --- + +class TestFindBestFspMatch: + """Unit tests for the shared _find_best_fsp_match helper.""" + + @pytest.fixture() + def fsps(self): + return [ + FileSharePath( + zone="z", name="short", + mount_path="/mnt/short", + linux_path="/linux/short", + mac_path="smb://server/short", + ), + FileSharePath( + zone="z", name="long", + mount_path="/mnt/long", + linux_path="/linux/short/nested", + mac_path="smb://server/short/nested", + ), + ] + + def test_longest_match_wins(self, fsps): + result = _find_best_fsp_match( + fsps, "smb://server/short/nested/file.txt", + lambda fsp: [fsp.mac_path], + ) + assert result is not None + assert result[0].name == "long" + assert result[1] == "file.txt" + + def test_boundary_safety(self, fsps): + """A prefix that doesn't end on a separator boundary must not match.""" + result = _find_best_fsp_match( + fsps, "/linux/shortcut/file.txt", + lambda fsp: [fsp.linux_path], + ) + assert result is None + + def test_exact_match_returns_empty_subpath(self, fsps): + result = _find_best_fsp_match( + fsps, "/linux/short", + lambda fsp: [fsp.linux_path], + ) + assert result is not None + assert result[0].name == "short" + assert result[1] == "" + + def test_none_candidates_are_skipped(self): + fsp = FileSharePath(zone="z", name="test", mount_path="/mnt/test") + result = _find_best_fsp_match( + [fsp], "/mnt/test/file", + lambda f: [None, f.mount_path, None], + ) + assert result is not None + assert result[1] == "file" + + def test_custom_separator(self): + """os.sep separator is respected for boundary checks.""" + fsp = FileSharePath(zone="z", name="test", mount_path="/mnt/test") + # With "/" separator, this should match + result_slash = _find_best_fsp_match( + [fsp], "/mnt/test/sub", + lambda f: [f.mount_path], + separator="/", + ) + assert result_slash is not None + assert result_slash[1] == "sub" + + def test_no_fsps_returns_none(self): + result = _find_best_fsp_match([], "/any/path", lambda f: [f.mount_path]) + assert result is None + + def test_no_matching_candidate_returns_none(self): + fsp = FileSharePath(zone="z", name="test", mount_path="/mnt/test") + result = _find_best_fsp_match( + [fsp], "/completely/different", + lambda f: [f.mount_path], + ) + assert result is None + +