Provide an adapted implementation of HttpResponseExpectation #317
Conversation
There was a problem hiding this comment.
I haven't read the full PR yet as I noticed the implementation consists in a custom operator.
As far as I know, it is recommended to favor composition (io.reactivex.Single#compose) instead of custom operators (io.reactivex.Single#lift).
As the Javadoc of lift suggests, there are some optimizations that a custom operator must implement itself, that built-in operators implement already.
Besides, if I remember correctly, this is what @jponge recommends for Mutiny extensions as well (start with composition and use custom operators as a last resort).
I would imagine the same feature (HttpResponse expectations) will be implemented with composition in Mutiny, it would be nice to be consistent.
I will send a PR to your branch to show the difference (and how small it is in user experience at the same time).
See #318 |
|
excellent @tsegismont I merged your PR to this PR |
|
@tsegismont constants is not possible because of the variance that needs to adapt to the actual single type |
|
I'll let this PR in your hands @tsegismont |
tsegismont
force-pushed
the
http-response-expectation
branch
from
2e9dced
to
69618c3
Compare
Motivation: HttpResponseExpectation as defined in vertx-core is specific to Vert.x Future API and cannot be reused by RxJava as is. Changes: Add an adaptor for HttpResponseExpectation that leverages the lift operator to bridge this feature.
As far as I know, it is recommended to favor composition (io.reactivex.Single#compose) instead of custom operators (io.reactivex.Single#lift). As the Javadoc of lift suggests, there are some optimizations that a custom operator must implement itself, that built-in operators implement already. Besides, if I remember correctly, this is what @jponge recommends for Mutiny extensions as well (start with composition and use custom operators as a last resort). I would imagine the same feature (HttpResponse expectations) will be implemented with composition in Mutiny, it would be nice to be consistent. Signed-off-by: Thomas Segismont <[email protected]>
Signed-off-by: Thomas Segismont <[email protected]>
tsegismont
force-pushed
the
http-response-expectation
branch
from
f8e6dca
to
8067565
Compare
|
I've added documentation, and I'm now waiting for feedback from @jponge before merging. |
|
Looking into it shortly |
|
It's in line with what we had discussed for Mutiny, aka we'd need something that turns Vert.x expectations into Mutiny operators so they can be |
|
Thanks @jponge |
Motivation:
HttpResponseExpectation as defined in vertx-core is specific to Vert.x Future API and cannot be reused by RxJava as is.
Changes:
Add an adaptor for HttpResponseExpectation that leverages the lift operator to bridge this feature.