From 5e242e021b9f32d2d136e4f3324cb56be415f484 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Wed, 6 Apr 2022 03:32:09 +0200 Subject: [PATCH] Package registry changes (#19305) * removed debug logs * fixed SELECT * removed unneeded error type * use common SearchVersions method * remove empty container upload versions * return err --- integrations/api_packages_test.go | 33 ++++- models/packages/conan/references.go | 6 +- models/packages/conan/search.go | 4 +- models/packages/package.go | 8 +- models/packages/package_blob.go | 2 +- models/packages/package_file.go | 2 +- models/packages/package_version.go | 173 ++++++++++------------ routers/api/packages/composer/composer.go | 4 +- routers/api/packages/npm/npm.go | 7 +- routers/api/packages/nuget/nuget.go | 6 +- routers/api/packages/rubygems/rubygems.go | 15 +- routers/api/v1/packages/package.go | 4 +- routers/web/admin/packages.go | 6 +- routers/web/repo/packages.go | 8 +- routers/web/user/package.go | 15 +- services/packages/container/cleanup.go | 31 ++-- services/packages/packages.go | 11 +- 17 files changed, 185 insertions(+), 150 deletions(-) diff --git a/integrations/api_packages_test.go b/integrations/api_packages_test.go index 263e7cea53f..b3f6e88d9f2 100644 --- a/integrations/api_packages_test.go +++ b/integrations/api_packages_test.go @@ -9,11 +9,15 @@ import ( "fmt" "net/http" "testing" + "time" - "code.gitea.io/gitea/models/packages" + "code.gitea.io/gitea/models/db" + packages_model "code.gitea.io/gitea/models/packages" + container_model "code.gitea.io/gitea/models/packages/container" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" api "code.gitea.io/gitea/modules/structs" + packages_service "code.gitea.io/gitea/services/packages" "github.com/stretchr/testify/assert" ) @@ -43,7 +47,7 @@ func TestPackageAPI(t *testing.T) { DecodeJSON(t, resp, &apiPackages) assert.Len(t, apiPackages, 1) - assert.Equal(t, string(packages.TypeGeneric), apiPackages[0].Type) + assert.Equal(t, string(packages_model.TypeGeneric), apiPackages[0].Type) assert.Equal(t, packageName, apiPackages[0].Name) assert.Equal(t, packageVersion, apiPackages[0].Version) assert.NotNil(t, apiPackages[0].Creator) @@ -62,7 +66,7 @@ func TestPackageAPI(t *testing.T) { var p *api.Package DecodeJSON(t, resp, &p) - assert.Equal(t, string(packages.TypeGeneric), p.Type) + assert.Equal(t, string(packages_model.TypeGeneric), p.Type) assert.Equal(t, packageName, p.Name) assert.Equal(t, packageVersion, p.Version) assert.NotNil(t, p.Creator) @@ -100,3 +104,26 @@ func TestPackageAPI(t *testing.T) { MakeRequest(t, req, http.StatusNoContent) }) } + +func TestPackageCleanup(t *testing.T) { + defer prepareTestEnv(t)() + + time.Sleep(time.Second) + + pbs, err := packages_model.FindExpiredUnreferencedBlobs(db.DefaultContext, time.Duration(0)) + assert.NoError(t, err) + assert.NotEmpty(t, pbs) + + _, err = packages_model.GetInternalVersionByNameAndVersion(db.DefaultContext, 2, packages_model.TypeContainer, "test", container_model.UploadVersion) + assert.NoError(t, err) + + err = packages_service.Cleanup(nil, time.Duration(0)) + assert.NoError(t, err) + + pbs, err = packages_model.FindExpiredUnreferencedBlobs(db.DefaultContext, time.Duration(0)) + assert.NoError(t, err) + assert.Empty(t, pbs) + + _, err = packages_model.GetInternalVersionByNameAndVersion(db.DefaultContext, 2, packages_model.TypeContainer, "test", container_model.UploadVersion) + assert.ErrorIs(t, err, packages_model.ErrPackageNotExist) +} diff --git a/models/packages/conan/references.go b/models/packages/conan/references.go index 4b7b2014304..e47e689af72 100644 --- a/models/packages/conan/references.go +++ b/models/packages/conan/references.go @@ -65,14 +65,14 @@ func findPropertyValues(ctx context.Context, propertyName string, ownerID int64, in2 := builder. Select("package_file.id"). From("package_file"). - Join("INNER", "package_version", "package_version.id = package_file.version_id"). - Join("INNER", "package", "package.id = package_version.package_id"). + InnerJoin("package_version", "package_version.id = package_file.version_id"). + InnerJoin("package", "package.id = package_version.package_id"). Where(cond) query := builder. Select("package_property.value, MAX(package_file.created_unix) AS created_unix"). From("package_property"). - Join("INNER", "package_file", "package_file.id = package_property.ref_id"). + InnerJoin("package_file", "package_file.id = package_property.ref_id"). Where(builder.Eq{"package_property.name": propertyName}.And(builder.In("package_property.ref_id", in2))). GroupBy("package_property.value"). OrderBy("created_unix DESC") diff --git a/models/packages/conan/search.go b/models/packages/conan/search.go index c274a7ce025..6a2cfa38f59 100644 --- a/models/packages/conan/search.go +++ b/models/packages/conan/search.go @@ -74,8 +74,8 @@ func SearchRecipes(ctx context.Context, opts *RecipeSearchOptions) ([]string, er query := builder. Select("package.name, package_version.version, package_file.id"). From("package_file"). - Join("INNER", "package_version", "package_version.id = package_file.version_id"). - Join("INNER", "package", "package.id = package_version.package_id"). + InnerJoin("package_version", "package_version.id = package_file.version_id"). + InnerJoin("package", "package.id = package_version.package_id"). Where(cond) results := make([]struct { diff --git a/models/packages/package.go b/models/packages/package.go index 05170ab3f43..373bd86d9f7 100644 --- a/models/packages/package.go +++ b/models/packages/package.go @@ -190,13 +190,15 @@ func GetPackagesByType(ctx context.Context, ownerID int64, packageType Type) ([] // DeletePackagesIfUnreferenced deletes a package if there are no associated versions func DeletePackagesIfUnreferenced(ctx context.Context) error { in := builder. - Select("package_version.package_id"). + Select("package.id"). From("package"). - Join("LEFT", "package_version", "package_version.package_id = package.id"). + LeftJoin("package_version", "package_version.package_id = package.id"). Where(builder.Expr("package_version.id IS NULL")) _, err := db.GetEngine(ctx). - Where(builder.In("package.id", in)). + // double select workaround for MySQL + // https://stackoverflow.com/questions/4471277/mysql-delete-from-with-subquery-as-condition + Where(builder.In("package.id", builder.Select("id").From(in, "temp"))). Delete(&Package{}) return err diff --git a/models/packages/package_blob.go b/models/packages/package_blob.go index d9a8314c880..8c701d4285d 100644 --- a/models/packages/package_blob.go +++ b/models/packages/package_blob.go @@ -67,7 +67,7 @@ func FindExpiredUnreferencedBlobs(ctx context.Context, olderThan time.Duration) pbs := make([]*PackageBlob, 0, 10) return pbs, db.GetEngine(ctx). Table("package_blob"). - Join("LEFT OUTER", "package_file", "package_file.blob_id = package_blob.id"). + Join("LEFT", "package_file", "package_file.blob_id = package_blob.id"). Where("package_file.id IS NULL AND package_blob.created_unix < ?", time.Now().Add(-olderThan).Unix()). Find(&pbs) } diff --git a/models/packages/package_file.go b/models/packages/package_file.go index df36467548c..8f304ce8ac4 100644 --- a/models/packages/package_file.go +++ b/models/packages/package_file.go @@ -147,7 +147,7 @@ func (opts *PackageFileSearchOptions) toConds() builder.Cond { in := builder. Select("package_version.id"). From("package_version"). - Join("INNER", "package", "package.id = package_version.package_id"). + InnerJoin("package", "package.id = package_version.package_id"). Where(versionCond) cond = cond.And(builder.In("package_file.version_id", in)) diff --git a/models/packages/package_version.go b/models/packages/package_version.go index f7c6d4dc586..78e76c50545 100644 --- a/models/packages/package_version.go +++ b/models/packages/package_version.go @@ -12,16 +12,13 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "xorm.io/builder" ) -var ( - // ErrDuplicatePackageVersion indicates a duplicated package version error - ErrDuplicatePackageVersion = errors.New("Package version does exist already") - // ErrPackageVersionNotExist indicates a package version not exist error - ErrPackageVersionNotExist = errors.New("Package version does not exist") -) +// ErrDuplicatePackageVersion indicates a duplicated package version error +var ErrDuplicatePackageVersion = errors.New("Package version already exists") func init() { db.RegisterModel(new(PackageVersion)) @@ -99,75 +96,49 @@ func GetInternalVersionByNameAndVersion(ctx context.Context, ownerID int64, pack } func getVersionByNameAndVersion(ctx context.Context, ownerID int64, packageType Type, name, version string, isInternal bool) (*PackageVersion, error) { - var cond builder.Cond = builder.Eq{ - "package.owner_id": ownerID, - "package.type": packageType, - "package.lower_name": strings.ToLower(name), - "package_version.is_internal": isInternal, - } - pv := &PackageVersion{ - LowerVersion: strings.ToLower(version), - } - has, err := db.GetEngine(ctx). - Join("INNER", "package", "package.id = package_version.package_id"). - Where(cond). - Get(pv) + pvs, _, err := SearchVersions(ctx, &PackageSearchOptions{ + OwnerID: ownerID, + Type: packageType, + Name: SearchValue{ + ExactMatch: true, + Value: name, + }, + Version: SearchValue{ + ExactMatch: true, + Value: version, + }, + IsInternal: isInternal, + Paginator: db.NewAbsoluteListOptions(0, 1), + }) if err != nil { return nil, err } - if !has { + if len(pvs) == 0 { return nil, ErrPackageNotExist } - - return pv, nil + return pvs[0], nil } // GetVersionsByPackageType gets all versions of a specific type func GetVersionsByPackageType(ctx context.Context, ownerID int64, packageType Type) ([]*PackageVersion, error) { - var cond builder.Cond = builder.Eq{ - "package.owner_id": ownerID, - "package.type": packageType, - "package_version.is_internal": false, - } - - pvs := make([]*PackageVersion, 0, 10) - return pvs, db.GetEngine(ctx). - Where(cond). - Join("INNER", "package", "package.id = package_version.package_id"). - Find(&pvs) + pvs, _, err := SearchVersions(ctx, &PackageSearchOptions{ + OwnerID: ownerID, + Type: packageType, + }) + return pvs, err } // GetVersionsByPackageName gets all versions of a specific package func GetVersionsByPackageName(ctx context.Context, ownerID int64, packageType Type, name string) ([]*PackageVersion, error) { - var cond builder.Cond = builder.Eq{ - "package.owner_id": ownerID, - "package.type": packageType, - "package.lower_name": strings.ToLower(name), - "package_version.is_internal": false, - } - - pvs := make([]*PackageVersion, 0, 10) - return pvs, db.GetEngine(ctx). - Where(cond). - Join("INNER", "package", "package.id = package_version.package_id"). - Find(&pvs) -} - -// GetVersionsByFilename gets all versions which are linked to a filename -func GetVersionsByFilename(ctx context.Context, ownerID int64, packageType Type, filename string) ([]*PackageVersion, error) { - var cond builder.Cond = builder.Eq{ - "package.owner_id": ownerID, - "package.type": packageType, - "package_file.lower_name": strings.ToLower(filename), - "package_version.is_internal": false, - } - - pvs := make([]*PackageVersion, 0, 10) - return pvs, db.GetEngine(ctx). - Where(cond). - Join("INNER", "package_file", "package_file.version_id = package_version.id"). - Join("INNER", "package", "package.id = package_version.package_id"). - Find(&pvs) + pvs, _, err := SearchVersions(ctx, &PackageSearchOptions{ + OwnerID: ownerID, + Type: packageType, + Name: SearchValue{ + ExactMatch: true, + Value: name, + }, + }) + return pvs, err } // DeleteVersionByID deletes a version by id @@ -183,21 +154,32 @@ func HasVersionFileReferences(ctx context.Context, versionID int64) (bool, error }) } +// SearchValue describes a value to search +// If ExactMatch is true, the field must match the value otherwise a LIKE search is performed. +type SearchValue struct { + Value string + ExactMatch bool +} + // PackageSearchOptions are options for SearchXXX methods +// Besides IsInternal are all fields optional and are not used if they have their default value (nil, "", 0) type PackageSearchOptions struct { - OwnerID int64 - RepoID int64 - Type string - PackageID int64 - QueryName string - QueryVersion string - Properties map[string]string - Sort string + OwnerID int64 + RepoID int64 + Type Type + PackageID 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 + IsInternal bool + HasFileWithName string // only results are found which are associated with a file with the specific name + HasFiles util.OptionalBool // only results are found which have associated files + Sort string db.Paginator } func (opts *PackageSearchOptions) toConds() builder.Cond { - var cond builder.Cond = builder.Eq{"package_version.is_internal": false} + var cond builder.Cond = builder.Eq{"package_version.is_internal": opts.IsInternal} if opts.OwnerID != 0 { cond = cond.And(builder.Eq{"package.owner_id": opts.OwnerID}) @@ -211,11 +193,19 @@ func (opts *PackageSearchOptions) toConds() builder.Cond { if opts.PackageID != 0 { cond = cond.And(builder.Eq{"package.id": opts.PackageID}) } - if opts.QueryName != "" { - cond = cond.And(builder.Like{"package.lower_name", strings.ToLower(opts.QueryName)}) + if opts.Name.Value != "" { + if opts.Name.ExactMatch { + cond = cond.And(builder.Eq{"package.lower_name": strings.ToLower(opts.Name.Value)}) + } else { + cond = cond.And(builder.Like{"package.lower_name", strings.ToLower(opts.Name.Value)}) + } } - if opts.QueryVersion != "" { - cond = cond.And(builder.Like{"package_version.lower_version", strings.ToLower(opts.QueryVersion)}) + if opts.Version.Value != "" { + if opts.Version.ExactMatch { + cond = cond.And(builder.Eq{"package_version.lower_version": strings.ToLower(opts.Version.Value)}) + } else { + cond = cond.And(builder.Like{"package_version.lower_version", strings.ToLower(opts.Version.Value)}) + } } if len(opts.Properties) != 0 { @@ -238,6 +228,22 @@ func (opts *PackageSearchOptions) toConds() builder.Cond { }) } + if opts.HasFileWithName != "" { + fileCond := builder.Expr("package_file.version_id = package_version.id").And(builder.Eq{"package_file.lower_name": strings.ToLower(opts.HasFileWithName)}) + + cond = cond.And(builder.Exists(builder.Select("package_file.id").From("package_file").Where(fileCond))) + } + + if !opts.HasFiles.IsNone() { + var filesCond builder.Cond = builder.Exists(builder.Select("package_file.id").From("package_file").Where(builder.Expr("package_file.version_id = package_version.id"))) + + if opts.HasFiles.IsFalse() { + filesCond = builder.Not{filesCond} + } + + cond = cond.And(filesCond) + } + return cond } @@ -297,20 +303,3 @@ func SearchLatestVersions(ctx context.Context, opts *PackageSearchOptions) ([]*P count, err := sess.FindAndCount(&pvs) return pvs, count, err } - -// FindVersionsByPropertyNameAndValue gets all package versions which are associated with a specific property + value -func FindVersionsByPropertyNameAndValue(ctx context.Context, packageID int64, name, value string) ([]*PackageVersion, error) { - var cond builder.Cond = builder.Eq{ - "package_property.ref_type": PropertyTypeVersion, - "package_property.name": name, - "package_property.value": value, - "package_version.package_id": packageID, - "package_version.is_internal": false, - } - - pvs := make([]*PackageVersion, 0, 5) - return pvs, db.GetEngine(ctx). - Where(cond). - Join("INNER", "package_property", "package_property.ref_id = package_version.id"). - Find(&pvs) -} diff --git a/routers/api/packages/composer/composer.go b/routers/api/packages/composer/composer.go index 22a452325ef..23de28c7f9e 100644 --- a/routers/api/packages/composer/composer.go +++ b/routers/api/packages/composer/composer.go @@ -63,8 +63,8 @@ func SearchPackages(ctx *context.Context) { opts := &packages_model.PackageSearchOptions{ OwnerID: ctx.Package.Owner.ID, - Type: string(packages_model.TypeComposer), - QueryName: ctx.FormTrim("q"), + Type: packages_model.TypeComposer, + Name: packages_model.SearchValue{Value: ctx.FormTrim("q")}, Paginator: &paginator, } if ctx.FormTrim("type") != "" { diff --git a/routers/api/packages/npm/npm.go b/routers/api/packages/npm/npm.go index 50151ee5ea6..d127134d445 100644 --- a/routers/api/packages/npm/npm.go +++ b/routers/api/packages/npm/npm.go @@ -256,7 +256,12 @@ func setPackageTag(tag string, pv *packages_model.PackageVersion, deleteOnly boo } defer committer.Close() - pvs, err := packages_model.FindVersionsByPropertyNameAndValue(ctx, pv.PackageID, npm_module.TagProperty, tag) + pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + PackageID: pv.PackageID, + Properties: map[string]string{ + npm_module.TagProperty: tag, + }, + }) if err != nil { return err } diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go index 3af7155fae6..013c0c1e335 100644 --- a/routers/api/packages/nuget/nuget.go +++ b/routers/api/packages/nuget/nuget.go @@ -39,9 +39,9 @@ func ServiceIndex(ctx *context.Context) { // SearchService https://docs.microsoft.com/en-us/nuget/api/search-query-service-resource#search-for-packages func SearchService(ctx *context.Context) { pvs, count, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - OwnerID: ctx.Package.Owner.ID, - Type: string(packages_model.TypeNuGet), - QueryName: ctx.FormTrim("q"), + OwnerID: ctx.Package.Owner.ID, + Type: packages_model.TypeNuGet, + Name: packages_model.SearchValue{Value: ctx.FormTrim("q")}, Paginator: db.NewAbsoluteListOptions( ctx.FormInt("skip"), ctx.FormInt("take"), diff --git a/routers/api/packages/rubygems/rubygems.go b/routers/api/packages/rubygems/rubygems.go index a5a9b779abb..6fdd03e8ea7 100644 --- a/routers/api/packages/rubygems/rubygems.go +++ b/routers/api/packages/rubygems/rubygems.go @@ -41,7 +41,7 @@ func EnumeratePackages(ctx *context.Context) { func EnumeratePackagesLatest(ctx *context.Context) { pvs, _, err := packages_model.SearchLatestVersions(ctx, &packages_model.PackageSearchOptions{ OwnerID: ctx.Package.Owner.ID, - Type: string(packages_model.TypeRubyGems), + Type: packages_model.TypeRubyGems, }) if err != nil { apiError(ctx, http.StatusInternalServerError, err) @@ -96,7 +96,7 @@ func ServePackageSpecification(ctx *context.Context) { return } - pvs, err := packages_model.GetVersionsByFilename(ctx, ctx.Package.Owner.ID, packages_model.TypeRubyGems, filename[:len(filename)-10]+"gem") + pvs, err := getVersionsByFilename(ctx, filename[:len(filename)-10]+"gem") if err != nil { apiError(ctx, http.StatusInternalServerError, err) return @@ -158,7 +158,7 @@ func ServePackageSpecification(ctx *context.Context) { func DownloadPackageFile(ctx *context.Context) { filename := ctx.Params("filename") - pvs, err := packages_model.GetVersionsByFilename(ctx, ctx.Package.Owner.ID, packages_model.TypeRubyGems, filename) + pvs, err := getVersionsByFilename(ctx, filename) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return @@ -283,3 +283,12 @@ func DeletePackage(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) } } + +func getVersionsByFilename(ctx *context.Context, filename string) ([]*packages_model.PackageVersion, error) { + pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + OwnerID: ctx.Package.Owner.ID, + Type: packages_model.TypeRubyGems, + HasFileWithName: filename, + }) + return pvs, err +} diff --git a/routers/api/v1/packages/package.go b/routers/api/v1/packages/package.go index 8952241222b..b445e8e2f83 100644 --- a/routers/api/v1/packages/package.go +++ b/routers/api/v1/packages/package.go @@ -56,8 +56,8 @@ func ListPackages(ctx *context.APIContext) { pvs, count, err := packages.SearchVersions(ctx, &packages.PackageSearchOptions{ OwnerID: ctx.Package.Owner.ID, - Type: packageType, - QueryName: query, + Type: packages.Type(packageType), + Name: packages.SearchValue{Value: query}, Paginator: &listOptions, }) if err != nil { diff --git a/routers/web/admin/packages.go b/routers/web/admin/packages.go index 22be37526fe..79bf025dd28 100644 --- a/routers/web/admin/packages.go +++ b/routers/web/admin/packages.go @@ -31,9 +31,9 @@ func Packages(ctx *context.Context) { sort := ctx.FormTrim("sort") pvs, total, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - QueryName: query, - Type: packageType, - Sort: sort, + Type: packages_model.Type(packageType), + Name: packages_model.SearchValue{Value: query}, + Sort: sort, Paginator: &db.ListOptions{ PageSize: setting.UI.PackagesPagingNum, Page: page, diff --git a/routers/web/repo/packages.go b/routers/web/repo/packages.go index f796bb0de5c..b4db2d5787b 100644 --- a/routers/web/repo/packages.go +++ b/routers/web/repo/packages.go @@ -32,10 +32,10 @@ func Packages(ctx *context.Context) { PageSize: setting.UI.PackagesPagingNum, Page: page, }, - OwnerID: ctx.ContextUser.ID, - RepoID: ctx.Repo.Repository.ID, - QueryName: query, - Type: packageType, + OwnerID: ctx.ContextUser.ID, + RepoID: ctx.Repo.Repository.ID, + Type: packages.Type(packageType), + Name: packages.SearchValue{Value: query}, }) if err != nil { ctx.ServerError("SearchLatestVersions", err) diff --git a/routers/web/user/package.go b/routers/web/user/package.go index edbb4aadf6f..7fecf9e7688 100644 --- a/routers/web/user/package.go +++ b/routers/web/user/package.go @@ -43,9 +43,9 @@ func ListPackages(ctx *context.Context) { PageSize: setting.UI.PackagesPagingNum, Page: page, }, - OwnerID: ctx.ContextUser.ID, - Type: packageType, - QueryName: query, + OwnerID: ctx.ContextUser.ID, + Type: packages_model.Type(packageType), + Name: packages_model.SearchValue{Value: query}, }) if err != nil { ctx.ServerError("SearchLatestVersions", err) @@ -219,9 +219,12 @@ func ListPackageVersions(ctx *context.Context) { } default: pvs, total, err = packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ - Paginator: pagination, - PackageID: p.ID, - QueryVersion: query, + Paginator: pagination, + PackageID: p.ID, + Version: packages_model.SearchValue{ + ExactMatch: false, + Value: query, + }, }) if err != nil { ctx.ServerError("SearchVersions", err) diff --git a/services/packages/container/cleanup.go b/services/packages/container/cleanup.go index 91992a4d7f6..390a0b7b052 100644 --- a/services/packages/container/cleanup.go +++ b/services/packages/container/cleanup.go @@ -10,6 +10,7 @@ import ( packages_model "code.gitea.io/gitea/models/packages" container_model "code.gitea.io/gitea/models/packages/container" + "code.gitea.io/gitea/modules/util" ) // Cleanup removes expired container data @@ -43,10 +44,7 @@ func cleanupExpiredUploadedBlobs(ctx context.Context, olderThan time.Duration) e return err } - versions := make(map[int64]struct{}) for _, pf := range pfs { - versions[pf.VersionID] = struct{}{} - if err := packages_model.DeleteAllProperties(ctx, packages_model.PropertyTypeFile, pf.ID); err != nil { return err } @@ -55,19 +53,26 @@ func cleanupExpiredUploadedBlobs(ctx context.Context, olderThan time.Duration) e } } - for versionID := range versions { - has, err := packages_model.HasVersionFileReferences(ctx, versionID) - if err != nil { + pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + Type: packages_model.TypeContainer, + Version: packages_model.SearchValue{ + ExactMatch: true, + Value: container_model.UploadVersion, + }, + IsInternal: true, + HasFiles: util.OptionalBoolFalse, + }) + if err != nil { + return err + } + + for _, pv := range pvs { + if err := packages_model.DeleteAllProperties(ctx, packages_model.PropertyTypeVersion, pv.ID); err != nil { return err } - if !has { - if err := packages_model.DeleteAllProperties(ctx, packages_model.PropertyTypeVersion, versionID); err != nil { - return err - } - if err := packages_model.DeleteVersionByID(ctx, versionID); err != nil { - return err - } + if err := packages_model.DeleteVersionByID(ctx, pv.ID); err != nil { + return err } } diff --git a/services/packages/packages.go b/services/packages/packages.go index 7f90f80bafc..7f25fce5b85 100644 --- a/services/packages/packages.go +++ b/services/packages/packages.go @@ -336,7 +336,7 @@ func DeletePackageFile(ctx context.Context, pf *packages_model.PackageFile) erro return packages_model.DeleteFileByID(ctx, pf.ID) } -// Cleanup removes old unreferenced package blobs +// Cleanup removes expired package data func Cleanup(unused context.Context, olderThan time.Duration) error { ctx, committer, err := db.TxContext() if err != nil { @@ -345,24 +345,20 @@ func Cleanup(unused context.Context, olderThan time.Duration) error { defer committer.Close() if err := container_service.Cleanup(ctx, olderThan); err != nil { - log.Error("hier") return err } if err := packages_model.DeletePackagesIfUnreferenced(ctx); err != nil { - log.Error("hier2") return err } pbs, err := packages_model.FindExpiredUnreferencedBlobs(ctx, olderThan) if err != nil { - log.Error("hier3") return err } for _, pb := range pbs { if err := packages_model.DeleteBlobByID(ctx, pb.ID); err != nil { - log.Error("hier4") return err } } @@ -403,10 +399,9 @@ func GetFileStreamByPackageVersionAndFileID(ctx context.Context, owner *user_mod pv, err := packages_model.GetVersionByID(ctx, versionID) if err != nil { - if err == packages_model.ErrPackageVersionNotExist { - return nil, nil, packages_model.ErrPackageNotExist + if err != packages_model.ErrPackageNotExist { + log.Error("Error getting package version: %v", err) } - log.Error("Error getting package version: %v", err) return nil, nil, err }