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

Add a setter for middleware #577

Merged
merged 10 commits into from
Nov 24, 2021
Merged

Conversation

dvdplm
Copy link
Contributor

@dvdplm dvdplm commented Nov 23, 2021

…and an example.

Came about as way to review #576

impl middleware::Middleware for ManInTheMiddle {
type Instant = u64;
fn on_request(&self) -> Self::Instant {
self.when + 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.when + 1
self.when += 1;
self.when

@@ -56,7 +56,7 @@ const MAX_CONNECTIONS: u64 = 100;

/// A WebSocket JSON RPC server.
#[derive(Debug)]
pub struct Server<M = ()> {
pub struct Server<M> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was trying to make Builder::default() work and this seemed to be in the way, but I don't think this was the problem. OTOH: do we need it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep it, all it does is if you don't use middleware you can just write WsServer instead of WsServer<()>.

ws-server/src/server.rs Outdated Show resolved Hide resolved
ws-server/src/server.rs Outdated Show resolved Hide resolved
dvdplm and others added 2 commits November 24, 2021 14:18
Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>
Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>
Copy link
Contributor

@maciejhirsz maciejhirsz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to merge at will, any kind of polish can be done on the upstream branch too.

ws-server/src/server.rs Outdated Show resolved Hide resolved
Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>
@dvdplm dvdplm marked this pull request as ready for review November 24, 2021 13:27
@dvdplm dvdplm merged commit c8eac37 into mh-metrics-middleware Nov 24, 2021
@dvdplm dvdplm deleted the dp-add-middleware-setter branch November 24, 2021 13:27
dvdplm added a commit that referenced this pull request Dec 1, 2021
* Squashed MethodSink

* Middleware WIP

* Passing all the information through

* Unnecessary `false`

* Apply suggestions from code review

Co-authored-by: David <dvdplm@gmail.com>

* Add a setter for middleware (#577)

* Fix try-build tests

* Add a middleware setter and an example

* Actually add the example

* Grumbles

* Use an atomic

* Set middleware with a constructor instead

* Resolve a todo

* Update ws-server/src/server.rs

Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>

* Update ws-server/src/server.rs

Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>

* Update ws-server/src/server.rs

Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>

Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>

* Middleware::on_response for batches

* Middleware in HTTP

* fmt

* Server builder for HTTP

* Use actual time in the example

* HTTP example

* Middleware to capture method not found calls

* An example of adding multiple middlewares. (#581)

* Add an example of adding multiple middlewares.

* Update examples/multi-middleware.rs

Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>

* Update examples/Cargo.toml

Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>

Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>

* Move `Middleware` to jsonrpsee-types (#582)

* Move `Middleware` to jsonrpsee-types

* Move Middleware trait to jsonrpsee-types

* Add some docs.

* Link middleware to `with_middleware` methods in docs

* Doctests

* Doc comment fixed

* Clean up a TODO

* Switch back to `set_middleware`

* fmt

* Tests

* Add `on_connect` and `on_disconnect`

* Add note to future selves

Co-authored-by: David <dvdplm@gmail.com>
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.

2 participants