Merge: main -> env/prod (#240 参拝時のGitHubイベント取得を100件から300件に広げる)

Author: claude

app/functions-go/sanpai.go

@@ -129,15 +129,82 @@ type feedItem struct {
 	Event githubEvent
 }
 
-// fetchGitHubFeed は GitHub Events API から公開アクティビティを取得する
-// (Node版 get_feed と同一のURL・クエリパラメータ)。
-func fetchGitHubFeed(ctx context.Context, username string) ([]feedItem, error) {
+// firestoreBatchMaxWrites は Firestore の書き込みバッチ1回あたりの上限。
+const firestoreBatchMaxWrites = 500
+
+// githubFeedPerPage は1ページあたりの取得件数(Events APIの上限)。
+const githubFeedPerPage = 100
+
+// githubFeedMaxPages は遡るページ数の上限。Events API は最大300件(100件×3ページ)
+// までしか返さないため、それ以上は要求しても無駄。
+const githubFeedMaxPages = 3
+
+// fetchGitHubFeed は GitHub Events API から公開アクティビティを取得する。
+//
+// 以前は1ページ(100件)だけ取って終わりだったため、前回の参拝から公開イベントが
+// 100件を超えると超過分を永久に取りこぼしていた(90日を過ぎると GitHub からも
+// 返らなくなるため、後から拾い直せない)。since(前回の参拝時刻)まで遡って取る。
+//
+// 取得済みの範囲に入ったページで打ち切るので、普段の参拝は1リクエストのまま。
+// 全ページ舐めるのは初回参拝や、久しぶりの参拝で100件を超えている人だけ。
+func fetchGitHubFeed(ctx context.Context, username string, since time.Time) ([]feedItem, error) {
+	var all []feedItem
+	// ページの取得中に新しいイベントが増えると窓がずれ、同じイベントが2つの
+	// ページに載ることがある。重複したまま返すと、同一バッチ内で同じドキュメントへ
+	// 2回書くことになり Firestore に弾かれる(参拝そのものが失敗する)うえ、
+	// ポイントと能力値も二重計上になる。1ページだけ取っていた頃は GitHub が
+	// ページ内のID一意を保証していたので起きなかった。
+	seen := make(map[string]bool)
+	for page := 1; page <= githubFeedMaxPages; page++ {
+		items, err := fetchGitHubFeedPage(ctx, username, page)
+		if err != nil {
+			// 途中のページで失敗したら参拝ごと失敗させる。ここで取れた分だけで
+			// 進めると last_sanpai が進んでしまい、取れなかったイベントを
+			// 二度と拾えなくなる(取りこぼしを止めるための変更なので本末転倒)。
+			// 失敗しても last_sanpai は書き換えないため、やり直せる。
+			return nil, err
+		}
+		for _, it := range items {
+			if it.Event.ID != "" && seen[it.Event.ID] {
+				continue
+			}
+			seen[it.Event.ID] = true
+			all = append(all, it)
+		}
+		// 最終ページ(埋まっていない)ならこれ以上は無い。
+		if len(items) < githubFeedPerPage {
+			break
+		}
+		// 集計済みの時刻まで遡れたら十分。
+		if reachedSince(items, since) {
+			break
+		}
+	}
+	return all, nil
+}
+
+// reachedSince はページ内に since 以前のイベントが含まれるかを返す(純関数)。
+// Events API は新しい順に返すので、1件でも含まれていればそれ以上遡る必要はない。
+func reachedSince(items []feedItem, since time.Time) bool {
+	for _, it := range items {
+		t, err := time.Parse(time.RFC3339, it.Event.CreatedAt)
+		if err != nil {
+			continue
+		}
+		if !t.After(since) {
+			return true
+		}
+	}
+	return false
+}
+
+func fetchGitHubFeedPage(ctx context.Context, username string, page int) ([]feedItem, error) {
 	clientID := os.Getenv("GITHUB_CLIENT_ID")
 	clientSecret := os.Getenv("GITHUB_CLIENT_SECRET")
 
 	reqURL := fmt.Sprintf(
-		"%s/users/%s/events/public?per_page=100",
-		githubAPIBaseURL, url.PathEscape(username),
+		"%s/users/%s/events/public?per_page=%d&page=%d",
+		githubAPIBaseURL, url.PathEscape(username), githubFeedPerPage, page,
 	)
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
 	if err != nil {
@@ -323,19 +390,20 @@ func runSanpai(ctx context.Context, w http.ResponseWriter, client *firestore.Cli
 		}
 	}
 
-	feed, err := fetchGitHubFeed(ctx, userData.ScreenName)
-	if err != nil {
-		// Node版はGitHub取得失敗時に例外化し外側catchで "missing server error." になる。
-		return err
-	}
-
+	// 前回の参拝時刻。ここまで遡って取得し、ここより新しいイベントだけを集計する。
 	var since time.Time
 	if hasLastSanpai {
 		since = userData.LastSanpai
 	} else {
 		since, _ = time.Parse(time.RFC3339, "2008-04-01T00:00:00Z")
 	}
 
+	feed, err := fetchGitHubFeed(ctx, userData.ScreenName, since)
+	if err != nil {
+		// Node版はGitHub取得失敗時に例外化し外側catchで "missing server error." になる。
+		return err
+	}
+
 	var splited []feedItem
 	for _, it := range feed {
 		created, err := time.Parse(time.RFC3339, it.Event.CreatedAt)
@@ -360,19 +428,29 @@ func runSanpai(ctx context.Context, w http.ResponseWriter, client *firestore.Cli
 	}
 
 	// アクティビティ反映
-	batch := client.Batch()
+	// Firestore のバッチは1回あたり500件までなので分割して書く。
+	// 1ページ(100件)しか取っていなかった頃は上限に当たりようがなかったが、
+	// 300件まで遡るようになった(#239)ので余裕が減っている。ページ数を増やしても
+	// ここが破綻しないよう、件数に依らない形にしておく。
 	activityColl := userRef.Collection("github_activities")
-	for _, it := range splited {
-		docRef := activityColl.Doc(it.Event.ID)
-		batch.Set(docRef, map[string]interface{}{
-			"id":         it.Event.ID,
-			"type":       it.Event.Type,
-			"created_at": it.Event.CreatedAt,
-			"raw":        string(it.Raw),
-		})
-	}
-	if _, err := batch.Commit(ctx); err != nil {
-		return err
+	for start := 0; start < len(splited); start += firestoreBatchMaxWrites {
+		end := start + firestoreBatchMaxWrites
+		if end > len(splited) {
+			end = len(splited)
+		}
+		batch := client.Batch()
+		for _, it := range splited[start:end] {
+			docRef := activityColl.Doc(it.Event.ID)
+			batch.Set(docRef, map[string]interface{}{
+				"id":         it.Event.ID,
+				"type":       it.Event.Type,
+				"created_at": it.Event.CreatedAt,
+				"raw":        string(it.Raw),
+			})
+		}
+		if _, err := batch.Commit(ctx); err != nil {
+			return err
+		}
 	}
 
 	// 意図的な省略: Node版にはここに「2022/1/1〜1/3はポイント3倍」という

app/functions-go/sanpai_test.go

@@ -8,6 +8,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -102,7 +103,7 @@ func TestFetchGitHubFeed_SendsBasicAuthHeader(t *testing.T) {
 	defer srv.Close()
 	withMockGitHub(t, srv)
 
-	if _, err := fetchGitHubFeed(context.Background(), "octocat"); err != nil {
+	if _, err := fetchGitHubFeed(context.Background(), "octocat", time.Time{}); err != nil {
 		t.Fatalf("fetchGitHubFeed: %v", err)
 	}
 }
@@ -122,7 +123,7 @@ func TestFetchGitHubFeed_NoCredentials(t *testing.T) {
 	defer srv.Close()
 	withMockGitHub(t, srv)
 
-	if _, err := fetchGitHubFeed(context.Background(), "octocat"); err != nil {
+	if _, err := fetchGitHubFeed(context.Background(), "octocat", time.Time{}); err != nil {
 		t.Fatalf("fetchGitHubFeed: %v", err)
 	}
 }
@@ -356,3 +357,175 @@ func TestSanpaiHandler_MissingAuthorizationHeader(t *testing.T) {
 		t.Errorf("status = %d, want 401", rec.Code)
 	}
 }
+
+// events を新しい順に n 件返すモックページを組み立てる(created_at は base から1分ずつ遡る)。
+func mockEventsPage(base time.Time, offset, n int) string {
+	items := make([]string, 0, n)
+	for i := 0; i < n; i++ {
+		at := base.Add(-time.Duration(offset+i) * time.Minute).UTC().Format(time.RFC3339)
+		items = append(items, fmt.Sprintf(
+			`{"id":"e%d","type":"PushEvent","created_at":%q,"repo":{"name":"o/r"},"payload":{}}`,
+			offset+i, at))
+	}
+	return "[" + strings.Join(items, ",") + "]"
+}
+
+// 普段の参拝(前回から100件も動いていない)では1ページで打ち切ること。
+// ここが増えると全ユーザーのGitHub API呼び出しが毎回3倍になる。
+func TestFetchGitHubFeed_StopsAtFirstPageWhenCaughtUp(t *testing.T) {
+	now := time.Now()
+	since := now.Add(-30 * time.Minute) // 30件目より新しい
+	var pages int32
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		atomic.AddInt32(&pages, 1)
+		w.Header().Set("Content-Type", "application/json")
+		// 1ページ丸ごと(100件)返す。ただし since 以前の分が含まれる。
+		_, _ = w.Write([]byte(mockEventsPage(now, 0, githubFeedPerPage)))
+	}))
+	defer srv.Close()
+	withMockGitHub(t, srv)
+
+	items, err := fetchGitHubFeed(context.Background(), "octocat", since)
+	if err != nil {
+		t.Fatalf("fetchGitHubFeed: %v", err)
+	}
+	if got := atomic.LoadInt32(&pages); got != 1 {
+		t.Errorf("取得済みの範囲に達したら打ち切るべき: %d ページ取得した", got)
+	}
+	if len(items) != githubFeedPerPage {
+		t.Errorf("件数 = %d, want %d", len(items), githubFeedPerPage)
+	}
+}
+
+// 100件を超えて動いている場合は上限(3ページ=300件)まで遡ること。
+// これが無いと超過分を永久に取りこぼす(#239)。
+func TestFetchGitHubFeed_PaginatesUpToMaxPages(t *testing.T) {
+	now := time.Now()
+	since := now.Add(-365 * 24 * time.Hour) // どのページにも到達しない
+	var pages int32
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		p := atomic.AddInt32(&pages, 1)
+		if got := r.URL.Query().Get("page"); got != fmt.Sprint(p) {
+			t.Errorf("page パラメータ = %q, want %d", got, p)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(mockEventsPage(now, int(p-1)*githubFeedPerPage, githubFeedPerPage)))
+	}))
+	defer srv.Close()
+	withMockGitHub(t, srv)
+
+	items, err := fetchGitHubFeed(context.Background(), "octocat", since)
+	if err != nil {
+		t.Fatalf("fetchGitHubFeed: %v", err)
+	}
+	if got := atomic.LoadInt32(&pages); got != githubFeedMaxPages {
+		t.Errorf("ページ数 = %d, want %d (Events APIの上限)", got, githubFeedMaxPages)
+	}
+	want := githubFeedPerPage * githubFeedMaxPages
+	if len(items) != want {
+		t.Errorf("件数 = %d, want %d", len(items), want)
+	}
+}
+
+// 埋まっていないページが来たらそこで終わり(存在しないページを叩かない)。
+func TestFetchGitHubFeed_StopsOnShortPage(t *testing.T) {
+	now := time.Now()
+	var pages int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		p := atomic.AddInt32(&pages, 1)
+		w.Header().Set("Content-Type", "application/json")
+		if p == 1 {
+			_, _ = w.Write([]byte(mockEventsPage(now, 0, githubFeedPerPage)))
+			return
+		}
+		_, _ = w.Write([]byte(mockEventsPage(now, githubFeedPerPage, 3)))
+	}))
+	defer srv.Close()
+	withMockGitHub(t, srv)
+
+	items, err := fetchGitHubFeed(context.Background(), "octocat", time.Time{})
+	if err != nil {
+		t.Fatalf("fetchGitHubFeed: %v", err)
+	}
+	if got := atomic.LoadInt32(&pages); got != 2 {
+		t.Errorf("ページ数 = %d, want 2 (2ページ目が埋まっていないので打ち切る)", got)
+	}
+	if len(items) != githubFeedPerPage+3 {
+		t.Errorf("件数 = %d, want %d", len(items), githubFeedPerPage+3)
+	}
+}
+
+func TestReachedSince(t *testing.T) {
+	now := time.Now()
+	items := []feedItem{
+		{Event: githubEvent{CreatedAt: now.Add(-1 * time.Minute).UTC().Format(time.RFC3339)}},
+		{Event: githubEvent{CreatedAt: now.Add(-10 * time.Minute).UTC().Format(time.RFC3339)}},
+		{Event: githubEvent{CreatedAt: "壊れた値"}},
+	}
+	if !reachedSince(items, now.Add(-5*time.Minute)) {
+		t.Errorf("since 以前の要素があるので true のはず")
+	}
+	if reachedSince(items, now.Add(-60*time.Minute)) {
+		t.Errorf("すべて since より新しいので false のはず")
+	}
+	if reachedSince(nil, now) {
+		t.Errorf("空なら false")
+	}
+}
+
+// ページの取得中に新しいイベントが増えると窓がずれ、同じイベントが2つのページに
+// 載ることがある。重複したまま返すと、同一バッチ内で同じドキュメントへ2回書く
+// ことになり Firestore に弾かれる(参拝そのものが失敗する)うえ、ポイントと
+// 能力値も二重計上になる。
+func TestFetchGitHubFeed_DedupesAcrossPages(t *testing.T) {
+	now := time.Now()
+	var pages int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		p := atomic.AddInt32(&pages, 1)
+		w.Header().Set("Content-Type", "application/json")
+		// 2ページとも同じ offset から返す = 全件重複する状況を作る。
+		_, _ = w.Write([]byte(mockEventsPage(now, 0, githubFeedPerPage)))
+		_ = p
+	}))
+	defer srv.Close()
+	withMockGitHub(t, srv)
+
+	items, err := fetchGitHubFeed(context.Background(), "octocat", time.Time{})
+	if err != nil {
+		t.Fatalf("fetchGitHubFeed: %v", err)
+	}
+	if len(items) != githubFeedPerPage {
+		t.Errorf("重複を除いた件数 = %d, want %d", len(items), githubFeedPerPage)
+	}
+	seen := map[string]bool{}
+	for _, it := range items {
+		if seen[it.Event.ID] {
+			t.Fatalf("重複したイベントIDが残っている: %s", it.Event.ID)
+		}
+		seen[it.Event.ID] = true
+	}
+}
+
+// 途中のページで失敗したら参拝ごと失敗させる(部分的に進めると last_sanpai が
+// 進んで、取れなかったイベントを二度と拾えなくなる)。
+func TestFetchGitHubFeed_FailsWhenLaterPageFails(t *testing.T) {
+	now := time.Now()
+	var pages int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		p := atomic.AddInt32(&pages, 1)
+		if p == 1 {
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(mockEventsPage(now, 0, githubFeedPerPage)))
+			return
+		}
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer srv.Close()
+	withMockGitHub(t, srv)
+
+	if _, err := fetchGitHubFeed(context.Background(), "octocat", time.Time{}); err == nil {
+		t.Errorf("2ページ目が失敗したらエラーを返すべき(部分的に進めない)")
+	}
+}