oss/forgejo
oss/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2025-10-10 19:32:02 +00:00
Code Wiki Activity

[v11.0/forgejo] fix: package cleanup rules are not applied when there are more than 200 packages (depends on MAX_RESPONSE_ITEMS) (#9219) (#9232)

Browse source 
backport of #9219

---

Before it has only processed the newest 200 (or 50 for default `MAX_RESPONSE_ITEMS: 50`) versions.
After it processes all versions.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9219
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
Co-committed-by: Michael Kriese <michael.kriese@visualon.de>
(cherry picked from commit c697de9517)

Conflicts:
	tests/integration/api_packages_test.go

Trivial import conflict and missing helper addition

<!--start release-notes-assistant-->

## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Bug fixes
  - [PR](https://codeberg.org/forgejo/forgejo/pulls/9232): <!--number 9232 --><!--line 0 --><!--description Zml4OiBwYWNrYWdlIGNsZWFudXAgcnVsZXMgYXJlIG5vdCBhcHBsaWVkIHdoZW4gdGhlcmUgYXJlIG1vcmUgdGhhbiAyMDAgcGFja2FnZXMgKGRlcGVuZHMgb24gYE1BWF9SRVNQT05TRV9JVEVNU2ApICgjOTIxOSk=-->fix: package cleanup rules are not applied when there are more than 200 packages (depends on `MAX_RESPONSE_ITEMS`) (#9219)<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9232
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
This commit is contained in:
Michael Kriese 2025-09-11 15:02:53 +02:00 • committed by Earl Warren
parent aab1813acb
commit 5c5ed7b0b7
4 changed files with 42 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

4
routers/web/shared/packages/packages.go
View file

@ -9,7 +9,6 @@ import (
"net/http"
"time"
"forgejo.org/models/db"
packages_model "forgejo.org/models/packages"
repo_model "forgejo.org/models/repo"
user_model "forgejo.org/models/user"
@ -168,13 +167,12 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) {
PackageID: p.ID,
IsInternal: optional.Some(false),
Sort: packages_model.SortCreatedDesc,
Paginator: db.NewAbsoluteListOptions(pcr.KeepCount, 200),
})
if err != nil {
ctx.ServerError("SearchVersions", err)
return
}
for _, pv := range pvs {
for _, pv := range pvs[pcr.KeepCount:] {
if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil {
ctx.ServerError("ShouldBeSkipped", err)
return

3
services/packages/cleanup/cleanup.go
View file

@ -64,13 +64,12 @@ func ExecuteCleanupRules(outerCtx context.Context) error {
PackageID: p.ID,
IsInternal: optional.Some(false),
Sort: packages_model.SortCreatedDesc,
Paginator: db.NewAbsoluteListOptions(pcr.KeepCount, 200),
})
if err != nil {
return fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err)
}
versionDeleted := false
for _, pv := range pvs {
for _, pv := range pvs[pcr.KeepCount:] {
if pcr.Type == packages_model.TypeContainer {
if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil {
return fmt.Errorf("CleanupRule [%d]: container.ShouldBeSkipped failed: %w", pcr.ID, err)

26
tests/integration/api_packages_test.go
View file

@ -19,8 +19,10 @@ import (
unit_model "forgejo.org/models/unit"
"forgejo.org/models/unittest"
user_model "forgejo.org/models/user"
"forgejo.org/modules/container"
"forgejo.org/modules/setting"
api "forgejo.org/modules/structs"
"forgejo.org/modules/translation"
"forgejo.org/modules/util"
packages_service "forgejo.org/services/packages"
packages_cleanup_service "forgejo.org/services/packages/cleanup"
@ -527,6 +529,9 @@ func TestPackageCleanup(t *testing.T) {
t.Run("CleanupRules", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
locale := translation.NewLocale("en-US")
session := loginUser(t, user.Name)
type version struct {
Version string
ShouldExist bool
@ -648,17 +653,34 @@ func TestPackageCleanup(t *testing.T) {
pcr, err := packages_model.InsertCleanupRule(db.DefaultContext, c.Rule)
require.NoError(t, err)
req := NewRequest(t, "GET", fmt.Sprintf("/user/settings/packages/rules/%d/preview", pcr.ID))
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
toDelete := container.FilterSlice(c.Versions, func(v version) (version, bool) {
return v, !v.ShouldExist
})
deletedCount := len(toDelete)
// disabled rule would delete everything
if !pcr.Enabled {
deletedCount = 1
htmlDoc.AssertSelection(t, htmlDoc.FindByText(fmt.Sprintf("a[href='/%s/-/packages/generic/package/keep']", user.Name), "keep"), true)
}
htmlDoc.AssertSelection(t, htmlDoc.FindByText("p", locale.TrString("packages.owner.settings.cleanuprules.preview.overview", deletedCount)), true)
err = packages_cleanup_service.CleanupTask(db.DefaultContext, duration)
require.NoError(t, err)
for _, v := range c.Versions {
pv, err := packages_model.GetVersionByNameAndVersion(db.DefaultContext, user.ID, packages_model.TypeGeneric, "package", v.Version)
if v.ShouldExist {
require.NoError(t, err)
require.NoError(t, err, `version "%s" should exist`, v.Version)
err = packages_service.DeletePackageVersionAndReferences(db.DefaultContext, pv)
require.NoError(t, err)
} else {
require.ErrorIs(t, err, packages_model.ErrPackageNotExist)
require.ErrorIs(t, err, packages_model.ErrPackageNotExist, `version "%s" should not exist`, v.Version)
}
}

16
tests/integration/html_helper.go
View file

@ -76,11 +76,27 @@ func (doc *HTMLDoc) Find(selector string) *goquery.Selection {
return doc.doc.Find(selector)
}
// FindByText gets all elements by selector that also has the given text
func (doc *HTMLDoc) FindByText(selector, text string) *goquery.Selection {
return doc.doc.Find(selector).FilterFunction(func(i int, s *goquery.Selection) bool {
return s.Text() == text
})
}
// GetCSRF for getting CSRF token value from input
func (doc *HTMLDoc) GetCSRF() string {
return doc.GetInputValueByName("_csrf")
}
// AssertSelection check if selection exists or does not exist depending on checkExists
func (doc *HTMLDoc) AssertSelection(t testing.TB, selection *goquery.Selection, checkExists bool) {
if checkExists {
assert.Equal(t, 1, selection.Length())
} else {
assert.Equal(t, 0, selection.Length())
}
}
// AssertElement check if element by selector exists or does not exist depending on checkExists
func (doc *HTMLDoc) AssertElement(t testing.TB, selector string, checkExists bool) {
sel := doc.doc.Find(selector)