feat(subtitles): add external provider capability probe (#618)
A deployment with no external subtitle providers configured answers
POST /api/v1/subtitles/search with 200 {"results": null} — byte-identical
to a search that ran and matched nothing. Clients had no way to tell "this
server cannot do that" from "nothing found for this file", so they offered
an in-player search entry point that could never succeed. That reached us
as a bug report against the clients for a feature that was simply not
enabled here.
Add GET /api/v1/subtitles/providers/status, following the per-subsystem
capability convention already used by /subtitles/ai/status and
/items/trailers/capability:
{"schema_version": 1, "enabled": true, "providers": ["opensubtitles"]}
Provider names are safe for any authenticated viewer — they already travel
in every SubtitleResult.provider and DownloadedSubtitle.provider. The
credentials behind them stay in the admin-only provider config.
Two details worth noting for review:
The whole /subtitles group is conditional on DB + S3 + repo, so on a
storage-less deployment the probe would 404 — leaving clients to interpret
exactly the ambiguous signal the probe exists to replace. An else branch
mounts the probe alone, answering enabled:false. Only one of the two
groups registers per boot.
The path is two segments on purpose: a bare /providers would shadow the
one-segment /{media_file_id} route, while /providers/status never competes
with it in chi.
Search and download behavior are unchanged.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -98,6 +98,62 @@ func (h *SubtitleSearchHandler) authorizeMediaFile(w http.ResponseWriter, r *htt
|
||||
return authorizeMediaFileAccess(w, r, h.FileAuthorizer, fileID)
|
||||
}
|
||||
|
||||
// subtitleProviderStatusResponse tells a client whether this deployment can
|
||||
// search external subtitle providers at all, following the per-subsystem
|
||||
// capability convention (/subtitles/ai/status, /items/trailers/capability).
|
||||
//
|
||||
// Without it a search on a server with no providers configured answers exactly
|
||||
// like a search that ran and matched nothing, so a player has no way to tell
|
||||
// "this server cannot do that" from "nothing found for this file" and ends up
|
||||
// offering an entry point that can never succeed. A client that finds enabled
|
||||
// false should disable the search action and say why rather than let the user
|
||||
// run a query that is guaranteed to come back empty.
|
||||
//
|
||||
// Provider names are safe to return to any authenticated viewer: they already
|
||||
// travel in every SubtitleResult.provider and DownloadedSubtitle.provider. The
|
||||
// credentials behind them stay in the admin-only provider config.
|
||||
type subtitleProviderStatusResponse struct {
|
||||
SchemaVersion int `json:"schema_version"`
|
||||
// Enabled reports that at least one provider is registered, so
|
||||
// POST /subtitles/search can actually reach an upstream.
|
||||
Enabled bool `json:"enabled"`
|
||||
// Providers is the registered provider identifiers, sorted. Always a
|
||||
// list — empty rather than null — so clients can iterate it unguarded.
|
||||
Providers []string `json:"providers"`
|
||||
}
|
||||
|
||||
// HandleProviderStatus reports whether external subtitle search is available
|
||||
// here, so the player can show or hide the entry point.
|
||||
// GET /api/v1/subtitles/providers/status
|
||||
//
|
||||
// It answers even when the subtitle subsystem is unwired, because enabled:false
|
||||
// is the answer in that case; the router registers a fallback so a client never
|
||||
// has to interpret a 404 on the probe itself.
|
||||
func (h *SubtitleSearchHandler) HandleProviderStatus(w http.ResponseWriter, _ *http.Request) {
|
||||
providers := []string{}
|
||||
if h != nil && h.manager != nil {
|
||||
providers = h.manager.ProviderNames()
|
||||
}
|
||||
writeJSON(w, http.StatusOK, subtitleProviderStatusResponse{
|
||||
SchemaVersion: 1,
|
||||
Enabled: len(providers) > 0,
|
||||
Providers: providers,
|
||||
})
|
||||
}
|
||||
|
||||
// WriteSubtitleProvidersDisabledStatus answers the subtitle provider capability
|
||||
// probe with a 200 {"enabled": false, "providers": []} when no subtitle handler
|
||||
// is wired, so the client gets a clean negative instead of a 404 (the 2-segment
|
||||
// /providers/status path is not shadowed by the 1-segment /{media_file_id}
|
||||
// route — they never compete in chi's router).
|
||||
func WriteSubtitleProvidersDisabledStatus(w http.ResponseWriter, _ *http.Request) {
|
||||
writeJSON(w, http.StatusOK, subtitleProviderStatusResponse{
|
||||
SchemaVersion: 1,
|
||||
Enabled: false,
|
||||
Providers: []string{},
|
||||
})
|
||||
}
|
||||
|
||||
// HandleSearch handles POST /api/v1/subtitles/search
|
||||
func (h *SubtitleSearchHandler) HandleSearch(w http.ResponseWriter, r *http.Request) {
|
||||
var req searchSubtitlesRequest
|
||||
|
||||
@@ -235,6 +235,109 @@ func TestHandleDeleteRequiresAccessToMediaFile(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// stubSubtitleProvider is a registerable no-op provider; only its name matters
|
||||
// to the capability probe.
|
||||
type stubSubtitleProvider struct {
|
||||
name string
|
||||
}
|
||||
|
||||
func (s stubSubtitleProvider) Name() string { return s.name }
|
||||
|
||||
func (s stubSubtitleProvider) Search(context.Context, subtitles.SearchRequest) ([]subtitles.SubtitleResult, error) {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (s stubSubtitleProvider) Download(context.Context, string) ([]byte, subtitles.SubtitleFormat, error) {
|
||||
return nil, subtitles.FormatSRT, nil
|
||||
}
|
||||
|
||||
func decodeProviderStatus(t *testing.T, rr *httptest.ResponseRecorder) struct {
|
||||
SchemaVersion int `json:"schema_version"`
|
||||
Enabled bool `json:"enabled"`
|
||||
Providers []string `json:"providers"`
|
||||
} {
|
||||
t.Helper()
|
||||
|
||||
var resp struct {
|
||||
SchemaVersion int `json:"schema_version"`
|
||||
Enabled bool `json:"enabled"`
|
||||
Providers []string `json:"providers"`
|
||||
}
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, body = %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("decode response: %v", err)
|
||||
}
|
||||
if resp.SchemaVersion != 1 {
|
||||
t.Fatalf("schema_version = %d, want 1", resp.SchemaVersion)
|
||||
}
|
||||
return resp
|
||||
}
|
||||
|
||||
func TestHandleProviderStatusWithoutProviders(t *testing.T) {
|
||||
repo := newMockSubtitleRepoForHandler()
|
||||
manager := subtitles.NewManager(repo, newMockS3ClientForHandler(), "test-bucket")
|
||||
handler := NewSubtitleSearchHandler(manager, repo, stubSubtitleMediaResolver{})
|
||||
|
||||
rr := httptest.NewRecorder()
|
||||
handler.HandleProviderStatus(rr, newSubtitleAuthRequest(http.MethodGet, "/subtitles/providers/status", nil))
|
||||
|
||||
resp := decodeProviderStatus(t, rr)
|
||||
if resp.Enabled {
|
||||
t.Fatal("enabled = true, want false with no providers registered")
|
||||
}
|
||||
if len(resp.Providers) != 0 {
|
||||
t.Fatalf("providers = %v, want empty", resp.Providers)
|
||||
}
|
||||
// The empty list must reach the wire as [] — a null would read to a
|
||||
// client as a missing field rather than "no providers here".
|
||||
if !bytes.Contains(rr.Body.Bytes(), []byte(`"providers":[]`)) {
|
||||
t.Fatalf("body = %s, want providers serialized as []", rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleProviderStatusWithProviders(t *testing.T) {
|
||||
repo := newMockSubtitleRepoForHandler()
|
||||
manager := subtitles.NewManager(repo, newMockS3ClientForHandler(), "test-bucket")
|
||||
manager.RegisterProvider(stubSubtitleProvider{name: "subdl"})
|
||||
manager.RegisterProvider(stubSubtitleProvider{name: "opensubtitles"})
|
||||
handler := NewSubtitleSearchHandler(manager, repo, stubSubtitleMediaResolver{})
|
||||
|
||||
rr := httptest.NewRecorder()
|
||||
handler.HandleProviderStatus(rr, newSubtitleAuthRequest(http.MethodGet, "/subtitles/providers/status", nil))
|
||||
|
||||
resp := decodeProviderStatus(t, rr)
|
||||
if !resp.Enabled {
|
||||
t.Fatal("enabled = false, want true with providers registered")
|
||||
}
|
||||
want := []string{"opensubtitles", "subdl"}
|
||||
if len(resp.Providers) != len(want) {
|
||||
t.Fatalf("providers = %v, want %v", resp.Providers, want)
|
||||
}
|
||||
for i, name := range want {
|
||||
if resp.Providers[i] != name {
|
||||
t.Fatalf("providers = %v, want %v (sorted)", resp.Providers, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteSubtitleProvidersDisabledStatus(t *testing.T) {
|
||||
rr := httptest.NewRecorder()
|
||||
WriteSubtitleProvidersDisabledStatus(rr, newSubtitleAuthRequest(http.MethodGet, "/subtitles/providers/status", nil))
|
||||
|
||||
resp := decodeProviderStatus(t, rr)
|
||||
if resp.Enabled {
|
||||
t.Fatal("enabled = true, want false from the disabled fallback")
|
||||
}
|
||||
if len(resp.Providers) != 0 {
|
||||
t.Fatalf("providers = %v, want empty", resp.Providers)
|
||||
}
|
||||
if !bytes.Contains(rr.Body.Bytes(), []byte(`"providers":[]`)) {
|
||||
t.Fatalf("body = %s, want providers serialized as []", rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
type handlerMockSubtitleRepo struct {
|
||||
subtitles map[int]*subtitles.DownloadedSubtitle
|
||||
nextID int
|
||||
|
||||
@@ -2575,6 +2575,11 @@ func NewRouter(deps Dependencies) chi.Router {
|
||||
}
|
||||
}
|
||||
r.Route("/subtitles", func(r chi.Router) {
|
||||
// Capability probe for external subtitle search. Two
|
||||
// segments on purpose: a bare /providers would shadow
|
||||
// the /{media_file_id} route below, while
|
||||
// /providers/status never competes with it in chi.
|
||||
r.Get("/providers/status", subtitleSearchHandler.HandleProviderStatus)
|
||||
r.Post("/search", subtitleSearchHandler.HandleSearch)
|
||||
r.Post("/download", subtitleSearchHandler.HandleDownload)
|
||||
r.Post("/upload", subtitleSearchHandler.HandleUpload)
|
||||
@@ -2595,6 +2600,17 @@ func NewRouter(deps Dependencies) chi.Router {
|
||||
r.Get("/{media_file_id}", subtitleSearchHandler.HandleList)
|
||||
r.Delete("/{id}", subtitleSearchHandler.HandleDelete)
|
||||
})
|
||||
} else {
|
||||
// The whole group above is conditional (it needs the DB,
|
||||
// S3 and the subtitle repo), so on a storage-less
|
||||
// deployment the capability probe would 404 — leaving a
|
||||
// client to interpret the same ambiguous status the probe
|
||||
// exists to replace. Mount the probe alone, answering
|
||||
// enabled:false, so feature detection always gets a real
|
||||
// answer.
|
||||
r.Route("/subtitles", func(r chi.Router) {
|
||||
r.Get("/providers/status", handlers.WriteSubtitleProvidersDisabledStatus)
|
||||
})
|
||||
}
|
||||
|
||||
// Playback routes.
|
||||
|
||||
@@ -52,6 +52,24 @@ func (m *Manager) RemoveProvider(name string) {
|
||||
delete(m.providers, name)
|
||||
}
|
||||
|
||||
// ProviderNames returns the names of every currently registered provider,
|
||||
// sorted for a stable response. Empty when none are configured.
|
||||
//
|
||||
// The slice is always non-nil so callers can hand it straight to a JSON
|
||||
// response without it marshalling as null — clients feature-detecting subtitle
|
||||
// search read an empty list as "no providers here", not as a missing field.
|
||||
func (m *Manager) ProviderNames() []string {
|
||||
m.mu.RLock()
|
||||
names := make([]string, 0, len(m.providers))
|
||||
for name := range m.providers {
|
||||
names = append(names, name)
|
||||
}
|
||||
m.mu.RUnlock()
|
||||
|
||||
sort.Strings(names)
|
||||
return names
|
||||
}
|
||||
|
||||
// Search fans out to all registered providers concurrently.
|
||||
func (m *Manager) Search(ctx context.Context, req SearchRequest) (*SearchResponse, error) {
|
||||
m.mu.RLock()
|
||||
|
||||
@@ -116,6 +116,53 @@ func (m *mockS3Client) DeleteObject(_ context.Context, _, key string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
type stubProvider struct {
|
||||
name string
|
||||
}
|
||||
|
||||
func (s stubProvider) Name() string { return s.name }
|
||||
|
||||
func (s stubProvider) Search(context.Context, SearchRequest) ([]SubtitleResult, error) {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (s stubProvider) Download(context.Context, string) ([]byte, SubtitleFormat, error) {
|
||||
return nil, FormatSRT, nil
|
||||
}
|
||||
|
||||
func TestManagerProviderNames(t *testing.T) {
|
||||
manager := NewManager(newMockSubtitleRepo(), newMockS3Client(), "test-bucket")
|
||||
|
||||
names := manager.ProviderNames()
|
||||
if names == nil {
|
||||
t.Fatal("ProviderNames() = nil, want non-nil empty slice")
|
||||
}
|
||||
if len(names) != 0 {
|
||||
t.Fatalf("ProviderNames() = %v, want empty", names)
|
||||
}
|
||||
|
||||
manager.RegisterProvider(stubProvider{name: "subdl"})
|
||||
manager.RegisterProvider(stubProvider{name: "opensubtitles"})
|
||||
manager.RegisterProvider(stubProvider{name: "subsource"})
|
||||
|
||||
want := []string{"opensubtitles", "subdl", "subsource"}
|
||||
names = manager.ProviderNames()
|
||||
if len(names) != len(want) {
|
||||
t.Fatalf("ProviderNames() = %v, want %v", names, want)
|
||||
}
|
||||
for i, name := range want {
|
||||
if names[i] != name {
|
||||
t.Fatalf("ProviderNames() = %v, want %v (sorted)", names, want)
|
||||
}
|
||||
}
|
||||
|
||||
manager.RemoveProvider("subdl")
|
||||
names = manager.ProviderNames()
|
||||
if len(names) != 2 || names[0] != "opensubtitles" || names[1] != "subsource" {
|
||||
t.Fatalf("ProviderNames() after removal = %v, want [opensubtitles subsource]", names)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManagerUploadStoresSubtitle(t *testing.T) {
|
||||
repo := newMockSubtitleRepo()
|
||||
s3 := newMockS3Client()
|
||||
|
||||
Reference in New Issue
Block a user