Merge pull request #8465 from creative-commoners/pulls/4.3/extensible-bug

NEW Add method to flush extra_methods static cache data and implement into test state

src/Core/Extensible.php

@@ -3,12 +3,9 @@
 namespace SilverStripe\Core;
 
 use InvalidArgumentException;
-use SilverStripe\Control\RequestHandler;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Dev\Deprecation;
-use SilverStripe\ORM\DataExtension;
-use SilverStripe\ORM\DataObject;
 use SilverStripe\View\ViewableData;
 
 /**
@@ -214,6 +211,14 @@ trait Extensible
         return true;
     }
 
+    /**
+     * Clears all cached extra_methods cache data
+     */
+    public static function flush_extra_methods_cache()
+    {
+        self::$extra_methods = [];
+    }
+
 
     /**
      * Remove an extension from a class.

src/Dev/State/ExtensionTestState.php

@@ -13,16 +13,6 @@ use SilverStripe\ORM\DataObject;
  */
 class ExtensionTestState implements TestState
 {
-    /**
-     * @var array
-     */
-    protected $extensionsToReapply = [];
-
-    /**
-     * @var array
-     */
-    protected $extensionsToRemove = [];
-
     /**
      * Called on setup
      *
@@ -30,6 +20,7 @@ class ExtensionTestState implements TestState
      */
     public function setUp(SapphireTest $test)
     {
+        DataObject::flush_extra_methods_cache();
     }
 
     public function tearDown(SapphireTest $test)
@@ -40,8 +31,6 @@ class ExtensionTestState implements TestState
     {
         // May be altered by another class
         $isAltered = false;
-        $this->extensionsToReapply = [];
-        $this->extensionsToRemove = [];
 
         /** @var string|SapphireTest $class */
         /** @var string|DataObject $dataClass */
@@ -57,10 +46,6 @@ class ExtensionTestState implements TestState
                 if (!class_exists($extension) || !$dataClass::has_extension($extension)) {
                     continue;
                 }
-                if (!isset($this->extensionsToReapply[$dataClass])) {
-                    $this->extensionsToReapply[$dataClass] = [];
-                }
-                $this->extensionsToReapply[$dataClass][] = $extension;
                 $dataClass::remove_extension($extension);
                 $isAltered = true;
             }
@@ -77,10 +62,6 @@ class ExtensionTestState implements TestState
                     throw new LogicException("Test {$class} requires extension {$extension} which doesn't exist");
                 }
                 if (!$dataClass::has_extension($extension)) {
-                    if (!isset($this->extensionsToRemove[$dataClass])) {
-                        $this->extensionsToRemove[$dataClass] = [];
-                    }
-                    $this->extensionsToRemove[$dataClass][] = $extension;
                     $dataClass::add_extension($extension);
                     $isAltered = true;
                 }
@@ -104,23 +85,5 @@ class ExtensionTestState implements TestState
 
     public function tearDownOnce($class)
     {
-        // @todo: This isn't strictly necessary to restore extensions, but only to ensure that
-        // Object::$extra_methods is properly flushed. This should be replaced with a simple
-        // flush mechanism for each $class.
-        /** @var string|DataObject $dataClass */
-
-        // Remove extensions added for testing
-        foreach ($this->extensionsToRemove as $dataClass => $extensions) {
-            foreach ($extensions as $extension) {
-                $dataClass::remove_extension($extension);
-            }
-        }
-
-        // Reapply ones removed
-        foreach ($this->extensionsToReapply as $dataClass => $extensions) {
-            foreach ($extensions as $extension) {
-                $dataClass::add_extension($extension);
-            }
-        }
     }
 }

tests/php/Core/Injector/InjectorTest.php

@@ -21,12 +21,13 @@ use SilverStripe\Core\Tests\Injector\InjectorTest\NeedsBothCirculars;
 use SilverStripe\Core\Tests\Injector\InjectorTest\NewRequirementsBackend;
 use SilverStripe\Core\Tests\Injector\InjectorTest\OriginalRequirementsBackend;
 use SilverStripe\Core\Tests\Injector\InjectorTest\OtherTestObject;
+use SilverStripe\Core\Tests\Injector\InjectorTest\SomeCustomisedExtension;
+use SilverStripe\Core\Tests\Injector\InjectorTest\SomeExtension;
 use SilverStripe\Core\Tests\Injector\InjectorTest\TestObject;
 use SilverStripe\Core\Tests\Injector\InjectorTest\TestSetterInjections;
 use SilverStripe\Core\Tests\Injector\InjectorTest\TestStaticInjections;
 use SilverStripe\Dev\SapphireTest;
-use SilverStripe\Dev\TestOnly;
+use SilverStripe\Security\Member;
-use stdClass;
 
 define('TEST_SERVICES', __DIR__ . '/AopProxyServiceTest');
 
@@ -1047,4 +1048,24 @@ class InjectorTest extends SapphireTest
         Injector::unnest();
         $this->nestingLevel--;
     }
+
+    /**
+     * Tests that overloaded extensions work, see {@link Extensible::getExtensionInstance()}
+     */
+    public function testExtendedExtensions()
+    {
+        Config::modify()
+            ->set(Injector::class, SomeExtension::class, [
+                'class' => SomeCustomisedExtension::class,
+            ])
+            ->merge(Member::class, 'extensions', [
+                SomeExtension::class,
+            ]);
+
+        /** @var Member|SomeExtension $member */
+        $member = new Member();
+        $this->assertTrue($member->hasExtension(SomeExtension::class));
+        $this->assertTrue($member->hasMethod('someMethod'));
+        $this->assertSame('bar', $member->someMethod());
+    }
 }

tests/php/Core/Injector/InjectorTest/SomeCustomisedExtension.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace SilverStripe\Core\Tests\Injector\InjectorTest;
+
+use SilverStripe\Dev\TestOnly;
+
+class SomeCustomisedExtension extends SomeExtension implements TestOnly
+{
+    public function someMethod()
+    {
+        return 'bar';
+    }
+}

tests/php/Core/Injector/InjectorTest/SomeExtension.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace SilverStripe\Core\Tests\Injector\InjectorTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataExtension;
+
+class SomeExtension extends DataExtension implements TestOnly
+{
+    public function someMethod()
+    {
+        return 'foo';
+    }
+}