svn commit: r1835891 - in /ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product: ProductSearch.java ProductSearchSession.java

Author: jleroux
Date: Sat Jul 14 09:52:26 2018
New Revision: 1835891

URL: http://svn.apache.org/viewvc?rev=1835891&view=rev
Log:
Fixed: Refactor ContentWorkerInterface methods signatures
(OFBIZ-9164)

If you go at: catalog/control/main then open the lookup on the left for category
 id, click the first category (20111) and then click on search, you will be
prompted with this:
java.lang.IllegalArgumentException: Error running script at location
[component://product/groovyScripts/catalog/find/KeywordSearch.groovy]:
java.lang.NullPointerException

This is the result of the changes I made for this issue 1.5 years ago.

I missed the case where a null is passed instead of a dispatcher in
getProductCategoryContentAsText()
which is then used inside of getProductCategoryContentAsText:
ContentWorker.renderContentAsText()
The problem is, that the method renderContentAsText as a result of my changes
uses the dispatcher to get the delegator.
Because the dispatcher is a given a null value at this point, this results in a
null pointer.

The solution was to introduce and abstract prettyPrintConstraint method with a
LocalDispatcher as argument and to implement it in CategoryConstraint class.

Other implementations don't use getProductCategoryContentAsText so can simply
return null.

CatalogConstraint.prettyPrintConstraint with a delegator is now not
used OOTB but I prefer to keep its implementation for custom projects which
might use it in other circumstances. I though put a TODO to possibly simply
return null.

Also while at it remove a unused requestParams variable in ProductSearchSession

Thanks: Dennis Balkir for the detailed reporting

Modified:
    ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearch.java
    ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearchSession.java

Modified: ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearch.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearch.java?rev=1835891&r1=1835890&r2=1835891&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearch.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearch.java Sat Jul 14 09:52:26 2018
@@ -38,6 +38,7 @@ import org.apache.ofbiz.base.util.UtilPr
 import org.apache.ofbiz.base.util.UtilValidate;
 import org.apache.ofbiz.common.KeywordSearchUtil;
 import org.apache.ofbiz.entity.Delegator;
+import org.apache.ofbiz.entity.GenericDelegator;
 import org.apache.ofbiz.entity.GenericEntityException;
 import org.apache.ofbiz.entity.GenericValue;
 import org.apache.ofbiz.entity.condition.EntityComparisonOperator;
@@ -58,6 +59,7 @@ import org.apache.ofbiz.entity.util.Enti
 import org.apache.ofbiz.entity.util.EntityUtil;
 import org.apache.ofbiz.party.party.PartyHelper;
 import org.apache.ofbiz.product.category.CategoryContentWrapper;
+import org.apache.ofbiz.service.LocalDispatcher;
 
 
 /**
@@ -827,6 +829,7 @@ public class ProductSearch {
         public abstract void addConstraint(ProductSearchContext productSearchContext);
         /** pretty print for log messages and even UI stuff */
         public abstract String prettyPrintConstraint(Delegator delegator, boolean detailed, Locale locale);
+        public abstract String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale);
     }
 
 
@@ -921,6 +924,14 @@ public class ProductSearch {
             return true;
         }
 
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
+
     }
 
     @SuppressWarnings("serial")
@@ -967,6 +978,7 @@ public class ProductSearch {
         }
 
         /** pretty print for log messages and even UI stuff */
+        // TODO This is not used OOTB since OFBIZ-9164 and could simply return null, kept for custom projects
         @Override
         public String prettyPrintConstraint(Delegator delegator, boolean detailed, Locale locale) {
             GenericValue productCategory = null;
@@ -995,6 +1007,36 @@ public class ProductSearch {
             return ppBuf.toString();
         }
 
+        /** pretty print for log messages and even UI stuff */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            GenericValue productCategory = null;
+            GenericDelegator delegator = (GenericDelegator) dispatcher.getDelegator();
+            try {
+                productCategory = EntityQuery.use(delegator).from("ProductCategory").where("productCategoryId", productCategoryId).cache().queryOne();
+            } catch (GenericEntityException e) {
+                Debug.logError(e, "Error finding ProductCategory information for constraint pretty print", module);
+            }
+            StringBuilder ppBuf = new StringBuilder();
+            ppBuf.append(UtilProperties.getMessage(resource, "ProductCategory", locale)).append(": ");
+            if (productCategory != null) {
+                String catInfo = CategoryContentWrapper.getProductCategoryContentAsText(productCategory, "CATEGORY_NAME", locale, dispatcher, "html");
+                if (UtilValidate.isEmpty(catInfo)) {
+                    catInfo = CategoryContentWrapper.getProductCategoryContentAsText(productCategory, "DESCRIPTION", locale, dispatcher, "html");
+                }
+                ppBuf.append(catInfo);
+            }
+            if (productCategory == null || detailed) {
+                ppBuf.append(" [");
+                ppBuf.append(productCategoryId);
+                ppBuf.append("]");
+            }
+            if (includeSubCategories) {
+                ppBuf.append(" (").append(UtilProperties.getMessage(resource, "ProductIncludeAllSubCategories", locale)).append(")");
+            }
+            return ppBuf.toString();
+        }
+
         @Override
         public int hashCode() {
             final int prime = 31;
@@ -1138,6 +1180,14 @@ public class ProductSearch {
             return true;
         }
 
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
+
     }
 
 
