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 <jrasm91@gmail.com>
Co-authored-by: Alex Tran <alex.tran1502@gmail.com>
Co-authored-by: Jason Rasmussen <jason@rasm.me>
This commit is contained in:
Justin Forseth
2024-08-01 21:27:40 -06:00
committed by GitHub
parent 3afb5b497f
commit d3a5490e71
21 changed files with 378 additions and 217 deletions
+2 -1
View File
@@ -62,6 +62,7 @@ export class SearchController {
@Get('suggestions')
@Authenticated()
getSearchSuggestions(@Auth() auth: AuthDto, @Query() dto: SearchSuggestionRequestDto): Promise<string[]> {
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<string[]>;
}
}
+18 -18
View File
@@ -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 {
+5 -5
View File
@@ -26,9 +26,9 @@ export interface IMetadataRepository {
readTags(path: string): Promise<ImmichTags | null>;
writeTags(path: string, tags: Partial<Tags>): Promise<void>;
extractBinaryTag(tagName: string, path: string): Promise<Buffer>;
getCountries(userId: string): Promise<string[]>;
getStates(userId: string, country?: string): Promise<string[]>;
getCities(userId: string, country?: string, state?: string): Promise<string[]>;
getCameraMakes(userId: string, model?: string): Promise<string[]>;
getCameraModels(userId: string, make?: string): Promise<string[]>;
getCountries(userId: string): Promise<Array<string | null>>;
getStates(userId: string, country?: string): Promise<Array<string | null>>;
getCities(userId: string, country?: string, state?: string): Promise<Array<string | null>>;
getCameraMakes(userId: string, model?: string): Promise<Array<string | null>>;
getCameraModels(userId: string, make?: string): Promise<Array<string | null>>;
}
+6 -6
View File
@@ -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 {
+5 -15
View File
@@ -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
+16 -31
View File
@@ -57,49 +57,42 @@ export class MetadataRepository implements IMetadataRepository {
@GenerateSql({ params: [DummyValue.UUID] })
async getCountries(userId: string): Promise<string[]> {
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<string[]> {
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<string[]> {
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<string[]> {
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<string[]> {
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 !== '');
}
}
@@ -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);
});
});
});
+14 -6
View File
@@ -120,22 +120,30 @@ export class SearchService {
return assets.map((asset) => mapAsset(asset));
}
getSearchSuggestions(auth: AuthDto, dto: SearchSuggestionRequestDto): Promise<string[]> {
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 [];
}
}
}
+7 -1
View File
@@ -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']);
+13 -3
View File
@@ -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 };