fixes issue 40016: Concurrency problems in MyOpenbravoActionHandler
authorCarlos Aristu <carlos.aristu@openbravo.com>
Thu, 24 Jan 2019 11:10:47 +0100
changeset 35323 4ab5cd94e65e
parent 35322 699f8e73b89f
child 35324 e3d66bfad0a5
fixes issue 40016: Concurrency problems in MyOpenbravoActionHandler

As part of the changes done to decrease the time spent to load the widgets after login, the WidgetProviders are cached[1]. This kind of classes had a private field named widgetClass (a DAL object) which could cause errors when accessing to some of its properties in a concurrent way.

To fix this issue we are now keeping in cache the properties of the WidgetClass instead of keeping the reference to the DAL object.

[1] https://code.openbravo.com/erp/devel/pi/rev/2a1a09ee6e6852cb977efa1a987e27d2811e9127#l1.178
modules/org.openbravo.client.myob/src/org/openbravo/client/myob/WidgetProvider.java
--- a/modules/org.openbravo.client.myob/src/org/openbravo/client/myob/WidgetProvider.java	Thu Jan 24 10:54:34 2019 +0100
+++ b/modules/org.openbravo.client.myob/src/org/openbravo/client/myob/WidgetProvider.java	Thu Jan 24 11:10:47 2019 +0100
@@ -11,7 +11,7 @@
  * under the License.
  * The Original Code is Openbravo ERP.
  * The Initial Developer of the Original Code is Openbravo SLU
- * All portions are Copyright (C) 2010-2018 Openbravo SLU
+ * All portions are Copyright (C) 2010-2019 Openbravo SLU
  * All Rights Reserved.
  * Contributor(s):  ______________________________________.
  ************************************************************************
@@ -27,8 +27,6 @@
 import java.util.Map;
 import java.util.Set;
 
-import javax.inject.Inject;
-
 import org.apache.log4j.Logger;
 import org.codehaus.jettison.json.JSONException;
 import org.codehaus.jettison.json.JSONObject;
@@ -109,30 +107,30 @@
 
   private Map<String, Object> parameters = new HashMap<String, Object>();
 
-  // note is only set if the widgetprovider is created
-  // through the MyOBUtils class.
-  private WidgetClass widgetClass;
-
-  @Inject
-  private MyOBUtils myOBUtils;
+  // note this is only set if the widgetprovider is created through the MyOBUtils class
+  private JSONObject widgetClassDefinition;
 
   // prevent anyone else from creating a widgetprovider directly
   protected WidgetProvider() {
   }
 
   public JSONObject getWidgetClassDefinition() {
+    return widgetClassDefinition;
+  }
+
+  private void setWidgetClassDefinition(WidgetClass widgetClass) {
     try {
-      final JSONObject jsonObject = new JSONObject();
-      jsonObject.put(MyOpenbravoWidgetComponent.CLASSNAMEPARAMETER,
-          this.getClientSideWidgetClassName());
-      jsonObject.put(WIDGETCLASSID, widgetClass.getId());
-      jsonObject.put(TITLE, MyOBUtils.getWidgetTitle(widgetClass));
-      jsonObject.put(HEIGHT, widgetClass.getHeight() + WIDGET_HEADER_HEIGHT);
-      jsonObject.put(MENU_ITEMS, MyOBUtils.getWidgetMenuItems(widgetClass));
+      widgetClassDefinition = new JSONObject();
+      widgetClassDefinition.put(WIDGETCLASSID, widgetClass.getId());
+      widgetClassDefinition.put(MyOpenbravoWidgetComponent.CLASSNAMEPARAMETER,
+          getClientSideWidgetClassName());
+      widgetClassDefinition.put(TITLE, MyOBUtils.getWidgetTitle(widgetClass));
+      widgetClassDefinition.put(HEIGHT, widgetClass.getHeight() + WIDGET_HEADER_HEIGHT);
+      widgetClassDefinition.put(MENU_ITEMS, MyOBUtils.getWidgetMenuItems(widgetClass));
       if (widgetClass.getWidgetSuperclass() != null) {
-        jsonObject.put(CAN_MAXIMIZE, widgetClass.getWidgetSuperclass().isCanMaximize());
+        widgetClassDefinition.put(CAN_MAXIMIZE, widgetClass.getWidgetSuperclass().isCanMaximize());
       } else {
-        jsonObject.put(CAN_MAXIMIZE, widgetClass.isCanMaximize());
+        widgetClassDefinition.put(CAN_MAXIMIZE, widgetClass.isCanMaximize());
       }
 
       final JSONObject aboutFieldDefinitions = new JSONObject();
@@ -230,10 +228,9 @@
         fieldDefinition.put(PARAMETERTITLE, getParameterLabel(parameter));
         fieldDefinitions.add(fieldDefinition);
       }
-      jsonObject.put(PARAMETERS, defaultParameters);
-      jsonObject.put(FIELDDEFINITIONS, fieldDefinitions);
-      jsonObject.put(ABOUTFIELDDEFINITIONS, aboutFieldDefinitions);
-      return jsonObject;
+      widgetClassDefinition.put(PARAMETERS, defaultParameters);
+      widgetClassDefinition.put(FIELDDEFINITIONS, fieldDefinitions);
+      widgetClassDefinition.put(ABOUTFIELDDEFINITIONS, aboutFieldDefinitions);
     } catch (Exception e) {
       throw new OBException(e);
     }
