Add new websocket handler and skeleton for its deps #6630
Conversation
* Added websocket controller * Added mock block provider * Added data provider factory * Added websocket handler * Added websocket config * Added a tiny POC test for websocket handler
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6630 +/- ##
===========================================
- Coverage 41.22% 40.95% -0.28%
===========================================
Files 2054 66 -1988
Lines 182347 9199 -173148
===========================================
- Hits 75175 3767 -71408
+ Misses 100871 5153 -95718
+ Partials 6301 279 -6022
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Speaking of tests, I see it the following way: we should write a wrap websocket connection with our custom mock and write unit tests for websocket controller (it accepts ws conn as argument). This will require some code, so a separate task should be created for this #6631 |
| return websocket.DefaultDialer.Dial(wsURL, nil) | ||
| } | ||
|
|
||
| func (s *WsHandlerSuite) TestSubscribeRequest() { |
There was a problem hiding this comment.
This test is only to make sure we get back data from data providers. this should removed in favor of #6631
* Use contexts as function arguments * Move models to folder for consistency * Change parse msg function * Simplify mock block provider to remove dedlock
|
|
||
| func (c *Controller) handleAction(ctx context.Context, action string, message interface{}) error { |
There was a problem hiding this comment.
rather than passing the action, you can use the type in the switch. this also allows you to avoid casting
| func (c *Controller) handleUnsubscribe(ctx context.Context, msg models.UnsubscribeMessageRequest) { | ||
| id, err := uuid.Parse(msg.ID) | ||
| if err != nil { | ||
| c.logger.Debug().Err(err).Msg("error parsing message ID") |
There was a problem hiding this comment.
we should return an error response to the client here.
There was a problem hiding this comment.
| func (c *Controller) handleSubscribe(ctx context.Context, msg models.SubscribeMessageRequest) { | ||
| dp := c.dataProvidersFactory.NewDataProvider(ctx, c.communicationChannel, msg.Topic) | ||
| c.dataProviders.Add(dp.ID(), dp) | ||
| dp.Run(ctx) |
There was a problem hiding this comment.
we should return a response here
| } | ||
|
|
||
| controller := NewWebSocketController(logger, h.websocketConfig, h.streamApi, h.streamConfig, conn) | ||
| controller.HandleConnection(context.TODO()) |
There was a problem hiding this comment.
i believe this needs to be blocking, and HandleConnection is currently non-blocking
| config Config | ||
| conn *websocket.Conn | ||
| communicationChannel chan interface{} | ||
| dataProviders *concurrentmap.ConcurrentMap[uuid.UUID, dp.DataProvider] |
There was a problem hiding this comment.
We could use string here, as a key, as this is what we get from Subscription
There was a problem hiding this comment.
I think uuid is more compact, faster to compare, and better named for ID
There was a problem hiding this comment.
The idea here was to reuse the existing ID from subscription.Subscriptionas DataProvider ID, as it was already generated as UUID (uuid.New().String()) but stored as a string.
Closes #6593 #6583 #6617
Relates to #6584