diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md
index a9b21db12..c28eb73d7 100644
--- a/docs/en/04_Changelogs/4.0.0.md
+++ b/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,8 +1360,12 @@ 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"
@@ -1306,7 +1373,7 @@ use SilverStripe\ORM\DataObject;
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.
#### 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.
#### General and Core Deprecated API
diff --git a/src/Core/ClassInfo.php b/src/Core/ClassInfo.php
index bc3eaf30a..12e707d82 100644
--- a/src/Core/ClassInfo.php
+++ b/src/Core/ClassInfo.php
@@ -197,7 +197,7 @@ class ClassInfo
// Merge with descendants
$descendants = ClassLoader::inst()->getManifest()->getDescendantsOf($className);
return array_merge(
- [ $lowerClassName => $className ],
+ [$lowerClassName => $className],
$descendants
);
}
@@ -441,6 +441,10 @@ class ClassInfo
} 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 @@ class ClassInfo
$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 @@ class ClassInfo
} 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
diff --git a/src/Core/Config/Middleware/ExtensionMiddleware.php b/src/Core/Config/Middleware/ExtensionMiddleware.php
index ef64c2994..3edbc5b0d 100644
--- a/src/Core/Config/Middleware/ExtensionMiddleware.php
+++ b/src/Core/Config/Middleware/ExtensionMiddleware.php
@@ -64,6 +64,8 @@ class ExtensionMiddleware implements Middleware
$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'");
}
diff --git a/src/Core/CustomMethods.php b/src/Core/CustomMethods.php
index f0f3f45fc..91b3038ea 100644
--- a/src/Core/CustomMethods.php
+++ b/src/Core/CustomMethods.php
@@ -67,27 +67,30 @@ trait CustomMethods
$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 @@ trait CustomMethods
{
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);
diff --git a/src/Core/Extensible.php b/src/Core/Extensible.php
index 7eb30ab1b..e4bb1be1b 100644
--- a/src/Core/Extensible.php
+++ b/src/Core/Extensible.php
@@ -139,8 +139,8 @@ trait Extensible
$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 @@ trait Extensible
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 @@ trait Extensible
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 @@ trait Extensible
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;
}
}
diff --git a/src/Core/Extension.php b/src/Core/Extension.php
index 253ea0d6a..c7fe7f6c3 100644
--- a/src/Core/Extension.php
+++ b/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 @@ abstract class Extension
/**
* 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 @@ abstract class Extension
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 @@ abstract class Extension
*/
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, '('), '.');
}
}
diff --git a/src/Core/Injector/Injector.php b/src/Core/Injector/Injector.php
index 446b7d22a..583ed4534 100644
--- a/src/Core/Injector/Injector.php
+++ b/src/Core/Injector/Injector.php
@@ -193,6 +193,16 @@ class Injector implements ContainerInterface
*/
protected $configLocator;
+ /**
+ * Specify a service type singleton
+ */
+ const SINGLETON = 'singleton';
+
+ /**
+ * Specif ya service type prototype
+ */
+ const PROTOTYPE = 'prototype';
+
/**
* Create a new injector.
*
@@ -584,7 +594,7 @@ class Injector implements ContainerInterface
$type = isset($spec['type']) ? $spec['type'] : null;
}
- if ($id && (!$type || $type != 'prototype')) {
+ if ($id && (!$type || $type !== self::PROTOTYPE)) {
// this ABSOLUTELY must be set before the object is injected.
// This prevents circular reference errors down the line
$this->serviceCache[$id] = $object;
@@ -827,8 +837,8 @@ class Injector implements ContainerInterface
*/
public function getServiceName($name)
{
- // common case, get it overwith first
- if (isset($this->specs[$name])) {
+ // Lazy load in spec (disable inheritance to check exact service name)
+ if ($this->getServiceSpec($name, false)) {
return $name;
}
@@ -919,10 +929,9 @@ class Injector implements ContainerInterface
*
* @param string $name The name of the service to retrieve. If not a registered
* service, then a class of the given name is instantiated
- * @param boolean $asSingleton Whether to register the created object as a singleton
- * if no other configuration is found
- * @param array $constructorArgs Optional set of arguments to pass as constructor arguments
- * if this object is to be created from scratch (with $asSingleton = false)
+ * @param bool $asSingleton If set to false a new instance will be returned.
+ * If true a singleton will be returned unless the spec is type=prototype'
+ * @param array $constructorArgs Args to pass in to the constructor. Note: Ignored for singletons
* @return mixed Instance of the specified object
*/
public function get($name, $asSingleton = true, $constructorArgs = [])
@@ -939,70 +948,42 @@ class Injector implements ContainerInterface
/**
* Returns the service, or `null` if it doesnt' exist. See {@link get()} for main usage.
*
- * @param string $name
- * @param boolean $asSingleton
- * @param array $constructorArgs
- * @return mixed|null Instance of the specified object (if it exists)
+ * @param string $name The name of the service to retrieve. If not a registered
+ * service, then a class of the given name is instantiated
+ * @param bool $asSingleton If set to false a new instance will be returned.
+ * If true a singleton will be returned unless the spec is type=prototype'
+ * @param array $constructorArgs Args to pass in to the constructor. Note: Ignored for singletons
+ * @return mixed Instance of the specified object
*/
protected function getNamedService($name, $asSingleton = true, $constructorArgs = [])
{
- // Allow service names of the form "%$ServiceName"
- if (substr($name, 0, 2) == '%$') {
- $name = substr($name, 2);
- }
-
// Normalise service / args
list($name, $constructorArgs) = $this->normaliseArguments($name, $constructorArgs);
- // reassign the name as it might actually be a compound name
- if ($serviceName = $this->getServiceName($name)) {
- // check to see what the type of bean is. If it's a prototype,
- // we don't want to return the singleton version of it.
- $spec = $this->specs[$serviceName];
- $type = isset($spec['type']) ? $spec['type'] : null;
- // if we're explicitly a prototype OR we're not wanting a singleton
- if (($type && $type == 'prototype') || !$asSingleton) {
- if ($spec && $constructorArgs) {
- $spec['constructor'] = $constructorArgs;
- } else {
- // convert any _configured_ constructor args.
- // we don't call this for get() calls where someone passes in
- // constructor args, otherwise we end up calling convertServiceParams
- // way too often
- $this->updateSpecConstructor($spec);
- }
- return $this->instantiate($spec, $serviceName, !$type ? 'prototype' : $type);
- } else {
- if (!isset($this->serviceCache[$serviceName])) {
- $this->updateSpecConstructor($spec);
- $this->instantiate($spec, $serviceName);
- }
- return $this->serviceCache[$serviceName];
- }
- }
- $config = $this->configLocator->locateConfigFor($name);
- if ($config) {
- $this->load(array($name => $config));
- if (isset($this->specs[$name])) {
- $spec = $this->specs[$name];
- $this->updateSpecConstructor($spec);
- if ($constructorArgs) {
- $spec['constructor'] = $constructorArgs;
- }
- return $this->instantiate($spec, $name);
- }
- }
- // If we've got this far, we're dealing with a case of a user wanting
- // to create an object based on its name. So, we need to fake its config
- // if the user wants it managed as a singleton service style object
- $spec = array('class' => $name, 'constructor' => $constructorArgs);
- if ($asSingleton) {
- // need to load the spec in; it'll be given the singleton type by default
- $this->load(array($name => $spec));
- return $this->instantiate($spec, $name);
+ // Resolve name with the appropriate spec, or a suitable mock for new services
+ list($name, $spec) = $this->getServiceNamedSpec($name, $constructorArgs);
+
+ // Check if we are getting a prototype or singleton
+ $type = $asSingleton
+ ? (isset($spec['type']) ? $spec['type'] : self::SINGLETON)
+ : self::PROTOTYPE;
+
+ // Return existing instance for singletons
+ if ($type === self::SINGLETON && isset($this->serviceCache[$name])) {
+ return $this->serviceCache[$name];
}
- return $this->instantiate($spec);
+ // Update constructor args
+ if ($type === self::PROTOTYPE && $constructorArgs) {
+ // Passed in args are expected to already be normalised (no service references)
+ $spec['constructor'] = $constructorArgs;
+ } else {
+ // Resolve references in constructor args
+ $this->updateSpecConstructor($spec);
+ }
+
+ // Build instance
+ return $this->instantiate($spec, $name, $type);
}
/**
@@ -1016,6 +997,11 @@ class Injector implements ContainerInterface
*/
protected function normaliseArguments($name, $args = [])
{
+ // Allow service names of the form "%$ServiceName"
+ if (substr($name, 0, 2) == '%$') {
+ $name = substr($name, 2);
+ }
+
if (strstr($name, '(')) {
list($name, $extraArgs) = ClassInfo::parse_class_spec($name);
if ($args) {
@@ -1027,6 +1013,61 @@ class Injector implements ContainerInterface
return [ $name, $args ];
}
+ /**
+ * Get or build a named service and specification
+ *
+ * @param string $name Service name
+ * @param array $constructorArgs Optional constructor args
+ * @return array
+ */
+ protected function getServiceNamedSpec($name, $constructorArgs = [])
+ {
+ $spec = $this->getServiceSpec($name);
+ if ($spec) {
+ // Resolve to exact service name (in case inherited)
+ $name = $this->getServiceName($name);
+ } else {
+ // Late-generate config spec for non-configured spec
+ $spec = [
+ 'class' => $name,
+ 'constructor' => $constructorArgs,
+ ];
+ }
+ return [$name, $spec];
+ }
+
+ /**
+ * Search for spec, lazy-loading in from config locator.
+ * Falls back to parent service name if unloaded
+ *
+ * @param string $name
+ * @param bool $inherit Set to true to inherit from parent service if `.` suffixed
+ * E.g. 'Psr/Log/LoggerInterface.custom' would fail over to 'Psr/Log/LoggerInterface'
+ * @return mixed|object
+ */
+ public function getServiceSpec($name, $inherit = true)
+ {
+ if (isset($this->specs[$name])) {
+ return $this->specs[$name];
+ }
+
+ // Lazy load
+ $config = $this->configLocator->locateConfigFor($name);
+ if ($config) {
+ $this->load([$name => $config]);
+ if (isset($this->specs[$name])) {
+ return $this->specs[$name];
+ }
+ }
+
+ // Fail over to parent service if allowed
+ if (!$inherit || !strpos($name, '.')) {
+ return null;
+ }
+
+ return $this->getServiceSpec(substr($name, 0, strrpos($name, '.')));
+ }
+
/**
* Magic method to return an item directly
*
diff --git a/src/Dev/State/ExtensionTestState.php b/src/Dev/State/ExtensionTestState.php
index af33ba74c..29c4dd09c 100644
--- a/src/Dev/State/ExtensionTestState.php
+++ b/src/Dev/State/ExtensionTestState.php
@@ -5,7 +5,6 @@ namespace SilverStripe\Dev\State;
use LogicException;
use SilverStripe\Core\Extension;
use SilverStripe\Core\Injector\Injector;
-use SilverStripe\Dev\Debug;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\DataObject;
@@ -90,7 +89,10 @@ class ExtensionTestState implements TestState
// clear singletons, they're caching old extension info
// which is used in DatabaseAdmin->doBuild()
- Injector::inst()->unregisterObjects(DataObject::class);
+ Injector::inst()->unregisterObjects([
+ DataObject::class,
+ Extension::class
+ ]);
// If we have altered the schema, but SapphireTest::setUpBeforeClass() would not otherwise
// reset the schema (if there were extra objects) then force a reset
diff --git a/src/ORM/Hierarchy/Hierarchy.php b/src/ORM/Hierarchy/Hierarchy.php
index 8db80a394..73f0a4e9b 100644
--- a/src/ORM/Hierarchy/Hierarchy.php
+++ b/src/ORM/Hierarchy/Hierarchy.php
@@ -23,21 +23,6 @@ use Exception;
*/
class Hierarchy extends DataExtension
{
-
- /**
- * Cache for {@see numChildren()}
- *
- * @var int
- */
- protected $_cache_numChildren = null;
-
- /**
- * Cache for {@see Children()}
- *
- * @var SS_List
- */
- protected $_cache_children = null;
-
/**
* The lower bounds for the amount of nodes to mark. If set, the logic will expand nodes until it reaches at least
* this number, and then stops. Root nodes will always show regardless of this settting. Further nodes can be
@@ -86,6 +71,17 @@ class Hierarchy extends DataExtension
*/
private static $hide_from_cms_tree = array();
+ /**
+ * Prevent virtual page virtualising these fields
+ *
+ * @config
+ * @var array
+ */
+ private static $non_virtual_fields = [
+ '_cache_children',
+ '_cache_numChildren',
+ ];
+
public static function get_extra_config($class, $extension, $args)
{
return array(
@@ -176,17 +172,19 @@ class Hierarchy extends DataExtension
*/
public function Children()
{
- if ($this->_cache_children) {
- return $this->_cache_children;
+ $children = $this->owner->_cache_children;
+ if ($children) {
+ return $children;
}
- $this->_cache_children = $this
+ $children = $this
->owner
->stageChildren(false)
->filterByCallback(function (DataObject $record) {
return $record->canView();
});
- return $this->_cache_children;
+ $this->owner->_cache_children = $children;
+ return $children;
}
/**
@@ -268,18 +266,21 @@ class Hierarchy extends DataExtension
public function numChildren($cache = true)
{
// Load if caching
- if ($cache && isset($this->_cache_numChildren)) {
- return $this->_cache_numChildren;
+ if ($cache) {
+ $numChildren = $this->owner->_cache_numChildren;
+ if (isset($numChildren)) {
+ return $numChildren;
+ }
}
// We call stageChildren(), because Children() has canView() filtering
- $children = (int)$this->owner->stageChildren(true)->Count();
+ $numChildren = (int)$this->owner->stageChildren(true)->Count();
// Save if caching
if ($cache) {
- $this->_cache_numChildren = $children;
+ $this->owner->_cache_numChildren = $numChildren;
}
- return $children;
+ return $numChildren;
}
/**
@@ -308,7 +309,8 @@ class Hierarchy extends DataExtension
{
$hideFromHierarchy = $this->owner->config()->hide_from_hierarchy;
$hideFromCMSTree = $this->owner->config()->hide_from_cms_tree;
- $staged = DataObject::get($this->ownerBaseClass)
+ $baseClass = $this->owner->baseClass();
+ $staged = DataObject::get($baseClass)
->filter('ParentID', (int)$this->owner->ID)
->exclude('ID', (int)$this->owner->ID);
if ($hideFromHierarchy) {
@@ -374,11 +376,12 @@ class Hierarchy extends DataExtension
if (empty($parentID)) {
return null;
}
- $idSQL = $this->owner->getSchema()->sqlColumnForField($this->ownerBaseClass, 'ID');
- return DataObject::get_one($this->ownerBaseClass, array(
- array($idSQL => $parentID),
+ $baseClass = $this->owner->baseClass();
+ $idSQL = $this->owner->getSchema()->sqlColumnForField($baseClass, 'ID');
+ return DataObject::get_one($baseClass, [
+ [$idSQL => $parentID],
$filter
- ));
+ ]);
}
/**
@@ -424,13 +427,10 @@ class Hierarchy extends DataExtension
* Flush all Hierarchy caches:
* - Children (instance)
* - NumChildren (instance)
- * - Marked (global)
- * - Expanded (global)
- * - TreeOpened (global)
*/
public function flushCache()
{
- $this->_cache_children = null;
- $this->_cache_numChildren = null;
+ $this->owner->_cache_children = null;
+ $this->owner->_cache_numChildren = null;
}
}
diff --git a/src/Security/Group.php b/src/Security/Group.php
index e843b26a2..5ba3fc424 100755
--- a/src/Security/Group.php
+++ b/src/Security/Group.php
@@ -616,11 +616,7 @@ class Group extends DataObject
*/
public function AllChildrenIncludingDeleted()
{
- /** @var Hierarchy $extInstance */
- $extInstance = $this->getExtensionInstance(Hierarchy::class);
- $extInstance->setOwner($this);
- $children = $extInstance->AllChildrenIncludingDeleted();
- $extInstance->clearOwner();
+ $children = parent::AllChildrenIncludingDeleted();
$filteredChildren = new ArrayList();
diff --git a/tests/php/Core/ObjectTest.php b/tests/php/Core/ObjectTest.php
index 9ed76f09c..46a4c4619 100644
--- a/tests/php/Core/ObjectTest.php
+++ b/tests/php/Core/ObjectTest.php
@@ -52,7 +52,14 @@ class ObjectTest extends SapphireTest
$objs[] = new ObjectTest\T2();
// All these methods should exist and return true
- $trueMethods = array('testMethod','otherMethod','someMethod','t1cMethod','normalMethod', 'failoverCallback');
+ $trueMethods = [
+ 'testMethod',
+ 'otherMethod',
+ 'someMethod',
+ 't1cMethod',
+ 'normalMethod',
+ 'failoverCallback'
+ ];
foreach ($objs as $i => $obj) {
foreach ($trueMethods as $method) {
@@ -224,33 +231,33 @@ class ObjectTest extends SapphireTest
$this->assertTrue(
singleton(ExtensionTest::class)->hasExtension(ExtendTest1::class),
"Extensions are detected when set on Object::\$extensions on instance hasExtension() without"
- . " case-sensitivity"
+ . " case-sensitivity"
);
// ObjectTest_ExtendTest2 is built in via $extensions (with parameters)
$this->assertTrue(
ExtensionTest::has_extension(ExtendTest2::class),
"Extensions are detected with static has_extension() when set on Object::\$extensions with"
- . " additional parameters"
+ . " additional parameters"
);
$this->assertTrue(
singleton(ExtensionTest::class)->hasExtension(ExtendTest2::class),
"Extensions are detected with instance hasExtension() when set on Object::\$extensions with"
- . " additional parameters"
+ . " additional parameters"
);
$this->assertFalse(
ExtensionTest::has_extension(ExtendTest3::class),
"Other extensions available in the system are not present unless explicitly added to this object"
- . " when checking through has_extension()"
+ . " when checking through has_extension()"
);
$this->assertFalse(
singleton(ExtensionTest::class)->hasExtension(ExtendTest3::class),
"Other extensions available in the system are not present unless explicitly added to this object"
- . " when checking through instance hasExtension()"
+ . " when checking through instance hasExtension()"
);
// ObjectTest_ExtendTest3 is added manually
- ExtensionTest::add_extension(ExtendTest3::class .'("Param")');
+ ExtensionTest::add_extension(ExtendTest3::class . '("Param")');
$this->assertTrue(
ExtensionTest::has_extension(ExtendTest3::class),
"Extensions are detected with static has_extension() when added through add_extension()"
@@ -322,7 +329,7 @@ class ObjectTest extends SapphireTest
public function testRemoveExtensionWithParameters()
{
- ObjectTest\ExtensionRemoveTest::add_extension(ExtendTest2::class.'("MyParam")');
+ ObjectTest\ExtensionRemoveTest::add_extension(ExtendTest2::class . '("MyParam")');
$this->assertTrue(
ObjectTest\ExtensionRemoveTest::has_extension(ExtendTest2::class),
@@ -361,7 +368,7 @@ class ObjectTest extends SapphireTest
public function testExtend()
{
- $object = new ObjectTest\ExtendTest();
+ $object = new ObjectTest\ExtendTest();
$argument = 'test';
$this->assertEquals($object->extend('extendableMethod'), array('ExtendTest2()'));
@@ -399,17 +406,25 @@ class ObjectTest extends SapphireTest
{
// Simple case
$this->assertEquals(
- array(Versioned::class,array('Stage', 'Live')),
+ array(Versioned::class, array('Stage', 'Live')),
ClassInfo::parse_class_spec("SilverStripe\\Versioned\\Versioned('Stage','Live')")
);
+ // Case with service identifier
+ $this->assertEquals(
+ [
+ Versioned::class . '.versioned',
+ ['Versioned'],
+ ],
+ ClassInfo::parse_class_spec("SilverStripe\\Versioned\\Versioned.versioned('Versioned')")
+ );
// String with commas
$this->assertEquals(
- array(Versioned::class,array('Stage,Live', 'Stage')),
+ array(Versioned::class, array('Stage,Live', 'Stage')),
ClassInfo::parse_class_spec("SilverStripe\\Versioned\\Versioned('Stage,Live','Stage')")
);
// String with quotes
$this->assertEquals(
- array(Versioned::class,array('Stage\'Stage,Live\'Live', 'Live')),
+ array(Versioned::class, array('Stage\'Stage,Live\'Live', 'Live')),
ClassInfo::parse_class_spec("SilverStripe\\Versioned\\Versioned('Stage\\'Stage,Live\\'Live','Live')")
);
@@ -425,26 +440,36 @@ class ObjectTest extends SapphireTest
// Array
$this->assertEquals(
- array('Enum',array(array('Accepted', 'Pending', 'Declined', 'Unsubmitted'), 'Unsubmitted')),
+ array('Enum', array(array('Accepted', 'Pending', 'Declined', 'Unsubmitted'), 'Unsubmitted')),
ClassInfo::parse_class_spec("Enum(array('Accepted', 'Pending', 'Declined', 'Unsubmitted'), 'Unsubmitted')")
);
// Nested array
$this->assertEquals(
- array('Enum',array(array('Accepted', 'Pending', 'Declined', array('UnsubmittedA','UnsubmittedB')),
- 'Unsubmitted')),
+ [
+ 'Enum',
+ [
+ ['Accepted', 'Pending', 'Declined', ['UnsubmittedA', 'UnsubmittedB']],
+ 'Unsubmitted'
+ ]
+ ],
ClassInfo::parse_class_spec(
"Enum(array('Accepted', 'Pending', 'Declined', array('UnsubmittedA','UnsubmittedB')), 'Unsubmitted')"
)
);
// 5.4 Shorthand Array
$this->assertEquals(
- array('Enum',array(array('Accepted', 'Pending', 'Declined', 'Unsubmitted'), 'Unsubmitted')),
+ array('Enum', array(array('Accepted', 'Pending', 'Declined', 'Unsubmitted'), 'Unsubmitted')),
ClassInfo::parse_class_spec("Enum(['Accepted', 'Pending', 'Declined', 'Unsubmitted'], 'Unsubmitted')")
);
// 5.4 Nested shorthand array
$this->assertEquals(
- array('Enum',array(array('Accepted', 'Pending', 'Declined', array('UnsubmittedA','UnsubmittedB')),
- 'Unsubmitted')),
+ [
+ 'Enum',
+ [
+ ['Accepted', 'Pending', 'Declined', ['UnsubmittedA', 'UnsubmittedB']],
+ 'Unsubmitted'
+ ]
+ ],
ClassInfo::parse_class_spec(
"Enum(['Accepted', 'Pending', 'Declined', ['UnsubmittedA','UnsubmittedB']], 'Unsubmitted')"
)
diff --git a/tests/php/ORM/DataExtensionTest.php b/tests/php/ORM/DataExtensionTest.php
index 62bc5df66..2f935e7da 100644
--- a/tests/php/ORM/DataExtensionTest.php
+++ b/tests/php/ORM/DataExtensionTest.php
@@ -292,7 +292,11 @@ class DataExtensionTest extends SapphireTest
$extension->clearOwner();
$this->assertEquals($obj1, $extension->getOwner());
- // Clear original owner
+ // Clear pushed null
+ $extension->clearOwner();
+ $this->assertNull($extension->getOwner());
+
+ // Clear original null
$extension->clearOwner();
$this->assertNull($extension->getOwner());