From d5c8091e089ed9b56574ef6488c99aac01a75298 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 17 Mar 2025 09:00:34 +0000 Subject: [PATCH] perf: optimize converting releases to feed items (#7221) - `releasesToFeedItems` is called to convert release structs to feed items, which is then used to render RSS or Atom feeds. - Optimize the loading of attributes for the releases, introduce `ReleaseList` type which uses caching to load repository and publishers. It also no longer loads release attachments and downloads counts as that is not used in feed items. - Optimize the composing of meta by introducing caching, this operation is especially slow when the owner is an organization. - Add unit test (ensures new `LoadAttributes` works correctly). - Add integration test (ensures that feed output is still as expected). Loading https://codeberg.org/forgejo/forgejo/releases.rss reduced from ~15s to ~1s. (It is currently is deployed on codeberg.org) Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7221 Reviewed-by: Otto Co-authored-by: Gusted Co-committed-by: Gusted --- models/repo/release_list.go | 45 +++++++++++++ models/repo/release_list_test.go | 42 ++++++++++++ routers/web/feed/convert.go | 22 ++++--- tests/integration/release_feed_test.go | 91 ++++++++++++++++++++++++++ 4 files changed, 192 insertions(+), 8 deletions(-) create mode 100644 models/repo/release_list.go create mode 100644 models/repo/release_list_test.go create mode 100644 tests/integration/release_feed_test.go diff --git a/models/repo/release_list.go b/models/repo/release_list.go new file mode 100644 index 0000000000..6c33262125 --- /dev/null +++ b/models/repo/release_list.go @@ -0,0 +1,45 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package repo + +import ( + "context" + + user_model "code.gitea.io/gitea/models/user" +) + +type ReleaseList []*Release + +// LoadAttributes loads the repository and publisher for the releases. +func (r ReleaseList) LoadAttributes(ctx context.Context) error { + repoCache := make(map[int64]*Repository) + userCache := make(map[int64]*user_model.User) + + for _, release := range r { + var err error + repo, ok := repoCache[release.RepoID] + if !ok { + repo, err = GetRepositoryByID(ctx, release.RepoID) + if err != nil { + return err + } + repoCache[release.RepoID] = repo + } + release.Repo = repo + + publisher, ok := userCache[release.PublisherID] + if !ok { + publisher, err = user_model.GetUserByID(ctx, release.PublisherID) + if err != nil { + if !user_model.IsErrUserNotExist(err) { + return err + } + publisher = user_model.NewGhostUser() + } + userCache[release.PublisherID] = publisher + } + release.Publisher = publisher + } + return nil +} diff --git a/models/repo/release_list_test.go b/models/repo/release_list_test.go new file mode 100644 index 0000000000..cbd77683d0 --- /dev/null +++ b/models/repo/release_list_test.go @@ -0,0 +1,42 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package repo + +import ( + "testing" + + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReleaseListLoadAttributes(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + releases := ReleaseList{&Release{ + RepoID: 1, + PublisherID: 1, + }, &Release{ + RepoID: 2, + PublisherID: 2, + }, &Release{ + RepoID: 1, + PublisherID: 2, + }, &Release{ + RepoID: 2, + PublisherID: 1, + }} + + require.NoError(t, releases.LoadAttributes(t.Context())) + + assert.EqualValues(t, 1, releases[0].Repo.ID) + assert.EqualValues(t, 1, releases[0].Publisher.ID) + assert.EqualValues(t, 2, releases[1].Repo.ID) + assert.EqualValues(t, 2, releases[1].Publisher.ID) + assert.EqualValues(t, 1, releases[2].Repo.ID) + assert.EqualValues(t, 2, releases[2].Publisher.ID) + assert.EqualValues(t, 2, releases[3].Repo.ID) + assert.EqualValues(t, 1, releases[3].Publisher.ID) +} diff --git a/routers/web/feed/convert.go b/routers/web/feed/convert.go index d3d01a87e0..5f7687d803 100644 --- a/routers/web/feed/convert.go +++ b/routers/web/feed/convert.go @@ -298,14 +298,14 @@ func GetFeedType(name string, req *http.Request) (bool, string, string) { return false, name, "" } -// feedActionsToFeedItems convert gitea's Repo's Releases to feeds Item -func releasesToFeedItems(ctx *context.Context, releases []*repo_model.Release) (items []*feeds.Item, err error) { - for _, rel := range releases { - err := rel.LoadAttributes(ctx) - if err != nil { - return nil, err - } +// feedActionsToFeedItems convert repository releases into feed items. +func releasesToFeedItems(ctx *context.Context, releases repo_model.ReleaseList) (items []*feeds.Item, err error) { + if err := releases.LoadAttributes(ctx); err != nil { + return nil, err + } + composeCache := make(map[int64]map[string]string) + for _, rel := range releases { var title string var content template.HTML @@ -315,13 +315,19 @@ func releasesToFeedItems(ctx *context.Context, releases []*repo_model.Release) ( title = rel.Title } + metas, ok := composeCache[rel.RepoID] + if !ok { + metas = rel.Repo.ComposeMetas(ctx) + composeCache[rel.RepoID] = metas + } + link := &feeds.Link{Href: rel.HTMLURL()} content, err = markdown.RenderString(&markup.RenderContext{ Ctx: ctx, Links: markup.Links{ Base: rel.Repo.Link(), }, - Metas: rel.Repo.ComposeMetas(ctx), + Metas: metas, }, rel.Note) if err != nil { return nil, err diff --git a/tests/integration/release_feed_test.go b/tests/integration/release_feed_test.go new file mode 100644 index 0000000000..fd1f42c3cd --- /dev/null +++ b/tests/integration/release_feed_test.go @@ -0,0 +1,91 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package integration + +import ( + "net/http" + "regexp" + "testing" + + "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" +) + +func TestReleaseFeed(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + normalize := func(body string) string { + // Remove port. + body = regexp.MustCompile(`localhost:\d+`).ReplaceAllString(body, "localhost") + // date is timezone dependent. + body = regexp.MustCompile(`.*`).ReplaceAllString(body, "") + body = regexp.MustCompile(`.*`).ReplaceAllString(body, "") + return body + } + t.Run("RSS feed", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + resp := MakeRequest(t, NewRequest(t, "GET", "/user2/repo1/releases.rss"), http.StatusOK) + assert.EqualValues(t, ` + + Releases for user2/repo1 + http://localhost/user2/repo1/release + + + + pre-release + http://localhost/user2/repo1/releases/tag/v1.0 + + some text for a pre release

+]]> + user2 + 5: http://localhost/user2/repo1/releases/tag/v1.0 + + + + testing-release + http://localhost/user2/repo1/releases/tag/v1.1 + + user2 + 1: http://localhost/user2/repo1/releases/tag/v1.1 + + + +`, normalize(resp.Body.String())) + }) + + t.Run("Atom feed", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + resp := MakeRequest(t, NewRequest(t, "GET", "/user2/repo1/releases.atom"), http.StatusOK) + assert.EqualValues(t, ` + Releases for user2/repo1 + http://localhost/user2/repo1/release + + + + pre-release + + 5: http://localhost/user2/repo1/releases/tag/v1.0 + <p dir="auto">some text for a pre release</p> + + + user2 + user2@noreply.example.org + + + + testing-release + + 1: http://localhost/user2/repo1/releases/tag/v1.1 + + + user2 + user2@noreply.example.org + + +`, normalize(resp.Body.String())) + }) +}