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

vertx-rx-java - Unnecessary wrapping of handlers while invoking setters on delegate #1463

Closed
techpavan opened this issue Jun 10, 2016 · 7 comments

Comments

@techpavan
Copy link

While consuming vertx-rx-java, I see that handlers set on delegate are wrapped objects of input parameters. This seems to pose two issues -

  1. The input parameters in rxjava as well as core packages are of io.vertx.core.Handler type. Still, in rxjava package, we wrap it to the same type through constructor before passing it to delegate. This creates unnecessary objects.
  2. If null is passed as a parameter to rxjava package, it results in NPE in core package.

Example:
io.vertx.rxjava.core.http.HttpServerResponse#drainHandler, closeHandler

  public HttpServerResponse drainHandler(Handler<Void> handler) { 
    ((io.vertx.core.http.HttpServerResponse) delegate).drainHandler(new Handler<java.lang.Void>() {
      public void handle(java.lang.Void event) {
        handler.handle(event);
      }
    });
    return this;
  }

My use case expects to wait for some data to arrive from an external source and write it to client. I register to drain handler when I have excess amount of data and write queue gets full. When I do not have data to write (as I wait for my source to send some), drain events make no sense and I unregister by setting drain handler to null. Though my overall functionality is not impacted, there are NPEs thrown from vertx and additional objects are created.

@vietj
Copy link
Member

vietj commented Jun 10, 2016

can you still see that with 3.3.0-SNAPSHOT ?

@techpavan
Copy link
Author

Hi @vietj, I still see that in 3.3.0-SNAPSHOT.

@vietj
Copy link
Member

vietj commented Jun 13, 2016

I will try to improve this for this type. Beside the void type do you see any other type it applies to ?

I'll ping you when it is updated

@techpavan
Copy link
Author

Yes, it seems like Buffer (handler / bodyHandler), AsyncResult(push / sendFile), HttpFrame (customFrameHandler), HttpServerFileUpload (uploadHandler) also have issues. AsyncResult methods also have some logic and thus a simple adapter fix cannot help here.

@vietj
Copy link
Member

vietj commented Jun 13, 2016

for Buffer it is not the same class : io.vertx.core.buffer.Buffer versus io.vertx.rxjava.core.buffer.Buffer

On Jun 13, 2016, at 8:17 AM, techpavan notifications@github.com wrote:

Yes, it seems like Buffer (handler / bodyHandler), AsyncResult(push / sendFile), HttpFrame (customFrameHandler), HttpServerFileUpload (uploadHandler) also have issues. AsyncResult methods also have some logic and thus a simple adapter fix cannot help here.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #1463 (comment), or mute the thread https://github.com/notifications/unsubscribe/AANxiinz9QY36gCmlviRLwy_m3TQ0DyRks5qLPX3gaJpZM4Iy6rx.

@vietj
Copy link
Member

vietj commented Jun 13, 2016

btw can you reopen this issue in the correct repository https://github.com/vert-x3/vertx-rx ?

@techpavan
Copy link
Author

Created - vert-x3/vertx-rx#42

Got it about the Buffer, but we would need a null check in that case.

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

No branches or pull requests

2 participants