From 84009eaccb8e1c5d011cfc5861aae90624bf678a Mon Sep 17 00:00:00 2001 From: cgoldfeder Date: Tue, 15 Nov 2016 08:03:25 -0800 Subject: [PATCH] Scope down TransferData to only ContactResource and DomainResource HostResource and DomainApplication are not transferable, (or at least, not directly in the case of hosts) and have no need for the TransferData field. In a flat-flow world, we can push it down to where it's actually used. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=139201423 --- .../registry/flows/ResourceFlowUtils.java | 93 ++++++++----------- .../async/DeleteContactsAndHostsAction.java | 23 ++++- .../domain/DomainApplicationDeleteFlow.java | 9 +- .../flows/domain/DomainDeleteFlow.java | 6 +- .../domain/DomainTransferCancelFlow.java | 6 +- java/google/registry/model/EppResource.java | 38 ++------ .../registry/model/EppResourceUtils.java | 12 ++- .../model/contact/ContactResource.java | 76 ++++++++++----- .../registry/model/domain/DomainResource.java | 28 +++++- .../registry/model/host/HostResource.java | 17 ++-- .../whitebox/VerifyEntityIntegrityAction.java | 30 +++--- .../domain/DomainTransferApproveFlowTest.java | 12 ++- .../domain/DomainTransferCancelFlowTest.java | 4 +- .../domain/DomainTransferFlowTestCase.java | 13 ++- .../domain/DomainTransferRejectFlowTest.java | 4 +- .../domain/DomainTransferRequestFlowTest.java | 24 ++++- .../model/contact/ContactResourceTest.java | 2 +- .../model/domain/DomainApplicationTest.java | 25 ----- .../model/domain/DomainResourceTest.java | 2 +- .../registry/model/host/HostResourceTest.java | 11 --- javatests/google/registry/model/schema.txt | 3 - .../testing/AbstractEppResourceSubject.java | 36 ------- .../testing/ContactResourceSubject.java | 38 ++++++++ .../testing/DomainResourceSubject.java | 37 ++++++++ 24 files changed, 309 insertions(+), 240 deletions(-) diff --git a/java/google/registry/flows/ResourceFlowUtils.java b/java/google/registry/flows/ResourceFlowUtils.java index d43e1c76a..27be17c18 100644 --- a/java/google/registry/flows/ResourceFlowUtils.java +++ b/java/google/registry/flows/ResourceFlowUtils.java @@ -39,14 +39,15 @@ import google.registry.flows.EppException.ParameterValuePolicyErrorException; import google.registry.flows.EppException.ParameterValueRangeErrorException; import google.registry.flows.exceptions.MissingTransferRequestAuthInfoException; import google.registry.flows.exceptions.NotPendingTransferException; -import google.registry.flows.exceptions.NotTransferInitiatorException; import google.registry.flows.exceptions.ResourceAlreadyExistsException; import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException; import google.registry.flows.exceptions.ResourceToDeleteIsReferencedException; import google.registry.flows.exceptions.TooManyResourceChecksException; import google.registry.model.EppResource; import google.registry.model.EppResource.Builder; +import google.registry.model.EppResource.BuilderWithTransferData; 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.DomainResource; @@ -66,6 +67,7 @@ import google.registry.model.transfer.TransferResponse.DomainTransferResponse; import google.registry.model.transfer.TransferStatus; import java.util.List; import java.util.Set; +import javax.annotation.Nullable; import org.joda.time.DateTime; /** Static utility functions for resource flows. */ @@ -141,33 +143,6 @@ public final class ResourceFlowUtils { } } - /** - * Performs common deletion operations on an EPP resource and returns a builder for further - * modifications. This is broken out into ResourceFlowUtils in order to expose the functionality - * to async flows (i.e. mapreduces). - */ - @SuppressWarnings("unchecked") - public static Builder> - prepareDeletedResourceAsBuilder(R resource, DateTime now) { - Builder> builder = - (Builder>) resource.asBuilder() - .setDeletionTime(now) - .setStatusValues(null) - .setTransferData( - resource.getStatusValues().contains(StatusValue.PENDING_TRANSFER) - ? resource.getTransferData().asBuilder() - .setTransferStatus(TransferStatus.SERVER_CANCELLED) - .setServerApproveEntities(null) - .setServerApproveBillingEvent(null) - .setServerApproveAutorenewEvent(null) - .setServerApproveAutorenewPollMessage(null) - .setPendingTransferExpirationTime(null) - .build() - : resource.getTransferData()) - .wipeOut(); - return builder; - } - /** Update the relevant {@link ForeignKeyIndex} to cache the new deletion time. */ public static void updateForeignKeyIndexDeletionTime(R resource) { if (resource instanceof ForeignKeyedEppResource) { @@ -176,8 +151,9 @@ public final class ResourceFlowUtils { } /** If there is a transfer out, delete the server-approve entities and enqueue a poll message. */ - public static void handlePendingTransferOnDelete( - R resource, R newResource, DateTime now, HistoryEntry historyEntry) { + public static + void handlePendingTransferOnDelete( + R resource, R newResource, DateTime now, HistoryEntry historyEntry) { if (resource.getStatusValues().contains(StatusValue.PENDING_TRANSFER)) { TransferData oldTransferData = resource.getTransferData(); ofy().delete().keys(oldTransferData.getServerApproveEntities()); @@ -229,6 +205,26 @@ public final class ResourceFlowUtils { } } + /** + * Create a {@link TransferData} object representing a resolved transfer. + * + *

