Skip to content

Commit

Permalink
Merge pull request #7868 from kenjis/fix-factories-return-new-instance
Browse files Browse the repository at this point in the history
fix: Factories may not return shared instance
  • Loading branch information
kenjis committed Aug 29, 2023
2 parents 1804bab + 1b88479 commit bb0cd96
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 9 deletions.
50 changes: 41 additions & 9 deletions system/Config/Factories.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public static function define(string $component, string $alias, string $classnam
self::getOptions($component);

self::$aliases[$component][$alias] = $classname;
self::$updated[$component] = true;
}

/**
Expand All @@ -142,6 +143,7 @@ public static function __callStatic(string $component, array $arguments)
return new $class(...$arguments);
}

// Try to locate the class
if ($class = self::locateClass($options, $alias)) {
return new $class(...$arguments);
}
Expand All @@ -160,14 +162,8 @@ public static function __callStatic(string $component, array $arguments)
return null;
}

self::$instances[$options['component']][$class] = new $class(...$arguments);
self::$aliases[$options['component']][$alias] = $class;
self::$updated[$options['component']] = true;

// If a short classname is specified, also register FQCN to share the instance.
if (! isset(self::$aliases[$options['component']][$class])) {
self::$aliases[$options['component']][$class] = $class;
}
self::createInstance($options['component'], $class, $arguments);
self::setAlias($options['component'], $alias, $class);

return self::$instances[$options['component']][$class];
}
Expand All @@ -179,6 +175,7 @@ public static function __callStatic(string $component, array $arguments)
*/
private static function getDefinedInstance(array $options, string $alias, array $arguments)
{
// The alias is already defined.
if (isset(self::$aliases[$options['component']][$alias])) {
$class = self::$aliases[$options['component']][$alias];

Expand All @@ -189,15 +186,50 @@ private static function getDefinedInstance(array $options, string $alias, array
return self::$instances[$options['component']][$class];
}

self::$instances[$options['component']][$class] = new $class(...$arguments);
self::createInstance($options['component'], $class, $arguments);

return self::$instances[$options['component']][$class];
}
}

// Try to locate the class
if (! $class = self::locateClass($options, $alias)) {
return null;
}

// Check for an existing instance for the class
if (isset(self::$instances[$options['component']][$class])) {
self::setAlias($options['component'], $alias, $class);

return self::$instances[$options['component']][$class];
}

return null;
}

/**
* Creates the shared instance.
*/
private static function createInstance(string $component, string $class, array $arguments): void
{
self::$instances[$component][$class] = new $class(...$arguments);
self::$updated[$component] = true;
}

/**
* Sets alias
*/
private static function setAlias(string $component, string $alias, string $class): void
{
self::$aliases[$component][$alias] = $class;
self::$updated[$component] = true;

// If a short classname is specified, also register FQCN to share the instance.
if (! isset(self::$aliases[$component][$class]) && ! self::isNamespaced($alias)) {
self::$aliases[$component][$class] = $class;
}
}

/**
* Is the component Config?
*
Expand Down
9 changes: 9 additions & 0 deletions tests/system/Config/FactoriesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace CodeIgniter\Config;

use CodeIgniter\Test\CIUnitTestCase;
use Config\App;
use Config\Database;
use InvalidArgumentException;
use ReflectionClass;
Expand Down Expand Up @@ -322,6 +323,14 @@ public function testCanLoadTwoCellsWithSameShortName()
$this->assertNotSame($cell1, $cell2);
}

public function testCanLoadSharedConfigWithDifferentAlias()
{
$config1 = Factories::config(App::class);
$config2 = Factories::config('App');

$this->assertSame($config1, $config2);
}

public function testDefineSameAliasTwiceWithDifferentClasses()
{
$this->expectException(InvalidArgumentException::class);
Expand Down

0 comments on commit bb0cd96

Please sign in to comment.