Repository: nuveo/prest
Pull request #120 (2017-03-03 22:06:01)

possibility to reorder middlewares
#119 

codecov[bot]: # [Codecov](https://codecov.io/gh/nuveo/prest/pull/120?src=pr&el=h1) Report
> Merging [#120](https://codecov.io/gh/nuveo/prest/pull/120?src=pr&el=desc) into [master](https://codecov.io/gh/nuveo/prest/commit/44ec08a809793cf04a1647cf202de08a5335062b?src=pr&el=desc) will **decrease** coverage by `-0.19%`.
> The diff coverage is `83.33%`.


```diff
@@            Coverage Diff             @@
##           master     #120      +/-   ##
==========================================
- Coverage   89.71%   89.53%   -0.19%     
==========================================
  Files          10       10              
  Lines         817      822       +5     
==========================================
+ Hits          733      736       +3     
- Misses         54       55       +1     
- Partials       30       31       +1
```


| [Impacted Files](https://codecov.io/gh/nuveo/prest/pull/120?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [config/router/routers.go](https://codecov.io/gh/nuveo/prest/compare/44ec08a809793cf04a1647cf202de08a5335062b...d537e77cb3664dd8d01a88fd7c9d7e9096e205ec?src=pr&el=tree#diff-Y29uZmlnL3JvdXRlci9yb3V0ZXJzLmdv) | `100% <100%> (ø)` | |
| [config/middlewares/middlewares.go](https://codecov.io/gh/nuveo/prest/compare/44ec08a809793cf04a1647cf202de08a5335062b...d537e77cb3664dd8d01a88fd7c9d7e9096e205ec?src=pr&el=tree#diff-Y29uZmlnL21pZGRsZXdhcmVzL21pZGRsZXdhcmVzLmdv) | `81.81% <81.81%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/nuveo/prest/pull/120?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/nuveo/prest/pull/120?src=pr&el=footer). Last update [44ec08a...d537e77](https://codecov.io/gh/nuveo/prest/compare/44ec08a809793cf04a1647cf202de08a5335062b...d537e77cb3664dd8d01a88fd7c9d7e9096e205ec?src=pr&el=footer&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Patch:
diff --git a/CREATING_MODULES.md b/CREATING_MODULES.md
index 3213ead..62c6a8d 100644
--- a/CREATING_MODULES.md
+++ b/CREATING_MODULES.md
@@ -15,18 +15,24 @@ import (
 	"log"
 
 	"github.com/nuveo/prest/cmd"
-	"github.com/nuveo/prest/config"
+	"github.com/nuveo/prest/config/router"
+	"github.com/nuveo/prest/config/middlewares"
+	"github.com/urfave/negroni"
 )
 
 func main() {
-	// Get pREST app
-	n := config.GetApp()
+	// Reorder middlewares
+	middlewares.MiddlewareStack = []negroni.Handler{
+		negroni.Handler(negroni.NewRecovery()),
+		negroni.Handler(negroni.NewRecovery()),
+		negroni.Handler(negroni.HandlerFunc(CustomMiddleware)),
+	}
 
-	// Register custom middleware
-	n.Use(negroni.HandlerFunc(CustomMiddleware))
+	// Get pREST app
+	n := middlewares.GetApp()
 
 	// Get pPREST router
-	r := config.GetRouter()
+	r := router.Get()
 
 	// Register custom routes
 	r.HandleFunc("/ping", Pong).Methods("GET")
diff --git a/cmd/root.go b/cmd/root.go
index c148b7b..9d0e6d1 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -10,6 +10,8 @@ import (
 	// postgres driver for migrate
 	_ "github.com/mattes/migrate/driver/postgres"
 	"github.com/nuveo/prest/config"
+	cfgMiddleware "github.com/nuveo/prest/config/middlewares"
+	"github.com/nuveo/prest/config/router"
 	"github.com/nuveo/prest/controllers"
 	"github.com/nuveo/prest/middlewares"
 	"github.com/spf13/cobra"
@@ -50,13 +52,9 @@ func app() {
 		log.Fatalf("Error parsing conf: %s", err)
 	}
 
-	n := config.GetApp()
-	n.Use(negroni.HandlerFunc(middlewares.HandlerSet))
-	if cfg.JWTKey != "" {
-		n.Use(middlewares.JwtMiddleware(cfg.JWTKey))
-	}
+	n := cfgMiddleware.GetApp()
 
-	r := config.GetRouter()
+	r := router.Get()
 
 	r.HandleFunc("/databases", controllers.GetDatabases).Methods("GET")
 	r.HandleFunc("/schemas", controllers.GetSchemas).Methods("GET")
@@ -72,7 +70,7 @@ func app() {
 	crudRoutes.HandleFunc("/{database}/{schema}/{table}", controllers.UpdateTable).Methods("PUT", "PATCH")
 
 	r.PathPrefix("/").Handler(negroni.New(
-		negroni.HandlerFunc(middlewares.AccessControl),
+		middlewares.AccessControl(),
 		negroni.Wrap(crudRoutes),
 	))
 
diff --git a/config/middlewares.go b/config/middlewares.go
deleted file mode 100644
index d3c7fd3..0000000
--- a/config/middlewares.go
+++ /dev/null
@@ -1,17 +0,0 @@
-package config
-
-import "github.com/urfave/negroni"
-
-var app *negroni.Negroni
-
-func initApp() {
-	app = negroni.Classic()
-}
-
-// GetApp get negroni
-func GetApp() *negroni.Negroni {
-	if app == nil {
-		initApp()
-	}
-	return app
-}
diff --git a/config/middlewares/middlewares.go b/config/middlewares/middlewares.go
new file mode 100644
index 0000000..e5ac6cd
--- /dev/null
+++ b/config/middlewares/middlewares.go
@@ -0,0 +1,41 @@
+package middlewares
+
+import (
+	"github.com/nuveo/prest/config"
+	"github.com/nuveo/prest/middlewares"
+	"github.com/urfave/negroni"
+)
+
+var (
+	app *negroni.Negroni
+
+	// MiddlewareStack on pREST
+	MiddlewareStack []negroni.Handler
+
+	// BaseStack Middlewares
+	BaseStack = []negroni.Handler{
+		negroni.Handler(negroni.NewRecovery()),
+		negroni.Handler(negroni.NewRecovery()),
+		negroni.Handler(middlewares.AccessControl()),
+		negroni.Handler(middlewares.HandlerSet()),
+	}
+)
+
+func initApp() {
+	config.InitConf()
+	if len(MiddlewareStack) == 0 {
+		MiddlewareStack = append(MiddlewareStack, BaseStack...)
+	}
+	if config.PrestConf.JWTKey != "" {
+		MiddlewareStack = append(MiddlewareStack, negroni.Handler(middlewares.JwtMiddleware(config.PrestConf.JWTKey)))
+	}
+	app = negroni.New(MiddlewareStack...)
+}
+
+// GetApp get negroni
+func GetApp() *negroni.Negroni {
+	if app == nil {
+		initApp()
+	}
+	return app
+}
diff --git a/config/routers.go b/config/router/routers.go
similarity index 70%
rename from config/routers.go
rename to config/router/routers.go
index e278075..8982671 100644
--- a/config/routers.go
+++ b/config/router/routers.go
@@ -1,4 +1,4 @@
-package config
+package router
 
 import "github.com/gorilla/mux"
 
@@ -8,8 +8,8 @@ func initRouter() {
 	router = mux.NewRouter()
 }
 
-// GetRouter for pREST
-func GetRouter() *mux.Router {
+// Get Router for pREST
+func Get() *mux.Router {
 	if router == nil {
 		initRouter()
 	}
diff --git a/middlewares/middlewares.go b/middlewares/middlewares.go
index 6c97944..5ba9dd3 100644
--- a/middlewares/middlewares.go
+++ b/middlewares/middlewares.go
@@ -12,32 +12,36 @@ import (
 )
 
 // HandlerSet add content type header
-func HandlerSet(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
-	w.Header().Set("Content-Type", "application/json")
-	next(w, r)
+func HandlerSet() negroni.Handler {
+	return negroni.HandlerFunc(func(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
+		w.Header().Set("Content-Type", "application/json")
+		next(w, r)
+	})
 }
 
 // AccessControl is a middleware to handle permissions on tables in pREST
-func AccessControl(rw http.ResponseWriter, rq *http.Request, next http.HandlerFunc) {
-	mapPath := getVars(rq.URL.Path)
-	if mapPath == nil {
-		next(rw, rq)
-		return
-	}
-
-	permission := permissionByMethod(rq.Method)
-	if permission == "" {
-		next(rw, rq)
-		return
-	}
-
-	if postgres.TablePermissions(mapPath["table"], permission) {
-		next(rw, rq)
-		return
-	}
-
-	err := fmt.Errorf("required authorization to table %s", mapPath["table"])
-	http.Error(rw, err.Error(), http.StatusUnauthorized)
+func AccessControl() negroni.Handler {
+	return negroni.HandlerFunc(func(rw http.ResponseWriter, rq *http.Request, next http.HandlerFunc) {
+		mapPath := getVars(rq.URL.Path)
+		if mapPath == nil {
+			next(rw, rq)
+			return
+		}
+
+		permission := permissionByMethod(rq.Method)
+		if permission == "" {
+			next(rw, rq)
+			return
+		}
+
+		if postgres.TablePermissions(mapPath["table"], permission) {
+			next(rw, rq)
+			return
+		}
+
+		err := fmt.Errorf("required authorization to table %s", mapPath["table"])
+		http.Error(rw, err.Error(), http.StatusUnauthorized)
+	})
 }
 
 // JwtMiddleware check if actual request have JWT


Test patch:
diff --git a/config/middlewares/middlewares_test.go b/config/middlewares/middlewares_test.go
new file mode 100644
index 0000000..25b34d6
--- /dev/null
+++ b/config/middlewares/middlewares_test.go
@@ -0,0 +1,82 @@
+package middlewares
+
+import (
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/gorilla/mux"
+	"github.com/urfave/negroni"
+)
+
+func TestInitApp(t *testing.T) {
+	app = nil
+	initApp()
+	if app == nil {
+		t.Errorf("App should not be nil.")
+	}
+	MiddlewareStack = []negroni.Handler{}
+}
+
+func TestGetApp(t *testing.T) {
+	app = nil
+	n := GetApp()
+	if n == nil {
+		t.Errorf("Should return an app.")
+	}
+	MiddlewareStack = []negroni.Handler{}
+}
+
+func TestGetAppWithReorderedMiddleware(t *testing.T) {
+	app = nil
+	MiddlewareStack = []negroni.Handler{
+		negroni.Handler(negroni.HandlerFunc(customMiddleware)),
+	}
+	r := mux.NewRouter()
+	r.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {})
+	n := GetApp()
+	n.UseHandler(r)
+	server := httptest.NewServer(n)
+	defer server.Close()
+	resp, err := http.Get(server.URL)
+	if err != nil {
+		t.Fatal("Expected run without errors but was", err.Error())
+	}
+	body, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		t.Fatal("Expected run without errors but was", err.Error())
+	}
+	defer resp.Body.Close()
+	if !strings.Contains(string(body), "Calling custom middleware") {
+		t.Error("do not contains 'Calling custom middleware'")
+	}
+	if strings.Contains(resp.Header.Get("Content-Type"), "application/json") {
+		t.Error("content type shouldn't be application/json but was")
+	}
+	MiddlewareStack = []negroni.Handler{}
+}
+
+func TestGetAppWithoutReorderedMiddleware(t *testing.T) {
+	app = nil
+	r := mux.NewRouter()
+	r.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {})
+	n := GetApp()
+	n.UseHandler(r)
+	server := httptest.NewServer(n)
+	defer server.Close()
+	resp, err := http.Get(server.URL)
+	if err != nil {
+		t.Fatal("Expected run without errors but was", err.Error())
+	}
+	if !strings.Contains(resp.Header.Get("Content-Type"), "application/json") {
+		t.Error("content type should be application/json but not was", resp.Header.Get("Content-Type"))
+	}
+	MiddlewareStack = []negroni.Handler{}
+}
+
+func customMiddleware(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
+	w.Write([]byte("Calling custom middleware"))
+	next(w, r)
+}
diff --git a/config/middlewares_test.go b/config/middlewares_test.go
deleted file mode 100644
index eb0646e..0000000
--- a/config/middlewares_test.go
+++ /dev/null
@@ -1,18 +0,0 @@
-package config
-
-import "testing"
-
-func TestInitApp(t *testing.T) {
-	initApp()
-	if app == nil {
-		t.Errorf("App should not be nil.")
-	}
-}
-
-func TestGetApp(t *testing.T) {
-	app = nil
-	n := GetApp()
-	if n == nil {
-		t.Errorf("Should return an app.")
-	}
-}
diff --git a/config/routers_test.go b/config/router/routers_test.go
similarity index 75%
rename from config/routers_test.go
rename to config/router/routers_test.go
index e5c32a8..9619273 100644
--- a/config/routers_test.go
+++ b/config/router/routers_test.go
@@ -1,4 +1,4 @@
-package config
+package router
 
 import "testing"
 
@@ -9,9 +9,9 @@ func TestInitRouter(t *testing.T) {
 	}
 }
 
-func TestGetRouter(t *testing.T) {
+func TestGet(t *testing.T) {
 	router = nil
-	r := GetRouter()
+	r := Get()
 	if r == nil {
 		t.Errorf("Should return a router.")
 	}