Merge pull request #9952 from creative-commoners/pulls/4.7/allow-class-to-inject-over-parent

All works perfect.

src/ORM/Connect/TableBuilder.php

@@ -0,0 +1,69 @@
+<?php
+
+namespace SilverStripe\ORM\Connect;
+
+use SilverStripe\Control\Director;
+use SilverStripe\Core\Injector\Injectable;
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\DB;
+
+class TableBuilder
+{
+    use Injectable;
+
+    public function buildTables(DBSchemaManager $dbSchema, array $dataClasses, array $extraDataObjects = [], bool $quiet = false, bool $testMode = false, bool $showRecordCounts = false)
+    {
+        $dbSchema->schemaUpdate(function () use ($dataClasses, $extraDataObjects, $testMode, $quiet, $showRecordCounts) {
+            $dataObjectSchema = DataObject::getSchema();
+
+            foreach ($dataClasses as $dataClass) {
+                // Check if class exists before trying to instantiate - this sidesteps any manifest weirdness
+                if (!class_exists($dataClass)) {
+                    continue;
+                }
+
+                // Check if this class should be excluded as per testing conventions
+                /** @var DataObject $SNG */
+                $SNG = new $dataClass([], DataObject::CREATE_SINGLETON);
+                if (!$testMode && $SNG instanceof TestOnly) {
+                    continue;
+                }
+
+                // Log data
+                if (!$quiet) {
+                    $tableName = $dataObjectSchema->tableName($dataClass);
+                    if ($showRecordCounts && DB::get_schema()->hasTable($tableName)) {
+                        try {
+                            $count = DB::query("SELECT COUNT(*) FROM \"$tableName\"")->value();
+                            $countSuffix = " ($count records)";
+                        } catch (\Exception $e) {
+                            $countSuffix = " (error getting record count)";
+                        }
+                    } else {
+                        $countSuffix = "";
+                    }
+
+                    if (Director::is_cli()) {
+                        echo " * $tableName$countSuffix\n";
+                    } else {
+                        echo "<li>$tableName$countSuffix</li>\n";
+                    }
+                }
+
+                // Instruct the class to apply its schema to the database
+                $SNG->requireTable();
+            }
+
+            // If we have additional dataobjects which need schema (i.e. for tests), do so here:
+            if ($extraDataObjects) {
+                foreach ($extraDataObjects as $dataClass) {
+                    $SNG = new $dataClass([], DataObject::CREATE_SINGLETON);
+                    if ($SNG instanceof DataObject) {
+                        $SNG->requireTable();
+                    }
+                }
+            }
+        });
+    }
+}

src/ORM/Connect/TempDatabase.php

@@ -244,29 +244,9 @@ class TempDatabase
 
         $schema = $this->getConn()->getSchemaManager();
         $schema->quiet();
-        $schema->schemaUpdate(
-            function () use ($dataClasses, $extraDataObjects) {
-                foreach ($dataClasses as $dataClass) {
-                    // Check if class exists before trying to instantiate - this sidesteps any manifest weirdness
-                    if (class_exists($dataClass ?? '')) {
-                        $SNG = singleton($dataClass);
-                        if (!($SNG instanceof TestOnly)) {
-                            $SNG->requireTable();
-                        }
-                    }
-                }
 
-                // If we have additional dataobjects which need schema, do so here:
+        $tableBuilder = TableBuilder::singleton();
-                if ($extraDataObjects) {
+        $tableBuilder->buildTables($schema, $dataClasses, $extraDataObjects, true);
-                    foreach ($extraDataObjects as $dataClass) {
-                        $SNG = singleton($dataClass);
-                        if (singleton($dataClass) instanceof DataObject) {
-                            $SNG->requireTable();
-                        }
-                    }
-                }
-            }
-        );
 
         Config::modify()->set(DBSchemaManager::class, 'check_and_repair_on_build', $oldCheckAndRepairOnBuild);
 

src/ORM/DatabaseAdmin.php

@@ -14,6 +14,7 @@ use SilverStripe\Dev\Deprecation;
 use SilverStripe\Dev\DevelopmentAdmin;
 use SilverStripe\Dev\TestOnly;
 use SilverStripe\ORM\Connect\DatabaseException;
+use SilverStripe\ORM\Connect\TableBuilder;
 use SilverStripe\ORM\FieldType\DBClassName;
 use SilverStripe\Security\Permission;
 use SilverStripe\Security\Security;
@@ -304,46 +305,8 @@ class DatabaseAdmin extends Controller
 
         // Initiate schema update
         $dbSchema = DB::get_schema();
