From 9882e84aa6d402c3f1cc0b876657056acf799984 Mon Sep 17 00:00:00 2001 From: Marcin Kurczewski Date: Fri, 9 May 2014 21:23:54 +0200 Subject: [PATCH] Finished user validation; increased readability --- src/Api/Jobs/EditUserEmailJob.php | 2 +- src/Models/Entities/CommentEntity.php | 9 +-- src/Models/Entities/PostEntity.php | 2 +- src/Models/Entities/TagEntity.php | 2 +- src/Models/Entities/UserEntity.php | 31 ++++++---- src/Models/UserModel.php | 11 ---- tests/JobTests/EditUserEmailJobTest.php | 79 +++++++++++++++++++++++++ 7 files changed, 105 insertions(+), 31 deletions(-) create mode 100644 tests/JobTests/EditUserEmailJobTest.php diff --git a/src/Api/Jobs/EditUserEmailJob.php b/src/Api/Jobs/EditUserEmailJob.php index bd4ad48d..f02d14a2 100644 --- a/src/Api/Jobs/EditUserEmailJob.php +++ b/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) diff --git a/src/Models/Entities/CommentEntity.php b/src/Models/Entities/CommentEntity.php index 56e7d7f5..f8d25c27 100644 --- a/src/Models/Entities/CommentEntity.php +++ b/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() diff --git a/src/Models/Entities/PostEntity.php b/src/Models/Entities/PostEntity.php index 839b9862..11a08d1e 100644 --- a/src/Models/Entities/PostEntity.php +++ b/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) diff --git a/src/Models/Entities/TagEntity.php b/src/Models/Entities/TagEntity.php index 54f37bf1..391352db 100644 --- a/src/Models/Entities/TagEntity.php +++ b/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() diff --git a/src/Models/Entities/UserEntity.php b/src/Models/Entities/UserEntity.php index 86ceb1d9..3e59cd60 100644 --- a/src/Models/Entities/UserEntity.php +++ b/src/Models/Entities/UserEntity.php @@ -45,8 +45,8 @@ final class UserEntity extends AbstractEntity implements IValidatable { $this->validateUserName(); $this->validatePassword(); - - //todo: validate e-mails + $this->validateAccessRank(); + $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) - throw new Exception('Cannot set special access rank "%s"', $accessRank->toString()); - - return $accessRank; + if ($this->accessRank->toInteger() == AccessRank::Nobody) + throw new Exception('Cannot set special access rank "%s"', $this->accessRank->toString()); } + 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() diff --git a/src/Models/UserModel.php b/src/Models/UserModel.php index 9216f796..fd1b027b 100644 --- a/src/Models/UserModel.php +++ b/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]'; diff --git a/tests/JobTests/EditUserEmailJobTest.php b/tests/JobTests/EditUserEmailJobTest.php new file mode 100644 index 00000000..d0a66ad4 --- /dev/null +++ b/tests/JobTests/EditUserEmailJobTest.php @@ -0,0 +1,79 @@ +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'); + + } +}