From 45a78bbc8efb88ef3e52cbf683a6aa736715bc82 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 8 Sep 2026 23:15:40 +0800 Subject: [PATCH] refactor: npm route handlers (#39275) --- modules/packages/npm/creator.go | 10 ++++-- routers/api/packages/api.go | 38 ++++++++-------------- routers/api/packages/npm/npm.go | 20 +++++++++--- tests/integration/api_packages_npm_test.go | 28 +++++++++------- 4 files changed, 54 insertions(+), 42 deletions(-) diff --git a/modules/packages/npm/creator.go b/modules/packages/npm/creator.go index e0a6bde74d2..2425e2f5ef2 100644 --- a/modules/packages/npm/creator.go +++ b/modules/packages/npm/creator.go @@ -14,6 +14,7 @@ import ( "io" "regexp" "strings" + "sync" "time" "gitea.dev/modules/json" @@ -36,7 +37,12 @@ var ( ErrInvalidIntegrity = util.NewInvalidArgumentErrorf("failed to validate integrity") ) -var nameMatch = regexp.MustCompile(`^(@[a-z0-9-][a-z0-9-._]*/)?[a-z0-9-][a-z0-9-._]*$`) +const RegexpNamePart = `[a-z0-9-][a-z0-9-._]*` + +var nameMatch = sync.OnceValue(func() *regexp.Regexp { + // either "@scope/pkg" or "pkg", where "scope" and "pkg" are RegexpNamePart + return regexp.MustCompile(`^(@` + RegexpNamePart + `/)?` + RegexpNamePart + `$`) +}) // Package represents a npm package type Package struct { @@ -463,7 +469,7 @@ func validateName(name string) bool { if len(name) == 0 || len(name) > 214 { return false } - return nameMatch.MatchString(name) + return nameMatch().MatchString(name) } // PackageDeprecation is the result of parsing an npm deprecate request body. diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index b4da77a6c46..3fe70135b29 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -9,6 +9,7 @@ import ( auth_model "gitea.dev/models/auth" "gitea.dev/models/perm" "gitea.dev/modules/log" + npm_module "gitea.dev/modules/packages/npm" "gitea.dev/modules/setting" "gitea.dev/modules/web" "gitea.dev/routers/api/packages/alpine" @@ -404,9 +405,13 @@ func CommonRoutes() *web.Router { }, reqPackageAccess(perm.AccessModeRead)) }) r.Group("/npm", func() { - r.Group("/@{scope}/{id}", func() { + // HINT: NPM-ROUTE-PATH-PATTERN: search this keyword to see more details + scopeRegexp := `^@` + npm_module.RegexpNamePart + `$` + idRegexp := `^(@` + npm_module.RegexpNamePart + `%2[fF])?` + npm_module.RegexpNamePart + `$` + addPackageHandlers := func() { r.Get("", npm.PackageMetadata) r.Put("", reqPackageAccess(perm.AccessModeWrite), npm.UploadPackage) + r.Get("/{version}", npm.PackageVersionMetadata) r.Group("/-/{version}/{filename}", func() { r.Get("", npm.DownloadPackageFile) r.Delete("/-rev/{revision}", reqPackageAccess(perm.AccessModeWrite), npm.DeletePackageVersion) @@ -416,34 +421,19 @@ func CommonRoutes() *web.Router { r.Delete("", npm.DeletePackage) r.Put("", npm.DeletePreview) }, reqPackageAccess(perm.AccessModeWrite)) - }) - r.Group("/{id}", func() { - r.Get("", npm.PackageMetadata) - r.Put("", reqPackageAccess(perm.AccessModeWrite), npm.UploadPackage) - r.Group("/-/{version}/{filename}", func() { - r.Get("", npm.DownloadPackageFile) - r.Delete("/-rev/{revision}", reqPackageAccess(perm.AccessModeWrite), npm.DeletePackageVersion) - }) - r.Get("/-/{filename}", npm.DownloadPackageFileByName) - r.Group("/-rev/{revision}", func() { - r.Delete("", npm.DeletePackage) - r.Put("", npm.DeletePreview) - }, reqPackageAccess(perm.AccessModeWrite)) - }) - r.Group("/-/package/@{scope}/{id}/dist-tags", func() { + } + r.Group("/{scope:"+scopeRegexp+"}/{id:"+idRegexp+"}", addPackageHandlers) + r.Group("/{id:"+idRegexp+"}", addPackageHandlers) + + addPackageDistTagsHandlers := func() { r.Get("", npm.ListPackageTags) r.Group("/{tag}", func() { r.Put("", npm.AddPackageTag) r.Delete("", npm.DeletePackageTag) }, reqPackageAccess(perm.AccessModeWrite)) - }) - r.Group("/-/package/{id}/dist-tags", func() { - r.Get("", npm.ListPackageTags) - r.Group("/{tag}", func() { - r.Put("", npm.AddPackageTag) - r.Delete("", npm.DeletePackageTag) - }, reqPackageAccess(perm.AccessModeWrite)) - }) + } + r.Group("/-/package/{scope:"+scopeRegexp+"}/{id:"+idRegexp+"}/dist-tags", addPackageDistTagsHandlers) + r.Group("/-/package/{id:"+idRegexp+"}/dist-tags", addPackageDistTagsHandlers) r.Group("/-/v1/search", func() { r.Get("", npm.PackageSearch) }) diff --git a/routers/api/packages/npm/npm.go b/routers/api/packages/npm/npm.go index bae2b2d355a..980bb085763 100644 --- a/routers/api/packages/npm/npm.go +++ b/routers/api/packages/npm/npm.go @@ -41,14 +41,22 @@ func apiError(ctx *context.Context, status int, obj any) { } // packageNameFromParams gets the package name from the url parameters -// Variations: /name/, /@scope/name/, /@scope%2Fname/ func packageNameFromParams(ctx *context.Context) string { + // Real examples: these 2 both should work: + // * "https://registry.npmjs.org/@angular/core" + // * "https://registry.npmjs.org/@angular%2Fcore" + // + // HINT: NPM-ROUTE-PATH-PATTERN: The cases for the path parameters: + // * ".../TheName/...": id="TheName" + // * ".../@TheScope/TheName/...": scope="@TheScope", id="TheName" + // * ".../@TheScope%2FTheName/...": id="@TheScope/TheName" scope := ctx.PathParam("scope") - id := ctx.PathParam("id") + fullOrSub := ctx.PathParam("id") // may be a full name or a subpath of the full package name if scope != "" { - return fmt.Sprintf("@%s/%s", scope, id) + // now id is the subpath of the full package name, e.g. "core" in "@angular/core" + return fmt.Sprintf("%s/%s", scope, fullOrSub) } - return id + return fullOrSub // id is the full package name, e.g.: "@angular/core" or "lodash" } // PackageMetadata returns the metadata for a single package @@ -79,6 +87,10 @@ func PackageMetadata(ctx *context.Context) { ctx.JSON(http.StatusOK, resp) } +func PackageVersionMetadata(ctx *context.Context) { + ctx.HTTPError(http.StatusNotImplemented, "not implemented") +} + // DownloadPackageFile serves the content of a package func DownloadPackageFile(ctx *context.Context) { packageName := packageNameFromParams(ctx) diff --git a/tests/integration/api_packages_npm_test.go b/tests/integration/api_packages_npm_test.go index 55a3ea70724..8928d448e85 100644 --- a/tests/integration/api_packages_npm_test.go +++ b/tests/integration/api_packages_npm_test.go @@ -169,23 +169,27 @@ func TestPackageNpm(t *testing.T) { t.Run("Download", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - req := NewRequest(t, "GET", fmt.Sprintf("%s/-/%s/%s", root, packageVersion, filename)). - AddTokenAuth(token) - resp := MakeRequest(t, req, http.StatusOK) + rootPaths := []string{ + fmt.Sprintf("/api/packages/%s/npm/@scope/test-package", user.Name), + fmt.Sprintf("/api/packages/%s/npm/@scope%%2ftest-package", user.Name), + } + for _, root := range rootPaths { + req := NewRequest(t, "GET", fmt.Sprintf("%s/-/%s/%s", root, packageVersion, filename)).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + b, _ := base64.StdEncoding.DecodeString(attachmentData) + assert.Equal(t, b, resp.Body.Bytes()) - b, _ := base64.StdEncoding.DecodeString(attachmentData) - assert.Equal(t, b, resp.Body.Bytes()) - - req = NewRequest(t, "GET", fmt.Sprintf("%s/-/%s", root, filename)). - AddTokenAuth(token) - resp = MakeRequest(t, req, http.StatusOK) - - assert.Equal(t, b, resp.Body.Bytes()) + req = NewRequest(t, "GET", fmt.Sprintf("%s/-/%s", root, filename)).AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusOK) + assert.Equal(t, b, resp.Body.Bytes()) + req = NewRequest(t, "GET", fmt.Sprintf("%s/%s", root, packageVersion)).AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotImplemented) + } pvs, err := packages.GetVersionsByPackageType(t.Context(), user.ID, packages.TypeNpm) assert.NoError(t, err) assert.Len(t, pvs, 1) - assert.Equal(t, int64(2), pvs[0].DownloadCount) + assert.Equal(t, int64(4), pvs[0].DownloadCount) }) t.Run("PackageMetadata", func(t *testing.T) {