You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shiro.apache.org by Les Hazlewood <lh...@apache.org> on 2009/11/12 17:39:30 UTC
Re: Circular dependency between SubjectFactory and
DefaultSecurityManager
Hi Kalle,
You're right, there is a circular dependency and it is mostly a
remnant of the old implementation. Since the default SubjectFactory
implementation needed a a SecurityManager instance to build its
instances, it was made available as an injected property.
It has always bothered me that you couldn't cleanly inject a
SubjectFactory into a security manager because of this circular
reference - a 'chicken and the egg' kind of thing.
But now that you reminded me of it, I think adding the SecurityManager
reference to the context Map right before the
SubjectFactory.createInstance method is called is a great idea - no
more configuration circular dependency and the underlying
implementation can do whatever it wants - either use the one in the
map or use its own if necessary.
I personally prefer this to adding a method argument to the
SubjectFactory interface because I personally view the need of the
SecurityManager instance is really an implementation type of detail -
not something that should change the interface contract IMO.
My other desire to add it to the context map is based on some other
refactoring I haven't yet completed:
There is an interface, org.apache.shiro.util.Factory<T>. I want to
change this a bit (change createInstance to getInstance and let the
implementation decide if a new object or cached object is returned).
I was also thinking of adding a new interface:
org.apache.shiro.util.ContextualFactory<T,U> where T is the factory
output type and U is the factory method input type. The
SubjectFactory interface could then just be a sub-interface of the
latter.
Thoughts?
- Les
On Wed, Nov 11, 2009 at 5:56 PM, Kalle Korhonen
<ka...@gmail.com> wrote:
> I was just looking at implementing a custom SubjectFactory and noticed
> that there's a circular dependency between SubjectFactory and
> DefaultSecurityManager. I assume that SubjectFactory requires
> SecurityManager just so that it can pass it to the created Subject, is
> that correct? If there's no other reason for it, wouldn't it be
> cleaner if a securityManager would be passed in as a second argument
> of createSubject(Map context)? Or even in the context itself, but I
> suppose a reference to SecurityManager is required for creating a
> Subject.
>
> Kalle
>
Re: Circular dependency between SubjectFactory and
DefaultSecurityManager
Posted by Kalle Korhonen <ka...@gmail.com>.
Yeap, works great, thanks!
Kalle
On Sat, Nov 14, 2009 at 3:30 PM, Les Hazlewood <lh...@apache.org> wrote:
> Hi Kalle,
>
> Give it a shot now.
>
> - Les
>
> On Sat, Nov 14, 2009 at 6:07 PM, Les Hazlewood <lh...@apache.org> wrote:
>> Sure, I'll look in to it.
>>
>> On Sat, Nov 14, 2009 at 5:02 PM, Kalle Korhonen
>> <ka...@gmail.com> wrote:
>>> Created https://issues.apache.org/jira/browse/SHIRO-114, but just
>>> noticed that Jira says "You don't have permission to work on this
>>> issue" and I cannot assign it to myself. Some permission issue I
>>> suppose, Les can you fix that (since you are Jira project lead you
>>> likely have admin permissions to the project)?
>>>
>>> Kalle
>>>
>>>
>>> On Sat, Nov 14, 2009 at 1:34 PM, Les Hazlewood <lh...@apache.org> wrote:
>>>> On Sat, Nov 14, 2009 at 3:13 PM, Kalle Korhonen
>>>> <ka...@gmail.com> wrote:
>>>>> First of all, I'm always impressed and appreciative you always taking
>>>>> time to write such comprehensive and well-thought out replies, so
>>>>> thanks for that Les.
>>>>
>>>> My pleasure - I like to be as descriptive and helpful as I am able
>>>> (time willing)
>>>>
>>>>> I'll open an issue
>>>>> for making SubjectFactory part of the context and I can even take care
>>>>> of the refactoring, shouldn't be too difficult.
>>>>
>>>> Sounds good!
>>>>
>>>> Thanks,
>>>>
>>>> Les
>>>>
>>>
>>
>
Re: Circular dependency between SubjectFactory and
DefaultSecurityManager
Posted by Les Hazlewood <lh...@apache.org>.
Hi Kalle,
Give it a shot now.
- Les
On Sat, Nov 14, 2009 at 6:07 PM, Les Hazlewood <lh...@apache.org> wrote:
> Sure, I'll look in to it.
>
> On Sat, Nov 14, 2009 at 5:02 PM, Kalle Korhonen
> <ka...@gmail.com> wrote:
>> Created https://issues.apache.org/jira/browse/SHIRO-114, but just
>> noticed that Jira says "You don't have permission to work on this
>> issue" and I cannot assign it to myself. Some permission issue I
>> suppose, Les can you fix that (since you are Jira project lead you
>> likely have admin permissions to the project)?
>>
>> Kalle
>>
>>
>> On Sat, Nov 14, 2009 at 1:34 PM, Les Hazlewood <lh...@apache.org> wrote:
>>> On Sat, Nov 14, 2009 at 3:13 PM, Kalle Korhonen
>>> <ka...@gmail.com> wrote:
>>>> First of all, I'm always impressed and appreciative you always taking
>>>> time to write such comprehensive and well-thought out replies, so
>>>> thanks for that Les.
>>>
>>> My pleasure - I like to be as descriptive and helpful as I am able
>>> (time willing)
>>>
>>>> I'll open an issue
>>>> for making SubjectFactory part of the context and I can even take care
>>>> of the refactoring, shouldn't be too difficult.
>>>
>>> Sounds good!
>>>
>>> Thanks,
>>>
>>> Les
>>>
>>
>
Re: Circular dependency between SubjectFactory and
DefaultSecurityManager
Posted by Les Hazlewood <lh...@apache.org>.
Sure, I'll look in to it.
On Sat, Nov 14, 2009 at 5:02 PM, Kalle Korhonen
<ka...@gmail.com> wrote:
> Created https://issues.apache.org/jira/browse/SHIRO-114, but just
> noticed that Jira says "You don't have permission to work on this
> issue" and I cannot assign it to myself. Some permission issue I
> suppose, Les can you fix that (since you are Jira project lead you
> likely have admin permissions to the project)?
>
> Kalle
>
>
> On Sat, Nov 14, 2009 at 1:34 PM, Les Hazlewood <lh...@apache.org> wrote:
>> On Sat, Nov 14, 2009 at 3:13 PM, Kalle Korhonen
>> <ka...@gmail.com> wrote:
>>> First of all, I'm always impressed and appreciative you always taking
>>> time to write such comprehensive and well-thought out replies, so
>>> thanks for that Les.
>>
>> My pleasure - I like to be as descriptive and helpful as I am able
>> (time willing)
>>
>>> I'll open an issue
>>> for making SubjectFactory part of the context and I can even take care
>>> of the refactoring, shouldn't be too difficult.
>>
>> Sounds good!
>>
>> Thanks,
>>
>> Les
>>
>
Re: Circular dependency between SubjectFactory and
DefaultSecurityManager
Posted by Kalle Korhonen <ka...@gmail.com>.
Created https://issues.apache.org/jira/browse/SHIRO-114, but just
noticed that Jira says "You don't have permission to work on this
issue" and I cannot assign it to myself. Some permission issue I
suppose, Les can you fix that (since you are Jira project lead you
likely have admin permissions to the project)?
Kalle
On Sat, Nov 14, 2009 at 1:34 PM, Les Hazlewood <lh...@apache.org> wrote:
> On Sat, Nov 14, 2009 at 3:13 PM, Kalle Korhonen
> <ka...@gmail.com> wrote:
>> First of all, I'm always impressed and appreciative you always taking
>> time to write such comprehensive and well-thought out replies, so
>> thanks for that Les.
>
> My pleasure - I like to be as descriptive and helpful as I am able
> (time willing)
>
>> I'll open an issue
>> for making SubjectFactory part of the context and I can even take care
>> of the refactoring, shouldn't be too difficult.
>
> Sounds good!
>
> Thanks,
>
> Les
>
Re: Circular dependency between SubjectFactory and
DefaultSecurityManager
Posted by Les Hazlewood <lh...@apache.org>.
On Sat, Nov 14, 2009 at 3:13 PM, Kalle Korhonen
<ka...@gmail.com> wrote:
> First of all, I'm always impressed and appreciative you always taking
> time to write such comprehensive and well-thought out replies, so
> thanks for that Les.
My pleasure - I like to be as descriptive and helpful as I am able
(time willing)
> I'll open an issue
> for making SubjectFactory part of the context and I can even take care
> of the refactoring, shouldn't be too difficult.
Sounds good!
Thanks,
Les
Re: Circular dependency between SubjectFactory and
DefaultSecurityManager
Posted by Kalle Korhonen <ka...@gmail.com>.
On Thu, Nov 12, 2009 at 8:39 AM, Les Hazlewood <lh...@apache.org> wrote:
> You're right, there is a circular dependency and it is mostly a
> remnant of the old implementation. Since the default SubjectFactory
> implementation needed a a SecurityManager instance to build its
> instances, it was made available as an injected property.
First of all, I'm always impressed and appreciative you always taking
time to write such comprehensive and well-thought out replies, so
thanks for that Les.
> But now that you reminded me of it, I think adding the SecurityManager
> reference to the context Map right before the
> SubjectFactory.createInstance method is called is a great idea - no
> more configuration circular dependency and the underlying
> implementation can do whatever it wants - either use the one in the
> map or use its own if necessary.
I think that's the key - whether securityManager is optional or not.
If it's not, it might just as well be an additional parameter but if
there's even a possibility that some implementation of a
SubjectFactory would not need it or would want to instead provide its
own, then it's probably better to pass it in as part of the context
map.
> I personally prefer this to adding a method argument to the
> SubjectFactory interface because I personally view the need of the
> SecurityManager instance is really an implementation type of detail -
> not something that should change the interface contract IMO.
Ok, so it sounds like you see use for allowing specific
implementations of SubjectFactory to handle SecurityManager as they
best see fit - I trust your judgement with that. I'll open an issue
for making SubjectFactory part of the context and I can even take care
of the refactoring, shouldn't be too difficult.
> There is an interface, org.apache.shiro.util.Factory<T>. I want to
> change this a bit (change createInstance to getInstance and let the
> implementation decide if a new object or cached object is returned).
> I was also thinking of adding a new interface:
> org.apache.shiro.util.ContextualFactory<T,U> where T is the factory
> output type and U is the factory method input type. The
> SubjectFactory interface could then just be a sub-interface of the
> latter.
Sounds good.
Kalle