Finished user validation; increased readability

src/Api/Jobs/EditUserEmailJob.php

@@ -15,7 +15,7 @@ class EditUserEmailJob extends AbstractUserJob
 				throw new SimpleException('E-mail address is required - you will be sent confirmation e-mail.');
 
 		$user = $this->user;
-		$newEmail = UserModel::validateEmail($this->getArgument(self::NEW_EMAIL));
+		$newEmail = $this->getArgument(self::NEW_EMAIL);
 
 		$oldEmail = $user->getConfirmedEmail();
 		if ($oldEmail == $newEmail)

src/Models/Entities/CommentEntity.php

@@ -22,13 +22,12 @@ final class CommentEntity extends AbstractEntity implements IValidatable
 
 	public function validate()
 	{
-		$text = trim($this->getText());
 		$config = getConfig();
 
-		if (strlen($text) < $config->comments->minLength)
+		if (strlen($this->getText()) < $config->comments->minLength)
 			throw new SimpleException('Comment must have at least %d characters', $config->comments->minLength);
 
-		if (strlen($text) > $config->comments->maxLength)
+		if (strlen($this->getText()) > $config->comments->maxLength)
 			throw new SimpleException('Comment must have at most %d characters', $config->comments->maxLength);
 
 		if (!$this->getPostId())
@@ -36,8 +35,6 @@ final class CommentEntity extends AbstractEntity implements IValidatable
 
 		if (!$this->getCreationTime())
 			throw new SimpleException('Trying to save comment that has no creation date specified');
-
-		$this->setText($text);
 	}
 
 	public function getText()
@@ -52,7 +49,7 @@ final class CommentEntity extends AbstractEntity implements IValidatable
 
 	public function setText($text)
 	{
-		$this->text = $text;
+		$this->text = $text === null ? null : trim($text);
 	}
 
 	public function getPost()

src/Models/Entities/PostEntity.php

@@ -336,7 +336,7 @@ final class PostEntity extends AbstractEntity implements IValidatable
 
 	public function setSource($source)
 	{
-		$this->source = trim($source);
+		$this->source = $source === null ? null : trim($source);
 	}
 
 	public function getThumbCustomPath($width = null, $height = null)

src/Models/Entities/TagEntity.php

@@ -41,7 +41,7 @@ final class TagEntity extends AbstractEntity implements IValidatable
 
 	public function setName($name)
 	{
-		$this->name = trim($name);
+		$this->name = $name === null ? null : trim($name);
 	}
 
 	public function getName()

src/Models/Entities/UserEntity.php

@@ -45,8 +45,8 @@ final class UserEntity extends AbstractEntity implements IValidatable
 	{
 		$this->validateUserName();
 		$this->validatePassword();
-
+		$this->validateAccessRank();
-		//todo: validate e-mails
+		$this->validateEmails();
 
 		if (empty($this->getAccessRank()))
 			throw new Exception('No access rank detected');
@@ -114,16 +114,25 @@ final class UserEntity extends AbstractEntity implements IValidatable
 			throw new SimpleException('Password contains invalid characters');
 	}
 
-	public static function validateAccessRank(AccessRank $accessRank)
+	public function validateAccessRank()
 	{
-		$accessRank->validate();
+		$this->accessRank->validate();
 
-		if ($accessRank->toInteger() == AccessRank::Nobody)
+		if ($this->accessRank->toInteger() == AccessRank::Nobody)
-			throw new Exception('Cannot set special access rank "%s"', $accessRank->toString());
+			throw new Exception('Cannot set special access rank "%s"', $this->accessRank->toString());
-
-		return $accessRank;
 	}
 
