From 3abeba82fc15111a84cb5ebe62674108dcde54f8 Mon Sep 17 00:00:00 2001 From: Manu Mtz-Almeida Date: Tue, 7 Apr 2015 12:27:02 +0200 Subject: [PATCH 01/10] Context redirect uses the built-in redirect facility --- context.go | 4 ++-- render/render.go | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/context.go b/context.go index c028a79..e876842 100644 --- a/context.go +++ b/context.go @@ -234,9 +234,9 @@ func (c *Context) HTMLString(code int, format string, values ...interface{}) { // Returns a HTTP redirect to the specific location. func (c *Context) Redirect(code int, location string) { if code >= 300 && code <= 308 { - c.Render(code, render.Redirect, location) + c.Render(code, render.Redirect, c.Request, location) } else { - log.Panicf("Cannot send a redirect with status code %d", code) + log.Panicf("Cannot redirect with status code %d", code) } } diff --git a/render/render.go b/render/render.go index 90d5497..525adae 100644 --- a/render/render.go +++ b/render/render.go @@ -44,8 +44,9 @@ var ( ) func (_ redirectRender) Render(w http.ResponseWriter, code int, data ...interface{}) error { - w.Header().Set("Location", data[0].(string)) - w.WriteHeader(code) + req := data[0].(*http.Request) + location := data[1].(string) + http.Redirect(w, req, location, code) return nil } From ea962038e151598b9c564aed2e2d544d1541780e Mon Sep 17 00:00:00 2001 From: Manu Mtz-Almeida Date: Tue, 7 Apr 2015 12:27:23 +0200 Subject: [PATCH 02/10] Cosmetic changes --- debug.go | 2 +- gin.go | 6 ++---- mode.go | 14 +++++++------- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/debug.go b/debug.go index cfac22c..3670b98 100644 --- a/debug.go +++ b/debug.go @@ -7,7 +7,7 @@ package gin import "log" func IsDebugging() bool { - return gin_mode == debugCode + return ginMode == debugCode } func debugRoute(httpMethod, absolutePath string, handlers []HandlerFunc) { diff --git a/gin.go b/gin.go index fa8b12d..7cf4de5 100644 --- a/gin.go +++ b/gin.go @@ -83,8 +83,7 @@ func (engine *Engine) reuseContext(c *Context) { func (engine *Engine) LoadHTMLGlob(pattern string) { if IsDebugging() { - r := &render.HTMLDebugRender{Glob: pattern} - engine.HTMLRender = r + engine.HTMLRender = &render.HTMLDebugRender{Glob: pattern} } else { templ := template.Must(template.ParseGlob(pattern)) engine.SetHTMLTemplate(templ) @@ -93,8 +92,7 @@ func (engine *Engine) LoadHTMLGlob(pattern string) { func (engine *Engine) LoadHTMLFiles(files ...string) { if IsDebugging() { - r := &render.HTMLDebugRender{Files: files} - engine.HTMLRender = r + engine.HTMLRender = &render.HTMLDebugRender{Files: files} } else { templ := template.Must(template.ParseFiles(files...)) engine.SetHTMLTemplate(templ) diff --git a/mode.go b/mode.go index c9ff032..de0a87f 100644 --- a/mode.go +++ b/mode.go @@ -22,8 +22,8 @@ const ( testCode = iota ) -var gin_mode int = debugCode -var mode_name string = DebugMode +var ginMode int = debugCode +var modeName string = DebugMode func init() { value := os.Getenv(GIN_MODE) @@ -37,17 +37,17 @@ func init() { func SetMode(value string) { switch value { case DebugMode: - gin_mode = debugCode + ginMode = debugCode case ReleaseMode: - gin_mode = releaseCode + ginMode = releaseCode case TestMode: - gin_mode = testCode + ginMode = testCode default: log.Panic("gin mode unknown: " + value) } - mode_name = value + modeName = value } func Mode() string { - return mode_name + return modeName } From ee3b67eda1704c7008644d6b3be3c042ee2b1258 Mon Sep 17 00:00:00 2001 From: Manu Mtz-Almeida Date: Tue, 7 Apr 2015 12:30:16 +0200 Subject: [PATCH 03/10] Experimenting with new validation library!!! --- binding/binding.go | 8 ++++- binding/get_form.go | 2 +- binding/json.go | 5 ++- binding/post_form.go | 2 +- binding/validate.go | 79 -------------------------------------------- binding/xml.go | 5 ++- 6 files changed, 13 insertions(+), 88 deletions(-) delete mode 100644 binding/validate.go diff --git a/binding/binding.go b/binding/binding.go index f76efba..26babeb 100644 --- a/binding/binding.go +++ b/binding/binding.go @@ -4,7 +4,11 @@ package binding -import "net/http" +import ( + "net/http" + + "gopkg.in/joeybloggs/go-validate-yourself.v4" +) const ( MIMEJSON = "application/json" @@ -21,6 +25,8 @@ type Binding interface { Bind(*http.Request, interface{}) error } +var _validator = validator.NewValidator("binding", validator.BakedInValidators) + var ( JSON = jsonBinding{} XML = xmlBinding{} diff --git a/binding/get_form.go b/binding/get_form.go index 6226c51..7e0ea94 100644 --- a/binding/get_form.go +++ b/binding/get_form.go @@ -19,5 +19,5 @@ func (_ getFormBinding) Bind(req *http.Request, obj interface{}) error { if err := mapForm(obj, req.Form); err != nil { return err } - return Validate(obj) + return _validator.ValidateStruct(obj) } diff --git a/binding/json.go b/binding/json.go index 731626c..6470e1d 100644 --- a/binding/json.go +++ b/binding/json.go @@ -18,9 +18,8 @@ func (_ jsonBinding) Name() string { func (_ jsonBinding) Bind(req *http.Request, obj interface{}) error { decoder := json.NewDecoder(req.Body) - if err := decoder.Decode(obj); err == nil { - return Validate(obj) - } else { + if err := decoder.Decode(obj); err != nil { return err } + return _validator.ValidateStruct(obj) } diff --git a/binding/post_form.go b/binding/post_form.go index 9a0f0b6..0c876d7 100644 --- a/binding/post_form.go +++ b/binding/post_form.go @@ -19,5 +19,5 @@ func (_ postFormBinding) Bind(req *http.Request, obj interface{}) error { if err := mapForm(obj, req.PostForm); err != nil { return err } - return Validate(obj) + return _validator.ValidateStruct(obj) } diff --git a/binding/validate.go b/binding/validate.go deleted file mode 100644 index b743405..0000000 --- a/binding/validate.go +++ /dev/null @@ -1,79 +0,0 @@ -// Copyright 2014 Manu Martinez-Almeida. All rights reserved. -// Use of this source code is governed by a MIT style -// license that can be found in the LICENSE file. - -package binding - -import ( - "errors" - "reflect" - "strings" -) - -func Validate(obj interface{}) error { - return validate(obj, "{{ROOT}}") -} - -func validate(obj interface{}, parent string) error { - typ, val := inspectObject(obj) - switch typ.Kind() { - case reflect.Struct: - return validateStruct(typ, val, parent) - - case reflect.Slice: - return validateSlice(typ, val, parent) - - default: - return errors.New("The object is not a slice or struct.") - } -} - -func inspectObject(obj interface{}) (typ reflect.Type, val reflect.Value) { - typ = reflect.TypeOf(obj) - val = reflect.ValueOf(obj) - if typ.Kind() == reflect.Ptr { - typ = typ.Elem() - val = val.Elem() - } - return -} - -func validateSlice(typ reflect.Type, val reflect.Value, parent string) error { - if typ.Elem().Kind() == reflect.Struct { - for i := 0; i < val.Len(); i++ { - itemValue := val.Index(i).Interface() - if err := validate(itemValue, parent); err != nil { - return err - } - } - } - return nil -} - -func validateStruct(typ reflect.Type, val reflect.Value, parent string) error { - for i := 0; i < typ.NumField(); i++ { - field := typ.Field(i) - // Allow ignored and unexported fields in the struct - // TODO should include || field.Tag.Get("form") == "-" - if len(field.PkgPath) > 0 { - continue - } - - fieldValue := val.Field(i).Interface() - requiredField := strings.Index(field.Tag.Get("binding"), "required") > -1 - - if requiredField { - zero := reflect.Zero(field.Type).Interface() - if reflect.DeepEqual(zero, fieldValue) { - return errors.New("Required " + field.Name + " in " + parent) - } - } - fieldType := field.Type.Kind() - if fieldType == reflect.Struct || fieldType == reflect.Slice { - if err := validate(fieldValue, field.Name); err != nil { - return err - } - } - } - return nil -} diff --git a/binding/xml.go b/binding/xml.go index b6c07c2..69b38a6 100644 --- a/binding/xml.go +++ b/binding/xml.go @@ -17,9 +17,8 @@ func (_ xmlBinding) Name() string { func (_ xmlBinding) Bind(req *http.Request, obj interface{}) error { decoder := xml.NewDecoder(req.Body) - if err := decoder.Decode(obj); err == nil { - return Validate(obj) - } else { + if err := decoder.Decode(obj); err != nil { return err } + return _validator.ValidateStruct(obj) } From a887e395f3a477fbdfe14dfd3a9b8d5518445143 Mon Sep 17 00:00:00 2001 From: Manu Mtz-Almeida Date: Tue, 7 Apr 2015 16:06:53 +0200 Subject: [PATCH 04/10] Fixes integration with "go-validate-yourself" http://stackoverflow.com/questions/29138591/hiding-nil-values-understanding-why-golang-fails-here --- binding/get_form.go | 5 ++++- binding/json.go | 5 ++++- binding/post_form.go | 5 ++++- binding/xml.go | 5 ++++- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/binding/get_form.go b/binding/get_form.go index 7e0ea94..a171788 100644 --- a/binding/get_form.go +++ b/binding/get_form.go @@ -19,5 +19,8 @@ func (_ getFormBinding) Bind(req *http.Request, obj interface{}) error { if err := mapForm(obj, req.Form); err != nil { return err } - return _validator.ValidateStruct(obj) + if err := _validator.ValidateStruct(obj); err != nil { + return error(err) + } + return nil } diff --git a/binding/json.go b/binding/json.go index 6470e1d..1f38618 100644 --- a/binding/json.go +++ b/binding/json.go @@ -21,5 +21,8 @@ func (_ jsonBinding) Bind(req *http.Request, obj interface{}) error { if err := decoder.Decode(obj); err != nil { return err } - return _validator.ValidateStruct(obj) + if err := _validator.ValidateStruct(obj); err != nil { + return error(err) + } + return nil } diff --git a/binding/post_form.go b/binding/post_form.go index 0c876d7..dfd7381 100644 --- a/binding/post_form.go +++ b/binding/post_form.go @@ -19,5 +19,8 @@ func (_ postFormBinding) Bind(req *http.Request, obj interface{}) error { if err := mapForm(obj, req.PostForm); err != nil { return err } - return _validator.ValidateStruct(obj) + if err := _validator.ValidateStruct(obj); err != nil { + return error(err) + } + return nil } diff --git a/binding/xml.go b/binding/xml.go index 69b38a6..70f6293 100644 --- a/binding/xml.go +++ b/binding/xml.go @@ -20,5 +20,8 @@ func (_ xmlBinding) Bind(req *http.Request, obj interface{}) error { if err := decoder.Decode(obj); err != nil { return err } - return _validator.ValidateStruct(obj) + if err := _validator.ValidateStruct(obj); err != nil { + return error(err) + } + return nil } From 9828435f70032925bc63a1ef1e27d71bb2e1f922 Mon Sep 17 00:00:00 2001 From: Manu Mtz-Almeida Date: Tue, 7 Apr 2015 18:14:33 +0200 Subject: [PATCH 05/10] Fixes failing unit test --- binding/form_mapping.go | 2 -- context_test.go | 55 ----------------------------------------- 2 files changed, 57 deletions(-) diff --git a/binding/form_mapping.go b/binding/form_mapping.go index e406245..a6ac241 100644 --- a/binding/form_mapping.go +++ b/binding/form_mapping.go @@ -6,7 +6,6 @@ package binding import ( "errors" - "fmt" "log" "reflect" "strconv" @@ -27,7 +26,6 @@ func mapForm(ptr interface{}, form map[string][]string) error { inputFieldName = typeField.Name } inputValue, exists := form[inputFieldName] - fmt.Println("Field: "+inputFieldName+" Value: ", inputValue) if !exists { continue diff --git a/context_test.go b/context_test.go index 8585325..6aa794a 100644 --- a/context_test.go +++ b/context_test.go @@ -374,39 +374,6 @@ func TestBindingJSONEncoding(t *testing.T) { } } -func TestBindingJSONNoContentType(t *testing.T) { - - body := bytes.NewBuffer([]byte("{\"foo\":\"bar\"}")) - - r := New() - r.POST("/binding/json", func(c *Context) { - var body struct { - Foo string `json:"foo"` - } - if c.Bind(&body) { - c.JSON(200, H{"parsed": body.Foo}) - } - - }) - - req, _ := http.NewRequest("POST", "/binding/json", body) - w := httptest.NewRecorder() - - r.ServeHTTP(w, req) - - if w.Code != 400 { - t.Errorf("Response code should be Bad request, was: %d", w.Code) - } - - if w.Body.String() == "{\"parsed\":\"bar\"}\n" { - t.Errorf("Response should not be {\"parsed\":\"bar\"}, was: %s", w.Body.String()) - } - - if w.HeaderMap.Get("Content-Type") == "application/json" { - t.Errorf("Content-Type should not be application/json, was %s", w.HeaderMap.Get("Content-Type")) - } -} - func TestBindingJSONMalformed(t *testing.T) { body := bytes.NewBuffer([]byte("\"foo\":\"bar\"\n")) @@ -495,25 +462,3 @@ func TestClientIP(t *testing.T) { t.Errorf("ClientIP should not be %s, but clientip:1234", clientIP) } } - -func TestClientIPWithXForwardedForWithProxy(t *testing.T) { - r := New() - r.Use(ForwardedFor()) - - var clientIP string = "" - r.GET("/", func(c *Context) { - clientIP = c.ClientIP() - }) - - body := bytes.NewBuffer([]byte("")) - req, _ := http.NewRequest("GET", "/", body) - req.RemoteAddr = "172.16.8.3:1234" - req.Header.Set("X-Real-Ip", "realip") - req.Header.Set("X-Forwarded-For", "1.2.3.4, 10.10.0.4, 192.168.0.43, 172.16.8.4") - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - - if clientIP != "1.2.3.4:0" { - t.Errorf("ClientIP should not be %s, but 1.2.3.4:0", clientIP) - } -} From 0b7dce4bc986241f97d90df3f9e7dbd89806307f Mon Sep 17 00:00:00 2001 From: Manu Mtz-Almeida Date: Tue, 7 Apr 2015 18:14:49 +0200 Subject: [PATCH 06/10] Updates godeps --- Godeps/Godeps.json | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 8af74d1..afc04ec 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -1,10 +1,14 @@ { "ImportPath": "github.com/gin-gonic/gin", - "GoVersion": "go1.3", + "GoVersion": "go1.4.2", "Deps": [ { - "ImportPath": "github.com/julienschmidt/httprouter", - "Rev": "b428fda53bb0a764fea9c76c9413512eda291dec" + "ImportPath": "github.com/mattn/go-colorable", + "Rev": "043ae16291351db8465272edf465c9f388161627" + }, + { + "ImportPath": "github.com/stretchr/testify/assert", + "Rev": "de7fcff264cd05cc0c90c509ea789a436a0dd206" } ] } From 6c788a43004f9763178738a2c2f7a6d276fd60d5 Mon Sep 17 00:00:00 2001 From: Manu Mtz-Almeida Date: Tue, 7 Apr 2015 18:37:17 +0200 Subject: [PATCH 07/10] Adds default file log option --- logger.go | 15 ++++++++------- mode.go | 3 +++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/logger.go b/logger.go index 478953a..4418ef8 100644 --- a/logger.go +++ b/logger.go @@ -5,10 +5,9 @@ package gin import ( - "log" + "fmt" + "io" "time" - - "github.com/mattn/go-colorable" ) var ( @@ -39,9 +38,10 @@ func ErrorLoggerT(typ uint32) HandlerFunc { } func Logger() HandlerFunc { - stdlogger := log.New(colorable.NewColorableStdout(), "", 0) - //errlogger := log.New(os.Stderr, "", 0) + return LoggerInFile(DefaultLogFile) +} +func LoggerInFile(out io.Writer) HandlerFunc { return func(c *Context) { // Start timer start := time.Now() @@ -58,15 +58,16 @@ func Logger() HandlerFunc { statusCode := c.Writer.Status() statusColor := colorForStatus(statusCode) methodColor := colorForMethod(method) + comment := c.Errors.String() - stdlogger.Printf("[GIN] %v |%s %3d %s| %12v | %s |%s %s %-7s %s\n%s", + fmt.Fprintf(out, "[GIN] %v |%s %3d %s| %12v | %s |%s %s %-7s %s\n%s", end.Format("2006/01/02 - 15:04:05"), statusColor, statusCode, reset, latency, clientIP, methodColor, reset, method, c.Request.URL.Path, - c.Errors.String(), + comment, ) } } diff --git a/mode.go b/mode.go index de0a87f..21b9ac5 100644 --- a/mode.go +++ b/mode.go @@ -7,6 +7,8 @@ package gin import ( "log" "os" + + "github.com/mattn/go-colorable" ) const GIN_MODE = "GIN_MODE" @@ -22,6 +24,7 @@ const ( testCode = iota ) +var DefaultLogFile = colorable.NewColorableStdout() var ginMode int = debugCode var modeName string = DebugMode From 598c78297c0db605ef34e274a38e9374da77a06a Mon Sep 17 00:00:00 2001 From: Manu Mtz-Almeida Date: Tue, 7 Apr 2015 18:50:16 +0200 Subject: [PATCH 08/10] NoWritten and DefaultStatus must be unexported variables --- response_writer.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/response_writer.go b/response_writer.go index 269ab1b..3e8f54f 100644 --- a/response_writer.go +++ b/response_writer.go @@ -12,8 +12,8 @@ import ( ) const ( - NoWritten = -1 - DefaultStatus = 200 + noWritten = -1 + defaultStatus = 200 ) type ( @@ -38,8 +38,8 @@ type ( func (w *responseWriter) reset(writer http.ResponseWriter) { w.ResponseWriter = writer - w.size = NoWritten - w.status = DefaultStatus + w.size = noWritten + w.status = defaultStatus } func (w *responseWriter) WriteHeader(code int) { @@ -74,7 +74,7 @@ func (w *responseWriter) Size() int { } func (w *responseWriter) Written() bool { - return w.size != NoWritten + return w.size != noWritten } // Implements the http.Hijacker interface From dcdf7b92f457b27459a8a58e3e5a9770b02a9ad9 Mon Sep 17 00:00:00 2001 From: Manu Mtz-Almeida Date: Tue, 7 Apr 2015 18:52:33 +0200 Subject: [PATCH 09/10] Error middleware does not write if the it is already written --- logger.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/logger.go b/logger.go index 4418ef8..0e02600 100644 --- a/logger.go +++ b/logger.go @@ -29,10 +29,11 @@ func ErrorLoggerT(typ uint32) HandlerFunc { return func(c *Context) { c.Next() - errs := c.Errors.ByType(typ) - if len(errs) > 0 { - // -1 status code = do not change current one - c.JSON(-1, c.Errors) + if !c.Writer.Written() { + errs := c.Errors.ByType(typ) + if len(errs) > 0 { + c.JSON(-1, c.Errors) + } } } } From 3fce8efcc6d6fd3e0d5a9cf67e8a816e370ccbe1 Mon Sep 17 00:00:00 2001 From: Manu Mtz-Almeida Date: Tue, 7 Apr 2015 18:56:17 +0200 Subject: [PATCH 10/10] Renames LoggerInFile() to LoggerWithFile() --- logger.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/logger.go b/logger.go index 0e02600..fedfe24 100644 --- a/logger.go +++ b/logger.go @@ -39,10 +39,10 @@ func ErrorLoggerT(typ uint32) HandlerFunc { } func Logger() HandlerFunc { - return LoggerInFile(DefaultLogFile) + return LoggerWithFile(DefaultLogFile) } -func LoggerInFile(out io.Writer) HandlerFunc { +func LoggerWithFile(out io.Writer) HandlerFunc { return func(c *Context) { // Start timer start := time.Now()