From b36e2ca4195298d2e4516e3022b953543f62f470 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 29 Oct 2023 15:14:47 +0100 Subject: [PATCH] List all Debian package versions in `Packages` (#27786) Closes #27783 This PR lists all and not only the latest package versions in the `Packages` index. --- models/packages/container/search.go | 3 ++ models/packages/debian/search.go | 45 +++++++++++------- models/packages/package_version.go | 3 ++ services/packages/debian/repository.go | 15 +++--- tests/integration/api_packages_debian_test.go | 46 +++++++++++-------- 5 files changed, 70 insertions(+), 42 deletions(-) diff --git a/models/packages/container/search.go b/models/packages/container/search.go index 9a16b3ae4..5df35117c 100644 --- a/models/packages/container/search.go +++ b/models/packages/container/search.go @@ -207,6 +207,9 @@ func (opts *ImageTagsSearchOptions) configureOrderBy(e db.Engine) { default: e.Desc("package_version.created_unix") } + + // Sort by id for stable order with duplicates in the other field + e.Asc("package_version.id") } // SearchImageTags gets a sorted list of the tags of an image diff --git a/models/packages/debian/search.go b/models/packages/debian/search.go index c63a31930..77c4a1846 100644 --- a/models/packages/debian/search.go +++ b/models/packages/debian/search.go @@ -21,8 +21,7 @@ type PackageSearchOptions struct { Architecture string } -// SearchLatestPackages gets the latest packages matching the search options -func SearchLatestPackages(ctx context.Context, opts *PackageSearchOptions) ([]*packages.PackageFileDescriptor, error) { +func (opts *PackageSearchOptions) toCond() builder.Cond { var cond builder.Cond = builder.Eq{ "package_file.is_lead": true, "package.type": packages.TypeDebian, @@ -62,28 +61,40 @@ func SearchLatestPackages(ctx context.Context, opts *PackageSearchOptions) ([]*p }) } - cond = cond. - And(builder.Expr("pv2.id IS NULL")) + return cond +} - joinCond := builder. - Expr("package_version.package_id = pv2.package_id AND (package_version.created_unix < pv2.created_unix OR (package_version.created_unix = pv2.created_unix AND package_version.id < pv2.id))"). - And(builder.Eq{"pv2.is_internal": false}) +// ExistPackages tests if there are packages matching the search options +func ExistPackages(ctx context.Context, opts *PackageSearchOptions) (bool, error) { + return db.GetEngine(ctx). + Table("package_file"). + Join("INNER", "package_version", "package_version.id = package_file.version_id"). + Join("INNER", "package", "package.id = package_version.package_id"). + Where(opts.toCond()). + Exist(new(packages.PackageFile)) +} - pfs := make([]*packages.PackageFile, 0, 10) - err := db.GetEngine(ctx). +// SearchPackages gets the packages matching the search options +func SearchPackages(ctx context.Context, opts *PackageSearchOptions, iter func(*packages.PackageFileDescriptor)) error { + return db.GetEngine(ctx). Table("package_file"). Select("package_file.*"). Join("INNER", "package_version", "package_version.id = package_file.version_id"). - Join("LEFT", "package_version pv2", joinCond). Join("INNER", "package", "package.id = package_version.package_id"). - Where(cond). - Desc("package_version.created_unix"). - Find(&pfs) - if err != nil { - return nil, err - } + Where(opts.toCond()). + Asc("package.lower_name", "package_version.created_unix"). + Iterate(new(packages.PackageFile), func(_ int, bean any) error { + pf := bean.(*packages.PackageFile) - return packages.GetPackageFileDescriptors(ctx, pfs) + pfd, err := packages.GetPackageFileDescriptor(ctx, pf) + if err != nil { + return err + } + + iter(pfd) + + return nil + }) } // GetDistributions gets all available distributions diff --git a/models/packages/package_version.go b/models/packages/package_version.go index 4392c70f7..9999fc4da 100644 --- a/models/packages/package_version.go +++ b/models/packages/package_version.go @@ -278,6 +278,9 @@ func (opts *PackageSearchOptions) configureOrderBy(e db.Engine) { default: e.Desc("package_version.created_unix") } + + // Sort by id for stable order with duplicates in the other field + e.Asc("package_version.id") } // SearchVersions gets all versions of packages matching the search options diff --git a/services/packages/debian/repository.go b/services/packages/debian/repository.go index 1cc687ec8..cbde53f96 100644 --- a/services/packages/debian/repository.go +++ b/services/packages/debian/repository.go @@ -165,18 +165,17 @@ func buildRepositoryFiles(ctx context.Context, ownerID int64, repoVersion *packa // https://wiki.debian.org/DebianRepository/Format#A.22Packages.22_Indices func buildPackagesIndices(ctx context.Context, ownerID int64, repoVersion *packages_model.PackageVersion, distribution, component, architecture string) error { - pfds, err := debian_model.SearchLatestPackages(ctx, &debian_model.PackageSearchOptions{ + opts := &debian_model.PackageSearchOptions{ OwnerID: ownerID, Distribution: distribution, Component: component, Architecture: architecture, - }) - if err != nil { - return err } // Delete the package indices if there are no packages - if len(pfds) == 0 { + if has, err := debian_model.ExistPackages(ctx, opts); err != nil { + return err + } else if !has { key := fmt.Sprintf("%s|%s|%s", distribution, component, architecture) for _, filename := range []string{"Packages", "Packages.gz", "Packages.xz"} { pf, err := packages_model.GetFileForVersionByName(ctx, repoVersion.ID, filename, key) @@ -211,7 +210,7 @@ func buildPackagesIndices(ctx context.Context, ownerID int64, repoVersion *packa w := io.MultiWriter(packagesContent, gzw, xzw) addSeparator := false - for _, pfd := range pfds { + if err := debian_model.SearchPackages(ctx, opts, func(pfd *packages_model.PackageFileDescriptor) { if addSeparator { fmt.Fprintln(w) } @@ -225,6 +224,8 @@ func buildPackagesIndices(ctx context.Context, ownerID int64, repoVersion *packa fmt.Fprintf(w, "SHA1: %s\n", pfd.Blob.HashSHA1) fmt.Fprintf(w, "SHA256: %s\n", pfd.Blob.HashSHA256) fmt.Fprintf(w, "SHA512: %s\n", pfd.Blob.HashSHA512) + }); err != nil { + return err } gzw.Close() @@ -238,7 +239,7 @@ func buildPackagesIndices(ctx context.Context, ownerID int64, repoVersion *packa {"Packages.gz", packagesGzipContent}, {"Packages.xz", packagesXzContent}, } { - _, err = packages_service.AddFileToPackageVersionInternal( + _, err := packages_service.AddFileToPackageVersionInternal( ctx, repoVersion, &packages_service.PackageFileCreationInfo{ diff --git a/tests/integration/api_packages_debian_test.go b/tests/integration/api_packages_debian_test.go index f796d617d..6c43f72a7 100644 --- a/tests/integration/api_packages_debian_test.go +++ b/tests/integration/api_packages_debian_test.go @@ -31,6 +31,7 @@ func TestPackageDebian(t *testing.T) { packageName := "gitea" packageVersion := "1.0.3" + packageVersion2 := "1.0.4" packageDescription := "Package Description" createArchive := func(name, version, architecture string) io.Reader { @@ -80,11 +81,11 @@ func TestPackageDebian(t *testing.T) { for _, component := range components { for _, architecture := range architectures { t.Run(fmt.Sprintf("[Component:%s,Architecture:%s]", component, architecture), func(t *testing.T) { + uploadURL := fmt.Sprintf("%s/pool/%s/%s/upload", rootURL, distribution, component) + t.Run("Upload", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - uploadURL := fmt.Sprintf("%s/pool/%s/%s/upload", rootURL, distribution, component) - req := NewRequestWithBody(t, "PUT", uploadURL, bytes.NewReader([]byte{})) MakeRequest(t, req, http.StatusUnauthorized) @@ -100,18 +101,17 @@ func TestPackageDebian(t *testing.T) { AddBasicAuthHeader(req, user.Name) MakeRequest(t, req, http.StatusCreated) - pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeDebian) + pv, err := packages.GetVersionByNameAndVersion(db.DefaultContext, user.ID, packages.TypeDebian, packageName, packageVersion) assert.NoError(t, err) - assert.Len(t, pvs, 1) - pd, err := packages.GetPackageDescriptor(db.DefaultContext, pvs[0]) + pd, err := packages.GetPackageDescriptor(db.DefaultContext, pv) assert.NoError(t, err) assert.Nil(t, pd.SemVer) assert.IsType(t, &debian_module.Metadata{}, pd.Metadata) assert.Equal(t, packageName, pd.Package.Name) assert.Equal(t, packageVersion, pd.Version.Version) - pfs, err := packages.GetFilesByVersionID(db.DefaultContext, pvs[0].ID) + pfs, err := packages.GetFilesByVersionID(db.DefaultContext, pv.ID) assert.NoError(t, err) assert.NotEmpty(t, pfs) assert.Condition(t, func() bool { @@ -162,17 +162,23 @@ func TestPackageDebian(t *testing.T) { t.Run("Packages", func(t *testing.T) { defer tests.PrintCurrentTest(t)() + req := NewRequestWithBody(t, "PUT", uploadURL, createArchive(packageName, packageVersion2, architecture)) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusCreated) + url := fmt.Sprintf("%s/dists/%s/%s/binary-%s/Packages", rootURL, distribution, component, architecture) - req := NewRequest(t, "GET", url) + req = NewRequest(t, "GET", url) resp := MakeRequest(t, req, http.StatusOK) body := resp.Body.String() - assert.Contains(t, body, "Package: "+packageName) - assert.Contains(t, body, "Version: "+packageVersion) - assert.Contains(t, body, "Architecture: "+architecture) - assert.Contains(t, body, fmt.Sprintf("Filename: pool/%s/%s/%s_%s_%s.deb", distribution, component, packageName, packageVersion, architecture)) + assert.Contains(t, body, "Package: "+packageName+"\n") + assert.Contains(t, body, "Version: "+packageVersion+"\n") + assert.Contains(t, body, "Version: "+packageVersion2+"\n") + assert.Contains(t, body, "Architecture: "+architecture+"\n") + assert.Contains(t, body, fmt.Sprintf("Filename: pool/%s/%s/%s_%s_%s.deb\n", distribution, component, packageName, packageVersion, architecture)) + assert.Contains(t, body, fmt.Sprintf("Filename: pool/%s/%s/%s_%s_%s.deb\n", distribution, component, packageName, packageVersion2, architecture)) req = NewRequest(t, "GET", url+".gz") MakeRequest(t, req, http.StatusOK) @@ -198,14 +204,14 @@ func TestPackageDebian(t *testing.T) { body := resp.Body.String() - assert.Contains(t, body, "Components: "+strings.Join(components, " ")) - assert.Contains(t, body, "Architectures: "+strings.Join(architectures, " ")) + assert.Contains(t, body, "Components: "+strings.Join(components, " ")+"\n") + assert.Contains(t, body, "Architectures: "+strings.Join(architectures, " ")+"\n") for _, component := range components { for _, architecture := range architectures { - assert.Contains(t, body, fmt.Sprintf("%s/binary-%s/Packages", component, architecture)) - assert.Contains(t, body, fmt.Sprintf("%s/binary-%s/Packages.gz", component, architecture)) - assert.Contains(t, body, fmt.Sprintf("%s/binary-%s/Packages.xz", component, architecture)) + assert.Contains(t, body, fmt.Sprintf("%s/binary-%s/Packages\n", component, architecture)) + assert.Contains(t, body, fmt.Sprintf("%s/binary-%s/Packages.gz\n", component, architecture)) + assert.Contains(t, body, fmt.Sprintf("%s/binary-%s/Packages.xz\n", component, architecture)) } } @@ -241,6 +247,10 @@ func TestPackageDebian(t *testing.T) { AddBasicAuthHeader(req, user.Name) MakeRequest(t, req, http.StatusNoContent) + req = NewRequest(t, "DELETE", fmt.Sprintf("%s/pool/%s/%s/%s/%s/%s", rootURL, distribution, component, packageName, packageVersion2, architecture)) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusNoContent) + req = NewRequest(t, "GET", fmt.Sprintf("%s/dists/%s/%s/binary-%s/Packages", rootURL, distribution, component, architecture)) MakeRequest(t, req, http.StatusNotFound) } @@ -250,7 +260,7 @@ func TestPackageDebian(t *testing.T) { body := resp.Body.String() - assert.Contains(t, body, "Components: "+strings.Join(components, " ")) - assert.Contains(t, body, "Architectures: "+architectures[1]) + assert.Contains(t, body, "Components: "+strings.Join(components, " ")+"\n") + assert.Contains(t, body, "Architectures: "+architectures[1]+"\n") }) }