Skip to content

V5 6 0 - #1175

Merged
ellite merged 16 commits into
mainfrom
v5_6_0
Aug 23, 2026
Merged

V5 6 0#1175
ellite merged 16 commits into
mainfrom
v5_6_0

Conversation

@ellite

@ellite ellite commented Aug 23, 2026

Copy link
Copy Markdown
Owner

No description provided.

thorstenhornung1 and others added 16 commits August 16, 2026 11:43
The exchange-rate cron updated currency rows by code alone:

    UPDATE currencies SET rate = :rate WHERE code = :code

Every user has their own currency rows, and each rate is converted against
that user's main currency. Updating by code therefore overwrote every other
user's rates with a conversion base that is not theirs, on every scheduled
refresh. A user whose main currency is USD would silently get rates derived
from another user's EUR base, and all their converted amounts with them.

The manual refresh endpoint already scoped its writes, so the two paths
disagreed about the same table.

Single-user installations are unaffected, which is why this went unnoticed.

Also adds a small test suite, because the fix is one word and the guarantee
is what matters: a rate write that forgets the user filter now fails the
tests instead of reaching production. The harness needs no Composer and runs
in a container (dev/test.sh), matching the way Wallos vendors its libraries.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Price conversion looked the exchange rate up per converted value:

    SELECT rate FROM currencies WHERE id = :currency

Eight call sites did this, each inside a loop over subscriptions, statistics
rows or calendar entries. Rendering a list of 200 subscriptions therefore
issued 200 rate queries for data that changes once a day and is already in
memory for display.

Rates are now loaded once per connection and answered from a map. Measured
against a seeded database with 200 subscriptions: 200 queries and 41ms become
1 query and 0.3ms.

The map is keyed by the connection object through a WeakMap rather than an id,
because ids are reused once a connection is closed and a later connection
would inherit stale rates.

Two behaviours are deliberately preserved: a lookup that resolved a currency
by id alone still resolves any currency, and one that filtered by user still
only sees that user's currencies. A missing rate leaves the price untouched,
as before — and so does a rate of zero, which previously raised a division
error everywhere except the budget calculation, which already guarded it.

Verified by diffing the rendered subscriptions, statistics and calendar pages
before and after the change: byte identical.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tement

The rate refresh prepared a new statement for every currency and committed
each write on its own. Two consequences:

A refresh that stops halfway — a provider response that breaks off, a failing
write — leaves some currencies converted against the new base and some against
the old one. Rates are only comparable when they share a base, so the result
looks plausible and is wrong, which is worse than not refreshing at all.

Preparing the same statement once per currency also does the parsing work
repeatedly for a statement that never changes.

Both refresh paths now prepare the update once, reuse it across the loop, and
wrap one user's rates together with their refresh timestamp in a transaction.
A failed write rolls the user's refresh back and reports it, instead of
leaving a half-converted set behind.

The cron job keeps its per-user granularity: one user's failure does not
affect the users refreshed before or after them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every subscription query filters by user_id, usually with "inactive" and a
next_payment comparison, and the notification cron adds "notify". No index
covered any of them, so each one scanned the whole table.

Measured on a database with 10 users and 10,000 subscriptions:

  subscription list          26.2 ms -> 1.8 ms
  notification cron query     2.4 ms -> 0.8 ms
  calendar date range         3.3 ms -> 0.8 ms

Two indexes are enough. Candidates on category_id, payer_user_id and
payment_method_id were measured as well and deliberately left out: the
user_id prefix of the first index already serves those filters and the extra
indexes produced no further improvement, while every index costs write time.

That cost was measured too: 2,000 inserts take 9ms without these indexes and
17ms with them. Wallos writes subscriptions one at a time, so this is not a
path where the difference is noticeable, and the read side is on every page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Drop client_secret from the required-field gate that decides whether
the OIDC login button and callback are enabled: it's an empty string
for public clients, which the token exchange already handles fine.
Also surface misconfiguration instead of failing silently, by noting
which required fields are still empty when OIDC is enabled but not
considered configured.

Fixes #1169
Fixes #1144
The admin all-users branch built a WHERE-less query that every filter
appended " AND " onto, producing invalid SQL and a fatal error instead
of JSON. Give it a WHERE 1 = 1 base like the per-user branch, guard
prepare() failures with a clean error response, and correct the
docblock (payment vs payment_method, category as a list, the "true"
string vs integer boolean encodings, and the undocumented
all-user-subscription/users fields).

Fixes #1157
Fixes #1159
Percent-encode the username in the otpauth label so the URL passed
to qrcode.min.js is pure ASCII. The bundled encoder's UTF-8 byte
buffer leaks stale bytes across multibyte characters, which either
throws a "code length overflow" error or silently corrupts the QR
payload.

Fixes #1147
The converted rows built into $subscriptionsToReturn were never used:
the calendar was built from a second, unconverted execution of the
same statement (which also bound a placeholder the query doesn't
have). Build the calendar from $subscriptionsToReturn instead, switch
the displayed currency symbol to the main currency when a price was
converted, and fall back to "No category" / "Unknown member" /
"Unknown payment method" for subscriptions pointing at deleted rows,
matching get_subscriptions.php.

