refactor(server): immich file responses (immich-app#5641)

* refactor(server): immich file response * chore: open api * chore: tests * chore: fix logger import --------- Co-authored-by: Alex Tran <[email protected]>
matthieurosset · Dec 12, 2023 · cbca698 · cbca698
1 parent af7c4ae
commit cbca698
Show file tree

Hide file tree

Showing 25 changed files with 186 additions and 154 deletions.
diff --git a/cli/src/api/open-api/api.ts b/cli/src/api/open-api/api.ts
diff --git a/mobile/openapi/doc/AssetApi.md b/mobile/openapi/doc/AssetApi.md
diff --git a/mobile/openapi/doc/PersonApi.md b/mobile/openapi/doc/PersonApi.md
diff --git a/mobile/openapi/doc/UserApi.md b/mobile/openapi/doc/UserApi.md
diff --git a/mobile/openapi/lib/api/user_api.dart b/mobile/openapi/lib/api/user_api.dart
diff --git a/mobile/openapi/test/user_api_test.dart b/mobile/openapi/test/user_api_test.dart
diff --git a/server/immich-openapi-specs.json b/server/immich-openapi-specs.json
@@ -1465,6 +1465,15 @@
       "get": {
         "operationId": "serveFile",
         "parameters": [
+          {
+            "name": "id",
+            "required": true,
+            "in": "path",
+            "schema": {
+              "format": "uuid",
+              "type": "string"
+            }
+          },
           {
             "name": "isThumb",
             "required": false,
@@ -1483,15 +1492,6 @@
               "type": "boolean"
             }
           },
-          {
-            "name": "id",
-            "required": true,
-            "in": "path",
-            "schema": {
-              "format": "uuid",
-              "type": "string"
-            }
-          },
           {
             "name": "key",
             "required": false,
@@ -1926,13 +1926,7 @@
         "responses": {
           "200": {
             "content": {
-              "image/jpeg": {
-                "schema": {
-                  "format": "binary",
-                  "type": "string"
-                }
-              },
-              "image/webp": {
+              "application/octet-stream": {
                 "schema": {
                   "format": "binary",
                   "type": "string"
@@ -4499,7 +4493,7 @@
         "responses": {
           "200": {
             "content": {
-              "image/jpeg": {
+              "application/octet-stream": {
                 "schema": {
                   "format": "binary",
                   "type": "string"
@@ -6080,9 +6074,10 @@
         "responses": {
           "200": {
             "content": {
-              "application/json": {
+              "application/octet-stream": {
                 "schema": {
-                  "type": "object"
+                  "format": "binary",
+                  "type": "string"
                 }
               }
             },

diff --git a/server/src/domain/asset/asset.service.spec.ts b/server/src/domain/asset/asset.service.spec.ts
@@ -16,6 +16,7 @@ import {
 } from '@test';
 import { when } from 'jest-when';
 import { Readable } from 'stream';
+import { ImmichFileResponse } from '../domain.util';
 import { JobName } from '../job';
 import {
   AssetStats,
@@ -474,15 +475,16 @@ describe(AssetService.name, () => {
     });
 
     it('should download a file', async () => {
-      const stream = new Readable();
-
       accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1']));
       assetMock.getByIds.mockResolvedValue([assetStub.image]);
-      storageMock.createReadStream.mockResolvedValue({ stream });
-
-      await expect(sut.downloadFile(authStub.admin, 'asset-1')).resolves.toEqual({ stream });
 
-      expect(storageMock.createReadStream).toHaveBeenCalledWith(assetStub.image.originalPath, 'image/jpeg');
+      await expect(sut.downloadFile(authStub.admin, 'asset-1')).resolves.toEqual(
+        new ImmichFileResponse({
+          path: '/original/path.jpg',
+          contentType: 'image/jpeg',
+          cacheControl: false,
+        }),
+      );
     });
 
     it('should download an archive', async () => {

diff --git a/server/src/domain/asset/asset.service.ts b/server/src/domain/asset/asset.service.ts
@@ -7,7 +7,7 @@ import sanitize from 'sanitize-filename';
 import { AccessCore, Permission } from '../access';
 import { AuthDto } from '../auth';
 import { mimeTypes } from '../domain.constant';
-import { HumanReadableSize, usePagination } from '../domain.util';
+import { HumanReadableSize, ImmichFileResponse, usePagination } from '../domain.util';
 import { IAssetDeletionJob, ISidecarWriteJob, JOBS_ASSET_PAGINATION_SIZE, JobName } from '../job';
 import {
   CommunicationEvent,
@@ -274,7 +274,7 @@ export class AssetService {
 
     return { ...options, userIds };
   }
-  async downloadFile(auth: AuthDto, id: string): Promise<ImmichReadStream> {
+  async downloadFile(auth: AuthDto, id: string): Promise<ImmichFileResponse> {
     await this.access.requirePermission(auth, Permission.ASSET_DOWNLOAD, id);
 
     const [asset] = await this.assetRepository.getByIds([id]);
@@ -286,7 +286,11 @@ export class AssetService {
       throw new BadRequestException('Asset is offline');
     }
 
-    return this.storageRepository.createReadStream(asset.originalPath, mimeTypes.lookup(asset.originalPath));
+    return new ImmichFileResponse({
+      path: asset.originalPath,
+      contentType: mimeTypes.lookup(asset.originalPath),
+      cacheControl: false,
+    });
   }
 
   async getDownloadInfo(auth: AuthDto, dto: DownloadInfoDto): Promise<DownloadResponseDto> {

diff --git a/server/src/domain/domain.util.ts b/server/src/domain/domain.util.ts
@@ -16,6 +16,16 @@ import { CronJob } from 'cron';
 import { basename, extname } from 'node:path';
 import sanitize from 'sanitize-filename';
 
+export class ImmichFileResponse {
+  public readonly path!: string;
+  public readonly contentType!: string;
+  public readonly cacheControl!: boolean;
+
+  constructor(response: ImmichFileResponse) {
+    Object.assign(this, response);
+  }
+}
+
 export interface OpenGraphTags {
   title: string;
   description: string;

diff --git a/server/src/domain/person/person.service.spec.ts b/server/src/domain/person/person.service.spec.ts
@@ -18,6 +18,7 @@ import {
   personStub,
 } from '@test';
 import { BulkIdErrorReason } from '../asset';
+import { ImmichFileResponse } from '../domain.util';
 import { JobName } from '../job';
 import {
   IAssetRepository,
@@ -203,8 +204,13 @@ describe(PersonService.name, () => {
     it('should serve the thumbnail', async () => {
       personMock.getById.mockResolvedValue(personStub.noName);
       accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1']));
-      await sut.getThumbnail(authStub.admin, 'person-1');
-      expect(storageMock.createReadStream).toHaveBeenCalledWith('/path/to/thumbnail.jpg', 'image/jpeg');
+      await expect(sut.getThumbnail(authStub.admin, 'person-1')).resolves.toEqual(
+        new ImmichFileResponse({
+          path: '/path/to/thumbnail.jpg',
+          contentType: 'image/jpeg',
+          cacheControl: true,
+        }),
+      );
       expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
     });
   });

diff --git a/server/src/domain/person/person.service.ts b/server/src/domain/person/person.service.ts
@@ -5,7 +5,7 @@ import { AccessCore, Permission } from '../access';
 import { AssetResponseDto, BulkIdErrorReason, BulkIdResponseDto, mapAsset } from '../asset';
 import { AuthDto } from '../auth';
 import { mimeTypes } from '../domain.constant';
-import { usePagination } from '../domain.util';
+import { ImmichFileResponse, usePagination } from '../domain.util';
 import { IBaseJob, IEntityJob, JOBS_ASSET_PAGINATION_SIZE, JobName } from '../job';
 import { FACE_THUMBNAIL_SIZE } from '../media';
 import {
@@ -20,7 +20,6 @@ import {
   ISmartInfoRepository,
   IStorageRepository,
   ISystemConfigRepository,
-  ImmichReadStream,
   UpdateFacesData,
   WithoutProperty,
 } from '../repositories';
@@ -173,14 +172,18 @@ export class PersonService {
     return this.repository.getStatistics(id);
   }
 
-  async getThumbnail(auth: AuthDto, id: string): Promise<ImmichReadStream> {
+  async getThumbnail(auth: AuthDto, id: string): Promise<ImmichFileResponse> {
     await this.access.requirePermission(auth, Permission.PERSON_READ, id);
     const person = await this.repository.getById(id);
     if (!person || !person.thumbnailPath) {
       throw new NotFoundException();
     }
 
-    return this.storageRepository.createReadStream(person.thumbnailPath, mimeTypes.lookup(person.thumbnailPath));
+    return new ImmichFileResponse({
+      path: person.thumbnailPath,
+      contentType: mimeTypes.lookup(person.thumbnailPath),
+      cacheControl: true,
+    });
   }
 
   async getAssets(auth: AuthDto, id: string): Promise<AssetResponseDto[]> {

diff --git a/server/src/domain/user/user.service.spec.ts b/server/src/domain/user/user.service.spec.ts
@@ -17,7 +17,7 @@ import {
   userStub,
 } from '@test';
 import { when } from 'jest-when';
-import { Readable } from 'stream';
+import { ImmichFileResponse } from '../domain.util';
 import { JobName } from '../job';
 import {
   IAlbumRepository,
@@ -390,15 +390,17 @@ describe(UserService.name, () => {
     });
 
     it('should return the profile picture', async () => {
-      const stream = new Readable();
-
       userMock.get.mockResolvedValue(userStub.profilePath);
-      storageMock.createReadStream.mockResolvedValue({ stream });
 
-      await expect(sut.getProfileImage(userStub.profilePath.id)).resolves.toEqual({ stream });
+      await expect(sut.getProfileImage(userStub.profilePath.id)).resolves.toEqual(
+        new ImmichFileResponse({
+          path: '/path/to/profile.jpg',
+          contentType: 'image/jpeg',
+          cacheControl: false,
+        }),
+      );
 
       expect(userMock.get).toHaveBeenCalledWith(userStub.profilePath.id, {});
-      expect(storageMock.createReadStream).toHaveBeenCalledWith('/path/to/profile.jpg', 'image/jpeg');
     });
   });
 

diff --git a/server/src/domain/user/user.service.ts b/server/src/domain/user/user.service.ts
@@ -2,6 +2,7 @@ import { UserEntity } from '@app/infra/entities';
 import { BadRequestException, ForbiddenException, Inject, Injectable, Logger, NotFoundException } from '@nestjs/common';
 import { randomBytes } from 'crypto';
 import { AuthDto } from '../auth';
+import { ImmichFileResponse } from '../domain.util';
 import { IEntityJob, JobName } from '../job';
 import {
   IAlbumRepository,
@@ -11,7 +12,6 @@ import {
   ILibraryRepository,
   IStorageRepository,
   IUserRepository,
-  ImmichReadStream,
   UserFindOptions,
 } from '../repositories';
 import { StorageCore, StorageFolder } from '../storage';
@@ -99,12 +99,17 @@ export class UserService {
     await this.jobRepository.queue({ name: JobName.DELETE_FILES, data: { files: [user.profileImagePath] } });
   }
 
-  async getProfileImage(id: string): Promise<ImmichReadStream> {
+  async getProfileImage(id: string): Promise<ImmichFileResponse> {
     const user = await this.findOrFail(id, {});
     if (!user.profileImagePath) {
       throw new NotFoundException('User does not have a profile image');
     }
-    return this.storageRepository.createReadStream(user.profileImagePath, 'image/jpeg');
+
+    return new ImmichFileResponse({
+      path: user.profileImagePath,
+      contentType: 'image/jpeg',
+      cacheControl: false,
+    });
   }
 
   async resetAdminPassword(ask: (admin: UserResponseDto) => Promise<string | undefined>) {