diff --git a/core/src/main/java/google/registry/flows/CheckApiAction.java b/core/src/main/java/google/registry/flows/CheckApiAction.java index c5e69f930..e4d339a08 100644 --- a/core/src/main/java/google/registry/flows/CheckApiAction.java +++ b/core/src/main/java/google/registry/flows/CheckApiAction.java @@ -44,8 +44,8 @@ import dagger.Module; import dagger.Provides; import google.registry.flows.domain.DomainFlowUtils.BadCommandForRegistryPhaseException; import google.registry.flows.domain.DomainFlowUtils.InvalidIdnDomainLabelException; +import google.registry.model.ForeignKeyUtils; import google.registry.model.domain.Domain; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.tld.Registry; import google.registry.model.tld.label.ReservationType; import google.registry.monitoring.whitebox.CheckApiMetric; @@ -156,7 +156,7 @@ public class CheckApiAction implements Runnable { } private boolean checkExists(String domainString, DateTime now) { - return !ForeignKeyIndex.loadCached(Domain.class, ImmutableList.of(domainString), now).isEmpty(); + return !ForeignKeyUtils.loadCached(Domain.class, ImmutableList.of(domainString), now).isEmpty(); } private Optional checkReserved(InternetDomainName domainName) { diff --git a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java index 2248af63e..ff8e20959 100644 --- a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java +++ b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java @@ -17,7 +17,6 @@ package google.registry.flows; import static com.google.common.collect.Sets.intersection; import static google.registry.model.EppResourceUtils.isLinked; import static google.registry.model.EppResourceUtils.loadByForeignKey; -import static google.registry.model.index.ForeignKeyIndex.loadAndGetKey; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; @@ -38,6 +37,7 @@ import google.registry.flows.exceptions.TooManyResourceChecksException; import google.registry.model.EppResource; import google.registry.model.EppResource.ForeignKeyedEppResource; import google.registry.model.EppResource.ResourceWithTransferData; +import google.registry.model.ForeignKeyUtils; import google.registry.model.contact.Contact; import google.registry.model.domain.Domain; import google.registry.model.domain.DomainBase; @@ -45,7 +45,6 @@ import google.registry.model.domain.Period; import google.registry.model.domain.rgp.GracePeriodStatus; import google.registry.model.eppcommon.AuthInfo; import google.registry.model.eppcommon.StatusValue; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.transfer.TransferStatus; import google.registry.persistence.VKey; import java.util.List; @@ -70,22 +69,17 @@ public final class ResourceFlowUtils { /** * Check whether if there are domains linked to the resource to be deleted. Throws an exception if * so. - * - *

Note that in datastore this is a smoke test as the query for linked domains is eventually - * consistent, so we only check a few domains to fail fast. */ public static void checkLinkedDomains( final String targetId, final DateTime now, final Class resourceClass) throws EppException { EppException failfastException = tm().transact( () -> { - final ForeignKeyIndex fki = ForeignKeyIndex.load(resourceClass, targetId, now); - if (fki == null) { + VKey key = ForeignKeyUtils.load(resourceClass, targetId, now); + if (key == null) { return new ResourceDoesNotExistException(resourceClass, targetId); } - return isLinked(fki.getResourceKey(), now) - ? new ResourceToDeleteIsReferencedException() - : null; + return isLinked(key, now) ? new ResourceToDeleteIsReferencedException() : null; }); if (failfastException != null) { throw failfastException; @@ -118,7 +112,7 @@ public final class ResourceFlowUtils { public static void verifyResourceDoesNotExist( Class clazz, String targetId, DateTime now, String registrarId) throws EppException { - VKey key = loadAndGetKey(clazz, targetId, now); + VKey key = ForeignKeyUtils.load(clazz, targetId, now); if (key != null) { R resource = tm().loadByKey(key); // These are similar exceptions, but we can track them internally as log-based metrics. diff --git a/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java b/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java index c83b66671..f5c52884b 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java @@ -53,6 +53,7 @@ import google.registry.flows.custom.DomainCheckFlowCustomLogic.BeforeResponseRet import google.registry.flows.domain.token.AllocationTokenDomainCheckResults; import google.registry.flows.domain.token.AllocationTokenFlowUtils; import google.registry.model.EppResource; +import google.registry.model.ForeignKeyUtils; import google.registry.model.billing.BillingEvent; import google.registry.model.domain.Domain; import google.registry.model.domain.DomainCommand.Check; @@ -70,7 +71,6 @@ import google.registry.model.eppoutput.CheckData.DomainCheck; import google.registry.model.eppoutput.CheckData.DomainCheckData; import google.registry.model.eppoutput.EppResponse; import google.registry.model.eppoutput.EppResponse.ResponseExtension; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.model.tld.Registry; import google.registry.model.tld.Registry.TldState; @@ -169,8 +169,8 @@ public final class DomainCheckFlow implements Flow { // TODO: Use as of date from fee extension v0.12 instead of now, if specified. .setAsOfDate(now) .build()); - ImmutableMap> existingDomains = - ForeignKeyIndex.load(Domain.class, domainNames, now); + ImmutableMap> existingDomains = + ForeignKeyUtils.load(Domain.class, domainNames, now); Optional allocationTokenExtension = eppInput.getSingleExtension(AllocationTokenExtension.class); Optional tokenDomainCheckResults = @@ -227,7 +227,7 @@ public final class DomainCheckFlow implements Flow { private Optional getMessageForCheck( InternetDomainName domainName, - ImmutableMap> existingDomains, + ImmutableMap> existingDomains, ImmutableMap tokenCheckResults, ImmutableMap tldStates, Optional allocationToken) { @@ -251,7 +251,7 @@ public final class DomainCheckFlow implements Flow { /** Handle the fee check extension. */ private ImmutableList getResponseExtensions( ImmutableMap domainNames, - ImmutableMap> existingDomains, + ImmutableMap> existingDomains, ImmutableSet availableDomains, DateTime now, Optional allocationToken) @@ -297,14 +297,14 @@ public final class DomainCheckFlow implements Flow { * renewal is part of the cost of a restore. * *

This may be resource-intensive for large checks of many restore fees, but those are - * comparatively rare, and we are at least using an in-memory cache. Also this will get a lot + * comparatively rare, and we are at least using an in-memory cache. Also, this will get a lot * nicer in Cloud SQL when we can SELECT just the fields we want rather than having to load the * entire entity. */ private ImmutableMap loadDomainsForRestoreChecks( FeeCheckCommandExtension feeCheck, ImmutableMap domainNames, - ImmutableMap> existingDomains) { + ImmutableMap> existingDomains) { ImmutableList restoreCheckDomains; if (feeCheck instanceof FeeCheckCommandExtensionV06) { // The V06 fee extension supports specifying the command fees to check on a per-domain basis. @@ -329,7 +329,7 @@ public final class DomainCheckFlow implements Flow { ImmutableMap> existingDomainsToLoad = restoreCheckDomains.stream() .filter(existingDomains::containsKey) - .collect(toImmutableMap(d -> d, d -> existingDomains.get(d).getResourceKey())); + .collect(toImmutableMap(d -> d, existingDomains::get)); ImmutableMap, EppResource> loadedDomains = EppResource.loadCached(ImmutableList.copyOf(existingDomainsToLoad.values())); return ImmutableMap.copyOf( 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 12a5cda99..db28d7548 100644 --- a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java @@ -26,7 +26,6 @@ import static google.registry.flows.host.HostFlowUtils.lookupSuperordinateDomain import static google.registry.flows.host.HostFlowUtils.validateHostName; import static google.registry.flows.host.HostFlowUtils.verifySuperordinateDomainNotInPendingDelete; import static google.registry.flows.host.HostFlowUtils.verifySuperordinateDomainOwnership; -import static google.registry.model.index.ForeignKeyIndex.loadAndGetKey; import static google.registry.model.reporting.HistoryEntry.Type.HOST_UPDATE; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.CollectionUtils.isNullOrEmpty; @@ -46,6 +45,7 @@ import google.registry.flows.TransactionalFlow; import google.registry.flows.annotations.ReportingSpec; import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException; import google.registry.model.EppResource; +import google.registry.model.ForeignKeyUtils; import google.registry.model.ImmutableObject; import google.registry.model.domain.Domain; import google.registry.model.domain.metadata.MetadataExtension; @@ -147,7 +147,7 @@ public final class HostUpdateFlow implements TransactionalFlow { EppResource owningResource = firstNonNull(oldSuperordinateDomain, existingHost); verifyUpdateAllowed( command, existingHost, newSuperordinateDomain.orElse(null), owningResource, isHostRename); - if (isHostRename && loadAndGetKey(Host.class, newHostName, now) != null) { + if (isHostRename && ForeignKeyUtils.load(Host.class, newHostName, now) != null) { throw new HostAlreadyExistsException(newHostName); } AddRemove add = command.getInnerAdd(); diff --git a/core/src/main/java/google/registry/model/EppResourceUtils.java b/core/src/main/java/google/registry/model/EppResourceUtils.java index 04b25cf58..5c6d7318a 100644 --- a/core/src/main/java/google/registry/model/EppResourceUtils.java +++ b/core/src/main/java/google/registry/model/EppResourceUtils.java @@ -36,7 +36,6 @@ import google.registry.model.contact.Contact; import google.registry.model.domain.Domain; import google.registry.model.eppcommon.StatusValue; import google.registry.model.host.Host; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.reporting.HistoryEntry; import google.registry.model.reporting.HistoryEntryDao; import google.registry.model.tld.Registry; @@ -117,20 +116,19 @@ public final class EppResourceUtils { } /** - * Loads the last created version of an {@link EppResource} from Datastore by foreign key, using a - * cache. + * Loads the last created version of an {@link EppResource} from the database by foreign key, + * using a cache. * *

Returns null if no resource with this foreign key was ever created, or if the most recently * created resource was deleted before time "now". * *

Loading an {@link EppResource} by itself is not sufficient to know its current state since * it may have various expirable conditions and status values that might implicitly change its - * state as time progresses even if it has not been updated in Datastore. Rather, the resource + * state as time progresses even if it has not been updated in the database. Rather, the resource * must be combined with a timestamp to view its current state. We use a global last updated - * timestamp on the resource's entity group (which is essentially free since all writes to the - * entity group must be serialized anyways) to guarantee monotonically increasing write times, and - * forward our projected time to the greater of this timestamp or "now". This guarantees that - * we're not projecting into the past. + * timestamp to guarantee monotonically increasing write times, and forward our projected time to + * the greater of this timestamp or "now". This guarantees that we're not projecting into the + * past. * *

Do not call this cached version for anything that needs transactional consistency. It should * only be used when it's OK if the data is potentially being out of date, e.g. WHOIS. @@ -150,19 +148,18 @@ public final class EppResourceUtils { checkArgument( ForeignKeyedEppResource.class.isAssignableFrom(clazz), "loadByForeignKey may only be called for foreign keyed EPP resources"); - ForeignKeyIndex fki = + VKey key = useCache - ? ForeignKeyIndex.loadCached(clazz, ImmutableList.of(foreignKey), now) - .getOrDefault(foreignKey, null) - : ForeignKeyIndex.load(clazz, foreignKey, now); - // The value of fki.getResourceKey() might be null for hard-deleted prober data. - if (fki == null || isAtOrAfter(now, fki.getDeletionTime()) || fki.getResourceKey() == null) { + ? ForeignKeyUtils.loadCached(clazz, ImmutableList.of(foreignKey), now).get(foreignKey) + : ForeignKeyUtils.load(clazz, foreignKey, now); + // The returned key is null if the resource is hard deleted or soft deleted by the given time. + if (key == null) { return Optional.empty(); } T resource = useCache - ? EppResource.loadCached(fki.getResourceKey()) - : tm().transact(() -> tm().loadByKeyIfPresent(fki.getResourceKey()).orElse(null)); + ? EppResource.loadCached(key) + : tm().transact(() -> tm().loadByKeyIfPresent(key).orElse(null)); if (resource == null || isAtOrAfter(now, resource.getDeletionTime())) { return Optional.empty(); } @@ -178,7 +175,7 @@ public final class EppResourceUtils { } /** - * Checks multiple {@link EppResource} objects from Datastore by unique ids. + * Checks multiple {@link EppResource} objects from the database by unique ids. * *

There are currently no resources that support checks and do not use foreign keys. If we need * to support that case in the future, we can loosen the type to allow any {@link EppResource} and @@ -190,7 +187,7 @@ public final class EppResourceUtils { */ public static ImmutableSet checkResourcesExist( Class clazz, List uniqueIds, final DateTime now) { - return ForeignKeyIndex.load(clazz, uniqueIds, now).keySet(); + return ForeignKeyUtils.load(clazz, uniqueIds, now).keySet(); } /** diff --git a/core/src/main/java/google/registry/model/ForeignKeyUtils.java b/core/src/main/java/google/registry/model/ForeignKeyUtils.java new file mode 100644 index 000000000..0dbf8e7a1 --- /dev/null +++ b/core/src/main/java/google/registry/model/ForeignKeyUtils.java @@ -0,0 +1,236 @@ +// Copyright 2022 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.model; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static google.registry.config.RegistryConfig.getEppResourceCachingDuration; +import static google.registry.config.RegistryConfig.getEppResourceMaxCachedEntries; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static google.registry.persistence.transaction.TransactionManagerFactory.replicaJpaTm; + +import com.github.benmanes.caffeine.cache.CacheLoader; +import com.github.benmanes.caffeine.cache.LoadingCache; +import com.google.auto.value.AutoValue; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import com.google.common.collect.Streams; +import google.registry.config.RegistryConfig; +import google.registry.model.contact.Contact; +import google.registry.model.domain.Domain; +import google.registry.model.host.Host; +import google.registry.persistence.VKey; +import google.registry.persistence.transaction.JpaTransactionManager; +import google.registry.util.NonFinalForTesting; +import java.time.Duration; +import java.util.Collection; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Optional; +import javax.annotation.Nullable; +import org.joda.time.DateTime; + +/** + * Util class to map a foreign key to the {@link VKey} to the active instance of {@link EppResource} + * whose unique repoId matches the foreign key string at a given time. The instance is never + * deleted, but it is updated if a newer entity becomes the active entity. + */ +public final class ForeignKeyUtils { + + private ForeignKeyUtils() {} + + private static final ImmutableMap, String> + RESOURCE_TYPE_TO_FK_PROPERTY = + ImmutableMap.of( + Contact.class, "contactId", + Domain.class, "fullyQualifiedDomainName", + Host.class, "fullyQualifiedHostName"); + + /** + * Loads a {@link VKey} to an {@link EppResource} from the database by foreign key. + * + *

Returns null if no resource with this foreign key was ever created, or if the most recently + * created resource was deleted before time "now". + * + * @param clazz the resource type to load + * @param foreignKey foreign key to match + * @param now the current logical time to use when checking for soft deletion of the foreign key + * index + */ + @Nullable + public static VKey load( + Class clazz, String foreignKey, DateTime now) { + return load(clazz, ImmutableList.of(foreignKey), now).get(foreignKey); + } + + /** + * Load a map of {@link String} foreign keys to {@link VKey}s to {@link EppResource} that are + * active at or after the specified moment in time. + * + *

The returned map will omit any foreign keys for which the {@link EppResource} doesn't exist + * or has been soft deleted. + */ + public static ImmutableMap> load( + Class clazz, Collection foreignKeys, final DateTime now) { + return load(clazz, foreignKeys, false).entrySet().stream() + .filter(e -> now.isBefore(e.getValue().deletionTime())) + .collect(toImmutableMap(Entry::getKey, e -> VKey.createSql(clazz, e.getValue().repoId()))); + } + + /** + * Helper method to load {@link VKey}s to all the most recent {@link EppResource}s for the given + * foreign 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 production, the {@code deletionTime} for entities with the same foreign key + * should monotonically increase as one cannot create a domain/host/contact with the same foreign + * key without soft deleting the existing resource first. However, in test, there's no such + * guarantee and one must make sure that no two resources with the same foreign key exist with the + * same max {@code deleteTime}, usually {@code END_OF_TIME}, lest this method throws an error due + * to duplicate keys. + */ + private static ImmutableMap load( + Class clazz, Collection foreignKeys, boolean useReplicaJpaTm) { + String fkProperty = RESOURCE_TYPE_TO_FK_PROPERTY.get(clazz); + JpaTransactionManager jpaTmToUse = useReplicaJpaTm ? replicaJpaTm() : jpaTm(); + return jpaTmToUse.transact( + () -> + jpaTmToUse + .query( + ("SELECT %fkProperty%, repoId, deletionTime FROM %entity% WHERE (%fkProperty%," + + " deletionTime) IN (SELECT %fkProperty%, MAX(deletionTime) FROM" + + " %entity% WHERE %fkProperty% IN (:fks) GROUP BY %fkProperty%)") + .replace("%fkProperty%", fkProperty) + .replace("%entity%", clazz.getSimpleName()), + Object[].class) + .setParameter("fks", foreignKeys) + .getResultStream() + .collect( + toImmutableMap( + row -> (String) row[0], + row -> MostRecentResource.create((String) row[1], (DateTime) row[2])))); + } + + private static final CacheLoader, Optional> + CACHE_LOADER = + new CacheLoader, Optional>() { + + @Override + public Optional load(VKey key) { + return loadAll(ImmutableList.of(key)).get(key); + } + + @Override + public Map, Optional> loadAll( + Iterable> keys) { + if (!keys.iterator().hasNext()) { + return ImmutableMap.of(); + } + // It is safe to use the resource type of first element because when this function is + // called, it is always passed with a list of VKeys with the same type. + Class clazz = keys.iterator().next().getKind(); + ImmutableList foreignKeys = + Streams.stream(keys) + .map(key -> (String) key.getSqlKey()) + .collect(toImmutableList()); + ImmutableMap existingKeys = + ForeignKeyUtils.load(clazz, foreignKeys, true); + // The above map only contains keys that exist in the database, so we re-add the + // missing ones with Optional.empty() values for caching. + return Maps.asMap( + ImmutableSet.copyOf(keys), + key -> Optional.ofNullable(existingKeys.get((String) key.getSqlKey()))); + } + }; + + /** + * A limited size, limited time cache for foreign-keyed entities. + * + *

This is only used to cache foreign-keyed entities for the purposes of checking whether they + * exist (and if so, what entity they point to) during a few domain flows. Any other operations on + * foreign keys should not use this cache. + * + *

Note that here the key of the {@link LoadingCache} is of type {@code VKey}, but they are not legal {VKey}s to {@link EppResource}s, whose keys are the SQL + * primary keys, i.e. the {@code repoId}s. Instead, their keys are the foreign keys used to query + * the database. We use {@link VKey} here because it is a convenient composite class that contains + * both the resource type and the foreign key, which are needed to for the query and caching. + * + *

Also note that the value type of this cache is {@link Optional} because the foreign keys in + * question are coming from external commands, and thus don't necessarily represent entities in + * our system that actually exist. So we cache the fact that they *don't* exist by using + * Optional.empty(), then several layers up the EPP command will fail with an error message like + * "The contact with given IDs (blah) don't exist." + */ + @NonFinalForTesting + private static LoadingCache, Optional> + foreignKeyCache = createForeignKeyMapCache(getEppResourceCachingDuration()); + + private static LoadingCache, Optional> + createForeignKeyMapCache(Duration expiry) { + return CacheUtils.newCacheBuilder(expiry) + .maximumSize(getEppResourceMaxCachedEntries()) + .build(CACHE_LOADER); + } + + @VisibleForTesting + public static void setCacheForTest(Optional expiry) { + Duration effectiveExpiry = expiry.orElse(getEppResourceCachingDuration()); + foreignKeyCache = createForeignKeyMapCache(effectiveExpiry); + } + + /** + * Load a list of {@link VKey} to {@link EppResource} instances by class and foreign key strings + * that are active at or after the specified moment in time, using the cache if enabled. + * + *

The returned map will omit any keys for which the {@link EppResource} doesn't exist or has + * been soft deleted. + * + *

Don't use the cached version of this method unless you really need it for performance + * reasons, and are OK with the trade-offs in loss of transactional consistency. + */ + public static ImmutableMap> loadCached( + Class clazz, Collection foreignKeys, final DateTime now) { + if (!RegistryConfig.isEppResourceCachingEnabled()) { + return load(clazz, foreignKeys, now); + } + return foreignKeyCache + .getAll( + foreignKeys.stream().map(fk -> VKey.createSql(clazz, fk)).collect(toImmutableList())) + .entrySet() + .stream() + .filter(e -> e.getValue().isPresent() && now.isBefore(e.getValue().get().deletionTime())) + .collect( + toImmutableMap( + e -> (String) e.getKey().getSqlKey(), + e -> VKey.createSql(clazz, e.getValue().get().repoId()))); + } + + @AutoValue + abstract static class MostRecentResource { + + abstract String repoId(); + + abstract DateTime deletionTime(); + + static MostRecentResource create(String repoId, DateTime deletionTime) { + return new AutoValue_ForeignKeyUtils_MostRecentResource(repoId, deletionTime); + } + } +} diff --git a/core/src/main/java/google/registry/model/contact/Contact.java b/core/src/main/java/google/registry/model/contact/Contact.java index 68eac17b3..1b8547756 100644 --- a/core/src/main/java/google/registry/model/contact/Contact.java +++ b/core/src/main/java/google/registry/model/contact/Contact.java @@ -14,7 +14,6 @@ package google.registry.model.contact; -import com.googlecode.objectify.Key; import google.registry.model.EppResource.ForeignKeyedEppResource; import google.registry.model.annotations.ExternalMessagingName; import google.registry.model.annotations.ReportedOn; @@ -46,13 +45,13 @@ import org.joda.time.DateTime; @Index(columnList = "searchName") }) @ExternalMessagingName("contact") -@WithStringVKey +@WithStringVKey(compositeKey = true) @Access(AccessType.FIELD) public class Contact extends ContactBase implements ForeignKeyedEppResource { @Override public VKey createVKey() { - return VKey.create(Contact.class, getRepoId(), Key.create(this)); + return VKey.createSql(Contact.class, getRepoId()); } @Override diff --git a/core/src/main/java/google/registry/model/domain/Domain.java b/core/src/main/java/google/registry/model/domain/Domain.java index 96d1dfaa0..bd9add44b 100644 --- a/core/src/main/java/google/registry/model/domain/Domain.java +++ b/core/src/main/java/google/registry/model/domain/Domain.java @@ -66,7 +66,7 @@ import org.joda.time.DateTime; @Index(columnList = "transfer_billing_event_id"), @Index(columnList = "transfer_billing_recurrence_id") }) -@WithStringVKey +@WithStringVKey(compositeKey = true) @ExternalMessagingName("domain") @Access(AccessType.FIELD) public class Domain extends DomainBase implements ForeignKeyedEppResource { @@ -148,7 +148,7 @@ public class Domain extends DomainBase implements ForeignKeyedEppResource { @Override public VKey createVKey() { - return VKey.create(Domain.class, getRepoId(), Key.create(this)); + return VKey.createSql(Domain.class, getRepoId()); } @Override diff --git a/core/src/main/java/google/registry/model/domain/DomainCommand.java b/core/src/main/java/google/registry/model/domain/DomainCommand.java index 183b90212..b4dcd4615 100644 --- a/core/src/main/java/google/registry/model/domain/DomainCommand.java +++ b/core/src/main/java/google/registry/model/domain/DomainCommand.java @@ -17,7 +17,6 @@ package google.registry.model.domain; import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.collect.Iterables.getOnlyElement; -import static com.google.common.collect.Maps.transformValues; import static com.google.common.collect.Sets.difference; import static google.registry.util.CollectionUtils.difference; import static google.registry.util.CollectionUtils.forceEmptyToNull; @@ -31,6 +30,7 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import google.registry.model.EppResource; +import google.registry.model.ForeignKeyUtils; import google.registry.model.ImmutableObject; import google.registry.model.contact.Contact; import google.registry.model.eppinput.ResourceCommand.AbstractSingleResourceCommand; @@ -39,7 +39,6 @@ import google.registry.model.eppinput.ResourceCommand.ResourceCreateOrChange; import google.registry.model.eppinput.ResourceCommand.ResourceUpdate; import google.registry.model.eppinput.ResourceCommand.SingleResourceCommand; import google.registry.model.host.Host; -import google.registry.model.index.ForeignKeyIndex; import google.registry.persistence.VKey; import java.util.Set; import javax.annotation.Nullable; @@ -445,13 +444,12 @@ public class DomainCommand { private static ImmutableMap> loadByForeignKeysCached( final Set foreignKeys, final Class clazz, final DateTime now) throws InvalidReferencesException { - ImmutableMap> fkis = - ForeignKeyIndex.loadCached(clazz, foreignKeys, now); - if (!fkis.keySet().equals(foreignKeys)) { + ImmutableMap> fks = ForeignKeyUtils.loadCached(clazz, foreignKeys, now); + if (!fks.keySet().equals(foreignKeys)) { throw new InvalidReferencesException( - clazz, ImmutableSet.copyOf(difference(foreignKeys, fkis.keySet()))); + clazz, ImmutableSet.copyOf(difference(foreignKeys, fks.keySet()))); } - return ImmutableMap.copyOf(transformValues(fkis, ForeignKeyIndex::getResourceKey)); + return fks; } /** Exception to throw when referenced objects don't exist. */ diff --git a/core/src/main/java/google/registry/model/host/Host.java b/core/src/main/java/google/registry/model/host/Host.java index 414571212..6d76972d4 100644 --- a/core/src/main/java/google/registry/model/host/Host.java +++ b/core/src/main/java/google/registry/model/host/Host.java @@ -14,7 +14,6 @@ package google.registry.model.host; -import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; import google.registry.model.EppResource.ForeignKeyedEppResource; import google.registry.model.annotations.ExternalMessagingName; @@ -52,7 +51,7 @@ import javax.persistence.AccessType; @javax.persistence.Index(columnList = "currentSponsorRegistrarId") }) @ExternalMessagingName("host") -@WithStringVKey +@WithStringVKey(compositeKey = true) @Access(AccessType.FIELD) // otherwise it'll use the default if the repoId (property) public class Host extends HostBase implements ForeignKeyedEppResource { @@ -65,7 +64,7 @@ public class Host extends HostBase implements ForeignKeyedEppResource { @Override public VKey createVKey() { - return VKey.create(Host.class, getRepoId(), Key.create(this)); + return VKey.createSql(Host.class, getRepoId()); } @Override diff --git a/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java b/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java deleted file mode 100644 index 9547467ae..000000000 --- a/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java +++ /dev/null @@ -1,311 +0,0 @@ -// Copyright 2017 The Nomulus Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package google.registry.model.index; - -import static com.google.common.collect.ImmutableList.toImmutableList; -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.persistence.transaction.TransactionManagerFactory.jpaTm; -import static google.registry.persistence.transaction.TransactionManagerFactory.replicaJpaTm; -import static google.registry.util.CollectionUtils.entriesToImmutableMap; -import static google.registry.util.TypeUtils.instantiate; - -import com.github.benmanes.caffeine.cache.CacheLoader; -import com.github.benmanes.caffeine.cache.LoadingCache; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableBiMap; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -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 google.registry.config.RegistryConfig; -import google.registry.model.BackupGroupRoot; -import google.registry.model.CacheUtils; -import google.registry.model.EppResource; -import google.registry.model.contact.Contact; -import google.registry.model.domain.Domain; -import google.registry.model.host.Host; -import google.registry.persistence.VKey; -import google.registry.persistence.transaction.CriteriaQueryBuilder; -import google.registry.persistence.transaction.JpaTransactionManager; -import google.registry.util.NonFinalForTesting; -import java.time.Duration; -import java.util.Collection; -import java.util.Comparator; -import java.util.Map; -import java.util.Optional; -import javax.annotation.Nullable; -import org.joda.time.DateTime; - -/** - * Class to map a foreign key to the active instance of {@link EppResource} whose unique id matches - * the foreign key string. The instance is never deleted, but it is updated if a newer entity - * becomes the active entity. - */ -public abstract class ForeignKeyIndex extends BackupGroupRoot { - - /** The {@link ForeignKeyIndex} type for {@link Contact} entities. */ - public static class ForeignKeyContactIndex extends ForeignKeyIndex {} - - /** The {@link ForeignKeyIndex} type for {@link Domain} entities. */ - public static class ForeignKeyDomainIndex extends ForeignKeyIndex {} - - /** The {@link ForeignKeyIndex} type for {@link Host} entities. */ - public static class ForeignKeyHostIndex extends ForeignKeyIndex {} - - private static final ImmutableBiMap< - Class, Class>> - RESOURCE_CLASS_TO_FKI_CLASS = - ImmutableBiMap.of( - Contact.class, ForeignKeyContactIndex.class, - Domain.class, ForeignKeyDomainIndex.class, - Host.class, ForeignKeyHostIndex.class); - - private static final ImmutableMap, String> - RESOURCE_CLASS_TO_FKI_PROPERTY = - ImmutableMap.of( - Contact.class, "contactId", - Domain.class, "fullyQualifiedDomainName", - Host.class, "fullyQualifiedHostName"); - - String foreignKey; - - /** - * The deletion time of this {@link ForeignKeyIndex}. - * - *

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. - */ - DateTime deletionTime; - - /** The referenced resource. */ - VKey reference; - - public String getForeignKey() { - return foreignKey; - } - - public DateTime getDeletionTime() { - return deletionTime; - } - - public VKey getResourceKey() { - return reference; - } - - @SuppressWarnings("unchecked") - public static Class> mapToFkiClass( - Class resourceClass) { - return (Class>) RESOURCE_CLASS_TO_FKI_CLASS.get(resourceClass); - } - - /** Create a {@link ForeignKeyIndex} instance for a resource, expiring at a specified time. */ - @SuppressWarnings("unchecked") - private static ForeignKeyIndex create( - E resource, DateTime deletionTime) { - Class resourceClass = (Class) resource.getClass(); - ForeignKeyIndex instance = instantiate(mapToFkiClass(resourceClass)); - instance.reference = (VKey) resource.createVKey(); - instance.foreignKey = resource.getForeignKey(); - instance.deletionTime = deletionTime; - return instance; - } - - /** - * 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 - * check that the referenced resource actually exists. However, for normal epp resources, it is - * safe to assume that the referenced resource exists if the foreign key index does. - * - * @param clazz the resource type to load - * @param foreignKey id to match - * @param now the current logical time to use when checking for soft deletion of the foreign key - * index - */ - @Nullable - public static VKey loadAndGetKey( - Class clazz, String foreignKey, DateTime now) { - ForeignKeyIndex index = load(clazz, foreignKey, now); - return index == null ? null : index.getResourceKey(); - } - - /** - * Load a {@link ForeignKeyIndex} by class and id string that is active at or after the specified - * moment in time. - * - *

This will return null if the {@link ForeignKeyIndex} doesn't exist or has been soft deleted. - */ - @Nullable - public static ForeignKeyIndex load( - Class clazz, String foreignKey, DateTime now) { - return load(clazz, ImmutableList.of(foreignKey), now).get(foreignKey); - } - - /** - * Load a map of {@link ForeignKeyIndex} instances by class and id strings that are active at or - * after the specified moment in time. - * - *

The returned map will omit any keys for which the {@link ForeignKeyIndex} doesn't exist or - * has been soft deleted. - */ - public static ImmutableMap> load( - Class clazz, Collection foreignKeys, final DateTime now) { - return loadIndexesFromStore(clazz, foreignKeys, false).entrySet().stream() - .filter(e -> now.isBefore(e.getValue().getDeletionTime())) - .collect(entriesToImmutableMap()); - } - - /** - * Helper method to load all of the most recent {@link ForeignKeyIndex}es for the given foreign - * 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. - */ - private static - ImmutableMap> loadIndexesFromStore( - 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 CacheLoader>, Optional>>() { - - @Override - public Optional> load(VKey> key) { - String foreignKey = key.getSqlKey().toString(); - return Optional.ofNullable( - loadIndexesFromStore( - RESOURCE_CLASS_TO_FKI_CLASS.inverse().get(key.getKind()), - ImmutableSet.of(foreignKey), - true) - .get(foreignKey)); - } - - @Override - public Map>, Optional>> loadAll( - Iterable>> keys) { - if (!keys.iterator().hasNext()) { - return ImmutableMap.of(); - } - Class resourceClass = - RESOURCE_CLASS_TO_FKI_CLASS.inverse().get(keys.iterator().next().getKind()); - ImmutableSet foreignKeys = - Streams.stream(keys).map(v -> v.getSqlKey().toString()).collect(toImmutableSet()); - ImmutableSet>> typedKeys = ImmutableSet.copyOf(keys); - ImmutableMap> existingFkis = - 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( - typedKeys, - (VKey> key) -> - Optional.ofNullable(existingFkis.getOrDefault(key.getSqlKey().toString(), null))); - } - }; - - /** - * A limited size, limited time cache for foreign key entities. - * - *

This is only used to cache foreign key entities for the purposes of checking whether they - * exist (and if so, what entity they point to) during a few domain flows. Any other operations on - * foreign keys should not use this cache. - * - *

Note that the value type of this cache is Optional because the foreign keys in question are - * coming from external commands, and thus don't necessarily represent entities in our system that - * actually exist. So we cache the fact that they *don't* exist by using Optional.empty(), and - * then several layers up the EPP command will fail with an error message like "The contact with - * given IDs (blah) don't exist." - */ - @NonFinalForTesting - private static LoadingCache>, Optional>> - cacheForeignKeyIndexes = createForeignKeyIndexesCache(getEppResourceCachingDuration()); - - private static LoadingCache>, Optional>> - createForeignKeyIndexesCache(Duration expiry) { - return CacheUtils.newCacheBuilder(expiry) - .maximumSize(getEppResourceMaxCachedEntries()) - .build(CACHE_LOADER); - } - - @VisibleForTesting - public static void setCacheForTest(Optional expiry) { - Duration effectiveExpiry = expiry.orElse(getEppResourceCachingDuration()); - cacheForeignKeyIndexes = createForeignKeyIndexesCache(effectiveExpiry); - } - - /** - * Load a list of {@link ForeignKeyIndex} instances by class and id strings that are active at or - * after the specified moment in time, using the cache if enabled. - * - *

The returned map will omit any keys for which the {@link ForeignKeyIndex} doesn't exist or - * has been soft deleted. - * - *

Don't use the cached version of this method unless you really need it for performance - * reasons, and are OK with the trade-offs in loss of transactional consistency. - */ - public static ImmutableMap> loadCached( - Class clazz, Collection foreignKeys, final DateTime now) { - if (!RegistryConfig.isEppResourceCachingEnabled()) { - 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.createSql(fkiClass, fk)) - .collect(toImmutableList()); - // This cast is safe because when we loaded ForeignKeyIndexes above we used type clazz, which - // is scoped to E. - @SuppressWarnings("unchecked") - ImmutableMap> fkisFromCache = - cacheForeignKeyIndexes.getAll(fkiVKeys).entrySet().stream() - .filter(entry -> entry.getValue().isPresent()) - .filter(entry -> now.isBefore(entry.getValue().get().getDeletionTime())) - .collect( - toImmutableMap( - entry -> entry.getKey().getSqlKey().toString(), - entry -> (ForeignKeyIndex) entry.getValue().get())); - return fkisFromCache; - } -} diff --git a/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java b/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java index 86b0df057..8bafb6f7e 100644 --- a/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java +++ b/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java @@ -16,7 +16,6 @@ package google.registry.rdap; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.model.EppResourceUtils.loadByForeignKeyCached; -import static google.registry.model.index.ForeignKeyIndex.loadAndGetKey; import static google.registry.model.ofy.ObjectifyService.auditedOfy; import static google.registry.persistence.transaction.TransactionManagerFactory.replicaJpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; @@ -36,6 +35,7 @@ import com.google.common.flogger.FluentLogger; import com.google.common.net.InetAddresses; import com.google.common.primitives.Booleans; import com.googlecode.objectify.cmd.Query; +import google.registry.model.ForeignKeyUtils; import google.registry.model.domain.Domain; import google.registry.model.host.Host; import google.registry.persistence.VKey; @@ -331,9 +331,9 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { return getNameserverRefsByLdhNameWithSuffix(partialStringQuery); } // If there's no suffix, query the host resources. Query the resources themselves, rather than - // the foreign key indexes, because then we have an index on fully qualified host name and - // deletion time, so we can check the deletion status in the query itself. The initial string - // must be present, to avoid querying every host in the system. This restriction is enforced by + // the foreign keys, because then we have an index on fully qualified host name and deletion + // time, so we can check the deletion status in the query itself. The initial string must be + // present, to avoid querying every host in the system. This restriction is enforced by // {@link queryItems}. // // Only return the first maxNameserversInFirstStage nameservers. This could result in an @@ -400,7 +400,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { : ImmutableList.of(host.get().createVKey()); } else { VKey hostKey = - loadAndGetKey( + ForeignKeyUtils.load( Host.class, partialStringQuery.getInitialString(), shouldIncludeDeleted() ? START_OF_TIME : getRequestTime()); @@ -442,7 +442,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { } } else { VKey hostKey = - loadAndGetKey( + ForeignKeyUtils.load( Host.class, fqhn, shouldIncludeDeleted() ? START_OF_TIME : getRequestTime()); if (hostKey != null) { builder.add(hostKey); diff --git a/core/src/main/java/google/registry/tools/CommandUtilities.java b/core/src/main/java/google/registry/tools/CommandUtilities.java index 3d76ea3cc..7a30a8572 100644 --- a/core/src/main/java/google/registry/tools/CommandUtilities.java +++ b/core/src/main/java/google/registry/tools/CommandUtilities.java @@ -19,10 +19,10 @@ import static com.google.common.base.Preconditions.checkState; import com.google.common.base.Ascii; import com.google.common.base.Strings; import google.registry.model.EppResource; +import google.registry.model.ForeignKeyUtils; import google.registry.model.contact.Contact; import google.registry.model.domain.Domain; import google.registry.model.host.Host; -import google.registry.model.index.ForeignKeyIndex; import google.registry.persistence.VKey; import org.joda.time.DateTime; @@ -42,7 +42,7 @@ class CommandUtilities { } public VKey getKey(String uniqueId, DateTime now) { - return ForeignKeyIndex.loadAndGetKey(clazz, uniqueId, now); + return ForeignKeyUtils.load(clazz, uniqueId, now); } } diff --git a/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java b/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java index 90e813c59..d514b1a0c 100644 --- a/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java +++ b/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java @@ -31,6 +31,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; +import google.registry.model.ForeignKeyUtils; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Recurring; import google.registry.model.domain.Domain; @@ -38,7 +39,6 @@ import google.registry.model.domain.DomainHistory; import google.registry.model.domain.Period; import google.registry.model.domain.Period.Unit; import google.registry.model.eppcommon.StatusValue; -import google.registry.model.index.ForeignKeyIndex; import google.registry.model.poll.PollMessage; import google.registry.model.reporting.HistoryEntry.Type; import google.registry.util.Clock; @@ -88,7 +88,7 @@ class UnrenewDomainCommand extends ConfirmingCommand implements CommandWithRemot new ImmutableMap.Builder<>(); for (String domainName : mainParameters) { - if (ForeignKeyIndex.load(Domain.class, domainName, START_OF_TIME) == null) { + if (ForeignKeyUtils.load(Domain.class, domainName, START_OF_TIME) == null) { domainsNonexistentBuilder.add(domainName); continue; } diff --git a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java index ef75dda1b..b129669f8 100644 --- a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java +++ b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java @@ -98,7 +98,7 @@ public class ResaveEntityActionTest { clock.advanceOneMilli(); assertThat(domain.getCurrentSponsorRegistrarId()).isEqualTo("TheRegistrar"); runAction( - domain.createVKey().getOfyKey().getString(), + domain.createVKey().stringify(), DateTime.parse("2016-02-06T10:00:01Z"), ImmutableSortedSet.of()); Domain resavedDomain = loadByEntity(domain); @@ -128,7 +128,7 @@ public class ResaveEntityActionTest { assertThat(domain.getGracePeriods()).isNotEmpty(); runAction( - domain.createVKey().getOfyKey().getString(), + domain.createVKey().stringify(), requestedTime, ImmutableSortedSet.of(requestedTime.plusDays(5))); Domain resavedDomain = loadByEntity(domain); diff --git a/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java b/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java index 65398d8b1..7d06c900f 100644 --- a/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java +++ b/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java @@ -104,7 +104,7 @@ public class PublishDnsUpdatesActionTest { persistActiveSubordinateHost("ns1.example.xn--q9jyb4c", domain1); persistActiveSubordinateHost("ns2.example.xn--q9jyb4c", domain1); Domain domain2 = persistActiveDomain("example2.xn--q9jyb4c"); - persistActiveSubordinateHost("ns1.example.xn--q9jyb4c", domain2); + persistActiveSubordinateHost("ns1.example2.xn--q9jyb4c", domain2); clock.advanceOneMilli(); } diff --git a/core/src/test/java/google/registry/flows/contact/ContactUpdateFlowTest.java b/core/src/test/java/google/registry/flows/contact/ContactUpdateFlowTest.java index f77f49458..c61d00d1a 100644 --- a/core/src/test/java/google/registry/flows/contact/ContactUpdateFlowTest.java +++ b/core/src/test/java/google/registry/flows/contact/ContactUpdateFlowTest.java @@ -52,7 +52,6 @@ class ContactUpdateFlowTest extends ResourceFlowTestCase { Host renamedHost = doSuccessfulTest(); assertThat(renamedHost.isSubordinate()).isTrue(); assertDnsTasksEnqueued("ns1.example.tld", "ns2.example.tld"); - ForeignKeyIndex oldFkiAfterRename = - ForeignKeyIndex.load(Host.class, oldHostName(), clock.nowUtc()); - assertThat(oldFkiAfterRename).isNull(); + VKey oldVKeyAfterRename = ForeignKeyUtils.load(Host.class, oldHostName(), clock.nowUtc()); + assertThat(oldVKeyAfterRename).isNull(); } @Test @@ -982,13 +982,13 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { @Test void testFailure_addRemoveSameStatusValues() throws Exception { createTld("tld"); - persistActiveDomain("example.tld"); + Domain domain = persistActiveDomain("example.tld"); setEppHostUpdateInput( "ns1.example.tld", "ns2.example.tld", "", ""); - persistActiveSubordinateHost(oldHostName(), persistActiveDomain("example.tld")); + persistActiveSubordinateHost(oldHostName(), domain); EppException thrown = assertThrows(AddRemoveSameValueException.class, this::runFlow); assertAboutEppExceptions().that(thrown).marshalsToXml(); } diff --git a/core/src/test/java/google/registry/model/ForeignKeyUtilsTest.java b/core/src/test/java/google/registry/model/ForeignKeyUtilsTest.java new file mode 100644 index 000000000..36ee561f3 --- /dev/null +++ b/core/src/test/java/google/registry/model/ForeignKeyUtilsTest.java @@ -0,0 +1,153 @@ +// Copyright 2022 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.model; + +import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.persistActiveContact; +import static google.registry.testing.DatabaseHelper.persistActiveDomain; +import static google.registry.testing.DatabaseHelper.persistActiveHost; +import static google.registry.testing.DatabaseHelper.persistResource; +import static org.joda.time.DateTimeZone.UTC; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import google.registry.model.contact.Contact; +import google.registry.model.domain.Domain; +import google.registry.model.host.Host; +import google.registry.persistence.transaction.JpaTestExtensions; +import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; +import google.registry.testing.FakeClock; +import google.registry.testing.TestCacheExtension; +import java.time.Duration; +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 ForeignKeyUtils}. */ +class ForeignKeyUtilsTest { + + private final FakeClock fakeClock = new FakeClock(DateTime.now(UTC)); + + @RegisterExtension + public final JpaIntegrationTestExtension jpaIntegrationTestExtension = + new JpaTestExtensions.Builder().withClock(fakeClock).buildIntegrationTestExtension(); + + @RegisterExtension + public final TestCacheExtension testCacheExtension = + new TestCacheExtension.Builder().withForeignKeyCache(Duration.ofDays(1)).build(); + + @BeforeEach + void setUp() { + createTld("com"); + } + + @Test + void testSuccess_loadHost() { + Host host = persistActiveHost("ns1.example.com"); + assertThat(ForeignKeyUtils.load(Host.class, "ns1.example.com", fakeClock.nowUtc())) + .isEqualTo(host.createVKey()); + } + + @Test + void testSuccess_loadDomain() { + Domain domain = persistActiveDomain("example.com"); + assertThat(ForeignKeyUtils.load(Domain.class, "example.com", fakeClock.nowUtc())) + .isEqualTo(domain.createVKey()); + } + + @Test + void testSuccess_loadContact() { + Contact contact = persistActiveContact("john-doe"); + assertThat(ForeignKeyUtils.load(Contact.class, "john-doe", fakeClock.nowUtc())) + .isEqualTo(contact.createVKey()); + } + + @Test + void testSuccess_loadMostRecentResource() { + Host host = persistActiveHost("ns1.example.com"); + persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build()); + fakeClock.advanceOneMilli(); + Host newHost = persistActiveHost("ns1.example.com"); + assertThat(ForeignKeyUtils.load(Host.class, "ns1.example.com", fakeClock.nowUtc())) + .isEqualTo(newHost.createVKey()); + } + + @Test + void testSuccess_loadNonexistentForeignKey_returnsNull() { + assertThat(ForeignKeyUtils.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull(); + } + + @Test + void testSuccess_loadDeletedForeignKey_returnsNull() { + Host host = persistActiveHost("ns1.example.com"); + persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build()); + assertThat(ForeignKeyUtils.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull(); + } + + @Test + void testSuccess_mostRecentKeySoftDeleted_returnsNull() { + Host host1 = persistActiveHost("ns1.example.com"); + fakeClock.advanceOneMilli(); + persistResource(host1.asBuilder().setDeletionTime(fakeClock.nowUtc()).build()); + assertThat(ForeignKeyUtils.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull(); + } + + @Test + void testSuccess_batchLoad_skipsDeletedAndNonexistent() { + Host host1 = persistActiveHost("ns1.example.com"); + Host host2 = persistActiveHost("ns2.example.com"); + persistResource(host2.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build()); + assertThat( + ForeignKeyUtils.load( + Host.class, + ImmutableList.of("ns1.example.com", "ns2.example.com", "ns3.example.com"), + fakeClock.nowUtc())) + .containsExactlyEntriesIn(ImmutableMap.of("ns1.example.com", host1.createVKey())); + persistResource(host1.asBuilder().setDeletionTime(fakeClock.nowUtc()).build()); + fakeClock.advanceOneMilli(); + Host newHost1 = persistActiveHost("ns1.example.com"); + assertThat( + ForeignKeyUtils.loadCached( + Host.class, + ImmutableList.of("ns1.example.com", "ns2.example.com", "ns3.example.com"), + fakeClock.nowUtc())) + .containsExactlyEntriesIn(ImmutableMap.of("ns1.example.com", newHost1.createVKey())); + } + + @Test + void testSuccess_loadHostsCached_cacheIsStale() { + Host host1 = persistActiveHost("ns1.example.com"); + Host host2 = persistActiveHost("ns2.example.com"); + persistResource(host2.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build()); + assertThat( + ForeignKeyUtils.loadCached( + Host.class, + ImmutableList.of("ns1.example.com", "ns2.example.com", "ns3.example.com"), + fakeClock.nowUtc())) + .containsExactlyEntriesIn(ImmutableMap.of("ns1.example.com", host1.createVKey())); + persistResource(host1.asBuilder().setDeletionTime(fakeClock.nowUtc()).build()); + fakeClock.advanceOneMilli(); + persistActiveHost("ns1.example.com"); + // Even though a new host1 is now live, the cache still returns the VKey to the old one. + assertThat( + ForeignKeyUtils.loadCached( + Host.class, + ImmutableList.of("ns1.example.com", "ns2.example.com", "ns3.example.com"), + fakeClock.nowUtc())) + .containsExactlyEntriesIn(ImmutableMap.of("ns1.example.com", host1.createVKey())); + } +} diff --git a/core/src/test/java/google/registry/model/domain/DomainTest.java b/core/src/test/java/google/registry/model/domain/DomainTest.java index e1a9fb4d2..bf702c041 100644 --- a/core/src/test/java/google/registry/model/domain/DomainTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainTest.java @@ -168,9 +168,8 @@ public class DomainTest { domain = persistResource( cloneAndSetAutoTimestamps( - new Domain.Builder() - .setDomainName("example.com") - .setRepoId("4-COM") + domain + .asBuilder() .setCreationRegistrarId("TheRegistrar") .setLastEppUpdateTime(fakeClock.nowUtc()) .setLastEppUpdateRegistrarId("NewRegistrar") diff --git a/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java b/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java deleted file mode 100644 index 6ce393696..000000000 --- a/core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright 2017 The Nomulus Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package google.registry.model.index; - -import static com.google.common.truth.Truth.assertThat; -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.host.Host; -import google.registry.testing.TestCacheExtension; -import java.time.Duration; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -/** Unit tests for {@link ForeignKeyIndex}. */ -class ForeignKeyIndexTest extends EntityTestCase { - - @RegisterExtension - public final TestCacheExtension testCacheExtension = - new TestCacheExtension.Builder().withForeignIndexKeyCache(Duration.ofDays(1)).build(); - - @BeforeEach - void setUp() { - createTld("com"); - } - - @Test - void testLoadForNonexistentForeignKey_returnsNull() { - assertThat(ForeignKeyIndex.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull(); - } - - @Test - void testLoadForDeletedForeignKey_returnsNull() { - Host host = persistActiveHost("ns1.example.com"); - persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build()); - assertThat(ForeignKeyIndex.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull(); - } - - @Test - void testLoad_newerKeyHasBeenSoftDeleted() { - Host host1 = persistActiveHost("ns1.example.com"); - fakeClock.advanceOneMilli(); - persistResource(host1.asBuilder().setDeletionTime(fakeClock.nowUtc()).build()); - assertThat(ForeignKeyIndex.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull(); - } - - @Test - void testBatchLoad_skipsDeletedAndNonexistent() { - persistActiveHost("ns1.example.com"); - Host host = persistActiveHost("ns2.example.com"); - persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build()); - assertThat( - ForeignKeyIndex.load( - Host.class, - ImmutableList.of("ns1.example.com", "ns2.example.com", "ns3.example.com"), - fakeClock.nowUtc()) - .keySet()) - .containsExactly("ns1.example.com"); - } - - @Test - void testDeadCodeThatDeletedScrapCommandsReference() { - persistActiveHost("omg"); - assertThat(ForeignKeyIndex.load(Host.class, "omg", fakeClock.nowUtc()).getForeignKey()) - .isEqualTo("omg"); - } -} diff --git a/core/src/test/java/google/registry/testing/TestCacheExtension.java b/core/src/test/java/google/registry/testing/TestCacheExtension.java index 2c5c06673..cdfd3cb21 100644 --- a/core/src/test/java/google/registry/testing/TestCacheExtension.java +++ b/core/src/test/java/google/registry/testing/TestCacheExtension.java @@ -15,13 +15,13 @@ package google.registry.testing; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Maps; import google.registry.model.EppResource; -import google.registry.model.index.ForeignKeyIndex; +import google.registry.model.ForeignKeyUtils; import google.registry.model.tld.label.PremiumListDao; import google.registry.model.tmch.ClaimsListDao; import java.time.Duration; -import java.util.Map; +import java.util.ArrayList; +import java.util.List; import java.util.Optional; import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeEachCallback; @@ -53,37 +53,30 @@ public class TestCacheExtension implements BeforeEachCallback, AfterEachCallback /** Builder for {@link TestCacheExtension}. */ public static class Builder { - private final Map cacheHandlerMap = Maps.newHashMap(); + private final List cacheHandlers = new ArrayList<>(); public Builder withEppResourceCache(Duration expiry) { - cacheHandlerMap.put( - "EppResource.cacheEppResources", - new TestCacheHandler(EppResource::setCacheForTest, expiry)); + cacheHandlers.add(new TestCacheHandler(EppResource::setCacheForTest, expiry)); return this; } - public Builder withForeignIndexKeyCache(Duration expiry) { - cacheHandlerMap.put( - "ForeignKeyIndex.cacheForeignKeyIndexes", - new TestCacheHandler(ForeignKeyIndex::setCacheForTest, expiry)); + public Builder withForeignKeyCache(Duration expiry) { + cacheHandlers.add(new TestCacheHandler(ForeignKeyUtils::setCacheForTest, expiry)); return this; } public Builder withPremiumListsCache(Duration expiry) { - cacheHandlerMap.put( - "PremiumListSqlDao.premiumListCache", - new TestCacheHandler(PremiumListDao::setPremiumListCacheForTest, expiry)); + cacheHandlers.add(new TestCacheHandler(PremiumListDao::setPremiumListCacheForTest, expiry)); return this; } public Builder withClaimsListCache(Duration expiry) { - cacheHandlerMap.put( - "ClaimsListDao.CACHE", new TestCacheHandler(ClaimsListDao::setCacheForTest, expiry)); + cacheHandlers.add(new TestCacheHandler(ClaimsListDao::setCacheForTest, expiry)); return this; } public TestCacheExtension build() { - return new TestCacheExtension(ImmutableList.copyOf(cacheHandlerMap.values())); + return new TestCacheExtension(ImmutableList.copyOf(cacheHandlers)); } } diff --git a/core/src/test/java/google/registry/tools/GetContactCommandTest.java b/core/src/test/java/google/registry/tools/GetContactCommandTest.java index 59dbf3158..d4493f7cb 100644 --- a/core/src/test/java/google/registry/tools/GetContactCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetContactCommandTest.java @@ -42,11 +42,7 @@ class GetContactCommandTest extends CommandTestCase { persistActiveContact("sh8013"); runCommand("sh8013"); assertInStdout("contactId=sh8013"); - assertInStdout( - "Websafe key: " - + "kind:Contact" - + "@sql:rO0ABXQABjItUk9JRA" - + "@ofy:agR0ZXN0chMLEgdDb250YWN0IgYyLVJPSUQM"); + assertInStdout("Websafe key: " + "kind:Contact" + "@sql:rO0ABXQABjItUk9JRA"); } @Test @@ -54,11 +50,7 @@ class GetContactCommandTest extends CommandTestCase { persistActiveContact("sh8013"); runCommand("sh8013", "--expand"); assertInStdout("contactId=sh8013"); - assertInStdout( - "Websafe key: " - + "kind:Contact" - + "@sql:rO0ABXQABjItUk9JRA" - + "@ofy:agR0ZXN0chMLEgdDb250YWN0IgYyLVJPSUQM"); + assertInStdout("Websafe key: " + "kind:Contact" + "@sql:rO0ABXQABjItUk9JRA"); assertNotInStdout("LiveRef"); } @@ -69,16 +61,8 @@ class GetContactCommandTest extends CommandTestCase { runCommand("sh8013", "jd1234"); assertInStdout("contactId=sh8013"); assertInStdout("contactId=jd1234"); - assertInStdout( - "Websafe key: " - + "kind:Contact" - + "@sql:rO0ABXQABjItUk9JRA" - + "@ofy:agR0ZXN0chMLEgdDb250YWN0IgYyLVJPSUQM"); - assertInStdout( - "Websafe key: " - + "kind:Contact" - + "@sql:rO0ABXQABjMtUk9JRA" - + "@ofy:agR0ZXN0chMLEgdDb250YWN0IgYzLVJPSUQM"); + assertInStdout("Websafe key: " + "kind:Contact" + "@sql:rO0ABXQABjItUk9JRA"); + assertInStdout("Websafe key: " + "kind:Contact" + "@sql:rO0ABXQABjMtUk9JRA"); } @Test diff --git a/core/src/test/java/google/registry/tools/GetDomainCommandTest.java b/core/src/test/java/google/registry/tools/GetDomainCommandTest.java index b74cddbef..e66a01d52 100644 --- a/core/src/test/java/google/registry/tools/GetDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetDomainCommandTest.java @@ -39,11 +39,7 @@ class GetDomainCommandTest extends CommandTestCase { runCommand("example.tld"); assertInStdout("fullyQualifiedDomainName=example.tld"); assertInStdout("Contact=VKey(sql:3-ROID"); - assertInStdout( - "Websafe key: " - + "kind:Domain" - + "@sql:rO0ABXQABTItVExE" - + "@ofy:agR0ZXN0chELEgZEb21haW4iBTItVExEDA"); + assertInStdout("Websafe key: " + "kind:Domain" + "@sql:rO0ABXQABTItVExE"); } @Test @@ -52,11 +48,7 @@ class GetDomainCommandTest extends CommandTestCase { runCommand("example.tld", "--expand"); assertInStdout("fullyQualifiedDomainName=example.tld"); assertInStdout("sqlKey=3-ROID"); - assertInStdout( - "Websafe key: " - + "kind:Domain" - + "@sql:rO0ABXQABTItVExE" - + "@ofy:agR0ZXN0chELEgZEb21haW4iBTItVExEDA"); + assertInStdout("Websafe key: " + "kind:Domain" + "@sql:rO0ABXQABTItVExE"); assertNotInStdout("LiveRef"); } @@ -76,16 +68,8 @@ class GetDomainCommandTest extends CommandTestCase { runCommand("example.tld", "example2.tld"); assertInStdout("fullyQualifiedDomainName=example.tld"); assertInStdout("fullyQualifiedDomainName=example2.tld"); - assertInStdout( - "Websafe key: " - + "kind:Domain" - + "@sql:rO0ABXQABTItVExE" - + "@ofy:agR0ZXN0chELEgZEb21haW4iBTItVExEDA"); - assertInStdout( - "Websafe key: " - + "kind:Domain" - + "@sql:rO0ABXQABTQtVExE" - + "@ofy:agR0ZXN0chELEgZEb21haW4iBTQtVExEDA"); + assertInStdout("Websafe key: " + "kind:Domain" + "@sql:rO0ABXQABTItVExE"); + assertInStdout("Websafe key: " + "kind:Domain" + "@sql:rO0ABXQABTQtVExE"); } @Test diff --git a/core/src/test/java/google/registry/tools/GetHostCommandTest.java b/core/src/test/java/google/registry/tools/GetHostCommandTest.java index cbff4a284..55366907b 100644 --- a/core/src/test/java/google/registry/tools/GetHostCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetHostCommandTest.java @@ -42,11 +42,7 @@ class GetHostCommandTest extends CommandTestCase { persistActiveHost("ns1.example.tld"); runCommand("ns1.example.tld"); assertInStdout("fullyQualifiedHostName=ns1.example.tld"); - assertInStdout( - "Websafe key: " - + "kind:Host" - + "@sql:rO0ABXQABjItUk9JRA" - + "@ofy:agR0ZXN0chALEgRIb3N0IgYyLVJPSUQM"); + assertInStdout("Websafe key: " + "kind:Host" + "@sql:rO0ABXQABjItUk9JRA"); } @Test @@ -54,11 +50,7 @@ class GetHostCommandTest extends CommandTestCase { persistActiveHost("ns1.example.tld"); runCommand("ns1.example.tld", "--expand"); assertInStdout("fullyQualifiedHostName=ns1.example.tld"); - assertInStdout( - "Websafe key: " - + "kind:Host" - + "@sql:rO0ABXQABjItUk9JRA" - + "@ofy:agR0ZXN0chALEgRIb3N0IgYyLVJPSUQM"); + assertInStdout("Websafe key: " + "kind:Host" + "@sql:rO0ABXQABjItUk9JRA"); assertNotInStdout("LiveRef"); } @@ -69,16 +61,8 @@ class GetHostCommandTest extends CommandTestCase { runCommand("ns1.example.tld", "ns2.example.tld"); assertInStdout("fullyQualifiedHostName=ns1.example.tld"); assertInStdout("fullyQualifiedHostName=ns2.example.tld"); - assertInStdout( - "Websafe key: " - + "kind:Host" - + "@sql:rO0ABXQABjItUk9JRA" - + "@ofy:agR0ZXN0chALEgRIb3N0IgYyLVJPSUQM"); - assertInStdout( - "Websafe key: " - + "kind:Host" - + "@sql:rO0ABXQABjMtUk9JRA" - + "@ofy:agR0ZXN0chALEgRIb3N0IgYzLVJPSUQM"); + assertInStdout("Websafe key: " + "kind:Host" + "@sql:rO0ABXQABjItUk9JRA"); + assertInStdout("Websafe key: " + "kind:Host" + "@sql:rO0ABXQABjMtUk9JRA"); } @Test diff --git a/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java b/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java index ec7fcab06..72612da25 100644 --- a/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateDomainCommandTest.java @@ -160,11 +160,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase> nameservers = ImmutableSet.of(host1.createVKey(), host2.createVKey()); - persistResource( - DatabaseHelper.newDomain("example.tld").asBuilder().setNameservers(nameservers).build()); + persistResource(domain.asBuilder().setNameservers(nameservers).build()); runCommandForced( "--client=NewRegistrar", "--nameservers=ns2.zdns.google,ns3.zdns.google", "example.tld"); eppVerifier.verifySent("domain_update_set_nameservers.xml"); @@ -243,7 +238,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase techContactKey = techContact.createVKey(); persistResource( - DatabaseHelper.newDomain("example.tld") + domain .asBuilder() .setContacts( ImmutableSet.of( @@ -261,7 +256,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase> nameservers = ImmutableSet.of(host.createVKey()); persistResource( - DatabaseHelper.newDomain("example.tld") + domain .asBuilder() .setStatusValues( ImmutableSet.of( @@ -370,7 +365,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase techContactKey = techContact.createVKey(); persistResource( - DatabaseHelper.newDomain("example.tld") + domain .asBuilder() .setContacts( ImmutableSet.of( @@ -394,7 +389,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase> nameservers = ImmutableSet.of(host.createVKey()); persistResource( - DatabaseHelper.newDomain("example.tld") + domain .asBuilder() .setStatusValues(ImmutableSet.of(SERVER_UPDATE_PROHIBITED)) .setNameservers(nameservers) @@ -419,7 +414,7 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase> nameservers = ImmutableSet.of(host.createVKey()); persistResource( - DatabaseHelper.newDomain("example.tld") + domain .asBuilder() .setStatusValues(ImmutableSet.of(PENDING_DELETE)) .setNameservers(nameservers) diff --git a/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java b/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java index 1c690cd42..ea6e9de97 100644 --- a/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java +++ b/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java @@ -398,7 +398,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { server.runInAppEngineEnvironment( () -> { createTld("tld"); - persistResource(DatabaseHelper.newDomain("example.tld")); + persistResource(DatabaseHelper.newDomain("example-lock.tld")); saveRegistryLock( new RegistryLock.Builder() .setRegistrarPocId("johndoe@theregistrar.com") @@ -406,7 +406,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { .setRegistrarId("TheRegistrar") .setVerificationCode("f1be78a2-2d61-458c-80f0-9dd8f2f8625f") .isSuperuser(false) - .setDomainName("example.tld") + .setDomainName("example-lock.tld") .build()); return null; }); diff --git a/core/src/test/java/google/registry/whois/WhoisActionTest.java b/core/src/test/java/google/registry/whois/WhoisActionTest.java index c70c62988..b27bf41fb 100644 --- a/core/src/test/java/google/registry/whois/WhoisActionTest.java +++ b/core/src/test/java/google/registry/whois/WhoisActionTest.java @@ -81,7 +81,7 @@ public class WhoisActionTest { public final TestCacheExtension testCacheExtension = new TestCacheExtension.Builder() .withEppResourceCache(Duration.ofDays(1)) - .withForeignIndexKeyCache(Duration.ofDays(1)) + .withForeignKeyCache(Duration.ofDays(1)) .build(); private final FakeResponse response = new FakeResponse(); diff --git a/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_registryLockVerify_success_page.png b/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_registryLockVerify_success_page.png index 76acb814d..00fef9e05 100644 Binary files a/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_registryLockVerify_success_page.png and b/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_registryLockVerify_success_page.png differ