diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml index c2072db19..a1199ee46 100644 --- a/config/checkstyle/checkstyle.xml +++ b/config/checkstyle/checkstyle.xml @@ -61,12 +61,6 @@ by Joshua Bloch in his book Effective Java --> - - - - - - diff --git a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java index 3e2a6b5ee..d4f8bbeaf 100644 --- a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java +++ b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java @@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.batch.BatchModule.PARAM_DRY_RUN; import static google.registry.config.RegistryEnvironment.PRODUCTION; -import static google.registry.model.ResourceTransferUtils.updateForeignKeyIndexDeletionTime; 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; @@ -267,8 +266,6 @@ public class DeleteProberDataAction implements Runnable { // messages, or auto-renews because those will all be hard-deleted the next time the job runs // anyway. tm().putAll(ImmutableList.of(deletedDomain, historyEntry)); - // updating foreign keys is a no-op in SQL - updateForeignKeyIndexDeletionTime(deletedDomain); dnsQueue.addDomainRefreshTask(deletedDomain.getDomainName()); } } diff --git a/core/src/main/java/google/registry/flows/contact/ContactCreateFlow.java b/core/src/main/java/google/registry/flows/contact/ContactCreateFlow.java index d17c43842..47a50a79b 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactCreateFlow.java +++ b/core/src/main/java/google/registry/flows/contact/ContactCreateFlow.java @@ -41,7 +41,6 @@ import google.registry.model.eppinput.ResourceCommand; import google.registry.model.eppoutput.CreateData.ContactCreateData; import google.registry.model.eppoutput.EppResponse; import google.registry.model.index.EppResourceIndex; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.reporting.HistoryEntry; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import javax.inject.Inject; @@ -100,7 +99,6 @@ public final class ContactCreateFlow implements TransactionalFlow { ImmutableSet.of( newContact, historyBuilder.build(), - ForeignKeyIndex.create(newContact, newContact.getDeletionTime()), EppResourceIndex.create(Key.create(newContact)))); return responseBuilder .setResData(ContactCreateData.create(newContact.getContactId(), now)) diff --git a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java index 30b5ed03f..d617b72f7 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java @@ -106,7 +106,6 @@ import google.registry.model.eppinput.ResourceCommand; import google.registry.model.eppoutput.CreateData.DomainCreateData; import google.registry.model.eppoutput.EppResponse; import google.registry.model.index.EppResourceIndex; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse; import google.registry.model.poll.PollMessage; import google.registry.model.poll.PollMessage.Autorenew; @@ -404,7 +403,6 @@ public final class DomainCreateFlow implements TransactionalFlow { entitiesToSave.add( domain, domainHistory, - ForeignKeyIndex.create(domain, domain.getDeletionTime()), EppResourceIndex.create(Key.create(domain))); if (allocationToken.isPresent() && TokenType.SINGLE_USE.equals(allocationToken.get().getTokenType())) { diff --git a/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java b/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java index 4f36e43b9..e13b7af19 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java @@ -29,7 +29,6 @@ import static google.registry.flows.domain.DomainFlowUtils.updateAutorenewRecurr import static google.registry.flows.domain.DomainFlowUtils.verifyNotInPredelegation; import static google.registry.model.ResourceTransferUtils.denyPendingTransfer; import static google.registry.model.ResourceTransferUtils.handlePendingTransferOnDelete; -import static google.registry.model.ResourceTransferUtils.updateForeignKeyIndexDeletionTime; import static google.registry.model.eppoutput.Result.Code.SUCCESS; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACTION_PENDING; import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.ADD_FIELDS; @@ -257,7 +256,6 @@ public final class DomainDeleteFlow implements TransactionalFlow { Domain newDomain = builder.build(); DomainHistory domainHistory = buildDomainHistory(newDomain, registry, now, durationUntilDelete, inAddGracePeriod); - updateForeignKeyIndexDeletionTime(newDomain); handlePendingTransferOnDelete(existingDomain, newDomain, now, domainHistory); // Close the autorenew billing event and poll message. This may delete the poll message. Store // the updated recurring billing event, we'll need it later and can't reload it. diff --git a/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java b/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java index 556e557d5..f6119253e 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java @@ -27,7 +27,6 @@ import static google.registry.flows.domain.DomainFlowUtils.validateFeeChallenge; import static google.registry.flows.domain.DomainFlowUtils.verifyNotReserved; import static google.registry.flows.domain.DomainFlowUtils.verifyPremiumNameIsNotBlocked; import static google.registry.flows.domain.DomainFlowUtils.verifyRegistrarIsActive; -import static google.registry.model.ResourceTransferUtils.updateForeignKeyIndexDeletionTime; import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_RESTORE; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.DateTimeUtils.END_OF_TIME; @@ -188,7 +187,6 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { autorenewPollMessage, now, registrarId); - updateForeignKeyIndexDeletionTime(newDomain); DomainHistory domainHistory = buildDomainHistory(newDomain, now); entitiesToSave.add(newDomain, domainHistory, autorenewEvent, autorenewPollMessage); tm().putAll(entitiesToSave.build()); diff --git a/core/src/main/java/google/registry/flows/host/HostCreateFlow.java b/core/src/main/java/google/registry/flows/host/HostCreateFlow.java index b1576e59b..8149e3b29 100644 --- a/core/src/main/java/google/registry/flows/host/HostCreateFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostCreateFlow.java @@ -50,7 +50,6 @@ import google.registry.model.host.Host; import google.registry.model.host.HostCommand.Create; import google.registry.model.host.HostHistory; import google.registry.model.index.EppResourceIndex; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import java.util.Optional; import javax.inject.Inject; @@ -135,7 +134,6 @@ public final class HostCreateFlow implements TransactionalFlow { ImmutableSet.of( newHost, historyBuilder.build(), - ForeignKeyIndex.create(newHost, newHost.getDeletionTime()), EppResourceIndex.create(Key.create(newHost))); if (superordinateDomain.isPresent()) { tm().update( diff --git a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java index dc0029ca1..12a5cda99 100644 --- a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java @@ -57,7 +57,6 @@ import google.registry.model.host.HostCommand.Update; import google.registry.model.host.HostCommand.Update.AddRemove; import google.registry.model.host.HostCommand.Update.Change; import google.registry.model.host.HostHistory; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.persistence.VKey; import java.util.Objects; @@ -194,11 +193,7 @@ public final class HostUpdateFlow implements TransactionalFlow { ImmutableSet.Builder entitiesToInsert = new ImmutableSet.Builder<>(); ImmutableSet.Builder entitiesToUpdate = new ImmutableSet.Builder<>(); entitiesToUpdate.add(newHost); - // Keep the {@link ForeignKeyIndex} for this host up to date. if (isHostRename) { - // Update the foreign key for the old host name and save one for the new host name. - entitiesToUpdate.add(ForeignKeyIndex.create(existingHost, now)); - entitiesToUpdate.add(ForeignKeyIndex.create(newHost, newHost.getDeletionTime())); updateSuperordinateDomains(existingHost, newHost); } enqueueTasks(existingHost, newHost); diff --git a/core/src/main/java/google/registry/model/EntityClasses.java b/core/src/main/java/google/registry/model/EntityClasses.java index fa8e3f184..b44fede22 100644 --- a/core/src/main/java/google/registry/model/EntityClasses.java +++ b/core/src/main/java/google/registry/model/EntityClasses.java @@ -25,7 +25,6 @@ import google.registry.model.host.Host; import google.registry.model.host.HostHistory; import google.registry.model.index.EppResourceIndex; import google.registry.model.index.EppResourceIndexBucket; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.reporting.HistoryEntry; import google.registry.model.server.ServerSecret; @@ -42,9 +41,6 @@ public final class EntityClasses { DomainHistory.class, EppResourceIndex.class, EppResourceIndexBucket.class, - ForeignKeyIndex.ForeignKeyContactIndex.class, - ForeignKeyIndex.ForeignKeyDomainIndex.class, - ForeignKeyIndex.ForeignKeyHostIndex.class, GaeUserIdConverter.class, HistoryEntry.class, Host.class, diff --git a/core/src/main/java/google/registry/model/ResourceTransferUtils.java b/core/src/main/java/google/registry/model/ResourceTransferUtils.java index f4bb48163..0b5f4e153 100644 --- a/core/src/main/java/google/registry/model/ResourceTransferUtils.java +++ b/core/src/main/java/google/registry/model/ResourceTransferUtils.java @@ -23,13 +23,11 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import google.registry.model.EppResource.BuilderWithTransferData; -import google.registry.model.EppResource.ForeignKeyedEppResource; import google.registry.model.EppResource.ResourceWithTransferData; import google.registry.model.contact.Contact; import google.registry.model.domain.Domain; import google.registry.model.eppcommon.StatusValue; import google.registry.model.eppcommon.Trid; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.poll.PendingActionNotificationResponse; import google.registry.model.poll.PendingActionNotificationResponse.ContactPendingActionNotificationResponse; import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse; @@ -104,13 +102,6 @@ public final class ResourceTransferUtils { checkState(eppResource instanceof Contact || eppResource instanceof Domain); } - /** Update the relevant {@link ForeignKeyIndex} to cache the new deletion time. */ - public static void updateForeignKeyIndexDeletionTime(R resource) { - if (resource instanceof ForeignKeyedEppResource) { - tm().insert(ForeignKeyIndex.create(resource, resource.getDeletionTime())); - } - } - /** If there is a transfer out, delete the server-approve entities and enqueue a poll message. */ public static void handlePendingTransferOnDelete( diff --git a/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java b/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java index 0e90d2e2a..9547467ae 100644 --- a/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java +++ b/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java @@ -19,10 +19,8 @@ import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.config.RegistryConfig.getEppResourceCachingDuration; import static google.registry.config.RegistryConfig.getEppResourceMaxCachedEntries; -import static google.registry.model.ofy.ObjectifyService.auditedOfy; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.replicaJpaTm; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.CollectionUtils.entriesToImmutableMap; import static google.registry.util.TypeUtils.instantiate; @@ -36,17 +34,10 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import com.google.common.collect.Multimaps; import com.google.common.collect.Streams; -import com.googlecode.objectify.Key; -import com.googlecode.objectify.annotation.Entity; -import com.googlecode.objectify.annotation.Id; -import com.googlecode.objectify.annotation.Index; import google.registry.config.RegistryConfig; import google.registry.model.BackupGroupRoot; import google.registry.model.CacheUtils; -import google.registry.model.CacheUtils.AppEngineEnvironmentCacheLoader; import google.registry.model.EppResource; -import google.registry.model.annotations.DeleteAfterMigration; -import google.registry.model.annotations.ReportedOn; import google.registry.model.contact.Contact; import google.registry.model.domain.Domain; import google.registry.model.host.Host; @@ -67,22 +58,15 @@ import org.joda.time.DateTime; * the foreign key string. The instance is never deleted, but it is updated if a newer entity * becomes the active entity. */ -@DeleteAfterMigration public abstract class ForeignKeyIndex extends BackupGroupRoot { /** The {@link ForeignKeyIndex} type for {@link Contact} entities. */ - @ReportedOn - @Entity public static class ForeignKeyContactIndex extends ForeignKeyIndex {} /** The {@link ForeignKeyIndex} type for {@link Domain} entities. */ - @ReportedOn - @Entity public static class ForeignKeyDomainIndex extends ForeignKeyIndex {} /** The {@link ForeignKeyIndex} type for {@link Host} entities. */ - @ReportedOn - @Entity public static class ForeignKeyHostIndex extends ForeignKeyIndex {} private static final ImmutableBiMap< @@ -100,23 +84,18 @@ public abstract class ForeignKeyIndex extends BackupGroup Domain.class, "fullyQualifiedDomainName", Host.class, "fullyQualifiedHostName"); - @Id String foreignKey; + String foreignKey; /** * The deletion time of this {@link ForeignKeyIndex}. * - *

This will generally be equal to the deletion time of {@link #topReference}. However, in the + *

This will generally be equal to the deletion time of {@link #reference}. However, in the * case of a {@link Host} that was renamed, this field will hold the time of the rename. */ - @Index DateTime deletionTime; + DateTime deletionTime; - /** - * The referenced resource. - * - *

This field holds a key to the only referenced resource. It is named "topReference" for - * historical reasons. - */ - VKey topReference; + /** The referenced resource. */ + VKey reference; public String getForeignKey() { return foreignKey; @@ -127,7 +106,7 @@ public abstract class ForeignKeyIndex extends BackupGroup } public VKey getResourceKey() { - return topReference; + return reference; } @SuppressWarnings("unchecked") @@ -137,26 +116,19 @@ public abstract class ForeignKeyIndex extends BackupGroup } /** Create a {@link ForeignKeyIndex} instance for a resource, expiring at a specified time. */ - public static ForeignKeyIndex create( + @SuppressWarnings("unchecked") + private static ForeignKeyIndex create( E resource, DateTime deletionTime) { - @SuppressWarnings("unchecked") Class resourceClass = (Class) resource.getClass(); ForeignKeyIndex instance = instantiate(mapToFkiClass(resourceClass)); - instance.topReference = (VKey) resource.createVKey(); + instance.reference = (VKey) resource.createVKey(); instance.foreignKey = resource.getForeignKey(); instance.deletionTime = deletionTime; return instance; } - /** Create a {@link ForeignKeyIndex} key for a resource. */ - public static Key> createKey(E resource) { - @SuppressWarnings("unchecked") - Class resourceClass = (Class) resource.getClass(); - return Key.create(mapToFkiClass(resourceClass), resource.getForeignKey()); - } - /** - * Loads a {@link Key} to an {@link EppResource} from Datastore by foreign key. + * Loads a {@link VKey} to an {@link EppResource} from the database by foreign key. * *

Returns null if no foreign key index with this foreign key was ever created, or if the most * recently created foreign key index was deleted before time "now". This method does not actually @@ -172,7 +144,7 @@ public abstract class ForeignKeyIndex extends BackupGroup public static VKey loadAndGetKey( Class clazz, String foreignKey, DateTime now) { ForeignKeyIndex index = load(clazz, foreignKey, now); - return (index == null) ? null : index.getResourceKey(); + return index == null ? null : index.getResourceKey(); } /** @@ -196,7 +168,7 @@ public abstract class ForeignKeyIndex extends BackupGroup */ public static ImmutableMap> load( Class clazz, Collection foreignKeys, final DateTime now) { - return loadIndexesFromStore(clazz, foreignKeys, true, false).entrySet().stream() + return loadIndexesFromStore(clazz, foreignKeys, false).entrySet().stream() .filter(e -> now.isBefore(e.getValue().getDeletionTime())) .collect(entriesToImmutableMap()); } @@ -206,55 +178,37 @@ public abstract class ForeignKeyIndex extends BackupGroup * keys, regardless of whether or not they have been soft-deleted. * *

Used by both the cached (w/o deletion check) and the non-cached (with deletion check) calls. - * - *

Note that in the cached case, we wish to run this outside of any transaction because we may - * be loading many entities, going over the Datastore limit on the number of enrolled entity - * groups per transaction (25). If we require consistency, however, we must use a transaction. - * - * @param inTransaction whether or not to use an Objectify transaction */ private static ImmutableMap> loadIndexesFromStore( - Class clazz, - Collection foreignKeys, - boolean inTransaction, - boolean useReplicaJpaTm) { - if (tm().isOfy()) { - Class> fkiClass = mapToFkiClass(clazz); - return ImmutableMap.copyOf( - inTransaction - ? auditedOfy().load().type(fkiClass).ids(foreignKeys) - : tm().doTransactionless(() -> auditedOfy().load().type(fkiClass).ids(foreignKeys))); - } else { - String property = RESOURCE_CLASS_TO_FKI_PROPERTY.get(clazz); - JpaTransactionManager jpaTmToUse = useReplicaJpaTm ? replicaJpaTm() : jpaTm(); - ImmutableList> indexes = - jpaTmToUse.transact( - () -> - jpaTmToUse - .criteriaQuery( - CriteriaQueryBuilder.create(clazz) - .whereFieldIsIn(property, foreignKeys) - .build()) - .getResultStream() - .map(e -> ForeignKeyIndex.create(e, e.getDeletionTime())) - .collect(toImmutableList())); - // We need to find and return the entities with the maximum deletionTime for each foreign key. - return Multimaps.index(indexes, ForeignKeyIndex::getForeignKey).asMap().entrySet().stream() - .map( - entry -> - Maps.immutableEntry( - entry.getKey(), - entry.getValue().stream() - .max(Comparator.comparing(ForeignKeyIndex::getDeletionTime)) - .get())) - .collect(entriesToImmutableMap()); - } + Class clazz, Collection foreignKeys, boolean useReplicaJpaTm) { + String property = RESOURCE_CLASS_TO_FKI_PROPERTY.get(clazz); + JpaTransactionManager jpaTmToUse = useReplicaJpaTm ? replicaJpaTm() : jpaTm(); + ImmutableList> indexes = + jpaTmToUse.transact( + () -> + jpaTmToUse + .criteriaQuery( + CriteriaQueryBuilder.create(clazz) + .whereFieldIsIn(property, foreignKeys) + .build()) + .getResultStream() + .map(e -> create(e, e.getDeletionTime())) + .collect(toImmutableList())); + // We need to find and return the entities with the maximum deletionTime for each foreign key. + return Multimaps.index(indexes, ForeignKeyIndex::getForeignKey).asMap().entrySet().stream() + .map( + entry -> + Maps.immutableEntry( + entry.getKey(), + entry.getValue().stream() + .max(Comparator.comparing(ForeignKeyIndex::getDeletionTime)) + .get())) + .collect(entriesToImmutableMap()); } static final CacheLoader>, Optional>> CACHE_LOADER = - new AppEngineEnvironmentCacheLoader< - VKey>, Optional>>() { + new CacheLoader>, Optional>>() { @Override public Optional> load(VKey> key) { @@ -263,7 +217,6 @@ public abstract class ForeignKeyIndex extends BackupGroup loadIndexesFromStore( RESOURCE_CLASS_TO_FKI_CLASS.inverse().get(key.getKind()), ImmutableSet.of(foreignKey), - false, true) .get(foreignKey)); } @@ -280,7 +233,7 @@ public abstract class ForeignKeyIndex extends BackupGroup Streams.stream(keys).map(v -> v.getSqlKey().toString()).collect(toImmutableSet()); ImmutableSet>> typedKeys = ImmutableSet.copyOf(keys); ImmutableMap> existingFkis = - loadIndexesFromStore(resourceClass, foreignKeys, false, true); + loadIndexesFromStore(resourceClass, foreignKeys, true); // ofy omits keys that don't have values in Datastore, so re-add them in // here with Optional.empty() values. return Maps.asMap( @@ -333,14 +286,14 @@ public abstract class ForeignKeyIndex extends BackupGroup public static ImmutableMap> loadCached( Class clazz, Collection foreignKeys, final DateTime now) { if (!RegistryConfig.isEppResourceCachingEnabled()) { - return tm().doTransactionless(() -> load(clazz, foreignKeys, now)); + return load(clazz, foreignKeys, now); } Class> fkiClass = mapToFkiClass(clazz); // Safe to cast VKey> to VKey> @SuppressWarnings("unchecked") ImmutableList>> fkiVKeys = foreignKeys.stream() - .map(fk -> (VKey>) VKey.create(fkiClass, fk)) + .map(fk -> (VKey>) VKey.createSql(fkiClass, fk)) .collect(toImmutableList()); // This cast is safe because when we loaded ForeignKeyIndexes above we used type clazz, which // is scoped to E. 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 7888a054b..28c33ed45 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -31,9 +31,6 @@ import com.google.common.collect.Streams; import com.google.common.flogger.FluentLogger; import google.registry.model.ImmutableObject; import google.registry.model.index.EppResourceIndex; -import google.registry.model.index.ForeignKeyIndex.ForeignKeyContactIndex; -import google.registry.model.index.ForeignKeyIndex.ForeignKeyDomainIndex; -import google.registry.model.index.ForeignKeyIndex.ForeignKeyHostIndex; import google.registry.persistence.JpaRetries; import google.registry.persistence.VKey; import google.registry.util.Clock; @@ -83,11 +80,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { // to exclude the Datastore specific entities when the underlying tm() is jpaTm(). // TODO(b/176108270): Remove this property after database migration. private static final ImmutableSet> IGNORED_ENTITY_CLASSES = - ImmutableSet.of( - EppResourceIndex.class, - ForeignKeyContactIndex.class, - ForeignKeyDomainIndex.class, - ForeignKeyHostIndex.class); + ImmutableSet.of(EppResourceIndex.class); // EntityManagerFactory is thread safe. private final EntityManagerFactory emf; diff --git a/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java b/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java index 1cc4e9c47..c38e47ccd 100644 --- a/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java +++ b/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java @@ -106,7 +106,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { } /** - * Setup a domain with a transfer that should have been server approved a day ago. + * Set up a domain with a transfer that should have been server approved a day ago. * *

The transfer is from "TheRegistrar" to "NewRegistrar". */ @@ -508,7 +508,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .asBuilder() .setLastTransferTime(lastTransferTime) .build()); - // Set the new domain to have a last transfer time that is different than the last transfer + // Set the new domain to have a last transfer time that is different from the last transfer // time on the host in question. persistResource( DatabaseHelper.newDomain("example.tld") @@ -546,7 +546,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .asBuilder() .setLastTransferTime(clock.nowUtc().minusDays(5)) .build()); - // Set the new domain to have a last transfer time that is different than the last transfer + // Set the new domain to have a last transfer time that is different from the last transfer // time on the host in question. persistResource( DatabaseHelper.newDomain("example.tld") @@ -814,7 +814,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { clock.nowUtc().minusDays(2), clock.nowUtc().minusDays(3)); } - /** Test when the new superdordinate domain has never been transferred before. */ + /** Test when the new superordinate domain has never been transferred before. */ @Test void testSuccess_externalToSubord_lastTransferTimeNotOverridden_whenNull() throws Exception { doExternalToInternalLastTransferTimeTest(clock.nowUtc().minusDays(2), null); diff --git a/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java b/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java index 9381fd72d..c22b4010f 100644 --- a/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java +++ b/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java @@ -23,9 +23,6 @@ import google.registry.model.domain.DomainHistory; import google.registry.model.host.Host; import google.registry.model.index.EppResourceIndex; import google.registry.model.index.EppResourceIndexBucket; -import google.registry.model.index.ForeignKeyIndex.ForeignKeyContactIndex; -import google.registry.model.index.ForeignKeyIndex.ForeignKeyDomainIndex; -import google.registry.model.index.ForeignKeyIndex.ForeignKeyHostIndex; import google.registry.model.reporting.HistoryEntry; import google.registry.model.server.ServerSecret; import google.registry.testing.TestObject; @@ -44,8 +41,6 @@ public class ClassPathManagerTest { * below are all classes supported in CLASS_REGISTRY. This test breaks if someone changes a * classname without preserving the original name. */ - assertThat(ClassPathManager.getClass("ForeignKeyContactIndex")) - .isEqualTo(ForeignKeyContactIndex.class); assertThat(ClassPathManager.getClass("Host")).isEqualTo(Host.class); assertThat(ClassPathManager.getClass("Contact")).isEqualTo(Contact.class); assertThat(ClassPathManager.getClass("GaeUserIdConverter")).isEqualTo(GaeUserIdConverter.class); @@ -53,12 +48,8 @@ public class ClassPathManagerTest { .isEqualTo(EppResourceIndexBucket.class); assertThat(ClassPathManager.getClass("Domain")).isEqualTo(Domain.class); assertThat(ClassPathManager.getClass("HistoryEntry")).isEqualTo(HistoryEntry.class); - assertThat(ClassPathManager.getClass("ForeignKeyHostIndex")) - .isEqualTo(ForeignKeyHostIndex.class); assertThat(ClassPathManager.getClass("ServerSecret")).isEqualTo(ServerSecret.class); assertThat(ClassPathManager.getClass("EppResourceIndex")).isEqualTo(EppResourceIndex.class); - assertThat(ClassPathManager.getClass("ForeignKeyDomainIndex")) - .isEqualTo(ForeignKeyDomainIndex.class); } @Test @@ -91,8 +82,6 @@ public class ClassPathManagerTest { * The classes below are all classes supported in CLASS_NAME_REGISTRY. This test breaks if * someone changes a classname without preserving the original name. */ - assertThat(ClassPathManager.getClassName(ForeignKeyContactIndex.class)) - .isEqualTo("ForeignKeyContactIndex"); assertThat(ClassPathManager.getClassName(Host.class)).isEqualTo("Host"); assertThat(ClassPathManager.getClassName(Contact.class)).isEqualTo("Contact"); assertThat(ClassPathManager.getClassName(GaeUserIdConverter.class)) @@ -101,12 +90,8 @@ public class ClassPathManagerTest { .isEqualTo("EppResourceIndexBucket"); assertThat(ClassPathManager.getClassName(Domain.class)).isEqualTo("Domain"); assertThat(ClassPathManager.getClassName(HistoryEntry.class)).isEqualTo("HistoryEntry"); - assertThat(ClassPathManager.getClassName(ForeignKeyHostIndex.class)) - .isEqualTo("ForeignKeyHostIndex"); assertThat(ClassPathManager.getClassName(ServerSecret.class)).isEqualTo("ServerSecret"); assertThat(ClassPathManager.getClassName(EppResourceIndex.class)).isEqualTo("EppResourceIndex"); - assertThat(ClassPathManager.getClassName(ForeignKeyDomainIndex.class)) - .isEqualTo("ForeignKeyDomainIndex"); } @Test diff --git a/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java b/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java index 6ba5020d5..6ce393696 100644 --- a/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java +++ b/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java @@ -15,17 +15,13 @@ package google.registry.model.index; import static com.google.common.truth.Truth.assertThat; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistActiveHost; import static google.registry.testing.DatabaseHelper.persistResource; import com.google.common.collect.ImmutableList; import google.registry.model.EntityTestCase; -import google.registry.model.domain.Domain; import google.registry.model.host.Host; -import google.registry.model.index.ForeignKeyIndex.ForeignKeyHostIndex; -import google.registry.testing.DatabaseHelper; import google.registry.testing.TestCacheExtension; import java.time.Duration; import org.junit.jupiter.api.BeforeEach; @@ -44,16 +40,6 @@ class ForeignKeyIndexTest extends EntityTestCase { createTld("com"); } - @Test - void testModifyForeignKeyIndex_notThrowExceptionInSql() { - Domain domain = DatabaseHelper.newDomain("test.com"); - ForeignKeyIndex fki = ForeignKeyIndex.create(domain, fakeClock.nowUtc()); - tm().transact(() -> tm().insert(fki)); - tm().transact(() -> tm().put(fki)); - tm().transact(() -> tm().delete(fki)); - tm().transact(() -> tm().update(fki)); - } - @Test void testLoadForNonexistentForeignKey_returnsNull() { assertThat(ForeignKeyIndex.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull(); @@ -62,11 +48,7 @@ class ForeignKeyIndexTest extends EntityTestCase { @Test void testLoadForDeletedForeignKey_returnsNull() { Host host = persistActiveHost("ns1.example.com"); - if (tm().isOfy()) { - persistResource(ForeignKeyIndex.create(host, fakeClock.nowUtc().minusDays(1))); - } else { - persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build()); - } + persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build()); assertThat(ForeignKeyIndex.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull(); } @@ -74,15 +56,7 @@ class ForeignKeyIndexTest extends EntityTestCase { void testLoad_newerKeyHasBeenSoftDeleted() { Host host1 = persistActiveHost("ns1.example.com"); fakeClock.advanceOneMilli(); - if (tm().isOfy()) { - ForeignKeyHostIndex fki = new ForeignKeyHostIndex(); - fki.foreignKey = "ns1.example.com"; - fki.topReference = host1.createVKey(); - fki.deletionTime = fakeClock.nowUtc(); - persistResource(fki); - } else { - persistResource(host1.asBuilder().setDeletionTime(fakeClock.nowUtc()).build()); - } + persistResource(host1.asBuilder().setDeletionTime(fakeClock.nowUtc()).build()); assertThat(ForeignKeyIndex.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull(); } @@ -90,11 +64,7 @@ class ForeignKeyIndexTest extends EntityTestCase { void testBatchLoad_skipsDeletedAndNonexistent() { persistActiveHost("ns1.example.com"); Host host = persistActiveHost("ns2.example.com"); - if (tm().isOfy()) { - persistResource(ForeignKeyIndex.create(host, fakeClock.nowUtc().minusDays(1))); - } else { - persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build()); - } + persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build()); assertThat( ForeignKeyIndex.load( Host.class, diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index 898db1af8..76bc9fd61 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -63,7 +63,6 @@ import com.googlecode.objectify.Key; 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.ImmutableObject; import google.registry.model.billing.BillingEvent; @@ -91,7 +90,6 @@ import google.registry.model.eppcommon.Trid; import google.registry.model.host.Host; import google.registry.model.index.EppResourceIndex; import google.registry.model.index.EppResourceIndexBucket; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.poll.PollMessage; import google.registry.model.pricing.StaticPremiumListPricingEngine; import google.registry.model.registrar.Registrar; @@ -1011,9 +1009,6 @@ public class DatabaseHelper { .that(resource.getRepoId()) .isNotEmpty(); saver.accept(index); - if (resource instanceof ForeignKeyedEppResource) { - saver.accept(ForeignKeyIndex.create(resource, resource.getDeletionTime())); - } } /** Persists an object in the DB for tests. */ diff --git a/core/src/test/resources/google/registry/model/schema.txt b/core/src/test/resources/google/registry/model/schema.txt index 296ab0256..dd071b62c 100644 --- a/core/src/test/resources/google/registry/model/schema.txt +++ b/core/src/test/resources/google/registry/model/schema.txt @@ -294,21 +294,6 @@ class google.registry.model.index.EppResourceIndex { class google.registry.model.index.EppResourceIndexBucket { @Id long bucketId; } -class google.registry.model.index.ForeignKeyIndex$ForeignKeyContactIndex { - @Id java.lang.String foreignKey; - google.registry.persistence.VKey topReference; - org.joda.time.DateTime deletionTime; -} -class google.registry.model.index.ForeignKeyIndex$ForeignKeyDomainIndex { - @Id java.lang.String foreignKey; - google.registry.persistence.VKey topReference; - org.joda.time.DateTime deletionTime; -} -class google.registry.model.index.ForeignKeyIndex$ForeignKeyHostIndex { - @Id java.lang.String foreignKey; - google.registry.persistence.VKey topReference; - org.joda.time.DateTime deletionTime; -} class google.registry.model.reporting.DomainTransactionRecord { google.registry.model.reporting.DomainTransactionRecord$TransactionReportField reportField; java.lang.Integer reportAmount; diff --git a/util/src/main/java/google/registry/util/TypeUtils.java b/util/src/main/java/google/registry/util/TypeUtils.java index 86e26a5a2..fafccbf51 100644 --- a/util/src/main/java/google/registry/util/TypeUtils.java +++ b/util/src/main/java/google/registry/util/TypeUtils.java @@ -28,7 +28,9 @@ import java.lang.reflect.Modifier; import java.util.function.Predicate; /** Utilities methods related to reflection. */ -public class TypeUtils { +public final class TypeUtils { + + private TypeUtils() {} /** A {@code TypeToken} that removes an ugly cast in the common cases of getting a known type. */ public static class TypeInstantiator extends TypeToken { @@ -47,7 +49,8 @@ public class TypeUtils { } public static T instantiate(Class clazz) { - checkArgument(Modifier.isPublic(clazz.getModifiers()), + checkArgument( + Modifier.isPublic(clazz.getModifiers()), "AppEngine's custom security manager won't let us reflectively access non-public types"); try { return clazz.getConstructor().newInstance(); @@ -59,14 +62,15 @@ public class TypeUtils { /** * Instantiate a class with the specified constructor argument. * - *

Because we use arg1's type to lookup the constructor, this only works if arg1's class is - * exactly the same type as the constructor argument. Subtypes are not allowed. + *

Because we use {@code arg}'s type to look up the constructor, this only works if arg1's + * class is exactly the same type as the constructor argument. Subtypes are not allowed. */ - public static T instantiate(Class clazz, U arg1) { - checkArgument(Modifier.isPublic(clazz.getModifiers()), + public static T instantiate(Class clazz, U arg) { + checkArgument( + Modifier.isPublic(clazz.getModifiers()), "AppEngine's custom security manager won't let us reflectively access non-public types"); try { - return clazz.getConstructor(arg1.getClass()).newInstance(arg1); + return clazz.getConstructor(arg.getClass()).newInstance(arg); } catch (ReflectiveOperationException e) { throw new RuntimeException(e); } @@ -106,8 +110,10 @@ public class TypeUtils { T enumField = (T) field.get(null); builder.put(field.getName(), enumField); } catch (IllegalArgumentException | IllegalAccessException e) { - throw new RuntimeException(String.format( - "Could not retrieve static final field mapping for %s", clazz.getName()), e); + throw new RuntimeException( + String.format( + "Could not retrieve static final field mapping for %s", clazz.getName()), + e); } } } @@ -115,8 +121,7 @@ public class TypeUtils { } /** Returns a predicate that tests whether classes are annotated with the given annotation. */ - public static Predicate> hasAnnotation( - final Class annotation) { + public static Predicate> hasAnnotation(final Class annotation) { return clazz -> clazz.isAnnotationPresent(annotation); }