Skip to content

Commit

Permalink
feat(server): clean up interrupted upload files (immich-app#14265)
Browse files Browse the repository at this point in the history
* feat(server): clean up interrupted upload files

* pr feedback

* remove console.log

* handle all errors

* remove return in callback function

* programming in bed is a bad idea
  • Loading branch information
alextran1502 authored and yosit committed Nov 21, 2024
1 parent 22bfebe commit 6dfd4f3
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 19 deletions.
25 changes: 7 additions & 18 deletions server/src/middleware/file-upload.interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { RouteKey } from 'src/enum';
import { ILoggerRepository } from 'src/interfaces/logger.interface';
import { AuthRequest } from 'src/middleware/auth.guard';
import { AssetMediaService, UploadFile } from 'src/services/asset-media.service';
import { asRequest, mapToUploadFile } from 'src/utils/asset.util';

export interface UploadFiles {
assetData: ImmichFile[];
Expand All @@ -35,16 +36,6 @@ export interface ImmichFile extends Express.Multer.File {
checksum: Buffer;
}

export function mapToUploadFile(file: ImmichFile): UploadFile {
return {
uuid: file.uuid,
checksum: file.checksum,
originalPath: file.path,
originalName: Buffer.from(file.originalname, 'latin1').toString('utf8'),
size: file.size,
};
}

type DiskStorageCallback = (error: Error | null, result: string) => void;

type ImmichMulterFile = Express.Multer.File & { uuid: string };
Expand All @@ -62,14 +53,6 @@ const callbackify = <T>(target: (...arguments_: any[]) => T, callback: Callback<
}
};

const asRequest = (request: AuthRequest, file: Express.Multer.File) => {
return {
auth: request.user || null,
fieldName: file.fieldname as UploadFieldName,
file: mapToUploadFile(file as ImmichFile),
};
};

@Injectable()
export class FileUploadInterceptor implements NestInterceptor {
private handlers: {
Expand Down Expand Up @@ -141,6 +124,12 @@ export class FileUploadInterceptor implements NestInterceptor {

private handleFile(request: AuthRequest, file: Express.Multer.File, callback: Callback<Partial<ImmichFile>>) {
(file as ImmichMulterFile).uuid = randomUUID();

request.on('error', (error) => {
this.logger.warn('Request error while uploading file, cleaning up', error);
this.assetService.onUploadError(request, file).catch(this.logger.error);
});

if (!this.isAssetUploadFile(file)) {
this.defaultStorage._handleFile(request, file, callback);
return;
Expand Down
25 changes: 25 additions & 0 deletions server/src/services/asset-media.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { IAssetRepository } from 'src/interfaces/asset.interface';
import { IJobRepository, JobName } from 'src/interfaces/job.interface';
import { IStorageRepository } from 'src/interfaces/storage.interface';
import { IUserRepository } from 'src/interfaces/user.interface';
import { AuthRequest } from 'src/middleware/auth.guard';
import { AssetMediaService } from 'src/services/asset-media.service';
import { ImmichFileResponse } from 'src/utils/file';
import { assetStub } from 'test/fixtures/asset.stub';
Expand Down Expand Up @@ -879,4 +880,28 @@ describe(AssetMediaService.name, () => {
expect(assetMock.getByChecksums).toHaveBeenCalledWith(authStub.admin.user.id, [file1, file2]);
});
});

describe('onUploadError', () => {
it('should queue a job to delete the uploaded file', async () => {
const request = { user: authStub.user1 } as AuthRequest;

const file = {
fieldname: UploadFieldName.ASSET_DATA,
originalname: 'image.jpg',
mimetype: 'image/jpeg',
buffer: Buffer.from(''),
size: 1000,
uuid: 'random-uuid',
checksum: Buffer.from('checksum', 'utf8'),
originalPath: 'upload/upload/user-id/ra/nd/random-uuid.jpg',
} as unknown as Express.Multer.File;

await sut.onUploadError(request, file);

expect(jobMock.queue).toHaveBeenCalledWith({
name: JobName.DELETE_FILES,
data: { files: ['upload/upload/user-id/ra/nd/random-uuid.jpg'] },
});
});
});
});
11 changes: 10 additions & 1 deletion server/src/services/asset-media.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ import { AuthDto } from 'src/dtos/auth.dto';
import { ASSET_CHECKSUM_CONSTRAINT, AssetEntity } from 'src/entities/asset.entity';
import { AssetStatus, AssetType, CacheControl, Permission, StorageFolder } from 'src/enum';
import { JobName } from 'src/interfaces/job.interface';
import { AuthRequest } from 'src/middleware/auth.guard';
import { BaseService } from 'src/services/base.service';
import { requireUploadAccess } from 'src/utils/access';
import { getAssetFiles, onBeforeLink } from 'src/utils/asset.util';
import { asRequest, getAssetFiles, onBeforeLink } from 'src/utils/asset.util';
import { ImmichFileResponse } from 'src/utils/file';
import { mimeTypes } from 'src/utils/mime-types';
import { fromChecksum } from 'src/utils/request';
Expand Down Expand Up @@ -118,6 +119,14 @@ export class AssetMediaService extends BaseService {
return folder;
}

async onUploadError(request: AuthRequest, file: Express.Multer.File) {
const uploadFilename = this.getUploadFilename(asRequest(request, file));
const uploadFolder = this.getUploadFolder(asRequest(request, file));
const uploadPath = `${uploadFolder}/${uploadFilename}`;

await this.jobRepository.queue({ name: JobName.DELETE_FILES, data: { files: [uploadPath] } });
}

async uploadAsset(
auth: AuthDto,
dto: AssetMediaCreateDto,
Expand Down
22 changes: 22 additions & 0 deletions server/src/utils/asset.util.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { BadRequestException } from '@nestjs/common';
import { 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';
import { AssetFileEntity } from 'src/entities/asset-files.entity';
import { AssetFileType, AssetType, Permission } from 'src/enum';
import { IAccessRepository } from 'src/interfaces/access.interface';
import { IAssetRepository } from 'src/interfaces/asset.interface';
import { IEventRepository } from 'src/interfaces/event.interface';
import { IPartnerRepository } from 'src/interfaces/partner.interface';
import { AuthRequest } from 'src/middleware/auth.guard';
import { ImmichFile } from 'src/middleware/file-upload.interceptor';
import { UploadFile } from 'src/services/asset-media.service';
import { checkAccess } from 'src/utils/access';

export interface IBulkAsset {
Expand Down Expand Up @@ -181,3 +185,21 @@ export const onAfterUnlink = async (
await assetRepository.update({ id: livePhotoVideoId, isVisible: true });
await eventRepository.emit('asset.show', { assetId: livePhotoVideoId, userId });
};

export function mapToUploadFile(file: ImmichFile): UploadFile {
return {
uuid: file.uuid,
checksum: file.checksum,
originalPath: file.path,
originalName: Buffer.from(file.originalname, 'latin1').toString('utf8'),
size: file.size,
};
}

export const asRequest = (request: AuthRequest, file: Express.Multer.File) => {
return {
auth: request.user || null,
fieldName: file.fieldname as UploadFieldName,
file: mapToUploadFile(file as ImmichFile),
};
};

0 comments on commit 6dfd4f3

Please sign in to comment.