Breaking change in 4.11: Injector can't handle anonymous classes

[n8-dev]

Affected Version

4.11

Description

In our unit tests we use the Injector process to mock specific function returns.

With the change of 4.10 to 4.11 something has changed with the underlying Injector process, possibly the factory that gets used.

This could effect not just test cases.

When they used to run, now the follow error is returned

TypeError: class_exists(): Argument #1 ($class) must be of type string, class@anonymous given

Steps to Reproduce

Code sample:

A phpunit test

    public function testUserDoesNotExist(): void
    {
        Injector::inst()->load([
            foobarMiddleware::class => [
                'class' => new class {
                    // phpcs:ignore SlevomatCodingStandard.Functions.UnusedParameter
                    public function getMemberIfFooBarr(string $accessToken): ?DataObject
                    {
                        return null;
                    }
                },
            ],
        ]);

        $request = $this->createRequest('POST', []);
        $controller = ThingController::create();
        $response = $controller->handleCreateThing($request);

        $this->assertSame(400, $response->getStatusCode());
        $this->assertSame('User does not exist', $response->getUserMessage());
    }

->handleCreateThing()

Simply triggers a
foobarMiddleware::create();

That class being

final class foobarMiddleware extends (some things that ultimately extend from HTTPMiddleware)
{

    use Injectable;

  ...
  // none of this code or the other extended files before HTTPMiddleware touch the Injectable functions
}

Acceptance criteria

• You can use anonymous classes with Injector.

PRs

• Allow classes to be passed into Injector::inst()->load() #10539
• FIX Allow passing objects to InjectionCreator::create() #10555

[kinglozzer]

Looks like this may have been caused by #10265. I’m not sure if injecting anonymous classes like this was ever officially supported (?) but if this is the only piece of code that’s preventing it working then I’d be in favour of re-adding support:

silverstripe-framework/src/Core/Injector/InjectionCreator.php

Lines 10 to 14 in 58a072f

	public function create($class, array $params = [])
	{
	if (!class_exists($class ?? '')) {
	throw new InjectorNotFoundException("Class {$class} does not exist");
	}

If that is indeed the cause, we can probably just return $class if $class is already an object right?

[GuySartorelli]

If $class is already an object, it may not have the params set on it. There's an argument to be made that create() should always create a new instance. There's another argument to be made that it should just set the param values on the existing object.

I think we'd have to retain the previous behaviour though, which would have been creating a new instance most likely.
In CMS 5 we can reduce the scope of support for this to explicitly only allow instances of anonymous classes. Instances of concrete classes shouldn't be supported going forward.

[emteknetnz]

If $class is already an object, it may not have the params set on it.

It can if we just go if (is_object($class)) { $class = get_class($class); };

note: the php anonymous class syntax will instantiate a new instance, rather than assign the class to a variable.

I think we'd have to retain the previous behaviour though, which would have been creating a new instance most likely

Yes, previous behaviour created new instances - https://github.com/silverstripe/silverstripe-framework/pull/10265/files#diff-a6f8392a1ea245fb07612de639deda3040d399bc87258fba6210693f2739d4e8L28

In CMS 5 we can reduce the scope of support for this to explicitly only allow instances of anonymous classes. Instances of concrete classes shouldn't be supported going forward.

It don't think we need to since we can just InjectionCreator::create() will always create a new instance. If desired though we could see if there's a class@anonymous in $class if is_object($class), though if we're going to allow this level of flexibility I don't see a big issue of also allows concrete classes instances to also be used (don't know why you would though)

[michalkleiner]

The PR fixing this regression has been merged.