View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
327 | PHPOF2 | Default | public | 19 Jan 2010 09:22 | 22 Nov 2010 23:59 |
Reporter | mrosenquist | Assigned To | |||
Priority | normal | Severity | feature | Reproducibility | have not tried |
Status | new | Resolution | open | ||
Product Version | 0.11.2 | ||||
Summary | 327: Possibly add a very limited set of validation for issues that do not throw db errors when calling update/insert | ||||
Description | This 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. | ||||
Tags | No tags attached. | ||||
Attached Files | 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; + } } | ||||
|
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) |
|
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. |
|
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 |
|
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"); } |
|
Added a patch which has some limited validation |
|
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. |
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 |