diff --git a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java index 8ab7d43b7..95e267726 100644 --- a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java +++ b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java @@ -57,8 +57,6 @@ public final class AsyncTaskEnqueuer { public static final String QUEUE_ASYNC_DELETE = "async-delete-pull"; public static final String QUEUE_ASYNC_HOST_RENAME = "async-host-rename-pull"; - public static final String PATH_RESAVE_ENTITY = "/_dr/task/resaveEntity"; - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final Duration MAX_ASYNC_ETA = Duration.standardDays(30); @@ -112,7 +110,7 @@ public final class AsyncTaskEnqueuer { logger.atInfo().log("Enqueuing async re-save of %s to run at %s.", entityKey, whenToResave); String backendHostname = appEngineServiceUtils.getServiceHostname("backend"); TaskOptions task = - TaskOptions.Builder.withUrl(PATH_RESAVE_ENTITY) + TaskOptions.Builder.withUrl(ResaveEntityAction.PATH) .method(Method.POST) .header("Host", backendHostname) .countdownMillis(etaDuration.getMillis()) diff --git a/core/src/main/java/google/registry/batch/RelockDomainAction.java b/core/src/main/java/google/registry/batch/RelockDomainAction.java index 44ef261bd..4947614e2 100644 --- a/core/src/main/java/google/registry/batch/RelockDomainAction.java +++ b/core/src/main/java/google/registry/batch/RelockDomainAction.java @@ -16,7 +16,6 @@ package google.registry.batch; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableSet.toImmutableSet; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.request.Action.Method.POST; @@ -33,6 +32,7 @@ import google.registry.model.eppcommon.StatusValue; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; import google.registry.model.registry.RegistryLockDao; +import google.registry.persistence.VKey; import google.registry.request.Action; import google.registry.request.Parameter; import google.registry.request.Response; @@ -125,6 +125,7 @@ public class RelockDomainAction implements Runnable { response.setContentType(MediaType.PLAIN_TEXT_UTF_8); // nb: DomainLockUtils relies on the JPA transaction being the outermost transaction + // if we have Datastore as the primary DB (if SQL is the primary DB, it's irrelevant) jpaTm().transact(() -> tm().transact(this::relockDomain)); } @@ -139,12 +140,8 @@ public class RelockDomainAction implements Runnable { new IllegalArgumentException( String.format("Unknown revision ID %d", oldUnlockRevisionId))); domain = - ofy() - .load() - .type(DomainBase.class) - .id(oldLock.getRepoId()) - .now() - .cloneProjectedAtTime(jpaTm().getTransactionTime()); + tm().loadByKey(VKey.create(DomainBase.class, oldLock.getRepoId())) + .cloneProjectedAtTime(tm().getTransactionTime()); } catch (Throwable t) { handleTransientFailure(Optional.ofNullable(oldLock), t); return; diff --git a/core/src/main/java/google/registry/batch/ResaveEntityAction.java b/core/src/main/java/google/registry/batch/ResaveEntityAction.java index d50f196aa..84b892a49 100644 --- a/core/src/main/java/google/registry/batch/ResaveEntityAction.java +++ b/core/src/main/java/google/registry/batch/ResaveEntityAction.java @@ -17,7 +17,6 @@ package google.registry.batch; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_REQUESTED_TIME; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESAVE_TIMES; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; @@ -26,6 +25,7 @@ import com.google.common.flogger.FluentLogger; import com.googlecode.objectify.Key; import google.registry.model.EppResource; import google.registry.model.ImmutableObject; +import google.registry.persistence.VKey; import google.registry.request.Action; import google.registry.request.Action.Method; import google.registry.request.Parameter; @@ -74,16 +74,17 @@ public class ResaveEntityAction implements Runnable { public void run() { logger.atInfo().log( "Re-saving entity %s which was enqueued at %s.", resourceKey, requestedTime); - tm().transact(() -> { - ImmutableObject entity = ofy().load().key(resourceKey).now(); - ofy().save().entity( - (entity instanceof EppResource) - ? ((EppResource) entity).cloneProjectedAtTime(tm().getTransactionTime()) : entity - ); - if (!resaveTimes.isEmpty()) { - asyncTaskEnqueuer.enqueueAsyncResave(entity, requestedTime, resaveTimes); - } - }); + tm().transact( + () -> { + ImmutableObject entity = tm().loadByKey(VKey.from(resourceKey)); + tm().put( + (entity instanceof EppResource) + ? ((EppResource) entity).cloneProjectedAtTime(tm().getTransactionTime()) + : entity); + if (!resaveTimes.isEmpty()) { + asyncTaskEnqueuer.enqueueAsyncResave(entity, requestedTime, resaveTimes); + } + }); response.setPayload("Entity re-saved."); } } diff --git a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java index df563ec8c..82bea3b11 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -475,6 +475,9 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { private int internalDelete(VKey key) { checkArgumentNotNull(key, "key must be specified"); assertInTransaction(); + if (IGNORED_ENTITY_CLASSES.contains(key.getKind())) { + return 0; + } EntityType entityType = getEntityType(key.getKind()); ImmutableSet entityIds = getEntityIdsFromSqlKey(entityType, key.getSqlKey()); // TODO(b/179158393): use Criteria for query to leave not doubt about sql injection risk. diff --git a/core/src/main/java/google/registry/tools/DomainLockUtils.java b/core/src/main/java/google/registry/tools/DomainLockUtils.java index 7dcb7aad4..81d289f84 100644 --- a/core/src/main/java/google/registry/tools/DomainLockUtils.java +++ b/core/src/main/java/google/registry/tools/DomainLockUtils.java @@ -16,7 +16,6 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; import static google.registry.model.EppResourceUtils.loadByForeignKeyCached; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; @@ -29,6 +28,7 @@ import google.registry.config.RegistryConfig.Config; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Reason; import google.registry.model.domain.DomainBase; +import google.registry.model.domain.DomainHistory; import google.registry.model.registry.Registry; import google.registry.model.registry.RegistryLockDao; import google.registry.model.reporting.HistoryEntry; @@ -363,8 +363,8 @@ public final class DomainLockUtils { String reason = String.format( "%s of a domain through a RegistryLock operation", isLock ? "Lock" : "Unlock"); - HistoryEntry historyEntry = - new HistoryEntry.Builder() + DomainHistory domainHistory = + new DomainHistory.Builder() .setClientId(domain.getCurrentSponsorClientId()) .setBySuperuser(lock.isSuperuser()) .setRequestedByRegistrar(!lock.isSuperuser()) @@ -373,7 +373,8 @@ public final class DomainLockUtils { .setParent(Key.create(domain)) .setReason(reason) .build(); - ofy().save().entities(domain, historyEntry); + tm().update(domain); + tm().insert(domainHistory); if (!lock.isSuperuser()) { // admin actions shouldn't affect billing BillingEvent.OneTime oneTime = new BillingEvent.OneTime.Builder() @@ -383,9 +384,9 @@ public final class DomainLockUtils { .setCost(Registry.get(domain.getTld()).getRegistryLockOrUnlockBillingCost()) .setEventTime(now) .setBillingTime(now) - .setParent(historyEntry) + .setParent(domainHistory) .build(); - ofy().save().entity(oneTime); + tm().insert(oneTime); } } } diff --git a/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java b/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java index 3c42119a4..7eef6bbd1 100644 --- a/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java +++ b/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java @@ -19,7 +19,6 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_REQUESTED_TIME; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESAVE_TIMES; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY; -import static google.registry.batch.AsyncTaskEnqueuer.PATH_RESAVE_ENTITY; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_DELETE; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_HOST_RENAME; @@ -100,7 +99,7 @@ public class AsyncTaskEnqueuerTest { assertTasksEnqueued( QUEUE_ASYNC_ACTIONS, new TaskMatcher() - .url(PATH_RESAVE_ENTITY) + .url(ResaveEntityAction.PATH) .method("POST") .header("Host", "backend.hostname.fake") .header("content-type", "application/x-www-form-urlencoded") @@ -122,7 +121,7 @@ public class AsyncTaskEnqueuerTest { assertTasksEnqueued( QUEUE_ASYNC_ACTIONS, new TaskMatcher() - .url(PATH_RESAVE_ENTITY) + .url(ResaveEntityAction.PATH) .method("POST") .header("Host", "backend.hostname.fake") .header("content-type", "application/x-www-form-urlencoded") diff --git a/core/src/test/java/google/registry/batch/RelockDomainActionTest.java b/core/src/test/java/google/registry/batch/RelockDomainActionTest.java index f2760094c..a3dca5714 100644 --- a/core/src/test/java/google/registry/batch/RelockDomainActionTest.java +++ b/core/src/test/java/google/registry/batch/RelockDomainActionTest.java @@ -18,9 +18,9 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS; import static google.registry.model.eppcommon.StatusValue.PENDING_DELETE; import static google.registry.model.eppcommon.StatusValue.PENDING_TRANSFER; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.testing.DatabaseHelper.createTlds; -import static google.registry.testing.DatabaseHelper.deleteResource; +import static google.registry.testing.DatabaseHelper.deleteTestDomain; +import static google.registry.testing.DatabaseHelper.loadByEntity; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.persistActiveHost; import static google.registry.testing.DatabaseHelper.persistDomainAsDeleted; @@ -45,9 +45,11 @@ import google.registry.model.host.HostResource; import google.registry.schema.domain.RegistryLock; import google.registry.testing.AppEngineExtension; import google.registry.testing.DeterministicStringGenerator; +import google.registry.testing.DualDatabaseTest; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.TaskQueueHelper.TaskMatcher; +import google.registry.testing.TestOfyAndSql; import google.registry.testing.UserInfo; import google.registry.tools.DomainLockUtils; import google.registry.util.AppEngineServiceUtils; @@ -60,7 +62,6 @@ import org.joda.time.DateTime; import org.joda.time.Duration; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.RegisterExtension; import org.mockito.Mock; @@ -68,6 +69,7 @@ import org.mockito.junit.jupiter.MockitoExtension; /** Unit tests for {@link RelockDomainAction}. */ @ExtendWith(MockitoExtension.class) +@DualDatabaseTest public class RelockDomainActionTest { private static final String DOMAIN_NAME = "example.tld"; @@ -104,12 +106,12 @@ public class RelockDomainActionTest { domain = persistResource(newDomainBase(DOMAIN_NAME, host)); oldLock = domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, CLIENT_ID, POC_ID, false); - assertThat(reloadDomain(domain).getStatusValues()) + assertThat(loadByEntity(domain).getStatusValues()) .containsAtLeastElementsIn(REGISTRY_LOCK_STATUSES); oldLock = domainLockUtils.administrativelyApplyUnlock( DOMAIN_NAME, CLIENT_ID, false, Optional.empty()); - assertThat(reloadDomain(domain).getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES); + assertThat(loadByEntity(domain).getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES); AppEngineServiceUtils appEngineServiceUtils = mock(AppEngineServiceUtils.class); lenient() @@ -126,10 +128,10 @@ public class RelockDomainActionTest { verifyNoMoreInteractions(sendEmailService); } - @Test + @TestOfyAndSql void testLock() { action.run(); - assertThat(reloadDomain(domain).getStatusValues()) + assertThat(loadByEntity(domain).getStatusValues()) .containsAtLeastElementsIn(REGISTRY_LOCK_STATUSES); // the old lock should have a reference to the relock @@ -138,7 +140,7 @@ public class RelockDomainActionTest { .isEqualTo(newLock); } - @Test + @TestOfyAndSql void testFailure_unknownCode() throws Exception { action = createAction(12128675309L); action.run(); @@ -147,7 +149,7 @@ public class RelockDomainActionTest { assertTaskEnqueued(1, 12128675309L, Duration.standardMinutes(10)); // should retry, transient } - @Test + @TestOfyAndSql void testFailure_pendingDelete() throws Exception { persistResource(domain.asBuilder().setStatusValues(ImmutableSet.of(PENDING_DELETE)).build()); action.run(); @@ -159,7 +161,7 @@ public class RelockDomainActionTest { assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); } - @Test + @TestOfyAndSql void testFailure_pendingTransfer() throws Exception { persistResource(domain.asBuilder().setStatusValues(ImmutableSet.of(PENDING_TRANSFER)).build()); action.run(); @@ -171,7 +173,7 @@ public class RelockDomainActionTest { assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); } - @Test + @TestOfyAndSql void testFailure_domainAlreadyLocked() { domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, CLIENT_ID, null, true); action.run(); @@ -181,7 +183,7 @@ public class RelockDomainActionTest { assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); } - @Test + @TestOfyAndSql void testFailure_domainDeleted() throws Exception { persistDomainAsDeleted(domain, clock.nowUtc()); action.run(); @@ -193,7 +195,7 @@ public class RelockDomainActionTest { assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); } - @Test + @TestOfyAndSql void testFailure_domainTransferred() throws Exception { persistResource(domain.asBuilder().setPersistedCurrentSponsorClientId("NewRegistrar").build()); action.run(); @@ -207,29 +209,31 @@ public class RelockDomainActionTest { assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); } - @Test + @TestOfyAndSql public void testFailure_transientFailure_enqueuesTask() { // Hard-delete the domain to simulate a DB failure - deleteResource(domain); + deleteTestDomain(domain, clock.nowUtc()); action.run(); assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); - assertThat(response.getPayload()).isEqualTo("Re-lock failed: null"); + // Cannot assert the entire payload status since the Java object ID will vary + assertThat(response.getPayload()).startsWith("Re-lock failed: VKey"); assertTaskEnqueued(1); } - @Test + @TestOfyAndSql void testFailure_sufficientTransientFailures_sendsEmail() throws Exception { // Hard-delete the domain to simulate a DB failure - deleteResource(domain); + deleteTestDomain(domain, clock.nowUtc()); action = createAction(oldLock.getRevisionId(), RelockDomainAction.FAILURES_BEFORE_EMAIL); action.run(); assertTaskEnqueued(RelockDomainAction.FAILURES_BEFORE_EMAIL + 1); assertTransientFailureEmail(); assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); - assertThat(response.getPayload()).isEqualTo("Re-lock failed: null"); + // Cannot assert the entire payload status since the Java object ID will vary + assertThat(response.getPayload()).startsWith("Re-lock failed: VKey"); } - @Test + @TestOfyAndSql void testSuccess_afterSufficientFailures_sendsEmail() throws Exception { action = createAction(oldLock.getRevisionId(), RelockDomainAction.FAILURES_BEFORE_EMAIL + 1); action.run(); @@ -237,7 +241,7 @@ public class RelockDomainActionTest { assertSuccessEmailSent(); } - @Test + @TestOfyAndSql void testFailure_relockAlreadySet() { RegistryLock newLock = domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, CLIENT_ID, null, true); @@ -251,9 +255,9 @@ public class RelockDomainActionTest { assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); } - @Test + @TestOfyAndSql void testFailure_slowsDown() throws Exception { - deleteResource(domain); + deleteTestDomain(domain, clock.nowUtc()); action = createAction(oldLock.getRevisionId(), RelockDomainAction.ATTEMPTS_BEFORE_SLOWDOWN); action.run(); assertTaskEnqueued( @@ -328,10 +332,6 @@ public class RelockDomainActionTest { .etaDelta(duration.minus(standardSeconds(30)), duration.plus(standardSeconds(30)))); } - private DomainBase reloadDomain(DomainBase domain) { - return ofy().load().entity(domain).now(); - } - private RelockDomainAction createAction(Long oldUnlockRevisionId) throws Exception { return createAction(oldUnlockRevisionId, 0); } diff --git a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java index 26bad4fae..cbc3a0ad1 100644 --- a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java +++ b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java @@ -17,10 +17,9 @@ package google.registry.batch; import static com.google.common.truth.Truth.assertThat; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_REQUESTED_TIME; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY; -import static google.registry.batch.AsyncTaskEnqueuer.PATH_RESAVE_ENTITY; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.loadByEntity; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.persistActiveContact; import static google.registry.testing.DatabaseHelper.persistDomainWithDependentResources; @@ -43,14 +42,15 @@ import google.registry.model.eppcommon.StatusValue; import google.registry.model.ofy.Ofy; import google.registry.request.Response; import google.registry.testing.AppEngineExtension; +import google.registry.testing.DualDatabaseTest; import google.registry.testing.FakeClock; import google.registry.testing.InjectExtension; import google.registry.testing.TaskQueueHelper.TaskMatcher; +import google.registry.testing.TestOfyAndSql; import google.registry.util.AppEngineServiceUtils; import org.joda.time.DateTime; import org.joda.time.Duration; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.RegisterExtension; import org.mockito.Mock; @@ -60,6 +60,7 @@ import org.mockito.quality.Strictness; /** Unit tests for {@link ResaveEntityAction}. */ @ExtendWith(MockitoExtension.class) +@DualDatabaseTest public class ResaveEntityActionTest { @RegisterExtension @@ -93,7 +94,7 @@ public class ResaveEntityActionTest { } @MockitoSettings(strictness = Strictness.LENIENT) - @Test + @TestOfyAndSql void test_domainPendingTransfer_isResavedAndTransferCompleted() { DomainBase domain = persistDomainWithPendingTransfer( @@ -110,12 +111,12 @@ public class ResaveEntityActionTest { clock.advanceOneMilli(); assertThat(domain.getCurrentSponsorClientId()).isEqualTo("TheRegistrar"); runAction(Key.create(domain), DateTime.parse("2016-02-06T10:00:01Z"), ImmutableSortedSet.of()); - DomainBase resavedDomain = ofy().load().entity(domain).now(); + DomainBase resavedDomain = loadByEntity(domain); assertThat(resavedDomain.getCurrentSponsorClientId()).isEqualTo("NewRegistrar"); verify(response).setPayload("Entity re-saved."); } - @Test + @TestOfyAndSql void test_domainPendingDeletion_isResavedAndReenqueued() { DomainBase newDomain = newDomainBase("domain.tld"); DomainBase domain = @@ -137,13 +138,13 @@ public class ResaveEntityActionTest { assertThat(domain.getGracePeriods()).isNotEmpty(); runAction(Key.create(domain), requestedTime, ImmutableSortedSet.of(requestedTime.plusDays(5))); - DomainBase resavedDomain = ofy().load().entity(domain).now(); + DomainBase resavedDomain = loadByEntity(domain); assertThat(resavedDomain.getGracePeriods()).isEmpty(); assertTasksEnqueued( QUEUE_ASYNC_ACTIONS, new TaskMatcher() - .url(PATH_RESAVE_ENTITY) + .url(ResaveEntityAction.PATH) .method("POST") .header("Host", "backend.hostname.fake") .header("content-type", "application/x-www-form-urlencoded") 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 f60db23c3..8d8ff6d0b 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java @@ -19,7 +19,6 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_REQUESTED_TIME; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESAVE_TIMES; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY; -import static google.registry.batch.AsyncTaskEnqueuer.PATH_RESAVE_ENTITY; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS; import static google.registry.flows.domain.DomainTransferFlowTestCase.persistWithPendingTransfer; import static google.registry.model.EppResourceUtils.loadByForeignKey; @@ -64,6 +63,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.googlecode.objectify.Key; +import google.registry.batch.ResaveEntityAction; import google.registry.flows.EppException; import google.registry.flows.EppException.UnimplementedExtensionException; import google.registry.flows.EppRequestSource; @@ -299,7 +299,7 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase doFailingTest("domain_transfer_cancel.xml")); diff --git a/core/src/test/java/google/registry/flows/domain/DomainTransferFlowTestCase.java b/core/src/test/java/google/registry/flows/domain/DomainTransferFlowTestCase.java index 75043c9f8..f4432e2d8 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainTransferFlowTestCase.java +++ b/core/src/test/java/google/registry/flows/domain/DomainTransferFlowTestCase.java @@ -17,20 +17,15 @@ package google.registry.flows.domain; import static com.google.common.base.Preconditions.checkState; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createBillingEventForTransfer; import static google.registry.testing.DatabaseHelper.createTld; -import static google.registry.testing.DatabaseHelper.deleteResource; -import static google.registry.testing.DatabaseHelper.getBillingEvents; import static google.registry.testing.DatabaseHelper.getOnlyHistoryEntryOfType; -import static google.registry.testing.DatabaseHelper.loadAllOf; import static google.registry.testing.DatabaseHelper.persistActiveContact; import static google.registry.testing.DatabaseHelper.persistDomainWithDependentResources; import static google.registry.testing.DatabaseHelper.persistDomainWithPendingTransfer; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.DomainBaseSubject.assertAboutDomains; import static google.registry.util.DateTimeUtils.END_OF_TIME; -import static google.registry.util.DateTimeUtils.START_OF_TIME; import com.google.common.base.Ascii; import com.google.common.collect.ImmutableSet; @@ -44,10 +39,8 @@ import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; import google.registry.model.eppcommon.StatusValue; import google.registry.model.host.HostResource; -import google.registry.model.poll.PollMessage; import google.registry.model.registry.Registry; import google.registry.model.reporting.HistoryEntry; -import google.registry.model.reporting.HistoryEntryDao; import google.registry.model.transfer.TransferData; import google.registry.model.transfer.TransferStatus; import google.registry.testing.AppEngineExtension; @@ -166,30 +159,6 @@ abstract class DomainTransferFlowTestCase .build(); } - /** - * Delete "domain" and all history records, billing events, poll messages and subordinate hosts. - */ - void deleteTestDomain(DomainBase domain) { - Iterable billingEvents = getBillingEvents(); - Iterable historyEntries = - HistoryEntryDao.loadAllHistoryObjects(START_OF_TIME, END_OF_TIME); - Iterable pollMessages = loadAllOf(PollMessage.class); - tm().transact( - () -> { - deleteResource(domain); - for (BillingEvent event : billingEvents) { - deleteResource(event); - } - for (PollMessage pollMessage : pollMessages) { - deleteResource(pollMessage); - } - deleteResource(subordinateHost); - for (HistoryEntry hist : historyEntries) { - deleteResource(hist); - } - }); - } - void assertTransferFailed( DomainBase domain, TransferStatus status, TransferData oldTransferData) { assertAboutDomains().that(domain) diff --git a/core/src/test/java/google/registry/flows/domain/DomainTransferQueryFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainTransferQueryFlowTest.java index 924479fd2..3cfa26ee5 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainTransferQueryFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainTransferQueryFlowTest.java @@ -16,6 +16,7 @@ package google.registry.flows.domain; import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.DatabaseHelper.assertBillingEvents; +import static google.registry.testing.DatabaseHelper.deleteTestDomain; import static google.registry.testing.DatabaseHelper.getPollMessages; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.DomainBaseSubject.assertAboutDomains; @@ -231,7 +232,7 @@ class DomainTransferQueryFlowTest @Test void testFailure_nonexistentDomain() throws Exception { - deleteTestDomain(domain); + deleteTestDomain(domain, clock.nowUtc()); ResourceDoesNotExistException thrown = assertThrows( ResourceDoesNotExistException.class, () -> doFailingTest("domain_transfer_query.xml")); 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 a5f4eb42d..d23f08a81 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java @@ -20,7 +20,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_REQUESTED_TIME; import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY; -import static google.registry.batch.AsyncTaskEnqueuer.PATH_RESAVE_ENTITY; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS; import static google.registry.model.registry.Registry.TldState.QUIET_PERIOD; import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_SUCCESSFUL; @@ -60,6 +59,7 @@ 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; import google.registry.flows.FlowUtils.UnknownCurrencyEppException; @@ -512,7 +512,7 @@ class DomainTransferRequestFlowTest assertTasksEnqueued( QUEUE_ASYNC_ACTIONS, new TaskMatcher() - .url(PATH_RESAVE_ENTITY) + .url(ResaveEntityAction.PATH) .method("POST") .header("Host", "backend.hostname.fake") .header("content-type", "application/x-www-form-urlencoded") diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index f7dc18b99..9e69c3976 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -68,6 +68,7 @@ import google.registry.dns.writer.VoidDnsWriter; import google.registry.model.Buildable; import google.registry.model.EppResource; import google.registry.model.EppResource.ForeignKeyedEppResource; +import google.registry.model.EppResourceUtils; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Flag; import google.registry.model.billing.BillingEvent.Reason; @@ -108,6 +109,7 @@ import google.registry.model.registry.label.PremiumListDualDao; import google.registry.model.registry.label.ReservedList; import google.registry.model.registry.label.ReservedListDualDatabaseDao; import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntryDao; import google.registry.model.transfer.ContactTransferData; import google.registry.model.transfer.DomainTransferData; import google.registry.model.transfer.TransferData; @@ -462,6 +464,36 @@ public class DatabaseHelper { disallowRegistrarAccess("NewRegistrar", tld); } + /** + * Deletes "domain" and all history records, billing events, poll messages and subordinate hosts. + */ + public static void deleteTestDomain(DomainBase domain, DateTime now) { + Iterable billingEvents = getBillingEvents(); + Iterable historyEntries = + HistoryEntryDao.loadHistoryObjectsForResource(domain.createVKey()); + Iterable pollMessages = loadAllOf(PollMessage.class); + tm().transact( + () -> { + deleteResource(domain); + for (BillingEvent event : billingEvents) { + deleteResource(event); + } + for (PollMessage pollMessage : pollMessages) { + deleteResource(pollMessage); + } + domain + .getSubordinateHosts() + .forEach( + hostname -> + deleteResource( + EppResourceUtils.loadByForeignKey(HostResource.class, hostname, now) + .get())); + for (HistoryEntry hist : historyEntries) { + deleteResource(hist); + } + }); + } + public static void allowRegistrarAccess(String clientId, String tld) { Registrar registrar = loadRegistrar(clientId); persistResource( diff --git a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java index 7529aeb7e..4dfa4c2f4 100644 --- a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java +++ b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java @@ -16,11 +16,11 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.testing.DatabaseHelper.assertNoBillingEvents; import static google.registry.testing.DatabaseHelper.createTlds; import static google.registry.testing.DatabaseHelper.getHistoryEntriesOfType; import static google.registry.testing.DatabaseHelper.getOnlyHistoryEntryOfType; +import static google.registry.testing.DatabaseHelper.loadByEntity; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.persistActiveHost; import static google.registry.testing.DatabaseHelper.persistResource; @@ -48,9 +48,11 @@ import google.registry.schema.domain.RegistryLock; import google.registry.testing.AppEngineExtension; import google.registry.testing.DatabaseHelper; import google.registry.testing.DeterministicStringGenerator; +import google.registry.testing.DualDatabaseTest; import google.registry.testing.FakeClock; import google.registry.testing.SqlHelper; import google.registry.testing.TaskQueueHelper.TaskMatcher; +import google.registry.testing.TestOfyAndSql; import google.registry.testing.UserInfo; import google.registry.util.AppEngineServiceUtils; import google.registry.util.StringGenerator.Alphabets; @@ -60,10 +62,10 @@ import java.util.stream.Collectors; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; /** Unit tests for {@link google.registry.tools.DomainLockUtils}. */ +@DualDatabaseTest public final class DomainLockUtilsTest { private static final String DOMAIN_NAME = "example.tld"; @@ -99,7 +101,7 @@ public final class DomainLockUtilsTest { appEngineServiceUtils, clock, standardSeconds(90))); } - @Test + @TestOfyAndSql void testSuccess_createLock() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); @@ -107,7 +109,7 @@ public final class DomainLockUtilsTest { assertThat(lock.getLockCompletionTime().isPresent()).isFalse(); } - @Test + @TestOfyAndSql void testSuccess_createUnlock() { domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); RegistryLock lock = @@ -116,7 +118,7 @@ public final class DomainLockUtilsTest { assertThat(lock.getUnlockCompletionTime().isPresent()).isFalse(); } - @Test + @TestOfyAndSql void testSuccess_createUnlock_adminUnlockingAdmin() { domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", null, true); RegistryLock lock = @@ -125,7 +127,7 @@ public final class DomainLockUtilsTest { assertThat(lock.getUnlockCompletionTime().isPresent()).isFalse(); } - @Test + @TestOfyAndSql void testSuccess_createLock_previousLockExpired() { domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); clock.advanceBy(standardDays(1)); @@ -135,7 +137,7 @@ public final class DomainLockUtilsTest { verifyProperlyLockedDomain(false); } - @Test + @TestOfyAndSql void testSuccess_createUnlock_previousUnlockRequestExpired() { domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); domainLockUtils.saveNewRegistryUnlockRequest( @@ -145,10 +147,10 @@ public final class DomainLockUtilsTest { domainLockUtils.saveNewRegistryUnlockRequest( DOMAIN_NAME, "TheRegistrar", false, Optional.empty()); domainLockUtils.verifyAndApplyUnlock(unlockRequest.getVerificationCode(), false); - assertThat(reloadDomain().getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES); + assertThat(loadByEntity(domain).getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES); } - @Test + @TestOfyAndSql void testSuccess_applyLockDomain() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); @@ -156,7 +158,7 @@ public final class DomainLockUtilsTest { verifyProperlyLockedDomain(false); } - @Test + @TestOfyAndSql void testSuccess_applyUnlockDomain() { domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); RegistryLock unlock = @@ -166,7 +168,7 @@ public final class DomainLockUtilsTest { verifyProperlyUnlockedDomain(false); } - @Test + @TestOfyAndSql void testSuccess_applyAdminLock_onlyHistoryEntry() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); @@ -174,7 +176,7 @@ public final class DomainLockUtilsTest { verifyProperlyLockedDomain(true); } - @Test + @TestOfyAndSql void testSuccess_applyAdminUnlock_onlyHistoryEntry() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); @@ -186,20 +188,20 @@ public final class DomainLockUtilsTest { verifyProperlyUnlockedDomain(true); } - @Test + @TestOfyAndSql void testSuccess_administrativelyLock_nonAdmin() { domainLockUtils.administrativelyApplyLock( DOMAIN_NAME, "TheRegistrar", "Marla.Singer@crr.com", false); verifyProperlyLockedDomain(false); } - @Test + @TestOfyAndSql void testSuccess_administrativelyLock_admin() { domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", null, true); verifyProperlyLockedDomain(true); } - @Test + @TestOfyAndSql void testSuccess_administrativelyUnlock_nonAdmin() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); @@ -209,7 +211,7 @@ public final class DomainLockUtilsTest { verifyProperlyUnlockedDomain(false); } - @Test + @TestOfyAndSql void testSuccess_administrativelyUnlock_admin() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); @@ -219,7 +221,7 @@ public final class DomainLockUtilsTest { verifyProperlyUnlockedDomain(true); } - @Test + @TestOfyAndSql void testSuccess_regularLock_relockSet() { domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); RegistryLock oldLock = @@ -233,7 +235,7 @@ public final class DomainLockUtilsTest { .isEqualTo(newLock.getRevisionId()); } - @Test + @TestOfyAndSql void testSuccess_administrativelyLock_relockSet() { domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); RegistryLock oldLock = @@ -246,7 +248,7 @@ public final class DomainLockUtilsTest { .isEqualTo(newLock.getRevisionId()); } - @Test + @TestOfyAndSql void testSuccess_createUnlock_relockDuration() { domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); RegistryLock lock = @@ -255,7 +257,7 @@ public final class DomainLockUtilsTest { assertThat(lock.getRelockDuration()).isEqualTo(Optional.of(standardDays(1))); } - @Test + @TestOfyAndSql void testSuccess_unlock_relockSubmitted() { domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); RegistryLock lock = @@ -277,7 +279,7 @@ public final class DomainLockUtilsTest { standardDays(6).plus(standardSeconds(30)))); } - @Test + @TestOfyAndSql void testSuccess_adminCanLockLockedDomain_withNoSavedLock() { // in the case of inconsistencies / errors, admins should have the ability to override // whatever statuses exist on the domain @@ -288,7 +290,7 @@ public final class DomainLockUtilsTest { assertThat(resultLock.getLockCompletionTime()).isEqualTo(Optional.of(clock.nowUtc())); } - @Test + @TestOfyAndSql void testSuccess_adminCanLockUnlockedDomain_withSavedLock() { // in the case of inconsistencies / errors, admins should have the ability to override // what the RegistryLock table says @@ -309,7 +311,7 @@ public final class DomainLockUtilsTest { assertThat(resultLock.getLockCompletionTime()).isEqualTo(Optional.of(clock.nowUtc())); } - @Test + @TestOfyAndSql void testFailure_createUnlock_alreadyPendingUnlock() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); @@ -327,7 +329,7 @@ public final class DomainLockUtilsTest { .isEqualTo("A pending unlock action already exists for example.tld"); } - @Test + @TestOfyAndSql void testFailure_createUnlock_nonAdminUnlockingAdmin() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); @@ -342,7 +344,7 @@ public final class DomainLockUtilsTest { .isEqualTo("Non-admin user cannot unlock admin-locked domain example.tld"); } - @Test + @TestOfyAndSql void testFailure_createLock_unknownDomain() { assertThat( assertThrows( @@ -354,7 +356,7 @@ public final class DomainLockUtilsTest { .isEqualTo("Domain doesn't exist"); } - @Test + @TestOfyAndSql void testFailure_createLock_alreadyPendingLock() { domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); assertThat( @@ -367,7 +369,7 @@ public final class DomainLockUtilsTest { .isEqualTo("A pending or completed lock action already exists for example.tld"); } - @Test + @TestOfyAndSql void testFailure_createLock_alreadyLocked() { persistResource(domain.asBuilder().setStatusValues(REGISTRY_LOCK_STATUSES).build()); assertThat( @@ -380,7 +382,7 @@ public final class DomainLockUtilsTest { .isEqualTo("Domain example.tld is already locked"); } - @Test + @TestOfyAndSql void testFailure_createUnlock_alreadyUnlocked() { assertThat( assertThrows( @@ -392,12 +394,12 @@ public final class DomainLockUtilsTest { .isEqualTo("Domain example.tld is already unlocked"); } - @Test + @TestOfyAndSql void testFailure_applyLock_alreadyApplied() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false); - domain = reloadDomain(); + domain = loadByEntity(domain); assertThat( assertThrows( IllegalArgumentException.class, @@ -407,7 +409,7 @@ public final class DomainLockUtilsTest { assertNoDomainChanges(); } - @Test + @TestOfyAndSql void testFailure_applyLock_expired() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); @@ -421,7 +423,7 @@ public final class DomainLockUtilsTest { assertNoDomainChanges(); } - @Test + @TestOfyAndSql void testFailure_applyLock_nonAdmin_applyAdminLock() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); @@ -434,7 +436,7 @@ public final class DomainLockUtilsTest { assertNoDomainChanges(); } - @Test + @TestOfyAndSql void testFailure_applyUnlock_alreadyUnlocked() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); @@ -453,7 +455,7 @@ public final class DomainLockUtilsTest { assertNoDomainChanges(); } - @Test + @TestOfyAndSql void testFailure_applyLock_alreadyLocked() { RegistryLock lock = domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); @@ -475,7 +477,8 @@ public final class DomainLockUtilsTest { } private void verifyProperlyLockedDomain(boolean isAdmin) { - assertThat(reloadDomain().getStatusValues()).containsAtLeastElementsIn(REGISTRY_LOCK_STATUSES); + assertThat(loadByEntity(domain).getStatusValues()) + .containsAtLeastElementsIn(REGISTRY_LOCK_STATUSES); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_UPDATE); assertThat(historyEntry.getRequestedByRegistrar()).isEqualTo(!isAdmin); assertThat(historyEntry.getBySuperuser()).isEqualTo(isAdmin); @@ -489,7 +492,7 @@ public final class DomainLockUtilsTest { } private void verifyProperlyUnlockedDomain(boolean isAdmin) { - assertThat(reloadDomain().getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES); + assertThat(loadByEntity(domain).getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES); ImmutableList historyEntries = getHistoryEntriesOfType(domain, HistoryEntry.Type.DOMAIN_UPDATE); assertThat(historyEntries.size()).isEqualTo(2); @@ -507,12 +510,8 @@ public final class DomainLockUtilsTest { } } - private DomainBase reloadDomain() { - return ofy().load().entity(domain).now(); - } - private void assertNoDomainChanges() { - assertThat(reloadDomain()).isEqualTo(domain); + assertThat(loadByEntity(domain)).isEqualTo(domain); } private void assertBillingEvent(HistoryEntry historyEntry) {