On 07/19/2011 01:54 PM, Les Hazlewood wrote:
> Hi Jared,
>
> You're not the first to come across this gotcha, and I think the
> 'sessionMode' property should be removed (or at least deprecated and
> an exception throw indicating why it is a bad idea).
Doesn't surprise me that I'm not the first.  I'm not sure that the
sessionMode property is necessarily bad - but it should just be a
convenience method, and I don't think that the field should be saved
(and subsequently used to determine whether or not we're using container
session).
> As for a patch, I'd certainly appreciate it - it sounds logical to me,
> but there is a bit of a misnomer:  the 'httpSessionMode' _really_
> means servlet container sessions are used and Shiro's native session
> management is not.  If you had a WebSessionManager interface and the
> DefaultWebSessionManager implementation implemented that interface and
> it returned 'false' for its isHttpSession() implementation, that'd be
> counter intuitive IMO (since it is actually supporting an HttpSession
> implementation).
>
> We're really trying to see if the servlet container is being used to
> support Sessions or not.  Perhaps it should be renamed to
> 'isServletContainerSessions()' to indicate this?  Thoughts?
That certainly makes sense - httpSession is really fairly
generic...container session makes it much clearer what we're talking
about. 

Would you want to change the method name on WebSecurityManager as well? 
The name of the method there is really the reason that I had picked that
for the WebSessionManager.  Also, one thing that I'm not clear on is
behavior for a custom session manager implementation (perhaps using
ehcache as this interface method mentions).  Should it do rewriting like
container sessions, or like native sessions?

Thanks,
Jared

Reply via email to