Restore ofy keys in GracePeriod objects (#846)

* Restore ofy keys in GracePeriod objects

Restore the ofy keys when loading GracePeriod object from SQL.  There's no
clear way to do this using the normal approach (fix-up during a PostLoad
method) because fixups to these violate immutability after hibernate has
already obtained their hash values.  Instead, we force reconstitution of the
ofy keys in all public methods that access them (including equals() and
hashCode()) so that they can be generated before an invalid hash is generated.

As part of this change, convert the GracePeriod id from an autogenerated
sequence to a UUID allocated from ObjectifyService and enhance ImmutableObject
to allow it to exclude certain fields from hash/equals and print.

The ImmutableObject enhancements are necessary because we compare grace
periods against locally created test objects in a number of unit tests and
there's no way this can work with GracePeriods loaded from SQL currently, as
they will have an identifier field generated from the database and the test
objects will have an identifier field of null (or a new unique value, after
this change).

Removing autogeneration from GracePeriod ids ended up being likely not
strictly necessary for this change (it was a consequence of an earlier
iteration).  However, it does alleviate the problem of mutation of an
immutable object after creation and is more in line with how we've decided to
allocate other identifiers.

* Changed needed after rebase.
This commit is contained in:
Michael Muller 2020-10-26 13:38:14 -04:00 committed by GitHub
parent 576c05ff5f
commit 86bdd154bc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 1625 additions and 1406 deletions

View file

@ -805,30 +805,35 @@ class EppLifecycleDomainTest extends EppTestCase {
// As the losing registrar, read the request poll message, and then ack it.
assertThatLoginSucceeds("NewRegistrar", "foo-BAR2");
String messageId = "1-C-EXAMPLE-20-26-2001";
assertThatCommand("poll.xml")
.atTime("2001-01-01T00:01:00Z")
.hasResponse("poll_response_domain_transfer_request.xml");
assertThatCommand("poll_ack.xml", ImmutableMap.of("ID", "1-C-EXAMPLE-17-23-2001"))
.hasResponse("poll_response_domain_transfer_request.xml", ImmutableMap.of("ID", messageId));
assertThatCommand("poll_ack.xml", ImmutableMap.of("ID", messageId))
.atTime("2001-01-01T00:01:00Z")
.hasResponse("poll_ack_response_empty.xml");
// Five days in the future, expect a server approval poll message to the loser, and ack it.
messageId = "1-C-EXAMPLE-20-25-2001";
assertThatCommand("poll.xml")
.atTime("2001-01-06T00:01:00Z")
.hasResponse("poll_response_domain_transfer_server_approve_loser.xml");
assertThatCommand("poll_ack.xml", ImmutableMap.of("ID", "1-C-EXAMPLE-17-22-2001"))
.hasResponse(
"poll_response_domain_transfer_server_approve_loser.xml",
ImmutableMap.of("ID", messageId));
assertThatCommand("poll_ack.xml", ImmutableMap.of("ID", messageId))
.atTime("2001-01-06T00:01:00Z")
.hasResponse("poll_ack_response_empty.xml");
assertThatLogoutSucceeds();
// Also expect a server approval poll message to the winner, with the transfer request trid.
messageId = "1-C-EXAMPLE-20-24-2001";
assertThatLoginSucceeds("TheRegistrar", "password2");
assertThatCommand("poll.xml")
.atTime("2001-01-06T00:02:00Z")
.hasResponse(
"poll_response_domain_transfer_server_approve_winner.xml",
ImmutableMap.of("SERVER_TRID", transferRequestTrid));
assertThatCommand("poll_ack.xml", ImmutableMap.of("ID", "1-C-EXAMPLE-17-21-2001"))
ImmutableMap.of("SERVER_TRID", transferRequestTrid, "ID", messageId));
assertThatCommand("poll_ack.xml", ImmutableMap.of("ID", messageId))
.atTime("2001-01-06T00:02:00Z")
.hasResponse("poll_ack_response_empty.xml");
assertThatLogoutSucceeds();

View file

@ -78,6 +78,7 @@ public class DomainBaseSqlTest {
private HostResource host;
private ContactResource contact;
private ContactResource contact2;
private ImmutableSet<GracePeriod> gracePeriods;
@BeforeEach
void setUp() {
@ -506,6 +507,20 @@ public class DomainBaseSqlTest {
.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()));
jpaTm().insert(contact);
jpaTm().insert(contact2);
@ -517,6 +532,7 @@ public class DomainBaseSqlTest {
.setAutorenewPollMessage(autorenewPollMessage.createVKey())
.setDeletePollMessage(deletePollMessage.createVKey())
.setTransferData(transferData)
.setGracePeriods(gracePeriods)
.build();
historyEntry = historyEntry.asBuilder().setDomainContent(domain).build();
jpaTm().insert(historyEntry);
@ -553,6 +569,7 @@ public class DomainBaseSqlTest {
.isEqualTo(originalTransferData.getServerApproveAutorenewEvent());
assertThat(persistedTransferData.getServerApproveAutorenewPollMessage())
.isEqualTo(originalTransferData.getServerApproveAutorenewPollMessage());
assertThat(persisted.getGracePeriods()).isEqualTo(gracePeriods);
}
@Test
@ -624,6 +641,21 @@ public class DomainBaseSqlTest {
createLegacyVKey(
PollMessage.Autorenew.class, autorenewPollMessage.getId()))
.build();
gracePeriods =
ImmutableSet.of(
GracePeriod.create(
GracePeriodStatus.ADD,
"4-COM",
END_OF_TIME,
"registrar1",
createLegacyVKey(
BillingEvent.OneTime.class, oneTimeBillingEvent.getId())),
GracePeriod.createForRecurring(
GracePeriodStatus.AUTO_RENEW,
"4-COM",
END_OF_TIME,
"registrar1",
createLegacyVKey(BillingEvent.Recurring.class, billEvent.getId())));
jpaTm().insert(contact);
jpaTm().insert(contact2);
@ -639,6 +671,7 @@ public class DomainBaseSqlTest {
.setDeletePollMessage(
createLegacyVKey(PollMessage.OneTime.class, deletePollMessage.getId()))
.setTransferData(transferData)
.setGracePeriods(gracePeriods)
.build();
historyEntry = historyEntry.asBuilder().setDomainContent(domain).build();
jpaTm().insert(historyEntry);
@ -675,6 +708,7 @@ public class DomainBaseSqlTest {
.isEqualTo(originalTransferData.getServerApproveAutorenewEvent());
assertThat(persistedTransferData.getServerApproveAutorenewPollMessage())
.isEqualTo(originalTransferData.getServerApproveAutorenewPollMessage());
assertThat(domain.getGracePeriods()).isEqualTo(gracePeriods);
}
private <T> VKey<T> createLegacyVKey(Class<T> clazz, long id) {

View file

@ -38,6 +38,7 @@ import com.google.common.collect.Ordering;
import com.google.common.collect.Streams;
import com.googlecode.objectify.Key;
import google.registry.model.EntityTestCase;
import google.registry.model.ImmutableObject;
import google.registry.model.billing.BillingEvent;
import google.registry.model.billing.BillingEvent.Reason;
import google.registry.model.contact.ContactResource;
@ -830,4 +831,63 @@ public class DomainBaseTest extends EntityTestCase {
assertThat(getOnlyElement(clone.getGracePeriods()).getType())
.isEqualTo(GracePeriodStatus.TRANSFER);
}
@Test
void testHistoryIdRestoration() {
// Verify that history ids for billing events are restored during load from datastore. History
// ids are not used by business code or persisted in datastore, but only to reconstruct
// objectify keys when loading from SQL.
DateTime now = fakeClock.nowUtc();
domain =
persistResource(
domain
.asBuilder()
.setRegistrationExpirationTime(now.plusYears(1))
.setGracePeriods(
ImmutableSet.of(
GracePeriod.createForRecurring(
GracePeriodStatus.AUTO_RENEW,
domain.getRepoId(),
now.plusDays(1),
"NewRegistrar",
recurringBillKey),
GracePeriod.create(
GracePeriodStatus.RENEW,
domain.getRepoId(),
now.plusDays(1),
"NewRegistrar",
oneTimeBillKey)))
.build());
ImmutableSet<BillEventInfo> historyIds =
domain.getGracePeriods().stream()
.map(
gp ->
new BillEventInfo(
gp.getRecurringBillingEvent(), gp.billingEventRecurringHistoryId,
gp.getOneTimeBillingEvent(), gp.billingEventOneTimeHistoryId))
.collect(toImmutableSet());
assertThat(historyIds)
.isEqualTo(
ImmutableSet.of(
new BillEventInfo(null, null, oneTimeBillKey, historyEntryKey.getId()),
new BillEventInfo(recurringBillKey, historyEntryKey.getId(), null, null)));
}
static class BillEventInfo extends ImmutableObject {
VKey<BillingEvent.Recurring> billingEventRecurring;
Long billingEventRecurringHistoryId;
VKey<BillingEvent.OneTime> billingEventOneTime;
Long billingEventOneTimeHistoryId;
BillEventInfo(
VKey<BillingEvent.Recurring> billingEventRecurring,
Long billingEventRecurringHistoryId,
VKey<BillingEvent.OneTime> billingEventOneTime,
Long billingEventOneTimeHistoryId) {
this.billingEventRecurring = billingEventRecurring;
this.billingEventRecurringHistoryId = billingEventRecurringHistoryId;
this.billingEventOneTime = billingEventOneTime;
this.billingEventOneTimeHistoryId = billingEventOneTimeHistoryId;
}
}
}

View file

@ -44,6 +44,7 @@ public class GracePeriodTest {
private final DateTime now = DateTime.now(UTC);
private BillingEvent.OneTime onetime;
private VKey<BillingEvent.Recurring> recurringKey;
@BeforeEach
void before() {
@ -59,6 +60,14 @@ public class GracePeriodTest {
.setPeriodYears(1)
.setTargetId("foo.google")
.build();
recurringKey =
VKey.create(
Recurring.class,
12345,
Key.create(
Key.create(Key.create(DomainBase.class, "1-TEST"), HistoryEntry.class, 343L),
Recurring.class,
12345));
}
@Test
@ -71,6 +80,24 @@ public class GracePeriodTest {
assertThat(gracePeriod.getClientId()).isEqualTo("TheRegistrar");
assertThat(gracePeriod.getExpirationTime()).isEqualTo(now.plusDays(1));
assertThat(gracePeriod.hasBillingEvent()).isTrue();
assertThat(gracePeriod.billingEventOneTimeHistoryId).isEqualTo(12345L);
assertThat(gracePeriod.billingEventRecurringHistoryId).isNull();
}
@Test
void testSuccess_forRecurringEvent() {
GracePeriod gracePeriod =
GracePeriod.createForRecurring(
GracePeriodStatus.AUTO_RENEW, "1-TEST", now.plusDays(1), "TheRegistrar", recurringKey);
assertThat(gracePeriod.getType()).isEqualTo(GracePeriodStatus.AUTO_RENEW);
assertThat(gracePeriod.getDomainRepoId()).isEqualTo("1-TEST");
assertThat(gracePeriod.getOneTimeBillingEvent()).isNull();
assertThat(gracePeriod.getRecurringBillingEvent()).isEqualTo(recurringKey);
assertThat(gracePeriod.getClientId()).isEqualTo("TheRegistrar");
assertThat(gracePeriod.getExpirationTime()).isEqualTo(now.plusDays(1));
assertThat(gracePeriod.hasBillingEvent()).isTrue();
assertThat(gracePeriod.billingEventOneTimeHistoryId).isNull();
assertThat(gracePeriod.billingEventRecurringHistoryId).isEqualTo(343L);
}
@Test
@ -98,11 +125,6 @@ public class GracePeriodTest {
@Test
void testFailure_createForRecurring_notAutoRenew() {
Key<Recurring> recurringKey =
Key.create(
Key.create(Key.create(DomainBase.class, "1-TEST"), HistoryEntry.class, 343L),
Recurring.class,
12345);
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
@ -112,7 +134,7 @@ public class GracePeriodTest {
"1-TEST",
now.plusDays(1),
"TheRegistrar",
VKey.create(Recurring.class, 12345, recurringKey)));
recurringKey));
assertThat(thrown).hasMessageThat().contains("autorenew");
}
}

View file

@ -108,7 +108,7 @@ class EppLifecycleToolsTest extends EppTestCase {
.atTime("2001-06-08T00:00:00Z")
.hasResponse("poll_response_unrenew.xml");
assertThatCommand("poll_ack.xml", ImmutableMap.of("ID", "1-8-TLD-17-18-2001"))
assertThatCommand("poll_ack.xml", ImmutableMap.of("ID", "1-8-TLD-23-24-2001"))
.atTime("2001-06-08T00:00:01Z")
.hasResponse("poll_ack_response_empty.xml");
@ -129,7 +129,7 @@ class EppLifecycleToolsTest extends EppTestCase {
.hasResponse(
"poll_response_autorenew.xml",
ImmutableMap.of(
"ID", "1-8-TLD-17-20-2003",
"ID", "1-8-TLD-23-26-2003",
"QDATE", "2003-06-01T00:02:00Z",
"DOMAIN", "example.tld",
"EXDATE", "2004-06-01T00:02:00Z"));

View file

@ -3,7 +3,7 @@
<result code="1301">
<msg>Command completed successfully; ack to dequeue</msg>
</result>
<msgQ count="1" id="1-C-EXAMPLE-17-23-2001">
<msgQ count="1" id="%ID%">
<qDate>2001-01-01T00:00:00Z</qDate>
<msg>Transfer requested.</msg>
</msgQ>

View file

@ -3,7 +3,7 @@
<result code="1301">
<msg>Command completed successfully; ack to dequeue</msg>
</result>
<msgQ count="1" id="1-C-EXAMPLE-17-22-2001">
<msgQ count="1" id="%ID%">
<qDate>2001-01-06T00:00:00Z</qDate>
<msg>Transfer approved.</msg>
</msgQ>

View file

@ -4,7 +4,7 @@
<result code="1301">
<msg>Command completed successfully; ack to dequeue</msg>
</result>
<msgQ count="1" id="1-C-EXAMPLE-17-21-2001">
<msgQ count="1" id="%ID%">
<qDate>2001-01-06T00:00:00Z</qDate>
<msg>Transfer approved.</msg>
</msgQ>

View file

@ -3,7 +3,7 @@
<result code="1301">
<msg>Command completed successfully; ack to dequeue</msg>
</result>
<msgQ count="1" id="1-8-TLD-17-18-2001">
<msgQ count="1" id="1-8-TLD-23-24-2001">
<qDate>2001-06-07T00:00:00Z</qDate>
<msg>Domain example.tld was unrenewed by 3 years; now expires at 2003-06-01T00:02:00.000Z.</msg>
</msgQ>