From ad4aa73328b621fe13e67eec7ec0600d1353d162 Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Tue, 1 Nov 2022 21:17:20 -0400 Subject: [PATCH] Remove ofy support from HistoryEntry (#1823) This PR removes all Ofy related cruft around `HistoryEntry` and its three subclasses in order to support dual-write to datastore and SQL. The class structure was refactored to take advantage of inheritance to reduce code duplication and improve clarity. Note that for the embedded EPP resources, either their columns are all empty (for pre-3.0 entities imported into SQL), including their unique foreign key (domain name, host name, contact id) and the update timestamp; or they are filled as expected (for entities that were written since dual writing was implemented). Therefore the check for foreign key column nullness in the various `@PostLoad` methods in the original code is an no-op as the EPP resource would have been loaded as null. In another word, there is no case where the update timestamp is null but other columns are not. See the following query for the most recent entries in each table where the foreign key column or the update timestamp are null -- they are the same. ``` [I]postgres=> select MAX(history_modification_time) from "DomainHistory" where update_timestamp is null; max ---------------------------- 2021-09-27 15:56:52.502+00 (1 row) [I]postgres=> select MAX(history_modification_time) from "DomainHistory" where domain_name is null; max ---------------------------- 2021-09-27 15:56:52.502+00 (1 row) [I]postgres=> select MAX(history_modification_time) from "ContactHistory" where update_timestamp is null; max ---------------------------- 2021-09-27 15:56:04.311+00 (1 row) [I]postgres=> select MAX(history_modification_time) from "ContactHistory" where contact_id is null; max ---------------------------- 2021-09-27 15:56:04.311+00 (1 row) [I]postgres=> select MAX(history_modification_time) from "HostHistory" where update_timestamp is null; max ---------------------------- 2021-09-27 15:52:16.517+00 (1 row) [I]postgres=> select MAX(history_modification_time) from "HostHistory" where host_name is null; max ---------------------------- 2021-09-27 15:52:16.517+00 (1 row) ``` --- .../batch/DeleteProberDataAction.java | 57 +-- .../google/registry/beam/rde/RdePipeline.java | 71 ++-- .../java/google/registry/flows/FlowUtils.java | 10 +- .../flows/contact/ContactFlowUtils.java | 19 +- .../contact/ContactTransferApproveFlow.java | 3 +- .../contact/ContactTransferCancelFlow.java | 5 +- .../contact/ContactTransferRejectFlow.java | 3 +- .../contact/ContactTransferRequestFlow.java | 37 +- .../flows/domain/DomainCreateFlow.java | 38 +- .../flows/domain/DomainDeleteFlow.java | 37 +- .../flows/domain/DomainFlowUtils.java | 17 +- .../flows/domain/DomainRenewFlow.java | 66 ++-- .../domain/DomainRestoreRequestFlow.java | 52 ++- .../domain/DomainTransferApproveFlow.java | 35 +- .../domain/DomainTransferCancelFlow.java | 16 +- .../domain/DomainTransferRejectFlow.java | 12 +- .../domain/DomainTransferRequestFlow.java | 26 +- .../flows/domain/DomainTransferUtils.java | 51 +-- .../flows/domain/DomainUpdateFlow.java | 9 +- .../token/AllocationTokenFlowUtils.java | 11 +- .../google/registry/model/EntityClasses.java | 14 +- .../model/UpdateAutoTimestampEntity.java | 2 +- .../registry/model/billing/BillingEvent.java | 16 +- .../model/contact/ContactHistory.java | 162 ++------ .../registry/model/domain/DomainHistory.java | 270 ++++--------- .../registry/model/domain/GracePeriod.java | 21 +- .../domain/secdns/DomainDsDataHistory.java | 9 +- .../model/domain/token/AllocationToken.java | 25 +- .../registry/model/host/HostHistory.java | 172 ++------- .../registry/model/ofy/ObjectifyService.java | 2 - .../registry/model/poll/PollMessage.java | 35 +- .../reporting/DomainTransactionRecord.java | 11 +- .../model/reporting/HistoryEntry.java | 357 +++++------------- .../model/reporting/HistoryEntryDao.java | 105 ++---- .../EppHistoryVKeyTranslatorFactory.java | 107 ------ .../persistence/DomainHistoryVKey.java | 57 --- .../registry/persistence/EppHistoryVKey.java | 106 ------ .../registry/rdap/RdapJsonFormatter.java | 76 ++-- .../registry/tools/DomainLockUtils.java | 8 +- .../tools/GetAllocationTokenCommand.java | 18 +- .../tools/GetHistoryEntriesCommand.java | 4 +- .../registry/tools/UnrenewDomainCommand.java | 4 +- ...CreateCancellationsForOneTimesCommand.java | 2 +- ...xpandRecurringBillingEventsActionTest.java | 9 +- .../WipeOutContactHistoryPiiActionTest.java | 19 +- .../registry/beam/rde/RdePipelineTest.java | 19 +- .../flows/domain/DomainCheckFlowTest.java | 6 +- .../flows/domain/DomainCreateFlowTest.java | 22 +- .../flows/domain/DomainDeleteFlowTest.java | 12 +- .../flows/domain/DomainPricingLogicTest.java | 2 +- .../flows/domain/DomainRenewFlowTest.java | 12 +- .../domain/DomainRestoreRequestFlowTest.java | 4 +- .../domain/DomainTransferApproveFlowTest.java | 16 +- .../domain/DomainTransferCancelFlowTest.java | 8 +- .../domain/DomainTransferRejectFlowTest.java | 10 +- .../domain/DomainTransferRequestFlowTest.java | 13 +- .../token/AllocationTokenFlowUtilsTest.java | 11 +- .../registry/model/OteStatsTestHelper.java | 30 +- .../registry/model/SchemaVersionTest.java | 2 + .../model/billing/BillingEventTest.java | 6 +- .../model/common/ClassPathManagerTest.java | 3 - .../registry/model/domain/DomainSqlTest.java | 140 +------ .../registry/model/domain/DomainTest.java | 12 +- .../model/domain/GracePeriodTest.java | 4 +- .../domain/token/AllocationTokenTest.java | 15 +- .../model/history/ContactHistoryTest.java | 29 +- .../model/history/DomainHistoryTest.java | 22 +- .../model/history/HostHistoryTest.java | 29 +- .../registry/model/poll/PollMessageTest.java | 6 +- .../model/reporting/HistoryEntryDaoTest.java | 5 +- .../model/reporting/HistoryEntryTest.java | 35 +- .../EppHistoryVKeyTranslatorFactoryTest.java | 36 -- .../VKeyTranslatorFactoryTest.java | 15 - .../persistence/DomainHistoryVKeyTest.java | 89 ----- .../rde/DomainToXjcConverterTest.java | 2 +- .../java/google/registry/rde/RdeFixtures.java | 4 +- .../registry/testing/DatabaseHelper.java | 23 +- .../testing/FullFieldsTestEntityHelper.java | 26 +- .../registry/testing/HistoryEntrySubject.java | 25 +- .../google/registry/testing/UserInfo.java | 2 +- .../tools/AckPollMessagesCommandTest.java | 6 +- .../DeleteAllocationTokensCommandTest.java | 7 +- .../GenerateAllocationTokensCommandTest.java | 9 +- .../tools/GetAllocationTokenCommandTest.java | 6 +- .../tools/UnrenewDomainCommandTest.java | 2 +- .../google/registry/model/schema.txt | 129 ------- .../sql/schema/db-schema.sql.generated | 8 +- docs/flows.md | 10 +- 88 files changed, 845 insertions(+), 2213 deletions(-) delete mode 100644 core/src/main/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactory.java delete mode 100644 core/src/main/java/google/registry/persistence/DomainHistoryVKey.java delete mode 100644 core/src/main/java/google/registry/persistence/EppHistoryVKey.java delete mode 100644 core/src/test/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactoryTest.java delete mode 100644 core/src/test/java/google/registry/persistence/DomainHistoryVKeyTest.java diff --git a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java index 872b10def..3d17d6443 100644 --- a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java +++ b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java @@ -101,15 +101,20 @@ public class DeleteProberDataAction implements Runnable { @Inject DnsQueue dnsQueue; - @Inject @Parameter(PARAM_DRY_RUN) boolean isDryRun; + @Inject + @Parameter(PARAM_DRY_RUN) + boolean isDryRun; /** List of TLDs to work on. If empty - will work on all TLDs that end with .test. */ - @Inject @Parameter(PARAM_TLDS) ImmutableSet tlds; + @Inject + @Parameter(PARAM_TLDS) + ImmutableSet tlds; @Inject @Config("registryAdminClientId") String registryAdminRegistrarId; - @Inject DeleteProberDataAction() {} + @Inject + DeleteProberDataAction() {} @Override public void run() { @@ -151,7 +156,7 @@ public class DeleteProberDataAction implements Runnable { DateTime now = tm().getTransactionTime(); // Scroll through domains, soft-deleting as necessary (very few will be soft-deleted) and // keeping track of which domains to hard-delete (there can be many, so we batch them up) - ScrollableResults scrollableResult = + try (ScrollableResults scrollableResult = jpaTm() .query(DOMAIN_QUERY_STRING, Domain.class) .setParameter("tlds", deletableTlds) @@ -161,28 +166,30 @@ public class DeleteProberDataAction implements Runnable { .setParameter("nowAutoTimestamp", CreateAutoTimestamp.create(now)) .unwrap(Query.class) .setCacheMode(CacheMode.IGNORE) - .scroll(ScrollMode.FORWARD_ONLY); - ImmutableList.Builder domainRepoIdsToHardDelete = new ImmutableList.Builder<>(); - ImmutableList.Builder hostNamesToHardDelete = new ImmutableList.Builder<>(); - for (int i = 1; scrollableResult.next(); i = (i + 1) % BATCH_SIZE) { - Domain domain = (Domain) scrollableResult.get(0); - processDomain( - domain, - domainRepoIdsToHardDelete, - hostNamesToHardDelete, - softDeletedDomains, - hardDeletedDomains); - // Batch the deletion and DB flush + session clearing so we don't OOM - if (i == 0) { - hardDeleteDomainsAndHosts(domainRepoIdsToHardDelete.build(), hostNamesToHardDelete.build()); - domainRepoIdsToHardDelete = new ImmutableList.Builder<>(); - hostNamesToHardDelete = new ImmutableList.Builder<>(); - jpaTm().getEntityManager().flush(); - jpaTm().getEntityManager().clear(); + .scroll(ScrollMode.FORWARD_ONLY)) { + ImmutableList.Builder domainRepoIdsToHardDelete = new ImmutableList.Builder<>(); + ImmutableList.Builder hostNamesToHardDelete = new ImmutableList.Builder<>(); + for (int i = 1; scrollableResult.next(); i = (i + 1) % BATCH_SIZE) { + Domain domain = (Domain) scrollableResult.get(0); + processDomain( + domain, + domainRepoIdsToHardDelete, + hostNamesToHardDelete, + softDeletedDomains, + hardDeletedDomains); + // Batch the deletion and DB flush + session clearing, so we don't OOM + if (i == 0) { + hardDeleteDomainsAndHosts( + domainRepoIdsToHardDelete.build(), hostNamesToHardDelete.build()); + domainRepoIdsToHardDelete = new ImmutableList.Builder<>(); + hostNamesToHardDelete = new ImmutableList.Builder<>(); + jpaTm().getEntityManager().flush(); + jpaTm().getEntityManager().clear(); + } } + // process the remainder + hardDeleteDomainsAndHosts(domainRepoIdsToHardDelete.build(), hostNamesToHardDelete.build()); } - // process the remainder - hardDeleteDomainsAndHosts(domainRepoIdsToHardDelete.build(), hostNamesToHardDelete.build()); } private void processDomain( @@ -236,7 +243,7 @@ public class DeleteProberDataAction implements Runnable { .setParameter("repoIds", domainRepoIds) .executeUpdate(); jpaTm() - .query("DELETE FROM DomainHistory WHERE domainRepoId IN :repoIds") + .query("DELETE FROM DomainHistory WHERE repoId IN :repoIds") .setParameter("repoIds", domainRepoIds) .executeUpdate(); jpaTm() diff --git a/core/src/main/java/google/registry/beam/rde/RdePipeline.java b/core/src/main/java/google/registry/beam/rde/RdePipeline.java index 14d879bab..1233cd4c7 100644 --- a/core/src/main/java/google/registry/beam/rde/RdePipeline.java +++ b/core/src/main/java/google/registry/beam/rde/RdePipeline.java @@ -55,7 +55,7 @@ import google.registry.model.rde.RdeMode; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.Registrar.Type; import google.registry.model.reporting.HistoryEntry; -import google.registry.model.reporting.HistoryEntryDao; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.persistence.PersistenceModule.TransactionIsolationLevel; import google.registry.persistence.VKey; import google.registry.rde.DepositFragment; @@ -71,11 +71,9 @@ import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; -import java.lang.reflect.InvocationTargetException; import java.util.HashSet; import javax.inject.Inject; import javax.inject.Singleton; -import javax.persistence.IdClass; import org.apache.beam.sdk.Pipeline; import org.apache.beam.sdk.PipelineResult; import org.apache.beam.sdk.coders.KvCoder; @@ -172,6 +170,7 @@ import org.joda.time.DateTime; * @see Using * Flex Templates */ +@SuppressWarnings("ALL") @Singleton public class RdePipeline implements Serializable { @@ -191,16 +190,6 @@ public class RdePipeline implements Serializable { private static final ImmutableSet IGNORED_REGISTRAR_TYPES = Sets.immutableEnumSet(Registrar.Type.MONITORING, Registrar.Type.TEST); - // The field name of the EPP resource embedded in its corresponding history entry. - private static final ImmutableMap, String> EPP_RESOURCE_FIELD_NAME = - ImmutableMap.of( - DomainHistory.class, - "domainBase", - ContactHistory.class, - "contactBase", - HostHistory.class, - "hostBase"); - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); @Inject @@ -337,26 +326,20 @@ public class RdePipeline implements Serializable { */ private PCollection> getMostRecentHistoryEntries( Pipeline pipeline, Class historyClass) { - String repoIdFieldName = HistoryEntryDao.REPO_ID_FIELD_NAMES.get(historyClass); - String resourceFieldName = EPP_RESOURCE_FIELD_NAME.get(historyClass); return pipeline.apply( String.format("Load most recent %s", historyClass.getSimpleName()), RegistryJpaIO.read( - ("SELECT %repoIdField%, id FROM %entity% WHERE (%repoIdField%, modificationTime)" - + " IN (SELECT %repoIdField%, MAX(modificationTime) FROM %entity% WHERE" - + " modificationTime <= :watermark GROUP BY %repoIdField%) AND" - + " %resourceField%.deletionTime > :watermark AND" - + " COALESCE(%resourceField%.creationClientId, '') NOT LIKE 'prober-%' AND" - + " COALESCE(%resourceField%.currentSponsorClientId, '') NOT LIKE 'prober-%'" - + " AND COALESCE(%resourceField%.lastEppUpdateClientId, '') NOT LIKE" + ("SELECT repoId, revisionId FROM %entity% WHERE (repoId, modificationTime) IN" + + " (SELECT repoId, MAX(modificationTime) FROM %entity% WHERE" + + " modificationTime <= :watermark GROUP BY repoId) AND resource.deletionTime" + + " > :watermark AND COALESCE(resource.creationClientId, '') NOT LIKE" + + " 'prober-%' AND COALESCE(resource.currentSponsorClientId, '') NOT LIKE" + + " 'prober-%' AND COALESCE(resource.lastEppUpdateClientId, '') NOT LIKE" + " 'prober-%' " + (historyClass == DomainHistory.class - ? "AND %resourceField%.tld IN " - + "(SELECT id FROM Tld WHERE tldType = 'REAL')" + ? "AND resource.tld IN " + "(SELECT id FROM Tld WHERE tldType = 'REAL')" : "")) - .replace("%entity%", historyClass.getSimpleName()) - .replace("%repoIdField%", repoIdFieldName) - .replace("%resourceField%", resourceFieldName), + .replace("%entity%", historyClass.getSimpleName()), ImmutableMap.of("watermark", watermark), Object[].class, row -> KV.of((String) row[0], (long) row[1])) @@ -380,38 +363,28 @@ public class RdePipeline implements Serializable { checkState( dedupedIds.size() == 1, "Multiple unique revision IDs detected for %s repo ID %s: %s", - EPP_RESOURCE_FIELD_NAME.get(historyEntryClazz), + historyEntryClazz.getSimpleName(), repoId, ids); logger.atSevere().log( "Duplicate revision IDs detected for %s repo ID %s: %s", - EPP_RESOURCE_FIELD_NAME.get(historyEntryClazz), repoId, ids); + historyEntryClazz.getSimpleName(), repoId, ids); } return loadResourceByHistoryEntryId(historyEntryClazz, repoId, ids.get(0)); } private EppResource loadResourceByHistoryEntryId( Class historyEntryClazz, String repoId, long revisionId) { - try { - Class idClazz = historyEntryClazz.getAnnotation(IdClass.class).value(); - Serializable idObject = - (Serializable) - idClazz.getConstructor(String.class, long.class).newInstance(repoId, revisionId); - return jpaTm() - .transact(() -> jpaTm().loadByKey(VKey.createSql(historyEntryClazz, idObject))) - .getResourceAtPointInTime() - .map(resource -> resource.cloneProjectedAtTime(watermark)) - .get(); - } catch (NoSuchMethodException - | InvocationTargetException - | InstantiationException - | IllegalAccessException e) { - throw new RuntimeException( - String.format( - "Cannot load resource from %s with repoId %s and revisionId %s", - historyEntryClazz.getSimpleName(), repoId, revisionId), - e); - } + + return jpaTm() + .transact( + () -> + jpaTm() + .loadByKey( + VKey.createSql(historyEntryClazz, new HistoryEntryId(repoId, revisionId)))) + .getResourceAtPointInTime() + .map(resource -> resource.cloneProjectedAtTime(watermark)) + .get(); } /** diff --git a/core/src/main/java/google/registry/flows/FlowUtils.java b/core/src/main/java/google/registry/flows/FlowUtils.java index 49aa308eb..e0a32333d 100644 --- a/core/src/main/java/google/registry/flows/FlowUtils.java +++ b/core/src/main/java/google/registry/flows/FlowUtils.java @@ -23,7 +23,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.Throwables; import com.google.common.flogger.FluentLogger; -import com.googlecode.objectify.Key; import google.registry.flows.EppException.CommandUseErrorException; import google.registry.flows.EppException.ParameterValueRangeErrorException; import google.registry.flows.EppException.SyntaxErrorException; @@ -34,7 +33,7 @@ import google.registry.model.eppcommon.EppXmlTransformer; import google.registry.model.eppinput.EppInput.WrongProtocolVersionException; import google.registry.model.eppoutput.EppOutput; import google.registry.model.host.InetAddressAdapter.IpVersionMismatchException; -import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.translators.CurrencyUnitAdapter.UnknownCurrencyException; import google.registry.xml.XmlException; import java.util.List; @@ -103,9 +102,8 @@ public final class FlowUtils { } } - public static Key createHistoryKey( - EppResource parent, Class clazz) { - return Key.create(Key.create(parent), clazz, allocateId()); + public static HistoryEntryId createHistoryEntryId(EppResource parent) { + return new HistoryEntryId(parent.getRepoId(), allocateId()); } /** Registrar is not logged in. */ @@ -118,7 +116,7 @@ public final class FlowUtils { /** IP address version mismatch. */ public static class IpAddressVersionMismatchException extends ParameterValueRangeErrorException { public IpAddressVersionMismatchException() { - super("IP adddress version mismatch"); + super("IP address version mismatch"); } } 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 16049b344..b90a130e6 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactFlowUtils.java +++ b/core/src/main/java/google/registry/flows/contact/ContactFlowUtils.java @@ -20,17 +20,15 @@ import com.google.common.base.CharMatcher; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.Sets; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.EppException.ParameterValuePolicyErrorException; import google.registry.flows.EppException.ParameterValueSyntaxErrorException; import google.registry.model.contact.Contact; import google.registry.model.contact.ContactAddress; -import google.registry.model.contact.ContactHistory; -import google.registry.model.contact.ContactHistory.ContactHistoryId; import google.registry.model.contact.PostalInfo; import google.registry.model.poll.PendingActionNotificationResponse.ContactPendingActionNotificationResponse; import google.registry.model.poll.PollMessage; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.transfer.TransferData; import google.registry.model.transfer.TransferResponse.ContactTransferResponse; import java.util.Set; @@ -69,10 +67,7 @@ public class ContactFlowUtils { /** Create a poll message for the gaining client in a transfer. */ static PollMessage createGainingTransferPollMessage( - String targetId, - TransferData transferData, - DateTime now, - Key contactHistoryKey) { + String targetId, TransferData transferData, DateTime now, HistoryEntryId contactHistoryId) { return new PollMessage.OneTime.Builder() .setRegistrarId(transferData.getGainingRegistrarId()) .setEventTime(transferData.getPendingTransferExpirationTime()) @@ -85,23 +80,19 @@ public class ContactFlowUtils { transferData.getTransferStatus().isApproved(), transferData.getTransferRequestTrid(), now))) - .setContactHistoryId( - new ContactHistoryId( - contactHistoryKey.getParent().getName(), contactHistoryKey.getId())) + .setContactHistoryId(contactHistoryId) .build(); } /** Create a poll message for the losing client in a transfer. */ static PollMessage createLosingTransferPollMessage( - String targetId, TransferData transferData, Key contactHistoryKey) { + String targetId, TransferData transferData, HistoryEntryId contactHistoryId) { return new PollMessage.OneTime.Builder() .setRegistrarId(transferData.getLosingRegistrarId()) .setEventTime(transferData.getPendingTransferExpirationTime()) .setMsg(transferData.getTransferStatus().getMessage()) .setResponseData(ImmutableList.of(createTransferResponse(targetId, transferData))) - .setContactHistoryId( - new ContactHistoryId( - contactHistoryKey.getParent().getName(), contactHistoryKey.getId())) + .setContactHistoryId(contactHistoryId) .build(); } diff --git a/core/src/main/java/google/registry/flows/contact/ContactTransferApproveFlow.java b/core/src/main/java/google/registry/flows/contact/ContactTransferApproveFlow.java index 05e4c05a4..8b4ad9bc8 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactTransferApproveFlow.java +++ b/core/src/main/java/google/registry/flows/contact/ContactTransferApproveFlow.java @@ -26,7 +26,6 @@ import static google.registry.model.reporting.HistoryEntry.Type.CONTACT_TRANSFER import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; import google.registry.flows.FlowModule.RegistrarId; @@ -93,7 +92,7 @@ public final class ContactTransferApproveFlow implements TransactionalFlow { // Create a poll message for the gaining client. PollMessage gainingPollMessage = createGainingTransferPollMessage( - targetId, newContact.getTransferData(), now, Key.create(contactHistory)); + targetId, newContact.getTransferData(), now, contactHistory.getHistoryEntryId()); tm().insertAll(ImmutableSet.of(contactHistory, gainingPollMessage)); tm().update(newContact); // Delete the billing event and poll messages that were written in case the transfer would have diff --git a/core/src/main/java/google/registry/flows/contact/ContactTransferCancelFlow.java b/core/src/main/java/google/registry/flows/contact/ContactTransferCancelFlow.java index 15f38265a..4d3f79f38 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactTransferCancelFlow.java +++ b/core/src/main/java/google/registry/flows/contact/ContactTransferCancelFlow.java @@ -26,7 +26,6 @@ import static google.registry.model.reporting.HistoryEntry.Type.CONTACT_TRANSFER import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; import google.registry.flows.FlowModule.RegistrarId; @@ -73,7 +72,7 @@ public final class ContactTransferCancelFlow implements TransactionalFlow { @Inject ContactTransferCancelFlow() {} @Override - public final EppResponse run() throws EppException { + public EppResponse run() throws EppException { extensionManager.register(MetadataExtension.class); validateRegistrarIsLoggedIn(registrarId); extensionManager.validate(); @@ -89,7 +88,7 @@ public final class ContactTransferCancelFlow implements TransactionalFlow { // Create a poll message for the losing client. PollMessage losingPollMessage = createLosingTransferPollMessage( - targetId, newContact.getTransferData(), Key.create(contactHistory)); + targetId, newContact.getTransferData(), contactHistory.getHistoryEntryId()); tm().insertAll(ImmutableSet.of(contactHistory, losingPollMessage)); tm().update(newContact); // Delete the billing event and poll messages that were written in case the transfer would have diff --git a/core/src/main/java/google/registry/flows/contact/ContactTransferRejectFlow.java b/core/src/main/java/google/registry/flows/contact/ContactTransferRejectFlow.java index 5c801e474..b1c6968fa 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactTransferRejectFlow.java +++ b/core/src/main/java/google/registry/flows/contact/ContactTransferRejectFlow.java @@ -26,7 +26,6 @@ import static google.registry.model.reporting.HistoryEntry.Type.CONTACT_TRANSFER import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; import google.registry.flows.FlowModule.RegistrarId; @@ -86,7 +85,7 @@ public final class ContactTransferRejectFlow implements TransactionalFlow { historyBuilder.setType(CONTACT_TRANSFER_REJECT).setContact(newContact).build(); PollMessage gainingPollMessage = createGainingTransferPollMessage( - targetId, newContact.getTransferData(), now, Key.create(contactHistory)); + targetId, newContact.getTransferData(), now, contactHistory.getHistoryEntryId()); tm().insertAll(ImmutableSet.of(contactHistory, gainingPollMessage)); tm().update(newContact); // Delete the billing event and poll messages that were written in case the transfer would have 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 65b3dda25..0817140f9 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactTransferRequestFlow.java +++ b/core/src/main/java/google/registry/flows/contact/ContactTransferRequestFlow.java @@ -14,7 +14,7 @@ package google.registry.flows.contact; -import static google.registry.flows.FlowUtils.createHistoryKey; +import static google.registry.flows.FlowUtils.createHistoryEntryId; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; import static google.registry.flows.ResourceFlowUtils.verifyAuthInfo; @@ -28,7 +28,6 @@ import static google.registry.model.reporting.HistoryEntry.Type.CONTACT_TRANSFER import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.config.RegistryConfig.Config; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; @@ -46,6 +45,7 @@ import google.registry.model.eppcommon.StatusValue; import google.registry.model.eppcommon.Trid; import google.registry.model.eppoutput.EppResponse; import google.registry.model.poll.PollMessage; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.model.transfer.ContactTransferData; import google.registry.model.transfer.TransferStatus; @@ -74,21 +74,27 @@ import org.joda.time.Duration; @ReportingSpec(ActivityReportField.CONTACT_TRANSFER_REQUEST) public final class ContactTransferRequestFlow implements TransactionalFlow { - private static final ImmutableSet DISALLOWED_STATUSES = ImmutableSet.of( - StatusValue.CLIENT_TRANSFER_PROHIBITED, - StatusValue.PENDING_DELETE, - StatusValue.SERVER_TRANSFER_PROHIBITED); + private static final ImmutableSet DISALLOWED_STATUSES = + ImmutableSet.of( + StatusValue.CLIENT_TRANSFER_PROHIBITED, + StatusValue.PENDING_DELETE, + StatusValue.SERVER_TRANSFER_PROHIBITED); @Inject ExtensionManager extensionManager; @Inject Optional authInfo; @Inject @RegistrarId String gainingClientId; @Inject @TargetId String targetId; - @Inject @Config("contactAutomaticTransferLength") Duration automaticTransferLength; + + @Inject + @Config("contactAutomaticTransferLength") + Duration automaticTransferLength; @Inject ContactHistory.Builder historyBuilder; @Inject Trid trid; @Inject EppResponse.Builder responseBuilder; - @Inject ContactTransferRequestFlow() {} + + @Inject + ContactTransferRequestFlow() {} @Override public EppResponse run() throws EppException { @@ -120,29 +126,31 @@ public final class ContactTransferRequestFlow implements TransactionalFlow { .setPendingTransferExpirationTime(transferExpirationTime) .setTransferStatus(TransferStatus.SERVER_APPROVED) .build(); - Key contactHistoryKey = createHistoryKey(existingContact, ContactHistory.class); - historyBuilder.setId(contactHistoryKey.getId()).setType(CONTACT_TRANSFER_REQUEST); + HistoryEntryId contactHistoryId = createHistoryEntryId(existingContact); + historyBuilder + .setRevisionId(contactHistoryId.getRevisionId()) + .setType(CONTACT_TRANSFER_REQUEST); // If the transfer is server approved, this message will be sent to the losing registrar. */ PollMessage serverApproveLosingPollMessage = - createLosingTransferPollMessage(targetId, serverApproveTransferData, contactHistoryKey); + createLosingTransferPollMessage(targetId, serverApproveTransferData, contactHistoryId); // If the transfer is server approved, this message will be sent to the gaining registrar. */ PollMessage serverApproveGainingPollMessage = createGainingTransferPollMessage( - targetId, serverApproveTransferData, now, contactHistoryKey); + targetId, serverApproveTransferData, now, contactHistoryId); ContactTransferData pendingTransferData = serverApproveTransferData .asBuilder() .setTransferStatus(TransferStatus.PENDING) .setServerApproveEntities( serverApproveGainingPollMessage.getContactRepoId(), - contactHistoryKey.getId(), + contactHistoryId.getRevisionId(), ImmutableSet.of( serverApproveGainingPollMessage.createVKey(), serverApproveLosingPollMessage.createVKey())) .build(); // When a transfer is requested, a poll message is created to notify the losing registrar. PollMessage requestPollMessage = - createLosingTransferPollMessage(targetId, pendingTransferData, contactHistoryKey) + createLosingTransferPollMessage(targetId, pendingTransferData, contactHistoryId) .asBuilder() .setEventTime(now) // Unlike the serverApprove messages, this applies immediately. .build(); @@ -165,4 +173,3 @@ public final class ContactTransferRequestFlow implements TransactionalFlow { .build(); } } - 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 9b7180a0a..dcb67abeb 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java @@ -86,7 +86,6 @@ import google.registry.model.domain.Domain; 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; @@ -110,6 +109,7 @@ import google.registry.model.poll.PollMessage.Autorenew; import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.model.tld.Registry; import google.registry.model.tld.Registry.TldState; @@ -327,14 +327,14 @@ public final class DomainCreateFlow implements TransactionalFlow { FeesAndCredits feesAndCredits = pricingLogic.getCreatePrice( registry, targetId, now, years, isAnchorTenant, allocationToken); - validateFeeChallenge(targetId, now, feeCreate, feesAndCredits); + validateFeeChallenge(feeCreate, feesAndCredits); Optional secDnsCreate = validateSecDnsExtension(eppInput.getSingleExtension(SecDnsCreateExtension.class)); DateTime registrationExpirationTime = leapSafeAddYears(now, years); String repoId = createDomainRepoId(allocateId(), registry.getTldStr()); long historyRevisionId = allocateId(); - DomainHistoryId domainHistoryId = new DomainHistoryId(repoId, historyRevisionId); - historyBuilder.setId(historyRevisionId); + HistoryEntryId domainHistoryId = new HistoryEntryId(repoId, historyRevisionId); + historyBuilder.setRevisionId(historyRevisionId); // Bill for the create. BillingEvent.OneTime createBillingEvent = createOneTimeBillingEvent( @@ -406,7 +406,8 @@ public final class DomainCreateFlow implements TransactionalFlow { if (allocationToken.isPresent() && TokenType.SINGLE_USE.equals(allocationToken.get().getTokenType())) { entitiesToSave.add( - allocationTokenFlowUtils.redeemToken(allocationToken.get(), domainHistory.createVKey())); + allocationTokenFlowUtils.redeemToken( + allocationToken.get(), domainHistory.getHistoryEntryId())); } enqueueTasks(domain, hasSignedMarks, hasClaimsNotice); @@ -562,7 +563,7 @@ public final class DomainCreateFlow implements TransactionalFlow { boolean isReserved, int years, FeesAndCredits feesAndCredits, - DomainHistoryId domainHistoryId, + HistoryEntryId domainHistoryId, Optional allocationToken, DateTime now) { ImmutableSet.Builder flagsBuilder = new ImmutableSet.Builder<>(); @@ -596,7 +597,7 @@ public final class DomainCreateFlow implements TransactionalFlow { } private Recurring createAutorenewBillingEvent( - DomainHistoryId domainHistoryId, + HistoryEntryId domainHistoryId, DateTime registrationExpirationTime, RenewalPriceInfo renewalpriceInfo) { return new BillingEvent.Recurring.Builder() @@ -613,7 +614,7 @@ public final class DomainCreateFlow implements TransactionalFlow { } private Autorenew createAutorenewPollMessage( - DomainHistoryId domainHistoryId, DateTime registrationExpirationTime) { + HistoryEntryId domainHistoryId, DateTime registrationExpirationTime) { return new PollMessage.Autorenew.Builder() .setTargetId(targetId) .setRegistrarId(registrarId) @@ -634,7 +635,7 @@ public final class DomainCreateFlow implements TransactionalFlow { .setEventTime(createBillingEvent.getEventTime()) .setBillingTime(createBillingEvent.getBillingTime()) .setFlags(createBillingEvent.getFlags()) - .setDomainHistoryId(createBillingEvent.getDomainHistoryId()) + .setDomainHistoryId(createBillingEvent.getHistoryEntryId()) .build(); } @@ -674,11 +675,11 @@ public final class DomainCreateFlow implements TransactionalFlow { Optional allocationToken, FeesAndCredits feesAndCredits) { if (isAnchorTenant) { - if (allocationToken.isPresent()) { - checkArgument( - allocationToken.get().getRenewalPriceBehavior() != RenewalPriceBehavior.SPECIFIED, - "Renewal price behavior cannot be SPECIFIED for anchor tenant"); - } + allocationToken.ifPresent( + token -> + checkArgument( + token.getRenewalPriceBehavior() != RenewalPriceBehavior.SPECIFIED, + "Renewal price behavior cannot be SPECIFIED for anchor tenant")); return RenewalPriceInfo.create(RenewalPriceBehavior.NONPREMIUM, null); } else if (allocationToken.isPresent() && allocationToken.get().getRenewalPriceBehavior() == RenewalPriceBehavior.SPECIFIED) { @@ -705,9 +706,12 @@ public final class DomainCreateFlow implements TransactionalFlow { private static ImmutableList createResponseExtensions( Optional feeCreate, FeesAndCredits feesAndCredits) { - return feeCreate.isPresent() - ? ImmutableList.of(createFeeCreateResponse(feeCreate.get(), feesAndCredits)) - : ImmutableList.of(); + return feeCreate + .map( + feeCreateCommandExtension -> + ImmutableList.of( + createFeeCreateResponse(feeCreateCommandExtension, feesAndCredits))) + .orElseGet(ImmutableList::of); } /** Signed marks are only allowed during sunrise. */ 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 e13b7af19..3d6bc07c6 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java @@ -16,7 +16,7 @@ package google.registry.flows.domain; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Strings.isNullOrEmpty; -import static google.registry.flows.FlowUtils.createHistoryKey; +import static google.registry.flows.FlowUtils.createHistoryEntryId; import static google.registry.flows.FlowUtils.persistEntityChanges; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; @@ -43,7 +43,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Sets; -import com.googlecode.objectify.Key; import google.registry.batch.AsyncTaskEnqueuer; import google.registry.dns.DnsQueue; import google.registry.flows.EppException; @@ -66,7 +65,6 @@ import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Recurring; import google.registry.model.domain.Domain; 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; @@ -88,6 +86,7 @@ import google.registry.model.poll.PendingActionNotificationResponse.DomainPendin import google.registry.model.poll.PollMessage; import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.model.tld.Registry; import google.registry.model.tld.Registry.TldType; @@ -181,8 +180,8 @@ public final class DomainDeleteFlow implements TransactionalFlow { ? Duration.ZERO // By default, this should be 30 days of grace, and 5 days of pending delete. : redemptionGracePeriodLength.plus(pendingDeleteLength); - Key domainHistoryKey = createHistoryKey(existingDomain, DomainHistory.class); - historyBuilder.setId(domainHistoryKey.getId()); + HistoryEntryId domainHistoryId = createHistoryEntryId(existingDomain); + historyBuilder.setRevisionId(domainHistoryId.getRevisionId()); DateTime deletionTime = now.plus(durationUntilDelete); if (durationUntilDelete.equals(Duration.ZERO)) { builder.setDeletionTime(now).setStatusValues(null); @@ -214,7 +213,7 @@ public final class DomainDeleteFlow implements TransactionalFlow { // it is synchronous). if (durationUntilDelete.isLongerThan(Duration.ZERO) || isSuperuser) { PollMessage.OneTime deletePollMessage = - createDeletePollMessage(existingDomain, domainHistoryKey, deletionTime); + createDeletePollMessage(existingDomain, domainHistoryId, deletionTime); entitiesToSave.add(deletePollMessage); builder.setDeletePollMessage(deletePollMessage.createVKey()); } @@ -224,7 +223,7 @@ public final class DomainDeleteFlow implements TransactionalFlow { if (durationUntilDelete.isLongerThan(Duration.ZERO) && !registrarId.equals(existingDomain.getPersistedCurrentSponsorRegistrarId())) { entitiesToSave.add( - createImmediateDeletePollMessage(existingDomain, domainHistoryKey, now, deletionTime)); + createImmediateDeletePollMessage(existingDomain, domainHistoryId, now, deletionTime)); } // Cancel any grace periods that were still active, and set the expiration time accordingly. @@ -233,14 +232,9 @@ public final class DomainDeleteFlow implements TransactionalFlow { // No cancellation is written if the grace period was not for a billable event. if (gracePeriod.hasBillingEvent()) { entitiesToSave.add( - BillingEvent.Cancellation.forGracePeriod( - gracePeriod, - now, - new DomainHistoryId( - domainHistoryKey.getParent().getName(), domainHistoryKey.getId()), - targetId)); + BillingEvent.Cancellation.forGracePeriod(gracePeriod, now, domainHistoryId, targetId)); if (gracePeriod.getOneTimeBillingEvent() != null) { - // Take the amount of amount of registration time being refunded off the expiration time. + // Take the amount of registration time being refunded off the expiration time. // This can be either add grace periods or renew grace periods. BillingEvent.OneTime oneTime = tm().loadByKey(gracePeriod.getOneTimeBillingEvent()); newExpirationTime = newExpirationTime.minusYears(oneTime.getPeriodYears()); @@ -262,7 +256,7 @@ public final class DomainDeleteFlow implements TransactionalFlow { Recurring existingRecurring = tm().loadByKey(existingDomain.getAutorenewBillingEvent()); BillingEvent.Recurring recurringBillingEvent = updateAutorenewRecurrenceEndTime( - existingDomain, existingRecurring, now, domainHistory.getDomainHistoryId()); + existingDomain, existingRecurring, now, domainHistory.getHistoryEntryId()); // If there's a pending transfer, the gaining client's autorenew billing // event and poll message will already have been deleted in // ResourceDeleteFlow since it's listed in serverApproveEntities. @@ -343,7 +337,7 @@ public final class DomainDeleteFlow implements TransactionalFlow { } private PollMessage.OneTime createDeletePollMessage( - Domain existingDomain, Key domainHistoryKey, DateTime deletionTime) { + Domain existingDomain, HistoryEntryId domainHistoryId, DateTime deletionTime) { Optional metadataExtension = eppInput.getSingleExtension(MetadataExtension.class); boolean hasMetadataMessage = @@ -362,21 +356,16 @@ public final class DomainDeleteFlow implements TransactionalFlow { ImmutableList.of( DomainPendingActionNotificationResponse.create( existingDomain.getDomainName(), true, trid, deletionTime))) - .setDomainHistoryId( - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build(); } private PollMessage.OneTime createImmediateDeletePollMessage( - Domain existingDomain, - Key domainHistoryKey, - DateTime now, - DateTime deletionTime) { + Domain existingDomain, HistoryEntryId domainHistoryId, DateTime now, DateTime deletionTime) { return new PollMessage.OneTime.Builder() .setRegistrarId(existingDomain.getPersistedCurrentSponsorRegistrarId()) .setEventTime(now) - .setDomainHistoryId( - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .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 240e06cd4..0ded6c8f0 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java @@ -88,7 +88,6 @@ 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; @@ -121,7 +120,7 @@ import google.registry.model.registrar.Registrar; import google.registry.model.registrar.Registrar.State; import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; -import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.tld.Registry; import google.registry.model.tld.Registry.TldState; import google.registry.model.tld.Registry.TldType; @@ -429,7 +428,7 @@ public class DomainFlowUtils { contacts.stream() .collect( toImmutableSetMultimap( - DesignatedContact::getType, contact -> contact.getContactKey())); + DesignatedContact::getType, DesignatedContact::getContactKey)); // If any contact type has multiple contacts: if (contactsByType.asMap().values().stream().anyMatch(v -> v.size() > 1)) { @@ -590,7 +589,7 @@ public class DomainFlowUtils { Domain domain, Recurring existingRecurring, DateTime newEndTime, - @Nullable DomainHistoryId historyId) { + @Nullable HistoryEntryId historyId) { Optional autorenewPollMessage = tm().loadByKeyIfPresent(domain.getAutorenewPollMessage()); @@ -773,8 +772,6 @@ public class DomainFlowUtils { * domain names. */ public static void validateFeeChallenge( - String domainName, - DateTime priceTime, final Optional feeCommand, FeesAndCredits feesAndCredits) throws EppException { @@ -1135,9 +1132,9 @@ public class DomainFlowUtils { Duration maxSearchPeriod, final ImmutableSet cancelableFields) { - List recentHistoryEntries = + List recentHistoryEntries = findRecentHistoryEntries(domain, now, maxSearchPeriod); - Optional entryToCancel = + Optional entryToCancel = Streams.findLast( recentHistoryEntries.stream() .filter( @@ -1174,11 +1171,11 @@ public class DomainFlowUtils { return recordsBuilder.build(); } - private static List findRecentHistoryEntries( + private static List findRecentHistoryEntries( Domain domain, DateTime now, Duration maxSearchPeriod) { return jpaTm() .query( - "FROM DomainHistory WHERE modificationTime >= :beginning AND domainRepoId = " + "FROM DomainHistory WHERE modificationTime >= :beginning AND repoId = " + ":repoId ORDER BY modificationTime ASC", DomainHistory.class) .setParameter("beginning", now.minus(maxSearchPeriod)) 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 ade96a4d4..916cec59f 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java @@ -14,7 +14,7 @@ package google.registry.flows.domain; -import static google.registry.flows.FlowUtils.createHistoryKey; +import static google.registry.flows.FlowUtils.createHistoryEntryId; import static google.registry.flows.FlowUtils.persistEntityChanges; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; @@ -38,7 +38,6 @@ import static google.registry.util.DateTimeUtils.leapSafeAddYears; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.EppException.ParameterValueRangeErrorException; import google.registry.flows.ExtensionManager; @@ -62,7 +61,6 @@ import google.registry.model.billing.BillingEvent.Recurring; import google.registry.model.domain.Domain; 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; @@ -83,6 +81,7 @@ import google.registry.model.eppoutput.EppResponse; import google.registry.model.poll.PollMessage; import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.model.tld.Registry; import java.util.Optional; @@ -100,7 +99,7 @@ import org.joda.time.Duration; * *

ICANN prohibits any registration from being longer than ten years so if the request would * result in a registration greater than ten years long it will fail. In practice this means it's - * impossible to request a ten year renewal, since that will always cause the new registration to be + * impossible to request a ten-year renewal, since that will always cause the new registration to be * longer than 10 years unless it comes in at the exact millisecond that the domain would have * expired. * @@ -200,44 +199,36 @@ public final class DomainRenewFlow implements TransactionalFlow { now, years, existingRecurringBillingEvent); - validateFeeChallenge(targetId, now, feeRenew, feesAndCredits); + validateFeeChallenge(feeRenew, feesAndCredits); flowCustomLogic.afterValidation( AfterValidationParameters.newBuilder() .setExistingDomain(existingDomain) .setNow(now) .setYears(years) .build()); - Key domainHistoryKey = createHistoryKey(existingDomain, DomainHistory.class); - historyBuilder.setId(domainHistoryKey.getId()); + HistoryEntryId domainHistoryId = createHistoryEntryId(existingDomain); + historyBuilder.setRevisionId(domainHistoryId.getRevisionId()); String tld = existingDomain.getTld(); // Bill for this explicit renew itself. BillingEvent.OneTime explicitRenewEvent = createRenewBillingEvent( - tld, feesAndCredits.getTotalCost(), years, domainHistoryKey, allocationToken, now); + tld, feesAndCredits.getTotalCost(), years, domainHistoryId, allocationToken, now); // Create a new autorenew billing event and poll message starting at the new expiration time. BillingEvent.Recurring newAutorenewEvent = newAutorenewBillingEvent(existingDomain) .setEventTime(newExpirationTime) .setRenewalPrice(existingRecurringBillingEvent.getRenewalPrice().orElse(null)) .setRenewalPriceBehavior(existingRecurringBillingEvent.getRenewalPriceBehavior()) - .setDomainHistoryId( - new DomainHistoryId( - domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build(); PollMessage.Autorenew newAutorenewPollMessage = newAutorenewPollMessage(existingDomain) .setEventTime(newExpirationTime) - .setDomainHistoryId( - new DomainHistoryId( - domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build(); // End the old autorenew billing event and poll message now. This may delete the poll message. Recurring existingRecurring = tm().loadByKey(existingDomain.getAutorenewBillingEvent()); - updateAutorenewRecurrenceEndTime( - existingDomain, - existingRecurring, - now, - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())); + updateAutorenewRecurrenceEndTime(existingDomain, existingRecurring, now, domainHistoryId); Domain newDomain = existingDomain .asBuilder() @@ -260,7 +251,8 @@ public final class DomainRenewFlow implements TransactionalFlow { if (allocationToken.isPresent() && TokenType.SINGLE_USE.equals(allocationToken.get().getTokenType())) { entitiesToSave.add( - allocationTokenFlowUtils.redeemToken(allocationToken.get(), domainHistory.createVKey())); + allocationTokenFlowUtils.redeemToken( + allocationToken.get(), domainHistory.getHistoryEntryId())); } EntityChanges entityChanges = flowCustomLogic.beforeSave( @@ -339,7 +331,7 @@ public final class DomainRenewFlow implements TransactionalFlow { String tld, Money renewCost, int years, - Key domainHistoryKey, + HistoryEntryId domainHistoryId, Optional allocationToken, DateTime now) { return new BillingEvent.OneTime.Builder() @@ -355,27 +347,27 @@ public final class DomainRenewFlow implements TransactionalFlow { .map(AllocationToken::createVKey) .orElse(null)) .setBillingTime(now.plus(Registry.get(tld).getRenewGracePeriodLength())) - .setDomainHistoryId( - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build(); } private ImmutableList createResponseExtensions( FeesAndCredits feesAndCredits, Optional feeRenew) { - return feeRenew.isPresent() - ? ImmutableList.of( - feeRenew - .get() - .createResponseBuilder() - .setCurrency(feesAndCredits.getCurrency()) - .setFees( - ImmutableList.of( - Fee.create( - feesAndCredits.getRenewCost().getAmount(), - FeeType.RENEW, - feesAndCredits.hasPremiumFeesOfType(FeeType.RENEW)))) - .build()) - : ImmutableList.of(); + return feeRenew + .map( + feeRenewCommandExtension -> + ImmutableList.of( + feeRenewCommandExtension + .createResponseBuilder() + .setCurrency(feesAndCredits.getCurrency()) + .setFees( + ImmutableList.of( + Fee.create( + feesAndCredits.getRenewCost().getAmount(), + FeeType.RENEW, + feesAndCredits.hasPremiumFeesOfType(FeeType.RENEW)))) + .build())) + .orElseGet(ImmutableList::of); } /** The current expiration date is incorrect. */ 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 f6119253e..75d99eeb6 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java @@ -14,7 +14,7 @@ package google.registry.flows.domain; -import static google.registry.flows.FlowUtils.createHistoryKey; +import static google.registry.flows.FlowUtils.createHistoryEntryId; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo; @@ -34,7 +34,6 @@ import static google.registry.util.DateTimeUtils.END_OF_TIME; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.net.InternetDomainName; -import com.googlecode.objectify.Key; import google.registry.dns.DnsQueue; import google.registry.flows.EppException; import google.registry.flows.EppException.CommandUseErrorException; @@ -52,7 +51,6 @@ import google.registry.model.billing.BillingEvent.Reason; import google.registry.model.domain.Domain; 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; @@ -67,6 +65,7 @@ import google.registry.model.eppoutput.EppResponse; import google.registry.model.poll.PollMessage; import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.model.tld.Registry; import java.util.Optional; @@ -85,12 +84,12 @@ import org.joda.time.DateTime; * *

This flow is called a restore "request" because technically it is only supposed to signal that * the registrar requests the restore, which the registry can choose to process or not based on a - * restore report that is submitted through an out of band process and details the request. However, - * in practice this flow does the restore immediately. This is allowable because all of the fields - * on a restore report are optional or have default values, and so by policy when the request comes - * in we consider it to have been accompanied by a default-initialized report which we auto-approve. + * restore report that is submitted through an out-of-band process and details the request. However, + * in practice this flow does the restore immediately. This is allowable because all the fields on a + * restore report are optional or have default values, and so by policy when the request comes in we + * consider it to have been accompanied by a default-initialized report which we auto-approve. * - *

Restores cost a fixed restore fee plus a one year renewal fee for the domain. The domain is + *

Restores cost a fixed restore fee plus a one-year renewal fee for the domain. The domain is * restored to a single year expiration starting at the restore time, regardless of what the * original expiration time was. * @@ -147,10 +146,8 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { Optional feeUpdate = eppInput.getSingleExtension(FeeUpdateCommandExtension.class); verifyRestoreAllowed(command, existingDomain, feeUpdate, feesAndCredits, now); - Key domainHistoryKey = createHistoryKey(existingDomain, DomainHistory.class); - historyBuilder.setId(domainHistoryKey.getId()); - DomainHistoryId domainHistoryId = - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId()); + HistoryEntryId domainHistoryId = createHistoryEntryId(existingDomain); + historyBuilder.setRevisionId(domainHistoryId.getRevisionId()); ImmutableSet.Builder entitiesToSave = new ImmutableSet.Builder<>(); DateTime newExpirationTime = @@ -175,9 +172,7 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { newAutorenewPollMessage(existingDomain) .setEventTime(newExpirationTime) .setAutorenewEndTime(END_OF_TIME) - .setDomainHistoryId( - new DomainHistoryId( - domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build(); Domain newDomain = performRestore( @@ -231,7 +226,7 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { if (!existingDomain.getGracePeriodStatuses().contains(GracePeriodStatus.REDEMPTION)) { throw new DomainNotEligibleForRestoreException(); } - validateFeeChallenge(targetId, now, feeUpdate, feesAndCredits); + validateFeeChallenge(feeUpdate, feesAndCredits); } private static Domain performRestore( @@ -259,17 +254,17 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { } private OneTime createRenewBillingEvent( - DomainHistoryId domainHistoryId, Money renewCost, DateTime now) { + HistoryEntryId domainHistoryId, Money renewCost, DateTime now) { return prepareBillingEvent(domainHistoryId, renewCost, now).setReason(Reason.RENEW).build(); } private BillingEvent.OneTime createRestoreBillingEvent( - DomainHistoryId domainHistoryId, Money restoreCost, DateTime now) { + HistoryEntryId domainHistoryId, Money restoreCost, DateTime now) { return prepareBillingEvent(domainHistoryId, restoreCost, now).setReason(Reason.RESTORE).build(); } private OneTime.Builder prepareBillingEvent( - DomainHistoryId domainHistoryId, Money cost, DateTime now) { + HistoryEntryId domainHistoryId, Money cost, DateTime now) { return new BillingEvent.OneTime.Builder() .setTargetId(targetId) .setRegistrarId(registrarId) @@ -297,15 +292,16 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { FeeType.RENEW, feesAndCredits.hasPremiumFeesOfType(FeeType.RENEW))); } - return feeUpdate.isPresent() - ? ImmutableList.of( - feeUpdate - .get() - .createResponseBuilder() - .setCurrency(feesAndCredits.getCurrency()) - .setFees(fees.build()) - .build()) - : ImmutableList.of(); + return feeUpdate + .map( + feeUpdateCommandExtension -> + ImmutableList.of( + feeUpdateCommandExtension + .createResponseBuilder() + .setCurrency(feesAndCredits.getCurrency()) + .setFees(fees.build()) + .build())) + .orElseGet(ImmutableList::of); } /** Restore command cannot have other changes specified. */ 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 f47410d26..e26467b6c 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainTransferApproveFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainTransferApproveFlow.java @@ -15,7 +15,7 @@ package google.registry.flows.domain; import static com.google.common.collect.Iterables.getOnlyElement; -import static google.registry.flows.FlowUtils.createHistoryKey; +import static google.registry.flows.FlowUtils.createHistoryEntryId; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.computeExDateForApprovalTime; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; @@ -36,7 +36,6 @@ import static google.registry.util.DateTimeUtils.END_OF_TIME; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; import google.registry.flows.FlowModule.RegistrarId; @@ -52,7 +51,6 @@ import google.registry.model.billing.BillingEvent.Reason; import google.registry.model.billing.BillingEvent.Recurring; import google.registry.model.domain.Domain; 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; @@ -62,6 +60,7 @@ import google.registry.model.eppinput.EppInput; import google.registry.model.eppoutput.EppResponse; import google.registry.model.poll.PollMessage; import google.registry.model.reporting.DomainTransactionRecord; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.model.tld.Registry; import google.registry.model.transfer.DomainTransferData; @@ -146,8 +145,8 @@ public final class DomainTransferApproveFlow implements TransactionalFlow { // Create a transfer billing event for 1 year, unless the superuser extension was used to set // the transfer period to zero. There is not a transfer cost if the transfer period is zero. Recurring existingRecurring = tm().loadByKey(existingDomain.getAutorenewBillingEvent()); - Key domainHistoryKey = createHistoryKey(existingDomain, DomainHistory.class); - historyBuilder.setId(domainHistoryKey.getId()); + HistoryEntryId domainHistoryId = createHistoryEntryId(existingDomain); + historyBuilder.setRevisionId(domainHistoryId.getRevisionId()); Optional billingEvent = transferData.getTransferPeriod().getValue() == 0 ? Optional.empty() @@ -167,9 +166,7 @@ public final class DomainTransferApproveFlow implements TransactionalFlow { .getRenewCost()) .setEventTime(now) .setBillingTime(now.plus(Registry.get(tld).getTransferGracePeriodLength())) - .setDomainHistoryId( - new DomainHistoryId( - domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build()); ImmutableList.Builder entitiesToSave = new ImmutableList.Builder<>(); // If we are within an autorenew grace period, cancel the autorenew billing event and don't @@ -185,20 +182,12 @@ public final class DomainTransferApproveFlow implements TransactionalFlow { if (billingEvent.isPresent()) { entitiesToSave.add( BillingEvent.Cancellation.forGracePeriod( - autorenewGrace, - now, - new DomainHistoryId( - domainHistoryKey.getParent().getName(), domainHistoryKey.getId()), - targetId)); + autorenewGrace, now, domainHistoryId, targetId)); } } // Close the old autorenew event and poll message at the transfer time (aka now). This may end // up deleting the poll message. - updateAutorenewRecurrenceEndTime( - existingDomain, - existingRecurring, - now, - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())); + updateAutorenewRecurrenceEndTime(existingDomain, existingRecurring, now, domainHistoryId); DateTime newExpirationTime = computeExDateForApprovalTime(existingDomain, now, transferData.getTransferPeriod()); // Create a new autorenew event starting at the expiration time. @@ -212,9 +201,7 @@ public final class DomainTransferApproveFlow implements TransactionalFlow { .setRenewalPriceBehavior(existingRecurring.getRenewalPriceBehavior()) .setRenewalPrice(existingRecurring.getRenewalPrice().orElse(null)) .setRecurrenceEndTime(END_OF_TIME) - .setDomainHistoryId( - new DomainHistoryId( - domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build(); // Create a new autorenew poll message. PollMessage.Autorenew gainingClientAutorenewPollMessage = @@ -224,9 +211,7 @@ public final class DomainTransferApproveFlow implements TransactionalFlow { .setEventTime(newExpirationTime) .setAutorenewEndTime(END_OF_TIME) .setMsg("Domain was auto-renewed.") - .setDomainHistoryId( - new DomainHistoryId( - domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build(); // Construct the post-transfer domain. Domain partiallyApprovedDomain = @@ -264,7 +249,7 @@ public final class DomainTransferApproveFlow implements TransactionalFlow { // Create a poll message for the gaining client. PollMessage gainingClientPollMessage = createGainingTransferPollMessage( - targetId, newDomain.getTransferData(), newExpirationTime, now, domainHistoryKey); + targetId, newDomain.getTransferData(), newExpirationTime, now, domainHistoryId); billingEvent.ifPresent(entitiesToSave::add); entitiesToSave.add( autorenewEvent, diff --git a/core/src/main/java/google/registry/flows/domain/DomainTransferCancelFlow.java b/core/src/main/java/google/registry/flows/domain/DomainTransferCancelFlow.java index a12d6fc0b..7d5aea443 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainTransferCancelFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainTransferCancelFlow.java @@ -14,7 +14,7 @@ package google.registry.flows.domain; -import static google.registry.flows.FlowUtils.createHistoryKey; +import static google.registry.flows.FlowUtils.createHistoryEntryId; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; import static google.registry.flows.ResourceFlowUtils.verifyHasPendingTransfer; @@ -32,7 +32,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import static google.registry.util.DateTimeUtils.END_OF_TIME; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; import google.registry.flows.FlowModule.RegistrarId; @@ -47,6 +46,7 @@ import google.registry.model.domain.metadata.MetadataExtension; import google.registry.model.eppcommon.AuthInfo; import google.registry.model.eppoutput.EppResponse; import google.registry.model.reporting.DomainTransactionRecord; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.model.tld.Registry; import google.registry.model.transfer.TransferStatus; @@ -83,7 +83,9 @@ public final class DomainTransferCancelFlow implements TransactionalFlow { @Inject @Superuser boolean isSuperuser; @Inject DomainHistory.Builder historyBuilder; @Inject EppResponse.Builder responseBuilder; - @Inject DomainTransferCancelFlow() {} + + @Inject + DomainTransferCancelFlow() {} @Override public EppResponse run() throws EppException { @@ -100,9 +102,9 @@ public final class DomainTransferCancelFlow implements TransactionalFlow { } Registry registry = Registry.get(existingDomain.getTld()); - Key domainHistoryKey = createHistoryKey(existingDomain, DomainHistory.class); + HistoryEntryId domainHistoryId = createHistoryEntryId(existingDomain); historyBuilder - .setId(domainHistoryKey.getId()) + .setRevisionId(domainHistoryId.getRevisionId()) .setOtherRegistrarId(existingDomain.getTransferData().getLosingRegistrarId()); Domain newDomain = @@ -112,12 +114,12 @@ public final class DomainTransferCancelFlow implements TransactionalFlow { newDomain, domainHistory, createLosingTransferPollMessage( - targetId, newDomain.getTransferData(), null, domainHistoryKey)); + targetId, newDomain.getTransferData(), null, domainHistoryId)); // Reopen the autorenew event and poll message that we closed for the implicit transfer. This // may recreate the autorenew poll message if it was deleted when the transfer request was made. Recurring existingRecurring = tm().loadByKey(existingDomain.getAutorenewBillingEvent()); updateAutorenewRecurrenceEndTime( - existingDomain, existingRecurring, END_OF_TIME, domainHistory.getDomainHistoryId()); + existingDomain, existingRecurring, END_OF_TIME, domainHistory.getHistoryEntryId()); // Delete the billing event and poll messages that were written in case the transfer would have // been implicitly server approved. tm().delete(existingDomain.getTransferData().getServerApproveEntities()); diff --git a/core/src/main/java/google/registry/flows/domain/DomainTransferRejectFlow.java b/core/src/main/java/google/registry/flows/domain/DomainTransferRejectFlow.java index 0b0f8d280..7f88b2756 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainTransferRejectFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainTransferRejectFlow.java @@ -14,7 +14,7 @@ package google.registry.flows.domain; -import static google.registry.flows.FlowUtils.createHistoryKey; +import static google.registry.flows.FlowUtils.createHistoryEntryId; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; import static google.registry.flows.ResourceFlowUtils.verifyHasPendingTransfer; @@ -34,7 +34,6 @@ import static google.registry.util.CollectionUtils.union; import static google.registry.util.DateTimeUtils.END_OF_TIME; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; import google.registry.flows.FlowModule.RegistrarId; @@ -49,6 +48,7 @@ import google.registry.model.domain.metadata.MetadataExtension; import google.registry.model.eppcommon.AuthInfo; import google.registry.model.eppoutput.EppResponse; import google.registry.model.reporting.DomainTransactionRecord; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.model.tld.Registry; import google.registry.model.transfer.TransferStatus; @@ -95,9 +95,9 @@ public final class DomainTransferRejectFlow implements TransactionalFlow { DateTime now = tm().getTransactionTime(); Domain existingDomain = loadAndVerifyExistence(Domain.class, targetId, now); Registry registry = Registry.get(existingDomain.getTld()); - Key domainHistoryKey = createHistoryKey(existingDomain, DomainHistory.class); + HistoryEntryId domainHistoryId = createHistoryEntryId(existingDomain); historyBuilder - .setId(domainHistoryKey.getId()) + .setRevisionId(domainHistoryId.getRevisionId()) .setOtherRegistrarId(existingDomain.getTransferData().getGainingRegistrarId()); verifyOptionalAuthInfo(authInfo, existingDomain); @@ -113,12 +113,12 @@ public final class DomainTransferRejectFlow implements TransactionalFlow { newDomain, domainHistory, createGainingTransferPollMessage( - targetId, newDomain.getTransferData(), null, now, domainHistoryKey)); + targetId, newDomain.getTransferData(), null, now, domainHistoryId)); // Reopen the autorenew event and poll message that we closed for the implicit transfer. This // may end up recreating the poll message if it was deleted upon the transfer request. Recurring existingRecurring = tm().loadByKey(existingDomain.getAutorenewBillingEvent()); updateAutorenewRecurrenceEndTime( - existingDomain, existingRecurring, END_OF_TIME, domainHistory.getDomainHistoryId()); + existingDomain, existingRecurring, END_OF_TIME, domainHistory.getHistoryEntryId()); // Delete the billing event and poll messages that were written in case the transfer would have // been implicitly server approved. tm().delete(existingDomain.getTransferData().getServerApproveEntities()); 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 e56c2d829..b9d0630c4 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainTransferRequestFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainTransferRequestFlow.java @@ -14,7 +14,7 @@ package google.registry.flows.domain; -import static google.registry.flows.FlowUtils.createHistoryKey; +import static google.registry.flows.FlowUtils.createHistoryEntryId; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.computeExDateForApprovalTime; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; @@ -38,7 +38,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import google.registry.batch.AsyncTaskEnqueuer; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; @@ -57,7 +56,6 @@ import google.registry.model.billing.BillingEvent.Recurring; import google.registry.model.domain.Domain; import google.registry.model.domain.DomainCommand.Transfer; import google.registry.model.domain.DomainHistory; -import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.Period; import google.registry.model.domain.fee.FeeTransferCommandExtension; import google.registry.model.domain.fee.FeeTransformResponseExtension; @@ -73,6 +71,7 @@ import google.registry.model.eppoutput.EppResponse; import google.registry.model.poll.PollMessage; import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.model.tld.Registry; import google.registry.model.transfer.DomainTransferData; @@ -196,16 +195,16 @@ public final class DomainTransferRequestFlow implements TransactionalFlow { // If the period is zero, then there is no fee for the transfer. Recurring existingRecurring = tm().loadByKey(existingDomain.getAutorenewBillingEvent()); Optional feesAndCredits = - (period.getValue() == 0) + period.getValue() == 0 ? Optional.empty() : Optional.of( pricingLogic.getTransferPrice(registry, targetId, now, existingRecurring)); if (feesAndCredits.isPresent()) { - validateFeeChallenge(targetId, now, feeTransfer, feesAndCredits.get()); + validateFeeChallenge(feeTransfer, feesAndCredits.get()); } - Key domainHistoryKey = createHistoryKey(existingDomain, DomainHistory.class); + HistoryEntryId domainHistoryId = createHistoryEntryId(existingDomain); historyBuilder - .setId(domainHistoryKey.getId()) + .setRevisionId(domainHistoryId.getRevisionId()) .setOtherRegistrarId(existingDomain.getCurrentSponsorRegistrarId()); DateTime automaticTransferTime = superuserExtension @@ -230,7 +229,7 @@ public final class DomainTransferRequestFlow implements TransactionalFlow { createTransferServerApproveEntities( automaticTransferTime, serverApproveNewExpirationTime, - domainHistoryKey, + domainHistoryId, existingDomain, existingRecurring, trid, @@ -241,7 +240,7 @@ public final class DomainTransferRequestFlow implements TransactionalFlow { DomainTransferData pendingTransferData = createPendingTransferData( domainAtTransferTime.getRepoId(), - domainHistoryKey.getId(), + domainHistoryId.getRevisionId(), new DomainTransferData.Builder() .setTransferRequestTrid(trid) .setTransferRequestTime(now) @@ -254,7 +253,7 @@ public final class DomainTransferRequestFlow implements TransactionalFlow { // Create a poll message to notify the losing registrar that a transfer was requested. PollMessage requestPollMessage = createLosingTransferPollMessage( - targetId, pendingTransferData, serverApproveNewExpirationTime, domainHistoryKey) + targetId, pendingTransferData, serverApproveNewExpirationTime, domainHistoryId) .asBuilder() .setEventTime(now) .build(); @@ -263,10 +262,7 @@ public final class DomainTransferRequestFlow implements TransactionalFlow { // cloneProjectedAtTime() will replace these old autorenew entities with the server approve ones // that we've created in this flow and stored in pendingTransferData. updateAutorenewRecurrenceEndTime( - existingDomain, - existingRecurring, - automaticTransferTime, - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())); + existingDomain, existingRecurring, automaticTransferTime, domainHistoryId); Domain newDomain = existingDomain .asBuilder() @@ -385,7 +381,7 @@ public final class DomainTransferRequestFlow implements TransactionalFlow { private static ImmutableList createResponseExtensions( Optional feesAndCredits, Optional feeTransfer) { - return (feeTransfer.isPresent() && feesAndCredits.isPresent()) + return feeTransfer.isPresent() && feesAndCredits.isPresent() ? ImmutableList.of( feeTransfer .get() 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 2cb4df90f..fa1b8dd53 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainTransferUtils.java +++ b/core/src/main/java/google/registry/flows/domain/DomainTransferUtils.java @@ -20,20 +20,18 @@ import static google.registry.util.DateTimeUtils.END_OF_TIME; import com.google.common.collect.ImmutableList; 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; import google.registry.model.billing.BillingEvent.Recurring; import google.registry.model.domain.Domain; -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; import google.registry.model.eppcommon.Trid; import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse; import google.registry.model.poll.PollMessage; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.tld.Registry; import google.registry.model.transfer.DomainTransferData; import google.registry.model.transfer.TransferData; @@ -109,7 +107,7 @@ public final class DomainTransferUtils { public static ImmutableSet createTransferServerApproveEntities( DateTime automaticTransferTime, DateTime serverApproveNewExpirationTime, - Key domainHistoryKey, + HistoryEntryId domainHistoryId, Domain existingDomain, Recurring existingRecurring, Trid trid, @@ -135,38 +133,38 @@ public final class DomainTransferUtils { builder.add( createTransferBillingEvent( automaticTransferTime, - domainHistoryKey, + domainHistoryId, targetId, gainingRegistrarId, registry, cost))); createOptionalAutorenewCancellation( - automaticTransferTime, now, domainHistoryKey, targetId, existingDomain, transferCost) + automaticTransferTime, now, domainHistoryId, targetId, existingDomain, transferCost) .ifPresent(builder::add); return builder .add( createGainingClientAutorenewEvent( existingRecurring, serverApproveNewExpirationTime, - domainHistoryKey, + domainHistoryId, targetId, gainingRegistrarId)) .add( createGainingClientAutorenewPollMessage( - serverApproveNewExpirationTime, domainHistoryKey, targetId, gainingRegistrarId)) + serverApproveNewExpirationTime, domainHistoryId, targetId, gainingRegistrarId)) .add( createGainingTransferPollMessage( targetId, serverApproveTransferData, serverApproveNewExpirationTime, now, - domainHistoryKey)) + domainHistoryId)) .add( createLosingTransferPollMessage( targetId, serverApproveTransferData, serverApproveNewExpirationTime, - domainHistoryKey)) + domainHistoryId)) .build(); } @@ -176,7 +174,7 @@ public final class DomainTransferUtils { TransferData transferData, @Nullable DateTime extendedRegistrationExpirationTime, DateTime now, - Key domainHistoryKey) { + HistoryEntryId domainHistoryId) { return new PollMessage.OneTime.Builder() .setRegistrarId(transferData.getGainingRegistrarId()) .setEventTime(transferData.getPendingTransferExpirationTime()) @@ -189,8 +187,7 @@ public final class DomainTransferUtils { transferData.getTransferStatus().isApproved(), transferData.getTransferRequestTrid(), now))) - .setDomainHistoryId( - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build(); } @@ -199,7 +196,7 @@ public final class DomainTransferUtils { String targetId, TransferData transferData, @Nullable DateTime extendedRegistrationExpirationTime, - Key domainHistoryKey) { + HistoryEntryId domainHistoryId) { return new PollMessage.OneTime.Builder() .setRegistrarId(transferData.getLosingRegistrarId()) .setEventTime(transferData.getPendingTransferExpirationTime()) @@ -207,8 +204,7 @@ public final class DomainTransferUtils { .setResponseData( ImmutableList.of( createTransferResponse(targetId, transferData, extendedRegistrationExpirationTime))) - .setDomainHistoryId( - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build(); } @@ -230,7 +226,7 @@ public final class DomainTransferUtils { private static PollMessage.Autorenew createGainingClientAutorenewPollMessage( DateTime serverApproveNewExpirationTime, - Key domainHistoryKey, + HistoryEntryId domainHistoryId, String targetId, String gainingRegistrarId) { return new PollMessage.Autorenew.Builder() @@ -239,15 +235,14 @@ public final class DomainTransferUtils { .setEventTime(serverApproveNewExpirationTime) .setAutorenewEndTime(END_OF_TIME) .setMsg("Domain was auto-renewed.") - .setDomainHistoryId( - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build(); } private static BillingEvent.Recurring createGainingClientAutorenewEvent( Recurring existingRecurring, DateTime serverApproveNewExpirationTime, - Key domainHistoryKey, + HistoryEntryId domainHistoryId, String targetId, String gainingRegistrarId) { return new BillingEvent.Recurring.Builder() @@ -259,8 +254,7 @@ public final class DomainTransferUtils { .setRecurrenceEndTime(END_OF_TIME) .setRenewalPriceBehavior(existingRecurring.getRenewalPriceBehavior()) .setRenewalPrice(existingRecurring.getRenewalPrice().orElse(null)) - .setDomainHistoryId( - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .build(); } @@ -283,7 +277,7 @@ public final class DomainTransferUtils { private static Optional createOptionalAutorenewCancellation( DateTime automaticTransferTime, DateTime now, - Key domainHistoryKey, + HistoryEntryId domainHistoryId, String targetId, Domain existingDomain, Optional transferCost) { @@ -294,11 +288,7 @@ public final class DomainTransferUtils { if (autorenewGracePeriod != null && transferCost.isPresent()) { return Optional.of( BillingEvent.Cancellation.forGracePeriod( - autorenewGracePeriod, - now, - new DomainHistoryId( - domainHistoryKey.getParent().getName(), domainHistoryKey.getId()), - targetId) + autorenewGracePeriod, now, domainHistoryId, targetId) .asBuilder() .setEventTime(automaticTransferTime) .build()); @@ -308,7 +298,7 @@ public final class DomainTransferUtils { private static BillingEvent.OneTime createTransferBillingEvent( DateTime automaticTransferTime, - Key domainHistoryKey, + HistoryEntryId domainHistoryId, String targetId, String gainingRegistrarId, Registry registry, @@ -321,8 +311,7 @@ public final class DomainTransferUtils { .setPeriodYears(1) .setEventTime(automaticTransferTime) .setBillingTime(automaticTransferTime.plus(registry.getTransferGracePeriodLength())) - .setDomainHistoryId( - new DomainHistoryId(domainHistoryKey.getParent().getName(), domainHistoryKey.getId())) + .setDomainHistoryId(domainHistoryId) .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 02b3334b1..2d222a568 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java @@ -208,12 +208,9 @@ public final class DomainUpdateFlow implements TransactionalFlow { /** Determines if any of the changes to new domain should trigger DNS update. */ private boolean requiresDnsUpdate(Domain existingDomain, Domain newDomain) { - if (existingDomain.shouldPublishToDns() != newDomain.shouldPublishToDns() + return existingDomain.shouldPublishToDns() != newDomain.shouldPublishToDns() || !Objects.equals(newDomain.getDsData(), existingDomain.getDsData()) - || !Objects.equals(newDomain.getNsHosts(), existingDomain.getNsHosts())) { - return true; - } - return false; + || !Objects.equals(newDomain.getNsHosts(), existingDomain.getNsHosts()); } /** Fail if the object doesn't exist or was deleted. */ @@ -363,7 +360,7 @@ public final class DomainUpdateFlow implements TransactionalFlow { .map(StatusValue::getXmlName) .collect(toImmutableSortedSet(Ordering.natural())); - String msg = ""; + String msg; if (addedServerStatuses.size() > 0 && removedServerStatuses.size() > 0) { msg = String.format( diff --git a/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java b/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java index 629096fed..a87c81f64 100644 --- a/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java +++ b/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java @@ -36,7 +36,7 @@ import google.registry.model.domain.token.AllocationToken.TokenBehavior; import google.registry.model.domain.token.AllocationToken.TokenStatus; import google.registry.model.domain.token.AllocationToken.TokenType; import google.registry.model.domain.token.AllocationTokenExtension; -import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.tld.Registry; import google.registry.persistence.VKey; import java.util.List; @@ -95,12 +95,11 @@ public class AllocationTokenFlowUtils { } /** Redeems a SINGLE_USE {@link AllocationToken}, returning the redeemed copy. */ - public AllocationToken redeemToken( - AllocationToken token, VKey redemptionHistoryEntry) { + public AllocationToken redeemToken(AllocationToken token, HistoryEntryId redemptionHistoryId) { checkArgument( TokenType.SINGLE_USE.equals(token.getTokenType()), "Only SINGLE_USE tokens can be marked as redeemed"); - return token.asBuilder().setRedemptionHistoryEntry(redemptionHistoryEntry).build(); + return token.asBuilder().setRedemptionHistoryId(redemptionHistoryId).build(); } /** @@ -110,7 +109,7 @@ public class AllocationTokenFlowUtils { * * @throws EppException if the token is invalid in any way */ - private void validateToken( + private static void validateToken( InternetDomainName domainName, AllocationToken token, String registrarId, DateTime now) throws EppException { @@ -138,7 +137,7 @@ public class AllocationTokenFlowUtils { } /** Loads a given token and validates that it is not redeemed */ - private AllocationToken loadToken(String token) throws EppException { + private static AllocationToken loadToken(String token) throws EppException { if (Strings.isNullOrEmpty(token)) { // We load the token directly from the input XML. If it's null or empty we should throw // an InvalidAllocationTokenException before the database load attempt fails. diff --git a/core/src/main/java/google/registry/model/EntityClasses.java b/core/src/main/java/google/registry/model/EntityClasses.java index 8594b8436..371e2f097 100644 --- a/core/src/main/java/google/registry/model/EntityClasses.java +++ b/core/src/main/java/google/registry/model/EntityClasses.java @@ -18,12 +18,8 @@ import com.google.common.collect.ImmutableSet; import google.registry.model.annotations.DeleteAfterMigration; import google.registry.model.common.GaeUserIdConverter; import google.registry.model.contact.Contact; -import google.registry.model.contact.ContactHistory; import google.registry.model.domain.Domain; -import google.registry.model.domain.DomainHistory; import google.registry.model.host.Host; -import google.registry.model.host.HostHistory; -import google.registry.model.reporting.HistoryEntry; /** Sets of classes of the Objectify-registered entities in use throughout the model. */ @DeleteAfterMigration @@ -31,15 +27,7 @@ public final class EntityClasses { /** Set of entity classes. */ public static final ImmutableSet> ALL_CLASSES = - ImmutableSet.of( - Contact.class, - ContactHistory.class, - Domain.class, - DomainHistory.class, - GaeUserIdConverter.class, - HistoryEntry.class, - Host.class, - HostHistory.class); + ImmutableSet.of(Contact.class, Domain.class, GaeUserIdConverter.class, Host.class); private EntityClasses() {} } diff --git a/core/src/main/java/google/registry/model/UpdateAutoTimestampEntity.java b/core/src/main/java/google/registry/model/UpdateAutoTimestampEntity.java index af54ed679..5184c0f4c 100644 --- a/core/src/main/java/google/registry/model/UpdateAutoTimestampEntity.java +++ b/core/src/main/java/google/registry/model/UpdateAutoTimestampEntity.java @@ -30,7 +30,7 @@ public abstract class UpdateAutoTimestampEntity extends ImmutableObject implements UnsafeSerializable { /** - * An automatically managed timestamp of when this object was last written to Datastore. + * An automatically managed timestamp of when this object was last written to the database. * *

Note that this is distinct from the EPP-specified {@link EppResource#lastEppUpdateTime}, in * that this is updated on every save, rather than only in response to an {@code } command diff --git a/core/src/main/java/google/registry/model/billing/BillingEvent.java b/core/src/main/java/google/registry/model/billing/BillingEvent.java index 6c282a65f..f3dd36a0c 100644 --- a/core/src/main/java/google/registry/model/billing/BillingEvent.java +++ b/core/src/main/java/google/registry/model/billing/BillingEvent.java @@ -32,10 +32,10 @@ import google.registry.model.annotations.OfyIdAllocation; import google.registry.model.annotations.ReportedOn; import google.registry.model.common.TimeOfYear; import google.registry.model.domain.DomainHistory; -import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.GracePeriod; import google.registry.model.domain.rgp.GracePeriodStatus; import google.registry.model.domain.token.AllocationToken; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.transfer.TransferData.TransferServerApproveEntity; import google.registry.persistence.VKey; import google.registry.persistence.WithVKey; @@ -204,8 +204,8 @@ public abstract class BillingEvent extends ImmutableObject return targetId; } - public DomainHistoryId getDomainHistoryId() { - return new DomainHistoryId(domainRepoId, domainHistoryRevisionId); + public HistoryEntryId getHistoryEntryId() { + return new HistoryEntryId(domainRepoId, domainHistoryRevisionId); } public ImmutableSet getFlags() { @@ -259,14 +259,14 @@ public abstract class BillingEvent extends ImmutableObject return thisCastToDerived(); } - public B setDomainHistoryId(DomainHistoryId domainHistoryId) { - getInstance().domainHistoryRevisionId = domainHistoryId.getId(); - getInstance().domainRepoId = domainHistoryId.getDomainRepoId(); + public B setDomainHistoryId(HistoryEntryId domainHistoryId) { + getInstance().domainHistoryRevisionId = domainHistoryId.getRevisionId(); + getInstance().domainRepoId = domainHistoryId.getRepoId(); return thisCastToDerived(); } public B setDomainHistory(DomainHistory domainHistory) { - return setDomainHistoryId(domainHistory.getDomainHistoryId()); + return setDomainHistoryId(domainHistory.getHistoryEntryId()); } @Override @@ -653,7 +653,7 @@ public abstract class BillingEvent extends ImmutableObject public static Cancellation forGracePeriod( GracePeriod gracePeriod, DateTime eventTime, - DomainHistoryId domainHistoryId, + HistoryEntryId domainHistoryId, String targetId) { checkArgument( gracePeriod.hasBillingEvent(), 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 bb2d86c8f..a6f7983b3 100644 --- a/core/src/main/java/google/registry/model/contact/ContactHistory.java +++ b/core/src/main/java/google/registry/model/contact/ContactHistory.java @@ -14,24 +14,18 @@ package google.registry.model.contact; -import com.googlecode.objectify.Key; -import com.googlecode.objectify.annotation.EntitySubclass; import google.registry.model.EppResource; -import google.registry.model.ImmutableObject; -import google.registry.model.UnsafeSerializable; -import google.registry.model.contact.ContactHistory.ContactHistoryId; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; -import java.io.Serializable; import java.util.Optional; import javax.annotation.Nullable; import javax.persistence.Access; import javax.persistence.AccessType; +import javax.persistence.AttributeOverride; import javax.persistence.Column; import javax.persistence.Entity; -import javax.persistence.Id; -import javax.persistence.IdClass; -import javax.persistence.PostLoad; +import javax.persistence.Index; +import javax.persistence.Table; /** * A persisted history entry representing an EPP modification to a contact. @@ -39,53 +33,26 @@ import javax.persistence.PostLoad; *

In addition to the general history fields (e.g. action time, registrar ID) we also persist a * copy of the contact entity at this point in time. We persist a raw {@link ContactBase} so that * the foreign-keyed fields in that class can refer to this object. - * - *

This class is only marked as a Datastore entity subclass and registered with Objectify so that - * when building it its ID can be auto-populated by Objectify. It is converted to its superclass - * {@link HistoryEntry} when persisted to Datastore using {@link - * google.registry.persistence.transaction.TransactionManager}. */ @Entity -@javax.persistence.Table( +@Table( indexes = { - @javax.persistence.Index(columnList = "creationTime"), - @javax.persistence.Index(columnList = "historyRegistrarId"), - @javax.persistence.Index(columnList = "historyType"), - @javax.persistence.Index(columnList = "historyModificationTime") + @Index(columnList = "creationTime"), + @Index(columnList = "historyRegistrarId"), + @Index(columnList = "historyType"), + @Index(columnList = "historyModificationTime") }) -@EntitySubclass +@AttributeOverride(name = "repoId", column = @Column(name = "contactRepoId")) @Access(AccessType.FIELD) -@IdClass(ContactHistoryId.class) -public class ContactHistory extends HistoryEntry implements UnsafeSerializable { +public class ContactHistory extends HistoryEntry { - // Store ContactBase instead of Contact so we don't pick up its @Id - // Nullable for the sake of pre-Registry-3.0 history objects - @Nullable ContactBase contactBase; + // Store ContactBase instead of Contact, so we don't pick up its @Id + // @Nullable for the sake of pre-Registry-3.0 history objects + @Nullable ContactBase resource; - @Id - @Access(AccessType.PROPERTY) - public String getContactRepoId() { - // We need to handle null case here because Hibernate sometimes accesses this method before - // parent gets initialized - return parent == null ? null : parent.getName(); - } - - /** This method is private because it is only used by Hibernate. */ - @SuppressWarnings("unused") - private void setContactRepoId(String contactRepoId) { - parent = Key.create(Contact.class, contactRepoId); - } - - @Id - @Column(name = "historyRevisionId") - @Access(AccessType.PROPERTY) @Override - public long getId() { - return super.getId(); - } - - public ContactHistoryId getContactHistoryId() { - return new ContactHistoryId(getContactRepoId(), getId()); + protected ContactBase getResource() { + return resource; } /** @@ -95,19 +62,13 @@ public class ContactHistory extends HistoryEntry implements UnsafeSerializable { *

Will be absent for objects created prior to the Registry 3.0 SQL migration. */ public Optional getContactBase() { - return Optional.ofNullable(contactBase); - } - - /** The key to the {@link Contact} this is based off of. */ - public VKey getParentVKey() { - return VKey.create(Contact.class, getContactRepoId()); + return Optional.ofNullable(resource); } /** Creates a {@link VKey} instance for this entity. */ - @SuppressWarnings("unchecked") @Override public VKey createVKey() { - return (VKey) createVKey(Key.create(this)); + return VKey.createSql(ContactHistory.class, getHistoryEntryId()); } @Override @@ -115,77 +76,6 @@ public class ContactHistory extends HistoryEntry implements UnsafeSerializable { return getContactBase().map(contactBase -> new Contact.Builder().copyFrom(contactBase).build()); } - @PostLoad - void postLoad() { - // Normally Hibernate would see that the contact fields are all null and would fill contactBase - // with a null object. Unfortunately, the updateTimestamp is never null in SQL. - if (contactBase != null && contactBase.getContactId() == null) { - contactBase = null; - } - if (contactBase != null && contactBase.getRepoId() == null) { - // contactBase hasn't been fully constructed yet, so it's ok to go in and mutate it. Though - // the use of the Builder is not necessarily problematic in this case, this is still safer as - // the Builder can do things like comparisons that compute the hash code. - contactBase.setRepoId(parent.getName()); - } - } - - /** Class to represent the composite primary key of {@link ContactHistory} entity. */ - public static class ContactHistoryId extends ImmutableObject implements Serializable { - - private String contactRepoId; - - private Long id; - - /** Hibernate requires this default constructor. */ - private ContactHistoryId() {} - - public ContactHistoryId(String contactRepoId, long id) { - this.contactRepoId = contactRepoId; - this.id = id; - } - - /** - * Returns the contact repository id. - * - *

This method is private because it is only used by Hibernate. - */ - public String getContactRepoId() { - return contactRepoId; - } - - /** - * Returns the history revision id. - * - *

This method is private because it is only used by Hibernate. - */ - public long getId() { - return id; - } - - /** - * Sets the contact repository id. - * - *

This method is private because it is only used by Hibernate and should not be used - * externally to keep immutability. - */ - @SuppressWarnings("unused") - private void setContactRepoId(String contactRepoId) { - this.contactRepoId = contactRepoId; - } - - /** - * Sets the history revision id. - * - *

This method is private because it is only used by Hibernate and should not be used - * externally to keep immutability. - */ - @SuppressWarnings("unused") - private void setId(long id) { - this.id = id; - } - } - @Override public Builder asBuilder() { return new Builder(clone(this)); @@ -199,23 +89,13 @@ public class ContactHistory extends HistoryEntry implements UnsafeSerializable { super(instance); } - public Builder setContact(@Nullable ContactBase contactBase) { - // Nullable for the sake of pre-Registry-3.0 history objects - if (contactBase == null) { - return this; - } - getInstance().contactBase = contactBase; - return super.setParent(contactBase); - } - - public Builder setContactRepoId(String contactRepoId) { - getInstance().parent = Key.create(Contact.class, contactRepoId); - return this; + public Builder setContact(ContactBase contactBase) { + getInstance().resource = contactBase; + return setRepoId(contactBase); } public Builder wipeOutPii() { - getInstance().contactBase = - getInstance().getContactBase().get().asBuilder().wipeOut().build(); + getInstance().resource = getInstance().resource.asBuilder().wipeOut().build(); return this; } } 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 10f1c030c..a18e05810 100644 --- a/core/src/main/java/google/registry/model/domain/DomainHistory.java +++ b/core/src/main/java/google/registry/model/domain/DomainHistory.java @@ -18,12 +18,8 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; -import com.googlecode.objectify.annotation.EntitySubclass; import com.googlecode.objectify.annotation.Ignore; import google.registry.model.EppResource; -import google.registry.model.ImmutableObject; -import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.GracePeriod.GracePeriodHistory; import google.registry.model.domain.secdns.DomainDsData; import google.registry.model.domain.secdns.DomainDsDataHistory; @@ -31,7 +27,6 @@ import google.registry.model.host.Host; import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; -import java.io.Serializable; import java.util.HashSet; import java.util.Optional; import java.util.Set; @@ -45,8 +40,6 @@ import javax.persistence.Column; import javax.persistence.ElementCollection; import javax.persistence.Entity; import javax.persistence.FetchType; -import javax.persistence.Id; -import javax.persistence.IdClass; import javax.persistence.Index; import javax.persistence.JoinColumn; import javax.persistence.JoinColumns; @@ -62,11 +55,6 @@ import org.hibernate.Hibernate; *

In addition to the general history fields (e.g. action time, registrar ID) we also persist a * copy of the domain entity at this point in time. We persist a raw {@link DomainBase} so that the * foreign-keyed fields in that class can refer to this object. - * - *

This class is only marked as a Datastore entity subclass and registered with Objectify so that - * when building it its ID can be auto-populated by Objectify. It is converted to its superclass - * {@link HistoryEntry} when persisted to Datastore using {@link - * google.registry.persistence.transaction.TransactionManager}. */ @Entity @Table( @@ -76,49 +64,44 @@ import org.hibernate.Hibernate; @Index(columnList = "historyType"), @Index(columnList = "historyModificationTime") }) -@EntitySubclass @Access(AccessType.FIELD) -@IdClass(DomainHistoryId.class) +@AttributeOverride(name = "repoId", column = @Column(name = "domainRepoId")) public class DomainHistory extends HistoryEntry { - // Store DomainBase instead of Domain so we don't pick up its @Id - // Nullable for the sake of pre-Registry-3.0 history objects - @Nullable DomainBase domainBase; + // Store DomainBase instead of Domain, so we don't pick up its @Id + // @Nullable for the sake of pre-Registry-3.0 history objects + @Nullable DomainBase resource; - @Id - @Access(AccessType.PROPERTY) - public String getDomainRepoId() { - // We need to handle null case here because Hibernate sometimes accesses this method before - // parent gets initialized - return parent == null ? null : parent.getName(); + @Override + protected DomainBase getResource() { + return resource; } - /** This method is private because it is only used by Hibernate. */ - @SuppressWarnings("unused") - private void setDomainRepoId(String domainRepoId) { - parent = Key.create(Domain.class, domainRepoId); + /** + * The values of all the fields on the {@link DomainBase} object after the action represented by + * this history object was executed. + * + *

Will be absent for objects created prior to the Registry 3.0 SQL migration. + */ + public Optional getDomainBase() { + return Optional.ofNullable(resource); } // We could have reused domainBase.nsHosts here, but Hibernate throws a weird exception after // we change to use a composite primary key. - // TODO(b/166776754): Investigate if we can reuse domainBase.nsHosts for storing host keys. @ElementCollection @JoinTable( name = "DomainHistoryHost", - indexes = { - @Index( - columnList = - "domain_history_history_revision_id,domain_history_domain_repo_id,host_repo_id", - unique = true), - }) - @ImmutableObject.EmptySetToNull + indexes = + @Index( + columnList = + "domain_history_history_revision_id,domain_history_domain_repo_id,host_repo_id", + unique = true)) + @EmptySetToNull @Column(name = "host_repo_id") Set> nsHosts; - @OneToMany( - cascade = {CascadeType.ALL}, - fetch = FetchType.EAGER, - orphanRemoval = true) + @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER, orphanRemoval = true) @JoinColumns({ @JoinColumn( name = "domainHistoryRevisionId", @@ -135,10 +118,7 @@ public class DomainHistory extends HistoryEntry { @Ignore Set dsDataHistories = new HashSet<>(); - @OneToMany( - cascade = {CascadeType.ALL}, - fetch = FetchType.EAGER, - orphanRemoval = true) + @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER, orphanRemoval = true) @JoinColumns({ @JoinColumn( name = "domainHistoryRevisionId", @@ -152,19 +132,14 @@ public class DomainHistory extends HistoryEntry { updatable = false) }) // HashSet rather than ImmutableSet so that Hibernate can fill them out lazily on request - @Ignore Set gracePeriodHistories = new HashSet<>(); - @Override - @Nullable - @Access(AccessType.PROPERTY) + /** The length of time that a create, allocate, renewal, or transfer request was issued for. */ @AttributeOverrides({ @AttributeOverride(name = "unit", column = @Column(name = "historyPeriodUnit")), @AttributeOverride(name = "value", column = @Column(name = "historyPeriodValue")) }) - public Period getPeriod() { - return super.getPeriod(); - } + Period period; /** * For transfers, the id of the other registrar. @@ -174,56 +149,29 @@ public class DomainHistory extends HistoryEntry { * registrar is the gaining party. */ @Nullable - @Access(AccessType.PROPERTY) @Column(name = "historyOtherRegistrarId") - @Override - public String getOtherRegistrarId() { - return super.getOtherRegistrarId(); - } + String otherRegistrarId; /** * Logging field for transaction reporting. * - *

This will be empty for any DomainHistory/HistoryEntry generated before this field was added, - * mid-2017, as well as any action that does not generate billable events (e.g. updates). - * - *

This method is dedicated for Hibernate, external caller should use {@link - * #getDomainTransactionRecords()}. + *

This will be empty for any DomainHistory/HistoryEntry generated before this field was added + * (mid-2017), as well as any action that does not generate billable events (e.g. contact/host + * updates). * */ - @Access(AccessType.PROPERTY) - @OneToMany( - cascade = {CascadeType.ALL}, - fetch = FetchType.EAGER, - orphanRemoval = true) + @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER, orphanRemoval = true) @JoinColumn(name = "historyRevisionId", referencedColumnName = "historyRevisionId") @JoinColumn(name = "domainRepoId", referencedColumnName = "domainRepoId") - @SuppressWarnings("unused") - private Set getInternalDomainTransactionRecords() { - return domainTransactionRecords; - } + @EmptySetToNull + Set domainTransactionRecords; - /** Sets the domain transaction records. This method is dedicated for Hibernate. */ - @SuppressWarnings("unused") - private void setInternalDomainTransactionRecords( - Set domainTransactionRecords) { - super.setDomainTransactionRecords(domainTransactionRecords); - } - - @Id - @Column(name = "historyRevisionId") - @Access(AccessType.PROPERTY) - @Override - public long getId() { - return super.getId(); - } - - public DomainHistoryId getDomainHistoryId() { - return new DomainHistoryId(getDomainRepoId(), getId()); + public Set getDomainTransactionRecords() { + return nullToEmptyImmutableCopy(domainTransactionRecords); } /** Returns keys to the {@link Host} that are the nameservers for the domain. */ public Set> getNsHosts() { - return nsHosts; + return ImmutableSet.copyOf(nsHosts); } /** Returns the collection of {@link DomainDsDataHistory} instances. */ @@ -231,30 +179,14 @@ public class DomainHistory extends HistoryEntry { return nullToEmptyImmutableCopy(dsDataHistories); } - /** - * The values of all the fields on the {@link DomainBase} object after the action represented by - * this history object was executed. - * - *

Will be absent for objects created prior to the Registry 3.0 SQL migration. - */ - public Optional getDomainBase() { - return Optional.ofNullable(domainBase); - } - - /** The key to the {@link Domain} this is based off of. */ - public VKey getParentVKey() { - return VKey.create(Domain.class, getDomainRepoId()); - } - public Set getGracePeriodHistories() { return nullToEmptyImmutableCopy(gracePeriodHistories); } /** Creates a {@link VKey} instance for this entity. */ - @SuppressWarnings("unchecked") @Override public VKey createVKey() { - return (VKey) createVKey(Key.create(this)); + return VKey.createSql(DomainHistory.class, getHistoryEntryId()); } @Override @@ -262,102 +194,55 @@ public class DomainHistory extends HistoryEntry { return getDomainBase().map(domainBase -> new Domain.Builder().copyFrom(domainBase).build()); } + public String getOtherRegistrarId() { + return otherRegistrarId; + } + + public Period getPeriod() { + return period; + } + + @Override @PostLoad - void postLoad() { + protected void postLoad() { // TODO(b/188044616): Determine why Eager loading doesn't work here. Hibernate.initialize(domainTransactionRecords); Hibernate.initialize(nsHosts); Hibernate.initialize(dsDataHistories); Hibernate.initialize(gracePeriodHistories); - if (domainBase != null) { - domainBase.nsHosts = nullToEmptyImmutableCopy(nsHosts); - domainBase.gracePeriods = + if (resource != null) { + resource.nsHosts = nullToEmptyImmutableCopy(nsHosts); + resource.gracePeriods = gracePeriodHistories.stream() .map(GracePeriod::createFromHistory) .collect(toImmutableSet()); - domainBase.dsData = + resource.dsData = dsDataHistories.stream().map(DomainDsData::create).collect(toImmutableSet()); - // Normally Hibernate would see that the domain fields are all null and would fill - // domainBase with a null object. Unfortunately, the updateTimestamp is never null in SQL. - if (domainBase.getDomainName() == null) { - domainBase = null; - } else { - if (domainBase.getRepoId() == null) { - // domainBase still hasn't been fully constructed yet, so it's ok to go in and mutate - // it. In fact, we have to because going through the builder causes the hash codes of - // contained objects to be calculated prematurely. - domainBase.setRepoId(parent.getName()); - } - } } + processResourcePostLoad(); } private static void fillAuxiliaryFieldsFromDomain(DomainHistory domainHistory) { - if (domainHistory.domainBase != null) { - domainHistory.nsHosts = nullToEmptyImmutableCopy(domainHistory.domainBase.nsHosts); + DomainBase domainBase = domainHistory.resource; + if (domainBase != null) { + domainHistory.nsHosts = nullToEmptyImmutableCopy(domainBase.nsHosts); domainHistory.dsDataHistories = - nullToEmptyImmutableCopy(domainHistory.domainBase.getDsData()).stream() + nullToEmptyImmutableCopy(domainBase.getDsData()).stream() .filter(dsData -> dsData.getDigest() != null && dsData.getDigest().length > 0) - .map(dsData -> DomainDsDataHistory.createFrom(domainHistory.id, dsData)) + .map(dsData -> DomainDsDataHistory.createFrom(domainHistory.getRevisionId(), dsData)) .collect(toImmutableSet()); domainHistory.gracePeriodHistories = - nullToEmptyImmutableCopy(domainHistory.domainBase.getGracePeriods()).stream() - .map(gracePeriod -> GracePeriodHistory.createFrom(domainHistory.id, gracePeriod)) + nullToEmptyImmutableCopy(domainBase.getGracePeriods()).stream() + .map( + gracePeriod -> + GracePeriodHistory.createFrom(domainHistory.getRevisionId(), gracePeriod)) .collect(toImmutableSet()); } else { domainHistory.nsHosts = ImmutableSet.of(); } } - /** Class to represent the composite primary key of {@link DomainHistory} entity. */ - public static class DomainHistoryId extends ImmutableObject implements Serializable { - - private String domainRepoId; - - private Long id; - - /** Hibernate requires this default constructor. */ - private DomainHistoryId() {} - - public DomainHistoryId(String domainRepoId, long id) { - this.domainRepoId = domainRepoId; - this.id = id; - } - - /** Returns the domain repository id. */ - public String getDomainRepoId() { - return domainRepoId; - } - - /** Returns the history revision id. */ - public long getId() { - return id; - } - - /** - * Sets the domain repository id. - * - *

This method is private because it is only used by Hibernate and should not be used - * externally to keep immutability. - */ - @SuppressWarnings("unused") - private void setDomainRepoId(String domainRepoId) { - this.domainRepoId = domainRepoId; - } - - /** - * Sets the history revision id. - * - *

This method is private because it is only used by Hibernate and should not be used - * externally to keep immutability. - */ - @SuppressWarnings("unused") - private void setId(long id) { - this.id = id; - } - } - @Override public Builder asBuilder() { return new Builder(clone(this)); @@ -371,35 +256,30 @@ public class DomainHistory extends HistoryEntry { super(instance); } - public Builder setDomain(@Nullable DomainBase domainBase) { - // Nullable for the sake of pre-Registry-3.0 history objects - if (domainBase == null) { - return this; - } - // TODO(b/203609982): if actual type of domainBase is Domain, convert to DomainBase - // Note: a DomainHistory fetched by JPA has DomainBase in this field. Allowing Domain - // in the setter makes equality checks messy. - getInstance().domainBase = domainBase; - if (domainBase instanceof Domain) { - super.setParent(domainBase); - } else { - super.setParent(Key.create(Domain.class, domainBase.getRepoId())); - } + public Builder setDomain(DomainBase domainBase) { + getInstance().resource = domainBase; + return setRepoId(domainBase); + } + + public Builder setPeriod(Period period) { + getInstance().period = period; return this; } - public Builder setDomainRepoId(String domainRepoId) { - getInstance().parent = Key.create(Domain.class, domainRepoId); + public Builder setOtherRegistrarId(String otherRegistrarId) { + getInstance().otherRegistrarId = otherRegistrarId; + return this; + } + + public Builder setDomainTransactionRecords( + ImmutableSet domainTransactionRecords) { + getInstance().domainTransactionRecords = domainTransactionRecords; return this; } @Override public DomainHistory build() { DomainHistory instance = super.build(); - // TODO(b/171990736): Assert instance.domainBase is not null after database migration. - // Note that we cannot assert that instance.domainBase is not null here because this - // builder is also used to convert legacy HistoryEntry objects to DomainHistory, when - // domainBase is not available. fillAuxiliaryFieldsFromDomain(instance); return instance; } 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 cea377b37..118b9570e 100644 --- a/core/src/main/java/google/registry/model/domain/GracePeriod.java +++ b/core/src/main/java/google/registry/model/domain/GracePeriod.java @@ -21,8 +21,8 @@ import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.google.common.annotations.VisibleForTesting; 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.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.persistence.VKey; import javax.annotation.Nullable; import javax.persistence.Access; @@ -63,7 +63,8 @@ public class GracePeriod extends GracePeriodBase { @Nullable VKey billingEventOneTime, @Nullable VKey billingEventRecurring, @Nullable Long gracePeriodId) { - checkArgument((billingEventOneTime == null) || (billingEventRecurring == null), + checkArgument( + billingEventOneTime == null || billingEventRecurring == null, "A grace period can have at most one billing event"); checkArgument( (billingEventRecurring != null) == GracePeriodStatus.AUTO_RENEW.equals(type), @@ -176,18 +177,6 @@ public class GracePeriod extends GracePeriodBase { billingEvent.createVKey()); } - /** - * Returns a clone of this {@link GracePeriod} with {@link #domainRepoId} set to the given value - * and reconstructed history ids. - * - *

TODO(b/162739503): Remove this function after fully migrating to Cloud SQL. - */ - GracePeriod cloneAfterOfyLoad(String domainRepoId) { - GracePeriod clone = clone(this); - clone.domainRepoId = checkArgumentNotNull(domainRepoId); - return clone; - } - /** Entity class to represent a historic {@link GracePeriod}. */ @Entity(name = "GracePeriodHistory") @Table(indexes = @Index(columnList = "domainRepoId")) @@ -203,8 +192,8 @@ public class GracePeriod extends GracePeriodBase { return super.getGracePeriodId(); } - public DomainHistoryId getDomainHistoryId() { - return new DomainHistoryId(getDomainRepoId(), domainHistoryRevisionId); + public HistoryEntryId getHistoryEntryId() { + return new HistoryEntryId(getDomainRepoId(), domainHistoryRevisionId); } static GracePeriodHistory createFrom(long historyRevisionId, GracePeriod gracePeriod) { diff --git a/core/src/main/java/google/registry/model/domain/secdns/DomainDsDataHistory.java b/core/src/main/java/google/registry/model/domain/secdns/DomainDsDataHistory.java index 0c9ce94a8..212ddc35e 100644 --- a/core/src/main/java/google/registry/model/domain/secdns/DomainDsDataHistory.java +++ b/core/src/main/java/google/registry/model/domain/secdns/DomainDsDataHistory.java @@ -16,9 +16,8 @@ package google.registry.model.domain.secdns; import static google.registry.model.IdService.allocateId; -import google.registry.model.UnsafeSerializable; import google.registry.model.domain.DomainHistory; -import google.registry.model.domain.DomainHistory.DomainHistoryId; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import javax.persistence.Access; import javax.persistence.AccessType; import javax.persistence.Column; @@ -27,7 +26,7 @@ import javax.persistence.Id; /** Entity class to represent a historic {@link DomainDsData}. */ @Entity -public class DomainDsDataHistory extends DomainDsDataBase implements UnsafeSerializable { +public class DomainDsDataHistory extends DomainDsDataBase { @Id Long dsDataHistoryRevisionId; @@ -53,8 +52,8 @@ public class DomainDsDataHistory extends DomainDsDataBase implements UnsafeSeria return instance; } - public DomainHistory.DomainHistoryId getDomainHistoryId() { - return new DomainHistoryId(getDomainRepoId(), domainHistoryRevisionId); + public HistoryEntryId getHistoryEntryId() { + return new HistoryEntryId(getDomainRepoId(), domainHistoryRevisionId); } @Override diff --git a/core/src/main/java/google/registry/model/domain/token/AllocationToken.java b/core/src/main/java/google/registry/model/domain/token/AllocationToken.java index f57f8852f..5a7ec6f94 100644 --- a/core/src/main/java/google/registry/model/domain/token/AllocationToken.java +++ b/core/src/main/java/google/registry/model/domain/token/AllocationToken.java @@ -38,8 +38,7 @@ import google.registry.model.CreateAutoTimestamp; import google.registry.model.UpdateAutoTimestampEntity; import google.registry.model.billing.BillingEvent.RenewalPriceBehavior; import google.registry.model.common.TimedTransitionProperty; -import google.registry.model.reporting.HistoryEntry; -import google.registry.persistence.DomainHistoryVKey; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.persistence.VKey; import google.registry.persistence.WithVKey; import java.util.Optional; @@ -154,11 +153,9 @@ public class AllocationToken extends UpdateAutoTimestampEntity implements Builda @Nullable @AttributeOverrides({ @AttributeOverride(name = "repoId", column = @Column(name = "redemption_domain_repo_id")), - @AttributeOverride( - name = "historyRevisionId", - column = @Column(name = "redemption_domain_history_id")) + @AttributeOverride(name = "revisionId", column = @Column(name = "redemption_domain_history_id")) }) - DomainHistoryVKey redemptionHistoryEntry; + HistoryEntryId redemptionHistoryId; /** The fully-qualified domain name that this token is limited to, if any. */ @Nullable String domainName; @@ -212,13 +209,12 @@ public class AllocationToken extends UpdateAutoTimestampEntity implements Builda return token; } - public Optional> getRedemptionHistoryEntry() { - return Optional.ofNullable( - redemptionHistoryEntry == null ? null : redemptionHistoryEntry.createDomainHistoryVKey()); + public Optional getRedemptionHistoryId() { + return Optional.ofNullable(redemptionHistoryId); } public boolean isRedeemed() { - return redemptionHistoryEntry != null; + return redemptionHistoryId != null; } public Optional getDomainName() { @@ -309,7 +305,7 @@ public class AllocationToken extends UpdateAutoTimestampEntity implements Builda getInstance().domainName == null || TokenType.SINGLE_USE.equals(getInstance().tokenType), "Domain name can only be specified for SINGLE_USE tokens"); checkArgument( - getInstance().redemptionHistoryEntry == null + getInstance().redemptionHistoryId == null || TokenType.SINGLE_USE.equals(getInstance().tokenType), "Redemption history entry can only be specified for SINGLE_USE tokens"); checkArgument( @@ -345,10 +341,9 @@ public class AllocationToken extends UpdateAutoTimestampEntity implements Builda return this; } - public Builder setRedemptionHistoryEntry(VKey redemptionHistoryEntry) { - checkArgumentNotNull(redemptionHistoryEntry, "Redemption history entry must not be null"); - getInstance().redemptionHistoryEntry = - DomainHistoryVKey.create(redemptionHistoryEntry.getOfyKey()); + public Builder setRedemptionHistoryId(HistoryEntryId redemptionHistoryId) { + checkArgumentNotNull(redemptionHistoryId, "Redemption history entry ID must not be null"); + getInstance().redemptionHistoryId = redemptionHistoryId; return this; } 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 147b7e07f..96c7a6460 100644 --- a/core/src/main/java/google/registry/model/host/HostHistory.java +++ b/core/src/main/java/google/registry/model/host/HostHistory.java @@ -1,11 +1,8 @@ // Copyright 2020 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 -// +// 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. @@ -14,24 +11,18 @@ package google.registry.model.host; -import com.googlecode.objectify.Key; -import com.googlecode.objectify.annotation.EntitySubclass; import google.registry.model.EppResource; -import google.registry.model.ImmutableObject; -import google.registry.model.UnsafeSerializable; -import google.registry.model.host.HostHistory.HostHistoryId; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; -import java.io.Serializable; import java.util.Optional; import javax.annotation.Nullable; import javax.persistence.Access; import javax.persistence.AccessType; +import javax.persistence.AttributeOverride; import javax.persistence.Column; import javax.persistence.Entity; -import javax.persistence.Id; -import javax.persistence.IdClass; -import javax.persistence.PostLoad; +import javax.persistence.Index; +import javax.persistence.Table; /** * A persisted history entry representing an EPP modification to a host. @@ -39,54 +30,27 @@ import javax.persistence.PostLoad; *

In addition to the general history fields (e.g. action time, registrar ID) we also persist a * copy of the host entity at this point in time. We persist a raw {@link HostBase} so that the * foreign-keyed fields in that class can refer to this object. - * - *

This class is only marked as a Datastore entity subclass and registered with Objectify so that - * when building it its ID can be auto-populated by Objectify. It is converted to its superclass - * {@link HistoryEntry} when persisted to Datastore using {@link - * google.registry.persistence.transaction.TransactionManager}. */ @Entity -@javax.persistence.Table( +@Table( indexes = { - @javax.persistence.Index(columnList = "creationTime"), - @javax.persistence.Index(columnList = "historyRegistrarId"), - @javax.persistence.Index(columnList = "hostName"), - @javax.persistence.Index(columnList = "historyType"), - @javax.persistence.Index(columnList = "historyModificationTime") + @Index(columnList = "creationTime"), + @Index(columnList = "historyRegistrarId"), + @Index(columnList = "hostName"), + @Index(columnList = "historyType"), + @Index(columnList = "historyModificationTime") }) -@EntitySubclass @Access(AccessType.FIELD) -@IdClass(HostHistoryId.class) -public class HostHistory extends HistoryEntry implements UnsafeSerializable { +@AttributeOverride(name = "repoId", column = @Column(name = "hostRepoId")) +public class HostHistory extends HistoryEntry { - // Store HostBase instead of Host so we don't pick up its @Id - // Nullable for the sake of pre-Registry-3.0 history objects - @Nullable HostBase hostBase; + // Store HostBase instead of Host, so we don't pick up its @Id + // @Nullable for the sake of pre-Registry-3.0 history objects + @Nullable HostBase resource; - @Id - @Access(AccessType.PROPERTY) - public String getHostRepoId() { - // We need to handle null case here because Hibernate sometimes accesses this method before - // parent gets initialized - return parent == null ? null : parent.getName(); - } - - /** This method is private because it is only used by Hibernate. */ - @SuppressWarnings("unused") - private void setHostRepoId(String hostRepoId) { - parent = Key.create(Host.class, hostRepoId); - } - - @Id - @Column(name = "historyRevisionId") - @Access(AccessType.PROPERTY) @Override - public long getId() { - return super.getId(); - } - - public HostHistoryId getHostHistoryId() { - return new HostHistoryId(getHostRepoId(), getId()); + protected HostBase getResource() { + return resource; } /** @@ -96,19 +60,13 @@ public class HostHistory extends HistoryEntry implements UnsafeSerializable { *

Will be absent for objects created prior to the Registry 3.0 SQL migration. */ public Optional getHostBase() { - return Optional.ofNullable(hostBase); - } - - /** The key to the {@link Host} this is based off of. */ - public VKey getParentVKey() { - return VKey.create(Host.class, getHostRepoId()); + return Optional.ofNullable(resource); } /** Creates a {@link VKey} instance for this entity. */ - @SuppressWarnings("unchecked") @Override public VKey createVKey() { - return (VKey) createVKey(Key.create(this)); + return VKey.createSql(HostHistory.class, getHistoryEntryId()); } @Override @@ -116,82 +74,13 @@ public class HostHistory extends HistoryEntry implements UnsafeSerializable { return getHostBase().map(hostBase -> new Host.Builder().copyFrom(hostBase).build()); } - @PostLoad - void postLoad() { - // Normally Hibernate would see that the host fields are all null and would fill hostBase - // with a null object. Unfortunately, the updateTimestamp is never null in SQL. - if (hostBase != null && hostBase.getHostName() == null) { - hostBase = null; - } - if (hostBase != null && hostBase.getRepoId() == null) { - // hostBase hasn't been fully constructed yet, so it's ok to go in and mutate it. Though the - // use of the Builder is not necessarily problematic in this case, this is still safer as the - // Builder can do things like comparisons that compute the hash code. - hostBase.setRepoId(parent.getName()); - } - } - - /** Class to represent the composite primary key of {@link HostHistory} entity. */ - public static class HostHistoryId extends ImmutableObject implements Serializable { - - private String hostRepoId; - - private Long id; - - /** Hibernate requires this default constructor. */ - private HostHistoryId() {} - - public HostHistoryId(String hostRepoId, long id) { - this.hostRepoId = hostRepoId; - this.id = id; - } - - /** - * Returns the host repository id. - * - *

This method is private because it is only used by Hibernate. - */ - public String getHostRepoId() { - return hostRepoId; - } - - /** - * Returns the history revision id. - * - *

This method is private because it is only used by Hibernate. - */ - public long getId() { - return id; - } - - /** - * Sets the host repository id. - * - *

This method is private because it is only used by Hibernate and should not be used - * externally to keep immutability. - */ - @SuppressWarnings("unused") - private void setHostRepoId(String hostRepoId) { - this.hostRepoId = hostRepoId; - } - - /** - * Sets the history revision id. - * - *

This method is private because it is only used by Hibernate and should not be used - * externally to keep immutability. - */ - @SuppressWarnings("unused") - private void setId(long id) { - this.id = id; - } - } - @Override public Builder asBuilder() { return new Builder(clone(this)); } + + public static class Builder extends HistoryEntry.Builder { public Builder() {} @@ -200,18 +89,9 @@ public class HostHistory extends HistoryEntry implements UnsafeSerializable { super(instance); } - public Builder setHost(@Nullable HostBase hostBase) { - // Nullable for the sake of pre-Registry-3.0 history objects - if (hostBase == null) { - return this; - } - getInstance().hostBase = hostBase; - return super.setParent(hostBase); - } - - public Builder setHostRepoId(String hostRepoId) { - getInstance().parent = Key.create(Host.class, hostRepoId); - return this; + public Builder setHost(HostBase hostBase) { + getInstance().resource = hostBase; + return setRepoId(hostBase); } } } diff --git a/core/src/main/java/google/registry/model/ofy/ObjectifyService.java b/core/src/main/java/google/registry/model/ofy/ObjectifyService.java index bb7a1fd89..e7c2310c9 100644 --- a/core/src/main/java/google/registry/model/ofy/ObjectifyService.java +++ b/core/src/main/java/google/registry/model/ofy/ObjectifyService.java @@ -40,7 +40,6 @@ import google.registry.model.translators.BloomFilterOfStringTranslatorFactory; import google.registry.model.translators.CidrAddressBlockTranslatorFactory; import google.registry.model.translators.CurrencyUnitTranslatorFactory; import google.registry.model.translators.DurationTranslatorFactory; -import google.registry.model.translators.EppHistoryVKeyTranslatorFactory; import google.registry.model.translators.InetAddressTranslatorFactory; import google.registry.model.translators.ReadableInstantUtcTranslatorFactory; import google.registry.model.translators.VKeyTranslatorFactory; @@ -118,7 +117,6 @@ public class ObjectifyService { new CidrAddressBlockTranslatorFactory(), new CurrencyUnitTranslatorFactory(), new DurationTranslatorFactory(), - new EppHistoryVKeyTranslatorFactory(), new InetAddressTranslatorFactory(), new MoneyStringTranslatorFactory(), new ReadableInstantUtcTranslatorFactory(), 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 ecff003e1..2656aff18 100644 --- a/core/src/main/java/google/registry/model/poll/PollMessage.java +++ b/core/src/main/java/google/registry/model/poll/PollMessage.java @@ -28,19 +28,17 @@ import google.registry.model.annotations.ExternalMessagingName; import google.registry.model.annotations.OfyIdAllocation; import google.registry.model.contact.Contact; import google.registry.model.contact.ContactHistory; -import google.registry.model.contact.ContactHistory.ContactHistoryId; import google.registry.model.domain.Domain; 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.Host; import google.registry.model.host.HostHistory; -import google.registry.model.host.HostHistory.HostHistoryId; import google.registry.model.poll.PendingActionNotificationResponse.ContactPendingActionNotificationResponse; import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse; import google.registry.model.poll.PendingActionNotificationResponse.HostPendingActionNotificationResponse; import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.transfer.TransferData.TransferServerApproveEntity; import google.registry.model.transfer.TransferResponse; import google.registry.model.transfer.TransferResponse.ContactTransferResponse; @@ -197,10 +195,10 @@ public abstract class PollMessage extends ImmutableObject } /** - * Gets the name of the underlying resource that the PollMessage is for, regardless of the type of - * the resource. + * Gets the repo ID of the underlying resource that the PollMessage is for, regardless of the type + * of the resource. */ - public String getResourceName() { + public String getResourceId() { return domainRepoId != null ? domainRepoId : contactRepoId != null ? contactRepoId : hostRepoId; } @@ -262,34 +260,35 @@ public abstract class PollMessage extends ImmutableObject return thisCastToDerived(); } - public B setDomainHistoryId(DomainHistoryId historyId) { - getInstance().domainRepoId = historyId.getDomainRepoId(); - getInstance().domainHistoryRevisionId = historyId.getId(); + public B setDomainHistoryId(HistoryEntryId historyId) { + getInstance().domainRepoId = historyId.getRepoId(); + getInstance().domainHistoryRevisionId = historyId.getRevisionId(); return thisCastToDerived(); } - public B setContactHistoryId(ContactHistoryId historyId) { - getInstance().contactRepoId = historyId.getContactRepoId(); - getInstance().contactHistoryRevisionId = historyId.getId(); + public B setContactHistoryId(HistoryEntryId historyId) { + getInstance().contactRepoId = historyId.getRepoId(); + getInstance().contactHistoryRevisionId = historyId.getRevisionId(); return thisCastToDerived(); } - public B setHostHistoryId(HostHistoryId historyId) { - getInstance().hostRepoId = historyId.getHostRepoId(); - getInstance().hostHistoryRevisionId = historyId.getId(); + public B setHostHistoryId(HistoryEntryId historyId) { + getInstance().hostRepoId = historyId.getRepoId(); + getInstance().hostHistoryRevisionId = historyId.getRevisionId(); return thisCastToDerived(); } public B setHistoryEntry(HistoryEntry history) { + HistoryEntryId historyId = history.getHistoryEntryId(); // Set the appropriate field based on the history entry type. if (history instanceof DomainHistory) { - return setDomainHistoryId(((DomainHistory) history).getDomainHistoryId()); + return setDomainHistoryId(historyId); } if (history instanceof ContactHistory) { - return setContactHistoryId(((ContactHistory) history).getContactHistoryId()); + return setContactHistoryId(historyId); } if (history instanceof HostHistory) { - return setHostHistoryId(((HostHistory) history).getHostHistoryId()); + return setHostHistoryId(historyId); } return thisCastToDerived(); } diff --git a/core/src/main/java/google/registry/model/reporting/DomainTransactionRecord.java b/core/src/main/java/google/registry/model/reporting/DomainTransactionRecord.java index 25eb0cb9a..f8b9cecbf 100644 --- a/core/src/main/java/google/registry/model/reporting/DomainTransactionRecord.java +++ b/core/src/main/java/google/registry/model/reporting/DomainTransactionRecord.java @@ -21,7 +21,7 @@ import com.google.common.collect.ImmutableSet; import google.registry.model.Buildable; import google.registry.model.ImmutableObject; import google.registry.model.UnsafeSerializable; -import google.registry.model.domain.DomainHistory.DomainHistoryId; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.EnumType; @@ -48,6 +48,7 @@ public class DomainTransactionRecord extends ImmutableObject @Id @GeneratedValue(strategy = GenerationType.IDENTITY) @Insignificant + @SuppressWarnings("unused") Long id; /** The TLD this record operates on. */ @@ -66,8 +67,8 @@ public class DomainTransactionRecord extends ImmutableObject * The time this Transaction takes effect (counting grace periods and other nuances). * *

Net adds, renews and transfers are modificationTime + 5 days for the grace period, while - * Autorenews have a 45 day grace period. For deletions, this is the purge date of the domain. And - * for restored names, this is the modificationTime, if done in the 30 day redemption period. + * Autorenews have a 45-day grace period. For deletions, this is the purge date of the domain. And + * for restored names, this is the modificationTime, if done in the 30-day redemption period. * * @see @@ -178,8 +179,8 @@ public class DomainTransactionRecord extends ImmutableObject } } - public DomainHistoryId getDomainHistoryId() { - return new DomainHistoryId(domainRepoId, historyRevisionId); + public HistoryEntryId getHistoryEntryId() { + return new HistoryEntryId(domainRepoId, historyRevisionId); } public DateTime getReportingTime() { diff --git a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java b/core/src/main/java/google/registry/model/reporting/HistoryEntry.java index 37c425172..24827305d 100644 --- a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java +++ b/core/src/main/java/google/registry/model/reporting/HistoryEntry.java @@ -15,75 +15,49 @@ package google.registry.model.reporting; import static com.google.common.base.Preconditions.checkArgument; -import static com.googlecode.objectify.Key.getKind; -import static google.registry.util.CollectionUtils.nullToEmpty; -import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; -import com.googlecode.objectify.annotation.Entity; -import com.googlecode.objectify.annotation.Id; -import com.googlecode.objectify.annotation.Ignore; -import com.googlecode.objectify.annotation.Index; -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.ReportedOn; -import google.registry.model.contact.Contact; +import google.registry.model.annotations.OfyIdAllocation; import google.registry.model.contact.ContactBase; import google.registry.model.contact.ContactHistory; -import google.registry.model.contact.ContactHistory.ContactHistoryId; -import google.registry.model.domain.Domain; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainHistory; -import google.registry.model.domain.DomainHistory.DomainHistoryId; -import google.registry.model.domain.Period; import google.registry.model.eppcommon.Trid; -import google.registry.model.host.Host; import google.registry.model.host.HostBase; import google.registry.model.host.HostHistory; -import google.registry.model.host.HostHistory.HostHistoryId; -import google.registry.persistence.VKey; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import java.util.Optional; -import java.util.Set; import javax.annotation.Nullable; import javax.persistence.Access; import javax.persistence.AccessType; import javax.persistence.AttributeOverride; import javax.persistence.AttributeOverrides; import javax.persistence.Column; +import javax.persistence.Embeddable; import javax.persistence.EnumType; import javax.persistence.Enumerated; +import javax.persistence.Id; +import javax.persistence.IdClass; import javax.persistence.MappedSuperclass; -import javax.persistence.Transient; +import javax.persistence.PostLoad; import org.apache.commons.lang3.BooleanUtils; -import org.hibernate.collection.internal.PersistentSet; import org.joda.time.DateTime; /** * A record of an EPP command that mutated a resource. * - *

Due to historical reasons this class is persisted only to Datastore. It has three subclasses - * that include the parent resource itself which are persisted to Cloud SQL. During migration this - * class cannot be made abstract in order for the class to be persisted and loaded to and from - * Datastore. However it should never be used directly in the Java code itself. When it is loaded - * from Datastore it should be converted to a subclass for handling and when a new history entry is - * built it should always be a subclass, which is automatically converted to HistoryEntry when - * persisting to Datastore. - * - *

Some care has been taken to make it close to impossible to use this class directly, but the - * user should still exercise caution. After the migration is complete this class will be made - * abstract. + *

This abstract class has three subclasses that include the parent resource itself and are + * persisted to Cloud SQL. */ -@ReportedOn -@Entity @MappedSuperclass @Access(AccessType.FIELD) -public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSerializable { +@IdClass(HistoryEntryId.class) +public abstract class HistoryEntry extends ImmutableObject + implements Buildable, UnsafeSerializable { /** Represents the type of history entry. */ public enum Type { @@ -127,36 +101,34 @@ public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSe /** Resource was created by an escrow file import. */ RDE_IMPORT, /** - * A synthetic history entry created by a tool or back-end migration script outside of the scope - * of usual EPP flows. These are sometimes needed to serve as parents for billing events or poll + * A synthetic history entry created by a tool or back-end migration script outside the scope of + * usual EPP flows. These are sometimes needed to serve as parents for billing events or poll * messages that otherwise wouldn't have a suitable parent. */ SYNTHETIC } - /** - * The autogenerated id of this event. Note that, this field is marked as {@link Transient} in the - * SQL schema, this is because the child class of {@link HistoryEntry}, e.g. {@link - * DomainHistory}, uses a composite primary key which the id is part of, and Hibernate requires - * that all the {@link javax.persistence.Id} fields must be put in the exact same class. - */ - @Id @Transient @VisibleForTesting public Long id; + /** The autogenerated id of this event. */ + @Id + @OfyIdAllocation + @Column(nullable = false, name = "historyRevisionId") + protected Long revisionId; - /** The resource this event mutated. */ - @Parent @Transient protected Key parent; + /** + * The repo ID of the embedded {@link EppResource} that this event mutated. + * + *

Note that the embedded EPP resource is of a base type for which the repo ID field is + * {@code @Transient}, which is NOT persisted as part of the embedded entity. After a {@link + * HistoryEntry} is loaded from SQL, the {@link #postLoad()} methods re-populates the field inside + * the EPP resource. + */ + @Id protected String repoId; /** The type of history entry. */ @Column(nullable = false, name = "historyType") @Enumerated(EnumType.STRING) Type type; - /** - * The length of time that a create, allocate, renewal, or transfer request was issued for. Will - * be null for all other types. - */ - @Ignore @Transient // domain-specific - Period period; - /** * The actual EPP xml of the command, stored as bytes to be agnostic of encoding. * @@ -166,28 +138,15 @@ public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSe byte[] xmlBytes; /** The time the command occurred, represented by the ofy transaction time. */ - @Index @Column(nullable = false, name = "historyModificationTime") DateTime modificationTime; /** The id of the registrar that sent the command. */ - @Index @Column(name = "historyRegistrarId") String clientId; - /** - * For transfers, the id of the other registrar. - * - *

For requests and cancels, the other registrar is the losing party (because the registrar - * sending the EPP transfer command is the gaining party). For approves and rejects, the other - * registrar is the gaining party. - */ - @Transient // domain-specific - String otherClientId; - /** Transaction id that made this change, or null if the entry was not created by a flow. */ @Nullable - @Ignore @AttributeOverrides({ @AttributeOverride( name = "clientTransactionId", @@ -210,51 +169,33 @@ public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSe @Column(name = "historyRequestedByRegistrar") Boolean requestedByRegistrar; - /** - * Logging field for transaction reporting. - * - *

This will be empty for any HistoryEntry generated before this field was added. This will - * also be empty if the HistoryEntry refers to an EPP mutation that does not affect domain - * transaction counts (such as contact or host mutations). - */ - @Ignore - @Transient // domain-specific - @ImmutableObject.EmptySetToNull - protected Set domainTransactionRecords; - - // Make it impossible to instantiate a HistoryEntry explicitly. One should only instantiate a - // subtype of HistoryEntry. - protected HistoryEntry() { - super(); - } - - public long getId() { - // For some reason, Hibernate throws NPE during some initialization phase if we don't deal with + public long getRevisionId() { + // For some reason, Hibernate throws NPE during some initialization phases if we don't deal with // the null case. Setting the id to 0L when it is null should be fine because 0L for primitive // type is considered as null for wrapper class in the Hibernate context. - return id == null ? 0L : id; + return revisionId == null ? 0L : revisionId; } - /** This method exists solely to satisfy Hibernate. Use the {@link Builder} instead. */ - @SuppressWarnings("UnusedMethod") - private void setId(long id) { - this.id = id; + protected abstract EppResource getResource(); + + public Class getResourceType() { + return getResource().getClass(); } - public Key getParent() { - return parent; + public String getRepoId() { + return repoId; + } + + public HistoryEntryId getHistoryEntryId() { + return new HistoryEntryId(repoId, revisionId); } public Type getType() { return type; } - public Period getPeriod() { - return period; - } - public byte[] getXmlBytes() { - return xmlBytes; + return xmlBytes == null ? null : xmlBytes.clone(); } public DateTime getModificationTime() { @@ -265,10 +206,6 @@ public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSe return clientId; } - public String getOtherRegistrarId() { - return otherClientId; - } - /** Returns the TRID, which may be null if the entry was not created by a normal flow. */ @Nullable public Trid getTrid() { @@ -287,146 +224,36 @@ public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSe return requestedByRegistrar; } - public Set getDomainTransactionRecords() { - return nullToEmptyImmutableCopy(domainTransactionRecords); - } + public abstract Optional getResourceAtPointInTime(); - /** - * Throws an error when attempting to retrieve the EppResource at this point in time. - * - *

Subclasses must override this to return the resource; it is non-abstract for legacy reasons - * and objects created prior to the Registry 3.0 migration. - */ - public Optional getResourceAtPointInTime() { - throw new UnsupportedOperationException( - "Raw HistoryEntry objects do not store the resource at that point in time."); - } - - /** This method exists solely to satisfy Hibernate. Use the {@link Builder} instead. */ - @SuppressWarnings("UnusedMethod") - private void setPeriod(Period period) { - this.period = period; - } - - /** This method exists solely to satisfy Hibernate. Use the {@link Builder} instead. */ - @SuppressWarnings("UnusedMethod") - private void setOtherRegistrarId(String otherRegistrarId) { - this.otherClientId = otherRegistrarId; - } - - /** This method exists solely to satisfy Hibernate. Use the {@link Builder} instead. */ - @SuppressWarnings("UnusedMethod") - protected void setDomainTransactionRecords( - Set domainTransactionRecords) { - // Note: how we wish to treat this Hibernate setter depends on the current state of the object - // and what's passed in. The key principle is that we wish to maintain the link between parent - // and child objects, meaning that we should keep around whichever of the two sets (the - // parameter vs the class variable) and clear/populate that as appropriate. - // - // If the class variable is a PersistentSet and we overwrite it here, Hibernate will throw - // an exception "A collection with cascade=”all-delete-orphan” was no longer referenced by the - // owning entity instance". See https://stackoverflow.com/questions/5587482 for more details. - if (this.domainTransactionRecords instanceof PersistentSet) { - Set nonNullRecords = nullToEmpty(domainTransactionRecords); - this.domainTransactionRecords.retainAll(nonNullRecords); - this.domainTransactionRecords.addAll(nonNullRecords); - } else { - this.domainTransactionRecords = domainTransactionRecords; + protected void processResourcePostLoad() { + // Post-Registry 3.0 entity should always have the resource field, whereas pre-Registry 3.0 + // will return a null resource. + if (getResource() != null && getResource().getRepoId() == null) { + // The repoId field in EppResource is transient, so we go ahead and set it to the value read + // from SQL. + getResource().setRepoId(repoId); } } - /** - * Throws an error when trying to get a builder from a bare {@link HistoryEntry}. - * - *

This method only exists to satisfy the requirement that the {@link HistoryEntry} is NOT - * abstract, it should never be called directly and all three of the subclass of {@link - * HistoryEntry} implements it. - */ - @Override - public Builder asBuilder() { - throw new UnsupportedOperationException( - "You should never attempt to build a HistoryEntry from a raw HistoryEntry. A raw " - + "HistoryEntry should only exist internally when persisting to datastore. If you need " - + "to build from a raw HistoryEntry, use " - + "{Contact,Host,Domain}History.Builder.copyFrom(HistoryEntry) instead."); + @PostLoad + protected void postLoad() { + processResourcePostLoad(); } - /** - * Clones and returns a {@code HistoryEntry} objec - * - *

This is useful when converting a subclass to the base class to persist to Datastore. - */ - public HistoryEntry asHistoryEntry() { - HistoryEntry historyEntry = new HistoryEntry(); - copy(this, historyEntry); - return historyEntry; - } + @Override + public abstract Builder asBuilder(); protected static void copy(HistoryEntry src, HistoryEntry dst) { - dst.id = src.id; - dst.parent = src.parent; + dst.revisionId = src.revisionId; dst.type = src.type; - dst.period = src.period; dst.xmlBytes = src.xmlBytes; dst.modificationTime = src.modificationTime; dst.clientId = src.clientId; - dst.otherClientId = src.otherClientId; dst.trid = src.trid; dst.bySuperuser = src.bySuperuser; dst.reason = src.reason; dst.requestedByRegistrar = src.requestedByRegistrar; - dst.domainTransactionRecords = - src.domainTransactionRecords == null - ? null - : ImmutableSet.copyOf(src.domainTransactionRecords); - } - - @SuppressWarnings("unchecked") - public HistoryEntry toChildHistoryEntity() { - String parentKind = getParent().getKind(); - final HistoryEntry resultEntity; - // can't use a switch statement since we're calling getKind() - if (parentKind.equals(getKind(Domain.class))) { - resultEntity = - new DomainHistory.Builder().copyFrom(this).setDomainRepoId(parent.getName()).build(); - } else if (parentKind.equals(getKind(Host.class))) { - resultEntity = - new HostHistory.Builder().copyFrom(this).setHostRepoId(parent.getName()).build(); - } else if (parentKind.equals(getKind(Contact.class))) { - resultEntity = - new ContactHistory.Builder().copyFrom(this).setContactRepoId(parent.getName()).build(); - } else { - throw new IllegalStateException( - String.format("Unknown kind of HistoryEntry parent %s", parentKind)); - } - return resultEntity; - } - - /** Creates a {@link VKey} instance from a {@link Key} instance. */ - public static VKey createVKey(Key key) { - String repoId = key.getParent().getName(); - long id = key.getId(); - Key parent = key.getParent(); - String parentKind = parent.getKind(); - if (parentKind.equals(getKind(Domain.class))) { - return VKey.create( - DomainHistory.class, - new DomainHistoryId(repoId, id), - Key.create(parent, DomainHistory.class, id)); - } else if (parentKind.equals(getKind(Host.class))) { - return VKey.create( - HostHistory.class, - new HostHistoryId(repoId, id), - Key.create(parent, HostHistory.class, id)); - } else if (parentKind.equals(getKind(Contact.class))) { - return VKey.create( - ContactHistory.class, - new ContactHistoryId(repoId, id), - Key.create(parent, ContactHistory.class, id)); - } else { - throw new IllegalStateException( - String.format("Unknown kind of HistoryEntry parent %s", parentKind)); - } } /** A builder for {@link HistoryEntry} since it is immutable */ @@ -451,7 +278,8 @@ public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSe @Override public T build() { - // TODO(mcilwain): Add null checking for id/parent once DB migration is complete. + checkArgumentNotNull(getInstance().getResource(), "EPP resource must be specified"); + checkArgumentNotNull(getInstance().repoId, "repoId must be specified"); checkArgumentNotNull(getInstance().type, "History entry type must be specified"); checkArgumentNotNull(getInstance().modificationTime, "Modification time must be specified"); checkArgumentNotNull(getInstance().clientId, "Registrar ID must be specified"); @@ -462,19 +290,15 @@ public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSe return super.build(); } - public B setId(Long id) { - getInstance().id = id; + public B setRevisionId(Long revisionId) { + getInstance().revisionId = revisionId; return thisCastToDerived(); } - protected B setParent(EppResource parent) { - getInstance().parent = Key.create(parent); - return thisCastToDerived(); - } - - // Until we move completely to SQL, override this in subclasses (e.g. HostHistory) to set VKeys - protected B setParent(Key parent) { - getInstance().parent = parent; + protected B setRepoId(EppResource eppResource) { + if (eppResource != null) { + getInstance().repoId = eppResource.getRepoId(); + } return thisCastToDerived(); } @@ -483,13 +307,8 @@ public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSe return thisCastToDerived(); } - public B setPeriod(Period period) { - getInstance().period = period; - return thisCastToDerived(); - } - public B setXmlBytes(byte[] xmlBytes) { - getInstance().xmlBytes = xmlBytes; + getInstance().xmlBytes = xmlBytes == null ? null : xmlBytes.clone(); return thisCastToDerived(); } @@ -503,11 +322,6 @@ public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSe return thisCastToDerived(); } - public B setOtherRegistrarId(String otherRegistrarId) { - getInstance().otherClientId = otherRegistrarId; - return thisCastToDerived(); - } - public B setTrid(Trid trid) { getInstance().trid = trid; return thisCastToDerived(); @@ -527,12 +341,6 @@ public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSe getInstance().requestedByRegistrar = requestedByRegistrar; return thisCastToDerived(); } - - public B setDomainTransactionRecords( - ImmutableSet domainTransactionRecords) { - getInstance().setDomainTransactionRecords(domainTransactionRecords); - return thisCastToDerived(); - } } public static @@ -549,4 +357,41 @@ public class HistoryEntry extends ImmutableObject implements Buildable, UnsafeSe "Class %s does not have an associated HistoryEntry", parent.getClass().getName())); } } + + /** Class to represent the composite primary key of a {@link HistoryEntry}. */ + @Embeddable + @Access(AccessType.PROPERTY) + public static class HistoryEntryId extends ImmutableObject implements UnsafeSerializable { + + private String repoId; + + private long revisionId; + + protected HistoryEntryId() {} + + public HistoryEntryId(String repoId, long revisionId) { + this.repoId = repoId; + this.revisionId = revisionId; + } + + /** Returns the {@code history_revision_id} of the {@link HistoryEntry}. */ + public long getRevisionId() { + return revisionId; + } + + @SuppressWarnings("unused") + private void setRevisionId(long revisionId) { + this.revisionId = revisionId; + } + + /** Returns the {@code [domain|contact|host]_repo_id} of the {@link HistoryEntry}. */ + public String getRepoId() { + return repoId; + } + + @SuppressWarnings("unused") + private void setRepoId(String repoId) { + this.repoId = repoId; + } + } } diff --git a/core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java b/core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java index 3cf03b29b..8c00edf39 100644 --- a/core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java +++ b/core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java @@ -40,12 +40,7 @@ import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; import org.joda.time.DateTime; -/** - * Retrieves {@link HistoryEntry} descendants (e.g. {@link DomainHistory}). - * - *

This class is configured to retrieve either from Datastore or SQL, depending on which database - * is currently considered the primary database. - */ +/** Retrieves {@link HistoryEntry} descendants (e.g. {@link DomainHistory}). */ public class HistoryEntryDao { public static ImmutableMap, Class> @@ -58,15 +53,6 @@ public class HistoryEntryDao { Host.class, HostHistory.class); - public static ImmutableMap, String> REPO_ID_FIELD_NAMES = - ImmutableMap.of( - ContactHistory.class, - "contactRepoId", - DomainHistory.class, - "domainRepoId", - HostHistory.class, - "hostRepoId"); - /** Loads all history objects in the times specified, including all types. */ public static ImmutableList loadAllHistoryObjects( DateTime afterTime, DateTime beforeTime) { @@ -74,59 +60,58 @@ public class HistoryEntryDao { .transact( () -> new ImmutableList.Builder() - .addAll( - loadAllHistoryObjectsFromSql(ContactHistory.class, afterTime, beforeTime)) - .addAll( - loadAllHistoryObjectsFromSql(DomainHistory.class, afterTime, beforeTime)) - .addAll(loadAllHistoryObjectsFromSql(HostHistory.class, afterTime, beforeTime)) + .addAll(loadAllHistoryObjects(ContactHistory.class, afterTime, beforeTime)) + .addAll(loadAllHistoryObjects(DomainHistory.class, afterTime, beforeTime)) + .addAll(loadAllHistoryObjects(HostHistory.class, afterTime, beforeTime)) .build()); } /** Loads all history objects corresponding to the given {@link EppResource}. */ public static ImmutableList loadHistoryObjectsForResource( - VKey parentKey) { - return loadHistoryObjectsForResource(parentKey, START_OF_TIME, END_OF_TIME); + VKey resourceKey) { + return loadHistoryObjectsForResource(resourceKey, START_OF_TIME, END_OF_TIME); } /** - * Loads all history objects corresponding to the given {@link EppResource} and casted to the + * Loads all history objects corresponding to the given {@link EppResource} and cast to the * appropriate subclass. */ public static ImmutableList loadHistoryObjectsForResource( - VKey parentKey, Class subclazz) { - return loadHistoryObjectsForResource(parentKey, START_OF_TIME, END_OF_TIME, subclazz); + VKey resourceKey, Class subclazz) { + return loadHistoryObjectsForResource(resourceKey, START_OF_TIME, END_OF_TIME, subclazz); } /** Loads all history objects in the time period specified for the given {@link EppResource}. */ public static ImmutableList loadHistoryObjectsForResource( - VKey parentKey, DateTime afterTime, DateTime beforeTime) { + VKey resourceKey, DateTime afterTime, DateTime beforeTime) { return jpaTm() - .transact(() -> loadHistoryObjectsForResourceFromSql(parentKey, afterTime, beforeTime)); + .transact(() -> loadHistoryObjectsForResourceInternal(resourceKey, afterTime, beforeTime)); } /** * Loads all history objects in the time period specified for the given {@link EppResource} and - * casted to the appropriate subclass. + * cast to the appropriate subclass. * - *

Note that the subclass must be explicitly provided because we need the compile time - * information of T to return an {@code ImmutableList}, even though at runtime we can call - * {@link #getHistoryClassFromParent(Class)} to obtain it, which we also did to confirm that the - * provided subclass is indeed correct. + *

Note that the subclass must be explicitly provided because we need compile time information + * of T to return an {@code ImmutableList}, even though at runtime we can call {@link + * #getHistoryClassFromParent(Class)} to obtain it, which we also did to confirm that the provided + * subclass is indeed correct. */ - public static ImmutableList loadHistoryObjectsForResource( - VKey parentKey, + private static ImmutableList loadHistoryObjectsForResource( + VKey resourceKey, DateTime afterTime, DateTime beforeTime, Class subclazz) { - Class expectedSubclazz = getHistoryClassFromParent(parentKey.getKind()); + Class expectedSubclazz = + getHistoryClassFromParent(resourceKey.getKind()); checkArgument( subclazz.equals(expectedSubclazz), "The supplied HistoryEntry subclass %s is incompatible with the EppResource %s, " + "use %s instead", subclazz.getSimpleName(), - parentKey.getKind().getSimpleName(), + resourceKey.getKind().getSimpleName(), expectedSubclazz.getSimpleName()); - return loadHistoryObjectsForResource(parentKey, afterTime, beforeTime).stream() + return loadHistoryObjectsForResource(resourceKey, afterTime, beforeTime).stream() .map(subclazz::cast) .collect(toImmutableList()); } @@ -138,14 +123,14 @@ public class HistoryEntryDao { .transact( () -> Streams.concat( - loadHistoryObjectFromSqlByRegistrars(ContactHistory.class, registrarIds), - loadHistoryObjectFromSqlByRegistrars(DomainHistory.class, registrarIds), - loadHistoryObjectFromSqlByRegistrars(HostHistory.class, registrarIds)) + loadHistoryObjectByRegistrarsInternal(ContactHistory.class, registrarIds), + loadHistoryObjectByRegistrarsInternal(DomainHistory.class, registrarIds), + loadHistoryObjectByRegistrarsInternal(HostHistory.class, registrarIds)) .sorted(Comparator.comparing(HistoryEntry::getModificationTime)) .collect(toImmutableList())); } - private static Stream loadHistoryObjectFromSqlByRegistrars( + private static Stream loadHistoryObjectByRegistrarsInternal( Class historyClass, ImmutableCollection registrarIds) { return jpaTm() .criteriaQuery( @@ -155,45 +140,33 @@ public class HistoryEntryDao { .getResultStream(); } - private static ImmutableList loadHistoryObjectsForResourceFromSql( - VKey parentKey, DateTime afterTime, DateTime beforeTime) { - // The class we're searching from is based on which parent type (e.g. Domain) we have - Class historyClass = getHistoryClassFromParent(parentKey.getKind()); - // The field representing repo ID unfortunately varies by history class - String repoIdFieldName = getRepoIdFieldNameFromHistoryClass(historyClass); + private static ImmutableList loadHistoryObjectsForResourceInternal( + VKey resourceKey, DateTime afterTime, DateTime beforeTime) { + // The class we're searching from is based on which resource type (e.g. Domain) we have + Class historyClass = getHistoryClassFromParent(resourceKey.getKind()); CriteriaBuilder criteriaBuilder = jpaTm().getEntityManager().getCriteriaBuilder(); CriteriaQuery criteriaQuery = CriteriaQueryBuilder.create(historyClass) .where("modificationTime", criteriaBuilder::greaterThanOrEqualTo, afterTime) .where("modificationTime", criteriaBuilder::lessThanOrEqualTo, beforeTime) - .where(repoIdFieldName, criteriaBuilder::equal, parentKey.getSqlKey().toString()) - .orderByAsc("id") + .where("repoId", criteriaBuilder::equal, resourceKey.getSqlKey().toString()) + .orderByAsc("revisionId") + .orderByAsc("modificationTime") .build(); - return ImmutableList.sortedCopyOf( - Comparator.comparing(HistoryEntry::getModificationTime), - jpaTm().criteriaQuery(criteriaQuery).getResultList()); + return ImmutableList.copyOf(jpaTm().criteriaQuery(criteriaQuery).getResultList()); } public static Class getHistoryClassFromParent( - Class parent) { - if (!RESOURCE_TYPES_TO_HISTORY_TYPES.containsKey(parent)) { + Class resourceType) { + if (!RESOURCE_TYPES_TO_HISTORY_TYPES.containsKey(resourceType)) { throw new IllegalArgumentException( - String.format("Unknown history type for parent %s", parent.getName())); + String.format("Unknown history type for resourceType %s", resourceType.getName())); } - return RESOURCE_TYPES_TO_HISTORY_TYPES.get(parent); + return RESOURCE_TYPES_TO_HISTORY_TYPES.get(resourceType); } - public static String getRepoIdFieldNameFromHistoryClass( - Class historyClass) { - if (!REPO_ID_FIELD_NAMES.containsKey(historyClass)) { - throw new IllegalArgumentException( - String.format("Unknown history type %s", historyClass.getName())); - } - return REPO_ID_FIELD_NAMES.get(historyClass); - } - - private static List loadAllHistoryObjectsFromSql( + private static List loadAllHistoryObjects( Class historyClass, DateTime afterTime, DateTime beforeTime) { CriteriaBuilder criteriaBuilder = jpaTm().getEntityManager().getCriteriaBuilder(); return jpaTm() diff --git a/core/src/main/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactory.java b/core/src/main/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactory.java deleted file mode 100644 index dabf12850..000000000 --- a/core/src/main/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactory.java +++ /dev/null @@ -1,107 +0,0 @@ -// Copyright 2020 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.model.translators; - -import static com.google.common.collect.ImmutableMap.toImmutableMap; -import static java.util.function.Function.identity; - -import com.google.appengine.api.datastore.Key; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import google.registry.persistence.DomainHistoryVKey; -import google.registry.persistence.EppHistoryVKey; -import java.lang.reflect.Constructor; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import javax.annotation.Nullable; - -/** Translator factory for {@link EppHistoryVKey}. */ -public class EppHistoryVKeyTranslatorFactory - extends AbstractSimpleTranslatorFactory { - - public EppHistoryVKeyTranslatorFactory() { - super(EppHistoryVKey.class); - } - - // This map is used when we need to convert the raw Datastore key to its VKey instance. We have - // one dedicated VKey class, e.g. DomainHistoryVKey, for each such kind of entity, and we need - // a way to map the raw Datastore key to its VKey class. So, we use the kind path as the key of - // the map, and the kind path is created by concatenating all the kind strings in a raw Datastore - // key, e.g. the map key for ContactPollMessageVKey is "Contact/HistoryEntry/PollMessage". - @VisibleForTesting - static final ImmutableMap> kindPathToVKeyClass = - ImmutableSet.of(DomainHistoryVKey.class).stream() - .collect(toImmutableMap(EppHistoryVKeyTranslatorFactory::getKindPath, identity())); - - /** - * Gets the kind path string for the given {@link Class}. - * - *

This method calls the getKindPath method on an instance of the given {@link Class} to get - * the kind path string. - */ - private static String getKindPath(Class clazz) { - try { - Constructor constructor = clazz.getDeclaredConstructor(); - constructor.setAccessible(true); - Object instance = constructor.newInstance(); - Method getKindPathMethod = EppHistoryVKey.class.getDeclaredMethod("getKindPath"); - getKindPathMethod.setAccessible(true); - return (String) getKindPathMethod.invoke(instance); - } catch (Throwable t) { - throw new IllegalStateException(t); - } - } - - @Override - SimpleTranslator createTranslator() { - return new SimpleTranslator() { - - @Nullable - @Override - public EppHistoryVKey loadValue(@Nullable Key datastoreValue) { - if (datastoreValue == null) { - return null; - } else { - com.googlecode.objectify.Key ofyKey = - com.googlecode.objectify.Key.create(datastoreValue); - String kindPath = EppHistoryVKey.createKindPath(ofyKey); - if (kindPathToVKeyClass.containsKey(kindPath)) { - Class vKeyClass = kindPathToVKeyClass.get(kindPath); - try { - Method createVKeyMethod = - vKeyClass.getDeclaredMethod("create", com.googlecode.objectify.Key.class); - return (EppHistoryVKey) createVKeyMethod.invoke(null, ofyKey); - } catch (NoSuchMethodException e) { - throw new IllegalStateException( - "Missing static method create(com.googlecode.objectify.Key) on " + vKeyClass); - } catch (IllegalAccessException | InvocationTargetException e) { - throw new IllegalStateException("Error invoking createVKey on " + vKeyClass, e); - } - } else { - throw new IllegalStateException( - "Missing EppHistoryVKey implementation for kind path: " + kindPath); - } - } - } - - @Nullable - @Override - public Key saveValue(@Nullable EppHistoryVKey pojoValue) { - throw new UnsupportedOperationException("saveValue for EppHistory keys removed."); - } - }; - } -} diff --git a/core/src/main/java/google/registry/persistence/DomainHistoryVKey.java b/core/src/main/java/google/registry/persistence/DomainHistoryVKey.java deleted file mode 100644 index e0a42b2aa..000000000 --- a/core/src/main/java/google/registry/persistence/DomainHistoryVKey.java +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2020 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.persistence; - -import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; - -import com.googlecode.objectify.Key; -import google.registry.model.domain.Domain; -import google.registry.model.domain.DomainHistory; -import google.registry.model.domain.DomainHistory.DomainHistoryId; -import google.registry.model.reporting.HistoryEntry; -import java.io.Serializable; -import javax.persistence.Embeddable; - -/** {@link VKey} for {@link HistoryEntry} which parent is {@link Domain}. */ -@Embeddable -public class DomainHistoryVKey extends EppHistoryVKey { - - // Hibernate requires a default constructor - private DomainHistoryVKey() {} - - private DomainHistoryVKey(String repoId, long historyRevisionId) { - super(repoId, historyRevisionId); - } - - @Override - public Serializable createSqlKey() { - return new DomainHistoryId(repoId, historyRevisionId); - } - - /** Creates {@link DomainHistoryVKey} from the given {@link Key} instance. */ - public static DomainHistoryVKey create(Key ofyKey) { - checkArgumentNotNull(ofyKey, "ofyKey must be specified"); - String repoId = ofyKey.getParent().getName(); - long historyRevisionId = ofyKey.getId(); - return new DomainHistoryVKey(repoId, historyRevisionId); - } - - public VKey createDomainHistoryVKey() { - return VKey.create( - DomainHistory.class, - createSqlKey(), - Key.create(Key.create(Domain.class, repoId), DomainHistory.class, historyRevisionId)); - } -} diff --git a/core/src/main/java/google/registry/persistence/EppHistoryVKey.java b/core/src/main/java/google/registry/persistence/EppHistoryVKey.java deleted file mode 100644 index d9e683c47..000000000 --- a/core/src/main/java/google/registry/persistence/EppHistoryVKey.java +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright 2020 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.persistence; - -import com.google.common.base.Joiner; -import com.googlecode.objectify.Key; -import google.registry.model.EppResource; -import google.registry.model.ImmutableObject; -import google.registry.model.reporting.HistoryEntry; -import google.registry.util.TypeUtils.TypeInstantiator; -import java.io.Serializable; -import javax.annotation.Nullable; -import javax.persistence.Access; -import javax.persistence.AccessType; -import javax.persistence.MappedSuperclass; - -/** - * Base class for {@link VKey} which ofyKey has a {@link HistoryEntry} key as its parent and a key - * for EPP resource as its grandparent. - * - *

For such a {@link VKey}, we need to provide two type parameters to indicate the type of {@link - * VKey} itself and the type of EPP resource respectively. - * - * @param type of the {@link VKey} - * @param type of the EPP resource that the key belongs to - */ -@MappedSuperclass -@Access(AccessType.FIELD) -public abstract class EppHistoryVKey - extends ImmutableObject implements Serializable { - - private static final long serialVersionUID = -3906580677709539818L; - - String repoId; - - Long historyRevisionId; - - // Hibernate requires a default constructor. - EppHistoryVKey() {} - - EppHistoryVKey(String repoId, long historyRevisionId) { - this.repoId = repoId; - this.historyRevisionId = historyRevisionId; - } - - /** - * Returns the kind path for the ofyKey in this instance. - * - *

This method is only used reflectively by {@link EppHistoryVKeyTranslatorFactory} to get the - * kind path for a given {@link EppHistoryVKey} instance so it is marked as a private method. - * - * @see #createKindPath(Key) - */ - @SuppressWarnings("unused") - private String getKindPath() { - String eppKind = Key.getKind(new TypeInstantiator(getClass()) {}.getExactType()); - String keyKind = Key.getKind(new TypeInstantiator(getClass()) {}.getExactType()); - if (keyKind.equals(Key.getKind(HistoryEntry.class))) { - return createKindPath(eppKind, keyKind); - } else { - return createKindPath(eppKind, Key.getKind(HistoryEntry.class), keyKind); - } - } - - /** - * Creates the kind path for the given ofyKey}. - * - *

The kind path is a string including all kind names(delimited by slash) of a hierarchical - * {@link Key}, e.g., the kind path for BillingEvent.OneTime is "Domain/HistoryEntry/OneTime". - */ - @Nullable - public static String createKindPath(@Nullable Key ofyKey) { - if (ofyKey == null) { - return null; - } else if (ofyKey.getParent() == null) { - return ofyKey.getKind(); - } else { - return createKindPath(createKindPath(ofyKey.getParent()), ofyKey.getKind()); - } - } - - private static String createKindPath(String... kinds) { - return Joiner.on("/").join(kinds); - } - - /** Creates a {@link VKey} from this instance. */ - @Override - public VKey createVKey() { - Class vKeyType = new TypeInstantiator(getClass()) {}.getExactType(); - return VKey.createSql(vKeyType, createSqlKey()); - } - - public abstract Serializable createSqlKey(); -} diff --git a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java index 72aad49dd..df79debe6 100644 --- a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java +++ b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java @@ -103,12 +103,21 @@ public class RdapJsonFormatter { private DateTime requestTime = null; - @Inject @Config("rdapTos") ImmutableList rdapTos; - @Inject @Config("rdapTosStaticUrl") @Nullable String rdapTosStaticUrl; + @Inject + @Config("rdapTos") + ImmutableList rdapTos; + + @Inject + @Config("rdapTosStaticUrl") + @Nullable + String rdapTosStaticUrl; + @Inject @FullServletPath String fullServletPath; @Inject RdapAuthorization rdapAuthorization; @Inject Clock clock; - @Inject RdapJsonFormatter() {} + + @Inject + RdapJsonFormatter() {} /** * What type of data to generate. @@ -156,15 +165,15 @@ public class RdapJsonFormatter { /** * JPQL query template for finding the latest history entry per event type for an EPP entity. * - *

User should replace '%entityName%', '%repoIdField%', and '%repoIdValue%' with valid values. - * A DomainHistory query may look like below: {@code select e from DomainHistory e where + *

User should replace '%entityName%' and '%repoIdValue%' with valid values. A {@code + * DomainHistory} query may look like below: {@code select e from DomainHistory e where * domainRepoId = '17-Q9JYB4C' and modificationTime in (select max(modificationTime) from * DomainHistory where domainRepoId = '17-Q9JYB4C' and type is not null group by type)} */ private static final String GET_LAST_HISTORY_BY_TYPE_JPQL_TEMPLATE = - "select e from %entityName% e where %repoIdField% = '%repoIdValue%' and modificationTime in " + "select e from %entityName% e where repoId = '%repoIdValue%' and modificationTime in " + " (select max(modificationTime) from %entityName% where " - + " %repoIdField% = '%repoIdValue%' and type is not null group by type)"; + + " repoId = '%repoIdValue%' and type is not null group by type)"; /** Map of EPP status values to the RDAP equivalents. */ private static final ImmutableMap STATUS_TO_RDAP_STATUS_MAP = @@ -886,19 +895,17 @@ public class RdapJsonFormatter { // 2.3.2.3 An event of *eventAction* type *transfer*, with the last date and time that the // domain was transferred. The event of *eventAction* type *transfer* MUST be omitted if the // domain name has not been transferred since it was created. - VKey resourceVkey = resource.createVKey(); - Class historyClass = - HistoryEntryDao.getHistoryClassFromParent(resourceVkey.getKind()); - String entityName = historyClass.getAnnotation(Entity.class).name(); - if (Strings.isNullOrEmpty(entityName)) { - entityName = historyClass.getSimpleName(); - } - String repoIdFieldName = HistoryEntryDao.getRepoIdFieldNameFromHistoryClass(historyClass); - String jpql = - GET_LAST_HISTORY_BY_TYPE_JPQL_TEMPLATE - .replace("%entityName%", entityName) - .replace("%repoIdField%", repoIdFieldName) - .replace("%repoIdValue%", resourceVkey.getSqlKey().toString()); + VKey resourceVkey = resource.createVKey(); + Class historyClass = + HistoryEntryDao.getHistoryClassFromParent(resourceVkey.getKind()); + String entityName = historyClass.getAnnotation(Entity.class).name(); + if (Strings.isNullOrEmpty(entityName)) { + entityName = historyClass.getSimpleName(); + } + String jpql = + GET_LAST_HISTORY_BY_TYPE_JPQL_TEMPLATE + .replace("%entityName%", entityName) + .replace("%repoIdValue%", resourceVkey.getSqlKey().toString()); Iterable historyEntries = replicaJpaTm() .transact( @@ -974,9 +981,7 @@ public class RdapJsonFormatter { /** Creates an RDAP event object as defined by RFC 9083. */ private static Event makeEvent( EventAction eventAction, @Nullable String eventActor, DateTime eventDate) { - Event.Builder builder = Event.builder() - .setEventAction(eventAction) - .setEventDate(eventDate); + Event.Builder builder = Event.builder().setEventAction(eventAction).setEventDate(eventDate); if (eventActor != null) { builder.setEventActor(eventActor); } @@ -1042,10 +1047,7 @@ public class RdapJsonFormatter { addressArray.add( new Locale("en", nullToEmpty(address.getCountryCode())) .getDisplayCountry(new Locale("en"))); - vcardArrayBuilder.add(Vcard.create( - "adr", - "text", - addressArray)); + vcardArrayBuilder.add(Vcard.create("adr", "text", addressArray)); } /** Creates a vCard phone number entry. */ @@ -1074,8 +1076,7 @@ public class RdapJsonFormatter { private static ImmutableSet makeStatusValueList( ImmutableSet statusValues, boolean isRedacted, boolean isDeleted) { Stream stream = - statusValues - .stream() + statusValues.stream() .map(status -> STATUS_TO_RDAP_STATUS_MAP.getOrDefault(status, RdapStatus.OBSCURED)); if (isRedacted) { stream = Streams.concat(stream, Stream.of(RdapStatus.REMOVED)); @@ -1083,24 +1084,19 @@ public class RdapJsonFormatter { if (isDeleted) { stream = Streams.concat( - stream.filter(not(RdapStatus.ACTIVE::equals)), - Stream.of(RdapStatus.INACTIVE)); + stream.filter(not(RdapStatus.ACTIVE::equals)), Stream.of(RdapStatus.INACTIVE)); } return stream .sorted(Ordering.natural().onResultOf(RdapStatus::getDisplayName)) .collect(toImmutableSet()); } - /** - * Create a link relative to the RDAP server endpoint. - */ + /** Create a link relative to the RDAP server endpoint. */ String makeRdapServletRelativeUrl(String part, String... moreParts) { return makeServerRelativeUrl(fullServletPath, part, moreParts); } - /** - * Create a link relative to some base server - */ + /** Create a link relative to some base server */ static String makeServerRelativeUrl(String baseServer, String part, String... moreParts) { String relativePath = Paths.get(part, moreParts).toString(); if (baseServer.endsWith("/")) { @@ -1117,11 +1113,7 @@ public class RdapJsonFormatter { */ private Link makeSelfLink(String type, String name) { String url = makeRdapServletRelativeUrl(type, name); - return Link.builder() - .setRel("self") - .setHref(url) - .setType("application/rdap+json") - .build(); + return Link.builder().setRel("self").setHref(url).setType("application/rdap+json").build(); } /** diff --git a/core/src/main/java/google/registry/tools/DomainLockUtils.java b/core/src/main/java/google/registry/tools/DomainLockUtils.java index 99c8c1bd4..795417adc 100644 --- a/core/src/main/java/google/registry/tools/DomainLockUtils.java +++ b/core/src/main/java/google/registry/tools/DomainLockUtils.java @@ -47,7 +47,7 @@ import org.joda.time.Duration; /** * Utility functions for validating and applying {@link RegistryLock}s. * - *

For both locks and unlocks, a lock must be requested via the createRegistry*Requst methods + *

For both locks and unlocks, a lock must be requested via the createRegistry*Request methods * then verified through the verifyAndApply* methods. These methods will verify that the domain in * question is in a lock/unlockable state and will return the lock object. */ @@ -57,7 +57,7 @@ public final class DomainLockUtils { private final StringGenerator stringGenerator; private final String registryAdminRegistrarId; - private CloudTasksUtils cloudTasksUtils; + private final CloudTasksUtils cloudTasksUtils; @Inject public DomainLockUtils( @@ -152,7 +152,7 @@ public final class DomainLockUtils { tm().transact(() -> removeLockStatuses(newLock, isAdmin, now)); return newLock; }); - // Submit relock outside of the transaction to make sure that it fully succeeded + // Submit relock outside the transaction to make sure that it fully succeeded submitRelockIfNecessary(lock); return lock; } @@ -200,7 +200,7 @@ public final class DomainLockUtils { tm().transact(() -> removeLockStatuses(result, isAdmin, now)); return result; }); - // Submit relock outside of the transaction to make sure that it fully succeeded + // Submit relock outside the transaction to make sure that it fully succeeded submitRelockIfNecessary(lock); return lock; } diff --git a/core/src/main/java/google/registry/tools/GetAllocationTokenCommand.java b/core/src/main/java/google/registry/tools/GetAllocationTokenCommand.java index bfd5ccafa..5190e7362 100644 --- a/core/src/main/java/google/registry/tools/GetAllocationTokenCommand.java +++ b/core/src/main/java/google/registry/tools/GetAllocationTokenCommand.java @@ -22,8 +22,8 @@ import com.beust.jcommander.Parameters; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; -import com.googlecode.objectify.Key; import google.registry.model.domain.Domain; +import google.registry.model.domain.DomainHistory; import google.registry.model.domain.token.AllocationToken; import google.registry.persistence.VKey; import java.util.Collection; @@ -62,13 +62,12 @@ final class GetAllocationTokenCommand implements CommandWithRemoteApi { if (loadedTokens.containsKey(token)) { AllocationToken loadedToken = loadedTokens.get(token); System.out.println(loadedToken.toString()); - if (!loadedToken.getRedemptionHistoryEntry().isPresent()) { + if (!loadedToken.getRedemptionHistoryId().isPresent()) { System.out.printf("Token %s was not redeemed.\n", token); } else { - Key domainOfyKey = - loadedToken.getRedemptionHistoryEntry().get().getOfyKey().getParent(); - Domain domain = - domains.get(VKey.create(Domain.class, domainOfyKey.getName(), domainOfyKey)); + VKey domainKey = + VKey.createSql(Domain.class, loadedToken.getRedemptionHistoryId().get().getRepoId()); + Domain domain = domains.get(domainKey); if (domain == null) { System.out.printf("ERROR: Token %s was redeemed but domain can't be loaded.\n", token); } else { @@ -89,12 +88,11 @@ final class GetAllocationTokenCommand implements CommandWithRemoteApi { Collection tokens) { ImmutableList> domainKeys = tokens.stream() - .map(AllocationToken::getRedemptionHistoryEntry) + .map(AllocationToken::getRedemptionHistoryId) .filter(Optional::isPresent) .map(Optional::get) - .map(key -> tm().loadByKey(key)) - .map(he -> (Key) he.getParent()) - .map(key -> VKey.create(Domain.class, key.getName(), key)) + .map(hi -> tm().loadByKey(VKey.createSql(DomainHistory.class, hi))) + .map(dh -> VKey.createSql(Domain.class, dh.getRepoId())) .collect(toImmutableList()); ImmutableMap.Builder, Domain> domainsBuilder = new ImmutableMap.Builder<>(); for (List> keys : Lists.partition(domainKeys, BATCH_SIZE)) { diff --git a/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java b/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java index 7e4366d09..c33a4937d 100644 --- a/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java +++ b/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java @@ -70,8 +70,8 @@ final class GetHistoryEntriesCommand implements CommandWithRemoteApi { "Client: %s\nTime: %s\nClient TRID: %s\nServer TRID: %s\n%s\n", entry.getRegistrarId(), entry.getModificationTime(), - (entry.getTrid() == null) ? null : entry.getTrid().getClientTransactionId().orElse(null), - (entry.getTrid() == null) ? null : entry.getTrid().getServerTransactionId(), + entry.getTrid() == null ? null : entry.getTrid().getClientTransactionId().orElse(null), + entry.getTrid() == null ? null : entry.getTrid().getServerTransactionId(), entry.getXmlBytes() == null ? String.format("[no XML stored for %s]\n", entry.getType()) : XmlTransformer.prettyPrint(entry.getXmlBytes())); diff --git a/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java b/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java index d514b1a0c..55916f693 100644 --- a/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java +++ b/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java @@ -132,7 +132,7 @@ class UnrenewDomainCommand extends ConfirmingCommand implements CommandWithRemot if (!domainsExpiringTooSoon.isEmpty()) { System.err.printf("Domains expiring too soon: %s\n\n", domainsExpiringTooSoon); } - checkArgument(!foundInvalidDomains, "Aborting because some domains cannot be unrewed"); + checkArgument(!foundInvalidDomains, "Aborting because some domains cannot be unrenewed"); } @Override @@ -218,7 +218,7 @@ class UnrenewDomainCommand extends ConfirmingCommand implements CommandWithRemot // End the old autorenew billing event and poll message now. Recurring existingRecurring = tm().loadByKey(domain.getAutorenewBillingEvent()); updateAutorenewRecurrenceEndTime( - domain, existingRecurring, now, domainHistory.getDomainHistoryId()); + domain, existingRecurring, now, domainHistory.getHistoryEntryId()); Domain newDomain = domain .asBuilder() diff --git a/core/src/main/java/google/registry/tools/javascrap/CreateCancellationsForOneTimesCommand.java b/core/src/main/java/google/registry/tools/javascrap/CreateCancellationsForOneTimesCommand.java index 1a7df9b72..b9d823f1d 100644 --- a/core/src/main/java/google/registry/tools/javascrap/CreateCancellationsForOneTimesCommand.java +++ b/core/src/main/java/google/registry/tools/javascrap/CreateCancellationsForOneTimesCommand.java @@ -103,7 +103,7 @@ public class CreateCancellationsForOneTimesCommand extends ConfirmingCommand new Cancellation.Builder() .setOneTimeEventKey(oneTime.createVKey()) .setBillingTime(oneTime.getBillingTime()) - .setDomainHistoryId(oneTime.getDomainHistoryId()) + .setDomainHistoryId(oneTime.getHistoryEntryId()) .setRegistrarId(oneTime.getRegistrarId()) .setEventTime(oneTime.getEventTime()) .setReason(BillingEvent.Reason.ERROR) diff --git a/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java b/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java index 80932bcd2..10a12f919 100644 --- a/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java +++ b/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java @@ -37,7 +37,6 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Iterables; -import com.googlecode.objectify.Key; import google.registry.flows.custom.DomainPricingCustomLogic; import google.registry.flows.domain.DomainPricingLogic; import google.registry.model.billing.BillingEvent; @@ -126,7 +125,7 @@ public class ExpandRecurringBillingEventsActionTest { action.response = new FakeResponse(); action.run(); // Need to save the current test time before running the action, which increments the clock. - // The execution time (e. g. transaction time) is captured when the action starts running so + // The execution time (e.g. transaction time) is captured when the action starts running so // the passage of time afterward does not affect the timestamp stored in the billing events. currentTestTime = clock.nowUtc(); } @@ -139,13 +138,13 @@ public class ExpandRecurringBillingEventsActionTest { private void assertHistoryEntryMatches( Domain domain, - HistoryEntry actual, + DomainHistory actual, String registrarId, DateTime billingTime, boolean shouldHaveTxRecord) { assertThat(actual.getBySuperuser()).isFalse(); assertThat(actual.getRegistrarId()).isEqualTo(registrarId); - assertThat(actual.getParent()).isEqualTo(Key.create(domain)); + assertThat(actual.getRepoId()).isEqualTo(domain.getRepoId()); assertThat(actual.getPeriod()).isEqualTo(Period.create(1, YEARS)); assertThat(actual.getReason()) .isEqualTo("Domain autorenewal by ExpandRecurringBillingEventsAction"); @@ -298,7 +297,7 @@ public class ExpandRecurringBillingEventsActionTest { runAction(); List persistedEntries = getHistoryEntriesOfType(domain, DOMAIN_AUTORENEW, DomainHistory.class); - for (HistoryEntry persistedEntry : persistedEntries) { + for (DomainHistory persistedEntry : persistedEntries) { assertHistoryEntryMatches( domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true); } diff --git a/core/src/test/java/google/registry/batch/WipeOutContactHistoryPiiActionTest.java b/core/src/test/java/google/registry/batch/WipeOutContactHistoryPiiActionTest.java index a249d3c74..22ec68691 100644 --- a/core/src/test/java/google/registry/batch/WipeOutContactHistoryPiiActionTest.java +++ b/core/src/test/java/google/registry/batch/WipeOutContactHistoryPiiActionTest.java @@ -286,13 +286,12 @@ class WipeOutContactHistoryPiiActionTest { void wipeOutContactHistoryData_wipesOutNoEntity() { jpaTm() .transact( - () -> { - assertThat( - action.wipeOutContactHistoryData( - action.getNextContactHistoryEntitiesWithPiiBatch( - clock.nowUtc().minusMonths(MIN_MONTHS_BEFORE_WIPE_OUT)))) - .isEqualTo(0); - }); + () -> + assertThat( + action.wipeOutContactHistoryData( + action.getNextContactHistoryEntitiesWithPiiBatch( + clock.nowUtc().minusMonths(MIN_MONTHS_BEFORE_WIPE_OUT)))) + .isEqualTo(0)); } @Test @@ -317,9 +316,9 @@ class WipeOutContactHistoryPiiActionTest { /** persists a number of ContactHistory entities for load and query testing. */ ImmutableList persistLotsOfContactHistoryEntities( int numOfEntities, int minusMonths, int minusDays, Contact contact) { - ImmutableList.Builder expectedEntitesBuilder = new ImmutableList.Builder<>(); + ImmutableList.Builder expectedEntitiesBuilder = new ImmutableList.Builder<>(); for (int i = 0; i < numOfEntities; i++) { - expectedEntitesBuilder.add( + expectedEntitiesBuilder.add( persistResource( new ContactHistory() .asBuilder() @@ -329,7 +328,7 @@ class WipeOutContactHistoryPiiActionTest { .setContact(persistResource(contact)) .build())); } - return expectedEntitesBuilder.build(); + return expectedEntitiesBuilder.build(); } boolean areAllPiiFieldsWiped(ContactBase contactBase) { 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 50d07abb4..de2c024df 100644 --- a/core/src/test/java/google/registry/beam/rde/RdePipelineTest.java +++ b/core/src/test/java/google/registry/beam/rde/RdePipelineTest.java @@ -133,13 +133,13 @@ public class RdePipelineTest { private final ImmutableList brdaFragments = ImmutableList.of( - DepositFragment.create(RdeResourceType.DOMAIN, "\n", ""), - DepositFragment.create(RdeResourceType.REGISTRAR, "\n", "")); + DepositFragment.create(DOMAIN, "\n", ""), + DepositFragment.create(REGISTRAR, "\n", "")); private final ImmutableList rdeFragments = ImmutableList.of( - DepositFragment.create(RdeResourceType.DOMAIN, "\n", ""), - DepositFragment.create(RdeResourceType.REGISTRAR, "\n", ""), + DepositFragment.create(DOMAIN, "\n", ""), + DepositFragment.create(REGISTRAR, "\n", ""), DepositFragment.create(CONTACT, "\n", ""), DepositFragment.create(HOST, "\n", "")); @@ -183,7 +183,6 @@ public class RdePipelineTest { .setReason("reason") .setRequestedByRegistrar(true) .setContact(contact) - .setContactRepoId(contact.getRepoId()) .build()); } @@ -207,7 +206,6 @@ public class RdePipelineTest { .setReason("reason") .setRequestedByRegistrar(true) .setDomain(domain) - .setDomainRepoId(domain.getRepoId()) .setDomainTransactionRecords(ImmutableSet.of(transactionRecord)) .setOtherRegistrarId("otherClient") .setPeriod(Period.create(1, Period.Unit.YEARS)) @@ -226,7 +224,6 @@ public class RdePipelineTest { .setReason("reason") .setRequestedByRegistrar(true) .setHost(hostBase) - .setHostRepoId(hostBase.getRepoId()) .build()); } @@ -389,7 +386,7 @@ public class RdePipelineTest { // The same registrars are attached to all the pending deposits. .containsExactly("New Registrar", "The Registrar", "external_monitoring"); // Domain fragments. - if (kv.getKey().tld().equals("soy")) { + if ("soy".equals(kv.getKey().tld())) { assertThat( getFragmentForType(kv, DOMAIN) .map(getXmlElement(DOMAIN_NAME_PATTERN)) @@ -404,7 +401,7 @@ public class RdePipelineTest { } if (kv.getKey().mode().equals(FULL)) { // Contact fragments for hello.soy. - if (kv.getKey().tld().equals("soy")) { + if ("soy".equals(kv.getKey().tld())) { assertThat( getFragmentForType(kv, CONTACT) .map(getXmlElement(CONTACT_ID_PATTERN)) @@ -528,7 +525,7 @@ public class RdePipelineTest { decryptGhostrydeGcsFile(prefix + "soy_2000-01-01_thin_S1_" + revision + ".xml.ghostryde"); assertThat(brdaOutputFile) .isEqualTo( - readResourceUtf8(this.getClass(), "reducer_brda.xml") + readResourceUtf8(getClass(), "reducer_brda.xml") .replace("%RESEND%", manual ? "" : " resend=\"1\"")); compareLength(brdaOutputFile, prefix + "soy_2000-01-01_thin_S1_" + revision + ".xml.length"); @@ -577,7 +574,7 @@ public class RdePipelineTest { } private static Function getXmlElement(String pattern) { - return (fragment) -> { + return fragment -> { Matcher matcher = Pattern.compile(pattern).matcher(fragment.xml()); checkState(matcher.find(), "Missing %s in xml.", pattern); return matcher.group(1); diff --git a/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java index e17c1a7f7..6d6ad9222 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java @@ -42,7 +42,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Ordering; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.FlowUtils.NotLoggedInException; import google.registry.flows.FlowUtils.UnknownCurrencyEppException; @@ -78,6 +77,7 @@ import google.registry.model.domain.token.AllocationToken; import google.registry.model.domain.token.AllocationToken.TokenStatus; import google.registry.model.eppcommon.StatusValue; import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.tld.Registry; import google.registry.model.tld.Registry.TldState; import google.registry.model.tld.label.ReservedList; @@ -192,12 +192,12 @@ class DomainCheckFlowTest extends ResourceCheckFlowTestCase historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 1L); + HistoryEntryId historyEntryId = new HistoryEntryId(domain.getRepoId(), 1L); persistResource( new AllocationToken.Builder() .setToken("abc123") .setTokenType(SINGLE_USE) - .setRedemptionHistoryEntry(HistoryEntry.createVKey(historyEntryKey)) + .setRedemptionHistoryId(historyEntryId) .build()); doCheckTest( create(false, "example1.tld", "In use"), 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 96fdda69b..885c35778 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java @@ -72,7 +72,6 @@ import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Iterables; import com.google.common.collect.Ordering; import com.google.common.truth.Truth8; -import com.googlecode.objectify.Key; import google.registry.config.RegistryConfig; import google.registry.flows.EppException; import google.registry.flows.EppException.UnimplementedExtensionException; @@ -172,6 +171,7 @@ import google.registry.model.registrar.Registrar.State; import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.tld.Registry; import google.registry.model.tld.Registry.TldState; import google.registry.model.tld.Registry.TldType; @@ -530,12 +530,12 @@ class DomainCreateFlowTest extends ResourceFlowTestCase historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 505L); + HistoryEntryId historyEntryId = new HistoryEntryId(domain.getRepoId(), 505L); persistResource( new AllocationToken.Builder() .setToken("abc123") .setTokenType(SINGLE_USE) - .setRedemptionHistoryEntry(HistoryEntry.createVKey(historyEntryKey)) + .setRedemptionHistoryId(historyEntryId) .build()); clock.advanceOneMilli(); EppException thrown = @@ -556,8 +556,8 @@ class DomainCreateFlowTest extends ResourceFlowTestCase tm().loadByEntity(token)).getRedemptionHistoryEntry()) - .hasValue(HistoryEntry.createVKey(Key.create(historyEntry))); + assertThat(tm().transact(() -> tm().loadByEntity(token)).getRedemptionHistoryId()) + .hasValue(historyEntry.getHistoryEntryId()); } // DomainTransactionRecord is not propagated. @@ -1355,10 +1355,8 @@ class DomainCreateFlowTest extends ResourceFlowTestCase tm().loadByKey(VKey.createSql(AllocationToken.class, token))); assertThat(reloadedToken.isRedeemed()).isTrue(); - assertThat(reloadedToken.getRedemptionHistoryEntry()) - .hasValue( - HistoryEntry.createVKey( - Key.create(getHistoryEntries(reloadResourceByForeignKey()).get(0)))); + assertThat(reloadedToken.getRedemptionHistoryId()) + .hasValue(getHistoryEntries(reloadResourceByForeignKey()).get(0).getHistoryEntryId()); } private void assertAllocationTokenWasNotRedeemed(String token) { @@ -2570,7 +2568,7 @@ class DomainCreateFlowTest extends ResourceFlowTestCase loadFile( "domain_renew_response.xml", ImmutableMap.of("DOMAIN", "example.tld", "EXDATE", "2002-04-03T22:00:00.0Z"))); - assertThat(DatabaseHelper.loadByEntity(allocationToken).getRedemptionHistoryEntry()) - .isPresent(); + assertThat(DatabaseHelper.loadByEntity(allocationToken).getRedemptionHistoryId()).isPresent(); } @Test @@ -745,12 +744,12 @@ class DomainRenewFlowTest extends ResourceFlowTestCase ImmutableMap.of("DOMAIN", "example.tld", "YEARS", "2", "TOKEN", "abc123")); persistDomain(); Domain domain = persistActiveDomain("foo.tld"); - Key historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 505L); + HistoryEntryId historyEntryId = new HistoryEntryId(domain.getRepoId(), 505L); persistResource( new AllocationToken.Builder() .setToken("abc123") .setTokenType(SINGLE_USE) - .setRedemptionHistoryEntry(HistoryEntry.createVKey(historyEntryKey)) + .setRedemptionHistoryId(historyEntryId) .build()); clock.advanceOneMilli(); EppException thrown = @@ -1188,7 +1187,8 @@ class DomainRenewFlowTest extends ResourceFlowTestCase .build()); runFlow(); Domain domain = reloadResourceByForeignKey(); - HistoryEntry historyEntry = getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_RENEW); + DomainHistory historyEntry = + (DomainHistory) getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_RENEW); assertThat(historyEntry.getDomainTransactionRecords()) .containsExactly( DomainTransactionRecord.create( diff --git a/core/src/test/java/google/registry/flows/domain/DomainRestoreRequestFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainRestoreRequestFlowTest.java index e074550d5..c49fec83c 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainRestoreRequestFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainRestoreRequestFlowTest.java @@ -773,8 +773,8 @@ class DomainRestoreRequestFlowTest extends ResourceFlowTestCase historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 505L); + HistoryEntryId historyEntryId = new HistoryEntryId(domain.getRepoId(), 505L); persistResource( new AllocationToken.Builder() .setToken("abc123") .setTokenType(SINGLE_USE) - .setRedemptionHistoryEntry(HistoryEntry.createVKey(historyEntryKey)) + .setRedemptionHistoryId(historyEntryId) .build()); setEppInput("domain_transfer_approve_allocation_token.xml"); EppException thrown = diff --git a/core/src/test/java/google/registry/flows/domain/DomainTransferCancelFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainTransferCancelFlowTest.java index 6b708c5ab..688cd7d68 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainTransferCancelFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainTransferCancelFlowTest.java @@ -145,7 +145,7 @@ class DomainTransferCancelFlowTest .that(historyEntryTransferCancel) .hasRegistrarId("NewRegistrar") .and() - .hasOtherClientId("TheRegistrar"); + .hasOtherRegistrarId("TheRegistrar"); // The only billing event left should be the original autorenew event, now reopened. assertBillingEvents( getLosingClientAutorenewEvent().asBuilder().setRecurrenceEndTime(END_OF_TIME).build()); @@ -381,7 +381,8 @@ class DomainTransferCancelFlowTest void testIcannTransactionRecord_noRecordsToCancel() throws Exception { clock.advanceOneMilli(); runFlow(); - HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_CANCEL); + DomainHistory persistedEntry = + (DomainHistory) getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_CANCEL); // No cancellation records should be produced assertThat(persistedEntry.getDomainTransactionRecords()).isEmpty(); } @@ -410,7 +411,8 @@ class DomainTransferCancelFlowTest ImmutableSet.of(previousSuccessRecord, notCancellableRecord)) .build()); runFlow(); - HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_CANCEL); + DomainHistory persistedEntry = + (DomainHistory) getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_CANCEL); // We should only produce a cancellation record for the original transfer success assertThat(persistedEntry.getDomainTransactionRecords()) .containsExactly(previousSuccessRecord.asBuilder().setReportAmount(-1).build()); diff --git a/core/src/test/java/google/registry/flows/domain/DomainTransferRejectFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainTransferRejectFlowTest.java index 0f1bd6994..98a1f542e 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainTransferRejectFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainTransferRejectFlowTest.java @@ -111,7 +111,9 @@ class DomainTransferRejectFlowTest .hasLastEppUpdateClientId("TheRegistrar"); final HistoryEntry historyEntryTransferRejected = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REJECT); - assertAboutHistoryEntries().that(historyEntryTransferRejected).hasOtherClientId("NewRegistrar"); + assertAboutHistoryEntries() + .that(historyEntryTransferRejected) + .hasOtherRegistrarId("NewRegistrar"); assertLastHistoryContainsResource(domain); // The only billing event left should be the original autorenew event, now reopened. assertBillingEvents( @@ -352,7 +354,8 @@ class DomainTransferRejectFlowTest void testIcannTransactionRecord_noRecordsToCancel() throws Exception { setUpGracePeriodDurations(); runFlow(); - HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REJECT); + DomainHistory persistedEntry = + (DomainHistory) getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REJECT); // We should only produce transfer nacked records, reported now assertThat(persistedEntry.getDomainTransactionRecords()) .containsExactly(DomainTransactionRecord.create("tld", clock.nowUtc(), TRANSFER_NACKED, 1)); @@ -376,7 +379,8 @@ class DomainTransferRejectFlowTest ImmutableSet.of(previousSuccessRecord, notCancellableRecord)) .build()); runFlow(); - HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REJECT); + DomainHistory persistedEntry = + (DomainHistory) getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REJECT); // We should only produce cancellation records for the original success records and nack records assertThat(persistedEntry.getDomainTransactionRecords()) .containsExactly( diff --git a/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java index 6f24c9423..718c29a7f 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java @@ -61,7 +61,6 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.common.collect.Streams; -import com.googlecode.objectify.Key; import google.registry.batch.ResaveEntityAction; import google.registry.flows.EppException; import google.registry.flows.EppRequestSource; @@ -114,6 +113,7 @@ import google.registry.model.registrar.Registrar; import google.registry.model.registrar.Registrar.State; import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.tld.Registry; import google.registry.model.tld.label.PremiumList; import google.registry.model.tld.label.PremiumListDao; @@ -500,7 +500,7 @@ class DomainTransferRequestFlowTest .that(historyEntryTransferRequest) .hasPeriodYears(1) .and() - .hasOtherClientId("TheRegistrar"); + .hasOtherRegistrarId("TheRegistrar"); // Verify correct fields were set. assertTransferRequested( domain, implicitTransferTime, Period.create(1, Unit.YEARS), expectedExpirationTime); @@ -608,7 +608,7 @@ class DomainTransferRequestFlowTest .that(historyEntryTransferRequest) .hasPeriodYears(expectedPeriod.getValue()) .and() - .hasOtherClientId("TheRegistrar"); + .hasOtherRegistrarId("TheRegistrar"); // Verify correct fields were set. assertTransferRequested(domain, implicitTransferTime, expectedPeriod, expectedExpirationTime); @@ -1682,7 +1682,8 @@ class DomainTransferRequestFlowTest .build()); clock.advanceOneMilli(); runTest("domain_transfer_request.xml", UserPrivileges.NORMAL); - HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REQUEST); + DomainHistory persistedEntry = + (DomainHistory) getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REQUEST); // We should produce a transfer success record assertThat(persistedEntry.getDomainTransactionRecords()) .containsExactly( @@ -1795,12 +1796,12 @@ class DomainTransferRequestFlowTest void testFailure_allocationTokenAlreadyRedeemed() throws Exception { setupDomain("example", "tld"); Domain domain = DatabaseHelper.newDomain("foo.tld"); - Key historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 505L); + HistoryEntryId historyEntryId = new HistoryEntryId(domain.getRepoId(), 505L); persistResource( new AllocationToken.Builder() .setToken("abc123") .setTokenType(SINGLE_USE) - .setRedemptionHistoryEntry(HistoryEntry.createVKey(historyEntryKey)) + .setRedemptionHistoryId(historyEntryId) .build()); setEppInput("domain_transfer_request_allocation_token.xml", ImmutableMap.of("TOKEN", "abc123")); EppException thrown = diff --git a/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java b/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java index 31970e8d1..95ce8c8b3 100644 --- a/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java +++ b/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java @@ -37,7 +37,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Maps; import com.google.common.net.InternetDomainName; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.domain.token.AllocationTokenFlowUtils.AllocationTokenNotInPromotionException; import google.registry.flows.domain.token.AllocationTokenFlowUtils.AllocationTokenNotValidForRegistrarException; @@ -48,7 +47,7 @@ import google.registry.model.domain.DomainCommand; import google.registry.model.domain.token.AllocationToken; import google.registry.model.domain.token.AllocationToken.TokenStatus; import google.registry.model.domain.token.AllocationTokenExtension; -import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.model.tld.Registry; import google.registry.testing.AppEngineExtension; import google.registry.testing.DatabaseHelper; @@ -256,7 +255,7 @@ class AllocationTokenFlowUtilsTest { @Test void test_validateTokenCreate_promoCancelled() { - // the promo would be valid but it was cancelled 12 hours ago + // the promo would be valid, but it was cancelled 12 hours ago persistResource( createOneMonthPromoTokenBuilder(DateTime.now(UTC).minusDays(1)) .setTokenStatusTransitions( @@ -271,7 +270,7 @@ class AllocationTokenFlowUtilsTest { @Test void test_validateTokenExistingDomain_promoCancelled() { - // the promo would be valid but it was cancelled 12 hours ago + // the promo would be valid, but it was cancelled 12 hours ago persistResource( createOneMonthPromoTokenBuilder(DateTime.now(UTC).minusDays(1)) .setTokenStatusTransitions( @@ -306,12 +305,12 @@ class AllocationTokenFlowUtilsTest { @Test void test_checkDomainsWithToken_showsFailureMessageForRedeemedToken() { Domain domain = persistActiveDomain("example.tld"); - Key historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 1051L); + HistoryEntryId historyEntryId = new HistoryEntryId(domain.getRepoId(), 1051L); persistResource( new AllocationToken.Builder() .setToken("tokeN") .setTokenType(SINGLE_USE) - .setRedemptionHistoryEntry(HistoryEntry.createVKey(historyEntryKey)) + .setRedemptionHistoryId(historyEntryId) .build()); assertThat( flowUtils diff --git a/core/src/test/java/google/registry/model/OteStatsTestHelper.java b/core/src/test/java/google/registry/model/OteStatsTestHelper.java index 85ece590f..bf515a175 100644 --- a/core/src/test/java/google/registry/model/OteStatsTestHelper.java +++ b/core/src/test/java/google/registry/model/OteStatsTestHelper.java @@ -41,7 +41,7 @@ public final class OteStatsTestHelper { DateTime now = DateTime.now(DateTimeZone.UTC); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistActiveDomain("xn--abc-873b2e7eb1k8a4lpjvv.tld").getRepoId()) + .setDomain(persistActiveDomain("xn--abc-873b2e7eb1k8a4lpjvv.tld")) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_CREATE) .setXmlBytes(getBytes("domain_create_idn.xml")) @@ -49,7 +49,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistActiveDomain("example.tld").getRepoId()) + .setDomain(persistActiveDomain("example.tld")) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_RESTORE) .setXmlBytes(getBytes("domain_restore.xml")) @@ -57,7 +57,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new HostHistory.Builder() - .setHostRepoId(persistDeletedHost("ns1.example.tld", now).getRepoId()) + .setHost(persistDeletedHost("ns1.example.tld", now)) .setRegistrarId(oteAccount1) .setType(Type.HOST_DELETE) .setXmlBytes(getBytes("host_delete.xml")) @@ -86,7 +86,7 @@ public final class OteStatsTestHelper { DateTime now = DateTime.now(DateTimeZone.UTC); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistActiveDomain("exampleone.tld").getRepoId()) + .setDomain(persistActiveDomain("exampleone.tld")) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_CREATE) .setXmlBytes(getBytes("domain_create_sunrise.xml")) @@ -94,7 +94,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistActiveDomain("example-one.tld").getRepoId()) + .setDomain(persistActiveDomain("example-one.tld")) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_CREATE) .setXmlBytes(getBytes("domain_create_claim_notice.xml")) @@ -102,7 +102,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistActiveDomain("example.tld").getRepoId()) + .setDomain(persistActiveDomain("example.tld")) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_CREATE) .setXmlBytes(getBytes("domain_create_anchor_tenant_fee_standard.xml")) @@ -110,7 +110,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistActiveDomain("example.tld").getRepoId()) + .setDomain(persistActiveDomain("example.tld")) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_CREATE) .setXmlBytes(getBytes("domain_create_dsdata.xml")) @@ -118,7 +118,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistDeletedDomain("example.tld", now).getRepoId()) + .setDomain(persistDeletedDomain("example.tld", now)) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_DELETE) .setXmlBytes(getBytes("domain_delete.xml")) @@ -126,7 +126,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistActiveDomain("example.tld").getRepoId()) + .setDomain(persistActiveDomain("example.tld")) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_TRANSFER_APPROVE) .setXmlBytes(getBytes("domain_transfer_approve.xml")) @@ -134,7 +134,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistActiveDomain("example.tld").getRepoId()) + .setDomain(persistActiveDomain("example.tld")) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_TRANSFER_CANCEL) .setXmlBytes(getBytes("domain_transfer_cancel.xml")) @@ -142,7 +142,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistActiveDomain("example.tld").getRepoId()) + .setDomain(persistActiveDomain("example.tld")) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_TRANSFER_REJECT) .setXmlBytes(getBytes("domain_transfer_reject.xml")) @@ -150,7 +150,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistActiveDomain("example.tld").getRepoId()) + .setDomain(persistActiveDomain("example.tld")) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_TRANSFER_REQUEST) .setXmlBytes(getBytes("domain_transfer_request.xml")) @@ -158,7 +158,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new DomainHistory.Builder() - .setDomainRepoId(persistActiveDomain("example.tld").getRepoId()) + .setDomain(persistActiveDomain("example.tld")) .setRegistrarId(oteAccount1) .setType(Type.DOMAIN_UPDATE) .setXmlBytes(getBytes("domain_update_with_secdns.xml")) @@ -166,7 +166,7 @@ public final class OteStatsTestHelper { .build()); persistResource( new HostHistory.Builder() - .setHostRepoId(persistActiveHost("example.tld").getRepoId()) + .setHost(persistActiveHost("example.tld")) .setRegistrarId(oteAccount1) .setType(Type.HOST_CREATE) .setXmlBytes(getBytes("host_create_complete.xml")) @@ -178,7 +178,7 @@ public final class OteStatsTestHelper { for (int i = 0; i < 10; i++) { persistResource( new HostHistory.Builder() - .setHostRepoId(persistActiveHost("example.tld").getRepoId()) + .setHost(persistActiveHost("example.tld")) .setRegistrarId(oteAccount1) .setType(Type.HOST_UPDATE) .setXmlBytes(getBytes("host_update.xml")) diff --git a/core/src/test/java/google/registry/model/SchemaVersionTest.java b/core/src/test/java/google/registry/model/SchemaVersionTest.java index f596d6938..8b8b0c120 100644 --- a/core/src/test/java/google/registry/model/SchemaVersionTest.java +++ b/core/src/test/java/google/registry/model/SchemaVersionTest.java @@ -14,6 +14,7 @@ package google.registry.model; +import google.registry.model.annotations.DeleteAfterMigration; import google.registry.testing.AppEngineExtension; import google.registry.testing.GoldenFileTestHelper; import org.junit.jupiter.api.Test; @@ -24,6 +25,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; * *

If the test breaks, the instructions below will be printed. */ +@DeleteAfterMigration public class SchemaVersionTest { @RegisterExtension diff --git a/core/src/test/java/google/registry/model/billing/BillingEventTest.java b/core/src/test/java/google/registry/model/billing/BillingEventTest.java index 945b47973..34e4fdfa3 100644 --- a/core/src/test/java/google/registry/model/billing/BillingEventTest.java +++ b/core/src/test/java/google/registry/model/billing/BillingEventTest.java @@ -262,7 +262,7 @@ public class BillingEventTest extends EntityTestCase { BillingEvent.Cancellation.forGracePeriod( GracePeriod.forBillingEvent(GracePeriodStatus.ADD, domain.getRepoId(), oneTime), domainHistory2.getModificationTime(), - domainHistory2.getDomainHistoryId(), + domainHistory2.getHistoryEntryId(), "foo.tld"); // Set ID to be the same to ignore for the purposes of comparison. assertThat(newCancellation.asBuilder().setId(cancellationOneTime.getId()).build()) @@ -280,7 +280,7 @@ public class BillingEventTest extends EntityTestCase { "TheRegistrar", recurring.createVKey()), domainHistory2.getModificationTime(), - domainHistory2.getDomainHistoryId(), + domainHistory2.getHistoryEntryId(), "foo.tld"); // Set ID to be the same to ignore for the purposes of comparison. assertThat(newCancellation.asBuilder().setId(cancellationRecurring.getId()).build()) @@ -300,7 +300,7 @@ public class BillingEventTest extends EntityTestCase { now.plusDays(1), "a registrar"), domainHistory.getModificationTime(), - domainHistory.getDomainHistoryId(), + domainHistory.getHistoryEntryId(), "foo.tld")); assertThat(thrown).hasMessageThat().contains("grace period without billing event"); } diff --git a/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java b/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java index 02bc4b3d3..b3e398bce 100644 --- a/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java +++ b/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java @@ -21,7 +21,6 @@ import google.registry.model.contact.Contact; import google.registry.model.domain.Domain; import google.registry.model.domain.DomainHistory; import google.registry.model.host.Host; -import google.registry.model.reporting.HistoryEntry; import google.registry.testing.TestObject; import org.junit.jupiter.api.Test; @@ -42,7 +41,6 @@ public class ClassPathManagerTest { assertThat(ClassPathManager.getClass("Contact")).isEqualTo(Contact.class); assertThat(ClassPathManager.getClass("GaeUserIdConverter")).isEqualTo(GaeUserIdConverter.class); assertThat(ClassPathManager.getClass("Domain")).isEqualTo(Domain.class); - assertThat(ClassPathManager.getClass("HistoryEntry")).isEqualTo(HistoryEntry.class); } @Test @@ -80,7 +78,6 @@ public class ClassPathManagerTest { assertThat(ClassPathManager.getClassName(GaeUserIdConverter.class)) .isEqualTo("GaeUserIdConverter"); assertThat(ClassPathManager.getClassName(Domain.class)).isEqualTo("Domain"); - assertThat(ClassPathManager.getClassName(HistoryEntry.class)).isEqualTo("HistoryEntry"); } @Test diff --git a/core/src/test/java/google/registry/model/domain/DomainSqlTest.java b/core/src/test/java/google/registry/model/domain/DomainSqlTest.java index 10b3481aa..862218ba0 100644 --- a/core/src/test/java/google/registry/model/domain/DomainSqlTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainSqlTest.java @@ -29,16 +29,11 @@ import static google.registry.testing.SqlHelper.assertThrowForeignKeyViolation; 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; import static org.joda.time.DateTimeZone.UTC; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Sets; -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; import google.registry.model.billing.BillingEvent.RenewalPriceBehavior; import google.registry.model.contact.Contact; import google.registry.model.domain.DesignatedContact.Type; @@ -50,16 +45,12 @@ import google.registry.model.domain.token.AllocationToken.TokenStatus; import google.registry.model.eppcommon.AuthInfo.PasswordAuth; import google.registry.model.eppcommon.StatusValue; import google.registry.model.host.Host; -import google.registry.model.poll.PollMessage; -import google.registry.model.reporting.HistoryEntry; import google.registry.model.transfer.ContactTransferData; -import google.registry.model.transfer.DomainTransferData; import google.registry.persistence.VKey; import google.registry.testing.AppEngineExtension; import google.registry.testing.FakeClock; import google.registry.util.SerializeUtils; import java.util.Arrays; -import org.joda.money.Money; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -80,14 +71,12 @@ public class DomainSqlTest { .build(); private Domain domain; - private DomainHistory historyEntry; private VKey contactKey; private VKey contact2Key; private VKey host1VKey; private Host host; private Contact contact; private Contact contact2; - private ImmutableSet gracePeriods; private AllocationToken allocationToken; @BeforeEach @@ -411,131 +400,8 @@ public class DomainSqlTest { insertInDb(contact, contact2, domain, host); } - @Test - void persistDomainWithCompositeVKeys() { - createTld("com"); - historyEntry = - new DomainHistory.Builder() - .setId(100L) - .setType(HistoryEntry.Type.DOMAIN_CREATE) - .setPeriod(Period.create(1, Period.Unit.YEARS)) - .setModificationTime(DateTime.now(UTC)) - .setDomainRepoId("4-COM") - .setRegistrarId("registrar1") - // 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.Recurring billEvent = - new BillingEvent.Recurring.Builder() - .setId(200L) - .setReason(Reason.RENEW) - .setFlags(ImmutableSet.of(Flag.AUTO_RENEW)) - .setTargetId("example.com") - .setRegistrarId("registrar1") - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setRecurrenceEndTime(END_OF_TIME) - .setDomainHistory(historyEntry) - .build(); - PollMessage.Autorenew autorenewPollMessage = - new PollMessage.Autorenew.Builder() - .setId(300L) - .setRegistrarId("registrar1") - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setHistoryEntry(historyEntry) - .build(); - PollMessage.OneTime deletePollMessage = - new PollMessage.OneTime.Builder() - .setId(400L) - .setRegistrarId("registrar1") - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setHistoryEntry(historyEntry) - .build(); - BillingEvent.OneTime oneTimeBillingEvent = - 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") - .setBillingTime(DateTime.now(UTC)) - .setCost(Money.of(USD, 100)) - .setEventTime(DateTime.now(UTC).plusYears(1)) - .setDomainHistory(historyEntry) - .build(); - DomainTransferData transferData = - new DomainTransferData.Builder() - .setServerApproveBillingEvent(oneTimeBillingEvent.createVKey()) - .setServerApproveAutorenewEvent(billEvent.createVKey()) - .setServerApproveAutorenewPollMessage(autorenewPollMessage.createVKey()) - .build(); - gracePeriods = - ImmutableSet.of( - GracePeriod.create( - GracePeriodStatus.ADD, - "4-COM", - END_OF_TIME, - "registrar1", - oneTimeBillingEvent.createVKey()), - GracePeriod.createForRecurring( - GracePeriodStatus.AUTO_RENEW, - "4-COM", - END_OF_TIME, - "registrar1", - billEvent.createVKey())); - - domain = - domain - .asBuilder() - .setAutorenewBillingEvent(billEvent.createVKey()) - .setAutorenewPollMessage(autorenewPollMessage.createVKey()) - .setDeletePollMessage(deletePollMessage.createVKey()) - .setTransferData(transferData) - .setGracePeriods(gracePeriods) - .build(); - historyEntry = historyEntry.asBuilder().setDomain(domain).build(); - insertInDb( - contact, - contact2, - host, - historyEntry, - autorenewPollMessage, - billEvent, - deletePollMessage, - oneTimeBillingEvent, - domain); - - // Store the existing BillingRecurrence VKey. This happens after the event has been persisted. - Domain persisted = loadByKey(domain.createVKey()); - - // Verify that the domain data has been persisted. - // dsData still isn't persisted. gracePeriods appears to have the same values but for some - // reason is showing up as different. - assertEqualDomainExcept(persisted, "creationTime", "dsData", "gracePeriods"); - - // Verify that the DomainBase object from the history record sets the fields correctly. - DomainHistory persistedHistoryEntry = loadByKey(historyEntry.createVKey()); - assertThat(persistedHistoryEntry.getDomainBase().get().getAutorenewPollMessage()) - .isEqualTo(domain.getAutorenewPollMessage()); - assertThat(persistedHistoryEntry.getDomainBase().get().getAutorenewBillingEvent()) - .isEqualTo(domain.getAutorenewBillingEvent()); - assertThat(persistedHistoryEntry.getDomainBase().get().getDeletePollMessage()) - .isEqualTo(domain.getDeletePollMessage()); - DomainTransferData persistedTransferData = - persistedHistoryEntry.getDomainBase().get().getTransferData(); - DomainTransferData originalTransferData = domain.getTransferData(); - assertThat(persistedTransferData.getServerApproveBillingEvent()) - .isEqualTo(originalTransferData.getServerApproveBillingEvent()); - assertThat(persistedTransferData.getServerApproveAutorenewEvent()) - .isEqualTo(originalTransferData.getServerApproveAutorenewEvent()); - assertThat(persistedTransferData.getServerApproveAutorenewPollMessage()) - .isEqualTo(originalTransferData.getServerApproveAutorenewPollMessage()); - assertThat(persisted.getGracePeriods()).isEqualTo(gracePeriods); - } - - private VKey createKey(Class clazz, String name) { - return VKey.create(clazz, name, Key.create(clazz, name)); + private VKey createKey(Class clazz, String key) { + return VKey.createSql(clazz, key); } private void assertEqualDomainExcept(Domain thatDomain, String... excepts) { @@ -548,7 +414,7 @@ public class DomainSqlTest { .build(); // Note that the equality comparison forces a lazy load of all fields. assertAboutImmutableObjects().that(thatDomain).isEqualExceptFields(domain, moreExcepts); - // Transfer data cannot be directly compared due to serverApproveEtities inequalities + // Transfer data cannot be directly compared due to serverApproveEntities inequalities assertAboutImmutableObjects() .that(domain.getTransferData()) .isEqualExceptFields(thatDomain.getTransferData(), "serverApproveEntities"); diff --git a/core/src/test/java/google/registry/model/domain/DomainTest.java b/core/src/test/java/google/registry/model/domain/DomainTest.java index d5ce67deb..d6f510091 100644 --- a/core/src/test/java/google/registry/model/domain/DomainTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainTest.java @@ -109,7 +109,7 @@ public class DomainTest { domainHistory = persistResource( new DomainHistory.Builder() - .setDomainRepoId(domain.getRepoId()) + .setDomain(domain) .setModificationTime(fakeClock.nowUtc()) .setType(HistoryEntry.Type.DOMAIN_CREATE) .setRegistrarId("TheRegistrar") @@ -129,11 +129,11 @@ public class DomainTest { .createVKey(); DomainHistory historyEntry = new DomainHistory.Builder() - .setId(100L) + .setRevisionId(100L) .setType(HistoryEntry.Type.DOMAIN_CREATE) .setPeriod(Period.create(1, Period.Unit.YEARS)) .setModificationTime(DateTime.now(UTC)) - .setDomainRepoId(domain.getRepoId()) + .setDomain(domain) .setRegistrarId(domain.getCurrentSponsorRegistrarId()) // These are non-null, but I don't think some tests set them. .setReason("felt like it") @@ -202,8 +202,8 @@ public class DomainTest { .setLosingRegistrarId("NewRegistrar") .setPendingTransferExpirationTime(fakeClock.nowUtc()) .setServerApproveEntities( - historyEntry.getDomainRepoId(), - historyEntry.getId(), + historyEntry.getRepoId(), + historyEntry.getRevisionId(), ImmutableSet.of(oneTimeBillKey, recurringBillKey, autorenewPollKey)) .setServerApproveBillingEvent(oneTimeBillKey) .setServerApproveAutorenewEvent(recurringBillKey) @@ -438,7 +438,7 @@ public class DomainTest { .setServerApproveBillingEvent(transferBillingEvent.createVKey()) .setServerApproveEntities( domain.getRepoId(), - historyEntry.getId(), + historyEntry.getRevisionId(), ImmutableSet.of(transferBillingEvent.createVKey())) .build()) .addGracePeriod( diff --git a/core/src/test/java/google/registry/model/domain/GracePeriodTest.java b/core/src/test/java/google/registry/model/domain/GracePeriodTest.java index ac635b95e..1a61838b3 100644 --- a/core/src/test/java/google/registry/model/domain/GracePeriodTest.java +++ b/core/src/test/java/google/registry/model/domain/GracePeriodTest.java @@ -21,8 +21,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Reason; import google.registry.model.billing.BillingEvent.Recurring; -import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.rgp.GracePeriodStatus; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.persistence.VKey; import google.registry.testing.AppEngineExtension; import org.joda.money.CurrencyUnit; @@ -53,7 +53,7 @@ public class GracePeriodTest { .setBillingTime(now.plusDays(1)) .setRegistrarId("TheRegistrar") .setCost(Money.of(CurrencyUnit.USD, 42)) - .setDomainHistoryId(new DomainHistoryId("domain", 12345)) + .setDomainHistoryId(new HistoryEntryId("domain", 12345)) .setReason(Reason.CREATE) .setPeriodYears(1) .setTargetId("foo.google") diff --git a/core/src/test/java/google/registry/model/domain/token/AllocationTokenTest.java b/core/src/test/java/google/registry/model/domain/token/AllocationTokenTest.java index b863edd33..6563091a5 100644 --- a/core/src/test/java/google/registry/model/domain/token/AllocationTokenTest.java +++ b/core/src/test/java/google/registry/model/domain/token/AllocationTokenTest.java @@ -33,7 +33,6 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; -import com.googlecode.objectify.Key; import google.registry.model.Buildable; import google.registry.model.EntityTestCase; import google.registry.model.billing.BillingEvent.RenewalPriceBehavior; @@ -41,7 +40,7 @@ import google.registry.model.domain.Domain; import google.registry.model.domain.token.AllocationToken.RegistrationBehavior; import google.registry.model.domain.token.AllocationToken.TokenStatus; import google.registry.model.domain.token.AllocationToken.TokenType; -import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.util.SerializeUtils; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; @@ -82,12 +81,12 @@ public class AllocationTokenTest extends EntityTestCase { assertThat(loadByEntity(unlimitedUseToken)).isEqualTo(unlimitedUseToken); Domain domain = persistActiveDomain("example.foo"); - Key historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 1); + HistoryEntryId historyEntryId = new HistoryEntryId(domain.getRepoId(), 1); AllocationToken singleUseToken = persistResource( new AllocationToken.Builder() .setToken("abc123Single") - .setRedemptionHistoryEntry(HistoryEntry.createVKey(historyEntryKey)) + .setRedemptionHistoryId(historyEntryId) .setDomainName("example.foo") .setCreationTimeForTest(DateTime.parse("2010-11-12T05:00:00Z")) .setTokenType(SINGLE_USE) @@ -119,12 +118,12 @@ public class AllocationTokenTest extends EntityTestCase { assertThat(SerializeUtils.serializeDeserialize(persisted)).isEqualTo(persisted); Domain domain = persistActiveDomain("example.foo"); - Key historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 1); + HistoryEntryId historyEntryId = new HistoryEntryId(domain.getRepoId(), 1); AllocationToken singleUseToken = persistResource( new AllocationToken.Builder() .setToken("abc123Single") - .setRedemptionHistoryEntry(HistoryEntry.createVKey(historyEntryKey)) + .setRedemptionHistoryId(historyEntryId) .setDomainName("example.foo") .setCreationTimeForTest(DateTime.parse("2010-11-12T05:00:00Z")) .setTokenType(SINGLE_USE) @@ -306,12 +305,12 @@ public class AllocationTokenTest extends EntityTestCase { @Test void testBuild_redemptionHistoryEntryOnlyInSingleUse() { Domain domain = persistActiveDomain("blahdomain.foo"); - Key historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 1); + HistoryEntryId historyEntryId = new HistoryEntryId(domain.getRepoId(), 1); AllocationToken.Builder builder = new AllocationToken.Builder() .setToken("foobar") .setTokenType(TokenType.UNLIMITED_USE) - .setRedemptionHistoryEntry(HistoryEntry.createVKey(historyEntryKey)); + .setRedemptionHistoryId(historyEntryId); IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, builder::build); assertThat(thrown) .hasMessageThat() diff --git a/core/src/test/java/google/registry/model/history/ContactHistoryTest.java b/core/src/test/java/google/registry/model/history/ContactHistoryTest.java index c25c6de8e..4f90c424c 100644 --- a/core/src/test/java/google/registry/model/history/ContactHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/ContactHistoryTest.java @@ -54,7 +54,7 @@ public class ContactHistoryTest extends EntityTestCase { () -> { ContactHistory fromDatabase = jpaTm().loadByKey(contactHistory.createVKey()); assertContactHistoriesEqual(fromDatabase, contactHistory); - assertThat(fromDatabase.getParentVKey()).isEqualTo(contactHistory.getParentVKey()); + assertThat(fromDatabase.getRepoId()).isEqualTo(contactHistory.getRepoId()); }); } @@ -70,24 +70,6 @@ public class ContactHistoryTest extends EntityTestCase { assertThat(SerializeUtils.serializeDeserialize(fromDatabase)).isEqualTo(fromDatabase); } - @Test - void testLegacyPersistence_nullContactBase() { - Contact contact = newContactWithRoid("contactId", "contact1"); - insertInDb(contact); - Contact contactFromDb = loadByEntity(contact); - ContactHistory contactHistory = - createContactHistory(contactFromDb).asBuilder().setContact(null).build(); - insertInDb(contactHistory); - - jpaTm() - .transact( - () -> { - ContactHistory fromDatabase = jpaTm().loadByKey(contactHistory.createVKey()); - assertContactHistoriesEqual(fromDatabase, contactHistory); - assertThat(fromDatabase.getParentVKey()).isEqualTo(contactHistory.getParentVKey()); - }); - } - @Test void testWipeOutPii_assertsAllPiiFieldsAreNull() { ContactHistory originalEntity = @@ -153,16 +135,13 @@ public class ContactHistoryTest extends EntityTestCase { .setReason("reason") .setRequestedByRegistrar(true) .setContact(contact) - .setContactRepoId(contact.getRepoId()) .build(); } static void assertContactHistoriesEqual(ContactHistory one, ContactHistory two) { + assertAboutImmutableObjects().that(one).isEqualExceptFields(two, "resource"); assertAboutImmutableObjects() - .that(one) - .isEqualExceptFields(two, "contactBase", "contactRepoId"); - assertAboutImmutableObjects() - .that(one.getContactBase().orElse(null)) - .isEqualExceptFields(two.getContactBase().orElse(null), "repoId"); + .that(one.getContactBase().get()) + .isEqualExceptFields(two.getContactBase().get()); } } diff --git a/core/src/test/java/google/registry/model/history/DomainHistoryTest.java b/core/src/test/java/google/registry/model/history/DomainHistoryTest.java index fff084768..d72a16218 100644 --- a/core/src/test/java/google/registry/model/history/DomainHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/DomainHistoryTest.java @@ -69,7 +69,7 @@ public class DomainHistoryTest extends EntityTestCase { () -> { DomainHistory fromDatabase = jpaTm().loadByKey(domainHistory.createVKey()); assertDomainHistoriesEqual(fromDatabase, domainHistory); - assertThat(fromDatabase.getParentVKey()).isEqualTo(domainHistory.getParentVKey()); + assertThat(fromDatabase.getRepoId()).isEqualTo(domainHistory.getRepoId()); }); } @@ -83,23 +83,6 @@ public class DomainHistoryTest extends EntityTestCase { assertThat(SerializeUtils.serializeDeserialize(fromDatabase)).isEqualTo(fromDatabase); } - @Test - void testLegacyPersistence_nullResource() { - Domain domain = addGracePeriodForSql(createDomainWithContactsAndHosts()); - DomainHistory domainHistory = createDomainHistory(domain).asBuilder().setDomain(null).build(); - insertInDb(domainHistory); - - jpaTm() - .transact( - () -> { - DomainHistory fromDatabase = jpaTm().loadByKey(domainHistory.createVKey()); - assertDomainHistoriesEqual(fromDatabase, domainHistory); - assertThat(fromDatabase.getParentVKey()).isEqualTo(domainHistory.getParentVKey()); - assertThat(fromDatabase.getNsHosts()) - .containsExactlyElementsIn(domainHistory.getNsHosts()); - }); - } - static Domain createDomainWithContactsAndHosts() { createTld("tld"); Host host = newHostWithRoid("ns1.example.com", "host1"); @@ -134,7 +117,7 @@ public class DomainHistoryTest extends EntityTestCase { } static void assertDomainHistoriesEqual(DomainHistory one, DomainHistory two) { - assertAboutImmutableObjects().that(one).isEqualExceptFields(two, "domainBase"); + assertAboutImmutableObjects().that(one).isEqualExceptFields(two, "resource"); assertAboutImmutableObjects() .that(one.getDomainBase().get()) .isEqualExceptFields(two.getDomainBase().get(), "updateTimestamp"); @@ -159,7 +142,6 @@ public class DomainHistoryTest extends EntityTestCase { .setReason("reason") .setRequestedByRegistrar(true) .setDomain(domain) - .setDomainRepoId(domain.getRepoId()) .setDomainTransactionRecords(ImmutableSet.of(transactionRecord)) .setOtherRegistrarId("otherClient") .setPeriod(Period.create(1, Period.Unit.YEARS)) diff --git a/core/src/test/java/google/registry/model/history/HostHistoryTest.java b/core/src/test/java/google/registry/model/history/HostHistoryTest.java index 4fdfa1db1..e06b6c9b5 100644 --- a/core/src/test/java/google/registry/model/history/HostHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/HostHistoryTest.java @@ -50,7 +50,7 @@ public class HostHistoryTest extends EntityTestCase { () -> { HostHistory fromDatabase = jpaTm().loadByKey(hostHistory.createVKey()); assertHostHistoriesEqual(fromDatabase, hostHistory); - assertThat(fromDatabase.getParentVKey()).isEqualTo(hostHistory.getParentVKey()); + assertThat(fromDatabase.getRepoId()).isEqualTo(hostHistory.getRepoId()); }); } @@ -65,29 +65,11 @@ public class HostHistoryTest extends EntityTestCase { assertThat(SerializeUtils.serializeDeserialize(fromDatabase)).isEqualTo(fromDatabase); } - @Test - void testLegacyPersistence_nullHostBase() { - Host host = newHostWithRoid("ns1.example.com", "host1"); - insertInDb(host); - - Host hostFromDb = loadByEntity(host); - HostHistory hostHistory = createHostHistory(hostFromDb).asBuilder().setHost(null).build(); - insertInDb(hostHistory); - - jpaTm() - .transact( - () -> { - HostHistory fromDatabase = jpaTm().loadByKey(hostHistory.createVKey()); - assertHostHistoriesEqual(fromDatabase, hostHistory); - assertThat(fromDatabase.getParentVKey()).isEqualTo(hostHistory.getParentVKey()); - }); - } - - private void assertHostHistoriesEqual(HostHistory one, HostHistory two) { - assertAboutImmutableObjects().that(one).isEqualExceptFields(two, "hostBase"); + private static void assertHostHistoriesEqual(HostHistory one, HostHistory two) { + assertAboutImmutableObjects().that(one).isEqualExceptFields(two, "resource"); assertAboutImmutableObjects() - .that(one.getHostBase().orElse(null)) - .isEqualExceptFields(two.getHostBase().orElse(null), "repoId"); + .that(one.getHostBase().get()) + .isEqualExceptFields(two.getHostBase().get(), "repoId"); } private HostHistory createHostHistory(HostBase hostBase) { @@ -101,7 +83,6 @@ public class HostHistoryTest extends EntityTestCase { .setReason("reason") .setRequestedByRegistrar(true) .setHost(hostBase) - .setHostRepoId(hostBase.getRepoId()) .build(); } } 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 a5889a0c6..82a097d74 100644 --- a/core/src/test/java/google/registry/model/poll/PollMessageTest.java +++ b/core/src/test/java/google/registry/model/poll/PollMessageTest.java @@ -41,7 +41,6 @@ import org.junit.jupiter.api.Test; /** Unit tests for {@link PollMessage}. */ public class PollMessageTest extends EntityTestCase { - private Domain domain; private HistoryEntry historyEntry; private PollMessage.OneTime oneTime; private PollMessage.Autorenew autoRenew; @@ -54,7 +53,7 @@ public class PollMessageTest extends EntityTestCase { void setUp() { createTld("foobar"); Contact contact = persistActiveContact("contact1234"); - domain = persistResource(DatabaseHelper.newDomain("foo.foobar", contact)); + Domain domain = persistResource(DatabaseHelper.newDomain("foo.foobar", contact)); historyEntry = persistResource( new DomainHistory.Builder() @@ -68,8 +67,7 @@ public class PollMessageTest extends EntityTestCase { .setBySuperuser(false) .setReason("reason") .setRequestedByRegistrar(false) - .build() - .toChildHistoryEntity()); + .build()); oneTime = new PollMessage.OneTime.Builder() .setId(100L) diff --git a/core/src/test/java/google/registry/model/reporting/HistoryEntryDaoTest.java b/core/src/test/java/google/registry/model/reporting/HistoryEntryDaoTest.java index d6a46b090..42d3bebba 100644 --- a/core/src/test/java/google/registry/model/reporting/HistoryEntryDaoTest.java +++ b/core/src/test/java/google/registry/model/reporting/HistoryEntryDaoTest.java @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.model.ImmutableObjectSubject.immutableObjectCorrespondence; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; -import static google.registry.testing.DatabaseHelper.newDomain; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.util.DateTimeUtils.END_OF_TIME; @@ -76,7 +75,7 @@ class HistoryEntryDaoTest extends EntityTestCase { @Test void testSimpleLoadAll() { assertThat(HistoryEntryDao.loadAllHistoryObjects(START_OF_TIME, END_OF_TIME)) - .comparingElementsUsing(immutableObjectCorrespondence("nsHosts", "domainBase")) + .comparingElementsUsing(immutableObjectCorrespondence("nsHosts", "resource")) .containsExactly(domainHistory); } @@ -98,7 +97,7 @@ class HistoryEntryDaoTest extends EntityTestCase { tm().transact( () -> assertThat(HistoryEntryDao.loadHistoryObjectsForResource(domain.createVKey())) - .comparingElementsUsing(immutableObjectCorrespondence("nsHosts", "domainBase")) + .comparingElementsUsing(immutableObjectCorrespondence("nsHosts", "resource")) .containsExactly(domainHistory)); } diff --git a/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java b/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java index a9ed59cb6..30f68b345 100644 --- a/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java +++ b/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.persistActiveContact; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistResource; import static java.nio.charset.StandardCharsets.UTF_8; @@ -25,6 +26,7 @@ import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableSet; import google.registry.model.EntityTestCase; +import google.registry.model.contact.Contact; import google.registry.model.contact.ContactHistory; import google.registry.model.domain.Domain; import google.registry.model.domain.DomainHistory; @@ -36,15 +38,16 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; /** Unit tests for {@link HistoryEntry}. */ - class HistoryEntryTest extends EntityTestCase { private DomainHistory domainHistory; + private Contact contact; @BeforeEach void setUp() { createTld("foobar"); Domain domain = persistActiveDomain("foo.foobar"); + contact = persistActiveContact("someone"); DomainTransactionRecord transactionRecord = new DomainTransactionRecord.Builder() .setTld("foobar") @@ -78,10 +81,26 @@ class HistoryEntryTest extends EntityTestCase { DomainHistory fromDatabase = tm().loadByEntity(domainHistory); assertAboutImmutableObjects() .that(fromDatabase) - .isEqualExceptFields(domainHistory, "domainBase"); + .isEqualExceptFields(domainHistory, "resource"); }); } + @Test + void testBuilder_resourceMustBeSpecified() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> + new ContactHistory.Builder() + .setRevisionId(5L) + .setModificationTime(DateTime.parse("1985-07-12T22:30:00Z")) + .setRegistrarId("TheRegistrar") + .setReason("Reason") + .setType(HistoryEntry.Type.CONTACT_CREATE) + .build()); + assertThat(thrown).hasMessageThat().isEqualTo("EPP resource must be specified"); + } + @Test void testBuilder_typeMustBeSpecified() { IllegalArgumentException thrown = @@ -89,7 +108,8 @@ class HistoryEntryTest extends EntityTestCase { IllegalArgumentException.class, () -> new ContactHistory.Builder() - .setId(5L) + .setContact(contact) + .setRevisionId(5L) .setModificationTime(DateTime.parse("1985-07-12T22:30:00Z")) .setRegistrarId("TheRegistrar") .setReason("Reason") @@ -104,7 +124,8 @@ class HistoryEntryTest extends EntityTestCase { IllegalArgumentException.class, () -> new ContactHistory.Builder() - .setId(5L) + .setContact(contact) + .setRevisionId(5L) .setType(HistoryEntry.Type.CONTACT_CREATE) .setRegistrarId("TheRegistrar") .setReason("Reason") @@ -119,7 +140,8 @@ class HistoryEntryTest extends EntityTestCase { IllegalArgumentException.class, () -> new ContactHistory.Builder() - .setId(5L) + .setRevisionId(5L) + .setContact(contact) .setType(HistoryEntry.Type.CONTACT_CREATE) .setModificationTime(DateTime.parse("1985-07-12T22:30:00Z")) .setReason("Reason") @@ -134,7 +156,8 @@ class HistoryEntryTest extends EntityTestCase { IllegalArgumentException.class, () -> new ContactHistory.Builder() - .setId(5L) + .setContact(contact) + .setRevisionId(5L) .setType(HistoryEntry.Type.SYNTHETIC) .setModificationTime(DateTime.parse("1985-07-12T22:30:00Z")) .setRegistrarId("TheRegistrar") diff --git a/core/src/test/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactoryTest.java b/core/src/test/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactoryTest.java deleted file mode 100644 index fc6a371cd..000000000 --- a/core/src/test/java/google/registry/model/translators/EppHistoryVKeyTranslatorFactoryTest.java +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright 2020 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.model.translators; - -import static google.registry.model.translators.EppHistoryVKeyTranslatorFactory.kindPathToVKeyClass; -import static org.junit.jupiter.api.Assertions.fail; - -import org.junit.jupiter.api.Test; - -/** Unit test for {@link EppHistoryVKeyTranslatorFactory}. */ -class EppHistoryVKeyTranslatorFactoryTest { - - @Test - void assertAllVKeyClassesHavingCreateFromOfyKeyMethod() { - kindPathToVKeyClass.forEach( - (kindPath, vKeyClass) -> { - try { - vKeyClass.getDeclaredMethod("create", com.googlecode.objectify.Key.class); - } catch (NoSuchMethodException e) { - fail("Missing static method create(com.googlecode.objectify.Key) on " + vKeyClass, e); - } - }); - } -} diff --git a/core/src/test/java/google/registry/model/translators/VKeyTranslatorFactoryTest.java b/core/src/test/java/google/registry/model/translators/VKeyTranslatorFactoryTest.java index dbb9cc663..bb3c34263 100644 --- a/core/src/test/java/google/registry/model/translators/VKeyTranslatorFactoryTest.java +++ b/core/src/test/java/google/registry/model/translators/VKeyTranslatorFactoryTest.java @@ -21,9 +21,6 @@ import static google.registry.testing.DatabaseHelper.persistActiveContact; import com.googlecode.objectify.Key; import google.registry.model.common.ClassPathManager; import google.registry.model.domain.Domain; -import google.registry.model.domain.DomainHistory; -import google.registry.model.domain.DomainHistory.DomainHistoryId; -import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; import google.registry.testing.AppEngineExtension; import google.registry.testing.TestObject; @@ -57,18 +54,6 @@ public class VKeyTranslatorFactoryTest { assertThat(vkey.getSqlKey()).isEqualTo("ROID-1"); } - @Test - void testEntityWithAncestor() { - Key domainKey = Key.create(Domain.class, "ROID-1"); - Key historyEntryKey = Key.create(domainKey, HistoryEntry.class, 10L); - - VKey vkey = VKeyTranslatorFactory.createVKey(historyEntryKey); - - assertThat(vkey.getKind()).isEqualTo(DomainHistory.class); - assertThat(vkey.getOfyKey()).isEqualTo(historyEntryKey); - assertThat(vkey.getSqlKey()).isEqualTo(new DomainHistoryId("ROID-1", 10L)); - } - @Test void testExtraEntityClass() { TestObject testObject = TestObject.create("id", "field"); diff --git a/core/src/test/java/google/registry/persistence/DomainHistoryVKeyTest.java b/core/src/test/java/google/registry/persistence/DomainHistoryVKeyTest.java deleted file mode 100644 index 1ee271790..000000000 --- a/core/src/test/java/google/registry/persistence/DomainHistoryVKeyTest.java +++ /dev/null @@ -1,89 +0,0 @@ -// Copyright 2020 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.persistence; - -import static com.google.common.truth.Truth.assertThat; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; - -import com.googlecode.objectify.Key; -import com.googlecode.objectify.annotation.Entity; -import com.googlecode.objectify.annotation.Id; -import google.registry.model.ImmutableObject; -import google.registry.model.domain.Domain; -import google.registry.model.domain.DomainHistory.DomainHistoryId; -import google.registry.model.reporting.HistoryEntry; -import google.registry.testing.AppEngineExtension; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -/** Unit test for {@link DomainHistoryVKey}. */ -class DomainHistoryVKeyTest { - - @RegisterExtension - final AppEngineExtension appEngine = - AppEngineExtension.builder() - .withCloudSql() - .withOfyTestEntities(TestEntity.class) - .withJpaUnitTestEntities(TestEntity.class) - .build(); - - @Test - void testRestoreSymmetricVKey() { - Key ofyKey = - Key.create(Key.create(Domain.class, "domainRepoId"), HistoryEntry.class, 10L); - DomainHistoryVKey domainHistoryVKey = DomainHistoryVKey.create(ofyKey); - TestEntity original = new TestEntity(domainHistoryVKey); - tm().transact(() -> tm().insert(original)); - 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.createSqlKey()) - .isEqualTo(new DomainHistoryId("domainRepoId", 10L)); - assertThat(persisted.domainHistoryVKey.createVKey()) - .isEqualTo(VKey.createSql(HistoryEntry.class, new DomainHistoryId("domainRepoId", 10L))); - } - - @Test - void testCreateSymmetricVKeyFromOfyKey() { - Key ofyKey = - Key.create(Key.create(Domain.class, "domainRepoId"), HistoryEntry.class, 10L); - DomainHistoryVKey domainHistoryVKey = DomainHistoryVKey.create(ofyKey); - assertThat(domainHistoryVKey.createSqlKey()) - .isEqualTo(new DomainHistoryId("domainRepoId", 10L)); - assertThat(domainHistoryVKey.createVKey()) - .isEqualTo( - VKey.create(HistoryEntry.class, new DomainHistoryId("domainRepoId", 10L), ofyKey)); - } - - @Entity - @javax.persistence.Entity(name = "TestEntity") - private static class TestEntity extends ImmutableObject { - - @Id @javax.persistence.Id String id = "id"; - - DomainHistoryVKey domainHistoryVKey; - - TestEntity() {} - - TestEntity(DomainHistoryVKey domainHistoryVKey) { - this.domainHistoryVKey = domainHistoryVKey; - } - - @Override - public VKey createVKey() { - return VKey.createSql(TestEntity.class, id); - } - } -} diff --git a/core/src/test/java/google/registry/rde/DomainToXjcConverterTest.java b/core/src/test/java/google/registry/rde/DomainToXjcConverterTest.java index 0f92d98c5..6e7eb4c32 100644 --- a/core/src/test/java/google/registry/rde/DomainToXjcConverterTest.java +++ b/core/src/test/java/google/registry/rde/DomainToXjcConverterTest.java @@ -365,7 +365,7 @@ public class DomainToXjcConverterTest { .createVKey()) .setServerApproveEntities( domain.getRepoId(), - domainHistory.getId(), + domainHistory.getRevisionId(), ImmutableSet.of(billingEvent.createVKey())) .setTransferRequestTime(DateTime.parse("1919-01-01T00:00:00Z")) .setTransferStatus(TransferStatus.PENDING) diff --git a/core/src/test/java/google/registry/rde/RdeFixtures.java b/core/src/test/java/google/registry/rde/RdeFixtures.java index f4d78ef97..f1b60bf4e 100644 --- a/core/src/test/java/google/registry/rde/RdeFixtures.java +++ b/core/src/test/java/google/registry/rde/RdeFixtures.java @@ -205,8 +205,8 @@ final class RdeFixtures { .build()) .createVKey()) .setServerApproveEntities( - historyEntry.getDomainRepoId(), - historyEntry.getId(), + historyEntry.getRepoId(), + historyEntry.getRevisionId(), ImmutableSet.of(billingEvent.createVKey())) .setTransferRequestTime(DateTime.parse("1991-01-01T00:00:00Z")) .setTransferStatus(TransferStatus.PENDING) diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index 4fedc0d33..cff4b2447 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -550,15 +550,14 @@ public final class DatabaseHelper { public static Contact persistContactWithPendingTransfer( Contact contact, DateTime requestTime, DateTime expirationTime, DateTime now) { - HistoryEntry historyEntryContactTransfer = + ContactHistory historyEntryContactTransfer = persistResource( new ContactHistory.Builder() .setType(HistoryEntry.Type.CONTACT_TRANSFER_REQUEST) .setContact(persistResource(contact)) .setModificationTime(now) .setRegistrarId(contact.getCurrentSponsorRegistrarId()) - .build() - .toChildHistoryEntity()); + .build()); return persistResource( contact .asBuilder() @@ -568,8 +567,8 @@ public final class DatabaseHelper { createContactTransferDataBuilder(requestTime, expirationTime) .setPendingTransferExpirationTime(now.plus(getContactAutomaticTransferLength())) .setServerApproveEntities( - ((ContactHistory) historyEntryContactTransfer).getContactRepoId(), - historyEntryContactTransfer.getId(), + historyEntryContactTransfer.getRepoId(), + historyEntryContactTransfer.getRevisionId(), ImmutableSet.of( // Pretend it's 3 days since the request persistResource( @@ -732,8 +731,8 @@ public final class DatabaseHelper { .setServerApproveAutorenewPollMessage( gainingClientAutorenewPollMessage.createVKey()) .setServerApproveEntities( - historyEntryDomainTransfer.getDomainRepoId(), - historyEntryDomainTransfer.getId(), + historyEntryDomainTransfer.getRepoId(), + historyEntryDomainTransfer.getRevisionId(), ImmutableSet.of( transferBillingEvent.createVKey(), gainingClientAutorenewEvent.createVKey(), @@ -1106,15 +1105,13 @@ public final class DatabaseHelper { tm().loadAllOf(PollMessage.class).stream() .filter( pollMessage -> - pollMessage - .getResourceName() - .equals(historyEntry.getParent().getName()) - && pollMessage.getHistoryRevisionId() == historyEntry.getId() + pollMessage.getResourceId().equals(historyEntry.getRepoId()) + && pollMessage.getHistoryRevisionId() + == historyEntry.getRevisionId() && pollMessage .getType() .getResourceClass() - .getName() - .equals(historyEntry.getParent().getKind())) + .equals(historyEntry.getResourceType())) .collect(toImmutableList()))); } diff --git a/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java b/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java index b0d89c465..9370b1e03 100644 --- a/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java +++ b/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java @@ -30,6 +30,7 @@ import google.registry.model.contact.ContactPhoneNumber; import google.registry.model.contact.PostalInfo; import google.registry.model.domain.DesignatedContact; import google.registry.model.domain.Domain; +import google.registry.model.domain.DomainHistory; import google.registry.model.domain.Period; import google.registry.model.domain.secdns.DomainDsData; import google.registry.model.eppcommon.StatusValue; @@ -381,16 +382,19 @@ public final class FullFieldsTestEntityHelper { Period period, String reason, DateTime modificationTime) { - return HistoryEntry.createBuilderForResource(resource) - .setType(type) - .setPeriod(period) - .setXmlBytes("".getBytes(UTF_8)) - .setModificationTime(modificationTime) - .setRegistrarId(resource.getPersistedCurrentSponsorRegistrarId()) - .setTrid(Trid.create("ABC-123", "server-trid")) - .setBySuperuser(false) - .setReason(reason) - .setRequestedByRegistrar(false) - .build(); + HistoryEntry.Builder builder = + HistoryEntry.createBuilderForResource(resource) + .setType(type) + .setXmlBytes("".getBytes(UTF_8)) + .setModificationTime(modificationTime) + .setRegistrarId(resource.getPersistedCurrentSponsorRegistrarId()) + .setTrid(Trid.create("ABC-123", "server-trid")) + .setBySuperuser(false) + .setReason(reason) + .setRequestedByRegistrar(false); + if (builder instanceof DomainHistory.Builder) { + ((DomainHistory.Builder) builder).setPeriod(period); + } + return builder.build(); } } diff --git a/core/src/test/java/google/registry/testing/HistoryEntrySubject.java b/core/src/test/java/google/registry/testing/HistoryEntrySubject.java index 5e46be75c..da18930c3 100644 --- a/core/src/test/java/google/registry/testing/HistoryEntrySubject.java +++ b/core/src/test/java/google/registry/testing/HistoryEntrySubject.java @@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertAbout; import com.google.common.truth.FailureMetadata; import com.google.common.truth.SimpleSubjectBuilder; import com.google.common.truth.Subject; +import google.registry.model.domain.DomainHistory; import google.registry.model.domain.Period; import google.registry.model.reporting.HistoryEntry; import google.registry.testing.TruthChainer.And; @@ -55,8 +56,12 @@ public class HistoryEntrySubject extends Subject { return hasValue(registrarId, actual.getRegistrarId(), "getRegistrarId()"); } - public And hasOtherClientId(String otherClientId) { - return hasValue(otherClientId, actual.getOtherRegistrarId(), "getOtherRegistrarId()"); + public And hasOtherRegistrarId(String otherRegistrarId) { + if (!(actual instanceof DomainHistory)) { + failWithActual(simpleFact("expected to be DomainHistory")); + } + return hasValue( + otherRegistrarId, ((DomainHistory) actual).getOtherRegistrarId(), "getOtherRegistrarId()"); } public And hasModificationTime(DateTime modificationTime) { @@ -68,18 +73,25 @@ public class HistoryEntrySubject extends Subject { } public And hasPeriod() { - if (actual.getPeriod() == null) { + if (!(actual instanceof DomainHistory)) { + failWithActual(simpleFact("expected to be DomainHistory")); + } + if (((DomainHistory) actual).getPeriod() == null) { failWithActual(simpleFact("expected to have a period")); } return new And<>(this); } public And hasPeriodYears(int years) { + if (!(actual instanceof DomainHistory)) { + failWithActual(simpleFact("expected to be DomainHistory")); + } + Period actualPeriod = ((DomainHistory) actual).getPeriod(); return hasPeriod() .and() - .hasValue(Period.Unit.YEARS, actual.getPeriod().getUnit(), "getPeriod().getUnit()") + .hasValue(Period.Unit.YEARS, actualPeriod.getUnit(), "getPeriod().getUnit()") .and() - .hasValue(years, actual.getPeriod().getValue(), "getPeriod().getValue()"); + .hasValue(years, actualPeriod.getValue(), "getPeriod().getValue()"); } public And hasNoXml() { @@ -93,8 +105,7 @@ public class HistoryEntrySubject extends Subject { return hasValue(reason, actual.getReason(), "getReason()"); } - public And hasMetadataRequestedByRegistrar( - boolean requestedByRegistrar) { + public And hasMetadataRequestedByRegistrar(boolean requestedByRegistrar) { return hasValue( requestedByRegistrar, actual.getRequestedByRegistrar(), "getRequestedByRegistrar()"); } diff --git a/core/src/test/java/google/registry/testing/UserInfo.java b/core/src/test/java/google/registry/testing/UserInfo.java index 7000a55eb..b2217dc32 100644 --- a/core/src/test/java/google/registry/testing/UserInfo.java +++ b/core/src/test/java/google/registry/testing/UserInfo.java @@ -17,7 +17,7 @@ package google.registry.testing; import com.google.auto.value.AutoValue; /** - * Container for values passed to {@link AppEngineExtension} to set the logged in user for tests. + * Container for values passed to {@link AppEngineExtension} to set the logged-in user for tests. */ @AutoValue public abstract class UserInfo { diff --git a/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java b/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java index 0f329fb7d..01d6e4ada 100644 --- a/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java +++ b/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java @@ -24,7 +24,6 @@ import static google.registry.testing.DatabaseHelper.persistResource; import com.google.common.collect.ImmutableList; import google.registry.model.domain.Domain; import google.registry.model.domain.DomainHistory; -import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.poll.PollMessage; import google.registry.model.poll.PollMessage.Autorenew; import google.registry.model.poll.PollMessage.OneTime; @@ -54,10 +53,9 @@ public class AckPollMessagesCommandTest extends CommandTestCase historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 1051L); - builder.setRedemptionHistoryEntry(HistoryEntry.createVKey(historyEntryKey)); + HistoryEntryId historyEntryId = new HistoryEntryId(domain.getRepoId(), 1051L); + builder.setRedemptionHistoryId(historyEntryId); } return persistResource(builder.build()); } diff --git a/core/src/test/java/google/registry/tools/GenerateAllocationTokensCommandTest.java b/core/src/test/java/google/registry/tools/GenerateAllocationTokensCommandTest.java index 759069422..5597b1249 100644 --- a/core/src/test/java/google/registry/tools/GenerateAllocationTokensCommandTest.java +++ b/core/src/test/java/google/registry/tools/GenerateAllocationTokensCommandTest.java @@ -42,8 +42,7 @@ import com.google.common.collect.Iterables; import com.google.common.io.Files; import google.registry.model.domain.token.AllocationToken; import google.registry.model.domain.token.AllocationToken.TokenStatus; -import google.registry.model.reporting.HistoryEntry; -import google.registry.persistence.VKey; +import google.registry.model.reporting.HistoryEntry.HistoryEntryId; import google.registry.testing.DeterministicStringGenerator; import google.registry.testing.DeterministicStringGenerator.Rule; import google.registry.testing.FakeClock; @@ -446,12 +445,12 @@ class GenerateAllocationTokensCommandTest extends CommandTestCase redemptionHistoryEntry, + @Nullable HistoryEntryId redemptionHistoryEntryId, @Nullable String domainName) { AllocationToken.Builder builder = new AllocationToken.Builder().setToken(token).setTokenType(SINGLE_USE); - if (redemptionHistoryEntry != null) { - builder.setRedemptionHistoryEntry(redemptionHistoryEntry); + if (redemptionHistoryEntryId != null) { + builder.setRedemptionHistoryId(redemptionHistoryEntryId); } builder.setDomainName(domainName); return builder.build(); diff --git a/core/src/test/java/google/registry/tools/GetAllocationTokenCommandTest.java b/core/src/test/java/google/registry/tools/GetAllocationTokenCommandTest.java index e74c0aa8e..03aa668a1 100644 --- a/core/src/test/java/google/registry/tools/GetAllocationTokenCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetAllocationTokenCommandTest.java @@ -25,10 +25,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.beust.jcommander.ParameterException; import com.google.common.collect.ImmutableList; -import com.googlecode.objectify.Key; import google.registry.model.domain.Domain; import google.registry.model.domain.token.AllocationToken; -import google.registry.model.reporting.HistoryEntry; import org.joda.time.DateTime; import org.junit.jupiter.api.Test; @@ -83,8 +81,8 @@ class GetAllocationTokenCommandTest extends CommandTestCase parent; - boolean bySuperuser; - byte[] xmlBytes; - google.registry.model.contact.ContactBase contactBase; - google.registry.model.reporting.HistoryEntry$Type type; - java.lang.Boolean requestedByRegistrar; - java.lang.String clientId; - java.lang.String otherClientId; - java.lang.String reason; - org.joda.time.DateTime modificationTime; -} class google.registry.model.domain.Domain { @Id java.lang.String repoId; google.registry.persistence.VKey adminContact; @@ -62,43 +37,6 @@ class google.registry.model.domain.Domain { org.joda.time.DateTime lastTransferTime; org.joda.time.DateTime registrationExpirationTime; } -class google.registry.model.domain.DomainBase { - @Id java.lang.String repoId; - 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; - java.lang.String currentSponsorClientId; - java.lang.String domainName; - java.lang.String idnTableName; - java.lang.String lastEppUpdateClientId; - java.lang.String smdId; - java.lang.String tld; - java.util.Set> nsHosts; - java.util.Set subordinateHosts; - org.joda.time.DateTime autorenewEndTime; - org.joda.time.DateTime deletionTime; - org.joda.time.DateTime lastEppUpdateTime; - org.joda.time.DateTime lastTransferTime; - org.joda.time.DateTime registrationExpirationTime; -} -class google.registry.model.domain.DomainHistory { - @Id java.lang.Long id; - @Parent com.googlecode.objectify.Key parent; - boolean bySuperuser; - byte[] xmlBytes; - google.registry.model.domain.DomainBase domainBase; - google.registry.model.reporting.HistoryEntry$Type type; - java.lang.Boolean requestedByRegistrar; - java.lang.String clientId; - java.lang.String otherClientId; - java.lang.String reason; - java.util.Set> nsHosts; - org.joda.time.DateTime modificationTime; -} class google.registry.model.host.Host { @Id java.lang.String repoId; google.registry.persistence.VKey superordinateDomain; @@ -112,70 +50,3 @@ class google.registry.model.host.Host { org.joda.time.DateTime lastSuperordinateChange; org.joda.time.DateTime lastTransferTime; } -class google.registry.model.host.HostBase { - @Id java.lang.String repoId; - google.registry.persistence.VKey superordinateDomain; - java.lang.String creationClientId; - java.lang.String currentSponsorClientId; - java.lang.String hostName; - java.lang.String lastEppUpdateClientId; - java.util.Set inetAddresses; - org.joda.time.DateTime deletionTime; - org.joda.time.DateTime lastEppUpdateTime; - org.joda.time.DateTime lastSuperordinateChange; - org.joda.time.DateTime lastTransferTime; -} -class google.registry.model.host.HostHistory { - @Id java.lang.Long id; - @Parent com.googlecode.objectify.Key parent; - boolean bySuperuser; - byte[] xmlBytes; - google.registry.model.host.HostBase hostBase; - google.registry.model.reporting.HistoryEntry$Type type; - java.lang.Boolean requestedByRegistrar; - java.lang.String clientId; - java.lang.String otherClientId; - java.lang.String reason; - org.joda.time.DateTime modificationTime; -} -class google.registry.model.reporting.HistoryEntry { - @Id java.lang.Long id; - @Parent com.googlecode.objectify.Key parent; - boolean bySuperuser; - byte[] xmlBytes; - google.registry.model.reporting.HistoryEntry$Type type; - java.lang.Boolean requestedByRegistrar; - java.lang.String clientId; - java.lang.String otherClientId; - java.lang.String reason; - org.joda.time.DateTime modificationTime; -} -enum google.registry.model.reporting.HistoryEntry$Type { - CONTACT_CREATE; - CONTACT_DELETE; - CONTACT_DELETE_FAILURE; - CONTACT_PENDING_DELETE; - CONTACT_TRANSFER_APPROVE; - CONTACT_TRANSFER_CANCEL; - CONTACT_TRANSFER_REJECT; - CONTACT_TRANSFER_REQUEST; - CONTACT_UPDATE; - DOMAIN_ALLOCATE; - DOMAIN_AUTORENEW; - DOMAIN_CREATE; - DOMAIN_DELETE; - DOMAIN_RENEW; - DOMAIN_RESTORE; - DOMAIN_TRANSFER_APPROVE; - DOMAIN_TRANSFER_CANCEL; - DOMAIN_TRANSFER_REJECT; - DOMAIN_TRANSFER_REQUEST; - DOMAIN_UPDATE; - HOST_CREATE; - HOST_DELETE; - HOST_DELETE_FAILURE; - HOST_PENDING_DELETE; - HOST_UPDATE; - RDE_IMPORT; - SYNTHETIC; -} 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 c5c77f816..c71557244 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/db/src/main/resources/sql/schema/db-schema.sql.generated @@ -22,8 +22,8 @@ discount_premiums boolean not null, discount_years int4 not null, domain_name text, - redemption_domain_history_id int8, redemption_domain_repo_id text, + redemption_domain_history_id int8, registration_behavior text not null, renewal_price_behavior text not null, token_status_transitions hstore, @@ -333,6 +333,9 @@ history_server_transaction_id text, history_type text not null, history_xml_bytes bytea, + history_other_registrar_id text, + history_period_unit text, + history_period_value int4, admin_contact text, auth_info_repo_id text, auth_info_value text, @@ -384,9 +387,6 @@ last_epp_update_time timestamptz, statuses text[], update_timestamp timestamptz, - history_other_registrar_id text, - history_period_unit text, - history_period_value int4, primary key (domain_repo_id, history_revision_id) ); diff --git a/docs/flows.md b/docs/flows.md index 1ea25fff1..bc34cd028 100644 --- a/docs/flows.md +++ b/docs/flows.md @@ -463,7 +463,7 @@ automatic one-year renewal at the instant a domain would expire). ICANN prohibits any registration from being longer than ten years so if the request would result in a registration greater than ten years long it will fail. -In practice this means it's impossible to request a ten year renewal, since that +In practice this means it's impossible to request a ten-year renewal, since that will always cause the new registration to be longer than 10 years unless it comes in at the exact millisecond that the domain would have expired. @@ -523,14 +523,14 @@ cannot be restored. After that period anyone can re-register this name. This flow is called a restore "request" because technically it is only supposed to signal that the registrar requests the restore, which the registry can choose -to process or not based on a restore report that is submitted through an out of -band process and details the request. However, in practice this flow does the -restore immediately. This is allowable because all of the fields on a restore +to process or not based on a restore report that is submitted through an +out-of-band process and details the request. However, in practice this flow does +the restore immediately. This is allowable because all the fields on a restore report are optional or have default values, and so by policy when the request comes in we consider it to have been accompanied by a default-initialized report which we auto-approve. -Restores cost a fixed restore fee plus a one year renewal fee for the domain. +Restores cost a fixed restore fee plus a one-year renewal fee for the domain. The domain is restored to a single year expiration starting at the restore time, regardless of what the original expiration time was.