refactor(server): update asset endpoint (immich-app#3973)

* refactor(server): update asset * chore: open api
reeseovine · Sep 5, 2023 · 454737c · 454737c
1 parent 26bc889
commit 454737c
Show file tree

Hide file tree

Showing 27 changed files with 237 additions and 184 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/UpdateAssetDto.md b/mobile/openapi/doc/UpdateAssetDto.md
diff --git a/mobile/openapi/lib/api/asset_api.dart b/mobile/openapi/lib/api/asset_api.dart
diff --git a/mobile/openapi/lib/model/update_asset_dto.dart b/mobile/openapi/lib/model/update_asset_dto.dart
diff --git a/mobile/openapi/test/asset_api_test.dart b/mobile/openapi/test/asset_api_test.dart
diff --git a/mobile/openapi/test/update_asset_dto_test.dart b/mobile/openapi/test/update_asset_dto_test.dart
diff --git a/server/immich-openapi-specs.json b/server/immich-openapi-specs.json
@@ -2020,7 +2020,6 @@
     },
     "/asset/{id}": {
       "put": {
-        "description": "Update an asset",
         "operationId": "updateAsset",
         "parameters": [
           {
@@ -7424,18 +7423,6 @@
           },
           "isFavorite": {
             "type": "boolean"
-          },
-          "tagIds": {
-            "example": [
-              "bf973405-3f2a-48d2-a687-2ed4167164be",
-              "dd41870b-5d00-46d2-924e-1d8489a0aa0f",
-              "fad77c3f-deef-4e7e-9608-14c1aa4e559a"
-            ],
-            "items": {
-              "type": "string"
-            },
-            "title": "Array of tag IDs to add to the asset",
-            "type": "array"
           }
         },
         "type": "object"

diff --git a/server/src/domain/asset/asset.repository.ts b/server/src/domain/asset/asset.repository.ts
@@ -1,4 +1,4 @@
-import { AssetEntity, AssetType } from '@app/infra/entities';
+import { AssetEntity, AssetType, ExifEntity } from '@app/infra/entities';
 import { Paginated, PaginationOptions } from '../domain.util';
 
 export type AssetStats = Record<AssetType, number>;
@@ -86,4 +86,5 @@ export interface IAssetRepository {
   getStatistics(ownerId: string, options: AssetStatsOptions): Promise<AssetStats>;
   getTimeBuckets(options: TimeBucketOptions): Promise<TimeBucketItem[]>;
   getByTimeBucket(timeBucket: string, options: TimeBucketOptions): Promise<AssetEntity[]>;
+  upsertExif(exif: Partial<ExifEntity>): Promise<void>;
 }
diff --git a/server/src/domain/asset/asset.service.spec.ts b/server/src/domain/asset/asset.service.spec.ts
@@ -519,6 +519,30 @@ describe(AssetService.name, () => {
     });
   });
 
