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 @@ -312,7 +312,6 @@ 'ConpherenceRoomListController' => 'applications/conpherence/controller/ConpherenceRoomListController.php', 'ConpherenceRoomTestCase' => 'applications/conpherence/__tests__/ConpherenceRoomTestCase.php', 'ConpherenceSchemaSpec' => 'applications/conpherence/storage/ConpherenceSchemaSpec.php', - 'ConpherenceSettings' => 'applications/conpherence/constants/ConpherenceSettings.php', 'ConpherenceTestCase' => 'applications/conpherence/__tests__/ConpherenceTestCase.php', 'ConpherenceThread' => 'applications/conpherence/storage/ConpherenceThread.php', 'ConpherenceThreadIndexEngineExtension' => 'applications/conpherence/engineextension/ConpherenceThreadIndexEngineExtension.php', @@ -3636,6 +3635,7 @@ 'PhabricatorUserPreferencesPHIDType' => 'applications/settings/phid/PhabricatorUserPreferencesPHIDType.php', 'PhabricatorUserPreferencesQuery' => 'applications/settings/query/PhabricatorUserPreferencesQuery.php', 'PhabricatorUserPreferencesTransaction' => 'applications/settings/storage/PhabricatorUserPreferencesTransaction.php', + 'PhabricatorUserPreferencesTransactionQuery' => 'applications/settings/query/PhabricatorUserPreferencesTransactionQuery.php', 'PhabricatorUserProfile' => 'applications/people/storage/PhabricatorUserProfile.php', 'PhabricatorUserProfileEditor' => 'applications/people/editor/PhabricatorUserProfileEditor.php', 'PhabricatorUserRealNameField' => 'applications/people/customfield/PhabricatorUserRealNameField.php', @@ -4565,7 +4565,6 @@ 'ConpherenceRoomListController' => 'ConpherenceController', 'ConpherenceRoomTestCase' => 'ConpherenceTestCase', 'ConpherenceSchemaSpec' => 'PhabricatorConfigSchemaSpec', - 'ConpherenceSettings' => 'ConpherenceConstants', 'ConpherenceTestCase' => 'PhabricatorTestCase', 'ConpherenceThread' => array( 'ConpherenceDAO', @@ -8439,6 +8438,7 @@ 'PhabricatorUserPreferencesPHIDType' => 'PhabricatorPHIDType', 'PhabricatorUserPreferencesQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorUserPreferencesTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorUserPreferencesTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorUserProfile' => 'PhabricatorUserDAO', 'PhabricatorUserProfileEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorUserRealNameField' => 'PhabricatorUserCustomField', diff --git a/src/applications/conpherence/constants/ConpherenceSettings.php b/src/applications/conpherence/constants/ConpherenceSettings.php deleted file mode 100644 --- a/src/applications/conpherence/constants/ConpherenceSettings.php +++ /dev/null @@ -1,22 +0,0 @@ -<?php - -final class ConpherenceSettings extends ConpherenceConstants { - - const EMAIL_ALWAYS = 0; - const NOTIFICATIONS_ONLY = 1; - - public static function getHumanString($constant) { - $string = pht('Unknown setting.'); - - switch ($constant) { - case self::EMAIL_ALWAYS: - $string = pht('Email me every update.'); - break; - case self::NOTIFICATIONS_ONLY: - $string = pht('Notifications only.'); - break; - } - - return $string; - } -} diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -127,9 +127,14 @@ } $participant->setSettings(array('notifications' => $notifications)); $participant->save(); + + $label = PhabricatorConpherenceNotificationsSetting::getSettingLabel( + $notifications); + $result = pht( 'Updated notification settings to "%s".', - ConpherenceSettings::getHumanString($notifications)); + $label); + return id(new AphrontAjaxResponse()) ->setContent($result); break; diff --git a/src/applications/conpherence/controller/ConpherenceWidgetController.php b/src/applications/conpherence/controller/ConpherenceWidgetController.php --- a/src/applications/conpherence/controller/ConpherenceWidgetController.php +++ b/src/applications/conpherence/controller/ConpherenceWidgetController.php @@ -2,17 +2,6 @@ final class ConpherenceWidgetController extends ConpherenceController { - private $userPreferences; - - public function setUserPreferences(PhabricatorUserPreferences $pref) { - $this->userPreferences = $pref; - return $this; - } - - public function getUserPreferences() { - return $this->userPreferences; - } - public function shouldAllowPublic() { return true; } @@ -35,8 +24,6 @@ } $this->setConpherence($conpherence); - $this->setUserPreferences($user->loadPreferences()); - switch ($request->getStr('widget')) { case 'widgets-people': $content = $this->renderPeopleWidgetPaneContent(); @@ -143,28 +130,24 @@ ), $text); } - $default = ConpherenceSettings::EMAIL_ALWAYS; - $preference = $this->getUserPreferences(); - if ($preference) { - $default = $preference->getPreference( - PhabricatorUserPreferences::PREFERENCE_CONPH_NOTIFICATIONS, - ConpherenceSettings::EMAIL_ALWAYS); - } + $notification_key = PhabricatorConpherenceNotificationsSetting::SETTINGKEY; + $notification_default = $viewer->getUserSetting($notification_key); + $settings = $participant->getSettings(); $notifications = idx( $settings, 'notifications', - $default); + $notification_default); $options = id(new AphrontFormRadioButtonControl()) ->addButton( - ConpherenceSettings::EMAIL_ALWAYS, - ConpherenceSettings::getHumanString( - ConpherenceSettings::EMAIL_ALWAYS), + PhabricatorConpherenceNotificationsSetting::VALUE_CONPHERENCE_EMAIL, + PhabricatorConpherenceNotificationsSetting::getSettingLabel( + PhabricatorConpherenceNotificationsSetting::VALUE_CONPHERENCE_EMAIL), '') ->addButton( - ConpherenceSettings::NOTIFICATIONS_ONLY, - ConpherenceSettings::getHumanString( - ConpherenceSettings::NOTIFICATIONS_ONLY), + PhabricatorConpherenceNotificationsSetting::VALUE_CONPHERENCE_NOTIFY, + PhabricatorConpherenceNotificationsSetting::getSettingLabel( + PhabricatorConpherenceNotificationsSetting::VALUE_CONPHERENCE_NOTIFY), '') ->setName('notifications') ->setValue($notifications); diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php --- a/src/applications/conpherence/editor/ConpherenceEditor.php +++ b/src/applications/conpherence/editor/ConpherenceEditor.php @@ -541,26 +541,29 @@ $participant_phids = mpull($participants, 'getParticipantPHID'); - $preferences = id(new PhabricatorUserPreferencesQuery()) + $users = id(new PhabricatorPeopleQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->withUserPHIDs($participant_phids) + ->withPHIDs($participant_phids) + ->needUserSettings(true) ->execute(); - $preferences = mpull($preferences, null, 'getUserPHID'); + $users = mpull($users, null, 'getPHID'); + + $notification_key = PhabricatorConpherenceNotificationsSetting::SETTINGKEY; + $notification_email = + PhabricatorConpherenceNotificationsSetting::VALUE_CONPHERENCE_EMAIL; foreach ($participants as $phid => $participant) { - $default = ConpherenceSettings::EMAIL_ALWAYS; - $preference = idx($preferences, $phid); - if ($preference) { - $default = $preference->getPreference( - PhabricatorUserPreferences::PREFERENCE_CONPH_NOTIFICATIONS, - ConpherenceSettings::EMAIL_ALWAYS); + $user = idx($users, $phid); + if ($user) { + $default = $user->getUserSetting($notification_key); + } else { + $default = $notification_email; } + $settings = $participant->getSettings(); - $notifications = idx( - $settings, - 'notifications', - $default); - if ($notifications == ConpherenceSettings::EMAIL_ALWAYS) { + $notifications = idx($settings, 'notifications', $default); + + if ($notifications == $notification_email) { $to_phids[] = $phid; } } diff --git a/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php --- a/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php +++ b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php @@ -20,12 +20,16 @@ public function newValueForUsers($key, array $users) { $viewer = $this->getViewer(); + $user_phids = mpull($users, 'getPHID'); + $preferences = id(new PhabricatorUserPreferencesQuery()) ->setViewer($viewer) - ->withUserPHIDs(mpull($users, 'getPHID')) + ->withUserPHIDs($user_phids) ->execute(); - return mpull($preferences, 'getPreferences', 'getUserPHID'); + $empty = array_fill_keys($user_phids, array()); + + return mpull($preferences, 'getPreferences', 'getUserPHID') + $empty; } } diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php --- a/src/applications/people/query/PhabricatorPeopleQuery.php +++ b/src/applications/people/query/PhabricatorPeopleQuery.php @@ -23,6 +23,7 @@ private $needProfileImage; private $needAvailability; private $needBadges; + private $cacheKeys = array(); public function withIDs(array $ids) { $this->ids = $ids; @@ -119,6 +120,18 @@ return $this; } + public function needUserSettings($need) { + $cache_key = PhabricatorUserPreferencesCacheType::KEY_PREFERENCES; + + if ($need) { + $this->cacheKeys[$cache_key] = true; + } else { + unset($this->cacheKeys[$cache_key]); + } + + return $this; + } + public function newResultObject() { return new PhabricatorUser(); } @@ -238,6 +251,8 @@ } } + $this->fillUserCaches($users); + return $users; } @@ -481,4 +496,91 @@ } } + private function fillUserCaches(array $users) { + if (!$this->cacheKeys) { + return; + } + + $user_map = mpull($users, null, 'getPHID'); + $keys = array_keys($this->cacheKeys); + + $hashes = array(); + foreach ($keys as $key) { + $hashes[] = PhabricatorHash::digestForIndex($key); + } + + // First, pull any available caches. If we wanted to be particularly clever + // we could do this with JOINs in the main query. + + $cache_table = new PhabricatorUserCache(); + $cache_conn = $cache_table->establishConnection('r'); + + $cache_data = queryfx_all( + $cache_conn, + 'SELECT cacheKey, userPHID, cacheData FROM %T + WHERE cacheIndex IN (%Ls) AND userPHID IN (%Ls)', + $cache_table->getTableName(), + $hashes, + array_keys($user_map)); + + $need = array(); + + $cache_data = igroup($cache_data, 'userPHID'); + foreach ($user_map as $user_phid => $user) { + $raw_rows = idx($cache_data, $user_phid, array()); + if (!$raw_rows) { + continue; + } + $raw_data = ipull($raw_rows, 'cacheData', 'cacheKey'); + + foreach ($keys as $key) { + if (isset($raw_data[$key]) || array_key_exists($key, $raw_data)) { + continue; + } + $need[$key][$user_phid] = $user; + } + + $user->attachRawCacheData($raw_data); + } + + // If we missed any cache values, bulk-construct them now. This is + // usually much cheaper than generating them on-demand for each user + // record. + + if (!$need) { + return; + } + + $writes = array(); + foreach ($need as $cache_key => $need_users) { + $type = PhabricatorUserCacheType::getCacheTypeForKey($cache_key); + if (!$type) { + continue; + } + + $data = $type->newValueForUsers($cache_key, $need_users); + + foreach ($data as $user_phid => $value) { + $raw_value = $type->getValueForStorage($value); + $data[$user_phid] = $raw_value; + $writes[] = array( + 'userPHID' => $user_phid, + 'key' => $cache_key, + 'type' => $type, + 'value' => $raw_value, + ); + } + + foreach ($need_users as $user_phid => $user) { + if (isset($data[$user_phid]) || array_key_exists($user_phid, $data)) { + $user->attachRawCacheData( + array( + $cache_key => $data[$user_phid], + )); + } + } + } + + PhabricatorUserCache::writeCaches($writes); + } } diff --git a/src/applications/people/storage/PhabricatorUserCache.php b/src/applications/people/storage/PhabricatorUserCache.php --- a/src/applications/people/storage/PhabricatorUserCache.php +++ b/src/applications/people/storage/PhabricatorUserCache.php @@ -42,27 +42,54 @@ $key, $user_phid, $raw_value) { + self::writeCaches( + array( + array( + 'type' => $type, + 'key' => $key, + 'userPHID' => $user_phid, + 'value' => $raw_value, + ), + )); + } + public static function writeCaches(array $values) { if (PhabricatorEnv::isReadOnly()) { return; } + if (!$values) { + return; + } + $table = new self(); $conn_w = $table->establishConnection('w'); + $sql = array(); + foreach ($values as $value) { + $key = $value['key']; + + $sql[] = qsprintf( + $conn_w, + '(%s, %s, %s, %s, %s)', + $value['userPHID'], + PhabricatorHash::digestForIndex($key), + $key, + $value['value'], + $value['type']->getUserCacheType()); + } + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); - queryfx( - $conn_w, - 'INSERT INTO %T (userPHID, cacheIndex, cacheKey, cacheData, cacheType) - VALUES (%s, %s, %s, %s, %s) - ON DUPLICATE KEY UPDATE cacheData = VALUES(cacheData)', - $table->getTableName(), - $user_phid, - PhabricatorHash::digestForIndex($key), - $key, - $raw_value, - $type->getUserCacheType()); + foreach (PhabricatorLiskDAO::chunkSQL($sql) as $chunk) { + queryfx( + $conn_w, + 'INSERT INTO %T (userPHID, cacheIndex, cacheKey, cacheData, cacheType) + VALUES %Q + ON DUPLICATE KEY UPDATE cacheData = VALUES(cacheData)', + $table->getTableName(), + $chunk); + } unset($unguarded); } diff --git a/src/applications/settings/query/PhabricatorUserPreferencesTransactionQuery.php b/src/applications/settings/query/PhabricatorUserPreferencesTransactionQuery.php new file mode 100644 --- /dev/null +++ b/src/applications/settings/query/PhabricatorUserPreferencesTransactionQuery.php @@ -0,0 +1,10 @@ +<?php + +final class PhabricatorUserPreferencesTransactionQuery + extends PhabricatorApplicationTransactionQuery { + + public function getTemplateApplicationTransaction() { + return new PhabricatorUserPreferencesTransaction(); + } + +} diff --git a/src/applications/settings/setting/PhabricatorConpherenceNotificationsSetting.php b/src/applications/settings/setting/PhabricatorConpherenceNotificationsSetting.php --- a/src/applications/settings/setting/PhabricatorConpherenceNotificationsSetting.php +++ b/src/applications/settings/setting/PhabricatorConpherenceNotificationsSetting.php @@ -32,6 +32,15 @@ } protected function getSelectOptions() { + return self::getOptionsMap(); + } + + public static function getSettingLabel($key) { + $labels = self::getOptionsMap(); + return idx($labels, $key, pht('Unknown ("%s")', $key)); + } + + private static function getOptionsMap() { return array( self::VALUE_CONPHERENCE_EMAIL => pht('Send Email'), self::VALUE_CONPHERENCE_NOTIFY => pht('Send Notifications'),