From b256f2d7b2db27318728a63b7c2565241f87452c Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 27 May 2016 05:58:46 -0700 Subject: [PATCH] Prepare UserPreferences for transactions Summary: Ref T4103. This give preferences a PHID, policy/transaction interfaces, a transaction table, and a Query class. This doesn't actually change how they're edited, yet. Test Plan: - Ran migrations. - Inspected database for date created, date modified, PHIDs. - Changed some of my preferences. - Deleted a user's preferences, verified they reset properly. - Set some preferences as a new user, got a new row. - Destroyed a user, verified their preferences were destroyed. - Sent Conpherence messages. - Send mail. - Tried to edit another user's settings. - Tried to edit a bot's settings as a non-admin. - Edited a bot's settings as an admin (technically, none of the editable settings are actually stored in the settings table, currently). Reviewers: chad Reviewed By: chad Maniphest Tasks: T4103 Differential Revision: https://secure.phabricator.com/D15991 --- .../autopatches/20160531.pref.01.xaction.sql | 19 +++ .../20160531.pref.02.datecreatecol.sql | 2 + .../20160531.pref.03.datemodcol.sql | 2 + .../20160531.pref.04.datecreateval.sql | 2 + .../20160531.pref.05.datemodval.sql | 2 + .../autopatches/20160531.pref.06.phidcol.sql | 2 + .../autopatches/20160531.pref.07.phidval.php | 17 +++ src/__phutil_library_map__.php | 13 +- .../conpherence/editor/ConpherenceEditor.php | 14 ++- .../feed/PhabricatorFeedStoryPublisher.php | 7 +- .../storage/PhabricatorMetaMTAMail.php | 7 +- .../people/storage/PhabricatorUser.php | 16 +-- .../PhabricatorUserPreferencesPHIDType.php | 40 ++++++ .../query/PhabricatorUserPreferencesQuery.php | 118 +++++++++++++++++ .../storage/PhabricatorUserPreferences.php | 119 +++++++++++++++++- .../PhabricatorUserPreferencesTransaction.php | 18 +++ 16 files changed, 379 insertions(+), 19 deletions(-) create mode 100644 resources/sql/autopatches/20160531.pref.01.xaction.sql create mode 100644 resources/sql/autopatches/20160531.pref.02.datecreatecol.sql create mode 100644 resources/sql/autopatches/20160531.pref.03.datemodcol.sql create mode 100644 resources/sql/autopatches/20160531.pref.04.datecreateval.sql create mode 100644 resources/sql/autopatches/20160531.pref.05.datemodval.sql create mode 100644 resources/sql/autopatches/20160531.pref.06.phidcol.sql create mode 100644 resources/sql/autopatches/20160531.pref.07.phidval.php create mode 100644 src/applications/settings/phid/PhabricatorUserPreferencesPHIDType.php create mode 100644 src/applications/settings/query/PhabricatorUserPreferencesQuery.php create mode 100644 src/applications/settings/storage/PhabricatorUserPreferencesTransaction.php diff --git a/resources/sql/autopatches/20160531.pref.01.xaction.sql b/resources/sql/autopatches/20160531.pref.01.xaction.sql new file mode 100644 index 0000000000..0ff33f4b59 --- /dev/null +++ b/resources/sql/autopatches/20160531.pref.01.xaction.sql @@ -0,0 +1,19 @@ +CREATE TABLE {$NAMESPACE}_user.user_preferencestransaction ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + phid VARBINARY(64) NOT NULL, + authorPHID VARBINARY(64) NOT NULL, + objectPHID VARBINARY(64) NOT NULL, + viewPolicy VARBINARY(64) NOT NULL, + editPolicy VARBINARY(64) NOT NULL, + commentPHID VARBINARY(64) DEFAULT NULL, + commentVersion INT UNSIGNED NOT NULL, + transactionType VARCHAR(32) COLLATE {$COLLATE_TEXT} NOT NULL, + oldValue LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + newValue LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + contentSource LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + metadata LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + UNIQUE KEY `key_phid` (`phid`), + KEY `key_object` (`objectPHID`) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20160531.pref.02.datecreatecol.sql b/resources/sql/autopatches/20160531.pref.02.datecreatecol.sql new file mode 100644 index 0000000000..5f583fc972 --- /dev/null +++ b/resources/sql/autopatches/20160531.pref.02.datecreatecol.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.user_preferences + ADD dateCreated INT UNSIGNED NOT NULL; diff --git a/resources/sql/autopatches/20160531.pref.03.datemodcol.sql b/resources/sql/autopatches/20160531.pref.03.datemodcol.sql new file mode 100644 index 0000000000..bd9ebc96f7 --- /dev/null +++ b/resources/sql/autopatches/20160531.pref.03.datemodcol.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.user_preferences + ADD dateModified INT UNSIGNED NOT NULL; diff --git a/resources/sql/autopatches/20160531.pref.04.datecreateval.sql b/resources/sql/autopatches/20160531.pref.04.datecreateval.sql new file mode 100644 index 0000000000..fcaa8e0e0d --- /dev/null +++ b/resources/sql/autopatches/20160531.pref.04.datecreateval.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_user.user_preferences + SET dateCreated = UNIX_TIMESTAMP() WHERE dateCreated = 0; diff --git a/resources/sql/autopatches/20160531.pref.05.datemodval.sql b/resources/sql/autopatches/20160531.pref.05.datemodval.sql new file mode 100644 index 0000000000..8571509782 --- /dev/null +++ b/resources/sql/autopatches/20160531.pref.05.datemodval.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_user.user_preferences + SET dateModified = UNIX_TIMESTAMP() WHERE dateModified = 0; diff --git a/resources/sql/autopatches/20160531.pref.06.phidcol.sql b/resources/sql/autopatches/20160531.pref.06.phidcol.sql new file mode 100644 index 0000000000..ff6ac80010 --- /dev/null +++ b/resources/sql/autopatches/20160531.pref.06.phidcol.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.user_preferences + ADD phid VARBINARY(64) NOT NULL; diff --git a/resources/sql/autopatches/20160531.pref.07.phidval.php b/resources/sql/autopatches/20160531.pref.07.phidval.php new file mode 100644 index 0000000000..d5cb614c09 --- /dev/null +++ b/resources/sql/autopatches/20160531.pref.07.phidval.php @@ -0,0 +1,17 @@ +establishConnection('w'); + +foreach (new LiskMigrationIterator($table) as $row) { + if ($row->getPHID() !== '') { + continue; + } + + queryfx( + $conn_w, + 'UPDATE %T SET phid = %s WHERE id = %d', + $table->getTableName(), + $table->generatePHID(), + $row->getID()); +} diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6c5ac74685..f3f92e6998 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3592,6 +3592,9 @@ phutil_register_library_map(array( 'PhabricatorUserLogView' => 'applications/people/view/PhabricatorUserLogView.php', 'PhabricatorUserPHIDResolver' => 'applications/phid/resolver/PhabricatorUserPHIDResolver.php', 'PhabricatorUserPreferences' => 'applications/settings/storage/PhabricatorUserPreferences.php', + 'PhabricatorUserPreferencesPHIDType' => 'applications/settings/phid/PhabricatorUserPreferencesPHIDType.php', + 'PhabricatorUserPreferencesQuery' => 'applications/settings/query/PhabricatorUserPreferencesQuery.php', + 'PhabricatorUserPreferencesTransaction' => 'applications/settings/storage/PhabricatorUserPreferencesTransaction.php', 'PhabricatorUserProfile' => 'applications/people/storage/PhabricatorUserProfile.php', 'PhabricatorUserProfileEditor' => 'applications/people/editor/PhabricatorUserProfileEditor.php', 'PhabricatorUserRealNameField' => 'applications/people/customfield/PhabricatorUserRealNameField.php', @@ -8342,7 +8345,15 @@ phutil_register_library_map(array( ), 'PhabricatorUserLogView' => 'AphrontView', 'PhabricatorUserPHIDResolver' => 'PhabricatorPHIDResolver', - 'PhabricatorUserPreferences' => 'PhabricatorUserDAO', + 'PhabricatorUserPreferences' => array( + 'PhabricatorUserDAO', + 'PhabricatorPolicyInterface', + 'PhabricatorDestructibleInterface', + 'PhabricatorApplicationTransactionInterface', + ), + 'PhabricatorUserPreferencesPHIDType' => 'PhabricatorPHIDType', + 'PhabricatorUserPreferencesQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorUserPreferencesTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorUserProfile' => 'PhabricatorUserDAO', 'PhabricatorUserProfileEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorUserRealNameField' => 'PhabricatorUserCustomField', diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php index 9f2e3e2b2a..c68bb429a3 100644 --- a/src/applications/conpherence/editor/ConpherenceEditor.php +++ b/src/applications/conpherence/editor/ConpherenceEditor.php @@ -533,13 +533,20 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { protected function getMailTo(PhabricatorLiskDAO $object) { $to_phids = array(); + $participants = $object->getParticipants(); - if (empty($participants)) { + if (!$participants) { return $to_phids; } - $preferences = id(new PhabricatorUserPreferences()) - ->loadAllWhere('userPHID in (%Ls)', array_keys($participants)); + + $participant_phids = mpull($participants, 'getParticipantPHID'); + + $preferences = id(new PhabricatorUserPreferencesQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withUserPHIDs($participant_phids) + ->execute(); $preferences = mpull($preferences, null, 'getUserPHID'); + foreach ($participants as $phid => $participant) { $default = ConpherenceSettings::EMAIL_ALWAYS; $preference = idx($preferences, $phid); @@ -557,6 +564,7 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { $to_phids[] = $phid; } } + return $to_phids; } diff --git a/src/applications/feed/PhabricatorFeedStoryPublisher.php b/src/applications/feed/PhabricatorFeedStoryPublisher.php index 5a3ec6ea87..be476012c3 100644 --- a/src/applications/feed/PhabricatorFeedStoryPublisher.php +++ b/src/applications/feed/PhabricatorFeedStoryPublisher.php @@ -207,9 +207,10 @@ final class PhabricatorFeedStoryPublisher extends Phobject { $tags = $this->getMailTags(); if ($tags) { - $all_prefs = id(new PhabricatorUserPreferences())->loadAllWhere( - 'userPHID in (%Ls)', - $phids); + $all_prefs = id(new PhabricatorUserPreferencesQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withUserPHIDs($phids) + ->execute(); $all_prefs = mpull($all_prefs, null, 'getUserPHID'); } diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index 1ae5884228..95740ecbd4 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -940,9 +940,10 @@ final class PhabricatorMetaMTAMail } } - $all_prefs = id(new PhabricatorUserPreferences())->loadAllWhere( - 'userPHID in (%Ls)', - $actor_phids); + $all_prefs = id(new PhabricatorUserPreferencesQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withUserPHIDs($actor_phids) + ->execute(); $all_prefs = mpull($all_prefs, null, 'getUserPHID'); $value_email = PhabricatorUserPreferences::MAILTAG_PREFERENCE_EMAIL; diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 85b2590d33..855392fc62 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -494,9 +494,10 @@ final class PhabricatorUser $preferences = null; if ($this->getPHID()) { - $preferences = id(new PhabricatorUserPreferences())->loadOneWhere( - 'userPHID = %s', - $this->getPHID()); + $preferences = id(new PhabricatorUserPreferencesQuery()) + ->setViewer($this) + ->withUsers(array($this)) + ->executeOne(); } if (!$preferences) { @@ -1293,11 +1294,12 @@ final class PhabricatorUser $external->delete(); } - $prefs = id(new PhabricatorUserPreferences())->loadAllWhere( - 'userPHID = %s', - $this->getPHID()); + $prefs = id(new PhabricatorUserPreferencesQuery()) + ->setViewer($engine->getViewer()) + ->withUsers(array($this)) + ->execute(); foreach ($prefs as $pref) { - $pref->delete(); + $engine->destroyObject($pref); } $profiles = id(new PhabricatorUserProfile())->loadAllWhere( diff --git a/src/applications/settings/phid/PhabricatorUserPreferencesPHIDType.php b/src/applications/settings/phid/PhabricatorUserPreferencesPHIDType.php new file mode 100644 index 0000000000..8bd341b8c6 --- /dev/null +++ b/src/applications/settings/phid/PhabricatorUserPreferencesPHIDType.php @@ -0,0 +1,40 @@ +withPHIDs($phids); + } + + public function loadHandles( + PhabricatorHandleQuery $query, + array $handles, + array $objects) { + + $viewer = $query->getViewer(); + foreach ($handles as $phid => $handle) { + $preferences = $objects[$phid]; + + $handle->setName(pht('Settings %d', $preferences->getID())); + } + } + +} diff --git a/src/applications/settings/query/PhabricatorUserPreferencesQuery.php b/src/applications/settings/query/PhabricatorUserPreferencesQuery.php new file mode 100644 index 0000000000..fc3a332c57 --- /dev/null +++ b/src/applications/settings/query/PhabricatorUserPreferencesQuery.php @@ -0,0 +1,118 @@ +ids = $ids; + return $this; + } + + public function withPHIDs(array $phids) { + $this->phids = $phids; + return $this; + } + + public function withUserPHIDs(array $phids) { + $this->userPHIDs = $phids; + return $this; + } + + public function withUsers(array $users) { + assert_instances_of($users, 'PhabricatorUser'); + $this->users = mpull($users, null, 'getPHID'); + $this->withUserPHIDs(array_keys($this->users)); + return $this; + } + + public function newResultObject() { + return new PhabricatorUserPreferences(); + } + + protected function loadPage() { + return $this->loadStandardPage($this->newResultObject()); + } + + protected function willFilterPage(array $prefs) { + $user_phids = mpull($prefs, 'getUserPHID'); + $user_phids = array_filter($user_phids); + + // If some of the preferences are attached to users, try to use any objects + // we were handed first. If we're missing some, load them. + + if ($user_phids) { + $users = $this->users; + + $user_phids = array_fuse($user_phids); + $load_phids = array_diff_key($user_phids, $users); + $load_phids = array_keys($load_phids); + + if ($load_phids) { + $load_users = id(new PhabricatorPeopleQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($load_phids) + ->execute(); + $load_users = mpull($load_users, null, 'getPHID'); + $users += $load_users; + } + } else { + $users = array(); + } + + foreach ($prefs as $key => $pref) { + $user_phid = $pref->getUserPHID(); + if (!$user_phid) { + $pref->attachUser(null); + continue; + } + + $user = idx($users, $user_phid); + if (!$user) { + $this->didRejectResult($pref); + unset($prefs[$key]); + continue; + } + + $pref->attachUser($user); + } + + return $prefs; + } + + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); + + if ($this->ids !== null) { + $where[] = qsprintf( + $conn, + 'id IN (%Ld)', + $this->ids); + } + + if ($this->phids !== null) { + $where[] = qsprintf( + $conn, + 'phid IN (%Ls)', + $this->phids); + } + + if ($this->userPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'userPHID IN (%Ls)', + $this->userPHIDs); + } + + return $where; + } + + public function getQueryApplicationClass() { + return 'PhabricatorSettingsApplication'; + } + +} diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php index c14cc9b910..f90f82c4aa 100644 --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -1,6 +1,11 @@ true, self::CONFIG_SERIALIZATION => array( 'preferences' => self::SERIALIZATION_JSON, ), - self::CONFIG_TIMESTAMPS => false, self::CONFIG_KEY_SCHEMA => array( 'userPHID' => array( 'columns' => array('userPHID'), @@ -66,6 +73,11 @@ final class PhabricatorUserPreferences extends PhabricatorUserDAO { ) + parent::getConfiguration(); } + public function generatePHID() { + return PhabricatorPHID::generateNewPHID( + PhabricatorUserPreferencesPHIDType::TYPECONST); + } + public function getPreference($key, $default = null) { return idx($this->preferences, $key, $default); } @@ -115,4 +127,107 @@ final class PhabricatorUserPreferences extends PhabricatorUserDAO { return preg_replace('([^a-z0-9 ,"./]+)i', '', $monospaced); } + public function attachUser(PhabricatorUser $user = null) { + $this->user = $user; + return $this; + } + + public function getUser() { + return $this->assertAttached($this->user); + } + + public function hasManagedUser() { + $user_phid = $this->getUserPHID(); + if (!$user_phid) { + return false; + } + + $user = $this->getUser(); + if ($user->getIsSystemAgent() || $user->getIsMailingList()) { + return true; + } + + return false; + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + ); + } + + public function getPolicy($capability) { + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + $user_phid = $this->getUserPHID(); + if ($user_phid) { + return $user_phid; + } + + return PhabricatorPolicies::getMostOpenPolicy(); + case PhabricatorPolicyCapability::CAN_EDIT: + if ($this->hasManagedUser()) { + return PhabricatorPolicies::POLICY_ADMIN; + } + + $user_phid = $this->getUserPHID(); + if ($user_phid) { + return $user_phid; + } + + return PhabricatorPolicies::POLICY_ADMIN; + } + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + if ($this->hasManagedUser()) { + if ($viewer->getIsAdmin()) { + return true; + } + } + + return false; + } + + public function describeAutomaticCapability($capability) { + return null; + } + + +/* -( PhabricatorDestructibleInterface )----------------------------------- */ + + + public function destroyObjectPermanently( + PhabricatorDestructionEngine $engine) { + $this->delete(); + } + + +/* -( PhabricatorApplicationTransactionInterface )------------------------- */ + + + public function getApplicationTransactionEditor() { + // TODO: Implement. + throw new PhutilMethodNotImplementedException(); + } + + public function getApplicationTransactionObject() { + return $this; + } + + public function getApplicationTransactionTemplate() { + return new PhabricatorUserPreferencesTransaction(); + } + + public function willRenderTimeline( + PhabricatorApplicationTransactionView $timeline, + AphrontRequest $request) { + return $timeline; + } + } diff --git a/src/applications/settings/storage/PhabricatorUserPreferencesTransaction.php b/src/applications/settings/storage/PhabricatorUserPreferencesTransaction.php new file mode 100644 index 0000000000..2ffe86d85c --- /dev/null +++ b/src/applications/settings/storage/PhabricatorUserPreferencesTransaction.php @@ -0,0 +1,18 @@ +