From 0c820a6ec4e75256f2c29f7b55501a20ce58c074 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 13 Nov 2024 15:18:35 +0530 Subject: [PATCH] [web] Fix search when an album only has symlinks The uniquification would prevent albums that only contains photos that are already present in another album from appearing in search results. --- web/apps/photos/src/pages/gallery.tsx | 7 +----- .../new/photos/components/gallery/reducer.ts | 25 +------------------ web/packages/new/photos/services/files.ts | 23 +++++++++++++++++ .../new/photos/services/search/index.ts | 14 +++++++++-- .../new/photos/services/search/types.ts | 13 ++++++++++ .../new/photos/services/search/worker.ts | 16 ++++++------ 6 files changed, 58 insertions(+), 40 deletions(-) diff --git a/web/apps/photos/src/pages/gallery.tsx b/web/apps/photos/src/pages/gallery.tsx index 0f286d8ad4..f6deba1397 100644 --- a/web/apps/photos/src/pages/gallery.tsx +++ b/web/apps/photos/src/pages/gallery.tsx @@ -24,7 +24,6 @@ import { SearchResultsHeader, } from "@/new/photos/components/gallery"; import { - uniqueFilesByID, useGalleryReducer, type GalleryBarMode, } from "@/new/photos/components/gallery/reducer"; @@ -401,11 +400,7 @@ export default function Gallery() { }, []); useEffect( - () => - setSearchCollectionsAndFiles({ - collections: collections ?? [], - files: uniqueFilesByID(files ?? []), - }), + () => setSearchCollectionsAndFiles({ collections, files }), [collections, files], ); diff --git a/web/packages/new/photos/components/gallery/reducer.ts b/web/packages/new/photos/components/gallery/reducer.ts index 1a90a6afc0..83b98f1f28 100644 --- a/web/packages/new/photos/components/gallery/reducer.ts +++ b/web/packages/new/photos/components/gallery/reducer.ts @@ -33,7 +33,7 @@ import { getLatestVersionFiles, groupFilesByCollectionID, } from "../../services/file"; -import { sortFiles } from "../../services/files"; +import { sortFiles, uniqueFilesByID } from "../../services/files"; import { isArchivedCollection, isArchivedFile, @@ -763,29 +763,6 @@ const galleryReducer: React.Reducer = ( export const useGalleryReducer = () => useReducer(galleryReducer, initialGalleryState); -/** - * File IDs themselves are unique across all the files for the user (in fact, - * they're unique across all the files in an Ente instance). However, we still - * can have multiple entries for the same file ID in our local database because - * the unit of account is not actually a file, but a "Collection File": a - * collection and file pair. - * - * For example, if the same file is symlinked into two collections, then we will - * have two "Collection File" entries for it, both with the same file ID, but - * with different collection IDs. - * - * This function returns files such that only one of these entries (the newer - * one in case of dupes) is returned. - */ -export const uniqueFilesByID = (files: EnteFile[]) => { - const seen = new Set(); - return files.filter(({ id }) => { - if (seen.has(id)) return false; - seen.add(id); - return true; - }); -}; - /** * Compute archived collection IDs from their dependencies. */ diff --git a/web/packages/new/photos/services/files.ts b/web/packages/new/photos/services/files.ts index cbd4122961..1fba3c407b 100644 --- a/web/packages/new/photos/services/files.ts +++ b/web/packages/new/photos/services/files.ts @@ -61,6 +61,29 @@ export const sortFiles = (files: EnteFile[], sortAsc = false) => { }); }; +/** + * File IDs themselves are unique across all the files for the user (in fact, + * they're unique across all the files in an Ente instance). However, we still + * can have multiple entries for the same file ID in our local database because + * the unit of account is not actually a file, but a "Collection File": a + * collection and file pair. + * + * For example, if the same file is symlinked into two collections, then we will + * have two "Collection File" entries for it, both with the same file ID, but + * with different collection IDs. + * + * This function returns files such that only one of these entries (the newer + * one in case of dupes) is returned. + */ +export const uniqueFilesByID = (files: EnteFile[]) => { + const seen = new Set(); + return files.filter(({ id }) => { + if (seen.has(id)) return false; + seen.add(id); + return true; + }); +}; + export const TRASH = "file-trash"; export async function getLocalTrash() { diff --git a/web/packages/new/photos/services/search/index.ts b/web/packages/new/photos/services/search/index.ts index 07e3cd326d..6b0a593419 100644 --- a/web/packages/new/photos/services/search/index.ts +++ b/web/packages/new/photos/services/search/index.ts @@ -3,6 +3,7 @@ import { masterKeyFromSession } from "@/base/session-store"; import { ComlinkWorker } from "@/base/worker/comlink-worker"; import { FileType } from "@/media/file-type"; import i18n, { t } from "i18next"; +import { uniqueFilesByID } from "../files"; import { clipMatches, isMLEnabled, isMLSupported } from "../ml"; import type { NamedPerson } from "../ml/people"; import type { @@ -54,8 +55,17 @@ export const searchDataSync = () => /** * Set the collections and files over which we should search. */ -export const setSearchCollectionsAndFiles = (cf: SearchCollectionsAndFiles) => - void worker().then((w) => w.setCollectionsAndFiles(cf)); +export const setSearchCollectionsAndFiles = ({ + collections, + files, +}: Omit) => + void worker().then((w) => + w.setCollectionsAndFiles({ + collections: collections, + files: uniqueFilesByID(files), + collectionFiles: files, + }), + ); /** * Set the (named) people that we should search across. diff --git a/web/packages/new/photos/services/search/types.ts b/web/packages/new/photos/services/search/types.ts index c8012b8050..ec05988df5 100644 --- a/web/packages/new/photos/services/search/types.ts +++ b/web/packages/new/photos/services/search/types.ts @@ -49,7 +49,20 @@ export interface SearchOption { */ export interface SearchCollectionsAndFiles { collections: Collection[]; + /** + * Unique files (by ID). + * + * @see {@link uniqueFilesByID}. + */ files: EnteFile[]; + /** + * One entry per collection/file pair. + * + * Whenever the same file (ID) is in multiple collections, the + * {@link collectionFiles} will have multiple entries with the same file ID, + * one per collection in which that file (ID) occurs. + */ + collectionFiles: EnteFile[]; } export interface LabelledSearchDateComponents { diff --git a/web/packages/new/photos/services/search/worker.ts b/web/packages/new/photos/services/search/worker.ts index a3f03a2be9..d4d4667062 100644 --- a/web/packages/new/photos/services/search/worker.ts +++ b/web/packages/new/photos/services/search/worker.ts @@ -36,6 +36,7 @@ export class SearchWorker { private collectionsAndFiles: SearchCollectionsAndFiles = { collections: [], files: [], + collectionFiles: [], }; private people: NamedPerson[] = []; @@ -94,19 +95,16 @@ export class SearchWorker { * Return {@link EnteFile}s that satisfy the given {@link suggestion}. */ filterSearchableFiles(suggestion: SearchSuggestion) { - return filterSearchableFiles( - this.collectionsAndFiles.files, - suggestion, - ); + return filterSearchableFiles(this.collectionsAndFiles, suggestion); } /** * Batched variant of {@link filterSearchableFiles}. */ filterSearchableFilesMulti(suggestions: SearchSuggestion[]) { - const files = this.collectionsAndFiles.files; + const cf = this.collectionsAndFiles; return suggestions - .map((sg) => [filterSearchableFiles(files, sg), sg] as const) + .map((sg) => [filterSearchableFiles(cf, sg), sg] as const) .filter(([files]) => files.length); } } @@ -350,11 +348,13 @@ const locationSuggestions = ( }; const filterSearchableFiles = ( - files: EnteFile[], + { files, collectionFiles }: SearchCollectionsAndFiles, suggestion: SearchSuggestion, ) => sortMatchesIfNeeded( - files.filter((f) => isMatchingFile(f, suggestion)), + (suggestion.type == "collection" ? collectionFiles : files).filter( + (f) => isMatchingFile(f, suggestion), + ), suggestion, );