zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-07-09 04:33:28 +02:00
Code Issues Projects Releases Packages Wiki Activity

Make sure post load work happens in GracePeriod (#878)

Browse source 
* Make sure post load work happens in GracePeriod

The GracePeriod method with ofy @OnLoad annotation is not called.

Apparently Ofy only checks for @OnLoad on first-class entities,
not embedded ones.

Added a call to this method from DomainContent's OnLoad method.

Reproduced issue with a test and verified that the fix works.
This commit is contained in:
Weimin Yu 2020-11-23 13:47:27 -05:00 • committed by GitHub
parent fcd79e7c18
commit 33499aaf9e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 19 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

3
core/src/main/java/google/registry/model/domain/DomainContent.java
View file

@ -311,6 +311,9 @@ public class DomainContent extends EppResource
nullToEmptyImmutableCopy(gracePeriods).stream()
.map(gracePeriod -> gracePeriod.cloneAfterOfyLoad(getRepoId()))
.collect(toImmutableSet());
// TODO(b/169873747): Remove this method after explicitly re-saving all domain entities.
// See also: GradePeriod.onLoad.
gracePeriods.forEach(GracePeriod::onLoad);
// Restore history record ids.
autorenewPollMessageHistoryId = getHistoryId(autorenewPollMessage);

6
core/src/main/java/google/registry/model/domain/GracePeriod.java
View file

@ -19,7 +19,6 @@ import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
import com.google.common.annotations.VisibleForTesting;
import com.googlecode.objectify.annotation.Embed;
import com.googlecode.objectify.annotation.OnLoad;
import google.registry.model.billing.BillingEvent;
import google.registry.model.billing.BillingEvent.Recurring;
import google.registry.model.domain.rgp.GracePeriodStatus;
@ -54,7 +53,10 @@ public class GracePeriod extends GracePeriodBase implements DatastoreAndSqlEntit
}
// TODO(b/169873747): Remove this method after explicitly re-saving all domain entities.
@OnLoad
// This method is invoked from DomainContent.load(): Objectify's @OnLoad annotation
// apparently does not work on embedded objects inside an entity.
// Changing signature to void onLoad(@AlsoLoad("gracePeriodId") Long gracePeriodId)
// would not work. Method is not called if gracePeriodId is null.
void onLoad() {
if (gracePeriodId == null) {
gracePeriodId = ObjectifyService.allocateId();

12
core/src/test/java/google/registry/model/domain/DomainBaseTest.java
View file

@ -19,6 +19,8 @@ import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static google.registry.model.EppResourceUtils.loadByForeignKey;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
import static google.registry.testing.DatabaseHelper.cloneAndSetAutoTimestamps;
import static google.registry.testing.DatabaseHelper.createTld;
import static google.registry.testing.DatabaseHelper.newDomainBase;
@ -30,6 +32,7 @@ import static org.joda.money.CurrencyUnit.USD;
import static org.joda.time.DateTimeZone.UTC;
import static org.junit.jupiter.api.Assertions.assertThrows;
import com.google.appengine.api.datastore.Entity;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
@ -164,6 +167,15 @@ public class DomainBaseTest extends EntityTestCase {
.build()));
}
@Test
void testGracePeriod_nullIdFromOfy() {
Entity entity = ofyTm().transact(() -> ofy().save().toEntity(domain));
entity.setUnindexedProperty("gracePeriods.gracePeriodId", null);
DomainBase domainFromEntity = ofyTm().transact(() -> ofy().load().fromEntity(entity));
GracePeriod gracePeriod = domainFromEntity.getGracePeriods().iterator().next();
assertThat(gracePeriod.gracePeriodId).isNotNull();
}
@Test
void testPersistence() {
// Note that this only verifies that the value stored under the foreign key is the same as that