Skip to content

Commit d3c8449

Browse files
committed
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. BUG= http://b/537294004
1 parent a2f0035 commit d3c8449

5 files changed

Lines changed: 57 additions & 14 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,8 @@ static String provideGainingRegistrarId(HttpServletRequest req) {
169169

170170
@Provides
171171
@Parameter("losingRegistrarId")
172-
static String provideLosingRegistrarId(HttpServletRequest req) {
173-
return extractRequiredParameter(req, "losingRegistrarId");
172+
static Optional<String> provideLosingRegistrarId(HttpServletRequest req) {
173+
return extractOptionalParameter(req, "losingRegistrarId");
174174
}
175175

176176
@Provides

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public class BulkDomainTransferAction implements Runnable {
109109
private final RateLimiter rateLimiter;
110110
private final ImmutableList<String> bulkTransferDomainNames;
111111
private final String gainingRegistrarId;
112-
private final String losingRegistrarId;
112+
private final Optional<String> losingRegistrarId;
113113
private final boolean requestedByRegistrar;
114114
private final String reason;
115115
private final Response response;
@@ -127,7 +127,7 @@ public class BulkDomainTransferAction implements Runnable {
127127
@Named("standardRateLimiter") RateLimiter rateLimiter,
128128
@Parameter("bulkTransferDomainNames") ImmutableList<String> bulkTransferDomainNames,
129129
@Parameter("gainingRegistrarId") String gainingRegistrarId,
130-
@Parameter("losingRegistrarId") String losingRegistrarId,
130+
@Parameter("losingRegistrarId") Optional<String> losingRegistrarId,
131131
@Parameter("requestedByRegistrar") boolean requestedByRegistrar,
132132
@Parameter("reason") String reason,
133133
Response response) {
@@ -225,7 +225,7 @@ private boolean shouldSkipDomain(String domainName) {
225225
alreadyTransferred++;
226226
return true;
227227
}
228-
if (!currentRegistrarId.equals(losingRegistrarId)) {
228+
if (losingRegistrarId.isPresent() && !currentRegistrarId.equals(losingRegistrarId.get())) {
229229
logger.atWarning().log(
230230
"Domain '%s' had unexpected registrar '%s'", domainName, currentRegistrarId);
231231
errors++;

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.io.File;
3434
import java.io.IOException;
3535
import java.util.List;
36+
import java.util.Optional;
3637

3738
/**
3839
* A command to bulk-transfer any number of domains from one registrar to another.
@@ -76,8 +77,7 @@ public class BulkDomainTransferCommand extends ConfirmingCommand implements Comm
7677

7778
@Parameter(
7879
names = {"-l", "--losing_registrar_id"},
79-
description = "The ID of the registrar from which domains should be transferred",
80-
required = true)
80+
description = "The ID of the registrar from which domains should be transferred")
8181
private String losingRegistrarId;
8282

8383
@Parameter(
@@ -118,14 +118,17 @@ protected String execute() throws Exception {
118118
Registrar.loadByRegistrarIdCached(gainingRegistrarId).isPresent(),
119119
"Gaining registrar %s doesn't exist",
120120
gainingRegistrarId);
121-
checkArgument(
122-
Registrar.loadByRegistrarIdCached(losingRegistrarId).isPresent(),
123-
"Losing registrar %s doesn't exist",
124-
losingRegistrarId);
121+
if (losingRegistrarId != null) {
122+
checkArgument(
123+
Registrar.loadByRegistrarIdCached(losingRegistrarId).isPresent(),
124+
"Losing registrar %s doesn't exist",
125+
losingRegistrarId);
126+
}
125127

126128
ImmutableMap.Builder<String, Object> paramsBuilder = new ImmutableMap.Builder<>();
127129
paramsBuilder.put("gainingRegistrarId", gainingRegistrarId);
128-
paramsBuilder.put("losingRegistrarId", losingRegistrarId);
130+
Optional.ofNullable(losingRegistrarId)
131+
.ifPresent(id -> paramsBuilder.put("losingRegistrarId", id));
129132
paramsBuilder.put("requestedByRegistrar", requestedByRegistrar);
130133
paramsBuilder.put("reason", reason);
131134
if (maxQps > 0) {

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

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import google.registry.testing.FakeLockHandler;
4040
import google.registry.testing.FakeResponse;
4141
import java.time.Instant;
42+
import java.util.Optional;
4243
import org.junit.jupiter.api.BeforeEach;
4344
import org.junit.jupiter.api.Test;
4445
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -127,7 +128,22 @@ void testSuccess_normalRun() {
127128
assertThat(deletedDomain.getUpdateTimestamp().getTimestamp()).isEqualTo(preRunTime);
128129
}
129130

130-
private BulkDomainTransferAction createAction(String... domains) {
131+
@Test
132+
void testSuccess_withoutLosingRegistrarId() {
133+
BulkDomainTransferAction action =
134+
createActionWithOptionalLosingRegistrar(
135+
Optional.empty(), "active.tld", "alreadytransferred.tld");
136+
fakeClock.advanceOneMilli();
137+
Instant now = fakeClock.now();
138+
action.run();
139+
assertThat(response.getStatus()).isEqualTo(200);
140+
activeDomain = loadByEntity(activeDomain);
141+
assertThat(activeDomain.cloneProjectedAtTime(now).getCurrentSponsorRegistrarId())
142+
.isEqualTo("NewRegistrar");
143+
}
144+
145+
private BulkDomainTransferAction createActionWithOptionalLosingRegistrar(
146+
Optional<String> losingRegistrarId, String... domains) {
131147
EppController eppController =
132148
DaggerEppTestComponent.builder()
133149
.fakesAndMocksModule(FakesAndMocksModule.create(new FakeClock()))
@@ -140,9 +156,13 @@ private BulkDomainTransferAction createAction(String... domains) {
140156
rateLimiter,
141157
ImmutableList.copyOf(domains),
142158
"NewRegistrar",
143-
"TheRegistrar",
159+
losingRegistrarId,
144160
true,
145161
"reason",
146162
response);
147163
}
164+
165+
private BulkDomainTransferAction createAction(String... domains) {
166+
return createActionWithOptionalLosingRegistrar(Optional.of("TheRegistrar"), domains);
167+
}
148168
}

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,4 +165,24 @@ void testFailure_bothDomainMethodsSpecified() {
165165
.isEqualTo(
166166
"Must specify exactly one input method, either --domains or --domain_names_file");
167167
}
168+
169+
@Test
170+
void testSuccess_noLosingRegistrarId() throws Exception {
171+
runCommandForced(
172+
"--gaining_registrar_id", "NewRegistrar",
173+
"--reason", "someReason",
174+
"--domains", "foo.tld,bar.tld");
175+
verify(connection)
176+
.sendPostRequest(
177+
"/_dr/task/bulkDomainTransfer",
178+
ImmutableMap.of(
179+
"gainingRegistrarId",
180+
"NewRegistrar",
181+
"requestedByRegistrar",
182+
false,
183+
"reason",
184+
"someReason"),
185+
MediaType.PLAIN_TEXT_UTF_8,
186+
"[\"foo.tld\",\"bar.tld\"]".getBytes(UTF_8));
187+
}
168188
}

0 commit comments

Comments
 (0)