feat(server): do not automatically download android motion videos (#11774)

feat(server): do not automatically download embedded android motion videos
This commit is contained in:
Jason Rasmussen 2024-08-15 16:06:16 -04:00 committed by GitHub
parent ed6971222c
commit 32c05ea950
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 151 additions and 28 deletions

View File

@ -1,5 +1,3 @@
version: '3.8'
name: immich-e2e name: immich-e2e
services: services:

View File

@ -236,6 +236,32 @@ describe('/users', () => {
const after = await getMyPreferences({ headers: asBearerAuth(admin.accessToken) }); const after = await getMyPreferences({ headers: asBearerAuth(admin.accessToken) });
expect(after).toMatchObject({ download: { archiveSize: 1_234_567 } }); 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', () => { describe('GET /users/:id', () => {

View File

@ -14,25 +14,31 @@ class DownloadResponse {
/// Returns a new [DownloadResponse] instance. /// Returns a new [DownloadResponse] instance.
DownloadResponse({ DownloadResponse({
required this.archiveSize, required this.archiveSize,
this.includeEmbeddedVideos = false,
}); });
int archiveSize; int archiveSize;
bool includeEmbeddedVideos;
@override @override
bool operator ==(Object other) => identical(this, other) || other is DownloadResponse && bool operator ==(Object other) => identical(this, other) || other is DownloadResponse &&
other.archiveSize == archiveSize; other.archiveSize == archiveSize &&
other.includeEmbeddedVideos == includeEmbeddedVideos;
@override @override
int get hashCode => int get hashCode =>
// ignore: unnecessary_parenthesis // ignore: unnecessary_parenthesis
(archiveSize.hashCode); (archiveSize.hashCode) +
(includeEmbeddedVideos.hashCode);
@override @override
String toString() => 'DownloadResponse[archiveSize=$archiveSize]'; String toString() => 'DownloadResponse[archiveSize=$archiveSize, includeEmbeddedVideos=$includeEmbeddedVideos]';
Map<String, dynamic> toJson() { Map<String, dynamic> toJson() {
final json = <String, dynamic>{}; final json = <String, dynamic>{};
json[r'archiveSize'] = this.archiveSize; json[r'archiveSize'] = this.archiveSize;
json[r'includeEmbeddedVideos'] = this.includeEmbeddedVideos;
return json; return json;
} }
@ -45,6 +51,7 @@ class DownloadResponse {
return DownloadResponse( return DownloadResponse(
archiveSize: mapValueOfType<int>(json, r'archiveSize')!, archiveSize: mapValueOfType<int>(json, r'archiveSize')!,
includeEmbeddedVideos: mapValueOfType<bool>(json, r'includeEmbeddedVideos')!,
); );
} }
return null; return null;
@ -93,6 +100,7 @@ class DownloadResponse {
/// The list of required keys that must be present in a JSON. /// The list of required keys that must be present in a JSON.
static const requiredKeys = <String>{ static const requiredKeys = <String>{
'archiveSize', 'archiveSize',
'includeEmbeddedVideos',
}; };
} }

View File

@ -14,6 +14,7 @@ class DownloadUpdate {
/// Returns a new [DownloadUpdate] instance. /// Returns a new [DownloadUpdate] instance.
DownloadUpdate({ DownloadUpdate({
this.archiveSize, this.archiveSize,
this.includeEmbeddedVideos,
}); });
/// Minimum value: 1 /// Minimum value: 1
@ -25,17 +26,27 @@ class DownloadUpdate {
/// ///
int? archiveSize; 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 @override
bool operator ==(Object other) => identical(this, other) || other is DownloadUpdate && bool operator ==(Object other) => identical(this, other) || other is DownloadUpdate &&
other.archiveSize == archiveSize; other.archiveSize == archiveSize &&
other.includeEmbeddedVideos == includeEmbeddedVideos;
@override @override
int get hashCode => int get hashCode =>
// ignore: unnecessary_parenthesis // ignore: unnecessary_parenthesis
(archiveSize == null ? 0 : archiveSize!.hashCode); (archiveSize == null ? 0 : archiveSize!.hashCode) +
(includeEmbeddedVideos == null ? 0 : includeEmbeddedVideos!.hashCode);
@override @override
String toString() => 'DownloadUpdate[archiveSize=$archiveSize]'; String toString() => 'DownloadUpdate[archiveSize=$archiveSize, includeEmbeddedVideos=$includeEmbeddedVideos]';
Map<String, dynamic> toJson() { Map<String, dynamic> toJson() {
final json = <String, dynamic>{}; final json = <String, dynamic>{};
@ -44,6 +55,11 @@ class DownloadUpdate {
} else { } else {
// json[r'archiveSize'] = null; // json[r'archiveSize'] = null;
} }
if (this.includeEmbeddedVideos != null) {
json[r'includeEmbeddedVideos'] = this.includeEmbeddedVideos;
} else {
// json[r'includeEmbeddedVideos'] = null;
}
return json; return json;
} }
@ -56,6 +72,7 @@ class DownloadUpdate {
return DownloadUpdate( return DownloadUpdate(
archiveSize: mapValueOfType<int>(json, r'archiveSize'), archiveSize: mapValueOfType<int>(json, r'archiveSize'),
includeEmbeddedVideos: mapValueOfType<bool>(json, r'includeEmbeddedVideos'),
); );
} }
return null; return null;

View File

@ -8497,10 +8497,15 @@
"properties": { "properties": {
"archiveSize": { "archiveSize": {
"type": "integer" "type": "integer"
},
"includeEmbeddedVideos": {
"default": false,
"type": "boolean"
} }
}, },
"required": [ "required": [
"archiveSize" "archiveSize",
"includeEmbeddedVideos"
], ],
"type": "object" "type": "object"
}, },
@ -8527,6 +8532,9 @@
"archiveSize": { "archiveSize": {
"minimum": 1, "minimum": 1,
"type": "integer" "type": "integer"
},
"includeEmbeddedVideos": {
"type": "boolean"
} }
}, },
"type": "object" "type": "object"

View File

@ -86,6 +86,7 @@ export type AvatarResponse = {
}; };
export type DownloadResponse = { export type DownloadResponse = {
archiveSize: number; archiveSize: number;
includeEmbeddedVideos: boolean;
}; };
export type EmailNotificationsResponse = { export type EmailNotificationsResponse = {
albumInvite: boolean; albumInvite: boolean;
@ -115,6 +116,7 @@ export type AvatarUpdate = {
}; };
export type DownloadUpdate = { export type DownloadUpdate = {
archiveSize?: number; archiveSize?: number;
includeEmbeddedVideos?: boolean;
}; };
export type EmailNotificationsUpdate = { export type EmailNotificationsUpdate = {
albumInvite?: boolean; albumInvite?: boolean;

View File

@ -33,12 +33,15 @@ class EmailNotificationsUpdate {
albumUpdate?: boolean; albumUpdate?: boolean;
} }
class DownloadUpdate { class DownloadUpdate implements Partial<DownloadResponse> {
@Optional() @Optional()
@IsInt() @IsInt()
@IsPositive() @IsPositive()
@ApiProperty({ type: 'integer' }) @ApiProperty({ type: 'integer' })
archiveSize?: number; archiveSize?: number;
@ValidateBoolean({ optional: true })
includeEmbeddedVideos?: boolean;
} }
class PurchaseUpdate { class PurchaseUpdate {
@ -104,6 +107,8 @@ class EmailNotificationsResponse {
class DownloadResponse { class DownloadResponse {
@ApiProperty({ type: 'integer' }) @ApiProperty({ type: 'integer' })
archiveSize!: number; archiveSize!: number;
includeEmbeddedVideos: boolean = false;
} }
class PurchaseResponse { class PurchaseResponse {

View File

@ -35,6 +35,7 @@ export interface UserPreferences {
}; };
download: { download: {
archiveSize: number; archiveSize: number;
includeEmbeddedVideos: boolean;
}; };
purchase: { purchase: {
showSupportBadge: boolean; showSupportBadge: boolean;
@ -65,6 +66,7 @@ export const getDefaultPreferences = (user: { email: string }): UserPreferences
}, },
download: { download: {
archiveSize: HumanReadableSize.GiB * 4, archiveSize: HumanReadableSize.GiB * 4,
includeEmbeddedVideos: false,
}, },
purchase: { purchase: {
showSupportBadge: true, showSupportBadge: true,

View File

@ -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<AssetEntity[]>,
);
await expect(sut.getDownloadInfo(authStub.admin, { assetIds })).resolves.toEqual({
totalSize: 25_000,
archives: [
{
assetIds: [assetStub.livePhotoStillAsset.id],
size: 25_000,
},
],
});
});
}); });
}); });

