Remove names from packages on automatic transfers (#1827)

* Remove names from packages on automatic transfers

* Add more tests

* Remove unneccesary local variable

* Eliminate unnecessary api call

* Reformat if blocks

* Don't overwrite existingRecurring

core/src/main/java/google/registry/flows/domain/DomainTransferRequestFlow.java

@@ -194,11 +194,21 @@ public final class DomainTransferRequestFlow implements TransactionalFlow {
     }
     // If the period is zero, then there is no fee for the transfer.
     Recurring existingRecurring = tm().loadByKey(existingDomain.getAutorenewBillingEvent());
-    Optional<FeesAndCredits> feesAndCredits =
+    Optional<FeesAndCredits> feesAndCredits;
-        period.getValue() == 0
+    if (period.getValue() == 0) {
-            ? Optional.empty()
+      feesAndCredits = Optional.empty();
-            : Optional.of(
+    } else if (!existingDomain.getCurrentPackageToken().isPresent()) {
-                pricingLogic.getTransferPrice(registry, targetId, now, existingRecurring));
+      feesAndCredits =
+          Optional.of(pricingLogic.getTransferPrice(registry, targetId, now, existingRecurring));
+    } else {
+      // If existing domain is in a package, calculate the transfer price with default renewal price
+      // behavior
+      feesAndCredits =
+          period.getValue() == 0
+              ? Optional.empty()
+              : Optional.of(pricingLogic.getTransferPrice(registry, targetId, now, null));
+    }
+
     if (feesAndCredits.isPresent()) {
       validateFeeChallenge(feeTransfer, feesAndCredits.get());
     }

core/src/main/java/google/registry/flows/domain/DomainTransferUtils.java

@@ -24,6 +24,7 @@ import google.registry.model.billing.BillingEvent;
 import google.registry.model.billing.BillingEvent.Flag;
 import google.registry.model.billing.BillingEvent.Reason;
 import google.registry.model.billing.BillingEvent.Recurring;
+import google.registry.model.billing.BillingEvent.RenewalPriceBehavior;
 import google.registry.model.domain.Domain;
 import google.registry.model.domain.GracePeriod;
 import google.registry.model.domain.Period;
@@ -144,7 +145,13 @@ public final class DomainTransferUtils {
     return builder
         .add(
             createGainingClientAutorenewEvent(
-                existingRecurring,
+                existingDomain.getCurrentPackageToken().isPresent()
+                    ? existingRecurring
+                        .asBuilder()
+                        .setRenewalPriceBehavior(RenewalPriceBehavior.DEFAULT)
+                        .setRenewalPrice(null)
+                        .build()
+                    : existingRecurring,
                 serverApproveNewExpirationTime,
                 domainHistoryId,
                 targetId,

core/src/main/java/google/registry/model/domain/DomainBase.java

@@ -481,7 +481,8 @@ public class DomainBase extends EppResource
               // Set the speculatively-written new autorenew events as the domain's autorenew
               // events.
               .setAutorenewBillingEvent(transferData.getServerApproveAutorenewEvent())
-              .setAutorenewPollMessage(transferData.getServerApproveAutorenewPollMessage());
+              .setAutorenewPollMessage(transferData.getServerApproveAutorenewPollMessage())
+              .setCurrentPackageToken(null);
       if (transferData.getTransferPeriod().getValue() == 1) {
         // Set the grace period using a key to the pre-scheduled transfer billing event.  Not using
         // GracePeriod.forBillingEvent() here in order to avoid the actual Datastore fetch.

core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java

@@ -21,6 +21,7 @@ import static com.google.common.truth.Truth8.assertThat;
 import static google.registry.batch.AsyncTaskEnqueuer.PARAM_REQUESTED_TIME;
 import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY;
 import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS;
+import static google.registry.model.domain.token.AllocationToken.TokenType.PACKAGE;
 import static google.registry.model.domain.token.AllocationToken.TokenType.SINGLE_USE;
 import static google.registry.model.domain.token.AllocationToken.TokenType.UNLIMITED_USE;
 import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_SUCCESSFUL;
@@ -1321,6 +1322,154 @@ class DomainTransferRequestFlowTest
             .build());
   }
 
+  @Test
+  void testSuccess_specifiedRenewalPrice_notCarriedOverForPackageName() throws Exception {
+    setupDomain("example", "tld");
+    persistResource(Registry.get("tld").asBuilder().build());
+    domain = loadByEntity(domain);
+    persistResource(
+        loadByKey(domain.getAutorenewBillingEvent())
+            .asBuilder()
+            .setRenewalPriceBehavior(RenewalPriceBehavior.SPECIFIED)
+            .setRenewalPrice(Money.of(USD, new BigDecimal("18.79")))
+            .build());
+    AllocationToken allocationToken =
+        persistResource(
+            new AllocationToken.Builder()
+                .setToken("abc123")
+                .setTokenType(PACKAGE)
+                .setRenewalPriceBehavior(RenewalPriceBehavior.SPECIFIED)
+                .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar"))
+                .build());
+    domain =
+        persistResource(
+            domain.asBuilder().setCurrentPackageToken(allocationToken.createVKey()).build());
+    DateTime now = clock.nowUtc();
+
+    setEppInput("domain_transfer_request.xml");
+    eppLoader.replaceAll("JD1234-REP", contact.getRepoId());
+    runFlowAssertResponse(loadFile("domain_transfer_request_response.xml"));
+    domain = loadByEntity(domain);
+
+    DomainHistory requestHistory =
+        getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REQUEST, DomainHistory.class);
+    // Check that the server approve billing recurrence (which will reify after 5 days if the
+    // transfer is not explicitly acked) maintains the non-premium behavior.
+    assertBillingEventsForResource(
+        domain,
+        new BillingEvent.OneTime.Builder()
+            .setBillingTime(now.plusDays(10)) // 5 day pending transfer + 5 day billing grace period
+            .setEventTime(now.plusDays(5))
+            .setRegistrarId("NewRegistrar")
+            .setCost(Money.of(USD, new BigDecimal("11.00")))
+            .setDomainHistory(requestHistory)
+            .setReason(Reason.TRANSFER)
+            .setPeriodYears(1)
+            .setTargetId("example.tld")
+            .build(),
+        getGainingClientAutorenewEvent()
+            .asBuilder()
+            .setRenewalPriceBehavior(RenewalPriceBehavior.DEFAULT)
+            .setRenewalPrice(null)
+            .setDomainHistory(requestHistory)
+            .build(),
+        getLosingClientAutorenewEvent()
+            .asBuilder()
+            .setRecurrenceEndTime(now.plusDays(5))
+            .setRenewalPriceBehavior(RenewalPriceBehavior.SPECIFIED)
+            .setRenewalPrice(Money.of(USD, new BigDecimal("18.79")))
+            .build());
+  }
+
+  @Test
+  void testSuccess_defaultRenewalPrice_carriedOverForPackageName() throws Exception {
+    setupDomain("example", "tld");
+    persistResource(Registry.get("tld").asBuilder().build());
+    domain = loadByEntity(domain);
+    persistResource(
+        loadByKey(domain.getAutorenewBillingEvent())
+            .asBuilder()
+            .setRenewalPriceBehavior(RenewalPriceBehavior.DEFAULT)
+            .build());
+    AllocationToken allocationToken =
+        persistResource(
+            new AllocationToken.Builder()
+                .setToken("abc123")
+                .setTokenType(PACKAGE)
+                .setRenewalPriceBehavior(RenewalPriceBehavior.SPECIFIED)
+                .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar"))
+                .build());
+    domain =
+        persistResource(
+            domain.asBuilder().setCurrentPackageToken(allocationToken.createVKey()).build());
+    DateTime now = clock.nowUtc();
+
+    setEppInput("domain_transfer_request.xml");
+    eppLoader.replaceAll("JD1234-REP", contact.getRepoId());
+    runFlowAssertResponse(loadFile("domain_transfer_request_response.xml"));
+    domain = loadByEntity(domain);
+
+    DomainHistory requestHistory =
+        getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REQUEST, DomainHistory.class);
+    // Check that the server approve billing recurrence (which will reify after 5 days if the
+    // transfer is not explicitly acked) maintains the non-premium behavior.
+    assertBillingEventsForResource(
+        domain,
+        new BillingEvent.OneTime.Builder()
+            .setBillingTime(now.plusDays(10)) // 5 day pending transfer + 5 day billing grace period
+            .setEventTime(now.plusDays(5))
+            .setRegistrarId("NewRegistrar")
+            .setCost(Money.of(USD, new BigDecimal("11.00")))
+            .setDomainHistory(requestHistory)
+            .setReason(Reason.TRANSFER)
+            .setPeriodYears(1)
+            .setTargetId("example.tld")
+            .build(),
+        getGainingClientAutorenewEvent()
+            .asBuilder()
+            .setRenewalPriceBehavior(RenewalPriceBehavior.DEFAULT)
+            .setRenewalPrice(null)
+            .setDomainHistory(requestHistory)
+            .build(),
+        getLosingClientAutorenewEvent()
+            .asBuilder()
+            .setRecurrenceEndTime(now.plusDays(5))
+            .setRenewalPriceBehavior(RenewalPriceBehavior.DEFAULT)
+            .build());
+  }
+
+  @Test
+  void testSuccess_packageName_zeroPeriod() throws Exception {
+    setupDomain("example", "tld");
+    persistResource(Registry.get("tld").asBuilder().build());
+    domain = loadByEntity(domain);
+    persistResource(
+        loadByKey(domain.getAutorenewBillingEvent())
+            .asBuilder()
+            .setRenewalPriceBehavior(RenewalPriceBehavior.DEFAULT)
+            .build());
+    AllocationToken allocationToken =
+        persistResource(
+            new AllocationToken.Builder()
+                .setToken("abc123")
+                .setTokenType(PACKAGE)
+                .setRenewalPriceBehavior(RenewalPriceBehavior.SPECIFIED)
+                .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar"))
+                .build());
+    domain =
+        persistResource(
+            domain.asBuilder().setCurrentPackageToken(allocationToken.createVKey()).build());
+
+    doSuccessfulSuperuserExtensionTest(
+        "domain_transfer_request_superuser_extension.xml",
+        "domain_transfer_request_response_su_ext_zero_period_zero_transfer_length.xml",
+        domain.getRegistrationExpirationTime().plusYears(0),
+        ImmutableMap.of("PERIOD", "0", "AUTOMATIC_TRANSFER_LENGTH", "0"),
+        Optional.empty(),
+        Period.create(0, Unit.YEARS),
+        Duration.ZERO);
+  }
+
   private void runWrongCurrencyTest(Map<String, String> substitutions) {
     Map<String, String> fullSubstitutions = Maps.newHashMap();
     fullSubstitutions.putAll(substitutions);

core/src/test/java/google/registry/model/domain/DomainTest.java

@@ -19,6 +19,7 @@ import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
+import static google.registry.model.billing.BillingEvent.RenewalPriceBehavior.SPECIFIED;
 import static google.registry.model.domain.token.AllocationToken.TokenType.PACKAGE;
 import static google.registry.model.domain.token.AllocationToken.TokenType.SINGLE_USE;
 import static google.registry.testing.DatabaseHelper.cloneAndSetAutoTimestamps;
@@ -48,7 +49,6 @@ import google.registry.model.ImmutableObjectSubject;
 import google.registry.model.billing.BillingEvent;
 import google.registry.model.billing.BillingEvent.Flag;
 import google.registry.model.billing.BillingEvent.Reason;
-import google.registry.model.billing.BillingEvent.RenewalPriceBehavior;
 import google.registry.model.contact.Contact;
 import google.registry.model.domain.DesignatedContact.Type;
 import google.registry.model.domain.launch.LaunchNotice;
@@ -821,6 +821,45 @@ public class DomainTest {
         .isEqualTo(newExpiration);
   }
 
+  @Test
+  void testClone_removesPackageFromTransferredDomain() {
+    // If the transfer implicitly succeeded, the expiration time should be extended even if it
+    // hadn't already expired
+    DateTime now = DateTime.now(UTC);
+    DateTime transferExpirationTime = now.minusDays(1);
+    DateTime previousExpiration = now.plusWeeks(2);
+
+    DomainTransferData transferData =
+        new DomainTransferData.Builder()
+            .setPendingTransferExpirationTime(transferExpirationTime)
+            .setTransferStatus(TransferStatus.PENDING)
+            .setGainingRegistrarId("TheRegistrar")
+            .build();
+    Period extensionPeriod = transferData.getTransferPeriod();
+    DateTime newExpiration = previousExpiration.plusYears(extensionPeriod.getValue());
+    AllocationToken allocationToken =
+        persistResource(
+            new AllocationToken.Builder()
+                .setToken("abc123")
+                .setTokenType(PACKAGE)
+                .setRenewalPriceBehavior(SPECIFIED)
+                .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar"))
+                .build());
+    domain =
+        persistResource(
+            domain
+                .asBuilder()
+                .setRegistrationExpirationTime(previousExpiration)
+                .setTransferData(transferData)
+                .setCurrentPackageToken(allocationToken.createVKey())
+                .build());
+
+    assertThat(domain.getCurrentPackageToken()).isPresent();
+    Domain clonedDomain = domain.cloneProjectedAtTime(now);
+    assertThat(clonedDomain.getRegistrationExpirationTime()).isEqualTo(newExpiration);
+    assertThat(clonedDomain.getCurrentPackageToken()).isEmpty();
+  }
+
   @Test
   void testClone_doesNotExtendExpirationForPendingTransfer() {
     // Pending transfers shouldn't affect the expiration time
@@ -846,6 +885,41 @@ public class DomainTest {
         .isEqualTo(previousExpiration);
   }
 
+  @Test
+  void testClone_doesNotRemovePackageForPendingTransfer() {
+    // Pending transfers shouldn't affect the expiration time
+    DateTime now = DateTime.now(UTC);
+    DateTime transferExpirationTime = now.plusDays(1);
+    DateTime previousExpiration = now.plusWeeks(2);
+
+    DomainTransferData transferData =
+        new DomainTransferData.Builder()
+            .setPendingTransferExpirationTime(transferExpirationTime)
+            .setTransferStatus(TransferStatus.PENDING)
+            .setGainingRegistrarId("TheRegistrar")
+            .build();
+    AllocationToken allocationToken =
+        persistResource(
+            new AllocationToken.Builder()
+                .setToken("abc123")
+                .setTokenType(PACKAGE)
+                .setRenewalPriceBehavior(SPECIFIED)
+                .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar"))
+                .build());
+    domain =
+        persistResource(
+            domain
+                .asBuilder()
+                .setRegistrationExpirationTime(previousExpiration)
+                .setTransferData(transferData)
+                .setCurrentPackageToken(allocationToken.createVKey())
+                .build());
+
+    Domain clonedDomain = domain.cloneProjectedAtTime(now);
+    assertThat(clonedDomain.getRegistrationExpirationTime()).isEqualTo(previousExpiration);
+    assertThat(clonedDomain.getCurrentPackageToken().get()).isEqualTo(allocationToken.createVKey());
+  }
+
   @Test
   void testClone_transferDuringAutorenew() {
     // When the domain is an autorenew grace period, we should not extend the registration
@@ -1001,7 +1075,7 @@ public class DomainTest {
         new AllocationToken.Builder()
             .setToken("abc123")
             .setTokenType(PACKAGE)
-            .setRenewalPriceBehavior(RenewalPriceBehavior.SPECIFIED)
+            .setRenewalPriceBehavior(SPECIFIED)
             .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar"))
             .build();
     IllegalArgumentException thrown =
@@ -1018,7 +1092,7 @@ public class DomainTest {
             new AllocationToken.Builder()
                 .setToken("abc123")
                 .setTokenType(PACKAGE)
-                .setRenewalPriceBehavior(RenewalPriceBehavior.SPECIFIED)
+                .setRenewalPriceBehavior(SPECIFIED)
                 .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar"))
                 .build());
     domain = domain.asBuilder().setCurrentPackageToken(allocationToken.createVKey()).build();