From fb6b36438276940052e03228b37984b2ea207a29 Mon Sep 17 00:00:00 2001 From: Admin Date: Wed, 4 Mar 2026 22:14:23 +0500 Subject: [PATCH] refactor: audit, split server.go, add unit tests, and fix latent bugs - Remove dead code: browser cdp/content_scrape strategies, writer package, printUsage, downloadAndStoreCoverCLI in main.go - Fix bugs: defer-in-loop in pocketbase deleteWhere, listAll() pagination hard cap removed, splitChapterTitle off-by-one in date extraction - Split server.go (~1700 lines) into focused handler files: handlers_audio, handlers_browse, handlers_progress, handlers_ranking, handlers_scrape - Export htmlutil.AttrVal/TextContent/ResolveURL; add storage/coverutil.go to consolidate duplicate helpers - Flatten deeply nested conditionals: voices() early-return guards, ScrapeCatalogue next-link double attr scan, chapterNumberFromKey dead strings.Cut line, splitChapterTitle double-nested unit/suffix loop - Add unit tests: htmlutil (9 funcs), novelfire ScrapeMetadata (3 cases), orchestrator Run (5 cases), storage chapterNumberFromKey/splitChapterTitle (22 cases); all pass with go build/vet/test clean --- scraper/cmd/scraper/main.go | 86 +- scraper/go.mod | 3 +- scraper/go.sum | 2 - scraper/internal/browser/cdp.go | 137 -- .../browser/{content_scrape.go => common.go} | 74 +- scraper/internal/e2e/e2e_test.go | 7 - scraper/internal/novelfire/ranking_test.go | 60 - scraper/internal/novelfire/scraper.go | 30 +- scraper/internal/novelfire/scraper_test.go | 78 + .../orchestrator/orchestrator_test.go | 312 ++++ scraper/internal/scraper/htmlutil/htmlutil.go | 33 +- .../scraper/htmlutil/htmlutil_test.go | 221 +++ scraper/internal/server/handlers_audio.go | 541 ++++++ scraper/internal/server/handlers_browse.go | 476 ++++++ scraper/internal/server/handlers_progress.go | 103 ++ scraper/internal/server/handlers_ranking.go | 86 + scraper/internal/server/handlers_scrape.go | 247 +++ scraper/internal/server/integration_test.go | 10 +- scraper/internal/server/server.go | 1501 +---------------- scraper/internal/storage/coverutil.go | 59 + scraper/internal/storage/hybrid.go | 45 +- .../storage/hybrid_integration_test.go | 12 +- scraper/internal/storage/hybrid_unit_test.go | 77 + scraper/internal/storage/integration_test.go | 7 +- scraper/internal/storage/pocketbase.go | 68 +- scraper/internal/writer/writer.go | 476 ------ 26 files changed, 2359 insertions(+), 2392 deletions(-) delete mode 100644 scraper/internal/browser/cdp.go rename scraper/internal/browser/{content_scrape.go => common.go} (60%) create mode 100644 scraper/internal/orchestrator/orchestrator_test.go create mode 100644 scraper/internal/scraper/htmlutil/htmlutil_test.go create mode 100644 scraper/internal/server/handlers_audio.go create mode 100644 scraper/internal/server/handlers_browse.go create mode 100644 scraper/internal/server/handlers_progress.go create mode 100644 scraper/internal/server/handlers_ranking.go create mode 100644 scraper/internal/server/handlers_scrape.go create mode 100644 scraper/internal/storage/coverutil.go create mode 100644 scraper/internal/storage/hybrid_unit_test.go delete mode 100644 scraper/internal/writer/writer.go diff --git a/scraper/cmd/scraper/main.go b/scraper/cmd/scraper/main.go index 60faab3..923a88d 100644 --- a/scraper/cmd/scraper/main.go +++ b/scraper/cmd/scraper/main.go @@ -10,10 +10,6 @@ // // Environment variables: // -// BROWSERLESS_URL Browserless base URL (default: http://localhost:3030) -// BROWSERLESS_TOKEN Browserless API token (default: "") -// BROWSERLESS_STRATEGY content | scrape | cdp | direct (default: direct; chapter list+text always use direct HTTP) -// BROWSERLESS_MAX_CONCURRENT Max simultaneous browser sessions (default: 5) // SCRAPER_WORKERS Chapter goroutine count (default: NumCPU) // SCRAPER_HTTP_ADDR HTTP listen address (default: :8080) // KOKORO_URL Kokoro-FastAPI base URL (default: "") @@ -33,9 +29,7 @@ package main import ( "context" "fmt" - "io" "log/slog" - "net/http" "os" "os/exec" "os/signal" @@ -48,6 +42,7 @@ import ( "github.com/libnovel/scraper/internal/browser" "github.com/libnovel/scraper/internal/novelfire" "github.com/libnovel/scraper/internal/orchestrator" + "github.com/libnovel/scraper/internal/scraper/htmlutil" "github.com/libnovel/scraper/internal/server" "github.com/libnovel/scraper/internal/storage" ) @@ -343,7 +338,7 @@ func runSaveBrowse(ctx context.Context, args []string, store storage.Store, log // Download cover image in the background (best-effort). if novel.coverURL != "" { - go downloadAndStoreCoverCLI(store, log, coverKey, novel.coverURL) + go storage.DownloadAndStoreCover(store, log, coverKey, novel.coverURL) } } if len(novels) > 0 { @@ -420,9 +415,9 @@ func parseSaveBrowseListings(htmlBytes []byte, novelFireBase string) []novelList // Extract cover URL from data-src or src on img tags. if cur.coverURL == "" && strings.Contains(trimmed, " pages to capture (default: 5) Environment variables: - BROWSERLESS_URL Browserless base URL (default: http://localhost:3030) - BROWSERLESS_TOKEN API token (default: "") - BROWSERLESS_STRATEGY content|scrape|cdp|direct (default: direct; chapter list+text always use direct HTTP) - BROWSERLESS_MAX_CONCURRENT Max simultaneous sessions (default: 5) - BROWSERLESS_TIMEOUT HTTP request timeout sec (default: 90) SCRAPER_WORKERS Chapter goroutines (default: NumCPU = %d) - SCRAPER_STATIC_ROOT Output directory (default: ./static/books) SCRAPER_HTTP_ADDR HTTP listen address (default: :8080) + SCRAPER_TIMEOUT HTTP request timeout sec (default: 90) KOKORO_URL Kokoro-FastAPI base URL (default: "", TTS disabled) KOKORO_VOICE Default TTS voice (default: af_bella) + POCKETBASE_URL PocketBase base URL (default: http://localhost:8090) + POCKETBASE_ADMIN_EMAIL PocketBase admin email (default: admin@libnovel.local) + POCKETBASE_ADMIN_PASSWORD PocketBase admin password (default: changeme123) + MINIO_ENDPOINT MinIO endpoint host:port (default: localhost:9000) + MINIO_ACCESS_KEY MinIO access key (default: admin) + MINIO_SECRET_KEY MinIO secret key (default: changeme123) + MINIO_USE_SSL MinIO TLS (default: false) + MINIO_BUCKET_CHAPTERS Chapter objects bucket (default: libnovel-chapters) + MINIO_BUCKET_AUDIO Audio objects bucket (default: libnovel-audio) MINIO_BUCKET_BROWSE Browse snapshots bucket (default: libnovel-browse) + BROWSERLESS_URL Browserless WS endpoint (default: http://localhost:3030) SINGLEFILE_PATH Path to single-file CLI (default: single-file) LOG_LEVEL debug|info|warn|error (default: info) `, runtime.NumCPU()) diff --git a/scraper/go.mod b/scraper/go.mod index 61b5e73..7828b2b 100644 --- a/scraper/go.mod +++ b/scraper/go.mod @@ -3,10 +3,8 @@ module github.com/libnovel/scraper go 1.25.0 require ( - github.com/gorilla/websocket v1.5.3 github.com/minio/minio-go/v7 v7.0.98 golang.org/x/net v0.51.0 - gopkg.in/yaml.v3 v3.0.1 ) require ( @@ -27,4 +25,5 @@ require ( golang.org/x/crypto v0.48.0 // indirect golang.org/x/sys v0.41.0 // indirect golang.org/x/text v0.34.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/scraper/go.sum b/scraper/go.sum index 43ef929..f4750f9 100644 --- a/scraper/go.sum +++ b/scraper/go.sum @@ -6,8 +6,6 @@ github.com/go-ini/ini v1.67.0 h1:z6ZrTEZqSWOTyH2FlglNbNgARyHG8oLW9gMELqKr06A= github.com/go-ini/ini v1.67.0/go.mod h1:ByCAeIL28uOIIG0E3PJtZPDL8WnHpFKFOtgjp+3Ies8= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= -github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/klauspost/compress v1.18.2 h1:iiPHWW0YrcFgpBYhsA6D1+fqHssJscY/Tm/y2Uqnapk= github.com/klauspost/compress v1.18.2/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4= github.com/klauspost/cpuid/v2 v2.0.1/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= diff --git a/scraper/internal/browser/cdp.go b/scraper/internal/browser/cdp.go deleted file mode 100644 index bdbe0cb..0000000 --- a/scraper/internal/browser/cdp.go +++ /dev/null @@ -1,137 +0,0 @@ -package browser - -import ( - "context" - "encoding/json" - "fmt" - "net/http" - "strings" - "sync/atomic" - "time" - - "github.com/gorilla/websocket" -) - -// cdpClient implements BrowserClient using the CDP WebSocket endpoint. -type cdpClient struct { - cfg Config - sem chan struct{} -} - -// NewCDPClient returns a BrowserClient that uses CDP WebSocket sessions. -func NewCDPClient(cfg Config) BrowserClient { - if cfg.Timeout == 0 { - cfg.Timeout = 60 * time.Second - } - return &cdpClient{cfg: cfg, sem: makeSem(cfg.MaxConcurrent)} -} - -func (c *cdpClient) Strategy() Strategy { return StrategyCDP } - -func (c *cdpClient) GetContent(_ context.Context, _ ContentRequest) (string, error) { - return "", fmt.Errorf("CDP client does not support /content; use NewContentClient") -} - -func (c *cdpClient) ScrapePage(_ context.Context, _ ScrapeRequest) (ScrapeResponse, error) { - return ScrapeResponse{}, fmt.Errorf("CDP client does not support /scrape; use NewScrapeClient") -} - -// CDPSession opens a WebSocket to the Browserless /devtools/browser endpoint, -// navigates to pageURL, and invokes fn with a live CDPConn. -func (c *cdpClient) CDPSession(ctx context.Context, pageURL string, fn CDPSessionFunc) error { - if err := acquire(ctx, c.sem); err != nil { - return fmt.Errorf("cdp: semaphore: %w", err) - } - defer release(c.sem) - - // Build WebSocket URL: ws://host:port/devtools/browser?token=...&url=... - wsURL := strings.Replace(c.cfg.BaseURL, "http://", "ws://", 1) - wsURL = strings.Replace(wsURL, "https://", "wss://", 1) - wsURL += "/devtools/browser" - sep := "?" - if c.cfg.Token != "" { - wsURL += sep + "token=" + c.cfg.Token - sep = "&" - } - wsURL += sep + "url=" + pageURL - - dialer := websocket.Dialer{ - HandshakeTimeout: 15 * time.Second, - Proxy: http.ProxyFromEnvironment, - } - - conn, _, err := dialer.DialContext(ctx, wsURL, nil) - if err != nil { - return fmt.Errorf("cdp: dial %s: %w", wsURL, err) - } - - cdp := &cdpConn{ws: conn} - defer cdp.Close() - - return fn(ctx, cdp) -} - -// ─── cdpConn ───────────────────────────────────────────────────────────────── - -type cdpConn struct { - ws *websocket.Conn - counter atomic.Int64 -} - -type cdpRequest struct { - ID int64 `json:"id"` - Method string `json:"method"` - Params map[string]any `json:"params,omitempty"` -} - -type cdpResponse struct { - ID int64 `json:"id"` - Result map[string]any `json:"result,omitempty"` - Error *struct { - Code int `json:"code"` - Message string `json:"message"` - } `json:"error,omitempty"` -} - -func (c *cdpConn) Send(ctx context.Context, method string, params map[string]any) (map[string]any, error) { - id := c.counter.Add(1) - - req := cdpRequest{ID: id, Method: method, Params: params} - data, err := json.Marshal(req) - if err != nil { - return nil, fmt.Errorf("cdp send: marshal: %w", err) - } - - if dl, ok := ctx.Deadline(); ok { - _ = c.ws.SetWriteDeadline(dl) - } - if err := c.ws.WriteMessage(websocket.TextMessage, data); err != nil { - return nil, fmt.Errorf("cdp send: write: %w", err) - } - - // Read messages until we find the response matching our id. - for { - if dl, ok := ctx.Deadline(); ok { - _ = c.ws.SetReadDeadline(dl) - } - _, msg, err := c.ws.ReadMessage() - if err != nil { - return nil, fmt.Errorf("cdp send: read: %w", err) - } - var resp cdpResponse - if err := json.Unmarshal(msg, &resp); err != nil { - continue // skip non-JSON frames (events etc.) - } - if resp.ID != id { - continue // event or different command reply - } - if resp.Error != nil { - return nil, fmt.Errorf("cdp error %d: %s", resp.Error.Code, resp.Error.Message) - } - return resp.Result, nil - } -} - -func (c *cdpConn) Close() error { - return c.ws.Close() -} diff --git a/scraper/internal/browser/content_scrape.go b/scraper/internal/browser/common.go similarity index 60% rename from scraper/internal/browser/content_scrape.go rename to scraper/internal/browser/common.go index a0e7a85..a84adf3 100644 --- a/scraper/internal/browser/content_scrape.go +++ b/scraper/internal/browser/common.go @@ -55,6 +55,8 @@ func release(sem chan struct{}) { } } +// ─── /content client ────────────────────────────────────────────────────────── + // contentClient implements BrowserClient using the /content endpoint. type contentClient struct { cfg Config @@ -121,75 +123,5 @@ func (c *contentClient) ScrapePage(_ context.Context, _ ScrapeRequest) (ScrapeRe } func (c *contentClient) CDPSession(_ context.Context, _ string, _ CDPSessionFunc) error { - return fmt.Errorf("content client does not support CDP; use NewCDPClient") -} - -// ─── /scrape client ─────────────────────────────────────────────────────────── - -type scrapeClient struct { - cfg Config - http *http.Client - sem chan struct{} -} - -// NewScrapeClient returns a BrowserClient that uses POST /scrape. -func NewScrapeClient(cfg Config) BrowserClient { - if cfg.Timeout == 0 { - cfg.Timeout = 90 * time.Second - } - return &scrapeClient{ - cfg: cfg, - http: &http.Client{Timeout: cfg.Timeout}, - sem: makeSem(cfg.MaxConcurrent), - } -} - -func (c *scrapeClient) Strategy() Strategy { return StrategyScrape } - -func (c *scrapeClient) GetContent(_ context.Context, _ ContentRequest) (string, error) { - return "", fmt.Errorf("scrape client does not support /content; use NewContentClient") -} - -func (c *scrapeClient) ScrapePage(ctx context.Context, req ScrapeRequest) (ScrapeResponse, error) { - if err := acquire(ctx, c.sem); err != nil { - return ScrapeResponse{}, fmt.Errorf("scrape: semaphore: %w", err) - } - defer release(c.sem) - - body, err := json.Marshal(req) - if err != nil { - return ScrapeResponse{}, fmt.Errorf("scrape: marshal request: %w", err) - } - - url := c.cfg.BaseURL + "/scrape" - if c.cfg.Token != "" { - url += "?token=" + c.cfg.Token - } - - httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(body)) - if err != nil { - return ScrapeResponse{}, fmt.Errorf("scrape: build request: %w", err) - } - httpReq.Header.Set("Content-Type", "application/json") - - resp, err := c.http.Do(httpReq) - if err != nil { - return ScrapeResponse{}, fmt.Errorf("scrape: do request: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - b, _ := io.ReadAll(resp.Body) - return ScrapeResponse{}, fmt.Errorf("scrape: unexpected status %d: %s", resp.StatusCode, b) - } - - var result ScrapeResponse - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - return ScrapeResponse{}, fmt.Errorf("scrape: decode response: %w", err) - } - return result, nil -} - -func (c *scrapeClient) CDPSession(_ context.Context, _ string, _ CDPSessionFunc) error { - return fmt.Errorf("scrape client does not support CDP; use NewCDPClient") + return fmt.Errorf("content client does not support CDP") } diff --git a/scraper/internal/e2e/e2e_test.go b/scraper/internal/e2e/e2e_test.go index 1210ed9..278845f 100644 --- a/scraper/internal/e2e/e2e_test.go +++ b/scraper/internal/e2e/e2e_test.go @@ -802,13 +802,6 @@ func assertURLAccessibleWithRetry(t *testing.T, ctx context.Context, url, label // ─── stdlib helpers ─────────────────────────────────────────────────────────── -func min(a, b int) int { - if a < b { - return a - } - return b -} - func chapterNumbers(refs []scraper.ChapterRef) []int { ns := make([]int, len(refs)) for i, r := range refs { diff --git a/scraper/internal/novelfire/ranking_test.go b/scraper/internal/novelfire/ranking_test.go index 1f6c13b..0299b19 100644 --- a/scraper/internal/novelfire/ranking_test.go +++ b/scraper/internal/novelfire/ranking_test.go @@ -2,12 +2,9 @@ package novelfire import ( "context" - "os" - "path/filepath" "testing" "github.com/libnovel/scraper/internal/scraper" - "github.com/libnovel/scraper/internal/writer" ) // rankingPage1HTML is a realistic mock of the popular genre listing page @@ -149,60 +146,3 @@ func TestScrapeRanking_EmptyPage(t *testing.T) { t.Errorf("expected 0 entries for empty page, got %d", len(entries)) } } - -// TestWriteRanking_RoundTrip verifies WriteRanking → ReadRankingItems -// faithfully reconstructs the original slice. -func TestWriteRanking_RoundTrip(t *testing.T) { - dir := t.TempDir() - w := writer.New(dir) - - items := []writer.RankingItem{ - {Rank: 1, Slug: "the-iron-throne", Title: "The Iron Throne", Status: "Ongoing", - Genres: []string{"Fantasy", "Action"}, SourceURL: "https://novelfire.net/book/the-iron-throne"}, - {Rank: 2, Slug: "shadow-mage", Title: "Shadow Mage", Status: "Completed", - Genres: []string{"Magic"}, SourceURL: "https://novelfire.net/book/shadow-mage"}, - } - - if err := w.WriteRanking(items); err != nil { - t.Fatalf("WriteRanking failed: %v", err) - } - - rankingFile := filepath.Join(dir, "ranking.json") - if _, err := os.Stat(rankingFile); err != nil { - t.Fatalf("ranking.json not created: %v", err) - } - - got, err := w.ReadRankingItems() - if err != nil { - t.Fatalf("ReadRankingItems failed: %v", err) - } - if len(got) != len(items) { - t.Fatalf("expected %d items, got %d", len(items), len(got)) - } - for i, want := range items { - if got[i].Rank != want.Rank { - t.Errorf("item[%d].Rank = %d, want %d", i, got[i].Rank, want.Rank) - } - if got[i].Slug != want.Slug { - t.Errorf("item[%d].Slug = %q, want %q", i, got[i].Slug, want.Slug) - } - if got[i].Title != want.Title { - t.Errorf("item[%d].Title = %q, want %q", i, got[i].Title, want.Title) - } - if got[i].Status != want.Status { - t.Errorf("item[%d].Status = %q, want %q", i, got[i].Status, want.Status) - } - if len(got[i].Genres) != len(want.Genres) { - t.Errorf("item[%d].Genres len = %d, want %d", i, len(got[i].Genres), len(want.Genres)) - } else { - for j, g := range want.Genres { - if got[i].Genres[j] != g { - t.Errorf("item[%d].Genres[%d] = %q, want %q", i, j, got[i].Genres[j], g) - } - } - } - if got[i].SourceURL != want.SourceURL { - t.Errorf("item[%d].SourceURL = %q, want %q", i, got[i].SourceURL, want.SourceURL) - } - } -} diff --git a/scraper/internal/novelfire/scraper.go b/scraper/internal/novelfire/scraper.go index 5be55b0..c03e86d 100644 --- a/scraper/internal/novelfire/scraper.go +++ b/scraper/internal/novelfire/scraper.go @@ -155,18 +155,8 @@ func (s *Scraper) ScrapeCatalogue(ctx context.Context) (<-chan scraper.Catalogue } nextHref := "" for _, a := range htmlutil.FindAll(root, scraper.Selector{Tag: "a", Multiple: true}) { - isNext := false - for _, attr := range a.Attr { - if attr.Key == "rel" && attr.Val == "next" { - isNext = true - } - } - if isNext { - for _, attr := range a.Attr { - if attr.Key == "href" { - nextHref = attr.Val - } - } + if htmlutil.AttrVal(a, "rel") == "next" { + nextHref = htmlutil.AttrVal(a, "href") break } } @@ -683,20 +673,8 @@ func (s *Scraper) ScrapeChapterText(ctx context.Context, ref scraper.ChapterRef) // ─── helpers ───────────────────────────────────────────────────────────────── -func resolveURL(base, href string) string { - if strings.HasPrefix(href, "http://") || strings.HasPrefix(href, "https://") { - return href - } - b, err := url.Parse(base) - if err != nil { - return base + href - } - ref, err := url.Parse(href) - if err != nil { - return base + href - } - return b.ResolveReference(ref).String() -} +// resolveURL is a thin alias over htmlutil.ResolveURL kept for readability. +func resolveURL(base, href string) string { return htmlutil.ResolveURL(base, href) } func slugFromURL(bookURL string) string { u, err := url.Parse(bookURL) diff --git a/scraper/internal/novelfire/scraper_test.go b/scraper/internal/novelfire/scraper_test.go index e629d38..1f46f36 100644 --- a/scraper/internal/novelfire/scraper_test.go +++ b/scraper/internal/novelfire/scraper_test.go @@ -141,6 +141,84 @@ func TestChapterNumberFromURL(t *testing.T) { } } +// ── ScrapeMetadata ──────────────────────────────────────────────────────────── + +func TestScrapeMetadata_ParsesFields(t *testing.T) { + html := ` +