View File

@ -1,6 +1,7 @@
import { BadRequestException, Inject, Injectable } from '@nestjs/common'; import { BadRequestException, Inject, Injectable } from '@nestjs/common';
import { parse } from 'node:path'; import { parse } from 'node:path';
import { AccessCore } from 'src/cores/access.core'; import { AccessCore } from 'src/cores/access.core';
import { StorageCore } from 'src/cores/storage.core';
import { AssetIdsDto } from 'src/dtos/asset.dto'; import { AssetIdsDto } from 'src/dtos/asset.dto';
import { AuthDto } from 'src/dtos/auth.dto'; import { AuthDto } from 'src/dtos/auth.dto';
import { DownloadArchiveInfo, DownloadInfoDto, DownloadResponseDto } from 'src/dtos/download.dto'; import { DownloadArchiveInfo, DownloadInfoDto, DownloadResponseDto } from 'src/dtos/download.dto';
@ -12,6 +13,7 @@ import { ILoggerRepository } from 'src/interfaces/logger.interface';
import { ImmichReadStream, IStorageRepository } from 'src/interfaces/storage.interface'; import { ImmichReadStream, IStorageRepository } from 'src/interfaces/storage.interface';
import { HumanReadableSize } from 'src/utils/bytes'; import { HumanReadableSize } from 'src/utils/bytes';
import { usePagination } from 'src/utils/pagination'; import { usePagination } from 'src/utils/pagination';
import { getPreferences } from 'src/utils/preferences';
@Injectable() @Injectable()
export class DownloadService { export class DownloadService {
@ -32,12 +34,22 @@ export class DownloadService {
const archives: DownloadArchiveInfo[] = []; const archives: DownloadArchiveInfo[] = [];
let archive: DownloadArchiveInfo = { size: 0, assetIds: [] }; let archive: DownloadArchiveInfo = { size: 0, assetIds: [] };
const preferences = getPreferences(auth.user);
const assetPagination = await this.getDownloadAssets(auth, dto); const assetPagination = await this.getDownloadAssets(auth, dto);
for await (const assets of assetPagination) { for await (const assets of assetPagination) {
// motion part of live photos // motion part of live photos
const motionIds = assets.map((asset) => asset.livePhotoVideoId).filter<string>((id): id is string => !!id); const motionIds = assets.map((asset) => asset.livePhotoVideoId).filter((id): id is string => !!id);
if (motionIds.length > 0) { 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) { for (const asset of assets) {

View File

@ -14,13 +14,21 @@
SettingInputFieldType, SettingInputFieldType,
} from '$lib/components/shared-components/settings/setting-input-field.svelte'; } from '$lib/components/shared-components/settings/setting-input-field.svelte';
import { ByteUnit, convertFromBytes, convertToBytes } from '$lib/utils/byte-units'; 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 archiveSize = convertFromBytes($preferences?.download?.archiveSize || 4, ByteUnit.GiB);
let includeEmbeddedVideos = $preferences?.download?.includeEmbeddedVideos || false;
const handleSave = async () => { const handleSave = async () => {
try { try {
const dto = { download: { archiveSize: Math.floor(convertToBytes(archiveSize, ByteUnit.GiB)) } }; const newPreferences = await updateMyPreferences({
const newPreferences = await updateMyPreferences({ userPreferencesUpdateDto: dto }); userPreferencesUpdateDto: {
download: {
archiveSize: Math.floor(convertToBytes(archiveSize, ByteUnit.GiB)),
includeEmbeddedVideos,
},
},
});
$preferences = newPreferences; $preferences = newPreferences;
notificationController.show({ message: $t('saved_settings'), type: NotificationType.Info }); notificationController.show({ message: $t('saved_settings'), type: NotificationType.Info });
@ -34,14 +42,17 @@
<div in:fade={{ duration: 500 }}> <div in:fade={{ duration: 500 }}>
<form autocomplete="off" on:submit|preventDefault> <form autocomplete="off" on:submit|preventDefault>
<div class="ml-4 mt-4 flex flex-col gap-4"> <div class="ml-4 mt-4 flex flex-col gap-4">
<div class="ml-4"> <SettingInputField
<SettingInputField inputType={SettingInputFieldType.NUMBER}
inputType={SettingInputFieldType.NUMBER} label={$t('archive_size')}
label={$t('archive_size')} desc={$t('archive_size_description')}
desc={$t('archive_size_description')} bind:value={archiveSize}
bind:value={archiveSize} />
/> <SettingSwitch
</div> title={$t('download_include_embedded_motion_videos')}
subtitle={$t('download_include_embedded_motion_videos_description')}
bind:checked={includeEmbeddedVideos}
></SettingSwitch>
<div class="flex justify-end"> <div class="flex justify-end">
<Button type="submit" size="sm" on:click={() => handleSave()}>{$t('save')}</Button> <Button type="submit" size="sm" on:click={() => handleSave()}>{$t('save')}</Button>
</div> </div>

View File

@ -368,7 +368,7 @@
"appears_in": "Appears in", "appears_in": "Appears in",
"archive": "Archive", "archive": "Archive",
"archive_or_unarchive_photo": "Archive or unarchive photo", "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)", "archive_size_description": "Configure the archive size for downloads (in GiB)",
"archived_count": "{count, plural, other {Archived #}}", "archived_count": "{count, plural, other {Archived #}}",
"are_these_the_same_person": "Are these the same person?", "are_these_the_same_person": "Are these the same person?",
@ -512,6 +512,8 @@
"do_not_show_again": "Do not show this message again", "do_not_show_again": "Do not show this message again",
"done": "Done", "done": "Done",
"download": "Download", "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": "Download",
"download_settings_description": "Manage settings related to asset download", "download_settings_description": "Manage settings related to asset download",
"downloading": "Downloading", "downloading": "Downloading",

View File

@ -172,13 +172,19 @@ export const downloadFile = async (asset: AssetResponseDto) => {
}, },
]; ];
const isAndroidMotionVideo = (asset: AssetResponseDto) => {
return asset.originalPath.includes('encoded-video');
};
if (asset.livePhotoVideoId) { if (asset.livePhotoVideoId) {
const motionAsset = await getAssetInfo({ id: asset.livePhotoVideoId, key: getKey() }); const motionAsset = await getAssetInfo({ id: asset.livePhotoVideoId, key: getKey() });
assets.push({ if (!isAndroidMotionVideo(motionAsset) || get(preferences).download.includeEmbeddedVideos) {
filename: motionAsset.originalFileName, assets.push({
id: asset.livePhotoVideoId, filename: motionAsset.originalFileName,
size: motionAsset.exifInfo?.fileSizeInByte || 0, id: asset.livePhotoVideoId,
}); size: motionAsset.exifInfo?.fileSizeInByte || 0,
});
}
} }
for (const { filename, id, size } of assets) { for (const { filename, id, size } of assets) {