Restructure invoice updates and implement propagation of status from the external system into the database.

2 files changed, 90 insertions, 28 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainer.java
index 5c37e0e4d0b..64631799804 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainer.java
@@ -10,9 +10,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.billing.BillStatus;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.BillingController;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.BillingDatabaseClient;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.BillingReporter;
-import com.yahoo.vespa.hosted.controller.api.integration.billing.FailedInvoiceUpdate;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.InvoiceUpdate;
-import com.yahoo.vespa.hosted.controller.api.integration.billing.ModifiableInvoiceUpdate;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.Plan;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.PlanRegistry;
 import com.yahoo.vespa.hosted.controller.tenant.CloudTenant;
@@ -73,18 +71,34 @@ public class BillingReportMaintainer extends ControllerMaintainer {
         var tenants = cloudTenants();
         var billsNeedingMaintenance = databaseClient.readBills().stream()
                 .filter(bill -> bill.getExportedId().isPresent())
-                .filter(exported -> exported.status() == BillStatus.OPEN)
+                .filter(exported -> ! exported.status().isFinal())
                 .toList();
 
         for (var bill : billsNeedingMaintenance) {
             var exportedId = bill.getExportedId().orElseThrow();
             var update = reporter.maintainInvoice(tenants.get(bill.tenant()), bill);
-            if (update instanceof ModifiableInvoiceUpdate modifiable && ! modifiable.isEmpty()) {
-                log.fine(invoiceMessage(bill.id(), exportedId) + " was updated with " + modifiable.itemsUpdate());
-            } else if (update instanceof FailedInvoiceUpdate failed && failed.reason == FailedInvoiceUpdate.Reason.REMOVED) {
-                log.fine(invoiceMessage(bill.id(), exportedId) +  " has been deleted in the external system");
-                // Reset the exportedId to null, so that we don't maintain it again
-                databaseClient.setExportedInvoiceId(bill.id(), null);
+            switch (update.type()) {
+                case UNMODIFIED -> log.finer(() ->invoiceMessage(bill.id(), exportedId) + " was not modified");
+                case MODIFIED -> log.fine(invoiceMessage(bill.id(), exportedId) + " was updated with " + update.itemsUpdate().get());
+                case UNMODIFIABLE -> {
+                    if (bill.status() != BillStatus.FROZEN) {
+                        log.fine(() -> invoiceMessage(bill.id(), exportedId) + " is now unmodifiable");
+                        databaseClient.setStatus(bill.id(), "system", BillStatus.FROZEN);
+                    }
+                }
+                case REMOVED -> {
+                    log.fine(() -> invoiceMessage(bill.id(), exportedId) + " has been deleted in the external system");
+                    // Reset the exportedId to null, so that we don't maintain it again
+                    databaseClient.setExportedInvoiceId(bill.id(), null);
+                }
+                case PAID -> {
+                    log.fine(() -> invoiceMessage(bill.id(), exportedId) + " has been paid in the external system");
+                    databaseClient.setStatus(bill.id(), "system", BillStatus.SUCCESSFUL);
+                }
+                case VOIDED -> {
+                    log.fine(() -> invoiceMessage(bill.id(), exportedId) + " has been voided in the external system");
+                    databaseClient.setStatus(bill.id(), "system", BillStatus.VOID);
+                }
             }
             updates.add(update);
         }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainerTest.java
index 3cffe659669..6adabad557d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/BillingReportMaintainerTest.java
@@ -8,8 +8,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.billing.Bill;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.BillStatus;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.BillingDatabaseClient;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.BillingReporterMock;
-import com.yahoo.vespa.hosted.controller.api.integration.billing.FailedInvoiceUpdate;
-import com.yahoo.vespa.hosted.controller.api.integration.billing.ModifiableInvoiceUpdate;
+import com.yahoo.vespa.hosted.controller.api.integration.billing.InvoiceUpdate;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.PlanRegistryMock;
 import com.yahoo.vespa.hosted.controller.tenant.BillingReference;
 import com.yahoo.vespa.hosted.controller.tenant.CloudTenant;
@@ -18,7 +17,10 @@ import org.junit.jupiter.api.Test;
 import java.time.Duration;
 import java.time.LocalDate;
 import java.time.ZoneOffset;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -50,23 +52,30 @@ public class BillingReportMaintainerTest {
     }
 
     @Test
-    void only_open_bills_with_exported_id_are_maintained() {
+    void only_non_final_bills_with_exported_id_are_maintained() {
         var t1 = tester.createTenant("t1");
 
         var bill1 = createBill(t1, "non-exported", billingDb);
-        var bill2 = createBill(t1, "exported", billingDb);
+        var bill2 = createBill(t1, "exported-and-modified", billingDb);
         var bill3 = createBill(t1, "exported-and-frozen", billingDb);
+        var bill4 = createBill(t1, "exported-and-successful", billingDb);
+        var bill5 = createBill(t1, "exported-and-void", billingDb);
         billingDb.setStatus(bill3, "foo", BillStatus.FROZEN);
+        billingDb.setStatus(bill4, "foo", BillStatus.SUCCESSFUL);
+        billingDb.setStatus(bill5, "foo", BillStatus.VOID);
 
         exportBills(t1, bill2, bill3);
-        var updates = maintainer.maintainInvoices();
+        reporter.modifyInvoice(bill2);
+        var updates = toMap(maintainer.maintainInvoices());
 
         assertTrue(billingDb.readBill(bill1).get().getExportedId().isEmpty());
 
-        // Only the exported open bill is maintained
-        assertEquals(1, updates.size());
-        assertEquals(bill2, updates.get(0).billId());
-        assertEquals(ModifiableInvoiceUpdate.class, updates.get(0).getClass());
+        // Only the exported non-final bills are maintained
+        assertEquals(2, updates.size());
+        assertEquals(Set.of(bill2, bill3), updates.keySet());
+
+        var bill2Update = updates.get(bill2);
+        assertEquals(InvoiceUpdate.Type.MODIFIED, bill2Update.type());
         var exportedBill = billingDb.readBill(bill2).get();
         assertEquals("EXPORTED-" + exportedBill.id().value(), exportedBill.getExportedId().get());
         // Verify that the bill has been updated with a marker line item by the mock
@@ -74,10 +83,11 @@ public class BillingReportMaintainerTest {
         assertEquals(1, lineItems.size());
         assertEquals("maintained", lineItems.get(0).id());
 
-        // The frozen bill is untouched by the maintainer
+        // Verify that the frozen bill is unmodified and has not changed state.
+        var bill3Update = updates.get(bill3);
+        assertEquals(InvoiceUpdate.Type.UNMODIFIED, bill3Update.type());
         var frozenBill = billingDb.readBill(bill3).get();
-        assertEquals("EXPORTED-" + frozenBill.id().value(), frozenBill.getExportedId().get());
-        assertEquals(0, frozenBill.lineItems().size());
+        assertEquals(BillStatus.FROZEN, frozenBill.status());
     }
 
     @Test
@@ -88,16 +98,15 @@ public class BillingReportMaintainerTest {
 
         var updates = maintainer.maintainInvoices();
         assertEquals(1, updates.size());
-        assertEquals(ModifiableInvoiceUpdate.class, updates.get(0).getClass());
+        assertEquals(InvoiceUpdate.Type.UNMODIFIED, updates.get(0).type());
 
         // Delete invoice from the external system
-        reporter.deleteExportedBill(bill1);
+        reporter.deleteInvoice(bill1);
 
         // Maintainer should report that the invoice has been removed
         updates = maintainer.maintainInvoices();
         assertEquals(1, updates.size());
-        assertEquals(FailedInvoiceUpdate.class, updates.get(0).getClass());
-        assertEquals(FailedInvoiceUpdate.Reason.REMOVED, ((FailedInvoiceUpdate)updates.get(0)).reason);
+        assertEquals(InvoiceUpdate.Type.REMOVED, updates.get(0).type());
 
         // The bill should no longer be maintained
         updates = maintainer.maintainInvoices();
@@ -107,13 +116,12 @@ public class BillingReportMaintainerTest {
     @Test
     void it_is_allowed_to_re_export_bills_whose_invoice_has_been_deleted_in_the_external_system() {
         var t1 = tester.createTenant("t1");
-
         var bill1 = createBill(t1, "exported-then-deleted", billingDb);
 
         // Export the bill, then delete it in the external system
         exportBills(t1, bill1);
         maintainer.maintainInvoices();
-        reporter.deleteExportedBill(bill1);
+        reporter.deleteInvoice(bill1);
         maintainer.maintainInvoices();
 
         // Ensure it is currently ignored by the maintainer
@@ -124,7 +132,47 @@ public class BillingReportMaintainerTest {
         exportBills(t1, bill1);
         updates = maintainer.maintainInvoices();
         assertEquals(1, updates.size());
-        assertEquals(ModifiableInvoiceUpdate.class, updates.get(0).getClass());
+        assertEquals(InvoiceUpdate.Type.UNMODIFIED, updates.get(0).type());
+    }
+
+    @Test
+    void bill_state_is_updated_upon_changes_in_the_external_system() {
+        var t1 = tester.createTenant("t1");
+        var frozen = createBill(t1, "foo", billingDb);
+        var paid   = createBill(t1, "foo", billingDb);
+        var voided = createBill(t1, "foo", billingDb);
+        exportBills(t1, frozen, paid, voided);
+
+        var updates = toMap(maintainer.maintainInvoices());
+        assertEquals(3, updates.size());
+        updates.forEach((id, update) -> {
+            assertEquals(InvoiceUpdate.Type.UNMODIFIED, update.type());
+            assertEquals(BillStatus.OPEN, billingDb.readBill(id).get().status());
+        });
+
+        reporter.freezeInvoice(frozen);
+        reporter.payInvoice(paid);
+        reporter.voidInvoice(voided);
+        updates = toMap(maintainer.maintainInvoices());
+
+        assertEquals(3, updates.size());
+
+        assertEquals(InvoiceUpdate.Type.UNMODIFIABLE, updates.get(frozen).type());
+        assertEquals(BillStatus.FROZEN, billingDb.readBill(frozen).get().status());
+
+        assertEquals(InvoiceUpdate.Type.PAID, updates.get(paid).type());
+        assertEquals(BillStatus.SUCCESSFUL, billingDb.readBill(paid).get().status());
+
+        assertEquals(InvoiceUpdate.Type.VOIDED, updates.get(voided).type());
+        assertEquals(BillStatus.VOID, billingDb.readBill(voided).get().status());
+    }
+
+    private static Map<Bill.Id, InvoiceUpdate> toMap(Iterable<InvoiceUpdate> updates) {
+        var map = new HashMap<Bill.Id, InvoiceUpdate>();
+        for (var update : updates) {
+            map.put(update.billId(), update);
+        }
+        return map;
     }
 
     private static Bill.Id createBill(TenantName tenantName, String agent, BillingDatabaseClient billingDb) {