@@ -242,13 +239,12 @@
   protected void addDefaultWidgetProperties(JSONObject jsonObject, WidgetInstance widgetInstance)
       throws JSONException {
     jsonObject.put(WIDGETCLASSID, widgetInstance.getWidgetClass().getId());
-    jsonObject.put(MyOpenbravoWidgetComponent.CLASSNAMEPARAMETER,
-        myOBUtils.getWidgetProvider(widgetClass).getClientSideWidgetClassName());
+    jsonObject.put(MyOpenbravoWidgetComponent.CLASSNAMEPARAMETER, getClientSideWidgetClassName());
     jsonObject.put(DBINSTANCEID, widgetInstance.getId());
     jsonObject.put(TITLE, MyOBUtils.getWidgetTitle(widgetInstance));
     jsonObject.put(COLNUM, widgetInstance.getColumnPosition());
     jsonObject.put(ROWNUM, widgetInstance.getSequenceInColumn());
-    jsonObject.put(HEIGHT, widgetClass.getHeight() + WIDGET_HEADER_HEIGHT);
+    jsonObject.put(HEIGHT, widgetClassDefinition.get(HEIGHT));
     jsonObject.put(PRIORITY, widgetInstance.getRelativePriority());
 
     final JSONObject widgetParameters = new JSONObject();
@@ -271,7 +267,7 @@
   }
 
   public String getClientSideWidgetClassName() {
-    return KernelConstants.ID_PREFIX + getWidgetClass().getId();
+    return KernelConstants.ID_PREFIX + getWidgetClassId();
   }
 
   /**
@@ -279,7 +275,7 @@
    * 
    */
   public String generate() {
-    return "isc.defineClass('" + KernelConstants.ID_PREFIX + getWidgetClass().getId()
+    return "isc.defineClass('" + KernelConstants.ID_PREFIX + getWidgetClassId()
         + "', isc.OBShowParameterWidget);";
   }
 
@@ -444,11 +440,21 @@
   }
 
   public WidgetClass getWidgetClass() {
-    return widgetClass;
+    return OBDal.getInstance().get(WidgetClass.class, getWidgetClassId());
+  }
+
+  private String getWidgetClassId() {
+    String widgetClassId = null;
+    try {
+      widgetClassId = widgetClassDefinition.getString(WIDGETCLASSID);
+    } catch (JSONException ignore) {
+      // should not happen if the WidgetProvider instance is initialized properly
+    }
+    return widgetClassId;
   }
 
   public void setWidgetClass(WidgetClass widgetClass) {
-    this.widgetClass = widgetClass;
+    setWidgetClassDefinition(widgetClass);
   }
 
   private boolean getBooleanValueFromString(String value) {