API Extensions are now stateless

ENHANCEMENT Injector now lazy-loads services more intelligently

docs/en/04_Changelogs/4.0.0.md

@@ -578,6 +578,69 @@ In some places it may still be necessary to access the session object where no r
 In rare cases it is still possible to access the request of the current controller via
 `Controller::curr()->getRequest()` to gain access to the current session.
 
+#### Upgrade extensions to work as singletons
+
+In SilverStripe 4.0 extensions are now all singletons, meaning that state stored as protected vars
+within extensions are now shared across all object instances that use this extension.
+
+As a result, you must take care that all protected vars are either refactored to be stored against
+the owner object, or are keyed in a fashion distinct to the parent.
+
+E.g.
+
+```diff
+class MyExtension extends Extension {
+    public function getContent() {
+-        if (!$this->contentCache) {
+-            $this->contentCache = $this->generateContent();
+-        }
+-        return $this->contentCache;
++        $contentCache = $this->owner->getField('contentCache');
++        if (!$contentCache) {
++            $contentCache = $this->generateContent();
++            $this->owner->setField('contentCache', $contentCache);
++        }
++        return $contentCache;
+    }
+}
+```
+
+In addition when using extensions with distinct constructor arguments it's advisable to
+use `yml` to register you those constructor arguments and use a service name or alias
+in `private static $extensions`.
+
+E.g. this is the service definition for the various versioned extension variants
+
+```yaml
+---
+Name: versionedextension
+---
+SilverStripe\Core\Injector\Injector:
+  # Versioning only
+  SilverStripe\Versioned\Versioned.versioned:
+    class: SilverStripe\Versioned\Versioned
+    constructor:
+      mode: Versioned
+  # Staging and Versioning
+  SilverStripe\Versioned\Versioned.stagedversioned:
+    class: SilverStripe\Versioned\Versioned
+    constructor:
+      mode: StagedVersioned
+  # Default is alias for .stagedversioned
+  SilverStripe\Versioned\Versioned: '%$SilverStripe\Versioned\Versioned.stagedversioned'
+```
+
+Upgrade your extension references as below:
+
+```diff
+class MyClass extends DataObject {
+    private static $extensions = [
+-        Versioned::class . '(Versioned)',
++        Versioned::class . '.versioned',
+    ];
+}
+```
+
 #### Compatibility with the new front-end building tools
 
 If you are using Requirements from 3.x then your scripts should continue to work as they did before.
@@ -1297,16 +1360,20 @@ Rather than declaring the list of stages a model has, the constructor for `Versi
 parameter, which declares whether or not the model is versioned and has a draft and live stage, or alternatively
 if it only has versioning without staging.
 
+Each form of this extension is registered under the appropriate service identifier, which you should use in your
+model as below:
+
 ```php
 use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\Versioning\Versioned;
 
 /**
  * This model has staging and versioning. Stages will be "Stage" and "Live"
  */
 class MyStagedModel extends SilverStripe\ORM\DataObject
 {
     private static $extensions = [
-        "SilverStripe\\ORM\\Versioning\\Versioned('StagedVersioned')"
+        Versioned::class . '.stagedversioned',
     ];
 }
 
@@ -1316,7 +1383,7 @@ class MyStagedModel extends SilverStripe\ORM\DataObject
 class MyVersionedModel extends DataObject
 {
     private static $extensions = [
-        "SilverStripe\\ORM\\Versioning\\Versioned('Versioned')"
+        Versioned::class . '.versioned',
     ];
 }
 ```
@@ -1860,6 +1927,8 @@ that it belongs to, e.g. `themes/mytheme/templates/MyVendor/Foobar/Model/MyModel
 * `RequestFilter` has been deprecated in favour of
   [HTTPMiddleware](/developer_guides/controllers/middlewares). Also the legacy RequestFilter
   API has changed: $session and $dataModel variables removed from preRequest / postRequest.
+* `Extension` instances are now singletons and no longer are constructed once per extended object.
+  See the 'Upgrade extensions to work as singletons' section on this page for more information.
 
 #### <a name="overview-general-removed"></a>General and Core Removed API
 
