Move webhook type from int to string (#13664)

* Move webhook type from int to string

* rename webhook_services

* finish refactor

* Fix merge

* Ignore unnecessary ci

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: zeripath <art27@cantab.net>

.golangci.yml

@@ -101,6 +101,9 @@ issues:
     - path: cmd/dump.go
       linters:
         - dupl
+    - path: services/webhook/webhook.go
+      linters:
+        - structcheck
     - text: "commentFormatting: put a space between `//` and comment text"
       linters:
         - gocritic

models/migrations/migrations.go

@@ -265,6 +265,8 @@ var migrations = []Migration{
 	NewMigration("update reactions constraint", updateReactionConstraint),
 	// v160 -> v161
 	NewMigration("Add block on official review requests branch protection", addBlockOnOfficialReviewRequests),
+	// v161 -> v162
+	NewMigration("Convert task type from int to string", convertTaskTypeToString),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v161.go

@@ -0,0 +1,59 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+	"xorm.io/xorm"
+)
+
+func convertTaskTypeToString(x *xorm.Engine) error {
+	const (
+		GOGS int = iota + 1
+		SLACK
+		GITEA
+		DISCORD
+		DINGTALK
+		TELEGRAM
+		MSTEAMS
+		FEISHU
+		MATRIX
+	)
+
+	var hookTaskTypes = map[int]string{
+		GITEA:    "gitea",
+		GOGS:     "gogs",
+		SLACK:    "slack",
+		DISCORD:  "discord",
+		DINGTALK: "dingtalk",
+		TELEGRAM: "telegram",
+		MSTEAMS:  "msteams",
+		FEISHU:   "feishu",
+		MATRIX:   "matrix",
+	}
+
+	type HookTask struct {
+		Typ string `xorm:"char(16) index"`
+	}
+	if err := x.Sync2(new(HookTask)); err != nil {
+		return err
+	}
+
+	for i, s := range hookTaskTypes {
+		if _, err := x.Exec("UPDATE hook_task set typ = ? where type=?", s, i); err != nil {
+			return err
+		}
+	}
+
+	sess := x.NewSession()
+	defer sess.Close()
+	if err := sess.Begin(); err != nil {
+		return err
+	}
+	if err := dropTableColumns(sess, "hook_task", "type"); err != nil {
+		return err
+	}
+
+	return sess.Commit()
+}

models/webhook.go

@@ -547,69 +547,21 @@ func copyDefaultWebhooksToRepo(e Engine, repoID int64) error {
 //        \/                    \/              \/     \/     \/
 
 // HookTaskType is the type of an hook task
-type HookTaskType int
+type HookTaskType string
 
 // Types of hook tasks
 const (
-	GOGS HookTaskType = iota + 1
-	SLACK
-	GITEA
-	DISCORD
-	DINGTALK
-	TELEGRAM
-	MSTEAMS
-	FEISHU
-	MATRIX
+	GITEA    HookTaskType = "gitea"
+	GOGS     HookTaskType = "gogs"
+	SLACK    HookTaskType = "slack"
+	DISCORD  HookTaskType = "discord"
+	DINGTALK HookTaskType = "dingtalk"
+	TELEGRAM HookTaskType = "telegram"
+	MSTEAMS  HookTaskType = "msteams"
+	FEISHU   HookTaskType = "feishu"
+	MATRIX   HookTaskType = "matrix"
 )
 
-var hookTaskTypes = map[string]HookTaskType{
-	"gitea":    GITEA,
-	"gogs":     GOGS,
-	"slack":    SLACK,
-	"discord":  DISCORD,
-	"dingtalk": DINGTALK,
-	"telegram": TELEGRAM,
-	"msteams":  MSTEAMS,
-	"feishu":   FEISHU,
-	"matrix":   MATRIX,
-}
-
-// ToHookTaskType returns HookTaskType by given name.
-func ToHookTaskType(name string) HookTaskType {
-	return hookTaskTypes[name]
-}
-
-// Name returns the name of an hook task type
-func (t HookTaskType) Name() string {
-	switch t {
-	case GITEA:
-		return "gitea"
-	case GOGS:
-		return "gogs"
-	case SLACK:
-		return "slack"
-	case DISCORD:
-		return "discord"
-	case DINGTALK:
-		return "dingtalk"
-	case TELEGRAM:
-		return "telegram"
-	case MSTEAMS:
-		return "msteams"
-	case FEISHU:
-		return "feishu"
-	case MATRIX:
-		return "matrix"
-	}
-	return ""
-}
-
-// IsValidHookTaskType returns true if given name is a valid hook task type.
-func IsValidHookTaskType(name string) bool {
-	_, ok := hookTaskTypes[name]
-	return ok
-}
-
 // HookEventType is the type of an hook event
 type HookEventType string
 
@@ -687,7 +639,7 @@ type HookTask struct {
 	RepoID          int64 `xorm:"INDEX"`
 	HookID          int64
 	UUID            string
-	Type            HookTaskType
+	Typ             HookTaskType
 	URL             string `xorm:"TEXT"`
 	Signature       string `xorm:"TEXT"`
 	api.Payloader   `xorm:"-"`

models/webhook_test.go

@@ -185,28 +185,6 @@ func TestDeleteWebhookByOrgID(t *testing.T) {
 	assert.True(t, IsErrWebhookNotExist(err))
 }
 
-func TestToHookTaskType(t *testing.T) {
-	assert.Equal(t, GOGS, ToHookTaskType("gogs"))
-	assert.Equal(t, SLACK, ToHookTaskType("slack"))
-	assert.Equal(t, GITEA, ToHookTaskType("gitea"))
-	assert.Equal(t, TELEGRAM, ToHookTaskType("telegram"))
-}
-
-func TestHookTaskType_Name(t *testing.T) {
-	assert.Equal(t, "gogs", GOGS.Name())
-	assert.Equal(t, "slack", SLACK.Name())
-	assert.Equal(t, "gitea", GITEA.Name())
-	assert.Equal(t, "telegram", TELEGRAM.Name())
-}
-
-func TestIsValidHookTaskType(t *testing.T) {
-	assert.True(t, IsValidHookTaskType("gogs"))
-	assert.True(t, IsValidHookTaskType("slack"))
-	assert.True(t, IsValidHookTaskType("gitea"))
-	assert.True(t, IsValidHookTaskType("telegram"))
-	assert.False(t, IsValidHookTaskType("invalid"))
-}
-
 func TestHookTasks(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 	hookTasks, err := HookTasks(1, 1)
@@ -225,7 +203,7 @@ func TestCreateHookTask(t *testing.T) {
 	hookTask := &HookTask{
 		RepoID:    3,
 		HookID:    3,
-		Type:      GITEA,
+		Typ:       GITEA,
 		URL:       "http://www.example.com/unit_test",
 		Payloader: &api.PushPayload{},
 	}

modules/convert/convert.go

@@ -16,7 +16,7 @@ import (
 	"code.gitea.io/gitea/modules/structs"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
-	"code.gitea.io/gitea/modules/webhook"
+	"code.gitea.io/gitea/services/webhook"
 
 	"github.com/unknwon/com"
 )
@@ -237,7 +237,7 @@ func ToHook(repoLink string, w *models.Webhook) *api.Hook {
 
 	return &api.Hook{
 		ID:      w.ID,
-		Type:    w.HookTaskType.Name(),
+		Type:    string(w.HookTaskType),
 		URL:     fmt.Sprintf("%s/settings/hooks/%d", repoLink, w.ID),
 		Active:  w.IsActive,
 		Config:  config,