1
0
mirror of https://github.com/immich-app/immich.git synced 2024-11-24 08:52:28 +02:00

fix(server): storage usage calculation for motion photos (#8722)

* ignore non external assets in external libraries during syncUsage

* only update storage usage if asset is from internal libraries

* update storage usage on motion photo video asset creation

* updated metadata service tests

* added a test

* simplified syncUsage condition

* check for library type upload instead of not external

* fixed broken sql

---------

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>
This commit is contained in:
Kevin Huang 2024-04-16 20:04:59 -07:00 committed by GitHub
parent 6d4d0f86cf
commit 17dc12cf7d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 50 additions and 4 deletions

View File

@ -159,10 +159,12 @@ SET
COALESCE(SUM(exif."fileSizeInByte"), 0)
FROM
"assets" "assets"
LEFT JOIN "libraries" "library" ON "library"."id" = "assets"."libraryId"
AND ("library"."deletedAt" IS NULL)
LEFT JOIN "exif" "exif" ON "exif"."assetId" = "assets"."id"
WHERE
"assets"."ownerId" = users.id
AND NOT "assets"."isExternal"
AND "library"."type" = 'UPLOAD'
),
"updatedAt" = CURRENT_TIMESTAMP
WHERE

View File

@ -2,6 +2,7 @@ import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm';
import { DummyValue, GenerateSql } from 'src/decorators';
import { AssetEntity } from 'src/entities/asset.entity';
import { LibraryType } from 'src/entities/library.entity';
import { UserEntity } from 'src/entities/user.entity';
import {
IUserRepository,
@ -117,11 +118,14 @@ export class UserRepository implements IUserRepository {
@GenerateSql({ params: [DummyValue.UUID] })
async syncUsage(id?: string) {
// we can't use parameters with getQuery, hence the template string
const subQuery = this.assetRepository
.createQueryBuilder('assets')
.select('COALESCE(SUM(exif."fileSizeInByte"), 0)')
.leftJoin('assets.library', 'library')
.leftJoin('assets.exifInfo', 'exif')
.where('assets.ownerId = users.id AND NOT assets.isExternal')
.where('assets.ownerId = users.id')
.andWhere(`library.type = '${LibraryType.UPLOAD}'`)
.withDeleted();
const query = this.userRepository

View File

@ -392,7 +392,9 @@ export class AssetService {
}
await this.assetRepository.remove(asset);
if (asset.library.type === LibraryType.UPLOAD) {
await this.userRepository.updateUsage(asset.ownerId, -(asset.exifInfo?.fileSizeInByte || 0));
}
this.eventRepository.clientSend(ClientEvent.ASSET_DELETE, asset.ownerId, id);
// TODO refactor this to use cascades

View File

@ -18,6 +18,7 @@ import { IMoveRepository } from 'src/interfaces/move.interface';
import { IPersonRepository } from 'src/interfaces/person.interface';
import { IStorageRepository } from 'src/interfaces/storage.interface';
import { ISystemConfigRepository } from 'src/interfaces/system-config.interface';
import { IUserRepository } from 'src/interfaces/user.interface';
import { MetadataService, Orientation } from 'src/services/metadata.service';
import { assetStub } from 'test/fixtures/asset.stub';
import { fileStub } from 'test/fixtures/file.stub';
@ -35,6 +36,7 @@ import { newMoveRepositoryMock } from 'test/repositories/move.repository.mock';
import { newPersonRepositoryMock } from 'test/repositories/person.repository.mock';
import { newStorageRepositoryMock } from 'test/repositories/storage.repository.mock';
import { newSystemConfigRepositoryMock } from 'test/repositories/system-config.repository.mock';
import { newUserRepositoryMock } from 'test/repositories/user.repository.mock';
import { Mocked } from 'vitest';
describe(MetadataService.name, () => {
@ -50,6 +52,7 @@ describe(MetadataService.name, () => {
let storageMock: Mocked<IStorageRepository>;
let eventMock: Mocked<IEventRepository>;
let databaseMock: Mocked<IDatabaseRepository>;
let userMock: Mocked<IUserRepository>;
let loggerMock: Mocked<ILoggerRepository>;
let sut: MetadataService;
@ -66,6 +69,7 @@ describe(MetadataService.name, () => {
storageMock = newStorageRepositoryMock();
mediaMock = newMediaRepositoryMock();
databaseMock = newDatabaseRepositoryMock();
userMock = newUserRepositoryMock();
loggerMock = newLoggerRepositoryMock();
sut = new MetadataService(
@ -81,6 +85,7 @@ describe(MetadataService.name, () => {
personMock,
storageMock,
configMock,
userMock,
loggerMock,
);
});
@ -372,6 +377,7 @@ describe(MetadataService.name, () => {
);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added
expect(userMock.updateUsage).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.ownerId, 512);
expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
expect(assetMock.update).toHaveBeenNthCalledWith(1, {
id: assetStub.livePhotoStillAsset.id,
@ -400,6 +406,7 @@ describe(MetadataService.name, () => {
);
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added
expect(userMock.updateUsage).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.ownerId, 512);
expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
expect(assetMock.update).toHaveBeenNthCalledWith(1, {
id: assetStub.livePhotoStillAsset.id,
@ -426,6 +433,7 @@ describe(MetadataService.name, () => {
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
expect(storageMock.readFile).toHaveBeenCalledWith(assetStub.livePhotoStillAsset.originalPath, expect.any(Object));
expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added
expect(userMock.updateUsage).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.ownerId, 512);
expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
expect(assetMock.update).toHaveBeenNthCalledWith(1, {
id: assetStub.livePhotoStillAsset.id,
@ -444,6 +452,8 @@ describe(MetadataService.name, () => {
cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
assetMock.getByChecksum.mockResolvedValue(null);
assetMock.create.mockImplementation((asset) => Promise.resolve({ ...assetStub.livePhotoMotionAsset, ...asset }));
const video = randomBytes(512);
storageMock.readFile.mockResolvedValue(video);
await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
expect(jobMock.queue).toHaveBeenNthCalledWith(2, {
@ -452,7 +462,7 @@ describe(MetadataService.name, () => {
});
});
it('should not create a new motionphoto video asset if the of the extracted video matches an existing asset', async () => {
it('should not create a new motion photo video asset if the hash of the extracted video matches an existing asset', async () => {
assetMock.getByIds.mockResolvedValue([assetStub.livePhotoStillAsset]);
metadataMock.readTags.mockResolvedValue({
Directory: 'foo/bar/',
@ -462,6 +472,8 @@ describe(MetadataService.name, () => {
});
cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
assetMock.getByChecksum.mockResolvedValue(assetStub.livePhotoMotionAsset);
const video = randomBytes(512);
storageMock.readFile.mockResolvedValue(video);
await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
expect(assetMock.create).toHaveBeenCalledTimes(0);
@ -495,6 +507,26 @@ describe(MetadataService.name, () => {
});
});
it('should not update storage usage if motion photo is external', async () => {
assetMock.getByIds.mockResolvedValue([
{ ...assetStub.livePhotoStillAsset, livePhotoVideoId: null, isExternal: true },
]);
metadataMock.readTags.mockResolvedValue({
Directory: 'foo/bar/',
MotionPhoto: 1,
MicroVideo: 1,
MicroVideoOffset: 1,
});
cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
assetMock.getByChecksum.mockResolvedValue(null);
assetMock.create.mockResolvedValue(assetStub.livePhotoMotionAsset);
const video = randomBytes(512);
storageMock.readFile.mockResolvedValue(video);
await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
expect(userMock.updateUsage).not.toHaveBeenCalled();
});
it('should save all metadata', async () => {
const tags: ImmichTags = {
BitsPerSample: 1,

View File

@ -32,6 +32,7 @@ import { IMoveRepository } from 'src/interfaces/move.interface';
import { IPersonRepository } from 'src/interfaces/person.interface';
import { IStorageRepository } from 'src/interfaces/storage.interface';
import { ISystemConfigRepository } from 'src/interfaces/system-config.interface';
import { IUserRepository } from 'src/interfaces/user.interface';
import { handlePromiseError } from 'src/utils/misc';
import { usePagination } from 'src/utils/pagination';
@ -114,6 +115,7 @@ export class MetadataService {
@Inject(IPersonRepository) personRepository: IPersonRepository,
@Inject(IStorageRepository) private storageRepository: IStorageRepository,
@Inject(ISystemConfigRepository) configRepository: ISystemConfigRepository,
@Inject(IUserRepository) private userRepository: IUserRepository,
@Inject(ILoggerRepository) private logger: ILoggerRepository,
) {
this.logger.setContext(MetadataService.name);
@ -446,10 +448,14 @@ export class MetadataService {
this.storageCore.ensureFolders(motionPath);
await this.storageRepository.writeFile(motionAsset.originalPath, video);
await this.jobRepository.queue({ name: JobName.METADATA_EXTRACTION, data: { id: motionAsset.id } });
if (!asset.isExternal) {
await this.userRepository.updateUsage(asset.ownerId, video.byteLength);
}
}
if (asset.livePhotoVideoId !== motionAsset.id) {
await this.assetRepository.update({ id: asset.id, livePhotoVideoId: motionAsset.id });
// If the asset already had an associated livePhotoVideo, delete it, because
// its checksum doesn't match the checksum of the motionAsset we just extracted
// (if it did, getByChecksum() would've returned a motionAsset with the same ID as livePhotoVideoId)