refactor(server): use includeNull in query for search suggestions (#…

…14626)

* use `includeNull`

* push down `includeNull` into query, inner joins

* remove filter

* update sql

* fix tests

* maybe fix e2e

* more e2e tests

* handle no exif row

* whoops

* update sql

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);
+    });
   });
 });

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>>;
 }

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

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 {