From 4e312f2482fd37073af690b5df167c39d0fea550 Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Fri, 14 May 2021 19:54:35 -0400 Subject: [PATCH] Add sanity checks to history entry construction (#1156) * Add sanity checks to history entry construction * Add more missing setClientId() calls and delete scrap tool * Merge branch 'master' into synthetic-requestedby * Set more client IDs * Merge branch 'master' into synthetic-requestedby --- .../model/reporting/HistoryEntry.java | 9 + ...DedupeRecurringBillingEventIdsCommand.java | 191 ------------- .../google/registry/tools/RegistryTool.java | 1 - .../batch/DeleteExpiredDomainsActionTest.java | 1 + .../batch/DeleteProberDataActionTest.java | 13 +- ...xpandRecurringBillingEventsActionTest.java | 27 +- .../beam/initsql/DomainBaseUtilTest.java | 9 +- .../beam/initsql/InitSqlPipelineTest.java | 1 + .../flows/domain/DomainDeleteFlowTest.java | 2 + .../flows/domain/DomainInfoFlowTest.java | 1 + .../flows/domain/DomainRenewFlowTest.java | 1 + .../domain/DomainRestoreRequestFlowTest.java | 1 + .../domain/DomainTransferApproveFlowTest.java | 1 + .../domain/DomainTransferCancelFlowTest.java | 1 + .../domain/DomainTransferRejectFlowTest.java | 1 + .../flows/domain/DomainUpdateFlowTest.java | 2 + .../flows/poll/PollRequestFlowTest.java | 28 +- .../inputs/ChildEntityInputTest.java | 27 +- .../model/billing/BillingEventTest.java | 2 + .../model/domain/DomainBaseSqlTest.java | 4 +- .../registry/model/domain/DomainBaseTest.java | 19 +- .../google/registry/model/ofy/OfyTest.java | 3 +- .../model/reporting/HistoryEntryTest.java | 67 +++++ .../rde/DomainBaseToXjcConverterTest.java | 1 + .../java/google/registry/rde/RdeFixtures.java | 8 +- .../registry/testing/DatabaseHelper.java | 5 +- .../tools/AckPollMessagesCommandTest.java | 1 + ...peRecurringBillingEventIdsCommandTest.java | 258 ------------------ .../tools/UpdateDomainCommandTest.java | 1 + .../server/KillAllEppResourcesActionTest.java | 93 ++++--- .../ResaveAllHistoryEntriesActionTest.java | 20 +- 31 files changed, 276 insertions(+), 523 deletions(-) delete mode 100644 core/src/main/java/google/registry/tools/DedupeRecurringBillingEventIdsCommand.java delete mode 100644 core/src/test/java/google/registry/tools/DedupeRecurringBillingEventIdsCommandTest.java 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 f390a56d2..080e5c90f 100644 --- a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java +++ b/core/src/main/java/google/registry/model/reporting/HistoryEntry.java @@ -14,8 +14,10 @@ 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.nullToEmptyImmutableCopy; +import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableSet; @@ -383,6 +385,13 @@ public class HistoryEntry extends ImmutableObject implements Buildable, Datastor @Override public T build() { + // TODO(mcilwain): Add null checking for id/parent once DB migration is complete. + 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"); + checkArgument( + !getInstance().type.equals(Type.SYNTHETIC) || !getInstance().requestedByRegistrar, + "Synthetic history entries cannot be requested by a registrar"); return super.build(); } diff --git a/core/src/main/java/google/registry/tools/DedupeRecurringBillingEventIdsCommand.java b/core/src/main/java/google/registry/tools/DedupeRecurringBillingEventIdsCommand.java deleted file mode 100644 index e131e4346..000000000 --- a/core/src/main/java/google/registry/tools/DedupeRecurringBillingEventIdsCommand.java +++ /dev/null @@ -1,191 +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.tools; - -import static com.google.common.collect.ImmutableSet.toImmutableSet; -import static google.registry.model.ofy.ObjectifyService.auditedOfy; - -import com.beust.jcommander.Parameters; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Sets; -import com.googlecode.objectify.Key; -import google.registry.model.billing.BillingEvent; -import google.registry.model.billing.BillingEvent.OneTime; -import google.registry.model.billing.BillingEvent.Recurring; -import google.registry.model.domain.DomainBase; -import google.registry.model.domain.GracePeriod; -import google.registry.model.transfer.DomainTransferData; -import google.registry.model.transfer.TransferData.TransferServerApproveEntity; -import google.registry.persistence.VKey; -import java.util.List; -import java.util.Set; - -/** - * A command that re-saves the problematic {@link BillingEvent.Recurring} entities with unique IDs. - * - *

This command is used to address the duplicate id issue we found for certain {@link - * BillingEvent.Recurring} entities. The command reassigns an application wide unique id to the - * problematic entity and resaves it, it also resaves the entity having reference to the problematic - * entity with the updated id. - * - *

To use this command, you will need to provide the path to a file containing a list of strings - * representing the literal of Objectify key for the problematic entities. An example key literal - * is: - * - * 

- * "DomainBase", "111111-TEST", "HistoryEntry", 2222222, "Recurring", 3333333
- *
- * - *

Note that the double quotes are part of the key literal. The key literal can be retrieved from - * the column __key__.path in BigQuery. - */ -@Parameters( - separators = " =", - commandDescription = "Dedupe BillingEvent.Recurring entities with duplicate IDs.") -public class DedupeRecurringBillingEventIdsCommand extends ReadEntityFromKeyPathCommand { - - @Override - void process(Recurring recurring) { - // Loads the associated DomainBase and BillingEvent.OneTime entities that - // may have reference to this BillingEvent.Recurring entity. - Key domainKey = getGrandParentAsDomain(Key.create(recurring)); - DomainBase domain = auditedOfy().load().key(domainKey).now(); - List oneTimes = - auditedOfy().load().type(BillingEvent.OneTime.class).ancestor(domainKey).list(); - - VKey oldRecurringVKey = recurring.createVKey(); - // By setting id to 0L, Buildable.build() will assign an application wide unique id to it. - Recurring uniqIdRecurring = recurring.asBuilder().setId(0L).build(); - VKey newRecurringVKey = uniqIdRecurring.createVKey(); - - // After having the unique id for the BillingEvent.Recurring entity, we also need to - // update the references in other entities to point to the new BillingEvent.Recurring - // entity. - updateReferenceInOneTimeBillingEvent(oneTimes, oldRecurringVKey, newRecurringVKey); - updateReferenceInDomain(domain, oldRecurringVKey, newRecurringVKey); - - stageEntityKeyChange(recurring, uniqIdRecurring); - } - - /** - * Resaves {@link BillingEvent.OneTime} entities with updated {@link - * BillingEvent.OneTime#cancellationMatchingBillingEvent}. - * - *

{@link BillingEvent.OneTime#cancellationMatchingBillingEvent} is a {@link VKey} to a {@link - * BillingEvent.Recurring} entity. So, if the {@link BillingEvent.Recurring} entity gets a new key - * by changing its id, we need to update {@link - * BillingEvent.OneTime#cancellationMatchingBillingEvent} as well. - */ - private void updateReferenceInOneTimeBillingEvent( - List oneTimes, VKey oldRecurringVKey, VKey newRecurringVKey) { - oneTimes.forEach( - oneTime -> { - if (oneTime.getCancellationMatchingBillingEvent() != null - && oneTime.getCancellationMatchingBillingEvent().equals(oldRecurringVKey)) { - BillingEvent.OneTime updatedOneTime = - oneTime.asBuilder().setCancellationMatchingBillingEvent(newRecurringVKey).build(); - stageEntityChange(oneTime, updatedOneTime); - appendChangeMessage( - String.format( - "Changed cancellationMatchingBillingEvent in entity %s from %s to %s\n", - oneTime.createVKey().getOfyKey(), - oneTime.getCancellationMatchingBillingEvent().getOfyKey(), - updatedOneTime.getCancellationMatchingBillingEvent().getOfyKey())); - } - }); - } - - /** - * Resaves {@link DomainBase} entity with updated references to {@link BillingEvent.Recurring} - * entity. - * - *

The following 4 fields in the domain entity can be or have a reference to this - * BillingEvent.Recurring entity, so we need to check them and replace them with the new entity - * when necessary: - * - *

    - *
  1. domain.autorenewBillingEvent, see {@link DomainBase#autorenewBillingEvent} - *
  2. domain.transferData.serverApproveAutorenewEvent, see {@link - * DomainTransferData#serverApproveAutorenewEvent} - *
  3. domain.transferData.serverApproveEntities, see {@link - * DomainTransferData#serverApproveEntities} - *
  4. domain.gracePeriods.billingEventRecurring, see {@link GracePeriod#billingEventRecurring} - *
- */ - private void updateReferenceInDomain( - DomainBase domain, VKey oldRecurringVKey, VKey newRecurringVKey) { - DomainBase.Builder domainBuilder = domain.asBuilder(); - StringBuilder domainChange = - new StringBuilder( - String.format( - "Resaved domain %s with following changes:\n", domain.createVKey().getOfyKey())); - - if (domain.getAutorenewBillingEvent() != null - && domain.getAutorenewBillingEvent().equals(oldRecurringVKey)) { - domainBuilder.setAutorenewBillingEvent(newRecurringVKey); - domainChange.append( - String.format( - " Changed autorenewBillingEvent from %s to %s.\n", - oldRecurringVKey, newRecurringVKey)); - } - - if (domain.getTransferData().getServerApproveAutorenewEvent() != null - && domain.getTransferData().getServerApproveAutorenewEvent().equals(oldRecurringVKey)) { - Set> serverApproveEntities = - Sets.union( - Sets.difference( - domain.getTransferData().getServerApproveEntities(), - ImmutableSet.of(oldRecurringVKey)), - ImmutableSet.of(newRecurringVKey)); - domainBuilder.setTransferData( - domain - .getTransferData() - .asBuilder() - .setServerApproveEntities(ImmutableSet.copyOf(serverApproveEntities)) - .setServerApproveAutorenewEvent(newRecurringVKey) - .build()); - domainChange.append( - String.format( - " Changed transferData.serverApproveAutoRenewEvent from %s to %s.\n", - oldRecurringVKey, newRecurringVKey)); - domainChange.append( - String.format( - " Changed transferData.serverApproveEntities to remove %s and add %s.\n", - oldRecurringVKey, newRecurringVKey)); - } - - ImmutableSet updatedGracePeriod = - domain.getGracePeriods().stream() - .map( - gracePeriod -> - gracePeriod.getRecurringBillingEvent().equals(oldRecurringVKey) - ? gracePeriod.cloneWithRecurringBillingEvent(newRecurringVKey) - : gracePeriod) - .collect(toImmutableSet()); - if (!updatedGracePeriod.equals(domain.getGracePeriods())) { - domainBuilder.setGracePeriods(updatedGracePeriod); - domainChange.append( - String.format( - " Changed gracePeriods to remove %s and add %s.\n", - oldRecurringVKey, newRecurringVKey)); - } - - DomainBase updatedDomain = domainBuilder.build(); - if (!updatedDomain.equals(domain)) { - stageEntityChange(domain, updatedDomain); - appendChangeMessage(domainChange.toString()); - } - } -} diff --git a/core/src/main/java/google/registry/tools/RegistryTool.java b/core/src/main/java/google/registry/tools/RegistryTool.java index ebfea9bde..0f808b58a 100644 --- a/core/src/main/java/google/registry/tools/RegistryTool.java +++ b/core/src/main/java/google/registry/tools/RegistryTool.java @@ -53,7 +53,6 @@ public final class RegistryTool { .put("create_tld", CreateTldCommand.class) .put("curl", CurlCommand.class) .put("dedupe_one_time_billing_event_ids", DedupeOneTimeBillingEventIdsCommand.class) - .put("dedupe_recurring_billing_event_ids", DedupeRecurringBillingEventIdsCommand.class) .put("delete_allocation_tokens", DeleteAllocationTokensCommand.class) .put("delete_contact_by_roid", DeleteContactByRoidCommand.class) .put("delete_domain", DeleteDomainCommand.class) diff --git a/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java b/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java index 06c823d05..e5e135561 100644 --- a/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java +++ b/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java @@ -170,6 +170,7 @@ class DeleteExpiredDomainsActionTest { .setType(DOMAIN_CREATE) .setParent(pendingExpirationDomain) .setModificationTime(clock.nowUtc().minusMonths(9)) + .setClientId(pendingExpirationDomain.getCreationClientId()) .build()); BillingEvent.Recurring autorenewBillingEvent = persistResource(createAutorenewBillingEvent(createHistoryEntry).build()); diff --git a/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java b/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java index 124c06edf..8c8207a88 100644 --- a/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java +++ b/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java @@ -279,11 +279,14 @@ class DeleteProberDataActionTest extends MapreduceTestCase persistDomainAndDescendants(String fqdn) { DomainBase domain = persistDeletedDomain(fqdn, DELETION_TIME); - HistoryEntry historyEntry = persistSimpleResource( - new HistoryEntry.Builder() - .setParent(domain) - .setType(HistoryEntry.Type.DOMAIN_CREATE) - .build()); + HistoryEntry historyEntry = + persistSimpleResource( + new HistoryEntry.Builder() + .setParent(domain) + .setType(HistoryEntry.Type.DOMAIN_CREATE) + .setClientId("TheRegistrar") + .setModificationTime(DELETION_TIME.minusYears(3)) + .build()); BillingEvent.OneTime billingEvent = persistSimpleResource( new BillingEvent.OneTime.Builder() .setParent(historyEntry) diff --git a/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java b/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java index 6b454eb56..8ca36fdd6 100644 --- a/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java +++ b/core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java @@ -19,6 +19,7 @@ import static google.registry.model.common.Cursor.CursorType.RECURRING_BILLING; import static google.registry.model.domain.Period.Unit.YEARS; import static google.registry.model.ofy.ObjectifyService.auditedOfy; import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_AUTORENEW; +import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.assertBillingEvents; import static google.registry.testing.DatabaseHelper.assertBillingEventsForResource; @@ -84,9 +85,20 @@ public class ExpandRecurringBillingEventsActionTest action.clock = clock; action.cursorTimeParam = Optional.empty(); createTld("tld"); - domain = persistResource(newDomainBase("example.tld").asBuilder() - .setCreationTimeForTest(DateTime.parse("1999-01-05T00:00:00Z")).build()); - historyEntry = persistResource(new HistoryEntry.Builder().setParent(domain).build()); + domain = + persistResource( + newDomainBase("example.tld") + .asBuilder() + .setCreationTimeForTest(DateTime.parse("1999-01-05T00:00:00Z")) + .build()); + historyEntry = + persistResource( + new HistoryEntry.Builder() + .setClientId(domain.getCreationClientId()) + .setType(HistoryEntry.Type.DOMAIN_CREATE) + .setModificationTime(DateTime.parse("1999-01-05T00:00:00Z")) + .setParent(domain) + .build()); recurring = new BillingEvent.Recurring.Builder() .setParent(historyEntry) @@ -174,7 +186,14 @@ public class ExpandRecurringBillingEventsActionTest void testSuccess_expandSingleEvent_deletedDomain() throws Exception { DateTime deletionTime = DateTime.parse("2000-08-01T00:00:00Z"); DomainBase deletedDomain = persistDeletedDomain("deleted.tld", deletionTime); - historyEntry = persistResource(new HistoryEntry.Builder().setParent(deletedDomain).build()); + historyEntry = + persistResource( + new HistoryEntry.Builder() + .setParent(deletedDomain) + .setClientId(deletedDomain.getCreationClientId()) + .setModificationTime(deletedDomain.getCreationTime()) + .setType(DOMAIN_CREATE) + .build()); recurring = persistResource( new BillingEvent.Recurring.Builder() diff --git a/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java b/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java index 8707bd48d..b45bd27b3 100644 --- a/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java +++ b/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java @@ -100,7 +100,14 @@ public class DomainBaseUtilTest { .build()) .createVKey(); Key historyEntryKey = - Key.create(persistResource(new HistoryEntry.Builder().setParent(domainKey).build())); + Key.create( + persistResource( + new HistoryEntry.Builder() + .setParent(domainKey) + .setType(HistoryEntry.Type.DOMAIN_CREATE) + .setClientId("TheRegistrar") + .setModificationTime(fakeClock.nowUtc().minusYears(1)) + .build())); oneTimeBillKey = Key.create(historyEntryKey, BillingEvent.OneTime.class, 1); recurringBillKey = VKey.from(Key.create(historyEntryKey, BillingEvent.Recurring.class, 2)); VKey autorenewPollKey = diff --git a/core/src/test/java/google/registry/beam/initsql/InitSqlPipelineTest.java b/core/src/test/java/google/registry/beam/initsql/InitSqlPipelineTest.java index 9800c93d1..294638b20 100644 --- a/core/src/test/java/google/registry/beam/initsql/InitSqlPipelineTest.java +++ b/core/src/test/java/google/registry/beam/initsql/InitSqlPipelineTest.java @@ -181,6 +181,7 @@ class InitSqlPipelineTest { new HistoryEntry.Builder() .setParent(domainKey) .setModificationTime(fakeClock.nowUtc()) + .setClientId(registrar1.getClientId()) .setType(HistoryEntry.Type.DOMAIN_CREATE) .build()); persistResource( diff --git a/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java index e99ea967a..a86ccda8d 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java @@ -179,6 +179,7 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase { void testSuccess_contactDelete() throws Exception { // Contact delete poll messages do not have any response data, so ensure that no // response data block is produced in the poll message. - HistoryEntry historyEntry = persistResource(new HistoryEntry.Builder() - .setClientId("NewRegistrar") - .setModificationTime(clock.nowUtc().minusDays(1)) - .setType(HistoryEntry.Type.CONTACT_DELETE) - .setParent(contact) - .build()); + HistoryEntry historyEntry = + persistResource( + new HistoryEntry.Builder() + .setClientId("NewRegistrar") + .setModificationTime(clock.nowUtc().minusDays(1)) + .setType(HistoryEntry.Type.CONTACT_DELETE) + .setParent(contact) + .build()); persistResource( new PollMessage.OneTime.Builder() .setClientId("NewRegistrar") @@ -229,12 +231,14 @@ class PollRequestFlowTest extends FlowTestCase { void testSuccess_hostDelete() throws Exception { // Host delete poll messages do not have any response data, so ensure that no // response data block is produced in the poll message. - HistoryEntry historyEntry = persistResource(new HistoryEntry.Builder() - .setClientId("NewRegistrar") - .setModificationTime(clock.nowUtc().minusDays(1)) - .setType(HistoryEntry.Type.HOST_DELETE) - .setParent(host) - .build()); + HistoryEntry historyEntry = + persistResource( + new HistoryEntry.Builder() + .setClientId("NewRegistrar") + .setModificationTime(clock.nowUtc().minusDays(1)) + .setType(HistoryEntry.Type.HOST_DELETE) + .setParent(host) + .build()); persistResource( new PollMessage.OneTime.Builder() .setClientId("NewRegistrar") diff --git a/core/src/test/java/google/registry/mapreduce/inputs/ChildEntityInputTest.java b/core/src/test/java/google/registry/mapreduce/inputs/ChildEntityInputTest.java index 002ee33a8..429e6e380 100644 --- a/core/src/test/java/google/registry/mapreduce/inputs/ChildEntityInputTest.java +++ b/core/src/test/java/google/registry/mapreduce/inputs/ChildEntityInputTest.java @@ -80,10 +80,22 @@ class ChildEntityInputTest { domainA = persistEppResourceInFirstBucket(newDomainBase("a.tld", contact)); domainHistoryEntryA = persistResource( - new DomainHistory.Builder().setDomain(domainA).setModificationTime(now).build()); + new DomainHistory.Builder() + .setDomain(domainA) + .setType(HistoryEntry.Type.DOMAIN_CREATE) + .setDomain(domainA) + .setModificationTime(now) + .setClientId(domainA.getCreationClientId()) + .build()); contactHistoryEntry = persistResource( - new ContactHistory.Builder().setContact(contact).setModificationTime(now).build()); + new ContactHistory.Builder() + .setContact(contact) + .setType(HistoryEntry.Type.CONTACT_CREATE) + .setContact(contact) + .setModificationTime(now) + .setClientId(contact.getCreationClientId()) + .build()); oneTimeA = persistResource( new BillingEvent.OneTime.Builder() @@ -113,7 +125,13 @@ class ChildEntityInputTest { domainB = persistEppResourceInFirstBucket(newDomainBase("b.tld")); domainHistoryEntryB = persistResource( - new DomainHistory.Builder().setDomain(domainB).setModificationTime(now).build()); + new DomainHistory.Builder() + .setDomain(domainB) + .setType(HistoryEntry.Type.DOMAIN_CREATE) + .setDomain(domainB) + .setModificationTime(now) + .setClientId(domainB.getCreationClientId()) + .build()); oneTimeB = persistResource( new BillingEvent.OneTime.Builder() @@ -295,8 +313,9 @@ class ChildEntityInputTest { persistResource( new DomainHistory.Builder() .setDomain(domain) + .setType(HistoryEntry.Type.DOMAIN_CREATE) .setModificationTime(now) - .setClientId(i + ".tld") + .setClientId(domain.getCreationClientId()) .build()) .asHistoryEntry()); persistResource(EppResourceIndex.create(getBucketKey(i), Key.create(domain))); 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 ddd1e3494..184bfa903 100644 --- a/core/src/test/java/google/registry/model/billing/BillingEventTest.java +++ b/core/src/test/java/google/registry/model/billing/BillingEventTest.java @@ -80,6 +80,7 @@ public class BillingEventTest extends EntityTestCase { .setDomain(domain) .setModificationTime(now) .setRequestedByRegistrar(false) + .setClientId("TheRegistrar") .setType(HistoryEntry.Type.DOMAIN_CREATE) .setXmlBytes(new byte[0]) .build()); @@ -89,6 +90,7 @@ public class BillingEventTest extends EntityTestCase { .setDomain(domain) .setModificationTime(now.plusDays(1)) .setRequestedByRegistrar(false) + .setClientId("TheRegistrar") .setType(HistoryEntry.Type.DOMAIN_CREATE) .setXmlBytes(new byte[0]) .build()); diff --git a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java index 0f3942db8..3c3dc160c 100644 --- a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java @@ -443,7 +443,7 @@ public class DomainBaseSqlTest { .setModificationTime(DateTime.now(UTC)) .setParent(Key.create(DomainBase.class, "4-COM")) .setDomainRepoId("4-COM") - + .setClientId("registrar1") // These are non-null, but I don't think some tests set them. .setReason("felt like it") .setRequestedByRegistrar(false) @@ -574,7 +574,7 @@ public class DomainBaseSqlTest { .setModificationTime(DateTime.now(UTC)) .setParent(Key.create(DomainBase.class, "4-COM")) .setDomainRepoId("4-COM") - + .setClientId("registrar1") // These are non-null, but I don't think some tests set them. .setReason("felt like it") .setRequestedByRegistrar(false) diff --git a/core/src/test/java/google/registry/model/domain/DomainBaseTest.java b/core/src/test/java/google/registry/model/domain/DomainBaseTest.java index 0f4dd593e..0c8d934c2 100644 --- a/core/src/test/java/google/registry/model/domain/DomainBaseTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainBaseTest.java @@ -63,6 +63,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; /** Unit tests for {@link DomainBase}. */ +@SuppressWarnings("WeakerAccess") // Referred to by EppInputTest. public class DomainBaseTest extends EntityTestCase { private DomainBase domain; @@ -98,7 +99,13 @@ public class DomainBaseTest extends EntityTestCase { .createVKey(); historyEntryKey = Key.create( - persistResource(new HistoryEntry.Builder().setParent(domainKey.getOfyKey()).build())); + persistResource( + new HistoryEntry.Builder() + .setParent(domainKey.getOfyKey()) + .setModificationTime(fakeClock.nowUtc()) + .setType(HistoryEntry.Type.DOMAIN_CREATE) + .setClientId("aregistrar") + .build())); oneTimeBillKey = VKey.from(Key.create(historyEntryKey, BillingEvent.OneTime.class, 1)); recurringBillKey = VKey.from(Key.create(historyEntryKey, BillingEvent.Recurring.class, 2)); VKey autorenewPollKey = @@ -112,7 +119,7 @@ public class DomainBaseTest extends EntityTestCase { new DomainBase.Builder() .setDomainName("example.com") .setRepoId("4-COM") - .setCreationClientId("a registrar") + .setCreationClientId("aregistrar") .setLastEppUpdateTime(fakeClock.nowUtc()) .setLastEppUpdateClientId("AnotherRegistrar") .setLastTransferTime(fakeClock.nowUtc()) @@ -356,7 +363,13 @@ public class DomainBaseTest extends EntityTestCase { } private void doExpiredTransferTest(DateTime oldExpirationTime) { - HistoryEntry historyEntry = new HistoryEntry.Builder().setParent(domain).build(); + HistoryEntry historyEntry = + new HistoryEntry.Builder() + .setParent(domain) + .setModificationTime(fakeClock.nowUtc()) + .setClientId(domain.getCurrentSponsorClientId()) + .setType(HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST) + .build(); BillingEvent.OneTime transferBillingEvent = persistResource( new BillingEvent.OneTime.Builder() diff --git a/core/src/test/java/google/registry/model/ofy/OfyTest.java b/core/src/test/java/google/registry/model/ofy/OfyTest.java index 935d9ec21..d5d8e29a8 100644 --- a/core/src/test/java/google/registry/model/ofy/OfyTest.java +++ b/core/src/test/java/google/registry/model/ofy/OfyTest.java @@ -71,8 +71,9 @@ public class OfyTest { createTld("tld"); someObject = new HistoryEntry.Builder() - .setClientId("client id") + .setClientId("clientid") .setModificationTime(START_OF_TIME) + .setType(HistoryEntry.Type.CONTACT_CREATE) .setParent(persistActiveContact("parentContact")) .setTrid(Trid.create("client", "server")) .setXmlBytes("".getBytes(UTF_8)) 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 8f4f340dc..39d1df8a7 100644 --- a/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java +++ b/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java @@ -14,6 +14,7 @@ package google.registry.model.reporting; +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.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; @@ -21,6 +22,7 @@ import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistResource; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -33,6 +35,7 @@ import google.registry.model.reporting.DomainTransactionRecord.TransactionReport import google.registry.testing.DualDatabaseTest; import google.registry.testing.TestOfyAndSql; import google.registry.testing.TestOfyOnly; +import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; /** Unit tests for {@link HistoryEntry}. */ @@ -87,6 +90,70 @@ class HistoryEntryTest extends EntityTestCase { }); } + @TestOfyAndSql + void testBuilder_typeMustBeSpecified() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> + new HistoryEntry.Builder() + .setId(5L) + .setModificationTime(DateTime.parse("1985-07-12T22:30:00Z")) + .setClientId("TheRegistrar") + .setReason("Reason") + .build()); + assertThat(thrown).hasMessageThat().isEqualTo("History entry type must be specified"); + } + + @TestOfyAndSql + void testBuilder_modificationTimeMustBeSpecified() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> + new HistoryEntry.Builder() + .setId(5L) + .setType(HistoryEntry.Type.CONTACT_CREATE) + .setClientId("TheRegistrar") + .setReason("Reason") + .build()); + assertThat(thrown).hasMessageThat().isEqualTo("Modification time must be specified"); + } + + @TestOfyAndSql + void testBuilder_registrarIdMustBeSpecified() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> + new HistoryEntry.Builder() + .setId(5L) + .setType(HistoryEntry.Type.CONTACT_CREATE) + .setModificationTime(DateTime.parse("1985-07-12T22:30:00Z")) + .setReason("Reason") + .build()); + assertThat(thrown).hasMessageThat().isEqualTo("Registrar ID must be specified"); + } + + @TestOfyAndSql + void testBuilder_syntheticHistoryEntries_mustNotBeRequestedByRegistrar() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> + new HistoryEntry.Builder() + .setId(5L) + .setType(HistoryEntry.Type.SYNTHETIC) + .setModificationTime(DateTime.parse("1985-07-12T22:30:00Z")) + .setClientId("TheRegistrar") + .setReason("Reason") + .setRequestedByRegistrar(true) + .build()); + assertThat(thrown) + .hasMessageThat() + .isEqualTo("Synthetic history entries cannot be requested by a registrar"); + } + @TestOfyOnly void testIndexing() throws Exception { verifyIndexing(domainHistory.asHistoryEntry(), "modificationTime", "clientId"); diff --git a/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java b/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java index 47ec369ef..dfeaec625 100644 --- a/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java +++ b/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java @@ -225,6 +225,7 @@ public class DomainBaseToXjcConverterTest { .setModificationTime(clock.nowUtc()) .setType(HistoryEntry.Type.DOMAIN_CREATE) .setDomain(domain) + .setClientId(domain.getCreationClientId()) .build()); BillingEvent.OneTime billingEvent = persistResource( diff --git a/core/src/test/java/google/registry/rde/RdeFixtures.java b/core/src/test/java/google/registry/rde/RdeFixtures.java index f9a055d05..fed23535a 100644 --- a/core/src/test/java/google/registry/rde/RdeFixtures.java +++ b/core/src/test/java/google/registry/rde/RdeFixtures.java @@ -66,7 +66,13 @@ final class RdeFixtures { .createVKey()) .build(); HistoryEntry historyEntry = - persistResource(new HistoryEntry.Builder().setParent(domain).build()); + persistResource( + new HistoryEntry.Builder() + .setParent(domain) + .setType(HistoryEntry.Type.DOMAIN_CREATE) + .setModificationTime(clock.nowUtc()) + .setClientId("TheRegistrar") + .build()); clock.advanceOneMilli(); BillingEvent.OneTime billingEvent = persistResourceWithCommitLog( diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index b03e9118e..dc90a8b78 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -545,6 +545,7 @@ public class DatabaseHelper { .setType(HistoryEntry.Type.CONTACT_TRANSFER_REQUEST) .setParent(persistResource(contact)) .setModificationTime(now) + .setClientId(contact.getCurrentSponsorClientId()) .build() .toChildHistoryEntity()); return persistResource( @@ -616,6 +617,7 @@ public class DatabaseHelper { .setType(HistoryEntry.Type.DOMAIN_CREATE) .setModificationTime(now) .setDomain(domain) + .setClientId(domain.getCreationClientId()) .build()); BillingEvent.Recurring autorenewEvent = persistResource( @@ -657,6 +659,7 @@ public class DatabaseHelper { .setType(HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST) .setModificationTime(tm().transact(() -> tm().getTransactionTime())) .setDomain(domain) + .setClientId("TheRegistrar") .build()); BillingEvent.OneTime transferBillingEvent = persistResource( @@ -1085,7 +1088,7 @@ public class DatabaseHelper { tm().put( new HistoryEntry.Builder() .setParent(resource) - .setClientId(resource.getPersistedCurrentSponsorClientId()) + .setClientId(resource.getCreationClientId()) .setType(getHistoryEntryType(resource)) .setModificationTime(tm().getTransactionTime()) .build() diff --git a/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java b/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java index c61f17be7..6d9406bb5 100644 --- a/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java +++ b/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java @@ -62,6 +62,7 @@ public class AckPollMessagesCommandTest extends CommandTestCase { - - private final DateTime now = DateTime.now(UTC); - private DomainBase domain1; - private DomainBase domain2; - private HistoryEntry historyEntry1; - private HistoryEntry historyEntry2; - private BillingEvent.Recurring recurring1; - private BillingEvent.Recurring recurring2; - - @BeforeEach - void beforeEach() { - createTld("tld"); - domain1 = persistActiveDomain("foo.tld"); - domain2 = persistActiveDomain("bar.tld"); - historyEntry1 = - persistResource( - new HistoryEntry.Builder().setParent(domain1).setModificationTime(now).build()); - historyEntry2 = - persistResource( - new HistoryEntry.Builder() - .setParent(domain2) - .setModificationTime(now.plusDays(1)) - .build()); - recurring1 = - persistResource( - new BillingEvent.Recurring.Builder() - .setParent(historyEntry1) - .setFlags(ImmutableSet.of(Flag.AUTO_RENEW)) - .setReason(Reason.RENEW) - .setEventTime(now.plusYears(1)) - .setRecurrenceEndTime(END_OF_TIME) - .setClientId("a registrar") - .setTargetId("foo.tld") - .build()); - recurring2 = - persistResource( - new BillingEvent.Recurring.Builder() - .setId(recurring1.getId()) - .setParent(historyEntry2) - .setFlags(ImmutableSet.of(Flag.AUTO_RENEW)) - .setReason(Reason.RENEW) - .setEventTime(now.plusYears(1)) - .setRecurrenceEndTime(END_OF_TIME) - .setClientId("a registrar") - .setTargetId("bar.tld") - .build()); - } - - @Test - void testOnlyResaveBillingEventsCorrectly() throws Exception { - assertThat(recurring1.getId()).isEqualTo(recurring2.getId()); - - runCommand( - "--force", - "--key_paths_file", - writeToNamedTmpFile("keypath.txt", getKeyPathLiteral(recurring1, recurring2))); - - assertNotChangeExceptUpdateTime(domain1, domain2, historyEntry1, historyEntry2); - assertNotInDatastore(recurring1, recurring2); - - ImmutableList recurrings = loadAllRecurrings(); - assertThat(recurrings.size()).isEqualTo(2); - - recurrings.forEach( - newRecurring -> { - if (newRecurring.getTargetId().equals("foo.tld")) { - assertSameRecurringEntityExceptId(newRecurring, recurring1); - } else if (newRecurring.getTargetId().equals("bar.tld")) { - assertSameRecurringEntityExceptId(newRecurring, recurring2); - } else { - fail("Unknown BillingEvent.Recurring entity: " + newRecurring.createVKey()); - } - }); - } - - @Test - void testResaveAssociatedDomainAndOneTimeBillingEventCorrectly() throws Exception { - assertThat(recurring1.getId()).isEqualTo(recurring2.getId()); - GracePeriod gracePeriod = - GracePeriod.createForRecurring( - GracePeriodStatus.AUTO_RENEW, - domain1.getRepoId(), - now.plusDays(45), - "a registrar", - recurring1.createVKey()); - domain1 = - persistResource( - domain1 - .asBuilder() - .setAutorenewBillingEvent(recurring1.createVKey()) - .setGracePeriods(ImmutableSet.of(gracePeriod)) - .setTransferData( - new DomainTransferData.Builder() - .setServerApproveAutorenewEvent(recurring1.createVKey()) - .setServerApproveEntities(ImmutableSet.of(recurring1.createVKey())) - .build()) - .build()); - - BillingEvent.OneTime oneTime = - persistResource( - new BillingEvent.OneTime.Builder() - .setClientId("a registrar") - .setTargetId("foo.tld") - .setParent(historyEntry1) - .setReason(Reason.CREATE) - .setFlags(ImmutableSet.of(Flag.SYNTHETIC)) - .setSyntheticCreationTime(now) - .setPeriodYears(2) - .setCost(Money.of(USD, 1)) - .setEventTime(now) - .setBillingTime(now.plusDays(5)) - .setCancellationMatchingBillingEvent(recurring1.createVKey()) - .build()); - - runCommand( - "--force", - "--key_paths_file", - writeToNamedTmpFile("keypath.txt", getKeyPathLiteral(recurring1, recurring2))); - - assertNotChangeExceptUpdateTime(domain2, historyEntry1, historyEntry2); - assertNotInDatastore(recurring1, recurring2); - ImmutableList recurrings = loadAllRecurrings(); - assertThat(recurrings.size()).isEqualTo(2); - - recurrings.forEach( - newRecurring -> { - if (newRecurring.getTargetId().equals("foo.tld")) { - assertSameRecurringEntityExceptId(newRecurring, recurring1); - - BillingEvent.OneTime persistedOneTime = auditedOfy().load().entity(oneTime).now(); - assertAboutImmutableObjects() - .that(persistedOneTime) - .isEqualExceptFields(oneTime, "cancellationMatchingBillingEvent"); - assertThat(persistedOneTime.getCancellationMatchingBillingEvent()) - .isEqualTo(newRecurring.createVKey()); - - DomainBase persistedDomain = auditedOfy().load().entity(domain1).now(); - assertAboutImmutableObjects() - .that(persistedDomain) - .isEqualExceptFields( - domain1, - "updateTimestamp", - "revisions", - "gracePeriods", - "transferData", - "autorenewBillingEvent"); - assertThat(persistedDomain.getAutorenewBillingEvent()) - .isEqualTo(newRecurring.createVKey()); - assertThat(persistedDomain.getGracePeriods()) - .containsExactly( - GracePeriod.createForRecurring( - GracePeriodStatus.AUTO_RENEW, - domain1.getRepoId(), - now.plusDays(45), - "a registrar", - newRecurring.createVKey(), - gracePeriod.getGracePeriodId())); - assertThat(persistedDomain.getTransferData().getServerApproveAutorenewEvent()) - .isEqualTo(newRecurring.createVKey()); - assertThat(persistedDomain.getTransferData().getServerApproveEntities()) - .containsExactly(newRecurring.createVKey()); - - } else if (newRecurring.getTargetId().equals("bar.tld")) { - assertSameRecurringEntityExceptId(newRecurring, recurring2); - } else { - fail("Unknown BillingEvent.Recurring entity: " + newRecurring.createVKey()); - } - }); - } - - private static void assertNotInDatastore(ImmutableObject... entities) { - for (ImmutableObject entity : entities) { - assertThat(auditedOfy().load().entity(entity).now()).isNull(); - } - } - - private static void assertNotChangeExceptUpdateTime(ImmutableObject... entities) { - for (ImmutableObject entity : entities) { - assertAboutImmutableObjects() - .that(loadByEntity(entity)) - .isEqualExceptFields(entity, "updateTimestamp", "revisions"); - } - } - - private static void assertSameRecurringEntityExceptId( - BillingEvent.Recurring recurring1, BillingEvent.Recurring recurring2) { - assertAboutImmutableObjects().that(recurring1).isEqualExceptFields(recurring2, "id"); - } - - private static ImmutableList loadAllRecurrings() { - return ImmutableList.copyOf(auditedOfy().load().type(BillingEvent.Recurring.class)); - } - - private static String getKeyPathLiteral(Object... entities) { - return Arrays.stream(entities) - .map( - entity -> { - Key key = Key.create(entity); - return String.format( - "\"DomainBase\", \"%s\", \"HistoryEntry\", %s, \"%s\", %s", - key.getParent().getParent().getName(), - key.getParent().getId(), - key.getKind(), - key.getId()); - }) - .reduce((k1, k2) -> k1 + "\n" + k2) - .get(); - } -} diff --git a/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java b/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java index 4674f9fed..38ae384c7 100644 --- a/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java @@ -299,6 +299,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase, HistoryEntry.Type> + HISTORY_ENTRY_CREATE_TYPES = + ImmutableMap.of( + DomainBase.class, + HistoryEntry.Type.DOMAIN_CREATE, + ContactResource.class, + HistoryEntry.Type.CONTACT_CREATE, + HostResource.class, + HistoryEntry.Type.HOST_CREATE); + @Test void testKill() throws Exception { createTld("tld1"); createTld("tld2"); - for (EppResource resource : asList( - persistActiveDomain("foo.tld1"), - persistActiveDomain("foo.tld2"), - persistActiveContact("foo"), - persistActiveContact("foo"), - persistActiveHost("ns.foo.tld1"), - persistActiveHost("ns.foo.tld2"))) { - HistoryEntry history = new HistoryEntry.Builder().setParent(resource).build(); - for (ImmutableObject descendant : asList( - history, - new PollMessage.OneTime.Builder() - .setParent(history) - .setClientId("") - .setEventTime(START_OF_TIME) - .build(), - new PollMessage.Autorenew.Builder() - .setParent(history) - .setClientId("") - .setEventTime(START_OF_TIME) - .build(), - new BillingEvent.OneTime.Builder() - .setParent(history) - .setBillingTime(START_OF_TIME) - .setEventTime(START_OF_TIME) - .setClientId("") - .setTargetId("") - .setReason(Reason.CREATE) - .setPeriodYears(1) - .setCost(Money.of(CurrencyUnit.USD, 1)) - .build(), - new BillingEvent.Recurring.Builder() - .setParent(history) - .setEventTime(START_OF_TIME) - .setClientId("") - .setTargetId("") - .setReason(Reason.RENEW) - .build())) { + for (EppResource resource : + asList( + persistActiveDomain("foo.tld1"), + persistActiveDomain("foo.tld2"), + persistActiveContact("foo"), + persistActiveContact("foo"), + persistActiveHost("ns.foo.tld1"), + persistActiveHost("ns.foo.tld2"))) { + HistoryEntry history = + new HistoryEntry.Builder() + .setParent(resource) + .setClientId(resource.getCreationClientId()) + .setModificationTime(resource.getCreationTime()) + .setType(HISTORY_ENTRY_CREATE_TYPES.get(resource.getClass())) + .build(); + for (ImmutableObject descendant : + asList( + history, + new PollMessage.OneTime.Builder() + .setParent(history) + .setClientId("") + .setEventTime(START_OF_TIME) + .build(), + new PollMessage.Autorenew.Builder() + .setParent(history) + .setClientId("") + .setEventTime(START_OF_TIME) + .build(), + new BillingEvent.OneTime.Builder() + .setParent(history) + .setBillingTime(START_OF_TIME) + .setEventTime(START_OF_TIME) + .setClientId("") + .setTargetId("") + .setReason(Reason.CREATE) + .setPeriodYears(1) + .setCost(Money.of(CurrencyUnit.USD, 1)) + .build(), + new BillingEvent.Recurring.Builder() + .setParent(history) + .setEventTime(START_OF_TIME) + .setClientId("") + .setTargetId("") + .setReason(Reason.RENEW) + .build())) { persistResource(descendant); } } diff --git a/core/src/test/java/google/registry/tools/server/ResaveAllHistoryEntriesActionTest.java b/core/src/test/java/google/registry/tools/server/ResaveAllHistoryEntriesActionTest.java index 32dd9d9ca..034d0c23e 100644 --- a/core/src/test/java/google/registry/tools/server/ResaveAllHistoryEntriesActionTest.java +++ b/core/src/test/java/google/registry/tools/server/ResaveAllHistoryEntriesActionTest.java @@ -55,9 +55,25 @@ class ResaveAllHistoryEntriesActionTest extends MapreduceTestCase