View Issue Details

IDProjectCategoryView StatusLast Update
327PHPOF2Defaultpublic22 Nov 2010 23:59
Reportermrosenquist Assigned To 
PrioritynormalSeverityfeatureReproducibilityhave not tried
Status newResolutionopen 
Product Version0.11.2 
Summary327: Possibly add a very limited set of validation for issues that do not throw db errors when calling update/insert
DescriptionThis is not a replacement for controller validation, only to make sure common cases do not get overlooked.

The main issue is with varchar/char fields, there is no error generated when storing a value that is too long for the field will just be cut. This may possibly extend to lob fields.

Optionally the min max values for int could be checked.

Less of an issue but timestamp fields could be checked.

TagsNo tags attached.
Attached Files
test.php (1,046 bytes)
DBRow.php.327.patch (2,205 bytes)   
Index: DBRow.php
===================================================================
--- DBRow.php	(revision 37487)
+++ DBRow.php	(working copy)
@@ -1048,5 +1048,68 @@
 			throw new Exception('Cannot get changes - the original values were not stored');
 		}	
 	}
+	
+	/**
+	 * Very basic validation to insure that cases that would not cause errors 
+	 * when doing inserts and updates can be checked for before doing an insert/update
+	 *
+	 * @param array $errors - (optional)
+	 *
+	 * @return bool False if a condition is not valid
+	 */
+	public function isValid(&$errors=array())
+	{
+		$this->db->loadModule('Reverse');
+	
+		foreach (array_merge($this->fields_normal, $this->pri_keys) as $field) {
+			$field_defs = $this->db->reverse->getTableFieldDefinition($this->table->name, $field);
+			foreach ($field_defs as $def) {
+
+				if (strcmp($def['mdb2type'], 'text')==0) {
+					$length = null;
+					if (array_key_exists('length', $def)) {
+						$length = (int)$def['length'];
+					} else {
+						switch($def['nativetype']) {
+							case 'longtext':
+								$length = 4294967295;
+								break;
+						
+							case 'mediumtext':
+								$length = 16777215;
+								break;
+						
+							case 'text':
+								$length = 65535;
+								break;
+
+							case 'tinytext':
+								$length = 255;
+								break;
+						}
+					}
+					if (is_int($length)) 
+						if (!function_exists('mb_strlen')) {
+							function mb_strlen($string)
+							{
+								return strlen($string);
+							}
+						}
+						if (mb_strlen($this->$field)>$length) {
+						$errors[$field] = 'The value of "' . $this->table->getLabel($field) . '" exceeds the input length of ' . $length;
+						break;
+					}
+				} else if (array_key_exists('notnull', $def) && $def['notnull'] && is_null($this->$field)) {
+					$errors[$field] = 'The value of "' . $this->table->getLabel($field) . '" cannot be null';
+					break;
+				} else if (strcmp($def['mdb2type'], 'integer')==0 && !is_numeric($this->$field) && !is_null($this->$field)) {
+					$errors[$field] = 'The value of "' . $this->table->getLabel($field) . '" must be numeric';
+					break;					
+				}
+			}
+		}
+				
+		return count($errors)==0;
+	}
 }
 
DBRow.php.327.patch (2,205 bytes)   

Activities

mrosenquist

19 Jan 2010 09:23

reporter   ~331

Show warnings
mysql> create table one (varchar10 varchar(10) default NULL);
Query OK, 0 rows affected (0.04 sec)

mysql> insert into one values ('12345678901234567890');
Query OK, 1 row affected, 1 warning (0.03 sec)

mysql> show warnings;
+---------+------+------------------------------------------------+
| Level | Code | Message |
+---------+------+------------------------------------------------+
| Warning | 1265 | Data truncated for column 'varchar10' at row 1 |
+---------+------+------------------------------------------------+
1 row in set (0.02 sec)

mysql> select * from one;
+------------+
| varchar10 |
+------------+
| 1234567890 |
+------------+
1 row in set (0.00 sec)

timj

21 Jan 2010 19:36

manager   ~332

It sounds like a nice idea and I don't have a problem with the principle, but I would want to get the data from MDB2. Also some of the logic could be quite complicated (unless MDB2 abstracts it) for example comparing the length of a character field when you have multibyte charsets with varying lengths etc.

mrosenquist

15 Mar 2010 13:27

reporter   ~341

In the MDB2_Driver_Common there are two method referring to warnings.
http://pear.php.net/package/MDB2/docs/latest/MDB2/MDB2_Driver_Common.html#methodresetWarnings
http://pear.php.net/package/MDB2/docs/latest/MDB2/MDB2_Driver_Common.html#methodgetWarnings

however on closer inspection of getWarnings it would appear that it is only internal and never gets the warnings from mysql when using the mysqli extension

mkierat

15 Mar 2010 13:47

reporter   ~342

The following google answer - http://answers.google.com/answers/threadview/id/730396.html - describes two ways: using temporary tables and transactions.

We could do something with transactions. It would only work for innodb and I am not sure what would be performance impact. Also we could not use autocommit (not sure if we use this anywhere). See attached file and the code below for eaxmple:

//would need to reset warnings before this - couldn't see "RESET WARNINGS" or similar
echo "testing error transaction\n";
$db->query("insert into test2 values ('7892345234523523450')");
if ($db->query('SHOW WARNINGS')->numRows() > 0) { //SELECT @@warning_count; will also work (even better)
    echo "rolling back\n";
    $db->query("ROLLBACK");
} else {
    echo "committing transaction\n";
    $db->query("COMMIT");
}

mrosenquist

19 Nov 2010 13:46

reporter   ~381

Added a patch which has some limited validation

timj

22 Nov 2010 23:59

manager   ~382

Thanks. The patch is generally quite nice, however it would still be better in MDB2; is there really no validation functions in that? (I haven't checked). Anyway, a couple of problems with it in PHPOF2:

1. hardcoded references to MySQL datatypes and lengths. These would need to be abstracted into the db-specific module.

2. use of English text for errors; if I was going to import it I would change it to return consts instead, something like this:

$errors[$field] = self::VALIDATION_ERR_DATASIZE; //etc.

Issue History

Date Modified Username Field Change
19 Jan 2010 09:22 mrosenquist New Issue
19 Jan 2010 09:23 mrosenquist Note Added: 331
21 Jan 2010 19:36 timj Note Added: 332
15 Mar 2010 13:27 mrosenquist Note Added: 341
15 Mar 2010 13:47 mkierat Note Added: 342
15 Mar 2010 13:47 mkierat File Added: test.php
19 Nov 2010 13:46 mrosenquist File Added: DBRow.php.327.patch
19 Nov 2010 13:46 mrosenquist Note Added: 381
22 Nov 2010 23:59 timj Note Added: 382