Check for missing BSA unblockable domains (#2394)

* Check for missing BSA unblockable domains

All unblockable domains created before the last refresh run should be
reported as unblockable (registered).

All reserved domains that are not registered should be reported as
unblockable (reserved). Note that transient errors may be reported for
newly added reserved domains since we do not maintain update time for
when a reserved label is associated with a TLD. However, this scenario
is extremely rare in operations.

* Addressing review

core/src/main/java/google/registry/bsa/BsaValidateAction.java

@@ -17,12 +17,20 @@ package google.registry.bsa;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Throwables.getStackTraceAsString;
 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.batchReadBsaLabelText;
+import static google.registry.bsa.persistence.Queries.queryMissedRegisteredUnblockables;
+import static google.registry.bsa.persistence.Queries.queryUnblockableDomainByLabels;
+import static google.registry.model.tld.Tld.isEnrolledWithBsa;
+import static google.registry.model.tld.Tlds.getTldEntitiesOfType;
 import static google.registry.persistence.transaction.TransactionManagerFactory.replicaTm;
 import static google.registry.request.Action.Method.GET;
 import static google.registry.request.Action.Method.POST;
+import static google.registry.util.BatchedStreams.toBatches;
 import static javax.servlet.http.HttpServletResponse.SC_OK;
 
 import com.google.common.base.Joiner;
@@ -38,9 +46,12 @@ import google.registry.bsa.api.UnblockableDomain;
 import google.registry.bsa.api.UnblockableDomain.Reason;
 import google.registry.bsa.persistence.DownloadScheduler;
 import google.registry.bsa.persistence.Queries;
+import google.registry.bsa.persistence.Queries.DomainLifeSpan;
 import google.registry.config.RegistryConfig.Config;
 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.persistence.VKey;
 import google.registry.request.Action;
 import google.registry.request.Response;
@@ -124,7 +135,8 @@ public class BsaValidateAction implements Runnable {
 
     ImmutableList.Builder<String> errorsBuilder = new ImmutableList.Builder<>();
     errorsBuilder.addAll(checkBsaLabels(downloadJobName.get()));
-    errorsBuilder.addAll(checkUnblockableDomains());
+    errorsBuilder.addAll(checkWronglyReportedUnblockableDomains());
+    errorsBuilder.addAll(checkMissingUnblockableDomains());
 
     ImmutableList<String> errors = errorsBuilder.build();
 
@@ -173,7 +185,7 @@ public class BsaValidateAction implements Runnable {
     return errors.build();
   }
 
-  ImmutableList<String> checkUnblockableDomains() {
+  ImmutableList<String> checkWronglyReportedUnblockableDomains() {
     ImmutableList.Builder<String> errors = new ImmutableList.Builder<>();
     Optional<UnblockableDomain> lastRead = Optional.empty();
     ImmutableList<UnblockableDomain> batch;
@@ -263,6 +275,77 @@ public class BsaValidateAction implements Runnable {
     return labelsBuilder.build();
   }
 
+  ImmutableList<String> checkMissingUnblockableDomains() {
+    DateTime now = clock.nowUtc();
+    ImmutableList.Builder<String> errors = new ImmutableList.Builder<>();
+    errors.addAll(checkForMissingReservedUnblockables(now));
+    errors.addAll(checkForMissingRegisteredUnblockables(now));
+    return errors.build();
+  }
+
+  ImmutableList<String> checkForMissingRegisteredUnblockables(DateTime now) {
+    ImmutableList.Builder<String> errors = new ImmutableList.Builder<>();
+    ImmutableList<Tld> bsaEnabledTlds =
+        getTldEntitiesOfType(TldType.REAL).stream()
+            .filter(tld -> isEnrolledWithBsa(tld, now))
+            .collect(toImmutableList());
+    DateTime stalenessThreshold = now.minus(maxStaleness);
+    bsaEnabledTlds.stream()
+        .map(Tld::getTldStr)
+        .map(tld -> bsaQuery(() -> queryMissedRegisteredUnblockables(tld, now)))
+        .flatMap(ImmutableList::stream)
+        .filter(domainLifeSpan -> domainLifeSpan.creationTime().isBefore(stalenessThreshold))
+        .map(DomainLifeSpan::domainName)
+        .forEach(
+            domainName ->
+                errors.add(
+                    String.format(
+                        "Registered domain %s missing or not recorded as REGISTERED", domainName)));
+    return errors.build();
+  }
+
+  ImmutableList<String> checkForMissingReservedUnblockables(DateTime now) {
+    ImmutableList.Builder<String> errors = new ImmutableList.Builder<>();
+    try (Stream<ImmutableList<String>> reservedNames =
+        toBatches(getAllReservedNames(now), transactionBatchSize)) {
+      reservedNames
+          .map(this::checkOneBatchReservedDomainsForMissingUnblockables)
+          .forEach(errors::addAll);
+    }
+    return errors.build();
+  }
+
+  ImmutableList<String> checkOneBatchReservedDomainsForMissingUnblockables(
+      ImmutableList<String> batch) {
+    ImmutableSet<String> labels =
+        batch.stream()
+            .map(InternetDomainName::from)
+            .map(d -> d.parts().get(0))
+            .collect(toImmutableSet());
+    ImmutableMap<String, UnblockableDomain> persistedUnblockables =
+        bsaQuery(
+            () ->
+                queryUnblockableDomainByLabels(labels)
+                    .collect(toImmutableMap(UnblockableDomain::domainName, x -> x)));
+    ImmutableList.Builder<String> errors = new ImmutableList.Builder<>();
+    ImmutableSet<UnblockableDomain.Reason> acceptableReasons =
+        ImmutableSet.of(Reason.REGISTERED, Reason.RESERVED);
+    for (var domainName : batch) {
+      if (!persistedUnblockables.containsKey(domainName)) {
+        errors.add(String.format("Missing unblockable domain: %s is reserved.", domainName));
+        continue;
+      }
+      var unblockable = persistedUnblockables.get(domainName);
+      if (!acceptableReasons.contains(unblockable.reason())) {
+        errors.add(
+            String.format(
+                "Wrong unblockable reason: %s should be reserved or registered, found %s.",
+                domainName, unblockable.reason()));
+      }
+    }
+    return errors.build();
+  }
+
   static String parseBlockListLine(String line) {
     int firstComma = line.indexOf(',');
     checkArgument(firstComma > 0, "Invalid block list line: %s", line);

core/src/main/java/google/registry/bsa/persistence/Queries.java

@@ -19,12 +19,14 @@ import static com.google.common.collect.ImmutableList.toImmutableList;
 import static google.registry.bsa.BsaStringUtils.DOMAIN_SPLITTER;
 import static google.registry.bsa.BsaTransactions.bsaQuery;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
+import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import google.registry.bsa.api.UnblockableDomain;
 import google.registry.model.CreateAutoTimestamp;
+import java.sql.Timestamp;
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -66,6 +68,11 @@ public final class Queries {
         .collect(toImmutableList());
   }
 
+  public static Stream<UnblockableDomain> queryUnblockableDomainByLabels(
+      ImmutableCollection<String> labels) {
+    return queryBsaUnblockableDomainByLabels(labels).map(BsaUnblockableDomain::toUnblockableDomain);
+  }
+
   static Stream<BsaUnblockableDomain> queryBsaUnblockableDomainByLabels(
       ImmutableCollection<String> labels) {
     return ((Stream<?>)
@@ -141,4 +148,55 @@ public final class Queries {
             .setParameter("tlds", tlds)
             .getResultList());
   }
+
+  /**
+   * Finds all currently registered domains that match BSA labels but are not recorded as
+   * unblockable.
+   *
+   * @return The missing unblockables and their creation and deletion time.
+   */
+  public static ImmutableList<DomainLifeSpan> queryMissedRegisteredUnblockables(
+      String tld, DateTime now) {
+    String sqlTemplate =
+        """
+    SELECT l.domain_name, creation_time, deletion_time
+    FROM
+        (SELECT d.domain_name, d.creation_time, d.deletion_time
+         FROM
+             "Domain" d
+         JOIN
+             (SELECT concat(label, '.', :tld) AS domain_name from "BsaLabel") b
+         ON b.domain_name = d.domain_name
+         WHERE deletion_time > ':now') l
+    LEFT OUTER JOIN
+        (SELECT concat(label, '.', tld) as domain_name
+         FROM "BsaUnblockableDomain"
+         WHERE tld = :tld and reason = 'REGISTERED') r
+    ON l.domain_name = r.domain_name
+    WHERE r.domain_name is null;
+    """;
+    // Native query: Hibernate's setParameter wrongly converts DateTime to bytea
+    String sql = sqlTemplate.replace(":now", now.toString());
+
+    return ((Stream<?>)
+            tm().getEntityManager()
+                .createNativeQuery(sql)
+                .setParameter("tld", tld)
+                .getResultStream())
+        .map(Object[].class::cast)
+        .map(
+            row ->
+                new DomainLifeSpan(
+                    (String) row[0],
+                    toDateTime((Timestamp) row[1]),
+                    toDateTime((Timestamp) row[2])))
+        .collect(toImmutableList());
+  }
+
+  // For testing convenience: 'assertEquals' fails between `new DateTime(timestamp)` and below.
+  static DateTime toDateTime(Timestamp timestamp) {
+    return new DateTime(timestamp.getTime(), UTC);
+  }
+
+  public record DomainLifeSpan(String domainName, DateTime creationTime, DateTime deletionTime) {}
 }

core/src/test/java/google/registry/bsa/BsaValidateActionTest.java

@@ -15,13 +15,18 @@
 package google.registry.bsa;
 
 import static com.google.common.base.Throwables.getStackTraceAsString;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
 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.persistence.BsaTestingUtils.persistBsaLabel;
 import static google.registry.bsa.persistence.BsaTestingUtils.persistDownloadSchedule;
 import static google.registry.bsa.persistence.BsaTestingUtils.persistUnblockableDomain;
 import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.persistActiveDomain;
 import static google.registry.testing.DatabaseHelper.persistDeletedDomain;
+import static google.registry.testing.DatabaseHelper.persistResource;
+import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.startsWith;
@@ -43,13 +48,16 @@ import google.registry.bsa.persistence.BsaTestingUtils;
 import google.registry.gcs.GcsUtils;
 import google.registry.groups.GmailClient;
 import google.registry.model.domain.Domain;
+import google.registry.model.tld.label.ReservationType;
 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.tldconfig.idn.IdnTableEnum;
 import google.registry.util.EmailMessage;
+import java.util.Optional;
 import java.util.concurrent.Callable;
+import java.util.stream.Stream;
 import javax.mail.internet.InternetAddress;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
@@ -269,7 +277,7 @@ public class BsaValidateActionTest {
     persistUnblockableDomain(UnblockableDomain.of("label", "app", Reason.REGISTERED));
     when(idnChecker.getAllValidIdns(anyString())).thenReturn(ImmutableSet.of(IdnTableEnum.JA));
 
-    assertThat(action.checkUnblockableDomains()).isEmpty();
+    assertThat(action.checkWronglyReportedUnblockableDomains()).isEmpty();
   }
 
   @Test
@@ -280,10 +288,75 @@ public class BsaValidateActionTest {
     persistUnblockableDomain(UnblockableDomain.of("label", "app", Reason.RESERVED));
     when(idnChecker.getAllValidIdns(anyString())).thenReturn(ImmutableSet.of(IdnTableEnum.JA));
 
-    assertThat(action.checkUnblockableDomains())
+    assertThat(action.checkWronglyReportedUnblockableDomains())
         .containsExactly("label.app: should be REGISTERED, found RESERVED");
   }
 
+  @Test
+  void checkForMissingReservedUnblockables_success() {
+    persistResource(
+        createTld("app").asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build());
+    persistResource(
+        createTld("dev").asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build());
+    persistBsaLabel("registered-reserved");
+    persistBsaLabel("reserved-only");
+    persistBsaLabel("reserved-missing");
+    persistBsaLabel("invalid-in-app");
+
+    persistUnblockableDomain(UnblockableDomain.of("registered-reserved", "app", Reason.REGISTERED));
+    persistUnblockableDomain(UnblockableDomain.of("reserved-only", "app", Reason.RESERVED));
+    persistUnblockableDomain(UnblockableDomain.of("invalid-in-app", "dev", Reason.RESERVED));
+
+    createReservedList(
+        "rl",
+        Stream.of("registered-reserved", "reserved-only", "reserved-missing")
+            .collect(toImmutableMap(x -> x, x -> ReservationType.RESERVED_FOR_SPECIFIC_USE)));
+    addReservedListsToTld("app", ImmutableList.of("rl"));
+
+    ImmutableList<String> errors = action.checkForMissingReservedUnblockables(fakeClock.nowUtc());
+    assertThat(errors)
+        .containsExactly("Missing unblockable domain: reserved-missing.app is reserved.");
+  }
+
+  @Test
+  void checkForMissingReservedUnblockablesInOneTld_success() {
+    persistResource(
+        createTld("app").asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build());
+    persistResource(
+        createTld("dev").asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build());
+    persistBsaLabel("reserved-missing-in-app");
+    persistUnblockableDomain(
+        UnblockableDomain.of("reserved-missing-in-app", "dev", Reason.REGISTERED));
+
+    createReservedList(
+        "rl",
+        Stream.of("reserved-missing-in-app")
+            .collect(toImmutableMap(x -> x, x -> ReservationType.RESERVED_FOR_SPECIFIC_USE)));
+    addReservedListsToTld("app", ImmutableList.of("rl"));
+    addReservedListsToTld("dev", ImmutableList.of("rl"));
+
+    ImmutableList<String> errors = action.checkForMissingReservedUnblockables(fakeClock.nowUtc());
+    assertThat(errors)
+        .containsExactly("Missing unblockable domain: reserved-missing-in-app.app is reserved.");
+  }
+
+  @Test
+  void checkForMissingRegisteredUnblockables_success() {
+    persistResource(
+        createTld("app").asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build());
+    persistBsaLabel("registered");
+    persistBsaLabel("registered-missing");
+    persistUnblockableDomain(UnblockableDomain.of("registered", "app", Reason.REGISTERED));
+    persistUnblockableDomain(UnblockableDomain.of("registered-missing", "app", Reason.RESERVED));
+    persistActiveDomain("registered.app");
+    persistActiveDomain("registered-missing.app");
+
+    ImmutableList<String> errors = action.checkForMissingRegisteredUnblockables(fakeClock.nowUtc());
+    assertThat(errors)
+        .containsExactly(
+            "Registered domain registered-missing.app missing or not recorded as REGISTERED");
+  }
+
   @Test
   void notificationSent_cannotAcquireLock() {
     when(bsaLock.executeWithLock(any())).thenReturn(false);

core/src/test/java/google/registry/bsa/persistence/QueriesTest.java

@@ -21,18 +21,22 @@ import static google.registry.bsa.persistence.Queries.batchReadBsaLabelText;
 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.queryMissedRegisteredUnblockables;
 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;
 import static google.registry.testing.DatabaseHelper.newDomain;
+import static google.registry.testing.DatabaseHelper.persistActiveDomain;
 import static google.registry.testing.DatabaseHelper.persistDomainAsDeleted;
 import static google.registry.testing.DatabaseHelper.persistNewRegistrar;
-import static google.registry.testing.DatabaseHelper.persistResource;
+import static google.registry.util.DateTimeUtils.END_OF_TIME;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import google.registry.bsa.api.UnblockableDomain;
 import google.registry.bsa.persistence.BsaUnblockableDomain.Reason;
+import google.registry.bsa.persistence.Queries.DomainLifeSpan;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension;
 import google.registry.testing.FakeClock;
@@ -200,16 +204,14 @@ class QueriesTest {
     createTlds("tld");
     persistNewRegistrar("TheRegistrar");
     // time 0:
-    persistResource(
-        newDomain("d1.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build());
+    persistActiveDomain("d1.tld", fakeClock.nowUtc());
     // 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());
+    persistActiveDomain("d2.tld", fakeClock.nowUtc());
     fakeClock.advanceOneMilli();
     // Now is time 2
     assertThat(
@@ -226,16 +228,14 @@ class QueriesTest {
     createTlds("tld");
     persistNewRegistrar("TheRegistrar");
     // time 0:
-    persistResource(
-        newDomain("d1.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build());
+    persistActiveDomain("d1.tld", fakeClock.nowUtc());
     // 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());
+    persistActiveDomain("d2.tld", fakeClock.nowUtc());
     fakeClock.advanceOneMilli();
     // Now is time 2, ask for domains created since time 1
     assertThat(
@@ -251,10 +251,8 @@ class QueriesTest {
     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());
+    persistActiveDomain("d1.tld", fakeClock.nowUtc());
+    persistActiveDomain("d2.tld2", fakeClock.nowUtc());
     fakeClock.advanceOneMilli();
     assertThat(
             bsaQuery(
@@ -263,4 +261,38 @@ class QueriesTest {
                         ImmutableList.of("tld"), testStartTime, fakeClock.nowUtc())))
         .containsExactly("d1.tld");
   }
+
+  @Test
+  void queryMissedRegisteredUnblockables_success() {
+    createTlds("tld", "tld2");
+    persistNewRegistrar("TheRegistrar");
+    DateTime time1 = fakeClock.nowUtc();
+    persistActiveDomain("unblocked1.tld", fakeClock.nowUtc());
+    persistActiveDomain("unblocked2.tld2", fakeClock.nowUtc());
+    persistActiveDomain("label1.tld", fakeClock.nowUtc());
+    persistActiveDomain("label2.tld2", fakeClock.nowUtc());
+    fakeClock.advanceOneMilli();
+    DateTime time2 = fakeClock.nowUtc();
+    persistDomainAsDeleted(
+        newDomain("label3.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build(),
+        fakeClock.nowUtc().plusMillis(1));
+    // Deleted in the future
+    persistDomainAsDeleted(
+        newDomain("label3.tld2").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build(),
+        fakeClock.nowUtc().plusHours(1));
+    fakeClock.advanceOneMilli();
+    assertThat(bsaQuery(() -> queryMissedRegisteredUnblockables("tld", fakeClock.nowUtc())))
+        .containsExactly(new DomainLifeSpan("label1.tld", time1, END_OF_TIME));
+    assertThat(bsaQuery(() -> queryMissedRegisteredUnblockables("tld2", fakeClock.nowUtc())))
+        .containsExactly(
+            new DomainLifeSpan("label2.tld2", time1, END_OF_TIME),
+            new DomainLifeSpan("label3.tld2", time2, time2.plusHours(1)));
+
+    BsaTestingUtils.persistUnblockableDomain(
+        UnblockableDomain.of("label2", "tld2", UnblockableDomain.Reason.REGISTERED));
+    BsaTestingUtils.persistUnblockableDomain(
+        UnblockableDomain.of("label3", "tld2", UnblockableDomain.Reason.RESERVED));
+    assertThat(bsaQuery(() -> queryMissedRegisteredUnblockables("tld2", fakeClock.nowUtc())))
+        .containsExactly(new DomainLifeSpan("label3.tld2", time2, time2.plusHours(1)));
+  }
 }