Package registry changes (#19305) * removed debug logs * fixed SELECT * removed unneeded error type * use common SearchVersions method * remove empty container upload versions * return err

Changes

17 changed files (+185/-150)

• 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) + }
• 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")
• 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 {
• 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
• 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) }
• 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))
• 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) - }
• 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") != "" {
• 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 }
• 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"),
• 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 + }
• 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 {
• 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,
• 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)
• 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)
• 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 } }
• 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 }