🐵 prepare next version: improve the hero and mvc path parameters bindings

Former-commit-id: 0626b91c6448b5cebf1d04ee3f115cde68aa3d6d

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.

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)
 }

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)

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")
+	}
+}

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)
 }

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)