Skip to content

Commit

Permalink
fix: don't delete offline files from disk when emptying trash
Browse files Browse the repository at this point in the history
  • Loading branch information
etnoy committed Dec 18, 2024
1 parent e0fc873 commit 0cffb62
Show file tree
Hide file tree
Showing 6 changed files with 293 additions and 22 deletions.
27 changes: 27 additions & 0 deletions e2e/src/api/specs/asset.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
LoginResponseDto,
SharedLinkType,
getAssetInfo,
getAssetStatistics,
getConfig,
getMyUser,
updateConfig,
Expand Down Expand Up @@ -782,6 +783,32 @@ describe('/asset', () => {
expect(after.isTrashed).toBe(true);
});

it('should permanently delete an asset from trash', async () => {
const { id: assetId } = await utils.createAsset(admin.accessToken);

{
const { status } = await request(app)
.delete('/assets')
.send({ ids: [assetId] })
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(204);
}

const trashed = await utils.getAssetInfo(admin.accessToken, assetId);
expect(trashed.isTrashed).toBe(true);

{
const { status } = await request(app)
.delete('/assets')
.send({ ids: [assetId], force: true })
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(204);
}

const after = await getAssetStatistics({ isTrashed: true }, { headers: asBearerAuth(admin.accessToken) });
expect(after.total).toBe(0);

Check failure on line 809 in e2e/src/api/specs/asset.e2e-spec.ts

View workflow job for this annotation

GitHub Actions / End-to-End Tests (Server & CLI)

src/api/specs/asset.e2e-spec.ts > /asset > DELETE /assets > should permanently delete an asset from trash

AssertionError: expected 1 to be +0 // Object.is equality - Expected + Received - 0 + 1 ❯ src/api/specs/asset.e2e-spec.ts:809:27
});

