Make losing client ID optional in bulk transfers

When executing bulk domain transfers with an explicit list of domain names or
a domain names file, enforcing by losing registrar ID is often unnecessary
and redundant (b/537294004).

This commit makes --losing_registrar_id an optional command-line parameter
and updates BulkDomainTransferAction and BatchModule to handle an optional
losing sponsor ID. Existing behavior is preserved when the parameter is
explicitly supplied.

Author: CydeWeys

core/src/main/java/google/registry/batch/BatchModule.java

@@ -169,8 +169,8 @@ static String provideGainingRegistrarId(HttpServletRequest req) {
 
   @Provides
   @Parameter("losingRegistrarId")
-  static String provideLosingRegistrarId(HttpServletRequest req) {
-    return extractRequiredParameter(req, "losingRegistrarId");
+  static Optional<String> provideLosingRegistrarId(HttpServletRequest req) {
+    return extractOptionalParameter(req, "losingRegistrarId");
   }
 
   @Provides

core/src/main/java/google/registry/batch/BulkDomainTransferAction.java

@@ -109,7 +109,7 @@ public class BulkDomainTransferAction implements Runnable {
   private final RateLimiter rateLimiter;
   private final ImmutableList<String> bulkTransferDomainNames;
   private final String gainingRegistrarId;
-  private final String losingRegistrarId;
+  private final Optional<String> losingRegistrarId;
   private final boolean requestedByRegistrar;
   private final String reason;
   private final Response response;
@@ -127,7 +127,7 @@ public class BulkDomainTransferAction implements Runnable {
       @Named("standardRateLimiter") RateLimiter rateLimiter,
       @Parameter("bulkTransferDomainNames") ImmutableList<String> bulkTransferDomainNames,
       @Parameter("gainingRegistrarId") String gainingRegistrarId,
-      @Parameter("losingRegistrarId") String losingRegistrarId,
+      @Parameter("losingRegistrarId") Optional<String> losingRegistrarId,
       @Parameter("requestedByRegistrar") boolean requestedByRegistrar,
       @Parameter("reason") String reason,
       Response response) {
@@ -225,7 +225,7 @@ private boolean shouldSkipDomain(String domainName) {
       alreadyTransferred++;
       return true;
     }
-    if (!currentRegistrarId.equals(losingRegistrarId)) {
+    if (losingRegistrarId.isPresent() && !currentRegistrarId.equals(losingRegistrarId.get())) {
       logger.atWarning().log(
           "Domain '%s' had unexpected registrar '%s'", domainName, currentRegistrarId);
       errors++;

core/src/main/java/google/registry/tools/BulkDomainTransferCommand.java

@@ -76,8 +76,7 @@ public class BulkDomainTransferCommand extends ConfirmingCommand implements Comm
 
   @Parameter(
       names = {"-l", "--losing_registrar_id"},
-      description = "The ID of the registrar from which domains should be transferred",
-      required = true)
+      description = "The ID of the registrar from which domains should be transferred")
   private String losingRegistrarId;
 
   @Parameter(
@@ -118,14 +117,18 @@ protected String execute() throws Exception {
         Registrar.loadByRegistrarIdCached(gainingRegistrarId).isPresent(),
         "Gaining registrar %s doesn't exist",
         gainingRegistrarId);
-    checkArgument(
-        Registrar.loadByRegistrarIdCached(losingRegistrarId).isPresent(),
-        "Losing registrar %s doesn't exist",
-        losingRegistrarId);
+    if (losingRegistrarId != null) {
+      checkArgument(
+          Registrar.loadByRegistrarIdCached(losingRegistrarId).isPresent(),
+          "Losing registrar %s doesn't exist",
+          losingRegistrarId);
+    }
 
     ImmutableMap.Builder<String, Object> paramsBuilder = new ImmutableMap.Builder<>();
     paramsBuilder.put("gainingRegistrarId", gainingRegistrarId);
-    paramsBuilder.put("losingRegistrarId", losingRegistrarId);
+    if (losingRegistrarId != null) {
+      paramsBuilder.put("losingRegistrarId", losingRegistrarId);
+    }
     paramsBuilder.put("requestedByRegistrar", requestedByRegistrar);
     paramsBuilder.put("reason", reason);
     if (maxQps > 0) {

core/src/test/java/google/registry/batch/BulkDomainTransferActionTest.java

@@ -39,6 +39,7 @@
 import google.registry.testing.FakeLockHandler;
 import google.registry.testing.FakeResponse;
 import java.time.Instant;
+import java.util.Optional;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -127,7 +128,22 @@ void testSuccess_normalRun() {
     assertThat(deletedDomain.getUpdateTimestamp().getTimestamp()).isEqualTo(preRunTime);
   }
 
-  private BulkDomainTransferAction createAction(String... domains) {
+  @Test
+  void testSuccess_withoutLosingRegistrarId() {
+    BulkDomainTransferAction action =
+        createActionWithOptionalLosingRegistrar(
+            Optional.empty(), "active.tld", "alreadytransferred.tld");
+    fakeClock.advanceOneMilli();
+    Instant now = fakeClock.now();
+    action.run();
+    assertThat(response.getStatus()).isEqualTo(200);
+    activeDomain = loadByEntity(activeDomain);
+    assertThat(activeDomain.cloneProjectedAtTime(now).getCurrentSponsorRegistrarId())
+        .isEqualTo("NewRegistrar");
+  }
+
+  private BulkDomainTransferAction createActionWithOptionalLosingRegistrar(
+      Optional<String> losingRegistrarId, String... domains) {
     EppController eppController =
         DaggerEppTestComponent.builder()
             .fakesAndMocksModule(FakesAndMocksModule.create(new FakeClock()))
@@ -140,9 +156,13 @@ private BulkDomainTransferAction createAction(String... domains) {
         rateLimiter,
         ImmutableList.copyOf(domains),
         "NewRegistrar",
-        "TheRegistrar",
+        losingRegistrarId,
         true,
         "reason",
         response);
   }
+
+  private BulkDomainTransferAction createAction(String... domains) {
+    return createActionWithOptionalLosingRegistrar(Optional.of("TheRegistrar"), domains);
+  }
 }

core/src/test/java/google/registry/tools/BulkDomainTransferCommandTest.java

@@ -165,4 +165,24 @@ void testFailure_bothDomainMethodsSpecified() {
         .isEqualTo(
             "Must specify exactly one input method, either --domains or --domain_names_file");
   }
+
+  @Test
+  void testSuccess_noLosingRegistrarId() throws Exception {
+    runCommandForced(
+        "--gaining_registrar_id", "NewRegistrar",
+        "--reason", "someReason",
+        "--domains", "foo.tld,bar.tld");
+    verify(connection)
+        .sendPostRequest(
+            "/_dr/task/bulkDomainTransfer",
+            ImmutableMap.of(
+                "gainingRegistrarId",
+                "NewRegistrar",
+                "requestedByRegistrar",
+                false,
+                "reason",
+                "someReason"),
+            MediaType.PLAIN_TEXT_UTF_8,
+            "[\"foo.tld\",\"bar.tld\"]".getBytes(UTF_8));
+  }
 }