diff --git a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java index de3bcd781..67b48b183 100644 --- a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java +++ b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java @@ -23,6 +23,7 @@ import static google.registry.model.ResourceTransferUtils.updateForeignKeyIndexD import static google.registry.model.ofy.ObjectifyService.auditedOfy; import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_DELETE; import static google.registry.model.tld.Registries.getTldsOfType; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.request.Action.Method.POST; import static google.registry.request.RequestParameters.PARAM_TLDS; @@ -42,6 +43,7 @@ import google.registry.config.RegistryEnvironment; import google.registry.dns.DnsQueue; import google.registry.mapreduce.MapreduceRunner; import google.registry.mapreduce.inputs.EppResourceInputs; +import google.registry.model.CreateAutoTimestamp; import google.registry.model.EppResourceUtils; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainHistory; @@ -54,15 +56,18 @@ import google.registry.request.Parameter; import google.registry.request.Response; import google.registry.request.auth.Auth; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import javax.inject.Inject; +import org.hibernate.CacheMode; +import org.hibernate.ScrollMode; +import org.hibernate.ScrollableResults; +import org.hibernate.query.Query; import org.joda.time.DateTime; import org.joda.time.Duration; /** - * Deletes all prober DomainBases and their subordinate history entries, poll messages, and - * billing events, along with their ForeignKeyDomainIndex and EppResourceIndex entities. - * - *

See: https://www.youtube.com/watch?v=xuuv0syoHnM + * Deletes all prober DomainBases and their subordinate history entries, poll messages, and billing + * events, along with their ForeignKeyDomainIndex and EppResourceIndex entities. */ @Action( service = Action.Service.BACKEND, @@ -73,10 +78,51 @@ public class DeleteProberDataAction implements Runnable { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + /** + * The maximum amount of time we allow a prober domain to be in use. + * + *

In practice, the prober's connection will time out well before this duration. This includes + * a decent buffer. + */ + private static final Duration DOMAIN_USED_DURATION = Duration.standardHours(1); + + /** + * The minimum amount of time we want a domain to be "soft deleted". + * + *

The domain has to remain soft deleted for at least enough time for the DNS task to run and + * remove it from DNS itself. This is probably on the order of minutes. + */ + private static final Duration SOFT_DELETE_DELAY = Duration.standardHours(1); + + private static final DnsQueue dnsQueue = DnsQueue.create(); + + // Domains to delete must: + // 1. Be in one of the prober TLDs + // 2. Not be a nic domain + // 3. Have no subordinate hosts + // 4. Not still be used (within an hour of creation time) + // 5. Either be active (creationTime <= now < deletionTime) or have been deleted a while ago (this + // prevents accidental double-map with the same key from immediately deleting active domains) + // + // Note: creationTime must be compared to a Java object (CreateAutoTimestamp) but deletionTime can + // be compared directly to the SQL timestamp (it's a DateTime) + private static final String DOMAIN_QUERY_STRING = + "FROM Domain d WHERE d.tld IN :tlds AND d.fullyQualifiedDomainName NOT LIKE 'nic.%' AND" + + " (d.subordinateHosts IS EMPTY OR d.subordinateHosts IS NULL) AND d.creationTime <" + + " :creationTimeCutoff AND ((d.creationTime <= :nowAutoTimestamp AND d.deletionTime >" + + " current_timestamp()) OR d.deletionTime < :nowMinusSoftDeleteDelay) ORDER BY d.repoId"; + + /** Number of domains to retrieve and delete per SQL transaction. */ + private static final int BATCH_SIZE = 1000; + @Inject @Parameter(PARAM_DRY_RUN) boolean isDryRun; /** List of TLDs to work on. If empty - will work on all TLDs that end with .test. */ @Inject @Parameter(PARAM_TLDS) ImmutableSet tlds; - @Inject @Config("registryAdminClientId") String registryAdminClientId; + + @Inject + @Config("registryAdminClientId") + String registryAdminRegistrarId; + @Inject MapreduceRunner mrRunner; @Inject Response response; @Inject DeleteProberDataAction() {} @@ -84,25 +130,14 @@ public class DeleteProberDataAction implements Runnable { @Override public void run() { checkState( - !Strings.isNullOrEmpty(registryAdminClientId), + !Strings.isNullOrEmpty(registryAdminRegistrarId), "Registry admin client ID must be configured for prober data deletion to work"); - mrRunner - .setJobName("Delete prober data") - .setModuleName("backend") - .runMapOnly( - new DeleteProberDataMapper(getProberRoidSuffixes(), isDryRun, registryAdminClientId), - ImmutableList.of(EppResourceInputs.createKeyInput(DomainBase.class))) - .sendLinkToMapreduceConsole(response); - } - - private ImmutableSet getProberRoidSuffixes() { checkArgument( !PRODUCTION.equals(RegistryEnvironment.get()) || tlds.stream().allMatch(tld -> tld.endsWith(".test")), "On production, can only work on TLDs that end with .test"); ImmutableSet deletableTlds = - getTldsOfType(TldType.TEST) - .stream() + getTldsOfType(TldType.TEST).stream() .filter(tld -> tlds.isEmpty() ? tld.endsWith(".test") : tlds.contains(tld)) .collect(toImmutableSet()); checkArgument( @@ -110,10 +145,161 @@ public class DeleteProberDataAction implements Runnable { "If tlds are given, they must all exist and be TEST tlds. Given: %s, not found: %s", tlds, Sets.difference(tlds, deletableTlds)); - return deletableTlds - .stream() - .map(tld -> Registry.get(tld).getRoidSuffix()) - .collect(toImmutableSet()); + ImmutableSet proberRoidSuffixes = + deletableTlds.stream() + .map(tld -> Registry.get(tld).getRoidSuffix()) + .collect(toImmutableSet()); + if (tm().isOfy()) { + mrRunner + .setJobName("Delete prober data") + .setModuleName("backend") + .runMapOnly( + new DeleteProberDataMapper(proberRoidSuffixes, isDryRun, registryAdminRegistrarId), + ImmutableList.of(EppResourceInputs.createKeyInput(DomainBase.class))) + .sendLinkToMapreduceConsole(response); + } else { + runSqlJob(deletableTlds); + } + } + + private void runSqlJob(ImmutableSet deletableTlds) { + AtomicInteger softDeletedDomains = new AtomicInteger(); + AtomicInteger hardDeletedDomains = new AtomicInteger(); + jpaTm().transact(() -> processDomains(deletableTlds, softDeletedDomains, hardDeletedDomains)); + logger.atInfo().log( + "%s %d domains.", + isDryRun ? "Would have soft-deleted" : "Soft-deleted", softDeletedDomains.get()); + logger.atInfo().log( + "%s %d domains.", + isDryRun ? "Would have hard-deleted" : "Hard-deleted", hardDeletedDomains.get()); + } + + private void processDomains( + ImmutableSet deletableTlds, + AtomicInteger softDeletedDomains, + AtomicInteger hardDeletedDomains) { + DateTime now = tm().getTransactionTime(); + // Scroll through domains, soft-deleting as necessary (very few will be soft-deleted) and + // keeping track of which domains to hard-delete (there can be many, so we batch them up) + ScrollableResults scrollableResult = + jpaTm() + .query(DOMAIN_QUERY_STRING, DomainBase.class) + .setParameter("tlds", deletableTlds) + .setParameter( + "creationTimeCutoff", CreateAutoTimestamp.create(now.minus(DOMAIN_USED_DURATION))) + .setParameter("nowMinusSoftDeleteDelay", now.minus(SOFT_DELETE_DELAY)) + .setParameter("nowAutoTimestamp", CreateAutoTimestamp.create(now)) + .unwrap(Query.class) + .setCacheMode(CacheMode.IGNORE) + .scroll(ScrollMode.FORWARD_ONLY); + ImmutableList.Builder domainRepoIdsToHardDelete = new ImmutableList.Builder<>(); + ImmutableList.Builder hostNamesToHardDelete = new ImmutableList.Builder<>(); + for (int i = 1; scrollableResult.next(); i = (i + 1) % BATCH_SIZE) { + DomainBase domain = (DomainBase) scrollableResult.get(0); + processDomain( + domain, + domainRepoIdsToHardDelete, + hostNamesToHardDelete, + softDeletedDomains, + hardDeletedDomains); + // Batch the deletion and DB flush + session clearing so we don't OOM + if (i == 0) { + hardDeleteDomainsAndHosts(domainRepoIdsToHardDelete.build(), hostNamesToHardDelete.build()); + domainRepoIdsToHardDelete = new ImmutableList.Builder<>(); + hostNamesToHardDelete = new ImmutableList.Builder<>(); + jpaTm().getEntityManager().flush(); + jpaTm().getEntityManager().clear(); + } + } + // process the remainder + hardDeleteDomainsAndHosts(domainRepoIdsToHardDelete.build(), hostNamesToHardDelete.build()); + } + + private void processDomain( + DomainBase domain, + ImmutableList.Builder domainRepoIdsToHardDelete, + ImmutableList.Builder hostNamesToHardDelete, + AtomicInteger softDeletedDomains, + AtomicInteger hardDeletedDomains) { + // If the domain is still active, that means that the prober encountered a failure and did not + // successfully soft-delete the domain (thus leaving its DNS entry published). We soft-delete + // it now so that the DNS entry can be handled. The domain will then be hard-deleted the next + // time the job is run. + if (EppResourceUtils.isActive(domain, tm().getTransactionTime())) { + if (isDryRun) { + logger.atInfo().log( + "Would soft-delete the active domain: %s (%s)", + domain.getDomainName(), domain.getRepoId()); + } else { + softDeleteDomain(domain, registryAdminRegistrarId, dnsQueue); + } + softDeletedDomains.incrementAndGet(); + } else { + if (isDryRun) { + logger.atInfo().log( + "Would hard-delete the non-active domain: %s (%s) and its dependents", + domain.getDomainName(), domain.getRepoId()); + } else { + domainRepoIdsToHardDelete.add(domain.getRepoId()); + hostNamesToHardDelete.addAll(domain.getSubordinateHosts()); + } + hardDeletedDomains.incrementAndGet(); + } + } + + private void hardDeleteDomainsAndHosts( + ImmutableList domainRepoIds, ImmutableList hostNames) { + jpaTm() + .query("DELETE FROM Host WHERE fullyQualifiedHostName IN :hostNames") + .setParameter("hostNames", hostNames) + .executeUpdate(); + jpaTm() + .query("DELETE FROM BillingEvent WHERE domainRepoId IN :repoIds") + .setParameter("repoIds", domainRepoIds) + .executeUpdate(); + jpaTm() + .query("DELETE FROM BillingRecurrence WHERE domainRepoId IN :repoIds") + .setParameter("repoIds", domainRepoIds) + .executeUpdate(); + jpaTm() + .query("DELETE FROM BillingCancellation WHERE domainRepoId IN :repoIds") + .setParameter("repoIds", domainRepoIds) + .executeUpdate(); + jpaTm() + .query("DELETE FROM DomainHistory WHERE domainRepoId IN :repoIds") + .setParameter("repoIds", domainRepoIds) + .executeUpdate(); + jpaTm() + .query("DELETE FROM PollMessage WHERE domainRepoId IN :repoIds") + .setParameter("repoIds", domainRepoIds) + .executeUpdate(); + jpaTm() + .query("DELETE FROM Domain WHERE repoId IN :repoIds") + .setParameter("repoIds", domainRepoIds) + .executeUpdate(); + } + + // Take a DNS queue + admin registrar id as input so that it can be called from the mapper as well + private static void softDeleteDomain( + DomainBase domain, String registryAdminRegistrarId, DnsQueue localDnsQueue) { + DomainBase deletedDomain = + domain.asBuilder().setDeletionTime(tm().getTransactionTime()).setStatusValues(null).build(); + DomainHistory historyEntry = + new DomainHistory.Builder() + .setDomain(domain) + .setType(DOMAIN_DELETE) + .setModificationTime(tm().getTransactionTime()) + .setBySuperuser(true) + .setReason("Deletion of prober data") + .setClientId(registryAdminRegistrarId) + .build(); + // Note that we don't bother handling grace periods, billing events, pending transfers, poll + // messages, or auto-renews because those will all be hard-deleted the next time the job runs + // anyway. + tm().putAllWithoutBackup(ImmutableList.of(deletedDomain, historyEntry)); + // updating foreign keys is a no-op in SQL + updateForeignKeyIndexDeletionTime(deletedDomain); + localDnsQueue.addDomainRefreshTask(deletedDomain.getDomainName()); } /** Provides the map method that runs for each existing DomainBase entity. */ @@ -122,32 +308,17 @@ public class DeleteProberDataAction implements Runnable { private static final DnsQueue dnsQueue = DnsQueue.create(); private static final long serialVersionUID = -7724537393697576369L; - /** - * The maximum amount of time we allow a prober domain to be in use. - * - * In practice, the prober's connection will time out well before this duration. This includes a - * decent buffer. - * - */ - private static final Duration DOMAIN_USED_DURATION = Duration.standardHours(1); - - /** - * The minimum amount of time we want a domain to be "soft deleted". - * - * The domain has to remain soft deleted for at least enough time for the DNS task to run and - * remove it from DNS itself. This is probably on the order of minutes. - */ - private static final Duration SOFT_DELETE_DELAY = Duration.standardHours(1); - private final ImmutableSet proberRoidSuffixes; private final Boolean isDryRun; - private final String registryAdminClientId; + private final String registryAdminRegistrarId; public DeleteProberDataMapper( - ImmutableSet proberRoidSuffixes, Boolean isDryRun, String registryAdminClientId) { + ImmutableSet proberRoidSuffixes, + Boolean isDryRun, + String registryAdminRegistrarId) { this.proberRoidSuffixes = proberRoidSuffixes; this.isDryRun = isDryRun; - this.registryAdminClientId = registryAdminClientId; + this.registryAdminRegistrarId = registryAdminRegistrarId; } @Override @@ -203,7 +374,7 @@ public class DeleteProberDataAction implements Runnable { logger.atInfo().log( "Would soft-delete the active domain: %s (%s)", domainName, domainKey); } else { - softDeleteDomain(domain); + tm().transact(() -> softDeleteDomain(domain, registryAdminRegistrarId, dnsQueue)); } getContext().incrementCounter("domains soft-deleted"); return; @@ -223,8 +394,7 @@ public class DeleteProberDataAction implements Runnable { tm().transact( () -> { // This ancestor query selects all descendant HistoryEntries, BillingEvents, - // PollMessages, - // and TLD-specific entities, as well as the domain itself. + // PollMessages, and TLD-specific entities, as well as the domain itself. List> domainAndDependentKeys = auditedOfy().load().ancestor(domainKey).keys().list(); ImmutableSet> allKeys = @@ -243,32 +413,5 @@ public class DeleteProberDataAction implements Runnable { getContext().incrementCounter("domains hard-deleted"); getContext().incrementCounter("total entities hard-deleted", entitiesDeleted); } - - private void softDeleteDomain(final DomainBase domain) { - tm().transactNew( - () -> { - DomainBase deletedDomain = - domain - .asBuilder() - .setDeletionTime(tm().getTransactionTime()) - .setStatusValues(null) - .build(); - DomainHistory historyEntry = - new DomainHistory.Builder() - .setDomain(domain) - .setType(DOMAIN_DELETE) - .setModificationTime(tm().getTransactionTime()) - .setBySuperuser(true) - .setReason("Deletion of prober data") - .setClientId(registryAdminClientId) - .build(); - // Note that we don't bother handling grace periods, billing events, pending - // transfers, poll messages, or auto-renews because these will all be hard-deleted - // the next time the mapreduce runs anyway. - tm().putAll(deletedDomain, historyEntry); - updateForeignKeyIndexDeletionTime(deletedDomain); - dnsQueue.addDomainRefreshTask(deletedDomain.getDomainName()); - }); - } } } 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 7ec304a3d..4b73f30f0 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -48,6 +48,8 @@ import google.registry.util.SystemSleeper; import java.io.Serializable; import java.lang.reflect.Array; import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.Calendar; import java.util.Collections; import java.util.Date; @@ -73,7 +75,6 @@ import javax.persistence.TemporalType; import javax.persistence.TypedQuery; import javax.persistence.criteria.CriteriaQuery; import javax.persistence.metamodel.EntityType; -import javax.persistence.metamodel.SingularAttribute; import org.joda.time.DateTime; /** Implementation of {@link JpaTransactionManager} for JPA compatible database. */ @@ -694,10 +695,22 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { private static ImmutableSet getEntityIdsFromIdContainer( EntityType entityType, Object idContainer) { return entityType.getIdClassAttributes().stream() - .map(SingularAttribute::getName) .map( - idName -> { - Object idValue = getFieldValue(idContainer, idName); + attribute -> { + String idName = attribute.getName(); + // The object may use either Java getters or field names to represent the ID object. + // Attempt the Java getter, then fall back to the field name if that fails. + String methodName = attribute.getJavaMember().getName(); + Object idValue; + try { + Method method = idContainer.getClass().getDeclaredMethod(methodName); + method.setAccessible(true); + idValue = method.invoke(idContainer); + } catch (NoSuchMethodException + | IllegalAccessException + | InvocationTargetException e) { + idValue = getFieldValue(idContainer, idName); + } return new EntityId(idName, idValue); }) .collect(toImmutableSet()); diff --git a/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java b/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java index a24c4fc39..5f48f9923 100644 --- a/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java +++ b/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java @@ -15,10 +15,13 @@ package google.registry.batch; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.EppResourceUtils.loadByForeignKey; -import static google.registry.model.ofy.ObjectifyService.auditedOfy; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.loadByEntitiesIfPresent; +import static google.registry.testing.DatabaseHelper.loadByEntity; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistActiveHost; @@ -46,18 +49,20 @@ import google.registry.model.poll.PollMessage; import google.registry.model.reporting.HistoryEntry; import google.registry.model.tld.Registry; import google.registry.model.tld.Registry.TldType; +import google.registry.testing.DualDatabaseTest; import google.registry.testing.FakeResponse; import google.registry.testing.SystemPropertyExtension; +import google.registry.testing.TestOfyAndSql; import google.registry.testing.mapreduce.MapreduceTestCase; import java.util.Optional; import java.util.Set; import org.joda.money.Money; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; /** Unit tests for {@link DeleteProberDataAction}. */ +@DualDatabaseTest class DeleteProberDataActionTest extends MapreduceTestCase { private static final DateTime DELETION_TIME = DateTime.parse("2010-01-01T00:00:00.000Z"); @@ -93,7 +98,7 @@ class DeleteProberDataActionTest extends MapreduceTestCase tldEntities = persistLotsOfDomains("tld"); Set exampleEntities = persistLotsOfDomains("example"); @@ -110,14 +115,14 @@ class DeleteProberDataActionTest extends MapreduceTestCase ibEntities = persistLotsOfDomains("ib-any.test"); Set oaEntities = persistLotsOfDomains("oa-canary.test"); runMapreduce(); - assertNotDeleted(tldEntities); - assertNotDeleted(exampleEntities); - assertNotDeleted(notTestEntities); - assertDeleted(ibEntities); - assertDeleted(oaEntities); + assertAllExist(tldEntities); + assertAllExist(exampleEntities); + assertAllExist(notTestEntities); + assertAllAbsent(ibEntities); + assertAllAbsent(oaEntities); } - @Test + @TestOfyAndSql void testSuccess_deletesAllAndOnlyGivenTlds() throws Exception { Set tldEntities = persistLotsOfDomains("tld"); Set exampleEntities = persistLotsOfDomains("example"); @@ -126,14 +131,14 @@ class DeleteProberDataActionTest extends MapreduceTestCase oaEntities = persistLotsOfDomains("oa-canary.test"); action.tlds = ImmutableSet.of("example", "ib-any.test"); runMapreduce(); - assertNotDeleted(tldEntities); - assertNotDeleted(notTestEntities); - assertNotDeleted(oaEntities); - assertDeleted(exampleEntities); - assertDeleted(ibEntities); + assertAllExist(tldEntities); + assertAllExist(notTestEntities); + assertAllExist(oaEntities); + assertAllAbsent(exampleEntities); + assertAllAbsent(ibEntities); } - @Test + @TestOfyAndSql void testFail_givenNonTestTld() { action.tlds = ImmutableSet.of("not-test.test"); IllegalArgumentException thrown = @@ -143,7 +148,7 @@ class DeleteProberDataActionTest extends MapreduceTestCase fkiNic = ForeignKeyIndex.load(DomainBase.class, "nic.ib-any.test", START_OF_TIME); Set ibEntities = persistLotsOfDomains("ib-any.test"); runMapreduce(); - assertDeleted(ibEntities); - assertNotDeleted(ImmutableSet.of(nic, fkiNic)); + assertAllAbsent(ibEntities); + assertAllExist(ImmutableSet.of(nic)); + if (tm().isOfy()) { + assertAllExist(ImmutableSet.of(fkiNic)); + } } - @Test + @TestOfyAndSql void testDryRun_doesntDeleteData() throws Exception { Set tldEntities = persistLotsOfDomains("tld"); Set oaEntities = persistLotsOfDomains("oa-canary.test"); action.isDryRun = true; runMapreduce(); - assertNotDeleted(tldEntities); - assertNotDeleted(oaEntities); + assertAllExist(tldEntities); + assertAllExist(oaEntities); } - @Test + @TestOfyAndSql void testSuccess_activeDomain_isSoftDeleted() throws Exception { - DomainBase domain = persistResource( - newDomainBase("blah.ib-any.test") - .asBuilder() - .setCreationTimeForTest(DateTime.now(UTC).minusYears(1)) - .build()); + DomainBase domain = + persistResource( + newDomainBase("blah.ib-any.test") + .asBuilder() + .setCreationTimeForTest(DateTime.now(UTC).minusYears(1)) + .build()); runMapreduce(); DateTime timeAfterDeletion = DateTime.now(UTC); - assertThat(loadByForeignKey(DomainBase.class, "blah.ib-any.test", timeAfterDeletion)) - .isEmpty(); - assertThat(auditedOfy().load().entity(domain).now().getDeletionTime()) - .isLessThan(timeAfterDeletion); + assertThat(loadByForeignKey(DomainBase.class, "blah.ib-any.test", timeAfterDeletion)).isEmpty(); + assertThat(loadByEntity(domain).getDeletionTime()).isLessThan(timeAfterDeletion); assertDnsTasksEnqueued("blah.ib-any.test"); } - @Test + @TestOfyAndSql void testSuccess_activeDomain_doubleMapSoftDeletes() throws Exception { DomainBase domain = persistResource( newDomainBase("blah.ib-any.test") @@ -214,12 +221,11 @@ class DeleteProberDataActionTest extends MapreduceTestCase fki = - ForeignKeyIndex.load(DomainBase.class, fqdn, START_OF_TIME); - EppResourceIndex eppIndex = - auditedOfy().load().entity(EppResourceIndex.create(Key.create(domain))).now(); - return ImmutableSet.of( - domain, historyEntry, billingEvent, pollMessage, fki, eppIndex); + PollMessage.OneTime pollMessage = + persistSimpleResource( + new PollMessage.OneTime.Builder() + .setParent(historyEntry) + .setEventTime(DELETION_TIME) + .setClientId("TheRegistrar") + .setMsg("Domain registered") + .build()); + ImmutableSet.Builder builder = + new ImmutableSet.Builder() + .add(domain) + .add(historyEntry) + .add(billingEvent) + .add(pollMessage); + if (tm().isOfy()) { + builder + .add(ForeignKeyIndex.load(DomainBase.class, fqdn, START_OF_TIME)) + .add(loadByEntity(EppResourceIndex.create(Key.create(domain)))); + } + return builder.build(); } private static Set persistLotsOfDomains(String tld) { @@ -322,15 +337,15 @@ class DeleteProberDataActionTest extends MapreduceTestCase entities) { - for (ImmutableObject entity : entities) { - assertThat(auditedOfy().load().entity(entity).now()).isNotNull(); - } + private static void assertAllExist(Iterable entities) { + assertWithMessage("Expected entities to exist in the DB but they were deleted") + .that(loadByEntitiesIfPresent(entities)) + .containsExactlyElementsIn(entities); } - private static void assertDeleted(Iterable entities) { - for (ImmutableObject entity : entities) { - assertThat(auditedOfy().load().entity(entity).now()).isNull(); - } + private static void assertAllAbsent(Iterable entities) { + assertWithMessage("Expected entities to not exist in the DB, but they did") + .that(loadByEntitiesIfPresent(entities)) + .isEmpty(); } } diff --git a/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java b/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java index 4c540e7a9..55288cf24 100644 --- a/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java +++ b/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java @@ -75,7 +75,8 @@ class JpaTransactionManagerImplTest { new JpaTestRules.Builder() .withInitScript(fileClassPath(getClass(), "test_schema.sql")) .withClock(fakeClock) - .withEntityClass(TestEntity.class, TestCompoundIdEntity.class) + .withEntityClass( + TestEntity.class, TestCompoundIdEntity.class, TestNamedCompoundIdEntity.class) .buildUnitTestRule(); @Test @@ -272,6 +273,24 @@ class JpaTransactionManagerImplTest { .isEqualTo(compoundIdEntity); } + @Test + void createNamedCompoundIdEntity_succeeds() { + // Compound IDs should also work even if the field names don't match up exactly + TestNamedCompoundIdEntity entity = new TestNamedCompoundIdEntity("foo", 1); + jpaTm().transact(() -> jpaTm().insert(entity)); + jpaTm() + .transact( + () -> { + assertThat(jpaTm().exists(entity)).isTrue(); + assertThat( + jpaTm() + .loadByKey( + VKey.createSql( + TestNamedCompoundIdEntity.class, new NamedCompoundId("foo", 1)))) + .isEqualTo(entity); + }); + } + @Test void saveAllNew_succeeds() { moreEntities.forEach( @@ -779,4 +798,71 @@ class JpaTransactionManagerImplTest { this.age = age; } } + + // An entity should still behave properly if the name fields in the ID are different + @Entity(name = "TestNamedCompoundIdEntity") + @IdClass(NamedCompoundId.class) + private static class TestNamedCompoundIdEntity extends ImmutableObject { + private String name; + private int age; + + private TestNamedCompoundIdEntity() {} + + private TestNamedCompoundIdEntity(String name, int age) { + this.name = name; + this.age = age; + } + + @Id + public String getNameField() { + return name; + } + + @Id + public int getAgeField() { + return age; + } + + @SuppressWarnings("unused") + private void setNameField(String name) { + this.name = name; + } + + @SuppressWarnings("unused") + private void setAgeField(int age) { + this.age = age; + } + } + + private static class NamedCompoundId implements Serializable { + String nameField; + int ageField; + + private NamedCompoundId() {} + + private NamedCompoundId(String nameField, int ageField) { + this.nameField = nameField; + this.ageField = ageField; + } + + @SuppressWarnings("unused") + private String getNameField() { + return nameField; + } + + @SuppressWarnings("unused") + private int getAgeField() { + return ageField; + } + + @SuppressWarnings("unused") + private void setNameField(String nameField) { + this.nameField = nameField; + } + + @SuppressWarnings("unused") + private void setAgeField(int ageField) { + this.ageField = ageField; + } + } } diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index ab11a3168..0821afc06 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -443,7 +443,7 @@ public class DatabaseHelper { * 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 billingEvents = getBillingEvents(domain); Iterable historyEntries = HistoryEntryDao.loadHistoryObjectsForResource(domain.createVKey()); Iterable pollMessages = loadAllOf(PollMessage.class); @@ -791,13 +791,13 @@ public class DatabaseHelper { return transactIfJpaTm( () -> Iterables.concat( - tm().loadAllOf(BillingEvent.OneTime.class).stream() + tm().loadAllOfStream(BillingEvent.OneTime.class) .filter(oneTime -> oneTime.getDomainRepoId().equals(resource.getRepoId())) .collect(toImmutableList()), - tm().loadAllOf(BillingEvent.Recurring.class).stream() + tm().loadAllOfStream(BillingEvent.Recurring.class) .filter(recurring -> recurring.getDomainRepoId().equals(resource.getRepoId())) .collect(toImmutableList()), - tm().loadAllOf(BillingEvent.Cancellation.class).stream() + tm().loadAllOfStream(BillingEvent.Cancellation.class) .filter( cancellation -> cancellation.getDomainRepoId().equals(resource.getRepoId())) .collect(toImmutableList()))); @@ -1351,7 +1351,25 @@ public class DatabaseHelper { } /** - * Asserts that the given entity is detached from the current JPA entity manager. + * Loads all given entities from the database if possible. + * + *

If the transaction manager is Cloud SQL, then this creates an inner wrapping transaction for + * convenience, so you don't need to wrap it in a transaction at the callsite. + * + *

Nonexistent entities are absent from the resulting list, but no {@link + * NoSuchElementException} will be thrown. + */ + public static ImmutableList loadByEntitiesIfPresent(Iterable entities) { + return transactIfJpaTm(() -> tm().loadByEntitiesIfPresent(entities)); + } + + /** Returns whether or not the given entity exists in the database. */ + public static boolean existsInDatabase(Object object) { + return transactIfJpaTm(() -> tm().exists(object)); + } + + /** + * In JPA mode, asserts that the given entity is detached from the current entity manager. * *

Returns the original entity object. */