Updated Branches: refs/heads/master ec0da156f -> ccb4b0fc6
WICKET-5027 no need for huge if-else in FormTester when ListMultipleChoice, CheckGroup, RadioGroup and Select have properly implemented getModelValue() Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/ccb4b0fc Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/ccb4b0fc Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/ccb4b0fc Branch: refs/heads/master Commit: ccb4b0fc6d77452d2ed7d079d0fcfc7b0b34d418 Parents: ec0da15 Author: svenmeier <[email protected]> Authored: Wed Feb 6 15:10:49 2013 +0100 Committer: svenmeier <[email protected]> Committed: Wed Feb 6 15:10:49 2013 +0100 ---------------------------------------------------------------------- .../apache/wicket/markup/html/form/CheckGroup.java | 27 ++++ .../markup/html/form/ListMultipleChoice.java | 9 +- .../apache/wicket/markup/html/form/RadioGroup.java | 23 ++++ .../org/apache/wicket/util/tester/FormTester.java | 99 ++------------- .../extensions/markup/html/form/select/Select.java | 35 +++++- 5 files changed, 97 insertions(+), 96 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/ccb4b0fc/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckGroup.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckGroup.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckGroup.java index 6e42d8b..ef6a9a8 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckGroup.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckGroup.java @@ -28,6 +28,7 @@ import org.apache.wicket.util.convert.ConversionException; import org.apache.wicket.util.lang.Generics; import org.apache.wicket.util.string.Strings; import org.apache.wicket.util.visit.IVisit; +import org.apache.wicket.util.visit.IVisitor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -102,6 +103,32 @@ public class CheckGroup<T> extends FormComponent<Collection<T>> implements IOnCh } @Override + protected String getModelValue() + { + final StringBuilder builder = new StringBuilder(); + + final Collection<T> ts = getModelObject(); + + visitChildren(Check.class, new IVisitor<Check<T>, Void>() + { + @Override + public void component(Check<T> check, IVisit<Void> visit) + { + if (ts.contains(check.getModelObject())) + { + if (builder.length() > 0) + { + builder.append(VALUE_SEPARATOR); + } + builder.append(check.getValue()); + } + } + }); + + return builder.toString(); + } + + @Override protected Collection<T> convertValue(String[] values) throws ConversionException { List<T> collection = Generics.newArrayList(); http://git-wip-us.apache.org/repos/asf/wicket/blob/ccb4b0fc/wicket-core/src/main/java/org/apache/wicket/markup/html/form/ListMultipleChoice.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/ListMultipleChoice.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/ListMultipleChoice.java index 9dc5eb9..be0d4d9 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/ListMultipleChoice.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/ListMultipleChoice.java @@ -198,18 +198,23 @@ public class ListMultipleChoice<T> extends AbstractChoice<Collection<T>, T> @Override public final String getModelValue() { - final Collection<T> selectedValues = getModelObject(); final AppendingStringBuffer buffer = new AppendingStringBuffer(); + + final Collection<T> selectedValues = getModelObject(); if (selectedValues != null) { final List<? extends T> choices = getChoices(); for (T object : selectedValues) { + if (buffer.length() > 0) + { + buffer.append(VALUE_SEPARATOR); + } int index = choices.indexOf(object); buffer.append(getChoiceRenderer().getIdValue(object, index)); - buffer.append(VALUE_SEPARATOR); } } + return buffer.toString(); } http://git-wip-us.apache.org/repos/asf/wicket/blob/ccb4b0fc/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioGroup.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioGroup.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioGroup.java index 03dc39a..b577615 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioGroup.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioGroup.java @@ -22,6 +22,7 @@ import org.apache.wicket.markup.html.WebMarkupContainer; import org.apache.wicket.model.IModel; import org.apache.wicket.util.convert.ConversionException; import org.apache.wicket.util.visit.IVisit; +import org.apache.wicket.util.visit.IVisitor; /** * Component used to connect instances of Radio components into a group. Instances of Radio have to @@ -88,6 +89,28 @@ public class RadioGroup<T> extends FormComponent<T> implements IOnChangeListener return super.getStatelessHint(); } + @Override + protected String getModelValue() + { + final StringBuilder builder = new StringBuilder(); + + final T t = getModelObject(); + + visitChildren(Radio.class, new IVisitor<Radio<T>, Void>() + { + @Override + public void component(Radio<T> radio, IVisit<Void> visit) + { + if (getModelComparator().compare(RadioGroup.this, radio.getDefaultModelObject())) + { + builder.append(radio.getValue()); + visit.stop(); + } + } + }); + + return builder.toString(); + } /** * @see org.apache.wicket.markup.html.form.FormComponent#convertValue(String[]) http://git-wip-us.apache.org/repos/asf/wicket/blob/ccb4b0fc/wicket-core/src/main/java/org/apache/wicket/util/tester/FormTester.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/util/tester/FormTester.java b/wicket-core/src/main/java/org/apache/wicket/util/tester/FormTester.java index 3ecc8ee..904da20 100644 --- a/wicket-core/src/main/java/org/apache/wicket/util/tester/FormTester.java +++ b/wicket-core/src/main/java/org/apache/wicket/util/tester/FormTester.java @@ -20,7 +20,6 @@ package org.apache.wicket.util.tester; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; -import java.util.Collection; import java.util.HashSet; import java.util.List; @@ -31,9 +30,7 @@ import org.apache.wicket.WicketRuntimeException; import org.apache.wicket.markup.html.form.AbstractSingleSelectChoice; import org.apache.wicket.markup.html.form.AbstractTextComponent; import org.apache.wicket.markup.html.form.Check; -import org.apache.wicket.markup.html.form.CheckBox; import org.apache.wicket.markup.html.form.CheckGroup; -import org.apache.wicket.markup.html.form.DropDownChoice; import org.apache.wicket.markup.html.form.Form; import org.apache.wicket.markup.html.form.FormComponent; import org.apache.wicket.markup.html.form.IChoiceRenderer; @@ -41,13 +38,13 @@ import org.apache.wicket.markup.html.form.IFormSubmittingComponent; import org.apache.wicket.markup.html.form.IOnChangeListener; import org.apache.wicket.markup.html.form.ListMultipleChoice; import org.apache.wicket.markup.html.form.Radio; -import org.apache.wicket.markup.html.form.RadioChoice; import org.apache.wicket.markup.html.form.RadioGroup; import org.apache.wicket.markup.html.form.upload.FileUploadField; import org.apache.wicket.protocol.http.mock.MockHttpServletRequest; import org.apache.wicket.util.file.File; import org.apache.wicket.util.lang.Args; import org.apache.wicket.util.string.StringValue; +import org.apache.wicket.util.string.Strings; import org.apache.wicket.util.visit.IVisit; import org.apache.wicket.util.visit.IVisitor; @@ -418,77 +415,21 @@ public class FormTester */ public static String[] getInputValue(FormComponent<?> formComponent) { - // do nothing for invisible or disabled component -- the browser would not send any - // parameter for a disabled component - if (!(formComponent.isVisibleInHierarchy() && formComponent.isEnabledInHierarchy())) + // the browser sends parameters for visible and enabled components only + if (formComponent.isVisibleInHierarchy() && formComponent.isEnabledInHierarchy()) { - return new String[] { }; - } - - // if component is text field and do not have exist value, fill - // blank String if required - if (formComponent instanceof AbstractTextComponent) - { - return new String[] { getFormComponentValue(formComponent) }; - } - else if ((formComponent instanceof DropDownChoice) || - (formComponent instanceof RadioChoice) || (formComponent instanceof CheckBox)) - { - return new String[] { getFormComponentValue(formComponent) }; - } - else if (formComponent instanceof ListMultipleChoice) - { - return getFormComponentValue(formComponent).split(FormComponent.VALUE_SEPARATOR); - } - else if (formComponent instanceof CheckGroup) - { - final Collection<?> checkGroupValues = (Collection<?>)formComponent.getDefaultModelObject(); - final List<String> result = new ArrayList<String>(); - formComponent.visitChildren(Check.class, new IVisitor<Component, Void>() - { - @Override - public void component(final Component component, final IVisit<Void> visit) - { - if (checkGroupValues.contains(component.getDefaultModelObject())) - { - result.add(getFormComponentValue((Check<?>)component)); - } - } - }); - return result.toArray(new String[result.size()]); - } - else if (formComponent instanceof RadioGroup) - { - // TODO 1.5: see if all these transformations can be factored out into - // checkgroup/radiogroup by them implementing some sort of interface { - // getValue(); } otherwise all these implementation details leak into the tester - final Object value = formComponent.getDefaultModelObject(); - String result = null; - if (value != null) + // TODO are text components the only ones with an inherent single value + if (formComponent instanceof AbstractTextComponent) { - result = formComponent.visitChildren(Radio.class, new IVisitor<Component, String>() - { - @Override - public void component(final Component component, final IVisit<String> visit) - { - if (value.equals(component.getDefaultModelObject())) - { - visit.stop(getFormComponentValue((Radio<?>)component)); - } - else - { - visit.dontGoDeeper(); - } - } - }); - } - if (result == null) - { - return new String[] { }; + return new String[] { getFormComponentValue(formComponent) }; } else { - return new String[] { result }; + String value = getFormComponentValue(formComponent); + if (!Strings.isEmpty(value)) + { + return value.split(FormComponent.VALUE_SEPARATOR); + } } } return new String[] { }; @@ -504,24 +445,6 @@ public class FormTester return val; } - private static String getFormComponentValue(final Check<?> formComponent) - { - boolean oldEscape = formComponent.getEscapeModelStrings(); - formComponent.setEscapeModelStrings(false); - String val = formComponent.getValue(); - formComponent.setEscapeModelStrings(oldEscape); - return val; - } - - private static String getFormComponentValue(final Radio<?> formComponent) - { - boolean oldEscape = formComponent.getEscapeModelStrings(); - formComponent.setEscapeModelStrings(false); - String val = formComponent.getValue(); - formComponent.setEscapeModelStrings(oldEscape); - return val; - } - /** * Retrieves the current <code>Form</code> object. * http://git-wip-us.apache.org/repos/asf/wicket/blob/ccb4b0fc/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/select/Select.java ---------------------------------------------------------------------- diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/select/Select.java b/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/select/Select.java index 1434f84..91e4de7 100644 --- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/select/Select.java +++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/select/Select.java @@ -34,8 +34,7 @@ import org.apache.wicket.util.visit.IVisitor; /** * Component that represents a <code><select></code> box. Elements are provided by one or more - * <code>SelectOptions</code> components in the hierarchy below the - * <code>Select</code> component. + * <code>SelectOptions</code> components in the hierarchy below the <code>Select</code> component. * * Advantages to the standard choice components is that the user has a lot more control over the * markup between the <select> tag and its children <option> tags: allowing for such @@ -65,10 +64,10 @@ import org.apache.wicket.util.visit.IVisitor; * SelectOptions * </p> * <p> - * <strong>Note</strong>: due to the usage of a SelectOption for each <option> the memory footprint - * of the page will grow with the number of <option>s you need. Consider using - * {@link org.apache.wicket.markup.html.form.DropDownChoice} component if it is able to fulfill your - * requirements. + * <strong>Note</strong>: due to the usage of a SelectOption for each <option> the memory + * footprint of the page will grow with the number of <option>s you need. Consider using + * {@link org.apache.wicket.markup.html.form.DropDownChoice} component if it is able to fulfill your + * requirements. * </p> * * @see SelectOption @@ -104,6 +103,30 @@ public class Select<T> extends FormComponent<T> } @Override + protected String getModelValue() + { + final StringBuilder builder = new StringBuilder(); + + visitChildren(SelectOption.class, new IVisitor<SelectOption<T>, Void>() + { + @Override + public void component(SelectOption<T> option, IVisit<Void> visit) + { + if (isSelected(option.getDefaultModel())) + { + if (builder.length() > 0) + { + builder.append(VALUE_SEPARATOR); + } + builder.append(option.getValue()); + } + } + }); + + return builder.toString(); + } + + @Override protected void convertInput() { boolean supportsMultiple = getModelObject() instanceof Collection;
