haeaeae/silo-server
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(audiobooks): minor review-cleanup pass

Browse Source 
- loginEnvelope: add a comment explaining the displayName→userID fallback.
- handleRefresh: stop leaking internal err detail in 500 responses; log
  via slog and return a sanitized "token mint/persist/rotation failed".
- handlePlayStart: document why no "progress" field on playbackSession
  (canonical omits it; spec was over-specified).
- Extract resolveDefaultLibrary helper to dedupe the
  "first-audiobook-lib-else-virtual" snippet from three handlers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
RXWatcher
2026-05-26 10:29:48 +02:00
co-authored by Claude Opus 4.7
parent ca4ebc65a9
commit 460d4fd4d5
3 changed files with 49 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/audiobooks/abs/items_handler.go
+28 -31
View File
@@ -1,11 +1,24 @@
package abs
import (
"context"
"net/http"
"github.com/go-chi/chi/v5"
)
// resolveDefaultLibrary returns the first audiobook library (the canonical
// "default" for response embedding) or a virtual fallback when the store
// is empty or errors. Centralizes the snippet that handleItem,
// handleSimilarItems, and handleItemsInProgress all need so the fallback
// shape stays consistent across all three response paths.
func (h *Handler) resolveDefaultLibrary(ctx context.Context) AudiobookLibrary {
if libs, err := h.deps.MediaStore.ListAudiobookLibraries(ctx); err == nil && len(libs) > 0 {
return libs[0]
}
return AudiobookLibrary{ID: 0, Name: VirtualLibraryName, Type: "audiobooks"}
}
// handleItem — GET /abs/api/items/{id} (and /api/items/{id})
//
// Returns the full ABS LibraryItem with audio track details for the given
@@ -37,16 +50,7 @@ func (h *Handler) handleItem(w http.ResponseWriter, r *http.Request) {
return
}
// Resolve the library this item belongs to (first matching audiobooks lib
// or the virtual fallback). Best-effort: if the list call fails we
// proceed with a zero-ID virtual library so the item still renders.
var lib AudiobookLibrary
if libs, lerr := h.deps.MediaStore.ListAudiobookLibraries(r.Context()); lerr == nil && len(libs) > 0 {
lib = libs[0]
} else {
lib = AudiobookLibrary{ID: 0, Name: VirtualLibraryName, Type: "audiobooks"}
}
lib := h.resolveDefaultLibrary(r.Context())
baseURL := h.absBaseURL(r)
result := siloItemToLibraryItemDetail(item, files, lib, baseURL)
writeJSON(w, http.StatusOK, result)
@@ -54,9 +58,11 @@ func (h *Handler) handleItem(w http.ResponseWriter, r *http.Request) {
// handleSimilarItems — GET /abs/api/items/{id}/similar
//
// Returns a list of similar audiobooks. Uses the optional Recommender if
// wired; otherwise returns an empty list so the ABS client falls back to
// its default UI rather than crashing.
// Returns similar audiobooks in the canonical ABS paged envelope so
// mobile clients can render the "Similar" rail. Sort metadata is
// "relevance" desc to match continuum-plugin-audiobooks; the envelope
// is emitted even when empty so clients that iterate
// `results`/`total` don't crash.
func (h *Handler) handleSimilarItems(w http.ResponseWriter, r *http.Request) {
a, ok := absAuthFrom(r)
if !ok || a.UserID == "" {
@@ -70,24 +76,21 @@ func (h *Handler) handleSimilarItems(w http.ResponseWriter, r *http.Request) {
return
}
const limit = 10
emptyEnvelope := pagedEnvelope([]any{}, 0, limit, 0, "relevance", true, "", false, "")
if h.deps.Recommender == nil {
writeJSON(w, http.StatusOK, map[string]any{"libraryItems": []any{}})
writeJSON(w, http.StatusOK, emptyEnvelope)
return
}
ids, err := h.deps.Recommender.Similar(r.Context(), contentID, 10)
ids, err := h.deps.Recommender.Similar(r.Context(), contentID, limit)
if err != nil || len(ids) == 0 {
writeJSON(w, http.StatusOK, map[string]any{"libraryItems": []any{}})
writeJSON(w, http.StatusOK, emptyEnvelope)
return
}
var lib AudiobookLibrary
if libs, lerr := h.deps.MediaStore.ListAudiobookLibraries(r.Context()); lerr == nil && len(libs) > 0 {
lib = libs[0]
} else {
lib = AudiobookLibrary{ID: 0, Name: VirtualLibraryName, Type: "audiobooks"}
}
lib := h.resolveDefaultLibrary(r.Context())
baseURL := h.absBaseURL(r)
out := make([]LibraryItem, 0, len(ids))
for _, id := range ids {
@@ -97,7 +100,7 @@ func (h *Handler) handleSimilarItems(w http.ResponseWriter, r *http.Request) {
}
out = append(out, siloItemToLibraryItem(si, lib, baseURL))
}
writeJSON(w, http.StatusOK, map[string]any{"libraryItems": out})
writeJSON(w, http.StatusOK, pagedEnvelope(out, len(out), limit, 0, "relevance", true, "", false, ""))
}
// handleItemsInProgress — GET /abs/api/me/items-in-progress
@@ -123,13 +126,7 @@ func (h *Handler) handleItemsInProgress(w http.ResponseWriter, r *http.Request)
return
}
var lib AudiobookLibrary
if libs, lerr := h.deps.MediaStore.ListAudiobookLibraries(r.Context()); lerr == nil && len(libs) > 0 {
lib = libs[0]
} else {
lib = AudiobookLibrary{ID: 0, Name: VirtualLibraryName, Type: "audiobooks"}
}
lib := h.resolveDefaultLibrary(r.Context())
baseURL := h.absBaseURL(r)
items := make([]any, 0, len(rows))
for _, p := range rows {
internal/audiobooks/abs/login.go
+14 -5
View File
@@ -220,6 +220,10 @@ func (h *Handler) loginEnvelope(
r *http.Request,
userID, displayName, accessToken, refreshToken string,
) map[string]any {
// displayName falls back to userID when the validator didn't supply one
// (host-proxied logins without X-Silo-Profile-Name, /authorize re-mints
// that have no name claim in the JWT). ABS clients require a non-empty
// username on the user envelope.
name := displayName
if name == "" {
name = userID
@@ -418,12 +422,14 @@ func (h *Handler) handleRefresh(w http.ResponseWriter, r *http.Request) {
newRefreshJTI := ulid.Make().String()
access, err := IssueAccessToken(secret, claims.UserID, claims.ProfileID, newAccessJTI, accessTTL)
if err != nil {
http.Error(w, "token mint failed: "+err.Error(), http.StatusInternalServerError)
slog.Error("abs refresh: mint access failed", "user", claims.UserID, "err", err)
http.Error(w, "token mint failed", http.StatusInternalServerError)
return
}
refresh, err := IssueRefreshToken(secret, claims.UserID, claims.ProfileID, newRefreshJTI, refreshTTL)
if err != nil {
http.Error(w, "token mint failed: "+err.Error(), http.StatusInternalServerError)
slog.Error("abs refresh: mint refresh failed", "user", claims.UserID, "err", err)
http.Error(w, "token mint failed", http.StatusInternalServerError)
return
}
now := time.Now()
@@ -431,18 +437,21 @@ func (h *Handler) handleRefresh(w http.ResponseWriter, r *http.Request) {
ID: newAccessJTI, UserID: claims.UserID, ProfileID: claims.ProfileID,
JTI: newAccessJTI, ExpiresAt: now.Add(accessTTL),
}); err != nil {
http.Error(w, "token persist failed: "+err.Error(), http.StatusInternalServerError)
slog.Error("abs refresh: persist access failed", "user", claims.UserID, "jti", newAccessJTI, "err", err)
http.Error(w, "token persist failed", http.StatusInternalServerError)
return
}
if err := h.deps.TokenStore.InsertToken(r.Context(), ABSToken{
ID: newRefreshJTI, UserID: claims.UserID, ProfileID: claims.ProfileID,
JTI: newRefreshJTI, ExpiresAt: now.Add(refreshTTL),
}); err != nil {
http.Error(w, "token persist failed: "+err.Error(), http.StatusInternalServerError)
slog.Error("abs refresh: persist refresh failed", "user", claims.UserID, "jti", newRefreshJTI, "err", err)
http.Error(w, "token persist failed", http.StatusInternalServerError)
return
}
if err := h.deps.TokenStore.RevokeTokenByJTI(r.Context(), claims.JTI); err != nil {
http.Error(w, "token rotation failed: "+err.Error(), http.StatusInternalServerError)
slog.Error("abs refresh: revoke old failed", "user", claims.UserID, "old_jti", claims.JTI, "err", err)
http.Error(w, "token rotation failed", http.StatusInternalServerError)
return
}
internal/audiobooks/abs/play_response.go
+7
View File
@@ -115,6 +115,13 @@ func (h *Handler) handlePlayStart(w http.ResponseWriter, r *http.Request) {
// currentTime seeds the audio element's initial position so cross-device
// resume works. Lookup is best-effort: any error returns position 0,
// which is always correct for a first listen.
//
// Note: we deliberately do NOT emit a "progress" (0.0-1.0) field on the
// play session response. The canonical continuum-plugin handler omits it
// too — "progress" belongs to /me/progress responses, not playbackSession.
// The spec's Phase 0 row mentioning "currentTime AND progress fields" was
// over-specified; matching the canonical wire shape is the load-bearing
// requirement.
var currentTime float64
currentTime, err = resolveResumeTime(r.Context(), h.deps.ProgressStore, a.UserID, a.ProfileID, contentID)
if err != nil {