Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Params surgery: Split out the types of params and provided a unified …

…view

This allows the Params Filter to be moved after the RouterFilter, which
allows it to be subject to the FilterConfigurator.   It will also help
when we extend Params to handle JSON and XML params.
  • Loading branch information...
commit da8b6b4bfb12c0bc319e5ddd1381068c227b23ff 1 parent 47fcc74
@robfig authored
View
3  binder_test.go
@@ -156,7 +156,8 @@ var fileBindings = []struct{ val, arrval, f interface{} }{
func TestBinder(t *testing.T) {
// Reuse the mvc_test.go multipart request to test the binder.
- params := ParseParams(NewRequest(getMultipartRequest()))
+ params := &Params{}
+ ParseParams(params, NewRequest(getMultipartRequest()))
params.Values = PARAMS
// Values
View
1  controller.go
@@ -38,6 +38,7 @@ func NewController(req *Request, resp *Response) *Controller {
return &Controller{
Request: req,
Response: resp,
+ Params: new(Params),
Args: map[string]interface{}{},
RenderArgs: map[string]interface{}{
"RunMode": RunMode,
View
2  filter.go
@@ -15,9 +15,9 @@ type InitializingFilter interface {
// It may be set by the application on initialization.
var Filters = []Filter{
PanicFilter,
- ParamsFilter,
RouterFilter,
FilterConfiguringFilter,
+ ParamsFilter,
SessionFilter,
FlashFilter,
ValidationFilter,
View
2  invoker.go
@@ -29,7 +29,7 @@ func (f actionInvoker) Call(c *Controller, _ FilterChain) {
boundArg = reflect.ValueOf(c.Request.Websocket)
} else {
TRACE.Println("Binding:", arg.Name, "as", arg.Type)
- boundArg = c.Params.Bind(arg.Name, arg.Type)
+ boundArg = Bind(c.Params, arg.Name, arg.Type)
}
methodArgs = append(methodArgs, boundArg)
}
View
4 invoker_test.go
@@ -100,7 +100,9 @@ func TestMultiEmbedding(t *testing.T) {
func BenchmarkInvoker(b *testing.B) {
startFakeBookingApp()
- var c Controller
+ c := Controller{
+ RenderArgs: make(map[string]interface{}),
+ }
if err := c.SetAction("Hotels", "Show"); err != nil {
b.Errorf("Failed to set action: %s", err)
return
View
93 params.go
@@ -7,22 +7,30 @@ import (
"reflect"
)
-// These provide a unified view of the request params.
+// Params provides a unified view of the request params.
// Includes:
// - URL query string
// - Form values
// - File uploads
+//
+// Warning: param maps other than Values may be nil if there were none.
type Params struct {
- url.Values
- Files map[string][]*multipart.FileHeader
- tmpFiles []*os.File // Temp files used during the request.
-}
+ url.Values // A unified view of all the individual param maps below.
+
+ // Set by the router
+ Fixed url.Values // Fixed parameters from the route, e.g. App.Action("fixed param")
+ Route url.Values // Parameters extracted from the route, e.g. /customers/{id}
-func ParseParams(req *Request) *Params {
- var files map[string][]*multipart.FileHeader
+ // Set by the ParamsFilter
+ Query url.Values // Parameters from the query string, e.g. /index?limit=10
+ Form url.Values // Parameters from the request body.
- // Always want the url parameters.
- values := req.URL.Query()
+ Files map[string][]*multipart.FileHeader // Files uploaded in a multipart form
+ tmpFiles []*os.File // Temp files used during the request.
+}
+
+func ParseParams(params *Params, req *Request) {
+ params.Query = req.URL.Query()
// Parse the body depending on the content type.
switch req.ContentType {
@@ -31,11 +39,7 @@ func ParseParams(req *Request) *Params {
if err := req.ParseForm(); err != nil {
WARN.Println("Error parsing request body:", err)
} else {
- for key, vals := range req.Form {
- for _, val := range vals {
- values.Add(key, val)
- }
- }
+ params.Form = req.Form
}
case "multipart/form-data":
@@ -44,20 +48,61 @@ func ParseParams(req *Request) *Params {
if err := req.ParseMultipartForm(32 << 20 /* 32 MB */); err != nil {
WARN.Println("Error parsing request body:", err)
} else {
- for key, vals := range req.MultipartForm.Value {
- for _, val := range vals {
- values.Add(key, val)
- }
- }
- files = req.MultipartForm.File
+ params.Form = req.MultipartForm.Value
+ params.Files = req.MultipartForm.File
}
}
- return &Params{Values: values, Files: files}
+ params.Values = params.calcValues()
+}
+
+// Bind looks for the named parameter, converts it to the requested type, and
+// writes it into "dest", which must be settable. If the value can not be
+// parsed, "dest" is set to the zero value.
+func (p *Params) Bind(dest interface{}, name string) {
+ value := reflect.ValueOf(dest)
+ if !value.CanSet() {
+ panic("Passed a non-settable variable to Bind: " + name)
+ }
+ value.Set(Bind(p, name, value.Type().Elem()))
}
-func (p *Params) Bind(name string, typ reflect.Type) reflect.Value {
- return Bind(p, name, typ)
+// calcValues returns a unified view of the component param maps.
+func (p *Params) calcValues() url.Values {
+ numParams := len(p.Query) + len(p.Fixed) + len(p.Route) + len(p.Form)
+
+ // If there were no params, return an empty map.
+ if numParams == 0 {
+ return make(url.Values, 0)
+ }
+
+ // If only one of the param sources has anything, return that directly.
+ switch numParams {
+ case len(p.Query):
+ return p.Query
+ case len(p.Route):
+ return p.Route
+ case len(p.Fixed):
+ return p.Fixed
+ case len(p.Form):
+ return p.Form
+ }
+
+ // Copy everything into the same map.
+ values := make(url.Values, numParams)
+ for k, v := range p.Query {
+ values[k] = append(values[k], v...)
+ }
+ for k, v := range p.Fixed {
+ values[k] = append(values[k], v...)
+ }
+ for k, v := range p.Route {
+ values[k] = append(values[k], v...)
+ }
+ for k, v := range p.Form {
+ values[k] = append(values[k], v...)
+ }
+ return values
}
var ParamsFilter paramsFilter
@@ -65,7 +110,7 @@ var ParamsFilter paramsFilter
type paramsFilter struct{}
func (f paramsFilter) Call(c *Controller, fc FilterChain) {
- c.Params = ParseParams(c.Request)
+ ParseParams(c.Params, c.Request)
// Clean up from the request.
defer func() {
View
2  params_test.go
@@ -86,6 +86,7 @@ func getMultipartRequest() *http.Request {
func BenchmarkParams(b *testing.B) {
c := Controller{
Request: NewRequest(getMultipartRequest()),
+ Params: &Params{},
}
for i := 0; i < b.N; i++ {
ParamsFilter.Call(&c, NilChain)
@@ -95,6 +96,7 @@ func BenchmarkParams(b *testing.B) {
func TestMultipartForm(t *testing.T) {
c := Controller{
Request: NewRequest(getMultipartRequest()),
+ Params: &Params{},
}
ParamsFilter.Call(&c, NilChain)
View
12 router.go
@@ -427,16 +427,20 @@ func (f routerFilter) Call(c *Controller, fc FilterChain) {
return
}
- // Add the route Params to the Request Params.
- for key, value := range route.Params {
- url.Values(c.Params.Values).Add(key, value)
+ // Add the route and fixed params to the Request Params.
+ for k, v := range route.Params {
+ if c.Params.Route == nil {
+ c.Params.Route = make(map[string][]string)
+ }
+ c.Params.Route[k] = []string{v}
}
// Add the fixed parameters mapped by name.
+ c.Params.Fixed = make(url.Values)
for i, value := range route.FixedParams {
if i < len(c.MethodType.Args) {
arg := c.MethodType.Args[i]
- c.Params.Values.Set(arg.Name, value)
+ c.Params.Fixed.Set(arg.Name, value)
} else {
WARN.Println("Too many parameters to", route.Action, "trying to add", value)
break
View
3  router_test.go
@@ -386,7 +386,8 @@ func BenchmarkRouterFilter(b *testing.B) {
{Request: NewRequest(staticRequest)},
}
for _, c := range controllers {
- c.Params = ParseParams(c.Request)
+ c.Params = &Params{}
+ ParseParams(c.Params, c.Request)
}
b.ResetTimer()
View
2  skeleton/app/init.go
@@ -5,9 +5,9 @@ import "github.com/robfig/revel"
func init() {
revel.Filters = revel.FilterChain{
revel.PanicFilter,
- revel.ParamsFilter,
revel.RouterFilter,
revel.FilterConfiguringFilter,
+ revel.ParamsFilter,
revel.SessionFilter,
revel.FlashFilter,
revel.ValidationFilter,
Please sign in to comment.
Something went wrong with that request. Please try again.