remote/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-08 10:27:08 +02:00
Code Issues Projects Releases Packages Wiki Activity

Merge branch 'master' into lyrics-source-provenance

Browse source
This commit is contained in:
Yuuta 2026-09-10 03:25:36 +03:00 • committed by GitHub
commit 1bb4cac843
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
276 changed files with 6978 additions and 2329 deletions
Download patch file Download diff file Expand all files Collapse all files

2
.devcontainer/devcontainer.json
View file

@ -4,7 +4,7 @@
"dockerfile": "Dockerfile",
"args": {
// Update the VARIANT arg to pick a version of Go: 1, 1.15, 1.14
"VARIANT": "1.26",
"VARIANT": "1.27",
// Options
"INSTALL_NODE": "true",
"NODE_VERSION": "v24"

60
.github/workflows/coverage-on-pr.yml vendored Normal file
View file

@ -0,0 +1,60 @@
name: Report coverage on PR
on:
workflow_run:
workflows: ['Pipeline: Test, Lint, Build']
types: [completed]
jobs:
comment:
name: Comment coverage report
if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success'
runs-on: ubuntu-latest
permissions:
contents: read
actions: read
pull-requests: write
env:
COVERAGE_COMMENT: 'true'
steps:
# Only the config, from the base branch: this job holds a write token, so
# it must never check out the fork.
- name: Check out the octocov config
uses: actions/checkout@v7
with:
sparse-checkout: .octocov.yml
sparse-checkout-cone-mode: false
persist-credentials: false
# Into a subdirectory. A pull_request run executes the fork's own copy of
# pipeline.yml, so every file in here is attacker-controlled.
- uses: actions/download-artifact@v8
with:
name: octocov-pr
path: untrusted
run-id: ${{ github.event.workflow_run.id }}
github-token: ${{ github.token }}
- name: Verify the artifact and take the coverage profile
id: pr
env:
GH_TOKEN: ${{ github.token }}
HEAD_SHA: ${{ github.event.workflow_run.head_sha }}
run: |
number=$(head -c 20 untrusted/pr_number | tr -d '[:space:]')
case "$number" in ''|*[!0-9]*)
echo "::error::artifact pr_number is not a number"; exit 1;;
esac
sha=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$number" --jq .head.sha)
if [ "$sha" != "$HEAD_SHA" ]; then
echo "::error::artifact claims PR #$number, but its head $sha is not $HEAD_SHA"; exit 1
fi
cp untrusted/coverage.out coverage.out
echo "number=$number" >> "$GITHUB_OUTPUT"
- uses: k1LoW/octocov-action@v1
env:
# A workflow_run job looks like a push to the default branch. Point
# octocov back at the pull request and at the run that produced it.
GITHUB_PULL_REQUEST_NUMBER: ${{ steps.pr.outputs.number }}
OCTOCOV_GITHUB_REF: refs/pull/${{ steps.pr.outputs.number }}/merge
OCTOCOV_GITHUB_SHA: ${{ github.event.workflow_run.head_sha }}
OCTOCOV_GITHUB_RUN_ID: ${{ github.event.workflow_run.id }}

11
.github/workflows/download-link-on-pr.yml vendored
View file

@ -34,16 +34,19 @@ jobs:
}
const {data: {artifacts}} = await github.rest.actions.listWorkflowRunArtifacts({owner, repo, run_id});
if (!artifacts.length) {
const downloadable = artifacts.filter((art) => !art.name.startsWith('octocov-'));
if (!downloadable.length) {
return core.error(`No artifacts found`);
}
let body = `Download the artifacts for this pull request:\n`;
for (const art of artifacts) {
const header = `Download the artifacts for this pull request:`;
let body = `${header}\n`;
for (const art of downloadable) {
body += `\n* [${art.name}.zip](https://nightly.link/${owner}/${repo}/actions/artifacts/${art.id}.zip)`;
}
const {data: comments} = await github.rest.issues.listComments({repo, owner, issue_number});
const existing_comment = comments.find((c) => c.user.login === 'github-actions[bot]');
// Match on the body too: octocov also comments as github-actions[bot].
const existing_comment = comments.find((c) => c.user.login === 'github-actions[bot]' && c.body.startsWith(header));
if (existing_comment) {
core.info(`Updating comment ${existing_comment.id}`);
await github.rest.issues.updateComment({repo, owner, comment_id: existing_comment.id, body});

92
.github/workflows/pipeline.yml vendored
View file

@ -137,8 +137,10 @@ jobs:
- name: Download dependencies
run: go mod download
- name: Test
run: go test -shuffle=on -tags netgo,sqlite_fts5 -race ./... -v
# Name must stay unique across the workflow: octocov matches step names
# by name across every job, and waits for each match to finish.
- name: Test with coverage
run: go test -shuffle=on -tags netgo,sqlite_fts5 -race -v -covermode=atomic -coverprofile=coverage.out $(go list ./... | grep -v '/plugins$')
- name: Test ndpgen
run: |
@ -147,6 +149,84 @@ jobs:
go build -o ndpgen .
./ndpgen --help
- name: Upload coverage profile
uses: actions/upload-artifact@v7
with:
name: octocov-go
path: coverage.out
if-no-files-found: error
go-plugins:
name: Test Go plugins
runs-on: ubuntu-latest
steps:
- name: Check out code into the Go module directory
uses: actions/checkout@v7
- uses: actions/setup-go@v6
id: setup-go
with:
go-version-file: go.mod
# Without this, the suite recompiles every test plugin WASM module,
# which dominates its runtime under -race.
- name: Cache the WASM compilation cache
uses: actions/cache@v6
with:
path: plugins/testdata/.wazero-cache
key: wazero-${{ runner.os }}-go${{ steps.setup-go.outputs.go-version }}-${{ hashFiles('plugins/testdata/*/*.go', 'plugins/testdata/*/go.*', 'plugins/pdk/go/**/*.go', 'plugins/pdk/go/go.*') }}
restore-keys: wazero-${{ runner.os }}-
- name: Test plugins
run: go tool ginkgo -p -race -tags netgo,sqlite_fts5 --cover --covermode=atomic --coverprofile=coverage.out --output-dir=. ./plugins/
- name: Upload coverage profile
uses: actions/upload-artifact@v7
with:
name: octocov-plugins
path: coverage.out
if-no-files-found: error
coverage:
name: Report coverage
runs-on: ubuntu-latest
needs: [go, go-plugins]
permissions:
contents: read
actions: write
env:
COVERAGE_COMMENT: 'false'
steps:
- uses: actions/checkout@v7
- uses: actions/download-artifact@v8
with:
pattern: octocov-*
# Merge here rather than letting octocov do it: octocov reports statement
# coverage for a single profile, but switches to line counting for several.
- name: Merge coverage profiles
run: |
echo "mode: atomic" > coverage.out
awk 'FNR==1 && /^mode:/ {next} {k=$1" "$2; c[k]+=$3} END {for (k in c) print k, c[k]}' \
octocov-*/coverage.out | sort >> coverage.out
- uses: k1LoW/octocov-action@v1
- name: Save the PR number for the comment workflow
if: github.event_name == 'pull_request'
run: echo "${{ github.event.pull_request.number }}" > pr_number
- name: Upload the merged profile for the comment workflow
if: github.event_name == 'pull_request'
uses: actions/upload-artifact@v7
with:
name: octocov-pr
path: |
coverage.out
pr_number
if-no-files-found: error
go-windows:
name: Test Go code (Windows)
runs-on: windows-2022
@ -213,12 +293,12 @@ jobs:
run: go test -shuffle=on -tags netgo,sqlite_fts5 ./... -v
- name: Test ndpgen
shell: pwsh
shell: bash
run: |
cd plugins\cmd\ndpgen
cd plugins/cmd/ndpgen
go test -shuffle=on -v
go build -o ndpgen.exe .
.\ndpgen.exe --help
./ndpgen.exe --help
js:
name: Test JS code
@ -284,7 +364,7 @@ jobs:
build:
name: Build
needs: [js, go, go-windows, go-lint, i18n-lint, git-version, check-push-enabled, validate-migrations]
needs: [js, go, go-plugins, go-windows, go-lint, i18n-lint, git-version, check-push-enabled, validate-migrations]
strategy:
matrix:
platform: [ linux/amd64, linux/arm64, linux/arm/v5, linux/arm/v6, linux/arm/v7, linux/386, linux/riscv64, darwin/amd64, darwin/arm64, windows/amd64, windows/386 ]

6
.gitignore vendored
View file

@ -43,4 +43,8 @@ go.work*
.playwright-mcp/
# Temp benchmark files
zz_*_test.go
zz_*_test.go
# wazero compilation cache for the plugins test suite
/plugins/testdata/.wazero-cache/
/plugins/testdata/*.stage/

44
.octocov.yml Normal file
View file

@ -0,0 +1,44 @@
# Code coverage reporting for pull requests. See https://github.com/k1LoW/octocov
# The 30s default is not enough: scanning this repo's artifacts for the baseline
# eats most of it, leaving none for the report upload.
timeout: 5m
coverage:
# A single pre-merged profile: octocov reports statements for one path, but
# switches to line counting when it merges several itself.
paths:
- coverage.out
# Not code under test: tests/ holds the mocks and helpers, *_gen.go is generated.
# Both patterns need the '**/' prefix: the comment workflow has no source tree,
# so octocov cannot shorten the profile's import paths to repo-relative ones.
exclude:
- '**/tests/**'
- '**/*_gen.go'
codeToTestRatio:
# Needs the pull request's own source, which the comment workflow must not
# check out: it holds a write token.
if: env.COVERAGE_COMMENT != 'true'
code:
- '**/*.go'
- '!**/*_test.go'
- '!**/*_gen.go'
test:
- '**/*_test.go'
testExecutionTime:
if: true
steps:
- Test with coverage
- Test plugins
diff:
datastores:
- artifact://${GITHUB_REPOSITORY}
comment:
# Only the 'Report coverage on PR' workflow sets this: a pull_request run from
# a fork gets a read-only token, so commenting from here 403s.
if: env.COVERAGE_COMMENT == 'true'
updatePrevious: true
summary:
if: true
report:
if: is_default_branch
datastores:
- artifact://${GITHUB_REPOSITORY}

45
Dockerfile
View file

@ -2,7 +2,7 @@ FROM --platform=$BUILDPLATFORM ghcr.io/crazy-max/osxcross:14.5-debian AS osxcros
########################################################################################################################
### Build xx (original image: tonistiigi/xx)
FROM --platform=$BUILDPLATFORM alpine:3.20 AS xx-build
FROM --platform=$BUILDPLATFORM alpine:3.22 AS xx-build
# v1.9.0
ENV XX_VERSION=a5592eab7a57895e8d385394ff12241bc65ecd50
@ -43,7 +43,7 @@ COPY --from=ui /build /build
########################################################################################################################
### Build Navidrome binary for Docker image (dynamic musl, enables native libwebp via dlopen)
FROM --platform=$BUILDPLATFORM golang:1.26-alpine AS build-alpine
FROM --platform=$BUILDPLATFORM golang:1.27-alpine AS build-alpine
COPY --from=xx / /
ARG TARGETPLATFORM
@ -85,7 +85,7 @@ EOT
########################################################################################################################
### Build Navidrome binary for standalone distribution (static glibc, cross-compiled)
FROM --platform=$BUILDPLATFORM golang:1.26-trixie AS base
FROM --platform=$BUILDPLATFORM golang:1.27-trixie AS base
RUN apt-get update && apt-get install -y clang lld
COPY --from=xx / /
WORKDIR /workspace
@ -152,19 +152,52 @@ RUN xx-verify --static /out/navidrome*
FROM scratch AS binary
COPY --from=build /out /
########################################################################################################################
### Build no-op stubs for mpv's video-output libraries
# mpv links libEGL/libgbm for video output only; Navidrome drives it headless, for audio.
# Real mesa pulls in LLVM + gallium (+218MB uncompressed), so ship stubs it never calls.
FROM --platform=$BUILDPLATFORM alpine:3.22 AS mpv-stubs
COPY --from=xx / /
RUN apk add --no-cache clang lld binutils mesa-egl mesa-gbm
ARG TARGETPLATFORM
RUN xx-apk add --no-cache musl-dev
RUN <<EOT
set -e
mkdir -p /out
for so in libEGL.so.1 libgbm.so.1; do
readelf -sW /usr/lib/$so \
| awk '$5 == "GLOBAL" && $7 != "UND" { print $8 }' \
| sed 's/@.*//' \
| grep -vE '^(_init|_fini|_edata|_end|__bss_start|_GLOBAL_OFFSET_TABLE_)$' \
| sort -u \
| awk '{ print "void " $1 "(void) {}" }' > /tmp/stub.c
test -s /tmp/stub.c
xx-clang -shared -nostdlib -fPIC -Wl,-soname,$so -o /out/$so /tmp/stub.c
xx-verify /out/$so
done
EOT
########################################################################################################################
### Build Final Image
FROM alpine:3.20 AS final
FROM alpine:3.22 AS final
LABEL maintainer="deluan@navidrome.org"
LABEL org.opencontainers.image.source="https://github.com/navidrome/navidrome"
# Install runtime dependencies
# - libwebp + symlinks: enables native WebP encoding via purego/dlopen
RUN apk add -U --no-cache ffmpeg mpv sqlite libwebp libwebpdemux libwebpmux && \
# The mesa/LLVM stack mpv pulls in for video output is dropped in this same layer,
# otherwise the deleted bytes still ship in the image.
RUN apk add -U --no-cache curl ffmpeg mpv sqlite libwebp libwebpdemux libwebpmux && \
for lib in libwebp libwebpdemux libwebpmux; do \
target=$(ls /usr/lib/$lib.so.* 2>/dev/null | head -1) && \
[ -n "$target" ] && ln -sf "$target" /usr/lib/$lib.so; \
done
done && \
rm -rf /usr/lib/gallium-pipe /usr/lib/dri \
/usr/lib/libEGL.so* /usr/lib/libgbm.so* /usr/lib/libgallium*.so /usr/lib/libLLVM.so* \
/usr/lib/libGL.so* /usr/lib/libGLESv2.so* /usr/lib/libglapi.so*
COPY --from=mpv-stubs /out/ /usr/lib/
RUN mpv --no-video --ao=null --version > /dev/null
# Copy navidrome binary (musl build for Docker, enables native libwebp)
COPY --from=build-alpine /out/navidrome /app/

2
Makefile
View file

@ -20,7 +20,7 @@ IMAGE_PLATFORMS ?= $(shell echo $(SUPPORTED_PLATFORMS) | tr ',' '\n' | grep "lin
PLATFORMS ?= $(SUPPORTED_PLATFORMS)
DOCKER_TAG ?= deluan/navidrome:develop
GOLANGCI_LINT_VERSION ?= v2.12.0
GOLANGCI_LINT_VERSION ?= v2.13.2
UI_SRC_FILES := $(shell find ui -type f -not -path "ui/build/*" -not -path "ui/node_modules/*")

44
adapters/deezer/client.go
View file

@ -13,15 +13,26 @@ import (
"strings"
"github.com/microcosm-cc/bluemonday"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/log"
)
const apiBaseURL = "https://api.deezer.com"
const authBaseURL = "https://auth.deezer.com"
var (
ErrNotFound = errors.New("deezer: not found")
)
// errCodeQuota is Deezer's "Quota limit exceeded"; it arrives in the body, with HTTP 200
// and no rate-limit headers, so the body code is the only signal.
const errCodeQuota = 4
type deezerError struct {
Type string `json:"type"`
Message string `json:"message"`
Code int `json:"code"`
}
func (e *deezerError) Error() string {
return fmt.Sprintf("deezer error(%d): %s", e.Code, e.Message)
}
type httpDoer interface {
Do(req *http.Request) (*http.Response, error)
@ -56,7 +67,7 @@ func (c *client) searchArtists(ctx context.Context, name string, limit int) ([]A
}
if len(results.Data) == 0 {
return nil, ErrNotFound
return nil, agents.ErrNotFound
}
return results.Data, nil
}
@ -74,20 +85,31 @@ func (c *client) makeRequest(req *http.Request, response any) error {
return err
}
// Checked before the status: a throttled request still answers 200, and decoding its body
// into a result type yields an empty one, which reads as "nothing found".
if err := parseBodyError(data); err != nil {
return err
}
if resp.StatusCode != 200 {
return c.parseError(data)
return fmt.Errorf("deezer http status: (%d)", resp.StatusCode)
}
return json.Unmarshal(data, response)
}
func (c *client) parseError(data []byte) error {
var deezerError Error
err := json.Unmarshal(data, &deezerError)
if err != nil {
return err
// parseBodyError returns the error Deezer reported in the body, or nil when it reported none.
func parseBodyError(data []byte) error {
var body errorResponse
// Discarded: a payload that is not an error object leaves Error nil, which is the "none" answer.
_ = json.Unmarshal(data, &body)
switch {
case body.Error == nil:
return nil
case body.Error.Code == errCodeQuota:
return errors.Join(body.Error, agents.ErrRetryLater)
default:
return body.Error
}
return fmt.Errorf("deezer error(%d): %s", deezerError.Error.Code, deezerError.Error.Message)
}
func (c *client) getRelatedArtists(ctx context.Context, artistID int) ([]Artist, error) {

34
adapters/deezer/client_test.go
View file

@ -2,12 +2,14 @@ package deezer
import (
"bytes"
"errors"
"fmt"
"io"
"net/http"
"os"
"time"
"github.com/navidrome/navidrome/core/agents"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
@ -41,7 +43,37 @@ var _ = Describe("client", func() {
})
_, err := client.searchArtists(GinkgoT().Context(), "Michael Jackson", 20)
Expect(err).To(MatchError(ErrNotFound))
Expect(err).To(MatchError(agents.ErrNotFound))
})
// Deezer answers 200 with no rate-limit headers when throttling, so this body is the only signal.
It("reports an exhausted quota as a retryable error, not as a missing artist", func() {
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(
`{"error":{"type":"Exception","message":"Quota limit exceeded","code":4}}`)),
})
_, err := client.searchArtists(GinkgoT().Context(), "Michael Jackson", 20)
Expect(err).To(HaveOccurred())
Expect(err).ToNot(MatchError(agents.ErrNotFound),
"a throttled lookup would otherwise settle the artist as having no image")
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
Expect(err.Error()).To(ContainSubstring("Quota limit exceeded"))
})
It("reports a non-quota body error as a plain error", func() {
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(
`{"error":{"type":"Exception","message":"Invalid query","code":100}}`)),
})
_, err := client.searchArtists(GinkgoT().Context(), "Michael Jackson", 20)
Expect(err).To(HaveOccurred())
Expect(err).ToNot(MatchError(agents.ErrNotFound))
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeFalse(),
"only a throttle asks the caller to come back later")
})
})

9
adapters/deezer/deezer.go
View file

@ -5,7 +5,6 @@ import (
"context"
"errors"
"fmt"
"net/http"
"slices"
"strings"
@ -15,6 +14,7 @@ import (
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/cache"
"github.com/navidrome/navidrome/utils/httpclient"
"github.com/navidrome/navidrome/utils/slice"
)
@ -36,9 +36,7 @@ func deezerConstructor(dataStore model.DataStore) agents.Interface {
dataStore: dataStore,
languages: conf.Server.Deezer.Languages,
}
httpClient := &http.Client{
Timeout: consts.DefaultHttpClientTimeOut,
}
httpClient := httpclient.New(consts.DefaultHttpClientTimeOut)
cachedHttpClient := cache.NewHTTPClient(httpClient, consts.DefaultHttpClientTimeOut)
agent.client = newClient(cachedHttpClient)
return agent
@ -93,9 +91,6 @@ func isPlaceholderPicture(url string) bool {
func (s *deezerAgent) searchArtist(ctx context.Context, name string) (*Artist, error) {
artists, err := s.client.searchArtists(ctx, name, deezerArtistSearchLimit)
if errors.Is(err, ErrNotFound) || len(artists) == 0 {
return nil, agents.ErrNotFound
}
if err != nil {
return nil, err
}

17
adapters/deezer/deezer_test.go
View file

@ -3,6 +3,7 @@ package deezer
import (
"bytes"
"context"
"errors"
"fmt"
"io"
"net/http"
@ -80,6 +81,22 @@ var _ = Describe("deezerAgent", func() {
Expect(artist.ID).To(Equal(2))
})
// The artwork worker settles an artist as "no image" on agents.ErrNotFound, so a throttled
// lookup reaching that here would record a permanent absence.
It("surfaces an exhausted quota instead of reporting the artist as not found", func() {
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(
`{"error":{"type":"Exception","message":"Quota limit exceeded","code":4}}`)),
})
_, err := agent.searchArtist(ctx, "Queen")
Expect(err).To(HaveOccurred())
Expect(err).ToNot(MatchError(agents.ErrNotFound))
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
})
It("returns ErrNotFound when no result matches the name exactly", func() {
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
StatusCode: 200,

8
adapters/deezer/responses.go
View file

@ -22,12 +22,8 @@ type Artist struct {
Type string `json:"type"`
}
type Error struct {
Error struct {
Type string `json:"type"`
Message string `json:"message"`
Code int `json:"code"`
} `json:"error"`
type errorResponse struct {
Error *deezerError `json:"error"`
}
type RelatedArtists struct {

2
adapters/deezer/responses_test.go
View file

@ -26,7 +26,7 @@ var _ = Describe("Responses", func() {
Describe("Error", func() {
It("parses the error response correctly", func() {
var errorResp Error
var errorResp errorResponse
body := []byte(`{"error":{"type":"MissingParameterException","message":"Missing parameters: q","code":501}}`)
err := json.Unmarshal(body, &errorResp)
Expect(err).To(BeNil())

8
adapters/lastfm/agent.go
View file

@ -18,6 +18,7 @@ import (
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/cache"
"github.com/navidrome/navidrome/utils/httpclient"
"golang.org/x/net/html"
)
@ -59,9 +60,7 @@ func lastFMConstructor(ds model.DataStore) *lastfmAgent {
secret: conf.Server.LastFM.Secret,
sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty},
}
hc := &http.Client{
Timeout: consts.DefaultHttpClientTimeOut,
}
hc := httpclient.New(consts.DefaultHttpClientTimeOut)
chc := cache.NewHTTPClient(hc, consts.DefaultHttpClientTimeOut)
l.httpClient = chc
l.client = newClient(l.apiKey, l.secret, chc)
@ -406,7 +405,8 @@ func (l *lastfmAgent) Scrobble(ctx context.Context, userId string, s scrobbler.S
log.Warn(ctx, "Last.fm client.scrobble returned error", "track", s.Title, err)
return errors.Join(err, scrobbler.ErrRetryLater)
}
if lfErr.Code == 11 || lfErr.Code == 16 {
// 11: service offline; 16: temporarily unavailable. Rate limiting is mapped by the client.
if lfErr.Code == 11 || lfErr.Code == 16 || errors.Is(err, scrobbler.ErrRetryLater) {
return errors.Join(err, scrobbler.ErrRetryLater)
}
return errors.Join(err, scrobbler.ErrUnrecoverable)

19
adapters/lastfm/agent_test.go
View file

@ -100,6 +100,15 @@ var _ = Describe("lastfmAgent", func() {
Expect(httpClient.RequestCount).To(Equal(1))
Expect(httpClient.SavedRequest.URL.Query().Get("artist")).To(Equal("U2"))
})
It("returns ErrRetryLater on error 29 (rate limit exceeded)", func() {
httpClient.Res = http.Response{
Body: io.NopCloser(bytes.NewBufferString(`{"error":29,"message":"Rate limit exceeded"}`)),
StatusCode: 200,
}
_, err := agent.GetArtistBiography(ctx, "123", "U2", "")
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
})
})
Describe("Language Fallback", func() {
@ -497,6 +506,16 @@ var _ = Describe("lastfmAgent", func() {
Expect(err).To(MatchError(scrobbler.ErrRetryLater))
})
It("returns ErrRetryLater on error 29 (rate limit exceeded)", func() {
httpClient.Res = http.Response{
Body: io.NopCloser(bytes.NewBufferString(`{"error":29,"message":"Rate limit exceeded"}`)),
StatusCode: 200,
}
err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()})
Expect(errors.Is(err, scrobbler.ErrRetryLater)).To(BeTrue())
})
It("returns ErrRetryLater on http errors", func() {
httpClient.Res = http.Response{
Body: io.NopCloser(bytes.NewBufferString(`internal server error`)),

5
adapters/lastfm/auth_router.go
View file

@ -18,6 +18,7 @@ import (
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/server"
"github.com/navidrome/navidrome/utils/httpclient"
"github.com/navidrome/navidrome/utils/req"
)
@ -41,9 +42,7 @@ func NewRouter(ds model.DataStore) *Router {
sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty},
}
r.Handler = r.routes()
hc := &http.Client{
Timeout: consts.DefaultHttpClientTimeOut,
}
hc := httpclient.New(consts.DefaultHttpClientTimeOut)
r.client = newClient(r.apiKey, r.secret, hc)
return r
}

9
adapters/lastfm/auth_router_test.go
View file

@ -214,5 +214,14 @@ var _ = Describe("auth_router", func() {
_, err = verifyLinkToken(nonExpiringToken)
Expect(err).To(MatchError("link token missing expiration"))
})
It("rejects a Jellyfin access token", func() {
usr := &model.User{ID: "u1", UserName: "johndoe"}
tokenStr, err := auth.CreateAPIToken(usr, auth.AudienceJellyfin)
Expect(err).ToNot(HaveOccurred())
_, err = verifyLinkToken(tokenStr)
Expect(err).To(HaveOccurred())
})
})
})

11
adapters/lastfm/client.go
View file

@ -5,6 +5,7 @@ import (
"crypto/md5"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/url"
@ -14,11 +15,15 @@ import (
"strings"
"time"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/log"
)
const (
apiBaseUrl = "https://ws.audioscrobbler.com/2.0/"
// errCodeRateLimit is Last.fm's "rate limit exceeded"; it arrives in the body, with HTTP 200
// and no rate-limit headers, so the body code is the only signal.
errCodeRateLimit = 29
)
type lastFMError struct {
@ -225,7 +230,11 @@ func (c *client) makeRequest(ctx context.Context, method string, params url.Valu
return nil, jsonErr
}
if response.Error != 0 {
return &response, &lastFMError{Code: response.Error, Message: response.Message}
var err error = &lastFMError{Code: response.Error, Message: response.Message}
if response.Error == errCodeRateLimit {
err = errors.Join(err, &agents.RetryLaterError{})
}
return &response, err
}
return &response, nil

6
adapters/listenbrainz/agent.go
View file

@ -3,7 +3,6 @@ package listenbrainz
import (
"context"
"errors"
"net/http"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
@ -12,6 +11,7 @@ import (
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/cache"
"github.com/navidrome/navidrome/utils/httpclient"
"github.com/navidrome/navidrome/utils/slice"
)
@ -33,9 +33,7 @@ func listenBrainzConstructor(ds model.DataStore) *listenBrainzAgent {
sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty},
baseURL: conf.Server.ListenBrainz.BaseURL,
}
hc := &http.Client{
Timeout: consts.DefaultHttpClientTimeOut,
}
hc := httpclient.New(consts.DefaultHttpClientTimeOut)
chc := cache.NewHTTPClient(hc, consts.DefaultHttpClientTimeOut)
l.client = newClient(l.baseURL, chc)
return l

13
adapters/listenbrainz/agent_test.go
View file

@ -164,6 +164,19 @@ var _ = Describe("listenBrainzAgent", func() {
err := agent.Scrobble(ctx, "user-1", sc)
Expect(err).To(MatchError(scrobbler.ErrUnrecoverable))
})
It("keeps a 429 scrobble for retry and carries the delay", func() {
httpClient.Res = http.Response{
StatusCode: 429,
Header: http.Header{"X-Ratelimit-Reset-In": []string{"7"}},
Body: io.NopCloser(bytes.NewBufferString(`{"code":429,"error":"rate limited"}`)),
}
err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()})
Expect(errors.Is(err, scrobbler.ErrRetryLater)).To(BeTrue())
retry, ok := errors.AsType[*agents.RetryLaterError](err)
Expect(ok).To(BeTrue())
Expect(retry.RetryIn).To(Equal(7 * time.Second))
})
})
Describe("GetArtistUrl", func() {

5
adapters/listenbrainz/auth_router.go
View file

@ -16,6 +16,7 @@ import (
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/server"
"github.com/navidrome/navidrome/utils/httpclient"
)
type sessionKeysRepo interface {
@ -37,9 +38,7 @@ func NewRouter(ds model.DataStore) *Router {
sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty},
}
r.Handler = r.routes()
hc := &http.Client{
Timeout: consts.DefaultHttpClientTimeOut,
}
hc := httpclient.New(consts.DefaultHttpClientTimeOut)
r.client = newClient(conf.Server.ListenBrainz.BaseURL, hc)
return r
}

17
adapters/listenbrainz/client.go
View file

@ -13,6 +13,7 @@ import (
"slices"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/log"
)
@ -21,6 +22,12 @@ const (
labsBase = "https://labs.api.listenbrainz.org/"
)
// retryLaterErr reads the wait ListenBrainz asked for. It sends X-RateLimit-Reset-In
// (delta-seconds) on every response, including the 429, and never Retry-After.
func retryLaterErr(h http.Header) *agents.RetryLaterError {
return &agents.RetryLaterError{RetryIn: agents.ParseRetryIn(h.Get("X-RateLimit-Reset-In"))}
}
var (
ErrorNotFound = errors.New("listenbrainz: not found")
)
@ -174,6 +181,9 @@ func (c *client) makeAuthenticatedRequest(ctx context.Context, method string, en
}
defer resp.Body.Close()
if resp.StatusCode == http.StatusTooManyRequests {
return nil, retryLaterErr(resp.Header)
}
decoder := json.NewDecoder(resp.Body)
var response listenBrainzResponse
@ -185,6 +195,10 @@ func (c *client) makeAuthenticatedRequest(ctx context.Context, method string, en
return nil, jsonErr
}
if response.Code != 0 && response.Code != 200 {
// LB also reports rate limiting as a body code, not only as an HTTP status.
if response.Code == http.StatusTooManyRequests {
return &response, retryLaterErr(resp.Header)
}
return &response, &listenBrainzError{Code: response.Code, Message: response.Error}
}
@ -211,6 +225,9 @@ func (c *client) makeGenericRequest(ctx context.Context, method string, endpoint
// On a 200 code, there is no code. Decode using using error message if it exists
if resp.StatusCode != 200 {
defer resp.Body.Close()
if resp.StatusCode == http.StatusTooManyRequests {
return nil, retryLaterErr(resp.Header)
}
decoder := json.NewDecoder(resp.Body)
var lbzError lbzHttpError

73
adapters/listenbrainz/client_test.go
View file

@ -4,13 +4,17 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"os"
"strings"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@ -461,4 +465,73 @@ var _ = Describe("client", func() {
}))
})
})
Describe("rate limiting", func() {
It("returns RetryLaterError with the header delay on 429", func() {
httpClient.Res = http.Response{
StatusCode: 429,
Header: http.Header{"X-Ratelimit-Reset-In": []string{"3"}},
Body: io.NopCloser(strings.NewReader(`{"code":429,"error":"You have exceeded your rate limit."}`)),
}
_, err := client.validateToken(context.Background(), "token")
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
retry, ok := errors.AsType[*agents.RetryLaterError](err)
Expect(ok).To(BeTrue())
Expect(retry.RetryIn).To(Equal(3 * time.Second))
})
It("returns RetryLaterError with zero delay when no header is present", func() {
httpClient.Res = http.Response{
StatusCode: 429,
Body: io.NopCloser(strings.NewReader(`{"code":429,"error":"rate limited"}`)),
}
_, err := client.validateToken(context.Background(), "token")
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
retry, _ := errors.AsType[*agents.RetryLaterError](err)
Expect(retry.RetryIn).To(BeZero())
})
DescribeTable("caps absurd header values at one hour",
func(header string) {
httpClient.Res = http.Response{
StatusCode: 429,
Header: http.Header{"X-Ratelimit-Reset-In": []string{header}},
Body: io.NopCloser(strings.NewReader(`{"code":429,"error":"rate limited"}`)),
}
_, err := client.validateToken(context.Background(), "token")
retry, _ := errors.AsType[*agents.RetryLaterError](err)
Expect(retry.RetryIn).To(Equal(time.Hour))
},
Entry("a large value", "999999"),
Entry("a huge value", "99999999999"),
// Scaling this to nanoseconds before capping wraps past 2^64, landing on ~0.29s.
Entry("a value that overflows int64 nanoseconds", "18446744074"),
)
It("maps a body-level 429 sent with a non-429 status", func() {
httpClient.Res = http.Response{
StatusCode: 200,
Header: http.Header{"X-Ratelimit-Reset-In": []string{"7"}},
Body: io.NopCloser(strings.NewReader(`{"code":429,"error":"You have exceeded your rate limit."}`)),
}
_, err := client.validateToken(context.Background(), "token")
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
retry, ok := errors.AsType[*agents.RetryLaterError](err)
Expect(ok).To(BeTrue())
Expect(retry.RetryIn).To(Equal(7 * time.Second))
})
It("returns RetryLaterError on a 429 from makeGenericRequest", func() {
httpClient.Res = http.Response{
StatusCode: 429,
Header: http.Header{"X-Ratelimit-Reset-In": []string{"5"}},
Body: io.NopCloser(strings.NewReader(`{"code":429,"error":"rate limited"}`)),
}
_, err := client.getArtistUrl(context.Background(), "1")
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
retry, ok := errors.AsType[*agents.RetryLaterError](err)
Expect(ok).To(BeTrue())
Expect(retry.RetryIn).To(Equal(5 * time.Second))
})
})
})

184
cmd/artwork.go
View file

