fix #683 (comment)

HISTORY.md

@@ -17,6 +17,10 @@ Developers are not forced to upgrade if they don't really need it. Upgrade whene
 
 **How to upgrade**: Open your command-line and execute this command: `go get -u github.com/kataras/iris`.
 
+# Sa, 23 July 2017 | v8.0.7
+
+Fix [It's true that with UseGlobal the "/path1.txt" route call the middleware but cause the prepend, the order is inversed](https://github.com/kataras/iris/issues/683#issuecomment-317229068)
+
 # Sa, 22 July 2017 | v8.0.5 & v8.0.6
 
 No API Changes.

README.md

@@ -17,7 +17,7 @@ Iris is a fast, simple and efficient micro web framework for Go. It provides a b
 ### 📑 Table of contents
 
 * [Installation](#-installation)
-* [Latest changes](https://github.com/kataras/iris/blob/master/HISTORY.md#sa-22-july-2017--v805--v806)
+* [Latest changes](https://github.com/kataras/iris/blob/master/HISTORY.md#sa-23-july-2017--v807)
 * [Learn](#-learn)
 	* [HTTP Listening](_examples/#http-listening)
 	* [Configuration](_examples/#configuration)
@@ -339,7 +339,7 @@ Thank You for your trust!
 
 ### 📌 Version
 
-Current: **8.0.6**
+Current: **8.0.7**
 
 Each new release is pushed to the master. It stays there until the next version. When a next version is released then the previous version goes to its own branch with `gopkg.in` as its import path (and its own vendor folder), in order to keep it working "for-ever".
 

_examples/file-server/single-page-application/basic/main_test.go

@@ -47,7 +47,7 @@ var urls = []resource{
 
 func TestSPA(t *testing.T) {
 	app := newApp()
-	e := httptest.New(t, app, httptest.Debug(true))
+	e := httptest.New(t, app, httptest.Debug(false))
 
 	for _, u := range urls {
 		url := u.String()

core/router/api_builder.go

@@ -76,12 +76,20 @@ type APIBuilder struct {
 	// to the end-user.
 	reporter *errors.Reporter
 
-	// the per-party middleware
+	// the per-party handlers, order
+	// of handlers registration matters.
 	middleware context.Handlers
-	// the per-party routes (useful only for done middleware)
+	// the global middleware handlers, order of call doesn't matters, order
+	// of handlers registration matters. We need a secondary field for this
+	// because `UseGlobal` registers handlers that should be executed
+	// even before the `middleware` handlers, and in the same time keep the order
+	// of handlers registration, so the same type of handlers are being called in order.
+	beginGlobalHandlers context.Handlers
+	// the per-party routes registry (useful for `Done` and `UseGlobal` only)
 	apiRoutes []*Route
-	// the per-party done middleware
-	doneHandlers context.Handlers
+	// the per-party done handlers, order
+	// of handlers registration matters.
+	doneGlobalHandlers context.Handlers
 	// the per-party
 	relativePath string
 }
@@ -138,13 +146,16 @@ func (rb *APIBuilder) Handle(method string, registeredPath string, handlers ...c
 
 	fullpath := rb.relativePath + registeredPath // for now, keep the last "/" if any,  "/xyz/"
 
-	routeHandlers := joinHandlers(rb.middleware, handlers)
+	// global begin handlers -> middleware that are registered before route registration
+	// -> handlers that are passed to this Handle function.
+	routeHandlers := joinHandlers(append(rb.beginGlobalHandlers, rb.middleware...), handlers)
+	// -> done handlers after all
+	if len(rb.doneGlobalHandlers) > 0 {
+		routeHandlers = append(routeHandlers, rb.doneGlobalHandlers...) // register the done middleware, if any
+	}
 
 	// here we separate the subdomain and relative path
 	subdomain, path := splitSubdomainAndPath(fullpath)
-	if len(rb.doneHandlers) > 0 {
-		routeHandlers = append(routeHandlers, rb.doneHandlers...) // register the done middleware, if any
-	}
 
 	r, err := NewRoute(method, subdomain, path, routeHandlers, rb.macros)
 	if err != nil { // template path parser errors:
@@ -187,16 +198,17 @@ func (rb *APIBuilder) Party(relativePath string, handlers ...context.Handler) Pa
 	}
 
 	fullpath := parentPath + relativePath
-	// append the parent's +child's handlers
+	// append the parent's + child's handlers
 	middleware := joinHandlers(rb.middleware, handlers)
 
 	return &APIBuilder{
 		// global/api builder
-		macros:            rb.macros,
-		routes:            rb.routes,
-		errorCodeHandlers: rb.errorCodeHandlers,
-		doneHandlers:      rb.doneHandlers,
-		reporter:          rb.reporter,
+		macros:              rb.macros,
+		routes:              rb.routes,
+		errorCodeHandlers:   rb.errorCodeHandlers,
+		beginGlobalHandlers: rb.beginGlobalHandlers,
+		doneGlobalHandlers:  rb.doneGlobalHandlers,
+		reporter:            rb.reporter,
 		// per-party/children
 		middleware:   middleware,
 		relativePath: fullpath,
@@ -256,34 +268,39 @@ func (rb *APIBuilder) GetRoute(routeName string) *Route {
 
 // Use appends Handler(s) to the current Party's routes and child routes.
 // If the current Party is the root, then it registers the middleware to all child Parties' routes too.
+//
+// Call order matters, it should be called right before the routes that they care about these handlers.
+//
+// If it's called after the routes then these handlers will never be executed.
+// Use `UseGlobal` if you want to register begin handlers(middleware)
+// that should be always run before all application's routes.
 func (rb *APIBuilder) Use(middleware ...context.Handler) {
 	rb.middleware = append(rb.middleware, middleware...)
 }
 
 // Done appends to the very end, Handler(s) to the current Party's routes and child routes
 // The difference from .Use is that this/or these Handler(s) are being always running last.
 func (rb *APIBuilder) Done(handlers ...context.Handler) {
-	if len(rb.apiRoutes) > 0 { // register these middleware on previous-party-defined routes, it called after the party's route methods (Handle/HandleFunc/Get/Post/Put/Delete/...)
-		for i, n := 0, len(rb.apiRoutes); i < n; i++ {
-			routeInfo := rb.apiRoutes[i]
-			routeInfo.Handlers = append(routeInfo.Handlers, handlers...)
-		}
-	} else {
-		// register them on the doneHandlers, which will be used on Handle to append these middlweare as the last handler(s)
-		rb.doneHandlers = append(rb.doneHandlers, handlers...)
+	for _, r := range rb.routes.routes {
+		r.done(handlers) // append the handlers to the existing routes
 	}
+	// set as done handlers for the next routes as well.
+	rb.doneGlobalHandlers = append(rb.doneGlobalHandlers, handlers...)
 }
 
-// UseGlobal registers Handler middleware  to the beginning, prepends them instead of append
+// UseGlobal registers handlers that should run before all routes,
+// including all parties, subdomains
+// and other middleware that were registered before or will be after.
+// It doesn't care about call order, it will prepend the handlers to all
+// existing routes and the future routes that may being registered.
 //
-// Use it when you want to add a global middleware to all parties, to all routes in  all subdomains
-// It should be called right before Listen functions
+// It's always a good practise to call it right before the `Application#Run` function.
 func (rb *APIBuilder) UseGlobal(handlers ...context.Handler) {
 	for _, r := range rb.routes.routes {
-		r.Handlers = append(handlers, r.Handlers...) // prepend the handlers
+		r.use(handlers) // prepend the handlers to the existing routes
 	}
-	rb.middleware = append(handlers, rb.middleware...) // set as middleware on the next routes too
-	// rb.Use(handlers...)
+	// set as begin handlers for the next routes as well.
+	rb.beginGlobalHandlers = append(rb.beginGlobalHandlers, handlers...)
 }
 
 // None registers an "offline" route

core/router/handler.go

@@ -114,6 +114,9 @@ func (h *routerHandler) Build(provider RoutesProvider) error {
 	rp := errors.NewReporter()
 
 	for _, r := range registeredRoutes {
+		// build the r.Handlers based on begin and done handlers, if any.
+		r.BuildHandlers()
+
 		if r.Subdomain != "" {
 			h.hosts = true
 		}

core/router/route.go

@@ -17,7 +17,15 @@ type Route struct {
 	Subdomain string          // "admin."
 	tmpl      *macro.Template // Tmpl().Src: "/api/user/{id:int}"
 	Path      string          // "/api/user/:id"
-	Handlers  context.Handlers
+	// temp storage, they're appended to the Handlers on build.
+	// Execution happens before Handlers, can be empty.
+	beginHandlers context.Handlers
+	// Handlers are the main route's handlers, executed by order.
+	// Cannot be empty.
+	Handlers context.Handlers
+	// temp storage, they're appended to the Handlers on build.
+	// Execution happens after Begin and main Handler(s), can be empty.
+	doneHandlers context.Handlers
 	// FormattedPath all dynamic named parameters (if any) replaced with %v,
 	// used by Application to validate param values of a Route based on its name.
 	FormattedPath string
@@ -57,6 +65,47 @@ func NewRoute(method, subdomain, unparsedPath string,
 	return route, nil
 }
 
+// use adds explicit begin handlers(middleware) to this route,
+// It's being called internally, it's useless for outsiders
+// because `Handlers` field is exported.
+// The callers of this function are: `APIBuilder#UseGlobal` and `APIBuilder#Done`.
+//
+// BuildHandlers should be called to build the route's `Handlers`.
+func (r *Route) use(handlers context.Handlers) {
+	if len(handlers) == 0 {
+		return
+	}
+	r.beginHandlers = append(r.beginHandlers, handlers...)
+}
+
+// use adds explicit done handlers to this route.
+// It's being called internally, it's useless for outsiders
+// because `Handlers` field is exported.
+// The callers of this function are: `APIBuilder#UseGlobal` and `APIBuilder#Done`.
+//
+// BuildHandlers should be called to build the route's `Handlers`.
+func (r *Route) done(handlers context.Handlers) {
+	if len(handlers) == 0 {
+		return
+	}
+	r.doneHandlers = append(r.doneHandlers, handlers...)
+}
+
+// BuildHandlers is executed automatically by the router handler
+// at the `Application#Build` state. Do not call it manually, unless
+// you were defined your own request mux handler.
+func (r *Route) BuildHandlers() {
+	if len(r.beginHandlers) > 0 {
+		r.Handlers = append(r.beginHandlers, r.Handlers...)
+		r.beginHandlers = r.beginHandlers[0:0]
+	}
+
+	if len(r.doneHandlers) > 0 {
+		r.Handlers = append(r.Handlers, r.doneHandlers...)
+		r.doneHandlers = r.doneHandlers[0:0]
+	} // note: no mutex needed, this should be called in-sync when server is not running of course.
+}
+
 // String returns the form of METHOD, SUBDOMAIN, TMPL PATH
 func (r Route) String() string {
 	return fmt.Sprintf("%s %s%s",

core/router/router_handlers_order_test.go

@@ -0,0 +1,147 @@
+// black-box testing
+//
+// see _examples/routing/main_test.go for the most common router tests that you may want to see,
+// this is a test which makes sure that the APIBuilder's `UseGlobal`, `Use` and `Done` functions are
+// working as expected.
+
+package router_test
+
+import (
+	"testing"
+
+	"github.com/kataras/iris"
+	"github.com/kataras/iris/context"
+
+	"github.com/kataras/iris/httptest"
+)
+
+// test registering of below handlers
+// with a different order but the route's final
+// response should be the same at all cases.
+var (
+	mainResponse = "main"
+	mainHandler  = func(ctx context.Context) {
+		ctx.WriteString(mainResponse)
+		ctx.Next()
+	}
+
+	firstUseResponse = "use1"
+	firstUseHandler  = func(ctx context.Context) {
+		ctx.WriteString(firstUseResponse)
+		ctx.Next()
+	}
+
+	secondUseResponse = "use2"
+	secondUseHandler  = func(ctx context.Context) {
+		ctx.WriteString(secondUseResponse)
+		ctx.Next()
+	}
+
+	firstUseGlobalResponse = "useglobal1"
+	firstUseGlobalHandler  = func(ctx context.Context) {
+		ctx.WriteString(firstUseGlobalResponse)
+		ctx.Next()
+	}
+
+	secondUseGlobalResponse = "useglobal2"
+	secondUseGlobalHandler  = func(ctx context.Context) {
+		ctx.WriteString(secondUseGlobalResponse)
+		ctx.Next()
+	}
+
+	firstDoneResponse = "done1"
+	firstDoneHandler  = func(ctx context.Context) {
+		ctx.WriteString(firstDoneResponse)
+		ctx.Next()
+	}
+
+	secondDoneResponse = "done2"
+	secondDoneHandler  = func(ctx context.Context) {
+		ctx.WriteString(secondDoneResponse)
+	}
+
+	finalResponse = firstUseGlobalResponse + secondUseGlobalResponse +
+		firstUseResponse + secondUseResponse + mainResponse + firstDoneResponse + secondDoneResponse
+
+	testResponse = func(t *testing.T, app *iris.Application, path string) {
+		e := httptest.New(t, app)
+		e.GET(path).Expect().Status(httptest.StatusOK).Body().Equal(finalResponse)
+	}
+)
+
+func TestMiddlewareByRouteDef(t *testing.T) {
+	app := iris.New()
+	app.Get("/mypath", firstUseGlobalHandler, secondUseGlobalHandler, firstUseHandler, secondUseHandler,
+		mainHandler, firstDoneHandler, secondDoneHandler)
+
+	testResponse(t, app, "/mypath")
+}
+func TestMiddlewareByUseAndDoneDef(t *testing.T) {
+	app := iris.New()
+	app.Use(firstUseGlobalHandler, secondUseGlobalHandler, firstUseHandler, secondUseHandler)
+	app.Done(firstDoneHandler, secondDoneHandler)
+
+	app.Get("/mypath", mainHandler)
+
+	testResponse(t, app, "/mypath")
+}
+
+func TestMiddlewareByUseUseGlobalAndDoneDef(t *testing.T) {
+	app := iris.New()
+	app.Use(firstUseHandler, secondUseHandler)
+	// if failed then UseGlobal didnt' registered these handlers even before the
+	// existing middleware.
+	app.UseGlobal(firstUseGlobalHandler, secondUseGlobalHandler)
+	app.Done(firstDoneHandler, secondDoneHandler)
+
+	app.Get("/mypath", mainHandler)
+
+	testResponse(t, app, "/mypath")
+}
+
+func TestMiddlewareByUseDoneAndUseGlobalDef(t *testing.T) {
+	app := iris.New()
+
+	app.Use(firstUseHandler, secondUseHandler)
+	app.Done(firstDoneHandler, secondDoneHandler)
+
+	app.Get("/mypath", mainHandler)
+
+	// if failed then UseGlobal was unable to
+	// prepend these handlers to the route was registered before
+	// OR
+	// when order failed because these should be executed in order, first the firstUseGlobalHandler,
+	// because they are the same type (global begin handlers)
+	app.UseGlobal(firstUseGlobalHandler)
+	app.UseGlobal(secondUseGlobalHandler)
+
+	testResponse(t, app, "/mypath")
+}
+
+func TestMiddlewareByUseGlobalUseAndDoneDef(t *testing.T) {
+	app := iris.New()
+
+	app.UseGlobal(firstUseGlobalHandler)
+	app.UseGlobal(secondUseGlobalHandler)
+	app.Use(firstUseHandler, secondUseHandler)
+
+	app.Get("/mypath", mainHandler)
+
+	app.Done(firstDoneHandler, secondDoneHandler)
+
+	testResponse(t, app, "/mypath")
+}
+
+func TestMiddlewareByDoneUseAndUseGlobalDef(t *testing.T) {
+	app := iris.New()
+	app.Done(firstDoneHandler, secondDoneHandler)
+
+	app.Use(firstUseHandler, secondUseHandler)
+
+	app.Get("/mypath", mainHandler)
+
+	app.UseGlobal(firstUseGlobalHandler)
+	app.UseGlobal(secondUseGlobalHandler)
+
+	testResponse(t, app, "/mypath")
+}