Swift registry metadata: preserve more JSON fields and accept empty metadata (#37254)

models/packages/package_property.go

@@ -52,13 +52,13 @@ func InsertProperty(ctx context.Context, refType PropertyType, refID int64, name
 // GetProperties gets all properties
 func GetProperties(ctx context.Context, refType PropertyType, refID int64) ([]*PackageProperty, error) {
 	pps := make([]*PackageProperty, 0, 10)
-	return pps, db.GetEngine(ctx).Where("ref_type = ? AND ref_id = ?", refType, refID).Find(&pps)
+	return pps, db.GetEngine(ctx).Where("ref_type = ? AND ref_id = ?", refType, refID).OrderBy("id").Find(&pps)
 }
 
 // GetPropertiesByName gets all properties with a specific name
 func GetPropertiesByName(ctx context.Context, refType PropertyType, refID int64, name string) ([]*PackageProperty, error) {
 	pps := make([]*PackageProperty, 0, 10)
-	return pps, db.GetEngine(ctx).Where("ref_type = ? AND ref_id = ? AND name = ?", refType, refID, name).Find(&pps)
+	return pps, db.GetEngine(ctx).Where("ref_type = ? AND ref_id = ? AND name = ?", refType, refID, name).OrderBy("id").Find(&pps)
 }
 
 // UpdateProperty updates a property

modules/packages/swift/metadata.go

@@ -47,6 +47,7 @@ type Metadata struct {
 	Keywords      []string             `json:"keywords,omitempty"`
 	RepositoryURL string               `json:"repository_url,omitempty"`
 	License       string               `json:"license,omitempty"`
+	LicenseURL    string               `json:"license_url,omitempty"`
 	Author        Person               `json:"author"`
 	Manifests     map[string]*Manifest `json:"manifests,omitempty"`
 }
@@ -67,7 +68,8 @@ type SoftwareSourceCode struct {
 	Keywords            []string            `json:"keywords,omitempty"`
 	CodeRepository      string              `json:"codeRepository,omitempty"`
 	License             string              `json:"license,omitempty"`
-	Author              Person              `json:"author"`
+	LicenseURL          string              `json:"licenseURL,omitempty"`
+	Author              *Person             `json:"author,omitempty"`
 	ProgrammingLanguage ProgrammingLanguage `json:"programmingLanguage"`
 	RepositoryURLs      []string            `json:"repositoryURLs,omitempty"`
 }
@@ -181,26 +183,31 @@ func ParsePackage(sr io.ReaderAt, size int64, mr io.Reader) (*Package, error) {
 		if err := json.NewDecoder(mr).Decode(&ssc); err != nil {
 			return nil, err
 		}
-
 		p.Metadata.Description = ssc.Description
 		p.Metadata.Keywords = ssc.Keywords
 		p.Metadata.License = ssc.License
-		author := Person{
+		p.Metadata.LicenseURL = ssc.LicenseURL
-			Name:       ssc.Author.Name,
+		if ssc.Author != nil {
-			GivenName:  ssc.Author.GivenName,
+			author := Person{
-			MiddleName: ssc.Author.MiddleName,
+				Name:       ssc.Author.Name,
-			FamilyName: ssc.Author.FamilyName,
+				GivenName:  ssc.Author.GivenName,
+				MiddleName: ssc.Author.MiddleName,
+				FamilyName: ssc.Author.FamilyName,
+			}
+			// If Name is not provided, generate it from individual name components
+			if author.Name == "" {
+				author.Name = author.String()
+			}
+			p.Metadata.Author = author
 		}
-		// If Name is not provided, generate it from individual name components
-		if author.Name == "" {
-			author.Name = author.String()
-		}
-		p.Metadata.Author = author
 
 		p.Metadata.RepositoryURL = ssc.CodeRepository
 		if !validation.IsValidURL(p.Metadata.RepositoryURL) {
 			p.Metadata.RepositoryURL = ""
 		}
+		if !validation.IsValidURL(p.Metadata.LicenseURL) {
+			p.Metadata.LicenseURL = ""
+		}
 
 		p.RepositoryURLs = ssc.RepositoryURLs
 	}

modules/packages/swift/metadata_test.go

@@ -4,11 +4,12 @@
 package swift
 
 import (
-	"archive/zip"
 	"bytes"
 	"strings"
 	"testing"
 
+	"code.gitea.io/gitea/modules/test"
+
 	"github.com/hashicorp/go-version"
 	"github.com/stretchr/testify/assert"
 )
@@ -18,36 +19,24 @@ const (
 	packageVersion       = "1.0.1"
 	packageDescription   = "Package Description"
 	packageRepositoryURL = "https://gitea.io/gitea/gitea"
+	packageLicenseURL    = "https://opensource.org/license/mit"
 	packageAuthor        = "KN4CK3R"
 	packageLicense       = "MIT"
 )
 
 func TestParsePackage(t *testing.T) {
-	createArchive := func(files map[string][]byte) *bytes.Reader {
-		var buf bytes.Buffer
-		zw := zip.NewWriter(&buf)
-		for filename, content := range files {
-			w, _ := zw.Create(filename)
-			w.Write(content)
-		}
-		zw.Close()
-		return bytes.NewReader(buf.Bytes())
-	}
-
 	t.Run("MissingManifestFile", func(t *testing.T) {
-		data := createArchive(map[string][]byte{"dummy.txt": {}})
+		data := test.WriteZipArchive(map[string]string{"dummy.txt": ""})
-
+		p, err := ParsePackage(bytes.NewReader(data.Bytes()), int64(data.Len()), nil)
-		p, err := ParsePackage(data, data.Size(), nil)
 		assert.Nil(t, p)
 		assert.ErrorIs(t, err, ErrMissingManifestFile)
 	})
 
 	t.Run("ManifestFileTooLarge", func(t *testing.T) {
-		data := createArchive(map[string][]byte{
+		data := test.WriteZipArchive(map[string]string{
-			"Package.swift": make([]byte, maxManifestFileSize+1),
+			"Package.swift": strings.Repeat("a", maxManifestFileSize+1),
 		})
-
+		p, err := ParsePackage(bytes.NewReader(data.Bytes()), int64(data.Len()), nil)
-		p, err := ParsePackage(data, data.Size(), nil)
 		assert.Nil(t, p)
 		assert.ErrorIs(t, err, ErrManifestFileTooLarge)
 	})
@@ -56,12 +45,12 @@ func TestParsePackage(t *testing.T) {
 		content1 := "// swift-tools-version:5.7\n//\n//  Package.swift"
 		content2 := "// swift-tools-version:5.6\n//\n//  Package@swift-5.6.swift"
 
-		data := createArchive(map[string][]byte{
+		data := test.WriteZipArchive(map[string]string{
-			"Package.swift":           []byte(content1),
+			"Package.swift":           content1,
-			"Package@swift-5.5.swift": []byte(content2),
+			"Package@swift-5.5.swift": content2,
 		})
 
-		p, err := ParsePackage(data, data.Size(), nil)
+		p, err := ParsePackage(bytes.NewReader(data.Bytes()), int64(data.Len()), nil)
 		assert.NotNil(t, p)
 		assert.NoError(t, err)
 
@@ -77,14 +66,13 @@ func TestParsePackage(t *testing.T) {
 	})
 
 	t.Run("WithMetadata", func(t *testing.T) {
-		data := createArchive(map[string][]byte{
+		data := test.WriteZipArchive(map[string]string{
-			"Package.swift": []byte("// swift-tools-version:5.7\n//\n//  Package.swift"),
+			"Package.swift": "// swift-tools-version:5.7\n//\n//  Package.swift",
 		})
 
 		p, err := ParsePackage(
-			data,
+			bytes.NewReader(data.Bytes()), int64(data.Len()),
-			data.Size(),
+			strings.NewReader(`{"name":"`+packageName+`","version":"`+packageVersion+`","description":"`+packageDescription+`","keywords":["swift","package"],"license":"`+packageLicense+`","licenseURL":"`+packageLicenseURL+`","codeRepository":"`+packageRepositoryURL+`","author":{"givenName":"`+packageAuthor+`"},"repositoryURLs":["`+packageRepositoryURL+`"]}`),
-			strings.NewReader(`{"name":"`+packageName+`","version":"`+packageVersion+`","description":"`+packageDescription+`","keywords":["swift","package"],"license":"`+packageLicense+`","codeRepository":"`+packageRepositoryURL+`","author":{"givenName":"`+packageAuthor+`"},"repositoryURLs":["`+packageRepositoryURL+`"]}`),
 		)
 		assert.NotNil(t, p)
 		assert.NoError(t, err)
@@ -97,6 +85,7 @@ func TestParsePackage(t *testing.T) {
 		assert.Equal(t, packageDescription, p.Metadata.Description)
 		assert.ElementsMatch(t, []string{"swift", "package"}, p.Metadata.Keywords)
 		assert.Equal(t, packageLicense, p.Metadata.License)
+		assert.Equal(t, packageLicenseURL, p.Metadata.LicenseURL)
 		assert.Equal(t, packageAuthor, p.Metadata.Author.Name)
 		assert.Equal(t, packageAuthor, p.Metadata.Author.GivenName)
 		assert.Equal(t, packageRepositoryURL, p.Metadata.RepositoryURL)
@@ -104,14 +93,13 @@ func TestParsePackage(t *testing.T) {
 	})
 
 	t.Run("WithExplicitNameField", func(t *testing.T) {
-		data := createArchive(map[string][]byte{
+		data := test.WriteZipArchive(map[string]string{
-			"Package.swift": []byte("// swift-tools-version:5.7\n//\n//  Package.swift"),
+			"Package.swift": "// swift-tools-version:5.7\n//\n//  Package.swift",
 		})
 
 		authorName := "John Doe"
 		p, err := ParsePackage(
-			data,
+			bytes.NewReader(data.Bytes()), int64(data.Len()),
-			data.Size(),
 			strings.NewReader(`{"name":"`+packageName+`","version":"`+packageVersion+`","description":"`+packageDescription+`","author":{"name":"`+authorName+`","givenName":"John","familyName":"Doe"}}`),
 		)
 		assert.NotNil(t, p)
@@ -122,15 +110,30 @@ func TestParsePackage(t *testing.T) {
 		assert.Equal(t, "Doe", p.Metadata.Author.FamilyName)
 	})
 
+	t.Run("WithEmptyJSONMetadata", func(t *testing.T) {
+		data := test.WriteZipArchive(map[string]string{
+			"Package.swift": "// swift-tools-version:5.7\n//\n//  Package.swift",
+		})
+
+		p, err := ParsePackage(
+			bytes.NewReader(data.Bytes()), int64(data.Len()),
+			strings.NewReader(`{}`),
+		)
+		assert.NotNil(t, p)
+		assert.NoError(t, err)
+		assert.NotNil(t, p.Metadata)
+		assert.Empty(t, p.Metadata.Author.Name)
+		assert.Empty(t, p.RepositoryURLs)
+	})
+
 	t.Run("NameFieldGeneration", func(t *testing.T) {
-		data := createArchive(map[string][]byte{
+		data := test.WriteZipArchive(map[string]string{
-			"Package.swift": []byte("// swift-tools-version:5.7\n//\n//  Package.swift"),
+			"Package.swift": "// swift-tools-version:5.7\n//\n//  Package.swift",
 		})
 
 		// Test with only individual name components - Name should be auto-generated
 		p, err := ParsePackage(
-			data,
+			bytes.NewReader(data.Bytes()), int64(data.Len()),
-			data.Size(),
 			strings.NewReader(`{"author":{"givenName":"John","middleName":"Q","familyName":"Doe"}}`),
 		)
 		assert.NotNil(t, p)

routers/api/packages/swift/swift.go

@@ -198,6 +198,23 @@ func PackageVersionMetadata(ctx *context.Context) {
 	}
 
 	metadata := pd.Metadata.(*swift_module.Metadata)
+	repositoryURLs := make([]string, 0, len(pd.VersionProperties))
+	for _, property := range pd.VersionProperties {
+		if property.Name == swift_module.PropertyRepositoryURL {
+			repositoryURLs = append(repositoryURLs, property.Value)
+		}
+	}
+
+	var author *swift_module.Person
+	if metadata.Author.Name != "" || metadata.Author.GivenName != "" || metadata.Author.MiddleName != "" || metadata.Author.FamilyName != "" {
+		author = &swift_module.Person{
+			Type:       "Person",
+			Name:       metadata.Author.Name,
+			GivenName:  metadata.Author.GivenName,
+			MiddleName: metadata.Author.MiddleName,
+			FamilyName: metadata.Author.FamilyName,
+		}
+	}
 
 	setResponseHeaders(ctx.Resp, &headers{})
 
@@ -220,18 +237,14 @@ func PackageVersionMetadata(ctx *context.Context) {
 			Keywords:       metadata.Keywords,
 			CodeRepository: metadata.RepositoryURL,
 			License:        metadata.License,
+			LicenseURL:     metadata.LicenseURL,
+			Author:         author,
 			ProgrammingLanguage: swift_module.ProgrammingLanguage{
 				Type: "ComputerLanguage",
 				Name: "Swift",
 				URL:  "https://swift.org",
 			},
-			Author: swift_module.Person{
+			RepositoryURLs: repositoryURLs,
-				Type:       "Person",
-				Name:       metadata.Author.String(),
-				GivenName:  metadata.Author.GivenName,
-				MiddleName: metadata.Author.MiddleName,
-				FamilyName: metadata.Author.FamilyName,
-			},
 		},
 	})
 }

tests/integration/api_packages_swift_test.go

@@ -35,9 +35,26 @@ func TestPackageSwift(t *testing.T) {
 	packageID := packageScope + "." + packageName
 	packageVersion := "1.0.3"
 	packageVersion2 := "1.0.4"
+	packageVersion3 := "1.0.5"
 	packageAuthor := "KN4CK3R"
 	packageDescription := "Gitea Test Package"
-	packageRepositoryURL := "https://gitea.io/gitea/gitea"
+	packageCodeRepositoryURL := "https://gitea.io/gitea/gitea" // this one is not used as a property, it is meta
+	packageLicenseURL := "https://opensource.org/license/mit"
+	packageRepositoryURL1 := "https://gitea.io/gitea/repo"
+	packageRepositoryURLs := []string{packageRepositoryURL1, "https://gitea.io/gitea/repo.git", "ssh://git@gitea.io/gitea/repo.git"}
+	makePackageMetadataJSON := func(ver string) string {
+		tmpl := `{
+	"name":"` + packageName + `",
+	"version":"%s",
+	"description":"` + packageDescription + `",
+	"codeRepository":"` + packageCodeRepositoryURL + `",
+	"licenseURL":"` + packageLicenseURL + `",
+	"author":{"givenName":"` + packageAuthor + `"},
+	"repositoryURLs":["` + strings.Join(packageRepositoryURLs, `","`) + `"]
+}`
+		return fmt.Sprintf(tmpl, ver)
+	}
+
 	contentManifest1 := "// swift-tools-version:5.7\n//\n//  Package.swift"
 	contentManifest2 := "// swift-tools-version:5.6\n//\n//  Package@swift-5.6.swift"
 
@@ -135,7 +152,7 @@ func TestPackageSwift(t *testing.T) {
 				"Package.swift":           contentManifest1,
 				"Package@swift-5.6.swift": contentManifest2,
 			}),
-			`{"name":"`+packageName+`","version":"`+packageVersion+`","description":"`+packageDescription+`","codeRepository":"`+packageRepositoryURL+`","author":{"givenName":"`+packageAuthor+`"},"repositoryURLs":["`+packageRepositoryURL+`"]}`,
+			makePackageMetadataJSON(packageVersion),
 		)
 
 		pvs, err := packages.GetVersionsByPackageType(t.Context(), user.ID, packages.TypeSwift)
@@ -153,8 +170,8 @@ func TestPackageSwift(t *testing.T) {
 		assert.Len(t, metadata.Manifests, 2)
 		assert.Equal(t, contentManifest1, metadata.Manifests[""].Content)
 		assert.Equal(t, contentManifest2, metadata.Manifests["5.6"].Content)
-		assert.Len(t, pd.VersionProperties, 1)
+		assert.Len(t, pd.VersionProperties, 3)
-		assert.Equal(t, packageRepositoryURL, pd.VersionProperties.GetByName(swift_module.PropertyRepositoryURL))
+		assert.Equal(t, packageRepositoryURL1, pd.VersionProperties.GetByName(swift_module.PropertyRepositoryURL))
 
 		pfs, err := packages.GetFilesByVersionID(t.Context(), pvs[0].ID)
 		assert.NoError(t, err)
@@ -212,7 +229,7 @@ func TestPackageSwift(t *testing.T) {
 				"Package.swift":           contentManifest1,
 				"Package@swift-5.6.swift": contentManifest2,
 			}),
-			`{"name":"`+packageName+`","version":"`+packageVersion2+`","description":"`+packageDescription+`","codeRepository":"`+packageRepositoryURL+`","author":{"givenName":"`+packageAuthor+`"},"repositoryURLs":["`+packageRepositoryURL+`"]}`,
+			makePackageMetadataJSON(packageVersion2),
 		)
 
 		pvs, err := packages.GetVersionsByPackageType(t.Context(), user.ID, packages.TypeSwift)
@@ -230,8 +247,8 @@ func TestPackageSwift(t *testing.T) {
 		assert.Len(t, metadata.Manifests, 2)
 		assert.Equal(t, contentManifest1, metadata.Manifests[""].Content)
 		assert.Equal(t, contentManifest2, metadata.Manifests["5.6"].Content)
-		assert.Len(t, pd.VersionProperties, 1)
+		assert.Len(t, pd.VersionProperties, 3)
-		assert.Equal(t, packageRepositoryURL, pd.VersionProperties.GetByName(swift_module.PropertyRepositoryURL))
+		assert.Equal(t, packageRepositoryURL1, pd.VersionProperties.GetByName(swift_module.PropertyRepositoryURL))
 
 		pfs, err := packages.GetFilesByVersionID(t.Context(), thisPackageVersion.ID)
 		assert.NoError(t, err)
@@ -330,8 +347,11 @@ func TestPackageSwift(t *testing.T) {
 		assert.Equal(t, packageVersion, result.Metadata.Version)
 		assert.Equal(t, packageDescription, result.Metadata.Description)
 		assert.Equal(t, "Swift", result.Metadata.ProgrammingLanguage.Name)
+		assert.Equal(t, packageLicenseURL, result.Metadata.LicenseURL)
+		require.NotNil(t, result.Metadata.Author)
 		assert.Equal(t, packageAuthor, result.Metadata.Author.Name)
 		assert.Equal(t, packageAuthor, result.Metadata.Author.GivenName)
+		assert.ElementsMatch(t, packageRepositoryURLs, result.Metadata.RepositoryURLs)
 
 		req = NewRequest(t, "GET", fmt.Sprintf("%s/%s/%s/%s.json", url, packageScope, packageName, packageVersion)).
 			AddBasicAuth(user.Name)
@@ -340,6 +360,41 @@ func TestPackageSwift(t *testing.T) {
 		assert.Equal(t, body, resp.Body.String())
 	})
 
+	t.Run("UploadEmptyJSONMetadata", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		uploadURL := fmt.Sprintf("%s/%s/%s/%s", url, packageScope, packageName, packageVersion3)
+		var body bytes.Buffer
+		mpw := multipart.NewWriter(&body)
+
+		part, err := mpw.CreateFormFile("source-archive", "source-archive.zip")
+		require.NoError(t, err)
+		_, err = io.Copy(part, test.WriteZipArchive(map[string]string{
+			"Package.swift":           contentManifest1,
+			"Package@swift-5.6.swift": contentManifest2,
+		}))
+		require.NoError(t, err)
+		require.NoError(t, mpw.WriteField("metadata", "{}"))
+		require.NoError(t, mpw.Close())
+
+		req := NewRequestWithBody(t, "PUT", uploadURL, &body).
+			SetHeader("Content-Type", mpw.FormDataContentType()).
+			SetHeader("Accept", swift_router.AcceptJSON).
+			AddBasicAuth(user.Name)
+		MakeRequest(t, req, http.StatusCreated)
+
+		req = NewRequest(t, "GET", fmt.Sprintf("%s/%s/%s/%s", url, packageScope, packageName, packageVersion3)).
+			AddBasicAuth(user.Name).
+			SetHeader("Accept", swift_router.AcceptJSON)
+		resp := MakeRequest(t, req, http.StatusOK)
+		result := DecodeJSON(t, resp, &swift_router.PackageVersionMetadataResponse{})
+
+		assert.Nil(t, result.Metadata.Author)
+		assert.Empty(t, result.Metadata.RepositoryURLs)
+		assert.Empty(t, result.Metadata.CodeRepository)
+		assert.Empty(t, result.Metadata.LicenseURL)
+	})
+
 	t.Run("DownloadManifest", func(t *testing.T) {
 		manifestURL := fmt.Sprintf("%s/%s/%s/%s/Package.swift", url, packageScope, packageName, packageVersion)
 
@@ -397,7 +452,7 @@ func TestPackageSwift(t *testing.T) {
 		req = NewRequest(t, "GET", url+"/identifiers?url=https://unknown.host/")
 		MakeRequest(t, req, http.StatusNotFound)
 
-		req = NewRequest(t, "GET", url+"/identifiers?url="+packageRepositoryURL).
+		req = NewRequest(t, "GET", url+"/identifiers?url="+packageRepositoryURL1).
 			SetHeader("Accept", swift_router.AcceptJSON)
 		resp = MakeRequest(t, req, http.StatusOK)