refactor: Move a transport protocol-related logic from the framework core #280
Conversation
jakubkoci
changed the title
| jest.mock('../TransportService') | ||
| jest.mock('../EnvelopeService') | ||
|
|
||
| const logger = testLogger |
There was a problem hiding this comment.
maybe just import logger from ../../__tests__/logger?
There was a problem hiding this comment.
I like const logger = testLogger more because you can simply change it to const logger = new ConsoleLogger(...) when want to log into the console during the development.
| * @returns mock function with type annotations | ||
| */ | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| function mockFunction<T extends (...args: any[]) => any>(fn: T): jest.MockedFunction<T> { |
There was a problem hiding this comment.
Can't we import this from the helpers file?
There was a problem hiding this comment.
Ahh, I didn't know about that. I updated the JS doc at least :)
| export class WebSocketTransportSession implements TransportSession { | ||
| public readonly type = 'websocket' | ||
| public socket?: WebSocket | ||
|
|
||
| public constructor(socket?: WebSocket) { | ||
| this.socket = socket | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Is the transport session for the inbound transport, outbound transport or both?
There was a problem hiding this comment.
Hmm. That's a good question. I would say it's for both. A developer sets the session when calling agent.recieveMessage(message, session) passing inbound message. Then it can be used for outbound messages.
|
My last PR broke the test and got into main 😬. I'm on it Fixed in #282 |
Signed-off-by: Jakub Koci <jakub.koci@gmail.com>
…text Signed-off-by: Jakub Koci <jakub.koci@gmail.com>
Signed-off-by: Jakub Koci <jakub.koci@gmail.com>
Signed-off-by: Jakub Koci <jakub.koci@gmail.com>
|
Thanks, @TimoGlastra for the review and suggestions 👍 PR updated. |
Codecov Report
@@ Coverage Diff @@
## main #280 +/- ##
==========================================
- Coverage 88.76% 88.63% -0.13%
==========================================
Files 216 216
Lines 3899 3881 -18
Branches 438 431 -7
==========================================
- Hits 3461 3440 -21
- Misses 438 441 +3
Continue to review full report at Codecov.
|
I did some refactorings to help me add support for multiple transports in the following PR:
MessageSenderEnvelopeServicetoMessageSenderTransportServiceand from the core of the framework.transporttosessionwhich hopefully express the meaning better.As a next step I'm considering:
MessageSender.packMessageand passserviceobject topackMessagepackMessageandsendMessagefor every available service. Eventually, we could remove the logic fromcreateOutboundMessage.outboundMessageinsideOutboundTransporterand error handlingIf you think it doesn't make sense to merge this without full functionality I can continue with adding changes. But I realized that it contains a good amount of changes already and it could be worth merging sooner.