From fd367adbaf85f0fcb4b80c830f829ae61d30de68 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 5 Mar 2018 14:07:47 -0800
Subject: [PATCH] Parameterize PhabricatorGlobalLock

Summary:
Ref T13096. Currently, we do a fair amount of clever digesting and string manipulation to build lock names which are less than 64 characters long while still being reasonably readable.

Instead, do more of this automatically. This will let lock acquisition become simpler and make it more possible to build a useful lock log.

Test Plan: Ran `bin/repository update`, saw a reasonable lock acquire and release.

Maniphest Tasks: T13096

Differential Revision: https://secure.phabricator.com/D19173
---
 .../engine/PhabricatorRepositoryEngine.php    | 11 +++---
 .../util/PhabricatorGlobalLock.php            | 38 +++++++++++++------
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/src/applications/repository/engine/PhabricatorRepositoryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryEngine.php
index 244ed0cd45..fed5b09521 100644
--- a/src/applications/repository/engine/PhabricatorRepositoryEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryEngine.php
@@ -56,19 +56,18 @@ abstract class PhabricatorRepositoryEngine extends Phobject {
     $lock_key,
     $lock_device_only) {
 
-    $lock_parts = array();
-    $lock_parts[] = $lock_key;
-    $lock_parts[] = $repository->getID();
+    $lock_parts = array(
+      'repositoryPHID' => $repository->getPHID(),
+    );
 
     if ($lock_device_only) {
       $device = AlmanacKeys::getLiveDevice();
       if ($device) {
-        $lock_parts[] = $device->getID();
+        $lock_parts['devicePHID'] = $device->getPHID();
       }
     }
 
-    $lock_name = implode(':', $lock_parts);
-    return PhabricatorGlobalLock::newLock($lock_name);
+    return PhabricatorGlobalLock::newLock($lock_key, $lock_parts);
   }
 
 
diff --git a/src/infrastructure/util/PhabricatorGlobalLock.php b/src/infrastructure/util/PhabricatorGlobalLock.php
index 26e11d1899..8aecb40873 100644
--- a/src/infrastructure/util/PhabricatorGlobalLock.php
+++ b/src/infrastructure/util/PhabricatorGlobalLock.php
@@ -28,6 +28,7 @@
  */
 final class PhabricatorGlobalLock extends PhutilLock {
 
+  private $parameters;
   private $conn;
   private $isExternalConnection = false;
 
@@ -37,27 +38,42 @@ final class PhabricatorGlobalLock extends PhutilLock {
 /* -(  Constructing Locks  )------------------------------------------------- */
 
 
-  public static function newLock($name) {
+  public static function newLock($name, $parameters = array()) {
     $namespace = PhabricatorLiskDAO::getStorageNamespace();
     $namespace = PhabricatorHash::digestToLength($namespace, 20);
 
-    $full_name = 'ph:'.$namespace.':'.$name;
+    $parts = array();
+    ksort($parameters);
+    foreach ($parameters as $key => $parameter) {
+      if (!preg_match('/^[a-zA-Z0-9]+\z/', $key)) {
+        throw new Exception(
+          pht(
+            'Lock parameter key "%s" must be alphanumeric.',
+            $key));
+      }
 
-    $length_limit = 64;
-    if (strlen($full_name) > $length_limit) {
-      throw new Exception(
-        pht(
-          'Lock name "%s" is too long (full lock name is "%s"). The '.
-          'full lock name must not be longer than %s bytes.',
-          $name,
-          $full_name,
-          new PhutilNumber($length_limit)));
+      if (!is_scalar($parameter) && !is_null($parameter)) {
+        throw new Exception(
+          pht(
+            'Lock parameter for key "%s" must be a scalar.',
+            $key));
+      }
+
+      $value = phutil_json_encode($parameter);
+      $parts[] = "{$key}={$value}";
     }
+    $parts = implode(', ', $parts);
 
+    $local = "{$name}({$parts})";
+    $local = PhabricatorHash::digestToLength($local, 20);
+
+    $full_name = "ph:{$namespace}:{$local}";
     $lock = self::getLock($full_name);
     if (!$lock) {
       $lock = new PhabricatorGlobalLock($full_name);
       self::registerLock($lock);
+
+      $lock->parameters = $parameters;
     }
 
     return $lock;