Support nuspec manifest download for nuget packages

[viceice]

Support downloading nuget nuspec manifest¹. This is useful for renovate because it uses this api to find the corresponding repository

• Store nuspec along with nupkg on upload
• allow downloading nuspec
• add doctor command to add missing nuspec files

Footnotes

1. https://learn.microsoft.com/en-us/nuget/api/package-base-address-resource#download-package-manifest-nuspec ↩

[KN4CK3R]

I think it would be better if we would store the nuspec file as additional file in the package version instead of generating it. The original file could/will have additional data we don't need for the package registry and don't store in the metadata. So we can't re-generate the full nuspec file afterwards.

[viceice]

sure, but that's out of my go skills. sorry.

this implementation satisfies our needs for renovate bot.

[viceice]

new idea:

read nuspec from package and add to cache. so later request can leverage the cache.

cache can also be filled when package is uploaded.

WDYT?

[viceice]

we can also store it aside the package and serve directly if available. fallback to extract from package and put to cache in both cases.
this way we don't need a migration.

[viceice]

@KN4CK3R Can you please check this version?

[viceice]

Converting to draft, doctor is not working as expected.

[KN4CK3R]

sure, but that's out of my go skills. sorry.

this implementation satisfies our needs for renovate bot.

It's easier than what you have currently actually:

diff --git a/modules/packages/nuget/metadata.go b/modules/packages/nuget/metadata.go
index 6769c514cc..1e98ddffde 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
@@ -138,8 +139,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(archive *zip.Reader, 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
 	}
 
@@ -212,10 +214,11 @@ func ParseNuspecMetaData(archive *zip.Reader, r io.Reader) (*Package, error) {
 		}
 	}
 	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
 }

diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go
index c28bc6c9d9..34dcda4e2d 100644
--- a/routers/api/packages/nuget/nuget.go
+++ b/routers/api/packages/nuget/nuget.go
@@ -431,7 +431,7 @@ func UploadPackage(ctx *context.Context) {
 		return
 	}
 
-	_, _, err := packages_service.CreatePackageAndAddFile(
+	pv, _, err := packages_service.CreatePackageAndAddFile(
 		ctx,
 		&packages_service.PackageCreationInfo{
 			PackageInfo: packages_service.PackageInfo{
@@ -465,6 +465,35 @@ 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)),
+			},
+			Creator: ctx.Doer,
+			Data:    nuspecBuf,
+			IsLead:  false,
+		},
+	)
+	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)
 }

Nothing else is needed (+ tests). I have a branch currently with some NuGet changes for #30265. If you don't mind I could add this there too and add you as co-author. It doesn't add the nuspec file to existing packages with a migration but that may not be a big problem. I thought about a button in the package settings which would reinitialize (meta) data from the stored package (similar to what your doctor command would do) but that would be different PR.

[viceice]

I've done the complex part, because I don't want to store the full manifest in the database a second time.

most other nuget API implementations are storing the nuspec aside the nupkg.

feel free to do your own implementation instead of this.

[KN4CK3R]

It's not stored in the database but as file, same as your code.

I'm surprised the tests are passing because the hashed buffer is read twice (first nuspec parsing and when storing the file). The second read should already be at the end.

[viceice]

It's not stored in the database but as file, same as your code.

🙈 I should have read your diff more closely

I'm surprised the tests are passing because the hashed buffer is read twice (first nuspec parsing and when storing the file). The second read should already be at the end.

I've a seek to begin somewhere 🙃

[KN4CK3R]

I've a seek to begin somewhere 🙃

Searched and did not find it. But it's here in line 633
https://github.com/go-gitea/gitea/pull/28921/files#diff-efe687650466302ae2e35f382b2a66571e28065c6909e25893e515d5c9cc79f9R633

[viceice]

Updated PR with your suggestions and added a doctor integration test

[KN4CK3R]

Will send some additions for the tests tomorrow. Then we can merge it.

[wxiaoguang]

CI fails

[PulsarFX]

@KN4CK3R does this PR add support for reading repository data from metadata block, too?
https://learn.microsoft.com/en-us/nuget/reference/nuspec#repository

<?xml version="1.0"?>
<package xmlns="http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd">
    <metadata>
        ...
        <repository type="git" url="https://github.com/NuGet/NuGet.Client.git" branch="dev" commit="e1c65e4524cd70ee6e22abe33e6cb6ec73938cb3" />
        ...
    </metadata>
</package>

Or is repository data already available in 1.21 somehow?

[KN4CK3R]

It's only displayed on the package page but I think you talk about auto-linking a package to repository? Then no, that's not part of this PR.

[PulsarFX]

It's only displayed on the package page but I think you talk about auto-linking a package to repository? Then no, that's not part of this PR.

Thanks. I am looking for a way to read the <repository> tag from the nuspec to get the commit data from it. Then I'll try reading the nuspec, as stated in the initial post for this PR. This should work, right?

[KN4CK3R]

If you want to access the nuspec file from an external service, then yes, you can download that file from /api/packages/<user>/nuget/package/<name>/<version>/<name>.nuspec. But only for packages uploaded after this PR was merged if you use the nightly version of Gitea.

[PulsarFX]

Thanks! Works with 1.22.0-RC.1 for newly uploaded packages or also new versions of a package.