Page MenuHomePhorge
Differential D25051 Diff 2350 src/applications/files/storage/PhabricatorFile.php

Changeset View

Standalone View

View Options

src/applications/files/storage/PhabricatorFile.php

Show First 20 Lines • Show All 1,779 Lines • ▼ Show 20 Lines
/* -( PhabricatorDestructibleInterface )----------------------------------- */ /* -( PhabricatorDestructibleInterface )----------------------------------- */
public function destroyObjectPermanently( public function destroyObjectPermanently(
PhabricatorDestructionEngine $engine) { PhabricatorDestructionEngine $engine) {
$this->openTransaction(); $this->openTransaction();
$attachments = id(new PhabricatorFileAttachment())->loadAllWhere(
speckUnsubmitted
Not Done
Inline Actions

Should this just be using the destruction engine passed in?

speck: Should this just be using the destruction engine passed in?
valerio.bozzolanUnsubmitted
Not Done
Inline Actions

Do you mean having line 1736 $engine->destroyObject($attachment)?

Probably yes 🤔

And maybe same in PhabricatorFileAttachmentDestructionEngineExtension:19

valerio.bozzolan: Do you mean having line 1736 `$engine->destroyObject($attachment)`? Probably yes 🤔 And maybe…
'filePHID = %s',
$this->getPHID());
valerio.bozzolanUnsubmitted
Done
Inline Actions

✅ Note that I like that you have built this query in the raw way, instead of using PhabricatorFileAttachmentQuery() since in the raw way it does not try to pull the objects, and, it will not discard results if objects are already deleted for some reasons, and, there is no extra overhead for the omnipotent user thing.

valerio.bozzolan: ✅ Note that I like that you have built this query in the raw way, instead of using…
foreach ($attachments as $attachment) {
$attachment->delete();
aviveyUnsubmitted
Not Done
Inline Actions

I don't like using mpull for the side-effect of the method invoked.

avivey: I don't like using `mpull` for the side-effect of the method invoked.
valerio.bozzolanUnsubmitted
Not Done
Inline Actions

I agree, also because it returns an array of [null, null, null, null] which is not useful to read. A simple foreach() may be better

valerio.bozzolan: I agree, also because it returns an array of `[null, null, null, null]` which is not useful to…
valerio.bozzolanUnsubmitted
Not Done
Inline Actions
foreach ($attachments as $attachment) {
- $attachment->delete();
+ $engine->destroyObject($attachment);
}
$this->delete();

I'm unsure. It's just an idea thanks to speck (2)

valerio.bozzolan: I'm unsure. It's just an idea thanks to speck (2)
}
$this->delete(); $this->delete();
$this->saveTransaction(); $this->saveTransaction();
} }
/* -( PhabricatorConduitResultInterface )---------------------------------- */ /* -( PhabricatorConduitResultInterface )---------------------------------- */
▲ Show 20 Lines • Show All 49 Lines • Show Last 20 Lines
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0