fixed issue 30308: use get to reload grid object on save/update
authorAsier Lostalé <asier.lostale@openbravo.com>
Tue, 07 Jul 2015 10:45:53 +0200
changeset 27089 9bedf4b77a0a
parent 27088 8adae72ecaee
child 27090 0390d75ed291
fixed issue 30308: use get to reload grid object on save/update

Backed out changesets: 2268a549839c2268a549839c & ec134f647522ec134f647522
because New OBDal.refresh(BaseOBObject, boolean) had some problems:

* Unlike OBDal.refresh(Object) method, it does not refresh current Object
instance but creates a new one which is returned
* Parameter object instance can be evicted from DAL's cache, so reusing it
after calling this method can result in a LazyInitializationException
* If parameter object is already in DAL's cache, refreshing with useCache=false
has no effect
* If parameter object is not yet initialized bob.getId() might result in a
LazyInitializationException

After those backouts code remains as it was originally doing the evict and get
within DefaultJsonDataService.update method.
modules/org.openbravo.service.json/src/org/openbravo/service/json/DefaultJsonDataService.java
src/org/openbravo/dal/service/OBDal.java
--- a/modules/org.openbravo.service.json/src/org/openbravo/service/json/DefaultJsonDataService.java	Tue Jul 07 09:30:33 2015 +0200
+++ b/modules/org.openbravo.service.json/src/org/openbravo/service/json/DefaultJsonDataService.java	Tue Jul 07 10:45:53 2015 +0200
@@ -182,8 +182,8 @@
         if (parameters.containsKey(JsonConstants.SUMMARY_PARAMETER)) {
           final JSONObject singleResult = new JSONObject();
           if (queryService.getSummaryFields().size() == 1) {
-            singleResult.put(queryService.getSummaryFields().get(0), queryService.buildOBQuery()
-                .createQuery().uniqueResult());
+            singleResult.put(queryService.getSummaryFields().get(0),
+                queryService.buildOBQuery().createQuery().uniqueResult());
           } else {
             final Object[] os = (Object[]) queryService.buildOBQuery().createQuery().uniqueResult();
             int i = 0;
@@ -229,8 +229,8 @@
         }
 
         jsonResponse.put(JsonConstants.RESPONSE_STARTROW, startRow);
-        jsonResponse.put(JsonConstants.RESPONSE_ENDROW, (bobs.size() > 0 ? bobs.size() + startRow
-            - 1 : 0));
+        jsonResponse.put(JsonConstants.RESPONSE_ENDROW,
+            (bobs.size() > 0 ? bobs.size() + startRow - 1 : 0));
         // bobs can be empty and count > 0 if the order by forces a join without results
         if (bobs.isEmpty()) {
           if (startRow > 0) {
@@ -245,11 +245,12 @@
         }
       }
 
-      final DataToJsonConverter toJsonConverter = OBProvider.getInstance().get(
-          DataToJsonConverter.class);
+      final DataToJsonConverter toJsonConverter = OBProvider.getInstance()
+          .get(DataToJsonConverter.class);
       toJsonConverter.setAdditionalProperties(JsonUtils.getAdditionalProperties(parameters));
       toJsonConverter.setSelectedProperties(selectedProperties);
-      if (StringUtils.isNotEmpty(displayField) && (!displayField.equals(JsonConstants.IDENTIFIER))) {
+      if (StringUtils.isNotEmpty(displayField)
+          && (!displayField.equals(JsonConstants.IDENTIFIER))) {
         toJsonConverter.setDisplayProperty(displayField);
       }
       final List<JSONObject> jsonObjects = toJsonConverter.toJsonObjects(bobs);
@@ -275,8 +276,8 @@
 
     String selectedProperties = parameters.get(JsonConstants.SELECTEDPROPERTIES_PARAMETER);
 
-    final DataToJsonConverter toJsonConverter = OBProvider.getInstance().get(
-        DataToJsonConverter.class);
+    final DataToJsonConverter toJsonConverter = OBProvider.getInstance()
+        .get(DataToJsonConverter.class);
     toJsonConverter.setAdditionalProperties(JsonUtils.getAdditionalProperties(parameters));
     // Convert to Json only the properties specified in the request. If no properties are specified,
     // all of them will be converted to Json
@@ -302,7 +303,8 @@
         // Clear session every 1000 records to prevent huge memory consumption in case of big loops
         if (i % 1000 == 0) {
           OBDal.getInstance().getSession().clear();
-          log.debug("clearing in record " + i + " elapsed time " + (System.currentTimeMillis() - t));
+          log.debug(
+              "clearing in record " + i + " elapsed time " + (System.currentTimeMillis() - t));
         }
       }
     } finally {
@@ -320,8 +322,8 @@
       boolean forCountOperation, boolean forSubEntity, boolean filterOnReadableOrganizations) {
     boolean hasSubentity = false;
     String entityName = parameters.get(JsonConstants.ENTITYNAME);
-    final DataEntityQueryService queryService = OBProvider.getInstance().get(
-        DataEntityQueryService.class);
+    final DataEntityQueryService queryService = OBProvider.getInstance()
+        .get(DataEntityQueryService.class);
 
     if (!forSubEntity && parameters.get(JsonConstants.DISTINCT_PARAMETER) != null) {
       // this is the main entity of a 'contains' (used in FK drop down lists), it will create also
@@ -333,12 +335,12 @@
         // Showing the records unfiltered improves the performance if the referenced table has just
         // a few records and the referencing table has lots
         final String distinctPropertyPath = parameters.get(JsonConstants.DISTINCT_PARAMETER);
-        final Property distinctProperty = DalUtil.getPropertyFromPath(ModelProvider.getInstance()
-            .getEntity(entityName), distinctPropertyPath);
+        final Property distinctProperty = DalUtil.getPropertyFromPath(
+            ModelProvider.getInstance().getEntity(entityName), distinctPropertyPath);
         final Entity distinctEntity = distinctProperty.getTargetEntity();
         queryService.setEntityName(distinctEntity.getName());
-        queryService
-            .addFilterParameter(JsonConstants.SHOW_FK_DROPDOWN_UNFILTERED_PARAMETER, "true");
+        queryService.addFilterParameter(JsonConstants.SHOW_FK_DROPDOWN_UNFILTERED_PARAMETER,
+            "true");
         queryService.setFilterOnReadableOrganizations(filterOnReadableOrganizations);
         if (parameters.containsKey(JsonConstants.USE_ALIAS)) {
           queryService.setUseAlias();
@@ -353,8 +355,8 @@
           for (String criterion : criteria.split(JsonConstants.IN_PARAMETER_SEPARATOR)) {
             try {
               JSONObject jsonCriterion = new JSONObject(criterion);
-              if (jsonCriterion.getString("fieldName").equals(
-                  distinctPropertyPath + "$" + JsonConstants.IDENTIFIER)) {
+              if (jsonCriterion.getString("fieldName")
+                  .equals(distinctPropertyPath + "$" + JsonConstants.IDENTIFIER)) {
                 jsonCriterion.put("fieldName", JsonConstants.IDENTIFIER);
                 baseCriteria = jsonCriterion.toString();
               }
@@ -374,8 +376,8 @@
       } else {
 
         final String distinctPropertyPath = parameters.get(JsonConstants.DISTINCT_PARAMETER);
-        final Property distinctProperty = DalUtil.getPropertyFromPath(ModelProvider.getInstance()
-            .getEntity(entityName), distinctPropertyPath);
+        final Property distinctProperty = DalUtil.getPropertyFromPath(
+            ModelProvider.getInstance().getEntity(entityName), distinctPropertyPath);
         final Entity distinctEntity = distinctProperty.getTargetEntity();
 
         // criteria needs to be split in two parts:
@@ -389,8 +391,8 @@
           for (String criterion : criteria.split(JsonConstants.IN_PARAMETER_SEPARATOR)) {
             try {
               JSONObject jsonCriterion = new JSONObject(criterion);
-              if (jsonCriterion.getString("fieldName").equals(
-                  distinctPropertyPath + "$" + JsonConstants.IDENTIFIER)) {
+              if (jsonCriterion.getString("fieldName")
+                  .equals(distinctPropertyPath + "$" + JsonConstants.IDENTIFIER)) {
                 jsonCriterion.put("fieldName", JsonConstants.IDENTIFIER);
                 baseCriteria = jsonCriterion.toString();
               } else {
@@ -432,10 +434,9 @@
         queryService.setFilterOnActive(false);
 
         // create now subentity
-        queryService.setSubEntity(
-            entityName,
-            createSetQueryService(paramSubCriteria, forCountOperation, true,
-                filterOnReadableOrganizations), distinctProperty, distinctPropertyPath);
+        queryService.setSubEntity(entityName, createSetQueryService(paramSubCriteria,
+            forCountOperation, true, filterOnReadableOrganizations), distinctProperty,
+            distinctPropertyPath);
       }
     } else {
       queryService.setEntityName(entityName);
@@ -471,19 +472,19 @@
         && parameters.get(JsonConstants.TARGETRECORDID_PARAMETER) != null
         && !"null".equals(parameters.get(JsonConstants.TARGETRECORDID_PARAMETER))
         && !"true".equals(parameters.get("_directNavigation"))) {
-      log.warn("Datasource request with targetRecordId but without directNavigation detected. This type of requests should be avoided because they result in a query that performs poorly. Parameters: "
-          + convertParameterToString(parameters));
+      log.warn(
+          "Datasource request with targetRecordId but without directNavigation detected. This type of requests should be avoided because they result in a query that performs poorly. Parameters: "
+              + convertParameterToString(parameters));
     }
 
     if (!directNavigation) {
       // set the where/org filter parameters and the @ parameters
       for (String key : parameters.keySet()) {
-        if (key.equals(JsonConstants.WHERE_PARAMETER)
-            || key.equals(JsonConstants.IDENTIFIER)
+        if (key.equals(JsonConstants.WHERE_PARAMETER) || key.equals(JsonConstants.IDENTIFIER)
             || key.equals(JsonConstants.ORG_PARAMETER)
             || key.equals(JsonConstants.TARGETRECORDID_PARAMETER)
-            || (key.startsWith(DataEntityQueryService.PARAM_DELIMITER) && key
-                .endsWith(DataEntityQueryService.PARAM_DELIMITER))) {
+            || (key.startsWith(DataEntityQueryService.PARAM_DELIMITER)
+                && key.endsWith(DataEntityQueryService.PARAM_DELIMITER))) {
           queryService.addFilterParameter(key, parameters.get(key));
         }
 
@@ -629,10 +630,10 @@
 
       try {
         // create the result info before deleting to prevent Hibernate errors
-        final DataToJsonConverter toJsonConverter = OBProvider.getInstance().get(
-            DataToJsonConverter.class);
-        final List<JSONObject> jsonObjects = toJsonConverter.toJsonObjects(Collections
-            .singletonList(bob));
+        final DataToJsonConverter toJsonConverter = OBProvider.getInstance()
+            .get(DataToJsonConverter.class);
+        final List<JSONObject> jsonObjects = toJsonConverter
+            .toJsonObjects(Collections.singletonList(bob));
 
         final JSONObject jsonResult = new JSONObject();
         final JSONObject jsonResponse = new JSONObject();
@@ -681,11 +682,11 @@
    */
   public String update(Map<String, String> parameters, String content) {
     try {
-      final boolean sendOriginalIdBack = "true".equals(parameters
-          .get(JsonConstants.SEND_ORIGINAL_ID_BACK));
+      final boolean sendOriginalIdBack = "true"
+          .equals(parameters.get(JsonConstants.SEND_ORIGINAL_ID_BACK));
 
-      final JsonToDataConverter fromJsonConverter = OBProvider.getInstance().get(
-          JsonToDataConverter.class);
+      final JsonToDataConverter fromJsonConverter = OBProvider.getInstance()
+          .get(JsonToDataConverter.class);
 
       String localContent = content;
       if (parameters.containsKey(ADD_FLAG)) {
@@ -755,12 +756,14 @@
         // refresh the objects from the db as they can have changed
         // put the refreshed objects into a new array as we are going to retrieve them using
         // OBDal.getInstance().get as performs better than OBDal.getInstance().getSession().refresh
-        // We use OBDal.getInstance().get inside OBDal.getInstance().refresh method after removing
-        // the bob from the session cache
         // See issue https://issues.openbravo.com/view.php?id=30308
         final List<BaseOBObject> refreshedBobs = new ArrayList<BaseOBObject>();
         for (BaseOBObject bob : bobs) {
-          BaseOBObject refreshedBob = OBDal.getInstance().refresh(bob, false);
+          // Remove the bob instance from the session cache with evict
+          OBDal.getInstance().getSession().evict(bob);
+          // With get() we retrieve the object from db as we have cleared it from cache with evict()
+          BaseOBObject refreshedBob = OBDal.getInstance().get(bob.getEntityName(),
+              DalUtil.getId(bob));
           // if object has computed columns refresh from the database too
           if (refreshedBob.getEntity().hasComputedColumns()) {
             OBDal.getInstance().getSession()
@@ -771,8 +774,8 @@
 
         // almost successfull, now create the response
         // needs to be done before the close of the session
-        final DataToJsonConverter toJsonConverter = OBProvider.getInstance().get(
-            DataToJsonConverter.class);
+        final DataToJsonConverter toJsonConverter = OBProvider.getInstance()
+            .get(DataToJsonConverter.class);
         toJsonConverter.setAdditionalProperties(JsonUtils.getAdditionalProperties(parameters));
         final List<JSONObject> jsonObjects = toJsonConverter.toJsonObjects(refreshedBobs);
 
@@ -780,8 +783,8 @@
           // now it is assumed that the jsonObjects are the same size and the same location
           // in the array
           if (jsonObjects.size() != originalData.size()) {
-            throw new OBException("Unequal sizes in json data processed " + jsonObjects.size()
-                + " " + originalData.size());
+            throw new OBException("Unequal sizes in json data processed " + jsonObjects.size() + " "
+                + originalData.size());
           }
 
           // now add the old id back
@@ -804,7 +807,8 @@
         if (parameters.containsKey(ADD_FLAG)) {
           result = doPostAction(parameters, jsonResult.toString(), DataSourceAction.ADD, content);
         } else {
-          result = doPostAction(parameters, jsonResult.toString(), DataSourceAction.UPDATE, content);
+          result = doPostAction(parameters, jsonResult.toString(), DataSourceAction.UPDATE,
+              content);
         }
 
         OBDal.getInstance().commitAndClose();
@@ -1085,7 +1089,8 @@
       JSONArray criteria = jsonCriteria.getJSONArray("criteria");
       for (int i = 0; i < criteria.length(); i++) {
         JSONObject criterion = criteria.getJSONObject(i);
-        if (criterion.has("fieldName") && JsonConstants.ID.equals(criterion.getString("fieldName"))) {
+        if (criterion.has("fieldName")
+            && JsonConstants.ID.equals(criterion.getString("fieldName"))) {
           return true;
         }
       }
--- a/src/org/openbravo/dal/service/OBDal.java	Tue Jul 07 09:30:33 2015 +0200
+++ b/src/org/openbravo/dal/service/OBDal.java	Tue Jul 07 10:45:53 2015 +0200
@@ -285,24 +285,6 @@
   }
 
   /**
-   * Refresh the given BaseOBObject.
-   * 
-   * @param bob
-   *          the BaseOBObject to refresh
-   * @param useCache
-   *          a flag to indicate if the refresh is done from cache (true) or from db (false)
-   * @return the refreshed BaseOBObject, or null if none found
-   */
-  public BaseOBObject refresh(BaseOBObject bob, boolean useCache) {
-    if (!useCache) {
-      // Remove the bob instance from the session cache with evict
-      // Now with get() we will retrieve the object from database
-      SessionHandler.getInstance().getSession().evict(bob);
-    }
-    return get(bob.getEntityName(), bob.getId());
-  }
-
-  /**
    * Retrieves an object from the database using the class and id.
    * 
    * @param clazz