Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experimental/jsonrpc #73

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Experimental/jsonrpc #73

wants to merge 36 commits into from

Conversation

pmlopes
Copy link
Member

@pmlopes pmlopes commented Oct 25, 2022

Motivation:

This is the final project from Google Summer of code 2022 by @lucifer4j.

This PR is the preparation to merge the contribution to the master branch.

pmlopes and others added 30 commits October 25, 2022 14:19
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]>
This way we can configure WS connections to send response in text or
binary.
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.
lucifer4j and others added 6 commits October 25, 2022 14:20
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.
Signed-off-by: Paulo Lopes <[email protected]>
Signed-off-by: Paulo Lopes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants