Stateless (Flyweight) and current user

Adam Young ayoung at redhat.com
Thu Apr 1 15:35:40 UTC 2010


On 04/01/2010 11:20 AM, Dmitri Dolguikh wrote:
> On 04/01/2010 12:16 PM, Justin Harris wrote:
>    
>> ----- "Bryan Kearney"<bkearney at redhat.com>   wrote:
>>
>>
>>      
>>> On 04/01/2010 10:55 AM, Justin Harris wrote:
>>>
>>>        
>>>> ----- "Bryan Kearney"<bkearney at redhat.com>    wrote:
>>>>
>>>>
>>>>          
>>>>> On 04/01/2010 10:40 AM, Adam Young wrote:
>>>>>
>>>>>            
>>>>>> The current implementation of candlepin is complete oblivious to
>>>>>>
>>>>>>              
>>>>> the
>>>>>
>>>>>            
>>>>>> caller.  We can authenticate, but once we do, we do not tailor
>>>>>>
>>>>>>              
>>> the
>>>
>>>        
>>>>>> output of the calls based on who logged in.
>>>>>>
>>>>>> If we are to implement the multi-org functionality, we need to
>>>>>>
>>>>>>              
>>>>> rpovide a
>>>>>
>>>>>            
>>>>>> mechanism for checking "who asked this."
>>>>>>
>>>>>>
>>>>>> For example, on  ConsumerResource.list is implemented as:
>>>>>>
>>>>>>       public List<Consumer>     list() {
>>>>>>              return consumerCurator.findAll();
>>>>>>          }
>>>>>>
>>>>>>
>>>>>> To filter this via user, we have three choices.  I'm going to use
>>>>>> rincipal for the user, as we don't havea first calss user object
>>>>>>
>>>>>>              
>>>>> yet,
>>>>>
>>>>>            
>>>>>> but we can change that later.
>>>>>>
>>>>>> 1.  Change the method signature
>>>>>>
>>>>>>
>>>>>>       public List<Consumer>     list(Principal p) {
>>>>>>              return consumerCurator.findByOwner(getOwner(p));
>>>>>>          }
>>>>>>
>>>>>>
>>>>>> 2.  Add it to the ConsumerResource constructor via dependency
>>>>>>
>>>>>>              
>>>>> injection.
>>>>>
>>>>>            
>>>>>> (Note that I wouldn't really implement getOwner this way, just to
>>>>>> demostrate)
>>>>>>
>>>>>>
>>>>>>       public List<Consumer>     list() {
>>>>>>              return
>>>>>> consumerCurator.findByOwner(this.getOwner(this.getPrinciapl()));
>>>>>>          }
>>>>>>
>>>>>> 3.  Fetch it from a context like JNDI
>>>>>>
>>>>>>       public List<Consumer>     list() {
>>>>>>              Principal p = (Principal)new
>>>>>> InitialContext().resolve(Principal.class.getName());
>>>>>>              return consumerCurator.findByOwner(this.getOwner(p));
>>>>>>          }
>>>>>>
>>>>>>
>>>>>>
>>>>>> Option 1 is best for cases where this is going to be only
>>>>>>
>>>>>>              
>>> required
>>>
>>>        
>>>>> for
>>>>>
>>>>>            
>>>>>> one or two methods.  THe Principal will be needed for just about
>>>>>>
>>>>>>              
>>>>> every
>>>>>
>>>>>            
>>>>>> method.
>>>>>>
>>>>>> Option 2 means that the Resource Objects will no longer be
>>>>>>
>>>>>>              
>>>>> stateless,
>>>>>
>>>>>            
>>>>>> but instead will have to be created either on each request, or
>>>>>>
>>>>>>              
>>> once
>>>
>>>        
>>>>> per
>>>>>
>>>>>            
>>>>>> session
>>>>>>
>>>>>> Option 3 menas we have non-explicit dependencies.
>>>>>>
>>>>>>
>>>>>> Do we need a meeting for this?
>>>>>>
>>>>>>              
>>>>> I would say no, and I would be +1 for the Option 3. I would suggest
>>>>>
>>>>>            
>>> we
>>>
>>>        
>>>>> use a Context object which could hide me from JNDI, Guce, or name
>>>>>
>>>>>            
>>> your
>>>
>>>        
>>>>> magic thread local pattern..
>>>>>
>>>>>            
>>>> I think that this is a losing argument for me, but I am going to
>>>>
>>>>          
>>> echo that I think having a resource look different depending on who is
>>> looking at it violates RESTiness and also sets us up for a tough road
>>> wrt maintenance.  I really would like to explore organizing our URLs
>>> to specify the collections more accurately, and using the user/roles
>>> to either allow or deny access - and that is all.
>>>
>>>        
>>>>     - Justin
>>>>
>>>>
>>>>          
>>> This is a red herring a bit in the discussion. Think of it how do you
>>>
>>> get data from the resource into the curator.
>>>
>>> FWIW.. I dont think it does violate restfullness. I am seeing eaxmples
>>>
>>> of people using media to coerce what comes back. User and security is
>>>
>>> not much different.
>>>
>>>        
>> Yes I see your point now.  Sorry about that. :)   Has anyone looked into if Guice can do a lot of this for us?  I'm not sure about the component lifecycles for Guice, but I know that for example Seam will do dependency injection per request, so that we can have our auth filters set up the current Owner, Roles, etc. and just pass those in as parameters to the resource (or curators).  Relying on the DI framework can also simplify unit testing because you just pass in what you want and don't have to worry about mocking out JNDI contexts which would make me want to punch myself in the face.
>>
>>      
> Yup. we can have http-request-scoped objects. And a custom 'provider' to
> inject those.
> -d
>    



Yep.  The reason to not do that is to limit the amount of DI overhead 
per request call.    We aren't doing that thus far, but we might want to 
do so in the future.


Of course, then you have the question about making all object references 
into instance variables.

What my gut is telling me is that stuff that survives over multple 
requests should be member variables of the Resources, and stuff that is 
per-request should be parameters to functions.  I'd hold off on 
createing a general purpose "context" until we are certain we need it, 
and instead add the Principal or User object to the method calls that 
require it.

I missed the JAAS discussion, as I think it happened prior to me joining 
the team.  Why did we remove JAAS, and why do we think we might want it 
back now?

>>
>>      
>>> -- bk
>>>
>>>        
>> _______________________________________________
>> candlepin mailing list
>> candlepin at lists.fedorahosted.org
>> https://fedorahosted.org/mailman/listinfo/candlepin
>>
>>      
> _______________________________________________
> candlepin mailing list
> candlepin at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/candlepin
>    




More information about the candlepin mailing list