From 22947b131ef40560018219d8ca569ff569cb266d Mon Sep 17 00:00:00 2001 From: Yonghwan SO Date: Tue, 22 Mar 2022 23:10:12 +0900 Subject: [PATCH 1/4] intermediately, introduce Home struct --- app.go | 29 ++++++++++++++++------------- home.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 13 deletions(-) create mode 100644 home.go diff --git a/app.go b/app.go index 804061e43..8ac10e0d8 100644 --- a/app.go +++ b/app.go @@ -14,15 +14,13 @@ import ( // Without an App you can't do much! type App struct { Options - // Middleware returns the current MiddlewareStack for the App/Group. - Middleware *MiddlewareStack `json:"-"` - ErrorHandlers ErrorHandlers `json:"-"` - router *mux.Router - moot *sync.RWMutex - routes RouteList - root *App - children []*App - filepaths []string + // Middleware, ErrorHandlers, router, and filepaths are moved to Home. + Home + moot *sync.RWMutex + routes RouteList + // TODO: to be deprecated #road-to-v1 + root *App + children []*App // Routenamer for the app. This field provides the ability to override the // base route namer for something more specific to the app. @@ -44,11 +42,16 @@ func New(opts Options) *App { a := &App{ Options: opts, - ErrorHandlers: ErrorHandlers{ - http.StatusNotFound: defaultErrorHandler, - http.StatusInternalServerError: defaultErrorHandler, + Home: Home{ + name: opts.Name, + host: opts.Host, + prefix: opts.Prefix, + ErrorHandlers: ErrorHandlers{ + http.StatusNotFound: defaultErrorHandler, + http.StatusInternalServerError: defaultErrorHandler, + }, + router: mux.NewRouter(), }, - router: mux.NewRouter(), moot: &sync.RWMutex{}, routes: RouteList{}, children: []*App{}, diff --git a/home.go b/home.go new file mode 100644 index 000000000..79255e547 --- /dev/null +++ b/home.go @@ -0,0 +1,39 @@ +package buffalo + +import ( + "github.com/gorilla/mux" +) + +/* TODO: consider to split out Home (or Router, whatever) from App #road-to-v1 + Group and Domain based multi-homing are actually not an App if the concept + of the App represents the application. The App should be only one for whole + application. + + For an extreme example, App.Group().Stop() or even App.Group().Serve() are + still valid function calls while they should not be allowed and the result + could be strage. +*/ + +// Home is a container for Domains and Groups that independantly serves a +// group of pages with its own Middleware and ErrorHandlers. It is usually +// a multi-homed server domain or group of paths under a certain prefix. +// +// While the App is for managing whole application life cycle along with its +// default Home, including initializing and stopping its all components such +// as listeners and long-running jobs, Home is only for a specific group of +// services to serve its service logic efficiently. +type Home struct { + app *App // will replace App.root + appSelf *App // temporary while the App is in action. + // replace Options' Name, Host, and Prefix + name string + host string + prefix string + + // moved from App + // Middleware returns the current MiddlewareStack for the App/Group. + Middleware *MiddlewareStack `json:"-"` + ErrorHandlers ErrorHandlers `json:"-"` + router *mux.Router + filepaths []string +} From 211ee6893f3f87a729602c59e541ba1413fc1bfd Mon Sep 17 00:00:00 2001 From: Yonghwan SO Date: Wed, 23 Mar 2022 19:34:51 +0900 Subject: [PATCH 2/4] adapted to the new struct (with small bugfix) --- app.go | 2 ++ route.go | 1 + route_info.go | 2 +- route_mappings.go | 58 +++++++++++++++++++++++++++++------------------ 4 files changed, 40 insertions(+), 23 deletions(-) diff --git a/app.go b/app.go index 8ac10e0d8..12a4dc348 100644 --- a/app.go +++ b/app.go @@ -58,6 +58,8 @@ func New(opts Options) *App { RouteNamer: baseRouteNamer{}, } + a.Home.app = a // replace root. + a.Home.appSelf = a // temporary, reverse reference to the group app. dem := a.defaultErrorMiddleware a.Middleware = newMiddlewareStack(dem) diff --git a/route.go b/route.go index b5f7b3111..a0a04cff7 100644 --- a/route.go +++ b/route.go @@ -11,6 +11,7 @@ import ( // Routes returns a list of all of the routes defined // in this application. func (a *App) Routes() RouteList { + // CHKME: why this function is exported? can we deprecate it? if a.root != nil { return a.root.routes } diff --git a/route_info.go b/route_info.go index b5e20d512..c6be5a636 100644 --- a/route_info.go +++ b/route_info.go @@ -49,7 +49,7 @@ func (ri *RouteInfo) Alias(aliases ...string) *RouteInfo { func (ri *RouteInfo) Name(name string) *RouteInfo { routeIndex := -1 for index, route := range ri.App.Routes() { - if route.Path == ri.Path && route.Method == ri.Method { + if route.App.host == ri.App.host && route.Path == ri.Path && route.Method == ri.Method { routeIndex = index break } diff --git a/route_mappings.go b/route_mappings.go index 4f42b0551..824e87abf 100644 --- a/route_mappings.go +++ b/route_mappings.go @@ -20,6 +20,8 @@ const ( AssetsAgeVarName = "ASSETS_MAX_AGE" ) +// These method functions will be moved to Home structure. + // GET maps an HTTP "GET" request to the path and the specified handler. func (a *App) GET(p string, h Handler) *RouteInfo { return a.addRoute("GET", p, h) @@ -92,17 +94,21 @@ func (a *App) Mount(p string, h http.Handler) { a.Host("{subdomain:[a-z]+}.example.com") */ func (a *App) Host(h string) *App { + // TODO: move this function to app.go or home.go eventually. + // in the end, it should return *Home. g := New(a.Options) + g.host = h g.router = a.router.Host(h).Subrouter() g.RouteNamer = a.RouteNamer g.Middleware = a.Middleware.clone() g.ErrorHandlers = a.ErrorHandlers - g.root = a - if a.root != nil { - g.root = a.root - } + g.app = a.app // will replace g.root + g.root = g.app // will be deprecated + + // to be replaced with child Homes. currently, only used in grifts. + a.children = append(a.children, g) return g } @@ -228,6 +234,7 @@ func (a *App) Resource(p string, r Resource) *App { g.DELETE(path.Join(spath), r.Destroy).ResourceName = resourceName g.Prefix = path.Join(g.Prefix, spath) + g.prefix = g.Prefix return g } @@ -254,18 +261,26 @@ func (a *App) ANY(p string, h Handler) { g.GET("/users/:user_id, APIUserShowHandler) */ func (a *App) Group(groupPath string) *App { + // TODO: move this function to app.go or home.go eventually. g := New(a.Options) + // keep them for v0 compatibility g.Prefix = path.Join(a.Prefix, groupPath) g.Name = g.Prefix + // for Home structure + g.prefix = path.Join(a.prefix, groupPath) + g.host = a.host + g.name = g.prefix + g.router = a.router g.RouteNamer = a.RouteNamer g.Middleware = a.Middleware.clone() g.ErrorHandlers = a.ErrorHandlers - g.root = a - if a.root != nil { - g.root = a.root - } + + g.app = a.app // will replace g.root + g.root = g.app // will be deprecated + + // to be replaced with child Homes. currently, only used in grifts. a.children = append(a.children, g) return g } @@ -280,13 +295,15 @@ func (a *App) RouteHelpers() map[string]RouteHelperFunc { return rh } -func (a *App) addRoute(method string, url string, h Handler) *RouteInfo { - a.moot.Lock() - defer a.moot.Unlock() +func (e *Home) addRoute(method string, url string, h Handler) *RouteInfo { + // NOTE: lock the root app (not this app). only the root has the affective + // routes list. + e.app.moot.Lock() + defer e.app.moot.Unlock() - url = path.Join(a.Prefix, url) - url = a.normalizePath(url) - name := a.RouteNamer.NameRoute(url) + url = path.Join(e.prefix, url) + url = e.app.normalizePath(url) + name := e.app.RouteNamer.NameRoute(url) hs := funcKey(h) r := &RouteInfo{ @@ -294,22 +311,19 @@ func (a *App) addRoute(method string, url string, h Handler) *RouteInfo { Path: url, HandlerName: hs, Handler: h, - App: a, + App: e.appSelf, // CHKME: to be replaced with Home Aliases: []string{}, } - r.MuxRoute = a.router.Handle(url, r).Methods(method) + r.MuxRoute = e.router.Handle(url, r).Methods(method) r.Name(name) - routes := a.Routes() + routes := e.app.Routes() routes = append(routes, r) + // do we really need to sort this? sort.Sort(routes) - if a.root != nil { - a.root.routes = routes - } else { - a.routes = routes - } + e.app.routes = routes return r } From 16d6f7c072952ec193bdc47c2000da834f27d513 Mon Sep 17 00:00:00 2001 From: Yonghwan SO Date: Thu, 24 Mar 2022 23:35:18 +0900 Subject: [PATCH 3/4] fixed issues related to multi-homing and task {routes,middleware} --- grifts.go | 30 ++++++++++++++++-------------- route.go | 13 +++++++++++-- route_mappings.go | 38 ++++++++++++++++++++++++-------------- 3 files changed, 51 insertions(+), 30 deletions(-) diff --git a/grifts.go b/grifts.go index 84af4a774..1cd5a28c1 100644 --- a/grifts.go +++ b/grifts.go @@ -44,22 +44,24 @@ func printMiddleware(a *App) { func printMiddlewareByRoute(a *App) { mws := map[string]string{} + // TODO: middleware is 'per App' can it be a loop for Apps? for _, r := range a.Routes() { - if mws[r.App.Name] == "" { - pname := "" + key := r.App.host + r.App.name + if mws[key] == "" { + pname := r.App.host if parent := getParentApp(r.App.root, r.App.Name); parent != nil { - pname = parent.Name + pname += parent.Name } - mws[r.App.Name] = r.App.Middleware.String() - if mws[pname] != mws[r.App.Name] { - fmt.Printf("-> %s\n", r.App.Name) - printMiddlewareStackWithIndent(mws[r.App.Name]) + mws[key] = r.App.Middleware.String() + if pname == key || mws[pname] != mws[key] { + fmt.Printf("-> %s\n", key) + printMiddlewareStackWithIndent(mws[key]) } else { - fmt.Printf("-> %s (see: %v)\n", r.App.Name, pname) + fmt.Printf("-> %s (see: %v)\n", key, pname) } } - s := "\n" + mws[r.App.Name] + s := "\n" + mws[key] for k := range r.App.Middleware.skips { mw := strings.Split(k, funcKeyDelimeter)[0] h := strings.Split(k, funcKeyDelimeter)[1] @@ -67,10 +69,10 @@ func printMiddlewareByRoute(a *App) { s = strings.Replace(s, "\n"+mw, "", 1) } } - if "\n"+mws[r.App.Name] != s { + if "\n"+mws[key] != s { ahn := strings.Split(r.HandlerName, "/") hn := ahn[len(ahn)-1] - fmt.Printf("-> %s %s (by %s)\n", r.Method, r.Path, hn) + fmt.Printf("-> %s %s (by %s)\n", r.Method, r.App.host+r.Path, hn) printMiddlewareStackWithIndent(s) } } @@ -106,10 +108,10 @@ func routesGrift(a *App) { grift.Add("routes", func(c *grift.Context) error { routes := a.Routes() w := tabwriter.NewWriter(os.Stdout, 0, 0, 1, ' ', tabwriter.Debug) - fmt.Fprintln(w, "METHOD\t PATH\t ALIASES\t NAME\t HANDLER") - fmt.Fprintln(w, "------\t ----\t -------\t ----\t -------") + fmt.Fprintln(w, "METHOD\t HOST\t PATH\t ALIASES\t NAME\t HANDLER") + fmt.Fprintln(w, "------\t ----\t ----\t -------\t ----\t -------") for _, r := range routes { - fmt.Fprintf(w, "%s\t %s\t %s\t %s\t %s\n", r.Method, r.Path, strings.Join(r.Aliases, " "), r.PathName, r.HandlerName) + fmt.Fprintf(w, "%s\t %s\t %s\t %s\t %s\t %s\n", r.Method, r.App.host, r.Path, strings.Join(r.Aliases, " "), r.PathName, r.HandlerName) } w.Flush() return nil diff --git a/route.go b/route.go index a0a04cff7..cab38c424 100644 --- a/route.go +++ b/route.go @@ -65,11 +65,20 @@ type RouteHelperFunc func(opts map[string]interface{}) (template.HTML, error) // and the name of the Handler defined to process that route. type RouteList []*RouteInfo +var methodOrder = map[string]string{ + "GET": "1", + "POST": "2", + "PUT": "3", + "DELETE": "4", +} + func (a RouteList) Len() int { return len(a) } func (a RouteList) Swap(i, j int) { a[i], a[j] = a[j], a[i] } func (a RouteList) Less(i, j int) bool { - x := a[i].Path // + a[i].Method - y := a[j].Path // + a[j].Method + // NOTE: it was used for sorting of app.routes but we don't sort the routes anymore. + // keep it for compatibility but could be deprecated. + x := a[i].App.host + a[i].Path + methodOrder[a[i].Method] + y := a[j].App.host + a[j].Path + methodOrder[a[j].Method] return x < y } diff --git a/route_mappings.go b/route_mappings.go index 824e87abf..5bf33f483 100644 --- a/route_mappings.go +++ b/route_mappings.go @@ -7,7 +7,6 @@ import ( "os" "path" "reflect" - "sort" "strings" "github.com/gobuffalo/envy" @@ -162,6 +161,11 @@ type editable interface { // to the appropriate RESTful mappings. Resource returns the *App // associated with this group of mappings so you can set middleware, etc... // on that group, just as if you had used the a.Group functionality. +// +// Resource automatically creates a URL `/resources/new` if the resource +// has a function `New()`. So it could act as a restriction for the value +// of `resource_id`. URL `/resources/new` will always show the resource +// creation page instead of showing the resource called `new`. /* a.Resource("/users", &UsersResource{}) @@ -170,12 +174,12 @@ type editable interface { ur := &UsersResource{} g := a.Group("/users") g.GET("/", ur.List) // GET /users => ur.List + g.POST("/", ur.Create) // POST /users => ur.Create g.GET("/new", ur.New) // GET /users/new => ur.New g.GET("/{user_id}", ur.Show) // GET /users/{user_id} => ur.Show + g.PUT("/{user_id}", ur.Update) // PUT /users/{user_id} => ur.Update + g.DELETE("/{user_id}", ur.Destroy) // DELETE /users/{user_id} => ur.Destroy g.GET("/{user_id}/edit", ur.Edit) // GET /users/{user_id}/edit => ur.Edit - g.POST("/", ur.Create) // POST /users => ur.Create - g.PUT("/{user_id}", ur.Update) PUT /users/{user_id} => ur.Update - g.DELETE("/{user_id}", ur.Destroy) DELETE /users/{user_id} => ur.Destroy */ func (a *App) Resource(p string, r Resource) *App { g := a.Group(p) @@ -208,9 +212,14 @@ func (a *App) Resource(p string, r Resource) *App { spath := path.Join(p, "{"+paramName+"}") + // This order will become the order of route evaluation too. setFuncKey(r.List, fmt.Sprintf(handlerName, "List")) g.GET(p, r.List).ResourceName = resourceName + setFuncKey(r.Create, fmt.Sprintf(handlerName, "Create")) + g.POST(p, r.Create).ResourceName = resourceName + + // NOTE: it makes restriction that resource id cannot be 'new'. if n, ok := r.(newable); ok { setFuncKey(n.New, fmt.Sprintf(handlerName, "New")) g.GET(path.Join(p, "new"), n.New).ResourceName = resourceName @@ -219,20 +228,17 @@ func (a *App) Resource(p string, r Resource) *App { setFuncKey(r.Show, fmt.Sprintf(handlerName, "Show")) g.GET(path.Join(spath), r.Show).ResourceName = resourceName - if n, ok := r.(editable); ok { - setFuncKey(n.Edit, fmt.Sprintf(handlerName, "Edit")) - g.GET(path.Join(spath, "edit"), n.Edit).ResourceName = resourceName - } - - setFuncKey(r.Create, fmt.Sprintf(handlerName, "Create")) - g.POST(p, r.Create).ResourceName = resourceName - setFuncKey(r.Update, fmt.Sprintf(handlerName, "Update")) g.PUT(path.Join(spath), r.Update).ResourceName = resourceName setFuncKey(r.Destroy, fmt.Sprintf(handlerName, "Destroy")) g.DELETE(path.Join(spath), r.Destroy).ResourceName = resourceName + if n, ok := r.(editable); ok { + setFuncKey(n.Edit, fmt.Sprintf(handlerName, "Edit")) + g.GET(path.Join(spath, "edit"), n.Edit).ResourceName = resourceName + } + g.Prefix = path.Join(g.Prefix, spath) g.prefix = g.Prefix @@ -320,8 +326,12 @@ func (e *Home) addRoute(method string, url string, h Handler) *RouteInfo { routes := e.app.Routes() routes = append(routes, r) - // do we really need to sort this? - sort.Sort(routes) + // NOTE: sorting is fancy but we lose the evaluation order information + // of routing decision. Let's keep the routes as registered order so + // developers can easily evaluate the order with `buffalo routes` and + // can debug any routing priority issue. (just keep the original line + // as history reference) + //sort.Sort(routes) e.app.routes = routes From efeb149fe79c206bcd3e1a9444fde51afae15d51 Mon Sep 17 00:00:00 2001 From: Yonghwan SO Date: Fri, 25 Mar 2022 00:37:16 +0900 Subject: [PATCH 4/4] simplified, renamed the multi-homing function (Host -> VirtualHost) --- route_mappings.go | 51 ++++++++++++++++++++---------------------- route_mappings_test.go | 6 ++--- 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/route_mappings.go b/route_mappings.go index 5bf33f483..c0f7d4d89 100644 --- a/route_mappings.go +++ b/route_mappings.go @@ -85,33 +85,6 @@ func (a *App) Mount(p string, h http.Handler) { a.ANY(path, WrapHandler(http.StripPrefix(prefix, h))) } -// Host creates a new "*App" group that matches the domain passed. -// This is useful for creating groups of end-points for different domains. -/* - a.Host("www.example.com") - a.Host("{subdomain}.example.com") - a.Host("{subdomain:[a-z]+}.example.com") -*/ -func (a *App) Host(h string) *App { - // TODO: move this function to app.go or home.go eventually. - // in the end, it should return *Home. - g := New(a.Options) - - g.host = h - g.router = a.router.Host(h).Subrouter() - g.RouteNamer = a.RouteNamer - g.Middleware = a.Middleware.clone() - g.ErrorHandlers = a.ErrorHandlers - - g.app = a.app // will replace g.root - g.root = g.app // will be deprecated - - // to be replaced with child Homes. currently, only used in grifts. - a.children = append(a.children, g) - - return g -} - // ServeFiles maps an path to a directory on disk to serve static files. // Useful for JavaScript, images, CSS, etc... /* @@ -291,6 +264,30 @@ func (a *App) Group(groupPath string) *App { return g } +// VirtualHost creates a new `*App` that inherits from it's parent `*App`. +// All pre-configured things on the parent App such as middlewares will be +// applied, and can be modified only for this child App. +// +// This is a multi-homing feature similar to the `VirtualHost` in Apache +// or multiple `server`s in nginx. One important different behavior is that +// there is no concept of the `default` host in buffalo (at least for now) +// and the routing decision will be made with the "first match" manner. +// (e.g. if you have already set the route for '/' for the root App before +// setting up a virualhost, the route of the root App will be picked up +// even if the client makes a request to the specified domain.) +/* + a.VirtualHost("www.example.com") + a.VirtualHost("{subdomain}.example.com") + a.VirtualHost("{subdomain:[a-z]+}.example.com") +*/ +func (a *App) VirtualHost(h string) *App { + g := a.Group("/") + g.host = h + g.router = a.router.Host(h).Subrouter() + + return g +} + // RouteHelpers returns a map of BuildPathHelper() for each route available in the app. func (a *App) RouteHelpers() map[string]RouteHelperFunc { rh := map[string]RouteHelperFunc{} diff --git a/route_mappings_test.go b/route_mappings_test.go index fe80e2888..5a189e316 100644 --- a/route_mappings_test.go +++ b/route_mappings_test.go @@ -155,13 +155,13 @@ func Test_App_Routes_Resource(t *testing.T) { } } -func Test_App_Host(t *testing.T) { +func Test_App_VirtualHost(t *testing.T) { r := require.New(t) a1 := New(Options{}) r.Nil(a1.root) - h1 := a1.Host("www.example.com") + h1 := a1.VirtualHost("www.example.com") h1.GET("/foo", voidHandler) routes := h1.Routes() @@ -177,7 +177,7 @@ func Test_App_Host(t *testing.T) { a2 := New(Options{}) r.Nil(a1.root) - h2 := a2.Host("{subdomain}.example.com") + h2 := a2.VirtualHost("{subdomain}.example.com") h2.GET("/foo", voidHandler) h2.GET("/foo/{id}", voidHandler).Name("fooID")