Paid off technical debt regarding validation

src/Controllers/AuthController.php

@@ -30,15 +30,10 @@ final class AuthController extends AbstractController
 	{
 		if (isset($this->inputReader->userName) and isset($this->inputReader->password))
 		{
-			$this->userService->validateUserName($this->inputReader->userName);
-			$this->passwordService->validatePassword($this->inputReader->password);
-
 			$this->authService->loginFromCredentials($this->inputReader->userName, $this->inputReader->password);
 		}
 		elseif (isset($this->inputReader->token))
 		{
-			if (!$this->inputReader->token)
-				throw new \DomainException('Authentication token cannot be empty.');
 			$this->authService->loginFromToken($this->inputReader->token);
 		}
 		else

src/Services/AuthService.php

@@ -6,18 +6,22 @@ final class AuthService
 	private $loggedInUser = null;
 	private $loginToken = null;
 
+	private $validator;
 	private $passwordService;
 	private $timeService;
 	private $userDao;
 	private $tokenDao;
 
 	public function __construct(
+		\Szurubooru\Validator $validator,
 		\Szurubooru\Services\PasswordService $passwordService,
 		\Szurubooru\Services\TimeService $timeService,
 		\Szurubooru\Dao\TokenDao $tokenDao,
 		\Szurubooru\Dao\UserDao $userDao)
 	{
 		$this->loggedInUser = $this->getAnonymousUser();
+
+		$this->validator = $validator;
 		$this->passwordService = $passwordService;
 		$this->timeService = $timeService;
 		$this->tokenDao = $tokenDao;
@@ -41,6 +45,9 @@ final class AuthService
 
 	public function loginFromCredentials($userName, $password)
 	{
+		$this->validator->validateUserName($userName);
+		$this->validator->validatePassword($password);
+
 		$user = $this->userDao->getByName($userName);
 		if (!$user)
 			throw new \InvalidArgumentException('User not found.');
@@ -56,6 +63,8 @@ final class AuthService
 
 	public function loginFromToken($loginTokenName)
 	{
+		$this->validator->validateToken($loginTokenName);
+
 		$loginToken = $this->tokenDao->getByName($loginTokenName);
 		if (!$loginToken)
 			throw new \Exception('Invalid login token.');

src/Services/EmailService.php

@@ -3,13 +3,4 @@ namespace Szurubooru\Services;
 
 class EmailService
 {
-	//todo: refactor this to generic validation
-	public function validateEmail($email)
-	{
-		if (!$email)
-			return;
-
-		if (!preg_match('/^[^@]+@[^@]+\.\w+$/', $email))
-			throw new \DomainException('Specified e-mail appears to be invalid.');
-	}
 }

src/Services/PasswordService.php

@@ -10,25 +10,6 @@ class PasswordService
 		$this->config = $config;
 	}
 
-	//todo: refactor this to generic validation
-	public function validatePassword($password)
-	{
-		if (!$password)
-			throw new \DomainException('Password cannot be empty.');
-
-		$minPasswordLength = intval($this->config->security->minPasswordLength);
-		if (strlen($password) < $minPasswordLength)
-			throw new \DomainException('Password must have at least ' . $minPasswordLength . ' character(s).');
-
-		if (preg_match('/[^\x20-\x7f]/', $password))
-		{
-			throw new \DomainException(
-				'Password should contain only characters from ASCII range to avoid potential problems with encoding.');
-		}
-
-		return true;
-	}
-
 	public function getHash($password)
 	{
 		return hash('sha256', $this->config->security->secret . '/' . $password);

src/Services/UserService.php

@@ -3,20 +3,20 @@ namespace Szurubooru\Services;
 
 class UserService
 {
-	private $config;
+	private $validator;
 	private $userDao;
 	private $passwordService;
 	private $emailService;
 	private $timeService;
 
 	public function __construct(
-		\Szurubooru\Config $config,
+		\Szurubooru\Validator $validator,
 		\Szurubooru\Dao\UserDao $userDao,
 		\Szurubooru\Services\PasswordService $passwordService,
 		\Szurubooru\Services\EmailService $emailService,
 		\Szurubooru\Services\TimeService $timeService)
 	{
-		$this->config = $config;
+		$this->validator = $validator;
 		$this->userDao = $userDao;
 		$this->passwordService = $passwordService;
 		$this->emailService = $emailService;
@@ -25,9 +25,9 @@ class UserService
 
 	public function register(\Szurubooru\FormData\RegistrationFormData $formData)
 	{
-		$this->validateUserName($formData->name);
+		$this->validator->validateUserName($formData->name);
-		$this->passwordService->validatePassword($formData->password);
+		$this->validator->validatePassword($formData->password);
-		$this->emailService->validateEmail($formData->email);
+		$this->validator->validateEmail($formData->email);
 
 		if ($this->userDao->getByName($formData->name))
 			throw new \DomainException('User with this name already exists.');
@@ -48,20 +48,4 @@ class UserService
 
 		return $this->userDao->save($user);
 	}
-
-	//todo: refactor this to generic validation
-	public function validateUserName(&$userName)
-	{
-		$userName = trim($userName);
-
-		if (!$userName)
-			throw new \DomainException('User name cannot be empty.');
-
-		$minUserNameLength = intval($this->config->users->minUserNameLength);
-		$maxUserNameLength = intval($this->config->users->maxUserNameLength);
-		if (strlen($userName) < $minUserNameLength)
-			throw new \DomainException('User name must have at least ' . $minUserNameLength . ' character(s).');
-		if (strlen($userName) > $maxUserNameLength)
-			throw new \DomainException('User name must have at most ' . $maxUserNameLength . ' character(s).');
-	}
 }

src/Validator.php

@@ -0,0 +1,78 @@
+<?php
+namespace Szurubooru;
+
+class Validator
+{
+	private $config;
+
+	public function __construct(\Szurubooru\Config $config)
+	{
+		$this->config = $config;
+	}
+
+	public function validateNonEmpty($subject, $subjectName = 'Object')
+	{
+		if (!$subject)
+			throw new \DomainException($subjectName . ' cannot be empty.');
+	}
+
+	public function validateLength($subject, $minLength, $maxLength, $subjectName = 'Object')
+	{
+		$this->validateMinLength($subject, $minLength, $subjectName);
+		$this->validateMaxLength($subject, $maxLength, $subjectName);
+	}
+
+	public function validateMinLength($subject, $minLength, $subjectName = 'Object')
+	{
+		if (strlen($subject) < $minLength)
+			throw new \DomainException($subjectName . ' must have at least ' . $minLength . ' character(s).');
+	}
+
+	public function validateMaxLength($subject, $maxLength, $subjectName = 'Object')
+	{
+		if (strlen($subject) > $maxLength)
+			throw new \DomainException($subjectName . ' must have at most ' . $maxLength . ' character(s).');
+	}
+
+	public function validateUserName(&$userName)
+	{
+		$userName = trim($userName);
+
+		$minUserNameLength = intval($this->config->users->minUserNameLength);
+		$maxUserNameLength = intval($this->config->users->maxUserNameLength);
+		$this->validateNonEmpty($userName, 'User name');
+		$this->validateLength($userName, $minUserNameLength, $maxUserNameLength, 'User name');
+
+		if (preg_match('/[^a-zA-Z_-]/', $userName))
+		{
+			throw new \DomainException('User name may contain only characters, numbers, underscore (_) and dash (-).');
+		}
+	}
+
+	public function validateEmail($email)
+	{
+		if (!$email)
+			return;
+
+		if (!preg_match('/^[^@]+@[^@]+\.\w+$/', $email))
+			throw new \DomainException('Specified e-mail appears to be invalid.');
+	}
+
+	public function validatePassword($password)
+	{
+		$minPasswordLength = intval($this->config->security->minPasswordLength);
+		$this->validateNonEmpty($password, 'Password');
+		$this->validateMinLength($password, $minPasswordLength, 'Password');
+
+		if (preg_match('/[^\x20-\x7f]/', $password))
+		{
+			throw new \DomainException(
+				'Password may contain only characters from ASCII range to avoid potential problems with encoding.');
+		}
+	}
+
+	public function validateToken($token)
+	{
+		$this->validateNonEmpty($token, 'Token');
+	}
+}

tests/Services/AuthServiceTest.php

@@ -3,6 +3,7 @@ namespace Szurubooru\Tests\Services;
 
 class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase
 {
+	private $validatorMock;
 	private $passwordServiceMock;
 	private $timeServiceMock;
 	private $tokenDaoMock;
@@ -10,6 +11,7 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase
 
 	public function setUp()
 	{
+		$this->validatorMock = $this->mock(\Szurubooru\Validator::class);
 		$this->passwordServiceMock = $this->mock(\Szurubooru\Services\PasswordService::class);
 		$this->timeServiceMock = $this->mock(\Szurubooru\Services\TimeService::class);
 		$this->tokenDaoMock = $this->mock(\Szurubooru\Dao\TokenDao::class);
@@ -91,6 +93,7 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase
 	private function getAuthService()
 	{
 		return new \Szurubooru\Services\AuthService(
+			$this->validatorMock,
 			$this->passwordServiceMock,
 			$this->timeServiceMock,
 			$this->tokenDaoMock,

tests/Services/EmailServiceTest.php

@@ -1,28 +0,0 @@
-<?php
-namespace Szurubooru\Tests\Services;
-
-class EmailServiceTest extends \Szurubooru\Tests\AbstractTestCase
-{
-	public function testEmailWithoutAt()
-	{
-		$emailService = new \Szurubooru\Services\EmailService();
-
-		$this->setExpectedException(\DomainException::class);
-		$emailService->validateEmail('ghost');
-	}
-
-	public function testEmailWithoutDotInDomain()
-	{
-		$emailService = new \Szurubooru\Services\EmailService();
-
-		$this->setExpectedException(\DomainException::class);
-		$emailService->validateEmail('ghost@cemetery');
-	}
-
-	public function testValidEmail()
-	{
-		$emailService = new \Szurubooru\Services\EmailService();
-
-		$this->assertNull($emailService->validateEmail('ghost@cemetery.consulting'));
-	}
-}

tests/Services/UserServiceTest.php

@@ -3,7 +3,7 @@ namespace Szurubooru\Tests\Services;
 
 final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase
 {
-	private $configMock;
+	private $validatorMock;
 	private $userDaoMock;
 	private $passwordServiceMock;
 	private $emailServiceMock;
@@ -11,7 +11,7 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase
 
 	public function setUp()
 	{
-		$this->configMock = new \Szurubooru\Config;
+		$this->validatorMock = $this->mock(\Szurubooru\Validator::class);
 		$this->userDaoMock = $this->mock(\Szurubooru\Dao\UserDao::class);
 		$this->passwordServiceMock = $this->mock(\Szurubooru\Services\PasswordService::class);
 		$this->emailServiceMock = $this->mock(\Szurubooru\Services\EmailService::class);
@@ -25,11 +25,6 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase
 		$formData->password = 'password';
 		$formData->email = 'email';
 
-		//todo: this shouldn't be needed. refactor validation
-		$this->configMock->users = new \StdClass;
-		$this->configMock->users->minUserNameLength = 0;
-		$this->configMock->users->maxUserNameLength = 50;
-
 		$this->passwordServiceMock->method('getHash')->willReturn('hash');
 		$this->timeServiceMock->method('getCurrentTime')->willReturn('now');
 		$this->userDaoMock->method('hasAnyUsers')->willReturn(true);
@@ -52,11 +47,6 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase
 		$formData->password = 'password';
 		$formData->email = 'email';
 
-		//todo: this shouldn't be needed. refactor validation
-		$this->configMock->users = new \StdClass;
-		$this->configMock->users->minUserNameLength = 0;
-		$this->configMock->users->maxUserNameLength = 50;
-
 		$this->userDaoMock->method('hasAnyUsers')->willReturn(false);
 		$this->userDaoMock->method('save')->will($this->returnArgument(0));
 
@@ -73,11 +63,6 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase
 		$formData->password = 'password';
 		$formData->email = 'email';
 
-		//todo: this shouldn't be needed. refactor validation
-		$this->configMock->users = new \StdClass;
-		$this->configMock->users->minUserNameLength = 0;
-		$this->configMock->users->maxUserNameLength = 50;
-
 		$this->userDaoMock->method('hasAnyUsers')->willReturn(true);
 		$this->userDaoMock->method('getByName')->willReturn(new \Szurubooru\Entities\User());
 		$this->userDaoMock->method('save')->will($this->returnArgument(0));
@@ -91,7 +76,7 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase
 	private function getUserService()
 	{
 		return new \Szurubooru\Services\UserService(
-			$this->configMock,
+			$this->validatorMock,
 			$this->userDaoMock,
 			$this->passwordServiceMock,
 			$this->emailServiceMock,

tests/ValidatorTest.php

@@ -0,0 +1,149 @@
+<?php
+namespace Szurubooru\Tests;
+
+final class ValidatorTest extends \Szurubooru\Tests\AbstractTestCase
+{
+	private $config;
+
+	public function setUp()
+	{
+		$this->config = new \Szurubooru\Config();
+	}
+
+	public function testMinLengthName()
+	{
+		$validator = $this->getValidator();
+		$this->setExpectedException(\Exception::class, 'Object must have at least 50 character(s)');
+		$validator->validateMinLength('too short', 50);
+	}
+
+	public function testMaxLengthName()
+	{
+		$validator = $this->getValidator();
+		$this->setExpectedException(\Exception::class, 'Object must have at most 1 character(s)');
+		$validator->validateMaxLength('too long', 1);
+	}
+
+	public function testValidLengthName()
+	{
+		$validator = $this->getValidator();
+		$this->assertNull($validator->validateLength('fitting', 1, 50));
+		$this->assertNull($validator->validateMaxLength('fitting', 50));
+		$this->assertNull($validator->validateMinLength('fitting', 1));
+	}
+
+	public function testEmptyUserName()
+	{
+		$this->config->users = new \StdClass;
+		$this->config->users->minUserNameLength = 0;
+		$this->config->users->maxUserNameLength = 1;
+		$this->setExpectedException(\Exception::class, 'User name cannot be empty');
+		$userName = '';
+		$validator = $this->getValidator();
+		$validator->validateUserName($userName);
+	}
+
+	public function testTooShortUserName()
+	{
+		$this->config->users = new \StdClass;
+		$this->config->users->minUserNameLength = 30;
+		$this->config->users->maxUserNameLength = 50;
+		$this->setExpectedException(\Exception::class, 'User name must have at least 30 character(s)');
+		$userName = 'godzilla';
+		$validator = $this->getValidator();
+		$validator->validateUserName($userName);
+	}
+
+	public function testTooLongUserName()
+	{
+		$this->config->users = new \StdClass;
+		$this->config->users->minUserNameLength = 30;
+		$this->config->users->maxUserNameLength = 50;
+		$this->setExpectedException(\Exception::class, 'User name must have at most 50 character(s)');
+		$userName = 'godzilla' . str_repeat('a', 50);
+		$validator = $this->getValidator();
+		$validator->validateUserName($userName);
+	}
+
+	public function testUserNameWithSpaces()
+	{
+		$this->config->users = new \StdClass;
+		$this->config->users->minUserNameLength = 0;
+		$this->config->users->maxUserNameLength = 100;
+		$userName = ' godzilla ';
+		$validator = $this->getValidator();
+		$validator->validateUserName($userName);
+		$this->assertEquals('godzilla', $userName);
+	}
+
+	public function testUserNameWithInvalidCharacters()
+	{
+		$this->config->users = new \StdClass;
+		$this->config->users->minUserNameLength = 0;
+		$this->config->users->maxUserNameLength = 100;
+		$userName = '..:xXx:godzilla:xXx:..';
+		$this->setExpectedException(\Exception::class, 'User name may contain only');
+		$validator = $this->getValidator();
+		$validator->validateUserName($userName);
+	}
+
+	public function testEmailWithoutAt()
+	{
+		$validator = $this->getValidator();
+		$this->setExpectedException(\DomainException::class);
+		$validator->validateEmail('ghost');
+	}
+
+	public function testEmailWithoutDotInDomain()
+	{
+		$validator = $this->getValidator();
+		$this->setExpectedException(\DomainException::class);
+		$validator->validateEmail('ghost@cemetery');
+	}
+
+	public function testValidEmail()
+	{
+		$validator = $this->getValidator();
+		$this->assertNull($validator->validateEmail('ghost@cemetery.consulting'));
+	}
+
+	public function testEmptyPassword()
+	{
+		$this->config->security = new \StdClass;
+		$this->config->security->minPasswordLength = 0;
+		$this->setExpectedException(\Exception::class, 'Password cannot be empty');
+		$validator = $this->getValidator();
+		$validator->validatePassword('');
+	}
+
+	public function testTooShortPassword()
+	{
+		$this->config->security = new \StdClass;
+		$this->config->security->minPasswordLength = 10000;
+		$this->setExpectedException(\Exception::class, 'Password must have at least 10000 character(s)');
+		$validator = $this->getValidator();
+		$validator->validatePassword('password123');
+	}
+
+	public function testNonAsciiPassword()
+	{
+		$this->config->security = new \StdClass;
+		$this->config->security->minPasswordLength = 0;
+		$this->setExpectedException(\Exception::class, 'Password may contain only');
+		$validator = $this->getValidator();
+		$validator->validatePassword('良いパスワード');
+	}
+
+	public function testValidPassword()
+	{
+		$this->config->security = new \StdClass;
+		$this->config->security->minPasswordLength = 0;
+		$validator = $this->getValidator();
+		$this->assertNull($validator->validatePassword('password'));
+	}
+
+	private function getValidator()
+	{
+		return new \Szurubooru\Validator($this->config);
+	}
+}