Skip to content

fix(server): full-size images not migrated or deleted correctly #17308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions server/src/cores/storage.core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { MoveRepository } from 'src/repositories/move.repository';
import { PersonRepository } from 'src/repositories/person.repository';
import { StorageRepository } from 'src/repositories/storage.repository';
import { SystemMetadataRepository } from 'src/repositories/system-metadata.repository';
import { getAssetFiles } from 'src/utils/asset.util';
import { getAssetFile } from 'src/utils/asset.util';
import { getConfig } from 'src/utils/config';

export interface MoveRequest {
Expand Down Expand Up @@ -117,8 +117,7 @@ export class StorageCore {

async moveAssetImage(asset: AssetEntity, pathType: GeneratedImageType, format: ImageFormat) {
const { id: entityId, files } = asset;
const { thumbnailFile, previewFile } = getAssetFiles(files);
const oldFile = pathType === AssetPathType.PREVIEW ? previewFile : thumbnailFile;
const oldFile = getAssetFile(files, pathType);
return this.moveFile({
entityId,
pathType,
Expand Down
4 changes: 2 additions & 2 deletions server/src/services/asset-media.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ describe(AssetMediaService.name, () => {
sut.viewThumbnail(authStub.admin, assetStub.image.id, { size: AssetMediaSize.PREVIEW }),
).resolves.toEqual(
new ImmichFileResponse({
path: assetStub.image.files[0].path,
path: '/uploads/user-id/thumbs/path.jpg',
cacheControl: CacheControl.PRIVATE_WITH_CACHE,
contentType: 'image/jpeg',
fileName: 'asset-id_preview.jpg',
Expand All @@ -599,7 +599,7 @@ describe(AssetMediaService.name, () => {
sut.viewThumbnail(authStub.admin, assetStub.image.id, { size: AssetMediaSize.THUMBNAIL }),
).resolves.toEqual(
new ImmichFileResponse({
path: assetStub.image.files[1].path,
path: '/uploads/user-id/webp/path.ext',
cacheControl: CacheControl.PRIVATE_WITH_CACHE,
contentType: 'application/octet-stream',
fileName: 'asset-id_thumbnail.ext',
Expand Down
19 changes: 17 additions & 2 deletions server/src/services/asset.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,7 @@ describe(AssetService.name, () => {
files: [
'/uploads/user-id/webp/path.ext',
'/uploads/user-id/thumbs/path.jpg',
'/uploads/user-id/fullsize/path.webp',
assetWithFace.encodedVideoPath,
assetWithFace.sidecarPath,
assetWithFace.originalPath,
Expand Down Expand Up @@ -637,7 +638,14 @@ describe(AssetService.name, () => {
{
name: JobName.DELETE_FILES,
data: {
files: [undefined, undefined, undefined, undefined, 'fake_path/asset_1.jpeg'],
files: [
'/uploads/user-id/webp/path.ext',
'/uploads/user-id/thumbs/path.jpg',
'/uploads/user-id/fullsize/path.webp',
undefined,
undefined,
'fake_path/asset_1.jpeg',
],
},
},
],
Expand All @@ -658,7 +666,14 @@ describe(AssetService.name, () => {
{
name: JobName.DELETE_FILES,
data: {
files: [undefined, undefined, undefined, undefined, 'fake_path/asset_1.jpeg'],
files: [
'/uploads/user-id/webp/path.ext',
'/uploads/user-id/thumbs/path.jpg',
'/uploads/user-id/fullsize/path.webp',
undefined,
undefined,
'fake_path/asset_1.jpeg',
],
},
},
],
Expand Down
4 changes: 2 additions & 2 deletions server/src/services/asset.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,8 @@ export class AssetService extends BaseService {
}
}

const { thumbnailFile, previewFile } = getAssetFiles(asset.files);
const files = [thumbnailFile?.path, previewFile?.path, asset.encodedVideoPath];
const { fullsizeFile, previewFile, thumbnailFile } = getAssetFiles(asset.files);
const files = [thumbnailFile?.path, previewFile?.path, fullsizeFile?.path, asset.encodedVideoPath];

if (deleteOnDisk) {
files.push(asset.sidecarPath, asset.originalPath);
Expand Down
10 changes: 8 additions & 2 deletions server/src/services/audit.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,14 @@ export class AuditService extends BaseService {
for await (const assets of pagination) {
assetCount += assets.length;
for (const { id, files, originalPath, encodedVideoPath, isExternal, checksum } of assets) {
const { previewFile, thumbnailFile } = getAssetFiles(files);
for (const file of [originalPath, previewFile?.path, encodedVideoPath, thumbnailFile?.path]) {
const { fullsizeFile, previewFile, thumbnailFile } = getAssetFiles(files);
for (const file of [
originalPath,
fullsizeFile?.path,
previewFile?.path,
encodedVideoPath,
thumbnailFile?.path,
]) {
track(file);
}

Expand Down
6 changes: 3 additions & 3 deletions server/src/services/duplicate.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ import { mapAsset } from 'src/dtos/asset-response.dto';
import { AuthDto } from 'src/dtos/auth.dto';
import { DuplicateResponseDto } from 'src/dtos/duplicate.dto';
import { AssetEntity } from 'src/entities/asset.entity';
import { JobName, JobStatus, QueueName } from 'src/enum';
import { AssetFileType, JobName, JobStatus, QueueName } from 'src/enum';
import { WithoutProperty } from 'src/repositories/asset.repository';
import { AssetDuplicateResult } from 'src/repositories/search.repository';
import { BaseService } from 'src/services/base.service';
import { JobOf } from 'src/types';
import { getAssetFiles } from 'src/utils/asset.util';
import { getAssetFile } from 'src/utils/asset.util';
import { isDuplicateDetectionEnabled } from 'src/utils/misc';
import { usePagination } from 'src/utils/pagination';

Expand Down Expand Up @@ -69,7 +69,7 @@ export class DuplicateService extends BaseService {
return JobStatus.SKIPPED;
}

const { previewFile } = getAssetFiles(asset.files);
const previewFile = getAssetFile(asset.files, AssetFileType.PREVIEW);
if (!previewFile) {
this.logger.warn(`Asset ${id} is missing preview image`);
return JobStatus.FAILED;
Expand Down
18 changes: 18 additions & 0 deletions server/src/services/media.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,24 @@ describe(MediaService.name, () => {
});

await expect(sut.handleAssetMigration({ id: assetStub.image.id })).resolves.toBe(JobStatus.SUCCESS);
expect(mocks.move.create).toHaveBeenCalledWith({
entityId: assetStub.image.id,
pathType: AssetPathType.FULLSIZE,
oldPath: '/uploads/user-id/fullsize/path.webp',
newPath: 'upload/thumbs/user-id/as/se/asset-id-fullsize.jpeg',
});
expect(mocks.move.create).toHaveBeenCalledWith({
entityId: assetStub.image.id,
pathType: AssetPathType.PREVIEW,
oldPath: '/uploads/user-id/thumbs/path.jpg',
newPath: 'upload/thumbs/user-id/as/se/asset-id-preview.jpeg',
});
expect(mocks.move.create).toHaveBeenCalledWith({
entityId: assetStub.image.id,
pathType: AssetPathType.THUMBNAIL,
oldPath: '/uploads/user-id/webp/path.ext',
newPath: 'upload/thumbs/user-id/as/se/asset-id-thumbnail.webp',
});
expect(mocks.move.create).toHaveBeenCalledTimes(3);
});
});
Expand Down
2 changes: 1 addition & 1 deletion server/src/services/media.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export class MediaService extends BaseService {
return JobStatus.FAILED;
}

await this.storageCore.moveAssetImage(asset, AssetPathType.FULLSIZE, ImageFormat.JPEG);
await this.storageCore.moveAssetImage(asset, AssetPathType.FULLSIZE, image.fullsize.format);
await this.storageCore.moveAssetImage(asset, AssetPathType.PREVIEW, image.preview.format);
await this.storageCore.moveAssetImage(asset, AssetPathType.THUMBNAIL, image.thumbnail.format);
await this.storageCore.moveAssetVideo(asset);
Expand Down
10 changes: 7 additions & 3 deletions server/src/services/notification.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { BadRequestException, Injectable } from '@nestjs/common';
import { OnEvent, OnJob } from 'src/decorators';
import { SystemConfigSmtpDto } from 'src/dtos/system-config.dto';
import { AlbumEntity } from 'src/entities/album.entity';
import { JobName, JobStatus, QueueName } from 'src/enum';
import { AssetFileType, JobName, JobStatus, QueueName } from 'src/enum';
import { ArgOf } from 'src/repositories/event.repository';
import { EmailTemplate } from 'src/repositories/notification.repository';
import { BaseService } from 'src/services/base.service';
import { EmailImageAttachment, IEntityJob, INotifyAlbumUpdateJob, JobItem, JobOf } from 'src/types';
import { getAssetFiles } from 'src/utils/asset.util';
import { getAssetFile } from 'src/utils/asset.util';
import { getFilenameExtension } from 'src/utils/file';
import { getExternalDomain } from 'src/utils/misc';
import { isEqualObject } from 'src/utils/object';
Expand Down Expand Up @@ -398,7 +398,11 @@ export class NotificationService extends BaseService {
}

const albumThumbnail = await this.assetRepository.getById(album.albumThumbnailAssetId, { files: true });
const { thumbnailFile } = getAssetFiles(albumThumbnail?.files);
if (!albumThumbnail) {
return;
}

const thumbnailFile = getAssetFile(albumThumbnail.files, AssetFileType.THUMBNAIL);
if (!thumbnailFile) {
return;
}
Expand Down
7 changes: 4 additions & 3 deletions server/src/services/person.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { AssetEntity } from 'src/entities/asset.entity';
import { FaceSearchEntity } from 'src/entities/face-search.entity';
import { PersonEntity } from 'src/entities/person.entity';
import {
AssetFileType,
AssetType,
CacheControl,
ImageFormat,
Expand All @@ -42,7 +43,7 @@ import { BoundingBox } from 'src/repositories/machine-learning.repository';
import { UpdateFacesData } from 'src/repositories/person.repository';
import { BaseService } from 'src/services/base.service';
import { CropOptions, ImageDimensions, InputDimensions, JobItem, JobOf } from 'src/types';
import { getAssetFiles } from 'src/utils/asset.util';
import { getAssetFile } from 'src/utils/asset.util';
import { ImmichFileResponse } from 'src/utils/file';
import { mimeTypes } from 'src/utils/mime-types';
import { isFaceImportEnabled, isFacialRecognitionEnabled } from 'src/utils/misc';
Expand Down Expand Up @@ -300,7 +301,7 @@ export class PersonService extends BaseService {

const relations = { exifInfo: true, faces: { person: false, withDeleted: true }, files: true };
const [asset] = await this.assetRepository.getByIds([id], relations);
const { previewFile } = getAssetFiles(asset.files);
const previewFile = getAssetFile(asset.files, AssetFileType.PREVIEW);
if (!asset || !previewFile) {
return JobStatus.FAILED;
}
Expand Down Expand Up @@ -674,7 +675,7 @@ export class PersonService extends BaseService {
throw new Error(`Asset ${asset.id} dimensions are unknown`);
}

const { previewFile } = getAssetFiles(asset.files);
const previewFile = getAssetFile(asset.files, AssetFileType.PREVIEW);
if (!previewFile) {
throw new Error(`Asset ${asset.id} has no preview path`);
}
Expand Down
6 changes: 3 additions & 3 deletions server/src/services/smart-info.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { Injectable } from '@nestjs/common';
import { SystemConfig } from 'src/config';
import { JOBS_ASSET_PAGINATION_SIZE } from 'src/constants';
import { OnEvent, OnJob } from 'src/decorators';
import { DatabaseLock, ImmichWorker, JobName, JobStatus, QueueName } from 'src/enum';
import { AssetFileType, DatabaseLock, ImmichWorker, JobName, JobStatus, QueueName } from 'src/enum';
import { WithoutProperty } from 'src/repositories/asset.repository';
import { ArgOf } from 'src/repositories/event.repository';
import { BaseService } from 'src/services/base.service';
import { JobOf } from 'src/types';
import { getAssetFiles } from 'src/utils/asset.util';
import { getAssetFile } from 'src/utils/asset.util';
import { getCLIPModelInfo, isSmartSearchEnabled } from 'src/utils/misc';
import { usePagination } from 'src/utils/pagination';

Expand Down Expand Up @@ -116,7 +116,7 @@ export class SmartInfoService extends BaseService {
return JobStatus.SKIPPED;
}

const { previewFile } = getAssetFiles(asset.files);
const previewFile = getAssetFile(asset.files, AssetFileType.PREVIEW);
if (!previewFile) {
return JobStatus.FAILED;
}
Expand Down
12 changes: 6 additions & 6 deletions server/src/utils/asset.util.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BadRequestException } from '@nestjs/common';
import { StorageCore } from 'src/cores/storage.core';
import { GeneratedImageType, StorageCore } from 'src/cores/storage.core';
import { BulkIdErrorReason, BulkIdResponseDto } from 'src/dtos/asset-ids.response.dto';
import { UploadFieldName } from 'src/dtos/asset-media.dto';
import { AuthDto } from 'src/dtos/auth.dto';
Expand All @@ -13,14 +13,14 @@ import { PartnerRepository } from 'src/repositories/partner.repository';
import { IBulkAsset, ImmichFile, UploadFile } from 'src/types';
import { checkAccess } from 'src/utils/access';

const getFileByType = (files: AssetFileEntity[] | undefined, type: AssetFileType) => {
export const getAssetFile = (files: AssetFileEntity[], type: AssetFileType | GeneratedImageType) => {
return (files || []).find((file) => file.type === type);
};

export const getAssetFiles = (files?: AssetFileEntity[]) => ({
fullsizeFile: getFileByType(files, AssetFileType.FULLSIZE),
previewFile: getFileByType(files, AssetFileType.PREVIEW),
thumbnailFile: getFileByType(files, AssetFileType.THUMBNAIL),
export const getAssetFiles = (files: AssetFileEntity[]) => ({
fullsizeFile: getAssetFile(files, AssetFileType.FULLSIZE),
previewFile: getAssetFile(files, AssetFileType.PREVIEW),
thumbnailFile: getAssetFile(files, AssetFileType.THUMBNAIL),
});

export const addAssets = async (
Expand Down
12 changes: 11 additions & 1 deletion server/test/fixtures/asset.stub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,16 @@ const thumbnailFile: AssetFileEntity = {
updatedAt: new Date('2023-02-23T05:06:29.716Z'),
};

const files: AssetFileEntity[] = [previewFile, thumbnailFile];
const fullsizeFile: AssetFileEntity = {
id: 'file-3',
assetId: 'asset-id',
type: AssetFileType.FULLSIZE,
path: '/uploads/user-id/fullsize/path.webp',
createdAt: new Date('2023-02-23T05:06:29.716Z'),
updatedAt: new Date('2023-02-23T05:06:29.716Z'),
};

const files: AssetFileEntity[] = [fullsizeFile, previewFile, thumbnailFile];

export const stackStub = (stackId: string, assets: AssetEntity[]): StackEntity => {
return {
Expand Down Expand Up @@ -553,6 +562,7 @@ export const assetStub = {
fileSizeInByte: 25_000,
timeZone: `America/New_York`,
},
files,
} as AssetEntity),

livePhotoWithOriginalFileName: Object.freeze({
Expand Down