[ofbiz-framework] branch trunk updated: Improved: Use FlexibleStringExpander in form widget lookup field field target parameters

This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 08a371d  Improved: Use FlexibleStringExpander in form widget lookup field field target parameters
08a371d is described below

commit 08a371d0471a506e663c879f30299f424fc70921
Author: Daniel Watford <[hidden email]>
AuthorDate: Tue Feb 25 20:41:09 2020 +0000

    Improved: Use FlexibleStringExpander in form widget lookup field field
    target parameters
   
    (OFBIZ-11418)
   
    Changed target-parameter attribute of the form widget's Lookup field to
    use the FlexibleStringExpander type rather than String. This has been
    done to allow reference to other form fields which may have had their
    parameter-name attribute set to a FlexibleStringExpander expression.
---
 .../apache/ofbiz/widget/model/ModelFormField.java  | 42 ++++++++++++------
 .../widget/renderer/macro/MacroFormRenderer.java   |  2 +-
 .../ofbiz/widget/model/ModelFormFieldTest.java     | 50 ++++++++++++++++++++++
 3 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormField.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormField.java
index 7d56524..075fc2e 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormField.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormField.java
@@ -24,6 +24,7 @@ import java.sql.Timestamp;
 import java.text.DateFormat;
 import java.text.NumberFormat;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
@@ -33,10 +34,12 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.StringTokenizer;
+import java.util.Optional;
 import java.util.TimeZone;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.apache.ofbiz.base.conversion.ConversionException;
 import org.apache.ofbiz.base.conversion.DateTimeConverters;