+  describe('update', () => {
+    it('should require asset write access for the id', async () => {
+      accessMock.asset.hasOwnerAccess.mockResolvedValue(false);
+      await expect(sut.update(authStub.admin, 'asset-1', { isArchived: false })).rejects.toBeInstanceOf(
+        BadRequestException,
+      );
+      expect(assetMock.save).not.toHaveBeenCalled();
+    });
+
+    it('should update the asset', async () => {
+      accessMock.asset.hasOwnerAccess.mockResolvedValue(true);
+      assetMock.save.mockResolvedValue(assetStub.image);
+      await sut.update(authStub.admin, 'asset-1', { isFavorite: true });
+      expect(assetMock.save).toHaveBeenCalledWith({ id: 'asset-1', isFavorite: true });
+    });
+
+    it('should update the exif description', async () => {
+      accessMock.asset.hasOwnerAccess.mockResolvedValue(true);
+      assetMock.save.mockResolvedValue(assetStub.image);
+      await sut.update(authStub.admin, 'asset-1', { description: 'Test description' });
+      expect(assetMock.upsertExif).toHaveBeenCalledWith({ assetId: 'asset-1', description: 'Test description' });
+    });
+  });
+
   describe('updateAll', () => {
     it('should require asset write access for all ids', async () => {
       accessMock.asset.hasOwnerAccess.mockResolvedValue(false);

diff --git a/server/src/domain/asset/asset.service.ts b/server/src/domain/asset/asset.service.ts
@@ -24,6 +24,7 @@ import {
   MemoryLaneDto,
   TimeBucketAssetDto,
   TimeBucketDto,
+  UpdateAssetDto,
   mapStats,
 } from './dto';
 import {
@@ -279,6 +280,19 @@ export class AssetService {
     return mapStats(stats);
   }
 
+  async update(authUser: AuthUserDto, id: string, dto: UpdateAssetDto): Promise<AssetResponseDto> {
+    await this.access.requirePermission(authUser, Permission.ASSET_UPDATE, id);
+
+    const { description, ...rest } = dto;
+    if (description !== undefined) {
+      await this.assetRepository.upsertExif({ assetId: id, description });
+    }
+
+    const asset = await this.assetRepository.save({ id, ...rest });
+    await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ASSET, data: { ids: [id] } });
+    return mapAsset(asset);
+  }
+
   async updateAll(authUser: AuthUserDto, dto: AssetBulkUpdateDto) {
     const { ids, ...options } = dto;
     await this.access.requirePermission(authUser, Permission.ASSET_UPDATE, ids);

diff --git a/server/src/domain/asset/dto/asset.dto.ts b/server/src/domain/asset/dto/asset.dto.ts
@@ -1,4 +1,4 @@
-import { IsBoolean } from 'class-validator';
+import { IsBoolean, IsString } from 'class-validator';
 import { Optional } from '../../domain.util';
 import { BulkIdsDto } from '../response-dto';
 
@@ -11,3 +11,17 @@ export class AssetBulkUpdateDto extends BulkIdsDto {
   @IsBoolean()
   isArchived?: boolean;
 }
+
+export class UpdateAssetDto {
+  @Optional()
+  @IsBoolean()
+  isFavorite?: boolean;
+
+  @Optional()
+  @IsBoolean()
+  isArchived?: boolean;
+
+  @Optional()
+  @IsString()
+  description?: string;
+}
diff --git a/server/src/immich/api-v1/asset/asset-repository.ts b/server/src/immich/api-v1/asset/asset-repository.ts
@@ -1,4 +1,4 @@
-import { AssetEntity, ExifEntity } from '@app/infra/entities';
+import { AssetEntity } from '@app/infra/entities';
 import { Injectable } from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';
 import { MoreThan } from 'typeorm';
@@ -7,7 +7,6 @@ import { Repository } from 'typeorm/repository/Repository';
 import { AssetSearchDto } from './dto/asset-search.dto';
 import { CheckExistingAssetsDto } from './dto/check-existing-assets.dto';
 import { SearchPropertiesDto } from './dto/search-properties.dto';
-import { UpdateAssetDto } from './dto/update-asset.dto';
 import { CuratedLocationsResponseDto } from './response-dto/curated-locations-response.dto';
 import { CuratedObjectsResponseDto } from './response-dto/curated-objects-response.dto';
 
@@ -26,7 +25,6 @@ export interface IAssetRepository {
     asset: Omit<AssetEntity, 'id' | 'createdAt' | 'updatedAt' | 'ownerId' | 'livePhotoVideoId'>,
   ): Promise<AssetEntity>;
   remove(asset: AssetEntity): Promise<void>;
-  update(userId: string, asset: AssetEntity, dto: UpdateAssetDto): Promise<AssetEntity>;
   getAllByUserId(userId: string, dto: AssetSearchDto): Promise<AssetEntity[]>;
   getAllByDeviceId(userId: string, deviceId: string): Promise<string[]>;
   getById(assetId: string): Promise<AssetEntity>;
@@ -42,10 +40,7 @@ export const IAssetRepository = 'IAssetRepository';
 
 @Injectable()
 export class AssetRepository implements IAssetRepository {
-  constructor(
-    @InjectRepository(AssetEntity) private assetRepository: Repository<AssetEntity>,
-    @InjectRepository(ExifEntity) private exifRepository: Repository<ExifEntity>,
-  ) {}
+  constructor(@InjectRepository(AssetEntity) private assetRepository: Repository<AssetEntity>) {}
 
   getSearchPropertiesByUserId(userId: string): Promise<SearchPropertiesDto[]> {
     return this.assetRepository
@@ -164,40 +159,6 @@ export class AssetRepository implements IAssetRepository {
     await this.assetRepository.remove(asset);
   }
 
-  /**
-   * Update asset
-   */
-  async update(userId: string, asset: AssetEntity, dto: UpdateAssetDto): Promise<AssetEntity> {
-    asset.isFavorite = dto.isFavorite ?? asset.isFavorite;
-    asset.isArchived = dto.isArchived ?? asset.isArchived;
-
-    if (asset.exifInfo != null) {
-      if (dto.description !== undefined) {
-        asset.exifInfo.description = dto.description;
-      }
-      await this.exifRepository.save(asset.exifInfo);
-    } else {
-      const exifInfo = new ExifEntity();
-      if (dto.description !== undefined) {
-        exifInfo.description = dto.description;
-      }
-      exifInfo.asset = asset;
-      await this.exifRepository.save(exifInfo);
-      asset.exifInfo = exifInfo;
-    }
-
-    await this.assetRepository.update(asset.id, {
-      isFavorite: asset.isFavorite,
-      isArchived: asset.isArchived,
-    });
-
-    return this.assetRepository.findOneOrFail({
-      where: {
-        id: asset.id,
-      },
-    });
-  }
-
   /**
    * Get assets by device's Id on the database
    * @param ownerId

diff --git a/server/src/immich/api-v1/asset/asset.controller.ts b/server/src/immich/api-v1/asset/asset.controller.ts
@@ -9,7 +9,6 @@ import {
   Param,
   ParseFilePipe,
   Post,
-  Put,
   Query,
   Response,
   UploadedFiles,
@@ -33,7 +32,6 @@ import { DeviceIdDto } from './dto/device-id.dto';
 import { GetAssetThumbnailDto } from './dto/get-asset-thumbnail.dto';
 import { SearchAssetDto } from './dto/search-asset.dto';
 import { ServeFileDto } from './dto/serve-file.dto';
-import { UpdateAssetDto } from './dto/update-asset.dto';
 import { AssetBulkUploadCheckResponseDto } from './response-dto/asset-check-response.dto';
 import { AssetFileUploadResponseDto } from './response-dto/asset-file-upload-response.dto';
 import { CheckDuplicateAssetResponseDto } from './response-dto/check-duplicate-asset-response.dto';
@@ -194,18 +192,6 @@ export class AssetController {
     return this.assetService.getAssetById(authUser, id);
   }
 
-  /**
-   * Update an asset
-   */
-  @Put('/:id')
-  updateAsset(
-    @AuthUser() authUser: AuthUserDto,
-    @Param() { id }: UUIDParamDto,
-    @Body(ValidationPipe) dto: UpdateAssetDto,
-  ): Promise<AssetResponseDto> {
-    return this.assetService.updateAsset(authUser, id, dto);
-  }
-
   @Delete('/')
   deleteAsset(
     @AuthUser() authUser: AuthUserDto,