Thanks Rishi,
Yes, good idea!
What do you suggest we use for delegatorName? I guess "default"?
Also for OFBIZ-9230 it should be rather
------------------------------------------------------------------------------------------------------------------------------------------------------
Index:
framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtilProperties.java
===================================================================
---
framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtilProperties.java
(revision 1784797)
+++
framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtilProperties.java
(working copy)
@@ -40,6 +40,7 @@
import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.base.util.collections.ResourceBundleMapWrapper;
import org.apache.ofbiz.entity.Delegator;
+import org.apache.ofbiz.entity.DelegatorFactory;
import org.apache.ofbiz.entity.GenericEntityException;
import org.apache.ofbiz.entity.GenericValue;
@@ -62,8 +63,11 @@
}
resource = resource.replace(".properties", "");
if (delegator == null) {
- Debug.logInfo("Could not get a system property for " + name + ".
Reason: the delegator is null", module);
- return results;
+ delegator = DelegatorFactory.getDelegator("default");
+ if (delegator == null) {
+ Debug.logInfo("Could not get a system property for " + name + ".
Reason: the delegator is null", module);
+ return results;
+ }
}
try {
GenericValue systemProperty = EntityQuery.use(delegator)
@@ -82,7 +86,7 @@
results.put("value", "");
return results;
}
- } catch (Exception e) {
+ } catch (GenericEntityException e) {
Debug.logInfo("Could not get a system property for " + name + " :
" + e.getMessage(), module);
}
return results;
------------------------------------------------------------------------------------------------------------------------------------------------------
BTW, though it should not hurt, I'm not sure we need a change in
WidgetWorker.getDelegator() . And if we do, we still need to handle the case
when
delegator = DelegatorFactory.getDelegator("default");
returns null
Jacques
Le 28/02/2017 à 12:49, Rishi Solanki a écrit :
+1 for using the GenericEntityException, we did similar effort in
OFBIZ-7539.
Also agree on Jacques concern that, we should also fix the NPE for
delegator coming from HtmlFormMacroLibrary.ftl. So I think for now the fix
provided in the mentioned ticket is fine OFBIZ-9230 and we should also use
the GenericEntityException. Later we should try to figure out why and when
system lose the delegator which causes the NPE. Once we would fix the
reason we should remove the fix added in the EntityUtilProperties class
method.
New fix proposal:
Change method WidgetWorker.getDelegator with following code;
public static Delegator getDelegator(Map<String, Object> context) {
Delegator delegator = (Delegator) context.get("delegator");
if (delegator == null) {
delegator = DelegatorFactory.getDelegator(delegatorName);
}
return delegator;
}
Please see if this proposal looks fine then we could try and see if it
works to fix the original issue reported in the ticket.
Thanks!
Rishi Solanki
Sr. Manager, Enterprise Software Development
HotWax Systems Pvt. Ltd.
Direct: +91-9893287847
http://www.hotwaxsystems.com
On Tue, Feb 28, 2017 at 1:49 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:
Thanks Wei,
Please All, refer to OFBIZ-9230 to see how I temporarily handled the issue.
I also agree that using GenericEntityException is better than just a
permissive Exception
But I believe we should rather fix the underlying issue I reported there.
So I was hesitant to agree with Wei though it's maybe a better way to get
to really fix the issue because it shows it at the UI level and non only in
logs.
Jacques
Le 28/02/2017 à 03:35, Wei Zhang a écrit :
Hi,
I found there is a try/catch block to catch Exception in
EntityUtilProperties.getSystemPropertyValue(). Which will catche a NPE
(delegator is null) when this method is called in
framework/widget/templates/HtmlFormMacroLibrary.ftl but we only get a
waring
message in the log file.
So I think we should catch GenericEntityException rather than Exception
here
to expose NPE or other runtime exceptions.
Thanks,
Wei
-----
程序羊
--
View this message in context: http://ofbiz.135035.n4.nabble.
com/Should-not-catch-Exception-in-EntityUtilProperties-getSy
stemPropertyValue-tp4702833.html
Sent from the OFBiz - Dev mailing list archive at Nabble.com.