This clears all the server-approve fields on the {@link TransferData} including the extended + * registration years field, sets the status field, and sets the expiration time of the last + * pending transfer to now. + */ + public static TransferData createResolvedTransferData( + TransferData oldTransferData, TransferStatus transferStatus, DateTime now) { + return oldTransferData.asBuilder() + .setExtendedRegistrationYears(null) + .setServerApproveEntities(null) + .setServerApproveBillingEvent(null) + .setServerApproveAutorenewEvent(null) + .setServerApproveAutorenewPollMessage(null) + .setTransferStatus(transferStatus) + .setPendingTransferExpirationTime(now) + .build(); + } + /** * Turn a resource into a builder with its pending transfer resolved. * @@ -236,21 +232,19 @@ public final class ResourceFlowUtils { * {@link TransferStatus}, clears all the server-approve fields on the {@link TransferData} * including the extended registration years field, and sets the expiration time of the last * pending transfer to now. + * + * @param now the time that the transfer was resolved, or null if the transfer was never actually + * resolved but the resource was deleted while it was still pending. */ @SuppressWarnings("unchecked") - private static EppResource.Builder resolvePendingTransfer( - R resource, TransferStatus transferStatus, DateTime now) { - return (EppResource.Builder) resource.asBuilder() + public static < + R extends EppResource & ResourceWithTransferData, + B extends EppResource.Builder & BuilderWithTransferData> B resolvePendingTransfer( + R resource, TransferStatus transferStatus, @Nullable DateTime now) { + return ((B) resource.asBuilder()) .removeStatusValue(StatusValue.PENDING_TRANSFER) - .setTransferData(resource.getTransferData().asBuilder() - .setExtendedRegistrationYears(null) - .setServerApproveEntities(null) - .setServerApproveBillingEvent(null) - .setServerApproveAutorenewEvent(null) - .setServerApproveAutorenewPollMessage(null) - .setTransferStatus(transferStatus) - .setPendingTransferExpirationTime(now) - .build()); + .setTransferData( + createResolvedTransferData(resource.getTransferData(), transferStatus, now)); } /** @@ -261,7 +255,7 @@ public final class ResourceFlowUtils { * including the extended registration years field, and sets the expiration time of the last * pending transfer to now. */ - public static R approvePendingTransfer( + public static R approvePendingTransfer( R resource, TransferStatus transferStatus, DateTime now) { Builder builder = resolvePendingTransfer(resource, transferStatus, now); builder @@ -278,13 +272,13 @@ public final class ResourceFlowUtils { * including the extended registration years field, sets the new client id, and sets the last * transfer time and the expiration time of the last pending transfer to now. */ - public static R denyPendingTransfer( + public static R denyPendingTransfer( R resource, TransferStatus transferStatus, DateTime now) { return resolvePendingTransfer(resource, transferStatus, now).build(); } - public static void verifyHasPendingTransfer(EppResource resource) - throws NotPendingTransferException { + public static void verifyHasPendingTransfer( + R resource) throws NotPendingTransferException { if (resource.getTransferData().getTransferStatus() != TransferStatus.PENDING) { throw new NotPendingTransferException(resource.getForeignKey()); } @@ -311,13 +305,6 @@ public final class ResourceFlowUtils { } } - public static void verifyIsGainingRegistrar(EppResource resource, String clientId) - throws NotTransferInitiatorException { - if (!clientId.equals(resource.getTransferData().getGainingClientId())) { - throw new NotTransferInitiatorException(); - } - } - /** Check that the given AuthInfo is present for a resource being transferred. */ public static void verifyAuthInfoPresentForResourceTransfer(Optional authInfo) throws EppException { diff --git a/java/google/registry/flows/async/DeleteContactsAndHostsAction.java b/java/google/registry/flows/async/DeleteContactsAndHostsAction.java index 34ed78373..d3cd2dc15 100644 --- a/java/google/registry/flows/async/DeleteContactsAndHostsAction.java +++ b/java/google/registry/flows/async/DeleteContactsAndHostsAction.java @@ -20,8 +20,8 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.math.IntMath.divide; import static com.googlecode.objectify.Key.getKind; +import static google.registry.flows.ResourceFlowUtils.createResolvedTransferData; import static google.registry.flows.ResourceFlowUtils.handlePendingTransferOnDelete; -import static google.registry.flows.ResourceFlowUtils.prepareDeletedResourceAsBuilder; import static google.registry.flows.ResourceFlowUtils.updateForeignKeyIndexDeletionTime; import static google.registry.model.EppResourceUtils.isActive; import static google.registry.model.EppResourceUtils.isDeleted; @@ -66,6 +66,7 @@ import google.registry.model.domain.DomainBase; import google.registry.model.host.HostResource; import google.registry.model.poll.PollMessage; import google.registry.model.reporting.HistoryEntry; +import google.registry.model.transfer.TransferStatus; import google.registry.request.Action; import google.registry.request.Response; import google.registry.util.Clock; @@ -313,7 +314,20 @@ public class DeleteContactsAndHostsAction implements Runnable { EppResource resourceToSave; if (deleteAllowed) { - resourceToSave = prepareDeletedResourceAsBuilder(resource, now).build(); + EppResource.Builder resourceToSaveBuilder; + if (resource instanceof ContactResource) { + ContactResource contact = (ContactResource) resource; + resourceToSaveBuilder = contact.asBuilder() + .setTransferData(createResolvedTransferData( + contact.getTransferData(), TransferStatus.SERVER_CANCELLED, null)) + .wipeOut(); + } else { + resourceToSaveBuilder = resource.asBuilder(); + } + resourceToSave = resourceToSaveBuilder + .setDeletionTime(now) + .setStatusValues(null) + .build(); performDeleteTasks(resource, resourceToSave, now, historyEntry); updateForeignKeyIndexDeletionTime(resourceToSave); } else { @@ -346,7 +360,10 @@ public class DeleteContactsAndHostsAction implements Runnable { HistoryEntry historyEntryForDelete) { if (existingResource instanceof ContactResource) { handlePendingTransferOnDelete( - existingResource, deletedResource, deletionTime, historyEntryForDelete); + (ContactResource) existingResource, + (ContactResource) deletedResource, + deletionTime, + historyEntryForDelete); } else if (existingResource instanceof HostResource) { HostResource host = (HostResource) existingResource; if (host.getSuperordinateDomain() != null) { diff --git a/java/google/registry/flows/domain/DomainApplicationDeleteFlow.java b/java/google/registry/flows/domain/DomainApplicationDeleteFlow.java index 5f22f1e44..9248b8889 100644 --- a/java/google/registry/flows/domain/DomainApplicationDeleteFlow.java +++ b/java/google/registry/flows/domain/DomainApplicationDeleteFlow.java @@ -15,8 +15,6 @@ package google.registry.flows.domain; import static google.registry.flows.FlowUtils.validateClientIsLoggedIn; -import static google.registry.flows.ResourceFlowUtils.handlePendingTransferOnDelete; -import static google.registry.flows.ResourceFlowUtils.prepareDeletedResourceAsBuilder; import static google.registry.flows.ResourceFlowUtils.updateForeignKeyIndexDeletionTime; import static google.registry.flows.ResourceFlowUtils.verifyExistence; import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo; @@ -100,15 +98,16 @@ public final class DomainApplicationDeleteFlow implements TransactionalFlow { throw new SunriseApplicationCannotBeDeletedInLandrushException(); } } - DomainApplication newApplication = - prepareDeletedResourceAsBuilder(existingApplication, now).build(); + DomainApplication newApplication = existingApplication.asBuilder() + .setDeletionTime(now) + .setStatusValues(null) + .build(); HistoryEntry historyEntry = historyBuilder .setType(HistoryEntry.Type.DOMAIN_APPLICATION_DELETE) .setModificationTime(now) .setParent(Key.create(existingApplication)) .build(); updateForeignKeyIndexDeletionTime(newApplication); - handlePendingTransferOnDelete(existingApplication, newApplication, now, historyEntry); handleExtraFlowLogic(tld, historyEntry, existingApplication, now); ofy().save().entities(newApplication, historyEntry); return responseBuilder.build(); diff --git a/java/google/registry/flows/domain/DomainDeleteFlow.java b/java/google/registry/flows/domain/DomainDeleteFlow.java index 7449bbd81..e6e6c745d 100644 --- a/java/google/registry/flows/domain/DomainDeleteFlow.java +++ b/java/google/registry/flows/domain/DomainDeleteFlow.java @@ -18,7 +18,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import static google.registry.flows.FlowUtils.validateClientIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.handlePendingTransferOnDelete; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; -import static google.registry.flows.ResourceFlowUtils.prepareDeletedResourceAsBuilder; +import static google.registry.flows.ResourceFlowUtils.resolvePendingTransfer; import static google.registry.flows.ResourceFlowUtils.updateForeignKeyIndexDeletionTime; import static google.registry.flows.ResourceFlowUtils.verifyNoDisallowedStatuses; import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo; @@ -70,6 +70,7 @@ import google.registry.model.poll.PollMessage; import google.registry.model.poll.PollMessage.OneTime; import google.registry.model.registry.Registry; import google.registry.model.reporting.HistoryEntry; +import google.registry.model.transfer.TransferStatus; import java.util.Set; import javax.annotation.Nullable; import javax.inject.Inject; @@ -119,7 +120,8 @@ public final class DomainDeleteFlow implements TransactionalFlow { Registry registry = Registry.get(existingDomain.getTld()); verifyDeleteAllowed(existingDomain, registry, now); HistoryEntry historyEntry = buildHistoryEntry(existingDomain, now); - Builder builder = (Builder) prepareDeletedResourceAsBuilder(existingDomain, now); + Builder builder = resolvePendingTransfer(existingDomain, TransferStatus.SERVER_CANCELLED, null); + builder.setDeletionTime(now).setStatusValues(null); // If the domain is in the Add Grace Period, we delete it immediately, which is already // reflected in the builder we just prepared. Otherwise we give it a PENDING_DELETE status. if (!existingDomain.getGracePeriodStatuses().contains(GracePeriodStatus.ADD)) { diff --git a/java/google/registry/flows/domain/DomainTransferCancelFlow.java b/java/google/registry/flows/domain/DomainTransferCancelFlow.java index 4306174d6..8c0edd6fe 100644 --- a/java/google/registry/flows/domain/DomainTransferCancelFlow.java +++ b/java/google/registry/flows/domain/DomainTransferCancelFlow.java @@ -18,7 +18,6 @@ import static google.registry.flows.FlowUtils.validateClientIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.denyPendingTransfer; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; import static google.registry.flows.ResourceFlowUtils.verifyHasPendingTransfer; -import static google.registry.flows.ResourceFlowUtils.verifyIsGainingRegistrar; import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo; import static google.registry.flows.domain.DomainFlowUtils.checkAllowedAccessToTld; import static google.registry.flows.domain.DomainFlowUtils.createLosingTransferPollMessage; @@ -34,6 +33,7 @@ import google.registry.flows.ExtensionManager; import google.registry.flows.FlowModule.ClientId; import google.registry.flows.FlowModule.TargetId; import google.registry.flows.TransactionalFlow; +import google.registry.flows.exceptions.NotTransferInitiatorException; import google.registry.model.ImmutableObject; import google.registry.model.domain.DomainResource; import google.registry.model.domain.metadata.MetadataExtension; @@ -81,7 +81,9 @@ public final class DomainTransferCancelFlow implements TransactionalFlow { DomainResource existingDomain = loadAndVerifyExistence(DomainResource.class, targetId, now); verifyOptionalAuthInfo(authInfo, existingDomain); verifyHasPendingTransfer(existingDomain); - verifyIsGainingRegistrar(existingDomain, clientId); + if (!clientId.equals(existingDomain.getTransferData().getGainingClientId())) { + throw new NotTransferInitiatorException(); + } checkAllowedAccessToTld(clientId, existingDomain.getTld()); HistoryEntry historyEntry = historyBuilder .setType(HistoryEntry.Type.DOMAIN_TRANSFER_CANCEL) diff --git a/java/google/registry/model/EppResource.java b/java/google/registry/model/EppResource.java index 8468ebeb8..2b7269e3d 100644 --- a/java/google/registry/model/EppResource.java +++ b/java/google/registry/model/EppResource.java @@ -117,10 +117,6 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable, /** Status values associated with this resource. */ Set status; - /** Data about any pending or past transfers on this contact. */ - @XmlTransient - TransferData transferData; - /** * Sorted map of {@link DateTime} keys (modified time) to {@link CommitLogManifest} entries. * @@ -161,15 +157,6 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable, return nullToEmptyImmutableCopy(status); } - public final TransferData getTransferData() { - return Optional.fromNullable(transferData).or(TransferData.EMPTY); - } - - /** Returns whether there is any transferData. */ - public final boolean hasTransferData() { - return transferData != null; - } - @XmlElement(name = "trDate") public DateTime getLastTransferTime() { return lastTransferTime; @@ -196,6 +183,16 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable, /** EppResources that are loaded via foreign keys should implement this marker interface. */ public interface ForeignKeyedEppResource {} + /** An interface for resources that have transfer data. */ + public interface ResourceWithTransferData { + public TransferData getTransferData(); + } + + /** An interface for builders of resources that have transfer data. */ + public interface BuilderWithTransferData> { + public B setTransferData(TransferData transferData); + } + /** Abstract builder for {@link EppResource} types. */ public abstract static class Builder> extends GenericBuilder { @@ -292,23 +289,12 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable, difference(getInstance().getStatusValues(), statusValues))); } - /** Set this resource's transfer data. */ - public B setTransferData(TransferData transferData) { - getInstance().transferData = transferData; - return thisCastToDerived(); - } - /** Set this resource's repoId. */ public B setRepoId(String repoId) { getInstance().repoId = repoId; return thisCastToDerived(); } - /** Wipe out any personal information in the resource. */ - public B wipeOut() { - return thisCastToDerived(); - } - /** Build the resource, nullifying empty strings and sets and setting defaults. */ @Override public T build() { @@ -323,10 +309,6 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable, /** Build the resource, nullifying empty strings and sets and setting defaults. */ public T buildWithoutImplicitStatusValues() { - // If TransferData is totally empty, set it to null. - if (TransferData.EMPTY.equals(getInstance().transferData)) { - setTransferData(null); - } // If there is no deletion time, set it to END_OF_TIME. setDeletionTime(Optional.fromNullable(getInstance().deletionTime).or(END_OF_TIME)); return ImmutableObject.cloneEmptyToNull(super.build()); diff --git a/java/google/registry/model/EppResourceUtils.java b/java/google/registry/model/EppResourceUtils.java index 65a5a891e..31ffa5870 100644 --- a/java/google/registry/model/EppResourceUtils.java +++ b/java/google/registry/model/EppResourceUtils.java @@ -29,7 +29,9 @@ import com.googlecode.objectify.Result; import com.googlecode.objectify.util.ResultNow; import google.registry.config.RegistryEnvironment; import google.registry.model.EppResource.Builder; +import google.registry.model.EppResource.BuilderWithTransferData; import google.registry.model.EppResource.ForeignKeyedEppResource; +import google.registry.model.EppResource.ResourceWithTransferData; import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainApplication; import google.registry.model.domain.DomainBase; @@ -201,8 +203,8 @@ public final class EppResourceUtils { } /** Process an automatic transfer on a resource. */ - public static void setAutomaticTransferSuccessProperties( - Builder builder, TransferData transferData) { + public static & BuilderWithTransferData> + void setAutomaticTransferSuccessProperties(B builder, TransferData transferData) { checkArgument(TransferStatus.PENDING.equals(transferData.getTransferStatus())); builder.removeStatusValue(StatusValue.PENDING_TRANSFER) .setTransferData(transferData.asBuilder() @@ -222,8 +224,10 @@ public final class EppResourceUtils { *
  • Process an automatic transfer. * */ - public static void projectResourceOntoBuilderAtTime( - T resource, Builder builder, DateTime now) { + public static < + T extends EppResource & ResourceWithTransferData, + B extends Builder & BuilderWithTransferData> + void projectResourceOntoBuilderAtTime(T resource, B builder, DateTime now) { TransferData transferData = resource.getTransferData(); // If there's a pending transfer that has expired, process it. DateTime expirationTime = transferData.getPendingTransferExpirationTime(); diff --git a/java/google/registry/model/contact/ContactResource.java b/java/google/registry/model/contact/ContactResource.java index f0bcaf53a..68245030c 100644 --- a/java/google/registry/model/contact/ContactResource.java +++ b/java/google/registry/model/contact/ContactResource.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static google.registry.model.EppResourceUtils.projectResourceOntoBuilderAtTime; import static google.registry.model.ofy.Ofy.RECOMMENDED_MEMCACHE_EXPIRATION; +import com.google.common.base.Optional; import com.google.common.base.Predicates; import com.google.common.collect.FluentIterable; import com.google.common.collect.Lists; @@ -28,8 +29,10 @@ import com.googlecode.objectify.annotation.Index; import com.googlecode.objectify.condition.IfNull; import google.registry.model.EppResource; import google.registry.model.EppResource.ForeignKeyedEppResource; +import google.registry.model.EppResource.ResourceWithTransferData; import google.registry.model.annotations.ExternalMessagingName; import google.registry.model.contact.PostalInfo.Type; +import google.registry.model.transfer.TransferData; import java.util.List; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlRootElement; @@ -64,7 +67,8 @@ import org.joda.time.DateTime; @Cache(expirationSeconds = RECOMMENDED_MEMCACHE_EXPIRATION) @Entity @ExternalMessagingName("contact") -public class ContactResource extends EppResource implements ForeignKeyedEppResource { +public class ContactResource extends EppResource + implements ForeignKeyedEppResource, ResourceWithTransferData { /** * Unique identifier for this contact. @@ -78,13 +82,16 @@ public class ContactResource extends EppResource implements ForeignKeyedEppResou /** * Localized postal info for the contact. All contained values must be representable in the 7-bit - * US-ASCII character set. Personal info; cleared by wipeOut(). + * US-ASCII character set. Personal info; cleared by {@link Builder#wipeOut}. */ @IgnoreSave(IfNull.class) @XmlTransient PostalInfo localizedPostalInfo; - /** Internationalized postal info for the contact. Personal info; cleared by wipeOut(). */ + /** + * Internationalized postal info for the contact. Personal info; cleared by + * {@link Builder#wipeOut}. + */ @IgnoreSave(IfNull.class) @XmlTransient PostalInfo internationalizedPostalInfo; @@ -92,21 +99,21 @@ public class ContactResource extends EppResource implements ForeignKeyedEppResou /** * Contact name used for name searches. This is set automatically to be the internationalized * postal name, or if null, the localized postal name, or if that is null as well, null. Personal - * info; cleared by wipeOut(). + * info; cleared by {@link Builder#wipeOut}. */ @Index @XmlTransient String searchName; - /** Contact’s voice number. Personal info; cleared by wipeOut(). */ + /** Contact’s voice number. Personal info; cleared by {@link Builder#wipeOut}. */ @IgnoreSave(IfNull.class) ContactPhoneNumber voice; - /** Contact’s fax number. Personal info; cleared by wipeOut(). */ + /** Contact’s fax number. Personal info; cleared by {@link Builder#wipeOut}. */ @IgnoreSave(IfNull.class) ContactPhoneNumber fax; - /** Contact’s email address. Personal info; cleared by wipeOut(). */ + /** Contact’s email address. Personal info; cleared by {@link Builder#wipeOut}. */ @IgnoreSave(IfNull.class) @XmlJavaTypeAdapter(CollapsedStringAdapter.class) String email; @@ -114,6 +121,10 @@ public class ContactResource extends EppResource implements ForeignKeyedEppResou /** Authorization info (aka transfer secret) of the contact. */ ContactAuthInfo authInfo; + /** Data about any pending or past transfers on this contact. */ + @XmlTransient + TransferData transferData; + // If any new fields are added which contain personal information, make sure they are cleared by // the wipeOut() function, so that data is not kept around for deleted contacts. @@ -154,6 +165,11 @@ public class ContactResource extends EppResource implements ForeignKeyedEppResou return disclose; } + @Override + public final TransferData getTransferData() { + return Optional.fromNullable(transferData).or(TransferData.EMPTY); + } + @Override public String getForeignKey() { return contactId; @@ -188,7 +204,9 @@ public class ContactResource extends EppResource implements ForeignKeyedEppResou } /** A builder for constructing {@link ContactResource}, since it is immutable. */ - public static class Builder extends EppResource.Builder { + public static class Builder extends EppResource.Builder + implements BuilderWithTransferData{ + public Builder() {} private Builder(ContactResource instance) { @@ -252,26 +270,42 @@ public class ContactResource extends EppResource implements ForeignKeyedEppResou } @Override + public Builder setTransferData(TransferData transferData) { + getInstance().transferData = transferData; + return this; + } + + /** + * Remove all personally identifying information about a contact. + * + *

    This should be used when deleting a contact so that the soft-deleted entity doesn't + * contain information that the registrant requested to be deleted. + */ public Builder wipeOut() { - this.setEmailAddress(null); - this.setFaxNumber(null); - this.setInternationalizedPostalInfo(null); - this.setLocalizedPostalInfo(null); - this.setVoiceNumber(null); - return super.wipeOut(); + setEmailAddress(null); + setFaxNumber(null); + setInternationalizedPostalInfo(null); + setLocalizedPostalInfo(null); + setVoiceNumber(null); + return this; } @Override public ContactResource build() { + ContactResource instance = getInstance(); + // If TransferData is totally empty, set it to null. + if (TransferData.EMPTY.equals(instance.transferData)) { + setTransferData(null); + } // Set the searchName using the internationalized and localized postal info names. - if ((getInstance().internationalizedPostalInfo != null) - && (getInstance().internationalizedPostalInfo.getName() != null)) { - getInstance().searchName = getInstance().internationalizedPostalInfo.getName(); - } else if ((getInstance().localizedPostalInfo != null) - && (getInstance().localizedPostalInfo.getName() != null)) { - getInstance().searchName = getInstance().localizedPostalInfo.getName(); + if ((instance.internationalizedPostalInfo != null) + && (instance.internationalizedPostalInfo.getName() != null)) { + instance.searchName = instance.internationalizedPostalInfo.getName(); + } else if ((instance.localizedPostalInfo != null) + && (instance.localizedPostalInfo.getName() != null)) { + instance.searchName = instance.localizedPostalInfo.getName(); } else { - getInstance().searchName = null; + instance.searchName = null; } return super.build(); } diff --git a/java/google/registry/model/domain/DomainResource.java b/java/google/registry/model/domain/DomainResource.java index c11137af0..a48ebfc08 100644 --- a/java/google/registry/model/domain/DomainResource.java +++ b/java/google/registry/model/domain/DomainResource.java @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package google.registry.model.domain; + package google.registry.model.domain; import static com.google.common.collect.Sets.intersection; import static google.registry.model.EppResourceUtils.projectResourceOntoBuilderAtTime; @@ -33,6 +33,7 @@ import com.googlecode.objectify.annotation.EntitySubclass; import com.googlecode.objectify.annotation.IgnoreSave; import com.googlecode.objectify.condition.IfNull; import google.registry.model.EppResource.ForeignKeyedEppResource; +import google.registry.model.EppResource.ResourceWithTransferData; import google.registry.model.annotations.ExternalMessagingName; import google.registry.model.billing.BillingEvent; import google.registry.model.domain.rgp.GracePeriodStatus; @@ -75,7 +76,8 @@ import org.joda.time.Interval; @Cache(expirationSeconds = RECOMMENDED_MEMCACHE_EXPIRATION) @EntitySubclass(index = true) @ExternalMessagingName("domain") -public class DomainResource extends DomainBase implements ForeignKeyedEppResource { +public class DomainResource extends DomainBase + implements ForeignKeyedEppResource, ResourceWithTransferData { /** The max number of years that a domain can be registered for, as set by ICANN policy. */ public static final int MAX_REGISTRATION_YEARS = 10; @@ -156,6 +158,10 @@ public class DomainResource extends DomainBase implements ForeignKeyedEppResourc @XmlTransient Key application; + /** Data about any pending or past transfers on this domain. */ + @XmlTransient + TransferData transferData; + public ImmutableSet getSubordinateHosts() { return nullToEmptyImmutableCopy(subordinateHosts); } @@ -192,6 +198,11 @@ public class DomainResource extends DomainBase implements ForeignKeyedEppResourc return application; } + @Override + public final TransferData getTransferData() { + return Optional.fromNullable(transferData).or(TransferData.EMPTY); + } + @Override public String getForeignKey() { return fullyQualifiedDomainName; @@ -356,7 +367,8 @@ public class DomainResource extends DomainBase implements ForeignKeyedEppResourc } /** A builder for constructing {@link DomainResource}, since it is immutable. */ - public static class Builder extends DomainBase.Builder { + public static class Builder extends DomainBase.Builder + implements BuilderWithTransferData { public Builder() {} @@ -366,6 +378,10 @@ public class DomainResource extends DomainBase implements ForeignKeyedEppResourc @Override public DomainResource build() { + // If TransferData is totally empty, set it to null. + if (TransferData.EMPTY.equals(getInstance().transferData)) { + setTransferData(null); + } // A DomainResource has status INACTIVE if there are no nameservers. if (getInstance().getNameservers().isEmpty()) { addStatusValue(StatusValue.INACTIVE); @@ -440,5 +456,11 @@ public class DomainResource extends DomainBase implements ForeignKeyedEppResourc getInstance().gracePeriods = difference(getInstance().getGracePeriods(), gracePeriod); return this; } + + @Override + public Builder setTransferData(TransferData transferData) { + getInstance().transferData = transferData; + return thisCastToDerived(); + } } } diff --git a/java/google/registry/model/host/HostResource.java b/java/google/registry/model/host/HostResource.java index 61a7b3ae1..395ee10d2 100644 --- a/java/google/registry/model/host/HostResource.java +++ b/java/google/registry/model/host/HostResource.java @@ -16,7 +16,6 @@ package google.registry.model.host; import static com.google.common.collect.Sets.difference; import static com.google.common.collect.Sets.union; -import static google.registry.model.EppResourceUtils.projectResourceOntoBuilderAtTime; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.Ofy.RECOMMENDED_MEMCACHE_EXPIRATION; import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; @@ -126,15 +125,14 @@ public class HostResource extends EppResource implements ForeignKeyedEppResource @Override public HostResource cloneProjectedAtTime(DateTime now) { Builder builder = this.asBuilder(); - projectResourceOntoBuilderAtTime(this, builder, now); if (superordinateDomain == null) { // If this was a subordinate host to a domain that was being transferred, there might be a // pending transfer still extant, so remove it. - builder.setTransferData(null).removeStatusValue(StatusValue.PENDING_TRANSFER); + builder.removeStatusValue(StatusValue.PENDING_TRANSFER); } else { - // For hosts with superordinate domains, the client id, last transfer time, and transfer data - // need to be read off the domain projected to the correct time. + // For hosts with superordinate domains, the client id, last transfer time, and transfer + // status value need to be read off the domain projected to the correct time. DomainResource domainAtTime = ofy().load().key(superordinateDomain).now() .cloneProjectedAtTime(now); builder.setCurrentSponsorClientId(domainAtTime.getCurrentSponsorClientId()); @@ -145,15 +143,14 @@ public class HostResource extends EppResource implements ForeignKeyedEppResource .isBefore(Optional.fromNullable(domainAtTime.getLastTransferTime()).or(START_OF_TIME))) { builder.setLastTransferTime(domainAtTime.getLastTransferTime()); } - // Copy the transfer status and data from the superordinate domain onto the host, because the - // host's doesn't matter and the superordinate domain always has the canonical data. - TransferData domainTransferData = domainAtTime.getTransferData(); - if (TransferStatus.PENDING.equals(domainTransferData.getTransferStatus())) { + // Copy the transfer status from the superordinate domain onto the host, because the host's + // doesn't matter and the superordinate domain always has the canonical data. + TransferStatus domainTransferStatus = domainAtTime.getTransferData().getTransferStatus(); + if (TransferStatus.PENDING.equals(domainTransferStatus)) { builder.addStatusValue(StatusValue.PENDING_TRANSFER); } else { builder.removeStatusValue(StatusValue.PENDING_TRANSFER); } - builder.setTransferData(domainTransferData); } return builder.build(); } diff --git a/java/google/registry/monitoring/whitebox/VerifyEntityIntegrityAction.java b/java/google/registry/monitoring/whitebox/VerifyEntityIntegrityAction.java index 23bcd41b5..d815c152f 100644 --- a/java/google/registry/monitoring/whitebox/VerifyEntityIntegrityAction.java +++ b/java/google/registry/monitoring/whitebox/VerifyEntityIntegrityAction.java @@ -242,21 +242,6 @@ public class VerifyEntityIntegrityAction implements Runnable { Key key = Key.create(domainBase); verifyExistence(key, domainBase.getReferencedContacts()); verifyExistence(key, domainBase.getNameservers()); - verifyExistence(key, domainBase.getTransferData().getServerApproveAutorenewEvent()); - verifyExistence(key, domainBase.getTransferData().getServerApproveAutorenewPollMessage()); - verifyExistence(key, domainBase.getTransferData().getServerApproveBillingEvent()); - verifyExistence(key, FluentIterable - .from(domainBase.getTransferData().getServerApproveEntities()) - .transform( - new Function, - Key>() { - @SuppressWarnings("unchecked") - @Override - public Key apply( - Key key) { - return (Key) key; - }}) - .toSet()); if (domainBase instanceof DomainApplication) { getContext().incrementCounter("domain applications"); DomainApplication application = (DomainApplication) domainBase; @@ -266,6 +251,21 @@ public class VerifyEntityIntegrityAction implements Runnable { } else if (domainBase instanceof DomainResource) { getContext().incrementCounter("domain resources"); DomainResource domain = (DomainResource) domainBase; + verifyExistence(key, domain.getTransferData().getServerApproveAutorenewEvent()); + verifyExistence(key, domain.getTransferData().getServerApproveAutorenewPollMessage()); + verifyExistence(key, domain.getTransferData().getServerApproveBillingEvent()); + verifyExistence(key, FluentIterable + .from(domain.getTransferData().getServerApproveEntities()) + .transform( + new Function, + Key>() { + @SuppressWarnings("unchecked") + @Override + public Key apply( + Key key) { + return (Key) key; + }}) + .toSet()); verifyExistence(key, domain.getApplication()); verifyExistence(key, domain.getAutorenewBillingEvent()); for (GracePeriod gracePeriod : domain.getGracePeriods()) { diff --git a/javatests/google/registry/flows/domain/DomainTransferApproveFlowTest.java b/javatests/google/registry/flows/domain/DomainTransferApproveFlowTest.java index 06a4d72b8..d90637ecb 100644 --- a/javatests/google/registry/flows/domain/DomainTransferApproveFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainTransferApproveFlowTest.java @@ -43,7 +43,6 @@ import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException; import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException; import google.registry.flows.domain.DomainFlowUtils.NotAuthorizedForTldException; import google.registry.flows.exceptions.NotPendingTransferException; -import google.registry.model.EppResource; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Cancellation; import google.registry.model.billing.BillingEvent.Cancellation.Builder; @@ -98,8 +97,8 @@ public class DomainTransferApproveFlowTest clock.advanceOneMilli(); } - private void assertTransferApproved(EppResource resource) { - assertAboutEppResources().that(resource) + private void assertTransferApproved(DomainResource domain) { + assertAboutDomains().that(domain) .hasTransferStatus(TransferStatus.CLIENT_APPROVED).and() .hasCurrentSponsorClientId("NewRegistrar").and() .hasLastTransferTime(clock.nowUtc()).and() @@ -107,6 +106,13 @@ public class DomainTransferApproveFlowTest .doesNotHaveStatusValue(StatusValue.PENDING_TRANSFER); } + private void assertTransferApproved(HostResource host) { + assertAboutEppResources().that(host) + .hasCurrentSponsorClientId("NewRegistrar").and() + .hasLastTransferTime(clock.nowUtc()).and() + .doesNotHaveStatusValue(StatusValue.PENDING_TRANSFER); + } + private void setEppLoader(String commandFilename) { setEppInput(commandFilename); // Replace the ROID in the xml file with the one generated in our test. diff --git a/javatests/google/registry/flows/domain/DomainTransferCancelFlowTest.java b/javatests/google/registry/flows/domain/DomainTransferCancelFlowTest.java index bdd9f6c0e..44432fa66 100644 --- a/javatests/google/registry/flows/domain/DomainTransferCancelFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainTransferCancelFlowTest.java @@ -112,9 +112,7 @@ public class DomainTransferCancelFlowTest assertAboutDomains().that(domain) .hasRegistrationExpirationTime(originalExpirationTime).and() .hasLastTransferTimeNotEqualTo(clock.nowUtc()); - assertTransferFailed( - reloadResourceAndCloneAtTime(subordinateHost, clock.nowUtc()), - TransferStatus.CLIENT_CANCELLED); + assertTransferFailed(reloadResourceAndCloneAtTime(subordinateHost, clock.nowUtc())); assertAboutDomains().that(domain).hasOneHistoryEntryEachOfTypes( HistoryEntry.Type.DOMAIN_CREATE, HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST, diff --git a/javatests/google/registry/flows/domain/DomainTransferFlowTestCase.java b/javatests/google/registry/flows/domain/DomainTransferFlowTestCase.java index 9c852a991..4678a9aa0 100644 --- a/javatests/google/registry/flows/domain/DomainTransferFlowTestCase.java +++ b/javatests/google/registry/flows/domain/DomainTransferFlowTestCase.java @@ -23,6 +23,7 @@ import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType; import static google.registry.testing.DatastoreHelper.persistActiveContact; import static google.registry.testing.DatastoreHelper.persistDomainWithPendingTransfer; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.DomainResourceSubject.assertAboutDomains; import static google.registry.testing.GenericEppResourceSubject.assertAboutEppResources; import static google.registry.util.DateTimeUtils.END_OF_TIME; @@ -210,19 +211,25 @@ public class DomainTransferFlowTestCase .build(); } - protected void assertTransferFailed(EppResource resource, TransferStatus status) { - assertAboutEppResources().that(resource) + protected void assertTransferFailed(DomainResource domain, TransferStatus status) { + assertAboutDomains().that(domain) .hasTransferStatus(status).and() .hasPendingTransferExpirationTime(clock.nowUtc()).and() .doesNotHaveStatusValue(StatusValue.PENDING_TRANSFER).and() .hasCurrentSponsorClientId("TheRegistrar"); - TransferData transferData = resource.getTransferData(); + TransferData transferData = domain.getTransferData(); assertThat(transferData.getServerApproveBillingEvent()).isNull(); assertThat(transferData.getServerApproveAutorenewEvent()).isNull(); assertThat(transferData.getServerApproveAutorenewPollMessage()).isNull(); assertThat(transferData.getServerApproveEntities()).isEmpty(); } + protected void assertTransferFailed(HostResource resource) { + assertAboutEppResources().that(resource) + .doesNotHaveStatusValue(StatusValue.PENDING_TRANSFER).and() + .hasCurrentSponsorClientId("TheRegistrar"); + } + /** Adds a .tld domain that has a pending transfer on it from TheRegistrar to NewRegistrar. */ protected void setupDomainWithPendingTransfer() throws Exception { setupDomainWithPendingTransfer("tld"); diff --git a/javatests/google/registry/flows/domain/DomainTransferRejectFlowTest.java b/javatests/google/registry/flows/domain/DomainTransferRejectFlowTest.java index 92f27e893..a952c4761 100644 --- a/javatests/google/registry/flows/domain/DomainTransferRejectFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainTransferRejectFlowTest.java @@ -82,9 +82,7 @@ public class DomainTransferRejectFlowTest // Transfer should have been rejected. Verify correct fields were set. domain = reloadResourceByForeignKey(); assertTransferFailed(domain, TransferStatus.CLIENT_REJECTED); - assertTransferFailed( - reloadResourceAndCloneAtTime(subordinateHost, clock.nowUtc()), - TransferStatus.CLIENT_REJECTED); + assertTransferFailed(reloadResourceAndCloneAtTime(subordinateHost, clock.nowUtc())); assertAboutDomains().that(domain) .hasRegistrationExpirationTime(originalExpirationTime).and() .hasLastTransferTimeNotEqualTo(clock.nowUtc()).and() diff --git a/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java b/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java index a371f4fa8..111fc57f5 100644 --- a/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java @@ -53,7 +53,6 @@ import google.registry.flows.exceptions.AlreadyPendingTransferException; import google.registry.flows.exceptions.MissingTransferRequestAuthInfoException; import google.registry.flows.exceptions.ObjectAlreadySponsoredException; import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException; -import google.registry.model.EppResource; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Cancellation.Builder; import google.registry.model.billing.BillingEvent.Flag; @@ -67,6 +66,7 @@ import google.registry.model.domain.TestExtraLogicManager.TestExtraLogicManagerS import google.registry.model.domain.rgp.GracePeriodStatus; import google.registry.model.eppcommon.AuthInfo.PasswordAuth; import google.registry.model.eppcommon.StatusValue; +import google.registry.model.host.HostResource; import google.registry.model.poll.PendingActionNotificationResponse; import google.registry.model.poll.PollMessage; import google.registry.model.registrar.Registrar; @@ -102,8 +102,8 @@ public class DomainTransferRequestFlowTest RegistryExtraFlowLogicProxy.setOverride("flags", TestExtraLogicManager.class); } - private void assertTransferRequested(EppResource resource) throws Exception { - assertAboutEppResources().that(resource) + private void assertTransferRequested(DomainResource domain) throws Exception { + assertAboutDomains().that(domain) .hasTransferStatus(TransferStatus.PENDING).and() .hasTransferGainingClientId("NewRegistrar").and() .hasTransferLosingClientId("TheRegistrar").and() @@ -114,9 +114,15 @@ public class DomainTransferRequestFlowTest .hasStatusValue(StatusValue.PENDING_TRANSFER); } - private void assertTransferApproved(EppResource resource) { + private void assertTransferRequested(HostResource host) throws Exception { + assertAboutEppResources().that(host) + .hasCurrentSponsorClientId("TheRegistrar").and() + .hasStatusValue(StatusValue.PENDING_TRANSFER); + } + + private void assertTransferApproved(DomainResource domain) { DateTime afterAutoAck = clock.nowUtc().plus(Registry.get("tld").getAutomaticTransferLength()); - assertAboutEppResources().that(resource) + assertAboutDomains().that(domain) .hasTransferStatus(TransferStatus.SERVER_APPROVED).and() .hasCurrentSponsorClientId("NewRegistrar").and() .hasLastTransferTime(afterAutoAck).and() @@ -124,6 +130,14 @@ public class DomainTransferRequestFlowTest .doesNotHaveStatusValue(StatusValue.PENDING_TRANSFER); } + private void assertTransferApproved(HostResource host) { + DateTime afterAutoAck = clock.nowUtc().plus(Registry.get("tld").getAutomaticTransferLength()); + assertAboutEppResources().that(host) + .hasCurrentSponsorClientId("NewRegistrar").and() + .hasLastTransferTime(afterAutoAck).and() + .doesNotHaveStatusValue(StatusValue.PENDING_TRANSFER); + } + /** * Runs a successful test. The extraExpectedBillingEvents parameter consists of cancellation * billing event builders that have had all of their attributes set except for the parent history diff --git a/javatests/google/registry/model/contact/ContactResourceTest.java b/javatests/google/registry/model/contact/ContactResourceTest.java index 76c13a98f..1a1dc6c12 100644 --- a/javatests/google/registry/model/contact/ContactResourceTest.java +++ b/javatests/google/registry/model/contact/ContactResourceTest.java @@ -167,7 +167,7 @@ public class ContactResourceTest extends EntityTestCase { ContactResource withNull = new ContactResource.Builder().setTransferData(null).build(); ContactResource withEmpty = withNull.asBuilder().setTransferData(TransferData.EMPTY).build(); assertThat(withNull).isEqualTo(withEmpty); - assertThat(withEmpty.hasTransferData()).isFalse(); + assertThat(withEmpty.transferData).isNull(); } @Test diff --git a/javatests/google/registry/model/domain/DomainApplicationTest.java b/javatests/google/registry/model/domain/DomainApplicationTest.java index 8f66b1b6f..0a7277b77 100644 --- a/javatests/google/registry/model/domain/DomainApplicationTest.java +++ b/javatests/google/registry/model/domain/DomainApplicationTest.java @@ -36,7 +36,6 @@ import com.google.common.collect.ImmutableSet; import com.googlecode.objectify.Key; import com.googlecode.objectify.VoidWork; import google.registry.model.EntityTestCase; -import google.registry.model.billing.BillingEvent; import google.registry.model.domain.launch.ApplicationStatus; import google.registry.model.domain.launch.LaunchNotice; import google.registry.model.domain.launch.LaunchPhase; @@ -47,9 +46,6 @@ import google.registry.model.eppcommon.Trid; import google.registry.model.host.HostResource; import google.registry.model.reporting.HistoryEntry; import google.registry.model.smd.EncodedSignedMark; -import google.registry.model.transfer.TransferData; -import google.registry.model.transfer.TransferData.TransferServerApproveEntity; -import google.registry.model.transfer.TransferStatus; import google.registry.testing.ExceptionRule; import org.joda.money.Money; import org.joda.time.DateTime; @@ -95,19 +91,6 @@ public class DomainApplicationTest extends EntityTestCase { .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2}))) .setLaunchNotice( LaunchNotice.create("tcnid", "validatorId", START_OF_TIME, START_OF_TIME)) - .setTransferData( - new TransferData.Builder() - .setExtendedRegistrationYears(0) - .setGainingClientId("gaining") - .setLosingClientId("losing") - .setPendingTransferExpirationTime(clock.nowUtc()) - .setServerApproveEntities( - ImmutableSet.>of( - Key.create(BillingEvent.OneTime.class, 1))) - .setTransferRequestTime(clock.nowUtc()) - .setTransferStatus(TransferStatus.SERVER_APPROVED) - .setTransferRequestTrid(Trid.create("client trid")) - .build()) .setCreationTrid(Trid.create("client creation trid")) .setPhase(LaunchPhase.LANDRUSH) // TODO(b/32447342): set period @@ -183,14 +166,6 @@ public class DomainApplicationTest extends EntityTestCase { .isNotNull(); } - @Test - public void testEmptyTransferDataBecomesNull() throws Exception { - DomainApplication withNull = emptyBuilder().setTransferData(null).build(); - DomainApplication withEmpty = withNull.asBuilder().setTransferData(TransferData.EMPTY).build(); - assertThat(withNull).isEqualTo(withEmpty); - assertThat(withEmpty.hasTransferData()).isFalse(); - } - @Test public void testToHydratedString_notCircular() { // If there are circular references, this will overflow the stack. diff --git a/javatests/google/registry/model/domain/DomainResourceTest.java b/javatests/google/registry/model/domain/DomainResourceTest.java index dce820bf4..fce52220a 100644 --- a/javatests/google/registry/model/domain/DomainResourceTest.java +++ b/javatests/google/registry/model/domain/DomainResourceTest.java @@ -231,7 +231,7 @@ public class DomainResourceTest extends EntityTestCase { newDomainResource("example.com").asBuilder().setTransferData(null).build(); DomainResource withEmpty = withNull.asBuilder().setTransferData(TransferData.EMPTY).build(); assertThat(withNull).isEqualTo(withEmpty); - assertThat(withEmpty.hasTransferData()).isFalse(); + assertThat(withEmpty.transferData).isNull(); } @Test diff --git a/javatests/google/registry/model/host/HostResourceTest.java b/javatests/google/registry/model/host/HostResourceTest.java index b7968fbd9..c022017d4 100644 --- a/javatests/google/registry/model/host/HostResourceTest.java +++ b/javatests/google/registry/model/host/HostResourceTest.java @@ -149,14 +149,6 @@ public class HostResourceTest extends EntityTestCase { .isNotNull(); } - @Test - public void testEmptyTransferDataBecomesNull() throws Exception { - HostResource withNull = new HostResource.Builder().setTransferData(null).build(); - HostResource withEmpty = withNull.asBuilder().setTransferData(TransferData.EMPTY).build(); - assertThat(withNull).isEqualTo(withEmpty); - assertThat(withEmpty.hasTransferData()).isFalse(); - } - @Test public void testImplicitStatusValues() { // OK is implicit if there's no other statuses. @@ -205,7 +197,6 @@ public class HostResourceTest extends EntityTestCase { hostResource.asBuilder() .setLastSuperordinateChange(superordinateChangeTime) .setLastTransferTime(hostTransferTime) - .setTransferData(null) .build()); return hostResource.cloneProjectedAtTime(clock.nowUtc()).getLastTransferTime(); } @@ -279,8 +270,6 @@ public class HostResourceTest extends EntityTestCase { .build()) .build()); HostResource afterTransfer = hostResource.cloneProjectedAtTime(clock.nowUtc().plusDays(1)); - assertThat(afterTransfer.getTransferData().getTransferStatus()) - .isEqualTo(TransferStatus.SERVER_APPROVED); assertThat(afterTransfer.getCurrentSponsorClientId()).isEqualTo("winner"); assertThat(afterTransfer.getLastTransferTime()).isEqualTo(clock.nowUtc().plusDays(1)); } diff --git a/javatests/google/registry/model/schema.txt b/javatests/google/registry/model/schema.txt index 8761c14a7..04e9b372f 100644 --- a/javatests/google/registry/model/schema.txt +++ b/javatests/google/registry/model/schema.txt @@ -206,7 +206,6 @@ class google.registry.model.domain.DomainApplication { google.registry.model.domain.launch.LaunchNotice launchNotice; google.registry.model.domain.launch.LaunchPhase phase; google.registry.model.eppcommon.Trid creationTrid; - google.registry.model.transfer.TransferData transferData; java.lang.String creationClientId; java.lang.String currentSponsorClientId; java.lang.String fullyQualifiedDomainName; @@ -233,7 +232,6 @@ class google.registry.model.domain.DomainBase { google.registry.model.UpdateAutoTimestamp updateTimestamp; google.registry.model.domain.DomainAuthInfo authInfo; google.registry.model.domain.launch.LaunchNotice launchNotice; - google.registry.model.transfer.TransferData transferData; java.lang.String creationClientId; java.lang.String currentSponsorClientId; java.lang.String fullyQualifiedDomainName; @@ -387,7 +385,6 @@ class google.registry.model.host.HostResource { com.googlecode.objectify.Key superordinateDomain; google.registry.model.CreateAutoTimestamp creationTime; google.registry.model.UpdateAutoTimestamp updateTimestamp; - google.registry.model.transfer.TransferData transferData; java.lang.String creationClientId; java.lang.String currentSponsorClientId; java.lang.String fullyQualifiedHostName; diff --git a/javatests/google/registry/testing/AbstractEppResourceSubject.java b/javatests/google/registry/testing/AbstractEppResourceSubject.java index 42b77fe91..2531a5b3a 100644 --- a/javatests/google/registry/testing/AbstractEppResourceSubject.java +++ b/javatests/google/registry/testing/AbstractEppResourceSubject.java @@ -26,7 +26,6 @@ import com.google.common.truth.Subject; import google.registry.model.EppResource; import google.registry.model.eppcommon.StatusValue; import google.registry.model.reporting.HistoryEntry; -import google.registry.model.transfer.TransferStatus; import google.registry.testing.TruthChainer.And; import google.registry.testing.TruthChainer.Which; import java.util.List; @@ -194,41 +193,6 @@ abstract class AbstractEppResourceSubject "has currentSponsorClientId"); } - public And hasTransferStatus(TransferStatus transferStatus) { - return hasValue( - transferStatus, - actual().getTransferData().getTransferStatus(), - "has transferStatus"); - } - - public And hasTransferRequestClientTrid(String clTrid) { - return hasValue( - clTrid, - actual().getTransferData().getTransferRequestTrid().getClientTransactionId(), - "has trid"); - } - - public And hasPendingTransferExpirationTime(DateTime pendingTransferExpirationTime) { - return hasValue( - pendingTransferExpirationTime, - actual().getTransferData().getPendingTransferExpirationTime(), - "has pendingTransferExpirationTime"); - } - - public And hasTransferGainingClientId(String gainingClientId) { - return hasValue( - gainingClientId, - actual().getTransferData().getGainingClientId(), - "has transfer ga"); - } - - public And hasTransferLosingClientId(String losingClientId) { - return hasValue( - losingClientId, - actual().getTransferData().getLosingClientId(), - "has transfer losingClientId"); - } - public And isActiveAt(DateTime time) { if (!isActive(actual(), time)) { fail("is active at " + time); diff --git a/javatests/google/registry/testing/ContactResourceSubject.java b/javatests/google/registry/testing/ContactResourceSubject.java index b7198c4b6..ea3e2d894 100644 --- a/javatests/google/registry/testing/ContactResourceSubject.java +++ b/javatests/google/registry/testing/ContactResourceSubject.java @@ -22,7 +22,9 @@ import com.google.common.truth.FailureStrategy; import google.registry.model.contact.ContactResource; import google.registry.model.contact.PostalInfo; import google.registry.model.eppcommon.AuthInfo; +import google.registry.model.transfer.TransferStatus; import google.registry.testing.TruthChainer.And; +import org.joda.time.DateTime; /** Truth subject for asserting things about {@link ContactResource} instances. */ public final class ContactResourceSubject @@ -124,6 +126,42 @@ public final class ContactResourceSubject return hasValue(pw, authInfo == null ? null : authInfo.getPw().getValue(), "has auth info pw"); } + public And hasTransferStatus(TransferStatus transferStatus) { + return hasValue( + transferStatus, + actual().getTransferData().getTransferStatus(), + "has transferStatus"); + } + + public And hasTransferRequestClientTrid(String clTrid) { + return hasValue( + clTrid, + actual().getTransferData().getTransferRequestTrid().getClientTransactionId(), + "has trid"); + } + + public And hasPendingTransferExpirationTime( + DateTime pendingTransferExpirationTime) { + return hasValue( + pendingTransferExpirationTime, + actual().getTransferData().getPendingTransferExpirationTime(), + "has pendingTransferExpirationTime"); + } + + public And hasTransferGainingClientId(String gainingClientId) { + return hasValue( + gainingClientId, + actual().getTransferData().getGainingClientId(), + "has transfer ga"); + } + + public And hasTransferLosingClientId(String losingClientId) { + return hasValue( + losingClientId, + actual().getTransferData().getLosingClientId(), + "has transfer losingClientId"); + } + public static DelegatedVerb assertAboutContacts() { return assertAbout(new SubjectFactory()); } diff --git a/javatests/google/registry/testing/DomainResourceSubject.java b/javatests/google/registry/testing/DomainResourceSubject.java index b852e51dc..13039c923 100644 --- a/javatests/google/registry/testing/DomainResourceSubject.java +++ b/javatests/google/registry/testing/DomainResourceSubject.java @@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertAbout; import com.google.common.truth.AbstractVerb.DelegatedVerb; import com.google.common.truth.FailureStrategy; import google.registry.model.domain.DomainResource; +import google.registry.model.transfer.TransferStatus; import google.registry.testing.TruthChainer.And; import java.util.Objects; import org.joda.time.DateTime; @@ -32,6 +33,42 @@ public final class DomainResourceSubject private static class SubjectFactory extends ReflectiveSubjectFactory{} + public And hasTransferStatus(TransferStatus transferStatus) { + return hasValue( + transferStatus, + actual().getTransferData().getTransferStatus(), + "has transferStatus"); + } + + public And hasTransferRequestClientTrid(String clTrid) { + return hasValue( + clTrid, + actual().getTransferData().getTransferRequestTrid().getClientTransactionId(), + "has trid"); + } + + public And hasPendingTransferExpirationTime( + DateTime pendingTransferExpirationTime) { + return hasValue( + pendingTransferExpirationTime, + actual().getTransferData().getPendingTransferExpirationTime(), + "has pendingTransferExpirationTime"); + } + + public And hasTransferGainingClientId(String gainingClientId) { + return hasValue( + gainingClientId, + actual().getTransferData().getGainingClientId(), + "has transfer ga"); + } + + public And hasTransferLosingClientId(String losingClientId) { + return hasValue( + losingClientId, + actual().getTransferData().getLosingClientId(), + "has transfer losingClientId"); + } + public And hasRegistrationExpirationTime(DateTime expiration) { if (!Objects.equals(actual().getRegistrationExpirationTime(), expiration)) { failWithBadResults(