shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Les Hazlewood <lhazlew...@apache.org>
Subject Re: SessionManager API modification
Date Wed, 19 May 2010 16:28:43 GMT
Hi Brian,

Yeah, the work done was specifically to remove the brittleness of the
thread binding/unbinding requirements - it was rather pervasive across
the codebase and made many parts feel rather 'hacky'.

Now there are only two objects bound to the thread - a Subject and a
SecurityManager, both of which are managed automatically by
ThreadState mechanisms to bind and unbind as necessary.  In web
applications a WebSubject additionally retains the request/response
pair that triggered its creation.

In my opinion, the best way for anything to ensure this thread binding
occurs and is released automatically is to use the Subject Builder
mechanisms - they are especially useful in test cases:

//normal app:
Subject subject = Subject.Builder(securityManager). ... .buildSubject();
subject.execute(new Runnable() {
    public void run() {
        //execute logic/tests here that needs to call SecurityUtils.getSubject()
    }
}

You do the same thing with web tests, but you use the WebSubject.Builder:
Subject subject = WebSubject.Builder(securityManager, request,
response). ... .buildSubject();

Web-based interactions require the WebSubject to retain the request
and response - you'll want to ensure you've built them into the
Subject instance as shown above.

If you don't like the subject.execute approach, you can alternatively
build the subject as above, but bind and unbind from the thread
manually.  You typically wouldn't want to do this in application code
because the execute* methods guarantee thread safety - you usually
wouldn't want to lose that safety net/guarantee in app code.  But it
might be useful in tests:

Subject subject = //build subject as above
ThreadContext.bind(securityManager);
ThreadContext.bind(subject);
runTest();
ThreadContext.remove();

The stuff before runTest() could be done in a @Before method and
ThreadContext.remove() could be done in an @After method.

HTH!

Les

On Wed, May 19, 2010 at 5:53 AM, Brian Demers <brian.demers@gmail.com> wrote:
> The new changes work here too ( all my tests pass anyway)
>
> I have one question though.  My base test class was setting mock requests
> and responses.  Which I am assuming is no longer needed now that the
> WebSessionManager doesn't require them..
>
> WebUtils.bind( mockRequest );
> WebUtils.bind( mockResponse );
>
> Is there an equivalent?  If so is there any reason I would want to set them
> for a test.
>
> Thanks
>
> On Wed, May 19, 2010 at 2:17 AM, Les Hazlewood <lhazlewood@apache.org>wrote:
>
>> Thanks again!
>>
>> On Tue, May 18, 2010 at 10:13 PM, Tauren Mills <tauren@tauren.com> wrote:
>> > Now using the latest maven snapshot (170) from:
>> >
>> https://repository.apache.org/content/repositories/snapshots/org/apache/shiro/shiro-core/1.0-incubating-SNAPSHOT/
>> >
>> > All is looking good...
>> >
>> > Tauren
>> >
>> >
>> > On Tue, May 18, 2010 at 7:42 PM, Les Hazlewood <lhazlewood@apache.org>
>> wrote:
>> >> Hi Tauren,
>> >>
>> >> Can you please verify the trunk now that the merge has been completed?
>> >>
>> >> Thanks,
>> >>
>> >> Les
>> >>
>> >> On Tue, May 18, 2010 at 7:34 PM, Tauren Mills <tauren@tauren.com>
>> wrote:
>> >>> Les,
>> >>>
>> >>> I just did an svn update, mvn clean, mvn install of your branch, and
>> >>> so far all looks good. No more exceptions or odd behavior.  Thanks
so
>> >>> much! I'll let you know if I run into anything else.
>> >>>
>> >>> Tauren
>> >>>
>> >>>
>> >>> On Tue, May 18, 2010 at 7:03 PM, Les Hazlewood <lhazlewood@apache.org>
>> wrote:
>> >>>> I've implemented the fix for SHIRO-164 into the branch.  It looks
like
>> >>>> I'm going to have to commit this to trunk, however, all tests pass
and
>> >>>> real-world applications tested against it are doing well.
>> >>>>
>> >>>> Barring any user-reported issues, I think we're all done with code
>> >>>> (maybe some JavaDoc, but no programming).  I'll bump the release
>> >>>> thread next to see what our next steps are.
>> >>>>
>> >>>> On Mon, May 17, 2010 at 5:24 PM, Les Hazlewood <lhazlewood@apache.org>
>> wrote:
>> >>>>> Just a quick note - I'm committing these changes to a branch
for peer
>> >>>>> review.  If good, we can merge.
>> >>>>>
>> >>>>> - Les
>> >>>>>
>> >>>>> On Mon, May 17, 2010 at 3:15 PM, Les Hazlewood <
>> lhazlewood@apache.org> wrote:
>> >>>>>> I've got one thing left to address before resolving SHIRO-162
[1]:
>> >>>>>>
>> >>>>>> Up to this point, the work to remove lots of cross-referenced
>> >>>>>> constants and the ThreadContext usages has gone very well.
 However,
>> >>>>>> it has become readily apparent of the architectural flaws
of the
>> >>>>>> existing SessionManager interface:
>> >>>>>>
>> >>>>>> All of the methods except for 'start' mandate that all session
data
>> >>>>>> can be resolved based on a session ID.  However, this is
definitely
>> >>>>>> not true for ServletContainer environments.
>> >>>>>>
>> >>>>>> So, the web-based SecurityManager and SessionManager implementations
>> >>>>>> still need to resort to brittle ThreadContext usages with
keyed
>> >>>>>> constants to pull ServletRequest/ServletResponse pairs off
of the
>> >>>>>> thread.  It is less than ideal and feels quite hacky in
places.
>> >>>>>>
>> >>>>>> It makes sense to me to move the ID-based methods to a
>> sub-interface,
>> >>>>>> maybe something like NativeSessionManager to account for
this.
>> >>>>>> Without this, the Web SecurityManager/SessionManager implementations
>> >>>>>> will remain complex and somewhat difficult to trace.
>> >>>>>>
>> >>>>>> Since we're still waiting for Infra to enable something
for Kalle
>> >>>>>> before we finish our last issue, what do you guys think
of me doing
>> >>>>>> this today?  I already have an implementation plan in place
(and
>> most
>> >>>>>> stuff is already done, I just don't want to commit without
the dev
>> >>>>>> team being ok with this), and it will be complete today
(with tests)
>> >>>>>> if allowed to proceed.  Note that this has no end-user
impact on the
>> >>>>>> Subject or Session API.
>> >>>>>>
>> >>>>>> Thoughts?
>> >>>>>>
>> >>>>>> Les
>> >>>>>>
>> >>>>>> [1] https://issues.apache.org/jira/browse/SHIRO-162
>> >>>>>>
>> >>>>>
>> >>>>
>> >>>
>> >>
>> >
>>
>

Mime
View raw message