From bc72fd7f4614a6f576e7835adc10929e9b460b7d Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Fri, 5 Apr 2024 09:53:41 +0200 Subject: [PATCH] feat: improve nuget nuspec api --- modules/packages/nuget/metadata.go | 180 ++++++------------ routers/api/packages/nuget/nuget.go | 86 +++++---- services/doctor/packages_nuget.go | 161 ++++++++++++++++ tests/integration/api_packages_nuget_test.go | 40 ++-- .../integration/doctor_packages_nuget_test.go | 121 ++++++++++++ 5 files changed, 413 insertions(+), 175 deletions(-) create mode 100644 services/doctor/packages_nuget.go create mode 100644 tests/integration/doctor_packages_nuget_test.go diff --git a/modules/packages/nuget/metadata.go b/modules/packages/nuget/metadata.go index b32151cdf5..264b4612fb 100644 --- a/modules/packages/nuget/metadata.go +++ b/modules/packages/nuget/metadata.go @@ -48,10 +48,11 @@ const maxNuspecFileSize = 3 * 1024 * 1024 // Package represents a Nuget package type Package struct { - PackageType PackageType - ID string - Version string - Metadata *Metadata + PackageType PackageType + ID string + Version string + Metadata *Metadata + NuspecContent *bytes.Buffer } // Metadata represents the metadata of a Nuget package @@ -71,50 +72,34 @@ type Dependency struct { Version string `json:"version"` } -type nuspecPackageType struct { - Name string `xml:"name,attr"` -} - -type nuspecPackageTypes struct { - PackageType []nuspecPackageType `xml:"packageType"` -} - -type nuspecRepository struct { - URL string `xml:"url,attr,omitempty"` - Type string `xml:"type,attr,omitempty"` -} -type nuspecDependency struct { - ID string `xml:"id,attr"` - Version string `xml:"version,attr"` - Exclude string `xml:"exclude,attr,omitempty"` -} - -type nuspecGroup struct { - TargetFramework string `xml:"targetFramework,attr"` - Dependency []nuspecDependency `xml:"dependency"` -} - -type nuspecDependencies struct { - Group []nuspecGroup `xml:"group"` -} - -type nuspeceMetadata struct { - ID string `xml:"id"` - Version string `xml:"version"` - Authors string `xml:"authors"` - RequireLicenseAcceptance bool `xml:"requireLicenseAcceptance,omitempty"` - ProjectURL string `xml:"projectUrl,omitempty"` - Description string `xml:"description"` - ReleaseNotes string `xml:"releaseNotes,omitempty"` - PackageTypes *nuspecPackageTypes `xml:"packageTypes,omitempty"` - Repository *nuspecRepository `xml:"repository,omitempty"` - Dependencies *nuspecDependencies `xml:"dependencies,omitempty"` -} - type nuspecPackage struct { - XMLName xml.Name `xml:"package"` - Xmlns string `xml:"xmlns,attr"` - Metadata nuspeceMetadata `xml:"metadata"` + Metadata struct { + ID string `xml:"id"` + Version string `xml:"version"` + Authors string `xml:"authors"` + RequireLicenseAcceptance bool `xml:"requireLicenseAcceptance"` + ProjectURL string `xml:"projectUrl"` + Description string `xml:"description"` + ReleaseNotes string `xml:"releaseNotes"` + PackageTypes struct { + PackageType []struct { + Name string `xml:"name,attr"` + } `xml:"packageType"` + } `xml:"packageTypes"` + Repository struct { + URL string `xml:"url,attr"` + } `xml:"repository"` + Dependencies struct { + Group []struct { + TargetFramework string `xml:"targetFramework,attr"` + Dependency []struct { + ID string `xml:"id,attr"` + Version string `xml:"version,attr"` + Exclude string `xml:"exclude,attr"` + } `xml:"dependency"` + } `xml:"group"` + } `xml:"dependencies"` + } `xml:"metadata"` } // ParsePackageMetaData parses the metadata of a Nuget package file @@ -146,8 +131,9 @@ func ParsePackageMetaData(r io.ReaderAt, size int64) (*Package, error) { // ParseNuspecMetaData parses a Nuspec file to retrieve the metadata of a Nuget package func ParseNuspecMetaData(r io.Reader) (*Package, error) { + var nuspecBuf bytes.Buffer var p nuspecPackage - if err := xml.NewDecoder(r).Decode(&p); err != nil { + if err := xml.NewDecoder(io.TeeReader(r, &nuspecBuf)).Decode(&p); err != nil { return nil, err } @@ -165,12 +151,10 @@ func ParseNuspecMetaData(r io.Reader) (*Package, error) { } packageType := DependencyPackage - if p.Metadata.PackageTypes != nil { - for _, pt := range p.Metadata.PackageTypes.PackageType { - if pt.Name == "SymbolsPackage" { - packageType = SymbolsPackage - break - } + for _, pt := range p.Metadata.PackageTypes.PackageType { + if pt.Name == "SymbolsPackage" { + packageType = SymbolsPackage + break } } @@ -179,34 +163,32 @@ func ParseNuspecMetaData(r io.Reader) (*Package, error) { ReleaseNotes: p.Metadata.ReleaseNotes, Authors: p.Metadata.Authors, ProjectURL: p.Metadata.ProjectURL, + RepositoryURL: p.Metadata.Repository.URL, RequireLicenseAcceptance: p.Metadata.RequireLicenseAcceptance, Dependencies: make(map[string][]Dependency), } - if p.Metadata.Repository != nil { - m.RepositoryURL = p.Metadata.Repository.URL - } - if p.Metadata.Dependencies != nil { - for _, group := range p.Metadata.Dependencies.Group { - deps := make([]Dependency, 0, len(group.Dependency)) - for _, dep := range group.Dependency { - if dep.ID == "" || dep.Version == "" { - continue - } - deps = append(deps, Dependency{ - ID: dep.ID, - Version: dep.Version, - }) - } - if len(deps) > 0 { - m.Dependencies[group.TargetFramework] = deps + + for _, group := range p.Metadata.Dependencies.Group { + deps := make([]Dependency, 0, len(group.Dependency)) + for _, dep := range group.Dependency { + if dep.ID == "" || dep.Version == "" { + continue } + deps = append(deps, Dependency{ + ID: dep.ID, + Version: dep.Version, + }) + } + if len(deps) > 0 { + m.Dependencies[group.TargetFramework] = deps } } return &Package{ - PackageType: packageType, - ID: p.Metadata.ID, - Version: toNormalizedVersion(v), - Metadata: m, + PackageType: packageType, + ID: p.Metadata.ID, + Version: toNormalizedVersion(v), + Metadata: m, + NuspecContent: &nuspecBuf, }, nil } @@ -225,51 +207,3 @@ func toNormalizedVersion(v *version.Version) string { } return buf.String() } - -// returning any here because we use a private type and we don't need the type for xml marshalling -func GenerateNuspec(pd *Package) any { - m := nuspeceMetadata{ - ID: pd.ID, - Version: pd.Version, - Authors: pd.Metadata.Authors, - Description: pd.Metadata.Description, - ProjectURL: pd.Metadata.ProjectURL, - RequireLicenseAcceptance: pd.Metadata.RequireLicenseAcceptance, - } - - if pd.Metadata.RepositoryURL != "" { - m.Repository = &nuspecRepository{ - URL: pd.Metadata.RepositoryURL, - } - } - - groups := len(pd.Metadata.Dependencies) - if groups > 0 { - m.Dependencies = &nuspecDependencies{ - Group: make([]nuspecGroup, 0, groups), - } - - for tgf, deps := range pd.Metadata.Dependencies { - if len(deps) == 0 { - continue - } - gDeps := make([]nuspecDependency, 0, len(deps)) - for _, dep := range deps { - gDeps = append(gDeps, nuspecDependency{ - ID: dep.ID, - Version: dep.Version, - }) - } - - m.Dependencies.Group = append(m.Dependencies.Group, nuspecGroup{ - TargetFramework: tgf, - Dependency: gDeps, - }) - } - } - - return &nuspecPackage{ - Xmlns: "http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd", - Metadata: m, - } -} diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go index 0eb817c1a3..09156ece6b 100644 --- a/routers/api/packages/nuget/nuget.go +++ b/routers/api/packages/nuget/nuget.go @@ -395,49 +395,28 @@ func DownloadPackageFile(ctx *context.Context) { packageVersion := ctx.Params("version") filename := ctx.Params("filename") - if filename == fmt.Sprintf("%s.nuspec", packageName) { - pv, err := packages_model.GetVersionByNameAndVersion(ctx, ctx.Package.Owner.ID, packages_model.TypeNuGet, packageName, packageVersion) - if err != nil { + s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + ctx, + &packages_service.PackageInfo{ + Owner: ctx.Package.Owner, + PackageType: packages_model.TypeNuGet, + Name: packageName, + Version: packageVersion, + }, + &packages_service.PackageFileInfo{ + Filename: filename, + }, + ) + if err != nil { + if err == packages_model.ErrPackageNotExist || err == packages_model.ErrPackageFileNotExist { apiError(ctx, http.StatusNotFound, err) return } - - pd, err := packages_model.GetPackageDescriptor(ctx, pv) - if err != nil { - apiError(ctx, http.StatusInternalServerError, err) - return - } - pkg := &nuget_module.Package{ - ID: pd.Package.Name, - Version: packageVersion, - Metadata: pd.Metadata.(*nuget_module.Metadata), - } - - xmlResponse(ctx, http.StatusOK, nuget_module.GenerateNuspec(pkg)) - } else { - s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( - ctx, - &packages_service.PackageInfo{ - Owner: ctx.Package.Owner, - PackageType: packages_model.TypeNuGet, - Name: packageName, - Version: packageVersion, - }, - &packages_service.PackageFileInfo{ - Filename: filename, - }, - ) - if err != nil { - if err == packages_model.ErrPackageNotExist || err == packages_model.ErrPackageFileNotExist { - apiError(ctx, http.StatusNotFound, err) - return - } - apiError(ctx, http.StatusInternalServerError, err) - return - } - - helper.ServePackageFile(ctx, s, u, pf) + apiError(ctx, http.StatusInternalServerError, err) + return } + + helper.ServePackageFile(ctx, s, u, pf) } // UploadPackage creates a new package with the metadata contained in the uploaded nupgk file @@ -453,7 +432,7 @@ func UploadPackage(ctx *context.Context) { return } - _, _, err := packages_service.CreatePackageAndAddFile( + pv, _, err := packages_service.CreatePackageAndAddFile( ctx, &packages_service.PackageCreationInfo{ PackageInfo: packages_service.PackageInfo{ @@ -487,6 +466,33 @@ func UploadPackage(ctx *context.Context) { return } + nuspecBuf, err := packages_module.CreateHashedBufferFromReaderWithSize(np.NuspecContent, np.NuspecContent.Len()) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + defer nuspecBuf.Close() + + _, err = packages_service.AddFileToPackageVersionInternal( + ctx, + pv, + &packages_service.PackageFileCreationInfo{ + PackageFileInfo: packages_service.PackageFileInfo{ + Filename: strings.ToLower(fmt.Sprintf("%s.nuspec", np.ID)), + }, + Data: nuspecBuf, + }, + ) + if err != nil { + switch err { + case packages_service.ErrQuotaTotalCount, packages_service.ErrQuotaTypeSize, packages_service.ErrQuotaTotalSize: + apiError(ctx, http.StatusForbidden, err) + default: + apiError(ctx, http.StatusInternalServerError, err) + } + return + } + ctx.Status(http.StatusCreated) } diff --git a/services/doctor/packages_nuget.go b/services/doctor/packages_nuget.go new file mode 100644 index 0000000000..8c0a2d856d --- /dev/null +++ b/services/doctor/packages_nuget.go @@ -0,0 +1,161 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package doctor + +import ( + "context" + "fmt" + "slices" + "strings" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/packages" + "code.gitea.io/gitea/modules/log" + packages_module "code.gitea.io/gitea/modules/packages" + nuget_module "code.gitea.io/gitea/modules/packages/nuget" + packages_service "code.gitea.io/gitea/services/packages" + + "xorm.io/builder" +) + +func init() { + Register(&Check{ + Title: "Extract Nuget Nuspec Files to content store", + Name: "packages-nuget-nuspec", + IsDefault: false, + Run: PackagesNugetNuspecCheck, + Priority: 15, + InitStorage: true, + }) +} + +func PackagesNugetNuspecCheck(ctx context.Context, logger log.Logger, autofix bool) error { + found := 0 + fixed := 0 + errors := 0 + + err := db.Iterate(ctx, builder.Eq{"package.type": packages.TypeNuGet, "package.is_internal": false}, func(ctx context.Context, pkg *packages.Package) error { + logger.Info("Processing package %s", pkg.Name) + + pvs, _, err := packages.SearchVersions(ctx, &packages.PackageSearchOptions{ + Type: packages.TypeNuGet, + PackageID: pkg.ID, + }) + if err != nil { + // Should never happen + logger.Error("Failed to search for versions for package %s: %v", pkg.Name, err) + return err + } + + logger.Info("Found %d versions for package %s", len(pvs), pkg.Name) + + for _, pv := range pvs { + + pfs, err := packages.GetFilesByVersionID(ctx, pv.ID) + if err != nil { + logger.Error("Failed to get files for package version %s %s: %v", pkg.Name, pv.Version, err) + errors++ + continue + } + + if slices.ContainsFunc(pfs, func(pf *packages.PackageFile) bool { return strings.HasSuffix(pf.LowerName, ".nuspec") }) { + logger.Debug("Nuspec file already exists for %s %s", pkg.Name, pv.Version) + continue + } + + nupkgIdx := slices.IndexFunc(pfs, func(pf *packages.PackageFile) bool { return pf.IsLead }) + + if nupkgIdx < 0 { + logger.Error("Missing nupkg file for %s %s", pkg.Name, pv.Version) + errors++ + continue + } + + pf := pfs[nupkgIdx] + + logger.Warn("Missing nuspec file found for %s %s", pkg.Name, pv.Version) + found++ + + if !autofix { + continue + } + + s, _, _, err := packages_service.GetPackageFileStream(ctx, pf) + if err != nil { + logger.Error("Failed to get nupkg file stream for %s %s: %v", pkg.Name, pv.Version, err) + errors++ + continue + } + defer s.Close() + + buf, err := packages_module.CreateHashedBufferFromReader(s) + if err != nil { + logger.Error("Failed to create hashed buffer for nupkg from reader for %s %s: %v", pkg.Name, pv.Version, err) + errors++ + continue + } + defer buf.Close() + + np, err := nuget_module.ParsePackageMetaData(buf, buf.Size()) + if err != nil { + logger.Error("Failed to parse package metadata for %s %s: %v", pkg.Name, pv.Version, err) + errors++ + continue + } + + nuspecBuf, err := packages_module.CreateHashedBufferFromReaderWithSize(np.NuspecContent, np.NuspecContent.Len()) + if err != nil { + logger.Error("Failed to create hashed buffer for nuspec from reader for %s %s: %v", pkg.Name, pv.Version, err) + errors++ + continue + } + defer nuspecBuf.Close() + + _, err = packages_service.AddFileToPackageVersionInternal( + ctx, + pv, + &packages_service.PackageFileCreationInfo{ + PackageFileInfo: packages_service.PackageFileInfo{ + Filename: fmt.Sprintf("%s.nuspec", pkg.LowerName), + }, + Data: nuspecBuf, + IsLead: false, + }, + ) + if err != nil { + logger.Error("Failed to add nuspec file for %s %s: %v", pkg.Name, pv.Version, err) + errors++ + continue + } + + fixed++ + } + + return nil + }) + if err != nil { + logger.Error("Failed to iterate over users: %v", err) + return err + } + + if autofix { + if fixed > 0 { + logger.Info("Fixed %d package versions by extracting nuspec files", fixed) + } else { + logger.Info("No package versions with missing nuspec files found") + } + } else { + if found > 0 { + logger.Info("Found %d package versions with missing nuspec files", found) + } else { + logger.Info("No package versions with missing nuspec files found") + } + } + + if errors > 0 { + return fmt.Errorf("failed to fix %d nuspec files", errors) + } + + return nil +} diff --git a/tests/integration/api_packages_nuget_test.go b/tests/integration/api_packages_nuget_test.go index eb67693010..991f37fe74 100644 --- a/tests/integration/api_packages_nuget_test.go +++ b/tests/integration/api_packages_nuget_test.go @@ -112,6 +112,20 @@ func TestPackageNuGet(t *testing.T) { return &buf } + nuspec := ` + + + ` + packageName + ` + ` + packageVersion + ` + ` + packageAuthors + ` + ` + packageDescription + ` + + + + + + + ` content, _ := io.ReadAll(createPackage(packageName, packageVersion)) url := fmt.Sprintf("/api/packages/%s/nuget", user.Name) @@ -224,7 +238,7 @@ func TestPackageNuGet(t *testing.T) { pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeNuGet) assert.NoError(t, err) - assert.Len(t, pvs, 1) + assert.Len(t, pvs, 1, "Should have one version") pd, err := packages.GetPackageDescriptor(db.DefaultContext, pvs[0]) assert.NoError(t, err) @@ -235,7 +249,7 @@ func TestPackageNuGet(t *testing.T) { pfs, err := packages.GetFilesByVersionID(db.DefaultContext, pvs[0].ID) assert.NoError(t, err) - assert.Len(t, pfs, 1) + assert.Len(t, pfs, 2, "Should have 2 files: nuget and nuspec") assert.Equal(t, fmt.Sprintf("%s.%s.nupkg", packageName, packageVersion), pfs[0].Name) assert.True(t, pfs[0].IsLead) @@ -302,16 +316,27 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) pfs, err := packages.GetFilesByVersionID(db.DefaultContext, pvs[0].ID) assert.NoError(t, err) - assert.Len(t, pfs, 3) + assert.Len(t, pfs, 4, "Should have 4 files: nupkg, snupkg, nuspec and pdb") for _, pf := range pfs { switch pf.Name { case fmt.Sprintf("%s.%s.nupkg", packageName, packageVersion): + assert.True(t, pf.IsLead) + + pb, err := packages.GetBlobByID(db.DefaultContext, pf.BlobID) + assert.NoError(t, err) + assert.Equal(t, int64(414), pb.Size) case fmt.Sprintf("%s.%s.snupkg", packageName, packageVersion): assert.False(t, pf.IsLead) pb, err := packages.GetBlobByID(db.DefaultContext, pf.BlobID) assert.NoError(t, err) assert.Equal(t, int64(616), pb.Size) + case fmt.Sprintf("%s.nuspec", packageName): + assert.False(t, pf.IsLead) + + pb, err := packages.GetBlobByID(db.DefaultContext, pf.BlobID) + assert.NoError(t, err) + assert.Equal(t, int64(453), pb.Size) case symbolFilename: assert.False(t, pf.IsLead) @@ -357,15 +382,6 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) AddBasicAuth(user.Name) resp = MakeRequest(t, req, http.StatusOK) - nuspec := `` + "\n" + - `` + - `` + packageName + `` + packageVersion + `` + packageAuthors + `` + packageDescription + `` + - `` + - // https://github.com/golang/go/issues/21399 go can't generate self-closing tags - `` + - `` + - `` - assert.Equal(t, nuspec, resp.Body.String()) checkDownloadCount(1) diff --git a/tests/integration/doctor_packages_nuget_test.go b/tests/integration/doctor_packages_nuget_test.go new file mode 100644 index 0000000000..29e4f6055f --- /dev/null +++ b/tests/integration/doctor_packages_nuget_test.go @@ -0,0 +1,121 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "archive/zip" + "bytes" + "fmt" + "io" + "strings" + "testing" + + "code.gitea.io/gitea/models/db" + packages_model "code.gitea.io/gitea/models/packages" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/log" + packages_module "code.gitea.io/gitea/modules/packages" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" + doctor "code.gitea.io/gitea/services/doctor" + packages_service "code.gitea.io/gitea/services/packages" + "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" +) + +func TestDoctorPackagesNuget(t *testing.T) { + defer tests.PrepareTestEnv(t, 1)() + // use local storage for tests because minio is too flaky + defer test.MockVariableValue(&setting.Packages.Storage.Type, setting.LocalStorageType)() + + logger := log.GetLogger("doctor") + + ctx := db.DefaultContext + + packageName := "test.package" + packageVersion := "1.0.3" + packageAuthors := "KN4CK3R" + packageDescription := "Gitea Test Package" + + createPackage := func(id, version string) io.Reader { + var buf bytes.Buffer + archive := zip.NewWriter(&buf) + w, _ := archive.Create("package.nuspec") + w.Write([]byte(` + + + ` + id + ` + ` + version + ` + ` + packageAuthors + ` + ` + packageDescription + ` + + + + + + + `)) + archive.Close() + return &buf + } + + pkg := createPackage(packageName, packageVersion) + + pkgBuf, err := packages_module.CreateHashedBufferFromReader(pkg) + assert.NoError(t, err, "Error creating hashed buffer from nupkg") + defer pkgBuf.Close() + + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + assert.NoError(t, err, "Error getting user by ID 2") + + t.Run("PackagesNugetNuspecCheck", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + pi := &packages_service.PackageInfo{ + Owner: doer, + PackageType: packages_model.TypeNuGet, + Name: packageName, + Version: packageVersion, + } + _, _, err := packages_service.CreatePackageAndAddFile( + ctx, + &packages_service.PackageCreationInfo{ + PackageInfo: *pi, + SemverCompatible: true, + Creator: doer, + Metadata: nil, + }, + &packages_service.PackageFileCreationInfo{ + PackageFileInfo: packages_service.PackageFileInfo{ + Filename: strings.ToLower(fmt.Sprintf("%s.%s.nupkg", packageName, packageVersion)), + }, + Creator: doer, + Data: pkgBuf, + IsLead: true, + }, + ) + assert.NoError(t, err, "Error creating package and adding file") + + assert.NoError(t, doctor.PackagesNugetNuspecCheck(ctx, logger, true), "Doctor check failed") + + s, _, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + ctx, + &packages_service.PackageInfo{ + Owner: doer, + PackageType: packages_model.TypeNuGet, + Name: packageName, + Version: packageVersion, + }, + &packages_service.PackageFileInfo{ + Filename: strings.ToLower(fmt.Sprintf("%s.nuspec", packageName)), + }, + ) + + assert.NoError(t, err, "Error getting nuspec file stream by package name and version") + defer s.Close() + + assert.Equal(t, fmt.Sprintf("%s.nuspec", packageName), pf.Name, "Not a nuspec") + }) +}