Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Factories may not return shared instance #7868

Merged
merged 8 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading