svn commit: r1837254 - in /ofbiz/ofbiz-framework/trunk/framework/widget/src: main/java/org/apache/ofbiz/widget/renderer/ test/ test/java/ test/java/org/ test/java/org/apache/ test/java/org/apache/ofbiz/ test/java/org/apache/ofbiz/widget/ test/java/org/...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r1837254 - in /ofbiz/ofbiz-framework/trunk/framework/widget/src: main/java/org/apache/ofbiz/widget/renderer/ test/ test/java/ test/java/org/ test/java/org/apache/ test/java/org/apache/ofbiz/ test/java/org/apache/ofbiz/widget/ test/java/org/...

pgil
Author: pgil
Date: Wed Aug  1 20:02:09 2018
New Revision: 1837254

URL: http://svn.apache.org/viewvc?rev=1837254&view=rev
Log:
Improved : Factorize and Refactor filtering of duplicated ‘use-when’ fields in ‘FormRenderer’
(OFBIZ-10502)

Thanks Mathieu for providing unit test and improvment patches

Added:
    ofbiz/ofbiz-framework/trunk/framework/widget/src/test/
    ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/
    ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/
    ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/
    ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/ofbiz/
    ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/ofbiz/widget/
    ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/ofbiz/widget/renderer/
    ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/ofbiz/widget/renderer/FormRendererTest.java   (with props)
Modified:
    ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/FormRenderer.java

Modified: ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/FormRenderer.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/FormRenderer.java?rev=1837254&r1=1837253&r2=1837254&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/FormRenderer.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/FormRenderer.java Wed Aug  1 20:02:09 2018
@@ -31,6 +31,7 @@ import java.util.NoSuchElementException;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
+import java.util.stream.Collectors;
 
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.UtilGenerics;
@@ -679,6 +680,25 @@ public class FormRenderer {
         formStringRenderer.renderFormatItemRowClose(writer, localContext, modelForm);
     }
 
+    // Filter the field lists by removing the ones with both the same names and "use-when" values.
+    // Keep all fields without a "use-when" attribute.
+    List<ModelFormField> getUsedFields(Map<String, Object> context) {
+        HashMap<String, Boolean> seenUseWhen = new HashMap<>();
+        return modelForm.getFieldList().stream()
+                .filter(field -> {
+                    if (!field.isUseWhenEmpty()) {
+                        String name = field.getName();
+                        boolean shouldUse = field.shouldUse(context);
+                        if (seenUseWhen.containsKey(name)) {
+                            return shouldUse != seenUseWhen.get(name);
+                        }
+                        seenUseWhen.put(name, shouldUse);
+                    }
+                    return true;
+                })
+                .collect(Collectors.toList());
+    }
+
     private void renderItemRows(Appendable writer, Map<String, Object> context, FormStringRenderer formStringRenderer,
             boolean formPerItem, int numOfColumns) throws IOException {
         String lookupName = modelForm.getListName();
@@ -769,26 +789,7 @@ public class FormRenderer {
                      Debug.logVerbose("In form got another row, context is: " + localContext, module);
                 }
 
-                // Check to see if there is a field, same name and same use-when (could come from extended form)
-                List<ModelFormField> tempFieldList = new LinkedList<>();
-                tempFieldList.addAll(modelForm.getFieldList());
-                for (int j = 0; j < tempFieldList.size(); j++) {
-                    ModelFormField modelFormField = tempFieldList.get(j);
-                    if (!modelFormField.isUseWhenEmpty()) {
-                        boolean shouldUse1 = modelFormField.shouldUse(localContext);
-                        for (int i = j + 1; i < tempFieldList.size(); i++) {
-                            ModelFormField curField = tempFieldList.get(i);
-                            if (curField.getName() != null && curField.getName().equals(modelFormField.getName())) {
-                                boolean shouldUse2 = curField.shouldUse(localContext);
-                                if (shouldUse1 == shouldUse2) {
-                                    tempFieldList.remove(i--);
-                                }
-                            } else {
-                                continue;
-                            }
-                        }
-                    }
-                }
+                List<ModelFormField> tempFieldList = getUsedFields(localContext);
 
                 // Each single item is rendered in one or more rows if its fields have
                 // different "position" attributes. All the fields with the same position
@@ -1014,30 +1015,10 @@ public class FormRenderer {
             modelForm.setHideHeader(true);
         }
     }
