recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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

epriestley 5d1359d7 360597d8

+15 -7
+4
resources/sql/autopatches/20160928.repo.messagecount.sql
··· 1 + /* Reset message counts to fix the bug in T11705 which caused some of them to 2 + become very large. */ 3 + UPDATE {$NAMESPACE}_repository.repository_statusmessage 4 + SET messageCount = 0;
+11 -7
src/applications/repository/storage/PhabricatorRepository.php
··· 1651 1651 } else { 1652 1652 // If the existing message has the same code (e.g., we just hit an 1653 1653 // error and also previously hit an error) we increment the message 1654 - // count by 1. This allows us to determine how many times in a row 1655 - // we've run into an error. 1654 + // count. This allows us to determine how many times in a row we've 1655 + // run into an error. 1656 + 1657 + // NOTE: The assignments in "ON DUPLICATE KEY UPDATE" are evaluated 1658 + // in order, so the "messageCount" assignment must occur before the 1659 + // "statusCode" assignment. See T11705. 1656 1660 1657 1661 queryfx( 1658 1662 $conn_w, ··· 1661 1665 messageCount) 1662 1666 VALUES (%d, %s, %s, %s, %d, %d) 1663 1667 ON DUPLICATE KEY UPDATE 1664 - statusCode = VALUES(statusCode), 1665 - parameters = VALUES(parameters), 1666 - epoch = VALUES(epoch), 1667 1668 messageCount = 1668 1669 IF( 1669 1670 statusCode = VALUES(statusCode), 1670 - messageCount + 1, 1671 - VALUES(messageCount))', 1671 + messageCount + VALUES(messageCount), 1672 + VALUES(messageCount)), 1673 + statusCode = VALUES(statusCode), 1674 + parameters = VALUES(parameters), 1675 + epoch = VALUES(epoch)', 1672 1676 $table_name, 1673 1677 $this->getID(), 1674 1678 $status_type,