You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by bo...@apache.org on 2013/11/02 17:04:38 UTC
git commit: TAP5-2204: Change to allow "always",
"never" or "auto" as "secure" param value for Select component,
with "auto" as the default.
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 <bo...@apache.org>
Authored: Sat Nov 2 11:48:23 2013 -0400
Committer: Bob Harner <bo...@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);