The Iron Throne

+ Jane Doe +
+ Ongoing +
FantasyAction
+

A sweeping epic set in a magical world.

+ 42 Chapters + ` + + s := newScraper(html) + meta, err := s.ScrapeMetadata(context.Background(), "https://novelfire.net/book/the-iron-throne") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if meta.Slug != "the-iron-throne" { + t.Errorf("Slug = %q, want %q", meta.Slug, "the-iron-throne") + } + if meta.Title != "The Iron Throne" { + t.Errorf("Title = %q, want %q", meta.Title, "The Iron Throne") + } + if meta.Author != "Jane Doe" { + t.Errorf("Author = %q, want %q", meta.Author, "Jane Doe") + } + if meta.Cover != "https://cdn.example.com/cover.jpg" { + t.Errorf("Cover = %q, want %q", meta.Cover, "https://cdn.example.com/cover.jpg") + } + if meta.Status != "Ongoing" { + t.Errorf("Status = %q, want %q", meta.Status, "Ongoing") + } + if len(meta.Genres) != 2 || meta.Genres[0] != "Fantasy" || meta.Genres[1] != "Action" { + t.Errorf("Genres = %v, want [Fantasy Action]", meta.Genres) + } + if !strings.Contains(meta.Summary, "sweeping epic") { + t.Errorf("Summary = %q, want it to contain 'sweeping epic'", meta.Summary) + } + if meta.TotalChapters != 42 { + t.Errorf("TotalChapters = %d, want 42", meta.TotalChapters) + } +} + +func TestScrapeMetadata_RelativeCoverURL(t *testing.T) { + html := ` +

