Skip to content

Commit 95cef9c

Browse files
committed
Address PR review: trim comments, split checkout components, metering fixes
- Trim verbose/AI-style comments across account-link metering files - Split checkout status views into CheckoutFinalizing/CheckoutActivationSlow - Dedup: 5-min workflow-window parity with cloud (last_metered_at + window prop) - Gate: deplete spend cap by local unsynced usage for capped subscriptions too - DRY per-category mapping via BillingCategory EnumMap - Meter hot path: hash during temp-file write (DigestOutputStream); load sync state once
1 parent bf6d6ba commit 95cef9c

17 files changed

Lines changed: 443 additions & 323 deletions

File tree

app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkClient.java

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,8 @@
3838
* /api/v1/instance/revoke-self}).
3939
* </ul>
4040
*
41-
* <p>Uses {@code java.net.http.HttpClient} (the established self-hosted outbound pattern, see
42-
* {@code AiEngineClient}). The base URL + client are injectable so tests can stub the SaaS
43-
* endpoint.
41+
* <p>Uses {@code java.net.http.HttpClient} (the established self-hosted outbound pattern; see
42+
* {@code AiEngineClient}); base URL + client are injectable so tests can stub SaaS.
4443
*/
4544
@Slf4j
4645
@Service
@@ -94,11 +93,9 @@ public int status() {
9493
}
9594

9695
/**
97-
* Authoritative deny (401/403) from the entitlement endpoint — the device credential is revoked
98-
* or invalid. Distinct from a transport/server failure (which returns {@code null} and fails
99-
* open): the cache must BLOCK billable work on this rather than serve a stale entitled
100-
* snapshot. Unchecked so it propagates cleanly through {@link #fetchEntitlement}'s transport
101-
* try/catch.
96+
* Authoritative deny (401/403) — the device credential is revoked or invalid. Unlike a
97+
* transport/server failure (which returns {@code null} and fails open), the cache must BLOCK on
98+
* this. Unchecked so it propagates through {@link #fetchEntitlement}'s transport try/catch.
10299
*/
103100
public static final class RevokedException extends RuntimeException {
104101
private final int status;
@@ -150,11 +147,9 @@ public RegisterResult register(String supabaseJwt, String instanceName) throws I
150147
}
151148

