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

Injector makes invalid instantiation request #1804

Open
NightJar opened this issue Aug 1, 2024 · 2 comments
Open

Injector makes invalid instantiation request #1804

NightJar opened this issue Aug 1, 2024 · 2 comments
Labels

Comments

@NightJar
Copy link
Contributor

NightJar commented Aug 1, 2024

Resulting in

ArgumentCountError: Too few arguments to function SilverStripe\Control\HTTPRequest::__construct(), 0 passed in [...]vendor/silverstripe/framework/src/Core/Injector/InjectionCreator.php on line 35 and at least 2 expected

$request = Injector::inst()->get(HTTPRequest::class);


Thanks @GuySartorelli for pointing out (via user's slack) that it's looking for a pre-registered request.
Seems like this is a new code execution path in CMS 5, as the error doesn't occur when running tests against CMS 4.

The context of the error is attempting to run a PHPUnit test with a normal "just enough" setup (i.e. it's not production traffic):

        $admin = new ModelAdmin();
        $request = $admin->getRequest();
        $request->setSession(new Session([]));
        $admin->doInit();

Interestingly the offending line is not getRequest() as one might think, but rather doInit()

The execution path is via getCombinedClientConfig() which falls throught to the sudo mode controller that for whatever reason uses Injector::get rather than $this->getRequest(). Using getRequest would cause no issue, as per the default request set (but not registered with Injector) in RequestHandler::__construct():

   public function __construct()
   {
       $this->brokenOnConstruct = false;

       $this->setRequest(new NullHTTPRequest());  // because of this

       parent::__construct();
   }
@GuySartorelli
Copy link
Member

Thanks for raising this!

You could work around this by injecting the request: Injector::inst()->registerService(HTTPRequest::class, $request);

Given this is only happening in a unit test and there's an obvious workaround I can't promise the issue will be prioritised any time in the near future.

@NightJar
Copy link
Contributor Author

NightJar commented Aug 1, 2024

Yes, I was just about to add this in my own comment.
To clarify in the same example from the OP (slightly expanded to include namespace usage changes):

+use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\Session;
+use SilverStripe\Core\Injector\Injector;
        $admin = new ModelAdmin();
        $request = $admin->getRequest();
+       Injector::inst()->registerService($request, HTTPRequest::class);
        $request->setSession(new Session([]));
        $admin->doInit();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants