From 736057e3ff330ccd56c69b074887d37bcaf976a9 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Wed, 17 Jan 2018 15:16:06 -0500 Subject: [PATCH 1/8] asynchttp: tweak formatting of import list It looks nicer when standard library and external library imports are grouped. Signed-off-by: John Mulligan --- asynchttp.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/asynchttp.go b/asynchttp.go index 9cbffa2..56a0992 100644 --- a/asynchttp.go +++ b/asynchttp.go @@ -16,12 +16,13 @@ package rest import ( - "github.com/gorilla/mux" - "github.com/heketi/utils" - "github.com/lpabon/godbc" "net/http" "sync" "time" + + "github.com/gorilla/mux" + "github.com/heketi/utils" + "github.com/lpabon/godbc" ) var ( From b25941615e51ed725789cd579958a9726caf1fea Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Wed, 17 Jan 2018 15:25:52 -0500 Subject: [PATCH 2/8] asynchttp: split background function handling into its own function Signed-off-by: John Mulligan --- asynchttp.go | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/asynchttp.go b/asynchttp.go index 56a0992..f20b905 100644 --- a/asynchttp.go +++ b/asynchttp.go @@ -122,21 +122,7 @@ func (a *AsyncHttpManager) AsyncHttpRedirectFunc(w http.ResponseWriter, handlerfunc func() (string, error)) { handler := a.NewHandler() - go func() { - logger.Info("Started job %v", handler.id) - - ts := time.Now() - url, err := handlerfunc() - logger.Info("Completed job %v in %v", handler.id, time.Since(ts)) - - if err != nil { - handler.CompletedWithError(err) - } else if url != "" { - handler.CompletedWithLocation(url) - } else { - handler.Completed() - } - }() + handler.handle(handlerfunc) http.Redirect(w, r, handler.Url(), http.StatusAccepted) } @@ -268,3 +254,22 @@ func (h *AsyncHttpHandler) Completed() { godbc.Ensure(h.location == "") godbc.Ensure(h.err == nil) } + +// handle starts running the given function in the background (goroutine). +func (h *AsyncHttpHandler) handle(f func() (string, error)) { + go func() { + logger.Info("Started job %v", h.id) + + ts := time.Now() + url, err := f() + logger.Info("Completed job %v in %v", h.id, time.Since(ts)) + + if err != nil { + h.CompletedWithError(err) + } else if url != "" { + h.CompletedWithLocation(url) + } else { + h.Completed() + } + }() +} From 5835d4a35389c090c3faee1d33e9040c9007eff7 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Wed, 17 Jan 2018 15:31:40 -0500 Subject: [PATCH 3/8] asynchttp: add method for creating async handlers by specifying ids Extend the library API so that callers can construct handlers with predetermined IDs. Signed-off-by: John Mulligan --- asynchttp.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/asynchttp.go b/asynchttp.go index f20b905..d09cf57 100644 --- a/asynchttp.go +++ b/asynchttp.go @@ -56,9 +56,16 @@ func NewAsyncHttpManager(route string) *AsyncHttpManager { // Only use this function if you need to do every step by hand. // It is recommended to use AsyncHttpRedirectFunc() instead func (a *AsyncHttpManager) NewHandler() *AsyncHttpHandler { + return a.NewHandlerWithId(utils.GenUUID()) +} + +// NewHandlerWithId constructs and returns an AsyncHttpHandler with the +// given ID. Compare to NewHandler() which automatically generates its +// own ID. +func (a *AsyncHttpManager) NewHandlerWithId(id string) *AsyncHttpHandler { handler := &AsyncHttpHandler{ manager: a, - id: utils.GenUUID(), + id: id, } a.lock.Lock() From 5fa54fe61da40820b311e04827138da094a36167 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Wed, 17 Jan 2018 15:35:37 -0500 Subject: [PATCH 4/8] asynchttp: prevent overwriting existing handlers Now that we're allowing for custom async IDs we add in extra protection against overwriting an existing handler by reusing an id. Signed-off-by: John Mulligan --- asynchttp.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/asynchttp.go b/asynchttp.go index d09cf57..d4c7e48 100644 --- a/asynchttp.go +++ b/asynchttp.go @@ -71,6 +71,8 @@ func (a *AsyncHttpManager) NewHandlerWithId(id string) *AsyncHttpHandler { a.lock.Lock() defer a.lock.Unlock() + _, idPresent := a.handlers[handler.id] + godbc.Require(!idPresent) a.handlers[handler.id] = handler return handler From 7b6402c0e84d2647cf7172cbd3d08b45e79763ab Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Wed, 17 Jan 2018 15:46:54 -0500 Subject: [PATCH 5/8] asynchttp: support providing custom id generator functions This change allows users of the library to provide new ID generator functions. These functions take no arguments an return a string. This does not change the default ID generator which is still the Heketi utils "GenUUID". Signed-off-by: John Mulligan --- asynchttp.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/asynchttp.go b/asynchttp.go index d4c7e48..ba53d58 100644 --- a/asynchttp.go +++ b/asynchttp.go @@ -39,6 +39,7 @@ type AsyncHttpHandler struct { // Manager of asynchronous operations type AsyncHttpManager struct { + IdGen func() string lock sync.RWMutex route string handlers map[string]*AsyncHttpHandler @@ -49,6 +50,7 @@ func NewAsyncHttpManager(route string) *AsyncHttpManager { return &AsyncHttpManager{ route: route, handlers: make(map[string]*AsyncHttpHandler), + IdGen: utils.GenUUID, } } @@ -56,7 +58,7 @@ func NewAsyncHttpManager(route string) *AsyncHttpManager { // Only use this function if you need to do every step by hand. // It is recommended to use AsyncHttpRedirectFunc() instead func (a *AsyncHttpManager) NewHandler() *AsyncHttpHandler { - return a.NewHandlerWithId(utils.GenUUID()) + return a.NewHandlerWithId(a.NewId()) } // NewHandlerWithId constructs and returns an AsyncHttpHandler with the @@ -78,6 +80,12 @@ func (a *AsyncHttpManager) NewHandlerWithId(id string) *AsyncHttpHandler { return handler } +// NewId returns a new string id for a handler. This string is not preserved +// internally and must be passed to another function to be used. +func (a *AsyncHttpManager) NewId() string { + return a.IdGen() +} + // Create an asynchronous operation handler and return the appropiate // information the caller. // This function will call handlerfunc() in a new go routine, then From 23cfaed90fbaf059fe7c75bbfe49dfbe5505b730 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Wed, 17 Jan 2018 17:03:25 -0500 Subject: [PATCH 6/8] asynchttp: add a method to do async with custom ids Adds an AsyncHttpRedirectUsing method that takes both a custom id and a callback function, rather than a callback alone. Add a test for the new method. Signed-off-by: John Mulligan --- asynchttp.go | 10 +++++++++ asynchttp_test.go | 56 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/asynchttp.go b/asynchttp.go index ba53d58..5877e6c 100644 --- a/asynchttp.go +++ b/asynchttp.go @@ -143,6 +143,16 @@ func (a *AsyncHttpManager) AsyncHttpRedirectFunc(w http.ResponseWriter, http.Redirect(w, r, handler.Url(), http.StatusAccepted) } +func (a *AsyncHttpManager) AsyncHttpRedirectUsing(w http.ResponseWriter, + r *http.Request, + id string, + handlerfunc func() (string, error)) { + + handler := a.NewHandlerWithId(id) + handler.handle(handlerfunc) + http.Redirect(w, r, handler.Url(), http.StatusAccepted) +} + // Handler for asynchronous operation status // Register this handler with a router like Gorilla Mux // diff --git a/asynchttp_test.go b/asynchttp_test.go index 18a7848..13bae0d 100644 --- a/asynchttp_test.go +++ b/asynchttp_test.go @@ -24,6 +24,7 @@ import ( "io/ioutil" "net/http" "net/http/httptest" + "strings" "sync" "testing" "time" @@ -418,3 +419,58 @@ func TestApplicationWithRedirectFunc(t *testing.T) { } } + +func TestAsyncHttpRedirectUsing(t *testing.T) { + + // Setup asynchronous manager + route := "/x" + manager := NewAsyncHttpManager(route) + + // Setup the route + router := mux.NewRouter() + router.HandleFunc(route+"/{id}", manager.HandlerStatus).Methods("GET") + router.HandleFunc("/result", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain; charset=UTF-8") + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, "HelloWorld") + }).Methods("GET") + + router.HandleFunc("/app", func(w http.ResponseWriter, r *http.Request) { + manager.AsyncHttpRedirectUsing(w, r, "bob", func() (string, error) { + time.Sleep(500 * time.Millisecond) + return "/result", nil + }) + }).Methods("GET") + + // Setup the server + ts := httptest.NewServer(router) + defer ts.Close() + + r, err := http.Get(ts.URL + "/app") + tests.Assert(t, r.StatusCode == http.StatusAccepted) + tests.Assert(t, err == nil) + location, err := r.Location() + tests.Assert(t, err == nil) + + tests.Assert(t, strings.Contains(location.String(), "bob"), + `expected "bob" in newloc, got:`, location) + + for { + // Since Get automatically redirects, we will + // just keep asking until we get a body + r, err := http.Get(location.String()) + tests.Assert(t, err == nil) + tests.Assert(t, r.StatusCode == http.StatusOK) + if r.ContentLength > 0 { + body, err := ioutil.ReadAll(r.Body) + r.Body.Close() + tests.Assert(t, err == nil) + tests.Assert(t, string(body) == "HelloWorld") + break + } else { + tests.Assert(t, r.Header.Get("X-Pending") == "true") + time.Sleep(time.Millisecond) + } + } + +} From 1c2ffe8e69c898dfe26b159f6a6fdea43d0c5be8 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Wed, 17 Jan 2018 17:15:13 -0500 Subject: [PATCH 7/8] asynchttp: test that custom id generators generate custom ids Signed-off-by: John Mulligan --- asynchttp_test.go | 69 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/asynchttp_test.go b/asynchttp_test.go index 13bae0d..7f757c7 100644 --- a/asynchttp_test.go +++ b/asynchttp_test.go @@ -474,3 +474,72 @@ func TestAsyncHttpRedirectUsing(t *testing.T) { } } + +func TestAsyncHttpRedirectFuncCustomIds(t *testing.T) { + + // Setup asynchronous manager + route := "/x" + manager := NewAsyncHttpManager(route) + + i := 0 + manager.IdGen = func() string { + s := fmt.Sprintf("abc-%v%v%v", i, i, i) + i += 1 + return s + } + + // Setup the route + router := mux.NewRouter() + router.HandleFunc(route+"/{id}", manager.HandlerStatus).Methods("GET") + router.HandleFunc("/result", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain; charset=UTF-8") + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, "HelloWorld") + }).Methods("GET") + + router.HandleFunc("/app", func(w http.ResponseWriter, r *http.Request) { + manager.AsyncHttpRedirectFunc(w, r, func() (string, error) { + time.Sleep(500 * time.Millisecond) + return "/result", nil + }) + }).Methods("GET") + + // Setup the server + ts := httptest.NewServer(router) + defer ts.Close() + + for j := 0; j < 4; j++ { + r, err := http.Get(ts.URL + "/app") + tests.Assert(t, r.StatusCode == http.StatusAccepted) + tests.Assert(t, err == nil) + location, err := r.Location() + tests.Assert(t, err == nil) + + // test that our custom id generator generated the IDs + // according to our pattern + tests.Assert(t, strings.Contains(location.String(), "abc-"), + `expected "abc-" in newloc, got:`, location) + part := fmt.Sprintf("-%v%v%v", j, j, j) + tests.Assert(t, strings.Contains(location.String(), part), + "expected", part, "in newloc, got:", location) + + for { + // Since Get automatically redirects, we will + // just keep asking until we get a body + r, err := http.Get(location.String()) + tests.Assert(t, err == nil) + tests.Assert(t, r.StatusCode == http.StatusOK) + if r.ContentLength > 0 { + body, err := ioutil.ReadAll(r.Body) + r.Body.Close() + tests.Assert(t, err == nil) + tests.Assert(t, string(body) == "HelloWorld") + break + } else { + tests.Assert(t, r.Header.Get("X-Pending") == "true") + time.Sleep(time.Millisecond) + } + } + } + +} From a382a24c960d0316de006181ebd0723739288445 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Wed, 4 Apr 2018 09:50:29 -0400 Subject: [PATCH 8/8] update go versions in .travis.yml This is a quick update of the go versions used by this code. The travis configuration should probably be updated more thoroughly in a future change. Signed-off-by: John Mulligan --- .travis.yml | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1b2eb7d..5541d5e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,16 +8,11 @@ env: secure: ENMA5XgK92+t5tia+tgiCSx2mrXSEwLKgbRkj7ZKezdCdH2Hq9/waf7zWyXxXZ3ZQOzvQEkzHDRULwoaareST4cTY365euHT7UW4uFEqFOfz2kk8HrCFkHPJnkXaosgBqGfwB7uxFpxqcFzaZ4YPcXdYdKEfqaz2Q7MJsQdXpqlFRWa3pmwMj8nu//b/0MRfRvQUya420jhHqyt4G8os+2/VcTt8XCi83nr/RaKSyAyVZLl/5zwvQPmv0TRt/VobvnxNGBpwFxzFhxF9JH0VS/h4Mu/j0QNSQ0Rg4ip8JzQxjzvdXmT4CVQX9f+42NrI7MmserpjSy8OZBRpUkfX0eTFWJL/pzgpYxdhW3/8dA0/IGoCj0kGmvoW0x4KSn8XKOPKJYZkdZwKSzNYMbqEDZY0rXS7AAnqVkoMjgz8JQcSfo17Lwvu+7bztK2/JnMQlkOxY+HNSaeACeKKxco24vrtvfoUW4xx0c3scju82NrngaiXrffLymbN4U5DoEP70Hd/BLeMfcP+34MDn+AOLBOeGTgU6nbziY9gZhlPdBGO6aG5PrwXEJDtnpQNU1Pe8/b8QYvIkWAY77cZlQfg6eQ27a/u5U5MLfCIH9kebERtsNyXqIoOnQXbIFma9m1UWr4L7FQSndmYnkbrMhIXvXEJdDghUtGEwvAaOlyj5MI= matrix: include: - - go: 1.2.2 - env: OPTIONS="" - - go: 1.3.3 - env: OPTIONS="-race" - - go: 1.4.2 + - go: "1.10" env: COVERAGE="true" GOTOOLS="yes" - - go: 1.5 + - go: 1.8.7 env: OPTIONS="-race" GOTOOLS="yes" before_script: -- if [[ "$GOTOOLS" = "yes" ]] ; then go get golang.org/x/tools/cmd/vet; fi - if [[ "$GOTOOLS" = "yes" ]] ; then go get golang.org/x/tools/cmd/cover; fi script: - if [[ "$GOTOOLS" = "yes" ]] ; then go fmt ./... | wc -l | grep 0 ; fi