FIX: Ensure that repeated setting/unsetting doesn’t corrode forceChan…

…ge()

src/ORM/DataObject.php

@@ -166,11 +166,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
      */
     const CHANGE_NONE = 0;
 
-    /**
-     * Represents a field that has had a change forced
-     */
-    const CHANGE_FORCED = 0.5;
-
     /**
      * Represents a field that has changed type, although not the loosely defined value.
      * (before !== after && before == after)
@@ -193,14 +188,23 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
      *
      * @var array
      */
-    private $changed;
+    private $changed = [];
+
+    /**
+     * A flag to indicate that a "strict" change of the entire record been forced
+     * Use {@link getChangedFields()} and {@link isChanged()} to inspect
+     * the changed state.
+     *
+     * @var boolean
+     */
+    private $changeForced = false;
 
     /**
      * The database record (in the same format as $record), before
      * any changes.
      * @var array
      */
-    protected $original;
+    protected $original = [];
 
     /**
      * Used by onBeforeDelete() to ensure child classes call parent::onBeforeDelete()
@@ -392,7 +396,8 @@ public function __construct($record = null, $isSingleton = false, $queryParams =
         }
 
         // prevent populateDefaults() and setField() from marking overwritten defaults as changed
-        $this->changed = array();
+        $this->changed = [];
+        $this->changeForced = false;
     }
 
     /**
@@ -1112,37 +1117,26 @@ public function merge($rightObj, $priority = 'right', $includeRelations = true,
 
     /**
      * Forces the record to think that all its data has changed.
-     * Doesn't write to the database. Only sets fields as changed
-     * if they are not already marked as changed.
+     * Doesn't write to the database. Force-change preseved until
+     * next write. Existing CHANGE_VALUE or CHANGE_STRICT values
+     * are preserved.
      *
      * @return $this
      */
     public function forceChange()
     {
         // Ensure lazy fields loaded
         $this->loadLazyFields();
-        $fields = static::getSchema()->fieldSpecs(static::class);
 
-        // $this->record might not contain the blank values so we loop on $this->inheritedDatabaseFields() as well
-        $fieldNames = array_unique(array_merge(
-            array_keys($this->record),
-            array_keys($fields)
-        ));
-
-        foreach ($fieldNames as $fieldName) {
-            if (!isset($this->changed[$fieldName])) {
-                $this->changed[$fieldName] = self::CHANGE_FORCED;
-            }
-            // Populate the null values in record so that they actually get written
+        // Populate the null values in record so that they actually get written
+        foreach (array_keys(static::getSchema()->fieldSpecs(static::class)) as $fieldName) {
             if (!isset($this->record[$fieldName])) {
                 $this->record[$fieldName] = null;
             }
         }
 
-        // @todo Find better way to allow versioned to write a new version after forceChange
-        if ($this->isChanged('Version')) {
-            unset($this->changed['Version']);
-        }
+        $this->changeForced = true;
+
         return $this;
     }
 
@@ -1379,11 +1373,13 @@ protected function prepareManipulationTable($baseTable, $now, $isNewRecord, &$ma
         $table = $schema->tableName($class);
         $manipulation[$table] = array();
 
+        $changed = $this->getChangedFields();
+
         // Extract records for this table
         foreach ($this->record as $fieldName => $fieldValue) {
             // we're not attempting to reset the BaseTable->ID
             // Ignore unchanged fields or attempts to reset the BaseTable->ID
-            if (empty($this->changed[$fieldName]) || ($table === $baseTable && $fieldName === 'ID')) {
+            if (empty($changed[$fieldName]) || ($table === $baseTable && $fieldName === 'ID')) {
                 continue;
             }
 
@@ -1529,6 +1525,7 @@ public function write($showDebug = false, $forceInsert = false, $forceWrite = fa
             // Reset isChanged data
             // DBComposites properly bound to the parent record will also have their isChanged value reset
             $this->changed = [];
+            $this->changeForced = false;
             $this->original = $this->record;
         } else {
             if ($showDebug) {
@@ -2530,13 +2527,25 @@ public function getChangedFields($databaseFieldsOnly = false, $changeLevel = sel
             }
         }
 
+        // If change was forced, then derive change data from $this->record
+        if ($this->changeForced && $changeLevel <= self::CHANGE_STRICT) {
+            $changed = array_combine(
+                array_keys($this->record),
+                array_fill(0, count($this->record), self::CHANGE_STRICT)
+            );
+            // @todo Find better way to allow versioned to write a new version after forceChange
+            unset($changed['Version']);
+        } else {
+            $changed = $this->changed;
+        }
+
         if (is_array($databaseFieldsOnly)) {
-            $fields = array_intersect_key((array)$this->changed, array_flip($databaseFieldsOnly));
+            $fields = array_intersect_key($changed, array_flip($databaseFieldsOnly));
         } elseif ($databaseFieldsOnly) {
             $fieldsSpecs = static::getSchema()->fieldSpecs(static::class);
-            $fields = array_intersect_key((array)$this->changed, $fieldsSpecs);
+            $fields = array_intersect_key($changed, $fieldsSpecs);
         } else {
-            $fields = $this->changed;
+            $fields = $changed;
         }
 
         // Filter the list to those of a certain change level
@@ -2650,7 +2659,7 @@ public function setField($fieldName, $val)
                     $this->changed[$fieldName] = self::CHANGE_VALUE;
                 }
             // Value has been restored to its original, remove any record of the change
-            } elseif (isset($this->changed[$fieldName]) && $this->changed[$fieldName] !== self::CHANGE_FORCED) {
+            } elseif (isset($this->changed[$fieldName])) {
                 unset($this->changed[$fieldName]);
             }
 

tests/php/ORM/DataObjectTest.php

@@ -802,17 +802,30 @@ public function testForceChangeCantBeCancelledUntilWrite()
         $this->assertFalse($obj->isChanged('FirstName'));
         $this->assertFalse($obj->isChanged('Surname'));
 
+        // Force change marks the records as changed
         $obj->forceChange();
         $this->assertTrue($obj->isChanged('FirstName'));
         $this->assertTrue($obj->isChanged('Surname'));
 
+        // ...but not if we explicitly ask if the value has changed
         $this->assertFalse($obj->isChanged('FirstName', DataObject::CHANGE_VALUE));
         $this->assertFalse($obj->isChanged('Surname', DataObject::CHANGE_VALUE));
 
+        // Not overwritten by setting the value to is original value
         $obj->FirstName = 'Captain';
         $this->assertTrue($obj->isChanged('FirstName'));
         $this->assertTrue($obj->isChanged('Surname'));
 
+        // Not overwritten by changing it to something else and back again
+        $obj->FirstName = 'Captain-changed';
+        $this->assertTrue($obj->isChanged('FirstName', DataObject::CHANGE_VALUE));
+
+        $obj->FirstName = 'Captain';
+        $this->assertFalse($obj->isChanged('FirstName', DataObject::CHANGE_VALUE));
+        $this->assertTrue($obj->isChanged('FirstName'));
+        $this->assertTrue($obj->isChanged('Surname'));
+
+        // Cleared after write
         $obj->write();
         $this->assertFalse($obj->isChanged('FirstName'));
         $this->assertFalse($obj->isChanged('Surname'));