Allow multiple files in generic packages (#20661)

* Allow multiple files in generic packages.
* Add deletion of a single file.
* Update docs.
* Change version check.

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

2 files changed, 68 insertions, 40 deletions

diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go
index 4b274860dc..84bdce30fa 100644
--- a/routers/api/packages/api.go
+++ b/routers/api/packages/api.go
@@ -156,12 +156,15 @@ func Routes() *web.Route {
 			})
 		})
 		r.Group("/generic", func() {
-			r.Group("/{packagename}/{packageversion}/{filename}", func() {
-				r.Get("", generic.DownloadPackageFile)
-				r.Group("", func() {
-					r.Put("", generic.UploadPackage)
-					r.Delete("", generic.DeletePackage)
-				}, reqPackageAccess(perm.AccessModeWrite))
+			r.Group("/{packagename}/{packageversion}", func() {
+				r.Delete("", reqPackageAccess(perm.AccessModeWrite), generic.DeletePackage)
+				r.Group("/{filename}", func() {
+					r.Get("", generic.DownloadPackageFile)
+					r.Group("", func() {
+						r.Put("", generic.UploadPackage)
+						r.Delete("", generic.DeletePackageFile)
+					}, reqPackageAccess(perm.AccessModeWrite))
+				})
 			})
 		})
 		r.Group("/helm", func() {
diff --git a/routers/api/packages/generic/generic.go b/routers/api/packages/generic/generic.go
index 9a3a185d9d..79e5afb03c 100644
--- a/routers/api/packages/generic/generic.go
+++ b/routers/api/packages/generic/generic.go
@@ -31,22 +31,16 @@ func apiError(ctx *context.Context, status int, obj interface{}) {
 
 // DownloadPackageFile serves the specific generic package.
 func DownloadPackageFile(ctx *context.Context) {
-	packageName, packageVersion, filename, err := sanitizeParameters(ctx)
-	if err != nil {
-		apiError(ctx, http.StatusBadRequest, err)
-		return
-	}
-
 	s, pf, err := packages_service.GetFileStreamByPackageNameAndVersion(
 		ctx,
 		&packages_service.PackageInfo{
 			Owner:       ctx.Package.Owner,
 			PackageType: packages_model.TypeGeneric,
-			Name:        packageName,
-			Version:     packageVersion,
+			Name:        ctx.Params("packagename"),
+			Version:     ctx.Params("packageversion"),
 		},
 		&packages_service.PackageFileInfo{
-			Filename: filename,
+			Filename: ctx.Params("filename"),
 		},
 	)
 	if err != nil {
@@ -65,9 +59,17 @@ func DownloadPackageFile(ctx *context.Context) {
 // UploadPackage uploads the specific generic package.
 // Duplicated packages get rejected.
 func UploadPackage(ctx *context.Context) {
-	packageName, packageVersion, filename, err := sanitizeParameters(ctx)
-	if err != nil {
-		apiError(ctx, http.StatusBadRequest, err)
+	packageName := ctx.Params("packagename")
+	filename := ctx.Params("filename")
+
+	if !packageNameRegex.MatchString(packageName) || !filenameRegex.MatchString(filename) {
+		apiError(ctx, http.StatusBadRequest, errors.New("Invalid package name or filename"))
+		return
+	}
+
+	packageVersion := ctx.Params("packageversion")
+	if packageVersion != strings.TrimSpace(packageVersion) {
+		apiError(ctx, http.StatusBadRequest, errors.New("Invalid package version"))
 		return
 	}
 
@@ -88,7 +90,7 @@ func UploadPackage(ctx *context.Context) {
 	}
 	defer buf.Close()
 
-	_, _, err = packages_service.CreatePackageAndAddFile(
+	_, _, err = packages_service.CreatePackageOrAddFileToExisting(
 		&packages_service.PackageCreationInfo{
 			PackageInfo: packages_service.PackageInfo{
 				Owner:       ctx.Package.Owner,
@@ -107,8 +109,8 @@ func UploadPackage(ctx *context.Context) {
 		},
 	)
 	if err != nil {
-		if err == packages_model.ErrDuplicatePackageVersion {
-			apiError(ctx, http.StatusBadRequest, err)
+		if err == packages_model.ErrDuplicatePackageFile {
+			apiError(ctx, http.StatusConflict, err)
 			return
 		}
 		apiError(ctx, http.StatusInternalServerError, err)
@@ -120,19 +122,13 @@ func UploadPackage(ctx *context.Context) {
 
 // DeletePackage deletes the specific generic package.
 func DeletePackage(ctx *context.Context) {
-	packageName, packageVersion, _, err := sanitizeParameters(ctx)
-	if err != nil {
-		apiError(ctx, http.StatusBadRequest, err)
-		return
-	}
-
-	err = packages_service.RemovePackageVersionByNameAndVersion(
+	err := packages_service.RemovePackageVersionByNameAndVersion(
 		ctx.Doer,
 		&packages_service.PackageInfo{
 			Owner:       ctx.Package.Owner,
 			PackageType: packages_model.TypeGeneric,
-			Name:        packageName,
-			Version:     packageVersion,
+			Name:        ctx.Params("packagename"),
+			Version:     ctx.Params("packageversion"),
 		},
 	)
 	if err != nil {
@@ -144,21 +140,50 @@ func DeletePackage(ctx *context.Context) {
 		return
 	}
 
-	ctx.Status(http.StatusOK)
+	ctx.Status(http.StatusNoContent)
 }
 
-func sanitizeParameters(ctx *context.Context) (string, string, string, error) {
-	packageName := ctx.Params("packagename")
-	filename := ctx.Params("filename")
+// DeletePackageFile deletes the specific file of a generic package.
+func DeletePackageFile(ctx *context.Context) {
+	pv, pf, err := func() (*packages_model.PackageVersion, *packages_model.PackageFile, error) {
+		pv, err := packages_model.GetVersionByNameAndVersion(ctx, ctx.Package.Owner.ID, packages_model.TypeGeneric, ctx.Params("packagename"), ctx.Params("packageversion"))
+		if err != nil {
+			return nil, nil, err
+		}
 
-	if !packageNameRegex.MatchString(packageName) || !filenameRegex.MatchString(filename) {
-		return "", "", "", errors.New("Invalid package name or filename")
+		pf, err := packages_model.GetFileForVersionByName(ctx, pv.ID, ctx.Params("filename"), packages_model.EmptyFileKey)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		return pv, pf, nil
+	}()
+	if err != nil {
+		if err == packages_model.ErrPackageNotExist || err == packages_model.ErrPackageFileNotExist {
+			apiError(ctx, http.StatusNotFound, err)
+			return
+		}
+		apiError(ctx, http.StatusInternalServerError, err)
+		return
 	}
 
-	packageVersion := strings.TrimSpace(ctx.Params("packageversion"))
-	if packageVersion == "" {
-		return "", "", "", errors.New("Invalid package version")
+	pfs, err := packages_model.GetFilesByVersionID(ctx, pv.ID)
+	if err != nil {
+		apiError(ctx, http.StatusInternalServerError, err)
+		return
+	}
+
+	if len(pfs) == 1 {
+		if err := packages_service.RemovePackageVersion(ctx.Doer, pv); err != nil {
+			apiError(ctx, http.StatusInternalServerError, err)
+			return
+		}
+	} else {
+		if err := packages_service.DeletePackageFile(ctx, pf); err != nil {
+			apiError(ctx, http.StatusInternalServerError, err)
+			return
+		}
 	}
 
-	return packageName, packageVersion, filename, nil
+	ctx.Status(http.StatusNoContent)
 }