[
https://issues.apache.org/jira/browse/OFBIZ-9230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15893856#comment-15893856
]
Jacques Le Roux commented on OFBIZ-9230:
----------------------------------------
Let's think about the situation in all its aspects (and be ready to dive in ;)).
First your proposition for WidgetWorker would be actually something like this
{code}
Index: framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java
===================================================================
--- framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java
(revision 1785155)
+++ framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java
(working copy)
@@ -35,6 +35,7 @@
import org.apache.ofbiz.base.util.UtilHttp;
import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.entity.Delegator;
+import org.apache.ofbiz.entity.DelegatorFactory;
import org.apache.ofbiz.service.LocalDispatcher;
import org.apache.ofbiz.webapp.control.ConfigXMLReader;
import org.apache.ofbiz.webapp.control.RequestHandler;
@@ -410,6 +411,16 @@
public static Delegator getDelegator(Map<String, Object> context) {
Delegator delegator = (Delegator) context.get("delegator");
+ if (delegator == null) {
+ // this will be the common case for now as the delegator isn't
available where we want to do this
+ // we'll cheat a little here and assume the default delegator
+ Debug.logError("Could not get a delegator using the 'default'
delegator", module);
+ delegator = DelegatorFactory.getDelegator("default");
+ if (delegator == null) {
+ Debug.logError("Could not get a delegator even trying with the
'default' delegator", module);
+ return null;
+ }
+ }
return delegator;
}
}
{code}
In the convo, you suggested
{code}
delegator = DelegatorFactory.getDelegator(delegatorName);
{code}
I asked you what should be in delegatorName, but you did not answer and that's
the crux of the problem. We will see that later
I suggested to use in EntityUtilProperties.getSystemPropertyValue()
{code}
delegator = DelegatorFactory.getDelegator("default");
{code}
because it fixes the problem at hand while the patch above for getDelegator()
does not (simply try it w/o the getSystemPropertyValue() patch).
So far so good, we saw that we have the same kind of think in checkRhsType(),
which should then actually be patched with
{code}
Index:
framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityExpr.java
===================================================================
---
framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityExpr.java
(revision 1785155)
+++
framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityExpr.java
(working copy)
@@ -18,11 +18,13 @@
*******************************************************************************/
package org.apache.ofbiz.entity.condition;
+import java.io.IOException;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import org.apache.ofbiz.base.util.Debug;
+import org.apache.ofbiz.base.util.GeneralException;
import org.apache.ofbiz.base.util.ObjectType;
import org.apache.ofbiz.base.util.UtilGenerics;
import org.apache.ofbiz.entity.Delegator;
@@ -161,7 +163,7 @@
visitor.acceptEntityExpr(this);
}
- public void checkRhsType(ModelEntity modelEntity, Delegator delegator) {
+ public void checkRhsType(ModelEntity modelEntity, Delegator delegator)
throws IOException, GeneralException {
if (this.rhs == null || this.rhs == GenericEntity.NULL_FIELD ||
modelEntity == null) return;
Object value = this.rhs;
@@ -179,9 +181,13 @@
}
if (delegator == null) {
- // this will be the common case for now as the delegator isn't
available where we want to do this
- // we'll cheat a little here and assume the default delegator
delegator = DelegatorFactory.getDelegator("default");
+ Debug.logError("Could not get a delegator using the 'default'
delegator", module);
+ if (delegator == null) {
+ Debug.logError("Could not get a delegator even trying with the
'default' delegator", module);
+ GeneralException exception = new GeneralException("Could not
get a delegator even trying with the 'default' delegator");
+ throw exception;
+ }
}
String fieldName = null;
{code}
And now I get to the reason for this analysis: *multitenant*! My patch for
getSystemPropertyValue() works in a simple tenant context. But in a
multitenant context it will always refer to the "default" tenant, which can be
a problem. Same for the other patches above including the checkRhsType() one.
BTW the change introduced there is prior (2008) to multitenant insertion
(2010), so is also an issue. Now if you look at OFBIZ-6884, which broke the
current issue, it's motivated by multitenant, and EntityUtilProperties is
mostly useful in a multitenant context, see the problem!
So to summarise:
# My getSystemPropertyValue() patch fixes the issue Wei reported, but is just a
workaround, and is not enough and even wrong in a multitenant context.
# Same for my proposition for checkRhsType(). At first glance it's slightly
better than what we have now because it's throws an error in log in case
'default' is not there. But it's a rare case in a simple tenant context (not a
problem OOTB, works since 2008, but could be a problem with custom changes in
the EntityEngine...). Anyway using default defeat the purpose of multitenant
and is an issue by itself.
# Your proposition for getDelegator() is not related with the problem at hand.
I'm not sure it helps in other contexts. So I'd refrain to commit it since it
inserts a problem in a multitenant context.
So all those solutions are only working in a simple tenant context. I believe
we should refrain to commit any and look for a real solution, ie why the
delegator misses here.
Now there are 2 temporary solutions:
* The one I committed already, which hide the problem in UI and only shows it
in log
* Reverting my commit and replaces the Exception handling by a
GenericEntityException but then the error will show in the UI. Try it, not sure
it's better!
So I suggest we let things as is and close is as unresolved. And create 2 new
Jira issues:
# One for checkRhsType() which is wrong in a multitenant context.
# Another to properly fixes the issue at hand
In both we can refer to the analysis here to explain what we should do. I'm
really yet unsure about what to do for checkRhsType() :/
To put is simply, even if there are easy workarounds in a simple tenant
context, this is a big worry for the multitenant feature which heavilyu relies
on EntityUtilProperties.getSystemPropertyValueI(). To a point that I wonder if
there are no other issues which makes the multitenant feature so fragile that
it either needs to be totally fixed or removed!
> Missing reference to the delegator in
> framework/widget/templates/HtmlFormMacroLibrary.ftl
> -----------------------------------------------------------------------------------------
>
> Key: OFBIZ-9230
> URL: https://issues.apache.org/jira/browse/OFBIZ-9230
> Project: OFBiz
> Issue Type: Bug
> Components: ALL COMPONENTS
> Affects Versions: Trunk
> Reporter: Wei Zhang
> Assignee: Jacques Le Roux
> Priority: Minor
> Attachments: OFBIZ-9230.patch
>
>
> To reproduce
> 1. Load test data <SystemProperty systemResourceId="widget"
> systemPropertyId="widget.autocompleter.defaultMinLength"
> systemPropertyValue="3"/>
> 2. Open https://localhost:8443/partymgr/control/main
> 3. You will still get warning below in log file
> {quote}
> 2017-02-24 12:56:16,348 |http-nio-8443-exec-7 |EntityUtilProperties
> |I| Could not get a system property for widget.autocompleter.defaultMinLength
> : null
> {quote}
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