@ -42,9 +42,10 @@ func init() {
"stored trace of the last resolution; also initializes plugin agents, which may open "+
"external connections")
artworkReprocessCmd.Flags().StringSliceVar(&artworkKinds, "kind", nil,
"kinds to reprocess ("+kindPrefixes(artwork.RecheckKinds)+"); repeatable")
"kinds to reprocess ("+kindPrefixes(artwork.ReprocessKinds)+"); repeatable")
artworkReprocessCmd.Flags().StringSliceVar(&artworkSources, "source", nil,
"only items currently resolved from these sources (e.g. folder, external:deezer, absent)")
"only items currently resolved from these sources (e.g. folder, external:deezer, absent, "+
"or failed for the absent ones that gave up)")
artworkReprocessCmd.Flags().BoolVar(&artworkAll, "all", false, "reprocess every kind")
artworkReprocessCmd.Flags().BoolVar(&artworkDryRun, "dry-run", false,
"report what would be queued and exit without queueing")
@ -113,7 +114,7 @@ var artworkCancelCmd = &cobra.Command{
"Work already picked up is not interrupted, and an item with no artwork yet can be\n" +
"queued again by the hourly re-check. The selection is applied again when you confirm,\n" +
"so anything queued after the preview is cancelled too. Use it to call off a bulk\n" +
"backfill, not to stop the worker.",
"reprocess, not to stop the worker.",
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, args []string) {
runCancel(cmd.Context())
@ -122,7 +123,7 @@ var artworkCancelCmd = &cobra.Command{
var artworkStatusCmd = &cobra.Command{
Use: "status",
Short: "Report the artwork queue, where artwork resolves from, and the backfill state",
Short: "Report the artwork queue, where artwork resolves from, and the config state",
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, args []string) {
runStatus(cmd.Context())
@ -146,9 +147,11 @@ type sourceCount struct {
count int64
}
// absentCount partitions a kind's absent states: noImage was answered, failed gave up.
type absentCount struct {
kind model.Kind
model.ArtworkAbsentStat
kind model.Kind
noImage int64
failed int64
}
type statusReport struct {
@ -170,16 +173,6 @@ func queueTotal(stats []model.ArtworkQueueStat) int64 {
return n
}
func (r statusReport) backfillQueued() int64 {
var n int64
for _, s := range r.queue {
if s.Priority == model.ArtworkPriorityBackfill {
n += s.Count
}
}
return n
}
func collectStatus(ctx context.Context, ds model.DataStore) (statusReport, error) {
q := ds.ArtworkQueue(ctx)
var rep statusReport
@ -188,8 +181,7 @@ func collectStatus(ctx context.Context, ds model.DataStore) (statusReport, error
return rep, fmt.Errorf("breaking the artwork queue down by kind: %w", err)
}
cutoff := time.Now().Add(-artwork.StaleAbsentAge)
for _, k := range artwork.RecheckKinds {
for _, k := range artwork.ReprocessKinds {
sources, err := q.SourcesInUse(k)
if err != nil {
return rep, fmt.Errorf("listing the sources in use by %s artwork: %w", k, err)
@ -201,12 +193,15 @@ func collectStatus(ctx context.Context, ds model.DataStore) (statusReport, error
return rep, fmt.Errorf("counting %s artwork resolved from %s: %w", k, displaySource(s), err)
}
rep.sources = append(rep.sources, sourceCount{kind: k, source: s, count: n})
// An absent state is exactly a row with no source, so it needs no second query.
if s == "" {
failed, err := q.CountBySource(k, []string{model.ArtworkSourceFailed})
if err != nil {
return rep, fmt.Errorf("counting failed %s artwork: %w", k, err)
}
rep.absent = append(rep.absent, absentCount{kind: k, noImage: n - failed, failed: failed})
}
}
stat, err := q.CountAbsent(k, cutoff)
if err != nil {
return rep, fmt.Errorf("counting absent %s artwork: %w", k, err)
}
rep.absent = append(rep.absent, absentCount{kind: k, ArtworkAbsentStat: stat})
}
rep.current, rep.inputs = artwork.ConfigFingerprint(), artwork.FingerprintInputs()
@ -234,19 +229,20 @@ func formatStatus(rep statusReport) string {
}
fmt.Fprintln(w, "\nAbsent (resolved, no image found)")
fmt.Fprintln(w, " KIND\tABSENT\tDUE FOR RECHECK")
fmt.Fprintln(w, " KIND\tNO IMAGE\tFAILED")
for _, a := range rep.absent {
fmt.Fprintf(w, " %s\t%d\t%d\n", a.kind, a.Total, a.Stale)
fmt.Fprintf(w, " %s\t%d\t%d\n", a.kind, a.noImage, a.failed)
}
fmt.Fprintf(w, " (eligible once the last attempt is older than %gh; re-queued %d per kind per hour, oldest first)\n",
artwork.StaleAbsentAge.Hours(), artwork.StaleAbsentRecheckBatch)
fmt.Fprintln(w, " (nothing retries these; 'artwork reprocess --source absent' retries both columns)")
fmt.Fprintln(w, " (failed = gave up rather than being answered, so the ones most likely to resolve;\n"+
" 'artwork reprocess --source failed' retries just those)")
fmt.Fprintln(w, "\nBackfill")
fmt.Fprintf(w, " State:\t%s\n", backfillState(rep))
fmt.Fprintln(w, "\nConfig")
fmt.Fprintf(w, " State:\t%s\n", configState(rep))
fmt.Fprintf(w, " Stored fingerprint:\t%s\n", cmp.Or(rep.stored, "(none)"))
fmt.Fprintf(w, " Current fingerprint:\t%s\n", rep.current)
if len(rep.inputs) > 0 {
fmt.Fprintln(w, " Fingerprint inputs (changing any of these re-resolves the whole library):")
fmt.Fprintln(w, " Fingerprint inputs (changing any of these makes the stored artwork stale):")
for _, in := range rep.inputs {
fmt.Fprintf(w, " %s:\t%s\n", in.Name, in.Value)
}
@ -256,18 +252,10 @@ func formatStatus(rep statusReport) string {
return sb.String()
}
// backfillState leads with the queued backlog: by the time anyone runs this, backfill has usually
// already stored the new fingerprint, and "up to date" would bury the flood it is still working through.
func backfillState(rep statusReport) string {
pending := "fingerprint changed — every artist, album, playlist and radio will be re-enqueued on the next startup"
if n := rep.backfillQueued(); n > 0 {
if rep.stored != rep.current {
return fmt.Sprintf("backfill running: %d items queued, and %s", n, pending)
}
return fmt.Sprintf("backfill running: %d items queued (fingerprint up to date)", n)
}
func configState(rep statusReport) string {
if rep.stored != rep.current {
return pending
return "fingerprint changed — stored artwork keeps the old resolution; " +
"run 'artwork reprocess --all' to apply it"
}
return "up to date"
}
@ -297,8 +285,8 @@ type artworkPriority struct {
var knownPriorities = []artworkPriority{
{"bump", model.ArtworkPriorityBump},
{"scan", model.ArtworkPriorityScan},
{"backfill", model.ArtworkPriorityBackfill},
{"recheck", model.ArtworkPriorityRecheck},
{"backfill", model.ArtworkPriorityBackfill},
}
// priorityName falls back to the number: a row written by a newer version still has to print.
@ -351,29 +339,41 @@ func runReprocess(ctx context.Context) {
func selectedKinds(kinds, sources []string, all bool) ([]model.Kind, error) {
// A source filter on its own is already a complete selection, so it does not also need a kind.
if all || (len(kinds) == 0 && len(sources) > 0) {
return artwork.RecheckKinds, nil
return artwork.ReprocessKinds, nil
}
if len(kinds) == 0 {
return nil, fmt.Errorf("no selector given: pass --kind, --source or --all")
}
return parseAll(kinds, func(s string) (model.Kind, error) {
return parseArtworkKind(s, artwork.RecheckKinds)
return parseArtworkKind(s, artwork.ReprocessKinds)
})
}
// absentSource is how the stored empty source — resolved, no image — is spelled on the CLI.
const absentSource = "absent"
// absentSource is how the stored empty source — resolved, no image — is spelled on the CLI, and
// failedSource the subset of it that gave up rather than being answered.
const (
absentSource = "absent"
failedSource = "failed"
)
func repositorySources(sources []string) []string {
return slice.Map(sources, func(s string) string {
if s == absentSource {
switch s {
case absentSource:
return ""
case failedSource:
return model.ArtworkSourceFailed
}
return s
})
}
func displaySource(s string) string { return cmp.Or(s, absentSource) }
func displaySource(s string) string {
if s == model.ArtworkSourceFailed {
return failedSource
}
return cmp.Or(s, absentSource)
}
type confirmFunc func(out io.Writer, total, external int64) bool
@ -447,7 +447,7 @@ func validateSources(q model.ArtworkQueueRepository, sources []string) error {
return nil
}
var inUse []string
for _, k := range artwork.RecheckKinds {
for _, k := range artwork.ReprocessKinds {
found, err := q.SourcesInUse(k)
if err != nil {
return fmt.Errorf("listing the sources in use by %s artwork: %w", k, err)
@ -456,14 +456,16 @@ func validateSources(q model.ArtworkQueueRepository, sources []string) error {
}
var unknown []string
for _, s := range sources {
if s != "" && !slices.Contains(inUse, s) { // the reserved absent source is valid even when nothing is absent
// The reserved absent and failed sources are valid even when nothing currently matches them.
if s != "" && s != model.ArtworkSourceFailed && !slices.Contains(inUse, s) {
unknown = append(unknown, displaySource(s))
}
}
if len(unknown) == 0 {
return nil
}
valid := slice.Map(inUse, displaySource)
// failed is accepted but never stored, so listing only what is in use would hide it.
valid := append(slice.Map(inUse, displaySource), failedSource)
slices.Sort(valid)
return fmt.Errorf("no artwork resolves from %s; sources in use: %s",
strings.Join(unknown, ", "), cmp.Or(strings.Join(valid, ", "), "(none)"))
@ -478,6 +480,18 @@ func reprocessArtwork(ctx context.Context, ds model.DataStore, kinds []model.Kin
return err
}
// Derived from what actually drives the queries, so a filter added to this signature cannot
// silently keep stamping the fingerprint for a partial run.
markApplied := func() error {
if len(sources) > 0 || len(kinds) < len(artwork.ReprocessKinds) {
return nil
}
if err := artwork.MarkConfigApplied(ctx, ds); err != nil {
return fmt.Errorf("recording the applied artwork config: %w", err)
}
return nil
}
matched := make([]int64, len(kinds))
var total, external int64
for i, k := range kinds {
@ -496,8 +510,9 @@ func reprocessArtwork(ctx context.Context, ds model.DataStore, kinds []model.Kin
fmt.Fprintln(out, "\nDry run: nothing was queued.")
return nil
case total == 0:
// An empty match set still leaves nothing resolved under the old config.
fmt.Fprintln(out, "Nothing was queued.")
return nil
return markApplied()
case !confirm(out, total, external):
fmt.Fprintln(out, "Aborted: nothing was queued.")
return nil
@ -519,7 +534,7 @@ func reprocessArtwork(ctx context.Context, ds model.DataStore, kinds []model.Kin
if skipped := total - queued; skipped > 0 {
fmt.Fprintf(out, "Already queued, left unchanged: %d (priority and retry backoff untouched).\n", skipped)
}
return nil
return markApplied()
}
func runCancel(ctx context.Context) {
@ -546,7 +561,7 @@ func cancelSelection(kinds, priorities []string, all bool) ([]model.Kind, []int,
if len(kinds) == 0 && len(priorities) == 0 {
return nil, nil, fmt.Errorf("no selector given: pass --kind, --priority or --all")
}
// RefreshableKinds, not RecheckKinds: media files are queued, so --kind must reach them.
// RefreshableKinds, not ReprocessKinds: media files are queued, so --kind must reach them.
outKinds, err := parseAll(kinds, func(s string) (model.Kind, error) {
return parseArtworkKind(s, artwork.RefreshableKinds)
})
@ -648,7 +663,7 @@ func refreshItems(ctx context.Context, ds model.DataStore, targets []model.Artwo
for _, t := range targets {
kind, id := t.Kind, t.ID
// artwork.Refresh would happily queue an id that does not exist, orphaning a queue row.
if _, err := artworkItemName(ctx, ds, kind, id); err != nil {
if _, err := artwork.ItemName(ctx, ds, kind, id); err != nil {
log.Error(ctx, "Item not found", "kind", kind, "id", id, err)
failed++
continue
@ -963,7 +978,7 @@ func runExplain(ctx context.Context, args []string) {
}
kind, id := targets[0].Kind, targets[0].ID
name, err := artworkItemName(ctx, ds, kind, id)
name, err := artwork.ItemName(ctx, ds, kind, id)
if err != nil {
log.Fatal(ctx, "Item not found", "kind", kind, "id", id, err)
}
@ -1005,60 +1020,3 @@ func runExplain(ctx context.Context, args []string) {
log.Fatal(ctx, "Failed to resolve artwork", "kind", kind, "id", id, rep.resolveErr)
}
}
// artworkItemName looks the entity up under its own kind, so a mismatched kind/id pair is
// reported as not found instead of silently explaining another entity's artwork.
func artworkItemName(ctx context.Context, ds model.DataStore, kind model.Kind, id string) (string, error) {
switch kind {
case model.KindArtistArtwork:
ar, err := ds.Artist(ctx).Get(id)
if err != nil {
return "", err
}
return ar.Name, nil
case model.KindAlbumArtwork:
al, err := ds.Album(ctx).Get(id)
if err != nil {
return "", err
}
return al.Name, nil
case model.KindPlaylistArtwork:
pls, err := ds.Playlist(ctx).Get(id)
if err != nil {
return "", err
}
return pls.Name, nil
case model.KindRadioArtwork:
rd, err := ds.Radio(ctx).Get(id)
if err != nil {
return "", err
}
return rd.Name, nil
case model.KindMediaFileArtwork:
mf, err := ds.MediaFile(ctx).Get(id)
if err != nil {
return "", err
}
return mf.Title, nil
case model.KindDiscArtwork:
return discArtworkName(ctx, ds, id)
}
return "", fmt.Errorf("unsupported kind %q", kind.Prefix())
}
func discArtworkName(ctx context.Context, ds model.DataStore, id string) (string, error) {
albumID, discNumber, err := model.ParseDiscArtworkID(id)
if err != nil {
return "", err
}
al, err := ds.Album(ctx).Get(albumID)
if err != nil {
return "", err
}
name := fmt.Sprintf("%s (disc %d)", al.Name, discNumber)
// The subtitle is itself a DiscArtPriority candidate, so name it where the chain can be read against it.
if subtitle := strings.TrimSpace(al.Discs[discNumber]); subtitle != "" {
name += ": " + subtitle
}
return name, nil
}

128
cmd/artwork_test.go
View file

@ -19,20 +19,20 @@ import (
var _ = Describe("parseArtworkKind", func() {
It("accepts a supported kind", func() {
k, err := parseArtworkKind("ar", artwork.RecheckKinds)
k, err := parseArtworkKind("ar", artwork.ReprocessKinds)
Expect(err).ToNot(HaveOccurred())
Expect(k).To(Equal(model.KindArtistArtwork))
})
It("rejects an unknown kind and lists the valid ones", func() {
_, err := parseArtworkKind("zz", artwork.RecheckKinds)
_, err := parseArtworkKind("zz", artwork.ReprocessKinds)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("ar"))
Expect(err.Error()).To(ContainSubstring("al"))
})
It("rejects a known kind the command does not accept", func() {
_, err := parseArtworkKind("mf", artwork.RecheckKinds)
_, err := parseArtworkKind("mf", artwork.ReprocessKinds)
Expect(err).To(HaveOccurred())
})
@ -423,33 +423,6 @@ var _ = Describe("explainConfig", func() {
)
})
var _ = Describe("discArtworkName", func() {
var ds *tests.MockDataStore
BeforeEach(func() {
albumRepo := tests.CreateMockAlbumRepo()
albumRepo.SetData(model.Albums{{ID: "al-1", Name: "Sandinista!", Discs: model.Discs{2: "Side Three"}}})
ds = &tests.MockDataStore{MockedAlbum: albumRepo}
})
It("names the album, the disc and its subtitle", func() {
name, err := artworkItemName(context.Background(), ds, model.KindDiscArtwork, "al-1:2")
Expect(err).ToNot(HaveOccurred())
Expect(name).To(Equal("Sandinista! (disc 2): Side Three"))
})
It("omits the subtitle when the disc has none", func() {
name, err := artworkItemName(context.Background(), ds, model.KindDiscArtwork, "al-1:1")
Expect(err).ToNot(HaveOccurred())
Expect(name).To(Equal("Sandinista! (disc 1)"))
})
It("rejects an id that is not <albumID>:<disc>", func() {
_, err := artworkItemName(context.Background(), ds, model.KindDiscArtwork, "al-1")
Expect(err).To(HaveOccurred())
})
})
var _ = Describe("artwork refresh command", func() {
It("requires at least one argument", func() {
Expect(artworkRefreshCmd.Args(artworkRefreshCmd, []string{})).To(HaveOccurred())
@ -468,13 +441,13 @@ var _ = Describe("artwork reprocess selection", func() {
It("returns every kind for --all", func() {
ks, err := selectedKinds(nil, nil, true)
Expect(err).ToNot(HaveOccurred())
Expect(ks).To(ConsistOf(artwork.RecheckKinds))
Expect(ks).To(ConsistOf(artwork.ReprocessKinds))
})
It("returns every kind for a source filter given without a kind", func() {
ks, err := selectedKinds(nil, []string{"folder"}, false)
Expect(err).ToNot(HaveOccurred())
Expect(ks).To(ConsistOf(artwork.RecheckKinds), "--source alone is already a complete selection")
Expect(ks).To(ConsistOf(artwork.ReprocessKinds), "--source alone is already a complete selection")
})
It("returns only the named kinds", func() {
@ -538,6 +511,11 @@ var _ = Describe("repositorySources", func() {
Expect(repositorySources([]string{"absent", "folder"})).To(Equal([]string{"", "folder"}))
})
It("maps the failed name onto the pseudo-source, and back for display", func() {
Expect(repositorySources([]string{failedSource})).To(Equal([]string{model.ArtworkSourceFailed}))
Expect(displaySource(model.ArtworkSourceFailed)).To(Equal(failedSource))
})
It("keeps an empty selection empty, meaning every source", func() {
Expect(repositorySources(nil)).To(BeEmpty())
})
@ -633,6 +611,25 @@ var _ = Describe("reprocessArtwork", func() {
Expect(queue.Count()).To(BeZero())
})
DescribeTable("records the applied config only for a run that leaves nothing on the old one",
func(selected []model.Kind, sources []string, dryRun, applied bool) {
Expect(ds.Property(ctx).Put(consts.ArtConfFingerprintPropertyKey, "stale-fingerprint")).To(Succeed())
Expect(reprocessArtwork(ctx, ds, selected, sources, imageAgents, dryRun, accept, &out)).To(Succeed())
want := "stale-fingerprint"
if applied {
want = artwork.ConfigFingerprint()
}
Expect(ds.Property(ctx).Get(consts.ArtConfFingerprintPropertyKey)).To(Equal(want))
},
Entry("every kind, unfiltered", artwork.ReprocessKinds, nil, false, true),
Entry("every kind, but nothing matched", artwork.ReprocessKinds, []string{}, false, true),
Entry("filtered by source", artwork.ReprocessKinds, []string{"external:deezer"}, false, false),
Entry("a subset of kinds", []model.Kind{model.KindAlbumArtwork}, nil, false, false),
Entry("a dry run applies nothing", artwork.ReprocessKinds, nil, true, false),
)
It("queues the matching items at recheck priority, leaving their artwork state alone", func() {
Expect(reprocessArtwork(ctx, ds, kinds, []string{"external:deezer"}, imageAgents, false, accept, &out)).To(Succeed())
@ -786,6 +783,14 @@ var _ = Describe("reprocessArtwork", func() {
imageAgents, true, accept, &out)).ToNot(Succeed(), "a typo must still be rejected")
})
It("names failed among the valid sources when rejecting a typo", func() {
err := reprocessArtwork(ctx, ds, kinds, []string{"faild"}, imageAgents, true, accept, &out)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("failed"),
"failed is accepted but never stored, so it has to be named explicitly")
})
It("accepts a source another kind uses, letting the empty selection report itself", func() {
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindArtistArtwork}, []string{"folder"},
imageAgents, false, decline, &out)).To(Succeed())
@ -818,14 +823,16 @@ var _ = Describe("collectStatus", func() {
ImageType: model.ImageTypePrimary, Source: source, Hash: hash, AttemptedAt: attempted})).To(Succeed())
}
put(model.KindArtistArtwork, "ar-1", "external:deezer", "h1", time.Now())
put(model.KindArtistArtwork, "ar-2", "", "", time.Now().Add(-artwork.StaleAbsentAge-time.Hour))
put(model.KindArtistArtwork, "ar-3", "", "", time.Now())
put(model.KindArtistArtwork, "ar-2", "", "", time.Now().Add(-24*time.Hour))
// ar-3 is absent because it gave up, so the two absent artists split across the columns.
Expect(art.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "ar-3",
ImageType: model.ImageTypePrimary, LastFailure: "[]", AttemptedAt: time.Now()})).To(Succeed())
put(model.KindAlbumArtwork, "al-1", "folder", "h2", time.Now())
Expect(queue.Enqueue(model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar-9",
ImageType: model.ImageTypePrimary, Priority: model.ArtworkPriorityBackfill})).To(Succeed())
})
It("reports the queue, the source distribution and the absent ages", func() {
It("reports the queue, the source distribution and the absent totals", func() {
rep, err := collectStatus(ctx, ds)
Expect(err).ToNot(HaveOccurred())
@ -836,8 +843,8 @@ var _ = Describe("collectStatus", func() {
sourceCount{kind: model.KindArtistArtwork, source: "", count: 2},
sourceCount{kind: model.KindAlbumArtwork, source: "folder", count: 1},
))
Expect(rep.absent).To(ContainElement(absentCount{kind: model.KindArtistArtwork,
ArtworkAbsentStat: model.ArtworkAbsentStat{Total: 2, Stale: 1}}))
Expect(rep.absent).To(ContainElement(absentCount{kind: model.KindArtistArtwork, noImage: 1, failed: 1}),
"two absent artists, one answered and one that gave up")
})
It("compares the stored fingerprint against the current one", func() {
@ -871,7 +878,7 @@ var _ = Describe("formatStatus", func() {
{kind: model.KindArtistArtwork, source: "", count: 2},
},
absent: []absentCount{
{kind: model.KindArtistArtwork, ArtworkAbsentStat: model.ArtworkAbsentStat{Total: 2, Stale: 1}},
{kind: model.KindArtistArtwork, noImage: 1, failed: 1},
},
inputs: []artwork.FingerprintInput{{Name: "Agents", Value: "deezer,lastfm"}},
stored: "abc123",
@ -904,50 +911,37 @@ var _ = Describe("formatStatus", func() {
Expect(sources).To(MatchRegexp(`artist\s+absent\s+2`))
})
It("prints the absent total and how many are due for recheck", func() {
absent := block(formatStatus(rep), "Absent (resolved, no image found)")
Expect(absent).To(MatchRegexp(`artist\s+2\s+1`))
It("partitions the absent states into answered and gave up", func() {
out := block(formatStatus(rep), "Absent (resolved, no image found)")
Expect(out).To(ContainSubstring("NO IMAGE"))
Expect(out).To(MatchRegexp(`artist\s+1\s+1`), "1 answered plus 1 failed, summing to 2 absent")
Expect(formatStatus(rep)).To(ContainSubstring("artwork reprocess --source failed"))
})
It("states the recheck window and the drip rate the absent counts are bucketed against", func() {
Expect(formatStatus(rep)).To(ContainSubstring("168h"))
Expect(formatStatus(rep)).To(ContainSubstring("100 per kind per hour"))
It("says absent states are never retried on their own, and names both commands that do", func() {
out := formatStatus(rep)
Expect(out).To(ContainSubstring("nothing retries these"))
Expect(out).To(ContainSubstring("artwork reprocess --source absent"))
Expect(out).To(ContainSubstring("artwork reprocess --source failed"))
})
It("leads with the queued backlog, which is the finding, not with the fingerprint verdict", func() {
out := block(formatStatus(rep), "Backfill")
Expect(out).To(MatchRegexp(`State:\s+backfill running: 2 items queued`),
"an operator scanning for trouble must not read 'up to date' while 2 items churn")
Expect(out).To(ContainSubstring("fingerprint up to date"))
})
It("keeps the re-enqueue warning while a backfill is already running", func() {
rep.stored = "older"
out := block(formatStatus(rep), "Backfill")
Expect(out).To(MatchRegexp(`State:\s+backfill running: 2 items queued`))
Expect(out).To(ContainSubstring("re-enqueued"),
"the stored fingerprint is still stale, so a second full re-enqueue is pending on top of this one")
})
It("reports up to date only once the backfill has drained", func() {
rep.queue = []model.ArtworkQueueStat{{ItemKind: "al", Priority: model.ArtworkPriorityScan, Count: 1}}
Expect(block(formatStatus(rep), "Backfill")).To(MatchRegexp(`State:\s+up to date`))
It("reports a matching fingerprint as up to date, whatever else is queued", func() {
Expect(block(formatStatus(rep), "Config")).To(MatchRegexp(`State:\s+up to date`))
})
It("echoes the config inputs a fingerprint change would have come from", func() {
out := block(formatStatus(rep), "Backfill")
out := block(formatStatus(rep), "Config")
Expect(out).To(MatchRegexp(`Agents:\s+deezer,lastfm`))
Expect(out).To(ContainSubstring("abc123"), "the fingerprint values themselves must be printed")
})
It("reports a changed fingerprint as a pending re-resolve of everything", func() {
It("reports a changed fingerprint as stale artwork, and names the command that applies it", func() {
rep.stored = "older"
rep.queue = nil
out := formatStatus(rep)
Expect(out).To(ContainSubstring("fingerprint changed"))
Expect(out).To(ContainSubstring("artwork reprocess --all"))
Expect(out).ToNot(ContainSubstring("up to date"))
})
@ -1041,7 +1035,7 @@ var _ = Describe("needsImageAgents", func() {
It("is false once the chains no longer reach an agent", func() {
conf.Server.CoverArtPriority = "cover.*"
conf.Server.ArtistArtPriority = "artist.*"
Expect(needsImageAgents(artwork.RecheckKinds)).To(BeFalse())
Expect(needsImageAgents(artwork.ReprocessKinds)).To(BeFalse())
})
})

41
cmd/root.go
View file

@ -2,6 +2,7 @@ package cmd
import (
"context"
"net/http"
"os"
"os/signal"
"strings"
@ -138,7 +139,7 @@ func startServer(ctx context.Context) func() error {
a.MountRouter("Prometheus metrics", conf.Server.Prometheus.MetricsPath, p.GetHandler())
}
if conf.Server.DevEnableProfiler {
a.MountRouter("Profiling", "/debug", middleware.Profiler())
a.MountRouter("Profiling", "/debug", profilerHandler())
}
if strings.HasPrefix(conf.Server.UILoginBackgroundURL, "/") {
a.MountRouter("Background images", conf.Server.UILoginBackgroundURL, backgrounds.NewHandler())
@ -147,6 +148,14 @@ func startServer(ctx context.Context) func() error {
}
}
// profilerHandler returns the pprof handler. net/http/pprof resolves the profile
// name from the raw request path, so the BasePath has to come off first.
func profilerHandler() http.Handler {
// A trailing or root slash would make StripPrefix drop the leading slash chi needs.
basePath := strings.TrimRight(conf.Server.BasePath, "/")
return http.StripPrefix(basePath, middleware.Profiler())
}
// schedulePeriodicScan schedules a periodic scan of the music library, if configured.
func schedulePeriodicScan(ctx context.Context) func() error {
return func() error {
@ -357,21 +366,18 @@ func startArtworkWorker(ctx context.Context, worker *artwork.Worker) func() erro
}
}
// scheduleArtworkHousekeeping runs the startup fingerprint backfill and registers the
// recurring stale-absent recheck and prune jobs.
// scheduleArtworkHousekeeping registers the recurring missing-state and prune jobs, and
// reports an artwork config change without acting on it.
func scheduleArtworkHousekeeping(ctx context.Context, worker *artwork.Worker) func() error {
return func() error {
schedulerInstance := scheduler.GetInstance()
if _, err := schedulerInstance.Add(consts.ArtworkStaleAbsentRecheckSchedule, func() {
if err := worker.EnqueueStaleAbsentAll(ctx); err != nil {
log.Error(ctx, "Error enqueueing stale artwork rechecks", err)
}
if _, err := schedulerInstance.Add(consts.ArtworkEnqueueMissingSchedule, func() {
if err := worker.EnqueueMissingAll(ctx); err != nil {
log.Error(ctx, "Error enqueueing missing artwork rechecks", err)
}
}); err != nil {
log.Error(ctx, "Error scheduling artwork stale-absent recheck", err)
log.Error(ctx, "Error scheduling artwork missing-state recheck", err)
}
if _, err := schedulerInstance.Add(consts.ArtworkPruneSchedule, func() {
@ -388,23 +394,8 @@ func scheduleArtworkHousekeeping(ctx context.Context, worker *artwork.Worker) fu
log.Error(ctx, "Error enqueueing missing artwork rechecks", err)
}
backfilled, err := worker.Backfill(ctx)
if err != nil {
log.Error(ctx, "Error running artwork backfill", err)
return nil
}
if !backfilled {
return nil
}
log.Info(ctx, "Artwork backfill enqueued, scheduling a follow-up prune")
timer := time.NewTimer(consts.ArtworkPostBackfillPruneDelay)
defer timer.Stop()
select {
case <-timer.C:
if err := worker.RunPrune(ctx); err != nil {
log.Error(ctx, "Error running post-backfill artwork prune", err)
}
case <-ctx.Done():
if err := worker.ReconcileConfig(ctx); err != nil {
log.Error(ctx, "Error checking the artwork config fingerprint", err)
}
return nil
}

46
cmd/root_test.go Normal file
View file

@ -0,0 +1,46 @@
package cmd
import (
"net/http"
"net/http/httptest"
"path"
"runtime/pprof"
"github.com/go-chi/chi/v5"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = pprof.NewProfile("nd-profiler-test")
var _ = Describe("profilerHandler", func() {
// Mirrors how server.MountRouter mounts the handler.
mount := func() http.Handler {
router := chi.NewRouter()
router.Mount(path.Join(conf.Server.BasePath, "/debug"), profilerHandler())
return router
}
BeforeEach(func() {
DeferCleanup(configtest.SetupConfig())
})
DescribeTable("serves a named profile",
func(basePath string) {
conf.Server.BasePath = basePath
w := httptest.NewRecorder()
target := path.Join(basePath, "/debug/pprof/nd-profiler-test") + "?debug=1"
mount().ServeHTTP(w, httptest.NewRequest(http.MethodGet, target, nil))
Expect(w.Code).To(Equal(http.StatusOK))
Expect(w.Body.String()).To(HavePrefix("nd-profiler-test profile: total 0"))
},
Entry("without a BasePath", ""),
Entry("with a BasePath", "/music"),
Entry("with a root BasePath", "/"),
Entry("with a trailing-slash BasePath", "/music/"),
)
})

9
cmd/wire_gen.go
View file

@ -76,7 +76,10 @@ func CreateNativeAPIRouter(ctx context.Context) *nativeapi.Router {
library := core.NewLibrary(dataStore, modelScanner, watcher, broker, manager)
user := core.NewUser(dataStore, manager)
maintenance := core.NewMaintenance(dataStore)
router := nativeapi.New(dataStore, share, playlistsPlaylists, insights, library, user, maintenance, manager, uploader)
agentsAgents := agents.GetAgents(dataStore, manager)
matcherMatcher := matcher.New(dataStore)
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher, broker)
router := nativeapi.New(dataStore, share, playlistsPlaylists, insights, library, user, maintenance, manager, uploader, provider)
return router
}
@ -97,7 +100,7 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router {
manager := plugins.GetManager(dataStore, broker, metricsMetrics)
agentsAgents := agents.GetAgents(dataStore, manager)
matcherMatcher := matcher.New(dataStore)
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher)
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher, broker)
uploader := artwork.NewUploader(dataStore)
playlistsPlaylists := playlists.NewPlaylists(dataStore, uploader)
modelScanner := scanner.New(ctx, dataStore, broker, playlistsPlaylists, metricsMetrics)
@ -129,7 +132,7 @@ func CreateJellyfinAPIRouter(ctx context.Context) *jellyfin.Router {
playlistsPlaylists := playlists.NewPlaylists(dataStore, uploader)
agentsAgents := agents.GetAgents(dataStore, manager)
matcherMatcher := matcher.New(dataStore)
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher)
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher, broker)
sonicSonic := sonic.New(dataStore, manager, matcherMatcher)
lyricsLyrics := lyrics.NewLyrics(dataStore, manager)
router := jellyfin.New(dataStore, artworkArtwork, mediaStreamer, transcodeDecider, players, playTracker, playlistsPlaylists, provider, sonicSonic, lyricsLyrics, broker)

35
conf/configuration.go
View file

@ -73,6 +73,7 @@ type configOptions struct {
Matcher matcherOptions `json:",omitzero"`
RecentlyAddedByModTime bool
PreferSortTags bool
EnableNaturalSorting bool
IgnoredArticles string
IndexGroups string
FFmpegPath string
@ -314,6 +315,12 @@ var currentGOOS = func() string {
return runtime.GOOS
}
// TLSEnabled reports whether the server serves HTTPS. Both halves are required,
// so callers cannot infer it from the certificate alone.
func (c *configOptions) TLSEnabled() bool {
return c.TLSCert != "" && c.TLSKey != ""
}
var (
Server = &configOptions{}
hooks []func()
@ -344,6 +351,13 @@ func LoadFromFile(confFile string) {
Load(true)
}
func durationNonNegativeOrDefault(val *time.Duration, original time.Duration) {
if val.Nanoseconds() < 0 {
log.Warn("Duration is a negative value. Using default value", "value", *val, "default", original)
*val = original
}
}
func Load(noConfigDump bool) {
parseIniFileConfiguration()
remapEnvVarKeysFromConfig()
@ -411,6 +425,20 @@ func Load(noConfigDump bool) {
log.SetLogSourceLine(Server.DevLogSourceLine)
log.SetRedacting(Server.EnableLogRedacting)
durationNonNegativeOrDefault(&Server.SessionTimeout, consts.DefaultSessionTimeout)
durationNonNegativeOrDefault(&Server.SmartPlaylistRefreshDelay, consts.DefaultSmartRefresh)
durationNonNegativeOrDefault(&Server.DefaultShareExpiration, consts.DefaultShareExpiration)
durationNonNegativeOrDefault(&Server.UIPlaybackReportInterval, consts.DefaultUIPlaybackReportInterval)
durationNonNegativeOrDefault(&Server.AuthWindowLength, consts.DefaultAuthWindowLength)
durationNonNegativeOrDefault(&Server.Scanner.WatcherWait, consts.DefaultWatcherWait)
durationNonNegativeOrDefault(&Server.DevActivityPanelUpdateRate, consts.DefaultActivityPanelUpdateRate)
durationNonNegativeOrDefault(&Server.DevArtworkThrottleBacklogTimeout, consts.RequestThrottleBacklogTimeout)
durationNonNegativeOrDefault(&Server.DevArtistInfoTimeToLive, consts.ArtistInfoTimeToLive)
durationNonNegativeOrDefault(&Server.DevAlbumInfoTimeToLive, consts.AlbumInfoTimeToLive)
durationNonNegativeOrDefault(&Server.DevInsightsInitialDelay, consts.InsightsInitialDelay)
durationNonNegativeOrDefault(&Server.DevPluginCompilationTimeout, consts.DefaultPluginCompilationTimeout)
// Log deprecated, removed and unknown options
for _, o := range deprecatedOptions {
logDeprecatedOptions(o.name, o.replacement)
@ -960,7 +988,7 @@ func setViperDefaults() {
viper.SetDefault("autoimportplaylists", true)
viper.SetDefault("defaultplaylistpublicvisibility", false)
viper.SetDefault("playlistspath", "")
viper.SetDefault("smartPlaylistRefreshDelay", 5*time.Second)
viper.SetDefault("smartPlaylistRefreshDelay", consts.DefaultSmartRefresh)
viper.SetDefault("enabledownloads", true)
viper.SetDefault("enableexternalservices", true)
viper.SetDefault("enablem3uexternalalbumart", false)
@ -973,6 +1001,7 @@ func setViperDefaults() {
viper.SetDefault("matcher.fuzzythreshold", 85)
viper.SetDefault("recentlyaddedbymodtime", false)
viper.SetDefault("prefersorttags", false)
viper.SetDefault("enablenaturalsorting", false)
viper.SetDefault("ignoredarticles", "The El La Los Las Le Les Os As O A")
viper.SetDefault("indexgroups", "A B C D E F G H I J K L M N O P Q R S T U V W X-Z(XYZ) [Unknown]([)")
viper.SetDefault("ffmpegpath", "")
@ -1003,14 +1032,14 @@ func setViperDefaults() {
viper.SetDefault("maximagesize", consts.DefaultMaxImageSize)
viper.SetDefault("enablesharing", true)
viper.SetDefault("shareurl", "")
viper.SetDefault("defaultshareexpiration", 8760*time.Hour)
viper.SetDefault("defaultshareexpiration", consts.DefaultShareExpiration)
viper.SetDefault("defaultdownloadableshare", false)
viper.SetDefault("gatrackingid", "")
viper.SetDefault("enableinsightscollector", true)
viper.SetDefault("enablescheduleddbanalyze", true)
viper.SetDefault("enablelogredacting", true)
viper.SetDefault("authrequestlimit", 5)
viper.SetDefault("authwindowlength", 20*time.Second)
viper.SetDefault("authwindowlength", consts.DefaultAuthWindowLength)
viper.SetDefault("passwordencryptionkey", "")
viper.SetDefault("extauth.userheader", "Remote-User")
viper.SetDefault("extauth.trustedsources", "")

72
conf/configuration_test.go
View file

@ -6,8 +6,11 @@ import (
"os"
"path/filepath"
"testing"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/log"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@ -452,4 +455,73 @@ var _ = Describe("Configuration", func() {
Entry("INI format", "ini"),
Entry("JSON format", "json"),
)
It("should use default values for negative duration fields", func() {
filename := filepath.Join("testdata", "invalid_duration.toml")
conf.InitConfig(filename, false)
conf.Load(true)
server := conf.Server
Expect(server.SessionTimeout).To(Equal(consts.DefaultSessionTimeout))
Expect(server.SmartPlaylistRefreshDelay).To(Equal(consts.DefaultSmartRefresh))
Expect(server.DefaultShareExpiration).To(Equal(consts.DefaultShareExpiration))
Expect(server.UIPlaybackReportInterval).To(Equal(consts.DefaultUIPlaybackReportInterval))
Expect(server.AuthWindowLength).To(Equal(consts.DefaultAuthWindowLength))
Expect(server.Scanner.WatcherWait).To(Equal(consts.DefaultWatcherWait))
Expect(server.DevActivityPanelUpdateRate).To(Equal(consts.DefaultActivityPanelUpdateRate))
Expect(server.DevArtworkThrottleBacklogTimeout).To(Equal(consts.RequestThrottleBacklogTimeout))
Expect(server.DevArtistInfoTimeToLive).To(Equal(consts.ArtistInfoTimeToLive))
Expect(server.DevAlbumInfoTimeToLive).To(Equal(consts.AlbumInfoTimeToLive))
Expect(server.DevInsightsInitialDelay).To(Equal(consts.InsightsInitialDelay))
Expect(server.DevPluginCompilationTimeout).To(Equal(consts.DefaultPluginCompilationTimeout))
})
It("should use parsed values for duration fields", func() {
conf.InitConfig(filepath.Join("testdata", "valid_duration.toml"), false)
conf.Load(true)
configured := 1 * time.Second
server := conf.Server
Expect(server.SessionTimeout).To(Equal(configured))
Expect(server.SmartPlaylistRefreshDelay).To(Equal(configured))
Expect(server.DefaultShareExpiration).To(Equal(configured))
Expect(server.UIPlaybackReportInterval).To(Equal(configured))
Expect(server.AuthWindowLength).To(Equal(configured))
Expect(server.Scanner.WatcherWait).To(Equal(configured))
Expect(server.DevActivityPanelUpdateRate).To(Equal(configured))
Expect(server.DevArtworkThrottleBacklogTimeout).To(Equal(configured))
Expect(server.DevArtistInfoTimeToLive).To(Equal(configured))
Expect(server.DevAlbumInfoTimeToLive).To(Equal(configured))
Expect(server.DevInsightsInitialDelay).To(Equal(configured))
Expect(server.DevPluginCompilationTimeout).To(Equal(configured))
})
})
var _ = Describe("TLSEnabled", func() {
BeforeEach(func() {
DeferCleanup(configtest.SetupConfig())
})
It("is false when neither the certificate nor the key is set", func() {
Expect(conf.Server.TLSEnabled()).To(BeFalse())
})
It("is true when both the certificate and the key are set", func() {
conf.Server.TLSCert = "cert.pem"
conf.Server.TLSKey = "key.pem"
Expect(conf.Server.TLSEnabled()).To(BeTrue())
})
It("is false when only the certificate is set", func() {
conf.Server.TLSCert = "cert.pem"
Expect(conf.Server.TLSEnabled()).To(BeFalse())
})
It("is false when only the key is set", func() {
conf.Server.TLSKey = "key.pem"
Expect(conf.Server.TLSEnabled()).To(BeFalse())
})
})

12
conf/testdata/invalid_duration.toml vendored Normal file
View file

@ -0,0 +1,12 @@
SessionTimeout = "-10s"
SmartPlaylistRefreshDelay = "-10s"
UIPlaybackReportInterval = "-10s"
AuthWindowLength = "-10s"
DefaultShareExpiration = "-10s"
Scanner.WatcherWait = "-10s"
DevActivityPanelUpdateRate = "-10s"
DevArtworkThrottleBacklogTimeout = "-10s"
DevArtistInfoTimeToLive = "-10s"
DevAlbumInfoTimeToLive = "-10s"
DevInsightsInitialDelay = "-10s"
DevPluginCompilationTimeout = "-10s"

12
conf/testdata/valid_duration.toml vendored Normal file
View file

@ -0,0 +1,12 @@
SessionTimeout = "1s"
SmartPlaylistRefreshDelay = "1s"
UIPlaybackReportInterval = "1s"
AuthWindowLength = "1s"
DefaultShareExpiration = "1s"
Scanner.WatcherWait = "1s"
DevActivityPanelUpdateRate = "1s"
DevArtworkThrottleBacklogTimeout = "1s"
DevArtistInfoTimeToLive = "1s"
DevAlbumInfoTimeToLive = "1s"
DevInsightsInitialDelay = "1s"
DevPluginCompilationTimeout = "1s"

17
consts/consts.go
View file

@ -24,8 +24,8 @@ const (
LastDBAnalyzeAttemptAtKey = "LastDBAnalyzeAttemptAt"
DBAnalyzePendingKey = "DBAnalyzePending"
DBAnalyzeFailureCountKey = "DBAnalyzeFailureCount"
// ArtConfFingerprintPropertyKey is the model.PropertyRepository key Backfill compares against
// to detect artwork-affecting config changes across restarts.
// ArtConfFingerprintPropertyKey is the model.PropertyRepository key the artwork config check
// compares against to detect artwork-affecting config changes across restarts.
ArtConfFingerprintPropertyKey = "ArtConfFingerprint"
UIAuthorizationHeader = "X-ND-Authorization"
@ -34,14 +34,15 @@ const (
JWTPublicSecretKey = "JWTPublicSecret"
JWTIssuer = "ND"
DefaultSessionTimeout = 48 * time.Hour
DefaultSmartRefresh = 5 * time.Second
DefaultShareExpiration = 8760 * time.Hour
CookieExpiry = 365 * 24 * 3600 // One year
DBAnalyzeCheckSchedule = "@every 30m"
DBAnalyzeMaxAge = 24 * time.Hour
ArtworkStaleAbsentRecheckSchedule = "@every 1h"
ArtworkPruneSchedule = "@daily"
ArtworkPostBackfillPruneDelay = 10 * time.Minute
ArtworkEnqueueMissingSchedule = "@every 1h"
ArtworkPruneSchedule = "@daily"
// DefaultEncryptionKey This is the encryption key used if none is specified in the `PasswordEncryptionKey` option
// Never ever change this! Or it will break all Navidrome installations that don't set the config option
@ -72,6 +73,7 @@ const (
DefaultUILoginBackgroundURLOffline = "data:image/png;base64," + DefaultUILoginBackgroundOffline
DefaultMaxSidebarPlaylists = 100
DefaultAuthWindowLength = 20 * time.Second
RequestThrottleBacklogLimit = 100
RequestThrottleBacklogTimeout = time.Minute
@ -107,6 +109,9 @@ const (
DefaultScannerExtractor = "taglib"
DefaultWatcherWait = 5 * time.Second
Zwsp = string('\u200b')
DefaultActivityPanelUpdateRate = 300 * time.Millisecond
DefaultPluginCompilationTimeout = time.Minute
)
const (
@ -201,7 +206,7 @@ var (
}
)
var HTTPUserAgent = "Navidrome" + "/" + Version
var HTTPUserAgent = "Navidrome/" + Version + " - https://github.com/navidrome"
var (
VariousArtists = "Various Artists"

154
core/agents/agents.go
View file

@ -1,9 +1,13 @@
package agents
import (
"cmp"
"context"
"errors"
"maps"
"slices"
"strings"
"sync"
"time"
"github.com/navidrome/navidrome/conf"
@ -22,11 +26,43 @@ type PluginLoader interface {
LoadMediaAgent(name string) (Interface, bool)
}
// agentCooldown is the default cooldown duration for an agent that returns a RetryLaterError without a specific
// RetryIn duration.
const agentCooldown = time.Minute
// errUnsupported marks an agent that does not implement the requested method: it never ran,
// so it neither answered nor throttled.
var errUnsupported = errors.New("agent does not support this method")
// Agents is a meta-agent that aggregates multiple built-in and plugin agents. It tries each enabled agent in order
// until one returns valid data.
type Agents struct {
ds model.DataStore
pluginLoader PluginLoader
cooldowns cooldowns
}
// cooldowns remembers, across dispatches, which agents asked to be left alone and until when.
type cooldowns struct {
mu sync.RWMutex
until map[string]time.Time
}
func (c *cooldowns) active(name string) bool {
c.mu.RLock()
defer c.mu.RUnlock()
return time.Now().Before(c.until[name])
}
// park keeps whichever deadline is later, so a call still in flight when a longer cooldown
// starts cannot cut it short when it finally answers.
func (c *cooldowns) park(name string, d time.Duration) {
until := time.Now().Add(d)
c.mu.Lock()
defer c.mu.Unlock()
if until.After(c.until[name]) {
c.until[name] = until
}
}
// GetAgents returns the singleton instance of Agents
@ -41,6 +77,7 @@ func createAgents(ds model.DataStore, pluginLoader PluginLoader) *Agents {
return &Agents{
ds: ds,
pluginLoader: pluginLoader,
cooldowns: cooldowns{until: map[string]time.Time{}},
}
}
@ -90,12 +127,19 @@ func (a *Agents) getEnabledAgentNames() []enabledAgent {
} else if isPlugin {
validAgents = append(validAgents, enabledAgent{name: name, isPlugin: true})
} else {
log.Debug("Unknown agent ignored", "name", name)
log.Debug("Unknown agent ignored", "name", name, "available", availableAgentNames(availablePlugins))
}
}
return validAgents
}
// availableAgentNames returns every name accepted by the Agents config option.
func availableAgentNames(plugins []string) []string {
names := append(slices.Collect(maps.Keys(Map)), plugins...)
slices.Sort(names)
return names
}
func (a *Agents) getAgent(ea enabledAgent) Interface {
if ea.isPlugin {
// Try to load WASM plugin agent (if plugin loader is available)
@ -171,7 +215,7 @@ func (a *Agents) GetArtistMBID(ctx context.Context, id string, name string) (str
return callAgentMethod(ctx, a, "GetArtistMBID", func(ag Interface) (string, error) {
retriever, ok := ag.(ArtistMBIDRetriever)
if !ok {
return "", ErrNotFound
return "", errUnsupported
}
return retriever.GetArtistMBID(ctx, id, name)
})
@ -188,7 +232,7 @@ func (a *Agents) GetArtistURL(ctx context.Context, id, name, mbid string) (strin
return callAgentMethod(ctx, a, "GetArtistURL", func(ag Interface) (string, error) {
retriever, ok := ag.(ArtistURLRetriever)
if !ok {
return "", ErrNotFound
return "", errUnsupported
}
return retriever.GetArtistURL(ctx, id, name, mbid)
})
@ -205,7 +249,7 @@ func (a *Agents) GetArtistBiography(ctx context.Context, id, name, mbid string)
return callAgentMethod(ctx, a, "GetArtistBiography", func(ag Interface) (string, error) {
retriever, ok := ag.(ArtistBiographyRetriever)
if !ok {
return "", ErrNotFound
return "", errUnsupported
}
return retriever.GetArtistBiography(ctx, id, name, mbid)
})
@ -224,7 +268,11 @@ func (a *Agents) GetSimilarArtists(ctx context.Context, id, name, mbid string, l
overLimit := int(float64(limit) * conf.Server.DevExternalArtistFetchMultiplier)
start := time.Now()
attempts := newAttempts(&a.cooldowns)
for _, enabledAgent := range a.getEnabledAgentNames() {
if attempts.skip(enabledAgent.name) {
continue
}
ag := a.getAgent(enabledAgent)
if ag == nil {
continue
@ -237,6 +285,7 @@ func (a *Agents) GetSimilarArtists(ctx context.Context, id, name, mbid string, l
continue
}
similar, err := retriever.GetSimilarArtists(ctx, id, name, mbid, overLimit)
attempts.record(enabledAgent.name, err)
if len(similar) > 0 && err == nil {
if log.IsGreaterOrEqualTo(log.LevelTrace) {
log.Debug(ctx, "Got Similar Artists", "agent", ag.AgentName(), "artist", name, "similar", similar, "elapsed", time.Since(start))
@ -246,7 +295,7 @@ func (a *Agents) GetSimilarArtists(ctx context.Context, id, name, mbid string, l
return similar, err
}
}
return nil, ErrNotFound
return nil, attempts.noResultErr()
}
func (a *Agents) GetArtistImages(ctx context.Context, id, name, mbid string) ([]ExternalImage, error) {
@ -260,7 +309,7 @@ func (a *Agents) GetArtistImages(ctx context.Context, id, name, mbid string) ([]
return callAgentSliceMethod(ctx, a, "GetArtistImages", func(ag Interface) ([]ExternalImage, error) {
retriever, ok := ag.(ArtistImageRetriever)
if !ok {
return nil, ErrNotFound
return nil, errUnsupported
}
return retriever.GetArtistImages(ctx, id, name, mbid)
})
@ -281,7 +330,7 @@ func (a *Agents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid str
return callAgentSliceMethod(ctx, a, "GetArtistTopSongs", func(ag Interface) ([]Song, error) {
retriever, ok := ag.(ArtistTopSongsRetriever)
if !ok {
return nil, ErrNotFound
return nil, errUnsupported
}
return retriever.GetArtistTopSongs(ctx, id, artistName, mbid, overLimit)
})
@ -295,7 +344,7 @@ func (a *Agents) GetAlbumInfo(ctx context.Context, name, artist, mbid string) (*
return callAgentMethod(ctx, a, "GetAlbumInfo", func(ag Interface) (*AlbumInfo, error) {
retriever, ok := ag.(AlbumInfoRetriever)
if !ok {
return nil, ErrNotFound
return nil, errUnsupported
}
return retriever.GetAlbumInfo(ctx, name, artist, mbid)
})
@ -309,7 +358,7 @@ func (a *Agents) GetAlbumImages(ctx context.Context, name, artist, mbid string)
return callAgentSliceMethod(ctx, a, "GetAlbumImages", func(ag Interface) ([]ExternalImage, error) {
retriever, ok := ag.(AlbumImageRetriever)
if !ok {
return nil, ErrNotFound
return nil, errUnsupported
}
return retriever.GetAlbumImages(ctx, name, artist, mbid)
})
@ -320,7 +369,7 @@ func (a *Agents) GetSimilarSongsByTrack(ctx context.Context, id, name, artist, m
return callAgentSliceMethod(ctx, a, "GetSimilarSongsByTrack", func(ag Interface) ([]Song, error) {
retriever, ok := ag.(SimilarSongsByTrackRetriever)
if !ok {
return nil, ErrNotFound
return nil, errUnsupported
}
return retriever.GetSimilarSongsByTrack(ctx, id, name, artist, mbid, count)
})
@ -331,7 +380,7 @@ func (a *Agents) GetSimilarSongsByAlbum(ctx context.Context, id, name, artist, m
return callAgentSliceMethod(ctx, a, "GetSimilarSongsByAlbum", func(ag Interface) ([]Song, error) {
retriever, ok := ag.(SimilarSongsByAlbumRetriever)
if !ok {
return nil, ErrNotFound
return nil, errUnsupported
}
return retriever.GetSimilarSongsByAlbum(ctx, id, name, artist, mbid, count)
})
@ -349,16 +398,61 @@ func (a *Agents) GetSimilarSongsByArtist(ctx context.Context, id, name, mbid str
return callAgentSliceMethod(ctx, a, "GetSimilarSongsByArtist", func(ag Interface) ([]Song, error) {
retriever, ok := ag.(SimilarSongsByArtistRetriever)
if !ok {
return nil, ErrNotFound
return nil, errUnsupported
}
return retriever.GetSimilarSongsByArtist(ctx, id, name, mbid, count)
})
}
func callAgentMethod[T comparable](ctx context.Context, agents *Agents, methodName string, fn func(Interface) (T, error)) (T, error) {
// agentAttempts tallies what the enabled agents did in one dispatch.
type agentAttempts struct {
cooldowns *cooldowns
throttled bool
answered bool
}
func newAttempts(c *cooldowns) agentAttempts {
return agentAttempts{cooldowns: c}
}
// skip reports whether name is still cooling down, counting it as throttled for this dispatch.
func (t *agentAttempts) skip(name string) bool {
if !t.cooldowns.active(name) {
return false
}
t.throttled = true
return true
}
// record files one agent's outcome, parking it when it asked to be retried later.
func (t *agentAttempts) record(name string, err error) {
switch retry, isRetryLater := errors.AsType[*RetryLaterError](err); {
case errors.Is(err, errUnsupported):
case isRetryLater:
t.cooldowns.park(name, cmp.Or(retry.RetryIn, agentCooldown))
t.throttled = true
default:
t.answered = true
}
}
// noResultErr tells a retryable empty dispatch (nobody answered) from a definitive miss.
func (t *agentAttempts) noResultErr() error {
if t.throttled && !t.answered {
return ErrRetryLater
}
return ErrNotFound
}
// callAgent tries each enabled agent in order until found reports a usable result.
func callAgent[T any](ctx context.Context, agents *Agents, methodName string, fn func(Interface) (T, error), found func(T) bool) (T, error) {
var zero T
start := time.Now()
attempts := newAttempts(&agents.cooldowns)
for _, enabledAgent := range agents.getEnabledAgentNames() {
if attempts.skip(enabledAgent.name) {
continue
}
ag := agents.getAgent(enabledAgent)
if ag == nil {
continue
@ -367,41 +461,29 @@ func callAgentMethod[T comparable](ctx context.Context, agents *Agents, methodNa
break
}
result, err := fn(ag)
attempts.record(enabledAgent.name, err)
if err != nil {
log.Trace(ctx, "Agent method call error", "method", methodName, "agent", ag.AgentName(), "error", err)
continue
}
if result != zero {
if found(result) {
log.Debug(ctx, "Got result", "method", methodName, "agent", ag.AgentName(), "elapsed", time.Since(start))
return result, nil
}
}
return zero, ErrNotFound
return zero, attempts.noResultErr()
}
func callAgentMethod[T comparable](ctx context.Context, agents *Agents, methodName string, fn func(Interface) (T, error)) (T, error) {
return callAgent(ctx, agents, methodName, fn, func(result T) bool {
var zero T
return result != zero
})
}
func callAgentSliceMethod[T any](ctx context.Context, agents *Agents, methodName string, fn func(Interface) ([]T, error)) ([]T, error) {
start := time.Now()
for _, enabledAgent := range agents.getEnabledAgentNames() {
ag := agents.getAgent(enabledAgent)
if ag == nil {
continue
}
if utils.IsCtxDone(ctx) {
break
}
results, err := fn(ag)
if err != nil {
log.Trace(ctx, "Agent method call error", "method", methodName, "agent", ag.AgentName(), "error", err)
continue
}
if len(results) > 0 {
log.Debug(ctx, "Got results", "method", methodName, "agent", ag.AgentName(), "count", len(results), "elapsed", time.Since(start))
return results, nil
}
}
return nil, ErrNotFound
return callAgent(ctx, agents, methodName, fn, func(results []T) bool { return len(results) > 0 })
}
var _ Interface = (*Agents)(nil)

143
core/agents/agents_test.go
View file

@ -3,6 +3,8 @@ package agents
import (
"context"
"errors"
"slices"
"time"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
@ -14,6 +16,29 @@ import (
. "github.com/onsi/gomega"
)
var _ = Describe("cooldowns", func() {
// Calls to one agent overlap, so a short cooldown can land after a long one started.
It("keeps the longer deadline when a shorter park lands after it", func() {
c := cooldowns{until: map[string]time.Time{}}
c.park("fake", time.Hour)
c.park("fake", time.Millisecond)
time.Sleep(10 * time.Millisecond)
Expect(c.active("fake")).To(BeTrue())
})
It("extends the deadline when the later park is longer", func() {
c := cooldowns{until: map[string]time.Time{}}
c.park("fake", time.Millisecond)
c.park("fake", time.Hour)
time.Sleep(10 * time.Millisecond)
Expect(c.active("fake")).To(BeTrue())
})
})
var _ = Describe("Agents", func() {
var ctx context.Context
var cancel context.CancelFunc
@ -67,6 +92,22 @@ var _ = Describe("Agents", func() {
Expect(ags).ToNot(ContainElement("disabled"))
})
Describe("availableAgentNames", func() {
It("combines built-in agents with the given plugins", func() {
names := availableAgentNames([]string{"apple-music"})
Expect(names).To(ContainElements("apple-music", LocalAgentName, "fake", "empty"))
})
It("returns the names sorted", func() {
names := availableAgentNames([]string{"zz-plugin", "aa-plugin"})
Expect(slices.IsSorted(names)).To(BeTrue())
})
It("works when there are no plugins", func() {
Expect(availableAgentNames(nil)).To(ContainElement(LocalAgentName))
})
})
Describe("GetArtistMBID", func() {
It("returns on first match", func() {
Expect(ag.GetArtistMBID(ctx, "123", "test")).To(Equal("mbid"))
@ -160,6 +201,102 @@ var _ = Describe("Agents", func() {
})
})
Describe("cooldown", func() {
It("skips an agent that returned RetryLaterError until the deadline", func() {
mock.Err = &RetryLaterError{RetryIn: time.Hour}
_, err := ag.GetArtistBiography(ctx, "id", "name", "mbid")
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
// Immediately after: agent is skipped, not called
mock.Err = nil
calls := mock.Calls
_, err = ag.GetArtistBiography(ctx, "id", "name", "mbid")
Expect(mock.Calls).To(Equal(calls))
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
})
// Providers that throttle without saying for how long (Last.fm sends no delay at all)
// must still be parked, or the aggregate keeps calling them on every request.
It("parks an agent that asked to be retried without a delay", func() {
mock.Err = ErrRetryLater
_, err := ag.GetArtistBiography(ctx, "id", "name", "mbid")
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
mock.Err = nil
calls := mock.Calls
_, err = ag.GetArtistBiography(ctx, "id", "name", "mbid")
Expect(mock.Calls).To(Equal(calls), "the default cooldown must outlast the request")
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
})
It("calls the agent again once the cooldown expires", func() {
mock.Err = &RetryLaterError{RetryIn: 10 * time.Millisecond}
_, err := ag.GetArtistBiography(ctx, "id", "name", "mbid")
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
mock.Err = nil
Eventually(func() (string, error) {
return ag.GetArtistBiography(ctx, "id", "name", "mbid")
}, 5*time.Second, 10*time.Millisecond).Should(Equal("bio"))
})
It("returns ErrNotFound, not ErrRetryLater, when agents failed for other reasons", func() {
mock.Err = errors.New("boom")
_, err := ag.GetArtistBiography(ctx, "id", "name", "mbid")
Expect(errors.Is(err, ErrNotFound)).To(BeTrue())
Expect(errors.Is(err, ErrRetryLater)).To(BeFalse())
})
// ErrRetryLater tells the caller "nobody answered, do not cache this". A definitive
// answer from any other agent is an answer, throttled peer or not.
It("returns ErrNotFound when another agent answered with a definitive miss", func() {
other := &mockAgent{Err: ErrNotFound}
Register("fake2", func(model.DataStore) Interface { return other })
conf.Server.Agents = "fake,fake2"
ag = createAgents(ds, nil)
mock.Err = &RetryLaterError{RetryIn: time.Hour}
_, err := ag.GetArtistBiography(ctx, "id", "name", "mbid")
Expect(errors.Is(err, ErrNotFound)).To(BeTrue())
Expect(errors.Is(err, ErrRetryLater)).To(BeFalse())
// The cooldown was still recorded for the throttled agent
calls := mock.Calls
_, _ = ag.GetArtistBiography(ctx, "id", "name", "mbid")
Expect(mock.Calls).To(Equal(calls))
})
It("returns ErrNotFound when another agent answered with an empty slice", func() {
empty := &testImageAgent{Name: "emptyImages"}
Register("emptyImages", func(model.DataStore) Interface { return empty })
conf.Server.Agents = "fake,emptyImages"
ag = createAgents(ds, nil)
mock.Err = &RetryLaterError{RetryIn: time.Hour}
_, err := ag.GetArtistImages(ctx, "123", "test", "mb123")
Expect(errors.Is(err, ErrNotFound)).To(BeTrue())
Expect(errors.Is(err, ErrRetryLater)).To(BeFalse())
})
It("returns ErrRetryLater from GetSimilarArtists when only cooling agents remain", func() {
mock.Err = &RetryLaterError{RetryIn: time.Hour}
_, err := ag.GetSimilarArtists(ctx, "123", "test", "mb123", 2)
Expect(errors.Is(err, ErrRetryLater)).To(BeTrue())
})
It("returns ErrNotFound from GetSimilarArtists when another agent answered", func() {
other := &mockAgent{Err: ErrNotFound}
Register("fake2", func(model.DataStore) Interface { return other })
conf.Server.Agents = "fake,fake2"
ag = createAgents(ds, nil)
mock.Err = &RetryLaterError{RetryIn: time.Hour}
_, err := ag.GetSimilarArtists(ctx, "123", "test", "mb123", 2)
Expect(errors.Is(err, ErrNotFound)).To(BeTrue())
Expect(errors.Is(err, ErrRetryLater)).To(BeFalse())
})
})
Describe("GetArtistImages", func() {
It("returns on first match", func() {
Expect(ag.GetArtistImages(ctx, "123", "test", "mb123")).To(Equal([]ExternalImage{{
@ -423,8 +560,9 @@ var _ = Describe("Agents", func() {
})
type mockAgent struct {
Args []any
Err error
Args []any
Err error
Calls int
}
func (a *mockAgent) AgentName() string {
@ -449,6 +587,7 @@ func (a *mockAgent) GetArtistURL(_ context.Context, id, name, mbid string) (stri
func (a *mockAgent) GetArtistBiography(_ context.Context, id, name, mbid string) (string, error) {
a.Args = []any{id, name, mbid}
a.Calls++
if a.Err != nil {
return "", a.Err
}

51
core/agents/interfaces.go
View file

@ -3,6 +3,9 @@ package agents
import (
"context"
"errors"
"fmt"
"strconv"
"time"
"github.com/gohugoio/hashstructure"
"github.com/navidrome/navidrome/model"
@ -52,11 +55,49 @@ func (s Song) Equals(other Song) bool {
return h1 == h2
}
var (
// ErrNotFound means the provider answered and had nothing. Return the underlying error
// for a fault instead, or callers that back off on faults will treat it as definitive.
ErrNotFound = errors.New("not found")
)
// ErrNotFound means the provider answered and had nothing. Return the underlying error
// for a fault instead, or callers that back off on faults will treat it as definitive.
var ErrNotFound = errors.New("not found")
// ErrRetryLater is the zero-delay RetryLaterError: the provider is temporarily unavailable
// or throttling us, but did not say for how long. Both errors.Is(err, ErrRetryLater) and
// errors.AsType[*RetryLaterError] match it and every delay-carrying variant.
// Treat it as immutable; build a new RetryLaterError to name a delay.
var ErrRetryLater = &RetryLaterError{}
// RetryLaterError asks callers to back off, optionally for the delay the provider requested.
type RetryLaterError struct {
RetryIn time.Duration
}
func (e *RetryLaterError) Error() string {
if e.RetryIn > 0 {
return fmt.Sprintf("retry later (in %s)", e.RetryIn)
}
return "retry later"
}
func (e *RetryLaterError) Is(target error) bool {
_, ok := target.(*RetryLaterError)
return ok
}
// MaxRetryIn caps a delay parsed from a provider, so a bogus value cannot park it indefinitely.
const MaxRetryIn = time.Hour
const maxRetryInSeconds = int(MaxRetryIn / time.Second)
// ParseRetryIn reads a provider's delay given in seconds, from a header or a plugin token.
// Anything unparseable or non-positive means unspecified.
func ParseRetryIn(seconds string) time.Duration {
// Clamp in seconds: scaling first would wrap a huge value past int64 nanoseconds,
// turning "wait an age" into a fraction of a second. Parse at a fixed width so the
// cap holds on the 32-bit targets we ship, where a plain Atoi would overflow first.
secs, err := strconv.ParseInt(seconds, 10, 64)
if err != nil || secs <= 0 {
return 0
}
return time.Duration(min(secs, int64(maxRetryInSeconds))) * time.Second
}
// AlbumInfoRetriever provides album info (no images)
type AlbumInfoRetriever interface {

47
core/agents/interfaces_test.go
View file

@ -1,27 +1,42 @@
package agents
package agents_test
import (
"errors"
"fmt"
"time"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/core/scrobbler"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("Song.Equals", func() {
base := Song{ID: "1", Name: "S", Artists: []Artist{{ID: "x", Name: "A"}}}
It("true for identical songs incl Artists", func() {
Expect(base.Equals(base)).To(BeTrue())
var _ = Describe("RetryLaterError", func() {
It("matches the ErrRetryLater sentinel via errors.Is", func() {
err := &agents.RetryLaterError{RetryIn: 30 * time.Second}
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
})
It("false when Artists differ", func() {
other := base
other.Artists = []Artist{{ID: "y", Name: "B"}}
Expect(base.Equals(other)).To(BeFalse())
It("matches through errors.Join and wrapping", func() {
err := fmt.Errorf("calling LB: %w", errors.Join(errors.New("http 429"), &agents.RetryLaterError{}))
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
})
It("false when a scalar differs", func() {
other := base
other.Name = "T"
Expect(base.Equals(other)).To(BeFalse())
It("exposes the delay through the wrapped error", func() {
err := errors.Join(errors.New("http 429"), &agents.RetryLaterError{RetryIn: 42 * time.Second})
retry, ok := errors.AsType[*agents.RetryLaterError](err)
Expect(ok).To(BeTrue())
Expect(retry.RetryIn).To(Equal(42 * time.Second))
})
It("true when both have empty Artists and equal scalars", func() {
a := Song{ID: "1", Name: "S"}
Expect(a.Equals(a)).To(BeTrue())
It("matches the sentinel too, reporting no delay", func() {
retry, ok := errors.AsType[*agents.RetryLaterError](agents.ErrRetryLater)
Expect(ok).To(BeTrue())
Expect(retry.RetryIn).To(BeZero())
})
It("is the same sentinel as scrobbler.ErrRetryLater", func() {
Expect(errors.Is(scrobbler.ErrRetryLater, agents.ErrRetryLater)).To(BeTrue())
Expect(errors.Is(&agents.RetryLaterError{}, scrobbler.ErrRetryLater)).To(BeTrue())
})
})

27
core/agents/song_test.go Normal file
View file

@ -0,0 +1,27 @@
package agents
import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("Song.Equals", func() {
base := Song{ID: "1", Name: "S", Artists: []Artist{{ID: "x", Name: "A"}}}
It("true for identical songs incl Artists", func() {
Expect(base.Equals(base)).To(BeTrue())
})
It("false when Artists differ", func() {
other := base
other.Artists = []Artist{{ID: "y", Name: "B"}}
Expect(base.Equals(other)).To(BeFalse())
})
It("false when a scalar differs", func() {
other := base
other.Name = "T"
Expect(base.Equals(other)).To(BeFalse())
})
It("true when both have empty Artists and equal scalars", func() {
a := Song{ID: "1", Name: "S"}
Expect(a.Equals(a)).To(BeTrue())
})
})

36
core/artwork/agent_images.go
View file

@ -2,6 +2,7 @@ package artwork
import (
"context"
"errors"
"io"
"net/url"
@ -41,22 +42,36 @@ func bestImageURL(imgs []agents.ExternalImage) *url.URL {
return best
}
// fetchArtistImage tries each enabled artist-image agent in order. extErr is true only when no
// longerRetry keeps whichever external failure asks for the longer wait, so one provider's
// short delay cannot shorten another's.
func longerRetry(a, b error) error {
if a == nil {
return b
}
var ra, rb *agents.RetryLaterError
if errors.As(b, &rb) && (!errors.As(a, &ra) || rb.RetryIn > ra.RetryIn) {
return b
}
return a
}
// fetchArtistImage tries each enabled artist-image agent in order. The error is non-nil only when no
// agent succeeded and at least one failed transiently.
func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar model.Artist) (r io.ReadCloser, agentName string, extErr bool) {
func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar model.Artist) (io.ReadCloser, string, error) {
// Synthetic artists would otherwise get an unrelated agent result assigned to them.
switch ar.ID {
case consts.UnknownArtistID, consts.VariousArtistsID:
traceFrom(ctx).add(TraceStep{Candidate: externalCandidate, Outcome: OutcomeSkipped, Detail: "synthetic artist"})
return nil, "", false
return nil, "", nil
}
name := externalName(ar.Name)
imageAgents := ag.ArtistImageAgents()
if len(imageAgents) == 0 {
traceFrom(ctx).add(TraceStep{Candidate: externalCandidate, Outcome: OutcomeSkipped,
Detail: "no enabled agent provides artist images"})
return nil, "", false
return nil, "", nil
}
var extErr error
for _, a := range imageAgents {
reader, path, err := gate(a.Name, func() (io.ReadCloser, string, error) {
imgs, err := a.Retriever.GetArtistImages(ctx, ar.ID, name, ar.MbzArtistID)
@ -71,10 +86,10 @@ func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar
})
recordAgent(ctx, a.Name, reader, path, err)
if reader != nil {
return reader, a.Name, false
return reader, a.Name, nil
}
if isTransientExternal(err) {
extErr = true
extErr = longerRetry(extErr, err)
log.Debug(ctx, "Artwork: External artist-image lookup failed", "agent", a.Name, "artist", ar.Name, err)
}
}
@ -82,14 +97,15 @@ func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar
}
// fetchAlbumImage is the album counterpart of fetchArtistImage.
func fetchAlbumImage(ctx context.Context, ag *agents.Agents, gate gateFunc, al model.Album) (r io.ReadCloser, agentName string, extErr bool) {
func fetchAlbumImage(ctx context.Context, ag *agents.Agents, gate gateFunc, al model.Album) (io.ReadCloser, string, error) {
name, artist := externalName(al.Name), externalName(al.AlbumArtist)
imageAgents := ag.AlbumImageAgents()
if len(imageAgents) == 0 {
traceFrom(ctx).add(TraceStep{Candidate: externalCandidate, Outcome: OutcomeSkipped,
Detail: "no enabled agent provides album images"})
return nil, "", false
return nil, "", nil
}
var extErr error
for _, a := range imageAgents {
reader, path, err := gate(a.Name, func() (io.ReadCloser, string, error) {
imgs, err := a.Retriever.GetAlbumImages(ctx, name, artist, al.MbzAlbumID)
@ -104,10 +120,10 @@ func fetchAlbumImage(ctx context.Context, ag *agents.Agents, gate gateFunc, al m
})
recordAgent(ctx, a.Name, reader, path, err)
if reader != nil {
return reader, a.Name, false
return reader, a.Name, nil
}
if isTransientExternal(err) {
extErr = true
extErr = longerRetry(extErr, err)
log.Debug(ctx, "Artwork: External album-image lookup failed", "agent", a.Name, "album", al.Name, err)
}
}

65
core/artwork/agent_images_test.go
View file

@ -2,11 +2,13 @@ package artwork
import (
"context"
"errors"
"io"
"net/http"
"net/http/httptest"
"strings"
"sync"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
@ -153,11 +155,11 @@ var _ = Describe("agent images", func() {
a := &fakeImageAgent{name: "agentA", imgs: []agents.ExternalImage{img("/a", 100)}}
ag := imageAgents(a)
r, name, extErr := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1", Name: "Artist"})
r, name, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1", Name: "Artist"})
Expect(r).ToNot(BeNil())
defer r.Close()
Expect(name).To(Equal("agentA"))
Expect(extErr).To(BeFalse())
Expect(err).ToNot(HaveOccurred())
})
It("skips the external lookup for synthetic artists", func() {
@ -165,10 +167,10 @@ var _ = Describe("agent images", func() {
ag := imageAgents(a)
for _, id := range []string{consts.UnknownArtistID, consts.VariousArtistsID} {
r, name, extErr := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: id, Name: "Various Artists"})
r, name, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: id, Name: "Various Artists"})
Expect(r).To(BeNil())
Expect(name).To(BeEmpty())
Expect(extErr).To(BeFalse())
Expect(err).ToNot(HaveOccurred())
}
Expect(a.artistCalls).To(Equal(0), "synthetic artists never reach the agents")
})
@ -177,9 +179,9 @@ var _ = Describe("agent images", func() {
ag := imageAgents()
t := &ChainTrace{}
r, _, extErr := fetchArtistImage(withTrace(ctx, t), ag, passthroughGate, model.Artist{ID: "ar1"})
r, _, err := fetchArtistImage(withTrace(ctx, t), ag, passthroughGate, model.Artist{ID: "ar1"})
Expect(r).To(BeNil())
Expect(extErr).To(BeFalse())
Expect(err).ToNot(HaveOccurred())
Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "external", Outcome: OutcomeSkipped,
Detail: "no enabled agent provides artist images"}}),
"a configured external token must never be silently absent from the chain")
@ -211,11 +213,11 @@ var _ = Describe("agent images", func() {
b := &fakeImageAgent{name: "agentB", imgs: []agents.ExternalImage{img("/b", 50)}}
ag := imageAgents(a, b)
r, name, extErr := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
r, name, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
Expect(r).ToNot(BeNil())
defer r.Close()
Expect(name).To(Equal("agentB"))
Expect(extErr).To(BeFalse(), "a later hit clears an earlier agent's error")
Expect(err).ToNot(HaveOccurred(), "a later hit clears an earlier agent's error")
Expect(a.artistCalls).To(Equal(1))
Expect(b.artistCalls).To(Equal(1))
})
@ -225,20 +227,43 @@ var _ = Describe("agent images", func() {
b := &fakeImageAgent{name: "agentB", err: agents.ErrNotFound}
ag := imageAgents(a, b)
r, name, extErr := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
r, name, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
Expect(r).To(BeNil())
Expect(name).To(BeEmpty())
Expect(extErr).To(BeFalse(), "not-found is definitive, never a transient failure")
Expect(err).ToNot(HaveOccurred(), "not-found is definitive, never a transient failure")
})
It("reports extErr when one agent fails transiently and the rest find nothing", func() {
It("reports an error when one agent fails transiently and the rest find nothing", func() {
a := &fakeImageAgent{name: "agentA", err: agents.ErrNotFound}
b := &fakeImageAgent{name: "agentB", err: context.DeadlineExceeded}
ag := imageAgents(a, b)
r, _, extErr := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
r, _, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
Expect(r).To(BeNil())
Expect(extErr).To(BeTrue())
Expect(err).To(HaveOccurred())
})
// The worker reschedules on this delay, so it is only honored if the agent loop
// returns it. Two throttled agents: the longest wait is the one that must survive.
It("returns the longest retry delay the providers asked for", func() {
a := &fakeImageAgent{name: "agentA", err: &agents.RetryLaterError{RetryIn: 10 * time.Second}}
b := &fakeImageAgent{name: "agentB", err: &agents.RetryLaterError{RetryIn: 5 * time.Second}}
ag := imageAgents(a, b)
r, _, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
Expect(r).To(BeNil())
retry, ok := errors.AsType[*agents.RetryLaterError](err)
Expect(ok).To(BeTrue())
Expect(retry.RetryIn).To(Equal(10 * time.Second))
})
It("returns no delay when the provider did not ask for one", func() {
ag := imageAgents(&fakeImageAgent{name: "agentA", err: errors.New("boom")})
_, _, err := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1"})
Expect(err).To(HaveOccurred())
_, ok := errors.AsType[*agents.RetryLaterError](err)
Expect(ok).To(BeFalse(), "a plain failure must not look like a throttle")
})
})
@ -247,11 +272,11 @@ var _ = Describe("agent images", func() {
a := &fakeImageAgent{name: "agentA", imgs: []agents.ExternalImage{img("/a", 100)}}
ag := imageAgents(a)
r, name, extErr := fetchAlbumImage(ctx, ag, passthroughGate, model.Album{Name: "Album", AlbumArtist: "Artist"})
r, name, err := fetchAlbumImage(ctx, ag, passthroughGate, model.Album{Name: "Album", AlbumArtist: "Artist"})
Expect(r).ToNot(BeNil())
defer r.Close()
Expect(name).To(Equal("agentA"))
Expect(extErr).To(BeFalse())
Expect(err).ToNot(HaveOccurred())
Expect(a.albumCalls).To(Equal(1))
})
@ -259,21 +284,21 @@ var _ = Describe("agent images", func() {
ag := imageAgents()
t := &ChainTrace{}
r, _, extErr := fetchAlbumImage(withTrace(ctx, t), ag, passthroughGate, model.Album{Name: "Album"})
r, _, err := fetchAlbumImage(withTrace(ctx, t), ag, passthroughGate, model.Album{Name: "Album"})
Expect(r).To(BeNil())
Expect(extErr).To(BeFalse())
Expect(err).ToNot(HaveOccurred())
Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "external", Outcome: OutcomeSkipped,
Detail: "no enabled agent provides album images"}}),
"a configured external token must never be silently absent from the chain")
})
It("reports extErr when the only agent fails transiently", func() {
It("reports an error when the only agent fails transiently", func() {
a := &fakeImageAgent{name: "agentA", err: context.DeadlineExceeded}
ag := imageAgents(a)
r, _, extErr := fetchAlbumImage(ctx, ag, passthroughGate, model.Album{Name: "Album"})
r, _, err := fetchAlbumImage(ctx, ag, passthroughGate, model.Album{Name: "Album"})
Expect(r).To(BeNil())
Expect(extErr).To(BeTrue())
Expect(err).To(HaveOccurred())
})
})

9
core/artwork/artwork.go
View file

@ -118,10 +118,6 @@ func (s *service) Get(ctx context.Context, artID model.ArtworkID, size int, squa
}
}
// requestRecheckAge throttles view-triggered rechecks so reopening a genuinely-absent page can't
// hammer external services; below StaleAbsentAge to catch younger absences.
const requestRecheckAge = time.Hour
func (s *service) serveEntity(ctx context.Context, artID model.ArtworkID, size int, square bool) (*Image, error) {
ia, err := s.ds.Artwork(ctx).GetItemArtwork(artID.Kind, artID.ID, model.ImageTypePrimary)
switch {
@ -130,10 +126,7 @@ func (s *service) serveEntity(ctx context.Context, artID model.ArtworkID, size i
case err != nil:
return nil, err
case ia.Hash == "":
// Inserts an immediately-eligible recheck for a settled absent row.
if time.Since(ia.AttemptedAt) > requestRecheckAge {
s.enqueue(ctx, artID, model.ArtworkPriorityBump)
}
// Settled absent: only an explicit reprocess or refresh retries it.
return nil, ErrUnavailable
default:
return s.serveHash(ctx, artID, ia, size, square)

17
core/artwork/artwork_test.go
View file

@ -204,28 +204,15 @@ var _ = Describe("Artwork", func() {
Expect(err).To(MatchError(ErrUnavailable))
})
It("does not re-enqueue a recently-attempted absent state", func() {
It("never re-enqueues an absent state on view, however old", func() {
Expect(artRepo.PutItemArtwork(&model.ItemArtwork{
ItemKind: "al", ItemID: "al4", AttemptedAt: time.Now(),
ItemKind: "al", ItemID: "al4", AttemptedAt: time.Now().Add(-365 * 24 * time.Hour),
})).To(Succeed())
_, err := svc.Get(ctx, model.MustParseArtworkID("al-al4"), 0, false)
Expect(err).To(MatchError(ErrUnavailable))
Expect(queueRepo.Data).To(BeEmpty())
})
It("promotes a stale absent state at Bump priority on view", func() {
Expect(artRepo.PutItemArtwork(&model.ItemArtwork{
ItemKind: "al", ItemID: "al4b", AttemptedAt: time.Now().Add(-2 * requestRecheckAge),
})).To(Succeed())
_, err := svc.Get(ctx, model.MustParseArtworkID("al-al4b"), 0, false)
Expect(err).To(MatchError(ErrUnavailable))
Expect(queueRepo.Data[primaryKey("al", "al4b")].Priority).To(Equal(model.ArtworkPriorityBump))
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al4b", model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
Expect(ia.Hash).To(BeEmpty())
})
})
Describe("provisional read-through", func() {

19
core/artwork/gate.go
View file

@ -1,6 +1,7 @@
package artwork
import (
"cmp"
"context"
"errors"
"io"
@ -95,6 +96,9 @@ type breaker struct {
// generation identifies the current open episode, so an answer from a call admitted before
// the breaker opened cannot be mistaken for evidence that it has recovered.
generation int
// probeAfter overrides the probe delay for the current episode when a provider named its own
// back-off; zero falls back to breakerProbeAfter.
probeAfter time.Duration
}
func newBreaker() *breaker { return &breaker{} }
@ -107,7 +111,7 @@ func (b *breaker) allow() (bool, int) {
if b.failures < breakerThreshold {
return true, 0
}
if time.Since(b.openedAt) >= breakerProbeAfter {
if time.Since(b.openedAt) >= cmp.Or(b.probeAfter, breakerProbeAfter) {
b.openedAt = time.Now() // start a fresh probe window so only one caller passes
return true, b.generation
}
@ -121,11 +125,24 @@ func (b *breaker) record(name string, gen int, err error) {
}
b.mu.Lock()
defer b.mu.Unlock()
// An explicit back-off is a definitive "stop for this long", so it opens the breaker at once
// with the provider's own delay instead of waiting for the failure threshold.
if retry, ok := errors.AsType[*agents.RetryLaterError](err); ok && retry.RetryIn > 0 {
b.recoveries = 0
b.failures = breakerThreshold
b.openedAt = time.Now()
b.probeAfter = retry.RetryIn
b.generation++
log.Warn("Artwork: Circuit breaker opened for agent, provider asked to back off", "agent", name,
"probeAfter", retry.RetryIn)
return
}
if isTransientExternal(err) {
b.recoveries = 0
b.failures++
if b.failures == breakerThreshold {
b.openedAt = time.Now()
b.probeAfter = 0
b.generation++
log.Warn("Artwork: Circuit breaker opened for agent", "agent", name,
"consecutiveFailures", b.failures, "probeAfter", breakerProbeAfter, err)

13
core/artwork/gate_test.go
View file

@ -2,7 +2,9 @@ package artwork
import (
"errors"
"time"
"github.com/navidrome/navidrome/core/agents"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
@ -40,4 +42,15 @@ var _ = Describe("breaker", func() {
Expect(allowed(b)).To(BeFalse(),
"answers from calls admitted before the breaker opened must not close it")
})
It("opens at once when a provider asks to retry later, honoring its delay", func() {
b := newBreaker()
Expect(allowed(b)).To(BeTrue(), "starts closed")
// A single explicit back-off opens the breaker without reaching the failure threshold.
b.record("agentA", 0, &agents.RetryLaterError{RetryIn: 5 * time.Second})
Expect(allowed(b)).To(BeFalse(), "an explicit back-off opens the breaker immediately")
Expect(b.probeAfter).To(Equal(5*time.Second), "the provider's delay drives the probe interval")
})
})

182
core/artwork/housekeeping.go
View file

@ -6,26 +6,18 @@ import (
"slices"
"strconv"
"strings"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/slice"
"github.com/zeebo/xxh3"
)
// StaleAbsentAge is how long an absent state is trusted before a recheck retries it.
const StaleAbsentAge = 7 * 24 * time.Hour
// StaleAbsentRecheckBatch caps how many absent states each hourly tick re-queues per kind,
// oldest first, so external agents see a flat drip instead of a daily burst.
const StaleAbsentRecheckBatch = 100
// RecheckKinds omits media files: they resolve embedded-only, at scan or on view.
var RecheckKinds = []model.Kind{
// ReprocessKinds omits media files: they resolve embedded-only, at scan or on view. Artists lead
// so bulk enqueues give the most external-dependent kind a queue headstart.
var ReprocessKinds = []model.Kind{
model.KindArtistArtwork, model.KindAlbumArtwork, model.KindPlaylistArtwork, model.KindRadioArtwork,
}
@ -34,14 +26,16 @@ var RecheckKinds = []model.Kind{
func KeepsState(kind model.Kind) bool { return kind != model.KindDiscArtwork }
// RefreshableKinds is every kind Refresh can clear and re-queue, so it holds exactly the kinds
// KeepsState admits. Media files are absent from RecheckKinds but belong here: the worker
// resolves them, it just never revisits them on its own.
var RefreshableKinds = append(slices.Clone(RecheckKinds), model.KindMediaFileArtwork)
// KeepsState admits. Media files are absent from ReprocessKinds but belong here: the worker
// resolves them, it just never enumerates them in bulk.
var RefreshableKinds = append(slices.Clone(ReprocessKinds), model.KindMediaFileArtwork)
// hasRecheckPath reports whether a periodic job will revisit this kind, making an absent settle recoverable.
func hasRecheckPath(prefix string) bool {
// settlesAbsentOnGiveUp reports whether an exhausted retry budget records an absent state. Media
// files are excluded because retrying one costs nothing: they resolve embedded-only, from a local
// read, and only a view ever enqueues them.
func settlesAbsentOnGiveUp(prefix string) bool {
kind, ok := model.ParseKind(prefix)
return ok && slices.Contains(RecheckKinds, kind)
return ok && KeepsState(kind) && kind != model.KindMediaFileArtwork
}
// artworkEpoch invalidates all resolution state when bumped; bump it whenever resolution semantics change.
@ -72,93 +66,36 @@ func ConfigFingerprint() string {
return fmt.Sprintf("%016x", xxh3.Hash([]byte(raw)))
}
// backfillSummary is what a backfill enqueued. MaxExternalLookups is an upper estimate for one
// attempt per item, not a bound: a local hit ends the walk, and a retry asks the agents again.
type backfillSummary struct {
Ran bool
PerKind map[string]int64
Items int64
MaxExternalLookups int64
}
// backfill enqueues artwork resolution for every entity when the config fingerprint changed.
func backfill(ctx context.Context, ds model.DataStore, agentCount func() ImageAgentCount) (backfillSummary, error) {
start := time.Now()
ctx = auth.WithAdminUser(ctx, ds)
// ReconcileConfigFingerprint warns when the artwork config changed since the library was last
// resolved under it. Nothing re-resolves on its own; applying a change is an explicit reprocess.
func ReconcileConfigFingerprint(ctx context.Context, ds model.DataStore) error {
current := ConfigFingerprint()
props := ds.Property(ctx)
stored, err := props.DefaultGet(consts.ArtConfFingerprintPropertyKey, "")
stored, err := ds.Property(ctx).DefaultGet(consts.ArtConfFingerprintPropertyKey, "")
if err != nil {
return backfillSummary{}, err
return err
}
if stored == current {
return backfillSummary{}, nil
}
// Artists first: few entities, most external-dependent, so they get a queue headstart.
kinds := []struct {
kind model.Kind
fetch func() ([]string, error)
}{
{model.KindArtistArtwork, func() ([]string, error) { return ds.Artist(ctx).GetAllIDs() }},
{model.KindAlbumArtwork, func() ([]string, error) { return ds.Album(ctx).GetAllIDs() }},
{model.KindPlaylistArtwork, func() ([]string, error) { return ds.Playlist(ctx).GetAllIDs() }},
{model.KindRadioArtwork, func() ([]string, error) { return ds.Radio(ctx).GetAllIDs() }},
}
// Counted here, not by the caller: building the agent list constructs every enabled agent, and
// an unchanged fingerprint returns above without ever needing the number.
agents := agentCount()
summary := backfillSummary{Ran: true, PerKind: map[string]int64{}}
for _, k := range kinds {
ids, err := k.fetch()
if err != nil {
return backfillSummary{}, err
}
if err := enqueueBackfillKind(ctx, ds, k.kind, ids); err != nil {
return backfillSummary{}, err
}
n := int64(len(ids))
summary.PerKind[k.kind.Prefix()] = n
summary.Items += n
summary.MaxExternalLookups += n * ExternalLookupsPerItem(k.kind, agents)
}
if err := props.Put(consts.ArtConfFingerprintPropertyKey, current); err != nil {
return backfillSummary{}, err
}
log.Info(ctx, "Artwork: Config fingerprint changed, backfill enqueued", "items", summary.Items,
"byKind", summary.PerKind, "maxExternalLookups", summary.MaxExternalLookups,
"elapsed", time.Since(start))
return summary, nil
}
func enqueueBackfillKind(ctx context.Context, ds model.DataStore, kind model.Kind, ids []string) error {
if len(ids) == 0 {
return nil
}
items := slice.Map(ids, func(id string) model.ArtworkQueueItem {
return model.ArtworkQueueItem{
ItemKind: kind.Prefix(), ItemID: id, ImageType: model.ImageTypePrimary, Priority: model.ArtworkPriorityBackfill,
}
})
return ds.ArtworkQueue(ctx).Enqueue(items...)
}
func enqueueStaleAbsentAll(ctx context.Context, ds model.DataStore) error {
cutoff := time.Now().Add(-StaleAbsentAge)
queue := ds.ArtworkQueue(ctx)
for _, kind := range RecheckKinds {
if _, err := queue.EnqueueStaleAbsent(kind, cutoff, StaleAbsentRecheckBatch); err != nil {
return err
}
switch stored {
case current:
case "":
// An unset fingerprint counts as current; the alternative warns every upgrading install once.
return MarkConfigApplied(ctx, ds)
default:
log.Warn(ctx, "Artwork: Config changed since the last full reprocess. Stored artwork keeps "+
"the old resolution; run 'navidrome artwork reprocess --all' to apply the change",
"stored", stored, "current", current, "inputs", FingerprintInputs())
}
return nil
}
// MarkConfigApplied records the current fingerprint as the one the library is resolved under.
func MarkConfigApplied(ctx context.Context, ds model.DataStore) error {
return ds.Property(ctx).Put(consts.ArtConfFingerprintPropertyKey, ConfigFingerprint())
}
// enqueueMissingAll is the safety net for entities a scan never enqueued (added between scans, or scanner off).
func enqueueMissingAll(ctx context.Context, ds model.DataStore) error {
queue := ds.ArtworkQueue(ctx)
for _, kind := range RecheckKinds {
for _, kind := range ReprocessKinds {
if _, err := queue.EnqueueAllMissing(kind, model.ArtworkPriorityRecheck); err != nil {
return err
}
@ -166,6 +103,63 @@ func enqueueMissingAll(ctx context.Context, ds model.DataStore) error {
return nil
}
// ItemName resolves a kind+id to the entity's display name, and errors when the item
// does not exist. Callers use it to reject ids that would otherwise orphan a queue row.
func ItemName(ctx context.Context, ds model.DataStore, kind model.Kind, id string) (string, error) {
switch kind {
case model.KindArtistArtwork:
ar, err := ds.Artist(ctx).Get(id)
if err != nil {
return "", err
}
return ar.Name, nil
case model.KindAlbumArtwork:
al, err := ds.Album(ctx).Get(id)
if err != nil {
return "", err
}
return al.Name, nil
case model.KindPlaylistArtwork:
pls, err := ds.Playlist(ctx).Get(id)
if err != nil {
return "", err
}
return pls.Name, nil
case model.KindRadioArtwork:
rd, err := ds.Radio(ctx).Get(id)
if err != nil {
return "", err
}
return rd.Name, nil
case model.KindMediaFileArtwork:
mf, err := ds.MediaFile(ctx).Get(id)
if err != nil {
return "", err
}
return mf.Title, nil
case model.KindDiscArtwork:
return discArtworkName(ctx, ds, id)
}
return "", fmt.Errorf("unsupported kind %q", kind.Prefix())
}
func discArtworkName(ctx context.Context, ds model.DataStore, id string) (string, error) {
albumID, discNumber, err := model.ParseDiscArtworkID(id)
if err != nil {
return "", err
}
al, err := ds.Album(ctx).Get(albumID)
if err != nil {
return "", err
}
name := fmt.Sprintf("%s (disc %d)", al.Name, discNumber)
// The subtitle is itself a DiscArtPriority candidate, so name it where the chain can be read against it.
if subtitle := strings.TrimSpace(al.Discs[discNumber]); subtitle != "" {
name += ": " + subtitle
}
return name, nil
}
// Refresh drops an item's resolved artwork state and re-queues it at Bump priority.
func Refresh(ctx context.Context, ds model.DataStore, kind model.Kind, id string) error {
if err := ds.Artwork(ctx).DeleteForItems(kind, []string{id}); err != nil {

301
core/artwork/housekeeping_test.go
View file

@ -2,59 +2,17 @@ package artwork
import (
"context"
"fmt"
"slices"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
// visibilityPlaylistDS models playlist_repository's userFilter: a private playlist is only
// visible when the ctx carries an admin, so headless work must wrap ctx with one first.
type visibilityPlaylistDS struct {
*tests.MockDataStore
private model.Playlist
tracks model.PlaylistTrackRepository
}
func (v *visibilityPlaylistDS) Playlist(ctx context.Context) model.PlaylistRepository {
repo := tests.CreateMockPlaylistRepo()
repo.TracksRepo = v.tracks
if u, ok := request.UserFrom(ctx); ok && u.IsAdmin {
repo.SetData(model.Playlists{v.private})
}
return repo
}
func adminUserRepo() *tests.MockedUserRepo {
repo := tests.CreateMockUserRepo()
Expect(repo.Put(&model.User{ID: "admin", UserName: "admin", IsAdmin: true})).To(Succeed())
return repo
}
func noAgents() ImageAgentCount { return ImageAgentCount{} }
// orderTrackingQueueRepo records the item kind of each Enqueue call, so tests can
// assert phase ordering (artists-first) that same-priority timestamps can't guarantee.
type orderTrackingQueueRepo struct {
*tests.MockArtworkQueueRepo
callKinds []string
}
func (o *orderTrackingQueueRepo) Enqueue(items ...model.ArtworkQueueItem) error {
if len(items) > 0 {
o.callKinds = append(o.callKinds, items[0].ItemKind)
}
return o.MockArtworkQueueRepo.Enqueue(items...)
}
var _ = Describe("RefreshableKinds", func() {
// The two are meant to describe the same fact. Nothing but this test stops them from drifting,
// and a drift would have `artwork explain` report state for a kind that keeps none.
@ -72,7 +30,7 @@ var _ = Describe("Housekeeping", func() {
var (
ctx context.Context
ds *tests.MockDataStore
queueRepo *orderTrackingQueueRepo
queueRepo *tests.MockArtworkQueueRepo
propRepo *tests.MockedPropertyRepo
)
@ -84,52 +42,24 @@ var _ = Describe("Housekeeping", func() {
conf.Server.Agents = "spotify"
conf.Server.EnableExternalServices = true
queueRepo = &orderTrackingQueueRepo{MockArtworkQueueRepo: tests.CreateMockArtworkQueueRepo()}
queueRepo = tests.CreateMockArtworkQueueRepo()
propRepo = &tests.MockedPropertyRepo{}
ds = &tests.MockDataStore{MockedArtworkQueue: queueRepo, MockedProperty: propRepo}
})
seedEntities := func() {
artistRepo := tests.CreateMockArtistRepo()
artistRepo.SetData(model.Artists{{ID: "ar1"}, {ID: "ar2"}})
ds.MockedArtist = artistRepo
albumRepo := tests.CreateMockAlbumRepo()
albumRepo.SetData(model.Albums{{ID: "al1"}})
ds.MockedAlbum = albumRepo
playlistRepo := tests.CreateMockPlaylistRepo()
playlistRepo.SetData(model.Playlists{{ID: "pl1"}})
ds.MockedPlaylist = playlistRepo
radioRepo := tests.CreateMockedRadioRepo()
radioRepo.All = model.Radios{{ID: "ra1"}}
ds.MockedRadio = radioRepo
}
Describe("Fingerprint", func() {
It("changes when a fingerprint-affecting config value changes", func() {
f1 := ConfigFingerprint()
conf.Server.CoverArtPriority = "folder, embedded"
f2 := ConfigFingerprint()
Expect(f1).NotTo(Equal(f2))
})
DescribeTable("changes when a fingerprint-affecting config value changes",
func(change func()) {
before := ConfigFingerprint()
change()
Expect(ConfigFingerprint()).NotTo(Equal(before))
},
Entry("CoverArtPriority", func() { conf.Server.CoverArtPriority = "folder, embedded" }),
Entry("ArtistImageFolder", func() { conf.Server.ArtistImageFolder = "/after" }),
Entry("EnableM3UExternalAlbumArt", func() { conf.Server.EnableM3UExternalAlbumArt = true }),
)
It("changes when ArtistImageFolder changes", func() {
conf.Server.ArtistImageFolder = "/before"
f1 := ConfigFingerprint()
conf.Server.ArtistImageFolder = "/after"
Expect(ConfigFingerprint()).NotTo(Equal(f1))
})
It("changes when EnableM3UExternalAlbumArt is toggled", func() {
conf.Server.EnableM3UExternalAlbumArt = false
f1 := ConfigFingerprint()
conf.Server.EnableM3UExternalAlbumArt = true
Expect(ConfigFingerprint()).NotTo(Equal(f1))
})
// Pinned: a changed formula re-resolves every library on upgrade, flooding external providers.
// Pinned: a changed formula tells every existing install its artwork config went stale.
It("hashes a given config to a stable value", func() {
conf.Server.CoverArtPriority = "cover.*, embedded"
conf.Server.ArtistArtPriority = "artist.*, external"
@ -157,145 +87,23 @@ var _ = Describe("Housekeeping", func() {
f1 := ConfigFingerprint()
consts.Version = original + "-next"
Expect(ConfigFingerprint()).To(Equal(f1),
"the version must not invalidate artwork state: it would re-resolve every entity on every build")
"the version must not invalidate artwork state: every build would report a stale config")
})
})
Describe("Backfill", func() {
It("enqueues nothing and returns false when the stored fingerprint matches", func() {
seedEntities()
Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, ConfigFingerprint())).To(Succeed())
Describe("ReconcileConfigFingerprint", func() {
It("records the current fingerprint when none was ever stored", func() {
Expect(ReconcileConfigFingerprint(ctx, ds)).To(Succeed())
counted := false
s, err := backfill(ctx, ds, func() ImageAgentCount {
counted = true
return ImageAgentCount{Artist: 3, Album: 2}
})
Expect(err).ToNot(HaveOccurred())
Expect(s).To(Equal(backfillSummary{}))
Expect(counted).To(BeFalse(), "building the agent list constructs every agent; an unchanged fingerprint must not pay for it")
count, err := queueRepo.Count()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(BeZero())
Expect(propRepo.Get(consts.ArtConfFingerprintPropertyKey)).To(Equal(ConfigFingerprint()))
})
It("runs the backfill when no fingerprint was ever stored", func() {
seedEntities()
s, err := backfill(ctx, ds, noAgents)
Expect(err).ToNot(HaveOccurred())
Expect(s.Ran).To(BeTrue())
count, err := queueRepo.Count()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(Equal(int64(5))) // 2 artists + 1 album + 1 playlist + 1 radio
stored, err := propRepo.Get(consts.ArtConfFingerprintPropertyKey)
Expect(err).ToNot(HaveOccurred())
Expect(stored).To(Equal(ConfigFingerprint()))
})
It("enqueues a private playlist by resolving it under an admin context", func() {
ds.MockedUser = adminUserRepo()
vds := &visibilityPlaylistDS{
MockDataStore: ds,
private: model.Playlist{ID: "plPrivate", OwnerID: "admin"},
tracks: &tests.MockPlaylistTrackRepo{},
}
s, err := backfill(ctx, vds, noAgents)
Expect(err).ToNot(HaveOccurred())
Expect(s.Ran).To(BeTrue())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "pl", "plPrivate")).ToNot(BeNil())
})
It("enqueues artists before albums/playlists/radios, all at Backfill priority", func() {
seedEntities()
It("leaves a stale fingerprint stored, so the warning survives a restart", func() {
Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, "stale-fingerprint")).To(Succeed())
s, err := backfill(ctx, ds, noAgents)
Expect(err).ToNot(HaveOccurred())
Expect(s.Ran).To(BeTrue())
Expect(ReconcileConfigFingerprint(ctx, ds)).To(Succeed())
Expect(queueRepo.callKinds).ToNot(BeEmpty())
firstOther := slices.IndexFunc(queueRepo.callKinds, func(k string) bool { return k != "ar" })
Expect(firstOther).ToNot(Equal(0), "artists must be the first Enqueue call")
if firstOther >= 0 {
Expect(queueRepo.callKinds[firstOther:]).ToNot(ContainElement("ar"),
"no artist Enqueue may follow another kind")
}
for _, it := range queueRepo.Data {
Expect(it.Priority).To(Equal(model.ArtworkPriorityBackfill))
Expect(it.ItemKind).To(BeElementOf("ar", "al", "pl", "ra"))
}
})
It("reports what it enqueued, per kind and as an external-lookup ceiling", func() {
conf.Server.ArtistArtPriority = "artist.*, external"
conf.Server.CoverArtPriority = "cover.*, external"
conf.Server.EnableM3UExternalAlbumArt = false
seedEntities()
s, err := backfill(ctx, ds, func() ImageAgentCount { return ImageAgentCount{Artist: 3, Album: 2} })
Expect(err).ToNot(HaveOccurred())
Expect(s.Ran).To(BeTrue())
Expect(s.PerKind).To(Equal(map[string]int64{"ar": 2, "al": 1, "pl": 1, "ra": 1}))
Expect(s.Items).To(Equal(int64(5)))
// 2 artists x 3 agents, 1 album x 2, 1 playlist grid x 2, and radios never fetch.
Expect(s.MaxExternalLookups).To(Equal(int64(6 + 2 + PlaylistGridSamples*2)))
})
})
Describe("EnqueueStaleAbsentAll", func() {
var artRepo *tests.MockArtworkRepo
BeforeEach(func() {
artRepo = tests.CreateMockArtworkRepo()
ds.MockedArtwork = artRepo
queueRepo.ItemArtworkSource = artRepo
})
It("enqueues only absent entries older than the recheck window, across all kinds", func() {
old := time.Now().Add(-StaleAbsentAge - time.Hour)
recent := time.Now().Add(-StaleAbsentAge + time.Hour)
artRepo.ItemData["ar-stale"] = model.ItemArtwork{ItemKind: "ar", ItemID: "ar1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old}
artRepo.ItemData["al-stale"] = model.ItemArtwork{ItemKind: "al", ItemID: "al1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old}
artRepo.ItemData["pl-stale"] = model.ItemArtwork{ItemKind: "pl", ItemID: "pl1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old}
artRepo.ItemData["ra-stale"] = model.ItemArtwork{ItemKind: "ra", ItemID: "ra1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old}
artRepo.ItemData["ar-recent"] = model.ItemArtwork{ItemKind: "ar", ItemID: "ar2", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: recent}
artRepo.ItemData["al-resolved"] = model.ItemArtwork{ItemKind: "al", ItemID: "al2", ImageType: model.ImageTypePrimary, Hash: "somehash", AttemptedAt: old}
err := enqueueStaleAbsentAll(ctx, ds)
Expect(err).ToNot(HaveOccurred())
Expect(queueRepo.Data).To(HaveLen(4))
for _, it := range queueRepo.Data {
Expect(it.Priority).To(Equal(model.ArtworkPriorityRecheck))
}
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ar", "ar1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "pl", "pl1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ra", "ra1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ar", "ar2")).To(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al2")).To(BeNil())
})
It("caps each tick at the recheck batch, oldest attempts first", func() {
for i := range StaleAbsentRecheckBatch + 1 {
id := fmt.Sprintf("ar%d", i)
artRepo.ItemData[id] = model.ItemArtwork{ItemKind: "ar", ItemID: id, ImageType: model.ImageTypePrimary,
Hash: "", AttemptedAt: time.Now().Add(-StaleAbsentAge - time.Duration(i+1)*time.Minute)}
}
Expect(enqueueStaleAbsentAll(ctx, ds)).To(Succeed())
Expect(queueRepo.Data).To(HaveLen(StaleAbsentRecheckBatch))
// ar0 has the newest attempted_at of the cohort, so it is the one left out.
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ar", "ar0")).To(BeNil())
Expect(propRepo.Get(consts.ArtConfFingerprintPropertyKey)).To(Equal("stale-fingerprint"))
})
})
@ -315,8 +123,8 @@ var _ = Describe("Housekeeping", func() {
})
It("enqueues only entities that have no item_artwork row, across all kinds", func() {
artRepo.ItemData["al-resolved"] = model.ItemArtwork{ItemKind: "al", ItemID: "al1", ImageType: model.ImageTypePrimary, Hash: "somehash", AttemptedAt: time.Now()}
artRepo.ItemData["ar-absent"] = model.ItemArtwork{ItemKind: "ar", ItemID: "ar1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: time.Now()}
artRepo.ItemData["al-resolved"] = model.ItemArtwork{ItemKind: "al", ItemID: "al1", ImageType: model.ImageTypePrimary, Hash: "somehash"}
artRepo.ItemData["ar-absent"] = model.ItemArtwork{ItemKind: "ar", ItemID: "ar1", ImageType: model.ImageTypePrimary, Hash: ""}
err := enqueueMissingAll(ctx, ds)
Expect(err).ToNot(HaveOccurred())
@ -324,11 +132,64 @@ var _ = Describe("Housekeeping", func() {
for _, it := range queueRepo.Data {
Expect(it.Priority).To(Equal(model.ArtworkPriorityRecheck))
}
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al2")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "pl", "pl1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ra", "ra1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al1")).To(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ar", "ar1")).To(BeNil())
Expect(findQueued(queueRepo, "al", "al2")).ToNot(BeNil())
Expect(findQueued(queueRepo, "pl", "pl1")).ToNot(BeNil())
Expect(findQueued(queueRepo, "ra", "ra1")).ToNot(BeNil())
Expect(findQueued(queueRepo, "al", "al1")).To(BeNil())
Expect(findQueued(queueRepo, "ar", "ar1")).To(BeNil())
})
})
})
var _ = Describe("ItemName", func() {
var ds *tests.MockDataStore
var ctx context.Context
BeforeEach(func() {
ctx = context.Background()
albumRepo := tests.CreateMockAlbumRepo()
albumRepo.SetData(model.Albums{
{ID: "al-1", Name: "Kid A"},
{ID: "al-2", Name: "Sandinista!", Discs: model.Discs{2: "Side Three"}},
})
ds = &tests.MockDataStore{MockedAlbum: albumRepo}
Expect(ds.Artist(ctx).(*tests.MockArtistRepo).Put(&model.Artist{ID: "ar-1", Name: "Radiohead"})).To(Succeed())
})
It("returns the album name", func() {
Expect(ItemName(ctx, ds, model.KindAlbumArtwork, "al-1")).To(Equal("Kid A"))
})
It("returns the artist name", func() {
Expect(ItemName(ctx, ds, model.KindArtistArtwork, "ar-1")).To(Equal("Radiohead"))
})
It("errors for an unknown album", func() {
_, err := ItemName(ctx, ds, model.KindAlbumArtwork, "nope")
Expect(err).To(MatchError(model.ErrNotFound))
})
It("errors for an unsupported kind", func() {
// model.Kind is a struct with unexported fields, so the zero value is the only
// unsupported Kind constructible from outside package model.
_, err := ItemName(ctx, ds, model.Kind{}, "al-1")
Expect(err).To(HaveOccurred())
})
Context("disc artwork", func() {
It("names the album, the disc and its subtitle", func() {
Expect(ItemName(ctx, ds, model.KindDiscArtwork, "al-2:2")).
To(Equal("Sandinista! (disc 2): Side Three"))
})
It("omits the subtitle when the disc has none", func() {
Expect(ItemName(ctx, ds, model.KindDiscArtwork, "al-2:1")).
To(Equal("Sandinista! (disc 1)"))
})
It("rejects an id that is not <albumID>:<disc>", func() {
_, err := ItemName(ctx, ds, model.KindDiscArtwork, "al-2")
Expect(err).To(HaveOccurred())
})
})
})

32
core/artwork/processor.go
View file

@ -16,6 +16,7 @@ import (
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/core/artwork/blurhash"
"github.com/navidrome/navidrome/core/artwork/dominant"
"github.com/navidrome/navidrome/core/artwork/thumbhash"
@ -80,7 +81,7 @@ type processor struct {
// acquire resolves one queue item end to end: find an image, hash/decode/
// blurhash it, place its bytes, and persist the resulting state.
func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (out outcome, got *acquired) {
func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (out outcome, got *acquired, retryIn time.Duration) {
repo := p.ds.Artwork(ctx)
start := time.Now()
defer func() {
@ -92,10 +93,13 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
if err != nil {
traceStage(ctx, "resolve", err)
log.Warn(ctx, "Artwork: Could not resolve item", "kind", item.ItemKind, "id", item.ItemID, err)
return outcomeFailed, nil
return outcomeFailed, nil, 0
}
if retry, ok := errors.AsType[*agents.RetryLaterError](res.extErr); ok {
retryIn = retry.RetryIn
}
if res.reader == nil {
if res.extError || res.localError {
if res.extErr != nil || res.localError {
// A fault is not a definitive "no image": never settle absent, keep serving old state.
// A chainless resolver (playlist/radio) records no step, so leave a fallback or explain is blank.
if t := traceFrom(ctx); len(t.Steps()) == 0 {
@ -106,10 +110,10 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
t.add(TraceStep{Candidate: cmp.Or(res.source, "source"), Outcome: outcome})
}
log.Debug(ctx, "Artwork: No image, but a source faulted; keeping previous state",
"kind", item.ItemKind, "id", item.ItemID, "extError", res.extError, "localError", res.localError)
return outcomeFailed, nil
"kind", item.ItemKind, "id", item.ItemID, "extErr", res.extErr, "localError", res.localError)
return outcomeFailed, nil, retryIn
}
return writeAbsent(ctx, repo, item), nil
return writeAbsent(ctx, repo, item), nil, 0
}
defer res.reader.Close()
@ -118,7 +122,7 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
if err != nil {
traceStage(ctx, "read", err)
log.Warn(ctx, "Artwork: Failed to read resolved image", "kind", item.ItemKind, "id", item.ItemID, "source", res.source, err)
return outcomeFailed, nil
return outcomeFailed, nil, retryIn
}
log.Debug(ctx, "Artwork: Read resolved image", "kind", item.ItemKind, "id", item.ItemID,
"source", res.source, "bytes", len(data), "elapsed", time.Since(readStart))
@ -128,7 +132,7 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
if err != nil {
traceStage(ctx, "hash", err)
log.Warn(ctx, "Artwork: Failed to hash image", "kind", item.ItemKind, "id", item.ItemID, err)
return outcomeFailed, nil
return outcomeFailed, nil, retryIn
}
log.Trace(ctx, "Artwork: Hashed image", "kind", item.ItemKind, "id", item.ItemID,
"hash", hash, "bytes", len(data), "elapsed", time.Since(hashStart))
@ -152,14 +156,14 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
if err != nil {
traceStage(ctx, "decode", err)
log.Warn(ctx, "Artwork: Failed to decode resolved image", "kind", item.ItemKind, "id", item.ItemID, err)
return outcomeFailed, nil
return outcomeFailed, nil, retryIn
}
log.Debug(ctx, "Artwork: Decoded new image", "kind", item.ItemKind, "id", item.ItemID, "hash", hash,
"width", art.Width, "height", art.Height, "mime", art.Mime, "elapsed", time.Since(decodeStart))
default:
traceStage(ctx, "lookup", err)
log.Warn(ctx, "Artwork: Failed to look up image hash", "kind", item.ItemKind, "id", item.ItemID, err)
return outcomeFailed, nil
return outcomeFailed, nil, retryIn
}
art.SizeBytes = int64(len(data))
@ -167,15 +171,15 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o
if err != nil {
traceStage(ctx, "store", err)
log.Warn(ctx, "Artwork: Failed to persist resolved image", "kind", item.ItemKind, "id", item.ItemID, err)
return outcomeFailed, nil
return outcomeFailed, nil, retryIn
}
got = &acquired{ia: ia, mime: art.Mime, data: data}
if res.extError {
if res.extErr != nil {
log.Debug(ctx, "Artwork: Serving a lower-priority source after an external failure",
"kind", item.ItemKind, "id", item.ItemID, "source", res.source)
return outcomeFoundStale, got
return outcomeFoundStale, got, retryIn
}
return outcomeFound, got
return outcomeFound, got, retryIn
}
// persist places the bytes and commits the rows referencing them, excluding Prune for that

57
core/artwork/processor_test.go
View file

@ -90,7 +90,7 @@ var _ = Describe("processor.acquire", func() {
{ID: "al1", Name: "Album", FolderIDs: []string{"f1"}},
})
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al1"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al1"})
Expect(out).To(Equal(outcomeFound))
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al1", model.ImageTypePrimary)
@ -127,7 +127,7 @@ var _ = Describe("processor.acquire", func() {
{ID: "alL1", Name: "Album", FolderIDs: []string{"f1"}},
})
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alL1"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alL1"})
Expect(out).To(Equal(outcomeFound))
Expect(lock.locks).To(BeNumerically(">", 0), "the write window must exclude prune")
Expect(lock.held()).To(BeFalse(), "the window must close before acquire returns")
@ -141,7 +141,7 @@ var _ = Describe("processor.acquire", func() {
{ID: "alL2", Name: "Album", FolderIDs: []string{"f1"}},
})
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alL2"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alL2"})
Expect(out).To(Equal(outcomeAbsent))
Expect(lock.locks).To(BeZero())
})
@ -153,7 +153,7 @@ var _ = Describe("processor.acquire", func() {
})
folderRepo.result = nil
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al2"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al2"})
Expect(out).To(Equal(outcomeFound))
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al2", model.ImageTypePrimary)
@ -176,7 +176,7 @@ var _ = Describe("processor.acquire", func() {
{ID: "al3", Name: "Album"},
})
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al3"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al3"})
Expect(out).To(Equal(outcomeAbsent))
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al3", model.ImageTypePrimary)
@ -197,7 +197,7 @@ var _ = Describe("processor.acquire", func() {
{ID: "al-io", Name: "Album", FolderIDs: []string{"f1"}},
})
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al-io"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al-io"})
Expect(out).To(Equal(outcomeFailed))
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al-io", model.ImageTypePrimary)
@ -222,7 +222,7 @@ var _ = Describe("processor.acquire", func() {
DeferCleanup(func() { _ = os.Chmod(upload, 0o600) })
radioRepo.Data["ra-io"] = &model.Radio{ID: "ra-io", Name: "Station", UploadedImage: "ra-io.jpg"}
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra-io"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra-io"})
Expect(out).To(Equal(outcomeFailed))
_, err := artRepo.GetItemArtwork(model.KindRadioArtwork, "ra-io", model.ImageTypePrimary)
@ -249,7 +249,7 @@ var _ = Describe("processor.acquire", func() {
radioRepo.Data["ra-tr"] = &model.Radio{ID: "ra-tr", Name: "Station", UploadedImage: "ra-tr.jpg"}
trace := &ChainTrace{}
out, _ := proc.acquire(withTrace(ctx, trace), model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra-tr"})
out, _, _ := proc.acquire(withTrace(ctx, trace), model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra-tr"})
Expect(out).To(Equal(outcomeFailed))
steps := trace.Steps()
@ -265,13 +265,26 @@ var _ = Describe("processor.acquire", func() {
})
imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("agent timed out")})
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al4"})
out, _, retryIn := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al4"})
Expect(out).To(Equal(outcomeFailed))
Expect(retryIn).To(BeZero(), "a plain failure asks for no particular delay")
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al4", model.ImageTypePrimary)
Expect(err).To(MatchError(model.ErrNotFound))
})
It("failed-on-extError: reports the delay a throttled provider asked for", func() {
conf.Server.CoverArtPriority = "external"
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{
{ID: "al4r", Name: "Album"},
})
imageAgents(&fakeImageAgent{name: "throttled", err: &agents.RetryLaterError{RetryIn: 42 * time.Second}})
out, _, retryIn := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al4r"})
Expect(out).To(Equal(outcomeFailed))
Expect(retryIn).To(Equal(42 * time.Second))
})
It("found-stale: a fallback hit after a transient external failure persists state and returns outcomeFoundStale", func() {
conf.Server.CoverArtPriority = "external, cover.jpg"
folderRepo.result = []model.Folder{{
@ -283,7 +296,7 @@ var _ = Describe("processor.acquire", func() {
})
imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("agent timed out")})
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alstale"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alstale"})
Expect(out).To(Equal(outcomeFoundStale))
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alstale", model.ImageTypePrimary)
@ -300,7 +313,7 @@ var _ = Describe("processor.acquire", func() {
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alU", Name: "Album", FolderIDs: []string{"f1"}}})
folderRepo.result = []model.Folder{{Path: "album", ImageFiles: []string{"cover.jpg"}}}
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alU"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alU"})
Expect(out).To(Equal(outcomeFound))
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alU", model.ImageTypePrimary)
@ -320,7 +333,7 @@ var _ = Describe("processor.acquire", func() {
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alE", Name: "Album", FolderIDs: []string{"f1"}}})
folderRepo.result = []model.Folder{{Path: "album", ImageFiles: []string{"cover.jpg"}}}
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alE"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alE"})
Expect(out).To(Equal(outcomeFailed))
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alE", model.ImageTypePrimary)
@ -338,7 +351,7 @@ var _ = Describe("processor.acquire", func() {
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alX", Name: "Album"}})
imageAgents(&fakeImageAgent{name: "deezerFake", imgs: []agents.ExternalImage{{URL: srv.URL, Size: 500}}})
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alX"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alX"})
Expect(out).To(Equal(outcomeFailed))
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alX", model.ImageTypePrimary)
@ -357,7 +370,7 @@ var _ = Describe("processor.acquire", func() {
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alext", Name: "Album"}})
imageAgents(&fakeImageAgent{name: "deezerFake", imgs: []agents.ExternalImage{{URL: srv.URL, Size: 500}}})
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alext"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alext"})
Expect(out).To(Equal(outcomeFound))
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alext", model.ImageTypePrimary)
@ -382,7 +395,7 @@ var _ = Describe("processor.acquire", func() {
{ID: "al6", Name: "Album B", FolderIDs: []string{"f1"}},
})
out1, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al5"})
out1, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al5"})
Expect(out1).To(Equal(outcomeFound))
ia1, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al5", model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
@ -392,7 +405,7 @@ var _ = Describe("processor.acquire", func() {
poisoned.BlurHash = "SENTINEL"
artRepo.Data[ia1.Hash] = poisoned
out2, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al6"})
out2, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al6"})
Expect(out2).To(Equal(outcomeFound))
ia2, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al6", model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
@ -422,7 +435,7 @@ var _ = Describe("processor.acquire", func() {
})
folderRepo.result = []model.Folder{{Path: "album-a", ImageFiles: []string{"cover.jpg"}}}
outN, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alA"})
outN, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alA"})
Expect(outN).To(Equal(outcomeFound))
iaA, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alA", model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
@ -436,7 +449,7 @@ var _ = Describe("processor.acquire", func() {
artRepo.Data[iaA.Hash] = poisoned
folderRepo.result = []model.Folder{{Path: "album-b", ImageFiles: []string{"cover.jpg"}}}
outN, _ = proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alB"})
outN, _, _ = proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alB"})
Expect(outN).To(Equal(outcomeFound))
iaB, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alB", model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
@ -467,7 +480,7 @@ var _ = Describe("processor.acquire", func() {
radioRepo.Data = map[string]*model.Radio{"ra1": {ID: "ra1", Name: "Radio", UploadedImage: "ra1_test.jpg"}}
ds.MockedRadio = radioRepo
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra1"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra1"})
Expect(out).To(Equal(outcomeFailed))
_, err := artRepo.GetItemArtwork(model.KindRadioArtwork, "ra1", model.ImageTypePrimary)
@ -488,7 +501,7 @@ var _ = Describe("processor.acquire", func() {
radioRepo.Data = map[string]*model.Radio{"big": {ID: "big", Name: "Radio", UploadedImage: "big_test.jpg"}}
ds.MockedRadio = radioRepo
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "big"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "big"})
Expect(out).To(Equal(outcomeFailed))
_, err = artRepo.GetItemArtwork(model.KindRadioArtwork, "big", model.ImageTypePrimary)
@ -554,7 +567,7 @@ var _ = Describe("processor.acquire", func() {
Expect(err).ToNot(HaveOccurred())
Expect(artRepo.PutImage(&model.Artwork{Hash: hash, Mime: "application/octet-stream"})).To(Succeed())
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alM"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alM"})
Expect(out).To(Equal(outcomeFound))
upgraded, err := artRepo.GetImage(hash)
@ -574,7 +587,7 @@ var _ = Describe("processor.acquire", func() {
Expect(os.WriteFile(blockedRoot, []byte("x"), 0600)).To(Succeed())
proc.store = NewImageStore(blockedRoot)
out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al7"})
out, _, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al7"})
Expect(out).To(Equal(outcomeFailed))
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al7", model.ImageTypePrimary)

49
core/artwork/resolve.go
View file

@ -25,9 +25,9 @@ type resolution struct {
source string // model.ItemArtwork.Source value: "folder", "embedded", "external", "upload", "generated"
sourcePath string // backing library/upload file (folder/upload: the image; embedded: the audio file); "" otherwise
refMtime int64 // sourcePath mtime (unix-nanoseconds) at resolution; 0 when no sourcePath
// external source errored/timed out. With no reader it forces failed (never absent);
// on a hit a higher-priority external step failed—serve this, but retry later.
extError bool
// a faulted external source, carrying the provider's requested delay when it named one.
// With no reader it forces failed (never absent); on a hit, serve this but retry later.
extErr error
// a local source that should have been readable wasn't. With no reader it forces failed,
// so a transient I/O fault never records absent.
localError bool
@ -36,14 +36,15 @@ type resolution struct {
// chainState carries what a priority walk has seen so far. A hit takes extErr with it so a
// transient external failure still retries; localErr is dropped, as the scanner re-lists changes.
type chainState struct {
extErr, localErr bool
trace *ChainTrace // nil only where no caller attached one
extErr error
localErr bool
trace *ChainTrace // nil only where no caller attached one
}
// try stamps the accumulated external failure onto a hit, and records the miss otherwise.
func (c *chainState) try(candidate string, res resolution, ok bool) (resolution, bool) {
if ok {
res.extError = c.extErr
res.extErr = c.extErr
c.record(candidate, OutcomeHit, res.sourcePath)
return res, true
}
@ -62,7 +63,7 @@ func (c *chainState) record(candidate string, out Outcome, detail string) {
// exhausted is the outcome when no source in the chain yielded an image.
func (c *chainState) exhausted() resolution {
return resolution{extError: c.extErr, localError: c.localErr}
return resolution{extErr: c.extErr, localError: c.localErr}
}
// externalSource holds the agents to ask and the rate limiter/circuit breaker to ask them through.
@ -181,16 +182,16 @@ func chainFetchesExternal(priority string) bool {
// Album and artist fetches stop here when the resolver is local-only, rather than at each point in
// the chain walk; resolvePlaylist gates the third network path, the m3u image URL, itself.
func (r *resolver) fetchExternalAlbum(ctx context.Context, al model.Album) (io.ReadCloser, string, bool) {
func (r *resolver) fetchExternalAlbum(ctx context.Context, al model.Album) (io.ReadCloser, string, error) {
if r.ext == nil {
return nil, "", false
return nil, "", nil
}
return fetchAlbumImage(ctx, r.ext.agents, r.ext.gate, al)
}
func (r *resolver) fetchExternalArtist(ctx context.Context, ar model.Artist) (io.ReadCloser, string, bool) {
func (r *resolver) fetchExternalArtist(ctx context.Context, ar model.Artist) (io.ReadCloser, string, error) {
if r.ext == nil {
return nil, "", false
return nil, "", nil
}
return fetchArtistImage(ctx, r.ext.agents, r.ext.gate, ar)
}
@ -223,10 +224,10 @@ func (r *resolver) resolveAlbum(ctx context.Context, albumID string) (resolution
return res, nil
}
case pattern == externalCandidate:
if rd, name, isErr := r.fetchExternalAlbum(ctx, *al); rd != nil {
if rd, name, err := r.fetchExternalAlbum(ctx, *al); rd != nil {
return resolution{reader: rd, source: ExternalPrefix + name}, nil
} else if isErr {
chain.extErr = true
} else if err != nil {
chain.extErr = longerRetry(chain.extErr, err)
}
case len(imgFiles) > 0:
res, ok := resolveFolderFile(ctx, lib, imgFiles, pattern)
@ -285,10 +286,10 @@ func (r *resolver) resolveArtist(ctx context.Context, artistID string) (resoluti
}
switch {
case pattern == externalCandidate:
if rd, name, isErr := r.fetchExternalArtist(ctx, *ar); rd != nil {
if rd, name, err := r.fetchExternalArtist(ctx, *ar); rd != nil {
return resolution{reader: rd, source: ExternalPrefix + name}, nil
} else if isErr {
chain.extErr = true
} else if err != nil {
chain.extErr = longerRetry(chain.extErr, err)
}
case pattern == "image-folder":
res, ok := resolveArtistImageFolder(ar)
@ -332,7 +333,7 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso
return resolution{}, err
}
var extErr bool
var extErr error
for _, src := range []struct{ path, source string }{
{pl.UploadedImagePath(), "upload"},
{findPlaylistSidecarPath(ctx, pl.Path), "folder"},
@ -366,7 +367,7 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso
if res, ok, err := resolveExternalStep(r.ext.gate, "m3u", sf); ok {
return res, nil
} else if err != nil {
extErr = true
extErr = longerRetry(extErr, err)
// Record it here with its detail: once album sampling adds its own steps, the processor's
// empty-trace fallback no longer fires, and the error that forced the retry would be lost.
traceFrom(ctx).add(TraceStep{Candidate: ExternalPrefix + "m3u", Outcome: OutcomeError, Detail: err.Error()})
@ -389,8 +390,8 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso
}
continue
}
if res.extError {
extErr = true
if res.extErr != nil {
extErr = longerRetry(extErr, res.extErr)
}
if res.reader == nil {
continue
@ -409,7 +410,7 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso
if tileErr != nil {
return resolution{}, fmt.Errorf("resolvePlaylist: sampled album art failed: %w", tileErr)
}
return resolution{extError: extErr}, nil
return resolution{extErr: extErr}, nil
}
// Grow to 4 tiles by repeating what we have.
switch len(tiles) {
@ -420,9 +421,9 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso
}
grid, err := assembleTiles(tiles)
if err != nil {
return resolution{extError: extErr}, nil //nolint:nilerr // encode failure is a soft "no image", not a resolution error
return resolution{extErr: extErr}, nil //nolint:nilerr // encode failure is a soft "no image", not a resolution error
}
return resolution{reader: grid, source: "generated", extError: extErr}, nil
return resolution{reader: grid, source: "generated", extErr: extErr}, nil
}
// resolveRadio serves only an uploaded image; there is no fallback.

46
core/artwork/resolve_test.go
View file

@ -100,7 +100,7 @@ var _ = Describe("resolveItem", func() {
Expect(res.source).To(Equal("embedded"))
Expect(filepath.ToSlash(res.sourcePath)).To(HaveSuffix("tests/fixtures/artist/an-album/test.mp3"))
Expect(res.refMtime).To(BeNumerically(">", 0))
Expect(res.extError).To(BeFalse())
Expect(res.extErr).ToNot(HaveOccurred())
})
It("resolves absent when the track has no cover art", func() {
@ -111,7 +111,7 @@ var _ = Describe("resolveItem", func() {
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mf2"})
Expect(err).ToNot(HaveOccurred())
Expect(res.reader).To(BeNil())
Expect(res.extError).To(BeFalse())
Expect(res.extErr).ToNot(HaveOccurred())
})
It("resolves absent when media file cover art is disabled", func() {
@ -154,7 +154,7 @@ var _ = Describe("resolveItem", func() {
Expect(res.source).To(Equal("folder"))
Expect(filepath.ToSlash(res.sourcePath)).To(HaveSuffix("tests/fixtures/artist/an-album/cover.jpg"))
Expect(res.refMtime).To(BeNumerically(">", 0))
Expect(res.extError).To(BeFalse())
Expect(res.extErr).ToNot(HaveOccurred())
})
It("falls back to embedded art when no folder image matches", func() {
@ -172,7 +172,7 @@ var _ = Describe("resolveItem", func() {
Expect(res.refMtime).To(BeNumerically(">", 0))
})
It("sets extError when the external source errors without being not-found", func() {
It("sets extErr when the external source errors without being not-found", func() {
conf.Server.CoverArtPriority = "external"
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{
{ID: "al3", Name: "Album"},
@ -182,10 +182,10 @@ var _ = Describe("resolveItem", func() {
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al3"})
Expect(err).ToNot(HaveOccurred())
Expect(res.reader).To(BeNil())
Expect(res.extError).To(BeTrue())
Expect(res.extErr).To(HaveOccurred())
})
It("does not set extError when the external source reports not-found", func() {
It("does not set extErr when the external source reports not-found", func() {
conf.Server.CoverArtPriority = "external"
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{
{ID: "al4", Name: "Album"},
@ -195,10 +195,10 @@ var _ = Describe("resolveItem", func() {
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al4"})
Expect(err).ToNot(HaveOccurred())
Expect(res.reader).To(BeNil())
Expect(res.extError).To(BeFalse())
Expect(res.extErr).ToNot(HaveOccurred())
})
It("carries extError onto a fallback folder hit after a transient external failure", func() {
It("carries extErr onto a fallback folder hit after a transient external failure", func() {
conf.Server.CoverArtPriority = "external, cover.jpg"
folderRepo.result = []model.Folder{{
Path: "tests/fixtures/artist/an-album",
@ -214,10 +214,10 @@ var _ = Describe("resolveItem", func() {
Expect(res.reader).ToNot(BeNil())
defer res.reader.Close()
Expect(res.source).To(Equal("folder"))
Expect(res.extError).To(BeTrue())
Expect(res.extErr).To(HaveOccurred())
})
It("does not carry extError onto a fallback folder hit after a definitive external not-found", func() {
It("does not carry extErr onto a fallback folder hit after a definitive external not-found", func() {
conf.Server.CoverArtPriority = "external, cover.jpg"
folderRepo.result = []model.Folder{{
Path: "tests/fixtures/artist/an-album",
@ -233,7 +233,7 @@ var _ = Describe("resolveItem", func() {
Expect(res.reader).ToNot(BeNil())
defer res.reader.Close()
Expect(res.source).To(Equal("folder"))
Expect(res.extError).To(BeFalse())
Expect(res.extErr).ToNot(HaveOccurred())
})
It("routes the external step through the injected gate, keyed by agent name", func() {
@ -250,7 +250,7 @@ var _ = Describe("resolveItem", func() {
res, err := newResolver(ds, ag, ffm, gate).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al5"})
Expect(err).ToNot(HaveOccurred())
Expect(res.extError).To(BeTrue())
Expect(res.extErr).To(HaveOccurred())
Expect(gatedNames).To(Equal([]string{"failAgent"}))
})
})
@ -298,7 +298,7 @@ var _ = Describe("resolveItem", func() {
Expect(filepath.ToSlash(res.sourcePath)).To(HaveSuffix("tests/fixtures/artist/an-album/artist.png"))
})
It("sets extError when the external source errors without being not-found", func() {
It("sets extErr when the external source errors without being not-found", func() {
conf.Server.ArtistArtPriority = "external"
artistRepo := tests.CreateMockArtistRepo()
artistRepo.SetData(model.Artists{{ID: "ar3", Name: "Artist"}})
@ -308,10 +308,10 @@ var _ = Describe("resolveItem", func() {
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar3"})
Expect(err).ToNot(HaveOccurred())
Expect(res.reader).To(BeNil())
Expect(res.extError).To(BeTrue())
Expect(res.extErr).To(HaveOccurred())
})
It("does not set extError when the external source reports not-found", func() {
It("does not set extErr when the external source reports not-found", func() {
conf.Server.ArtistArtPriority = "external"
artistRepo := tests.CreateMockArtistRepo()
artistRepo.SetData(model.Artists{{ID: "ar4", Name: "Artist"}})
@ -321,7 +321,7 @@ var _ = Describe("resolveItem", func() {
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar4"})
Expect(err).ToNot(HaveOccurred())
Expect(res.reader).To(BeNil())
Expect(res.extError).To(BeFalse())
Expect(res.extErr).ToNot(HaveOccurred())
})
It("routes the external step through the injected gate, keyed by agent name", func() {
@ -338,7 +338,7 @@ var _ = Describe("resolveItem", func() {
res, err := newResolver(ds, ag, ffm, gate).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar5"})
Expect(err).ToNot(HaveOccurred())
Expect(res.extError).To(BeTrue())
Expect(res.extErr).To(HaveOccurred())
Expect(gatedNames).To(Equal([]string{"failAgent"}))
})
})
@ -516,7 +516,7 @@ var _ = Describe("resolveItem", func() {
res, err := newResolver(ds, ag, ffm, gate).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "ple"})
Expect(err).ToNot(HaveOccurred())
Expect(res.reader).To(BeNil())
Expect(res.extError).To(BeTrue())
Expect(res.extErr).To(HaveOccurred())
Expect(gatedNames).To(Equal([]string{"m3u"}), "the playlist URL fetch is gated under \"m3u\"")
})
@ -537,7 +537,7 @@ var _ = Describe("resolveItem", func() {
res, err := newResolver(ds, ag, ffm, gate).resolve(withTrace(ctx, trace),
model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plm3u"})
Expect(err).ToNot(HaveOccurred())
Expect(res.extError).To(BeTrue())
Expect(res.extErr).To(HaveOccurred())
steps := trace.Steps()
var m3u *TraceStep
@ -562,7 +562,7 @@ var _ = Describe("resolveItem", func() {
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plm"})
Expect(err).ToNot(HaveOccurred())
Expect(res.reader).To(BeNil())
Expect(res.extError).To(BeFalse())
Expect(res.extErr).ToNot(HaveOccurred())
})
It("treats an ExternalImageURL 404 as a definitive miss and falls through to the grid", func() {
@ -582,7 +582,7 @@ var _ = Describe("resolveItem", func() {
Expect(res.reader).ToNot(BeNil())
defer res.reader.Close()
Expect(res.source).To(Equal("generated"))
Expect(res.extError).To(BeFalse())
Expect(res.extErr).ToNot(HaveOccurred())
})
// A local resolver holds no agents: reaching the external branch would panic, not degrade.
@ -594,7 +594,7 @@ var _ = Describe("resolveItem", func() {
res, err := newLocalResolver(ds, ffm).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alx"})
Expect(err).ToNot(HaveOccurred())
Expect(res.reader).To(BeNil())
Expect(res.extError).To(BeFalse(), "a skipped step is not a failed one")
Expect(res.extErr).ToNot(HaveOccurred(), "a skipped step is not a failed one")
})
// The worker resolving the same playlist is asserted alongside, so this cannot pass vacuously.
@ -642,7 +642,7 @@ var _ = Describe("resolveItem", func() {
res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl500"})
Expect(err).ToNot(HaveOccurred())
Expect(res.reader).To(BeNil())
Expect(res.extError).To(BeTrue())
Expect(res.extErr).To(HaveOccurred())
})
It("yields an empty resolution when no album has art", func() {

17
core/artwork/sources.go
View file

@ -10,16 +10,14 @@ import (
"net/http"
"net/url"
"path/filepath"
"reflect"
"regexp"
"runtime"
"strings"
"time"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/ffmpeg"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/httpclient"
"go.senan.xyz/taglib"
)
@ -29,16 +27,6 @@ var errSourceUnreadable = errors.New("artwork source unreadable")
type sourceFunc func() (r io.ReadCloser, path string, err error)
func (f sourceFunc) String() string {
name := runtime.FuncForPC(reflect.ValueOf(f).Pointer()).Name()
name = strings.TrimPrefix(name, "github.com/navidrome/navidrome/core/artwork.")
if _, after, found := strings.Cut(name, ")."); found {
name = after
}
name = strings.TrimSuffix(name, ".func1")
return name
}
func fromExternalFile(ctx context.Context, libFS fs.FS, files []string, pattern string) sourceFunc {
return func() (io.ReadCloser, string, error) {
var openErr error
@ -163,9 +151,8 @@ type readCloser struct {
}
func fromURL(ctx context.Context, imageUrl *url.URL) (io.ReadCloser, string, error) {
hc := http.Client{Timeout: 5 * time.Second}
hc := httpclient.New(5 * time.Second)
req, _ := http.NewRequestWithContext(ctx, http.MethodGet, imageUrl.String(), nil)
req.Header.Set("User-Agent", consts.HTTPUserAgent)
resp, err := hc.Do(req) //nolint:gosec
if err != nil {
return nil, "", err

34
core/artwork/worker.go
View file

@ -23,8 +23,8 @@ import (
const (
workerPollInterval = 5 * time.Second
backoffBase = 5 * time.Second
// giveUpAfter bounds the retry budget from enqueue; past it the item falls to the
// periodic stale-absent recheck.
// giveUpAfter bounds the retry budget from enqueue; past it the item settles and only an
// explicit reprocess retries it.
giveUpAfter = 12 * time.Hour
)
@ -40,7 +40,6 @@ type drainPool struct {
// independently, and pruneMu serializes prune against the store-write window.
type Worker struct {
proc *processor
agents *agents.Agents
cache cache.FileCache
ffmpeg ffmpeg.FFmpeg
broker events.Broker
@ -55,7 +54,6 @@ type Worker struct {
func NewWorker(ds model.DataStore, store *ImageStore, ag *agents.Agents, ffmpeg ffmpeg.FFmpeg, broker events.Broker, imgCache cache.FileCache) *Worker {
w := &Worker{
proc: &processor{ds: ds, store: store},
agents: ag,
cache: imgCache,
ffmpeg: ffmpeg,
broker: broker,
@ -133,17 +131,9 @@ func (w *Worker) RunPrune(ctx context.Context) error {
return prune(ctx, w.proc.ds, w.proc.store)
}
// Backfill enqueues every entity for re-resolution when the artwork config fingerprint changed,
// artists first. It reports whether the backfill ran.
func (w *Worker) Backfill(ctx context.Context) (bool, error) {
s, err := backfill(ctx, w.proc.ds, func() ImageAgentCount { return NewImageAgentCount(w.agents) })
return s.Ran, err
}
// EnqueueStaleAbsentAll requeues known-absent entries older than StaleAbsentAge, at most
// StaleAbsentRecheckBatch per kind, oldest first.
func (w *Worker) EnqueueStaleAbsentAll(ctx context.Context) error {
return enqueueStaleAbsentAll(ctx, w.proc.ds)
// ReconcileConfig records the artwork config fingerprint, or warns when it changed.
func (w *Worker) ReconcileConfig(ctx context.Context) error {
return ReconcileConfigFingerprint(ctx, w.proc.ds)
}
// EnqueueMissingAll requeues entities with no artwork state row: the safety net for anything
@ -241,7 +231,7 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) (outc
item.ImageType = cmp.Or(item.ImageType, model.ImageTypePrimary)
trace := &ChainTrace{}
ctx = withTrace(ctx, trace)
out, got := w.proc.acquire(ctx, item)
out, got, retryIn := w.proc.acquire(ctx, item)
queue := w.proc.ds.ArtworkQueue(ctx)
switch out {
@ -252,7 +242,7 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) (outc
log.Warn(ctx, "Artwork: Could not delete processed queue item", "kind", item.ItemKind, "id", item.ItemID, err)
}
case outcomeFoundStale, outcomeFailed:
retryAt := time.Now().Add(backoff(item.Attempts))
retryAt := time.Now().Add(retryDelay(item.Attempts, retryIn))
encoded := trace.encode("")
if retryAt.Before(item.EnqueuedAt.Add(giveUpAfter)) {
// A mid-flight re-enqueue reset retry_at; stale backoff must not stomp its
@ -265,10 +255,9 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) (outc
"budgetLeft", time.Until(item.EnqueuedAt.Add(giveUpAfter)))
break
}
// Absent is only recoverable where a periodic recheck revisits it, so other kinds keep
// no row; art already being served is kept, as exhaustion means unreachable, not removed.
// Art already being served is kept: exhaustion means unreachable, not removed.
settled := "kept previous state"
if out == outcomeFailed && hasRecheckPath(item.ItemKind) && !w.hasResolvedArtwork(ctx, item) {
if out == outcomeFailed && settlesAbsentOnGiveUp(item.ItemKind) && !w.hasResolvedArtwork(ctx, item) {
writeAbsent(ctx, w.proc.ds.Artwork(ctx), item)
settled = "recorded absent"
}
@ -341,3 +330,8 @@ func backoffFor(attempts int, jitter float64) time.Duration {
func backoff(attempts int) time.Duration {
return backoffFor(attempts, rand.Float64()*0.8-0.4) //nolint:gosec // retry jitter, not security-sensitive
}
// retryDelay is how long a failed item waits: our backoff, unless the provider asked for longer.
func retryDelay(attempts int, hint time.Duration) time.Duration {
return max(backoff(attempts), hint)
}

2
core/artwork/worker_soak_test.go
View file

@ -95,7 +95,7 @@ var _ = Describe("Worker soak", func() {
start := time.Now()
for i := range soakCycles {
it := items[i%len(items)]
out, _ := proc.acquire(context.Background(), it)
out, _, _ := proc.acquire(context.Background(), it)
// Read-back exercises the surfaces a caller would use after acquisition.
if out == outcomeFound {

63
core/artwork/worker_test.go
View file

@ -15,6 +15,7 @@ import (
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/server/events"
"github.com/navidrome/navidrome/tests"
"github.com/navidrome/navidrome/utils/cache"
@ -115,6 +116,29 @@ func findQueued(q *tests.MockArtworkQueueRepo, kind, id string) *model.ArtworkQu
return nil
}
// visibilityPlaylistDS models playlist_repository's userFilter: a private playlist is only
// visible when the ctx carries an admin, so headless work must wrap ctx with one first.
type visibilityPlaylistDS struct {
*tests.MockDataStore
private model.Playlist
tracks model.PlaylistTrackRepository
}
func (v *visibilityPlaylistDS) Playlist(ctx context.Context) model.PlaylistRepository {
repo := tests.CreateMockPlaylistRepo()
repo.TracksRepo = v.tracks
if u, ok := request.UserFrom(ctx); ok && u.IsAdmin {
repo.SetData(model.Playlists{v.private})
}
return repo
}
func adminUserRepo() *tests.MockedUserRepo {
repo := tests.CreateMockUserRepo()
Expect(repo.Put(&model.User{ID: "admin", UserName: "admin", IsAdmin: true})).To(Succeed())
return repo
}
var _ = Describe("Worker", func() {
var (
ctx context.Context
@ -245,6 +269,23 @@ var _ = Describe("Worker", func() {
Expect(err).To(MatchError(model.ErrNotFound), "a timeout must never settle on absent")
})
It("reschedules past the provider's requested delay when it exceeds the backoff", func() {
conf.Server.CoverArtPriority = "external"
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "al9", Name: "Album"}})
// Well above backoff(0)'s jittered ceiling, so only the hint can produce this retry_at.
const askedFor = 90 * time.Minute
imageAgents(&fakeImageAgent{name: "throttledAgent", err: &agents.RetryLaterError{RetryIn: askedFor}})
Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ItemKind: "al", ItemID: "al9"})).To(Succeed())
n, err := w.drain(ctx, 2)
Expect(err).ToNot(HaveOccurred())
Expect(n).To(Equal(1))
it := findQueued(queueRepo, "al", "al9")
Expect(it).ToNot(BeNil())
Expect(it.RetryAt).To(BeTemporally("~", time.Now().Add(askedFor), time.Minute))
})
It("reschedules a found-stale item via MarkFailed while keeping its served state", func() {
conf.Server.CoverArtPriority = "external, cover.jpg"
folderRepo.result = []model.Folder{{
@ -424,9 +465,9 @@ var _ = Describe("Worker", func() {
Expect(ia.Hash).To(Equal("cafebabe"), "recording the failure must not disturb the served art")
})
// Media files are excluded from RecheckKinds, so an absent row here would never be
// revisited: a transient read error would look permanent.
It("does not settle absent on exhaustion for a kind with no recheck path", func() {
// Only a view enqueues a media file, and an absent row is exactly what stops a view from
// doing so: a transient read error would look permanent.
It("does not settle absent on exhaustion for a media file", func() {
conf.Server.EnableMediaFileCoverArt = true
ds.MockedMediaFile = tests.CreateMockMediaFileRepo()
ds.MockedMediaFile.(*tests.MockMediaFileRepo).SetData(model.MediaFiles{
@ -911,3 +952,19 @@ var _ = Describe("backoff", func() {
}
})
})
var _ = Describe("retryDelay", func() {
It("uses the backoff schedule when the provider asked for nothing", func() {
d := retryDelay(0, 0)
Expect(d).To(BeNumerically(">=", 3*time.Second))
Expect(d).To(BeNumerically("<=", 7*time.Second))
})
It("waits the provider's delay when it is longer than the backoff", func() {
Expect(retryDelay(0, time.Hour)).To(Equal(time.Hour))
})
It("keeps the backoff when it is longer than the provider's delay", func() {
Expect(retryDelay(4, time.Second)).To(BeNumerically(">=", 3*time.Second))
})
})

59
core/auth/auth.go
View file

@ -4,6 +4,8 @@ import (
"cmp"
"context"
"crypto/sha256"
"errors"
"slices"
"sync"
"time"
@ -26,6 +28,13 @@ var (
PublicTokenAuth *jwtauth.JWTAuth
)
// Audiences a session token can be scoped to. A token with no audience is accepted anywhere.
const (
AudienceJellyfin = "jellyfin"
AudienceSubsonic = "subsonic"
AudienceNative = "native"
)
// Init creates the JWTAuth objects from the secrets stored in the DB.
// Missing or undecryptable secrets are regenerated and stored.
func Init(ds model.DataStore) {
@ -66,15 +75,20 @@ func CreateExpiringPublicToken(exp time.Time, claims Claims) (string, error) {
return token, err
}
func CreateToken(u *model.User) (string, error) {
claims := Claims{
func userClaims(u *model.User, audience []string) Claims {
return Claims{
Issuer: consts.JWTIssuer,
Subject: u.UserName,
IssuedAt: time.Now(),
UserID: u.ID,
IsAdmin: u.IsAdmin,
Epoch: u.TokenEpoch,
Audience: audience,
}
token, _, err := TokenAuth.Encode(claims.ToMap())
}
func CreateToken(u *model.User) (string, error) {
token, _, err := TokenAuth.Encode(userClaims(u, nil).ToMap())
if err != nil {
return "", err
}
@ -82,10 +96,20 @@ func CreateToken(u *model.User) (string, error) {
return TouchToken(token)
}
// CreateAPIToken mints a non-expiring token scoped to one API, matching how Jellyfin
// clients expect tokens to behave. Revocation is by token epoch, not expiry.
func CreateAPIToken(u *model.User, audience string) (string, error) {
_, token, err := TokenAuth.Encode(userClaims(u, []string{audience}).ToMap())
return token, err
}
func TouchToken(token jwt.Token) (string, error) {
claims := ClaimsFromToken(token).
WithExpiresAt(time.Now().UTC().Add(conf.Server.SessionTimeout))
_, newToken, err := TokenAuth.Encode(claims.ToMap())
return TouchClaims(ClaimsFromToken(token))
}
func TouchClaims(c Claims) (string, error) {
c = c.WithExpiresAt(time.Now().UTC().Add(conf.Server.SessionTimeout))
_, newToken, err := TokenAuth.Encode(c.ToMap())
return newToken, err
}
@ -106,6 +130,29 @@ func ValidatePublic(tokenStr string) (Claims, error) {
return ClaimsFromToken(token), nil
}
var (
ErrTokenRevoked = errors.New("token revoked")
ErrWrongAudience = errors.New("token not valid for this API")
ErrWrongUser = errors.New("token issued for a different user")
)
// CheckClaims gates a session token against the user it names. Callers must have already
// verified the signature; this adds revocation and API scoping on top.
func CheckClaims(c Claims, usr model.User, audience string) error {
// Usernames can be reused: deleting a user and recreating the name yields a new random id
// at epoch 0, which an old token would otherwise match.
if c.UserID != "" && c.UserID != usr.ID {
return ErrWrongUser
}
if c.Epoch != usr.TokenEpoch {
return ErrTokenRevoked
}
if len(c.Audience) > 0 && !slices.Contains(c.Audience, audience) {
return ErrWrongAudience
}
return nil
}
func WithAdminUser(ctx context.Context, ds model.DataStore) context.Context {
u, err := ds.User(ctx).FindFirstAdmin()
if err != nil {

109
core/auth/auth_test.go
View file

@ -151,4 +151,113 @@ var _ = Describe("Auth", func() {
Expect(decodedClaims.ExpiresAt.Sub(yesterday)).To(BeNumerically(">=", oneDay))
})
})
Describe("CreateAPIToken", func() {
var usr *model.User
BeforeEach(func() {
usr = &model.User{ID: "123", UserName: "johndoe", TokenEpoch: 4}
})
It("does not expire", func() {
tokenStr, err := auth.CreateAPIToken(usr, auth.AudienceJellyfin)
Expect(err).ToNot(HaveOccurred())
claims, err := auth.Validate(tokenStr)
Expect(err).ToNot(HaveOccurred())
Expect(claims.ExpiresAt.IsZero()).To(BeTrue())
})
It("carries the audience and the user's epoch", func() {
tokenStr, err := auth.CreateAPIToken(usr, auth.AudienceJellyfin)
Expect(err).ToNot(HaveOccurred())
claims, err := auth.Validate(tokenStr)
Expect(err).ToNot(HaveOccurred())
Expect(claims.Audience).To(Equal([]string{"jellyfin"}))
Expect(claims.Epoch).To(Equal(4))
Expect(claims.Subject).To(Equal("johndoe"))
Expect(claims.UserID).To(Equal("123"))
})
})
Describe("CreateToken with an epoch", func() {
It("carries the epoch and still expires", func() {
usr := &model.User{ID: "123", UserName: "johndoe", TokenEpoch: 9}
tokenStr, err := auth.CreateToken(usr)
Expect(err).ToNot(HaveOccurred())
claims, err := auth.Validate(tokenStr)
Expect(err).ToNot(HaveOccurred())
Expect(claims.Epoch).To(Equal(9))
Expect(claims.Audience).To(BeEmpty())
Expect(claims.ExpiresAt).To(BeTemporally(">", time.Now()))
})
})
Describe("TouchClaims", func() {
It("preserves custom claims and refreshes the expiry", func() {
tokenStr, err := auth.TouchClaims(auth.Claims{Subject: "johndoe", UserID: "123", Epoch: 5})
Expect(err).ToNot(HaveOccurred())
claims, err := auth.Validate(tokenStr)
Expect(err).ToNot(HaveOccurred())
Expect(claims.Epoch).To(Equal(5))
Expect(claims.Subject).To(Equal("johndoe"))
Expect(claims.ExpiresAt).To(BeTemporally(">", time.Now()))
})
})
Describe("CheckClaims", func() {
usr := model.User{ID: "123", UserName: "johndoe", TokenEpoch: 2}
It("accepts a matching epoch and audience", func() {
c := auth.Claims{Epoch: 2, Audience: []string{auth.AudienceJellyfin}}
Expect(auth.CheckClaims(c, usr, auth.AudienceJellyfin)).To(Succeed())
})
It("accepts a token with no audience on any API", func() {
c := auth.Claims{Epoch: 2}
Expect(auth.CheckClaims(c, usr, auth.AudienceNative)).To(Succeed())
Expect(auth.CheckClaims(c, usr, auth.AudienceJellyfin)).To(Succeed())
Expect(auth.CheckClaims(c, usr, auth.AudienceSubsonic)).To(Succeed())
})
It("rejects a stale epoch", func() {
c := auth.Claims{Epoch: 1, Audience: []string{auth.AudienceJellyfin}}
Expect(auth.CheckClaims(c, usr, auth.AudienceJellyfin)).To(MatchError(auth.ErrTokenRevoked))
})
It("rejects a token minted for another API", func() {
c := auth.Claims{Epoch: 2, Audience: []string{auth.AudienceJellyfin}}
Expect(auth.CheckClaims(c, usr, auth.AudienceNative)).To(MatchError(auth.ErrWrongAudience))
Expect(auth.CheckClaims(c, usr, auth.AudienceSubsonic)).To(MatchError(auth.ErrWrongAudience))
})
It("accepts a multi-audience token that includes this API", func() {
c := auth.Claims{Epoch: 2, Audience: []string{"other", auth.AudienceNative}}
Expect(auth.CheckClaims(c, usr, auth.AudienceNative)).To(Succeed())
})
It("accepts a pre-upgrade token against a never-bumped user", func() {
fresh := model.User{ID: "456", UserName: "newbie"}
Expect(auth.CheckClaims(auth.Claims{}, fresh, auth.AudienceNative)).To(Succeed())
})
It("accepts a token whose user id matches", func() {
c := auth.Claims{UserID: "123", Epoch: 2}
Expect(auth.CheckClaims(c, usr, auth.AudienceNative)).To(Succeed())
})
It("rejects a token for a deleted user recreated under the same name", func() {
recreated := model.User{ID: "new-random-id", UserName: "johndoe"}
c := auth.Claims{UserID: "123", Audience: []string{auth.AudienceJellyfin}}
Expect(auth.CheckClaims(c, recreated, auth.AudienceJellyfin)).To(MatchError(auth.ErrWrongUser))
})
It("accepts a token that carries no user id", func() {
fresh := model.User{ID: "456", UserName: "newbie"}
Expect(auth.CheckClaims(auth.Claims{}, fresh, auth.AudienceNative)).To(Succeed())
})
})
})

32
core/auth/claims.go
View file

@ -11,7 +11,8 @@ import (
type Claims struct {
// Standard JWT claims
Issuer string
Subject string // username for session tokens
Subject string // username for session tokens
Audience []string // which API may accept this token; empty means any
IssuedAt time.Time
ExpiresAt time.Time
@ -22,6 +23,7 @@ type Claims struct {
Format string // "f" - audio format
BitRate int // "b" - audio bitrate
ShareID string // "sid" - share ID for share stream tokens
Epoch int // "ep" - the user's token_epoch at mint time
}
// ToMap converts Claims to a map[string]any for use with TokenAuth.Encode().
@ -34,6 +36,9 @@ func (c Claims) ToMap() map[string]any {
if c.Subject != "" {
m[jwt.SubjectKey] = c.Subject
}
if len(c.Audience) > 0 {
m[jwt.AudienceKey] = c.Audience
}
if !c.IssuedAt.IsZero() {
m[jwt.IssuedAtKey] = c.IssuedAt.UTC().Unix()
}
@ -58,6 +63,9 @@ func (c Claims) ToMap() map[string]any {
if c.ShareID != "" {
m["sid"] = c.ShareID
}
if c.Epoch != 0 {
m["ep"] = c.Epoch
}
return m
}
@ -73,6 +81,7 @@ func ClaimsFromToken(token jwt.Token) Claims {
c.Subject, _ = token.Subject()
c.IssuedAt, _ = token.IssuedAt()
c.ExpiresAt, _ = token.Expiration()
c.Audience, _ = token.Audience()
var uid string
if err := token.Get("uid", &uid); err == nil {
@ -90,15 +99,24 @@ func ClaimsFromToken(token jwt.Token) Claims {
if err := token.Get("f", &f); err == nil {
c.Format = f
}
if err := token.Get("b", &c.BitRate); err != nil {
var bf float64
if err := token.Get("b", &bf); err == nil {
c.BitRate = int(bf)
}
}
c.BitRate = intClaim(token, "b")
var sid string
if err := token.Get("sid", &sid); err == nil {
c.ShareID = sid
}
c.Epoch = intClaim(token, "ep")
return c
}
// intClaim reads a numeric claim, which a parsed token may decode as either int or float64.
func intClaim(token jwt.Token, key string) int {
var i int
if err := token.Get(key, &i); err == nil {
return i
}
var f float64
if err := token.Get(key, &f); err == nil {
return int(f)
}
return 0
}

40
core/auth/claims_test.go
View file

@ -105,4 +105,44 @@ var _ = Describe("Claims", func() {
})
})
Describe("Audience and Epoch claims", func() {
It("omits both when zero", func() {
m := auth.Claims{ID: "artwork-id"}.ToMap()
Expect(m).ToNot(HaveKey("aud"))
Expect(m).ToNot(HaveKey("ep"))
})
It("includes them when set", func() {
m := auth.Claims{Subject: "u", Epoch: 3, Audience: []string{"jellyfin"}}.ToMap()
Expect(m).To(HaveKeyWithValue("ep", 3))
Expect(m).To(HaveKeyWithValue("aud", []string{"jellyfin"}))
})
It("round-trips through a signed token", func() {
tokenAuth := jwtauth.New("HS256", []byte("test-secret"), nil)
_, tokenStr, err := tokenAuth.Encode(auth.Claims{
Subject: "u", Epoch: 7, Audience: []string{"jellyfin"},
}.ToMap())
Expect(err).ToNot(HaveOccurred())
token, err := jwtauth.VerifyToken(tokenAuth, tokenStr)
Expect(err).ToNot(HaveOccurred())
claims := auth.ClaimsFromToken(token)
Expect(claims.Epoch).To(Equal(7))
Expect(claims.Audience).To(Equal([]string{"jellyfin"}))
})
It("reads a token that has neither claim", func() {
tokenAuth := jwtauth.New("HS256", []byte("test-secret"), nil)
_, tokenStr, err := tokenAuth.Encode(auth.Claims{Subject: "u"}.ToMap())
Expect(err).ToNot(HaveOccurred())
token, err := jwtauth.VerifyToken(tokenAuth, tokenStr)
Expect(err).ToNot(HaveOccurred())
claims := auth.ClaimsFromToken(token)
Expect(claims.Epoch).To(BeZero())
Expect(claims.Audience).To(BeEmpty())
})
})
})

110
core/external/provider.go vendored
View file

@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"slices"
"sort"
"strings"
"time"
@ -14,6 +15,7 @@ import (
"github.com/navidrome/navidrome/core/matcher"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/server/events"
"github.com/navidrome/navidrome/utils"
. "github.com/navidrome/navidrome/utils/gg"
"github.com/navidrome/navidrome/utils/slice"
@ -33,12 +35,14 @@ type Provider interface {
UpdateArtistInfo(ctx context.Context, id string, count int, includeNotPresent bool) (*model.Artist, error)
SimilarSongs(ctx context.Context, id string, count int) (model.MediaFiles, error)
TopSongs(ctx context.Context, artist, artistId string, count int) (model.MediaFiles, error)
RefreshInfo(ctx context.Context, kind model.Kind, id string) error
}
type provider struct {
ds model.DataStore
ag Agents
matcher *matcher.Matcher
broker events.Broker
artistQueue refreshQueue[auxArtist]
albumQueue refreshQueue[auxAlbum]
}
@ -83,13 +87,17 @@ type Agents interface {
agents.SimilarSongsByArtistRetriever
}
func NewProvider(ds model.DataStore, agents Agents, m *matcher.Matcher) Provider {
e := &provider{ds: ds, ag: agents, matcher: m}
func NewProvider(ds model.DataStore, agents Agents, m *matcher.Matcher, broker events.Broker) Provider {
e := &provider{ds: ds, ag: agents, matcher: m, broker: broker}
e.artistQueue = newRefreshQueue(context.TODO(), e.populateArtistInfo)
e.albumQueue = newRefreshQueue(context.TODO(), e.populateAlbumInfo)
return e
}
func (e *provider) broadcastRefresh(ctx context.Context, resource, id string) {
e.broker.SendBroadcastMessage(ctx, (&events.RefreshResource{}).With(resource, id))
}
func (e *provider) getAlbum(ctx context.Context, id string) (auxAlbum, error) {
var entity any
entity, err := model.GetEntityByID(ctx, e.ds, id)
@ -140,7 +148,8 @@ func (e *provider) populateAlbumInfo(ctx context.Context, album auxAlbum) (auxAl
start := time.Now()
albumName := album.Name()
info, err := e.ag.GetAlbumInfo(ctx, albumName, album.AlbumArtist, album.MbzAlbumID)
if errors.Is(err, agents.ErrNotFound) {
// Throttled joins not-found: no answer to store, and an unstamped timestamp retries next call.
if errors.Is(err, agents.ErrNotFound) || errors.Is(err, agents.ErrRetryLater) {
return album, nil
}
if err != nil {
@ -179,6 +188,7 @@ func (e *provider) populateAlbumInfo(ctx context.Context, album auxAlbum) (auxAl
"elapsed", time.Since(start), err)
} else {
log.Trace(ctx, "AlbumInfo collected", "album", album, "elapsed", time.Since(start))
e.broadcastRefresh(ctx, "album", album.ID)
}
return album, nil
@ -244,38 +254,81 @@ func (e *provider) populateArtistInfo(ctx context.Context, artist auxArtist) (au
start := time.Now()
// Get MBID first, if it is not yet available
artistName := artist.Name()
var mbidErr error
if artist.MbzArtistID == "" {
mbid, err := e.ag.GetArtistMBID(ctx, artist.ID, artistName)
mbidErr = err
if mbid != "" && err == nil {
artist.MbzArtistID = mbid
}
}
// Call all registered agents and collect information
// Call all registered agents and collect information. The group carries no context, so a
// returned error does not cancel the siblings; only throttling is reported back.
g := errgroup.Group{}
g.SetLimit(2)
g.Go(func() error { _ = e.callGetImage(ctx, e.ag, &artist); return nil })
g.Go(func() error { e.callGetBiography(ctx, e.ag, &artist); return nil })
g.Go(func() error { e.callGetURL(ctx, e.ag, &artist); return nil })
g.Go(func() error { e.callGetSimilarArtists(ctx, e.ag, &artist, maxSimilarArtists, true); return nil })
_ = g.Wait()
g.Go(func() error { return retryLaterOnly(e.callGetImage(ctx, e.ag, &artist)) })
g.Go(func() error { return retryLaterOnly(e.callGetBiography(ctx, e.ag, &artist)) })
g.Go(func() error { return retryLaterOnly(e.callGetURL(ctx, e.ag, &artist)) })
g.Go(func() error {
return retryLaterOnly(e.callGetSimilarArtists(ctx, e.ag, &artist, maxSimilarArtists, true))
})
throttled := errors.Is(g.Wait(), agents.ErrRetryLater) || errors.Is(mbidErr, agents.ErrRetryLater)
if utils.IsCtxDone(ctx) {
log.Warn(ctx, "ArtistInfo update canceled", "id", artist.ID, "name", artistName, "elapsed", time.Since(start), ctx.Err())
return artist, ctx.Err()
}
artist.ExternalInfoUpdatedAt = new(time.Now())
// A throttled round keeps the previous timestamp, so the next call retries instead of
// serving an empty cache entry for the whole TTL.
if !throttled {
artist.ExternalInfoUpdatedAt = new(time.Now())
}
err := e.ds.Artist(ctx).UpdateExternalInfo(&artist.Artist)
if err != nil {
log.Error(ctx, "Error trying to update artist external information", "id", artist.ID, "name", artistName,
"elapsed", time.Since(start), err)
} else {
log.Trace(ctx, "ArtistInfo collected", "artist", artist, "elapsed", time.Since(start))
e.broadcastRefresh(ctx, "artist", artist.ID)
}
return artist, nil
}
// infoKinds are the kinds RefreshInfo can act on. Callers check this instead of restating
// the set, so the switch below stays the only place that has to know how each kind loads.
var infoKinds = []model.Kind{model.KindArtistArtwork, model.KindAlbumArtwork}
// HasInfo reports whether a kind has external info to refresh.
func HasInfo(kind model.Kind) bool { return slices.Contains(infoKinds, kind) }
// RefreshInfo re-fetches external info for one item, ignoring the TTL. It is synchronous:
// callers that must not block are responsible for detaching it.
func (e *provider) RefreshInfo(ctx context.Context, kind model.Kind, id string) error {
ctx, cancel := context.WithTimeout(ctx, refreshTimeout)
defer cancel()
switch kind {
case model.KindArtistArtwork:
artist, err := e.getArtist(ctx, id)
if err != nil {
return err
}
_, err = e.populateArtistInfo(ctx, artist)
return err
case model.KindAlbumArtwork:
album, err := e.getAlbum(ctx, id)
if err != nil {
return err
}
_, err = e.populateAlbumInfo(ctx, album)
return err
default:
return model.ErrNotFound
}
}
func (e *provider) TopSongs(ctx context.Context, artistName, id string, count int) (model.MediaFiles, error) {
artist, err := e.findArtist(ctx, artistName, id)
if err != nil {
@ -291,8 +344,9 @@ func (e *provider) TopSongs(ctx context.Context, artistName, id string, count in
songs, err := e.getMatchingTopSongs(ctx, e.ag, artist, count)
if err != nil {
switch {
case errors.Is(err, agents.ErrNotFound):
log.Trace(ctx, "TopSongs not found", "name", artistName)
// Throttled is not an answer, but the caller keeps the empty 200 it got before.
case errors.Is(err, agents.ErrNotFound), errors.Is(err, agents.ErrRetryLater):
log.Trace(ctx, "TopSongs not found", "name", artistName, err)
return nil, model.ErrNotFound
case errors.Is(err, context.Canceled):
log.Debug(ctx, "TopSongs call canceled", err)
@ -342,22 +396,33 @@ func (e *provider) getMatchingTopSongs(ctx context.Context, agent agents.ArtistT
return mfs, nil
}
func (e *provider) callGetURL(ctx context.Context, agent agents.ArtistURLRetriever, artist *auxArtist) {
artisURL, err := agent.GetArtistURL(ctx, artist.ID, artist.Name(), artist.MbzArtistID)
if err != nil {
return
// retryLaterOnly discards every failure the caller does not act on, so errgroup's
// first-error slot is reserved for the throttling signal.
func retryLaterOnly(err error) error {
if errors.Is(err, agents.ErrRetryLater) {
return err
}
artist.ExternalUrl = artisURL
return nil
}
func (e *provider) callGetBiography(ctx context.Context, agent agents.ArtistBiographyRetriever, artist *auxArtist) {
func (e *provider) callGetURL(ctx context.Context, agent agents.ArtistURLRetriever, artist *auxArtist) error {
artisURL, err := agent.GetArtistURL(ctx, artist.ID, artist.Name(), artist.MbzArtistID)
if err != nil {
return err
}
artist.ExternalUrl = artisURL
return nil
}
func (e *provider) callGetBiography(ctx context.Context, agent agents.ArtistBiographyRetriever, artist *auxArtist) error {
bio, err := agent.GetArtistBiography(ctx, artist.ID, artist.Name(), artist.MbzArtistID)
if err != nil {
return
return err
}
bio = str.SanitizeText(bio)
bio = strings.ReplaceAll(bio, "\n", " ")
artist.Biography = strings.ReplaceAll(bio, "<a ", "<a target='_blank' ")
return nil
}
// callGetImage populates artist's image URLs. A transient agent failure is
@ -385,19 +450,20 @@ func (e *provider) callGetImage(ctx context.Context, agent agents.ArtistImageRet
}
func (e *provider) callGetSimilarArtists(ctx context.Context, agent agents.ArtistSimilarRetriever, artist *auxArtist,
limit int, includeNotPresent bool) {
limit int, includeNotPresent bool) error {
artistName := artist.Name()
similar, err := agent.GetSimilarArtists(ctx, artist.ID, artistName, artist.MbzArtistID, limit)
if len(similar) == 0 || err != nil {
return
return err
}
start := time.Now()
sa, err := e.mapSimilarArtists(ctx, similar, limit, includeNotPresent)
log.Debug(ctx, "Mapped Similar Artists", "artist", artistName, "numSimilar", len(sa), "elapsed", time.Since(start))
if err != nil {
return
return err
}
artist.SimilarArtists = sa
return nil
}
func (e *provider) mapSimilarArtists(ctx context.Context, similar []agents.Artist, limit int, includeNotPresent bool) (model.Artists, error) {

156
core/external/provider_refreshinfo_test.go vendored Normal file
View file

@ -0,0 +1,156 @@
package external_test
import (
"context"
"slices"
"sync"
"time"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/core/external"
"github.com/navidrome/navidrome/core/matcher"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/server/events"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/stretchr/testify/mock"
)
type fakeBroker struct {
events.Broker
mu sync.Mutex
events []events.Event
}
func (f *fakeBroker) SendBroadcastMessage(_ context.Context, e events.Event) {
f.mu.Lock()
defer f.mu.Unlock()
f.events = append(f.events, e)
}
func (f *fakeBroker) sent() []events.Event {
f.mu.Lock()
defer f.mu.Unlock()
return slices.Clone(f.events)
}
var _ = Describe("Provider - RefreshInfo", func() {
var (
ctx context.Context
p external.Provider
ds *tests.MockDataStore
ag *mockAgents
broker *fakeBroker
mockArtistRepo *tests.MockArtistRepo
mockAlbumRepo *tests.MockAlbumRepo
)
expectArtistAgents := func() {
ag.On("GetArtistMBID", mock.Anything, mock.Anything, mock.Anything).Return("mbid-1", nil)
ag.On("GetArtistImages", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return([]agents.ExternalImage{}, nil)
ag.On("GetArtistBiography", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return("Fresh Bio", nil)
ag.On("GetArtistURL", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return("http://artist.url", nil)
ag.On("GetSimilarArtists", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return([]agents.Artist{}, nil)
}
expectAlbumAgents := func() {
ag.On("GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(&agents.AlbumInfo{URL: "http://album.url", Description: "Fresh Notes"}, nil)
ag.On("GetAlbumImages", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return([]agents.ExternalImage{}, nil)
}
BeforeEach(func() {
ctx = GinkgoT().Context()
ds = new(tests.MockDataStore)
ag = new(mockAgents)
broker = &fakeBroker{}
p = external.NewProvider(ds, ag, matcher.New(ds), broker)
mockArtistRepo = ds.Artist(ctx).(*tests.MockArtistRepo)
mockAlbumRepo = ds.Album(ctx).(*tests.MockAlbumRepo)
})
It("repopulates an artist even when its info is fresh", func() {
fresh := time.Now()
mockArtistRepo.SetData(model.Artists{{
ID: "ar-1", Name: "Test Artist", Biography: "stale", ExternalInfoUpdatedAt: &fresh,
}})
expectArtistAgents()
Expect(p.RefreshInfo(ctx, model.KindArtistArtwork, "ar-1")).To(Succeed())
saved, err := mockArtistRepo.Get("ar-1")
Expect(err).ToNot(HaveOccurred())
Expect(saved.Biography).To(Equal("Fresh Bio"))
})
It("repopulates an album even when its info is fresh", func() {
fresh := time.Now()
mockAlbumRepo.SetData(model.Albums{{
ID: "al-1", Name: "Test Album", AlbumArtist: "Test Artist",
Description: "stale", ExternalInfoUpdatedAt: &fresh,
}})
expectAlbumAgents()
Expect(p.RefreshInfo(ctx, model.KindAlbumArtwork, "al-1")).To(Succeed())
saved, err := mockAlbumRepo.Get("al-1")
Expect(err).ToNot(HaveOccurred())
Expect(saved.Description).To(Equal("Fresh Notes"))
})
It("returns ErrNotFound for an unknown id", func() {
Expect(p.RefreshInfo(ctx, model.KindArtistArtwork, "nope")).To(MatchError(model.ErrNotFound))
})
It("returns ErrNotFound for a kind with no external info", func() {
Expect(p.RefreshInfo(ctx, model.KindPlaylistArtwork, "pl-1")).To(MatchError(model.ErrNotFound))
})
It("broadcasts a RefreshResource naming the artist", func() {
mockArtistRepo.SetData(model.Artists{{ID: "ar-1", Name: "Test Artist"}})
expectArtistAgents()
Expect(p.RefreshInfo(ctx, model.KindArtistArtwork, "ar-1")).To(Succeed())
sent := broker.sent()
Expect(sent).To(HaveLen(1))
rr, ok := sent[0].(*events.RefreshResource)
Expect(ok).To(BeTrue())
Expect(rr.Data(rr)).To(ContainSubstring("ar-1"))
Expect(rr.Data(rr)).To(ContainSubstring("artist"))
})
It("broadcasts a RefreshResource naming the album", func() {
mockAlbumRepo.SetData(model.Albums{{ID: "al-1", Name: "Test Album", AlbumArtist: "Test Artist"}})
expectAlbumAgents()
Expect(p.RefreshInfo(ctx, model.KindAlbumArtwork, "al-1")).To(Succeed())
sent := broker.sent()
Expect(sent).To(HaveLen(1))
Expect(sent[0].Data(sent[0])).To(ContainSubstring("album"))
Expect(sent[0].Data(sent[0])).To(ContainSubstring("al-1"))
})
It("does not broadcast when the artist cannot be loaded", func() {
mockArtistRepo.SetData(model.Artists{{ID: "ar-1", Name: "Test Artist"}})
expectArtistAgents()
mockArtistRepo.SetError(true)
_ = p.RefreshInfo(ctx, model.KindArtistArtwork, "ar-1")
Expect(broker.sent()).To(BeEmpty())
})
It("reports which kinds have external info", func() {
Expect(external.HasInfo(model.KindArtistArtwork)).To(BeTrue())
Expect(external.HasInfo(model.KindAlbumArtwork)).To(BeTrue())
Expect(external.HasInfo(model.KindPlaylistArtwork)).To(BeFalse())
})
})

3
core/external/provider_similarsongs.go vendored
View file

@ -167,6 +167,7 @@ func (e *provider) seedMix(ctx context.Context, count int, sample func() (model.
if len(matched) == 0 {
matched = seeds
}
//nolint:gosec // shuffle order is not a security decision
rand.Shuffle(len(matched), func(i, j int) { matched[i], matched[j] = matched[j], matched[i] })
if len(matched) > count {
matched = matched[:count]
@ -239,7 +240,7 @@ func (e *provider) similarSongsFallback(ctx context.Context, id string, count in
return nil, err
}
e.callGetSimilarArtists(ctx, e.ag, &artist, 15, false)
_ = e.callGetSimilarArtists(ctx, e.ag, &artist, 15, false)
if utils.IsCtxDone(ctx) {
log.Warn(ctx, "SimilarSongs call canceled", ctx.Err())
return nil, ctx.Err()

2
core/external/provider_similarsongs_test.go vendored
View file

@ -61,7 +61,7 @@ var _ = Describe("Provider - SimilarSongs", func() {
similarAgent: mockSimilarAgent,
}
provider = NewProvider(ds, agentsCombined, matcher.New(ds))
provider = NewProvider(ds, agentsCombined, matcher.New(ds), &fakeBroker{})
})
// Resolves track-1 through the GetEntityByID probe order and on to its artist. Left permissive:

17
core/external/provider_topsongs_test.go vendored
View file

@ -45,7 +45,7 @@ var _ = Describe("Provider - TopSongs", func() {
ag = new(mockAgents)
p = NewProvider(ds, ag, matcher.New(ds))
p = NewProvider(ds, ag, matcher.New(ds), &fakeBroker{})
})
It("returns top songs for a known artist", func() {
@ -232,6 +232,21 @@ var _ = Describe("Provider - TopSongs", func() {
ag.AssertExpectations(GinkgoT())
})
// This endpoint answered with an empty list before retry-later existed; it must keep doing so.
It("returns an empty list, not a client error, when the agents are throttled", func() {
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 5).
Return(nil, agents.ErrRetryLater).Once()
songs, err := p.TopSongs(ctx, "Artist One", "", 5)
Expect(songs).To(BeEmpty())
Expect(err).To(MatchError(model.ErrNotFound), "the handler renders this as an empty 200")
Expect(err).ToNot(MatchError(agents.ErrRetryLater))
ag.AssertExpectations(GinkgoT())
})
It("returns fewer songs if count is less than available top songs", func() {
// Mock finding the artist
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}

24
core/external/provider_updatealbuminfo_test.go vendored
View file

@ -34,7 +34,7 @@ var _ = Describe("Provider - UpdateAlbumInfo", func() {
ctx = GinkgoT().Context()
ds = new(tests.MockDataStore)
ag = new(mockAgents)
p = external.NewProvider(ds, ag, matcher.New(ds))
p = external.NewProvider(ds, ag, matcher.New(ds), &fakeBroker{})
mockAlbumRepo = ds.Album(ctx).(*tests.MockAlbumRepo)
conf.Server.DevAlbumInfoTimeToLive = 1 * time.Hour
})
@ -164,4 +164,26 @@ var _ = Describe("Provider - UpdateAlbumInfo", func() {
ag.AssertExpectations(GinkgoT())
})
It("returns the original album, unstamped, when the agents are throttled", func() {
originalAlbum := &model.Album{
ID: "al-throttled",
Name: "Throttled Album",
AlbumArtist: "Throttled Artist",
MbzAlbumID: "mbid-throttled",
}
mockAlbumRepo.SetData(model.Albums{*originalAlbum})
ag.On("GetAlbumInfo", ctx, "Throttled Album", "Throttled Artist", "mbid-throttled").
Return(nil, agents.ErrRetryLater)
updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-throttled")
Expect(err).NotTo(HaveOccurred())
Expect(updatedAlbum).NotTo(BeNil())
Expect(*updatedAlbum).To(Equal(*originalAlbum))
Expect(updatedAlbum.ExternalInfoUpdatedAt).To(BeNil())
ag.AssertExpectations(GinkgoT())
})
})

21
core/external/provider_updateartistinfo_test.go vendored
View file

@ -37,7 +37,7 @@ var _ = Describe("Provider - UpdateArtistInfo", func() {
ctx = GinkgoT().Context()
ds = new(tests.MockDataStore)
ag = new(mockAgents)
p = external.NewProvider(ds, ag, matcher.New(ds))
p = external.NewProvider(ds, ag, matcher.New(ds), &fakeBroker{})
mockArtistRepo = ds.Artist(ctx).(*tests.MockArtistRepo)
})
@ -104,6 +104,25 @@ var _ = Describe("Provider - UpdateArtistInfo", func() {
ag.AssertExpectations(GinkgoT())
})
// Stamping a throttled round would cache the empty result for the whole TTL.
It("does not stamp ExternalInfoUpdatedAt when the agents are throttled", func() {
originalArtist := &model.Artist{ID: "ar-throttled", Name: "Throttled Artist"}
mockArtistRepo.SetData(model.Artists{*originalArtist})
ag.On("GetArtistMBID", ctx, "ar-throttled", "Throttled Artist").Return("", agents.ErrRetryLater).Once()
ag.On("GetArtistImages", ctx, "ar-throttled", "Throttled Artist", "").Return(nil, agents.ErrRetryLater).Once()
ag.On("GetArtistBiography", ctx, "ar-throttled", "Throttled Artist", "").Return("", agents.ErrRetryLater).Once()
ag.On("GetArtistURL", ctx, "ar-throttled", "Throttled Artist", "").Return("", agents.ErrRetryLater).Once()
ag.On("GetSimilarArtists", ctx, "ar-throttled", "Throttled Artist", "", 100).Return(nil, agents.ErrRetryLater).Once()
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-throttled", 10, false)
Expect(err).ToNot(HaveOccurred())
Expect(updatedArtist).NotTo(BeNil())
Expect(updatedArtist.ExternalInfoUpdatedAt).To(BeNil())
ag.AssertExpectations(GinkgoT())
})
It("preserves decoded plain text in biography storage", func() {
originalArtist := &model.Artist{
ID: "ar-encoded-bio",

4
core/library.go
View file

@ -191,7 +191,7 @@ func (r *libraryRepositoryWrapper) Save(entity any) (string, error) {
return strconv.Itoa(lib.ID), nil
}
func (r *libraryRepositoryWrapper) Update(id string, entity any, _ ...string) error {
func (r *libraryRepositoryWrapper) Update(id string, entity any, cols ...string) error {
lib := entity.(*model.Library)
libID, err := strconv.Atoi(id)
if err != nil {
@ -211,7 +211,7 @@ func (r *libraryRepositoryWrapper) Update(id string, entity any, _ ...string) er
pathChanged := originalLib.Path != lib.Path
err = r.LibraryRepository.Put(lib)
err = r.LibraryRepository.Put(lib, cols...)
if err != nil {
return r.mapError(err)
}

9
core/library_test.go
View file

@ -188,6 +188,15 @@ var _ = Describe("Library Service", func() {
Expect(libraryRepo.Data[1].Path).To(Equal(newTempDir))
})
It("forwards the columns sent by the client to the repository", func() {
library := &model.Library{ID: 1, Name: "Updated Library", Path: tempDir}
err := repo.Update("1", library, "name", "path")
Expect(err).NotTo(HaveOccurred())
Expect(libraryRepo.PutCols).To(Equal([]string{"name", "path"}))
})
It("fails when library doesn't exist", func() {
// Create a unique temporary directory to avoid path conflicts
uniqueTempDir, err := os.MkdirTemp("", "navidrome-nonexistent-")

7
core/metrics/insights.go
View file

@ -26,6 +26,7 @@ import (
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/plugins"
"github.com/navidrome/navidrome/server/events"
"github.com/navidrome/navidrome/utils/httpclient"
"github.com/navidrome/navidrome/utils/singleton"
)
@ -95,9 +96,7 @@ func (c *insightsCollector) sendInsights(ctx context.Context) {
log.Trace(ctx, "No users found, skipping Insights data collection")
return
}
hc := &http.Client{
Timeout: consts.DefaultHttpClientTimeOut,
}
hc := httpclient.New(consts.DefaultHttpClientTimeOut)
data := c.collect(ctx)
if data == nil {
return
@ -199,7 +198,7 @@ var staticData = sync.OnceValue(func() insights.Data {
// Config info
data.Config.LogLevel = conf.Server.LogLevel
data.Config.LogFileConfigured = conf.Server.LogFile != ""
data.Config.TLSConfigured = conf.Server.TLSCert != "" && conf.Server.TLSKey != ""
data.Config.TLSConfigured = conf.Server.TLSEnabled()
data.Config.DefaultBackgroundURLSet = conf.Server.UILoginBackgroundURL == consts.DefaultUILoginBackgroundURL
data.Config.EnableArtworkPrecache = conf.Server.EnableArtworkPrecache
data.Config.EnableArtworkUpload = conf.Server.EnableArtworkUpload

1
core/playback/queue.go
View file

@ -100,6 +100,7 @@ func (pd *Queue) Shuffle() {
backupID = current.ID
}
//nolint:gosec // shuffle order is not a security decision
rand.Shuffle(len(pd.Items), func(i, j int) { pd.Items[i], pd.Items[j] = pd.Items[j], pd.Items[i] })
var err error

4
core/playlists/parse_m3u.go
View file

@ -25,8 +25,8 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, folder *m
return err
}
var mfs model.MediaFiles
// Chunk size of 100 lines, as each line can generate up to 4 lookup candidates
// (NFC/NFD × raw/lowercase), and SQLite has a max expression tree depth of 1000.
// Chunked so a huge playlist is not held in memory at once. Each line yields up to
// 4 lookup candidates (NFC/NFD × raw/lowercase), far below SQLite's 32766 variables.
for lines := range slice.CollectChunks(slice.LinesFrom(reader), 100) {
filteredLines := make([]string, 0, len(lines))
for _, line := range lines {

41
core/publicurl/publicurl.go
View file

@ -2,7 +2,7 @@ package publicurl
import (
"cmp"
"net/http"
"context"
"net/url"
"path"
"strconv"
@ -13,35 +13,36 @@ import (
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/utils/gg"
)
// ImageURL generates a public URL for artwork images.
// It creates a signed token for the artwork ID and builds a complete public URL.
func ImageURL(req *http.Request, artID model.ArtworkID, size int) string {
func ImageURL(ctx context.Context, artID model.ArtworkID, size int) string {
token, _ := auth.CreatePublicToken(auth.Claims{ID: artID.String()})
uri := path.Join(consts.URLPathPublicImages, token)
params := url.Values{}
if size > 0 {
params.Add("size", strconv.Itoa(size))
}
return PublicURL(req, uri, params)
return PublicURL(ctx, uri, params)
}
// PublicURL builds a full URL for public-facing resources.
// It uses ShareURL from config if available, otherwise falls back to extracting
// the scheme and host from the provided http.Request.
// If req is nil and ShareURL is not set, it defaults to http://localhost.
func PublicURL(req *http.Request, u string, params url.Values) string {
// It uses ShareURL from config if available, otherwise falls back to the address the
// client used to reach the server, recorded in the context.
func PublicURL(ctx context.Context, u string, params url.Values) string {
if conf.Server.ShareURL == "" {
return AbsoluteURL(req, u, params)
return AbsoluteURL(ctx, u, params)
}
shareUrl, err := url.Parse(conf.Server.ShareURL)
if err != nil {
return AbsoluteURL(req, u, params)
return AbsoluteURL(ctx, u, params)
}
buildUrl, err := url.Parse(u)
if err != nil {
return AbsoluteURL(req, u, params)
return AbsoluteURL(ctx, u, params)
}
buildUrl.Scheme = shareUrl.Scheme
buildUrl.Host = shareUrl.Host
@ -55,13 +56,12 @@ func PublicURL(req *http.Request, u string, params url.Values) string {
}
// AbsoluteURL builds an absolute URL from a relative path.
// It uses BaseHost/BaseScheme from config if available, otherwise extracts
// the scheme and host from the http.Request.
// If req is nil and BaseHost is not set, it defaults to http://localhost.
func AbsoluteURL(req *http.Request, u string, params url.Values) string {
// It uses BaseHost/BaseScheme from config if available, otherwise the address the client
// used to reach the server, recorded in the context by the server's address middleware.
func AbsoluteURL(ctx context.Context, u string, params url.Values) string {
buildUrl, err := url.Parse(u)
if err != nil {
log.Error(req.Context(), "Failed to parse URL path", "url", u, err)
log.Error(ctx, "Failed to parse URL path", "url", u, err)
return ""
}
if strings.HasPrefix(u, "/") {
@ -69,12 +69,13 @@ func AbsoluteURL(req *http.Request, u string, params url.Values) string {
if conf.Server.BaseHost != "" {
buildUrl.Scheme = cmp.Or(conf.Server.BaseScheme, "http")
buildUrl.Host = conf.Server.BaseHost
} else if req != nil {
buildUrl.Scheme = req.URL.Scheme
buildUrl.Host = req.Host
} else if scheme, host, ok := request.ServerAddressFrom(ctx); ok {
buildUrl.Scheme = scheme
buildUrl.Host = host
} else {
buildUrl.Scheme = "http"
buildUrl.Host = "localhost"
log.Debug(ctx, "Building a public URL with no public address available; set ShareURL to make it reachable", "url", u)
buildUrl.Scheme = gg.If(conf.Server.TLSEnabled(), "https", "http")
buildUrl.Host = "localhost:" + strconv.Itoa(conf.Server.Port)
}
}
if len(params) > 0 {

113
core/publicurl/publicurl_test.go
View file

@ -1,7 +1,7 @@
package publicurl_test
import (
"net/http"
"context"
"net/url"
"testing"
@ -12,6 +12,7 @@ import (
"github.com/navidrome/navidrome/core/publicurl"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@ -36,24 +37,17 @@ var _ = Describe("Public URL Utilities", func() {
})
It("uses ShareURL as the base", func() {
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
result := publicurl.PublicURL(r, "/path/to/resource", nil)
result := publicurl.PublicURL(context.Background(), "/path/to/resource", nil)
Expect(result).To(Equal("https://share.example.com/path/to/resource"))
})
It("includes query parameters", func() {
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
params := url.Values{"size": []string{"300"}, "format": []string{"png"}}
result := publicurl.PublicURL(r, "/image/123", params)
result := publicurl.PublicURL(context.Background(), "/image/123", params)
Expect(result).To(ContainSubstring("https://share.example.com/image/123"))
Expect(result).To(ContainSubstring("size=300"))
Expect(result).To(ContainSubstring("format=png"))
})
It("works without a request", func() {
result := publicurl.PublicURL(nil, "/path/to/resource", nil)
Expect(result).To(Equal("https://share.example.com/path/to/resource"))
})
})
When("ShareURL includes a path", func() {
@ -62,21 +56,19 @@ var _ = Describe("Public URL Utilities", func() {
})
It("prepends the ShareURL path to the resource", func() {
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
result := publicurl.PublicURL(r, "/share/img/hash", nil)
result := publicurl.PublicURL(context.Background(), "/share/img/hash", nil)
Expect(result).To(Equal("https://example.com/navi/share/img/hash"))
})
It("prepends the ShareURL path and includes query parameters", func() {
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
params := url.Values{"size": []string{"600"}}
result := publicurl.PublicURL(r, "/share/img/hash", params)
result := publicurl.PublicURL(context.Background(), "/share/img/hash", params)
Expect(result).To(Equal("https://example.com/navi/share/img/hash?size=600"))
})
It("handles trailing slash in ShareURL path", func() {
conf.Server.ShareURL = "https://example.com/navi/"
result := publicurl.PublicURL(nil, "/share/img/hash", nil)
result := publicurl.PublicURL(context.Background(), "/share/img/hash", nil)
Expect(result).To(Equal("https://example.com/navi/share/img/hash"))
})
})
@ -87,15 +79,15 @@ var _ = Describe("Public URL Utilities", func() {
})
It("falls back to AbsoluteURL with request", func() {
r, _ := http.NewRequest("GET", "https://myserver.com/test", nil)
r.Host = "myserver.com"
result := publicurl.PublicURL(r, "/path/to/resource", nil)
ctx := request.WithServerAddress(context.Background(), "https", "myserver.com")
result := publicurl.PublicURL(ctx, "/path/to/resource", nil)
Expect(result).To(Equal("https://myserver.com/path/to/resource"))
})
It("falls back to localhost without request", func() {
result := publicurl.PublicURL(nil, "/path/to/resource", nil)
Expect(result).To(Equal("http://localhost/path/to/resource"))
It("falls back to localhost on the configured port without request", func() {
conf.Server.Port = 4533
result := publicurl.PublicURL(context.Background(), "/path/to/resource", nil)
Expect(result).To(Equal("http://localhost:4533/path/to/resource"))
})
})
})
@ -109,15 +101,13 @@ var _ = Describe("Public URL Utilities", func() {
})
It("uses BaseHost and BaseScheme", func() {
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
result := publicurl.AbsoluteURL(r, "/path/to/resource", nil)
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
Expect(result).To(Equal("https://configured.example.com/path/to/resource"))
})
It("defaults to http scheme if BaseScheme is empty", func() {
conf.Server.BaseScheme = ""
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
result := publicurl.AbsoluteURL(r, "/path/to/resource", nil)
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
Expect(result).To(Equal("http://configured.example.com/path/to/resource"))
})
})
@ -129,15 +119,30 @@ var _ = Describe("Public URL Utilities", func() {
})
It("extracts host from request", func() {
r, _ := http.NewRequest("GET", "https://request.example.com/test", nil)
r.Host = "request.example.com"
result := publicurl.AbsoluteURL(r, "/path/to/resource", nil)
ctx := request.WithServerAddress(context.Background(), "https", "request.example.com")
result := publicurl.AbsoluteURL(ctx, "/path/to/resource", nil)
Expect(result).To(Equal("https://request.example.com/path/to/resource"))
})
It("falls back to localhost without request", func() {
result := publicurl.AbsoluteURL(nil, "/path/to/resource", nil)
Expect(result).To(Equal("http://localhost/path/to/resource"))
It("falls back to localhost on the configured port without request", func() {
conf.Server.Port = 8080
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
Expect(result).To(Equal("http://localhost:8080/path/to/resource"))
})
It("uses https in the fallback when TLS is configured", func() {
conf.Server.Port = 4533
conf.Server.TLSCert = "cert.pem"
conf.Server.TLSKey = "key.pem"
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
Expect(result).To(Equal("https://localhost:4533/path/to/resource"))
})
It("stays on http when only the certificate is configured", func() {
conf.Server.Port = 4533
conf.Server.TLSCert = "cert.pem"
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
Expect(result).To(Equal("http://localhost:4533/path/to/resource"))
})
})
@ -149,24 +154,21 @@ var _ = Describe("Public URL Utilities", func() {
})
It("prepends BasePath to the URL", func() {
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
result := publicurl.AbsoluteURL(r, "/path/to/resource", nil)
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
Expect(result).To(Equal("https://example.com/navidrome/path/to/resource"))
})
})
It("passes through absolute URLs unchanged", func() {
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
result := publicurl.AbsoluteURL(r, "https://other.example.com/path", nil)
result := publicurl.AbsoluteURL(context.Background(), "https://other.example.com/path", nil)
Expect(result).To(Equal("https://other.example.com/path"))
})
It("includes query parameters", func() {
conf.Server.BaseHost = "example.com"
conf.Server.BaseScheme = "https"
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
params := url.Values{"key": []string{"value"}}
result := publicurl.AbsoluteURL(r, "/path", params)
result := publicurl.AbsoluteURL(context.Background(), "/path", params)
Expect(result).To(Equal("https://example.com/path?key=value"))
})
})
@ -180,20 +182,51 @@ var _ = Describe("Public URL Utilities", func() {
It("generates a URL with the artwork token", func() {
artID := model.NewArtworkID(model.KindAlbumArtwork, "album-123", nil)
result := publicurl.ImageURL(nil, artID, 0)
result := publicurl.ImageURL(context.Background(), artID, 0)
Expect(result).To(HavePrefix("https://share.example.com/share/img/"))
})
It("includes size parameter when provided", func() {
artID := model.NewArtworkID(model.KindArtistArtwork, "artist-1", nil)
result := publicurl.ImageURL(nil, artID, 300)
result := publicurl.ImageURL(context.Background(), artID, 300)
Expect(result).To(ContainSubstring("size=300"))
})
It("omits size parameter when zero", func() {
artID := model.NewArtworkID(model.KindMediaFileArtwork, "track-1", nil)
result := publicurl.ImageURL(nil, artID, 0)
result := publicurl.ImageURL(context.Background(), artID, 0)
Expect(result).ToNot(ContainSubstring("size="))
})
})
Describe("ImageURL address precedence", func() {
var artID model.ArtworkID
BeforeEach(func() {
auth.PublicTokenAuth = jwtauth.New("HS256", []byte("test secret"), nil)
artID = model.NewArtworkID(model.KindMediaFileArtwork, "track-1", nil)
})
It("uses the address of the request that triggered the call", func() {
ctx := request.WithServerAddress(context.Background(), "https", "music.example.com")
result := publicurl.ImageURL(ctx, artID, 300)
Expect(result).To(HavePrefix("https://music.example.com/share/img/"))
Expect(result).To(ContainSubstring("size=300"))
})
It("prefers ShareURL over the address in the context", func() {
conf.Server.ShareURL = "https://share.example.com"
ctx := request.WithServerAddress(context.Background(), "https", "music.example.com")
result := publicurl.ImageURL(ctx, artID, 0)
Expect(result).To(HavePrefix("https://share.example.com/share/img/"))
})
It("falls back to localhost on the configured port when no address is available", func() {
conf.Server.Port = 4533
result := publicurl.ImageURL(context.Background(), artID, 0)
Expect(result).To(HavePrefix("http://localhost:4533/share/img/"))
})
})
})

47
core/scrobbler/buffered_scrobbler.go
View file

@ -5,6 +5,7 @@ import (
"errors"
"time"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
@ -126,42 +127,52 @@ func (b *bufferedScrobbler) run(ctx context.Context) {
timer.Stop()
defer timer.Stop()
failures := 0
backingOff := false
for {
if b.processQueue(ctx) {
failures = 0
timer.Stop()
} else {
timer.Reset(backoffDelay(failures))
if failures < maxRetryShift {
failures++
// While a backoff window is open the timer is already armed for the rest of it, so a
// wake (a new play enqueued) must not drain: that is the hammering this avoids.
if !backingOff {
if ok, retryIn := b.processQueue(ctx); ok {
failures = 0
timer.Stop()
} else {
timer.Reset(max(backoffDelay(failures), retryIn))
backingOff = true
if failures < maxRetryShift {
failures++
}
}
}
select {
case <-b.wakeSignal:
case <-timer.C:
backingOff = false
case <-ctx.Done():
return
}
}
}
func (b *bufferedScrobbler) processQueue(ctx context.Context) bool {
func (b *bufferedScrobbler) processQueue(ctx context.Context) (bool, time.Duration) {
buffer := b.ds.ScrobbleBuffer(ctx)
userIds, err := buffer.UserIDs(b.service)
if err != nil {
log.Error(ctx, "Error retrieving userIds from scrobble buffer", "scrobbler", b.service, err)
return false
return false, 0
}
result := true
var retryIn time.Duration
for _, userId := range userIds {
if !b.processUserQueue(ctx, userId) {
ok, d := b.processUserQueue(ctx, userId)
if !ok {
result = false
retryIn = max(retryIn, d)
}
}
return result
return result, retryIn
}
func (b *bufferedScrobbler) processUserQueue(ctx context.Context, userId string) bool {
func (b *bufferedScrobbler) processUserQueue(ctx context.Context, userId string) (bool, time.Duration) {
// Scrobbles are drained on a background context that no longer carries the
// request's authenticated user. Restore it from the buffered userId so that
// scrobblers relying on the user in the context (e.g. plugins) still get it.
@ -175,25 +186,25 @@ func (b *bufferedScrobbler) processUserQueue(ctx context.Context, userId string)
entry, err := buffer.Next(b.service, userId)
if err != nil {
log.Error(ctx, "Error reading from scrobble buffer", "scrobbler", b.service, err)
return false
return false, 0
}
if entry == nil {
return true
return true, 0
}
s, ok := b.loader()
if !ok {
log.Warn(ctx, "Scrobbler not available, will retry later", "scrobbler", b.service)
return false
return false, 0
}
log.Debug(ctx, "Sending scrobble", "scrobbler", b.service, "track", entry.Title, "artist", entry.Artist)
err = s.Scrobble(ctx, entry.UserID, Scrobble{
MediaFile: entry.MediaFile,
TimeStamp: entry.PlayTime,
})
if errors.Is(err, ErrRetryLater) {
if retry, ok := errors.AsType[*agents.RetryLaterError](err); ok {
log.Warn(ctx, "Could not send scrobble. Will be retried", "userId", entry.UserID,
"track", entry.Title, "artist", entry.Artist, "scrobbler", b.service, err)
return false
return false, retry.RetryIn
}
if err != nil {
log.Error(ctx, "Error sending scrobble to service. Discarding", "scrobbler", b.service,
@ -203,7 +214,7 @@ func (b *bufferedScrobbler) processUserQueue(ctx context.Context, userId string)
if err != nil {
log.Error(ctx, "Error removing entry from scrobble buffer", "userId", entry.UserID,
"track", entry.Title, "artist", entry.Artist, "scrobbler", b.service, err)
return false
return false, 0
}
}
}

123
core/scrobbler/buffered_scrobbler_test.go
View file

@ -2,11 +2,13 @@ package scrobbler
import (
"context"
"errors"
"sync/atomic"
"testing"
"testing/synctest"
"time"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
@ -158,19 +160,129 @@ func TestBufferedScrobblerBackoffSchedule(t *testing.T) {
g.Expect(flaky.count.Load()).To(Equal(want), "retry did not fire after the %s backoff", gap)
}
// Once the service recovers, waking the loop drains the buffered entry.
// Once the service recovers, the buffered entry drains when the open
// backoff window closes (a wake alone must not drain it early).
flaky.succeed()
bs.sendWakeSignal()
synctest.Wait()
g.Expect(buffer.Length()).To(Equal(int64(1)), "wake during backoff drained early")
time.Sleep(80 * time.Second)
synctest.Wait()
g.Expect(buffer.Length()).To(Equal(int64(0)))
})
}
func TestBufferedScrobblerBackoffWindow(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
buffer := tests.CreateMockedScrobbleBufferRepo()
userRepo := tests.CreateMockUserRepo()
_ = userRepo.Put(&model.User{ID: "user1", UserName: "alice"})
ds := &tests.MockDataStore{MockedScrobbleBuffer: buffer, MockedUser: userRepo}
scr := &fakeScrobbler{Authorized: true}
scr.SetError(errors.Join(errors.New("boom"), ErrRetryLater))
bs := newBufferedScrobbler(ds, scr, "test")
defer bs.Stop()
// First enqueue: one immediate attempt, then a 5s window opens.
_ = bs.Scrobble(context.Background(), "user1", Scrobble{MediaFile: model.MediaFile{ID: "1"}, TimeStamp: time.Now()})
synctest.Wait()
if got := scr.ScrobbleAttempts(); got != 1 {
t.Fatalf("expected 1 attempt after first enqueue, got %d", got)
}
// A wake inside the window must NOT trigger an early attempt.
time.Sleep(1 * time.Second)
_ = bs.Scrobble(context.Background(), "user1", Scrobble{MediaFile: model.MediaFile{ID: "2"}, TimeStamp: time.Now()})
synctest.Wait()
if got := scr.ScrobbleAttempts(); got != 1 {
t.Fatalf("wake during backoff drained early: %d attempts", got)
}
// When the 5s window closes, the retry happens.
time.Sleep(4100 * time.Millisecond)
synctest.Wait()
if got := scr.ScrobbleAttempts(); got != 2 {
t.Fatalf("expected retry after window, got %d attempts", got)
}
})
}
func TestBufferedScrobblerHonorsServerDelay(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
buffer := tests.CreateMockedScrobbleBufferRepo()
userRepo := tests.CreateMockUserRepo()
_ = userRepo.Put(&model.User{ID: "user1", UserName: "alice"})
ds := &tests.MockDataStore{MockedScrobbleBuffer: buffer, MockedUser: userRepo}
scr := &fakeScrobbler{Authorized: true}
scr.SetError(errors.Join(errors.New("429"), &agents.RetryLaterError{RetryIn: 30 * time.Second}))
bs := newBufferedScrobbler(ds, scr, "test")
defer bs.Stop()
_ = bs.Scrobble(context.Background(), "user1", Scrobble{MediaFile: model.MediaFile{ID: "1"}, TimeStamp: time.Now()})
synctest.Wait()
if got := scr.ScrobbleAttempts(); got != 1 {
t.Fatalf("expected 1 attempt, got %d", got)
}
// The 5s exponential floor is overridden by the 30s server delay.
time.Sleep(20 * time.Second)
synctest.Wait()
if got := scr.ScrobbleAttempts(); got != 1 {
t.Fatalf("retried before server delay elapsed: %d attempts", got)
}
time.Sleep(10100 * time.Millisecond)
synctest.Wait()
if got := scr.ScrobbleAttempts(); got != 2 {
t.Fatalf("expected retry after server delay, got %d attempts", got)
}
})
}
// The drain visits users in an arbitrary order, so the longest delay must win regardless
// of which user was seen last.
func TestBufferedScrobblerTakesTheLongestServerDelayAcrossUsers(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
buffer := tests.CreateMockedScrobbleBufferRepo()
userRepo := tests.CreateMockUserRepo()
_ = userRepo.Put(&model.User{ID: "user1", UserName: "alice"})
_ = userRepo.Put(&model.User{ID: "user2", UserName: "bob"})
ds := &tests.MockDataStore{MockedScrobbleBuffer: buffer, MockedUser: userRepo}
scr := &recoveringScrobbler{delays: map[string]time.Duration{
"user1": 10 * time.Second,
"user2": 45 * time.Second,
}}
// Both are buffered before the drain goroutine exists: it drains once on startup, and
// seeing only one user there would park it on that user's delay, ignoring the other.
_ = buffer.Enqueue("test", "user1", "1", time.Now())
_ = buffer.Enqueue("test", "user2", "2", time.Now())
bs := newBufferedScrobbler(ds, scr, "test")
defer bs.Stop()
synctest.Wait()
if got := scr.count.Load(); got != 2 {
t.Fatalf("expected both users drained, got %d attempts", got)
}
time.Sleep(30 * time.Second)
synctest.Wait()
if got := scr.count.Load(); got != 2 {
t.Fatalf("retried on the shorter delay: %d attempts", got)
}
time.Sleep(15100 * time.Millisecond)
synctest.Wait()
if got := scr.count.Load(); got != 4 {
t.Fatalf("expected a retry after the longest delay, got %d attempts", got)
}
})
}
// recoveringScrobbler is a race-safe Scrobbler whose error can be toggled while
// the buffered scrobbler's goroutine is draining, to exercise retry then recovery.
// With delays set, it instead fails every scrobble asking for that user's delay.
type recoveringScrobbler struct {
err atomic.Pointer[error]
count atomic.Int32
err atomic.Pointer[error]
count atomic.Int32
delays map[string]time.Duration
}
func (f *recoveringScrobbler) fail(err error) { f.err.Store(&err) }
@ -182,8 +294,11 @@ func (f *recoveringScrobbler) NowPlaying(context.Context, string, *model.MediaFi
return nil
}
func (f *recoveringScrobbler) Scrobble(_ context.Context, _ string, _ Scrobble) error {
func (f *recoveringScrobbler) Scrobble(_ context.Context, userId string, _ Scrobble) error {
f.count.Add(1)
if f.delays != nil {
return errors.Join(errors.New("429"), &agents.RetryLaterError{RetryIn: f.delays[userId]})
}
if e := f.err.Load(); e != nil {
return *e
}

4
core/scrobbler/interfaces.go
View file

@ -5,6 +5,7 @@ import (
"errors"
"time"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/model"
)
@ -15,7 +16,8 @@ type Scrobble struct {
var (
ErrNotAuthorized = errors.New("not authorized")
ErrRetryLater = errors.New("retry later")
// ErrRetryLater is an alias of agents.ErrRetryLater so adapters and plugins share one identity.
ErrRetryLater = agents.ErrRetryLater
ErrUnrecoverable = errors.New("unrecoverable")
)

37
core/scrobbler/play_tracker_test.go
View file

@ -293,7 +293,7 @@ var _ = Describe("PlayTracker", func() {
})
It("increments play counts even if it cannot scrobble", func() {
fake.Error = errors.New("error")
fake.SetError(errors.New("error"))
err := tracker.Submit(ctx, []Submission{{TrackID: "123", Timestamp: time.Now()}})
@ -1414,7 +1414,25 @@ type fakeScrobbler struct {
position atomic.Int32
LastScrobble atomic.Pointer[Scrobble]
LastPlaybackReport atomic.Pointer[PlaybackSession]
Error error
err atomic.Pointer[error]
scrobbleAttempts atomic.Int32
}
// SetError sets the error returned by IsAuthorized/NowPlaying/Scrobble/PlaybackReport.
func (f *fakeScrobbler) SetError(err error) {
f.err.Store(&err)
}
func (f *fakeScrobbler) getError() error {
if e := f.err.Load(); e != nil {
return *e
}
return nil
}
// ScrobbleAttempts returns how many times Scrobble was called.
func (f *fakeScrobbler) ScrobbleAttempts() int32 {
return f.scrobbleAttempts.Load()
}
func (f *fakeScrobbler) GetNowPlayingCalled() bool {
@ -1440,13 +1458,13 @@ func (f *fakeScrobbler) GetTrack() *model.MediaFile {
}
func (f *fakeScrobbler) IsAuthorized(ctx context.Context, userId string) bool {
return f.Error == nil && f.Authorized
return f.getError() == nil && f.Authorized
}
func (f *fakeScrobbler) NowPlaying(ctx context.Context, userId string, track *model.MediaFile, position int) error {
f.nowPlayingCalled.Store(true)
if f.Error != nil {
return f.Error
if err := f.getError(); err != nil {
return err
}
f.userID.Store(&userId)
// Capture username from context (this is what plugin scrobblers do)
@ -1478,16 +1496,17 @@ func (f *fakeScrobbler) Scrobble(ctx context.Context, userId string, s Scrobble)
}
f.LastScrobble.Store(&s)
f.ScrobbleCalled.Store(true)
if f.Error != nil {
return f.Error
f.scrobbleAttempts.Add(1)
if err := f.getError(); err != nil {
return err
}
return nil
}
func (f *fakeScrobbler) PlaybackReport(ctx context.Context, info PlaybackSession) error {
f.PlaybackReportCalled.Store(true)
if f.Error != nil {
return f.Error
if err := f.getError(); err != nil {
return err
}
f.userID.Store(new(info.UserId))
f.LastPlaybackReport.Store(&info)

3
core/share.go
View file

@ -123,8 +123,7 @@ func (r *shareRepositoryWrapper) Save(entity any) (string, error) {
s.Contents = str.TruncateRunes(s.Contents, 30, "...")
id, err = r.Persistable.Save(s)
return id, err
return r.Persistable.Save(s)
}
func (r *shareRepositoryWrapper) Update(id string, entity any, _ ...string) error {

18
core/storage/local/deviceid_unix.go Normal file
View file

@ -0,0 +1,18 @@
//go:build !windows
package local
import (
"io/fs"
"syscall"
)
// deviceID identifies the filesystem a file lives on, used to key birth time support per mount.
// It is returned opaquely because its width varies by platform, and it is only used as a map key.
func deviceID(fi fs.FileInfo) (any, bool) {
st, ok := fi.Sys().(*syscall.Stat_t)
if !ok {
return nil, false
}
return st.Dev, true
}

8
core/storage/local/deviceid_windows.go Normal file
View file

@ -0,0 +1,8 @@
//go:build windows
package local
import "io/fs"
// deviceID has no Windows equivalent, and none is needed: birth time comes straight from FileInfo.
func deviceID(fs.FileInfo) (any, bool) { return nil, false }

40
core/storage/local/local.go
View file

@ -6,6 +6,7 @@ import (
"net/url"
"os"
"path/filepath"
"sync"
"sync/atomic"
"time"
@ -61,6 +62,8 @@ type localFS struct {
fs.FS
extractor Extractor
root string
// devices whose statx never reports a birth time (NFS, rclone/FUSE), so we ask each only once
noBirthTime sync.Map
}
// ResolveSymlink implements storage.SymlinkResolverFS. It resolves the whole chain at the
@ -84,7 +87,11 @@ func (lfs *localFS) ReadTags(path ...string) (map[string]metadata.Info, error) {
if err != nil {
return nil, err
}
v.FileInfo = localFileInfo{info}
v.FileInfo = localFileInfo{
FileInfo: info,
path: filepath.Join(lfs.root, filepath.FromSlash(path)),
noBirthTime: &lfs.noBirthTime,
}
res[path] = v
}
}
@ -95,15 +102,46 @@ func (lfs *localFS) ReadTags(path ...string) (map[string]metadata.Info, error) {
// with metadata.FileInfo
type localFileInfo struct {
fs.FileInfo
path string
noBirthTime *sync.Map
}
func (lfi localFileInfo) BirthTime() time.Time {
if ts := times.Get(lfi.FileInfo); ts.HasBirthTime() {
return ts.BirthTime()
}
if bt, ok := lfi.statxBirthTime(); ok {
return bt
}
return time.Now()
}
// statxBirthTime reads the birth time from the path, which on Linux is the only way to get it.
// Filesystems that never report one are remembered per device, so a scan asks each only once.
func (lfi localFileInfo) statxBirthTime() (time.Time, bool) {
if lfi.path == "" {
return time.Time{}, false
}
dev, hasDev := deviceID(lfi.FileInfo)
memo := lfi.noBirthTime
if hasDev && memo != nil {
if _, skip := memo.Load(dev); skip {
return time.Time{}, false
}
}
ts, err := times.Stat(lfi.path)
if err != nil {
return time.Time{}, false
}
if ts.HasBirthTime() {
return ts.BirthTime(), true
}
if hasDev && memo != nil {
memo.Store(dev, struct{}{})
}
return time.Time{}, false
}
func init() {
storage.Register(storage.LocalSchemaID, newLocalStorage)
}

33
core/storage/local/local_test.go
View file

@ -6,8 +6,10 @@ import (
"os"
"path/filepath"
"runtime"
"sync"
"time"
"github.com/djherbis/times"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
@ -440,6 +442,37 @@ var _ = Describe("LocalStorage", func() {
// Should be around the current time (within last few minutes)
Expect(birthTime).To(BeTemporally("~", time.Now(), 5*time.Minute))
})
It("reads the birth time from the path, not the time of the call", func() {
// On Linux, birth time is only available via statx(2) on the path.
lfi := localFileInfo{FileInfo: fileInfo, path: testFile}
time.Sleep(300 * time.Millisecond)
Expect(lfi.BirthTime()).To(BeTemporally("<", time.Now().Add(-200*time.Millisecond)))
})
It("does not remember filesystems that do report a birth time", func() {
memo := &sync.Map{}
lfi := localFileInfo{FileInfo: fileInfo, path: testFile, noBirthTime: memo}
lfi.BirthTime()
count := 0
memo.Range(func(_, _ any) bool { count++; return true })
Expect(count).To(BeZero())
})
It("skips statx on filesystems already known to have none", func() {
if times.Get(fileInfo).HasBirthTime() {
Skip("this platform reports birth time from FileInfo, so statx is never called")
}
dev, ok := deviceID(fileInfo)
Expect(ok).To(BeTrue())
memo := &sync.Map{}
memo.Store(dev, struct{}{})
lfi := localFileInfo{FileInfo: fileInfo, path: testFile, noBirthTime: memo}
time.Sleep(300 * time.Millisecond)
Expect(lfi.BirthTime()).To(BeTemporally("~", time.Now(), 100*time.Millisecond))
})
})
It("should delegate all other FileInfo methods", func() {

6
core/stream/media_streamer.go
View file

@ -152,8 +152,9 @@ func (s *Stream) EstimatedContentLength() int {
// Serve writes the stream to the HTTP response. For seekable streams it uses http.ServeContent
// (supporting range requests). For non-seekable streams it writes directly and logs any errors.
// Returns the number of bytes written and an error only when io.Copy fails with 0 bytes written
// Returns the number of bytes written and an error only when it fails with 0 bytes written
// (meaning the HTTP 200 status has not been flushed yet and the caller can still send an error response).
// Once bytes are on the wire it panics with http.ErrAbortHandler instead, aborting the response.
// Empty output (0 bytes, no error) is logged but not treated as an error.
func (s *Stream) Serve(ctx context.Context, w http.ResponseWriter, r *http.Request) (int64, error) {
if s.Seekable() {
@ -183,7 +184,8 @@ func (s *Stream) Serve(ctx context.Context, w http.ResponseWriter, r *http.Reque
w.Header().Del("Content-Length")
return 0, fmt.Errorf("sending transcoded file: %w", err)
}
return c, nil
// The 200 is already sent, so dropping the connection is the only way to say "truncated".
panic(http.ErrAbortHandler)
}
if c == 0 {
log.Error(ctx, "Transcoding returned empty output, ffmpeg may have failed. "+

51
core/stream/media_streamer_test.go
View file

@ -1,12 +1,18 @@
package stream_test
import (
"bytes"
"context"
"errors"
"io"
"net/http"
"net/http/httptest"
"os"
"testing/iotest"
"time"
"github.com/go-chi/chi/v5/middleware"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/core/stream"
@ -140,4 +146,49 @@ var _ = Describe("MediaStreamer", func() {
Expect(s.Seekable()).To(BeTrue())
})
})
Context("Serve", func() {
var mf *model.MediaFile
BeforeEach(func() {
var err error
mf, err = ds.MediaFile(ctx).Get("123")
Expect(err).ToNot(HaveOccurred())
})
It("keeps empty output a non-error, so callers still reply 200 with an empty body", func() {
s := stream.NewStream(mf, "mp3", 128, io.NopCloser(bytes.NewReader(nil)))
w := httptest.NewRecorder()
r := httptest.NewRequest(http.MethodGet, "/", nil)
n, err := s.Serve(ctx, w, r)
Expect(err).ToNot(HaveOccurred())
Expect(n).To(BeZero())
Expect(w.Code).To(Equal(http.StatusOK))
})
It("aborts the response when the source fails after sending data", func() {
src := io.NopCloser(io.MultiReader(
bytes.NewReader(bytes.Repeat([]byte("a"), 64*1024)),
iotest.ErrReader(errors.New("transcoder died")),
))
server := httptest.NewServer(serveHandler(stream.NewStream(mf, "mp3", 128, src)))
DeferCleanup(server.Close)
resp, err := http.Get(server.URL)
Expect(err).ToNot(HaveOccurred())
defer resp.Body.Close()
// A client-side read failure is the only observable proof the response was aborted.
_, err = io.ReadAll(resp.Body)
Expect(err).To(HaveOccurred())
})
})
})
// Serve runs behind the real server's Recoverer, which must let ErrAbortHandler through.
func serveHandler(s *stream.Stream) http.Handler {
return middleware.Recoverer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, _ = s.Serve(r.Context(), w, r)
}))
}

10
core/stream/token_test.go
View file

@ -232,6 +232,16 @@ var _ = Describe("Token", func() {
_, err := svc.ResolveRequestFromToken(ctx, token, mf, 0)
Expect(err).To(MatchError(ErrTokenStale))
})
It("rejects a Jellyfin access token", func() {
mf := &model.MediaFile{ID: "song-1", UpdatedAt: sourceTime}
usr := &model.User{ID: "u1", UserName: "johndoe"}
tokenStr, err := auth.CreateAPIToken(usr, auth.AudienceJellyfin)
Expect(err).ToNot(HaveOccurred())
_, err = svc.ResolveRequestFromToken(ctx, tokenStr, mf, 0)
Expect(err).To(MatchError(ErrTokenInvalid))
})
})
Describe("paramsFromToken", func() {

1
db/backup_test.go
View file

@ -68,6 +68,7 @@ var _ = Describe("database backups", func() {
timesShuffled = make([]time.Time, len(timesDecreasingChronologically))
copy(timesShuffled, timesDecreasingChronologically)
//nolint:gosec // shuffle order is not a security decision
rand.Shuffle(len(timesShuffled), func(i, j int) {
timesShuffled[i], timesShuffled[j] = timesShuffled[j], timesShuffled[i]
})

23
db/db.go
View file

@ -6,6 +6,7 @@ import (
"embed"
"errors"
"fmt"
"sync"
"time"
"github.com/mattn/go-sqlite3"
@ -13,10 +14,15 @@ import (
_ "github.com/navidrome/navidrome/db/migrations"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/utils/hasher"
"github.com/navidrome/navidrome/utils/natural"
"github.com/navidrome/navidrome/utils/singleton"
"github.com/pressly/goose/v3"
)
// NaturalCollation sorts embedded numbers by value. It is registered on every
// connection, but only referenced when conf.Server.EnableNaturalSorting is on.
const NaturalCollation = "NATSORT"
var (
Dialect = "sqlite3"
Driver = Dialect + "_custom"
@ -28,12 +34,21 @@ var embedMigrations embed.FS
const migrationsFolder = "migrations"
// sql.Register panics if called twice, so guard it: the singleton instance can be reset
// (tests/benchmarks) and rebuilt, but the driver is process-global and registers only once.
var registerDriverOnce sync.Once
func Db() *sql.DB {
return singleton.GetInstance(func() *sql.DB {
sql.Register(Driver, &sqlite3.SQLiteDriver{
ConnectHook: func(conn *sqlite3.SQLiteConn) error {
return conn.RegisterFunc("SEEDEDRAND", hasher.HashFunc(), false)
},
registerDriverOnce.Do(func() {
sql.Register(Driver, &sqlite3.SQLiteDriver{
ConnectHook: func(conn *sqlite3.SQLiteConn) error {
if err := conn.RegisterFunc("SEEDEDRAND", hasher.HashFunc(), false); err != nil {
return err
}
return conn.RegisterCollation(NaturalCollation, natural.CompareFold)
},
})
})
Path = conf.Server.DbPath
if Path == ":memory:" {

7
db/migrations/20260822062750_add_user_token_epoch.sql Normal file
View file

@ -0,0 +1,7 @@
-- +goose Up
ALTER TABLE user ADD COLUMN token_epoch INTEGER NOT NULL DEFAULT 0;
-- +goose Down
ALTER TABLE user DROP COLUMN token_epoch;

9
db/migrations/20260901225726_normalize_artwork_last_failure.sql Normal file
View file

@ -0,0 +1,9 @@
-- +goose Up
-- 20260819204637 added last_failure with DEFAULT '[]', so every row already in the table got a
-- non-empty value. That is how a give-up is now told apart from a definitive "no image", which
-- would report every pre-existing absent row as failed.
UPDATE item_artwork SET last_failure = '' WHERE last_failure = '[]';
-- +goose Down
-- Irreversible: a genuine give-up and a backfilled default are indistinguishable once normalized.
SELECT 1;

20
go.mod
View file

@ -1,9 +1,9 @@
module github.com/navidrome/navidrome
go 1.26
go 1.27
// Fork to implement raw tags support
replace go.senan.xyz/taglib => github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3
replace go.senan.xyz/taglib => github.com/deluan/go-taglib v0.0.0-20260905051825-df1d035571df
require (
github.com/Masterminds/squirrel v1.5.4
@ -13,14 +13,14 @@ require (
github.com/deluan/sanitize v0.0.0-20241120162836-fdfd8fdfaa55
github.com/dexterlb/mpvipc v0.0.0-20260722094525-0cf47d745b36
github.com/djherbis/atime v1.1.0
github.com/djherbis/fscache v0.10.2-0.20231127215153-442a07e326c4
github.com/djherbis/stream v1.4.0
github.com/djherbis/fscache v0.10.2-0.20260829235704-6d85d5878c22
github.com/djherbis/stream v1.5.1
github.com/djherbis/times v1.6.0
github.com/dustin/go-humanize v1.0.1
github.com/extism/go-sdk v1.7.1
github.com/fatih/structs v1.1.0
github.com/gen2brain/webp v0.6.4
github.com/go-chi/chi/v5 v5.3.1
github.com/go-chi/chi/v5 v5.3.2
github.com/go-chi/cors v1.2.2
github.com/go-chi/httprate v0.16.0
github.com/go-chi/jwtauth/v5 v5.4.0
@ -40,7 +40,7 @@ require (
github.com/microcosm-cc/bluemonday v1.0.27
github.com/mileusna/useragent v1.3.5
github.com/onsi/ginkgo/v2 v2.32.1
github.com/onsi/gomega v1.42.1
github.com/onsi/gomega v1.43.0
github.com/pelletier/go-toml/v2 v2.4.3
github.com/pmezard/go-difflib v1.0.0
github.com/pocketbase/dbx v1.12.0
@ -50,10 +50,10 @@ require (
github.com/robfig/cron/v3 v3.0.1
github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06
github.com/santhosh-tekuri/jsonschema/v6 v6.0.3
github.com/sirupsen/logrus v1.10.0
github.com/sirupsen/logrus v1.10.2
github.com/spf13/cobra v1.10.2
github.com/spf13/viper v1.21.0
github.com/stretchr/testify v1.12.0
github.com/stretchr/testify v1.12.1
github.com/tetratelabs/wazero v1.12.0
github.com/unrolled/secure v1.17.0
github.com/xrash/smetrics v0.0.0-20250705151800-55b8f293f342
@ -89,7 +89,7 @@ require (
github.com/goccy/go-json v0.10.6 // indirect
github.com/goccy/go-yaml v1.19.2 // indirect
github.com/google/go-cmp v0.7.0 // indirect
github.com/google/pprof v0.0.0-20260802141513-ef3492d7dac3 // indirect
github.com/google/pprof v0.0.0-20260825171938-4d453200e7d9 // indirect
github.com/google/subcommands v1.2.0 // indirect
github.com/gorilla/css v1.0.1 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
@ -101,7 +101,7 @@ require (
github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect
github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect
github.com/lestrrat-go/blackmagic v1.0.4 // indirect
github.com/lestrrat-go/dsig v1.3.0 // indirect
github.com/lestrrat-go/dsig v1.4.0 // indirect
github.com/lestrrat-go/dsig-secp256k1 v1.0.0 // indirect
github.com/lestrrat-go/httpcc v1.0.1 // indirect
github.com/lestrrat-go/httprc/v3 v3.0.6 // indirect

36
go.sum
View file

@ -29,8 +29,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1 h1:5RVFMOWjMyRy8cARdy79nAmgYw3hK/4HUq48LQ6Wwqo=
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1/go.mod h1:ZXNYxsqcloTdSy/rNShjYzMhyjf0LaoftYK0p+A3h40=
github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3 h1:j7eSXqgtjhlNfwnMEzRdXnJGZTEw4I7J9TeQAll83bU=
github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3/go.mod h1:QGxQ4Z1IWyY9w56xNEFjYAaWE8uSxA/gneQ7RPcFJrY=
github.com/deluan/go-taglib v0.0.0-20260905051825-df1d035571df h1:LdLQVAWVc6hCzqnrfVIEXOhP+r0iSit+EvsXwZDyL70=
github.com/deluan/go-taglib v0.0.0-20260905051825-df1d035571df/go.mod h1:QGxQ4Z1IWyY9w56xNEFjYAaWE8uSxA/gneQ7RPcFJrY=
github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf h1:tb246l2Zmpt/GpF9EcHCKTtwzrd0HGfEmoODFA/qnk4=
github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf/go.mod h1:tSgDythFsl0QgS/PFWfIZqcJKnkADWneY80jaVRlqK8=
github.com/deluan/sanitize v0.0.0-20241120162836-fdfd8fdfaa55 h1:wSCnggTs2f2ji6nFwQmfwgINcmSMj0xF0oHnoyRSPe4=
@ -39,10 +39,10 @@ github.com/dexterlb/mpvipc v0.0.0-20260722094525-0cf47d745b36 h1:KtPfdSST6e0vJbM
github.com/dexterlb/mpvipc v0.0.0-20260722094525-0cf47d745b36/go.mod h1:RkQWLNITKkXHLP7LXxZSgEq+uFWU25M5qW7qfEhL9Wc=
github.com/djherbis/atime v1.1.0 h1:rgwVbP/5by8BvvjBNrbh64Qz33idKT3pSnMSJsxhi0g=
github.com/djherbis/atime v1.1.0/go.mod h1:28OF6Y8s3NQWwacXc5eZTsEsiMzp7LF8MbXE+XJPdBE=
github.com/djherbis/fscache v0.10.2-0.20231127215153-442a07e326c4 h1:wdZllsLrDJtYfHiAKogB4PNHSDeO+v+5S3eqSWHGDlc=
github.com/djherbis/fscache v0.10.2-0.20231127215153-442a07e326c4/go.mod h1:dHWjlanKIxaHVH1xJOTb4kzP800XdcXlgJ6JYlR2DPU=
github.com/djherbis/stream v1.4.0 h1:aVD46WZUiq5kJk55yxJAyw6Kuera6kmC3i2vEQyW/AE=
github.com/djherbis/stream v1.4.0/go.mod h1:cqjC1ZRq3FFwkGmUtHwcldbnW8f0Q4YuVsGW1eAFtOk=
github.com/djherbis/fscache v0.10.2-0.20260829235704-6d85d5878c22 h1:GNKxzBirvK9arfVRGVebhFYBp3tnOZG3nlIog6N5/6I=
github.com/djherbis/fscache v0.10.2-0.20260829235704-6d85d5878c22/go.mod h1:Bbk9SqpJcg/saiPfG6byM1G4G/LQndknrsLVOQ+VJqY=
github.com/djherbis/stream v1.5.1 h1:9AoCl0bnm7imWT2uUORqT8aLuTh+YllyynYpolpjIqY=
github.com/djherbis/stream v1.5.1/go.mod h1:cqjC1ZRq3FFwkGmUtHwcldbnW8f0Q4YuVsGW1eAFtOk=
github.com/djherbis/times v1.6.0 h1:w2ctJ92J8fBvWPxugmXIv7Nz7Q3iDMKNx9v5ocVH20c=
github.com/djherbis/times v1.6.0/go.mod h1:gOHeRAz2h+VJNZ5Gmc/o7iD9k4wW7NMVqieYCY99oc0=
github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI=
@ -69,8 +69,8 @@ github.com/gkampitakis/go-diff v1.3.2 h1:Qyn0J9XJSDTgnsgHRdz9Zp24RaJeKMUHg2+PDZZ
github.com/gkampitakis/go-diff v1.3.2/go.mod h1:LLgOrpqleQe26cte8s36HTWcTmMEur6OPYerdAAS9tk=
github.com/gkampitakis/go-snaps v0.5.15 h1:amyJrvM1D33cPHwVrjo9jQxX8g/7E2wYdZ+01KS3zGE=
github.com/gkampitakis/go-snaps v0.5.15/go.mod h1:HNpx/9GoKisdhw9AFOBT1N7DBs9DiHo/hGheFGBZ+mc=
github.com/go-chi/chi/v5 v5.3.1 h1:3j4HZLGZQ3JpMCrPJF/Jl3mYJfWLKBfNJ6quurUGCf8=
github.com/go-chi/chi/v5 v5.3.1/go.mod h1:R+tYY2hNuVUUjxoPtqUdgBqevM9s9njzkTLutVsOCto=
github.com/go-chi/chi/v5 v5.3.2 h1:5YQkICvTCSZ25hoRsyJazN0scjzKGiu4VAUc7H1o1nY=
github.com/go-chi/chi/v5 v5.3.2/go.mod h1:R+tYY2hNuVUUjxoPtqUdgBqevM9s9njzkTLutVsOCto=
github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE=
github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58=
github.com/go-chi/httprate v0.16.0 h1:8V5DH9j6pSK6UQoBsTpvMyFxycqaKEIToyPKzHJjUa8=
@ -101,8 +101,8 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU=
github.com/google/go-pipeline v0.0.0-20230411140531-6cbedfc1d3fc h1:hd+uUVsB1vdxohPneMrhGH2YfQuH5hRIK9u4/XCeUtw=
github.com/google/go-pipeline v0.0.0-20230411140531-6cbedfc1d3fc/go.mod h1:SL66SJVysrh7YbDCP9tH30b8a9o/N2HeiQNUm85EKhc=
github.com/google/pprof v0.0.0-20260802141513-ef3492d7dac3 h1:LMLX+LgTNWpfvCBdFebv6EsYotImrt/Ppc5cXIriCSo=
github.com/google/pprof v0.0.0-20260802141513-ef3492d7dac3/go.mod h1:jl5iWTm0/hd5PjEYEOuwAJ57L/CibdZfrqZ5XA5GrCk=
github.com/google/pprof v0.0.0-20260825171938-4d453200e7d9 h1:dl4UZiszMU+NKHirOiCKTC+hRuNAQ0moHPxSg6WcU1o=
github.com/google/pprof v0.0.0-20260825171938-4d453200e7d9/go.mod h1:jl5iWTm0/hd5PjEYEOuwAJ57L/CibdZfrqZ5XA5GrCk=
github.com/google/subcommands v1.2.0 h1:vWQspBTo2nEqTUFita5/KeEWlUL8kQObDFbub/EN9oE=
github.com/google/subcommands v1.2.0/go.mod h1:ZjhPrFU+Olkh9WazFPsl27BQ4UPiG37m3yTrtFlrHVk=
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
@ -151,8 +151,8 @@ github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 h1:P6pPBnrTSX3DEVR4fDembhR
github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6FmdpVm2joNMFikkuWg0EoCKLGUMNw=
github.com/lestrrat-go/blackmagic v1.0.4 h1:IwQibdnf8l2KoO+qC3uT4OaTWsW7tuRQXy9TRN9QanA=
github.com/lestrrat-go/blackmagic v1.0.4/go.mod h1:6AWFyKNNj0zEXQYfTMPfZrAXUWUfTIZ5ECEUEJaijtw=
github.com/lestrrat-go/dsig v1.3.0 h1:phjMOCXvYzhuIgn7Voe2rex8z166vGfxRxmqM25P9/Q=
github.com/lestrrat-go/dsig v1.3.0/go.mod h1:RD2eOaidyPvpc7IJQoO3Qq52RWdy8ZcJs8lrOnoa1Kc=
github.com/lestrrat-go/dsig v1.4.0 h1:g7LUjK8cT74A5DzBXJI5HzsJuLhoYN0Wzj4nuOMIrH8=
github.com/lestrrat-go/dsig v1.4.0/go.mod h1:I8Nddg/vN2cUl/h8N7SRRApLnNNeyZPIqLYpvpOtGGo=
github.com/lestrrat-go/dsig-secp256k1 v1.0.0 h1:JpDe4Aybfl0soBvoVwjqDbp+9S1Y2OM7gcrVVMFPOzY=
github.com/lestrrat-go/dsig-secp256k1 v1.0.0/go.mod h1:CxUgAhssb8FToqbL8NjSPoGQlnO4w3LG1P0qPWQm/NU=
github.com/lestrrat-go/httpcc v1.0.1 h1:ydWCStUeJLkpYyjLDHihupbn2tYmZ7m22BGkcvZZrIE=
@ -187,8 +187,8 @@ github.com/ogier/pflag v0.0.1 h1:RW6JSWSu/RkSatfcLtogGfFgpim5p7ARQ10ECk5O750=
github.com/ogier/pflag v0.0.1/go.mod h1:zkFki7tvTa0tafRvTBIZTvzYyAu6kQhPZFnshFFPE+g=
github.com/onsi/ginkgo/v2 v2.32.1 h1:6tlvcDm/3sE8lGJbZ4+d4mO3RLy24/tQWOFzVSQNIfw=
github.com/onsi/ginkgo/v2 v2.32.1/go.mod h1:+aXOY+vzZ5mu2iI2HpTZUPmM//oQfsNFX6gU9kNcA44=
github.com/onsi/gomega v1.42.1 h1:iN1rCUX+44NZ1Dc97MPoeFYbFR0vh8zxoxMFwKdyZ6I=
github.com/onsi/gomega v1.42.1/go.mod h1:REff/hsDsodHoKlWsP2mAPhu1+5/6hVYNf9rIEBpeSg=
github.com/onsi/gomega v1.43.0 h1:VlG/1FxqNxhSO+lq/OHBNaaqwiBK/mO8JbVkX9Y+FeU=
github.com/onsi/gomega v1.43.0/go.mod h1:REff/hsDsodHoKlWsP2mAPhu1+5/6hVYNf9rIEBpeSg=
github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdDPYVpY=
github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY=
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
@ -232,8 +232,8 @@ github.com/segmentio/asm v1.2.1/go.mod h1:BqMnlJP91P8d+4ibuonYZw9mfnzI9HfxselHZr
github.com/sethvargo/go-retry v0.4.0 h1:9qy1OoIAxBL+gBYnkTnTnWle5wlfsXQlwRzIbbpdqPw=
github.com/sethvargo/go-retry v0.4.0/go.mod h1:tvsjdKG6xfiCx4LSiUZ06kcv38xvdVQwv8R6/VnnVWg=
github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
github.com/sirupsen/logrus v1.10.0 h1:T8MxJJXVZkfcC5zSRMRAg2F8+lxjmUCGGWPzFxO+Msc=
github.com/sirupsen/logrus v1.10.0/go.mod h1:FXZFonkDAnFozmO+5hGAFvB0Yg9/j2SIhA/QuIkP180=
github.com/sirupsen/logrus v1.10.2 h1:G2SED73/qrAu6YwbdxOD6peLkCBI3z7L+ykJFTXJBBo=
github.com/sirupsen/logrus v1.10.2/go.mod h1:SLEg8TqYulVKKfIGHldVp2K2aYz2DKSVBq4g/H5bR7Q=
github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d h1:zE9ykElWQ6/NYmHa3jpm/yHnI4xSofP+UP6SpjHcSeM=
github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc=
github.com/smartystreets/goconvey v1.6.4 h1:fv0U8FUIMPNf1L9lnHLvLhgicrIVChEkdzIKYqbNC9s=
@ -266,8 +266,8 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
github.com/stretchr/testify v1.12.0 h1:K6Mr6jO9JICuend/5xzTM03ydSV3vdNRYAdPSukj8uI=
github.com/stretchr/testify v1.12.0/go.mod h1:bOYBZb5qJ00vPzWfIqBUZPaxK8jWiXc6d3ErP4Ca9Gw=
github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE=
github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg=
github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8=
github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU=
github.com/tetratelabs/wabin v0.0.0-20230304001439-f6f874872834 h1:ZF+QBjOI+tILZjBaFj3HgFonKXUcwgJ4djLb6i42S3Q=

49
log/log.go
View file

@ -1,6 +1,7 @@
package log
import (
"cmp"
"context"
"errors"
"fmt"
@ -9,9 +10,10 @@ import (
"net/http"
"os"
"runtime"
"sort"
"slices"
"strings"
"sync"
"sync/atomic"
"time"
"github.com/sirupsen/logrus"
@ -47,8 +49,13 @@ var redacted = &Hook{
// External services query params. Values can be JWTs (dots, dashes), so match everything up
// to the next query separator or whitespace, not just word chars. A [\w]+ class would stop
// at a JWT's first '.' and leak its payload and signature.
"([^\\w]api_key=)[^&\\s]+",
// at a JWT's first '.' and leak its payload and signature. Case-insensitive with an
// optional underscore: the API accepts api_key, apikey and ApiKey alike.
"(?i)([^\\w]api_?key=)[^&\\s]+",
// Sensitive request headers, logged as a JSON blob at trace level and never matched by the
// query-param patterns above. Blank the whole value array; values may hold escaped quotes.
`(?i)("(?:Authorization|X-Emby-Token|X-MediaBrowser-Token|X-Nd-Authorization)":\[")[^\]]*("\])`,
},
}
@ -71,18 +78,19 @@ type levelPath struct {
}
var (
currentLevel Level
loggerMu sync.RWMutex
defaultLogger = logrus.New()
logSourceLine = false
rootPath string
logLevels []levelPath
currentLevel atomic.Uint32
hasLogLevelOverrides atomic.Bool
loggerMu sync.RWMutex
defaultLogger = logrus.New()
logSourceLine = false
rootPath string
logLevels []levelPath
)
// SetLevel sets the global log level used by the simple logger.
func SetLevel(l Level) {
loggerMu.Lock()
currentLevel = l
currentLevel.Store(uint32(l))
defaultLogger.Level = logrus.TraceLevel
loggerMu.Unlock()
logrus.SetLevel(logrus.Level(l))
@ -121,9 +129,10 @@ func SetLogLevels(levels map[string]string) {
for k, v := range levels {
logLevels = append(logLevels, levelPath{path: k, level: ParseLogLevel(v)})
}
sort.Slice(logLevels, func(i, j int) bool {
return logLevels[i].path > logLevels[j].path
slices.SortFunc(logLevels, func(a, b levelPath) int {
return cmp.Compare(b.path, a.path)
})
hasLogLevelOverrides.Store(len(logLevels) != 0)
}
func SetLogSourceLine(enabled bool) {
@ -188,9 +197,7 @@ func SetDefaultLogger(l *logrus.Logger) *logrus.Logger {
}
func CurrentLevel() Level {
loggerMu.RLock()
defer loggerMu.RUnlock()
return currentLevel
return Level(currentLevel.Load())
}
// IsGreaterOrEqualTo returns true if the caller's current log level is equal or greater than the provided level.
@ -243,18 +250,18 @@ func Writer() io.Writer {
}
func shouldLog(requiredLevel Level, skip int) bool {
loggerMu.RLock()
level := currentLevel
levels := logLevels
loggerMu.RUnlock()
level := Level(currentLevel.Load())
if level >= requiredLevel {
return true
}
if len(levels) == 0 {
if !hasLogLevelOverrides.Load() {
return false
}
loggerMu.RLock()
levels := logLevels
loggerMu.RUnlock()
_, file, _, ok := runtime.Caller(skip)
if !ok {
return false

29
log/log_test.go
View file

@ -2,7 +2,9 @@ package log
import (
"context"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"testing"
"time"
@ -92,7 +94,7 @@ var _ = Describe("Logger", func() {
SetLogSourceLine(true)
Error("A crash happened")
// NOTE: This assertion breaks if the line number above changes
Expect(hook.LastEntry().Data[" source"]).To(ContainSubstring("/log/log_test.go:93"))
Expect(hook.LastEntry().Data[" source"]).To(ContainSubstring("/log/log_test.go:95"))
Expect(hook.LastEntry().Message).To(Equal("A crash happened"))
})
@ -264,5 +266,30 @@ var _ = Describe("Logger", func() {
msg := "/jellyfin/Audio/abc/universal?static=true&api_key=eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJhZG1pbiJ9.c2ln-X_1&other=1"
Expect(Redact(msg)).To(Equal("/jellyfin/Audio/abc/universal?static=true&api_key=[REDACTED]&other=1"))
})
DescribeTable("redacts every api_key spelling the Jellyfin API accepts",
func(param string) {
msg := "/jellyfin/Audio/abc/File?" + param + "=SECRET&other=1"
Expect(Redact(msg)).To(Equal("/jellyfin/Audio/abc/File?" + param + "=[REDACTED]&other=1"))
},
Entry("api_key", "api_key"),
Entry("apikey", "apikey"),
Entry("ApiKey", "ApiKey"),
Entry("APIKEY", "APIKEY"),
)
It("redacts sensitive request headers in a logged header blob", func() {
h := http.Header{
"Authorization": {`MediaBrowser Client="Finamp", Token="jwt-secret"`},
"X-Emby-Token": {"emby-secret"},
"X-Mediabrowser-Token": {"mb-secret"},
"X-Nd-Authorization": {"Bearer nd-secret"},
"User-Agent": {"Finamp/1.0"},
}
blob, _ := json.Marshal(h)
got := Redact(string(blob))
Expect(got).ToNot(ContainSubstring("secret"))
Expect(got).To(ContainSubstring(`"User-Agent":["Finamp/1.0"]`))
})
})
})

1
model/album.go
View file

@ -143,7 +143,6 @@ type AlbumRepository interface {
UpdateExternalInfo(*Album) error
Get(id string) (*Album, error)
GetAll(...QueryOptions) (Albums, error)
GetAllIDs(...QueryOptions) ([]string, error)
// GetSoleAlbumArtistIDsInSubtrees returns the sole album artists of the albums with folders in
// any of the given library-relative subtrees.
GetSoleAlbumArtistIDsInSubtrees(lib Library, paths ...string) ([]string, error)

1
model/artist.go
View file

@ -90,7 +90,6 @@ type ArtistRepository interface {
UpdateExternalInfo(a *Artist) error
Get(id string) (*Artist, error)
GetAll(options ...QueryOptions) (Artists, error)
GetAllIDs(options ...QueryOptions) ([]string, error)
GetCursor(options ...QueryOptions) (ArtistCursor, error)
GetIndex(includeMissing bool, libraryIds []int, roles ...Role) (ArtistIndexes, error)

33
model/artwork.go
View file

@ -18,6 +18,10 @@ type Artwork struct {
const ImageTypePrimary = "primary"
// ArtworkSourceFailed is a pseudo-source selecting absent states that exhausted the retry budget
// rather than being answered. The "!" keeps it from colliding with a stored source value.
const ArtworkSourceFailed = "!failed"
// ItemImage is per-entity artwork state hydrated at query time; never persisted.
type ItemImage struct {
ImageHash string `structs:"-" json:"imageHash,omitempty"`
@ -88,11 +92,12 @@ func (i ItemArtworkInfo) Image() ItemImage {
}
type ArtworkQueueItem struct {
ItemKind string `structs:"item_kind"`
ItemID string `structs:"item_id"`
ImageType string `structs:"image_type"`
Priority int `structs:"priority"`
Attempts int `structs:"attempts"`
ItemKind string `structs:"item_kind"`
ItemID string `structs:"item_id"`
ImageType string `structs:"image_type"`
Priority int `structs:"priority"`
Attempts int `structs:"attempts"`
// RetryAt is the earliest time the drain may take this row, not when it will run.
RetryAt time.Time `structs:"retry_at"`
EnqueuedAt time.Time `structs:"enqueued_at"`
// Trace is why the last attempt failed. Only Get reads it; the drain projects it away.
@ -101,7 +106,9 @@ type ArtworkQueueItem struct {
// Queue priorities: higher drains first.
const (
ArtworkPriorityRecheck = 0
ArtworkPriorityRecheck = 0
// ArtworkPriorityBackfill sits between the hourly sweep and scan-driven work. Nothing enqueues
// it today; it stays named so a row still carrying it can be reported and cancelled.
ArtworkPriorityBackfill = 10
ArtworkPriorityScan = 50
ArtworkPriorityBump = 100
@ -134,15 +141,13 @@ type ArtworkQueueRepository interface {
// EnqueuePreservingBackoff upserts like Enqueue but preserves an existing row's retry_at, so a
// request-triggered read-through never resets a failed resolution's backoff.
EnqueuePreservingBackoff(items ...ArtworkQueueItem) error
// EnqueueStaleAbsent inserts queue rows (priority Recheck) for absent states older than cutoff, oldest
// first; limit caps the selection, so already-queued rows use up budget (backpressure when the drain stalls).
EnqueueStaleAbsent(kind Kind, attemptedBefore time.Time, limit int) (int64, error)
// EnqueueAllMissing inserts queue rows for all entities with no item_artwork row, at the given priority.
EnqueueAllMissing(kind Kind, priority int) (int64, error)
// EnqueueIfMissing inserts only for items with no item_artwork row yet.
EnqueueIfMissing(items ...ArtworkQueueItem) error
// CountBySource reports how many items of a kind currently resolve from the given sources.
// An empty sources slice means every source; "" matches absent state.
// An empty sources slice means every source; "" matches absent state, and the pseudo-source
// ArtworkSourceFailed matches the absent states that gave up.
CountBySource(kind Kind, sources []string) (int64, error)
// SourcesInUse lists the distinct sources items of a kind currently resolve from, "" included.
SourcesInUse(kind Kind) ([]string, error)
@ -161,9 +166,6 @@ type ArtworkQueueRepository interface {
// CountQueued reports the pending rows matching the kinds and priorities, grouped by both;
// an empty filter means every one.
CountQueued(kinds []Kind, priorities []int) ([]ArtworkQueueStat, error)
// CountAbsent reports the absent states of a kind, and how many are past the given cutoff,
// eligible for EnqueueStaleAbsent (which drains them limit rows per call).
CountAbsent(kind Kind, attemptedBefore time.Time) (ArtworkAbsentStat, error)
// PurgeDangling removes queue rows whose entity no longer exists.
PurgeDangling() (int64, error)
// PurgeQueued removes pending rows matching the kinds and priorities; an empty filter means every one.
@ -175,8 +177,3 @@ type ArtworkQueueStat struct {
Priority int
Count int64
}
type ArtworkAbsentStat struct {
Total int64
Stale int64
}

2
model/library.go
View file

@ -45,7 +45,7 @@ type LibraryRepository interface {
GetPath(id int) (string, error)
GetAll(...QueryOptions) (Libraries, error)
CountAll(...QueryOptions) (int64, error)
Put(*Library) error
Put(l *Library, colsToUpdate ...string) error
Delete(id int) error
StoreMusicFolder() error
AddArtist(id int, artistID string) error

2
model/mediafile.go
View file

@ -553,8 +553,6 @@ type MediaFileRepository interface {
// expression, using the logged user's annotations. Limit and offset are ignored.
MatchesCriteria(id string, c criteria.Criteria) (bool, error)
GetCursor(options ...QueryOptions) (MediaFileCursor, error)
// GetAllIDs returns just the media_file IDs for the same row set as GetAll.
GetAllIDs(options ...QueryOptions) ([]string, error)
// GetAlbumIDsByFolder returns the distinct IDs of albums with non-missing tracks in the given
// folders or their direct children.
GetAlbumIDsByFolder(lib Library, folderIDs ...string) ([]string, error)

6
model/mediafile_test.go
View file

@ -218,11 +218,11 @@ var _ = Describe("MediaFiles", func() {
{Tags: Tags{"genre": []string{"Alternative", "Rock"}}},
}
})
It("sets the correct Genre, sorted by frequency, then alphabetically", func() {
It("sets the correct Genre, sorted by frequency, then by order of appearance", func() {
album := mfs.ToAlbum()
Expect(album.Tags).To(HaveLen(2))
Expect(album.Tags).To(HaveKeyWithValue(TagGenre, []string{"Rock", "Alternative", "Punk"}))
Expect(album.Tags).To(HaveKeyWithValue(TagMood, []string{"Chill", "Happy"}))
Expect(album.Tags).To(HaveKeyWithValue(TagGenre, []string{"Rock", "Punk", "Alternative"}))
Expect(album.Tags).To(HaveKeyWithValue(TagMood, []string{"Happy", "Chill"}))
})
})
When("we have tags with mismatching case", func() {

14
model/metadata/metadata.go
View file

@ -9,6 +9,7 @@ import (
"strconv"
"strings"
"time"
"unicode/utf8"
"github.com/google/uuid"
"github.com/navidrome/navidrome/consts"
@ -366,6 +367,14 @@ func sanitize(filePath string, tagName model.TagName, tag model.TagConf, value s
if len(value) > maxLength {
log.Trace("Truncated tag value", "tag", tagName, "value", value, "length", len(value), "maxLength", maxLength)
value = value[:maxLength]
// Drop the partial rune the cut may have left: at most 3 trailing bytes,
// so a pre-existing invalid run elsewhere is never consumed.
for range 3 {
if r, size := utf8.DecodeLastRuneInString(value); r != utf8.RuneError || size != 1 {
break
}
value = value[:len(value)-1]
}
}
switch tag.Type {
@ -387,11 +396,14 @@ func sanitize(filePath string, tagName model.TagName, tag model.TagConf, value s
return ""
}
case model.TagTypeUUID:
_, err := uuid.Parse(value)
u, err := uuid.Parse(value)
if err != nil {
log.Trace("Invalid UUID tag value", "tag", tagName, "value", value)
return ""
}
// Store the canonical form: uuid.Parse accepts braces, urn: prefixes and any
// two-byte wrapper, and a wrapped value would never match an exact-match query
value = u.String()
}
return value
}

Some files were not shown because too many files have changed in this diff Show more