-        $dbSchema->schemaUpdate(function () use ($dataClasses, $testMode, $quiet, $showRecordCounts) {
+        $tableBuilder = TableBuilder::singleton();
-            $dataObjectSchema = DataObject::getSchema();
+        $tableBuilder->buildTables($dbSchema, $dataClasses, [], $quiet, $testMode, $showRecordCounts);
-
-            foreach ($dataClasses as $dataClass) {
-                // Check if class exists before trying to instantiate - this sidesteps any manifest weirdness
-                if (!class_exists($dataClass ?? '')) {
-                    continue;
-                }
-
-                // Check if this class should be excluded as per testing conventions
-                $SNG = singleton($dataClass);
-                if (!$testMode && $SNG instanceof TestOnly) {
-                    continue;
-                }
-                $tableName = $dataObjectSchema->tableName($dataClass);
-
-                // Log data
-                if (!$quiet) {
-                    if ($showRecordCounts && DB::get_schema()->hasTable($tableName)) {
-                        try {
-                            $count = DB::query("SELECT COUNT(*) FROM \"$tableName\"")->value();
-                            $countSuffix = " ($count records)";
-                        } catch (Exception $e) {
-                            $countSuffix = " (error getting record count)";
-                        }
-                    } else {
-                        $countSuffix = "";
-                    }
-
-                    if (Director::is_cli()) {
-                        echo " * $tableName$countSuffix\n";
-                    } else {
-                        echo "<li>$tableName$countSuffix</li>\n";
-                    }
-                }
-
-                // Instruct the class to apply its schema to the database
-                $SNG->requireTable();
-            }
-        });
         ClassInfo::reset_db_cache();
 
         if (!$quiet && !Director::is_cli()) {

tests/php/ORM/DataObjectTest.php

@@ -5,6 +5,7 @@ namespace SilverStripe\ORM\Tests;
 use InvalidArgumentException;
 use LogicException;
 use SilverStripe\Core\Config\Config;
+use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\i18n\i18n;
 use SilverStripe\ORM\Connect\MySQLDatabase;
@@ -63,11 +64,14 @@ class DataObjectTest extends SapphireTest
         DataObjectTest\RelationChildSecond::class,
         DataObjectTest\MockDynamicAssignmentDataObject::class,
         DataObjectTest\TreeNode::class,
+        DataObjectTest\OverriddenDataObject::class,
+        DataObjectTest\InjectedDataObject::class,
     ];
 
     protected function setUp(): void
     {
         parent::setUp();
+        Config::modify()->merge(Injector::class, DataObjectTest\OverriddenDataObject::class, ['class' => DataObjectTest\InjectedDataObject::class]);
 
         $validator = Member::password_validator();
         if ($validator) {
@@ -186,6 +190,26 @@ class DataObjectTest extends SapphireTest
         );
     }
 
+    public function testTableBuiltForInjectedDataObject()
+    {
+        // Test we get the correct injected class
+        $obj = DataObjectTest\OverriddenDataObject::create();
+        $this->assertSame(DataObjectTest\InjectedDataObject::class, get_class($obj));
+
+        // Test both tables are built
+        $schema = DataObject::getSchema();
+        $this->assertTrue($schema->classHasTable(DataObjectTest\OverriddenDataObject::class));
+        $this->assertTrue($schema->classHasTable(DataObjectTest\InjectedDataObject::class));
+
+        // Test fields from both the overridden and injected class exist
+        $obj->EmploymentType = 'Some type';
+        $obj->NewField = 'Some value';
+        $obj->write();
+        $objFromOrm = DataObjectTest\OverriddenDataObject::get()->first();
+        $this->assertSame('Some type', $objFromOrm->EmploymentType);
+        $this->assertSame('Some value', $objFromOrm->NewField);
+    }
+
     public function testConstructAcceptsValues()
     {
         // Values can be an array...

tests/php/ORM/DataObjectTest/InjectedDataObject.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DataObjectTest;
+
+use SilverStripe\Dev\TestOnly;
+
+class InjectedDataObject extends OverriddenDataObject implements TestOnly
+{
+    private static $db = [
+        'NewField' => 'Varchar',
+    ];
+}

tests/php/ORM/DataObjectTest/OverriddenDataObject.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DataObjectTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+
+class OverriddenDataObject extends DataObject implements TestOnly
+{
+    private static $table_name = 'DataObjectTest_OverriddenDataObject';
+
+    private static $db = [
+        'Salary' => 'BigInt',
+        'EmploymentType' => 'Varchar',
+    ];
+
+    private static $has_one = [
+        'CurrentCompany' => Company::class,
+    ];
+}