Ah I totally forgot to mention that this also removes the swallowed exception that we had in ModelFormAction and ModelTreeAction, but not in ModelActionUtil from where I extracted the method

It's now consistent and if ever, for a very unlikely reason, the exception is 
thrown, people will know about it and will take appropriate measures.

Jacques


Le 31/03/2017 à 21:37, jler...@apache.org a écrit :
Author: jleroux
Date: Fri Mar 31 19:37:00 2017
New Revision: 1789737

URL: http://svn.apache.org/viewvc?rev=1789737&view=rev
Log:
Improved: Refactor the runAction method in AbstractModelAction, ModelFormAction
and ModelTreeAction classes
(OFBIZ-9287)

This 3 methods share a pattern which should be refactored in a common protected
  method. The pattern is
{code}
if (!this.resultMapNameAcsr.isEmpty()) {
     this.resultMapNameAcsr.put(context, result);
     String queryString = (String) result.get("queryString");
     context.put("queryString", queryString);
     context.put("queryStringMap", result.get("queryStringMap"));
     if (UtilValidate.isNotEmpty(queryString)) {
         try {
             String queryStringEncoded = queryString.replaceAll("&", "%26");
             context.put("queryStringEncoded", queryStringEncoded);
         } catch (PatternSyntaxException e) {
          // obviously a PatternSyntaxException should not occur here
         }
     }
} else {
     context.putAll(result);
}
{code}
Enough for a refactor I'd say.

I created a ModelActionUtil class where I extracted the method I used in the 3
other classes.

Added:
     
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelActionUtil.java
   (with props)
Modified:
     
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/AbstractModelAction.java
     
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormAction.java
     
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelTreeAction.java

Modified: 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/AbstractModelAction.java
URL: 
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/AbstractModelAction.java?rev=1789737&r1=1789736&r2=1789737&view=diff
==============================================================================
--- 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/AbstractModelAction.java
 (original)
+++ 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/AbstractModelAction.java
 Fri Mar 31 19:37:00 2017
@@ -28,7 +28,6 @@ import java.util.List;
  import java.util.Locale;
  import java.util.Map;
  import java.util.TimeZone;
-import java.util.regex.PatternSyntaxException;
import javax.servlet.ServletContext;
  import javax.servlet.http.HttpSession;
@@ -705,22 +704,7 @@ public abstract class AbstractModelActio
                      EntityFinderUtil.expandFieldMapToContext(this.fieldMap, 
context, serviceContext);
                  }
                  Map<String, Object> result = 
