Bug fixes and unit tests for models/webhook (#751)

models/fixtures/hook_task.yml

@@ -0,0 +1,5 @@
+-
+  id: 1
+  repo_id: 1
+  hook_id: 1
+  uuid: uuid1

models/fixtures/webhook.yml

@@ -0,0 +1,24 @@
+-
+  id: 1
+  repo_id: 1
+  url: www.example.com/url1
+  content_type: 1 # json
+  events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}'
+  is_active: true
+
+-
+  id: 2
+  repo_id: 1
+  url: www.example.com/url2
+  content_type: 1 # json
+  events: '{"push_only":false,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":true}}'
+  is_active: false
+
+-
+  id: 3
+  org_id: 3
+  repo_id: 3
+  url: www.example.com/url3
+  content_type: 1 # json
+  events: '{"push_only":false,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":true}}'
+  is_active: true

models/setup_for_test.go

@@ -16,6 +16,8 @@ import (
 	"gopkg.in/testfixtures.v2"
 )
 
+const NonexistentID = 9223372036854775807
+
 func TestMain(m *testing.M) {
 	if err := CreateTestEngine(); err != nil {
 		fmt.Printf("Error creating test engine: %v\n", err)

models/webhook.go

@@ -231,10 +231,8 @@ func GetWebhookByOrgID(orgID, id int64) (*Webhook, error) {
 // GetActiveWebhooksByRepoID returns all active webhooks of repository.
 func GetActiveWebhooksByRepoID(repoID int64) ([]*Webhook, error) {
 	webhooks := make([]*Webhook, 0, 5)
-	return webhooks, x.Find(&webhooks, &Webhook{
-		RepoID:   repoID,
-		IsActive: true,
-	})
+	return webhooks, x.Where("is_active=?", true).
+		Find(&webhooks, &Webhook{RepoID: repoID})
 }
 
 // GetWebhooksByRepoID returns all webhooks of a repository.
@@ -243,6 +241,21 @@ func GetWebhooksByRepoID(repoID int64) ([]*Webhook, error) {
 	return webhooks, x.Find(&webhooks, &Webhook{RepoID: repoID})
 }
 
+// GetActiveWebhooksByOrgID returns all active webhooks for an organization.
+func GetActiveWebhooksByOrgID(orgID int64) (ws []*Webhook, err error) {
+	err = x.
+		Where("org_id=?", orgID).
+		And("is_active=?", true).
+		Find(&ws)
+	return ws, err
+}
+
+// GetWebhooksByOrgID returns all webhooks for an organization.
+func GetWebhooksByOrgID(orgID int64) (ws []*Webhook, err error) {
+	err = x.Find(&ws, &Webhook{OrgID: orgID})
+	return ws, err
+}
+
 // UpdateWebhook updates information of webhook.
 func UpdateWebhook(w *Webhook) error {
 	_, err := x.Id(w.ID).AllCols().Update(w)
@@ -285,21 +298,6 @@ func DeleteWebhookByOrgID(orgID, id int64) error {
 	})
 }
 
-// GetWebhooksByOrgID returns all webhooks for an organization.
-func GetWebhooksByOrgID(orgID int64) (ws []*Webhook, err error) {
-	err = x.Find(&ws, &Webhook{OrgID: orgID})
-	return ws, err
-}
-
-// GetActiveWebhooksByOrgID returns all active webhooks for an organization.
-func GetActiveWebhooksByOrgID(orgID int64) (ws []*Webhook, err error) {
-	err = x.
-		Where("org_id=?", orgID).
-		And("is_active=?", true).
-		Find(&ws)
-	return ws, err
-}
-
 //   ___ ___                __   ___________              __
 //  /   |   \  ____   ____ |  | _\__    ___/____    _____|  | __
 // /    ~    \/  _ \ /  _ \|  |/ / |    |  \__  \  /  ___/  |/ /
@@ -505,7 +503,7 @@ func PrepareWebhooks(repo *Repository, event HookEventType, p api.Payloader) err
 			}
 		}
 
-		// Use separate objects so modifcations won't be made on payload on non-Gogs type hooks.
+		// Use separate objects so modifications won't be made on payload on non-Gogs type hooks.
 		switch w.HookTaskType {
 		case SLACK:
 			payloader, err = GetSlackPayload(p, event, w.Meta)

models/webhook_test.go

@@ -0,0 +1,263 @@
+// Copyright 2017 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 models
+
+import (
+	"encoding/json"
+	"testing"
+
+	api "code.gitea.io/sdk/gitea"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestHookContentType_Name(t *testing.T) {
+	assert.Equal(t, "json", ContentTypeJSON.Name())
+	assert.Equal(t, "form", ContentTypeForm.Name())
+}
+
+func TestIsValidHookContentType(t *testing.T) {
+	assert.True(t, IsValidHookContentType("json"))
+	assert.True(t, IsValidHookContentType("form"))
+	assert.False(t, IsValidHookContentType("invalid"))
+}
+
+func TestWebhook_GetSlackHook(t *testing.T) {
+	w := &Webhook{
+		Meta: `{"channel": "foo", "username": "username", "color": "blue"}`,
+	}
+	slackHook := w.GetSlackHook()
+	assert.Equal(t, *slackHook, SlackMeta{
+		Channel:  "foo",
+		Username: "username",
+		Color:    "blue",
+	})
+}
+
+func TestWebhook_History(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	webhook := AssertExistsAndLoadBean(t, &Webhook{ID: 1}).(*Webhook)
+	tasks, err := webhook.History(0)
+	assert.NoError(t, err)
+	assert.Len(t, tasks, 1)
+	assert.Equal(t, int64(1), tasks[0].ID)
+
+	webhook = AssertExistsAndLoadBean(t, &Webhook{ID: 2}).(*Webhook)
+	tasks, err = webhook.History(0)
+	assert.NoError(t, err)
+	assert.Len(t, tasks, 0)
+}
+
+func TestWebhook_UpdateEvent(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	webhook := AssertExistsAndLoadBean(t, &Webhook{ID: 1}).(*Webhook)
+	hookEvent := &HookEvent{
+		PushOnly:       true,
+		SendEverything: false,
+		ChooseEvents:   false,
+		HookEvents: HookEvents{
+			Create:      false,
+			Push:        true,
+			PullRequest: false,
+		},
+	}
+	webhook.HookEvent = hookEvent
+	assert.NoError(t, webhook.UpdateEvent())
+	assert.NotEmpty(t, webhook.Events)
+	actualHookEvent := &HookEvent{}
+	assert.NoError(t, json.Unmarshal([]byte(webhook.Events), actualHookEvent))
+	assert.Equal(t, *hookEvent, *actualHookEvent)
+}
+
+func TestWebhook_EventsArray(t *testing.T) {
+	assert.Equal(t, []string{"create", "push", "pull_request"},
+		(&Webhook{
+			HookEvent: &HookEvent{SendEverything: true},
+		}).EventsArray(),
+	)
+
+	assert.Equal(t, []string{"push"},
+		(&Webhook{
+			HookEvent: &HookEvent{PushOnly: true},
+		}).EventsArray(),
+	)
+}
+
+func TestCreateWebhook(t *testing.T) {
+	hook := &Webhook{
+		RepoID:      3,
+		URL:         "www.example.com/unit_test",
+		ContentType: ContentTypeJSON,
+		Events:      `{"push_only":false,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":true}}`,
+	}
+	AssertNotExistsBean(t, hook)
+	assert.NoError(t, CreateWebhook(hook))
+	AssertExistsAndLoadBean(t, hook)
+}
+
+func TestGetWebhookByRepoID(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	hook, err := GetWebhookByRepoID(1, 1)
+	assert.NoError(t, err)
+	assert.Equal(t, int64(1), hook.ID)
+
+	_, err = GetWebhookByRepoID(NonexistentID, NonexistentID)
+	assert.Error(t, err)
+	assert.True(t, IsErrWebhookNotExist(err))
+}
+
+func TestGetWebhookByOrgID(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	hook, err := GetWebhookByOrgID(3, 3)
+	assert.NoError(t, err)
+	assert.Equal(t, int64(3), hook.ID)
+
+	_, err = GetWebhookByOrgID(NonexistentID, NonexistentID)
+	assert.Error(t, err)
+	assert.True(t, IsErrWebhookNotExist(err))
+}
+
+func TestGetActiveWebhooksByRepoID(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	hooks, err := GetActiveWebhooksByRepoID(1)
+	assert.NoError(t, err)
+	assert.Len(t, hooks, 1)
+	assert.Equal(t, int64(1), hooks[0].ID)
+	assert.True(t, hooks[0].IsActive)
+}
+
+func TestGetWebhooksByRepoID(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	hooks, err := GetWebhooksByRepoID(1)
+	assert.NoError(t, err)
+	assert.Len(t, hooks, 2)
+	assert.Equal(t, int64(1), hooks[0].ID)
+	assert.Equal(t, int64(2), hooks[1].ID)
+}
+
+func TestGetActiveWebhooksByOrgID(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	hooks, err := GetActiveWebhooksByOrgID(3)
+	assert.NoError(t, err)
+	assert.Len(t, hooks, 1)
+	assert.Equal(t, int64(3), hooks[0].ID)
+	assert.True(t, hooks[0].IsActive)
+}
+
+func TestGetWebhooksByOrgID(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	hooks, err := GetWebhooksByOrgID(3)
+	assert.NoError(t, err)
+	assert.Len(t, hooks, 1)
+	assert.Equal(t, int64(3), hooks[0].ID)
+	assert.True(t, hooks[0].IsActive)
+
+}
+
+func TestUpdateWebhook(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	hook := AssertExistsAndLoadBean(t, &Webhook{ID: 2}).(*Webhook)
+	hook.IsActive = true
+	hook.ContentType = ContentTypeForm
+	AssertNotExistsBean(t, hook)
+	assert.NoError(t, UpdateWebhook(hook))
+	AssertExistsAndLoadBean(t, hook)
+}
+
+func TestDeleteWebhookByRepoID(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	AssertExistsAndLoadBean(t, &Webhook{ID: 2, RepoID: 1})
+	assert.NoError(t, DeleteWebhookByRepoID(1, 2))
+	AssertNotExistsBean(t, &Webhook{ID: 2, RepoID: 1})
+
+	err := DeleteWebhookByRepoID(NonexistentID, NonexistentID)
+	assert.Error(t, err)
+	assert.True(t, IsErrWebhookNotExist(err))
+}
+
+func TestDeleteWebhookByOrgID(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	AssertExistsAndLoadBean(t, &Webhook{ID: 3, OrgID: 3})
+	assert.NoError(t, DeleteWebhookByOrgID(3, 3))
+	AssertNotExistsBean(t, &Webhook{ID: 3, OrgID: 3})
+
+	err := DeleteWebhookByOrgID(NonexistentID, NonexistentID)
+	assert.Error(t, err)
+	assert.True(t, IsErrWebhookNotExist(err))
+}
+
+func TestToHookTaskType(t *testing.T) {
+	assert.Equal(t, GOGS, ToHookTaskType("gogs"))
+	assert.Equal(t, SLACK, ToHookTaskType("slack"))
+}
+
+func TestHookTaskType_Name(t *testing.T) {
+	assert.Equal(t, "gogs", GOGS.Name())
+	assert.Equal(t, "slack", SLACK.Name())
+}
+
+func TestIsValidHookTaskType(t *testing.T) {
+	assert.True(t, IsValidHookTaskType("gogs"))
+	assert.True(t, IsValidHookTaskType("slack"))
+	assert.False(t, IsValidHookTaskType("invalid"))
+}
+
+func TestHookTasks(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	hookTasks, err := HookTasks(1, 1)
+	assert.NoError(t, err)
+	assert.Len(t, hookTasks, 1)
+	assert.Equal(t, int64(1), hookTasks[0].ID)
+
+	hookTasks, err = HookTasks(NonexistentID, 1)
+	assert.NoError(t, err)
+	assert.Len(t, hookTasks, 0)
+}
+
+func TestCreateHookTask(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+	hookTask := &HookTask{
+		RepoID:    3,
+		HookID:    3,
+		Type:      GOGS,
+		URL:       "http://www.example.com/unit_test",
+		Payloader: &api.PushPayload{},
+	}
+	AssertNotExistsBean(t, hookTask)
+	assert.NoError(t, CreateHookTask(hookTask))
+	AssertExistsAndLoadBean(t, hookTask)
+}
+
+func TestUpdateHookTask(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	hook := AssertExistsAndLoadBean(t, &HookTask{ID: 1}).(*HookTask)
+	hook.PayloadContent = "new payload content"
+	hook.DeliveredString = "new delivered string"
+	hook.IsDelivered = true
+	AssertNotExistsBean(t, hook)
+	assert.NoError(t, UpdateHookTask(hook))
+	AssertExistsAndLoadBean(t, hook)
+}
+
+func TestPrepareWebhooks(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
+	hookTasks := []*HookTask{
+		{RepoID: repo.ID, HookID: 1, EventType: HookEventPush},
+	}
+	for _, hookTask := range hookTasks {
+		AssertNotExistsBean(t, hookTask)
+	}
+	assert.NoError(t, PrepareWebhooks(repo, HookEventPush, &api.PushPayload{}))
+	for _, hookTask := range hookTasks {
+		AssertExistsAndLoadBean(t, hookTask)
+	}
+}
+
+// TODO TestHookTask_deliver
+
+// TODO TestDeliverHooks