diff --git a/models/packages/nuget/search.go b/models/packages/nuget/search.go index 7a505ff08f..a4b23f31d5 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/models/packages/package_version.go b/models/packages/package_version.go index bb7fd895f8..5672e0efbf 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,6 +283,18 @@ func (opts *PackageSearchOptions) configureOrderBy(e db.Engine) { e.Desc("package_version.id") // Sort by id for stable order with duplicates in the other field } +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.Paginator) + count, err := sess.FindAndCount(&pvs) + return pvs, count, err + } + err := sess.Find(&pvs) + 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). @@ -289,16 +302,7 @@ func SearchVersions(ctx context.Context, opts *PackageSearchOptions) ([]*Package Table("package_version"). Join("INNER", "package", "package.id = package_version.package_id"). Where(opts.ToConds()) - - 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) } // SearchLatestVersions gets the latest version of every package matching the search options @@ -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 diff --git a/routers/web/shared/packages/packages.go b/routers/web/shared/packages/packages.go index 3d1795b42c..a18dedf89c 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" @@ -159,12 +158,18 @@ 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) @@ -177,7 +182,6 @@ func SetRulePreviewContext(ctx *context.Context, owner *user_model.User) { if pcr.MatchFullName { toMatch = p.LowerName + "/" + pv.LowerVersion } - if pcr.KeepPatternMatcher != nil && pcr.KeepPatternMatcher.MatchString(toMatch) { continue } diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index b7ba2b6ac4..959babe7cd 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -32,127 +32,136 @@ func CleanupTask(ctx context.Context, olderThan time.Duration) error { return CleanupExpiredData(ctx, olderThan) } -func ExecuteCleanupRules(outerCtx context.Context) error { - ctx, committer, err := db.TxContext(outerCtx) +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, + IsInternal: optional.Some(false), + Sort: packages_model.SortCreatedDesc, + }) if err != nil { - return err + return false, fmt.Errorf("CleanupRule [%d]: SearchVersions failed: %w", pcr.ID, err) } - defer committer.Close() + if pcr.KeepCount > 0 { + if pcr.KeepCount < len(pvs) { + pvs = pvs[pcr.KeepCount:] + } else { + pvs = nil + } + } + for _, pv := range pvs { + if pcr.Type == packages_model.TypeContainer { + if skip, err := container_service.ShouldBeSkipped(ctx, pcr, p, pv); err != nil { + 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 + } + } + 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: %v", pcr.ID, err) + continue + } + versionDeleted = true + } + return versionDeleted, nil +} - err = packages_model.IterateEnabledCleanupRules(ctx, func(ctx context.Context, pcr *packages_model.PackageCleanupRule) error { +func executeCleanupOneRule(ctx context.Context, pcr *packages_model.PackageCleanupRule) error { + 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) + } + + anyVersionDeleted := false + for _, p := range packages { + versionDeleted := false + err = db.WithTx(ctx, func(ctx context.Context) (err error) { + versionDeleted, err = executeCleanupOneRulePackage(ctx, pcr, p) + return err + }) + if err != nil { + log.Error("CleanupRule [%d]: executeCleanupOneRulePackage(%d) failed: %v", pcr.ID, p.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) + } + 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 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 { select { - case <-outerCtx.Done(): + 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) - } - - olderThan := time.Now().AddDate(0, 0, -pcr.RemoveDays) - - packages, err := packages_model.GetPackagesByType(ctx, pcr.OwnerID, pcr.Type) + err := executeCleanupOneRule(ctx, pcr) if err != nil { - return fmt.Errorf("CleanupRule [%d]: GetPackagesByType failed: %w", pcr.ID, err) - } - - 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, - 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 - } - } - - 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 - } - - 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) - } - - 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 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) - } - } + log.Error("CleanupRule [%d]: executeCleanupOneRule failed: %v", pcr.ID, err) } return nil }) - if err != nil { - return err - } - - return committer.Commit() } func CleanupExpiredData(outerCtx context.Context, olderThan time.Duration) error { diff --git a/tests/integration/api_packages_test.go b/tests/integration/api_packages_test.go index 786addbd76..106b0befea 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, @@ -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) } }