tapestry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bobhar...@apache.org
Subject git commit: TAP5-2204: Change to allow "always", "never" or "auto" as "secure" param value for Select component, with "auto" as the default.
Date Sat, 02 Nov 2013 16:04:38 GMT
Updated Branches:
  refs/heads/master ddf4ecbbb -> b77b87ed1


TAP5-2204: Change to allow "always", "never" or "auto" as "secure" param
value for Select component, with "auto" as the default.

Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/b77b87ed
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/b77b87ed
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/b77b87ed

Branch: refs/heads/master
Commit: b77b87ed1f5585e6ec481ccb60cd29a985291539
Parents: ddf4ecb
Author: Bob Harner <bobharner@apache.org>
Authored: Sat Nov 2 11:48:23 2013 -0400
Committer: Bob Harner <bobharner@apache.org>
Committed: Sat Nov 2 11:53:48 2013 -0400

----------------------------------------------------------------------
 .../tapestry5/ComponentParameterConstants.java  |  15 ++-
 .../tapestry5/corelib/components/Select.java    |  29 +++--
 .../tapestry5/corelib/data/SecureOption.java    |  45 +++++++
 .../tapestry5/modules/TapestryModule.java       |  10 +-
 .../corelib/components/SelectTest.java          | 125 +++++++++++++------
 5 files changed, 173 insertions(+), 51 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b77b87ed/tapestry-core/src/main/java/org/apache/tapestry5/ComponentParameterConstants.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/ComponentParameterConstants.java
b/tapestry-core/src/main/java/org/apache/tapestry5/ComponentParameterConstants.java
index 0c4a561..151f523 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/ComponentParameterConstants.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/ComponentParameterConstants.java
@@ -72,24 +72,33 @@ public class ComponentParameterConstants
 
     /**
      * The default position where to insert content into {@link org.apache.tapestry5.corelib.components.Form}.
-     * Default to "above".
+     * Defaults to "above".
      */
     public static final String FORMINJECTOR_INSERT_POSITION = "tapestry.components.forminjector_insert_position";
 
     /**
      * The default name for a JS function to use to show the injected content by
      * {@link org.apache.tapestry5.corelib.components.FormInjector}.
-     * Default to "highlight".
+     * Defaults to "highlight".
      */
     public static final String FORMINJECTOR_SHOW_FUNCTION = "tapestry.components.forminjector_show_function";
 
     /**
      * The default size of rows to display in a {@link org.apache.tapestry5.corelib.components.Palette}
-     * component. Default to 10.
+     * component. Defaults to 10.
      */
     public static final String PALETTE_ROWS_SIZE = "tapestry.components.palette_rows_size";
 
     /**
+     * The default for whether components that use a SelectModel (e.g.
+     * {@link org.apache.tapestry5.corelib.components.Select}) enforce
+     * that the submitted value is one of the values in the SelectModel
+     * 
+     * @since 5.4
+     */
+    public static final String VALIDATE_WITH_MODEL = "tapestry.components.validate_with_model";
+
+    /**
      * The default name of a JS function attached to Tapestry.ElementEffect object to use
for the initial
      * visualization of a {@link org.apache.tapestry5.corelib.components.Zone}.
      * Defaults to "show"

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b77b87ed/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Select.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Select.java
b/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Select.java
index d5e3ff2..df55fcb 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Select.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Select.java
@@ -18,6 +18,7 @@ import org.apache.tapestry5.*;
 import org.apache.tapestry5.annotations.*;
 import org.apache.tapestry5.corelib.base.AbstractField;
 import org.apache.tapestry5.corelib.data.BlankOption;
+import org.apache.tapestry5.corelib.data.SecureOption;
 import org.apache.tapestry5.corelib.mixins.RenderDisabled;
 import org.apache.tapestry5.internal.TapestryInternalUtils;
 import org.apache.tapestry5.internal.util.CaptureResultCallback;
@@ -39,7 +40,7 @@ import java.util.List;
  * A core part of this component is the {@link ValueEncoder} (the encoder parameter) that
is used to convert between
  * server-side values and unique client-side strings. In some cases, a {@link ValueEncoder}
can be generated automatically from
  * the type of the value parameter. The {@link ValueEncoderSource} service provides an encoder
in these situations; it
- * can be overriden by binding the encoder parameter, or extended by contributing a {@link
ValueEncoderFactory} into the
+ * can be overridden by binding the encoder parameter, or extended by contributing a {@link
ValueEncoderFactory} into the
  * service's configuration.
  *
  * @tapestrydoc
@@ -78,14 +79,19 @@ public class Select extends AbstractField
     private ValueEncoder encoder;
 
     /**
-     * If true (the default), then the submitted value must be present in the {@link SelectModel},
or a
-     * validation errors occurs. If false, then the Tapestry 5.3 (and earlier) behavior is
allowed. The insecure
-     * behavior could theoretically allow a selection to be made that was not presented to
the user.
+     * Controls whether the submitted value is validated to be one of the values in
+     * the {@link SelectModel}. If "never", then no such validation is performed,
+     * theoretically allowing a selection to be made that was not presented to
+     * the user.  Note that an "always" value here requires the SelectModel to
+     * still exist (or be created again) when the form is submitted, whereas a
+     * "never" value does not.  Defaults to "auto", which causes the validation
+     * to occur only if the SelectModel is present (not null) when the form is
+     * submitted.
      *
      * @since 5.4
      */
