Skip to content

Commit

Permalink
Remove obsolete TransferData.extendedRegistrationYears
Browse files Browse the repository at this point in the history
Now that transfers are always restricted to 1 year, it's unnecessary to store
extendedRegistrationYears on TransferData - it will always be equal to 1.  This
simplifies logic in a few other places, e.g. RdeDomainImportAction.

I verified in BigQuery that no DomainBases exist with extendedRegistrationYears
values that aren't either null or equal to 1.  At some point we should remove
the persisted fields from datastore via e.g. resaving all those domains, but
it's low priority and can wait until we have some more pressing migration.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=150373897
  • Loading branch information
nfelt authored and CydeWeys committed Mar 21, 2017
1 parent 70fbdcc commit 09f619c
Show file tree
Hide file tree
Showing 29 changed files with 54 additions and 215 deletions.
18 changes: 8 additions & 10 deletions java/google/registry/flows/ResourceFlowUtils.java
Expand Up @@ -96,15 +96,14 @@ public static TransferResponse createTransferResponse(
builder = new ContactTransferResponse.Builder().setContactId(eppResource.getForeignKey());
} else {
DomainResource domain = (DomainResource) eppResource;
builder = new DomainTransferResponse.Builder()
.setFullyQualifiedDomainNameName(eppResource.getForeignKey())
.setExtendedRegistrationExpirationTime(
ADD_EXDATE_STATUSES.contains(transferData.getTransferStatus())
? extendRegistrationWithCap(
now,
domain.getRegistrationExpirationTime(),
transferData.getExtendedRegistrationYears())
: null);
builder =
new DomainTransferResponse.Builder()
.setFullyQualifiedDomainNameName(eppResource.getForeignKey())
// TODO(b/25084229): fix exDate computation logic.
.setExtendedRegistrationExpirationTime(
ADD_EXDATE_STATUSES.contains(transferData.getTransferStatus())
? extendRegistrationWithCap(now, domain.getRegistrationExpirationTime(), 1)
: null);
}
builder.setGainingClientId(transferData.getGainingClientId())
.setLosingClientId(transferData.getLosingClientId())
Expand Down Expand Up @@ -218,7 +217,6 @@ public static TransferData createResolvedTransferData(
TransferData oldTransferData, TransferStatus transferStatus, DateTime now) {
checkArgument(!oldTransferData.equals(TransferData.EMPTY), "No old transfer to resolve.");
return oldTransferData.asBuilder()
.setExtendedRegistrationYears(null)
.setServerApproveEntities(null)
.setServerApproveBillingEvent(null)
.setServerApproveAutorenewEvent(null)
Expand Down
10 changes: 5 additions & 5 deletions java/google/registry/flows/domain/DomainTransferApproveFlow.java
Expand Up @@ -108,20 +108,20 @@ public final EppResponse run() throws EppException {
.setOtherClientId(gainingClientId)
.setParent(Key.create(existingDomain))
.build();
int extraYears = transferData.getExtendedRegistrationYears();
// Bill for the transfer.
BillingEvent.OneTime billingEvent = new BillingEvent.OneTime.Builder()
.setReason(Reason.TRANSFER)
.setTargetId(targetId)
.setClientId(gainingClientId)
.setPeriodYears(extraYears)
.setCost(getDomainRenewCost(targetId, transferData.getTransferRequestTime(), extraYears))
.setPeriodYears(1)
.setCost(getDomainRenewCost(targetId, transferData.getTransferRequestTime(), 1))
.setEventTime(now)
.setBillingTime(now.plus(Registry.get(tld).getTransferGracePeriodLength()))
.setParent(historyEntry)
.build();
// If we are within an autorenew grace period, cancel the autorenew billing event and reduce
// the number of years to extend the registration by one to "subsume" the autorenew.
// If we are within an autorenew grace period, cancel the autorenew billing event and don't
// increase the registration time, since the transfer subsumes the autorenew's extra year.
int extraYears = 1; // All transfers are one year.
GracePeriod autorenewGrace =
getOnlyElement(existingDomain.getGracePeriodsOfType(GracePeriodStatus.AUTO_RENEW), null);
if (autorenewGrace != null) {
Expand Down
Expand Up @@ -85,11 +85,8 @@ public final EppResponse run() throws EppException {
DateTime newExpirationTime = null;
if (transferData.getTransferStatus().isApproved()
|| transferData.getTransferStatus().equals(TransferStatus.PENDING)) {
// TODO(b/25084229): This is not quite right.
newExpirationTime = extendRegistrationWithCap(
now,
domain.getRegistrationExpirationTime(),
transferData.getExtendedRegistrationYears());
// TODO(b/25084229): fix exDate computation logic.
newExpirationTime = extendRegistrationWithCap(now, domain.getRegistrationExpirationTime(), 1);
}
return responseBuilder
.setResData(createTransferResponse(targetId, transferData, newExpirationTime))
Expand Down
20 changes: 8 additions & 12 deletions java/google/registry/flows/domain/DomainTransferRequestFlow.java
Expand Up @@ -128,7 +128,6 @@ public final EppResponse run() throws EppException {
extensionManager.validate();
validateClientIsLoggedIn(gainingClientId);
Period period = ((Transfer) resourceCommand).getPeriod();
int years = period.getValue();
DateTime now = ofy().getTransactionTime();
DomainResource existingDomain = loadAndVerifyExistence(DomainResource.class, targetId, now);
verifyTransferAllowed(existingDomain, period, now);
Expand All @@ -143,25 +142,24 @@ public final EppResponse run() throws EppException {
DateTime automaticTransferTime = now.plus(registry.getAutomaticTransferLength());

// If the domain will be in the auto-renew grace period at the moment of transfer, the transfer
// will subsume the autorenew, so we reduce by 1 the number of years to extend the registration.
// Note that the regular "years" remains the same since it affects the transfer charge amount.
// The gaining registrar is still billed for the full years; the losing registrar will get a
// will subsume the autorenew, so we don't add the normal extra year from the transfer.
// The gaining registrar is still billed for the extra year; the losing registrar will get a
// cancellation for the autorenew written out within createTransferServerApproveEntities().
//
// 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 registrationExtensionYears = years;
int extraYears = 1;
DomainResource domainAtTransferTime =
existingDomain.cloneProjectedAtTime(automaticTransferTime);
if (!domainAtTransferTime.getGracePeriodsOfType(GracePeriodStatus.AUTO_RENEW).isEmpty()) {
registrationExtensionYears--;
extraYears--;
}
// The new expiration time if there is a server approval.
DateTime serverApproveNewExpirationTime =
extendRegistrationWithCap(
automaticTransferTime,
domainAtTransferTime.getRegistrationExpirationTime(),
registrationExtensionYears);
extraYears);
// Create speculative entities in anticipation of an automatic server approval.
ImmutableSet<TransferServerApproveEntity> serverApproveEntities =
createTransferServerApproveEntities(
Expand All @@ -172,11 +170,10 @@ public final EppResponse run() throws EppException {
trid,
gainingClientId,
feesAndCredits.getTotalCost(),
years,
now);
// Create the transfer data that represents the pending transfer.
TransferData pendingTransferData = createPendingTransferData(
createTransferDataBuilder(existingDomain, automaticTransferTime, years, now),
createTransferDataBuilder(existingDomain, automaticTransferTime, now),
serverApproveEntities);
// Create a poll message to notify the losing registrar that a transfer was requested.
PollMessage requestPollMessage = createLosingTransferPollMessage(
Expand Down Expand Up @@ -262,14 +259,13 @@ private HistoryEntry buildHistory(Period period, DomainResource existingResource
}

private Builder createTransferDataBuilder(
DomainResource existingDomain, DateTime automaticTransferTime, int years, DateTime now) {
DomainResource existingDomain, DateTime automaticTransferTime, DateTime now) {
return new TransferData.Builder()
.setTransferRequestTrid(trid)
.setTransferRequestTime(now)
.setGainingClientId(gainingClientId)
.setLosingClientId(existingDomain.getCurrentSponsorClientId())
.setPendingTransferExpirationTime(automaticTransferTime)
.setExtendedRegistrationYears(years);
.setPendingTransferExpirationTime(automaticTransferTime);
}

private DomainTransferResponse createResponse(
Expand Down
15 changes: 5 additions & 10 deletions java/google/registry/flows/domain/DomainTransferUtils.java
Expand Up @@ -92,13 +92,12 @@ public static ImmutableSet<TransferServerApproveEntity> createTransferServerAppr
Trid trid,
String gainingClientId,
Money transferCost,
int years,
DateTime now) {
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, years, now)
existingDomain, trid, gainingClientId, automaticTransferTime, now)
.setTransferStatus(TransferStatus.SERVER_APPROVED)
.build();
Registry registry = Registry.get(existingDomain.getTld());
Expand All @@ -110,8 +109,7 @@ public static ImmutableSet<TransferServerApproveEntity> createTransferServerAppr
targetId,
gainingClientId,
registry,
transferCost,
years))
transferCost))
.addAll(
createOptionalAutorenewCancellation(
automaticTransferTime, historyEntry, targetId, existingDomain)
Expand Down Expand Up @@ -255,14 +253,13 @@ private static BillingEvent.OneTime createTransferBillingEvent(
String targetId,
String gainingClientId,
Registry registry,
Money transferCost,
int years) {
Money transferCost) {
return new BillingEvent.OneTime.Builder()
.setReason(Reason.TRANSFER)
.setTargetId(targetId)
.setClientId(gainingClientId)
.setCost(transferCost)
.setPeriodYears(years)
.setPeriodYears(1)
.setEventTime(automaticTransferTime)
.setBillingTime(automaticTransferTime.plus(registry.getTransferGracePeriodLength()))
.setParent(historyEntry)
Expand All @@ -274,15 +271,13 @@ private static Builder createTransferDataBuilder(
Trid trid,
String gainingClientId,
DateTime automaticTransferTime,
int years,
DateTime now) {
return new TransferData.Builder()
.setTransferRequestTrid(trid)
.setTransferRequestTime(now)
.setGainingClientId(gainingClientId)
.setLosingClientId(existingDomain.getCurrentSponsorClientId())
.setPendingTransferExpirationTime(automaticTransferTime)
.setExtendedRegistrationYears(years);
.setPendingTransferExpirationTime(automaticTransferTime);
}

private DomainTransferUtils() {}
Expand Down
5 changes: 2 additions & 3 deletions java/google/registry/model/domain/DomainResource.java
Expand Up @@ -247,9 +247,8 @@ && isBeforeOrAt(transferExpirationTime, now)) {
cloneProjectedAtTime(transferExpirationTime.minusMillis(1));
// 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 reduce the number of years added to the
// expiration time by one to account for the year added by the autorenew.
int extraYears = transferData.getExtendedRegistrationYears();
// need to worry about billing, but we do need to cancel out the expiration time increase.
int extraYears = 1; // All transfers are one year.
if (domainAtTransferTime.getGracePeriodStatuses().contains(GracePeriodStatus.AUTO_RENEW)) {
extraYears--;
}
Expand Down
16 changes: 0 additions & 16 deletions java/google/registry/model/transfer/TransferData.java
Expand Up @@ -81,12 +81,6 @@ public class TransferData extends BaseTransferObject implements Buildable {
/** The transaction id of the most recent transfer request (or null if there never was one). */
Trid transferRequestTrid;

/**
* The number of years to add to the registration expiration time if this transfer is approved.
* Can be null if never transferred, or for resource types where it's not applicable.
*/
Integer extendedRegistrationYears;

public ImmutableSet<Key<? extends TransferServerApproveEntity>> getServerApproveEntities() {
return nullToEmptyImmutableCopy(serverApproveEntities);
}
Expand All @@ -107,10 +101,6 @@ public Trid getTransferRequestTrid() {
return transferRequestTrid;
}

public Integer getExtendedRegistrationYears() {
return extendedRegistrationYears;
}

@Override
public Builder asBuilder() {
return new Builder(clone(this));
Expand Down Expand Up @@ -155,12 +145,6 @@ public Builder setTransferRequestTrid(Trid transferRequestTrid) {
getInstance().transferRequestTrid = transferRequestTrid;
return this;
}

/** Set the years to add to the registration if this transfer completes. */
public Builder setExtendedRegistrationYears(Integer extendedRegistrationYears) {
getInstance().extendedRegistrationYears = extendedRegistrationYears;
return thisCastToDerived();
}
}

/**
Expand Down
5 changes: 2 additions & 3 deletions java/google/registry/rde/DomainResourceToXjcConverter.java
Expand Up @@ -17,7 +17,6 @@
import static google.registry.model.ofy.ObjectifyService.ofy;

import com.google.common.base.Ascii;
import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.googlecode.objectify.Key;
Expand Down Expand Up @@ -257,9 +256,9 @@ private static XjcRdeDomainTransferDataType convertTransferData(
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) {
int years = Optional.fromNullable(model.getExtendedRegistrationYears()).or(0);
bean.setExDate(domainExpires.plusYears(years));
bean.setExDate(domainExpires.plusYears(1));
} else {
bean.setExDate(domainExpires);
}
Expand Down
6 changes: 2 additions & 4 deletions java/google/registry/rde/imports/RdeDomainImportAction.java
Expand Up @@ -189,19 +189,17 @@ public void vrun() {
Money transferCost = getDomainRenewCost(
domain.getFullyQualifiedDomainName(),
transferData.getPendingTransferExpirationTime(),
transferData.getExtendedRegistrationYears());
1);
// Create speculative entities in anticipation of an automatic server approval.
ImmutableSet<TransferServerApproveEntity> serverApproveEntities =
createTransferServerApproveEntities(
transferData.getPendingTransferExpirationTime(),
domain.getRegistrationExpirationTime()
.plusYears(transferData.getExtendedRegistrationYears()),
domain.getRegistrationExpirationTime().plusYears(1),
historyEntry,
domain,
historyEntry.getTrid(),
transferData.getGainingClientId(),
transferCost,
transferData.getExtendedRegistrationYears(),
transferData.getTransferRequestTime());
transferData =
createPendingTransferData(transferData.asBuilder(), serverApproveEntities);
Expand Down
26 changes: 4 additions & 22 deletions java/google/registry/rde/imports/XjcToDomainResourceConverter.java
Expand Up @@ -60,7 +60,6 @@
import java.security.SecureRandom;
import java.util.Random;
import org.joda.time.DateTime;
import org.joda.time.Years;

/** Utility class that converts an {@link XjcRdeDomainElement} into a {@link DomainResource}. */
final class XjcToDomainResourceConverter extends XjcToEppResourceConverter {
Expand Down Expand Up @@ -208,7 +207,7 @@ static DomainResource convertDomain(
? ImmutableSet.<DelegationSignerData>of()
: ImmutableSet.copyOf(
transform(domain.getSecDNS().getDsDatas(), SECDNS_CONVERTER)))
.setTransferData(convertDomainTransferData(domain.getTrnData(), domain.getExDate()))
.setTransferData(convertDomainTransferData(domain.getTrnData()))
// authInfo pw must be a token between 6 and 16 characters in length
// generate a token of 16 characters as the default authInfo pw
.setAuthInfo(DomainAuthInfo
Expand Down Expand Up @@ -241,34 +240,17 @@ private static ImmutableSet<Key<HostResource>> convertNameservers(XjcDomainNsTyp
}

/** Converts {@link XjcRdeDomainTransferDataType} to {@link TransferData}. */
private static TransferData convertDomainTransferData(XjcRdeDomainTransferDataType data,
DateTime domainExpiration) {
private static TransferData convertDomainTransferData(XjcRdeDomainTransferDataType data) {
if (data == null) {
return TransferData.EMPTY;
}
// If the transfer is pending, calculate the number of years to add to the domain expiration
// on approval of the transfer.
TransferStatus transferStatus = TRANSFER_STATUS_MAPPER.xmlToEnum(data.getTrStatus().value());
// Get new expiration date
DateTime newExpirationTime = domainExpiration;
if (transferStatus == TransferStatus.PENDING) {
// Default to domain expiration time plus one year if no expiration is specified
if (data.getExDate() == null) {
newExpirationTime = newExpirationTime.plusYears(1);
} else {
newExpirationTime = data.getExDate();
}
}
// TODO(b/25084229): read in the exDate and store it somewhere.
return new TransferData.Builder()
.setTransferStatus(transferStatus)
.setTransferStatus(TRANSFER_STATUS_MAPPER.xmlToEnum(data.getTrStatus().value()))
.setGainingClientId(data.getReRr().getValue())
.setLosingClientId(data.getAcRr().getValue())
.setTransferRequestTime(data.getReDate())
.setPendingTransferExpirationTime(data.getAcDate())
// This will be wrong for domains that are not in pending transfer,
// but there isn't a reliable way to calculate it.
.setExtendedRegistrationYears(
Years.yearsBetween(domainExpiration, newExpirationTime).getYears())
.build();
}

Expand Down
Expand Up @@ -298,18 +298,15 @@ public void testSuccess_autorenewBeforeTransfer() throws Exception {
DateTime oldExpirationTime = clock.nowUtc().minusDays(1);
persistResource(domain.asBuilder()
.setRegistrationExpirationTime(oldExpirationTime)
.setTransferData(domain.getTransferData().asBuilder()
.setExtendedRegistrationYears(2)
.build())
.build());
// The autorenew should be subsumed into the transfer resulting in 2 years of renewal in total.
// The autorenew should be subsumed into the transfer resulting in 1 year of renewal in total.
clock.advanceOneMilli();
doSuccessfulTest(
"tld",
"domain_transfer_approve_domain_authinfo.xml",
"domain_transfer_approve_response_autorenew.xml",
oldExpirationTime.plusYears(2),
2,
oldExpirationTime.plusYears(1),
1,
// Expect the grace period for autorenew to be cancelled.
new BillingEvent.Cancellation.Builder()
.setReason(Reason.RENEW)
Expand Down
Expand Up @@ -101,7 +101,6 @@ static DomainResource persistWithPendingTransfer(DomainResource domain) {
TRANSFER_REQUEST_TIME,
TRANSFER_EXPIRATION_TIME,
EXTENDED_REGISTRATION_EXPIRATION_TIME,
EXTENDED_REGISTRATION_YEARS,
TRANSFER_REQUEST_TIME);
}

Expand Down Expand Up @@ -176,8 +175,7 @@ protected BillingEvent.OneTime getBillingEventForImplicitTransfer() {
domain,
historyEntry,
TRANSFER_REQUEST_TIME,
TRANSFER_EXPIRATION_TIME,
EXTENDED_REGISTRATION_YEARS);
TRANSFER_EXPIRATION_TIME);
}

/** Get the autorenew event that the losing client will have after a SERVER_APPROVED transfer. */
Expand Down
Expand Up @@ -148,10 +148,10 @@ public void testSuccess_serverCancelled() throws Exception {

@Test
public void testSuccess_tenYears() throws Exception {
// Extend registration by 9 years here; with the extra 1 year from the transfer, we should
// hit the 10-year capping.
domain = persistResource(domain.asBuilder()
.setTransferData(domain.getTransferData().asBuilder()
.setExtendedRegistrationYears(10)
.build())
.setRegistrationExpirationTime(domain.getRegistrationExpirationTime().plusYears(9))
.build());
doSuccessfulTest(
"domain_transfer_query.xml",
Expand Down

0 comments on commit 09f619c

Please sign in to comment.