Persist transferredRegistrationExpirationTime (exDate) on TransferData

This CL adds transferredRegistrationExpirationTime as a TransferData field
persisted to Datastore.  It's only relevant for domains, and it represents the
registration expiration time resulting from the approval of the most recent
transfer request.  For pending transfers, we assume the transfer will be
server-approved, and thus in DomainTransferRequestFlow we set this field to the
existing computed value serverApproveNewExpirationTime, which is what we use
for setting up the server-approve autorenew billing event and poll message.
In DomainTransferApproveFlow we overwrite this field with the freshly computed
newExpirationTime, whereas in DomainTransferCancel/RejectFlow (and in the
implicit cancel of DomainDeleteFlow during a pending transfer) we null it out.

There are two key benefits to having this field, which are described in more
detail in b/36405140.

1) b/25084229 - it allows storage of a frozen value to back the "exDate" field
   of DomainTransferResponse, which we can use to fix various errors with how
   exDate display currently works.

2) b/36354434 - it allows DomainResource.cloneProjectedAtTime() to just directly
   set the registrationExpirationTime to this value, without computing it de
   novo, which reduces duplicated logic and ensures that the new expiration time
   matches the autorenew child objects.

This CL only starts writing the field on TransferData as persisted directly on
the DomainResource itself.  We'll then want to backfill the field for at
least pending transfers, whether expired or not (so we can do (2) above), but
I think we might as well backfill it for all pending and approved transfers
so that we also fix (1) even for historical transfers.  And then we can start
actually reading the field for both purposes.  (Note that for (1), this will
only fix synchronous transfer responses served via DomainTransferQueryFlow,
not async transfer responses served via poll messages, since these have already
been persisted with a potentially bad exDate, but I don't think it's worth a
backfill for those).

One last naming note: I chose the verbose transferredRegistrationExpirationTime
rather than the extendedRegistrationExpirationTime of DomainTransferResponse
because (as is the case in autorenew grace, or for a superuser transfer) the
new registration time isn't necessarily extended at all; it may be the same as
the pre-transfer expiration time.  Also, including "registration" helps clarify
w.r.t. pendingTransferExpirationTime which refers confusingly to the expiry of
the transfer itself, rather than the domain registration.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=171858083
This commit is contained in:
nickfelt 2017-10-11 12:23:32 -07:00 committed by jianglai
parent 680abcb343
commit 184b2b56ac
12 changed files with 132 additions and 84 deletions

View file

