Add more transports, schema validation and refactorings #72
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The parser in the test assumes that it will get only 1 message i.e the message of the pong. However actually, the register request's response is also received by it. Since register is tested elsewhere and not needed to test ping, do not register in this test. Signed-off-by: Lucifer Morningstar <[email protected]>
The parser in the test assumes that it will get only 1 message i.e the message of the pong. However actually, three messages will arrive. 1 ACK for register, 1 ACK for publish and finally the message for publish message itself. Update the test to handle 3 messages. Signed-off-by: Lucifer Morningstar <[email protected]>
The parser in the test was not handling messages. Add handling for 1 ACK for register, 1 ACK for publish and 1 ACK for unregister in addition to publish and send message. The error checking also looks inconsistent/broken at the moment for now fix by conforming to whatever bridge returns. Reevaluate after tests are fixed. Signed-off-by: Lucifer Morningstar <[email protected]>
Fix test assertions like message format for invalid send. Signed-off-by: Lucifer Morningstar <[email protected]>
The JSON-RPC 2.0 spec mandates value of id in response to be same as that in the request. The broken test assertion was trying to test otherwise, hence was wrong. Signed-off-by: Lucifer Morningstar <[email protected]>
The format of message in JSON-RPC is different from TCP bridge so need to fix test assertions accordingly. Signed-off-by: Lucifer Morningstar <[email protected]>
The implementation is looking for a top level failureCode field in the message which would be right for Vertx protocol but not for the JSON-RPC implementation. For now, I have changed it to look for an error field in the params. This may need adjusting as I couldn't find the case of JSON-RPC client sending back error message to server in the spec. Signed-off-by: Lucifer Morningstar <[email protected]>
Signed-off-by: Paulo Lopes <[email protected]>
Signed-off-by: Paulo Lopes <[email protected]>
This way we can configure WS connections to send response in text or binary.
…rn type to latter
For http request, end handler will always be invoked after the request data has been read. While for websockets and tcp sockets, it is only invoked at end of connection. So, it makes sense to have it in the latter but not in http transport. Also, the actions we took in the end handler was remove existing subscriptions but that should actually be done when response has ended. Therefore, added it as bodyHandler to response instead.
For http request, end handler will always be invoked after the request data has been read. While for websockets and tcp sockets, it is only invoked at end of connection. So, it makes sense to have it in the latter but not in http transport. Also, the actions we took in the end handler was remove existing subscriptions but that should actually be done when response has ended. Therefore, added it as bodyHandler to response instead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR covers a lot of stuff.