On 10/31/2014 08:28 AM, Emmanuel Lécharny wrote: > looking at the way we use the managers, we do create a new instance of > each one of them everytime we need to execute some operation.
A new instance is created the first time needed and may be cached afterwards as it is merely a POJO. The createInstance must be able to.... 1. accept a reference to session containing delegated admin context. This is used for ARBAC02 style permission checks to occur within RBAC managers. 2. accept a tenant id. This scopes the subsequent operations to a particular subtree within the DIT - i.e. multitenancy If either of the above two instance varables are set on the manager, the instance will not be thread safe. If either of the two above change in between operations, a new manager must be created to correspond with the new context. On 10/31/2014 08:28 AM, Emmanuel Lécharny wrote:> There are five problems in this approach : > > 1) Once we have determinated the type of connector, we can't switch to > any other. This is due to the first test : > > if ( Strings.isEmpty( dAdminClassName ) ) > > As soon as we have defined the class name, it's for ever, unless we set > it to null, which is not really an option. True On 10/31/2014 08:28 AM, Emmanuel Lécharny wrote:> 2) It's static : we base the selection of a class by a gloabbly defined > value, IS_REST. There is no way for the user to switch this to something > different : Also true. Not sure of a use case where this creates a problem. On 10/31/2014 08:28 AM, Emmanuel Lécharny wrote:> 4) It's costly, as we have to call a Class.forName( className > ).newInstance() every time we want an instance. > Again true On 10/31/2014 08:28 AM, Emmanuel Lécharny wrote:> > 5) It's not context free, as we inject a contextId into the instance. Yes there are two possible injections, refer to my earlier comments. On 10/31/2014 08:28 AM, Emmanuel Lécharny wrote:> Here, a container approach would be way better. A lightweight container > could do the Wtrick. You require an instance of a class based on a config > parameter which is not static, and you get back an instance. The > container is responsible for the instanciation. Here, the container > would take two parameters : the type of manager, and the type of > connector. Something like : > > DelAdminMgr delAdminMgr = container.createInstance( "DelAdminMgr", > context.getConnectionType() ); Would work On 10/31/2014 08:28 AM, Emmanuel Lécharny wrote:> Dealing with the second aspect is a bit more complex, as it requires we > check all the code to be sure that the instance is stateless. Although > it's important to think about doing this check, and eventually move to a > stateless instance, because it will require way less instances to be > created, and we won't have any more to care about the potential memory > cost of it (even if we will have to pass a context to each instance). > > In any case, if the instance is stateless, we can stop thinking about > creating a new instance for each call, we just have to return the first > instance we created. And even if they are not stateless, assuming that > the contextID is the only parameter in used, then we could manage a > cache of instance for each specific context. Are you saying that the manager impl should be stateless? What about multitenancy and ARBAC req's? The ARBAC context is for a particular user so I don't think a cache will work here.
