Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

BUG Prevent unnecessary reconstruction of ClassName field #2916

Merged
merged 1 commit into from

2 participants

@tractorcow
Collaborator

DataObject::database_fields ensures that any ClassName of obsolete classes with existing records are maintained when determining the classes for the 'ClassName' Enum field.

The problem is that the ClassName values retrieved from the database are unnecessarily triggering a re-order of the ClassName field on every dev/build. This results in the occasional ClassName regeneration, with no changes other than the order of the enum values. This is most apparent in a new website, where the database is generated, and default records are created. Those default records then necessitate the rebuild of that field, depending on the order they were created. Anyone who's started a new blank project and run two concurrent dev/builds will know what I'm talking about.

This fix ensures that for all known classes, these values will always be present at the beginning of the spec, and the order is determined by the code, rather than the much more volatile values stored in the database.

@simonwelsh simonwelsh merged commit ccaca9a into from
@tractorcow tractorcow deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 3, 2014
  1. @tractorcow
This page is out of date. Refresh to see the latest.
View
2  model/DataObject.php
@@ -237,7 +237,7 @@ public static function database_fields($class) {
$db = DB::getConn();
if($db->hasField($class, 'ClassName')) {
$existing = $db->query("SELECT DISTINCT \"ClassName\" FROM \"$class\"")->column();
- $classNames = array_unique(array_merge($existing, $classNames));
+ $classNames = array_unique(array_merge($classNames, $existing));
}
self::$classname_spec_cache[$class] = "Enum('" . implode(', ', $classNames) . "')";
View
51 tests/model/DataObjectSchemaGenerationTest.php
@@ -124,6 +124,57 @@ public function testIndexesRerequestChanges() {
// Restore old indexes
Config::unnest();
}
+
+ /**
+ * Tests the generation of the ClassName spec and ensure it's not unnecessarily influenced
+ * by the order of classnames of existing records
+ */
+ public function testClassNameSpecGeneration() {
+
+ // Test with blank entries
+ DataObject::clear_classname_spec_cache();
+ $fields = DataObject::database_fields('DataObjectSchemaGenerationTest_DO');
+ $this->assertEquals(
+ "Enum('DataObjectSchemaGenerationTest_DO, DataObjectSchemaGenerationTest_IndexDO')",
+ $fields['ClassName']
+ );
+
+ // Test with instance of subclass
+ $item1 = new DataObjectSchemaGenerationTest_IndexDO();
+ $item1->write();
+ DataObject::clear_classname_spec_cache();
+ $fields = DataObject::database_fields('DataObjectSchemaGenerationTest_DO');
+ $this->assertEquals(
+ "Enum('DataObjectSchemaGenerationTest_DO, DataObjectSchemaGenerationTest_IndexDO')",
+ $fields['ClassName']
+ );
+ $item1->delete();
+
+ // Test with instance of main class
+ $item2 = new DataObjectSchemaGenerationTest_DO();
+ $item2->write();
+ DataObject::clear_classname_spec_cache();
+ $fields = DataObject::database_fields('DataObjectSchemaGenerationTest_DO');
+ $this->assertEquals(
+ "Enum('DataObjectSchemaGenerationTest_DO, DataObjectSchemaGenerationTest_IndexDO')",
+ $fields['ClassName']
+ );
+ $item2->delete();
+
+ // Test with instances of both classes
+ $item1 = new DataObjectSchemaGenerationTest_IndexDO();
+ $item1->write();
+ $item2 = new DataObjectSchemaGenerationTest_DO();
+ $item2->write();
+ DataObject::clear_classname_spec_cache();
+ $fields = DataObject::database_fields('DataObjectSchemaGenerationTest_DO');
+ $this->assertEquals(
+ "Enum('DataObjectSchemaGenerationTest_DO, DataObjectSchemaGenerationTest_IndexDO')",
+ $fields['ClassName']
+ );
+ $item1->delete();
+ $item2->delete();
+ }
}
class DataObjectSchemaGenerationTest_DO extends DataObject implements TestOnly {
Something went wrong with that request. Please try again.