[BUGFIX] Activating NULL value field does not work with blank string
authorOliver Hader <oliver@typo3.org>
Tue, 20 Nov 2012 11:20:35 +0000 (12:20 +0100)
committerOliver Hader <oliver.hader@typo3.org>
Tue, 20 Nov 2012 13:28:32 +0000 (14:28 +0100)
Activating a field that supports NULL values and just using a
blank string ("") does not work. The problem is a strcmp() call
that returns a false-positive on comparing NULL to blank strings

Change-Id: I59417f5f5cd814db15e2b6b725f1778d098014f6
Fixes: #43139
Releases: 6.0
Reviewed-on: http://review.typo3.org/16599
Reviewed-by: Oliver Hader
Tested-by: Oliver Hader
typo3/sysext/core/Classes/DataHandling/DataHandler.php
typo3/sysext/core/Tests/Unit/DataHandling/DataHandlerTest.php

index a7ede19..fe72943 100644 (file)
@@ -6022,12 +6022,12 @@ class DataHandler {
                        $GLOBALS['TYPO3_DB']->sql_free_result($res);
                        // Unset the fields which are similar:
                        foreach ($fieldArray as $col => $val) {
-                               $isAlreadyNull = ($val === NULL && $currentRecord[$col] === NULL);
-                               $isNotNull = ($val !== NULL);
-                               // Unset field values if they are empty, which is "0" for integer types and "" for string - except the current field holds MM relations.
-                               // If NULL values shall be stored, then the value will only be unset if the current stored value is already NULL.
+                               $fieldConfiguration = $GLOBALS['TCA'][$table]['columns'][$col]['config'];
+                               $isNullField = (!empty($fieldConfiguration['eval']) && \TYPO3\CMS\Core\Utility\GeneralUtility::inList($fieldConfiguration['eval'], 'null'));
+
+                               // Unset fields if stored and submitted values are equal - except the current field holds MM relations.
                                // In general this avoids to store superfluous data which also will be visualized in the editing history.
-                               if (!$GLOBALS['TCA'][$table]['columns'][$col]['config']['MM'] && ($isAlreadyNull || $isNotNull && (!strcmp($val, $currentRecord[$col]) || $cRecTypes[$col] == 'int' && $currentRecord[$col] == 0 && !strcmp($val, '')))) {
+                               if (!$fieldConfiguration['MM'] && $this->isSubmittedValueEqualToStoredValue($val, $currentRecord[$col], $cRecTypes[$col], $isNullField)) {
                                        unset($fieldArray[$col]);
                                } else {
                                        if (!isset($this->mmHistoryRecords[($table . ':' . $id)]['oldRecord'][$col])) {
@@ -6050,6 +6050,42 @@ class DataHandler {
        }
 
        /**
+        * Determines whether submitted values and stored values are equal.
+        * This prevents from adding superfluous field changes which would be shown in the record history as well.
+        * For NULL fields (see accordant TCA definition 'eval' = 'null'), a special handling is required since
+        * (!strcmp(NULL, '')) would be a false-positive.
+        *
+        * @param mixed $submittedValue Value that has submitted (e.g. from a backend form)
+        * @param mixed $storedValue Value that is currently stored in the database
+        * @param string $storedType SQL type of the stored value column (see mysql_field_type(), e.g 'int', 'string',  ...)
+        * @param boolean $allowNull Whether NULL values are allowed by accordant TCA definition ('eval' = 'null')
+        * @return boolean Whether both values are considered to be equal
+        */
+       protected function isSubmittedValueEqualToStoredValue($submittedValue, $storedValue, $storedType, $allowNull = FALSE) {
+               // No NULL values are allowed, this is the regular behaviour.
+               // Thus, check whether strings are the same or whether integer values are empty ("0" or "").
+               if (!$allowNull) {
+                       $result = (
+                               !strcmp($submittedValue, $storedValue)
+                               || $storedType == 'int' && $storedValue == 0 && !strcmp($submittedValue, '')
+                       );
+               // Null values are allowed, but currently there's a real (not NULL) value.
+               // Thus, ensure no NULL value was submitted and fallback to the regular behaviour.
+               } elseif ($storedValue !== NULL) {
+                       $result = (
+                               $submittedValue !== NULL
+                               && $this->isSubmittedValueEqualToStoredValue($submittedValue, $storedValue, $storedType, FALSE)
+                       );
+               // Null values are allowed, and currently there's a NULL value.
+               // Thus, check whether a NULL value was submitted.
+               } else {
+                       $result = ($submittedValue === NULL);
+               }
+
+               return $result;
+       }
+
+       /**
         * Calculates the bitvalue of the permissions given in a string, comma-sep
         *
         * @param string $string List of pMap strings
index 83a0e38..8b5d200 100644 (file)
@@ -316,6 +316,171 @@ class DataHandlerTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $this->assertStringEndsWith($expected, $this->fixture->errorLog[0]);
        }
 
+       /**
+        * @param boolean $expected
+        * @param string $submittedValue
+        * @param string $storedValue
+        * @param string $storedType
+        * @param boolean $allowNull
+        * @dataProvider equalSubmittedAndStoredValuesAreDeterminedDataProvider
+        * @test
+        */
+       public function equalSubmittedAndStoredValuesAreDetermined($expected, $submittedValue, $storedValue, $storedType, $allowNull) {
+               $result = $this->callInaccessibleMethod(
+                       $this->fixture,
+                       'isSubmittedValueEqualToStoredValue',
+                       $submittedValue, $storedValue, $storedType, $allowNull
+               );
+               $this->assertEquals($expected, $result);
+       }
+
+       /**
+        * @return array
+        */
+       public function equalSubmittedAndStoredValuesAreDeterminedDataProvider() {
+               return array(
+                       // String
+                       'string value "" vs. ""' => array(
+                               TRUE,
+                               '', '', 'string', FALSE
+                       ),
+                       'string value 0 vs. "0"' => array(
+                               TRUE,
+                               0, '0', 'string', FALSE
+                       ),
+                       'string value 1 vs. "1"' => array(
+                               TRUE,
+                               1, '1', 'string', FALSE
+                       ),
+                       'string value "0" vs. ""' => array(
+                               FALSE,
+                               '0', '', 'string', FALSE
+                       ),
+                       'string value 0 vs. ""' => array(
+                               FALSE,
+                               0, '', 'string', FALSE
+                       ),
+                       'string value null vs. ""' => array(
+                               TRUE,
+                               NULL, '', 'string', FALSE
+                       ),
+                       // Integer
+                       'integer value 0 vs. 0' => array(
+                               TRUE,
+                               0, 0, 'int', FALSE
+                       ),
+                       'integer value "0" vs. "0"' => array(
+                               TRUE,
+                               '0', '0', 'int', FALSE
+                       ),
+                       'integer value 0 vs. "0"' => array(
+                               TRUE,
+                               0, '0', 'int', FALSE
+                       ),
+                       'integer value "" vs. "0"' => array(
+                               TRUE,
+                               '', '0', 'int', FALSE
+                       ),
+                       'integer value 1 vs. 1' => array(
+                               TRUE,
+                               1, 1, 'int', FALSE
+                       ),
+                       'integer value 1 vs. "1"' => array(
+                               TRUE,
+                               1, '1', 'int', FALSE
+                       ),
+                       'integer value "0" vs. "1"' => array(
+                               FALSE,
+                               '0', '1', 'int', FALSE
+                       ),
+                       // String with allowed NULL values
+                       'string with allowed null value "" vs. ""' => array(
+                               TRUE,
+                               '', '', 'string', TRUE
+                       ),
+                       'string with allowed null value 0 vs. "0"' => array(
+                               TRUE,
+                               0, '0', 'string', TRUE
+                       ),
+                       'string with allowed null value 1 vs. "1"' => array(
+                               TRUE,
+                               1, '1', 'string', TRUE
+                       ),
+                       'string with allowed null value "0" vs. ""' => array(
+                               FALSE,
+                               '0', '', 'string', TRUE
+                       ),
+                       'string with allowed null value 0 vs. ""' => array(
+                               FALSE,
+                               0, '', 'string', TRUE
+                       ),
+                       'string with allowed null value null vs. ""' => array(
+                               FALSE,
+                               NULL, '', 'string', TRUE
+                       ),
+                       'string with allowed null value "" vs. null' => array(
+                               FALSE,
+                               '', NULL, 'string', TRUE
+                       ),
+                       'string with allowed null value null vs. null' => array(
+                               TRUE,
+                               NULL, NULL, 'string', TRUE
+                       ),
+                       // Integer with allowed NULL values
+                       'integer with allowed null value 0 vs. 0' => array(
+                               TRUE,
+                               0, 0, 'int', TRUE
+                       ),
+                       'integer with allowed null value "0" vs. "0"' => array(
+                               TRUE,
+                               '0', '0', 'int', TRUE
+                       ),
+                       'integer with allowed null value 0 vs. "0"' => array(
+                               TRUE,
+                               0, '0', 'int', TRUE
+                       ),
+                       'integer with allowed null value "" vs. "0"' => array(
+                               TRUE,
+                               '', '0', 'int', TRUE
+                       ),
+                       'integer with allowed null value 1 vs. 1' => array(
+                               TRUE,
+                               1, 1, 'int', TRUE
+                       ),
+                       'integer with allowed null value 1 vs. "1"' => array(
+                               TRUE,
+                               1, '1', 'int', TRUE
+                       ),
+                       'integer with allowed null value "0" vs. "1"' => array(
+                               FALSE,
+                               '0', '1', 'int', TRUE
+                       ),
+                       'integer with allowed null value null vs. ""' => array(
+                               FALSE,
+                               NULL, '', 'int', TRUE
+                       ),
+                       'integer with allowed null value "" vs. null' => array(
+                               FALSE,
+                               '', NULL, 'int', TRUE
+                       ),
+                       'integer with allowed null value null vs. null' => array(
+                               TRUE,
+                               NULL, NULL, 'int', TRUE
+                       ),
+                       'integer with allowed null value null vs. "0"' => array(
+                               FALSE,
+                               NULL, '0', 'int', TRUE
+                       ),
+                       'integer with allowed null value "0" vs. null' => array(
+                               FALSE,
+                               '0', NULL, 'int', TRUE
+                       ),
+                       'integer with allowed null value null vs. null' => array(
+                               TRUE,
+                               NULL, NULL, 'int', TRUE
+                       ),
+               );
+       }
 }
 
 ?>
\ No newline at end of file