From 3218982b3a7e12e3f5a17ef10549dfda74c47dc1 Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Mon, 11 Sep 2023 08:14:59 +0100 Subject: [PATCH] Engine: Integrate IsRunning into order manager interface; enhance websocket manager accordingly (#1337) * Engine: Fix false test passes for nil OrderManager TestWebsocketRoutineManagerSetup tests that passing a nil value returns errNilOrderManager; However that's not actually what would happen when order manager is configured off. The arguments to setupWebsocketRoutineManager are interface types. When a nil pointer of interface type is passed, it does NOT equal nil. nil is a primitive type. A nil pointer of interface type has the value (nil;type). See [the Go lang spec](https://golang.org/ref/spec#Comparison_operators): ``` Interface values are comparable. Two interface values are equal if they have identical dynamic types and equal dynamic values or if both have value nil. ``` So that means that whilst this test was passing, because it was sending in a real nil value and comparing it to a real nil, that's not what would happen at runtime. At runtime the bot.OrderManager would be a nil pointer to a concrete type *OrderManager, and so not comparible to nil. This commit just fixes that oversight, and explains the often misunderstood mechanics of comparing interface types to nil. In practical terms this means that the tests assert that the WSRM would not run without a OM, but in fact it actually would. And panic later. This commit SHOULD introduce a FAILing test. Sorry if you're bisecting. * WSM: Fix error on OrderManager not enabled It's okay for OrderManager to not be enabled; It's configurable. Remove the WSM setup protection and move it to runtime using IsRunning. Left the WSM deps so they can be fixed as apporpriate to each. --- engine/subsystem_types.go | 1 + engine/websocketroutine_manager.go | 9 ++++++--- engine/websocketroutine_manager_test.go | 7 +------ engine/websocketroutine_manager_types.go | 1 - 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/engine/subsystem_types.go b/engine/subsystem_types.go index 5b245f752c8..e64337759ec 100644 --- a/engine/subsystem_types.go +++ b/engine/subsystem_types.go @@ -53,6 +53,7 @@ type iCommsManager interface { // iOrderManager defines a limited scoped order manager type iOrderManager interface { + IsRunning() bool Exists(*order.Detail) bool Add(*order.Detail) error Cancel(context.Context, *order.Cancel) error diff --git a/engine/websocketroutine_manager.go b/engine/websocketroutine_manager.go index ff56ad5877f..55bb744285a 100644 --- a/engine/websocketroutine_manager.go +++ b/engine/websocketroutine_manager.go @@ -22,9 +22,6 @@ func setupWebsocketRoutineManager(exchangeManager iExchangeManager, orderManager if exchangeManager == nil { return nil, errNilExchangeManager } - if orderManager == nil { - return nil, errNilOrderManager - } if syncer == nil { return nil, errNilCurrencyPairSyncer } @@ -287,6 +284,9 @@ func (m *WebsocketRoutineManager) websocketDataHandler(exchName string, data int } m.syncer.PrintOrderbookSummary(base, "websocket", nil) case *order.Detail: + if !m.orderManager.IsRunning() { + return nil + } if !m.orderManager.Exists(d) { err := m.orderManager.Add(d) if err != nil { @@ -310,6 +310,9 @@ func (m *WebsocketRoutineManager) websocketDataHandler(exchName string, data int m.printOrderSummary(od, true) } case []order.Detail: + if !m.orderManager.IsRunning() { + return nil + } for x := range d { if !m.orderManager.Exists(&d[x]) { err := m.orderManager.Add(&d[x]) diff --git a/engine/websocketroutine_manager_test.go b/engine/websocketroutine_manager_test.go index 3d175334767..a864d52a289 100644 --- a/engine/websocketroutine_manager_test.go +++ b/engine/websocketroutine_manager_test.go @@ -21,12 +21,7 @@ func TestWebsocketRoutineManagerSetup(t *testing.T) { t.Errorf("error '%v', expected '%v'", err, errNilExchangeManager) } - _, err = setupWebsocketRoutineManager(NewExchangeManager(), nil, nil, nil, false) - if !errors.Is(err, errNilOrderManager) { - t.Errorf("error '%v', expected '%v'", err, errNilOrderManager) - } - - _, err = setupWebsocketRoutineManager(NewExchangeManager(), &OrderManager{}, nil, nil, false) + _, err = setupWebsocketRoutineManager(NewExchangeManager(), (*OrderManager)(nil), nil, nil, false) if !errors.Is(err, errNilCurrencyPairSyncer) { t.Errorf("error '%v', expected '%v'", err, errNilCurrencyPairSyncer) } diff --git a/engine/websocketroutine_manager_types.go b/engine/websocketroutine_manager_types.go index c6d2bccf338..17780f26c87 100644 --- a/engine/websocketroutine_manager_types.go +++ b/engine/websocketroutine_manager_types.go @@ -8,7 +8,6 @@ import ( ) var ( - errNilOrderManager = errors.New("nil order manager received") errNilCurrencyPairSyncer = errors.New("nil currency pair syncer received") errNilCurrencyConfig = errors.New("nil currency config received") errNilCurrencyPairFormat = errors.New("nil currency pair format received")