From d254c1f8b14e8ba4fc41f4aabdf67c25f7096a2b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 28 Jan 2019 11:29:00 -0800
Subject: [PATCH] Use "null", not "-1", as a local "no version" marker when
 performing intracluster repository sync

Summary:
Ref T13242. See <https://discourse.phabricator-community.org/t/out-of-range-value-for-column-deviceversion/2218>.

The synchronization log column is `uint32?` and `-1` doesn't go into that column.

Since we're only using `-1` for convenience to cheat through `$max_version > $this_version` checks, use `null` instead and make the checks more explicit.

Test Plan: Reproducing this is a bit tricky and I cheated fairly heavily to force the code down this pathway without actually building a multi-device cluster, but I did reproduce the original exception, apply the patch, and observe that it fixed things.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13242

Differential Revision: https://secure.phabricator.com/D20047
---
 .../protocol/DiffusionRepositoryClusterEngine.php        | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php b/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php
index c72021f0c1..717e730ab1 100644
--- a/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php
+++ b/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php
@@ -188,7 +188,7 @@ final class DiffusionRepositoryClusterEngine extends Phobject {
     if ($this_version) {
       $this_version = (int)$this_version->getRepositoryVersion();
     } else {
-      $this_version = -1;
+      $this_version = null;
     }
 
     if ($versions) {
@@ -197,7 +197,7 @@ final class DiffusionRepositoryClusterEngine extends Phobject {
       // leader, we want to fetch from a leader and then update our version.
 
       $max_version = (int)max(mpull($versions, 'getRepositoryVersion'));
-      if ($max_version > $this_version) {
+      if (($this_version === null) || ($max_version > $this_version)) {
         if ($repository->isHosted()) {
           $fetchable = array();
           foreach ($versions as $version) {
@@ -206,6 +206,7 @@ final class DiffusionRepositoryClusterEngine extends Phobject {
             }
           }
 
+
           $this->synchronizeWorkingCopyFromDevices(
             $fetchable,
             $this_version,
@@ -445,10 +446,10 @@ final class DiffusionRepositoryClusterEngine extends Phobject {
     if ($this_version) {
       $this_version = (int)$this_version->getRepositoryVersion();
     } else {
-      $this_version = -1;
+      $this_version = null;
     }
 
-    if ($new_version > $this_version) {
+    if (($this_version === null) || ($new_version > $this_version)) {
       PhabricatorRepositoryWorkingCopyVersion::updateVersion(
         $repository_phid,
         $device_phid,