diff --git a/.github/workflows/ci-backend.yml b/.github/workflows/ci-backend.yml index 5517456447..b789f38012 100644 --- a/.github/workflows/ci-backend.yml +++ b/.github/workflows/ci-backend.yml @@ -66,13 +66,13 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v9 with: - version: "v2.13.2" + version: "v2.14.0" working-directory: backend/app - name: golangci-lint on example directory uses: golangci/golangci-lint-action@v9 with: - version: "v2.13.2" + version: "v2.14.0" args: --config ../../.golangci.yml working-directory: backend/_example/memory_store diff --git a/backend/app/rest/api/rest.go b/backend/app/rest/api/rest.go index 839427440c..15ad5a645c 100644 --- a/backend/app/rest/api/rest.go +++ b/backend/app/rest/api/rest.go @@ -91,6 +91,10 @@ const hardBodyLimit = 1024 * 64 // limit size of body const openRouteLimiter = 10 // limit for open routes const lastCommentsScope = "last" +// lastVotesScope is carried only by the JSON last comments, the one response under that scope with +// per-user vote data, so a vote does not also drop the feeds +const lastVotesScope = "last-votes" + type commentsWithInfo struct { Comments []store.Comment `json:"comments"` Info store.PostInfo `json:"info"` diff --git a/backend/app/rest/api/rest_private.go b/backend/app/rest/api/rest_private.go index 2590797d28..325732572a 100644 --- a/backend/app/rest/api/rest_private.go +++ b/backend/app/rest/api/rest_private.go @@ -287,7 +287,7 @@ func (s *private) voteCtrl(w http.ResponseWriter, r *http.Request) { rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't vote for comment", code) return } - s.cache.Flush(cache.Flusher(locator.SiteID).Scopes(locator.URL, comment.User.ID)) + s.cache.Flush(cache.Flusher(locator.SiteID).Scopes(locator.URL, comment.User.ID, lastVotesScope)) R.RenderJSON(w, R.JSON{"id": comment.ID, "score": comment.Score}) } diff --git a/backend/app/rest/api/rest_public.go b/backend/app/rest/api/rest_public.go index cbbf15d874..d7bd66b005 100644 --- a/backend/app/rest/api/rest_public.go +++ b/backend/app/rest/api/rest_public.go @@ -198,7 +198,7 @@ func (s *public) lastCommentsCtrl(w http.ResponseWriter, r *http.Request) { return } - key := cache.NewKey(siteID).ID(URLKey(r)).Scopes(lastCommentsScope) + key := cache.NewKey(siteID).ID(URLKeyWithUser(r)).Scopes(lastCommentsScope, lastVotesScope) data, err := s.cache.Get(key, func() ([]byte, error) { comments, e := s.dataService.Last(siteID, limit, sinceTime, rest.GetUserOrEmpty(r)) if e != nil { diff --git a/backend/app/rest/api/rest_public_test.go b/backend/app/rest/api/rest_public_test.go index 30fcbf88f5..5c1852293a 100644 --- a/backend/app/rest/api/rest_public_test.go +++ b/backend/app/rest/api/rest_public_test.go @@ -12,6 +12,7 @@ import ( "os" "strconv" "strings" + "sync/atomic" "testing" "time" @@ -369,6 +370,86 @@ func TestRest_FindUserView(t *testing.T) { assert.Equal(t, id2, comments.Comments[0].ID) } +// TestRest_LastVoteIsPerUser covers the per-user vote field in the last comments response, which +// is cached, so a response built for one user must not be served to another, and a vote has to +// replace what the voter already has cached +func TestRest_LastVoteIsPerUser(t *testing.T) { + // startupT's cache stores nothing, which would hide the defect this covers + lru, err := cache.NewLruCache(cache.NewOpts[[]byte]().MaxKeys(100)) + require.NoError(t, err) + ts, _, teardown := startupT(t, func(srv *Rest) { srv.Cache = cache.NewScache[[]byte](lru) }) + defer teardown() + + const postURL = "https://radio-t.com/blah-last-vote" + id := addComment(t, store.Comment{Text: "vote target", Locator: store.Locator{SiteID: "remark42", URL: postURL}}, ts) + + voteSeenBy := func(tkn string) (vote, score int) { + r, e := http.NewRequest(http.MethodGet, ts.URL+"/api/v1/last/5?site=remark42", http.NoBody) + require.NoError(t, e) + rr, e := sendReq(r, tkn) + require.NoError(t, e) + body, e := io.ReadAll(rr.Body) + require.NoError(t, e) + require.NoError(t, rr.Body.Close()) + require.Equal(t, http.StatusOK, rr.StatusCode, string(body)) + comments := []store.Comment{} + require.NoError(t, json.Unmarshal(body, &comments)) + require.Len(t, comments, 1) + require.Equal(t, id, comments[0].ID) + return comments[0].Vote, comments[0].Score + } + + // the voter has a cached response from before the vote, which the vote has to invalidate + vote, score := voteSeenBy(dev2Token) + require.Equal(t, 0, vote) + require.Equal(t, 0, score) + + // the site feed shows no votes, so a vote must leave it cached + feed := func() { + body, code := get(t, ts.URL+"/api/v1/rss/site?site=remark42") + require.Equal(t, http.StatusOK, code, body) + } + feed() + feedMisses := atomic.LoadInt64(&lru.Misses) + + req, err := http.NewRequest(http.MethodPut, + fmt.Sprintf("%s/api/v1/vote/%s?site=remark42&url=%s&vote=1", ts.URL, id, postURL), http.NoBody) + require.NoError(t, err) + resp, err := sendReq(req, dev2Token) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusOK, resp.StatusCode) + + vote, score = voteSeenBy(dev2Token) + assert.Equal(t, 1, vote, "the voter sees their vote right after casting it") + assert.Equal(t, 1, score, "the score includes the vote right after it is cast") + vote, score = voteSeenBy("") + assert.Equal(t, 0, vote, "an anonymous reader sees no vote") + assert.Equal(t, 1, score) + vote, _ = voteSeenBy(devToken) + assert.Equal(t, 0, vote, "another user sees no vote") + vote, _ = voteSeenBy(dev2Token) + assert.Equal(t, 1, vote, "the voter still sees their vote") + + misses := atomic.LoadInt64(&lru.Misses) + feed() + assert.Equal(t, misses, atomic.LoadInt64(&lru.Misses), "the site feed is served from the cache after a vote") + assert.Greater(t, misses, feedMisses, "misses are counted, so the check above is not vacuous") + + // a new comment still refreshes the cached last comments of every user + addComment(t, store.Comment{Text: "another one", Locator: store.Locator{SiteID: "remark42", URL: postURL + "-2"}}, ts) + r, err := http.NewRequest(http.MethodGet, ts.URL+"/api/v1/last/5?site=remark42", http.NoBody) + require.NoError(t, err) + rr, err := sendReq(r, dev2Token) + require.NoError(t, err) + body, err := io.ReadAll(rr.Body) + require.NoError(t, err) + require.NoError(t, rr.Body.Close()) + comments := []store.Comment{} + require.NoError(t, json.Unmarshal(body, &comments)) + assert.Len(t, comments, 2, "a new comment reaches the voter's cached last comments") +} + func TestRest_Last(t *testing.T) { ts, srv, teardown := startupT(t) defer teardown()