ServerRequestInterface injection into container failing

[jamesrweb]

As the container is just middleware, it would be nice to have the ServerRequestInterface for the request being injectable when required.

I personally would like to do something like the following:

$container = new Container([
  ApiKey::class => function(ServerRequestInterface $request) {
      $headerLine = $request->getHeaderLine("API-Key");
      assert(is_string($headerLine);
      
      return ApiKey::fromString($headerLine);
  }
]);

Currently if I do this though, it throws a 500 and does not propagate the ServerRequestInterface through to the next middleware it seems.

[jamesrweb]

This is especially useful when the api key, in this case, is not part of the controller logic but instead, the repository logic. Since repositories are injected via DI, it would be nice to be able to also then inject the API key into their container block and pass into the constructor, for example:

$container = new Container([
  ApiKey::class => function(ServerRequestInterface $request) {
      $headerLine = $request->getHeaderLine("API-Key");
      assert(is_string($headerLine);
      
      return ApiKey::fromString($headerLine);
  },
  SomeRepositoryInterface::class => function(ApiKey $apiKey, ...) {
     return new SomeRepository($apiKey, ...);
  }
]);

[SimonFrings]

@jamesrweb Thanks for bringing this up 👍

I like the idea, but I don't think using the container this way is the best way to do it. This example will work if you only have one user with an "API-Key", once you have multiple users you run into the risk of exposing the same key to different users. What we can do is using a request middleware to check out the incoming request and apply different behavior (attributes etc.) based on the requests header or parameter. This could also be a new feature idea ^^

[jamesrweb]

The main reason to think of this is to have better type safety and reduce the amount of assertions required in the controllers because always asserting in every controller to check if the attribute is an instance of api key is quite tedious, especially when you scale it to multiple attributes in a controller needing asserted upon for a myriad of types. The container could return an ApiKey either way using the null object pattern as one example and then when the value is injected, no assertions would be required down tree, it would either be there or not but type wise, it would be sound and work as expected.

Does that make sense? 😅

[SimonFrings]

Yes it makes sense, but I'm not sure if this is really worth it. I can understand that it's a bit more uncomfortable to always use the assertions, but it essentially works and changing this in the container also comes with a few consequences to think of (name collisions, etc.).

To keep you in the loop: We're currently working towards an upcoming v0.17.0 to focus a bit more on refactoring the current handler structure first and then add more handlers to make things even easier (won't spoil too much 🤫). We're also planning to make improvements to the container in future versions, so we'll have another look at your suggestion then and focus on the handlers for now 👍

[jamesrweb]

Sounds good to me 😄