@@ -1239,6 +1289,14 @@ public class ProductSearch {
             return true;
         }
 
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
+
     }
 
     @SuppressWarnings("serial")
@@ -1338,6 +1396,14 @@ public class ProductSearch {
             return true;
         }
 
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
+
     }
 
 
@@ -1452,6 +1518,14 @@ public class ProductSearch {
             return true;
         }
 
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
+
     }
 
     @SuppressWarnings("serial")
@@ -1588,6 +1662,14 @@ public class ProductSearch {
             return true;
         }
 
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
+
     }
 
     @SuppressWarnings("serial")
@@ -1651,6 +1733,14 @@ public class ProductSearch {
             return true;
         }
 
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
+
     }
 
     @SuppressWarnings("serial")
@@ -1745,6 +1835,14 @@ public class ProductSearch {
             return true;
         }
 
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
+
     }
 
     @SuppressWarnings("serial")
@@ -1870,6 +1968,14 @@ public class ProductSearch {
             return true;
         }
 
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
+
     }
 
     @SuppressWarnings("serial")
@@ -1932,6 +2038,14 @@ public class ProductSearch {
             return true;
         }
 
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
+
     }
 
     @SuppressWarnings("serial")
@@ -1976,6 +2090,14 @@ public class ProductSearch {
             }
             return true;
         }
+
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
     }
 
     @SuppressWarnings("serial")
@@ -2020,6 +2142,14 @@ public class ProductSearch {
             }
             return true;
         }
+
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
     }
 
     @SuppressWarnings("serial")
@@ -2126,6 +2256,14 @@ public class ProductSearch {
             }
             return true;
         }
+
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
     }
 
     @SuppressWarnings("serial")
@@ -2189,6 +2327,14 @@ public class ProductSearch {
             return true;
         }
 
+        /* (non-Javadoc)
+         * @see org.apache.ofbiz.product.product.ProductSearch.ProductSearchConstraint#prettyPrintConstraint(org.apache.ofbiz.service.LocalDispatcher, boolean, java.util.Locale)
+         */
+        @Override
+        public String prettyPrintConstraint(LocalDispatcher dispatcher, boolean detailed, Locale locale) {
+            return null;
+        }
+
     }
 
     // ======================================================================

Modified: ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearchSession.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearchSession.java?rev=1835891&r1=1835890&r2=1835891&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearchSession.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearchSession.java Sat Jul 14 09:52:26 2018
@@ -64,6 +64,7 @@ import org.apache.ofbiz.product.product.
 import org.apache.ofbiz.product.product.ProductSearch.ResultSortOrder;
 import org.apache.ofbiz.product.product.ProductSearch.SortKeywordRelevancy;
 import org.apache.ofbiz.product.store.ProductStoreWorker;
+import org.apache.ofbiz.service.LocalDispatcher;
 import org.apache.ofbiz.webapp.control.RequestHandler;
 import org.apache.ofbiz.webapp.stats.VisitHandler;
 
@@ -297,6 +298,25 @@ public class ProductSearchSession {
             }
             return constraintStrings;
         }
+        public List<String> searchGetConstraintStrings(boolean detailed, LocalDispatcher dispatcher, Locale locale) {
+            List<ProductSearchConstraint> productSearchConstraintList = this.getConstraintList();
+            List<String> constraintStrings = new LinkedList<>();
+            if (productSearchConstraintList == null) {
+                return constraintStrings;
+            }
+            for (ProductSearchConstraint productSearchConstraint: productSearchConstraintList) {
+                if (productSearchConstraint == null) {
+                    continue;
+                }                
+                String constraintString = productSearchConstraint.prettyPrintConstraint(dispatcher, detailed, locale);
+                if (UtilValidate.isNotEmpty(constraintString)) {
+                    constraintStrings.add(constraintString);
+                } else {
+                    constraintStrings.add("Description not available");
+                }
+            }
+            return constraintStrings;
+        }
     }
 
     public static ProductSearchOptions getProductSearchOptions(HttpSession session) {
@@ -379,7 +399,6 @@ public class ProductSearchSession {
     public static final String checkDoKeywordOverride(HttpServletRequest request, HttpServletResponse response) {
         HttpSession session = request.getSession();
         Delegator delegator = (Delegator) request.getAttribute("delegator");
-        Map<String, Object> requestParams = UtilHttp.getParameterMap(request);
 
         // get the current productStoreId
         String productStoreId = ProductStoreWorker.getProductStoreId(request);
@@ -470,7 +489,8 @@ public class ProductSearchSession {
     public static List<String> searchGetConstraintStrings(boolean detailed, HttpSession session, Delegator delegator) {
         Locale locale = UtilHttp.getLocale(session);
         ProductSearchOptions productSearchOptions = getProductSearchOptions(session);
-        return productSearchOptions.searchGetConstraintStrings(detailed, delegator, locale);
+        LocalDispatcher dispatcher = (LocalDispatcher) session.getAttribute("dispatcher");
+        return productSearchOptions.searchGetConstraintStrings(detailed, dispatcher, locale);
     }
 
     public static String searchGetSortOrderString(boolean detailed, HttpServletRequest request) {