152149
/**
153-
* Revokes this instance's own credential on the SaaS side ({@code POST
154-
* /api/v1/instance/revoke-self}), authenticated by the device credential — a credential is
155-
* allowed to revoke its own identity. Best-effort: returns {@code false} if SaaS is unreachable
156-
* or rejects the call, so the caller (local unlink) can still clear locally and log the orphan
157-
* row for follow-up. Idempotent on SaaS (already-revoked → still 204).
150+
* Revokes this instance's own credential on the SaaS side, authenticated by that credential.
151+
* Best-effort: returns {@code false} if SaaS is unreachable or rejects, so the caller (local
152+
* unlink) can still clear locally and log the orphan for follow-up. Idempotent on SaaS.
158153
*/
159154
public boolean revokeSelf(String deviceId, String deviceSecret) {
160155
try {
@@ -227,14 +222,11 @@ public InstanceEntitlement fetchEntitlement(String deviceId, String deviceSecret
227222
}
228223

229224
/**
230-
* Reports the current period's cumulative per-category units to {@code POST
231-
* /api/v1/instance/sync} and returns the fresh entitlement in the same reply — one round-trip
232-
* both reports usage and refreshes the gate. SaaS bills the delta against its own last-seen
233-
* cumulative, so reporting the same totals twice charges nothing (idempotent).
234-
*
235-
* <p>Same three outcomes as {@link #fetchEntitlement}: 2xx → parsed snapshot; 401/403 → {@link
236-
* RevokedException}; transport / other non-2xx / malformed body → {@code null} (caller must not
237-
* advance its last-synced markers, so the usage retries on the next sync).
225+
* Reports the period's cumulative per-category units to {@code POST /api/v1/instance/sync} and
226+
* returns the fresh entitlement in the same reply — one round-trip both reports and refreshes.
227+
* SaaS bills the delta against its last-seen cumulative, so resending the same totals is
228+
* idempotent. Same three outcomes as {@link #fetchEntitlement}; on {@code null} the caller must
229+
* not advance its last-synced markers so the usage retries next sync.
238230
*/
239231
public InstanceEntitlement reportUsage(
240232
String deviceId,
@@ -248,8 +240,7 @@ public InstanceEntitlement reportUsage(
248240
try {
249241
ObjectNode root = mapper.createObjectNode();
250242
root.put("syncSeq", syncSeq);
251-
// periodStart as an explicit ISO-8601 string so it round-trips to the SaaS
252-
// LocalDateTime regardless of the mapper's time-module config.
243+
// Explicit ISO-8601 string so it round-trips regardless of the mapper's time config.
253244
root.put("periodStart", periodStart.toString());
254245
ObjectNode units = root.putObject("cumulativeUnits");
255246
units.put("api", apiUnits);

app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkProperties.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package stirling.software.proprietary.accountlink;
22

3+
import java.time.Duration;
4+
35
import org.springframework.boot.context.properties.ConfigurationProperties;
46
import org.springframework.stereotype.Component;
57

@@ -62,5 +64,13 @@ public static class Metering {
6264
* Block billable work after this many days with no successful sync (fail-open → closed).
6365
*/
6466
private int graceDays = 3;
67+
68+
/**
69+
* Dedup window for identical input sets. A re-run of the same inputs within this window is
70+
* treated as workflow chaining and not re-charged; the same inputs run again after it are
71+
* billed afresh. Mirrors the cloud's {@code payg.lineage.workflow-window} so the same op
72+
* costs the same on the instance and in the cloud.
73+
*/
74+
private Duration workflowWindow = Duration.ofMinutes(5);
6575
}
6676
}

app/proprietary/src/main/java/stirling/software/proprietary/accountlink/EntitlementCache.java

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,13 @@
1212
import lombok.extern.slf4j.Slf4j;
1313

1414
/**
15-
* Caches the linked team's entitlement so the request-time gate does not call the SaaS backend on
16-
* every billable request. Single-slot (one instance = one linked team), TTL-based.
15+
* Caches the linked team's entitlement so the request-time gate needn't call SaaS on every billable
16+
* request. Single-slot (one instance = one linked team), TTL-based.
1717
*
18-
* <p>Fail-open friendly for TRANSPORT failures: {@link #current()} returns the freshest snapshot it
19-
* has, even if a refresh just failed; it returns {@link Optional#empty()} only when nothing has
20-
* ever been fetched <i>and</i> the latest refresh failed (the gate treats empty as "unknown →
21-
* allow").
22-
*
23-
* <p>But an AUTHORITATIVE deny (revoked/invalid credential → {@link
24-
* AccountLinkClient.RevokedException}) is NOT a transport failure: the snapshot is replaced with a
25-
* {@link EntitlementState#REVOKED} blocked entitlement so the gate stops billable work immediately
26-
* rather than serving a stale entitled snapshot.
18+
* <p>A transport failure fails open — {@link #current()} keeps serving the freshest snapshot it has
19+
* and returns {@link Optional#empty()} ("unknown → allow") only when nothing was ever fetched. An
20+
* authoritative deny ({@link AccountLinkClient.RevokedException}) does not: the snapshot is
21+
* replaced with a {@link EntitlementState#REVOKED} entitlement so the gate blocks immediately.
2722
*/
2823
@Slf4j
2924
@Service
@@ -63,9 +58,8 @@ public EntitlementCache(
6358
* not linked or the SaaS side is unreachable and we have no prior snapshot.
6459
*/
6560
public Optional<InstanceEntitlement> current() {
66-
// Single-flight: when stale, exactly one thread refreshes (blocking on the SaaS
67-
// call) while concurrent callers serve the last snapshot — no thundering herd of
68-
// synchronous round-trips on the billable hot path. Safe because the gate fails open.
61+
// Single-flight: when stale, exactly one thread refreshes while concurrent callers serve
62+
// the last snapshot — no thundering herd of round-trips on the billable hot path.
6963
if (isStale(snapshot) && refreshing.compareAndSet(false, true)) {
7064
try {
7165
refresh();
@@ -77,16 +71,15 @@ public Optional<InstanceEntitlement> current() {
7771
}
7872

7973
private boolean isStale(Snapshot snap) {
80-
// fetchedAt is the last *attempt* time (stamped on success AND failure), so a failed
81-
// fetch backs off for a full TTL instead of every billable request re-triggering a
82-
// blocking round-trip against a dead/slow SaaS endpoint.
74+
// fetchedAt is the last *attempt* time (stamped on success and failure), so a failed fetch
75+
// backs off a full TTL instead of every request re-triggering a round-trip to a dead SaaS.
8376
return Duration.between(snap.fetchedAt(), Instant.now()).compareTo(ttl) >= 0;
8477
}
8578

8679
/**
87-
* Pulls a fresh snapshot. Keeps the previous entitlement on a TRANSPORT failure (fail-open) but
88-
* still stamps the attempt time so re-fetches throttle to the TTL; on an AUTHORITATIVE deny
89-
* (revoked credential) replaces it with a blocked snapshot so the gate stops billable work.
80+
* Pulls a fresh snapshot. On a transport failure keeps the previous entitlement but stamps the
81+
* attempt time so re-fetches throttle to the TTL; on an authoritative deny replaces it with a
82+
* blocked snapshot.
9083
*/
9184
void refresh() {
9285
Optional<DeviceCredential> cred = credentialStore.get();
@@ -101,15 +94,15 @@ void refresh() {
10194
if (fresh != null) {
10295
snapshot = new Snapshot(fresh, Instant.now());
10396
} else {
104-
// Unreachable / server error: keep the last known entitlement (may be null) but
105-
// stamp the attempt so we don't hammer SaaS; the gate fails open in the meantime.
97+
// Unreachable / server error: keep the last known entitlement but stamp the attempt
98+
// so we don't hammer SaaS; the gate fails open meanwhile.
10699
log.debug(
107100
"Entitlement refresh failed; reusing last known snapshot, backing off a TTL");
108101
snapshot = new Snapshot(snapshot.entitlement(), Instant.now());
109102
}
110103
} catch (AccountLinkClient.RevokedException e) {
111-
// Authoritative deny — credential revoked/invalid. Do NOT fail open: block immediately
112-
// rather than serving the stale entitled snapshot until the next unlink.
104+
// Authoritative deny — block immediately rather than serving the stale entitled
105+
// snapshot.
113106
log.info(
114107
"Entitlement denied (HTTP {}); blocking billable work for the revoked credential",
115108
e.status());
@@ -123,8 +116,8 @@ public void invalidate() {
123116
}
124117

125118
/**
126-
* Seeds the cache with an entitlement obtained out-of-band the daily usage sync gets a fresh
127-
* snapshot back in its reply, so adopting it here saves a redundant fetch. No-op on null.
119+
* Seeds the cache with an entitlement obtained out-of-band (the sync reply carries a fresh
120+
* one), saving a redundant fetch. No-op on null.
128121
*/
129122
public void accept(InstanceEntitlement fresh) {
130123
if (fresh != null) {

app/proprietary/src/main/java/stirling/software/proprietary/accountlink/InstanceEntitlementGate.java

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -64,27 +64,30 @@ public GateDecision evaluate(boolean billable) {
6464
Optional<InstanceEntitlement> entitlement =
6565
linked ? entitlementCache.current() : Optional.empty();
6666
boolean graceExpired = linked && entitlement.isEmpty() && isGraceExpired();
67-
// For an unsubscribed team the one-time free grant is the ceiling, and it depletes in real
68-
// time as billable work accrues locally between daily syncs. Subtract the not-yet-synced
69-
// local usage from the last-synced free balance so the gate stops AT the grant, rather than
70-
// running until the next sync charges the backlog and only then flips. Subscribed teams
71-
// bill
72-
// past the grant (their gate is the money cap), so this doesn't apply to them — pass 0.
67+
// Deplete the applicable ceiling — free grant (unsubscribed) or spend cap (capped
68+
// subscription) — by local usage not yet synced, so the gate stops in real time instead of
69+
// overshooting until the next sync. An uncapped subscription has no ceiling to deplete → 0.
7370
long pendingUnsynced =
74-
entitlement.map(e -> !e.subscribed()).orElse(false)
71+
entitlement.map(InstanceEntitlementGate::depletesCeiling).orElse(false)
7572
? localUsageService.currentPeriodUnsynced().totalUnsyncedUnits()
7673
: 0L;
7774
return decide(true, true, linked, entitlement, graceExpired, pendingUnsynced);
7875
}
7976

77+
/** Whether local unsynced usage pushes against a real ceiling (free grant or a spend cap). */
78+
private static boolean depletesCeiling(InstanceEntitlement e) {
79+
return !e.subscribed() || e.periodCapUnits() != null;
80+
}
81+
8082
/**
8183
* Pure decision function — no Spring, no I/O. {@code entitlement} empty means "unknown"
8284
* (unreachable): when linked, that fails open unless {@code graceExpired} (the metering grace
8385
* window elapsed with no authoritative contact), in which case it blocks.
8486
*
85-
* @param pendingUnsyncedUnits billable units accrued locally since the last sync — subtracted
86-
* from an unsubscribed team's free balance so the grant depletes in real time (0 for
87-
* subscribed / unknown-entitlement cases, where it has no effect).
87+
* @param pendingUnsyncedUnits billable units accrued locally since the last sync — depletes the
88+
* free grant (unsubscribed) or the spend cap (capped subscription) in real time so the gate
89+
* stops without waiting for the next sync (0 for uncapped-subscribed / unknown-entitlement
90+
* cases, where it has no effect).
8891
*/
8992
public static GateDecision decide(
9093
boolean flagEnabled,
@@ -103,10 +106,9 @@ public static GateDecision decide(
103106
return GateDecision.block(GateDecision.Reason.NOT_LINKED);
104107
}
105108
if (entitlement.isEmpty()) {
106-
// Linked but entitlement source unreachable. Normally fail open (never hard-block on
107-
// our
108-
// inability to reach billing) — but once the grace window has expired, block so the
109-
// fail-open can't grant unbounded free/unbilled billable work indefinitely.
109+
// Linked but entitlement unreachable: fail open, unless the grace window has expired
110+
// (so
111+
// the fail-open can't grant unbounded unbilled work forever).
110112
return graceExpired
111113
? GateDecision.block(GateDecision.Reason.GRACE_EXPIRED)
112114
: GateDecision.allow(GateDecision.Reason.FAIL_OPEN);
@@ -122,11 +124,9 @@ public static GateDecision decide(
122124
}
123125

124126
/**
125-
* True when metering is on and SaaS has been unreachable past the grace window — i.e. it's been
126-
* {@code graceDays} since the last authoritative contact. The reference is the last successful
127-
* daily sync (persisted, survives restart), falling back to the link time for a never-synced
128-
* instance. {@code graceDays <= 0} disables the backstop; metering off never blocks (nothing
129-
* accrues, so a stale sync must not gate manual-free or pre-metering work).
127+
* True when metering is on and it's been {@code graceDays} since the last authoritative contact
128+
* (last successful sync, or link time if never synced). {@code graceDays <= 0} or metering off
129+
* disables the backstop.
130130
*/
131131
private boolean isGraceExpired() {
132132
AccountLinkProperties.Metering metering = properties.getMetering();
@@ -158,13 +158,17 @@ private static boolean entitled(InstanceEntitlement e, long pendingUnsyncedUnits
158158
return false;
159159
}
160160
if (e.subscribed()) {
161-
// Subscribed: allowed unless a period cap is set and exceeded.
162-
return e.periodCapUnits() == null || e.periodSpendUnits() < e.periodCapUnits();
161+
if (e.periodCapUnits() == null) {
162+
return true; // uncapped subscription
163+
}
164+
// Project the cap the way the grant is projected: synced paid spend plus the paid part
165+
// of local usage not yet synced (free grant is consumed first, so only the excess
166+
// bills) — stops at the cap in real time instead of overshooting until the next sync.
167+
long pendingPaid = Math.max(0, pendingUnsyncedUnits - e.freeRemainingUnits());
168+
return e.periodSpendUnits() + pendingPaid < e.periodCapUnits();
163169
}
164-
// Unsubscribed: the free pool must cover both what SaaS has already charged (already netted
165-
// out of freeRemainingUnits) and the local usage accrued since the last sync but not yet
166-
// reported. Depleting by the pending delta stops the (grant+1)-th unit here in real time,
167-
// instead of allowing it until a sync reconciles and the gate belatedly flips.
170+
// Unsubscribed: free pool must cover SaaS-charged usage (in freeRemainingUnits) plus local
171+
// usage not yet synced — deplete by the pending delta so we stop at the grant in real time.
168172
return e.freeRemainingUnits() - pendingUnsyncedUnits > 0;
169173
}
170174
}

0 commit comments

Comments
 (0)