fix issue #4176: refactor Router with option pattern

adapter/app.go

@@ -74,7 +74,7 @@ func oldMiddlewareToNew(mws []MiddleWare) []web.MiddleWare {
 //  beego.Router("/api/update",&RestController{},"put:UpdateFood")
 //  beego.Router("/api/delete",&RestController{},"delete:DeleteFood")
 func Router(rootpath string, c ControllerInterface, mappingMethods ...string) *App {
-	return (*App)(web.Router(rootpath, c, mappingMethods...))
+	return (*App)(web.Router(rootpath, c, web.WithMethods(c, mappingMethods...)))
 }
 
 // UnregisterFixedRoute unregisters the route with the specified fixedRoute. It is particularly useful

adapter/namespace.go

@@ -272,7 +272,7 @@ func NSInclude(cList ...ControllerInterface) LinkNamespace {
 // NSRouter call Namespace Router
 func NSRouter(rootpath string, c ControllerInterface, mappingMethods ...string) LinkNamespace {
 	return func(namespace *Namespace) {
-		web.Router(rootpath, c, mappingMethods...)
+		web.Router(rootpath, c, web.WithMethods(c, mappingMethods...))
 	}
 }
 

adapter/router.go

@@ -87,7 +87,7 @@ func NewControllerRegister() *ControllerRegister {
 //	Add("/api",&RestController{},"get,post:ApiFunc"
 //	Add("/simple",&SimpleController{},"get:GetFunc;post:PostFunc")
 func (p *ControllerRegister) Add(pattern string, c ControllerInterface, mappingMethods ...string) {
-	(*web.ControllerRegister)(p).Add(pattern, c, mappingMethods...)
+	(*web.ControllerRegister)(p).Add(pattern, c, web.WithMethods(c, mappingMethods...))
 }
 
 // Include only when the Runmode is dev will generate router file in the router/auto.go from the controller

server/web/admin.go

@@ -112,13 +112,13 @@ func registerAdmin() error {
 			HttpServer: NewHttpServerWithCfg(BConfig),
 		}
 		// keep in mind that all data should be html escaped to avoid XSS attack
-		beeAdminApp.Router("/", c, "get:AdminIndex")
+		beeAdminApp.Router("/", c, WithMethods(c, "get:AdminIndex"))
-		beeAdminApp.Router("/qps", c, "get:QpsIndex")
+		beeAdminApp.Router("/qps", c, WithMethods(c, "get:QpsIndex"))
-		beeAdminApp.Router("/prof", c, "get:ProfIndex")
+		beeAdminApp.Router("/prof", c, WithMethods(c, "get:ProfIndex"))
-		beeAdminApp.Router("/healthcheck", c, "get:Healthcheck")
+		beeAdminApp.Router("/healthcheck", c, WithMethods(c, "get:Healthcheck"))
-		beeAdminApp.Router("/task", c, "get:TaskStatus")
+		beeAdminApp.Router("/task", c, WithMethods(c, "get:TaskStatus"))
-		beeAdminApp.Router("/listconf", c, "get:ListConf")
+		beeAdminApp.Router("/listconf", c, WithMethods(c, "get:ListConf"))
-		beeAdminApp.Router("/metrics", c, "get:PrometheusMetrics")
+		beeAdminApp.Router("/metrics", c, WithMethods(c, "get:PrometheusMetrics"))
 
 		go beeAdminApp.Run()
 	}

server/web/flash_test.go

@@ -40,7 +40,7 @@ func TestFlashHeader(t *testing.T) {
 
 	// setup the handler
 	handler := NewControllerRegister()
-	handler.Add("/", &TestFlashController{}, "get:TestWriteFlash")
+	handler.Add("/", &TestFlashController{}, WithMethods(&TestFlashController{}, "get:TestWriteFlash"))
 	handler.ServeHTTP(w, r)
 
 	// get the Set-Cookie value

server/web/namespace.go

@@ -99,7 +99,7 @@ func (n *Namespace) Filter(action string, filter ...FilterFunc) *Namespace {
 // Router same as beego.Rourer
 // refer: https://godoc.org/github.com/astaxie/beego#Router
 func (n *Namespace) Router(rootpath string, c ControllerInterface, mappingMethods ...string) *Namespace {
-	n.handlers.Add(rootpath, c, mappingMethods...)
+	n.handlers.Add(rootpath, c, WithMethods(c, mappingMethods...))
 	return n
 }
 

server/web/router.go

@@ -120,10 +120,18 @@ type ControllerInfo struct {
 	methodParams   []*param.MethodParam
 }
 
+type ControllerOptions func(*ControllerInfo)
+
 func (c *ControllerInfo) GetPattern() string {
 	return c.pattern
 }
 
+func WithMethods(ctrlInterface ControllerInterface, mappingMethod ...string) ControllerOptions {
+	return func(c *ControllerInfo) {
+		c.methods = parseMappingMethods(ctrlInterface, mappingMethod)
+	}
+}
+
 // ControllerRegister containers registered router rules, controller handlers and filters.
 type ControllerRegister struct {
 	routers      map[string]*Tree
@@ -171,15 +179,19 @@ func NewControllerRegisterWithCfg(cfg *Config) *ControllerRegister {
 //	Add("/api/delete",&RestController{},"delete:DeleteFood")
 //	Add("/api",&RestController{},"get,post:ApiFunc"
 //	Add("/simple",&SimpleController{},"get:GetFunc;post:PostFunc")
-func (p *ControllerRegister) Add(pattern string, c ControllerInterface, mappingMethods ...string) {
+func (p *ControllerRegister) Add(pattern string, c ControllerInterface, opts ...ControllerOptions) {
-	p.addWithMethodParams(pattern, c, nil, mappingMethods...)
+	p.addWithMethodParams(pattern, c, nil, opts...)
 }
 
-func (p *ControllerRegister) addWithMethodParams(pattern string, c ControllerInterface, methodParams []*param.MethodParam, mappingMethods ...string) {
+func parseMappingMethods(c ControllerInterface, mappingMethods []string) map[string]string {
 	reflectVal := reflect.ValueOf(c)
 	t := reflect.Indirect(reflectVal).Type()
 	methods := make(map[string]string)
-	if len(mappingMethods) > 0 {
+
+	if len(mappingMethods) == 0 {
+		return methods
+	}
+
 	semi := strings.Split(mappingMethods[0], ";")
 	for _, v := range semi {
 		colon := strings.Split(v, ":")
@@ -188,22 +200,44 @@ func (p *ControllerRegister) addWithMethodParams(pattern string, c ControllerInt
 		}
 		comma := strings.Split(colon[0], ",")
 		for _, m := range comma {
-				if m == "*" || HTTPMETHOD[strings.ToUpper(m)] {
+			if m != "*" && !HTTPMETHOD[strings.ToUpper(m)] {
-					if val := reflectVal.MethodByName(colon[1]); val.IsValid() {
-						methods[strings.ToUpper(m)] = colon[1]
-					} else {
-						panic("'" + colon[1] + "' method doesn't exist in the controller " + t.Name())
-					}
-				} else {
 				panic(v + " is an invalid method mapping. Method doesn't exist " + m)
 			}
+			if val := reflectVal.MethodByName(colon[1]); val.IsValid() {
+				methods[strings.ToUpper(m)] = colon[1]
+				continue
 			}
+			panic("'" + colon[1] + "' method doesn't exist in the controller " + t.Name())
 		}
 	}
 
+	return methods
+}
+
+func (p *ControllerRegister) addRouterForMethod(route *ControllerInfo) {
+	if len(route.methods) == 0 {
+		for m := range HTTPMETHOD {
+			p.addToRouter(m, route.pattern, route)
+		}
+		return
+	}
+	for k := range route.methods {
+		if k != "*" {
+			p.addToRouter(k, route.pattern, route)
+			continue
+		}
+		for m := range HTTPMETHOD {
+			p.addToRouter(m, route.pattern, route)
+		}
+	}
+}
+
+func (p *ControllerRegister) addWithMethodParams(pattern string, c ControllerInterface, methodParams []*param.MethodParam, opts ...ControllerOptions) {
+	reflectVal := reflect.ValueOf(c)
+	t := reflect.Indirect(reflectVal).Type()
+
 	route := &ControllerInfo{}
 	route.pattern = pattern
-	route.methods = methods
 	route.routerType = routerTypeBeego
 	route.controllerType = t
 	route.initialize = func() ControllerInterface {
@@ -229,23 +263,11 @@ func (p *ControllerRegister) addWithMethodParams(pattern string, c ControllerInt
 
 		return execController
 	}
-
 	route.methodParams = methodParams
-	if len(methods) == 0 {
+	for i := range opts {
-		for m := range HTTPMETHOD {
+		opts[i](route)
-			p.addToRouter(m, pattern, route)
-		}
-	} else {
-		for k := range methods {
-			if k == "*" {
-				for m := range HTTPMETHOD {
-					p.addToRouter(m, pattern, route)
-				}
-			} else {
-				p.addToRouter(k, pattern, route)
-			}
-		}
 	}
+	p.addRouterForMethod(route)
 }
 
 func (p *ControllerRegister) addToRouter(method, pattern string, r *ControllerInfo) {
@@ -274,7 +296,7 @@ func (p *ControllerRegister) Include(cList ...ControllerInterface) {
 					p.InsertFilter(f.Pattern, f.Pos, f.Filter, WithReturnOnOutput(f.ReturnOnOutput), WithResetParams(f.ResetParams))
 				}
 
-				p.addWithMethodParams(a.Router, c, a.MethodParams, strings.Join(a.AllowHTTPMethods, ",")+":"+a.Method)
+				p.addWithMethodParams(a.Router, c, a.MethodParams, WithMethods(c, strings.Join(a.AllowHTTPMethods, ",")+":"+a.Method))
 			}
 		}
 	}

server/web/router_test.go

@@ -89,8 +89,8 @@ func (jc *JSONController) Get() {
 
 func TestUrlFor(t *testing.T) {
 	handler := NewControllerRegister()
-	handler.Add("/api/list", &TestController{}, "*:List")
+	handler.Add("/api/list", &TestController{}, WithMethods(&TestController{}, "*:List"))
-	handler.Add("/person/:last/:first", &TestController{}, "*:Param")
+	handler.Add("/person/:last/:first", &TestController{}, WithMethods(&TestController{}, "*:Param"))
 	if a := handler.URLFor("TestController.List"); a != "/api/list" {
 		logs.Info(a)
 		t.Errorf("TestController.List must equal to /api/list")
@@ -113,9 +113,9 @@ func TestUrlFor3(t *testing.T) {
 
 func TestUrlFor2(t *testing.T) {
 	handler := NewControllerRegister()
-	handler.Add("/v1/:v/cms_:id(.+)_:page(.+).html", &TestController{}, "*:List")
+	handler.Add("/v1/:v/cms_:id(.+)_:page(.+).html", &TestController{}, WithMethods(&TestController{}, "*:List"))
-	handler.Add("/v1/:username/edit", &TestController{}, "get:GetURL")
+	handler.Add("/v1/:username/edit", &TestController{}, WithMethods(&TestController{}, "get:GetURL"))
-	handler.Add("/v1/:v(.+)_cms/ttt_:id(.+)_:page(.+).html", &TestController{}, "*:Param")
+	handler.Add("/v1/:v(.+)_cms/ttt_:id(.+)_:page(.+).html", &TestController{}, WithMethods(&TestController{}, "*:Param"))
 	handler.Add("/:year:int/:month:int/:title/:entid", &TestController{})
 	if handler.URLFor("TestController.GetURL", ":username", "astaxie") != "/v1/astaxie/edit" {
 		logs.Info(handler.URLFor("TestController.GetURL"))
@@ -145,7 +145,7 @@ func TestUserFunc(t *testing.T) {
 	w := httptest.NewRecorder()
 
 	handler := NewControllerRegister()
-	handler.Add("/api/list", &TestController{}, "*:List")
+	handler.Add("/api/list", &TestController{}, WithMethods(&TestController{}, "*:List"))
 	handler.ServeHTTP(w, r)
 	if w.Body.String() != "i am list" {
 		t.Errorf("user define func can't run")
@@ -235,7 +235,7 @@ func TestRouteOk(t *testing.T) {
 	w := httptest.NewRecorder()
 
 	handler := NewControllerRegister()
-	handler.Add("/person/:last/:first", &TestController{}, "get:GetParams")
+	handler.Add("/person/:last/:first", &TestController{}, WithMethods(&TestController{}, "get:GetParams"))
 	handler.ServeHTTP(w, r)
 	body := w.Body.String()
 	if body != "anderson+thomas+kungfu" {
@@ -249,7 +249,7 @@ func TestManyRoute(t *testing.T) {
 	w := httptest.NewRecorder()
 
 	handler := NewControllerRegister()
-	handler.Add("/beego:id([0-9]+)-:page([0-9]+).html", &TestController{}, "get:GetManyRouter")
+	handler.Add("/beego:id([0-9]+)-:page([0-9]+).html", &TestController{}, WithMethods(&TestController{}, "get:GetManyRouter"))
 	handler.ServeHTTP(w, r)
 
 	body := w.Body.String()
@@ -266,7 +266,7 @@ func TestEmptyResponse(t *testing.T) {
 	w := httptest.NewRecorder()
 
 	handler := NewControllerRegister()
-	handler.Add("/beego-empty.html", &TestController{}, "get:GetEmptyBody")
+	handler.Add("/beego-empty.html", &TestController{}, WithMethods(&TestController{}, "get:GetEmptyBody"))
 	handler.ServeHTTP(w, r)
 
 	if body := w.Body.String(); body != "" {

server/web/server.go

@@ -266,8 +266,8 @@ func (app *HttpServer) Run(addr string, mws ...MiddleWare) {
 }
 
 // Router see HttpServer.Router
-func Router(rootpath string, c ControllerInterface, mappingMethods ...string) *HttpServer {
+func Router(rootpath string, c ControllerInterface, opts ...ControllerOptions) *HttpServer {
-	return BeeApp.Router(rootpath, c, mappingMethods...)
+	return BeeApp.Router(rootpath, c, opts...)
 }
 
 // Router adds a patterned controller handler to BeeApp.
@@ -286,8 +286,8 @@ func Router(rootpath string, c ControllerInterface, mappingMethods ...string) *H
 //  beego.Router("/api/create",&RestController{},"post:CreateFood")
 //  beego.Router("/api/update",&RestController{},"put:UpdateFood")
 //  beego.Router("/api/delete",&RestController{},"delete:DeleteFood")
-func (app *HttpServer) Router(rootPath string, c ControllerInterface, mappingMethods ...string) *HttpServer {
+func (app *HttpServer) Router(rootPath string, c ControllerInterface, opts ...ControllerOptions) *HttpServer {
-	app.Handlers.Add(rootPath, c, mappingMethods...)
+	app.Handlers.Add(rootPath, c, opts...)
 	return app
 }
 

server/web/unregroute_test.go

@@ -75,9 +75,9 @@ func TestUnregisterFixedRouteRoot(t *testing.T) {
 	var method = "GET"
 
 	handler := NewControllerRegister()
-	handler.Add("/", &TestPreUnregController{}, "get:GetFixedRoot")
+	handler.Add("/", &TestPreUnregController{}, WithMethods(&TestPreUnregController{}, "get:GetFixedRoot"))
-	handler.Add("/level1", &TestPreUnregController{}, "get:GetFixedLevel1")
+	handler.Add("/level1", &TestPreUnregController{}, WithMethods(&TestPreUnregController{}, "get:GetFixedLevel1"))
-	handler.Add("/level1/level2", &TestPreUnregController{}, "get:GetFixedLevel2")
+	handler.Add("/level1/level2", &TestPreUnregController{}, WithMethods(&TestPreUnregController{}, "get:GetFixedLevel2"))
 
 	// Test original root
 	testHelperFnContentCheck(t, handler, "Test original root",
@@ -96,7 +96,7 @@ func TestUnregisterFixedRouteRoot(t *testing.T) {
 
 	// Replace the root path TestPreUnregController action with the action from
 	// TestPostUnregController
-	handler.Add("/", &TestPostUnregController{}, "get:GetFixedRoot")
+	handler.Add("/", &TestPostUnregController{}, WithMethods(&TestPostUnregController{}, "get:GetFixedRoot"))
 
 	// Test replacement root (expect change)
 	testHelperFnContentCheck(t, handler, "Test replacement root (expect change)", method, "/", contentRootReplacement)
@@ -117,9 +117,9 @@ func TestUnregisterFixedRouteLevel1(t *testing.T) {
 	var method = "GET"
 
 	handler := NewControllerRegister()
-	handler.Add("/", &TestPreUnregController{}, "get:GetFixedRoot")
+	handler.Add("/", &TestPreUnregController{}, WithMethods(&TestPreUnregController{}, "get:GetFixedRoot"))
-	handler.Add("/level1", &TestPreUnregController{}, "get:GetFixedLevel1")
+	handler.Add("/level1", &TestPreUnregController{}, WithMethods(&TestPreUnregController{}, "get:GetFixedLevel1"))
-	handler.Add("/level1/level2", &TestPreUnregController{}, "get:GetFixedLevel2")
+	handler.Add("/level1/level2", &TestPreUnregController{}, WithMethods(&TestPreUnregController{}, "get:GetFixedLevel2"))
 
 	// Test original root
 	testHelperFnContentCheck(t, handler,
@@ -146,7 +146,7 @@ func TestUnregisterFixedRouteLevel1(t *testing.T) {
 
 	// Replace the "level1" path TestPreUnregController action with the action from
 	// TestPostUnregController
-	handler.Add("/level1", &TestPostUnregController{}, "get:GetFixedLevel1")
+	handler.Add("/level1", &TestPostUnregController{}, WithMethods(&TestPostUnregController{}, "get:GetFixedLevel1"))
 
 	// Test replacement root (expect no change from the original)
 	testHelperFnContentCheck(t, handler, "Test replacement root (expect no change from the original)", method, "/", contentRootOriginal)
@@ -167,9 +167,9 @@ func TestUnregisterFixedRouteLevel2(t *testing.T) {
 	var method = "GET"
 
 	handler := NewControllerRegister()
-	handler.Add("/", &TestPreUnregController{}, "get:GetFixedRoot")
+	handler.Add("/", &TestPreUnregController{}, WithMethods(&TestPreUnregController{}, "get:GetFixedRoot"))
-	handler.Add("/level1", &TestPreUnregController{}, "get:GetFixedLevel1")
+	handler.Add("/level1", &TestPreUnregController{}, WithMethods(&TestPreUnregController{}, "get:GetFixedLevel1"))
-	handler.Add("/level1/level2", &TestPreUnregController{}, "get:GetFixedLevel2")
+	handler.Add("/level1/level2", &TestPreUnregController{}, WithMethods(&TestPreUnregController{}, "get:GetFixedLevel2"))
 
 	// Test original root
 	testHelperFnContentCheck(t, handler,
@@ -196,7 +196,7 @@ func TestUnregisterFixedRouteLevel2(t *testing.T) {
 
 	// Replace the "/level1/level2" path TestPreUnregController action with the action from
 	// TestPostUnregController
-	handler.Add("/level1/level2", &TestPostUnregController{}, "get:GetFixedLevel2")
+	handler.Add("/level1/level2", &TestPostUnregController{}, WithMethods(&TestPostUnregController{}, "get:GetFixedLevel2"))
 
 	// Test replacement root (expect no change from the original)
 	testHelperFnContentCheck(t, handler, "Test replacement root (expect no change from the original)", method, "/", contentRootOriginal)