Relative Cover

+
+ ` + + s := newScraper(html) + meta, err := s.ScrapeMetadata(context.Background(), "https://novelfire.net/book/relative-cover") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Relative cover URL should be resolved against the base domain. + if !strings.HasPrefix(meta.Cover, "https://novelfire.net") { + t.Errorf("Cover = %q, expected it to be resolved to an absolute URL", meta.Cover) + } +} + +func TestScrapeMetadata_MissingFields(t *testing.T) { + // Minimal page — everything absent; should succeed without panicking. + html := `` + + s := newScraper(html) + meta, err := s.ScrapeMetadata(context.Background(), "https://novelfire.net/book/empty-novel") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if meta.Slug != "empty-novel" { + t.Errorf("Slug = %q, want %q", meta.Slug, "empty-novel") + } + if meta.TotalChapters != 0 { + t.Errorf("TotalChapters = %d, want 0 for missing chapter-count", meta.TotalChapters) + } +} + // ── ScrapeChapterList (position vs URL numbering) ───────────────────────────── // TestScrapeChapterList_NumbersFromURL verifies that when the chapter list HTML diff --git a/scraper/internal/orchestrator/orchestrator_test.go b/scraper/internal/orchestrator/orchestrator_test.go new file mode 100644 index 0000000..9a1de11 --- /dev/null +++ b/scraper/internal/orchestrator/orchestrator_test.go @@ -0,0 +1,312 @@ +package orchestrator + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/libnovel/scraper/internal/scraper" + "github.com/libnovel/scraper/internal/storage" + "io" + "log/slog" +) + +// ── mock NovelScraper ───────────────────────────────────────────────────────── + +type mockScraper struct { + catalogue []scraper.CatalogueEntry + meta scraper.BookMeta + metaErr error + chapters []scraper.ChapterRef + chapterTextFn func(ref scraper.ChapterRef) (scraper.Chapter, error) +} + +func (m *mockScraper) SourceName() string { return "mock" } + +func (m *mockScraper) ScrapeCatalogue(_ context.Context) (<-chan scraper.CatalogueEntry, <-chan error) { + entries := make(chan scraper.CatalogueEntry, len(m.catalogue)) + errs := make(chan error, 1) + for _, e := range m.catalogue { + entries <- e + } + close(entries) + close(errs) + return entries, errs +} + +func (m *mockScraper) ScrapeMetadata(_ context.Context, _ string) (scraper.BookMeta, error) { + return m.meta, m.metaErr +} + +func (m *mockScraper) ScrapeChapterList(_ context.Context, _ string) ([]scraper.ChapterRef, error) { + return m.chapters, nil +} + +func (m *mockScraper) ScrapeChapterText(_ context.Context, ref scraper.ChapterRef) (scraper.Chapter, error) { + if m.chapterTextFn != nil { + return m.chapterTextFn(ref) + } + return scraper.Chapter{Ref: ref, Text: "stub text"}, nil +} + +func (m *mockScraper) ScrapeRanking(_ context.Context, _ int) (<-chan scraper.BookMeta, <-chan error) { + ch := make(chan scraper.BookMeta) + errs := make(chan error) + close(ch) + close(errs) + return ch, errs +} + +// ── mock Store ──────────────────────────────────────────────────────────────── + +// mockStore records which methods were called; only implements what the +// orchestrator touches. All other methods panic so unexpected calls surface +// as test failures rather than silent no-ops. +type mockStore struct { + mu sync.Mutex + writtenMeta []scraper.BookMeta + writtenChapters []scraper.Chapter + existingSlugs map[string]map[int]bool // slug → chapterNum → exists +} + +func newMockStore() *mockStore { + return &mockStore{existingSlugs: make(map[string]map[int]bool)} +} + +func (s *mockStore) ChapterExists(_ context.Context, slug string, ref scraper.ChapterRef) bool { + s.mu.Lock() + defer s.mu.Unlock() + if m, ok := s.existingSlugs[slug]; ok { + return m[ref.Number] + } + return false +} + +func (s *mockStore) WriteChapter(_ context.Context, slug string, ch scraper.Chapter) error { + s.mu.Lock() + defer s.mu.Unlock() + s.writtenChapters = append(s.writtenChapters, ch) + return nil +} + +func (s *mockStore) WriteMetadata(_ context.Context, meta scraper.BookMeta) error { + s.mu.Lock() + defer s.mu.Unlock() + s.writtenMeta = append(s.writtenMeta, meta) + return nil +} + +// Unimplemented Store methods — panic so accidental calls surface immediately. +func (s *mockStore) ReadMetadata(_ context.Context, _ string) (scraper.BookMeta, bool, error) { + panic("ReadMetadata not expected") +} +func (s *mockStore) ListBooks(_ context.Context) ([]scraper.BookMeta, error) { + panic("ListBooks not expected") +} +func (s *mockStore) LocalSlugs(_ context.Context) (map[string]bool, error) { + panic("LocalSlugs not expected") +} +func (s *mockStore) MetadataMtime(_ context.Context, _ string) int64 { return 0 } +func (s *mockStore) ReadChapter(_ context.Context, _ string, _ int) (string, error) { + panic("ReadChapter not expected") +} +func (s *mockStore) ListChapters(_ context.Context, _ string) ([]storage.ChapterInfo, error) { + panic("ListChapters not expected") +} +func (s *mockStore) CountChapters(_ context.Context, _ string) int { return 0 } +func (s *mockStore) ReindexChapters(_ context.Context, _ string) (int, error) { + panic("ReindexChapters not expected") +} +func (s *mockStore) WriteRankingItem(_ context.Context, _ storage.RankingItem) error { return nil } +func (s *mockStore) ReadRankingItems(_ context.Context) ([]storage.RankingItem, error) { + return nil, nil +} +func (s *mockStore) RankingFreshEnough(_ context.Context, _ time.Duration) (bool, error) { + return false, nil +} +func (s *mockStore) GetAudioCache(_ context.Context, _ string) (string, bool) { return "", false } +func (s *mockStore) SetAudioCache(_ context.Context, _, _ string) error { return nil } +func (s *mockStore) PutAudio(_ context.Context, _ string, _ []byte) error { return nil } +func (s *mockStore) GetProgress(_ context.Context, _, _ string) (storage.ReadingProgress, bool) { + return storage.ReadingProgress{}, false +} +func (s *mockStore) SetProgress(_ context.Context, _ string, _ storage.ReadingProgress) error { + return nil +} +func (s *mockStore) AllProgress(_ context.Context, _ string) ([]storage.ReadingProgress, error) { + return nil, nil +} +func (s *mockStore) DeleteProgress(_ context.Context, _, _ string) error { return nil } +func (s *mockStore) AudioObjectKey(_ string, _ int, _ string) string { return "" } + +func (s *mockStore) AudioExists(_ context.Context, _ string) bool { return false } +func (s *mockStore) PresignChapter(_ context.Context, _ string, _ int, _ time.Duration) (string, error) { + return "", nil +} +func (s *mockStore) PresignAudio(_ context.Context, _ string, _ time.Duration) (string, error) { + return "", nil +} +func (s *mockStore) SaveBrowsePage(_ context.Context, _, _ string) error { return nil } +func (s *mockStore) GetBrowsePage(_ context.Context, _ string) (string, bool, error) { + return "", false, nil +} +func (s *mockStore) BrowseHTMLKey(_ string, _ int) string { return "" } +func (s *mockStore) BrowseCoverKey(_, _ string) string { return "" } +func (s *mockStore) SaveBrowseAsset(_ context.Context, _ string, _ []byte, _ string) error { + return nil +} +func (s *mockStore) GetBrowseAsset(_ context.Context, _ string) ([]byte, string, bool, error) { + return nil, "", false, nil +} +func (s *mockStore) CreateScrapeTask(_ context.Context, _, _ string) (string, error) { + return "task-id", nil +} +func (s *mockStore) UpdateScrapeTask(_ context.Context, _ string, _ storage.ScrapeTaskUpdate) error { + return nil +} +func (s *mockStore) ListScrapeTasks(_ context.Context) ([]storage.ScrapeTask, error) { + return nil, nil +} + +// ── helpers ─────────────────────────────────────────────────────────────────── + +func discardLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(io.Discard, nil)) +} + +// ── tests ───────────────────────────────────────────────────────────────────── + +// TestRun_SingleBook verifies the happy-path single-book scrape: metadata is +// persisted and all chapters are written to the store. +func TestRun_SingleBook(t *testing.T) { + novel := &mockScraper{ + meta: scraper.BookMeta{Slug: "the-iron-throne", Title: "The Iron Throne"}, + chapters: []scraper.ChapterRef{ + {Number: 1, Title: "Chapter 1", URL: "https://example.com/book/ch-1"}, + {Number: 2, Title: "Chapter 2", URL: "https://example.com/book/ch-2"}, + {Number: 3, Title: "Chapter 3", URL: "https://example.com/book/ch-3"}, + }, + } + store := newMockStore() + + o := New(Config{Workers: 2, SingleBookURL: "https://example.com/book/the-iron-throne"}, novel, discardLogger(), store) + if err := o.Run(context.Background()); err != nil { + t.Fatalf("Run() returned error: %v", err) + } + + store.mu.Lock() + defer store.mu.Unlock() + + if len(store.writtenMeta) != 1 { + t.Errorf("writtenMeta count = %d, want 1", len(store.writtenMeta)) + } + if len(store.writtenChapters) != 3 { + t.Errorf("writtenChapters count = %d, want 3", len(store.writtenChapters)) + } +} + +// TestRun_SingleBook_SkipsExistingChapters verifies that chapters already in +// the store are not re-scraped. +func TestRun_SingleBook_SkipsExistingChapters(t *testing.T) { + novel := &mockScraper{ + meta: scraper.BookMeta{Slug: "test-novel", Title: "Test Novel"}, + chapters: []scraper.ChapterRef{ + {Number: 1, Title: "Chapter 1"}, + {Number: 2, Title: "Chapter 2"}, + }, + } + store := newMockStore() + // Mark chapter 1 as already existing. + store.existingSlugs["test-novel"] = map[int]bool{1: true} + + o := New(Config{Workers: 1, SingleBookURL: "https://example.com/book/test-novel"}, novel, discardLogger(), store) + if err := o.Run(context.Background()); err != nil { + t.Fatalf("Run() returned error: %v", err) + } + + store.mu.Lock() + defer store.mu.Unlock() + + // Only chapter 2 should have been written; chapter 1 was skipped. + if len(store.writtenChapters) != 1 { + t.Errorf("writtenChapters count = %d, want 1 (skipped ch1)", len(store.writtenChapters)) + } + if store.writtenChapters[0].Ref.Number != 2 { + t.Errorf("expected chapter 2 to be written, got chapter %d", store.writtenChapters[0].Ref.Number) + } +} + +// TestRun_CatalogueMode verifies that catalogue mode processes all books. +func TestRun_CatalogueMode(t *testing.T) { + novel := &mockScraper{ + catalogue: []scraper.CatalogueEntry{ + {Title: "Book A", URL: "https://example.com/book/a"}, + {Title: "Book B", URL: "https://example.com/book/b"}, + }, + meta: scraper.BookMeta{Slug: "book-slug", Title: "A Book"}, + chapters: []scraper.ChapterRef{{Number: 1, Title: "Chapter 1"}}, + } + store := newMockStore() + + o := New(Config{Workers: 2}, novel, discardLogger(), store) + if err := o.Run(context.Background()); err != nil { + t.Fatalf("Run() returned error: %v", err) + } + + store.mu.Lock() + defer store.mu.Unlock() + + // 2 books → 2 metadata writes, 2 chapter writes (one chapter per book). + if len(store.writtenMeta) != 2 { + t.Errorf("writtenMeta count = %d, want 2", len(store.writtenMeta)) + } + if len(store.writtenChapters) != 2 { + t.Errorf("writtenChapters count = %d, want 2", len(store.writtenChapters)) + } +} + +// TestRun_OnProgress_Called verifies that the OnProgress callback fires at +// least once upon completion. +func TestRun_OnProgress_Called(t *testing.T) { + novel := &mockScraper{ + meta: scraper.BookMeta{Slug: "progress-book", Title: "Progress Book"}, + chapters: []scraper.ChapterRef{{Number: 1, Title: "Chapter 1"}}, + } + store := newMockStore() + + var callCount int + o := New(Config{ + Workers: 1, + SingleBookURL: "https://example.com/book/progress-book", + OnProgress: func(_ Progress) { + callCount++ + }, + }, novel, discardLogger(), store) + + if err := o.Run(context.Background()); err != nil { + t.Fatalf("Run() returned error: %v", err) + } + if callCount == 0 { + t.Error("OnProgress was never called") + } +} + +// TestRun_ContextCancelled verifies that Run returns a non-nil error when the +// context is cancelled before work completes. +func TestRun_ContextCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cancel immediately + + novel := &mockScraper{ + meta: scraper.BookMeta{Slug: "cancel-book", Title: "Cancel Book"}, + chapters: []scraper.ChapterRef{{Number: 1}}, + } + store := newMockStore() + + o := New(Config{Workers: 1, SingleBookURL: "https://example.com/book/cancel-book"}, novel, discardLogger(), store) + err := o.Run(ctx) + if err == nil { + t.Error("expected non-nil error when context is cancelled, got nil") + } +} diff --git a/scraper/internal/scraper/htmlutil/htmlutil.go b/scraper/internal/scraper/htmlutil/htmlutil.go index 7a25980..202cee8 100644 --- a/scraper/internal/scraper/htmlutil/htmlutil.go +++ b/scraper/internal/scraper/htmlutil/htmlutil.go @@ -3,6 +3,7 @@ package htmlutil import ( + "net/url" "regexp" "strings" @@ -10,6 +11,24 @@ import ( "golang.org/x/net/html" ) +// ResolveURL returns an absolute URL. If href is already absolute it is +// returned unchanged. Otherwise it is resolved against base using standard +// URL resolution (handles relative paths, absolute paths, etc.). +func ResolveURL(base, href string) string { + if strings.HasPrefix(href, "http://") || strings.HasPrefix(href, "https://") { + return href + } + b, err := url.Parse(base) + if err != nil { + return base + href + } + ref, err := url.Parse(href) + if err != nil { + return base + href + } + return b.ResolveReference(ref).String() +} + // ParseHTML parses raw HTML and returns the root node. func ParseHTML(raw string) (*html.Node, error) { return html.Parse(strings.NewReader(raw)) @@ -48,8 +67,8 @@ matched: return true } -// attrVal returns the value of attribute key from node n. -func attrVal(n *html.Node, key string) string { +// AttrVal returns the value of attribute key from node n. +func AttrVal(n *html.Node, key string) string { for _, a := range n.Attr { if a.Key == key { return a.Val @@ -58,8 +77,11 @@ func attrVal(n *html.Node, key string) string { return "" } -// textContent returns the concatenated text content of all descendant text nodes. -func textContent(n *html.Node) string { +// attrVal is an unexported alias kept for internal use within this package. +func attrVal(n *html.Node, key string) string { return AttrVal(n, key) } + +// TextContent returns the concatenated text content of all descendant text nodes. +func TextContent(n *html.Node) string { var sb strings.Builder var walk func(*html.Node) walk = func(cur *html.Node) { @@ -74,6 +96,9 @@ func textContent(n *html.Node) string { return strings.TrimSpace(sb.String()) } +// textContent is an unexported alias kept for internal use within this package. +func textContent(n *html.Node) string { return TextContent(n) } + // FindFirst returns the first node matching sel within root. func FindFirst(root *html.Node, sel scraper.Selector) *html.Node { var found *html.Node diff --git a/scraper/internal/scraper/htmlutil/htmlutil_test.go b/scraper/internal/scraper/htmlutil/htmlutil_test.go new file mode 100644 index 0000000..d9356a3 --- /dev/null +++ b/scraper/internal/scraper/htmlutil/htmlutil_test.go @@ -0,0 +1,221 @@ +package htmlutil + +import ( + "strings" + "testing" + + "github.com/libnovel/scraper/internal/scraper" +) + +// ── ResolveURL ──────────────────────────────────────────────────────────────── + +func TestResolveURL(t *testing.T) { + cases := []struct{ base, href, want string }{ + // Already absolute → unchanged. + {"https://example.com", "https://other.com/page", "https://other.com/page"}, + {"https://example.com", "http://other.com/page", "http://other.com/page"}, + // Absolute path. + {"https://example.com", "/book/slug", "https://example.com/book/slug"}, + // Relative path. + {"https://example.com/genre/all", "page?p=2", "https://example.com/genre/page?p=2"}, + // Empty href → base itself. + {"https://example.com", "", "https://example.com"}, + } + for _, c := range cases { + got := ResolveURL(c.base, c.href) + if got != c.want { + t.Errorf("ResolveURL(%q, %q) = %q, want %q", c.base, c.href, got, c.want) + } + } +} + +// ── AttrVal ─────────────────────────────────────────────────────────────────── + +func TestAttrVal(t *testing.T) { + root, err := ParseHTML(`text`) + if err != nil { + t.Fatal(err) + } + a := FindFirst(root, scraper.Selector{Tag: "a"}) + if a == nil { + t.Fatal("expected to find ") + } + if got := AttrVal(a, "href"); got != "/book/slug" { + t.Errorf("AttrVal href = %q, want %q", got, "/book/slug") + } + if got := AttrVal(a, "class"); got != "link" { + t.Errorf("AttrVal class = %q, want %q", got, "link") + } + if got := AttrVal(a, "missing"); got != "" { + t.Errorf("AttrVal missing = %q, want empty", got) + } +} + +// ── TextContent ─────────────────────────────────────────────────────────────── + +func TestTextContent(t *testing.T) { + root, err := ParseHTML(`

