Add plan: live vault balance from /money endpoint

- 4 tasks: failing test, helper implementation, vaultLoop switch, smoke check - New helper fetchLiveVaultBalance(string $user): ?int in includes/utilities.php - vaultLoop prefers the live value, falls back to MAX(running_balance) - Per-request cache to avoid N HTTP calls - Test hook via $GLOBALS['liveBalanceTestHook'] for unit testing - Falls back gracefully on API failure
2026-08-05 09:23:15 -05:00
parent 64e320c6a5
commit 0613d00fbb
1 changed files with 389 additions and 0 deletions
@@ -0,0 +1,389 @@
+# Live Vault Balance From `/money` Endpoint — Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Replace `MAX(running_balance)` (which uses stale `/log` data) with the live `/money` endpoint's `vault` field for the balances shown in the right-side panel. Falls back to the SQL MAX when the API fails.
+
+**Architecture:** New `fetchLiveVaultBalance(string $user): ?int` helper in `includes/utilities.php`. Hits `https://api.torn.com/v2/user?selections=money` with the user's API key. Returns the `money.vault` integer or null on failure. In-memory static cache so multiple calls per request reuse one fetch.
+
+**Tech Stack:** PHP 8.1+, SQLite3 via PDO, cURL. No new dependencies.
+
+## Global Constraints
+
+- PHP 8.1+ syntax only.
+- API authentication: `Authorization: ApiKey <key>` header.
+- Idempotency: every change must be safe to re-run.
+- The `running_balance` column, the "Balance After" column in `buildTable`, and the `backfillRunningBalances` helper are unchanged. This plan only changes the *displayed* balance (the right-side panel), not the per-entry historical record.
+- Falls back gracefully when the API fails — the page must still render.
+- Commits per task. Do not push.
+- All existing helpers outside the rewritten ones stay byte-identical.
+
+---
+
+### Task 1: Write the failing test for `fetchLiveVaultBalance`
+
+**Files:**
+- Create: `tests/live_balance_test.php`
+
+**Interfaces (the test exercises these):**
+- `fetchLiveVaultBalance(string $user): ?int`
+  - On success, returns the integer `vault` amount from `/money`.
+  - On API failure (network error, bad JSON, missing field), returns null.
+  - Caches the result within a single PHP request — multiple calls for the same user do not re-fetch.
+
+The test does NOT need a live API call. It exercises the helper by either:
+- Mocking at the `executeApiCall` boundary (preferred — test swaps in a stub function), OR
+- Mocking the API response via a wrapper.
+
+Choose whichever is cleanest. Since `executeApiCall` is a plain PHP function (not a class method), wrapping it for testing is awkward. Instead, design the helper to accept an optional override URL, OR refactor the helper to internally call a swappable transport function.
+
+**Recommended approach:** Add a static class property or file-static variable `$liveBalanceTestHook` in `includes/utilities.php` that defaults to null. If set, the helper calls it instead of `executeApiCall`. The test sets the hook before calling the helper, then unsets it.
+
+This is a 2-line addition to the helper and makes the test trivial. Don't add a sophisticated test harness.
+
+- [ ] **Step 1: Create the test file**
+
+Create `tests/live_balance_test.php`:
+
+```php
+<?php
+/**
+ * Test for fetchLiveVaultBalance() — the live vault balance helper.
+ *
+ * Uses a swappable test hook so we don't make real API calls.
+ *
+ * Run: php tests/live_balance_test.php
+ * Exit code 0 = pass.
+ */
+
+declare(strict_types=1);
+
+if (!defined('USER_KEYS')) {
+    define('USER_KEYS', [
+        'zarathos' => 'test-key-not-used',
+        'symos'    => 'test-key-not-used',
+    ]);
+}
+
+require_once __DIR__ . '/../includes/exceptions.php';
+require_once __DIR__ . '/../includes/utilities.php';
+
+function fail(string $message): void {
+    fwrite(STDERR, "FAIL: $message\n");
+    exit(1);
+}
+
+function assertSame($expected, $actual, string $label): void {
+    if ($expected !== $actual) {
+        $exp = var_export($expected, true);
+        $act = var_export($actual, true);
+        fail("$label: expected $exp, got $act");
+    }
+}
+
+// Test hook: when set, fetchLiveVaultBalance calls this closure instead of
+// executeApiCall. The closure receives ($url, $apiKey) and returns the
+// decoded response array (or throws).
+$GLOBALS['liveBalanceTestHook'] = null;
+
+function liveBalanceHook($url, $apiKey) {
+    $hook = $GLOBALS['liveBalanceTestHook'] ?? null;
+    if ($hook === null) {
+        fail('Test hook not set — fetchLiveVaultBalance called executeApiCall');
+    }
+    return $hook($url, $apiKey);
+}
+
+// --- Test 1: success path returns the vault amount as int ---
+$GLOBALS['liveBalanceTestHook'] = function ($url, $apiKey) {
+    assertSame('zarathos', $apiKey === 'test-key-not-used' ? 'zarathos' : 'symos', 'api key routed');
+    // (just verify the hook got called)
+    return ['money' => ['vault' => 160627669, 'wallet' => 4000]];
+};
+$balance = fetchLiveVaultBalance('zarathos');
+assertSame(160627669, $balance, 'success returns vault amount');
+
+// --- Test 2: failure path returns null ---
+$GLOBALS['liveBalanceTestHook'] = function () {
+    throw new CurlErrorException('connection refused');
+};
+$balance = fetchLiveVaultBalance('zarathos');
+assertSame(null, $balance, 'failure returns null');
+
+// --- Test 3: missing money.vault returns null ---
+$GLOBALS['liveBalanceTestHook'] = function () {
+    return ['error' => ['code' => 4]];
+};
+$balance = fetchLiveVaultBalance('zarathos');
+assertSame(null, $balance, 'missing field returns null');
+
+// --- Test 4: cache: second call does not invoke the hook twice ---
+$invocations = 0;
+$GLOBALS['liveBalanceTestHook'] = function () use (&$invocations) {
+    $invocations++;
+    return ['money' => ['vault' => 100]];
+};
+fetchLiveVaultBalance('symos');
+fetchLiveVaultBalance('symos');
+fetchLiveVaultBalance('symos');
+assertSame(1, $invocations, 'cache prevents repeated fetches');
+
+echo "OK: fetchLiveVaultBalance tests passed.\n";
+```
+
+The test sets `$GLOBALS['liveBalanceTestHook']` and the helper must check this global and call it instead of `executeApiCall`. This is the swappable-transport mechanism described above.
+
+- [ ] **Step 2: Run the test and verify it fails**
+
+Run: `php tests/live_balance_test.php`
+Expected: failure with `Test hook not set` (because the helper doesn't exist yet, and even if it did, the test hook mechanism isn't there yet).
+
+- [ ] **Step 3: Commit the failing test**
+
+```bash
+git add tests/live_balance_test.php
+git commit -m "Add failing test for fetchLiveVaultBalance"
+```
+
+---
+
+### Task 2: Implement `fetchLiveVaultBalance` with test hook
+
+**Files:**
+- Modify: `includes/utilities.php` (append new function + a tiny test-hook shim)
+
+**Interfaces:**
+- `fetchLiveVaultBalance(string $user): ?int`
+  - Returns `int` on success (the `money.vault` value).
+  - Returns `null` on any failure (no exception is raised).
+  - Per-request in-memory cache: subsequent calls for the same user return the cached value without hitting the API.
+
+- [ ] **Step 1: Add the test-hook shim**
+
+At the very top of `includes/utilities.php`, immediately after the `require_once` lines that pull in exceptions, add (only if the helper is being unit-tested):
+
+```php
+// Test hook for fetchLiveVaultBalance(). When set, this closure is called
+// instead of executeApiCall. Set $GLOBALS['liveBalanceTestHook'] to a
+// closure($url, $apiKey): array in test code. Production code leaves it null.
+$GLOBALS['liveBalanceTestHook'] = $GLOBALS['liveBalanceTestHook'] ?? null;
+```
+
+(If the file already requires things at the top, just place the line after the existing requires. The point is: the helper checks this global.)
+
+- [ ] **Step 2: Append the new function**
+
+Append this function to `includes/utilities.php` (after `consoleLog`):
+
+```php
+/**
+ * Fetch the live vault balance for a user from the v2 /money endpoint.
+ *
+ * Returns the `money.vault` value as an int, or null if the API call
+ * fails, returns invalid JSON, or doesn't include `money.vault`. The
+ * result is cached for the duration of the PHP request so multiple
+ * callers (e.g., `vaultLoop` for one user, then the all-users sum)
+ * don't re-fetch.
+ *
+ * @param string $user The user whose vault balance to fetch.
+ *
+ * @return int|null The vault amount in pennies, or null on failure.
+ */
+function fetchLiveVaultBalance($user) {
+    static $cache = [];
+
+    if (!array_key_exists($user, $cache)) {
+        if (!array_key_exists($user, USER_KEYS)) {
+            $cache[$user] = null;
+            return null;
+        }
+
+        $url = 'https://api.torn.com/v2/user?selections=money';
+        try {
+            $hook = $GLOBALS['liveBalanceTestHook'] ?? null;
+            $responseData = $hook !== null
+                ? $hook($url, USER_KEYS[$user])
+                : executeApiCall($url, USER_KEYS[$user]);
+
+            if (!isset($responseData['money']['vault'])) {
+                $cache[$user] = null;
+                return null;
+            }
+
+            $cache[$user] = (int)$responseData['money']['vault'];
+        } catch (Exception $e) {
+            $cache[$user] = null;
+        }
+    }
+
+    return $cache[$user];
+}
+```
+
+- [ ] **Step 3: Run the test and verify it passes**
+
+Run: `php tests/live_balance_test.php`
+Expected: `OK: fetchLiveVaultBalance tests passed.`
+
+- [ ] **Step 4: Run the existing golden-file test to ensure no regression**
+
+Run: `php tests/process_log_entries_test.php`
+Expected: `OK: processLogEntries v2 golden-file test passed (100 entries).`
+
+- [ ] **Step 5: Verify the file parses**
+
+Run: `php -l includes/utilities.php`
+Expected: `No syntax errors detected`.
+
+- [ ] **Step 6: Commit**
+
+```bash
+git add includes/utilities.php
+git commit -m "Add fetchLiveVaultBalance helper using v2 /money endpoint"
+```
+
+---
+
+### Task 3: Update `vaultLoop` to use the live balance with fallback
+
+**Files:**
+- Modify: `includes/utilities.php` (`vaultLoop` only)
+
+**Interfaces:**
+- `vaultLoop($name=null)`: returns the live vault balance (from `fetchLiveVaultBalance`) when available; falls back to `MAX(running_balance)` when the helper returns null. Behavior contract unchanged: returns int, or 0 when nothing applies.
+
+- [ ] **Step 1: Replace `vaultLoop`**
+
+In `includes/utilities.php`, replace the existing `vaultLoop` function (at the SQL-based implementation from the previous balance-fix plan) with:
+
+```php
+function vaultLoop ($name=null) {
+    $liveBalance = $name === null
+        ? null
+        : fetchLiveVaultBalance($name);
+
+    if ($liveBalance !== null) {
+        return $liveBalance;
+    }
+
+    // Fallback: derive from stored entries. Used when the live /money call
+    // fails or is not configured for the user.
+    $pdo = getDatabaseConnection();
+
+    if ($name === null) {
+        $stmt = $pdo->query(
+            'SELECT COALESCE(SUM(max_balance), 0) FROM '
+            . '(SELECT MAX(running_balance) AS max_balance FROM vault GROUP BY user)'
+        );
+
+        return (int)$stmt->fetchColumn();
+    }
+
+    $stmt = $pdo->prepare('SELECT MAX(running_balance) FROM vault WHERE user = :user');
+    $stmt->bindValue(':user', $name);
+    $stmt->execute();
+
+    return (int)$stmt->fetchColumn();
+}
+```
+
+The all-users path (`$name === null`) skips the live helper entirely and goes straight to the SQL fallback — because the live helper is per-user; aggregating live values would require N HTTP calls, which we don't want on every page load. The all-users total displayed in the right-side panel will be the sum of per-user MAX(running_balance) fallback values — fine for the migration window and can be improved in a follow-up if it ever matters.
+
+- [ ] **Step 2: Run the existing test to ensure no regression**
+
+Run: `php tests/process_log_entries_test.php`
+Expected: `OK: processLogEntries v2 golden-file test passed (100 entries).`
+
+- [ ] **Step 3: Run the new test to ensure it still passes**
+
+Run: `php tests/live_balance_test.php`
+Expected: `OK: fetchLiveVaultBalance tests passed.`
+
+- [ ] **Step 4: Verify the file parses**
+
+Run: `php -l includes/utilities.php`
+Expected: `No syntax errors detected`.
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add includes/utilities.php
+git commit -m "vaultLoop: prefer live /money balance, fall back to MAX(running_balance)"
+```
+
+---
+
+### Task 4: End-to-end smoke check
+
+**Files:** No code changes. Verification only.
+
+- [ ] **Step 1: Run the test suite**
+
+Run both tests:
+
+```bash
+cd "C:\Users\ksolo\Projects\Games\Torn\Torn Vault Tracker"
+php tests/process_log_entries_test.php
+php tests/live_balance_test.php
+```
+
+Expected: both print OK.
+
+- [ ] **Step 2: Verify the live DB state**
+
+```bash
+cd "C:\Users\ksolo\Projects\Games\Torn\Torn Vault Tracker"
+php -r "
+\$db = new PDO('sqlite:data/vault.db');
+foreach(\$db->query('SELECT user, COUNT(*) AS n, MAX(running_balance) AS b FROM vault GROUP BY user') as \$r) {
+    echo \$r['user'] . ': ' . \$r['n'] . ' rows, MAX(running_balance)=\$' . number_format(\$r['b']) . PHP_EOL;
+}
+"
+```
+
+Expected: per-user row counts unchanged; `MAX(running_balance)` values may match the new live balance or not depending on API state.
+
+- [ ] **Step 3: Probe the test instance**
+
+```
+curl -s http://127.0.0.1:8001/ | grep -E 'class=\"(user|vault)\"|<h2>|<h3>'
+```
+
+Expected: the right-side "Zarathos balance:" and "Symos balance:" values should be the LIVE values from the API. They will match the `/money` endpoint's `vault` field for the appropriate API key. If the keys in `config.php` are swapped (a separate concern the user is aware of), the displayed values will be reversed from the user's actual balance — that is expected behavior pending the user's manual config edit.
+
+- [ ] **Step 4: Verify no v1 references remain**
+
+```bash
+cd "C:\Users\ksolo\Projects\Games\Torn\Torn Vault Tracker"
+grep -n "firstRun\|?key=" functions.php index.php includes/utilities.php includes/exceptions.php
+```
+
+Expected: no matches.
+
+- [ ] **Step 5: Commit if any incidental fixes were needed**
+
+If Step 1, 2, 3, or 4 produced any required fixes, commit them. Otherwise, no commit.
+
+---
+
+## Self-Review Notes
+
+**Spec coverage:**
+
+| Requirement | Task |
+|---|---|
+| Use live `/money` endpoint for displayed balances | Task 2 (helper) + Task 3 (vaultLoop) |
+| Fall back gracefully when API fails | Task 2 (try/catch) + Task 3 (fallback path) |
+| Per-request cache to avoid N HTTP calls | Task 2 (static `$cache`) |
+| Test the helper without making live API calls | Task 1 (test hook) |
+| Existing tests still pass | Tasks 2 & 3 (re-run both tests after each) |
+| End-to-end smoke check | Task 4 |
+
+**Placeholder scan:** No "TODO", "TBD", "implement later".
+
+**Type consistency:** `fetchLiveVaultBalance(string $user): ?int` returns int|null. `vaultLoop` consumes `?int` and returns int. `generateBalance`/`getSpace` wrap `vaultLoop` and continue to work.
+
+**Risks addressed:**
+- API failure → null → fallback to MAX(running_balance) → page renders. No change in failure mode.
+- Hook abuse → only affects test execution. In production, `$GLOBALS['liveBalanceTestHook']` is null (set by the one-line shim) and `executeApiCall` runs.
+- All-users path uses SQL fallback, not live; document this in the function code so future readers understand.