Apply allow_override in same way as v2

This patch alters how the allow_override flag works in order to better
mimic how it was done in v2. This means:

- the flag is `false` by default.
- the flag only kicks in when new configuration would change a service
  that has already been fetched and cached internally (i.e., it exists
  in the `$services` array).

The implication is that the following will never be problematic:

- abstract factories (as they are a factory of "last resort")
- initializers (they only apply to newly created instances)

Tests were updated to reflect the changes.

Author: weierophinney

src/ServiceManager.php

@@ -59,7 +59,7 @@ class ServiceManager implements ServiceLocatorInterface
      *
      * @param bool
      */
-    protected $allowOverride = true;
+    protected $allowOverride = false;
 
     /**
      * @var ContainerInterface
@@ -275,15 +275,12 @@ public function getAllowOverride()
      * @param  array $config
      * @return self
      * @throws ContainerModificationsNotAllowedException if the allow
-     *     override flag has been toggled off.
+     *     override flag has been toggled off, and a service instance
+     *     exists for a given service.
      */
     public function configure(array $config)
     {
-        if ($this->allowOverride === false) {
-            throw new ContainerModificationsNotAllowedException(
-                'Overrides have been disabled on this container instance'
-            );
-        }
+        $this->validateOverrides($config);
 
         if (isset($config['services'])) {
             $this->services = $config['services'] + $this->services;
@@ -785,4 +782,87 @@ private function createFactoriesForInvokables(array $invokables)
         }
         return $factories;
     }
+
+    /**
+     * Determine if one or more services already exist in the container.
+     *
+     * If the allow override flag is true, this method does nothing.
+     *
+     * Otherwise, it checks against each of the following service types,
+     * if present, and validates that none are defining services that
+     * already exist; if they do, it raises an exception indicating
+     * modification is not allowed.
+     *
+     * @param array $config
+     * @throws ContainerModificationsNotAllowedException if any services
+     *     provided already have instances available.
+     */
+    private function validateOverrides(array $config)
+    {
+        if ($this->allowOverride) {
+            return;
+        }
+
+        if (isset($config['services'])) {
+            $this->validateOverrideSet(array_keys($config['services']), 'service');
+        }
+
+        if (isset($config['aliases'])) {
+            $this->validateOverrideSet(array_keys($config['aliases']), 'alias');
+        }
+
+        if (isset($config['invokables'])) {
+            $this->validateOverrideSet(array_keys($config['invokables']), 'invokable class');
+        }
+
+        if (isset($config['factories'])) {
+            $this->validateOverrideSet(array_keys($config['factories']), 'factory');
+        }
+
+        if (isset($config['delegators'])) {
+            $this->validateOverrideSet(array_keys($config['delegators']), 'delegator');
+        }
+
+        if (isset($config['shared'])) {
+            $this->validateOverrideSet(array_keys($config['shared']), 'sharing rule');
+        }
+
+        if (isset($config['lazy_services']['class_map'])) {
+            $this->validateOverrideSet(array_keys($config['lazy_services']['class_map']), 'lazy service');
+        }
+    }
+
+    /**
+     * Determine if one or more services already exist for a given type.
+     *
+     * Loops through the provided service names, checking if any have current
+     * service instances; if not, it returns, but otherwise, it raises an
+     * exception indicating modification is not allowed.
+     *
+     * @param string[] $services
+     * @param string $type Type of service being checked.
+     * @throws ContainerModificationsNotAllowedException if any services
+     *     provided already have instances available.
+     */
+    private function validateOverrideSet(array $services, $type)
+    {
+        $detected = [];
+        foreach ($services as $service) {
+            if (isset($this->services[$service])) {
+                $detected[] = $service;
+            }
+        }
+
+        if (empty($detected)) {
+            return;
+        }
+
+        throw new ContainerModificationsNotAllowedException(sprintf(
+            'An updated/new %s is not allowed, as the container does not allow '
+            . 'changes for services with existing instances; the following '
+            . 'already exist in the container: %s',
+            $type,
+            implode(', ', $detected)
+        ));
+    }
 }