-    @Parameter(value = "true")
-    private boolean secure;
+    @Parameter(value = BindingConstants.SYMBOL + ":" + ComponentParameterConstants.VALIDATE_WITH_MODEL)
+    private SecureOption secure;
 
     /**
      * The model used to identify the option groups and options to be presented to the user.
This can be generated
@@ -254,11 +260,20 @@ public class Select extends AbstractField
             return null;
         }
 
-        if (!secure)
+        // can we skip the check for the value being in the model?
+        if (secure == SecureOption.NEVER || (secure == SecureOption.AUTO && model
== null))
         {
             return encoder.toValue(submittedValue);
         }
 
+        // for entity types the SelectModel may be unintentionally null when the form is
submitted
+        if (model == null)
+        {
+            throw new ValidationException("Model is null when validating submitted option."
+
+                    " To fix: persist the SeletModel or recreate it upon form submission,"
+
+                    " or change the 'secure' parameter.");
+        }
+
         return findValueInModel(submittedValue);
     }
 

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b77b87ed/tapestry-core/src/main/java/org/apache/tapestry5/corelib/data/SecureOption.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/corelib/data/SecureOption.java
b/tapestry-core/src/main/java/org/apache/tapestry5/corelib/data/SecureOption.java
new file mode 100644
index 0000000..b730324
--- /dev/null
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/corelib/data/SecureOption.java
@@ -0,0 +1,45 @@
+// Copyright 2008 The Apache Software Foundation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.apache.tapestry5.corelib.data;
+
+/**
+ * Possible values of the "secure" parameter for components that use a
+ * {@link SelectModel} (such as {@link Select}), to control whether the submitted
+ * value must be one of the values in the SelectModel.
+ */
+public enum SecureOption
+{
+    /**
+     * Always check that the submitted value is found in the SelectModel (and
+     * record a validation error if the SelectModel is not provided (null).
+     */
+    ALWAYS,
+
+    /**
+     * Never check that submitted value is found in the SelectModel. It is left
+     * to the user of the component to validate the submitted value.
+     */
+    NEVER,
+
+    /**
+     * The default: check that the submitted value is found in the SelectModel,
+     * unless the SelectModel is not provided (null) at the time of submission.
+     * Since SelectModels are automatically provided for enums but not custom
+     * classes, this is the most useful option in cases where you don't want to
+     * persist a custom SelectModel across a form submission or recreate it
+     * when the form is submitted). 
+     */
+    AUTO;
+}

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b77b87ed/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
index fd578b7..e5b7dd9 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
@@ -21,6 +21,7 @@ import org.apache.tapestry5.annotations.*;
 import org.apache.tapestry5.annotations.ContentType;
 import org.apache.tapestry5.beaneditor.DataTypeConstants;
 import org.apache.tapestry5.beaneditor.Validate;
+import org.apache.tapestry5.corelib.data.SecureOption;
 import org.apache.tapestry5.grid.GridConstants;
 import org.apache.tapestry5.grid.GridDataSource;
 import org.apache.tapestry5.internal.*;
@@ -2081,7 +2082,7 @@ public final class TapestryModule
 
         configuration.add(SymbolConstants.APPLICATION_FOLDER, "");
 
