Numbers from the 97k-track library on roy, what was learned about
PGlite and the single-session model, the fixes on this branch, the
limitations, and ideas for follow-up work.
The single-user backend has no autovacuum, and each scan leaves a dead
version of every updated row (all 29,602 library_artist rows per stats
refresh). VACUUM (ANALYZE) takes 9.7 s on the 97k-track library.
After the post-scan ANALYZE the planner probed library_artist once per
artist for the role filter written as stats->'role'->>'m' IS NOT NULL:
29,602 lookups at 3.8 ms each, 116 s for the artist list on roy. The
containment form (stats @> '{"role": {}}') is served by the GIN index:
0.2 s for the same list.
A count built from a sorted builder failed with 'column must appear in
the GROUP BY clause' (the library list); counting the ids in a subquery
makes the carried ORDER BY harmless. optimizePagination relied on
SQLite's rowid and is now a no-op.
annotationBoolFilter serves both starred (boolean) and rating (integer);
the boolean form broke the Top Rated lists with 'COALESCE types integer
and boolean cannot be matched'.
The starred filter used COALESCE(starred, 0) > 0, which PostgreSQL
rejects for a boolean column ('COALESCE types boolean and integer cannot
be matched'); the Favourites lists returned 500.
The fts backend needs SQLite's FTS5 virtual tables; on this branch any
search returned 'syntax error at or near MATCH'. The LIKE backend works
unchanged on PostgreSQL.
A client that starts a handshake and disconnects before the password
step (a pool connection attempt timing out behind a long scanner
transaction) left the single backend in ClientAuthInProgress, and it
then answered other clients' queries without ReadyForQuery, or read
their query as an empty one, until the next handshake completed. Each
such reply broke a pooled connection and, during a scan, the scanner
transaction on it. The bridge now replies AuthenticationOk, the cached
ParameterStatus messages, BackendKeyData and ReadyForQuery itself for
every client after the first, answers SSLRequest with N, and drops
CancelRequest connections.
PGlite's interactive_one discards an incoming request when output from
the previous frame is still pending ('flush after frame'), which showed
up as replies without ReadyForQuery, first after handshakes and then in
bursts mid-scan, each one breaking a pooled connection and the scanner
transaction on it. After a complete reply the bridge now runs one empty
tick and appends anything the backend still had, and logs the message
tags of any reply that is still incomplete.
The whole pglite data dir was mounted as the guest's /tmp, which also
exposed the cluster twice. The guest only needs its password file and
the shared-memory stand-ins there, so <DataFolder>/pglite/tmp is mounted
instead. The cluster and /dev mounts are unchanged.
PGlite splices a fixed list of -c settings into every start and never
reads postgresql.conf or postgresql.auto.conf on restart (ALTER SYSTEM
writes to the wrong directory and pg_reload_conf is a no-op without a
postmaster), so the only way to lower max_wal_size is that list. With
128MB, a CHECKPOINT trims pg_wal from 405 MB to 134 MB locally instead
of letting it grow to the 1 GB default.
PGlite's single-user backend has no background checkpointer, so nothing
written during a scan is checkpointed until shutdown, and a crash would
replay the whole scan's WAL. Run CHECKPOINT right after the post-scan
ANALYZE. The log directory itself stays around max_wal_size because
PostgreSQL recycles segments rather than deleting them.
The three counters each joined media_file_artists to media_file again,
about 10 s per pass on the 97k-track library. A shared materialized
artist_rows set is joined once and aggregated three times: 35 s to 25 s
on roy for a full refresh, 240 ms to 138 ms per 1000-artist batch
locally.
database/sql keeps only 2 idle connections by default, so with 4 open
the pool closed and reopened connections constantly: about 225
Terminate+handshake cycles per minute during a scan on roy. Every
handshake goes through the single backend, and the first query after a
handshake is where the lost replies were seen.
Three times during a scan on a real library, a new pool connection's
first query got no bytes back at all, which the previous commit turned
from a hang into an error. A run always emits at least a completion, so
no output means the packet was not run; the bridge now hands it over
again (twice at most) and logs it.
The stats UPDATE now joins the materialized counters (with a LEFT JOIN
so artists without files still get '{}') instead of a correlated
subquery, and a full refresh runs as a single statement: 3.3 s for 30k
artists locally versus 30 batches. Batches of 1000 remain for partial
refreshes (0.24 s each).
library.total_size was integer; a 2 TB library overflowed it at the end
of the first scan. It is bigint now, matching the Go int64.
The bridge no longer synthesizes a reply for Terminate, which never gets
one by design.
When the backend produced no bytes for a request, or a reply without
ReadyForQuery, the bridge forwarded it as-is and the client waited for
ever while the session was free. On roy this froze the UI: a pooled
connection hung right after its handshake and every request queued in
the pool behind it. Incomplete replies now get a synthesized error plus
ReadyForQuery, and the bridge logs them, so a glitch becomes a visible
SQL error that the pool can retry.
PostgreSQL inlines single-use CTEs, so the correlated subquery in the
final UPDATE re-ran the whole aggregation for every library_artist row,
rescanning all media files of the library three times per artist. On
the 97k-track library one 1000-artist batch ran for more than five
minutes. MATERIALIZED computes each CTE once per batch: 0.3 s versus
1.1 s locally, and orders of magnitude on the real plan.
The CTE placed inside NOT EXISTS is correlated and re-evaluated for every
tag row, so it was as slow as NOT IN on the real library (still running
after 10 minutes). id IN (SELECT id FROM tag EXCEPT <used ids>) builds
the unused set once with a hash set-op: about 1 s at 1M tag references,
with or without planner statistics.
On a 97k-track library the tag purge in the scanner GC ran for many
minutes: PostgreSQL re-evaluates a NOT IN subquery of ~1M tag references
per tag row. A materialized set with NOT EXISTS takes 1.4 s locally
versus 55 s for NOT IN at that scale; work_mem made no difference.
PostgreSQL rejects SELECT DISTINCT with ORDER BY random(), which the
playlist cover picker uses to sample album ids. GROUP BY yields the same
distinct set and allows the volatile ordering.
The bridge gave every reply write a 5 second deadline. Scanner phase 3
streams all touched albums through a cursor and runs queries per row on
other pooled connections, which queue on the single session, so a large
library could not drain the reply in time and the bridge closed the pipe
mid-message ('unexpected EOF'). Replies now block as long as the client
needs, and client connections are closed on shutdown instead.
Playlist rules were bound as []byte, which pgx sends as bytea; the text
column then stored the hex form and the JSON parser failed on it.
Restore the go and coverage jobs on the spike branch, but test only
conf/ (which passes here) so a coverage artifact is still produced and
the Report coverage on PR workflow stops failing on every push.
The PGlite fork stubs MemoryContextResetAndDeleteChildren, which
PostgreSQL 17 removed, as an empty macro, so MessageContext was never
reset and each statement leaked its parse and plan state (about 10 KB).
On a real library the process grew to 15 GB during the first scan and
the host swapped itself to a crawl. The new build maps the macro to
MemoryContextReset (message_context_reset.diff); memory now stays flat
across thousands of statements. Adds PGlite.MemorySize for measuring.
In PostgreSQL, a bare column name in ON CONFLICT DO UPDATE is ambiguous
between the target row and excluded, and MAX() is an aggregate rather
than the two-argument function SQLite provides. Every artwork enqueue
failed with 'column reference "priority" is ambiguous' on a real
library.
pglite.wasi.gz is PostgreSQL 17.5 built for WASI at -O2 without the wizer
snapshot, gzipped from 14.6 MB to 4.9 MB. db/pglite/embed.go go:embed's
it, so the pipeline cannot build the binary without it in the tree. The
recipe that produces it is in db/pglite/build/README.md.
Comment out the go, go-plugins, go-windows and coverage jobs and the npm
test step, and drop the test jobs from the build job's needs, so the
pipeline still builds. The persistence and other suites that create an
in-memory SQLite database cannot run on this branch and would fail every
push. Commented rather than deleted so they are easy to restore.
Replace the 131 SQLite migrations with a single goose migration that creates
the whole schema in PostgreSQL (the user table becomes user_account, json
columns become jsonb, seed rows for the default library and transcodings).
Port the repositories' SQL just far enough for Navidrome to start, scan,
log in and browse artists and albums: json_agg/jsonb_* for the SQLite JSON
functions, lower() for COLLATE NOCASE, ON CONFLICT DO NOTHING, coalesce on
annotations, stricter GROUP BY, explicit casts under the simple protocol,
sequential queries in library.RefreshStats. db/ is now Postgres-only, with
backup/restore stubbed and DevExternalScanner forced off.
Embed the PGlite module like go-taglib: pglite.wasi.gz is go:embed'ed,
decompressed in memory and compiled by wazero with an on-disk compilation
cache, so the module itself is never written to disk. The cluster is
created by initdb at runtime on the first start; initdb runs in a
throwaway wasm instance and the backend in a fresh one, otherwise the next
start cannot find a valid checkpoint. Since initdb creates only template1,
a short-lived backend then runs CREATE DATABASE navidrome and the real
backend starts on it. The build recipe and patches for the -O2, no-wizer
module live in db/pglite/build.
Adds db/pglite, a bridge that runs the PGlite WASI build of PostgreSQL 17.5
inside the Navidrome process with wazero and exposes it to database/sql
through pgx over the real wire protocol. Selected with DbPath = "pglite://<dir>".
The bridge handles what the WASI build cannot do on its own: it emulates dup2
onto stdin (wazero refuses fd_renumber on preopens), replays ParameterStatus
for connections after the first, synthesizes a missing ReadyForQuery after an
error, recovers from the trap every PG ERROR raises, and passes wire bytes
through shared wasm memory instead of files (round-trip floor 377 µs to 39 µs).
Several connections are accepted and serialized onto the single backend,
holding the session across a whole transaction and a whole handshake; a client
that disconnects mid-transaction gets a ROLLBACK.
db.go opens the pglite:// scheme, skips the SQLite-only migrations, and can
apply a translated schema from ND_PGLITE_SCHEMA. The shared count() helper now
adds its ORDER BY only for SQLite, since Postgres rejects it next to
count(distinct ...).
The wasm archive is not committed. The README explains how to build it and
lists the limits found: one session, one process (DevExternalScanner must be
off), shared session state, simple protocol only.
`buildPlaylist` reported `evaluated_at` as `changed` for smart playlists, so a
rename or comment edit was invisible to clients until the next evaluation. A
never-evaluated smart playlist also reported the current time on every call,
which never settled.
Report `updated_at` for every playlist. `refreshCounters` now syncs the stamp it
writes back onto the model, and `refreshSmartPlaylist` reuses it for
`evaluated_at`. `changed` therefore still equals the evaluation time for a
just-evaluated playlist, and `validUntil` stays anchored to it.
Original Subsonic always bumps `changed` on any playlist update, so this also
aligns the behavior with upstream.
The Refresh Metadata action was only reachable from the Album and Artist context menus, so it could not be triggered from AlbumShow or ArtistShow. This adds an icon-only button, with a tooltip, to the action toolbar on both detail pages. Like the menu entry, it is only rendered for admins.
The button is built on react-admin's Button rather than a plain IconButton: the surrounding toolbars use the former, so the theme colour and the icon-only swap at the xs breakpoint are inherited instead of restated. The dataProvider call and its two notifications move into a new useRefreshMetadata hook, which ContextMenus now shares, keeping a single copy of that logic.
A playlistTrack id is a position in the playlist, not a stable key, so refetching
a row by id after the annotation is saved can return a different song: in a smart
playlist filtered on that annotation the track is gone and every later row has
shifted up. The stale-keyed record then renders as a duplicate of its neighbour.
Signed-off-by: Bjørn A. Andersen <polybjorn@users.noreply.github.com>
Co-authored-by: Bjørn A. Andersen <polybjorn@users.noreply.github.com>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
The exclusion only worked on master. Coverage profiles name files by import
path; octocov shortens those to repo-relative paths using the checked-out
source, but coverage-on-pr.yml sparse-checks-out only .octocov.yml, so the
paths stay as github.com/navidrome/navidrome/tests/mock_*.go and 'tests/**'
never matched. '**/*_gen.go' matched either way, which is why only the 30
tests/ files leaked.
Every pull request since b77fb45 therefore reported ~-3.7% against master:
447 files on the base side, 477 on the pull request side (#6002, #6069).
* chore: ensure that all durations are nonnegative
* make sure you actually include the test file
* test(conf): use non-zero durations in the valid_duration fixture
Zero is the boundary between the accepted and rejected ranges, so it
passes even if the guard is off by one. 1s exercises an ordinary value.
---------
Co-authored-by: Deluan <deluan@navidrome.org>
An absent artwork state was revisited by an hourly job, by viewing the entity, and
by the startup backfill on any artwork config change. On a large library the last
one queued tens of thousands of external lookups at once and got the provider to
rate-limit us for hours.
Nothing revisits an absent state now. Retrying is explicit: `artwork reprocess` on
the CLI, or the refresh button in the UI. The config fingerprint survives only as an
advisory, warning at startup and naming the command that clears it.
Since absent is terminal, `artwork status` splits it into two disjoint columns, and
`--source failed` targets only the ones that gave up rather than being answered.
Both read through the filter CountBySource and EnqueueBySource already share, so the
reported number is the set the command acts on.
Also fixes the last_failure default left by 20260819204637, which marked every
pre-existing absent row as failed, and removes the code the deleted retry paths
orphaned.
The coverage profile counted the tests/ package and the *_gen.go files, none of
which are code under test: tests/ is the mock and helper package, and generated
code is never hand-tested. Together they added 2926 uncounted statements at 0%,
pulling the reported number down by almost 6 points (70.13% -> 75.98% on the
current master profile).
octocov's coverage.exclude takes doublestar globs matched against git-root-relative
paths. All 26 mock_*.go files live under tests/, so the single 'tests/**' pattern
covers them.
* fix(deezer): treat an exhausted quota as a throttle, not as a missing artist
Deezer reports quota exhaustion in the response body, with HTTP 200 and no
rate-limit headers. The client only looked for errors when the status was
not 200, so a throttled reply was decoded into an empty result type, and an
empty search became ErrNotFound. The agent then compounded it: it tested
`errors.Is(err, ErrNotFound) || len(artists) == 0` before testing err, so
any failed search — which also returns no artists — reported not-found too.
The artwork worker settles an entity as "no image" on agents.ErrNotFound.
So being throttled did not make Navidrome back off; it made it record the
artist as having no artwork, and move on to do the same to the next one.
Errors are now parsed out of the body regardless of status, and the quota
code is joined with agents.RetryLaterError so the circuit breaker and the
artwork retry budget see a throttle for what it is. The agent checks err
before the empty-result case.
Last.fm already handles this exact shape (client.go errCodeRateLimit, with
a comment noting the 200-with-body-error pattern); this brings Deezer in
line with it, including the zero-delay RetryLaterError so both providers
share the default cooldown rather than a per-provider number.
Measured against the live API to pin the shape: a 120-request burst
returned 54 results and 66 quota replies, every one of them HTTP 200 with
{"error":{"type":"Exception","message":"Quota limit exceeded","code":4}}
and no Retry-After or rate-limit headers. A single request 5s later
succeeded, so the window is short and a cooldown fully clears it.
* refactor(deezer): fold the error envelope into one type
The envelope declared the code and message inline, parseBodyError copied
them field by field into a second struct with the same shape, and a zero
Code stood in for "no error reported". Making the envelope hold a pointer
to the error type removes all three: absent is nil, present is the error
itself, and the value returned needs no conversion.
searchArtist loses its empty-result branch. searchArtists converts an
empty result to errNotFound and returns early on any error, so it never
answers with no artists and no error, and the branch could not run. What
it left behind was a comment explaining an ordering that only mattered
while the branch existed.
ErrNotFound is unexported: nothing outside this package referenced it,
and it sat three lines from agents.ErrNotFound, which is a different
error with the opposite meaning for callers.
Throttling now joins agents.ErrRetryLater, the sentinel documented as the
zero-delay RetryLaterError, rather than allocating an equivalent value.
* refactor(deezer): return agents.ErrNotFound from the client
The client raised a package-local sentinel that the agent then translated
into agents.ErrNotFound, one call site each. Deezer was the only adapter
carrying its own: last.fm and listenbrainz have none.
The client already reports throttling with agents.ErrRetryLater, so it
already speaks the agent vocabulary; saying "not found" in the same words
costs nothing and lets searchArtist drop to plain error propagation.
* test(scrobbler): remove a race in the longest-server-delay test
newBufferedScrobbler starts its drain goroutine, and run() drains once
before it ever waits on the wake signal. The test enqueued user2, then
enqueued user1 via Scrobble, so that startup drain could land between
the two: it saw only user2, took its 45s delay, and set backingOff. The
wake from the second enqueue is then deliberately ignored — a wake
during a backoff window must not drain, which is the hammering the
window exists to prevent — so user1 was never attempted and the first
assertion read 1 instead of 2.
Buffering both users before the goroutine exists removes the window.
The test no longer goes through Scrobble, which the sibling tests
already cover; what this one is about is which delay wins.
Reproduced deterministically by forcing the interleaving with a
synctest.Wait between the two enqueues, which fails with the same
"expected both users drained, got 1 attempts" seen in CI. With both
enqueued first, that same forced drain passes.
An explicit RetryLaterError now opens the agent's breaker immediately for the
provider's own delay, instead of counting it as one generic failure that needs
five to open and then always probes after a fixed minute.
* ci: comment coverage on pull requests from forks
A pull_request run from a fork gets a read-only GITHUB_TOKEN, so octocov could not post its comment: it logged a 403 and exited 0, leaving the job green and the PR silent. The 'permissions:' block cannot grant what the token does not have.
The comment now comes from a workflow_run workflow, which runs on the base repository and does get a write token. The pipeline job keeps the job summary and the default-branch baseline, and hands the merged profile and the PR number to it as an artifact.
A workflow_run job otherwise looks like a push to the default branch, so octocov is pointed back at the pull request and at the run that produced the profile via its OCTOCOV_ environment overrides. Without the run id override the test execution time would be read from the wrong run; without the ref override a fork's coverage would be stored as the master baseline.
The job holds a write token, so it reads .octocov.yml from the base branch rather than from the fork.
* ci: stop checking out the fork in the coverage comment workflow
CodeQL flagged the pull request checkout as untrusted code in a privileged context (actions/untrusted-checkout/high): the job holds a write token. The checkout existed only so the code-to-test ratio would reflect the pull request, which does not justify the alert.
The workflow now checks out just .octocov.yml from the base branch, and the ratio is skipped when reporting from there. Coverage and its delta against master, the metrics that motivated the report, are unaffected: they come from the profile the pipeline uploads.
* ci: treat the coverage artifact as untrusted input
A pull_request run executes the fork's own copy of pipeline.yml, so every file in the octocov-pr artifact is attacker-controlled. The artifact was extracted into the workspace root, on top of the base-branch checkout, and download-artifact truncates existing files. A fork could therefore replace .octocov.yml before octocov loaded it.
That is not only a config swap. config.Load expands ${VAR} from the job environment and the action sets OCTOCOV_GITHUB_TOKEN, so a crafted comment.message posts the privileged job's token into a public comment; a body: section rewrites a pull request description, which pull-requests: write allows.
The artifact now lands in a subdirectory and only coverage.out is copied out, after pr_number is checked to be digits and the named pull request's head is confirmed to be the sha that triggered this run. Without that check the artifact could aim the comment at any open pull request, and unvalidated content reached GITHUB_OUTPUT.
* ci: report Go test coverage on pull requests
Adds octocov to the existing 'Test Go code' job. It reads the coverage
profile, posts a PR comment with the coverage percentage and the delta
against master, and writes the same report to the job summary.
The master-branch report is stored as a GitHub Actions artifact, so no
external service or secret is needed.
* ci: merge the plugins job coverage into the same report
The plugins suite runs in its own job, so its coverage was missing from
the report. Both jobs now upload their profile as an artifact and a new
'Report coverage' job merges them into a single PR comment.
* ci: update the coverage comment in place instead of reposting
octocov's default is to collapse the previous comment and create a new
one. updatePrevious edits the existing comment instead, so a PR keeps a
single coverage comment across pushes.
* ci: fix octocov timeout and step-time lookup
Storing the report hit the 30s default timeout: scanning this repo's
artifacts for the baseline consumed it first. Raise it to 5m.
The step-time lookup also matched the Windows job's 'Test' step and
waited for a job that was still running, so execution time was dropped
from the report. Rename the step to make it unique.
* ci: only store the coverage baseline from the default branch
* ci: report statement coverage instead of line coverage
octocov reports statement coverage for a single profile but switches to
line counting when it merges several itself, which made the number
disagree with 'go tool cover -func'. Merge the two job profiles into one
file first, so the reported number matches what developers see locally.
* ci: stop the download-link comment from clobbering the coverage report
Both comments are posted by github-actions[bot], and the download-link
job updated the first bot comment it found. On a new PR the coverage
comment is created first, so it would be overwritten. Match on the body
as well, and keep the coverage profiles out of the download list.
* test(plugins): build test plugins in Go instead of shelling out to make
The plugins suite built its .ndp test packages by running `make -C
plugins/testdata`, which needs make and zip on the PATH. That is the reason
the 26 WASM-dependent spec files are tagged //go:build !windows.
buildTestPlugins now does the same work in Go: the same mtime check make
performed, `GOOS=wasip1 GOARCH=wasm go build` per plugin, and archive/zip
for the package. TinyGo was already optional and unused in CI, so nothing is
lost there. The first plugin builds on its own so the shared wasip1 stdlib
and PDK objects land in the build cache before the rest fan out: on a cold
cache that is 2.2s against 3.4s for the sequential make and 7.3s for an
unrestrained fan-out.
Packaging moved into a writeNdp helper shared with createTestPackage, which
was already writing the same two-entry archive. Entries are written in a
fixed order, so the .ndp bytes are now reproducible; the loader hashes those
bytes, and `zip` also stored file mtimes, so the previous packages differed
on every rebuild.
The Makefile is unchanged and still works for building the plugins by hand.
Removing the !windows tags is a separate step, once CI is green here.
* test(plugins): run the WASM plugin specs on Windows
With the test plugins now built in Go, nothing in the suite needs a Unix
toolchain, so the //go:build !windows tags come off all 25 spec files. The
Windows CI job runs `go test ./...`, so it picks the suite up with no
workflow change.
plugins_suite_windows_test.go existed only to bootstrap the handful of specs
that compiled on Windows; plugins_suite_test.go now serves both.
* test(plugins): skip the planted-symlink spec where symlinks need privileges
os.Symlink needs an elevated token or Developer Mode on Windows, so the
unconditional Expect(...).To(Succeed()) would fail for contributors running
the suite on an ordinary Windows box. The elevated GitHub runner hides this.
The equivalent spec in sandbox_fs_internal_test.go already attempts the
symlink and skips on error; this does the same, keeping the pin live
everywhere it can run, including Windows CI.
* fix(ci): stop the Windows ndpgen test failing silently
The ndpgen suite builds its helper binary to %TEMP%\ndpgen-test, and Windows
will not exec a file without an executable extension, so the "supports
verbose mode" spec has been failing there. Nobody noticed because the
Test ndpgen step ran under pwsh, which carries on after a non-zero exit and
takes the step's status from the last command, so the job stayed green with
a FAIL line in its log.
Add the .exe suffix, and run the step under bash like the Linux job does, so
a failure in any of its three commands fails the job.
* fix(plugins): build public URLs on the caller's address instead of localhost
The artwork host service had no `*http.Request`, so it passed `nil` to
`publicurl.ImageURL`. With neither `ShareURL` nor `BaseURL` configured, that
produced `http://localhost/share/img/...`, which is useless to anything outside
the server. The Discord Rich Presence plugin explicitly drops localhost URLs, so
it fell back to the Navidrome logo instead of the real cover art.
`serverAddressMiddleware` already works out the client-facing scheme and host
from the `X-Forwarded-*` headers. It now also records them in the request
context, and `publicurl` takes a `context.Context` instead of an `*http.Request`
so any caller can reach them. Extism passes the caller's context through to host
functions, so plugins invoked during a request now get a reachable URL with no
configuration.
Switching the parameter also removes the need for a second, parallel entry
point: the package previously wanted only a scheme, a host, and a context, and
took a whole request to get them. `AbsoluteURL` no longer dereferences a
possibly-nil request on its parse-error path.
Plugin calls that start from `context.Background()` (scheduler and websocket
callbacks, the buffered scrobble drain) still fall back to localhost, since they
have no request to learn from. A debug log now points at `ShareURL` when that
happens.
* fix(publicurl): include the configured port in the localhost fallback
The last-resort fallback built `http://localhost/...`, which points at port 80
and so is unreachable for a server listening anywhere else — the default 4533
included. Use `conf.Server.Port` so a consumer on the same machine can actually
fetch the URL.
* fix(publicurl): use https in the localhost fallback when TLS is configured
The fallback hardcoded the http scheme, so a TLS-only server with no BaseURL
advertised a URL it does not answer on. Mirror the server's own switch, which
requires both a certificate and a key.
* refactor(publicurl): tidy the localhost fallback and its tests
Use gg.If for the fallback scheme so it reads as an expression, like the
BaseScheme branch above it, instead of assigning http and overwriting it.
Drop two tests the ctx refactor left redundant: one asserted PublicURL "works
without a request" but became a byte-identical copy of the ShareURL spec once
the *http.Request parameter went away, and the two port specs differed only in
the integer, where the non-default port is the stronger assertion.
* refactor(conf): add TLSEnabled and use it instead of repeating the predicate
Whether the server speaks HTTPS was decided inline in three unconnected
places. This PR added the third, in a URL-building package that has no
business inferring the transport config.
Move the rule to conf, next to the fields it derives from, and call it from
publicurl and the insights collector. server.Run keeps its own expression: it
takes the certificate and key as parameters, and its test passes values that
do not come from the config.
* fix(nativeapi): stop partial PUTs from clearing untouched columns
The REST layer parses the request body's top-level JSON keys and passes them
to Repository.Update as colsToUpdate. The radio and library repositories
discarded that list and issued a full-row UPDATE, so any field absent from
the body was written as its zero value.
For radio this wiped uploaded_image, deleting the station's cover on every
partial update (the Web UI is unaffected because its form submits the whole
record). For library it silently cleared remote_path and default_new_users.
Thread the column list through to Put in both repositories, and extract the
column-selection half of filterUpdateValues into selectUpdateColumns so
library, which hand-builds its update map, shares the same rule instead of
copying it.
Fixes#6057
* refactor(persistence): drop pluginRepository's dead rest.Persistable methods
Save and Update had no callers: PUT /api/plugin/{id} is served by the
hand-written updatePlugin handler over a typed request struct, and the
route only wires rest.GetAll and rest.Get. Both methods delegated to Put,
which upserts all twelve columns, so wiring rest.Put to this repository
would have reintroduced the partial-update clobbering fixed in the previous
commit. Removing them, along with the rest.Persistable assertion, makes
that a compile error instead of a silent data loss.
Put itself is unchanged and still backs plugin discovery.