@ -172,9 +172,18 @@ public final class DomainTransferApproveFlow implements TransactionalFlow {
.setMsg("Domain was auto-renewed.")
.setParent(historyEntry)
.build();
// Construct the post-transfer domain.
DomainResource partiallyApprovedDomain =
approvePendingTransfer(existingDomain, TransferStatus.CLIENT_APPROVED, now);
DomainResource newDomain =
approvePendingTransfer(existingDomain, TransferStatus.CLIENT_APPROVED, now)
partiallyApprovedDomain
.asBuilder()
// Update the transferredRegistrationExpirationTime here since approvePendingTransfer()
// doesn't know what to set it to and leaves it null.
.setTransferData(
partiallyApprovedDomain.getTransferData().asBuilder()
.setTransferredRegistrationExpirationTime(newExpirationTime)
.build())
.setRegistrationExpirationTime(newExpirationTime)
.setAutorenewBillingEvent(Key.create(autorenewEvent))
.setAutorenewPollMessage(Key.create(gainingClientAutorenewPollMessage))

View file

@ -69,7 +69,6 @@ import google.registry.model.reporting.DomainTransactionRecord.TransactionReport
import google.registry.model.reporting.HistoryEntry;
import google.registry.model.reporting.IcannReportingTypes.ActivityReportField;
import google.registry.model.transfer.TransferData;
import google.registry.model.transfer.TransferData.Builder;
import google.registry.model.transfer.TransferData.TransferServerApproveEntity;
import google.registry.model.transfer.TransferResponse.DomainTransferResponse;
import google.registry.model.transfer.TransferStatus;
@ -206,7 +205,13 @@ public final class DomainTransferRequestFlow implements TransactionalFlow {
// Create the transfer data that represents the pending transfer.
TransferData pendingTransferData =
createPendingTransferData(
createTransferDataBuilder(existingDomain, automaticTransferTime, now),
new TransferData.Builder()
.setTransferRequestTrid(trid)
.setTransferRequestTime(now)
.setGainingClientId(gainingClientId)
.setLosingClientId(existingDomain.getCurrentSponsorClientId())
.setPendingTransferExpirationTime(automaticTransferTime)
.setTransferredRegistrationExpirationTime(serverApproveNewExpirationTime),
serverApproveEntities,
period);
// Create a poll message to notify the losing registrar that a transfer was requested.
@ -316,16 +321,6 @@ public final class DomainTransferRequestFlow implements TransactionalFlow {
.build();
}
private Builder createTransferDataBuilder(
DomainResource existingDomain, DateTime automaticTransferTime, DateTime now) {
return new TransferData.Builder()
.setTransferRequestTrid(trid)
.setTransferRequestTime(now)
.setGainingClientId(gainingClientId)
.setLosingClientId(existingDomain.getCurrentSponsorClientId())
.setPendingTransferExpirationTime(automaticTransferTime);
}
private DomainTransferResponse createResponse(
Period period, DomainResource existingDomain, DomainResource newDomain, DateTime now) {
// If the registration were approved this instant, this is what the new expiration would be,

View file

@ -35,7 +35,6 @@ import google.registry.model.poll.PollMessage;
import google.registry.model.registry.Registry;
import google.registry.model.reporting.HistoryEntry;
import google.registry.model.transfer.TransferData;
import google.registry.model.transfer.TransferData.Builder;
import google.registry.model.transfer.TransferData.TransferServerApproveEntity;
import google.registry.model.transfer.TransferResponse.DomainTransferResponse;
import google.registry.model.transfer.TransferStatus;
@ -115,8 +114,13 @@ public final class DomainTransferUtils {
String targetId = existingDomain.getFullyQualifiedDomainName();
// Create a TransferData for the server-approve case to use for the speculative poll messages.
TransferData serverApproveTransferData =
createTransferDataBuilder(
existingDomain, trid, gainingClientId, automaticTransferTime, now)
new TransferData.Builder()
.setTransferRequestTrid(trid)
.setTransferRequestTime(now)
.setGainingClientId(gainingClientId)
.setLosingClientId(existingDomain.getCurrentSponsorClientId())
.setPendingTransferExpirationTime(automaticTransferTime)
.setTransferredRegistrationExpirationTime(serverApproveNewExpirationTime)
.setTransferStatus(TransferStatus.SERVER_APPROVED)
.build();
Registry registry = Registry.get(existingDomain.getTld());
@ -291,19 +295,5 @@ public final class DomainTransferUtils {
.build();
}
private static Builder createTransferDataBuilder(
DomainResource existingDomain,
Trid trid,
String gainingClientId,
DateTime automaticTransferTime,
DateTime now) {
return new TransferData.Builder()
.setTransferRequestTrid(trid)
.setTransferRequestTime(now)
.setGainingClientId(gainingClientId)
.setLosingClientId(existingDomain.getCurrentSponsorClientId())
.setPendingTransferExpirationTime(automaticTransferTime);
}
private DomainTransferUtils() {}
}

View file

@ -30,6 +30,8 @@ import google.registry.model.domain.Period.Unit;
import google.registry.model.eppcommon.Trid;
import google.registry.model.poll.PollMessage;
import java.util.Set;
import javax.annotation.Nullable;
import org.joda.time.DateTime;
/**
* Common transfer data for {@link EppResource} types. Only applies to domains and contacts;
@ -41,6 +43,34 @@ public class TransferData extends BaseTransferObject implements Buildable {
public static final TransferData EMPTY = new TransferData();
/** The transaction id of the most recent transfer request (or null if there never was one). */
Trid transferRequestTrid;
/**
* The period to extend the registration upon completion of the transfer.
*
* <p>By default, domain transfers are for one year. This can be changed to zero by using the
* superuser EPP extension.
*/
Period transferPeriod = Period.create(1, Unit.YEARS);
/**
* The registration expiration time resulting from the approval - speculative or actual - of the
* most recent transfer request, applicable for domains only.
*
* <p>For pending transfers, this is the expiration time that will take effect under a projected
* server approval. For approved transfers, this is the actual expiration time of the domain as
* of the moment of transfer completion. For rejected or cancelled transfers, this field will be
* reset to null.
*
* <p>Note that even when this field is set, it does not necessarily mean that the post-transfer
* domain has a new expiration time. Superuser transfers may not include a bundled 1 year renewal
* at all, or even when a renewal is bundled, for a transfer during the autorenew grace period the
* bundled renewal simply subsumes the recent autorenewal, resulting in the same expiration time.
*/
// TODO(b/36405140): backfill this field for existing domains to which it should apply.
DateTime transferredRegistrationExpirationTime;
/**
* The billing event and poll messages associated with a server-approved transfer.
*
@ -80,33 +110,7 @@ public class TransferData extends BaseTransferObject implements Buildable {
@IgnoreSave(IfNull.class)
Key<PollMessage.Autorenew> serverApproveAutorenewPollMessage;
/** The transaction id of the most recent transfer request (or null if there never was one). */
Trid transferRequestTrid;
/**
* The period to extend the registration upon completion of the transfer.
*
* <p>By default, domain transfers are for one year. This can be changed to zero by using the
* superuser EPP extension.
*/
Period transferPeriod = Period.create(1, Unit.YEARS);
public ImmutableSet<Key<? extends TransferServerApproveEntity>> getServerApproveEntities() {
return nullToEmptyImmutableCopy(serverApproveEntities);
}
public Key<BillingEvent.OneTime> getServerApproveBillingEvent() {
return serverApproveBillingEvent;
}
public Key<BillingEvent.Recurring> getServerApproveAutorenewEvent() {
return serverApproveAutorenewEvent;
}
public Key<PollMessage.Autorenew> getServerApproveAutorenewPollMessage() {
return serverApproveAutorenewPollMessage;
}
@Nullable
public Trid getTransferRequestTrid() {
return transferRequestTrid;
}
@ -115,6 +119,30 @@ public class TransferData extends BaseTransferObject implements Buildable {
return transferPeriod;
}
@Nullable
public DateTime getTransferredRegistrationExpirationTime() {
return transferredRegistrationExpirationTime;
}
public ImmutableSet<Key<? extends TransferServerApproveEntity>> getServerApproveEntities() {
return nullToEmptyImmutableCopy(serverApproveEntities);
}
@Nullable
public Key<BillingEvent.OneTime> getServerApproveBillingEvent() {
return serverApproveBillingEvent;
}
@Nullable
public Key<BillingEvent.Recurring> getServerApproveAutorenewEvent() {
return serverApproveAutorenewEvent;
}
@Nullable
public Key<PollMessage.Autorenew> getServerApproveAutorenewPollMessage() {
return serverApproveAutorenewPollMessage;
}
@Override
public Builder asBuilder() {
return new Builder(clone(this));
@ -153,6 +181,22 @@ public class TransferData extends BaseTransferObject implements Buildable {
super(instance);
}
public Builder setTransferRequestTrid(Trid transferRequestTrid) {
getInstance().transferRequestTrid = transferRequestTrid;
return this;
}
public Builder setTransferPeriod(Period transferPeriod) {
getInstance().transferPeriod = transferPeriod;
return this;
}
public Builder setTransferredRegistrationExpirationTime(
DateTime transferredRegistrationExpirationTime) {
getInstance().transferredRegistrationExpirationTime = transferredRegistrationExpirationTime;
return this;
}
public Builder setServerApproveEntities(
ImmutableSet<Key<? extends TransferServerApproveEntity>> serverApproveEntities) {
getInstance().serverApproveEntities = serverApproveEntities;
@ -176,16 +220,6 @@ public class TransferData extends BaseTransferObject implements Buildable {
getInstance().serverApproveAutorenewPollMessage = serverApproveAutorenewPollMessage;
return this;
}
public Builder setTransferRequestTrid(Trid transferRequestTrid) {
getInstance().transferRequestTrid = transferRequestTrid;
return this;
}
public Builder setTransferPeriod(Period transferPeriod) {
getInstance().transferPeriod = transferPeriod;
return this;
}
}
/**

View file

@ -236,13 +236,13 @@ final class XjcToDomainResourceConverter extends XjcToEppResourceConverter {
if (data == null) {
return TransferData.EMPTY;
}
// TODO(b/25084229): read in the exDate and store it somewhere.
return new TransferData.Builder()
.setTransferStatus(TRANSFER_STATUS_MAPPER.xmlToEnum(data.getTrStatus().value()))
.setGainingClientId(data.getReRr().getValue())
.setLosingClientId(data.getAcRr().getValue())
.setTransferRequestTime(data.getReDate())
.setPendingTransferExpirationTime(data.getAcDate())
.setTransferredRegistrationExpirationTime(data.getExDate())
.build();
}

View file

@ -116,6 +116,7 @@ public class DomainTransferApproveFlowTest
oldTransferData.copyConstantFieldsToBuilder()
.setTransferStatus(TransferStatus.CLIENT_APPROVED)
.setPendingTransferExpirationTime(clock.nowUtc())
.setTransferredRegistrationExpirationTime(domain.getRegistrationExpirationTime())
.build());
}

View file

@ -98,7 +98,8 @@ public class DomainTransferCancelFlowTest
"NewRegistrar",
TRANSFER_REQUEST_TIME,
TRANSFER_EXPIRATION_TIME,
TRANSFER_REQUEST_TIME));
TRANSFER_REQUEST_TIME,
EXTENDED_REGISTRATION_EXPIRATION_TIME));
assertPollMessages(
"TheRegistrar",
createPollMessageForImplicitTransfer(
@ -107,7 +108,8 @@ public class DomainTransferCancelFlowTest
"TheRegistrar",
TRANSFER_REQUEST_TIME,
TRANSFER_EXPIRATION_TIME,
TRANSFER_REQUEST_TIME));
TRANSFER_REQUEST_TIME,
EXTENDED_REGISTRATION_EXPIRATION_TIME));
clock.advanceOneMilli();
// Setup done; run the test.

View file

@ -130,8 +130,10 @@ public class DomainTransferRequestFlowTest
}
private void assertTransferRequested(
DomainResource domain, DateTime automaticTransferTime, Period expectedPeriod)
throws Exception {
DomainResource domain,
DateTime automaticTransferTime,
Period expectedPeriod,
DateTime expectedExpirationTime) throws Exception {
assertAboutDomains().that(domain)
.hasCurrentSponsorClientId("TheRegistrar").and()
.hasStatusValue(StatusValue.PENDING_TRANSFER);
@ -152,6 +154,7 @@ public class DomainTransferRequestFlowTest
.setTransferPeriod(expectedPeriod)
.setTransferStatus(TransferStatus.PENDING)
.setPendingTransferExpirationTime(automaticTransferTime)
.setTransferredRegistrationExpirationTime(expectedExpirationTime)
// Don't compare the server-approve entity fields; they're hard to reconstruct
// and logic later will check them.
.build());
@ -178,6 +181,7 @@ public class DomainTransferRequestFlowTest
.setTransferPeriod(expectedPeriod)
.setTransferStatus(TransferStatus.SERVER_APPROVED)
.setPendingTransferExpirationTime(automaticTransferTime)
.setTransferredRegistrationExpirationTime(domain.getRegistrationExpirationTime())
// Server-approve entity fields should all be nulled out.
.build());
}
@ -219,7 +223,8 @@ public class DomainTransferRequestFlowTest
.and()
.hasOtherClientId("TheRegistrar");
// Verify correct fields were set.
assertTransferRequested(domain, implicitTransferTime, Period.create(1, Unit.YEARS));
assertTransferRequested(
domain, implicitTransferTime, Period.create(1, Unit.YEARS), expectedExpirationTime);
subordinateHost = reloadResourceAndCloneAtTime(subordinateHost, clock.nowUtc());
assertAboutHosts().that(subordinateHost).hasNoHistoryEntries();
@ -530,7 +535,7 @@ public class DomainTransferRequestFlowTest
.and()
.hasOtherClientId("TheRegistrar");
// Verify correct fields were set.
assertTransferRequested(domain, implicitTransferTime, expectedPeriod);
assertTransferRequested(domain, implicitTransferTime, expectedPeriod, expectedExpirationTime);
subordinateHost = reloadResourceAndCloneAtTime(subordinateHost, clock.nowUtc());
assertAboutHosts().that(subordinateHost).hasNoHistoryEntries();

View file

@ -139,6 +139,7 @@ public class DomainResourceTest extends EntityTestCase {
.setTransferRequestTime(clock.nowUtc().plusDays(1))
.setTransferStatus(TransferStatus.SERVER_APPROVED)
.setTransferRequestTrid(Trid.create("client-trid", "server-trid"))
.setTransferredRegistrationExpirationTime(clock.nowUtc().plusYears(2))
.build())
.setDeletePollMessage(onetimePollKey)
.setAutorenewBillingEvent(recurringBillKey)

View file

@ -913,6 +913,7 @@ class google.registry.model.transfer.TransferData {
java.util.Set<com.googlecode.objectify.Key<? extends google.registry.model.transfer.TransferData$TransferServerApproveEntity>> serverApproveEntities;
org.joda.time.DateTime pendingTransferExpirationTime;
org.joda.time.DateTime transferRequestTime;
org.joda.time.DateTime transferredRegistrationExpirationTime;
}
class google.registry.model.transfer.TransferResponse$ContactTransferResponse {
google.registry.model.transfer.TransferStatus transferStatus;

View file

@ -164,6 +164,8 @@ public class RdapJsonFormatterTest {
.setTransferRequestTime(clock.nowUtc().minusDays(1))
.setLosingClientId("TheRegistrar")
.setPendingTransferExpirationTime(clock.nowUtc().plusYears(100))
.setTransferredRegistrationExpirationTime(
DateTime.parse("2111-10-08T00:44:59Z"))
.build())
.build())))
.build());

View file

@ -103,6 +103,7 @@ import google.registry.model.transfer.TransferStatus;
import google.registry.tmch.LordnTask;
import java.util.Arrays;
import java.util.List;
import javax.annotation.Nullable;
import org.joda.money.Money;
import org.joda.time.DateTime;
@ -457,20 +458,22 @@ public class DatastoreHelper {
}
public static PollMessage.OneTime createPollMessageForImplicitTransfer(
EppResource contact,
EppResource resource,
HistoryEntry historyEntry,
String clientId,
DateTime requestTime,
DateTime expirationTime,
DateTime now) {
DateTime now,
@Nullable DateTime extendedRegistrationExpirationTime) {
TransferData transferData =
createTransferDataBuilder(requestTime, expirationTime)
.setTransferredRegistrationExpirationTime(extendedRegistrationExpirationTime)
.build();
return new PollMessage.OneTime.Builder()
.setClientId(clientId)
.setEventTime(expirationTime)
.setMsg("Transfer server approved.")
.setResponseData(ImmutableList.of(
createTransferResponse(contact, createTransferDataBuilder(requestTime, expirationTime)
.build(),
now)))
.setResponseData(ImmutableList.of(createTransferResponse(resource, transferData, now)))
.setParent(historyEntry)
.build();
}
@ -519,7 +522,8 @@ public class DatastoreHelper {
"NewRegistrar",
requestTime,
expirationTime,
now))),
now,
null))),
Key.create(persistResource(
createPollMessageForImplicitTransfer(
contact,
@ -527,7 +531,8 @@ public class DatastoreHelper {
"TheRegistrar",
requestTime,
expirationTime,
now)))))
now,
null)))))
.setTransferRequestTrid(Trid.create("transferClient-trid", "transferServer-trid"))
.build())
.build());
@ -591,6 +596,7 @@ public class DatastoreHelper {
.addStatusValue(StatusValue.PENDING_TRANSFER)
.setTransferData(transferDataBuilder
.setPendingTransferExpirationTime(expirationTime)
.setTransferredRegistrationExpirationTime(extendedRegistrationExpirationTime)
.setServerApproveBillingEvent(Key.create(transferBillingEvent))
.setServerApproveAutorenewEvent(Key.create(gainingClientAutorenewEvent))
.setServerApproveAutorenewPollMessage(Key.create(gainingClientAutorenewPollMessage))
@ -605,7 +611,8 @@ public class DatastoreHelper {
"NewRegistrar",
requestTime,
expirationTime,
now))),
now,
extendedRegistrationExpirationTime))),
Key.create(persistResource(
createPollMessageForImplicitTransfer(
domain,
@ -613,7 +620,8 @@ public class DatastoreHelper {
"TheRegistrar",
requestTime,
expirationTime,
now)))))
now,
extendedRegistrationExpirationTime)))))
.setTransferRequestTrid(Trid.create("transferClient-trid", "transferServer-trid"))
.build())
.build());