diff --git a/core/src/main/java/google/registry/flows/contact/ContactFlowUtils.java b/core/src/main/java/google/registry/flows/contact/ContactFlowUtils.java index 870e9641d..143cbc9f7 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactFlowUtils.java +++ b/core/src/main/java/google/registry/flows/contact/ContactFlowUtils.java @@ -26,6 +26,7 @@ import google.registry.flows.EppException.ParameterValuePolicyErrorException; import google.registry.flows.EppException.ParameterValueSyntaxErrorException; import google.registry.model.contact.ContactAddress; import google.registry.model.contact.ContactHistory; +import google.registry.model.contact.ContactHistory.ContactHistoryId; import google.registry.model.contact.ContactResource; import google.registry.model.contact.PostalInfo; import google.registry.model.poll.PendingActionNotificationResponse.ContactPendingActionNotificationResponse; @@ -84,7 +85,9 @@ public class ContactFlowUtils { transferData.getTransferStatus().isApproved(), transferData.getTransferRequestTrid(), now))) - .setParentKey(contactHistoryKey) + .setContactHistoryId( + new ContactHistoryId( + contactHistoryKey.getParent().getName(), contactHistoryKey.getId())) .build(); } @@ -96,7 +99,9 @@ public class ContactFlowUtils { .setEventTime(transferData.getPendingTransferExpirationTime()) .setMsg(transferData.getTransferStatus().getMessage()) .setResponseData(ImmutableList.of(createTransferResponse(targetId, transferData))) - .setParentKey(contactHistoryKey) + .setContactHistoryId( + new ContactHistoryId( + contactHistoryKey.getParent().getName(), contactHistoryKey.getId())) .build(); } diff --git a/core/src/main/java/google/registry/flows/contact/ContactTransferRequestFlow.java b/core/src/main/java/google/registry/flows/contact/ContactTransferRequestFlow.java index 165ef00f8..9bf469eec 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactTransferRequestFlow.java +++ b/core/src/main/java/google/registry/flows/contact/ContactTransferRequestFlow.java @@ -134,6 +134,8 @@ public final class ContactTransferRequestFlow implements TransactionalFlow { .asBuilder() .setTransferStatus(TransferStatus.PENDING) .setServerApproveEntities( + serverApproveGainingPollMessage.getContactRepoId(), + contactHistoryKey.getId(), ImmutableSet.of( serverApproveGainingPollMessage.createVKey(), serverApproveLosingPollMessage.createVKey())) diff --git a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java index bc5baa6ff..da230f714 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java @@ -87,6 +87,7 @@ import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainCommand; import google.registry.model.domain.DomainCommand.Create; import google.registry.model.domain.DomainHistory; +import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.GracePeriod; import google.registry.model.domain.Period; import google.registry.model.domain.fee.FeeCreateCommandExtension; @@ -357,7 +358,8 @@ public final class DomainCreateFlow implements TransactionalFlow { .setIdnTableName(validateDomainNameWithIdnTables(domainName)) .setRegistrationExpirationTime(registrationExpirationTime) .setAutorenewBillingEvent(autorenewBillingEvent.createVKey()) - .setAutorenewPollMessage(autorenewPollMessage.createVKey()) + .setAutorenewPollMessage( + autorenewPollMessage.createVKey(), autorenewPollMessage.getHistoryRevisionId()) .setLaunchNotice(hasClaimsNotice ? launchCreate.get().getNotice() : null) .setSmdId(signedMarkId) .setDsData(secDnsCreate.map(SecDnsCreateExtension::getDsData).orElse(null)) @@ -576,7 +578,8 @@ public final class DomainCreateFlow implements TransactionalFlow { .setRegistrarId(registrarId) .setEventTime(registrationExpirationTime) .setMsg("Domain was auto-renewed.") - .setParentKey(domainHistoryKey) + .setDomainHistoryId( + new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) .build(); } @@ -608,7 +611,7 @@ public final class DomainCreateFlow implements TransactionalFlow { ImmutableList.of( DomainPendingActionNotificationResponse.create( fullyQualifiedDomainName, true, historyEntry.getTrid(), now))) - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build(); } diff --git a/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java b/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java index 90ed74a0e..4b766e1af 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java @@ -66,6 +66,7 @@ import google.registry.model.ImmutableObject; import google.registry.model.billing.BillingEvent; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainHistory; +import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.GracePeriod; import google.registry.model.domain.fee.BaseFee.FeeType; import google.registry.model.domain.fee.Credit; @@ -355,7 +356,8 @@ public final class DomainDeleteFlow implements TransactionalFlow { ImmutableList.of( DomainPendingActionNotificationResponse.create( existingDomain.getDomainName(), true, trid, deletionTime))) - .setParentKey(domainHistoryKey) + .setDomainHistoryId( + new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) .build(); } @@ -367,7 +369,8 @@ public final class DomainDeleteFlow implements TransactionalFlow { return new PollMessage.OneTime.Builder() .setRegistrarId(existingDomain.getPersistedCurrentSponsorRegistrarId()) .setEventTime(now) - .setParentKey(domainHistoryKey) + .setDomainHistoryId( + new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) .setMsg( String.format( "Domain %s was deleted by registry administrator with final deletion effective: %s", diff --git a/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java index f24ce5e0f..f33453e29 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java @@ -63,7 +63,6 @@ import com.google.common.collect.Multimaps; import com.google.common.collect.Sets; import com.google.common.collect.Streams; import com.google.common.net.InternetDomainName; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.EppException.AuthorizationErrorException; import google.registry.flows.EppException.CommandUseErrorException; @@ -89,6 +88,7 @@ import google.registry.model.domain.DomainCommand.CreateOrUpdate; import google.registry.model.domain.DomainCommand.InvalidReferencesException; import google.registry.model.domain.DomainCommand.Update; import google.registry.model.domain.DomainHistory; +import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.ForeignKeyedDesignatedContact; import google.registry.model.domain.Period; import google.registry.model.domain.fee.BaseFee; @@ -592,14 +592,15 @@ public class DomainFlowUtils { // where all autorenew poll messages had already been delivered (this would cause the poll // message to be deleted), and then subsequently the transfer was canceled, rejected, or deleted // (which would cause the poll message to be recreated here). - Key existingAutorenewKey = domain.getAutorenewPollMessage().getOfyKey(); PollMessage.Autorenew updatedAutorenewPollMessage = autorenewPollMessage.isPresent() ? autorenewPollMessage.get().asBuilder().setAutorenewEndTime(newEndTime).build() : newAutorenewPollMessage(domain) - .setId(existingAutorenewKey.getId()) + .setId((Long) domain.getAutorenewPollMessage().getSqlKey()) .setAutorenewEndTime(newEndTime) - .setParentKey(existingAutorenewKey.getParent()) + .setDomainHistoryId( + new DomainHistoryId( + domain.getRepoId(), domain.getAutorenewPollMessageHistoryId())) .build(); // If the resultant autorenew poll message would have no poll messages to deliver, then just diff --git a/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java b/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java index 563ba33d7..832745d5d 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java @@ -58,6 +58,7 @@ import google.registry.model.billing.BillingEvent.Recurring; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainCommand.Renew; import google.registry.model.domain.DomainHistory; +import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.DomainRenewData; import google.registry.model.domain.GracePeriod; import google.registry.model.domain.Period; @@ -187,7 +188,9 @@ public final class DomainRenewFlow implements TransactionalFlow { PollMessage.Autorenew newAutorenewPollMessage = newAutorenewPollMessage(existingDomain) .setEventTime(newExpirationTime) - .setParentKey(domainHistoryKey) + .setDomainHistoryId( + new DomainHistoryId( + domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) .build(); // End the old autorenew billing event and poll message now. This may delete the poll message. updateAutorenewRecurrenceEndTime(existingDomain, now); @@ -198,7 +201,9 @@ public final class DomainRenewFlow implements TransactionalFlow { .setLastEppUpdateRegistrarId(registrarId) .setRegistrationExpirationTime(newExpirationTime) .setAutorenewBillingEvent(newAutorenewEvent.createVKey()) - .setAutorenewPollMessage(newAutorenewPollMessage.createVKey()) + .setAutorenewPollMessage( + newAutorenewPollMessage.createVKey(), + newAutorenewPollMessage.getHistoryRevisionId()) .addGracePeriod( GracePeriod.forBillingEvent( GracePeriodStatus.RENEW, existingDomain.getRepoId(), explicitRenewEvent)) diff --git a/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java b/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java index 17aaba76c..a09547066 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java @@ -53,6 +53,7 @@ import google.registry.model.billing.BillingEvent.Reason; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainCommand.Update; import google.registry.model.domain.DomainHistory; +import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.fee.BaseFee.FeeType; import google.registry.model.domain.fee.Fee; import google.registry.model.domain.fee.FeeTransformResponseExtension; @@ -173,7 +174,9 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { newAutorenewPollMessage(existingDomain) .setEventTime(newExpirationTime) .setAutorenewEndTime(END_OF_TIME) - .setParentKey(domainHistoryKey) + .setDomainHistoryId( + new DomainHistoryId( + domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) .build(); DomainBase newDomain = performRestore( @@ -245,7 +248,8 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { .setGracePeriods(null) .setDeletePollMessage(null) .setAutorenewBillingEvent(autorenewEvent.createVKey()) - .setAutorenewPollMessage(autorenewPollMessage.createVKey()) + .setAutorenewPollMessage( + autorenewPollMessage.createVKey(), autorenewPollMessage.getHistoryRevisionId()) // Clear the autorenew end time so if it had expired but is now explicitly being restored, // it won't immediately be deleted again. .setAutorenewEndTime(Optional.empty()) diff --git a/core/src/main/java/google/registry/flows/domain/DomainTransferApproveFlow.java b/core/src/main/java/google/registry/flows/domain/DomainTransferApproveFlow.java index d8cedfb78..c89495f0b 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainTransferApproveFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainTransferApproveFlow.java @@ -51,6 +51,7 @@ import google.registry.model.billing.BillingEvent.Flag; import google.registry.model.billing.BillingEvent.Reason; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainHistory; +import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.GracePeriod; import google.registry.model.domain.metadata.MetadataExtension; import google.registry.model.domain.rgp.GracePeriodStatus; @@ -176,7 +177,9 @@ public final class DomainTransferApproveFlow implements TransactionalFlow { .setEventTime(newExpirationTime) .setAutorenewEndTime(END_OF_TIME) .setMsg("Domain was auto-renewed.") - .setParentKey(domainHistoryKey) + .setDomainHistoryId( + new DomainHistoryId( + domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) .build(); // Construct the post-transfer domain. DomainBase partiallyApprovedDomain = @@ -194,7 +197,9 @@ public final class DomainTransferApproveFlow implements TransactionalFlow { .build()) .setRegistrationExpirationTime(newExpirationTime) .setAutorenewBillingEvent(autorenewEvent.createVKey()) - .setAutorenewPollMessage(gainingClientAutorenewPollMessage.createVKey()) + .setAutorenewPollMessage( + gainingClientAutorenewPollMessage.createVKey(), + gainingClientAutorenewPollMessage.getHistoryRevisionId()) // Remove all the old grace periods and add a new one for the transfer. .setGracePeriods( billingEvent diff --git a/core/src/main/java/google/registry/flows/domain/DomainTransferRequestFlow.java b/core/src/main/java/google/registry/flows/domain/DomainTransferRequestFlow.java index 970fe90dd..eecf84bdb 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainTransferRequestFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainTransferRequestFlow.java @@ -208,6 +208,8 @@ public final class DomainTransferRequestFlow implements TransactionalFlow { // Create the transfer data that represents the pending transfer. DomainTransferData pendingTransferData = createPendingTransferData( + domainAtTransferTime.getRepoId(), + domainHistoryKey.getId(), new DomainTransferData.Builder() .setTransferRequestTrid(trid) .setTransferRequestTime(now) diff --git a/core/src/main/java/google/registry/flows/domain/DomainTransferUtils.java b/core/src/main/java/google/registry/flows/domain/DomainTransferUtils.java index 523576c8e..514fc6442 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainTransferUtils.java +++ b/core/src/main/java/google/registry/flows/domain/DomainTransferUtils.java @@ -26,6 +26,7 @@ import google.registry.model.billing.BillingEvent.Flag; import google.registry.model.billing.BillingEvent.Reason; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainHistory; +import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.GracePeriod; import google.registry.model.domain.Period; import google.registry.model.domain.rgp.GracePeriodStatus; @@ -51,6 +52,8 @@ public final class DomainTransferUtils { /** Sets up {@link TransferData} for a domain with links to entities for server approval. */ public static DomainTransferData createPendingTransferData( + String domainRepoId, + Long historyId, DomainTransferData.Builder transferDataBuilder, ImmutableSet serverApproveEntities, Period transferPeriod) { @@ -82,7 +85,7 @@ public final class DomainTransferUtils { .map(PollMessage.Autorenew.class::cast) .collect(onlyElement()) .createVKey()) - .setServerApproveEntities(serverApproveEntityKeys.build()) + .setServerApproveEntities(domainRepoId, historyId, serverApproveEntityKeys.build()) .setTransferPeriod(transferPeriod) .build(); } @@ -180,7 +183,8 @@ public final class DomainTransferUtils { transferData.getTransferStatus().isApproved(), transferData.getTransferRequestTrid(), now))) - .setParentKey(domainHistoryKey) + .setDomainHistoryId( + new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) .build(); } @@ -197,7 +201,8 @@ public final class DomainTransferUtils { .setResponseData( ImmutableList.of( createTransferResponse(targetId, transferData, extendedRegistrationExpirationTime))) - .setParentKey(domainHistoryKey) + .setDomainHistoryId( + new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) .build(); } @@ -228,7 +233,8 @@ public final class DomainTransferUtils { .setEventTime(serverApproveNewExpirationTime) .setAutorenewEndTime(END_OF_TIME) .setMsg("Domain was auto-renewed.") - .setParentKey(domainHistoryKey) + .setDomainHistoryId( + new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) .build(); } diff --git a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java index c1fb58b3c..a01b4908d 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java @@ -66,6 +66,7 @@ import google.registry.flows.domain.DomainFlowUtils.NameserversNotSpecifiedForTl import google.registry.model.ImmutableObject; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Reason; +import google.registry.model.domain.DesignatedContact; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainCommand.Update; import google.registry.model.domain.DomainCommand.Update.AddRemove; @@ -243,6 +244,13 @@ public final class DomainUpdateFlow implements TransactionalFlow { validateRegistrantIsntBeingRemoved(change); Optional secDnsUpdate = eppInput.getSingleExtension(SecDnsUpdateExtension.class); + + // We have to verify no duplicate contacts _before_ constructing the domain because it is + // illegal to construct a domain with duplicate contacts. + Sets.SetView newContacts = + Sets.union(Sets.difference(domain.getContacts(), remove.getContacts()), add.getContacts()); + validateNoDuplicateContacts(newContacts); + DomainBase.Builder domainBuilder = domain .asBuilder() @@ -263,8 +271,8 @@ public final class DomainUpdateFlow implements TransactionalFlow { .removeStatusValues(remove.getStatusValues()) .addNameservers(add.getNameservers().stream().collect(toImmutableSet())) .removeNameservers(remove.getNameservers().stream().collect(toImmutableSet())) - .addContacts(add.getContacts()) .removeContacts(remove.getContacts()) + .addContacts(add.getContacts()) .setRegistrant(firstNonNull(change.getRegistrant(), domain.getRegistrant())) .setAuthInfo(firstNonNull(change.getAuthInfo(), domain.getAuthInfo())); Optional superuserExt = @@ -286,7 +294,6 @@ public final class DomainUpdateFlow implements TransactionalFlow { } private void validateNewState(DomainBase newDomain) throws EppException { - validateNoDuplicateContacts(newDomain.getContacts()); validateRequiredContactsPresent(newDomain.getRegistrant(), newDomain.getContacts()); validateDsData(newDomain.getDsData()); validateNameserversCountForTld( @@ -360,7 +367,7 @@ public final class DomainUpdateFlow implements TransactionalFlow { return Optional.ofNullable( new PollMessage.OneTime.Builder() - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .setEventTime(now) .setRegistrarId(existingDomain.getCurrentSponsorRegistrarId()) .setMsg(msg) diff --git a/core/src/main/java/google/registry/model/EppResourceUtils.java b/core/src/main/java/google/registry/model/EppResourceUtils.java index f6097c64a..9c040f68d 100644 --- a/core/src/main/java/google/registry/model/EppResourceUtils.java +++ b/core/src/main/java/google/registry/model/EppResourceUtils.java @@ -226,7 +226,7 @@ public final class EppResourceUtils { checkArgument(TransferStatus.PENDING.equals(transferData.getTransferStatus())); TransferData.Builder transferDataBuilder = transferData.asBuilder(); transferDataBuilder.setTransferStatus(TransferStatus.SERVER_APPROVED); - transferDataBuilder.setServerApproveEntities(null); + transferDataBuilder.setServerApproveEntities(null, null, null); if (transferData instanceof DomainTransferData) { ((DomainTransferData.Builder) transferDataBuilder) .setServerApproveBillingEvent(null) diff --git a/core/src/main/java/google/registry/model/ResourceTransferUtils.java b/core/src/main/java/google/registry/model/ResourceTransferUtils.java index 747c7fe47..478c704f9 100644 --- a/core/src/main/java/google/registry/model/ResourceTransferUtils.java +++ b/core/src/main/java/google/registry/model/ResourceTransferUtils.java @@ -128,7 +128,7 @@ public final class ResourceTransferUtils { createTransferResponse(newResource, newResource.getTransferData()), createPendingTransferNotificationResponse( resource, oldTransferData.getTransferRequestTrid(), false, now))) - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build()); } } diff --git a/core/src/main/java/google/registry/model/contact/ContactHistory.java b/core/src/main/java/google/registry/model/contact/ContactHistory.java index 03f8ae351..573149d5f 100644 --- a/core/src/main/java/google/registry/model/contact/ContactHistory.java +++ b/core/src/main/java/google/registry/model/contact/ContactHistory.java @@ -84,6 +84,10 @@ public class ContactHistory extends HistoryEntry implements UnsafeSerializable { return super.getId(); } + public ContactHistoryId getContactHistoryId() { + return new ContactHistoryId(getContactRepoId(), getId()); + } + /** * The values of all the fields on the {@link ContactBase} object after the action represented by * this history object was executed. @@ -147,8 +151,7 @@ public class ContactHistory extends HistoryEntry implements UnsafeSerializable { * *

This method is private because it is only used by Hibernate. */ - @SuppressWarnings("unused") - private String getContactRepoId() { + public String getContactRepoId() { return contactRepoId; } @@ -157,8 +160,7 @@ public class ContactHistory extends HistoryEntry implements UnsafeSerializable { * *

This method is private because it is only used by Hibernate. */ - @SuppressWarnings("unused") - private long getId() { + public long getId() { return id; } diff --git a/core/src/main/java/google/registry/model/domain/DomainBase.java b/core/src/main/java/google/registry/model/domain/DomainBase.java index c94ca4987..331089d3a 100644 --- a/core/src/main/java/google/registry/model/domain/DomainBase.java +++ b/core/src/main/java/google/registry/model/domain/DomainBase.java @@ -150,9 +150,7 @@ public class DomainBase extends DomainContent implements ForeignKeyedEppResource /** Post-load method to eager load the collections. */ @PostLoad - @Override protected void postLoad() { - super.postLoad(); // TODO(b/188044616): Determine why Eager loading doesn't work here. Hibernate.initialize(dsData); Hibernate.initialize(gracePeriods); @@ -190,7 +188,9 @@ public class DomainBase extends DomainContent implements ForeignKeyedEppResource public Builder copyFrom(DomainContent domainContent) { this.getInstance().copyUpdateTimestamp(domainContent); return this.setAuthInfo(domainContent.getAuthInfo()) - .setAutorenewPollMessage(domainContent.getAutorenewPollMessage()) + .setAutorenewPollMessage( + domainContent.getAutorenewPollMessage(), + domainContent.getAutorenewPollMessageHistoryId()) .setAutorenewBillingEvent(domainContent.getAutorenewBillingEvent()) .setAutorenewEndTime(domainContent.getAutorenewEndTime()) .setContacts(domainContent.getContacts()) diff --git a/core/src/main/java/google/registry/model/domain/DomainContent.java b/core/src/main/java/google/registry/model/domain/DomainContent.java index be4d90d0e..bb83fa171 100644 --- a/core/src/main/java/google/registry/model/domain/DomainContent.java +++ b/core/src/main/java/google/registry/model/domain/DomainContent.java @@ -42,12 +42,10 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Ordering; import com.google.common.collect.Sets; -import com.google.common.collect.Streams; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Ignore; import com.googlecode.objectify.annotation.IgnoreSave; import com.googlecode.objectify.annotation.Index; -import com.googlecode.objectify.annotation.OnLoad; import com.googlecode.objectify.condition.IfNull; import google.registry.dns.RefreshDnsAction; import google.registry.flows.ResourceFlowUtils; @@ -82,7 +80,6 @@ import javax.persistence.Column; import javax.persistence.Embeddable; import javax.persistence.Embedded; import javax.persistence.MappedSuperclass; -import javax.persistence.PostLoad; import javax.persistence.Transient; import org.hibernate.collection.internal.PersistentSet; import org.joda.time.DateTime; @@ -139,23 +136,12 @@ public class DomainContent extends EppResource /** References to hosts that are the nameservers for the domain. */ @EmptySetToNull @Index @Transient Set> nsHosts; - /** - * The union of the contacts visible via {@link #getContacts} and {@link #getRegistrant}. - * - *

These are stored in one field so that we can query across all contacts at once. - */ - @Transient Set allContacts; + /** Contacts. */ + VKey adminContact; - /** - * Contacts as they are stored in cloud SQL. - * - *

This information is duplicated in allContacts, and must be kept in sync with it. - */ - @Ignore VKey adminContact; - - @Ignore VKey billingContact; - @Ignore VKey techContact; - @Ignore VKey registrantContact; + VKey billingContact; + VKey techContact; + VKey registrantContact; /** Authorization info (aka transfer secret) of the domain. */ @Embedded @@ -165,12 +151,7 @@ public class DomainContent extends EppResource }) DomainAuthInfo authInfo; - /** - * Data used to construct DS records for this domain. - * - *

This is {@literal @}XmlTransient because it needs to be returned under the "extension" tag - * of an info response rather than inside the "infData" tag. - */ + /** Data used to construct DS records for this domain. */ @Transient Set dsData; /** @@ -178,7 +159,6 @@ public class DomainContent extends EppResource * *

It's {@literal @}XmlTransient because it's not returned in an info response. */ - @IgnoreSave(IfNull.class) @Embedded @AttributeOverrides({ @AttributeOverride(name = "noticeId.tcnId", column = @Column(name = "launch_notice_tcn_id")), @@ -218,15 +198,6 @@ public class DomainContent extends EppResource @Column(name = "deletion_poll_message_id") VKey deletePollMessage; - /** - * History record for the delete poll message. - * - *

Here so we can restore the original ofy key from sql. - */ - @Column(name = "deletion_poll_message_history_id") - @Ignore - Long deletePollMessageHistoryId; - /** * The recurring billing event associated with this domain's autorenewals. * @@ -238,15 +209,6 @@ public class DomainContent extends EppResource @Column(name = "billing_recurrence_id") VKey autorenewBillingEvent; - /** - * History record for the autorenew billing event. - * - *

Here so we can restore the original ofy key from sql. - */ - @Column(name = "billing_recurrence_history_id") - @Ignore - Long autorenewBillingEventHistoryId; - /** * The recurring poll message associated with this domain's autorenewals. * @@ -332,75 +294,6 @@ public class DomainContent extends EppResource return Optional.ofNullable(dnsRefreshRequestTime); } - @OnLoad - void load() { - // Reconstitute all of the contacts so that they have VKeys. - allContacts = - allContacts.stream().map(DesignatedContact::reconstitute).collect(toImmutableSet()); - setContactFields(allContacts, true); - - // We have to return the cloned object here because the original object's hashcode is not - // correct due to the change to its domainRepoId and history ids. The cloned object will have a - // null hashcode so that it can get a recalculated hashcode when its hashCode() is invoked. - // TODO(b/162739503): Remove this after fully migrating to Cloud SQL. - gracePeriods = - nullToEmptyImmutableCopy(gracePeriods).stream() - .map(gracePeriod -> gracePeriod.cloneAfterOfyLoad(getRepoId())) - .collect(toImmutableSet()); - - // Restore history record ids. - autorenewPollMessageHistoryId = getHistoryId(autorenewPollMessage); - autorenewBillingEventHistoryId = getHistoryId(autorenewBillingEvent); - deletePollMessageHistoryId = getHistoryId(deletePollMessage); - - // Fix PollMessage VKeys. - autorenewPollMessage = PollMessage.Autorenew.convertVKey(autorenewPollMessage); - deletePollMessage = PollMessage.OneTime.convertVKey(deletePollMessage); - - dsData = - nullToEmptyImmutableCopy(dsData).stream() - .map(dsData -> dsData.cloneWithDomainRepoId(getRepoId())) - .collect(toImmutableSet()); - - if (transferData != null) { - transferData.convertVKeys(); - } - } - - @PostLoad - protected void postLoad() { - // Reconstitute the contact list. - ImmutableSet.Builder contactsBuilder = new ImmutableSet.Builder<>(); - - if (registrantContact != null) { - contactsBuilder.add( - DesignatedContact.create(DesignatedContact.Type.REGISTRANT, registrantContact)); - } - if (billingContact != null) { - contactsBuilder.add(DesignatedContact.create(DesignatedContact.Type.BILLING, billingContact)); - } - if (techContact != null) { - contactsBuilder.add(DesignatedContact.create(DesignatedContact.Type.TECH, techContact)); - } - if (adminContact != null) { - contactsBuilder.add(DesignatedContact.create(DesignatedContact.Type.ADMIN, adminContact)); - } - - allContacts = contactsBuilder.build(); - - // Reconstitute the composite ofy keys from the SQL data. - Key myKey = Key.create(DomainBase.class, getRepoId()); - deletePollMessage = restoreOfyFrom(myKey, deletePollMessage, deletePollMessageHistoryId); - autorenewBillingEvent = - restoreOfyFrom(myKey, autorenewBillingEvent, autorenewBillingEventHistoryId); - autorenewPollMessage = - restoreOfyFrom(myKey, autorenewPollMessage, autorenewPollMessageHistoryId); - - if (transferData != null) { - transferData.restoreOfyKeys(myKey); - } - } - public static VKey restoreOfyFrom(Key domainKey, VKey key, Long historyId) { if (historyId == null) { // This is a legacy key (or a null key, in which case this works too) @@ -430,6 +323,10 @@ public class DomainContent extends EppResource return autorenewPollMessage; } + public Long getAutorenewPollMessageHistoryId() { + return autorenewPollMessageHistoryId; + } + public ImmutableSet getGracePeriods() { return nullToEmptyImmutableCopy(gracePeriods); } @@ -609,7 +506,9 @@ public class DomainContent extends EppResource // Set the speculatively-written new autorenew events as the domain's autorenew // events. .setAutorenewBillingEvent(transferData.getServerApproveAutorenewEvent()) - .setAutorenewPollMessage(transferData.getServerApproveAutorenewPollMessage()); + .setAutorenewPollMessage( + transferData.getServerApproveAutorenewPollMessage(), + transferData.getServerApproveAutorenewPollMessageHistoryId()); if (transferData.getTransferPeriod().getValue() == 1) { // Set the grace period using a key to the prescheduled transfer billing event. Not using // GracePeriod.forBillingEvent() here in order to avoid the actual Datastore fetch. @@ -731,9 +630,7 @@ public class DomainContent extends EppResource /** Associated contacts for the domain (other than registrant). */ public ImmutableSet getContacts() { - return nullToEmpty(allContacts).stream() - .filter(IS_REGISTRANT.negate()) - .collect(toImmutableSet()); + return getAllContacts(false); } public DomainAuthInfo getAuthInfo() { @@ -742,12 +639,29 @@ public class DomainContent extends EppResource /** Returns all referenced contacts from this domain. */ public ImmutableSet> getReferencedContacts() { - return nullToEmptyImmutableCopy(allContacts).stream() + return nullToEmptyImmutableCopy(getAllContacts(true)).stream() .map(DesignatedContact::getContactKey) .filter(Objects::nonNull) .collect(toImmutableSet()); } + private ImmutableSet getAllContacts(boolean includeRegistrant) { + ImmutableSet.Builder builder = new ImmutableSet.Builder<>(); + if (includeRegistrant && registrantContact != null) { + builder.add(DesignatedContact.create(DesignatedContact.Type.REGISTRANT, registrantContact)); + } + if (adminContact != null) { + builder.add(DesignatedContact.create(DesignatedContact.Type.ADMIN, adminContact)); + } + if (billingContact != null) { + builder.add(DesignatedContact.create(DesignatedContact.Type.BILLING, billingContact)); + } + if (techContact != null) { + builder.add(DesignatedContact.create(DesignatedContact.Type.TECH, techContact)); + } + return builder.build(); + } + public String getTld() { return tld; } @@ -764,7 +678,13 @@ public class DomainContent extends EppResource if (includeRegistrant) { registrantContact = null; } + HashSet contactsDiscovered = new HashSet(); for (DesignatedContact contact : contacts) { + checkArgument( + !contactsDiscovered.contains(contact.getType()), + "Duplicate contact type %s in designated contact set.", + contact.getType()); + contactsDiscovered.add(contact.getType()); switch (contact.getType()) { case BILLING: billingContact = contact.getContactKey(); @@ -793,24 +713,6 @@ public class DomainContent extends EppResource + " use DomainBase instead"); } - /** - * Obtains a history id from the given key. - * - *

The key must be a composite key either of the form domain-key/history-key/long-event-key or - * EntityGroupRoot/long-event-key (for legacy keys). In the latter case or for a null key returns - * a history id of null. - */ - public static Long getHistoryId(VKey key) { - if (key == null) { - return null; - } - Key parent = key.getOfyKey().getParent(); - if (parent == null || parent.getKind().equals("EntityGroupRoot")) { - return null; - } - return parent.getId(); - } - /** Predicate to determine if a given {@link DesignatedContact} is the registrant. */ static final Predicate IS_REGISTRANT = (DesignatedContact contact) -> DesignatedContact.Type.REGISTRANT.equals(contact.type); @@ -848,10 +750,6 @@ public class DomainContent extends EppResource instance.autorenewEndTime = firstNonNull(getInstance().autorenewEndTime, END_OF_TIME); checkArgumentNotNull(emptyToNull(instance.fullyQualifiedDomainName), "Missing domainName"); - if (instance.getRegistrant() == null - && instance.allContacts.stream().anyMatch(IS_REGISTRANT)) { - throw new IllegalArgumentException("registrant is null but is in allContacts"); - } checkArgumentNotNull(instance.getRegistrant(), "Missing registrant"); instance.tld = getTldFromDomainName(instance.fullyQualifiedDomainName); @@ -885,13 +783,6 @@ public class DomainContent extends EppResource } public B setRegistrant(VKey registrant) { - // Replace the registrant contact inside allContacts. - getInstance().allContacts = - union( - getInstance().getContacts(), - DesignatedContact.create( - DesignatedContact.Type.REGISTRANT, checkArgumentNotNull(registrant))); - // Set the registrant field specifically. getInstance().registrantContact = registrant; return thisCastToDerived(); @@ -939,13 +830,6 @@ public class DomainContent extends EppResource public B setContacts(ImmutableSet contacts) { checkArgument(contacts.stream().noneMatch(IS_REGISTRANT), "Registrant cannot be a contact"); - // Replace the non-registrant contacts inside allContacts. - getInstance().allContacts = - Streams.concat( - nullToEmpty(getInstance().allContacts).stream().filter(IS_REGISTRANT), - contacts.stream()) - .collect(toImmutableSet()); - // Set the individual fields. getInstance().setContactFields(contacts, false); return thisCastToDerived(); @@ -992,19 +876,19 @@ public class DomainContent extends EppResource public B setDeletePollMessage(VKey deletePollMessage) { getInstance().deletePollMessage = deletePollMessage; - getInstance().deletePollMessageHistoryId = getHistoryId(deletePollMessage); return thisCastToDerived(); } public B setAutorenewBillingEvent(VKey autorenewBillingEvent) { getInstance().autorenewBillingEvent = autorenewBillingEvent; - getInstance().autorenewBillingEventHistoryId = getHistoryId(autorenewBillingEvent); return thisCastToDerived(); } - public B setAutorenewPollMessage(VKey autorenewPollMessage) { + public B setAutorenewPollMessage( + @Nullable VKey autorenewPollMessage, + @Nullable Long autorenewPollMessageHistoryId) { getInstance().autorenewPollMessage = autorenewPollMessage; - getInstance().autorenewPollMessageHistoryId = getHistoryId(autorenewPollMessage); + getInstance().autorenewPollMessageHistoryId = autorenewPollMessageHistoryId; return thisCastToDerived(); } diff --git a/core/src/main/java/google/registry/model/domain/DomainHistory.java b/core/src/main/java/google/registry/model/domain/DomainHistory.java index 30b2c5b6c..5b1b1c3b9 100644 --- a/core/src/main/java/google/registry/model/domain/DomainHistory.java +++ b/core/src/main/java/google/registry/model/domain/DomainHistory.java @@ -326,23 +326,13 @@ public class DomainHistory extends HistoryEntry { this.id = id; } - /** - * Returns the domain repository id. - * - *

This method is private because it is only used by Hibernate. - */ - @SuppressWarnings("unused") - private String getDomainRepoId() { + /** Returns the domain repository id. */ + public String getDomainRepoId() { return domainRepoId; } - /** - * Returns the history revision id. - * - *

This method is private because it is only used by Hibernate. - */ - @SuppressWarnings("unused") - private long getId() { + /** Returns the history revision id. */ + public long getId() { return id; } diff --git a/core/src/main/java/google/registry/model/domain/GracePeriod.java b/core/src/main/java/google/registry/model/domain/GracePeriod.java index 9db7171f6..94dc6adb7 100644 --- a/core/src/main/java/google/registry/model/domain/GracePeriod.java +++ b/core/src/main/java/google/registry/model/domain/GracePeriod.java @@ -24,8 +24,6 @@ import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Recurring; import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.rgp.GracePeriodStatus; -import google.registry.persistence.BillingVKey.BillingEventVKey; -import google.registry.persistence.BillingVKey.BillingRecurrenceVKey; import google.registry.persistence.VKey; import javax.annotation.Nullable; import javax.persistence.Access; @@ -78,8 +76,8 @@ public class GracePeriod extends GracePeriodBase { instance.domainRepoId = checkArgumentNotNull(domainRepoId); instance.expirationTime = checkArgumentNotNull(expirationTime); instance.clientId = checkArgumentNotNull(registrarId); - instance.billingEventOneTime = BillingEventVKey.create(billingEventOneTime); - instance.billingEventRecurring = BillingRecurrenceVKey.create(billingEventRecurring); + instance.billingEventOneTime = billingEventOneTime; + instance.billingEventRecurring = billingEventRecurring; return instance; } @@ -126,8 +124,8 @@ public class GracePeriod extends GracePeriodBase { history.domainRepoId, history.expirationTime, history.clientId, - history.billingEventOneTime == null ? null : history.billingEventOneTime.createVKey(), - history.billingEventRecurring == null ? null : history.billingEventRecurring.createVKey(), + history.billingEventOneTime, + history.billingEventRecurring, history.gracePeriodId); } diff --git a/core/src/main/java/google/registry/model/domain/GracePeriodBase.java b/core/src/main/java/google/registry/model/domain/GracePeriodBase.java index 0c22ed065..57074b3a0 100644 --- a/core/src/main/java/google/registry/model/domain/GracePeriodBase.java +++ b/core/src/main/java/google/registry/model/domain/GracePeriodBase.java @@ -20,8 +20,6 @@ import google.registry.model.ImmutableObject; import google.registry.model.UnsafeSerializable; import google.registry.model.billing.BillingEvent; import google.registry.model.domain.rgp.GracePeriodStatus; -import google.registry.persistence.BillingVKey.BillingEventVKey; -import google.registry.persistence.BillingVKey.BillingRecurrenceVKey; import google.registry.persistence.VKey; import javax.persistence.Access; import javax.persistence.AccessType; @@ -66,7 +64,8 @@ public class GracePeriodBase extends ImmutableObject implements UnsafeSerializab */ // NB: Would @IgnoreSave(IfNull.class), but not allowed for @Embed collections. @Access(AccessType.FIELD) - BillingEventVKey billingEventOneTime = null; + @Column(name = "billing_event_id") + VKey billingEventOneTime = null; /** * The recurring billing event corresponding to the action that triggered this grace period, if @@ -74,7 +73,8 @@ public class GracePeriodBase extends ImmutableObject implements UnsafeSerializab */ // NB: Would @IgnoreSave(IfNull.class), but not allowed for @Embed collections. @Access(AccessType.FIELD) - BillingRecurrenceVKey billingEventRecurring = null; + @Column(name = "billing_recurrence_id") + VKey billingEventRecurring = null; public long getGracePeriodId() { return gracePeriodId; @@ -112,7 +112,7 @@ public class GracePeriodBase extends ImmutableObject implements UnsafeSerializab * period is not AUTO_RENEW. */ public VKey getOneTimeBillingEvent() { - return billingEventOneTime == null ? null : billingEventOneTime.createVKey(); + return billingEventOneTime; } /** @@ -120,6 +120,6 @@ public class GracePeriodBase extends ImmutableObject implements UnsafeSerializab * period is AUTO_RENEW. */ public VKey getRecurringBillingEvent() { - return billingEventRecurring == null ? null : billingEventRecurring.createVKey(); + return billingEventRecurring; } } diff --git a/core/src/main/java/google/registry/model/host/HostHistory.java b/core/src/main/java/google/registry/model/host/HostHistory.java index ea40fc322..ee0f61e56 100644 --- a/core/src/main/java/google/registry/model/host/HostHistory.java +++ b/core/src/main/java/google/registry/model/host/HostHistory.java @@ -85,6 +85,10 @@ public class HostHistory extends HistoryEntry implements UnsafeSerializable { return super.getId(); } + public HostHistoryId getHostHistoryId() { + return new HostHistoryId(getHostRepoId(), getId()); + } + /** * The values of all the fields on the {@link HostBase} object after the action represented by * this history object was executed. @@ -147,8 +151,7 @@ public class HostHistory extends HistoryEntry implements UnsafeSerializable { * *

This method is private because it is only used by Hibernate. */ - @SuppressWarnings("unused") - private String getHostRepoId() { + public String getHostRepoId() { return hostRepoId; } @@ -157,8 +160,7 @@ public class HostHistory extends HistoryEntry implements UnsafeSerializable { * *

This method is private because it is only used by Hibernate. */ - @SuppressWarnings("unused") - private long getId() { + public long getId() { return id; } diff --git a/core/src/main/java/google/registry/model/poll/PollMessage.java b/core/src/main/java/google/registry/model/poll/PollMessage.java index 3fce7a691..5430838b4 100644 --- a/core/src/main/java/google/registry/model/poll/PollMessage.java +++ b/core/src/main/java/google/registry/model/poll/PollMessage.java @@ -14,10 +14,8 @@ package google.registry.model.poll; -import static com.google.common.collect.ImmutableList.toImmutableList; -import static google.registry.util.CollectionUtils.forceEmptyToNull; -import static google.registry.util.CollectionUtils.isNullOrEmpty; -import static google.registry.util.CollectionUtils.nullToEmpty; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; @@ -28,18 +26,22 @@ import com.googlecode.objectify.annotation.EntitySubclass; import com.googlecode.objectify.annotation.Id; import com.googlecode.objectify.annotation.Ignore; import com.googlecode.objectify.annotation.Index; -import com.googlecode.objectify.annotation.OnLoad; -import com.googlecode.objectify.annotation.Parent; import google.registry.model.Buildable; import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.model.UnsafeSerializable; import google.registry.model.annotations.ExternalMessagingName; import google.registry.model.annotations.ReportedOn; +import google.registry.model.contact.ContactHistory; +import google.registry.model.contact.ContactHistory.ContactHistoryId; import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; +import google.registry.model.domain.DomainHistory; +import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.DomainRenewData; import google.registry.model.eppoutput.EppResponse.ResponseData; +import google.registry.model.host.HostHistory; +import google.registry.model.host.HostHistory.HostHistoryId; import google.registry.model.host.HostResource; import google.registry.model.poll.PendingActionNotificationResponse.ContactPendingActionNotificationResponse; import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse; @@ -51,7 +53,7 @@ import google.registry.model.transfer.TransferResponse.ContactTransferResponse; import google.registry.model.transfer.TransferResponse.DomainTransferResponse; import google.registry.persistence.VKey; import google.registry.persistence.WithLongVKey; -import java.util.List; +import google.registry.util.NullIgnoringCollectionBuilder; import java.util.Optional; import javax.annotation.Nullable; import javax.persistence.AttributeOverride; @@ -63,7 +65,6 @@ import javax.persistence.Embedded; import javax.persistence.Inheritance; import javax.persistence.InheritanceType; import javax.persistence.PostLoad; -import javax.persistence.Transient; import org.joda.time.DateTime; /** @@ -100,14 +101,54 @@ import org.joda.time.DateTime; public abstract class PollMessage extends ImmutableObject implements Buildable, TransferServerApproveEntity, UnsafeSerializable { + /** Indicates the type of entity the poll message is for. */ + public enum Type { + DOMAIN(1L, DomainBase.class), + CONTACT(2L, ContactResource.class), + HOST(3L, HostResource.class); + + private final long id; + private final Class clazz; + + Type(long id, Class clazz) { + this.id = id; + this.clazz = clazz; + } + + /** + * Returns a numeric id for the enum, which is used as part of an externally published string + * key for the message. + */ + public long getId() { + return id; + } + + /** Returns the class of the underlying resource for the poll message type. */ + public Class getResourceClass() { + return clazz; + } + + /** + * Returns the type specified by the identifer, {@code Optional.empty()} if the id is out of + * range. + */ + public static Optional fromId(long id) { + for (Type val : values()) { + if (val.id == id) { + return Optional.of(val); + } + } + + return Optional.empty(); + } + } + /** Entity id. */ @Id @javax.persistence.Id @Column(name = "poll_message_id") Long id; - @Parent @DoNotHydrate @Transient Key parent; - /** The registrar that this poll message will be delivered to. */ @Index @Column(name = "registrar_id", nullable = false) @@ -122,6 +163,8 @@ public abstract class PollMessage extends ImmutableObject @Column(name = "message") String msg; + // TODO(b/456803336): Replace these fields with {Domain,Contact,Host}HistoryId objects. + @Ignore String domainRepoId; @Ignore String contactRepoId; @@ -134,10 +177,6 @@ public abstract class PollMessage extends ImmutableObject @Ignore Long hostHistoryRevisionId; - public Key getParentKey() { - return parent; - } - public Long getId() { return id; } @@ -154,36 +193,59 @@ public abstract class PollMessage extends ImmutableObject return msg; } + /** + * Returns the domain repo id. + * + *

This may only be used on a Domain poll event. + */ + public String getDomainRepoId() { + checkArgument(getType() == Type.DOMAIN); + return domainRepoId; + } + + /** + * Returns the contact repo id. + * + *

This may only be used on a ContactResource poll event. + */ + public String getContactRepoId() { + checkArgument(getType() == Type.CONTACT); + return contactRepoId; + } + + /** + * Returns the host repo id. + * + *

This may only be used on a HostResource poll event. + */ + public String getHostRepoId() { + checkArgument(getType() == Type.DOMAIN); + return hostRepoId; + } + + /** + * Gets the name of the underlying resource that the PollMessage is for, regardless of the type of + * the resource. + */ + public String getResourceName() { + return domainRepoId != null + ? domainRepoId + : (contactRepoId != null ? contactRepoId : hostRepoId); + } + + /** Gets the underlying history revision id, regardless of the type of the resource. */ + public Long getHistoryRevisionId() { + return domainHistoryRevisionId != null + ? domainHistoryRevisionId + : (contactHistoryRevisionId != null ? contactHistoryRevisionId : hostHistoryRevisionId); + } + + public Type getType() { + return domainRepoId != null ? Type.DOMAIN : (contactRepoId != null ? Type.CONTACT : Type.HOST); + } + public abstract ImmutableList getResponseData(); - @PostLoad - void postLoad() { - if (domainRepoId != null) { - parent = - Key.create( - Key.create(DomainBase.class, domainRepoId), - HistoryEntry.class, - domainHistoryRevisionId); - } else if (contactRepoId != null) { - parent = - Key.create( - Key.create(ContactResource.class, contactRepoId), - HistoryEntry.class, - contactHistoryRevisionId); - } else if (hostHistoryRevisionId != null) { - parent = - Key.create( - Key.create(HostResource.class, hostRepoId), - HistoryEntry.class, - hostHistoryRevisionId); - } - } - - @OnLoad - void onLoad() { - setSqlForeignKeys(this); - } - @Override public abstract VKey createVKey(); @@ -234,46 +296,77 @@ public abstract class PollMessage extends ImmutableObject return thisCastToDerived(); } - public B setParent(HistoryEntry parent) { - getInstance().parent = Key.create(parent); + public B setDomainHistoryId(DomainHistoryId historyId) { + getInstance().domainRepoId = historyId.getDomainRepoId(); + getInstance().domainHistoryRevisionId = historyId.getId(); return thisCastToDerived(); } - public B setParentKey(Key parentKey) { - getInstance().parent = parentKey; + public B setContactHistoryId(ContactHistoryId historyId) { + getInstance().contactRepoId = historyId.getContactRepoId(); + getInstance().contactHistoryRevisionId = historyId.getId(); return thisCastToDerived(); } + public B setHostHistoryId(HostHistoryId historyId) { + getInstance().hostRepoId = historyId.getHostRepoId(); + getInstance().hostHistoryRevisionId = historyId.getId(); + return thisCastToDerived(); + } + + public B setHistoryEntry(HistoryEntry history) { + // Set the appropriate field based on the history entry type. + if (history instanceof DomainHistory) { + return setDomainHistoryId(((DomainHistory) history).getDomainHistoryId()); + } else if (history instanceof ContactHistory) { + return setContactHistoryId(((ContactHistory) history).getContactHistoryId()); + } else if (history instanceof HostHistory) { + return setHostHistoryId(((HostHistory) history).getHostHistoryId()); + } + return thisCastToDerived(); + } + + /** + * Given an array containing pairs of objects, verifies that both members of exactly one of the + * pairs is non-null. + */ + private boolean exactlyOnePairNonNull(Object... pairs) { + int count = 0; + checkArgument(pairs.length % 2 == 0, "Odd number of arguments provided."); + for (int i = 0; i < pairs.length; i += 2) { + // Add the number of non-null elements of each pair, after which the count should either be + // zero (a non-null pair hasn't been found yet) or two (exactly one non-null pair has been + // found). + count += (pairs[i] != null ? 1 : 0) + (pairs[i + 1] != null ? 1 : 0); + if (count != 0 && count != 2) { + return false; + } + } + + // Verify that we've found a non-null pair. + return count == 2; + } + @Override public T build() { T instance = getInstance(); checkArgumentNotNull(instance.clientId, "clientId must be specified"); checkArgumentNotNull(instance.eventTime, "eventTime must be specified"); - checkArgumentNotNull(instance.parent, "parent must be specified"); - checkArgumentNotNull(instance.parent.getParent(), "parent.getParent() must be specified"); - setSqlForeignKeys(instance); + checkState( + exactlyOnePairNonNull( + instance.domainRepoId, + instance.domainHistoryRevisionId, + instance.contactRepoId, + instance.contactHistoryRevisionId, + instance.hostRepoId, + instance.hostHistoryRevisionId), + "the repo id and history revision id must be defined for exactly one of domain, " + + "contact or host: " + + instance); return super.build(); } } - private static void setSqlForeignKeys(PollMessage pollMessage) { - String grandparentKind = pollMessage.parent.getParent().getKind(); - String repoId = pollMessage.parent.getParent().getName(); - long historyRevisionId = pollMessage.parent.getId(); - if (Key.getKind(DomainBase.class).equals(grandparentKind)) { - pollMessage.domainRepoId = repoId; - pollMessage.domainHistoryRevisionId = historyRevisionId; - } else if (Key.getKind(ContactResource.class).equals(grandparentKind)) { - pollMessage.contactRepoId = repoId; - pollMessage.contactHistoryRevisionId = historyRevisionId; - } else if (Key.getKind(HostResource.class).equals(grandparentKind)) { - pollMessage.hostRepoId = repoId; - pollMessage.hostHistoryRevisionId = historyRevisionId; - } else { - throw new IllegalArgumentException("Unknown grandparent kind: " + grandparentKind); - } - } - /** * A one-time poll message. * @@ -285,20 +378,6 @@ public abstract class PollMessage extends ImmutableObject @WithLongVKey(compositeKey = true) public static class OneTime extends PollMessage { - // Response data. Objectify cannot persist a base class type, so we must have a separate field - // to hold every possible derived type of ResponseData that we might store. - @Transient - List contactPendingActionNotificationResponses; - - @Transient List contactTransferResponses; - - @Transient @ImmutableObject.DoNotCompare - List domainPendingActionNotificationResponses; - - @Transient @ImmutableObject.DoNotCompare List domainTransferResponses; - - @Transient List hostPendingActionNotificationResponses; - @Ignore @Embedded @AttributeOverrides({ @@ -379,78 +458,38 @@ public abstract class PollMessage extends ImmutableObject @Override public ImmutableList getResponseData() { - return new ImmutableList.Builder() - .addAll(nullToEmpty(contactPendingActionNotificationResponses)) - .addAll(nullToEmpty(contactTransferResponses)) - .addAll(nullToEmpty(domainPendingActionNotificationResponses)) - .addAll(nullToEmpty(domainTransferResponses)) - .addAll(nullToEmpty(hostPendingActionNotificationResponses)) + return NullIgnoringCollectionBuilder.create(new ImmutableList.Builder()) + .add(pendingActionNotificationResponse) + .add(transferResponse) + .getBuilder() .build(); } - @Override - @OnLoad - void onLoad() { - super.onLoad(); - // Take the Objectify-specific fields and map them to the SQL-specific fields, if applicable - if (!isNullOrEmpty(contactPendingActionNotificationResponses)) { - contactId = contactPendingActionNotificationResponses.get(0).getId().value; - pendingActionNotificationResponse = contactPendingActionNotificationResponses.get(0); - } - if (!isNullOrEmpty(hostPendingActionNotificationResponses)) { - hostId = hostPendingActionNotificationResponses.get(0).nameOrId.value; - pendingActionNotificationResponse = hostPendingActionNotificationResponses.get(0); - } - if (!isNullOrEmpty(contactTransferResponses)) { - contactId = contactTransferResponses.get(0).getContactId(); - transferResponse = contactTransferResponses.get(0); - } - if (!isNullOrEmpty(domainPendingActionNotificationResponses)) { - pendingActionNotificationResponse = domainPendingActionNotificationResponses.get(0); - fullyQualifiedDomainName = pendingActionNotificationResponse.nameOrId.value; - } - if (!isNullOrEmpty(domainTransferResponses)) { - fullyQualifiedDomainName = domainTransferResponses.get(0).getFullyQualifiedDomainName(); - transferResponse = domainTransferResponses.get(0); - extendedRegistrationExpirationTime = - domainTransferResponses.get(0).getExtendedRegistrationExpirationTime(); - } - } - - @Override @PostLoad void postLoad() { - super.postLoad(); - // Take the SQL-specific fields and map them to the Objectify-specific fields, if applicable if (pendingActionNotificationResponse != null) { + // Promote the pending action notification response to its specialized type. if (contactId != null) { - ContactPendingActionNotificationResponse contactPendingResponse = + pendingActionNotificationResponse = ContactPendingActionNotificationResponse.create( pendingActionNotificationResponse.nameOrId.value, pendingActionNotificationResponse.getActionResult(), pendingActionNotificationResponse.getTrid(), pendingActionNotificationResponse.processedDate); - pendingActionNotificationResponse = contactPendingResponse; - contactPendingActionNotificationResponses = ImmutableList.of(contactPendingResponse); } else if (fullyQualifiedDomainName != null) { - DomainPendingActionNotificationResponse domainPendingResponse = + pendingActionNotificationResponse = DomainPendingActionNotificationResponse.create( pendingActionNotificationResponse.nameOrId.value, pendingActionNotificationResponse.getActionResult(), pendingActionNotificationResponse.getTrid(), pendingActionNotificationResponse.processedDate); - pendingActionNotificationResponse = domainPendingResponse; - domainPendingActionNotificationResponses = ImmutableList.of(domainPendingResponse); } else if (hostId != null) { - HostPendingActionNotificationResponse hostPendingActionNotificationResponse = + pendingActionNotificationResponse = HostPendingActionNotificationResponse.create( pendingActionNotificationResponse.nameOrId.value, pendingActionNotificationResponse.getActionResult(), pendingActionNotificationResponse.getTrid(), pendingActionNotificationResponse.processedDate); - pendingActionNotificationResponse = hostPendingActionNotificationResponse; - hostPendingActionNotificationResponses = - ImmutableList.of(hostPendingActionNotificationResponse); } } if (transferResponse != null) { @@ -467,7 +506,6 @@ public abstract class PollMessage extends ImmutableObject .setPendingTransferExpirationTime( transferResponse.getPendingTransferExpirationTime()) .build(); - contactTransferResponses = ImmutableList.of((ContactTransferResponse) transferResponse); } else if (fullyQualifiedDomainName != null) { transferResponse = new DomainTransferResponse.Builder() @@ -480,7 +518,6 @@ public abstract class PollMessage extends ImmutableObject transferResponse.getPendingTransferExpirationTime()) .setExtendedRegistrationExpirationTime(extendedRegistrationExpirationTime) .build(); - domainTransferResponses = ImmutableList.of((DomainTransferResponse) transferResponse); } } } @@ -497,66 +534,52 @@ public abstract class PollMessage extends ImmutableObject public Builder setResponseData(ImmutableList responseData) { OneTime instance = getInstance(); - instance.contactPendingActionNotificationResponses = - forceEmptyToNull( - responseData.stream() - .filter(ContactPendingActionNotificationResponse.class::isInstance) - .map(ContactPendingActionNotificationResponse.class::cast) - .collect(toImmutableList())); + // Note: In its current form, the code will basically just ignore everything but the first + // PendingActionNotificationResponse and TransferResponse in responseData, and will override + // any identifier fields (e.g. contactId, fullyQualifiedDomainName) obtained from the + // PendingActionNotificationResponse if a TransferResponse is found with different values + // for those fields. It is not clear what the constraints should be on this data or + // whether we should enforce them here, though historically we have not, so the current + // implementation doesn't. - instance.contactTransferResponses = - forceEmptyToNull( - responseData.stream() - .filter(ContactTransferResponse.class::isInstance) - .map(ContactTransferResponse.class::cast) - .collect(toImmutableList())); + // Extract the first PendingActionNotificationResponse element from the list if there is + // one. + instance.pendingActionNotificationResponse = + responseData.stream() + .filter(PendingActionNotificationResponse.class::isInstance) + .map(PendingActionNotificationResponse.class::cast) + .findFirst() + .orElse(null); - instance.domainPendingActionNotificationResponses = - forceEmptyToNull( - responseData.stream() - .filter(DomainPendingActionNotificationResponse.class::isInstance) - .map(DomainPendingActionNotificationResponse.class::cast) - .collect(toImmutableList())); - instance.domainTransferResponses = - forceEmptyToNull( - responseData.stream() - .filter(DomainTransferResponse.class::isInstance) - .map(DomainTransferResponse.class::cast) - .collect(toImmutableList())); - - instance.hostPendingActionNotificationResponses = - forceEmptyToNull( - responseData.stream() - .filter(HostPendingActionNotificationResponse.class::isInstance) - .map(HostPendingActionNotificationResponse.class::cast) - .collect(toImmutableList())); - - // Set the generic pending-action field as appropriate - if (instance.contactPendingActionNotificationResponses != null) { - instance.pendingActionNotificationResponse = - instance.contactPendingActionNotificationResponses.get(0); - instance.contactId = - instance.contactPendingActionNotificationResponses.get(0).nameOrId.value; - } else if (instance.domainPendingActionNotificationResponses != null) { - instance.pendingActionNotificationResponse = - instance.domainPendingActionNotificationResponses.get(0); + // Set identifier fields based on the type of the notification response. + if (instance.pendingActionNotificationResponse + instanceof ContactPendingActionNotificationResponse) { + instance.contactId = instance.pendingActionNotificationResponse.nameOrId.value; + } else if (instance.pendingActionNotificationResponse + instanceof DomainPendingActionNotificationResponse) { instance.fullyQualifiedDomainName = - instance.domainPendingActionNotificationResponses.get(0).nameOrId.value; - } else if (instance.hostPendingActionNotificationResponses != null) { - instance.pendingActionNotificationResponse = - instance.hostPendingActionNotificationResponses.get(0); - instance.hostId = instance.hostPendingActionNotificationResponses.get(0).nameOrId.value; + instance.pendingActionNotificationResponse.nameOrId.value; + } else if (instance.pendingActionNotificationResponse + instanceof HostPendingActionNotificationResponse) { + instance.hostId = instance.pendingActionNotificationResponse.nameOrId.value; } - // Set the generic transfer response field as appropriate - if (instance.contactTransferResponses != null) { - instance.contactId = getInstance().contactTransferResponses.get(0).getContactId(); - instance.transferResponse = getInstance().contactTransferResponses.get(0); - } else if (instance.domainTransferResponses != null) { - instance.fullyQualifiedDomainName = - instance.domainTransferResponses.get(0).getFullyQualifiedDomainName(); - instance.transferResponse = getInstance().domainTransferResponses.get(0); + + // Extract the first TransferResponse from the list. + instance.transferResponse = + responseData.stream() + .filter(TransferResponse.class::isInstance) + .map(TransferResponse.class::cast) + .findFirst() + .orElse(null); + + // Set the identifier according to the TransferResponse type. + if (instance.transferResponse instanceof ContactTransferResponse) { + instance.contactId = ((ContactTransferResponse) instance.transferResponse).getContactId(); + } else if (instance.transferResponse instanceof DomainTransferResponse) { + DomainTransferResponse response = (DomainTransferResponse) instance.transferResponse; + instance.fullyQualifiedDomainName = response.getFullyQualifiedDomainName(); instance.extendedRegistrationExpirationTime = - instance.domainTransferResponses.get(0).getExtendedRegistrationExpirationTime(); + response.getExtendedRegistrationExpirationTime(); } return this; } diff --git a/core/src/main/java/google/registry/model/poll/PollMessageExternalKeyConverter.java b/core/src/main/java/google/registry/model/poll/PollMessageExternalKeyConverter.java index 2fe91275a..78b0aa307 100644 --- a/core/src/main/java/google/registry/model/poll/PollMessageExternalKeyConverter.java +++ b/core/src/main/java/google/registry/model/poll/PollMessageExternalKeyConverter.java @@ -14,19 +14,12 @@ package google.registry.model.poll; -import static com.google.common.collect.ImmutableMap.toImmutableMap; import com.google.common.base.Splitter; -import com.google.common.collect.ImmutableMap; import com.googlecode.objectify.Key; -import google.registry.model.EppResource; -import google.registry.model.contact.ContactResource; -import google.registry.model.domain.DomainBase; -import google.registry.model.host.HostResource; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; import java.util.List; -import java.util.Map; /** * A converter between external key strings for {@link PollMessage}s (i.e. what registrars use to @@ -50,28 +43,13 @@ public class PollMessageExternalKeyConverter { /** An exception thrown when an external key cannot be parsed. */ public static class PollMessageExternalKeyParseException extends RuntimeException {} - /** Maps that detail the correspondence between EppResource classes and external IDs. */ - private static final ImmutableMap> ID_TO_CLASS_MAP = - ImmutableMap.of( - 1L, DomainBase.class, - 2L, ContactResource.class, - 3L, HostResource.class); - - private static final ImmutableMap KEY_KIND_TO_ID_MAP = - ID_TO_CLASS_MAP.entrySet().stream() - .collect(toImmutableMap(entry -> entry.getValue().getSimpleName(), Map.Entry::getKey)); - /** Returns an external poll message ID for the given poll message. */ public static String makePollMessageExternalId(PollMessage pollMessage) { - @SuppressWarnings("unchecked") - Key ancestorResource = - (Key) (Key) pollMessage.getParentKey().getParent(); - long externalKeyClassId = KEY_KIND_TO_ID_MAP.get(ancestorResource.getKind()); return String.format( "%d-%s-%d-%d-%d", - externalKeyClassId, - ancestorResource.getName(), - pollMessage.getParentKey().getId(), + pollMessage.getType().getId(), + pollMessage.getResourceName(), + pollMessage.getHistoryRevisionId(), pollMessage.getId(), pollMessage.getEventTime().getYear()); } @@ -92,10 +70,10 @@ public class PollMessageExternalKeyConverter { throw new PollMessageExternalKeyParseException(); } try { - Class resourceClazz = ID_TO_CLASS_MAP.get(Long.parseLong(idComponents.get(0))); - if (resourceClazz == null) { - throw new PollMessageExternalKeyParseException(); - } + Class resourceClazz = + PollMessage.Type.fromId(Long.parseLong(idComponents.get(0))) + .orElseThrow(() -> new PollMessageExternalKeyParseException()) + .getResourceClass(); return VKey.from( Key.create( Key.create( @@ -115,4 +93,3 @@ public class PollMessageExternalKeyConverter { private PollMessageExternalKeyConverter() {} } - diff --git a/core/src/main/java/google/registry/model/transfer/DomainTransferData.java b/core/src/main/java/google/registry/model/transfer/DomainTransferData.java index 6c6161267..6276d77de 100644 --- a/core/src/main/java/google/registry/model/transfer/DomainTransferData.java +++ b/core/src/main/java/google/registry/model/transfer/DomainTransferData.java @@ -14,25 +14,20 @@ package google.registry.model.transfer; -import static google.registry.util.CollectionUtils.forceEmptyToNull; import static google.registry.util.CollectionUtils.isNullOrEmpty; -import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; -import com.googlecode.objectify.annotation.AlsoLoad; import com.googlecode.objectify.annotation.Embed; import com.googlecode.objectify.annotation.Ignore; import com.googlecode.objectify.annotation.IgnoreSave; -import com.googlecode.objectify.annotation.OnLoad; import com.googlecode.objectify.annotation.Unindex; import com.googlecode.objectify.condition.IfNull; import google.registry.model.billing.BillingEvent; -import google.registry.model.domain.DomainBase; import google.registry.model.domain.Period; import google.registry.model.domain.Period.Unit; import google.registry.model.poll.PollMessage; import google.registry.persistence.VKey; +import google.registry.util.NullIgnoringCollectionBuilder; import java.util.Set; import javax.annotation.Nullable; import javax.persistence.AttributeOverride; @@ -40,7 +35,6 @@ import javax.persistence.AttributeOverrides; import javax.persistence.Column; import javax.persistence.Embeddable; import javax.persistence.Embedded; -import javax.persistence.PostLoad; import org.joda.time.DateTime; /** Transfer data for domain. */ @@ -85,10 +79,6 @@ public class DomainTransferData extends TransferData @Column(name = "transfer_billing_cancellation_id") public VKey billingCancellationId; - @Ignore - @Column(name = "transfer_billing_cancellation_history_id") - Long billingCancellationHistoryId; - /** * The regular one-time billing event that will be charged for a server-approved transfer. * @@ -99,10 +89,6 @@ public class DomainTransferData extends TransferData @Column(name = "transfer_billing_event_id") VKey serverApproveBillingEvent; - @Ignore - @Column(name = "transfer_billing_event_history_id") - Long serverApproveBillingEventHistoryId; - /** * The autorenew billing event that should be associated with this resource after the transfer. * @@ -113,10 +99,6 @@ public class DomainTransferData extends TransferData @Column(name = "transfer_billing_recurrence_id") VKey serverApproveAutorenewEvent; - @Ignore - @Column(name = "transfer_billing_recurrence_history_id") - Long serverApproveAutorenewEventHistoryId; - /** * The autorenew poll message that should be associated with this resource after the transfer. * @@ -127,6 +109,10 @@ public class DomainTransferData extends TransferData @Column(name = "transfer_autorenew_poll_message_id") VKey serverApproveAutorenewPollMessage; + /** + * Autorenew history, which we need to preserve because it's often used in contexts where we + * haven't loaded the autorenew object. + */ @Ignore @Column(name = "transfer_autorenew_poll_message_history_id") Long serverApproveAutorenewPollMessageHistoryId; @@ -136,36 +122,6 @@ public class DomainTransferData extends TransferData return super.copyConstantFieldsToBuilder().setTransferPeriod(this.transferPeriod); } - /** - * Restores the set of ofy keys after loading from SQL using the specified {@code rootKey}. - * - *

This is for use by DomainBase/DomainHistory PostLoad methods ONLY. - */ - public void restoreOfyKeys(Key rootKey) { - serverApproveBillingEvent = - DomainBase.restoreOfyFrom( - rootKey, serverApproveBillingEvent, serverApproveBillingEventHistoryId); - serverApproveAutorenewEvent = - DomainBase.restoreOfyFrom( - rootKey, serverApproveAutorenewEvent, serverApproveAutorenewEventHistoryId); - serverApproveAutorenewPollMessage = - DomainBase.restoreOfyFrom( - rootKey, serverApproveAutorenewPollMessage, serverApproveAutorenewPollMessageHistoryId); - billingCancellationId = - DomainBase.restoreOfyFrom(rootKey, billingCancellationId, billingCancellationHistoryId); - - // Reconstruct server approve entities. We currently have to call postLoad() a _second_ time - // if any of the billing objects have been reconstituted, as they are part of that set. - // TODO(b/183010623): Normalize the approaches to VKey reconstitution for the TransferData - // hierarchy (the logic currently lives either in PostLoad or here, depending on the key). - if (billingCancellationId != null - || serverApproveBillingEvent != null - || serverApproveAutorenewEvent != null) { - serverApproveEntities = null; - postLoad(); - } - } - /** * Fix the VKey "kind" for the PollMessage keys. * @@ -176,30 +132,6 @@ public class DomainTransferData extends TransferData PollMessage.Autorenew.convertVKey(serverApproveAutorenewPollMessage); } - @SuppressWarnings("unused") // For Hibernate. - private void loadServerApproveBillingEventHistoryId( - @AlsoLoad("serverApproveBillingEvent") VKey val) { - serverApproveBillingEventHistoryId = DomainBase.getHistoryId(val); - } - - @SuppressWarnings("unused") // For Hibernate. - private void loadServerApproveAutorenewEventHistoryId( - @AlsoLoad("serverApproveAutorenewEvent") VKey val) { - serverApproveAutorenewEventHistoryId = DomainBase.getHistoryId(val); - } - - @SuppressWarnings("unused") // For Hibernate. - private void loadServerApproveAutorenewPollMessageHistoryId( - @AlsoLoad("serverApproveAutorenewPollMessage") VKey val) { - serverApproveAutorenewPollMessageHistoryId = DomainBase.getHistoryId(val); - } - - @SuppressWarnings("unused") // For Hibernate. - private void billingCancellationHistoryId( - @AlsoLoad("billingCancellationHistoryId") VKey val) { - billingCancellationHistoryId = DomainBase.getHistoryId(val); - } - public Period getTransferPeriod() { return transferPeriod; } @@ -214,64 +146,32 @@ public class DomainTransferData extends TransferData return serverApproveBillingEvent; } - @VisibleForTesting - @Nullable - public Long getServerApproveBillingEventHistoryId() { - return serverApproveBillingEventHistoryId; - } - @Nullable public VKey getServerApproveAutorenewEvent() { return serverApproveAutorenewEvent; } - @VisibleForTesting - @Nullable - public Long getServerApproveAutorenewEventHistoryId() { - return serverApproveAutorenewEventHistoryId; - } - @Nullable public VKey getServerApproveAutorenewPollMessage() { return serverApproveAutorenewPollMessage; } - @VisibleForTesting @Nullable public Long getServerApproveAutorenewPollMessageHistoryId() { return serverApproveAutorenewPollMessageHistoryId; } - @OnLoad @Override - void onLoad( - @AlsoLoad("serverApproveEntities") - Set> serverApproveEntities) { - super.onLoad(serverApproveEntities); - mapBillingCancellationEntityToField(serverApproveEntities, this); - } - - @PostLoad - @Override - void postLoad() { - super.postLoad(); - // The superclass's serverApproveEntities should include the billing events if present - ImmutableSet.Builder> serverApproveEntitiesBuilder = + public ImmutableSet> getServerApproveEntities() { + ImmutableSet.Builder> builder = new ImmutableSet.Builder<>(); - if (serverApproveEntities != null) { - serverApproveEntitiesBuilder.addAll(serverApproveEntities); - } - if (serverApproveBillingEvent != null) { - serverApproveEntitiesBuilder.add(serverApproveBillingEvent); - } - if (serverApproveAutorenewEvent != null) { - serverApproveEntitiesBuilder.add(serverApproveAutorenewEvent); - } - if (billingCancellationId != null) { - serverApproveEntitiesBuilder.add(billingCancellationId); - } - serverApproveEntities = forceEmptyToNull(serverApproveEntitiesBuilder.build()); - hashCode = null; // reset the hash code since we may have changed the entities + builder.addAll(super.getServerApproveEntities()); + return NullIgnoringCollectionBuilder.create(builder) + .add(serverApproveBillingEvent) + .add(serverApproveAutorenewEvent) + .add(billingCancellationId) + .getBuilder() + .build(); } @Override @@ -291,7 +191,6 @@ public class DomainTransferData extends TransferData DomainTransferData domainTransferData) { if (isNullOrEmpty(serverApproveEntities)) { domainTransferData.billingCancellationId = null; - domainTransferData.billingCancellationHistoryId = null; } else { domainTransferData.billingCancellationId = (VKey) @@ -299,10 +198,6 @@ public class DomainTransferData extends TransferData .filter(k -> k.getKind().equals(BillingEvent.Cancellation.class)) .findFirst() .orElse(null); - domainTransferData.billingCancellationHistoryId = - domainTransferData.billingCancellationId != null - ? DomainBase.getHistoryId(domainTransferData.billingCancellationId) - : null; } } @@ -315,12 +210,6 @@ public class DomainTransferData extends TransferData super(instance); } - @Override - public DomainTransferData build() { - mapBillingCancellationEntityToField(getInstance().serverApproveEntities, getInstance()); - return super.build(); - } - public Builder setTransferPeriod(Period transferPeriod) { getInstance().transferPeriod = transferPeriod; return this; @@ -335,24 +224,28 @@ public class DomainTransferData extends TransferData public Builder setServerApproveBillingEvent( VKey serverApproveBillingEvent) { getInstance().serverApproveBillingEvent = serverApproveBillingEvent; - getInstance().serverApproveBillingEventHistoryId = - DomainBase.getHistoryId(serverApproveBillingEvent); return this; } public Builder setServerApproveAutorenewEvent( VKey serverApproveAutorenewEvent) { getInstance().serverApproveAutorenewEvent = serverApproveAutorenewEvent; - getInstance().serverApproveAutorenewEventHistoryId = - DomainBase.getHistoryId(serverApproveAutorenewEvent); return this; } public Builder setServerApproveAutorenewPollMessage( VKey serverApproveAutorenewPollMessage) { getInstance().serverApproveAutorenewPollMessage = serverApproveAutorenewPollMessage; - getInstance().serverApproveAutorenewPollMessageHistoryId = - DomainBase.getHistoryId(serverApproveAutorenewPollMessage); + return this; + } + + @Override + public Builder setServerApproveEntities( + String repoId, + Long historyId, + ImmutableSet> serverApproveEntities) { + super.setServerApproveEntities(repoId, historyId, serverApproveEntities); + mapBillingCancellationEntityToField(serverApproveEntities, getInstance()); return this; } } diff --git a/core/src/main/java/google/registry/model/transfer/TransferData.java b/core/src/main/java/google/registry/model/transfer/TransferData.java index 7e7edd981..4918e3c54 100644 --- a/core/src/main/java/google/registry/model/transfer/TransferData.java +++ b/core/src/main/java/google/registry/model/transfer/TransferData.java @@ -14,26 +14,20 @@ package google.registry.model.transfer; +import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.util.CollectionUtils.isNullOrEmpty; import static google.registry.util.CollectionUtils.nullToEmpty; -import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; -import com.googlecode.objectify.annotation.AlsoLoad; import com.googlecode.objectify.annotation.Ignore; -import com.googlecode.objectify.annotation.IgnoreSave; -import com.googlecode.objectify.condition.IfNull; import google.registry.model.Buildable; import google.registry.model.EppResource; -import google.registry.model.contact.ContactResource; -import google.registry.model.domain.DomainBase; import google.registry.model.eppcommon.Trid; import google.registry.model.poll.PollMessage; -import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; +import google.registry.util.NullIgnoringCollectionBuilder; import google.registry.util.TypeUtils.TypeInstantiator; import java.util.Set; import javax.annotation.Nullable; @@ -42,8 +36,6 @@ import javax.persistence.AttributeOverrides; import javax.persistence.Column; import javax.persistence.Embedded; import javax.persistence.MappedSuperclass; -import javax.persistence.PostLoad; -import javax.persistence.Transient; /** * Common transfer data for {@link EppResource} types. Only applies to domains and contacts; hosts @@ -66,20 +58,6 @@ public abstract class TransferData< }) Trid transferRequestTrid; - /** - * The billing event and poll messages associated with a server-approved transfer. - * - *

This field should be null if there is not currently a pending transfer or if the object - * being transferred is not a domain. If there is a pending transfer for a domain there should be - * a number of poll messages and billing events for both the gaining and losing registrars. If the - * pending transfer is explicitly approved, rejected or cancelled, the referenced entities should - * be deleted. - */ - @Transient - @DoNotCompare - @IgnoreSave(IfNull.class) - Set> serverApproveEntities; - @Ignore @Column(name = "transfer_repo_id") String repoId; @@ -89,11 +67,7 @@ public abstract class TransferData< Long historyEntryId; // The pollMessageId1 and pollMessageId2 are used to store the IDs for gaining and losing poll - // messages in Cloud SQL, and they are added to replace the VKeys in serverApproveEntities. - // Although we can distinguish which is which when we construct the TransferData instance from - // the transfer request flow, when the instance is loaded from Datastore, we cannot make this - // distinction because they are just VKeys. Also, the only way we use serverApproveEntities is to - // just delete all the entities referenced by the VKeys, so we don't need to make the distinction. + // messages in Cloud SQL. // // In addition, there may be a third poll message for the autorenew poll message on domain // transfer if applicable. @@ -111,13 +85,23 @@ public abstract class TransferData< public abstract boolean isEmpty(); + public Long getHistoryEntryId() { + return historyEntryId; + } + @Nullable public Trid getTransferRequestTrid() { return transferRequestTrid; } public ImmutableSet> getServerApproveEntities() { - return nullToEmptyImmutableCopy(serverApproveEntities); + return NullIgnoringCollectionBuilder.create( + new ImmutableSet.Builder>()) + .add(pollMessageId1 != null ? VKey.createSql(PollMessage.class, pollMessageId1) : null) + .add(pollMessageId2 != null ? VKey.createSql(PollMessage.class, pollMessageId2) : null) + .add(pollMessageId3 != null ? VKey.createSql(PollMessage.class, pollMessageId3) : null) + .getBuilder() + .build(); } @Override @@ -147,67 +131,16 @@ public abstract class TransferData< return newBuilder; } - void onLoad( - @AlsoLoad("serverApproveEntities") - Set> serverApproveEntities) { - mapServerApproveEntitiesToFields(serverApproveEntities, this); - } - - @PostLoad - void postLoad() { - mapFieldsToServerApproveEntities(); - } - - /** - * Reconstructs serverApproveEntities set from the individual fields, e.g. repoId, historyEntryId, - * pollMessageId1. - */ - void mapFieldsToServerApproveEntities() { - if (repoId == null) { - return; - } - Key eppKey; - if (getClass().equals(DomainTransferData.class)) { - eppKey = Key.create(DomainBase.class, repoId); - } else { - eppKey = Key.create(ContactResource.class, repoId); - } - Key historyEntryKey = Key.create(eppKey, HistoryEntry.class, historyEntryId); - ImmutableSet.Builder> entityKeysBuilder = - new ImmutableSet.Builder<>(); - if (pollMessageId1 != null) { - Key ofyKey = Key.create(historyEntryKey, PollMessage.class, pollMessageId1); - entityKeysBuilder.add(PollMessage.createVKey(ofyKey)); - } - if (pollMessageId2 != null) { - Key ofyKey = Key.create(historyEntryKey, PollMessage.class, pollMessageId2); - entityKeysBuilder.add(PollMessage.createVKey(ofyKey)); - } - if (pollMessageId3 != null) { - Key ofyKey = Key.create(historyEntryKey, PollMessage.class, pollMessageId3); - entityKeysBuilder.add(PollMessage.createVKey(ofyKey)); - } - serverApproveEntities = entityKeysBuilder.build(); - } - /** Maps serverApproveEntities set to the individual fields. */ static void mapServerApproveEntitiesToFields( Set> serverApproveEntities, TransferData transferData) { if (isNullOrEmpty(serverApproveEntities)) { - transferData.historyEntryId = null; - transferData.repoId = null; transferData.pollMessageId1 = null; transferData.pollMessageId2 = null; transferData.pollMessageId3 = null; return; } - // Each element in serverApproveEntities should have the exact same Key as its - // parent. So, we can use any to set historyEntryId and repoId. - Key key = serverApproveEntities.iterator().next().getOfyKey(); - transferData.historyEntryId = key.getParent().getId(); - transferData.repoId = key.getParent().getParent().getName(); - ImmutableList sortedPollMessageIds = getSortedPollMessageIds(serverApproveEntities); if (sortedPollMessageIds.size() >= 1) { transferData.pollMessageId1 = sortedPollMessageIds.get(0); @@ -250,14 +183,21 @@ public abstract class TransferData< } public B setServerApproveEntities( + String repoId, + Long historyId, ImmutableSet> serverApproveEntities) { - getInstance().serverApproveEntities = serverApproveEntities; + getInstance().repoId = repoId; + getInstance().historyEntryId = historyId; + mapServerApproveEntitiesToFields(serverApproveEntities, getInstance()); return thisCastToDerived(); } @Override public T build() { - mapServerApproveEntitiesToFields(getInstance().serverApproveEntities, getInstance()); + if (getInstance().pollMessageId1 != null) { + checkState(getInstance().repoId != null, "Repo id undefined"); + checkState(getInstance().historyEntryId != null, "History entry undefined"); + } return super.build(); } } diff --git a/core/src/main/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactory.java b/core/src/main/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactory.java index 5e8030bf1..8ca7627ac 100644 --- a/core/src/main/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactory.java +++ b/core/src/main/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactory.java @@ -103,7 +103,7 @@ public class EppHistoryVKeyTranslatorFactory @Nullable @Override public Key saveValue(@Nullable EppHistoryVKey pojoValue) { - return pojoValue == null ? null : pojoValue.createOfyKey().getRaw(); + throw new UnsupportedOperationException("saveValue for EppHistory keys removed."); } }; } diff --git a/core/src/main/java/google/registry/persistence/BillingVKey.java b/core/src/main/java/google/registry/persistence/BillingVKey.java index 5d7adcd3a..cc0b8121b 100644 --- a/core/src/main/java/google/registry/persistence/BillingVKey.java +++ b/core/src/main/java/google/registry/persistence/BillingVKey.java @@ -69,11 +69,6 @@ public abstract class BillingVKey extends EppHistoryVKey { super(repoId, historyRevisionId, billingEventId); } - @Override - public Key createOfyKey() { - return Key.create(createHistoryEntryKey(), BillingEvent.OneTime.class, billingId); - } - /** Creates a {@link BillingEventVKey} instance from the given {@link Key} instance. */ public static BillingEventVKey create(@Nullable Key ofyKey) { if (ofyKey == null) { @@ -87,7 +82,7 @@ public abstract class BillingVKey extends EppHistoryVKey { /** Creates a {@link BillingEventVKey} instance from the given {@link VKey} instance. */ public static BillingEventVKey create(@Nullable VKey vKey) { - return vKey == null ? null : create(vKey.getOfyKey()); + return vKey == null ? null : new BillingEventVKey(null, 0, (Long) vKey.getSqlKey()); } } @@ -111,11 +106,6 @@ public abstract class BillingVKey extends EppHistoryVKey { super(repoId, historyRevisionId, billingEventId); } - @Override - public Key createOfyKey() { - return Key.create(createHistoryEntryKey(), BillingEvent.Recurring.class, billingId); - } - /** Creates a {@link BillingRecurrenceVKey} instance from the given {@link Key} instance. */ public static BillingRecurrenceVKey create(@Nullable Key ofyKey) { if (ofyKey == null) { @@ -129,7 +119,7 @@ public abstract class BillingVKey extends EppHistoryVKey { /** Creates a {@link BillingRecurrenceVKey} instance from the given {@link VKey} instance. */ public static BillingRecurrenceVKey create(@Nullable VKey vKey) { - return vKey == null ? null : create(vKey.getOfyKey()); + return vKey == null ? null : new BillingRecurrenceVKey(null, 0, (Long) vKey.getSqlKey()); } } } diff --git a/core/src/main/java/google/registry/persistence/DomainHistoryVKey.java b/core/src/main/java/google/registry/persistence/DomainHistoryVKey.java index 764a8f0b3..00dd17bfa 100644 --- a/core/src/main/java/google/registry/persistence/DomainHistoryVKey.java +++ b/core/src/main/java/google/registry/persistence/DomainHistoryVKey.java @@ -40,11 +40,6 @@ public class DomainHistoryVKey extends EppHistoryVKey return new DomainHistoryId(repoId, historyRevisionId); } - @Override - public Key createOfyKey() { - return Key.create(Key.create(DomainBase.class, repoId), HistoryEntry.class, historyRevisionId); - } - /** Creates {@link DomainHistoryVKey} from the given {@link Key} instance. */ public static DomainHistoryVKey create(Key ofyKey) { checkArgumentNotNull(ofyKey, "ofyKey must be specified"); diff --git a/core/src/main/java/google/registry/persistence/EppHistoryVKey.java b/core/src/main/java/google/registry/persistence/EppHistoryVKey.java index c71cf8b53..3b1df7ed6 100644 --- a/core/src/main/java/google/registry/persistence/EppHistoryVKey.java +++ b/core/src/main/java/google/registry/persistence/EppHistoryVKey.java @@ -99,10 +99,8 @@ public abstract class EppHistoryVKey extends Immutable @Override public VKey createVKey() { Class vKeyType = new TypeInstantiator(getClass()) {}.getExactType(); - return VKey.create(vKeyType, createSqlKey(), createOfyKey()); + return VKey.createSql(vKeyType, createSqlKey()); } public abstract Serializable createSqlKey(); - - public abstract Key createOfyKey(); } diff --git a/core/src/main/java/google/registry/persistence/VKey.java b/core/src/main/java/google/registry/persistence/VKey.java index 580240997..fccf6a93b 100644 --- a/core/src/main/java/google/registry/persistence/VKey.java +++ b/core/src/main/java/google/registry/persistence/VKey.java @@ -16,6 +16,7 @@ package google.registry.persistence; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; +import static google.registry.model.ImmutableObject.Insignificant; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.google.common.base.Splitter; @@ -52,7 +53,8 @@ public class VKey extends ImmutableObject implements Serializable { // The SQL key for the referenced entity. Serializable sqlKey; - // The objectify key for the referenced entity. + // The objectify key for the referenced entity. Marked Insignificant to exclude it from + // hashing/equality. @Insignificant Key ofyKey; Class kind; diff --git a/core/src/main/java/google/registry/tools/EnqueuePollMessageCommand.java b/core/src/main/java/google/registry/tools/EnqueuePollMessageCommand.java index 907cdd3c6..9c7f02404 100644 --- a/core/src/main/java/google/registry/tools/EnqueuePollMessageCommand.java +++ b/core/src/main/java/google/registry/tools/EnqueuePollMessageCommand.java @@ -118,7 +118,7 @@ class EnqueuePollMessageCommand extends MutatingCommand { null, new PollMessage.OneTime.Builder() .setRegistrarId(registrarId) - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .setEventTime(tm().getTransactionTime()) .setMsg(message) .build()); diff --git a/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java b/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java index 409f8e240..91c731f4d 100644 --- a/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java +++ b/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java @@ -201,7 +201,7 @@ class UnrenewDomainCommand extends ConfirmingCommand implements CommandWithRemot String.format( "Domain %s was unrenewed by %d years; now expires at %s.", domainName, period, newExpirationTime)) - .setParent(domainHistory) + .setHistoryEntry(domainHistory) .setEventTime(now) .build(); // Create a new autorenew billing event and poll message starting at the new expiration time. @@ -213,7 +213,7 @@ class UnrenewDomainCommand extends ConfirmingCommand implements CommandWithRemot PollMessage.Autorenew newAutorenewPollMessage = newAutorenewPollMessage(domain) .setEventTime(newExpirationTime) - .setParent(domainHistory) + .setHistoryEntry(domainHistory) .build(); // End the old autorenew billing event and poll message now. updateAutorenewRecurrenceEndTime(domain, now); @@ -224,7 +224,9 @@ class UnrenewDomainCommand extends ConfirmingCommand implements CommandWithRemot .setLastEppUpdateTime(now) .setLastEppUpdateRegistrarId(domain.getCurrentSponsorRegistrarId()) .setAutorenewBillingEvent(newAutorenewEvent.createVKey()) - .setAutorenewPollMessage(newAutorenewPollMessage.createVKey()) + .setAutorenewPollMessage( + newAutorenewPollMessage.createVKey(), + newAutorenewPollMessage.getHistoryRevisionId()) .build(); // In order to do it'll need to write out a new HistoryEntry (likely of type SYNTHETIC), a new // autorenew billing event and poll message, and a new one time poll message at the present time diff --git a/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java b/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java index b0ca800a3..24cacf862 100644 --- a/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java +++ b/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java @@ -191,7 +191,8 @@ class DeleteExpiredDomainsActionTest { .asBuilder() .setAutorenewEndTime(Optional.of(clock.nowUtc().minusDays(10))) .setAutorenewBillingEvent(autorenewBillingEvent.createVKey()) - .setAutorenewPollMessage(autorenewPollMessage.createVKey()) + .setAutorenewPollMessage( + autorenewPollMessage.createVKey(), autorenewPollMessage.getHistoryRevisionId()) .build()); return pendingExpirationDomain; @@ -216,6 +217,6 @@ class DeleteExpiredDomainsActionTest { .setRegistrarId("TheRegistrar") .setEventTime(clock.nowUtc().plusYears(1)) .setAutorenewEndTime(END_OF_TIME) - .setParent(createHistoryEntry); + .setHistoryEntry(createHistoryEntry); } } diff --git a/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java b/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java index 4f7f7932e..e3ccfdfdf 100644 --- a/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java +++ b/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java @@ -316,7 +316,7 @@ class DeleteProberDataActionTest { PollMessage.OneTime pollMessage = persistSimpleResource( new PollMessage.OneTime.Builder() - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .setEventTime(DELETION_TIME) .setRegistrarId("TheRegistrar") .setMsg("Domain registered") diff --git a/core/src/test/java/google/registry/beam/rde/RdePipelineTest.java b/core/src/test/java/google/registry/beam/rde/RdePipelineTest.java index 60f134a0c..0b5752941 100644 --- a/core/src/test/java/google/registry/beam/rde/RdePipelineTest.java +++ b/core/src/test/java/google/registry/beam/rde/RdePipelineTest.java @@ -311,6 +311,10 @@ public class RdePipelineTest { persistDomainHistory( helloDomain .asBuilder() + .removeContacts( + helloDomain.getContacts().stream() + .filter(dc -> dc.getType() == DesignatedContact.Type.ADMIN) + .collect(toImmutableSet())) .addContacts( ImmutableSet.of( DesignatedContact.create(DesignatedContact.Type.ADMIN, contact3.createVKey()))) diff --git a/core/src/test/java/google/registry/flows/contact/ContactTransferRequestFlowTest.java b/core/src/test/java/google/registry/flows/contact/ContactTransferRequestFlowTest.java index 474509529..5c2c8ce99 100644 --- a/core/src/test/java/google/registry/flows/contact/ContactTransferRequestFlowTest.java +++ b/core/src/test/java/google/registry/flows/contact/ContactTransferRequestFlowTest.java @@ -107,6 +107,8 @@ class ContactTransferRequestFlowTest // Make the server-approve entities field a no-op comparison; it's easier to // do this comparison separately below. .setServerApproveEntities( + contact.getRepoId(), + contact.getTransferData().getHistoryEntryId(), forceEmptyToNull(contact.getTransferData().getServerApproveEntities())) .build()); assertNoBillingEvents(); diff --git a/core/src/test/java/google/registry/flows/custom/TestDomainCreateFlowCustomLogic.java b/core/src/test/java/google/registry/flows/custom/TestDomainCreateFlowCustomLogic.java index b5b8ba07d..bbd184b10 100644 --- a/core/src/test/java/google/registry/flows/custom/TestDomainCreateFlowCustomLogic.java +++ b/core/src/test/java/google/registry/flows/custom/TestDomainCreateFlowCustomLogic.java @@ -34,7 +34,7 @@ public class TestDomainCreateFlowCustomLogic extends DomainCreateFlowCustomLogic if (parameters.newDomain().getDomainName().startsWith("custom-logic-test")) { PollMessage extraPollMessage = new PollMessage.OneTime.Builder() - .setParent(parameters.historyEntry()) + .setHistoryEntry(parameters.historyEntry()) .setEventTime(tm().getTransactionTime()) .setRegistrarId(getSessionMetadata().get().getRegistrarId()) .setMsg("Custom logic was triggered") diff --git a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java index 9200b5ab0..baef214ef 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java @@ -348,7 +348,7 @@ class DomainCreateFlowTest extends ResourceFlowTestCase { .setRegistrarId(getRegistrarIdForFlow()) .setEventTime(clock.nowUtc().minusDays(1)) .setMsg("Some poll message.") - .setParent(createHistoryEntryForEppResource(domain)) + .setHistoryEntry(createHistoryEntryForEppResource(domain)) .build()); } @@ -83,7 +83,7 @@ class PollAckFlowTest extends FlowTestCase { .setAutorenewEndTime(endTime) .setMsg("Domain was auto-renewed.") .setTargetId("example.com") - .setParent(createHistoryEntryForEppResource(domain)) + .setHistoryEntry(createHistoryEntryForEppResource(domain)) .build()); } @@ -102,7 +102,7 @@ class PollAckFlowTest extends FlowTestCase { .setRegistrarId(getRegistrarIdForFlow()) .setEventTime(clock.nowUtc().minusDays(1)) .setMsg("Some poll message.") - .setParent(createHistoryEntryForEppResource(contact)) + .setHistoryEntry(createHistoryEntryForEppResource(contact)) .build()); assertTransactionalFlow(true); runFlowAssertResponse(loadFile("poll_ack_response_empty.xml")); @@ -117,7 +117,7 @@ class PollAckFlowTest extends FlowTestCase { .setRegistrarId(getRegistrarIdForFlow()) .setEventTime(clock.nowUtc().minusDays(1)) .setMsg("Some poll message.") - .setParent(createHistoryEntryForEppResource(contact)) + .setHistoryEntry(createHistoryEntryForEppResource(contact)) .build()); assertTransactionalFlow(true); assertThrows(MessageDoesNotExistException.class, this::runFlow); @@ -205,7 +205,7 @@ class PollAckFlowTest extends FlowTestCase { .setRegistrarId(getRegistrarIdForFlow()) .setEventTime(clock.nowUtc().minusDays(1)) .setMsg("Some poll message.") - .setParent(createHistoryEntryForEppResource(contact)) + .setHistoryEntry(createHistoryEntryForEppResource(contact)) .build()); assertTransactionalFlow(true); assertThrows(InvalidMessageIdException.class, this::runFlow); @@ -240,7 +240,7 @@ class PollAckFlowTest extends FlowTestCase { .setRegistrarId("TheRegistrar") .setEventTime(clock.nowUtc().minusDays(1)) .setMsg("Some poll message.") - .setParent(createHistoryEntryForEppResource(domain)) + .setHistoryEntry(createHistoryEntryForEppResource(domain)) .build()); assertTransactionalFlow(true); assertThrows(NotAuthorizedToAckMessageException.class, this::runFlow); @@ -254,7 +254,7 @@ class PollAckFlowTest extends FlowTestCase { .setRegistrarId(getRegistrarIdForFlow()) .setEventTime(clock.nowUtc().plusDays(1)) .setMsg("Some poll message.") - .setParent(createHistoryEntryForEppResource(domain)) + .setHistoryEntry(createHistoryEntryForEppResource(domain)) .build()); assertTransactionalFlow(true); Exception e = assertThrows(MessageDoesNotExistException.class, this::runFlow); diff --git a/core/src/test/java/google/registry/flows/poll/PollRequestFlowTest.java b/core/src/test/java/google/registry/flows/poll/PollRequestFlowTest.java index 92b6bf6f5..ed7e8a90c 100644 --- a/core/src/test/java/google/registry/flows/poll/PollRequestFlowTest.java +++ b/core/src/test/java/google/registry/flows/poll/PollRequestFlowTest.java @@ -87,7 +87,7 @@ class PollRequestFlowTest extends FlowTestCase { .setPendingTransferExpirationTime(clock.nowUtc().minusDays(1)) .setExtendedRegistrationExpirationTime(clock.nowUtc().plusYears(1)) .build())) - .setParent(createHistoryEntryForEppResource(domain)) + .setHistoryEntry(createHistoryEntryForEppResource(domain)) .build()); } @@ -125,7 +125,7 @@ class PollRequestFlowTest extends FlowTestCase { .setLosingRegistrarId("NewRegistrar") .setPendingTransferExpirationTime(clock.nowUtc()) .build())) - .setParent(createHistoryEntryForEppResource(contact)) + .setHistoryEntry(createHistoryEntryForEppResource(contact)) .build()); assertTransactionalFlow(false); runFlowAssertResponse(loadFile("poll_response_contact_transfer.xml")); @@ -145,7 +145,7 @@ class PollRequestFlowTest extends FlowTestCase { true, Trid.create("ABC-12345", "other-trid"), clock.nowUtc()))) - .setParent(createHistoryEntryForEppResource(domain)) + .setHistoryEntry(createHistoryEntryForEppResource(domain)) .build()); assertTransactionalFlow(false); runFlowAssertResponse(loadFile("poll_response_domain_pending_notification.xml")); @@ -169,7 +169,7 @@ class PollRequestFlowTest extends FlowTestCase { true, Trid.create("ABC-12345", "other-trid"), clock.nowUtc()))) - .setParent(createHistoryEntryForEppResource(domain)) + .setHistoryEntry(createHistoryEntryForEppResource(domain)) .build()); assertTransactionalFlow(false); runFlowAssertResponse(loadFile("poll_message_domain_pending_action_immediate_delete.xml")); @@ -183,7 +183,7 @@ class PollRequestFlowTest extends FlowTestCase { .setEventTime(clock.nowUtc().minusDays(1)) .setMsg("Domain was auto-renewed.") .setTargetId("test.example") - .setParent(createHistoryEntryForEppResource(domain)) + .setHistoryEntry(createHistoryEntryForEppResource(domain)) .build()); assertTransactionalFlow(false); runFlowAssertResponse(loadFile("poll_response_autorenew.xml")); @@ -201,7 +201,7 @@ class PollRequestFlowTest extends FlowTestCase { .setRegistrarId("BadRegistrar") .setEventTime(clock.nowUtc().minusDays(1)) .setMsg("Poll message") - .setParent(createHistoryEntryForEppResource(domain)) + .setHistoryEntry(createHistoryEntryForEppResource(domain)) .build()); runFlowAssertResponse(loadFile("poll_response_empty.xml")); } @@ -213,7 +213,7 @@ class PollRequestFlowTest extends FlowTestCase { .setRegistrarId(getRegistrarIdForFlow()) .setEventTime(clock.nowUtc().plusDays(1)) .setMsg("Poll message") - .setParent(createHistoryEntryForEppResource(domain)) + .setHistoryEntry(createHistoryEntryForEppResource(domain)) .build()); runFlowAssertResponse(loadFile("poll_response_empty.xml")); } @@ -226,7 +226,7 @@ class PollRequestFlowTest extends FlowTestCase { .setEventTime(clock.nowUtc().plusDays(1)) .setMsg("Domain was auto-renewed.") .setTargetId("target.example") - .setParent(createHistoryEntryForEppResource(domain)) + .setHistoryEntry(createHistoryEntryForEppResource(domain)) .build()); assertTransactionalFlow(false); runFlowAssertResponse(loadFile("poll_response_empty.xml")); @@ -248,7 +248,7 @@ class PollRequestFlowTest extends FlowTestCase { new PollMessage.OneTime.Builder() .setRegistrarId("NewRegistrar") .setMsg("Deleted contact jd1234") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .setEventTime(clock.nowUtc().minusDays(1)) .build()); assertTransactionalFlow(false); @@ -271,7 +271,7 @@ class PollRequestFlowTest extends FlowTestCase { new PollMessage.OneTime.Builder() .setRegistrarId("NewRegistrar") .setMsg("Deleted host ns1.test.example") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .setEventTime(clock.nowUtc().minusDays(1)) .build()); clock.advanceOneMilli(); diff --git a/core/src/test/java/google/registry/model/contact/ContactResourceTest.java b/core/src/test/java/google/registry/model/contact/ContactResourceTest.java index cf805c372..eb32b8633 100644 --- a/core/src/test/java/google/registry/model/contact/ContactResourceTest.java +++ b/core/src/test/java/google/registry/model/contact/ContactResourceTest.java @@ -156,7 +156,7 @@ public class ContactResourceTest extends EntityTestCase { originalContact .getTransferData() .asBuilder() - .setServerApproveEntities(null) + .setServerApproveEntities(null, null, null) .build()) .build(); assertAboutImmutableObjects().that(persisted).isEqualExceptFields(fixed, "updateTimestamp"); diff --git a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java index 7ab9735cd..aa30b99be 100644 --- a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java @@ -422,14 +422,14 @@ public class DomainBaseSqlTest { .setId(300L) .setRegistrarId("registrar1") .setEventTime(DateTime.now(UTC).plusYears(1)) - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build(); PollMessage.OneTime deletePollMessage = new PollMessage.OneTime.Builder() .setId(400L) .setRegistrarId("registrar1") .setEventTime(DateTime.now(UTC).plusYears(1)) - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build(); BillingEvent.OneTime oneTimeBillingEvent = new BillingEvent.OneTime.Builder() @@ -469,7 +469,8 @@ public class DomainBaseSqlTest { domain .asBuilder() .setAutorenewBillingEvent(billEvent.createVKey()) - .setAutorenewPollMessage(autorenewPollMessage.createVKey()) + .setAutorenewPollMessage( + autorenewPollMessage.createVKey(), autorenewPollMessage.getHistoryRevisionId()) .setDeletePollMessage(deletePollMessage.createVKey()) .setTransferData(transferData) .setGracePeriods(gracePeriods) @@ -548,14 +549,14 @@ public class DomainBaseSqlTest { .setId(300L) .setRegistrarId("registrar1") .setEventTime(DateTime.now(UTC).plusYears(1)) - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build(); PollMessage.OneTime deletePollMessage = new PollMessage.OneTime.Builder() .setId(400L) .setRegistrarId("registrar1") .setEventTime(DateTime.now(UTC).plusYears(1)) - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build(); BillingEvent.OneTime oneTimeBillingEvent = new BillingEvent.OneTime.Builder() @@ -572,6 +573,8 @@ public class DomainBaseSqlTest { .build(); DomainTransferData transferData = createPendingTransferData( + domain.getRepoId(), + historyEntry.getId(), new DomainTransferData.Builder() .setTransferRequestTrid(Trid.create("foo", "bar")) .setTransferRequestTime(fakeClock.nowUtc()) @@ -601,7 +604,8 @@ public class DomainBaseSqlTest { .setAutorenewBillingEvent( createLegacyVKey(BillingEvent.Recurring.class, billEvent.getId())) .setAutorenewPollMessage( - createLegacyVKey(PollMessage.Autorenew.class, autorenewPollMessage.getId())) + createLegacyVKey(PollMessage.Autorenew.class, autorenewPollMessage.getId()), + autorenewPollMessage.getHistoryRevisionId()) .setDeletePollMessage( createLegacyVKey(PollMessage.OneTime.class, deletePollMessage.getId())) .setTransferData(transferData) diff --git a/core/src/test/java/google/registry/model/domain/DomainBaseTest.java b/core/src/test/java/google/registry/model/domain/DomainBaseTest.java index a92d69a13..cde8e2e7e 100644 --- a/core/src/test/java/google/registry/model/domain/DomainBaseTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainBaseTest.java @@ -21,6 +21,7 @@ import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.testing.DatabaseHelper.cloneAndSetAutoTimestamps; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.insertInDb; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.newHostResource; import static google.registry.testing.DatabaseHelper.persistActiveContact; @@ -28,6 +29,7 @@ import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistActiveHost; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.DomainBaseSubject.assertAboutDomains; +import static google.registry.testing.SqlHelper.saveRegistrar; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.money.CurrencyUnit.USD; @@ -41,7 +43,6 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Ordering; import com.google.common.collect.Streams; import com.googlecode.objectify.Key; -import google.registry.model.EntityTestCase; import google.registry.model.ImmutableObject; import google.registry.model.ImmutableObjectSubject; import google.registry.model.billing.BillingEvent; @@ -62,15 +63,28 @@ import google.registry.model.tld.Registry; import google.registry.model.transfer.DomainTransferData; import google.registry.model.transfer.TransferStatus; import google.registry.persistence.VKey; +import google.registry.testing.AppEngineExtension; +import google.registry.testing.FakeClock; import java.util.Optional; import org.joda.money.Money; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; /** Unit tests for {@link DomainBase}. */ @SuppressWarnings("WeakerAccess") // Referred to by EppInputTest. -public class DomainBaseTest extends EntityTestCase { +public class DomainBaseTest { + + protected FakeClock fakeClock = new FakeClock(DateTime.now(UTC)); + + @RegisterExtension + public final AppEngineExtension appEngine = + AppEngineExtension.builder() + .withDatastoreAndCloudSql() + .enableJpaEntityCoverageCheck(true) + .withClock(fakeClock) + .build(); private DomainBase domain; private VKey oneTimeBillKey; @@ -80,6 +94,10 @@ public class DomainBaseTest extends EntityTestCase { @BeforeEach void setUp() { + saveRegistrar("registrar1"); + saveRegistrar("registrar2"); + saveRegistrar("losing"); + saveRegistrar("gaining"); createTld("com"); domain = persistActiveDomain("example.com"); VKey hostKey = persistActiveHost("ns1.example.com").createVKey(); @@ -107,19 +125,48 @@ public class DomainBaseTest extends EntityTestCase { .setParent(domainHistory) .build()) .createVKey(); - recurringBillKey = - persistResource( - new BillingEvent.Recurring.Builder() - .setReason(Reason.RENEW) - .setFlags(ImmutableSet.of(Flag.AUTO_RENEW)) - .setTargetId(domain.getDomainName()) - .setRegistrarId(domain.getCurrentSponsorRegistrarId()) - .setDomainRepoId(domain.getRepoId()) - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setRecurrenceEndTime(END_OF_TIME) - .setParent(domainHistory) - .build()) - .createVKey(); + DomainHistory historyEntry = + new DomainHistory.Builder() + .setId(100L) + .setType(HistoryEntry.Type.DOMAIN_CREATE) + .setPeriod(Period.create(1, Period.Unit.YEARS)) + .setModificationTime(DateTime.now(UTC)) + .setDomainRepoId(domain.getRepoId()) + .setRegistrarId(domain.getCurrentSponsorRegistrarId()) + // These are non-null, but I don't think some tests set them. + .setReason("felt like it") + .setRequestedByRegistrar(false) + .setXmlBytes(new byte[0]) + .build(); + BillingEvent.OneTime oneTimeBill = + new BillingEvent.OneTime.Builder() + .setId(500L) + // Use SERVER_STATUS so we don't have to add a period. + .setReason(Reason.SERVER_STATUS) + .setTargetId("example.com") + .setRegistrarId("registrar1") + .setDomainRepoId("4-COM") + .setBillingTime(DateTime.now(UTC)) + .setCost(Money.of(USD, 100)) + .setEventTime(DateTime.now(UTC).plusYears(1)) + .setParent(historyEntry) + .build(); + oneTimeBillKey = oneTimeBill.createVKey(); + BillingEvent.Recurring recurringBill = + new BillingEvent.Recurring.Builder() + .setId(200L) + .setReason(Reason.RENEW) + .setFlags(ImmutableSet.of(Flag.AUTO_RENEW)) + .setTargetId(domain.getDomainName()) + .setRegistrarId(domain.getCurrentSponsorRegistrarId()) + .setDomainRepoId(domain.getRepoId()) + .setDomainHistoryRevisionId(historyEntry.getId()) + .setEventTime(DateTime.now(UTC).plusYears(1)) + .setRecurrenceEndTime(END_OF_TIME) + .setParent(historyEntry) + .build(); + insertInDb(historyEntry, oneTimeBill, recurringBill); + recurringBillKey = recurringBill.createVKey(); VKey autorenewPollKey = VKey.from(Key.create(Key.create(domainHistory), PollMessage.Autorenew.class, 3)); VKey onetimePollKey = @@ -134,6 +181,7 @@ public class DomainBaseTest extends EntityTestCase { .setCreationRegistrarId("TheRegistrar") .setLastEppUpdateTime(fakeClock.nowUtc()) .setLastEppUpdateRegistrarId("NewRegistrar") + .setPersistedCurrentSponsorRegistrarId("NewRegistrar") .setLastTransferTime(fakeClock.nowUtc()) .setStatusValues( ImmutableSet.of( @@ -144,7 +192,6 @@ public class DomainBaseTest extends EntityTestCase { StatusValue.SERVER_RENEW_PROHIBITED, StatusValue.SERVER_HOLD)) .setRegistrant(contact1Key) - .setContacts(ImmutableSet.of(DesignatedContact.create(Type.ADMIN, contact2Key))) .setNameservers(ImmutableSet.of(hostKey)) .setSubordinateHosts(ImmutableSet.of("ns1.example.com")) .setPersistedCurrentSponsorRegistrarId("NewRegistrar") @@ -160,6 +207,8 @@ public class DomainBaseTest extends EntityTestCase { .setLosingRegistrarId("NewRegistrar") .setPendingTransferExpirationTime(fakeClock.nowUtc()) .setServerApproveEntities( + historyEntry.getDomainRepoId(), + historyEntry.getId(), ImmutableSet.of(oneTimeBillKey, recurringBillKey, autorenewPollKey)) .setServerApproveBillingEvent(oneTimeBillKey) .setServerApproveAutorenewEvent(recurringBillKey) @@ -170,7 +219,7 @@ public class DomainBaseTest extends EntityTestCase { .build()) .setDeletePollMessage(onetimePollKey) .setAutorenewBillingEvent(recurringBillKey) - .setAutorenewPollMessage(autorenewPollKey) + .setAutorenewPollMessage(autorenewPollKey, historyEntry.getId()) .setSmdId("smdid") .addGracePeriod( GracePeriod.create( @@ -200,19 +249,6 @@ public class DomainBaseTest extends EntityTestCase { .hasValue(domain); } - @Test - void testVKeyRestoration() { - assertThat(domain.deletePollMessageHistoryId).isEqualTo(domainHistory.getId()); - assertThat(domain.autorenewBillingEventHistoryId).isEqualTo(domainHistory.getId()); - assertThat(domain.autorenewPollMessageHistoryId).isEqualTo(domainHistory.getId()); - assertThat(domain.getTransferData().getServerApproveBillingEventHistoryId()) - .isEqualTo(domainHistory.getId()); - assertThat(domain.getTransferData().getServerApproveAutorenewEventHistoryId()) - .isEqualTo(domainHistory.getId()); - assertThat(domain.getTransferData().getServerApproveAutorenewPollMessageHistoryId()) - .isEqualTo(domainHistory.getId()); - } - @Test void testEmptyStringsBecomeNull() { assertThat( @@ -238,7 +274,6 @@ public class DomainBaseTest extends EntityTestCase { .isNotNull(); } - @Test void testEmptySetsAndArraysBecomeNull() { assertThat( newDomainBase("example.com") @@ -407,7 +442,10 @@ public class DomainBaseTest extends EntityTestCase { .setPendingTransferExpirationTime(fakeClock.nowUtc().plusDays(1)) .setGainingRegistrarId("TheRegistrar") .setServerApproveBillingEvent(transferBillingEvent.createVKey()) - .setServerApproveEntities(ImmutableSet.of(transferBillingEvent.createVKey())) + .setServerApproveEntities( + domain.getRepoId(), + historyEntry.getId(), + ImmutableSet.of(transferBillingEvent.createVKey())) .build()) .addGracePeriod( // Okay for billing event to be null since the point of this grace period is just diff --git a/core/src/test/java/google/registry/model/poll/PollMessageExternalKeyConverterTest.java b/core/src/test/java/google/registry/model/poll/PollMessageExternalKeyConverterTest.java index 615a35cc2..b2a309d7e 100644 --- a/core/src/test/java/google/registry/model/poll/PollMessageExternalKeyConverterTest.java +++ b/core/src/test/java/google/registry/model/poll/PollMessageExternalKeyConverterTest.java @@ -83,7 +83,7 @@ public class PollMessageExternalKeyConverterTest { .setRegistrarId("TheRegistrar") .setEventTime(clock.nowUtc()) .setMsg("Test poll message") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build()); assertThat(makePollMessageExternalId(pollMessage)).isEqualTo("1-2-FOOBAR-4-5-2007"); assertVKeysEqual(parsePollMessageExternalId("1-2-FOOBAR-4-5-2007"), pollMessage.createVKey()); @@ -100,7 +100,7 @@ public class PollMessageExternalKeyConverterTest { .setRegistrarId("TheRegistrar") .setEventTime(clock.nowUtc()) .setMsg("Test poll message") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build()); assertThat(makePollMessageExternalId(pollMessage)).isEqualTo("2-5-ROID-6-7-2007"); assertVKeysEqual(parsePollMessageExternalId("2-5-ROID-6-7-2007"), pollMessage.createVKey()); @@ -117,7 +117,7 @@ public class PollMessageExternalKeyConverterTest { .setRegistrarId("TheRegistrar") .setEventTime(clock.nowUtc()) .setMsg("Test poll message") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build()); assertThat(makePollMessageExternalId(pollMessage)).isEqualTo("3-5-ROID-6-7-2007"); assertVKeysEqual(parsePollMessageExternalId("3-5-ROID-6-7-2007"), pollMessage.createVKey()); @@ -170,6 +170,5 @@ public class PollMessageExternalKeyConverterTest { || two.getKind().isAssignableFrom(one.getKind())) .isTrue(); assertThat(one.getSqlKey()).isEqualTo(two.getSqlKey()); - assertThat(one.getOfyKey()).isEqualTo(two.getOfyKey()); } } diff --git a/core/src/test/java/google/registry/model/poll/PollMessageTest.java b/core/src/test/java/google/registry/model/poll/PollMessageTest.java index 04f7cd00f..b4a181bcf 100644 --- a/core/src/test/java/google/registry/model/poll/PollMessageTest.java +++ b/core/src/test/java/google/registry/model/poll/PollMessageTest.java @@ -80,7 +80,7 @@ public class PollMessageTest extends EntityTestCase { .setRegistrarId("TheRegistrar") .setEventTime(fakeClock.nowUtc()) .setMsg("Test poll message") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build(); autoRenew = new PollMessage.Autorenew.Builder() @@ -88,7 +88,7 @@ public class PollMessageTest extends EntityTestCase { .setRegistrarId("TheRegistrar") .setEventTime(fakeClock.nowUtc()) .setMsg("Test poll message") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .setAutorenewEndTime(fakeClock.nowUtc().plusDays(365)) .setTargetId("foobar.foo") .build(); @@ -116,7 +116,7 @@ public class PollMessageTest extends EntityTestCase { .setRegistrarId("TheRegistrar") .setEventTime(fakeClock.nowUtc()) .setMsg("Test poll message") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build()); assertThat(tm().transact(() -> tm().loadByEntity(pollMessage))).isEqualTo(pollMessage); } @@ -135,16 +135,15 @@ public class PollMessageTest extends EntityTestCase { .setRegistrarId("TheRegistrar") .setEventTime(fakeClock.nowUtc()) .setMsg("Test poll message") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .setResponseData(ImmutableList.of(hostPendingActionNotificationResponse)) .build(); persistResource(pollMessage); assertThat(tm().transact(() -> tm().loadByEntity(pollMessage).getMsg())) .isEqualTo(pollMessage.msg); assertThat( - tm().transact(() -> tm().loadByEntity(pollMessage)) - .hostPendingActionNotificationResponses) - .contains(hostPendingActionNotificationResponse); + tm().transact(() -> tm().loadByEntity(pollMessage)).pendingActionNotificationResponse) + .isEqualTo(hostPendingActionNotificationResponse); } @TestSqlOnly @@ -155,7 +154,7 @@ public class PollMessageTest extends EntityTestCase { .setRegistrarId("TheRegistrar") .setEventTime(fakeClock.nowUtc()) .setMsg("Test poll message") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build()); PollMessage persisted = tm().transact(() -> tm().loadByEntity(pollMessage)); assertThat(SerializeUtils.serializeDeserialize(persisted)).isEqualTo(persisted); @@ -169,7 +168,7 @@ public class PollMessageTest extends EntityTestCase { .setRegistrarId("TheRegistrar") .setEventTime(fakeClock.nowUtc()) .setMsg("Test poll message") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .setAutorenewEndTime(fakeClock.nowUtc().plusDays(365)) .setTargetId("foobar.foo") .build()); @@ -184,7 +183,7 @@ public class PollMessageTest extends EntityTestCase { .setRegistrarId("TheRegistrar") .setEventTime(fakeClock.nowUtc()) .setMsg("Test poll message") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .setAutorenewEndTime(fakeClock.nowUtc().plusDays(365)) .setTargetId("foobar.foo") .build()); @@ -200,7 +199,7 @@ public class PollMessageTest extends EntityTestCase { .setRegistrarId("TheRegistrar") .setEventTime(fakeClock.nowUtc()) .setMsg("Test poll message") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .setAutorenewEndTime(fakeClock.nowUtc().plusDays(365)) .setTargetId("foobar.foo") .build()); diff --git a/core/src/test/java/google/registry/model/transfer/TransferDataTest.java b/core/src/test/java/google/registry/model/transfer/TransferDataTest.java index ad98ab324..72764cb4c 100644 --- a/core/src/test/java/google/registry/model/transfer/TransferDataTest.java +++ b/core/src/test/java/google/registry/model/transfer/TransferDataTest.java @@ -95,6 +95,8 @@ public class TransferDataTest { .setPendingTransferExpirationTime(now) .setTransferStatus(TransferStatus.PENDING) .setServerApproveEntities( + "4-TLD", + 1356L, ImmutableSet.of( transferBillingEventKey, otherServerApproveBillingEventKey, diff --git a/core/src/test/java/google/registry/persistence/DomainHistoryVKeyTest.java b/core/src/test/java/google/registry/persistence/DomainHistoryVKeyTest.java index 5590f974e..f8d7f55d8 100644 --- a/core/src/test/java/google/registry/persistence/DomainHistoryVKeyTest.java +++ b/core/src/test/java/google/registry/persistence/DomainHistoryVKeyTest.java @@ -55,17 +55,10 @@ class DomainHistoryVKeyTest { TestEntity persisted = tm().transact(() -> tm().loadByKey(original.createVKey())); assertThat(persisted).isEqualTo(original); // Double check that the persisted.domainHistoryVKey is a symmetric VKey - assertThat(persisted.domainHistoryVKey.createOfyKey()) - .isEqualTo( - Key.create(Key.create(DomainBase.class, "domainRepoId"), HistoryEntry.class, 10L)); assertThat(persisted.domainHistoryVKey.createSqlKey()) .isEqualTo(new DomainHistoryId("domainRepoId", 10L)); assertThat(persisted.domainHistoryVKey.createVKey()) - .isEqualTo( - VKey.create( - HistoryEntry.class, - new DomainHistoryId("domainRepoId", 10L), - Key.create(Key.create(DomainBase.class, "domainRepoId"), HistoryEntry.class, 10L))); + .isEqualTo(VKey.createSql(HistoryEntry.class, new DomainHistoryId("domainRepoId", 10L))); } @TestOfyAndSql @@ -73,7 +66,6 @@ class DomainHistoryVKeyTest { Key ofyKey = Key.create(Key.create(DomainBase.class, "domainRepoId"), HistoryEntry.class, 10L); DomainHistoryVKey domainHistoryVKey = DomainHistoryVKey.create(ofyKey); - assertThat(domainHistoryVKey.createOfyKey()).isEqualTo(ofyKey); assertThat(domainHistoryVKey.createSqlKey()) .isEqualTo(new DomainHistoryId("domainRepoId", 10L)); assertThat(domainHistoryVKey.createVKey()) diff --git a/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java b/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java index 2bdb89377..3430d6826 100644 --- a/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java +++ b/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java @@ -333,9 +333,10 @@ public class DomainBaseToXjcConverterTest { .setEventTime(END_OF_TIME) .setAutorenewEndTime(END_OF_TIME) .setMsg("Domain was auto-renewed.") - .setParent(domainHistory) + .setHistoryEntry(domainHistory) .build()) - .createVKey()) + .createVKey(), + domainHistory.getId()) .setTransferData( new DomainTransferData.Builder() .setGainingRegistrarId("NewRegistrar") @@ -362,10 +363,13 @@ public class DomainBaseToXjcConverterTest { .setEventTime(END_OF_TIME) .setAutorenewEndTime(END_OF_TIME) .setMsg("Domain was auto-renewed.") - .setParent(domainHistory) + .setHistoryEntry(domainHistory) .build()) .createVKey()) - .setServerApproveEntities(ImmutableSet.of(billingEvent.createVKey())) + .setServerApproveEntities( + domain.getRepoId(), + domainHistory.getId(), + ImmutableSet.of(billingEvent.createVKey())) .setTransferRequestTime(DateTime.parse("1919-01-01T00:00:00Z")) .setTransferStatus(TransferStatus.PENDING) .setTransferredRegistrationExpirationTime( diff --git a/core/src/test/java/google/registry/rde/RdeFixtures.java b/core/src/test/java/google/registry/rde/RdeFixtures.java index 7d874e025..7bd3629c8 100644 --- a/core/src/test/java/google/registry/rde/RdeFixtures.java +++ b/core/src/test/java/google/registry/rde/RdeFixtures.java @@ -174,9 +174,10 @@ final class RdeFixtures { .setEventTime(END_OF_TIME) .setAutorenewEndTime(END_OF_TIME) .setMsg("Domain was auto-renewed.") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build()) - .createVKey()) + .createVKey(), + historyEntry.getId()) .setTransferData( new DomainTransferData.Builder() .setGainingRegistrarId("gaining") @@ -203,10 +204,13 @@ final class RdeFixtures { .setEventTime(END_OF_TIME) .setAutorenewEndTime(END_OF_TIME) .setMsg("Domain was auto-renewed.") - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build()) .createVKey()) - .setServerApproveEntities(ImmutableSet.of(billingEvent.createVKey())) + .setServerApproveEntities( + historyEntry.getDomainRepoId(), + historyEntry.getId(), + ImmutableSet.of(billingEvent.createVKey())) .setTransferRequestTime(DateTime.parse("1991-01-01T00:00:00Z")) .setTransferStatus(TransferStatus.PENDING) .setTransferredRegistrationExpirationTime( diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index ec356cd57..ed866ac5d 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -510,7 +510,7 @@ public class DatabaseHelper { .setEventTime(expirationTime) .setMsg("Transfer server approved.") .setResponseData(ImmutableList.of(createTransferResponse(resource, transferData))) - .setParent(historyEntry) + .setHistoryEntry(historyEntry) .build(); } @@ -549,6 +549,8 @@ public class DatabaseHelper { createContactTransferDataBuilder(requestTime, expirationTime) .setPendingTransferExpirationTime(now.plus(getContactAutomaticTransferLength())) .setServerApproveEntities( + ((ContactHistory) historyEntryContactTransfer).getContactRepoId(), + historyEntryContactTransfer.getId(), ImmutableSet.of( // Pretend it's 3 days since the request persistResource( @@ -630,13 +632,14 @@ public class DatabaseHelper { .setEventTime(expirationTime) .setAutorenewEndTime(END_OF_TIME) .setMsg("Domain was auto-renewed.") - .setParent(historyEntryDomainCreate) + .setHistoryEntry(historyEntryDomainCreate) .build()); return persistResource( domain .asBuilder() .setAutorenewBillingEvent(autorenewEvent.createVKey()) - .setAutorenewPollMessage(autorenewPollMessage.createVKey()) + .setAutorenewPollMessage( + autorenewPollMessage.createVKey(), autorenewPollMessage.getHistoryRevisionId()) .build()); } @@ -676,7 +679,7 @@ public class DatabaseHelper { .setEventTime(extendedRegistrationExpirationTime) .setAutorenewEndTime(END_OF_TIME) .setMsg("Domain was auto-renewed.") - .setParent(historyEntryDomainTransfer) + .setHistoryEntry(historyEntryDomainTransfer) .build()); // Modify the existing autorenew event to reflect the pending transfer. persistResource( @@ -711,6 +714,8 @@ public class DatabaseHelper { .setServerApproveAutorenewPollMessage( gainingClientAutorenewPollMessage.createVKey()) .setServerApproveEntities( + historyEntryDomainTransfer.getDomainRepoId(), + historyEntryDomainTransfer.getId(), ImmutableSet.of( transferBillingEvent.createVKey(), gainingClientAutorenewEvent.createVKey(), @@ -887,9 +892,7 @@ public class DatabaseHelper { return transactIfJpaTm( () -> tm().loadAllOf(PollMessage.class).stream() - .filter( - pollMessage -> - pollMessage.getParentKey().getParent().getName().equals(domain.getRepoId())) + .filter(pollMessage -> pollMessage.getDomainRepoId().equals(domain.getRepoId())) .collect(toImmutableList())); } @@ -909,13 +912,7 @@ public class DatabaseHelper { return transactIfJpaTm( () -> tm().loadAllOf(PollMessage.class).stream() - .filter( - pollMessage -> - pollMessage - .getParentKey() - .getParent() - .getName() - .equals(resource.getRepoId())) + .filter(pollMessage -> pollMessage.getDomainRepoId().equals(resource.getRepoId())) .filter(pollMessage -> pollMessage.getRegistrarId().equals(registrarId)) .filter( pollMessage -> @@ -1176,7 +1173,14 @@ public class DatabaseHelper { () -> tm().loadAllOf(PollMessage.class).stream() .filter( - pollMessage -> pollMessage.getParentKey().equals(Key.create(historyEntry))) + pollMessage -> + pollMessage.getResourceName().equals(historyEntry.getParent().getName()) + && pollMessage.getHistoryRevisionId() == historyEntry.getId() + && pollMessage + .getType() + .getResourceClass() + .getName() + .equals(historyEntry.getParent().getKind())) .collect(toImmutableList()))); } diff --git a/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java b/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java index 592af6336..21992b698 100644 --- a/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java +++ b/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java @@ -23,9 +23,9 @@ import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.persistResource; import com.google.common.collect.ImmutableList; -import com.googlecode.objectify.Key; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainHistory; +import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.ofy.Ofy; import google.registry.model.poll.PollMessage; import google.registry.model.poll.PollMessage.Autorenew; @@ -100,7 +100,7 @@ public class AckPollMessagesCommandTest extends CommandTestCase { persistActiveDomain("example.tld"); runCommand("example.tld"); assertInStdout("fullyQualifiedDomainName=example.tld"); - assertInStdout("contact=Key(ContactResource(\"3-ROID\"))"); + assertInStdout("Contact=VKey(sql:3-ROID"); assertInStdout( "Websafe key: " + "kind:DomainBase" @@ -51,7 +51,7 @@ class GetDomainCommandTest extends CommandTestCase { persistActiveDomain("example.tld"); runCommand("example.tld", "--expand"); assertInStdout("fullyQualifiedDomainName=example.tld"); - assertInStdout("contact=Key(ContactResource(\"3-ROID\"))"); + assertInStdout("sqlKey=3-ROID"); assertInStdout( "Websafe key: " + "kind:DomainBase" @@ -66,7 +66,7 @@ class GetDomainCommandTest extends CommandTestCase { persistActiveDomain("xn--aualito-txac.xn--q9jyb4c"); runCommand("çauçalito.みんな", "--expand"); assertInStdout("fullyQualifiedDomainName=xn--aualito-txac.xn--q9jyb4c"); - assertInStdout("contact=Key(ContactResource(\"4-ROID\"))"); + assertInStdout("sqlKey=4-ROID"); } @Test diff --git a/core/src/test/java/google/registry/tools/UnrenewDomainCommandTest.java b/core/src/test/java/google/registry/tools/UnrenewDomainCommandTest.java index e4b43643f..7fb966a06 100644 --- a/core/src/test/java/google/registry/tools/UnrenewDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/UnrenewDomainCommandTest.java @@ -35,7 +35,6 @@ import static google.registry.testing.HistoryEntrySubject.assertAboutHistoryEntr import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Flag; import google.registry.model.billing.BillingEvent.Reason; @@ -148,7 +147,7 @@ public class UnrenewDomainCommandTest extends CommandTestCase contact; - google.registry.model.domain.DesignatedContact$Type type; -} -enum google.registry.model.domain.DesignatedContact$Type { - ADMIN; - BILLING; - REGISTRANT; - TECH; -} class google.registry.model.domain.DomainAuthInfo { google.registry.model.eppcommon.AuthInfo$PasswordAuth pw; } @@ -207,6 +197,10 @@ class google.registry.model.domain.DomainBase { google.registry.model.domain.launch.LaunchNotice launchNotice; google.registry.model.transfer.DomainTransferData transferData; google.registry.persistence.VKey autorenewBillingEvent; + google.registry.persistence.VKey adminContact; + google.registry.persistence.VKey billingContact; + google.registry.persistence.VKey registrantContact; + google.registry.persistence.VKey techContact; google.registry.persistence.VKey autorenewPollMessage; google.registry.persistence.VKey deletePollMessage; java.lang.String creationClientId; @@ -216,7 +210,6 @@ class google.registry.model.domain.DomainBase { java.lang.String lastEppUpdateClientId; java.lang.String smdId; java.lang.String tld; - java.util.Set allContacts; java.util.Set gracePeriods; java.util.Set dsData; java.util.Set status; @@ -236,6 +229,10 @@ class google.registry.model.domain.DomainContent { google.registry.model.domain.launch.LaunchNotice launchNotice; google.registry.model.transfer.DomainTransferData transferData; google.registry.persistence.VKey autorenewBillingEvent; + google.registry.persistence.VKey adminContact; + google.registry.persistence.VKey billingContact; + google.registry.persistence.VKey registrantContact; + google.registry.persistence.VKey techContact; google.registry.persistence.VKey autorenewPollMessage; google.registry.persistence.VKey deletePollMessage; java.lang.String creationClientId; @@ -245,7 +242,6 @@ class google.registry.model.domain.DomainContent { java.lang.String lastEppUpdateClientId; java.lang.String smdId; java.lang.String tld; - java.util.Set allContacts; java.util.Set gracePeriods; java.util.Set dsData; java.util.Set status; @@ -278,16 +274,16 @@ class google.registry.model.domain.DomainHistory { } class google.registry.model.domain.GracePeriod { google.registry.model.domain.rgp.GracePeriodStatus type; - google.registry.persistence.BillingVKey$BillingEventVKey billingEventOneTime; - google.registry.persistence.BillingVKey$BillingRecurrenceVKey billingEventRecurring; + google.registry.persistence.VKey billingEventOneTime; + google.registry.persistence.VKey billingEventRecurring; java.lang.String clientId; long gracePeriodId; org.joda.time.DateTime expirationTime; } class google.registry.model.domain.GracePeriod$GracePeriodHistory { google.registry.model.domain.rgp.GracePeriodStatus type; - google.registry.persistence.BillingVKey$BillingEventVKey billingEventOneTime; - google.registry.persistence.BillingVKey$BillingRecurrenceVKey billingEventRecurring; + google.registry.persistence.VKey billingEventOneTime; + google.registry.persistence.VKey billingEventRecurring; java.lang.Long domainHistoryRevisionId; java.lang.Long gracePeriodHistoryRevisionId; java.lang.String clientId; @@ -469,35 +465,14 @@ class google.registry.model.index.ForeignKeyIndex$ForeignKeyHostIndex { google.registry.persistence.VKey topReference; org.joda.time.DateTime deletionTime; } -class google.registry.model.poll.PendingActionNotificationResponse$ContactPendingActionNotificationResponse { - google.registry.model.eppcommon.Trid trid; - google.registry.model.poll.PendingActionNotificationResponse$NameOrId nameOrId; - org.joda.time.DateTime processedDate; -} -class google.registry.model.poll.PendingActionNotificationResponse$DomainPendingActionNotificationResponse { - google.registry.model.eppcommon.Trid trid; - google.registry.model.poll.PendingActionNotificationResponse$NameOrId nameOrId; - org.joda.time.DateTime processedDate; -} -class google.registry.model.poll.PendingActionNotificationResponse$HostPendingActionNotificationResponse { - google.registry.model.eppcommon.Trid trid; - google.registry.model.poll.PendingActionNotificationResponse$NameOrId nameOrId; - org.joda.time.DateTime processedDate; -} -class google.registry.model.poll.PendingActionNotificationResponse$NameOrId { - boolean actionResult; - java.lang.String value; -} class google.registry.model.poll.PollMessage { @Id java.lang.Long id; - @Parent com.googlecode.objectify.Key parent; java.lang.String clientId; java.lang.String msg; org.joda.time.DateTime eventTime; } class google.registry.model.poll.PollMessage$Autorenew { @Id java.lang.Long id; - @Parent com.googlecode.objectify.Key parent; java.lang.String clientId; java.lang.String msg; java.lang.String targetId; @@ -506,14 +481,8 @@ class google.registry.model.poll.PollMessage$Autorenew { } class google.registry.model.poll.PollMessage$OneTime { @Id java.lang.Long id; - @Parent com.googlecode.objectify.Key parent; java.lang.String clientId; java.lang.String msg; - java.util.List contactPendingActionNotificationResponses; - java.util.List domainPendingActionNotificationResponses; - java.util.List hostPendingActionNotificationResponses; - java.util.List contactTransferResponses; - java.util.List domainTransferResponses; org.joda.time.DateTime eventTime; } class google.registry.model.rde.RdeRevision { @@ -745,7 +714,6 @@ class google.registry.model.transfer.ContactTransferData { google.registry.model.transfer.TransferStatus transferStatus; java.lang.String gainingClientId; java.lang.String losingClientId; - java.util.Set> serverApproveEntities; org.joda.time.DateTime pendingTransferExpirationTime; org.joda.time.DateTime transferRequestTime; } @@ -758,28 +726,10 @@ class google.registry.model.transfer.DomainTransferData { google.registry.persistence.VKey serverApproveAutorenewPollMessage; java.lang.String gainingClientId; java.lang.String losingClientId; - java.util.Set> 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; - java.lang.String contactId; - java.lang.String gainingClientId; - java.lang.String losingClientId; - org.joda.time.DateTime pendingTransferExpirationTime; - org.joda.time.DateTime transferRequestTime; -} -class google.registry.model.transfer.TransferResponse$DomainTransferResponse { - google.registry.model.transfer.TransferStatus transferStatus; - java.lang.String fullyQualifiedDomainName; - java.lang.String gainingClientId; - java.lang.String losingClientId; - org.joda.time.DateTime extendedRegistrationExpirationTime; - org.joda.time.DateTime pendingTransferExpirationTime; - org.joda.time.DateTime transferRequestTime; -} enum google.registry.model.transfer.TransferStatus { CLIENT_APPROVED; CLIENT_CANCELLED; diff --git a/db/src/main/resources/sql/schema/db-schema.sql.generated b/db/src/main/resources/sql/schema/db-schema.sql.generated index 4747100b4..b4b5fbb8e 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/db/src/main/resources/sql/schema/db-schema.sql.generated @@ -265,13 +265,11 @@ auth_info_repo_id text, auth_info_value text, billing_recurrence_id int8, - billing_recurrence_history_id int8, autorenew_end_time timestamptz, autorenew_poll_message_id int8, autorenew_poll_message_history_id int8, billing_contact text, deletion_poll_message_id int8, - deletion_poll_message_history_id int8, dns_refresh_request_time timestamptz, domain_name text, idn_table_name text, @@ -286,14 +284,11 @@ subordinate_hosts text[], tech_contact text, tld text, - transfer_billing_cancellation_history_id int8, transfer_billing_cancellation_id int8, transfer_billing_recurrence_id int8, - transfer_billing_recurrence_history_id int8, transfer_autorenew_poll_message_id int8, transfer_autorenew_poll_message_history_id int8, transfer_billing_event_id int8, - transfer_billing_event_history_id int8, transfer_renew_period_unit text, transfer_renew_period_value int4, transfer_registration_expiration_time timestamptz, @@ -339,13 +334,11 @@ auth_info_repo_id text, auth_info_value text, billing_recurrence_id int8, - billing_recurrence_history_id int8, autorenew_end_time timestamptz, autorenew_poll_message_id int8, autorenew_poll_message_history_id int8, billing_contact text, deletion_poll_message_id int8, - deletion_poll_message_history_id int8, dns_refresh_request_time timestamptz, domain_name text, idn_table_name text, @@ -360,14 +353,11 @@ subordinate_hosts text[], tech_contact text, tld text, - transfer_billing_cancellation_history_id int8, transfer_billing_cancellation_id int8, transfer_billing_recurrence_id int8, - transfer_billing_recurrence_history_id int8, transfer_autorenew_poll_message_id int8, transfer_autorenew_poll_message_history_id int8, transfer_billing_event_id int8, - transfer_billing_event_history_id int8, transfer_renew_period_unit text, transfer_renew_period_value int4, transfer_registration_expiration_time timestamptz, @@ -422,11 +412,7 @@ create table "GracePeriod" ( grace_period_id int8 not null, billing_event_id int8, - billing_event_history_id int8, - billing_event_domain_repo_id text, billing_recurrence_id int8, - billing_recurrence_history_id int8, - billing_recurrence_domain_repo_id text, registrar_id text not null, domain_repo_id text not null, expiration_time timestamptz not null, @@ -437,11 +423,7 @@ create table "GracePeriodHistory" ( grace_period_history_revision_id int8 not null, billing_event_id int8, - billing_event_history_id int8, - billing_event_domain_repo_id text, billing_recurrence_id int8, - billing_recurrence_history_id int8, - billing_recurrence_domain_repo_id text, registrar_id text not null, domain_repo_id text not null, expiration_time timestamptz not null, diff --git a/util/src/main/java/google/registry/util/NullIgnoringCollectionBuilder.java b/util/src/main/java/google/registry/util/NullIgnoringCollectionBuilder.java new file mode 100644 index 000000000..78eb706c3 --- /dev/null +++ b/util/src/main/java/google/registry/util/NullIgnoringCollectionBuilder.java @@ -0,0 +1,50 @@ +// Copyright 2022 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.util; + +import com.google.common.collect.ImmutableCollection; +import javax.annotation.Nullable; + +/** + * Collection builder that simply ignores requests to add null elements. + * + *

Useful because we do this in a number of cases. + */ +public class NullIgnoringCollectionBuilder> { + + private B builder; + + private NullIgnoringCollectionBuilder(B builder) { + this.builder = builder; + } + + public static > + NullIgnoringCollectionBuilder create(B2 builder) { + return new NullIgnoringCollectionBuilder(builder); + } + + /** If 'elem' is not null, add it to the builder. */ + public NullIgnoringCollectionBuilder add(@Nullable E elem) { + if (elem != null) { + builder.add(elem); + } + return this; + } + + /** Returns the underlying builder. */ + public B getBuilder() { + return builder; + } +} diff --git a/util/src/test/java/google/registry/util/NullIgnoringCollectionBuilderTest.java b/util/src/test/java/google/registry/util/NullIgnoringCollectionBuilderTest.java new file mode 100644 index 000000000..1053766eb --- /dev/null +++ b/util/src/test/java/google/registry/util/NullIgnoringCollectionBuilderTest.java @@ -0,0 +1,54 @@ +// Copyright 2022 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.util; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import org.junit.jupiter.api.Test; + +public class NullIgnoringCollectionBuilderTest { + + public NullIgnoringCollectionBuilderTest() {} + + @Test + public void testNullSafeCollections() { + assertThat( + NullIgnoringCollectionBuilder.create(new ImmutableList.Builder()) + .getBuilder() + .build()) + .isEqualTo(ImmutableList.of()); + + assertThat( + NullIgnoringCollectionBuilder.create(new ImmutableList.Builder()) + .add(100) + .add(200) + .add(null) + .getBuilder() + .build()) + .containsExactly(100, 200) + .inOrder(); + + assertThat( + NullIgnoringCollectionBuilder.create(new ImmutableSet.Builder()) + .add(100) + .add(200) + .add(null) + .getBuilder() + .build()) + .containsExactly(100, 200); + } +}