Add unit tests for controllers & fix issue where api route was in the wrong location for template

core/template.go

@@ -20,10 +20,10 @@ import (
 )
 
 var (
-	errTemplateNameInvalid    = errors.New("Invalid Template Name")
-	errTemplateDataInvalid    = errors.New("Invalid Template Data")
-	errTemplateCreatedInvalid = errors.New("Invalid Template Created Value")
-	errTemplateUpdatedInvalid = errors.New("Invalid Template Updated Value")
+	errTemplateNameInvalid = errors.New("No Template Name Provided")
+	errTemplateDataInvalid = errors.New("No Template Data Provided")
+	//errTemplateCreatedInvalid = errors.New("Invalid Template Created Value")
+	//errTemplateUpdatedInvalid = errors.New("Invalid Template Updated Value")
 )
 
 type (
@@ -70,10 +70,10 @@ func (s *Template) Validate() error {
 		return errTemplateNameInvalid
 	case len(s.Data) == 0:
 		return errTemplateDataInvalid
-	case s.Created == 0:
-		return errTemplateCreatedInvalid
-	case s.Updated == 0:
-		return errTemplateUpdatedInvalid
+	//case s.Created == 0:
+	//	return errTemplateCreatedInvalid
+	//case s.Updated == 0:
+	//	return errTemplateUpdatedInvalid
 	default:
 		return nil
 	}

handler/api/api.go

@@ -15,6 +15,7 @@
 package api
 
 import (
+	"github.com/drone/drone/handler/api/template"
 	"net/http"
 	"os"
 
@@ -40,7 +41,6 @@ import (
 	"github.com/drone/drone/handler/api/repos/sign"
 	globalsecrets "github.com/drone/drone/handler/api/secrets"
 	"github.com/drone/drone/handler/api/system"
-	"github.com/drone/drone/handler/api/template"
 	"github.com/drone/drone/handler/api/user"
 	"github.com/drone/drone/handler/api/user/remote"
 	"github.com/drone/drone/handler/api/users"
@@ -257,15 +257,6 @@ func (s Server) Handler() http.Handler {
 				r.Delete("/{secret}", secrets.HandleDelete(s.Repos, s.Secrets))
 			})
 
-			r.Route("/templates", func(r chi.Router) {
-				r.Use(acl.CheckWriteAccess())
-				r.Get("/", template.HandleList(s.Template))
-				r.Post("/", template.HandleCreate(s.Template))
-				r.Get("/{name}", template.HandleFind(s.Template))
-				r.Patch("/{name}", template.HandleUpdate(s.Template))
-				r.Delete("/{name}", template.HandleDelete(s.Template))
-			})
-
 			r.Route("/sign", func(r chi.Router) {
 				r.Use(acl.CheckWriteAccess())
 				r.Post("/", sign.HandleSign(s.Repos))
@@ -366,6 +357,16 @@ func (s Server) Handler() http.Handler {
 		r.With(acl.CheckMembership(s.Orgs, true)).Delete("/{namespace}/{name}", globalsecrets.HandleDelete(s.Globals))
 	})
 
+	r.Route("/templates", func(r chi.Router) {
+		r.With(acl.AuthorizeAdmin).Get("/", template.HandleAll(s.Template))
+		r.With(acl.CheckMembership(s.Orgs, false)).Get("/{namespace}", template.HandleList(s.Template))
+		r.With(acl.CheckMembership(s.Orgs, true)).Post("/{namespace}", template.HandleCreate(s.Template))
+		r.With(acl.CheckMembership(s.Orgs, false)).Get("/{namespace}/{name}", template.HandleFind(s.Template))
+		r.With(acl.CheckMembership(s.Orgs, true)).Post("/{namespace}/{name}", template.HandleUpdate(s.Template))
+		r.With(acl.CheckMembership(s.Orgs, true)).Patch("/{namespace}/{name}", template.HandleUpdate(s.Template))
+		r.With(acl.CheckMembership(s.Orgs, true)).Delete("/{namespace}/{name}", template.HandleDelete(s.Template))
+	})
+
 	r.Route("/system", func(r chi.Router) {
 		r.Use(acl.AuthorizeAdmin)
 		// r.Get("/license", system.HandleLicense())

handler/api/template/all_test.go

@@ -6,24 +6,26 @@
 
 package template
 
-//func TestHandleAll(t *testing.T) {
-//	controller := gomock.NewController(t)
-//	defer controller.Finish()
-//
-//	secrets := mock.NewMockGlobalSecretStore(controller)
-//	secrets.EXPECT().ListAll(gomock.Any()).Return(dummySecretList, nil)
-//
-//	w := httptest.NewRecorder()
-//	r := httptest.NewRequest("GET", "/", nil)
-//
-//	HandleAll(secrets).ServeHTTP(w, r)
-//	if got, want := w.Code, http.StatusOK; want != got {
-//		t.Errorf("Want response code %d, got %d", want, got)
-//	}
-//
-//	got, want := []*core.Secret{}, dummySecretListScrubbed
-//	json.NewDecoder(w.Body).Decode(&got)
-//	if diff := cmp.Diff(got, want); len(diff) != 0 {
-//		t.Errorf(diff)
-//	}
-//}
+import (
+	"github.com/drone/drone/mock"
+	"github.com/golang/mock/gomock"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestHandleAll(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	templates := mock.NewMockTemplateStore(controller)
+	templates.EXPECT().ListAll(gomock.Any()).Return(dummyTemplateList, nil)
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", nil)
+
+	HandleAll(templates).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusOK; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+}

handler/api/template/create_test.go

@@ -0,0 +1,151 @@
+// Copyright 2019 Drone.IO Inc. All rights reserved.
+// Use of this source code is governed by the Drone Non-Commercial License
+// that can be found in the LICENSE file.
+
+// +build !oss
+
+package template
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"github.com/drone/drone/core"
+	"github.com/drone/drone/handler/api/errors"
+	"github.com/drone/drone/mock"
+	"github.com/go-chi/chi"
+	"github.com/golang/mock/gomock"
+	"github.com/google/go-cmp/cmp"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestHandleCreate(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	templates := mock.NewMockTemplateStore(controller)
+	templates.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil)
+
+	c := new(chi.Context)
+
+	in := new(bytes.Buffer)
+	json.NewEncoder(in).Encode(dummyTemplate)
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", in)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleCreate(templates).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusOK; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+}
+
+func TestHandleCreate_ValidationErrorName(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	c := new(chi.Context)
+	in := new(bytes.Buffer)
+	json.NewEncoder(in).Encode(&core.Template{Name: "", Data: "my_data"})
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", in)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleCreate(nil).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusBadRequest; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+
+	got, want := &errors.Error{}, &errors.Error{Message: "No Template Name Provided"}
+	json.NewDecoder(w.Body).Decode(got)
+	if diff := cmp.Diff(got, want); len(diff) != 0 {
+		t.Errorf(diff)
+	}
+}
+
+func TestHandleCreate_ValidationErrorData(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	c := new(chi.Context)
+	in := new(bytes.Buffer)
+	json.NewEncoder(in).Encode(&core.Template{Name: "my_template", Data: ""})
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", in)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleCreate(nil).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusBadRequest; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+
+	got, want := &errors.Error{}, &errors.Error{Message: "No Template Data Provided"}
+	json.NewDecoder(w.Body).Decode(got)
+	if diff := cmp.Diff(got, want); len(diff) != 0 {
+		t.Errorf(diff)
+	}
+}
+
+func TestHandleCreate_BadRequest(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	c := new(chi.Context)
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", nil)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleCreate(nil).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusBadRequest; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+
+	got, want := &errors.Error{}, &errors.Error{Message: "EOF"}
+	json.NewDecoder(w.Body).Decode(got)
+	if diff := cmp.Diff(got, want); len(diff) != 0 {
+		t.Errorf(diff)
+	}
+}
+
+func TestHandleCreate_CreateError(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	templates := mock.NewMockTemplateStore(controller)
+	templates.EXPECT().Create(gomock.Any(), gomock.Any()).Return(errors.ErrNotFound)
+
+	c := new(chi.Context)
+	in := new(bytes.Buffer)
+	json.NewEncoder(in).Encode(dummyTemplate)
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", in)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleCreate(templates).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusInternalServerError; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+
+	got, want := new(errors.Error), errors.ErrNotFound
+	json.NewDecoder(w.Body).Decode(got)
+	if diff := cmp.Diff(got, want); len(diff) != 0 {
+		t.Errorf(diff)
+	}
+}

handler/api/template/delete_test.go

@@ -0,0 +1,100 @@
+// Copyright 2019 Drone.IO Inc. All rights reserved.
+// Use of this source code is governed by the Drone Non-Commercial License
+// that can be found in the LICENSE file.
+
+// +build !oss
+
+package template
+
+import (
+	"context"
+	"encoding/json"
+	"github.com/drone/drone/handler/api/errors"
+	"github.com/drone/drone/mock"
+	"github.com/go-chi/chi"
+	"github.com/golang/mock/gomock"
+	"github.com/google/go-cmp/cmp"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestHandleDelete(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	template := mock.NewMockTemplateStore(controller)
+	template.EXPECT().FindName(gomock.Any(), dummyTemplate.Name).Return(dummyTemplate, nil)
+	template.EXPECT().Delete(gomock.Any(), dummyTemplate).Return(nil)
+
+	c := new(chi.Context)
+	c.URLParams.Add("name", "my_template")
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", nil)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleDelete(template).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusNoContent; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+}
+
+func TestHandleDelete_TemplateNotFound(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	template := mock.NewMockTemplateStore(controller)
+	template.EXPECT().FindName(gomock.Any(), dummyTemplate.Name).Return(nil, errors.ErrNotFound)
+
+	c := new(chi.Context)
+	c.URLParams.Add("name", "my_template")
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", nil)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleDelete(template).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusNotFound; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+
+	got, want := new(errors.Error), errors.ErrNotFound
+	json.NewDecoder(w.Body).Decode(got)
+	if diff := cmp.Diff(got, want); len(diff) != 0 {
+		t.Errorf(diff)
+	}
+}
+
+func TestHandleDelete_DeleteError(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	template := mock.NewMockTemplateStore(controller)
+	template.EXPECT().FindName(gomock.Any(), dummyTemplate.Name).Return(dummyTemplate, nil)
+	template.EXPECT().Delete(gomock.Any(), dummyTemplate).Return(errors.ErrNotFound)
+
+	c := new(chi.Context)
+	c.URLParams.Add("name", "my_template")
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", nil)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleDelete(template).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusInternalServerError; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+
+	got, want := new(errors.Error), errors.ErrNotFound
+	json.NewDecoder(w.Body).Decode(got)
+	if diff := cmp.Diff(got, want); len(diff) != 0 {
+		t.Errorf(diff)
+	}
+}

handler/api/template/list_test.go

@@ -0,0 +1,21 @@
+// Copyright 2019 Drone.IO Inc. All rights reserved.
+// Use of this source code is governed by the Drone Non-Commercial License
+// that can be found in the LICENSE file.
+
+// +build !oss
+
+package template
+
+import "github.com/drone/drone/core"
+
+var (
+	dummyTemplate = &core.Template{
+		Name:    "my_template",
+		Data:    "my_data",
+		Created: 1,
+		Updated: 2,
+	}
+	dummyTemplateList = []*core.Template{
+		dummyTemplate,
+	}
+)

handler/api/template/update_test.go

@@ -0,0 +1,142 @@
+// Copyright 2019 Drone.IO Inc. All rights reserved.
+// Use of this source code is governed by the Drone Non-Commercial License
+// that can be found in the LICENSE file.
+
+// +build !oss
+
+package template
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"github.com/drone/drone/core"
+	"github.com/drone/drone/handler/api/errors"
+	"github.com/drone/drone/mock"
+	"github.com/go-chi/chi"
+	"github.com/golang/mock/gomock"
+	"github.com/google/go-cmp/cmp"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestHandleUpdate(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	template := mock.NewMockTemplateStore(controller)
+	template.EXPECT().FindName(gomock.Any(), dummyTemplate.Name).Return(dummyTemplate, nil)
+	template.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil)
+
+	c := new(chi.Context)
+	c.URLParams.Add("name", "my_template")
+
+	in := new(bytes.Buffer)
+	json.NewEncoder(in).Encode(dummyTemplate)
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", in)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleUpdate(template).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusOK; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+}
+
+func TestHandleUpdate_ValidationErrorData(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	template := mock.NewMockTemplateStore(controller)
+	template.EXPECT().FindName(gomock.Any(), dummyTemplate.Name).Return(&core.Template{Name: "my_template"}, nil)
+
+	c := new(chi.Context)
+	c.URLParams.Add("name", "my_template")
+
+	in := new(bytes.Buffer)
+	json.NewEncoder(in).Encode(&core.Secret{Data: ""})
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", in)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleUpdate(template).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusBadRequest; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+
+	got, want := new(errors.Error), &errors.Error{Message: "No Template Data Provided"}
+	json.NewDecoder(w.Body).Decode(got)
+	if diff := cmp.Diff(got, want); len(diff) != 0 {
+		t.Errorf(diff)
+	}
+}
+
+func TestHandleUpdate_TemplateNotFound(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	template := mock.NewMockTemplateStore(controller)
+	template.EXPECT().FindName(gomock.Any(), dummyTemplate.Name).Return(nil, errors.ErrNotFound)
+
+	c := new(chi.Context)
+	c.URLParams.Add("name", "my_template")
+
+	in := new(bytes.Buffer)
+	json.NewEncoder(in).Encode(&core.Secret{})
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", in)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleUpdate(template).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusNotFound; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+
+	got, want := new(errors.Error), errors.ErrNotFound
+	json.NewDecoder(w.Body).Decode(got)
+	if diff := cmp.Diff(got, want); len(diff) != 0 {
+		t.Errorf(diff)
+	}
+}
+
+func TestHandleUpdate_UpdateError(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	template := mock.NewMockTemplateStore(controller)
+	template.EXPECT().FindName(gomock.Any(), dummyTemplate.Name).Return(&core.Template{Name: "my_template"}, nil)
+	template.EXPECT().Update(gomock.Any(), gomock.Any()).Return(errors.ErrNotFound)
+
+	c := new(chi.Context)
+	c.URLParams.Add("name", "my_template")
+
+	in := new(bytes.Buffer)
+	json.NewEncoder(in).Encode(&core.Template{Data: "my_data"})
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/", in)
+	r = r.WithContext(
+		context.WithValue(context.Background(), chi.RouteCtxKey, c),
+	)
+
+	HandleUpdate(template).ServeHTTP(w, r)
+	if got, want := w.Code, http.StatusInternalServerError; want != got {
+		t.Errorf("Want response code %d, got %d", want, got)
+	}
+
+	got, want := new(errors.Error), errors.ErrNotFound
+	json.NewDecoder(w.Body).Decode(got)
+	if diff := cmp.Diff(got, want); len(diff) != 0 {
+		t.Errorf(diff)
+	}
+}

mock/mock.go

@@ -6,4 +6,4 @@
 
 package mock
 
-//go:generate mockgen -package=mock -destination=mock_gen.go github.com/drone/drone/core Pubsub,Canceler,ConvertService,ValidateService,NetrcService,Renewer,HookParser,UserService,RepositoryService,CommitService,StatusService,HookService,FileService,Batcher,BuildStore,CronStore,LogStore,PermStore,SecretStore,GlobalSecretStore,StageStore,StepStore,RepositoryStore,UserStore,Scheduler,Session,OrganizationService,SecretService,RegistryService,ConfigService,Transferer,Triggerer,Syncer,LogStream,WebhookSender,LicenseService
+//go:generate mockgen -package=mock -destination=mock_gen.go github.com/drone/drone/core Pubsub,Canceler,ConvertService,ValidateService,NetrcService,Renewer,HookParser,UserService,RepositoryService,CommitService,StatusService,HookService,FileService,Batcher,BuildStore,CronStore,LogStore,PermStore,SecretStore,GlobalSecretStore,StageStore,StepStore,RepositoryStore,UserStore,Scheduler,Session,OrganizationService,SecretService,RegistryService,ConfigService,Transferer,Triggerer,Syncer,LogStream,WebhookSender,LicenseService,TemplateStore