From 454737ca793f655af033aaaaf5cbfc8032c45bce Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Mon, 4 Sep 2023 22:25:31 -0400 Subject: [PATCH] refactor(server): update asset endpoint (#3973) * refactor(server): update asset * chore: open api --- cli/src/api/open-api/api.ts | 14 +- mobile/openapi/doc/AssetApi.md | 2 - mobile/openapi/doc/UpdateAssetDto.md | 1 - mobile/openapi/lib/api/asset_api.dart | 7 +- .../openapi/lib/model/update_asset_dto.dart | 15 +- mobile/openapi/test/asset_api_test.dart | 2 - .../openapi/test/update_asset_dto_test.dart | 5 - server/immich-openapi-specs.json | 13 -- server/src/domain/asset/asset.repository.ts | 3 +- server/src/domain/asset/asset.service.spec.ts | 24 ++++ server/src/domain/asset/asset.service.ts | 14 ++ server/src/domain/asset/dto/asset.dto.ts | 16 ++- .../immich/api-v1/asset/asset-repository.ts | 43 +----- .../immich/api-v1/asset/asset.controller.ts | 14 -- .../immich/api-v1/asset/asset.service.spec.ts | 1 - .../src/immich/api-v1/asset/asset.service.ts | 16 --- .../api-v1/asset/dto/update-asset.dto.ts | 33 ----- server/src/immich/app.module.ts | 4 +- .../immich/controllers/asset.controller.ts | 10 ++ server/src/infra/entities/index.ts | 2 + .../infra/repositories/asset.repository.ts | 11 +- .../src/microservices/microservices.module.ts | 8 +- .../metadata-extraction.processor.ts | 7 +- server/test/e2e/asset.e2e-spec.ts | 136 ++++++++++++++++++ server/test/fixtures/error.stub.ts | 5 + .../repositories/asset.repository.mock.ts | 1 + web/src/api/open-api/api.ts | 14 +- 27 files changed, 237 insertions(+), 184 deletions(-) delete mode 100644 server/src/immich/api-v1/asset/dto/update-asset.dto.ts create mode 100644 server/test/e2e/asset.e2e-spec.ts diff --git a/cli/src/api/open-api/api.ts b/cli/src/api/open-api/api.ts index 56167aaf3..2301c02e1 100644 --- a/cli/src/api/open-api/api.ts +++ b/cli/src/api/open-api/api.ts @@ -3417,12 +3417,6 @@ export interface UpdateAssetDto { * @memberof UpdateAssetDto */ 'isFavorite'?: boolean; - /** - * - * @type {Array} - * @memberof UpdateAssetDto - */ - 'tagIds'?: Array; } /** * @@ -6299,7 +6293,7 @@ export const AssetApiAxiosParamCreator = function (configuration?: Configuration }; }, /** - * Update an asset + * * @param {string} id * @param {UpdateAssetDto} updateAssetDto * @param {*} [options] Override http request option. @@ -6778,7 +6772,7 @@ export const AssetApiFp = function(configuration?: Configuration) { return createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration); }, /** - * Update an asset + * * @param {string} id * @param {UpdateAssetDto} updateAssetDto * @param {*} [options] Override http request option. @@ -7035,7 +7029,7 @@ export const AssetApiFactory = function (configuration?: Configuration, basePath return localVarFp.serveFile(requestParameters.id, requestParameters.isThumb, requestParameters.isWeb, requestParameters.key, options).then((request) => request(axios, basePath)); }, /** - * Update an asset + * * @param {AssetApiUpdateAssetRequest} requestParameters Request parameters. * @param {*} [options] Override http request option. * @throws {RequiredError} @@ -7952,7 +7946,7 @@ export class AssetApi extends BaseAPI { } /** - * Update an asset + * * @param {AssetApiUpdateAssetRequest} requestParameters Request parameters. * @param {*} [options] Override http request option. * @throws {RequiredError} diff --git a/mobile/openapi/doc/AssetApi.md b/mobile/openapi/doc/AssetApi.md index 4bec9fa2a..f977c8ed4 100644 --- a/mobile/openapi/doc/AssetApi.md +++ b/mobile/openapi/doc/AssetApi.md @@ -1368,8 +1368,6 @@ Name | Type | Description | Notes -Update an asset - ### Example ```dart import 'package:openapi/api.dart'; diff --git a/mobile/openapi/doc/UpdateAssetDto.md b/mobile/openapi/doc/UpdateAssetDto.md index 3e14538b3..d214ebd47 100644 --- a/mobile/openapi/doc/UpdateAssetDto.md +++ b/mobile/openapi/doc/UpdateAssetDto.md @@ -11,7 +11,6 @@ Name | Type | Description | Notes **description** | **String** | | [optional] **isArchived** | **bool** | | [optional] **isFavorite** | **bool** | | [optional] -**tagIds** | **List** | | [optional] [default to const []] [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md) diff --git a/mobile/openapi/lib/api/asset_api.dart b/mobile/openapi/lib/api/asset_api.dart index 9067f8dd0..86ef067b9 100644 --- a/mobile/openapi/lib/api/asset_api.dart +++ b/mobile/openapi/lib/api/asset_api.dart @@ -1384,10 +1384,7 @@ class AssetApi { return null; } - /// Update an asset - /// - /// Note: This method returns the HTTP [Response]. - /// + /// Performs an HTTP 'PUT /asset/{id}' operation and returns the [Response]. /// Parameters: /// /// * [String] id (required): @@ -1419,8 +1416,6 @@ class AssetApi { ); } - /// Update an asset - /// /// Parameters: /// /// * [String] id (required): diff --git a/mobile/openapi/lib/model/update_asset_dto.dart b/mobile/openapi/lib/model/update_asset_dto.dart index 20368f9d2..d1f3570ef 100644 --- a/mobile/openapi/lib/model/update_asset_dto.dart +++ b/mobile/openapi/lib/model/update_asset_dto.dart @@ -16,7 +16,6 @@ class UpdateAssetDto { this.description, this.isArchived, this.isFavorite, - this.tagIds = const [], }); /// @@ -43,25 +42,21 @@ class UpdateAssetDto { /// bool? isFavorite; - List tagIds; - @override bool operator ==(Object other) => identical(this, other) || other is UpdateAssetDto && other.description == description && other.isArchived == isArchived && - other.isFavorite == isFavorite && - other.tagIds == tagIds; + other.isFavorite == isFavorite; @override int get hashCode => // ignore: unnecessary_parenthesis (description == null ? 0 : description!.hashCode) + (isArchived == null ? 0 : isArchived!.hashCode) + - (isFavorite == null ? 0 : isFavorite!.hashCode) + - (tagIds.hashCode); + (isFavorite == null ? 0 : isFavorite!.hashCode); @override - String toString() => 'UpdateAssetDto[description=$description, isArchived=$isArchived, isFavorite=$isFavorite, tagIds=$tagIds]'; + String toString() => 'UpdateAssetDto[description=$description, isArchived=$isArchived, isFavorite=$isFavorite]'; Map toJson() { final json = {}; @@ -80,7 +75,6 @@ class UpdateAssetDto { } else { // json[r'isFavorite'] = null; } - json[r'tagIds'] = this.tagIds; return json; } @@ -95,9 +89,6 @@ class UpdateAssetDto { description: mapValueOfType(json, r'description'), isArchived: mapValueOfType(json, r'isArchived'), isFavorite: mapValueOfType(json, r'isFavorite'), - tagIds: json[r'tagIds'] is List - ? (json[r'tagIds'] as List).cast() - : const [], ); } return null; diff --git a/mobile/openapi/test/asset_api_test.dart b/mobile/openapi/test/asset_api_test.dart index ea45fc7d1..ecbfdf4c2 100644 --- a/mobile/openapi/test/asset_api_test.dart +++ b/mobile/openapi/test/asset_api_test.dart @@ -144,8 +144,6 @@ void main() { // TODO }); - // Update an asset - // //Future updateAsset(String id, UpdateAssetDto updateAssetDto) async test('test updateAsset', () async { // TODO diff --git a/mobile/openapi/test/update_asset_dto_test.dart b/mobile/openapi/test/update_asset_dto_test.dart index 57ebfcee2..b2966e961 100644 --- a/mobile/openapi/test/update_asset_dto_test.dart +++ b/mobile/openapi/test/update_asset_dto_test.dart @@ -31,11 +31,6 @@ void main() { // TODO }); - // List tagIds (default value: const []) - test('to test the property `tagIds`', () async { - // TODO - }); - }); diff --git a/server/immich-openapi-specs.json b/server/immich-openapi-specs.json index c38db8cbf..2856819c5 100644 --- 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 index d1e7a8fe6..dfa953966 100644 --- 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; @@ -86,4 +86,5 @@ export interface IAssetRepository { getStatistics(ownerId: string, options: AssetStatsOptions): Promise; getTimeBuckets(options: TimeBucketOptions): Promise; getByTimeBucket(timeBucket: string, options: TimeBucketOptions): Promise; + upsertExif(exif: Partial): Promise; } diff --git a/server/src/domain/asset/asset.service.spec.ts b/server/src/domain/asset/asset.service.spec.ts index cdb1f23e6..457690014 100644 --- 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 index fcba677b4..d85fe175a 100644 --- 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 { + 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 index 7bab8ffb2..f6ba61b20 100644 --- 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 index a87a115e9..0466b3668 100644 --- 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, ): Promise; remove(asset: AssetEntity): Promise; - update(userId: string, asset: AssetEntity, dto: UpdateAssetDto): Promise; getAllByUserId(userId: string, dto: AssetSearchDto): Promise; getAllByDeviceId(userId: string, deviceId: string): Promise; getById(assetId: string): Promise; @@ -42,10 +40,7 @@ export const IAssetRepository = 'IAssetRepository'; @Injectable() export class AssetRepository implements IAssetRepository { - constructor( - @InjectRepository(AssetEntity) private assetRepository: Repository, - @InjectRepository(ExifEntity) private exifRepository: Repository, - ) {} + constructor(@InjectRepository(AssetEntity) private assetRepository: Repository) {} getSearchPropertiesByUserId(userId: string): Promise { 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 { - 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 index 5ce5ff8cb..52d9f40c5 100644 --- 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 { - return this.assetService.updateAsset(authUser, id, dto); - } - @Delete('/') deleteAsset( @AuthUser() authUser: AuthUserDto, diff --git a/server/src/immich/api-v1/asset/asset.service.spec.ts b/server/src/immich/api-v1/asset/asset.service.spec.ts index ef57cf6a9..8145888ce 100644 --- a/server/src/immich/api-v1/asset/asset.service.spec.ts +++ b/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(), diff --git a/server/src/immich/api-v1/asset/asset.service.ts b/server/src/immich/api-v1/asset/asset.service.ts index b397b2688..77b3f73e0 100644 --- a/server/src/immich/api-v1/asset/asset.service.ts +++ b/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 { - 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); diff --git a/server/src/immich/api-v1/asset/dto/update-asset.dto.ts b/server/src/immich/api-v1/asset/dto/update-asset.dto.ts deleted file mode 100644 index 37eb3cb9f..000000000 --- a/server/src/immich/api-v1/asset/dto/update-asset.dto.ts +++ /dev/null @@ -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; -} diff --git a/server/src/immich/app.module.ts b/server/src/immich/app.module.ts index e82de2bce..a2b01cf8c 100644 --- a/server/src/immich/app.module.ts +++ b/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, diff --git a/server/src/immich/controllers/asset.controller.ts b/server/src/immich/controllers/asset.controller.ts index 258b485ef..2e6bb4774 100644 --- a/server/src/immich/controllers/asset.controller.ts +++ b/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 { return this.service.updateAll(authUser, dto); } + + @Put(':id') + updateAsset( + @AuthUser() authUser: AuthUserDto, + @Param() { id }: UUIDParamDto, + @Body() dto: UpdateDto, + ): Promise { + return this.service.update(authUser, id, dto); + } } diff --git a/server/src/infra/entities/index.ts b/server/src/infra/entities/index.ts index 632a8d6b4..c2a7ad797 100644 --- a/server/src/infra/entities/index.ts +++ b/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, diff --git a/server/src/infra/repositories/asset.repository.ts b/server/src/infra/repositories/asset.repository.ts index 2d4a0e91c..fd5c23475 100644 --- a/server/src/infra/repositories/asset.repository.ts +++ b/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 = { @Injectable() export class AssetRepository implements IAssetRepository { - constructor(@InjectRepository(AssetEntity) private repository: Repository) {} + constructor( + @InjectRepository(AssetEntity) private repository: Repository, + @InjectRepository(ExifEntity) private exifRepository: Repository, + ) {} + + async upsertExif(exif: Partial): Promise { + await this.exifRepository.upsert(exif, { conflictPaths: ['assetId'] }); + } getByDate(ownerId: string, date: Date): Promise { // For reference of a correct approach although slower diff --git a/server/src/microservices/microservices.module.ts b/server/src/microservices/microservices.module.ts index 7dabbc145..28abb6571 100644 --- a/server/src/microservices/microservices.module.ts +++ b/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 {} diff --git a/server/src/microservices/processors/metadata-extraction.processor.ts b/server/src/microservices/processors/metadata-extraction.processor.ts index 5d421ebfd..aa9704bfd 100644 --- a/server/src/microservices/processors/metadata-extraction.processor.ts +++ b/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, 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; diff --git a/server/test/e2e/asset.e2e-spec.ts b/server/test/e2e/asset.e2e-spec.ts new file mode 100644 index 000000000..3a43563df --- /dev/null +++ b/server/test/e2e/asset.e2e-spec.ts @@ -0,0 +1,136 @@ +import { IAssetRepository, LoginResponseDto } from '@app/domain'; +import { AppModule, AssetController } from '@app/immich'; +import { AssetEntity, AssetType } from '@app/infra/entities'; +import { INestApplication } from '@nestjs/common'; +import { Test, TestingModule } from '@nestjs/testing'; +import { randomBytes } from 'crypto'; +import request from 'supertest'; +import { errorStub, uuidStub } from '../fixtures'; +import { api, db } from '../test-utils'; + +const user1Dto = { + email: 'user1@immich.app', + password: 'Password123', + firstName: 'User 1', + lastName: 'Test', +}; + +const user2Dto = { + email: 'user2@immich.app', + password: 'Password123', + firstName: 'User 2', + lastName: 'Test', +}; + +let assetCount = 0; +const createAsset = (repository: IAssetRepository, loginResponse: LoginResponseDto): Promise => { + const id = assetCount++; + return repository.save({ + ownerId: loginResponse.userId, + checksum: randomBytes(20), + originalPath: `/tests/test_${id}`, + deviceAssetId: `test_${id}`, + deviceId: 'e2e-test', + fileCreatedAt: new Date(), + fileModifiedAt: new Date(), + type: AssetType.IMAGE, + originalFileName: `test_${id}`, + }); +}; + +describe(`${AssetController.name} (e2e)`, () => { + let app: INestApplication; + let server: any; + let assetRepository: IAssetRepository; + let user1: LoginResponseDto; + let user2: LoginResponseDto; + let asset1: AssetEntity; + let asset2: AssetEntity; + + beforeAll(async () => { + const moduleFixture: TestingModule = await Test.createTestingModule({ + imports: [AppModule], + }).compile(); + + app = await moduleFixture.createNestApplication().init(); + server = app.getHttpServer(); + assetRepository = app.get(IAssetRepository); + }); + + beforeEach(async () => { + await db.reset(); + await api.adminSignUp(server); + const admin = await api.adminLogin(server); + + await api.userApi.create(server, admin.accessToken, user1Dto); + user1 = await api.login(server, { email: user1Dto.email, password: user1Dto.password }); + asset1 = await createAsset(assetRepository, user1); + + await api.userApi.create(server, admin.accessToken, user2Dto); + user2 = await api.login(server, { email: user2Dto.email, password: user2Dto.password }); + asset2 = await createAsset(assetRepository, user2); + }); + + afterAll(async () => { + await db.disconnect(); + await app.close(); + }); + + describe('PUT /asset/:id', () => { + it('should require authentication', async () => { + const { status, body } = await request(server).put(`/asset/:${uuidStub.notFound}`); + expect(status).toBe(401); + expect(body).toEqual(errorStub.unauthorized); + }); + + it('should require a valid id', async () => { + const { status, body } = await request(server) + .put(`/asset/${uuidStub.invalid}`) + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(400); + expect(body).toEqual(errorStub.badRequest); + }); + + it('should require access', async () => { + const { status, body } = await request(server) + .put(`/asset/${asset2.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(400); + expect(body).toEqual(errorStub.noPermission); + }); + + it('should favorite an asset', async () => { + expect(asset1).toMatchObject({ isFavorite: false }); + + const { status, body } = await request(server) + .put(`/asset/${asset1.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`) + .send({ isFavorite: true }); + expect(body).toMatchObject({ id: asset1.id, isFavorite: true }); + expect(status).toEqual(200); + }); + + it('should archive an asset', async () => { + expect(asset1).toMatchObject({ isArchived: false }); + + const { status, body } = await request(server) + .put(`/asset/${asset1.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`) + .send({ isArchived: true }); + expect(body).toMatchObject({ id: asset1.id, isArchived: true }); + expect(status).toEqual(200); + }); + + it('should set the description', async () => { + const { status, body } = await request(server) + .put(`/asset/${asset1.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`) + .send({ description: 'Test asset description' }); + expect(body).toMatchObject({ + id: asset1.id, + exifInfo: expect.objectContaining({ description: 'Test asset description' }), + }); + expect(status).toEqual(200); + }); + }); +}); diff --git a/server/test/fixtures/error.stub.ts b/server/test/fixtures/error.stub.ts index 4cfc95809..0eb487053 100644 --- a/server/test/fixtures/error.stub.ts +++ b/server/test/fixtures/error.stub.ts @@ -24,6 +24,11 @@ export const errorStub = { statusCode: 400, message: expect.any(Array), }, + noPermission: { + error: 'Bad Request', + statusCode: 400, + message: expect.stringContaining('Not found or no'), + }, incorrectLogin: { error: 'Unauthorized', statusCode: 401, diff --git a/server/test/repositories/asset.repository.mock.ts b/server/test/repositories/asset.repository.mock.ts index ecd5d5c10..a9a13185d 100644 --- a/server/test/repositories/asset.repository.mock.ts +++ b/server/test/repositories/asset.repository.mock.ts @@ -2,6 +2,7 @@ import { IAssetRepository } from '@app/domain'; export const newAssetRepositoryMock = (): jest.Mocked => { return { + upsertExif: jest.fn(), getByDate: jest.fn(), getByIds: jest.fn().mockResolvedValue([]), getByAlbumId: jest.fn(), diff --git a/web/src/api/open-api/api.ts b/web/src/api/open-api/api.ts index 56167aaf3..2301c02e1 100644 --- a/web/src/api/open-api/api.ts +++ b/web/src/api/open-api/api.ts @@ -3417,12 +3417,6 @@ export interface UpdateAssetDto { * @memberof UpdateAssetDto */ 'isFavorite'?: boolean; - /** - * - * @type {Array} - * @memberof UpdateAssetDto - */ - 'tagIds'?: Array; } /** * @@ -6299,7 +6293,7 @@ export const AssetApiAxiosParamCreator = function (configuration?: Configuration }; }, /** - * Update an asset + * * @param {string} id * @param {UpdateAssetDto} updateAssetDto * @param {*} [options] Override http request option. @@ -6778,7 +6772,7 @@ export const AssetApiFp = function(configuration?: Configuration) { return createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration); }, /** - * Update an asset + * * @param {string} id * @param {UpdateAssetDto} updateAssetDto * @param {*} [options] Override http request option. @@ -7035,7 +7029,7 @@ export const AssetApiFactory = function (configuration?: Configuration, basePath return localVarFp.serveFile(requestParameters.id, requestParameters.isThumb, requestParameters.isWeb, requestParameters.key, options).then((request) => request(axios, basePath)); }, /** - * Update an asset + * * @param {AssetApiUpdateAssetRequest} requestParameters Request parameters. * @param {*} [options] Override http request option. * @throws {RequiredError} @@ -7952,7 +7946,7 @@ export class AssetApi extends BaseAPI { } /** - * Update an asset + * * @param {AssetApiUpdateAssetRequest} requestParameters Request parameters. * @param {*} [options] Override http request option. * @throws {RequiredError}