refactor: npm route handlers (#39275)

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.

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.Group("/{scope:"+scopeRegexp+"}/{id:"+idRegexp+"}", addPackageHandlers)
-				r.Get("", npm.PackageMetadata)
+			r.Group("/{id:"+idRegexp+"}", addPackageHandlers)
-				r.Put("", reqPackageAccess(perm.AccessModeWrite), npm.UploadPackage)
+
-				r.Group("/-/{version}/{filename}", func() {
+			addPackageDistTagsHandlers := 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.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.Group("/-/package/{scope:"+scopeRegexp+"}/{id:"+idRegexp+"}/dist-tags", addPackageDistTagsHandlers)
-				r.Get("", npm.ListPackageTags)
+			r.Group("/-/package/{id:"+idRegexp+"}/dist-tags", addPackageDistTagsHandlers)
-				r.Group("/{tag}", func() {
-					r.Put("", npm.AddPackageTag)
-					r.Delete("", npm.DeletePackageTag)
-				}, reqPackageAccess(perm.AccessModeWrite))
-			})
 			r.Group("/-/v1/search", func() {
 				r.Get("", npm.PackageSearch)
 			})

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)

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)).
+		rootPaths := []string{
-			AddTokenAuth(token)
+			fmt.Sprintf("/api/packages/%s/npm/@scope/test-package", user.Name),
-		resp := MakeRequest(t, req, http.StatusOK)
+			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)
+			req = NewRequest(t, "GET", fmt.Sprintf("%s/-/%s", root, filename)).AddTokenAuth(token)
-		assert.Equal(t, b, resp.Body.Bytes())
+			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) {