From 85f9758f6bff1935d73406657fdb9fd715f430cb Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Tue, 24 Mar 2026 19:39:22 +0000 Subject: [PATCH 01/11] refactor: move FileSelector components and BreadcrumbSegment to shared locations Move FileSelector components from BrowsePage/FileSelector/ to FileSelector/ and BreadcrumbSegment from BrowsePage/ to widgets/ for better reusability. Update import paths in consumers. --- frontend/src/components/ui/AppsPage/AppLaunchForm.tsx | 2 +- frontend/src/components/ui/BrowsePage/Crumbs.tsx | 2 +- frontend/src/components/ui/Dialogs/ConvertFile.tsx | 2 +- .../{BrowsePage => }/FileSelector/FileSelectorBreadcrumbs.tsx | 2 +- .../ui/{BrowsePage => }/FileSelector/FileSelectorButton.tsx | 0 .../ui/{BrowsePage => }/FileSelector/FileSelectorTable.tsx | 0 .../components/ui/{BrowsePage => widgets}/BreadcrumbSegment.tsx | 0 7 files changed, 4 insertions(+), 4 deletions(-) rename frontend/src/components/ui/{BrowsePage => }/FileSelector/FileSelectorBreadcrumbs.tsx (97%) rename frontend/src/components/ui/{BrowsePage => }/FileSelector/FileSelectorButton.tsx (100%) rename frontend/src/components/ui/{BrowsePage => }/FileSelector/FileSelectorTable.tsx (100%) rename frontend/src/components/ui/{BrowsePage => widgets}/BreadcrumbSegment.tsx (100%) diff --git a/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx b/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx index 9728eae4c..19ca624d2 100644 --- a/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx +++ b/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx @@ -10,7 +10,7 @@ import { HiOutlineTrash } from 'react-icons/hi'; -import FileSelectorButton from '@/components/ui/BrowsePage/FileSelector/FileSelectorButton'; +import FileSelectorButton from '@/components/ui/FileSelector/FileSelectorButton'; import { usePreferencesContext } from '@/contexts/PreferencesContext'; import { validatePaths } from '@/queries/appsQueries'; import { useClusterDefaultsQuery } from '@/queries/jobsQueries'; diff --git a/frontend/src/components/ui/BrowsePage/Crumbs.tsx b/frontend/src/components/ui/BrowsePage/Crumbs.tsx index 95300ff3a..5edd18411 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 c6e2072ea..ee5beae61 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 34faa0a49..8cd1b3f28 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 100% rename from frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorButton.tsx rename to frontend/src/components/ui/FileSelector/FileSelectorButton.tsx diff --git a/frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorTable.tsx b/frontend/src/components/ui/FileSelector/FileSelectorTable.tsx similarity index 100% rename from frontend/src/components/ui/BrowsePage/FileSelector/FileSelectorTable.tsx rename to frontend/src/components/ui/FileSelector/FileSelectorTable.tsx 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 From 381ad6dfa1068e289040b3f58b70f04a126db193 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Tue, 24 Mar 2026 19:39:27 +0000 Subject: [PATCH 02/11] fix: include 'local' group in FSP visibility filtering --- frontend/src/utils/groupFiltering.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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' + ); } /** From 6bcfbd422eef80b92507c6ec5d1bf123cd782663 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Tue, 24 Mar 2026 19:39:35 +0000 Subject: [PATCH 03/11] feat: add search filtering for files and folders in file selector Add text-based filtering at all navigation levels (zones, FSPs, and filesystem). Search clears automatically on navigation. Wrap navigateToLocation in useCallback and refactor handleItemDoubleClick to use it. --- .../ui/FileSelector/FileSelectorButton.tsx | 47 +++++++++- frontend/src/hooks/useFileSelector.ts | 90 +++++++++++++------ 2 files changed, 105 insertions(+), 32 deletions(-) diff --git a/frontend/src/components/ui/FileSelector/FileSelectorButton.tsx b/frontend/src/components/ui/FileSelector/FileSelectorButton.tsx index 6ac84033a..7efaa45d3 100644 --- a/frontend/src/components/ui/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'; @@ -56,7 +56,12 @@ export default function FileSelectorButton({ navigateToLocation, selectItem, handleItemDoubleClick, - reset + reset, + searchQuery, + handleSearchChange, + clearSearch, + isFilteredByGroups, + userHasGroups } = useFileSelector({ initialLocation, initialPath: showDialog ? effectiveInitialPath : undefined, @@ -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 */}
diff --git a/frontend/src/hooks/useFileSelector.ts b/frontend/src/hooks/useFileSelector.ts index b79867e95..603caa40a 100644 --- a/frontend/src/hooks/useFileSelector.ts +++ b/frontend/src/hooks/useFileSelector.ts @@ -1,4 +1,5 @@ import { useState, useCallback, useEffect, useMemo, useRef } from 'react'; +import type { ChangeEvent } from 'react'; import { useZoneAndFspMapContext } from '@/contexts/ZonesAndFspMapContext'; import { usePreferencesContext } from '@/contexts/PreferencesContext'; @@ -62,6 +63,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(() => { @@ -185,6 +202,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 +233,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 +255,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 +269,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 ? { @@ -342,39 +382,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 +412,11 @@ export default function useFileSelector(options?: FileSelectorOptions) { navigateToLocation, selectItem, handleItemDoubleClick, - reset + reset, + searchQuery, + handleSearchChange, + clearSearch, + isFilteredByGroups, + userHasGroups }; } From cac79d3b3acb565dbdfddd58fee9c21e94dd5bb8 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Tue, 24 Mar 2026 19:39:46 +0000 Subject: [PATCH 04/11] style: match file selector styling to file browser Use filled HiFolder icon instead of HiOutlineFolder. Match row hover, selection, and alternating row colors to the file browser. Use text-foreground for cell text instead of text-primary-light. Remove hover underline from breadcrumb segments in dialogs. --- .../componentTests/FileSelector.test.tsx | 271 ++++++++++++++++++ .../ui/FileSelector/FileSelectorTable.tsx | 8 +- .../ui/widgets/BreadcrumbSegment.tsx | 2 +- 3 files changed, 276 insertions(+), 5 deletions(-) create mode 100644 frontend/src/__tests__/componentTests/FileSelector.test.tsx 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/components/ui/FileSelector/FileSelectorTable.tsx b/frontend/src/components/ui/FileSelector/FileSelectorTable.tsx index 6a85e42e3..772d8c917 100644 --- a/frontend/src/components/ui/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'; @@ -95,7 +95,7 @@ export default function FileSelectorTable({ } else { // At filesystem level: show folder or file icon icon = file.is_dir ? ( - + ) : ( ); @@ -182,14 +182,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/widgets/BreadcrumbSegment.tsx b/frontend/src/components/ui/widgets/BreadcrumbSegment.tsx index 72d79f950..230db69e6 100644 --- a/frontend/src/components/ui/widgets/BreadcrumbSegment.tsx +++ b/frontend/src/components/ui/widgets/BreadcrumbSegment.tsx @@ -56,7 +56,7 @@ export default function BreadcrumbSegment({ // Callback-based navigation (for dialogs) {label} From a8200ee4268dab0121d6b3e5132fa88145f0b0c1 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 27 Mar 2026 15:30:23 +0000 Subject: [PATCH 05/11] refactor: extract shared resolvePathToFsp utility for path-to-FSP resolution Move path-to-FSP matching logic into a shared utility that checks all path formats (linux, mac, windows, mount_path) with path-boundary safety. Both useNavigationInput and useFileSelector now use this shared function. --- .../__tests__/unitTests/pathHandling.test.ts | 151 +++++++++++++++++- frontend/src/hooks/useFileSelector.ts | 38 +---- frontend/src/hooks/useNavigationInput.ts | 66 +------- frontend/src/utils/pathHandling.ts | 58 ++++++- 4 files changed, 220 insertions(+), 93 deletions(-) diff --git a/frontend/src/__tests__/unitTests/pathHandling.test.ts b/frontend/src/__tests__/unitTests/pathHandling.test.ts index 11e33afdf..ba7d9de83 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,154 @@ 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/hooks/useFileSelector.ts b/frontend/src/hooks/useFileSelector.ts index 603caa40a..9814a61d3 100644 --- a/frontend/src/hooks/useFileSelector.ts +++ b/frontend/src/hooks/useFileSelector.ts @@ -5,7 +5,10 @@ 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'; @@ -92,35 +95,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('/'); @@ -134,7 +112,7 @@ export default function useFileSelector(options?: FileSelectorOptions) { setState({ currentLocation: { type: 'filesystem', - fspName: bestFsp.name, + fspName: resolved.fsp.name, path: subPath || '.' }, selectedItem: null 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/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 }; From 9490adfd605623955765f9886977e094048298c0 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 27 Mar 2026 15:30:36 +0000 Subject: [PATCH 06/11] feat: show file selector paths in user's preferred format Add separate displayPath (user's OS preference) and fullPath (server format) to the file selector's selected item. The dialog and form inputs show the display path while submissions use the server path. --- .../components/ui/AppsPage/AppLaunchForm.tsx | 22 +++++++++++++++--- .../ui/FileSelector/FileSelectorButton.tsx | 16 ++++++------- .../ui/FileSelector/FileSelectorTable.tsx | 1 + frontend/src/hooks/useFileSelector.ts | 23 ++++++++++++++++--- 4 files changed, 48 insertions(+), 14 deletions(-) diff --git a/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx b/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx index 1fcf28168..24f212db6 100644 --- a/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx +++ b/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx @@ -61,6 +61,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 +133,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 />
diff --git a/frontend/src/components/ui/FileSelector/FileSelectorButton.tsx b/frontend/src/components/ui/FileSelector/FileSelectorButton.tsx index 7efaa45d3..9963bc565 100644 --- a/frontend/src/components/ui/FileSelector/FileSelectorButton.tsx +++ b/frontend/src/components/ui/FileSelector/FileSelectorButton.tsx @@ -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; @@ -83,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(); } }; @@ -226,7 +226,7 @@ export default function FileSelectorButton({ {state.selectedItem ? ( - {state.selectedItem.fullPath} + {state.selectedItem.displayPath} ) : (
diff --git a/frontend/src/components/ui/FileSelector/FileSelectorTable.tsx b/frontend/src/components/ui/FileSelector/FileSelectorTable.tsx index 772d8c917..a97ff98f6 100644 --- a/frontend/src/components/ui/FileSelector/FileSelectorTable.tsx +++ b/frontend/src/components/ui/FileSelector/FileSelectorTable.tsx @@ -34,6 +34,7 @@ type FileSelectorTableProps = { name: string; isDir: boolean; fullPath: string; + displayPath: string; } | null; readonly zonesData: Record | undefined; readonly onItemClick: (item: FileOrFolder) => void; diff --git a/frontend/src/hooks/useFileSelector.ts b/frontend/src/hooks/useFileSelector.ts index 9814a61d3..b95a2a4e4 100644 --- a/frontend/src/hooks/useFileSelector.ts +++ b/frontend/src/hooks/useFileSelector.ts @@ -23,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; }; @@ -281,6 +282,7 @@ export default function useFileSelector(options?: FileSelectorOptions) { const selectItem = useCallback( (item?: FileOrFolder) => { let fullPath = ''; + let displayPath = ''; let name = ''; let isDir = true; @@ -291,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 @@ -321,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 @@ -329,6 +339,11 @@ export default function useFileSelector(options?: FileSelectorOptions) { currentFsp, item.path ); + displayPath = getPreferredPathForDisplay( + pathPreference, + currentFsp, + item.path + ); } name = item.name; @@ -342,7 +357,8 @@ export default function useFileSelector(options?: FileSelectorOptions) { selectedItem: { name, isDir, - fullPath + fullPath, + displayPath } })); } @@ -351,6 +367,7 @@ export default function useFileSelector(options?: FileSelectorOptions) { state.currentLocation, currentFsp, effectivePathPreference, + pathPreference, mode, zonesAndFspQuery.data ] From 884bf0918085a96b72caefd32ca7bcbe6aeba1f9 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 27 Mar 2026 15:31:04 +0000 Subject: [PATCH 07/11] fix: expand tilde in app job file/directory parameters Expand ~ using pwd.getpwuid(os.geteuid()) before shlex.quote() wraps the value in single quotes, which would prevent bash tilde expansion. Uses euid so it works when the server runs as root with seteuid. --- fileglancer/apps/core.py | 7 +++++++ tests/test_apps.py | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/fileglancer/apps/core.py b/fileglancer/apps/core.py index 9ffea075f..8e315a292 100644 --- a/fileglancer/apps/core.py +++ b/fileglancer/apps/core.py @@ -484,6 +484,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/tests/test_apps.py b/tests/test_apps.py index 199a306e0..5509fa80c 100644 --- a/tests/test_apps.py +++ b/tests/test_apps.py @@ -12,6 +12,7 @@ verify_requirements, _container_sif_name, _build_container_script, + build_command, ) @@ -467,3 +468,42 @@ 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 From 8bbf6fe918cb6c515643b7c41aa399f0a1cede5e Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 27 Mar 2026 19:12:44 +0000 Subject: [PATCH 08/11] refactor: extract shared _find_best_fsp_match helper for FSP path matching Extract the duplicated "iterate FSPs, longest prefix match with boundary check" loop into a private _find_best_fsp_match() helper in database.py. Refactor find_fsp_from_absolute_path to use it. No behavior change. --- fileglancer/database.py | 82 +++++++++++++++++++++++++++++---------- tests/test_database.py | 85 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 20 deletions(-) diff --git a/fileglancer/database.py b/fileglancer/database.py index f1f885923..37dbfcf92 100644 --- a/fileglancer/database.py +++ b/fileglancer/database.py @@ -526,6 +526,55 @@ 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*. + + This is the shared core of both ``resolve_any_path_format`` (which checks + all OS path fields) and ``find_fsp_from_absolute_path`` (which checks + 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 +595,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/tests/test_database.py b/tests/test_database.py index ac6a8a178..89a6ebd20 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,85 @@ 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 + From 1afaa1a435212fde3e3e42d3bf45d41446951d22 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 27 Mar 2026 19:13:19 +0000 Subject: [PATCH 09/11] fix: resolve Mac/Windows-format paths in app job validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pasting a Mac (smb://…) or Windows (\\…) path into an app form field and submitting failed because validation rejected the non-Linux prefix. Now both frontend and backend resolve paths against all FSP path formats (mount_path, linux_path, mac_path, windows_path) before validation, converting to the canonical server mount_path. --- fileglancer/apps/core.py | 15 +++ fileglancer/database.py | 31 +++++ .../components/ui/AppsPage/AppLaunchForm.tsx | 33 +++++- tests/test_apps.py | 109 +++++++++++++++++ tests/test_database.py | 110 ++++++++++++++++++ 5 files changed, 295 insertions(+), 3 deletions(-) diff --git a/fileglancer/apps/core.py b/fileglancer/apps/core.py index 8e315a292..a3e426e2e 100644 --- a/fileglancer/apps/core.py +++ b/fileglancer/apps/core.py @@ -404,6 +404,13 @@ def validate_path_in_filestore(path_value: str, session) -> str | None: mounts via the database. Returns an error message string if invalid, or None if valid. """ + # Resolve Mac/Windows/alternate-Linux paths to mount_path format first, + # before syntax validation rejects them. + from fileglancer.database import resolve_any_path_format + resolved = resolve_any_path_format(session, path_value) + if resolved is not None: + path_value = resolved + # Syntax check first error = validate_path_for_shell(path_value) if error: @@ -484,6 +491,14 @@ def _validate_parameter_value(param: AppParameter, value, session=None) -> str: if param.type in ("file", "directory"): str_val = str_val.replace("\\", "/") + # Resolve Mac (smb://), Windows (//server/share), or alternate Linux + # paths to the server's mount_path so validation and shell commands + # use the canonical server path. + if session is not None: + from fileglancer.database import resolve_any_path_format + resolved = resolve_any_path_format(session, str_val) + if resolved is not None: + str_val = resolved # 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. diff --git a/fileglancer/database.py b/fileglancer/database.py index 37dbfcf92..7405d3541 100644 --- a/fileglancer/database.py +++ b/fileglancer/database.py @@ -575,6 +575,37 @@ def _find_best_fsp_match( return (best_fsp, subpath) +def resolve_any_path_format(session: Session, path_value: str) -> Optional[str]: + """ + Resolve a path in any OS format (Mac smb://, Windows UNC, Linux) to the + server's mount_path equivalent. + + Normalises backslashes to forward slashes, then checks the input against + every FSP's mount_path, linux_path, mac_path, and windows_path. Returns + the mount_path-based equivalent (mount_path + subpath) if a match is found, + or None if no FSP matches. + """ + normalized = path_value.replace("\\", "/") + paths = get_file_share_paths(session) + + def _all_path_formats(fsp: FileSharePath): + return [ + fsp.mount_path, + fsp.linux_path, + fsp.mac_path, + fsp.windows_path.replace("\\", "/") if fsp.windows_path else None, + ] + + result = _find_best_fsp_match(paths, normalized, _all_path_formats) + if result is None: + return None + + fsp, subpath = result + if subpath: + return fsp.mount_path + "/" + subpath + return fsp.mount_path + + 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. diff --git a/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx b/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx index 24f212db6..cba8a4553 100644 --- a/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx +++ b/frontend/src/components/ui/AppsPage/AppLaunchForm.tsx @@ -12,9 +12,13 @@ import { 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, @@ -688,6 +692,7 @@ export default function AppLaunchForm({ initialContainerArgs }: AppLaunchFormProps) { const { defaultExtraArgs } = usePreferencesContext(); + const { zonesAndFspQuery } = useZoneAndFspMapContext(); const clusterDefaultsQuery = useClusterDefaultsQuery(); const allParams = flattenParameters([ ...entryPoint.parameters, @@ -776,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 @@ -830,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://') && @@ -897,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/tests/test_apps.py b/tests/test_apps.py index 5509fa80c..b6fcbb606 100644 --- a/tests/test_apps.py +++ b/tests/test_apps.py @@ -469,6 +469,51 @@ def test_syntax_error_short_circuits(self): assert error is not None assert "invalid characters" in error + def test_mac_smb_path_resolved_to_mount_path(self, tmp_path): + """Mac smb:// path matching an FSP's mac_path resolves to mount_path.""" + test_dir = tmp_path / "scratch" + test_dir.mkdir() + + from fileglancer.model import FileSharePath + fsp = FileSharePath( + zone="test", name="test", + mount_path=str(tmp_path), + mac_path="smb://server/share", + ) + + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]), \ + patch("fileglancer.database.find_fsp_from_absolute_path", + return_value=(fsp, "scratch")): + error = validate_path_in_filestore("smb://server/share/scratch", mock_session) + assert error is None + + def test_windows_unc_path_resolved_to_mount_path(self, tmp_path): + """Windows UNC path matching an FSP's windows_path resolves to mount_path.""" + test_dir = tmp_path / "data" + test_dir.mkdir() + + from fileglancer.model import FileSharePath + fsp = FileSharePath( + zone="test", name="test", + mount_path=str(tmp_path), + windows_path="\\\\server\\share", + ) + + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]), \ + patch("fileglancer.database.find_fsp_from_absolute_path", + return_value=(fsp, "data")): + error = validate_path_in_filestore("\\\\server\\share\\data", mock_session) + assert error is None + + def test_unresolvable_smb_path_rejected(self): + """smb:// path that matches no FSP is rejected.""" + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[]): + error = validate_path_in_filestore("smb://unknown/share/foo", mock_session) + assert error is not None + class TestBuildCommandTildeExpansion: """build_command expands ~ in file/directory params so shlex quoting works.""" @@ -507,3 +552,67 @@ def test_bare_tilde_expanded(self, entry_point): def test_absolute_path_unchanged(self, entry_point): cmd = build_command(entry_point, {"output_dir": "/data/output"}) assert "/data/output" in cmd + + +class TestBuildCommandPathResolution: + """build_command resolves Mac/Windows paths to mount_path when a session is provided.""" + + @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_mac_smb_path_resolved_in_command(self, entry_point, tmp_path): + """Mac smb:// path is resolved to mount_path in the built command.""" + (tmp_path / "output").mkdir() + + from fileglancer.model import FileSharePath + fsp = FileSharePath( + zone="test", name="test", + mount_path=str(tmp_path), + mac_path="smb://server/share", + ) + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]), \ + patch("fileglancer.database.find_fsp_from_absolute_path", + return_value=(fsp, "output")): + cmd = build_command( + entry_point, + {"output_dir": "smb://server/share/output"}, + session=mock_session, + ) + assert str(tmp_path) + "/output" in cmd + assert "smb://" not in cmd + + def test_windows_unc_path_resolved_in_command(self, entry_point, tmp_path): + """Windows UNC path is resolved to mount_path in the built command.""" + (tmp_path / "data").mkdir() + + from fileglancer.model import FileSharePath + fsp = FileSharePath( + zone="test", name="test", + mount_path=str(tmp_path), + windows_path="\\\\server\\share", + ) + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]), \ + patch("fileglancer.database.find_fsp_from_absolute_path", + return_value=(fsp, "data")): + cmd = build_command( + entry_point, + {"output_dir": "\\\\server\\share\\data"}, + session=mock_session, + ) + assert str(tmp_path) + "/data" in cmd + assert "\\\\" not in cmd diff --git a/tests/test_database.py b/tests/test_database.py index 89a6ebd20..61da26450 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -504,3 +504,113 @@ def test_no_matching_candidate_returns_none(self): ) assert result is None + +# --- resolve_any_path_format tests --- + +class TestResolveAnyPathFormat: + """resolve_any_path_format converts Mac/Windows/Linux paths to mount_path.""" + + def test_mac_smb_path(self): + fsp = FileSharePath( + zone="test", name="test", + mount_path="/mnt/share", + mac_path="smb://server/share", + ) + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): + result = resolve_any_path_format(mock_session, "smb://server/share/sub/dir") + assert result == "/mnt/share/sub/dir" + + def test_windows_unc_path(self): + fsp = FileSharePath( + zone="test", name="test", + mount_path="/mnt/share", + windows_path="\\\\server\\share", + ) + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): + result = resolve_any_path_format(mock_session, "\\\\server\\share\\sub\\dir") + assert result == "/mnt/share/sub/dir" + + def test_linux_path(self): + fsp = FileSharePath( + zone="test", name="test", + mount_path="/mnt/share", + linux_path="/linux/share", + ) + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): + result = resolve_any_path_format(mock_session, "/linux/share/file.txt") + assert result == "/mnt/share/file.txt" + + def test_mount_path_exact(self): + fsp = FileSharePath( + zone="test", name="test", + mount_path="/mnt/share", + ) + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): + result = resolve_any_path_format(mock_session, "/mnt/share") + assert result == "/mnt/share" + + def test_no_match_returns_none(self): + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[]): + result = resolve_any_path_format(mock_session, "smb://unknown/share") + assert result is None + + def test_longest_match_wins(self): + fsp_short = FileSharePath( + zone="test", name="parent", + mount_path="/mnt/parent", + mac_path="smb://server/parent", + ) + fsp_long = FileSharePath( + zone="test", name="child", + mount_path="/mnt/parent/child", + mac_path="smb://server/parent/child", + ) + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp_short, fsp_long]): + result = resolve_any_path_format(mock_session, "smb://server/parent/child/file.txt") + assert result == "/mnt/parent/child/file.txt" + + def test_boundary_safety(self): + """Should not match /linux/abc against /linux/a.""" + fsp = FileSharePath( + zone="test", name="test", + mount_path="/mnt/a", + linux_path="/linux/a", + ) + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): + result = resolve_any_path_format(mock_session, "/linux/abc/file.txt") + assert result is None + + def test_mount_path_already_resolved_returns_mount_path(self): + """A path already in mount_path format should still resolve correctly.""" + fsp = FileSharePath( + zone="test", name="test", + mount_path="/mnt/share", + mac_path="smb://server/share", + ) + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): + result = resolve_any_path_format(mock_session, "/mnt/share/data/output") + assert result == "/mnt/share/data/output" + + def test_windows_path_with_dollar_sign(self): + """Windows paths with $ (e.g. admin shares) should resolve correctly.""" + fsp = FileSharePath( + zone="test", name="test", + mount_path="/mnt/scicompsoft", + windows_path="\\\\prfs.hhmi.org\\scicompsoft$", + ) + mock_session = MagicMock() + with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): + result = resolve_any_path_format( + mock_session, + "\\\\prfs.hhmi.org\\scicompsoft$\\truhlara\\scratch" + ) + assert result == "/mnt/scicompsoft/truhlara/scratch" + From 63d979088952f886e8d3561cbeff6c2afc3e8af7 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 27 Mar 2026 15:16:52 -0400 Subject: [PATCH 10/11] chore: prettier formatting --- frontend/src/__tests__/unitTests/pathHandling.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/frontend/src/__tests__/unitTests/pathHandling.test.ts b/frontend/src/__tests__/unitTests/pathHandling.test.ts index ba7d9de83..a2fff14fb 100644 --- a/frontend/src/__tests__/unitTests/pathHandling.test.ts +++ b/frontend/src/__tests__/unitTests/pathHandling.test.ts @@ -290,10 +290,7 @@ describe('resolvePathToFsp', () => { }); test('matches a Windows-style path with backslashes', () => { - const result = resolvePathToFsp( - '\\\\win\\a\\sub\\folder', - zonesAndFspData - ); + 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'); From b046c9c7151aa142e6172ca4ba189bf735278ecc Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 10 Apr 2026 10:49:32 -0400 Subject: [PATCH 11/11] fix: remove unnecessary path resolution code from backend - frontend already sends path in linux format --- fileglancer/apps/core.py | 15 ------ fileglancer/database.py | 35 +------------ tests/test_apps.py | 106 ------------------------------------- tests/test_database.py | 109 --------------------------------------- 4 files changed, 1 insertion(+), 264 deletions(-) diff --git a/fileglancer/apps/core.py b/fileglancer/apps/core.py index 286f086b3..4d9a2cde4 100644 --- a/fileglancer/apps/core.py +++ b/fileglancer/apps/core.py @@ -435,13 +435,6 @@ def validate_path_in_filestore(path_value: str, session) -> str | None: mounts via the database. Returns an error message string if invalid, or None if valid. """ - # Resolve Mac/Windows/alternate-Linux paths to mount_path format first, - # before syntax validation rejects them. - from fileglancer.database import resolve_any_path_format - resolved = resolve_any_path_format(session, path_value) - if resolved is not None: - path_value = resolved - # Syntax check first error = validate_path_for_shell(path_value) if error: @@ -522,14 +515,6 @@ def _validate_parameter_value(param: AppParameter, value, session=None) -> str: if param.type in ("file", "directory"): str_val = str_val.replace("\\", "/") - # Resolve Mac (smb://), Windows (//server/share), or alternate Linux - # paths to the server's mount_path so validation and shell commands - # use the canonical server path. - if session is not None: - from fileglancer.database import resolve_any_path_format - resolved = resolve_any_path_format(session, str_val) - if resolved is not None: - str_val = resolved # 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. diff --git a/fileglancer/database.py b/fileglancer/database.py index 7405d3541..987c44c4a 100644 --- a/fileglancer/database.py +++ b/fileglancer/database.py @@ -534,9 +534,7 @@ def _find_best_fsp_match( ) -> Optional[tuple[FileSharePath, str]]: """Find the FSP whose candidate path is the longest prefix of *normalized_input*. - This is the shared core of both ``resolve_any_path_format`` (which checks - all OS path fields) and ``find_fsp_from_absolute_path`` (which checks - filesystem-resolved mount paths). + Used by ``find_fsp_from_absolute_path`` to check filesystem-resolved mount paths. Args: fsps: All file share paths to search. @@ -575,37 +573,6 @@ def _find_best_fsp_match( return (best_fsp, subpath) -def resolve_any_path_format(session: Session, path_value: str) -> Optional[str]: - """ - Resolve a path in any OS format (Mac smb://, Windows UNC, Linux) to the - server's mount_path equivalent. - - Normalises backslashes to forward slashes, then checks the input against - every FSP's mount_path, linux_path, mac_path, and windows_path. Returns - the mount_path-based equivalent (mount_path + subpath) if a match is found, - or None if no FSP matches. - """ - normalized = path_value.replace("\\", "/") - paths = get_file_share_paths(session) - - def _all_path_formats(fsp: FileSharePath): - return [ - fsp.mount_path, - fsp.linux_path, - fsp.mac_path, - fsp.windows_path.replace("\\", "/") if fsp.windows_path else None, - ] - - result = _find_best_fsp_match(paths, normalized, _all_path_formats) - if result is None: - return None - - fsp, subpath = result - if subpath: - return fsp.mount_path + "/" + subpath - return fsp.mount_path - - 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. diff --git a/tests/test_apps.py b/tests/test_apps.py index c84a3d08d..35e99f367 100644 --- a/tests/test_apps.py +++ b/tests/test_apps.py @@ -527,50 +527,6 @@ def test_syntax_error_short_circuits(self): assert error is not None assert "invalid characters" in error - def test_mac_smb_path_resolved_to_mount_path(self, tmp_path): - """Mac smb:// path matching an FSP's mac_path resolves to mount_path.""" - test_dir = tmp_path / "scratch" - test_dir.mkdir() - - from fileglancer.model import FileSharePath - fsp = FileSharePath( - zone="test", name="test", - mount_path=str(tmp_path), - mac_path="smb://server/share", - ) - - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]), \ - patch("fileglancer.database.find_fsp_from_absolute_path", - return_value=(fsp, "scratch")): - error = validate_path_in_filestore("smb://server/share/scratch", mock_session) - assert error is None - - def test_windows_unc_path_resolved_to_mount_path(self, tmp_path): - """Windows UNC path matching an FSP's windows_path resolves to mount_path.""" - test_dir = tmp_path / "data" - test_dir.mkdir() - - from fileglancer.model import FileSharePath - fsp = FileSharePath( - zone="test", name="test", - mount_path=str(tmp_path), - windows_path="\\\\server\\share", - ) - - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]), \ - patch("fileglancer.database.find_fsp_from_absolute_path", - return_value=(fsp, "data")): - error = validate_path_in_filestore("\\\\server\\share\\data", mock_session) - assert error is None - - def test_unresolvable_smb_path_rejected(self): - """smb:// path that matches no FSP is rejected.""" - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[]): - error = validate_path_in_filestore("smb://unknown/share/foo", mock_session) - assert error is not None class TestBuildCommandTildeExpansion: @@ -612,65 +568,3 @@ def test_absolute_path_unchanged(self, entry_point): assert "/data/output" in cmd -class TestBuildCommandPathResolution: - """build_command resolves Mac/Windows paths to mount_path when a session is provided.""" - - @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_mac_smb_path_resolved_in_command(self, entry_point, tmp_path): - """Mac smb:// path is resolved to mount_path in the built command.""" - (tmp_path / "output").mkdir() - - from fileglancer.model import FileSharePath - fsp = FileSharePath( - zone="test", name="test", - mount_path=str(tmp_path), - mac_path="smb://server/share", - ) - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]), \ - patch("fileglancer.database.find_fsp_from_absolute_path", - return_value=(fsp, "output")): - cmd = build_command( - entry_point, - {"output_dir": "smb://server/share/output"}, - session=mock_session, - ) - assert str(tmp_path) + "/output" in cmd - assert "smb://" not in cmd - - def test_windows_unc_path_resolved_in_command(self, entry_point, tmp_path): - """Windows UNC path is resolved to mount_path in the built command.""" - (tmp_path / "data").mkdir() - - from fileglancer.model import FileSharePath - fsp = FileSharePath( - zone="test", name="test", - mount_path=str(tmp_path), - windows_path="\\\\server\\share", - ) - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]), \ - patch("fileglancer.database.find_fsp_from_absolute_path", - return_value=(fsp, "data")): - cmd = build_command( - entry_point, - {"output_dir": "\\\\server\\share\\data"}, - session=mock_session, - ) - assert str(tmp_path) + "/data" in cmd - assert "\\\\" not in cmd diff --git a/tests/test_database.py b/tests/test_database.py index 61da26450..93ae91d54 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -505,112 +505,3 @@ def test_no_matching_candidate_returns_none(self): assert result is None -# --- resolve_any_path_format tests --- - -class TestResolveAnyPathFormat: - """resolve_any_path_format converts Mac/Windows/Linux paths to mount_path.""" - - def test_mac_smb_path(self): - fsp = FileSharePath( - zone="test", name="test", - mount_path="/mnt/share", - mac_path="smb://server/share", - ) - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): - result = resolve_any_path_format(mock_session, "smb://server/share/sub/dir") - assert result == "/mnt/share/sub/dir" - - def test_windows_unc_path(self): - fsp = FileSharePath( - zone="test", name="test", - mount_path="/mnt/share", - windows_path="\\\\server\\share", - ) - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): - result = resolve_any_path_format(mock_session, "\\\\server\\share\\sub\\dir") - assert result == "/mnt/share/sub/dir" - - def test_linux_path(self): - fsp = FileSharePath( - zone="test", name="test", - mount_path="/mnt/share", - linux_path="/linux/share", - ) - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): - result = resolve_any_path_format(mock_session, "/linux/share/file.txt") - assert result == "/mnt/share/file.txt" - - def test_mount_path_exact(self): - fsp = FileSharePath( - zone="test", name="test", - mount_path="/mnt/share", - ) - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): - result = resolve_any_path_format(mock_session, "/mnt/share") - assert result == "/mnt/share" - - def test_no_match_returns_none(self): - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[]): - result = resolve_any_path_format(mock_session, "smb://unknown/share") - assert result is None - - def test_longest_match_wins(self): - fsp_short = FileSharePath( - zone="test", name="parent", - mount_path="/mnt/parent", - mac_path="smb://server/parent", - ) - fsp_long = FileSharePath( - zone="test", name="child", - mount_path="/mnt/parent/child", - mac_path="smb://server/parent/child", - ) - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp_short, fsp_long]): - result = resolve_any_path_format(mock_session, "smb://server/parent/child/file.txt") - assert result == "/mnt/parent/child/file.txt" - - def test_boundary_safety(self): - """Should not match /linux/abc against /linux/a.""" - fsp = FileSharePath( - zone="test", name="test", - mount_path="/mnt/a", - linux_path="/linux/a", - ) - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): - result = resolve_any_path_format(mock_session, "/linux/abc/file.txt") - assert result is None - - def test_mount_path_already_resolved_returns_mount_path(self): - """A path already in mount_path format should still resolve correctly.""" - fsp = FileSharePath( - zone="test", name="test", - mount_path="/mnt/share", - mac_path="smb://server/share", - ) - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): - result = resolve_any_path_format(mock_session, "/mnt/share/data/output") - assert result == "/mnt/share/data/output" - - def test_windows_path_with_dollar_sign(self): - """Windows paths with $ (e.g. admin shares) should resolve correctly.""" - fsp = FileSharePath( - zone="test", name="test", - mount_path="/mnt/scicompsoft", - windows_path="\\\\prfs.hhmi.org\\scicompsoft$", - ) - mock_session = MagicMock() - with patch("fileglancer.database.get_file_share_paths", return_value=[fsp]): - result = resolve_any_path_format( - mock_session, - "\\\\prfs.hhmi.org\\scicompsoft$\\truhlara\\scratch" - ) - assert result == "/mnt/scicompsoft/truhlara/scratch" -