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

feat(Response): expose method to reset the response #346

Merged
merged 2 commits into from
Jan 28, 2017

Conversation

munendrasn
Copy link
Collaborator

@decebals,
This is required because on calling httpServletResponse.reset(), only response gets reset. But other params like headers, cookies also need to erased from Response class

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 18.214% when pulling 02b743e on munendrasn:add-reset into b6a195c on decebals:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 18.214% when pulling 02b743e on munendrasn:add-reset into b6a195c on decebals:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.8%) to 19.023% when pulling 1d2ada1 on munendrasn:add-reset into b6a195c on decebals:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.8%) to 19.023% when pulling 40f3c9d on munendrasn:add-reset into b6a195c on decebals:master.

/**
* @author munendrasn
*/
@RunWith(MockitoJUnitRunner.class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that we need this annotation. This annotation is useful if you use mock annotation, for example

@Mock
private HttpServletResponse servletResponse;

instead of

private HttpServletResponse servletResponse = mock(HttpServletResponse.class);

import javax.servlet.http.HttpServletResponse;

import java.nio.charset.StandardCharsets;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra empty line (please use optimize import from IntelliJ for example)

@decebals
Copy link
Member

I am happy to see unit test code.
Also, as a curiosity, when you use Response.rest() method (a use case). Thanks!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.8%) to 19.023% when pulling 33cb7ff on munendrasn:add-reset into b6a195c on decebals:master.

@munendrasn
Copy link
Collaborator Author

@decebals ,
In some cases, I'm writing directly to writer or outputStream. Also, setting headers for response using response.header() or routeContext.setHeader().
Now, If any error or exception occurs in application, all set headers need to be removed (using servletResponse.reset()).

But, this doesn't reset headers, cookies in Response. Hence, exposing this helper will help in required reset

@decebals decebals merged commit 22bbc72 into pippo-java:master Jan 28, 2017
@munendrasn munendrasn deleted the add-reset branch January 28, 2017 09:48
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

Successfully merging this pull request may close these issues.

None yet

3 participants