diff --git a/core/src/main/java/google/registry/bsa/BsaRefreshAction.java b/core/src/main/java/google/registry/bsa/BsaRefreshAction.java index 39adc5bc1..8fdb17536 100644 --- a/core/src/main/java/google/registry/bsa/BsaRefreshAction.java +++ b/core/src/main/java/google/registry/bsa/BsaRefreshAction.java @@ -65,7 +65,7 @@ public class BsaRefreshAction implements Runnable { GcsClient gcsClient, BsaReportSender bsaReportSender, @Config("bsaTxnBatchSize") int transactionBatchSize, - @Config("domainTxnMaxDuration") Duration domainTxnMaxDuration, + @Config("domainCreateTxnCommitTimeLag") Duration domainTxnMaxDuration, BsaLock bsaLock, Clock clock, Response response) { diff --git a/core/src/main/java/google/registry/bsa/BsaTransactions.java b/core/src/main/java/google/registry/bsa/BsaTransactions.java index dc6960c36..fd0044ff3 100644 --- a/core/src/main/java/google/registry/bsa/BsaTransactions.java +++ b/core/src/main/java/google/registry/bsa/BsaTransactions.java @@ -14,7 +14,10 @@ package google.registry.bsa; +import static com.google.common.base.Verify.verify; import static google.registry.persistence.PersistenceModule.TransactionIsolationLevel.TRANSACTION_REPEATABLE_READ; +import static google.registry.persistence.transaction.JpaTransactionManagerImpl.isInTransaction; +import static google.registry.persistence.transaction.TransactionManagerFactory.replicaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.errorprone.annotations.CanIgnoreReturnValue; @@ -23,19 +26,23 @@ import java.util.concurrent.Callable; /** * Helpers for executing JPA transactions for BSA processing. * - *

All mutating transactions for BSA may be executed at the {@code TRANSACTION_REPEATABLE_READ} - * level. + *

All mutating transactions for BSA are executed at the {@code TRANSACTION_REPEATABLE_READ} + * level since the global {@link BsaLock} ensures there is a single writer at any time. + * + *

All domain and label queries can use the replica since all processing are snapshot based. */ public final class BsaTransactions { @CanIgnoreReturnValue public static T bsaTransact(Callable work) { + verify(!isInTransaction(), "May only be used for top-level transactions."); return tm().transact(work, TRANSACTION_REPEATABLE_READ); } @CanIgnoreReturnValue public static T bsaQuery(Callable work) { - return tm().transact(work, TRANSACTION_REPEATABLE_READ); + verify(!isInTransaction(), "May only be used for top-level transactions."); + return replicaTm().transact(work, TRANSACTION_REPEATABLE_READ); } private BsaTransactions() {} diff --git a/core/src/main/java/google/registry/bsa/ReservedDomainsUtils.java b/core/src/main/java/google/registry/bsa/ReservedDomainsUtils.java index 24d00efe3..0aa20bc79 100644 --- a/core/src/main/java/google/registry/bsa/ReservedDomainsUtils.java +++ b/core/src/main/java/google/registry/bsa/ReservedDomainsUtils.java @@ -19,6 +19,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.bsa.BsaStringUtils.DOMAIN_JOINER; import static google.registry.flows.domain.DomainFlowUtils.isReserved; import static google.registry.model.tld.Tlds.findTldForName; +import static google.registry.model.tld.label.ReservedList.loadReservedLists; import com.google.common.collect.ImmutableSet; import com.google.common.net.InternetDomainName; @@ -51,12 +52,9 @@ public final class ReservedDomainsUtils { .flatMap(ImmutableSet::stream); } - /** Returns */ + /** Returns all reserved domains in a given {@code tld} as of {@code now}. */ static ImmutableSet getAllReservedDomainsInTld(Tld tld, DateTime now) { - return tld.getReservedListNames().stream() - .map(ReservedList::get) - .filter(Optional::isPresent) - .map(Optional::get) + return loadReservedLists(tld.getReservedListNames()).stream() .map(ReservedList::getReservedListEntries) .map(Map::keySet) .flatMap(Set::stream) diff --git a/core/src/main/java/google/registry/bsa/UploadBsaUnavailableDomainsAction.java b/core/src/main/java/google/registry/bsa/UploadBsaUnavailableDomainsAction.java index a0bfc0e70..01a9cfa5b 100644 --- a/core/src/main/java/google/registry/bsa/UploadBsaUnavailableDomainsAction.java +++ b/core/src/main/java/google/registry/bsa/UploadBsaUnavailableDomainsAction.java @@ -206,12 +206,10 @@ public class UploadBsaUnavailableDomainsAction implements Runnable { new ImmutableSortedSet.Builder<>(Ordering.natural()); for (Tld tld : bsaEnabledTlds) { for (ReservedList reservedList : loadReservedLists(tld.getReservedListNames())) { - if (reservedList.getShouldPublish()) { unavailableDomains.addAll( reservedList.getReservedListEntries().keySet().stream() .map(label -> toDomain(label, tld)) .collect(toImmutableSet())); - } } } diff --git a/core/src/main/java/google/registry/bsa/persistence/DomainsRefresher.java b/core/src/main/java/google/registry/bsa/persistence/DomainsRefresher.java index 80d4c9de2..81323734b 100644 --- a/core/src/main/java/google/registry/bsa/persistence/DomainsRefresher.java +++ b/core/src/main/java/google/registry/bsa/persistence/DomainsRefresher.java @@ -17,9 +17,14 @@ package google.registry.bsa.persistence; 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.bsa.BsaTransactions.bsaQuery; import static google.registry.bsa.ReservedDomainsUtils.getAllReservedNames; import static google.registry.bsa.ReservedDomainsUtils.isReservedDomain; -import static google.registry.bsa.persistence.Queries.queryLivesDomains; +import static google.registry.bsa.persistence.Queries.batchReadUnblockables; +import static google.registry.bsa.persistence.Queries.queryBsaLabelByLabels; +import static google.registry.bsa.persistence.Queries.queryNewlyCreatedDomains; +import static google.registry.model.tld.Tld.isEnrolledWithBsa; +import static google.registry.model.tld.Tlds.getTldEntitiesOfType; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static java.util.stream.Collectors.groupingBy; @@ -36,6 +41,8 @@ import google.registry.bsa.api.UnblockableDomain.Reason; import google.registry.bsa.api.UnblockableDomainChange; import google.registry.model.ForeignKeyUtils; import google.registry.model.domain.Domain; +import google.registry.model.tld.Tld; +import google.registry.model.tld.Tld.TldType; import google.registry.util.BatchedStreams; import java.util.List; import java.util.Map; @@ -124,7 +131,7 @@ public final class DomainsRefresher { ImmutableList batch; Optional lastRead = Optional.empty(); do { - batch = Queries.batchReadUnblockables(lastRead, transactionBatchSize); + batch = batchReadUnblockables(lastRead, transactionBatchSize); if (!batch.isEmpty()) { lastRead = Optional.of(batch.get(batch.size() - 1)); changes.addAll(recheckStaleDomainsBatch(batch)); @@ -191,22 +198,33 @@ public final class DomainsRefresher { } public ImmutableList getNewUnblockables() { - ImmutableSet newCreated = getNewlyCreatedUnblockables(prevRefreshStartTime, now); - ImmutableSet newReserved = getNewlyReservedUnblockables(now, transactionBatchSize); - SetView reservedNotCreated = Sets.difference(newReserved, newCreated); - return Streams.concat( - newCreated.stream() - .map(name -> UnblockableDomain.of(name, Reason.REGISTERED)) - .map(UnblockableDomainChange::ofNew), - reservedNotCreated.stream() - .map(name -> UnblockableDomain.of(name, Reason.RESERVED)) - .map(UnblockableDomainChange::ofNew)) - .collect(toImmutableList()); + return bsaQuery( + () -> { + // TODO(weiminyu): both methods below use `queryBsaLabelByLabels`. Should combine. + ImmutableSet newCreated = getNewlyCreatedUnblockables(prevRefreshStartTime, now); + ImmutableSet newReserved = + getNewlyReservedUnblockables(now, transactionBatchSize); + SetView reservedNotCreated = Sets.difference(newReserved, newCreated); + return Streams.concat( + newCreated.stream() + .map(name -> UnblockableDomain.of(name, Reason.REGISTERED)) + .map(UnblockableDomainChange::ofNew), + reservedNotCreated.stream() + .map(name -> UnblockableDomain.of(name, Reason.RESERVED)) + .map(UnblockableDomainChange::ofNew)) + .collect(toImmutableList()); + }); } static ImmutableSet getNewlyCreatedUnblockables( DateTime prevRefreshStartTime, DateTime now) { - ImmutableSet liveDomains = queryLivesDomains(prevRefreshStartTime, now); + ImmutableSet bsaEnabledTlds = + getTldEntitiesOfType(TldType.REAL).stream() + .filter(tld -> isEnrolledWithBsa(tld, now)) + .map(Tld::getTldStr) + .collect(toImmutableSet()); + ImmutableSet liveDomains = + queryNewlyCreatedDomains(bsaEnabledTlds, prevRefreshStartTime, now); return getUnblockedDomainNames(liveDomains); } @@ -222,7 +240,7 @@ public final class DomainsRefresher { Map> labelToNames = domainNames.stream().collect(groupingBy(BsaStringUtils::getLabelInDomain)); ImmutableSet bsaLabels = - Queries.queryBsaLabelByLabels(ImmutableSet.copyOf(labelToNames.keySet())) + queryBsaLabelByLabels(ImmutableSet.copyOf(labelToNames.keySet())) .map(BsaLabel::getLabel) .collect(toImmutableSet()); return labelToNames.entrySet().stream() diff --git a/core/src/main/java/google/registry/bsa/persistence/Queries.java b/core/src/main/java/google/registry/bsa/persistence/Queries.java index dcc9c05a9..e92e764a7 100644 --- a/core/src/main/java/google/registry/bsa/persistence/Queries.java +++ b/core/src/main/java/google/registry/bsa/persistence/Queries.java @@ -16,14 +16,13 @@ package google.registry.bsa.persistence; import static com.google.common.base.Verify.verify; import static google.registry.bsa.BsaStringUtils.DOMAIN_SPLITTER; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static google.registry.bsa.BsaTransactions.bsaQuery; +import static google.registry.persistence.transaction.JpaTransactionManagerImpl.em; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import google.registry.model.CreateAutoTimestamp; -import google.registry.model.ForeignKeyUtils; -import google.registry.model.domain.Domain; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -36,15 +35,14 @@ class Queries { private Queries() {} private static Object detach(Object obj) { - tm().getEntityManager().detach(obj); + em().detach(obj); return obj; } static Stream queryBsaUnblockableDomainByLabels( ImmutableCollection labels) { return ((Stream) - tm().getEntityManager() - .createQuery("FROM BsaUnblockableDomain WHERE label in (:labels)") + em().createQuery("FROM BsaUnblockableDomain WHERE label in (:labels)") .setParameter("labels", labels) .getResultStream()) .map(Queries::detach) @@ -53,8 +51,7 @@ class Queries { static Stream queryBsaLabelByLabels(ImmutableCollection labels) { return ((Stream) - tm().getEntityManager() - .createQuery("FROM BsaLabel where label in (:labels)") + em().createQuery("FROM BsaLabel where label in (:labels)") .setParameter("labels", labels) .getResultStream()) .map(Queries::detach) @@ -62,8 +59,7 @@ class Queries { } static int deleteBsaLabelByLabels(ImmutableCollection labels) { - return tm().getEntityManager() - .createQuery("DELETE FROM BsaLabel where label IN (:deleted_labels)") + return em().createQuery("DELETE FROM BsaLabel where label IN (:deleted_labels)") .setParameter("deleted_labels", labels) .executeUpdate(); } @@ -71,14 +67,15 @@ class Queries { static ImmutableList batchReadUnblockables( Optional lastRead, int batchSize) { return ImmutableList.copyOf( - tm().getEntityManager() - .createQuery( - "FROM BsaUnblockableDomain d WHERE d.label > :label OR (d.label = :label AND d.tld" - + " > :tld) ORDER BY d.tld, d.label ") - .setParameter("label", lastRead.map(d -> d.label).orElse("")) - .setParameter("tld", lastRead.map(d -> d.tld).orElse("")) - .setMaxResults(batchSize) - .getResultList()); + bsaQuery( + () -> + em().createQuery( + "FROM BsaUnblockableDomain d WHERE d.label > :label OR (d.label = :label" + + " AND d.tld > :tld) ORDER BY d.tld, d.label ") + .setParameter("label", lastRead.map(d -> d.label).orElse("")) + .setParameter("tld", lastRead.map(d -> d.tld).orElse("")) + .setMaxResults(batchSize) + .getResultList())); } static ImmutableSet queryUnblockablesByNames(ImmutableSet domains) { @@ -96,17 +93,20 @@ class Queries { "SELECT CONCAT(d.label, '.', d.tld) FROM \"BsaUnblockableDomain\" d " + "WHERE (d.label, d.tld) IN (%s)", labelTldParis); - return ImmutableSet.copyOf(tm().getEntityManager().createNativeQuery(sql).getResultList()); + return ImmutableSet.copyOf(em().createNativeQuery(sql).getResultList()); } - static ImmutableSet queryLivesDomains(DateTime minCreationTime, DateTime now) { - ImmutableSet candidates = - ImmutableSet.copyOf( - tm().getEntityManager() - .createQuery( - "SELECT domainName FROM Domain WHERE creationTime >= :time ", String.class) - .setParameter("time", CreateAutoTimestamp.create(minCreationTime)) - .getResultList()); - return ImmutableSet.copyOf(ForeignKeyUtils.load(Domain.class, candidates, now).keySet()); + static ImmutableSet queryNewlyCreatedDomains( + ImmutableCollection tlds, DateTime minCreationTime, DateTime now) { + return ImmutableSet.copyOf( + em().createQuery( + "SELECT domainName FROM Domain WHERE creationTime >= :minCreationTime " + + "AND deletionTime > :now " + + "AND tld in (:tlds)", + String.class) + .setParameter("minCreationTime", CreateAutoTimestamp.create(minCreationTime)) + .setParameter("now", now) + .setParameter("tlds", tlds) + .getResultList()); } } diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index c107ba1f3..4e8f5af47 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -1472,9 +1472,9 @@ public final class RegistryConfig { } @Provides - @Config("domainTxnMaxDuration") - public static Duration provideDomainTxnMaxDuration(RegistryConfigSettings config) { - return Duration.standardSeconds(config.bsa.domainTxnMaxDurationSeconds); + @Config("domainCreateTxnCommitTimeLag") + public static Duration provideDomainCreateTxnCommitTimeLag(RegistryConfigSettings config) { + return Duration.standardSeconds(config.bsa.domainCreateTxnCommitTimeLagSeconds); } @Provides diff --git a/core/src/main/java/google/registry/config/RegistryConfigSettings.java b/core/src/main/java/google/registry/config/RegistryConfigSettings.java index 93e801a5f..eeca2d47b 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -273,7 +273,7 @@ public class RegistryConfigSettings { public int bsaDownloadIntervalMinutes; public int bsaMaxNopIntervalHours; public int bsaTxnBatchSize; - public int domainTxnMaxDurationSeconds; + public int domainCreateTxnCommitTimeLagSeconds; public String authUrl; public int authTokenExpirySeconds; public Map dataUrls; diff --git a/core/src/main/java/google/registry/config/files/default-config.yaml b/core/src/main/java/google/registry/config/files/default-config.yaml index 927a3ca47..c0d6310aa 100644 --- a/core/src/main/java/google/registry/config/files/default-config.yaml +++ b/core/src/main/java/google/registry/config/files/default-config.yaml @@ -624,9 +624,18 @@ bsa: # Max time period during which downloads can be skipped because checksums have # not changed from the previous one. bsaMaxNopIntervalHours: 24 - # A very lax upper bound of the time it takes to execute a transaction that - # mutates a domain. Please See `BsaRefreshAction` for use case. - domainTxnMaxDurationSeconds: 60 + # A very lax upper bound of the lag between a domain-creating transaction's + # recorded and actual commit time. In Nomulus, a domain's creation time is the + # start time of the transaction, while the domain is only visible after the + # transaction commits. Let `l` represents this lag, then at any point of time + # `t`, a query of domains by creation time is only guaranteed to find those + # created before `t - l`. Please See `BsaRefreshAction` for use case. + # + # The value below is decided by finding the longest domain-creation EPP + # request over the past 3 months (60 seconds, which is much longer than the + # transaction time), and add to it the maximum allowed replication lag (30 + # seconds). + domainCreateTxnCommitTimeLagSeconds: 90 # Number of entities (labels and unblockable domains) to process in a single # DB transaction. bsaTxnBatchSize: 1000 diff --git a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManager.java b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManager.java index 848112711..d393f3b79 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManager.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManager.java @@ -35,7 +35,13 @@ public interface JpaTransactionManager extends TransactionManager { * Returns the {@link EntityManager} for the current request. * *

The returned instance is closed when the current transaction completes. + * + * @deprecated Use the static {@link JpaTransactionManagerImpl#em} method for now. In current + * implementation the entity manager is obtained from a static {@code ThreadLocal} object that + * is set up by the outermost {@link #transact} call. As an instance method, this method gives + * the illusion that the call site has control over which database instance to use. */ + @Deprecated // See Javadoc above. EntityManager getEntityManager(); /** 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 b6d5b16e6..ce094bda3 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -91,6 +91,26 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { private static final ThreadLocal transactionInfo = ThreadLocal.withInitial(TransactionInfo::new); + /** Returns true if inside a transaction; returns false otherwise. */ + public static boolean isInTransaction() { + return transactionInfo.get().inTransaction; + } + + /** + * Returns the {@link EntityManager} for the current database transaction. + * + *

This method must be call from inside a transaction. + */ + public static EntityManager em() { + EntityManager entityManager = transactionInfo.get().entityManager; + if (entityManager == null) { + throw new PersistenceException( + "No EntityManager has been initialized. getEntityManager() must be invoked in the scope" + + " of a transaction"); + } + return entityManager; + } + public JpaTransactionManagerImpl(EntityManagerFactory emf, Clock clock, boolean readOnly) { this.emf = emf; this.clock = clock; @@ -111,6 +131,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { return emf.createEntityManager(); } + @Deprecated // See Javadoc of interface method. @Override public EntityManager getEntityManager() { EntityManager entityManager = transactionInfo.get().entityManager; @@ -137,6 +158,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { return getEntityManager().createQuery(sqlString); } + @Deprecated // See Javadoc of instance method. @Override public boolean inTransaction() { return transactionInfo.get().inTransaction; diff --git a/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java b/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java index 020b194fa..3c7b4b034 100644 --- a/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java +++ b/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java @@ -36,7 +36,14 @@ public interface TransactionManager { * *

Note that this function is kept for backward compatibility. We will review the use case * later when adding the cloud sql implementation. + * + * @deprecated Use the static {@link JpaTransactionManagerImpl#isInTransaction()} method for now. + * In current implementation the entity manager is obtained from a static {@code ThreadLocal} + * object that is set up by the outermost {@link #transact} call. As an instance method, this + * method gives the illusion that the call site has control over which database instance to + * use. */ + @Deprecated // See Javadoc above. boolean inTransaction(); /** diff --git a/core/src/test/java/google/registry/bsa/ReservedDomainsTestingUtils.java b/core/src/test/java/google/registry/bsa/ReservedDomainsTestingUtils.java new file mode 100644 index 000000000..720124761 --- /dev/null +++ b/core/src/test/java/google/registry/bsa/ReservedDomainsTestingUtils.java @@ -0,0 +1,65 @@ +// Copyright 2024 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.bsa; + +import static google.registry.testing.DatabaseHelper.persistResource; +import static google.registry.util.DateTimeUtils.START_OF_TIME; + +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 google.registry.model.tld.Tld; +import google.registry.model.tld.label.ReservationType; +import google.registry.model.tld.label.ReservedList; +import google.registry.model.tld.label.ReservedList.ReservedListEntry; +import google.registry.model.tld.label.ReservedListDao; + +/** Helpers for setting up reserved lists in tests. */ +public final class ReservedDomainsTestingUtils { + + private ReservedDomainsTestingUtils() {} + + public static void createReservedList( + String listName, ImmutableMap reservedLabels) { + ImmutableMap entries = + ImmutableMap.copyOf( + Maps.transformEntries( + reservedLabels, (key, value) -> ReservedListEntry.create(key, value, ""))); + + ReservedListDao.save( + new ReservedList.Builder() + .setName(listName) + .setCreationTimestamp(START_OF_TIME) + .setShouldPublish(true) + .setReservedListMap(entries) + .build()); + } + + public static void createReservedList( + String listName, String label, ReservationType reservationType) { + createReservedList(listName, ImmutableMap.of(label, reservationType)); + } + + public static void addReservedListsToTld(String tldStr, ImmutableList listNames) { + Tld tld = Tld.get(tldStr); + ImmutableSet reservedLists = + new ImmutableSet.Builder() + .addAll(tld.getReservedListNames()) + .addAll(listNames) + .build(); + persistResource(tld.asBuilder().setReservedListsByName(reservedLists).build()); + } +} diff --git a/core/src/test/java/google/registry/bsa/ReservedDomainsUtilsTest.java b/core/src/test/java/google/registry/bsa/ReservedDomainsUtilsTest.java index 03db03dbf..cff461c1a 100644 --- a/core/src/test/java/google/registry/bsa/ReservedDomainsUtilsTest.java +++ b/core/src/test/java/google/registry/bsa/ReservedDomainsUtilsTest.java @@ -15,6 +15,8 @@ package google.registry.bsa; import static com.google.common.truth.Truth.assertThat; +import static google.registry.bsa.ReservedDomainsTestingUtils.addReservedListsToTld; +import static google.registry.bsa.ReservedDomainsTestingUtils.createReservedList; import static google.registry.bsa.ReservedDomainsUtils.getAllReservedDomainsInTld; import static google.registry.model.tld.Tld.TldState.GENERAL_AVAILABILITY; import static google.registry.model.tld.Tld.TldState.START_DATE_SUNRISE; @@ -26,13 +28,10 @@ import static google.registry.model.tld.label.ReservationType.RESERVED_FOR_SPECI import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistResource; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import google.registry.model.tld.Tld; -import google.registry.model.tld.label.ReservedList; -import google.registry.model.tld.label.ReservedList.ReservedListEntry; -import google.registry.model.tld.label.ReservedListDao; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension; import google.registry.testing.FakeClock; @@ -51,41 +50,19 @@ class ReservedDomainsUtilsTest { @BeforeEach void setup() { - ImmutableMap byType = + createReservedList( + "testlist", ImmutableMap.of( - "sunrise", - ReservedListEntry.create("sunrise", ALLOWED_IN_SUNRISE, ""), - "specific", - ReservedListEntry.create("specific", RESERVED_FOR_SPECIFIC_USE, ""), - "anchor", - ReservedListEntry.create("anchor", RESERVED_FOR_ANCHOR_TENANT, ""), - "fully", - ReservedListEntry.create("fully", FULLY_BLOCKED, ""), - "name", - ReservedListEntry.create("name", NAME_COLLISION, "")); - - ImmutableMap altList = + "sunrise", ALLOWED_IN_SUNRISE, + "specific", RESERVED_FOR_SPECIFIC_USE, + "anchor", RESERVED_FOR_ANCHOR_TENANT, + "fully", FULLY_BLOCKED, + "name", NAME_COLLISION)); + createReservedList( + "testlist2", ImmutableMap.of( - "anchor", - ReservedListEntry.create("anchor", RESERVED_FOR_ANCHOR_TENANT, ""), - "somethingelse", - ReservedListEntry.create("somethingelse", RESERVED_FOR_ANCHOR_TENANT, "")); - - ReservedListDao.save( - new ReservedList.Builder() - .setName("testlist") - .setCreationTimestamp(fakeClock.nowUtc()) - .setShouldPublish(false) - .setReservedListMap(byType) - .build()); - - ReservedListDao.save( - new ReservedList.Builder() - .setName("testlist2") - .setCreationTimestamp(fakeClock.nowUtc()) - .setShouldPublish(false) - .setReservedListMap(altList) - .build()); + "anchor", RESERVED_FOR_ANCHOR_TENANT, + "somethingelse", RESERVED_FOR_ANCHOR_TENANT)); createTld("tld"); persistResource( @@ -95,15 +72,11 @@ class ReservedDomainsUtilsTest { ImmutableSortedMap.of( fakeClock.nowUtc(), START_DATE_SUNRISE, fakeClock.nowUtc().plusMillis(1), GENERAL_AVAILABILITY)) - .setReservedListsByName(ImmutableSet.of("testlist")) .build()); + addReservedListsToTld("tld", ImmutableList.of("testlist")); createTld("tld2"); - persistResource( - Tld.get("tld2") - .asBuilder() - .setReservedListsByName(ImmutableSet.of("testlist", "testlist2")) - .build()); + addReservedListsToTld("tld2", ImmutableList.of("testlist", "testlist2")); } @Test diff --git a/core/src/test/java/google/registry/bsa/persistence/BsaLabelUtilsTest.java b/core/src/test/java/google/registry/bsa/persistence/BsaLabelUtilsTest.java index d9d1fd7a6..74e5f3e6e 100644 --- a/core/src/test/java/google/registry/bsa/persistence/BsaLabelUtilsTest.java +++ b/core/src/test/java/google/registry/bsa/persistence/BsaLabelUtilsTest.java @@ -50,7 +50,7 @@ public class BsaLabelUtilsTest { @Test void isLabelBlocked_yes() { - persistBsaLabel("abc", fakeClock.nowUtc()); + persistBsaLabel("abc"); assertThat(isLabelBlocked("abc")).isTrue(); } diff --git a/core/src/test/java/google/registry/bsa/persistence/BsaTestingUtils.java b/core/src/test/java/google/registry/bsa/persistence/BsaTestingUtils.java index cd8cd14b4..7b5b0b7cf 100644 --- a/core/src/test/java/google/registry/bsa/persistence/BsaTestingUtils.java +++ b/core/src/test/java/google/registry/bsa/persistence/BsaTestingUtils.java @@ -26,10 +26,13 @@ public final class BsaTestingUtils { public static final Duration DEFAULT_DOWNLOAD_INTERVAL = Duration.standardHours(1); public static final Duration DEFAULT_NOP_INTERVAL = Duration.standardDays(1); + /** An arbitrary point of time used as BsaLabels' creation time. */ + public static final DateTime BSA_LABEL_CREATION_TIME = DateTime.parse("2023-12-31T00:00:00Z"); + private BsaTestingUtils() {} - public static void persistBsaLabel(String domainLabel, DateTime creationTime) { - tm().transact(() -> tm().put(new BsaLabel(domainLabel, creationTime))); + public static void persistBsaLabel(String domainLabel) { + tm().transact(() -> tm().put(new BsaLabel(domainLabel, BSA_LABEL_CREATION_TIME))); } public static DownloadScheduler createDownloadScheduler(Clock clock) { diff --git a/core/src/test/java/google/registry/bsa/persistence/DomainRefresherTest.java b/core/src/test/java/google/registry/bsa/persistence/DomainsRefresherTest.java similarity index 64% rename from core/src/test/java/google/registry/bsa/persistence/DomainRefresherTest.java rename to core/src/test/java/google/registry/bsa/persistence/DomainsRefresherTest.java index 00ec10679..cbf46c068 100644 --- a/core/src/test/java/google/registry/bsa/persistence/DomainRefresherTest.java +++ b/core/src/test/java/google/registry/bsa/persistence/DomainsRefresherTest.java @@ -15,7 +15,8 @@ package google.registry.bsa.persistence; import static com.google.common.truth.Truth.assertThat; -import static google.registry.bsa.BsaTransactions.bsaTransact; +import static google.registry.bsa.ReservedDomainsTestingUtils.addReservedListsToTld; +import static google.registry.bsa.ReservedDomainsTestingUtils.createReservedList; import static google.registry.bsa.persistence.BsaTestingUtils.persistBsaLabel; import static google.registry.model.tld.label.ReservationType.RESERVED_FOR_SPECIFIC_USE; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; @@ -24,15 +25,11 @@ import static google.registry.testing.DatabaseHelper.newDomain; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.util.DateTimeUtils.START_OF_TIME; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableList; import google.registry.bsa.api.UnblockableDomain; import google.registry.bsa.api.UnblockableDomainChange; import google.registry.bsa.persistence.BsaUnblockableDomain.Reason; import google.registry.model.tld.Tld; -import google.registry.model.tld.label.ReservedList; -import google.registry.model.tld.label.ReservedList.ReservedListEntry; -import google.registry.model.tld.label.ReservedListDao; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension; import google.registry.testing.FakeClock; @@ -44,7 +41,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; /** Unit tests for {@link DomainsRefresher}. */ -public class DomainRefresherTest { +public class DomainsRefresherTest { FakeClock fakeClock = new FakeClock(DateTime.parse("2023-11-09T02:08:57.880Z")); @@ -66,52 +63,54 @@ public class DomainRefresherTest { } @Test - void staleUnblockableRemoved_wasRegistered() { - persistBsaLabel("label", fakeClock.nowUtc().minus(Duration.standardDays(1))); + void registeredUnblockable_removed_afterDomainIsDeleted() { + persistBsaLabel("label"); tm().transact(() -> tm().insert(BsaUnblockableDomain.of("label.tld", Reason.REGISTERED))); - assertThat(bsaTransact(refresher::refreshStaleUnblockables)) + assertThat(refresher.refreshStaleUnblockables()) .containsExactly( UnblockableDomainChange.ofDeleted( UnblockableDomain.of("label.tld", UnblockableDomain.Reason.REGISTERED))); } @Test - void staleUnblockableRemoved_wasReserved() { - persistBsaLabel("label", fakeClock.nowUtc().minus(Duration.standardDays(1))); + void reservedUnblockable_removed_whenReservedLabelIsRemoved() { + persistBsaLabel("label"); tm().transact(() -> tm().insert(BsaUnblockableDomain.of("label.tld", Reason.RESERVED))); - assertThat(bsaTransact(refresher::refreshStaleUnblockables)) + assertThat(refresher.refreshStaleUnblockables()) .containsExactly( UnblockableDomainChange.ofDeleted( UnblockableDomain.of("label.tld", UnblockableDomain.Reason.RESERVED))); } @Test - void newUnblockableAdded_isRegistered() { + void regsiteredUnblockable_added_whenDomainIsAdded() { persistResource(newDomain("label.tld")); - persistBsaLabel("label", fakeClock.nowUtc().minus(Duration.standardDays(1))); - assertThat(bsaTransact(refresher::getNewUnblockables)) + persistBsaLabel("label"); + assertThat(refresher.getNewUnblockables()) .containsExactly( UnblockableDomainChange.ofNew( UnblockableDomain.of("label.tld", UnblockableDomain.Reason.REGISTERED))); } @Test - void newUnblockableAdded_isReserved() { - persistBsaLabel("label", fakeClock.nowUtc().minus(Duration.standardDays(1))); - setReservedList("label"); - assertThat(bsaTransact(refresher::getNewUnblockables)) + void reservedUnblockable_added_whenReservedLabelIsAdded() { + persistBsaLabel("label"); + createReservedList("reservedList", "label", RESERVED_FOR_SPECIFIC_USE); + addReservedListsToTld("tld", ImmutableList.of("reservedList")); + assertThat(refresher.getNewUnblockables()) .containsExactly( UnblockableDomainChange.ofNew( UnblockableDomain.of("label.tld", UnblockableDomain.Reason.RESERVED))); } @Test - void staleUnblockableDowngraded_registeredToReserved() { - persistBsaLabel("label", fakeClock.nowUtc().minus(Duration.standardDays(1))); - setReservedList("label"); + void registeredUnblockable_changedToReserved_whenDomainIsDeletedButLabelIsReserved() { + persistBsaLabel("label"); + createReservedList("reservedList", "label", RESERVED_FOR_SPECIFIC_USE); + addReservedListsToTld("tld", ImmutableList.of("reservedList")); tm().transact(() -> tm().insert(BsaUnblockableDomain.of("label.tld", Reason.REGISTERED))); - assertThat(bsaTransact(refresher::refreshStaleUnblockables)) + assertThat(refresher.refreshStaleUnblockables()) .containsExactly( UnblockableDomainChange.ofChanged( UnblockableDomain.of("label.tld", UnblockableDomain.Reason.REGISTERED), @@ -119,12 +118,12 @@ public class DomainRefresherTest { } @Test - void staleUnblockableUpgraded_reservedToRegisteredButNotReserved() { - persistBsaLabel("label", fakeClock.nowUtc().minus(Duration.standardDays(1))); + void reservedUnblockableUpgraded_changedToRegistered_whenDomainIsCreatedButNoLongerReserved() { + persistBsaLabel("label"); tm().transact(() -> tm().insert(BsaUnblockableDomain.of("label.tld", Reason.RESERVED))); persistResource(newDomain("label.tld")); - assertThat(bsaTransact(refresher::refreshStaleUnblockables)) + assertThat(refresher.refreshStaleUnblockables()) .containsExactly( UnblockableDomainChange.ofChanged( UnblockableDomain.of("label.tld", UnblockableDomain.Reason.RESERVED), @@ -132,31 +131,17 @@ public class DomainRefresherTest { } @Test - void staleUnblockableUpgraded_wasReserved_isReservedAndRegistered() { - persistBsaLabel("label", fakeClock.nowUtc().minus(Duration.standardDays(1))); - setReservedList("label"); + void reservedUnblockableUpgraded_changedToRegistered_whenDomainIsCreatedAndStillReserved() { + persistBsaLabel("label"); + createReservedList("reservedList", "label", RESERVED_FOR_SPECIFIC_USE); + addReservedListsToTld("tld", ImmutableList.of("reservedList")); tm().transact(() -> tm().insert(BsaUnblockableDomain.of("label.tld", Reason.RESERVED))); persistResource(newDomain("label.tld")); - assertThat(bsaTransact(refresher::refreshStaleUnblockables)) + assertThat(refresher.refreshStaleUnblockables()) .containsExactly( UnblockableDomainChange.ofChanged( UnblockableDomain.of("label.tld", UnblockableDomain.Reason.RESERVED), UnblockableDomain.Reason.REGISTERED)); } - - private void setReservedList(String label) { - ImmutableMap reservedNameMap = - ImmutableMap.of(label, ReservedListEntry.create(label, RESERVED_FOR_SPECIFIC_USE, "")); - - ReservedListDao.save( - new ReservedList.Builder() - .setName("testlist") - .setCreationTimestamp(fakeClock.nowUtc()) - .setShouldPublish(false) - .setReservedListMap(reservedNameMap) - .build()); - persistResource( - Tld.get("tld").asBuilder().setReservedListsByName(ImmutableSet.of("testlist")).build()); - } } diff --git a/core/src/test/java/google/registry/bsa/persistence/QueriesTest.java b/core/src/test/java/google/registry/bsa/persistence/QueriesTest.java index 4d7ae8243..b94d568cf 100644 --- a/core/src/test/java/google/registry/bsa/persistence/QueriesTest.java +++ b/core/src/test/java/google/registry/bsa/persistence/QueriesTest.java @@ -16,10 +16,11 @@ package google.registry.bsa.persistence; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.truth.Truth.assertThat; +import static google.registry.bsa.BsaTransactions.bsaQuery; import static google.registry.bsa.persistence.Queries.deleteBsaLabelByLabels; import static google.registry.bsa.persistence.Queries.queryBsaLabelByLabels; import static google.registry.bsa.persistence.Queries.queryBsaUnblockableDomainByLabels; -import static google.registry.bsa.persistence.Queries.queryLivesDomains; +import static google.registry.bsa.persistence.Queries.queryNewlyCreatedDomains; import static google.registry.bsa.persistence.Queries.queryUnblockablesByNames; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTlds; @@ -182,7 +183,7 @@ class QueriesTest { } @Test - void queryLivesDomains_onlyLiveDomainsReturned() { + void queryNewlyCreatedDomains_onlyLiveDomainsReturned() { DateTime testStartTime = fakeClock.nowUtc(); createTlds("tld"); persistNewRegistrar("TheRegistrar"); @@ -199,7 +200,29 @@ class QueriesTest { newDomain("d2.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build()); fakeClock.advanceOneMilli(); // Now is time 2 - assertThat(tm().transact(() -> queryLivesDomains(testStartTime, fakeClock.nowUtc()))) + assertThat( + bsaQuery( + () -> + queryNewlyCreatedDomains( + ImmutableList.of("tld"), testStartTime, fakeClock.nowUtc()))) .containsExactly("d1.tld", "d2.tld"); } + + @Test + void queryNewlyCreatedDomains_onlyDomainsInRequestedTldsReturned() { + DateTime testStartTime = fakeClock.nowUtc(); + createTlds("tld", "tld2"); + persistNewRegistrar("TheRegistrar"); + persistResource( + newDomain("d1.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build()); + persistResource( + newDomain("d2.tld2").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build()); + fakeClock.advanceOneMilli(); + assertThat( + bsaQuery( + () -> + queryNewlyCreatedDomains( + ImmutableList.of("tld"), testStartTime, fakeClock.nowUtc()))) + .containsExactly("d1.tld"); + } } diff --git a/core/src/test/java/google/registry/flows/CheckApiActionTest.java b/core/src/test/java/google/registry/flows/CheckApiActionTest.java index a773e6d8c..77991cb85 100644 --- a/core/src/test/java/google/registry/flows/CheckApiActionTest.java +++ b/core/src/test/java/google/registry/flows/CheckApiActionTest.java @@ -288,7 +288,7 @@ class CheckApiActionTest { @Test void testSuccess_blockedByBsa() { - BsaTestingUtils.persistBsaLabel("rich", START_OF_TIME); + BsaTestingUtils.persistBsaLabel("rich"); persistResource( Tld.get("example").asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build()); assertThat(getCheckResponse("rich.example")) diff --git a/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java index b05b5642f..bb87d89ea 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java @@ -14,6 +14,7 @@ package google.registry.flows.domain; +import static google.registry.bsa.persistence.BsaTestingUtils.persistBsaLabel; import static google.registry.model.billing.BillingBase.RenewalPriceBehavior.DEFAULT; import static google.registry.model.billing.BillingBase.RenewalPriceBehavior.NONPREMIUM; import static google.registry.model.billing.BillingBase.RenewalPriceBehavior.SPECIFIED; @@ -44,7 +45,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Ordering; -import google.registry.bsa.persistence.BsaTestingUtils; import google.registry.flows.EppException; import google.registry.flows.FlowUtils.NotLoggedInException; import google.registry.flows.FlowUtils.UnknownCurrencyEppException; @@ -160,7 +160,7 @@ class DomainCheckFlowTest extends ResourceCheckFlowTestCase