refactor(server): update asset endpoint (#3973)

* refactor(server): update asset

* chore: open api

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

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

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

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

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

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,

server/src/immich/api-v1/asset/asset.service.spec.ts

@@ -96,7 +96,6 @@ describe('AssetService', () => {
       create: jest.fn(),
       remove: jest.fn(),
 
-      update: jest.fn(),
       getAllByUserId: jest.fn(),
       getAllByDeviceId: jest.fn(),
       getById: jest.fn(),

server/src/immich/api-v1/asset/asset.service.ts

@@ -41,7 +41,6 @@ import { GetAssetThumbnailDto, GetAssetThumbnailFormatEnum } from './dto/get-ass
 import { SearchAssetDto } from './dto/search-asset.dto';
 import { SearchPropertiesDto } from './dto/search-properties.dto';
 import { ServeFileDto } from './dto/serve-file.dto';
-import { UpdateAssetDto } from './dto/update-asset.dto';
 import {
   AssetBulkUploadCheckResponseDto,
   AssetRejectReason,
@@ -203,21 +202,6 @@ export class AssetService {
     return data;
   }
 
-  public async updateAsset(authUser: AuthUserDto, assetId: string, dto: UpdateAssetDto): Promise<AssetResponseDto> {
-    await this.access.requirePermission(authUser, Permission.ASSET_UPDATE, assetId);
-
-    const asset = await this._assetRepository.getById(assetId);
-    if (!asset) {
-      throw new BadRequestException('Asset not found');
-    }
-
-    const updatedAsset = await this._assetRepository.update(authUser.id, asset, dto);
-
-    await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ASSET, data: { ids: [assetId] } });
-
-    return mapAsset(updatedAsset);
-  }
-
   async serveThumbnail(authUser: AuthUserDto, assetId: string, query: GetAssetThumbnailDto, res: Res) {
     await this.access.requirePermission(authUser, Permission.ASSET_VIEW, assetId);
 

server/src/immich/api-v1/asset/dto/update-asset.dto.ts

@@ -1,33 +0,0 @@
-import { Optional } from '@app/domain';
-import { ApiProperty } from '@nestjs/swagger';
-import { IsArray, IsBoolean, IsNotEmpty, IsString } from 'class-validator';
-
-export class UpdateAssetDto {
-  @Optional()
-  @IsBoolean()
-  isFavorite?: boolean;
-
-  @Optional()
-  @IsBoolean()
-  isArchived?: boolean;
-
-  @Optional()
-  @IsArray()
-  @IsString({ each: true })
-  @IsNotEmpty({ each: true })
-  @ApiProperty({
-    isArray: true,
-    type: String,
-    title: 'Array of tag IDs to add to the asset',
-    example: [
-      'bf973405-3f2a-48d2-a687-2ed4167164be',
-      'dd41870b-5d00-46d2-924e-1d8489a0aa0f',
-      'fad77c3f-deef-4e7e-9608-14c1aa4e559a',
-    ],
-  })
-  tagIds?: string[];
-
-  @Optional()
-  @IsString()
-  description?: string;
-}

server/src/immich/app.module.ts

@@ -1,6 +1,6 @@
 import { DomainModule } from '@app/domain';
 import { InfraModule } from '@app/infra';
-import { AssetEntity, ExifEntity } from '@app/infra/entities';
+import { AssetEntity } from '@app/infra/entities';
 import { Module, OnModuleDestroy, OnModuleInit } from '@nestjs/common';
 import { APP_GUARD } from '@nestjs/core';
 import { ScheduleModule } from '@nestjs/schedule';
@@ -35,7 +35,7 @@ import {
     //
     DomainModule.register({ imports: [InfraModule] }),
     ScheduleModule.forRoot(),
-    TypeOrmModule.forFeature([AssetEntity, ExifEntity]),
+    TypeOrmModule.forFeature([AssetEntity]),
   ],
   controllers: [
     AssetController,

server/src/immich/controllers/asset.controller.ts

@@ -16,6 +16,7 @@ import {
   TimeBucketAssetDto,
   TimeBucketDto,
   TimeBucketResponseDto,
+  UpdateAssetDto as UpdateDto,
 } from '@app/domain';
 import { Body, Controller, Get, HttpCode, HttpStatus, Param, Post, Put, Query, StreamableFile } from '@nestjs/common';
 import { ApiOkResponse, ApiTags } from '@nestjs/swagger';
@@ -90,4 +91,13 @@ export class AssetController {
   updateAssets(@AuthUser() authUser: AuthUserDto, @Body() dto: AssetBulkUpdateDto): Promise<void> {
     return this.service.updateAll(authUser, dto);
   }
+
+  @Put(':id')
+  updateAsset(
+    @AuthUser() authUser: AuthUserDto,
+    @Param() { id }: UUIDParamDto,
+    @Body() dto: UpdateDto,
+  ): Promise<AssetResponseDto> {
+    return this.service.update(authUser, id, dto);
+  }
 }

server/src/infra/entities/index.ts

@@ -3,6 +3,7 @@ import { APIKeyEntity } from './api-key.entity';
 import { AssetFaceEntity } from './asset-face.entity';
 import { AssetEntity } from './asset.entity';
 import { AuditEntity } from './audit.entity';
+import { ExifEntity } from './exif.entity';
 import { PartnerEntity } from './partner.entity';
 import { PersonEntity } from './person.entity';
 import { SharedLinkEntity } from './shared-link.entity';
@@ -33,6 +34,7 @@ export const databaseEntities = [
   AssetEntity,
   AssetFaceEntity,
   AuditEntity,
+  ExifEntity,
   PartnerEntity,
   PersonEntity,
   SharedLinkEntity,

server/src/infra/repositories/asset.repository.ts

@@ -18,7 +18,7 @@ import { Injectable } from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';
 import { DateTime } from 'luxon';
 import { FindOptionsRelations, FindOptionsWhere, In, IsNull, Not, Repository } from 'typeorm';
-import { AssetEntity, AssetType } from '../entities';
+import { AssetEntity, AssetType, ExifEntity } from '../entities';
 import OptionalBetween from '../utils/optional-between.util';
 import { paginate } from '../utils/pagination.util';
 
@@ -29,7 +29,14 @@ const truncateMap: Record<TimeBucketSize, string> = {
 
 @Injectable()
 export class AssetRepository implements IAssetRepository {
-  constructor(@InjectRepository(AssetEntity) private repository: Repository<AssetEntity>) {}
+  constructor(
+    @InjectRepository(AssetEntity) private repository: Repository<AssetEntity>,
+    @InjectRepository(ExifEntity) private exifRepository: Repository<ExifEntity>,
+  ) {}
+
+  async upsertExif(exif: Partial<ExifEntity>): Promise<void> {
+    await this.exifRepository.upsert(exif, { conflictPaths: ['assetId'] });
+  }
 
   getByDate(ownerId: string, date: Date): Promise<AssetEntity[]> {
     // For reference of a correct approach although slower

server/src/microservices/microservices.module.ts

@@ -1,17 +1,11 @@
 import { DomainModule } from '@app/domain';
 import { InfraModule } from '@app/infra';
-import { ExifEntity } from '@app/infra/entities';
 import { Module } from '@nestjs/common';
-import { TypeOrmModule } from '@nestjs/typeorm';
 import { AppService } from './app.service';
 import { MetadataExtractionProcessor } from './processors/metadata-extraction.processor';
 
 @Module({
-  imports: [
-    //
-    DomainModule.register({ imports: [InfraModule] }),
-    TypeOrmModule.forFeature([ExifEntity]),
-  ],
+  imports: [DomainModule.register({ imports: [InfraModule] })],
   providers: [MetadataExtractionProcessor, AppService],
 })
 export class MicroservicesModule {}

server/src/microservices/processors/metadata-extraction.processor.ts

@@ -18,7 +18,6 @@ import {
 import { AssetEntity, AssetType, ExifEntity } from '@app/infra/entities';
 import { Inject, Logger } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
-import { InjectRepository } from '@nestjs/typeorm';
 import tz_lookup from '@photostructure/tz-lookup';
 import { exiftool, Tags } from 'exiftool-vendored';
 import ffmpeg, { FfprobeData } from 'fluent-ffmpeg';
@@ -26,7 +25,6 @@ import { Duration } from 'luxon';
 import fs from 'node:fs';
 import path from 'node:path';
 import sharp from 'sharp';
-import { Repository } from 'typeorm/repository/Repository';
 import { promisify } from 'util';
 import { parseLatitude, parseLongitude } from '../utils/exif/coordinates';
 import { exifTimeZone, exifToDate } from '../utils/exif/date-time';
@@ -65,7 +63,6 @@ export class MetadataExtractionProcessor {
     @Inject(IGeocodingRepository) private geocodingRepository: IGeocodingRepository,
     @Inject(ICryptoRepository) private cryptoRepository: ICryptoRepository,
     @Inject(IStorageRepository) private storageRepository: IStorageRepository,
-    @InjectRepository(ExifEntity) private exifRepository: Repository<ExifEntity>,
 
     configService: ConfigService,
   ) {
@@ -407,7 +404,7 @@ export class MetadataExtractionProcessor {
       }
     }
 
-    await this.exifRepository.upsert(newExif, { conflictPaths: ['assetId'] });
+    await this.assetRepository.upsertExif(newExif);
     await this.assetRepository.save({
       id: asset.id,
       fileCreatedAt: fileCreatedAt || undefined,
@@ -506,7 +503,7 @@ export class MetadataExtractionProcessor {
       }
     }
 
-    await this.exifRepository.upsert(newExif, { conflictPaths: ['assetId'] });
+    await this.assetRepository.upsertExif(newExif);
     await this.assetRepository.save({ id: asset.id, duration: durationString, fileCreatedAt });
 
     return true;