diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 270b50d225..a72ccdbeb6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3640,6 +3640,8 @@ phutil_register_library_map(array( 'PhabricatorUserIconField' => 'applications/people/customfield/PhabricatorUserIconField.php', 'PhabricatorUserLog' => 'applications/people/storage/PhabricatorUserLog.php', 'PhabricatorUserLogView' => 'applications/people/view/PhabricatorUserLogView.php', + 'PhabricatorUserMessageCountCacheType' => 'applications/people/cache/PhabricatorUserMessageCountCacheType.php', + 'PhabricatorUserNotificationCountCacheType' => 'applications/people/cache/PhabricatorUserNotificationCountCacheType.php', 'PhabricatorUserPHIDResolver' => 'applications/phid/resolver/PhabricatorUserPHIDResolver.php', 'PhabricatorUserPreferences' => 'applications/settings/storage/PhabricatorUserPreferences.php', 'PhabricatorUserPreferencesCacheType' => 'applications/people/cache/PhabricatorUserPreferencesCacheType.php', @@ -8451,6 +8453,8 @@ phutil_register_library_map(array( 'PhabricatorPolicyInterface', ), 'PhabricatorUserLogView' => 'AphrontView', + 'PhabricatorUserMessageCountCacheType' => 'PhabricatorUserCacheType', + 'PhabricatorUserNotificationCountCacheType' => 'PhabricatorUserCacheType', 'PhabricatorUserPHIDResolver' => 'PhabricatorPHIDResolver', 'PhabricatorUserPreferences' => array( 'PhabricatorUserDAO', diff --git a/src/applications/aphlict/query/AphlictDropdownDataQuery.php b/src/applications/aphlict/query/AphlictDropdownDataQuery.php index e8a8edb59f..7a15ec7f64 100644 --- a/src/applications/aphlict/query/AphlictDropdownDataQuery.php +++ b/src/applications/aphlict/query/AphlictDropdownDataQuery.php @@ -46,17 +46,15 @@ final class AphlictDropdownDataQuery extends Phobject { $is_c_installed = PhabricatorApplication::isClassInstalledForViewer( $conpherence_app, $viewer); - $raw_message_count_number = null; - $message_count_number = null; if ($is_c_installed) { - $unread_status = ConpherenceParticipationStatus::BEHIND; - $unread = id(new ConpherenceParticipantCountQuery()) - ->withParticipantPHIDs(array($viewer->getPHID())) - ->withParticipationStatus($unread_status) - ->execute(); - $raw_message_count_number = idx($unread, $viewer->getPHID(), 0); + $raw_message_count_number = $viewer->getUnreadMessageCount(); $message_count_number = $this->formatNumber($raw_message_count_number); + } else { + $raw_message_count_number = null; + $message_count_number = null; } + + $conpherence_data = array( 'isInstalled' => $is_c_installed, 'countType' => 'messages', @@ -69,15 +67,15 @@ final class AphlictDropdownDataQuery extends Phobject { $is_n_installed = PhabricatorApplication::isClassInstalledForViewer( $notification_app, $viewer); - $notification_count_number = null; - $raw_notification_count_number = null; if ($is_n_installed) { - $raw_notification_count_number = - id(new PhabricatorFeedStoryNotification()) - ->countUnread($viewer); + $raw_notification_count_number = $viewer->getUnreadNotificationCount(); $notification_count_number = $this->formatNumber( $raw_notification_count_number); + } else { + $notification_count_number = null; + $raw_notification_count_number = null; } + $notification_data = array( 'isInstalled' => $is_n_installed, 'countType' => 'notifications', diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php index 2f01979df6..ea0fa7a295 100644 --- a/src/applications/conpherence/controller/ConpherenceViewController.php +++ b/src/applications/conpherence/controller/ConpherenceViewController.php @@ -68,9 +68,12 @@ final class ConpherenceViewController extends $latest_transaction = head($transactions); $participant = $conpherence->getParticipantIfExists($user->getPHID()); if ($participant) { - $write_guard = AphrontWriteGuard::beginScopedUnguardedWrites(); - $participant->markUpToDate($conpherence, $latest_transaction); - unset($write_guard); + if (!$participant->isUpToDate($conpherence)) { + $write_guard = AphrontWriteGuard::beginScopedUnguardedWrites(); + $participant->markUpToDate($conpherence, $latest_transaction); + $user->clearCacheData(PhabricatorUserMessageCountCacheType::KEY_COUNT); + unset($write_guard); + } } $data = ConpherenceTransactionRenderer::renderTransactions( diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php index bca70b4299..77dcf84eec 100644 --- a/src/applications/conpherence/editor/ConpherenceEditor.php +++ b/src/applications/conpherence/editor/ConpherenceEditor.php @@ -422,6 +422,10 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { $participant->save(); } + PhabricatorUserCache::clearCaches( + PhabricatorUserMessageCountCacheType::KEY_COUNT, + array_keys($participants)); + if ($xactions) { $data = array( 'type' => 'message', diff --git a/src/applications/conpherence/storage/ConpherenceParticipant.php b/src/applications/conpherence/storage/ConpherenceParticipant.php index a656dce776..32c3fb38e1 100644 --- a/src/applications/conpherence/storage/ConpherenceParticipant.php +++ b/src/applications/conpherence/storage/ConpherenceParticipant.php @@ -47,11 +47,16 @@ final class ConpherenceParticipant extends ConpherenceDAO { $this->setBehindTransactionPHID($xaction->getPHID()); $this->setSeenMessageCount($conpherence->getMessageCount()); $this->save(); + + PhabricatorUserCache::clearCache( + PhabricatorUserMessageCountCacheType::KEY_COUNT, + $this->getParticipantPHID()); } + return $this; } - private function isUpToDate(ConpherenceThread $conpherence) { + public function isUpToDate(ConpherenceThread $conpherence) { return ($this->getSeenMessageCount() == $conpherence->getMessageCount()) && diff --git a/src/applications/feed/PhabricatorFeedStoryPublisher.php b/src/applications/feed/PhabricatorFeedStoryPublisher.php index e4b62fdf29..3b59edc37b 100644 --- a/src/applications/feed/PhabricatorFeedStoryPublisher.php +++ b/src/applications/feed/PhabricatorFeedStoryPublisher.php @@ -159,7 +159,8 @@ final class PhabricatorFeedStoryPublisher extends Phobject { $will_receive_mail = array_fill_keys($this->mailRecipientPHIDs, true); - foreach (array_unique($subscribed_phids) as $user_phid) { + $user_phids = array_unique($subscribed_phids); + foreach ($user_phids as $user_phid) { if (isset($will_receive_mail[$user_phid])) { $mark_read = 1; } else { @@ -184,6 +185,10 @@ final class PhabricatorFeedStoryPublisher extends Phobject { $notif->getTableName(), implode(', ', $sql)); } + + PhabricatorUserCache::clearCaches( + PhabricatorUserNotificationCountCacheType::KEY_COUNT, + $user_phids); } private function sendNotification($chrono_key, array $subscribed_phids) { diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php index 03446d9154..5e1876acd6 100644 --- a/src/applications/files/query/PhabricatorFileQuery.php +++ b/src/applications/files/query/PhabricatorFileQuery.php @@ -134,6 +134,9 @@ final class PhabricatorFileQuery return $files; } + $viewer = $this->getViewer(); + $is_omnipotent = $viewer->isOmnipotent(); + // We need to load attached objects to perform policy checks for files. // First, load the edges. @@ -156,6 +159,13 @@ final class PhabricatorFileQuery continue; } + if ($is_omnipotent) { + // If the viewer is omnipotent, we don't need to load the associated + // objects either since they can certainly see the object. Skipping + // this can improve performance and prevent cycles. + continue; + } + foreach ($phids as $phid) { $object_phids[$phid] = true; } diff --git a/src/applications/notification/controller/PhabricatorNotificationClearController.php b/src/applications/notification/controller/PhabricatorNotificationClearController.php index 0b6027d379..a826ae0a6b 100644 --- a/src/applications/notification/controller/PhabricatorNotificationClearController.php +++ b/src/applications/notification/controller/PhabricatorNotificationClearController.php @@ -18,6 +18,10 @@ final class PhabricatorNotificationClearController $viewer->getPHID(), $chrono_key); + PhabricatorUserCache::clearCache( + PhabricatorUserNotificationCountCacheType::KEY_COUNT, + $viewer->getPHID()); + return id(new AphrontReloadResponse()) ->setURI('/notification/'); } diff --git a/src/applications/notification/controller/PhabricatorNotificationPanelController.php b/src/applications/notification/controller/PhabricatorNotificationPanelController.php index c9c3d9b94a..be7eeb914e 100644 --- a/src/applications/notification/controller/PhabricatorNotificationPanelController.php +++ b/src/applications/notification/controller/PhabricatorNotificationPanelController.php @@ -71,8 +71,7 @@ final class PhabricatorNotificationPanelController $content, $connection_ui); - $unread_count = id(new PhabricatorFeedStoryNotification()) - ->countUnread($viewer); + $unread_count = $viewer->getUnreadNotificationCount(); $json = array( 'content' => $content, diff --git a/src/applications/notification/storage/PhabricatorFeedStoryNotification.php b/src/applications/notification/storage/PhabricatorFeedStoryNotification.php index d0b4acab53..89d8db5336 100644 --- a/src/applications/notification/storage/PhabricatorFeedStoryNotification.php +++ b/src/applications/notification/storage/PhabricatorFeedStoryNotification.php @@ -60,20 +60,10 @@ final class PhabricatorFeedStoryNotification extends PhabricatorFeedDAO { $object_phid); unset($unguarded); - } - public function countUnread(PhabricatorUser $user) { - $conn = $this->establishConnection('r'); - - $data = queryfx_one( - $conn, - 'SELECT COUNT(*) as count - FROM %T - WHERE userPHID = %s AND hasViewed = 0', - $this->getTableName(), - $user->getPHID()); - - return $data['count']; + $count_key = PhabricatorUserNotificationCountCacheType::KEY_COUNT; + PhabricatorUserCache::clearCache($count_key, $user->getPHID()); + $user->clearCacheData($count_key); } } diff --git a/src/applications/people/cache/PhabricatorUserMessageCountCacheType.php b/src/applications/people/cache/PhabricatorUserMessageCountCacheType.php new file mode 100644 index 0000000000..52f53fe345 --- /dev/null +++ b/src/applications/people/cache/PhabricatorUserMessageCountCacheType.php @@ -0,0 +1,45 @@ +withParticipantPHIDs($user_phids) + ->withParticipationStatus($unread_status) + ->execute(); + + $empty = array_fill_keys($user_phids, 0); + return $unread + $empty; + } + +} diff --git a/src/applications/people/cache/PhabricatorUserNotificationCountCacheType.php b/src/applications/people/cache/PhabricatorUserNotificationCountCacheType.php new file mode 100644 index 0000000000..8eb1f464ce --- /dev/null +++ b/src/applications/people/cache/PhabricatorUserNotificationCountCacheType.php @@ -0,0 +1,50 @@ +establishConnection('r'); + + $rows = queryfx_all( + $conn_r, + 'SELECT userPHID, COUNT(*) N FROM %T + WHERE userPHID IN (%Ls) AND hasViewed = 0 + GROUP BY userPHID', + $table->getTableName(), + $user_phids); + + $empty = array_fill_keys($user_phids, 0); + return ipull($rows, 'N', 'userPHID') + $empty; + } + +} diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index ac13282923..5dcd3ea073 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -792,6 +792,16 @@ final class PhabricatorUser return $this->requireCacheData($uri_key); } + public function getUnreadNotificationCount() { + $notification_key = PhabricatorUserNotificationCountCacheType::KEY_COUNT; + return $this->requireCacheData($notification_key); + } + + public function getUnreadMessageCount() { + $message_key = PhabricatorUserMessageCountCacheType::KEY_COUNT; + return $this->requireCacheData($message_key); + } + public function getFullName() { if (strlen($this->getRealName())) { return $this->getUsername().' ('.$this->getRealName().')'; diff --git a/src/applications/people/storage/PhabricatorUserCache.php b/src/applications/people/storage/PhabricatorUserCache.php index 9bd1a9d2cb..c74b583923 100644 --- a/src/applications/people/storage/PhabricatorUserCache.php +++ b/src/applications/people/storage/PhabricatorUserCache.php @@ -97,10 +97,18 @@ final class PhabricatorUserCache extends PhabricatorUserDAO { } public static function clearCache($key, $user_phid) { + return self::clearCaches($key, array($user_phid)); + } + + public static function clearCaches($key, array $user_phids) { if (PhabricatorEnv::isReadOnly()) { return; } + if (!$user_phids) { + return; + } + $table = new self(); $conn_w = $table->establishConnection('w'); @@ -108,15 +116,14 @@ final class PhabricatorUserCache extends PhabricatorUserDAO { queryfx( $conn_w, - 'DELETE FROM %T WHERE cacheIndex = %s AND userPHID = %s', + 'DELETE FROM %T WHERE cacheIndex = %s AND userPHID IN (%Ls)', $table->getTableName(), PhabricatorHash::digestForIndex($key), - $user_phid); + $user_phids); unset($unguarded); } - public static function clearCacheForAllUsers($key) { if (PhabricatorEnv::isReadOnly()) { return; diff --git a/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php b/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php index 918142f95c..cfb7fe105f 100644 --- a/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php +++ b/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php @@ -162,7 +162,6 @@ final class PhabricatorUserPreferencesEditor PhabricatorUserPreferencesCacheType::KEY_PREFERENCES); } - return $xactions; } diff --git a/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php b/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php index 0369fbff54..8fbdc19ad6 100644 --- a/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php +++ b/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php @@ -84,8 +84,10 @@ final class PhabricatorApplicationTransactionPublishWorker $xaction_phids = idx($data, 'xactionPHIDs'); if (!$xaction_phids) { - throw new PhabricatorWorkerPermanentFailureException( - pht('Task has no transaction PHIDs!')); + // It's okay if we don't have any transactions. This can happen when + // creating objects or performing no-op updates. We will still apply + // meaningful side effects like updating search engine indexes. + return array(); } $viewer = PhabricatorUser::getOmnipotentUser();