Cache user notification and message counts

Summary:
Ref T4103. Ref T10078. This puts a user cache in front of notification and message counts.

This reduces the number of queries issued on every page by 4 (2x building the menu, 2x building Quicksand data).

Also fixes some minor issues:

  - Daemons could choke on sending mail in the user's translation.
  - No-op object updates could fail in the daemons.
  - Questionable data access pattern in the file query coming out of the profile file cache.

Test Plan:
  - Sent myself notifications. Saw count go up.
  - Cleared them by visiting objects and clearing all notifications. Saw count go down.
  - Sent myself messages. Saw count go up.
  - Cleared them by visiting threads. Saw count go down.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4103, T10078

Differential Revision: https://secure.phabricator.com/D16041

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',

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;
+      $raw_message_count_number = $viewer->getUnreadMessageCount();
-      $unread = id(new ConpherenceParticipantCountQuery())
-        ->withParticipantPHIDs(array($viewer->getPHID()))
-        ->withParticipationStatus($unread_status)
-        ->execute();
-      $raw_message_count_number = idx($unread, $viewer->getPHID(), 0);
       $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 =
+      $raw_notification_count_number = $viewer->getUnreadNotificationCount();
-        id(new PhabricatorFeedStoryNotification())
-        ->countUnread($viewer);
       $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',

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();
+      if (!$participant->isUpToDate($conpherence)) {
-      $participant->markUpToDate($conpherence, $latest_transaction);
+        $write_guard = AphrontWriteGuard::beginScopedUnguardedWrites();
-      unset($write_guard);
+        $participant->markUpToDate($conpherence, $latest_transaction);
+        $user->clearCacheData(PhabricatorUserMessageCountCacheType::KEY_COUNT);
+        unset($write_guard);
+      }
     }
 
     $data = ConpherenceTransactionRenderer::renderTransactions(

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',

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())
         &&

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) {

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;
       }

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/');
     }

src/applications/notification/controller/PhabricatorNotificationPanelController.php

@@ -71,8 +71,7 @@ final class PhabricatorNotificationPanelController
       $content,
       $connection_ui);
 
-    $unread_count = id(new PhabricatorFeedStoryNotification())
+    $unread_count = $viewer->getUnreadNotificationCount();
-      ->countUnread($viewer);
 
     $json = array(
       'content' => $content,

src/applications/notification/storage/PhabricatorFeedStoryNotification.php

@@ -60,20 +60,10 @@ final class PhabricatorFeedStoryNotification extends PhabricatorFeedDAO {
       $object_phid);
 
     unset($unguarded);
-  }
 
-  public function countUnread(PhabricatorUser $user) {
+    $count_key = PhabricatorUserNotificationCountCacheType::KEY_COUNT;
-    $conn = $this->establishConnection('r');
+    PhabricatorUserCache::clearCache($count_key, $user->getPHID());
-
+    $user->clearCacheData($count_key);
-    $data = queryfx_one(
-      $conn,
-      'SELECT COUNT(*) as count
-       FROM %T
-       WHERE userPHID = %s AND hasViewed = 0',
-      $this->getTableName(),
-      $user->getPHID());
-
-    return $data['count'];
   }
 
 }

src/applications/people/cache/PhabricatorUserMessageCountCacheType.php

@@ -0,0 +1,45 @@
+<?php
+
+final class PhabricatorUserMessageCountCacheType
+  extends PhabricatorUserCacheType {
+
+  const CACHETYPE = 'message.count';
+
+  const KEY_COUNT = 'user.message.count.v1';
+
+  public function getAutoloadKeys() {
+    return array(
+      self::KEY_COUNT,
+    );
+  }
+
+  public function canManageKey($key) {
+    return ($key === self::KEY_COUNT);
+  }
+
+  public function getValueFromStorage($value) {
+    return (int)$value;
+  }
+
+  public function getValueForStorage($value) {
+    return $value;
+  }
+
+  public function newValueForUsers($key, array $users) {
+    if (!$users) {
+      return array();
+    }
+
+    $user_phids = mpull($users, 'getPHID');
+
+    $unread_status = ConpherenceParticipationStatus::BEHIND;
+    $unread = id(new ConpherenceParticipantCountQuery())
+      ->withParticipantPHIDs($user_phids)
+      ->withParticipationStatus($unread_status)
+      ->execute();
+
+    $empty = array_fill_keys($user_phids, 0);
+    return $unread + $empty;
+  }
+
+}

src/applications/people/cache/PhabricatorUserNotificationCountCacheType.php

@@ -0,0 +1,50 @@
+<?php
+
+final class PhabricatorUserNotificationCountCacheType
+  extends PhabricatorUserCacheType {
+
+  const CACHETYPE = 'notification.count';
+
+  const KEY_COUNT = 'user.notification.count.v1';
+
+  public function getAutoloadKeys() {
+    return array(
+      self::KEY_COUNT,
+    );
+  }
+
+  public function canManageKey($key) {
+    return ($key === self::KEY_COUNT);
+  }
+
+  public function getValueFromStorage($value) {
+    return (int)$value;
+  }
+
+  public function getValueForStorage($value) {
+    return $value;
+  }
+
+  public function newValueForUsers($key, array $users) {
+    if (!$users) {
+      return array();
+    }
+
+    $user_phids = mpull($users, 'getPHID');
+
+    $table = new PhabricatorFeedStoryNotification();
+    $conn_r = $table->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;
+  }
+
+}

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().')';

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;

src/applications/settings/editor/PhabricatorUserPreferencesEditor.php

@@ -162,7 +162,6 @@ final class PhabricatorUserPreferencesEditor
         PhabricatorUserPreferencesCacheType::KEY_PREFERENCES);
     }
 
-
     return $xactions;
   }
 

src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php

@@ -84,8 +84,10 @@ final class PhabricatorApplicationTransactionPublishWorker
 
     $xaction_phids = idx($data, 'xactionPHIDs');
     if (!$xaction_phids) {
-      throw new PhabricatorWorkerPermanentFailureException(
+      // It's okay if we don't have any transactions. This can happen when
-        pht('Task has no transaction PHIDs!'));
+      // creating objects or performing no-op updates. We will still apply
+      // meaningful side effects like updating search engine indexes.
+      return array();
     }
 
     $viewer = PhabricatorUser::getOmnipotentUser();