From f61579b35029b9918f73638f6c41250a3c2eff72 Mon Sep 17 00:00:00 2001 From: Weimin Yu Date: Mon, 22 Jan 2024 12:23:29 -0500 Subject: [PATCH] Fix BsaRefreshAction bugs (#2294) * Fix BsaRefreshAction bugs Added functional tests for BsaRefreshAction, which checks for changes in domain registration and reservation, and apply them to the Unblockable domain list. Fixed a few bugs exposed by the tests. Also refactored a few other tests. --- .../google/registry/bsa/BsaRefreshAction.java | 22 +- .../google/registry/bsa/BsaStringUtils.java | 2 +- .../google/registry/bsa/BsaTransactions.java | 6 + .../java/google/registry/bsa/GcsClient.java | 2 +- .../registry/bsa/api/UnblockableDomain.java | 4 +- .../bsa/api/UnblockableDomainChange.java | 8 +- .../bsa/persistence/BsaUnblockableDomain.java | 4 + .../bsa/persistence/DomainsRefresher.java | 105 ++++-- .../bsa/BsaDownloadFunctionalTest.java | 9 +- .../bsa/BsaRefreshFunctionalTest.java | 319 ++++++++++++++++++ .../bsa/ReservedDomainsTestingUtils.java | 39 +++ .../bsa/persistence/BsaTestingUtils.java | 17 + .../bsa/persistence/LabelDiffUpdatesTest.java | 20 +- .../registry/bsa/persistence/QueriesTest.java | 26 ++ 14 files changed, 509 insertions(+), 74 deletions(-) create mode 100644 core/src/test/java/google/registry/bsa/BsaRefreshFunctionalTest.java diff --git a/core/src/main/java/google/registry/bsa/BsaRefreshAction.java b/core/src/main/java/google/registry/bsa/BsaRefreshAction.java index 8fdb17536..17ae90b89 100644 --- a/core/src/main/java/google/registry/bsa/BsaRefreshAction.java +++ b/core/src/main/java/google/registry/bsa/BsaRefreshAction.java @@ -54,7 +54,7 @@ public class BsaRefreshAction implements Runnable { private final GcsClient gcsClient; private final BsaReportSender bsaReportSender; private final int transactionBatchSize; - private final Duration domainTxnMaxDuration; + private final Duration domainCreateTxnCommitTimeLag; private final BsaLock bsaLock; private final Clock clock; private final Response response; @@ -65,7 +65,7 @@ public class BsaRefreshAction implements Runnable { GcsClient gcsClient, BsaReportSender bsaReportSender, @Config("bsaTxnBatchSize") int transactionBatchSize, - @Config("domainCreateTxnCommitTimeLag") Duration domainTxnMaxDuration, + @Config("domainCreateTxnCommitTimeLag") Duration domainCreateTxnCommitTimeLag, BsaLock bsaLock, Clock clock, Response response) { @@ -73,7 +73,7 @@ public class BsaRefreshAction implements Runnable { this.gcsClient = gcsClient; this.bsaReportSender = bsaReportSender; this.transactionBatchSize = transactionBatchSize; - this.domainTxnMaxDuration = domainTxnMaxDuration; + this.domainCreateTxnCommitTimeLag = domainCreateTxnCommitTimeLag; this.bsaLock = bsaLock; this.clock = clock; this.response = response; @@ -109,17 +109,21 @@ public class BsaRefreshAction implements Runnable { RefreshSchedule schedule = maybeSchedule.get(); DomainsRefresher refresher = new DomainsRefresher( - schedule.prevRefreshTime(), clock.nowUtc(), domainTxnMaxDuration, transactionBatchSize); + schedule.prevRefreshTime(), + clock.nowUtc(), + domainCreateTxnCommitTimeLag, + transactionBatchSize); switch (schedule.stage()) { case CHECK_FOR_CHANGES: ImmutableList blockabilityChanges = refresher.checkForBlockabilityChanges(); + // Always write even if no change. Easier for manual inspection of GCS bucket. + gcsClient.writeRefreshChanges(schedule.jobName(), blockabilityChanges.stream()); if (blockabilityChanges.isEmpty()) { logger.atInfo().log("No change to Unblockable domains found."); schedule.updateJobStage(RefreshStage.DONE); return null; } - gcsClient.writeRefreshChanges(schedule.jobName(), blockabilityChanges.stream()); schedule.updateJobStage(RefreshStage.APPLY_CHANGES); // Fall through case APPLY_CHANGES: @@ -132,10 +136,12 @@ public class BsaRefreshAction implements Runnable { case UPLOAD_REMOVALS: try (Stream changes = gcsClient.readRefreshChanges(schedule.jobName())) { + // Unblockables with changes in REASON are removed then added back. That is why they are + // included in this stage. Optional report = JsonSerializations.toUnblockableDomainsRemovalReport( changes - .filter(UnblockableDomainChange::isDelete) + .filter(UnblockableDomainChange::isChangeOrDelete) .map(UnblockableDomainChange::domainName)); if (report.isPresent()) { gcsClient.logRemovedUnblockableDomainsReport( @@ -153,12 +159,12 @@ public class BsaRefreshAction implements Runnable { Optional report = JsonSerializations.toUnblockableDomainsReport( changes - .filter(UnblockableDomainChange::isAddOrChange) + .filter(UnblockableDomainChange::isNewOrChange) .map(UnblockableDomainChange::newValue)); if (report.isPresent()) { gcsClient.logRemovedUnblockableDomainsReport( schedule.jobName(), LINE_SPLITTER.splitToStream(report.get())); - bsaReportSender.removeUnblockableDomainsUpdates(report.get()); + bsaReportSender.addUnblockableDomainsUpdates(report.get()); } else { logger.atInfo().log("No new Unblockable domains to add."); } diff --git a/core/src/main/java/google/registry/bsa/BsaStringUtils.java b/core/src/main/java/google/registry/bsa/BsaStringUtils.java index 5eccf4d53..c9d9bf2ee 100644 --- a/core/src/main/java/google/registry/bsa/BsaStringUtils.java +++ b/core/src/main/java/google/registry/bsa/BsaStringUtils.java @@ -30,7 +30,7 @@ public class BsaStringUtils { public static final Splitter LINE_SPLITTER = Splitter.on('\n'); public static String getLabelInDomain(String domainName) { - List parts = DOMAIN_SPLITTER.limit(1).splitToList(domainName); + List parts = DOMAIN_SPLITTER.splitToList(domainName); checkArgument(!parts.isEmpty(), "Not a valid domain: [%s]", domainName); return parts.get(0); } diff --git a/core/src/main/java/google/registry/bsa/BsaTransactions.java b/core/src/main/java/google/registry/bsa/BsaTransactions.java index fd0044ff3..84d6e5b18 100644 --- a/core/src/main/java/google/registry/bsa/BsaTransactions.java +++ b/core/src/main/java/google/registry/bsa/BsaTransactions.java @@ -21,6 +21,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.errorprone.annotations.CanIgnoreReturnValue; +import google.registry.persistence.transaction.TransactionManager.ThrowingRunnable; import java.util.concurrent.Callable; /** @@ -39,6 +40,11 @@ public final class BsaTransactions { return tm().transact(work, TRANSACTION_REPEATABLE_READ); } + public static void bsaTransact(ThrowingRunnable work) { + verify(!isInTransaction(), "May only be used for top-level transactions."); + tm().transact(work, TRANSACTION_REPEATABLE_READ); + } + @CanIgnoreReturnValue public static T bsaQuery(Callable work) { verify(!isInTransaction(), "May only be used for top-level transactions."); diff --git a/core/src/main/java/google/registry/bsa/GcsClient.java b/core/src/main/java/google/registry/bsa/GcsClient.java index 9ef8da07a..c257cae07 100644 --- a/core/src/main/java/google/registry/bsa/GcsClient.java +++ b/core/src/main/java/google/registry/bsa/GcsClient.java @@ -161,7 +161,7 @@ public class GcsClient { } Stream readRefreshChanges(String jobName) { - BlobId blobId = getBlobId(jobName, UNBLOCKABLE_DOMAINS_FILE); + BlobId blobId = getBlobId(jobName, REFRESHED_UNBLOCKABLE_DOMAINS_FILE); return readStream(blobId).map(UnblockableDomainChange::deserialize); } diff --git a/core/src/main/java/google/registry/bsa/api/UnblockableDomain.java b/core/src/main/java/google/registry/bsa/api/UnblockableDomain.java index e16b3205d..bf141a59f 100644 --- a/core/src/main/java/google/registry/bsa/api/UnblockableDomain.java +++ b/core/src/main/java/google/registry/bsa/api/UnblockableDomain.java @@ -28,9 +28,9 @@ import java.util.List; // TODO(1/15/2024): rename to UnblockableDomain. @AutoValue public abstract class UnblockableDomain { - abstract String domainName(); + public abstract String domainName(); - abstract Reason reason(); + public abstract Reason reason(); /** Reasons why a valid domain name cannot be blocked. */ public enum Reason { diff --git a/core/src/main/java/google/registry/bsa/api/UnblockableDomainChange.java b/core/src/main/java/google/registry/bsa/api/UnblockableDomainChange.java index 61c519cd9..5e36082a0 100644 --- a/core/src/main/java/google/registry/bsa/api/UnblockableDomainChange.java +++ b/core/src/main/java/google/registry/bsa/api/UnblockableDomainChange.java @@ -52,12 +52,16 @@ public abstract class UnblockableDomainChange { return UnblockableDomain.of(unblockable().domainName(), newReason().get()); } - public boolean isAddOrChange() { + public boolean isNewOrChange() { return newReason().isPresent(); } + public boolean isChangeOrDelete() { + return !isNew(); + } + public boolean isDelete() { - return !this.isAddOrChange(); + return !this.isNewOrChange(); } public boolean isNew() { diff --git a/core/src/main/java/google/registry/bsa/persistence/BsaUnblockableDomain.java b/core/src/main/java/google/registry/bsa/persistence/BsaUnblockableDomain.java index 3e00c0cd7..bfaa90e18 100644 --- a/core/src/main/java/google/registry/bsa/persistence/BsaUnblockableDomain.java +++ b/core/src/main/java/google/registry/bsa/persistence/BsaUnblockableDomain.java @@ -105,6 +105,10 @@ class BsaUnblockableDomain { return new BsaUnblockableDomain(parts.get(0), parts.get(1), reason); } + static BsaUnblockableDomain of(UnblockableDomain unblockable) { + return of(unblockable.domainName(), Reason.valueOf(unblockable.reason().name())); + } + static VKey vKey(String domainName) { ImmutableList parts = ImmutableList.copyOf(DOMAIN_SPLITTER.splitToList(domainName)); verify(parts.size() == 2, "Invalid domain name: %s", domainName); 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 81323734b..7da95e220 100644 --- a/core/src/main/java/google/registry/bsa/persistence/DomainsRefresher.java +++ b/core/src/main/java/google/registry/bsa/persistence/DomainsRefresher.java @@ -18,6 +18,7 @@ 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.BsaTransactions.bsaTransact; import static google.registry.bsa.ReservedDomainsUtils.getAllReservedNames; import static google.registry.bsa.ReservedDomainsUtils.isReservedDomain; import static google.registry.bsa.persistence.Queries.batchReadUnblockables; @@ -44,6 +45,7 @@ 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.HashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -107,6 +109,8 @@ public final class DomainsRefresher { ImmutableSet upgradedDomains = upgrades.stream().map(UnblockableDomainChange::domainName).collect(toImmutableSet()); + // Upgrades are collected in its own txn after downgrades so need reconciliation. Conflicts are + // unlikely but possible: domains may be recreated or reserved names added during the interval. ImmutableList trueDowngrades = downgrades.stream() .filter(c -> !upgradedDomains.contains(c.domainName())) @@ -198,22 +202,50 @@ public final class DomainsRefresher { } public ImmutableList getNewUnblockables() { - 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()); - }); + // TODO(weiminyu): both methods below use `queryBsaLabelByLabels`. Should combine in a single + // query. + HashSet newCreated = + Sets.newHashSet(bsaQuery(() -> getNewlyCreatedUnblockables(prevRefreshStartTime, now))); + // We cannot identify new reserved unblockables so must look at all of them. There are not many + // of these. + HashSet allReserved = + Sets.newHashSet(bsaQuery(() -> getAllReservedUnblockables(now, transactionBatchSize))); + HashSet reservedNotCreated = Sets.newHashSet(Sets.difference(allReserved, newCreated)); + + ImmutableList.Builder changes = new ImmutableList.Builder<>(); + ImmutableList batch; + Optional lastRead = Optional.empty(); + do { + batch = batchReadUnblockables(lastRead, transactionBatchSize); + if (batch.isEmpty()) { + break; + } + lastRead = Optional.of(batch.get(batch.size() - 1)); + for (BsaUnblockableDomain unblockable : batch) { + String domainName = unblockable.domainName(); + if (unblockable.reason.equals(BsaUnblockableDomain.Reason.REGISTERED)) { + newCreated.remove(domainName); + reservedNotCreated.remove(domainName); + } else { + reservedNotCreated.remove(domainName); + if (newCreated.remove(domainName)) { + changes.add( + UnblockableDomainChange.ofChanged( + unblockable.toUnblockableDomain(), Reason.REGISTERED)); + } + } + } + } while (batch.size() == transactionBatchSize); + + 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)) + .forEach(changes::add); + return changes.build(); } static ImmutableSet getNewlyCreatedUnblockables( @@ -225,18 +257,18 @@ public final class DomainsRefresher { .collect(toImmutableSet()); ImmutableSet liveDomains = queryNewlyCreatedDomains(bsaEnabledTlds, prevRefreshStartTime, now); - return getUnblockedDomainNames(liveDomains); + return getBlockedDomainNames(liveDomains); } - static ImmutableSet getNewlyReservedUnblockables(DateTime now, int batchSize) { + static ImmutableSet getAllReservedUnblockables(DateTime now, int batchSize) { Stream allReserved = getAllReservedNames(now); return BatchedStreams.toBatches(allReserved, batchSize) - .map(DomainsRefresher::getUnblockedDomainNames) + .map(DomainsRefresher::getBlockedDomainNames) .flatMap(ImmutableSet::stream) .collect(toImmutableSet()); } - static ImmutableSet getUnblockedDomainNames(ImmutableCollection domainNames) { + static ImmutableSet getBlockedDomainNames(ImmutableCollection domainNames) { Map> labelToNames = domainNames.stream().collect(groupingBy(BsaStringUtils::getLabelInDomain)); ImmutableSet bsaLabels = @@ -244,7 +276,7 @@ public final class DomainsRefresher { .map(BsaLabel::getLabel) .collect(toImmutableSet()); return labelToNames.entrySet().stream() - .filter(entry -> !bsaLabels.contains(entry.getKey())) + .filter(entry -> bsaLabels.contains(entry.getKey())) .map(Entry::getValue) .flatMap(List::stream) .collect(toImmutableSet()); @@ -257,20 +289,21 @@ public final class DomainsRefresher { .collect( groupingBy( change -> change.isDelete() ? "remove" : "change", toImmutableSet()))); - tm().transact( - () -> { - if (changesByType.containsKey("remove")) { - tm().delete( - changesByType.get("remove").stream() - .map(c -> BsaUnblockableDomain.vKey(c.domainName())) - .collect(toImmutableSet())); - } - if (changesByType.containsKey("change")) { - tm().putAll( - changesByType.get("change").stream() - .map(UnblockableDomainChange::newValue) - .collect(toImmutableSet())); - } - }); + bsaTransact( + () -> { + if (changesByType.containsKey("remove")) { + tm().delete( + changesByType.get("remove").stream() + .map(c -> BsaUnblockableDomain.vKey(c.domainName())) + .collect(toImmutableSet())); + } + if (changesByType.containsKey("change")) { + tm().putAll( + changesByType.get("change").stream() + .map(UnblockableDomainChange::newValue) + .map(BsaUnblockableDomain::of) + .collect(toImmutableSet())); + } + }); } } diff --git a/core/src/test/java/google/registry/bsa/BsaDownloadFunctionalTest.java b/core/src/test/java/google/registry/bsa/BsaDownloadFunctionalTest.java index 80664ede7..e58e6d468 100644 --- a/core/src/test/java/google/registry/bsa/BsaDownloadFunctionalTest.java +++ b/core/src/test/java/google/registry/bsa/BsaDownloadFunctionalTest.java @@ -118,15 +118,8 @@ class BsaDownloadFunctionalTest { gcsClient.readBlockList(downloadJob, BlockListType.BLOCK_PLUS)) { assertThat(blockListFile).containsExactly(BSA_CSV_HEADER, "abc,2", "def,3"); } - ImmutableList persistedLabels = - ImmutableList.copyOf( - tm().transact( - () -> - tm().getEntityManager() - .createNativeQuery("SELECT label from \"BsaLabel\"") - .getResultList())); // TODO(weiminyu): check intermediate files - assertThat(persistedLabels).containsExactly("abc", "def"); + assertThat(getPersistedLabels()).containsExactly("abc", "def"); } @Test diff --git a/core/src/test/java/google/registry/bsa/BsaRefreshFunctionalTest.java b/core/src/test/java/google/registry/bsa/BsaRefreshFunctionalTest.java new file mode 100644 index 000000000..401f2a26c --- /dev/null +++ b/core/src/test/java/google/registry/bsa/BsaRefreshFunctionalTest.java @@ -0,0 +1,319 @@ +// Copyright 2023 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 com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; +import static google.registry.bsa.ReservedDomainsTestingUtils.addReservedDomainToList; +import static google.registry.bsa.ReservedDomainsTestingUtils.addReservedListsToTld; +import static google.registry.bsa.ReservedDomainsTestingUtils.createReservedList; +import static google.registry.bsa.ReservedDomainsTestingUtils.removeReservedDomainFromList; +import static google.registry.bsa.persistence.BsaTestingUtils.createDownloadScheduler; +import static google.registry.bsa.persistence.BsaTestingUtils.persistBsaLabel; +import static google.registry.bsa.persistence.BsaTestingUtils.queryUnblockableDomains; +import static google.registry.model.tld.Tlds.getTldEntitiesOfType; +import static google.registry.model.tld.label.ReservationType.RESERVED_FOR_SPECIFIC_USE; +import static google.registry.testing.DatabaseHelper.createTlds; +import static google.registry.testing.DatabaseHelper.deleteTestDomain; +import static google.registry.testing.DatabaseHelper.persistActiveDomain; +import static google.registry.testing.DatabaseHelper.persistResource; +import static google.registry.util.DateTimeUtils.START_OF_TIME; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; + +import com.google.cloud.storage.contrib.nio.testing.LocalStorageHelper; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import google.registry.bsa.api.BsaReportSender; +import google.registry.bsa.api.UnblockableDomain; +import google.registry.bsa.api.UnblockableDomain.Reason; +import google.registry.bsa.api.UnblockableDomainChange; +import google.registry.bsa.persistence.BsaTestingUtils; +import google.registry.gcs.GcsUtils; +import google.registry.model.domain.Domain; +import google.registry.model.tld.Tld.TldType; +import google.registry.persistence.transaction.JpaTestExtensions; +import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension; +import google.registry.request.Response; +import google.registry.testing.FakeClock; +import google.registry.testing.FakeLockHandler; +import google.registry.testing.FakeResponse; +import java.util.Optional; +import org.joda.time.DateTime; +import org.joda.time.Duration; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.mockito.InOrder; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +/** + * Functional tests for refreshing the unblockable domains with recent registration and reservation + * changes. + */ +@ExtendWith(MockitoExtension.class) +class BsaRefreshFunctionalTest { + + static final DateTime TEST_START_TIME = DateTime.parse("2024-01-01T00:00:00Z"); + + static final String RESERVED_LIST_NAME = "reserved"; + + private final FakeClock fakeClock = new FakeClock(TEST_START_TIME); + + @RegisterExtension + JpaIntegrationWithCoverageExtension jpa = + new JpaTestExtensions.Builder().withClock(fakeClock).buildIntegrationWithCoverageExtension(); + + @Mock BsaReportSender bsaReportSender; + + private GcsClient gcsClient; + private Response response; + private BsaRefreshAction action; + + @BeforeEach + void setup() throws Exception { + gcsClient = + new GcsClient(new GcsUtils(LocalStorageHelper.getOptions()), "my-bucket", "SHA-256"); + response = new FakeResponse(); + action = + new BsaRefreshAction( + BsaTestingUtils.createRefreshScheduler(), + gcsClient, + bsaReportSender, + /* transactionBatchSize= */ 5, + /* domainCreateTxnCommitTimeLag= */ Duration.millis(1), + new BsaLock( + new FakeLockHandler(/* lockSucceeds= */ true), Duration.standardSeconds(30)), + fakeClock, + response); + + initDb(); + } + + private String getRefreshJobName(DateTime jobStartTime) { + return jobStartTime.toString() + "-refresh"; + } + + private void initDb() { + createTlds("app", "dev"); + getTldEntitiesOfType(TldType.REAL) + .forEach( + tld -> + persistResource( + tld.asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build())); + + createReservedList(RESERVED_LIST_NAME, "dummy", RESERVED_FOR_SPECIFIC_USE); + addReservedListsToTld("app", ImmutableList.of(RESERVED_LIST_NAME)); + + persistBsaLabel("blocked1"); + persistBsaLabel("blocked2"); + // Creates a download record so that refresher will not quit immediately. + createDownloadScheduler(fakeClock).schedule().get().updateJobStage(DownloadStage.DONE); + fakeClock.advanceOneMilli(); + } + + @Test + void newReservedDomain_addedAsUnblockable() throws Exception { + addReservedDomainToList( + RESERVED_LIST_NAME, ImmutableMap.of("blocked1", RESERVED_FOR_SPECIFIC_USE)); + String jobName = getRefreshJobName(fakeClock.nowUtc()); + action.run(); + UnblockableDomain newUnblockable = UnblockableDomain.of("blocked1.app", Reason.RESERVED); + assertThat(queryUnblockableDomains()).containsExactly(newUnblockable); + assertThat(gcsClient.readRefreshChanges(jobName)) + .containsExactly(UnblockableDomainChange.ofNew(newUnblockable)); + verify(bsaReportSender, never()).removeUnblockableDomainsUpdates(anyString()); + verify(bsaReportSender, times(1)) + .addUnblockableDomainsUpdates("{\n \"reserved\": [\n \"blocked1.app\"\n ]\n}"); + } + + @Test + void newRegisteredDomain_addedAsUnblockable() throws Exception { + persistActiveDomain("blocked1.dev", fakeClock.nowUtc()); + persistActiveDomain("dummy.dev", fakeClock.nowUtc()); + String jobName = getRefreshJobName(fakeClock.nowUtc()); + action.run(); + UnblockableDomain newUnblockable = UnblockableDomain.of("blocked1.dev", Reason.REGISTERED); + assertThat(queryUnblockableDomains()).containsExactly(newUnblockable); + assertThat(gcsClient.readRefreshChanges(jobName)) + .containsExactly(UnblockableDomainChange.ofNew(newUnblockable)); + + verify(bsaReportSender, never()).removeUnblockableDomainsUpdates(anyString()); + verify(bsaReportSender, times(1)) + .addUnblockableDomainsUpdates("{\n \"registered\": [\n \"blocked1.dev\"\n ]\n}"); + } + + @Test + void registeredUnblockable_unregistered() { + Domain domain = persistActiveDomain("blocked1.dev", fakeClock.nowUtc()); + action.run(); + assertThat(queryUnblockableDomains()) + .containsExactly(UnblockableDomain.of("blocked1.dev", Reason.REGISTERED)); + fakeClock.advanceOneMilli(); + deleteTestDomain(domain, fakeClock.nowUtc()); + fakeClock.advanceOneMilli(); + + String jobName = getRefreshJobName(fakeClock.nowUtc()); + Mockito.reset(bsaReportSender); + action.run(); + assertThat(queryUnblockableDomains()).isEmpty(); + assertThat(gcsClient.readRefreshChanges(jobName)) + .containsExactly( + UnblockableDomainChange.ofDeleted( + UnblockableDomain.of("blocked1.dev", Reason.REGISTERED))); + + verify(bsaReportSender, never()).addUnblockableDomainsUpdates(anyString()); + verify(bsaReportSender, times(1)).removeUnblockableDomainsUpdates("[\n \"blocked1.dev\"\n]"); + } + + @Test + void reservedUnblockable_noLongerReserved() { + addReservedDomainToList( + RESERVED_LIST_NAME, ImmutableMap.of("blocked1", RESERVED_FOR_SPECIFIC_USE)); + action.run(); + assertThat(queryUnblockableDomains()) + .containsExactly(UnblockableDomain.of("blocked1.app", Reason.RESERVED)); + fakeClock.advanceOneMilli(); + removeReservedDomainFromList(RESERVED_LIST_NAME, ImmutableSet.of("blocked1")); + + String jobName = getRefreshJobName(fakeClock.nowUtc()); + Mockito.reset(bsaReportSender); + action.run(); + assertThat(queryUnblockableDomains()).isEmpty(); + assertThat(gcsClient.readRefreshChanges(jobName)) + .containsExactly( + UnblockableDomainChange.ofDeleted( + UnblockableDomain.of("blocked1.app", Reason.RESERVED))); + + verify(bsaReportSender, never()).addUnblockableDomainsUpdates(anyString()); + verify(bsaReportSender, times(1)).removeUnblockableDomainsUpdates("[\n \"blocked1.app\"\n]"); + } + + @Test + void registeredAndReservedUnblockable_noLongerRegistered_stillUnblockable() throws Exception { + addReservedDomainToList( + RESERVED_LIST_NAME, ImmutableMap.of("blocked1", RESERVED_FOR_SPECIFIC_USE)); + Domain domain = persistActiveDomain("blocked1.app", fakeClock.nowUtc()); + action.run(); + assertThat(queryUnblockableDomains()) + .containsExactly(UnblockableDomain.of("blocked1.app", Reason.REGISTERED)); + fakeClock.advanceOneMilli(); + deleteTestDomain(domain, fakeClock.nowUtc()); + fakeClock.advanceOneMilli(); + + String jobName = getRefreshJobName(fakeClock.nowUtc()); + Mockito.reset(bsaReportSender); + action.run(); + assertThat(queryUnblockableDomains()) + .containsExactly(UnblockableDomain.of("blocked1.app", Reason.RESERVED)); + assertThat(gcsClient.readRefreshChanges(jobName)) + .containsExactly( + UnblockableDomainChange.ofChanged( + UnblockableDomain.of("blocked1.app", Reason.REGISTERED), Reason.RESERVED)); + InOrder inOrder = Mockito.inOrder(bsaReportSender); + inOrder.verify(bsaReportSender).removeUnblockableDomainsUpdates("[\n \"blocked1.app\"\n]"); + inOrder + .verify(bsaReportSender) + .addUnblockableDomainsUpdates("{\n \"reserved\": [\n \"blocked1.app\"\n ]\n}"); + } + + @Test + void reservedUblockable_becomesRegistered_changeToRegisterd() throws Exception { + addReservedDomainToList( + RESERVED_LIST_NAME, ImmutableMap.of("blocked1", RESERVED_FOR_SPECIFIC_USE)); + action.run(); + assertThat(queryUnblockableDomains()) + .containsExactly(UnblockableDomain.of("blocked1.app", Reason.RESERVED)); + fakeClock.advanceOneMilli(); + persistActiveDomain("blocked1.app", fakeClock.nowUtc()); + fakeClock.advanceOneMilli(); + + Mockito.reset(bsaReportSender); + String jobName = getRefreshJobName(fakeClock.nowUtc()); + action.run(); + UnblockableDomain changed = UnblockableDomain.of("blocked1.app", Reason.REGISTERED); + assertThat(queryUnblockableDomains()).containsExactly(changed); + assertThat(gcsClient.readRefreshChanges(jobName)) + .containsExactly( + UnblockableDomainChange.ofChanged( + UnblockableDomain.of("blocked1.app", Reason.RESERVED), Reason.REGISTERED)); + InOrder inOrder = Mockito.inOrder(bsaReportSender); + inOrder.verify(bsaReportSender).removeUnblockableDomainsUpdates("[\n \"blocked1.app\"\n]"); + inOrder + .verify(bsaReportSender) + .addUnblockableDomainsUpdates("{\n \"registered\": [\n \"blocked1.app\"\n ]\n}"); + } + + @Test + void newRegisteredAndReservedDomain_addedAsRegisteredUnblockable() throws Exception { + addReservedDomainToList( + RESERVED_LIST_NAME, ImmutableMap.of("blocked1", RESERVED_FOR_SPECIFIC_USE)); + persistActiveDomain("blocked1.app", fakeClock.nowUtc()); + String jobName = getRefreshJobName(fakeClock.nowUtc()); + action.run(); + UnblockableDomain newUnblockable = UnblockableDomain.of("blocked1.app", Reason.REGISTERED); + assertThat(queryUnblockableDomains()).containsExactly(newUnblockable); + assertThat(gcsClient.readRefreshChanges(jobName)) + .containsExactly(UnblockableDomainChange.ofNew(newUnblockable)); + } + + @Test + void registeredAndReservedUnblockable_noLongerReserved_noChange() throws Exception { + addReservedDomainToList( + RESERVED_LIST_NAME, ImmutableMap.of("blocked1", RESERVED_FOR_SPECIFIC_USE)); + persistActiveDomain("blocked1.app", fakeClock.nowUtc()); + action.run(); + assertThat(queryUnblockableDomains()) + .containsExactly(UnblockableDomain.of("blocked1.app", Reason.REGISTERED)); + fakeClock.advanceOneMilli(); + removeReservedDomainFromList(RESERVED_LIST_NAME, ImmutableSet.of("blocked1")); + fakeClock.advanceOneMilli(); + + Mockito.reset(bsaReportSender); + String jobName = getRefreshJobName(fakeClock.nowUtc()); + action.run(); + assertThat(queryUnblockableDomains()) + .containsExactly(UnblockableDomain.of("blocked1.app", Reason.REGISTERED)); + assertThat(gcsClient.readRefreshChanges(jobName)).isEmpty(); + verifyNoInteractions(bsaReportSender); + } + + @Test + void registeredUblockable_becomesReserved_noChange() throws Exception { + persistActiveDomain("blocked1.app", fakeClock.nowUtc()); + action.run(); + assertThat(queryUnblockableDomains()) + .containsExactly(UnblockableDomain.of("blocked1.app", Reason.REGISTERED)); + fakeClock.advanceOneMilli(); + addReservedDomainToList( + RESERVED_LIST_NAME, ImmutableMap.of("blocked1", RESERVED_FOR_SPECIFIC_USE)); + fakeClock.advanceOneMilli(); + + Mockito.reset(bsaReportSender); + String jobName = getRefreshJobName(fakeClock.nowUtc()); + action.run(); + assertThat(queryUnblockableDomains()) + .containsExactly(UnblockableDomain.of("blocked1.app", Reason.REGISTERED)); + assertThat(gcsClient.readRefreshChanges(jobName)).isEmpty(); + verifyNoInteractions(bsaReportSender); + } +} diff --git a/core/src/test/java/google/registry/bsa/ReservedDomainsTestingUtils.java b/core/src/test/java/google/registry/bsa/ReservedDomainsTestingUtils.java index 720124761..a7e16b441 100644 --- a/core/src/test/java/google/registry/bsa/ReservedDomainsTestingUtils.java +++ b/core/src/test/java/google/registry/bsa/ReservedDomainsTestingUtils.java @@ -62,4 +62,43 @@ public final class ReservedDomainsTestingUtils { .build(); persistResource(tld.asBuilder().setReservedListsByName(reservedLists).build()); } + + public static void addReservedDomainToList( + String listName, ImmutableMap reservedLabels) { + ImmutableMap existingEntries = + ReservedList.get(listName).get().getReservedListEntries(); + ImmutableMap newEntries = + 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( + new ImmutableMap.Builder() + .putAll(existingEntries) + .putAll(newEntries) + .buildKeepingLast()) + .build()); + } + + public static void removeReservedDomainFromList( + String listName, ImmutableSet removedLabels) { + ImmutableMap existingEntries = + ReservedList.get(listName).get().getReservedListEntries(); + ImmutableMap newEntries = + ImmutableMap.copyOf( + Maps.filterEntries(existingEntries, entry -> !removedLabels.contains(entry.getKey()))); + + ReservedListDao.save( + new ReservedList.Builder() + .setName(listName) + .setCreationTimestamp(START_OF_TIME) + .setShouldPublish(true) + .setReservedListMap(newEntries) + .build()); + } } 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 7b5b0b7cf..970971e29 100644 --- a/core/src/test/java/google/registry/bsa/persistence/BsaTestingUtils.java +++ b/core/src/test/java/google/registry/bsa/persistence/BsaTestingUtils.java @@ -14,8 +14,11 @@ package google.registry.bsa.persistence; +import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import com.google.common.collect.ImmutableList; +import google.registry.bsa.api.UnblockableDomain; import google.registry.util.Clock; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -35,7 +38,21 @@ public final class BsaTestingUtils { tm().transact(() -> tm().put(new BsaLabel(domainLabel, BSA_LABEL_CREATION_TIME))); } + public static void persistUnblockableDomain(UnblockableDomain unblockableDomain) { + tm().transact(() -> tm().put(BsaUnblockableDomain.of(unblockableDomain))); + } + public static DownloadScheduler createDownloadScheduler(Clock clock) { return new DownloadScheduler(DEFAULT_DOWNLOAD_INTERVAL, DEFAULT_NOP_INTERVAL, clock); } + + public static RefreshScheduler createRefreshScheduler() { + return new RefreshScheduler(); + } + + public static ImmutableList queryUnblockableDomains() { + return tm().transact(() -> tm().loadAllOf(BsaUnblockableDomain.class)).stream() + .map(BsaUnblockableDomain::toUnblockableDomain) + .collect(toImmutableList()); + } } diff --git a/core/src/test/java/google/registry/bsa/persistence/LabelDiffUpdatesTest.java b/core/src/test/java/google/registry/bsa/persistence/LabelDiffUpdatesTest.java index 6f8ab669e..bb8226d96 100644 --- a/core/src/test/java/google/registry/bsa/persistence/LabelDiffUpdatesTest.java +++ b/core/src/test/java/google/registry/bsa/persistence/LabelDiffUpdatesTest.java @@ -16,6 +16,8 @@ package google.registry.bsa.persistence; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; +import static google.registry.bsa.ReservedDomainsTestingUtils.addReservedListsToTld; +import static google.registry.bsa.ReservedDomainsTestingUtils.createReservedList; import static google.registry.bsa.persistence.LabelDiffUpdates.applyLabelDiff; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; @@ -26,7 +28,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import google.registry.bsa.IdnChecker; @@ -36,9 +37,6 @@ import google.registry.bsa.api.UnblockableDomain; import google.registry.bsa.persistence.BsaUnblockableDomain.Reason; 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; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension; import google.registry.testing.FakeClock; @@ -139,18 +137,8 @@ class LabelDiffUpdatesTest { @Test void applyLabelDiffs_newLabel() { persistActiveDomain("label.app"); - ReservedListDao.save( - new ReservedList.Builder() - .setReservedListMap( - ImmutableMap.of( - "label", - ReservedListEntry.create( - "label", ReservationType.RESERVED_FOR_SPECIFIC_USE, null))) - .setName("page_reserved") - .setCreationTimestamp(fakeClock.nowUtc()) - .build()); - ReservedList reservedList = ReservedList.get("page_reserved").get(); - tm().transact(() -> tm().put(page.asBuilder().setReservedLists(reservedList).build())); + createReservedList("page_reserved", "label", ReservationType.RESERVED_FOR_SPECIFIC_USE); + addReservedListsToTld("page", ImmutableList.of("page_reserved")); when(idnChecker.getForbiddingTlds(any())) .thenReturn(Sets.difference(ImmutableSet.of(dev), ImmutableSet.of()).immutableCopy()); 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 b94d568cf..e07abf011 100644 --- a/core/src/test/java/google/registry/bsa/persistence/QueriesTest.java +++ b/core/src/test/java/google/registry/bsa/persistence/QueriesTest.java @@ -208,6 +208,32 @@ class QueriesTest { .containsExactly("d1.tld", "d2.tld"); } + @Test + void queryNewlyCreatedDomains_onlyDomainsAfterMinCreationTimeReturned() { + DateTime testStartTime = fakeClock.nowUtc(); + createTlds("tld"); + persistNewRegistrar("TheRegistrar"); + // time 0: + persistResource( + newDomain("d1.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build()); + // time 0, deletion time 1 + persistDomainAsDeleted( + newDomain("will-delete.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build(), + fakeClock.nowUtc().plusMillis(1)); + fakeClock.advanceOneMilli(); + // time 1 + persistResource( + newDomain("d2.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build()); + fakeClock.advanceOneMilli(); + // Now is time 2, ask for domains created since time 1 + assertThat( + bsaQuery( + () -> + queryNewlyCreatedDomains( + ImmutableList.of("tld"), testStartTime.plusMillis(1), fakeClock.nowUtc()))) + .containsExactly("d2.tld"); + } + @Test void queryNewlyCreatedDomains_onlyDomainsInRequestedTldsReturned() { DateTime testStartTime = fakeClock.nowUtc();