Split /api/v1/admin/hooks into /api/v1/admin/system-hooks and /api/v1…

…/admin/default-hooks This should better address the ambiguity that led to go-gitea#23139. Rename parts of the supporting module to match this naming convention.
neuropoly · Jul 2, 2023 · 5384c1d · 5384c1d
1 parent 040cb8b
commit 5384c1d
Show file tree

Hide file tree

Showing 11 changed files with 312 additions and 254 deletions.
diff --git a/models/webhook/webhook_system.go b/models/webhook/webhook_system.go
@@ -11,19 +11,11 @@ import (
 	"code.gitea.io/gitea/modules/util"
 )
 
-// GetDefaultWebhooks returns all admin-default webhooks.
-func GetDefaultWebhooks(ctx context.Context) ([]*Webhook, error) {
-	webhooks := make([]*Webhook, 0, 5)
-	return webhooks, db.GetEngine(ctx).
-		Where("repo_id=? AND owner_id=? AND is_system_webhook=?", 0, 0, false).
-		Find(&webhooks)
-}
-
-// GetSystemOrDefaultWebhook returns admin system or default webhook by given ID.
-func GetSystemOrDefaultWebhook(ctx context.Context, id int64) (*Webhook, error) {
+// GetSystemWebhook returns admin default webhook by given ID.
+func GetAdminWebhook(ctx context.Context, id int64, isSystemWebhook bool) (*Webhook, error) {
 	webhook := &Webhook{ID: id}
 	has, err := db.GetEngine(ctx).
-		Where("repo_id=? AND owner_id=?", 0, 0).
+		Where("repo_id=? AND owner_id=? AND is_system_webhook=?", 0, 0, isSystemWebhook).
 		Get(webhook)
 	if err != nil {
 		return nil, err
@@ -33,34 +25,27 @@ func GetSystemOrDefaultWebhook(ctx context.Context, id int64) (*Webhook, error)
 	return webhook, nil
 }
 
-// GetSystemWebhooks returns all admin system webhooks.
-func GetSystemWebhooks(ctx context.Context, isActive util.OptionalBool) ([]*Webhook, error) {
-	webhooks := make([]*Webhook, 0, 5)
-	if isActive.IsNone() {
-		return webhooks, db.GetEngine(ctx).
-			Where("repo_id=? AND owner_id=? AND is_system_webhook=?", 0, 0, true).
-			Find(&webhooks)
+// returns all admin system or default webhooks.
+// isSystemWebhook == true gives system webhooks, otherwise gives default webhooks.
+// isActive filters system webhooks to those currently enabled or disabled; pass util.OptionalBoolNone to get both.
+// isActive is ignored when requesting default webhooks.
+func GetAdminWebhooks(ctx context.Context, isSystemWebhook bool, isActive util.OptionalBool) ([]*Webhook, error) {
+	if !isSystemWebhook {
+		isActive = util.OptionalBoolNone
 	}
-	return webhooks, db.GetEngine(ctx).
-		Where("repo_id=? AND owner_id=? AND is_system_webhook=? AND is_active = ?", 0, 0, true, isActive.IsTrue()).
-		Find(&webhooks)
-}
-
-// GetSystemOrDefaultWebhooks returns all admin webhooks.
-func GetSystemOrDefaultWebhooks(ctx context.Context, isActive util.OptionalBool) ([]*Webhook, error) {
 	webhooks := make([]*Webhook, 0, 5)
 	if isActive.IsNone() {
 		return webhooks, db.GetEngine(ctx).
-			Where("repo_id=? AND org_id=?", 0, 0).
+			Where("repo_id=? AND owner_id=? AND is_system_webhook=?", 0, 0, isSystemWebhook).
 			Find(&webhooks)
 	}
 	return webhooks, db.GetEngine(ctx).
-		Where("repo_id=? AND org_id=? AND is_active = ?", 0, 0, isActive.IsTrue()).
+		Where("repo_id=? AND owner_id=? AND is_system_webhook=? AND is_active = ?", 0, 0, isSystemWebhook, isActive.IsTrue()).
 		Find(&webhooks)
 }
 
-// DeleteDefaultSystemWebhook deletes an admin-configured default or system webhook (where Org and Repo ID both 0)
-func DeleteDefaultSystemWebhook(ctx context.Context, id int64) error {
+// DeleteWebhook deletes an admin-configured default or system webhook (where Org and Repo ID both 0)
+func DeleteAdminWebhook(ctx context.Context, id int64) error {
 	return db.WithTx(ctx, func(ctx context.Context) error {
 		count, err := db.GetEngine(ctx).
 			Where("repo_id=? AND owner_id=?", 0, 0).
@@ -78,7 +63,7 @@ func DeleteDefaultSystemWebhook(ctx context.Context, id int64) error {
 
 // CopyDefaultWebhooksToRepo creates copies of the default webhooks in a new repo
 func CopyDefaultWebhooksToRepo(ctx context.Context, repoID int64) error {
-	ws, err := GetDefaultWebhooks(ctx)
+	ws, err := GetAdminWebhooks(ctx, false, util.OptionalBoolNone)
 	if err != nil {
 		return fmt.Errorf("GetDefaultWebhooks: %v", err)
 	}

diff --git a/modules/structs/hook.go b/modules/structs/hook.go
@@ -24,7 +24,6 @@ type Hook struct {
 	Events              []string          `json:"events"`
 	AuthorizationHeader string            `json:"authorization_header"`
 	Active              bool              `json:"active"`
-	IsSystemWebhook     bool              `json:"is_system_webhook"`
 	// swagger:strfmt date-time
 	Updated time.Time `json:"updated_at"`
 	// swagger:strfmt date-time
@@ -49,8 +48,7 @@ type CreateHookOption struct {
 	BranchFilter        string                 `json:"branch_filter" binding:"GlobPattern"`
 	AuthorizationHeader string                 `json:"authorization_header"`
 	// default: false
-	Active          bool `json:"active"`
-	IsSystemWebhook bool `json:"is_system_webhook"`
+	Active bool `json:"active"`
 }
 
 // EditHookOption options when modify one hook
@@ -59,7 +57,6 @@ type EditHookOption struct {
 	Events              []string          `json:"events"`
 	BranchFilter        string            `json:"branch_filter" binding:"GlobPattern"`
 	AuthorizationHeader string            `json:"authorization_header"`
-	IsSystemWebhook     *bool             `json:"is_system_webhook"`
 	Active              *bool             `json:"active"`
 }
 

diff --git a/routers/api/v1/admin/hooks.go b/routers/api/v1/admin/hooks.go
@@ -17,33 +17,36 @@ import (
 	webhook_service "code.gitea.io/gitea/services/webhook"
 )
 
-// ListHooks list system's webhooks
+// swaggeroperation GET /admin/{hookType:default-hooks|system-hooks} admin adminListHooks
+
+// list system or default webhooks
 func ListHooks(ctx *context.APIContext) {
-	// swagger:operation GET /admin/hooks admin adminListHooks
+	// swagger:operation GET /admin/{hookType} admin adminListHooks
 	// ---
 	// summary: List system's webhooks
 	// produces:
 	// - application/json
 	// parameters:
-	// - name: page
-	//   in: query
-	//   description: page number of results to return (1-based)
-	//   type: integer
-	// - name: limit
-	//   in: query
-	//   description: page size of results
-	//   type: integer
+	// - name: hookType
+	//   in: path
+	//   description: whether the hook is system-wide or copied-to-each-new-repo
+	//   type: string
+	//   enum: [system-hooks, default-hooks]
+	//   required: true
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/HookList"
 
-	sysHooks, err := webhook.GetSystemOrDefaultWebhooks(ctx, util.OptionalBoolNone)
+	isSystemWebhook := ctx.Params(":hookType") == "system-hooks"
+
+	adminHooks, err := webhook.GetAdminWebhooks(ctx, isSystemWebhook, util.OptionalBoolNone)
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "GetSystemWebhooks", err)
+		ctx.Error(http.StatusInternalServerError, "GetAdminWebhooks", err)
 		return
 	}
-	hooks := make([]*api.Hook, len(sysHooks))
-	for i, hook := range sysHooks {
+
+	hooks := make([]*api.Hook, len(adminHooks))
+	for i, hook := range adminHooks {
 		h, err := webhook_service.ToHook(setting.AppURL+"/admin", hook)
 		if err != nil {
 			ctx.Error(http.StatusInternalServerError, "convert.ToHook", err)
@@ -54,14 +57,20 @@ func ListHooks(ctx *context.APIContext) {
 	ctx.JSON(http.StatusOK, hooks)
 }
 
-// GetHook get an organization's hook by id
+// get a system/default hook by id
 func GetHook(ctx *context.APIContext) {
-	// swagger:operation GET /admin/hooks/{id} admin adminGetHook
+	// swagger:operation GET /admin/{hookType}/{id} admin adminGetHook
 	// ---
 	// summary: Get a hook
 	// produces:
 	// - application/json
 	// parameters:
+	// - name: hookType
+	//   in: path
+	//   description: whether the hook is system-wide or copied-to-each-new-repo
+	//   type: string
+	//   enum: [system-hooks, default-hooks]
+	//   required: true
 	// - name: id
 	//   in: path
 	//   description: id of the hook to get
@@ -72,16 +81,19 @@ func GetHook(ctx *context.APIContext) {
 	//   "200":
 	//     "$ref": "#/responses/Hook"
 
+	isSystemWebhook := ctx.Params(":hookType") == "system-hooks"
+
 	hookID := ctx.ParamsInt64(":id")
-	hook, err := webhook.GetSystemOrDefaultWebhook(ctx, hookID)
+	hook, err := webhook.GetAdminWebhook(ctx, hookID, isSystemWebhook)
 	if err != nil {
 		if errors.Is(err, util.ErrNotExist) {
 			ctx.NotFound()
 		} else {
-			ctx.Error(http.StatusInternalServerError, "GetSystemOrDefaultWebhook", err)
+			ctx.Error(http.StatusInternalServerError, "GetAdminWebhook", err)
 		}
 		return
 	}
+
 	h, err := webhook_service.ToHook("/admin/", hook)
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "convert.ToHook", err)
@@ -90,16 +102,22 @@ func GetHook(ctx *context.APIContext) {
 	ctx.JSON(http.StatusOK, h)
 }
 
-// CreateHook create a hook for an organization
+// create a system or default hook
 func CreateHook(ctx *context.APIContext) {
-	// swagger:operation POST /admin/hooks admin adminCreateHook
+	// swagger:operation POST /admin/{hookType} admin adminCreateHook
 	// ---
 	// summary: Create a hook
 	// consumes:
 	// - application/json
 	// produces:
 	// - application/json
 	// parameters:
+	// - name: hookType
+	//   in: path
+	//   description: whether the hook is system-wide or copied-to-each-new-repo
+	//   type: string
+	//   enum: [system-hooks, default-hooks]
+	//   required: true
 	// - name: body
 	//   in: body
 	//   required: true
@@ -109,21 +127,29 @@ func CreateHook(ctx *context.APIContext) {
 	//   "201":
 	//     "$ref": "#/responses/Hook"
 
+	isSystemWebhook := ctx.Params(":hookType") == "system-hooks"
+
 	form := web.GetForm(ctx).(*api.CreateHookOption)
 
-	utils.AddSystemHook(ctx, form)
+	utils.AddAdminHook(ctx, form, isSystemWebhook)
 }
 
-// EditHook modify a hook of a repository
+// modify a system or default hook
 func EditHook(ctx *context.APIContext) {
-	// swagger:operation PATCH /admin/hooks/{id} admin adminEditHook
+	// swagger:operation PATCH /admin/{hookType}/{id} admin adminEditHook
 	// ---
 	// summary: Update a hook
 	// consumes:
 	// - application/json
 	// produces:
 	// - application/json
 	// parameters:
+	// - name: hookType
+	//   in: path
+	//   description: whether the hook is system-wide or copied-to-each-new-repo
+	//   type: string
+	//   enum: [system-hooks, default-hooks]
+	//   required: true
 	// - name: id
 	//   in: path
 	//   description: id of the hook to update
@@ -138,21 +164,29 @@ func EditHook(ctx *context.APIContext) {
 	//   "200":
 	//     "$ref": "#/responses/Hook"
 
+	isSystemWebhook := ctx.Params(":hookType") == "system-hooks"
+
 	form := web.GetForm(ctx).(*api.EditHookOption)
 
 	// TODO in body params
 	hookID := ctx.ParamsInt64(":id")
-	utils.EditSystemHook(ctx, form, hookID)
+	utils.EditHook(ctx, form, hookID, isSystemWebhook)
 }
 
-// DeleteHook delete a system hook
+// delete a system or default hook
 func DeleteHook(ctx *context.APIContext) {
-	// swagger:operation DELETE /admin/hooks/{id} admin adminDeleteHook
+	// swagger:operation DELETE /admin/{hookType}/{id} admin adminDeleteHook
 	// ---
 	// summary: Delete a hook
 	// produces:
 	// - application/json
 	// parameters:
+	// - name: hookType
+	//   in: path
+	//   description: whether the hook is system-wide or copied-to-each-new-repo
+	//   type: string
+	//   enum: [system-hooks, default-hooks]
+	//   required: true
 	// - name: id
 	//   in: path
 	//   description: id of the hook to delete
@@ -164,11 +198,11 @@ func DeleteHook(ctx *context.APIContext) {
 	//     "$ref": "#/responses/empty"
 
 	hookID := ctx.ParamsInt64(":id")
-	if err := webhook.DeleteDefaultSystemWebhook(ctx, hookID); err != nil {
+	if err := webhook.DeleteAdminWebhook(ctx, hookID); err != nil {
 		if errors.Is(err, util.ErrNotExist) {
 			ctx.NotFound()
 		} else {
-			ctx.Error(http.StatusInternalServerError, "DeleteDefaultSystemWebhook", err)
+			ctx.Error(http.StatusInternalServerError, "DeleteAdminWebhook", err)
 		}
 		return
 	}

diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go
@@ -1376,12 +1376,12 @@ func Routes() *web.Route {
 				m.Get("", admin.GetAllEmails)
 				m.Get("/search", admin.SearchEmail)
 			})
-			m.Group("/unadopted", func() {
+			m.Group("/uinadopted", func() {
 				m.Get("", admin.ListUnadoptedRepositories)
 				m.Post("/{username}/{reponame}", admin.AdoptRepository)
 				m.Delete("/{username}/{reponame}", admin.DeleteUnadoptedRepository)
 			})
-			m.Group("/hooks", func() {
+			m.Group("/{hookType:system-hooks|default-hooks}", func() {
 				m.Combo("").Get(admin.ListHooks).
 					Post(bind(api.CreateHookOption{}), admin.CreateHook)
 				m.Combo("/{id}").Get(admin.GetHook).