check for "no change"

server/api/api.go

@@ -70,26 +70,28 @@ func (h *Handler) handleErrorWithCode(w http.ResponseWriter, code int, errTitle
 
 func (h *Handler) adminOrPluginRequired(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		var err error
+		authorized := false
+		pluginId := r.Header.Get("Mattermost-Plugin-ID")
+		if pluginId != "" {
+			// All other plugins are allowed
+			authorized = true
+		}
+
 		userID := r.Header.Get("Mattermost-User-ID")
 		if userID != "" {
-			authorized, err := h.authorization.IsAuthorizedAdmin(userID)
+			authorized, err = h.authorization.IsAuthorizedAdmin(userID)
 			if err != nil {
 				http.Error(w, "Not authorized", http.StatusUnauthorized)
 				return
 			}
-			if authorized {
-				next.ServeHTTP(w, r)
-				return
-			}
 		}
 
-		pluginId := r.Header.Get("Mattermost-Plugin-ID")
-		if pluginId != "" {
-			next.ServeHTTP(w, r)
+		if !authorized {
+			http.Error(w, "Not authorized", http.StatusUnauthorized)
 			return
 		}
-
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
+		next.ServeHTTP(w, r)
 	})
 }
 
@@ -106,22 +108,31 @@ func (h *Handler) setLink(w http.ResponseWriter, r *http.Request) {
 
 	links := h.store.GetLinks()
 	found := false
+	changed := false
 	for i := range links {
 		if links[i].Name == newLink.Name || links[i].Pattern == newLink.Pattern {
-			links[i] = newLink
+			if !links[i].Equals(newLink) {
+				links[i] = newLink
+				changed = true
+			}
 			found = true
 			break
 		}
 	}
 	if !found {
 		links = append(h.store.GetLinks(), newLink)
+		changed = true
+	}
+	status := http.StatusNotModified
+	if changed {
 		if err := h.store.SaveLinks(links); err != nil {
 			h.handleError(w, fmt.Errorf("Unable to save link: %w", err))
 			return
 		}
+		status = http.StatusOK
 	}
 
 	w.Header().Set("Content-Type", "application/json")
-	w.WriteHeader(http.StatusOK)
+	w.WriteHeader(status)
 	_, _ = w.Write([]byte(`{"status": "OK"}`))
 }

server/api/api_test.go

@@ -0,0 +1,187 @@
+package api
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/mattermost/mattermost-plugin-autolink/server/autolink"
+	"github.com/stretchr/testify/require"
+)
+
+type authorizeAll struct{}
+
+func (authorizeAll) IsAuthorizedAdmin(string) (bool, error) {
+	return true, nil
+}
+
+type linkStore struct {
+	prev       []autolink.Autolink
+	saveCalled *bool
+	saved      *[]autolink.Autolink
+}
+
+func (s *linkStore) GetLinks() []autolink.Autolink {
+	return s.prev
+}
+
+func (s *linkStore) SaveLinks(links []autolink.Autolink) error {
+	*s.saved = links
+	*s.saveCalled = true
+	return nil
+}
+
+func TestSetLink(t *testing.T) {
+	for _, tc := range []struct {
+		name             string
+		method           string
+		prevLinks        []autolink.Autolink
+		link             autolink.Autolink
+		expectSaveCalled bool
+		expectSaved      []autolink.Autolink
+		expectStatus     int
+	}{
+		{
+			name: "happy simple",
+			link: autolink.Autolink{
+				Name: "test",
+			},
+			expectStatus:     http.StatusOK,
+			expectSaveCalled: true,
+			expectSaved: []autolink.Autolink{
+				autolink.Autolink{
+					Name: "test",
+				},
+			},
+		},
+		{
+			name: "add new link",
+			link: autolink.Autolink{
+				Name:     "test1",
+				Pattern:  ".*1",
+				Template: "test1",
+			},
+			prevLinks: []autolink.Autolink{
+				autolink.Autolink{
+					Name:     "test2",
+					Pattern:  ".*2",
+					Template: "test2",
+				},
+			},
+			expectStatus:     http.StatusOK,
+			expectSaveCalled: true,
+			expectSaved: []autolink.Autolink{
+				autolink.Autolink{
+					Name:     "test2",
+					Pattern:  ".*2",
+					Template: "test2",
+				},
+				autolink.Autolink{
+					Name:     "test1",
+					Pattern:  ".*1",
+					Template: "test1",
+				},
+			},
+		}, {
+			name: "replace link",
+			link: autolink.Autolink{
+				Name:     "test2",
+				Pattern:  ".*2",
+				Template: "new template",
+			},
+			prevLinks: []autolink.Autolink{
+				autolink.Autolink{
+					Name:     "test1",
+					Pattern:  ".*1",
+					Template: "test1",
+				},
+				autolink.Autolink{
+					Name:     "test2",
+					Pattern:  ".*2",
+					Template: "test2",
+				},
+				autolink.Autolink{
+					Name:     "test3",
+					Pattern:  ".*3",
+					Template: "test3",
+				},
+			},
+			expectStatus:     http.StatusOK,
+			expectSaveCalled: true,
+			expectSaved: []autolink.Autolink{
+				autolink.Autolink{
+					Name:     "test1",
+					Pattern:  ".*1",
+					Template: "test1",
+				},
+				autolink.Autolink{
+					Name:     "test2",
+					Pattern:  ".*2",
+					Template: "new template",
+				},
+				autolink.Autolink{
+					Name:     "test3",
+					Pattern:  ".*3",
+					Template: "test3",
+				},
+			},
+		},
+		{
+			name: "no change",
+			link: autolink.Autolink{
+				Name:     "test2",
+				Pattern:  ".*2",
+				Template: "test2",
+			},
+			prevLinks: []autolink.Autolink{
+				autolink.Autolink{
+					Name:     "test1",
+					Pattern:  ".*1",
+					Template: "test1",
+				},
+				autolink.Autolink{
+					Name:     "test2",
+					Pattern:  ".*2",
+					Template: "test2",
+				},
+			},
+			expectStatus:     http.StatusNotModified,
+			expectSaveCalled: false,
+		},
+	} {
+		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{},
+			)
+
+			body, err := json.Marshal(tc.link)
+			require.NoError(t, err)
+
+			w := httptest.NewRecorder()
+			method := "POST"
+			if tc.method != "" {
+				method = tc.method
+			}
+			r, err := http.NewRequest(method, "/api/v1/link", bytes.NewReader(body))
+			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)
+			require.Equal(t, tc.expectSaveCalled, saveCalled)
+			require.Equal(t, tc.expectSaved, saved)
+		})
+	}
+}

