diff --git a/docs/en/04_Changelogs/4.4.0.md b/docs/en/04_Changelogs/4.4.0.md index f37ba13a6..1b761129b 100644 --- a/docs/en/04_Changelogs/4.4.0.md +++ b/docs/en/04_Changelogs/4.4.0.md @@ -6,7 +6,7 @@ ## Upgrading {#upgrading} -tbc + - dev/build is now non-destructive for all Enums, not just ClassNames. This means your data won't be lost if you're switching between versions, but watch out for code that breaks when it sees an unrecognised value! ## Changes to internal APIs diff --git a/src/ORM/Connect/DBSchemaManager.php b/src/ORM/Connect/DBSchemaManager.php index 38ef02fd6..20ea84242 100644 --- a/src/ORM/Connect/DBSchemaManager.php +++ b/src/ORM/Connect/DBSchemaManager.php @@ -713,37 +713,6 @@ MESSAGE $this->transCreateField($table, $field, $spec_orig); $this->alterationMessage("Field $table.$field: created as $spec_orig", "created"); } elseif ($fieldValue != $specValue) { - // If enums/sets are being modified, then we need to fix existing data in the table. - // Update any records where the enum is set to a legacy value to be set to the default. - foreach (array('enum', 'set') as $enumtype) { - if (preg_match("/^$enumtype/i", $specValue)) { - $newStr = preg_replace("/(^$enumtype\\s*\\(')|('\\).*)/i", "", $spec_orig); - $new = preg_split("/'\\s*,\\s*'/", $newStr); - - $oldStr = preg_replace("/(^$enumtype\\s*\\(')|('\\).*)/i", "", $fieldValue); - $old = preg_split("/'\\s*,\\s*'/", $oldStr); - - $holder = array(); - foreach ($old as $check) { - if (!in_array($check, $new)) { - $holder[] = $check; - } - } - if (count($holder)) { - $default = explode('default ', $spec_orig); - $default = $default[1]; - $query = "UPDATE \"$table\" SET $field=$default WHERE $field IN ("; - for ($i = 0; $i + 1 < count($holder); $i++) { - $query .= "'{$holder[$i]}', "; - } - $query .= "'{$holder[$i]}')"; - $this->query($query); - $amount = $this->database->affectedRows(); - $this->alterationMessage("Changed $amount rows to default value of field $field" - . " (Value: $default)"); - } - } - } $this->transAlterField($table, $field, $spec_orig); $this->alterationMessage( "Field $table.$field: changed to $specValue (from {$fieldValue})", diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index 3fbf3afa4..6c3730703 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -19,6 +19,7 @@ use SilverStripe\i18n\i18n; use SilverStripe\i18n\i18nEntityProvider; use SilverStripe\ORM\Connect\MySQLSchemaManager; use SilverStripe\ORM\FieldType\DBClassName; +use SilverStripe\ORM\FieldType\DBEnum; use SilverStripe\ORM\FieldType\DBComposite; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\FieldType\DBField; @@ -3197,7 +3198,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity public static function reset() { // @todo Decouple these - DBClassName::clear_classname_cache(); + DBEnum::flushCache(); ClassInfo::reset_db_cache(); static::getSchema()->reset(); self::$_cache_get_one = array(); diff --git a/src/ORM/FieldType/DBClassName.php b/src/ORM/FieldType/DBClassName.php index 7b7689f7c..3255aef15 100644 --- a/src/ORM/FieldType/DBClassName.php +++ b/src/ORM/FieldType/DBClassName.php @@ -6,6 +6,7 @@ use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Config; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; +use SilverStripe\Dev\Deprecation; /** * Represents a classname selector, which respects obsolete clasess. @@ -29,14 +30,6 @@ class DBClassName extends DBEnum */ protected $record = null; - /** - * Classname spec cache for obsolete classes. The top level keys are the table, each of which contains - * nested arrays with keys mapped to field names. The values of the lowest level array are the classnames - * - * @var array - */ - protected static $classname_cache = array(); - private static $index = true; /** @@ -45,7 +38,8 @@ class DBClassName extends DBEnum */ public static function clear_classname_cache() { - self::$classname_cache = array(); + Deprecation::notice('4.3', 'Call DBEnum::flushCache() instead'); + DBEnum::flushCache(); } /** @@ -149,47 +143,6 @@ class DBClassName extends DBEnum return array_values($classNames); } - /** - * Get the list of classnames, including obsolete classes. - * - * If table or name are not set, or if it is not a valid field on the given table, - * then only known classnames are returned. - * - * Values cached in this method can be cleared via `DBClassName::clear_classname_cache();` - * - * @return array - */ - public function getEnumObsolete() - { - // Without a table or field specified, we can only retrieve known classes - $table = $this->getTable(); - $name = $this->getName(); - if (empty($table) || empty($name)) { - return $this->getEnum(); - } - - // Ensure the table level cache exists - if (empty(self::$classname_cache[$table])) { - self::$classname_cache[$table] = array(); - } - - // Check existing cache - if (!empty(self::$classname_cache[$table][$name])) { - return self::$classname_cache[$table][$name]; - } - - // Get all class names - $classNames = $this->getEnum(); - if (DB::get_schema()->hasField($table, $name)) { - $existing = DB::query("SELECT DISTINCT \"{$name}\" FROM \"{$table}\"")->column(); - $classNames = array_unique(array_merge($classNames, $existing)); - } - - // Cache and return - self::$classname_cache[$table][$name] = $classNames; - return $classNames; - } - public function setValue($value, $record = null, $markChanged = true) { parent::setValue($value, $record, $markChanged); diff --git a/src/ORM/FieldType/DBEnum.php b/src/ORM/FieldType/DBEnum.php index cb3126cec..3715365a7 100644 --- a/src/ORM/FieldType/DBEnum.php +++ b/src/ORM/FieldType/DBEnum.php @@ -20,7 +20,7 @@ class DBEnum extends DBString * * @var array */ - protected $enum = array(); + protected $enum = []; /** * Default value @@ -31,6 +31,22 @@ class DBEnum extends DBString private static $default_search_filter_class = 'ExactMatchFilter'; + /** + * Internal cache for obsolete enum values. The top level keys are the table, each of which contains + * nested arrays with keys mapped to field names. The values of the lowest level array are the enum values + * + * @var array + */ + protected static $enum_cache = []; + + /** + * Clear all cached enum values. + */ + public static function flushCache() + { + self::$enum_cache = []; + } + /** * Create a new Enum field, which is a value within a defined set, with an optional default. * @@ -88,7 +104,7 @@ class DBEnum extends DBString $parts = array( 'datatype' => 'enum', - 'enums' => $this->getEnum(), + 'enums' => $this->getEnumObsolete(), 'character set' => $charset, 'collate' => $collation, 'default' => $this->getDefault(), @@ -173,6 +189,48 @@ class DBEnum extends DBString return $this->enum; } + + /** + * Get the list of enum values, including obsolete values still present in the database + * + * If table or name are not set, or if it is not a valid field on the given table, + * then only known enum values are returned. + * + * Values cached in this method can be cleared via `DBEnum::flushCache();` + * + * @return array + */ + public function getEnumObsolete() + { + // Without a table or field specified, we can only retrieve known enum values + $table = $this->getTable(); + $name = $this->getName(); + if (empty($table) || empty($name)) { + return $this->getEnum(); + } + + // Ensure the table level cache exists + if (empty(self::$enum_cache[$table])) { + self::$enum_cache[$table] = array(); + } + + // Check existing cache + if (!empty(self::$enum_cache[$table][$name])) { + return self::$enum_cache[$table][$name]; + } + + // Get all enum values + $enumValues = $this->getEnum(); + if (DB::get_schema()->hasField($table, $name)) { + $existing = DB::query("SELECT DISTINCT \"{$name}\" FROM \"{$table}\"")->column(); + $enumValues = array_unique(array_merge($enumValues, $existing)); + } + + // Cache and return + self::$enum_cache[$table][$name] = $enumValues; + return $enumValues; + } + /** * Set enum options * diff --git a/tests/php/ORM/DBEnumTest.php b/tests/php/ORM/DBEnumTest.php index 3ed633f15..7fe37403a 100644 --- a/tests/php/ORM/DBEnumTest.php +++ b/tests/php/ORM/DBEnumTest.php @@ -5,9 +5,17 @@ namespace SilverStripe\ORM\Tests; use SilverStripe\Dev\SapphireTest; use SilverStripe\ORM\FieldType\DBEnum; use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\ORM\DB; class DBEnumTest extends SapphireTest { + + protected $extraDataObjects = [ + FieldType\DBEnumTestObject::class, + ]; + + protected $usesDatabase = true; + public function testDefault() { /** @var DBEnum $enum1 */ @@ -28,4 +36,66 @@ class DBEnumTest extends SapphireTest $this->assertEquals('B', $enum4->getDefaultValue()); $this->assertEquals('B', $enum4->getDefault()); } + + public function testObsoleteValues() + { + $obj = new FieldType\DBEnumTestObject(); + $colourField = $obj->obj('Colour'); + $colourField->setTable('FieldType_DBEnumTestObject'); + + // Test values prior to any database content + $this->assertEquals( + ['Red', 'Blue', 'Green'], + $colourField->getEnumObsolete() + ); + + // Test values with a record + $obj->Colour = 'Red'; + $obj->write(); + DBEnum::flushCache(); + + $this->assertEquals( + ['Red', 'Blue', 'Green'], + $colourField->getEnumObsolete() + ); + + // If the value is removed from the enum, obsolete content is still retained + $colourField->setEnum(['Blue', 'Green', 'Purple']); + DBEnum::flushCache(); + + $this->assertEquals( + ['Blue', 'Green', 'Purple', 'Red'], // Red on the end now, because it's obsolete + $colourField->getEnumObsolete() + ); + + // Check that old and new data is preserved after a schema update + DB::get_schema()->schemaUpdate(function () use ($colourField) { + $colourField->requireField(); + }); + + $obj2 = new FieldType\DBEnumTestObject(); + $obj2->Colour = 'Purple'; + $obj2->write(); + + $this->assertEquals( + ['Purple', 'Red'], + FieldType\DBEnumTestObject::get()->sort('Colour')->column('Colour') + ); + + // Ensure that enum columns are retained + $colourField->setEnum(['Blue', 'Green']); + $this->assertEquals( + ['Blue', 'Green', 'Purple', 'Red'], + $colourField->getEnumObsolete() + ); + + // If obsolete records are deleted, the extra values go away + $obj->delete(); + $obj2->delete(); + DBEnum::flushCache(); + $this->assertEquals( + ['Blue', 'Green'], + $colourField->getEnumObsolete() + ); + } } diff --git a/tests/php/ORM/DataObjectSchemaGenerationTest.php b/tests/php/ORM/DataObjectSchemaGenerationTest.php index b366d5d51..73313d63b 100644 --- a/tests/php/ORM/DataObjectSchemaGenerationTest.php +++ b/tests/php/ORM/DataObjectSchemaGenerationTest.php @@ -5,7 +5,7 @@ namespace SilverStripe\ORM\Tests; use SilverStripe\Core\Config\Config; use SilverStripe\ORM\Connect\MySQLSchemaManager; use SilverStripe\ORM\DB; -use SilverStripe\ORM\FieldType\DBClassName; +use SilverStripe\ORM\FieldType\DBEnum; use SilverStripe\ORM\DataObject; use SilverStripe\Dev\SapphireTest; use SilverStripe\ORM\Tests\DataObjectSchemaGenerationTest\SortedObject; @@ -208,7 +208,7 @@ class DataObjectSchemaGenerationTest extends SapphireTest $schema = DataObject::getSchema(); // Test with blank entries - DBClassName::clear_classname_cache(); + DBEnum::flushCache(); $do1 = new TestObject(); $fields = $schema->databaseFields(TestObject::class, false); $this->assertEquals("DBClassName", $fields['ClassName']); @@ -224,7 +224,7 @@ class DataObjectSchemaGenerationTest extends SapphireTest // Test with instance of subclass $item1 = new TestIndexObject(); $item1->write(); - DBClassName::clear_classname_cache(); + DBEnum::flushCache(); $this->assertEquals( [ TestObject::class, @@ -237,7 +237,7 @@ class DataObjectSchemaGenerationTest extends SapphireTest // Test with instance of main class $item2 = new TestObject(); $item2->write(); - DBClassName::clear_classname_cache(); + DBEnum::flushCache(); $this->assertEquals( [ TestObject::class, @@ -252,7 +252,7 @@ class DataObjectSchemaGenerationTest extends SapphireTest $item1->write(); $item2 = new TestObject(); $item2->write(); - DBClassName::clear_classname_cache(); + DBEnum::flushCache(); $this->assertEquals( [ TestObject::class, diff --git a/tests/php/ORM/DataObjectSchemaGenerationTest/TestIndexObject.php b/tests/php/ORM/DataObjectSchemaGenerationTest/TestIndexObject.php index 72032715e..d0d7134a2 100644 --- a/tests/php/ORM/DataObjectSchemaGenerationTest/TestIndexObject.php +++ b/tests/php/ORM/DataObjectSchemaGenerationTest/TestIndexObject.php @@ -8,7 +8,7 @@ class TestIndexObject extends TestObject implements TestOnly { private static $table_name = 'DataObjectSchemaGenerationTest_IndexDO'; private static $db = [ - 'Title' => 'Varchar(255)', + 'Title' => 'Varchar(192)', 'Content' => 'Text', ]; diff --git a/tests/php/ORM/FieldType/DBEnumTestObject.php b/tests/php/ORM/FieldType/DBEnumTestObject.php new file mode 100644 index 000000000..b9c70d71c --- /dev/null +++ b/tests/php/ORM/FieldType/DBEnumTestObject.php @@ -0,0 +1,15 @@ + 'Enum("Red,Blue,Green")', + ]; +} diff --git a/tests/php/Security/SecurityTest.php b/tests/php/Security/SecurityTest.php index 9ac158758..1c81911d4 100644 --- a/tests/php/Security/SecurityTest.php +++ b/tests/php/Security/SecurityTest.php @@ -14,7 +14,7 @@ use SilverStripe\Dev\FunctionalTest; use SilverStripe\i18n\i18n; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; -use SilverStripe\ORM\FieldType\DBClassName; +use SilverStripe\ORM\FieldType\DBEnum; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\FieldType\DBField; use SilverStripe\ORM\ValidationResult; @@ -671,7 +671,7 @@ class SecurityTest extends FunctionalTest public function testDatabaseIsReadyWithInsufficientMemberColumns() { Security::clear_database_is_ready(); - DBClassName::clear_classname_cache(); + DBEnum::flushCache(); // Assumption: The database has been built correctly by the test runner, // and has all columns present in the ORM