Skip to content

Conversation

roded
Copy link
Contributor

@roded roded commented Mar 12, 2016

Hi,
I'm not entirely sure this exhausts all the possible execution paths to Shiro realms, but it seems a good enough start.
Roded

@circlespainter
Copy link
Member

Hi, thanks for the contribution.

I'm really not knowledgeable about Shiro but there seems to be no integration code for the framework itself (and actually you don't even need Quasar as a compile dependency for this module at present: a testCompile one would be enough), only suspendable and suspendable-supers, so I think this module doesn't make Shiro's core use fibers. On the other hand you can always spawn and join fibers from threads if the Quasar runtime is running (like the tests are doing if I understand correctly) without special integration (BTW, you can use FiberUtil.runInFiber to do that).

So what's not clear to me is this: which Shiro functionality does the integration allow to be suspendable? [EDIT] Could you also provide some examples/docs?

Also, shouldn't the .ini files be in test/resources?

@circlespainter circlespainter self-assigned this Mar 13, 2016
@roded
Copy link
Contributor Author

roded commented Mar 13, 2016

Your comments are correct.

Shiro allows authentication/authorization functionality via a static method: SecurityUtils.getSubject(). On such access, a user 'Realm' can be queried for users' data and permissions.
In my case, my Realm makes use of the fibered allanbank MongoDB driver so the suspendable definitions are needed for instrumenting the Shiro part of the stack.

I'm not yet entirely sure that the static access to Shiro is the right way to go as it makes use ThreadLocal (which, I suspect, retains the correct state in different Quasar actors since Shiro uses an InheritableThreadLocal). But even otherwise, fibered code will be run via the Shiro stack.

@circlespainter
Copy link
Member

But why the Shiro stack could be suspended in a fiber if your FiberRealm launches a fiber (from a thread a presume) and immediately awaits (that is, blocks on) the fiber's completion? Also, with such a 1( thread):1(fiber) situation you're not making your realm more efficient through fibers.

@roded
Copy link
Contributor Author

roded commented Mar 13, 2016

I might be missing something.
My actual (non-testing) Realm is called directly from an actor's fiber, so the park that's done by the allanbank API it calls needs to propagate through Shiro code.
The new fibers in FiberedRealm are a way to test a park in the realm's methods.
Am I missing something?

@circlespainter
Copy link
Member

I see now, you're calling SecurityUtils.getSubject() from within a fiber spawned by the test thread and not from the thread directly, but then FiberedRealm's doGetAuthorizationInfo and doGetAuthenticationInfo spawn new ones with which the spawning strand synchronizes.

Don't use FiberUtil.runInFiber inside FiberedRealm's doGetAuthorizationInfo and doGetAuthenticationInfo but just perform the needed logic and add a Fiber.sleep() to actually make the fiber suspension happen.

If you could also update README.md and docs/index.md (Comsat docs; especially useful are information about which Shiro interfaces support fiber suspension as well as some example code, possibly included from tests) that would be really great.

@roded
Copy link
Contributor Author

roded commented Mar 13, 2016

Sure thing.

@circlespainter
Copy link
Member

You should also agree to contribution terms if you haven't already. Thanks!

@roded
Copy link
Contributor Author

roded commented Mar 13, 2016

Yep, already did.
No problem.

@roded
Copy link
Contributor Author

roded commented Mar 13, 2016

I think I'm done.
Let me know of anything that's amiss.

circlespainter added a commit that referenced this pull request Mar 13, 2016
Add Apache Shiro integration
@circlespainter circlespainter merged commit dc16ce3 into puniverse:master Mar 13, 2016
@circlespainter
Copy link
Member

Thanks!

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.

2 participants