From a6ec94e1a695828a8bf12c536e82d40112c68a4a Mon Sep 17 00:00:00 2001 From: "Gerasimos (Makis) Maropoulos" Date: Fri, 28 Aug 2020 04:11:56 +0300 Subject: [PATCH] overlap routing: and mvc: allow setting status code from a dependency or a middleware --- .../mvc/authenticated-controller/main.go | 12 +- context/context.go | 14 +++ core/router/api_builder.go | 29 ++++- core/router/party.go | 2 +- core/router/route_register_rule_test.go | 13 +- hero/func_result.go | 2 +- mvc/controller_overlap_test.go | 111 ++++++++++++++++++ mvc/controller_test.go | 49 -------- versioning/version.go | 4 +- versioning/versioning.go | 8 +- 10 files changed, 181 insertions(+), 63 deletions(-) create mode 100644 mvc/controller_overlap_test.go diff --git a/_examples/mvc/authenticated-controller/main.go b/_examples/mvc/authenticated-controller/main.go index 25a27955..f925ad30 100644 --- a/_examples/mvc/authenticated-controller/main.go +++ b/_examples/mvc/authenticated-controller/main.go @@ -63,7 +63,17 @@ func authDependency(ctx iris.Context, session *sessions.Session) Authenticated { if userID == 0 { // If execution was stopped // any controller's method will not be executed at all. - ctx.StopWithStatus(iris.StatusUnauthorized) + // + // Note that, the below will not fire the error to the user: + // ctx.StopWithStatus(iris.StatusUnauthorized) + // because of the imaginary: + // UnauthenticatedUserController.Get() (string, int) { + // return "...", iris.StatusOK + // } + // + // OR + // If you don't want to set a status code at all: + ctx.StopExecution() return 0 } diff --git a/context/context.go b/context/context.go index ccf8a774..00071bc3 100644 --- a/context/context.go +++ b/context/context.go @@ -1474,6 +1474,20 @@ func (ctx *Context) URLParamInt64Default(name string, def int64) int64 { return v } +// URLParamUint64 returns the url query parameter as uint64 value from a request. +// Returns 0 on parse errors or when the URL parameter does not exist in the Query. +func (ctx *Context) URLParamUint64(name string) uint64 { + if v := ctx.URLParam(name); v != "" { + n, err := strconv.ParseUint(v, 10, 64) + if err != nil { + return 0 + } + return n + } + + return 0 +} + // URLParamFloat64 returns the url query parameter as float64 value from a request, // returns an error and -1 if parse failed. func (ctx *Context) URLParamFloat64(name string) (float64, error) { diff --git a/core/router/api_builder.go b/core/router/api_builder.go index d682584e..5970f5ec 100644 --- a/core/router/api_builder.go +++ b/core/router/api_builder.go @@ -1,6 +1,7 @@ package router import ( + "errors" "fmt" "net/http" "os" @@ -143,8 +144,30 @@ func overlapRoute(r *Route, next *Route) { if !ctx.IsStopped() { return } - } else if !defaultOverlapFilter(ctx) { - return + } else { + prevStatusCode := ctx.GetStatusCode() + + if !defaultOverlapFilter(ctx) { + return + } + // set the status code that it was stopped with. + // useful for dependencies with StopWithStatus(XXX) + // instead of raw ctx.StopExecution(). + // The func_result now also catch the last registered status code + // of the chain, unless the controller returns an integer. + // See _examples/authenticated-controller. + if prevStatusCode > 0 { + // An exception when stored error + // exists and it's type of ErrNotFound. + // Example: + // Version was not found: + // we need to be able to send the status on the last not found version + // but reset the status code if a next available matched version was found. + // see: versioning.Handler. + if !errors.Is(ctx.GetErr(), context.ErrNotFound) { + ctx.StatusCode(prevStatusCode) + } + } } ctx.SetErr(nil) // clear any stored error. @@ -1051,7 +1074,7 @@ func (api *APIBuilder) Reset() Party { return api } -// ResetRouterFilters deactivates any pervious registered +// ResetRouterFilters deactivates any previous registered // router filters and the parents ones for this Party. // // Returns this Party. diff --git a/core/router/party.go b/core/router/party.go index 6bad7744..25915887 100644 --- a/core/router/party.go +++ b/core/router/party.go @@ -121,7 +121,7 @@ type Party interface { // // Returns this Party. Reset() Party - // ResetRouterFilters deactivates any pervious registered + // ResetRouterFilters deactivates any previous registered // router filters and the parents ones for this Party. // // Returns this Party. diff --git a/core/router/route_register_rule_test.go b/core/router/route_register_rule_test.go index 85668ddd..8beb27cb 100644 --- a/core/router/route_register_rule_test.go +++ b/core/router/route_register_rule_test.go @@ -1,6 +1,7 @@ package router_test import ( + "bytes" "reflect" "testing" @@ -11,6 +12,10 @@ import ( func TestRegisterRule(t *testing.T) { app := iris.New() + // collect the error on RouteError rule. + buf := new(bytes.Buffer) + app.Logger().SetTimeFormat("").DisableNewLine().SetOutput(buf) + v1 := app.Party("/v1") v1.SetRegisterRule(iris.RouteSkip) @@ -43,9 +48,9 @@ func TestRegisterRule(t *testing.T) { if route := v1.Get("/", getHandler); route != nil { t.Fatalf("expected duplicated route, with RouteError rule, to be nil but got: %#+v", route) } - // if expected, got := 1, len(v1.GetReporter().Errors); expected != got { - // t.Fatalf("expected api builder's errors length to be: %d but got: %d", expected, got) - // } + if expected, got := "[ERRO] new route: GET /v1 conflicts with an already registered one: GET /v1 route", buf.String(); expected != got { + t.Fatalf("expected api builder's error to be:\n'%s'\nbut got:\n'%s'", expected, got) + } } func testRegisterRule(e *httptest.Expect, expectedGetBody string) { @@ -73,6 +78,7 @@ func TestRegisterRuleOverlap(t *testing.T) { ctx.StopWithStatus(iris.StatusUnauthorized) }) usersRouter.Get("/", func(ctx iris.Context) { + ctx.StatusCode(iris.StatusOK) ctx.WriteString("data") }) @@ -99,6 +105,7 @@ func TestRegisterRuleOverlap(t *testing.T) { ctx.StopWithText(iris.StatusUnauthorized, "no access") }) usersRouter.Get("/p3", func(ctx iris.Context) { + ctx.StatusCode(iris.StatusOK) ctx.WriteString("p3 data") }) diff --git a/hero/func_result.go b/hero/func_result.go index 964cb6fe..e2b093e4 100644 --- a/hero/func_result.go +++ b/hero/func_result.go @@ -176,7 +176,7 @@ func dispatchFuncResult(ctx *context.Context, values []reflect.Value, handler Re // Except when err != nil then check if status code is < 400 and // if it's set it as DefaultErrStatusCode. // Except when found == false, then the status code is 404. - statusCode int + statusCode = ctx.GetStatusCode() // Get the current status code given by any previous middleware. // if not empty then use that as content type, // if empty and custom != nil then set it to application/json. contentType string diff --git a/mvc/controller_overlap_test.go b/mvc/controller_overlap_test.go new file mode 100644 index 00000000..d7897fc4 --- /dev/null +++ b/mvc/controller_overlap_test.go @@ -0,0 +1,111 @@ +// black-box testing +// Note: there is a test, for end-devs, of Controllers overlapping at _examples/mvc/authenticated-controller too. +package mvc_test + +import ( + "fmt" + "testing" + + "github.com/kataras/iris/v12" + "github.com/kataras/iris/v12/httptest" + "github.com/kataras/iris/v12/mvc" +) + +func TestControllerOverlap(t *testing.T) { + app := iris.New() + userRouter := app.Party("/user") + { + userRouter.SetRegisterRule(iris.RouteOverlap) + + // Initialize a new MVC application on top of the "userRouter". + userApp := mvc.New(userRouter) + // Register Dependencies. + userApp.Register(authDependency) + + // Register Controllers. + userApp.Handle(new(AuthenticatedUserController)) + userApp.Handle(new(UnauthenticatedUserController)) + } + + e := httptest.New(t, app) + e.GET("/user").Expect().Status(httptest.StatusUnauthorized).Body().Equal("unauth") + // Test raw stop execution with a status code sent on the controller's method. + e.GET("/user/with/status/on/method").Expect().Status(httptest.StatusBadRequest).Body().Equal("unauth") + // Test stop execution with status but last code sent through the controller's method. + e.GET("/user/with/status/on/method/too").Expect().Status(httptest.StatusInternalServerError).Body().Equal("unauth") + // Test raw stop execution and no status code sent on controller's method (should be OK). + e.GET("/user/with/no/status").Expect().Status(httptest.StatusOK).Body().Equal("unauth") + + // Test authenticated request. + e.GET("/user").WithQuery("id", 42).Expect().Status(httptest.StatusOK).Body().Equal("auth: 42") + + // Test HandleHTTPError method accepts a not found and returns a 404 + // from a shared controller and overlapped, the url parameter matters because this method was overlapped. + e.GET("/user/notfound").Expect().Status(httptest.StatusBadRequest). + Body().Equal("error: *mvc_test.UnauthenticatedUserController: from: 404 to: 400") + e.GET("/user/notfound").WithQuery("id", 42).Expect().Status(httptest.StatusBadRequest). + Body().Equal("error: *mvc_test.AuthenticatedUserController: from: 404 to: 400") +} + +type AuthenticatedTest uint64 + +func authDependency(ctx iris.Context) AuthenticatedTest { + // this will be executed on not found too and that's what we expect. + + userID := ctx.URLParamUint64("id") // just for the test. + if userID == 0 { + if ctx.GetStatusCode() == iris.StatusNotFound || // do not send 401 on not founds, keep 404 and let controller decide. + ctx.Path() == "/user/with/status/on/method" || ctx.Path() == "/user/with/np/status" { // leave controller method decide, raw stop execution. + ctx.StopExecution() + } else { + ctx.StopWithStatus(iris.StatusUnauthorized) + } + + return 0 + } + + return AuthenticatedTest(userID) +} + +type BaseControllerTest struct{} + +func (c *BaseControllerTest) HandleHTTPError(ctx iris.Context, code mvc.Code) (string, int) { + if ctx.GetStatusCode() != int(code) { + // should never happen. + panic("Context current status code and given mvc code do not match!") + } + + ctrlName := ctx.Controller().Type().String() + newCode := 400 + return fmt.Sprintf("error: %s: from: %d to: %d", ctrlName, int(code), newCode), newCode +} + +type UnauthenticatedUserController struct { + BaseControllerTest +} + +func (c *UnauthenticatedUserController) Get() string { + return "unauth" +} + +func (c *UnauthenticatedUserController) GetWithNoStatus() string { + return "unauth" +} + +func (c *UnauthenticatedUserController) GetWithStatusOnMethod() (string, int) { + return "unauth", iris.StatusBadRequest +} + +func (c *UnauthenticatedUserController) GetWithStatusOnMethodToo() (string, int) { + return "unauth", iris.StatusInternalServerError +} + +type AuthenticatedUserController struct { + BaseControllerTest + + CurrentUserID AuthenticatedTest +} + +func (c *AuthenticatedUserController) Get() string { + return fmt.Sprintf("auth: %d", c.CurrentUserID) +} diff --git a/mvc/controller_test.go b/mvc/controller_test.go index 62233b35..99ab24f0 100644 --- a/mvc/controller_test.go +++ b/mvc/controller_test.go @@ -660,55 +660,6 @@ func TestApplicationDependency(t *testing.T) { e.GET("/other").Expect().Status(httptest.StatusOK).Body().Equal("app2") } -// Authenticated type. -type Authenticated int64 - -// BasePrivateController base controller for private controllers. -type BasePrivateController struct { - CurrentUserID Authenticated - Ctx iris.Context // not-used. -} - -type publicController struct { - Ctx iris.Context // not-used. -} - -func (c *publicController) Get() iris.Map { - return iris.Map{"data": "things"} -} - -type privateController struct{ BasePrivateController } - -func (c *privateController) Get() iris.Map { - return iris.Map{"id": c.CurrentUserID} -} - -func TestControllerOverlapping(t *testing.T) { - app := iris.New() - - m := New(app) - m.Router.SetRegisterRule(iris.RouteOverlap) - - m.Register(func(ctx iris.Context) Authenticated { - if ctx.URLParam("name") == "kataras" { - return 1 - } - - ctx.StopWithStatus(iris.StatusForbidden) - return -1 - }) - - // Order matters. - m.Handle(new(privateController)) - m.Handle(new(publicController)) - - e := httptest.New(t, app) - e.GET("/").WithQuery("name", "kataras").Expect().Status(httptest.StatusOK). - JSON().Equal(iris.Map{"id": 1}) - e.GET("/").Expect().Status(httptest.StatusOK). - JSON().Equal(iris.Map{"data": "things"}) -} - type testControllerMethodHandlerBindStruct struct{} type bindStructData struct { diff --git a/versioning/version.go b/versioning/version.go index b5436d43..26be5541 100644 --- a/versioning/version.go +++ b/versioning/version.go @@ -1,7 +1,7 @@ package versioning import ( - "errors" + "fmt" "strings" "github.com/kataras/iris/v12/context" @@ -34,7 +34,7 @@ const ( // ErrNotFound reports whether a requested version // does not match with any of the server's implemented ones. -var ErrNotFound = errors.New("version not found") +var ErrNotFound = fmt.Errorf("version %w", context.ErrNotFound) // NotFoundHandler is the default version not found handler that // is executed from `NewMatcher` when no version is registered as available to dispatch a resource. diff --git a/versioning/versioning.go b/versioning/versioning.go index fb4d702f..4041f304 100644 --- a/versioning/versioning.go +++ b/versioning/versioning.go @@ -49,9 +49,11 @@ func Match(ctx *context.Context, expectedVersion string) bool { func Handler(version string) context.Handler { return func(ctx *context.Context) { if !Match(ctx, version) { - // Any overlapped handler - // can just clear the status code - // and the error to ignore this (see `NewGroup`). + // The overlapped handler has an exception + // of a type of context.NotFound (which versioning.ErrNotFound wraps) + // to clear the status code + // and the error to ignore this + // when available match version exists (see `NewGroup`). NotFoundHandler(ctx) return }