From 5a12b3d020c838bd55ea68548af5dff0b1e5562a Mon Sep 17 00:00:00 2001 From: Silviu Capota-Mera Date: Mon, 25 Sep 2017 20:52:19 -0400 Subject: [PATCH 1/6] Add the ability to unregister fixed routes Certain web application inherit most of the routes from a base application by using the underscore import (e.g. import _ "myoldapp.com/routers"). The new application might want to only overwrite certain pages, such as "/about" or "/faq" The proposed new UnregisterFixedRoute method allows unregistering of the specified paths. Usage (replace "GET" with "*" for all methods): beego.UnregisterFixedRoute("/yourpreviouspath", "GET") beego.Router("/yourpreviouspath", yourControllerAddress, "get:GetNewPage") The children paths are left intact. For example, /yourpreviouspath/oldchildsubpath should still continue to function in legacy mode. --- app.go | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/app.go b/app.go index 25ea2a04..b29bb962 100644 --- a/app.go +++ b/app.go @@ -207,6 +207,72 @@ func Router(rootpath string, c ControllerInterface, mappingMethods ...string) *A return BeeApp } +// UnregisterFixedRoute unregisters the route with the specified fixedRoute. It is particularly useful +// in web applications that inherit most routes from a base webapp via the underscore +// import, and aim to overwrite only certain paths. +// The method parameter can be empty or "*" for all HTTP methods, or a particular +// method type (e.g. "GET" or "POST") for selective removal. +// +// Usage (replace "GET" with "*" for all methods): +// beego.UnregisterFixedRoute("/yourpreviouspath", "GET") +// beego.Router("/yourpreviouspath", yourControllerAddress, "get:GetNewPage") +func UnregisterFixedRoute(fixedRoute string, method string) *App { + subPaths := splitPath(fixedRoute) + if method == "" || method == "*" { + for _, m := range HTTPMETHOD { + if _, ok := BeeApp.Handlers.routers[m]; !ok { + continue + } + if BeeApp.Handlers.routers[m].prefix == strings.Trim(fixedRoute, "/ ") { + delete(BeeApp.Handlers.routers, m) + return BeeApp + } + findAndRemoveTree(subPaths, BeeApp.Handlers.routers[m], m) + } + return BeeApp + } + // Single HTTP method + um := strings.ToUpper(method) + if _, ok := BeeApp.Handlers.routers[um]; ok { + if BeeApp.Handlers.routers[um].prefix == strings.Trim(fixedRoute, "/ ") { + delete(BeeApp.Handlers.routers, um) + return BeeApp + } + findAndRemoveTree(subPaths, BeeApp.Handlers.routers[um], um) + } + return BeeApp +} + +func findAndRemoveTree(paths []string, entryPointTree *Tree, method string) { + for i := range entryPointTree.fixrouters { + if entryPointTree.fixrouters[i].prefix == paths[0] { + if len(paths) == 1 { + if len(entryPointTree.fixrouters[i].fixrouters) > 0 { + // If the route had children subtrees, remove just the functional leaf, + // to allow children to function as before + if len(entryPointTree.fixrouters[i].leaves) > 0 { + entryPointTree.fixrouters[i].leaves[0] = nil + entryPointTree.fixrouters[i].leaves = entryPointTree.fixrouters[i].leaves[1:] + } + } else { + // Remove the *Tree from the fixrouters slice + entryPointTree.fixrouters[i] = nil + + if i == len(entryPointTree.fixrouters)-1 { + entryPointTree.fixrouters = entryPointTree.fixrouters[:i] + } else { + entryPointTree.fixrouters = append(entryPointTree.fixrouters[:i], entryPointTree.fixrouters[i+1:len(entryPointTree.fixrouters)]...) + } + } + return + } + findAndRemoveTree(paths[1:], entryPointTree.fixrouters[i], method) + } + } + // No match + return +} + // Include will generate router file in the router/xxx.go from the controller's comments // usage: // beego.Include(&BankAccount{}, &OrderController{},&RefundController{},&ReceiptController{}) From 51a61623632021c1e023daeca7cb905aa24b272d Mon Sep 17 00:00:00 2001 From: Silviu Capota-Mera Date: Mon, 25 Sep 2017 21:45:42 -0400 Subject: [PATCH 2/6] Update app.go --- app.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app.go b/app.go index b29bb962..4c506878 100644 --- a/app.go +++ b/app.go @@ -22,6 +22,7 @@ import ( "os" "path" "time" + "strings" "github.com/astaxie/beego/grace" "github.com/astaxie/beego/logs" From 5697c6d7cc7f9d8394208ace651a6b5c03722504 Mon Sep 17 00:00:00 2001 From: Silviu Capota-Mera Date: Mon, 25 Sep 2017 23:17:57 -0400 Subject: [PATCH 3/6] Remove redundant return to pass gosimple Travis check --- app.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/app.go b/app.go index 4c506878..d2b492cd 100644 --- a/app.go +++ b/app.go @@ -270,8 +270,6 @@ func findAndRemoveTree(paths []string, entryPointTree *Tree, method string) { findAndRemoveTree(paths[1:], entryPointTree.fixrouters[i], method) } } - // No match - return } // Include will generate router file in the router/xxx.go from the controller's comments From fd733f76f0e97972e056ed4e45cb3e45cd5b7913 Mon Sep 17 00:00:00 2001 From: Silviu Capota-Mera Date: Sat, 7 Oct 2017 12:14:28 -0400 Subject: [PATCH 4/6] simplify replacement of the base (root) tree --- app.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/app.go b/app.go index d2b492cd..b966fde4 100644 --- a/app.go +++ b/app.go @@ -225,7 +225,7 @@ func UnregisterFixedRoute(fixedRoute string, method string) *App { continue } if BeeApp.Handlers.routers[m].prefix == strings.Trim(fixedRoute, "/ ") { - delete(BeeApp.Handlers.routers, m) + findAndRemoveSingleTree(BeeApp.Handlers.routers[m]) return BeeApp } findAndRemoveTree(subPaths, BeeApp.Handlers.routers[m], m) @@ -236,7 +236,7 @@ func UnregisterFixedRoute(fixedRoute string, method string) *App { um := strings.ToUpper(method) if _, ok := BeeApp.Handlers.routers[um]; ok { if BeeApp.Handlers.routers[um].prefix == strings.Trim(fixedRoute, "/ ") { - delete(BeeApp.Handlers.routers, um) + findAndRemoveSingleTree(BeeApp.Handlers.routers[um]) return BeeApp } findAndRemoveTree(subPaths, BeeApp.Handlers.routers[um], um) @@ -272,6 +272,20 @@ func findAndRemoveTree(paths []string, entryPointTree *Tree, method string) { } } +func findAndRemoveSingleTree(entryPointTree *Tree) { + + if len(entryPointTree.fixrouters) > 0 { + // Remove the *Tree from the fixrouters slice + entryPointTree.fixrouters[0] = nil + entryPointTree.fixrouters = entryPointTree.fixrouters[1:] + } + + if len(entryPointTree.leaves) > 0 { + entryPointTree.leaves[0] = nil + entryPointTree.leaves = entryPointTree.leaves[1:] + } +} + // Include will generate router file in the router/xxx.go from the controller's comments // usage: // beego.Include(&BankAccount{}, &OrderController{},&RefundController{},&ReceiptController{}) From 8d59e7afd1947c48ac4c1320a1eec652f270b0a6 Mon Sep 17 00:00:00 2001 From: Silviu Capota-Mera Date: Sat, 7 Oct 2017 13:16:36 -0400 Subject: [PATCH 5/6] for the root path, all methods must be covered use continue instead of return --- app.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/app.go b/app.go index b966fde4..7bbc65a6 100644 --- a/app.go +++ b/app.go @@ -226,7 +226,7 @@ func UnregisterFixedRoute(fixedRoute string, method string) *App { } if BeeApp.Handlers.routers[m].prefix == strings.Trim(fixedRoute, "/ ") { findAndRemoveSingleTree(BeeApp.Handlers.routers[m]) - return BeeApp + continue } findAndRemoveTree(subPaths, BeeApp.Handlers.routers[m], m) } @@ -273,16 +273,16 @@ func findAndRemoveTree(paths []string, entryPointTree *Tree, method string) { } func findAndRemoveSingleTree(entryPointTree *Tree) { - - if len(entryPointTree.fixrouters) > 0 { - // Remove the *Tree from the fixrouters slice - entryPointTree.fixrouters[0] = nil - entryPointTree.fixrouters = entryPointTree.fixrouters[1:] + if entryPointTree == nil { + return } - - if len(entryPointTree.leaves) > 0 { - entryPointTree.leaves[0] = nil - entryPointTree.leaves = entryPointTree.leaves[1:] + if len(entryPointTree.fixrouters) > 0 { + // If the route had children subtrees, remove just the functional leaf, + // to allow children to function as before + if len(entryPointTree.leaves) > 0 { + entryPointTree.leaves[0] = nil + entryPointTree.leaves = entryPointTree.leaves[1:] + } } } From 9536d460d0e4ef9977d36d95f58a14256c26f048 Mon Sep 17 00:00:00 2001 From: Silviu Capota-Mera Date: Sat, 28 Oct 2017 14:49:41 -0400 Subject: [PATCH 6/6] Create test file for the route unregistering functionality --- unregroute_test.go | 226 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 226 insertions(+) create mode 100644 unregroute_test.go diff --git a/unregroute_test.go b/unregroute_test.go new file mode 100644 index 00000000..59936e6e --- /dev/null +++ b/unregroute_test.go @@ -0,0 +1,226 @@ +// Copyright 2014 beego Author. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package beego + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// +// The unregroute_test.go contains tests for the unregister route +// functionality, that allows overriding route paths in children project +// that embed parent routers. +// + +const contentRootOriginal = "ok-original-root" +const contentLevel1Original = "ok-original-level1" +const contentLevel2Original = "ok-original-level2" + +const contentRootReplacement = "ok-replacement-root" +const contentLevel1Replacement = "ok-replacement-level1" +const contentLevel2Replacement = "ok-replacement-level2" + +// TestPreUnregController will supply content for the original routes, +// before unregistration +type TestPreUnregController struct { + Controller +} + +func (tc *TestPreUnregController) GetFixedRoot() { + tc.Ctx.Output.Body([]byte(contentRootOriginal)) +} +func (tc *TestPreUnregController) GetFixedLevel1() { + tc.Ctx.Output.Body([]byte(contentLevel1Original)) +} +func (tc *TestPreUnregController) GetFixedLevel2() { + tc.Ctx.Output.Body([]byte(contentLevel2Original)) +} + +// TestPostUnregController will supply content for the overriding routes, +// after the original ones are unregistered. +type TestPostUnregController struct { + Controller +} + +func (tc *TestPostUnregController) GetFixedRoot() { + tc.Ctx.Output.Body([]byte(contentRootReplacement)) +} +func (tc *TestPostUnregController) GetFixedLevel1() { + tc.Ctx.Output.Body([]byte(contentLevel1Replacement)) +} +func (tc *TestPostUnregController) GetFixedLevel2() { + tc.Ctx.Output.Body([]byte(contentLevel2Replacement)) +} + +// TestUnregisterFixedRouteRoot replaces just the root fixed route path. +// In this case, for a path like "/level1/level2" or "/level1", those actions +// should remain intact, and continue to serve the original content. +func TestUnregisterFixedRouteRoot(t *testing.T) { + + var method string = "GET" + + handler := NewControllerRegister() + handler.Add("/", &TestPreUnregController{}, "get:GetFixedRoot") + handler.Add("/level1", &TestPreUnregController{}, "get:GetFixedLevel1") + handler.Add("/level1/level2", &TestPreUnregController{}, "get:GetFixedLevel2") + + // Test original root + testHelperFnContentCheck(t, handler, "Test original root", + method, "/", contentRootOriginal) + + // Test original level 1 + testHelperFnContentCheck(t, handler, "Test original level 1", + method, "/level1", contentLevel1Original) + + // Test original level 2 + testHelperFnContentCheck(t, handler, "Test original level 2", + method, "/level1/level2", contentLevel2Original) + + // Remove only the root path + findAndRemoveSingleTree(handler.routers[method]) + + // Replace the root path TestPreUnregController action with the action from + // TestPostUnregController + handler.Add("/", &TestPostUnregController{}, "get:GetFixedRoot") + + // Test replacement root (expect change) + testHelperFnContentCheck(t, handler, "Test replacement root (expect change)", method, "/", contentRootReplacement) + + // Test level 1 (expect no change from the original) + testHelperFnContentCheck(t, handler, "Test level 1 (expect no change from the original)", method, "/level1", contentLevel1Original) + + // Test level 2 (expect no change from the original) + testHelperFnContentCheck(t, handler, "Test level 2 (expect no change from the original)", method, "/level1/level2", contentLevel2Original) + +} + +// TestUnregisterFixedRouteLevel1 replaces just the "/level1" fixed route path. +// In this case, for a path like "/level1/level2" or "/", those actions +// should remain intact, and continue to serve the original content. +func TestUnregisterFixedRouteLevel1(t *testing.T) { + + var method string = "GET" + + handler := NewControllerRegister() + handler.Add("/", &TestPreUnregController{}, "get:GetFixedRoot") + handler.Add("/level1", &TestPreUnregController{}, "get:GetFixedLevel1") + handler.Add("/level1/level2", &TestPreUnregController{}, "get:GetFixedLevel2") + + // Test original root + testHelperFnContentCheck(t, handler, + "TestUnregisterFixedRouteLevel1.Test original root", + method, "/", contentRootOriginal) + + // Test original level 1 + testHelperFnContentCheck(t, handler, + "TestUnregisterFixedRouteLevel1.Test original level 1", + method, "/level1", contentLevel1Original) + + // Test original level 2 + testHelperFnContentCheck(t, handler, + "TestUnregisterFixedRouteLevel1.Test original level 2", + method, "/level1/level2", contentLevel2Original) + + // Remove only the level1 path + subPaths := splitPath("/level1") + if handler.routers[method].prefix == strings.Trim("/level1", "/ ") { + findAndRemoveSingleTree(handler.routers[method]) + } else { + findAndRemoveTree(subPaths, handler.routers[method], method) + } + + // Replace the "level1" path TestPreUnregController action with the action from + // TestPostUnregController + handler.Add("/level1", &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) + + // Test level 1 (expect change) + testHelperFnContentCheck(t, handler, "Test level 1 (expect change)", method, "/level1", contentLevel1Replacement) + + // Test level 2 (expect no change from the original) + testHelperFnContentCheck(t, handler, "Test level 2 (expect no change from the original)", method, "/level1/level2", contentLevel2Original) + +} + +// TestUnregisterFixedRouteLevel2 unregisters just the "/level1/level2" fixed +// route path. In this case, for a path like "/level1" or "/", those actions +// should remain intact, and continue to serve the original content. +func TestUnregisterFixedRouteLevel2(t *testing.T) { + + var method string = "GET" + + handler := NewControllerRegister() + handler.Add("/", &TestPreUnregController{}, "get:GetFixedRoot") + handler.Add("/level1", &TestPreUnregController{}, "get:GetFixedLevel1") + handler.Add("/level1/level2", &TestPreUnregController{}, "get:GetFixedLevel2") + + // Test original root + testHelperFnContentCheck(t, handler, + "TestUnregisterFixedRouteLevel1.Test original root", + method, "/", contentRootOriginal) + + // Test original level 1 + testHelperFnContentCheck(t, handler, + "TestUnregisterFixedRouteLevel1.Test original level 1", + method, "/level1", contentLevel1Original) + + // Test original level 2 + testHelperFnContentCheck(t, handler, + "TestUnregisterFixedRouteLevel1.Test original level 2", + method, "/level1/level2", contentLevel2Original) + + // Remove only the level2 path + subPaths := splitPath("/level1/level2") + if handler.routers[method].prefix == strings.Trim("/level1/level2", "/ ") { + findAndRemoveSingleTree(handler.routers[method]) + } else { + findAndRemoveTree(subPaths, handler.routers[method], method) + } + + // Replace the "/level1/level2" path TestPreUnregController action with the action from + // TestPostUnregController + handler.Add("/level1/level2", &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) + + // Test level 1 (expect no change from the original) + testHelperFnContentCheck(t, handler, "Test level 1 (expect no change from the original)", method, "/level1", contentLevel1Original) + + // Test level 2 (expect change) + testHelperFnContentCheck(t, handler, "Test level 2 (expect change)", method, "/level1/level2", contentLevel2Replacement) + +} + +func testHelperFnContentCheck(t *testing.T, handler *ControllerRegister, + testName, method, path, expectedBodyContent string) { + + r, err := http.NewRequest(method, path, nil) + if err != nil { + t.Errorf("httpRecorderBodyTest NewRequest error: %v", err) + return + } + w := httptest.NewRecorder() + handler.ServeHTTP(w, r) + body := w.Body.String() + if body != expectedBodyContent { + t.Errorf("%s: expected [%s], got [%s];", testName, expectedBodyContent, body) + } +}