1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-09 16:32:39 +01:00

Destroy file attachments when file is deleted, or object is deleted

Summary:
Adds file attachment deletion logics:

- PhabricatorFile: delete the attachment if file is deleted
- destruction engine extension: delete attachment if object is deleted
- SQL patch: delete existing leftover attachments from deleted files

To apply the cleanup, as usual, run:

    ./bin/storage upgrade

This cleanup may take some time, proportionally to the size of these tables:

    phabricator_file.file
    phabricator_file.file_attachment

Just as an indication: the storage upgrade, in a Phorge with `file` count 1.3M rows and `file_attachment` consisting in 9K rows, it may delete 170K rows in less than 1 second on average hardware.

Closes T15110

Test Plan:
Apply the patch, run `./bin/storage/upgrade`:

- no "Unknown Object" in any "Referenced Files" curtain of any object.

Have phd daemon running.

Upload file, attach the file to a task, delete the file from the web interface:

- no "Unknown Object" in "Referenced Files" curtain of that task.
- the query `SELECT * FROM file_attachment WHERE filePHID = '<file phid>'` returns empty result

Upload file, attach the file to a task, delete the task from the `./bin/remove destroy` workflow:

- the query `SELECT * FROM file_attachment WHERE objectPHID = '<task phid>'` returns empty result

Reviewers: O1 Blessed Committers, avivey, valerio.bozzolan

Reviewed By: O1 Blessed Committers, avivey, valerio.bozzolan

Subscribers: Ekubischta, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Tags: #files

Maniphest Tasks: T15110

Differential Revision: https://we.phorge.it/D25051
This commit is contained in:
Dylan F 2024-08-10 22:15:10 +02:00 committed by Valerio Bozzolan
parent b74f1ad519
commit 2b7c0ec92f
4 changed files with 38 additions and 0 deletions

View file

@ -0,0 +1,6 @@
USE {$NAMESPACE}_file;
DELETE FROM file_attachment
WHERE NOT EXISTS
(SELECT *
FROM file
WHERE phid=file_attachment.filePHID)

View file

@ -3453,6 +3453,7 @@ phutil_register_library_map(array(
'PhabricatorFileAES256StorageFormat' => 'applications/files/format/PhabricatorFileAES256StorageFormat.php', 'PhabricatorFileAES256StorageFormat' => 'applications/files/format/PhabricatorFileAES256StorageFormat.php',
'PhabricatorFileAltTextTransaction' => 'applications/files/xaction/PhabricatorFileAltTextTransaction.php', 'PhabricatorFileAltTextTransaction' => 'applications/files/xaction/PhabricatorFileAltTextTransaction.php',
'PhabricatorFileAttachment' => 'applications/files/storage/PhabricatorFileAttachment.php', 'PhabricatorFileAttachment' => 'applications/files/storage/PhabricatorFileAttachment.php',
'PhabricatorFileAttachmentDestructionEngineExtension' => 'applications/files/engineextension/PhabricatorFileAttachmentDestructionEngineExtension.php',
'PhabricatorFileAttachmentQuery' => 'applications/files/query/PhabricatorFileAttachmentQuery.php', 'PhabricatorFileAttachmentQuery' => 'applications/files/query/PhabricatorFileAttachmentQuery.php',
'PhabricatorFileBundleLoader' => 'applications/files/query/PhabricatorFileBundleLoader.php', 'PhabricatorFileBundleLoader' => 'applications/files/query/PhabricatorFileBundleLoader.php',
'PhabricatorFileChunk' => 'applications/files/storage/PhabricatorFileChunk.php', 'PhabricatorFileChunk' => 'applications/files/storage/PhabricatorFileChunk.php',
@ -9924,6 +9925,7 @@ phutil_register_library_map(array(
'PhabricatorPolicyInterface', 'PhabricatorPolicyInterface',
'PhabricatorExtendedPolicyInterface', 'PhabricatorExtendedPolicyInterface',
), ),
'PhabricatorFileAttachmentDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension',
'PhabricatorFileAttachmentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorFileAttachmentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorFileBundleLoader' => 'Phobject', 'PhabricatorFileBundleLoader' => 'Phobject',
'PhabricatorFileChunk' => array( 'PhabricatorFileChunk' => array(

View file

@ -0,0 +1,22 @@
<?php
final class PhabricatorFileAttachmentDestructionEngineExtension
extends PhabricatorDestructionEngineExtension {
const EXTENSIONKEY = 'file.attachments';
public function getExtensionName() {
return pht('File Attachments');
}
public function destroyObject(
PhabricatorDestructionEngine $engine,
$object) {
$attachments = id(new PhabricatorFileAttachment())->loadAllWhere(
'objectPHID = %s',
$object->getPHID());
foreach ($attachments as $attachment) {
$attachment->delete();
}
}
}

View file

@ -1785,6 +1785,14 @@ final class PhabricatorFile extends PhabricatorFileDAO
PhabricatorDestructionEngine $engine) { PhabricatorDestructionEngine $engine) {
$this->openTransaction(); $this->openTransaction();
$attachments = id(new PhabricatorFileAttachment())->loadAllWhere(
'filePHID = %s',
$this->getPHID());
foreach ($attachments as $attachment) {
$attachment->delete();
}
$this->delete(); $this->delete();
$this->saveTransaction(); $this->saveTransaction();
} }