From 921aab604ec759f2af997faf9b3822ce0e4ee340 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 28 Sep 2016 14:49:30 -0700
Subject: [PATCH] (stable) Fix an issue where repository message counts would
 never reset

Summary:
Fixes T11705. I did not realize that `ON DUPLICATE KEY UPDATE` was order-dependent, so the "reset" clause of this `IF(...)` never actually worked.

Reorder it so we check if we're changing the message type //first//, then actually change the message type.

This makes the count reset properly when a failing repository succeeds, or a working repository fails.

Test Plan:
  - On `master`, forced a working repository to fail a `bin/repository update`, saw the message change types (expected) but keep the old count (wrong!).
  - With this patch, repeated the process and saw the count reset properly.
  - Ran the patch, verified counts reset to 0.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11705

Differential Revision: https://secure.phabricator.com/D16623
---
 .../autopatches/20160928.repo.messagecount.sql |  4 ++++
 .../storage/PhabricatorRepository.php          | 18 +++++++++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)
 create mode 100644 resources/sql/autopatches/20160928.repo.messagecount.sql

diff --git a/resources/sql/autopatches/20160928.repo.messagecount.sql b/resources/sql/autopatches/20160928.repo.messagecount.sql
new file mode 100644
index 0000000000..e02b257c72
--- /dev/null
+++ b/resources/sql/autopatches/20160928.repo.messagecount.sql
@@ -0,0 +1,4 @@
+/* Reset message counts to fix the bug in T11705 which caused some of them to
+   become very large. */
+UPDATE {$NAMESPACE}_repository.repository_statusmessage
+  SET messageCount = 0;
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
index ee8e0ad54d..c27f562864 100644
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -1651,8 +1651,12 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     } else {
       // If the existing message has the same code (e.g., we just hit an
       // error and also previously hit an error) we increment the message
-      // count by 1. This allows us to determine how many times in a row
-      // we've run into an error.
+      // count. This allows us to determine how many times in a row we've
+      // run into an error.
+
+      // NOTE: The assignments in "ON DUPLICATE KEY UPDATE" are evaluated
+      // in order, so the "messageCount" assignment must occur before the
+      // "statusCode" assignment. See T11705.
 
       queryfx(
         $conn_w,
@@ -1661,14 +1665,14 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
             messageCount)
           VALUES (%d, %s, %s, %s, %d, %d)
           ON DUPLICATE KEY UPDATE
-            statusCode = VALUES(statusCode),
-            parameters = VALUES(parameters),
-            epoch = VALUES(epoch),
             messageCount =
               IF(
                 statusCode = VALUES(statusCode),
-                messageCount + 1,
-                VALUES(messageCount))',
+                messageCount + VALUES(messageCount),
+                VALUES(messageCount)),
+            statusCode = VALUES(statusCode),
+            parameters = VALUES(parameters),
+            epoch = VALUES(epoch)',
         $table_name,
         $this->getID(),
         $status_type,