You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by pg...@apache.org on 2018/08/01 20:02:09 UTC
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/...
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