Fix exDate calculation in domain transfer flows (#389)

* Fix exDate calculation in domain transfer flows

* Link ICANN rule for zero-period transfer

* Get rid of some unnecessary method parameters

* Add domain info commands to test case

core/src/main/java/google/registry/flows/ResourceFlowUtils.java

@@ -42,6 +42,8 @@ import google.registry.model.EppResource.ForeignKeyedEppResource;
 import google.registry.model.EppResource.ResourceWithTransferData;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.domain.DomainBase;
+import google.registry.model.domain.Period;
+import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.eppcommon.AuthInfo;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.index.ForeignKeyIndex;
@@ -247,6 +249,30 @@ public final class ResourceFlowUtils {
     }
   }
 
+  /**
+   * Computes the exDate for the domain at the given transfer approval time with an adjusted amount
+   * of transfer period years if the domain is in the auto renew grace period at the time of
+   * approval.
+   *
+   * @param domain is the domain already projected at approvalTime
+   */
+  public static DateTime computeExDateForApprovalTime(
+      DomainBase domain, DateTime approvalTime, Period period) {
+    boolean inAutoRenew = domain.getGracePeriodStatuses().contains(GracePeriodStatus.AUTO_RENEW);
+    // inAutoRenew is set to false if the period is zero because a zero-period transfer should not
+    // subsume an autorenew.
+    // https://www.icann.org/resources/unthemed-pages/appendix-07-2010-01-06-en section 3.1.2
+    // specifies that when a transfer occurs without a change to the expiration date, the losing
+    // registrar's account should still be charged for the autorenew.
+    if (period.getValue() == 0) {
+      inAutoRenew = false;
+    }
+    return DomainBase.extendRegistrationWithCap(
+        approvalTime,
+        domain.getRegistrationExpirationTime(),
+        period.getValue() - (inAutoRenew ? 1 : 0));
+  }
+
   /** Resource with this id does not exist. */
   public static class ResourceDoesNotExistException extends ObjectDoesNotExistException {
     public ResourceDoesNotExistException(Class<?> type, String targetId) {

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

@@ -16,6 +16,7 @@ package google.registry.flows.domain;
 
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static google.registry.flows.FlowUtils.validateClientIsLoggedIn;
+import static google.registry.flows.ResourceFlowUtils.computeExDateForApprovalTime;
 import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence;
 import static google.registry.flows.ResourceFlowUtils.verifyHasPendingTransfer;
 import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo;
@@ -26,7 +27,6 @@ import static google.registry.flows.domain.DomainFlowUtils.updateAutorenewRecurr
 import static google.registry.flows.domain.DomainTransferUtils.createGainingTransferPollMessage;
 import static google.registry.flows.domain.DomainTransferUtils.createTransferResponse;
 import static google.registry.model.ResourceTransferUtils.approvePendingTransfer;
-import static google.registry.model.domain.DomainBase.extendRegistrationWithCap;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_SUCCESSFUL;
 import static google.registry.model.transaction.TransactionManagerFactory.tm;
@@ -136,9 +136,7 @@ public final class DomainTransferApproveFlow implements TransactionalFlow {
     // increase the registration time, since the transfer subsumes the autorenew's extra year.
     GracePeriod autorenewGrace =
         getOnlyElement(existingDomain.getGracePeriodsOfType(GracePeriodStatus.AUTO_RENEW), null);
-    int extraYears = transferData.getTransferPeriod().getValue();
     if (autorenewGrace != null) {
-      extraYears = 0;
       // During a normal transfer, if the domain is in the auto-renew grace period, the auto-renew
       // billing event is cancelled and the gaining registrar is charged for the one year renewal.
       // But, if the superuser extension is used to request a transfer without an additional year
@@ -152,8 +150,8 @@ public final class DomainTransferApproveFlow implements TransactionalFlow {
     // Close the old autorenew event and poll message at the transfer time (aka now). This may end
     // up deleting the poll message.
     updateAutorenewRecurrenceEndTime(existingDomain, now);
-    DateTime newExpirationTime = extendRegistrationWithCap(
-        now, existingDomain.getRegistrationExpirationTime(), extraYears);
+    DateTime newExpirationTime =
+        computeExDateForApprovalTime(existingDomain, now, transferData.getTransferPeriod());
     // Create a new autorenew event starting at the expiration time.
     BillingEvent.Recurring autorenewEvent = new BillingEvent.Recurring.Builder()
         .setReason(Reason.RENEW)

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

@@ -18,13 +18,13 @@ import static google.registry.flows.FlowUtils.validateClientIsLoggedIn;
 import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence;
 import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo;
 import static google.registry.flows.domain.DomainTransferUtils.createTransferResponse;
-import static google.registry.model.domain.DomainBase.extendRegistrationWithCap;
 
 import google.registry.flows.EppException;
 import google.registry.flows.ExtensionManager;
 import google.registry.flows.Flow;
 import google.registry.flows.FlowModule.ClientId;
 import google.registry.flows.FlowModule.TargetId;
+import google.registry.flows.ResourceFlowUtils;
 import google.registry.flows.annotations.ReportingSpec;
 import google.registry.flows.exceptions.NoTransferHistoryToQueryException;
 import google.registry.flows.exceptions.NotAuthorizedToViewTransferException;
@@ -86,10 +86,12 @@ public final class DomainTransferQueryFlow implements Flow {
       throw new NotAuthorizedToViewTransferException();
     }
     DateTime newExpirationTime = null;
-    if (transferData.getTransferStatus().isApproved()
-        || transferData.getTransferStatus().equals(TransferStatus.PENDING)) {
-      // TODO(b/25084229): fix exDate computation logic.
-      newExpirationTime = extendRegistrationWithCap(now, domain.getRegistrationExpirationTime(), 1);
+    if (transferData.getTransferStatus().isApproved()) {
+      newExpirationTime = transferData.getTransferredRegistrationExpirationTime();
+    } else if (transferData.getTransferStatus().equals(TransferStatus.PENDING)) {
+      newExpirationTime =
+          ResourceFlowUtils.computeExDateForApprovalTime(
+              domain, now, domain.getTransferData().getTransferPeriod());
     }
     return responseBuilder
         .setResData(createTransferResponse(targetId, transferData, newExpirationTime))

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

@@ -15,6 +15,7 @@
 package google.registry.flows.domain;
 
 import static google.registry.flows.FlowUtils.validateClientIsLoggedIn;
+import static google.registry.flows.ResourceFlowUtils.computeExDateForApprovalTime;
 import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence;
 import static google.registry.flows.ResourceFlowUtils.verifyAuthInfo;
 import static google.registry.flows.ResourceFlowUtils.verifyAuthInfoPresentForResourceTransfer;
@@ -29,7 +30,6 @@ import static google.registry.flows.domain.DomainTransferUtils.createLosingTrans
 import static google.registry.flows.domain.DomainTransferUtils.createPendingTransferData;
 import static google.registry.flows.domain.DomainTransferUtils.createTransferResponse;
 import static google.registry.flows.domain.DomainTransferUtils.createTransferServerApproveEntities;
-import static google.registry.model.domain.DomainBase.extendRegistrationWithCap;
 import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACTION_PENDING;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.model.transaction.TransactionManagerFactory.tm;
@@ -56,7 +56,6 @@ import google.registry.model.domain.Period;
 import google.registry.model.domain.fee.FeeTransferCommandExtension;
 import google.registry.model.domain.fee.FeeTransformResponseExtension;
 import google.registry.model.domain.metadata.MetadataExtension;
-import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.domain.superuser.DomainTransferRequestSuperuserExtension;
 import google.registry.model.eppcommon.AuthInfo;
 import google.registry.model.eppcommon.StatusValue;
@@ -183,18 +182,10 @@ public final class DomainTransferRequestFlow implements TransactionalFlow {
     //
     // See b/19430703#comment17 and https://www.icann.org/news/advisory-2002-06-06-en for the
     // policy documentation for transfers subsuming autorenews within the autorenew grace period.
-    int extraYears = period.getValue();
-    DomainBase domainAtTransferTime =
-        existingDomain.cloneProjectedAtTime(automaticTransferTime);
-    if (!domainAtTransferTime.getGracePeriodsOfType(GracePeriodStatus.AUTO_RENEW).isEmpty()) {
-      extraYears = 0;
-    }
+    DomainBase domainAtTransferTime = existingDomain.cloneProjectedAtTime(automaticTransferTime);
     // The new expiration time if there is a server approval.
     DateTime serverApproveNewExpirationTime =
-        extendRegistrationWithCap(
-            automaticTransferTime,
-            domainAtTransferTime.getRegistrationExpirationTime(),
-            extraYears);
+        computeExDateForApprovalTime(domainAtTransferTime, automaticTransferTime, period);
     // Create speculative entities in anticipation of an automatic server approval.
     ImmutableSet<TransferServerApproveEntity> serverApproveEntities =
         createTransferServerApproveEntities(
@@ -337,10 +328,8 @@ public final class DomainTransferRequestFlow implements TransactionalFlow {
     // If the registration were approved this instant, this is what the new expiration would be,
     // because we cap at 10 years from the moment of approval. This is different than the server
     // approval new expiration time, which is capped at 10 years from the server approve time.
-    DateTime approveNowExtendedRegistrationTime = extendRegistrationWithCap(
-        now,
-        existingDomain.getRegistrationExpirationTime(),
-        period.getValue());
+    DateTime approveNowExtendedRegistrationTime =
+        computeExDateForApprovalTime(existingDomain, now, period);
     return createTransferResponse(
         targetId, newDomain.getTransferData(), approveNowExtendedRegistrationTime);
   }

core/src/main/java/google/registry/model/ResourceTransferUtils.java

@@ -17,7 +17,6 @@ package google.registry.model;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
-import static google.registry.model.domain.DomainBase.extendRegistrationWithCap;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 
 import com.google.common.collect.ImmutableList;
@@ -53,24 +52,22 @@ public final class ResourceTransferUtils {
       TransferStatus.PENDING, TransferStatus.CLIENT_APPROVED, TransferStatus.SERVER_APPROVED);
 
   /**
-   * Create a transfer response using the id and type of this resource and the specified
-   * {@link TransferData}.
+   * Create a transfer response using the id and type of this resource and the specified {@link
+   * TransferData}.
    */
   public static TransferResponse createTransferResponse(
-      EppResource eppResource, TransferData transferData, DateTime now) {
+      EppResource eppResource, TransferData transferData) {
     assertIsContactOrDomain(eppResource);
     TransferResponse.Builder<? extends TransferResponse, ?> builder;
     if (eppResource instanceof ContactResource) {
       builder = new ContactTransferResponse.Builder().setContactId(eppResource.getForeignKey());
     } else {
-      DomainBase domain = (DomainBase) eppResource;
       builder =
           new DomainTransferResponse.Builder()
               .setFullyQualifiedDomainName(eppResource.getForeignKey())
-              // TODO(b/25084229): fix exDate computation logic.
               .setExtendedRegistrationExpirationTime(
                   ADD_EXDATE_STATUSES.contains(transferData.getTransferStatus())
-                      ? extendRegistrationWithCap(now, domain.getRegistrationExpirationTime(), 1)
+                      ? transferData.getTransferredRegistrationExpirationTime()
                       : null);
     }
     builder.setGainingClientId(transferData.getGainingClientId())
@@ -118,16 +115,20 @@ public final class ResourceTransferUtils {
     if (resource.getStatusValues().contains(StatusValue.PENDING_TRANSFER)) {
       TransferData oldTransferData = resource.getTransferData();
       ofy().delete().keys(oldTransferData.getServerApproveEntities());
-      ofy().save().entity(new PollMessage.OneTime.Builder()
-          .setClientId(oldTransferData.getGainingClientId())
-          .setEventTime(now)
-          .setMsg(TransferStatus.SERVER_CANCELLED.getMessage())
-          .setResponseData(ImmutableList.of(
-              createTransferResponse(newResource, newResource.getTransferData(), now),
-              createPendingTransferNotificationResponse(
-                  resource, oldTransferData.getTransferRequestTrid(), false, now)))
-          .setParent(historyEntry)
-          .build());
+      ofy()
+          .save()
+          .entity(
+              new PollMessage.OneTime.Builder()
+                  .setClientId(oldTransferData.getGainingClientId())
+                  .setEventTime(now)
+                  .setMsg(TransferStatus.SERVER_CANCELLED.getMessage())
+                  .setResponseData(
+                      ImmutableList.of(
+                          createTransferResponse(newResource, newResource.getTransferData()),
+                          createPendingTransferNotificationResponse(
+                              resource, oldTransferData.getTransferRequestTrid(), false, now)))
+                  .setParent(historyEntry)
+                  .build());
     }
   }
 

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

@@ -45,6 +45,7 @@ import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.IgnoreSave;
 import com.googlecode.objectify.annotation.Index;
 import com.googlecode.objectify.condition.IfNull;
+import google.registry.flows.ResourceFlowUtils;
 import google.registry.model.EppResource;
 import google.registry.model.EppResource.ForeignKeyedEppResource;
 import google.registry.model.EppResource.ResourceWithTransferData;
@@ -377,28 +378,31 @@ public class DomainBase extends EppResource
       // exclusive of their ending), and we can just proceed with the transfer.
       DomainBase domainAtTransferTime =
           cloneProjectedAtTime(transferExpirationTime.minusMillis(1));
+
+      DateTime expirationDate = transferData.getTransferredRegistrationExpirationTime();
+      if (expirationDate == null) {
+        // Extend the registration by the correct number of years from the expiration time
+        // that was current on the domain right before the transfer, capped at 10 years from
+        // the moment of the transfer.
+        expirationDate =
+            ResourceFlowUtils.computeExDateForApprovalTime(
+                domainAtTransferTime, transferExpirationTime, transferData.getTransferPeriod());
+      }
       // If we are within an autorenew grace period, the transfer will subsume the autorenew. There
       // will already be a cancellation written in advance by the transfer request flow, so we don't
       // need to worry about billing, but we do need to cancel out the expiration time increase.
       // The transfer period saved in the transfer data will be one year, unless the superuser
       // extension set the transfer period to zero.
-      int extraYears = transferData.getTransferPeriod().getValue();
-      if (domainAtTransferTime.getGracePeriodStatuses().contains(GracePeriodStatus.AUTO_RENEW)) {
-        extraYears = 0;
-      }
       // Set the expiration, autorenew events, and grace period for the transfer. (Transfer ends
       // all other graces).
-      Builder builder = domainAtTransferTime.asBuilder()
-          // Extend the registration by the correct number of years from the expiration time that
-          // was current on the domain right before the transfer, capped at 10 years from the
-          // moment of the transfer.
-          .setRegistrationExpirationTime(extendRegistrationWithCap(
-              transferExpirationTime,
-              domainAtTransferTime.getRegistrationExpirationTime(),
-              extraYears))
-          // Set the speculatively-written new autorenew events as the domain's autorenew events.
-          .setAutorenewBillingEvent(transferData.getServerApproveAutorenewEvent())
-          .setAutorenewPollMessage(transferData.getServerApproveAutorenewPollMessage());
+      Builder builder =
+          domainAtTransferTime
+              .asBuilder()
+              .setRegistrationExpirationTime(expirationDate)
+              // Set the speculatively-written new autorenew events as the domain's autorenew
+              // events.
+              .setAutorenewBillingEvent(transferData.getServerApproveAutorenewEvent())
+              .setAutorenewPollMessage(transferData.getServerApproveAutorenewPollMessage());
       if (transferData.getTransferPeriod().getValue() == 1) {
         // Set the grace period using a key to the prescheduled transfer billing event.  Not using
         // GracePeriod.forBillingEvent() here in order to avoid the actual Datastore fetch.

core/src/main/java/google/registry/rde/DomainBaseToXjcConverter.java

@@ -30,7 +30,6 @@ import google.registry.model.domain.secdns.DelegationSignerData;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.rde.RdeMode;
 import google.registry.model.transfer.TransferData;
-import google.registry.model.transfer.TransferStatus;
 import google.registry.util.Idn;
 import google.registry.xjc.domain.XjcDomainContactAttrType;
 import google.registry.xjc.domain.XjcDomainContactType;
@@ -45,7 +44,6 @@ import google.registry.xjc.rgp.XjcRgpStatusType;
 import google.registry.xjc.rgp.XjcRgpStatusValueType;
 import google.registry.xjc.secdns.XjcSecdnsDsDataType;
 import google.registry.xjc.secdns.XjcSecdnsDsOrKeyType;
-import org.joda.time.DateTime;
 
 /** Utility class that turns {@link DomainBase} as {@link XjcRdeDomainElement}. */
 final class DomainBaseToXjcConverter {
@@ -241,8 +239,7 @@ final class DomainBaseToXjcConverter {
           // empty transfer records to get generated for deleted domains.
           // TODO(b/33289763): remove the hasGainingAndLosingRegistrars check in February 2017
           if (hasGainingAndLosingRegistrars(model)) {
-            bean.setTrnData(convertTransferData(model.getTransferData(),
-                model.getRegistrationExpirationTime()));
+            bean.setTrnData(convertTransferData(model.getTransferData()));
           }
         }
 
@@ -261,8 +258,7 @@ final class DomainBaseToXjcConverter {
   }
 
   /** Converts {@link TransferData} to {@link XjcRdeDomainTransferDataType}. */
-  private static XjcRdeDomainTransferDataType convertTransferData(
-      TransferData model, DateTime domainExpires) {
+  private static XjcRdeDomainTransferDataType convertTransferData(TransferData model) {
     XjcRdeDomainTransferDataType bean = new XjcRdeDomainTransferDataType();
     bean.setTrStatus(
         XjcEppcomTrStatusType.fromValue(model.getTransferStatus().getXmlName()));
@@ -270,12 +266,7 @@ final class DomainBaseToXjcConverter {
     bean.setAcRr(RdeUtil.makeXjcRdeRrType(model.getLosingClientId()));
     bean.setReDate(model.getTransferRequestTime());
     bean.setAcDate(model.getPendingTransferExpirationTime());
-    // TODO(b/25084229): fix exDate computation logic.
-    if (model.getTransferStatus() == TransferStatus.PENDING) {
-      bean.setExDate(domainExpires.plusYears(1));
-    } else {
-      bean.setExDate(domainExpires);
-    }
+    bean.setExDate(model.getTransferredRegistrationExpirationTime());
     return bean;
   }