From 4b6811d4c39e81fddbb838ae9d1a16c04201e3cd Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Wed, 2 Jun 2021 11:52:47 -0400 Subject: [PATCH] Handle cases where periodYears is NULL in a OneTime (#1187) There are cases where periodYears is not set when creating a OneTime billing event, for example when performing a registry lock (default cost = $0) or when performing a server status update, such as applying the serverUpdateProhibited status (default cost = $20). This is not currently handled currently in the billing pipeline because the parseFromRecord method checks for nullness for all fields. Even if it does not validate the fields, the null periodYears will still cause problem when the billing event is converted to CSV files. This PR alters the BigQuery SQL file to convert a NULL to 0 when creating the BillingEvent in the invoicing pipeline. It also sets the EndDate in the invoice CSV to an empty string when periodYears is 0. Note that when the cost is also 0, the billing event is filtered out in the invoice CSV so only the non-free OneTime with null periodYear will have an impact on the output. For detailed reports all billing events are included and the zero periodYears is printed as is. Setting the EndDate to empty is the correct behavior per go/manual-integration-csv#end-date. --- .../registry/beam/invoicing/BillingEvent.java | 9 ++++- .../beam/invoicing/sql/billing_events.sql | 2 +- .../beam/invoicing/BillingEventTest.java | 13 ++++++ .../beam/invoicing/InvoicingPipelineTest.java | 40 ++++++++++++++++++- .../beam/invoicing/billing_events_test.sql | 2 +- 5 files changed, 61 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/google/registry/beam/invoicing/BillingEvent.java b/core/src/main/java/google/registry/beam/invoicing/BillingEvent.java index 6aa33dc18..d047463a0 100644 --- a/core/src/main/java/google/registry/beam/invoicing/BillingEvent.java +++ b/core/src/main/java/google/registry/beam/invoicing/BillingEvent.java @@ -251,7 +251,14 @@ public abstract class BillingEvent implements Serializable { InvoiceGroupingKey getInvoiceGroupingKey() { return new AutoValue_BillingEvent_InvoiceGroupingKey( billingTime().toLocalDate().withDayOfMonth(1).toString(), - billingTime().toLocalDate().withDayOfMonth(1).plusYears(years()).minusDays(1).toString(), + years() == 0 + ? "" + : billingTime() + .toLocalDate() + .withDayOfMonth(1) + .plusYears(years()) + .minusDays(1) + .toString(), billingId(), String.format("%s - %s", registrarId(), tld()), String.format("%s | TLD: %s | TERM: %d-year", action(), tld(), years()), diff --git a/core/src/main/resources/google/registry/beam/invoicing/sql/billing_events.sql b/core/src/main/resources/google/registry/beam/invoicing/sql/billing_events.sql index 197f5267b..477684696 100644 --- a/core/src/main/resources/google/registry/beam/invoicing/sql/billing_events.sql +++ b/core/src/main/resources/google/registry/beam/invoicing/sql/billing_events.sql @@ -27,7 +27,7 @@ SELECT reason as action, targetId as domain, BillingEvent.domainRepoId as repositoryId, - periodYears as years, + IFNULL(periodYears, 0) as years, BillingEvent.currency AS currency, BillingEvent.amount as amount, -- We'll strip out non-useful flags downstream diff --git a/core/src/test/java/google/registry/beam/invoicing/BillingEventTest.java b/core/src/test/java/google/registry/beam/invoicing/BillingEventTest.java index 7366e9050..5216183e5 100644 --- a/core/src/test/java/google/registry/beam/invoicing/BillingEventTest.java +++ b/core/src/test/java/google/registry/beam/invoicing/BillingEventTest.java @@ -185,6 +185,19 @@ class BillingEventTest { + "myRegistrar - test,3,RENEW | TLD: test | TERM: 5-year,20.50,USD,"); } + @Test + void testConvertInvoiceGroupingKey_zeroYears_toCsv() { + GenericRecord record = schemaAndRecord.getRecord(); + record.put("years", 0); + schemaAndRecord = new SchemaAndRecord(record, null); + BillingEvent event = BillingEvent.parseFromRecord(schemaAndRecord); + InvoiceGroupingKey invoiceKey = event.getInvoiceGroupingKey(); + assertThat(invoiceKey.toCsv(3L)) + .isEqualTo( + "2017-10-01,,12345-CRRHELLO,61.50,USD,10125,1,PURCHASE," + + "myRegistrar - test,3,RENEW | TLD: test | TERM: 0-year,20.50,USD,"); + } + @Test void testInvoiceGroupingKeyCoder_deterministicSerialization() throws IOException { InvoiceGroupingKey invoiceKey = diff --git a/core/src/test/java/google/registry/beam/invoicing/InvoicingPipelineTest.java b/core/src/test/java/google/registry/beam/invoicing/InvoicingPipelineTest.java index e6e62b905..821df35c7 100644 --- a/core/src/test/java/google/registry/beam/invoicing/InvoicingPipelineTest.java +++ b/core/src/test/java/google/registry/beam/invoicing/InvoicingPipelineTest.java @@ -119,7 +119,37 @@ class InvoicingPipelineTest { 1, "USD", 0, - "SUNRISE ANCHOR_TENANT")); + "SUNRISE ANCHOR_TENANT"), + BillingEvent.create( + 1, + ZonedDateTime.of(2017, 10, 4, 0, 0, 0, 0, ZoneId.of("UTC")), + ZonedDateTime.of(2017, 10, 4, 0, 0, 0, 0, ZoneId.of("UTC")), + "theRegistrar", + "234", + "", + "test", + "SERVER_STATUS", + "locked.test", + "REPO-ID", + 0, + "USD", + 0, + ""), + BillingEvent.create( + 1, + ZonedDateTime.of(2017, 10, 4, 0, 0, 0, 0, ZoneId.of("UTC")), + ZonedDateTime.of(2017, 10, 4, 0, 0, 0, 0, ZoneId.of("UTC")), + "theRegistrar", + "234", + "", + "test", + "SERVER_STATUS", + "update-prohibited.test", + "REPO-ID", + 0, + "USD", + 20, + "")); private static final ImmutableMap> EXPECTED_DETAILED_REPORT_MAP = ImmutableMap.of( @@ -128,7 +158,11 @@ class InvoicingPipelineTest { "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,theRegistrar,234,," + "test,RENEW,mydomain2.test,REPO-ID,3,USD,20.50,", "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,theRegistrar,234,," - + "test,RENEW,mydomain.test,REPO-ID,3,USD,20.50,"), + + "test,RENEW,mydomain.test,REPO-ID,3,USD,20.50,", + "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,theRegistrar,234,," + + "test,SERVER_STATUS,update-prohibited.test,REPO-ID,0,USD,20.00,", + "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,theRegistrar,234,," + + "test,SERVER_STATUS,locked.test,REPO-ID,0,USD,0.00,"), "invoice_details_2017-10_theRegistrar_hello.csv", ImmutableList.of( "1,2017-10-02 00:00:00 UTC,2017-09-29 00:00:00 UTC,theRegistrar,234,," @@ -148,6 +182,8 @@ class InvoicingPipelineTest { + "RENEW | TLD: test | TERM: 3-year,20.50,USD,", "2017-10-01,2022-09-30,234,70.75,JPY,10125,1,PURCHASE,theRegistrar - hello,1," + "CREATE | TLD: hello | TERM: 5-year,70.75,JPY,", + "2017-10-01,,234,20.00,USD,10125,1,PURCHASE,theRegistrar - test,1," + + "SERVER_STATUS | TLD: test | TERM: 0-year,20.00,USD,", "2017-10-01,2018-09-30,456,20.50,USD,10125,1,PURCHASE,bestdomains - test,1," + "RENEW | TLD: test | TERM: 1-year,20.50,USD,116688"); diff --git a/core/src/test/resources/google/registry/beam/invoicing/billing_events_test.sql b/core/src/test/resources/google/registry/beam/invoicing/billing_events_test.sql index 1981a2d30..4d37e58e4 100644 --- a/core/src/test/resources/google/registry/beam/invoicing/billing_events_test.sql +++ b/core/src/test/resources/google/registry/beam/invoicing/billing_events_test.sql @@ -27,7 +27,7 @@ SELECT reason as action, targetId as domain, BillingEvent.domainRepoId as repositoryId, - periodYears as years, + IFNULL(periodYears, 0) as years, BillingEvent.currency AS currency, BillingEvent.amount as amount, -- We'll strip out non-useful flags downstream