rick/LEDMatrix
1
0
Fork 0
mirror of https://github.com/ChuckBuilds/LEDMatrix.git synced 2026-10-10 09:06:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/main' into claude/scan-order-compensation

Browse Source
This commit is contained in:
Chuck
2026-09-24 16:49:41 -04:00
parent c5281d9a45 4e61d7248a
commit 7b16e953d2
62 changed files with 6170 additions and 3494 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+58
View File
@@ -19,6 +19,30 @@ accepts both, but the store flags the old spelling as deprecated
## Unreleased
- The web service (`ledmatrix-web`) logs through `src.logging_config` like the
display service, so `journalctl -p err -u ledmatrix-web` works. Successful
GET/HEAD/OPTIONS requests (the UI's polling) are logged at DEBUG instead of
INFO; 4xx at WARNING, 5xx at ERROR. `LEDMATRIX_DEBUG=true` shows them again.
`web_interface/logging_config.py` is removed. The web cache
(`web_interface/cache.py`) now honours the TTL a value was stored with and is
thread-safe.
- One plugin-directory resolver, `src/plugin_system/plugin_dirs.py`, behind
discovery, `PluginManager.get_plugin_directory`, `PluginLoader`, the store and
state reconciliation. A manifest's `id` wins over a directory merely named for
the id; hidden and `.standalone-backup-` directories are never treated as
plugins (auto-update could previously try to update a backup); ids like
`a/b` or `..` resolve to nothing everywhere. Installs where each directory is
named for its manifest id, the installer's layout, behave as before.
- `/api/v3` routes answer an exception they don't handle themselves from one
blueprint error handler, with the same `{status, message, details}` body the
53 removed per-route catch-alls returned. `ErrorCategory` and the
`error_category` key are removed from `src.web_interface.errors` (nothing read
them); `exception_error_response()` replaces the `from_exception` +
`error_response` pairs. A failing plugin action script's error now names the
real failure instead of `UnboundLocalError`.
- `FontManager.get_font()` returns a BDF font at its native size when asked for
a size the file doesn't contain (5x7.bdf at 8 or 10px, say). It used to
return PIL's default font, a different typeface, so a plugin that relied on
@@ -33,6 +57,16 @@ accepts both, but the store flags the old spelling as deprecated
its own default (a failed lookup is retried after 30 minutes). Clearing an
app's location in the web UI now actually clears it; the save used to drop
the blank field, so the old value stayed.
- `src.common.bdf_font` — `load_bdf_face(path, size)` (a cached
`freetype.Face` plus the pixel size it really renders at, falling back to
the file's native strike) and `draw_bdf_text(draw, text, x, y, face, color)`.
`DisplayManager`, `FontManager`, `element_style` and the plugin test harness
now all load and draw BDF text through it; the panel's pixels are unchanged
and BDF text draws 10-250x faster. The plugin test harness's
`calendar_font` / `bdf_5x7_font` now has the panel's 7px size set: it used
to be an unsized face, so in golden images and `check_plugin` /
`dev_server` previews its text sat 6px above where the panel draws it (off
the canvas entirely near the top) and `get_font_height()` returned 0.
- The web UI's Fonts tab has a **Used by** column: the loaded plugins that
registered each font with `FontManager.register_manager_font()`, published
@@ -81,6 +115,23 @@ floor on the release that ships them):
- `src.common.api_helper`: `USER_AGENT`, `DEFAULT_HTTP_HEADERS` (read-only).
- `src.logo_downloader`: `fetch_logo`, `save_png_atomically`,
`shared_downloader`.
- `src.common.sports_card.unshare_element_fonts` takes an optional third
argument, `element_for_font` (default: the module's `ELEMENT_FOR_FONT`, so
existing calls are unchanged).
### Sports twins
- The `SportsCoreSharedMixin` helpers that behave identically to their
`sports_card` twins (`_card_option`, `_vs_text`, `_format_game_time`,
`_coerce_rgb`, `_crisp_size`, `_unshare_element_fonts`, the colour/month/
weekday/font-grid tables) are now thin wrappers over the `sports_card`
functions, and `_format_game_date` / `_schema_font_size` share its
formatting body and schema parser. No method was removed or renamed and
nothing renders differently: `test/test_sports_twins.py` checks each pair
against the same inputs, and the old and new mixin agree on every input
there. The pairs that do differ -- favourite-result colours on nested
payloads, the weekday's timezone, the element-name map, per-mode colours --
are left as they are and pinned in that test.
### Logo downloads
@@ -120,6 +171,13 @@ floor on the release that ships them):
when the count is only known to the display service.
- The Logs tab has a **Plugin errors** panel: per-plugin counts, repeating
errors and a Clear button.
- Credential redaction in exception text (`src/redaction.py`) takes time
proportional to the text, not its square. Two patterns were quadratic: URL
`user:password@`, on a long unbroken run of letters or digits (a hex digest,
an ID), and `Authorization:` followed by a long run of whitespace. Either
used to stall every thread of the display service for up to seconds each
time the snapshot was published: about 0.5s for 20k characters of hex, 8s
for 20k spaces. What gets redacted is unchanged.
### Removed
docs/PLUGIN_DEPENDENCY_GUIDE.md
+2 -1
View File
@@ -157,5 +157,6 @@ For more, see the [Plugin Dependency Troubleshooting Guide](PLUGIN_DEPENDENCY_TR
- Store installs: `src/plugin_system/store_manager.py` (`_install_dependencies`)
- Root install helper: `src/common/permission_utils.py` (`install_requirements_file`), `scripts/fix_perms/safe_pip_install.sh`
- Load-time installs: `src/plugin_system/plugin_loader.py` (`install_dependencies`)
- Sudo rules: `scripts/install/configure_web_sudo.sh`
- Sudo rules: `scripts/install/lib_sudoers.sh` (written by `first_time_install.sh`
and `scripts/install/configure_web_sudo.sh`)
- Manual installer: `scripts/install_plugin_dependencies.sh`
docs/REST_API_REFERENCE.md
+7 -2
View File
@@ -2111,13 +2111,18 @@ Errors use one of two shapes. Most endpoints answer:
}
```
Endpoints built on the structured error helper add a code and category:
An exception no route anticipated gets this shape too, with a 500, the
message `An error occurred; see logs for details`, and `details` naming the
exception type and text (credentials redacted). The api_v3 blueprint's
error handler produces it, so it is the same for every `/api/v3` route.
Endpoints built on the structured error helper add a code, and usually
suggested fixes (the web UI's error dialog lists them):
```json
{
"status": "error",
"error_code": "CONFIG_SAVE_FAILED",
"error_category": "configuration",
"message": "Error description",
"details": "optional",
"context": { },
docs/SCROLL_PERFORMANCE.md
+70
View File
@@ -303,6 +303,76 @@ journalctl -u ledmatrix --since "-5min" --no-pager | grep -iE "px/s|px/frame"
If a plugin logs its scroll config **twice** with different modes, the second
line is what is running.
---
## A tear across the middle on fast scrolls
**Symptom:** while text scrolls, the top and bottom halves of the panel look
shifted sideways against each other along a horizontal line at mid-height, and
the shift grows with scroll speed. It shows most in Vegas mode at high speed.
**It is the panel's scan, not the software.** The measured panel, like most
64-row panels, is multiplexed 1:32 (some panels of the same size scan
differently, so check yours): it lights two rows at a time, one from each half
(row 0 with row 32, row 1 with row 33, …), stepping down both halves together
once per refresh. So row 31,
the last row of the top half, lights almost a whole refresh period after row 32
right below it. Your eye follows moving text, and moving content that lights at
different times lands in different places, so the two rows meet with an offset
of roughly
```
offset ≈ scroll speed × refresh period
```
Each frame already reaches the panel whole (`SwapOnVSync` swaps complete frames
between refreshes), so there is nothing to fix in the render path; the shift is
created inside a single refresh. Other panel heights show it too, at the point
where their two scan halves meet.
On the 2×128×64 chain above, which refreshes at about 130 Hz flat out
(7.7 ms per pass):
| scroll speed | offset at the midline |
|---|---|
| 50 px/s (Vegas default) | ~0.4 px |
| 100 px/s | ~0.8 px |
| 150 px/s | ~1.2 px, plainly visible |
### What changes it
Only a shorter scan period (a faster refresh) or a slower scroll. Measure what
the panel actually achieves first. The library prints the rate with a carriage
return and no newline, so read it from the raw journal:
```bash
# set display.hardware.show_refresh_rate to true (web UI, Display tab), restart, then:
journalctl -u ledmatrix --since "-1min" --no-pager -o cat --all | grep -a -oE "[0-9.]+Hz" | tail -5
```
Turn it off again afterwards. Measured on that panel (Pi 4, single chain),
changing one setting at a time from `pwm_bits: 7`, `gpio_slowdown: 3`:
| change | refresh, uncapped | notes |
|---|---|---|
| none | ~130 Hz | the ceiling for this wiring |
| `pwm_bits: 6` | ~138 Hz | barely faster, and half the colour depth |
| `gpio_slowdown: 2` | ~130 Hz | no faster, **and visible glitching**; keep 3 |
| `limit_refresh_rate_hz: 0` | ~130 Hz | Vegas dropped from 100 to 72–95 fps as the refresh thread took more CPU |
None of these helps much, because the time goes into shifting each row's pixels
out: a 2×128 chain pushes 256 pixels per row down one output. What does help is
**fewer pixels per output**. On a bonnet with more than one output (the
`regular` and `classic` mappings have 3; `adafruit-hat` has 1), put each panel
on its own output and set `parallel` to the number of outputs used and
`chain_length` to the panels per output, for example `parallel: 2`,
`chain_length: 1` for two panels. Each refresh then shifts half the data, which
should roughly double the refresh rate and halve the offset. That is a cable
change, so measure again afterwards.
Short of rewiring, keep fast scrolls moderate: at the default 50 px/s the
offset is under half a pixel.
## Rebuilding the binding
```bash
first_time_install.sh
+58 -113
View File
@@ -502,6 +502,41 @@ print_rgbmatrix_build_failure() {
fi
}
# Set WEB_SERVICE_USER to the account ledmatrix-web.service runs as, or "root"
# when it cannot tell. Steps 3.1 and 11 choose plugin-directory ownership from
# it. The logic was pasted three times, identically, and is kept verbatim here.
# Note: install_web_service.sh and install_service.sh no longer contain the
# "User=root" / "User=${ACTUAL_USER}" strings grepped for below (the units come
# from systemd/*.service templates with User=__USER__), so until Step 8 has
# installed the unit this yields "root".
detect_web_service_user() {
WEB_SERVICE_USER="root"
if [ -f "/etc/systemd/system/ledmatrix-web.service" ]; then
# Check actual installed service file (most accurate)
WEB_SERVICE_USER=$(grep "^User=" /etc/systemd/system/ledmatrix-web.service | cut -d'=' -f2 || echo "root")
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh" ]; then
# Check install_web_service.sh (used by first_time_install.sh)
if grep -q "User=root" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
WEB_SERVICE_USER="root"
elif grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
WEB_SERVICE_USER="$ACTUAL_USER"
fi
elif [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" ]; then
# Check template file (may have placeholder)
WEB_SERVICE_USER=$(grep "^User=" "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" | cut -d'=' -f2 || echo "root")
# If template has placeholder, check install script
if [ "$WEB_SERVICE_USER" = "__USER__" ] || [ -z "$WEB_SERVICE_USER" ]; then
# Check install_service.sh to see what user it uses
if [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
WEB_SERVICE_USER="$ACTUAL_USER"
fi
fi
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
# Web service will be installed by install_service.sh as ACTUAL_USER
WEB_SERVICE_USER="$ACTUAL_USER"
fi
}
echo ""
echo "This script will perform the following steps:"
echo "1. Check prerequisites (network, disk, memory) and install system dependencies"
@@ -699,32 +734,7 @@ else
fi
# Determine ownership based on web service user
# Check if web service file exists and what user it runs as
WEB_SERVICE_USER="root"
if [ -f "/etc/systemd/system/ledmatrix-web.service" ]; then
# Check actual installed service file (most accurate)
WEB_SERVICE_USER=$(grep "^User=" /etc/systemd/system/ledmatrix-web.service | cut -d'=' -f2 || echo "root")
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh" ]; then
# Check install_web_service.sh (used by first_time_install.sh)
if grep -q "User=root" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
WEB_SERVICE_USER="root"
elif grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
WEB_SERVICE_USER="$ACTUAL_USER"
fi
elif [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" ]; then
# Check template file (may have placeholder)
WEB_SERVICE_USER=$(grep "^User=" "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" | cut -d'=' -f2 || echo "root")
# If template has placeholder, check install script
if [ "$WEB_SERVICE_USER" = "__USER__" ] || [ -z "$WEB_SERVICE_USER" ]; then
# Check install_service.sh to see what user it uses
if [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
WEB_SERVICE_USER="$ACTUAL_USER"
fi
fi
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
# Web service will be installed by install_service.sh as ACTUAL_USER
WEB_SERVICE_USER="$ACTUAL_USER"
fi
detect_web_service_user
# If web service runs as ACTUAL_USER (not root), set ownership to ACTUAL_USER
# so the web service can change permissions. Root service can still access via group (775).
@@ -758,32 +768,7 @@ if [ ! -d "$PLUGIN_REPOS_DIR" ]; then
fi
# Determine ownership based on web service user
# Check if web service file exists and what user it runs as
WEB_SERVICE_USER="root"
if [ -f "/etc/systemd/system/ledmatrix-web.service" ]; then
# Check actual installed service file (most accurate)
WEB_SERVICE_USER=$(grep "^User=" /etc/systemd/system/ledmatrix-web.service | cut -d'=' -f2 || echo "root")
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh" ]; then
# Check install_web_service.sh (used by first_time_install.sh)
if grep -q "User=root" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
WEB_SERVICE_USER="root"
elif grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
WEB_SERVICE_USER="$ACTUAL_USER"
fi
elif [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" ]; then
# Check template file (may have placeholder)
WEB_SERVICE_USER=$(grep "^User=" "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" | cut -d'=' -f2 || echo "root")
# If template has placeholder, check install script
if [ "$WEB_SERVICE_USER" = "__USER__" ] || [ -z "$WEB_SERVICE_USER" ]; then
# Check install_service.sh to see what user it uses
if [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
WEB_SERVICE_USER="$ACTUAL_USER"
fi
fi
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
# Web service will be installed by install_service.sh as ACTUAL_USER
WEB_SERVICE_USER="$ACTUAL_USER"
fi
detect_web_service_user
# If web service runs as ACTUAL_USER (not root), set ownership to ACTUAL_USER
# so the web service can change permissions. Root service can still access via group (775).
@@ -1516,51 +1501,31 @@ POWEROFF_PATH=$(which poweroff)
BASH_PATH=$(which bash)
JOURNALCTL_PATH=$(which journalctl 2>/dev/null || true)
# Create sudoers content
cat > "$SUDOERS_TMP" << EOF
# LED Matrix Web Interface passwordless sudo configuration
# This allows the web interface user to run specific commands without a password
# Allow $ACTUAL_USER to run specific commands without a password for the LED Matrix web interface
$ACTUAL_USER ALL=(ALL) NOPASSWD: $REBOOT_PATH
$ACTUAL_USER ALL=(ALL) NOPASSWD: $POWEROFF_PATH
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix.service
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix.service
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix.service
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH enable ledmatrix.service
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH disable ledmatrix.service
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH status ledmatrix.service
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix.service
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix-web.service
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web.service
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service
$ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/safe_plugin_rm.sh *
# Install a requirements.txt as root via vetted helper, so packages are visible
# to root-run ledmatrix.service (not just the web interface's own user).
$ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/safe_pip_install.sh *
EOF
if [ -n "$JOURNALCTL_PATH" ]; then
cat >> "$SUDOERS_TMP" << EOF
# NOEXEC, because these rules end in a wildcard and journalctl starts a pager
# when its output is a terminal. From that pager (less) a "!sh" is a root
# shell -- the standard journalctl escalation. The web interface always passes
# --no-pager, so nothing here needs it, but the rule cannot require a flag that
# sits in the middle of the command line. NOEXEC stops the command executing
# another program at all, which closes the hole without depending on wildcard
# matching subtleties.
$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix.service *
$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix *
$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *
EOF
# The rules themselves live in scripts/install/lib_sudoers.sh, shared with
# scripts/install/configure_web_sudo.sh so the two cannot drift apart again.
# If it is missing (a damaged checkout), keep whatever is already installed
# rather than failing the whole install; the gate below skips the install.
SUDOERS_VALID=1
SUDOERS_LIB="$PROJECT_ROOT_DIR/scripts/install/lib_sudoers.sh"
if [ -f "$SUDOERS_LIB" ]; then
# shellcheck source=scripts/install/lib_sudoers.sh
. "$SUDOERS_LIB"
web_sudoers_rules "$ACTUAL_USER" "$PROJECT_ROOT_DIR" "$SYSTEMCTL_PATH" "$BASH_PATH" \
"$REBOOT_PATH" "$POWEROFF_PATH" "$JOURNALCTL_PATH" > "$SUDOERS_TMP"
else
SUDOERS_VALID=0
echo "⚠ $SUDOERS_LIB not found; cannot generate the sudoers rules." >&2
echo "⚠ Leaving $SUDOERS_FILE unchanged. The web interface cannot control" >&2
echo " the display service until this is fixed." >&2
fi
# Never install rules we have not parsed. A malformed drop-in in
# /etc/sudoers.d makes sudo refuse every command for every user, which on a
# headless Pi leaves no way in at all. If the rules do not parse, say so and
# keep whatever is already installed.
SUDOERS_VALID=1
if command -v visudo >/dev/null 2>&1; then
if [ "$SUDOERS_VALID" = "0" ]; then
: # nothing was generated; already reported above
elif command -v visudo >/dev/null 2>&1; then
if ! visudo -c -f "$SUDOERS_TMP" >/dev/null 2>&1; then
SUDOERS_VALID=0
echo "⚠ The generated sudoers rules did not parse:" >&2
@@ -1690,28 +1655,8 @@ fi
# Re-apply plugin directory permissions based on web service user
echo "Re-applying plugin directory permissions..."
# Determine web service user (check installed service, install scripts, or template)
WEB_SERVICE_USER="root"
if [ -f "/etc/systemd/system/ledmatrix-web.service" ]; then
# Check actual installed service file (most accurate)
WEB_SERVICE_USER=$(grep "^User=" /etc/systemd/system/ledmatrix-web.service | cut -d'=' -f2 || echo "root")
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh" ]; then
# Check install_web_service.sh (used by first_time_install.sh)
if grep -q "User=root" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
WEB_SERVICE_USER="root"
elif grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
WEB_SERVICE_USER="$ACTUAL_USER"
fi
elif [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" ]; then
WEB_SERVICE_USER=$(grep "^User=" "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" | cut -d'=' -f2 || echo "root")
if [ "$WEB_SERVICE_USER" = "__USER__" ] || [ -z "$WEB_SERVICE_USER" ]; then
if [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
WEB_SERVICE_USER="$ACTUAL_USER"
fi
fi
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
WEB_SERVICE_USER="$ACTUAL_USER"
fi
# Determine ownership based on web service user
detect_web_service_user
# Set ownership based on web service user
if [ "$WEB_SERVICE_USER" = "$ACTUAL_USER" ] || [ "$WEB_SERVICE_USER" != "root" ]; then
scripts/install/configure_web_sudo.sh
+12 -50
View File
@@ -59,6 +59,16 @@ if [ ! -f "$SAFE_PIP_INSTALL_PATH" ]; then
exit 1
fi
# The rules are shared with first_time_install.sh (Step 10) so the two cannot
# drift apart; add or remove a grant in lib_sudoers.sh, not here.
SUDOERS_LIB="$PROJECT_DIR/lib_sudoers.sh"
if [ ! -f "$SUDOERS_LIB" ]; then
echo "Error: Sudoers rules library not found: $SUDOERS_LIB" >&2
exit 1
fi
# shellcheck source=scripts/install/lib_sudoers.sh
. "$SUDOERS_LIB"
echo "Command paths:"
echo " Python: $PYTHON_PATH"
echo " Systemctl: $SYSTEMCTL_PATH"
@@ -72,56 +82,8 @@ echo " Safe pip install: $SAFE_PIP_INSTALL_PATH"
# Create a temporary sudoers file
TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$"
{
echo "# LED Matrix Web Interface passwordless sudo configuration"
echo "# This allows the web interface user to run specific commands without a password"
echo ""
echo "# Allow $WEB_USER to run specific commands without a password for the LED Matrix web interface"
# Optional: reboot/poweroff (non-critical — skip if not found)
if [ -n "$REBOOT_PATH" ]; then
echo "$WEB_USER ALL=(ALL) NOPASSWD: $REBOOT_PATH"
fi
if [ -n "$POWEROFF_PATH" ]; then
echo "$WEB_USER ALL=(ALL) NOPASSWD: $POWEROFF_PATH"
fi
# Required: systemctl
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix.service"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix.service"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix.service"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH enable ledmatrix.service"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH disable ledmatrix.service"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH status ledmatrix.service"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix.service"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix-web.service"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web.service"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service"
# Optional: journalctl (non-critical — skip if not found)
#
# NOEXEC, matching first_time_install.sh. These rules end in a wildcard and
# journalctl starts a pager, so without it the caller can reach a shell:
# less runs "!command" as the user the pager belongs to, which here is
# root. NOEXEC stops the granted command executing anything of its own.
if [ -n "$JOURNALCTL_PATH" ]; then
echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix.service *"
echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix *"
echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *"
fi
echo ""
echo "# Allow web user to remove plugin directories via vetted helper script"
echo "# The helper validates that the target path resolves inside plugin-repos/ or plugins/"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $SAFE_RM_PATH *"
echo ""
echo "# Allow web user to install a plugin's requirements.txt as root via vetted"
echo "# helper script, so packages are visible to root-run ledmatrix.service"
echo "# (not just the web interface's own user). The helper validates the target"
echo "# is requirements.txt at the project root or under plugin-repos/ or plugins/."
echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $SAFE_PIP_INSTALL_PATH *"
} > "$TEMP_SUDOERS"
web_sudoers_rules "$WEB_USER" "$PROJECT_ROOT" "$SYSTEMCTL_PATH" "$BASH_PATH" \
"$REBOOT_PATH" "$POWEROFF_PATH" "$JOURNALCTL_PATH" > "$TEMP_SUDOERS"
# Never offer to install rules we have not parsed. A malformed drop-in in
# /etc/sudoers.d makes sudo refuse every command for every user.
scripts/install/lib_sudoers.sh
Executable
+76
View File
@@ -0,0 +1,76 @@
#!/bin/bash
#
# The web interface's passwordless-sudo allow-list, /etc/sudoers.d/ledmatrix_web.
#
# Sourced by first_time_install.sh (Step 10) and
# scripts/install/configure_web_sudo.sh. Both used to carry their own copy of
# these rules, and the copies drifted: one granted safe_pip_install.sh and the
# other did not. Each caller still owns its own validate (visudo -c) / install /
# confirm flow; this file only prints the rules.
#
# Add or remove a grant here and nowhere else.
# web_sudoers_rules WEB_USER PROJECT_ROOT SYSTEMCTL_PATH BASH_PATH REBOOT_PATH POWEROFF_PATH JOURNALCTL_PATH
#
# Print the ledmatrix_web sudoers rules to stdout.
#
# SYSTEMCTL_PATH and BASH_PATH are required, and the caller must make sure they
# are not empty: `visudo -c` does not catch every such rule (with an empty
# BASH_PATH the helper rules still parse, granting the script itself).
# first_time_install.sh stops on a failed `which`; configure_web_sudo.sh checks
# them before calling this.
# REBOOT_PATH, POWEROFF_PATH and JOURNALCTL_PATH are optional: pass "" and
# their rules are left out.
web_sudoers_rules() {
local WEB_USER="${1:-}"
local PROJECT_ROOT="${2:-}"
local SYSTEMCTL_PATH="${3:-}"
local BASH_PATH="${4:-}"
local REBOOT_PATH="${5:-}"
local POWEROFF_PATH="${6:-}"
local JOURNALCTL_PATH="${7:-}"
cat << EOF
# LED Matrix Web Interface passwordless sudo configuration
# This allows the web interface user to run specific commands without a password
# Allow $WEB_USER to run specific commands without a password for the LED Matrix web interface
EOF
if [ -n "$REBOOT_PATH" ]; then
printf '%s\n' "$WEB_USER ALL=(ALL) NOPASSWD: $REBOOT_PATH"
fi
if [ -n "$POWEROFF_PATH" ]; then
printf '%s\n' "$WEB_USER ALL=(ALL) NOPASSWD: $POWEROFF_PATH"
fi
cat << EOF
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix.service
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix.service
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix.service
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH enable ledmatrix.service
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH disable ledmatrix.service
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH status ledmatrix.service
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix.service
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix-web.service
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web.service
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service
$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_plugin_rm.sh *
# Install a requirements.txt as root via vetted helper, so packages are visible
# to root-run ledmatrix.service (not just the web interface's own user).
$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh *
EOF
if [ -n "$JOURNALCTL_PATH" ]; then
cat << EOF
# NOEXEC, because these rules end in a wildcard and journalctl starts a pager
# when its output is a terminal. From that pager (less) a "!sh" is a root
# shell -- the standard journalctl escalation. The web interface always passes
# --no-pager, so nothing here needs it, but the rule cannot require a flag that
# sits in the middle of the command line. NOEXEC stops the command executing
# another program at all, which closes the hole without depending on wildcard
# matching subtleties.
$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix.service *
$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix *
$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *
EOF
fi
}
src/background_data_service.py
+2 -1
View File
@@ -26,6 +26,7 @@ from enum import Enum
from concurrent.futures import ThreadPoolExecutor
import pytz
from src.cache_manager import CacheManager
from src.common.json_body import response_json
from src.common.espn_dates import (
RANGE_RETRY_SECONDS,
_note_range_rejected,
@@ -389,7 +390,7 @@ class BackgroundDataService:
response.raise_for_status()
else:
response.raise_for_status()
data = response.json()
data = response_json(response)
# Validate data structure
if not isinstance(data, dict):
src/cache/disk_cache.py
Vendored
+43
View File
@@ -7,6 +7,7 @@ Handles persistent disk-based caching with atomic writes and error recovery.
import json
import math
import os
import re
import stat
import time
import tempfile
@@ -98,6 +99,40 @@ def _replace_nonfinite(obj: Any) -> Any:
# deleted. Both halves are covered by test/test_cache_nonfinite_floats.py.
#: Enough of a record to hold its header: ``{"timestamp":<float>,"ttl":<n>,``.
_HEAD_BYTES = 256
#: A record written with its header first (CacheManager.set does). Anything
#: else -- older files with "data" first, records from other writers -- does not
#: match and is parsed in full, as before.
_HEAD_RE = re.compile(
rb'\A\s*\{\s*"timestamp"\s*:\s*(-?[0-9][0-9.eE+-]*)\s*'
rb'(?:,\s*"ttl"\s*:\s*(-?[0-9][0-9.eE+-]*))?\s*[,}]'
)
def _stale_from_head(head: bytes, max_age: Optional[int], now: float) -> bool:
"""True when a record's header alone shows it has expired.
Mirrors the expiry rule in DiskCache.get: a per-entry ttl wins over the
caller's max_age, and no limit at all means never stale. False whenever the
header cannot be read, so the full parse decides as it always did.
"""
match = _HEAD_RE.match(head)
if not match:
return False
try:
timestamp = float(match.group(1))
limit = max_age
if match.group(2) is not None:
ttl = float(match.group(2))
if ttl >= 0:
limit = ttl
except ValueError:
return False
return limit is not None and (now - timestamp) > limit
if orjson is not None:
# Encoding the cache record dominated the background fetch worker: on a
# Pi 4, stdlib json.dumps runs ~12ms per MB and holds the GIL for all of
@@ -266,6 +301,14 @@ class DiskCache:
try:
with self._lock:
with open(cache_path, 'rb') as f:
# Decide staleness from the header before paying for the
# parse. A stale read is the common case for the biggest
# records (a season schedule is re-fetched when its cache
# expires), and parsing 53MB to throw it away held the GIL
# for ~1.8s -- a visible freeze on the panel.
if _stale_from_head(f.read(_HEAD_BYTES), max_age, time.time()):
return None
f.seek(0)
record = _loads(f.read())
# Determine record timestamp (prefer embedded, else file mtime)
src/cache_manager.py
+9 -5
View File
@@ -522,8 +522,9 @@ class CacheManager:
def update_cache(self, data_type: str, data: Dict[str, Any]) -> bool:
"""Update cache with new data."""
cache_data = {
# Header first; see DiskCache's stale check.
'timestamp': time.time(),
'data': data,
'timestamp': time.time()
}
return self.save_cache(data_type, cache_data)
@@ -556,12 +557,15 @@ class CacheManager:
from the key and is only a fallback for entries that did not
say. Omit it to keep that inferred behaviour.
"""
cache_data = {
'data': data,
'timestamp': time.time()
}
# timestamp and ttl before data, so they are the first bytes on disk:
# DiskCache.get reads them from the head of the file and can call a
# record stale without parsing it. That matters for the big ones -- a
# whole MLB season is 53MB and ~1.8s of orjson.loads with the GIL held,
# paid in full only to learn the record had expired.
cache_data: Dict[str, Any] = {'timestamp': time.time()}
if ttl is not None:
cache_data['ttl'] = ttl
cache_data['data'] = data
self.save_cache(key, cache_data)
@deprecated("3.7.0")
src/common/README.md
+9
View File
@@ -36,6 +36,15 @@ Utilities for loading and managing team logos.
Utilities for text processing and formatting.
## BDF Fonts (`bdf_font.py`)
The one way to load and draw BDF bitmap fonts. `load_bdf_face(path, size)`
returns `(face, realised_px)`, falling back to the file's native strike when
it has none at `size`; `draw_bdf_text(draw, text, x, y, face, color)` draws
top-left anchored onto a PIL `ImageDraw` exactly as the panel does.
`DisplayManager`, `FontManager`, `element_style` and the plugin test harness
all go through it.
## Scroll Helpers (`scroll_helper.py`)
Utilities for scrolling text on the display.
src/common/bdf_font.py
+261
View File
@@ -0,0 +1,261 @@
"""Loading and drawing BDF bitmap fonts: one loader, one rasterizer.
BDF fonts are fixed-size bitmap strikes. FreeType renders them at the size
baked into the file and rejects any other size, and PIL cannot draw a
``freetype.Face`` at all, so the core draws BDF text itself, glyph by glyph.
This used to be done in several places that drifted apart:
``FontManager``, ``element_style`` and ``DisplayManager`` each loaded faces
their own way, and ``DisplayManager`` and the plugin test harness
(``VisualTestDisplayManager``) each had a copy of the glyph drawing loop. The
harness renders plugin golden images and ``check_plugin`` / ``dev_server``
previews, so a copy that differs from the panel's shows something the panel
never draws. Everything now goes through the two functions here:
* :func:`load_bdf_face` -- a ``freetype.Face`` at the requested pixel size,
or at the file's native strike when the file has no strike at that size.
* :func:`draw_bdf_text` -- draw a string in a ``freetype.Face`` onto a PIL
``ImageDraw``, top-left anchored like ``ImageDraw.text``.
Only PIL and freetype-py are imported, so the module is as cheap to import
from the test harness as from core.
"""
from __future__ import annotations
import ctypes
import logging
import os
import threading
from collections import OrderedDict
from typing import Any, Optional, Sequence, Tuple
from PIL import Image
try:
import freetype
except ImportError: # pragma: no cover - freetype-py is a core requirement
freetype = None
logger = logging.getLogger(__name__)
__all__ = ["read_bdf_native_size", "load_bdf_face", "draw_bdf_text"]
# --------------------------------------------------------------------------
# Loading
# --------------------------------------------------------------------------
def read_bdf_native_size(bdf_path: str) -> Optional[int]:
"""A BDF file's one true pixel size, read from its header, or None.
Prefers the PIXEL_SIZE property, which states the real pixel height
directly; falls back to the SIZE line's point-size only if PIXEL_SIZE is
absent, since point-size only equals pixel height at exactly 100dpi --
several bundled fonts (e.g. 6x13.bdf, 5x8.bdf) are defined at 75dpi, where
the two values genuinely differ. Stops at the first STARTCHAR.
"""
size_line_value = None
try:
with open(bdf_path, "r", encoding="ascii", errors="ignore") as f:
for line in f:
if line.startswith("PIXEL_SIZE"):
parts = line.split()
if len(parts) >= 2:
return int(float(parts[1]))
elif line.startswith("SIZE") and size_line_value is None:
# Format: "SIZE <point_size> <xres> <yres>"
parts = line.split()
if len(parts) >= 2:
size_line_value = int(float(parts[1]))
elif line.startswith("STARTCHAR"):
break
except (OSError, ValueError):
return None
return size_line_value
#: Loaded faces, keyed on (absolute path, requested size, mtime_ns, file size)
#: so a font file replaced on disk under the same name is loaded afresh.
#: Bounded LRU: the display process runs for weeks and every config save can
#: introduce a new (font, size) pair, but a panel draws from a handful.
_FACE_CACHE_MAX = 256
_face_cache: "OrderedDict[tuple, Tuple[Any, int]]" = OrderedDict()
_face_cache_lock = threading.Lock()
def _face_at(path: str, size_px: int) -> Any:
face = freetype.Face(path)
# Character size in 1/64th points at 72dpi == pixel size.
face.set_char_size(size_px * 64, size_px * 64, 72, 72)
return face
def load_bdf_face(path: str, size_px: int) -> Tuple[Any, int]:
"""``(face, realised_px)`` for the BDF file at ``path``.
``realised_px`` is ``size_px`` when the file has a strike at that size,
otherwise the file's native size: FreeType refuses any other size for a
bitmap font, and answering that with some other typeface (which both
``FontManager`` and ``element_style`` once did) is worse than drawing the
font that was asked for at the size it can do. Callers that lay out by
size need ``realised_px``, not the size they asked for.
Faces are cached per thread. A ``freetype.Face`` holds per-glyph state
(``load_char`` rewrites its glyph slot), and FreeType does not allow two
threads to use one face at once, so the display thread and a plugin's
update thread must never be handed the same object. Within a thread the
face is shared by every caller. Raises if the file can't be loaded at
either size.
"""
if freetype is None:
raise RuntimeError("freetype-py is not installed; BDF fonts need it")
size_px = int(size_px)
abs_path = os.path.abspath(path)
try:
st = os.stat(abs_path)
key = (threading.get_ident(), abs_path, size_px,
st.st_mtime_ns, st.st_size)
except OSError:
key = None # let freetype raise its own error below
if key is not None:
with _face_cache_lock:
cached = _face_cache.get(key)
if cached is not None:
_face_cache.move_to_end(key)
return cached
try:
entry = (_face_at(abs_path, size_px), size_px)
except Exception:
native = read_bdf_native_size(abs_path)
if not native or native == size_px:
raise
# A fresh Face: the first one already took a failed set_char_size.
entry = (_face_at(abs_path, native), native)
logger.debug(
"BDF font %s requested at %spx renders at its native %spx "
"(the file has no strike at the requested size)",
abs_path, size_px, native,
)
if key is not None:
with _face_cache_lock:
_face_cache[key] = entry
_face_cache.move_to_end(key)
while len(_face_cache) > _FACE_CACHE_MAX:
_face_cache.popitem(last=False)
return entry
def clear_face_cache() -> None:
"""Drop every cached face (tests; a font directory swapped wholesale)."""
with _face_cache_lock:
_face_cache.clear()
# --------------------------------------------------------------------------
# Drawing
# --------------------------------------------------------------------------
def _bitmap_bytes(bitmap: Any, nbytes: int) -> bytes:
"""The first ``nbytes`` of a glyph bitmap's buffer, zero-padded.
``bitmap.buffer`` builds a Python list one byte at a time; reading the
underlying FT_Bitmap directly is the same bytes without that cost.
"""
raw = getattr(bitmap, "_FT_Bitmap", None)
if raw is not None and raw.buffer:
return ctypes.string_at(raw.buffer, nbytes)
buf = bytes(bitmap.buffer[:nbytes])
if len(buf) < nbytes:
buf += bytes(nbytes - len(buf))
return buf
def _glyph_points(bitmap: Any, left: int, top: int,
clip_w: int, clip_h: int) -> list:
"""Every lit pixel of a glyph, clipped, as ``(x, y)`` pairs.
The reference definition of which pixels a glyph lights: the MSB-first
bit ``j`` of byte ``i * pitch + j // 8``. Used only where the fast path
below can't express exactly the same thing.
"""
buffer = bitmap.buffer
pitch = bitmap.pitch
points = []
for i in range(bitmap.rows):
for j in range(bitmap.width):
byte_index = i * pitch + (j // 8)
if byte_index < len(buffer) and buffer[byte_index] & (1 << (7 - (j % 8))):
px = left + j
py = top + i
if 0 <= px < clip_w and 0 <= py < clip_h:
points.append((px, py))
return points
def draw_bdf_text(draw: Any, text: str, x: int, y: int, face: Any,
color: Any = (255, 255, 255),
clip: Optional[Sequence[int]] = None) -> int:
"""Draw ``text`` in a ``freetype.Face`` with ``draw``; return the pen x.
``(x, y)`` is the top-left of the line, as for ``ImageDraw.text``: the
baseline is ``y`` plus the face's ascender. Each glyph's lit bits are set
to ``color`` exactly -- no blending, no anti-aliasing -- and pixels
outside ``[0, clip_w) x [0, clip_h)`` are skipped (``clip`` defaults to
the image size). The pen advances by each glyph's advance width.
Glyphs are drawn as 1-bit masks with ``ImageDraw.bitmap`` rather than a
point at a time, which is pixel-identical and far faster. A ``draw`` that
blends (``ImageDraw.Draw(rgb_image, "RGBA")``) is drawn point by point, so
a translucent colour still blends exactly as it always has.
Errors (a non-BDF ``face``, a bad colour) propagate after any glyphs
before the failing one are drawn; callers decide whether to log them.
"""
try:
ascender_px = face.size.ascender >> 6
except Exception:
ascender_px = 0
baseline_y = y + ascender_px
if clip is None:
clip_w, clip_h = draw.im.size
else:
clip_w, clip_h = int(clip[0]), int(clip[1])
blending = draw.mode != draw.im.mode
for char in text:
face.load_char(char)
glyph = face.glyph
bitmap = glyph.bitmap
rows, width, pitch = bitmap.rows, bitmap.width, bitmap.pitch
left = x + glyph.bitmap_left
top = baseline_y - glyph.bitmap_top
if rows > 0 and width > 0:
if blending or pitch <= 0:
points = _glyph_points(bitmap, left, top, clip_w, clip_h)
if points:
draw.point(points, fill=color)
else:
# The visible part of the glyph box, in glyph coordinates.
x0, y0 = max(0, -left), max(0, -top)
x1, y1 = min(width, clip_w - left), min(rows, clip_h - top)
if x0 < x1 and y0 < y1:
# Raw mode "1" with stride=pitch reads exactly the bits
# _glyph_points does, whatever the glyph's pixel mode.
mask = Image.frombytes(
"1", (width, rows), _bitmap_bytes(bitmap, rows * pitch),
"raw", "1", pitch)
if (x0, y0, x1, y1) != (0, 0, width, rows):
mask = mask.crop((x0, y0, x1, y1))
# An all-blank glyph draws nothing -- and, as before,
# never touches the colour.
if mask.getbbox() is not None:
draw.bitmap((left + x0, top + y0), mask, fill=color)
x += glyph.advance.x >> 6
return x
src/common/espn_dates.py
+10 -2
View File
@@ -39,6 +39,14 @@ from datetime import date, timedelta
from functools import partial
from typing import Any, Dict, List, Optional, Tuple
try:
from src.common.json_body import response_json
except ImportError:
# Plugins bundle copies of this module for older cores, which predate
# json_body; the stdlib parse is what those cores always used.
def response_json(response: Any) -> Any:
return response.json()
# Above this, ESPN returns a truncated list instead of an error. See module
# docstring: 500 is the largest value measured to return complete data.
ESPN_MAX_LIMIT = 500
@@ -194,7 +202,7 @@ def _fetch_one_chunk(
timeout=timeout,
)
response.raise_for_status()
return response.json()
return response_json(response)
except Exception as exc: # noqa: BLE001 - see docstring
if logger:
logger.warning("ESPN chunk %s failed, skipping it: %s", chunk, exc)
@@ -371,4 +379,4 @@ def fetch_espn_scoreboard(
if data is not None:
return data
response.raise_for_status()
return response.json()
return response_json(response)
src/common/json_body.py
+29
View File
@@ -0,0 +1,29 @@
"""Parse an HTTP response body as JSON, with orjson when it is installed.
``requests``' ``response.json()`` uses the stdlib parser. For the payloads the
sports plugins fetch -- a season schedule is tens of MB -- that runs ~1.7x
slower than orjson on a Pi 4 (3.1s against 1.8s for the 53MB MLB season), and
both hold the GIL for the whole parse, which freezes the display for as long.
Nothing else changes: the result is the same Python objects.
"""
from __future__ import annotations
from typing import Any
try:
import orjson
except ImportError: # optional dependency; see docs/SCROLL_PERFORMANCE.md
orjson = None
def response_json(response: Any) -> Any:
"""``response.json()``, parsed by orjson when available."""
body = getattr(response, "content", None)
if orjson is None or not isinstance(body, (bytes, bytearray)):
return response.json()
try:
return orjson.loads(body)
except orjson.JSONDecodeError:
# Let requests raise its usual error, with its usual message.
return response.json()
src/common/sports_card.py
+49 -16
View File
@@ -339,6 +339,18 @@ def format_game_date(config: Optional[Dict[str, Any]], logger, date_text: str,
if not raw:
return ""
fmt = str(scroll_card_option(config, "date_format", "abbrev") or "abbrev")
return _format_date_as(fmt, raw, lambda: weekday_for(config, logger, game))
def _format_date_as(fmt: str, raw: str, weekday, months=MONTH_ABBR) -> str:
"""Render a stripped, non-empty "M/D" *raw* in style *fmt*.
The body both date formatters share. They differ in which setting names the
style and in which zone the weekday is taken from (see
``SportsCoreSharedMixin._format_game_date``), so those arrive as arguments:
*weekday* is a zero-argument callable, only called for the "weekday" style.
*months* lets the mixin keep reading its (overridable) ``_MONTH_ABBR``.
"""
if fmt == "numeric":
return raw
parts = raw.replace("-", "/").split("/")
@@ -347,14 +359,14 @@ def format_game_date(config: Optional[Dict[str, Any]], logger, date_text: str,
month, day = int(parts[0]), int(parts[1])
if not 1 <= month <= 12:
return raw
name = MONTH_ABBR[month - 1]
name = months[month - 1]
if fmt == "numeric_day_first":
return f"{day}/{month}"
if fmt == "day_first":
return f"{day} {name}"
if fmt == "weekday":
weekday = weekday_for(config, logger, game)
return f"{weekday} {name} {day}" if weekday else f"{name} {day}"
day_name = weekday()
return f"{day_name} {name} {day}" if day_name else f"{name} {day}"
return f"{name} {day}"
@@ -388,6 +400,29 @@ def format_game_time(config: Optional[Dict[str, Any]], time_text: str) -> str:
_SCHEMA_FONT_SIZE_CACHE: Dict[str, Dict[str, int]] = {}
def _read_schema_font_sizes(schema_path: str) -> Dict[str, int]:
"""``{element: font_size default}`` from a config_schema.json. Raises.
The parse both schema-default lookups share. Each keeps its own cache --
this function per schema path, ``SportsCoreSharedMixin._schema_font_size``
per class -- because the lifetimes differ: a class is rebuilt when the
display service reloads a plugin, a module-level path cache is not. One
cache would change when a reloaded plugin sees an edited schema.
"""
import json
with open(schema_path) as fh:
schema = json.load(fh)
props = (schema.get('properties', {})
.get('customization', {})
.get('properties', {}))
sizes: Dict[str, int] = {}
for key, spec in props.items():
size = spec.get('properties', {}).get('font_size', {}).get('default')
if size is not None:
sizes[key] = int(size)
return sizes
def schema_font_size(schema_path: str, element_key) -> Optional[int]:
"""The font_size this plugin's config_schema.json declares, or None.
@@ -399,18 +434,8 @@ def schema_font_size(schema_path: str, element_key) -> Optional[int]:
return None
cache = _SCHEMA_FONT_SIZE_CACHE.get(schema_path)
if cache is None:
cache = {}
try:
import json
with open(schema_path) as fh:
schema = json.load(fh)
props = (schema.get('properties', {})
.get('customization', {})
.get('properties', {}))
for key, spec in props.items():
size = spec.get('properties', {}).get('font_size', {}).get('default')
if size is not None:
cache[key] = int(size)
cache = _read_schema_font_sizes(schema_path)
except Exception as exc:
# See sports_shared._schema_font_size: an unreadable schema
# silently disables the pixel-grid snap for every element.
@@ -444,7 +469,7 @@ def resolve_font_size(schema_path: str, element_config, element_key,
return crisp_size(font_name, default_size, aliases, grid_table)
def unshare_element_fonts(logger, fonts):
def unshare_element_fonts(logger, fonts, element_for_font=None):
"""Give each colourable element its own face object.
The colour a draw gets is resolved from the face it was handed, and
@@ -459,13 +484,21 @@ def unshare_element_fonts(logger, fonts):
the ability to tell two elements apart does. Faces that cannot be
rebuilt (a BDF loaded through freetype.Face, anything without a usable
path) are left shared, and their draws stay white as before.
*element_for_font* names the font keys to consider, in order (the first
holder of a face keeps it); it defaults to this module's
:data:`ELEMENT_FOR_FONT`. ``SportsCoreSharedMixin`` passes its own map,
which names different keys -- see ``resolve_font_color`` for why the two
vocabularies are kept apart.
"""
try:
from src.common.font_layout import load_truetype as _load
except ImportError: # pragma: no cover
return fonts
if element_for_font is None:
element_for_font = ELEMENT_FOR_FONT
seen = {}
for key in ELEMENT_FOR_FONT:
for key in element_for_font:
font = fonts.get(key)
if font is None:
continue
src/common/sports_shared.py
+60 -105
View File
@@ -64,14 +64,27 @@ live here. Only ``_SCORE_PROBE_TEXT`` varies -- afl and basketball reach three d
a side and override it, the same two that override ``_SCORE_PROBE`` on
``SportsGameRendererMixin``.
DELIBERATELY NOT MERGED WITH sports_card
----------------------------------------
Fourteen of these have same-named twins in ``src/common/sports_card.py``, which
the scoreboards' ``game_renderer.py`` already uses. They are NOT wired together
here. Only five are provably equivalent by source comparison; the other nine
differ in ways inspection cannot settle, and a wrong guess silently changes what
every scoreboard draws. Merging them needs differential testing against both
implementations, and is left for its own change.
TWINS IN sports_card
--------------------
Many of these have same-named twins in ``src/common/sports_card.py``, which the
scoreboards' ``game_renderer.py`` uses. ``test/test_sports_twins.py`` calls
each pair with the same inputs (the plugins' fixture games in every payload
shape, plus edge cases) and splits them in two:
- Identical: ``_card_option``, ``_vs_text``, ``_format_game_time``,
``_coerce_rgb``, ``_crisp_size``, ``_unshare_element_fonts`` (given the same
element map) and the constant tables. These are now thin wrappers over the
``sports_card`` function; ``_format_game_date`` and ``_schema_font_size``
share its body/parser while keeping their own setting, zone and cache.
``_resolve_font_size`` agrees too but keeps its body, because it dispatches
through the overridable ``_schema_font_size``/``_crisp_size``.
- Different, and pinned as they are: ``_side_is_favorite`` /
``_favorite_result`` / ``_recent_score_color`` (flat keys and the host's
favourites only), ``_weekday_for`` (the plugin's resolved zone, not
``config["timezone"]``), ``_font_color`` / ``_ELEMENT_FOR_FONT`` (another
element vocabulary), ``_element_color`` (passes ``SKIN_MODE``). Each shows
up in one display mode only, so which side is right is a product decision;
the test that pins it names the difference.
"""
from __future__ import annotations
@@ -87,6 +100,7 @@ import pytz
from src.common.espn_dates import fetch_espn_scoreboard
import requests
from PIL import Image, ImageDraw, ImageFont
from src.common import sports_card as _card
from src.common.font_layout import load_truetype
logger = logging.getLogger(__name__)
@@ -171,19 +185,17 @@ class SportsCoreSharedMixin:
_ELEMENT_FOR_FONT: ClassVar[Dict[str, str]] = {
"score": "score_text", "time": "period_text", "team": "team_text",
"detail": "detail_text", "status": "status_text"}
# The tables below are sports_card's (and font_layout's) values. The dicts
# are copies, so a caller that mutates one module's table -- or a subclass
# that replaces it -- does not reach into the other.
#: Default tint for a favourite team's finished game.
FAVORITE_RESULT_COLOR_DEFAULTS: ClassVar[Dict[str, Tuple[int, int, int]]] = {
"win": (0, 255, 0), "loss": (255, 0, 0), "tie": (255, 200, 0)}
_MONTH_ABBR: ClassVar[Tuple[str, ...]] = (
"Jan", "Feb", "Mar", "Apr", "May", "Jun",
"Jul", "Aug", "Sep", "Oct", "Nov", "Dec")
_WEEKDAY_ABBR: ClassVar[Tuple[str, ...]] = (
"Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun")
FAVORITE_RESULT_COLOR_DEFAULTS: ClassVar[Dict[str, Tuple[int, int, int]]] = dict(
_card.FAVORITE_RESULT_COLOR_DEFAULTS)
_MONTH_ABBR: ClassVar[Tuple[str, ...]] = _card.MONTH_ABBR
_WEEKDAY_ABBR: ClassVar[Tuple[str, ...]] = _card.WEEKDAY_ABBR
#: Bitmap fonts snap to their native pixel grid.
_FONT_PIXEL_GRID: ClassVar[Dict[str, int]] = {
"PressStart2P-Regular.ttf": 8, "4x6-font.ttf": 7}
_FONT_NAME_ALIASES: ClassVar[Dict[str, str]] = {
"press_start": "PressStart2P-Regular.ttf", "four_by_six": "4x6-font.ttf"}
_FONT_PIXEL_GRID: ClassVar[Dict[str, int]] = dict(_card.FONT_PIXEL_GRID)
_FONT_NAME_ALIASES: ClassVar[Dict[str, str]] = dict(_card.FONT_NAME_ALIASES)
#: Accepted values for the other-games quality filter.
_QUALITY_CHOICES: ClassVar[frozenset] = frozenset({"any", "ranked"})
#: How long to stay quiet between ranking-coverage warnings.
@@ -213,13 +225,11 @@ class SportsCoreSharedMixin:
"""Snap *desired* to the nearest size *font_file* renders crisply at.
A face with no known grid is returned unchanged, so a user-supplied
font is never second-guessed.
font is never second-guessed. The class's own tables are passed, so a
host that declares extra faces keeps them.
"""
font_file = cls._FONT_NAME_ALIASES.get(font_file, font_file)
grid = cls._FONT_PIXEL_GRID.get(font_file)
if not grid or not desired or desired <= 0:
return desired
return max(grid, int(round(float(desired) / grid)) * grid)
return _card.crisp_size(font_file, desired,
cls._FONT_NAME_ALIASES, cls._FONT_PIXEL_GRID)
#: Absolute path of this plugin's directory, declared by the plugin
#: itself. The mixin cannot work it out -- see _plugin_dir.
@@ -281,23 +291,19 @@ class SportsCoreSharedMixin:
"""The font_size this plugin's config_schema.json declares, or None."""
if not element_key:
return None
# Cached per class, not in sports_card's per-path cache: the display
# service rebuilds the class when it reloads a plugin, and that is
# what makes an edited schema take effect. Both caches parse through
# sports_card._read_schema_font_sizes.
cache = getattr(self.__class__, '_SCHEMA_FONT_SIZES', None)
if cache is None:
cache = {}
try:
import json
directory = self._plugin_dir()
if directory is None:
raise FileNotFoundError("no config_schema.json on the MRO")
with open(os.path.join(directory, 'config_schema.json')) as fh:
schema = json.load(fh)
props = (schema.get('properties', {})
.get('customization', {})
.get('properties', {}))
for key, spec in props.items():
size = spec.get('properties', {}).get('font_size', {}).get('default')
if size is not None:
cache[key] = int(size)
cache = _card._read_schema_font_sizes(
os.path.join(directory, 'config_schema.json'))
except Exception as exc:
# Say so. An unreadable schema is not cosmetic: every element's
# configured size then stops matching "the schema default", is
@@ -339,10 +345,7 @@ class SportsCoreSharedMixin:
def _card_option(self, key: str, default: Any = None) -> Any:
"""Read one key from the scroll_card config block."""
block = (self.config or {}).get("scroll_card")
if isinstance(block, dict) and block.get(key) is not None:
return block.get(key)
return default
return _card.scroll_card_option(self.config, key, default)
def _switch_upcoming_center(self) -> str:
"""Middle of the full-screen upcoming scorebug: 'vs', 'date_time' or 'none'."""
@@ -354,7 +357,7 @@ class SportsCoreSharedMixin:
def _vs_text(self) -> str:
"""Separator drawn between the teams -- "VS", "@", "at", anything."""
return str(self._card_option("vs_text", "VS"))
return _card.vs_text(self.config)
def _switch_date_format(self) -> str:
"""Date style for the full-screen scorebug.
@@ -374,28 +377,19 @@ class SportsCoreSharedMixin:
return fmt
def _format_game_date(self, date_text: str, game: Optional[Dict] = None) -> str:
"""Format an upcoming date per scroll_card.switch_date_format."""
"""Format an upcoming date per scroll_card.switch_date_format.
The formatting is sports_card's. What differs from the card's
``format_game_date`` is passed in: the setting (``switch_date_format``,
see :meth:`_switch_date_format`) and the weekday, which comes from
:meth:`_weekday_for` and so from this plugin's resolved timezone.
"""
raw = str(date_text or "").strip()
if not raw:
return raw
fmt = self._switch_date_format()
if fmt == "numeric":
return raw
parts = raw.replace("-", "/").split("/")
if not (len(parts) >= 2 and parts[0].strip().isdigit() and parts[1].strip().isdigit()):
return raw
month, day = int(parts[0]), int(parts[1])
if not 1 <= month <= 12:
return raw
name = self._MONTH_ABBR[month - 1]
if fmt == "numeric_day_first":
return f"{day}/{month}"
if fmt == "day_first":
return f"{day} {name}"
if fmt == "weekday":
weekday = self._weekday_for(game)
return f"{weekday} {name} {day}" if weekday else f"{name} {day}"
return f"{name} {day}"
return _card._format_date_as(self._switch_date_format(), raw,
lambda: self._weekday_for(game),
self._MONTH_ABBR)
def _weekday_for(self, game: Optional[Dict]) -> str:
"""Weekday abbreviation from the game's start time, or ''."""
@@ -413,22 +407,7 @@ class SportsCoreSharedMixin:
def _format_game_time(self, time_text: str) -> str:
"""Return the time as-is (12h) or converted to 24h."""
raw = str(time_text or "").strip()
if not raw or str(self._card_option("time_format", "12h")) != "24h":
return raw
cleaned = raw.upper().replace(" ", "")
meridiem = "AM" if cleaned.endswith("AM") else "PM" if cleaned.endswith("PM") else ""
if not meridiem:
return raw
try:
hh, _, mm = cleaned[:-2].partition(":")
hour, minute = int(hh), int(mm or 0)
except ValueError:
return raw
if not (0 <= hour <= 12 and 0 <= minute <= 59):
return raw
hour = hour % 12 + (12 if meridiem == "PM" else 0)
return f"{hour:02d}:{minute:02d}"
return _card.format_game_time(self.config, time_text)
def _scorebug_font(self, draw, text: str, width: int):
"""The face this scorebug draws its date and time in.
@@ -560,15 +539,7 @@ class SportsCoreSharedMixin:
@staticmethod
def _coerce_rgb(value, fallback):
"""Turn a configured [R, G, B] list into a clamped (r, g, b) tuple."""
# Checked before unpacking: a 3-character string ("123") would otherwise
# iterate into three digits and yield a colour rather than the fallback.
if not isinstance(value, (list, tuple)) or len(value) != 3:
return fallback
try:
r, g, b = (max(0, min(255, int(channel))) for channel in value)
except (TypeError, ValueError):
return fallback
return (r, g, b)
return _card.coerce_rgb(value, fallback)
@staticmethod
def _side_is_favorite(game: Dict, side: str, favorites: set) -> bool:
@@ -849,28 +820,12 @@ class SportsCoreSharedMixin:
the ability to tell two elements apart does. Faces that cannot be
rebuilt (a BDF loaded through freetype.Face, anything without a usable
path) are left shared, and their draws stay white as before.
The body is sports_card's; this class's own element map is passed, so
the keys considered are the ones this class colours by.
"""
try:
from src.common.font_layout import load_truetype as _load
except ImportError: # pragma: no cover
return fonts
seen = {}
for key in self._ELEMENT_FOR_FONT:
font = fonts.get(key)
if font is None:
continue
if id(font) not in seen:
seen[id(font)] = key
continue
path, size = getattr(font, "path", None), getattr(font, "size", None)
if not path or not size:
continue
try:
fonts[key] = _load(path, size)
except (OSError, ValueError, TypeError):
self.logger.debug(
"Could not un-share the %s face; it keeps the default colour", key)
return fonts
return _card.unshare_element_fonts(self.logger, fonts,
self._ELEMENT_FOR_FONT)
def _font_color(self, font, default: Tuple[int, int, int] = (255, 255, 255)):
"""Colour for whichever element owns this face.
src/display_manager.py
+16 -48
View File
@@ -34,6 +34,7 @@ else:
from contextlib import contextmanager
from pathlib import Path
from PIL import Image, ImageDraw, ImageFont
from src.common.bdf_font import draw_bdf_text, load_bdf_face
from src.common.font_layout import crisp_size, load_truetype, resolve_asset_path
from src import scan_order
from src.display_geometry import (
@@ -928,43 +929,16 @@ class DisplayManager:
logger.error(f"Error clearing display: {e}")
def _draw_bdf_text(self, text, x, y, color=(255, 255, 255), font=None):
"""Draw text using BDF font with proper bitmap handling."""
"""Draw text in a BDF ``freetype.Face`` with (x, y) as its top-left.
Delegates to :func:`src.common.bdf_font.draw_bdf_text`, which the
plugin test harness uses too, so previews and golden images show the
pixels the panel does. Clipped to the logical display size.
"""
try:
# Use the passed font or fall back to calendar_font
face = font if font else self.calendar_font
# Compute baseline from font ascender so caller can pass top-left y
try:
ascender_px = face.size.ascender >> 6
except Exception:
ascender_px = 0
baseline_y = y + ascender_px
for char in text:
face.load_char(char)
bitmap = face.glyph.bitmap
# Get glyph metrics
glyph_left = face.glyph.bitmap_left
glyph_top = face.glyph.bitmap_top
# Draw the character
for i in range(bitmap.rows):
for j in range(bitmap.width):
byte_index = i * bitmap.pitch + (j // 8)
if byte_index < len(bitmap.buffer):
byte = bitmap.buffer[byte_index]
if byte & (1 << (7 - (j % 8))):
# Calculate actual pixel position
pixel_x = x + glyph_left + j
pixel_y = baseline_y - glyph_top + i
# Only draw if within bounds
if (0 <= pixel_x < self.width and 0 <= pixel_y < self.height):
self.draw.point((pixel_x, pixel_y), fill=color)
# Move to next character
x += face.glyph.advance.x >> 6
draw_bdf_text(self.draw, text, x, y, face, color,
clip=(self.width, self.height))
except Exception as e:
logger.error(f"Error drawing BDF text: {e}", exc_info=True)
@@ -1013,19 +987,13 @@ class DisplayManager:
if not os.path.exists(self.calendar_font_path):
raise FileNotFoundError(f"Font file not found at {self.calendar_font_path}")
# Load with freetype for proper BDF handling
face = freetype.Face(self.calendar_font_path)
# A freshly constructed Face has no active size, so
# face.size.height is 0 until set_char_size is called -- and
# get_font_height() reads exactly that. Without this, every
# caller measuring the 5x7 face got 0 and stacked rows on top
# of one another; the "Calendar font size: 0 pixels" line
# below has been printing the symptom on every start-up.
# font_manager._load_bdf_font already does this; the two paths
# disagreed about whether a Face was usable for measurement.
# 5x7.bdf is a fixed strike, so FreeType renders 7px whatever
# is asked for -- this sets the metrics, not the raster.
face.set_char_size(_CALENDAR_FONT_PX * 64, _CALENDAR_FONT_PX * 64, 72, 72)
# load_bdf_face sets the size: a Face built without
# set_char_size reports face.size.height 0, and every caller
# measuring the 5x7 face with get_font_height() got 0 and
# stacked rows on top of one another. 5x7.bdf is a fixed
# strike, so FreeType renders 7px whatever is asked for --
# the size sets the metrics, not the raster.
face, _ = load_bdf_face(self.calendar_font_path, _CALENDAR_FONT_PX)
logger.info(f"5x7 calendar font loaded successfully from {self.calendar_font_path}")
logger.info(f"Calendar font size: {face.size.height >> 6} pixels")
src/element_style.py
+12 -47
View File
@@ -53,13 +53,9 @@ from dataclasses import dataclass
from typing import Any, Dict, Optional, Tuple, Union
from PIL import ImageFont
from src.common.bdf_font import load_bdf_face, read_bdf_native_size
from src.common.font_layout import load_truetype
try:
import freetype
except ImportError: # pragma: no cover - freetype ships with the core
freetype = None
logger = logging.getLogger(__name__)
# Core install root (the directory that contains src/ and assets/fonts/),
@@ -164,56 +160,25 @@ def native_bdf_size(font_name: str) -> Optional[int]:
def _read_bdf_native_size(path: str) -> Optional[int]:
"""A BDF file's own pixel size, delegated to FontManager.
"""A BDF file's own pixel size (the web UI's fonts API imports this name).
Deliberately not reimplemented: FontManager's reader prefers PIXEL_SIZE
over the SIZE line's point-size (they differ on the several bundled
fonts defined at 75dpi) and stops at the first STARTCHAR. Core always
ships it; the guard is for the plugin test harnesses that stub the
module out.
See :func:`src.common.bdf_font.read_bdf_native_size`: it prefers
PIXEL_SIZE over the SIZE line's point-size, which differ on the several
bundled fonts defined at 75dpi.
"""
try:
from src.font_manager import FontManager
return FontManager._read_bdf_native_size(path)
except Exception: # pragma: no cover - defensive
return None
return read_bdf_native_size(path)
def _load_bdf(path: str, size: int) -> Tuple[Any, int]:
"""A ``freetype.Face`` for a BDF file at the closest size it can do.
BDF fonts are fixed-size bitmap strikes, not scalable outlines:
FreeType accepts only the exact pixel size baked into the file and
raises for anything else. 32 of the 35 shipped fonts are BDF, so a
size the user picked in the web UI usually is not a valid strike.
Retrying at the file's native size is the behaviour SportsCore already
has (``_load_custom_font_from_element_config``). Without it this
function fell through to the generic except below and returned
*PressStart2P* — so choosing 5x7.bdf at size 10 silently rendered a
completely different typeface rather than 5x7 at 7px.
BDF fonts are fixed-size bitmap strikes: FreeType accepts only the pixel
size baked into the file, and 32 of the 35 shipped fonts are BDF, so a
size the user picked in the web UI usually is not a valid strike. The
shared loader retries at the native size; without that, 5x7.bdf at size
10 used to fall through to *PressStart2P*, a different typeface.
"""
if freetype is None:
raise RuntimeError("freetype not available for BDF fonts")
def _face_at(px: int) -> Any:
face = freetype.Face(path)
# Character size in 1/64th points at 72dpi == pixel size.
face.set_char_size(px * 64, px * 64, 72, 72)
return face
try:
return _face_at(size), size
except Exception:
native = _read_bdf_native_size(path)
if not native or native == size:
raise
# A fresh Face: the first one already took a failed set_char_size.
face = _face_at(native)
logger.debug("BDF font %s loaded at its native size %s "
"(requested %s is not a strike in this file)",
path, native, size)
return face, native
return load_bdf_face(path, size)
def load_font(font_name: str, size: int) -> Any:
src/font_manager.py
+11 -46
View File
@@ -38,6 +38,7 @@ import time
from collections import OrderedDict
from pathlib import Path
from PIL import ImageFont
from src.common.bdf_font import load_bdf_face, read_bdf_native_size
from src.common.font_layout import load_truetype, resolve_asset_path
from typing import Dict, Tuple, Optional, Union, Any, List
from src.deprecation import deprecated
@@ -517,29 +518,14 @@ class FontManager:
return font
def _load_bdf_font(self, font_path: str, size_px: int) -> freetype.Face:
"""Load a BDF font using FreeType."""
"""Load a BDF font through the shared loader.
A size the file has no strike for comes back at the native strike
rather than failing over to PIL's default font, a different typeface
(see :func:`src.common.bdf_font.load_bdf_face`).
"""
try:
native_size = self._read_bdf_native_size(font_path)
if native_size is not None and native_size != size_px:
# BDF is a fixed-strike bitmap format: FreeType renders the
# native size no matter what set_char_size asks for.
logger.debug(
"BDF font %s requested at %spx but renders at its native "
"%spx", font_path, size_px, native_size
)
face = freetype.Face(font_path)
try:
# Character size in 1/64th points at 72dpi == pixel size.
face.set_char_size(size_px * 64, size_px * 64, 72, 72)
except freetype.FT_Exception:
# FreeType rejects any size but the strike's own, and get_font
# used to answer that with PIL's default font -- a different
# typeface. Use the native strike, as element_style does.
if native_size is None or native_size == size_px:
raise
face = freetype.Face(font_path)
face.set_char_size(native_size * 64, native_size * 64, 72, 72)
return face
return load_bdf_face(font_path, size_px)[0]
except Exception as e:
logger.error(f"Error loading BDF font {font_path}: {e}")
raise
@@ -554,30 +540,9 @@ class FontManager:
@staticmethod
def _read_bdf_native_size(bdf_path: str) -> Optional[int]:
"""Read a BDF file's own header to find its one true pixel size.
Prefers the PIXEL_SIZE property, which states the real pixel height
directly; falls back to the SIZE line's point-size only if PIXEL_SIZE
is absent, since point-size only equals pixel height at exactly
100dpi — several bundled fonts (e.g. 6x13.bdf, 5x8.bdf) are defined
at 75dpi, where the two values genuinely differ."""
size_line_value = None
try:
with open(bdf_path, "r", encoding="ascii", errors="ignore") as f:
for line in f:
if line.startswith("PIXEL_SIZE"):
parts = line.split()
if len(parts) >= 2:
return int(float(parts[1]))
elif line.startswith("SIZE") and size_line_value is None:
# Format: "SIZE <point_size> <xres> <yres>"
parts = line.split()
if len(parts) >= 2:
size_line_value = int(float(parts[1]))
elif line.startswith("STARTCHAR"):
break
except (OSError, ValueError):
return None
return size_line_value
"""A BDF file's one true pixel size; see
:func:`src.common.bdf_font.read_bdf_native_size`."""
return read_bdf_native_size(bdf_path)
def _get_fallback_font(self) -> ImageFont.ImageFont:
"""Get a fallback font when loading fails."""
src/plugin_system/plugin_dirs.py
+345
View File
@@ -0,0 +1,345 @@
"""
One answer to "which directory holds plugin X?".
Five places used to answer it, each with its own rules and each re-parsing
every manifest per lookup: ``PluginManager`` discovery and
``get_plugin_directory``, ``PluginLoader.find_plugin_directory``,
``PluginStoreManager._find_plugin_path`` / ``list_installed_plugins`` and
``state_reconciliation.disk_plugin_ids``. The rules now live here once; what
still legitimately differs between callers (which directories to search,
whether a ``ledmatrix-`` prefix or a case difference counts as a match) is a
keyword argument at the call site, so a difference is always a visible choice
rather than an accident of which copy you read.
The rules
---------
* A directory is a *candidate* when it is a directory (a symlink to one counts:
dev plugins are symlinked in) and its name is neither hidden (leading ``.``)
nor carries :data:`BACKUP_MARKER`. store_manager renames a plugin aside with
that marker during install/rollback; the aside still holds a manifest, so
treating it as a plugin would resurrect a ghost.
* A plugin's id is its manifest ``id``. The directory name is only a fallback,
for callers that must still see a plugin whose manifest is missing an id.
* Resolving an id within one directory: a directory whose manifest declares
the id wins; among several, the one named exactly for the id, then
``ledmatrix-<id>``, then by name. Only when no manifest claims the id do
directory names count: ``<id>``, then ``ledmatrix-<id>`` (``prefix=True``),
then either of those ignoring case (``case_insensitive=True``). The name
fallback still returns a directory whose manifest is unreadable -- that is
how a broken plugin gets uninstalled or reinstalled. ``by_manifest=False``
(``PluginManager.get_plugin_directory``, whose discovery map already holds
the manifest answer) skips straight to the names.
* Several directories are searched one at a time, in the order given: the
first directory that resolves the id at all wins, by manifest or by name.
* The id must be one plain path segment (``safe_path_component``); anything
else resolves to nothing rather than being joined or truncated.
* Returned paths are ``search_dir / name`` and are not resolved, so a
symlinked dev plugin keeps the path that lies inside the search directory.
Manifests are read at most once per :class:`PluginDirectoryIndex`; one index is
one scan.
"""
from __future__ import annotations
import json
from dataclasses import dataclass, field
from pathlib import Path
from typing import Any, Dict, Iterable, List, Optional, Set, Union
from src.common.path_safety import safe_path_component
__all__ = [
'BACKUP_MARKER',
'PLUGIN_DIR_PREFIX',
'ManifestStatus',
'PluginDirEntry',
'PluginDirectoryIndex',
'is_ignored_dir_name',
'resolve_plugin_dir',
'store_search_dirs',
]
#: Substring store_manager embeds in a plugin directory it has set aside
#: (``<id>.standalone-backup-preinstall`` / ``-migrating``). Existing debris on
#: devices carries exactly this text, so it must never change.
BACKUP_MARKER = '.standalone-backup-'
#: Legacy repository naming (``ledmatrix-<id>``); some installs still use it
#: as the directory name.
PLUGIN_DIR_PREFIX = 'ledmatrix-'
PathLike = Union[str, Path]
class ManifestStatus:
"""What reading ``manifest.json`` in a candidate directory produced."""
OK = 'ok' # a JSON object with a non-empty "id"
MISSING = 'missing' # no manifest.json
UNREADABLE = 'unreadable' # I/O error or invalid JSON
NOT_OBJECT = 'not_object' # valid JSON, but not an object
NO_ID = 'no_id' # an object without a usable "id"
def is_ignored_dir_name(name: str) -> bool:
"""True for names that are never a plugin: hidden, or set aside mid-install."""
return name.startswith('.') or BACKUP_MARKER in name
@dataclass
class PluginDirEntry:
"""One candidate directory and its manifest, read once."""
path: Path
status: str
manifest: Optional[Any] = None
error: Optional[BaseException] = None
@property
def name(self) -> str:
return self.path.name
@property
def manifest_id(self) -> Optional[str]:
"""The manifest's ``id`` when the manifest is usable, else None."""
if self.status != ManifestStatus.OK:
return None
return self.manifest['id']
@property
def manifest_parses(self) -> bool:
"""The manifest exists and is valid JSON (of any shape)."""
return self.status in (ManifestStatus.OK, ManifestStatus.NOT_OBJECT,
ManifestStatus.NO_ID)
@property
def installed_id(self) -> str:
"""The manifest id, falling back to the directory name."""
return self.manifest_id or self.name
def _read_entry(path: Path) -> PluginDirEntry:
manifest_path = path / 'manifest.json'
if not manifest_path.is_file():
return PluginDirEntry(path, ManifestStatus.MISSING)
try:
with open(manifest_path, 'r', encoding='utf-8') as handle:
manifest = json.load(handle)
except (OSError, ValueError) as exc: # ValueError covers JSON + decode errors
return PluginDirEntry(path, ManifestStatus.UNREADABLE, error=exc)
if not isinstance(manifest, dict):
return PluginDirEntry(path, ManifestStatus.NOT_OBJECT, manifest)
plugin_id = manifest.get('id')
if not plugin_id or not isinstance(plugin_id, str):
return PluginDirEntry(path, ManifestStatus.NO_ID, manifest)
return PluginDirEntry(path, ManifestStatus.OK, manifest)
def _preference(plugin_id: str, name: str) -> tuple:
"""Sort key among directories that all claim ``plugin_id``."""
if name == plugin_id:
rank = 0
elif name == PLUGIN_DIR_PREFIX + plugin_id:
rank = 1
else:
rank = 2
return (rank, name)
@dataclass
class PluginDirectoryIndex:
"""Every candidate directory directly under ``root``, manifests read once.
Build one with :meth:`scan`. It is a snapshot: a directory added or
removed afterwards is not seen until the next scan.
"""
root: Path
entries: List[PluginDirEntry] = field(default_factory=list)
#: Set when ``root`` exists but could not be listed.
error: Optional[BaseException] = None
_plugins: Optional[Dict[str, PluginDirEntry]] = field(
default=None, init=False, repr=False, compare=False)
@classmethod
def scan(cls, root: PathLike) -> 'PluginDirectoryIndex':
root = Path(root)
index = cls(root)
try:
children = sorted(root.iterdir(), key=lambda p: p.name)
except FileNotFoundError:
return index
except OSError as exc:
index.error = exc
return index
for child in children:
if is_ignored_dir_name(child.name):
continue
try:
if not child.is_dir():
continue
except OSError:
continue
index.entries.append(_read_entry(child))
return index
# -- listing ----------------------------------------------------------
def plugins(self) -> Dict[str, PluginDirEntry]:
"""Manifest id -> entry, one entry per id.
When several directories declare the same id, the one named for it
wins, then ``ledmatrix-<id>``, then the first by name; see
:meth:`duplicates` for the losers.
"""
if self._plugins is not None:
return self._plugins
chosen: Dict[str, PluginDirEntry] = {}
for entry in self.entries:
plugin_id = entry.manifest_id
if plugin_id is None:
continue
current = chosen.get(plugin_id)
if current is None or (_preference(plugin_id, entry.name)
< _preference(plugin_id, current.name)):
chosen[plugin_id] = entry
self._plugins = chosen
return chosen
def duplicates(self) -> Dict[str, List[PluginDirEntry]]:
"""Ids declared by more than one directory -> every such entry."""
seen: Dict[str, List[PluginDirEntry]] = {}
for entry in self.entries:
if entry.manifest_id is not None:
seen.setdefault(entry.manifest_id, []).append(entry)
return {k: v for k, v in seen.items() if len(v) > 1}
def installed_ids(self, *, require_parseable_manifest: bool) -> Set[str]:
"""Ids of everything that counts as installed.
A directory counts when it has a manifest.json -- which must also be
valid JSON when ``require_parseable_manifest``. Its id is the manifest
id, or the directory name when the manifest does not carry one.
"""
ids: Set[str] = set()
for entry in self.entries:
if entry.status == ManifestStatus.MISSING:
continue
if require_parseable_manifest and not entry.manifest_parses:
continue
ids.add(entry.installed_id)
return ids
def entry_for_installed_id(self, plugin_id: str) -> Optional[PluginDirEntry]:
"""The entry :meth:`installed_ids` reported as ``plugin_id``."""
entry = self.plugins().get(plugin_id)
if entry is not None:
return entry
for entry in self.entries:
if entry.manifest_id is None and entry.name == plugin_id:
return entry
return None
# -- lookup -----------------------------------------------------------
def find(self, plugin_id: str, *, prefix: bool, case_insensitive: bool,
by_manifest: bool = True) -> Optional[Path]:
"""Resolve ``plugin_id`` within this directory (rules in the module doc)."""
plugin_id = _lookup_id(plugin_id)
if plugin_id is None:
return None
if by_manifest:
entry = self.plugins().get(plugin_id)
if entry is not None:
return entry.path
names = _candidate_names(plugin_id, prefix)
by_name = {e.name: e for e in self.entries}
for name in names:
if name in by_name:
return by_name[name].path
if case_insensitive:
for low in (n.lower() for n in names):
for entry in self.entries:
if entry.name.lower() == low:
return entry.path
return None
def _lookup_id(plugin_id: Any) -> Optional[str]:
"""``plugin_id`` if it can name a plugin directory at all, else None."""
plugin_id = safe_path_component(plugin_id)
if plugin_id is None or is_ignored_dir_name(plugin_id):
return None
return plugin_id
def _candidate_names(plugin_id: str, prefix: bool) -> List[str]:
names = [plugin_id]
if prefix:
names.append(PLUGIN_DIR_PREFIX + plugin_id)
return names
def _is_dir(path: Path) -> bool:
try:
return path.is_dir()
except OSError:
return False
def resolve_plugin_dir(plugin_id: Any, search_dirs: Iterable[PathLike], *,
prefix: bool, case_insensitive: bool = False,
by_manifest: bool = True) -> Optional[Path]:
"""The directory holding ``plugin_id``, searching ``search_dirs`` in order.
Each search directory is scanned once and each manifest in it read once.
``by_manifest=False`` skips the manifest pass and matches directory names
only, which reads no manifests at all.
Names are compared against the directory listing, never by probing
``search_dir / name``: on a case-insensitive filesystem that probe says
``Demo`` exists when the directory is ``demo``, which made the answer
depend on the platform.
"""
plugin_id = _lookup_id(plugin_id)
if plugin_id is None:
return None
for search_dir in search_dirs:
search_dir = Path(search_dir)
if by_manifest or case_insensitive:
found = PluginDirectoryIndex.scan(search_dir).find(
plugin_id, prefix=prefix, case_insensitive=case_insensitive,
by_manifest=by_manifest)
else:
found = _find_by_name(search_dir, _candidate_names(plugin_id, prefix))
if found is not None:
return found
return None
def _find_by_name(search_dir: Path, names: List[str]) -> Optional[Path]:
try:
present = {child.name for child in search_dir.iterdir()}
except OSError:
return None
for name in names:
if name in present and _is_dir(search_dir / name):
return search_dir / name
return None
def store_search_dirs(plugins_dir: PathLike) -> List[Path]:
"""Directories the plugin store searches: the configured one, then a
sibling ``plugins/`` (the legacy/dev location) when that is a different
directory. Discovery deliberately does NOT use this -- it scans only the
configured directory (see CLAUDE.md, test_discovery_path_contract.py)."""
plugins_dir = Path(plugins_dir)
dirs = [plugins_dir]
try:
base = plugins_dir if plugins_dir.is_absolute() else plugins_dir.resolve()
sibling = base.parent / 'plugins'
if sibling != base:
dirs.append(sibling)
except (OSError, ValueError):
pass
return dirs
src/plugin_system/plugin_loader.py
+15 -64
View File
@@ -8,7 +8,6 @@ Extracted from PluginManager to improve separation of concerns.
import importlib
import importlib.metadata
import importlib.util
import json
import os
import sys
import subprocess
@@ -21,6 +20,7 @@ from packaging.requirements import InvalidRequirement, Requirement
from src.exceptions import PluginError
from src.logging_config import get_logger
from src.plugin_system.plugin_dirs import resolve_plugin_dir
def requirements_has_real_deps(requirements_file: str) -> bool:
@@ -215,11 +215,12 @@ class PluginLoader:
"""
Find the plugin directory for a given plugin ID.
Tries multiple strategies:
1. Use plugin_directories mapping if available
2. Direct path matching
3. Case-insensitive directory matching
4. Manifest-based search
1. The discovery mapping, when it has the id and the path exists.
2. ``plugins_dir`` only, by the shared rules in
``src/plugin_system/plugin_dirs.py``: a directory whose manifest
declares the id wins; otherwise ``<id>`` or ``ledmatrix-<id>``,
matched case-insensitively. Backup and hidden directories are
never matched.
Args:
plugin_id: Plugin identifier
@@ -227,13 +228,9 @@ class PluginLoader:
plugin_directories: Optional mapping of plugin_id to directory
Returns:
Path to plugin directory or None if not found
Path to plugin directory or None if not found. An id that is not
one plain path segment finds nothing.
"""
# Sanitize plugin_id — os.path.basename is a CodeQL-recognized path sanitizer
plugin_id = os.path.basename(plugin_id or '')
if not plugin_id:
return None
# Strategy 1: Use mapping from discovery
if plugin_directories and plugin_id in plugin_directories:
plugin_dir = plugin_directories[plugin_id]
@@ -241,58 +238,12 @@ class PluginLoader:
self.logger.debug("Using plugin directory from discovery mapping: %s", plugin_dir)
return plugin_dir
# Strategy 2: Direct paths — resolve and validate they stay within plugins_dir
plugins_dir_resolved = plugins_dir.resolve()
for _candidate_name in (plugin_id, f"ledmatrix-{plugin_id}"):
_candidate = (plugins_dir_resolved / _candidate_name).resolve()
try:
_candidate.relative_to(plugins_dir_resolved)
except ValueError:
continue
if _candidate.exists():
return _candidate
# Strategy 3: Case-insensitive search
normalized_id = plugin_id.lower()
for item in plugins_dir.iterdir():
if not item.is_dir():
continue
item_name = item.name
if item_name.lower() == normalized_id:
return item
if item_name.lower() == f"ledmatrix-{plugin_id}".lower():
return item
# Strategy 4: Manifest-based search
self.logger.debug("Directory name search failed for %s, searching by manifest...", plugin_id)
for item in plugins_dir.iterdir():
if not item.is_dir():
continue
# Skip if already checked
if item.name.lower() == normalized_id or item.name.lower() == f"ledmatrix-{plugin_id}".lower():
continue
manifest_path = item / "manifest.json"
if manifest_path.exists():
try:
with open(manifest_path, 'r', encoding='utf-8') as f:
item_manifest = json.load(f)
item_manifest_id = item_manifest.get('id')
if item_manifest_id == plugin_id:
self.logger.info(
"Found plugin %s in directory %s (manifest ID matches)",
plugin_id,
item.name
)
return item
except (json.JSONDecodeError, Exception) as e:
self.logger.debug("Skipping %s due to manifest error: %s", item.name, e)
continue
return None
plugin_dir = resolve_plugin_dir(
plugin_id, [plugins_dir], prefix=True, case_insensitive=True)
if plugin_dir is not None and plugin_dir.name != plugin_id:
self.logger.debug("Found plugin %s in directory %s",
plugin_id, plugin_dir.name)
return plugin_dir
def install_dependencies(
self,
src/plugin_system/plugin_manager.py
+77 -80
View File
@@ -25,7 +25,9 @@ from src.plugin_system.plugin_state import PluginStateManager, PluginState
from src.plugin_system.schema_manager import (
CORE_VEGAS_TUNING_KEYS, SchemaManager, normalize_legacy_booleans,
)
from src.common.path_safety import safe_path_component
from src.plugin_system.plugin_dirs import (
ManifestStatus, PluginDirectoryIndex, resolve_plugin_dir,
)
from src.deprecation import deprecated
from src.common.permission_utils import (
ensure_directory_permissions,
@@ -175,89 +177,88 @@ class PluginManager:
self.logger.error("Could not create plugins directory %s: %s", self.plugins_dir, e, exc_info=True)
raise PluginError(f"Could not create plugins directory: {self.plugins_dir}", context={'error': str(e)}) from e
def _report_skip_once(self, key: str, message: str, *args: Any) -> None:
"""Warn about a skipped directory once per process, not per scan.
Discovery runs on every web UI page load and every config reconcile,
so warning unconditionally would put a line in the journal each time
someone opened a page -- the same log-volume problem this is meant to
help diagnose.
"""
reported = self.__dict__.setdefault('_skip_reported', set())
if key in reported:
return
reported.add(key)
self.logger.warning(message, *args)
def _scan_directory_for_plugins(self, directory: Path) -> List[str]:
"""
Scan a directory for plugins.
Which directories count and how an id maps to one is decided by
:class:`PluginDirectoryIndex` (``src/plugin_system/plugin_dirs.py``),
shared with the loader, the store and reconciliation. Only
``directory`` is scanned: discovery has no fallback to ``plugins/``.
Directories set aside mid-install (``BACKUP_MARKER`` in the name) are
skipped so they don't overwrite live entries.
Args:
directory: Directory to scan
Returns:
List of plugin IDs found
"""
plugin_ids = []
if not directory.exists():
return plugin_ids
return []
# Build new state locally before acquiring lock
new_manifests: Dict[str, Dict[str, Any]] = {}
new_directories: Dict[str, Path] = {}
try:
for item in directory.iterdir():
if not item.is_dir():
continue
# Skip backup directories so they don't overwrite live entries
if '.standalone-backup-' in item.name:
continue
manifest_path = item / "manifest.json"
if not manifest_path.exists():
# Once per directory per process. Discovery runs on every
# web UI page load and every config reconcile, so warning
# unconditionally would put a line in the journal each
# time someone opened a page -- the same log-volume
# problem this is meant to help diagnose.
# A directory here that carries no manifest is not a
# plugin. Said once, because the alternative is a plugin
# that is enabled in config, enabled in plugin state,
# present on disk, and simply absent from the running
# process with nothing anywhere to say why. Working that
# out afterwards means reading cache-file mtimes.
if item.name not in self._skip_reported:
self._skip_reported.add(item.name)
self.logger.warning(
"Skipping %s: no manifest.json, so it cannot be "
"loaded as a plugin", item.name)
continue
try:
with open(manifest_path, 'r', encoding='utf-8') as f:
manifest = json.load(f)
except (json.JSONDecodeError, PermissionError, OSError) as e:
self.logger.warning("Error reading manifest from %s: %s", manifest_path, e, exc_info=True)
continue
index = PluginDirectoryIndex.scan(directory)
if index.error is not None:
self.logger.error("Error scanning directory %s: %s", directory,
index.error, exc_info=index.error)
for entry in index.entries:
if entry.status == ManifestStatus.MISSING:
# A directory here that carries no manifest is not a plugin.
# Said once, because the alternative is a plugin that is
# enabled in config, enabled in plugin state, present on disk,
# and simply absent from the running process with nothing
# anywhere to say why. Working that out afterwards means
# reading cache-file mtimes.
self._report_skip_once(
entry.name, "Skipping %s: no manifest.json, so it cannot be "
"loaded as a plugin", entry.name)
elif entry.status == ManifestStatus.UNREADABLE:
self.logger.warning("Error reading manifest from %s: %s",
entry.path / "manifest.json", entry.error,
exc_info=entry.error)
elif entry.status == ManifestStatus.NOT_OBJECT:
# json.load accepts any JSON value, so a manifest holding
# null, [] or "text" parses and then raises AttributeError on
# .get(). Nothing here catches that -- the outer handler takes
# OSError/PermissionError only -- so a single malformed
# manifest aborted the whole scan and every other plugin on
# null, [] or "text" parses. It once raised AttributeError on
# .get() and aborted the whole scan, so every other plugin on
# disk, however healthy, silently failed to register.
if not isinstance(manifest, dict):
if item.name not in self._skip_reported:
self._skip_reported.add(item.name)
self.logger.warning(
"Skipping %s: its manifest.json is %s, not a JSON "
"object", item.name, type(manifest).__name__)
continue
self._report_skip_once(
entry.name, "Skipping %s: its manifest.json is %s, not a "
"JSON object", entry.name, type(entry.manifest).__name__)
elif entry.status == ManifestStatus.NO_ID:
# Parsed but unusable. This was the quietest path of all: the
# manifest is read successfully and then dropped.
self._report_skip_once(
entry.name, "Skipping %s: its manifest.json has no \"id\", "
"so there is nothing to register it under", entry.name)
plugin_id = manifest.get('id')
if not plugin_id:
# Parsed but unusable. This was the quietest path of all:
# the manifest is read successfully and then dropped.
if item.name not in self._skip_reported:
self._skip_reported.add(item.name)
self.logger.warning(
"Skipping %s: its manifest.json has no \"id\", so "
"there is nothing to register it under", item.name)
continue
plugins = index.plugins()
for plugin_id, entries in index.duplicates().items():
self._report_skip_once(
"duplicate:" + plugin_id,
"Plugin id %r is declared by %d directories (%s); using %s",
plugin_id, len(entries), ", ".join(e.name for e in entries),
plugins[plugin_id].name)
plugin_ids.append(plugin_id)
new_manifests[plugin_id] = manifest
new_directories[plugin_id] = item
except (OSError, PermissionError) as e:
self.logger.error("Error scanning directory %s: %s", directory, e, exc_info=True)
new_manifests: Dict[str, Dict[str, Any]] = {
plugin_id: entry.manifest for plugin_id, entry in plugins.items()}
new_directories: Dict[str, Path] = {
plugin_id: entry.path for plugin_id, entry in plugins.items()}
# Replace shared state under lock so uninstalled plugins don't linger
with self._discovery_lock:
@@ -266,7 +267,7 @@ class PluginManager:
self.plugin_directories.clear()
self.plugin_directories.update(new_directories)
return plugin_ids
return list(plugins)
def discover_plugins(self) -> List[str]:
"""
@@ -772,24 +773,20 @@ class PluginManager:
not one plain path segment (``..``, ``a/b``, an absolute path) is
refused instead of being joined onto ``plugins_dir``. The join is not
resolved further: dev plugins are symlinks into ``plugins_dir``.
The discovery map is authoritative. For an id discovery has not seen,
only directory names are tried -- ``<id>`` then ``ledmatrix-<id>``,
in ``plugins_dir`` only -- so a miss on a web request never reads
every manifest on disk. Rules: ``src/plugin_system/plugin_dirs.py``.
"""
with self._discovery_lock:
if plugin_id in self.plugin_directories:
return str(self.plugin_directories[plugin_id])
plugin_id = safe_path_component(plugin_id)
if plugin_id is None:
return None
plugin_dir = self.plugins_dir / plugin_id
if plugin_dir.exists():
return str(plugin_dir)
plugin_dir = self.plugins_dir / f"ledmatrix-{plugin_id}"
if plugin_dir.exists():
return str(plugin_dir)
return None
plugin_dir = resolve_plugin_dir(
plugin_id, [self.plugins_dir], prefix=True, case_insensitive=False,
by_manifest=False)
return str(plugin_dir) if plugin_dir is not None else None
def get_plugin_display_modes(self, plugin_id: str) -> List[str]:
"""
src/plugin_system/state_reconciliation.py
+26 -32
View File
@@ -15,6 +15,7 @@ from enum import Enum
from pathlib import Path
from src.core_config_keys import CORE_CONFIG_KEYS
from src.plugin_system.plugin_dirs import PluginDirectoryIndex
from src.plugin_system.state_manager import PluginStateManager
from src.logging_config import get_logger
@@ -102,31 +103,25 @@ def config_plugin_ids(config: Dict[str, Any], ignored_keys: Set[str]) -> Set[str
def disk_plugin_ids(plugins_dir) -> Set[str]:
"""Plugin ids actually installed on disk.
A directory counts only when it is not a standalone backup and its
manifest.json parses. A corrupt manifest must not read as installed, or a
live "in config but not on disk" finding gets cleared on the strength of an
unreadable file.
A directory counts only when it is not a standalone backup (or hidden)
and its manifest.json parses. A corrupt manifest must not read as
installed, or a live "in config but not on disk" finding gets cleared on
the strength of an unreadable file.
The id is the manifest's ``id`` -- what discovery registers and what the
config is keyed by -- and the directory name only when the manifest has
none. Directory names alone made a plugin living in ``ledmatrix-stocks/``
with id ``stocks`` read as both "stocks in config but not on disk" and
"ledmatrix-stocks on disk but not in config".
"""
ids: Set[str] = set()
root = Path(plugins_dir)
try:
if not root.exists():
return ids
for entry in root.iterdir():
if not entry.is_dir() or '.standalone-backup-' in entry.name:
continue
manifest = entry / "manifest.json"
if not manifest.exists():
continue
try:
with open(manifest, 'r') as f:
json.load(f)
except (OSError, ValueError):
continue
ids.add(entry.name)
return _disk_index(plugins_dir).installed_ids(require_parseable_manifest=True)
except OSError:
return ids
return ids
return set()
def _disk_index(plugins_dir) -> PluginDirectoryIndex:
return PluginDirectoryIndex.scan(Path(plugins_dir))
def still_unresolved(entries: List[Dict[str, Any]],
@@ -326,16 +321,15 @@ class StateReconciliation:
"""Get plugin state from disk (installed plugins)."""
state = {}
try:
# Membership comes from the shared extractor so the web interface
# re-checks stored findings against this same definition; the
# manifest is then re-read here only for version/name.
for plugin_id in disk_plugin_ids(self.plugins_dir):
manifest_path = self.plugins_dir / plugin_id / "manifest.json"
try:
with open(manifest_path, 'r') as f:
manifest = json.load(f)
except (OSError, ValueError): # nosec B112 - raced or corrupt; skip
continue
# Membership uses the same index and rule as disk_plugin_ids, so
# the web interface re-checks stored findings against this same
# definition; each manifest is read once, by the scan.
index = _disk_index(self.plugins_dir)
for plugin_id in index.installed_ids(require_parseable_manifest=True):
entry = index.entry_for_installed_id(plugin_id)
manifest = entry.manifest if entry is not None else None
if not isinstance(manifest, dict):
manifest = {}
state[plugin_id] = {
'exists_on_disk': True,
'version': manifest.get('version'),
src/plugin_system/store_manager.py
+45 -94
View File
@@ -28,6 +28,9 @@ from src.common.permission_utils import sudo_remove_directory, install_requireme
from src.plugin_system.plugin_loader import (
requirements_has_real_deps, requirements_are_satisfied, find_trusted_subdir
)
from src.plugin_system.plugin_dirs import (
BACKUP_MARKER, PluginDirectoryIndex, resolve_plugin_dir, store_search_dirs,
)
try:
from jsonschema import Draft7Validator, ValidationError
@@ -1233,9 +1236,9 @@ class PluginStoreManager:
Pass-through when nothing is installed, and when called from
`_reinstall_with_rollback`, which has already moved the old copy aside.
The aside name embeds '.standalone-backup-' so plugin discovery
(`plugin_manager._scan_directory_for_plugins`) skips it even though it
still holds a manifest.json.
The aside name embeds BACKUP_MARKER ('.standalone-backup-') so every
plugin directory lookup (src/plugin_system/plugin_dirs.py) skips it
even though it still holds a manifest.json.
Held under the per-plugin reinstall lock for the same reason
`_reinstall_with_rollback` is: the web UI runs Flask with
@@ -1250,7 +1253,7 @@ class PluginStoreManager:
return self._install_plugin_impl(plugin_id, branch)
backup_path = plugin_path.with_name(
f"{plugin_path.name}.standalone-backup-preinstall")
f"{plugin_path.name}{BACKUP_MARKER}preinstall")
if backup_path.exists() and not self._safe_remove_directory(backup_path):
# Can't stage a safety net. Better to attempt the install than
# to refuse outright, which is what callers got before this
@@ -2394,93 +2397,41 @@ class PluginStoreManager:
"""
Find the plugin path by checking the configured directory and standard plugins directory.
Searches the configured directory, then a sibling ``plugins/`` (the
case where plugins sit in plugins/ but config says plugin-repos/) --
a store-only fallback; discovery scans the configured directory only.
Each directory is searched completely before the next, by the shared
rules in ``src/plugin_system/plugin_dirs.py``: a directory whose
manifest declares the id wins, then a directory named exactly for it.
The manifest match matters because a directory name can differ from
the id its manifest declares (a hand-made or legacy layout such as
`ledmatrix-stocks/` holding id `stocks`); a lookup by directory name
alone reported such a plugin as not installed, so update_plugin()
silently did nothing.
No ``ledmatrix-`` prefix and no case folding here, unlike the loader:
a store operation may delete what this returns, so it only accepts a
directory that names the id exactly or declares it. Note that this
leaves registry ids like `stocks` unresolved when the installed
plugin is `ledmatrix-stocks/` declaring `ledmatrix-stocks` (the
monorepo's leaderboard, music, stocks and weather); passing
``prefix=True`` would resolve them, but update_plugin()'s reinstall
path has not been checked against that yet.
Args:
plugin_id: Plugin identifier
Returns:
Path to plugin directory if found, None otherwise
"""
# First check the configured plugins directory
plugin_path = self.plugins_dir / plugin_id
if plugin_path.exists():
return plugin_path
# Also check the standard 'plugins/' directory if it's different
# This handles the case where plugins are in plugins/ but config says plugin-repos/
try:
if self.plugins_dir.is_absolute():
project_root = self.plugins_dir.parent
else:
project_root = self.plugins_dir.resolve().parent
standard_plugins_dir = project_root / 'plugins'
if standard_plugins_dir.exists() and standard_plugins_dir != self.plugins_dir:
plugin_path = standard_plugins_dir / plugin_id
if plugin_path.exists():
return plugin_path
except (OSError, ValueError):
pass
# Last resort: the directory name may differ from the id being looked
# up. install_plugin() deliberately renames a plugin's directory to the
# MANIFEST id when it differs from the REGISTRY id (see the rename near
# "doesn't match registry ID" above), so `stocks` in the registry lands
# in `ledmatrix-stocks/`. Every lookup above is by directory name, so
# update_plugin("stocks") found nothing and reported the plugin as not
# installed -- silently, and for good: the user sees no error and stays
# on a stale version. Four installed plugins hit this in practice
# (leaderboard, music, stocks, weather).
#
# Deliberately last so the two lookups above keep their exact meaning;
# this only runs when a direct hit already failed. See
# test_discovery_path_contract.py, which pins that ordering.
for search_dir in self._candidate_plugin_dirs():
match = self._find_by_manifest_id(search_dir, plugin_id)
if match is not None:
self.logger.debug(
"Resolved plugin '%s' to %s via its manifest id "
"(directory name differs from the id)", plugin_id, match)
return match
return None
return resolve_plugin_dir(
plugin_id, self._candidate_plugin_dirs(), prefix=False,
case_insensitive=False)
def _candidate_plugin_dirs(self) -> List[Path]:
"""Directories that may hold installed plugins, configured one first."""
dirs = [self.plugins_dir]
try:
base = self.plugins_dir if self.plugins_dir.is_absolute() else self.plugins_dir.resolve()
sibling = base.parent / 'plugins'
if sibling != self.plugins_dir:
dirs.append(sibling)
except (OSError, ValueError):
pass
return [d for d in dirs if d.exists()]
@staticmethod
def _find_by_manifest_id(search_dir: Path, plugin_id: str) -> Optional[Path]:
"""A subdirectory of `search_dir` whose manifest declares `plugin_id`.
Skips half-finished installs: store_manager renames a directory aside
with '.standalone-backup-' during install and rollback, and treating
one as installed would resurrect a ghost plugin.
"""
try:
entries = sorted(search_dir.iterdir())
except (OSError, ValueError):
return None
for entry in entries:
if not entry.is_dir() or '.standalone-backup-' in entry.name:
continue
manifest = entry / 'manifest.json'
if not manifest.is_file():
continue
try:
with open(manifest, 'r', encoding='utf-8') as handle:
if json.load(handle).get('id') == plugin_id:
return entry
except (OSError, ValueError):
continue
return None
return [d for d in store_search_dirs(self.plugins_dir) if d.exists()]
def uninstall_plugin(self, plugin_id: str) -> bool:
"""
@@ -2600,9 +2551,9 @@ class PluginStoreManager:
field during the monorepo migration on a Pi with broken DNS — every
old-remote plugin was deleted and none could be re-downloaded).
The aside name embeds '.standalone-backup-' so plugin discovery
(plugin_manager._scan_directory_for_plugins) ignores it even though
it still contains a manifest.json.
The aside name embeds BACKUP_MARKER ('.standalone-backup-') so every
plugin directory lookup (src/plugin_system/plugin_dirs.py) ignores it
even though it still contains a manifest.json.
Held for the whole operation under a per-plugin_id lock: two
overlapping requests for the same plugin (double-click, two
@@ -2612,7 +2563,7 @@ class PluginStoreManager:
"""
with self._get_reinstall_lock(plugin_id):
backup_path = plugin_path.with_name(
f"{plugin_path.name}.standalone-backup-migrating")
f"{plugin_path.name}{BACKUP_MARKER}migrating")
# A stale aside from a previous crash would block the rename
if backup_path.exists():
if not self._safe_remove_directory(backup_path):
@@ -3085,15 +3036,15 @@ class PluginStoreManager:
"""
Get list of installed plugin IDs.
One entry per plugin directory in the configured directory that has a
manifest.json, named by the manifest's id (the directory name when
the manifest carries none, e.g. because it does not parse). Backup
and hidden directories are not plugins.
Returns:
List of plugin IDs
List of plugin IDs, sorted
"""
if not self.plugins_dir.exists():
return []
installed = []
for item in self.plugins_dir.iterdir():
if item.is_dir() and (item / "manifest.json").exists():
installed.append(item.name)
return installed
index = PluginDirectoryIndex.scan(self.plugins_dir)
return sorted(index.installed_ids(require_parseable_manifest=False))
src/plugin_system/testing/visual_display_manager.py
+17 -34
View File
@@ -14,13 +14,16 @@ PIL Image canvas and draws text using the actual project fonts.
MAINTENANCE WARNING: this class is a deliberate fork of
src/display_manager.py so it can run without hardware. It mirrors
these DisplayManager methods by name and behavior: _load_fonts,
_draw_bdf_text, get_font_height, get_text_width, draw_text,
get_font_height, get_text_width, draw_text,
draw_text_with_icons, draw_weather_icon (and the _draw_sun/_draw_cloud/
_draw_rain/_draw_snow/_draw_storm family), format_date_with_ordinal,
capture_mode, set_scrolling_state, is_currently_scrolling,
process_deferred_updates, update_display, render_size. A behavior
change to any of those in DisplayManager must be mirrored here, or
plugin visual tests will pass against stale behavior.
BDF text is not mirrored: both classes load BDF faces and draw BDF glyphs
through src/common/bdf_font.py, so those pixels cannot drift.
"""
import math
@@ -31,6 +34,7 @@ from pathlib import Path
from typing import Any, List, Optional, Tuple
from PIL import Image, ImageDraw, ImageFont
from src.common.bdf_font import draw_bdf_text, load_bdf_face
from src.common.font_layout import crisp_size, load_truetype
from src.logging_config import get_logger
@@ -147,16 +151,18 @@ class VisualTestDisplayManager:
self.small_font = load_truetype(ttf_path, crisp_size(press_start, 8))
self.font = self.regular_font # alias used by some code paths
# 5x7 BDF font via freetype
# 5x7 BDF font, loaded exactly as DisplayManager._load_fonts does
# (same loader, same 7px request as its _CALENDAR_FONT_PX). A bare
# freetype.Face has no active size, so its ascender reads 0 and
# every line drew a baseline too high.
try:
import freetype
bdf_path = str(fonts_dir / '5x7.bdf')
if not os.path.exists(bdf_path):
raise FileNotFoundError(f"BDF font not found: {bdf_path}")
face = freetype.Face(bdf_path)
face, _ = load_bdf_face(bdf_path, 7)
self.calendar_font = face
self.bdf_5x7_font = face
except (ImportError, FileNotFoundError, OSError) as e:
except Exception as e: # freetype missing or the file unloadable
logger.debug("BDF font not available, using small_font as fallback: %s", e)
self.calendar_font = self.small_font
self.bdf_5x7_font = self.small_font
@@ -300,41 +306,18 @@ class VisualTestDisplayManager:
logger.debug(f"Error drawing image: {e}")
def _draw_bdf_text(self, text, x, y, color=(255, 255, 255), font=None):
"""Draw text using BDF font with proper bitmap handling.
"""Draw text in a BDF ``freetype.Face`` with (x, y) as its top-left.
Replicated from DisplayManager._draw_bdf_text().
Not a copy: DisplayManager._draw_bdf_text calls the same
:func:`src.common.bdf_font.draw_bdf_text`, so what this draws is
what the panel draws.
"""
try:
if isinstance(color, list):
color = tuple(color)
face = font if font else self.calendar_font
# Compute baseline from font ascender
try:
ascender_px = face.size.ascender >> 6
except Exception:
ascender_px = 0
baseline_y = y + ascender_px
for char in text:
face.load_char(char)
bitmap = face.glyph.bitmap
glyph_left = face.glyph.bitmap_left
glyph_top = face.glyph.bitmap_top
for i in range(bitmap.rows):
for j in range(bitmap.width):
byte_index = i * bitmap.pitch + (j // 8)
if byte_index < len(bitmap.buffer):
byte = bitmap.buffer[byte_index]
if byte & (1 << (7 - (j % 8))):
pixel_x = x + glyph_left + j
pixel_y = baseline_y - glyph_top + i
if 0 <= pixel_x < self.width and 0 <= pixel_y < self.height:
self.draw.point((pixel_x, pixel_y), fill=color)
x += face.glyph.advance.x >> 6
draw_bdf_text(self.draw, text, x, y, face, color,
clip=(self.width, self.height))
except Exception as e:
logger.debug(f"Error drawing BDF text: {e}")
src/redaction.py
+15 -2
View File
@@ -24,8 +24,13 @@ _REDACT_CREDENTIAL = re.compile(
# silently leak the ones nobody thought of. Not covered by the generic pattern
# above, whose value part stops at whitespace and so would keep the credential
# once a space follows the scheme.
#
# The opening quote and the whitespace after it are one optional unit. Written
# `\s*["\']?\s*`, a whitespace run with no quote in it could be split between
# the two `\s*` in every possible way, and a header with no credential after
# it tried them all: quadratic, 8s for 20k spaces.
_REDACT_AUTH_HEADER = re.compile(
r'((?:proxy-)?authorization["\']?\s*[=:]\s*["\']?\s*'
r'((?:proxy-)?authorization["\']?\s*[=:]\s*(?:["\']\s*)?'
r'(?:[A-Za-z][\w.+-]*[ \t]+)?)' # optional scheme name, kept
r'([^\s,"\'<>}]+)', # the credential, redacted
re.IGNORECASE,
@@ -34,7 +39,15 @@ _REDACT_AUTH_HEADER = re.compile(
# Credentials embedded in a URL: https://user:password@host. requests quotes
# the full URL in its exceptions, so this is a realistic leak. The username is
# kept -- it identifies which account failed without being the secret.
_REDACT_URL_USERINFO = re.compile(r'([a-z][a-z0-9+.-]*://[^/\s:@]+:)([^/\s@]+)(@)',
#
# A match may only start where a run of scheme characters starts. Unanchored,
# `[a-z][a-z0-9+.-]*://` was tried from every letter of a long run (a hex
# digest, an ID, a blob of response body), each attempt reading to the end of
# the run: quadratic, 1.6s for 20k characters, all of it holding the GIL.
# Leading digits and `+.-` sit inside group 1 so the substitution puts them
# back; the scheme proper still has to start with a letter.
_REDACT_URL_USERINFO = re.compile(
r'((?<![a-z0-9+.-])[0-9+.-]*[a-z][a-z0-9+.-]*://[^/\s:@]+:)([^/\s@]+)(@)',
re.IGNORECASE)
src/web_interface/api_helpers.py
+37 -2
View File
@@ -9,7 +9,7 @@ from typing import Any, Optional, Dict, Tuple
from flask import jsonify, request
from src.web_interface.error_handler import create_error_response, create_success_response
from src.web_interface.errors import ErrorCode
from src.web_interface.errors import ErrorCode, WebInterfaceError
def success_response(
@@ -34,7 +34,8 @@ def success_response(
# metadata block for responses that have neither.
enriched = dict(metadata) if metadata is not None else {}
if hasattr(request, 'start_time'):
enriched['response_time_ms'] = int((time.time() - request.start_time) * 1000)
# request_logging stamps start_time from perf_counter, not the wall clock.
enriched['response_time_ms'] = int((time.perf_counter() - request.start_time) * 1000)
if metadata is not None or enriched:
response_data['metadata'] = enriched
@@ -74,6 +75,40 @@ def error_response(
)
def exception_error_response(
exc: Exception,
error_code: ErrorCode,
*,
with_context: bool = True,
status_code: int = 500
):
"""
error_response() for a caught exception, built by WebInterfaceError.
The message is the code's fixed, user-facing one -- never the exception
text. `details` comes from the exception's own `context` dict when it has
one, and `context` records the exception type. with_context=False leaves
the context out, as the operation-history routes always have.
Args:
exc: The exception being reported
error_code: Error code
with_context: Whether to include the context (exception type)
status_code: HTTP status code
Returns:
Flask jsonify response with status code
"""
error = WebInterfaceError.from_exception(exc, error_code)
return error_response(
error.error_code,
error.message,
details=error.details,
context=error.context if with_context else None,
status_code=status_code
)
def validate_request_json(required_fields: list, data: Optional[Dict] = None) -> Tuple[Optional[Dict], Optional[Any]]:
"""
Validate request JSON has required fields.
src/web_interface/error_handler.py
+34 -3
View File
@@ -7,9 +7,7 @@ Provides helpers for consistent error responses across API endpoints.
from typing import Any, Optional
from flask import jsonify
from src.web_interface.errors import (
WebInterfaceError, ErrorCode, ErrorCategory
)
from src.web_interface.errors import WebInterfaceError, ErrorCode
from src.logging_config import get_logger
from src.redaction import redact_credentials
@@ -72,6 +70,39 @@ def redact_text(text: str, max_length: int = _MAX_DETAIL_LENGTH) -> str:
return text
# What a failure nothing anticipated says. The detail beside it carries the
# actual diagnosis; this sentence only points at where the traceback went.
UNHANDLED_ERROR_MESSAGE = 'An error occurred; see logs for details'
def unhandled_exception_payload(exc: BaseException) -> dict:
"""JSON body for an exception no route handled: status, message, details.
Deliberately no `error_code`. The plugin API client (api_client.js) passes
a body that has one straight to the rich error modal, and wraps one that
has none as a plain API_ERROR toast; the api_v3 routes answered this shape
from their own catch-alls for years, so the UI is built around it.
"""
return {
'status': 'error',
'message': UNHANDLED_ERROR_MESSAGE,
'details': describe_exception(exc),
}
def http_exception_payload(error) -> dict:
"""JSON body for a werkzeug HTTPException (405, 400, 415, 413...).
Same shape web_interface/app.py's global handler returns, so a 4xx raised
inside an api_v3 route reads the same as one raised anywhere else.
"""
return {
'status': 'error',
'error_code': (error.name or 'HTTP_ERROR').upper().replace(' ', '_'),
'message': error.description,
}
def create_error_response(
error_code: ErrorCode,
message: str,
src/web_interface/errors.py
+5 -63
View File
@@ -1,7 +1,7 @@
"""
Structured error handling for web interface.
Provides error codes, categories, and consistent error response formatting.
Provides error codes and consistent error response formatting.
"""
from enum import Enum
@@ -9,17 +9,6 @@ from typing import Dict, Any, Optional, List
from dataclasses import dataclass
class ErrorCategory(Enum):
"""Error categories for classification."""
CONFIGURATION = "configuration"
PLUGIN = "plugin"
VALIDATION = "validation"
NETWORK = "network"
PERMISSION = "permission"
SYSTEM = "system"
UNKNOWN = "unknown"
class ErrorCode(Enum):
"""Error codes for specific error types."""
# Configuration errors
@@ -63,12 +52,11 @@ class WebInterfaceError:
"""
Structured error for web interface responses.
Provides consistent error format with error codes, categories,
messages, and context.
Provides consistent error format with error codes, messages, and
context.
"""
error_code: ErrorCode
message: str
category: ErrorCategory
details: Optional[str] = None
context: Optional[Dict[str, Any]] = None
suggested_fixes: Optional[List[str]] = None
@@ -78,7 +66,6 @@ class WebInterfaceError:
self,
error_code: ErrorCode,
message: str,
category: Optional[ErrorCategory] = None,
details: Optional[str] = None,
context: Optional[Dict[str, Any]] = None,
suggested_fixes: Optional[List[str]] = None,
@@ -86,7 +73,6 @@ class WebInterfaceError:
):
self.error_code = error_code
self.message = message
self.category = category or self._infer_category(error_code)
self.details = details
self.context = context or {}
# `is None`, not truthiness: an explicit [] means "this caller has
@@ -96,25 +82,6 @@ class WebInterfaceError:
else self._get_default_suggestions(error_code))
self.original_error = original_error
def _infer_category(self, error_code: ErrorCode) -> ErrorCategory:
"""Infer error category from error code."""
code_str = error_code.value
if code_str.startswith("CONFIG_"):
return ErrorCategory.CONFIGURATION
elif code_str.startswith("PLUGIN_"):
return ErrorCategory.PLUGIN
elif code_str.startswith("VALIDATION_") or code_str.startswith("SCHEMA_") or code_str == "INVALID_INPUT":
return ErrorCategory.VALIDATION
elif code_str.startswith("NETWORK_") or code_str == "API_ERROR" or code_str == "TIMEOUT":
return ErrorCategory.NETWORK
elif code_str.startswith("PERMISSION_") or code_str == "FILE_PERMISSION_ERROR":
return ErrorCategory.PERMISSION
elif code_str.startswith("SYSTEM_") or code_str == "SERVICE_UNAVAILABLE":
return ErrorCategory.SYSTEM
else:
return ErrorCategory.UNKNOWN
def _get_default_suggestions(self, error_code: ErrorCode) -> List[str]:
"""Get default suggested fixes for error code."""
suggestions_map = {
@@ -178,7 +145,6 @@ class WebInterfaceError:
result = {
"status": "error",
"error_code": self.error_code.value,
"error_category": self.category.value,
"message": self.message,
}
@@ -197,7 +163,7 @@ class WebInterfaceError:
def from_exception(
cls,
exception: Exception,
error_code: Optional[ErrorCode] = None,
error_code: ErrorCode,
context: Optional[Dict[str, Any]] = None
) -> 'WebInterfaceError':
"""
@@ -205,13 +171,9 @@ class WebInterfaceError:
Args:
exception: Exception to convert
error_code: Optional specific error code
error_code: The error code to report
context: Optional additional context
"""
# Infer error code from exception type if not provided
if not error_code:
error_code = cls._infer_error_code(exception)
# Build context
error_context = context or {}
error_context['exception_type'] = type(exception).__name__
@@ -252,26 +214,6 @@ class WebInterfaceError:
}
return messages.get(error_code, "An unexpected error occurred")
@classmethod
def _infer_error_code(cls, exception: Exception) -> ErrorCode:
"""Infer error code from exception type."""
exception_name = type(exception).__name__
if "Config" in exception_name:
return ErrorCode.CONFIG_LOAD_FAILED
elif "Plugin" in exception_name:
return ErrorCode.PLUGIN_LOAD_FAILED
elif "Permission" in exception_name or "Access" in exception_name:
return ErrorCode.PERMISSION_DENIED
elif "Validation" in exception_name or "Schema" in exception_name:
return ErrorCode.VALIDATION_ERROR
elif "Network" in exception_name or "Connection" in exception_name:
return ErrorCode.NETWORK_ERROR
elif "Timeout" in exception_name:
return ErrorCode.TIMEOUT
else:
return ErrorCode.UNKNOWN_ERROR
@classmethod
def _get_exception_details(cls, exception: Exception) -> Optional[str]:
"""Get additional details from exception."""
test/test_auto_update_verify.py
+2 -1
View File
@@ -320,5 +320,6 @@ def test_units_installers_and_updater_agree():
assert (f'systemd/{unit}', f'/etc/systemd/system/{unit}') in StartupValidator._UNITS
# Triggering takes no privilege any more; no sudoers rule should linger.
for sudoers in ('scripts/install/configure_web_sudo.sh', 'first_time_install.sh'):
for sudoers in ('scripts/install/configure_web_sudo.sh', 'first_time_install.sh',
'scripts/install/lib_sudoers.sh'):
assert not re.search(r'NOPASSWD:.*update-verify', (ROOT / sudoers).read_text(encoding='utf-8')), sudoers
test/test_bdf_font.py
+283
View File
@@ -0,0 +1,283 @@
"""The one BDF loader and the one BDF rasterizer (src/common/bdf_font.py).
DisplayManager, the plugin test harness (VisualTestDisplayManager), FontManager
and element_style used to carry their own copies of both. Plugin golden images
depend on the exact pixels, so the rasterizer is checked against a frozen copy
of the per-pixel loop DisplayManager._draw_bdf_text ran before it was shared
(``_reference_draw`` below) for every bundled BDF font, at native and off-strike
sizes, clipped and unclipped. Pixels are compared as raw bytes, never PNG
hashes, so a Pillow upgrade can't fake or mask a difference.
"""
import os
import sys
import types
from pathlib import Path
import freetype
import pytest
from PIL import Image, ImageDraw
os.environ.setdefault("EMULATOR", "true")
from src.common import bdf_font
from src.common.bdf_font import draw_bdf_text, load_bdf_face, read_bdf_native_size
FONTS_DIR = Path(__file__).resolve().parent.parent / "assets" / "fonts"
BDF_FONTS = sorted(p.name for p in FONTS_DIR.glob("*.bdf"))
STRINGS = [
"Hello, World!", "0123456789", "12:34 PM", "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~",
"AaBbGgJjQqYy|", "", " ", "café üñ 72°F —€",
]
def _reference_draw(draw, width, height, text, x, y, face, color):
"""DisplayManager._draw_bdf_text as it was before the shared rasterizer.
Frozen on purpose: it is the definition of the panel's output that the
fast path must reproduce. Do not "fix" it.
"""
try:
ascender_px = face.size.ascender >> 6
except Exception:
ascender_px = 0
baseline_y = y + ascender_px
for char in text:
face.load_char(char)
bitmap = face.glyph.bitmap
glyph_left = face.glyph.bitmap_left
glyph_top = face.glyph.bitmap_top
for i in range(bitmap.rows):
for j in range(bitmap.width):
byte_index = i * bitmap.pitch + (j // 8)
if byte_index < len(bitmap.buffer):
byte = bitmap.buffer[byte_index]
if byte & (1 << (7 - (j % 8))):
pixel_x = x + glyph_left + j
pixel_y = baseline_y - glyph_top + i
if 0 <= pixel_x < width and 0 <= pixel_y < height:
draw.point((pixel_x, pixel_y), fill=color)
x += face.glyph.advance.x >> 6
def _pair(size=(64, 32), mode="RGB", draw_mode=None):
a = Image.new(mode, size)
b = Image.new(mode, size)
return a, ImageDraw.Draw(a, draw_mode), b, ImageDraw.Draw(b, draw_mode)
def _assert_same(expected, actual, what):
assert expected.tobytes() == actual.tobytes(), what
def _sizes(name):
native = read_bdf_native_size(str(FONTS_DIR / name))
return sorted({native, native + 3, max(1, native - 2)})
# ---------------------------------------------------------------- rasterizer
@pytest.mark.parametrize("name", BDF_FONTS)
def test_every_bundled_font_matches_the_reference_raster(name):
path = str(FONTS_DIR / name)
w, h = 96, 24
cases = [((0, 0), (255, 255, 255), text) for text in STRINGS]
# Clipped on every edge, in a colour that isn't all-or-nothing per channel.
cases += [(xy, (12, 200, 77), text)
for xy in ((-4, -3), (w - 11, h - 5), (w // 2, -9))
for text in STRINGS[:2]]
for size in _sizes(name):
face, _ = load_bdf_face(path, size)
for (x, y), color, text in cases:
ref, rdraw, new, ndraw = _pair((w, h))
_reference_draw(rdraw, w, h, text, x, y, face, color)
draw_bdf_text(ndraw, text, x, y, face, color)
_assert_same(ref, new, (name, size, x, y, color, text))
def test_returns_the_pen_position():
face, _ = load_bdf_face(str(FONTS_DIR / "5x7.bdf"), 7)
_, _, _, draw = _pair()
assert draw_bdf_text(draw, "", 3, 0, face) == 3
assert draw_bdf_text(draw, "12:34", 3, 0, face) == 3 + 5 * 5
def test_clip_smaller_than_the_canvas_matches_the_reference():
# DisplayManager clips to its logical size, which is the canvas size in
# practice but not by construction; honour the clip as the loop did.
face, _ = load_bdf_face(str(FONTS_DIR / "6x10.bdf"), 10)
for clip in ((20, 7), (1, 1), (0, 0), (200, 200)):
ref, rdraw, new, ndraw = _pair()
_reference_draw(rdraw, clip[0], clip[1], "Mixed 123", -2, -1, face, (1, 2, 3))
draw_bdf_text(ndraw, "Mixed 123", -2, -1, face, (1, 2, 3), clip=clip)
_assert_same(ref, new, clip)
def test_blending_draw_blends_exactly_like_the_reference():
# ImageDraw.Draw(rgb, "RGBA") blends a translucent colour; a mask fill
# would not, so this path must fall back to points.
face, _ = load_bdf_face(str(FONTS_DIR / "7x13B.bdf"), 13)
ref, rdraw, new, ndraw = _pair(draw_mode="RGBA")
for img in (ref, new):
img.paste((40, 80, 120), (0, 0, *img.size))
_reference_draw(rdraw, 64, 32, "Blend", 1, 1, face, (255, 0, 0, 128))
draw_bdf_text(ndraw, "Blend", 1, 1, face, (255, 0, 0, 128))
_assert_same(ref, new, "blend")
colours = {c for _, c in new.getcolors()}
assert (255, 0, 0) not in colours and len(colours) == 2, colours # blended
@pytest.mark.parametrize("mode,color", [("L", 200), ("P", (255, 0, 0)), ("RGBA", (9, 8, 7, 255)), ("1", 1)])
def test_other_canvas_modes_match_the_reference(mode, color):
face, _ = load_bdf_face(str(FONTS_DIR / "5x8.bdf"), 8)
ref, rdraw, new, ndraw = _pair(mode=mode)
_reference_draw(rdraw, 64, 32, "Mode 42", 2, 2, face, color)
draw_bdf_text(ndraw, "Mode 42", 2, 2, face, color)
_assert_same(ref, new, mode)
def test_a_non_mono_face_reads_the_same_bits_as_the_reference():
# A plugin can hand draw_text a freetype.Face of a TTF: 8-bit gray glyphs
# whose pitch is not ceil(width/8). The loop read them as packed bits, and
# so must the fast path -- same (odd) pixels, not "better" ones.
face = freetype.Face(str(FONTS_DIR / "PressStart2P-Regular.ttf"))
face.set_char_size(8 * 64, 8 * 64, 72, 72)
ref, rdraw, new, ndraw = _pair()
_reference_draw(rdraw, 64, 32, "Gray", 0, 0, face, (255, 255, 255))
draw_bdf_text(ndraw, "Gray", 0, 0, face, (255, 255, 255))
_assert_same(ref, new, "gray face")
def test_errors_propagate_after_earlier_glyphs_are_drawn():
face, _ = load_bdf_face(str(FONTS_DIR / "5x7.bdf"), 7)
_, _, img, draw = _pair()
with pytest.raises(Exception):
draw_bdf_text(draw, "A", 0, 0, face, "not-a-colour")
# Blank glyphs never touch the colour, as before.
draw_bdf_text(draw, " ", 0, 0, face, "not-a-colour")
assert img.getbbox() is None
with pytest.raises(Exception):
draw_bdf_text(draw, "A", 0, 0, object())
# ------------------------------------------------------------------- callers
def _dm_stub(img):
from src.display_manager import DisplayManager
stub = types.SimpleNamespace(width=img.width, height=img.height,
draw=ImageDraw.Draw(img), calendar_font=None)
return DisplayManager, stub
@pytest.mark.parametrize("name", ["5x7.bdf", "tom-thumb.bdf", "9x18B.bdf", "MatrixChunky8X.bdf"])
def test_display_manager_and_test_harness_draw_the_reference_pixels(name):
from src.plugin_system.testing.visual_display_manager import VisualTestDisplayManager
face, _ = load_bdf_face(str(FONTS_DIR / name), 20) # off-strike for all four
for text in STRINGS:
ref, rdraw, dm_img, _ = _pair((64, 32))
_reference_draw(rdraw, 64, 32, text, -1, 3, face, (255, 128, 0))
DisplayManager, stub = _dm_stub(dm_img)
DisplayManager._draw_bdf_text(stub, text, -1, 3, (255, 128, 0), face)
_assert_same(ref, dm_img, ("DisplayManager", name, text))
vt = VisualTestDisplayManager(64, 32)
vt.draw_text(text, -1, 3, (255, 128, 0), font=face)
_assert_same(ref, vt.image, ("VisualTestDisplayManager", name, text))
def test_harness_calendar_font_is_the_panels():
# The harness built a bare freetype.Face with no size: ascender 0, so
# every calendar_font line drew a baseline too high, and its
# get_font_height() returned 0.
from src.plugin_system.testing.visual_display_manager import VisualTestDisplayManager
vt = VisualTestDisplayManager(64, 32)
panel_face, _ = load_bdf_face(str(FONTS_DIR / "5x7.bdf"), 7)
assert vt.calendar_font is panel_face
assert vt.get_font_height(vt.calendar_font) == panel_face.size.height >> 6 > 0
# -------------------------------------------------------------------- loader
def test_off_strike_size_loads_the_native_strike():
path = str(FONTS_DIR / "5x7.bdf")
face, realised = load_bdf_face(path, 10)
assert isinstance(face, freetype.Face)
assert realised == 7 and face.size.y_ppem == 7
assert load_bdf_face(path, 7)[1] == 7
def test_faces_are_cached_and_shared_by_every_loader():
from src.element_style import _load_bdf
from src.font_manager import FontManager
path = str(FONTS_DIR / "6x10.bdf")
face, realised = load_bdf_face(path, 12)
assert load_bdf_face(path, 12)[0] is face
assert _load_bdf(path, 12) == (face, realised)
assert FontManager({})._load_bdf_font(path, 12) is face
def test_touched_file_is_reloaded(tmp_path):
# The cache key carries the file's mtime, so a font re-uploaded under the
# same name is not served from a stale face.
target = tmp_path / "f.bdf"
target.write_bytes((FONTS_DIR / "5x7.bdf").read_bytes())
first, _ = load_bdf_face(str(target), 7)
assert load_bdf_face(str(target), 7)[0] is first
os.utime(target, ns=(10**9, 10**9))
assert load_bdf_face(str(target), 7)[0] is not first
@pytest.mark.skipif(sys.platform == "win32",
reason="FreeType holds the font file open; Windows refuses to overwrite it")
def test_replaced_file_is_reloaded(tmp_path):
target = tmp_path / "f.bdf"
target.write_bytes((FONTS_DIR / "5x7.bdf").read_bytes())
first, _ = load_bdf_face(str(target), 7)
target.write_bytes((FONTS_DIR / "6x10.bdf").read_bytes())
os.utime(target, ns=(1, 1))
second, realised = load_bdf_face(str(target), 7)
assert second is not first and realised == 10
def test_unloadable_file_raises(tmp_path):
with pytest.raises(Exception):
load_bdf_face(str(tmp_path / "missing.bdf"), 7)
bad = tmp_path / "bad.bdf"
bad.write_text("not a font")
with pytest.raises(Exception):
load_bdf_face(str(bad), 7)
def test_cache_is_bounded(monkeypatch):
monkeypatch.setattr(bdf_font, "_FACE_CACHE_MAX", 2)
bdf_font.clear_face_cache()
path = str(FONTS_DIR / "5x7.bdf")
for size in (7, 8, 9):
load_bdf_face(path, size)
assert len(bdf_font._face_cache) == 2
bdf_font.clear_face_cache()
def test_each_thread_gets_its_own_face():
# FreeType forbids two threads using one face at once: load_char rewrites
# the face's glyph slot. Within a thread the face is shared.
import threading
path = str(FONTS_DIR / "5x7.bdf")
here, _ = load_bdf_face(path, 7)
assert load_bdf_face(path, 7)[0] is here
other = []
worker = threading.Thread(target=lambda: other.append(load_bdf_face(path, 7)[0]))
worker.start()
worker.join()
assert other and other[0] is not here
def test_native_size_prefers_pixel_size_over_point_size():
# 6x13.bdf is defined at 75dpi: SIZE says 12 (points), PIXEL_SIZE 13.
assert read_bdf_native_size(str(FONTS_DIR / "6x13.bdf")) == 13
assert read_bdf_native_size(str(FONTS_DIR / "nope.bdf")) is None
test/test_cache_stale_header.py
+118
View File
@@ -0,0 +1,118 @@
"""A stale cache record is recognised from its header, without parsing it.
The sports plugins cache whole season schedules -- 53MB for MLB, 18MB for NHL.
When one expired, DiskCache.get parsed all of it (~1.8s of orjson.loads on a
Pi 4, GIL held, the whole display frozen) only to find the timestamp too old
and throw the result away. CacheManager.set now writes timestamp and ttl ahead
of the data, and DiskCache.get reads them from the first bytes of the file.
"""
import json
import time
from types import SimpleNamespace
import pytest
from src.cache import disk_cache as disk_cache_module
from src.cache.disk_cache import DiskCache, _stale_from_head
from src.common import json_body
@pytest.fixture
def disk(tmp_path):
return DiskCache(cache_dir=str(tmp_path))
@pytest.fixture
def parses(monkeypatch):
"""Count full parses of cache files."""
calls = []
real = disk_cache_module._loads
def counting(raw):
calls.append(len(raw))
return real(raw)
monkeypatch.setattr(disk_cache_module, "_loads", counting)
return calls
def _header_first(age=0.0, ttl=None, events=100):
record = {"timestamp": time.time() - age}
if ttl is not None:
record["ttl"] = ttl
record["data"] = {"events": [{"id": n, "name": "x" * 50} for n in range(events)]}
return record
def test_cache_manager_writes_the_header_first(monkeypatch):
from src.cache_manager import CacheManager
written = {}
manager = CacheManager.__new__(CacheManager)
monkeypatch.setattr(manager, "save_cache",
lambda key, record: written.update({key: record}),
raising=False)
CacheManager.set(manager, "k", {"events": []}, ttl=60)
assert list(written["k"]) == ["timestamp", "ttl", "data"]
CacheManager.set(manager, "k", {"events": []})
assert list(written["k"]) == ["timestamp", "data"]
def test_a_stale_record_is_not_parsed(disk, parses):
disk.set("season", _header_first(age=600))
assert disk.get("season", max_age=300) is None
assert parses == []
def test_a_fresh_record_is_parsed_and_returned(disk, parses):
disk.set("season", _header_first(age=10))
record = disk.get("season", max_age=300)
assert record["data"]["events"][0]["id"] == 0
assert len(parses) == 1
def test_the_entry_ttl_wins_over_max_age(disk, parses):
disk.set("long", _header_first(age=600, ttl=3600))
assert disk.get("long", max_age=300) is not None # ttl says fresh
disk.set("short", _header_first(age=60, ttl=30))
parses.clear()
assert disk.get("short", max_age=300) is None # ttl says stale
assert parses == []
def test_no_limit_means_never_stale(disk):
disk.set("forever", _header_first(age=10 ** 7))
assert disk.get("forever", max_age=None) is not None
def test_older_files_with_data_first_still_work(disk, parses):
# Records written before the header moved: parsed in full, as before.
disk.set("legacy_fresh", {"data": {"v": 1}, "timestamp": time.time()})
disk.set("legacy_stale", {"data": {"v": 1}, "timestamp": time.time() - 600})
assert disk.get("legacy_fresh", max_age=300)["data"] == {"v": 1}
assert disk.get("legacy_stale", max_age=300) is None
assert len(parses) == 2
@pytest.mark.parametrize("head, stale", [
(b'{"timestamp":100.0,"data":{}}', True),
(b'{"timestamp": 100.0, "ttl": 1000, "data": {}}', False), # stdlib spacing
(b'{"timestamp":1e2,"ttl":5,"data":1}', True),
(b'{"timestamp":100.0}', True),
(b'{"data":{},"timestamp":100.0}', False), # unknown layout
(b'{"timestamp":"100.0","data":{}}', False), # string: parse it
(b'', False),
])
def test_reading_the_header(head, stale):
assert _stale_from_head(head, 300, now=1000.0) is stale
def test_response_json_prefers_orjson_and_falls_back():
payload = {"events": [1, 2, 3]}
response = SimpleNamespace(content=json.dumps(payload).encode(),
json=lambda: pytest.fail("used the slow path"))
if json_body.orjson is None:
pytest.skip("orjson not installed")
assert json_body.response_json(response) == payload
# A response object without bytes content (a test double) still works.
assert json_body.response_json(SimpleNamespace(json=lambda: payload)) == payload
test/test_discovery_path_contract.py
+19 -13
View File
@@ -16,9 +16,13 @@ change to the fallback chains is a deliberate one.
The `.standalone-backup-` contract: store_manager renames a plugin dir aside
with that substring during install/rollback; discovery MUST skip such dirs
or a half-finished install would surface a ghost plugin. The substring is
duplicated as a literal in both files — this test breaks if either side
changes it unilaterally.
or a half-finished install would surface a ghost plugin. The substring now
lives once, as plugin_dirs.BACKUP_MARKER, which both sides import; its value
is pinned because debris already on devices carries exactly that text.
All of them now resolve through src/plugin_system/plugin_dirs.py; the
per-caller differences pinned here are explicit arguments there. The rules
themselves are covered table-style in test_plugin_dirs.py.
"""
import json
@@ -198,14 +202,16 @@ class TestStandaloneBackupContract:
found = _scanner()._scan_directory_for_plugins(plugins_dir)
assert found == ["real-plugin"]
def test_backup_substring_literal_matches_across_files(self):
"""The substring is duplicated in plugin_manager (skip check) and
store_manager (rename-aside names). If either side changes it, the
other silently stops honoring the contract — this test is the
tripwire."""
def test_backup_marker_is_shared_and_unchanged(self):
"""store_manager (rename-aside names) and every lookup (skip check)
must agree on the marker. Both now import one constant; the value is
pinned because renaming it would make existing debris on devices
visible as plugins again."""
from src.plugin_system import plugin_dirs
assert plugin_dirs.BACKUP_MARKER == '.standalone-backup-'
root = Path(__file__).resolve().parents[1]
pm_text = (root / "src/plugin_system/plugin_manager.py").read_text()
sm_text = (root / "src/plugin_system/store_manager.py").read_text()
assert "'.standalone-backup-'" in pm_text.replace('"', "'")
assert ".standalone-backup-" in sm_text
sm_text = (root / "src/plugin_system/store_manager.py").read_text(encoding="utf-8")
assert "{BACKUP_MARKER}preinstall" in sm_text
assert "{BACKUP_MARKER}migrating" in sm_text
assert plugin_dirs.is_ignored_dir_name(
"demo" + plugin_dirs.BACKUP_MARKER + "preinstall")
test/test_plugin_dirs.py
+343
View File
@@ -0,0 +1,343 @@
"""
Every "which directory holds plugin X?" answer, from one tree, per caller.
src/plugin_system/plugin_dirs.py holds the rules; the callers differ only in
explicit arguments (search dirs, ``ledmatrix-`` prefix, case folding, whether
the manifest pass runs). This file builds one project tree that exercises
every rule and pins each caller's answer for each id, so a change to either
the shared rules or a caller's arguments shows up as a table row.
Callers:
discovery PluginManager._scan_directory_for_plugins -> plugin_directories
pm_get PluginManager.get_plugin_directory before discovery has run
loader PluginLoader.find_plugin_directory (no discovery mapping)
store PluginStoreManager._find_plugin_path
"""
import json
import logging
import os
import sys
import threading
from pathlib import Path
import pytest
from src.plugin_system import plugin_dirs
from src.plugin_system.plugin_dirs import (
ManifestStatus, PluginDirectoryIndex, resolve_plugin_dir,
)
from src.plugin_system.plugin_loader import PluginLoader
from src.plugin_system.plugin_manager import PluginManager
from src.plugin_system.state_reconciliation import (
StateReconciliation, disk_plugin_ids,
)
from src.plugin_system.store_manager import PluginStoreManager
CONFIGURED = "plugin-repos"
SIBLING = "plugins"
def _write(base: Path, dir_name: str, manifest) -> Path:
d = base / dir_name
d.mkdir(parents=True)
if manifest is not None:
text = manifest if isinstance(manifest, str) else json.dumps(manifest)
(d / "manifest.json").write_text(text, encoding="utf-8")
return d
def _plugin(base: Path, dir_name: str, plugin_id: str, **extra) -> Path:
return _write(base, dir_name, dict({"id": plugin_id, "name": plugin_id,
"version": "1.0.0"}, **extra))
def _link_dir(link: Path, target: Path) -> None:
"""A symlink where the OS allows one; on Windows without the privilege,
a directory junction, which the code under test sees the same way
(is_dir() follows it, iterdir() lists it under the link's name)."""
try:
os.symlink(target, link, target_is_directory=True)
except OSError:
if sys.platform != "win32":
raise
import _winapi
_winapi.CreateJunction(str(target), str(link))
@pytest.fixture
def tree(tmp_path):
repos = tmp_path / CONFIGURED
legacy = tmp_path / SIBLING
repos.mkdir()
legacy.mkdir()
# configured dir (plugin-repos/)
_plugin(repos, "exact", "exact") # id == dir name
_plugin(repos, "ledmatrix-stocks", "stocks") # manifest id != dir name
_plugin(repos, "ledmatrix-legacy", "ledmatrix-legacy") # prefix only by name
_plugin(repos, "MixedCase", "MixedCase") # case differences
_plugin(repos, "renamed-dir", "other-id") # dir name belongs to no id
_plugin(repos, "shadow", "not-shadow") # name says one id ...
_plugin(repos, "real-shadow", "shadow") # ... manifest says it's here
_plugin(repos, "ghost.standalone-backup-preinstall", "ghost") # set aside
_plugin(repos, "exact.standalone-backup-migrating", "exact") # set aside, dup id
_plugin(repos, ".hidden", "hidden") # hidden / staging
_plugin(repos, "zz-dupe", "dupe") # duplicate ids:
_plugin(repos, "ledmatrix-dupe", "dupe") # prefix beats other,
_plugin(repos, "dupe", "dupe") # exact beats prefix
_write(repos, "broken", "{ not json") # unreadable manifest
_write(repos, "noid", {"name": "No id"}) # parses, no id
_write(repos, "listy", [1, 2]) # parses, not an object
_write(repos, "nomanifest", None) # not a plugin
_plugin(repos, "both", "both") # also in plugins/
_plugin(repos, "ledmatrix-weather", "weather") # manifest hit here vs
(repos / "README.md").write_text("not a dir") # a file, never a match
dev_target = _plugin(tmp_path / "dev-checkouts", "devplug-src", "devplug")
_link_dir(repos / "dev-link", dev_target) # symlinked dev plugin
# sibling dir (plugins/): store fallback only
_plugin(legacy, "both", "both")
_plugin(legacy, "legacy-only", "legacy-only")
_plugin(legacy, "ledmatrix-sibling", "sibling")
_plugin(legacy, "weather", "weather") # ... a name hit here
return tmp_path
def _discovery(root: Path) -> PluginManager:
pm = object.__new__(PluginManager)
pm.logger = logging.getLogger("test_plugin_dirs")
pm._discovery_lock = threading.RLock()
pm._skip_reported = set()
pm.plugin_manifests = {}
pm.plugin_directories = {}
pm.plugins_dir = root / CONFIGURED
return pm
def _answers(root: Path, plugin_id: str) -> dict:
repos = root / CONFIGURED
discovered = _discovery(root)
discovered._scan_directory_for_plugins(repos)
fresh = _discovery(root) # discovery not run: exercises the disk rules
store = PluginStoreManager(plugins_dir=str(repos),
uninstalled_registry_path=str(root / "u.json"))
def rel(p):
return None if p is None else Path(p).relative_to(root).as_posix()
return {
"discovery": rel(discovered.plugin_directories.get(plugin_id)),
"pm_get": rel(fresh.get_plugin_directory(plugin_id)),
"loader": rel(PluginLoader().find_plugin_directory(plugin_id, repos)),
"store": rel(store._find_plugin_path(plugin_id)),
}
R = CONFIGURED + "/"
S = SIBLING + "/"
# id discovery pm_get loader store
TABLE = [
("exact", R + "exact", R + "exact", R + "exact", R + "exact"),
# manifest id != dir name: the manifest finds it; pm_get by prefix
("stocks", R + "ledmatrix-stocks", R + "ledmatrix-stocks",
R + "ledmatrix-stocks", R + "ledmatrix-stocks"),
# ledmatrix- prefix: name-only callers with prefix=True; the store has none
("legacy", None, R + "ledmatrix-legacy", R + "ledmatrix-legacy", None),
("ledmatrix-legacy", R + "ledmatrix-legacy", R + "ledmatrix-legacy",
R + "ledmatrix-legacy", R + "ledmatrix-legacy"),
# case: only the loader folds case
("mixedcase", None, None, R + "MixedCase", None),
("MixedCase", R + "MixedCase", R + "MixedCase", R + "MixedCase", R + "MixedCase"),
# a directory name no manifest claims still resolves by name
("renamed-dir", None, R + "renamed-dir", R + "renamed-dir", R + "renamed-dir"),
("other-id", R + "renamed-dir", None, R + "renamed-dir", R + "renamed-dir"),
# manifest id wins over directory name (pm_get has no manifest pass)
("shadow", R + "real-shadow", R + "shadow", R + "real-shadow", R + "real-shadow"),
# backups and hidden dirs are never plugins, by id or by name
("ghost", None, None, None, None),
("ghost.standalone-backup-preinstall", None, None, None, None),
("hidden", None, None, None, None),
(".hidden", None, None, None, None),
# duplicate ids: exact name, then ledmatrix-<id>, then by name
("dupe", R + "dupe", R + "dupe", R + "dupe", R + "dupe"),
# unreadable manifest: found by name so it can be repaired/removed
("broken", None, R + "broken", R + "broken", R + "broken"),
("noid", None, R + "noid", R + "noid", R + "noid"),
("nomanifest", None, R + "nomanifest", R + "nomanifest", R + "nomanifest"),
("README.md", None, None, None, None),
# symlinked dev plugin: found through the link, path kept inside the dir
("devplug", R + "dev-link", None, R + "dev-link", R + "dev-link"),
("dev-link", None, R + "dev-link", R + "dev-link", R + "dev-link"),
# search order: only the store looks in plugins/, and configured first
("both", R + "both", R + "both", R + "both", R + "both"),
("legacy-only", None, None, None, S + "legacy-only"),
("sibling", None, None, None, S + "ledmatrix-sibling"),
# configured dir searched completely (manifest hit) before plugins/ (name hit)
("weather", R + "ledmatrix-weather", R + "ledmatrix-weather",
R + "ledmatrix-weather", R + "ledmatrix-weather"),
# not one plain path segment: nothing, never a join or a truncation
("../plugins/both", None, None, None, None),
("plugin-repos/exact", None, None, None, None),
("", None, None, None, None),
]
@pytest.mark.parametrize("plugin_id,discovery,pm_get,loader,store", TABLE,
ids=[row[0] or "<empty>" for row in TABLE])
def test_each_caller_resolves_each_id(tree, plugin_id, discovery, pm_get, loader, store):
assert _answers(tree, plugin_id) == {
"discovery": discovery, "pm_get": pm_get, "loader": loader, "store": store,
}
class TestListings:
def test_discovery_registers_manifest_ids_only(self, tree):
pm = _discovery(tree)
found = pm._scan_directory_for_plugins(tree / CONFIGURED)
assert sorted(found) == sorted([
"exact", "stocks", "ledmatrix-legacy", "MixedCase", "other-id",
"not-shadow", "shadow", "dupe", "both", "weather", "devplug",
])
assert len(found) == len(set(found)), "a duplicate id was listed twice"
assert set(pm.plugin_manifests) == set(found)
def test_store_lists_every_dir_with_a_manifest(self, tree):
store = PluginStoreManager(plugins_dir=str(tree / CONFIGURED),
uninstalled_registry_path=str(tree / "u.json"))
assert store.list_installed_plugins() == sorted([
"exact", "stocks", "ledmatrix-legacy", "MixedCase", "other-id",
"not-shadow", "shadow", "dupe", "both", "weather", "devplug",
# manifest present but no usable id: listed by directory name
"broken", "noid", "listy",
])
def test_reconciliation_counts_parseable_manifests(self, tree):
assert disk_plugin_ids(tree / CONFIGURED) == {
"exact", "stocks", "ledmatrix-legacy", "MixedCase", "other-id",
"not-shadow", "shadow", "dupe", "both", "weather", "devplug",
"noid", "listy",
}
def test_reconciliation_disk_state_is_keyed_like_config(self, tree):
recon = object.__new__(StateReconciliation)
recon.plugins_dir = tree / CONFIGURED
recon.logger = logging.getLogger("test_plugin_dirs")
state = recon._get_disk_state()
assert state["stocks"] == {"exists_on_disk": True, "version": "1.0.0",
"name": "stocks"}
assert "ledmatrix-stocks" not in state
# A manifest that is valid JSON but not an object used to abort the
# whole disk state with AttributeError.
assert state["listy"] == {"exists_on_disk": True, "version": None, "name": None}
def test_all_listings_skip_backups_and_hidden(self, tree):
store = PluginStoreManager(plugins_dir=str(tree / CONFIGURED),
uninstalled_registry_path=str(tree / "u.json"))
pm = _discovery(tree)
listings = {
"discovery": set(pm._scan_directory_for_plugins(tree / CONFIGURED)),
"store": set(store.list_installed_plugins()),
"reconciliation": disk_plugin_ids(tree / CONFIGURED),
}
for name, ids in listings.items():
assert not {"ghost", "hidden", ".hidden"} & ids, name
assert not any(plugin_dirs.BACKUP_MARKER in i for i in ids), name
# the backup of `exact` did not replace the live one
assert pm.plugin_directories["exact"] == tree / CONFIGURED / "exact"
class TestIndex:
def test_each_manifest_is_read_once_per_scan(self, tree, monkeypatch):
reads = []
real = plugin_dirs._read_entry
monkeypatch.setattr(plugin_dirs, "_read_entry",
lambda p: reads.append(p.name) or real(p))
index = PluginDirectoryIndex.scan(tree / CONFIGURED)
for plugin_id in ("exact", "stocks", "dupe", "shadow", "nope"):
index.find(plugin_id, prefix=True, case_insensitive=True)
index.plugins()
index.installed_ids(require_parseable_manifest=True)
assert len(reads) == len(set(reads)) == len(index.entries)
def test_manifest_statuses(self, tree):
index = PluginDirectoryIndex.scan(tree / CONFIGURED)
status = {e.name: e.status for e in index.entries}
assert status["exact"] == ManifestStatus.OK
assert status["broken"] == ManifestStatus.UNREADABLE
assert status["noid"] == ManifestStatus.NO_ID
assert status["listy"] == ManifestStatus.NOT_OBJECT
assert status["nomanifest"] == ManifestStatus.MISSING
assert "README.md" not in status
def test_duplicates_are_reported_with_every_claimant(self, tree):
dupes = PluginDirectoryIndex.scan(tree / CONFIGURED).duplicates()
assert sorted(e.name for e in dupes["dupe"]) == \
["dupe", "ledmatrix-dupe", "zz-dupe"]
# the backup of `exact` is not a claimant
assert "exact" not in dupes
def test_duplicate_preference_without_an_exact_name(self, tmp_path):
_plugin(tmp_path, "zz-dupe", "dupe")
_plugin(tmp_path, "aa-dupe", "dupe")
_plugin(tmp_path, "ledmatrix-dupe", "dupe")
assert resolve_plugin_dir("dupe", [tmp_path], prefix=False) == \
tmp_path / "ledmatrix-dupe"
(tmp_path / "ledmatrix-dupe" / "manifest.json").unlink()
assert resolve_plugin_dir("dupe", [tmp_path], prefix=False) == \
tmp_path / "aa-dupe"
def test_exact_name_beats_prefix_even_when_it_sorts_later(self, tmp_path):
_plugin(tmp_path, "ledmatrix-zeta", "zeta")
_plugin(tmp_path, "zeta", "zeta")
index = PluginDirectoryIndex.scan(tmp_path)
assert index.plugins()["zeta"].name == "zeta"
assert resolve_plugin_dir("zeta", [tmp_path], prefix=True) == tmp_path / "zeta"
@pytest.mark.parametrize("bad", [None, 5, b"exact", ["exact"]])
def test_non_string_ids_resolve_to_nothing(self, tree, bad):
for kwargs in ({"prefix": True}, {"prefix": True, "by_manifest": False},
{"prefix": False, "case_insensitive": True}):
assert resolve_plugin_dir(bad, [tree / CONFIGURED], **kwargs) is None
def test_missing_search_dir_is_empty_not_an_error(self, tmp_path):
index = PluginDirectoryIndex.scan(tmp_path / "absent")
assert index.entries == [] and index.error is None
assert resolve_plugin_dir("x", [tmp_path / "absent"], prefix=True) is None
def test_discovery_warns_once_about_a_duplicate(self, tree, caplog):
pm = _discovery(tree)
with caplog.at_level(logging.WARNING, logger="test_plugin_dirs"):
for _ in range(3):
pm._scan_directory_for_plugins(tree / CONFIGURED)
hits = [r for r in caplog.records if "'dupe'" in r.getMessage()]
assert len(hits) == 1
assert "zz-dupe" in hits[0].getMessage()
class TestAutoUpdateUsesManifestIds:
def test_update_targets_manifest_id_and_its_directory(self, tree, monkeypatch):
from web_interface import auto_update
store = PluginStoreManager(plugins_dir=str(tree / CONFIGURED),
uninstalled_registry_path=str(tree / "u.json"))
calls = []
def fake_update(plugin_id):
calls.append(plugin_id)
if plugin_id == "stocks":
path = tree / CONFIGURED / "ledmatrix-stocks" / "manifest.json"
m = json.loads(path.read_text(encoding="utf-8"))
m["version"] = "2.0.0"
path.write_text(json.dumps(m), encoding="utf-8")
return True
monkeypatch.setattr(store, "update_plugin", fake_update)
monkeypatch.setattr(store, "_get_local_git_info", lambda p: None)
updated, failed = auto_update.update_plugins(store)
assert "stocks" in calls and "ledmatrix-stocks" not in calls
assert not any(plugin_dirs.BACKUP_MARKER in c for c in calls)
# the version change was seen in ledmatrix-stocks/, not a missing stocks/
assert updated == ["stocks"] and failed == []
test/test_redaction.py
+110
View File
@@ -0,0 +1,110 @@
"""redact_credentials must stay linear in the length of its input.
Regressions under test, both quadratic regexes in src/redaction.py:
- The URL-userinfo pattern (`scheme://user:password@`) could start a match at
every letter of a run of scheme characters, and each attempt read to the end
of the run looking for `://`: 1.6s for a 20k-character run.
- The Authorization-header pattern had two `\\s*` separated only by an
optional quote, so a header followed by whitespace and no credential tried
every split of that whitespace between them: 8s for 20k spaces.
The display service redacts every message, stack trace and context value it
publishes in the error snapshot, and re.sub holds the GIL throughout, so an
exception quoting a hex digest or a long ID stalled the render loop with it.
test_error_snapshot_cross_process.py's snapshot-size test spent 140s here.
The fixed patterns have to redact exactly what the old ones did.
"""
import time
import pytest
from src.redaction import redact_credentials
# Each timed input took seconds before the fix and takes about a millisecond
# after it; the bound leaves CI plenty of headroom while still failing on a
# quadratic pattern.
_TIME_LIMIT = 1.0
def _timed(text):
start = time.perf_counter()
result = redact_credentials(text)
return result, time.perf_counter() - start
class TestUrlUserinfo:
@pytest.mark.parametrize("text,expected", [
("401 for https://user:hunter2@example.com/api",
"401 for https://user:<redacted>@example.com/api"),
("HTTPS://USER:HUNTER2@EXAMPLE.COM",
"HTTPS://USER:<redacted>@EXAMPLE.COM"),
("git+ssh://deploy:hunter2@host/repo",
"git+ssh://deploy:<redacted>@host/repo"),
# The scheme starts after digits or +.- in the same run. Those
# characters must survive, and the password must still go.
("1http://user:hunter2@host", "1http://user:<redacted>@host"),
("+.-http://user:hunter2@host", "+.-http://user:<redacted>@host"),
("a1+http://user:hunter2@host", "a1+http://user:<redacted>@host"),
("see a://u:first@b and c://v:second@d",
"see a://u:<redacted>@b and c://v:<redacted>@d"),
])
def test_password_is_redacted_and_the_rest_kept(self, text, expected):
assert redact_credentials(text) == expected
def test_a_url_without_a_password_is_untouched(self):
text = "GET https://user@example.com/path failed"
assert redact_credentials(text) == text
class TestAuthorizationHeader:
@pytest.mark.parametrize("text,expected", [
("Authorization: Bearer eyJ.SECRET.sig", "Authorization: Bearer <redacted>"),
("Proxy-Authorization: Basic dXNlcg==", "Proxy-Authorization: Basic <redacted>"),
("authorization: barecredential", "authorization: <redacted>"),
# Whitespace and an opening quote around the value, in either order.
('authorization=" Bearer tok"', 'authorization=" Bearer <redacted>"'),
("authorization: ' tok'", "authorization: ' <redacted>'"),
("authorization:\n\tBearer tok", "authorization:\n\tBearer <redacted>"),
])
def test_credential_is_redacted_and_the_rest_kept(self, text, expected):
assert redact_credentials(text) == expected
@pytest.mark.parametrize("text", ["authorization: ", "authorization: , next"])
def test_a_header_without_a_credential_is_untouched(self, text):
assert redact_credentials(text) == text
class TestLinearTime:
@pytest.mark.parametrize("unit", ["x", "0123456789abcdef", "1a", "a+", "1"])
def test_long_scheme_character_runs(self, unit):
text = (unit * 50_000)[:50_000]
result, elapsed = _timed(text)
assert result == text
assert elapsed < _TIME_LIMIT, f"{elapsed:.2f}s to redact {len(text)} chars of {unit!r}"
def test_a_credential_after_a_long_run_is_still_found(self):
run = "ab12" * 10_000
result, elapsed = _timed(f"{run} https://user:hunter2@example.com")
assert result == f"{run} https://user:<redacted>@example.com"
assert elapsed < _TIME_LIMIT
@pytest.mark.parametrize("header,whitespace", [
("authorization:", " "),
("Proxy-Authorization:", "\t"),
("authorization=", "\n"),
])
def test_a_header_followed_by_long_whitespace(self, header, whitespace):
text = header + whitespace * 20_000 + ","
result, elapsed = _timed(text)
assert result == text
assert elapsed < _TIME_LIMIT, (
f"{elapsed:.2f}s to redact {header!r} and {len(text) - len(header)} more chars")
def test_a_credential_after_long_whitespace_is_still_found(self):
gap = " " * 20_000
result, elapsed = _timed(f"authorization:{gap}Bearer tok")
assert result == f"authorization:{gap}Bearer <redacted>"
assert elapsed < _TIME_LIMIT
test/test_sports_twins.py
+672
View File
@@ -0,0 +1,672 @@
"""The twins: ``SportsCoreSharedMixin`` methods vs ``sports_card`` functions.
Every scoreboard draws the same game twice over: switch mode through its
``sports.py`` (``SportsCoreSharedMixin``, ``self._recent_score_color(...)``)
and scroll/Vegas mode through its ``game_renderer.py``
(``sports_card``, ``_card.recent_score_color(...)``). The two modules grew
same-named helpers independently, so this file calls each pair with the same
inputs and says which ones agree.
Two kinds of test live here, and the difference matters:
* ``TestIdentical`` -- pairs that agree on every input below. Most mixin
methods in this set are now thin wrappers over the ``sports_card`` function,
so the check is also what keeps a future "fix" to one side from quietly
becoming a divergence (a mixin body re-grown, a wrapper given different
arguments).
* ``TestPinnedDivergence`` -- pairs that do NOT agree. Their current behaviour
is pinned on purpose, with the minimal input that shows the difference. A
divergence here is user-visible (a colour, a weekday) in one display mode, and
which side is right is an owner decision, not a refactor. When that decision
is made, the test that pins it is the one to edit, deliberately.
The game corpus is the plugins' own harness fixtures
(``plugins/*/test/fixtures/mock.json`` in ledmatrix-plugins), reduced to the
keys these helpers read and embedded below, in the three payload shapes the
helpers are handed: flat (what ``_extract_game_details_common`` builds -- the
switch-mode input), flat plus nested (what the renderers'
``_normalize_game_payload`` hands the scroll card), and nested only. Point
``LEDMATRIX_PLUGINS`` at a ledmatrix-plugins checkout to add every event in
those fixtures to the corpus.
"""
import itertools
import json
import logging
import os
from datetime import datetime, timezone
from pathlib import Path
from zoneinfo import ZoneInfo
import pytest
from src.common import sports_card as C
from src.common.font_layout import load_truetype, resolve_asset_path
from src.common.sports_shared import SportsCoreSharedMixin
LOG = logging.getLogger("test_sports_twins")
# ---------------------------------------------------------------------------
# Corpus
# ---------------------------------------------------------------------------
#: (plugin, start, home abbr, home id, home score, away abbr, away id,
#: away score, state) -- one row per distinct event in the eight scoreboards'
#: test/fixtures/mock.json.
FIXTURE_EVENTS = [
("afl", "2026-07-10T09:40Z", "COLL", "17", "89", "NMFC", "5", "85", "post"),
("afl", "2026-07-11T03:15Z", "STK", "18", "0", "PORT", "7", "0", "pre"),
("afl", "2026-07-10T11:30Z", "FRE", "1", "54", "SYD", "4", "48", "in"),
("baseball", "2026-07-09T02:10Z", "LAD", "19", "5", "SF", "26", "3", "post"),
("baseball", "2026-07-10T10:05Z", "NYY", "10", "4", "BOS", "2", "3", "in"),
("baseball", "2026-07-11T23:10Z", "NYM", "21", "0", "ATL", "15", "0", "pre"),
("basketball", "2026-01-14T00:30Z", "BOS", "2", "112", "NY", "18", "104", "post"),
("basketball", "2026-01-15T03:30Z", "LAL", "13", "78", "GS", "9", "72", "in"),
("basketball", "2026-01-16T02:00Z", "DEN", "7", "0", "DAL", "6", "0", "pre"),
("football", "2026-01-14T01:15Z", "KC", "12", "27", "BUF", "2", "24", "post"),
("football", "2026-01-15T10:30Z", "PHI", "21", "17", "DAL", "6", "14", "in"),
("football", "2026-01-18T23:30Z", "DET", "8", "0", "GB", "9", "0", "pre"),
("hockey", "2026-01-14T00:00Z", "BOS", "1", "4", "TOR", "21", "2", "post"),
("hockey", "2026-01-15T10:30Z", "TB", "20", "3", "DAL", "9", "2", "in"),
("hockey", "2026-01-16T00:00Z", "CHI", "4", "0", "NYR", "13", "0", "pre"),
("lacrosse", "2026-04-14T18:00Z", "DUKE", "150", "14", "SYR", "183", "11", "post"),
("lacrosse", "2026-04-15T10:30Z", "JHU", "2305", "8", "UVA", "258", "7", "in"),
("lacrosse", "2026-04-16T22:00Z", "COR", "172", "0", "PSU", "213", "0", "pre"),
("nrl", "2026-07-10T09:00Z", "BRI", "16", "18", "PEN", "18", "12", "in"),
("nrl", "2026-07-09T09:00Z", "MEL", "12", "24", "SYD", "20", "10", "post"),
("nrl", "2026-07-12T09:00Z", "PAR", "14", "0", "PEN", "18", "0", "pre"),
("soccer", "2026-01-14T20:00Z", "ARS", "359", "2", "CHE", "363", "1", "post"),
("soccer", "2026-01-15T11:30Z", "LIV", "364", "1", "MNC", "382", "1", "in"),
("soccer", "2026-01-16T20:00Z", "TOT", "367", "0", "MAN", "360", "0", "pre"),
]
_LEAGUE = {"afl": "afl", "baseball": "mlb", "basketball": "nba", "football": "nfl",
"hockey": "nhl", "lacrosse": "ncaa_mens_lacrosse", "nrl": "nrl",
"soccer": "eng.1"}
def _extra_fixture_events():
"""Every event in a ledmatrix-plugins checkout, when one is named."""
raw = os.environ.get("LEDMATRIX_PLUGINS")
if not raw:
return []
root = Path(raw)
if (root / "plugins").is_dir():
root = root / "plugins"
rows = []
for path in sorted(root.glob("*-scoreboard/test/fixtures/mock.json")):
plugin = path.parts[-4].replace("-scoreboard", "")
try:
data = json.loads(path.read_text(encoding="utf-8"))
except (OSError, ValueError):
continue
for block in data.values():
for ev in (block.get("events") or []) if isinstance(block, dict) else []:
try:
comp = ev["competitions"][0]
sides = {c["homeAway"]: c for c in comp["competitors"]}
rows.append((plugin, ev["date"],
sides["home"]["team"]["abbreviation"],
sides["home"]["team"]["id"], sides["home"].get("score"),
sides["away"]["team"]["abbreviation"],
sides["away"]["team"]["id"], sides["away"].get("score"),
""))
except (KeyError, IndexError, TypeError):
continue
return rows
def _flat(row):
plugin, start, ha, hid, hs, aa, aid, as_, _state = row
return {
"league": _LEAGUE.get(plugin, plugin),
"home_abbr": ha, "home_id": hid, "home_score": hs,
"away_abbr": aa, "away_id": aid, "away_score": as_,
"start_time_utc": datetime.fromisoformat(start.replace("Z", "+00:00")),
}
def _with_nested(game):
"""The scroll card's input: flat keys kept, nested team dicts added."""
out = dict(game)
for side in ("home", "away"):
out[f"{side}_team"] = {"abbrev": game.get(f"{side}_abbr"),
"id": game.get(f"{side}_id"),
"score": game.get(f"{side}_score")}
return out
def _nested_only(game):
out = {k: v for k, v in _with_nested(game).items()
if not k.startswith(("home_abbr", "home_id", "home_score",
"away_abbr", "away_id", "away_score"))}
return out
_ROWS = FIXTURE_EVENTS + _extra_fixture_events()
FLAT_GAMES = [_flat(r) for r in _ROWS]
EDGE_FLAT_GAMES = [
# NRL: abbreviations are not unique, ids are.
{"league": "nrl", "home_abbr": "NEW", "home_id": "4", "home_score": "20",
"away_abbr": "NEW", "away_id": "12", "away_score": "10"},
{"league": "nfl", "home_abbr": "KC", "away_abbr": "BUF"},
{"league": "nfl", "home_abbr": "KC", "away_abbr": "BUF", "home_score": "", "away_score": ""},
{"league": "nfl", "home_abbr": "KC", "away_abbr": "BUF", "home_score": "-", "away_score": "-"},
{"league": "nfl", "home_abbr": "KC", "away_abbr": "BUF", "home_score": "5.0",
"away_score": "2.0"},
{"league": "nfl", "home_abbr": "KC", "home_id": 12, "away_abbr": "BUF", "away_id": 2,
"home_score": 3, "away_score": 3},
{"league": "nfl", "home_abbr": None, "away_abbr": "BUF", "home_score": "1",
"away_score": "2"},
{"league": "nfl", "home_abbr": " kc ", "away_abbr": "BUF", "home_score": "1",
"away_score": "2"},
]
ALL_FLAT = FLAT_GAMES + EDGE_FLAT_GAMES
SCROLL_SHAPED = [_with_nested(g) for g in ALL_FLAT]
def _favorite_choices(game):
"""Every way a favourites list can relate to this game."""
out = [[], ["AP_TOP_25"], ["NOBODY"]]
for side in ("home", "away"):
for key in ("abbr", "id"):
value = game.get(f"{side}_{key}")
if value is not None:
out.append([str(value)])
out.append([" " + str(value).lower() + " "])
if game.get("home_abbr") and game.get("away_abbr"):
out.append([game["home_abbr"], game["away_abbr"]])
return out
# ---------------------------------------------------------------------------
# Hosts
# ---------------------------------------------------------------------------
class _Host(SportsCoreSharedMixin):
"""The mixin with just the state these helpers read."""
def __init__(self, config=None, favorites=None, tz=timezone.utc, fonts=None):
self.config = config
self.favorite_teams = favorites
self.logger = LOG
self.fonts = fonts or {}
self._tz = tz
def _get_timezone(self):
return self._tz
#: The map seven of the eight scoreboards' sports.py declare over the mixin's
#: default (football is the one that inherits the default).
PLUGIN_ELEMENT_FOR_FONT = {
"odds": "odds_text", "score": "score_text", "time": "period_text",
"team": "team_name", "status": "status_text", "detail": "detail_text",
"rank": "rank_text",
}
def _call(fn, *args):
"""Result or the exception type, so a raise on one side is a difference."""
try:
return fn(*args)
except Exception as exc: # noqa: BLE001 - the type is the result here
return f"<raises {type(exc).__name__}>"
def _mismatches(pairs):
return [(label, a, b) for label, a, b in pairs if a != b]
RESULT_COLOURS = [
{"enabled": True},
{"enabled": True, "win_color": [1, 2, 3], "loss_color": "123",
"tie_color": [999, -1, "7"]},
{"enabled": False},
{},
]
SCROLL_CARD_CONFIGS = [
None, {}, {"scroll_card": None}, {"scroll_card": {}}, {"scroll_card": "notadict"},
{"scroll_card": {"vs_text": "@", "date_format": "weekday", "time_format": "24h",
"switch_date_format": "inherit"}},
{"scroll_card": {"vs_text": None, "date_format": "day_first", "time_format": "12h",
"switch_date_format": "inherit"}},
{"scroll_card": {"vs_text": 7, "date_format": "numeric", "switch_date_format": "inherit"}},
{"scroll_card": {"date_format": "numeric_day_first", "time_format": "24h",
"switch_date_format": "inherit"}},
{"scroll_card": {"date_format": "abbrev", "switch_date_format": "inherit"}},
{"scroll_card": {"date_format": "bogus", "switch_date_format": "inherit"}},
]
TIMES = ["7:30 PM", "12:00 AM", "12:05pm", "7 PM", "13:00 PM", "TBD", "", None,
"7:61 PM", "x:30 PM", " 9:05 am ", "12:00 PM", "0:15 AM"]
DATES = ["9/19", "09-19", "13/19", "Sep 19", "", None, "9/19/2026", " 1/2 ", "0/5"]
STARTS = [datetime(2026, 9, 19, 23, 30, tzinfo=timezone.utc), "2026-09-19T23:30Z",
"2026-09-20T02:00:00+00:00", "garbage", None, "", datetime(2026, 1, 1)]
TIMEZONES = ["America/New_York", "Australia/Sydney", "UTC", "Not/AZone", None]
PS = resolve_asset_path("assets/fonts/PressStart2P-Regular.ttf")
F46 = resolve_asset_path("assets/fonts/4x6-font.ttf")
def _font_sets():
a, b, c = load_truetype(PS, 8), load_truetype(PS, 16), load_truetype(F46, 7)
keys = ("odds", "score", "time", "team", "status", "detail", "rank")
return {
"distinct": {"score": a, "time": b, "team": c, "status": load_truetype(PS, 8),
"detail": load_truetype(F46, 7), "rank": load_truetype(F46, 14),
"odds": load_truetype(F46, 7)},
"score+time share": {"score": a, "time": a, "team": c},
"team+rank share": {"score": a, "time": b, "team": c, "rank": c},
"odds+score share": {"score": a, "odds": a, "time": b},
"all share": {k: a for k in keys},
}
def _partition(fonts):
"""Which keys still share one face object -- what unsharing decides."""
groups = {}
for key, font in fonts.items():
groups.setdefault(id(font), []).append(key)
return sorted(sorted(keys) for keys in groups.values())
def _faces(fonts):
return {k: (getattr(f, "path", None), getattr(f, "size", None)) for k, f in fonts.items()}
def _schema_dir(tmp_path, name, text):
d = tmp_path / name
d.mkdir()
if text is not None:
(d / "config_schema.json").write_text(text)
return d
SCHEMAS = {
"good": json.dumps({"properties": {"customization": {"properties": {
"score_text": {"properties": {"font_size": {"default": 10}}},
"period_text": {"properties": {"font_size": {"default": 8}}},
"detail_text": {"properties": {"font_size": {"default": "6"}}},
"team_name": {"properties": {"font": {"default": "x"}}}}}}}),
"bad_json": "{not json",
"bad_default": json.dumps({"properties": {"customization": {"properties": {
"score_text": {"properties": {"font_size": {"default": "big"}}}}}}}),
"missing": None,
}
# ---------------------------------------------------------------------------
# Identical pairs
# ---------------------------------------------------------------------------
class TestIdentical:
"""Pairs that agree on every input. Keep it that way."""
def test_scroll_card_option(self):
pairs = []
for i, cfg in enumerate(SCROLL_CARD_CONFIGS):
host = _Host(cfg)
for key, default in itertools.product(
("vs_text", "date_format", "time_format", "missing"), (None, "D", 0)):
pairs.append((f"cfg{i} {key} {default!r}",
_call(host._card_option, key, default),
_call(C.scroll_card_option, cfg, key, default)))
assert not _mismatches(pairs)
def test_vs_text(self):
pairs = [(f"cfg{i}", _call(_Host(cfg)._vs_text), _call(C.vs_text, cfg))
for i, cfg in enumerate(SCROLL_CARD_CONFIGS)]
assert not _mismatches(pairs)
def test_format_game_time(self):
pairs = [(f"cfg{i} {t!r}", _call(_Host(cfg)._format_game_time, t),
_call(C.format_game_time, cfg, t))
for (i, cfg), t in itertools.product(enumerate(SCROLL_CARD_CONFIGS), TIMES)]
assert not _mismatches(pairs)
def test_coerce_rgb(self):
values = [[1, 2, 3], (300, -4, "5"), "123", [1, 2], [1, 2, 3, 4], None, 42,
{"r": 1, "g": 2, "b": 3}, ["a", 1, 2], [1.9, 2, 3], [None, 1, 2]]
pairs = [(repr(v), _call(_Host._coerce_rgb, v, (4, 5, 6)),
_call(C.coerce_rgb, v, (4, 5, 6))) for v in values]
assert not _mismatches(pairs)
def test_crisp_size(self):
names = ["PressStart2P-Regular.ttf", "4x6-font.ttf", "press_start", "four_by_six",
"5by7.regular.ttf", "user.ttf", None]
sizes = [None, 0, -3, 1, 4, 6, 7, 8, 9, 10, 11, 12, 13, 14, 16, 20, 7.5, "8"]
pairs = [(f"{n} {s!r}", _call(_Host._crisp_size, n, s), _call(C.crisp_size, n, s))
for n, s in itertools.product(names, sizes)]
assert not _mismatches(pairs)
def test_crisp_size_honours_a_hosts_own_tables(self):
"""A class that declares extra faces keeps them through the wrapper."""
cls = type("Extra", (_Host,), {"_FONT_PIXEL_GRID": {"extra.ttf": 5},
"_FONT_NAME_ALIASES": {"x": "extra.ttf"}})
assert cls._crisp_size("x", 12) == C.crisp_size("x", 12, {"x": "extra.ttf"},
{"extra.ttf": 5}) == 10
def test_constant_tables(self):
assert SportsCoreSharedMixin.FAVORITE_RESULT_COLOR_DEFAULTS == \
C.FAVORITE_RESULT_COLOR_DEFAULTS
assert SportsCoreSharedMixin._MONTH_ABBR == C.MONTH_ABBR
assert SportsCoreSharedMixin._WEEKDAY_ABBR == C.WEEKDAY_ABBR
assert SportsCoreSharedMixin._FONT_PIXEL_GRID == C.FONT_PIXEL_GRID
assert SportsCoreSharedMixin._FONT_NAME_ALIASES == C.FONT_NAME_ALIASES
def test_constant_dicts_are_not_aliased(self):
# Equal, but separate objects: a caller mutating one table (tests do)
# must not reach into the other module.
assert SportsCoreSharedMixin.FAVORITE_RESULT_COLOR_DEFAULTS is not \
C.FAVORITE_RESULT_COLOR_DEFAULTS
assert SportsCoreSharedMixin._FONT_PIXEL_GRID is not C.FONT_PIXEL_GRID
assert SportsCoreSharedMixin._FONT_NAME_ALIASES is not C.FONT_NAME_ALIASES
@pytest.mark.parametrize("schema", sorted(SCHEMAS))
def test_schema_font_size_and_resolve_font_size(self, tmp_path, schema):
d = _schema_dir(tmp_path, schema, SCHEMAS[schema])
host = type("H_" + schema, (_Host,), {"_PLUGIN_DIR": str(d)})()
path = str(d / "config_schema.json")
pairs = []
for key in ("score_text", "period_text", "detail_text", "team_name", "nope", "", None):
pairs.append((f"schema {key!r}", _call(host._schema_font_size, key),
_call(C.schema_font_size, path, key)))
for ec, name, size in itertools.product(
(None, {}, {"font_size": 10}, {"font_size": "10"}, {"font_size": 11},
{"font_size": "big"}, {"font_size": None}, {"font_size": 8.7}),
("PressStart2P-Regular.ttf", "4x6-font.ttf", "press_start", "user.ttf"),
(6, 8, 10, None)):
pairs.append((f"resolve {key!r} {ec} {name} {size}",
_call(host._resolve_font_size, ec, key, size, name),
_call(C.resolve_font_size, path, ec, key, size, name)))
assert not _mismatches(pairs)
@pytest.mark.parametrize("element_map", ["mixin default", "plugin sports.py"])
def test_unshare_element_fonts_given_the_same_map(self, element_map):
"""Same map in, same faces out. (The maps themselves differ; pinned below.)"""
mapping = (SportsCoreSharedMixin._ELEMENT_FOR_FONT if element_map == "mixin default"
else PLUGIN_ELEMENT_FOR_FONT)
host = type("H", (_Host,), {"_ELEMENT_FOR_FONT": mapping})()
for name, fonts in _font_sets().items():
mine, theirs = dict(fonts), dict(fonts)
host._unshare_element_fonts(mine)
C.unshare_element_fonts(LOG, theirs, mapping)
assert _partition(mine) == _partition(theirs), name
assert _faces(mine) == _faces(theirs), name
def test_unshare_element_fonts_default_map_is_unchanged(self):
"""Omitting the new argument keeps the card's own map."""
for name, fonts in _font_sets().items():
default, explicit = dict(fonts), dict(fonts)
C.unshare_element_fonts(LOG, default)
C.unshare_element_fonts(LOG, explicit, C.ELEMENT_FOR_FONT)
assert _partition(default) == _partition(explicit), name
def test_format_game_date_when_both_read_the_same_setting_and_zone(self):
"""With ``switch_date_format: inherit`` the scorebug reads the card's
``date_format``; given the same zone the two then format identically."""
pairs = []
for (i, cfg), tzname in itertools.product(enumerate(SCROLL_CARD_CONFIGS[5:]),
TIMEZONES):
conf = dict(cfg, timezone=tzname) if tzname else dict(cfg)
host = _Host(conf, tz=C.card_tzinfo(conf, LOG))
for d, start in itertools.product(DATES, STARTS):
game = {"start_time_utc": start} if start is not None else {}
pairs.append((f"cfg{i} tz={tzname} {d!r} {start!r}",
_call(host._format_game_date, d, game),
_call(C.format_game_date, conf, LOG, d, game)))
pairs.append((f"weekday cfg{i} tz={tzname} {start!r}",
_call(host._weekday_for, game),
_call(C.weekday_for, conf, LOG, game)))
assert not _mismatches(pairs)
def test_format_game_date_honours_a_hosts_month_table(self):
"""The scoreboards redeclare _MONTH_ABBR; the shared body must read it."""
cls = type("Months", (_Host,), {"_MONTH_ABBR": tuple(f"M{i}" for i in range(1, 13))})
host = cls({"scroll_card": {"switch_date_format": "abbrev"}})
assert host._format_game_date("9/19") == "M9 19"
def test_favorite_result_on_the_games_the_scoreboards_build(self):
"""Production shape: the extractor stamps ``favorite_teams`` (the
manager's resolved list) on every game, and the manager holds the same
list. On those games -- flat for switch mode, flat plus nested for the
scroll card -- the two sides agree on every result and every colour."""
pairs = []
for game in ALL_FLAT:
for favs in _favorite_choices(game):
stamped = dict(game, favorite_teams=list(favs))
for colours in RESULT_COLOURS:
cfg = {"customization": {"favorite_result_colors": colours}}
host = _Host(cfg, favorites=list(favs))
pairs.append((f"{game} {favs}",
_call(host._favorite_result, stamped),
_call(C.favorite_result, cfg, _with_nested(stamped))))
pairs.append((f"{game} {favs} {colours}",
_call(host._recent_score_color, stamped, (9, 9, 9)),
_call(C.recent_score_color, cfg, LOG,
_with_nested(stamped), (9, 9, 9))))
assert not _mismatches(pairs)
# And the corpus is not vacuous: every verdict actually occurs.
verdicts = {a for _, a, _ in pairs if isinstance(a, str) or a is None}
assert {"win", "loss", "tie", None} <= verdicts
def test_side_is_favorite_on_flat_games(self):
pairs = []
for game in ALL_FLAT:
for favs in _favorite_choices(game):
fav_set = {str(f).strip().upper() for f in favs if str(f).strip()}
for side in ("home", "away"):
pairs.append((f"{game} {side} {fav_set}",
_call(_Host._side_is_favorite, game, side, fav_set),
_call(C.side_is_favorite, game, side, fav_set)))
assert not _mismatches(pairs)
def test_nrl_collision_is_resolved_the_same_way_on_both_sides(self):
"""The _favorite_key seam: NRL's "NEW" is two clubs. Neither helper
calls the seam; both match abbreviation OR id, so an id favourite picks
one club and an abbreviation favourite picks both (no verdict)."""
game = EDGE_FLAT_GAMES[0]
for favs, expected in ((["4"], "win"), (["12"], "loss"), (["NEW"], None)):
host = _Host({}, favorites=favs)
assert host._favorite_result(game) == expected
assert C.favorite_result({"favorite_teams": favs}, game) == expected
def test_an_ambiguous_nrl_abbreviation_tints_on_both_sides(self):
"""Agreed -- and at odds with NRL's own favourite rule.
NRL's resolver logs a shared abbreviation ("NEW") as an error and
passes it through unchanged, and its _is_favorite_game matches ids
only, so selection never treats "NEW" as a favourite. Both colour
helpers match on abbreviation too, so both modes tint a Knights result
(and a Warriors one) for a user who typed "NEW". Not a twin
divergence; a seam neither helper consults.
"""
on = {"customization": {"favorite_result_colors": {"enabled": True}}}
game = {"league": "3", "home_abbr": "NEW", "home_id": "4", "home_score": "20",
"away_abbr": "MEL", "away_id": "12", "away_score": "10",
"favorite_teams": ["NEW"]}
cfg = dict(on, favorite_teams=["NEW"])
assert _Host(cfg, favorites=["NEW"])._recent_score_color(game, (9, 9, 9)) \
== C.recent_score_color(cfg, LOG, game, (9, 9, 9)) == (0, 255, 0)
# ---------------------------------------------------------------------------
# Pinned divergences -- owner decision pending. Edit deliberately.
# ---------------------------------------------------------------------------
class TestPinnedDivergence:
"""Each test pins one difference between the twins as it stands today.
None of these is changed by the consolidation that made the identical pairs
wrappers: each one is a colour, a weekday or a font face that one display
mode shows differently from the other, so choosing a side is a product
decision. If you are here because one of these failed, you changed which
side wins -- make sure that was the decision, then update the pin.
"""
NESTED_WIN = {"league": "nhl",
"home_team": {"abbrev": "TB", "score": "4"},
"away_team": {"abbrev": "BOS", "score": "1"}}
def test_side_is_favorite_nested_payload(self):
# DIVERGENCE: the mixin reads only the flat <side>_abbr / <side>_id keys;
# the card also reads <side>_team.{abbrev,abbreviation,id}. Unreachable
# from the scoreboards' own extractors (always flat), reachable from a
# nested-only payload.
assert _Host._side_is_favorite(self.NESTED_WIN, "home", {"TB"}) is False
assert C.side_is_favorite(self.NESTED_WIN, "home", {"TB"}) is True
def test_favorite_result_nested_payload(self):
# DIVERGENCE: follows from the one above, plus score source: the mixin
# reads home_score/away_score only; the card prefers the nested score.
host = _Host({}, favorites=["TB"])
game = dict(self.NESTED_WIN, favorite_teams=["TB"])
assert host._favorite_result(game) is None
assert C.favorite_result({}, game) == "win"
def test_favorite_result_when_nested_and_flat_scores_disagree(self):
# DIVERGENCE: same game, two score sources. The mixin uses the flat
# score, the card the nested one. The renderers' normaliser only fills
# a nested score that is missing, so this needs a payload that already
# carried both.
game = {"home_abbr": "TB", "away_abbr": "BOS", "home_score": "1",
"away_score": "4", "home_team": {"abbrev": "TB", "score": "4"},
"away_team": {"abbrev": "BOS", "score": "1"}, "favorite_teams": ["TB"]}
assert _Host({}, favorites=["TB"])._favorite_result(game) == "loss"
assert C.favorite_result({}, game) == "win"
def test_favorite_result_favourite_sources(self):
# DIVERGENCE: where the favourites come from. The mixin reads only
# self.favorite_teams (the manager's list, resolved at construction);
# the card reads the game's stamped favorite_teams plus the config's
# league block (or root). All eight scoreboards stamp the game, so in
# production both see the same list -- this pins the hand-built case.
game = {"league": "mlb", "home_abbr": "ATL", "away_abbr": "NYM",
"home_score": "5", "away_score": "2"}
on = {"customization": {"favorite_result_colors": {"enabled": True}}}
# Host favourites only, nothing stamped, nothing in config:
assert _Host(on, favorites=["ATL"])._favorite_result(game) == "win"
assert C.favorite_result(on, game) is None
# Config league block only, host list empty:
cfg = dict(on, mlb={"favorite_teams": ["ATL"]})
assert _Host(cfg, favorites=[])._favorite_result(game) is None
assert C.favorite_result(cfg, game) == "win"
# Stamped on the game only, host list empty:
stamped = dict(game, favorite_teams=["ATL"])
assert _Host(on, favorites=[])._favorite_result(stamped) is None
assert C.favorite_result(on, stamped) == "win"
# ...which is what reaches the colour:
assert _Host(on, favorites=["ATL"])._recent_score_color(game, (9, 9, 9)) == (0, 255, 0)
assert C.recent_score_color(on, LOG, game, (9, 9, 9)) == (9, 9, 9)
def test_weekday_zone_source(self):
# DIVERGENCE, user-visible: the scorebug asks the plugin's
# _get_timezone() (plugin setting -> global setting -> system zone);
# the card reads only config["timezone"] and falls back to UTC. The
# scoreboards' schemas default that key to "", and the scroll display
# hands the renderer the plugin config, so a board that sets only the
# global zone gets UTC weekdays in scroll mode: an evening kickoff in
# New York is labelled with the next day.
game = {"start_time_utc": "2026-09-20T00:30:00+00:00"} # Sat 20:30 EDT
host = _Host({}, tz=ZoneInfo("America/New_York"))
assert host._weekday_for(game) == "Sat"
assert C.weekday_for({}, LOG, game) == "Sun"
cfg = {"scroll_card": {"date_format": "weekday", "switch_date_format": "inherit"}}
host = _Host(cfg, tz=ZoneInfo("America/New_York"))
assert host._format_game_date("9/19", game) == "Sat Sep 19"
assert C.format_game_date(cfg, LOG, "9/19", game) == "Sun Sep 19"
def test_weekday_out_of_range_start(self):
# DIVERGENCE: the mixin catches OverflowError from astimezone() and
# drops the weekday; the card lets it escape to its caller.
game = {"start_time_utc": "9999-12-31T23:59:00+00:00"}
sydney = {"timezone": "Australia/Sydney"}
assert _Host(sydney, tz=ZoneInfo("Australia/Sydney"))._weekday_for(game) == ""
with pytest.raises(OverflowError):
C.weekday_for(sydney, LOG, game)
def test_date_format_setting(self):
# DIVERGENCE BY DESIGN (documented on _switch_date_format): the scorebug
# reads scroll_card.switch_date_format (default "numeric", the "9/19"
# it has always drawn); the card reads scroll_card.date_format (default
# "abbrev"). "inherit" opts the scorebug into the card's setting.
assert _Host({})._format_game_date("9/19") == "9/19"
assert C.format_game_date({}, LOG, "9/19") == "Sep 19"
def test_upcoming_centre_setting(self):
# DIVERGENCE BY DESIGN: not a same-named twin, but the same question.
# switch_upcoming_center defaults to "date_time"; the card's
# upcoming_center to "vs". "inherit" opts the scorebug in.
assert _Host({})._switch_upcoming_center() == "date_time"
assert C.upcoming_center_mode({}) == "vs"
cfg = {"scroll_card": {"switch_upcoming_center": "inherit"}}
assert _Host(cfg)._switch_upcoming_center() == C.upcoming_center_mode(cfg) == "vs"
def test_element_for_font_maps(self):
# DIVERGENCE: the element vocabulary. The mixin default says team_text
# and has no rank/odds; the card says team_name and has rank but no
# odds. Seven scoreboards override the mixin map in sports.py with
# PLUGIN_ELEMENT_FOR_FONT (team_name, rank, odds); football inherits
# the default, and its schema declares team_name, not team_text.
assert SportsCoreSharedMixin._ELEMENT_FOR_FONT == {
"score": "score_text", "time": "period_text", "team": "team_text",
"detail": "detail_text", "status": "status_text"}
assert C.ELEMENT_FOR_FONT == {
"score": "score_text", "time": "period_text", "team": "team_name",
"status": "status_text", "detail": "detail_text", "rank": "rank_text"}
def test_font_color_team_element(self):
# DIVERGENCE (consequence of the maps): a colour set on team_name
# reaches the card's team face but not the mixin-default one.
team = load_truetype(F46, 7)
fonts = {"score": load_truetype(PS, 8), "team": team}
cfg = {"customization": {"team_name": {"text_color": [1, 1, 1]}}}
assert _Host(cfg, fonts=fonts)._font_color(team, (7, 7, 7)) == (7, 7, 7)
assert C.font_color(cfg, fonts, team, (7, 7, 7)) == (1, 1, 1)
plugin_host = type("P", (_Host,), {"_ELEMENT_FOR_FONT": PLUGIN_ELEMENT_FOR_FONT})
assert plugin_host(cfg, fonts=fonts)._font_color(team, (7, 7, 7)) == (1, 1, 1)
def test_unshare_element_fonts_odds_face(self):
# DIVERGENCE (consequence of the maps): the scoreboards' sports.py map
# includes "odds", so switch mode gives the odds face its own object;
# the card's map has no "odds", so scroll mode leaves it sharing the
# score's face (and _card.font_color then colours it as score_text).
shared = load_truetype(PS, 8)
mine = {"score": shared, "odds": shared}
theirs = dict(mine)
type("P", (_Host,), {"_ELEMENT_FOR_FONT": PLUGIN_ELEMENT_FOR_FONT})() \
._unshare_element_fonts(mine)
C.unshare_element_fonts(LOG, theirs)
assert mine["odds"] is not mine["score"]
assert theirs["odds"] is theirs["score"]
def test_schema_default_cache_lifetimes(self, tmp_path):
# DELIBERATE, and the reason there are still two caches: the mixin
# caches per class, the card per schema path. The display service
# builds new classes when it reloads a plugin, so switch mode picks up
# an edited schema then; the card's module-level cache does not. One
# shared cache would change what switch mode does after a reload.
d = _schema_dir(tmp_path, "reload", SCHEMAS["good"])
path = str(d / "config_schema.json")
first = type("First", (_Host,), {"_PLUGIN_DIR": str(d)})()
assert first._schema_font_size("score_text") == 10
assert C.schema_font_size(path, "score_text") == 10
(d / "config_schema.json").write_text(SCHEMAS["good"].replace("10", "16"))
reloaded = type("Reloaded", (_Host,), {"_PLUGIN_DIR": str(d)})()
assert reloaded._schema_font_size("score_text") == 16
assert first._schema_font_size("score_text") == 10
assert C.schema_font_size(path, "score_text") == 10
def test_element_color_mode(self):
# DIVERGENCE at the call site, not in a body: both resolve through
# src.element_style, but the mixin passes the instance's SKIN_MODE
# ("live"/"recent"/"upcoming", set by all eight scoreboards) and the
# renderers call _card.element_color with no mode, so a per-mode colour
# override applies in switch mode only.
cfg = {"customization": {"score_text": {"text_color": [255, 0, 0]},
"modes": {"recent": {"score_text": {"text_color": [0, 0, 255]}}}}}
host = _Host(cfg)
host.SKIN_MODE = "recent"
assert host._element_color("score_text") == (0, 0, 255)
assert C.element_color(cfg, "score_text") == (255, 0, 0)
test/test_sudo_allowlist_covers_calls.py
+3
View File
@@ -37,6 +37,9 @@ ROOT = Path(__file__).resolve().parent.parent
INSTALLERS = (
ROOT / "first_time_install.sh",
ROOT / "scripts" / "install" / "configure_wifi_permissions.sh",
# The ledmatrix_web rules, which first_time_install.sh and
# configure_web_sudo.sh both take from here.
ROOT / "scripts" / "install" / "lib_sudoers.sh",
)
#: Commands this change grants, each fully literal in the source.
test/test_sudoers_is_validated.py
+114 -12
View File
@@ -62,33 +62,135 @@ def test_configure_web_sudo_validates_before_installing():
assert validate < install, "the rules must be checked before they are installed"
def _render_first_time_sudoers(project_root, user):
"""Run the installer's own sudoers heredoc with realistic values."""
def test_a_missing_rules_library_installs_nothing():
"""If lib_sudoers.sh is missing, nothing is generated -- and an empty file
would pass `visudo -c` -- so that branch must set the flag the install is
gated on."""
body = _read(FIRST_TIME)
start = body.index("# Create sudoers content")
missing = body.index('if [ -f "$SUDOERS_LIB" ]; then')
flagged = body.index("SUDOERS_VALID=0", missing)
validate = body.index('visudo -c -f "$SUDOERS_TMP"')
install = body.index('cp "$SUDOERS_TMP" "$SUDOERS_FILE"')
gate = body.rindex('if [ "$SUDOERS_VALID" = "0" ]; then', 0, install)
assert missing < flagged < validate < gate < install
def _step10_generation(body):
"""first_time_install.sh's own Step 10 code that writes $SUDOERS_TMP."""
start = body.index("# The rules themselves live in scripts/install/lib_sudoers.sh")
end = body.index("# Never install rules we have not parsed.")
block = body[start:end]
out = os.path.join(project_root, "rendered")
return body[start:end]
def _run_step10_generation(project_root, user, out):
"""Run the installer's Step 10 generation with realistic values.
Returns the SUDOERS_VALID it leaves behind."""
script = "\n".join(
[
"set -euo pipefail",
"set -Eeuo pipefail",
f"ACTUAL_USER={user}",
f"PROJECT_ROOT_DIR={project_root}",
'SUDOERS_TMP="$(mktemp)"',
"PYTHON_PATH=$(which python3)",
f"PROJECT_ROOT_DIR='{project_root}'",
f"SUDOERS_TMP='{out}'",
"SUDOERS_FILE=/etc/sudoers.d/ledmatrix_web",
"SYSTEMCTL_PATH=/usr/bin/systemctl",
"REBOOT_PATH=/usr/sbin/reboot",
"POWEROFF_PATH=/usr/sbin/poweroff",
"BASH_PATH=$(which bash)",
"JOURNALCTL_PATH=/usr/bin/journalctl",
block,
f'cp "$SUDOERS_TMP" {out}',
_step10_generation(_read(FIRST_TIME)),
'printf %s "$SUDOERS_VALID"',
]
)
subprocess.run(["bash", "-c", script], check=True)
return subprocess.run(
["bash", "-c", script], check=True, capture_output=True, text=True
).stdout
def _render_first_time_sudoers(tmp, user):
"""The rules first_time_install.sh generates, via the shared library."""
out = os.path.join(tmp, "rendered")
assert _run_step10_generation(REPO_ROOT, user, out) == "1"
return out
def _run_step10(tmp, project_root, visudo_ok, existing=None):
"""Run all of Step 10 against a sudoers file in `tmp`, never /etc.
systemctl, reboot, poweroff, journalctl and visudo are stubs, so the
outcome does not depend on the machine running the test."""
body = _read(FIRST_TIME)
step = body[body.index('CURRENT_STEP="Configure passwordless sudo access"'):
body.index('CURRENT_STEP="Configure WiFi management permissions"')]
target = os.path.join(tmp, "ledmatrix_web")
real = 'SUDOERS_FILE="/etc/sudoers.d/ledmatrix_web"'
assert step.count(real) == 1
step = step.replace(real, f"SUDOERS_FILE='{target}'")
stubs = os.path.join(tmp, "stubs")
os.mkdir(stubs)
for name, code in (("systemctl", 0), ("reboot", 0), ("poweroff", 0),
("journalctl", 0), ("visudo", 0 if visudo_ok else 1)):
path = os.path.join(stubs, name)
with open(path, "w", encoding="utf-8") as handle:
handle.write(f"#!/bin/sh\nexit {code}\n")
os.chmod(path, 0o755)
if existing is not None:
with open(target, "w", encoding="utf-8") as handle:
handle.write(existing)
env = dict(os.environ, TMPDIR=tmp,
PATH=os.pathsep.join([stubs, os.path.dirname(sys.executable),
"/usr/bin", "/bin"]))
script = "\n".join(["set -Eeuo pipefail", "ACTUAL_USER=ledmatrix",
f"PROJECT_ROOT_DIR='{project_root}'", step])
result = subprocess.run(["bash", "-c", script], env=env,
capture_output=True, text=True)
assert result.returncode == 0, result.stdout + result.stderr
return target, stubs, result
_POSIX_STEP10 = pytest.mark.skipif(
sys.platform == "win32" or shutil.which("which") is None,
reason="needs a POSIX bash and which")
@_POSIX_STEP10
def test_step10_installs_the_generated_rules():
with tempfile.TemporaryDirectory() as tmp:
target, stubs, _ = _run_step10(tmp, REPO_ROOT, visudo_ok=True)
assert oct(os.stat(target).st_mode & 0o777) == "0o440"
with open(target, encoding="utf-8") as handle:
installed = handle.read()
lib = os.path.join(REPO_ROOT, "scripts", "install", "lib_sudoers.sh")
expected = subprocess.run(
["bash", "-c", '. "$1"; web_sudoers_rules ledmatrix "$2" "$3/systemctl" '
'"$(command -v bash)" "$3/reboot" "$3/poweroff" "$3/journalctl"',
"_", lib, REPO_ROOT, stubs],
check=True, capture_output=True, text=True,
env=dict(os.environ, PATH=os.pathsep.join([stubs, "/usr/bin", "/bin"])),
).stdout
assert installed == expected
assert not [f for f in os.listdir(tmp) if f.startswith("ledmatrix_web_sudoers.")]
@_POSIX_STEP10
def test_step10_without_the_library_keeps_the_existing_file():
with tempfile.TemporaryDirectory() as tmp:
target, _, result = _run_step10(tmp, tmp, visudo_ok=True, existing="keep\n")
with open(target, encoding="utf-8") as handle:
assert handle.read() == "keep\n"
assert "lib_sudoers.sh not found" in result.stderr
assert "Passwordless sudo access configured" not in result.stdout
@_POSIX_STEP10
def test_step10_keeps_the_existing_file_when_the_rules_do_not_parse():
with tempfile.TemporaryDirectory() as tmp:
target, _, result = _run_step10(tmp, REPO_ROOT, visudo_ok=False, existing="keep\n")
with open(target, encoding="utf-8") as handle:
assert handle.read() == "keep\n"
assert "did not parse" in result.stderr
@pytest.mark.skipif(sys.platform == "win32", reason="visudo is POSIX only")
@pytest.mark.skipif(VISUDO is None, reason="visudo not installed")
def test_the_rules_the_installer_emits_actually_parse():
test/test_sudoers_noexec_on_pagers.py
+7 -3
View File
@@ -30,10 +30,14 @@ ROOT = Path(__file__).resolve().parent.parent
INSTALLERS = (
ROOT / "first_time_install.sh",
ROOT / "scripts" / "install" / "configure_wifi_permissions.sh",
# Writes the same journalctl grants as first_time_install.sh. It was
# missing here, and because of that this suite passed while three
# ungranted wildcard rules sat in it.
# Used to write its own copy of the journalctl grants. It was missing
# here, and because of that this suite passed while three untagged
# wildcard rules sat in it. Both it and first_time_install.sh now take
# their rules from lib_sudoers.sh; they stay listed so a rule written
# directly into either one is still checked.
ROOT / "scripts" / "install" / "configure_web_sudo.sh",
# The ledmatrix_web rules, shared by both installers.
ROOT / "scripts" / "install" / "lib_sudoers.sh",
)
#: Commands that will start another program of their own accord -- a pager, an
test/test_web_error_detail.py
+46
View File
@@ -230,6 +230,52 @@ class TestHandlersCarryDetail:
"handlers returning the generic message without %s: %r"
% ("both a traceback log and the detail", offenders))
def test_no_api_v3_route_copies_the_blueprint_handler(self):
"""The generic catch-all lives once, on the blueprint.
Fifty-three routes carried their own copy of it -- log with exc_info,
return {status, "An error occurred; see logs for details",
describe_exception(e)}, 500 -- until they were folded into
`_api_v3_unhandled_exception`. A new copy changes nothing a caller
sees, so nothing else would notice it; this does. A handler that says
something *different* (its own message, extra keys, cleanup) is fine.
"""
import ast
import pathlib
generic = "An error occurred; see logs for details"
copies = []
for path in sorted(pathlib.Path("web_interface/blueprints/api_v3").glob("*.py")):
tree = ast.parse(path.read_text(encoding="utf-8"))
for fn in [n for n in ast.walk(tree) if isinstance(n, ast.FunctionDef)]:
for h in ast.walk(fn):
if not (isinstance(h, ast.ExceptHandler)
and isinstance(h.type, ast.Name)
and h.type.id == "Exception"):
continue
for r in [n for n in h.body if isinstance(n, ast.Return)]:
v = r.value
if not (isinstance(v, ast.Tuple) and len(v.elts) == 2
and isinstance(v.elts[0], ast.Call)
and getattr(v.elts[0].func, "id", None) == "jsonify"
and v.elts[0].args
and isinstance(v.elts[0].args[0], ast.Dict)):
continue
d = v.elts[0].args[0]
keys = {k.value for k in d.keys if isinstance(k, ast.Constant)}
message = [val.value for k, val in zip(d.keys, d.values)
if isinstance(k, ast.Constant) and k.value == "message"
and isinstance(val, ast.Constant)]
if keys == {"status", "message", "details"} and message == [generic]:
copies.append((path.name, fn.name))
# One is not a copy: execute_plugin_action's step-1 handler sits
# inside the route's `except subprocess.TimeoutExpired` arm, which
# would turn a plugin's own timeout into a 408 if this let it through.
assert copies == [("plugins.py", "execute_plugin_action")], (
"these handlers duplicate the api_v3 blueprint's errorhandler; "
"delete them and let the exception propagate: %r" % copies)
def test_client_errors_keep_their_own_status(self):
"""A 405 must not be reported as a server-side UNKNOWN_ERROR.
test/test_web_sudoers_installers_agree.py
+165 -78
View File
@@ -1,53 +1,189 @@
"""The two installers that write /etc/sudoers.d/ledmatrix_web must agree.
"""One generator writes /etc/sudoers.d/ledmatrix_web, and both installers use it.
first_time_install.sh (Step 10, a heredoc) and scripts/install/configure_web_sudo.sh
(a block of echo lines) each generate the web user's sudo allow-list. They
drifted: configure_web_sudo.sh granted scripts/fix_perms/safe_pip_install.sh
but first_time_install.sh did not, so on a device set up only by the first-time
first_time_install.sh (Step 10) and scripts/install/configure_web_sudo.sh each
used to carry their own copy of the web user's sudo allow-list -- a heredoc in
one, a block of echo lines in the other -- and the copies drifted:
configure_web_sudo.sh granted scripts/fix_perms/safe_pip_install.sh but
first_time_install.sh did not, so on a device set up only by the first-time
installer permission_utils.install_requirements_file could not use the root
wrapper and fell back to a user-level install that root-run ledmatrix.service
may not see (and the auto-update rollback reported its reinstall as failed).
This compares the granted command sets after normalising the spellings that
differ between the files but expand identically at install time:
$WEB_USER/$ACTUAL_USER, $PROJECT_ROOT/$PROJECT_ROOT_DIR, and the helper-path
variables configure_web_sudo.sh defines ($SAFE_RM_PATH, ...).
The rules now live once, in web_sudoers_rules() in
scripts/install/lib_sudoers.sh. What keeps them from drifting again:
* neither installer writes a rule line of its own, and each writes the
generator's output to the very file it then validates and installs;
* each passes its variables to the generator in the right positions -- checked
by running the installer's own call line with distinct values;
* the generator's grants are pinned to an explicit list below, so dropping,
adding or re-pathing a grant is a deliberate edit to this file.
It also checks that every fix_perms helper granted via sudo is hardened to
root:root in both scripts -- and, in first_time_install.sh, after Step 11's
root:root in both installers -- and, in first_time_install.sh, after Step 11's
project-wide chown to the user, which would otherwise undo it.
"""
import re
import shutil
import subprocess
import sys
from pathlib import Path
import pytest
ROOT = Path(__file__).resolve().parent.parent
FIRST_TIME = ROOT / "first_time_install.sh"
CONFIGURE = ROOT / "scripts" / "install" / "configure_web_sudo.sh"
LIB = ROOT / "scripts" / "install" / "lib_sudoers.sh"
#: Grants that intentionally exist in only one installer, as normalised
#: commands. There are none today; add one here with a reason rather than
#: loosening the comparison.
ONLY_IN_FIRST_TIME = frozenset()
ONLY_IN_CONFIGURE = frozenset()
#: Every grant web_sudoers_rules() writes, as (tags, command) with the
#: generator's own variable names. Changing the allow-list means changing this.
EXPECTED_GRANTS = frozenset({
("NOPASSWD:", "$REBOOT_PATH"),
("NOPASSWD:", "$POWEROFF_PATH"),
("NOPASSWD:", "$SYSTEMCTL_PATH start ledmatrix.service"),
("NOPASSWD:", "$SYSTEMCTL_PATH stop ledmatrix.service"),
("NOPASSWD:", "$SYSTEMCTL_PATH restart ledmatrix.service"),
("NOPASSWD:", "$SYSTEMCTL_PATH enable ledmatrix.service"),
("NOPASSWD:", "$SYSTEMCTL_PATH disable ledmatrix.service"),
("NOPASSWD:", "$SYSTEMCTL_PATH status ledmatrix.service"),
("NOPASSWD:", "$SYSTEMCTL_PATH is-active ledmatrix"),
("NOPASSWD:", "$SYSTEMCTL_PATH is-active ledmatrix.service"),
("NOPASSWD:", "$SYSTEMCTL_PATH start ledmatrix-web.service"),
("NOPASSWD:", "$SYSTEMCTL_PATH stop ledmatrix-web.service"),
("NOPASSWD:", "$SYSTEMCTL_PATH restart ledmatrix-web.service"),
("NOPASSWD:", "$BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_plugin_rm.sh *"),
("NOPASSWD:", "$BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh *"),
("NOPASSWD:NOEXEC:", "$JOURNALCTL_PATH -u ledmatrix.service *"),
("NOPASSWD:NOEXEC:", "$JOURNALCTL_PATH -u ledmatrix *"),
("NOPASSWD:NOEXEC:", "$JOURNALCTL_PATH -t ledmatrix *"),
})
#: The call each installer makes: its own names for the generator's arguments,
#: in order, and the file it writes the rules to.
CALLERS = {
FIRST_TIME: (("$ACTUAL_USER", "$PROJECT_ROOT_DIR", "$SYSTEMCTL_PATH", "$BASH_PATH",
"$REBOOT_PATH", "$POWEROFF_PATH", "$JOURNALCTL_PATH"), "$SUDOERS_TMP"),
CONFIGURE: (("$WEB_USER", "$PROJECT_ROOT", "$SYSTEMCTL_PATH", "$BASH_PATH",
"$REBOOT_PATH", "$POWEROFF_PATH", "$JOURNALCTL_PATH"), "$TEMP_SUDOERS"),
}
RULE = re.compile(r'(\S+) ALL=\(ALL\) (NOPASSWD:(?:NOEXEC:)?)\s*(.*?)"?$')
def _text(path):
return path.read_text(encoding="utf-8", errors="replace")
return path.read_text(encoding="utf-8", errors="replace").replace("\r\n", "\n")
def _web_sudoers_section(path):
"""The part of the script that writes the ledmatrix_web allow-list.
def _generator_grants():
"""{(tags, command)} for every rule line in lib_sudoers.sh."""
grants = set()
for line in _text(LIB).splitlines():
m = RULE.search(line.strip())
if m and m.group(1).endswith("$WEB_USER"):
grants.add((m.group(2), " ".join(m.group(3).split())))
return grants
first_time_install.sh also writes other files later (WiFi permissions are
delegated to a separate script, but keep this robust against future
additions), so restrict it to Step 10.
"""
def _call(path):
"""The installer's web_sudoers_rules statement, continuation lines joined."""
text = _text(path)
if path == FIRST_TIME:
start = text.index('CURRENT_STEP="Configure passwordless sudo access"')
end = text.index('CURRENT_STEP="Configure WiFi management permissions"')
return text[start:end]
return text
calls = re.findall(r"^[ \t]*web_sudoers_rules\b(?:[^\n]*\\\n)*[^\n]*$", text, re.M)
assert len(calls) == 1, f"{path.name}: expected one web_sudoers_rules call, found {calls}"
return calls[0]
def test_generator_grants_exactly_the_expected_rules():
grants = _generator_grants()
assert grants == EXPECTED_GRANTS, (
f"lib_sudoers.sh grants changed:\n added: {sorted(grants - EXPECTED_GRANTS)}\n"
f" removed: {sorted(EXPECTED_GRANTS - grants)}")
@pytest.mark.parametrize("installer", [FIRST_TIME, CONFIGURE], ids=lambda p: p.name)
def test_installer_writes_no_rules_of_its_own(installer):
"""A rule added to one installer only is how they drifted last time."""
own = [line for line in _text(installer).splitlines()
if "NOPASSWD" in line and not line.lstrip().startswith("#")]
assert not own, f"{installer.name} writes sudoers rules itself: {own}"
@pytest.mark.parametrize("installer", [FIRST_TIME, CONFIGURE], ids=lambda p: p.name)
def test_installer_sources_the_generator_and_writes_what_it_validates(installer):
text = _text(installer)
assert "lib_sudoers.sh" in text, f"{installer.name} does not source lib_sudoers.sh"
args, target = CALLERS[installer]
call = _call(installer)
words = call.replace("\\\n", " ").split()
assert words[0] == "web_sudoers_rules"
assert tuple(w.strip('"') for w in words[1:8]) == args, (
f"{installer.name} passes the generator's arguments out of order: {call}")
assert words[8:] == [">", f'"{target}"'], call
# ...and that file is the one it runs visudo on.
assert f'visudo -c -f "{target}"' in text
@pytest.mark.skipif(sys.platform == "win32" or shutil.which("bash") is None,
reason="needs a POSIX bash")
@pytest.mark.parametrize("installer", [FIRST_TIME, CONFIGURE], ids=lambda p: p.name)
def test_installer_call_renders_the_expected_rules(installer, tmp_path):
"""Run the installer's own call line, with a distinct value per argument."""
args, target = CALLERS[installer]
values = {
args[0]: "webuser", args[1]: "/srv/led root", args[2]: "/x/systemctl",
args[3]: "/x/bash", args[4]: "/x/reboot", args[5]: "/x/poweroff",
args[6]: "/x/journalctl", target: str(tmp_path / "out"),
}
assigns = "\n".join(f"{name[1:]}='{value}'" for name, value in values.items())
script = f"set -euo pipefail\n. '{LIB}'\n{assigns}\n{_call(installer)}\n"
subprocess.run(["bash", "-c", script], check=True)
rendered = set()
for line in (tmp_path / "out").read_text(encoding="utf-8").splitlines():
m = RULE.match(line)
if m:
assert m.group(1) == "webuser", line
rendered.add((m.group(2), m.group(3)))
subst = {"$SYSTEMCTL_PATH": "/x/systemctl", "$BASH_PATH": "/x/bash",
"$REBOOT_PATH": "/x/reboot", "$POWEROFF_PATH": "/x/poweroff",
"$JOURNALCTL_PATH": "/x/journalctl", "$PROJECT_ROOT": "/srv/led root"}
expected = set()
for tags, command in EXPECTED_GRANTS:
for var, value in subst.items():
command = command.replace(var, value)
expected.add((tags, command))
assert rendered == expected
@pytest.mark.skipif(sys.platform == "win32" or shutil.which("bash") is None,
reason="needs a POSIX bash")
def test_optional_tools_are_left_out_when_absent(tmp_path):
"""configure_web_sudo.sh passes "" for a missing reboot/poweroff/journalctl.
An empty path would otherwise leave `user ALL=(ALL) NOPASSWD: ` behind,
which visudo rejects, and the whole file would not be installed.
"""
out = subprocess.run(
["bash", "-c", f". '{LIB}'; web_sudoers_rules u /p /bin/systemctl /bin/bash '' '' ''"],
check=True, capture_output=True, text=True).stdout
rules = [line for line in out.splitlines() if RULE.match(line)]
assert len(rules) == len(EXPECTED_GRANTS) - 5
assert not [r for r in rules if r.rstrip().endswith("NOPASSWD:")]
assert "journalctl" not in out
def test_pip_install_helper_is_granted():
wanted = ("NOPASSWD:", "$BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh *")
assert wanted in _generator_grants()
def _granted_helpers():
helpers = set()
for _, command in _generator_grants():
m = re.search(r"scripts/fix_perms/([\w.-]+\.sh)", command)
if m:
helpers.add(m.group(1))
assert helpers, "no fix_perms helper grant found; the parser matched nothing"
return helpers
def _variables(text):
@@ -60,57 +196,9 @@ def _normalise(command, variables):
for _ in range(3): # helper paths reference $PROJECT_ROOT
command = re.sub(r"\$\{?([A-Z][A-Z0-9_]*)\}?",
lambda m: variables.get(m.group(1), m.group(0)), command)
command = command.replace("$PROJECT_ROOT_DIR", "$PROJECT_ROOT")
return " ".join(command.split())
def _grants(path):
"""{(tags, command)} for every ledmatrix_web rule the script writes."""
section = _web_sudoers_section(path)
variables = _variables(_text(path))
grants = set()
for line in section.splitlines():
m = re.search(r'\$(?:WEB_USER|ACTUAL_USER) ALL=\(ALL\) (NOPASSWD:(?:NOEXEC:)?)\s*(.*)$',
line)
if not m:
continue
command = m.group(2).rstrip().rstrip('"').rstrip()
grants.add((m.group(1), _normalise(command, variables)))
return grants
def test_both_installers_generate_rules():
# Guards against the parser silently matching nothing in either file.
assert len(_grants(FIRST_TIME)) >= 15
assert len(_grants(CONFIGURE)) >= 15
def test_installers_grant_the_same_commands():
first = _grants(FIRST_TIME)
configure = _grants(CONFIGURE)
only_first = {c for c in first - configure if c[1] not in ONLY_IN_FIRST_TIME}
only_configure = {c for c in configure - first if c[1] not in ONLY_IN_CONFIGURE}
assert not only_first and not only_configure, (
"ledmatrix_web sudoers drift between installers:\n"
f" only in first_time_install.sh: {sorted(only_first)}\n"
f" only in configure_web_sudo.sh: {sorted(only_configure)}")
def test_pip_install_helper_is_granted():
wanted = ("NOPASSWD:", "$BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh *")
assert wanted in _grants(FIRST_TIME)
assert wanted in _grants(CONFIGURE)
def _granted_helpers():
helpers = set()
for _, command in _grants(FIRST_TIME) | _grants(CONFIGURE):
m = re.search(r"scripts/fix_perms/([\w.-]+\.sh)", command)
if m:
helpers.add(m.group(1))
return helpers
def test_every_granted_helper_is_hardened_in_configure_web_sudo():
text = _text(CONFIGURE)
variables = _variables(text)
@@ -138,9 +226,8 @@ def test_no_grant_runs_a_file_the_web_user_can_edit():
rule for it lets the web user rewrite the file and run it as root. The
grants for display_controller.py, start_display.sh and stop_display.sh
were exactly that, and nothing ever ran them through sudo."""
for installer in (FIRST_TIME, CONFIGURE):
for _, command in _grants(installer):
for _, command in _generator_grants():
for token in command.split():
if token.startswith("$PROJECT_ROOT/"):
assert token.startswith("$PROJECT_ROOT/scripts/fix_perms/"), (
f"{installer.name} grants root on a user-owned file: {command}")
f"lib_sudoers.sh grants root on a user-owned file: {command}")
test/web_interface/test_api_v3_names_resolve.py
+25
View File
@@ -0,0 +1,25 @@
"""Names two rarely-run api_v3 paths call must exist.
Both slipped through because nothing exercised them: the Pixlet editor's
stop route only restarts the display after a SIGKILL, and the Starlark
device-location resolver only builds a cache manager when the web app has
not set one. Either raised NameError when it finally ran.
"""
from unittest.mock import patch
from test._api_v3_test_helpers import api_v3_module # noqa: F401
def test_the_editor_stop_route_can_restart_the_display():
from web_interface.blueprints.api_v3 import starlark
assert callable(starlark._run_systemctl_command)
def test_the_device_location_resolver_builds_without_a_cache_manager(api_v3_module):
pkg = api_v3_module
with patch.object(pkg.api_v3, 'cache_manager', None, create=True), \
patch.object(pkg, '_starlark_device_location', None):
resolver = pkg._get_starlark_device_location()
assert resolver.cache_manager is None
test/web_interface/test_api_v3_unhandled_errors.py
+221
View File
@@ -0,0 +1,221 @@
"""An exception no api_v3 route catches is answered once, by the blueprint.
Fifty-three routes used to end in a copy of the same catch-all:
except Exception as e:
logger.error(..., exc_info=True)
return jsonify({'status': 'error',
'message': 'An error occurred; see logs for details',
'details': describe_exception(e)}), 500
They were removed in favour of one errorhandler on the api_v3 blueprint. These
tests pin that the answer did not change: the same status, exactly the same
keys and values, credentials still redacted, and the traceback still logged.
The exact-equality matters. web_interface/app.py's global handler answers with
an extra `error_code: UNKNOWN_ERROR`, and the plugin API client routes a body
that carries an error_code to a different UI path (api_client.js) -- so
"falls through to the global handler" would not have been the same answer.
"""
import logging
import pytest
from flask import Flask
from werkzeug.exceptions import UnsupportedMediaType
from src.web_interface.error_handler import describe_exception
from web_interface.blueprints.api_v3 import api_v3
# A credential in three of the forms describe_exception redacts.
FORCED = RuntimeError(
"forced failure token=SECRET123 at https://u:pw1@example.com/x?api_key=K1")
# What every removed catch-all returned, written out rather than imported so
# a change to the shared payload cannot also change the expectation.
EXPECTED = {
'status': 'error',
'message': 'An error occurred; see logs for details',
'details': describe_exception(FORCED),
}
MANAGERS = ("config_manager", "plugin_manager", "plugin_store_manager",
"saved_repositories_manager", "schema_manager", "operation_queue",
"plugin_state_manager", "operation_history", "cache_manager")
class Boom:
"""A manager that fails on any use -- attribute, truthiness, call."""
def _raise(self, *args, **kwargs):
raise FORCED
__getattr__ = _raise
__bool__ = _raise
__call__ = _raise
__iter__ = _raise
__len__ = _raise
@pytest.fixture
def exploding_managers(monkeypatch):
for name in MANAGERS:
monkeypatch.setattr(api_v3, name, Boom(), raising=False)
# The WiFi routes build their own manager rather than using one above.
import src.wifi_manager
monkeypatch.setattr(src.wifi_manager, "WiFiManager", Boom())
@pytest.fixture
def client(exploding_managers):
"""The blueprint alone, on an app with no error handlers of its own."""
app = Flask(__name__)
app.register_blueprint(api_v3, url_prefix="/api/v3")
return app.test_client()
# A sample of routes whose catch-all was removed, across every module that
# lost one. Each reaches a manager (or WiFiManager) inside what used to be
# the try block.
REMOVED_CATCH_ALLS = [
("GET", "/api/v3/config/main", None),
("GET", "/api/v3/config/secrets", None),
("GET", "/api/v3/display/modes", None),
("POST", "/api/v3/display/on-demand/stop", {}),
("GET", "/api/v3/cache/list", None),
("GET", "/api/v3/plugins/installed", None),
("GET", "/api/v3/plugins/health", None),
("GET", "/api/v3/plugins/metrics/some-plugin", None),
("GET", "/api/v3/plugins/schema?plugin_id=some-plugin", None),
("GET", "/api/v3/plugins/store/list", None),
("GET", "/api/v3/plugins/saved-repositories", None),
("POST", "/api/v3/plugins/install", {"plugin_id": "some-plugin"}),
("POST", "/api/v3/plugins/config/reset?plugin_id=some-plugin", {}),
("GET", "/api/v3/plugins/limits/some-plugin", None),
("GET", "/api/v3/wifi/status", None),
("POST", "/api/v3/wifi/disconnect", {}),
]
@pytest.mark.parametrize("method,url,body", REMOVED_CATCH_ALLS,
ids=[f"{m} {u}" for m, u, _ in REMOVED_CATCH_ALLS])
def test_the_answer_is_what_the_catch_all_returned(client, caplog, method, url, body):
with caplog.at_level(logging.ERROR, logger="web_interface.blueprints.api_v3"):
resp = client.open(url, method=method, json=body)
assert resp.status_code == 500
assert resp.get_json() == EXPECTED
# The promise in the message: the traceback is in the log.
records = [r for r in caplog.records
if r.name == "web_interface.blueprints.api_v3"
and r.levelno >= logging.ERROR and r.exc_info]
assert records, "the unhandled exception was not logged with its traceback"
assert records[-1].exc_info[1] is FORCED
def test_credentials_are_redacted_from_the_detail(client):
body = client.get("/api/v3/plugins/installed").get_json()
for secret in ("SECRET123", "pw1", "K1"):
assert secret not in body["details"]
assert "<redacted>" in body["details"]
assert body["details"].startswith("RuntimeError: forced failure")
def test_a_client_error_keeps_its_own_status(client):
"""HTTPExceptions subclass Exception; a 415 must not become a 500."""
resp = client.post("/api/v3/plugins/assets/delete", data="not json",
content_type="text/plain")
assert resp.status_code == 415
body = resp.get_json()
assert body == {
'status': 'error',
'error_code': 'UNSUPPORTED_MEDIA_TYPE',
'message': body['message'],
}
assert "Content-Type" in body['message']
class TestInTheRealApp:
"""Mounted in web_interface/app.py, beside its global handlers."""
@pytest.fixture
def web_app(self):
import web_interface.app as web_app
return web_app
def test_the_blueprint_handler_answers_not_the_global_one(
self, web_app, exploding_managers):
resp = web_app.app.test_client().get("/api/v3/plugins/installed")
assert resp.status_code == 500
assert resp.get_json() == EXPECTED
def test_client_errors_read_the_same_as_the_global_handler(
self, web_app, exploding_managers):
"""The blueprint's 4xx shape must not drift from app.py's."""
resp = web_app.app.test_client().post(
"/api/v3/plugins/assets/delete", data="not json",
content_type="text/plain")
with web_app.app.test_request_context():
global_resp, global_status = web_app.handle_exception(
UnsupportedMediaType(description=resp.get_json()['message']))
assert resp.status_code == global_status == 415
assert resp.get_json() == global_resp.get_json()
def test_global_handler_shape(self, web_app):
"""Everything outside api_v3 still gets app.py's answer."""
with web_app.app.test_request_context("/somewhere"):
resp, status = web_app.handle_exception(FORCED)
body = resp.get_json()
assert status == 500
assert body == {
'status': 'error',
'error_code': 'UNKNOWN_ERROR',
'message': 'An error occurred; see logs for details',
'details': describe_exception(FORCED),
}
assert "SECRET123" not in body["details"]
class TestPluginActionStep1:
"""execute_plugin_action's OAuth step-1 handler reports the script's error.
The route bound a local `logger` in its JSON-parsing arm, which made
`logger` local to the whole function; every other `logger.error` in it
then raised UnboundLocalError. The step-1 handler therefore answered
"UnboundLocalError: cannot access local variable 'logger'" instead of
whatever the plugin's auth script actually raised.
"""
@pytest.fixture
def plugin_dir(self, tmp_path):
import json
d = tmp_path / "demo-plugin"
d.mkdir()
(d / "manifest.json").write_text(json.dumps({
"id": "demo-plugin",
"web_ui_actions": [{"id": "auth", "type": "script",
"script": "auth.py", "oauth_flow": True}],
}), encoding="utf-8")
(d / "auth.py").write_text(
"def get_auth_url():\n"
" raise RuntimeError('the auth script failed')\n",
encoding="utf-8")
return d
def test_the_script_error_reaches_the_response(self, plugin_dir, monkeypatch):
from unittest.mock import MagicMock
manager = MagicMock()
manager.get_plugin_directory.return_value = str(plugin_dir)
monkeypatch.setattr(api_v3, "plugin_manager", manager, raising=False)
app = Flask(__name__)
app.register_blueprint(api_v3, url_prefix="/api/v3")
resp = app.test_client().post(
"/api/v3/plugins/action",
json={"plugin_id": "demo-plugin", "action_id": "auth"})
assert resp.status_code == 500
body = resp.get_json()
assert body["details"] == "RuntimeError: the auth script failed"
assert body["message"] == 'An error occurred; see logs for details'
test/web_interface/test_cache.py
+145 -1
View File
@@ -1,9 +1,14 @@
"""Tests for the web interface's in-memory cache helpers."""
import sys
import threading
from typing import Iterator
import pytest
from web_interface.cache import delete_cached, get_cached, invalidate_cache, set_cached
from web_interface import cache as cache_module
from web_interface.cache import (
TTLCache, delete_cached, get_cached, invalidate_cache, set_cached,
)
@pytest.fixture(autouse=True)
@@ -44,3 +49,142 @@ def test_invalidate_cache_pattern() -> None:
invalidate_cache('fonts')
assert get_cached('fonts_catalog') is None
assert get_cached('plugins_list') == 2
# ---------------------------------------------------------------------------
# Expiry. set_cached used to accept ttl_seconds and ignore it; only the TTL a
# reader passed to get_cached counted, and get_cached defaulted to 60s.
# ---------------------------------------------------------------------------
class _Clock:
def __init__(self) -> None:
self.now = 1000.0
def __call__(self) -> float:
return self.now
@pytest.fixture
def clock() -> _Clock:
return _Clock()
def test_entry_expires_after_its_ttl(clock: _Clock) -> None:
c = TTLCache(clock=clock)
c.set('k', 'v', ttl=10)
clock.now += 9.9
assert c.get('k') == 'v'
clock.now += 0.1
assert c.get('k') is None
def test_default_ttl_applies_when_none_given(clock: _Clock) -> None:
c = TTLCache(default_ttl=5, clock=clock)
c.set('k', 'v')
clock.now += 4.9
assert c.get('k') == 'v'
clock.now += 0.1
assert c.get('k') is None
def test_reader_max_age_can_only_shorten(clock: _Clock) -> None:
c = TTLCache(clock=clock)
c.set('k', 'v', ttl=10)
clock.now += 5
assert c.get('k', max_age=6) == 'v'
assert c.get('k', max_age=5) is None
clock.now += 5
assert c.get('k', max_age=60) is None, "a reader extended a 10s entry"
def test_set_cached_ttl_is_honoured(monkeypatch: pytest.MonkeyPatch, clock: _Clock) -> None:
monkeypatch.setattr(cache_module, '_default_cache', TTLCache(clock=clock))
set_cached('short', 1, ttl_seconds=2)
set_cached('long', 2, ttl_seconds=300)
clock.now += 2
assert get_cached('short') is None, "set_cached ignored its ttl_seconds"
clock.now += 100 # past the old implicit 60s read default
assert get_cached('long') == 2
def test_get_cached_ttl_still_bounds_the_read(monkeypatch: pytest.MonkeyPatch, clock: _Clock) -> None:
"""The existing callers pass the TTL on both sides; that keeps working."""
monkeypatch.setattr(cache_module, '_default_cache', TTLCache(clock=clock))
set_cached('system_status', {'cpu': 1}, ttl_seconds=10)
clock.now += 9
assert get_cached('system_status', ttl_seconds=10) == {'cpu': 1}
clock.now += 1
assert get_cached('system_status', ttl_seconds=10) is None
def test_peek_returns_the_last_value_after_expiry(clock: _Clock) -> None:
c = TTLCache(clock=clock)
assert c.peek('k', 'fallback') == 'fallback'
c.set('k', True, ttl=1)
clock.now += 5
assert c.get('k') is None
assert c.peek('k', False) is True
def test_falsy_values_are_cached(clock: _Clock) -> None:
c = TTLCache(clock=clock)
c.set('k', False, ttl=10)
assert c.get('k', default='miss') is False
def test_clear_pattern_on_instance() -> None:
c = TTLCache()
c.set('fonts_catalog', 1)
c.set('system_status', 2)
c.clear('fonts')
assert c.peek('fonts_catalog') is None
assert c.get('system_status') == 2
c.clear()
assert c.peek('system_status') is None
def test_concurrent_expiry_reads_and_writes_do_not_raise() -> None:
"""The old dicts deleted expired keys inside get; two threads reading the
same expired key (or one reading while another invalidated) could raise
KeyError, which the endpoints turned into a 500."""
c = TTLCache()
keys = [f'k{n}' for n in range(8)]
errors = []
stop = threading.Event()
def reader() -> None:
try:
while not stop.is_set():
for key in keys:
c.get(key, max_age=0) # always expired for this reader
c.get(key)
c.peek(key)
c.clear('k1')
except Exception as exc: # pragma: no cover - the failure being tested
errors.append(exc)
def writer() -> None:
try:
for i in range(20000):
key = keys[i % len(keys)]
c.set(key, i, ttl=0 if i % 2 else 60)
if i % 7 == 0:
c.delete(key)
except Exception as exc: # pragma: no cover
errors.append(exc)
# Switch threads as often as possible so an unlocked check-then-act
# actually gets interleaved within the test's run time.
old_interval = sys.getswitchinterval()
sys.setswitchinterval(1e-6)
try:
readers = [threading.Thread(target=reader) for _ in range(4)]
for t in readers:
t.start()
writer()
stop.set()
for t in readers:
t.join()
finally:
sys.setswitchinterval(old_interval)
assert errors == []
test/web_interface/test_error_handler.py
+54 -1
View File
@@ -15,7 +15,7 @@ every api_v3 endpoint actually calls.
import pytest
from flask import Flask
from src.web_interface.api_helpers import success_response
from src.web_interface.api_helpers import exception_error_response, success_response
from src.web_interface.error_handler import (
create_error_response,
create_success_response,
@@ -64,6 +64,59 @@ class TestCreateErrorResponse:
assert response.get_json()["suggested_fixes"] == ["Try again"]
class TestExceptionErrorResponse:
"""The one-call form of from_exception() + error_response().
Nine plugin routes spelled the pair out by hand; these pin that the helper
answers exactly what that spelling did, so folding them changed nothing a
client sees.
"""
@staticmethod
def _by_hand(exc, code, with_context):
from src.web_interface.api_helpers import error_response
error = WebInterfaceError.from_exception(exc, code)
if with_context:
return error_response(error.error_code, error.message,
details=error.details, context=error.context,
status_code=500)
return error_response(error.error_code, error.message,
details=error.details, status_code=500)
@pytest.mark.parametrize("with_context", [True, False])
@pytest.mark.parametrize("code", [ErrorCode.SYSTEM_ERROR,
ErrorCode.CONFIG_SAVE_FAILED,
ErrorCode.PLUGIN_UPDATE_FAILED])
def test_same_answer_as_the_hand_written_pair(self, app, code, with_context):
exc = ValueError("token=SECRET boom")
exc.context = {"config_path": "/etc/x.json"}
with app.test_request_context():
got, got_status = exception_error_response(
exc, code, with_context=with_context)
want, want_status = self._by_hand(exc, code, with_context)
assert got_status == want_status == 500
assert got.get_json() == want.get_json()
def test_shape(self, app):
with app.test_request_context():
response, status = exception_error_response(
RuntimeError("token=SECRET"), ErrorCode.SYSTEM_ERROR)
assert status == 500
assert response.get_json() == {
"status": "error",
"error_code": "SYSTEM_ERROR",
"message": "A system error occurred",
"context": {"exception_type": "RuntimeError"},
"suggested_fixes": ["Review error details and try again"],
}
def test_without_context(self, app):
with app.test_request_context():
response, _ = exception_error_response(
RuntimeError("x"), ErrorCode.SYSTEM_ERROR, with_context=False)
assert "context" not in response.get_json()
class TestCreateSuccessResponse:
def test_bare_success(self):
assert create_success_response() == {"status": "success"}
test/web_interface/test_errors.py
+12 -59
View File
@@ -1,7 +1,7 @@
"""
Tests for src/web_interface/errors.py — the structured error type behind
every API error response (category inference, default suggestions, the
JSON shape, and exception conversion).
every API error response (default suggestions, the JSON shape, and
exception conversion).
Pure logic; no Flask context needed.
@@ -11,39 +11,7 @@ caller passing [] to mean "no suggestions" silently got the default list.
import pytest
from src.web_interface.errors import ErrorCategory, ErrorCode, WebInterfaceError
class TestCategoryInference:
@pytest.mark.parametrize("code,expected", [
(ErrorCode.CONFIG_SAVE_FAILED, ErrorCategory.CONFIGURATION),
(ErrorCode.CONFIG_ROLLBACK_FAILED, ErrorCategory.CONFIGURATION),
(ErrorCode.PLUGIN_NOT_FOUND, ErrorCategory.PLUGIN),
(ErrorCode.PLUGIN_OPERATION_CONFLICT, ErrorCategory.PLUGIN),
(ErrorCode.VALIDATION_ERROR, ErrorCategory.VALIDATION),
(ErrorCode.SCHEMA_VALIDATION_FAILED, ErrorCategory.VALIDATION),
(ErrorCode.INVALID_INPUT, ErrorCategory.VALIDATION),
(ErrorCode.NETWORK_ERROR, ErrorCategory.NETWORK),
(ErrorCode.API_ERROR, ErrorCategory.NETWORK),
(ErrorCode.TIMEOUT, ErrorCategory.NETWORK),
(ErrorCode.PERMISSION_DENIED, ErrorCategory.PERMISSION),
(ErrorCode.FILE_PERMISSION_ERROR, ErrorCategory.PERMISSION),
(ErrorCode.SYSTEM_ERROR, ErrorCategory.SYSTEM),
(ErrorCode.SERVICE_UNAVAILABLE, ErrorCategory.SYSTEM),
(ErrorCode.UNKNOWN_ERROR, ErrorCategory.UNKNOWN),
])
def test_every_code_prefix_maps_to_its_category(self, code, expected):
assert WebInterfaceError(code, "msg").category is expected
def test_explicit_category_overrides_inference(self):
error = WebInterfaceError(
ErrorCode.CONFIG_SAVE_FAILED, "msg", category=ErrorCategory.SYSTEM)
assert error.category is ErrorCategory.SYSTEM
def test_every_error_code_gets_a_category(self):
# No code may fall through uncategorized as the enum grows.
for code in ErrorCode:
assert isinstance(WebInterfaceError(code, "msg").category, ErrorCategory)
from src.web_interface.errors import ErrorCode, WebInterfaceError
class TestDefaultSuggestions:
@@ -79,8 +47,9 @@ class TestToDict:
result = WebInterfaceError(ErrorCode.SYSTEM_ERROR, "boom").to_dict()
assert result["status"] == "error"
assert result["error_code"] == "SYSTEM_ERROR"
assert result["error_category"] == "system"
assert result["message"] == "boom"
# No error_category: nothing in the UI, tests or plugins ever read it.
assert set(result) == {"status", "error_code", "message", "suggested_fixes"}
def test_details_included_when_set(self):
result = WebInterfaceError(
@@ -116,23 +85,7 @@ class TestToDict:
class TestFromException:
@pytest.mark.parametrize("exc_name,expected", [
("ConfigError", ErrorCode.CONFIG_LOAD_FAILED),
("PluginError", ErrorCode.PLUGIN_LOAD_FAILED),
("PermissionError", ErrorCode.PERMISSION_DENIED),
("AccessDenied", ErrorCode.PERMISSION_DENIED),
("ValidationError", ErrorCode.VALIDATION_ERROR),
("SchemaError", ErrorCode.VALIDATION_ERROR),
("NetworkError", ErrorCode.NETWORK_ERROR),
("ConnectionError", ErrorCode.NETWORK_ERROR),
("TimeoutError", ErrorCode.TIMEOUT),
("SomethingElse", ErrorCode.UNKNOWN_ERROR),
])
def test_code_inferred_from_exception_class_name(self, exc_name, expected):
exc = type(exc_name, (Exception,), {})("boom")
assert WebInterfaceError.from_exception(exc).error_code is expected
def test_explicit_code_skips_inference(self):
def test_the_given_code_is_reported(self):
error = WebInterfaceError.from_exception(
ValueError("boom"), error_code=ErrorCode.PLUGIN_NOT_FOUND)
assert error.error_code is ErrorCode.PLUGIN_NOT_FOUND
@@ -140,28 +93,28 @@ class TestFromException:
def test_message_is_the_safe_one_not_the_exception_text(self):
# The raw exception text is not echoed into `message`; that field is
# a fixed, user-facing string per code.
error = WebInterfaceError.from_exception(ValueError("secret-ish detail"))
error = WebInterfaceError.from_exception(ValueError("secret-ish detail"), ErrorCode.UNKNOWN_ERROR)
assert error.message == "An unexpected error occurred"
assert "secret-ish" not in error.message
def test_exception_type_recorded_in_context(self):
error = WebInterfaceError.from_exception(ValueError("boom"))
error = WebInterfaceError.from_exception(ValueError("boom"), ErrorCode.UNKNOWN_ERROR)
assert error.context["exception_type"] == "ValueError"
def test_caller_context_is_preserved_alongside_type(self):
error = WebInterfaceError.from_exception(
ValueError("boom"), context={"plugin_id": "clock"})
ValueError("boom"), ErrorCode.UNKNOWN_ERROR, context={"plugin_id": "clock"})
assert error.context["plugin_id"] == "clock"
assert error.context["exception_type"] == "ValueError"
def test_caller_supplied_exception_type_is_overwritten(self):
error = WebInterfaceError.from_exception(
ValueError("boom"), context={"exception_type": "Fake"})
ValueError("boom"), ErrorCode.UNKNOWN_ERROR, context={"exception_type": "Fake"})
assert error.context["exception_type"] == "ValueError"
def test_original_error_retained(self):
exc = ValueError("boom")
assert WebInterfaceError.from_exception(exc).original_error is exc
assert WebInterfaceError.from_exception(exc, ErrorCode.UNKNOWN_ERROR).original_error is exc
def test_every_code_has_a_safe_message(self):
for code in ErrorCode:
@@ -205,4 +158,4 @@ class TestExceptionDetails:
def test_details_flow_into_from_exception(self):
exc = ValueError("boom")
exc.context = {"config_path": "/etc/x.json"}
assert "config_path" in WebInterfaceError.from_exception(exc).details
assert "config_path" in WebInterfaceError.from_exception(exc, ErrorCode.UNKNOWN_ERROR).details
test/web_interface/test_systemctl_sudoers_alignment.py
+4 -3
View File
@@ -1,5 +1,6 @@
"""Guards that every privileged systemctl call the web interface makes is
covered by a passwordless-sudo grant in configure_web_sudo.sh.
covered by a passwordless-sudo grant in scripts/install/lib_sudoers.sh, which
both first_time_install.sh and configure_web_sudo.sh write the rules from.
The web interface runs headless (no TTY), so any `sudo` call that is not
matched by a NOPASSWD rule in /etc/sudoers.d/ledmatrix_web falls back to a
@@ -25,7 +26,7 @@ API_V3_PKG = PROJECT_ROOT / "web_interface" / "blueprints" / "api_v3"
def _api_v3_source() -> str:
return "\n".join(p.read_text() for p in sorted(API_V3_PKG.glob("*.py")))
SUDOERS_SCRIPT = PROJECT_ROOT / "scripts" / "install" / "configure_web_sudo.sh"
SUDOERS_SCRIPT = PROJECT_ROOT / "scripts" / "install" / "lib_sudoers.sh"
def _sudo_systemctl_calls(source: str) -> set[tuple[str, str]]:
@@ -64,7 +65,7 @@ def test_every_sudo_systemctl_call_is_granted() -> None:
uncovered = {c for c in calls if c not in rules}
assert not uncovered, (
"These sudo systemctl calls have no matching NOPASSWD grant in "
"configure_web_sudo.sh; they will fail headless with "
"lib_sudoers.sh; they will fail headless with "
"'sudo: a terminal is required to read the password': "
+ ", ".join(f"systemctl {v} {u}" for v, u in sorted(uncovered))
)
test/web_interface/test_web_logging.py
+179
View File
@@ -0,0 +1,179 @@
"""The web process logs the way the display process does.
web_interface/app.py used to call its own setup (web_interface/logging_config.py)
which replaced the root handlers with a plain stdout formatter. Under systemd
every line then reached the journal as PRIORITY=6, so
journalctl -p err -u ledmatrix-web
showed nothing while the web interface was logging errors. It also logged
every request at INFO, including what the UI polls: the journal on a Pi showed
``GET /api/v3/errors/summary - 200`` every minute per open tab.
"""
import logging
import os
import subprocess
import sys
import textwrap
from pathlib import Path
import pytest
from flask import Flask
from web_interface import request_logging
PROJECT_ROOT = Path(__file__).resolve().parents[2]
# ---------------------------------------------------------------------------
# The real app, imported the way systemd runs it
# ---------------------------------------------------------------------------
_CHILD = textwrap.dedent("""
import logging
import web_interface.app as web_app
# Startup reconciliation may try to reinstall plugins; not this test's job.
web_app._reconciliation_started = True
client = web_app.app.test_client()
client.get('/api/v3/errors/summary')
client.get('/favicon.ico')
client.get('/api/v3/no-such-endpoint')
logging.getLogger('web_interface.probe').error('probe error line')
logging.getLogger('web_interface.probe').info('probe info line')
""")
@pytest.fixture(scope="module")
def journal_output(tmp_path_factory):
"""Run the child with stdout as a file systemd would call the journal.
systemd sets JOURNAL_STREAM to the dev:ino of the stream it captures;
src.logging_config only adds priorities when stdout really is that stream,
so hand the child a file and name that file's dev:ino.
"""
out_path = tmp_path_factory.mktemp("journal") / "stdout.txt"
with open(out_path, "wb") as out:
st = os.fstat(out.fileno())
env = dict(os.environ)
env.update({
"JOURNAL_STREAM": f"{st.st_dev}:{st.st_ino}",
"PYTHONUTF8": "1",
"EMULATOR": "true",
"PYTHONPATH": str(PROJECT_ROOT),
})
env.pop("LEDMATRIX_DEBUG", None)
env.pop("LEDMATRIX_JSON_LOGGING", None)
proc = subprocess.run(
[sys.executable, "-c", _CHILD], cwd=str(PROJECT_ROOT), env=env,
stdout=out, stderr=subprocess.PIPE, timeout=180,
)
text = out_path.read_text(encoding="utf-8", errors="replace")
assert proc.returncode == 0, proc.stderr.decode(errors="replace")[-4000:]
return text.splitlines()
def test_error_reaches_the_journal_as_err(journal_output):
lines = [l for l in journal_output if "probe error line" in l]
assert lines, "\n".join(journal_output[-40:])
assert lines[0].startswith("<3>"), lines[0]
# Same readable shape as the display service (and what the log viewer strips).
assert " - ERROR - web_interface.probe - probe error line" in lines[0]
def test_info_reaches_the_journal_as_info(journal_output):
lines = [l for l in journal_output if "probe info line" in l]
assert lines and lines[0].startswith("<6>"), journal_output[-40:]
def test_polling_gets_are_not_logged_at_info(journal_output):
for path in ("/api/v3/errors/summary", "/favicon.ico"):
assert not [l for l in journal_output if f"GET {path} " in l], (
f"a successful GET {path} was logged by default")
def test_failed_request_is_still_logged(journal_output):
lines = [l for l in journal_output if "GET /api/v3/no-such-endpoint - 404" in l]
assert lines and lines[0].startswith("<4>"), journal_output[-40:]
# ---------------------------------------------------------------------------
# The level policy
# ---------------------------------------------------------------------------
@pytest.mark.parametrize("method,status,level", [
("GET", 200, logging.DEBUG),
("GET", 304, logging.DEBUG),
("HEAD", 200, logging.DEBUG),
("OPTIONS", 204, logging.DEBUG),
("get", 200, logging.DEBUG),
("POST", 200, logging.INFO),
("PUT", 204, logging.INFO),
("DELETE", 200, logging.INFO),
("PATCH", 302, logging.INFO),
("GET", 404, logging.WARNING),
("POST", 400, logging.WARNING),
("GET", 500, logging.ERROR),
("POST", 503, logging.ERROR),
])
def test_request_log_level(method, status, level):
assert request_logging.request_log_level(method, status) == level
@pytest.fixture
def tiny_app():
app = Flask(__name__)
request_logging.init_app(app)
@app.route("/poll")
def poll():
return "ok"
@app.route("/save", methods=["POST"])
def save():
return "saved"
@app.route("/boom")
def boom():
return "no", 500
return app.test_client()
def test_hooks_log_each_request_once_at_its_level(tiny_app, caplog):
caplog.set_level(logging.DEBUG, logger="web_interface.api")
tiny_app.get("/poll")
tiny_app.post("/save")
tiny_app.get("/boom")
tiny_app.get("/missing")
got = [(r.levelno, r.getMessage().split(" (")[0]) for r in caplog.records
if r.name == "web_interface.api"]
assert got == [
(logging.DEBUG, "GET /poll - 200"),
(logging.INFO, "POST /save - 200"),
(logging.ERROR, "GET /boom - 500"),
(logging.WARNING, "GET /missing - 404"),
]
def test_duration_is_rounded(tiny_app, caplog):
caplog.set_level(logging.DEBUG, logger="web_interface.api")
tiny_app.post("/save")
msg = caplog.records[-1].getMessage()
duration = msg.rsplit("(", 1)[1]
assert duration.endswith("ms)") and len(duration.split(".")[1]) == len("0ms)"), msg
def test_success_response_timing_uses_the_same_clock():
# request_logging stamps request.start_time from perf_counter; a reader
# subtracting it from time.time() reported ~1.8e12 ms (found on a Pi).
from src.web_interface.api_helpers import success_response
app = Flask(__name__)
request_logging.init_app(app)
@app.route('/timed')
def timed():
return success_response(data={}, metadata={})
body = app.test_client().get('/timed').get_json()
assert 0 <= body['metadata']['response_time_ms'] < 10_000
web_interface/app.py
+48 -66
View File
@@ -16,6 +16,17 @@ from datetime import datetime, timedelta
# Add parent directory to path for imports
sys.path.insert(0, str(Path(__file__).parent.parent))
# Configure logging before anything below logs: the same setup as the display
# service (run.py), so this process's journal lines carry their real syslog
# priority too (`journalctl -p err -u ledmatrix-web`). LEDMATRIX_DEBUG=true
# turns on DEBUG, which includes the routine per-request lines.
from src.logging_config import setup_logging
setup_logging(format_type=(
'json' if os.environ.get('LEDMATRIX_JSON_LOGGING', 'false').lower() == 'true'
else 'readable'))
logging.getLogger('werkzeug').setLevel(logging.WARNING) # request_logging covers requests
logging.getLogger('urllib3').setLevel(logging.WARNING)
from src.config_manager import ConfigManager
from src.web_interface.error_handler import describe_exception
from src.common.path_safety import (
@@ -284,34 +295,47 @@ try:
except ImportError:
pass
# Cached AP mode check — avoids creating a WiFiManager per request
_ap_mode_cache = {'value': False, 'timestamp': 0}
# systemctl answers, memoised so they are not a subprocess fork per request
# (AP mode) or per SSE tick (display service). A failed check keeps the last
# known answer for the same TTL rather than retrying on every request.
from web_interface.cache import TTLCache
_service_status_cache = TTLCache()
_AP_MODE_CACHE_TTL = 30 # seconds — AP mode is user-initiated; 30s is fine
# Cached ledmatrix service status for SSE stats stream
_ledmatrix_service_cache = {'active': False, 'timestamp': 0}
_LEDMATRIX_SERVICE_CACHE_TTL = 15 # seconds
# The only units _unit_is_active() may ask systemctl about: its argv is built
# from these literals, never from request data.
_CHECKABLE_UNITS = frozenset({'hostapd', 'ledmatrix'})
def _unit_is_active(unit, ttl):
"""`systemctl is-active <unit>`, cached for ``ttl`` seconds.
False where there is no systemctl (a dev machine); on a failed check, the
last known answer.
"""
if unit not in _CHECKABLE_UNITS:
raise ValueError(f"not a checkable unit: {unit!r}")
active = _service_status_cache.get(unit)
if active is not None:
return active
active = _service_status_cache.peek(unit, False)
if _SYSTEMCTL:
try:
result = subprocess.run([_SYSTEMCTL, 'is-active', unit], # nosec B603 - list argv, unit is from _CHECKABLE_UNITS # nosemgrep
capture_output=True, text=True, timeout=2)
active = result.stdout.strip() == 'active'
except (subprocess.SubprocessError, OSError) as e:
logging.getLogger('web_interface').warning(
"systemctl is-active %s failed: %s", unit, e)
_service_status_cache.set(unit, active, ttl=ttl)
return active
def is_ap_mode_active():
"""
Check if access point mode is currently active (cached, 30s TTL).
Uses a direct systemctl check instead of instantiating WiFiManager.
"""
now = time.time()
if (now - _ap_mode_cache['timestamp']) < _AP_MODE_CACHE_TTL:
return _ap_mode_cache['value']
try:
result = subprocess.run(
['systemctl', 'is-active', 'hostapd'],
capture_output=True, text=True, timeout=2
)
active = result.stdout.strip() == 'active'
_ap_mode_cache['value'] = active
_ap_mode_cache['timestamp'] = now
return active
except (subprocess.SubprocessError, OSError) as e:
logging.getLogger('web_interface').error(f"AP mode check failed: {e}")
return _ap_mode_cache['value']
return _unit_is_active('hostapd', _AP_MODE_CACHE_TTL)
# Captive portal detection endpoints
# When AP mode is active, return responses that TRIGGER the captive portal popup.
@@ -346,41 +370,9 @@ def success_txt():
return redirect(url_for('pages_v3.captive_setup'), code=302)
return 'success', 200
# Initialize logging
try:
from web_interface.logging_config import setup_web_interface_logging, log_api_request
# Use JSON logging in production, readable logs in development
use_json_logging = os.environ.get('LEDMATRIX_JSON_LOGGING', 'false').lower() == 'true'
setup_web_interface_logging(level='INFO', use_json=use_json_logging)
except ImportError:
# Logging config not available, use default
log_api_request = None
# Request timing and logging middleware
@app.before_request
def before_request():
"""Track request start time for logging."""
from flask import request
request.start_time = time.time()
@app.after_request
def after_request_logging(response):
"""Log API requests after response."""
if log_api_request:
try:
from flask import request
duration_ms = (time.time() - getattr(request, 'start_time', time.time())) * 1000
ip_address = request.remote_addr if hasattr(request, 'remote_addr') else None
log_api_request(
method=request.method,
path=request.path,
status_code=response.status_code,
duration_ms=duration_ms,
ip_address=ip_address
)
except Exception: # nosec B110 - request logging must never interrupt a live HTTP response
pass # Don't break response if logging fails
return response
# Request timing and logging (routine reads at DEBUG; see request_logging)
from web_interface import request_logging
request_logging.init_app(app)
# Global error handlers
@app.errorhandler(404)
@@ -693,17 +685,7 @@ def system_status_generator():
cpu_temp = metrics['cpu_temp']
# Check if display service is running (cached to avoid per-client subprocess forks)
now = time.time()
if (now - _ledmatrix_service_cache['timestamp']) >= _LEDMATRIX_SERVICE_CACHE_TTL:
if _SYSTEMCTL:
try:
result = subprocess.run([_SYSTEMCTL, 'is-active', 'ledmatrix'],
capture_output=True, text=True, timeout=2)
_ledmatrix_service_cache['active'] = result.stdout.strip() == 'active'
except (subprocess.SubprocessError, OSError) as e:
app.logger.warning("systemctl status check failed: %s", e)
_ledmatrix_service_cache['timestamp'] = now
service_active = _ledmatrix_service_cache['active']
service_active = _unit_is_active('ledmatrix', _LEDMATRIX_SERVICE_CACHE_TTL)
status = {
'timestamp': time.time(),
web_interface/auto_update.py
+6 -1
View File
@@ -172,6 +172,7 @@ def _plugin_fingerprint(store_manager, plugin_dir):
try:
with open(plugin_dir / 'manifest.json', 'r', encoding='utf-8') as f:
manifest = json.load(f)
if isinstance(manifest, dict): # valid JSON need not be an object
if manifest.get('local_only'):
return None
version = manifest.get('version')
@@ -190,8 +191,12 @@ def update_plugins(store_manager, operation_history=None):
"""Update every installed plugin that has an update. Returns (updated, failed)."""
updated, failed = [], []
plugins_dir = Path(store_manager.plugins_dir)
# list_installed_plugins() reports manifest ids, and a plugin's directory
# may be named differently (ledmatrix-stocks/ holding id "stocks"), so
# the directory comes from the store's own lookup, not a join.
find_dir = getattr(store_manager, '_find_plugin_path', None)
for plugin_id in sorted(store_manager.list_installed_plugins()):
plugin_dir = plugins_dir / plugin_id
plugin_dir = (find_dir(plugin_id) if find_dir else None) or plugins_dir / plugin_id
before = _plugin_fingerprint(store_manager, plugin_dir)
if before is None:
continue # local_only: managed by hand, never from the registry
web_interface/blueprints/api_v3/__init__.py
+43 -3
View File
@@ -35,13 +35,16 @@ from typing import Dict, Any, Optional, Tuple, Type
from urllib.parse import urlparse, urlunparse
logger = logging.getLogger(__name__)
# Import new infrastructure
from src.web_interface.api_helpers import success_response, error_response, validate_request_json
from src.web_interface.api_helpers import (success_response, error_response,
exception_error_response, validate_request_json)
from src.web_interface.errors import ErrorCode
from src.web_interface.secret_helpers import (find_secret_fields, mask_all_secret_values,
merge_secrets, remove_empty_secrets,
separate_secrets,
strip_masked_values)
from src.web_interface.error_handler import describe_exception, redact_text
from src.web_interface.error_handler import (describe_exception, http_exception_payload,
redact_text, unhandled_exception_payload)
from werkzeug.exceptions import HTTPException
from src.plugin_system.operation_types import OperationType
from src.web_interface.validators import (
validate_file_upload
@@ -114,6 +117,43 @@ SYSTEM_FONTS = frozenset([
'clr6x12', 'helvr12', 'texgyre-27'
])
api_v3 = Blueprint('api_v3', __name__)
@api_v3.errorhandler(Exception)
def _api_v3_unhandled_exception(error):
"""The answer for any exception an api_v3 route does not handle itself.
Fifty-odd routes used to end in the same four lines -- log the traceback,
return {status, message: "An error occurred; see logs for details",
details: describe_exception(e)} with a 500. This is those four lines, once.
A route still catches for itself when its failure needs something else: a
specific message, extra keys, an operation-history record, or cleanup.
It is registered on the blueprint, not left to web_interface/app.py's
global handler, because the two answers differ: the global one adds
`error_code: UNKNOWN_ERROR`, and the plugin API client treats a body with
an error_code differently from one without (see api_client.js). Tests that
mount this blueprint on a bare Flask app get the same answer as the real
app does, too.
`details` is describe_exception(), which redacts credentials and caps the
length. CodeQL reads returning it as stack-trace exposure; it is the
project's deliberate trade-off, because a device whose storage is failing
otherwise answers "see logs for details" from the log viewer too
(test_web_error_detail.py).
Werkzeug's HTTPExceptions subclass Exception, so a 400/405/413/415 raised
inside a route lands here as well; it goes back as itself, in the global
handler's shape. A 404 or explicit 500 never arrives: Flask prefers the
app's code-specific handlers over a blueprint's class-based one.
"""
if isinstance(error, HTTPException):
return jsonify(http_exception_payload(error)), error.code or 500
logger.error("Unhandled exception in %s", request.endpoint or request.path,
exc_info=error)
return jsonify(unhandled_exception_payload(error)), 500
def _get_plugin_version(plugin_id: str) -> str:
"""Read the installed version from a plugin's manifest.json.
@@ -1712,7 +1752,7 @@ def _get_starlark_device_location() -> DeviceLocationResolver:
global _starlark_device_location
if _starlark_device_location is None:
_starlark_device_location = DeviceLocationResolver(
getattr(api_v3, 'cache_manager', None) or _ensure_cache_manager(), logger)
getattr(api_v3, 'cache_manager', None), logger)
return _starlark_device_location
web_interface/blueprints/api_v3/config.py
-8
View File
@@ -47,15 +47,11 @@ def _day_setting(data, day, flat_key, nested_key):
@api_v3.route('/config/main', methods=['GET'])
def get_main_config():
"""Get main configuration, with credentials redacted."""
try:
if not api_v3.config_manager:
return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500
config = api_v3.config_manager.load_config()
return jsonify({'status': 'success', 'data': _redact_credentials(config)})
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/config/schedule', methods=['GET'])
def get_schedule_config():
"""Get current schedule configuration"""
@@ -1165,7 +1161,6 @@ def save_main_config():
@api_v3.route('/config/secrets', methods=['GET'])
def get_secrets_config():
"""Get secrets configuration"""
try:
if not api_v3.config_manager:
return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500
@@ -1176,9 +1171,6 @@ def get_secrets_config():
# alone so a client can still tell "set" from "not set".
return jsonify({'status': 'success',
'data': mask_all_secret_values(config)})
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/config/raw/main', methods=['POST'])
def save_raw_main_config():
"""Save raw main configuration JSON"""
web_interface/blueprints/api_v3/display.py
+1 -31
View File
@@ -6,7 +6,7 @@ endpoint names are unchanged by living here.
from web_interface.blueprints.api_v3 import (
_ensure_display_service_running,
_get_display_service_status, _stop_display_service, api_v3,
describe_exception, jsonify, logger, os, request, uuid,
jsonify, logger, os, request, uuid,
)
import web_interface.blueprints.api_v3 as _pkg
# Read through the module rather than bound by value: tests patch these
@@ -31,7 +31,6 @@ def _cache_manager():
@api_v3.route('/display/current', methods=['GET'])
def get_display_current():
"""Get current display state"""
try:
import base64
from PIL import Image
import io
@@ -67,9 +66,6 @@ def get_display_current():
'image': image_data # Base64 encoded image data or None if unavailable
}
return jsonify({'status': 'success', 'data': display_data})
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/display/modes', methods=['GET'])
def get_display_modes():
"""Every display mode that can be requested on-demand, with its plugin.
@@ -92,7 +88,6 @@ def get_display_modes():
for the duration -- so they are reported with enabled: false
rather than omitted.
"""
try:
if not api_v3.plugin_manager:
return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500
@@ -137,19 +132,9 @@ def get_display_modes():
})
return jsonify({'status': 'success', 'data': {'modes': modes}})
except Exception as exc:
# describe_exception, not a bare message: test_web_error_detail.py
# enforces that every handler here returns it, because a device whose
# storage is failing otherwise answers "see logs for details" from the
# log viewer too. It redacts credentials out of the exception text.
# CodeQL flags this as stack-trace exposure across all ~75 handlers;
# it is the project's deliberate, reviewed trade-off.
logger.error('Error in get_display_modes', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(exc)}), 500
@api_v3.route('/display/on-demand/status', methods=['GET'])
def get_on_demand_status():
"""Return the current on-demand display state."""
try:
cache = _cache_manager()
# memory_ttl=0: the display service writes this key, so only the file
# is current. This process's memory tier would keep serving the first
@@ -170,13 +155,9 @@ def get_on_demand_status():
'service': service_status
}
})
except Exception as exc:
logger.error('Error in get_on_demand_status', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(exc)}), 500
@api_v3.route('/display/on-demand/start', methods=['POST'])
def start_on_demand_display():
"""Request the display controller to run a specific plugin on-demand."""
try:
data = request.get_json(silent=True) or {}
plugin_id = data.get('plugin_id')
mode = data.get('mode')
@@ -278,13 +259,9 @@ def start_on_demand_display():
'service': service_result
}
return jsonify({'status': 'success', 'data': response_data})
except Exception as exc:
logger.error('Error in start_on_demand_display', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(exc)}), 500
@api_v3.route('/display/on-demand/stop', methods=['POST'])
def stop_on_demand_display():
"""Request the display controller to stop on-demand mode."""
try:
data = request.get_json(silent=True) or {}
stop_service = data.get('stop_service', False)
@@ -313,9 +290,6 @@ def stop_on_demand_display():
'service': service_result
}
})
except Exception as exc:
logger.error('Error in stop_on_demand_display', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(exc)}), 500
@api_v3.route('/display/current-status', methods=['GET'])
def get_current_display_status():
"""Return the display mode/plugin currently intended to be shown.
@@ -325,7 +299,6 @@ def get_current_display_status():
System Logs page) can show what's on screen without querying the display
process directly.
"""
try:
cache = _cache_manager()
# memory_ttl=0: written by the display service; see get_on_demand_status.
state = cache.get('display_current_state', max_age=120, memory_ttl=0)
@@ -336,6 +309,3 @@ def get_current_display_status():
'last_updated': None,
}
return jsonify({'status': 'success', 'data': state})
except Exception as e:
logger.error('Error in get_current_display_status', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
web_interface/blueprints/api_v3/fonts.py
+1 -23
View File
@@ -4,7 +4,7 @@ Routes decorate the shared `api_v3` Blueprint from ._common, so their
endpoint names are unchanged by living here.
"""
from web_interface.blueprints.api_v3 import (
PROJECT_ROOT, Path, Response, SYSTEM_FONTS, api_v3, describe_exception,
PROJECT_ROOT, Path, Response, SYSTEM_FONTS, api_v3,
jsonify, logger, os, re, request, validate_file_upload,
)
@@ -52,7 +52,6 @@ def _catalog_response(catalog):
@api_v3.route('/fonts/catalog', methods=['GET'])
def get_fonts_catalog():
"""Get fonts catalog"""
try:
# Check cache first (5 minute TTL)
try:
from web_interface.cache import get_cached, set_cached
@@ -157,15 +156,9 @@ def get_fonts_catalog():
logger.error("[FontCatalog] Failed to cache fonts_catalog", exc_info=True)
return _catalog_response(catalog)
except Exception as e:
logger.error("%s failed", request.path, exc_info=True)
return jsonify({'status': 'error',
'message': 'An error occurred; see logs for details',
'details': describe_exception(e)}), 500
@api_v3.route('/fonts/tokens', methods=['GET'])
def get_font_tokens():
"""Get font size tokens"""
try:
# This would integrate with the actual font system
# For now, return sample tokens
tokens = {
@@ -177,13 +170,9 @@ def get_font_tokens():
'xxl': 16
}
return jsonify({'status': 'success', 'data': {'tokens': tokens}})
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/fonts/upload', methods=['POST'])
def upload_font():
"""Upload font file"""
try:
if 'font_file' not in request.files:
return jsonify({'status': 'error', 'message': 'No font file provided'}), 400
@@ -256,13 +245,9 @@ def upload_font():
'filename': safe_filename,
'path': f'assets/fonts/{safe_filename}'
})
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/fonts/preview', methods=['GET'])
def get_font_preview() -> tuple[Response, int] | Response:
"""Generate a preview image of text rendered with a specific font"""
try:
from PIL import Image, ImageDraw, ImageFont
import io
import base64
@@ -399,13 +384,9 @@ def get_font_preview() -> tuple[Response, int] | Response:
'height': img_height
}
})
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/fonts/<font_family>', methods=['DELETE'])
def delete_font(font_family: str) -> tuple[Response, int] | Response:
"""Delete a user-uploaded font file"""
try:
# Security: Validate font_family to prevent path traversal
# Reject if it contains path separators or ..
if '..' in font_family or '/' in font_family or '\\' in font_family:
@@ -485,6 +466,3 @@ def delete_font(font_family: str) -> tuple[Response, int] | Response:
'status': 'success',
'message': f'Font {deleted_filename} deleted successfully'
})
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
web_interface/blueprints/api_v3/misc.py
-15
View File
@@ -186,13 +186,6 @@ def get_logs():
'status': 'error',
'message': 'Timeout while fetching logs'
}), 500
except Exception as e:
logger.error("%s failed", request.path, exc_info=True)
return jsonify({
'status': 'error',
'message': 'An error occurred; see logs for details',
'details': describe_exception(e)
}), 500
# Multi-Display Sync Endpoints
@api_v3.route('/sync/status', methods=['GET'])
def get_sync_status():
@@ -231,7 +224,6 @@ def get_sync_status():
@api_v3.route('/cache/list', methods=['GET'])
def list_cache_files():
"""List all cache files with metadata"""
try:
if not api_v3.cache_manager:
# Initialize cache manager if not already initialized
from src.cache_manager import CacheManager
@@ -248,13 +240,9 @@ def list_cache_files():
'total_files': len(cache_files)
}
})
except Exception as e:
logger.error('Error in list_cache_files', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/cache/delete', methods=['POST'])
def delete_cache_file():
"""Delete a specific cache file by key"""
try:
if not api_v3.cache_manager:
# Initialize cache manager if not already initialized
from src.cache_manager import CacheManager
@@ -279,9 +267,6 @@ def delete_cache_file():
'status': 'success',
'message': f'Cache file for key "{cache_key}" deleted successfully'
})
except Exception as e:
logger.error('Error in delete_cache_file', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
def _errors_cache():
"""The shared cache the display service publishes its errors to."""
if not api_v3.cache_manager:
web_interface/blueprints/api_v3/plugins.py
+14 -181
View File
@@ -14,6 +14,7 @@ from web_interface.blueprints.api_v3 import (
_set_missing_booleans_to_false,
_set_nested_value, _starlark_virtual_plugins, _toggle_starlark_app,
api_v3, datetime, deep_merge, describe_exception, error_response,
exception_error_response,
find_secret_fields, hashlib, json, jsonify, logger, logging,
merge_secrets, os, redact_text, remove_empty_secrets, request,
separate_secrets, shutil, stat, subprocess, success_response,
@@ -32,7 +33,6 @@ import web_interface.blueprints.api_v3 as _pkg
@api_v3.route('/plugins/installed', methods=['GET'])
def get_installed_plugins():
"""Get installed plugins"""
try:
if not api_v3.plugin_manager or not api_v3.plugin_store_manager:
return jsonify({'status': 'error', 'message': 'Plugin managers not initialized'}), 500
@@ -174,13 +174,9 @@ def get_installed_plugins():
plugins.extend(_starlark_virtual_plugins())
return jsonify({'status': 'success', 'data': {'plugins': plugins}})
except Exception as e:
logger.error('Error in get_installed_plugins', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/health', methods=['GET'])
def get_plugin_health():
"""Get health metrics for all plugins"""
try:
if not api_v3.plugin_manager:
return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500
@@ -214,13 +210,9 @@ def get_plugin_health():
'status': 'success',
'data': health_summaries
})
except Exception as e:
logger.error('Error in get_plugin_health', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/health/<plugin_id>', methods=['GET'])
def get_plugin_health_single(plugin_id):
"""Get health metrics for a specific plugin"""
try:
if not api_v3.plugin_manager:
return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500
@@ -238,13 +230,9 @@ def get_plugin_health_single(plugin_id):
'status': 'success',
'data': health_summary
})
except Exception as e:
logger.error('Error in get_plugin_health_single', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/health/<plugin_id>/reset', methods=['POST'])
def reset_plugin_health(plugin_id):
"""Reset health state for a plugin (manual recovery)"""
try:
if not api_v3.plugin_manager:
return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500
@@ -262,13 +250,9 @@ def reset_plugin_health(plugin_id):
'status': 'success',
'message': f'Health state reset for plugin {plugin_id}'
})
except Exception as e:
logger.error('Error in reset_plugin_health', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/metrics', methods=['GET'])
def get_plugin_metrics():
"""Get resource metrics for all plugins"""
try:
if not api_v3.plugin_manager:
return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500
@@ -301,13 +285,9 @@ def get_plugin_metrics():
'status': 'success',
'data': metrics_summaries
})
except Exception as e:
logger.error('Error in get_plugin_metrics', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/metrics/<plugin_id>', methods=['GET'])
def get_plugin_metrics_single(plugin_id):
"""Get resource metrics for a specific plugin"""
try:
if not api_v3.plugin_manager:
return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500
@@ -325,13 +305,9 @@ def get_plugin_metrics_single(plugin_id):
'status': 'success',
'data': metrics_summary
})
except Exception as e:
logger.error('Error in get_plugin_metrics_single', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/metrics/<plugin_id>/reset', methods=['POST'])
def reset_plugin_metrics(plugin_id):
"""Reset metrics for a plugin"""
try:
if not api_v3.plugin_manager:
return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500
@@ -349,13 +325,9 @@ def reset_plugin_metrics(plugin_id):
'status': 'success',
'message': f'Metrics reset for plugin {plugin_id}'
})
except Exception as e:
logger.error('Error in reset_plugin_metrics', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/limits/<plugin_id>', methods=['GET', 'POST'])
def manage_plugin_limits(plugin_id):
"""Get or set resource limits for a plugin"""
try:
if not api_v3.plugin_manager:
return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500
@@ -403,9 +375,6 @@ def manage_plugin_limits(plugin_id):
'status': 'success',
'message': f'Resource limits updated for plugin {plugin_id}'
})
except Exception as e:
logger.error('Error in manage_plugin_limits', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/toggle', methods=['POST'])
def toggle_plugin():
"""Toggle plugin enabled/disabled"""
@@ -546,14 +515,7 @@ def get_operation_status(operation_id):
return success_response(data=operation.to_dict())
except Exception as e:
from src.web_interface.errors import WebInterfaceError
error = WebInterfaceError.from_exception(e, ErrorCode.SYSTEM_ERROR)
return error_response(
error.error_code,
error.message,
details=error.details,
status_code=500
)
return exception_error_response(e, ErrorCode.SYSTEM_ERROR, with_context=False)
@api_v3.route('/plugins/operation/history', methods=['GET'])
def get_operation_history() -> Response:
"""Get operation history from the audit log."""
@@ -578,9 +540,7 @@ def get_operation_history() -> Response:
operation_type=operation_type
)
except (AttributeError, RuntimeError) as e:
from src.web_interface.errors import WebInterfaceError
error = WebInterfaceError.from_exception(e, ErrorCode.SYSTEM_ERROR)
return error_response(error.error_code, error.message, details=error.details, status_code=500)
return exception_error_response(e, ErrorCode.SYSTEM_ERROR, with_context=False)
return success_response(data=[record.to_dict() for record in history])
@api_v3.route('/plugins/operation/history', methods=['DELETE'])
@@ -596,9 +556,7 @@ def clear_operation_history() -> Response:
try:
api_v3.operation_history.clear_history()
except (OSError, RuntimeError) as e:
from src.web_interface.errors import WebInterfaceError
error = WebInterfaceError.from_exception(e, ErrorCode.SYSTEM_ERROR)
return error_response(error.error_code, error.message, details=error.details, status_code=500)
return exception_error_response(e, ErrorCode.SYSTEM_ERROR, with_context=False)
return success_response(message='Operation history cleared')
@api_v3.route('/plugins/state', methods=['GET'])
@@ -633,15 +591,7 @@ def get_plugin_state():
for plugin_id, state in all_states.items()
})
except Exception as e:
from src.web_interface.errors import WebInterfaceError
error = WebInterfaceError.from_exception(e, ErrorCode.SYSTEM_ERROR)
return error_response(
error.error_code,
error.message,
details=error.details,
context=error.context,
status_code=500
)
return exception_error_response(e, ErrorCode.SYSTEM_ERROR)
@api_v3.route('/plugins/state/reconcile', methods=['POST'])
def reconcile_plugin_state():
"""Reconcile plugin state across all sources"""
@@ -705,15 +655,7 @@ def reconcile_plugin_state():
message=result.message
)
except Exception as e:
from src.web_interface.errors import WebInterfaceError
error = WebInterfaceError.from_exception(e, ErrorCode.SYSTEM_ERROR)
return error_response(
error.error_code,
error.message,
details=error.details,
context=error.context,
status_code=500
)
return exception_error_response(e, ErrorCode.SYSTEM_ERROR)
def _drop_stale_reconciliation_findings(unresolved):
"""Re-check a stored reconciliation verdict against current state.
@@ -915,15 +857,7 @@ def get_plugin_config():
return success_response(data=plugin_config)
except Exception as e:
from src.web_interface.errors import WebInterfaceError
error = WebInterfaceError.from_exception(e, ErrorCode.CONFIG_LOAD_FAILED)
return error_response(
error.error_code,
error.message,
details=error.details,
context=error.context,
status_code=500
)
return exception_error_response(e, ErrorCode.CONFIG_LOAD_FAILED)
@api_v3.route('/plugins/update', methods=['POST'])
def update_plugin():
"""Update plugin"""
@@ -1168,8 +1102,6 @@ def update_plugin():
except Exception as e:
logger.error("Unhandled exception in update endpoint", exc_info=True)
from src.web_interface.errors import WebInterfaceError
error = WebInterfaceError.from_exception(e, ErrorCode.PLUGIN_UPDATE_FAILED)
if api_v3.operation_history:
api_v3.operation_history.record_operation(
"update",
@@ -1177,13 +1109,7 @@ def update_plugin():
status="failed",
error=str(e)
)
return error_response(
error.error_code,
error.message,
details=error.details,
context=error.context,
status_code=500
)
return exception_error_response(e, ErrorCode.PLUGIN_UPDATE_FAILED)
@api_v3.route('/plugins/uninstall', methods=['POST'])
def uninstall_plugin():
"""Uninstall plugin"""
@@ -1266,8 +1192,6 @@ def uninstall_plugin():
)
except Exception as e:
from src.web_interface.errors import WebInterfaceError
error = WebInterfaceError.from_exception(e, ErrorCode.PLUGIN_UNINSTALL_FAILED)
if api_v3.operation_history:
api_v3.operation_history.record_operation(
"uninstall",
@@ -1275,17 +1199,10 @@ def uninstall_plugin():
status="failed",
error=str(e)
)
return error_response(
error.error_code,
error.message,
details=error.details,
context=error.context,
status_code=500
)
return exception_error_response(e, ErrorCode.PLUGIN_UNINSTALL_FAILED)
@api_v3.route('/plugins/install', methods=['POST'])
def install_plugin():
"""Install plugin from store"""
try:
if not api_v3.plugin_store_manager:
return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500
@@ -1423,13 +1340,9 @@ def install_plugin():
status_code=500
)
except Exception as e:
logger.error('Error in install_plugin', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/install-from-url', methods=['POST'])
def install_plugin_from_url():
"""Install plugin from custom GitHub URL"""
try:
if not api_v3.plugin_store_manager:
return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500
@@ -1482,13 +1395,9 @@ def install_plugin_from_url():
'message': result.get('error', 'Failed to install plugin from URL')
}), 500
except Exception as e:
logger.error('Error in install_plugin_from_url', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/registry-from-url', methods=['POST'])
def get_registry_from_url():
"""Get plugin list from a registry-style monorepo URL"""
try:
if not api_v3.plugin_store_manager:
return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500
@@ -1519,25 +1428,17 @@ def get_registry_from_url():
'message': 'Failed to fetch registry from URL or URL does not contain a valid registry'
}), 400
except Exception as e:
logger.error('Error in get_registry_from_url', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/saved-repositories', methods=['GET'])
def get_saved_repositories():
"""Get all saved repositories"""
try:
if not api_v3.saved_repositories_manager:
return jsonify({'status': 'error', 'message': 'Saved repositories manager not initialized'}), 500
repositories = api_v3.saved_repositories_manager.get_all()
return jsonify({'status': 'success', 'data': {'repositories': repositories}})
except Exception as e:
logger.error('Error in get_saved_repositories', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/saved-repositories', methods=['POST'])
def add_saved_repository():
"""Add a repository to saved list"""
try:
if not api_v3.saved_repositories_manager:
return jsonify({'status': 'error', 'message': 'Saved repositories manager not initialized'}), 500
@@ -1566,13 +1467,9 @@ def add_saved_repository():
'status': 'error',
'message': 'Repository already exists or failed to save'
}), 400
except Exception as e:
logger.error('Error in add_saved_repository', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/saved-repositories', methods=['DELETE'])
def remove_saved_repository():
"""Remove a repository from saved list"""
try:
if not api_v3.saved_repositories_manager:
return jsonify({'status': 'error', 'message': 'Saved repositories manager not initialized'}), 500
@@ -1595,13 +1492,9 @@ def remove_saved_repository():
'status': 'error',
'message': 'Repository not found'
}), 404
except Exception as e:
logger.error('Error in remove_saved_repository', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/store/list', methods=['GET'])
def list_plugin_store():
"""Search plugin store"""
try:
if not api_v3.plugin_store_manager:
return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500
@@ -1649,13 +1542,9 @@ def list_plugin_store():
})
return jsonify({'status': 'success', 'data': {'plugins': formatted_plugins}})
except Exception as e:
logger.error('Error in list_plugin_store', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/store/github-status', methods=['GET'])
def get_github_auth_status():
"""Check if GitHub authentication is configured and validate token"""
try:
if not api_v3.plugin_store_manager:
return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500
@@ -1699,13 +1588,9 @@ def get_github_auth_status():
'error': error_message
}
})
except Exception as e:
logger.error('Error in get_github_auth_status', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/store/refresh', methods=['POST'])
def refresh_plugin_store():
"""Refresh plugin store repository"""
try:
if not api_v3.plugin_store_manager:
return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500
@@ -1725,9 +1610,6 @@ def refresh_plugin_store():
'message': message,
'plugin_count': plugin_count
})
except Exception as e:
logger.error('Error in refresh_plugin_store', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/config', methods=['POST'])
def save_plugin_config():
"""Save plugin configuration, separating secrets from regular config"""
@@ -2308,8 +2190,6 @@ def save_plugin_config():
return success_response(message=message)
except Exception as e:
from src.web_interface.errors import WebInterfaceError
error = WebInterfaceError.from_exception(e, ErrorCode.CONFIG_SAVE_FAILED)
if api_v3.operation_history:
api_v3.operation_history.record_operation(
"configure",
@@ -2317,13 +2197,7 @@ def save_plugin_config():
status="failed",
error=str(e)
)
return error_response(
error.error_code,
error.message,
details=error.details,
context=error.context,
status_code=500
)
return exception_error_response(e, ErrorCode.CONFIG_SAVE_FAILED)
def _merge_onto_stored_plugin_config(plugin_id, submitted_config, current_config=None):
"""A JSON plugin-config body merged onto the plugin's stored section.
@@ -2740,7 +2614,6 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr
@api_v3.route('/plugins/schema', methods=['GET'])
def get_plugin_schema():
"""Get plugin configuration schema"""
try:
plugin_id = request.args.get('plugin_id')
if not plugin_id:
return jsonify({'status': 'error', 'message': 'plugin_id required'}), 400
@@ -2778,13 +2651,9 @@ def get_plugin_schema():
}
return jsonify({'status': 'success', 'data': {'schema': default_schema}})
except Exception as e:
logger.error('Error in get_plugin_schema', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/config/reset', methods=['POST'])
def reset_plugin_config():
"""Reset plugin configuration to schema defaults"""
try:
if not api_v3.config_manager:
return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500
@@ -2857,9 +2726,6 @@ def reset_plugin_config():
'message': f'Plugin {plugin_id} configuration reset to defaults',
'data': {'config': defaults}
})
except Exception as e:
logger.error('Error in reset_plugin_config', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/action', methods=['POST'])
def execute_plugin_action():
"""Execute a plugin-defined action (e.g., authentication)"""
@@ -2868,8 +2734,10 @@ def execute_plugin_action():
try:
data = request.get_json(force=True) or {}
except Exception as e:
import logging
logger = logging.getLogger(__name__)
# The module logger, not a local one: binding `logger` anywhere in
# this function made every other `logger.error` here raise
# UnboundLocalError, so the step-1 handler below reported that
# instead of the plugin script's real failure.
logger.error(f"Error parsing JSON in execute_plugin_action: {e}")
return jsonify({
'status': 'error',
@@ -3172,9 +3040,6 @@ sys.exit(proc.returncode)
except subprocess.TimeoutExpired:
return jsonify({'status': 'error', 'message': 'Action timed out'}), 408
except Exception as e:
logger.error('Error in execute_plugin_action', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
def _plugin_uploads_dir(plugin_id):
"""assets/plugins/<plugin_id>/uploads for a request-supplied id, or None.
@@ -3187,7 +3052,6 @@ def _plugin_uploads_dir(plugin_id):
@api_v3.route('/plugins/assets/upload', methods=['POST'])
def upload_plugin_asset():
"""Upload asset files for a plugin"""
try:
plugin_id = request.form.get('plugin_id')
if not plugin_id:
return jsonify({'status': 'error', 'message': 'plugin_id is required'}), 400
@@ -3332,9 +3196,6 @@ def upload_plugin_asset():
'total_files': len(metadata)
})
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/<plugin_id>/static/<path:file_path>', methods=['GET'])
def serve_plugin_static(plugin_id, file_path):
"""Serve static files from plugin directory.
@@ -3353,7 +3214,6 @@ def serve_plugin_static(plugin_id, file_path):
``plugin-repos/foo-evil/x``, whose string does start with
``plugin-repos/foo``.
"""
try:
safe_plugin_id = safe_path_component(plugin_id)
if not safe_plugin_id:
return jsonify({'status': 'error', 'message': 'Invalid plugin ID'}), 400
@@ -3399,13 +3259,9 @@ def serve_plugin_static(plugin_id, file_path):
return Response(content, mimetype=content_type)
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/calendar/upload-credentials', methods=['POST'])
def upload_calendar_credentials():
"""Upload credentials.json file for calendar plugin"""
try:
if 'file' not in request.files:
return jsonify({'status': 'error', 'message': 'No file provided'}), 400
@@ -3476,9 +3332,6 @@ def upload_calendar_credentials():
'path': str(credentials_path)
})
except Exception as e:
logger.error('Error in upload_calendar_credentials', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/calendar/authenticate', methods=['POST'])
def authenticate_calendar():
"""Google OAuth for the calendar plugin, in the two steps it requires.
@@ -3492,7 +3345,6 @@ def authenticate_calendar():
The script persists the PKCE verifier from step 1 for step 2 to reuse; the
exchange fails with "Missing code verifier" otherwise.
"""
try:
plugin_dir = _pkg._calendar_plugin_dir()
if plugin_dir is None:
return jsonify({
@@ -3525,11 +3377,6 @@ def authenticate_calendar():
return jsonify(safe), 400
return jsonify(payload)
except Exception as e:
logger.error('Error in authenticate_calendar', exc_info=True)
return jsonify({'status': 'error',
'message': 'An error occurred; see logs for details',
'details': describe_exception(e)}), 500
@api_v3.route('/plugins/calendar/list-calendars', methods=['GET'])
def list_calendar_calendars():
"""The calendars this account can see, for the config picker.
@@ -3538,7 +3385,6 @@ def list_calendar_calendars():
picker is used interactively and a subprocess per click is slower than the
API call it would be wrapping.
"""
try:
plugin_dir = _pkg._calendar_plugin_dir()
if plugin_dir is None:
return jsonify({
@@ -3623,15 +3469,9 @@ def list_calendar_calendars():
return jsonify({'status': 'success', 'calendars': calendars})
except Exception as e:
logger.error('Error in list_calendar_calendars', exc_info=True)
return jsonify({'status': 'error',
'message': 'An error occurred; see logs for details',
'details': describe_exception(e)}), 500
@api_v3.route('/plugins/assets/delete', methods=['POST'])
def delete_plugin_asset():
"""Delete an asset file for a plugin"""
try:
data = request.get_json()
plugin_id = data.get('plugin_id')
image_id = data.get('image_id')
@@ -3677,13 +3517,9 @@ def delete_plugin_asset():
return jsonify({'status': 'success', 'message': 'Image deleted successfully'})
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/plugins/assets/list', methods=['GET'])
def list_plugin_assets():
"""List asset files for a plugin"""
try:
plugin_id = request.args.get('plugin_id')
if not plugin_id:
return jsonify({'status': 'error', 'message': 'plugin_id is required'}), 400
@@ -3706,6 +3542,3 @@ def list_plugin_assets():
return jsonify({'status': 'success', 'data': {'assets': assets}})
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
web_interface/blueprints/api_v3/starlark.py
+1
View File
@@ -11,6 +11,7 @@ from web_interface.blueprints.api_v3 import (
_PIXLET_EDITOR_DEFAULT_TIMEOUT, _PIXLET_EDITOR_MAX_TIMEOUT,
_PIXLET_EDITOR_SCRIPT, _PIXLET_EDITOR_STATE, _clear_pixlet_editor_state,
_find_pixlet_binary, _install_star_file, _pixlet_editor_alive,
_run_systemctl_command,
_pixlet_editor_status, _read_pixlet_editor_state,
_STARLARK_APPS_DIR, _standalone_render_starlark_app,
_starlark_github_token, _starlark_manifest_lock,
web_interface/blueprints/api_v3/system.py
-4
View File
@@ -24,7 +24,6 @@ import web_interface.blueprints.api_v3 as _pkg
@api_v3.route('/system/status', methods=['GET'])
def get_system_status():
"""Get system status"""
try:
# Check cache first (10 second TTL for system status)
try:
from web_interface.cache import get_cached, set_cached
@@ -110,9 +109,6 @@ def get_system_status():
pass # Cache write failed, but continue
return jsonify({'status': 'success', 'data': status})
except Exception as e:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@api_v3.route('/system/version', methods=['GET'])
def get_system_version():
"""Get LEDMatrix repository version"""
web_interface/blueprints/api_v3/wifi.py
-68
View File
@@ -111,7 +111,6 @@ def _parse_bool_ish(value):
@api_v3.route('/wifi/status', methods=['GET'])
def get_wifi_status():
"""Get current WiFi connection status"""
try:
from src.wifi_manager import WiFiManager
wifi_manager = WiFiManager()
@@ -132,13 +131,6 @@ def get_wifi_status():
'last_connect_attempt': _last_connect_snapshot(),
}
})
except Exception as e:
logger.error("%s failed", request.path, exc_info=True)
return jsonify({
'status': 'error',
'message': 'An error occurred; see logs for details',
'details': describe_exception(e)
}), 500
@api_v3.route('/wifi/scan', methods=['GET'])
def scan_wifi_networks():
"""Scan for available WiFi networks
@@ -219,7 +211,6 @@ def connect_wifi():
background (see _last_connect_attempt); otherwise it waits for the result.
"""
global _last_connect_attempt
try:
from src.wifi_manager import WiFiManager
data = request.get_json(silent=True)
@@ -290,16 +281,9 @@ def connect_wifi():
payload = _connect_result_payload(ssid, success, message)
_record_connect_result(ssid, payload)
return jsonify(payload), (200 if success else 400)
except Exception as e:
logger.error("Error connecting to WiFi", exc_info=True)
return jsonify({
'status': 'error',
'message': 'An error occurred; see logs for details', 'details': describe_exception(e)
}), 500
@api_v3.route('/wifi/disconnect', methods=['POST'])
def disconnect_wifi():
"""Disconnect from the current WiFi network"""
try:
from src.wifi_manager import WiFiManager
wifi_manager = WiFiManager()
@@ -315,16 +299,9 @@ def disconnect_wifi():
'status': 'error',
'message': message or 'Failed to disconnect from network'
}), 400
except Exception as e:
logger.error("Error disconnecting from WiFi", exc_info=True)
return jsonify({
'status': 'error',
'message': 'An error occurred; see logs for details', 'details': describe_exception(e)
}), 500
@api_v3.route('/wifi/ap/enable', methods=['POST'])
def enable_ap_mode():
"""Enable access point mode"""
try:
from src.wifi_manager import WiFiManager
wifi_manager = WiFiManager()
@@ -342,17 +319,9 @@ def enable_ap_mode():
'status': 'error',
'message': message
}), 400
except Exception as e:
logger.error("%s failed", request.path, exc_info=True)
return jsonify({
'status': 'error',
'message': 'An error occurred; see logs for details',
'details': describe_exception(e)
}), 500
@api_v3.route('/wifi/ap/disable', methods=['POST'])
def disable_ap_mode():
"""Disable access point mode"""
try:
from src.wifi_manager import WiFiManager
wifi_manager = WiFiManager()
@@ -368,17 +337,9 @@ def disable_ap_mode():
'status': 'error',
'message': message
}), 400
except Exception as e:
logger.error("%s failed", request.path, exc_info=True)
return jsonify({
'status': 'error',
'message': 'An error occurred; see logs for details',
'details': describe_exception(e)
}), 500
@api_v3.route('/wifi/ap/auto-enable', methods=['GET'])
def get_auto_enable_ap_mode():
"""Get auto-enable AP mode setting"""
try:
from src.wifi_manager import WiFiManager
wifi_manager = WiFiManager()
@@ -390,17 +351,9 @@ def get_auto_enable_ap_mode():
'auto_enable_ap_mode': auto_enable
}
})
except Exception as e:
logger.error("%s failed", request.path, exc_info=True)
return jsonify({
'status': 'error',
'message': 'An error occurred; see logs for details',
'details': describe_exception(e)
}), 500
@api_v3.route('/wifi/ap/auto-enable', methods=['POST'])
def set_auto_enable_ap_mode():
"""Set auto-enable AP mode setting"""
try:
from src.wifi_manager import WiFiManager
data = request.get_json(silent=True)
@@ -428,17 +381,9 @@ def set_auto_enable_ap_mode():
'auto_enable_ap_mode': auto_enable
}
})
except Exception as e:
logger.error("%s failed", request.path, exc_info=True)
return jsonify({
'status': 'error',
'message': 'An error occurred; see logs for details',
'details': describe_exception(e)
}), 500
@api_v3.route('/wifi/radio', methods=['GET'])
def get_wifi_radio():
"""Get current WiFi radio state (enabled/disabled) and wired-fallback status."""
try:
from src.wifi_manager import WiFiManager
wifi_manager = WiFiManager()
@@ -448,12 +393,6 @@ def get_wifi_radio():
'status': 'success',
'data': state
})
except Exception as e:
logger.error("Error getting WiFi radio state", exc_info=True)
return jsonify({
'status': 'error',
'message': 'An error occurred; see logs for details', 'details': describe_exception(e)
}), 500
@api_v3.route('/wifi/radio', methods=['POST'])
def set_wifi_radio():
"""Turn the WiFi radio on or off.
@@ -462,7 +401,6 @@ def set_wifi_radio():
unless Ethernet is connected or force=True, to avoid locking the user out
of this web interface.
"""
try:
from src.wifi_manager import WiFiManager
data = request.get_json(silent=True) or {}
@@ -506,9 +444,3 @@ def set_wifi_radio():
'message': message,
'reason': reason
}), 400
except Exception as e:
logger.error("Error setting WiFi radio state", exc_info=True)
return jsonify({
'status': 'error',
'message': 'An error occurred; see logs for details', 'details': describe_exception(e)
}), 500
web_interface/cache.py
+89 -30
View File
@@ -1,48 +1,107 @@
"""
Simple in-memory cache for expensive operations.
Separated from app.py to avoid circular import issues.
In-process TTL cache for the web interface.
The one place the web process memoises cheap-to-recompute values for a few
seconds or minutes (the font catalog, the system-status snapshot, systemctl
checks). It is per-process and in-memory only; data shared with the display
service goes through ``src.cache_manager.CacheManager`` instead.
Separated from app.py to avoid circular imports: blueprints import the
module-level helpers below lazily, inside their request handlers.
"""
import threading
import time
from typing import Any, Optional
from typing import Any, Callable, Dict, Optional, Tuple
# Simple in-memory cache for expensive operations
_cache = {}
_cache_timestamps = {}
class TTLCache:
"""A small thread-safe key/value store whose entries expire.
Each entry keeps the TTL it was stored with. A reader may additionally
pass ``max_age`` to ask for something fresher than that; an entry is only
returned while it is younger than both.
def get_cached(key: str, ttl_seconds: int = 60) -> Optional[Any]:
"""Get value from cache if not expired."""
if key in _cache:
if time.time() - _cache_timestamps[key] < ttl_seconds:
return _cache[key]
Expired entries are not dropped on read: :meth:`peek` still returns them,
which is what a "keep the last known answer if the refresh fails" caller
needs. They are replaced by the next :meth:`set` of the same key, so this
is meant for a small, fixed set of keys, not an unbounded key space.
Ages are measured with ``time.monotonic`` so a wall-clock jump (NTP sync
on a Pi that booted without an RTC) neither expires nor immortalises
everything at once.
"""
def __init__(self, default_ttl: float = 60,
clock: Callable[[], float] = time.monotonic):
self._default_ttl = default_ttl
self._clock = clock
self._lock = threading.Lock()
# key -> (value, stored_at, ttl)
self._entries: Dict[str, Tuple[Any, float, float]] = {}
def get(self, key: str, default: Any = None,
max_age: Optional[float] = None) -> Any:
"""The value for ``key`` if it is still fresh, else ``default``."""
with self._lock:
entry = self._entries.get(key)
if entry is None:
return default
value, stored_at, ttl = entry
age = self._clock() - stored_at
if age >= ttl or (max_age is not None and age >= max_age):
return default
return value
def peek(self, key: str, default: Any = None) -> Any:
"""The last value stored for ``key``, fresh or not."""
with self._lock:
entry = self._entries.get(key)
return default if entry is None else entry[0]
def set(self, key: str, value: Any, ttl: Optional[float] = None) -> None:
"""Store ``value`` for ``ttl`` seconds (the cache default if None)."""
ttl = self._default_ttl if ttl is None else ttl
with self._lock:
self._entries[key] = (value, self._clock(), ttl)
def delete(self, key: str) -> None:
"""Remove ``key`` if present."""
with self._lock:
self._entries.pop(key, None)
def clear(self, pattern: Optional[str] = None) -> None:
"""Remove every entry, or only those whose key contains ``pattern``."""
with self._lock:
if pattern is None:
self._entries.clear()
else:
# Expired, remove
del _cache[key]
del _cache_timestamps[key]
return None
for key in [k for k in self._entries if pattern in k]:
del self._entries[key]
def set_cached(key: str, value: Any, ttl_seconds: int = 60) -> None:
"""Set value in cache with TTL."""
_cache[key] = value
_cache_timestamps[key] = time.time()
# The shared cache behind the functional helpers the blueprints use.
_default_cache = TTLCache(default_ttl=60)
def get_cached(key: str, ttl_seconds: Optional[float] = None) -> Optional[Any]:
"""Get a value from the cache if it has not expired.
The entry expires after the TTL it was stored with; ``ttl_seconds``, when
given, is an extra upper bound on its age for this read.
"""
return _default_cache.get(key, max_age=ttl_seconds)
def set_cached(key: str, value: Any, ttl_seconds: float = 60) -> None:
"""Store a value in the cache for ``ttl_seconds``."""
_default_cache.set(key, value, ttl=ttl_seconds)
def delete_cached(key: str) -> None:
"""Remove a single key from the cache if present."""
_cache.pop(key, None)
_cache_timestamps.pop(key, None)
_default_cache.delete(key)
def invalidate_cache(pattern: Optional[str] = None) -> None:
"""Invalidate cache entries matching pattern, or all if pattern is None."""
if pattern is None:
_cache.clear()
_cache_timestamps.clear()
else:
keys_to_remove = [k for k in _cache.keys() if pattern in k]
for key in keys_to_remove:
del _cache[key]
del _cache_timestamps[key]
_default_cache.clear(pattern)
web_interface/logging_config.py
-110
View File
@@ -1,110 +0,0 @@
"""
Structured logging configuration for the web interface.
Provides JSON-formatted logs for production and readable logs for development.
"""
import logging
import json
import sys
from datetime import datetime
from typing import Optional
class JSONFormatter(logging.Formatter):
"""Formatter that outputs logs as JSON for structured logging."""
def format(self, record: logging.LogRecord) -> str:
"""Format log record as JSON."""
log_data = {
'timestamp': datetime.utcnow().isoformat(),
'level': record.levelname,
'logger': record.name,
'message': record.getMessage(),
'module': record.module,
'function': record.funcName,
'line': record.lineno,
}
# Add exception info if present
if record.exc_info:
log_data['exception'] = self.formatException(record.exc_info)
# Add extra fields if present
if hasattr(record, 'request_id'):
log_data['request_id'] = record.request_id
if hasattr(record, 'user_id'):
log_data['user_id'] = record.user_id
if hasattr(record, 'ip_address'):
log_data['ip_address'] = record.ip_address
if hasattr(record, 'duration_ms'):
log_data['duration_ms'] = record.duration_ms
return json.dumps(log_data)
def setup_web_interface_logging(level: str = 'INFO', use_json: bool = False):
"""
Set up logging for the web interface.
Args:
level: Log level (DEBUG, INFO, WARNING, ERROR)
use_json: If True, use JSON formatting (for production)
"""
# Get root logger
logger = logging.getLogger()
logger.setLevel(getattr(logging, level.upper()))
# Remove existing handlers
logger.handlers.clear()
# Create console handler
console_handler = logging.StreamHandler(sys.stdout)
console_handler.setLevel(getattr(logging, level.upper()))
# Set formatter
if use_json:
formatter = JSONFormatter()
else:
formatter = logging.Formatter(
'%(asctime)s - %(name)s - %(levelname)s - %(message)s',
datefmt='%Y-%m-%d %H:%M:%S'
)
console_handler.setFormatter(formatter)
logger.addHandler(console_handler)
# Set levels for specific loggers
logging.getLogger('werkzeug').setLevel(logging.WARNING) # Reduce Flask noise
logging.getLogger('urllib3').setLevel(logging.WARNING) # Reduce HTTP noise
def log_api_request(method: str, path: str, status_code: int, duration_ms: float,
ip_address: Optional[str] = None, **kwargs):
"""
Log an API request with structured data.
Args:
method: HTTP method
path: Request path
status_code: HTTP status code
duration_ms: Request duration in milliseconds
ip_address: Client IP address
**kwargs: Additional context
"""
logger = logging.getLogger('web_interface.api')
extra = {
'method': method,
'path': path,
'status_code': status_code,
'duration_ms': round(duration_ms, 2),
'ip_address': ip_address,
**kwargs
}
# Log at appropriate level based on status code
if status_code >= 500:
logger.error(f"{method} {path} - {status_code} ({duration_ms}ms)", extra=extra)
elif status_code >= 400:
logger.warning(f"{method} {path} - {status_code} ({duration_ms}ms)", extra=extra)
else:
logger.info(f"{method} {path} - {status_code} ({duration_ms}ms)", extra=extra)
web_interface/request_logging.py
+62
View File
@@ -0,0 +1,62 @@
"""
Per-request logging for the web interface.
Logging itself is configured by ``src.logging_config.setup_logging`` (the same
formatter and journald priorities as the display service); this module only
decides what one HTTP request is worth logging, and at which level.
The UI polls: the error summary, system status, display preview and log
streams are fetched every few seconds by every open tab. Logging each of those
at INFO buried everything else in the journal (``GET /api/v3/errors/summary -
200`` once a minute per tab, forever). So a request that only read something
and succeeded is DEBUG; one that changed something, or failed, is logged at a
level that shows up by default.
"""
import logging
import time
from flask import Flask, request
logger = logging.getLogger('web_interface.api')
#: Methods that do not change server state. A successful one is routine.
_READ_ONLY_METHODS = frozenset({'GET', 'HEAD', 'OPTIONS'})
def request_log_level(method: str, status_code: int) -> int:
"""The level a finished request is logged at."""
if status_code >= 500:
return logging.ERROR
if status_code >= 400:
return logging.WARNING
if method.upper() in _READ_ONLY_METHODS:
return logging.DEBUG
return logging.INFO
def log_request(method: str, path: str, status_code: int,
duration_ms: float) -> None:
"""Log one finished request."""
level = request_log_level(method, status_code)
if logger.isEnabledFor(level):
logger.log(level, "%s %s - %d (%.1fms)",
method, path, status_code, duration_ms)
def init_app(app: Flask) -> None:
"""Time every request and log it when its response is ready."""
@app.before_request
def _start_request_timer():
request.start_time = time.perf_counter()
@app.after_request
def _log_finished_request(response):
try:
started = getattr(request, 'start_time', None)
duration_ms = 0.0 if started is None else (time.perf_counter() - started) * 1000
log_request(request.method, request.path, response.status_code,
duration_ms)
except Exception: # nosec B110 - request logging must never interrupt a live HTTP response
pass
return response