server/autolink/autolink.go

@@ -21,6 +21,25 @@ type Autolink struct {
 	canReplaceAll bool
 }
 
+func (l Autolink) Equals(x Autolink) bool {
+	if l.Disabled != x.Disabled ||
+		l.DisableNonWordPrefix != x.DisableNonWordPrefix ||
+		l.DisableNonWordSuffix != x.DisableNonWordSuffix ||
+		l.Name != x.Name ||
+		l.Pattern != x.Pattern ||
+		len(l.Scope) != len(x.Scope) ||
+		l.Template != x.Template ||
+		l.WordMatch != x.WordMatch {
+		return false
+	}
+	for i, scope := range l.Scope {
+		if scope != x.Scope[i] {
+			return false
+		}
+	}
+	return true
+}
+
 // DisplayName returns a display name for the link.
 func (l Autolink) DisplayName() string {
 	if l.Name != "" {

server/autolink/autolink_test.go

@@ -557,3 +557,31 @@ func TestWordMatch(t *testing.T) {
 		})
 	}
 }
+func TestEquals(t *testing.T) {
+	for _, tc := range []struct {
+		l1, l2      autolink.Autolink
+		expectEqual bool
+	}{
+		{
+			l1: autolink.Autolink{
+				Name: "test",
+			},
+			expectEqual: false,
+		},
+		{
+			l1: autolink.Autolink{
+				Name: "test",
+			},
+			l2: autolink.Autolink{
+				Name: "test",
+			},
+			expectEqual: true,
+		},
+	} {
+
+		t.Run(tc.l1.Name+"-"+tc.l2.Name, func(t *testing.T) {
+			eq := tc.l1.Equals(tc.l2)
+			assert.Equal(t, tc.expectEqual, eq)
+		})
+	}
+}

server/autolinkplugin/plugin_test.go

@@ -354,7 +354,6 @@ func TestAPI(t *testing.T) {
 	api.On("UnregisterCommand", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return((*model.AppError)(nil))
 	api.On("GetChannel", mock.AnythingOfType("string")).Return(&testChannel, nil)
 	api.On("GetTeam", mock.AnythingOfType("string")).Return(&testTeam, nil)
-	api.On("GetUser", mock.AnythingOfType("string")).Return(&testTeam, nil)
 	api.On("SavePluginConfig", mock.AnythingOfType("map[string]interface {}")).Return(nil)
 
 	p := Plugin{}