Page MenuHomePhorge
Files F4874308

D26027.1749416983.diff
No OneTemporary
Actions

D26027.1749416983.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2217,6 +2217,7 @@
'PeopleDisableUsersCapability' => 'applications/people/capability/PeopleDisableUsersCapability.php',
'PeopleHovercardEngineExtension' => 'applications/people/engineextension/PeopleHovercardEngineExtension.php',
'PeopleMainMenuBarExtension' => 'applications/people/engineextension/PeopleMainMenuBarExtension.php',
+ 'PeopleProfilePictureBeforeDestructionEngineExtension' => 'applications/people/engineextension/PeopleProfilePictureBeforeDestructionEngineExtension.php',
'PeopleUserLogGarbageCollector' => 'applications/people/garbagecollector/PeopleUserLogGarbageCollector.php',
'Phabricator404Controller' => 'applications/base/controller/Phabricator404Controller.php',
'PhabricatorAWSConfigOptions' => 'applications/config/option/PhabricatorAWSConfigOptions.php',
@@ -8501,6 +8502,7 @@
'PeopleDisableUsersCapability' => 'PhabricatorPolicyCapability',
'PeopleHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension',
'PeopleMainMenuBarExtension' => 'PhabricatorMainMenuBarExtension',
+ 'PeopleProfilePictureBeforeDestructionEngineExtension' => 'PhabricatorBeforeDestructionEngineExtension',
'PeopleUserLogGarbageCollector' => 'PhabricatorGarbageCollector',
'Phabricator404Controller' => 'PhabricatorController',
'PhabricatorAWSConfigOptions' => 'PhabricatorApplicationConfigOptions',
diff --git a/src/applications/files/query/PhabricatorFileAttachmentQuery.php b/src/applications/files/query/PhabricatorFileAttachmentQuery.php
--- a/src/applications/files/query/PhabricatorFileAttachmentQuery.php
+++ b/src/applications/files/query/PhabricatorFileAttachmentQuery.php
@@ -4,25 +4,85 @@
extends PhabricatorCursorPagedPolicyAwareQuery {
private $objectPHIDs;
+ private $objectPHIDPrefix;
private $filePHIDs;
private $needFiles;
private $visibleFiles;
-
+ private $attachmentModes;
+
+ /**
+ * Filter with these object PHIDs.
+ *
+ * @param array<string> $object_phids Example: array('PHID-USER-123abc')
+ * @return $this
+ */
public function withObjectPHIDs(array $object_phids) {
$this->objectPHIDs = $object_phids;
return $this;
}
+ /**
+ * Filter with a PHID object type.
+ *
+ * This is just syntax sugar for the method withObjectPHIDPrefix(),
+ * so you can pass constants like PhabricatorPeopleUserPHIDType::TYPECONST.
+ *
+ * @param string $phid_type PHID type constant. Example: 'USER'.
+ * @return $this
+ */
+ public function withObjectPHIDType(string $phid_type) {
+ return $this->withObjectPHIDPrefix("PHID-{$phid_type}-");
+ }
+
+ /**
+ * Filter with a object PHID prefix string.
+ *
+ * @param string $phid_prefix PHID prefix. Example: 'PHID-USER-'
+ * @return $this
+ */
+ public function withObjectPHIDPrefix(string $phid_prefix) {
+ $this->objectPHIDPrefix = $phid_prefix;
+ return $this;
+ }
+
+ /**
+ * @param array<string> $file_phids Array of file PHIDs.
+ * @return $this
+ */
public function withFilePHIDs(array $file_phids) {
$this->filePHIDs = $file_phids;
return $this;
}
+ /**
+ * If the files must be visible by the current viewer.
+ *
+ * @param bool $visible_files
+ * @return $this
+ */
public function withVisibleFiles($visible_files) {
$this->visibleFiles = $visible_files;
return $this;
}
+ /**
+ * Filter with some attachment modes.
+ *
+ * @param array<string> $attachment_modes Attachment modes.
+ * E.g. array('attach', 'reference')
+ * @return $this
+ */
+ public function withAttachmentModes(array $attachment_modes) {
+ $this->attachmentModes = $attachment_modes;
+ return $this;
+ }
+
+ /**
+ * If you also need the file objects.
+ *
+ * @param bool $need True if you also need the file objects.
+ * @return $this
+ */
public function needFiles($need) {
$this->needFiles = $need;
return $this;
@@ -42,6 +102,13 @@
$this->objectPHIDs);
}
+ if ($this->objectPHIDPrefix !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'attachments.objectPHID LIKE %>',
+ $this->objectPHIDPrefix);
+ }
+
if ($this->filePHIDs !== null) {
$where[] = qsprintf(
$conn,
@@ -49,6 +116,13 @@
$this->filePHIDs);
}
+ if ($this->attachmentModes !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'attachments.attachmentMode IN (%Ls)',
+ $this->attachmentModes);
+ }
+
return $where;
}
diff --git a/src/applications/people/engineextension/PeopleProfilePictureBeforeDestructionEngineExtension.php b/src/applications/people/engineextension/PeopleProfilePictureBeforeDestructionEngineExtension.php
new file mode 100644
--- /dev/null
+++ b/src/applications/people/engineextension/PeopleProfilePictureBeforeDestructionEngineExtension.php
@@ -0,0 +1,107 @@
+<?php
+
+/**
+ * Before a profile picture is destroyed, set a builtin user profile picture.
+ * https://we.phorge.it/T16074
+ */
+final class PeopleProfilePictureBeforeDestructionEngineExtension
+ extends PhabricatorBeforeDestructionEngineExtension {
+
+ const EXTENSIONKEY = 'people-profiles';
+
+ public function getExtensionName(): string {
+ return pht('People Profile Pictures');
+ }
+
+ public function canBeforeDestroyObject(
+ PhabricatorDestructionEngine $engine,
+ $object): bool {
+
+ return ($object instanceof PhabricatorFile)
+ && $object->getIsProfileImage();
+ }
+
+ public function beforeDestroyObject(
+ PhabricatorDestructionEngine $destruction_engine,
+ $object) {
+
+ // File that will be destroyed soon.
+ // The file PHID is always non-empty at this point.
+ $file_phid = $object->getPHID();
+
+ // Note that a file that is used as profile images have
+ // the authorPHID = null, so it's not so obvious which
+ // is the affected user.
+ // https://we.phorge.it/T15407
+
+ // Note that we could find the affected users by running this
+ // very inefficient query that would lead to a full table scan:
+ // SELECT * FROM user WHERE profileImagePHID = $file_phid
+ // In the future it might make sense to add an index on 'profileImagePHID'
+ // to be able to avoid the next code.
+ // https://we.phorge.it/T16080
+
+ // We look at the file attachments to find the affected user efficiently.
+ // Note that file attachments are only available before destroying the file,
+ // and... fortunately we are inside a "Before Destruction" engine.
+ // This query is efficient thanks to the database index on 'filePHID' and
+ // the low cardinality of this result set.
+ $viewer = $destruction_engine->getViewer();
+ $file_attachments_query = new PhabricatorFileAttachmentQuery();
+ $file_attachments =
+ $file_attachments_query
+ ->setViewer($viewer)
+ ->withFilePHIDs(array($file_phid))
+ ->withObjectPHIDType(PhabricatorPeopleUserPHIDType::TYPECONST)
+ ->withAttachmentModes(array('attach'))
+ ->execute();
+ $attached_objects = mpull($file_attachments, 'getObject');
+
+ // Be 100% sure to only operate on users,
+ // and that these are really using this picture.
+ $affected_users = array();
+ foreach ($attached_objects as $attached_object) {
+ if (
+ ($attached_object instanceof PhabricatorUser)
+ && $attached_object->getProfileImagePHID() == $file_phid
+ ) {
+ $affected_users[] = $attached_object;
+ }
+ }
+
+ $user_table = new PhabricatorUser();
+
+ if (!$affected_users) {
+ // If we are here, the above fast speculation has found no users.
+ // It can happen when somebody manually used the "Detach File" button
+ // from the file (why people can generally do that? uhm).
+ // Only in this desperate case, we run this inefficient query.
+ // As already said, in the future we may want to add an index
+ // over the column 'profileImagePHID' in the table 'user',
+ // and in that case, we could avoid the above code about attachments.
+ // https://we.phorge.it/T16080
+ $affected_users = $user_table
+ ->loadAllWhere(
+ 'profileImagePHID = %s',
+ $file_phid);
+ }
+
+ // Avoid opening an empty transaction.
+ if (!$affected_users) {
+ return;
+ }
+
+ // Set the builtin profile image to each affected user.
+ // Premising that it's supposed to be just one user.
+ // Maybe in the future multiple users may use the same
+ // profile picture, so let's covers more corner cases,
+ // because we can.
+ $user_table->openTransaction();
+ foreach ($affected_users as $affected_user) {
+ $affected_user->setProfileImagePHID(null);
+ $affected_user->save();
+ }
+ $user_table->saveTransaction();
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Sun, Jun 8, 21:09 (6 h, 20 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1774926
Default Alt Text
D26027.1749416983.diff (8 KB)

Event Timeline

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