From f392fe7702ebb09773bf8cb6a08a369ef80f5ce5 Mon Sep 17 00:00:00 2001
From: Mert <101130780+mertalev@users.noreply.github.com>
Date: Tue, 19 Mar 2024 23:23:57 -0400
Subject: [PATCH] fix(server): "view all" for cities only showing 12 cities
 (#8035)

* view all cities

* increase limit

* rename endpoint

* optimize query

* remove pagination

* update sql

* linting

* revert sort by count in explore page for now

* fix query

* fix

* update sql

* move to search, add partner support

* update sql

* pr feedback

* euphemism

* parameters as separate variable

* move comment

* update sql

* linting
---
 mobile/openapi/README.md                      | Bin 25135 -> 25234 bytes
 mobile/openapi/doc/SearchApi.md               | Bin 15521 -> 17401 bytes
 mobile/openapi/lib/api/search_api.dart        | Bin 14647 -> 16129 bytes
 mobile/openapi/test/search_api_test.dart      | Bin 1624 -> 1752 bytes
 open-api/immich-openapi-specs.json            |  35 ++++++
 open-api/typescript-sdk/src/fetch-client.ts   |   8 ++
 .../domain/repositories/search.repository.ts  |   1 +
 server/src/domain/search/search.service.ts    |  46 ++++----
 .../immich/controllers/search.controller.ts   |   6 +
 .../infra/repositories/search.repository.ts   |  58 ++++++++++
 server/src/infra/sql/search.repository.sql    | 108 ++++++++++++++++++
 .../repositories/search.repository.mock.ts    |   1 +
 web/src/routes/(user)/places/+page.svelte     |  21 ++--
 web/src/routes/(user)/places/+page.ts         |   4 +-
 14 files changed, 256 insertions(+), 32 deletions(-)

diff --git a/mobile/openapi/README.md b/mobile/openapi/README.md
index d8ff4d30fed863ffc8b0839fcd3f11d858d93579..93829356827d99f6d676d81f6be6f580116b02e7 100644
GIT binary patch
delta 58
zcmZ2~gmKbQ#tkakJdVZ1sU^iumCl(Zm6P?gC3zCT0!fw0Ac2G0BCJ40X6oeo+G3j(
HwfW2ejjk1S

delta 14
VcmbPqlyUtL#tkako7=Uy%>geo1#<uZ

diff --git a/mobile/openapi/doc/SearchApi.md b/mobile/openapi/doc/SearchApi.md
index f63488222b156a7190c45a9548e5bad140fd3948..e4ab9ecfd3f0d626c2ff038606fc5f42e192ab46 100644
GIT binary patch
delta 207
zcmZ2j`LmsI!Yv)g;^NejVy8;y%#unityqoV)Wo9X499{@z1$S#^wg3>uzXTwGEjbU
z1Ct0Rkdc{MtWcw{@v;n)1*&!~I|ZN2;u0IMXi#c#L4ICws!K_}odVD#WThIL8-?Xq
nCVR7M@gNINZeR~Z6?nj&DurPh$em!p&G{U27&pIX+GGg;bT&-e

delta 17
Zcmey_&bY90!mZ6(jG|1N-MKbd0su#!2I>F+

diff --git a/mobile/openapi/lib/api/search_api.dart b/mobile/openapi/lib/api/search_api.dart
index 3a0bc56bb61882b7ed08c73ddc899b4f11b85128..386a2f35368a42587ae3436d48abdee54a821541 100644
GIT binary patch
delta 150
zcmdm9)L6IS1S3mwW=ZDc0v(;nKN&@M9E*!nONyN;oij@+CrdCzLuBK0k50B_{;mv`
z4N5I8$j>WIbt%cWv$s=7Pc1>w6vv_|i7XJFS(4#VQc~cVmzJ-wIZ){y<77YWDps(P
T$>+4ypsZioMVn7B=9>cm>z_G%

delta 12
TcmZpy+g`Nc1mosO?D^&ZD47M6

diff --git a/mobile/openapi/test/search_api_test.dart b/mobile/openapi/test/search_api_test.dart
index aa4a94847b4eeba806332fd499bf9af0cb16b631..801c97a180a4e00439a64bc9941d6ae0cfa2da25 100644
GIT binary patch
delta 72
zcmcb?bAxxoMMh=E;^Nejpw!}m{Ji2+my&!tJB9Sr60m5oQ>AleNu|c*gUsUmXo8a!
Mm@GD5Vr*jp0RMR#F#rGn

delta 12
Tcmcb?dxK}gMaIp6OzkWHB(DVB

diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json
index 82562100a1..f50abdffc2 100644
--- a/open-api/immich-openapi-specs.json
+++ b/open-api/immich-openapi-specs.json
@@ -4597,6 +4597,41 @@
         ]
       }
     },
+    "/search/cities": {
+      "get": {
+        "operationId": "getAssetsByCity",
+        "parameters": [],
+        "responses": {
+          "200": {
+            "content": {
+              "application/json": {
+                "schema": {
+                  "items": {
+                    "$ref": "#/components/schemas/AssetResponseDto"
+                  },
+                  "type": "array"
+                }
+              }
+            },
+            "description": ""
+          }
+        },
+        "security": [
+          {
+            "bearer": []
+          },
+          {
+            "cookie": []
+          },
+          {
+            "api_key": []
+          }
+        ],
+        "tags": [
+          "Search"
+        ]
+      }
+    },
     "/search/explore": {
       "get": {
         "operationId": "getExploreData",
diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts
index 6b50642520..00434aabac 100644
--- a/open-api/typescript-sdk/src/fetch-client.ts
+++ b/open-api/typescript-sdk/src/fetch-client.ts
@@ -2204,6 +2204,14 @@ export function search({ clip, motion, page, q, query, recent, size, smart, $typ
         ...opts
     }));
 }
+export function getAssetsByCity(opts?: Oazapfts.RequestOpts) {
+    return oazapfts.ok(oazapfts.fetchJson<{
+        status: 200;
+        data: AssetResponseDto[];
+    }>("/search/cities", {
+        ...opts
+    }));
+}
 export function getExploreData(opts?: Oazapfts.RequestOpts) {
     return oazapfts.ok(oazapfts.fetchJson<{
         status: 200;
diff --git a/server/src/domain/repositories/search.repository.ts b/server/src/domain/repositories/search.repository.ts
index 10182a44ec..bd4face86c 100644
--- a/server/src/domain/repositories/search.repository.ts
+++ b/server/src/domain/repositories/search.repository.ts
@@ -187,5 +187,6 @@ export interface ISearchRepository {
   searchFaces(search: FaceEmbeddingSearch): Promise<FaceSearchResult[]>;
   upsert(smartInfo: Partial<SmartInfoEntity>, embedding?: Embedding): Promise<void>;
   searchPlaces(placeName: string): Promise<GeodataPlacesEntity[]>;
+  getAssetsByCity(userIds: string[]): Promise<AssetEntity[]>;
   deleteAllSearchEmbeddings(): Promise<void>;
 }
diff --git a/server/src/domain/search/search.service.ts b/server/src/domain/search/search.service.ts
index 56c4498bce..4b15dfd51c 100644
--- a/server/src/domain/search/search.service.ts
+++ b/server/src/domain/search/search.service.ts
@@ -115,6 +115,32 @@ export class SearchService {
     return this.mapResponse(items, hasNextPage ? (page + 1).toString() : null);
   }
 
+  async getAssetsByCity(auth: AuthDto): Promise<AssetResponseDto[]> {
+    const userIds = await this.getUserIdsToSearch(auth);
+    const assets = await this.searchRepository.getAssetsByCity(userIds);
+    return assets.map((asset) => mapAsset(asset));
+  }
+
+  getSearchSuggestions(auth: AuthDto, dto: SearchSuggestionRequestDto): Promise<string[]> {
+    switch (dto.type) {
+      case SearchSuggestionType.COUNTRY: {
+        return this.metadataRepository.getCountries(auth.user.id);
+      }
+      case SearchSuggestionType.STATE: {
+        return this.metadataRepository.getStates(auth.user.id, dto.country);
+      }
+      case SearchSuggestionType.CITY: {
+        return this.metadataRepository.getCities(auth.user.id, dto.country, dto.state);
+      }
+      case SearchSuggestionType.CAMERA_MAKE: {
+        return this.metadataRepository.getCameraMakes(auth.user.id, dto.model);
+      }
+      case SearchSuggestionType.CAMERA_MODEL: {
+        return this.metadataRepository.getCameraModels(auth.user.id, dto.make);
+      }
+    }
+  }
+
   // TODO: remove after implementing new search filters
   /** @deprecated */
   async search(auth: AuthDto, dto: SearchDto): Promise<SearchResponseDto> {
@@ -191,24 +217,4 @@ export class SearchService {
       },
     };
   }
-
-  async getSearchSuggestions(auth: AuthDto, dto: SearchSuggestionRequestDto): Promise<string[]> {
-    switch (dto.type) {
-      case SearchSuggestionType.COUNTRY: {
-        return this.metadataRepository.getCountries(auth.user.id);
-      }
-      case SearchSuggestionType.STATE: {
-        return this.metadataRepository.getStates(auth.user.id, dto.country);
-      }
-      case SearchSuggestionType.CITY: {
-        return this.metadataRepository.getCities(auth.user.id, dto.country, dto.state);
-      }
-      case SearchSuggestionType.CAMERA_MAKE: {
-        return this.metadataRepository.getCameraMakes(auth.user.id, dto.model);
-      }
-      case SearchSuggestionType.CAMERA_MODEL: {
-        return this.metadataRepository.getCameraModels(auth.user.id, dto.make);
-      }
-    }
-  }
 }
diff --git a/server/src/immich/controllers/search.controller.ts b/server/src/immich/controllers/search.controller.ts
index d508531ddd..a3527a66a0 100644
--- a/server/src/immich/controllers/search.controller.ts
+++ b/server/src/immich/controllers/search.controller.ts
@@ -1,4 +1,5 @@
 import {
+  AssetResponseDto,
   AuthDto,
   MetadataSearchDto,
   PersonResponseDto,
@@ -55,6 +56,11 @@ export class SearchController {
     return this.service.searchPlaces(dto);
   }
 
+  @Get('cities')
+  getAssetsByCity(@Auth() auth: AuthDto): Promise<AssetResponseDto[]> {
+    return this.service.getAssetsByCity(auth);
+  }
+
   @Get('suggestions')
   getSearchSuggestions(@Auth() auth: AuthDto, @Query() dto: SearchSuggestionRequestDto): Promise<string[]> {
     return this.service.getSearchSuggestions(auth, dto);
diff --git a/server/src/infra/repositories/search.repository.ts b/server/src/infra/repositories/search.repository.ts
index f5d1cbda39..0e29506d1f 100644
--- a/server/src/infra/repositories/search.repository.ts
+++ b/server/src/infra/repositories/search.repository.ts
@@ -15,6 +15,7 @@ import { getCLIPModelInfo } from '@app/domain/smart-info/smart-info.constant';
 import {
   AssetEntity,
   AssetFaceEntity,
+  AssetType,
   GeodataPlacesEntity,
   SmartInfoEntity,
   SmartSearchEntity,
@@ -33,6 +34,7 @@ import { Instrumentation } from '../instrumentation';
 export class SearchRepository implements ISearchRepository {
   private logger = new ImmichLogger(SearchRepository.name);
   private faceColumns: string[];
+  private assetsByCityQuery: string;
 
   constructor(
     @InjectRepository(SmartInfoEntity) private repository: Repository<SmartInfoEntity>,
@@ -45,6 +47,14 @@ export class SearchRepository implements ISearchRepository {
       .getMetadata(AssetFaceEntity)
       .ownColumns.map((column) => column.propertyName)
       .filter((propertyName) => propertyName !== 'embedding');
+    this.assetsByCityQuery =
+      assetsByCityCte +
+      this.assetRepository
+        .createQueryBuilder('asset')
+        .innerJoinAndSelect('asset.exifInfo', 'exif')
+        .withDeleted()
+        .getQuery() +
+      ' INNER JOIN cte ON asset.id = cte."assetId"';
   }
 
   async init(modelName: string): Promise<void> {
@@ -220,6 +230,27 @@ export class SearchRepository implements ISearchRepository {
       .getMany();
   }
 
+  @GenerateSql({ params: [[DummyValue.UUID]] })
+  async getAssetsByCity(userIds: string[]): Promise<AssetEntity[]> {
+    const parameters = [userIds.join(', '), true, false, AssetType.IMAGE];
+    const rawRes = await this.repository.query(this.assetsByCityQuery, parameters);
+
+    const items: AssetEntity[] = [];
+    for (const res of rawRes) {
+      const item = { exifInfo: {} as Record<string, any> } as Record<string, any>;
+      for (const [key, value] of Object.entries(res)) {
+        if (key.startsWith('exif_')) {
+          item.exifInfo[key.replace('exif_', '')] = value;
+        } else {
+          item[key.replace('asset_', '')] = value;
+        }
+      }
+      items.push(item as AssetEntity);
+    }
+
+    return items;
+  }
+
   async upsert(smartInfo: Partial<SmartInfoEntity>, embedding?: Embedding): Promise<void> {
     await this.repository.upsert(smartInfo, { conflictPaths: ['assetId'] });
     if (!smartInfo.assetId || !embedding) {
@@ -290,3 +321,30 @@ export class SearchRepository implements ISearchRepository {
     return runtimeConfig;
   }
 }
+
+// the performance difference between this and the normal way is too huge to ignore, e.g. 3s vs 4ms
+const assetsByCityCte = `
+WITH RECURSIVE cte AS (
+  (
+    SELECT city, "assetId"
+    FROM exif
+    INNER JOIN assets ON exif."assetId" = assets.id
+    WHERE "ownerId" IN ($1) AND "isVisible" = $2 AND "isArchived" = $3 AND type = $4
+    ORDER BY city
+    LIMIT 1
+  )
+
+  UNION ALL
+  
+  SELECT l.city, l."assetId"
+  FROM cte c
+    , LATERAL (
+    SELECT city, "assetId"
+    FROM exif
+    INNER JOIN assets ON exif."assetId" = assets.id
+    WHERE city > c.city AND "ownerId" IN ($1) AND "isVisible" = $2 AND "isArchived" = $3 AND type = $4
+    ORDER BY city
+    LIMIT 1
+    ) l
+)
+`;
diff --git a/server/src/infra/sql/search.repository.sql b/server/src/infra/sql/search.repository.sql
index a11f8805a0..ff02391989 100644
--- a/server/src/infra/sql/search.repository.sql
+++ b/server/src/infra/sql/search.repository.sql
@@ -266,3 +266,111 @@ ORDER BY
   ) ASC
 LIMIT
   20
+
+-- SearchRepository.getAssetsByCity
+WITH RECURSIVE
+  cte AS (
+    (
+      SELECT
+        city,
+        "assetId"
+      FROM
+        exif
+        INNER JOIN assets ON exif."assetId" = assets.id
+      WHERE
+        "ownerId" IN ($1)
+        AND "isVisible" = $2
+        AND "isArchived" = $3
+        AND type = $4
+      ORDER BY
+        city
+      LIMIT
+        1
+    )
+    UNION ALL
+    SELECT
+      l.city,
+      l."assetId"
+    FROM
+      cte c,
+      LATERAL (
+        SELECT
+          city,
+          "assetId"
+        FROM
+          exif
+          INNER JOIN assets ON exif."assetId" = assets.id
+        WHERE
+          city > c.city
+          AND "ownerId" IN ($1)
+          AND "isVisible" = $2
+          AND "isArchived" = $3
+          AND type = $4
+        ORDER BY
+          city
+        LIMIT
+          1
+      ) l
+  )
+SELECT
+  "asset"."id" AS "asset_id",
+  "asset"."deviceAssetId" AS "asset_deviceAssetId",
+  "asset"."ownerId" AS "asset_ownerId",
+  "asset"."libraryId" AS "asset_libraryId",
+  "asset"."deviceId" AS "asset_deviceId",
+  "asset"."type" AS "asset_type",
+  "asset"."originalPath" AS "asset_originalPath",
+  "asset"."resizePath" AS "asset_resizePath",
+  "asset"."webpPath" AS "asset_webpPath",
+  "asset"."thumbhash" AS "asset_thumbhash",
+  "asset"."encodedVideoPath" AS "asset_encodedVideoPath",
+  "asset"."createdAt" AS "asset_createdAt",
+  "asset"."updatedAt" AS "asset_updatedAt",
+  "asset"."deletedAt" AS "asset_deletedAt",
+  "asset"."fileCreatedAt" AS "asset_fileCreatedAt",
+  "asset"."localDateTime" AS "asset_localDateTime",
+  "asset"."fileModifiedAt" AS "asset_fileModifiedAt",
+  "asset"."isFavorite" AS "asset_isFavorite",
+  "asset"."isArchived" AS "asset_isArchived",
+  "asset"."isExternal" AS "asset_isExternal",
+  "asset"."isReadOnly" AS "asset_isReadOnly",
+  "asset"."isOffline" AS "asset_isOffline",
+  "asset"."checksum" AS "asset_checksum",
+  "asset"."duration" AS "asset_duration",
+  "asset"."isVisible" AS "asset_isVisible",
+  "asset"."livePhotoVideoId" AS "asset_livePhotoVideoId",
+  "asset"."originalFileName" AS "asset_originalFileName",
+  "asset"."sidecarPath" AS "asset_sidecarPath",
+  "asset"."stackId" AS "asset_stackId",
+  "exif"."assetId" AS "exif_assetId",
+  "exif"."description" AS "exif_description",
+  "exif"."exifImageWidth" AS "exif_exifImageWidth",
+  "exif"."exifImageHeight" AS "exif_exifImageHeight",
+  "exif"."fileSizeInByte" AS "exif_fileSizeInByte",
+  "exif"."orientation" AS "exif_orientation",
+  "exif"."dateTimeOriginal" AS "exif_dateTimeOriginal",
+  "exif"."modifyDate" AS "exif_modifyDate",
+  "exif"."timeZone" AS "exif_timeZone",
+  "exif"."latitude" AS "exif_latitude",
+  "exif"."longitude" AS "exif_longitude",
+  "exif"."projectionType" AS "exif_projectionType",
+  "exif"."city" AS "exif_city",
+  "exif"."livePhotoCID" AS "exif_livePhotoCID",
+  "exif"."autoStackId" AS "exif_autoStackId",
+  "exif"."state" AS "exif_state",
+  "exif"."country" AS "exif_country",
+  "exif"."make" AS "exif_make",
+  "exif"."model" AS "exif_model",
+  "exif"."lensModel" AS "exif_lensModel",
+  "exif"."fNumber" AS "exif_fNumber",
+  "exif"."focalLength" AS "exif_focalLength",
+  "exif"."iso" AS "exif_iso",
+  "exif"."exposureTime" AS "exif_exposureTime",
+  "exif"."profileDescription" AS "exif_profileDescription",
+  "exif"."colorspace" AS "exif_colorspace",
+  "exif"."bitsPerSample" AS "exif_bitsPerSample",
+  "exif"."fps" AS "exif_fps"
+FROM
+  "assets" "asset"
+  INNER JOIN "exif" "exif" ON "exif"."assetId" = "asset"."id"
+  INNER JOIN cte ON asset.id = cte."assetId"
diff --git a/server/test/repositories/search.repository.mock.ts b/server/test/repositories/search.repository.mock.ts
index 5912d77451..7b428f0cc4 100644
--- a/server/test/repositories/search.repository.mock.ts
+++ b/server/test/repositories/search.repository.mock.ts
@@ -8,6 +8,7 @@ export const newSearchRepositoryMock = (): jest.Mocked<ISearchRepository> => {
     searchFaces: jest.fn(),
     upsert: jest.fn(),
     searchPlaces: jest.fn(),
+    getAssetsByCity: jest.fn(),
     deleteAllSearchEmbeddings: jest.fn(),
   };
 };
diff --git a/web/src/routes/(user)/places/+page.svelte b/web/src/routes/(user)/places/+page.svelte
index c5528fcb9d..01222ab6bc 100644
--- a/web/src/routes/(user)/places/+page.svelte
+++ b/web/src/routes/(user)/places/+page.svelte
@@ -3,20 +3,20 @@
   import Icon from '$lib/components/elements/icon.svelte';
   import UserPageLayout from '$lib/components/layouts/user-page-layout.svelte';
   import { AppRoute } from '$lib/constants';
-  import type { SearchExploreResponseDto } from '@immich/sdk';
   import { mdiMapMarkerOff } from '@mdi/js';
   import type { PageData } from './$types';
   import { getMetadataSearchQuery } from '$lib/utils/metadata-search';
+  import type { AssetResponseDto } from '@immich/sdk';
 
   export let data: PageData;
 
-  const CITY_FIELD = 'exifInfo.city';
-  const getFieldItems = (items: SearchExploreResponseDto[]) => {
-    const targetField = items.find((item) => item.fieldName === CITY_FIELD);
-    return targetField?.items || [];
+  type AssetWithCity = AssetResponseDto & {
+    exifInfo: {
+      city: string;
+    };
   };
 
-  $: places = getFieldItems(data.items);
+  $: places = data.items.filter((item): item is AssetWithCity => !!item.exifInfo?.city);
   $: hasPlaces = places.length > 0;
 
   let innerHeight: number;
@@ -27,17 +27,18 @@
 <UserPageLayout title="Places">
   {#if hasPlaces}
     <div class="flex flex-row flex-wrap gap-4">
-      {#each places as item (item.data.id)}
-        <a class="relative" href="{AppRoute.SEARCH}?{getMetadataSearchQuery({ city: item.value })}" draggable="false">
+      {#each places as item (item.id)}
+        {@const city = item.exifInfo.city}
+        <a class="relative" href="{AppRoute.SEARCH}?{getMetadataSearchQuery({ city })}" draggable="false">
           <div
             class="flex w-[calc((100vw-(72px+5rem))/2)] max-w-[156px] justify-center overflow-hidden rounded-xl brightness-75 filter"
           >
-            <Thumbnail thumbnailSize={156} asset={item.data} readonly />
+            <Thumbnail thumbnailSize={156} asset={item} readonly />
           </div>
           <span
             class="w-100 absolute bottom-2 w-full text-ellipsis px-1 text-center text-sm font-medium capitalize text-white backdrop-blur-[1px] hover:cursor-pointer"
           >
-            {item.value}
+            {city}
           </span>
         </a>
       {/each}
diff --git a/web/src/routes/(user)/places/+page.ts b/web/src/routes/(user)/places/+page.ts
index 5627111ce6..1f3a15fb64 100644
--- a/web/src/routes/(user)/places/+page.ts
+++ b/web/src/routes/(user)/places/+page.ts
@@ -1,10 +1,10 @@
 import { authenticate } from '$lib/utils/auth';
-import { getExploreData } from '@immich/sdk';
+import { getAssetsByCity } from '@immich/sdk';
 import type { PageLoad } from './$types';
 
 export const load = (async () => {
   await authenticate();
-  const items = await getExploreData();
+  const items = await getAssetsByCity();
 
   return {
     items,