Fixes #1158
Restoring a backup swapped wallos.db into place without running the
migration chain, so a pre-v5 backup fatals on the first query touching
a column added since (e.g. logo_text_color on the home screen or
stats page). Close the stale connection, reopen against the restored
file, and run includes/run_migrations.php, matching the same pattern
already used by endpoints/db/import.php.

Fixes #1164
createdatabase.php only echoes the setup token to stdout (captured by
docker logs) on a brand-new database. registration.php generates the
same token silently when the user table is empty and the token file
doesn't exist yet, which is what actually runs on an upgraded
container whose db already existed. Log it there too, so it reaches
the channel the UI already tells Docker users to check.

Fixes #1154
saveLogo() discarded imagepng()'s return value and always reported
success, so a write failure (e.g. an unwritable images/uploads/logos
on a freshly mounted volume, before startup.sh's chown runs) still
recorded the intended filename on the subscription — a phantom file
that 404s forever, or silently fails to update on edit. Check the
write result in both saveLogo() and resizeAndUploadLogo(), in both
the web endpoint and the duplicated v2 API endpoint, and surface the
failure instead of swallowing it:

- getLogoFromUrl() now returns a specific "failed to save" message
  instead of falling through to a misleading fetch-error message.
- add.php's direct-upload branch had no error path at all; it now
  sets the same logo_warning the URL path already used.
- api/subscriptions/set_subscriptions.php gets the same logo_warning
  field, newly documented in its docblock.
- the frontend already received logo_warning on success but never
  displayed it; it now shows alongside the success toast.
- set_subscriptions.php's edit branch was overwriting the existing
  logo filename with "" on a failed replacement (and marking it
  changed), wiping a working logo on a failed edit; it now keeps the
  original logo when the new one fails to save.

Fixes #1150
…ping

Cron-driven exchange rate refresh updated currencies by code alone,
so on multi-user instances the last user processed overwrote every
other user's rates with a conversion base that wasn't theirs. Scopes
the write to the user being refreshed, matching the manual refresh
endpoint's existing behavior. Includes the new tests/ harness this PR
introduces.
… cached rate map

Price conversion resolved the exchange rate with a separate query per
row across 8 call sites. Loads each connection's rates once into
memory (WeakMap-keyed so a long-lived worker doesn't leak them across
requests) and converts from that instead. Both call-site variants are
preserved exactly: lookups scoped to a user and lookups that resolve
any currency by id (needed by the admin all-user-subscriptions view).

Reviewed by reading every changed call site and running the test
suite directly (7 new cases, all passing). Auto-merged cleanly with
our own get_subscriptions.php and get_ical_feed.php fixes.
…refresh

Both refresh paths wrote one currency rate per statement, each
committed on its own, so a failure partway through left some
currencies converted against the new base and some against the old
one. Wraps one user's writes in a transaction and reuses one prepared
statement for the loop instead of re-preparing every iteration.

Reviewed by hand-tracing every path from BEGIN to COMMIT/ROLLBACK in
both files and running the test suite (4 new cases, all passing).
Clean merge, no conflicts.
…n queries

Adds two composite indexes on subscriptions: (user_id, inactive,
next_payment) for the list/calendar/active-subscription queries, and
(user_id, notify, inactive) for the notification cron. Every Wallos
query filtering subscriptions was a full table scan.

Reviewed the migration and confirmed no numbering collision (000055,
next after our own 000054). Ran the test suite, which checks the
real query plans via EXPLAIN QUERY PLAN rather than just asserting
the SQL text — all passing. Clean merge, no conflicts.
@ellite
ellite merged commit 267f057 into main Aug 23, 2026
10 checks passed
bigSmooth7867 pushed a commit to bigSmooth7867/swarm that referenced this pull request Aug 25, 2026
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [bellamy/wallos](https://github.com/ellite/Wallos) | patch | `5.4.4` → `5.4.5` |

---

### Release Notes

<details>
<summary>ellite/Wallos (bellamy/wallos)</summary>

### [`v5.4.5`](https://github.com/ellite/Wallos/blob/HEAD/CHANGELOG.md#545-2026-08-23)

[Compare Source](ellite/Wallos@v5.4.4...v5.4.5)

##### Bug Fixes

- scope currency rate updates to the user being refreshed ([#&#8203;1175](ellite/Wallos#1175)) ([267f057](ellite/Wallos@267f057)), closes [#&#8203;1150](ellite/Wallos#1150)

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4yNi4yIiwidXBkYXRlZEluVmVyIjoiNDQuMTQuNSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicGF0Y2giLCJyZW5vdmF0ZSJdfQ==-->

Reviewed-on: https://gitea.vcasaserver.com/omar/swarm/pulls/760
Co-authored-by: Renovate Bot <renovate-bot@vcasaserver.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants