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

[webactors] Provide mean to access underlying servlet request (or other implementation) #76

Closed
victornoel opened this issue Oct 5, 2016 · 7 comments

Comments

@victornoel
Copy link
Contributor

Hi,

When receiving an HttpRequest with a WebActor, it is not possible to access the underlying HttpServletRequest (or at least I didn't find a way to do so :) if we are sure the actor is running on a servlet.
My personal need is to be able to access the current HttpSession (because it contains authentication information). Well, I actually need the HttpServletRequest itself because I'm using a library that needs it to access the session as well as the request parameters.

I was expecting to be able to simply cast it to ServletHttpRequest and access the request parameter but the class is not visible and the parameter not accessible.

Would it be possible to change that situation? I guess this must be well thought since you may not want to expose too much of the internal APIs, but if you have a clear idea of how to do that, I can provide a PR :)

Thanks!

@pron
Copy link
Contributor

pron commented Oct 6, 2016

Hmm, this may require some thought. We can't have any types exposed via the API that require a specific dependency which may not exist, and the underlying engine may be different anyway. I'm open to suggestions for an API that returns an Object, which meaning depends on the implementation.

@victornoel
Copy link
Contributor Author

Yep, or at least let a user of the library cast the HttpRequest to one of its underlying type? In that case HttpServletRequest itself can expose HttpServletRequest and others!

No need for a generic API I think, because if at one point there will be a cast, it may be better to simply cast the HttpRequest itself… but then, HttpServletRequest becomes itself part of the actors-servlet API (but not the actors-api).

@victornoel
Copy link
Contributor Author

To be totally clear about what I mean, the idea would simply to make the class co.paralleluniverse.comsat.webactors.servlet.ServletHttpRequest public and add getters for HttpServletRequest request and HttpServletResponse response. That's all :)

@pron
Copy link
Contributor

pron commented Oct 7, 2016

I'm not opposed, but it would require some refactoring. For one, the class is currently named HttpRequestWrapper (which would need to be changed).

@victornoel
Copy link
Contributor Author

right, I didn't see it has been renamed apparently since 0.7.0 :)
What is wrong with the current name by the way? Just that it doesn't seem a very public name?

So, would you like me to propose a PR that does that (and rename HttpRequestWrapper back to HttpServletRequest)?

@pron
Copy link
Contributor

pron commented Oct 8, 2016

Yeah, it's a bad name, and yes a PR would be nice. Let's make it ServletHttpRequest, just to keep the implementation name first.

victornoel added a commit to victornoel/comsat that referenced this issue Oct 10, 2016
- Fixes puniverse#76
- Rename HttpRequestWrapper to {Netty,Undertow,Servlet}HttpRequest
- Make them public class
- Give access to each specific implementation underlying mechanisms
  through getters
@victornoel
Copy link
Contributor Author

I submitted #78! I also changed the name for the other implementations and made some choices as to what is visible publicly for each.

@pron pron closed this as completed in #78 Oct 10, 2016
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