-        // Grid component parameters defaults
+        // Grid component parameter defaults
         configuration.add(ComponentParameterConstants.GRID_ROWS_PER_PAGE, GridConstants.ROWS_PER_PAGE);
         configuration.add(ComponentParameterConstants.GRID_PAGER_POSITION, GridConstants.PAGER_POSITION);
         configuration.add(ComponentParameterConstants.GRID_EMPTY_BLOCK, GridConstants.EMPTY_BLOCK);
@@ -2091,13 +2092,16 @@ public final class TapestryModule
         configuration.add(ComponentParameterConstants.GRIDCOLUMNS_ASCENDING_ASSET, GridConstants.COLUMNS_ASCENDING);
         configuration.add(ComponentParameterConstants.GRIDCOLUMNS_DESCENDING_ASSET, GridConstants.COLUMNS_DESCENDING);
 
-        // FormInjector component parameters defaults
+        // FormInjector component parameter defaults
         configuration.add(ComponentParameterConstants.FORMINJECTOR_INSERT_POSITION, "above");
         configuration.add(ComponentParameterConstants.FORMINJECTOR_SHOW_FUNCTION, "highlight");
 
-        // Palette component parameters defaults
+        // Palette component parameter defaults
         configuration.add(ComponentParameterConstants.PALETTE_ROWS_SIZE, 10);
 
+        // Defaults for components that use a SelectModel
+        configuration.add(ComponentParameterConstants.VALIDATE_WITH_MODEL, SecureOption.ALWAYS);
+
         // Zone component parameters defaults
         configuration.add(ComponentParameterConstants.ZONE_SHOW_METHOD, "show");
         configuration.add(ComponentParameterConstants.ZONE_UPDATE_METHOD, "highlight");

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b77b87ed/tapestry-core/src/test/java/org/apache/tapestry5/corelib/components/SelectTest.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/corelib/components/SelectTest.java
b/tapestry-core/src/test/java/org/apache/tapestry5/corelib/components/SelectTest.java
index 8766775..9818ae4 100644
--- a/tapestry-core/src/test/java/org/apache/tapestry5/corelib/components/SelectTest.java
+++ b/tapestry-core/src/test/java/org/apache/tapestry5/corelib/components/SelectTest.java
@@ -16,6 +16,7 @@ package org.apache.tapestry5.corelib.components;
 
 import org.apache.tapestry5.*;
 import org.apache.tapestry5.corelib.data.BlankOption;
+import org.apache.tapestry5.corelib.data.SecureOption;
 import org.apache.tapestry5.dom.XMLMarkupModel;
 import org.apache.tapestry5.internal.OptionGroupModelImpl;
 import org.apache.tapestry5.internal.OptionModelImpl;
@@ -386,48 +387,77 @@ public class SelectTest extends InternalBaseTestCase
         WINDOWS, MAC, LINUX;
     }
 
+    /**
+     * TAP5-2204: When secure parameter is "always" there should be no
+     * validation error if the model is NOT null.
+     */
     @Test
-    public void submitted_option_found_when_secure() throws ValidationException
+    public void submitted_option_found_when_secure_always() throws ValidationException
     {
+        checkSubmittedOption(true, SecureOption.ALWAYS, null);
+    }
 
-        ValueEncoder<Platform> encoder = getService(ValueEncoderSource.class).getValueEncoder(Platform.class);
-
-        ValidationTracker tracker = mockValidationTracker();
-        Request request = mockRequest();
-        Messages messages = mockMessages();
-        FieldValidationSupport fvs = mockFieldValidationSupport();
-
-        expect(request.getParameter("xyz")).andReturn("MAC");
-
-        expect(messages.contains(EasyMock.anyObject(String.class))).andReturn(false).anyTimes();
-
-        Select select = new Select();
-
-        tracker.recordInput(select, "MAC");
-
-        fvs.validate(Platform.MAC, null, null);
-
-        replay();
-
-        SelectModel model = new EnumSelectModel(Platform.class, messages);
-
-        set(select, "encoder", encoder);
-        set(select, "model", model);
-        set(select, "request", request);
-        set(select, "secure", true);
-        set(select, "beanValidationDisabled", true); // Disable BeanValidationContextSupport
-        set(select, "tracker", tracker);
-        set(select, "fieldValidationSupport", fvs);
+    /**
+     * TAP5-2204: When secure parameter is "always" there should be a
+     * validation error if the model is null.
+     */
+    @Test
+    public void submitted_option_not_found_when_secure_always() throws ValidationException
+    {
+        checkSubmittedOption(false, SecureOption.ALWAYS, "is null when validating");
+    }
 