WidgetWorker.getDispatcher(context).runSync(serviceNameExpanded, serviceContext);
-                if (!this.resultMapNameAcsr.isEmpty()) {
-                    this.resultMapNameAcsr.put(context, result);
-                    String queryString = (String) result.get("queryString");
-                    context.put("queryString", queryString);
-                    context.put("queryStringMap", 
result.get("queryStringMap"));
-                    if (UtilValidate.isNotEmpty(queryString)) {
-                        try {
-                            String queryStringEncoded = queryString.replaceAll("&", 
"%26");
-                            context.put("queryStringEncoded", 
queryStringEncoded);
-                        } catch (PatternSyntaxException e) {
-                         // obviously a PatternSyntaxException should not 
occur here
-                        }
-                    }
-                } else {
-                    context.putAll(result);
-                }
+                ModelActionUtil.contextPutQueryStringOrAllResult(context, 
result, this.resultMapNameAcsr);
              } catch (GenericServiceException e) {
                  String errMsg = "Error calling service with name " + 
serviceNameExpanded + ": " + e.toString();
                  Debug.logError(e, errMsg, module);

Added: 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelActionUtil.java
URL: 
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelActionUtil.java?rev=1789737&view=auto
==============================================================================
--- 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelActionUtil.java
 (added)
+++ 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelActionUtil.java
 Fri Mar 31 19:37:00 2017
@@ -0,0 +1,46 @@
+/*******************************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ 
*******************************************************************************/
+package org.apache.ofbiz.widget.model;
+
+import java.util.Map;
+
+import org.apache.ofbiz.base.util.UtilValidate;
+import org.apache.ofbiz.base.util.collections.FlexibleMapAccessor;
+
+public class ModelActionUtil {
+
+    /**
+     * @param context
+     * @param result
+     */
+    protected static void contextPutQueryStringOrAllResult(Map<String, Object> context, 
Map<String, Object> result, FlexibleMapAccessor<Map<String, Object>> 
resultMapNameAcsr) {
+        if (!resultMapNameAcsr.isEmpty()) {
+            resultMapNameAcsr.put(context, result);
+            String queryString = (String) result.get("queryString");
+            context.put("queryString", queryString);
+            context.put("queryStringMap", result.get("queryStringMap"));
+            if (UtilValidate.isNotEmpty(queryString)) {
+                String queryStringEncoded = queryString.replaceAll("&", "%26");
+                context.put("queryStringEncoded", queryStringEncoded);
+            }
+        } else {
+            context.putAll(result);
+        }
+    }
+}

Propchange: 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelActionUtil.java
------------------------------------------------------------------------------
     svn:eol-style = native

Propchange: 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelActionUtil.java
------------------------------------------------------------------------------
     svn:keywords = Date Rev Author URL Id

Propchange: 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelActionUtil.java
------------------------------------------------------------------------------
     svn:mime-type = text/plain

Modified: 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormAction.java
URL: 
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormAction.java?rev=1789737&r1=1789736&r2=1789737&view=diff
==============================================================================
--- 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormAction.java
 (original)
+++ 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormAction.java
 Fri Mar 31 19:37:00 2017
@@ -24,7 +24,6 @@ import java.util.HashMap;
  import java.util.List;
  import java.util.ListIterator;
  import java.util.Map;
-import java.util.regex.PatternSyntaxException;
import org.apache.ofbiz.base.util.Debug;
  import org.apache.ofbiz.base.util.UtilGenerics;
@@ -208,22 +207,7 @@ public abstract class ModelFormAction {
                  } else {
                      result = 
WidgetWorker.getDispatcher(context).runSync(serviceNameExpanded, 
serviceContext);
                  }
-                if (!this.resultMapNameAcsr.isEmpty()) {
-                    this.resultMapNameAcsr.put(context, result);
-                    String queryString = (String) result.get("queryString");
-                    context.put("queryString", queryString);
-                    context.put("queryStringMap", 
result.get("queryStringMap"));
-                    if (UtilValidate.isNotEmpty(queryString)) {
-                        try {
-                            String queryStringEncoded = queryString.replaceAll("&", 
"%26");
-                            context.put("queryStringEncoded", 
queryStringEncoded);
-                        } catch (PatternSyntaxException e) {
-                         // obviously a PatternSyntaxException should not 
occur here
-                        }
-                    }
-                } else {
-                    context.putAll(result);
-                }
+                ModelActionUtil.contextPutQueryStringOrAllResult(context, 
result, this.resultMapNameAcsr);
                  String listName = resultMapListNameExdr.expandString(context);
                  Object listObj = result.get(listName);
                  if (listObj != null) {

Modified: 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelTreeAction.java
URL: 
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelTreeAction.java?rev=1789737&r1=1789736&r2=1789737&view=diff
==============================================================================
--- 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelTreeAction.java
 (original)
+++ 
ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelTreeAction.java
 Fri Mar 31 19:37:00 2017
@@ -23,7 +23,6 @@ import java.util.HashMap;
  import java.util.List;
  import java.util.ListIterator;
  import java.util.Map;
-import java.util.regex.PatternSyntaxException;
import org.apache.ofbiz.base.util.Debug;
  import org.apache.ofbiz.base.util.GeneralException;
@@ -407,22 +406,7 @@ public abstract class ModelTreeAction ex
                      EntityFinderUtil.expandFieldMapToContext(this.fieldMap, 
context, serviceContext);
                  }
                  Map<String, Object> result = 
WidgetWorker.getDispatcher(context).runSync(serviceNameExpanded, serviceContext);
-                if (!this.resultMapNameAcsr.isEmpty()) {
-                    this.resultMapNameAcsr.put(context, result);
-                    String queryString = (String) result.get("queryString");
-                    context.put("queryString", queryString);
-                    context.put("queryStringMap", 
result.get("queryStringMap"));
-                    if (UtilValidate.isNotEmpty(queryString)) {
-                        try {
-                            String queryStringEncoded = queryString.replaceAll("&", 
"%26");
-                            context.put("queryStringEncoded", 
queryStringEncoded);
-                        } catch (PatternSyntaxException e) {
-                            // obviously a PatternSyntaxException should not 
occur here
-                        }
-                    }
-                } else {
-                    context.putAll(result);
-                }
+                ModelActionUtil.contextPutQueryStringOrAllResult(context, 
result, this.resultMapNameAcsr);
                  String resultMapListName = 
resultMapListNameExdr.expandString(context);
                  //String resultMapListIteratorName = 
resultMapListIteratorNameExdr.expandString(context);
                  String resultMapValueName = 
resultMapValueNameExdr.expandString(context);




Reply via email to