diff --git a/core/src/main/resources/google/registry/reporting/icann/sql/transaction_counts.sql b/core/src/main/resources/google/registry/reporting/icann/sql/transaction_counts.sql index 763958f7b..88467b9b1 100644 --- a/core/src/main/resources/google/registry/reporting/icann/sql/transaction_counts.sql +++ b/core/src/main/resources/google/registry/reporting/icann/sql/transaction_counts.sql @@ -32,7 +32,16 @@ FROM ( WHEN field = 'TRANSFER_NACKED' THEN 'TRANSFER_GAINING_NACKED' ELSE field END AS metricName, - SUM(amount) AS metricValue + -- See b/290228682, there are edge cases in which the net_renew would be negative when + -- a domain is cancelled by superusers during renew grace period. The correct thing + -- to do is attribute the cancellation to the owning registrar, but that would require + -- changing the owing registrar of the the corresponding cancellation DomainHistory, + -- which has cascading effects that we don't want to deal with. As such we simply + -- floor the number here to zero to prevent any negative value from appearing, which + -- should have negligible impact as the edge cage happens very rarely, more specifically + -- when a cancellation happens during grace period by a registrar other than the the + -- owning one. All the numbers here should be non-negative to pass ICANN validation. + GREATEST(SUM(amount), 0) AS metricValue FROM ( SELECT CASE @@ -47,16 +56,7 @@ FROM ( END AS clientId, tld, report_field AS field, - -- See b/290228682, there are edge cases in which the net_renew would be negative when - -- a domain is cancelled by superusers during renew grace period. The correct thing - -- to do is attribute the cancellation to the owning registrar, but that would require - -- changing the owing registrar of the the corresponding cancellation DomainHistory, - -- which has cascading effects that we don't want to deal with. As such we simply - -- floor the number here to zero to prevent any negative value from appearing, which - -- should have negligible impact as the edge cage happens very rarely, more specifically - -- when a cancellation happens during grace period by a registrar other than the the - -- owning one. All the numbers here should be positive to pass ICANN validation. - GREATEST(report_amount, 0) AS amount, + report_amount AS amount, reporting_time AS reportingTime FROM EXTERNAL_QUERY("projects/%PROJECT_ID%/locations/us/connections/%PROJECT_ID%-sql", ''' SELECT history_type, history_other_registrar_id, history_registrar_id, domain_repo_id, history_revision_id FROM "DomainHistory";''') AS dh 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 5357b290b..b7afdfa01 100644 --- a/core/src/test/java/google/registry/reporting/icann/TransactionsReportingQueryBuilderTest.java +++ b/core/src/test/java/google/registry/reporting/icann/TransactionsReportingQueryBuilderTest.java @@ -15,6 +15,7 @@ package google.registry.reporting.icann; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -60,7 +61,8 @@ class TransactionsReportingQueryBuilderTest { for (String queryName : expectedQueryNames) { String actualTableName = String.format("%s_201709", queryName); String testFilename = String.format("%s_test.sql", queryName); - assertThat(actualQueries.get(actualTableName)) + assertWithMessage("Query expected in test data file %s differs", testFilename) + .that(actualQueries.get(actualTableName)) .isEqualTo(ReportingTestData.loadFile(testFilename)); } } diff --git a/core/src/test/resources/google/registry/reporting/icann/transaction_counts_test.sql b/core/src/test/resources/google/registry/reporting/icann/transaction_counts_test.sql index 2e52d80d3..4b1ee8c96 100644 --- a/core/src/test/resources/google/registry/reporting/icann/transaction_counts_test.sql +++ b/core/src/test/resources/google/registry/reporting/icann/transaction_counts_test.sql @@ -32,7 +32,16 @@ FROM ( WHEN field = 'TRANSFER_NACKED' THEN 'TRANSFER_GAINING_NACKED' ELSE field END AS metricName, - SUM(amount) AS metricValue + -- See b/290228682, there are edge cases in which the net_renew would be negative when + -- a domain is cancelled by superusers during renew grace period. The correct thing + -- to do is attribute the cancellation to the owning registrar, but that would require + -- changing the owing registrar of the the corresponding cancellation DomainHistory, + -- which has cascading effects that we don't want to deal with. As such we simply + -- floor the number here to zero to prevent any negative value from appearing, which + -- should have negligible impact as the edge cage happens very rarely, more specifically + -- when a cancellation happens during grace period by a registrar other than the the + -- owning one. All the numbers here should be non-negative to pass ICANN validation. + GREATEST(SUM(amount), 0) AS metricValue FROM ( SELECT CASE @@ -47,16 +56,7 @@ FROM ( END AS clientId, tld, report_field AS field, - -- See b/290228682, there are edge cases in which the net_renew would be negative when - -- a domain is cancelled by superusers during renew grace period. The correct thing - -- to do is attribute the cancellation to the owning registrar, but that would require - -- changing the owing registrar of the the corresponding cancellation DomainHistory, - -- which has cascading effects that we don't want to deal with. As such we simply - -- floor the number here to zero to prevent any negative value from appearing, which - -- should have negligible impact as the edge cage happens very rarely, more specifically - -- when a cancellation happens during grace period by a registrar other than the the - -- owning one. All the numbers here should be positive to pass ICANN validation. - GREATEST(report_amount, 0) AS amount, + report_amount AS amount, reporting_time AS reportingTime FROM EXTERNAL_QUERY("projects/domain-registry-alpha/locations/us/connections/domain-registry-alpha-sql", ''' SELECT history_type, history_other_registrar_id, history_registrar_id, domain_repo_id, history_revision_id FROM "DomainHistory";''') AS dh