Lazy-load custom methods and extensions on CustomMethods and Extensible traits

No longer need constructExtensions()

docs/en/04_Changelogs/4.0.0.md

@@ -429,8 +429,9 @@ Object has been superseded by a trio of traits which replace components of this
 
  - Injectable: Provides `MyClass::create()` and `MyClass::singleton()`
  - Configurable: Provides `MyClass::config()`
- - Extensible: Provides all methods related to extensions (E.g. add_extension()). Note:
+ - Extensible: Provides all methods related to extensions (E.g. add_extension()).
-   All classes which use this trait MUST invoke `constructExtensions()` in its constructor.
+ 
+All custom method and extension instances are constructed lazily.
  
 In particular specific Object class usages should be replaced as below:
 
@@ -450,12 +451,6 @@ Upgrade subclasses
 +    use Extensible;
 +    use Injectable;
 +    use Configurable;
-+    
-+    public function __construct()
-+    {
-+        // Only needed if using Extensible trait
-+        $this->constructExtensions();
-+    }
 +}
 ```
 

src/Control/Director.php

@@ -108,7 +108,6 @@ class Director implements TemplateGlobalProvider
 
     public function __construct()
     {
-        $this->constructExtensions();
     }
 
     /**

src/Core/CustomMethods.php

@@ -4,6 +4,7 @@ namespace SilverStripe\Core;
 
 use BadMethodCallException;
 use InvalidArgumentException;
+use SilverStripe\Dev\Deprecation;
 
 /**
  * Allows an object to declare a set of custom methods
@@ -16,7 +17,7 @@ trait CustomMethods
      *
      * @var array
      */
-    protected static $extra_methods = array();
+    protected static $extra_methods = [];
 
     /**
      * Name of methods to invoke by defineMethods for this instance
@@ -48,10 +49,6 @@ trait CustomMethods
         // If the method cache was cleared by an an Object::add_extension() / Object::remove_extension()
         // call, then we should rebuild it.
         $class = static::class;
-        if (!array_key_exists($class, self::$extra_methods)) {
-            $this->defineMethods();
-        }
-
         $config = $this->getExtraMethodConfig($method);
         if (empty($config)) {
             throw new BadMethodCallException(
@@ -60,6 +57,9 @@ trait CustomMethods
         }
 
         switch (true) {
+            case isset($config['callback']): {
+                return $config['callback']($this, $arguments);
+            }
             case isset($config['property']) : {
                 $obj = $config['index'] !== null ?
                     $this->{$config['property']}[$config['index']] :
@@ -89,20 +89,21 @@ trait CustomMethods
                     );
                 }
             }
-            case isset($config['wrap']):
+            case isset($config['wrap']): {
                 array_unshift($arguments, $config['method']);
                 return call_user_func_array(array($this, $config['wrap']), $arguments);
-
+            }
-                case isset($config['function']):
+            case isset($config['function']): {
                 return $config['function']($this, $arguments);
-
+            }
-                default:
+            default: {
                 throw new BadMethodCallException(
                     "Object->__call(): extra method $method is invalid on $class:"
                     . var_export($config, true)
                 );
             }
         }
+    }
 
     /**
      * Adds any methods from {@link Extension} instances attached to this object.
@@ -156,9 +157,13 @@ trait CustomMethods
      */
     protected function getExtraMethodConfig($method)
     {
-        $class = static::class;
+        // Lazy define methods
-        if (isset(self::$extra_methods[$class][strtolower($method)])) {
+        if (!isset(self::$extra_methods[static::class])) {
-            return self::$extra_methods[$class][strtolower($method)];
+            $this->defineMethods();
+        }
+
+        if (isset(self::$extra_methods[static::class][strtolower($method)])) {
+            return self::$extra_methods[static::class][strtolower($method)];
         }
         return null;
     }
@@ -228,6 +233,13 @@ trait CustomMethods
 
         $methods = $this->findMethodsFromExtension($extension);
         if ($methods) {
+            if ($extension instanceof Extension) {
+                Deprecation::notice(
+                    '5.0',
+                    'Register custom methods from extensions with addCallbackMethod.'
+                    . ' callSetOwnerFirst will be removed in 5.0'
+                );
+            }
             $methodInfo = array(
                 'property' => $property,
                 'index' => $index,
@@ -287,10 +299,23 @@ trait CustomMethods
      */
     protected function addWrapperMethod($method, $wrap)
     {
-        $class = static::class;
+        self::$extra_methods[static::class][strtolower($method)] = array(
-        self::$extra_methods[$class][strtolower($method)] = array (
             'wrap' => $wrap,
             'method' => $method
         );
     }
+
+    /**
+     * Add callback as a method.
+     *
+     * @param string $method Name of method
+     * @param callable $callback Callback to invoke.
+     * Note: $this is passed as first parameter to this callback and then $args as array
+     */
+    protected function addCallbackMethod($method, $callback)
+    {
+        self::$extra_methods[static::class][strtolower($method)] = [
+            'callback' => $callback,
+        ];
+    }
 }

src/Core/Extensible.php

@@ -6,29 +6,18 @@ 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;
 
 /**
  * Allows an object to have extensions applied to it.
- *
- * Bootstrap by calling $this->constructExtensions() in your class constructor.
- *
- * Requires CustomMethods trait
  */
 trait Extensible
 {
     use CustomMethods {
-        getExtraMethodConfig as getCustomMethodsConfig;
+        defineMethods as defineMethodsCustom;
-    }
-
-    protected function getExtraMethodConfig($method)
-    {
-        $config = $this->getCustomMethodsConfig($method);
-        // Ensure extension instances are populated before being accessed
-        if (isset($config['property']) && $config['property'] === 'extension_instances') {
-            $this->getExtensionInstances();
-        }
-        return $config;
     }
 
     /**
@@ -52,8 +41,6 @@ trait Extensible
      */
     private static $extensions = [];
 
-    private static $classes_constructed = array();
-
     /**
      * Classes that cannot be extended
      *
@@ -121,17 +108,20 @@ trait Extensible
         $this->afterExtendCallbacks[$method][] = $callback;
     }
 
+    /**
+     * @deprecated 4.0..5.0 Extensions and methods are now lazy-loaded
+     */
     protected function constructExtensions()
     {
-        // Register this trait as a method source
+        Deprecation::notice('5.0', 'constructExtensions does not need to be invoked and will be removed in 5.0');
-        $this->registerExtraMethodCallback('defineExtensionMethods', function () {
-            $this->defineExtensionMethods();
-        });
-
-        if (!isset(self::$classes_constructed[static::class])) {
-            $this->defineMethods();
-            self::$classes_constructed[static::class] = true;
     }
+
+    protected function defineMethods()
+    {
+        $this->defineMethodsCustom();
+
+        // Define extension methods
+        $this->defineExtensionMethods();
     }
 
     /**
@@ -139,16 +129,26 @@ trait Extensible
      * All these methods can then be called directly on the instance (transparently
      * mapped through {@link __call()}), or called explicitly through {@link extend()}.
      *
-     * @uses addMethodsFrom()
+     * @uses addCallbackMethod()
      */
     protected function defineExtensionMethods()
     {
         $extensions = $this->getExtensionInstances();
-        foreach (array_keys($extensions) as $key) {
+        foreach ($extensions as $extensionClass => $extensionInstance) {
-            $this->addMethodsFrom('extension_instances', $key);
+            foreach ($this->findMethodsFromExtension($extensionInstance) as $method) {
+                $this->addCallbackMethod($method, function ($inst, $args) use ($method, $extensionClass) {
+                    /** @var Extensible $inst */
+                    $extension = $inst->getExtensionInstance($extensionClass);
+                    $extension->setOwner($inst);
+                    try {
+                        return call_user_func_array([$extension, $method], $args);
+                    } finally {
+                        $extension->clearOwner();
+                    }
+                });
+            }
         }
     }
-
 
     /**
      * Add an extension to a specific class.
@@ -202,7 +202,6 @@ trait Extensible
 
         if ($subclasses) {
             foreach ($subclasses as $subclass) {
-                unset(self::$classes_constructed[$subclass]);
                 unset(self::$extra_methods[$subclass]);
             }
         }
@@ -215,8 +214,8 @@ trait Extensible
         Injector::inst()->unregisterNamedObject($class);
 
         // load statics now for DataObject classes
-        if (is_subclass_of($class, 'SilverStripe\\ORM\\DataObject')) {
+        if (is_subclass_of($class, DataObject::class)) {
-            if (!is_subclass_of($extensionClass, 'SilverStripe\\ORM\\DataExtension')) {
+            if (!is_subclass_of($extensionClass, DataExtension::class)) {
                 user_error("$extensionClass cannot be applied to $class without being a DataExtension", E_USER_ERROR);
             }
         }
@@ -272,7 +271,6 @@ trait Extensible
         $subclasses[] = $class;
         if ($subclasses) {
             foreach ($subclasses as $subclass) {
-                unset(self::$classes_constructed[$subclass]);
                 unset(self::$extra_methods[$subclass]);
             }
         }
@@ -468,11 +466,14 @@ trait Extensible
         foreach ($this->getExtensionInstances() as $instance) {
             if (method_exists($instance, $method)) {
                 $instance->setOwner($this);
+                try {
                     $value = $instance->$method($a1, $a2, $a3, $a4, $a5, $a6, $a7);
+                } finally {
+                    $instance->clearOwner();
+                }
                 if ($value !== null) {
                     $values[] = $value;
                 }
-                $instance->clearOwner();
             }
         }
 

src/Dev/BuildTask.php

@@ -22,7 +22,6 @@ abstract class BuildTask
 
     public function __construct()
     {
-        $this->constructExtensions();
     }
 
     /**

src/Forms/DefaultFormFactory.php

@@ -23,7 +23,6 @@ class DefaultFormFactory implements FormFactory
 
     public function __construct()
     {
-        $this->constructExtensions();
     }
 
     /**

src/Forms/FormTransformation.php

@@ -28,7 +28,6 @@ class FormTransformation
 
     public function __construct()
     {
-        $this->constructExtensions();
     }
 
     public function transform(FormField $field)

src/Forms/GridField/GridFieldConfig.php

@@ -38,7 +38,6 @@ class GridFieldConfig
     public function __construct()
     {
         $this->components = new ArrayList();
-        $this->constructExtensions();
     }
 
     /**

src/Forms/GridField/GridFieldDetailForm.php

@@ -84,7 +84,6 @@ class GridFieldDetailForm implements GridField_URLHandler
     public function __construct($name = 'DetailForm')
     {
         $this->name = $name;
-        $this->constructExtensions();
     }
 
     /**

src/Forms/GridField/GridFieldPrintButton.php

@@ -49,7 +49,6 @@ class GridFieldPrintButton implements GridField_HTMLProvider, GridField_ActionPr
     {
         $this->targetFragment = $targetFragment;
         $this->printColumns = $printColumns;
-        $this->constructExtensions();
     }
 
     /**

src/Forms/Validator.php

@@ -21,7 +21,6 @@ abstract class Validator
     public function __construct()
     {
         $this->resetResult();
-        $this->constructExtensions();
     }
 
     /**

src/Security/DefaultAdminService.php

@@ -37,7 +37,6 @@ class DefaultAdminService
 
     public function __construct()
     {
-        $this->constructExtensions();
     }
 
     /**

src/View/Parsers/ShortcodeParser.php

@@ -26,7 +26,6 @@ class ShortcodeParser
 
     public function __construct()
     {
-        $this->constructExtensions();
     }
 
     public function img_shortcode($attrs)

src/View/ViewableData.php

@@ -85,7 +85,6 @@ class ViewableData implements IteratorAggregate
 
     public function __construct()
     {
-        $this->constructExtensions();
     }
 
     // -----------------------------------------------------------------------------------------------------------------

tests/php/Core/ObjectTest.php

@@ -4,10 +4,8 @@ namespace SilverStripe\Core\Tests;
 
 use SilverStripe\Control\Controller;
 use SilverStripe\Core\ClassInfo;
-use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Extension;
 use SilverStripe\Core\Injector\Injector;
-use SilverStripe\Core\Manifest\ClassLoader;
 use SilverStripe\Core\Tests\ObjectTest\BaseObject;
 use SilverStripe\Core\Tests\ObjectTest\ExtendTest1;
 use SilverStripe\Core\Tests\ObjectTest\ExtendTest2;
@@ -54,7 +52,7 @@ class ObjectTest extends SapphireTest
         $objs[] = new ObjectTest\T2();
 
         // All these methods should exist and return true
-        $trueMethods = array('testMethod','otherMethod','someMethod','t1cMethod','normalMethod');
+        $trueMethods = array('testMethod','otherMethod','someMethod','t1cMethod','normalMethod', 'failoverCallback');
 
         foreach ($objs as $i => $obj) {
             foreach ($trueMethods as $method) {

tests/php/Core/ObjectTest/BaseObject.php

@@ -15,6 +15,5 @@ class BaseObject implements TestOnly
 
     public function __construct()
     {
-        $this->constructExtensions();
     }
 }

tests/php/Core/ObjectTest/T2.php

@@ -23,6 +23,9 @@ class T2 extends BaseObject
         $this->addMethodsFrom('failover');
         $this->addMethodsFrom('failoverArr', 0);
         $this->addMethodsFrom('failoverArr', 1);
+        $this->addCallbackMethod('failoverCallback', function ($inst, $args) {
+            return true;
+        });
     }
 
     public function wrappedMethod($val)

tests/php/ORM/ArrayListTest.php

@@ -941,9 +941,9 @@ class ArrayListTest extends SapphireTest
     {
         $list = new ArrayList(
             array(
-            array('Name' => 'Steve', 'ID' => 1, 'Age' => 21),
+            $steve = array('Name' => 'Steve', 'ID' => 1, 'Age' => 21),
             array('Name' => 'Bob', 'ID' => 2, 'Age' => 18),
-            array('Name' => 'Clair', 'ID' => 2, 'Age' => 21),
+            $clair = array('Name' => 'Clair', 'ID' => 2, 'Age' => 21),
             array('Name' => 'Oscar', 'ID' => 2, 'Age' => 52),
             array('Name' => 'Mike', 'ID' => 3, 'Age' => 43)
             )
@@ -955,13 +955,9 @@ class ArrayListTest extends SapphireTest
             }
         );
 
-        $expected = array(
-            new ArrayData(array('Name' => 'Steve', 'ID' => 1, 'Age' => 21)),
-            new ArrayData(array('Name' => 'Clair', 'ID' => 2, 'Age' => 21)),
-        );
-
         $this->assertEquals(2, $list->count());
-        $this->assertEquals($expected, $list->toArray(), 'List should only contain Steve and Clair');
+        $this->assertEquals($steve, $list[0]->toMap(), 'List should only contain Steve and Clair');
+        $this->assertEquals($clair, $list[1]->toMap(), 'List should only contain Steve and Clair');
         $this->assertTrue($list instanceof Filterable, 'The List should be of type SS_Filterable');
     }