test/CommonServiceLocatorBehaviorsTrait.php

@@ -719,17 +719,22 @@ public function testCanInjectSharingRules()
     public function methodsAffectedByOverrideSettings()
     {
         // @codingStandardsIgnoreStart
-        //  name                 => [ 'method to invoke',  [arguments for invocation]]
+        //  name                        => [ 'method to invoke',  [arguments for invocation]]
         return [
-            'setAlias'           => ['setAlias',           ['foo', 'bar']],
-            'setInvokableClass'  => ['setInvokableClass',  ['foo', __CLASS__]],
-            'setFactory'         => ['setFactory',         ['foo', function () {}]],
-            'addAbstractFactory' => ['addAbstractFactory', [TestAsset\SimpleAbstractFactory::class]],
-            'addDelegator'       => ['addDelegator',       ['foo', TestAsset\PreDelegator::class]],
-            'addInitializer'     => ['addInitializer',     [function () {}]],
-            'setService'         => ['setService',         ['foo', $this]],
-            'setShared'          => ['setShared',          ['foo', false]],
-            'configure'          => ['configure',          [[]]],
+            'setAlias'                  => ['setAlias',           ['foo', 'bar']],
+            'setInvokableClass'         => ['setInvokableClass',  ['foo', __CLASS__]],
+            'setFactory'                => ['setFactory',         ['foo', function () {}]],
+            'setService'                => ['setService',         ['foo', $this]],
+            'setShared'                 => ['setShared',          ['foo', false]],
+            'mapLazyService'            => ['mapLazyService',     ['foo', __CLASS__]],
+            'addDelegator'              => ['addDelegator',       ['foo', function () {}]],
+            'configure-alias'           => ['configure',          [['aliases'       => ['foo' => 'bar']]]],
+            'configure-invokable'       => ['configure',          [['invokables'    => ['foo' => 'foo']]]],
+            'configure-invokable-alias' => ['configure',          [['invokables'    => ['foo' => 'bar']]]],
+            'configure-factory'         => ['configure',          [['factories'     => ['foo' => function () {}]]]],
+            'configure-service'         => ['configure',          [['services'      => ['foo' => $this]]]],
+            'configure-shared'          => ['configure',          [['shared'        => ['foo' => false]]]],
+            'configure-lazy-service'    => ['configure',          [['lazy_services' => ['class_map' => ['foo' => __CLASS__]]]]],
         ];
         // @codingStandardsIgnoreEnd
     }
@@ -740,7 +745,7 @@ public function methodsAffectedByOverrideSettings()
      */
     public function testConfiguringInstanceRaisesExceptionIfAllowOverrideIsFalse($method, $args)
     {
-        $container = $this->createContainer();
+        $container = $this->createContainer(['services' => ['foo' => $this]]);
         $container->setAllowOverride(false);
         $this->setExpectedException(ContainerModificationsNotAllowedException::class);
         call_user_func_array([$container, $method], $args);
@@ -749,20 +754,20 @@ public function testConfiguringInstanceRaisesExceptionIfAllowOverrideIsFalse($me
     /**
      * @group mutation
      */
-    public function testAllowOverrideFlagIsTrueByDefault()
+    public function testAllowOverrideFlagIsFalseByDefault()
     {
         $container = $this->createContainer();
-        $this->assertTrue($container->getAllowOverride());
+        $this->assertFalse($container->getAllowOverride());
         return $container;
     }
 
     /**
      * @group mutation
-     * @depends testAllowOverrideFlagIsTrueByDefault
+     * @depends testAllowOverrideFlagIsFalseByDefault
      */
     public function testAllowOverrideFlagIsMutable($container)
     {
-        $container->setAllowOverride(false);
-        $this->assertFalse($container->getAllowOverride());
+        $container->setAllowOverride(true);
+        $this->assertTrue($container->getAllowOverride());
     }
 }