From bb66c10ad3a0cb8515feb8f916dd6553269eaabd Mon Sep 17 00:00:00 2001 From: "Gerasimos (Makis) Maropoulos" Date: Mon, 2 Mar 2020 19:48:53 +0200 Subject: [PATCH] :monkey_face: prepare next version: improve the hero and mvc path parameters bindings Former-commit-id: 0626b91c6448b5cebf1d04ee3f115cde68aa3d6d --- core/router/api_builder.go | 19 +++++------ hero/binding.go | 70 ++++++++++++++++++++++---------------- hero/container.go | 21 ++++++------ hero/handler.go | 4 +-- hero/handler_test.go | 30 ++++++++++++++++ hero/struct.go | 11 +++--- hero/struct_test.go | 18 +++++----- macro/template.go | 6 ++++ mvc/controller.go | 12 ++++--- 9 files changed, 122 insertions(+), 69 deletions(-) diff --git a/core/router/api_builder.go b/core/router/api_builder.go index 7d53ca95..b0f22a9a 100644 --- a/core/router/api_builder.go +++ b/core/router/api_builder.go @@ -296,10 +296,13 @@ func (api *APIBuilder) RegisterDependency(dependency interface{}) *hero.Dependen } // convertHandlerFuncs accepts Iris hero handlers and returns a slice of native Iris handlers. -func (api *APIBuilder) convertHandlerFuncs(handlersFn ...interface{}) context.Handlers { +func (api *APIBuilder) convertHandlerFuncs(relativePath string, handlersFn ...interface{}) context.Handlers { + fullpath := api.relativePath + relativePath + paramsCount := macro.CountParams(fullpath, *api.macros) + handlers := make(context.Handlers, 0, len(handlersFn)) for _, h := range handlersFn { - handlers = append(handlers, api.container.Handler(h)) + handlers = append(handlers, api.container.HandlerWithParams(h, paramsCount)) } // On that type of handlers the end-developer does not have to include the Context in the handler, @@ -322,7 +325,7 @@ func (api *APIBuilder) convertHandlerFuncs(handlersFn ...interface{}) context.Ha // // See `OnErrorFunc`, `RegisterDependency`, `UseFunc` and `DoneFunc` too. func (api *APIBuilder) HandleFunc(method, relativePath string, handlersFn ...interface{}) *Route { - handlers := api.convertHandlerFuncs(handlersFn...) + handlers := api.convertHandlerFuncs(relativePath, handlersFn...) return api.Handle(method, relativePath, handlers...) } @@ -578,8 +581,6 @@ func (api *APIBuilder) Party(relativePath string, handlers ...context.Handler) P // attach a new Container with correct dynamic path parameter start index for input arguments // based on the fullpath. childContainer := api.container.Clone() - fpath, _ := macro.Parse(fullpath, *api.macros) - childContainer.ParamStartIndex = len(fpath.Params) return &APIBuilder{ // global/api builder @@ -743,7 +744,7 @@ func (api *APIBuilder) Use(handlers ...context.Handler) { // // See `OnErrorFunc`, `RegisterDependency`, `DoneFunc` and `HandleFunc` for more. func (api *APIBuilder) UseFunc(handlersFn ...interface{}) { - handlers := api.convertHandlerFuncs(handlersFn...) + handlers := api.convertHandlerFuncs("/", handlersFn...) api.Use(handlers...) } @@ -776,7 +777,7 @@ func (api *APIBuilder) Done(handlers ...context.Handler) { // DoneFunc same as "Done" but it accepts dynamic functions as its "handlersFn" input. // See `OnErrorFunc`, `RegisterDependency`, `UseFunc` and `HandleFunc` for more. func (api *APIBuilder) DoneFunc(handlersFn ...interface{}) { - handlers := api.convertHandlerFuncs(handlersFn...) + handlers := api.convertHandlerFuncs("/", handlersFn...) api.Done(handlers...) } @@ -1060,10 +1061,6 @@ func getCaller() (string, int) { continue } - if strings.Contains(file, ".amd64/src/") { - continue - } - if !strings.Contains(file, "/kataras/iris") || strings.Contains(file, "/kataras/iris/_examples") || strings.Contains(file, "iris-contrib/examples") { diff --git a/hero/binding.go b/hero/binding.go index 10a1ba7f..9e50871c 100644 --- a/hero/binding.go +++ b/hero/binding.go @@ -111,33 +111,45 @@ func matchDependency(dep *Dependency, in reflect.Type) bool { return dep.DestType == nil || equalTypes(dep.DestType, in) } -func getBindingsFor(inputs []reflect.Type, deps []*Dependency, paramStartIndex int) (bindings []*binding) { - bindedInput := make(map[int]struct{}) +func getBindingsFor(inputs []reflect.Type, deps []*Dependency, paramsCount int) (bindings []*binding) { + // Path parameter start index is the result of [total path parameters] - [total func path parameters inputs], + // moving from last to first path parameters and first to last (probably) available input args. + // + // That way the above will work as expected: + // 1. mvc.New(app.Party("/path/{firstparam}")).Handle(....Controller.GetBy(secondparam string)) + // 2. mvc.New(app.Party("/path/{firstparam}/{secondparam}")).Handle(...Controller.GetBy(firstparam, secondparam string)) + // 3. usersRouter := app.Party("/users/{id:uint64}"); usersRouter.HandleFunc(method, "/", handler(id uint64)) + // 4. usersRouter.Party("/friends").HandleFunc(method, "/{friendID:uint64}", handler(friendID uint64)) + // + // Therefore, count the inputs that can be path parameters first. + shouldBindParams := make(map[int]struct{}) + totalParamsExpected := 0 + for i, in := range inputs { + if _, canBePathParameter := context.ParamResolvers[in]; !canBePathParameter { + continue + } + shouldBindParams[i] = struct{}{} - // lastParamIndex is used to bind parameters correctly when: - // otherDep, param1, param2 string and param1 string, otherDep, param2 string. - lastParamIndex := paramStartIndex - getParamIndex := func(index int) (paramIndex int) { - // if len(bindings) > 0 { - // // mostly, it means it's binding to a struct's method, which first value is always the ptr struct as its receiver. - // // so we decrement the parameter index otherwise first parameter would be declared as parameter index 1 instead of 0. - // paramIndex = len(bindings) + lastParamIndex - 1 - // lastParamIndex = paramIndex + 1 - // return paramIndex - // } - - // lastParamIndex = index + 1 - // return index - - paramIndex = lastParamIndex - lastParamIndex = paramIndex + 1 - return + totalParamsExpected++ } - for i, in := range inputs { //order matters. + startParamIndex := paramsCount - totalParamsExpected + if startParamIndex < 0 { + startParamIndex = 0 + } - _, canBePathParameter := context.ParamResolvers[in] - canBePathParameter = canBePathParameter && paramStartIndex != -1 // if -1 then parameter resolver is disabled. + lastParamIndex := startParamIndex + + getParamIndex := func() int { + paramIndex := lastParamIndex + lastParamIndex++ + return paramIndex + } + + bindedInput := make(map[int]struct{}) + + for i, in := range inputs { //order matters. + _, canBePathParameter := shouldBindParams[i] prevN := len(bindings) // to check if a new binding is attached; a dependency was matched (see below). @@ -160,7 +172,7 @@ func getBindingsFor(inputs []reflect.Type, deps []*Dependency, paramStartIndex i if canBePathParameter { // wrap the existing dependency handler. - paramHandler := paramDependencyHandler(getParamIndex((i))) + paramHandler := paramDependencyHandler(getParamIndex()) prevHandler := d.Handle d.Handle = func(ctx context.Context, input *Input) (reflect.Value, error) { v, err := paramHandler(ctx, input) @@ -190,7 +202,7 @@ func getBindingsFor(inputs []reflect.Type, deps []*Dependency, paramStartIndex i if canBePathParameter { // no new dependency added for this input, // let's check for path parameters. - bindings = append(bindings, paramBinding(i, getParamIndex(i), in)) + bindings = append(bindings, paramBinding(i, getParamIndex(), in)) continue } @@ -205,7 +217,7 @@ func getBindingsFor(inputs []reflect.Type, deps []*Dependency, paramStartIndex i return } -func getBindingsForFunc(fn reflect.Value, dependencies []*Dependency, paramStartIndex int) []*binding { +func getBindingsForFunc(fn reflect.Value, dependencies []*Dependency, paramsCount int) []*binding { fnTyp := fn.Type() if !isFunc(fnTyp) { panic("bindings: unresolved: not a func type") @@ -217,7 +229,7 @@ func getBindingsForFunc(fn reflect.Value, dependencies []*Dependency, paramStart inputs[i] = fnTyp.In(i) } - bindings := getBindingsFor(inputs, dependencies, paramStartIndex) + bindings := getBindingsFor(inputs, dependencies, paramsCount) if expected, got := n, len(bindings); expected > got { panic(fmt.Sprintf("expected [%d] bindings (input parameters) but got [%d]", expected, got)) } @@ -225,7 +237,7 @@ func getBindingsForFunc(fn reflect.Value, dependencies []*Dependency, paramStart return bindings } -func getBindingsForStruct(v reflect.Value, dependencies []*Dependency, paramStartIndex int, sorter Sorter) (bindings []*binding) { +func getBindingsForStruct(v reflect.Value, dependencies []*Dependency, paramsCount int, sorter Sorter) (bindings []*binding) { typ := indirectType(v.Type()) if typ.Kind() != reflect.Struct { panic("bindings: unresolved: no struct type") @@ -256,7 +268,7 @@ func getBindingsForStruct(v reflect.Value, dependencies []*Dependency, paramStar // fmt.Printf("Controller [%s] | Field Index: %v | Field Type: %s\n", typ, fields[i].Index, fields[i].Type) inputs[i] = fields[i].Type } - exportedBindings := getBindingsFor(inputs, dependencies, paramStartIndex) + exportedBindings := getBindingsFor(inputs, dependencies, paramsCount) // fmt.Printf("Controller [%s] Inputs length: %d vs Bindings length: %d\n", typ, n, len(exportedBindings)) if len(nonZero) >= len(exportedBindings) { // if all are fields are defined then just return. diff --git a/hero/container.go b/hero/container.go index 9aeca0b4..a9a60d8c 100644 --- a/hero/container.go +++ b/hero/container.go @@ -23,9 +23,6 @@ var Default = New() // // For a more high-level structure please take a look at the "mvc.go#Application". type Container struct { - // Indicates the path parameter start index for inputs binding. - // Defaults to 0. - ParamStartIndex int // Sorter specifies how the inputs should be sorted before binded. // Defaults to sort by "thinnest" target empty interface. Sorter Sorter @@ -81,9 +78,8 @@ func New(dependencies ...interface{}) *Container { copy(deps, BuiltinDependencies) c := &Container{ - ParamStartIndex: 0, - Sorter: sortByNumMethods, - Dependencies: deps, + Sorter: sortByNumMethods, + Dependencies: deps, GetErrorHandler: func(context.Context) ErrorHandler { return DefaultErrorHandler }, @@ -100,7 +96,6 @@ func New(dependencies ...interface{}) *Container { // It copies the ErrorHandler, Dependencies and all Options from "c" receiver. func (c *Container) Clone() *Container { cloned := New() - cloned.ParamStartIndex = c.ParamStartIndex cloned.GetErrorHandler = c.GetErrorHandler cloned.Sorter = c.Sorter clonedDeps := make([]*Dependency, len(c.Dependencies)) @@ -167,12 +162,18 @@ func Handler(fn interface{}) context.Handler { // It returns a standard `iris/context.Handler` which can be used anywhere in an Iris Application, // as middleware or as simple route handler or subdomain's handler. func (c *Container) Handler(fn interface{}) context.Handler { - return makeHandler(fn, c) + return c.HandlerWithParams(fn, 0) +} + +// HandlerWithParams same as `Handler` but it can receive a total path parameters counts +// to resolve coblex path parameters input dependencies. +func (c *Container) HandlerWithParams(fn interface{}, paramsCount int) context.Handler { + return makeHandler(fn, c, paramsCount) } // Struct accepts a pointer to a struct value and returns a structure which // contains bindings for the struct's fields and a method to // extract a Handler from this struct's method. -func (c *Container) Struct(ptrValue interface{}) *Struct { - return makeStruct(ptrValue, c) +func (c *Container) Struct(ptrValue interface{}, partyParamsCount int) *Struct { + return makeStruct(ptrValue, c, partyParamsCount) } diff --git a/hero/handler.go b/hero/handler.go index ef8d7973..002375df 100644 --- a/hero/handler.go +++ b/hero/handler.go @@ -55,7 +55,7 @@ var ( }) ) -func makeHandler(fn interface{}, c *Container) context.Handler { +func makeHandler(fn interface{}, c *Container, paramsCount int) context.Handler { if fn == nil { panic("makeHandler: function is nil") } @@ -77,7 +77,7 @@ func makeHandler(fn interface{}, c *Container) context.Handler { v := valueOf(fn) numIn := v.Type().NumIn() - bindings := getBindingsForFunc(v, c.Dependencies, c.ParamStartIndex) + bindings := getBindingsForFunc(v, c.Dependencies, paramsCount) return func(ctx context.Context) { inputs := make([]reflect.Value, numIn) diff --git a/hero/handler_test.go b/hero/handler_test.go index 529d2120..439978df 100644 --- a/hero/handler_test.go +++ b/hero/handler_test.go @@ -201,3 +201,33 @@ func TestDependentDependencies(t *testing.T) { e.GET("/h1").Expect().Status(httptest.StatusOK).Body().Equal("prefix: it is a deep dependency") e.GET("/h2").Expect().Status(httptest.StatusOK).Body().Equal("prefix: message") } + +func TestHandlerPathParams(t *testing.T) { + // See white box `TestPathParams` test too. + // All cases should pass. + app := iris.New() + handler := func(id uint64) string { + return fmt.Sprintf("%d", id) + } + + app.PartyFunc("/users", func(r iris.Party) { + r.HandleFunc(iris.MethodGet, "/{id:uint64}", handler) + }) + + app.PartyFunc("/editors/{id:uint64}", func(r iris.Party) { + r.HandleFunc(iris.MethodGet, "/", handler) + }) + + // should receive the last one, as we expected only one useful for MVC (there is a similar test there too). + app.HandleFunc(iris.MethodGet, "/{ownerID:uint64}/book/{booKID:uint64}", handler) + + e := httptest.New(t, app) + + for _, testReq := range []*httptest.Request{ + e.GET("/users/42"), + e.GET("/editors/42"), + e.GET("/1/book/42"), + } { + testReq.Expect().Status(httptest.StatusOK).Body().Equal("42") + } +} diff --git a/hero/struct.go b/hero/struct.go index ce17ebcb..4ab3eb8e 100644 --- a/hero/struct.go +++ b/hero/struct.go @@ -43,7 +43,7 @@ type Struct struct { Singleton bool } -func makeStruct(structPtr interface{}, c *Container) *Struct { +func makeStruct(structPtr interface{}, c *Container, partyParamsCount int) *Struct { v := valueOf(structPtr) typ := v.Type() if typ.Kind() != reflect.Ptr || indirectType(typ).Kind() != reflect.Struct { @@ -51,7 +51,7 @@ func makeStruct(structPtr interface{}, c *Container) *Struct { } // get struct's fields bindings. - bindings := getBindingsForStruct(v, c.Dependencies, c.ParamStartIndex, c.Sorter) + bindings := getBindingsForStruct(v, c.Dependencies, partyParamsCount, c.Sorter) // length bindings of 0, means that it has no fields or all mapped deps are static. // If static then Struct.Acquire will return the same "value" instance, otherwise it will create a new one. @@ -138,11 +138,14 @@ func (s *Struct) Acquire(ctx context.Context) (reflect.Value, error) { // MethodHandler accepts a "methodName" that should be a valid an exported // method of the struct and returns its converted Handler. -func (s *Struct) MethodHandler(methodName string) context.Handler { +// +// Second input is optional, +// even zero is a valid value and can resolve path parameters correctly if from root party. +func (s *Struct) MethodHandler(methodName string, paramsCount int) context.Handler { m, ok := s.ptrValue.Type().MethodByName(methodName) if !ok { panic(fmt.Sprintf("struct: method: %s does not exist", methodName)) } - return makeHandler(m.Func, s.Container) + return makeHandler(m.Func, s.Container, paramsCount) } diff --git a/hero/struct_test.go b/hero/struct_test.go index 33f9a33c..48ece882 100644 --- a/hero/struct_test.go +++ b/hero/struct_test.go @@ -34,15 +34,15 @@ func TestStruct(t *testing.T) { app := iris.New() b := New() - s := b.Struct(&testStruct{}) + s := b.Struct(&testStruct{}, 0) - postHandler := s.MethodHandler("MyHandler") // fallbacks such as {path} and {string} should registered first when same path. + postHandler := s.MethodHandler("MyHandler", 0) // fallbacks such as {path} and {string} should registered first when same path. app.Post("/{name:string}", postHandler) - postHandler2 := s.MethodHandler("MyHandler2") + postHandler2 := s.MethodHandler("MyHandler2", 0) app.Post("/{id:int}", postHandler2) - postHandler3 := s.MethodHandler("MyHandler3") + postHandler3 := s.MethodHandler("MyHandler3", 0) app.Post("/myHandler3", postHandler3) - getHandler := s.MethodHandler("MyHandler4") + getHandler := s.MethodHandler("MyHandler4", 0) app.Get("/myHandler4", getHandler) e := httptest.New(t, app) @@ -67,10 +67,10 @@ func (s *testStructErrorHandler) Handle(errText string) error { func TestStructErrorHandler(t *testing.T) { b := New() - s := b.Struct(&testStructErrorHandler{}) + s := b.Struct(&testStructErrorHandler{}, 0) app := iris.New() - app.Get("/{errText:string}", s.MethodHandler("Handle")) + app.Get("/{errText:string}", s.MethodHandler("Handle", 0)) expectedErrText := "an error" e := httptest.New(t, app) @@ -111,10 +111,10 @@ func TestStructFieldsSorter(t *testing.T) { // see https://github.com/kataras/ir b := New() b.Register(&testServiceImpl1{"parser"}) b.Register(&testServiceImpl2{24}) - s := b.Struct(&testControllerDependenciesSorter{}) + s := b.Struct(&testControllerDependenciesSorter{}, 0) app := iris.New() - app.Get("/", s.MethodHandler("Index")) + app.Get("/", s.MethodHandler("Index", 0)) e := httptest.New(t, app) diff --git a/macro/template.go b/macro/template.go index c97cbf26..845646fc 100644 --- a/macro/template.go +++ b/macro/template.go @@ -157,3 +157,9 @@ func Parse(src string, macros Macros) (Template, error) { return tmpl, nil } + +// CountParams returns the length of the dynamic path's input parameters. +func CountParams(fullpath string, macros Macros) int { + tmpl, _ := Parse(fullpath, macros) + return len(tmpl.Params) +} diff --git a/mvc/controller.go b/mvc/controller.go index c8563b8b..c590bfca 100644 --- a/mvc/controller.go +++ b/mvc/controller.go @@ -8,6 +8,7 @@ import ( "github.com/kataras/iris/v12/context" "github.com/kataras/iris/v12/core/router" "github.com/kataras/iris/v12/hero" + "github.com/kataras/iris/v12/macro" ) // BaseController is the optional controller interface, if it's @@ -226,7 +227,8 @@ func (c *ControllerActivator) isReservedMethod(name string) bool { func (c *ControllerActivator) attachInjector() { if c.injector == nil { - c.injector = c.app.container.Struct(c.Value) + partyCountParams := macro.CountParams(c.app.Router.GetRelPath(), *c.app.Router.Macros()) + c.injector = c.app.container.Struct(c.Value, partyCountParams) } } @@ -303,7 +305,7 @@ func (c *ControllerActivator) handleMany(method, path, funcName string, override return nil } - handler := c.handlerOf(funcName) + handler := c.handlerOf(path, funcName) // register the handler now. routes := c.app.Router.HandleMany(method, path, append(middleware, handler)...) @@ -332,10 +334,12 @@ func (c *ControllerActivator) handleMany(method, path, funcName string, override return routes } -func (c *ControllerActivator) handlerOf(methodName string) context.Handler { +func (c *ControllerActivator) handlerOf(relPath, methodName string) context.Handler { c.attachInjector() - handler := c.injector.MethodHandler(methodName) + fullpath := c.app.Router.GetRelPath() + relPath + paramsCount := macro.CountParams(fullpath, *c.app.Router.Macros()) + handler := c.injector.MethodHandler(methodName, paramsCount) if isBaseController(c.Type) { return func(ctx context.Context) {