From 17724e9f843ba64dace5f13d9af87e0b3062fb09 Mon Sep 17 00:00:00 2001 From: Marcus Olsson Date: Wed, 13 Nov 2019 04:02:44 -0800 Subject: [PATCH] Bus: Remove unused wildcard handlers and clean up tests (#20327) * Refactor bus tests * Remove wildcard listeners * Fix review comments --- pkg/bus/bus.go | 28 ++------- pkg/bus/bus_test.go | 136 ++++++++++++++++++++++++++++---------------- 2 files changed, 90 insertions(+), 74 deletions(-) diff --git a/pkg/bus/bus.go b/pkg/bus/bus.go index 9aa0dedf11c..54caf4907bb 100644 --- a/pkg/bus/bus.go +++ b/pkg/bus/bus.go @@ -38,7 +38,6 @@ type Bus interface { AddHandler(handler HandlerFunc) AddHandlerCtx(handler HandlerFunc) AddEventListener(handler HandlerFunc) - AddWildcardListener(handler HandlerFunc) // SetTransactionManager allows the user to replace the internal // noop TransactionManager that is responsible for manageing @@ -53,11 +52,10 @@ func (b *InProcBus) InTransaction(ctx context.Context, fn func(ctx context.Conte // InProcBus defines the bus structure type InProcBus struct { - handlers map[string]HandlerFunc - handlersWithCtx map[string]HandlerFunc - listeners map[string][]HandlerFunc - wildcardListeners []HandlerFunc - txMng TransactionManager + handlers map[string]HandlerFunc + handlersWithCtx map[string]HandlerFunc + listeners map[string][]HandlerFunc + txMng TransactionManager } // temp stuff, not sure how to handle bus instance, and init yet @@ -69,7 +67,6 @@ func New() Bus { bus.handlers = make(map[string]HandlerFunc) bus.handlersWithCtx = make(map[string]HandlerFunc) bus.listeners = make(map[string][]HandlerFunc) - bus.wildcardListeners = make([]HandlerFunc, 0) bus.txMng = &noopTransactionManager{} return bus @@ -152,21 +149,9 @@ func (b *InProcBus) Publish(msg Msg) error { } } - for _, listenerHandler := range b.wildcardListeners { - ret := reflect.ValueOf(listenerHandler).Call(params) - err := ret[0].Interface() - if err != nil { - return err.(error) - } - } - return nil } -func (b *InProcBus) AddWildcardListener(handler HandlerFunc) { - b.wildcardListeners = append(b.wildcardListeners, handler) -} - func (b *InProcBus) AddHandler(handler HandlerFunc) { handlerType := reflect.TypeOf(handler) queryTypeName := handlerType.In(0).Elem().Name() @@ -207,11 +192,6 @@ func AddEventListener(handler HandlerFunc) { globalBus.AddEventListener(handler) } -// AddWildcardListener attach a handler function to the wildcard listener -func AddWildcardListener(handler HandlerFunc) { - globalBus.AddWildcardListener(handler) -} - func Dispatch(msg Msg) error { return globalBus.Dispatch(msg) } diff --git a/pkg/bus/bus_test.go b/pkg/bus/bus_test.go index 25eaa07ab6a..a0febee3223 100644 --- a/pkg/bus/bus_test.go +++ b/pkg/bus/bus_test.go @@ -13,82 +13,118 @@ type testQuery struct { Resp string } -func TestDispatchCtxCanUseNormalHandlers(t *testing.T) { +func TestDispatch(t *testing.T) { bus := New() - handlerWithCtxCallCount := 0 - handlerCallCount := 0 + var invoked bool - handlerWithCtx := func(ctx context.Context, query *testQuery) error { - handlerWithCtxCallCount++ + bus.AddHandler(func(query *testQuery) error { + invoked = true return nil - } + }) - handler := func(query *testQuery) error { - handlerCallCount++ + err := bus.Dispatch(&testQuery{}) + require.NoError(t, err) + + require.True(t, invoked, "expected handler to be called") +} + +func TestDispatch_NoRegisteredHandler(t *testing.T) { + bus := New() + + err := bus.Dispatch(&testQuery{}) + require.Equal(t, err, ErrHandlerNotFound, + "expected bus to return HandlerNotFound since no handler is registered") +} + +func TestDispatch_ContextHandler(t *testing.T) { + bus := New() + + var invoked bool + + bus.AddHandlerCtx(func(ctx context.Context, query *testQuery) error { + invoked = true return nil - } + }) + + err := bus.Dispatch(&testQuery{}) + require.NoError(t, err) + + require.True(t, invoked, "expected handler to be called") +} + +func TestDispatchCtx(t *testing.T) { + bus := New() + + var invoked bool + + bus.AddHandlerCtx(func(ctx context.Context, query *testQuery) error { + invoked = true + return nil + }) + + err := bus.DispatchCtx(context.Background(), &testQuery{}) + require.NoError(t, err) + + require.True(t, invoked, "expected handler to be called") +} + +func TestDispatchCtx_NoRegisteredHandler(t *testing.T) { + bus := New() err := bus.DispatchCtx(context.Background(), &testQuery{}) require.Equal(t, err, ErrHandlerNotFound, "expected bus to return HandlerNotFound since no handler is registered") - - bus.AddHandler(handler) - - t.Run("when a normal handler is registered", func(t *testing.T) { - err := bus.Dispatch(&testQuery{}) - require.Nil(t, err) - - require.Equal(t, handlerCallCount, 1, - "Expected normal handler to be called 1 time. was called %d", handlerCallCount) - - t.Run("when a ctx handler is registered", func(t *testing.T) { - bus.AddHandlerCtx(handlerWithCtx) - err := bus.Dispatch(&testQuery{}) - require.Nil(t, err) - - require.Equal(t, handlerWithCtxCallCount, 1, - "Expected ctx handler to be called 1 time. was called %d", handlerWithCtxCallCount) - }) - }) - } -func TestQueryHandlerReturnsError(t *testing.T) { +func TestQuery(t *testing.T) { bus := New() + + want := "hello from handler" + + bus.AddHandler(func(q *testQuery) error { + q.Resp = want + return nil + }) + + q := &testQuery{} + + err := bus.Dispatch(q) + require.NoError(t, err, "unable to dispatch query") + + require.Equal(t, want, q.Resp) +} + +func TestQuery_HandlerReturnsError(t *testing.T) { + bus := New() + bus.AddHandler(func(query *testQuery) error { return errors.New("handler error") }) err := bus.Dispatch(&testQuery{}) - require.Error(t, err, "Send query failed") + require.Error(t, err, "expected error but got none") } -func TestQueryHandlerReturn(t *testing.T) { +func TestEvent(t *testing.T) { bus := New() - bus.AddHandler(func(q *testQuery) error { - q.Resp = "hello from handler" - return nil - }) - query := &testQuery{} - err := bus.Dispatch(query) + var invoked bool - require.Nil(t, err, "Send query failed") -} - -func TestEventListeners(t *testing.T) { - bus := New() - count := 0 bus.AddEventListener(func(query *testQuery) error { - count++ - return nil - }) - bus.AddEventListener(func(query *testQuery) error { - count += 10 + invoked = true return nil }) err := bus.Publish(&testQuery{}) - require.Nil(t, err, "Publish event failed") + require.NoError(t, err, "unable to publish event") + + require.True(t, invoked) +} + +func TestEvent_NoRegisteredListener(t *testing.T) { + bus := New() + + err := bus.Publish(&testQuery{}) + require.NoError(t, err, "unable to publish event") }