Merge pull request #3636 from stevie-mayhew/feature/validator

FEATURE: add validation to formfield subclasses

docs/en/changelogs/3.2.0.md

@@ -16,6 +16,7 @@
  * `Convert::html2raw` no longer wraps text by default and can decode single quotes.
  * `Mailer` no longer calls `xml2raw` on all email subject line, and now must be passed in via plain text.
  * `ErrorControlChain` now supports reload on exceptions
+ * `FormField::validate` now requires an instance of `Validator`
 
 #### Deprecated classes/methods removed
 
@@ -545,3 +546,4 @@ coding conventions. E.g. `DB::requireTable` is now `DB::require_table`
  * create_table_options now uses constants as API specific filters rather than strings.
    This is in order to promote better referencing of elements across the codebase.
    See `FulltextSearchable->enable` for example.
+ * `FormField` subclasses must now use `validate(Validator $validator)` as the interface has changed for this function

forms/CheckboxSetField.php

@@ -312,4 +312,46 @@ class CheckboxSetField extends OptionsetField {
 		return FormField::ExtraOptions();
 	}
 
+	/**
+	 * Validate this field
+	 *
+	 * @param Validator $validator
+	 * @return bool
+	 */
+	public function validate(Validator $validator) {
+		$values = $this->value;
+		if (!$values) {
+			return true;
+		}
+		$sourceArray = $this->getSourceAsArray();
+		if (is_array($values)) {
+			if (!array_intersect_key($sourceArray, $values)) {
+				$validator->validationError(
+					$this->name,
+					_t(
+						'CheckboxSetField.SOURCE_VALIDATION',
+						"Please select a value within the list provided. '{value}' is not a valid option",
+						array('value' => implode(' and ', array_diff($sourceArray, $values)))
+					),
+					"validation"
+				);
+				return false;
+			}
+		} else {
+			if (!in_array($this->value, $sourceArray)) {
+				$validator->validationError(
+					$this->name,
+					_t(
+						'CheckboxSetField.SOURCE_VALIDATION',
+						"Please select a value within the list provided. '{value}' is not a valid option",
+						array('value' => $this->value)
+					),
+					"validation"
+				);
+				return false;
+			}
+		}
+		return true;
+	}
+
 }

forms/CompositeField.php

@@ -341,7 +341,7 @@ class CompositeField extends FormField {
 					// Clear an internal cache
 					$this->sequentialSet = null;
 
-					// A true results indicates that the field was foudn
+					// A true results indicates that the field was found
 					return true;
 				}
 			}
@@ -358,7 +358,13 @@ class CompositeField extends FormField {
 		return $result;
 	}
 