+	public function validateEmails()
+	{
+		$this->validateEmail($this->getUnconfirmedEmail());
+		$this->validateEmail($this->getConfirmedEmail());
+	}
+
+	private function validateEmail($email)
+	{
+		if (!empty($email) and !TextHelper::isValidEmail($email))
+			throw new SimpleException('E-mail address appears to be invalid');
+	}
 
 	public function isBanned()
 	{
@@ -147,7 +156,7 @@ final class UserEntity extends AbstractEntity implements IValidatable
 
 	public function setName($name)
 	{
-		$this->name = trim($name);
+		$this->name = $name === null ? null : trim($name);
 	}
 
 	public function getJoinTime()
@@ -177,7 +186,7 @@ final class UserEntity extends AbstractEntity implements IValidatable
 
 	public function setUnconfirmedEmail($email)
 	{
-		$this->emailUnconfirmed = $email;
+		$this->emailUnconfirmed = $email === null ? null : trim($email);
 	}
 
 	public function getConfirmedEmail()
@@ -187,7 +196,7 @@ final class UserEntity extends AbstractEntity implements IValidatable
 
 	public function setConfirmedEmail($email)
 	{
-		$this->emailConfirmed = $email;
+		$this->emailConfirmed = $email === null ? null : trim($email);
 	}
 
 	public function isStaffConfirmed()

src/Models/UserModel.php

@@ -187,17 +187,6 @@ final class UserModel extends AbstractCrudModel
 		});
 	}
 
-	public static function validateEmail($email)
-	{
-		$email = trim($email);
-
-		if (!empty($email) and !TextHelper::isValidEmail($email))
-			throw new SimpleException('E-mail address appears to be invalid');
-
-		return $email;
-	}
-
-
 	public static function getAnonymousName()
 	{
 		return '[Anonymous user]';

tests/JobTests/EditUserEmailJobTest.php

@@ -0,0 +1,79 @@
+<?php
+class EditUserEmailJobTest extends AbstractTest
+{
+	public function testNoConfirmation()
+	{
+		getConfig()->registration->needEmailForRegistering = false;
+		Mailer::mockSending();
+		$this->assert->areEqual(0, Mailer::getMailCounter());
+
+		getConfig()->privileges->changeUserEmailNoConfirm = 'anonymous';
+		$this->grantAccess('changeUserEmail');
+
+		$user = $this->mockUser();
+
+		$user = $this->assert->doesNotThrow(function() use ($user)
+		{
+			return Api::run(
+				new EditUserEmailJob(),
+				[
+					EditUserEmailJob::USER_NAME => $user->getName(),
+					EditUserEmailJob::NEW_EMAIL => 'xena@other-side.gr',
+				]);
+		});
+
+		$this->assert->areEqual(null, $user->getUnconfirmedEmail());
+		$this->assert->areEqual('xena@other-side.gr', $user->getConfirmedEmail());
+
+		$this->assert->areEqual(0, Mailer::getMailCounter());
+	}
+
+	public function testConfirmation()
+	{
+		getConfig()->registration->needEmailForRegistering = false;
+		Mailer::mockSending();
+		$this->assert->areEqual(0, Mailer::getMailCounter());
+
+		getConfig()->privileges->changeUserEmailNoConfirm = 'admin';
+		$this->grantAccess('changeUserEmail');
+
+		$user = $this->mockUser();
+
+		$user = $this->assert->doesNotThrow(function() use ($user)
+		{
+			return Api::run(
+				new EditUserEmailJob(),
+				[
+					EditUserEmailJob::USER_NAME => $user->getName(),
+					EditUserEmailJob::NEW_EMAIL => 'xena@other-side.gr',
+				]);
+		});
+
+		$this->assert->areEqual('xena@other-side.gr', $user->getUnconfirmedEmail());
+		$this->assert->areEqual(null, $user->getConfirmedEmail());
+
+		$this->assert->areEqual(1, Mailer::getMailCounter());
+	}
+
+	public function testInvalidEmail()
+	{
+		getConfig()->registration->needEmailForRegistering = false;
+		Mailer::mockSending();
+
+		getConfig()->privileges->changeUserEmailNoConfirm = 'nobody';
+		$this->grantAccess('changeUserEmail');
+
+		$user = $this->mockUser();
+
+		$this->assert->throws(function() use ($user)
+		{
+			Api::run(
+				new EditUserEmailJob(),
+				[
+					EditUserEmailJob::USER_NAME => $user->getName(),
+					EditUserEmailJob::NEW_EMAIL => 'hrmfbpdvpds@brtedf',
+				]);
+		}, 'E-mail address appears to be invalid');
+
+	}
+}