Merge pull request #29077 from vespa-engine/bill-state-transitions

Add bill status logic.

4 files changed, 122 insertions, 3 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 d6c6262069b..4f35b47219a 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
@@ -5,12 +5,14 @@ package com.yahoo.vespa.hosted.controller.api.integration.billing;
  */
 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. Reviews should be done in this state.
+    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.
 
-    private static final String LEGACY_ISSUED = "ISSUED";         // Legacy state, used by historical bills
-    private static final String LEGACY_EXPORTED = "EXPORTED";     // Legacy state, used by historical bills
+    // Legacy states, used by historical bills
+    private static final String LEGACY_ISSUED = "ISSUED";
+    private static final String LEGACY_EXPORTED = "EXPORTED";
+    private static final String LEGACY_CANCELED = "CANCELED";
 
     private final String value;
 
@@ -24,6 +26,7 @@ public enum BillStatus {
 
     public static BillStatus from(String status) {
         if (LEGACY_ISSUED.equals(status) || LEGACY_EXPORTED.equals(status)) return OPEN;
+        if (LEGACY_CANCELED.equals(status)) return VOID;
         return Enum.valueOf(BillStatus.class, status.toUpperCase());
     }
 
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 6335ada1396..f0c7f806c8c 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
@@ -14,6 +14,17 @@ public class StatusHistory {
     SortedMap<ZonedDateTime, BillStatus> history;
 
     public StatusHistory(SortedMap<ZonedDateTime, BillStatus> history) {
+        // Validate the given history
+        var iter = history.values().iterator();
+        BillStatus next  = iter.hasNext() ? iter.next() : null;
+        while (iter.hasNext()) {
+            var current = next;
+            next = iter.next();
+            if (! validateStatus(current, next)) {
+                throw new IllegalArgumentException("Invalid transition from " + current + " to " + next);
+            }
+        }
+
         this.history = history;
     }
 
@@ -32,4 +43,19 @@ public class StatusHistory {
         return history;
     }
 
+    public void checkValidTransition(BillStatus newStatus) {
+        if (! validateStatus(current(), newStatus)) {
+            throw new IllegalArgumentException("Invalid transition from " + current() + " to " + newStatus);
+        }
+    }
+
+    private static boolean validateStatus(BillStatus current, BillStatus newStatus) {
+        return switch(current) {
+            case OPEN -> true;
+            case FROZEN -> newStatus != BillStatus.OPEN; // This could be subject to change.
+            case CLOSED -> newStatus == BillStatus.CLOSED;
+            case VOID -> newStatus == BillStatus.VOID;
+        };
+    }
+
 }
diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillStatusTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillStatusTest.java
index 9647a11a4ba..022474406b9 100644
--- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillStatusTest.java
+++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillStatusTest.java
@@ -13,6 +13,7 @@ public class BillStatusTest {
     void legacy_states_are_converted() {
         assertEquals(BillStatus.OPEN, BillStatus.from("ISSUED"));
         assertEquals(BillStatus.OPEN, BillStatus.from("EXPORTED"));
+        assertEquals(BillStatus.VOID, BillStatus.from("CANCELED"));
     }
 
 }
diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/billing/StatusHistoryTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/billing/StatusHistoryTest.java
new file mode 100644
index 00000000000..46a4c7e199c
--- /dev/null
+++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/billing/StatusHistoryTest.java
@@ -0,0 +1,89 @@
+package com.yahoo.vespa.hosted.controller.api.integration.billing;
+
+import org.junit.jupiter.api.Test;
+
+import java.time.Clock;
+import java.time.ZonedDateTime;
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.TreeMap;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * @author gjoranv
+ */
+public class StatusHistoryTest {
+
+    private final Clock clock = Clock.systemUTC();
+
+    @Test
+    void open_can_change_to_any_status() {
+        var history = StatusHistory.open(clock);
+        history.checkValidTransition(BillStatus.FROZEN);
+        history.checkValidTransition(BillStatus.CLOSED);
+        history.checkValidTransition(BillStatus.VOID);
+    }
+
+    @Test
+    void frozen_cannot_change_to_open() {
+        var history = new StatusHistory(historyWith(BillStatus.FROZEN));
+
+        history.checkValidTransition(BillStatus.CLOSED);
+        history.checkValidTransition(BillStatus.VOID);
+
+        assertThrows(IllegalArgumentException.class, () -> history.checkValidTransition(BillStatus.OPEN));
+    }
+
+    @Test
+    void closed_cannot_change() {
+        var history = new StatusHistory(historyWith(BillStatus.CLOSED));
+
+        assertThrows(IllegalArgumentException.class, () -> history.checkValidTransition(BillStatus.OPEN));
+        assertThrows(IllegalArgumentException.class, () -> history.checkValidTransition(BillStatus.FROZEN));
+        assertThrows(IllegalArgumentException.class, () -> history.checkValidTransition(BillStatus.VOID));
+    }
+
+    @Test
+    void void_cannot_change() {
+        var history = new StatusHistory(historyWith(BillStatus.VOID));
+
+        assertThrows(IllegalArgumentException.class, () -> history.checkValidTransition(BillStatus.OPEN));
+        assertThrows(IllegalArgumentException.class, () -> history.checkValidTransition(BillStatus.FROZEN));
+        assertThrows(IllegalArgumentException.class, () -> history.checkValidTransition(BillStatus.CLOSED));
+    }
+
+    @Test
+    void any_status_can_change_to_itself() {
+        var history = new StatusHistory(historyWith(BillStatus.OPEN));
+        history.checkValidTransition(BillStatus.OPEN);
+
+        history = new StatusHistory(historyWith(BillStatus.FROZEN));
+        history.checkValidTransition(BillStatus.FROZEN);
+
+        history = new StatusHistory(historyWith(BillStatus.CLOSED));
+        history.checkValidTransition(BillStatus.CLOSED);
+
+        history = new StatusHistory(historyWith(BillStatus.VOID));
+        history.checkValidTransition(BillStatus.VOID);
+    }
+
+    @Test
+    void it_validates_status_history_in_constructor() {
+        assertThrows(IllegalArgumentException.class, () -> new StatusHistory(historyWith(BillStatus.FROZEN, BillStatus.OPEN)));
+        assertThrows(IllegalArgumentException.class, () -> new StatusHistory(historyWith(BillStatus.CLOSED, BillStatus.OPEN)));
+        assertThrows(IllegalArgumentException.class, () -> new StatusHistory(historyWith(BillStatus.CLOSED, BillStatus.FROZEN)));
+        assertThrows(IllegalArgumentException.class, () -> new StatusHistory(historyWith(BillStatus.CLOSED, BillStatus.VOID)));
+        assertThrows(IllegalArgumentException.class, () -> new StatusHistory(historyWith(BillStatus.VOID, BillStatus.OPEN)));
+        assertThrows(IllegalArgumentException.class, () -> new StatusHistory(historyWith(BillStatus.VOID, BillStatus.FROZEN)));
+        assertThrows(IllegalArgumentException.class, () -> new StatusHistory(historyWith(BillStatus.VOID, BillStatus.CLOSED)));
+    }
+
+    private SortedMap<ZonedDateTime, BillStatus> historyWith(BillStatus... statuses) {
+        var history = new TreeMap<>(Map.of(ZonedDateTime.now(clock), BillStatus.OPEN));
+        for (var status : statuses) {
+            history.put(ZonedDateTime.now(clock), status);
+        }
+        return history;
+    }
+}