diff --git a/e2e/src/api/specs/search.e2e-spec.ts b/e2e/src/api/specs/search.e2e-spec.ts index 627fbb3e9e..11bb37be18 100644 --- a/e2e/src/api/specs/search.e2e-spec.ts +++ b/e2e/src/api/specs/search.e2e-spec.ts @@ -98,6 +98,7 @@ 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: 0, longitude: 0 }, // null island ]; const updates = coordinates.map((dto, i) => @@ -532,7 +533,7 @@ describe('/search', () => { expect(body).toEqual(errorDto.unauthorized); }); - it('should get suggestions for country', async () => { + it('should get suggestions for country (including null)', async () => { const { status, body } = await request(app) .get('/search/suggestions?type=country&includeNull=true') .set('Authorization', `Bearer ${admin.accessToken}`); @@ -555,7 +556,29 @@ describe('/search', () => { expect(status).toBe(200); }); - it('should get suggestions for state', async () => { + it('should get suggestions for country', async () => { + const { status, body } = await request(app) + .get('/search/suggestions?type=country') + .set('Authorization', `Bearer ${admin.accessToken}`); + expect(body).toEqual([ + 'Cuba', + 'France', + 'Georgia', + 'Germany', + 'Ghana', + 'Japan', + 'Morocco', + "People's Republic of China", + 'Russian Federation', + 'Singapore', + 'Spain', + 'Switzerland', + 'United States of America', + ]); + expect(status).toBe(200); + }); + + it('should get suggestions for state (including null)', async () => { const { status, body } = await request(app) .get('/search/suggestions?type=state&includeNull=true') .set('Authorization', `Bearer ${admin.accessToken}`); @@ -579,7 +602,30 @@ describe('/search', () => { expect(status).toBe(200); }); - it('should get suggestions for city', async () => { + it('should get suggestions for state', async () => { + const { status, body } = await request(app) + .get('/search/suggestions?type=state') + .set('Authorization', `Bearer ${admin.accessToken}`); + expect(body).toEqual([ + 'Andalusia', + 'Berlin', + 'Glarus', + 'Greater Accra', + 'Havana', + 'Île-de-France', + 'Marrakesh-Safi', + 'Mississippi', + 'New York', + 'Shanghai', + 'St.-Petersburg', + 'Tbilisi', + 'Tokyo', + 'Virginia', + ]); + expect(status).toBe(200); + }); + + it('should get suggestions for city (including null)', async () => { const { status, body } = await request(app) .get('/search/suggestions?type=city&includeNull=true') .set('Authorization', `Bearer ${admin.accessToken}`); @@ -604,7 +650,31 @@ describe('/search', () => { expect(status).toBe(200); }); - it('should get suggestions for camera make', async () => { + it('should get suggestions for city', async () => { + const { status, body } = await request(app) + .get('/search/suggestions?type=city') + .set('Authorization', `Bearer ${admin.accessToken}`); + 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', + ]); + expect(status).toBe(200); + }); + + it('should get suggestions for camera make (including null)', async () => { const { status, body } = await request(app) .get('/search/suggestions?type=camera-make&includeNull=true') .set('Authorization', `Bearer ${admin.accessToken}`); @@ -621,7 +691,23 @@ describe('/search', () => { expect(status).toBe(200); }); - it('should get suggestions for camera model', async () => { + it('should get suggestions for camera make', async () => { + const { status, body } = await request(app) + .get('/search/suggestions?type=camera-make') + .set('Authorization', `Bearer ${admin.accessToken}`); + expect(body).toEqual([ + 'Apple', + 'Canon', + 'FUJIFILM', + 'NIKON CORPORATION', + 'PENTAX Corporation', + 'samsung', + 'SONY', + ]); + expect(status).toBe(200); + }); + + it('should get suggestions for camera model (including null)', async () => { const { status, body } = await request(app) .get('/search/suggestions?type=camera-model&includeNull=true') .set('Authorization', `Bearer ${admin.accessToken}`); @@ -642,5 +728,26 @@ describe('/search', () => { ]); expect(status).toBe(200); }); + + it('should get suggestions for camera model', async () => { + const { status, body } = await request(app) + .get('/search/suggestions?type=camera-model') + .set('Authorization', `Bearer ${admin.accessToken}`); + expect(body).toEqual([ + 'Canon EOS 7D', + 'Canon EOS R5', + 'DSLR-A550', + 'FinePix S3Pro', + 'iPhone 7', + 'NIKON D700', + 'NIKON D750', + 'NIKON D80', + 'PENTAX K10D', + 'SM-F711N', + 'SM-S906U', + 'SM-T970', + ]); + expect(status).toBe(200); + }); }); }); diff --git a/server/src/interfaces/search.interface.ts b/server/src/interfaces/search.interface.ts index 87bf1bc4b1..d59291c883 100644 --- a/server/src/interfaces/search.interface.ts +++ b/server/src/interfaces/search.interface.ts @@ -170,6 +170,22 @@ export interface AssetDuplicateResult { distance: number; } +export interface GetStatesOptions { + country?: string; +} + +export interface GetCitiesOptions extends GetStatesOptions { + state?: string; +} + +export interface GetCameraModelsOptions { + make?: string; +} + +export interface GetCameraMakesOptions { + model?: string; +} + export interface ISearchRepository { searchMetadata(pagination: SearchPaginationOptions, options: AssetSearchOptions): Paginated<AssetEntity>; searchSmart(pagination: SearchPaginationOptions, options: SmartSearchOptions): Paginated<AssetEntity>; @@ -183,8 +199,8 @@ export interface ISearchRepository { getDimensionSize(): Promise<number>; setDimensionSize(dimSize: number): Promise<void>; getCountries(userIds: string[]): Promise<Array<string | null>>; - getStates(userIds: string[], country?: string): Promise<Array<string | null>>; - getCities(userIds: string[], country?: string, state?: string): Promise<Array<string | null>>; - getCameraMakes(userIds: string[], model?: string): Promise<Array<string | null>>; - getCameraModels(userIds: string[], make?: string): Promise<Array<string | null>>; + getStates(userIds: string[], options: GetStatesOptions): Promise<Array<string | null>>; + getCities(userIds: string[], options: GetCitiesOptions): Promise<Array<string | null>>; + getCameraMakes(userIds: string[], options: GetCameraMakesOptions): Promise<Array<string | null>>; + getCameraModels(userIds: string[], options: GetCameraModelsOptions): Promise<Array<string | null>>; } diff --git a/server/src/queries/search.repository.sql b/server/src/queries/search.repository.sql index 7de61ad03c..1084375059 100644 --- a/server/src/queries/search.repository.sql +++ b/server/src/queries/search.repository.sql @@ -585,52 +585,57 @@ SELECT DISTINCT ON ("exif"."country") "exif"."country" AS "country" FROM "exif" "exif" - LEFT JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" + INNER JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" AND ("asset"."deletedAt" IS NULL) WHERE "asset"."ownerId" IN ($1) + AND "exif"."country" != '' + AND "exif"."country" IS NOT NULL -- SearchRepository.getStates SELECT DISTINCT ON ("exif"."state") "exif"."state" AS "state" FROM "exif" "exif" - LEFT JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" + INNER JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" AND ("asset"."deletedAt" IS NULL) WHERE "asset"."ownerId" IN ($1) - AND "exif"."country" = $2 + AND "exif"."state" != '' + AND "exif"."state" IS NOT NULL -- SearchRepository.getCities SELECT DISTINCT ON ("exif"."city") "exif"."city" AS "city" FROM "exif" "exif" - LEFT JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" + INNER JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" AND ("asset"."deletedAt" IS NULL) WHERE "asset"."ownerId" IN ($1) - AND "exif"."country" = $2 - AND "exif"."state" = $3 + AND "exif"."city" != '' + AND "exif"."city" IS NOT NULL -- SearchRepository.getCameraMakes SELECT DISTINCT ON ("exif"."make") "exif"."make" AS "make" FROM "exif" "exif" - LEFT JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" + INNER JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" AND ("asset"."deletedAt" IS NULL) WHERE "asset"."ownerId" IN ($1) - AND "exif"."model" = $2 + AND "exif"."make" != '' + AND "exif"."make" IS NOT NULL -- SearchRepository.getCameraModels SELECT DISTINCT ON ("exif"."model") "exif"."model" AS "model" FROM "exif" "exif" - LEFT JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" + INNER JOIN "assets" "asset" ON "asset"."id" = "exif"."assetId" AND ("asset"."deletedAt" IS NULL) WHERE "asset"."ownerId" IN ($1) - AND "exif"."make" = $2 + AND "exif"."model" != '' + AND "exif"."model" IS NOT NULL diff --git a/server/src/repositories/search.repository.ts b/server/src/repositories/search.repository.ts index ba7d779e02..0a529f2f6e 100644 --- a/server/src/repositories/search.repository.ts +++ b/server/src/repositories/search.repository.ts @@ -17,6 +17,10 @@ import { AssetSearchOptions, FaceEmbeddingSearch, FaceSearchResult, + GetCameraMakesOptions, + GetCameraModelsOptions, + GetCitiesOptions, + GetStatesOptions, ISearchRepository, SearchPaginationOptions, SmartSearchOptions, @@ -342,23 +346,27 @@ export class SearchRepository implements ISearchRepository { @GenerateSql({ params: [[DummyValue.UUID]] }) async getCountries(userIds: string[]): Promise<string[]> { - const results = await this.exifRepository + const query = this.exifRepository .createQueryBuilder('exif') - .leftJoin('exif.asset', 'asset') + .innerJoin('exif.asset', 'asset') .where('asset.ownerId IN (:...userIds )', { userIds }) + .andWhere(`exif.country != ''`) + .andWhere('exif.country IS NOT NULL') .select('exif.country', 'country') - .distinctOn(['exif.country']) - .getRawMany<{ country: string }>(); + .distinctOn(['exif.country']); - return results.map(({ country }) => country).filter((item) => item !== ''); + const results = await query.getRawMany<{ country: string }>(); + return results.map(({ country }) => country); } @GenerateSql({ params: [[DummyValue.UUID], DummyValue.STRING] }) - async getStates(userIds: string[], country: string | undefined): Promise<string[]> { + async getStates(userIds: string[], { country }: GetStatesOptions): Promise<string[]> { const query = this.exifRepository .createQueryBuilder('exif') - .leftJoin('exif.asset', 'asset') + .innerJoin('exif.asset', 'asset') .where('asset.ownerId IN (:...userIds )', { userIds }) + .andWhere(`exif.state != ''`) + .andWhere('exif.state IS NOT NULL') .select('exif.state', 'state') .distinctOn(['exif.state']); @@ -367,16 +375,17 @@ export class SearchRepository implements ISearchRepository { } const result = await query.getRawMany<{ state: string }>(); - - return result.map(({ state }) => state).filter((item) => item !== ''); + return result.map(({ state }) => state); } @GenerateSql({ params: [[DummyValue.UUID], DummyValue.STRING, DummyValue.STRING] }) - async getCities(userIds: string[], country: string | undefined, state: string | undefined): Promise<string[]> { + async getCities(userIds: string[], { country, state }: GetCitiesOptions): Promise<string[]> { const query = this.exifRepository .createQueryBuilder('exif') - .leftJoin('exif.asset', 'asset') + .innerJoin('exif.asset', 'asset') .where('asset.ownerId IN (:...userIds )', { userIds }) + .andWhere(`exif.city != ''`) + .andWhere('exif.city IS NOT NULL') .select('exif.city', 'city') .distinctOn(['exif.city']); @@ -389,16 +398,17 @@ export class SearchRepository implements ISearchRepository { } const results = await query.getRawMany<{ city: string }>(); - - return results.map(({ city }) => city).filter((item) => item !== ''); + return results.map(({ city }) => city); } @GenerateSql({ params: [[DummyValue.UUID], DummyValue.STRING] }) - async getCameraMakes(userIds: string[], model: string | undefined): Promise<string[]> { + async getCameraMakes(userIds: string[], { model }: GetCameraMakesOptions): Promise<string[]> { const query = this.exifRepository .createQueryBuilder('exif') - .leftJoin('exif.asset', 'asset') + .innerJoin('exif.asset', 'asset') .where('asset.ownerId IN (:...userIds )', { userIds }) + .andWhere(`exif.make != ''`) + .andWhere('exif.make IS NOT NULL') .select('exif.make', 'make') .distinctOn(['exif.make']); @@ -407,15 +417,17 @@ export class SearchRepository implements ISearchRepository { } const results = await query.getRawMany<{ make: string }>(); - return results.map(({ make }) => make).filter((item) => item !== ''); + return results.map(({ make }) => make); } @GenerateSql({ params: [[DummyValue.UUID], DummyValue.STRING] }) - async getCameraModels(userIds: string[], make: string | undefined): Promise<string[]> { + async getCameraModels(userIds: string[], { make }: GetCameraModelsOptions): Promise<string[]> { const query = this.exifRepository .createQueryBuilder('exif') - .leftJoin('exif.asset', 'asset') + .innerJoin('exif.asset', 'asset') .where('asset.ownerId IN (:...userIds )', { userIds }) + .andWhere(`exif.model != ''`) + .andWhere('exif.model IS NOT NULL') .select('exif.model', 'model') .distinctOn(['exif.model']); @@ -424,7 +436,7 @@ export class SearchRepository implements ISearchRepository { } const results = await query.getRawMany<{ model: string }>(); - return results.map(({ model }) => model).filter((item) => item !== ''); + return results.map(({ model }) => model); } private getRuntimeConfig(numResults?: number): string | undefined { diff --git a/server/src/services/search.service.spec.ts b/server/src/services/search.service.spec.ts index 0f95d88083..3933526167 100644 --- a/server/src/services/search.service.spec.ts +++ b/server/src/services/search.service.spec.ts @@ -59,20 +59,84 @@ describe(SearchService.name, () => { }); describe('getSearchSuggestions', () => { - it('should return search suggestions (including null)', async () => { - searchMock.getCountries.mockResolvedValue(['USA', null]); + it('should return search suggestions for country', async () => { + searchMock.getCountries.mockResolvedValue(['USA']); + await expect( + sut.getSearchSuggestions(authStub.user1, { includeNull: false, type: SearchSuggestionType.COUNTRY }), + ).resolves.toEqual(['USA']); + expect(searchMock.getCountries).toHaveBeenCalledWith([authStub.user1.user.id]); + }); + + it('should return search suggestions for country (including null)', async () => { + searchMock.getCountries.mockResolvedValue(['USA']); await expect( sut.getSearchSuggestions(authStub.user1, { includeNull: true, type: SearchSuggestionType.COUNTRY }), ).resolves.toEqual(['USA', null]); expect(searchMock.getCountries).toHaveBeenCalledWith([authStub.user1.user.id]); }); - it('should return search suggestions (without null)', async () => { - searchMock.getCountries.mockResolvedValue(['USA', null]); + it('should return search suggestions for state', async () => { + searchMock.getStates.mockResolvedValue(['California']); await expect( - sut.getSearchSuggestions(authStub.user1, { includeNull: false, type: SearchSuggestionType.COUNTRY }), - ).resolves.toEqual(['USA']); - expect(searchMock.getCountries).toHaveBeenCalledWith([authStub.user1.user.id]); + sut.getSearchSuggestions(authStub.user1, { includeNull: false, type: SearchSuggestionType.STATE }), + ).resolves.toEqual(['California']); + expect(searchMock.getStates).toHaveBeenCalledWith([authStub.user1.user.id], expect.anything()); + }); + + it('should return search suggestions for state (including null)', async () => { + searchMock.getStates.mockResolvedValue(['California']); + await expect( + sut.getSearchSuggestions(authStub.user1, { includeNull: true, type: SearchSuggestionType.STATE }), + ).resolves.toEqual(['California', null]); + expect(searchMock.getStates).toHaveBeenCalledWith([authStub.user1.user.id], expect.anything()); + }); + + it('should return search suggestions for city', async () => { + searchMock.getCities.mockResolvedValue(['Denver']); + await expect( + sut.getSearchSuggestions(authStub.user1, { includeNull: false, type: SearchSuggestionType.CITY }), + ).resolves.toEqual(['Denver']); + expect(searchMock.getCities).toHaveBeenCalledWith([authStub.user1.user.id], expect.anything()); + }); + + it('should return search suggestions for city (including null)', async () => { + searchMock.getCities.mockResolvedValue(['Denver']); + await expect( + sut.getSearchSuggestions(authStub.user1, { includeNull: true, type: SearchSuggestionType.CITY }), + ).resolves.toEqual(['Denver', null]); + expect(searchMock.getCities).toHaveBeenCalledWith([authStub.user1.user.id], expect.anything()); + }); + + it('should return search suggestions for camera make', async () => { + searchMock.getCameraMakes.mockResolvedValue(['Nikon']); + await expect( + sut.getSearchSuggestions(authStub.user1, { includeNull: false, type: SearchSuggestionType.CAMERA_MAKE }), + ).resolves.toEqual(['Nikon']); + expect(searchMock.getCameraMakes).toHaveBeenCalledWith([authStub.user1.user.id], expect.anything()); + }); + + it('should return search suggestions for camera make (including null)', async () => { + searchMock.getCameraMakes.mockResolvedValue(['Nikon']); + await expect( + sut.getSearchSuggestions(authStub.user1, { includeNull: true, type: SearchSuggestionType.CAMERA_MAKE }), + ).resolves.toEqual(['Nikon', null]); + expect(searchMock.getCameraMakes).toHaveBeenCalledWith([authStub.user1.user.id], expect.anything()); + }); + + it('should return search suggestions for camera model', async () => { + searchMock.getCameraModels.mockResolvedValue(['Fujifilm X100VI']); + await expect( + sut.getSearchSuggestions(authStub.user1, { includeNull: false, type: SearchSuggestionType.CAMERA_MODEL }), + ).resolves.toEqual(['Fujifilm X100VI']); + expect(searchMock.getCameraModels).toHaveBeenCalledWith([authStub.user1.user.id], expect.anything()); + }); + + it('should return search suggestions for camera model (including null)', async () => { + searchMock.getCameraModels.mockResolvedValue(['Fujifilm X100VI']); + await expect( + sut.getSearchSuggestions(authStub.user1, { includeNull: true, type: SearchSuggestionType.CAMERA_MODEL }), + ).resolves.toEqual(['Fujifilm X100VI', null]); + expect(searchMock.getCameraModels).toHaveBeenCalledWith([authStub.user1.user.id], expect.anything()); }); }); }); diff --git a/server/src/services/search.service.ts b/server/src/services/search.service.ts index bf5bf9e311..7fc947a8b5 100644 --- a/server/src/services/search.service.ts +++ b/server/src/services/search.service.ts @@ -108,8 +108,11 @@ export class SearchService extends BaseService { async getSearchSuggestions(auth: AuthDto, dto: SearchSuggestionRequestDto) { const userIds = await this.getUserIdsToSearch(auth); - const results = await this.getSuggestions(userIds, dto); - return results.filter((result) => (dto.includeNull ? true : result !== null)); + const suggestions = await this.getSuggestions(userIds, dto); + if (dto.includeNull) { + suggestions.push(null); + } + return suggestions; } private getSuggestions(userIds: string[], dto: SearchSuggestionRequestDto) { @@ -118,19 +121,19 @@ export class SearchService extends BaseService { return this.searchRepository.getCountries(userIds); } case SearchSuggestionType.STATE: { - return this.searchRepository.getStates(userIds, dto.country); + return this.searchRepository.getStates(userIds, dto); } case SearchSuggestionType.CITY: { - return this.searchRepository.getCities(userIds, dto.country, dto.state); + return this.searchRepository.getCities(userIds, dto); } case SearchSuggestionType.CAMERA_MAKE: { - return this.searchRepository.getCameraMakes(userIds, dto.model); + return this.searchRepository.getCameraMakes(userIds, dto); } case SearchSuggestionType.CAMERA_MODEL: { - return this.searchRepository.getCameraModels(userIds, dto.make); + return this.searchRepository.getCameraModels(userIds, dto); } default: { - return []; + return [] as (string | null)[]; } } }