Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(server): don't delete offline files from disk when trash empties #14777

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+299 −23
Open

fix(server): don't delete offline files from disk when trash empties #14777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion e2e/src/api/specs/asset.e2e-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ describe('/asset', () => {
expect(body).toEqual(errorDto.badRequest('Not found or no asset.delete access'));
});

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

const before = await utils.getAssetInfo(admin.accessToken, assetId);
Expand All @@ -782,6 +782,38 @@ 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);
}

await utils.waitForWebsocketEvent({ event: 'assetDelete', id: assetId });

{
const { status } = await request(app)
.get(`/assets/${assetId}`)
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
}
});

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
View file
Open in desktop
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
View file
Open in desktop
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
Loading