Hello world

`) + if err != nil { + t.Fatal(err) + } + p := FindFirst(root, scraper.Selector{Tag: "p"}) + if p == nil { + t.Fatal("expected to find

") + } + if got := TextContent(p); got != "Hello world" { + t.Errorf("TextContent = %q, want %q", got, "Hello world") + } +} + +// ── FindFirst / FindAll ─────────────────────────────────────────────────────── + +func TestFindFirst_ByTag(t *testing.T) { + root, _ := ParseHTML(`

Title

Sub

`) + n := FindFirst(root, scraper.Selector{Tag: "h1"}) + if n == nil { + t.Fatal("expected to find

") + } + if TextContent(n) != "Title" { + t.Errorf("h1 text = %q, want %q", TextContent(n), "Title") + } +} + +func TestFindFirst_ByClass(t *testing.T) { + root, _ := ParseHTML(`JR`) + n := FindFirst(root, scraper.Selector{Tag: "span", Class: "author"}) + if n == nil { + t.Fatal("expected to find span.author") + } + if TextContent(n) != "JR" { + t.Errorf("author text = %q, want %q", TextContent(n), "JR") + } +} + +func TestFindFirst_ByID(t *testing.T) { + root, _ := ParseHTML(`

text

`) + n := FindFirst(root, scraper.Selector{ID: "content"}) + if n == nil { + t.Fatal("expected to find #content") + } +} + +func TestFindFirst_NoMatch(t *testing.T) { + root, _ := ParseHTML(`

