Fix incorrect time handling in deconstruct()

Apply patch from 'Amit Badkas' to solve issues where invalid times
were treated as valid.
Re-structure tests to use a dataprovider instead of copy + paste.

Fixes #2412

lib/Cake/Model/Model.php

@@ -1149,7 +1149,17 @@ class Model extends Object {
 			$timeFields = array('H' => 'hour', 'i' => 'min', 's' => 'sec');
 			$date = array();
 
-			if (isset($data['hour']) && isset($data['meridian']) && $data['hour'] != 12 && 'pm' == $data['meridian']) {
+			if (isset($data['meridian']) && empty($data['meridian'])) {
+				return null;
+			}
+
+			if (
+				isset($data['hour']) &&
+				isset($data['meridian']) &&
+				!empty($data['hour']) &&
+				$data['hour'] != 12 &&
+				'pm' == $data['meridian']
+			) {
 				$data['hour'] = $data['hour'] + 12;
 			}
 			if (isset($data['hour']) && isset($data['meridian']) && $data['hour'] == 12 && 'am' == $data['meridian']) {

lib/Cake/Test/Case/Model/ModelIntegrationTest.php

@@ -662,91 +662,85 @@ class ModelIntegrationTest extends BaseModelTest {
 	}
 
 /**
- * test deconstruct() with time fields.
+ * data provider for time tests.
  *
+ * @return array
+ */
+	public static function timeProvider() {
+		$db = ConnectionManager::getDataSource('test');
+		$now = $db->expression('NOW()');
+		return array(
+			// blank
+			array(
+				array('hour' => '', 'min' => '', 'meridian' => ''),
+				''
+			),
+			// missing hour
+			array(
+				array('hour' => '', 'min' => '00', 'meridian' => 'pm'),
+				''
+			),
+			// all blank
+			array(
+				array('hour' => '', 'min' => '', 'sec' => ''),
+				''
+			),
+			// set and empty merdian 
+			array(
+				array('hour' => '1', 'min' => '00', 'meridian' => ''),
+				''
+			),
+			// midnight
+			array(
+				array('hour' => '12', 'min' => '0', 'meridian' => 'am'),
+				'00:00:00'
+			),
+			array(
+				array('hour' => '00', 'min' => '00'),
+				'00:00:00'
+			),
+			// 3am
+			array(
+				array('hour' => '03', 'min' => '04', 'sec' => '04'),
+				'03:04:04'
+			),
+			array(
+				array('hour' => '3', 'min' => '4', 'sec' => '4'),
+				'03:04:04'
+			),
+			array(
+				array('hour' => '03', 'min' => '4', 'sec' => '4'),
+				'03:04:04'
+			),
+			array(
+				$now,
+				$now
+			)
+		);
+	}
+
+/**
+ * test deconstruct with time fields.
+ *
+ * @dataProvider timeProvider
  * @return void
  */
-	public function testDeconstructFieldsTime() {
+	public function testDeconstructFieldsTime($input, $result) {
 		$this->skipIf($this->db instanceof Sqlserver, 'This test is not compatible with SQL Server.');
 
 		$this->loadFixtures('Apple');
 		$TestModel = new Apple();
 
-		$data = array();
-		$data['Apple']['mytime']['hour'] = '';
-		$data['Apple']['mytime']['min'] = '';
-		$data['Apple']['mytime']['sec'] = '';
-
-		$TestModel->data = null;
-		$TestModel->set($data);
-		$expected = array('Apple' => array('mytime' => ''));
-		$this->assertEquals($TestModel->data, $expected);
-
-		$data = array();
-		$data['Apple']['mytime']['hour'] = '';
-		$data['Apple']['mytime']['min'] = '';
-		$data['Apple']['mytime']['meridan'] = '';
-
-		$TestModel->data = null;
-		$TestModel->set($data);
-		$expected = array('Apple' => array('mytime' => ''));
-		$this->assertEquals($TestModel->data, $expected, 'Empty values are not returning properly. %s');
-
-		$data = array();
-		$data['Apple']['mytime']['hour'] = '12';
-		$data['Apple']['mytime']['min'] = '0';
-		$data['Apple']['mytime']['meridian'] = 'am';
-
-		$TestModel->data = null;
-		$TestModel->set($data);
-		$expected = array('Apple' => array('mytime' => '00:00:00'));
-		$this->assertEquals($TestModel->data, $expected, 'Midnight is not returning proper values. %s');
-
-		$data = array();
-		$data['Apple']['mytime']['hour'] = '00';
-		$data['Apple']['mytime']['min'] = '00';
-
-		$TestModel->data = null;
-		$TestModel->set($data);
-		$expected = array('Apple' => array('mytime' => '00:00:00'));
-		$this->assertEquals($TestModel->data, $expected, 'Midnight is not returning proper values. %s');
-
-		$data = array();
-		$data['Apple']['mytime']['hour'] = '03';
-		$data['Apple']['mytime']['min'] = '04';
-		$data['Apple']['mytime']['sec'] = '04';
-
-		$TestModel->data = null;
-		$TestModel->set($data);
-		$expected = array('Apple' => array('mytime' => '03:04:04'));
-		$this->assertEquals($TestModel->data, $expected);
-
-		$data = array();
-		$data['Apple']['mytime']['hour'] = '3';
-		$data['Apple']['mytime']['min'] = '4';
-		$data['Apple']['mytime']['sec'] = '4';
-
-		$TestModel->data = null;
-		$TestModel->set($data);
-		$expected = array('Apple' => array('mytime' => '03:04:04'));
-		$this->assertEquals($TestModel->data, $expected);
-
-		$data = array();
-		$data['Apple']['mytime']['hour'] = '03';
-		$data['Apple']['mytime']['min'] = '4';
-		$data['Apple']['mytime']['sec'] = '4';
+		$data = array(
+			'Apple' => array(
+				'mytime' => $input
+			)
+		);
 
 		$TestModel->data = null;
 		$TestModel->set($data);
-		$expected = array('Apple' => array('mytime' => '03:04:04'));
+		$expected = array('Apple' => array('mytime' => $result));
 		$this->assertEquals($TestModel->data, $expected);
-
-		$db = ConnectionManager::getDataSource('test');
-		$data = array();
-		$data['Apple']['mytime'] = $db->expression('NOW()');
-		$TestModel->data = null;
-		$TestModel->set($data);
-		$this->assertEquals($TestModel->data, $data);
 	}
 
 /**