diff --git a/docs/guide/search.md b/docs/guide/search.md index d86ad1d0..023816fe 100644 --- a/docs/guide/search.md +++ b/docs/guide/search.md @@ -1,69 +1,106 @@ # Search -OxiCloud provides authenticated file and folder search with simple query parameters, advanced JSON criteria, pagination, recursive traversal, and in-memory result caching. +OxiCloud provides authenticated file and folder search with a +cursor-paginated response, filter/sort query parameters, recursive +traversal, and in-memory result caching. ## Endpoints | Method | Endpoint | Description | | --- | --- | --- | -| `GET` | `/api/search/` | Simple search using query parameters | -| `POST` | `/api/search/advanced` | Advanced search with a JSON body | +| `GET` | `/api/search` | Cursor-paginated search | | `GET` | `/api/search/suggest` | Lightweight autocomplete suggestions | | `DELETE` | `/api/admin/search/cache` | Flush the shared search results cache (admin only) | All search endpoints require authentication. The cache flush is additionally restricted to administrators — see [Result Caching](#result-caching). -## Simple Search Parameters +## Query Parameters | Parameter | Description | | --- | --- | -| `query` | Text to search in file and folder names | -| `type` | Comma-separated file extensions | -| `created_after` / `created_before` | Filter by creation time | -| `modified_after` / `modified_before` | Filter by modification time | -| `min_size` / `max_size` | Filter by file size in bytes | +| `query` | Text to search in file/folder names (and file content when the Tantivy index is enabled) | +| `type` | Comma-separated file extensions (files only) | | `folder_id` | Restrict search scope to one folder | | `recursive` | Search subfolders, defaults to `true` | -| `limit` | Maximum results, defaults to `100` | -| `offset` | Pagination offset | -| `sort_by` | `relevance`, `name`, `name_desc`, `date`, `date_desc`, `size`, or `size_desc` | +| `created_after` / `created_before` | Filter by creation time (unix seconds) | +| `modified_after` / `modified_before` | Filter by modification time | +| `min_size` / `max_size` | Filter by file size in bytes | +| `resource_types` | Comma-separated: `file`, `folder` (both by default) | +| `order_by` | `relevance` (default), `name`, `name_desc`, `date`, `date_desc`, `size`, `size_desc` | +| `reverse` | Reverse the sort order | +| `limit` | Page size (1–200, default 50) | +| `cursor` | Opaque cursor returned by the previous page | + +## Response Shape + +`/api/search` returns the same envelope as every other `/*/resources` +listing (folders, favorites, recent, trash, shared) so a single +client component can render all of them: + +```json +{ + "items": [ + { + "resource_type": "file", + "resource": { "id": "…", "name": "report.pdf", "size": 12345, "…": "…" }, + "meta": { + "score": 0.82, + "snippet": "…quarterly report…", + "via": "content" + } + }, + { + "resource_type": "folder", + "resource": { "id": "…", "name": "Reports", "…": "…" }, + "meta": { "score": 0.31, "via": "name" } + } + ], + "next_cursor": "eyJvZmZzZXQiOjUwLCJvcmRlcl9ieSI6InJlbGV2YW5jZSJ9", + "query_time_ms": 12, + "total": 137 +} +``` + +- `resource_type`: `"file"` or `"folder"` — tells the client which + variant of `resource` to render. +- `resource`: the same `FileDto` / `FolderDto` shape any other + endpoint would emit. +- `meta.score`: relevance in `[0, 1]`. +- `meta.snippet`: optional HTML-safe excerpt from the content index + (present only when the match came from file content). +- `meta.via`: `"name"`, `"content"`, or `"path"` — where the match + fired. +- `next_cursor`: opaque; present iff another page exists. Pass it + verbatim as `?cursor=…` for the next call. +- `total`: integer, approximate — reflects the caller-visible match + count (permission-filtered). Omitted when unknown; never leaks a + count for rows the caller cannot see. ### Example ```bash curl -H "Authorization: Bearer $TOKEN" \ - "https://oxicloud.example.com/api/search/?query=report&type=pdf,docx&recursive=true&limit=20" -``` - -## Advanced Search - -```json -{ - "name_contains": "report", - "file_types": ["pdf", "docx"], - "min_size": 1024, - "folder_id": "folder-uuid", - "recursive": true, - "limit": 50, - "offset": 0 -} + "https://oxicloud.example.com/api/search?query=report&type=pdf,docx&limit=20" ``` ## Suggestions -Use `/api/search/suggest?query=rep&limit=10` for quick autocomplete-style results. Suggestions can also be scoped to a folder with `folder_id`. +Use `/api/search/suggest?query=rep&limit=10` for quick +autocomplete-style results. Suggestions can also be scoped to a +folder with `folder_id`. ## Result Caching -Search results are cached in memory using the search criteria and user ID as the cache key. +Search results are cached in memory using the search criteria and +user ID as the cache key. - Cache TTL: 5 minutes - Max entries: 1000 - Manual invalidation: `DELETE /api/admin/search/cache` — admin-only. The endpoint calls `invalidate_all()` on the shared moka cache, so - one call cold-starts every subsequent search for every tenant; it's - an operator debug lever, not a per-user affordance. + one call cold-starts every subsequent search for every tenant; + it's an operator debug lever, not a per-user affordance. ## Feature Flag diff --git a/examples/bench_search_cache_mem.rs b/examples/bench_search_cache_mem.rs index d454e0ec..33d846da 100644 --- a/examples/bench_search_cache_mem.rs +++ b/examples/bench_search_cache_mem.rs @@ -145,6 +145,11 @@ fn synth_entry(idx: u64) -> Arc { blob_hash: pseudo_hex(&mut rng, 64), snippet: content_hit.then(|| SNIPPET.to_string()), match_source: Some(match_source.to_string()), + etag: pseudo_hex(&mut rng, 16), + created_by: None, + updated_by: None, + is_favorite: false, + is_shared: false, }); } diff --git a/src/application/dtos/search_dto.rs b/src/application/dtos/search_dto.rs index 0dc82a12..7cd49fe0 100644 --- a/src/application/dtos/search_dto.rs +++ b/src/application/dtos/search_dto.rs @@ -1,6 +1,11 @@ use serde::{Deserialize, Serialize}; +use std::collections::HashMap; use std::sync::Arc; -use utoipa::ToSchema; +use utoipa::{IntoParams, ToSchema}; +use uuid::Uuid; + +use crate::application::dtos::cursor::PageCursor; +use crate::application::dtos::grant_dto::{ResourceContentDto, ResourceTypeDto}; /** * Data Transfer Object for file search criteria. @@ -99,7 +104,16 @@ impl Default for SearchCriteriaDto { } } -/// A file search result enriched with server-computed metadata +/// A file search result enriched with server-computed metadata. +/// +/// Phase 1-plus extension (AuthZ-adjacent audit follow-up, 2026-07-26): +/// carries `etag`, `created_by`, `updated_by`, `is_favorite`, `is_shared` +/// through from `FileDto`. Pre-fix these fields were dropped at `enrich_file` +/// time, so the wire-normalised `SearchResourcesDto` handler couldn't +/// reconstruct a full `FileDto` for its `resource` slot — every result +/// looked unfavorited / unshared, and provenance was blank. The extra +/// columns come from `file_blob_read_repository.rs::search_files_paginated` +/// (SELECT'd inline, EXISTS subqueries for the caller-scoped booleans). #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] pub struct SearchFileResultDto { /// File ID @@ -148,9 +162,36 @@ pub struct SearchFileResultDto { /// "content" (discovered via the full-text content index). #[serde(default, skip_serializing_if = "Option::is_none")] pub match_source: Option, + /// HTTP ETag — derived from `blob_hash + modified_at`. Duplicates + /// `FileDto::etag` so the wire handler can hand a client the same + /// token for `If-Match` / `If-None-Match` conditional requests on + /// search results as it would on a folder listing. + #[serde(default)] + pub etag: String, + /// §14 provenance — user that originally created this file. `None` + /// when the referenced user has been deleted or for legacy rows. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub created_by: Option, + /// §14 provenance — user that performed the most recent mutation. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub updated_by: Option, + /// Caller-scoped: `true` when the requesting user has favorited + /// this file. Populated by an EXISTS subquery in the search SQL — + /// the search repo carries it back as a per-row bool that the + /// service plumbs into this DTO. + #[serde(default)] + pub is_favorite: bool, + /// Resource-scoped: `true` when the file has ANY explicit role-grant. + /// Populated by the sibling EXISTS on `storage.role_grants`. + #[serde(default)] + pub is_shared: bool, } -/// A folder search result enriched with server-computed metadata +/// A folder search result enriched with server-computed metadata. +/// +/// See `SearchFileResultDto` for the Phase 1-plus rationale — same +/// story: the six caller/provenance fields are carried through so the +/// wire-normalised handler can hand the frontend a complete `FolderDto`. #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] pub struct SearchFolderResultDto { /// Folder ID @@ -164,7 +205,7 @@ pub struct SearchFolderResultDto { /// Drive that owns this folder. Same column as `storage.folders.drive_id`, /// carried through so downstream callers (e.g. the NC search REPORT /// handler) can populate `FolderDto::drive_id` without a fallback sentinel. - pub drive_id: uuid::Uuid, + pub drive_id: Uuid, /// Creation timestamp pub created_at: u64, /// Last modification timestamp @@ -173,6 +214,25 @@ pub struct SearchFolderResultDto { pub is_root: bool, /// Relevance score (0-100) computed server-side pub relevance_score: u32, + /// HTTP ETag — folders derive theirs from the tree-etag propagator. + /// Duplicating it here keeps the wire handler's `FolderDto` + /// reconstruction complete. + #[serde(default)] + pub etag: String, + /// §14 provenance — creator user id. `None` for legacy folders. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub created_by: Option, + /// §14 provenance — last-mutator user id. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub updated_by: Option, + /// Caller-scoped: `true` when the requesting user has favorited + /// this folder. EXISTS on `auth.user_favorites`. + #[serde(default)] + pub is_favorite: bool, + /// Resource-scoped: `true` when the folder has any explicit + /// role-grant. EXISTS on `storage.role_grants`. + #[serde(default)] + pub is_shared: bool, } /** @@ -182,7 +242,7 @@ pub struct SearchFolderResultDto { * both files and folders that match the search criteria, along with pagination * information and server-computed metadata. */ -#[derive(Debug, Serialize, Deserialize, ToSchema)] +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] pub struct SearchResultsDto { /// Files matching the search criteria (enriched with metadata) pub files: Vec, @@ -252,6 +312,361 @@ impl SearchResultsDto { } } +// ═══════════════════════════════════════════════════════════════════════════ +// New wire shape — normalised to the `/*/resources` envelope +// (`items[] { resource_type, resource, meta }` + `next_cursor` + optional +// `total`/`query_time_ms`). Phase 1-plus: internal service still speaks +// `SearchCriteriaDto`/`SearchResultsDto`; the REST handler translates. +// ═══════════════════════════════════════════════════════════════════════════ + +/// Query parameters for `GET /api/search`. +/// +/// Mirrors `FolderResourcesQuery` for the shared axes (`limit`, `cursor`, +/// `order_by`, `resource_types`, `reverse`) then adds search-specific +/// filters (`query`, `folder_id`, `recursive`, `file_types`, +/// `created_after`/`before`, `modified_after`/`before`, `min_size`/ +/// `max_size`). `serde_urlencoded` doesn't support `#[serde(flatten)]`, +/// so the paging fields are inlined rather than composed from +/// `CursorQuery`. +#[derive(Debug, Deserialize, IntoParams)] +pub struct SearchResourcesQuery { + /// Search phrase (matched against name; optionally content when the + /// full-text index is enabled). Absent = "match everything," so + /// callers can page through with just a folder scope + filters. + pub query: Option, + + /// Maximum items per page (1–200, default 50). + #[serde(default = "SearchResourcesQuery::default_limit")] + pub limit: u32, + + /// Opaque cursor from a previous response. Absent = first page. + /// Encodes the current offset — Phase 1-plus still uses the + /// existing offset-based service internals under the hood. + pub cursor: Option, + + /// Sort dimension. Supported: `"relevance"` (default), `"name"`, + /// `"name_desc"`, `"date"` (= `modified_at`), `"date_desc"`, + /// `"size"`, `"size_desc"`. Names match the pre-normalisation + /// values `SearchCriteriaDto.sort_by` accepted so cached results + /// remain reachable. + pub order_by: Option, + + /// Comma-separated resource types to include, e.g. `"file,folder"`. + /// Absent = both. Matches the `FolderResourcesQuery` idiom. + pub resource_types: Option, + + /// Reverse the sort order. Default `false`. + #[serde(default)] + pub reverse: bool, + + /// Comma-separated file extensions filter, e.g. `"pdf,docx"`. + #[serde(rename = "type")] + pub type_filter: Option, + + /// Restrict search to this folder. + pub folder_id: Option, + + /// Recursive traversal below `folder_id`. Default `true`. + #[serde(default = "SearchResourcesQuery::default_recursive")] + pub recursive: bool, + + /// Minimum creation timestamp (seconds since epoch). + pub created_after: Option, + /// Maximum creation timestamp (seconds since epoch). + pub created_before: Option, + /// Minimum modification timestamp (seconds since epoch). + pub modified_after: Option, + /// Maximum modification timestamp (seconds since epoch). + pub modified_before: Option, + /// Minimum file size in bytes. + pub min_size: Option, + /// Maximum file size in bytes. + pub max_size: Option, +} + +impl SearchResourcesQuery { + pub fn default_limit() -> u32 { + 50 + } + pub fn default_recursive() -> bool { + true + } + pub fn limit_clamped(&self) -> usize { + self.limit.clamp(1, 200) as usize + } + pub fn decode_cursor(&self) -> Option { + self.cursor + .as_deref() + .and_then(SearchResourceCursor::decode) + } + /// Convert to the internal `SearchCriteriaDto` the service still + /// consumes. `limit` / `offset` come from the decoded cursor (or the + /// query's `limit` on the first page). Sort names pass through + /// verbatim — the service's `sort_by` matcher accepts the same set. + pub fn to_criteria(&self) -> SearchCriteriaDto { + let offset = self.decode_cursor().map(|c| c.offset).unwrap_or(0); + let file_types = self.type_filter.as_deref().map(|s| { + s.split(',') + .map(|t| t.trim().to_string()) + .filter(|t| !t.is_empty()) + .collect() + }); + SearchCriteriaDto { + name_contains: self.query.clone(), + file_types, + created_after: self.created_after, + created_before: self.created_before, + modified_after: self.modified_after, + modified_before: self.modified_before, + min_size: self.min_size, + max_size: self.max_size, + folder_id: self.folder_id.clone(), + recursive: self.recursive, + limit: self.limit_clamped(), + offset, + sort_by: self.order_by.clone().unwrap_or_else(default_sort_by), + } + } + /// Which resource kinds to include. `None` = both. Anything else + /// selects the intersection. + pub fn include_files(&self) -> bool { + match self.resource_types.as_deref() { + None => true, + Some(s) => s.split(',').any(|t| t.trim() == "file"), + } + } + pub fn include_folders(&self) -> bool { + match self.resource_types.as_deref() { + None => true, + Some(s) => s.split(',').any(|t| t.trim() == "folder"), + } + } +} + +/// Opaque cursor for `/api/search`. Encodes the offset the underlying +/// service still uses, plus the sort dimension so a page fetched with +/// a different `order_by` than the previous one cannot silently drift +/// into a broken keyset. Phase 2 (service rewrite) would replace this +/// with a true keyset cursor over `(sort_key, id)`. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SearchResourceCursor { + pub offset: usize, + pub order_by: String, +} +impl PageCursor for SearchResourceCursor {} + +/// Search-specific per-item metadata (relevance score, snippet, hit +/// source). Sits inline on each `SearchResourceItem` so consumers get +/// data locality — no keyed lookup. `ResourceList` ignores the field. +/// +/// Wire keys are shortened (`meta.score`, `meta.via`) vs the internal +/// `SearchFileResultDto` field names (`relevance_score`, `match_source`) +/// to keep the envelope compact on large result pages. +#[derive(Debug, Serialize, ToSchema)] +pub struct SearchMeta { + /// Relevance 0-100. Higher = better match. + pub score: u32, + /// Plain-text fragment around the first content-index hit. Absent + /// for name-only matches and for folder results. + #[serde(skip_serializing_if = "Option::is_none")] + pub snippet: Option, + /// Where the hit came from: `"name"` or `"content"`. Absent when the + /// origin is ambiguous (empty query → everything matches). + #[serde(skip_serializing_if = "Option::is_none")] + pub via: Option, +} + +/// One search result — same `resource_type + resource` shape as the +/// `/*/resources` envelopes so `ResourceList` consumes it as-is, plus +/// the inline `meta` for search-specific enrichment. +#[derive(Debug, Serialize, ToSchema)] +pub struct SearchResourceItem { + pub resource_type: ResourceTypeDto, + /// Full resource details (untagged: `FileDto | FolderDto | DriveDto`). + /// Shape determined by `resource_type`. + pub resource: ResourceContentDto, + /// Search-specific metadata for this row. + pub meta: SearchMeta, +} + +/// Response envelope for `GET /api/search` — cursor-paginated + search +/// metadata. `total` is an approximate caller-visible count (permission- +/// filtered) when the service can compute it cheaply, absent otherwise — +/// matches sibling envelope endpoints, which all serialise counts as +/// integers (see `app_password_dto`, `plugin_dto`, `pagination`). +#[derive(Debug, Serialize, ToSchema)] +pub struct SearchResourcesDto { + pub items: Vec, + /// Opaque cursor for the next page. Absent on the last page. + #[serde(skip_serializing_if = "Option::is_none")] + pub next_cursor: Option, + /// Server-side query time. UI shows "Found N in Xms" and admins use + /// it as a health signal. + pub query_time_ms: u64, + /// Approximate caller-visible total match count. Never leaks a count + /// for rows the caller cannot see. Omitted when unknown. + #[serde(skip_serializing_if = "Option::is_none")] + pub total: Option, +} + +impl SearchResourcesDto { + /// Build the envelope from the service's existing offset-paginated + /// result plus the request's cursor position. The service returns + /// `SearchResultsDto` with `total_count` + `has_more` derived from + /// COUNT(*) OVER(); we translate: + /// - `has_more` → derive `next_cursor` (encoding `offset + returned`). + /// - `total_count` → pass through as `Some(42)` when known, else `None`. + /// + /// Ownership: consumes the service result so the enriched DTOs move + /// into the `resource` slot without cloning. + pub fn from_service_result( + results: crate::application::dtos::search_dto::SearchResultsDto, + query: &SearchResourcesQuery, + ) -> Self { + let order_by = query.order_by.clone().unwrap_or_else(default_sort_by); + let current_offset = query.decode_cursor().map(|c| c.offset).unwrap_or(0); + let returned = results.files.len() + results.folders.len(); + + let next_cursor = if results.has_more { + Some( + SearchResourceCursor { + offset: current_offset + returned, + order_by: order_by.clone(), + } + .encode(), + ) + } else { + None + }; + + let total = results.total_count; + + // Build items in an order the UI expects: folders first (like the + // legacy split-shape) unless a specific sort is requested. When + // ordering by relevance / date / size the caller almost always + // wants interleaved output; when ordering by name the folders- + // first convention matches file managers. Splitting the choice + // by sort dimension keeps folder browsing intuitive. + let mut items: Vec = Vec::with_capacity(returned); + let query_lower = query + .query + .as_deref() + .map(|s| s.to_lowercase()) + .unwrap_or_default(); + let folders_first = matches!(order_by.as_str(), "name" | "name_desc"); + + if folders_first { + append_folders(&mut items, results.folders); + append_files(&mut items, results.files, &query_lower); + } else { + // Interleave by relevance_score (or the natural service order for + // date/size — the service already returns rows in the requested + // dimension, but folders and files come as two separate arrays + // that we merge here by score for `relevance`, or just append + // for size/date since the two arrays are individually ordered. + append_folders(&mut items, results.folders); + append_files(&mut items, results.files, &query_lower); + if order_by == "relevance" { + items.sort_by_key(|item| std::cmp::Reverse(item.meta.score)); + } + } + + Self { + items, + next_cursor, + query_time_ms: results.query_time_ms, + total, + } + } +} + +fn append_files( + items: &mut Vec, + files: Vec, + _query_lower: &str, +) { + for f in files { + let meta = SearchMeta { + score: f.relevance_score, + snippet: f.snippet.clone(), + via: f.match_source.clone(), + }; + // Reconstruct FileDto from the enriched search result. `size_formatted` + // and display fields were already computed by `enrich_file`; the + // Phase 1-plus extensions (etag / created_by / updated_by / + // is_favorite / is_shared) carry through so the DTO is complete. + let file_dto = crate::application::dtos::file_dto::FileDto { + id: f.id, + name: f.name, + path: f.path, + size: f.size, + mime_type: f.mime_type, + folder_id: f.folder_id, + created_at: f.created_at, + modified_at: f.modified_at, + icon_class: f.icon_class, + icon_special_class: f.icon_special_class, + category: f.category, + size_formatted: f.size_formatted, + content_hash: f.blob_hash, + etag: f.etag, + created_by: f.created_by, + updated_by: f.updated_by, + is_favorite: f.is_favorite, + is_shared: f.is_shared, + sort_date: None, + }; + items.push(SearchResourceItem { + resource_type: ResourceTypeDto::File, + resource: ResourceContentDto::File(file_dto), + meta, + }); + } +} + +fn append_folders(items: &mut Vec, folders: Vec) { + for f in folders { + let meta = SearchMeta { + score: f.relevance_score, + snippet: None, + via: None, + }; + let folder_dto = crate::application::dtos::folder_dto::FolderDto { + id: f.id.clone(), + name: f.name, + path: f.path, + parent_id: f.parent_id, + drive_id: f.drive_id, + created_at: f.created_at, + modified_at: f.modified_at, + is_root: f.is_root, + // Folders carry closed-set display fields — always the + // same three static strings. Cheap to build via `Arc::from` + // (interning-worthy but not on the search hot path). + icon_class: Arc::from("fas fa-folder"), + icon_special_class: Arc::from("folder-icon"), + category: Arc::from("Folder"), + etag: f.etag, + created_by: f.created_by, + updated_by: f.updated_by, + is_favorite: f.is_favorite, + is_shared: f.is_shared, + }; + items.push(SearchResourceItem { + resource_type: ResourceTypeDto::Folder, + resource: ResourceContentDto::Folder(folder_dto), + meta, + }); + } +} + +// `search_meta` map form was explored earlier and rejected in favour of +// inline `meta` per item (Ed 2026-07-26): data locality wins, no +// keyed-lookup step for consumers, matches the extensibility other +// `/*/resources` endpoints will want later. +#[allow(dead_code)] +fn _keep_hashmap_import_alive_for_future(_: HashMap) {} + /// DTO for search suggestion results (quick prefix search) #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] pub struct SearchSuggestionsDto { diff --git a/src/application/ports/storage_ports.rs b/src/application/ports/storage_ports.rs index b2d4dcbc..697fad68 100644 --- a/src/application/ports/storage_ports.rs +++ b/src/application/ports/storage_ports.rs @@ -160,13 +160,22 @@ pub trait FileReadPort: Send + Sync + 'static { /// are expanded inline via `storage.caller_group_ids($caller)`. /// /// # Returns - /// A tuple of (files, total_count) where files are paginated and filtered + /// A tuple `(files, caller_flags, total_count)`: + /// - `files`: the paginated + filtered file rows. + /// - `caller_flags`: parallel `Vec<(is_favorite, is_shared)>` aligned + /// 1:1 with `files` by index. Populated in-SQL via per-row EXISTS + /// subqueries on `auth.user_favorites` and `storage.role_grants` so + /// the caller's SPA can render badges without a follow-up round-trip + /// (same pattern the photos-timeline listing uses). Kept as a + /// parallel vec rather than folded into `File` so the domain + /// entity stays caller-agnostic. + /// - `total_count`: `COUNT(*) OVER()` total for pagination. async fn search_files_paginated( &self, folder_id: Option<&str>, criteria: &SearchCriteriaDto, caller_id: Uuid, - ) -> Result<(Vec, usize), DomainError>; + ) -> Result<(Vec, Vec<(bool, bool)>, usize), DomainError>; /// Search files recursively in a folder subtree using ltree. /// @@ -177,13 +186,14 @@ pub trait FileReadPort: Send + Sync + 'static { /// Post-PR-B: scoped by drive-membership grants (same semantics as /// `search_files_paginated`), not by `files.user_id`. /// - /// Returns a tuple of (matching files, total count for pagination). + /// Returns the same shape as [`Self::search_files_paginated`]: + /// `(files, caller_flags, total_count)`. async fn search_files_in_subtree( &self, root_folder_id: Option<&str>, criteria: &SearchCriteriaDto, caller_id: Uuid, - ) -> Result<(Vec, usize), DomainError> { + ) -> Result<(Vec, Vec<(bool, bool)>, usize), DomainError> { // Default: delegate to paginated search (non-recursive fallback) self.search_files_paginated(root_folder_id, criteria, caller_id) .await diff --git a/src/application/services/search_service.rs b/src/application/services/search_service.rs index 0163fca6..e322d9f3 100644 --- a/src/application/services/search_service.rs +++ b/src/application/services/search_service.rs @@ -304,6 +304,16 @@ impl SearchService { blob_hash: file.content_hash, snippet: None, match_source: (!query_lower.is_empty() && relevance > 0).then(|| "name".to_string()), + // Phase 1-plus: carry the FileDto fields the old enrich + // shape dropped. Populates the normalised + // `SearchResourcesDto` items with a complete `FileDto` so + // the frontend `ResourceList` renders favorites / share + // badges / provenance consistently with other listings. + etag: file.etag, + created_by: file.created_by, + updated_by: file.updated_by, + is_favorite: file.is_favorite, + is_shared: file.is_shared, } } @@ -329,6 +339,13 @@ impl SearchService { modified_at: folder.modified_at, is_root: folder.is_root, relevance_score: relevance, + // Phase 1-plus (see sibling `enrich_file`): carry the + // FolderDto fields the old enrich shape dropped. + etag: folder.etag, + created_by: folder.created_by, + updated_by: folder.updated_by, + is_favorite: folder.is_favorite, + is_shared: folder.is_shared, } } @@ -651,18 +668,12 @@ impl SearchUseCase for SearchService { // For non-recursive searches, use efficient database-level pagination // This avoids loading all files into memory if !criteria.recursive { - // The content-index lookup (drive resolve + Tantivy + - // ReBAC batch), the file page and the folder query are - // mutually independent — overlap them so the search pays - // ~max() instead of the serial sum (`suggest_with_perms` - // already used this shape; ROUND10 brought it here). - let (content_hits, files_page, folders_res) = tokio::join!( + // Same folders-first sequencing as the recursive branch + // below (see the block comment there for the rationale + // — SQL applies file offset+limit, so folder_count has + // to be known before the file query is issued). + let (content_hits, folders_res) = tokio::join!( self.lookup_content_hits(&criteria, user_id), - self.file_repository.search_files_paginated( - criteria.folder_id.as_deref(), - &criteria, - user_id, - ), self.folder_repository.search_folders( criteria.folder_id.as_deref(), criteria.name_contains.as_deref(), @@ -670,20 +681,57 @@ impl SearchUseCase for SearchService { false, ), ); - let (files, total_file_count) = files_page?; - let folders = folders_res?; + let (folders, folder_flags) = folders_res?; + + let folder_count = folders.len(); + let folders_before_page = criteria.offset.min(folder_count); + let folders_on_page = (folder_count - folders_before_page).min(criteria.limit); + let file_offset = criteria.offset - folders_before_page; + let file_limit_needed = criteria.limit - folders_on_page; + let file_limit_probe = file_limit_needed.max(1); + + let mut file_criteria = criteria.clone(); + file_criteria.offset = file_offset; + file_criteria.limit = file_limit_probe; + + let (files, file_flags, total_file_count) = self + .file_repository + .search_files_paginated( + criteria.folder_id.as_deref(), + &file_criteria, + user_id, + ) + .await?; // Convert to DTOs and enrich with metadata — one fused // pass, no intermediate Vec materialization. + // `file_flags` is aligned 1:1 with `files` (in-SQL + // per-row EXISTS on favorites + role_grants) so a + // simple parallel zip plumbs the caller-scoped + // booleans onto the FileDto before enrichment. let mut enriched_files: Vec = files .into_iter() - .map(|f| Self::enrich_file(FileDto::from(f), &query_lower)) + .zip(file_flags) + .map(|(f, (is_fav, is_shr))| { + let mut dto = FileDto::from(f); + dto.is_favorite = is_fav; + dto.is_shared = is_shr; + Self::enrich_file(dto, &query_lower) + }) .collect(); - // For folders, apply sorting and pagination in memory (usually fewer folders) + // For folders, apply sorting and pagination in memory (usually fewer folders). + // Same parallel-zip shape as the file branch above — + // `folder_flags` is aligned 1:1 by index. let mut enriched_folders: Vec = folders .into_iter() - .map(|f| Self::enrich_folder(FolderDto::from(f), &query_lower)) + .zip(folder_flags) + .map(|(f, (is_fav, is_shr))| { + let mut dto = FolderDto::from(f); + dto.is_favorite = is_fav; + dto.is_shared = is_shr; + Self::enrich_folder(dto, &query_lower) + }) .collect(); // Sort folders (cached_key avoids O(N log N) temporary String allocations) @@ -705,39 +753,35 @@ impl SearchUseCase for SearchService { } } - // Blend in content-discovered files before the pagination math. - let added = self + // Blend in content-discovered files, then truncate to + // the exact page size — see the recursive branch's + // block comment for why the truncate is required. + // + // Note: `total_count` is intentionally the pure SQL + // name-match count (plus folders) — NOT inflated by + // `added` content-hit rows. `added` counts hits that + // aren't already in the *current* SQL slice, which is + // per-page (different SQL rows on each page produce + // different dedup outcomes and a different `added`). + // Including it made the client-visible `total` + // flicker as the user paginated (2026-07-26 report: + // 4184 → 4186 across scope + page toggles for a + // stable dataset). Content-hits still bubble into + // each page's `items`; they just don't move the + // grand total. + let _added = self .merge_content_hits(content_hits, &mut enriched_files, &criteria, user_id) .await?; - let total_file_count = total_file_count + added; + enriched_files.truncate(file_limit_needed); - let folder_count = enriched_folders.len(); let total_count = total_file_count + folder_count; - // Combine and paginate (folders first, then files) - let start_idx = criteria.offset.min(total_count); - let end_idx = (criteria.offset + criteria.limit).min(total_count); - - let folder_start = start_idx.min(folder_count); - let folder_end = end_idx.min(folder_count); - // Move the page out of the owned vecs instead of - // deep-cloning the slice — the source is dropped right - // after (benches/ROUND11.md §11: −300 allocs per page). let paginated_folders: Vec<_> = enriched_folders .into_iter() - .skip(folder_start) - .take(folder_end - folder_start) - .collect(); - - let file_start = start_idx.saturating_sub(folder_count); - let file_end = end_idx - .saturating_sub(folder_count) - .min(enriched_files.len()); - let paginated_files: Vec<_> = enriched_files - .into_iter() - .skip(file_start) - .take(file_end - file_start) + .skip(folders_before_page) + .take(folders_on_page) .collect(); + let paginated_files = enriched_files; let elapsed_ms = start.elapsed().as_millis() as u64; @@ -757,16 +801,30 @@ impl SearchUseCase for SearchService { // ── Recursive search via ltree (single SQL query per entity type) ── // Uses PostgreSQL ltree GiST index to find all files and folders // in the subtree in O(1) queries, replacing the O(N) spawn-per-folder - // approach that could saturate the connection pool. The content - // lookup, subtree file query and folder query overlap (`join!`), - // same as the non-recursive branch. - let (content_hits, files_page, folders_res) = tokio::join!( + // approach that could saturate the connection pool. + // + // ── Correct pagination across a folders-then-files list ── + // Pre-fix the service ran (content, file-page, folder-page) + // in one `tokio::join!` with the SAME `criteria.offset/limit` + // going to the file SQL, then re-paginated in memory using + // ABSOLUTE offsets. That was doubly wrong: SQL already + // applied `[offset, offset+limit)` and the in-memory slice + // then tried to skip `offset` MORE rows — for any query + // with few folders this dropped whole pages (2026-07-26: + // 4002-file query returned 50 rows on page 1 then `items:[]` + // on page 2 with a valid `next_cursor`). + // + // Fix: fold folders + content lookup first (they're both + // cheap and folder_count is what tells us how many files + // to skip). Then run the file SQL with `offset` shifted by + // `folder_count` and `limit` reduced by whatever folders + // fit on the current page — so SQL returns EXACTLY the + // file slice that belongs here, no in-memory re-slicing. + // The min-1 probe below preserves `COUNT(*) OVER()` even + // when folders fill the whole page (LIMIT 0 → 0 rows → + // total_count column projects nowhere → false zero). + let (content_hits, folders_res) = tokio::join!( self.lookup_content_hits(&criteria, user_id), - self.file_repository.search_files_in_subtree( - criteria.folder_id.as_deref(), - &criteria, - user_id, - ), self.folder_repository.search_folders( criteria.folder_id.as_deref(), criteria.name_contains.as_deref(), @@ -774,19 +832,50 @@ impl SearchUseCase for SearchService { true, ), ); - let (found_files, total_file_count) = files_page?; - let found_folders: Vec = folders_res?; + let (found_folders, folder_flags): (Vec, Vec<(bool, bool)>) = folders_res?; + + let folder_count = found_folders.len(); + let folders_before_page = criteria.offset.min(folder_count); + let folders_on_page = (folder_count - folders_before_page).min(criteria.limit); + let file_offset = criteria.offset - folders_before_page; + let file_limit_needed = criteria.limit - folders_on_page; + // Probe with LIMIT ≥ 1 so `COUNT(*) OVER()` has a row to + // project onto; the extra row (if any) is truncated below. + let file_limit_probe = file_limit_needed.max(1); + + let mut file_criteria = criteria.clone(); + file_criteria.offset = file_offset; + file_criteria.limit = file_limit_probe; + + let (found_files, file_flags, total_file_count) = self + .file_repository + .search_files_in_subtree(criteria.folder_id.as_deref(), &file_criteria, user_id) + .await?; // ── Convert to DTOs and enrich with server-computed metadata ── // Fused single pass: no intermediate DTO Vec materialization. + // Same shape as the non-recursive branch above — parallel + // zip of `found_files` with the in-SQL caller_flags. let mut enriched_files: Vec = found_files .into_iter() - .map(|f| Self::enrich_file(FileDto::from(f), &query_lower)) + .zip(file_flags) + .map(|(f, (is_fav, is_shr))| { + let mut dto = FileDto::from(f); + dto.is_favorite = is_fav; + dto.is_shared = is_shr; + Self::enrich_file(dto, &query_lower) + }) .collect(); let mut enriched_folders: Vec = found_folders .into_iter() - .map(|f| Self::enrich_folder(FolderDto::from(f), &query_lower)) + .zip(folder_flags) + .map(|(f, (is_fav, is_shr))| { + let mut dto = FolderDto::from(f); + dto.is_favorite = is_fav; + dto.is_shared = is_shr; + Self::enrich_folder(dto, &query_lower) + }) .collect(); // ── Sort folders (cached_key avoids O(N log N) temporary String allocations) ── @@ -808,38 +897,35 @@ impl SearchUseCase for SearchService { } } - // Blend in content-discovered files before the pagination math. - let added = self + // Blend in content-discovered files. `merge_content_hits` + // pushes candidates from the Tantivy index onto the tail + // and re-sorts by the criteria; the SQL limit above only + // bounded the name-match set, so truncate after merging + // to the exact page size we intended to return. + // + // `total_count` is the pure SQL name-match count + folder + // count — see the non-recursive branch's block comment + // for why `added` is deliberately excluded (per-page + // dedup outcome, would flicker the client-visible + // total across pages). + let _added = self .merge_content_hits(content_hits, &mut enriched_files, &criteria, user_id) .await?; - let total_file_count = total_file_count + added; + enriched_files.truncate(file_limit_needed); - // ── Pagination (folders first, then files) ── - let folder_count = enriched_folders.len(); let total_count = total_file_count + folder_count; - let start_idx = criteria.offset.min(total_count); - let end_idx = (criteria.offset + criteria.limit).min(total_count); - let folder_start = start_idx.min(folder_count); - let folder_end = end_idx.min(folder_count); - // Move the page out instead of deep-cloning the slice — the - // recursive branch's vecs can hold the whole subtree match - // set, all dropped right after (benches/ROUND11.md §11). + // Fetches were pre-sliced: enriched_files is already the + // exact file page (SQL applied file_offset + file_limit), + // and folders_before_page / folders_on_page tell us which + // slice of `enriched_folders` belongs here. No in-memory + // absolute-offset math — see the block comment above. let paginated_folders: Vec<_> = enriched_folders .into_iter() - .skip(folder_start) - .take(folder_end - folder_start) - .collect(); - - let file_start = start_idx.saturating_sub(folder_count); - let file_end = end_idx - .saturating_sub(folder_count) - .min(enriched_files.len()); - let paginated_files: Vec<_> = enriched_files - .into_iter() - .skip(file_start) - .take(file_end - file_start) + .skip(folders_before_page) + .take(folders_on_page) .collect(); + let paginated_files = enriched_files; let elapsed_ms = start.elapsed().as_millis() as u64; @@ -960,6 +1046,11 @@ mod tests { blob_hash: String::new(), snippet: None, match_source: None, + etag: String::new(), + created_by: None, + updated_by: None, + is_favorite: false, + is_shared: false, } } @@ -997,6 +1088,11 @@ mod tests { modified_at: 0, is_root: false, relevance_score: 50, + etag: String::new(), + created_by: None, + updated_by: None, + is_favorite: false, + is_shared: false, }], 100, 0, diff --git a/src/application/services/share_service.rs b/src/application/services/share_service.rs index 97404b18..1beebf4c 100644 --- a/src/application/services/share_service.rs +++ b/src/application/services/share_service.rs @@ -920,8 +920,15 @@ mod tests { _folder_id: Option<&str>, _criteria: &crate::application::dtos::search_dto::SearchCriteriaDto, _user_id: Uuid, - ) -> Result<(Vec, usize), DomainError> { - Ok((Vec::new(), 0)) + ) -> Result< + ( + Vec, + Vec<(bool, bool)>, + usize, + ), + DomainError, + > { + Ok((Vec::new(), Vec::new(), 0)) } async fn stream_files_in_subtree( diff --git a/src/application/services/trash_service_test.rs b/src/application/services/trash_service_test.rs index f7572434..f4a7f8cc 100644 --- a/src/application/services/trash_service_test.rs +++ b/src/application/services/trash_service_test.rs @@ -532,8 +532,8 @@ impl FileReadPort for MockFileRepository { _folder_id: Option<&str>, _criteria: &crate::application::dtos::search_dto::SearchCriteriaDto, _user_id: Uuid, - ) -> std::result::Result<(Vec, usize), DomainError> { - Ok((Vec::new(), 0)) + ) -> std::result::Result<(Vec, Vec<(bool, bool)>, usize), DomainError> { + Ok((Vec::new(), Vec::new(), 0)) } async fn stream_files_in_subtree( diff --git a/src/common/stubs.rs b/src/common/stubs.rs index 4aa44b57..952874f7 100644 --- a/src/common/stubs.rs +++ b/src/common/stubs.rs @@ -126,8 +126,8 @@ impl FileReadPort for StubFileReadPort { _folder_id: Option<&str>, _criteria: &SearchCriteriaDto, _user_id: Uuid, - ) -> Result<(Vec, usize), DomainError> { - Ok((Vec::new(), 0)) + ) -> Result<(Vec, Vec<(bool, bool)>, usize), DomainError> { + Ok((Vec::new(), Vec::new(), 0)) } async fn stream_files_in_subtree( diff --git a/src/domain/repositories/folder_repository.rs b/src/domain/repositories/folder_repository.rs index 6d6d9568..349a033f 100644 --- a/src/domain/repositories/folder_repository.rs +++ b/src/domain/repositories/folder_repository.rs @@ -237,31 +237,39 @@ pub trait FolderRepository: Send + Sync + 'static { /// /// The default implementation falls back to `list_folders` + in-memory /// filter so that stubs and mocks compile without changes. + /// + /// Returns `(folders, caller_flags)` where `caller_flags` is a + /// parallel `Vec<(is_favorite, is_shared)>` aligned 1:1 with + /// `folders` by index. The default impl fills with `(false, false)` + /// so stubs stay trivial; the concrete PG impl computes them via + /// per-row EXISTS on `auth.user_favorites` and `storage.role_grants` + /// (matching the file-side `search_files_paginated` pattern). async fn search_folders( &self, parent_id: Option<&str>, name_contains: Option<&str>, caller_id: Uuid, recursive: bool, - ) -> Result, DomainError> { + ) -> Result<(Vec, Vec<(bool, bool)>), DomainError> { // Recursive with folder_id → use optimised ltree scan - if recursive && let Some(fid) = parent_id { - return self - .list_descendant_folders(fid, name_contains, caller_id) - .await; - } - // Fallback: load + filter in memory (stubs / mocks) - let all = self.list_folders(parent_id).await?; - match name_contains { - Some(q) if !q.is_empty() => { - let q = q.to_lowercase(); - Ok(all - .into_iter() - .filter(|f| f.name().to_lowercase().contains(&q)) - .collect()) + let folders = if recursive && let Some(fid) = parent_id { + self.list_descendant_folders(fid, name_contains, caller_id) + .await? + } else { + // Fallback: load + filter in memory (stubs / mocks) + let all = self.list_folders(parent_id).await?; + match name_contains { + Some(q) if !q.is_empty() => { + let q = q.to_lowercase(); + all.into_iter() + .filter(|f| f.name().to_lowercase().contains(&q)) + .collect() + } + _ => all, } - _ => Ok(all), - } + }; + let flags = vec![(false, false); folders.len()]; + Ok((folders, flags)) } /// Return up to `limit` folders whose name contains `query` (case-insensitive). diff --git a/src/infrastructure/repositories/pg/file_blob_read_repository.rs b/src/infrastructure/repositories/pg/file_blob_read_repository.rs index db17649e..3656e19a 100644 --- a/src/infrastructure/repositories/pg/file_blob_read_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_read_repository.rs @@ -1212,7 +1212,7 @@ impl FileReadPort for FileBlobReadRepository { folder_id: Option<&str>, criteria: &SearchCriteriaDto, caller_id: Uuid, - ) -> Result<(Vec, usize), DomainError> { + ) -> Result<(Vec, Vec<(bool, bool)>, usize), DomainError> { let offset = criteria.offset as i64; let limit = criteria.limit as i64; @@ -1250,6 +1250,15 @@ impl FileReadPort for FileBlobReadRepository { let limit_bind = bind_idx + 1; let offset_bind = bind_idx + 2; + // Per-row caller flags — populated in-SQL so the search result + // is a single round-trip. Mirrors the pattern used by the + // photos-timeline listing above (see `list_photos_paginated`). + // Both EXISTS clauses hit narrow indexes (favorites is keyed + // on `(user_id, item_id, item_type)`; role_grants on + // `(resource_type, resource_id)`), each a sub-ms lookup — the + // cost across a 100-row search page is a few ms of index + // probing vs a full second round-trip for the alternative + // post-hoc batch approach. let sql = format!( "SELECT fi.id, fi.name, fi.folder_id, fo.path, \ fi.size, fi.mime_type, \ @@ -1258,6 +1267,17 @@ impl FileReadPort for FileBlobReadRepository { fi.blob_hash, \ \ fi.created_by, fi.updated_by, \ + EXISTS ( \ + SELECT 1 FROM auth.user_favorites uf \ + WHERE uf.user_id = $1 \ + AND uf.item_id = fi.id::text \ + AND uf.item_type = 'file' \ + ) AS is_favorite, \ + EXISTS ( \ + SELECT 1 FROM storage.role_grants g \ + WHERE g.resource_id = fi.id \ + AND g.resource_type = 'file' \ + ) AS is_shared, \ COUNT(*) OVER() AS total_count \ FROM storage.files fi \ LEFT JOIN storage.folders fo ON fo.id = fi.folder_id \ @@ -1281,6 +1301,8 @@ impl FileReadPort for FileBlobReadRepository { String, Option, // created_by (§14) Option, // updated_by (§14) + bool, // is_favorite + bool, // is_shared i64, // total_count ), >(&sql) @@ -1303,20 +1325,25 @@ impl FileReadPort for FileBlobReadRepository { .map_err(|e| DomainError::internal_error("FileBlobRead", format!("search: {e}")))?; // total_count is the same in every row; 0 when result set is empty. - let total_count = rows.first().map_or(0, |r| r.11) as usize; + let total_count = rows.first().map_or(0, |r| r.13) as usize; - // Pre-size the result Vec (size-hint note in `hydrate`, ROUND20 §I1). + // Pre-size both parallel Vecs (size-hint note in `hydrate`, + // ROUND20 §I1). Caller-flags stay aligned with `files` by index. let mut files = Vec::with_capacity(rows.len()); - for (id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub, _total) in rows { + let mut caller_flags = Vec::with_capacity(rows.len()); + for (id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub, is_fav, is_shr, _total) in + rows + { files.push( Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) .map_err(|e| { DomainError::internal_error("FileBlobRead", format!("mapping: {e}")) })?, ); + caller_flags.push((is_fav, is_shr)); } - Ok((files, total_count)) + Ok((files, caller_flags, total_count)) } /// Recursive subtree search using ltree — single SQL query. @@ -1337,7 +1364,7 @@ impl FileReadPort for FileBlobReadRepository { root_folder_id: Option<&str>, criteria: &SearchCriteriaDto, caller_id: Uuid, - ) -> Result<(Vec, usize), DomainError> { + ) -> Result<(Vec, Vec<(bool, bool)>, usize), DomainError> { // When no root folder specified, delegate to existing paginated search let root_id = match root_folder_id { None => { @@ -1382,7 +1409,9 @@ impl FileReadPort for FileBlobReadRepository { let limit_bind = bind_idx + 1; let offset_bind = bind_idx + 2; - // ── Single query with COUNT(*) OVER() ── + // ── Single query with COUNT(*) OVER() + per-row caller flags ── + // See `search_files_paginated` above for the EXISTS-subquery + // rationale; same shape applies here. let sql = format!( "SELECT fi.id, fi.name, fi.folder_id, fo.path, \ fi.size, fi.mime_type, \ @@ -1391,6 +1420,17 @@ impl FileReadPort for FileBlobReadRepository { fi.blob_hash, \ \ fi.created_by, fi.updated_by, \ + EXISTS ( \ + SELECT 1 FROM auth.user_favorites uf \ + WHERE uf.user_id = $1 \ + AND uf.item_id = fi.id::text \ + AND uf.item_type = 'file' \ + ) AS is_favorite, \ + EXISTS ( \ + SELECT 1 FROM storage.role_grants g \ + WHERE g.resource_id = fi.id \ + AND g.resource_type = 'file' \ + ) AS is_shared, \ COUNT(*) OVER() AS total_count \ FROM storage.files fi \ JOIN storage.folders fo ON fo.id = fi.folder_id \ @@ -1414,6 +1454,8 @@ impl FileReadPort for FileBlobReadRepository { String, Option, // created_by (§14) Option, // updated_by (§14) + bool, // is_favorite + bool, // is_shared i64, // total_count ), >(&sql) @@ -1434,20 +1476,24 @@ impl FileReadPort for FileBlobReadRepository { DomainError::internal_error("FileBlobRead", format!("subtree search: {e}")) })?; - let total_count = rows.first().map_or(0, |r| r.11) as usize; + let total_count = rows.first().map_or(0, |r| r.13) as usize; - // Pre-size the result Vec (size-hint note in `hydrate`, ROUND20 §I1). + // Pre-size both Vecs, keep caller_flags aligned by index. let mut files = Vec::with_capacity(rows.len()); - for (id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub, _total) in rows { + let mut caller_flags = Vec::with_capacity(rows.len()); + for (id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub, is_fav, is_shr, _total) in + rows + { files.push( Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, cb, ub) .map_err(|e| { DomainError::internal_error("FileBlobRead", format!("subtree mapping: {e}")) })?, ); + caller_flags.push((is_fav, is_shr)); } - Ok((files, total_count)) + Ok((files, caller_flags, total_count)) } #[allow(clippy::type_complexity)] diff --git a/src/infrastructure/repositories/pg/folder_db_repository.rs b/src/infrastructure/repositories/pg/folder_db_repository.rs index c1cedb36..c2134ebc 100644 --- a/src/infrastructure/repositories/pg/folder_db_repository.rs +++ b/src/infrastructure/repositories/pg/folder_db_repository.rs @@ -49,6 +49,48 @@ type FolderRow = ( Option, ); +/// `FolderRow` + trailing `(is_favorite, is_shared)` — the caller-scoped +/// booleans populated by per-row EXISTS in `search_folders`. Split out so +/// the many `sqlx::query_as::<_, FolderRowWithFlags>(...)` sites stay +/// terse instead of repeating a 12-element inline tuple. +type FolderRowWithFlags = ( + Uuid, + String, + String, + Option, + Uuid, + i64, + i64, + i64, + Option, + Option, + bool, + bool, +); + +/// Return shape of `search_folders`: two parallel vecs aligned 1:1 +/// (folder domain entity + `(is_favorite, is_shared)` caller flags). +/// Kept as a type alias so the SQL builder + trait implementations +/// share one name (`clippy::type_complexity`). +pub(crate) type FoldersWithFlags = (Vec, Vec<(bool, bool)>); + +/// Convert `FolderRowWithFlags` rows into the `(Vec, caller_flags)` +/// pair `search_folders` returns. Keeps the two parallel vecs aligned +/// 1:1 by index. +fn build_folders_with_flags( + rows: Vec, +) -> Result { + let mut folders = Vec::with_capacity(rows.len()); + let mut flags = Vec::with_capacity(rows.len()); + for (id, name, path, pid, did, ca, ma, tma, cb, ub, is_fav, is_shr) in rows { + let folder = + FolderDbRepository::row_to_folder(id, name, path, pid, did, ca, ma, tma, cb, ub)?; + folders.push(folder); + flags.push((is_fav, is_shr)); + } + Ok((folders, flags)) +} + /// Type alias for paginated folder rows (includes total_count as /// the last element after the §14 provenance columns). Same /// column set as [`FolderRow`] plus the trailing count. @@ -1048,12 +1090,20 @@ impl FolderRepository for FolderDbRepository { name_contains: Option<&str>, caller_id: Uuid, recursive: bool, - ) -> Result, DomainError> { - // Recursive with folder scope → existing optimised ltree scan + ) -> Result<(Vec, Vec<(bool, bool)>), DomainError> { + // Recursive with folder scope → existing optimised ltree scan. + // `list_descendant_folders` doesn't compute caller_flags today — + // return `(false, false)` per row until the ltree path is + // upgraded in a follow-up. Bounded UI impact: subtree-scoped + // searches rarely surface a specific folder as favorited / + // shared, and the flags path elsewhere fills the gap for the + // hot `/api/search` case. if recursive && let Some(fid) = parent_id { - return self + let folders = self .list_descendant_folders(fid, name_contains, caller_id) - .await; + .await?; + let flags = vec![(false, false); folders.len()]; + return Ok((folders, flags)); } // Build optional name filter — use ILIKE (case-insensitive) so the @@ -1070,6 +1120,21 @@ impl FolderRepository for FolderDbRepository { _ => ("", None), }; + // Per-row caller-flag EXISTS subqueries. Same pattern as + // `search_files_paginated` in the sibling file repo: narrow + // indexes make this a few extra μs per row. + const FAV_SHR_COLUMNS: &str = "EXISTS ( \ + SELECT 1 FROM auth.user_favorites uf \ + WHERE uf.user_id = $1 \ + AND uf.item_id = fo.id::text \ + AND uf.item_type = 'folder' \ + ) AS is_favorite, \ + EXISTS ( \ + SELECT 1 FROM storage.role_grants g \ + WHERE g.resource_id = fo.id \ + AND g.resource_type = 'folder' \ + ) AS is_shared"; + if recursive { // Recursive, no folder scope → ALL folders in caller's readable drives let sql = format!( @@ -1078,7 +1143,8 @@ impl FolderRepository for FolderDbRepository { EXTRACT(EPOCH FROM fo.created_at)::bigint, \ EXTRACT(EPOCH FROM fo.updated_at)::bigint, \ EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint, \ - fo.created_by, fo.updated_by \ + fo.created_by, fo.updated_by, \ + {FAV_SHR_COLUMNS} \ FROM storage.folders fo \ WHERE {CALLER_CAN_READ_DRIVE} \ AND fo.is_trashed = false \ @@ -1086,7 +1152,7 @@ impl FolderRepository for FolderDbRepository { ORDER BY fo.name" ); - let rows: Vec = if let Some(ref pattern) = name_pattern { + let rows: Vec = if let Some(ref pattern) = name_pattern { sqlx::query_as(&sql) .bind(caller_id) .bind(pattern) @@ -1100,12 +1166,7 @@ impl FolderRepository for FolderDbRepository { } .map_err(|e| DomainError::internal_error("FolderDb", format!("search_folders: {e}")))?; - return rows - .into_iter() - .map(|(id, name, path, pid, did, ca, ma, tma, cb, ub)| { - Self::row_to_folder(id, name, path, pid, did, ca, ma, tma, cb, ub) - }) - .collect(); + return build_folders_with_flags(rows); } // Non-recursive: direct children of parent_id, restricted to drives @@ -1117,7 +1178,8 @@ impl FolderRepository for FolderDbRepository { EXTRACT(EPOCH FROM fo.created_at)::bigint, \ EXTRACT(EPOCH FROM fo.updated_at)::bigint, \ EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint, \ - fo.created_by, fo.updated_by \ + fo.created_by, fo.updated_by, \ + {FAV_SHR_COLUMNS} \ FROM storage.folders fo \ WHERE fo.parent_id = $2::uuid \ AND {CALLER_CAN_READ_DRIVE} \ @@ -1137,7 +1199,8 @@ impl FolderRepository for FolderDbRepository { EXTRACT(EPOCH FROM fo.created_at)::bigint, \ EXTRACT(EPOCH FROM fo.updated_at)::bigint, \ EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint, \ - fo.created_by, fo.updated_by \ + fo.created_by, fo.updated_by, \ + {FAV_SHR_COLUMNS} \ FROM storage.folders fo \ WHERE fo.parent_id IS NULL \ AND {CALLER_CAN_READ_DRIVE} \ @@ -1147,7 +1210,7 @@ impl FolderRepository for FolderDbRepository { ) }; - let rows: Vec = if let Some(pid) = parent_id { + let rows: Vec = if let Some(pid) = parent_id { if let Some(ref pattern) = name_pattern { sqlx::query_as(&sql) .bind(caller_id) @@ -1176,11 +1239,7 @@ impl FolderRepository for FolderDbRepository { } .map_err(|e| DomainError::internal_error("FolderDb", format!("search_folders: {e}")))?; - rows.into_iter() - .map(|(id, name, path, pid, did, ca, ma, tma, cb, ub)| { - Self::row_to_folder(id, name, path, pid, did, ca, ma, tma, cb, ub) - }) - .collect() + build_folders_with_flags(rows) } /// Lists all descendant folders in a subtree using ltree GiST index, diff --git a/src/interfaces/api/handlers/search_handler.rs b/src/interfaces/api/handlers/search_handler.rs index 32e4b107..5e3834d0 100644 --- a/src/interfaces/api/handlers/search_handler.rs +++ b/src/interfaces/api/handlers/search_handler.rs @@ -7,7 +7,7 @@ use serde_json::json; use tracing::{error, info}; use crate::application::dtos::search_dto::{ - SearchCriteriaDto, SearchResultsDto, SearchSuggestionsDto, + SearchResourcesDto, SearchResourcesQuery, SearchSuggestionsDto, }; use crate::application::ports::inbound::SearchUseCase; use crate::common::di::AppState; @@ -39,12 +39,32 @@ impl SearchHandler { // so `#[utoipa::path]` fails on every method in this impl block regardless of HTTP // verb or annotation content. All route handlers are free functions below. // TODO: collapse after utoipa upgrade. - pub(super) async fn search_files_get_impl( + /// `GET /api/search` — wire-normalised search endpoint. + /// + /// Returns the same `items[] { resource_type, resource, meta }` + /// envelope shape as every other `/*/resources` listing endpoint + /// (folders, favorites, recent, trash, shared) so the SPA's + /// `ResourceList` component consumes it as-is. Search-specific + /// enrichment (`meta.score` + optional `snippet` + `via`) sits + /// inline on each item. + /// + /// Phase 1-plus wire adapter: the internal `SearchService` still + /// speaks `SearchCriteriaDto`/`SearchResultsDto`. The query is + /// translated at this boundary; the result envelope is composed + /// via `SearchResourcesDto::from_service_result`. The `is_favorite` + /// / `is_shared` fields on each `FileDto`/`FolderDto` come from + /// per-row EXISTS subqueries in the search SQL (see + /// `search_files_paginated` and `search_folders`). + /// + /// The old `POST /api/search/advanced` variant was deleted in + /// the same PR — every field it accepted fits cleanly as a query + /// param. + pub(super) async fn search_resources_impl( State(state): State>, auth_user: AuthUser, - Query(params): Query, + Query(query): Query, ) -> impl IntoResponse { - info!("API: File search with parameters: {:?}", params); + info!("API: File search (normalized envelope)"); let search_service = match &state.applications.search_service { Some(service) => service, @@ -58,25 +78,13 @@ impl SearchHandler { } }; - let search_criteria = SearchCriteriaDto { - name_contains: params.query, - file_types: params - .type_filter - .map(|t| t.split(',').map(|s| s.trim().to_string()).collect()), - created_after: params.created_after, - created_before: params.created_before, - modified_after: params.modified_after, - modified_before: params.modified_before, - min_size: params.min_size, - max_size: params.max_size, - folder_id: params.folder_id, - recursive: params.recursive.unwrap_or(true), - limit: params.limit.unwrap_or(100).min(MAX_SEARCH_LIMIT), - offset: params.offset.unwrap_or(0), - sort_by: params.sort_by.unwrap_or_else(|| "relevance".to_string()), - }; + // Cap page size — `SearchResourcesQuery::limit_clamped` already + // hits `[1, 200]`, but re-clamp against MAX_SEARCH_LIMIT for + // defence-in-depth if the constant is ever raised above 200. + let mut criteria = query.to_criteria(); + criteria.limit = criteria.limit.min(MAX_SEARCH_LIMIT); - match search_service.search(search_criteria, auth_user.id).await { + match search_service.search(criteria, auth_user.id).await { Ok(results) => { info!( "Search completed in {}ms — {} files, {} folders", @@ -84,62 +92,18 @@ impl SearchHandler { results.files.len(), results.folders.len() ); - { - // Pre-sized serialization (benches/ROUND12.md §M1). - let rows = results.files.len() + results.folders.len(); - crate::interfaces::api::sized_json::sized_json( - 256 + rows * crate::interfaces::api::sized_json::EST_WRAPPED_ROW_BYTES, - &*results, - ) - } - } - Err(err) => { - error!("Search error: {}", err); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(json!({ "error": "Search error" })), + // Unwrap the Arc — the service caches `Arc` + // so consumers share the allocation. `from_service_result` + // consumes the DTO to move enriched rows into the envelope's + // `resource` slot without cloning; the Arc's shared clone + // pays one deep copy here but avoids allocating during the + // hot cache-hit path elsewhere. + let dto = SearchResourcesDto::from_service_result((*results).clone(), &query); + let rows = dto.items.len(); + crate::interfaces::api::sized_json::sized_json( + 256 + rows * crate::interfaces::api::sized_json::EST_WRAPPED_ROW_BYTES, + &dto, ) - .into_response() - } - } - } - - /// Advanced search with full criteria in the request body. - pub(super) async fn search_files_post_impl( - State(state): State>, - auth_user: AuthUser, - Json(criteria): Json, - ) -> impl IntoResponse { - info!("API: Advanced file search"); - - let search_service = match &state.applications.search_service { - Some(service) => service, - None => { - error!("Search service not available"); - return ( - StatusCode::SERVICE_UNAVAILABLE, - Json(json!({ "error": "Search service is not available" })), - ) - .into_response(); - } - }; - - match search_service.search(criteria, auth_user.id).await { - Ok(results) => { - info!( - "Advanced search completed in {}ms — {} files, {} folders", - results.query_time_ms, - results.files.len(), - results.folders.len() - ); - { - // Pre-sized serialization (benches/ROUND12.md §M1). - let rows = results.files.len() + results.folders.len(); - crate::interfaces::api::sized_json::sized_json( - 256 + rows * crate::interfaces::api::sized_json::EST_WRAPPED_ROW_BYTES, - &*results, - ) - } } Err(err) => { error!("Search error: {}", err); @@ -258,50 +222,6 @@ impl SearchHandler { } } -/// Search parameters for the GET /search endpoint -#[derive(Debug, serde::Deserialize)] -pub struct SearchParams { - /// Text to search in file and folder names - pub query: Option, - - /// Filter by file types (comma-separated extensions) - #[serde(rename = "type")] - pub type_filter: Option, - - /// Created after this timestamp - pub created_after: Option, - - /// Created before this timestamp - pub created_before: Option, - - /// Modified after this timestamp - pub modified_after: Option, - - /// Modified before this timestamp - pub modified_before: Option, - - /// Minimum file size in bytes - pub min_size: Option, - - /// Maximum file size in bytes - pub max_size: Option, - - /// Folder ID to limit the search scope - pub folder_id: Option, - - /// Recursive search in subfolders (default: true) - pub recursive: Option, - - /// Result limit for pagination - pub limit: Option, - - /// Offset for pagination - pub offset: Option, - - /// Sort order: relevance | name | name_desc | date | date_desc | size | size_desc - pub sort_by: Option, -} - /// Parameters for the GET /search/suggest endpoint #[derive(Debug, serde::Deserialize)] pub struct SuggestParams { @@ -334,45 +254,35 @@ pub struct SuggestParams { get, path = "/api/search", params( - ("query" = Option, Query, description = "Text to search in names"), - ("type" = Option, Query, description = "Comma-separated MIME type filter"), + ("query" = Option, Query, description = "Text to search in names / content"), + ("limit" = Option, Query, description = "Max items per page (1–200, default 50)"), + ("cursor" = Option, Query, description = "Opaque cursor from a previous response"), + ("order_by" = Option, Query, description = "Sort dimension: relevance (default) | name | name_desc | date | date_desc | size | size_desc"), + ("resource_types" = Option, Query, description = "Comma-separated: file, folder (both by default)"), + ("reverse" = Option, Query, description = "Reverse the sort order"), + ("type" = Option, Query, description = "Filter by file extensions (comma-separated)"), ("folder_id" = Option, Query, description = "Restrict search to this folder"), - ("recursive" = Option, Query, description = "Include sub-folders"), - ("limit" = Option, Query, description = "Max results"), - ("offset" = Option, Query, description = "Pagination offset"), + ("recursive" = Option, Query, description = "Recurse into subfolders (default true)"), + ("created_after" = Option, Query, description = "Minimum creation timestamp (unix seconds)"), + ("created_before" = Option, Query, description = "Maximum creation timestamp"), + ("modified_after" = Option, Query, description = "Minimum modification timestamp"), + ("modified_before" = Option, Query, description = "Maximum modification timestamp"), + ("min_size" = Option, Query, description = "Minimum file size (bytes)"), + ("max_size" = Option, Query, description = "Maximum file size (bytes)"), ), responses( - (status = 200, description = "Search results", body = SearchResultsDto), + (status = 200, description = "Search results (cursor-paginated envelope shared with /*/resources)", body = SearchResourcesDto), (status = 503, description = "Search service unavailable"), ), security(("bearerAuth" = [])), tag = "search" )] -pub async fn search_files_get( +pub async fn search_resources( state: State>, auth_user: AuthUser, - query: Query, + query: Query, ) -> impl IntoResponse { - SearchHandler::search_files_get_impl(state, auth_user, query).await -} - -#[utoipa::path( - post, - path = "/api/search/advanced", - request_body(content = SearchCriteriaDto, content_type = "application/json", description = "Search criteria"), - responses( - (status = 200, description = "Search results", body = SearchResultsDto), - (status = 503, description = "Search service unavailable"), - ), - security(("bearerAuth" = [])), - tag = "search" -)] -pub async fn search_files_post( - state: State>, - auth_user: AuthUser, - json: Json, -) -> impl IntoResponse { - SearchHandler::search_files_post_impl(state, auth_user, json).await + SearchHandler::search_resources_impl(state, auth_user, query).await } #[utoipa::path( diff --git a/src/interfaces/api/mod.rs b/src/interfaces/api/mod.rs index 15f07ecc..d94540a4 100644 --- a/src/interfaces/api/mod.rs +++ b/src/interfaces/api/mod.rs @@ -34,8 +34,8 @@ use crate::application::dtos::i18n_dto::{ use crate::application::dtos::pagination::{PaginationDto, PaginationRequestDto}; use crate::application::dtos::recent_dto::{RecentItemDto, RecentResourceItemDto}; use crate::application::dtos::search_dto::{ - SearchCriteriaDto, SearchFileResultDto, SearchFolderResultDto, SearchResultsDto, - SearchSuggestionItem, SearchSuggestionsDto, + SearchCriteriaDto, SearchFileResultDto, SearchFolderResultDto, SearchMeta, SearchResourceItem, + SearchResourcesDto, SearchResultsDto, SearchSuggestionItem, SearchSuggestionsDto, }; use crate::application::dtos::share_dto::{CreateShareDto, ShareDto, UpdateShareDto}; use crate::application::dtos::trash_dto::{ @@ -103,8 +103,7 @@ use crate::interfaces::api::handlers::file_handler::MoveFilePayload; handlers::folder_handler::delete_folder_with_trash, handlers::folder_handler::download_folder_zip, // Search handlers (free functions — see search_handler.rs for why) - handlers::search_handler::search_files_get, - handlers::search_handler::search_files_post, + handlers::search_handler::search_resources, handlers::search_handler::suggest_files, handlers::search_handler::clear_search_cache, // i18n handlers (free functions — see i18n_handler.rs for why) @@ -302,7 +301,15 @@ use crate::interfaces::api::handlers::file_handler::MoveFilePayload; MoveToTrashRequest, RestoreFromTrashRequest, DeletePermanentlyRequest, - // Search schemas + // Search schemas — wire envelope shares the /*/resources + // shape (SearchResourcesDto → items[] { resource_type, + // resource, meta }). The internal SearchCriteriaDto / + // SearchResultsDto types are still emitted so external + // consumers browsing the OpenAPI doc can see the service- + // layer shape referenced by other docs. + SearchResourcesDto, + SearchResourceItem, + SearchMeta, SearchCriteriaDto, SearchResultsDto, SearchFileResultDto, diff --git a/src/interfaces/api/routes.rs b/src/interfaces/api/routes.rs index e0de85ee..9959d4ff 100644 --- a/src/interfaces/api/routes.rs +++ b/src/interfaces/api/routes.rs @@ -88,9 +88,7 @@ use crate::interfaces::api::handlers::folder_handler::{ use crate::interfaces::api::handlers::i18n_handler::{ get_locales, get_translations_by_locale, translate, }; -use crate::interfaces::api::handlers::search_handler::{ - search_files_get, search_files_post, suggest_files, -}; +use crate::interfaces::api::handlers::search_handler::{search_resources, suggest_files}; use crate::interfaces::api::handlers::trash_handler; /// Creates root-level health check routes — mounted directly at `/`, not under `/api/`. @@ -298,11 +296,14 @@ pub fn create_api_routes(app_state: &Arc) -> Router> { let search_router = if search_service.is_some() { Router::new() // Simple search with query parameters - .route("/", get(search_files_get)) + // Cursor-paginated search with the `/*/resources` envelope + // (items + next_cursor + meta). `POST /search/advanced` was + // deleted alongside this normalization — every field it + // accepted fits fine as a query param, and it shared 100% + // of the service call with GET (no fast-vs-deep semantics). + .route("/", get(search_resources)) // Lightweight autocomplete suggestions .route("/suggest", get(suggest_files)) - // Advanced search with full criteria object - .route("/advanced", post(search_files_post)) // `DELETE /api/search/cache` used to live here as a per-user- // reachable endpoint. It's an operator-only debug lever // (moka `invalidate_all()` — nukes every tenant), so it diff --git a/tests/api/search_basic.hurl b/tests/api/search_basic.hurl index b034b24e..e1400363 100644 --- a/tests/api/search_basic.hurl +++ b/tests/api/search_basic.hurl @@ -126,7 +126,12 @@ Authorization: Bearer {{admin_token}} HTTP 200 [Asserts] -jsonpath "$.files" count >= 1 +# `/api/search` was normalised to the `/*/resources` envelope in +# PR search-normalize (2026-07): items[] carry `resource_type` + +# a `resource` (File | Folder | Drive) + inline search-meta. This +# assertion checks the same anti-regression property as before +# (needle file surfaces to its owner) against the new wire shape. +jsonpath "$.items" count >= 1 body contains "{{needle_file_id}}" @@ -140,8 +145,7 @@ Authorization: Bearer {{admin_token}} HTTP 200 [Asserts] -jsonpath "$.files" count == 0 -jsonpath "$.folders" count == 0 +jsonpath "$.items" count == 0 # ───────────────────────────────────────────────────────────── @@ -236,7 +240,7 @@ delay: 2500ms HTTP 200 [Asserts] # Admin sees the content match — proves indexing landed. -jsonpath "$.files" count >= 1 +jsonpath "$.items" count >= 1 body contains "{{canary_file_id}}" @@ -248,23 +252,22 @@ HTTP 200 # Bob has no access to admin's drive → Tantivy's Must-clause # filters every doc that doesn't carry one of Bob's drive_ids, # so the file vanishes entirely. -jsonpath "$.files" count == 0 -jsonpath "$.folders" count == 0 +jsonpath "$.items" count == 0 body not contains "{{canary_file_id}}" body not contains "ContentIndexCanaryXyzzy2026Drive" # Anti-enum: every count the response surfaces must reflect the # FILTERED set — i.e. zero when the caller has no accessible # hits. The §11 rule is "no 'you have N hidden matches' field -# anywhere". `total_count` is a legitimate pagination count and -# is OK as long as it equals the filtered total (zero here). The -# other field names below MUST stay absent: a future field -# called `hidden_count`/`filtered`/etc. that reveals matches -# Bob can't see would be the regression. -jsonpath "$.total_count" == 0 -jsonpath "$.has_more" == false +# anywhere". `total` is the visible-to-caller count (permission- +# filtered SUM); it's OK when it equals the visible total (zero +# here). Old `total_count` / `has_more` names are retired with +# the `files/folders` split. Names below MUST stay absent — a +# future field like `hidden_count`/`filtered`/etc. that reveals +# matches Bob can't see would be the regression. +jsonpath "$.total" == 0 +jsonpath "$.next_cursor" not exists jsonpath "$.hidden_count" not exists jsonpath "$.filtered" not exists -jsonpath "$.total" not exists # ─────────────────────────────────────────────────────────────