From fff63e3b2ae47ab4dae94243c6557c956c001d77 Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Tue, 5 Oct 2021 15:11:03 -0400 Subject: [PATCH] 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 --- .../persistence/PersistenceModule.java | 8 +++++ .../JpaTransactionManagerImpl.java | 6 ++-- .../icann/ActivityReportingQueryBuilder.java | 33 +++++++++++-------- .../reporting/icann/IcannReportingModule.java | 17 +++++++--- .../TransactionsReportingQueryBuilder.java | 18 ++++++---- .../beam/initsql/BackupTestStore.java | 9 ++--- .../model/ofy/CommitLogMutationTest.java | 5 ++- .../ActivityReportingQueryBuilderTest.java | 21 ++++++------ .../icann/IcannReportingModuleTest.java | 27 +++++++++++++-- .../icann/IcannReportingStagerTest.java | 11 +++---- ...TransactionsReportingQueryBuilderTest.java | 4 +-- 11 files changed, 103 insertions(+), 56 deletions(-) diff --git a/core/src/main/java/google/registry/persistence/PersistenceModule.java b/core/src/main/java/google/registry/persistence/PersistenceModule.java index 01af5ccda..22b6a4d2a 100644 --- a/core/src/main/java/google/registry/persistence/PersistenceModule.java +++ b/core/src/main/java/google/registry/persistence/PersistenceModule.java @@ -21,6 +21,7 @@ import static google.registry.config.RegistryConfig.getHibernateHikariIdleTimeou import static google.registry.config.RegistryConfig.getHibernateHikariMaximumPoolSize; import static google.registry.config.RegistryConfig.getHibernateHikariMinimumIdle; import static google.registry.config.RegistryConfig.getHibernateLogSqlQueries; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.api.client.auth.oauth2.Credential; import com.google.common.annotations.VisibleForTesting; @@ -34,6 +35,7 @@ import google.registry.config.RegistryConfig.Config; import google.registry.persistence.transaction.CloudSqlCredentialSupplier; import google.registry.persistence.transaction.JpaTransactionManager; import google.registry.persistence.transaction.JpaTransactionManagerImpl; +import google.registry.persistence.transaction.TransactionManager; import google.registry.privileges.secretmanager.SqlCredential; import google.registry.privileges.secretmanager.SqlCredentialStore; import google.registry.privileges.secretmanager.SqlUser; @@ -206,6 +208,12 @@ public abstract class PersistenceModule { } } + @Provides + @Singleton + static TransactionManager provideTransactionManager() { + return tm(); + } + @Provides @Singleton @AppEngineJpaTm 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 f156f6d7d..10a3fbf3a 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -98,14 +98,16 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { // EntityManagerFactory is thread safe. private final EntityManagerFactory emf; private final Clock clock; + // TODO(b/177588434): Investigate alternatives for managing transaction information. ThreadLocal // adds an unnecessary restriction that each request has to be processed by one thread // synchronously. - private final ThreadLocal transactionInfo = + private static final ThreadLocal transactionInfo = ThreadLocal.withInitial(TransactionInfo::new); + // If this value is present, use it to determine whether or not to replay SQL transactions to // Datastore, rather than using the schedule stored in Datastore. - private static ThreadLocal> replaySqlToDatastoreOverrideForTest = + private static final ThreadLocal> replaySqlToDatastoreOverrideForTest = ThreadLocal.withInitial(Optional::empty); public JpaTransactionManagerImpl(EntityManagerFactory emf, Clock clock) { diff --git a/core/src/main/java/google/registry/reporting/icann/ActivityReportingQueryBuilder.java b/core/src/main/java/google/registry/reporting/icann/ActivityReportingQueryBuilder.java index 3cb44b2ad..386ef2b64 100644 --- a/core/src/main/java/google/registry/reporting/icann/ActivityReportingQueryBuilder.java +++ b/core/src/main/java/google/registry/reporting/icann/ActivityReportingQueryBuilder.java @@ -16,6 +16,7 @@ package google.registry.reporting.icann; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.reporting.icann.IcannReportingModule.DATASTORE_EXPORT_DATA_SET; +import static google.registry.reporting.icann.IcannReportingModule.ICANN_REPORTING_DATA_SET; import static google.registry.reporting.icann.QueryBuilderUtils.getQueryFromFile; import static google.registry.reporting.icann.QueryBuilderUtils.getTableName; @@ -23,6 +24,7 @@ import com.google.common.collect.ImmutableMap; import google.registry.config.RegistryConfig.Config; import google.registry.util.SqlTemplate; import javax.inject.Inject; +import javax.inject.Named; import org.joda.time.LocalDate; import org.joda.time.YearMonth; import org.joda.time.format.DateTimeFormat; @@ -38,29 +40,32 @@ public final class ActivityReportingQueryBuilder implements QueryBuilder { static final String EPP_METRICS = "epp_metrics"; static final String WHOIS_COUNTS = "whois_counts"; static final String ACTIVITY_REPORT_AGGREGATION = "activity_report_aggregation"; - final String BIGQUERY_DATA_SET = tm().isOfy() ? "icann_reporting" : "cloud_sql_icann_reporting"; + + private final String projectId; + private final DnsCountQueryCoordinator dnsCountQueryCoordinator; + private final String icannReportingDataSet; @Inject - @Config("projectId") - String projectId; - - @Inject DnsCountQueryCoordinator dnsCountQueryCoordinator; - - @Inject - ActivityReportingQueryBuilder() {} + ActivityReportingQueryBuilder( + @Config("projectId") String projectId, + @Named(ICANN_REPORTING_DATA_SET) String icannReportingDataSet, + DnsCountQueryCoordinator dnsCountQueryCoordinator) { + this.projectId = projectId; + this.dnsCountQueryCoordinator = dnsCountQueryCoordinator; + this.icannReportingDataSet = icannReportingDataSet; + } /** Returns the aggregate query which generates the activity report from the saved view. */ @Override public String getReportQuery(YearMonth yearMonth) { return String.format( "#standardSQL\nSELECT * FROM `%s.%s.%s`", - projectId, BIGQUERY_DATA_SET, getTableName(ACTIVITY_REPORT_AGGREGATION, yearMonth)); + projectId, icannReportingDataSet, getTableName(ACTIVITY_REPORT_AGGREGATION, yearMonth)); } /** Sets the month we're doing activity reporting for, and returns the view query map. */ @Override public ImmutableMap getViewQueryMap(YearMonth yearMonth) { - LocalDate firstDayOfMonth = yearMonth.toLocalDate(1); // The pattern-matching is inclusive, so we subtract 1 day to only report that month's data. LocalDate lastDayOfMonth = yearMonth.toLocalDate(1).plusMonths(1).minusDays(1); @@ -102,7 +107,7 @@ public final class ActivityReportingQueryBuilder implements QueryBuilder { String eppQuery = SqlTemplate.create(getQueryFromFile("epp_metrics.sql")) .put("PROJECT_ID", projectId) - .put("ICANN_REPORTING_DATA_SET", BIGQUERY_DATA_SET) + .put("ICANN_REPORTING_DATA_SET", icannReportingDataSet) .put("MONTHLY_LOGS_TABLE", getTableName(MONTHLY_LOGS, yearMonth)) // All metadata logs for reporting come from google.registry.flows.FlowReporter. .put( @@ -114,7 +119,7 @@ public final class ActivityReportingQueryBuilder implements QueryBuilder { String whoisQuery = SqlTemplate.create(getQueryFromFile("whois_counts.sql")) .put("PROJECT_ID", projectId) - .put("ICANN_REPORTING_DATA_SET", BIGQUERY_DATA_SET) + .put("ICANN_REPORTING_DATA_SET", icannReportingDataSet) .put("MONTHLY_LOGS_TABLE", getTableName(MONTHLY_LOGS, yearMonth)) .build(); queriesBuilder.put(getTableName(WHOIS_COUNTS, yearMonth), whoisQuery); @@ -129,7 +134,7 @@ public final class ActivityReportingQueryBuilder implements QueryBuilder { .put( "REGISTRAR_OPERATING_STATUS_TABLE", getTableName(REGISTRAR_OPERATING_STATUS, yearMonth)) - .put("ICANN_REPORTING_DATA_SET", BIGQUERY_DATA_SET) + .put("ICANN_REPORTING_DATA_SET", icannReportingDataSet) .put("DNS_COUNTS_TABLE", getTableName(DNS_COUNTS, yearMonth)) .put("EPP_METRICS_TABLE", getTableName(EPP_METRICS, yearMonth)) .put("WHOIS_COUNTS_TABLE", getTableName(WHOIS_COUNTS, yearMonth)); @@ -147,7 +152,7 @@ public final class ActivityReportingQueryBuilder implements QueryBuilder { return queriesBuilder.build(); } - public void prepareForQuery(YearMonth yearMonth) throws InterruptedException { + void prepareForQuery(YearMonth yearMonth) throws InterruptedException { dnsCountQueryCoordinator.prepareForQuery(yearMonth); } } diff --git a/core/src/main/java/google/registry/reporting/icann/IcannReportingModule.java b/core/src/main/java/google/registry/reporting/icann/IcannReportingModule.java index faf94f27c..5c33b8a20 100644 --- a/core/src/main/java/google/registry/reporting/icann/IcannReportingModule.java +++ b/core/src/main/java/google/registry/reporting/icann/IcannReportingModule.java @@ -14,7 +14,6 @@ package google.registry.reporting.icann; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.request.RequestParameters.extractOptionalParameter; import static google.registry.request.RequestParameters.extractRequiredParameter; import static google.registry.request.RequestParameters.extractSetOfEnumParameters; @@ -24,9 +23,11 @@ import com.google.common.util.concurrent.MoreExecutors; import dagger.Module; import dagger.Provides; import google.registry.bigquery.BigqueryConnection; +import google.registry.persistence.transaction.TransactionManager; import google.registry.request.HttpException.BadRequestException; import google.registry.request.Parameter; import java.util.Optional; +import javax.inject.Named; import javax.servlet.http.HttpServletRequest; import org.joda.time.Duration; @@ -42,8 +43,7 @@ public final class IcannReportingModule { static final String PARAM_SUBDIR = "subdir"; static final String PARAM_REPORT_TYPES = "reportTypes"; - static final String ICANN_REPORTING_DATA_SET = - tm().isOfy() ? "icann_reporting" : "cloud_sql_icann_reporting"; + static final String ICANN_REPORTING_DATA_SET = "icannReportingDataSet"; static final String DATASTORE_EXPORT_DATA_SET = "latest_datastore_export"; static final String MANIFEST_FILE_NAME = "MANIFEST.txt"; @@ -88,11 +88,12 @@ public final class IcannReportingModule { */ @Provides static BigqueryConnection provideBigqueryConnection( - BigqueryConnection.Builder bigQueryConnectionBuilder) { + BigqueryConnection.Builder bigQueryConnectionBuilder, + @Named(ICANN_REPORTING_DATA_SET) String icannReportingDataSet) { try { return bigQueryConnectionBuilder .setExecutorService(MoreExecutors.newDirectExecutorService()) - .setDatasetId(ICANN_REPORTING_DATA_SET) + .setDatasetId(icannReportingDataSet) .setOverwrite(true) .setPollInterval(Duration.standardSeconds(1)) .build(); @@ -100,4 +101,10 @@ public final class IcannReportingModule { throw new RuntimeException("Could not initialize BigqueryConnection!", e); } } + + @Provides + @Named(ICANN_REPORTING_DATA_SET) + static String provideIcannReportingDataSet(TransactionManager tm) { + return tm.isOfy() ? "icann_reporting" : "cloud_sql_icann_reporting"; + } } diff --git a/core/src/main/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java b/core/src/main/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java index 507fe948b..c5a2a2ed1 100644 --- a/core/src/main/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java +++ b/core/src/main/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import google.registry.config.RegistryConfig.Config; import google.registry.util.SqlTemplate; import javax.inject.Inject; +import javax.inject.Named; import org.joda.time.DateTime; import org.joda.time.LocalTime; import org.joda.time.YearMonth; @@ -34,9 +35,16 @@ import org.joda.time.format.DateTimeFormatter; */ public final class TransactionsReportingQueryBuilder implements QueryBuilder { - @Inject @Config("projectId") String projectId; + final String projectId; + private final String icannReportingDataSet; - @Inject TransactionsReportingQueryBuilder() {} + @Inject + TransactionsReportingQueryBuilder( + @Config("projectId") String projectId, + @Named(ICANN_REPORTING_DATA_SET) String icannReportingDataSet) { + this.projectId = projectId; + this.icannReportingDataSet = icannReportingDataSet; + } static final String TRANSACTIONS_REPORT_AGGREGATION = "transactions_report_aggregation"; static final String REGISTRAR_IANA_ID = "registrar_iana_id"; @@ -51,9 +59,7 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder { public String getReportQuery(YearMonth yearMonth) { return String.format( "#standardSQL\nSELECT * FROM `%s.%s.%s`", - projectId, - ICANN_REPORTING_DATA_SET, - getTableName(TRANSACTIONS_REPORT_AGGREGATION, yearMonth)); + projectId, icannReportingDataSet, getTableName(TRANSACTIONS_REPORT_AGGREGATION, yearMonth)); } /** Sets the month we're doing transactions reporting for, and returns the view query map. */ @@ -151,7 +157,7 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder { .put("PROJECT_ID", projectId) .put("DATASTORE_EXPORT_DATA_SET", DATASTORE_EXPORT_DATA_SET) .put("REGISTRY_TABLE", "Registry") - .put("ICANN_REPORTING_DATA_SET", ICANN_REPORTING_DATA_SET) + .put("ICANN_REPORTING_DATA_SET", icannReportingDataSet) .put("REGISTRAR_IANA_ID_TABLE", getTableName(REGISTRAR_IANA_ID, yearMonth)) .put("TOTAL_DOMAINS_TABLE", getTableName(TOTAL_DOMAINS, yearMonth)) .put("TOTAL_NAMESERVERS_TABLE", getTableName(TOTAL_NAMESERVERS, yearMonth)) diff --git a/core/src/test/java/google/registry/beam/initsql/BackupTestStore.java b/core/src/test/java/google/registry/beam/initsql/BackupTestStore.java index 313c8f461..e742c273c 100644 --- a/core/src/test/java/google/registry/beam/initsql/BackupTestStore.java +++ b/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 deletes, Iterable 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; } diff --git a/core/src/test/java/google/registry/model/ofy/CommitLogMutationTest.java b/core/src/test/java/google/registry/model/ofy/CommitLogMutationTest.java index 67d974ff3..23b8f475d 100644 --- a/core/src/test/java/google/registry/model/ofy/CommitLogMutationTest.java +++ b/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)); } } diff --git a/core/src/test/java/google/registry/reporting/icann/ActivityReportingQueryBuilderTest.java b/core/src/test/java/google/registry/reporting/icann/ActivityReportingQueryBuilderTest.java index 95a55b823..9d9780a04 100644 --- a/core/src/test/java/google/registry/reporting/icann/ActivityReportingQueryBuilderTest.java +++ b/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 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 actualQueries = queryBuilder.getViewQueryMap(yearMonth); for (String queryName : expectedQueryNames) { String actualTableName = String.format("%s_201709", queryName); diff --git a/core/src/test/java/google/registry/reporting/icann/IcannReportingModuleTest.java b/core/src/test/java/google/registry/reporting/icann/IcannReportingModuleTest.java index 2d535bc61..03896a6f4 100644 --- a/core/src/test/java/google/registry/reporting/icann/IcannReportingModuleTest.java +++ b/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( diff --git a/core/src/test/java/google/registry/reporting/icann/IcannReportingStagerTest.java b/core/src/test/java/google/registry/reporting/icann/IcannReportingStagerTest.java index bfce9ddfc..627216fa7 100644 --- a/core/src/test/java/google/registry/reporting/icann/IcannReportingStagerTest.java +++ b/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; diff --git a/core/src/test/java/google/registry/reporting/icann/TransactionsReportingQueryBuilderTest.java b/core/src/test/java/google/registry/reporting/icann/TransactionsReportingQueryBuilderTest.java index 275f00b05..0be259e52 100644 --- a/core/src/test/java/google/registry/reporting/icann/TransactionsReportingQueryBuilderTest.java +++ b/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; }