Hi Dulanja,

Use the patch attached herewith instead .. (patch_v1.diff)

On Fri, Jan 11, 2013 at 12:34 PM, Muhammed Shariq <[email protected]> wrote:

> On Thu, Jan 10, 2013 at 10:25 AM, Afkham Azeez <[email protected]> wrote:
>
>>
>>
>> On Thu, Jan 10, 2013 at 10:14 AM, Muhammed Shariq <[email protected]>wrote:
>>
>>> On Thu, Jan 10, 2013 at 10:01 AM, Afkham Azeez <[email protected]> wrote:
>>>
>>>> However, in cases where you are sure that the ThreadLocal CC is
>>>> available, you must use getThreadLocalCarbonContext because that method is
>>>> much more efficient than getCarbonContext. What I feel is, we should ensure
>>>> that every thread contains the ThreadLocal CC, which means we will no
>>>> longer need the getCurrentContext method. At that point, from
>>>> getCurrentContext, we will just call getThreadLocalCarbonContext.
>>>
>>>
>>> This is what SInthuja did sometime ago (giving priority to the
>>> ThreadLocal CC), but the caused some issue as Senaka explained in his
>>> detailed mail .. Also I was trying to recall why I did some  changes in
>>> CarbonTomcatValve, and that too was fixing the same issue (username being
>>> null!), I fixed it by setting the username to the CC from the session, this
>>> also was causing a perf issue cz of the getSession() gets called for every
>>> request ..! Guess we need to look into the use of CC api in a bit more
>>> detail ..
>>>
>>
>> You fix was probably the right one. The problem should be unnecessarily
>> accessing the session. Instead, what you could have done was, when someone
>> called getUserName, at that point, get the session, and retrieve the
>> username & set it as an attribute in the CC. In subsequent calls to the the
>> same CC, you could simply return that attribute.
>>
>
> Yup at the moment it getUsername() simply returns what is assigned, if its
> null it doesn't try to fetch it. Also I I guess
> getThreadLocalCarbonContext() and getCarbonContext should have the
> same behavior isn't it?! And we should be able to use the ThreadLocal CC at
> anytime .. (at the moment this is not possible, we had to revert the patch
> that gave priority to thread local CC!) ..
>
> What we are doing in CarbonContext.getCUrrentContext() is give priority to
> MessageContext, since message context have all the information it gets
> populated (AFAIU :)) .. ThreadLocal CC is the last option and I guess it
> doesn't have all the information (cz MessageContext & ConfigurationContext
> happens to be null) .. so in that case we should populate those null fields
> .. hope what I am saying makes sense, thought I had it figured at the time
> but it seems a lil blur again!
>
> However for this particular case I have prepared a patch that tried to
> fetch username from the session within MessageContext .. so again if
> MessageContext is null username is gonna be null. Please have a look and
> check if it is correct ...
>
> @Dulanja - can you apply the attached path to carbon.utils and use the
> ThreadLocal CC and see if your getting the correct username ?!
>
>>
>>
>>>
>>>> Azeez
>>>>
>>>> On Wed, Jan 9, 2013 at 4:19 PM, Dimuthu Leelarathne 
>>>> <[email protected]>wrote:
>>>>
>>>>> Hi Dulanja,
>>>>>
>>>>> It is correct to use CurrentCarbonContext instead of
>>>>> ThreadLocalCarbonContext. You can read the mail titled "UserRegistry sends
>>>>> null to JDBCAuthorizationManager" from archives.  Basically this is the
>>>>> code that finds you the correct  CurrentCarbonContext. So correct thing is
>>>>> to first get CarbonContext from MessageContext and if it is not available
>>>>> try the TreadLocal one.
>>>>>
>>>>> public static CarbonContextDataHolder getCurrentCarbonContextHolder() {
>>>>>         try {
>>>>>             MessageContext messageContext =
>>>>> MessageContext.getCurrentMessageContext();
>>>>>             if (messageContext != null) {
>>>>>                 return getCurrentCarbonContextHolder(messageContext);
>>>>>             } else {
>>>>>                 return
>>>>> getCurrentCarbonContextHolder((ConfigurationContext) null);
>>>>>             }
>>>>>         } catch (NullPointerException ignore) {
>>>>>             // This is thrown when the message context is not
>>>>> initialized
>>>>>             // So return the Threadlocal
>>>>>              return getThreadLocalCarbonContextHolder();
>>>>>         } catch (NoClassDefFoundError ignore) {
>>>>>             // There can be situations where the CarbonContext is
>>>>> accessed, when there is no Axis2
>>>>>             // library on the classpath.
>>>>>             return getThreadLocalCarbonContextHolder();
>>>>>         }
>>>>>     }
>>>>>
>>>>> thanks,
>>>>> dimuthu
>>>>>
>>>>>
>>>>> On Wed, Jan 9, 2013 at 2:15 PM, Dulanja Liyanage <[email protected]>wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Appreciate if you can guide me whether it's safe/appropriate to use
>>>>>> the CurrentContext rather than the ThreadLocalCarbonContext, to get the
>>>>>> username and tenant domain.
>>>>>>
>>>>>> I'm not much knowledgeable about the design, but I feel using it will
>>>>>> be "plastering" the root cause.
>>>>>>
>>>>>> Thanks,
>>>>>> Dulanja
>>>>>>
>>>>>> On Wed, Jan 9, 2013 at 12:18 PM, Dulanja Liyanage 
>>>>>> <[email protected]>wrote:
>>>>>>
>>>>>>> Hi Shariq,
>>>>>>>
>>>>>>> I did as you suggested and now the username is available.
>>>>>>>
>>>>>>> What I'm not sure is how appropriate/safe this is.
>>>>>>>
>>>>>>> Thanks for the help.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Dulanja
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Jan 8, 2013 at 6:17 PM, Dulanja Liyanage 
>>>>>>> <[email protected]>wrote:
>>>>>>>
>>>>>>>> Hi Shariq,
>>>>>>>>
>>>>>>>> I will try that. Thanks!
>>>>>>>>
>>>>>>>> Dulanja
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Jan 8, 2013 at 6:12 PM, Muhammed Shariq <[email protected]>wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> This looks like an issue I encountered sometime back. We had to
>>>>>>>>> revert the fixes I did in TomcatValve cz it was causing some other 
>>>>>>>>> issue ..
>>>>>>>>> Can you try CarbonContext.getCurrentContext.getUsername() and see what
>>>>>>>>> happens .. AFAIR its getUsername simple does a return .. we should 
>>>>>>>>> fix it
>>>>>>>>> to check if username is null and if so set it properly .. usually 
>>>>>>>>> that is
>>>>>>>>> the behavior but there are some  edge cases it seem ...
>>>>>>>>>
>>>>>>>>> On Tue, Jan 8, 2013 at 3:53 PM, Dulanja Liyanage <[email protected]
>>>>>>>>> > wrote:
>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> I'm testing IS 4.1.0 running on Carbon 4.0.6. While debugging an
>>>>>>>>>> issue, I encountered the $subject.
>>>>>>>>>>
>>>>>>>>>> I logged in as 'admin', so, the username should return as such.
>>>>>>>>>> But it returns 'null'.
>>>>>>>>>>
>>>>>>>>>> However the
>>>>>>>>>> PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain()
>>>>>>>>>> returns 'carbon.super' as expected.
>>>>>>>>>>
>>>>>>>>>> I'm still debugging this and trying to figure out how
>>>>>>>>>> CarbonContext works in threads. If someone is already aware of a
>>>>>>>>>> reason/solution for this it will save time. :)
>>>>>>>>>>
>>>>>>>>>> Thanks!
>>>>>>>>>> Dulanja
>>>>>>>>>>
>>>>>>>>>> _______________________________________________
>>>>>>>>>> Dev mailing list
>>>>>>>>>> [email protected]
>>>>>>>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Thanks,
>>>>>>>>> Shariq.
>>>>>>>>> Phone: +94 777 202 225
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Dev mailing list
>>>>>> [email protected]
>>>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Dev mailing list
>>>>> [email protected]
>>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> *Afkham Azeez*
>>>> Director of Architecture; WSO2, Inc.; http://wso2.com
>>>> Member; Apache Software Foundation; http://www.apache.org/
>>>> * <http://www.apache.org/>**
>>>> email: **[email protected]* <[email protected]>* cell: +94 77 3320919
>>>> blog: **http://blog.afkham.org* <http://blog.afkham.org>*
>>>> twitter: 
>>>> **http://twitter.com/afkham_azeez*<http://twitter.com/afkham_azeez>
>>>> *
>>>> linked-in: **http://lk.linkedin.com/in/afkhamazeez*
>>>> *
>>>> *
>>>> *Lean . Enterprise . Middleware*
>>>>
>>>> _______________________________________________
>>>> Dev mailing list
>>>> [email protected]
>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>>>
>>>>
>>>
>>>
>>> --
>>> Thanks,
>>> Shariq.
>>> Phone: +94 777 202 225
>>>
>>
>>
>>
>> --
>> *Afkham Azeez*
>> Director of Architecture; WSO2, Inc.; http://wso2.com
>> Member; Apache Software Foundation; http://www.apache.org/
>> * <http://www.apache.org/>**
>> email: **[email protected]* <[email protected]>* cell: +94 77 3320919
>> blog: **http://blog.afkham.org* <http://blog.afkham.org>*
>> twitter: **http://twitter.com/afkham_azeez*<http://twitter.com/afkham_azeez>
>> *
>> linked-in: **http://lk.linkedin.com/in/afkhamazeez*
>> *
>> *
>> *Lean . Enterprise . Middleware*
>>
>
>
>
> --
> Thanks,
> Shariq.
> Phone: +94 777 202 225
>



-- 
Thanks,
Shariq.
Phone: +94 777 202 225

Attachment: patch_v1.diff
Description: Binary data

_______________________________________________
Dev mailing list
[email protected]
http://wso2.org/cgi-bin/mailman/listinfo/dev

Reply via email to