KevinMidboe/immich
1
0
Fork 0
mirror of https://github.com/KevinMidboe/immich.git synced 2025-12-08 20:29:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(server): harden move file (#4361)

Browse Source 
Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>
This commit is contained in:
Daniel Dietzler
2023-10-11 04:14:44 +02:00
committed by GitHub
parent 332a8d80f2
commit 09bf1c9175
31 changed files with 564 additions and 190 deletions
Download Patch File Download Diff File Expand all files Collapse all files

67
server/src/domain/storage-template/storage-template.service.spec.ts
View File

@@ -1,13 +1,23 @@
import { AssetPathType } from '@app/infra/entities';
import {
assetStub,
newAssetRepositoryMock,
newMoveRepositoryMock,
newPersonRepositoryMock,
newStorageRepositoryMock,
newSystemConfigRepositoryMock,
newUserRepositoryMock,
userStub,
} from '@test';
import { when } from 'jest-when';
import { IAssetRepository, IStorageRepository, ISystemConfigRepository, IUserRepository } from '../repositories';
import {
IAssetRepository,
IMoveRepository,
IPersonRepository,
IStorageRepository,
ISystemConfigRepository,
IUserRepository,
} from '../repositories';
import { defaults } from '../system-config/system-config.core';
import { StorageTemplateService } from './storage-template.service';
@@ -15,6 +25,8 @@ describe(StorageTemplateService.name, () => {
let sut: StorageTemplateService;
let assetMock: jest.Mocked<IAssetRepository>;
let configMock: jest.Mocked<ISystemConfigRepository>;
let moveMock: jest.Mocked<IMoveRepository>;
let personMock: jest.Mocked<IPersonRepository>;
let storageMock: jest.Mocked<IStorageRepository>;
let userMock: jest.Mocked<IUserRepository>;
@@ -25,10 +37,12 @@ describe(StorageTemplateService.name, () => {
beforeEach(async () => {
assetMock = newAssetRepositoryMock();
configMock = newSystemConfigRepositoryMock();
moveMock = newMoveRepositoryMock();
personMock = newPersonRepositoryMock();
storageMock = newStorageRepositoryMock();
userMock = newUserRepositoryMock();
sut = new StorageTemplateService(assetMock, configMock, defaults, storageMock, userMock);
sut = new StorageTemplateService(assetMock, configMock, defaults, moveMock, personMock, storageMock, userMock);
});
describe('handleMigrationSingle', () => {
@@ -86,6 +100,13 @@ describe(StorageTemplateService.name, () => {
});
assetMock.save.mockResolvedValue(assetStub.image);
userMock.getList.mockResolvedValue([userStub.user1]);
moveMock.create.mockResolvedValue({
id: '123',
entityId: assetStub.image.id,
pathType: AssetPathType.ORIGINAL,
oldPath: assetStub.image.originalPath,
newPath: 'upload/library/user-id/2023/2023-02-23/asset-id+1.jpg',
});
when(storageMock.checkFileExists)
.calledWith('upload/library/user-id/2023/2023-02-23/asset-id.jpg')
@@ -153,6 +174,13 @@ describe(StorageTemplateService.name, () => {
});
assetMock.save.mockResolvedValue(assetStub.image);
userMock.getList.mockResolvedValue([userStub.user1]);
moveMock.create.mockResolvedValue({
id: '123',
entityId: assetStub.image.id,
pathType: AssetPathType.ORIGINAL,
oldPath: assetStub.image.originalPath,
newPath: 'upload/library/user-id/2023/2023-02-23/asset-id.jpg',
});
await sut.handleMigration();
@@ -174,6 +202,13 @@ describe(StorageTemplateService.name, () => {
});
assetMock.save.mockResolvedValue(assetStub.image);
userMock.getList.mockResolvedValue([userStub.storageLabel]);
moveMock.create.mockResolvedValue({
id: '123',
entityId: assetStub.image.id,
pathType: AssetPathType.ORIGINAL,
oldPath: assetStub.image.originalPath,
newPath: 'upload/library/user-id/2023/2023-02-23/asset-id.jpg',
});
await sut.handleMigration();
@@ -194,6 +229,13 @@ describe(StorageTemplateService.name, () => {
hasNextPage: false,
});
storageMock.moveFile.mockRejectedValue(new Error('Read only system'));
moveMock.create.mockResolvedValue({
id: 'move-123',
entityId: '123',
pathType: AssetPathType.ORIGINAL,
oldPath: assetStub.image.originalPath,
newPath: '',
});
userMock.getList.mockResolvedValue([userStub.user1]);
await sut.handleMigration();
@@ -206,27 +248,6 @@ describe(StorageTemplateService.name, () => {
expect(assetMock.save).not.toHaveBeenCalled();
});
it('should move the asset back if the database fails', async () => {
assetMock.getAll.mockResolvedValue({
items: [assetStub.image],
hasNextPage: false,
});
assetMock.save.mockRejectedValue('Connection Error!');
userMock.getList.mockResolvedValue([userStub.user1]);
await sut.handleMigration();
expect(assetMock.getAll).toHaveBeenCalled();
expect(assetMock.save).toHaveBeenCalledWith({
id: assetStub.image.id,
originalPath: 'upload/library/user-id/2023/2023-02-23/asset-id.jpg',
});
expect(storageMock.moveFile.mock.calls).toEqual([
['/original/path.jpg', 'upload/library/user-id/2023/2023-02-23/asset-id.jpg'],
['upload/library/user-id/2023/2023-02-23/asset-id.jpg', '/original/path.jpg'],
]);
});
it('should not move read-only asset', async () => {
assetMock.getAll.mockResolvedValue({
items: [

67
server/src/domain/storage-template/storage-template.service.ts
View File

@@ -1,4 +1,4 @@
import { AssetEntity, AssetType, SystemConfig } from '@app/infra/entities';
import { AssetEntity, AssetPathType, AssetType, SystemConfig } from '@app/infra/entities';
import { Inject, Injectable, Logger } from '@nestjs/common';
import handlebar from 'handlebars';
import * as luxon from 'luxon';
@@ -6,7 +6,14 @@ import path from 'node:path';
import sanitize from 'sanitize-filename';
import { getLivePhotoMotionFilename, usePagination } from '../domain.util';
import { IEntityJob, JOBS_ASSET_PAGINATION_SIZE } from '../job';
import { IAssetRepository, IStorageRepository, ISystemConfigRepository, IUserRepository } from '../repositories';
import {
IAssetRepository,
IMoveRepository,
IPersonRepository,
IStorageRepository,
ISystemConfigRepository,
IUserRepository,
} from '../repositories';
import { StorageCore, StorageFolder } from '../storage';
import {
INITIAL_SYSTEM_CONFIG,
@@ -36,6 +43,8 @@ export class StorageTemplateService {
@Inject(IAssetRepository) private assetRepository: IAssetRepository,
@Inject(ISystemConfigRepository) configRepository: ISystemConfigRepository,
@Inject(INITIAL_SYSTEM_CONFIG) config: SystemConfig,
@Inject(IMoveRepository) moveRepository: IMoveRepository,
@Inject(IPersonRepository) personRepository: IPersonRepository,
@Inject(IStorageRepository) private storageRepository: IStorageRepository,
@Inject(IUserRepository) private userRepository: IUserRepository,
) {
@@ -43,7 +52,7 @@ export class StorageTemplateService {
this.configCore = SystemConfigCore.create(configRepository);
this.configCore.addValidator((config) => this.validate(config));
this.configCore.config$.subscribe((config) => this.onConfig(config));
this.storageCore = new StorageCore(storageRepository);
this.storageCore = new StorageCore(storageRepository, assetRepository, moveRepository, personRepository);
}
async handleMigrationSingle({ id }: IEntityJob) {
@@ -90,51 +99,29 @@ export class StorageTemplateService {
}
async moveAsset(asset: AssetEntity, metadata: MoveAssetMetadata) {
if (asset.isReadOnly || asset.isExternal) {
if (asset.isReadOnly || asset.isExternal || this.storageCore.isAndroidMotionPath(asset.originalPath)) {
// External assets are not affected by storage template
// TODO: shouldn't this only apply to external assets?
return;
}
const destination = await this.getTemplatePath(asset, metadata);
if (asset.originalPath !== destination) {
const source = asset.originalPath;
const { id, sidecarPath, originalPath } = asset;
const oldPath = originalPath;
const newPath = await this.getTemplatePath(asset, metadata);
let sidecarMoved = false;
try {
await this.storageRepository.moveFile(asset.originalPath, destination);
let sidecarDestination;
try {
if (asset.sidecarPath) {
sidecarDestination = `${destination}.xmp`;
await this.storageRepository.moveFile(asset.sidecarPath, sidecarDestination);
sidecarMoved = true;
}
await this.assetRepository.save({ id: asset.id, originalPath: destination, sidecarPath: sidecarDestination });
asset.originalPath = destination;
asset.sidecarPath = sidecarDestination || null;
} catch (error: any) {
this.logger.warn(
`Unable to save new originalPath to database, undoing move for path ${asset.originalPath} - filename ${asset.originalFileName} - id ${asset.id}`,
error?.stack,
);
// Either sidecar move failed or the save failed. Either way, move media back
await this.storageRepository.moveFile(destination, source);
if (asset.sidecarPath && sidecarDestination && sidecarMoved) {
// If the sidecar was moved, that means the saved failed. So move both the sidecar and the
// media back into their original positions
await this.storageRepository.moveFile(sidecarDestination, asset.sidecarPath);
}
}
} catch (error: any) {
this.logger.error(`Problem applying storage template`, error?.stack, { id: asset.id, source, destination });
try {
await this.storageCore.moveFile({ entityId: id, pathType: AssetPathType.ORIGINAL, oldPath, newPath });
if (sidecarPath) {
await this.storageCore.moveFile({
entityId: id,
pathType: AssetPathType.SIDECAR,
oldPath: sidecarPath,
newPath: `${newPath}.xmp`,
});
}
} catch (error: any) {
this.logger.error(`Problem applying storage template`, error?.stack, { id: asset.id, oldPath, newPath });
}
return asset;
}
private async getTemplatePath(asset: AssetEntity, metadata: MoveAssetMetadata): Promise<string> {