Skip to content

Commit b9c1ea8

Browse files
kaysondkaysond
committed
Fix formatting and fix/add tests
1 parent b5d79f9 commit b9c1ea8

File tree

4 files changed

+104
-35
lines changed

4 files changed

+104
-35
lines changed

Diff for: ‎server/src/domain/metadata/metadata.service.spec.ts

+95-27
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { AssetType, ExifEntity, SystemConfigKey } from '@app/infra/entities';
22
import {
33
assetStub,
4+
fileStub,
45
newAlbumRepositoryMock,
56
newAssetRepositoryMock,
67
newCommunicationRepositoryMock,
@@ -16,6 +17,7 @@ import {
1617
probeStub,
1718
} from '@test';
1819
import { randomBytes } from 'crypto';
20+
import { BinaryField } from 'exiftool-vendored';
1921
import { Stats } from 'fs';
2022
import { constants } from 'fs/promises';
2123
import { when } from 'jest-when';
@@ -343,61 +345,127 @@ describe(MetadataService.name, () => {
343345
);
344346
});
345347

346-
it('should apply motion photos', async () => {
348+
it('should extract the MotionPhotoVideo tag from Samsung HEIC motion photos', async () => {
347349
assetMock.getByIds.mockResolvedValue([{ ...assetStub.livePhotoStillAsset, livePhotoVideoId: null }]);
348350
metadataMock.readTags.mockResolvedValue({
349351
Directory: 'foo/bar/',
350-
MotionPhoto: 1,
351-
MicroVideo: 1,
352-
MicroVideoOffset: 1,
352+
MotionPhotoVideo: new BinaryField(0, ''),
353+
// The below two are included to ensure that the MotionPhotoVideo tag is extracted
354+
// instead of the EmbeddedVideoFile, since HEIC MotionPhotos include both
355+
EmbeddedVideoFile: new BinaryField(0, ''),
356+
EmbeddedVideoType: 'MotionPhoto_Data',
353357
});
354-
storageMock.readFile.mockResolvedValue(randomBytes(512));
355358
cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
356-
assetMock.getByChecksum.mockResolvedValue(assetStub.livePhotoMotionAsset);
359+
assetMock.getByChecksum.mockResolvedValue(null);
360+
assetMock.create.mockResolvedValue(assetStub.livePhotoMotionAsset);
361+
cryptoRepository.randomUUID.mockReturnValue(fileStub.livePhotoMotion.uuid);
362+
const video = randomBytes(512);
363+
metadataMock.extractBinaryTag.mockResolvedValue(video);
357364

358365
await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
366+
expect(metadataMock.extractBinaryTag).toHaveBeenCalledWith(
367+
assetStub.livePhotoStillAsset.originalPath,
368+
'MotionPhotoVideo',
369+
);
359370
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
360-
expect(storageMock.readFile).toHaveBeenCalledWith(assetStub.livePhotoStillAsset.originalPath, expect.any(Object));
361-
expect(assetMock.save).toHaveBeenCalledWith({
371+
expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added
372+
expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
373+
expect(assetMock.save).toHaveBeenNthCalledWith(1, {
362374
id: assetStub.livePhotoStillAsset.id,
363-
livePhotoVideoId: assetStub.livePhotoMotionAsset.id,
375+
livePhotoVideoId: fileStub.livePhotoMotion.uuid,
364376
});
365377
});
366378

367-
it('should create new motion asset if not found and link it with the photo', async () => {
379+
it('should extract the EmbeddedVideo tag from Samsung JPEG motion photos', async () => {
380+
assetMock.getByIds.mockResolvedValue([{ ...assetStub.livePhotoStillAsset, livePhotoVideoId: null }]);
381+
metadataMock.readTags.mockResolvedValue({
382+
Directory: 'foo/bar/',
383+
EmbeddedVideoFile: new BinaryField(0, ''),
384+
EmbeddedVideoType: 'MotionPhoto_Data',
385+
});
386+
cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
387+
assetMock.getByChecksum.mockResolvedValue(null);
388+
assetMock.create.mockResolvedValue(assetStub.livePhotoMotionAsset);
389+
cryptoRepository.randomUUID.mockReturnValue(fileStub.livePhotoMotion.uuid);
390+
const video = randomBytes(512);
391+
metadataMock.extractBinaryTag.mockResolvedValue(video);
392+
393+
await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
394+
expect(metadataMock.extractBinaryTag).toHaveBeenCalledWith(
395+
assetStub.livePhotoStillAsset.originalPath,
396+
'EmbeddedVideoFile',
397+
);
398+
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
399+
expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added
400+
expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
401+
expect(assetMock.save).toHaveBeenNthCalledWith(1, {
402+
id: assetStub.livePhotoStillAsset.id,
403+
livePhotoVideoId: fileStub.livePhotoMotion.uuid,
404+
});
405+
});
406+
407+
it('should extract the motion photo video from the XMP directory entry ', async () => {
368408
assetMock.getByIds.mockResolvedValue([{ ...assetStub.livePhotoStillAsset, livePhotoVideoId: null }]);
369409
metadataMock.readTags.mockResolvedValue({
370410
Directory: 'foo/bar/',
371411
MotionPhoto: 1,
372412
MicroVideo: 1,
373413
MicroVideoOffset: 1,
374414
});
415+
cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
416+
assetMock.getByChecksum.mockResolvedValue(null);
417+
assetMock.create.mockResolvedValue(assetStub.livePhotoMotionAsset);
418+
cryptoRepository.randomUUID.mockReturnValue(fileStub.livePhotoMotion.uuid);
375419
const video = randomBytes(512);
376420
storageMock.readFile.mockResolvedValue(video);
377-
cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
378-
assetMock.create.mockResolvedValueOnce(assetStub.livePhotoMotionAsset);
379-
assetMock.save.mockResolvedValueOnce(assetStub.livePhotoMotionAsset);
380421

381422
await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
382423
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
383424
expect(storageMock.readFile).toHaveBeenCalledWith(assetStub.livePhotoStillAsset.originalPath, expect.any(Object));
384-
expect(assetMock.create).toHaveBeenCalledWith(
385-
expect.objectContaining({
386-
type: AssetType.VIDEO,
387-
originalFileName: assetStub.livePhotoStillAsset.originalFileName,
388-
isVisible: false,
389-
isReadOnly: false,
390-
}),
391-
);
392-
expect(assetMock.save).toHaveBeenCalledWith({
425+
expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added
426+
expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
427+
expect(assetMock.save).toHaveBeenNthCalledWith(1, {
393428
id: assetStub.livePhotoStillAsset.id,
394-
livePhotoVideoId: assetStub.livePhotoMotionAsset.id,
429+
livePhotoVideoId: fileStub.livePhotoMotion.uuid,
395430
});
396-
expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
397-
expect(jobMock.queue).toHaveBeenCalledWith({
398-
name: JobName.METADATA_EXTRACTION,
399-
data: { id: assetStub.livePhotoMotionAsset.id },
431+
});
432+
433+
it('should delete old motion photo video assets if they do not match what is extracted', async () => {
434+
assetMock.getByIds.mockResolvedValue([assetStub.livePhotoStillAsset]);
435+
metadataMock.readTags.mockResolvedValue({
436+
Directory: 'foo/bar/',
437+
MotionPhoto: 1,
438+
MicroVideo: 1,
439+
MicroVideoOffset: 1,
440+
});
441+
cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
442+
assetMock.getByChecksum.mockResolvedValue(null);
443+
assetMock.create.mockResolvedValue(assetStub.livePhotoMotionAsset);
444+
445+
await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
446+
expect(jobMock.queue).toHaveBeenNthCalledWith(2, {
447+
name: JobName.ASSET_DELETION,
448+
data: { id: assetStub.livePhotoStillAsset.livePhotoVideoId },
449+
});
450+
});
451+
452+
it('should not create a new motionphoto video asset if the of the extracted video matches an existing asset', async () => {
453+
assetMock.getByIds.mockResolvedValue([assetStub.livePhotoStillAsset]);
454+
metadataMock.readTags.mockResolvedValue({
455+
Directory: 'foo/bar/',
456+
MotionPhoto: 1,
457+
MicroVideo: 1,
458+
MicroVideoOffset: 1,
400459
});
460+
cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
461+
assetMock.getByChecksum.mockResolvedValue(assetStub.livePhotoMotionAsset);
462+
463+
await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
464+
expect(assetMock.create).toHaveBeenCalledTimes(0);
465+
expect(storageMock.writeFile).toHaveBeenCalledTimes(0);
466+
// The still asset gets saved by handleMetadataExtraction, but not the video
467+
expect(assetMock.save).toHaveBeenCalledTimes(1);
468+
expect(jobMock.queue).toHaveBeenCalledTimes(0);
401469
});
402470

403471
it('should save all metadata', async () => {

Diff for: ‎server/src/domain/metadata/metadata.service.ts

+7-6
Original file line numberDiff line numberDiff line change
@@ -412,11 +412,11 @@ export class MetadataService {
412412
}
413413
const checksum = this.cryptoRepository.hashSha1(video);
414414

415-
416415
let motionAsset = await this.assetRepository.getByChecksum(asset.ownerId, checksum);
417416
if (!motionAsset) {
418-
// We
419-
const motionAssetId = this.cryptoRepository.randomUUID()
417+
// We create a UUID in advance so that each extracted video can have a unique filename
418+
// (allowing us to delete old ones if necessary)
419+
const motionAssetId = this.cryptoRepository.randomUUID();
420420
const motionPath = StorageCore.getAndroidMotionPath(asset, motionAssetId);
421421
const createdAt = asset.fileCreatedAt ?? asset.createdAt;
422422
motionAsset = await this.assetRepository.create({
@@ -435,7 +435,7 @@ export class MetadataService {
435435
deviceAssetId: 'NONE',
436436
deviceId: 'NONE',
437437
});
438-
438+
439439
this.storageCore.ensureFolders(motionPath);
440440
await this.storageRepository.writeFile(motionAsset.originalPath, video);
441441
await this.jobRepository.queue({ name: JobName.METADATA_EXTRACTION, data: { id: motionAsset.id } });
@@ -448,10 +448,11 @@ export class MetadataService {
448448
await this.jobRepository.queue({ name: JobName.ASSET_DELETION, data: { id: asset.livePhotoVideoId } });
449449
this.logger.log(`Removed old motion photo video asset (${asset.livePhotoVideoId})`);
450450
}
451-
452451
} else {
453452
this.logger.debug(
454-
`Asset ${asset.id}'s motion photo video with checksum ${checksum.toString('base64')} already exists in the repository`,
453+
`Asset ${asset.id}'s motion photo video with checksum ${checksum.toString(
454+
'base64',
455+
)} already exists in the repository`,
455456
);
456457
}
457458

Diff for: ‎server/test/fixtures/asset.stub.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ export const assetStub = {
401401
}),
402402

403403
livePhotoMotionAsset: Object.freeze({
404-
id: 'live-photo-motion-asset',
404+
id: fileStub.livePhotoMotion.uuid,
405405
originalPath: fileStub.livePhotoMotion.originalPath,
406406
ownerId: authStub.user1.user.id,
407407
type: AssetType.VIDEO,

Diff for: ‎server/test/fixtures/file.stub.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export const fileStub = {
77
size: 42,
88
}),
99
livePhotoMotion: Object.freeze({
10-
uuid: 'random-uuid',
10+
uuid: 'live-photo-motion-asset',
1111
originalPath: 'fake_path/asset_1.mp4',
1212
checksum: Buffer.from('live photo file hash', 'utf8'),
1313
originalName: 'asset_1.mp4',

0 commit comments

Comments
 (0)