fix: ranking refresh deadlock, switch to direct HTTP, add CI pipeline

- Fix channel drain goroutine deadlock in handleRankingRefresh: replace 'for { ... continue ... if nil break }' with 'for A != nil || B != nil' so the select never blocks on two nil channels - Switch ScrapeRanking from urlClient (Browserless) to client (direct HTTP) since novelfire.net/ranking is fully server-rendered — no JS needed - Add ranking unit tests (single page, multi-page, empty page, write round-trip) - Add .gitea/workflows/ci.yaml: lint, test, build jobs with commented-out Docker image push step for when runner has Docker available
2026-03-01 19:45:28 +05:00
parent afa457cedd
commit 26302058d0
4 changed files with 310 additions and 25 deletions
--- a/.gitea/workflows/ci.yaml
+++ b/.gitea/workflows/ci.yaml
@@ -0,0 +1,106 @@
+name: CI
+
+on:
+  push:
+    branches: ["main", "master"]
+    paths:
+      - "scraper/**"
+      - ".gitea/workflows/**"
+  pull_request:
+    branches: ["main", "master"]
+    paths:
+      - "scraper/**"
+      - ".gitea/workflows/**"
+
+defaults:
+  run:
+    working-directory: scraper
+
+jobs:
+  # ── lint & vet ───────────────────────────────────────────────────────────────
+  lint:
+    name: Lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: scraper/go.mod
+          cache-dependency-path: scraper/go.sum
+
+      - name: go vet
+        run: go vet ./...
+
+      - name: staticcheck
+        run: |
+          go install honnef.co/go/tools/cmd/staticcheck@latest
+          staticcheck ./...
+
+  # ── tests ────────────────────────────────────────────────────────────────────
+  test:
+    name: Test
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: scraper/go.mod
+          cache-dependency-path: scraper/go.sum
+
+      - name: Run tests
+        run: go test -race -count=1 -timeout=60s ./...
+
+  # ── build binary ─────────────────────────────────────────────────────────────
+  build:
+    name: Build
+    runs-on: ubuntu-latest
+    needs: [lint, test]
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: scraper/go.mod
+          cache-dependency-path: scraper/go.sum
+
+      - name: Build binary
+        run: |
+          CGO_ENABLED=0 GOOS=linux GOARCH=amd64 \
+            go build -ldflags="-s -w" -o bin/scraper ./cmd/scraper
+
+      - name: Upload binary artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: scraper-linux-amd64
+          path: scraper/bin/scraper
+          retention-days: 7
+
+  # ── docker build (& push) ────────────────────────────────────────────────────
+  # Uncomment once the runner has Docker available and a registry is configured.
+  #
+  # docker:
+  #   name: Docker
+  #   runs-on: ubuntu-latest
+  #   needs: [lint, test]
+  #   # Only push images on commits to the default branch, not on PRs.
+  #   # if: github.event_name == 'push'
+  #   steps:
+  #     - uses: actions/checkout@v4
+  #
+  #     - name: Log in to Gitea registry
+  #       uses: docker/login-action@v3
+  #       with:
+  #         registry: gitea.kalekber.cc
+  #         username: ${{ secrets.REGISTRY_USER }}
+  #         password: ${{ secrets.REGISTRY_TOKEN }}
+  #
+  #     - name: Build and push
+  #       uses: docker/build-push-action@v5
+  #       with:
+  #         context: ./scraper
+  #         push: true
+  #         tags: |
+  #           gitea.kalekber.cc/kamil/libnovel:latest
+  #           gitea.kalekber.cc/kamil/libnovel:${{ gitea.sha }}
--- a/scraper/internal/novelfire/ranking_test.go
+++ b/scraper/internal/novelfire/ranking_test.go
@@ -0,0 +1,187 @@
+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 novelfire.net/ranking?page=1.
+// It contains two novel-item entries and a "next" link for pagination tests.
+func rankingPage1HTML() string {
+	return `<!DOCTYPE html>
+<html><body>
+<ul class="rank-novels">
+  <li class="novel-item">
+    <figure class="cover"><a href="/book/the-iron-throne"><img data-src="/covers/iron-throne.jpg"></a></figure>
+    <div class="item-body">
+      <h2 class="title"><a href="/book/the-iron-throne">The Iron Throne</a></h2>
+      <span class="status">Ongoing</span>
+      <div class="categories"><div class="scroll"><span>Fantasy</span><span>Action</span></div></div>
+    </div>
+  </li>
+  <li class="novel-item">
+    <figure class="cover"><a href="/book/shadow-mage"><img data-src="/covers/shadow-mage.jpg"></a></figure>
+    <div class="item-body">
+      <h2 class="title"><a href="/book/shadow-mage">Shadow Mage</a></h2>
+      <span class="status">Completed</span>
+      <div class="categories"><div class="scroll"><span>Magic</span></div></div>
+    </div>
+  </li>
+</ul>
+<a class="next" href="/ranking?page=2">Next</a>
+</body></html>`
+}
+
+func rankingPage2HTML() string {
+	return `<!DOCTYPE html>
+<html><body>
+<ul class="rank-novels">
+  <li class="novel-item">
+    <figure class="cover"><a href="/book/void-hunter"><img data-src="/covers/void-hunter.jpg"></a></figure>
+    <div class="item-body">
+      <h2 class="title"><a href="/book/void-hunter">Void Hunter</a></h2>
+      <span class="status">Ongoing</span>
+      <div class="categories"><div class="scroll"><span>Sci-Fi</span></div></div>
+    </div>
+  </li>
+</ul>
+<!-- no .next link → last page -->
+</body></html>`
+}
+
+// drainRanking collects all entries from a ScrapeRanking call without
+// deadlocking. It uses "for A != nil || B != nil" — nil channels are never
+// selected, so setting one to nil effectively removes it from the select.
+func drainRanking(t *testing.T, entryCh <-chan scraper.BookMeta, errCh <-chan error) []scraper.BookMeta {
+	t.Helper()
+	var entries []scraper.BookMeta
+	for entryCh != nil || errCh != nil {
+		select {
+		case meta, ok := <-entryCh:
+			if !ok {
+				entryCh = nil
+			} else {
+				entries = append(entries, meta)
+			}
+		case err, ok := <-errCh:
+			if !ok {
+				errCh = nil
+			} else if err != nil {
+				t.Fatalf("unexpected scrape error: %v", err)
+			}
+		}
+	}
+	return entries
+}
+
+// TestScrapeRanking_SinglePage verifies a single page is parsed into entries
+// with sequential Ranking numbers using a stub client.
+// ScrapeRanking uses s.client (the main client, not urlClient) because the
+// ranking page is fully server-rendered.
+func TestScrapeRanking_SinglePage(t *testing.T) {
+	// newScraper passes the stub as s.client — exactly what ScrapeRanking uses.
+	s := newScraper(rankingPage1HTML())
+	entryCh, errCh := s.ScrapeRanking(context.Background(), 1)
+	entries := drainRanking(t, entryCh, errCh)
+
+	if len(entries) != 2 {
+		t.Fatalf("expected 2 entries, got %d", len(entries))
+	}
+	if entries[0].Ranking != 1 || entries[0].Title != "The Iron Throne" {
+		t.Errorf("entry[0]: got rank=%d title=%q, want rank=1 title=%q",
+			entries[0].Ranking, entries[0].Title, "The Iron Throne")
+	}
+	if entries[1].Ranking != 2 || entries[1].Title != "Shadow Mage" {
+		t.Errorf("entry[1]: got rank=%d title=%q, want rank=2 title=%q",
+			entries[1].Ranking, entries[1].Title, "Shadow Mage")
+	}
+}
+
+// TestScrapeRanking_MultiPage verifies pagination across two pages yields
+// contiguous rank numbers (1, 2, 3).
+func TestScrapeRanking_MultiPage(t *testing.T) {
+	// Use pagedStubClient for s.client so each GetContent call returns the
+	// next page. ScrapeRanking now calls s.client directly.
+	urlClient := &pagedStubClient{pages: []string{rankingPage1HTML(), rankingPage2HTML()}}
+	s := New(urlClient, nil, nil) // urlClient == nil → falls back to client
+
+	entryCh, errCh := s.ScrapeRanking(context.Background(), 0) // 0 = all pages
+	entries := drainRanking(t, entryCh, errCh)
+
+	if len(entries) != 3 {
+		t.Fatalf("expected 3 entries across 2 pages, got %d", len(entries))
+	}
+	want := []struct {
+		rank  int
+		title string
+	}{
+		{1, "The Iron Throne"},
+		{2, "Shadow Mage"},
+		{3, "Void Hunter"},
+	}
+	for i, w := range want {
+		if entries[i].Ranking != w.rank || entries[i].Title != w.title {
+			t.Errorf("entry[%d]: got rank=%d title=%q, want rank=%d title=%q",
+				i, entries[i].Ranking, entries[i].Title, w.rank, w.title)
+		}
+	}
+}
+
+// TestScrapeRanking_EmptyPage verifies that a page with no .rank-novels
+// container produces zero entries and closes channels cleanly (no deadlock).
+func TestScrapeRanking_EmptyPage(t *testing.T) {
+	s := newScraper(`<!DOCTYPE html><html><body><div class="no-rankings"></div></body></html>`)
+	entryCh, errCh := s.ScrapeRanking(context.Background(), 1)
+	entries := drainRanking(t, entryCh, errCh)
+
+	if len(entries) != 0 {
+		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.md")
+	if _, err := os.Stat(rankingFile); err != nil {
+		t.Fatalf("ranking.md 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].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)
+		}
+	}
+}
--- a/scraper/internal/novelfire/scraper.go
+++ b/scraper/internal/novelfire/scraper.go
@@ -389,13 +389,10 @@ func (s *Scraper) ScrapeRanking(ctx context.Context, maxPages int) (<-chan scrap
 			pageURL := fmt.Sprintf("%s%s?page=%d", baseURL, rankingPath, page)
 			s.log.Info("scraping ranking page", "page", page, "url", pageURL)

-			// Always use the urlClient for the ranking page — it requires JS rendering.
-			raw, err := s.urlClient.GetContent(ctx, browser.ContentRequest{
-				URL:                 pageURL,
-				WaitFor:             &browser.WaitForSelector{Selector: ".rank-novels", Timeout: 30000},
-				RejectResourceTypes: rejectResourceTypes,
-				GotoOptions:         &browser.GotoOptions{Timeout: 60000},
-				BestAttempt:         true,
+			// The ranking page is fully server-rendered; a direct HTTP GET is
+			// sufficient and avoids the Browserless round-trip overhead.
+			raw, err := s.client.GetContent(ctx, browser.ContentRequest{
+				URL: pageURL,
 			})
 			if err != nil {
 				s.log.Debug("ranking page fetch failed", "page", page, "url", pageURL, "err", err)
--- a/scraper/internal/server/ui.go
+++ b/scraper/internal/server/ui.go
@@ -455,35 +455,30 @@ func (s *Server) handleRankingRefresh(w http.ResponseWriter, r *http.Request) {
 		rankingCh, errCh := s.novel.ScrapeRanking(ctx, maxPages)

 		var rankingItems []writer.RankingItem
-		for {
+		for rankingCh != nil || errCh != nil {
 			select {
 			case meta, ok := <-rankingCh:
 				if !ok {
 					rankingCh = nil
-					continue
+				} else {
+					rankingItems = append(rankingItems, writer.RankingItem{
+						Rank:      meta.Ranking,
+						Slug:      meta.Slug,
+						Title:     meta.Title,
+						Author:    meta.Author,
+						Cover:     meta.Cover,
+						Status:    meta.Status,
+						Genres:    meta.Genres,
+						SourceURL: meta.SourceURL,
+					})
 				}
-				rankingItems = append(rankingItems, writer.RankingItem{
-					Rank:      meta.Ranking,
-					Slug:      meta.Slug,
-					Title:     meta.Title,
-					Author:    meta.Author,
-					Cover:     meta.Cover,
-					Status:    meta.Status,
-					Genres:    meta.Genres,
-					SourceURL: meta.SourceURL,
-				})
 			case err, ok := <-errCh:
 				if !ok {
 					errCh = nil
-					continue
-				}
-				if err != nil {
+				} else if err != nil {
 					s.log.Error("ranking scrape error", "err", err)
 				}
 			}
-			if rankingCh == nil && errCh == nil {
-				break
-			}
 		}

 		if len(rankingItems) > 0 {