diff --git a/core/src/main/resources/google/registry/reporting/icann/sql/cloud_sql_transaction_counts.sql b/core/src/main/resources/google/registry/reporting/icann/sql/cloud_sql_transaction_counts.sql index 1e64be742..006398a77 100644 --- a/core/src/main/resources/google/registry/reporting/icann/sql/cloud_sql_transaction_counts.sql +++ b/core/src/main/resources/google/registry/reporting/icann/sql/cloud_sql_transaction_counts.sql @@ -47,7 +47,16 @@ FROM ( END AS clientId, tld, report_field AS field, - report_amount AS amount, + -- 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. + MAX(report_amount, 0) 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/ActivityReportingQueryBuilderTest.java b/core/src/test/java/google/registry/reporting/icann/ActivityReportingQueryBuilderTest.java index 015f8bdfa..386c3e495 100644 --- a/core/src/test/java/google/registry/reporting/icann/ActivityReportingQueryBuilderTest.java +++ b/core/src/test/java/google/registry/reporting/icann/ActivityReportingQueryBuilderTest.java @@ -45,7 +45,7 @@ class ActivityReportingQueryBuilderTest { } @Test - void testIntermediaryQueryMatch_cloud_sql() { + void testIntermediaryQueryMatch_cloudSql() { ImmutableList expectedQueryNames = ImmutableList.of( ActivityReportingQueryBuilder.REGISTRAR_OPERATING_STATUS, diff --git a/core/src/test/resources/google/registry/reporting/icann/transaction_counts_test_cloud_sql.sql b/core/src/test/resources/google/registry/reporting/icann/transaction_counts_test_cloud_sql.sql index 78460eeaf..cf0db80e4 100644 --- a/core/src/test/resources/google/registry/reporting/icann/transaction_counts_test_cloud_sql.sql +++ b/core/src/test/resources/google/registry/reporting/icann/transaction_counts_test_cloud_sql.sql @@ -47,7 +47,16 @@ FROM ( END AS clientId, tld, report_field AS field, - report_amount AS amount, + -- 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. + MAX(report_amount, 0) 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