@@ -3152,7 +3155,7 @@ public class ModelFormField {
         private final String lookupPresentation;
         private final String lookupWidth;
         private final String showDescription;
-        private final String targetParameter;
+        private final FlexibleStringExpander targetParameter;
 
         public LookupField(Element element, ModelFormField modelFormField) {
             super(element, modelFormField);
@@ -3165,7 +3168,7 @@ public class ModelFormField {
             this.lookupPresentation = element.getAttribute("presentation");
             this.lookupWidth = element.getAttribute("width");
             this.showDescription = element.getAttribute("show-description");
-            this.targetParameter = element.getAttribute("target-parameter");
+            this.targetParameter = FlexibleStringExpander.getInstance(element.getAttribute("target-parameter"));
         }
 
         public LookupField(int fieldSource, ModelFormField modelFormField) {
@@ -3179,7 +3182,7 @@ public class ModelFormField {
             this.lookupPresentation = "";
             this.lookupWidth = "";
             this.showDescription = "";
-            this.targetParameter = "";
+            this.targetParameter = FlexibleStringExpander.getInstance("");
         }
 
         public LookupField(LookupField original, ModelFormField modelFormField) {
@@ -3247,19 +3250,32 @@ public class ModelFormField {
             return UtilValidate.isEmpty(this.showDescription) ? null : "true".equals(this.showDescription);
         }
 
-        public String getTargetParameter() {
+        public FlexibleStringExpander getTargetParameter() {
             return targetParameter;
         }
 
+        public String getTargetParameter(final Map<String, Object> context) {
+            return targetParameter.expandString(context);
+        }
+
+        /**
+         * @deprecated Target Parameter is potentially a FlexibleStringExpander expression and should therefore be
+         * evaluated within a given context.
+         * <p> Use {@link #getTargetParameter(Map)} instead.</p>
+         */
+        @Deprecated
         public List<String> getTargetParameterList() {
-            List<String> paramList = new LinkedList<>();
-            if (UtilValidate.isNotEmpty(this.targetParameter)) {
-                StringTokenizer stk = new StringTokenizer(this.targetParameter, ", ");
-                while (stk.hasMoreTokens()) {
-                    paramList.add(stk.nextToken());
-                }
-            }
-            return paramList;
+            // Evaluate target-parameter with a null context, treating it as a plain string.
+            return getTargetParameterList(null);
+        }
+
+        public List<String> getTargetParameterList(final Map<String, Object> context) {
+            return Optional.ofNullable(getTargetParameter(context))
+                    .map(x -> x.split(","))
+                    .map(Arrays::stream)
+                    .orElse(Stream.empty())
+                    .map(String::trim)
+                    .collect(Collectors.toList());
         }
 
         @Override
diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java
index 2a7ee14..029d383 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java
@@ -2241,7 +2241,7 @@ public final class MacroFormRenderer implements FormStringRenderer {
         StringBuilder targetParameterIter = new StringBuilder();
         StringBuilder imgSrc = new StringBuilder();
         // FIXME: refactor using the StringUtils methods
-        List<String> targetParameterList = lookupField.getTargetParameterList();
+        List<String> targetParameterList = lookupField.getTargetParameterList(context);
         targetParameterIter.append("[");
         for (String targetParameter : targetParameterList) {
             if (targetParameterIter.length() > 1) {
diff --git a/framework/widget/src/test/java/org/apache/ofbiz/widget/model/ModelFormFieldTest.java b/framework/widget/src/test/java/org/apache/ofbiz/widget/model/ModelFormFieldTest.java
index 16146ef..271e611 100644
--- a/framework/widget/src/test/java/org/apache/ofbiz/widget/model/ModelFormFieldTest.java
+++ b/framework/widget/src/test/java/org/apache/ofbiz/widget/model/ModelFormFieldTest.java
@@ -22,6 +22,7 @@ import static org.apache.ofbiz.widget.model.ModelFormField.from;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.when;
 
 import java.util.Arrays;
 import java.util.HashMap;
@@ -29,8 +30,11 @@ import java.util.List;
 import java.util.stream.Collectors;
 
 import com.google.common.collect.ImmutableMap;
+import org.hamcrest.Matchers;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mockito;
+import org.w3c.dom.Element;
 
 public class ModelFormFieldTest {
     private HashMap<String, Object> context;
@@ -105,4 +109,50 @@ public class ModelFormFieldTest {
         assertThat(field.getCurrentContainerId(ImmutableMap.of("prefix", "P1")), equalTo("P1IdValue"));
         assertThat(field.getCurrentContainerId(ImmutableMap.of("prefix", "P2")), equalTo("P2IdValue"));
     }
+
+    /**
+     * Ensures behaviour of deprecated method LookupField#getTargetParameterList is maintained while the underlying
+     * property type is changed from String to FlexibleStringExpander.
+     */
+    @Test
+    public void lookupFieldDeprecatedMethodTreatsTargetParameterAsString() {
+        Element element = Mockito.mock(Element.class);
+        when(element.getTagName()).thenReturn("lookup");
+        when(element.getAttribute("maxlength")).thenReturn("1");
+        when(element.getAttribute("size")).thenReturn("1");
+        when(element.getAttribute("target-parameter")).thenReturn("${prefix}TargetParam, ${key1}");
+
+        ModelFormField field = from(b -> b.setName("lookup-field"));
+        ModelFormField.LookupField lookupField = new ModelFormField.LookupField(element, field);
+
+        assertThat(lookupField.getTargetParameterList(), Matchers.contains("${prefix}TargetParam", "${key1}"));
+    }
+
+    @Test
+    public void lookupFieldUsesFlexibleTargetParameters() {
+        Element element = Mockito.mock(Element.class);
+        when(element.getTagName()).thenReturn("lookup");
+        when(element.getAttribute("maxlength")).thenReturn("1");
+        when(element.getAttribute("size")).thenReturn("1");
+        when(element.getAttribute("target-parameter")).thenReturn("${prefix}TargetParam");
+
+        ModelFormField field = from(b -> b.setName("lookup-field"));
+        ModelFormField.LookupField lookupField = new ModelFormField.LookupField(element, field);
+
+        assertThat(lookupField.getTargetParameterList(ImmutableMap.of("prefix", "P1")), Matchers.contains("P1TargetParam"));
+    }
+
+    @Test
+    public void lookupFieldEvaluatesExpressionBeforeSplitting() {
+        Element element = Mockito.mock(Element.class);
+        when(element.getTagName()).thenReturn("lookup");
+        when(element.getAttribute("maxlength")).thenReturn("1");
+        when(element.getAttribute("size")).thenReturn("1");
+        when(element.getAttribute("target-parameter")).thenReturn("${prefix}TargetParam, ${key1}");
+
+        ModelFormField field = from(b -> b.setName("lookup-field"));
+        ModelFormField.LookupField lookupField = new ModelFormField.LookupField(element, field);
+
+        assertThat(lookupField.getTargetParameterList(ImmutableMap.of("prefix", "P1", "key1", "AA,BB , CC")), Matchers.contains("P1TargetParam", "AA", "BB", "CC"));
+    }
 }