Merge pull request 'feat: improve nuget nuspec api' (#2996) from viceice/forgejo:feat/nuget/nuspec-api into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2996
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

modules/packages/nuget/metadata.go

@@ -52,6 +52,7 @@ type Package struct {
 	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 {
+type nuspecPackage struct {
+	Metadata struct {
 		ID                       string `xml:"id"`
 		Version                  string `xml:"version"`
 		Authors                  string `xml:"authors"`
-	RequireLicenseAcceptance bool                `xml:"requireLicenseAcceptance,omitempty"`
-	ProjectURL               string              `xml:"projectUrl,omitempty"`
+		RequireLicenseAcceptance bool   `xml:"requireLicenseAcceptance"`
+		ProjectURL               string `xml:"projectUrl"`
 		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"`
+		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,27 +151,23 @@ 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
 		}
 	}
-	}
 
 	m := &Metadata{
 		Description:              p.Metadata.Description,
 		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 {
@@ -201,12 +183,12 @@ func ParseNuspecMetaData(r io.Reader) (*Package, error) {
 			m.Dependencies[group.TargetFramework] = deps
 		}
 	}
-	}
 	return &Package{
 		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,
-	}
-}

routers/api/packages/nuget/nuget.go

@@ -395,26 +395,6 @@ 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 {
-			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{
@@ -437,7 +417,6 @@ func DownloadPackageFile(ctx *context.Context) {
 	}
 
 	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)
 }
 

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
+}

tests/integration/api_packages_nuget_test.go

@@ -112,6 +112,20 @@ func TestPackageNuGet(t *testing.T) {
 		return &buf
 	}
 
+	nuspec := `<?xml version="1.0" encoding="utf-8"?>
+		<package xmlns="http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd">
+			<metadata>
+				<id>` + packageName + `</id>
+				<version>` + packageVersion + `</version>
+				<authors>` + packageAuthors + `</authors>
+				<description>` + packageDescription + `</description>
+				<dependencies>
+					<group targetFramework=".NETStandard2.0">
+						<dependency id="Microsoft.CSharp" version="4.5.0" />
+					</group>
+				</dependencies>
+			</metadata>
+		</package>`
 	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 := `<?xml version="1.0" encoding="UTF-8"?>` + "\n" +
-			`<package xmlns="http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd"><metadata>` +
-			`<id>` + packageName + `</id><version>` + packageVersion + `</version><authors>` + packageAuthors + `</authors><description>` + packageDescription + `</description>` +
-			`<dependencies><group targetFramework=".NETStandard2.0">` +
-			// https://github.com/golang/go/issues/21399 go can't generate self-closing tags
-			`<dependency id="Microsoft.CSharp" version="4.5.0"></dependency>` +
-			`</group></dependencies>` +
-			`</metadata></package>`
-
 		assert.Equal(t, nuspec, resp.Body.String())
 
 		checkDownloadCount(1)

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(`<?xml version="1.0" encoding="utf-8"?>
+		<package xmlns="http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd">
+			<metadata>
+				<id>` + id + `</id>
+				<version>` + version + `</version>
+				<authors>` + packageAuthors + `</authors>
+				<description>` + packageDescription + `</description>
+				<dependencies>
+					<group targetFramework=".NETStandard2.0">
+						<dependency id="Microsoft.CSharp" version="4.5.0" />
+					</group>
+				</dependencies>
+			</metadata>
+		</package>`))
+		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")
+	})
+}