From 0b2922ac3ddd74dfa08be3ed75e8b3529d03518b Mon Sep 17 00:00:00 2001 From: Mark Kittisopikul Date: Mon, 11 May 2026 17:15:01 -0400 Subject: [PATCH 01/11] feat: cap directory listing at max_directory_count to avoid full scans Large directories caused slow responses because yield_file_infos_paginated read and sorted all entries regardless of pagination. Now at most max_directory_count+1 entries are read via itertools.islice; if truncated, entries are returned in filesystem order (sort skipped) and is_truncated is set in the API response. The frontend disables column sorting when is_truncated is true. max_directory_count defaults to 10000 and is configurable via settings. Co-Authored-By: Claude Sonnet 4.6 --- fileglancer/filestore.py | 31 +++++++++--- fileglancer/server.py | 3 +- fileglancer/settings.py | 4 ++ .../components/ui/BrowsePage/FileTable.tsx | 2 +- frontend/src/queries/fileQueries.ts | 5 +- tests/test_pagination.py | 49 +++++++++++++------ 6 files changed, 70 insertions(+), 24 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index b6d633e4..d8a408ef 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -3,6 +3,7 @@ rooted at a specific directory. """ +import itertools import os import stat try: @@ -18,6 +19,7 @@ from loguru import logger from .database import find_fsp_from_absolute_path +from .settings import get_settings from .model import FileSharePath # Default buffer size for streaming file contents @@ -490,7 +492,8 @@ def check_is_binary(self, path: Optional[str] = None, sample_size: int = 4096) - def yield_file_infos_paginated(self, path: Optional[str] = None, current_user: str = None, session = None, limit: int = 200, - cursor: Optional[str] = None) -> tuple[list[FileInfo], bool, Optional[str], int]: + cursor: Optional[str] = None, + max_count: Optional[int] = None) -> tuple[list[FileInfo], bool, Optional[str], int, bool]: """ Return a page of FileInfo objects for children of the given path. @@ -505,19 +508,35 @@ def yield_file_infos_paginated(self, path: Optional[str] = None, current_user: s limit: Maximum number of entries to return. cursor: Name of the last entry from the previous page. Entries after this name (in sort order) are returned. + max_count: Maximum number of directory entries to read and sort. + Defaults to Settings.max_directory_count. Directories with more + entries are truncated at this limit; files beyond max_count are + not accessible via pagination. Returns: - Tuple of (file_infos, has_more, next_cursor, total_count). + Tuple of (file_infos, has_more, next_cursor, total_count, is_truncated). + total_count is capped at max_count. is_truncated is True when the + directory has more entries than max_count. """ + if max_count is None: + max_count = get_settings().max_directory_count + full_path = self._check_path_in_root(path) # Compute user groups once for the entire listing user_groups = FileInfo._get_user_groups(current_user) if current_user else None - # Collect and sort all entries — scandir's is_dir() is free on Linux - entries = list(os.scandir(full_path)) - entries.sort(key=lambda e: (not e.is_dir(follow_symlinks=False), e.name)) + # Read max_count + 1 entries: the extra one detects truncation without + # reading the entire directory. + with os.scandir(full_path) as scanner: + entries = list(itertools.islice(scanner, max_count + 1)) total_count = len(entries) + is_truncated = total_count > max_count + if is_truncated: + entries = entries[:max_count] + total_count = max_count + else: + entries.sort(key=lambda e: (not e.is_dir(follow_symlinks=False), e.name)) # Apply cursor: skip past the cursor entry if cursor: @@ -548,7 +567,7 @@ def yield_file_infos_paginated(self, path: Optional[str] = None, current_user: s continue next_cursor = page_entries[-1].name if has_more and page_entries else None - return file_infos, has_more, next_cursor, total_count + return file_infos, has_more, next_cursor, total_count, is_truncated def yield_file_infos(self, path: Optional[str] = None, current_user: str = None, session = None) -> Generator[FileInfo, None, None]: """ diff --git a/fileglancer/server.py b/fileglancer/server.py index de3d0c7f..691e59b3 100644 --- a/fileglancer/server.py +++ b/fileglancer/server.py @@ -1398,7 +1398,7 @@ async def get_file_metadata(path_name: str, subpath: Optional[str] = Query(''), if file_info.is_dir: try: if limit is not None: - files, has_more, next_cursor, total_count = filestore.yield_file_infos_paginated( + files, has_more, next_cursor, total_count, is_truncated = filestore.yield_file_infos_paginated( subpath, current_user=username, session=session, limit=limit, cursor=cursor ) @@ -1406,6 +1406,7 @@ async def get_file_metadata(path_name: str, subpath: Optional[str] = Query(''), result["has_more"] = has_more result["next_cursor"] = next_cursor result["total_count"] = total_count + result["is_truncated"] = is_truncated else: files = list(filestore.yield_file_infos(subpath, current_user=username, session=session)) result["files"] = [json.loads(f.model_dump_json()) for f in files] diff --git a/fileglancer/settings.py b/fileglancer/settings.py index 3b21bdef..0b6eaa2b 100644 --- a/fileglancer/settings.py +++ b/fileglancer/settings.py @@ -78,6 +78,10 @@ class Settings(BaseSettings): # Maximum size of the sharing key LRU cache sharing_key_cache_size: int = 1000 + # Maximum number of directory entries reported in total_count for paginated listings. + # Prevents a full directory scan for the count in very large directories. + max_directory_count: int = 10000 + # OKTA OAuth/OIDC settings for authentication okta_domain: Optional[str] = None okta_client_id: Optional[str] = None diff --git a/frontend/src/components/ui/BrowsePage/FileTable.tsx b/frontend/src/components/ui/BrowsePage/FileTable.tsx index 9639463d..97b70a65 100644 --- a/frontend/src/components/ui/BrowsePage/FileTable.tsx +++ b/frontend/src/components/ui/BrowsePage/FileTable.tsx @@ -96,7 +96,7 @@ export default function Table({ parent = parent.parentElement; } }, []); - const sortingEnabled = !hasNextPage; + const sortingEnabled = !hasNextPage && !fileQuery.data?.isTruncated; const selectedFileNames = useMemo( () => new Set(fileBrowserState.selectedFiles.map(file => file.name)), diff --git a/frontend/src/queries/fileQueries.ts b/frontend/src/queries/fileQueries.ts index 2a1a9c31..8a6b0345 100644 --- a/frontend/src/queries/fileQueries.ts +++ b/frontend/src/queries/fileQueries.ts @@ -28,6 +28,7 @@ type FileBrowserPageResponse = { has_more?: boolean; next_cursor?: string | null; total_count?: number | null; + is_truncated?: boolean; }; export type FileQueryData = { @@ -37,6 +38,7 @@ export type FileQueryData = { errorMessage?: string; hasMore: boolean; totalCount: number | null; + isTruncated: boolean; }; // Query key factory for hierarchical cache management @@ -142,7 +144,8 @@ export default function useFileQuery( currentFileOrFolder, files: allFiles, hasMore: lastPage?.has_more ?? false, - totalCount + totalCount, + isTruncated: lastPage?.is_truncated ?? false }; }; diff --git a/tests/test_pagination.py b/tests/test_pagination.py index cfaef1ec..48dfa38e 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -37,30 +37,32 @@ class TestPaginatedListing: def test_first_page(self, pagination_store): """First page returns correct number of items with has_more=True.""" - infos, has_more, next_cursor, total_count = ( + infos, has_more, next_cursor, total_count, is_truncated = ( pagination_store.yield_file_infos_paginated(None, limit=5) ) assert len(infos) == 5 assert has_more is True assert next_cursor is not None assert total_count == 13 # 3 dirs + 10 files + assert is_truncated is False def test_full_listing_no_pagination(self, pagination_store): """When limit >= total entries, has_more is False and next_cursor is None.""" - infos, has_more, next_cursor, total_count = ( + infos, has_more, next_cursor, total_count, is_truncated = ( pagination_store.yield_file_infos_paginated(None, limit=100) ) assert len(infos) == 13 assert has_more is False assert next_cursor is None assert total_count == 13 + assert is_truncated is False def test_cursor_continuation(self, pagination_store): """Cursor returns the next page starting after the cursor entry.""" - infos1, _, cursor1, _ = pagination_store.yield_file_infos_paginated( + infos1, _, cursor1, _, _ = pagination_store.yield_file_infos_paginated( None, limit=5 ) - infos2, has_more2, cursor2, total2 = ( + infos2, has_more2, cursor2, total2, is_truncated2 = ( pagination_store.yield_file_infos_paginated(None, limit=5, cursor=cursor1) ) @@ -71,13 +73,14 @@ def test_cursor_continuation(self, pagination_store): assert len(infos2) == 5 assert has_more2 is True assert total2 == 13 + assert is_truncated2 is False def test_all_pages_cover_all_entries(self, pagination_store): """Iterating through all pages returns every entry exactly once.""" all_names = [] cursor = None while True: - infos, has_more, cursor, _ = ( + infos, has_more, cursor, _, _ = ( pagination_store.yield_file_infos_paginated(None, limit=4, cursor=cursor) ) all_names.extend(fi.name for fi in infos) @@ -89,7 +92,7 @@ def test_all_pages_cover_all_entries(self, pagination_store): def test_dirs_sorted_before_files(self, pagination_store): """Directories appear before files in the listing.""" - infos, _, _, _ = pagination_store.yield_file_infos_paginated( + infos, _, _, _, _ = pagination_store.yield_file_infos_paginated( None, limit=100 ) dir_indices = [i for i, fi in enumerate(infos) if fi.is_dir] @@ -98,7 +101,7 @@ def test_dirs_sorted_before_files(self, pagination_store): def test_alphabetical_within_type(self, pagination_store): """Names are sorted alphabetically within dirs and within files.""" - infos, _, _, _ = pagination_store.yield_file_infos_paginated( + infos, _, _, _, _ = pagination_store.yield_file_infos_paginated( None, limit=100 ) dir_names = [fi.name for fi in infos if fi.is_dir] @@ -108,7 +111,7 @@ def test_alphabetical_within_type(self, pagination_store): def test_deleted_cursor_fallback(self, pagination_dir, pagination_store): """When cursor entry no longer exists, listing starts from the beginning.""" - infos, has_more, _, total = ( + infos, has_more, _, total, is_truncated = ( pagination_store.yield_file_infos_paginated( None, limit=5, cursor="nonexistent_file" ) @@ -117,17 +120,18 @@ def test_deleted_cursor_fallback(self, pagination_dir, pagination_store): assert len(infos) == 5 assert has_more is True assert total == 13 + assert is_truncated is False # First entry should be the first dir (sorted first) assert infos[0].name == "dir_00" def test_last_page_boundary(self, pagination_store): """Last page has has_more=False and next_cursor=None.""" # Get first 10 entries - _, _, cursor, _ = pagination_store.yield_file_infos_paginated( + _, _, cursor, _, _ = pagination_store.yield_file_infos_paginated( None, limit=10 ) # Get remaining 3 - infos, has_more, next_cursor, _ = ( + infos, has_more, next_cursor, _, _ = ( pagination_store.yield_file_infos_paginated(None, limit=10, cursor=cursor) ) assert len(infos) == 3 @@ -137,10 +141,10 @@ def test_last_page_boundary(self, pagination_store): def test_exact_limit_boundary(self, pagination_store): """When entries remaining == limit, has_more is False.""" # 13 total, get first 10, then exactly 3 remain - _, _, cursor, _ = pagination_store.yield_file_infos_paginated( + _, _, cursor, _, _ = pagination_store.yield_file_infos_paginated( None, limit=10 ) - infos, has_more, next_cursor, _ = ( + infos, has_more, next_cursor, _, _ = ( pagination_store.yield_file_infos_paginated(None, limit=3, cursor=cursor) ) assert len(infos) == 3 @@ -149,13 +153,14 @@ def test_exact_limit_boundary(self, pagination_store): def test_limit_one(self, pagination_store): """Pagination works with limit=1.""" - infos, has_more, cursor, total = ( + infos, has_more, cursor, total, is_truncated = ( pagination_store.yield_file_infos_paginated(None, limit=1) ) assert len(infos) == 1 assert has_more is True assert cursor == infos[0].name assert total == 13 + assert is_truncated is False def test_empty_directory(self): """Paginated listing of an empty directory.""" @@ -163,13 +168,14 @@ def test_empty_directory(self): try: fsp = FileSharePath(zone="test", name="test", mount_path=temp_dir) store = Filestore(fsp) - infos, has_more, next_cursor, total = ( + infos, has_more, next_cursor, total, is_truncated = ( store.yield_file_infos_paginated(None, limit=10) ) assert infos == [] assert has_more is False assert next_cursor is None assert total == 0 + assert is_truncated is False finally: shutil.rmtree(temp_dir) @@ -181,12 +187,25 @@ def test_subdir_pagination(self, pagination_dir, pagination_store): with open(os.path.join(subdir, f"sub_{i}.txt"), "w") as f: f.write(f"sub {i}") - infos, has_more, _, total = ( + infos, has_more, _, total, is_truncated = ( pagination_store.yield_file_infos_paginated("dir_00", limit=3) ) assert len(infos) == 3 assert has_more is True assert total == 5 + assert is_truncated is False + + def test_truncation(self, pagination_dir): + """is_truncated is True and entries are in filesystem order (not sorted) when truncated.""" + fsp = FileSharePath(zone="test", name="test", mount_path=pagination_dir) + store = Filestore(fsp) + # max_count=5 on a 13-entry directory + infos, has_more, next_cursor, total_count, is_truncated = ( + store.yield_file_infos_paginated(None, limit=10, max_count=5) + ) + assert total_count == 5 + assert is_truncated is True + assert len(infos) == 5 # limit=10 > max_count=5, so all 5 returned def test_chroot_escape_raises(self, pagination_store): """Attempting to escape root raises ValueError.""" From 17be27e047b490095e1398dede3c664108175061 Mon Sep 17 00:00:00 2001 From: Mark Kittisopikul Date: Mon, 11 May 2026 17:17:32 -0400 Subject: [PATCH 02/11] Fix linting issues --- frontend/src/components/ui/BrowsePage/FileBrowser.tsx | 6 +++--- frontend/src/components/ui/Dialogs/ChangePermissions.tsx | 3 ++- .../src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/frontend/src/components/ui/BrowsePage/FileBrowser.tsx b/frontend/src/components/ui/BrowsePage/FileBrowser.tsx index 7beea4b9..cab32248 100644 --- a/frontend/src/components/ui/BrowsePage/FileBrowser.tsx +++ b/frontend/src/components/ui/BrowsePage/FileBrowser.tsx @@ -128,9 +128,9 @@ export default function FileBrowser({ const propertiesTarget = fileBrowserState.propertiesTarget; const isFavorite = Boolean( fspName && - folderPreferenceMap[ - makeMapKey('folder', `${fspName}_${propertiesTarget.path}`) - ] + folderPreferenceMap[ + makeMapKey('folder', `${fspName}_${propertiesTarget.path}`) + ] ); return [ diff --git a/frontend/src/components/ui/Dialogs/ChangePermissions.tsx b/frontend/src/components/ui/Dialogs/ChangePermissions.tsx index 48fa2ea4..63573cb4 100644 --- a/frontend/src/components/ui/Dialogs/ChangePermissions.tsx +++ b/frontend/src/components/ui/Dialogs/ChangePermissions.tsx @@ -228,7 +228,8 @@ export default function ChangePermissions({ className="!rounded-md" disabled={Boolean( mutations.changePermissions.isPending || - localPermissions === fileBrowserState.propertiesTarget.permissions + localPermissions === + fileBrowserState.propertiesTarget.permissions )} type="submit" > diff --git a/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx b/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx index 6bf87496..b94e9bae 100644 --- a/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx +++ b/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx @@ -279,7 +279,7 @@ export default function PropertiesDrawer({ } disabled={Boolean( externalDataUrlQuery.data || - fileBrowserState.propertiesTarget.hasRead === false + fileBrowserState.propertiesTarget.hasRead === false )} id="share-switch" label={ From de61f2080b5517f61f3016cf799327e7e2e9e3ce Mon Sep 17 00:00:00 2001 From: Mark Kittisopikul Date: Mon, 11 May 2026 17:28:01 -0400 Subject: [PATCH 03/11] chore: prettier formatting with pinned 3.8.3 Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/components/ui/BrowsePage/FileBrowser.tsx | 6 +++--- frontend/src/components/ui/Dialogs/ChangePermissions.tsx | 3 +-- .../src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/frontend/src/components/ui/BrowsePage/FileBrowser.tsx b/frontend/src/components/ui/BrowsePage/FileBrowser.tsx index cab32248..7beea4b9 100644 --- a/frontend/src/components/ui/BrowsePage/FileBrowser.tsx +++ b/frontend/src/components/ui/BrowsePage/FileBrowser.tsx @@ -128,9 +128,9 @@ export default function FileBrowser({ const propertiesTarget = fileBrowserState.propertiesTarget; const isFavorite = Boolean( fspName && - folderPreferenceMap[ - makeMapKey('folder', `${fspName}_${propertiesTarget.path}`) - ] + folderPreferenceMap[ + makeMapKey('folder', `${fspName}_${propertiesTarget.path}`) + ] ); return [ diff --git a/frontend/src/components/ui/Dialogs/ChangePermissions.tsx b/frontend/src/components/ui/Dialogs/ChangePermissions.tsx index 63573cb4..48fa2ea4 100644 --- a/frontend/src/components/ui/Dialogs/ChangePermissions.tsx +++ b/frontend/src/components/ui/Dialogs/ChangePermissions.tsx @@ -228,8 +228,7 @@ export default function ChangePermissions({ className="!rounded-md" disabled={Boolean( mutations.changePermissions.isPending || - localPermissions === - fileBrowserState.propertiesTarget.permissions + localPermissions === fileBrowserState.propertiesTarget.permissions )} type="submit" > diff --git a/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx b/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx index b94e9bae..6bf87496 100644 --- a/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx +++ b/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx @@ -279,7 +279,7 @@ export default function PropertiesDrawer({ } disabled={Boolean( externalDataUrlQuery.data || - fileBrowserState.propertiesTarget.hasRead === false + fileBrowserState.propertiesTarget.hasRead === false )} id="share-switch" label={ From e368d2c3870b8490f8eb1861e73a694f67d8c1f4 Mon Sep 17 00:00:00 2001 From: Mark Kittisopikul Date: Mon, 11 May 2026 18:09:58 -0400 Subject: [PATCH 04/11] fix: sort entries even when directory is truncated at max_directory_count Without sorting in the truncated case, entries were returned in arbitrary filesystem order, breaking cursor-based pagination and causing each page request to re-scan all max_count entries from scratch to locate the cursor. Co-Authored-By: Claude Sonnet 4.6 --- fileglancer/filestore.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index d8a408ef..8fb0dfeb 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -535,8 +535,7 @@ def yield_file_infos_paginated(self, path: Optional[str] = None, current_user: s if is_truncated: entries = entries[:max_count] total_count = max_count - else: - entries.sort(key=lambda e: (not e.is_dir(follow_symlinks=False), e.name)) + entries.sort(key=lambda e: (not e.is_dir(follow_symlinks=False), e.name)) # Apply cursor: skip past the cursor entry if cursor: From d9fe05cf9140548870c42ada09d02e356bc2c970 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Wed, 13 May 2026 14:27:09 -0400 Subject: [PATCH 05/11] feat: show notification in ui when folder is truncated --- .../components/ui/BrowsePage/FileTable.tsx | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/frontend/src/components/ui/BrowsePage/FileTable.tsx b/frontend/src/components/ui/BrowsePage/FileTable.tsx index 97b70a65..94d4ee5e 100644 --- a/frontend/src/components/ui/BrowsePage/FileTable.tsx +++ b/frontend/src/components/ui/BrowsePage/FileTable.tsx @@ -22,6 +22,10 @@ import { makeBrowseLink } from '@/utils/index'; import FgTooltip from '@/components/ui/widgets/FgTooltip'; import { FgStyledLink } from '@/components/ui/widgets/FgLink'; import { SortIcons } from '@/components/ui/Table/TableCard'; +import { + NotificationItem, + getNotificationStyles +} from '@/components/ui/Notifications/NotificationItem'; import { typeColumn, lastModifiedColumn, @@ -312,8 +316,26 @@ export default function Table({ navigate(link); }); + const isTruncated = fileQuery.data?.isTruncated ?? false; + return (
+ {isTruncated ? ( +
+ +
+ ) : null}
{table.getHeaderGroups().map(headerGroup => (
Date: Wed, 13 May 2026 15:56:08 -0400 Subject: [PATCH 06/11] refactor: pass configured truncation limit in file query response --- fileglancer/server.py | 1 + frontend/src/components/ui/BrowsePage/FileTable.tsx | 6 ++++-- frontend/src/queries/fileQueries.ts | 5 ++++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/fileglancer/server.py b/fileglancer/server.py index 691e59b3..03169f23 100644 --- a/fileglancer/server.py +++ b/fileglancer/server.py @@ -1407,6 +1407,7 @@ async def get_file_metadata(path_name: str, subpath: Optional[str] = Query(''), result["next_cursor"] = next_cursor result["total_count"] = total_count result["is_truncated"] = is_truncated + result["max_count"] = settings.max_directory_count else: files = list(filestore.yield_file_infos(subpath, current_user=username, session=session)) result["files"] = [json.loads(f.model_dump_json()) for f in files] diff --git a/frontend/src/components/ui/BrowsePage/FileTable.tsx b/frontend/src/components/ui/BrowsePage/FileTable.tsx index 94d4ee5e..5bbf07d7 100644 --- a/frontend/src/components/ui/BrowsePage/FileTable.tsx +++ b/frontend/src/components/ui/BrowsePage/FileTable.tsx @@ -317,6 +317,9 @@ export default function Table({ }); const isTruncated = fileQuery.data?.isTruncated ?? false; + const maxCount = fileQuery.data?.maxCount ?? null; + const truncationLimit = + maxCount?.toLocaleString() ?? 'the configured limit of'; return (
@@ -329,8 +332,7 @@ export default function Table({ id: 0, type: 'warning', title: 'Folder contents truncated', - message: - 'Only showing the first 10,000 items in this folder. Reorganize contents into sub-folders to see more.' + message: `This folder contains more than ${truncationLimit} items. Only ${truncationLimit} items are shown.` }} showDismissButton={false} /> diff --git a/frontend/src/queries/fileQueries.ts b/frontend/src/queries/fileQueries.ts index 8a6b0345..254d9e16 100644 --- a/frontend/src/queries/fileQueries.ts +++ b/frontend/src/queries/fileQueries.ts @@ -29,6 +29,7 @@ type FileBrowserPageResponse = { next_cursor?: string | null; total_count?: number | null; is_truncated?: boolean; + max_count?: number; }; export type FileQueryData = { @@ -39,6 +40,7 @@ export type FileQueryData = { hasMore: boolean; totalCount: number | null; isTruncated: boolean; + maxCount: number | null; }; // Query key factory for hierarchical cache management @@ -145,7 +147,8 @@ export default function useFileQuery( files: allFiles, hasMore: lastPage?.has_more ?? false, totalCount, - isTruncated: lastPage?.is_truncated ?? false + isTruncated: lastPage?.is_truncated ?? false, + maxCount: lastPage?.max_count ?? null }; }; From eded320939881513fe2e2577a94e04f12ca66bda Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 15 May 2026 10:06:29 -0400 Subject: [PATCH 07/11] fix: require max_count be passed to yield_file_infos_paginated --- fileglancer/filestore.py | 14 ++++++------ fileglancer/server.py | 3 ++- tests/test_pagination.py | 46 +++++++++++++++++++++++++--------------- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/fileglancer/filestore.py b/fileglancer/filestore.py index 8fb0dfeb..f14385b0 100644 --- a/fileglancer/filestore.py +++ b/fileglancer/filestore.py @@ -19,7 +19,6 @@ from loguru import logger from .database import find_fsp_from_absolute_path -from .settings import get_settings from .model import FileSharePath # Default buffer size for streaming file contents @@ -493,7 +492,7 @@ def check_is_binary(self, path: Optional[str] = None, sample_size: int = 4096) - def yield_file_infos_paginated(self, path: Optional[str] = None, current_user: str = None, session = None, limit: int = 200, cursor: Optional[str] = None, - max_count: Optional[int] = None) -> tuple[list[FileInfo], bool, Optional[str], int, bool]: + *, max_count: int) -> tuple[list[FileInfo], bool, Optional[str], int, bool]: """ Return a page of FileInfo objects for children of the given path. @@ -509,18 +508,17 @@ def yield_file_infos_paginated(self, path: Optional[str] = None, current_user: s cursor: Name of the last entry from the previous page. Entries after this name (in sort order) are returned. max_count: Maximum number of directory entries to read and sort. - Defaults to Settings.max_directory_count. Directories with more - entries are truncated at this limit; files beyond max_count are - not accessible via pagination. + Directories with more entries are truncated at this limit; + files beyond max_count are not accessible via pagination. + Required keyword arg — callers must pass the configured limit + (e.g. Settings.max_directory_count) so the advertised and + actual limits cannot drift. Returns: Tuple of (file_infos, has_more, next_cursor, total_count, is_truncated). total_count is capped at max_count. is_truncated is True when the directory has more entries than max_count. """ - if max_count is None: - max_count = get_settings().max_directory_count - full_path = self._check_path_in_root(path) # Compute user groups once for the entire listing diff --git a/fileglancer/server.py b/fileglancer/server.py index 03169f23..d29052ae 100644 --- a/fileglancer/server.py +++ b/fileglancer/server.py @@ -1400,7 +1400,8 @@ async def get_file_metadata(path_name: str, subpath: Optional[str] = Query(''), if limit is not None: files, has_more, next_cursor, total_count, is_truncated = filestore.yield_file_infos_paginated( subpath, current_user=username, session=session, - limit=limit, cursor=cursor + limit=limit, cursor=cursor, + max_count=settings.max_directory_count, ) result["files"] = [json.loads(f.model_dump_json()) for f in files] result["has_more"] = has_more diff --git a/tests/test_pagination.py b/tests/test_pagination.py index 48dfa38e..b40e4dc9 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -6,6 +6,10 @@ from fileglancer.filestore import Filestore from fileglancer.model import FileSharePath +# Matches the production default in Settings.max_directory_count; large enough +# that the small fixture directories used here never get truncated. +NO_TRUNCATION = 10000 + @pytest.fixture def pagination_dir(): @@ -38,7 +42,7 @@ class TestPaginatedListing: def test_first_page(self, pagination_store): """First page returns correct number of items with has_more=True.""" infos, has_more, next_cursor, total_count, is_truncated = ( - pagination_store.yield_file_infos_paginated(None, limit=5) + pagination_store.yield_file_infos_paginated(None, limit=5, max_count=NO_TRUNCATION) ) assert len(infos) == 5 assert has_more is True @@ -49,7 +53,7 @@ def test_first_page(self, pagination_store): def test_full_listing_no_pagination(self, pagination_store): """When limit >= total entries, has_more is False and next_cursor is None.""" infos, has_more, next_cursor, total_count, is_truncated = ( - pagination_store.yield_file_infos_paginated(None, limit=100) + pagination_store.yield_file_infos_paginated(None, limit=100, max_count=NO_TRUNCATION) ) assert len(infos) == 13 assert has_more is False @@ -60,10 +64,12 @@ def test_full_listing_no_pagination(self, pagination_store): def test_cursor_continuation(self, pagination_store): """Cursor returns the next page starting after the cursor entry.""" infos1, _, cursor1, _, _ = pagination_store.yield_file_infos_paginated( - None, limit=5 + None, limit=5, max_count=NO_TRUNCATION ) infos2, has_more2, cursor2, total2, is_truncated2 = ( - pagination_store.yield_file_infos_paginated(None, limit=5, cursor=cursor1) + pagination_store.yield_file_infos_paginated( + None, limit=5, cursor=cursor1, max_count=NO_TRUNCATION + ) ) # Second page should not overlap with first @@ -81,7 +87,9 @@ def test_all_pages_cover_all_entries(self, pagination_store): cursor = None while True: infos, has_more, cursor, _, _ = ( - pagination_store.yield_file_infos_paginated(None, limit=4, cursor=cursor) + pagination_store.yield_file_infos_paginated( + None, limit=4, cursor=cursor, max_count=NO_TRUNCATION + ) ) all_names.extend(fi.name for fi in infos) if not has_more: @@ -93,7 +101,7 @@ def test_all_pages_cover_all_entries(self, pagination_store): def test_dirs_sorted_before_files(self, pagination_store): """Directories appear before files in the listing.""" infos, _, _, _, _ = pagination_store.yield_file_infos_paginated( - None, limit=100 + None, limit=100, max_count=NO_TRUNCATION ) dir_indices = [i for i, fi in enumerate(infos) if fi.is_dir] file_indices = [i for i, fi in enumerate(infos) if not fi.is_dir] @@ -102,7 +110,7 @@ def test_dirs_sorted_before_files(self, pagination_store): def test_alphabetical_within_type(self, pagination_store): """Names are sorted alphabetically within dirs and within files.""" infos, _, _, _, _ = pagination_store.yield_file_infos_paginated( - None, limit=100 + None, limit=100, max_count=NO_TRUNCATION ) dir_names = [fi.name for fi in infos if fi.is_dir] file_names = [fi.name for fi in infos if not fi.is_dir] @@ -113,7 +121,7 @@ def test_deleted_cursor_fallback(self, pagination_dir, pagination_store): """When cursor entry no longer exists, listing starts from the beginning.""" infos, has_more, _, total, is_truncated = ( pagination_store.yield_file_infos_paginated( - None, limit=5, cursor="nonexistent_file" + None, limit=5, cursor="nonexistent_file", max_count=NO_TRUNCATION ) ) # Falls back to beginning @@ -128,11 +136,13 @@ def test_last_page_boundary(self, pagination_store): """Last page has has_more=False and next_cursor=None.""" # Get first 10 entries _, _, cursor, _, _ = pagination_store.yield_file_infos_paginated( - None, limit=10 + None, limit=10, max_count=NO_TRUNCATION ) # Get remaining 3 infos, has_more, next_cursor, _, _ = ( - pagination_store.yield_file_infos_paginated(None, limit=10, cursor=cursor) + pagination_store.yield_file_infos_paginated( + None, limit=10, cursor=cursor, max_count=NO_TRUNCATION + ) ) assert len(infos) == 3 assert has_more is False @@ -142,10 +152,12 @@ def test_exact_limit_boundary(self, pagination_store): """When entries remaining == limit, has_more is False.""" # 13 total, get first 10, then exactly 3 remain _, _, cursor, _, _ = pagination_store.yield_file_infos_paginated( - None, limit=10 + None, limit=10, max_count=NO_TRUNCATION ) infos, has_more, next_cursor, _, _ = ( - pagination_store.yield_file_infos_paginated(None, limit=3, cursor=cursor) + pagination_store.yield_file_infos_paginated( + None, limit=3, cursor=cursor, max_count=NO_TRUNCATION + ) ) assert len(infos) == 3 assert has_more is False @@ -154,7 +166,7 @@ def test_exact_limit_boundary(self, pagination_store): def test_limit_one(self, pagination_store): """Pagination works with limit=1.""" infos, has_more, cursor, total, is_truncated = ( - pagination_store.yield_file_infos_paginated(None, limit=1) + pagination_store.yield_file_infos_paginated(None, limit=1, max_count=NO_TRUNCATION) ) assert len(infos) == 1 assert has_more is True @@ -169,7 +181,7 @@ def test_empty_directory(self): fsp = FileSharePath(zone="test", name="test", mount_path=temp_dir) store = Filestore(fsp) infos, has_more, next_cursor, total, is_truncated = ( - store.yield_file_infos_paginated(None, limit=10) + store.yield_file_infos_paginated(None, limit=10, max_count=NO_TRUNCATION) ) assert infos == [] assert has_more is False @@ -188,7 +200,7 @@ def test_subdir_pagination(self, pagination_dir, pagination_store): f.write(f"sub {i}") infos, has_more, _, total, is_truncated = ( - pagination_store.yield_file_infos_paginated("dir_00", limit=3) + pagination_store.yield_file_infos_paginated("dir_00", limit=3, max_count=NO_TRUNCATION) ) assert len(infos) == 3 assert has_more is True @@ -210,9 +222,9 @@ def test_truncation(self, pagination_dir): def test_chroot_escape_raises(self, pagination_store): """Attempting to escape root raises ValueError.""" with pytest.raises(ValueError): - pagination_store.yield_file_infos_paginated("../") + pagination_store.yield_file_infos_paginated("../", max_count=NO_TRUNCATION) def test_nonexistent_path_raises(self, pagination_store): """Listing a nonexistent path raises FileNotFoundError.""" with pytest.raises((FileNotFoundError, PermissionError)): - pagination_store.yield_file_infos_paginated("nonexistent") + pagination_store.yield_file_infos_paginated("nonexistent", max_count=NO_TRUNCATION) From 9f44f032c3fbda428b6417fc5883b6324fa1e230 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 15 May 2026 10:16:11 -0400 Subject: [PATCH 08/11] fix: validate max_directory_count is greater than 0 --- fileglancer/settings.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fileglancer/settings.py b/fileglancer/settings.py index 0b6eaa2b..7fdfb1ef 100644 --- a/fileglancer/settings.py +++ b/fileglancer/settings.py @@ -131,6 +131,13 @@ def validate_external_proxy_url(cls, v): if v is None or (isinstance(v, str) and v.strip() == ''): raise ValueError("Add external_proxy_url to your config.yaml or FGC_EXTERNAL_PROXY_URL to your .env file") return v + + @field_validator('max_directory_count') + @classmethod + def validate_max_directory_count(cls, v: int) -> int: + if v <= 0: + raise ValueError('max_directory_count must be a positive integer') + return v @classmethod def settings_customise_sources( # noqa: PLR0913 From fde2d3bb82390e2622c252a0fd87c1a3da3171b9 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 15 May 2026 10:17:14 -0400 Subject: [PATCH 09/11] fix(test): truncated dirs are sorted dir-first then alphabetical --- tests/test_pagination.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/test_pagination.py b/tests/test_pagination.py index b40e4dc9..beb20c16 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -208,7 +208,7 @@ def test_subdir_pagination(self, pagination_dir, pagination_store): assert is_truncated is False def test_truncation(self, pagination_dir): - """is_truncated is True and entries are in filesystem order (not sorted) when truncated.""" + """is_truncated is True and the truncated subset is sorted (dirs first, then alpha).""" fsp = FileSharePath(zone="test", name="test", mount_path=pagination_dir) store = Filestore(fsp) # max_count=5 on a 13-entry directory @@ -218,6 +218,15 @@ def test_truncation(self, pagination_dir): assert total_count == 5 assert is_truncated is True assert len(infos) == 5 # limit=10 > max_count=5, so all 5 returned + # Truncated subset must still be sorted: dirs first, then alphabetical. + # The fixture has 3 dirs (dir_00..dir_02) which all sort before any file, + # so a sorted truncated page of 5 starts with all 3 dirs. + names = [fi.name for fi in infos] + assert names[:3] == ["dir_00", "dir_01", "dir_02"] + dir_names = [fi.name for fi in infos if fi.is_dir] + file_names = [fi.name for fi in infos if not fi.is_dir] + assert dir_names == sorted(dir_names) + assert file_names == sorted(file_names) def test_chroot_escape_raises(self, pagination_store): """Attempting to escape root raises ValueError.""" From 0c2f0a12b1b305a5b0600e0d7f02ae1071f79ef3 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 15 May 2026 10:17:54 -0400 Subject: [PATCH 10/11] fix: add language for sorting disabled in file table due to truncation --- frontend/src/components/ui/BrowsePage/FileTable.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/ui/BrowsePage/FileTable.tsx b/frontend/src/components/ui/BrowsePage/FileTable.tsx index 5bbf07d7..879c55d1 100644 --- a/frontend/src/components/ui/BrowsePage/FileTable.tsx +++ b/frontend/src/components/ui/BrowsePage/FileTable.tsx @@ -100,7 +100,14 @@ export default function Table({ parent = parent.parentElement; } }, []); - const sortingEnabled = !hasNextPage && !fileQuery.data?.isTruncated; + const sortingDisabledByTruncation = Boolean(fileQuery.data?.isTruncated); + const sortingDisabledByPagination = + !sortingDisabledByTruncation && Boolean(hasNextPage); + const sortingEnabled = + !sortingDisabledByTruncation && !sortingDisabledByPagination; + const sortingDisabledTooltip = sortingDisabledByTruncation + ? 'Sort unavailable: folder is too large and contents are truncated' + : 'Sort unavailable until all files are loaded'; const selectedFileNames = useMemo( () => new Set(fileBrowserState.selectedFiles.map(file => file.name)), @@ -373,7 +380,7 @@ export default function Table({ ) : ( ) From 58fcb819ab965082105e040035379b01648a06d9 Mon Sep 17 00:00:00 2001 From: Allison Truhlar Date: Fri, 15 May 2026 10:29:17 -0400 Subject: [PATCH 11/11] fix(tests): assert sort property, not specific entries, in test_truncation The truncated subset contains whatever max_count+1 entries os.scandir returned first; scandir order is filesystem-dependent, so the test's assertion that dirs appear first was a coincidence that held locally but not on CI. Assert what yield_file_infos_paginated actually guarantees: the returned subset is sorted, and repeat calls are stable. --- tests/test_pagination.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/tests/test_pagination.py b/tests/test_pagination.py index beb20c16..08f0d327 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -208,7 +208,7 @@ def test_subdir_pagination(self, pagination_dir, pagination_store): assert is_truncated is False def test_truncation(self, pagination_dir): - """is_truncated is True and the truncated subset is sorted (dirs first, then alpha).""" + """is_truncated is True, and the truncated subset is sorted so cursor pagination is stable.""" fsp = FileSharePath(zone="test", name="test", mount_path=pagination_dir) store = Filestore(fsp) # max_count=5 on a 13-entry directory @@ -218,15 +218,22 @@ def test_truncation(self, pagination_dir): assert total_count == 5 assert is_truncated is True assert len(infos) == 5 # limit=10 > max_count=5, so all 5 returned - # Truncated subset must still be sorted: dirs first, then alphabetical. - # The fixture has 3 dirs (dir_00..dir_02) which all sort before any file, - # so a sorted truncated page of 5 starts with all 3 dirs. - names = [fi.name for fi in infos] - assert names[:3] == ["dir_00", "dir_01", "dir_02"] - dir_names = [fi.name for fi in infos if fi.is_dir] - file_names = [fi.name for fi in infos if not fi.is_dir] - assert dir_names == sorted(dir_names) - assert file_names == sorted(file_names) + + # The truncated subset is whatever max_count+1 entries os.scandir + # happened to return first — we cannot predict which entries those + # are because scandir order is filesystem-dependent. What we *can* + # guarantee is that the returned subset is sorted (dirs-first, then + # name), so cursor-based pagination is stable across calls. + sort_key = lambda fi: (not fi.is_dir, fi.name) + assert [fi.name for fi in infos] == [fi.name for fi in sorted(infos, key=sort_key)] + + # Stability: a second call with the same arguments returns the same + # entries in the same order (the property the e368d2c3 sort was + # added to guarantee for cursor-based pagination). + infos2, _, _, _, _ = store.yield_file_infos_paginated( + None, limit=10, max_count=5 + ) + assert [fi.name for fi in infos2] == [fi.name for fi in infos] def test_chroot_escape_raises(self, pagination_store): """Attempting to escape root raises ValueError."""