Add a floor of zero to transaction report counts (#2074)

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.
This commit is contained in:
Lai Jiang 2023-07-12 12:56:09 -04:00 committed by GitHub
parent bdf9073a67
commit a9bec7effe
3 changed files with 21 additions and 3 deletions

View file

@ -47,7 +47,16 @@ FROM (
END AS clientId, END AS clientId,
tld, tld,
report_field AS field, 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 reporting_time AS reportingTime
FROM EXTERNAL_QUERY("projects/%PROJECT_ID%/locations/us/connections/%PROJECT_ID%-sql", 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 ''' SELECT history_type, history_other_registrar_id, history_registrar_id, domain_repo_id, history_revision_id FROM "DomainHistory";''') AS dh

View file

@ -45,7 +45,7 @@ class ActivityReportingQueryBuilderTest {
} }
@Test @Test
void testIntermediaryQueryMatch_cloud_sql() { void testIntermediaryQueryMatch_cloudSql() {
ImmutableList<String> expectedQueryNames = ImmutableList<String> expectedQueryNames =
ImmutableList.of( ImmutableList.of(
ActivityReportingQueryBuilder.REGISTRAR_OPERATING_STATUS, ActivityReportingQueryBuilder.REGISTRAR_OPERATING_STATUS,

View file

@ -47,7 +47,16 @@ FROM (
END AS clientId, END AS clientId,
tld, tld,
report_field AS field, 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 reporting_time AS reportingTime
FROM EXTERNAL_QUERY("projects/domain-registry-alpha/locations/us/connections/domain-registry-alpha-sql", 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 ''' SELECT history_type, history_other_registrar_id, history_registrar_id, domain_repo_id, history_revision_id FROM "DomainHistory";''') AS dh