diff --git a/README.md b/README.md index 41b3faa..cb11115 100644 --- a/README.md +++ b/README.md @@ -69,8 +69,11 @@ Intro and Skip Credits buttons appear without any local analysis. which is the confusing part: the rows are there, Plex serves them, and the client still shows no button. See [docs/troubleshooting.md](docs/troubleshooting.md) if you have a Pass and the button is still missing. -- **TMDb metadata is recommended** for accuracy. IMDb and Tvdb ids work as a - fallback but are less exact for TV episodes. +- **A TVDb id is the best metadata match.** It is the identity TheIntroDB stores + its timings under, and the numbering Plex assigns by default, so both ends + mean the same episode. TMDb and IMDb ids are used when an item has no TVDb one; + the season and episode are then read in that provider's numbering, which is not + always the numbering Plex used. The database is found automatically: the tool searches the platform's own locations (macOS, Windows, Linux packages, snaps, containers and FreeBSD) and @@ -448,7 +451,7 @@ cannot be told apart from a speed-up, so they are left alone. **No markers appear.** Markers are written by a run, not on playback. Check `plex-sync preview` to see what a run intends to do, and that your items have a -TMDb or IMDb id (`plex-sync library`). +TVDb, TMDb or IMDb id (`plex-sync library`). **"cannot confirm whether Plex is running".** Plex's process was not visible and its HTTP endpoint did not answer. Fix `plex.url`, or stop Plex and pass diff --git a/docs/architecture.md b/docs/architecture.md index 82bb486..2deadb5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -100,7 +100,9 @@ every row is community-verified. The alternate sources exist to cover what TheIntroDB does not have yet. They can never override it for a segment type it already answered: -1. **theintrodb** - the API, keyed by TMDb (IMDb and Tvdb as fallback). +1. **theintrodb** - the API, asked with the item's TVDb id where it has one + (TMDb, then IMDb after that). That is the API's own precedence, and the TVDb + numbering is the one Plex assigns by default. 2. **chapters** - chapter names Plex already extracted into the file's metadata ("Intro", "Opening Credits", "End Credits", "Recap"). Free: no lookups, no media reads, exact for the file on disk. Off by default. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 20a0943..bc6fe37 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -89,9 +89,10 @@ are removed by hand. ## "no-provider-id" -The item has no TMDb, IMDb or Tvdb id, so there is nothing to look it up by. -Set your Plex library's metadata agent to TMDb and refresh the item. TMDb is -also the most accurate source for episodes, which is why it is preferred. +The item has no TVDb, TMDb or IMDb id, so there is nothing to look it up by. +Set your Plex library's metadata agent to the Plex TV Series (TVDB) agent and +refresh the item. A TVDb id is what TheIntroDB is asked with: it is the identity +the timings are stored under, and the numbering Plex assigns by default. ## "cannot confirm whether Plex is running" @@ -217,9 +218,11 @@ which puts them back. made with the file length (`plex-sync preview` reports the source of each marker), and that the file has not been replaced since. - **Minutes out**: usually a PAL speed-up, or an episode-numbering mismatch. A - show that Plex numbers differently from TMDb (common with anime and - absolute-order shows) can receive another episode's timings. Those items are - not detectable automatically; exclude them with `--show` filters. + show whose episodes Plex numbers in a different display order (DVD, absolute, + or one Plex has matched to TMDb) can receive another episode's timings. + Lookups are made with the show's TVDb id, which is the numbering Plex uses by + default, so this is limited to shows whose order has been changed from the + default; `--show` filters exclude them. - **Warning about more than one TMDb id**: the item is ambiguous, so it is skipped rather than guessed at. Fix the metadata in Plex. diff --git a/internal/cli/commands.go b/internal/cli/commands.go index 2c3ff84..6235cba 100644 --- a/internal/cli/commands.go +++ b/internal/cli/commands.go @@ -106,7 +106,7 @@ item never gets markers.`), } fmt.Fprintf(out, "\n%d item(s): %d with a provider id, %d without\n", len(items), matched, unmatched) if unmatched > 0 { - fmt.Fprintln(out, "Items without a TMDb, IMDb or Tvdb id cannot be looked up. Set your agent to TMDb.") + fmt.Fprintln(out, "Items without a TVDb, TMDb or IMDb id cannot be looked up. Set your library's agent to the Plex TV Series (TVDB) agent.") } return nil }, diff --git a/internal/config/config.go b/internal/config/config.go index 5ce9530..bf0ac89 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -209,7 +209,7 @@ func Default() *Config { TimeoutS: 20, }, TheIntroDB: TheIntroDB{ - BaseURL: "https://api.theintrodb.org/v3", + BaseURL: "https://api.theintrodb.org/v4", TimeoutS: 20, DailyBudget: 1000, MaxPerWindow: 25, diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 167dd77..696ed94 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -33,6 +33,24 @@ func TestDefaultsAreUsable(t *testing.T) { } } +// The default base URL has to name a surface that still serves /user/stats. +// +// /v2 and /v3 are frozen at GET /media and answer nothing else, so a default +// pointing at either turns ValidateKey -- and the stats view behind it -- into +// "theintrodb: user stats returned HTTP 404: Cannot GET /v3/user/stats", which +// reads as a status failure rather than a verdict about the key. +func TestDefaultBaseURLIsNotAFrozenSurface(t *testing.T) { + got := Default().TheIntroDB.BaseURL + if got != "https://api.theintrodb.org/v4" { + t.Errorf("default base URL = %q, want the v4 surface", got) + } + for _, frozen := range []string{"/v2", "/v3"} { + if strings.HasSuffix(strings.TrimRight(got, "/"), frozen) { + t.Errorf("the default names %s, which carries GET /media only", frozen) + } + } +} + // The pacing floor is not cosmetic: the server ceiling is 30 requests per 10 // seconds, and pacing at exactly the ceiling produces 429s as soon as there is // any jitter. diff --git a/internal/model/model.go b/internal/model/model.go index fb7a2e8..8d100d9 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -8,6 +8,7 @@ package model import ( "fmt" "sort" + "strconv" "strings" ) @@ -71,7 +72,9 @@ const ( ) // ExternalIDs are the provider ids Plex matched for an item. Any of them may be -// missing; lookups prefer TMDb, which TheIntroDB keys its cut-aware data on. +// missing; lookups prefer TVDb, which is the identity TheIntroDB stores and +// answers by, and the numbering Plex uses for an episode unless the show has +// been set to a different display order. type ExternalIDs struct { TMDB *int `json:"tmdb,omitempty"` IMDb *string `json:"imdb,omitempty"` @@ -81,22 +84,59 @@ type ExternalIDs struct { // Any reports whether at least one id is present. func (e ExternalIDs) Any() bool { return e.TMDB != nil || e.IMDb != nil || e.TVDB != nil } +// ProviderID is one provider id, and the query parameter TheIntroDB is asked +// with it under. +type ProviderID struct { + // Provider is "tvdb", "tmdb" or "imdb"; the cache key records it. + Provider string + // Param is the API query parameter for this provider: "tvdb_id" and so on. + Param string + // Value is the id itself, as the request carries it. + Value string +} + +// IDs returns the item's ids in the order they are asked in: TVDb, then TMDb, +// then IMDb. +// +// That is the API's own precedence (`resolveMediaID`), so a lookup names the +// identity the answer is keyed on whenever the item has one. The numbering +// matters as much as the identity: an episode's season and episode are read in +// the numbering of the id that was sent, and only a TVDb id is read as the TVDb +// numbers, which is what Plex itself uses by default. Sent as tmdb_id, the same +// numbers are taken to be TMDb's, and an episode whose place in TMDb's list +// differs is answered with another episode's timings -- or with nothing at all. +// +// Both the cache key (LookupKey) and the request (tidb.buildQuery) are built +// from this one list, so they cannot drift apart. +func (e ExternalIDs) IDs() []ProviderID { + out := make([]ProviderID, 0, 3) + if e.TVDB != nil { + out = append(out, ProviderID{"tvdb", "tvdb_id", strconv.Itoa(*e.TVDB)}) + } + if e.TMDB != nil { + out = append(out, ProviderID{"tmdb", "tmdb_id", strconv.Itoa(*e.TMDB)}) + } + if e.IMDb != nil && *e.IMDb != "" { + out = append(out, ProviderID{"imdb", "imdb_id", *e.IMDb}) + } + return out +} + // LookupKey returns the cache key for this item, and false when no id is known. // // The key encodes provider, id and episode coordinates, so a cached miss for -// one episode can never shadow another. +// one episode can never shadow another. The provider is the first of IDs(), +// which is the provider buildQuery asks first: the key has to describe the +// request that was made, or an entry answered under one id would be served to a +// lookup asking under another. Changing the order renames the keys of items +// whose id set spans more than one provider, so those are asked again after an +// upgrade. func (e ExternalIDs) LookupKey(season, episode *int) (string, bool) { - var provider, value string - switch { - case e.TMDB != nil: - provider, value = "tmdb", fmt.Sprint(*e.TMDB) - case e.IMDb != nil && *e.IMDb != "": - provider, value = "imdb", *e.IMDb - case e.TVDB != nil: - provider, value = "tvdb", fmt.Sprint(*e.TVDB) - default: + ids := e.IDs() + if len(ids) == 0 { return "", false } + provider, value := ids[0].Provider, ids[0].Value if season == nil || episode == nil { return provider + ":" + value + ":movie", true } diff --git a/internal/model/model_test.go b/internal/model/model_test.go index 7e8ce87..44388f4 100644 --- a/internal/model/model_test.go +++ b/internal/model/model_test.go @@ -56,6 +56,66 @@ func TestSelectingAndDeselecting(t *testing.T) { } } +// The key names the id the request is built from. TVDb comes first because it +// is the identity TheIntroDB stores its data under and the numbering Plex +// assigns by default; TMDb and IMDb only answer for items without one. +func TestLookupKeyUsesTVDbFirst(t *testing.T) { + tmdb, tvdb := 615, 73871 + imdb := "tt0111161" + season, episode := 7, 1 + + cases := []struct { + name string + ids ExternalIDs + want string + }{ + {"all three", ExternalIDs{TMDB: &tmdb, IMDb: &imdb, TVDB: &tvdb}, "tvdb:73871:7:1"}, + {"no tvdb", ExternalIDs{TMDB: &tmdb, IMDb: &imdb}, "tmdb:615:7:1"}, + {"imdb only", ExternalIDs{IMDb: &imdb}, "imdb:tt0111161:7:1"}, + } + for _, tc := range cases { + got, ok := tc.ids.LookupKey(&season, &episode) + if !ok || got != tc.want { + t.Errorf("%s: LookupKey = %q, %v, want %q", tc.name, got, ok, tc.want) + } + } + + if got, ok := (ExternalIDs{TVDB: &tvdb}).LookupKey(nil, nil); !ok || got != "tvdb:73871:movie" { + t.Errorf("movie LookupKey = %q, %v, want tvdb:73871:movie", got, ok) + } + if _, ok := (ExternalIDs{}).LookupKey(&season, &episode); ok { + t.Error("an item with no ids produced a key") + } +} + +// IDs is what tidb.buildQuery turns into the request, so its order and the +// parameter names it carries are a contract, not an implementation detail. +func TestIDsInRequestOrder(t *testing.T) { + tmdb, tvdb := 615, 73871 + imdb := "tt0111161" + + got := ExternalIDs{TMDB: &tmdb, IMDb: &imdb, TVDB: &tvdb}.IDs() + want := []ProviderID{ + {"tvdb", "tvdb_id", "73871"}, + {"tmdb", "tmdb_id", "615"}, + {"imdb", "imdb_id", "tt0111161"}, + } + if len(got) != len(want) { + t.Fatalf("IDs() = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("IDs()[%d] = %v, want %v", i, got[i], want[i]) + } + } + + // An empty string is not an id: Plex writes those for an unmatched item. + empty := "" + if ids := (ExternalIDs{IMDb: &empty}).IDs(); len(ids) != 0 { + t.Errorf("an empty IMDb id produced %v", ids) + } +} + func TestMarkingForRescan(t *testing.T) { selection := &Selection{} diff --git a/internal/tidb/client_test.go b/internal/tidb/client_test.go index 3d36e4c..13542b3 100644 --- a/internal/tidb/client_test.go +++ b/internal/tidb/client_test.go @@ -997,6 +997,42 @@ func TestLookupFallsBackToIMDbAndTVDB(t *testing.T) { func ptr[T any](v T) *T { return &v } +// A TVDb id is asked for first even when the item also carries TMDb, because it +// is the identity the answer is keyed on and the numbering Plex uses by default. +// The season and episode travel with the id and are read in its numbering, so +// the two cannot be separated. +func TestLookupAsksByTVDbFirst(t *testing.T) { + h := newHarness(t, func(call int, w http.ResponseWriter, r *http.Request) { + js(w, 200, sampleBody) + }, nil) + + tmdb, tvdb, season, episode := 615, 73871, 7, 1 + item := model.LibraryItem{ + RatingKey: 1, Kind: model.KindEpisode, Title: "Space Pilot 3000", + ShowTitle: "Futurama", Season: &season, Episode: &episode, + IDs: model.ExternalIDs{TMDB: &tmdb, TVDB: &tvdb}, + } + + _, res, err := h.client.Lookup(context.Background(), item) + if err != nil { + t.Fatalf("Lookup: %v", err) + } + if res.Key != "tvdb:73871:7:1" { + t.Errorf("Key = %q, want tvdb:73871:7:1", res.Key) + } + + q := h.server.Last().Query + if got := q.Get("tvdb_id"); got != "73871" { + t.Errorf("tvdb_id = %q, want 73871 (query %v)", got, q) + } + if got := q.Get("tmdb_id"); got != "" { + t.Errorf("query carried tmdb_id = %q as well: one identity per request", got) + } + if q.Get("season") != "7" || q.Get("episode") != "1" { + t.Errorf("season/episode = %q/%q, want 7/1", q.Get("season"), q.Get("episode")) + } +} + func TestLookupWithoutAnIDNeverCallsTheAPI(t *testing.T) { h := newHarness(t, func(call int, w http.ResponseWriter, r *http.Request) { t.Error("the API was called for an item with no ids") @@ -1163,6 +1199,37 @@ func TestUserStats401(t *testing.T) { } } +// A base URL that does not serve /user/stats answers 404. That is not a verdict +// about the key, so it must not read as one and must not be terminal -- the v3 +// default produced exactly this, and the message named a status rather than the +// key, which was fine. +func TestUserStatsOnAFrozenSurfaceIsNotAKeyVerdict(t *testing.T) { + h := newHarness(t, func(call int, w http.ResponseWriter, r *http.Request) { + http.Error(w, "Cannot GET "+r.URL.Path, 404) + }, func(cfg *config.TheIntroDB) { cfg.APIKey = "good-key" }) + + _, err := h.client.UserStats(context.Background()) + if err == nil { + t.Fatal("a 404 on /user/stats should be an error") + } + var apiErr *Error + if !errors.As(err, &apiErr) { + t.Fatalf("error = %v, want an *Error", err) + } + if apiErr.Kind != KindStatus || apiErr.Status != 404 { + t.Errorf("kind = %q status = %d, want a %q 404", apiErr.Kind, apiErr.Status, KindStatus) + } + if apiErr.IsTerminal() { + t.Error("a 404 on /user/stats must not be terminal: the surface is the problem, not the key") + } + if strings.Contains(err.Error(), "rejected the API key") { + t.Errorf("a 404 must not be reported as a rejected key: %q", err.Error()) + } + if got := h.server.Last().Path; got != statsPath { + t.Errorf("path = %q, want %q", got, statsPath) + } +} + // --------------------------------------------------------------------------- // Usage accounting // --------------------------------------------------------------------------- diff --git a/internal/tidb/lookup.go b/internal/tidb/lookup.go index 96cb3b8..6318e0b 100644 --- a/internal/tidb/lookup.go +++ b/internal/tidb/lookup.go @@ -110,7 +110,7 @@ func (c *Client) lookup(ctx context.Context, item model.LibraryItem, force bool) }, &Error{ Kind: KindNoIDs, Message: fmt.Sprintf( - "theintrodb: %s has no TMDb, IMDb or TVDb id, so it cannot be looked up", + "theintrodb: %s has no TVDb, TMDb or IMDb id, so it cannot be looked up", item.Label()), } } @@ -263,17 +263,19 @@ func (c *Client) failure(key string, err error, started time.Time) LookupResult // // The duration is the real file length when it is known, which is what makes // the answer cut-aware; without it the timings may belong to a different cut. -// Season and episode are sent only for episodes. The provider order matches -// model.ExternalIDs.LookupKey, so the cache key and the query always agree. +// Season and episode are sent only for episodes. +// +// The id is the first of model.ExternalIDs.IDs(), the same list the cache key +// is taken from, so the key and the query cannot name different ids. TVDb goes +// first: it is the identity TheIntroDB keys its data on, and the numbering Plex +// uses by default, so an episode's season and episode mean the same thing to +// both ends. Asking as tmdb_id instead sends Plex's TVDb numbering as if it +// were TMDb's, which on a show whose orders differ returns another episode's +// timings or a 404. func (c *Client) buildQuery(item model.LibraryItem) url.Values { q := url.Values{} - switch { - case item.IDs.TMDB != nil: - q.Set("tmdb_id", strconv.Itoa(*item.IDs.TMDB)) - case item.IDs.IMDb != nil && *item.IDs.IMDb != "": - q.Set("imdb_id", *item.IDs.IMDb) - case item.IDs.TVDB != nil: - q.Set("tvdb_id", strconv.Itoa(*item.IDs.TVDB)) + if ids := item.IDs.IDs(); len(ids) > 0 { + q.Set(ids[0].Param, ids[0].Value) } if !item.IsMovie() && item.Season != nil && item.Episode != nil { q.Set("season", strconv.Itoa(*item.Season)) diff --git a/internal/tui/model.go b/internal/tui/model.go index 348693e..c7b8832 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -663,7 +663,7 @@ func (m *Model) togglePlanRescan() { item := work[m.planCursor] key, ok := item.Item.LookupKey() if !ok { - m.setError(fmt.Errorf("%s has no TMDb, IMDb or TVDb id, so it cannot be looked up", + m.setError(fmt.Errorf("%s has no TVDb, TMDb or IMDb id, so it cannot be looked up", item.Item.Label())) return }