-        select.processSubmission("xyz");
+    /**
+     * TAP5-2204: When secure parameter is "never" there should be no
+     * validation error if the model is NOT null.
+     */
+    @Test
+    public void submitted_option_ok_when_secure_never() throws ValidationException
+    {
+        checkSubmittedOption(true, SecureOption.NEVER, null);
+    }
 
-        verify();
+    /**
+     * TAP5-2204: When secure parameter is "never" there should be no
+     * validation error if the model is null.
+     */
+    @Test
+    public void submitted_option_ok_when_secure_never_no_model() throws ValidationException
+    {
+        checkSubmittedOption(false, SecureOption.NEVER, null);
+    }
 
-        assertEquals(get(select, "value"), Platform.MAC);
+    /**
+     * TAP5-2204: When secure parameter is "auto" there should be no
+     * validation error if the model is NOT null.
+     */
+    @Test
+    public void submitted_option_found_when_secure_auto() throws ValidationException
+    {
+        checkSubmittedOption(true, SecureOption.AUTO, null);
     }
 
+    /**
+     * TAP5-2204: When secure parameter is "auto" there should be no
+     * validation error if the model is null.
+     */
     @Test
-    public void submitted_option_not_found_when_secure() throws ValidationException
+    public void submitted_option_ok_when_secure_auto() throws ValidationException
+    {
+        checkSubmittedOption(false, SecureOption.AUTO, null);
+    }
+
+    /**
+     * Utility for testing the "secure" option with various values and model
+     * states. This avoids a lot of redundant test setup code.
+     * 
+     * @param withModel whether there should be a model to test against
+     * @param secureOption which "secure" option to test
+     * @param expectedError the expected error message, nor null if no error
+     * @throws ValidationException
+     */
+    private void checkSubmittedOption(boolean withModel, SecureOption secureOption,
+            String expectedError) throws ValidationException
     {
 
         ValueEncoder<Platform> encoder = getService(ValueEncoderSource.class).getValueEncoder(Platform.class);
@@ -435,6 +465,7 @@ public class SelectTest extends InternalBaseTestCase
         ValidationTracker tracker = mockValidationTracker();
         Request request = mockRequest();
         Messages messages = mockMessages();
+        FieldValidationSupport fvs = mockFieldValidationSupport();
 
         expect(request.getParameter("xyz")).andReturn("MAC");
 
@@ -444,21 +475,39 @@ public class SelectTest extends InternalBaseTestCase
 
         tracker.recordInput(select, "MAC");
 
-        tracker.recordError(EasyMock.eq(select), EasyMock.contains("option is not listed"));
+        // when not failing we will expect to call the fvs.validate method
+        if (expectedError == null)
+        {
+            fvs.validate(Platform.MAC, null, null);
+        }
+        else
+        {
+            tracker.recordError(EasyMock.eq(select), EasyMock.contains(expectedError));
+        }
 
         replay();
-
-        SelectModel model = new EnumSelectModel(Platform.class, messages, new Platform[]{Platform.WINDOWS,
Platform.LINUX});
+        
+        SelectModel model = null;
+        if (withModel)
+        {
+            model = new EnumSelectModel(Platform.class, messages);
+        }
 
         set(select, "encoder", encoder);
         set(select, "model", model);
         set(select, "request", request);
-        set(select, "secure", true);
+        set(select, "secure", secureOption);
         set(select, "beanValidationDisabled", true); // Disable BeanValidationContextSupport
         set(select, "tracker", tracker);
+        set(select, "fieldValidationSupport", fvs);
 
         select.processSubmission("xyz");
 
+        if (expectedError == null)
+        {
+            assertEquals(get(select, "value"), Platform.MAC);
+        }
+
         verify();
     }
 
@@ -492,7 +541,7 @@ public class SelectTest extends InternalBaseTestCase
         set(select, "encoder", encoder);
         set(select, "model", model);
         set(select, "request", request);
-        set(select, "secure", true);
+        set(select, "secure", SecureOption.ALWAYS);
         set(select, "beanValidationDisabled", true); // Disable BeanValidationContextSupport
         set(select, "tracker", tracker);
         set(select, "fieldValidationSupport", fvs);


Mime
View raw message