diff --git a/internal/services/image_processor/processor.go b/internal/services/image_processor/processor.go index 33ff245..f269123 100644 --- a/internal/services/image_processor/processor.go +++ b/internal/services/image_processor/processor.go @@ -74,6 +74,14 @@ func (p *ImageProcessorImpl[DM]) Process(ctx context.Context, data event.Event[D return data, nil } + // The larger copy where the source has one. Decided before the skip + // rule, so an object stored from the small copy is replaced once by the + // large one (its recorded source length differs) and then left alone. + if large := preferredSource(ctx, dataPayload.URL); large != dataPayload.URL { + log.Info("using the larger source", zap.String("url", large)) + dataPayload.URL = large + } + // A message arrives on every anime update, not only when the artwork // changes, so the image we already hold is usually the one being offered // again. Comparing the stored size against the source's Content-Length diff --git a/internal/services/image_processor/processor_test.go b/internal/services/image_processor/processor_test.go index b710ef6..12ccc23 100644 --- a/internal/services/image_processor/processor_test.go +++ b/internal/services/image_processor/processor_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "strconv" + "strings" "sync" "testing" @@ -45,13 +46,16 @@ func (m *memStore) PutObject(_ context.Context, data []byte, path, ct string, me return nil } func (m *memStore) Get(_ context.Context, path string) ([]byte, error) { return m.objs[path].data, nil } -func (m *memStore) Delete(_ context.Context, path string) error { delete(m.objs, path); return nil } +func (m *memStore) Delete(_ context.Context, path string) error { delete(m.objs, path); return nil } func (m *memStore) List(context.Context, string, bool) <-chan storage.Entry { ch := make(chan storage.Entry) close(ch) return ch } -func (m *memStore) Copy(_ context.Context, src, dst string) error { m.objs[dst] = m.objs[src]; return nil } +func (m *memStore) Copy(_ context.Context, src, dst string) error { + m.objs[dst] = m.objs[src] + return nil +} func (m *memStore) Exists(_ context.Context, path string) (bool, error) { _, ok := m.objs[path] return ok, nil @@ -191,3 +195,76 @@ func TestAFailedAnnouncementDoesNotFailTheStore(t *testing.T) { t.Fatal("object not stored") } } + +func TestLargerVariantNamesTheLCopyOfAMyAnimeListImage(t *testing.T) { + cases := map[string]string{ + "https://cdn.myanimelist.net/images/anime/1668/108792.jpg": "https://cdn.myanimelist.net/images/anime/1668/108792l.jpg", + "https://cdn.myanimelist.net/images/characters/9/310307.jpg": "https://cdn.myanimelist.net/images/characters/9/310307l.jpg", + "https://cdn.myanimelist.net/images/anime/1668/108792l.jpg": "", // already the large copy + "https://cdn.myanimelist.net/images/anime/1668/108792.jpg?s=1": "https://cdn.myanimelist.net/images/anime/1668/108792l.jpg?s=1", + "https://artworks.thetvdb.com/banners/posters/12345.jpg": "", // not MyAnimeList + "https://cdn.myanimelist.net/images/questionmark_23.gif": "", + "not a url": "", + } + for in, want := range cases { + if got := largerVariant(in); got != want { + t.Errorf("%s -> %q, want %q", in, got, want) + } + } +} + +// The large copy is stored when it is there, and its length is what gets +// recorded, so the next event compares against the right source. +func TestTheLargerCopyIsStoredWhenItExists(t *testing.T) { + large := append(jpeg, make([]byte, 400)...) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body := jpeg + if strings.HasSuffix(r.URL.Path, "l.jpg") { + body = large + } + w.Header().Set("Content-Length", strconv.Itoa(len(body))) + if r.Method == http.MethodGet { + w.Write(body) + } + })) + defer srv.Close() + host := strings.TrimPrefix(srv.URL, "http://") + malHosts[host] = true + defer delete(malHosts, host) + + store := newMemStore() + p := NewImageProcessor[string](store, nil) + run(t, p, srv.URL+"/images/anime/1/2.jpg") + + o := store.objs["/id-1"] + if len(o.data) != len(large) { + t.Fatalf("stored %d bytes, want the large copy (%d)", len(o.data), len(large)) + } + if o.meta[storage.MetaSourceLength] != strconv.Itoa(len(large)) || !strings.HasSuffix(o.meta[storage.MetaSourceURL], "/2l.jpg") { + t.Errorf("meta %v", o.meta) + } +} + +func TestFallsBackToTheSmallCopyWhenThereIsNoLargeOne(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "l.jpg") { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Length", strconv.Itoa(len(jpeg))) + if r.Method == http.MethodGet { + w.Write(jpeg) + } + })) + defer srv.Close() + host := strings.TrimPrefix(srv.URL, "http://") + malHosts[host] = true + defer delete(malHosts, host) + + store := newMemStore() + run(t, NewImageProcessor[string](store, nil), srv.URL+"/images/anime/1/2.jpg") + + if len(store.objs["/id-1"].data) != len(jpeg) { + t.Fatal("the small copy should have been stored") + } +} diff --git a/internal/services/image_processor/source.go b/internal/services/image_processor/source.go new file mode 100644 index 0000000..505d092 --- /dev/null +++ b/internal/services/image_processor/source.go @@ -0,0 +1,65 @@ +package image_processor + +import ( + "context" + "net/http" + "net/url" + "regexp" + "strings" +) + +/* +MyAnimeList serves two copies of every image under the same name: the +225px thumbnail the scraper records, and a larger one (about 424x600 for a +poster) with an `l` before the extension: + + https://cdn.myanimelist.net/images/anime/1668/108792.jpg 225x318 + https://cdn.myanimelist.net/images/anime/1668/108792l.jpg 424x600 + +The larger one is what gets stored when it exists. Nearly twice the +resolution at the source beats anything an upscaler can invent, and text on +a poster is legible at 424px where it was not at 225. +*/ + +// malHosts are the hosts the variant rule applies to. A var so a test can +// point it at a local server. +var malHosts = map[string]bool{"cdn.myanimelist.net": true} + +var malImage = regexp.MustCompile(`^(/images/.+/\d+)(\.(?:jpe?g|png|webp))$`) + +// largerVariant names the `l` copy of a MyAnimeList image URL, or "" when +// the URL is not one (or already is the large copy). +func largerVariant(src string) string { + u, err := url.Parse(src) + if err != nil || !malHosts[strings.ToLower(u.Host)] { + return "" + } + m := malImage.FindStringSubmatch(u.Path) + if m == nil { + return "" + } + u.Path = m[1] + "l" + m[2] + return u.String() +} + +// preferredSource is the URL to fetch: the larger copy when the host has +// one, else the URL as given. One HEAD per image; a miss falls back. +func preferredSource(ctx context.Context, src string) string { + large := largerVariant(src) + if large == "" { + return src + } + req, err := http.NewRequestWithContext(ctx, http.MethodHead, large, nil) + if err != nil { + return src + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + return src + } + resp.Body.Close() + if resp.StatusCode != http.StatusOK || resp.ContentLength <= 0 { + return src + } + return large +}