diff --git a/default_context.go b/default_context.go index 3f2eec99d..e88acbf4b 100644 --- a/default_context.go +++ b/default_context.go @@ -23,6 +23,9 @@ import ( var _ Context = &DefaultContext{} var _ context.Context = &DefaultContext{} +// TODO(sio4): #road-to-v1 - make DefaultContext private +// and only allow it to be generated by App.newContext() or any similar. + // DefaultContext is, as its name implies, a default // implementation of the Context interface. type DefaultContext struct { @@ -67,16 +70,22 @@ func (d *DefaultContext) Param(key string) string { // Set a value onto the Context. Any value set onto the Context // will be automatically available in templates. func (d *DefaultContext) Set(key string, value interface{}) { + if d.data == nil { + d.data = &sync.Map{} + } d.data.Store(key, value) } // Value that has previously stored on the context. func (d *DefaultContext) Value(key interface{}) interface{} { - if k, ok := key.(string); ok { + if k, ok := key.(string); ok && d.data != nil { if v, ok := d.data.Load(k); ok { return v } } + if d.Context == nil { + return nil + } return d.Context.Value(key) } @@ -235,6 +244,11 @@ func (d *DefaultContext) Redirect(status int, url string, args ...interface{}) e // Data contains all the values set through Get/Set. func (d *DefaultContext) Data() map[string]interface{} { m := map[string]interface{}{} + + if d.data == nil { + return m + } + d.data.Range(func(k, v interface{}) bool { s, ok := k.(string) if !ok { diff --git a/default_context_test.go b/default_context_test.go index 2823828cc..203781641 100644 --- a/default_context_test.go +++ b/default_context_test.go @@ -143,6 +143,15 @@ func Test_DefaultContext_GetSet(t *testing.T) { r.Equal("Mark", c.Value("name").(string)) } +func Test_DefaultContext_Set_Unconfigured(t *testing.T) { + r := require.New(t) + c := DefaultContext{} + + c.Set("name", "Yonghwan") + r.NotNil(c.Value("name")) + r.Equal("Yonghwan", c.Value("name").(string)) +} + func Test_DefaultContext_Value(t *testing.T) { r := require.New(t) c := basicContext() @@ -151,7 +160,12 @@ func Test_DefaultContext_Value(t *testing.T) { c.Set("name", "Mark") r.NotNil(c.Value("name")) r.Equal("Mark", c.Value("name").(string)) - r.Equal("Mark", c.Value("name").(string)) +} + +func Test_DefaultContext_Value_Unconfigured(t *testing.T) { + r := require.New(t) + c := DefaultContext{} + r.Nil(c.Value("name")) } func Test_DefaultContext_Render(t *testing.T) { @@ -301,3 +315,68 @@ func Test_DefaultContext_Bind_JSON(t *testing.T) { r.Equal("Mark", user.FirstName) } + +func Test_DefaultContext_Data(t *testing.T) { + r := require.New(t) + c := basicContext() + + r.EqualValues(map[string]interface{}{}, c.Data()) +} + +func Test_DefaultContext_Data_Unconfigured(t *testing.T) { + r := require.New(t) + c := DefaultContext{} + + r.EqualValues(map[string]interface{}{}, c.Data()) +} + +func Test_DefaultContext_String(t *testing.T) { + r := require.New(t) + c := basicContext() + c.Set("name", "Buffalo") + c.Set("language", "go") + + r.EqualValues("language: go\n\nname: Buffalo", c.String()) +} + +func Test_DefaultContext_String_EmptyData(t *testing.T) { + r := require.New(t) + c := basicContext() + r.EqualValues("", c.String()) +} + +func Test_DefaultContext_String_EmptyData_Unconfigured(t *testing.T) { + r := require.New(t) + c := DefaultContext{} + + r.EqualValues("", c.String()) +} + +func Test_DefaultContext_MarshalJSON(t *testing.T) { + r := require.New(t) + c := basicContext() + c.Set("name", "Buffalo") + c.Set("language", "go") + + jb, err := c.MarshalJSON() + r.NoError(err) + r.EqualValues(`{"language":"go","name":"Buffalo"}`, string(jb)) +} + +func Test_DefaultContext_MarshalJSON_EmptyData(t *testing.T) { + r := require.New(t) + c := basicContext() + + jb, err := c.MarshalJSON() + r.NoError(err) + r.EqualValues(`{}`, string(jb)) +} + +func Test_DefaultContext_MarshalJSON_EmptyData_Unconfigured(t *testing.T) { + r := require.New(t) + c := DefaultContext{} + + jb, err := c.MarshalJSON() + r.NoError(err) + r.EqualValues(`{}`, string(jb)) +} diff --git a/plugins/plugins_test.go b/plugins/plugins_test.go index 1f6317435..855850a1f 100644 --- a/plugins/plugins_test.go +++ b/plugins/plugins_test.go @@ -2,7 +2,7 @@ package plugins import ( "context" - "io/ioutil" + "os" "strings" "testing" "time" @@ -20,7 +20,7 @@ func TestAskBin_respectsTimeout(t *testing.T) { return } - if fileEntries, err := ioutil.ReadDir(from); err == nil { + if fileEntries, err := os.ReadDir(from); err == nil { found := false for _, e := range fileEntries { if strings.HasPrefix(e.Name(), "buffalo-") { diff --git a/router_test.go b/router_test.go index ff3584fa5..29bbdff3b 100644 --- a/router_test.go +++ b/router_test.go @@ -2,7 +2,7 @@ package buffalo import ( "fmt" - "io/ioutil" + "io" "net/http" "os" "path" @@ -98,7 +98,7 @@ func Test_Mount_Buffalo(t *testing.T) { r.NoError(err) res, err := http.DefaultClient.Do(req) r.NoError(err) - b, _ := ioutil.ReadAll(res.Body) + b, _ := io.ReadAll(res.Body) r.Equal(fmt.Sprintf("%s - %s/", m, u), string(b)) } } @@ -123,7 +123,7 @@ func Test_Mount_Buffalo_on_Group(t *testing.T) { r.NoError(err) res, err := http.DefaultClient.Do(req) r.NoError(err) - b, _ := ioutil.ReadAll(res.Body) + b, _ := io.ReadAll(res.Body) r.Equal(fmt.Sprintf("%s - %s/", m, u), string(b)) } } @@ -158,7 +158,7 @@ func Test_Mount_Handler(t *testing.T) { r.NoError(err) res, err := http.DefaultClient.Do(req) r.NoError(err) - b, _ := ioutil.ReadAll(res.Body) + b, _ := io.ReadAll(res.Body) r.Equal(fmt.Sprintf("%s - %s/", m, u), string(b)) } } @@ -197,7 +197,7 @@ func Test_PreHandlers(t *testing.T) { r.NoError(err) res, err := http.DefaultClient.Do(req) r.NoError(err) - b, err := ioutil.ReadAll(res.Body) + b, err := io.ReadAll(res.Body) r.NoError(err) r.Equal(v.Code, res.StatusCode) r.Equal(v.Result, string(b)) @@ -242,7 +242,7 @@ func Test_PreWares(t *testing.T) { r.NoError(err) res, err := http.DefaultClient.Do(req) r.NoError(err) - b, err := ioutil.ReadAll(res.Body) + b, err := io.ReadAll(res.Body) r.NoError(err) r.Equal(v.Code, res.StatusCode) r.Equal(v.Result, string(b)) @@ -269,7 +269,7 @@ func Test_Router(t *testing.T) { r.NoError(err) res, err := http.DefaultClient.Do(req) r.NoError(err) - b, _ := ioutil.ReadAll(res.Body) + b, _ := io.ReadAll(res.Body) r.Equal(fmt.Sprintf("%s|/router/tests", v), string(b)) } } @@ -552,7 +552,7 @@ func Test_Resource(t *testing.T) { r.NoError(err) res, err := c.Do(req) r.NoError(err) - b, err := ioutil.ReadAll(res.Body) + b, err := io.ReadAll(res.Body) r.NoError(err) r.Equal(test.Result, string(b)) } @@ -710,7 +710,7 @@ func Test_ResourceOnResource(t *testing.T) { r.NoError(err) res, err := c.Do(req) r.NoError(err) - b, err := ioutil.ReadAll(res.Body) + b, err := io.ReadAll(res.Body) r.NoError(err) r.Equal(test.Result, string(b)) } diff --git a/worker/simple.go b/worker/simple.go index 3f5788d40..76d9ef673 100644 --- a/worker/simple.go +++ b/worker/simple.go @@ -15,6 +15,9 @@ var _ Worker = &Simple{} // NewSimple creates a basic implementation of the Worker interface // that is backed using just the standard library and goroutines. func NewSimple() *Simple { + // TODO(sio4): #road-to-v1 - how to check if the worker is ready to work + // when worker should be initialized? how to check if worker is ready? + // and purpose of the context return NewSimpleWithContext(context.Background()) } @@ -49,6 +52,10 @@ type Simple struct { // Register Handler with the worker func (w *Simple) Register(name string, h Handler) error { + if name == "" || h == nil { + return fmt.Errorf("name or handler cannot be empty/nil") + } + w.moot.Lock() defer w.moot.Unlock() if _, ok := w.handlers[name]; ok { @@ -60,6 +67,7 @@ func (w *Simple) Register(name string, h Handler) error { // Start the worker func (w *Simple) Start(ctx context.Context) error { + // TODO(sio4): #road-to-v1 - define the purpose of Start clearly w.Logger.Info("starting Simple background worker") w.ctx, w.cancel = context.WithCancel(ctx) @@ -68,20 +76,30 @@ func (w *Simple) Start(ctx context.Context) error { // Stop the worker func (w *Simple) Stop() error { + // prevent job submission when stopping + w.moot.Lock() + defer w.moot.Unlock() + w.Logger.Info("stopping Simple background worker") + w.cancel() + w.wg.Wait() w.Logger.Info("all background jobs stopped completely") - w.cancel() return nil } // Perform a job as soon as possibly using a goroutine. func (w *Simple) Perform(job Job) error { + // Perform should not allow a job submission if the worker is not running + if err := w.ctx.Err(); err != nil { + return fmt.Errorf("worker is not ready to perform a job: %v", err) + } + w.Logger.Debugf("performing job %s", job) if job.Handler == "" { - err := fmt.Errorf("no handler name given for %s", job) + err := fmt.Errorf("no handler name given: %s", job) w.Logger.Error(err) return err } @@ -89,7 +107,7 @@ func (w *Simple) Perform(job Job) error { w.moot.Lock() defer w.moot.Unlock() if h, ok := w.handlers[job.Handler]; ok { - // TODO: consider to implement timeout and/or cancellation + // TODO(sio4): #road-to-v1 - consider timeout and/or cancellation w.wg.Add(1) go func() { defer w.wg.Done() @@ -118,11 +136,20 @@ func (w *Simple) PerformAt(job Job, t time.Time) error { // PerformIn performs a job after waiting for a specified amount // using a goroutine. func (w *Simple) PerformIn(job Job, d time.Duration) error { + // Perform should not allow a job submission if the worker is not running + if err := w.ctx.Err(); err != nil { + return fmt.Errorf("worker is not ready to perform a job: %v", err) + } + + w.wg.Add(1) // waiting job also should be counted go func() { + defer w.wg.Done() + select { case <-time.After(d): w.Perform(job) case <-w.ctx.Done(): + // TODO(sio4): #road-to-v1 - it should be guaranteed to be performed w.cancel() } }() diff --git a/worker/simple_test.go b/worker/simple_test.go index 6372d60ca..7abf0b499 100644 --- a/worker/simple_test.go +++ b/worker/simple_test.go @@ -1,6 +1,7 @@ package worker import ( + "context" "sync" "testing" "time" @@ -8,76 +9,188 @@ import ( "github.com/stretchr/testify/require" ) +func sampleHandler(Args) error { + return nil +} + +func Test_Simple_RegisterEmpty(t *testing.T) { + r := require.New(t) + + w := NewSimple() + err := w.Register("", sampleHandler) + r.Error(err) +} + +func Test_Simple_RegisterNil(t *testing.T) { + r := require.New(t) + + w := NewSimple() + err := w.Register("sample", nil) + r.Error(err) +} + +func Test_Simple_RegisterEmptyNil(t *testing.T) { + r := require.New(t) + + w := NewSimple() + err := w.Register("", nil) + r.Error(err) +} + +func Test_Simple_RegisterExisting(t *testing.T) { + r := require.New(t) + + w := NewSimple() + err := w.Register("sample", sampleHandler) + r.NoError(err) + + err = w.Register("sample", sampleHandler) + r.Error(err) +} + +func Test_Simple_StartStop(t *testing.T) { + r := require.New(t) + + w := NewSimple() + ctx := context.Background() + err := w.Start(ctx) + r.NoError(err) + r.NotNil(w.ctx) + r.Nil(w.ctx.Err()) + + err = w.Stop() + r.NoError(err) + r.NotNil(w.ctx) + r.NotNil(w.ctx.Err()) +} + func Test_Simple_Perform(t *testing.T) { r := require.New(t) var hit bool - wg := &sync.WaitGroup{} - wg.Add(1) w := NewSimple() + r.NoError(w.Start(context.Background())) + w.Register("x", func(Args) error { hit = true - wg.Done() return nil }) w.Perform(Job{ Handler: "x", }) - wg.Wait() + + // the worker should guarantee the job is finished before the worker stopped + r.NoError(w.Stop()) r.True(hit) } -func Test_Simple_PerformAt(t *testing.T) { +func Test_Simple_PerformBroken(t *testing.T) { r := require.New(t) var hit bool - wg := &sync.WaitGroup{} - wg.Add(1) w := NewSimple() + r.NoError(w.Start(context.Background())) + w.Register("x", func(Args) error { hit = true - wg.Done() + + //Index out of bounds on purpose + println([]string{}[0]) + return nil }) - w.PerformAt(Job{ + w.Perform(Job{ Handler: "x", - }, time.Now().Add(5*time.Millisecond)) - wg.Wait() + }) + + r.NoError(w.Stop()) r.True(hit) } -func Test_Simple_PerformBroken(t *testing.T) { +func Test_Simple_PerformWithEmptyJob(t *testing.T) { + r := require.New(t) + + w := NewSimple() + r.NoError(w.Start(context.Background())) + defer w.Stop() + + err := w.Perform(Job{}) + r.Error(err) +} + +func Test_Simple_PerformWithUnknownJob(t *testing.T) { + r := require.New(t) + + w := NewSimple() + r.NoError(w.Start(context.Background())) + defer w.Stop() + + err := w.Perform(Job{Handler: "unknown"}) + r.Error(err) +} + +/* TODO(sio4): #road-to-v1 - define the purpose of Start clearly + consider to make Perform to work only when the worker is started. +func Test_Simple_PerformBeforeStart(t *testing.T) { + r := require.New(t) + + w := NewSimple() + r.NoError(w.Register("sample", sampleHandler)) + + err := w.Perform(Job{Handler: "sample"}) + r.Error(err) +} +*/ + +func Test_Simple_PerformAfterStop(t *testing.T) { + r := require.New(t) + + w := NewSimple() + r.NoError(w.Register("sample", sampleHandler)) + r.NoError(w.Start(context.Background())) + r.NoError(w.Stop()) + + err := w.Perform(Job{Handler: "sample"}) + r.Error(err) +} + +func Test_Simple_PerformAt(t *testing.T) { r := require.New(t) var hit bool + w := NewSimple() + r.NoError(w.Start(context.Background())) + wg := &sync.WaitGroup{} wg.Add(1) - w := NewSimple() w.Register("x", func(Args) error { hit = true wg.Done() - - //Index out of bounds on purpose - println([]string{}[0]) - return nil }) - - w.Perform(Job{ + w.PerformAt(Job{ Handler: "x", - }) + }, time.Now().Add(5*time.Millisecond)) + + // how long does the handler take for assignment? hmm, + time.Sleep(100 * time.Millisecond) wg.Wait() r.True(hit) + + r.NoError(w.Stop()) } func Test_Simple_PerformIn(t *testing.T) { r := require.New(t) var hit bool + w := NewSimple() + r.NoError(w.Start(context.Background())) + wg := &sync.WaitGroup{} wg.Add(1) - w := NewSimple() + w.Register("x", func(Args) error { hit = true wg.Done() @@ -86,14 +199,61 @@ func Test_Simple_PerformIn(t *testing.T) { w.PerformIn(Job{ Handler: "x", }, 5*time.Millisecond) + + // how long does the handler take for assignment? hmm, + time.Sleep(100 * time.Millisecond) wg.Wait() r.True(hit) + + r.NoError(w.Stop()) } -func Test_Simple_NoHandler(t *testing.T) { +/* TODO(sio4): #road-to-v1 - define the purpose of Start clearly + consider to make Perform to work only when the worker is started. +func Test_Simple_PerformInBeforeStart(t *testing.T) { r := require.New(t) w := NewSimple() - err := w.Perform(Job{}) + r.NoError(w.Register("sample", sampleHandler)) + + err := w.PerformIn(Job{Handler: "sample"}, 5*time.Millisecond) + r.Error(err) +} +*/ + +func Test_Simple_PerformInAfterStop(t *testing.T) { + r := require.New(t) + + w := NewSimple() + r.NoError(w.Register("sample", sampleHandler)) + r.NoError(w.Start(context.Background())) + r.NoError(w.Stop()) + + err := w.PerformIn(Job{Handler: "sample"}, 5*time.Millisecond) r.Error(err) } + +/* TODO(sio4): #road-to-v1 - it should be guaranteed to be performed + consider to make PerformIn to guarantee the job execution +func Test_Simple_PerformInFollowedByStop(t *testing.T) { + r := require.New(t) + + var hit bool + w := NewSimple() + r.NoError(w.Start(context.Background())) + + w.Register("x", func(Args) error { + hit = true + return nil + }) + err := w.PerformIn(Job{ + Handler: "x", + }, 5*time.Millisecond) + r.NoError(err) + + // stop the worker immediately after PerformIn + r.NoError(w.Stop()) + + r.True(hit) +} +*/