From 78c7e8bcc042406ef51f48e2e86d1a1838f17ed3 Mon Sep 17 00:00:00 2001 From: Kshitij Katiyar Date: Thu, 6 Jul 2023 15:19:47 +0530 Subject: [PATCH 1/4] [MI-3267]:Added test cases for PR #213 'Add CRUD operations' --- server/api/api_test.go | 119 +++++++++++++++++++++++++++ server/autolinkclient/client_test.go | 44 ++++++++++ 2 files changed, 163 insertions(+) diff --git a/server/api/api_test.go b/server/api/api_test.go index faca9f9f..2f6e37c2 100644 --- a/server/api/api_test.go +++ b/server/api/api_test.go @@ -3,6 +3,8 @@ package api import ( "bytes" "encoding/json" + "fmt" + "io/ioutil" "net/http" "net/http/httptest" "testing" @@ -168,3 +170,120 @@ func TestSetLink(t *testing.T) { }) } } + +func TestGetLink(t *testing.T) { + for _, tc := range []struct { + name string + prevLinks []autolink.Autolink + autoLinkName string + expectStatus int + expectReturn string + }{ + { + name: "get the autolink", + autoLinkName: "test", + prevLinks: []autolink.Autolink{{ + Name: "test", + Pattern: ".*1", + Template: "test", + }}, + expectStatus: http.StatusOK, + expectReturn: `{"Name":"test","Disabled":false,"Pattern":".*1","Template":"test","Scope":null,"WordMatch":false,"DisableNonWordPrefix":false,"DisableNonWordSuffix":false,"ProcessBotPosts":false}`, + }, + { + name: "not found", + autoLinkName: "test", + prevLinks: []autolink.Autolink{{ + Name: "test1", + Pattern: ".*1", + Template: "test", + }}, + expectStatus: http.StatusInternalServerError, + expectReturn: "{\"error\":\"An internal error has occurred. Check app server logs for details.\",\"details\":\"no autolink found with name test\"}", + }, + } { + t.Run(tc.name, func(t *testing.T) { + var saved []autolink.Autolink + var saveCalled bool + + h := NewHandler( + &linkStore{ + prev: tc.prevLinks, + saveCalled: &saveCalled, + saved: &saved, + }, + authorizeAll{}, + ) + + w := httptest.NewRecorder() + r, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/api/v1/link?autolinkName=%s", tc.autoLinkName), nil) + require.NoError(t, err) + + r.Header.Set("Mattermost-Plugin-ID", "testfrom") + r.Header.Set("Mattermost-User-ID", "testuser") + + h.ServeHTTP(w, r) + + respBody, err := ioutil.ReadAll(w.Body) + require.NoError(t, err) + + require.Equal(t, tc.expectStatus, w.Code) + require.Equal(t, tc.expectReturn, string(respBody)) + }) + } +} + +func TestDeleteLink(t *testing.T) { + for _, tc := range []struct { + name string + prevLinks []autolink.Autolink + autoLinkName string + expectStatus int + }{ + { + name: "delete the autolink", + autoLinkName: "test", + prevLinks: []autolink.Autolink{{ + Name: "test", + Pattern: ".*1", + Template: "test", + }}, + expectStatus: http.StatusOK, + }, + { + name: "not found", + autoLinkName: "test", + prevLinks: []autolink.Autolink{{ + Name: "test1", + Pattern: ".*1", + Template: "test", + }}, + expectStatus: http.StatusNotModified, + }, + } { + t.Run(tc.name, func(t *testing.T) { + var saved []autolink.Autolink + var saveCalled bool + + h := NewHandler( + &linkStore{ + prev: tc.prevLinks, + saveCalled: &saveCalled, + saved: &saved, + }, + authorizeAll{}, + ) + + w := httptest.NewRecorder() + r, err := http.NewRequest(http.MethodDelete, fmt.Sprintf("/api/v1/link?autolinkName=%s", tc.autoLinkName), nil) + require.NoError(t, err) + + r.Header.Set("Mattermost-Plugin-ID", "testfrom") + r.Header.Set("Mattermost-User-ID", "testuser") + + h.ServeHTTP(w, r) + + require.Equal(t, tc.expectStatus, w.Code) + }) + } +} diff --git a/server/autolinkclient/client_test.go b/server/autolinkclient/client_test.go index 3d25e625..c8f219f4 100644 --- a/server/autolinkclient/client_test.go +++ b/server/autolinkclient/client_test.go @@ -1,7 +1,9 @@ package autolinkclient import ( + "io/ioutil" "net/http" + "strings" "testing" "github.com/mattermost/mattermost-server/v6/plugin/plugintest" @@ -53,3 +55,45 @@ func TestAddAutolinksErr(t *testing.T) { err := client.Add(autolink.Autolink{}) require.Error(t, err) } + +func TestDeleteAutolinks(t *testing.T) { + mockPluginAPI := &plugintest.API{} + + mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(&http.Response{StatusCode: http.StatusOK, Body: http.NoBody}) + + client := NewClientPlugin(mockPluginAPI) + err := client.Delete("") + require.Nil(t, err) +} + +func TestDeleteAutolinksErr(t *testing.T) { + mockPluginAPI := &plugintest.API{} + + mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(nil) + + client := NewClientPlugin(mockPluginAPI) + err := client.Delete("") + require.Error(t, err) +} + +func TestGetAutolinks(t *testing.T) { + mockPluginAPI := &plugintest.API{} + + r := ioutil.NopCloser(strings.NewReader("{}")) + + mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(&http.Response{StatusCode: http.StatusOK, Body: r}) + + client := NewClientPlugin(mockPluginAPI) + _, err := client.Get("") + require.Nil(t, err) +} + +func TestGetAutolinksErr(t *testing.T) { + mockPluginAPI := &plugintest.API{} + + mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(nil) + + client := NewClientPlugin(mockPluginAPI) + _, err := client.Get("") + require.Error(t, err) +} From efbaca69fce6802101a24ac7476c031ddb87318a Mon Sep 17 00:00:00 2001 From: Kshitij Katiyar Date: Thu, 6 Jul 2023 16:23:12 +0530 Subject: [PATCH 2/4] [MI-3267]:Fixed review comments --- server/api/api_test.go | 35 ++++------ server/autolinkclient/client_test.go | 100 +++++++++++++++++---------- 2 files changed, 77 insertions(+), 58 deletions(-) diff --git a/server/api/api_test.go b/server/api/api_test.go index 2f6e37c2..bb5bfbf2 100644 --- a/server/api/api_test.go +++ b/server/api/api_test.go @@ -172,32 +172,25 @@ func TestSetLink(t *testing.T) { } func TestGetLink(t *testing.T) { + autoLinkName := "test" + prevLinks := []autolink.Autolink{{ + Name: "test", + Pattern: ".*1", + Template: "test", + }} + for _, tc := range []struct { name string - prevLinks []autolink.Autolink - autoLinkName string expectStatus int expectReturn string }{ { name: "get the autolink", - autoLinkName: "test", - prevLinks: []autolink.Autolink{{ - Name: "test", - Pattern: ".*1", - Template: "test", - }}, expectStatus: http.StatusOK, expectReturn: `{"Name":"test","Disabled":false,"Pattern":".*1","Template":"test","Scope":null,"WordMatch":false,"DisableNonWordPrefix":false,"DisableNonWordSuffix":false,"ProcessBotPosts":false}`, }, { name: "not found", - autoLinkName: "test", - prevLinks: []autolink.Autolink{{ - Name: "test1", - Pattern: ".*1", - Template: "test", - }}, expectStatus: http.StatusInternalServerError, expectReturn: "{\"error\":\"An internal error has occurred. Check app server logs for details.\",\"details\":\"no autolink found with name test\"}", }, @@ -208,7 +201,7 @@ func TestGetLink(t *testing.T) { h := NewHandler( &linkStore{ - prev: tc.prevLinks, + prev: prevLinks, saveCalled: &saveCalled, saved: &saved, }, @@ -216,7 +209,7 @@ func TestGetLink(t *testing.T) { ) w := httptest.NewRecorder() - r, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/api/v1/link?autolinkName=%s", tc.autoLinkName), nil) + r, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/api/v1/link?autolinkName=%s", autoLinkName), nil) require.NoError(t, err) r.Header.Set("Mattermost-Plugin-ID", "testfrom") @@ -234,15 +227,14 @@ func TestGetLink(t *testing.T) { } func TestDeleteLink(t *testing.T) { + autoLinkName := "test" for _, tc := range []struct { name string prevLinks []autolink.Autolink - autoLinkName string expectStatus int }{ { - name: "delete the autolink", - autoLinkName: "test", + name: "delete the autolink", prevLinks: []autolink.Autolink{{ Name: "test", Pattern: ".*1", @@ -251,8 +243,7 @@ func TestDeleteLink(t *testing.T) { expectStatus: http.StatusOK, }, { - name: "not found", - autoLinkName: "test", + name: "not found", prevLinks: []autolink.Autolink{{ Name: "test1", Pattern: ".*1", @@ -275,7 +266,7 @@ func TestDeleteLink(t *testing.T) { ) w := httptest.NewRecorder() - r, err := http.NewRequest(http.MethodDelete, fmt.Sprintf("/api/v1/link?autolinkName=%s", tc.autoLinkName), nil) + r, err := http.NewRequest(http.MethodDelete, fmt.Sprintf("/api/v1/link?autolinkName=%s", autoLinkName), nil) require.NoError(t, err) r.Header.Set("Mattermost-Plugin-ID", "testfrom") diff --git a/server/autolinkclient/client_test.go b/server/autolinkclient/client_test.go index c8f219f4..aa0afde9 100644 --- a/server/autolinkclient/client_test.go +++ b/server/autolinkclient/client_test.go @@ -57,43 +57,71 @@ func TestAddAutolinksErr(t *testing.T) { } func TestDeleteAutolinks(t *testing.T) { - mockPluginAPI := &plugintest.API{} - - mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(&http.Response{StatusCode: http.StatusOK, Body: http.NoBody}) - - client := NewClientPlugin(mockPluginAPI) - err := client.Delete("") - require.Nil(t, err) -} - -func TestDeleteAutolinksErr(t *testing.T) { - mockPluginAPI := &plugintest.API{} - - mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(nil) - - client := NewClientPlugin(mockPluginAPI) - err := client.Delete("") - require.Error(t, err) + for _, tc := range []struct { + name string + errFound bool + }{ + { + name: "delete the autolink", + }, + { + name: "got error", + errFound: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + mockPluginAPI := &plugintest.API{} + + if tc.errFound { + mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(nil) + } else { + mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(&http.Response{StatusCode: http.StatusOK, Body: http.NoBody}) + } + + client := NewClientPlugin(mockPluginAPI) + err := client.Delete("") + + if tc.errFound { + require.Error(t, err) + } else { + require.Nil(t, err) + } + }) + } } func TestGetAutolinks(t *testing.T) { - mockPluginAPI := &plugintest.API{} - - r := ioutil.NopCloser(strings.NewReader("{}")) - - mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(&http.Response{StatusCode: http.StatusOK, Body: r}) - - client := NewClientPlugin(mockPluginAPI) - _, err := client.Get("") - require.Nil(t, err) -} - -func TestGetAutolinksErr(t *testing.T) { - mockPluginAPI := &plugintest.API{} - - mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(nil) - - client := NewClientPlugin(mockPluginAPI) - _, err := client.Get("") - require.Error(t, err) + for _, tc := range []struct { + name string + errFound bool + }{ + { + name: "get the autolink", + }, + { + name: "got error", + errFound: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + mockPluginAPI := &plugintest.API{} + + body := ioutil.NopCloser(strings.NewReader("{}")) + + if tc.errFound { + mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(nil) + } else { + mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(&http.Response{StatusCode: http.StatusOK, Body: body}) + } + + client := NewClientPlugin(mockPluginAPI) + _, err := client.Get("") + + if tc.errFound { + require.Error(t, err) + } else { + require.Nil(t, err) + } + }) + } } From cf4b86626e5c90396a03b9ff0e990d3afb3cced8 Mon Sep 17 00:00:00 2001 From: Kshitij Katiyar Date: Thu, 6 Jul 2023 20:16:18 +0530 Subject: [PATCH 3/4] [MI-3267]:Fixed review comments --- server/autolinkclient/client_test.go | 49 +++++++++++++++------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/server/autolinkclient/client_test.go b/server/autolinkclient/client_test.go index aa0afde9..e0c62338 100644 --- a/server/autolinkclient/client_test.go +++ b/server/autolinkclient/client_test.go @@ -1,6 +1,7 @@ package autolinkclient import ( + "errors" "io/ioutil" "net/http" "strings" @@ -59,29 +60,32 @@ func TestAddAutolinksErr(t *testing.T) { func TestDeleteAutolinks(t *testing.T) { for _, tc := range []struct { name string - errFound bool + setupAPI func(*plugintest.API) + err error }{ { name: "delete the autolink", + setupAPI: func(api *plugintest.API) { + body := ioutil.NopCloser(strings.NewReader("{}")) + api.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(&http.Response{StatusCode: http.StatusOK, Body: body}) + }, }, { - name: "got error", - errFound: true, + name: "got error", + setupAPI: func(api *plugintest.API) { + api.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(nil) + }, + err: errors.New("not able to delete the autolink"), }, } { t.Run(tc.name, func(t *testing.T) { mockPluginAPI := &plugintest.API{} - - if tc.errFound { - mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(nil) - } else { - mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(&http.Response{StatusCode: http.StatusOK, Body: http.NoBody}) - } + tc.setupAPI(mockPluginAPI) client := NewClientPlugin(mockPluginAPI) err := client.Delete("") - if tc.errFound { + if tc.err != nil { require.Error(t, err) } else { require.Nil(t, err) @@ -93,31 +97,32 @@ func TestDeleteAutolinks(t *testing.T) { func TestGetAutolinks(t *testing.T) { for _, tc := range []struct { name string - errFound bool + setupAPI func(*plugintest.API) + err error }{ { name: "get the autolink", + setupAPI: func(api *plugintest.API) { + body := ioutil.NopCloser(strings.NewReader("{}")) + api.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(&http.Response{StatusCode: http.StatusOK, Body: body}) + }, }, { - name: "got error", - errFound: true, + name: "got error", + setupAPI: func(api *plugintest.API) { + api.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(nil) + }, + err: errors.New("not able to get the autolink"), }, } { t.Run(tc.name, func(t *testing.T) { mockPluginAPI := &plugintest.API{} - - body := ioutil.NopCloser(strings.NewReader("{}")) - - if tc.errFound { - mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(nil) - } else { - mockPluginAPI.On("PluginHTTP", mock.AnythingOfType("*http.Request")).Return(&http.Response{StatusCode: http.StatusOK, Body: body}) - } + tc.setupAPI(mockPluginAPI) client := NewClientPlugin(mockPluginAPI) _, err := client.Get("") - if tc.errFound { + if tc.err != nil { require.Error(t, err) } else { require.Nil(t, err) From f31edf2c92d64b58d290efcbb0df2c80ee908185 Mon Sep 17 00:00:00 2001 From: Kshitij Katiyar Date: Fri, 7 Jul 2023 19:23:53 +0530 Subject: [PATCH 4/4] [MI-3267]:Fixed review comments --- server/api/api.go | 2 +- server/api/api_test.go | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/server/api/api.go b/server/api/api.go index df8ea444..40f77f12 100644 --- a/server/api/api.go +++ b/server/api/api.go @@ -144,7 +144,7 @@ func (h *Handler) deleteLink(w http.ResponseWriter, r *http.Request) { } } - status := http.StatusNotModified + status := http.StatusNotFound if found { if err := h.store.SaveLinks(links); err != nil { h.handleError(w, errors.Wrap(err, "unable to save the link")) diff --git a/server/api/api_test.go b/server/api/api_test.go index bb5bfbf2..2ade64c6 100644 --- a/server/api/api_test.go +++ b/server/api/api_test.go @@ -172,7 +172,6 @@ func TestSetLink(t *testing.T) { } func TestGetLink(t *testing.T) { - autoLinkName := "test" prevLinks := []autolink.Autolink{{ Name: "test", Pattern: ".*1", @@ -181,18 +180,21 @@ func TestGetLink(t *testing.T) { for _, tc := range []struct { name string + autoLinkName string expectStatus int expectReturn string }{ { name: "get the autolink", + autoLinkName: "test", expectStatus: http.StatusOK, expectReturn: `{"Name":"test","Disabled":false,"Pattern":".*1","Template":"test","Scope":null,"WordMatch":false,"DisableNonWordPrefix":false,"DisableNonWordSuffix":false,"ProcessBotPosts":false}`, }, { name: "not found", + autoLinkName: "test-1", expectStatus: http.StatusInternalServerError, - expectReturn: "{\"error\":\"An internal error has occurred. Check app server logs for details.\",\"details\":\"no autolink found with name test\"}", + expectReturn: `{"error":"An internal error has occurred. Check app server logs for details.","details":"no autolink found with name test-1"}`, }, } { t.Run(tc.name, func(t *testing.T) { @@ -209,7 +211,7 @@ func TestGetLink(t *testing.T) { ) w := httptest.NewRecorder() - r, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/api/v1/link?autolinkName=%s", autoLinkName), nil) + r, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/api/v1/link?autolinkName=%s", tc.autoLinkName), nil) require.NoError(t, err) r.Header.Set("Mattermost-Plugin-ID", "testfrom") @@ -249,7 +251,7 @@ func TestDeleteLink(t *testing.T) { Pattern: ".*1", Template: "test", }}, - expectStatus: http.StatusNotModified, + expectStatus: http.StatusNotFound, }, } { t.Run(tc.name, func(t *testing.T) {