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 <[email protected]> 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 >
