fix(server): add missing file extensions to library files (#8342)

* fix file extensions

* fix tests

* fix formatting

* fixed bug

* fix merts comments
This commit is contained in:
Jonathan Jogenfors
2024-03-29 03:51:07 +01:00
committed by GitHub
parent 3f61019ca1
commit ec48fccb30
3 changed files with 117 additions and 9 deletions

View File

@@ -501,17 +501,17 @@ describe(LibraryService.name, () => {
const mockLibraryJob: ILibraryFileJob = {
id: libraryStub.externalLibrary1.id,
ownerId: mockUser.id,
assetPath: '/data/user1/photo.jpg',
assetPath: assetStub.hasFileExtension.originalPath,
force: false,
};
storageMock.stat.mockResolvedValue({
size: 100,
mtime: assetStub.image.fileModifiedAt,
mtime: assetStub.hasFileExtension.fileModifiedAt,
ctime: new Date('2023-01-01'),
} as Stats);
assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.image);
assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.hasFileExtension);
await expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(JobStatus.SKIPPED);
@@ -548,6 +548,26 @@ describe(LibraryService.name, () => {
});
});
it('should import an asset that is missing a file extension', async () => {
// This tests for the case where the file extension is missing from the asset path.
// This happened in previous versions of Immich
const mockLibraryJob: ILibraryFileJob = {
id: libraryStub.externalLibrary1.id,
ownerId: mockUser.id,
assetPath: assetStub.missingFileExtension.originalPath,
force: false,
};
assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.missingFileExtension);
await expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(JobStatus.SUCCESS);
expect(assetMock.updateAll).toHaveBeenCalledWith(
[assetStub.missingFileExtension.id],
expect.objectContaining({ originalFileName: 'photo.jpg' }),
);
});
it('should set a missing asset to offline', async () => {
storageMock.stat.mockRejectedValue(new Error('Path not found'));
@@ -618,19 +638,20 @@ describe(LibraryService.name, () => {
it('should refresh an existing asset if forced', async () => {
const mockLibraryJob: ILibraryFileJob = {
id: assetStub.image.id,
ownerId: assetStub.image.ownerId,
assetPath: '/data/user1/photo.jpg',
ownerId: assetStub.hasFileExtension.ownerId,
assetPath: assetStub.hasFileExtension.originalPath,
force: true,
};
assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.image);
assetMock.create.mockResolvedValue(assetStub.image);
assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.hasFileExtension);
assetMock.create.mockResolvedValue(assetStub.hasFileExtension);
await expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(JobStatus.SUCCESS);
expect(assetMock.updateAll).toHaveBeenCalledWith([assetStub.image.id], {
expect(assetMock.updateAll).toHaveBeenCalledWith([assetStub.hasFileExtension.id], {
fileCreatedAt: new Date('2023-01-01'),
fileModifiedAt: new Date('2023-01-01'),
originalFileName: assetStub.hasFileExtension.originalFileName,
});
});

View File

@@ -443,6 +443,8 @@ export class LibraryService extends EventEmitter {
doRefresh = true;
}
const originalFileName = parse(assetPath).base;
if (!existingAssetEntity) {
// This asset is new to us, read it from disk
this.logger.debug(`Importing new asset: ${assetPath}`);
@@ -453,6 +455,12 @@ export class LibraryService extends EventEmitter {
`File modification time has changed, re-importing asset: ${assetPath}. Old mtime: ${existingAssetEntity.fileModifiedAt}. New mtime: ${stats.mtime}`,
);
doRefresh = true;
} else if (existingAssetEntity.originalFileName !== originalFileName) {
// TODO: We can likely remove this check in the second half of 2024 when all assets have likely been re-imported by all users
this.logger.debug(
`Asset is missing file extension, re-importing: ${assetPath}. Current incorrect filename: ${existingAssetEntity.originalFileName}.`,
);
doRefresh = true;
} else if (!job.force && stats && !existingAssetEntity.isOffline) {
// Asset exists on disk and in db and mtime has not changed. Also, we are not forcing refresn. Therefore, do nothing
this.logger.debug(`Asset already exists in database and on disk, will not import: ${assetPath}`);
@@ -510,7 +518,7 @@ export class LibraryService extends EventEmitter {
fileModifiedAt: stats.mtime,
localDateTime: stats.mtime,
type: assetType,
originalFileName: parse(assetPath).base,
originalFileName,
sidecarPath,
isReadOnly: true,
isExternal: true,
@@ -521,6 +529,7 @@ export class LibraryService extends EventEmitter {
await this.assetRepository.updateAll([existingAssetEntity.id], {
fileCreatedAt: stats.mtime,
fileModifiedAt: stats.mtime,
originalFileName,
});
} else {
// Not importing and not refreshing, do nothing