> On 2011-05-20 00:32:42, Ryan Baxter wrote: > > trunk/java/common/src/main/java/org/apache/shindig/auth/AuthenticationServletFilter.java, > > line 146 > > <https://reviews.apache.org/r/760/diff/1/?file=19450#file19450line146> > > > > Should we add the @Override notation here to indicate your overriding > > the method? It wasn't clear to me until I read your comment.
I think the @Override annotation should be added to methods thats being overriden. Here I just create method with protected scope so it could be overridden if this class is extended. - Henry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/760/#review690 ----------------------------------------------------------- On 2011-05-21 10:03:03, Henry Saputra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/760/ > ----------------------------------------------------------- > > (Updated 2011-05-21 10:03:03) > > > Review request for shindig. > > > Summary > ------- > > Update the AuthenticationServletFilter: > 1. Make the auth realm configurable via property or override able protected > method. > 2. Sets the auth header from the right handler. Currently the code just sets > the response's WWW-Authenticate header whenever an auth handler return null > st. So if the next handler return a security token, the response contains > WWW-Authenticate header from previous handler. This CR change the logic to > also add WWW-Authenticate header if token is not set or > InvalidAuthenticationException is thrown. > > > Diffs > ----- > > > trunk/java/common/src/main/java/org/apache/shindig/auth/AuthenticationServletFilter.java > 1125364 > > trunk/java/common/src/test/java/org/apache/shindig/auth/AuthenticationServletFilterTest.java > 1125364 > > Diff: https://reviews.apache.org/r/760/diff > > > Testing > ------- > > Update unit test for null st. > > > Thanks, > > Henry > >
