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

jhttp: Re-work the Bridge interface to require less plumbing. #52

Merged
merged 2 commits into from
Sep 8, 2021

Conversation

creachadair
Copy link
Owner

This change is in support of #46. It is a breaking change to the jhttp.Bridge API.

The purpose of a jhttp.Bridge is to convey requests from an HTTP handler into a
JSON-RPC server. The way I had implemented it, however, required interposing a
client, which complicated the logic of forwarding.

Instead of requiring a client, the Bridge now takes an unstarted server, and
starts it up on an in-memory channel. We still have to parse the requests to
detect whether there are any calls, since HTTP requires a response regardless
of whether the JSON-RPC server replies, but we no longer need to virtualize
request IDs or re-marshal the request values.

The purpose of a jhttp.Bridge is to convey requests from an HTTP handler into a
JSON-RPC server. The way I had implemented it, however, required interposing a
client, which complicated the logic of forwarding.

Instead of requiring a client, the Bridge now takes an unstarted server, and
starts it up on an in-memory channel. We still have to parse the requests to
detect whether there are any calls, since HTTP requires a response regardless
of whether the JSON-RPC server replies, but we no longer need to virtualize
request IDs or re-marshal the request values.

This is a breaking change to the Bridge API.
Allow the owner of the bridge to override the content type check.
@creachadair creachadair merged commit 94d2968 into default Sep 8, 2021
@creachadair creachadair deleted the bridge-rework branch September 8, 2021 23:18
creachadair added a commit that referenced this pull request Sep 12, 2021
Since #52 was merged, this method is no longer used.
creachadair added a commit that referenced this pull request Sep 27, 2021
creachadair added a commit that referenced this pull request Sep 27, 2021
This commit approximately reverts the interface changes to the HTTP Bridge
merged with #52. While those changes did simplify the implementation, they
did so at the cost of usability: Unvirtualized request IDs make it hard for clients
to avoid ID collisions, and the low-level channel interface made it impossible for
the client to access the HTTP request context for authentication.

While this is yet another breaking change to the Bridge interface, it fixes the
problems mentioned in #57, and the resulting interface is a little cleaner.

The main changes here are:

* Update the Bridge API to allow both client and server to be configured.
* Restore ID virtualization.
* Restore the SetID method to *Response.
* Restore and update the tests.
* Restore and improve the HTTP example code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant