Fix BigQuery data set name handling in activity reporting (#1361)

* Fix BigQuery data set name handling in activity reporting

This is not a constant (as it depends on runtime state), so it can't be named
using UPPER_SNAKE_CASE. Additionally, it's not good practice to use field
initialization when there's logic depending on runtime state involved. So this
PR changes the class to use constructor injection and moves the logic into the
constructor.

* Add fix for ICANN reporting provide

* Extract out ICANN reporting data set

* Inject TransactionManager

* Make TransactionInfo static (per Mike)

* Use ofyTm() in BackupTestStore

* Revert extraneous formatting

* Use auditedOfy in CommitLogMutationTest

core/src/test/java/google/registry/beam/initsql/BackupTestStore.java

@@ -16,7 +16,7 @@ package google.registry.beam.initsql;
 
 import static com.google.common.base.Preconditions.checkState;
 import static google.registry.model.ofy.ObjectifyService.auditedOfy;
-import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
+import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
 
 import com.google.appengine.api.datastore.DatastoreService;
 import com.google.appengine.api.datastore.DatastoreServiceFactory;
@@ -75,7 +75,8 @@ public final class BackupTestStore implements AutoCloseable {
   /** Returns the timestamp of the transaction. */
   long transact(Iterable<Object> deletes, Iterable<Object> newOrUpdated) {
     long timestamp = fakeClock.nowUtc().getMillis();
-    tm().transact(
+    ofyTm()
+        .transact(
             () -> {
               auditedOfy().delete().entities(deletes);
               auditedOfy().save().entities(newOrUpdated);
@@ -91,7 +92,7 @@ public final class BackupTestStore implements AutoCloseable {
   @SafeVarargs
   public final long insertOrUpdate(Object... entities) {
     long timestamp = fakeClock.nowUtc().getMillis();
-    tm().transact(() -> auditedOfy().save().entities(entities).now());
+    ofyTm().transact(() -> auditedOfy().save().entities(entities).now());
     fakeClock.advanceOneMilli();
     return timestamp;
   }
@@ -100,7 +101,7 @@ public final class BackupTestStore implements AutoCloseable {
   @SafeVarargs
   public final long delete(Object... entities) {
     long timestamp = fakeClock.nowUtc().getMillis();
-    tm().transact(() -> auditedOfy().delete().entities(entities).now());
+    ofyTm().transact(() -> auditedOfy().delete().entities(entities).now());
     fakeClock.advanceOneMilli();
     return timestamp;
   }

core/src/test/java/google/registry/model/ofy/CommitLogMutationTest.java

@@ -16,7 +16,6 @@ package google.registry.model.ofy;
 
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ofy.ObjectifyService.auditedOfy;
-import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.testing.DatabaseHelper.createTld;
 
 import com.google.appengine.api.datastore.Entity;
@@ -66,7 +65,7 @@ public class CommitLogMutationTest {
     Entity rawEntity = convertToEntityInTxn(someObject);
     // Needs to be in a transaction so that registry-saving-to-entity will work.
     CommitLogMutation mutation =
-        tm().transact(() -> CommitLogMutation.create(manifestKey, someObject));
+        auditedOfy().transact(() -> CommitLogMutation.create(manifestKey, someObject));
     assertThat(Key.create(mutation))
         .isEqualTo(CommitLogMutation.createKey(manifestKey, Key.create(someObject)));
     assertThat(mutation.getEntity()).isEqualTo(rawEntity);
@@ -86,6 +85,6 @@ public class CommitLogMutationTest {
   }
 
   private static Entity convertToEntityInTxn(final ImmutableObject object) {
-    return tm().transact(() -> auditedOfy().save().toEntity(object));
+    return auditedOfy().transact(() -> auditedOfy().save().toEntity(object));
   }
 }

core/src/test/java/google/registry/reporting/icann/ActivityReportingQueryBuilderTest.java

@@ -39,18 +39,17 @@ class ActivityReportingQueryBuilderTest {
 
   private final YearMonth yearMonth = new YearMonth(2017, 9);
 
-  private ActivityReportingQueryBuilder getQueryBuilder() {
-    ActivityReportingQueryBuilder queryBuilder = new ActivityReportingQueryBuilder();
-    queryBuilder.projectId = "domain-registry-alpha";
-    queryBuilder.dnsCountQueryCoordinator =
+  private ActivityReportingQueryBuilder createQueryBuilder(String datasetName) {
+    return new ActivityReportingQueryBuilder(
+        "domain-registry-alpha",
+        datasetName,
         new BasicDnsCountQueryCoordinator(
-            new BasicDnsCountQueryCoordinator.Params(null, queryBuilder.projectId));
-    return queryBuilder;
+            new BasicDnsCountQueryCoordinator.Params(null, "domain-registry-alpha")));
   }
 
   @TestOfyOnly
   void testAggregateQueryMatch_datastore() {
-    ActivityReportingQueryBuilder queryBuilder = getQueryBuilder();
+    ActivityReportingQueryBuilder queryBuilder = createQueryBuilder("icann_reporting");
     assertThat(queryBuilder.getReportQuery(yearMonth))
         .isEqualTo(
             "#standardSQL\nSELECT * FROM "
@@ -58,8 +57,8 @@ class ActivityReportingQueryBuilderTest {
   }
 
   @TestSqlOnly
-  void testAggregateQueryMatch_cloud_sql() {
-    ActivityReportingQueryBuilder queryBuilder = getQueryBuilder();
+  void testAggregateQueryMatch_cloudSql() {
+    ActivityReportingQueryBuilder queryBuilder = createQueryBuilder("cloud_sql_icann_reporting");
     assertThat(queryBuilder.getReportQuery(yearMonth))
         .isEqualTo(
             "#standardSQL\n"
@@ -78,7 +77,7 @@ class ActivityReportingQueryBuilderTest {
             ActivityReportingQueryBuilder.WHOIS_COUNTS,
             ActivityReportingQueryBuilder.ACTIVITY_REPORT_AGGREGATION);
 
-    ActivityReportingQueryBuilder queryBuilder = getQueryBuilder();
+    ActivityReportingQueryBuilder queryBuilder = createQueryBuilder("icann_reporting");
     ImmutableMap<String, String> actualQueries = queryBuilder.getViewQueryMap(yearMonth);
     for (String queryName : expectedQueryNames) {
       String actualTableName = String.format("%s_201709", queryName);
@@ -99,7 +98,7 @@ class ActivityReportingQueryBuilderTest {
             ActivityReportingQueryBuilder.WHOIS_COUNTS,
             ActivityReportingQueryBuilder.ACTIVITY_REPORT_AGGREGATION);
 
-    ActivityReportingQueryBuilder queryBuilder = getQueryBuilder();
+    ActivityReportingQueryBuilder queryBuilder = createQueryBuilder("cloud_sql_icann_reporting");
     ImmutableMap<String, String> actualQueries = queryBuilder.getViewQueryMap(yearMonth);
     for (String queryName : expectedQueryNames) {
       String actualTableName = String.format("%s_201709", queryName);

core/src/test/java/google/registry/reporting/icann/IcannReportingModuleTest.java

@@ -18,34 +18,55 @@ import static com.google.common.truth.Truth.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import google.registry.persistence.transaction.JpaTestExtensions;
+import google.registry.persistence.transaction.JpaTestExtensions.JpaUnitTestExtension;
 import javax.servlet.http.HttpServletRequest;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 /** Unit tests for {@link google.registry.reporting.icann.IcannReportingModule}. */
 class IcannReportingModuleTest {
 
-  @Test
-  void testProvideReportTypes() {
-    HttpServletRequest req = mock(HttpServletRequest.class);
+  @RegisterExtension
+  JpaUnitTestExtension jpaExtension = new JpaTestExtensions.Builder().buildUnitTestExtension();
 
+  @Test
+  void testProvideReportTypes_null() {
+    HttpServletRequest req = mock(HttpServletRequest.class);
     when(req.getParameter("reportTypes")).thenReturn(null);
     assertThat(IcannReportingModule.provideReportTypes(req))
         .containsExactly(
             IcannReportingModule.ReportType.ACTIVITY, IcannReportingModule.ReportType.TRANSACTIONS);
+  }
 
+  @Test
+  void testProvideReportTypes_empty() {
+    HttpServletRequest req = mock(HttpServletRequest.class);
     when(req.getParameter("reportTypes")).thenReturn("");
     assertThat(IcannReportingModule.provideReportTypes(req))
         .containsExactly(
             IcannReportingModule.ReportType.ACTIVITY, IcannReportingModule.ReportType.TRANSACTIONS);
+  }
 
+  @Test
+  void testProvideReportTypes_activity() {
+    HttpServletRequest req = mock(HttpServletRequest.class);
     when(req.getParameter("reportTypes")).thenReturn("activity");
     assertThat(IcannReportingModule.provideReportTypes(req))
         .containsExactly(IcannReportingModule.ReportType.ACTIVITY);
+  }
 
+  @Test
+  void testProvideReportTypes_transactions() {
+    HttpServletRequest req = mock(HttpServletRequest.class);
     when(req.getParameter("reportTypes")).thenReturn("transactions");
     assertThat(IcannReportingModule.provideReportTypes(req))
         .containsExactly(IcannReportingModule.ReportType.TRANSACTIONS);
+  }
 
+  @Test
+  void testProvideReportTypes_bothTypes() {
+    HttpServletRequest req = mock(HttpServletRequest.class);
     when(req.getParameter("reportTypes")).thenReturn("activity,transactions");
     assertThat(IcannReportingModule.provideReportTypes(req))
         .containsExactly(

core/src/test/java/google/registry/reporting/icann/IcannReportingStagerTest.java

@@ -54,13 +54,12 @@ class IcannReportingStagerTest {
 
   private IcannReportingStager createStager() {
     IcannReportingStager action = new IcannReportingStager();
-    ActivityReportingQueryBuilder activityBuilder = new ActivityReportingQueryBuilder();
-    activityBuilder.projectId = "test-project";
-    activityBuilder.dnsCountQueryCoordinator = new BasicDnsCountQueryCoordinator(null);
+    ActivityReportingQueryBuilder activityBuilder =
+        new ActivityReportingQueryBuilder(
+            "test-project", "icann_reporting", new BasicDnsCountQueryCoordinator(null));
     action.activityQueryBuilder = activityBuilder;
-    TransactionsReportingQueryBuilder transactionsBuilder = new TransactionsReportingQueryBuilder();
-    transactionsBuilder.projectId = "test-project";
-    action.transactionsQueryBuilder = transactionsBuilder;
+    action.transactionsQueryBuilder =
+        new TransactionsReportingQueryBuilder("test-project", "icann_reporting");
     action.reportingBucket = "test-bucket";
     action.bigquery = bigquery;
     action.gcsUtils = gcsUtils;

core/src/test/java/google/registry/reporting/icann/TransactionsReportingQueryBuilderTest.java

@@ -27,8 +27,8 @@ class TransactionsReportingQueryBuilderTest {
   private final YearMonth yearMonth = new YearMonth(2017, 9);
 
   private TransactionsReportingQueryBuilder getQueryBuilder() {
-    TransactionsReportingQueryBuilder queryBuilder = new TransactionsReportingQueryBuilder();
-    queryBuilder.projectId = "domain-registry-alpha";
+    TransactionsReportingQueryBuilder queryBuilder =
+        new TransactionsReportingQueryBuilder("domain-registry-alpha", "icann_reporting");
     return queryBuilder;
   }