ofbiz-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jacques Le Roux <jacques.le.r...@les7arts.com>
Subject Re: Should not catch Exception in EntityUtilProperties.getSystemPropertyValue()
Date Thu, 02 Mar 2017 09:40:59 GMT


Le 02/03/2017 à 08:00, Rishi Solanki a écrit :
> Thank you Wei, Jacques for your reply.
>
> What I was proposing to change method of WidgetWorker.getDelegator()
> method. I see whenever any form render the fields and requires delegator
> then it always tries to get delegator from WidgetWorker's mentioned method.
> Please refer MacroFormRenederer class as mentioned in the OFBIZ-9230 by
> Jacques.
>
> If we fix that method, then I think we should be fine and while rendering
> system will always have the delegator and this issue should not appear. In
> that case we won't required extra delegaor checks in the
> EntityUtilProperties class for this particular issue.
Actually no. EntityUtilProperties.getSystemPropertyValueI() I change in my patch is only used
by EntityUtilProperties methods (getMessage, 2 
getPropertyValue, propertyValueEqualsIgnoreCase)
But those are widely used in OFBiz, not only in the context of widget
> In case we requied this check from other place then we need to make sure delegator always
> passed to EntityUtilProperties methods, instead of adding condition in the
> EntityUtilProperties methods. Because many methods in that class uses the
> delegator as parameter.
The EntityUtilProperties methods which needs a delegator are actually those above because
they call getSystemPropertyValue() which needs the 
delegator, not for other reasons.
> Finally we should catch the GenericEntityException instead of Exception. In
> a way #1 and #3 needs to be work on and for #2 we should
>   let it be as is so that in future system will report if somewhere we are
> losing the delegator then we could take care. That means what Jacques did
> in his last fix should be fine, simply log delegator is missing on console.

So you propose not to use the "default" delegator but simply log the error. I agree this is
better.
But we will face issues in the UI like the one reported by 程序羊 in log (try replacing
Exception by GenericEntityException and then get to  
https://localhost:8443/partymgr/control/main)
And I don't see a simple and quick fix for the missing delegator :/
> This is what I'm proposing, but I'm okay if we want to fix delegator issue
> in the EntityUtilProperties class, if so then I would say to fix it for all
> methods which uses delegator as parameter.

If you mean EntityUtilProperties  methods then my workaround is "OK".
Actually there are 1 other places where we use
delegator = DelegatorFactory.getDelegator("default");
checkRhsType()
(also one in tests with delegatorCreationUsingFactoryGetDelegator() that we can neglect)

I attach in the Jira the patch I finally propose and we should continue to discuss there IMO

Thanks

Jacques


>
> Thanks!
>
>
>
>
>
> Rishi Solanki
> Sr. Manager, Enterprise Software Development
> HotWax Systems Pvt. Ltd.
> Direct: +91-9893287847
> http://www.hotwaxsystems.com
>
> On Thu, Mar 2, 2017 at 8:02 AM, Wei Zhang <zhangwei1979@outlook.com> wrote:
>
>> I think we need to do 3 things.
>>
>> 1. Catch GenericEntityException in
>> EntityUtilProperties.getSystemPropertyValue()
>> 2. Create a new Delegator if it is null in
>> EntityUtilProperties.getSystemPropertyValue()
>> 3. Find out how to get a delegator instance in
>> framework/widget/templates/HtmlFormMacroLibrary.ftl
>>
>>
>>
>>
>> -----
>> 程序羊
>> --
>> View this message in context: http://ofbiz.135035.n4.nabble.
>> com/Should-not-catch-Exception-in-EntityUtilProperties-
>> getSystemPropertyValue-tp4702833p4702909.html
>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>>
>


Mime
View raw message