Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Project-URL metadata field to get a PyPI package's homepage URL #33089

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 41 additions & 4 deletions routers/api/packages/pypi/pypi.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"regexp"
"sort"
"strings"
"unicode"

packages_model "code.gitea.io/gitea/models/packages"
packages_module "code.gitea.io/gitea/modules/packages"
Expand Down Expand Up @@ -139,9 +140,28 @@ func UploadPackageFile(ctx *context.Context) {
return
}

projectURL := ctx.Req.FormValue("home_page")
if !validation.IsValidURL(projectURL) {
projectURL = ""
// Ensure ctx.Req.Form exists.
_ = ctx.Req.ParseForm()

// TODO: Home-page is a deprecated metadata field. Remove this form lookup once it's no longer apart of the spec.
homepageURL := ctx.Req.FormValue("home_page")
if len(homepageURL) == 0 {
projectURLs := ctx.Req.Form["project_urls"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM

I think here it should use project_urls before home_page

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for _, purl := range projectURLs {
label, url, found := strings.Cut(purl, ",")
if !found {
continue
}
if normalizeLabel(label) != "homepage" {
continue
}
homepageURL = strings.TrimSpace(url)
break
}
}

if !validation.IsValidURL(homepageURL) {
homepageURL = ""
}

_, _, err = packages_service.CreatePackageOrAddFileToExisting(
Expand All @@ -160,7 +180,7 @@ func UploadPackageFile(ctx *context.Context) {
Description: ctx.Req.FormValue("description"),
LongDescription: ctx.Req.FormValue("long_description"),
Summary: ctx.Req.FormValue("summary"),
ProjectURL: projectURL,
ProjectURL: homepageURL,
License: ctx.Req.FormValue("license"),
RequiresPython: ctx.Req.FormValue("requires_python"),
},
Expand Down Expand Up @@ -189,6 +209,23 @@ func UploadPackageFile(ctx *context.Context) {
ctx.Status(http.StatusCreated)
}

// Normalizes a Project-URL label.
// See https://packaging.python.org/en/latest/specifications/well-known-project-urls/#label-normalization.
func normalizeLabel(label string) string {
var builder strings.Builder

// "A label is normalized by deleting all ASCII punctuation and whitespace, and then converting the result
// to lowercase."
for _, r := range label {
if unicode.IsPunct(r) || unicode.IsSpace(r) {
continue
}
builder.WriteRune(unicode.ToLower(r))
}

return builder.String()
}

func isValidNameAndVersion(packageName, packageVersion string) bool {
return nameMatcher.MatchString(packageName) && versionMatcher.MatchString(packageVersion)
}
10 changes: 10 additions & 0 deletions routers/api/packages/pypi/pypi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,13 @@ func TestIsValidNameAndVersion(t *testing.T) {
assert.False(t, isValidNameAndVersion("test-name", "1.0.1aa"))
assert.False(t, isValidNameAndVersion("test-name", "1.0.0-alpha.beta"))
}

func TestNormalizeLabel(t *testing.T) {
// Cases fetched from https://packaging.python.org/en/latest/specifications/well-known-project-urls/#label-normalization.
assert.Equal(t, "homepage", normalizeLabel("Homepage"))
assert.Equal(t, "homepage", normalizeLabel("Home-page"))
assert.Equal(t, "homepage", normalizeLabel("Home page"))
assert.Equal(t, "changelog", normalizeLabel("Change_Log"))
assert.Equal(t, "whatsnew", normalizeLabel("What's New?"))
assert.Equal(t, "github", normalizeLabel("github"))
}
70 changes: 64 additions & 6 deletions tests/integration/api_packages_pypi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"github.com/stretchr/testify/assert"
)

func TestPackagePyPI(t *testing.T) {

Check failure on line 26 in tests/integration/api_packages_pypi_test.go

View workflow job for this annotation

GitHub Actions / lint-backend

empty-lines: extra empty line at the end of a block (revive)

Check failure on line 26 in tests/integration/api_packages_pypi_test.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

empty-lines: extra empty line at the end of a block (revive)

Check failure on line 26 in tests/integration/api_packages_pypi_test.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

empty-lines: extra empty line at the end of a block (revive)
defer tests.PrepareTestEnv(t)()

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
Expand All @@ -32,15 +32,16 @@
packageVersion := "1!1.0.1+r1234"
packageAuthor := "KN4CK3R"
packageDescription := "Test Description"
projectURL := "https://example.com"

content := "test"
hashSHA256 := "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08"

root := fmt.Sprintf("/api/packages/%s/pypi", user.Name)

uploadFile := func(t *testing.T, filename, content string, expectedStatus int) {
body := &bytes.Buffer{}
writer := multipart.NewWriter(body)
createBasicMultipartFile := func(packageName, filename, content string) (body *bytes.Buffer, writer *multipart.Writer) {
body = &bytes.Buffer{}
writer = multipart.NewWriter(body)
part, _ := writer.CreateFormFile("content", filename)
_, _ = io.Copy(part, strings.NewReader(content))

Expand All @@ -52,14 +53,27 @@
writer.WriteField("sha256_digest", hashSHA256)
writer.WriteField("requires_python", "3.6")

_ = writer.Close()
return

Check failure on line 56 in tests/integration/api_packages_pypi_test.go

View workflow job for this annotation

GitHub Actions / lint-backend

bare-return: avoid using bare returns, please add return expressions (revive)

Check failure on line 56 in tests/integration/api_packages_pypi_test.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

bare-return: avoid using bare returns, please add return expressions (revive)

Check failure on line 56 in tests/integration/api_packages_pypi_test.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

bare-return: avoid using bare returns, please add return expressions (revive)
}

upload := func(t *testing.T, body *bytes.Buffer, contentType string, expectedStatus int) {
req := NewRequestWithBody(t, "POST", root, body).
SetHeader("Content-Type", writer.FormDataContentType()).
SetHeader("Content-Type", contentType).
AddBasicAuth(user.Name)
MakeRequest(t, req, expectedStatus)
}

uploadFile := func(t *testing.T, filename, content string, expectedStatus int) {
body, writer := createBasicMultipartFile(packageName, filename, content)

writer.WriteField("project_urls", "DOCUMENTATION , https://readthedocs.org")
writer.WriteField("project_urls", fmt.Sprintf("Home-page, %s", projectURL))

_ = writer.Close()

upload(t, body, writer.FormDataContentType(), expectedStatus)
}

t.Run("Upload", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()

Expand All @@ -74,6 +88,7 @@
assert.NoError(t, err)
assert.Nil(t, pd.SemVer)
assert.IsType(t, &pypi.Metadata{}, pd.Metadata)
assert.Equal(t, projectURL, pd.Metadata.(*pypi.Metadata).ProjectURL)
assert.Equal(t, packageName, pd.Package.Name)
assert.Equal(t, packageVersion, pd.Version.Version)

Expand Down Expand Up @@ -133,6 +148,48 @@
uploadFile(t, "test.tar.gz", content, http.StatusConflict)
})

t.Run("UploadUsingDeprecatedHomepageMetadata", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()

pkgName := "homepage-package"
body, writer := createBasicMultipartFile(pkgName, "test.whl", content)

writer.WriteField("home_page", projectURL)

_ = writer.Close()

upload(t, body, writer.FormDataContentType(), http.StatusCreated)

pvs, err := packages.GetVersionsByPackageName(db.DefaultContext, user.ID, packages.TypePyPI, pkgName)
assert.NoError(t, err)
assert.Len(t, pvs, 1)

pd, err := packages.GetPackageDescriptor(db.DefaultContext, pvs[0])
assert.NoError(t, err)
assert.IsType(t, &pypi.Metadata{}, pd.Metadata)
assert.Equal(t, projectURL, pd.Metadata.(*pypi.Metadata).ProjectURL)
})

t.Run("UploadWithoutAnyHomepageURLMetadata", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()

pkgName := "no-project-url-or-homepage-package"
body, writer := createBasicMultipartFile(pkgName, "test.whl", content)

_ = writer.Close()

upload(t, body, writer.FormDataContentType(), http.StatusCreated)

pvs, err := packages.GetVersionsByPackageName(db.DefaultContext, user.ID, packages.TypePyPI, pkgName)
assert.NoError(t, err)
assert.Len(t, pvs, 1)

pd, err := packages.GetPackageDescriptor(db.DefaultContext, pvs[0])
assert.NoError(t, err)
assert.IsType(t, &pypi.Metadata{}, pd.Metadata)
assert.Empty(t, pd.Metadata.(*pypi.Metadata).ProjectURL)
})

t.Run("Download", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()

Expand All @@ -147,7 +204,7 @@
downloadFile("test.whl")
downloadFile("test.tar.gz")

pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypePyPI)
pvs, err := packages.GetVersionsByPackageName(db.DefaultContext, user.ID, packages.TypePyPI, packageName)
assert.NoError(t, err)
assert.Len(t, pvs, 1)
assert.Equal(t, int64(2), pvs[0].DownloadCount)
Expand Down Expand Up @@ -179,4 +236,5 @@
}
}
})

}
Loading