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

Remove dependency of the core on javax.servlet #85

Merged

Conversation

@tbroyer
Copy link
Contributor

@tbroyer tbroyer commented Apr 7, 2013

This is a breaking change but rather with limited impact:

  • Context.getHttpServletRequest / Response methods were @Deprecated already
  • FlashCookie / SessionCookie's save method now take Result as a second parameter, but users are not supposed to call those methods (or are they?)
  • several classes relocated (ninja.asyncninja.servlet.async, ninja.utils.CookieHelpercookie.servlet.CookieHelper, ninja.utils.ResponseStreamsServletninja.servlet.ResponseStreamsServlet)
  • biggest breaking change: introduce a new ninja-servlet module where all servlet-related code lives (so that ninja-core doesn't depend on guice-servlet or javax.servlet)

This paves the way for supporting other containers, such as Netty or Grizzly.

TODO: move the test lib to an in-process model, possibly splitting the jetty-based implementation into its own module (or removing it entirely); similarly to how Jersey 2.0 provides several backends for tests.

@buildhive
Copy link

@buildhive buildhive commented Apr 7, 2013

reyez » ninja #286 SUCCESS
This pull request looks good
(what's this?)

@buildhive
Copy link

@buildhive buildhive commented Apr 7, 2013

reyez » ninja #287 SUCCESS
This pull request looks good
(what's this?)

@buildhive
Copy link

@buildhive buildhive commented Apr 7, 2013

reyez » ninja #288 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@buildhive
Copy link

@buildhive buildhive commented Apr 7, 2013

reyez » ninja #289 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@buildhive
Copy link

@buildhive buildhive commented Apr 7, 2013

reyez » ninja #290 SUCCESS
This pull request looks good
(what's this?)

@buildhive
Copy link

@buildhive buildhive commented Apr 7, 2013

reyez » ninja #291 SUCCESS
This pull request looks good
(what's this?)

@raphaelbauer
Copy link
Contributor

@raphaelbauer raphaelbauer commented Apr 8, 2013

Hi Thomas,

looks really good.

Just one thing: Why did you add Result to the Session and Flash Scopes? When you finalize the headers the request stuff gets copied over automatically... https://github.com/reyez/ninja/blob/develop/ninja-core/src/main/java/ninja/servlet/ContextImpl.java and finalizeHeaders . Maybe I am missing something here...

Thanks,

raphael

@tbroyer
Copy link
Contributor Author

@tbroyer tbroyer commented Apr 8, 2013

Have a look at the changes in FlashCookieImpl and SessionCookieImpl: they used getHttpRequestResponse to addCookie. If you remove that method, then they have no way to add a cookie to the response other than Result#addCookie, which is the proper of doing it: read from Context, write to Result.

@raphaelbauer
Copy link
Contributor

@raphaelbauer raphaelbauer commented Apr 8, 2013

Ah yes. I missed that one. Very cool Thomas...

raphaelbauer added a commit that referenced this pull request Apr 8, 2013
Remove dependency of the core on javax.servlet
@raphaelbauer raphaelbauer merged commit d730a1f into ninjaframework:develop Apr 8, 2013
@tbroyer tbroyer deleted the tbroyer:remove-servlet-dependency branch Apr 9, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants