From 06f0ec4f2f0648a6f5ed4b2c1cb8a3c3cf907771 Mon Sep 17 00:00:00 2001
From: guyben <guyben@google.com>
Date: Wed, 18 Oct 2017 14:57:45 -0700
Subject: [PATCH] Remove reference to TLD in Locks' ResourceName

The TLD is added separately to the lock (the lock is unique per
TLD-ResourceName pair), so there's no need to add it to the resourceName.

The current status is that the TLD was included twice in the lockId. After the fix - it'll only be included once.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=172659986
---
 java/google/registry/rde/EscrowTaskRunner.java         | 10 ++++------
 java/google/registry/rde/RdeStagingReducer.java        |  4 ++--
 .../google/registry/rde/EscrowTaskRunnerTest.java      |  6 ++----
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/java/google/registry/rde/EscrowTaskRunner.java b/java/google/registry/rde/EscrowTaskRunner.java
index 62725f2ee..5712549f6 100644
--- a/java/google/registry/rde/EscrowTaskRunner.java
+++ b/java/google/registry/rde/EscrowTaskRunner.java
@@ -22,8 +22,6 @@ import google.registry.model.common.Cursor.CursorType;
 import google.registry.model.registry.Registry;
 import google.registry.request.HttpException.NoContentException;
 import google.registry.request.HttpException.ServiceUnavailableException;
-import google.registry.request.Parameter;
-import google.registry.request.RequestParameters;
 import google.registry.request.lock.LockHandler;
 import google.registry.util.Clock;
 import google.registry.util.FormattingLogger;
@@ -70,7 +68,6 @@ class EscrowTaskRunner {
   private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
 
   @Inject Clock clock;
-  @Inject @Parameter(RequestParameters.PARAM_TLD) String tld;
   @Inject LockHandler lockHandler;
   @Inject EscrowTaskRunner() {}
 
@@ -113,12 +110,13 @@ class EscrowTaskRunner {
                   });
           return null;
         };
-    String lockName = String.format("%s %s", task.getClass().getSimpleName(), registry.getTld());
-    if (!lockHandler.executeWithLocks(lockRunner, tld, timeout, lockName)) {
+    String lockName = String.format("EscrowTaskRunner %s", task.getClass().getSimpleName());
+    if (!lockHandler.executeWithLocks(lockRunner, registry.getTldStr(), timeout, lockName)) {
       // This will happen if either: a) the task is double-executed; b) the task takes a long time
       // to run and the retry task got executed while the first one is still running. In both
       // situations the safest thing to do is to just return 503 so the task gets retried later.
-      throw new ServiceUnavailableException("Lock in use: " + lockName);
+      throw new ServiceUnavailableException(
+          String.format("Lock in use: %s for TLD: %s", lockName, registry.getTldStr()));
     }
   }
 }
diff --git a/java/google/registry/rde/RdeStagingReducer.java b/java/google/registry/rde/RdeStagingReducer.java
index 4f653268a..c0ac4ca9f 100644
--- a/java/google/registry/rde/RdeStagingReducer.java
+++ b/java/google/registry/rde/RdeStagingReducer.java
@@ -107,8 +107,8 @@ public final class RdeStagingReducer extends Reducer<PendingDeposit, DepositFrag
           reduceWithLock(key, fragments);
           return null;
         };
-    String lockName = String.format("RdeStaging %s %s", key.tld(), key.mode());
-    if (!lockHandler.executeWithLocks(lockRunner, null, lockTimeout, lockName)) {
+    String lockName = String.format("RdeStaging %s", key.mode());
+    if (!lockHandler.executeWithLocks(lockRunner, key.tld(), lockTimeout, lockName)) {
       logger.warningfmt("Lock in use: %s", lockName);
     }
   }
diff --git a/javatests/google/registry/rde/EscrowTaskRunnerTest.java b/javatests/google/registry/rde/EscrowTaskRunnerTest.java
index 753ae6c62..d292ba924 100644
--- a/javatests/google/registry/rde/EscrowTaskRunnerTest.java
+++ b/javatests/google/registry/rde/EscrowTaskRunnerTest.java
@@ -15,7 +15,6 @@
 package google.registry.rde;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth8.assertThat;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.persistResource;
@@ -67,7 +66,6 @@ public class EscrowTaskRunnerTest {
     registry = Registry.get("lol");
     runner = new EscrowTaskRunner();
     runner.clock = clock;
-    runner.tld = "lol";
     runner.lockHandler = new FakeLockHandler(true);
     DateTimeZone.setDefault(DateTimeZone.forID("America/New_York"));  // Make sure UTC stuff works.
   }
@@ -108,11 +106,11 @@ public class EscrowTaskRunnerTest {
 
   @Test
   public void testRun_lockIsntAvailable_throws503() throws Exception {
-    String lockName = task.getClass().getSimpleName() + " lol";
+    String lockName = "EscrowTaskRunner " + task.getClass().getSimpleName();
     clock.setTo(DateTime.parse("2006-06-06T00:30:00Z"));
     persistResource(
         Cursor.create(CursorType.RDE_STAGING, DateTime.parse("2006-06-06TZ"), registry));
-    thrown.expect(ServiceUnavailableException.class, "Lock in use: " + lockName);
+    thrown.expect(ServiceUnavailableException.class, "Lock in use: " + lockName + " for TLD: lol");
     runner.lockHandler = new FakeLockHandler(false);
     runner.lockRunAndRollForward(
         task, registry, standardSeconds(30), CursorType.RDE_STAGING, standardDays(1));