From 5a30833b0c41890f186948554310c1f7530f2240 Mon Sep 17 00:00:00 2001 From: Robert Lillack Date: Tue, 23 Mar 2021 11:47:34 +0100 Subject: [PATCH 1/7] Move watcher to internal package. Add tests. #16 --- cmd/mars-gen/main.go | 12 +- errors.go | 6 + watcher.go => internal/watcher/watcher.go | 135 ++++++---------------- internal/watcher/watcher_test.go | 115 ++++++++++++++++++ mars.go | 16 ++- router.go | 20 +++- server.go | 4 +- template.go | 14 ++- util.go | 14 --- watchfilter.go | 12 ++ 10 files changed, 213 insertions(+), 135 deletions(-) rename watcher.go => internal/watcher/watcher.go (51%) create mode 100644 internal/watcher/watcher_test.go create mode 100644 watchfilter.go diff --git a/cmd/mars-gen/main.go b/cmd/mars-gen/main.go index 13a7cc1..872d12a 100644 --- a/cmd/mars-gen/main.go +++ b/cmd/mars-gen/main.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "fmt" "go/format" "os" @@ -9,8 +10,6 @@ import ( "time" "github.com/codegangsta/cli" - - "github.com/roblillack/mars" ) func fatalf(layout string, args ...interface{}) { @@ -109,7 +108,12 @@ func reverseRoutes(ctx *cli.Context) { } func generateSources(tpl, filename string, templateArgs map[string]interface{}) { - sourceCode := mars.ExecuteTemplate(template.Must(template.New("").Parse(tpl)), templateArgs) + var b bytes.Buffer + + tmpl := template.Must(template.New("").Parse(tpl)) + if err := tmpl.Execute(&b, templateArgs); err != nil { + fatalf("Unable to create source file: %v", err) + } if err := os.MkdirAll(path.Dir(filename), 0755); err != nil { fatalf("Unable to create dir: %v", err) @@ -122,7 +126,7 @@ func generateSources(tpl, filename string, templateArgs map[string]interface{}) } defer file.Close() - formatted, err := format.Source([]byte(sourceCode)) + formatted, err := format.Source(b.Bytes()) if err != nil { fatalf("Failed to format file: %v", err) } diff --git a/errors.go b/errors.go index 1e59a83..b740f35 100644 --- a/errors.go +++ b/errors.go @@ -17,6 +17,8 @@ type Error struct { Link string // A configurable link to wrap the error source in } +var _ error = &Error{} + // An object to hold the per-source-line details. type sourceLine struct { Source string @@ -27,6 +29,10 @@ type sourceLine struct { // Construct a plaintext version of the error, taking account that fields are optionally set. // Returns e.g. Compilation Error (in views/header.html:51): expected right delim in end; got "}" func (e *Error) Error() string { + if e == nil { + return "" + } + loc := "" if e.Path != "" { line := "" diff --git a/watcher.go b/internal/watcher/watcher.go similarity index 51% rename from watcher.go rename to internal/watcher/watcher.go index 806349d..c15d3b3 100644 --- a/watcher.go +++ b/internal/watcher/watcher.go @@ -1,6 +1,7 @@ -package mars +package watcher import ( + "fmt" "os" "path" "path/filepath" @@ -14,39 +15,31 @@ import ( type Listener interface { // Refresh is invoked by the watcher on relevant filesystem events. // If the listener returns an error, it is served to the user on the current request. - Refresh() *Error -} - -// DiscerningListener allows the receiver to selectively watch files. -type DiscerningListener interface { - Listener - WatchDir(info os.FileInfo) bool - WatchFile(basename string) bool + Refresh() error } // Watcher allows listeners to register to be notified of changes under a given // directory. type Watcher struct { // Parallel arrays of watcher/listener pairs. - watchers []*fsnotify.Watcher - listeners []Listener - forceRefresh bool - lastError int - notifyMutex sync.Mutex + watchers []*fsnotify.Watcher + listeners []Listener + lastError int + notifyMutex sync.Mutex } -func NewWatcher() *Watcher { +func New() *Watcher { return &Watcher{ - forceRefresh: true, - lastError: -1, + // forceRefresh: true, + lastError: -1, } } // Listen registers for events within the given root directories (recursively). -func (w *Watcher) Listen(listener Listener, roots ...string) { +func (w *Watcher) Listen(listener Listener, roots ...string) error { watcher, err := fsnotify.NewWatcher() if err != nil { - ERROR.Fatal(err) + return err } // Replace the unbuffered Event channel with a buffered one. @@ -70,15 +63,14 @@ func (w *Watcher) Listen(listener Listener, roots ...string) { fi, err := os.Stat(p) if err != nil { - ERROR.Println("Failed to stat watched path", p, ":", err) - continue + return fmt.Errorf("Failed to stat watched path %s: %w", p, err) } // If it is a file, watch that specific file. if !fi.IsDir() { err = watcher.Add(p) if err != nil { - ERROR.Println("Failed to watch", p, ":", err) + return fmt.Errorf("Failed to watch %s: %w", p, err) } continue } @@ -87,93 +79,67 @@ func (w *Watcher) Listen(listener Listener, roots ...string) { watcherWalker = func(path string, info os.FileInfo, err error) error { if err != nil { - ERROR.Println("Error walking path:", err) - return nil + return err } // is it a symlinked template? link, err := os.Lstat(path) if err == nil && link.Mode()&os.ModeSymlink == os.ModeSymlink { - TRACE.Println("Watcher symlink: ", path) // lookup the actual target & check for goodness targetPath, err := filepath.EvalSymlinks(path) if err != nil { - ERROR.Println("Failed to read symlink", err) - return err + return fmt.Errorf("failed to read symlink %s: %w", path, err) } targetInfo, err := os.Stat(targetPath) if err != nil { - ERROR.Println("Failed to stat symlink target", err) - return err + return fmt.Errorf("failed to stat symlink target %s of %s: %w", targetPath, path, err) } // set the template path to the target of the symlink path = targetPath info = targetInfo - filepath.Walk(path, watcherWalker) + if err := filepath.Walk(path, watcherWalker); err != nil { + return err + } } if info.IsDir() { - if dl, ok := listener.(DiscerningListener); ok { - if !dl.WatchDir(info) { - return filepath.SkipDir - } - } - - err = watcher.Add(path) - if err != nil { - ERROR.Println("Failed to watch", path, ":", err) + if err := watcher.Add(path); err != nil { + return err } } return nil } // Else, walk the directory tree. - filepath.Walk(p, watcherWalker) - } - - if w.eagerRebuildEnabled() { - // Create goroutine to notify file changes in real time - go w.NotifyWhenUpdated(listener, watcher) + if err := filepath.Walk(p, watcherWalker); err != nil { + return fmt.Errorf("error walking path %s: %w", p, err) + } } w.watchers = append(w.watchers, watcher) w.listeners = append(w.listeners, listener) -} -// NotifyWhenUpdated notifies the watcher when a file event is received. -func (w *Watcher) NotifyWhenUpdated(listener Listener, watcher *fsnotify.Watcher) { - for { - select { - case ev := <-watcher.Events: - if w.rebuildRequired(ev, listener) { - // Serialize listener.Refresh() calls. - w.notifyMutex.Lock() - listener.Refresh() - w.notifyMutex.Unlock() - } - case <-watcher.Errors: - continue - } - } + return nil } // Notify causes the watcher to forward any change events to listeners. // It returns the first (if any) error returned. -func (w *Watcher) Notify() *Error { +func (w *Watcher) Notify() error { // Serialize Notify() calls. w.notifyMutex.Lock() defer w.notifyMutex.Unlock() - for i, watcher := range w.watchers { - listener := w.listeners[i] + for idx, watcher := range w.watchers { + listener := w.listeners[idx] // Pull all pending events / errors from the watcher. refresh := false for { select { case ev := <-watcher.Events: - if w.rebuildRequired(ev, listener) { + // Ignore changes to dotfiles. + if !strings.HasPrefix(path.Base(ev.Name), ".") { refresh = true } continue @@ -185,50 +151,15 @@ func (w *Watcher) Notify() *Error { break } - if w.forceRefresh || refresh || w.lastError == i { + if refresh || w.lastError == idx { err := listener.Refresh() if err != nil { - w.lastError = i + w.lastError = idx return err } } } - w.forceRefresh = false w.lastError = -1 return nil } - -// If watcher.mode is set to eager, the application is rebuilt immediately -// when a source file is changed. -// This feature is available only in dev mode. -func (w *Watcher) eagerRebuildEnabled() bool { - return Config.BoolDefault("mode.dev", true) && - Config.BoolDefault("watch", true) && - Config.StringDefault("watcher.mode", "normal") == "eager" -} - -func (w *Watcher) rebuildRequired(ev fsnotify.Event, listener Listener) bool { - // Ignore changes to dotfiles. - if strings.HasPrefix(path.Base(ev.Name), ".") { - return false - } - - if dl, ok := listener.(DiscerningListener); ok { - if !dl.WatchFile(ev.Name) || ev.Op&fsnotify.Chmod == fsnotify.Chmod { - return false - } - } - return true -} - -var WatchFilter = func(c *Controller, fc []Filter) { - if MainWatcher != nil { - err := MainWatcher.Notify() - if err != nil { - c.Result = c.RenderError(err) - return - } - } - fc[0](c, fc[1:]) -} diff --git a/internal/watcher/watcher_test.go b/internal/watcher/watcher_test.go new file mode 100644 index 0000000..ab35745 --- /dev/null +++ b/internal/watcher/watcher_test.go @@ -0,0 +1,115 @@ +package watcher + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "testing" + "time" +) + +type SimpleRefresher struct { + Refreshed bool + Error error +} + +func (l *SimpleRefresher) Refresh() error { + l.Refreshed = true + return l.Error +} + +func TestWatcher(t *testing.T) { + w := New() + tmp, err := os.MkdirTemp("", "mars-watcher-test") + if err != nil { + t.Fatal(err) + } + + bla := &SimpleRefresher{} + if err := w.Listen(bla, tmp); err != nil { + t.Errorf("unable to setup listener: %w", err) + } + + if err := w.Notify(); err != nil { + t.Errorf("unable to notify listeners: %w", err) + } + if bla.Refreshed { + t.Error("No changes to tmp dir yet, should not have been refreshed.") + } + + bla.Refreshed = false + if f, err := os.Create(filepath.Join(tmp, "yep.dada")); err != nil { + t.Fatal(err) + } else { + fmt.Fprintln(f, "Hello world!") + f.Close() + } + + time.Sleep(1 * time.Second) + + if err := w.Notify(); err != nil { + t.Errorf("unable to notify listeners: %w", err) + } + if !bla.Refreshed { + t.Error("Should have been refreshed.") + } + + if err := os.RemoveAll(tmp); err != nil { + t.Fatal(err) + } +} + +func TestErrorWhileRefreshing(t *testing.T) { + w := New() + tmp, err := os.MkdirTemp("", "mars-watcher-test") + if err != nil { + t.Fatal(err) + } + + bla := &SimpleRefresher{Error: errors.New("uh-oh something went wrong!!!11")} + if err := w.Listen(bla, tmp); err != nil { + t.Errorf("unable to setup listener: %w", err) + } + + if err := w.Notify(); err != nil { + t.Errorf("unable to notify listeners: %w", err) + } + if bla.Refreshed { + t.Error("No changes to tmp dir yet, should not have been refreshed.") + } + + bla.Refreshed = false + if f, err := os.Create(filepath.Join(tmp, "yep.dada")); err != nil { + t.Fatal(err) + } else { + fmt.Fprintln(f, "Hello world!") + f.Close() + } + + time.Sleep(1 * time.Second) + + if err := w.Notify(); err == nil { + t.Error("No error while refreshing") + } else if err != bla.Error { + t.Error("Wrong error seen while refreshing: %w", err) + } + if !bla.Refreshed { + t.Error("Should have been refreshed.") + } + + bla.Refreshed = false + bla.Error = nil + time.Sleep(1 * time.Second) + + if err := w.Notify(); err != nil { + t.Errorf("error not resovled yet: %w", err) + } + if !bla.Refreshed { + t.Error("Should have been refreshed.") + } + + if err := os.RemoveAll(tmp); err != nil { + t.Fatal(err) + } +} diff --git a/mars.go b/mars.go index b5e5d1b..8ea3784 100644 --- a/mars.go +++ b/mars.go @@ -13,6 +13,8 @@ import ( "github.com/agtorre/gocolorize" "github.com/robfig/config" + + "github.com/roblillack/mars/internal/watcher" ) const ( @@ -219,7 +221,7 @@ func setup() { // The "watch" config variable can turn on and off all watching. // (As a convenient way to control it all together.) if Config.BoolDefault("watch", DevMode) { - MainWatcher = NewWatcher() + MainWatcher = watcher.New() Filters = append([]Filter{WatchFilter}, Filters...) } @@ -247,13 +249,15 @@ func initializeFallbacks() { func SetupViews() { MainTemplateLoader = NewTemplateLoader([]string{path.Join(BasePath, ViewsPath)}) if err := MainTemplateLoader.Refresh(); err != nil { - ERROR.Fatalln(err.Error()) + ERROR.Fatalln(err) } // If desired (or by default), create a watcher for templates and routes. // The watcher calls Refresh() on things on the first request. if MainWatcher != nil && Config.BoolDefault("watch.templates", true) { - MainWatcher.Listen(MainTemplateLoader, MainTemplateLoader.paths...) + if err := MainWatcher.Listen(MainTemplateLoader, MainTemplateLoader.paths...); err != nil { + ERROR.Fatalln(err) + } } } @@ -263,13 +267,15 @@ func SetupViews() { func SetupRouter() { MainRouter = NewRouter(filepath.Join(BasePath, RoutesFile)) if err := MainRouter.Refresh(); err != nil { - ERROR.Fatalln(err.Error()) + ERROR.Fatalln(err) } // If desired (or by default), create a watcher for templates and routes. // The watcher calls Refresh() on things on the first request. if MainWatcher != nil && Config.BoolDefault("watch.routes", true) { - MainWatcher.Listen(MainRouter, MainRouter.path) + if err := MainWatcher.Listen(MainRouter, MainRouter.path); err != nil { + ERROR.Fatalln(err) + } } } diff --git a/router.go b/router.go index b70ee31..a029158 100644 --- a/router.go +++ b/router.go @@ -144,17 +144,23 @@ func (router *Router) Route(req *http.Request) *RouteMatch { // Refresh re-reads the routes file and re-calculates the routing table. // Returns an error if a specified action could not be found. -func (router *Router) Refresh() (err *Error) { - router.Routes, err = parseRoutesFile(router.path, "", true) - if err != nil { - return +func (router *Router) Refresh() error { + if r, err := parseRoutesFile(router.path, "", true); err != nil { + return err + } else { + router.Routes = r } - err = router.updateTree() - return + + if err := router.updateTree(); err != nil { + return err + } + + return nil } func (router *Router) updateTree() *Error { router.Tree = pathtree.New() + for _, route := range router.Routes { err := router.Tree.Add(route.TreePath(), route) @@ -168,6 +174,7 @@ func (router *Router) updateTree() *Error { return routeError(err, route.routesPath, "", route.line) } } + return nil } @@ -180,6 +187,7 @@ func parseRoutesFile(routesPath, joinedPath string, validate bool) ([]*Route, *E Description: err.Error(), } } + return parseRoutes(routesPath, joinedPath, string(contentBytes), validate) } diff --git a/server.go b/server.go index 2ff102b..960f2c8 100644 --- a/server.go +++ b/server.go @@ -12,12 +12,14 @@ import ( "time" "golang.org/x/net/websocket" + + "github.com/roblillack/mars/internal/watcher" ) var ( MainRouter *Router MainTemplateLoader *TemplateLoader - MainWatcher *Watcher + MainWatcher *watcher.Watcher Server *http.Server SecureServer *http.Server ) diff --git a/template.go b/template.go index 2d4a0aa..170e692 100644 --- a/template.go +++ b/template.go @@ -223,7 +223,7 @@ func (loader *TemplateLoader) createEmptyTemplateSet() *Error { // This scans the views directory and parses all templates as Go Templates. // If a template fails to parse, the error is set on the loader. // (It's awkward to refresh a single Go Template) -func (loader *TemplateLoader) Refresh() *Error { +func (loader *TemplateLoader) Refresh() error { TRACE.Printf("Refreshing templates from %s", loader.paths) loader.compileError = nil @@ -357,7 +357,11 @@ func (loader *TemplateLoader) Refresh() *Error { // If there was an error with the Funcs, set it and return immediately. if funcErr != nil { loader.compileError = funcErr.(*Error) - return loader.compileError + if loader.compileError == nil { + return nil + } else { + return loader.compileError + } } } @@ -379,7 +383,11 @@ func (loader *TemplateLoader) Refresh() *Error { } } - return loader.compileError + if loader.compileError == nil { + return nil + } else { + return loader.compileError + } } func (loader *TemplateLoader) WatchDir(info os.FileInfo) bool { diff --git a/util.go b/util.go index c0013a5..8a62d41 100644 --- a/util.go +++ b/util.go @@ -1,8 +1,6 @@ package mars import ( - "bytes" - "io" "io/ioutil" "net/url" "os" @@ -11,18 +9,6 @@ import ( "strings" ) -// Add some more methods to the default Template. -type ExecutableTemplate interface { - Execute(io.Writer, interface{}) error -} - -// Execute a template and returns the result as a string. -func ExecuteTemplate(tmpl ExecutableTemplate, data interface{}) string { - var b bytes.Buffer - tmpl.Execute(&b, data) - return b.String() -} - // Reads the lines of the given file. Panics in the case of error. func MustReadLines(filename string) []string { r, err := ReadLines(filename) diff --git a/watchfilter.go b/watchfilter.go new file mode 100644 index 0000000..ba2f231 --- /dev/null +++ b/watchfilter.go @@ -0,0 +1,12 @@ +package mars + +var WatchFilter = func(c *Controller, fc []Filter) { + if MainWatcher != nil { + err := MainWatcher.Notify() + if err != nil { + c.Result = c.RenderError(err) + return + } + } + fc[0](c, fc[1:]) +} From ee0bc9fe505e39ee69abad27b70d32da154f8ccf Mon Sep 17 00:00:00 2001 From: Robert Lillack Date: Tue, 23 Mar 2021 12:19:54 +0100 Subject: [PATCH 2/7] Make watcher tests work with older Go releases --- internal/watcher/watcher_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/watcher/watcher_test.go b/internal/watcher/watcher_test.go index ab35745..78b4665 100644 --- a/internal/watcher/watcher_test.go +++ b/internal/watcher/watcher_test.go @@ -3,6 +3,7 @@ package watcher import ( "errors" "fmt" + "math/rand" "os" "path/filepath" "testing" @@ -21,7 +22,9 @@ func (l *SimpleRefresher) Refresh() error { func TestWatcher(t *testing.T) { w := New() - tmp, err := os.MkdirTemp("", "mars-watcher-test") + + tmp := filepath.Join(os.TempDir(), fmt.Sprintf("mars-watcher-test-%d", rand.Uint32())) + err := os.MkdirAll(tmp, 0700) if err != nil { t.Fatal(err) } @@ -62,7 +65,9 @@ func TestWatcher(t *testing.T) { func TestErrorWhileRefreshing(t *testing.T) { w := New() - tmp, err := os.MkdirTemp("", "mars-watcher-test") + + tmp := filepath.Join(os.TempDir(), fmt.Sprintf("mars-watcher-test-%d", rand.Uint32())) + err := os.MkdirAll(tmp, 0700) if err != nil { t.Fatal(err) } From 335e0ebca9ac626a8797899d81a0737e2945a1c0 Mon Sep 17 00:00:00 2001 From: Robert Lillack Date: Tue, 23 Mar 2021 12:21:46 +0100 Subject: [PATCH 3/7] Set minimum Go version to 1.13 --- .travis.yml | 1 - go.mod | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0ccea7a..2703527 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,6 @@ env: - GO111MODULE=on go: - - 1.12 - 1.13 - 1.14 - 1.15 diff --git a/go.mod b/go.mod index fe20880..f1290bb 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/roblillack/mars -go 1.12 +go 1.13 require ( github.com/agtorre/gocolorize v1.0.0 From 648864ed02fdbd9056390487d35785c78238d270 Mon Sep 17 00:00:00 2001 From: Robert Lillack Date: Tue, 23 Mar 2021 13:51:05 +0100 Subject: [PATCH 4/7] Hide main watcher as this is an internal API --- mars.go | 10 +++++----- server.go | 2 +- watchfilter.go | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/mars.go b/mars.go index 8ea3784..27a85dc 100644 --- a/mars.go +++ b/mars.go @@ -221,7 +221,7 @@ func setup() { // The "watch" config variable can turn on and off all watching. // (As a convenient way to control it all together.) if Config.BoolDefault("watch", DevMode) { - MainWatcher = watcher.New() + mainWatcher = watcher.New() Filters = append([]Filter{WatchFilter}, Filters...) } @@ -254,8 +254,8 @@ func SetupViews() { // If desired (or by default), create a watcher for templates and routes. // The watcher calls Refresh() on things on the first request. - if MainWatcher != nil && Config.BoolDefault("watch.templates", true) { - if err := MainWatcher.Listen(MainTemplateLoader, MainTemplateLoader.paths...); err != nil { + if mainWatcher != nil && Config.BoolDefault("watch.templates", true) { + if err := mainWatcher.Listen(MainTemplateLoader, MainTemplateLoader.paths...); err != nil { ERROR.Fatalln(err) } } @@ -272,8 +272,8 @@ func SetupRouter() { // If desired (or by default), create a watcher for templates and routes. // The watcher calls Refresh() on things on the first request. - if MainWatcher != nil && Config.BoolDefault("watch.routes", true) { - if err := MainWatcher.Listen(MainRouter, MainRouter.path); err != nil { + if mainWatcher != nil && Config.BoolDefault("watch.routes", true) { + if err := mainWatcher.Listen(MainRouter, MainRouter.path); err != nil { ERROR.Fatalln(err) } } diff --git a/server.go b/server.go index 960f2c8..8e613f9 100644 --- a/server.go +++ b/server.go @@ -19,7 +19,7 @@ import ( var ( MainRouter *Router MainTemplateLoader *TemplateLoader - MainWatcher *watcher.Watcher + mainWatcher *watcher.Watcher Server *http.Server SecureServer *http.Server ) diff --git a/watchfilter.go b/watchfilter.go index ba2f231..4c7edb5 100644 --- a/watchfilter.go +++ b/watchfilter.go @@ -1,8 +1,8 @@ package mars var WatchFilter = func(c *Controller, fc []Filter) { - if MainWatcher != nil { - err := MainWatcher.Notify() + if mainWatcher != nil { + err := mainWatcher.Notify() if err != nil { c.Result = c.RenderError(err) return From ba9ed71fa7fbf2fb8277dede4792aa3d6091d8fd Mon Sep 17 00:00:00 2001 From: Robert Lillack Date: Tue, 23 Mar 2021 14:39:00 +0100 Subject: [PATCH 5/7] Move some utility functions around --- cmd/mars-gen/reflect.go | 25 ++++-- cookie.go | 20 +++++ filterconfig.go | 2 +- flash.go | 2 +- reflection.go | 18 ++++ reflection_test.go | 36 ++++++++ results.go | 2 +- session.go | 2 +- template.go | 11 ++- testing/equal.go | 43 ++++++++++ util_test.go => testing/equal_test.go | 2 +- testing/testsuite.go | 8 +- util.go | 119 -------------------------- validation.go | 2 +- 14 files changed, 157 insertions(+), 135 deletions(-) create mode 100644 cookie.go create mode 100644 reflection.go create mode 100644 reflection_test.go create mode 100644 testing/equal.go rename util_test.go => testing/equal_test.go (99%) delete mode 100644 util.go diff --git a/cmd/mars-gen/reflect.go b/cmd/mars-gen/reflect.go index aaa9920..c0ceee2 100644 --- a/cmd/mars-gen/reflect.go +++ b/cmd/mars-gen/reflect.go @@ -33,8 +33,6 @@ type SourceInfo struct { // controllerSpecs lists type info for all structs found under // app/controllers/... that embed (directly or indirectly) mars.Controller controllerSpecs []*TypeInfo - // testSuites list the types that constitute the set of application tests. - testSuites []*TypeInfo } // TypeInfo summarizes information about a struct type in the app source code. @@ -445,6 +443,15 @@ func getStructTypeDecl(decl ast.Decl, fset *token.FileSet) (spec *ast.TypeSpec, return } +func containsString(list []string, target string) bool { + for _, el := range list { + if el == target { + return true + } + } + return false +} + // TypesThatEmbed returns all types that (directly or indirectly) embed the // target type, which must be a fully qualified type name, // e.g. "github.com/roblillack/mars.Controller" @@ -462,8 +469,7 @@ func (s *SourceInfo) TypesThatEmbed(targetType string) (filtered []*TypeInfo) { // Look through all known structs. for _, spec := range s.StructSpecs { // If this one has been processed or is already in nodeQueue, then skip it. - if mars.ContainsString(processed, spec.String()) || - mars.ContainsString(nodeQueue, spec.String()) { + if containsString(processed, spec.String()) || containsString(nodeQueue, spec.String()) { continue } @@ -502,10 +508,19 @@ type TypeExpr struct { Valid bool } +func firstNonEmpty(strs ...string) string { + for _, str := range strs { + if len(str) > 0 { + return str + } + } + return "" +} + // TypeName returns the fully-qualified type name for this expression. // The caller may optionally specify a package name to override the default. func (e TypeExpr) TypeName(pkgOverride string) string { - pkgName := mars.FirstNonEmpty(pkgOverride, e.PkgName) + pkgName := firstNonEmpty(pkgOverride, e.PkgName) if pkgName == "" { return e.Expr } diff --git a/cookie.go b/cookie.go new file mode 100644 index 0000000..c88ab6a --- /dev/null +++ b/cookie.go @@ -0,0 +1,20 @@ +package mars + +import ( + "net/url" + "regexp" +) + +var ( + cookieKeyValueParser = regexp.MustCompile("\x00([^:]*):([^\x00]*)\x00") +) + +// parseKeyValueCookie takes the raw (escaped) cookie value and parses out key values. +func parseKeyValueCookie(val string, cb func(key, val string)) { + val, _ = url.QueryUnescape(val) + if matches := cookieKeyValueParser.FindAllStringSubmatch(val, -1); matches != nil { + for _, match := range matches { + cb(match[1], match[2]) + } + } +} diff --git a/filterconfig.go b/filterconfig.go index 0ecc41a..b4e7e43 100644 --- a/filterconfig.go +++ b/filterconfig.go @@ -85,7 +85,7 @@ func FilterAction(methodRef interface{}) FilterConfigurator { } controllerType := methodType.In(0) - method := FindMethod(controllerType, methodValue) + method := findMethod(controllerType, methodValue) if method == nil { panic("Action not found on controller " + controllerType.Name()) } diff --git a/flash.go b/flash.go index 51b2c8b..00f8c95 100644 --- a/flash.go +++ b/flash.go @@ -66,7 +66,7 @@ func restoreFlash(req *http.Request) Flash { Out: make(map[string]string), } if cookie, err := req.Cookie(CookiePrefix + "_FLASH"); err == nil { - ParseKeyValueCookie(cookie.Value, func(key, val string) { + parseKeyValueCookie(cookie.Value, func(key, val string) { flash.Data[key] = val }) } diff --git a/reflection.go b/reflection.go new file mode 100644 index 0000000..d58962a --- /dev/null +++ b/reflection.go @@ -0,0 +1,18 @@ +package mars + +import ( + "reflect" +) + +// Return the reflect.Method, given a Receiver type and Func value. +func findMethod(recvType reflect.Type, funcVal reflect.Value) *reflect.Method { + // It is not possible to get the name of the method from the Func. + // Instead, compare it to each method of the Controller. + for i := 0; i < recvType.NumMethod(); i++ { + method := recvType.Method(i) + if method.Func.Pointer() == funcVal.Pointer() { + return &method + } + } + return nil +} diff --git a/reflection_test.go b/reflection_test.go new file mode 100644 index 0000000..34b217c --- /dev/null +++ b/reflection_test.go @@ -0,0 +1,36 @@ +package mars + +import ( + "reflect" + "testing" +) + +type T struct{} + +func (t *T) Hello() {} + +func TestFindMethod(t *testing.T) { + for name, tv := range map[string]struct { + reflect.Type + reflect.Value + }{ + "Hello": {reflect.TypeOf(&T{}), reflect.ValueOf((*T).Hello)}, + "Helper": {reflect.TypeOf(t), reflect.ValueOf((*testing.T).Helper)}, + "": {reflect.TypeOf(t), reflect.ValueOf((reflect.Type).Comparable)}, + } { + m := findMethod(tv.Type, tv.Value) + if name == "" { + if m != nil { + t.Errorf("method found that shouldn't be here: %v", m) + } + continue + } + if m == nil { + t.Errorf("No method found when looking for %s", name) + continue + } + if m.Name != name { + t.Errorf("Expected method %s, got %s: %v", name, m.Name, m) + } + } +} diff --git a/results.go b/results.go index 9e83ee3..4b8d88d 100644 --- a/results.go +++ b/results.go @@ -387,7 +387,7 @@ func getRedirectUrl(item interface{}) (string, error) { if typ.Kind() == reflect.Func && typ.NumIn() > 0 { // Get the Controller Method recvType := typ.In(0) - method := FindMethod(recvType, val) + method := findMethod(recvType, val) if method == nil { return "", errors.New("couldn't find method") } diff --git a/session.go b/session.go index e1031ea..af7437b 100644 --- a/session.go +++ b/session.go @@ -125,7 +125,7 @@ func GetSessionFromCookie(cookie *http.Cookie) Session { return session } - ParseKeyValueCookie(data, func(key, val string) { + parseKeyValueCookie(data, func(key, val string) { session[key] = val }) diff --git a/template.go b/template.go index 170e692..75db6f8 100644 --- a/template.go +++ b/template.go @@ -470,6 +470,15 @@ func (loader *TemplateLoader) Template(name string, funcMaps ...Args) (Template, return GoTemplate{tmpl, loader, funcMap}, err } +// Reads the lines of the given file. +func readLines(filename string) ([]string, error) { + bytes, err := ioutil.ReadFile(filename) + if err != nil { + return nil, err + } + return strings.Split(string(bytes), "\n"), nil +} + // Adapter for Go Templates. type GoTemplate struct { *template.Template @@ -487,7 +496,7 @@ func (gotmpl GoTemplate) Render(wr io.Writer, arg interface{}) error { } func (gotmpl GoTemplate) Content() []string { - content, _ := ReadLines(gotmpl.loader.templatePaths[gotmpl.Name()]) + content, _ := readLines(gotmpl.loader.templatePaths[gotmpl.Name()]) return content } diff --git a/testing/equal.go b/testing/equal.go new file mode 100644 index 0000000..15021fa --- /dev/null +++ b/testing/equal.go @@ -0,0 +1,43 @@ +package testing + +import "reflect" + +// Equal is a helper for comparing value equality, following these rules: +// - Values with equivalent types are compared with reflect.DeepEqual +// - int, uint, and float values are compared without regard to the type width. +// for example, Equal(int32(5), int64(5)) == true +// - strings and byte slices are converted to strings before comparison. +// - else, return false. +func Equal(a, b interface{}) bool { + if reflect.TypeOf(a) == reflect.TypeOf(b) { + return reflect.DeepEqual(a, b) + } + switch a.(type) { + case int, int8, int16, int32, int64: + switch b.(type) { + case int, int8, int16, int32, int64: + return reflect.ValueOf(a).Int() == reflect.ValueOf(b).Int() + } + case uint, uint8, uint16, uint32, uint64: + switch b.(type) { + case uint, uint8, uint16, uint32, uint64: + return reflect.ValueOf(a).Uint() == reflect.ValueOf(b).Uint() + } + case float32, float64: + switch b.(type) { + case float32, float64: + return reflect.ValueOf(a).Float() == reflect.ValueOf(b).Float() + } + case string: + switch b.(type) { + case []byte: + return a.(string) == string(b.([]byte)) + } + case []byte: + switch b.(type) { + case string: + return b.(string) == string(a.([]byte)) + } + } + return false +} diff --git a/util_test.go b/testing/equal_test.go similarity index 99% rename from util_test.go rename to testing/equal_test.go index f4056ef..0b8e3c6 100644 --- a/util_test.go +++ b/testing/equal_test.go @@ -1,4 +1,4 @@ -package mars +package testing import ( "reflect" diff --git a/testing/testsuite.go b/testing/testsuite.go index f273dbd..5b9036a 100644 --- a/testing/testsuite.go +++ b/testing/testsuite.go @@ -16,9 +16,9 @@ import ( "regexp" "strings" - "github.com/roblillack/mars" - "golang.org/x/net/websocket" + + "github.com/roblillack/mars" ) type TestSuite struct { @@ -276,13 +276,13 @@ func (t *TestSuite) AssertHeader(name, value string) { } func (t *TestSuite) AssertEqual(expected, actual interface{}) { - if !mars.Equal(expected, actual) { + if !Equal(expected, actual) { panic(fmt.Errorf("(expected) %v != %v (actual)", expected, actual)) } } func (t *TestSuite) AssertNotEqual(expected, actual interface{}) { - if mars.Equal(expected, actual) { + if Equal(expected, actual) { panic(fmt.Errorf("(expected) %v == %v (actual)", expected, actual)) } } diff --git a/util.go b/util.go deleted file mode 100644 index 8a62d41..0000000 --- a/util.go +++ /dev/null @@ -1,119 +0,0 @@ -package mars - -import ( - "io/ioutil" - "net/url" - "os" - "reflect" - "regexp" - "strings" -) - -// Reads the lines of the given file. Panics in the case of error. -func MustReadLines(filename string) []string { - r, err := ReadLines(filename) - if err != nil { - panic(err) - } - return r -} - -// Reads the lines of the given file. Panics in the case of error. -func ReadLines(filename string) ([]string, error) { - bytes, err := ioutil.ReadFile(filename) - if err != nil { - return nil, err - } - return strings.Split(string(bytes), "\n"), nil -} - -func ContainsString(list []string, target string) bool { - for _, el := range list { - if el == target { - return true - } - } - return false -} - -// Return the reflect.Method, given a Receiver type and Func value. -func FindMethod(recvType reflect.Type, funcVal reflect.Value) *reflect.Method { - // It is not possible to get the name of the method from the Func. - // Instead, compare it to each method of the Controller. - for i := 0; i < recvType.NumMethod(); i++ { - method := recvType.Method(i) - if method.Func.Pointer() == funcVal.Pointer() { - return &method - } - } - return nil -} - -var ( - cookieKeyValueParser = regexp.MustCompile("\x00([^:]*):([^\x00]*)\x00") -) - -// Takes the raw (escaped) cookie value and parses out key values. -func ParseKeyValueCookie(val string, cb func(key, val string)) { - val, _ = url.QueryUnescape(val) - if matches := cookieKeyValueParser.FindAllStringSubmatch(val, -1); matches != nil { - for _, match := range matches { - cb(match[1], match[2]) - } - } -} - -// DirExists returns true if the given path exists and is a directory. -func DirExists(filename string) bool { - fileInfo, err := os.Stat(filename) - return err == nil && fileInfo.IsDir() -} - -func FirstNonEmpty(strs ...string) string { - for _, str := range strs { - if len(str) > 0 { - return str - } - } - return "" -} - -// Equal is a helper for comparing value equality, following these rules: -// - Values with equivalent types are compared with reflect.DeepEqual -// - int, uint, and float values are compared without regard to the type width. -// for example, Equal(int32(5), int64(5)) == true -// - strings and byte slices are converted to strings before comparison. -// - else, return false. -func Equal(a, b interface{}) bool { - if reflect.TypeOf(a) == reflect.TypeOf(b) { - return reflect.DeepEqual(a, b) - } - switch a.(type) { - case int, int8, int16, int32, int64: - switch b.(type) { - case int, int8, int16, int32, int64: - return reflect.ValueOf(a).Int() == reflect.ValueOf(b).Int() - } - case uint, uint8, uint16, uint32, uint64: - switch b.(type) { - case uint, uint8, uint16, uint32, uint64: - return reflect.ValueOf(a).Uint() == reflect.ValueOf(b).Uint() - } - case float32, float64: - switch b.(type) { - case float32, float64: - return reflect.ValueOf(a).Float() == reflect.ValueOf(b).Float() - } - case string: - switch b.(type) { - case []byte: - return a.(string) == string(b.([]byte)) - } - case []byte: - switch b.(type) { - case string: - return b.(string) == string(a.([]byte)) - } - } - return false -} diff --git a/validation.go b/validation.go index 8732bd4..de44474 100644 --- a/validation.go +++ b/validation.go @@ -235,7 +235,7 @@ func restoreValidationErrors(req *http.Request) ([]*ValidationError, error) { errors = make([]*ValidationError, 0, 5) ) if cookie, err = req.Cookie(CookiePrefix + "_ERRORS"); err == nil { - ParseKeyValueCookie(cookie.Value, func(key, val string) { + parseKeyValueCookie(cookie.Value, func(key, val string) { errors = append(errors, &ValidationError{ Key: key, Message: val, From 836f56e822244e85d754f5fae8f8b2d290cc5cef Mon Sep 17 00:00:00 2001 From: Robert Lillack Date: Tue, 23 Mar 2021 14:46:42 +0100 Subject: [PATCH 6/7] more cleanups --- router.go | 6 ------ static.go | 12 ++++++------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/router.go b/router.go index a029158..654c7d6 100644 --- a/router.go +++ b/router.go @@ -33,12 +33,6 @@ type RouteMatch struct { Params map[string][]string // e.g. {id: 123} } -type arg struct { - name string - index int - constraint *regexp.Regexp -} - // Prepares the route to be used in matching. func NewRoute(method, path, action, fixedArgs, routesPath string, line int) (r *Route) { diff --git a/static.go b/static.go index 9567141..445f93b 100644 --- a/static.go +++ b/static.go @@ -16,18 +16,18 @@ type Static struct { func init() { RegisterController((*Static)(nil), []*MethodType{ - &MethodType{ + { Name: "ServeFresh", Args: []*MethodArg{ - &MethodArg{Name: "prefix", Type: reflect.TypeOf((*string)(nil))}, - &MethodArg{Name: "filepath", Type: reflect.TypeOf((*string)(nil))}, + {Name: "prefix", Type: reflect.TypeOf((*string)(nil))}, + {Name: "filepath", Type: reflect.TypeOf((*string)(nil))}, }, }, - &MethodType{ + { Name: "Serve", Args: []*MethodArg{ - &MethodArg{Name: "prefix", Type: reflect.TypeOf((*string)(nil))}, - &MethodArg{Name: "filepath", Type: reflect.TypeOf((*string)(nil))}, + {Name: "prefix", Type: reflect.TypeOf((*string)(nil))}, + {Name: "filepath", Type: reflect.TypeOf((*string)(nil))}, }, }, }, From c16eb73a5d7d0f889606c19d2b147b7aadb8cb31 Mon Sep 17 00:00:00 2001 From: Robert Lillack Date: Tue, 23 Mar 2021 14:57:32 +0100 Subject: [PATCH 7/7] Simplify code a bit --- LICENSE | 2 +- binder.go | 2 +- binder_test.go | 12 ++-- cert_test.go | 4 +- fakeapp_test.go | 14 ++-- i18n.go | 2 +- internal/pathtree/tree_test.go | 2 +- router_test.go | 120 +++++++++++++++---------------- validators_test.go | 126 ++++++++++++++++----------------- 9 files changed, 142 insertions(+), 142 deletions(-) diff --git a/LICENSE b/LICENSE index a0529e9..b741914 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2015–2020 Rob Lillack +Copyright (c) 2015–2021 Rob Lillack Copyright (C) 2012 Rob Figueiredo All Rights Reserved. diff --git a/binder.go b/binder.go index b42eefe..372d57f 100644 --- a/binder.go +++ b/binder.go @@ -329,7 +329,7 @@ func unbindSlice(output map[string]string, name string, val interface{}) { func bindStruct(params *Params, name string, typ reflect.Type) reflect.Value { result := reflect.New(typ).Elem() fieldValues := make(map[string]reflect.Value) - for key, _ := range params.Values { + for key := range params.Values { if !strings.HasPrefix(key, name+".") { continue } diff --git a/binder_test.go b/binder_test.go index a1cd90e..a215ba0 100644 --- a/binder_test.go +++ b/binder_test.go @@ -274,18 +274,18 @@ var unbinderTestCases = map[string]interface{}{ // Some of the unbinding results are not exactly what is in PARAMS, since it // serializes implicit zero values explicitly. var unbinderOverrideAnswers = map[string]map[string]string{ - "arr": map[string]string{ + "arr": { "arr[0]": "1", "arr[1]": "2", "arr[2]": "0", "arr[3]": "3", }, - "A": map[string]string{ + "A": { "A.ID": "123", "A.Name": "rob", "A.B.Extra": "", }, - "arrC": map[string]string{ + "arrC": { "arrC[0].ID": "5", "arrC[0].Name": "rob", "arrC[0].B.Extra": "foo", @@ -293,9 +293,9 @@ var unbinderOverrideAnswers = map[string]map[string]string{ "arrC[1].Name": "bill", "arrC[1].B.Extra": "", }, - "m": map[string]string{"m[a]": "foo", "m[b]": "bar"}, - "m2": map[string]string{"m2[1]": "foo", "m2[2]": "bar"}, - "m3": map[string]string{"m3[a]": "1", "m3[b]": "2"}, + "m": {"m[a]": "foo", "m[b]": "bar"}, + "m2": {"m2[1]": "foo", "m2[2]": "bar"}, + "m3": {"m3[a]": "1", "m3[b]": "2"}, } func TestUnbinder(t *testing.T) { diff --git a/cert_test.go b/cert_test.go index e8e6fac..0db48e2 100644 --- a/cert_test.go +++ b/cert_test.go @@ -8,8 +8,8 @@ import ( func TestCertificateCreation(t *testing.T) { for org, domains := range map[string][]string{ - "ACME Inc.": []string{"acme.com", "acme.biz"}, - "Me": []string{"::1", "127.0.0.1"}, + "ACME Inc.": {"acme.com", "acme.biz"}, + "Me": {"::1", "127.0.0.1"}, } { keypair, err := createCertificate(org, strings.Join(domains, ", ")) if err != nil { diff --git a/fakeapp_test.go b/fakeapp_test.go index 22b7f22..f7dc39b 100644 --- a/fakeapp_test.go +++ b/fakeapp_test.go @@ -64,19 +64,19 @@ func (c MyStatic) Serve(prefix, filepath string) Result { func startFakeBookingApp() { RegisterController((*Hotels)(nil), []*MethodType{ - &MethodType{ + { Name: "Index", }, - &MethodType{ + { Name: "Boom", }, - &MethodType{ + { Name: "Show", Args: []*MethodArg{ {"id", reflect.TypeOf((*int)(nil))}, }, }, - &MethodType{ + { Name: "Book", Args: []*MethodArg{ {"id", reflect.TypeOf((*int)(nil))}, @@ -86,11 +86,11 @@ func startFakeBookingApp() { RegisterController((*Static)(nil), []*MethodType{ - &MethodType{ + { Name: "Serve", Args: []*MethodArg{ - &MethodArg{Name: "prefix", Type: reflect.TypeOf((*string)(nil))}, - &MethodArg{Name: "filepath", Type: reflect.TypeOf((*string)(nil))}, + {Name: "prefix", Type: reflect.TypeOf((*string)(nil))}, + {Name: "filepath", Type: reflect.TypeOf((*string)(nil))}, }, }, }) diff --git a/i18n.go b/i18n.go index 7d3a6de..ecf393c 100644 --- a/i18n.go +++ b/i18n.go @@ -31,7 +31,7 @@ var ( func MessageLanguages() []string { languages := make([]string, len(messages)) i := 0 - for language, _ := range messages { + for language := range messages { languages[i] = language i++ } diff --git a/internal/pathtree/tree_test.go b/internal/pathtree/tree_test.go index 59f14b2..5efda23 100644 --- a/internal/pathtree/tree_test.go +++ b/internal/pathtree/tree_test.go @@ -182,7 +182,7 @@ func BenchmarkTree100(b *testing.B) { b.ResetTimer() for i := 0; i < b.N/len(queries); i++ { - for k, _ := range queries { + for k := range queries { n.Find(k) } } diff --git a/router_test.go b/router_test.go index e3b2572..ff2ec3c 100644 --- a/router_test.go +++ b/router_test.go @@ -12,42 +12,42 @@ import ( // Data-driven tests that check that a given routes-file line translates into // the expected Route object. var routeTestCases = map[string]*Route{ - "get / Application.Index": &Route{ + "get / Application.Index": { Method: "GET", Path: "/", Action: "Application.Index", FixedParams: []string{}, }, - "post /app/:id Application.SaveApp": &Route{ + "post /app/:id Application.SaveApp": { Method: "POST", Path: "/app/:id", Action: "Application.SaveApp", FixedParams: []string{}, }, - "get /app/ Application.List": &Route{ + "get /app/ Application.List": { Method: "GET", Path: "/app/", Action: "Application.List", FixedParams: []string{}, }, - `get /app/:appId/ Application.Show`: &Route{ + `get /app/:appId/ Application.Show`: { Method: "GET", Path: `/app/:appId/`, Action: "Application.Show", FixedParams: []string{}, }, - `get /app-wild/*appId/ Application.WildShow`: &Route{ + `get /app-wild/*appId/ Application.WildShow`: { Method: "GET", Path: `/app-wild/*appId/`, Action: "Application.WildShow", FixedParams: []string{}, }, - `GET /public/:filepath Static.Serve("public")`: &Route{ + `GET /public/:filepath Static.Serve("public")`: { Method: "GET", Path: "/public/:filepath", Action: "Static.Serve", @@ -56,7 +56,7 @@ var routeTestCases = map[string]*Route{ }, }, - `GET /javascript/:filepath Static.Serve("public/js")`: &Route{ + `GET /javascript/:filepath Static.Serve("public/js")`: { Method: "GET", Path: "/javascript/:filepath", Action: "Static.Serve", @@ -65,28 +65,28 @@ var routeTestCases = map[string]*Route{ }, }, - "GET /files/:id.:extension Application.DownloadFile": &Route{ + "GET /files/:id.:extension Application.DownloadFile": { Method: "GET", Path: "/files/:id.:extension", Action: "Application.DownloadFile", FixedParams: []string{}, }, - "* /apps/:id/:action Application.:action": &Route{ + "* /apps/:id/:action Application.:action": { Method: "*", Path: "/apps/:id/:action", Action: "Application.:action", FixedParams: []string{}, }, - "* /:controller/:action :controller.:action": &Route{ + "* /:controller/:action :controller.:action": { Method: "*", Path: "/:controller/:action", Action: ":controller.:action", FixedParams: []string{}, }, - `GET / Application.Index("Test", "Test2")`: &Route{ + `GET / Application.Index("Test", "Test2")`: { Method: "GET", Path: "/", Action: "Application.Index", @@ -135,110 +135,110 @@ GET /favicon.ico 404 ` var routeMatchTestCases = map[*http.Request]*RouteMatch{ - &http.Request{ + { Method: "GET", URL: &url.URL{Path: "/"}, - }: &RouteMatch{ + }: { ControllerName: "Application", MethodName: "Index", FixedParams: []string{}, Params: map[string][]string{}, }, - &http.Request{ + { Method: "GET", URL: &url.URL{Path: "/test/"}, - }: &RouteMatch{ + }: { ControllerName: "Application", MethodName: "Index", FixedParams: []string{"Test", "Test2"}, Params: map[string][]string{}, }, - &http.Request{ + { Method: "GET", URL: &url.URL{Path: "/app/123"}, - }: &RouteMatch{ + }: { ControllerName: "Application", MethodName: "Show", FixedParams: []string{}, Params: map[string][]string{"id": {"123"}}, }, - &http.Request{ + { Method: "GET", URL: &url.URL{Path: "/app/123.png"}, - }: &RouteMatch{ + }: { ControllerName: "Application", MethodName: "ShowImage", FixedParams: []string{}, Params: map[string][]string{"id": {"123"}}, }, - &http.Request{ + { Method: "GET", URL: &url.URL{Path: "/app/123.jpg"}, - }: &RouteMatch{ + }: { ControllerName: "Application", MethodName: "ShowImageCustomExtension", FixedParams: []string{}, Params: map[string][]string{"id": {"123"}, "ext": {"jpg"}}, }, - &http.Request{ + { Method: "PATCH", URL: &url.URL{Path: "/app/123"}, - }: &RouteMatch{ + }: { ControllerName: "Application", MethodName: "Update", FixedParams: []string{}, Params: map[string][]string{"id": {"123"}}, }, - &http.Request{ + { Method: "POST", URL: &url.URL{Path: "/app/123"}, - }: &RouteMatch{ + }: { ControllerName: "Application", MethodName: "Save", FixedParams: []string{}, Params: map[string][]string{"id": {"123"}}, }, - &http.Request{ + { Method: "GET", URL: &url.URL{Path: "/app/123/"}, - }: &RouteMatch{ + }: { ControllerName: "Application", MethodName: "Show", FixedParams: []string{}, Params: map[string][]string{"id": {"123"}}, }, - &http.Request{ + { Method: "GET", URL: &url.URL{Path: "/public/css/style.css"}, - }: &RouteMatch{ + }: { ControllerName: "Static", MethodName: "Serve", FixedParams: []string{"public"}, Params: map[string][]string{"filepath": {"css/style.css"}}, }, - &http.Request{ + { Method: "GET", URL: &url.URL{Path: "/javascript/sessvars.js"}, - }: &RouteMatch{ + }: { ControllerName: "Static", MethodName: "Serve", FixedParams: []string{"public/js"}, Params: map[string][]string{"filepath": {"sessvars.js"}}, }, - &http.Request{ + { Method: "GET", URL: &url.URL{Path: "/Implicit/Route"}, - }: &RouteMatch{ + }: { ControllerName: "Implicit", MethodName: "Route", FixedParams: []string{}, @@ -249,10 +249,10 @@ var routeMatchTestCases = map[*http.Request]*RouteMatch{ }, }, - &http.Request{ + { Method: "GET", URL: &url.URL{Path: "/favicon.ico"}, - }: &RouteMatch{ + }: { ControllerName: "", MethodName: "", Action: "404", @@ -260,22 +260,22 @@ var routeMatchTestCases = map[*http.Request]*RouteMatch{ Params: map[string][]string{}, }, - &http.Request{ + { Method: "POST", URL: &url.URL{Path: "/app/123"}, Header: http.Header{"X-Http-Method-Override": []string{"PATCH"}}, - }: &RouteMatch{ + }: { ControllerName: "Application", MethodName: "Update", FixedParams: []string{}, Params: map[string][]string{"id": {"123"}}, }, - &http.Request{ + { Method: "GET", URL: &url.URL{Path: "/app/123"}, Header: http.Header{"X-Http-Method-Override": []string{"PATCH"}}, - }: &RouteMatch{ + }: { ControllerName: "Application", MethodName: "Show", FixedParams: []string{}, @@ -333,100 +333,100 @@ type ReverseRouteArgs struct { } var reverseRoutingTestCases = map[*ReverseRouteArgs]*ActionDefinition{ - &ReverseRouteArgs{ + { action: "Application.Index", args: map[string]string{}, - }: &ActionDefinition{ + }: { Url: "/", Method: "GET", Star: false, Action: "Application.Index", }, - &ReverseRouteArgs{ + { action: "Application.ShowImage", args: map[string]string{"id": "123"}, - }: &ActionDefinition{ + }: { Url: "/app/123.png", Method: "GET", Star: false, Action: "Application.ShowImage", }, - &ReverseRouteArgs{ + { action: "Application.Show", args: map[string]string{"id": "123"}, - }: &ActionDefinition{ + }: { Url: "/app/123/", Method: "GET", Star: false, Action: "Application.Show", }, - &ReverseRouteArgs{ + { action: "Implicit.Route", args: map[string]string{}, - }: &ActionDefinition{ + }: { Url: "/Implicit/Route", Method: "GET", Star: true, Action: "Implicit.Route", }, - &ReverseRouteArgs{ + { action: "Application.Save", args: map[string]string{"id": "123", "c": "http://continue"}, - }: &ActionDefinition{ + }: { Url: "/app/123?c=http%3A%2F%2Fcontinue", Method: "POST", Star: false, Action: "Application.Save", }, - &ReverseRouteArgs{ + { action: "Application.WildShow", args: map[string]string{"id": "123"}, - }: &ActionDefinition{ + }: { Url: "/app-wild/123/", Method: "GET", Star: false, Action: "Application.WildShow", }, - &ReverseRouteArgs{ + { action: "Application.WildShow", args: map[string]string{"id": "100% organic"}, - }: &ActionDefinition{ + }: { Url: "/app-wild/100%25%20organic/", Method: "GET", Star: false, Action: "Application.WildShow", }, - &ReverseRouteArgs{ + { action: "Application.Show", args: map[string]string{"id": "100% organic"}, - }: &ActionDefinition{ + }: { Url: "/app/100%25%20organic/", Method: "GET", Star: false, Action: "Application.Show", }, - &ReverseRouteArgs{ + { action: "Application.WildShow", args: map[string]string{"id": "folder/subfolder"}, - }: &ActionDefinition{ + }: { Url: "/app-wild/folder/subfolder/", Method: "GET", Star: false, Action: "Application.WildShow", }, - &ReverseRouteArgs{ + { action: "Application.Show", args: map[string]string{"id": "folder/subfolder"}, - }: &ActionDefinition{ + }: { Url: "/app/folder%2Fsubfolder/", Method: "GET", Star: false, @@ -455,7 +455,7 @@ func BenchmarkRouter(b *testing.B) { router.updateTree() b.ResetTimer() for i := 0; i < b.N/len(routeMatchTestCases); i++ { - for req, _ := range routeMatchTestCases { + for req := range routeMatchTestCases { r := router.Route(req) if r == nil { b.Errorf("Request not found: %s", req.URL.Path) diff --git a/validators_test.go b/validators_test.go index 8e4fb38..6cce4b7 100644 --- a/validators_test.go +++ b/validators_test.go @@ -35,60 +35,60 @@ func performTests(validator Validator, tests []Expect, t *testing.T) { func TestRequired(t *testing.T) { tests := []Expect{ - Expect{nil, false, "nil data"}, - Expect{"Testing", true, "non-empty string"}, - Expect{"", false, "empty string"}, - Expect{true, true, "true boolean"}, - Expect{false, false, "false boolean"}, - Expect{1, true, "positive integer"}, - Expect{-1, true, "negative integer"}, - Expect{0, false, "0 integer"}, - Expect{time.Now(), true, "current time"}, - Expect{time.Time{}, false, "a zero time"}, - Expect{func() {}, true, "other non-nil data types"}, + {nil, false, "nil data"}, + {"Testing", true, "non-empty string"}, + {"", false, "empty string"}, + {true, true, "true boolean"}, + {false, false, "false boolean"}, + {1, true, "positive integer"}, + {-1, true, "negative integer"}, + {0, false, "0 integer"}, + {time.Now(), true, "current time"}, + {time.Time{}, false, "a zero time"}, + {func() {}, true, "other non-nil data types"}, } // testing both the struct and the helper method - for _, required := range []Required{Required{}, ValidRequired()} { + for _, required := range []Required{{}, ValidRequired()} { performTests(required, tests, t) } } func TestMin(t *testing.T) { tests := []Expect{ - Expect{11, true, "val > min"}, - Expect{10, true, "val == min"}, - Expect{9, false, "val < min"}, - Expect{true, false, "TypeOf(val) != int"}, + {11, true, "val > min"}, + {10, true, "val == min"}, + {9, false, "val < min"}, + {true, false, "TypeOf(val) != int"}, } - for _, min := range []Min{Min{10}, ValidMin(10)} { + for _, min := range []Min{{10}, ValidMin(10)} { performTests(min, tests, t) } } func TestMax(t *testing.T) { tests := []Expect{ - Expect{9, true, "val < max"}, - Expect{10, true, "val == max"}, - Expect{11, false, "val > max"}, - Expect{true, false, "TypeOf(val) != int"}, + {9, true, "val < max"}, + {10, true, "val == max"}, + {11, false, "val > max"}, + {true, false, "TypeOf(val) != int"}, } - for _, max := range []Max{Max{10}, ValidMax(10)} { + for _, max := range []Max{{10}, ValidMax(10)} { performTests(max, tests, t) } } func TestRange(t *testing.T) { tests := []Expect{ - Expect{50, true, "min <= val <= max"}, - Expect{10, true, "val == min"}, - Expect{100, true, "val == max"}, - Expect{9, false, "val < min"}, - Expect{101, false, "val > max"}, + {50, true, "min <= val <= max"}, + {10, true, "val == min"}, + {100, true, "val == max"}, + {9, false, "val < min"}, + {101, false, "val > max"}, } goodValidators := []Range{ - Range{Min{10}, Max{100}}, + {Min{10}, Max{100}}, ValidRange(10, 100), } for _, rangeValidator := range goodValidators { @@ -96,13 +96,13 @@ func TestRange(t *testing.T) { } tests = []Expect{ - Expect{10, true, "min == val == max"}, - Expect{9, false, "val < min && val < max && min == max"}, - Expect{11, false, "val > min && val > max && min == max"}, + {10, true, "min == val == max"}, + {9, false, "val < min && val < max && min == max"}, + {11, false, "val > min && val > max && min == max"}, } goodValidators = []Range{ - Range{Min{10}, Max{10}}, + {Min{10}, Max{10}}, ValidRange(10, 10), } for _, rangeValidator := range goodValidators { @@ -122,7 +122,7 @@ func TestRange(t *testing.T) { // result in false since val can never be greater than min and less // than max when min > max badValidators := []Range{ - Range{Min{100}, Max{10}}, + {Min{100}, Max{10}}, ValidRange(100, 10), } for _, rangeValidator := range badValidators { @@ -133,16 +133,16 @@ func TestRange(t *testing.T) { func TestMinSize(t *testing.T) { greaterThanMessage := "len(val) >= min" tests := []Expect{ - Expect{"1", true, greaterThanMessage}, - Expect{"12", true, greaterThanMessage}, - Expect{[]int{1}, true, greaterThanMessage}, - Expect{[]int{1, 2}, true, greaterThanMessage}, - Expect{"", false, "len(val) <= min"}, - Expect{[]int{}, false, "len(val) <= min"}, - Expect{nil, false, "TypeOf(val) != string && TypeOf(val) != slice"}, + {"1", true, greaterThanMessage}, + {"12", true, greaterThanMessage}, + {[]int{1}, true, greaterThanMessage}, + {[]int{1, 2}, true, greaterThanMessage}, + {"", false, "len(val) <= min"}, + {[]int{}, false, "len(val) <= min"}, + {nil, false, "TypeOf(val) != string && TypeOf(val) != slice"}, } - for _, minSize := range []MinSize{MinSize{1}, ValidMinSize(1)} { + for _, minSize := range []MinSize{{1}, ValidMinSize(1)} { performTests(minSize, tests, t) } } @@ -150,41 +150,41 @@ func TestMinSize(t *testing.T) { func TestMaxSize(t *testing.T) { lessThanMessage := "len(val) <= max" tests := []Expect{ - Expect{"", true, lessThanMessage}, - Expect{"12", true, lessThanMessage}, - Expect{[]int{}, true, lessThanMessage}, - Expect{[]int{1, 2}, true, lessThanMessage}, - Expect{"123", false, "len(val) >= max"}, - Expect{[]int{1, 2, 3}, false, "len(val) >= max"}, - } - for _, maxSize := range []MaxSize{MaxSize{2}, ValidMaxSize(2)} { + {"", true, lessThanMessage}, + {"12", true, lessThanMessage}, + {[]int{}, true, lessThanMessage}, + {[]int{1, 2}, true, lessThanMessage}, + {"123", false, "len(val) >= max"}, + {[]int{1, 2, 3}, false, "len(val) >= max"}, + } + for _, maxSize := range []MaxSize{{2}, ValidMaxSize(2)} { performTests(maxSize, tests, t) } } func TestLength(t *testing.T) { tests := []Expect{ - Expect{"12", true, "len(val) == length"}, - Expect{[]int{1, 2}, true, "len(val) == length"}, - Expect{"123", false, "len(val) > length"}, - Expect{[]int{1, 2, 3}, false, "len(val) > length"}, - Expect{"1", false, "len(val) < length"}, - Expect{[]int{1}, false, "len(val) < length"}, - Expect{nil, false, "TypeOf(val) != string && TypeOf(val) != slice"}, - } - for _, length := range []Length{Length{2}, ValidLength(2)} { + {"12", true, "len(val) == length"}, + {[]int{1, 2}, true, "len(val) == length"}, + {"123", false, "len(val) > length"}, + {[]int{1, 2, 3}, false, "len(val) > length"}, + {"1", false, "len(val) < length"}, + {[]int{1}, false, "len(val) < length"}, + {nil, false, "TypeOf(val) != string && TypeOf(val) != slice"}, + } + for _, length := range []Length{{2}, ValidLength(2)} { performTests(length, tests, t) } } func TestMatch(t *testing.T) { tests := []Expect{ - Expect{"bca123", true, `"[abc]{3}\d*" matches "bca123"`}, - Expect{"bc123", false, `"[abc]{3}\d*" does not match "bc123"`}, - Expect{"", false, `"[abc]{3}\d*" does not match ""`}, + {"bca123", true, `"[abc]{3}\d*" matches "bca123"`}, + {"bc123", false, `"[abc]{3}\d*" does not match "bc123"`}, + {"", false, `"[abc]{3}\d*" does not match ""`}, } regex := regexp.MustCompile(`[abc]{3}\d*`) - for _, match := range []Match{Match{regex}, ValidMatch(regex)} { + for _, match := range []Match{{regex}, ValidMatch(regex)} { performTests(match, tests, t) } } @@ -206,7 +206,7 @@ func TestEmail(t *testing.T) { "aå.com", // domain containing unicode (however, unicode domains do exist in the state of xn--.com e.g. å.com = xn--5ca.com) } - for _, email := range []Email{Email{Match{emailPattern}}, ValidEmail()} { + for _, email := range []Email{{Match{emailPattern}}, ValidEmail()} { var currentEmail string // test invalid starting chars