Skip to content

Commit

Permalink
[web] Fix search when an album only has symlinks (#4025)
Browse files Browse the repository at this point in the history
The uniquification would prevent albums that only contains photos that
are
already present in another album from appearing in search results.
  • Loading branch information
mnvr authored Nov 13, 2024
2 parents fab883c + 0c820a6 commit fcfc697
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 40 deletions.
7 changes: 1 addition & 6 deletions web/apps/photos/src/pages/gallery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
SearchResultsHeader,
} from "@/new/photos/components/gallery";
import {
uniqueFilesByID,
useGalleryReducer,
type GalleryBarMode,
} from "@/new/photos/components/gallery/reducer";
Expand Down Expand Up @@ -401,11 +400,7 @@ export default function Gallery() {
}, []);

useEffect(
() =>
setSearchCollectionsAndFiles({
collections: collections ?? [],
files: uniqueFilesByID(files ?? []),
}),
() => setSearchCollectionsAndFiles({ collections, files }),
[collections, files],
);

Expand Down
25 changes: 1 addition & 24 deletions web/packages/new/photos/components/gallery/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
getLatestVersionFiles,
groupFilesByCollectionID,
} from "../../services/file";
import { sortFiles } from "../../services/files";
import { sortFiles, uniqueFilesByID } from "../../services/files";
import {
isArchivedCollection,
isArchivedFile,
Expand Down Expand Up @@ -763,29 +763,6 @@ const galleryReducer: React.Reducer<GalleryState, GalleryAction> = (
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<number>();
return files.filter(({ id }) => {
if (seen.has(id)) return false;
seen.add(id);
return true;
});
};

/**
* Compute archived collection IDs from their dependencies.
*/
Expand Down
23 changes: 23 additions & 0 deletions web/packages/new/photos/services/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<number>();
return files.filter(({ id }) => {
if (seen.has(id)) return false;
seen.add(id);
return true;
});
};

export const TRASH = "file-trash";

export async function getLocalTrash() {
Expand Down
14 changes: 12 additions & 2 deletions web/packages/new/photos/services/search/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<SearchCollectionsAndFiles, "collectionFiles">) =>
void worker().then((w) =>
w.setCollectionsAndFiles({
collections: collections,
files: uniqueFilesByID(files),
collectionFiles: files,
}),
);

/**
* Set the (named) people that we should search across.
Expand Down
13 changes: 13 additions & 0 deletions web/packages/new/photos/services/search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
16 changes: 8 additions & 8 deletions web/packages/new/photos/services/search/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export class SearchWorker {
private collectionsAndFiles: SearchCollectionsAndFiles = {
collections: [],
files: [],
collectionFiles: [],
};
private people: NamedPerson[] = [];

Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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,
);

Expand Down

0 comments on commit fcfc697

Please sign in to comment.