when using rules to delete packages, remove unclean bugs

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

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"
@@ -159,25 +158,29 @@ 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
 		}
+		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
 			}
-
 			toMatch := pv.LowerVersion
 			if pcr.MatchFullName {
 				toMatch = p.LowerName + "/" + pv.LowerVersion
 			}
-
 			if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) {
 				continue
 			}
@@ -187,7 +190,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)

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")
@@ -63,11 +57,17 @@ 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)
 			}
+			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 {
@@ -78,35 +78,30 @@ 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
 				}
 				if pv.CreatedUnix.AsLocalTime().After(olderThan) {
-					log.Debug("Rule[%d]: keep '%s/%s' (remove days)", pcr.ID, p.Name, pv.Version)
+					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 {
-					return fmt.Errorf("CleanupRule [%d]: DeletePackageVersionAndReferences failed: %w", pcr.ID, err)
+					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)
@@ -148,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 {

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,
@@ -686,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, v.Version)
 					}
 				}