-	public function validate($validator) {
+	/**
+	 * Validate this field
+	 *
+	 * @param Validator $validator
+	 * @return bool
+	 */
+	public function validate(Validator $validator) {
 		$valid = true;
 		foreach($this->children as $idx => $child){
 			$valid = ($child && $child->validate($validator) && $valid);

forms/ConfirmedPasswordField.php

@@ -314,11 +314,12 @@ class ConfirmedPasswordField extends FormField {
 	}
 
 	/**
-	 * @param Validator $validator
+	 * Validate this field
 	 *
-	 * @return boolean
+	 * @param Validator $validator
+	 * @return bool
 	 */
-	public function validate($validator) {
+	public function validate(Validator $validator) {
 		$name = $this->name;
 
 		// if field isn't visible, don't validate

forms/CreditCardField.php

@@ -58,7 +58,7 @@ class CreditCardField extends TextField {
 		else return $this->value;
 	}
 
-	public function validate($validator){
+	public function validate(Validator $validator){
 		// If the field is empty then don't return an invalidation message
 		if(!trim(implode("", $this->value))) return true;
 

forms/CurrencyField.php

@@ -43,7 +43,7 @@ class CurrencyField extends TextField {
 		return $this->castedCopy('CurrencyField_Readonly');
 	}
 
-	public function validate($validator) {
+	public function validate(Validator $validator) {
 		if(!empty ($this->value)
 				&& !preg_match('/^\s*(\-?\$?|\$\-?)?(\d{1,3}(\,\d{3})*|(\d+))(\.\d{2})?\s*$/', $this->value)) {
 

forms/DateField.php

@@ -336,7 +336,7 @@ class DateField extends TextField {
 	/**
 	 * @return Boolean
 	 */
-	public function validate($validator) {
+	public function validate(Validator $validator) {
 		$valid = true;
 
 		// Don't validate empty fields
@@ -496,10 +496,6 @@ class DateField_Disabled extends DateField {
 	public function Type() {
 		return "date_disabled readonly";
 	}
-
-	public function validate($validator) {
-		return true;
-	}
 }
 
 /**

forms/DatetimeField.php

@@ -323,7 +323,7 @@ class DatetimeField extends FormField {
 		}
 	}
 
-	public function validate($validator) {
+	public function validate(Validator $validator) {
 		$dateValid = $this->dateField->validate($validator);
 		$timeValid = $this->timeField->validate($validator);
 
@@ -391,7 +391,4 @@ class DatetimeField_Readonly extends DatetimeField {
 		return "<span class=\"readonly\" id=\"" . $this->id() . "\">$val</span>";
 	}
 
-	public function validate($validator) {
-		return true;
-	}
 }

forms/DropdownField.php

@@ -84,7 +84,7 @@
 class DropdownField extends FormField {
 
 	/**
-	 * @var Array $source Associative or numeric array of all dropdown items,
+	 * @var array|ArrayAccess $source Associative or numeric array of all dropdown items,
 	 * with array key as the submitted field value, and the array value as a
 	 * natural language description shown in the interface element.
 	 */
@@ -119,7 +119,7 @@ class DropdownField extends FormField {
 	/**
 	 * @param string $name The field name
 	 * @param string $title The field title
-	 * @param array $source An map of the dropdown items
+	 * @param array|ArrayAccess $source A map of the dropdown items
 	 * @param string $value The current value
 	 * @param Form $form The parent form
 	 */
@@ -223,14 +223,14 @@ class DropdownField extends FormField {
 	/**
 	 * Gets the source array including any empty default values.
 	 *
-	 * @return array
+	 * @return array|ArrayAccess
 	 */
 	public function getSource() {
 		return $this->source;
 	}
 
 	/**
-	 * @param array $source
+	 * @param array|ArrayAccess $source
 	 */
 	public function setSource($source) {
 		$this->source = $source;
@@ -286,4 +286,63 @@ class DropdownField extends FormField {
 
 		return $field;
 	}
+
+	/**
+	 * Get the source of this field as an array
+	 *
+	 * @return array
+	 */
+	public function getSourceAsArray()
+	{
+		$source = $this->getSource();
+		if (is_array($source)) {
+			return $source;
+		} else {
+			$sourceArray = array();
+			foreach ($source as $key => $value) {
+				$sourceArray[$key] = $value;
+			}
+		}
+		return $sourceArray;
+	}
+
+	/**
+	 * Validate this field
+	 *
+	 * @param Validator $validator
+	 * @return bool
+	 */
+	public function validate(Validator $validator) {
+		$source = $this->getSourceAsArray();
+		if (!array_key_exists($this->value, $source)) {
+			if ($this->getHasEmptyDefault() && !$this->value) {
+				return true;
+			}
+			$validator->validationError(
+				$this->name,
+				_t(
+					'DropdownField.SOURCE_VALIDATION',
+					"Please select a value within the list provided. {value} is not a valid option",
+					array('value' => $this->value)
+				),
+				"validation"
+			);
+			return false;
+		}
+		return true;
+	}
+
+	/**
+	 * Returns another instance of this field, but "cast" to a different class.
+	 *
+	 * @see FormField::castedCopy()
+	 *
+	 * @param String $classOrCopy
+	 * @return FormField
+	 */
+	public function castedCopy($classOrCopy) {
+		$field = parent::castedCopy($classOrCopy);
+		$field->setHasEmptyDefault($this->getHasEmptyDefault());
+		return $field;
+	}
 }

forms/EmailField.php

@@ -30,7 +30,7 @@ class EmailField extends TextField {
 	 * @param Validator $validator
 	 * @return String
 	 */
-	public function validate($validator) {
+	public function validate(Validator $validator) {
 		$this->value = trim($this->value);
 
 		$pcrePattern = '^[a-z0-9!#$%&\'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&\'*+/=?^_`{|}~-]+)*'

forms/FileField.php

@@ -181,7 +181,7 @@ class FileField extends FormField {
 		return ($this->folderName !== false) ? $this->folderName : Config::inst()->get('Upload', 'uploads_folder');
 	}
 
-	public function validate($validator) {
+	public function validate(Validator $validator) {
 		if(!isset($_FILES[$this->name])) return true;
 
 		$tmpFile = $_FILES[$this->name];

forms/FormField.php

@@ -15,7 +15,8 @@
  * format of a date or currency field. Define {@link saveInto()} to totally
  * customise saving. For example, data might be saved to the filesystem instead
  * of the data record, or saved to a component of the data record instead of
- * the data record itself.
+ * the data record itself. Define {@link validate()} to validate the form field
+ * and ensure that the content provided is valid.
  *
  * @package forms
  * @subpackage core
@@ -894,14 +895,13 @@ class FormField extends RequestHandler {
 			}
 
 	/**
-	 * Abstract method each {@link FormField} subclass must implement,
-	 * determines whether the field is valid or not based on the value.
-	 * @todo Make this abstract.
+	 * Validation method each {@link FormField} subclass should implement,
+	 * determining whether the field is valid or not based on the value.
 	 *
 	 * @param Validator
 	 * @return boolean
 	 */
-	public function validate($validator) {
+	public function validate(Validator $validator) {
 		return true;
 	}
 

forms/HiddenField.php

@@ -31,4 +31,5 @@ class HiddenField extends FormField {
 	function SmallFieldHolder($properties = array()) {
 		return $this->FieldHolder($properties);
 	}
+
 }

forms/ListboxField.php

@@ -203,7 +203,7 @@ class ListboxField extends DropdownField {
 	}
 
 	/**
-	 * Load a value into this CheckboxSetField
+	 * Load a value into this ListboxField
 	 */
 	public function setValue($val, $obj = null) {
 		// If we're not passed a value directly,
@@ -281,4 +281,45 @@ class ListboxField extends DropdownField {
 		return $this->defaultItems;
 	}
 
+	/**
+	 * Validate this field
+	 *
+	 * @param Validator $validator
+	 * @return bool
+	 */
+	public function validate(Validator $validator) {
+		$values = $this->value;
+		if (!$values) {
+			return true;
+		}
+		$source = $this->getSourceAsArray();
+		if (is_array($values)) {
+			if (!array_intersect_key($source,array_flip($values))) {
+				$validator->validationError(
+					$this->name,
+					_t(
+						"Please select a value within the list provided. {value} is not a valid option",
+						array('value' => $this->value)
+					),
+					"validation"
+				);
+				return false;
+			}
+		} else {
+			if (!array_key_exists($this->value, $source)) {
+				$validator->validationError(
+					$this->name,
+					_t(
+						'ListboxField.SOURCE_VALIDATION',
+						"Please select a value within the list provided. %s is not a valid option",
+						array('value' => $this->value)
+					),
+					"validation"
+				);
+				return false;
+			}
+		}
+		return true;
+	}
+
 }

forms/MemberDatetimeOptionsetField.php

@@ -119,7 +119,13 @@ class MemberDatetimeOptionsetField extends OptionsetField {
 		}
 	}
 
-	public function validate($validator) {
+	/**
+	 * Validate this field
+	 *
+	 * @param Validator $validator
+	 * @return bool
+	 */
+	public function validate(Validator $validator) {
 		$value = isset($_POST[$this->name . '_custom']) ? $_POST[$this->name . '_custom'] : null;
 		if(!$value) return true; // no custom value, don't validate
 

forms/MoneyField.php

@@ -177,4 +177,14 @@ class MoneyField extends FormField {
 	public function getLocale() {
 		return $this->_locale;
 	}
+
+	/**
+	 * Validate this field
+	 *
+	 * @param Validator $validator
+	 * @return bool
+	 */
+	public function validate(Validator $validator) {
+		return !(is_null($this->fieldAmount) || is_null($this->fieldCurrency));
+	}
 }

forms/NullableField.php

@@ -126,4 +126,5 @@ class NullableField extends FormField {
 		$result .= (is_null($this->value)) ? "<<null>>" : $this->value;
 		return result;
 	}
+
 }

forms/NumericField.php

@@ -20,8 +20,14 @@ class NumericField extends TextField {
 		));
 	}
 
-	public function validate($validator) {
-		if(!$this->value && !$validator->fieldIsRequired($this->name)) {
+	/**
+	 * Validate this field
+	 *
+	 * @param Validator $validator
+	 * @return bool
+	 */
+	public function validate(Validator $validator) {
+		if(!$this->value) {
 			return true;
 		}
 

forms/OptionsetField.php

@@ -120,5 +120,4 @@ class OptionsetField extends DropdownField {
 	public function ExtraOptions() {
 		return new ArrayList();
 	}
-
 }

forms/PhoneNumberField.php

@@ -127,9 +127,14 @@ class PhoneNumberField extends FormField {
 	}
 
 	/**
+	 * Validate this field
+	 *
 	 * @todo Very basic validation at the moment
+	 *
+	 * @param Validator $validator
+	 * @return bool
 	 */
-	public function validate($validator){
+	public function validate(Validator $validator){
 		$valid = preg_match(
 			'/^[0-9\+\-\(\)\s\#]*$/',
 			$this->joinPhoneNumber($this->value)

forms/ReadonlyField.php

@@ -66,4 +66,5 @@ class ReadonlyField extends FormField {
 	public function Type() {
 		return 'readonly';
 	}
+
 }

forms/TimeField.php

@@ -160,10 +160,12 @@ class TimeField extends TextField {
 	}
 
 	/**
-	 * @return Boolean
+	 * Validate this field
+	 *
+	 * @param Validator $validator
+	 * @return bool
 	 */
-	public function validate($validator) {
-		$valid = true;
+	public function validate(Validator $validator) {
 
 		// Don't validate empty fields
 		if(empty($this->value)) return true;
@@ -260,8 +262,4 @@ class TimeField_Readonly extends TimeField {
 
 		return "<span class=\"readonly\" id=\"" . $this->id() . "\">$val</span>";
 	}
-
-	public function validate($validator) {
-		return true;
-	}
 }

forms/UploadField.php

@@ -985,9 +985,7 @@ class UploadField extends FileField {
 	 * @param Validator $validator
 	 * @return boolean
 	 */
-	public function validate($validator) {
-
-		// @todo Test compatibility with RequiredFields
+	public function validate(Validator $validator) {
 		$name = $this->getName();
 		$files = $this->getItems();
 

forms/Validator.php

@@ -42,10 +42,10 @@ abstract class Validator extends Object {
 	/**
 	 * Callback to register an error on a field (Called from implementations of {@link FormField::validate})
 	 *
-	 * @param $fieldName name of the field
-	 * @param $message error message to display
-	 * @param $messageType optional parameter, gets loaded into the HTML class attribute in the rendered output.
-	 *                              See {@link getErrors()} for details.
+	 * @param string $fieldName name of the field
+	 * @param string $message error message to display
+	 * @param string $messageType optional parameter, gets loaded into the HTML class attribute in the rendered output.
+	 * See {@link getErrors()} for details.
 	 */
 	public function validationError($fieldName, $message, $messageType='') {
 		$this->errors[] = array(

lang/en.yml

@@ -83,6 +83,8 @@ en:
   CheckboxField:
     NOANSWER: No
     YESANSWER: Yes
+  CheckboxFieldSetField:
+    SOURCE_VALIDATION: 'Please select a value within the list provided. {value} is not a valid option'
   ConfirmedPasswordField:
     ATLEAST: 'Passwords must be at least {min} characters long.'
     BETWEEN: 'Passwords must be {min} to {max} characters long.'
@@ -129,6 +131,7 @@ en:
   DropdownField:
     CHOOSE: (Choose)
     CHOOSESEARCH: '(Choose or Search)'
+    SOURCE_VALIDATION: 'Please select a value within the list provided. {value} is not a valid option'
   EmailField:
     VALIDATION: 'Please enter an email address'
   Enum:
@@ -323,6 +326,8 @@ en:
   LeftAndMain_Menu_ss:
     Hello: Hi
     LOGOUT: 'Log out'
+  ListboxField:
+    SOURCE_VALIDATION: 'Please select a value within the list provided. {value} is not a valid option'
   LoginAttempt:
     Email: 'Email Address'
     IP: 'IP Address'

tests/control/RequestHandlingTest.php

@@ -358,7 +358,7 @@ class RequestHandlingTest_Controller extends Controller implements TestOnly {
 	}
 }
 
-class RequestHandlingTest_FormActionController extends Controller {
+class RequestHandlingTest_FormActionController extends Controller implements TestOnly {
 
 	protected $template = 'BlankPage';
 
@@ -417,7 +417,7 @@ class RequestHandlingTest_FormActionController extends Controller {
 /**
  * Simple extension for the test controller
  */
-class RequestHandlingTest_ControllerExtension extends Extension {
+class RequestHandlingTest_ControllerExtension extends Extension implements TestOnly {
 
 	public static $called_error = false;
 
@@ -490,7 +490,7 @@ class RequestHandlingTest_AllowedController extends Controller implements TestOn
 /**
  * Simple extension for the test controller - with allowed_actions define
  */
-class RequestHandlingTest_AllowedControllerExtension extends Extension {
+class RequestHandlingTest_AllowedControllerExtension extends Extension implements TestOnly {
 	private static $allowed_actions = array(
 		'otherExtendedMethod'
 	);
@@ -500,7 +500,7 @@ class RequestHandlingTest_AllowedControllerExtension extends Extension {
 	}
 }
 
-class RequestHandlingTest_ControllerFailover extends ViewableData {
+class RequestHandlingTest_ControllerFailover extends ViewableData implements TestOnly {
 	public function failoverMethod() {
 		return "failoverMethod";
 	}
@@ -509,7 +509,7 @@ class RequestHandlingTest_ControllerFailover extends ViewableData {
 /**
  * Form for the test
  */
-class RequestHandlingTest_Form extends Form {
+class RequestHandlingTest_Form extends Form implements TestOnly {
 	private static $url_handlers = array(
 		'fields/$FieldName' => 'handleField',
 		"POST " => "handleSubmission",
@@ -552,7 +552,7 @@ class RequestHandlingTest_ControllerFormWithAllowedActions extends Controller im
 	}
 }
 
-class RequestHandlingTest_FormWithAllowedActions extends Form {
+class RequestHandlingTest_FormWithAllowedActions extends Form implements TestOnly {
 
 	private static $allowed_actions = array(
 		'allowedformaction' => 1,
@@ -571,7 +571,7 @@ class RequestHandlingTest_FormWithAllowedActions extends Form {
 /**
  * Form field for the test
  */
-class RequestHandlingTest_FormField extends FormField {
+class RequestHandlingTest_FormField extends FormField implements TestOnly {
 	private static $url_handlers = array(
 		"POST " => "handleInPlaceEdit",
 		'' => 'handleField',
@@ -638,7 +638,7 @@ class RequestHandlingFieldTest_Controller extends Controller implements TestOnly
 /**
  * Form field for the test
  */
-class RequestHandlingTest_HandlingField extends FormField {
+class RequestHandlingTest_HandlingField extends FormField implements TestOnly {
 
 	private static $allowed_actions = array(
 		'actionOnField'

tests/forms/CheckboxFieldTest.php

@@ -139,6 +139,34 @@ class CheckboxFieldTest extends SapphireTest {
 
 	}
 
+	public function testValidation() {
+		$field = CheckboxField::create('Test', 'Testing');
+		$validator = new RequiredFields();
+		$field->setValue(1);
+		$this->assertTrue(
+			$field->validate($validator),
+			'Field correctly validates integers as allowed'
+		);
+		//string value should validate
+		$field->setValue("test");
+		$this->assertTrue(
+			$field->validate($validator),
+			'Field correctly validates words as allowed'
+		);
+		//empty string should validate
+		$field->setValue('');
+		$this->assertTrue(
+			$field->validate($validator),
+			'Field correctly validates empty strings as allowed'
+		);
+		//null should validate
+		$field->setValue(null);
+		$this->assertTrue(
+			$field->validate($validator),
+			'Field correct validates null as allowed'
+		);
+	}
+
 }
 class CheckboxFieldTest_Article extends DataObject implements TestOnly {
 

tests/forms/CheckboxSetFieldTest.php

@@ -144,6 +144,68 @@ class CheckboxSetFieldTest extends SapphireTest {
 		$this->assertEquals('Test,Another', $dbValue);
 	}
 
+	public function testValidationWithArray() {
+		//test with array input
+		$field = CheckboxSetField::create('Test', 'Testing', array(
+			"One" => "One",
+			"Two" => "Two",
+			"Three" => "Three"
+		));
+		$validator = new RequiredFields();
+		$field->setValue(array("One" => "One", "Two" => "Two"));
+		$this->assertTrue(
+			$field->validate($validator),
+			'Field validates values within source array'
+		);
+		//non valid value should fail
+		$field->setValue(array("Four" => "Four"));
+		$this->assertFalse(
+			$field->validate($validator),
+			'Field does not validate values outside of source array'
+		);
+		//non valid value included with valid options should succeed
+		$field->setValue(array("One" => "One", "Two" => "Two", "Four" => "Four"));
+		$this->assertTrue(
+			$field->validate($validator),
+			'Field validates when presented with mixed valid and invalid values'
+		);
+	}
+
+	public function testValidationWithDataList() {
+		//test with datalist input
+		$checkboxTestArticle = $this->objFromFixture('CheckboxSetFieldTest_Article', 'articlewithtags');
+		$tag1 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag1');
+		$tag2 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag2');
+		$tag3 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag3');
+		$field = CheckboxSetField::create('Test', 'Testing', $checkboxTestArticle->Tags()	->map());
+		$validator = new RequiredFields();
+		$field->setValue(array(
+			$tag1->ID => $tag1->ID,
+			$tag2->ID => $tag2->ID
+		));
+		$this->assertTrue(
+			$field->validate($validator),
+			'Validates values in source map'
+		);
+		//invalid value should fail
+		$fakeID = CheckboxSetFieldTest_Tag::get()->max('ID') + 1;
+		$field->setValue(array($fakeID => $fakeID));
+		$this->assertFalse(
+			$field->validate($validator),
+			'Field does not valid values outside of source map'
+		);
+		//non valid value included with valid options should succeed
+		$field->setValue(array(
+			$tag1->ID => $tag1->ID,
+			$tag2->ID => $tag2->ID,
+			$tag3->ID => $tag3->ID
+		));
+		$this->assertTrue(
+			$field->validate($validator),
+			'Validates when presented with mixed valid and invalid values'
+		);
+	}
+
 }
 
 /**

tests/forms/CheckboxSetFieldTest.yml

@@ -3,17 +3,8 @@ CheckboxSetFieldTest_Tag:
       Title: Tag 1
    tag2:
       Title: Tag 2
-CheckboxSetFieldTest_Article:
-   articlewithouttags:
-      Content: Article 1
-   articlewithtags:
-      Content: Article 2
-      Tags: =>CheckboxSetFieldTest_Tag.tag1,=>CheckboxSetFieldTest_Tag.tag2
-CheckboxSetFieldTest_Tag:
-   tag1:
-      Title: Tag 1
-   tag2:
-      Title: Tag 2
+   tag3:
+      Title: Tag 3
 CheckboxSetFieldTest_Article:
    articlewithouttags:
       Content: Article 1

tests/forms/CompositeFieldTest.php

@@ -60,4 +60,21 @@ class CompositeFieldTest extends SapphireTest {
 		$this->assertNotNull($legend);
 		$this->assertEquals('My legend', (string)$legend[0]);
 	}
+
+	public function testValidation() {
+		$field = CompositeField::create(
+			$fieldOne = DropdownField::create('A'),
+			$fieldTwo = TextField::create('B')
+		);
+		$validator = new RequiredFields();
+		$this->assertFalse(
+			$field->validate($validator),
+			"Validation fails when child is invalid"
+		);
+		$fieldOne->setEmptyString('empty');
+		$this->assertTrue(
+			$field->validate($validator),
+			"Validates when children are valid"
+		);
+	}
 }

tests/forms/ConfirmedPasswordFieldTest.php

@@ -61,15 +61,24 @@ class ConfirmedPasswordFieldTest extends SapphireTest {
 		));
 		$validator = new RequiredFields();
 		$form = new Form($this, 'Form', new FieldList($field), new FieldList(), $validator);
-		$this->assertTrue($field->validate($validator));
+		$this->assertTrue(
+			$field->validate($validator),
+			"Validates when both passwords are the same"
+		);
 		$field->setName("TestNew"); //try changing name of field
-		$this->assertTrue($field->validate($validator));
+		$this->assertTrue(
+			$field->validate($validator),
+			"Validates when field name is changed"
+		);
 		//non-matching password should make the field invalid
 		$field->setValue(array(
 			"_Password" => "abc123",
 			"_ConfirmPassword" => "123abc"
 		));
-		$this->assertFalse($field->validate($validator));
+		$this->assertFalse(
+			$field->validate($validator),
+			"Does not validate when passwords differ"
+		);
 	}
 
 }

tests/forms/CurrencyFieldTest.php

@@ -8,42 +8,55 @@ class CurrencyFieldTest extends SapphireTest {
 
 	public function testValidate() {
 		$f = new CurrencyField('TestField');
+		$validator = new RequiredFields();
 
 		$f->setValue('123.45');
 		$this->assertTrue(
-			$f->validate(new RequiredFields()),
+			$f->validate($validator),
 			'Validates positive decimals'
 		);
 
 		$f->setValue('-123.45');
 		$this->assertTrue(
-			$f->validate(new RequiredFields()),
+			$f->validate($validator),
 			'Validates negative decimals'
 		);
 
 		$f->setValue('$123.45');
 		$this->assertTrue(
-			$f->validate(new RequiredFields()),
+			$f->validate($validator),
 			'Validates positive decimals with sign'
 		);
 
 		$f->setValue('-$123.45');
 		$this->assertTrue(
-			$f->validate(new RequiredFields()),
+			$f->validate($validator),
 			'Validates negative decimals with sign'
 		);
 
 		$f->setValue('$-123.45');
 		$this->assertTrue(
-			$f->validate(new RequiredFields()),
+			$f->validate($validator),
 			'Validates negative decimals with sign'
 		);
 
 		$f->setValue('324511434634');
 		$this->assertTrue(
-			$f->validate(new RequiredFields()),
+			$f->validate($validator),
 			'Validates large integers'
 		);
+
+		$f->setValue('test$1.23test');
+		$this->assertTrue(
+			$f->validate($validator),
+			'Alphanumeric is valid'
+		);
+
+		$f->setValue('$test');
+		$this->assertTrue(
+			$f->validate($validator),
+			'Words are valid'
+		);
 	}
 
 	public function testSetValue() {
@@ -84,6 +97,18 @@ class CurrencyFieldTest extends SapphireTest {
 			$f->value, '$2.50',
 			'Truncates long decimal portions'
 		);
+
+		$f->setValue('test123.00test');
+		$this->assertEquals(
+			$f->value, '$123.00',
+			'Strips alpha values'
+		);
+
+		$f->setValue('test');
+		$this->assertEquals(
+			$f->value, '$0.00',
+			'Does not set alpha values'
+		);
 	}
 
 	public function testDataValue() {

tests/forms/DropdownFieldTest.php

@@ -310,4 +310,27 @@ class DropdownFieldTest extends SapphireTest {
 		return $foundDisabled;
 	}
 
+	public function testValidation() {
+		$field = DropdownField::create('Test', 'Testing', array(
+			"One" => "One",
+			"Two" => "Two"
+		));
+		$validator = new RequiredFields();
+		$form = new Form($this, 'Form', new FieldList($field), new FieldList(), $validator);
+		$field->setValue("One");
+		$this->assertTrue($field->validate($validator));
+		$field->setName("TestNew"); //try changing name of field
+		$this->assertTrue($field->validate($validator));
+		//non-existent value should make the field invalid
+		$field->setValue("Three");
+		$this->assertFalse($field->validate($validator));
+		//empty string shouldn't validate
+		$field->setValue('');
+		$this->assertFalse($field->validate($validator));
+		//empty field should validate after being set
+		$field->setEmptyString('Empty String');
+		$field->setValue('');
+		$this->assertTrue($field->validate($validator));
+	}
+
 }

tests/forms/FormFieldTest.php

@@ -3,7 +3,7 @@
  * @package framework
  * @subpackage tests
  */
-class FormFieldTest extends SapphireTest {
+class FormFieldTest extends SapphireTest implements TestOnly {
 
 	public function testAddExtraClass() {
 		$field = new FormField('MyField');
@@ -187,4 +187,4 @@ class FormFieldTest extends SapphireTest {
 		}
 	}
 
-}
+}

tests/forms/ListboxFieldTest.php

@@ -194,6 +194,76 @@ class ListboxFieldTest extends SapphireTest {
 		$field = new ListboxField('Choices', 'Choices', $choices);
 	}
 
+	public function testValidationWithArray() {
+		//test with array input
+		$field = ListboxField::create('Test', 'Testing', array(
+			1 => "One",
+			2 => "Two",
+			3 => "Three"
+		));
+		$validator = new RequiredFields();
+
+		$field->setValue(1);
+		$this->assertTrue(
+			$field->validate($validator),
+			'Validates values in source map'
+		);
+		$field->setMultiple(true);
+		$field->setValue(array(1));
+		$this->assertTrue(
+			$field->validate($validator),
+			'Validates values within source array'
+		);
+		//non valid value should fail
+		$field->setValue(4);
+		$this->assertFalse(
+			$field->validate($validator),
+			'Does not validates values not within source array'
+		);
+	}
+
+	public function testValidationWithDataList() {
+		//test with datalist input
+		$tag1 = $this->objFromFixture('ListboxFieldTest_Tag', 'tag1');
+		$tag2 = $this->objFromFixture('ListboxFieldTest_Tag', 'tag2');
+		$tag3 = $this->objFromFixture('ListboxFieldTest_Tag', 'tag3');
+		$field = ListboxField::create('Test', 'Testing', DataObject::get("ListboxFieldTest_Tag")->map()->toArray());
+		$validator = new RequiredFields();
+
+		$field->setValue(
+			$tag1->ID
+		);
+		$this->assertTrue(
+			$field->validate($validator),
+			'Field validates values in source map'
+		);
+
+		/**
+		 * @todo re-enable these tests when field validation is removed from {@link ListboxField::setValue()} and moved
+		 * to the {@link ListboxField::validate()} function
+		 */
+//		$field->setValue(4);
+//		$this->assertFalse(
+//			$field->validate($validator),
+//			'Field does not validate values outside of source map'
+//		);
+		$field->setMultiple(true);
+		$field->setValue(false, new ArrayData(array(
+			$tag1->ID => $tag1->ID,
+			$tag2->ID => $tag2->ID
+		)));
+		$this->assertTrue(
+			$field->validate($validator),
+			'Validates values in source map'
+		);
+		//invalid value should fail
+		$field->setValue(4);
+		$this->assertFalse(
+			$field->validate($validator),
+			'Does not validate values not within source map'
+		);
+	}
+
 }
 
 class ListboxFieldTest_DataObject extends DataObject implements TestOnly {

tests/forms/MemberDatetimeOptionsetFieldTest.php

@@ -89,11 +89,12 @@ class MemberDatetimeOptionsetFieldTest extends SapphireTest {
 
 	public function testDateFormValid() {
 		$field = new MemberDatetimeOptionsetField('DateFormat', 'DateFormat');
-		$this->assertTrue($field->validate(null));
+		$validator = new RequiredFields();
+		$this->assertTrue($field->validate($validator));
 		$_POST['DateFormat_custom'] = 'dd MM yyyy';
-		$this->assertTrue($field->validate(null));
+		$this->assertTrue($field->validate($validator));
 		$_POST['DateFormat_custom'] = 'sdfdsfdfd1244';
-		$this->assertFalse($field->validate(null));
+		$this->assertFalse($field->validate($validator));
 	}
 
 }

tests/forms/NumericFieldTest.php

@@ -14,7 +14,7 @@ class NumericFieldTest extends SapphireTest {
 		$field = new NumericField('Number');
 		$field->setValue('12.00');
 
-		$validator = new RequiredFields('Number');
+		$validator = new RequiredFields();
 		$this->assertTrue($field->validate($validator));
 
 		$field->setValue('12,00');
@@ -24,7 +24,7 @@ class NumericFieldTest extends SapphireTest {
 		$this->assertTrue($field->validate($validator));
 
 		$field->setValue(false);
-		$this->assertFalse($field->validate($validator));
+		$this->assertTrue($field->validate($validator));
 
 		i18n::set_locale('de_DE');
 		$field->setValue('12,00');

tests/forms/uploadfield/UploadFieldTest.php

@@ -463,7 +463,7 @@ class UploadFieldTest extends FunctionalTest {
 	}
 
 	public function testEdit() {
-		$this->loginWithPermission('ADMIN');
+		$memberID = $this->loginWithPermission('ADMIN');
 
 		$record = $this->objFromFixture('UploadFieldTest_Record', 'record1');
 		$file4 = $this->objFromFixture('File', 'file4');
@@ -474,7 +474,13 @@ class UploadFieldTest extends FunctionalTest {
 		$response = $this->get($baseUrl . '/edit');
 		$this->assertFalse($response->isError());
 
-		$response = $this->post($baseUrl . '/EditForm', array('Title' => 'File 4 modified'));
+		$response = $this->post(
+			$baseUrl . '/EditForm',
+				array(
+				'Title' => 'File 4 modified',
+				'OwnerID' => $memberID
+			)
+		);
 		$this->assertFalse($response->isError());
 
 		$record = DataObject::get_by_id($record->class, $record->ID, false);