constructor in controllers are always called - even on different classes

[christian-lorenz]

Hi,
there's some strange behavior on Graphqlite controllers when using a constructor.

So I've got some public controllers and some for an admin panel.

The admin panel ones should have an constructor / extending an abstract Controller with a __construct, the public ones not.

class FooAdminController extends AbstractAdminController 
{
   #[Query]
    public function getUsers() { ... }
}

abstract class AbstractAdminController
{
   public function __construct($someDependencyInjections) 
   {
      doSomethingWithTheInjection()
   }
}

However, this constructor is called EVERYWHERE and EVERYTIME - even if the AbstractController in not extended.
I also tried putting the constructor inside a single controller - it is always executed even when I call a mutation or query which is in a completely different controller.

Can you explain this behavior and how I can prevent it?

Symfony 5, GraphQLite 4. (yes, we need to update - when there's time :-/)

[oojacoboo]

So, I believe there was another discussion around this at some point - not entirely sure. But, I believe the classes in the type namespace are constructed as part of the container and used to build out the initial schema. Assuming that's the case, it could probably be improved. I don't know why you'd have an abstract class that's never extended. But, an abstract class should never attempt to be constructed. If you exclude the abstract class from your container, are you still having this issue?

[christian-lorenz]

Well, I played around. And it's not only about extending - if one Controller has a __construct() method on every other Controller seems to execute it as well.

for example:

FooController.php
class FooController {
   public function __construct() {
       die("adios");
   }

  #[Query]
  public function foo() {...}
}


BarController.php
class BarController {

 // No constructor

  #[Query]
  public function bar() {...}
}

and you query bar() - it will result in the die("adios").

Moving the abstract class (which should be used on some controllers but not all) out of the graphql namespace (like in src/AbstractControllers or something) doesn't help. If there's only one controller with a constructor in the project, it's called everytime - no matter which controller you're actually accessing.

I also think, it's because of the compiling (or however you call it) or so - because otherwise I couldn't explain this "magic".
And well the only workarounds seems to: Duplicate the code the constructor does in every mutation / query.
Or are there other ways? (and I know executing code in a constructor will not win the software design award - be in our case it's - let's say it's "ok" until refactoring ;) )

[oojacoboo]

Well, instantiating or initializing all of these controllers through the service container, to build out the schema, assuming that's what's happening, isn't ideal. Reflection would be better, but I'm unsure if there is a necessary reason for this. You're welcome to look into it and put together a PR.

That said, I would question what you're doing in this constructor that's causing any issues. A "controller"/service shouldn't be mutating any global state and shouldn't have any side-effects. Meaning, the instantiation of these services shouldn't matter, aside from the clock cycles.

[christian-lorenz]

Well I'm throwing a custom AccessDeniedException if a session ran out (no user on security) there which is intercepted in the admin client and handled differently than the (not) #[Logged] response.

[oojacoboo]

I can appreciate that design decision. But working with global state in a constructor isn't generally ideal. You might consider updating your middleware processing to handle this. Maybe you can add a custom attribute to your controller classes for Auth(z).

If you'd like to dig into the instantiation of these services to see if it can be refactored to use reflection for schema building, that'd be an acceptable improvement.