From e4f9fce47106490241bc2de45ba2ef243f95df09 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Wed, 14 Aug 2024 10:49:57 -0400 Subject: [PATCH] feat(server): do not automatically download embedded android motion videos --- e2e/docker-compose.yml | 2 -- e2e/src/api/specs/user.e2e-spec.ts | 26 ++++++++++++++++ .../openapi/lib/model/download_response.dart | 14 +++++++-- mobile/openapi/lib/model/download_update.dart | 23 ++++++++++++-- open-api/immich-openapi-specs.json | 10 +++++- open-api/typescript-sdk/src/fetch-client.ts | 2 ++ server/src/dtos/user-preferences.dto.ts | 7 ++++- server/src/entities/user-metadata.entity.ts | 2 ++ server/src/services/download.service.spec.ts | 26 ++++++++++++++++ server/src/services/download.service.ts | 16 ++++++++-- .../download-settings.svelte | 31 +++++++++++++------ web/src/lib/i18n/en.json | 4 ++- web/src/lib/utils/asset-utils.ts | 16 +++++++--- 13 files changed, 151 insertions(+), 28 deletions(-) diff --git a/e2e/docker-compose.yml b/e2e/docker-compose.yml index 436613d4a85224..b45ea4137f25a9 100644 --- a/e2e/docker-compose.yml +++ b/e2e/docker-compose.yml @@ -1,5 +1,3 @@ -version: '3.8' - name: immich-e2e services: diff --git a/e2e/src/api/specs/user.e2e-spec.ts b/e2e/src/api/specs/user.e2e-spec.ts index 15fe3de3bec377..1964dc67936421 100644 --- a/e2e/src/api/specs/user.e2e-spec.ts +++ b/e2e/src/api/specs/user.e2e-spec.ts @@ -236,6 +236,32 @@ describe('/users', () => { const after = await getMyPreferences({ headers: asBearerAuth(admin.accessToken) }); expect(after).toMatchObject({ download: { archiveSize: 1_234_567 } }); }); + + it('should require a boolean for download include embedded videos', async () => { + const { status, body } = await request(app) + .put(`/users/me/preferences`) + .send({ download: { includeEmbeddedVideos: 1_234_567.89 } }) + .set('Authorization', `Bearer ${admin.accessToken}`); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(['download.includeEmbeddedVideos must be a boolean value'])); + }); + + it('should update download include embedded videos', async () => { + const before = await getMyPreferences({ headers: asBearerAuth(admin.accessToken) }); + expect(before).toMatchObject({ download: { includeEmbeddedVideos: false } }); + + const { status, body } = await request(app) + .put(`/users/me/preferences`) + .send({ download: { includeEmbeddedVideos: true } }) + .set('Authorization', `Bearer ${admin.accessToken}`); + + expect(status).toBe(200); + expect(body).toMatchObject({ download: { includeEmbeddedVideos: true } }); + + const after = await getMyPreferences({ headers: asBearerAuth(admin.accessToken) }); + expect(after).toMatchObject({ download: { includeEmbeddedVideos: true } }); + }); }); describe('GET /users/:id', () => { diff --git a/mobile/openapi/lib/model/download_response.dart b/mobile/openapi/lib/model/download_response.dart index 8973e17ebe474b..25c5159a8b6557 100644 --- a/mobile/openapi/lib/model/download_response.dart +++ b/mobile/openapi/lib/model/download_response.dart @@ -14,25 +14,31 @@ class DownloadResponse { /// Returns a new [DownloadResponse] instance. DownloadResponse({ required this.archiveSize, + this.includeEmbeddedVideos = false, }); int archiveSize; + bool includeEmbeddedVideos; + @override bool operator ==(Object other) => identical(this, other) || other is DownloadResponse && - other.archiveSize == archiveSize; + other.archiveSize == archiveSize && + other.includeEmbeddedVideos == includeEmbeddedVideos; @override int get hashCode => // ignore: unnecessary_parenthesis - (archiveSize.hashCode); + (archiveSize.hashCode) + + (includeEmbeddedVideos.hashCode); @override - String toString() => 'DownloadResponse[archiveSize=$archiveSize]'; + String toString() => 'DownloadResponse[archiveSize=$archiveSize, includeEmbeddedVideos=$includeEmbeddedVideos]'; Map toJson() { final json = {}; json[r'archiveSize'] = this.archiveSize; + json[r'includeEmbeddedVideos'] = this.includeEmbeddedVideos; return json; } @@ -45,6 +51,7 @@ class DownloadResponse { return DownloadResponse( archiveSize: mapValueOfType(json, r'archiveSize')!, + includeEmbeddedVideos: mapValueOfType(json, r'includeEmbeddedVideos')!, ); } return null; @@ -93,6 +100,7 @@ class DownloadResponse { /// The list of required keys that must be present in a JSON. static const requiredKeys = { 'archiveSize', + 'includeEmbeddedVideos', }; } diff --git a/mobile/openapi/lib/model/download_update.dart b/mobile/openapi/lib/model/download_update.dart index 1629706415de30..2c3839a6878dc7 100644 --- a/mobile/openapi/lib/model/download_update.dart +++ b/mobile/openapi/lib/model/download_update.dart @@ -14,6 +14,7 @@ class DownloadUpdate { /// Returns a new [DownloadUpdate] instance. DownloadUpdate({ this.archiveSize, + this.includeEmbeddedVideos, }); /// Minimum value: 1 @@ -25,17 +26,27 @@ class DownloadUpdate { /// int? archiveSize; + /// + /// Please note: This property should have been non-nullable! Since the specification file + /// does not include a default value (using the "default:" property), however, the generated + /// source code must fall back to having a nullable type. + /// Consider adding a "default:" property in the specification file to hide this note. + /// + bool? includeEmbeddedVideos; + @override bool operator ==(Object other) => identical(this, other) || other is DownloadUpdate && - other.archiveSize == archiveSize; + other.archiveSize == archiveSize && + other.includeEmbeddedVideos == includeEmbeddedVideos; @override int get hashCode => // ignore: unnecessary_parenthesis - (archiveSize == null ? 0 : archiveSize!.hashCode); + (archiveSize == null ? 0 : archiveSize!.hashCode) + + (includeEmbeddedVideos == null ? 0 : includeEmbeddedVideos!.hashCode); @override - String toString() => 'DownloadUpdate[archiveSize=$archiveSize]'; + String toString() => 'DownloadUpdate[archiveSize=$archiveSize, includeEmbeddedVideos=$includeEmbeddedVideos]'; Map toJson() { final json = {}; @@ -44,6 +55,11 @@ class DownloadUpdate { } else { // json[r'archiveSize'] = null; } + if (this.includeEmbeddedVideos != null) { + json[r'includeEmbeddedVideos'] = this.includeEmbeddedVideos; + } else { + // json[r'includeEmbeddedVideos'] = null; + } return json; } @@ -56,6 +72,7 @@ class DownloadUpdate { return DownloadUpdate( archiveSize: mapValueOfType(json, r'archiveSize'), + includeEmbeddedVideos: mapValueOfType(json, r'includeEmbeddedVideos'), ); } return null; diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 91e32d1e05af35..08d282b0bf243b 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -8497,10 +8497,15 @@ "properties": { "archiveSize": { "type": "integer" + }, + "includeEmbeddedVideos": { + "default": false, + "type": "boolean" } }, "required": [ - "archiveSize" + "archiveSize", + "includeEmbeddedVideos" ], "type": "object" }, @@ -8527,6 +8532,9 @@ "archiveSize": { "minimum": 1, "type": "integer" + }, + "includeEmbeddedVideos": { + "type": "boolean" } }, "type": "object" diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index 9d97f4bcce0bdd..c74d2e0a0f5bc0 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -86,6 +86,7 @@ export type AvatarResponse = { }; export type DownloadResponse = { archiveSize: number; + includeEmbeddedVideos: boolean; }; export type EmailNotificationsResponse = { albumInvite: boolean; @@ -115,6 +116,7 @@ export type AvatarUpdate = { }; export type DownloadUpdate = { archiveSize?: number; + includeEmbeddedVideos?: boolean; }; export type EmailNotificationsUpdate = { albumInvite?: boolean; diff --git a/server/src/dtos/user-preferences.dto.ts b/server/src/dtos/user-preferences.dto.ts index 3305e1cce16257..6eb3b6f480a6db 100644 --- a/server/src/dtos/user-preferences.dto.ts +++ b/server/src/dtos/user-preferences.dto.ts @@ -32,12 +32,15 @@ class EmailNotificationsUpdate { albumUpdate?: boolean; } -class DownloadUpdate { +class DownloadUpdate implements Partial { @Optional() @IsInt() @IsPositive() @ApiProperty({ type: 'integer' }) archiveSize?: number; + + @ValidateBoolean({ optional: true }) + includeEmbeddedVideos?: boolean; } class PurchaseUpdate { @@ -103,6 +106,8 @@ class EmailNotificationsResponse { class DownloadResponse { @ApiProperty({ type: 'integer' }) archiveSize!: number; + + includeEmbeddedVideos: boolean = false; } class PurchaseResponse { diff --git a/server/src/entities/user-metadata.entity.ts b/server/src/entities/user-metadata.entity.ts index 73eb9e04aabad3..dcf75736f0ea57 100644 --- a/server/src/entities/user-metadata.entity.ts +++ b/server/src/entities/user-metadata.entity.ts @@ -47,6 +47,7 @@ export interface UserPreferences { }; download: { archiveSize: number; + includeEmbeddedVideos: boolean; }; purchase: { showSupportBadge: boolean; @@ -77,6 +78,7 @@ export const getDefaultPreferences = (user: { email: string }): UserPreferences }, download: { archiveSize: HumanReadableSize.GiB * 4, + includeEmbeddedVideos: false, }, purchase: { showSupportBadge: true, diff --git a/server/src/services/download.service.spec.ts b/server/src/services/download.service.spec.ts index 2d3c11a6f15da5..14fa7bab48f48a 100644 --- a/server/src/services/download.service.spec.ts +++ b/server/src/services/download.service.spec.ts @@ -226,5 +226,31 @@ describe(DownloadService.name, () => { ], }); }); + + it('should skip the video portion of an android live photo by default', async () => { + const assetIds = [assetStub.livePhotoStillAsset.id]; + const assets = [ + assetStub.livePhotoStillAsset, + { ...assetStub.livePhotoMotionAsset, originalPath: 'upload/encoded-video/uuid-MP.mp4' }, + ]; + + accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(assetIds)); + assetMock.getByIds.mockImplementation( + (ids) => + Promise.resolve( + ids.map((id) => assets.find((asset) => asset.id === id)).filter((asset) => !!asset), + ) as Promise, + ); + + await expect(sut.getDownloadInfo(authStub.admin, { assetIds })).resolves.toEqual({ + totalSize: 25_000, + archives: [ + { + assetIds: [assetStub.livePhotoStillAsset.id], + size: 25_000, + }, + ], + }); + }); }); }); diff --git a/server/src/services/download.service.ts b/server/src/services/download.service.ts index 11e4de83d94e2c..6ace8e111a7690 100644 --- a/server/src/services/download.service.ts +++ b/server/src/services/download.service.ts @@ -1,6 +1,7 @@ import { BadRequestException, Inject, Injectable } from '@nestjs/common'; import { parse } from 'node:path'; import { AccessCore, Permission } from 'src/cores/access.core'; +import { StorageCore } from 'src/cores/storage.core'; import { AssetIdsDto } from 'src/dtos/asset.dto'; import { AuthDto } from 'src/dtos/auth.dto'; import { DownloadArchiveInfo, DownloadInfoDto, DownloadResponseDto } from 'src/dtos/download.dto'; @@ -11,6 +12,7 @@ import { ILoggerRepository } from 'src/interfaces/logger.interface'; import { ImmichReadStream, IStorageRepository } from 'src/interfaces/storage.interface'; import { HumanReadableSize } from 'src/utils/bytes'; import { usePagination } from 'src/utils/pagination'; +import { getPreferences } from 'src/utils/preferences'; @Injectable() export class DownloadService { @@ -31,12 +33,22 @@ export class DownloadService { const archives: DownloadArchiveInfo[] = []; let archive: DownloadArchiveInfo = { size: 0, assetIds: [] }; + const preferences = getPreferences(auth.user); + const assetPagination = await this.getDownloadAssets(auth, dto); for await (const assets of assetPagination) { // motion part of live photos - const motionIds = assets.map((asset) => asset.livePhotoVideoId).filter((id): id is string => !!id); + const motionIds = assets.map((asset) => asset.livePhotoVideoId).filter((id): id is string => !!id); if (motionIds.length > 0) { - assets.push(...(await this.assetRepository.getByIds(motionIds, { exifInfo: true }))); + const motionAssets = await this.assetRepository.getByIds(motionIds, { exifInfo: true }); + for (const motionAsset of motionAssets) { + if ( + !StorageCore.isAndroidMotionPath(motionAsset.originalPath) || + preferences.download.includeEmbeddedVideos + ) { + assets.push(motionAsset); + } + } } for (const asset of assets) { diff --git a/web/src/lib/components/user-settings-page/download-settings.svelte b/web/src/lib/components/user-settings-page/download-settings.svelte index f103f348fc201a..f5b94ebee8f2be 100644 --- a/web/src/lib/components/user-settings-page/download-settings.svelte +++ b/web/src/lib/components/user-settings-page/download-settings.svelte @@ -14,13 +14,21 @@ SettingInputFieldType, } from '$lib/components/shared-components/settings/setting-input-field.svelte'; import { ByteUnit, convertFromBytes, convertToBytes } from '$lib/utils/byte-units'; + import SettingSwitch from '$lib/components/shared-components/settings/setting-switch.svelte'; let archiveSize = convertFromBytes($preferences?.download?.archiveSize || 4, ByteUnit.GiB); + let includeEmbeddedVideos = $preferences?.download?.includeEmbeddedVideos || false; const handleSave = async () => { try { - const dto = { download: { archiveSize: Math.floor(convertToBytes(archiveSize, ByteUnit.GiB)) } }; - const newPreferences = await updateMyPreferences({ userPreferencesUpdateDto: dto }); + const newPreferences = await updateMyPreferences({ + userPreferencesUpdateDto: { + download: { + archiveSize: Math.floor(convertToBytes(archiveSize, ByteUnit.GiB)), + includeEmbeddedVideos, + }, + }, + }); $preferences = newPreferences; notificationController.show({ message: $t('saved_settings'), type: NotificationType.Info }); @@ -34,14 +42,17 @@
-
- -
+ +
diff --git a/web/src/lib/i18n/en.json b/web/src/lib/i18n/en.json index f424e60a66be57..2af6c952a0e57b 100644 --- a/web/src/lib/i18n/en.json +++ b/web/src/lib/i18n/en.json @@ -367,7 +367,7 @@ "appears_in": "Appears in", "archive": "Archive", "archive_or_unarchive_photo": "Archive or unarchive photo", - "archive_size": "Archive Size", + "archive_size": "Archive size", "archive_size_description": "Configure the archive size for downloads (in GiB)", "archived_count": "{count, plural, other {Archived #}}", "are_these_the_same_person": "Are these the same person?", @@ -510,6 +510,8 @@ "do_not_show_again": "Do not show this message again", "done": "Done", "download": "Download", + "download_include_embedded_motion_videos": "Embedded videos", + "download_include_embedded_motion_videos_description": "Include videos embedded in motion photos as a separate file", "download_settings": "Download", "download_settings_description": "Manage settings related to asset download", "downloading": "Downloading", diff --git a/web/src/lib/utils/asset-utils.ts b/web/src/lib/utils/asset-utils.ts index a23c369009c08a..74a695770e8487 100644 --- a/web/src/lib/utils/asset-utils.ts +++ b/web/src/lib/utils/asset-utils.ts @@ -172,13 +172,19 @@ export const downloadFile = async (asset: AssetResponseDto) => { }, ]; + const isAndroidMotionVideo = (asset: AssetResponseDto) => { + return asset.originalPath.includes('encoded-video'); + }; + if (asset.livePhotoVideoId) { const motionAsset = await getAssetInfo({ id: asset.livePhotoVideoId, key: getKey() }); - assets.push({ - filename: motionAsset.originalFileName, - id: asset.livePhotoVideoId, - size: motionAsset.exifInfo?.fileSizeInByte || 0, - }); + if (!isAndroidMotionVideo(motionAsset) || get(preferences).download.includeEmbeddedVideos) { + assets.push({ + filename: motionAsset.originalFileName, + id: asset.livePhotoVideoId, + size: motionAsset.exifInfo?.fileSizeInByte || 0, + }); + } } for (const { filename, id, size } of assets) {