feat(app framework)!: Inject services into controller methods

Usually Nextcloud DI goes through constructor injection. This has the
implication that each instance of a class builds the full DI tree. That
is the injected services, their services, etc. Occasionally there is a
service that is only needed for one controller method. Then the DI tree
is build regardless if used or not.

If services are injected into the method, we only build the DI tree if
that method gets executed.

This is also how Laravel allows injection.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

1 files changed, 17 insertions, 20 deletions

diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php
index 8a206a429bd..2c8aedf99c4 100644
--- a/tests/Core/Controller/LoginControllerTest.php
+++ b/tests/Core/Controller/LoginControllerTest.php
@@ -78,9 +78,6 @@ class LoginControllerTest extends TestCase {
 	/** @var Throttler|MockObject */
 	private $throttler;
 
-	/** @var LoginChain|MockObject */
-	private $chain;
-
 	/** @var IInitialStateService|MockObject */
 	private $initialStateService;
 
@@ -104,7 +101,6 @@ class LoginControllerTest extends TestCase {
 		$this->twoFactorManager = $this->createMock(Manager::class);
 		$this->defaults = $this->createMock(Defaults::class);
 		$this->throttler = $this->createMock(Throttler::class);
-		$this->chain = $this->createMock(LoginChain::class);
 		$this->initialStateService = $this->createMock(IInitialStateService::class);
 		$this->webAuthnManager = $this->createMock(\OC\Authentication\WebAuthn\Manager::class);
 		$this->notificationManager = $this->createMock(IManager::class);
@@ -134,7 +130,6 @@ class LoginControllerTest extends TestCase {
 			$this->urlGenerator,
 			$this->defaults,
 			$this->throttler,
-			$this->chain,
 			$this->initialStateService,
 			$this->webAuthnManager,
 			$this->notificationManager,
@@ -448,11 +443,11 @@ class LoginControllerTest extends TestCase {
 		$this->assertEquals($expectedResponse, $this->loginController->showLoginForm('0', ''));
 	}
 
-	public function testLoginWithInvalidCredentials() {
+	public function testLoginWithInvalidCredentials(): void {
 		$user = 'MyUserName';
 		$password = 'secret';
 		$loginPageUrl = '/login?redirect_url=/apps/files';
-
+		$loginChain = $this->createMock(LoginChain::class);
 		$this->request
 			->expects($this->once())
 			->method('passesCSRFCheck')
@@ -464,7 +459,7 @@ class LoginControllerTest extends TestCase {
 			'/apps/files'
 		);
 		$loginResult = LoginResult::failure($loginData, LoginController::LOGIN_MSG_INVALIDPASSWORD);
-		$this->chain->expects($this->once())
+		$loginChain->expects($this->once())
 			->method('process')
 			->with($this->equalTo($loginData))
 			->willReturn($loginResult);
@@ -479,7 +474,7 @@ class LoginControllerTest extends TestCase {
 		$expected = new RedirectResponse($loginPageUrl);
 		$expected->throttle(['user' => 'MyUserName']);
 
-		$response = $this->loginController->tryLogin($user, $password, '/apps/files');
+		$response = $this->loginController->tryLogin($loginChain, $user, $password, '/apps/files');
 
 		$this->assertEquals($expected, $response);
 	}
@@ -487,7 +482,7 @@ class LoginControllerTest extends TestCase {
 	public function testLoginWithValidCredentials() {
 		$user = 'MyUserName';
 		$password = 'secret';
-
+		$loginChain = $this->createMock(LoginChain::class);
 		$this->request
 			->expects($this->once())
 			->method('passesCSRFCheck')
@@ -498,7 +493,7 @@ class LoginControllerTest extends TestCase {
 			$password
 		);
 		$loginResult = LoginResult::success($loginData);
-		$this->chain->expects($this->once())
+		$loginChain->expects($this->once())
 			->method('process')
 			->with($this->equalTo($loginData))
 			->willReturn($loginResult);
@@ -508,7 +503,7 @@ class LoginControllerTest extends TestCase {
 			->willReturn('/default/foo');
 
 		$expected = new RedirectResponse('/default/foo');
-		$this->assertEquals($expected, $this->loginController->tryLogin($user, $password));
+		$this->assertEquals($expected, $this->loginController->tryLogin($loginChain, $user, $password));
 	}
 
 	public function testLoginWithoutPassedCsrfCheckAndNotLoggedIn(): void {
@@ -519,7 +514,7 @@ class LoginControllerTest extends TestCase {
 			->willReturn('jane');
 		$password = 'secret';
 		$originalUrl = 'another%20url';
-
+		$loginChain = $this->createMock(LoginChain::class);
 		$this->request
 			->expects($this->once())
 			->method('passesCSRFCheck')
@@ -533,7 +528,7 @@ class LoginControllerTest extends TestCase {
 		$this->userSession->expects($this->never())
 			->method('createRememberMeToken');
 
-		$response = $this->loginController->tryLogin('Jane', $password, $originalUrl);
+		$response = $this->loginController->tryLogin($loginChain, 'Jane', $password, $originalUrl);
 
 		$expected = new RedirectResponse('');
 		$expected->throttle(['user' => 'Jane']);
@@ -549,7 +544,7 @@ class LoginControllerTest extends TestCase {
 		$password = 'secret';
 		$originalUrl = 'another url';
 		$redirectUrl = 'http://localhost/another url';
-
+		$loginChain = $this->createMock(LoginChain::class);
 		$this->request
 			->expects($this->once())
 			->method('passesCSRFCheck')
@@ -571,7 +566,7 @@ class LoginControllerTest extends TestCase {
 			->with('remember_login_cookie_lifetime')
 			->willReturn(1234);
 
-		$response = $this->loginController->tryLogin('Jane', $password, $originalUrl);
+		$response = $this->loginController->tryLogin($loginChain, 'Jane', $password, $originalUrl);
 
 		$expected = new RedirectResponse($redirectUrl);
 		$this->assertEquals($expected, $response);
@@ -581,7 +576,7 @@ class LoginControllerTest extends TestCase {
 		$user = 'MyUserName';
 		$password = 'secret';
 		$redirectUrl = 'https://next.cloud/apps/mail';
-
+		$loginChain = $this->createMock(LoginChain::class);
 		$this->request
 			->expects($this->once())
 			->method('passesCSRFCheck')
@@ -593,7 +588,7 @@ class LoginControllerTest extends TestCase {
 			'/apps/mail'
 		);
 		$loginResult = LoginResult::success($loginData);
-		$this->chain->expects($this->once())
+		$loginChain->expects($this->once())
 			->method('process')
 			->with($this->equalTo($loginData))
 			->willReturn($loginResult);
@@ -606,12 +601,13 @@ class LoginControllerTest extends TestCase {
 			->willReturn($redirectUrl);
 		$expected = new RedirectResponse($redirectUrl);
 
-		$response = $this->loginController->tryLogin($user, $password, '/apps/mail');
+		$response = $this->loginController->tryLogin($loginChain, $user, $password, '/apps/mail');
 
 		$this->assertEquals($expected, $response);
 	}
 
 	public function testToNotLeakLoginName() {
+		$loginChain = $this->createMock(LoginChain::class);
 		$this->request
 			->expects($this->once())
 			->method('passesCSRFCheck')
@@ -624,7 +620,7 @@ class LoginControllerTest extends TestCase {
 			'/apps/files'
 		);
 		$loginResult = LoginResult::failure($loginData, LoginController::LOGIN_MSG_INVALIDPASSWORD);
-		$this->chain->expects($this->once())
+		$loginChain->expects($this->once())
 			->method('process')
 			->with($this->equalTo($loginData))
 			->willReturnCallback(function (LoginData $data) use ($loginResult) {
@@ -643,6 +639,7 @@ class LoginControllerTest extends TestCase {
 		$expected->throttle(['user' => 'john']);
 
 		$response = $this->loginController->tryLogin(
+			$loginChain,
 			'john@doe.com',
 			'just wrong',
 			'/apps/files'