it('should clean up live photos', async () => {
const { id: motionId } = await utils.createAsset(admin.accessToken, {
assetData: { filename: 'test.mp4', bytes: makeRandomImage() },
Expand Down
205 changes: 195 additions & 10 deletions e2e/src/api/specs/library.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ describe('/libraries', () => {
expect(newAssets.items).toEqual([]);
});

it('should set an asset offline its file is not in any import path', async () => {
it('should set an asset offline if its file is not in any import path', async () => {
utils.createImageFile(`${testAssetDir}/temp/offline/offline.png`);

const library = await utils.createLibrary(admin.accessToken, {
Expand All @@ -515,10 +515,9 @@ describe('/libraries', () => {

utils.createDirectory(`${testAssetDir}/temp/another-path/`);

await request(app)
.put(`/libraries/${library.id}`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send({ importPaths: [`${testAssetDirInternal}/temp/another-path/`] });
await utils.updateLibrary(admin.accessToken, library.id, {
importPaths: [`${testAssetDirInternal}/temp/another-path/`],
});

const { status } = await request(app)
.post(`/libraries/${library.id}/scan`)
Expand Down Expand Up @@ -555,10 +554,7 @@ describe('/libraries', () => {
});
expect(assets.count).toBe(1);

await request(app)
.put(`/libraries/${library.id}`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send({ exclusionPatterns: ['**/directoryB/**'] });
await utils.updateLibrary(admin.accessToken, library.id, { exclusionPatterns: ['**/directoryB/**'] });

await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');
Expand All @@ -577,7 +573,7 @@ describe('/libraries', () => {
]);
});

it('should not trash an online asset', async () => {
it('should not set an asset offline if its file exists, is in an import path, and not covered by an exclusion pattern', async () => {
const library = await utils.createLibrary(admin.accessToken, {
ownerId: admin.userId,
importPaths: [`${testAssetDirInternal}/temp`],
Expand All @@ -601,6 +597,195 @@ describe('/libraries', () => {

expect(assets).toEqual(assetsBefore);
});

it('should set an offline asset to online if its file exists, is in an import path, and not covered by an exclusion pattern', async () => {
utils.createImageFile(`${testAssetDir}/temp/offline/offline.png`);

const library = await utils.createLibrary(admin.accessToken, {
ownerId: admin.userId,
importPaths: [`${testAssetDirInternal}/temp/offline`],
});

await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');

const { assets } = await utils.searchAssets(admin.accessToken, { libraryId: library.id });

utils.renameImageFile(`${testAssetDir}/temp/offline/offline.png`, `${testAssetDir}/temp/offline.png`);

{
const { status } = await request(app)
.post(`/libraries/${library.id}/scan`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send();
expect(status).toBe(204);
}

await utils.waitForQueueFinish(admin.accessToken, 'library');

const offlineAsset = await utils.getAssetInfo(admin.accessToken, assets.items[0].id);
expect(offlineAsset.isTrashed).toBe(true);
expect(offlineAsset.originalPath).toBe(`${testAssetDirInternal}/temp/offline/offline.png`);
expect(offlineAsset.isOffline).toBe(true);

{
const { assets } = await utils.searchAssets(admin.accessToken, { libraryId: library.id, withDeleted: true });
expect(assets.count).toBe(1);
}

utils.renameImageFile(`${testAssetDir}/temp/offline.png`, `${testAssetDir}/temp/offline/offline.png`);

{
const { status } = await request(app)
.post(`/libraries/${library.id}/scan`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send();
expect(status).toBe(204);
}

await utils.waitForQueueFinish(admin.accessToken, 'library');

const backOnlineAsset = await utils.getAssetInfo(admin.accessToken, assets.items[0].id);

expect(backOnlineAsset.isTrashed).toBe(false);
expect(backOnlineAsset.originalPath).toBe(`${testAssetDirInternal}/temp/offline/offline.png`);
expect(backOnlineAsset.isOffline).toBe(false);

{
const { assets } = await utils.searchAssets(admin.accessToken, { libraryId: library.id });
expect(assets.count).toBe(1);
}
});

it('should not set an offline asset to online if its file exists, is not covered by an exclusion pattern, but is outside of all import paths', async () => {
utils.createImageFile(`${testAssetDir}/temp/offline/offline.png`);

const library = await utils.createLibrary(admin.accessToken, {
ownerId: admin.userId,
importPaths: [`${testAssetDirInternal}/temp/offline`],
});

await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');

const { assets } = await utils.searchAssets(admin.accessToken, { libraryId: library.id });

utils.renameImageFile(`${testAssetDir}/temp/offline/offline.png`, `${testAssetDir}/temp/offline.png`);

{
const { status } = await request(app)
.post(`/libraries/${library.id}/scan`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send();
expect(status).toBe(204);
}

await utils.waitForQueueFinish(admin.accessToken, 'library');

{
const { assets } = await utils.searchAssets(admin.accessToken, { libraryId: library.id, withDeleted: true });
expect(assets.count).toBe(1);
}

const offlineAsset = await utils.getAssetInfo(admin.accessToken, assets.items[0].id);

expect(offlineAsset.isTrashed).toBe(true);
expect(offlineAsset.originalPath).toBe(`${testAssetDirInternal}/temp/offline/offline.png`);
expect(offlineAsset.isOffline).toBe(true);

utils.renameImageFile(`${testAssetDir}/temp/offline.png`, `${testAssetDir}/temp/offline/offline.png`);

utils.createDirectory(`${testAssetDir}/temp/another-path/`);

await utils.updateLibrary(admin.accessToken, library.id, {
importPaths: [`${testAssetDirInternal}/temp/another-path`],
});

{
const { status } = await request(app)
.post(`/libraries/${library.id}/scan`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send();
expect(status).toBe(204);
}

await utils.waitForQueueFinish(admin.accessToken, 'library');

const stillOfflineAsset = await utils.getAssetInfo(admin.accessToken, assets.items[0].id);

expect(stillOfflineAsset.isTrashed).toBe(true);
expect(stillOfflineAsset.originalPath).toBe(`${testAssetDirInternal}/temp/offline/offline.png`);
expect(stillOfflineAsset.isOffline).toBe(true);

{
const { assets } = await utils.searchAssets(admin.accessToken, { libraryId: library.id, withDeleted: true });
expect(assets.count).toBe(1);
}

utils.removeDirectory(`${testAssetDir}/temp/another-path/`);
});

it('should not set an offline asset to online if its file exists, is in an import path, but is covered by an exclusion pattern', async () => {
utils.createImageFile(`${testAssetDir}/temp/offline/offline.png`);

const library = await utils.createLibrary(admin.accessToken, {
ownerId: admin.userId,
importPaths: [`${testAssetDirInternal}/temp/offline`],
});

await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');

const { assets } = await utils.searchAssets(admin.accessToken, { libraryId: library.id });

utils.renameImageFile(`${testAssetDir}/temp/offline/offline.png`, `${testAssetDir}/temp/offline.png`);

{
const { status } = await request(app)
.post(`/libraries/${library.id}/scan`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send();
expect(status).toBe(204);
}

await utils.waitForQueueFinish(admin.accessToken, 'library');

{
const { assets } = await utils.searchAssets(admin.accessToken, { libraryId: library.id, withDeleted: true });
expect(assets.count).toBe(1);
}

const offlineAsset = await utils.getAssetInfo(admin.accessToken, assets.items[0].id);

expect(offlineAsset.isTrashed).toBe(true);
expect(offlineAsset.originalPath).toBe(`${testAssetDirInternal}/temp/offline/offline.png`);
expect(offlineAsset.isOffline).toBe(true);

utils.renameImageFile(`${testAssetDir}/temp/offline.png`, `${testAssetDir}/temp/offline/offline.png`);

await utils.updateLibrary(admin.accessToken, library.id, { exclusionPatterns: ['**/offline/**'] });

{
const { status } = await request(app)
.post(`/libraries/${library.id}/scan`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send();
expect(status).toBe(204);
}

await utils.waitForQueueFinish(admin.accessToken, 'library');

const stillOfflineAsset = await utils.getAssetInfo(admin.accessToken, assets.items[0].id);

expect(stillOfflineAsset.isTrashed).toBe(true);
expect(stillOfflineAsset.originalPath).toBe(`${testAssetDirInternal}/temp/offline/offline.png`);
expect(stillOfflineAsset.isOffline).toBe(true);

{
const { assets } = await utils.searchAssets(admin.accessToken, { libraryId: library.id, withDeleted: true });
expect(assets.count).toBe(1);
}
});
});

describe('POST /libraries/:id/validate', () => {
Expand Down
47 changes: 41 additions & 6 deletions e2e/src/api/specs/trash.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('/trash', () => {
expect(existsSync(before.originalPath)).toBe(false);
});

it('should not delete offline-trashed assets from disk', async () => {
it('should remove offline assets', async () => {
const library = await utils.createLibrary(admin.accessToken, {
ownerId: admin.userId,
importPaths: [`${testAssetDirInternal}/temp/offline`],
Expand All @@ -88,7 +88,7 @@ describe('/trash', () => {
expect(assets.items.length).toBe(1);
const asset = assets.items[0];

utils.removeImageFile(`${testAssetDir}/temp/offline/offline.png`);
await utils.updateLibrary(admin.accessToken, library.id, { exclusionPatterns: ['**/offline/**'] });

await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');
Expand All @@ -105,6 +105,41 @@ describe('/trash', () => {

const assetAfter = await utils.getAssetInfo(admin.accessToken, asset.id);
expect(assetAfter).toMatchObject({ isTrashed: true, isOffline: true });
});

it.skip('should not delete offline assets from disk', async () => {
// Can't be tested at the moment due to no mechanism to forward time
const library = await utils.createLibrary(admin.accessToken, {
ownerId: admin.userId,
importPaths: [`${testAssetDirInternal}/temp/offline`],
});

utils.createImageFile(`${testAssetDir}/temp/offline/offline.png`);

await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');

const { assets } = await utils.searchAssets(admin.accessToken, { libraryId: library.id });
expect(assets.items.length).toBe(1);
const asset = assets.items[0];

await utils.updateLibrary(admin.accessToken, library.id, { exclusionPatterns: ['**/offline/**'] });

await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');

const assetBefore = await utils.getAssetInfo(admin.accessToken, asset.id);
expect(assetBefore).toMatchObject({ isTrashed: true, isOffline: true });

utils.createImageFile(`${testAssetDir}/temp/offline/offline.png`);

const { status } = await request(app).post('/trash/empty').set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(200);

await utils.waitForQueueFinish(admin.accessToken, 'backgroundTask');

const after = await getAssetStatistics({ isTrashed: true }, { headers: asBearerAuth(admin.accessToken) });
expect(after.total).toBe(0);

expect(existsSync(`${testAssetDir}/temp/offline/offline.png`)).toBe(true);

Expand Down Expand Up @@ -137,7 +172,7 @@ describe('/trash', () => {
expect(after).toStrictEqual(expect.objectContaining({ id: assetId, isTrashed: false }));
});

it('should not restore offline-trashed assets', async () => {
it('should not restore offline assets', async () => {
const library = await utils.createLibrary(admin.accessToken, {
ownerId: admin.userId,
importPaths: [`${testAssetDirInternal}/temp/offline`],
Expand All @@ -152,7 +187,7 @@ describe('/trash', () => {
expect(assets.count).toBe(1);
const assetId = assets.items[0].id;

utils.removeImageFile(`${testAssetDir}/temp/offline/offline.png`);
await utils.updateLibrary(admin.accessToken, library.id, { exclusionPatterns: ['**/offline/**'] });

await scan(admin.accessToken, library.id);

Expand Down Expand Up @@ -195,7 +230,7 @@ describe('/trash', () => {
expect(after.isTrashed).toBe(false);
});

it('should not restore an offline-trashed asset', async () => {
it('should not restore an offline asset', async () => {
const library = await utils.createLibrary(admin.accessToken, {
ownerId: admin.userId,
importPaths: [`${testAssetDirInternal}/temp/offline`],
Expand All @@ -210,7 +245,7 @@ describe('/trash', () => {
expect(assets.count).toBe(1);
const assetId = assets.items[0].id;

utils.removeImageFile(`${testAssetDir}/temp/offline/offline.png`);
await utils.updateLibrary(admin.accessToken, library.id, { exclusionPatterns: ['**/offline/**'] });

await scan(admin.accessToken, library.id);
await utils.waitForQueueFinish(admin.accessToken, 'library');
Expand Down
Loading

0 comments on commit 0cffb62

Please sign in to comment.