Add reason and requestedByRegistrar to domain renew flow (#1378)

* Resolve merge conflict * Include reason and requestedByRegistrar in URS test file * Modify test cases for new parameters in renew flow * Add reason and registrar_request to renew domain command * Update comments for new params in renew flow * Make changes based on feedback
2025-07-24 11:38:35 +02:00 · 2021-10-13 11:41:02 -04:00 · 2021-10-13 11:41:02 -04:00 · 6c4881fa4c
commit 6c4881fa4c
parent b52e289037
11 changed files with 331 additions and 17 deletions
--- a/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java
+++ b/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java
@ -229,6 +229,15 @@ public final class DomainRenewFlow implements TransactionalFlow {

  private DomainHistory buildDomainHistory(
      DomainBase newDomain, DateTime now, Period period, Duration renewGracePeriod) {
+    Optional<MetadataExtension> metadataExtensionOpt =
+        eppInput.getSingleExtension(MetadataExtension.class);
+    if (metadataExtensionOpt.isPresent()) {
+      MetadataExtension metadataExtension = metadataExtensionOpt.get();
+      if (metadataExtension.getReason() != null) {
+        historyBuilder.setReason(metadataExtension.getReason());
+      }
+      historyBuilder.setRequestedByRegistrar(metadataExtension.getRequestedByRegistrar());
+    }
    return historyBuilder
        .setType(DOMAIN_RENEW)
        .setPeriod(period)
--- a/core/src/main/java/google/registry/tools/RenewDomainCommand.java
+++ b/core/src/main/java/google/registry/tools/RenewDomainCommand.java
@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.util.CollectionUtils.findDuplicates;
+import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 import static google.registry.util.PreconditionsUtils.checkArgumentPresent;

 import com.beust.jcommander.Parameter;
@ -53,6 +54,17 @@ final class RenewDomainCommand extends MutatingEppToolCommand {
  @Parameter(description = "Names of the domains to renew.", required = true)
  private List<String> mainParameters;

+  @Parameter(
+      names = {"--reason"},
+      description = "Reason for the change.")
+  String reason;
+
+  @Parameter(
+      names = {"--registrar_request"},
+      description = "Whether the change was requested by a registrar.",
+      arity = 1)
+  Boolean requestedByRegistrar;
+
  @Inject
  Clock clock;

@ -70,12 +82,23 @@ final class RenewDomainCommand extends MutatingEppToolCommand {
      checkArgumentPresent(domainOptional, "Domain '%s' does not exist or is deleted", domainName);
      setSoyTemplate(DomainRenewSoyInfo.getInstance(), DomainRenewSoyInfo.RENEWDOMAIN);
      DomainBase domain = domainOptional.get();
-      addSoyRecord(
-          isNullOrEmpty(clientId) ? domain.getCurrentSponsorRegistrarId() : clientId,
+
+      SoyMapData soyMapData =
          new SoyMapData(
              "domainName", domain.getDomainName(),
              "expirationDate", domain.getRegistrationExpirationTime().toString(DATE_FORMATTER),
-              "period", String.valueOf(period)));
+              "period", String.valueOf(period));
+
+      if (requestedByRegistrar != null) {
+        soyMapData.put("requestedByRegistrar", requestedByRegistrar.toString());
+      }
+      if (reason != null) {
+        checkArgumentNotNull(
+            requestedByRegistrar, "--registrar_request is required when --reason is specified");
+        soyMapData.put("reason", reason);
+      }
+      addSoyRecord(
+          isNullOrEmpty(clientId) ? domain.getCurrentSponsorRegistrarId() : clientId, soyMapData);
    }
  }
 }
--- a/core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java
+++ b/core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java
@ -163,7 +163,12 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand {
                  .toString(DateTimeFormat.forPattern("YYYY-MM-dd")),
              // period is the number of years to renew the registration for
              "period",
-              String.valueOf(1)));
+              String.valueOf(1),
+              // use the same values for reason and requestedByRegistrar from update flow
+              "reason",
+              (undo ? "Undo " : "") + "Uniform Rapid Suspension",
+              "requestedByRegistrar",
+              Boolean.toString(false)));
    }

    // trigger update flow
--- a/core/src/main/resources/google/registry/tools/soy/DomainRenew.soy
+++ b/core/src/main/resources/google/registry/tools/soy/DomainRenew.soy
@ -21,6 +21,8 @@
 {@param domainName: string}
 {@param expirationDate: string}
 {@param period: string}
+{@param? reason: string}
+{@param? requestedByRegistrar: string}
 <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
 <epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
  <command>
@ -32,6 +34,18 @@
        <domain:period unit="y">{$period}</domain:period>
      </domain:renew>
    </renew>
+    {if $reason or $requestedByRegistrar}
+      <extension>
+          <metadata:metadata xmlns:metadata="urn:google:params:xml:ns:metadata-1.0">
+            {if $reason}
+              <metadata:reason>{$reason}</metadata:reason>
+            {/if}
+            {if $requestedByRegistrar}
+              <metadata:requestedByRegistrar>{$requestedByRegistrar}</metadata:requestedByRegistrar>
+            {/if}
+          </metadata:metadata>
+      </extension>
+    {/if}
    <clTRID>RegistryTool</clTRID>
  </command>
 </epp>
--- a/core/src/test/java/google/registry/flows/domain/DomainRenewFlowTest.java
+++ b/core/src/test/java/google/registry/flows/domain/DomainRenewFlowTest.java
@ -42,6 +42,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 import google.registry.flows.EppException;
+import google.registry.flows.EppRequestSource;
 import google.registry.flows.FlowUtils.UnknownCurrencyEppException;
 import google.registry.flows.ResourceFlowTestCase;
 import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
@ -533,6 +534,55 @@ class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, DomainBa
            .build());
  }

+  @TestOfyAndSql
+  void testSuccess_metaData_withReasonAndRequestedByRegistrar() throws Exception {
+    eppRequestSource = EppRequestSource.TOOL;
+    setEppInput(
+        "domain_renew_metadata_with_reason_and_requestedByRegistrar.xml",
+        ImmutableMap.of(
+            "DOMAIN",
+            "example.tld",
+            "EXPDATE",
+            "2000-04-03",
+            "YEARS",
+            "1",
+            "REASON",
+            "domain-renew-test",
+            "REQUESTED",
+            "false"));
+    persistDomain();
+    runFlow();
+    DomainBase domain = reloadResourceByForeignKey();
+    assertAboutDomains()
+        .that(domain)
+        .hasOneHistoryEntryEachOfTypes(
+            HistoryEntry.Type.DOMAIN_CREATE, HistoryEntry.Type.DOMAIN_RENEW);
+    assertAboutHistoryEntries()
+        .that(getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_RENEW))
+        .hasMetadataReason("domain-renew-test")
+        .and()
+        .hasMetadataRequestedByRegistrar(false);
+  }
+
+  @TestOfyAndSql
+  void testSuccess_metaData_withRequestedByRegistrarOnly() throws Exception {
+    eppRequestSource = EppRequestSource.TOOL;
+    setEppInput("domain_renew_metadata_with_requestedByRegistrar_only.xml");
+
+    persistDomain();
+    runFlow();
+    DomainBase domain1 = reloadResourceByForeignKey();
+    assertAboutDomains()
+        .that(domain1)
+        .hasOneHistoryEntryEachOfTypes(
+            HistoryEntry.Type.DOMAIN_CREATE, HistoryEntry.Type.DOMAIN_RENEW);
+    assertAboutHistoryEntries()
+        .that(getOnlyHistoryEntryOfType(domain1, HistoryEntry.Type.DOMAIN_RENEW))
+        .hasMetadataReason(null)
+        .and()
+        .hasMetadataRequestedByRegistrar(true);
+  }
+
  @TestOfyAndSql
  void testFailure_neverExisted() throws Exception {
    ResourceDoesNotExistException thrown =
--- a/core/src/test/java/google/registry/tools/RenewDomainCommandTest.java
+++ b/core/src/test/java/google/registry/tools/RenewDomainCommandTest.java
@ -132,6 +132,55 @@ public class RenewDomainCommandTest extends EppToolCommandTestCase<RenewDomainCo
        .verifyNoMoreSent();
  }

+  @Test
+  void testSuccess_withReasonAndRegistrarRequest() throws Exception {
+    persistActiveDomain(
+        "domain.tld",
+        DateTime.parse("2014-09-05T05:05:05Z"),
+        DateTime.parse("2015-09-05T05:05:05Z"));
+    runCommandForced(
+        "domain.tld", "--period=1", "--reason=Renewing test domain", "--registrar_request=true");
+
+    eppVerifier
+        .expectRegistrarId("TheRegistrar")
+        .verifySent(
+            "domain_renew_via_urs.xml",
+            ImmutableMap.of(
+                "DOMAIN",
+                "domain.tld",
+                "EXPDATE",
+                "2015-09-05",
+                "YEARS",
+                "1",
+                "REASON",
+                "Renewing test domain",
+                "REQUESTED",
+                "true"));
+  }
+
+  @Test
+  void testSuccess_withReqistrarRequestOnly() throws Exception {
+    persistActiveDomain(
+        "domain.tld",
+        DateTime.parse("2014-09-05T05:05:05Z"),
+        DateTime.parse("2015-09-05T05:05:05Z"));
+    runCommandForced("domain.tld", "--period=1", "--registrar_request=true");
+
+    eppVerifier
+        .expectRegistrarId("TheRegistrar")
+        .verifySent(
+            "domain_renew_with_metadata_requestedByRegistrar_only.xml",
+            ImmutableMap.of(
+                "DOMAIN",
+                "domain.tld",
+                "EXPDATE",
+                "2015-09-05",
+                "YEARS",
+                "1",
+                "REQUESTED",
+                "true"));
+  }
+
  @Test
  void testFailure_domainDoesntExist() {
    IllegalArgumentException e =
@ -173,4 +222,19 @@ public class RenewDomainCommandTest extends EppToolCommandTestCase<RenewDomainCo
  void testFailure_missingDomainNames() {
    assertThrows(ParameterException.class, () -> runCommand("--period 4"));
  }
+
+  @Test
+  void testFailure_registrarRequestIsRequiredWhenReasonIsPresent() {
+    persistActiveDomain(
+        "domain.tld",
+        DateTime.parse("2014-09-05T05:05:05Z"),
+        DateTime.parse("2015-09-05T05:05:05Z"));
+    IllegalArgumentException e =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> runCommandForced("domain.tld", "--period=1", "--reason=testing_only"));
+    assertThat(e)
+        .hasMessageThat()
+        .isEqualTo("--registrar_request is required when --reason is specified");
+  }
 }
