From 9011d6d5f17e6e1e4dbf735e6087c79811b30eef Mon Sep 17 00:00:00 2001 From: yeyuanjie Date: Sat, 7 Jun 2025 13:59:29 +0800 Subject: [PATCH 01/12] when using rules to delete packages, remove unclean bugs --- models/packages/package_version.go | 4 + routers/web/shared/packages/packages.go | 84 ++++++++++-------- services/packages/cleanup/cleanup.go | 112 +++++++++++++----------- 3 files changed, 116 insertions(+), 84 deletions(-) diff --git a/models/packages/package_version.go b/models/packages/package_version.go index bb7fd895f81da..f5a7a5021b03f 100644 --- a/models/packages/package_version.go +++ b/models/packages/package_version.go @@ -180,6 +180,7 @@ type PackageSearchOptions struct { RepoID int64 Type Type PackageID int64 + LtVersionID int64 Name SearchValue // only results with the specific name are found Version SearchValue // only results with the specific version are found Properties map[string]string // only results are found which contain all listed version properties with the specific value @@ -210,6 +211,9 @@ func (opts *PackageSearchOptions) ToConds() builder.Cond { if opts.PackageID != 0 { cond = cond.And(builder.Eq{"package.id": opts.PackageID}) } + if opts.LtVersionID != 0 { + cond = cond.And(builder.Lt{"package_version.id": opts.LtVersionID}) + } if opts.Name.Value != "" { if opts.Name.ExactMatch { cond = cond.And(builder.Eq{"package.lower_name": strings.ToLower(opts.Name.Value)}) diff --git a/routers/web/shared/packages/packages.go b/routers/web/shared/packages/packages.go index 3d1795b42c041..c7c6cf49d3c4b 100644 --- a/routers/web/shared/packages/packages.go +++ b/routers/web/shared/packages/packages.go @@ -154,46 +154,60 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) { versionsToRemove := make([]*packages_model.PackageDescriptor, 0, 10) + limit := 200 for _, p := range packages { - pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - 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 { - if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil { - ctx.ServerError("ShouldBeSkipped", err) + lastVersionID := int64(0) + for { + pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + PackageID: p.ID, + IsInternal: optional.Some(false), + Sort: packages_model.SortCreatedDesc, + Paginator: db.NewAbsoluteListOptions(func() int { + if lastVersionID > 0 { + return 0 + } + return pcr.KeepCount + }(), limit), + LtVersionID: lastVersionID, + }) + if err != nil { + ctx.ServerError("SearchVersions", err) return - } else if skip { - continue } - - toMatch := pv.LowerVersion - if pcr.MatchFullName { - toMatch = p.LowerName + "/" + pv.LowerVersion - } - - if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { - continue + for _, pv := range pvs { + lastVersionID = pv.ID + if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil { + ctx.ServerError("ShouldBeSkipped", err) + return + } else if skip { + continue + } + + toMatch := pv.LowerVersion + if pcr.MatchFullName { + toMatch = p.LowerName + "/" + pv.LowerVersion + } + + if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { + continue + } + if pv.CreatedUnix.AsLocalTime().After(olderThan) { + continue + } + if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { + continue + } + + pd, err := packages_model.GetPackageDescriptor(ctx, pv) + if err != nil { + ctx.ServerError("GetPackageDescriptor", err) + return + } + versionsToRemove = append(versionsToRemove, pd) } - if pv.CreatedUnix.AsLocalTime().After(olderThan) { - continue - } - if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { - continue - } - - pd, err := packages_model.GetPackageDescriptor(ctx, pv) - if err != nil { - ctx.ServerError("GetPackageDescriptor", err) - return + if len(pvs) < limit { + break } - versionsToRemove = append(versionsToRemove, pd) } } diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index b7ba2b6ac4afc..cf00ba8eb4c17 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -58,65 +58,79 @@ func ExecuteCleanupRules(outerCtx context.Context) error { } anyVersionDeleted := false + limit := 200 for _, p := range packages { - pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - 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 { - 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) - } else if skip { - log.Debug("Rule[%d]: keep '%s/%s' (container)", pcr.ID, p.Name, pv.Version) - continue - } + lastVersionID := int64(0) + for { + pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + PackageID: p.ID, + IsInternal: optional.Some(false), + Sort: packages_model.SortCreatedDesc, + Paginator: db.NewAbsoluteListOptions(func() int { + if lastVersionID > 0 { + return 0 + } + return pcr.KeepCount + }(), limit), + LtVersionID: lastVersionID, + }) + if err != nil { + return fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err) } + versionDeleted := false + for _, pv := range pvs { + lastVersionID = pv.ID + 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) + } else if skip { + log.Debug("Rule[%d]: keep '%s/%s' (container)", pcr.ID, p.Name, pv.Version) + continue + } + } - toMatch := pv.LowerVersion - if pcr.MatchFullName { - toMatch = p.LowerName + "/" + pv.LowerVersion - } + toMatch := pv.LowerVersion + if pcr.MatchFullName { + toMatch = p.LowerName + "/" + pv.LowerVersion + } - if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { - log.Debug("Rule[%d]: keep '%s/%s' (keep pattern)", pcr.ID, p.Name, pv.Version) - continue - } - if pv.CreatedUnix.AsLocalTime().After(olderThan) { - log.Debug("Rule[%d]: keep '%s/%s' (remove days)", pcr.ID, p.Name, pv.Version) - continue - } - if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { - log.Debug("Rule[%d]: keep '%s/%s' (remove pattern)", pcr.ID, p.Name, pv.Version) - continue - } + if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { + log.Debug("Rule[%d]: keep '%s/%s' (keep pattern)", pcr.ID, p.Name, pv.Version) + continue + } + if pv.CreatedUnix.AsLocalTime().After(olderThan) { + log.Debug("Rule[%d]: keep '%s/%s' (remove days) %v", pcr.ID, p.Name, pv.Version, pv.CreatedUnix.FormatDate()) + continue + } + if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { + log.Debug("Rule[%d]: keep '%s/%s' (remove pattern)", pcr.ID, p.Name, pv.Version) + continue + } - log.Debug("Rule[%d]: remove '%s/%s'", pcr.ID, p.Name, pv.Version) + log.Debug("Rule[%d]: remove '%s/%s'", pcr.ID, p.Name, pv.Version) - if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil { - return fmt.Errorf("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err) - } + if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil { + return fmt.Errorf("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err) + } - versionDeleted = true - anyVersionDeleted = true - } + versionDeleted = true + anyVersionDeleted = true + } - if versionDeleted { - if pcr.Type == packages_model.TypeCargo { - owner, err := user_model.GetUserByID(ctx, pcr.OwnerID) - if err != nil { - return fmt.Errorf("GetUserByID failed: %w", err) - } - if err := cargo_service.UpdatePackageIndexIfExists(ctx, owner, owner, p.ID); err != nil { - return fmt.Errorf("CleanupRule [%d]: cargo.UpdatePackageIndexIfExists failed: %w", pcr.ID, err) + if versionDeleted { + if pcr.Type == packages_model.TypeCargo { + owner, err := user_model.GetUserByID(ctx, pcr.OwnerID) + if err != nil { + return fmt.Errorf("GetUserByID failed: %w", err) + } + if err := cargo_service.UpdatePackageIndexIfExists(ctx, owner, owner, p.ID); err != nil { + return fmt.Errorf("CleanupRule [%d]: cargo.UpdatePackageIndexIfExists failed: %w", pcr.ID, err) + } } } + if len(pvs) < limit { + break + } } } From 25bc9b6f84ef555b4682067dbbf6924167e5a98a Mon Sep 17 00:00:00 2001 From: yeyuanjie Date: Mon, 9 Jun 2025 13:55:40 +0800 Subject: [PATCH 02/12] add cleanup test over 200 --- routers/web/shared/packages/packages.go | 14 +++++--------- services/packages/cleanup/cleanup.go | 14 +++++--------- tests/integration/api_packages_test.go | 16 ++++++++++------ 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/routers/web/shared/packages/packages.go b/routers/web/shared/packages/packages.go index c7c6cf49d3c4b..c793a908c9883 100644 --- a/routers/web/shared/packages/packages.go +++ b/routers/web/shared/packages/packages.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/forms" @@ -159,15 +160,10 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) { lastVersionID := int64(0) for { pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - PackageID: p.ID, - IsInternal: optional.Some(false), - Sort: packages_model.SortCreatedDesc, - Paginator: db.NewAbsoluteListOptions(func() int { - if lastVersionID > 0 { - return 0 - } - return pcr.KeepCount - }(), limit), + PackageID: p.ID, + IsInternal: optional.Some(false), + Sort: packages_model.SortCreatedDesc, + Paginator: db.NewAbsoluteListOptions(util.Iif(lastVersionID > 0, 0, pcr.KeepCount), limit), LtVersionID: lastVersionID, }) if err != nil { diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index cf00ba8eb4c17..c3de50757dbc3 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" packages_module "code.gitea.io/gitea/modules/packages" + "code.gitea.io/gitea/modules/util" packages_service "code.gitea.io/gitea/services/packages" alpine_service "code.gitea.io/gitea/services/packages/alpine" arch_service "code.gitea.io/gitea/services/packages/arch" @@ -63,15 +64,10 @@ func ExecuteCleanupRules(outerCtx context.Context) error { lastVersionID := int64(0) for { pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - PackageID: p.ID, - IsInternal: optional.Some(false), - Sort: packages_model.SortCreatedDesc, - Paginator: db.NewAbsoluteListOptions(func() int { - if lastVersionID > 0 { - return 0 - } - return pcr.KeepCount - }(), limit), + PackageID: p.ID, + IsInternal: optional.Some(false), + Sort: packages_model.SortCreatedDesc, + Paginator: db.NewAbsoluteListOptions(util.Iif(lastVersionID > 0, 0, pcr.KeepCount), limit), LtVersionID: lastVersionID, }) if err != nil { diff --git a/tests/integration/api_packages_test.go b/tests/integration/api_packages_test.go index 786addbd76c2c..505d14b428997 100644 --- a/tests/integration/api_packages_test.go +++ b/tests/integration/api_packages_test.go @@ -636,12 +636,16 @@ func TestPackageCleanup(t *testing.T) { }, { Name: "Mixed", - Versions: []version{ - {Version: "keep", ShouldExist: true, Created: time.Now().Add(time.Duration(10000)).Unix()}, - {Version: "dummy", ShouldExist: true, Created: 1}, - {Version: "test-3", ShouldExist: true}, - {Version: "test-4", ShouldExist: false, Created: 1}, - }, + Versions: func(limit, removeDays int) []version { + aa := []version{ + {Version: "keep", ShouldExist: true, Created: time.Now().Add(time.Duration(10000)).Unix()}, + {Version: "dummy", ShouldExist: true, Created: 1}, + } + for i := range limit { + aa = append(aa, version{Version: fmt.Sprintf("test-%v", i+3), ShouldExist: util.Iif(i < removeDays, true, false), Created: time.Now().AddDate(0, 0, -i).Unix()}) + } + return aa + }(220, 7), Rule: &packages_model.PackageCleanupRule{ Enabled: true, KeepCount: 1, From 3bb2acdfbe299d0c87a66b65a0f40e2e8567b89b Mon Sep 17 00:00:00 2001 From: yeyuanjie Date: Wed, 11 Jun 2025 12:51:45 +0800 Subject: [PATCH 03/12] test out version name --- tests/integration/api_packages_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/api_packages_test.go b/tests/integration/api_packages_test.go index 505d14b428997..6e5c90bb35727 100644 --- a/tests/integration/api_packages_test.go +++ b/tests/integration/api_packages_test.go @@ -690,7 +690,7 @@ func TestPackageCleanup(t *testing.T) { err = packages_service.DeletePackageVersionAndReferences(db.DefaultContext, pv) assert.NoError(t, err) } else { - assert.ErrorIs(t, err, packages_model.ErrPackageNotExist) + assert.ErrorIs(t, err, packages_model.ErrPackageNotExist, fmt.Sprintf("ver:%v", v.Version)) } } From bc8f1e3661bdd2a95eab37bd2f820c1530b8b6d6 Mon Sep 17 00:00:00 2001 From: yeyuanjie Date: Wed, 11 Jun 2025 14:46:15 +0800 Subject: [PATCH 04/12] cleanup by rule error --- models/packages/package_version.go | 4 ---- routers/web/shared/packages/packages.go | 22 ++++++++++------------ services/packages/cleanup/cleanup.go | 25 ++++++++++++------------- 3 files changed, 22 insertions(+), 29 deletions(-) diff --git a/models/packages/package_version.go b/models/packages/package_version.go index f5a7a5021b03f..bb7fd895f81da 100644 --- a/models/packages/package_version.go +++ b/models/packages/package_version.go @@ -180,7 +180,6 @@ type PackageSearchOptions struct { RepoID int64 Type Type PackageID int64 - LtVersionID int64 Name SearchValue // only results with the specific name are found Version SearchValue // only results with the specific version are found Properties map[string]string // only results are found which contain all listed version properties with the specific value @@ -211,9 +210,6 @@ func (opts *PackageSearchOptions) ToConds() builder.Cond { if opts.PackageID != 0 { cond = cond.And(builder.Eq{"package.id": opts.PackageID}) } - if opts.LtVersionID != 0 { - cond = cond.And(builder.Lt{"package_version.id": opts.LtVersionID}) - } if opts.Name.Value != "" { if opts.Name.ExactMatch { cond = cond.And(builder.Eq{"package.lower_name": strings.ToLower(opts.Name.Value)}) diff --git a/routers/web/shared/packages/packages.go b/routers/web/shared/packages/packages.go index c793a908c9883..2f4919ccec699 100644 --- a/routers/web/shared/packages/packages.go +++ b/routers/web/shared/packages/packages.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/templates" - "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/forms" @@ -155,23 +154,24 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) { versionsToRemove := make([]*packages_model.PackageDescriptor, 0, 10) - limit := 200 for _, p := range packages { - lastVersionID := int64(0) + skip := pcr.KeepCount for { pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - PackageID: p.ID, - IsInternal: optional.Some(false), - Sort: packages_model.SortCreatedDesc, - Paginator: db.NewAbsoluteListOptions(util.Iif(lastVersionID > 0, 0, pcr.KeepCount), limit), - LtVersionID: lastVersionID, + PackageID: p.ID, + IsInternal: optional.Some(false), + Sort: packages_model.SortCreatedDesc, + Paginator: db.NewAbsoluteListOptions(skip, 200), }) if err != nil { ctx.ServerError("SearchVersions", err) return } + if len(pvs) == 0 { + break + } for _, pv := range pvs { - lastVersionID = pv.ID + skip += 1 if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil { ctx.ServerError("ShouldBeSkipped", err) return @@ -201,9 +201,7 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) { } versionsToRemove = append(versionsToRemove, pd) } - if len(pvs) < limit { - break - } + } } diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index c3de50757dbc3..dc953faada8ca 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" packages_module "code.gitea.io/gitea/modules/packages" - "code.gitea.io/gitea/modules/util" packages_service "code.gitea.io/gitea/services/packages" alpine_service "code.gitea.io/gitea/services/packages/alpine" arch_service "code.gitea.io/gitea/services/packages/arch" @@ -59,23 +58,25 @@ func ExecuteCleanupRules(outerCtx context.Context) error { } anyVersionDeleted := false - limit := 200 for _, p := range packages { - lastVersionID := int64(0) + skip := pcr.KeepCount for { pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - PackageID: p.ID, - IsInternal: optional.Some(false), - Sort: packages_model.SortCreatedDesc, - Paginator: db.NewAbsoluteListOptions(util.Iif(lastVersionID > 0, 0, pcr.KeepCount), limit), - LtVersionID: lastVersionID, + PackageID: p.ID, + IsInternal: optional.Some(false), + Sort: packages_model.SortCreatedDesc, + Paginator: db.NewAbsoluteListOptions(skip, 200), }) if err != nil { return fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err) } + if len(pvs) == 0 { + break + } + log.Debug("%v pvs %v", skip, len(pvs)) versionDeleted := false + skip += len(pvs) for _, pv := range pvs { - lastVersionID = pv.ID 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) @@ -108,7 +109,7 @@ func ExecuteCleanupRules(outerCtx context.Context) error { if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil { return fmt.Errorf("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err) } - + skip -= 1 versionDeleted = true anyVersionDeleted = true } @@ -124,9 +125,7 @@ func ExecuteCleanupRules(outerCtx context.Context) error { } } } - if len(pvs) < limit { - break - } + } } From 277b9c586fe082285ae58f2b7e28c8230ca3683b Mon Sep 17 00:00:00 2001 From: yeyuanjie Date: Wed, 11 Jun 2025 15:14:49 +0800 Subject: [PATCH 05/12] lint error --- routers/web/shared/packages/packages.go | 6 +----- services/packages/cleanup/cleanup.go | 9 +-------- tests/integration/api_packages_test.go | 2 +- 3 files changed, 3 insertions(+), 14 deletions(-) diff --git a/routers/web/shared/packages/packages.go b/routers/web/shared/packages/packages.go index 2f4919ccec699..fe63360166b85 100644 --- a/routers/web/shared/packages/packages.go +++ b/routers/web/shared/packages/packages.go @@ -171,19 +171,17 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) { break } for _, pv := range pvs { - skip += 1 + skip++ if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil { ctx.ServerError("ShouldBeSkipped", err) return } else if skip { continue } - toMatch := pv.LowerVersion if pcr.MatchFullName { toMatch = p.LowerName + "/" + pv.LowerVersion } - if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { continue } @@ -193,7 +191,6 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) { if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { continue } - pd, err := packages_model.GetPackageDescriptor(ctx, pv) if err != nil { ctx.ServerError("GetPackageDescriptor", err) @@ -201,7 +198,6 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) { } versionsToRemove = append(versionsToRemove, pd) } - } } diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index dc953faada8ca..dbb61bfe22cf1 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -73,7 +73,6 @@ func ExecuteCleanupRules(outerCtx context.Context) error { if len(pvs) == 0 { break } - log.Debug("%v pvs %v", skip, len(pvs)) versionDeleted := false skip += len(pvs) for _, pv := range pvs { @@ -85,12 +84,10 @@ func ExecuteCleanupRules(outerCtx context.Context) error { continue } } - toMatch := pv.LowerVersion if pcr.MatchFullName { toMatch = p.LowerName + "/" + pv.LowerVersion } - if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { log.Debug("Rule[%d]: keep '%s/%s' (keep pattern)", pcr.ID, p.Name, pv.Version) continue @@ -103,17 +100,14 @@ func ExecuteCleanupRules(outerCtx context.Context) error { log.Debug("Rule[%d]: keep '%s/%s' (remove pattern)", pcr.ID, p.Name, pv.Version) continue } - log.Debug("Rule[%d]: remove '%s/%s'", pcr.ID, p.Name, pv.Version) - if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil { return fmt.Errorf("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err) } - skip -= 1 + skip-- versionDeleted = true anyVersionDeleted = true } - if versionDeleted { if pcr.Type == packages_model.TypeCargo { owner, err := user_model.GetUserByID(ctx, pcr.OwnerID) @@ -125,7 +119,6 @@ func ExecuteCleanupRules(outerCtx context.Context) error { } } } - } } diff --git a/tests/integration/api_packages_test.go b/tests/integration/api_packages_test.go index 6e5c90bb35727..106b0befea9c0 100644 --- a/tests/integration/api_packages_test.go +++ b/tests/integration/api_packages_test.go @@ -690,7 +690,7 @@ func TestPackageCleanup(t *testing.T) { err = packages_service.DeletePackageVersionAndReferences(db.DefaultContext, pv) assert.NoError(t, err) } else { - assert.ErrorIs(t, err, packages_model.ErrPackageNotExist, fmt.Sprintf("ver:%v", v.Version)) + assert.ErrorIs(t, err, packages_model.ErrPackageNotExist, v.Version) } } From d130dd615e9641deb88018c5ee058c2ce99192b2 Mon Sep 17 00:00:00 2001 From: yeyuanjie Date: Thu, 12 Jun 2025 15:14:54 +0800 Subject: [PATCH 06/12] Remove the db transaction and load all corresponding versions into memory, ignoring deletion errors --- models/packages/package_version.go | 10 +- routers/web/shared/packages/packages.go | 78 ++++++++------- services/packages/cleanup/cleanup.go | 120 +++++++++++------------- 3 files changed, 97 insertions(+), 111 deletions(-) diff --git a/models/packages/package_version.go b/models/packages/package_version.go index bb7fd895f81da..1e0bf52359861 100644 --- a/models/packages/package_version.go +++ b/models/packages/package_version.go @@ -291,14 +291,14 @@ func SearchVersions(ctx context.Context, opts *PackageSearchOptions) ([]*Package Where(opts.ToConds()) opts.configureOrderBy(sess) - + pvs := make([]*PackageVersion, 0, 10) if opts.Paginator != nil { sess = db.SetSessionPagination(sess, opts) + count, err := sess.FindAndCount(&pvs) + return pvs, count, err } - - pvs := make([]*PackageVersion, 0, 10) - count, err := sess.FindAndCount(&pvs) - return pvs, count, err + err := sess.Find(&pvs) + return pvs, int64(len(pvs)), err } // SearchLatestVersions gets the latest version of every package matching the search options diff --git a/routers/web/shared/packages/packages.go b/routers/web/shared/packages/packages.go index fe63360166b85..22db91864f3b7 100644 --- a/routers/web/shared/packages/packages.go +++ b/routers/web/shared/packages/packages.go @@ -8,7 +8,6 @@ import ( "net/http" "time" - "code.gitea.io/gitea/models/db" packages_model "code.gitea.io/gitea/models/packages" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" @@ -155,49 +154,48 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) { versionsToRemove := make([]*packages_model.PackageDescriptor, 0, 10) for _, p := range packages { - skip := pcr.KeepCount - for { - pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - PackageID: p.ID, - IsInternal: optional.Some(false), - Sort: packages_model.SortCreatedDesc, - Paginator: db.NewAbsoluteListOptions(skip, 200), - }) - if err != nil { - ctx.ServerError("SearchVersions", err) + pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + PackageID: p.ID, + IsInternal: optional.Some(false), + Sort: packages_model.SortCreatedDesc, + }) + if err != nil { + ctx.ServerError("SearchVersions", err) + return + } + if pcr.KeepCount > 0 { + if pcr.KeepCount < len(pvs) { + pvs = pvs[pcr.KeepCount:] + } else { + pvs = nil + } + } + for _, pv := range pvs { + if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil { + ctx.ServerError("ShouldBeSkipped", err) return + } else if skip { + continue } - if len(pvs) == 0 { - break + toMatch := pv.LowerVersion + if pcr.MatchFullName { + toMatch = p.LowerName + "/" + pv.LowerVersion } - for _, pv := range pvs { - skip++ - if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil { - ctx.ServerError("ShouldBeSkipped", err) - return - } else if skip { - continue - } - toMatch := pv.LowerVersion - if pcr.MatchFullName { - toMatch = p.LowerName + "/" + pv.LowerVersion - } - if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { - continue - } - if pv.CreatedUnix.AsLocalTime().After(olderThan) { - continue - } - if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { - continue - } - pd, err := packages_model.GetPackageDescriptor(ctx, pv) - if err != nil { - ctx.ServerError("GetPackageDescriptor", err) - return - } - versionsToRemove = append(versionsToRemove, pd) + if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { + continue + } + if pv.CreatedUnix.AsLocalTime().After(olderThan) { + continue + } + if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { + continue + } + pd, err := packages_model.GetPackageDescriptor(ctx, pv) + if err != nil { + ctx.ServerError("GetPackageDescriptor", err) + return } + versionsToRemove = append(versionsToRemove, pd) } } diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index dbb61bfe22cf1..74e0873c4df81 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -33,13 +33,7 @@ func CleanupTask(ctx context.Context, olderThan time.Duration) error { } func ExecuteCleanupRules(outerCtx context.Context) error { - ctx, committer, err := db.TxContext(outerCtx) - if err != nil { - return err - } - defer committer.Close() - - err = packages_model.IterateEnabledCleanupRules(ctx, func(ctx context.Context, pcr *packages_model.PackageCleanupRule) error { + return packages_model.IterateEnabledCleanupRules(outerCtx, func(ctx context.Context, pcr *packages_model.PackageCleanupRule) error { select { case <-outerCtx.Done(): return db.ErrCancelledf("While processing package cleanup rules") @@ -59,64 +53,63 @@ func ExecuteCleanupRules(outerCtx context.Context) error { anyVersionDeleted := false for _, p := range packages { - skip := pcr.KeepCount - for { - pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - PackageID: p.ID, - IsInternal: optional.Some(false), - Sort: packages_model.SortCreatedDesc, - Paginator: db.NewAbsoluteListOptions(skip, 200), - }) - if err != nil { - return fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err) - } - if len(pvs) == 0 { - break + pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + PackageID: p.ID, + IsInternal: optional.Some(false), + Sort: packages_model.SortCreatedDesc, + }) + if err != nil { + return fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err) + } + if pcr.KeepCount > 0 { + if pcr.KeepCount < len(pvs) { + pvs = pvs[pcr.KeepCount:] + } else { + pvs = nil } - versionDeleted := false - skip += len(pvs) - for _, pv := range pvs { - 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) - } else if skip { - log.Debug("Rule[%d]: keep '%s/%s' (container)", pcr.ID, p.Name, pv.Version) - continue - } - } - toMatch := pv.LowerVersion - if pcr.MatchFullName { - toMatch = p.LowerName + "/" + pv.LowerVersion - } - if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { - log.Debug("Rule[%d]: keep '%s/%s' (keep pattern)", pcr.ID, p.Name, pv.Version) - continue - } - if pv.CreatedUnix.AsLocalTime().After(olderThan) { - log.Debug("Rule[%d]: keep '%s/%s' (remove days) %v", pcr.ID, p.Name, pv.Version, pv.CreatedUnix.FormatDate()) - continue - } - if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { - log.Debug("Rule[%d]: keep '%s/%s' (remove pattern)", pcr.ID, p.Name, pv.Version) + } + versionDeleted := false + for _, pv := range pvs { + 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) + } else if skip { + log.Debug("Rule[%d]: keep '%s/%s' (container)", pcr.ID, p.Name, pv.Version) continue } - log.Debug("Rule[%d]: remove '%s/%s'", pcr.ID, p.Name, pv.Version) - if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil { - return fmt.Errorf("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err) - } - skip-- - versionDeleted = true - anyVersionDeleted = true } - if versionDeleted { - if pcr.Type == packages_model.TypeCargo { - owner, err := user_model.GetUserByID(ctx, pcr.OwnerID) - if err != nil { - return fmt.Errorf("GetUserByID failed: %w", err) - } - if err := cargo_service.UpdatePackageIndexIfExists(ctx, owner, owner, p.ID); err != nil { - return fmt.Errorf("CleanupRule [%d]: cargo.UpdatePackageIndexIfExists failed: %w", pcr.ID, err) - } + toMatch := pv.LowerVersion + if pcr.MatchFullName { + toMatch = p.LowerName + "/" + pv.LowerVersion + } + if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { + log.Debug("Rule[%d]: keep '%s/%s' (keep pattern)", pcr.ID, p.Name, pv.Version) + continue + } + if pv.CreatedUnix.AsLocalTime().After(olderThan) { + log.Debug("Rule[%d]: keep '%s/%s' (remove days) %v", pcr.ID, p.Name, pv.Version, pv.CreatedUnix.FormatDate()) + continue + } + if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { + log.Debug("Rule[%d]: keep '%s/%s' (remove pattern)", pcr.ID, p.Name, pv.Version) + continue + } + log.Debug("Rule[%d]: remove '%s/%s'", pcr.ID, p.Name, pv.Version) + if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil { + log.Error("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err) + continue + } + versionDeleted = true + anyVersionDeleted = true + } + if versionDeleted { + if pcr.Type == packages_model.TypeCargo { + owner, err := user_model.GetUserByID(ctx, pcr.OwnerID) + if err != nil { + return fmt.Errorf("GetUserByID failed: %w", err) + } + if err := cargo_service.UpdatePackageIndexIfExists(ctx, owner, owner, p.ID); err != nil { + return fmt.Errorf("CleanupRule [%d]: cargo.UpdatePackageIndexIfExists failed: %w", pcr.ID, err) } } } @@ -150,11 +143,6 @@ func ExecuteCleanupRules(outerCtx context.Context) error { } return nil }) - if err != nil { - return err - } - - return committer.Commit() } func CleanupExpiredData(outerCtx context.Context, olderThan time.Duration) error { From dcc6867a92a387c2c673a22672776c9b7f209a20 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 18 Jun 2025 10:14:48 +0800 Subject: [PATCH 07/12] fix SearchVersions --- models/packages/package_version.go | 34 +++++++++++++----------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/models/packages/package_version.go b/models/packages/package_version.go index 1e0bf52359861..5672e0efbff3b 100644 --- a/models/packages/package_version.go +++ b/models/packages/package_version.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/util" "xorm.io/builder" + "xorm.io/xorm" ) // ErrDuplicatePackageVersion indicates a duplicated package version error @@ -187,7 +188,7 @@ type PackageSearchOptions struct { HasFileWithName string // only results are found which are associated with a file with the specific name HasFiles optional.Option[bool] // only results are found which have associated files Sort VersionSort - db.Paginator + Paginator db.Paginator } func (opts *PackageSearchOptions) ToConds() builder.Cond { @@ -282,18 +283,11 @@ func (opts *PackageSearchOptions) configureOrderBy(e db.Engine) { e.Desc("package_version.id") // Sort by id for stable order with duplicates in the other field } -// SearchVersions gets all versions of packages matching the search options -func SearchVersions(ctx context.Context, opts *PackageSearchOptions) ([]*PackageVersion, int64, error) { - sess := db.GetEngine(ctx). - Select("package_version.*"). - Table("package_version"). - Join("INNER", "package", "package.id = package_version.package_id"). - Where(opts.ToConds()) - +func searchVersionsBySession(sess *xorm.Session, opts *PackageSearchOptions) ([]*PackageVersion, int64, error) { opts.configureOrderBy(sess) pvs := make([]*PackageVersion, 0, 10) if opts.Paginator != nil { - sess = db.SetSessionPagination(sess, opts) + sess = db.SetSessionPagination(sess, opts.Paginator) count, err := sess.FindAndCount(&pvs) return pvs, count, err } @@ -301,6 +295,16 @@ func SearchVersions(ctx context.Context, opts *PackageSearchOptions) ([]*Package return pvs, int64(len(pvs)), err } +// SearchVersions gets all versions of packages matching the search options +func SearchVersions(ctx context.Context, opts *PackageSearchOptions) ([]*PackageVersion, int64, error) { + sess := db.GetEngine(ctx). + Select("package_version.*"). + Table("package_version"). + Join("INNER", "package", "package.id = package_version.package_id"). + Where(opts.ToConds()) + return searchVersionsBySession(sess, opts) +} + // SearchLatestVersions gets the latest version of every package matching the search options func SearchLatestVersions(ctx context.Context, opts *PackageSearchOptions) ([]*PackageVersion, int64, error) { in := builder. @@ -316,15 +320,7 @@ func SearchLatestVersions(ctx context.Context, opts *PackageSearchOptions) ([]*P Join("INNER", "package", "package.id = package_version.package_id"). Where(builder.In("package_version.id", in)) - opts.configureOrderBy(sess) - - if opts.Paginator != nil { - sess = db.SetSessionPagination(sess, opts) - } - - pvs := make([]*PackageVersion, 0, 10) - count, err := sess.FindAndCount(&pvs) - return pvs, count, err + return searchVersionsBySession(sess, opts) } // ExistVersion checks if a version matching the search options exist From fb79731d957e4d4004e1b36be5189defd8397885 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 18 Jun 2025 10:25:19 +0800 Subject: [PATCH 08/12] fix transaction --- services/packages/cleanup/cleanup.go | 209 +++++++++++++++------------ 1 file changed, 114 insertions(+), 95 deletions(-) diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index 74e0873c4df81..e8f95e28ff4a1 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -32,115 +32,134 @@ func CleanupTask(ctx context.Context, olderThan time.Duration) error { return CleanupExpiredData(ctx, olderThan) } -func ExecuteCleanupRules(outerCtx context.Context) error { - return packages_model.IterateEnabledCleanupRules(outerCtx, func(ctx context.Context, pcr *packages_model.PackageCleanupRule) error { - select { - case <-outerCtx.Done(): - return db.ErrCancelledf("While processing package cleanup rules") - default: +func executeCleanupOneRulePackage(ctx context.Context, pcr *packages_model.PackageCleanupRule, p *packages_model.Package) (err error, versionDeleted bool) { + olderThan := time.Now().AddDate(0, 0, -pcr.RemoveDays) + pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + PackageID: p.ID, + IsInternal: optional.Some(false), + Sort: packages_model.SortCreatedDesc, + }) + if err != nil { + return fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err), false + } + if pcr.KeepCount > 0 { + if pcr.KeepCount < len(pvs) { + pvs = pvs[pcr.KeepCount:] + } else { + pvs = nil } - - if err := pcr.CompiledPattern(); err != nil { - return fmt.Errorf("CleanupRule [%d]: CompilePattern failed: %w", pcr.ID, err) + } + for _, pv := range pvs { + 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), false + } else if skip { + log.Debug("Rule[%d]: keep '%s/%s' (container)", pcr.ID, p.Name, pv.Version) + continue + } + } + toMatch := pv.LowerVersion + if pcr.MatchFullName { + toMatch = p.LowerName + "/" + pv.LowerVersion } + if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { + log.Debug("Rule[%d]: keep '%s/%s' (keep pattern)", pcr.ID, p.Name, pv.Version) + continue + } + if pv.CreatedUnix.AsLocalTime().After(olderThan) { + log.Debug("Rule[%d]: keep '%s/%s' (remove days) %v", pcr.ID, p.Name, pv.Version, pv.CreatedUnix.FormatDate()) + continue + } + if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { + log.Debug("Rule[%d]: keep '%s/%s' (remove pattern)", pcr.ID, p.Name, pv.Version) + continue + } + log.Debug("Rule[%d]: remove '%s/%s'", pcr.ID, p.Name, pv.Version) + if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil { + log.Error("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err) + continue + } + versionDeleted = true + } + return nil, versionDeleted +} - olderThan := time.Now().AddDate(0, 0, -pcr.RemoveDays) +func executeCleanupOneRule(ctx context.Context, pcr *packages_model.PackageCleanupRule) error { + select { + case <-ctx.Done(): + return db.ErrCancelledf("While processing package cleanup rules") + default: + } + + if err := pcr.CompiledPattern(); err != nil { + return fmt.Errorf("CleanupRule [%d]: CompilePattern failed: %w", pcr.ID, err) + } + + packages, err := packages_model.GetPackagesByType(ctx, pcr.OwnerID, pcr.Type) + if err != nil { + return fmt.Errorf("CleanupRule [%d]: GetPackagesByType failed: %w", pcr.ID, err) + } - packages, err := packages_model.GetPackagesByType(ctx, pcr.OwnerID, pcr.Type) + anyVersionDeleted := false + for _, p := range packages { + versionDeleted := false + err = db.WithTx(ctx, func(ctx context.Context) (err error) { + err, versionDeleted = executeCleanupOneRulePackage(ctx, pcr, p) + return err + }) if err != nil { - return fmt.Errorf("CleanupRule [%d]: GetPackagesByType failed: %w", pcr.ID, err) + log.Error("CleanupRule [%d]: executeCleanupOneRulePackage(%d) failed: %v", pcr.ID, p.ID, err) + continue } - - anyVersionDeleted := false - for _, p := range packages { - pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - PackageID: p.ID, - IsInternal: optional.Some(false), - Sort: packages_model.SortCreatedDesc, - }) - if err != nil { - return fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err) - } - if pcr.KeepCount > 0 { - if pcr.KeepCount < len(pvs) { - pvs = pvs[pcr.KeepCount:] - } else { - pvs = nil - } - } - versionDeleted := false - for _, pv := range pvs { - 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) - } else if skip { - log.Debug("Rule[%d]: keep '%s/%s' (container)", pcr.ID, p.Name, pv.Version) - continue - } - } - toMatch := pv.LowerVersion - if pcr.MatchFullName { - toMatch = p.LowerName + "/" + pv.LowerVersion - } - if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { - log.Debug("Rule[%d]: keep '%s/%s' (keep pattern)", pcr.ID, p.Name, pv.Version) - continue - } - if pv.CreatedUnix.AsLocalTime().After(olderThan) { - log.Debug("Rule[%d]: keep '%s/%s' (remove days) %v", pcr.ID, p.Name, pv.Version, pv.CreatedUnix.FormatDate()) - continue - } - if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { - log.Debug("Rule[%d]: keep '%s/%s' (remove pattern)", pcr.ID, p.Name, pv.Version) - continue - } - log.Debug("Rule[%d]: remove '%s/%s'", pcr.ID, p.Name, pv.Version) - if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil { - log.Error("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err) - continue + anyVersionDeleted = anyVersionDeleted || versionDeleted + if versionDeleted { + if pcr.Type == packages_model.TypeCargo { + owner, err := user_model.GetUserByID(ctx, pcr.OwnerID) + if err != nil { + return fmt.Errorf("GetUserByID failed: %w", err) } - versionDeleted = true - anyVersionDeleted = true - } - if versionDeleted { - if pcr.Type == packages_model.TypeCargo { - owner, err := user_model.GetUserByID(ctx, pcr.OwnerID) - if err != nil { - return fmt.Errorf("GetUserByID failed: %w", err) - } - if err := cargo_service.UpdatePackageIndexIfExists(ctx, owner, owner, p.ID); err != nil { - return fmt.Errorf("CleanupRule [%d]: cargo.UpdatePackageIndexIfExists failed: %w", pcr.ID, err) - } + if err := cargo_service.UpdatePackageIndexIfExists(ctx, owner, owner, p.ID); err != nil { + return fmt.Errorf("CleanupRule [%d]: cargo.UpdatePackageIndexIfExists failed: %w", pcr.ID, err) } } } + } - if anyVersionDeleted { - switch pcr.Type { - case packages_model.TypeDebian: - if err := debian_service.BuildAllRepositoryFiles(ctx, pcr.OwnerID); err != nil { - return fmt.Errorf("CleanupRule [%d]: debian.BuildAllRepositoryFiles failed: %w", pcr.ID, err) - } - case packages_model.TypeAlpine: - if err := alpine_service.BuildAllRepositoryFiles(ctx, pcr.OwnerID); err != nil { - return fmt.Errorf("CleanupRule [%d]: alpine.BuildAllRepositoryFiles failed: %w", pcr.ID, err) - } - case packages_model.TypeRpm: - if err := rpm_service.BuildAllRepositoryFiles(ctx, pcr.OwnerID); err != nil { - return fmt.Errorf("CleanupRule [%d]: rpm.BuildAllRepositoryFiles failed: %w", pcr.ID, err) - } - case packages_model.TypeArch: - release, err := arch_service.AquireRegistryLock(ctx, pcr.OwnerID) - if err != nil { - return err - } - defer release() + if anyVersionDeleted { + switch pcr.Type { + case packages_model.TypeDebian: + if err := debian_service.BuildAllRepositoryFiles(ctx, pcr.OwnerID); err != nil { + return fmt.Errorf("CleanupRule [%d]: debian.BuildAllRepositoryFiles failed: %w", pcr.ID, err) + } + case packages_model.TypeAlpine: + if err := alpine_service.BuildAllRepositoryFiles(ctx, pcr.OwnerID); err != nil { + return fmt.Errorf("CleanupRule [%d]: alpine.BuildAllRepositoryFiles failed: %w", pcr.ID, err) + } + case packages_model.TypeRpm: + if err := rpm_service.BuildAllRepositoryFiles(ctx, pcr.OwnerID); err != nil { + return fmt.Errorf("CleanupRule [%d]: rpm.BuildAllRepositoryFiles failed: %w", pcr.ID, err) + } + case packages_model.TypeArch: + release, err := arch_service.AquireRegistryLock(ctx, pcr.OwnerID) + if err != nil { + return err + } + defer release() - if err := arch_service.BuildAllRepositoryFiles(ctx, pcr.OwnerID); err != nil { - return fmt.Errorf("CleanupRule [%d]: arch.BuildAllRepositoryFiles failed: %w", pcr.ID, err) - } + if err := arch_service.BuildAllRepositoryFiles(ctx, pcr.OwnerID); err != nil { + return fmt.Errorf("CleanupRule [%d]: arch.BuildAllRepositoryFiles failed: %w", pcr.ID, err) } } + } + return nil +} + +func ExecuteCleanupRules(ctx context.Context) error { + return packages_model.IterateEnabledCleanupRules(ctx, func(ctx context.Context, pcr *packages_model.PackageCleanupRule) error { + err := executeCleanupOneRule(ctx, pcr) + if err != nil { + log.Error("CleanupRule [%d]: executeCleanupOneRule failed: %v", pcr.ID, err) + } return nil }) } From a294142db0df30e9b3a7f085843e5ae408bb54f1 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 18 Jun 2025 10:29:29 +0800 Subject: [PATCH 09/12] fix lint --- models/packages/nuget/search.go | 2 +- routers/web/shared/packages/packages.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/models/packages/nuget/search.go b/models/packages/nuget/search.go index 7a505ff08f3e5..a4b23f31d55f0 100644 --- a/models/packages/nuget/search.go +++ b/models/packages/nuget/search.go @@ -33,7 +33,7 @@ func SearchVersions(ctx context.Context, opts *packages_model.PackageSearchOptio Where(cond). OrderBy("package.name ASC") if opts.Paginator != nil { - skip, take := opts.GetSkipTake() + skip, take := opts.Paginator.GetSkipTake() inner = inner.Limit(take, skip) } diff --git a/routers/web/shared/packages/packages.go b/routers/web/shared/packages/packages.go index 22db91864f3b7..a18dedf89c9e0 100644 --- a/routers/web/shared/packages/packages.go +++ b/routers/web/shared/packages/packages.go @@ -177,6 +177,7 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) { } else if skip { continue } + toMatch := pv.LowerVersion if pcr.MatchFullName { toMatch = p.LowerName + "/" + pv.LowerVersion @@ -190,6 +191,7 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) { if pcr.RemovePatternMatcher != nil && !pcr.RemovePatternMatcher.MatchString(toMatch) { continue } + pd, err := packages_model.GetPackageDescriptor(ctx, pv) if err != nil { ctx.ServerError("GetPackageDescriptor", err) From 54ae67c8dcf4ddb18f510c1605a9e702d01c523a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 18 Jun 2025 10:32:41 +0800 Subject: [PATCH 10/12] handle ctx cancel --- services/packages/cleanup/cleanup.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index e8f95e28ff4a1..7ea124308338e 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -85,12 +85,6 @@ func executeCleanupOneRulePackage(ctx context.Context, pcr *packages_model.Packa } func executeCleanupOneRule(ctx context.Context, pcr *packages_model.PackageCleanupRule) error { - select { - case <-ctx.Done(): - return db.ErrCancelledf("While processing package cleanup rules") - default: - } - if err := pcr.CompiledPattern(); err != nil { return fmt.Errorf("CleanupRule [%d]: CompilePattern failed: %w", pcr.ID, err) } @@ -156,6 +150,12 @@ func executeCleanupOneRule(ctx context.Context, pcr *packages_model.PackageClean func ExecuteCleanupRules(ctx context.Context) error { return packages_model.IterateEnabledCleanupRules(ctx, func(ctx context.Context, pcr *packages_model.PackageCleanupRule) error { + select { + case <-ctx.Done(): + return db.ErrCancelledf("While processing package cleanup rules") + default: + } + err := executeCleanupOneRule(ctx, pcr) if err != nil { log.Error("CleanupRule [%d]: executeCleanupOneRule failed: %v", pcr.ID, err) From 10903085e26caebbda84cf5f1d109e78b649d06b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 18 Jun 2025 11:02:55 +0800 Subject: [PATCH 11/12] fix lint --- services/packages/cleanup/cleanup.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index 7ea124308338e..aa8f594586fa7 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -32,7 +32,7 @@ func CleanupTask(ctx context.Context, olderThan time.Duration) error { return CleanupExpiredData(ctx, olderThan) } -func executeCleanupOneRulePackage(ctx context.Context, pcr *packages_model.PackageCleanupRule, p *packages_model.Package) (err error, versionDeleted bool) { +func executeCleanupOneRulePackage(ctx context.Context, pcr *packages_model.PackageCleanupRule, p *packages_model.Package) (versionDeleted bool, err error) { olderThan := time.Now().AddDate(0, 0, -pcr.RemoveDays) pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ PackageID: p.ID, @@ -40,7 +40,7 @@ func executeCleanupOneRulePackage(ctx context.Context, pcr *packages_model.Packa Sort: packages_model.SortCreatedDesc, }) if err != nil { - return fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err), false + return false, fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err) } if pcr.KeepCount > 0 { if pcr.KeepCount < len(pvs) { @@ -52,7 +52,7 @@ func executeCleanupOneRulePackage(ctx context.Context, pcr *packages_model.Packa for _, pv := range pvs { 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), false + return false, fmt.Errorf("CleanupRule [%d]: container.ShouldBeSkipped failed: %w", pcr.ID, err) } else if skip { log.Debug("Rule[%d]: keep '%s/%s' (container)", pcr.ID, p.Name, pv.Version) continue @@ -81,7 +81,7 @@ func executeCleanupOneRulePackage(ctx context.Context, pcr *packages_model.Packa } versionDeleted = true } - return nil, versionDeleted + return versionDeleted, nil } func executeCleanupOneRule(ctx context.Context, pcr *packages_model.PackageCleanupRule) error { @@ -98,7 +98,7 @@ func executeCleanupOneRule(ctx context.Context, pcr *packages_model.PackageClean for _, p := range packages { versionDeleted := false err = db.WithTx(ctx, func(ctx context.Context) (err error) { - err, versionDeleted = executeCleanupOneRulePackage(ctx, pcr, p) + versionDeleted, err = executeCleanupOneRulePackage(ctx, pcr, p) return err }) if err != nil { From 8891554f1b4861cb5935ed2038c140be762cd588 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 18 Jun 2025 12:17:58 +0800 Subject: [PATCH 12/12] Update services/packages/cleanup/cleanup.go --- services/packages/cleanup/cleanup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index aa8f594586fa7..959babe7cd587 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -76,7 +76,7 @@ func executeCleanupOneRulePackage(ctx context.Context, pcr *packages_model.Packa } log.Debug("Rule[%d]: remove '%s/%s'", pcr.ID, p.Name, pv.Version) if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil { - log.Error("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err) + log.Error("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %v", pcr.ID, err) continue } versionDeleted = true