From 33d88b97cc8f5642791f010b1fdcda100faa1200 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Fri, 22 May 2026 11:03:28 +0700 Subject: [PATCH] fix(lolschedule): paginate older pages to cover past days in week range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default lolesports schedule page is anchored near "now", so a calendar-aligned /lolschedule_week issued midweek was silently dropping events from Mon–Wed: we only walked pages.newer. fetchSchedulePage now also returns pages.older, and fetchEventsInRange walks older until the earliest collected event is ≤ from, then walks newer until ≥ to. Page budget bumped 3 → 8 to accommodate dense regular-season weeks. --- internal/modules/lolschedule/api_client.go | 100 +++++++++++++----- .../modules/lolschedule/api_client_test.go | 63 ++++++++++- 2 files changed, 133 insertions(+), 30 deletions(-) diff --git a/internal/modules/lolschedule/api_client.go b/internal/modules/lolschedule/api_client.go index 1ed15af..735df35 100644 --- a/internal/modules/lolschedule/api_client.go +++ b/internal/modules/lolschedule/api_client.go @@ -134,12 +134,15 @@ func (c *Client) baseURL() string { return apiURL } -// fetchSchedulePage retrieves one page of events. pageToken is the forward -// cursor from a previous call's `pages.newer`. -func (c *Client) fetchSchedulePage(ctx context.Context, pageToken string) ([]ScheduleEvent, string, error) { +// fetchSchedulePage retrieves one page of events. pageToken can be either a +// forward (`pages.newer`) or backward (`pages.older`) cursor — the upstream +// uses the same query param for both directions. Returns the page's events +// (sorted ascending by startTime) plus both cursor tokens for further +// navigation in either direction. +func (c *Client) fetchSchedulePage(ctx context.Context, pageToken string) ([]ScheduleEvent, string, string, error) { u, err := url.Parse(c.baseURL()) if err != nil { - return nil, "", fmt.Errorf("lolschedule parse url: %w", err) + return nil, "", "", fmt.Errorf("lolschedule parse url: %w", err) } q := u.Query() q.Set("hl", "en-US") @@ -150,7 +153,7 @@ func (c *Client) fetchSchedulePage(ctx context.Context, pageToken string) ([]Sch req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) if err != nil { - return nil, "", fmt.Errorf("lolschedule build request: %w", err) + return nil, "", "", fmt.Errorf("lolschedule build request: %w", err) } req.Header.Set("x-api-key", apiKey) req.Header.Set("User-Agent", userAgent) @@ -158,20 +161,20 @@ func (c *Client) fetchSchedulePage(ctx context.Context, pageToken string) ([]Sch resp, err := c.httpClient().Do(req) if err != nil { - return nil, "", fmt.Errorf("lolschedule do: %w", err) + return nil, "", "", fmt.Errorf("lolschedule do: %w", err) } defer func() { _ = resp.Body.Close() }() body, err := io.ReadAll(resp.Body) if err != nil { - return nil, "", fmt.Errorf("lolschedule read: %w", err) + return nil, "", "", fmt.Errorf("lolschedule read: %w", err) } if resp.StatusCode < 200 || resp.StatusCode >= 300 { log.Warn("lolschedule_fetch", "status", resp.StatusCode, "body", truncate(string(body), 500)) - return nil, "", fmt.Errorf("lolschedule API HTTP %d", resp.StatusCode) + return nil, "", "", fmt.Errorf("lolschedule API HTTP %d", resp.StatusCode) } var page schedulePage if err := json.Unmarshal(body, &page); err != nil { - return nil, "", fmt.Errorf("lolschedule decode: %w", err) + return nil, "", "", fmt.Errorf("lolschedule decode: %w", err) } // Drop pre/post-show segments; they aren't matches. out := make([]ScheduleEvent, 0, len(page.Data.Schedule.Events)) @@ -181,36 +184,77 @@ func (c *Client) fetchSchedulePage(ctx context.Context, pageToken string) ([]Sch } out = append(out, e) } - return out, page.Data.Schedule.Pages.Newer, nil + return out, page.Data.Schedule.Pages.Newer, page.Data.Schedule.Pages.Older, nil } -// fetchEventsInRange paginates forward until the supplied window is covered -// or maxPages is reached. Default page returns ~20 events; week view -// usually needs 1 extra page. +// earliestStart returns the first parseable startTime in a page that is sorted +// ascending. ok=false when no event has a valid timestamp. +func earliestStart(events []ScheduleEvent) (time.Time, bool) { + for _, e := range events { + if t, err := time.Parse(time.RFC3339, e.StartTime); err == nil { + return t, true + } + } + return time.Time{}, false +} + +// latestStart returns the last parseable startTime in a page that is sorted +// ascending. ok=false when no event has a valid timestamp. +func latestStart(events []ScheduleEvent) (time.Time, bool) { + for i := len(events) - 1; i >= 0; i-- { + if t, err := time.Parse(time.RFC3339, events[i].StartTime); err == nil { + return t, true + } + } + return time.Time{}, false +} + +// fetchEventsInRange covers [from, to) by walking both pagination directions +// from the default page (anchored at "now"). Walks older pages until the +// earliest collected event is ≤ from, then walks newer pages until the +// latest is ≥ to. Page budget caps both directions combined to bound +// upstream calls during dense weeks. func (c *Client) fetchEventsInRange(ctx context.Context, from, to time.Time, maxPages int) ([]ScheduleEvent, error) { if maxPages <= 0 { - maxPages = 3 + maxPages = 8 } - var collected []ScheduleEvent - pageToken := "" - for i := 0; i < maxPages; i++ { - events, newer, err := c.fetchSchedulePage(ctx, pageToken) + events, newer, older, err := c.fetchSchedulePage(ctx, "") + if err != nil { + return nil, err + } + collected := events + pages := 1 + + // Walk older while the window extends before what we've collected. + for pages < maxPages && older != "" { + if t, ok := earliestStart(collected); ok && !t.After(from) { + break + } + olderEvents, _, prevOlder, err := c.fetchSchedulePage(ctx, older) if err != nil { return nil, err } - collected = append(collected, events...) - // If the latest event in the page is already past our window end, stop. - if len(events) > 0 { - lastT, parseErr := time.Parse(time.RFC3339, events[len(events)-1].StartTime) - if parseErr == nil && !lastT.Before(to) { - break - } - } - if newer == "" { + // Each page is ascending and disjoint from the next, so prepending + // preserves overall ascending order. + collected = append(olderEvents, collected...) + older = prevOlder + pages++ + } + + // Walk newer while the window extends past what we've collected. + for pages < maxPages && newer != "" { + if t, ok := latestStart(collected); ok && !t.Before(to) { break } - pageToken = newer + newerEvents, nextNewer, _, err := c.fetchSchedulePage(ctx, newer) + if err != nil { + return nil, err + } + collected = append(collected, newerEvents...) + newer = nextNewer + pages++ } + out := make([]ScheduleEvent, 0, len(collected)) for _, e := range collected { t, err := time.Parse(time.RFC3339, e.StartTime) diff --git a/internal/modules/lolschedule/api_client_test.go b/internal/modules/lolschedule/api_client_test.go index fe3ea68..fc4d9b6 100644 --- a/internal/modules/lolschedule/api_client_test.go +++ b/internal/modules/lolschedule/api_client_test.go @@ -145,7 +145,7 @@ func TestFetchSchedulePage_DropsShowEvents(t *testing.T) { srv, _ := mkServer(t, sampleBody) c := &Client{HTTP: srv.Client(), URL: srv.URL} - events, _, err := c.fetchSchedulePage(context.Background(), "") + events, _, _, err := c.fetchSchedulePage(context.Background(), "") if err != nil { t.Fatal(err) } @@ -162,12 +162,71 @@ func TestFetchSchedulePage_NonJSONErrors(t *testing.T) { })) defer srv.Close() c := &Client{HTTP: srv.Client(), URL: srv.URL} - _, _, err := c.fetchSchedulePage(context.Background(), "") + _, _, _, err := c.fetchSchedulePage(context.Background(), "") if err == nil || !strings.Contains(err.Error(), "decode") { t.Errorf("non-JSON should produce decode error; got %v", err) } } +// TestFetchEventsInRange_WalksOlderForPastWindow guards the calendar-week +// regression: when `from` precedes the default page's earliest event, the +// fetcher must follow `pages.older` to pick up events earlier in the week. +// +// Server simulates 3 pages: +// - default (no pageToken) → Thu May 21 +// - pageToken=older1 → Wed May 20 (returns older=older2) +// - pageToken=older2 → Mon May 18 + Tue May 19 (no older) +// +// Asking for Mon May 18 → next Mon must return all 4 events. +func TestFetchEventsInRange_WalksOlderForPastWindow(t *testing.T) { + defaultBody := `{"data":{"schedule":{"events":[ + {"startTime":"2026-05-21T05:00:00Z","state":"completed","league":{"slug":"lck","name":"LCK"},"match":{"teams":[{"code":"BFX"},{"code":"HLE"}],"strategy":{"count":3}}} + ],"pages":{"newer":null,"older":"older1"}}}}` + older1Body := `{"data":{"schedule":{"events":[ + {"startTime":"2026-05-20T05:00:00Z","state":"completed","league":{"slug":"lck","name":"LCK"},"match":{"teams":[{"code":"GEN"},{"code":"T1"}],"strategy":{"count":3}}} + ],"pages":{"newer":"newerX","older":"older2"}}}}` + older2Body := `{"data":{"schedule":{"events":[ + {"startTime":"2026-05-18T05:00:00Z","state":"completed","league":{"slug":"lck","name":"LCK"},"match":{"teams":[{"code":"KT"},{"code":"DK"}],"strategy":{"count":3}}}, + {"startTime":"2026-05-19T05:00:00Z","state":"completed","league":{"slug":"lck","name":"LCK"},"match":{"teams":[{"code":"NS"},{"code":"BRO"}],"strategy":{"count":3}}} + ],"pages":{"newer":"newerY","older":null}}}}` + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Query().Get("pageToken") { + case "": + _, _ = w.Write([]byte(defaultBody)) + case "older1": + _, _ = w.Write([]byte(older1Body)) + case "older2": + _, _ = w.Write([]byte(older2Body)) + default: + t.Errorf("unexpected pageToken %q", r.URL.Query().Get("pageToken")) + w.WriteHeader(http.StatusBadRequest) + } + })) + defer srv.Close() + + c := &Client{HTTP: srv.Client(), URL: srv.URL} + from := time.Date(2026, 5, 18, 0, 0, 0, 0, time.UTC) + to := from.Add(7 * 24 * time.Hour) + + events, err := c.fetchEventsInRange(context.Background(), from, to, 0) + if err != nil { + t.Fatalf("fetch: %v", err) + } + if len(events) != 4 { + t.Errorf("events = %d, want 4 (Mon, Tue, Wed, Thu); got days=%v", len(events), eventDays(events)) + } +} + +func eventDays(events []ScheduleEvent) []string { + out := make([]string, 0, len(events)) + for _, e := range events { + out = append(out, e.StartTime) + } + return out +} + // truncate is internal but worth a smoke test — log payloads use it. func TestTruncate(t *testing.T) { if got := truncate("short", 10); got != "short" {