nothing

`) + n := FindFirst(root, scraper.Selector{Tag: "h1"}) + if n != nil { + t.Errorf("expected nil for missing tag, got %v", n) + } +} + +func TestFindAll_Multiple(t *testing.T) { + root, _ := ParseHTML(` +
  • A
    • +
  • B
    • +
  • C
    • + `) + nodes := FindAll(root, scraper.Selector{Tag: "li", Class: "novel-item"}) + if len(nodes) != 2 { + t.Errorf("FindAll novel-item = %d, want 2", len(nodes)) + } +} + +// ── ExtractFirst / ExtractAll ───────────────────────────────────────────────── + +func TestExtractFirst_TextNode(t *testing.T) { + root, _ := ParseHTML(`

    Shadow Slave

    `) + got := ExtractFirst(root, scraper.Selector{Tag: "h1", Class: "novel-title"}) + if got != "Shadow Slave" { + t.Errorf("ExtractFirst title = %q, want %q", got, "Shadow Slave") + } +} + +func TestExtractFirst_AttrNode(t *testing.T) { + root, _ := ParseHTML(``) + got := ExtractFirst(root, scraper.Selector{Tag: "img", Attr: "src"}) + if got != "/covers/slug.jpg" { + t.Errorf("ExtractFirst img src = %q, want %q", got, "/covers/slug.jpg") + } +} + +func TestExtractFirst_Missing(t *testing.T) { + root, _ := ParseHTML(``) + got := ExtractFirst(root, scraper.Selector{Tag: "h1"}) + if got != "" { + t.Errorf("ExtractFirst missing = %q, want empty", got) + } +} + +func TestExtractAll_Genres(t *testing.T) { + root, _ := ParseHTML(` +
    + Action + Fantasy +
    + `) + genresNode := FindFirst(root, scraper.Selector{Tag: "div", Class: "genres"}) + if genresNode == nil { + t.Fatal("expected genres div") + } + genres := ExtractAll(genresNode, scraper.Selector{Tag: "a"}) + if len(genres) != 2 { + t.Fatalf("genres = %v, want 2", genres) + } + if genres[0] != "Action" || genres[1] != "Fantasy" { + t.Errorf("genres = %v, want [Action Fantasy]", genres) + } +} + +// ── NodeToMarkdown ──────────────────────────────────────────────────────────── + +func TestNodeToMarkdown_Paragraphs(t *testing.T) { + root, _ := ParseHTML(`
    +

    First paragraph.

    +

    Second paragraph.

    +
    `) + container := FindFirst(root, scraper.Selector{ID: "content"}) + if container == nil { + t.Fatal("missing #content") + } + md := NodeToMarkdown(container) + if md == "" { + t.Fatal("NodeToMarkdown returned empty string") + } + for _, want := range []string{"First paragraph", "Second paragraph"} { + if !strings.Contains(md, want) { + t.Errorf("NodeToMarkdown missing %q in:\n%s", want, md) + } + } +} + +func TestNodeToMarkdown_Bold(t *testing.T) { + root, _ := ParseHTML(`

    He was very strong.

    `) + container := FindFirst(root, scraper.Selector{ID: "content"}) + md := NodeToMarkdown(container) + if !strings.Contains(md, "**very**") { + t.Errorf("NodeToMarkdown should wrap in **, got:\n%s", md) + } +} + +func TestNodeToMarkdown_ScriptStripped(t *testing.T) { + root, _ := ParseHTML(`

    Good

    `) + container := FindFirst(root, scraper.Selector{ID: "content"}) + md := NodeToMarkdown(container) + if strings.Contains(md, "alert") { + t.Errorf("NodeToMarkdown should strip