@@ -1945,6 +2014,8 @@ that it belongs to, e.g. `themes/mytheme/templates/MyVendor/Foobar/Model/MyModel
   Accordingly, `hasService()` has been renamed to `has()`, and `get()` will throw
   `SilverStripe\Core\Injector\InjectorNotFoundException` when the service can't be found.
 * Removed `CustomMethods::createMethod()`. Use closures instead.
+* Removed `Extension::$ownerBaseClass` property. You should use $this->owner->baseClass() instead.
+  The second argument of `Extension::setOwner()` has also been removed.
 
 #### <a name="overview-general-deprecated"></a>General and Core Deprecated API
 

src/Core/ClassInfo.php

@@ -197,7 +197,7 @@ public static function subclassesFor($nameOrObject)
         // Merge with descendants
         $descendants = ClassLoader::inst()->getManifest()->getDescendantsOf($className);
         return array_merge(
-            [ $lowerClassName => $className ],
+            [$lowerClassName => $className],
             $descendants
         );
     }
@@ -441,6 +441,10 @@ public static function parse_class_spec($classSpec)
             } elseif (is_array($token) && $token[0] === T_NS_SEPARATOR) {
                 $class .= $token[1];
                 $hadNamespace = true;
+            } elseif ($token === '.') {
+                // Treat service name separator as NS separator
+                $class .= '.';
+                $hadNamespace = true;
             } elseif ($hadNamespace && is_array($token) && $token[0] === T_STRING) {
                 $class .= $token[1];
                 $hadNamespace = false;
@@ -454,7 +458,11 @@ public static function parse_class_spec($classSpec)
                                 $result = stripcslashes(substr($argString, 1, -1));
                                 break;
                             case "'":
-                                $result = str_replace(array("\\\\", "\\'"), array("\\", "'"), substr($argString, 1, -1));
+                                $result = str_replace(
+                                    ["\\\\", "\\'"],
+                                    ["\\", "'"],
+                                    substr($argString, 1, -1)
+                                );
                                 break;
                             default:
                                 throw new Exception("Bad T_CONSTANT_ENCAPSED_STRING arg $argString");
@@ -506,7 +514,7 @@ public static function parse_class_spec($classSpec)
             } else {
                 if ($tokenName === '[') {
                     $result = array();
-                } elseif (($tokenName === ')' || $tokenName === ']') && ! empty($bucketStack)) {
+                } elseif (($tokenName === ')' || $tokenName === ']') && !empty($bucketStack)) {
                     // Store the bucket we're currently working on
                     $oldBucket = $bucket;
                     // Fetch the key for the bucket at the top of the stack

src/Core/Config/Middleware/ExtensionMiddleware.php

@@ -64,6 +64,8 @@ protected function getExtraConfig($class, $classConfig, $excludeMiddleware)
         $extensions = $extensionSourceConfig['extensions'];
         foreach ($extensions as $extension) {
             list($extensionClass, $extensionArgs) = ClassInfo::parse_class_spec($extension);
+            // Strip service name specifier
+            $extensionClass = strtok($extensionClass, '.');
             if (!class_exists($extensionClass)) {
                 throw new InvalidArgumentException("$class references nonexistent $extensionClass in 'extensions'");
             }

src/Core/CustomMethods.php

@@ -67,27 +67,30 @@ public function __call($method, $arguments)
                     $this->{$property}[$index] :
                     $this->{$property};
 
-                if ($obj) {
-                    if (!empty($config['callSetOwnerFirst'])) {
-                        /** @var Extension $obj */
-                        $obj->setOwner($this);
-                    }
-                    $retVal = call_user_func_array(array($obj, $method), $arguments);
-                    if (!empty($config['callSetOwnerFirst'])) {
-                        /** @var Extension $obj */
-                        $obj->clearOwner();
-                    }
-                    return $retVal;
+                if (!$obj) {
+                    throw new BadMethodCallException(
+                        "Object->__call(): {$class} cannot pass control to {$property}({$index})."
+                        . ' Perhaps this object was mistakenly destroyed?'
+                    );
                 }
 
-                throw new BadMethodCallException(
-                    "Object->__call(): {$class} cannot pass control to {$property}({$index})."
-                    . ' Perhaps this object was mistakenly destroyed?'
-                );
+                // Call without setOwner
+                if (empty($config['callSetOwnerFirst'])) {
+                    return $obj->$method(...$arguments);
+                }
+
+                /** @var Extension $obj */
+                try {
+                    $obj->setOwner($this);
+                    return $obj->$method(...$arguments);
+                } finally {
+                    $obj->clearOwner();
+                }
             }
             case isset($config['wrap']): {
                 array_unshift($arguments, $config['method']);
-                return call_user_func_array(array($this, $config['wrap']), $arguments);
+                $wrapped = $config['wrap'];
+                return $this->$wrapped(...$arguments);
             }
             case isset($config['function']): {
                 return $config['function']($this, $arguments);
@@ -192,11 +195,14 @@ protected function findMethodsFromExtension($extension)
     {
         if (method_exists($extension, 'allMethodNames')) {
             if ($extension instanceof Extension) {
-                $extension->setOwner($this);
-            }
-            $methods = $extension->allMethodNames(true);
-            if ($extension instanceof Extension) {
-                $extension->clearOwner();
+                try {
+                    $extension->setOwner($this);
+                    $methods = $extension->allMethodNames(true);
+                } finally {
+                    $extension->clearOwner();
+                }
+            } else {
+                $methods = $extension->allMethodNames(true);
             }
         } else {
             $class = get_class($extension);

src/Core/Extensible.php

@@ -139,8 +139,8 @@ protected function defineExtensionMethods()
                 $this->addCallbackMethod($method, function ($inst, $args) use ($method, $extensionClass) {
                     /** @var Extensible $inst */
                     $extension = $inst->getExtensionInstance($extensionClass);
-                    $extension->setOwner($inst);
                     try {
+                        $extension->setOwner($inst);
                         return call_user_func_array([$extension, $method], $args);
                     } finally {
                         $extension->clearOwner();
@@ -343,6 +343,8 @@ public static function get_extra_config_sources($class = null)
 
         foreach ($extensions as $extension) {
             list($extensionClass, $extensionArgs) = ClassInfo::parse_class_spec($extension);
+            // Strip service name specifier
+            $extensionClass = strtok($extensionClass, '.');
             $sources[] = $extensionClass;
 
             if (!class_exists($extensionClass)) {
@@ -465,8 +467,8 @@ public function extend($method, &$a1 = null, &$a2 = null, &$a3 = null, &$a4 = nu
 
         foreach ($this->getExtensionInstances() as $instance) {
             if (method_exists($instance, $method)) {
-                $instance->setOwner($this);
                 try {
+                    $instance->setOwner($this);
                     $value = $instance->$method($a1, $a2, $a3, $a4, $a5, $a6, $a7);
                 } finally {
                     $instance->clearOwner();
@@ -551,8 +553,7 @@ public function getExtensionInstances()
 
             if ($extensions) {
                 foreach ($extensions as $extension) {
-                    $instance = Injector::inst()->create($extension);
-                    $instance->setOwner(null, $class);
+                    $instance = Injector::inst()->get($extension);
                     $this->extension_instances[get_class($instance)] = $instance;
                 }
             }

src/Core/Extension.php

@@ -30,15 +30,7 @@ abstract class Extension
     protected $owner;
 
     /**
-     * The base class that this extension was applied to; $this->owner must be one of these
-     *
-     * @var DataObject
-     */
-    protected $ownerBaseClass;
-
-    /**
-     * Ownership stack for recursive methods.
-     * Last item is current owner.
+     * Stack of all parent owners, not including current owner
      *
      * @var array
      */
@@ -63,24 +55,29 @@ public static function add_to_class($class, $extensionClass, $args = null)
     /**
      * Set the owner of this extension.
      *
-     * @param object $owner The owner object,
-     * @param string $ownerBaseClass The base class that the extension is applied to; this may be
-     * the class of owner, or it may be a parent.  For example, if Versioned was applied to SiteTree,
-     * and then a Page object was instantiated, $owner would be a Page object, but $ownerBaseClass
-     * would be 'SiteTree'.
+     * @param object $owner The owner object
      */
-    public function setOwner($owner, $ownerBaseClass = null)
+    public function setOwner($owner)
     {
-        if ($owner) {
-            $this->ownerStack[] = $owner;
-        }
+        $this->ownerStack[] = $this->owner;
         $this->owner = $owner;
+    }
 
-        // Set ownerBaseClass
-        if ($ownerBaseClass) {
-            $this->ownerBaseClass = $ownerBaseClass;
-        } elseif (!$this->ownerBaseClass && $owner) {
-            $this->ownerBaseClass = get_class($owner);
+    /**
+     * Temporarily modify the owner. The original owner is ensured to be restored
+     *
+     * @param mixed $owner Owner to set
+     * @param callable $callback Callback to invoke
+     * @param array $args Args to pass to callback
+     * @return mixed
+     */
+    public function withOwner($owner, callable $callback, $args = [])
+    {
+        try {
+            $this->setOwner($owner);
+            return $callback(...$args);
+        } finally {
+            $this->clearOwner();
         }
     }
 
@@ -92,12 +89,7 @@ public function clearOwner()
         if (empty($this->ownerStack)) {
             throw new BadMethodCallException("clearOwner() called more than setOwner()");
         }
-        array_pop($this->ownerStack);
-        if ($this->ownerStack) {
-            $this->owner = end($this->ownerStack);
-        } else {
-            $this->owner = null;
-        }
+        $this->owner = array_pop($this->ownerStack);
     }
 
     /**
@@ -120,7 +112,7 @@ public function getOwner()
      */
     public static function get_classname_without_arguments($extensionStr)
     {
-        $parts = explode('(', $extensionStr);
-        return $parts[0];
+        // Split out both args and service name
+        return strtok(strtok($extensionStr, '('), '.');
     }
 }