Merge pull request #29190 from vespa-engine/update-more-bill-states

Update more bill states

6 files changed, 83 insertions, 87 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillStatus.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillStatus.java
index 4f35b47219a..00aa43cce5d 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillStatus.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillStatus.java
@@ -4,10 +4,10 @@ package com.yahoo.vespa.hosted.controller.api.integration.billing;
  * @author gjoranv
  */
 public enum BillStatus {
-    OPEN,       // All bills start in this state. The bill can be modified and exported/synced to external systems.
-    FROZEN,     // Syncing to external systems is switched off. No changes can be made.
-    CLOSED,     // End state for a valid bill.
-    VOID;       // End state, indicating that the bill is not valid.
+    OPEN,        // All bills start in this state. The bill can be modified and exported/synced to external systems.
+    FROZEN,      // Syncing to external systems is switched off. No changes can be made.
+    SUCCESSFUL,  // Final state for a valid bill.
+    VOID;        // Final state, indicating that the bill is not valid.
 
     // Legacy states, used by historical bills
     private static final String LEGACY_ISSUED = "ISSUED";
@@ -24,6 +24,13 @@ public enum BillStatus {
         return value;
     }
 
+    /**
+     * Returns true if the bill is in a final state.
+     */
+    public boolean isFinal() {
+        return this == SUCCESSFUL || this == VOID;
+    }
+
     public static BillStatus from(String status) {
         if (LEGACY_ISSUED.equals(status) || LEGACY_EXPORTED.equals(status)) return OPEN;
         if (LEGACY_CANCELED.equals(status)) return VOID;
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporterMock.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporterMock.java
index 899b31da361..21efa954cb0 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporterMock.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingReporterMock.java
@@ -16,7 +16,7 @@ public class BillingReporterMock implements BillingReporter {
     private final Clock clock;
     private final BillingDatabaseClient dbClient;
 
-    private final Map<Bill.Id, String> exportedBills = new HashMap<>();
+    private final Map<Bill.Id, InvoiceUpdate> exportedBills = new HashMap<>();
 
     public BillingReporterMock(Clock clock, BillingDatabaseClient dbClient) {
         this.clock = clock;
@@ -30,28 +30,58 @@ public class BillingReporterMock implements BillingReporter {
 
     @Override
     public InvoiceUpdate maintainInvoice(CloudTenant tenant, Bill bill) {
-        if (exportedBills.containsKey(bill.id())) {
+        if (! exportedBills.containsKey(bill.id())) {
+            // Given that it has been exported earlier (caller's responsibility), we can assume it has been removed.
+            return InvoiceUpdate.removed(bill.id());
+        }
+        if (exportedBills.get(bill.id()).type() == InvoiceUpdate.Type.MODIFIED) {
+            // modifyInvoice() has been called -> add a marker line item
+            if (bill.status() != BillStatus.OPEN) throw new IllegalArgumentException("Bill should be OPEN");
             dbClient.addLineItem(bill.tenant(), maintainedMarkerItem(), Optional.of(bill.id()));
-            return ModifiableInvoiceUpdate.of(bill.id(), 1, 0, 0);
-        } else {
-            return FailedInvoiceUpdate.removed(bill.id());
         }
+        return exportedBills.get(bill.id());
     }
 
     @Override
     public String exportBill(Bill bill, String exportMethod, CloudTenant tenant) {
         // Replace bill with a copy with exportedId set
         var exportedId = "EXPORTED-" + bill.id().value();
-        exportedBills.put(bill.id(), exportedId);
+        exportedBills.put(bill.id(), InvoiceUpdate.modifiable(bill.id(), null));
         dbClient.setExportedInvoiceId(bill.id(), exportedId);
         return exportedId;
     }
 
+    public void modifyInvoice(Bill.Id billId) {
+        ensureExported(billId);
+        var itemsUpdate = new InvoiceUpdate.ItemsUpdate(1, 0, 0);
+        exportedBills.put(billId, InvoiceUpdate.modifiable(billId, itemsUpdate));
+    }
+
+    public void freezeInvoice(Bill.Id billId) {
+        ensureExported(billId);
+        exportedBills.put(billId, InvoiceUpdate.unmodifiable(billId));
+    }
+
+    public void payInvoice(Bill.Id billId) {
+        ensureExported(billId);
+        exportedBills.put(billId, InvoiceUpdate.paid(billId));
+    }
+
+    public void voidInvoice(Bill.Id billId) {
+        ensureExported(billId);
+        exportedBills.put(billId, InvoiceUpdate.voided(billId));
+    }
+
     // Emulates deleting a bill in the external system.
-    public void deleteExportedBill(Bill.Id billId) {
+    public void deleteInvoice(Bill.Id billId) {
+        ensureExported(billId);
         exportedBills.remove(billId);
     }
 
+    private void ensureExported(Bill.Id billId) {
+        if (! exportedBills.containsKey(billId)) throw new IllegalArgumentException("Bill not exported");
+    }
+
     private static Bill.LineItem maintainedMarkerItem() {
         return new Bill.LineItem("maintained", "", BigDecimal.valueOf(0.0), "", "", ZonedDateTime.now());
     }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/FailedInvoiceUpdate.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/FailedInvoiceUpdate.java
deleted file mode 100644
index 9a93c7b05ec..00000000000
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/FailedInvoiceUpdate.java
+++ /dev/null
@@ -1,28 +0,0 @@
-package com.yahoo.vespa.hosted.controller.api.integration.billing;
-
-/**
- * @author gjoranv
- */
-public class FailedInvoiceUpdate extends InvoiceUpdate {
-
-    public enum Reason {
-        UNMODIFIABLE,
-        REMOVED
-    }
-
-    public final Reason reason;
-
-    public FailedInvoiceUpdate(Bill.Id billId, Reason reason) {
-        super(billId, ItemsUpdate.empty());
-        this.reason = reason;
-    }
-
-    public static FailedInvoiceUpdate unmodifiable(Bill.Id billId) {
-        return new FailedInvoiceUpdate(billId, Reason.UNMODIFIABLE);
-    }
-
-    public static FailedInvoiceUpdate removed(Bill.Id billId) {
-        return new FailedInvoiceUpdate(billId, Reason.REMOVED);
-    }
-
-}
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/InvoiceUpdate.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/InvoiceUpdate.java
index bb76834a483..4dfd9ef1fee 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/InvoiceUpdate.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/InvoiceUpdate.java
@@ -1,39 +1,54 @@
 package com.yahoo.vespa.hosted.controller.api.integration.billing;
 
-import java.util.Objects;
+import java.util.Optional;
 
 /**
- * Helper to track changes to an invoice.
+ * Helper to track changes to an invoice made by the controller. This should be independent
+ * of which external system that is being used.
  *
  * @author gjoranv
  */
-public abstract class InvoiceUpdate {
+public record InvoiceUpdate(Bill.Id billId, Type type, Optional<ItemsUpdate> itemsUpdate) {
+
+    public enum Type {
+        UNMODIFIED,    // The invoice was modifiable, but not modified by us
+        MODIFIED,      // The invoice was modified by us
+        UNMODIFIABLE,  // The invoice was unmodifiable in the external system
+        REMOVED,       // Removed from the external system, presumably for a valid reason
+        PAID,          // Reported paid from the external system
+        VOIDED         // Voided in the external system
+    }
+
+    public InvoiceUpdate {
+        if (type != Type.MODIFIED && itemsUpdate.isPresent())
+            throw new IllegalArgumentException("Items update is only allowed for modified invoices. Update type was " + type);
+    }
 
-    final Bill.Id billId;
-    final ItemsUpdate itemsUpdate;
+    public static InvoiceUpdate modifiable(Bill.Id billId, ItemsUpdate itemsUpdate) {
+        if (itemsUpdate == null || itemsUpdate.isEmpty()) {
+            return new InvoiceUpdate(billId, Type.UNMODIFIED, Optional.empty());
+        } else {
+            return new InvoiceUpdate(billId, Type.MODIFIED, Optional.of(itemsUpdate));
+        }
+    }
 
-    InvoiceUpdate(Bill.Id billId, ItemsUpdate itemsUpdate) {
-        this.billId = billId;
-        this.itemsUpdate = itemsUpdate;
+    public static InvoiceUpdate unmodifiable(Bill.Id billId) {
+        return new InvoiceUpdate(billId, Type.UNMODIFIABLE, Optional.empty());
     }
 
-    public Bill.Id billId() {
-        return billId;
+    public static InvoiceUpdate removed(Bill.Id billId) {
+        return new InvoiceUpdate(billId, Type.REMOVED, Optional.empty());
     }
 
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-        InvoiceUpdate that = (InvoiceUpdate) o;
-        return Objects.equals(billId, that.billId) && Objects.equals(itemsUpdate, that.itemsUpdate);
+    public static InvoiceUpdate paid(Bill.Id billId) {
+        return new InvoiceUpdate(billId, Type.PAID, Optional.empty());
     }
 
-    @Override
-    public int hashCode() {
-        return Objects.hash(billId, itemsUpdate);
+    public static InvoiceUpdate voided(Bill.Id billId) {
+        return new InvoiceUpdate(billId, Type.VOIDED, Optional.empty());
     }
 
+
     public record ItemsUpdate(int itemsAdded, int itemsRemoved, int itemsModified) {
 
         public boolean isEmpty() {
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/ModifiableInvoiceUpdate.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/ModifiableInvoiceUpdate.java
deleted file mode 100644
index 75cce564fc7..00000000000
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/ModifiableInvoiceUpdate.java
+++ /dev/null
@@ -1,28 +0,0 @@
-package com.yahoo.vespa.hosted.controller.api.integration.billing;
-
-/**
- * @author gjoranv
- */
-public class ModifiableInvoiceUpdate extends InvoiceUpdate {
-
-    public ModifiableInvoiceUpdate(Bill.Id billId, ItemsUpdate itemsUpdate) {
-        super(billId, itemsUpdate);
-    }
-
-    public ItemsUpdate itemsUpdate() {
-        return itemsUpdate;
-    }
-
-    public boolean isEmpty() {
-        return itemsUpdate.isEmpty();
-    }
-
-    public static ModifiableInvoiceUpdate of(Bill.Id billId, int itemsAdded, int itemsRemoved, int itemsModified) {
-        return new ModifiableInvoiceUpdate(billId, new ItemsUpdate(itemsAdded, itemsRemoved, itemsModified));
-    }
-
-    @Override
-    public boolean equals(Object o) {
-        return super.equals(o);
-    }
-}
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/StatusHistory.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/StatusHistory.java
index f0c7f806c8c..788995555a8 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/StatusHistory.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/StatusHistory.java
@@ -53,7 +53,7 @@ public class StatusHistory {
         return switch(current) {
             case OPEN -> true;
             case FROZEN -> newStatus != BillStatus.OPEN; // This could be subject to change.
-            case CLOSED -> newStatus == BillStatus.CLOSED;
+            case SUCCESSFUL -> newStatus == BillStatus.SUCCESSFUL;
             case VOID -> newStatus == BillStatus.VOID;
         };
     }