From cc6927b2d8337d0bf9024c3d77cff00a9397872f Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Tue, 9 Aug 2022 06:39:24 +0200 Subject: [PATCH] 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 Co-authored-by: wxiaoguang --- docs/content/doc/packages/generic.en-us.md | 75 ++++++++++- integrations/api_packages_generic_test.go | 147 ++++++++++++++++----- routers/api/packages/api.go | 15 ++- routers/api/packages/generic/generic.go | 93 ++++++++----- 4 files changed, 254 insertions(+), 76 deletions(-) diff --git a/docs/content/doc/packages/generic.en-us.md b/docs/content/doc/packages/generic.en-us.md index a82058da8a..dd0f5653ff 100644 --- a/docs/content/doc/packages/generic.en-us.md +++ b/docs/content/doc/packages/generic.en-us.md @@ -27,7 +27,7 @@ To authenticate to the Package Registry, you need to provide [custom HTTP header ## Publish a package To publish a generic package perform a HTTP PUT operation with the package content in the request body. -You cannot publish a package if a package of the same name and version already exists. You must delete the existing package first. +You cannot publish a file with the same name twice to a package. You must delete the existing package version first. ``` PUT https://gitea.example.com/api/packages/{owner}/generic/{package_name}/{package_version}/{file_name} @@ -36,9 +36,9 @@ PUT https://gitea.example.com/api/packages/{owner}/generic/{package_name}/{packa | Parameter | Description | | ----------------- | ----------- | | `owner` | The owner of the package. | -| `package_name` | The package name. It can contain only lowercase letters (`a-z`), uppercase letter (`A-Z`), numbers (`0-9`), dots (`.`), hyphens (`-`), or underscores (`_`). | -| `package_version` | The package version, a non-empty string. | -| `file_name` | The filename. It can contain only lowercase letters (`a-z`), uppercase letter (`A-Z`), numbers (`0-9`), dots (`.`), hyphens (`-`), or underscores (`_`). | +| `package_name` | The package name. It can contain only lowercase letters (`a-z`), uppercase letter (`A-Z`), numbers (`0-9`), dots (`.`), hyphens (`-`), pluses (`+`), or underscores (`_`). | +| `package_version` | The package version, a non-empty string without trailing or leading whitespaces. | +| `file_name` | The filename. It can contain only lowercase letters (`a-z`), uppercase letter (`A-Z`), numbers (`0-9`), dots (`.`), hyphens (`-`), pluses (`+`), or underscores (`_`). | Example request using HTTP Basic authentication: @@ -55,7 +55,8 @@ The server reponds with the following HTTP Status codes. | HTTP Status Code | Meaning | | ----------------- | ------- | | `201 Created` | The package has been published. | -| `400 Bad Request` | The package name and/or version are invalid or a package with the same name and version already exist. | +| `400 Bad Request` | The package name and/or version and/or file name are invalid. | +| `409 Conflict` | A file with the same name exist already in the package. | ## Download a package @@ -80,3 +81,67 @@ Example request using HTTP Basic authentication: curl --user your_username:your_token_or_password \ https://gitea.example.com/api/packages/testuser/generic/test_package/1.0.0/file.bin ``` + +The server reponds with the following HTTP Status codes. + +| HTTP Status Code | Meaning | +| ----------------- | ------- | +| `200 OK` | Success | +| `404 Not Found` | The package or file was not found. | + +## Delete a package + +To delete a generic package perform a HTTP DELETE operation. This will delete all files of this version. + +``` +DELETE https://gitea.example.com/api/packages/{owner}/generic/{package_name}/{package_version} +``` + +| Parameter | Description | +| ----------------- | ----------- | +| `owner` | The owner of the package. | +| `package_name` | The package name. | +| `package_version` | The package version. | + +Example request using HTTP Basic authentication: + +```shell +curl --user your_username:your_token_or_password -X DELETE \ + https://gitea.example.com/api/packages/testuser/generic/test_package/1.0.0 +``` + +The server reponds with the following HTTP Status codes. + +| HTTP Status Code | Meaning | +| ----------------- | ------- | +| `204 No Content` | Success | +| `404 Not Found` | The package was not found. | + +## Delete a package file + +To delete a file of a generic package perform a HTTP DELETE operation. This will delete the package version too if there is no file left. + +``` +DELETE https://gitea.example.com/api/packages/{owner}/generic/{package_name}/{package_version}/{filename} +``` + +| Parameter | Description | +| ----------------- | ----------- | +| `owner` | The owner of the package. | +| `package_name` | The package name. | +| `package_version` | The package version. | +| `filename` | The filename. | + +Example request using HTTP Basic authentication: + +```shell +curl --user your_username:your_token_or_password -X DELETE \ + https://gitea.example.com/api/packages/testuser/generic/test_package/1.0.0/file.bin +``` + +The server reponds with the following HTTP Status codes. + +| HTTP Status Code | Meaning | +| ----------------- | ------- | +| `204 No Content` | Success | +| `404 Not Found` | The package or file was not found. | diff --git a/integrations/api_packages_generic_test.go b/integrations/api_packages_generic_test.go index adaf99e981..c60a387f53 100644 --- a/integrations/api_packages_generic_test.go +++ b/integrations/api_packages_generic_test.go @@ -23,16 +23,16 @@ func TestPackageGeneric(t *testing.T) { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User) packageName := "te-st_pac.kage" - packageVersion := "1.0.3" + packageVersion := "1.0.3-te st" filename := "fi-le_na.me" content := []byte{1, 2, 3} - url := fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", user.Name, packageName, packageVersion, filename) + url := fmt.Sprintf("/api/packages/%s/generic/%s/%s", user.Name, packageName, packageVersion) t.Run("Upload", func(t *testing.T) { defer PrintCurrentTest(t)() - req := NewRequestWithBody(t, "PUT", url, bytes.NewReader(content)) + req := NewRequestWithBody(t, "PUT", url+"/"+filename, bytes.NewReader(content)) AddBasicAuthHeader(req, user.Name) MakeRequest(t, req, http.StatusCreated) @@ -55,54 +55,139 @@ func TestPackageGeneric(t *testing.T) { pb, err := packages.GetBlobByID(db.DefaultContext, pfs[0].BlobID) assert.NoError(t, err) assert.Equal(t, int64(len(content)), pb.Size) - }) - t.Run("UploadExists", func(t *testing.T) { - defer PrintCurrentTest(t)() + t.Run("Exists", func(t *testing.T) { + defer PrintCurrentTest(t)() - req := NewRequestWithBody(t, "PUT", url, bytes.NewReader(content)) - AddBasicAuthHeader(req, user.Name) - MakeRequest(t, req, http.StatusBadRequest) + req := NewRequestWithBody(t, "PUT", url+"/"+filename, bytes.NewReader(content)) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusConflict) + }) + + t.Run("Additional", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := NewRequestWithBody(t, "PUT", url+"/dummy.bin", bytes.NewReader(content)) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusCreated) + + // Check deduplication + pfs, err := packages.GetFilesByVersionID(db.DefaultContext, pvs[0].ID) + assert.NoError(t, err) + assert.Len(t, pfs, 2) + assert.Equal(t, pfs[0].BlobID, pfs[1].BlobID) + }) + + t.Run("InvalidParameter", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := NewRequestWithBody(t, "PUT", fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", user.Name, "invalid+package name", packageVersion, filename), bytes.NewReader(content)) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusBadRequest) + + req = NewRequestWithBody(t, "PUT", fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", user.Name, packageName, "%20test ", filename), bytes.NewReader(content)) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusBadRequest) + + req = NewRequestWithBody(t, "PUT", fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", user.Name, packageName, packageVersion, "inval+id.na me"), bytes.NewReader(content)) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusBadRequest) + }) }) t.Run("Download", func(t *testing.T) { defer PrintCurrentTest(t)() - req := NewRequest(t, "GET", url) + checkDownloadCount := func(count int64) { + pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeGeneric) + assert.NoError(t, err) + assert.Len(t, pvs, 1) + assert.Equal(t, count, pvs[0].DownloadCount) + } + + checkDownloadCount(0) + + req := NewRequest(t, "GET", url+"/"+filename) resp := MakeRequest(t, req, http.StatusOK) assert.Equal(t, content, resp.Body.Bytes()) - pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeGeneric) - assert.NoError(t, err) - assert.Len(t, pvs, 1) - assert.Equal(t, int64(1), pvs[0].DownloadCount) + checkDownloadCount(1) + + req = NewRequest(t, "GET", url+"/dummy.bin") + MakeRequest(t, req, http.StatusOK) + + checkDownloadCount(2) + + t.Run("NotExists", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := NewRequest(t, "GET", url+"/not.found") + MakeRequest(t, req, http.StatusNotFound) + }) }) t.Run("Delete", func(t *testing.T) { defer PrintCurrentTest(t)() - req := NewRequest(t, "DELETE", url) - AddBasicAuthHeader(req, user.Name) - MakeRequest(t, req, http.StatusOK) + t.Run("File", func(t *testing.T) { + defer PrintCurrentTest(t)() - pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeGeneric) - assert.NoError(t, err) - assert.Empty(t, pvs) - }) + req := NewRequest(t, "DELETE", url+"/"+filename) + MakeRequest(t, req, http.StatusUnauthorized) - t.Run("DownloadNotExists", func(t *testing.T) { - defer PrintCurrentTest(t)() + req = NewRequest(t, "DELETE", url+"/"+filename) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusNoContent) - req := NewRequest(t, "GET", url) - MakeRequest(t, req, http.StatusNotFound) - }) + req = NewRequest(t, "GET", url+"/"+filename) + MakeRequest(t, req, http.StatusNotFound) - t.Run("DeleteNotExists", func(t *testing.T) { - defer PrintCurrentTest(t)() + req = NewRequest(t, "DELETE", url+"/"+filename) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusNotFound) - req := NewRequest(t, "DELETE", url) - AddBasicAuthHeader(req, user.Name) - MakeRequest(t, req, http.StatusNotFound) + pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeGeneric) + assert.NoError(t, err) + assert.Len(t, pvs, 1) + + t.Run("RemovesVersion", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req = NewRequest(t, "DELETE", url+"/dummy.bin") + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusNoContent) + + pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeGeneric) + assert.NoError(t, err) + assert.Empty(t, pvs) + }) + }) + + t.Run("Version", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := NewRequestWithBody(t, "PUT", url+"/"+filename, bytes.NewReader(content)) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusCreated) + + req = NewRequest(t, "DELETE", url) + MakeRequest(t, req, http.StatusUnauthorized) + + req = NewRequest(t, "DELETE", url) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusNoContent) + + pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeGeneric) + assert.NoError(t, err) + assert.Empty(t, pvs) + + req = NewRequest(t, "GET", url+"/"+filename) + MakeRequest(t, req, http.StatusNotFound) + + req = NewRequest(t, "DELETE", url) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusNotFound) + }) }) } 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 } - return packageName, packageVersion, filename, nil + 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 + } + } + + ctx.Status(http.StatusNoContent) }