From d3a5490e711cdb891241190a3af0586dd55a7743 Mon Sep 17 00:00:00 2001 From: Justin Forseth <51516525+jforseth210@users.noreply.github.com> Date: Thu, 1 Aug 2024 21:27:40 -0600 Subject: [PATCH] feat(server): search unknown place (#10866) * Allow submission of null country * Update searchAssetBuilder to handle nulls andWhere({country:null}) produces `"exifInfo"."country" = NULL`. We want `"exifInfo"."country" IS NULL`, so we have to treat NULL as a special case * Allow null country in frontend * Make the query code a bit more straightforward * Remove unused brackets import * Remove log message * Don't change whitespace for no reason * Fix prettier style issue * Update search.dto.ts validators per @jrasm91's recommendation * Update api types * Combine null country and state into one guard clause * chore: clean up * chore: add e2e for null/empty city, state, country search * refactor: server returns suggestion for null values * chore: clean up --------- Co-authored-by: Jason Rasmussen Co-authored-by: Alex Tran Co-authored-by: Jason Rasmussen --- e2e/src/api/specs/search.e2e-spec.ts | 196 +++++++++++++++--- mobile/openapi/lib/api/search_api.dart | Bin 12851 -> 13180 bytes .../lib/model/metadata_search_dto.dart | Bin 30336 -> 28591 bytes .../openapi/lib/model/smart_search_dto.dart | Bin 22603 -> 20858 bytes open-api/immich-openapi-specs.json | 19 ++ open-api/typescript-sdk/src/fetch-client.ts | 24 ++- server/src/controllers/search.controller.ts | 3 +- server/src/dtos/search.dto.ts | 36 ++-- server/src/interfaces/metadata.interface.ts | 10 +- server/src/interfaces/search.interface.ts | 12 +- server/src/queries/metadata.repository.sql | 20 +- .../src/repositories/metadata.repository.ts | 47 ++--- server/src/services/search.service.spec.ts | 19 ++ server/src/services/search.service.ts | 20 +- server/src/utils/database.ts | 8 +- server/src/validation.ts | 16 +- .../shared-components/combobox.svelte | 14 +- .../search-bar/search-camera-section.svelte | 29 ++- .../search-bar/search-filter-box.svelte | 15 +- .../search-bar/search-location-section.svelte | 30 ++- .../[[assetId=id]]/+page.svelte | 2 + 21 files changed, 366 insertions(+), 154 deletions(-) diff --git a/e2e/src/api/specs/search.e2e-spec.ts b/e2e/src/api/specs/search.e2e-spec.ts index b1116d4d6e..beeaf1cc01 100644 --- a/e2e/src/api/specs/search.e2e-spec.ts +++ b/e2e/src/api/specs/search.e2e-spec.ts @@ -1,4 +1,4 @@ -import { AssetMediaResponseDto, LoginResponseDto, deleteAssets, getMapMarkers, updateAsset } from '@immich/sdk'; +import { AssetMediaResponseDto, LoginResponseDto, deleteAssets, updateAsset } from '@immich/sdk'; import { DateTime } from 'luxon'; import { readFile } from 'node:fs/promises'; import { join } from 'node:path'; @@ -32,9 +32,6 @@ describe('/search', () => { let assetOneJpg5: AssetMediaResponseDto; let assetSprings: AssetMediaResponseDto; let assetLast: AssetMediaResponseDto; - let cities: string[]; - let states: string[]; - let countries: string[]; beforeAll(async () => { await utils.resetDatabase(); @@ -85,7 +82,7 @@ describe('/search', () => { // note: the coordinates here are not the actual coordinates of the images and are random for most of them const coordinates = [ { latitude: 48.853_41, longitude: 2.3488 }, // paris - { latitude: 63.0695, longitude: -151.0074 }, // denali + { latitude: 35.6895, longitude: 139.691_71 }, // tokyo { latitude: 52.524_37, longitude: 13.410_53 }, // berlin { latitude: 1.314_663_1, longitude: 103.845_409_3 }, // singapore { latitude: 41.013_84, longitude: 28.949_66 }, // istanbul @@ -101,16 +98,15 @@ describe('/search', () => { { latitude: 31.634_16, longitude: -7.999_94 }, // marrakesh { latitude: 38.523_735_4, longitude: -78.488_619_4 }, // tanners ridge { latitude: 59.938_63, longitude: 30.314_13 }, // st. petersburg - { latitude: 35.6895, longitude: 139.691_71 }, // tokyo ]; - const updates = assets.map((asset, i) => - updateAsset({ id: asset.id, updateAssetDto: coordinates[i] }, { headers: asBearerAuth(admin.accessToken) }), + const updates = coordinates.map((dto, i) => + updateAsset({ id: assets[i].id, updateAssetDto: dto }, { headers: asBearerAuth(admin.accessToken) }), ); await Promise.all(updates); - for (const asset of assets) { - await utils.waitForWebsocketEvent({ event: 'assetUpdate', id: asset.id }); + for (const [i] of coordinates.entries()) { + await utils.waitForWebsocketEvent({ event: 'assetUpdate', id: assets[i].id }); } [ @@ -137,12 +133,6 @@ describe('/search', () => { assetLast = assets.at(-1) as AssetMediaResponseDto; await deleteAssets({ assetBulkDeleteDto: { ids: [assetSilver.id] } }, { headers: asBearerAuth(admin.accessToken) }); - - const mapMarkers = await getMapMarkers({}, { headers: asBearerAuth(admin.accessToken) }); - const nonTrashed = mapMarkers.filter((mark) => mark.id !== assetSilver.id); - cities = [...new Set(nonTrashed.map((mark) => mark.city).filter((entry): entry is string => !!entry))].sort(); - states = [...new Set(nonTrashed.map((mark) => mark.state).filter((entry): entry is string => !!entry))].sort(); - countries = [...new Set(nonTrashed.map((mark) => mark.country).filter((entry): entry is string => !!entry))].sort(); }, 30_000); afterAll(async () => { @@ -321,23 +311,120 @@ describe('/search', () => { }, { should: 'should search by city', - deferred: () => ({ dto: { city: 'Accra' }, assets: [assetHeic] }), + deferred: () => ({ + dto: { + city: 'Accra', + includeNull: true, + }, + assets: [assetHeic], + }), + }, + { + should: "should search city ('')", + deferred: () => ({ + dto: { + city: '', + isVisible: true, + includeNull: true, + }, + assets: [assetLast], + }), + }, + { + should: 'should search city (null)', + deferred: () => ({ + dto: { + city: null, + isVisible: true, + includeNull: true, + }, + assets: [assetLast], + }), }, { should: 'should search by state', - deferred: () => ({ dto: { state: 'New York' }, assets: [assetDensity] }), + deferred: () => ({ + dto: { + state: 'New York', + includeNull: true, + }, + assets: [assetDensity], + }), + }, + { + should: "should search state ('')", + deferred: () => ({ + dto: { + state: '', + isVisible: true, + withExif: true, + includeNull: true, + }, + assets: [assetLast, assetNotocactus], + }), + }, + { + should: 'should search state (null)', + deferred: () => ({ + dto: { + state: null, + isVisible: true, + includeNull: true, + }, + assets: [assetLast, assetNotocactus], + }), }, { should: 'should search by country', - deferred: () => ({ dto: { country: 'France' }, assets: [assetFalcon] }), + deferred: () => ({ + dto: { + country: 'France', + includeNull: true, + }, + assets: [assetFalcon], + }), + }, + { + should: "should search country ('')", + deferred: () => ({ + dto: { + country: '', + isVisible: true, + includeNull: true, + }, + assets: [assetLast], + }), + }, + { + should: 'should search country (null)', + deferred: () => ({ + dto: { + country: null, + isVisible: true, + includeNull: true, + }, + assets: [assetLast], + }), }, { should: 'should search by make', - deferred: () => ({ dto: { make: 'Canon' }, assets: [assetFalcon, assetDenali] }), + deferred: () => ({ + dto: { + make: 'Canon', + includeNull: true, + }, + assets: [assetFalcon, assetDenali], + }), }, { should: 'should search by model', - deferred: () => ({ dto: { model: 'Canon EOS 7D' }, assets: [assetDenali] }), + deferred: () => ({ + dto: { + model: 'Canon EOS 7D', + includeNull: true, + }, + assets: [assetDenali], + }), }, { should: 'should allow searching the upload library (libraryId: null)', @@ -450,32 +537,79 @@ describe('/search', () => { it('should get suggestions for country', async () => { const { status, body } = await request(app) - .get('/search/suggestions?type=country') + .get('/search/suggestions?type=country&includeNull=true') .set('Authorization', `Bearer ${admin.accessToken}`); - expect(body).toEqual(countries); + expect(body).toEqual([ + 'Cuba', + 'France', + 'Georgia', + 'Germany', + 'Ghana', + 'Japan', + 'Morocco', + "People's Republic of China", + 'Russian Federation', + 'Singapore', + 'Spain', + 'Switzerland', + 'United States of America', + null, + ]); expect(status).toBe(200); }); it('should get suggestions for state', async () => { const { status, body } = await request(app) - .get('/search/suggestions?type=state') + .get('/search/suggestions?type=state&includeNull=true') .set('Authorization', `Bearer ${admin.accessToken}`); - expect(body).toHaveLength(states.length); - expect(body).toEqual(expect.arrayContaining(states)); + expect(body).toEqual([ + 'Andalusia', + 'Berlin', + 'Glarus', + 'Greater Accra', + 'Havana', + 'Île-de-France', + 'Marrakesh-Safi', + 'Mississippi', + 'New York', + 'Shanghai', + 'St.-Petersburg', + 'Tbilisi', + 'Tokyo', + 'Virginia', + null, + ]); expect(status).toBe(200); }); it('should get suggestions for city', async () => { const { status, body } = await request(app) - .get('/search/suggestions?type=city') + .get('/search/suggestions?type=city&includeNull=true') .set('Authorization', `Bearer ${admin.accessToken}`); - expect(body).toEqual(cities); + expect(body).toEqual([ + 'Accra', + 'Berlin', + 'Glarus', + 'Havana', + 'Marrakesh', + 'Montalbán de Córdoba', + 'New York City', + 'Novena', + 'Paris', + 'Philadelphia', + 'Saint Petersburg', + 'Shanghai', + 'Stanley', + 'Tbilisi', + 'Tokyo', + null, + ]); expect(status).toBe(200); }); it('should get suggestions for camera make', async () => { const { status, body } = await request(app) - .get('/search/suggestions?type=camera-make') + .get('/search/suggestions?type=camera-make&includeNull=true') .set('Authorization', `Bearer ${admin.accessToken}`); expect(body).toEqual([ 'Apple', @@ -485,13 +619,14 @@ describe('/search', () => { 'PENTAX Corporation', 'samsung', 'SONY', + null, ]); expect(status).toBe(200); }); it('should get suggestions for camera model', async () => { const { status, body } = await request(app) - .get('/search/suggestions?type=camera-model') + .get('/search/suggestions?type=camera-model&includeNull=true') .set('Authorization', `Bearer ${admin.accessToken}`); expect(body).toEqual([ 'Canon EOS 7D', @@ -506,6 +641,7 @@ describe('/search', () => { 'SM-F711N', 'SM-S906U', 'SM-T970', + null, ]); expect(status).toBe(200); }); diff --git a/mobile/openapi/lib/api/search_api.dart b/mobile/openapi/lib/api/search_api.dart index 21af2d57cb837e141a087d3dc40f9c6440b5d93e..4b6cdfea78aa491d4417ebdd0e39672c0fd67f4a 100644 GIT binary patch delta 275 zcmdm-@+WPBET3Icetu4@LS|laPH9T2UujN`6_R8k&T$VU)XlCorJ??7k*<#ZX~lO>kFDo mBJtIAkhN`271+*1xr34?=L-j6byS}OlHFD)R(p%QkOcsa|6aua delta 52 zcmey9wmD^kEZ-z`j>!%p-jnz7MNa-8Vm&!t-C^@meq+YXu7XRMAd+)|5%a!ih9MmfAY*BCc{>WT%LbJWv%MCY0kgCjTm!Rh9^N^G Cy%8?} delta 78 zcmeyhi1G9W#tqCYn@t5{87J$Da7^ZA;e>NG%dm9uA$aSACNaWzoSQvGf|+4Fj?GKN HuUP^Bm?9S1 diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 5a327f1d34..6506a6293f 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -4727,6 +4727,15 @@ "type": "string" } }, + { + "name": "includeNull", + "required": false, + "in": "query", + "description": "This property was added in v111.0.0", + "schema": { + "type": "boolean" + } + }, { "name": "make", "required": false, @@ -9378,9 +9387,11 @@ "type": "string" }, "city": { + "nullable": true, "type": "string" }, "country": { + "nullable": true, "type": "string" }, "createdAfter": { @@ -9426,6 +9437,7 @@ "type": "boolean" }, "lensModel": { + "nullable": true, "type": "string" }, "libraryId": { @@ -9437,6 +9449,7 @@ "type": "string" }, "model": { + "nullable": true, "type": "string" }, "order": { @@ -9468,6 +9481,7 @@ "type": "number" }, "state": { + "nullable": true, "type": "string" }, "takenAfter": { @@ -10611,9 +10625,11 @@ "SmartSearchDto": { "properties": { "city": { + "nullable": true, "type": "string" }, "country": { + "nullable": true, "type": "string" }, "createdAfter": { @@ -10649,6 +10665,7 @@ "type": "boolean" }, "lensModel": { + "nullable": true, "type": "string" }, "libraryId": { @@ -10660,6 +10677,7 @@ "type": "string" }, "model": { + "nullable": true, "type": "string" }, "page": { @@ -10682,6 +10700,7 @@ "type": "number" }, "state": { + "nullable": true, "type": "string" }, "takenAfter": { diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index 85575893f0..ec2a230f77 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -708,8 +708,8 @@ export type SearchExploreResponseDto = { }; export type MetadataSearchDto = { checksum?: string; - city?: string; - country?: string; + city?: string | null; + country?: string | null; createdAfter?: string; createdBefore?: string; deviceAssetId?: string; @@ -723,10 +723,10 @@ export type MetadataSearchDto = { isNotInAlbum?: boolean; isOffline?: boolean; isVisible?: boolean; - lensModel?: string; + lensModel?: string | null; libraryId?: string | null; make?: string; - model?: string; + model?: string | null; order?: AssetOrder; originalFileName?: string; originalPath?: string; @@ -734,7 +734,7 @@ export type MetadataSearchDto = { personIds?: string[]; previewPath?: string; size?: number; - state?: string; + state?: string | null; takenAfter?: string; takenBefore?: string; thumbnailPath?: string; @@ -782,8 +782,8 @@ export type PlacesResponseDto = { name: string; }; export type SmartSearchDto = { - city?: string; - country?: string; + city?: string | null; + country?: string | null; createdAfter?: string; createdBefore?: string; deviceId?: string; @@ -794,15 +794,15 @@ export type SmartSearchDto = { isNotInAlbum?: boolean; isOffline?: boolean; isVisible?: boolean; - lensModel?: string; + lensModel?: string | null; libraryId?: string | null; make?: string; - model?: string; + model?: string | null; page?: number; personIds?: string[]; query: string; size?: number; - state?: string; + state?: string | null; takenAfter?: string; takenBefore?: string; trashedAfter?: string; @@ -2418,8 +2418,9 @@ export function searchSmart({ smartSearchDto }: { body: smartSearchDto }))); } -export function getSearchSuggestions({ country, make, model, state, $type }: { +export function getSearchSuggestions({ country, includeNull, make, model, state, $type }: { country?: string; + includeNull?: boolean; make?: string; model?: string; state?: string; @@ -2430,6 +2431,7 @@ export function getSearchSuggestions({ country, make, model, state, $type }: { data: string[]; }>(`/search/suggestions${QS.query(QS.explode({ country, + includeNull, make, model, state, diff --git a/server/src/controllers/search.controller.ts b/server/src/controllers/search.controller.ts index 688ff1c138..5b8c1eeece 100644 --- a/server/src/controllers/search.controller.ts +++ b/server/src/controllers/search.controller.ts @@ -62,6 +62,7 @@ export class SearchController { @Get('suggestions') @Authenticated() getSearchSuggestions(@Auth() auth: AuthDto, @Query() dto: SearchSuggestionRequestDto): Promise { - return this.service.getSearchSuggestions(auth, dto); + // TODO fix open api generation to indicate that results can be nullable + return this.service.getSearchSuggestions(auth, dto) as Promise; } } diff --git a/server/src/dtos/search.dto.ts b/server/src/dtos/search.dto.ts index 0874300d5f..b81321b873 100644 --- a/server/src/dtos/search.dto.ts +++ b/server/src/dtos/search.dto.ts @@ -1,6 +1,7 @@ import { ApiProperty } from '@nestjs/swagger'; import { Type } from 'class-transformer'; import { IsEnum, IsInt, IsNotEmpty, IsString, Max, Min } from 'class-validator'; +import { PropertyLifecycle } from 'src/decorators'; import { AlbumResponseDto } from 'src/dtos/album.dto'; import { AssetResponseDto } from 'src/dtos/asset-response.dto'; import { AssetOrder } from 'src/entities/album.entity'; @@ -75,34 +76,29 @@ class BaseSearchDto { takenAfter?: Date; @IsString() - @IsNotEmpty() - @Optional() - city?: string; + @Optional({ nullable: true, emptyToNull: true }) + city?: string | null; + + @IsString() + @Optional({ nullable: true, emptyToNull: true }) + state?: string | null; @IsString() @IsNotEmpty() - @Optional() - state?: string; + @Optional({ nullable: true, emptyToNull: true }) + country?: string | null; @IsString() - @IsNotEmpty() - @Optional() - country?: string; - - @IsString() - @IsNotEmpty() - @Optional() + @Optional({ nullable: true, emptyToNull: true }) make?: string; @IsString() - @IsNotEmpty() - @Optional() - model?: string; + @Optional({ nullable: true, emptyToNull: true }) + model?: string | null; @IsString() - @IsNotEmpty() - @Optional() - lensModel?: string; + @Optional({ nullable: true, emptyToNull: true }) + lensModel?: string | null; @IsInt() @Min(1) @@ -242,6 +238,10 @@ export class SearchSuggestionRequestDto { @IsString() @Optional() model?: string; + + @ValidateBoolean({ optional: true }) + @PropertyLifecycle({ addedAt: 'v111.0.0' }) + includeNull?: boolean; } class SearchFacetCountResponseDto { diff --git a/server/src/interfaces/metadata.interface.ts b/server/src/interfaces/metadata.interface.ts index 1ccd704b59..daba4184e3 100644 --- a/server/src/interfaces/metadata.interface.ts +++ b/server/src/interfaces/metadata.interface.ts @@ -26,9 +26,9 @@ export interface IMetadataRepository { readTags(path: string): Promise; writeTags(path: string, tags: Partial): Promise; extractBinaryTag(tagName: string, path: string): Promise; - getCountries(userId: string): Promise; - getStates(userId: string, country?: string): Promise; - getCities(userId: string, country?: string, state?: string): Promise; - getCameraMakes(userId: string, model?: string): Promise; - getCameraModels(userId: string, make?: string): Promise; + getCountries(userId: string): Promise>; + getStates(userId: string, country?: string): Promise>; + getCities(userId: string, country?: string, state?: string): Promise>; + getCameraMakes(userId: string, model?: string): Promise>; + getCameraModels(userId: string, make?: string): Promise>; } diff --git a/server/src/interfaces/search.interface.ts b/server/src/interfaces/search.interface.ts index c84b56c62e..c17f833615 100644 --- a/server/src/interfaces/search.interface.ts +++ b/server/src/interfaces/search.interface.ts @@ -95,12 +95,12 @@ export interface SearchPathOptions { } export interface SearchExifOptions { - city?: string; - country?: string; - lensModel?: string; - make?: string; - model?: string; - state?: string; + city?: string | null; + country?: string | null; + lensModel?: string | null; + make?: string | null; + model?: string | null; + state?: string | null; } export interface SearchEmbeddingOptions { diff --git a/server/src/queries/metadata.repository.sql b/server/src/queries/metadata.repository.sql index bed7d59ab6..077b4644b8 100644 --- a/server/src/queries/metadata.repository.sql +++ b/server/src/queries/metadata.repository.sql @@ -2,65 +2,55 @@ -- MetadataRepository.getCountries SELECT DISTINCT - ON ("exif"."country") "exif"."country" AS "exif_country", - "exif"."assetId" AS "exif_assetId" + ON ("exif"."country") "exif"."country" AS "country" FROM "exif" "exif" LEFT JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" AND ("asset"."deletedAt" IS NULL) WHERE "asset"."ownerId" = $1 - AND "exif"."country" IS NOT NULL -- MetadataRepository.getStates SELECT DISTINCT - ON ("exif"."state") "exif"."state" AS "exif_state", - "exif"."assetId" AS "exif_assetId" + ON ("exif"."state") "exif"."state" AS "state" FROM "exif" "exif" LEFT JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" AND ("asset"."deletedAt" IS NULL) WHERE "asset"."ownerId" = $1 - AND "exif"."state" IS NOT NULL AND "exif"."country" = $2 -- MetadataRepository.getCities SELECT DISTINCT - ON ("exif"."city") "exif"."city" AS "exif_city", - "exif"."assetId" AS "exif_assetId" + ON ("exif"."city") "exif"."city" AS "city" FROM "exif" "exif" LEFT JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" AND ("asset"."deletedAt" IS NULL) WHERE "asset"."ownerId" = $1 - AND "exif"."city" IS NOT NULL AND "exif"."country" = $2 AND "exif"."state" = $3 -- MetadataRepository.getCameraMakes SELECT DISTINCT - ON ("exif"."make") "exif"."make" AS "exif_make", - "exif"."assetId" AS "exif_assetId" + ON ("exif"."make") "exif"."make" AS "make" FROM "exif" "exif" LEFT JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" AND ("asset"."deletedAt" IS NULL) WHERE "asset"."ownerId" = $1 - AND "exif"."make" IS NOT NULL AND "exif"."model" = $2 -- MetadataRepository.getCameraModels SELECT DISTINCT - ON ("exif"."model") "exif"."model" AS "exif_model", - "exif"."assetId" AS "exif_assetId" + ON ("exif"."model") "exif"."model" AS "model" FROM "exif" "exif" LEFT JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" AND ("asset"."deletedAt" IS NULL) WHERE "asset"."ownerId" = $1 - AND "exif"."model" IS NOT NULL AND "exif"."make" = $2 diff --git a/server/src/repositories/metadata.repository.ts b/server/src/repositories/metadata.repository.ts index 3ca088e2d7..832cffbee6 100644 --- a/server/src/repositories/metadata.repository.ts +++ b/server/src/repositories/metadata.repository.ts @@ -57,49 +57,42 @@ export class MetadataRepository implements IMetadataRepository { @GenerateSql({ params: [DummyValue.UUID] }) async getCountries(userId: string): Promise { - const entity = await this.exifRepository + const results = await this.exifRepository .createQueryBuilder('exif') .leftJoin('exif.asset', 'asset') .where('asset.ownerId = :userId', { userId }) - .andWhere('exif.country IS NOT NULL') - .select('exif.country') + .select('exif.country', 'country') .distinctOn(['exif.country']) - .getMany(); + .getRawMany<{ country: string }>(); - return entity.map((e) => e.country ?? '').filter((c) => c !== ''); + return results.map(({ country }) => country).filter((item) => item !== ''); } @GenerateSql({ params: [DummyValue.UUID, DummyValue.STRING] }) async getStates(userId: string, country: string | undefined): Promise { - let result: ExifEntity[] = []; - const query = this.exifRepository .createQueryBuilder('exif') .leftJoin('exif.asset', 'asset') .where('asset.ownerId = :userId', { userId }) - .andWhere('exif.state IS NOT NULL') - .select('exif.state') + .select('exif.state', 'state') .distinctOn(['exif.state']); if (country) { query.andWhere('exif.country = :country', { country }); } - result = await query.getMany(); + const result = await query.getRawMany<{ state: string }>(); - return result.map((entity) => entity.state ?? '').filter((s) => s !== ''); + return result.map(({ state }) => state).filter((item) => item !== ''); } @GenerateSql({ params: [DummyValue.UUID, DummyValue.STRING, DummyValue.STRING] }) async getCities(userId: string, country: string | undefined, state: string | undefined): Promise { - let result: ExifEntity[] = []; - const query = this.exifRepository .createQueryBuilder('exif') .leftJoin('exif.asset', 'asset') .where('asset.ownerId = :userId', { userId }) - .andWhere('exif.city IS NOT NULL') - .select('exif.city') + .select('exif.city', 'city') .distinctOn(['exif.city']); if (country) { @@ -110,50 +103,42 @@ export class MetadataRepository implements IMetadataRepository { query.andWhere('exif.state = :state', { state }); } - result = await query.getMany(); + const results = await query.getRawMany<{ city: string }>(); - return result.map((entity) => entity.city ?? '').filter((c) => c !== ''); + return results.map(({ city }) => city).filter((item) => item !== ''); } @GenerateSql({ params: [DummyValue.UUID, DummyValue.STRING] }) async getCameraMakes(userId: string, model: string | undefined): Promise { - let result: ExifEntity[] = []; - const query = this.exifRepository .createQueryBuilder('exif') .leftJoin('exif.asset', 'asset') .where('asset.ownerId = :userId', { userId }) - .andWhere('exif.make IS NOT NULL') - .select('exif.make') + .select('exif.make', 'make') .distinctOn(['exif.make']); if (model) { query.andWhere('exif.model = :model', { model }); } - result = await query.getMany(); - - return result.map((entity) => entity.make ?? '').filter((m) => m !== ''); + const results = await query.getRawMany<{ make: string }>(); + return results.map(({ make }) => make).filter((item) => item !== ''); } @GenerateSql({ params: [DummyValue.UUID, DummyValue.STRING] }) async getCameraModels(userId: string, make: string | undefined): Promise { - let result: ExifEntity[] = []; - const query = this.exifRepository .createQueryBuilder('exif') .leftJoin('exif.asset', 'asset') .where('asset.ownerId = :userId', { userId }) - .andWhere('exif.model IS NOT NULL') - .select('exif.model') + .select('exif.model', 'model') .distinctOn(['exif.model']); if (make) { query.andWhere('exif.make = :make', { make }); } - result = await query.getMany(); - - return result.map((entity) => entity.model ?? '').filter((m) => m !== ''); + const results = await query.getRawMany<{ model: string }>(); + return results.map(({ model }) => model).filter((item) => item !== ''); } } diff --git a/server/src/services/search.service.spec.ts b/server/src/services/search.service.spec.ts index afc98b69de..89609d5d89 100644 --- a/server/src/services/search.service.spec.ts +++ b/server/src/services/search.service.spec.ts @@ -1,4 +1,5 @@ import { mapAsset } from 'src/dtos/asset-response.dto'; +import { SearchSuggestionType } from 'src/dtos/search.dto'; import { IAssetRepository } from 'src/interfaces/asset.interface'; import { ILoggerRepository } from 'src/interfaces/logger.interface'; import { IMachineLearningRepository } from 'src/interfaces/machine-learning.interface'; @@ -95,4 +96,22 @@ describe(SearchService.name, () => { expect(result).toEqual(expectedResponse); }); }); + + describe('getSearchSuggestions', () => { + it('should return search suggestions (including null)', async () => { + metadataMock.getCountries.mockResolvedValue(['USA', null]); + await expect( + sut.getSearchSuggestions(authStub.user1, { includeNull: true, type: SearchSuggestionType.COUNTRY }), + ).resolves.toEqual(['USA', null]); + expect(metadataMock.getCountries).toHaveBeenCalledWith(authStub.user1.user.id); + }); + + it('should return search suggestions (without null)', async () => { + metadataMock.getCountries.mockResolvedValue(['USA', null]); + await expect( + sut.getSearchSuggestions(authStub.user1, { includeNull: false, type: SearchSuggestionType.COUNTRY }), + ).resolves.toEqual(['USA']); + expect(metadataMock.getCountries).toHaveBeenCalledWith(authStub.user1.user.id); + }); + }); }); diff --git a/server/src/services/search.service.ts b/server/src/services/search.service.ts index 5010067a3f..1d746a03d8 100644 --- a/server/src/services/search.service.ts +++ b/server/src/services/search.service.ts @@ -120,22 +120,30 @@ export class SearchService { return assets.map((asset) => mapAsset(asset)); } - getSearchSuggestions(auth: AuthDto, dto: SearchSuggestionRequestDto): Promise { + async getSearchSuggestions(auth: AuthDto, dto: SearchSuggestionRequestDto) { + const results = await this.getSuggestions(auth.user.id, dto); + return results.filter((result) => (dto.includeNull ? true : result !== null)); + } + + private getSuggestions(userId: string, dto: SearchSuggestionRequestDto) { switch (dto.type) { case SearchSuggestionType.COUNTRY: { - return this.metadataRepository.getCountries(auth.user.id); + return this.metadataRepository.getCountries(userId); } case SearchSuggestionType.STATE: { - return this.metadataRepository.getStates(auth.user.id, dto.country); + return this.metadataRepository.getStates(userId, dto.country); } case SearchSuggestionType.CITY: { - return this.metadataRepository.getCities(auth.user.id, dto.country, dto.state); + return this.metadataRepository.getCities(userId, dto.country, dto.state); } case SearchSuggestionType.CAMERA_MAKE: { - return this.metadataRepository.getCameraMakes(auth.user.id, dto.model); + return this.metadataRepository.getCameraMakes(userId, dto.model); } case SearchSuggestionType.CAMERA_MODEL: { - return this.metadataRepository.getCameraModels(auth.user.id, dto.make); + return this.metadataRepository.getCameraModels(userId, dto.make); + } + default: { + return []; } } } diff --git a/server/src/utils/database.ts b/server/src/utils/database.ts index 944978bddd..ebf27270d5 100644 --- a/server/src/utils/database.ts +++ b/server/src/utils/database.ts @@ -48,7 +48,13 @@ export function searchAssetBuilder( ? builder.leftJoinAndSelect(`${builder.alias}.exifInfo`, 'exifInfo') : builder.leftJoin(`${builder.alias}.exifInfo`, 'exifInfo'); - builder.andWhere({ exifInfo }); + for (const [key, value] of Object.entries(exifInfo)) { + if (value === null) { + builder.andWhere(`exifInfo.${key} IS NULL`); + } else { + builder.andWhere(`exifInfo.${key} = :${key}`, { [key]: value }); + } + } } const id = _.pick(options, ['checksum', 'deviceAssetId', 'deviceId', 'id', 'libraryId']); diff --git a/server/src/validation.ts b/server/src/validation.ts index 063f2150a5..10f2e7b77b 100644 --- a/server/src/validation.ts +++ b/server/src/validation.ts @@ -66,6 +66,8 @@ export class UUIDParamDto { export interface OptionalOptions extends ValidationOptions { nullable?: boolean; + /** convert empty strings to null */ + emptyToNull?: boolean; } /** @@ -76,12 +78,20 @@ export interface OptionalOptions extends ValidationOptions { * @see IsOptional exported from `class-validator. */ // https://stackoverflow.com/a/71353929 -export function Optional({ nullable, ...validationOptions }: OptionalOptions = {}) { +export function Optional({ nullable, emptyToNull, ...validationOptions }: OptionalOptions = {}) { + const decorators: PropertyDecorator[] = []; + if (nullable === true) { - return IsOptional(validationOptions); + decorators.push(IsOptional(validationOptions)); + } else { + decorators.push(ValidateIf((object: any, v: any) => v !== undefined, validationOptions)); } - return ValidateIf((object: any, v: any) => v !== undefined, validationOptions); + if (emptyToNull) { + decorators.push(Transform(({ value }) => (value === '' ? null : value))); + } + + return applyDecorators(...decorators); } type UUIDOptions = { optional?: boolean; each?: boolean; nullable?: boolean }; diff --git a/web/src/lib/components/shared-components/combobox.svelte b/web/src/lib/components/shared-components/combobox.svelte index f087a6e18f..7cdcef9e40 100644 --- a/web/src/lib/components/shared-components/combobox.svelte +++ b/web/src/lib/components/shared-components/combobox.svelte @@ -4,9 +4,16 @@ value: string; }; - export function toComboBoxOptions(items: string[]) { - return items.map((item) => ({ label: item, value: item })); - } + export const asComboboxOptions = (values: string[]) => + values.map((value) => { + if (value === '') { + return { label: get(t)('unknown'), value: '' }; + } + + return { label: value, value }; + }); + + export const asSelectedOption = (value?: string) => (value === undefined ? undefined : asComboboxOptions([value])[0]); @@ -44,9 +57,9 @@ (filters.make = detail?.value)} - options={toComboBoxOptions(makes)} + options={asComboboxOptions(makes)} placeholder={$t('search_camera_make')} - selectedOption={makeFilter ? { label: makeFilter, value: makeFilter } : undefined} + selectedOption={asSelectedOption(makeFilter)} /> @@ -54,9 +67,9 @@ (filters.model = detail?.value)} - options={toComboBoxOptions(models)} + options={asComboboxOptions(models)} placeholder={$t('search_camera_model')} - selectedOption={modelFilter ? { label: modelFilter, value: modelFilter } : undefined} + selectedOption={asSelectedOption(modelFilter)} /> diff --git a/web/src/lib/components/shared-components/search-bar/search-filter-box.svelte b/web/src/lib/components/shared-components/search-bar/search-filter-box.svelte index 5fa92ac7b2..35e7ea7535 100644 --- a/web/src/lib/components/shared-components/search-bar/search-filter-box.svelte +++ b/web/src/lib/components/shared-components/search-bar/search-filter-box.svelte @@ -42,18 +42,23 @@ const toStartOfDayDate = (dateString: string) => parseUtcDate(dateString)?.startOf('day').toISODate() || undefined; const dispatch = createEventDispatcher<{ search: SmartSearchDto | MetadataSearchDto }>(); + // combobox and all the search components have terrible support for value | null so we use empty string instead. + function withNullAsUndefined(value: T | null) { + return value === null ? undefined : value; + } + let filter: SearchFilter = { context: 'query' in searchQuery ? searchQuery.query : '', filename: 'originalFileName' in searchQuery ? searchQuery.originalFileName : undefined, personIds: new Set('personIds' in searchQuery ? searchQuery.personIds : []), location: { - country: searchQuery.country, - state: searchQuery.state, - city: searchQuery.city, + country: withNullAsUndefined(searchQuery.country), + state: withNullAsUndefined(searchQuery.state), + city: withNullAsUndefined(searchQuery.city), }, camera: { - make: searchQuery.make, - model: searchQuery.model, + make: withNullAsUndefined(searchQuery.make), + model: withNullAsUndefined(searchQuery.model), }, date: { takenAfter: searchQuery.takenAfter ? toStartOfDayDate(searchQuery.takenAfter) : undefined, diff --git a/web/src/lib/components/shared-components/search-bar/search-location-section.svelte b/web/src/lib/components/shared-components/search-bar/search-location-section.svelte index 075a305cef..4ac59bb374 100644 --- a/web/src/lib/components/shared-components/search-bar/search-location-section.svelte +++ b/web/src/lib/components/shared-components/search-bar/search-location-section.svelte @@ -7,9 +7,9 @@