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

Natively Implement URLConnection #418

Open
jimfb opened this issue Jan 21, 2016 · 8 comments
Open

Natively Implement URLConnection #418

jimfb opened this issue Jan 21, 2016 · 8 comments

Comments

@jimfb
Copy link

jimfb commented Jan 21, 2016

I'm not sure if this is a good idea or a terrible idea, but here it goes...

Many java applications use URL.openConnection() to create a URLConnection which is then used to request data (ala ajax). In many cases, this could "just work" (if backed by an XHR request only when a websocket proxy is unavailable), and would be very intuitive. Some edge cases (like setting custom headers) would be unsupported and/or would need to throw a security exception or something.

Anyway, the downside is that some operations (like setting arbitrary headers?) would not be legal and would need to throw an exception, but that's no worse than the status quo where we throw an exception because the user hasn't setup a websocket proxy. I hate lying to users (screwing with URLConnection feels like a lie), but it would probably result in a very measurable chunk of code (like URLClassLoader) working out of the box.

Anyway, food for thought. cc @jvilk

@jvilk
Copy link
Member

jvilk commented Jan 23, 2016

I think it would be a great idea to have a DoppioURLConnection class, but I don't have the time to implement one.

I'd accept a PR for one, but it'd need to be accompanied by unit tests that use HttpURLConnection on the native JVM and DoppioURLConnection in Doppio.

@hrj
Copy link
Contributor

hrj commented Mar 6, 2016

Do I correctly understand this:
The JVM application can register a URLStreamHandlerFactory that allows a custom stream handler to be created. The stream handler is the one responsible for creating instances of HttpURLConnection.

Now, although it might be convenient to have an implementation of HttpURLConnection based on XHR in doppio, it is not required to be part of doppio's standard library. The application can implement and use its own URLStreamHandlerFactory and thus its own HttpURLConnection implementation.

If we really want it to be part of doppio itself, I could implement a XHR based stream-handler-factory and contribute it to doppio. The application can then choose to use this factory when it wants XHR, or use the default when it wants websockets. This way, the choice is made explicit.

Edit: some typos and clarifications.

@jvilk
Copy link
Member

jvilk commented Mar 6, 2016

Everything you've said is accurate from what I understand.

It would be really nice to have the XHR-based stream-handler-factory as a part of DoppioJVM itself. I feel like most users would want that by default. Perhaps we could introduce a system property that toggled its automatic registration on or off -- which would prevent people from needing to recompile their existing code to run in DoppioJVM.

@jvilk
Copy link
Member

jvilk commented May 8, 2016

@jimfb are you behind JavaPoly.js? I see a Jim is involved, but I dunno if it is you. Looks like they created what you proposed.

@jimfb
Copy link
Author

jimfb commented May 8, 2016

@jvilk Yep, that's me. We'd be happy to contribute any of this stuff upstream if you'd like it. I had originally thought this would require hacking/overriding system classes (which is why I created this issue), but it turned out to be possible to implement this completely in user land, so that's what we did. However, if you'd like this (or any other feature) to be merged upstream, just let us know and we'll get that process started!

@jvilk
Copy link
Member

jvilk commented May 8, 2016

Aha! So now things start to make sense. :)

Yes, I would really like this to be merged upstream. I lack the time to take any initiative on this until later in the week, though.

@hrj
Copy link
Contributor

hrj commented May 9, 2016

I could contribute the code now. However, it would be nice to have some clarity on #441 and #442 just to avoid breakage in case I haven't understood something correctly.

@nybbs2003
Copy link

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants