From eefd5bbc57f294c0aa949a9cc21bd07038359c84 Mon Sep 17 00:00:00 2001 From: Miroslav Cvetic Date: Thu, 29 May 2025 10:58:06 +0200 Subject: [PATCH 1/4] fix: update before getRepo --- pkg/handler/http.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/handler/http.go b/pkg/handler/http.go index 6e4b540..73f1b3c 100644 --- a/pkg/handler/http.go +++ b/pkg/handler/http.go @@ -75,6 +75,10 @@ func (h *HTTP) ServeHTTP(w http.ResponseWriter, r *http.Request) { route := Route(strings.TrimPrefix(r.URL.Path, h.basePath+"/")) if route == RouteGetRepo { + resp := h.repo.Update() + if !resp.Success { + h.l.Warn("repo update failed — serving previously cached data", zap.String("reason", resp.ErrorMessage)) + } h.repo.WriteRepoBytes(w) w.Header().Set("Content-Type", "application/json") return From 74d2ff75f542521f223071a29a9671c7e75bada8 Mon Sep 17 00:00:00 2001 From: Miroslav Cvetic Date: Thu, 29 May 2025 11:56:20 +0200 Subject: [PATCH 2/4] fix: add get repo test when update fails --- client/client_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/client/client_test.go b/client/client_test.go index d36585a..5787183 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1,12 +1,14 @@ package client_test import ( + "net/http/httptest" "sync" "testing" "time" "github.com/foomo/contentserver/client" "github.com/foomo/contentserver/content" + "github.com/foomo/contentserver/pkg/handler" "github.com/foomo/contentserver/pkg/repo" "github.com/foomo/contentserver/pkg/repo/mock" "github.com/stretchr/testify/assert" @@ -163,6 +165,53 @@ func initRepo(tb testing.TB, l *zap.Logger) *repo.Repo { return r } +func TestGetRepo_UpdateFails(t *testing.T) { + t.Helper() + t.Parallel() + + l := zaptest.NewLogger(t) + + // Step 1: create a working repo and let it populate history + testRepoServer, varDir := mock.GetMockData(t) + historyDir := varDir + + workingRepo := repo.New(l, + testRepoServer.URL+"/repo-two-dimensions.json", + repo.NewHistory(l, repo.HistoryWithHistoryDir(historyDir)), + ) + go func() { + if err := workingRepo.Start(t.Context()); err != nil { + t.Errorf("workingRepo.Start failed: %v", err) + } + }() + + // Give it time to persist JSON + time.Sleep(300 * time.Millisecond) + + // Step 2: create a new repo with a broken upstream, but reusing the same history + brokenRepo := repo.New(l, + "http://localhost:9999/this-will-fail-non-existent.json", // force failure + repo.NewHistory(l, repo.HistoryWithHistoryDir(historyDir)), + ) + go func() { + if err := brokenRepo.Start(t.Context()); err != nil { + t.Errorf("brokenRepo.Start failed: %v", err) + } + }() + + // Step 3: serve it + server := httptest.NewServer(handler.NewHTTP(l, brokenRepo)) + defer server.Close() + + client, err := client.NewHTTPClient(server.URL + pathContentserver) + require.NoError(t, err) + + // Step 4: trigger GetRepo, which will log warning and fall back to existing file + result, err := client.GetRepo(t.Context()) + require.NoError(t, err) + assert.NotEmpty(t, result, "expected fallback repo content from WriteRepoBytes") +} + // func dump(t *testing.T, v interface{}) { // t.Helper() // jsonBytes, err := json.MarshalIndent(v, "", " ") From fe1d9de6f153f6c6826b8f62e23ad5bf01da82a7 Mon Sep 17 00:00:00 2001 From: Daniel Thomas Date: Fri, 13 Jun 2025 22:21:59 +0200 Subject: [PATCH 3/4] fix: update persisted json after updating the repo using the poll routine --- pkg/handler/http.go | 4 ---- pkg/repo/loader.go | 9 +++++++++ pkg/repo/repo.go | 2 ++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/handler/http.go b/pkg/handler/http.go index 73f1b3c..6e4b540 100644 --- a/pkg/handler/http.go +++ b/pkg/handler/http.go @@ -75,10 +75,6 @@ func (h *HTTP) ServeHTTP(w http.ResponseWriter, r *http.Request) { route := Route(strings.TrimPrefix(r.URL.Path, h.basePath+"/")) if route == RouteGetRepo { - resp := h.repo.Update() - if !resp.Success { - h.l.Warn("repo update failed — serving previously cached data", zap.String("reason", resp.ErrorMessage)) - } h.repo.WriteRepoBytes(w) w.Header().Set("Content-Type", "application/json") return diff --git a/pkg/repo/loader.go b/pkg/repo/loader.go index 6910545..010ec04 100644 --- a/pkg/repo/loader.go +++ b/pkg/repo/loader.go @@ -305,6 +305,15 @@ func (r *Repo) update(ctx context.Context) (repoRuntime int64, err error) { if r.poll { r.pollVersion = repoURL } + + // Persist the JSON buffer after successful update + if err := r.history.Add(r.JSONBufferBytes()); err != nil { + r.l.Error("Failed to persist repo after update", zap.Error(err)) + metrics.HistoryPersistFailedCounter.WithLabelValues().Inc() + } else { + r.l.Info("Successfully persisted repo after update") + } + return repoRuntime, nil } diff --git a/pkg/repo/repo.go b/pkg/repo/repo.go index 111218d..df8a0ff 100644 --- a/pkg/repo/repo.go +++ b/pkg/repo/repo.go @@ -285,6 +285,8 @@ func (r *Repo) Update() (updateResponse *responses.Update) { if historyErr != nil { r.l.Error("Could not persist current repo in history", zap.Error(historyErr)) metrics.HistoryPersistFailedCounter.WithLabelValues().Inc() + } else { + r.l.Info("Successfully persisted current repo to history") } // add some stats for _, dimension := range r.Directory() { From d5498dde7338cfb1c7be39d15c2f40edb02836c1 Mon Sep 17 00:00:00 2001 From: Daniel Thomas Date: Fri, 13 Jun 2025 23:02:19 +0200 Subject: [PATCH 4/4] fix: remove unuseful unit test --- client/client_test.go | 49 ------------------------------------------- 1 file changed, 49 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 5787183..d36585a 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1,14 +1,12 @@ package client_test import ( - "net/http/httptest" "sync" "testing" "time" "github.com/foomo/contentserver/client" "github.com/foomo/contentserver/content" - "github.com/foomo/contentserver/pkg/handler" "github.com/foomo/contentserver/pkg/repo" "github.com/foomo/contentserver/pkg/repo/mock" "github.com/stretchr/testify/assert" @@ -165,53 +163,6 @@ func initRepo(tb testing.TB, l *zap.Logger) *repo.Repo { return r } -func TestGetRepo_UpdateFails(t *testing.T) { - t.Helper() - t.Parallel() - - l := zaptest.NewLogger(t) - - // Step 1: create a working repo and let it populate history - testRepoServer, varDir := mock.GetMockData(t) - historyDir := varDir - - workingRepo := repo.New(l, - testRepoServer.URL+"/repo-two-dimensions.json", - repo.NewHistory(l, repo.HistoryWithHistoryDir(historyDir)), - ) - go func() { - if err := workingRepo.Start(t.Context()); err != nil { - t.Errorf("workingRepo.Start failed: %v", err) - } - }() - - // Give it time to persist JSON - time.Sleep(300 * time.Millisecond) - - // Step 2: create a new repo with a broken upstream, but reusing the same history - brokenRepo := repo.New(l, - "http://localhost:9999/this-will-fail-non-existent.json", // force failure - repo.NewHistory(l, repo.HistoryWithHistoryDir(historyDir)), - ) - go func() { - if err := brokenRepo.Start(t.Context()); err != nil { - t.Errorf("brokenRepo.Start failed: %v", err) - } - }() - - // Step 3: serve it - server := httptest.NewServer(handler.NewHTTP(l, brokenRepo)) - defer server.Close() - - client, err := client.NewHTTPClient(server.URL + pathContentserver) - require.NoError(t, err) - - // Step 4: trigger GetRepo, which will log warning and fall back to existing file - result, err := client.GetRepo(t.Context()) - require.NoError(t, err) - assert.NotEmpty(t, result, "expected fallback repo content from WriteRepoBytes") -} - // func dump(t *testing.T, v interface{}) { // t.Helper() // jsonBytes, err := json.MarshalIndent(v, "", " ")