diff --git a/java/google/registry/reporting/icann/ActivityReportingQueryBuilder.java b/java/google/registry/reporting/icann/ActivityReportingQueryBuilder.java index 9aeafe905..aec5b1f05 100644 --- a/java/google/registry/reporting/icann/ActivityReportingQueryBuilder.java +++ b/java/google/registry/reporting/icann/ActivityReportingQueryBuilder.java @@ -16,13 +16,12 @@ package google.registry.reporting.icann; 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; import com.google.common.collect.ImmutableMap; -import com.google.common.io.Resources; import google.registry.config.RegistryConfig.Config; -import google.registry.util.ResourceUtils; import google.registry.util.SqlTemplate; -import java.net.URL; import javax.inject.Inject; import org.joda.time.LocalDate; import org.joda.time.YearMonth; @@ -44,8 +43,6 @@ public final class ActivityReportingQueryBuilder implements QueryBuilder { @Config("projectId") String projectId; - @Inject YearMonth yearMonth; - @Inject DnsCountQueryCoordinator dnsCountQueryCoordinator; @Inject @@ -53,28 +50,19 @@ public final class ActivityReportingQueryBuilder implements QueryBuilder { /** Returns the aggregate query which generates the activity report from the saved view. */ @Override - public String getReportQuery() { + public String getReportQuery(YearMonth yearMonth) { return String.format( "#standardSQL\nSELECT * FROM `%s.%s.%s`", - projectId, ICANN_REPORTING_DATA_SET, getTableName(ACTIVITY_REPORT_AGGREGATION)); + projectId, ICANN_REPORTING_DATA_SET, getTableName(ACTIVITY_REPORT_AGGREGATION, yearMonth)); } /** Sets the month we're doing activity reporting for, and returns the view query map. */ @Override - public ImmutableMap getViewQueryMap() { + 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); - return createQueryMap(firstDayOfMonth, lastDayOfMonth); - } - - public void prepareForQuery() throws Exception { - dnsCountQueryCoordinator.prepareForQuery(); - } - - /** Returns a map from view name to its associated SQL query. */ - private ImmutableMap createQueryMap( - LocalDate firstDayOfMonth, LocalDate lastDayOfMonth) { ImmutableMap.Builder queriesBuilder = ImmutableMap.builder(); String operationalRegistrarsQuery = @@ -83,10 +71,11 @@ public final class ActivityReportingQueryBuilder implements QueryBuilder { .put("DATASTORE_EXPORT_DATA_SET", DATASTORE_EXPORT_DATA_SET) .put("REGISTRAR_TABLE", "Registrar") .build(); - queriesBuilder.put(getTableName(REGISTRAR_OPERATING_STATUS), operationalRegistrarsQuery); + queriesBuilder.put( + getTableName(REGISTRAR_OPERATING_STATUS, yearMonth), operationalRegistrarsQuery); - String dnsCountsQuery = dnsCountQueryCoordinator.createQuery(); - queriesBuilder.put(getTableName(DNS_COUNTS), dnsCountsQuery); + String dnsCountsQuery = dnsCountQueryCoordinator.createQuery(yearMonth); + queriesBuilder.put(getTableName(DNS_COUNTS, yearMonth), dnsCountsQuery); // Convert reportingMonth into YYYYMMDD format for Bigquery table partition pattern-matching. DateTimeFormatter logTableFormatter = DateTimeFormat.forPattern("yyyyMMdd"); @@ -99,55 +88,47 @@ public final class ActivityReportingQueryBuilder implements QueryBuilder { .put("FIRST_DAY_OF_MONTH", logTableFormatter.print(firstDayOfMonth)) .put("LAST_DAY_OF_MONTH", logTableFormatter.print(lastDayOfMonth)) .build(); - queriesBuilder.put(getTableName(MONTHLY_LOGS), monthlyLogsQuery); + queriesBuilder.put(getTableName(MONTHLY_LOGS, yearMonth), monthlyLogsQuery); String eppQuery = SqlTemplate.create(getQueryFromFile("epp_metrics.sql")) .put("PROJECT_ID", projectId) .put("ICANN_REPORTING_DATA_SET", ICANN_REPORTING_DATA_SET) - .put("MONTHLY_LOGS_TABLE", getTableName(MONTHLY_LOGS)) + .put("MONTHLY_LOGS_TABLE", getTableName(MONTHLY_LOGS, yearMonth)) // All metadata logs for reporting come from google.registry.flows.FlowReporter. .put( "METADATA_LOG_PREFIX", "google.registry.flows.FlowReporter recordToLogs: FLOW-LOG-SIGNATURE-METADATA") .build(); - queriesBuilder.put(getTableName(EPP_METRICS), eppQuery); + queriesBuilder.put(getTableName(EPP_METRICS, yearMonth), eppQuery); String whoisQuery = SqlTemplate.create(getQueryFromFile("whois_counts.sql")) .put("PROJECT_ID", projectId) .put("ICANN_REPORTING_DATA_SET", ICANN_REPORTING_DATA_SET) - .put("MONTHLY_LOGS_TABLE", getTableName(MONTHLY_LOGS)) + .put("MONTHLY_LOGS_TABLE", getTableName(MONTHLY_LOGS, yearMonth)) .build(); - queriesBuilder.put(getTableName(WHOIS_COUNTS), whoisQuery); + queriesBuilder.put(getTableName(WHOIS_COUNTS, yearMonth), whoisQuery); String aggregateQuery = SqlTemplate.create(getQueryFromFile("activity_report_aggregation.sql")) .put("PROJECT_ID", projectId) .put("ICANN_REPORTING_DATA_SET", ICANN_REPORTING_DATA_SET) - .put("REGISTRAR_OPERATING_STATUS_TABLE", getTableName(REGISTRAR_OPERATING_STATUS)) - .put("DNS_COUNTS_TABLE", getTableName(DNS_COUNTS)) - .put("EPP_METRICS_TABLE", getTableName(EPP_METRICS)) - .put("WHOIS_COUNTS_TABLE", getTableName(WHOIS_COUNTS)) + .put( + "REGISTRAR_OPERATING_STATUS_TABLE", + getTableName(REGISTRAR_OPERATING_STATUS, yearMonth)) + .put("DNS_COUNTS_TABLE", getTableName(DNS_COUNTS, yearMonth)) + .put("EPP_METRICS_TABLE", getTableName(EPP_METRICS, yearMonth)) + .put("WHOIS_COUNTS_TABLE", getTableName(WHOIS_COUNTS, yearMonth)) .put("DATASTORE_EXPORT_DATA_SET", DATASTORE_EXPORT_DATA_SET) .put("REGISTRY_TABLE", "Registry") .build(); - queriesBuilder.put(getTableName(ACTIVITY_REPORT_AGGREGATION), aggregateQuery); + queriesBuilder.put(getTableName(ACTIVITY_REPORT_AGGREGATION, yearMonth), aggregateQuery); return queriesBuilder.build(); } - /** Returns the table name of the query, suffixed with the yearMonth in _yyyyMM format. */ - private String getTableName(String queryName) { - return String.format("%s_%s", queryName, DateTimeFormat.forPattern("yyyyMM").print(yearMonth)); - } - - /** Returns {@link String} for file in {@code reporting/sql/} directory. */ - private static String getQueryFromFile(String filename) { - return ResourceUtils.readResourceUtf8(getUrl(filename)); - } - - private static URL getUrl(String filename) { - return Resources.getResource(ActivityReportingQueryBuilder.class, "sql/" + filename); + public void prepareForQuery(YearMonth yearMonth) throws Exception { + dnsCountQueryCoordinator.prepareForQuery(yearMonth); } } diff --git a/java/google/registry/reporting/icann/BUILD b/java/google/registry/reporting/icann/BUILD index 19e1e2acc..b50b0ce6e 100644 --- a/java/google/registry/reporting/icann/BUILD +++ b/java/google/registry/reporting/icann/BUILD @@ -14,6 +14,7 @@ java_library( "//java/google/registry/gcs", "//java/google/registry/keyring/api", "//java/google/registry/model", + "//java/google/registry/reporting", "//java/google/registry/request", "//java/google/registry/request/auth", "//java/google/registry/util", diff --git a/java/google/registry/reporting/icann/BasicDnsCountQueryCoordinator.java b/java/google/registry/reporting/icann/BasicDnsCountQueryCoordinator.java index b35150b3f..2b640b7dc 100644 --- a/java/google/registry/reporting/icann/BasicDnsCountQueryCoordinator.java +++ b/java/google/registry/reporting/icann/BasicDnsCountQueryCoordinator.java @@ -17,6 +17,7 @@ package google.registry.reporting.icann; import com.google.common.io.Resources; import google.registry.util.ResourceUtils; import google.registry.util.SqlTemplate; +import org.joda.time.YearMonth; /** * DNS Count query for the basic case. @@ -26,7 +27,7 @@ public class BasicDnsCountQueryCoordinator implements DnsCountQueryCoordinator { BasicDnsCountQueryCoordinator(DnsCountQueryCoordinator.Params params) {} @Override - public String createQuery() { + public String createQuery(YearMonth yearMonth) { return SqlTemplate.create( ResourceUtils.readResourceUtf8( Resources.getResource(this.getClass(), "sql/" + "dns_counts.sql"))) @@ -34,5 +35,5 @@ public class BasicDnsCountQueryCoordinator implements DnsCountQueryCoordinator { } @Override - public void prepareForQuery() throws Exception {} + public void prepareForQuery(YearMonth yearMonth) throws Exception {} } diff --git a/java/google/registry/reporting/icann/DnsCountQueryCoordinator.java b/java/google/registry/reporting/icann/DnsCountQueryCoordinator.java index 1f2f9c10a..c2aaf5680 100644 --- a/java/google/registry/reporting/icann/DnsCountQueryCoordinator.java +++ b/java/google/registry/reporting/icann/DnsCountQueryCoordinator.java @@ -37,22 +37,18 @@ public interface DnsCountQueryCoordinator { public class Params { public BigqueryConnection bigquery; - /** The year and month of the report. */ - public YearMonth yearMonth; - /** The Google Cloud project id. */ public String projectId; - public Params(BigqueryConnection bigquery, YearMonth yearMonth, String projectId) { + public Params(BigqueryConnection bigquery, String projectId) { this.bigquery = bigquery; - this.yearMonth = yearMonth; this.projectId = projectId; } } /** Creates the string used to query bigtable for DNS count information. */ - String createQuery(); + String createQuery(YearMonth yearMonth); /** Do any necessry preparation for the DNS query. */ - void prepareForQuery() throws Exception; + void prepareForQuery(YearMonth yearMonth) throws Exception; } diff --git a/java/google/registry/reporting/icann/DnsCountQueryCoordinatorModule.java b/java/google/registry/reporting/icann/DnsCountQueryCoordinatorModule.java index bbf7241ff..12d989956 100644 --- a/java/google/registry/reporting/icann/DnsCountQueryCoordinatorModule.java +++ b/java/google/registry/reporting/icann/DnsCountQueryCoordinatorModule.java @@ -21,7 +21,6 @@ import dagger.Module; import dagger.Provides; import google.registry.bigquery.BigqueryConnection; import google.registry.config.RegistryConfig.Config; -import org.joda.time.YearMonth; /** Dagger module to provide the DnsCountQueryCoordinator. */ @Module @@ -31,10 +30,9 @@ public class DnsCountQueryCoordinatorModule { static DnsCountQueryCoordinator provideDnsCountQueryCoordinator( @Config("dnsCountQueryCoordinatorClass") String customClass, BigqueryConnection bigquery, - YearMonth yearMonth, @Config("projectId") String projectId) { DnsCountQueryCoordinator.Params params = - new DnsCountQueryCoordinator.Params(bigquery, yearMonth, projectId); + new DnsCountQueryCoordinator.Params(bigquery, projectId); DnsCountQueryCoordinator result = instantiate(getClassFromString(customClass, DnsCountQueryCoordinator.class), params); return result; diff --git a/java/google/registry/reporting/icann/IcannReportingModule.java b/java/google/registry/reporting/icann/IcannReportingModule.java index a11ba597b..82750f802 100644 --- a/java/google/registry/reporting/icann/IcannReportingModule.java +++ b/java/google/registry/reporting/icann/IcannReportingModule.java @@ -14,25 +14,20 @@ package google.registry.reporting.icann; -import static google.registry.request.RequestParameters.extractOptionalEnumParameter; import static google.registry.request.RequestParameters.extractOptionalParameter; -import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static google.registry.request.RequestParameters.extractRequiredParameter; +import static google.registry.request.RequestParameters.extractSetOfEnumParameters; -import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.MoreExecutors; import dagger.Module; import dagger.Provides; import google.registry.bigquery.BigqueryConnection; import google.registry.request.HttpException.BadRequestException; import google.registry.request.Parameter; -import java.lang.annotation.Documented; -import java.lang.annotation.Retention; import java.util.Optional; -import javax.inject.Qualifier; import javax.servlet.http.HttpServletRequest; import org.joda.time.Duration; -import org.joda.time.YearMonth; -import org.joda.time.format.DateTimeFormat; /** Module for dependencies required by ICANN monthly transactions/activity reporting. */ @Module @@ -45,28 +40,26 @@ public final class IcannReportingModule { } static final String PARAM_SUBDIR = "subdir"; - static final String PARAM_REPORT_TYPE = "reportType"; + static final String PARAM_REPORT_TYPES = "reportTypes"; static final String ICANN_REPORTING_DATA_SET = "icann_reporting"; static final String DATASTORE_EXPORT_DATA_SET = "latest_datastore_export"; static final String MANIFEST_FILE_NAME = "MANIFEST.txt"; - private static final String DEFAULT_SUBDIR = "icann/monthly"; /** Provides an optional subdirectory to store/upload reports to, extracted from the request. */ @Provides @Parameter(PARAM_SUBDIR) static Optional provideSubdirOptional(HttpServletRequest req) { - return extractOptionalParameter(req, PARAM_SUBDIR); + return extractOptionalParameter(req, PARAM_SUBDIR).map(IcannReportingModule::checkSubdirValid); } - /** Provides the subdirectory to store/upload reports to, defaults to icann/monthly/yearMonth. */ + /** Provides the subdirectory to store/upload reports to, extracted from the request. */ @Provides - @ReportingSubdir - static String provideSubdir( - @Parameter(PARAM_SUBDIR) Optional subdirOptional, YearMonth yearMonth) { - String subdir = - subdirOptional.orElse( - String.format( - "%s/%s", DEFAULT_SUBDIR, DateTimeFormat.forPattern("yyyy-MM").print(yearMonth))); + @Parameter(PARAM_SUBDIR) + static String provideSubdir(HttpServletRequest req) { + return checkSubdirValid(extractRequiredParameter(req, PARAM_SUBDIR)); + } + + static String checkSubdirValid(String subdir) { if (subdir.startsWith("/") || subdir.endsWith("/")) { throw new BadRequestException( String.format("subdir must not start or end with a \"/\", got %s instead.", subdir)); @@ -76,17 +69,11 @@ public final class IcannReportingModule { /** Provides an optional reportType to store/upload reports to, extracted from the request. */ @Provides - @Parameter(PARAM_REPORT_TYPE) - static Optional provideReportTypeOptional(HttpServletRequest req) { - return extractOptionalEnumParameter(req, ReportType.class, PARAM_REPORT_TYPE); - } - - /** Provides a list of reportTypes specified. If absent, we default to both report types. */ - @Provides - static ImmutableList provideReportTypes( - @Parameter(PARAM_REPORT_TYPE) Optional reportTypeOptional) { - return reportTypeOptional.map(ImmutableList::of) - .orElseGet(() -> ImmutableList.of(ReportType.ACTIVITY, ReportType.TRANSACTIONS)); + @Parameter(PARAM_REPORT_TYPES) + static ImmutableSet provideReportTypes(HttpServletRequest req) { + ImmutableSet reportTypes = + extractSetOfEnumParameters(req, ReportType.class, PARAM_REPORT_TYPES); + return reportTypes.isEmpty() ? ImmutableSet.copyOf(ReportType.values()) : reportTypes; } /** @@ -113,11 +100,4 @@ public final class IcannReportingModule { throw new RuntimeException("Could not initialize BigqueryConnection!", e); } } - - /** Dagger qualifier for the subdirectory we stage to/upload from. */ - @Qualifier - @Documented - @Retention(RUNTIME) - public @interface ReportingSubdir {} } - diff --git a/java/google/registry/reporting/icann/IcannReportingStager.java b/java/google/registry/reporting/icann/IcannReportingStager.java index 1f998ad61..65e908b5b 100644 --- a/java/google/registry/reporting/icann/IcannReportingStager.java +++ b/java/google/registry/reporting/icann/IcannReportingStager.java @@ -36,7 +36,6 @@ import google.registry.bigquery.BigqueryUtils.TableType; import google.registry.config.RegistryConfig.Config; import google.registry.gcs.GcsUtils; import google.registry.reporting.icann.IcannReportingModule.ReportType; -import google.registry.reporting.icann.IcannReportingModule.ReportingSubdir; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -62,10 +61,6 @@ public class IcannReportingStager { @Inject @Config("reportingBucket") String reportingBucket; - @Inject YearMonth yearMonth; - @Inject @ReportingSubdir - String subdir; - @Inject ActivityReportingQueryBuilder activityQueryBuilder; @Inject TransactionsReportingQueryBuilder transactionsQueryBuilder; @Inject GcsUtils gcsUtils; @@ -79,16 +74,17 @@ public class IcannReportingStager { * *

This is factored out to facilitate choosing which reports to upload, */ - ImmutableList stageReports(ReportType reportType) throws Exception { + ImmutableList stageReports(YearMonth yearMonth, String subdir, ReportType reportType) + throws Exception { QueryBuilder queryBuilder = (reportType == ReportType.ACTIVITY) ? activityQueryBuilder : transactionsQueryBuilder; if (reportType == ReportType.ACTIVITY) { // Prepare for the DNS count query, which may have special needs. - activityQueryBuilder.prepareForQuery(); + activityQueryBuilder.prepareForQuery(yearMonth); } - ImmutableMap viewQueryMap = queryBuilder.getViewQueryMap(); + ImmutableMap viewQueryMap = queryBuilder.getViewQueryMap(yearMonth); // Generate intermediary views for (Entry entry : viewQueryMap.entrySet()) { createIntermediaryTableView(entry.getKey(), entry.getValue(), reportType); @@ -96,14 +92,14 @@ public class IcannReportingStager { // Get an in-memory table of the aggregate query's result ImmutableTable reportTable = - bigquery.queryToLocalTableSync(queryBuilder.getReportQuery()); + bigquery.queryToLocalTableSync(queryBuilder.getReportQuery(yearMonth)); // Get report headers from the table schema and convert into CSV format String headerRow = constructRow(getHeaders(reportTable.columnKeySet())); return (reportType == ReportType.ACTIVITY) - ? stageActivityReports(headerRow, reportTable.rowMap().values()) - : stageTransactionsReports(headerRow, reportTable.rowMap().values()); + ? stageActivityReports(yearMonth, subdir, headerRow, reportTable.rowMap().values()) + : stageTransactionsReports(yearMonth, subdir, headerRow, reportTable.rowMap().values()); } private void createIntermediaryTableView(String queryName, String query, ReportType reportType) @@ -129,7 +125,10 @@ public class IcannReportingStager { /** Creates and stores activity reports on GCS, returns a list of files stored. */ private ImmutableList stageActivityReports( - String headerRow, ImmutableCollection> rows) + YearMonth yearMonth, + String subdir, + String headerRow, + ImmutableCollection> rows) throws IOException { ImmutableList.Builder manifestBuilder = new ImmutableList.Builder<>(); // Create a report csv for each tld from query table, and upload to GCS @@ -142,14 +141,18 @@ public class IcannReportingStager { ImmutableList rowStrings = ImmutableList.of(constructRow(row.values())); // Create and upload the activity report with a single row manifestBuilder.add( - saveReportToGcs(tld, createReport(headerRow, rowStrings), ReportType.ACTIVITY)); + saveReportToGcs( + tld, yearMonth, subdir, createReport(headerRow, rowStrings), ReportType.ACTIVITY)); } return manifestBuilder.build(); } /** Creates and stores transactions reports on GCS, returns a list of files stored. */ private ImmutableList stageTransactionsReports( - String headerRow, ImmutableCollection> rows) + YearMonth yearMonth, + String subdir, + String headerRow, + ImmutableCollection> rows) throws IOException { // Map from tld to rows ListMultimap tldToRows = ArrayListMultimap.create(); @@ -175,7 +178,11 @@ public class IcannReportingStager { tldToRows.put(tld, constructTotalRow(tldToTotals.get(tld))); manifestBuilder.add( saveReportToGcs( - tld, createReport(headerRow, tldToRows.get(tld)), ReportType.TRANSACTIONS)); + tld, + yearMonth, + subdir, + createReport(headerRow, tldToRows.get(tld)), + ReportType.TRANSACTIONS)); } return manifestBuilder.build(); } @@ -238,7 +245,8 @@ public class IcannReportingStager { } /** Stores a report on GCS, returning the name of the file stored. */ - private String saveReportToGcs(String tld, String reportCsv, ReportType reportType) + private String saveReportToGcs( + String tld, YearMonth yearMonth, String subdir, String reportCsv, ReportType reportType) throws IOException { // Upload resulting CSV file to GCS byte[] reportBytes = reportCsv.getBytes(UTF_8); @@ -256,7 +264,7 @@ public class IcannReportingStager { } /** Creates and stores a manifest file on GCS, indicating which reports were generated. */ - void createAndUploadManifest(ImmutableList filenames) throws IOException { + void createAndUploadManifest(String subdir, ImmutableList filenames) throws IOException { String reportBucketname = String.format("%s/%s", reportingBucket, subdir); final GcsFilename gcsFilename = new GcsFilename(reportBucketname, MANIFEST_FILE_NAME); StringBuilder manifestString = new StringBuilder(); diff --git a/java/google/registry/reporting/icann/IcannReportingStagingAction.java b/java/google/registry/reporting/icann/IcannReportingStagingAction.java index 9012257f5..c5595a05c 100644 --- a/java/google/registry/reporting/icann/IcannReportingStagingAction.java +++ b/java/google/registry/reporting/icann/IcannReportingStagingAction.java @@ -15,6 +15,8 @@ package google.registry.reporting.icann; import static com.google.common.base.Throwables.getRootCause; +import static google.registry.reporting.icann.IcannReportingModule.PARAM_REPORT_TYPES; +import static google.registry.reporting.icann.IcannReportingModule.PARAM_SUBDIR; import static google.registry.request.Action.Method.POST; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import static javax.servlet.http.HttpServletResponse.SC_OK; @@ -24,18 +26,21 @@ import com.google.appengine.api.taskqueue.TaskOptions; import com.google.appengine.api.taskqueue.TaskOptions.Method; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; import com.google.common.net.MediaType; import google.registry.bigquery.BigqueryJobFailureException; import google.registry.reporting.icann.IcannReportingModule.ReportType; -import google.registry.reporting.icann.IcannReportingModule.ReportingSubdir; import google.registry.request.Action; +import google.registry.request.Parameter; import google.registry.request.Response; import google.registry.request.auth.Auth; import google.registry.util.Retrier; +import java.util.Optional; import javax.inject.Inject; import org.joda.time.Duration; import org.joda.time.YearMonth; +import org.joda.time.format.DateTimeFormat; /** * Action that generates monthly ICANN activity and transactions reports. @@ -53,7 +58,7 @@ import org.joda.time.YearMonth; * to "icann/monthly/[yearMonth]". * *

reportTypes: the type of reports to generate. You can specify either 'activity' or - * 'transactions'. Defaults to generating both. + * 'transactions'. If none specified - defaults to generating both. */ @Action( service = Action.Service.BACKEND, @@ -66,10 +71,11 @@ public final class IcannReportingStagingAction implements Runnable { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final String CRON_QUEUE = "retryable-cron-tasks"; + private static final String DEFAULT_SUBDIR = "icann/monthly"; @Inject YearMonth yearMonth; - @Inject @ReportingSubdir String subdir; - @Inject ImmutableList reportTypes; + @Inject @Parameter(PARAM_SUBDIR) Optional overrideSubdir; + @Inject @Parameter(PARAM_REPORT_TYPES) ImmutableSet reportTypes; @Inject IcannReportingStager stager; @Inject Retrier retrier; @Inject Response response; @@ -79,14 +85,15 @@ public final class IcannReportingStagingAction implements Runnable { @Override public void run() { try { + String subdir = getSubdir(yearMonth); retrier.callWithRetry( () -> { ImmutableList.Builder manifestedFilesBuilder = new ImmutableList.Builder<>(); for (ReportType reportType : reportTypes) { - manifestedFilesBuilder.addAll(stager.stageReports(reportType)); + manifestedFilesBuilder.addAll(stager.stageReports(yearMonth, subdir, reportType)); } ImmutableList manifestedFiles = manifestedFilesBuilder.build(); - stager.createAndUploadManifest(manifestedFiles); + stager.createAndUploadManifest(subdir, manifestedFiles); logger.atInfo().log("Completed staging %d report files.", manifestedFiles.size()); emailUtils.emailResults( @@ -104,7 +111,7 @@ public final class IcannReportingStagingAction implements Runnable { TaskOptions.Builder.withUrl(IcannReportingUploadAction.PATH) .method(Method.POST) .countdownMillis(Duration.standardMinutes(2).getMillis()) - .param(IcannReportingModule.PARAM_SUBDIR, subdir); + .param(PARAM_SUBDIR, subdir); QueueFactory.getQueue(CRON_QUEUE).add(uploadTask); return null; }, @@ -124,4 +131,11 @@ public final class IcannReportingStagingAction implements Runnable { throw new RuntimeException("Staging action failed.", e); } } + + String getSubdir(YearMonth yearMonth) { + return IcannReportingModule.checkSubdirValid( + overrideSubdir.orElse( + String.format( + "%s/%s", DEFAULT_SUBDIR, DateTimeFormat.forPattern("yyyy-MM").print(yearMonth)))); + } } diff --git a/java/google/registry/reporting/icann/IcannReportingUploadAction.java b/java/google/registry/reporting/icann/IcannReportingUploadAction.java index fa01d594b..e85522f79 100644 --- a/java/google/registry/reporting/icann/IcannReportingUploadAction.java +++ b/java/google/registry/reporting/icann/IcannReportingUploadAction.java @@ -17,6 +17,7 @@ package google.registry.reporting.icann; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8; import static google.registry.reporting.icann.IcannReportingModule.MANIFEST_FILE_NAME; +import static google.registry.reporting.icann.IcannReportingModule.PARAM_SUBDIR; import static google.registry.request.Action.Method.POST; import static java.nio.charset.StandardCharsets.UTF_8; import static javax.servlet.http.HttpServletResponse.SC_OK; @@ -29,8 +30,8 @@ import com.google.common.flogger.FluentLogger; import com.google.common.io.ByteStreams; import google.registry.config.RegistryConfig.Config; import google.registry.gcs.GcsUtils; -import google.registry.reporting.icann.IcannReportingModule.ReportingSubdir; import google.registry.request.Action; +import google.registry.request.Parameter; import google.registry.request.Response; import google.registry.request.auth.Auth; import google.registry.util.Retrier; @@ -67,8 +68,7 @@ public final class IcannReportingUploadAction implements Runnable { @Config("reportingBucket") String reportingBucket; - @Inject @ReportingSubdir - String subdir; + @Inject @Parameter(PARAM_SUBDIR) String subdir; @Inject GcsUtils gcsUtils; @Inject IcannHttpReporter icannReporter; diff --git a/java/google/registry/reporting/icann/QueryBuilder.java b/java/google/registry/reporting/icann/QueryBuilder.java index 0b770494c..6c3ad00da 100644 --- a/java/google/registry/reporting/icann/QueryBuilder.java +++ b/java/google/registry/reporting/icann/QueryBuilder.java @@ -15,13 +15,14 @@ package google.registry.reporting.icann; import com.google.common.collect.ImmutableMap; +import org.joda.time.YearMonth; /** Interface defining the necessary methods to construct ICANN reporting SQL queries. */ public interface QueryBuilder { /** Returns a map from an intermediary view's table name to the query that generates it. */ - ImmutableMap getViewQueryMap(); + ImmutableMap getViewQueryMap(YearMonth yearMonth); /** Returns a query that retrieves the overall report from the previously generated view. */ - String getReportQuery(); + String getReportQuery(YearMonth yearMonth); } diff --git a/java/google/registry/reporting/icann/QueryBuilderUtils.java b/java/google/registry/reporting/icann/QueryBuilderUtils.java new file mode 100644 index 000000000..6a25de06c --- /dev/null +++ b/java/google/registry/reporting/icann/QueryBuilderUtils.java @@ -0,0 +1,40 @@ +// Copyright 2019 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.reporting.icann; + +import com.google.common.io.Resources; +import google.registry.util.ResourceUtils; +import java.net.URL; +import org.joda.time.YearMonth; +import org.joda.time.format.DateTimeFormat; + +final class QueryBuilderUtils { + + private QueryBuilderUtils() {} + + /** Returns the table name of the query, suffixed with the yearMonth in _yyyyMM format. */ + static String getTableName(String queryName, YearMonth yearMonth) { + return String.format("%s_%s", queryName, DateTimeFormat.forPattern("yyyyMM").print(yearMonth)); + } + + /** Returns {@link String} for file in {@code reporting/sql/} directory. */ + static String getQueryFromFile(String filename) { + return ResourceUtils.readResourceUtf8(getUrl(filename)); + } + + private static URL getUrl(String filename) { + return Resources.getResource(QueryBuilderUtils.class, "sql/" + filename); + } +} diff --git a/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java b/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java index 1642c9986..507fe948b 100644 --- a/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java +++ b/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java @@ -16,13 +16,12 @@ package google.registry.reporting.icann; 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; import com.google.common.collect.ImmutableMap; -import com.google.common.io.Resources; import google.registry.config.RegistryConfig.Config; -import google.registry.util.ResourceUtils; import google.registry.util.SqlTemplate; -import java.net.URL; import javax.inject.Inject; import org.joda.time.DateTime; import org.joda.time.LocalTime; @@ -37,8 +36,6 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder { @Inject @Config("projectId") String projectId; - @Inject YearMonth yearMonth; - @Inject TransactionsReportingQueryBuilder() {} static final String TRANSACTIONS_REPORT_AGGREGATION = "transactions_report_aggregation"; @@ -51,27 +48,21 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder { /** Returns the aggregate query which generates the transactions report from the saved view. */ @Override - public String getReportQuery() { + public String getReportQuery(YearMonth yearMonth) { return String.format( "#standardSQL\nSELECT * FROM `%s.%s.%s`", projectId, ICANN_REPORTING_DATA_SET, - getTableName(TRANSACTIONS_REPORT_AGGREGATION)); + getTableName(TRANSACTIONS_REPORT_AGGREGATION, yearMonth)); } /** Sets the month we're doing transactions reporting for, and returns the view query map. */ @Override - public ImmutableMap getViewQueryMap() { + public ImmutableMap getViewQueryMap(YearMonth yearMonth) { // Set the earliest date to to yearMonth on day 1 at 00:00:00 DateTime earliestReportTime = yearMonth.toLocalDate(1).toDateTime(new LocalTime(0, 0, 0)); // Set the latest date to yearMonth on the last day at 23:59:59.999 DateTime latestReportTime = earliestReportTime.plusMonths(1).minusMillis(1); - return createQueryMap(earliestReportTime, latestReportTime); - } - - /** Returns a map from view name to its associated SQL query. */ - private ImmutableMap createQueryMap( - DateTime earliestReportTime, DateTime latestReportTime) { ImmutableMap.Builder queriesBuilder = ImmutableMap.builder(); String registrarIanaIdQuery = @@ -80,7 +71,7 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder { .put("DATASTORE_EXPORT_DATA_SET", DATASTORE_EXPORT_DATA_SET) .put("REGISTRAR_TABLE", "Registrar") .build(); - queriesBuilder.put(getTableName(REGISTRAR_IANA_ID), registrarIanaIdQuery); + queriesBuilder.put(getTableName(REGISTRAR_IANA_ID, yearMonth), registrarIanaIdQuery); String totalDomainsQuery = SqlTemplate.create(getQueryFromFile("total_domains.sql")) @@ -89,7 +80,7 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder { .put("DOMAINBASE_TABLE", "DomainBase") .put("REGISTRAR_TABLE", "Registrar") .build(); - queriesBuilder.put(getTableName(TOTAL_DOMAINS), totalDomainsQuery); + queriesBuilder.put(getTableName(TOTAL_DOMAINS, yearMonth), totalDomainsQuery); DateTimeFormatter timestampFormatter = DateTimeFormat.forPattern("yyyy-MM-dd HH:mm:ss.SSS"); String totalNameserversQuery = @@ -101,7 +92,7 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder { .put("REGISTRAR_TABLE", "Registrar") .put("LATEST_REPORT_TIME", timestampFormatter.print(latestReportTime)) .build(); - queriesBuilder.put(getTableName(TOTAL_NAMESERVERS), totalNameserversQuery); + queriesBuilder.put(getTableName(TOTAL_NAMESERVERS, yearMonth), totalNameserversQuery); String transactionCountsQuery = SqlTemplate.create(getQueryFromFile("transaction_counts.sql")) @@ -117,7 +108,7 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder { .put("TRANSFER_NACKED_FIELD", "TRANSFER_GAINING_NACKED") .put("DEFAULT_FIELD", "field") .build(); - queriesBuilder.put(getTableName(TRANSACTION_COUNTS), transactionCountsQuery); + queriesBuilder.put(getTableName(TRANSACTION_COUNTS, yearMonth), transactionCountsQuery); String transactionTransferLosingQuery = SqlTemplate.create(getQueryFromFile("transaction_counts.sql")) @@ -134,7 +125,8 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder { .put("TRANSFER_NACKED_FIELD", "TRANSFER_LOSING_NACKED") .put("DEFAULT_FIELD", "NULL") .build(); - queriesBuilder.put(getTableName(TRANSACTION_TRANSFER_LOSING), transactionTransferLosingQuery); + queriesBuilder.put( + getTableName(TRANSACTION_TRANSFER_LOSING, yearMonth), transactionTransferLosingQuery); // App Engine log table suffixes use YYYYMMDD format DateTimeFormatter logTableFormatter = DateTimeFormat.forPattern("yyyyMMdd"); @@ -152,7 +144,7 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder { "METADATA_LOG_PREFIX", "google.registry.flows.FlowReporter recordToLogs: FLOW-LOG-SIGNATURE-METADATA") .build(); - queriesBuilder.put(getTableName(ATTEMPTED_ADDS), attemptedAddsQuery); + queriesBuilder.put(getTableName(ATTEMPTED_ADDS, yearMonth), attemptedAddsQuery); String aggregateQuery = SqlTemplate.create(getQueryFromFile("transactions_report_aggregation.sql")) @@ -160,31 +152,17 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder { .put("DATASTORE_EXPORT_DATA_SET", DATASTORE_EXPORT_DATA_SET) .put("REGISTRY_TABLE", "Registry") .put("ICANN_REPORTING_DATA_SET", ICANN_REPORTING_DATA_SET) - .put("REGISTRAR_IANA_ID_TABLE", getTableName(REGISTRAR_IANA_ID)) - .put("TOTAL_DOMAINS_TABLE", getTableName(TOTAL_DOMAINS)) - .put("TOTAL_NAMESERVERS_TABLE", getTableName(TOTAL_NAMESERVERS)) - .put("TRANSACTION_COUNTS_TABLE", getTableName(TRANSACTION_COUNTS)) - .put("TRANSACTION_TRANSFER_LOSING_TABLE", getTableName(TRANSACTION_TRANSFER_LOSING)) - .put("ATTEMPTED_ADDS_TABLE", getTableName(ATTEMPTED_ADDS)) + .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)) + .put("TRANSACTION_COUNTS_TABLE", getTableName(TRANSACTION_COUNTS, yearMonth)) + .put( + "TRANSACTION_TRANSFER_LOSING_TABLE", + getTableName(TRANSACTION_TRANSFER_LOSING, yearMonth)) + .put("ATTEMPTED_ADDS_TABLE", getTableName(ATTEMPTED_ADDS, yearMonth)) .build(); - queriesBuilder.put(getTableName(TRANSACTIONS_REPORT_AGGREGATION), aggregateQuery); + queriesBuilder.put(getTableName(TRANSACTIONS_REPORT_AGGREGATION, yearMonth), aggregateQuery); return queriesBuilder.build(); } - - /** Returns the table name of the query, suffixed with the yearMonth in _yyyyMM format. */ - private String getTableName(String queryName) { - return String.format("%s_%s", queryName, DateTimeFormat.forPattern("yyyyMM").print(yearMonth)); - } - - /** Returns {@link String} for file in {@code reporting/sql/} directory. */ - private static String getQueryFromFile(String filename) { - return ResourceUtils.readResourceUtf8(getUrl(filename)); - } - - private static URL getUrl(String filename) { - return Resources.getResource( - ActivityReportingQueryBuilder.class, "sql/" + filename); - } } - diff --git a/java/google/registry/request/RequestParameters.java b/java/google/registry/request/RequestParameters.java index db7cb6c7f..90af2d61b 100644 --- a/java/google/registry/request/RequestParameters.java +++ b/java/google/registry/request/RequestParameters.java @@ -96,7 +96,7 @@ public final class RequestParameters { } /** - * Returns all GET or POST parameters associated with {@code name} (or {@code nameLegacy}). + * Returns all GET or POST parameters associated with {@code name}. * *

The parameter value is assumed to be a comma-delimited set of values - so tlds=com,net would * result in ImmutableSet.of("com", "net"). @@ -109,36 +109,49 @@ public final class RequestParameters { * @param name the name of the parameter, should be in plural form (e.g. tlds=, not tld=) */ public static ImmutableSet extractSetOfParameters(HttpServletRequest req, String name) { + // First we make sure the user didn't accidentally try to pass the "set of parameters" as + // multiple tld=a&tld=b parameters instead of tld=a,b String[] parameters = req.getParameterValues(name); - if (parameters == null || parameters.length == 0) { - return ImmutableSet.of(); - } - if (parameters.length > 1) { + if (parameters != null && parameters.length > 1) { throw new BadRequestException( String.format( "Bad 'set of parameters' input! Received multiple values instead of single " + "comma-delimited value for parameter %s", name)); } + // Now we parse the single parameter. + // We use the req.getParameter(name) instead of parameters[0] to make tests more consistent (all + // extractXxx read the data from req.getParameter, so mocking the parameter is consistent) + String parameter = req.getParameter(name); + if (parameter == null || parameter.isEmpty()) { + return ImmutableSet.of(); + } return Splitter.on(',') - .splitToList(parameters[0]) + .splitToList(parameter) .stream() .filter(s -> !s.isEmpty()) .collect(toImmutableSet()); } /** - * Returns the first GET or POST parameter associated with {@code name}, absent otherwise. + * Returns all GET or POST parameters associated with {@code name}. * - * @throws BadRequestException if request parameter named {@code name} is not equal to any of the - * values in {@code enumClass} + *

The parameter value is assumed to be a comma-delimited set of values - so tlds=com,net would + * result in ImmutableSet.of("com", "net"). + * + *

Empty strings are not supported, and are automatically removed from the result. + * + *

Both missing parameter and parameter with empty value result in an empty set. + * + * @param req the request that has the parameter + * @param enumClass the Class of the expected Enum type + * @param name the name of the parameter, should be in plural form (e.g. tlds=, not tld=) + * @throws BadRequestException if any of the comma-delimited values of the request parameter named + * {@code name} aren't equal to any of the values in {@code enumClass} */ public static > Optional extractOptionalEnumParameter( HttpServletRequest req, Class enumClass, String name) { - String stringParam = req.getParameter(name); - return isNullOrEmpty(stringParam) - ? Optional.empty() - : Optional.of(extractEnumParameter(req, enumClass, name)); + return extractOptionalParameter(req, name).map(value -> getEnumValue(enumClass, value, name)); } /** @@ -149,11 +162,32 @@ public final class RequestParameters { */ public static > C extractEnumParameter(HttpServletRequest req, Class enumClass, String name) { + return getEnumValue(enumClass, extractRequiredParameter(req, name), name); + } + + /** + * Returns the first GET or POST parameter associated with {@code name}. + * + * @throws BadRequestException if request parameter named {@code name} is absent, empty, or not + * equal to any of the values in {@code enumClass} + */ + public static > ImmutableSet extractSetOfEnumParameters( + HttpServletRequest req, Class enumClass, String name) { + return extractSetOfParameters(req, name).stream() + .map(value -> getEnumValue(enumClass, value, name)) + .collect(toImmutableSet()); + } + + /** Translates a string name into the enum value, or throws a BadRequestException. */ + private static > C getEnumValue( + Class enumClass, String value, String parameterName) { try { - return Enum.valueOf(enumClass, Ascii.toUpperCase(extractRequiredParameter(req, name))); + return Enum.valueOf(enumClass, Ascii.toUpperCase(value)); } catch (IllegalArgumentException e) { throw new BadRequestException( - String.format("Invalid %s parameter: %s", enumClass.getSimpleName(), name)); + String.format( + "Invalid parameter %s: expected enum of type %s, but got '%s'", + parameterName, enumClass.getSimpleName(), value)); } } diff --git a/javatests/google/registry/reporting/icann/ActivityReportingQueryBuilderTest.java b/javatests/google/registry/reporting/icann/ActivityReportingQueryBuilderTest.java index 21e4ac9ce..6469da854 100644 --- a/javatests/google/registry/reporting/icann/ActivityReportingQueryBuilderTest.java +++ b/javatests/google/registry/reporting/icann/ActivityReportingQueryBuilderTest.java @@ -27,21 +27,21 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class ActivityReportingQueryBuilderTest { + private final YearMonth yearMonth = new YearMonth(2017, 9); + private ActivityReportingQueryBuilder getQueryBuilder() { ActivityReportingQueryBuilder queryBuilder = new ActivityReportingQueryBuilder(); - queryBuilder.yearMonth = new YearMonth(2017, 9); queryBuilder.projectId = "domain-registry-alpha"; queryBuilder.dnsCountQueryCoordinator = new BasicDnsCountQueryCoordinator( - new BasicDnsCountQueryCoordinator.Params(null, queryBuilder.yearMonth, - queryBuilder.projectId)); + new BasicDnsCountQueryCoordinator.Params(null, queryBuilder.projectId)); return queryBuilder; } @Test public void testAggregateQueryMatch() { ActivityReportingQueryBuilder queryBuilder = getQueryBuilder(); - assertThat(queryBuilder.getReportQuery()) + assertThat(queryBuilder.getReportQuery(yearMonth)) .isEqualTo( "#standardSQL\nSELECT * FROM " + "`domain-registry-alpha.icann_reporting.activity_report_aggregation_201709`"); @@ -59,7 +59,7 @@ public class ActivityReportingQueryBuilderTest { ActivityReportingQueryBuilder.ACTIVITY_REPORT_AGGREGATION); ActivityReportingQueryBuilder queryBuilder = getQueryBuilder(); - ImmutableMap actualQueries = queryBuilder.getViewQueryMap(); + ImmutableMap actualQueries = queryBuilder.getViewQueryMap(yearMonth); for (String queryName : expectedQueryNames) { String actualTableName = String.format("%s_201709", queryName); String testFilename = String.format("%s_test.sql", queryName); diff --git a/javatests/google/registry/reporting/icann/BUILD b/javatests/google/registry/reporting/icann/BUILD index 7a2f5fbd3..4633112f9 100644 --- a/javatests/google/registry/reporting/icann/BUILD +++ b/javatests/google/registry/reporting/icann/BUILD @@ -26,6 +26,7 @@ java_library( "@com_google_http_client", "@com_google_truth", "@com_google_truth_extensions_truth_java8_extension", + "@javax_servlet_api", "@joda_time", "@junit", "@org_mockito_all", diff --git a/javatests/google/registry/reporting/icann/IcannReportingModuleTest.java b/javatests/google/registry/reporting/icann/IcannReportingModuleTest.java index ca0e14c2d..c8dee5a6a 100644 --- a/javatests/google/registry/reporting/icann/IcannReportingModuleTest.java +++ b/javatests/google/registry/reporting/icann/IcannReportingModuleTest.java @@ -15,12 +15,10 @@ package google.registry.reporting.icann; import static com.google.common.truth.Truth.assertThat; -import static google.registry.testing.JUnitBackports.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; -import google.registry.reporting.icann.IcannReportingModule.ReportType; -import google.registry.request.HttpException.BadRequestException; -import java.util.Optional; -import org.joda.time.YearMonth; +import javax.servlet.http.HttpServletRequest; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -30,39 +28,30 @@ import org.junit.runners.JUnit4; public class IcannReportingModuleTest { @Test - public void testEmptySubDir_returnsDefaultSubdir() { - assertThat(IcannReportingModule.provideSubdir(Optional.empty(), new YearMonth(2017, 6))) - .isEqualTo("icann/monthly/2017-06"); - } + public void testProvideReportTypes() { + HttpServletRequest req = mock(HttpServletRequest.class); - @Test - public void testGivenSubdir_returnsManualSubdir() { - assertThat( - IcannReportingModule.provideSubdir(Optional.of("manual/dir"), new YearMonth(2017, 6))) - .isEqualTo("manual/dir"); - } + when(req.getParameter("reportTypes")).thenReturn(null); + assertThat(IcannReportingModule.provideReportTypes(req)) + .containsExactly( + IcannReportingModule.ReportType.ACTIVITY, IcannReportingModule.ReportType.TRANSACTIONS); - @Test - public void testInvalidSubdir_throwsException() { - BadRequestException thrown = - assertThrows( - BadRequestException.class, - () -> - IcannReportingModule.provideSubdir(Optional.of("/whoops"), new YearMonth(2017, 6))); - assertThat(thrown) - .hasMessageThat() - .contains("subdir must not start or end with a \"/\", got /whoops instead."); - } + when(req.getParameter("reportTypes")).thenReturn(""); + assertThat(IcannReportingModule.provideReportTypes(req)) + .containsExactly( + IcannReportingModule.ReportType.ACTIVITY, IcannReportingModule.ReportType.TRANSACTIONS); - @Test - public void testGivenReportType_returnsReportType() { - assertThat(IcannReportingModule.provideReportTypes(Optional.of(ReportType.ACTIVITY))) - .containsExactly(ReportType.ACTIVITY); - } + when(req.getParameter("reportTypes")).thenReturn("activity"); + assertThat(IcannReportingModule.provideReportTypes(req)) + .containsExactly(IcannReportingModule.ReportType.ACTIVITY); - @Test - public void testNoReportType_returnsBothReportTypes() { - assertThat(IcannReportingModule.provideReportTypes(Optional.empty())) - .containsExactly(ReportType.ACTIVITY, ReportType.TRANSACTIONS); + when(req.getParameter("reportTypes")).thenReturn("transactions"); + assertThat(IcannReportingModule.provideReportTypes(req)) + .containsExactly(IcannReportingModule.ReportType.TRANSACTIONS); + + when(req.getParameter("reportTypes")).thenReturn("activity,transactions"); + assertThat(IcannReportingModule.provideReportTypes(req)) + .containsExactly( + IcannReportingModule.ReportType.ACTIVITY, IcannReportingModule.ReportType.TRANSACTIONS); } } diff --git a/javatests/google/registry/reporting/icann/IcannReportingStagerTest.java b/javatests/google/registry/reporting/icann/IcannReportingStagerTest.java index 57fd56590..779927c50 100644 --- a/javatests/google/registry/reporting/icann/IcannReportingStagerTest.java +++ b/javatests/google/registry/reporting/icann/IcannReportingStagerTest.java @@ -50,6 +50,8 @@ public class IcannReportingStagerTest { BigqueryConnection bigquery = mock(BigqueryConnection.class); FakeResponse response = new FakeResponse(); GcsService gcsService = GcsServiceFactory.createGcsService(); + YearMonth yearMonth = new YearMonth(2017, 6); + String subdir = "icann/monthly/2017-06"; @Rule public final AppEngineRule appEngine = AppEngineRule.builder() @@ -61,16 +63,12 @@ public class IcannReportingStagerTest { IcannReportingStager action = new IcannReportingStager(); ActivityReportingQueryBuilder activityBuilder = new ActivityReportingQueryBuilder(); activityBuilder.projectId = "test-project"; - activityBuilder.yearMonth = new YearMonth(2017, 6); activityBuilder.dnsCountQueryCoordinator = new BasicDnsCountQueryCoordinator(null); action.activityQueryBuilder = activityBuilder; TransactionsReportingQueryBuilder transactionsBuilder = new TransactionsReportingQueryBuilder(); transactionsBuilder.projectId = "test-project"; - transactionsBuilder.yearMonth = new YearMonth(2017, 6); action.transactionsQueryBuilder = transactionsBuilder; action.reportingBucket = "test-bucket"; - action.yearMonth = new YearMonth(2017, 6); - action.subdir = "icann/monthly/2017-06"; action.bigquery = bigquery; action.gcsUtils = new GcsUtils(gcsService, 1024); return action; @@ -100,7 +98,7 @@ public class IcannReportingStagerTest { .build(); when(bigquery.queryToLocalTableSync(any(String.class))).thenReturn(activityReportTable); IcannReportingStager stager = createStager(); - stager.stageReports(ReportType.ACTIVITY); + stager.stageReports(yearMonth, subdir, ReportType.ACTIVITY); String expectedReport1 = "fooField,barField\r\n12,34"; String expectedReport2 = "fooField,barField\r\n56,78"; @@ -143,7 +141,7 @@ public class IcannReportingStagerTest { .build(); when(bigquery.queryToLocalTableSync(any(String.class))).thenReturn(transactionReportTable); IcannReportingStager stager = createStager(); - stager.stageReports(ReportType.TRANSACTIONS); + stager.stageReports(yearMonth, subdir, ReportType.TRANSACTIONS); String expectedReport1 = "registrar,iana,field\r\n\"reg1\",123,10\r\n\"reg2\",456,20\r\nTotals,,30"; @@ -165,7 +163,7 @@ public class IcannReportingStagerTest { IcannReportingStager stager = createStager(); ImmutableList filenames = ImmutableList.of("fooTld-transactions-201706.csv", "barTld-activity-201706.csv"); - stager.createAndUploadManifest(filenames); + stager.createAndUploadManifest(subdir, filenames); String expectedManifest = "fooTld-transactions-201706.csv\nbarTld-activity-201706.csv\n"; byte[] generatedManifest = diff --git a/javatests/google/registry/reporting/icann/IcannReportingStagingActionTest.java b/javatests/google/registry/reporting/icann/IcannReportingStagingActionTest.java index b068e8f6c..3dfeafa00 100644 --- a/javatests/google/registry/reporting/icann/IcannReportingStagingActionTest.java +++ b/javatests/google/registry/reporting/icann/IcannReportingStagingActionTest.java @@ -24,14 +24,17 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import google.registry.bigquery.BigqueryJobFailureException; import google.registry.reporting.icann.IcannReportingModule.ReportType; +import google.registry.request.HttpException.BadRequestException; import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.FakeSleeper; import google.registry.testing.TaskQueueHelper.TaskMatcher; import google.registry.util.Retrier; +import java.util.Optional; import org.joda.time.YearMonth; import org.junit.Before; import org.junit.Rule; @@ -46,6 +49,9 @@ public class IcannReportingStagingActionTest { FakeResponse response = new FakeResponse(); IcannReportingStager stager = mock(IcannReportingStager.class); ReportingEmailUtils emailUtils = mock(ReportingEmailUtils.class); + YearMonth yearMonth = new YearMonth(2017, 6); + String subdir = "default/dir"; + IcannReportingStagingAction action; @Rule public final AppEngineRule appEngine = AppEngineRule.builder() @@ -56,37 +62,36 @@ public class IcannReportingStagingActionTest { @Before public void setUp() throws Exception { - when(stager.stageReports(ReportType.ACTIVITY)).thenReturn(ImmutableList.of("a", "b")); - when(stager.stageReports(ReportType.TRANSACTIONS)).thenReturn(ImmutableList.of("c", "d")); - } - - private static void assertUploadTaskEnqueued(String subDir) { - TaskMatcher matcher = - new TaskMatcher() - .url("/_dr/task/icannReportingUpload") - .method("POST") - .param("subdir", subDir); - assertTasksEnqueued("retryable-cron-tasks", matcher); - } - - private IcannReportingStagingAction createAction(ImmutableList reportingMode) { - IcannReportingStagingAction action = new IcannReportingStagingAction(); - action.yearMonth = new YearMonth(2017, 6); - action.subdir = "default/dir"; - action.reportTypes = reportingMode; + action = new IcannReportingStagingAction(); + action.yearMonth = yearMonth; + action.overrideSubdir = Optional.of(subdir); + action.reportTypes = ImmutableSet.of(ReportType.ACTIVITY, ReportType.TRANSACTIONS); action.response = response; action.stager = stager; action.retrier = new Retrier(new FakeSleeper(new FakeClock()), 3); action.emailUtils = emailUtils; - return action; + + when(stager.stageReports(yearMonth, subdir, ReportType.ACTIVITY)) + .thenReturn(ImmutableList.of("a", "b")); + when(stager.stageReports(yearMonth, subdir, ReportType.TRANSACTIONS)) + .thenReturn(ImmutableList.of("c", "d")); + } + + private static void assertUploadTaskEnqueued(String subdir) { + TaskMatcher matcher = + new TaskMatcher() + .url("/_dr/task/icannReportingUpload") + .method("POST") + .param("subdir", subdir); + assertTasksEnqueued("retryable-cron-tasks", matcher); } @Test public void testActivityReportingMode_onlyStagesActivityReports() throws Exception { - IcannReportingStagingAction action = createAction(ImmutableList.of(ReportType.ACTIVITY)); + action.reportTypes = ImmutableSet.of(ReportType.ACTIVITY); action.run(); - verify(stager).stageReports(ReportType.ACTIVITY); - verify(stager).createAndUploadManifest(ImmutableList.of("a", "b")); + verify(stager).stageReports(yearMonth, subdir, ReportType.ACTIVITY); + verify(stager).createAndUploadManifest(subdir, ImmutableList.of("a", "b")); verify(emailUtils) .emailResults( "ICANN Monthly report staging summary [SUCCESS]", @@ -96,12 +101,10 @@ public class IcannReportingStagingActionTest { @Test public void testAbsentReportingMode_stagesBothReports() throws Exception { - IcannReportingStagingAction action = - createAction(ImmutableList.of(ReportType.ACTIVITY, ReportType.TRANSACTIONS)); action.run(); - verify(stager).stageReports(ReportType.ACTIVITY); - verify(stager).stageReports(ReportType.TRANSACTIONS); - verify(stager).createAndUploadManifest(ImmutableList.of("a", "b", "c", "d")); + verify(stager).stageReports(yearMonth, subdir, ReportType.ACTIVITY); + verify(stager).stageReports(yearMonth, subdir, ReportType.TRANSACTIONS); + verify(stager).createAndUploadManifest(subdir, ImmutableList.of("a", "b", "c", "d")); verify(emailUtils) .emailResults( "ICANN Monthly report staging summary [SUCCESS]", @@ -111,15 +114,13 @@ public class IcannReportingStagingActionTest { @Test public void testRetryOnBigqueryException() throws Exception { - IcannReportingStagingAction action = - createAction(ImmutableList.of(ReportType.ACTIVITY, ReportType.TRANSACTIONS)); - when(stager.stageReports(ReportType.TRANSACTIONS)) + when(stager.stageReports(yearMonth, subdir, ReportType.TRANSACTIONS)) .thenThrow(new BigqueryJobFailureException("Expected failure", null, null, null)) .thenReturn(ImmutableList.of("c", "d")); action.run(); - verify(stager, times(2)).stageReports(ReportType.ACTIVITY); - verify(stager, times(2)).stageReports(ReportType.TRANSACTIONS); - verify(stager).createAndUploadManifest(ImmutableList.of("a", "b", "c", "d")); + verify(stager, times(2)).stageReports(yearMonth, subdir, ReportType.ACTIVITY); + verify(stager, times(2)).stageReports(yearMonth, subdir, ReportType.TRANSACTIONS); + verify(stager).createAndUploadManifest(subdir, ImmutableList.of("a", "b", "c", "d")); verify(emailUtils) .emailResults( "ICANN Monthly report staging summary [SUCCESS]", @@ -129,15 +130,14 @@ public class IcannReportingStagingActionTest { @Test public void testEmailEng_onMoreThanRetriableFailure() throws Exception { - IcannReportingStagingAction action = - createAction(ImmutableList.of(ReportType.ACTIVITY)); - when(stager.stageReports(ReportType.ACTIVITY)) + action.reportTypes = ImmutableSet.of(ReportType.ACTIVITY); + when(stager.stageReports(yearMonth, subdir, ReportType.ACTIVITY)) .thenThrow(new BigqueryJobFailureException("Expected failure", null, null, null)); RuntimeException thrown = assertThrows(RuntimeException.class, action::run); assertThat(thrown).hasCauseThat().isInstanceOf(BigqueryJobFailureException.class); assertThat(thrown).hasMessageThat().isEqualTo("Staging action failed."); assertThat(thrown).hasCauseThat().hasMessageThat().isEqualTo("Expected failure"); - verify(stager, times(3)).stageReports(ReportType.ACTIVITY); + verify(stager, times(3)).stageReports(yearMonth, subdir, ReportType.ACTIVITY); verify(emailUtils) .emailResults( "ICANN Monthly report staging summary [FAILURE]", @@ -146,4 +146,29 @@ public class IcannReportingStagingActionTest { // Assert no upload task enqueued assertNoTasksEnqueued("retryable-cron-tasks"); } + + @Test + public void testEmptySubDir_returnsDefaultSubdir() { + action.overrideSubdir = Optional.empty(); + assertThat(action.getSubdir(new YearMonth(2017, 6))).isEqualTo("icann/monthly/2017-06"); + } + + @Test + public void testGivenSubdir_returnsManualSubdir() { + action.overrideSubdir = Optional.of("manual/dir"); + assertThat(action.getSubdir(new YearMonth(2017, 6))).isEqualTo("manual/dir"); + } + + @Test + public void testInvalidSubdir_throwsException() { + action.overrideSubdir = Optional.of("/whoops"); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> + action.getSubdir(new YearMonth(2017, 6))); + assertThat(thrown) + .hasMessageThat() + .contains("subdir must not start or end with a \"/\", got /whoops instead."); + } } diff --git a/javatests/google/registry/reporting/icann/TransactionsReportingQueryBuilderTest.java b/javatests/google/registry/reporting/icann/TransactionsReportingQueryBuilderTest.java index d650c9810..fcbc7d757 100644 --- a/javatests/google/registry/reporting/icann/TransactionsReportingQueryBuilderTest.java +++ b/javatests/google/registry/reporting/icann/TransactionsReportingQueryBuilderTest.java @@ -27,9 +27,10 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class TransactionsReportingQueryBuilderTest { + private final YearMonth yearMonth = new YearMonth(2017, 9); + private TransactionsReportingQueryBuilder getQueryBuilder() { TransactionsReportingQueryBuilder queryBuilder = new TransactionsReportingQueryBuilder(); - queryBuilder.yearMonth = new YearMonth(2017, 9); queryBuilder.projectId = "domain-registry-alpha"; return queryBuilder; } @@ -37,7 +38,7 @@ public class TransactionsReportingQueryBuilderTest { @Test public void testAggregateQueryMatch() { TransactionsReportingQueryBuilder queryBuilder = getQueryBuilder(); - assertThat(queryBuilder.getReportQuery()) + assertThat(queryBuilder.getReportQuery(yearMonth)) .isEqualTo( "#standardSQL\nSELECT * FROM " + "`domain-registry-alpha.icann_reporting.transactions_report_aggregation_201709`"); @@ -56,7 +57,7 @@ public class TransactionsReportingQueryBuilderTest { TransactionsReportingQueryBuilder.ATTEMPTED_ADDS); TransactionsReportingQueryBuilder queryBuilder = getQueryBuilder(); - ImmutableMap actualQueries = queryBuilder.getViewQueryMap(); + ImmutableMap actualQueries = queryBuilder.getViewQueryMap(yearMonth); for (String queryName : expectedQueryNames) { String actualTableName = String.format("%s_201709", queryName); String testFilename = String.format("%s_test.sql", queryName); diff --git a/javatests/google/registry/request/RequestParametersTest.java b/javatests/google/registry/request/RequestParametersTest.java index d122a0655..1ea1fa0e0 100644 --- a/javatests/google/registry/request/RequestParametersTest.java +++ b/javatests/google/registry/request/RequestParametersTest.java @@ -23,6 +23,7 @@ import static google.registry.request.RequestParameters.extractOptionalEnumParam import static google.registry.request.RequestParameters.extractOptionalParameter; import static google.registry.request.RequestParameters.extractRequiredDatetimeParameter; import static google.registry.request.RequestParameters.extractRequiredParameter; +import static google.registry.request.RequestParameters.extractSetOfEnumParameters; import static google.registry.request.RequestParameters.extractSetOfParameters; import static google.registry.testing.JUnitBackports.assertThrows; import static org.mockito.Mockito.mock; @@ -86,25 +87,25 @@ public class RequestParametersTest { @Test public void testExtractSetOfParameters_empty_returnsEmpty() { - when(req.getParameterValues("spin")).thenReturn(new String[]{""}); + when(req.getParameter("spin")).thenReturn(""); assertThat(extractSetOfParameters(req, "spin")).isEmpty(); } @Test public void testExtractSetOfParameters_oneValue_returnsValue() { - when(req.getParameterValues("spin")).thenReturn(new String[]{"bog"}); + when(req.getParameter("spin")).thenReturn("bog"); assertThat(extractSetOfParameters(req, "spin")).containsExactly("bog"); } @Test public void testExtractSetOfParameters_multipleValues_returnsAll() { - when(req.getParameterValues("spin")).thenReturn(new String[]{"bog,gob"}); + when(req.getParameter("spin")).thenReturn("bog,gob"); assertThat(extractSetOfParameters(req, "spin")).containsExactly("bog", "gob"); } @Test public void testExtractSetOfParameters_multipleValuesWithEmpty_removesEmpty() { - when(req.getParameterValues("spin")).thenReturn(new String[]{",bog,,gob,"}); + when(req.getParameter("spin")).thenReturn(",bog,,gob,"); assertThat(extractSetOfParameters(req, "spin")).containsExactly("bog", "gob"); } @@ -116,6 +117,53 @@ public class RequestParametersTest { assertThat(thrown).hasMessageThat().contains("spin"); } + @Test + public void testExtractSetOfEnumParameters_notPresent_returnsEmpty() { + assertThat(extractSetOfEnumParameters(req, Club.class, "spin")).isEmpty(); + } + + @Test + public void testExtractSetOfEnumParameters_empty_returnsEmpty() { + when(req.getParameter("spin")).thenReturn(""); + assertThat(extractSetOfEnumParameters(req, Club.class, "spin")).isEmpty(); + } + + @Test + public void testExtractSetOfEnumParameters_oneValue_returnsValue() { + when(req.getParameter("spin")).thenReturn("DANCE"); + assertThat(extractSetOfEnumParameters(req, Club.class, "spin")).containsExactly(Club.DANCE); + } + + @Test + public void testExtractSetOfEnumParameters_multipleValues_returnsAll() { + when(req.getParameter("spin")).thenReturn("DANCE,FLOOR"); + assertThat(extractSetOfEnumParameters(req, Club.class, "spin")) + .containsExactly(Club.DANCE, Club.FLOOR); + } + + @Test + public void testExtractSetOfEnumParameters_multipleValuesWithEmpty_removesEmpty() { + when(req.getParameter("spin")).thenReturn(",DANCE,,FLOOR,"); + assertThat(extractSetOfEnumParameters(req, Club.class, "spin")) + .containsExactly(Club.DANCE, Club.FLOOR); + } + + @Test + public void testExtractSetOfEnumParameters_multipleValues_caseInsensitive() { + when(req.getParameter("spin")).thenReturn("danCE,FlooR"); + assertThat(extractSetOfEnumParameters(req, Club.class, "spin")) + .containsExactly(Club.DANCE, Club.FLOOR); + } + + @Test + public void testExtractSetOfEnumParameters_multipleParameters_error() { + when(req.getParameterValues("spin")).thenReturn(new String[]{"DANCE", "FLOOR"}); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> extractSetOfEnumParameters(req, Club.class, "spin")); + assertThat(thrown).hasMessageThat().contains("spin"); + } + @Test public void testExtractBooleanParameter_notPresent_returnsFalse() { assertThat(extractBooleanParameter(req, "love")).isFalse();