--- a/core/src/test/java/google/registry/tools/UniformRapidSuspensionCommandTest.java
+++ b/core/src/test/java/google/registry/tools/UniformRapidSuspensionCommandTest.java
@ -242,13 +242,44 @@ class UniformRapidSuspensionCommandTest
  }

  @Test
-  void testRenewOneYear_renewFlowIsTriggered() throws Exception {
-    // this test case was written based on an existing test case,
-    // testUndo_removesLocksReplacesHostsAndDsData() but two things were modified to test the
-    // renew workflow, which were:
-    // 1) a different domain that contains creation time and expiration time
-    // 2) renew_one_year is set to true
+  void testRenewOneYearWithoutUndo_verifyReasonWithoutUndo() throws Exception {
+    persistDomainWithHosts(
+        newDomainBase("evil.tld")
+            .asBuilder()
+            .setCreationTimeForTest(DateTime.parse("2021-10-01T05:01:11Z"))
+            .setRegistrationExpirationTime(DateTime.parse("2022-10-01T05:01:11Z"))
+            .setPersistedCurrentSponsorRegistrarId("CharlestonRoad")
+            .build(),
+        defaultDsData,
+        urs1,
+        urs2);

+    runCommandForced(
+        "--domain_name=evil.tld",
+        "--hosts=ns1.example.com,ns2.example.com",
+        "--renew_one_year=true");
+
+    eppVerifier
+        .expectRegistrarId("CharlestonRoad")
+        .expectSuperuser()
+        .verifySent(
+            "domain_renew_via_urs.xml",
+            ImmutableMap.of(
+                "DOMAIN",
+                "evil.tld",
+                "EXPDATE",
+                "2022-10-01",
+                "YEARS",
+                "1",
+                "REASON",
+                "Uniform Rapid Suspension",
+                "REQUESTED",
+                "false"))
+        .verifySentAny();
+  }
+
+  @Test
+  void testRenewOneYearWithUndo_verifyReasonWithUndo() throws Exception {
    persistDomainWithHosts(
        newDomainBase("evil.tld")
            .asBuilder()
@ -266,21 +297,65 @@ class UniformRapidSuspensionCommandTest
        "--hosts=ns1.example.com,ns2.example.com",
        "--renew_one_year=true");

-    // verify if renew flow is triggered
    eppVerifier
        .expectRegistrarId("CharlestonRoad")
        .expectSuperuser()
        .verifySent(
-            "domain_renew.xml",
-            ImmutableMap.of("DOMAIN", "evil.tld", "EXPDATE", "2022-10-01", "YEARS", "1"));
+            "domain_renew_via_urs.xml",
+            ImmutableMap.of(
+                "DOMAIN",
+                "evil.tld",
+                "EXPDATE",
+                "2022-10-01",
+                "YEARS",
+                "1",
+                "REASON",
+                "Undo Uniform Rapid Suspension",
+                "REQUESTED",
+                "false"))
+        .verifySentAny();
+  }
+
+  @Test
+  void testRenewOneYear_verifyBothRenewAndUpdateFlowsAreTriggered() throws Exception {
+    persistDomainWithHosts(
+        newDomainBase("evil.tld")
+            .asBuilder()
+            .setCreationTimeForTest(DateTime.parse("2021-10-01T05:01:11Z"))
+            .setRegistrationExpirationTime(DateTime.parse("2022-10-01T05:01:11Z"))
+            .setPersistedCurrentSponsorRegistrarId("CharlestonRoad")
+            .build(),
+        defaultDsData,
+        urs1,
+        urs2);
+
+    runCommandForced(
+        "--domain_name=evil.tld",
+        "--undo",
+        "--hosts=ns1.example.com,ns2.example.com",
+        "--renew_one_year=true");

-    // verify if update flow is triggered
    eppVerifier
        .expectRegistrarId("CharlestonRoad")
        .expectSuperuser()
-        .verifySent("uniform_rapid_suspension_undo.xml")
-        .verifyNoMoreSent();
-    assertNotInStdout("--undo"); // Undo shouldn't print a new undo command.
+        .verifySent(
+            "domain_renew_via_urs.xml",
+            ImmutableMap.of(
+                "DOMAIN",
+                "evil.tld",
+                "EXPDATE",
+                "2022-10-01",
+                "YEARS",
+                "1",
+                "REASON",
+                "Undo Uniform Rapid Suspension",
+                "REQUESTED",
+                "false"));
+
+    eppVerifier
+        .expectRegistrarId("CharlestonRoad")
+        .expectSuperuser()
+        .verifySent("uniform_rapid_suspension_undo.xml");

    // verify that no other flows are triggered after the renew and update flows
    eppVerifier.verifyNoMoreSent();
--- a/core/src/test/resources/google/registry/flows/domain/domain_renew_metadata_with_reason_and_requestedByRegistrar.xml
+++ b/core/src/test/resources/google/registry/flows/domain/domain_renew_metadata_with_reason_and_requestedByRegistrar.xml
@ -0,0 +1,19 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <renew>
+      <domain:renew
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>%DOMAIN%</domain:name>
+        <domain:curExpDate>%EXPDATE%</domain:curExpDate>
+        <domain:period unit="y">%YEARS%</domain:period>
+      </domain:renew>
+    </renew>
+    <extension>
+      <metadata:metadata xmlns:metadata="urn:google:params:xml:ns:metadata-1.0">
+        <metadata:reason>%REASON%</metadata:reason>
+        <metadata:requestedByRegistrar>%REQUESTED%</metadata:requestedByRegistrar>
+      </metadata:metadata>
+    </extension>
+    <clTRID>ABC-12345</clTRID>
+  </command>
+</epp>
--- a/core/src/test/resources/google/registry/flows/domain/domain_renew_metadata_with_requestedByRegistrar_only.xml
+++ b/core/src/test/resources/google/registry/flows/domain/domain_renew_metadata_with_requestedByRegistrar_only.xml
@ -0,0 +1,18 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <renew>
+      <domain:renew
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>example.tld</domain:name>
+        <domain:curExpDate>2000-04-03</domain:curExpDate>
+        <domain:period unit="y">1</domain:period>
+      </domain:renew>
+    </renew>
+    <extension>
+      <metadata:metadata xmlns:metadata="urn:google:params:xml:ns:metadata-1.0">
+        <metadata:requestedByRegistrar>true</metadata:requestedByRegistrar>
+      </metadata:metadata>
+    </extension>
+    <clTRID>ABC-12345</clTRID>
+  </command>
+</epp>
--- a/core/src/test/resources/google/registry/tools/server/domain_renew_via_urs.xml
+++ b/core/src/test/resources/google/registry/tools/server/domain_renew_via_urs.xml
@ -0,0 +1,19 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <renew>
+      <domain:renew
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>%DOMAIN%</domain:name>
+        <domain:curExpDate>%EXPDATE%</domain:curExpDate>
+        <domain:period unit="y">%YEARS%</domain:period>
+      </domain:renew>
+    </renew>
+    <extension>
+      <metadata:metadata xmlns:metadata="urn:google:params:xml:ns:metadata-1.0">
+        <metadata:reason>%REASON%</metadata:reason>
+        <metadata:requestedByRegistrar>%REQUESTED%</metadata:requestedByRegistrar>
+      </metadata:metadata>
+    </extension>
+    <clTRID>RegistryTool</clTRID>
+  </command>
+</epp>
--- a/core/src/test/resources/google/registry/tools/server/domain_renew_with_metadata_requestedByRegistrar_only.xml
+++ b/core/src/test/resources/google/registry/tools/server/domain_renew_with_metadata_requestedByRegistrar_only.xml
@ -0,0 +1,18 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <renew>
+      <domain:renew
+          xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name>%DOMAIN%</domain:name>
+        <domain:curExpDate>%EXPDATE%</domain:curExpDate>
+        <domain:period unit="y">%YEARS%</domain:period>
+      </domain:renew>
+    </renew>
+    <extension>
+      <metadata:metadata xmlns:metadata="urn:google:params:xml:ns:metadata-1.0">
+        <metadata:requestedByRegistrar>%REQUESTED%</metadata:requestedByRegistrar>
+      </metadata:metadata>
+    </extension>
+    <clTRID>RegistryTool</clTRID>
+  </command>
+</epp>