+
     private void renderSingleFormString(Appendable writer, Map<String, Object> context,
             int positions) throws IOException {
-        List<ModelFormField> tempFieldList = new LinkedList<>();
-        tempFieldList.addAll(modelForm.getFieldList());
-
-        // Check to see if there is a field, same name and same use-when (could come from extended form)
-        for (int j = 0; j < tempFieldList.size(); j++) {
-            ModelFormField modelFormField = tempFieldList.get(j);
-            if (modelForm.getUseWhenFields().contains(modelFormField.getName())) {
-                boolean shouldUse1 = modelFormField.shouldUse(context);
-                for (int i = j + 1; i < tempFieldList.size(); i++) {
-                    ModelFormField curField = tempFieldList.get(i);
-                    if (curField.getName() != null && curField.getName().equals(modelFormField.getName())) {
-                        boolean shouldUse2 = curField.shouldUse(context);
-                        if (shouldUse1 == shouldUse2) {
-                            tempFieldList.remove(i--);
-                        }
-                    } else {
-                        continue;
-                    }
-                }
-            }
-        }
-
+        List<ModelFormField> tempFieldList = getUsedFields(context);
         Set<String> alreadyRendered = new TreeSet<>();
         FieldGroup lastFieldGroup = null;
         // render form open

Added: ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/ofbiz/widget/renderer/FormRendererTest.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/ofbiz/widget/renderer/FormRendererTest.java?rev=1837254&view=auto
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/ofbiz/widget/renderer/FormRendererTest.java (added)
+++ ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/ofbiz/widget/renderer/FormRendererTest.java Wed Aug  1 20:02:09 2018
@@ -0,0 +1,86 @@
+package org.apache.ofbiz.widget.renderer;
+
+import static org.hamcrest.CoreMatchers.hasItems;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+
+import org.apache.ofbiz.widget.model.ModelForm;
+import org.apache.ofbiz.widget.model.ModelFormField;
+import org.apache.ofbiz.widget.model.ModelFormFieldBuilder;
+import org.junit.Before;
+import org.junit.Test;
+
+public class FormRendererTest {
+    private ModelForm model;
+    private FormRenderer renderer;
+    private ArrayList<ModelFormField> fields;
+    private HashMap<String, Object> context;
+    private HashSet<String> useWhenFields;
+
+    @Before
+    public void setUp() throws Exception {
+        model = mock(ModelForm.class);
+        fields = new ArrayList<>();
+        when(model.getFieldList()).thenReturn(fields);
+        renderer = new FormRenderer(model, null);
+        context = new HashMap<>();
+        useWhenFields = new HashSet<>();
+        when(model.getUseWhenFields()).thenReturn(useWhenFields);
+    }
+
+    @Test
+    public void fieldsToRenderBasic() {
+        ModelFormField a = ModelFormField.from(new ModelFormFieldBuilder().setName("A"));
+        fields.add(a);
+        ModelFormField b = ModelFormField.from(new ModelFormFieldBuilder().setName("B"));
+        fields.add(b);
+        assertThat(renderer.getUsedFields(context), hasItems(a, b));
+        assertThat(renderer.getUsedFields(context).size(), is(2));
+    }
+
+    @Test
+    public void fieldsToRenderDuplicates() {
+        ModelFormField a1 = ModelFormField.from(new ModelFormFieldBuilder().setName("A"));
+        fields.add(a1);
+        ModelFormField b = ModelFormField.from(new ModelFormFieldBuilder().setName("B"));
+        fields.add(b);
+        ModelFormField a2 = ModelFormField.from(new ModelFormFieldBuilder().setName("A"));
+        fields.add(a2);
+        assertThat(renderer.getUsedFields(context), hasItems(a1, a2, b));
+        assertThat(renderer.getUsedFields(context).size(), is(3));
+    }
+
+    @Test
+    public void fieldsToRenderBasicUseWhen() {
+        ModelFormField a1 = ModelFormField.from(new ModelFormFieldBuilder().setName("A").setUseWhen("true"));
+        fields.add(a1);
+        useWhenFields.add(a1.getName());
+        ModelFormField a2 = ModelFormField.from(new ModelFormFieldBuilder().setName("A").setUseWhen("false"));
+        fields.add(a2);
+        useWhenFields.add(a2.getName());
+        assertThat(renderer.getUsedFields(context), hasItems(a1, a2));
+        assertThat(renderer.getUsedFields(context).size(), is(2));
+    }
+
+    @Test
+    public void fieldsToRenderDuplicatesUseWhen() {
+        ModelFormField a1 = ModelFormField.from(new ModelFormFieldBuilder().setName("A").setUseWhen("true"));
+        fields.add(a1);
+        useWhenFields.add(a1.getName());
+        ModelFormField a2 = ModelFormField.from(new ModelFormFieldBuilder().setName("A").setUseWhen("false"));
+        fields.add(a2);
+        useWhenFields.add(a2.getName());
+        ModelFormField a3 = ModelFormField.from(new ModelFormFieldBuilder().setName("A").setUseWhen("true"));
+        fields.add(a3);
+        useWhenFields.add(a3.getName());
+        assertThat(renderer.getUsedFields(context), hasItems(a1, a2));
+        assertThat(renderer.getUsedFields(context).size(), is(2));
+    }
+
+}

Propchange: ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/ofbiz/widget/renderer/FormRendererTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/ofbiz/widget/renderer/FormRendererTest.java
------------------------------------------------------------------------------
    svn:keywords = Date Rev Author URL Id

Propchange: ofbiz/ofbiz-framework/trunk/framework/widget/src/test/java/org/apache/ofbiz/widget/renderer/FormRendererTest.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain