Adjust Validator Signatures (#598)

systemli · Apr 20, 2024 · 86c3e64 · 86c3e64
1 parent 2e3ec40
commit 86c3e64
Show file tree

Hide file tree

Showing 10 changed files with 195 additions and 106 deletions.
diff --git a/src/Validator/Constraints/EmailAddressValidator.php b/src/Validator/Constraints/EmailAddressValidator.php
@@ -6,43 +6,18 @@
 use App\Entity\Domain;
 use App\Entity\ReservedName;
 use App\Entity\User;
-use App\Repository\AliasRepository;
-use App\Repository\DomainRepository;
-use App\Repository\ReservedNameRepository;
-use App\Repository\UserRepository;
 use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 use Symfony\Component\Validator\Exception\UnexpectedTypeException;
 
-/**
- * Class EmailAddressValidator.
- */
 class EmailAddressValidator extends ConstraintValidator
 {
-    private readonly AliasRepository $aliasRepository;
-    private readonly DomainRepository $domainRepository;
-    private readonly ReservedNameRepository $reservedNameRepository;
-    private readonly UserRepository $userRepository;
-
-    /**
-     * EmailAddressValidator constructor.
-     */
-    public function __construct(EntityManagerInterface $manager)
+    public function __construct(private readonly EntityManagerInterface $manager)
     {
-        $this->aliasRepository = $manager->getRepository(Alias::class);
-        $this->domainRepository = $manager->getRepository(Domain::class);
-        $this->reservedNameRepository = $manager->getRepository(ReservedName::class);
-        $this->userRepository = $manager->getRepository(User::class);
     }
 
-    /**
-     * Checks if the passed value is valid.
-     *
-     * @param string     $value      The value that should be validated
-     * @param Constraint $constraint The constraint for the validation
-     */
-    public function validate($value, Constraint $constraint): void
+    public function validate(mixed $value, Constraint $constraint): void
     {
         if (!$constraint instanceof EmailAddress) {
             throw new UnexpectedTypeException($constraint, EmailAddress::class);
@@ -57,21 +32,20 @@ public function validate($value, Constraint $constraint): void
         }
 
         [$localPart, $domain] = explode('@', $value);
+        $user = $this->manager->getRepository(User::class)->findOneBy(['email' => $value], null, true);
+        $alias = $this->manager->getRepository(Alias::class)->findOneBySource($value, true);
+        $reservedName = $this->manager->getRepository(ReservedName::class)->findByName($localPart);
 
-        if (null !== $this->userRepository->findOneBy(['email' => $value], null, true)) {
-            $this->context->addViolation('registration.email-already-taken');
-        } elseif (null !== $this->aliasRepository->findOneBySource($value, true)) {
-            $this->context->addViolation('registration.email-already-taken');
-        } elseif (null !== $this->reservedNameRepository->findByName($localPart)) {
+        if (null !== $user || null !== $alias || null !== $reservedName) {
             $this->context->addViolation('registration.email-already-taken');
         }
 
-        if (1 !== preg_match('/^[a-z0-9\-\_\.]*$/ui', $localPart)) {
+        if (1 !== preg_match('/^[a-z0-9\-_.]*$/ui', $localPart)) {
             $this->context->addViolation('registration.email-unexpected-characters');
         }
 
         // check if email domain is in domain repository
-        if (null === $this->domainRepository->findByName($domain)) {
+        if (null === $this->manager->getRepository(Domain::class)->findByName($domain)) {
             $this->context->addViolation('registration.email-domain-not-exists');
         }
     }

diff --git a/src/Validator/Constraints/EmailDomainValidator.php b/src/Validator/Constraints/EmailDomainValidator.php
@@ -8,33 +8,23 @@
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 
-/**
- * Class EmailDomainValidator.
- */
 class EmailDomainValidator extends ConstraintValidator
 {
-    /**
-     * EmailDomainValidator constructor.
-     */
     public function __construct(private readonly EntityManagerInterface $manager)
     {
     }
 
-    /**
-     * Checks if the passed value is valid.
-     *
-     * @param User       $value      The value that should be validated
-     * @param Constraint $constraint The constraint for the validation
-     */
-    public function validate($value, Constraint $constraint): void
+    public function validate(mixed $value, Constraint $constraint): void
     {
-        if ($value instanceof User) {
-            $name = substr(strrchr((string) $value->getEmail(), '@'), 1);
-            $domain = $this->manager->getRepository(Domain::class)->findOneBy(['name' => $name]);
+        if (!$value instanceof User) {
+            return;
+        }
+
+        $name = substr(strrchr((string)$value->getEmail(), '@'), 1);
+        $domain = $this->manager->getRepository(Domain::class)->findOneBy(['name' => $name]);
 
-            if (null === $domain) {
-                $this->context->addViolation('form.missing-domain');
-            }
+        if (null === $domain) {
+            $this->context->addViolation('form.missing-domain');
         }
     }
 }
diff --git a/src/Validator/Constraints/EmailLengthValidator.php b/src/Validator/Constraints/EmailLengthValidator.php
@@ -6,18 +6,9 @@
 use Symfony\Component\Validator\ConstraintValidator;
 use Symfony\Component\Validator\Exception\UnexpectedTypeException;
 
-/**
- * Class EmailLengthValidator.
- */
 class EmailLengthValidator extends ConstraintValidator
 {
-    /**
-     * Checks if the passed value is valid.
-     *
-     * @param string     $value      The value that should be validated
-     * @param Constraint $constraint The constraint for the validation
-     */
-    public function validate($value, Constraint $constraint): void
+    public function validate(mixed $value, Constraint $constraint): void
     {
         if (!$constraint instanceof EmailLength) {
             throw new UnexpectedTypeException($constraint, EmailLength::class);

diff --git a/src/Validator/Constraints/VoucherExistsValidator.php b/src/Validator/Constraints/VoucherExistsValidator.php
@@ -20,13 +20,7 @@ public function __construct(EntityManagerInterface $manager)
         $this->voucherRepository = $manager->getRepository(Voucher::class);
     }
 
-    /**
-     * Checks if the passed value is valid.
-     *
-     * @param mixed      $value      The value that should be validated
-     * @param Constraint $constraint The constraint for the validation
-     */
-    public function validate($value, Constraint $constraint): void
+    public function validate(mixed $value, Constraint $constraint): void
     {
         if (!$constraint instanceof VoucherExists) {
             throw new UnexpectedTypeException($constraint, VoucherExists::class);
@@ -36,7 +30,7 @@ public function validate($value, Constraint $constraint): void
             throw new UnexpectedTypeException($value, 'string');
         }
 
-        $stringValue = (string) $value;
+        $stringValue = (string)$value;
 
         if (true === $constraint->exists) {
             if (null === $voucher = $this->voucherRepository->findByCode($stringValue)) {

diff --git a/src/Validator/Constraints/VoucherUserValidator.php b/src/Validator/Constraints/VoucherUserValidator.php
@@ -11,12 +11,6 @@
 
 class VoucherUserValidator extends ConstraintValidator
 {
-    /**
-     * Checks if the passed value is valid.
-     *
-     * @param mixed      $value      The value that should be validated
-     * @param Constraint $constraint The constraint for the validation
-     */
     public function validate(mixed $value, Constraint $constraint): void
     {
         if (!$constraint instanceof VoucherUser) {
@@ -27,9 +21,8 @@ public function validate(mixed $value, Constraint $constraint): void
             return;
         }
 
-        /** @var User $user */
         $user = $value->getUser();
-        if ($user->hasRole(Roles::SUSPICIOUS)) {
+        if (null !== $user && $user->hasRole(Roles::SUSPICIOUS)) {
             $this->context->addViolation('voucher.suspicious-user');
         }
     }

diff --git a/src/Validator/PasswordChangeValidator.php b/src/Validator/PasswordChangeValidator.php
@@ -13,21 +13,11 @@
 
 class PasswordChangeValidator extends ConstraintValidator
 {
-    /**
-     * Constructor.
-     */
     public function __construct(private readonly TokenStorageInterface $storage, private readonly PasswordHasherFactoryInterface $passwordHasherFactory)
     {
     }
 
-    /**
-     * Checks if the passed value is valid.
-     *
-     * @param PasswordChange $value
-     *
-     * @throws UnexpectedTypeException
-     */
-    public function validate($value, Constraint $constraint): bool
+    public function validate(mixed $value, Constraint $constraint): void
     {
         if (!$value instanceof PasswordChange) {
             throw new UnexpectedTypeException('Wrong value type given', Registration::class);
@@ -39,16 +29,10 @@ public function validate($value, Constraint $constraint): bool
 
         if (!$hasher->verify($user->getPassword(), $value->password)) {
             $this->context->addViolation('form.wrong-password');
-
-            return false;
         }
 
         if ($value->password === $value->getPlainPassword()) {
             $this->context->addViolation('form.identical-passwords');
-
-            return false;
         }
-
-        return true;
     }
 }
diff --git a/src/Validator/PasswordPolicyValidator.php b/src/Validator/PasswordPolicyValidator.php
@@ -12,13 +12,10 @@ public function __construct(private readonly PasswordStrengthHandler $handler)
     {
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function validate($value, Constraint $constraint): bool
+    public function validate(mixed $value, Constraint $constraint): void
     {
-        if (empty($value)) {
-            return true;
+        if (empty($value) || !is_string($value)) {
+            return;
         }
 
         $errors = $this->handler->validate($value);
@@ -27,10 +24,6 @@ public function validate($value, Constraint $constraint): bool
             foreach ($errors as $error) {
                 $this->context->addViolation($error);
             }
-
-            return false;
         }
-
-        return true;
     }
 }
diff --git a/tests/Validator/Constraints/EmailDomainValidatorTest.php b/tests/Validator/Constraints/EmailDomainValidatorTest.php
@@ -0,0 +1,52 @@
+<?php
+
+namespace App\Tests\Validator\Constraints;
+
+use App\Entity\User;
+use App\Repository\DomainRepository;
+use App\Validator\Constraints\EmailDomain;
+use App\Validator\Constraints\EmailDomainValidator;
+use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
+
+class EmailDomainValidatorTest extends ConstraintValidatorTestCase
+{
+    protected function createValidator(): EmailDomainValidator
+    {
+        $repository = $this->createMock(DomainRepository::class);
+        $repository->expects($this->any())
+            ->method('findOneBy')
+            ->will($this->returnValue(null));
+
+        $manager = $this->createMock(EntityManagerInterface::class);
+        $manager->expects($this->any())
+            ->method('getRepository')
+            ->will($this->returnValue($repository));
+
+        return new EmailDomainValidator($manager);
+    }
+
+    public function testNullIsValid(): void
+    {
+        $this->validator->validate(null, new EmailDomain());
+
+        $this->assertNoViolation();
+    }
+
+    public function testEmptyStringIsValid(): void
+    {
+        $this->validator->validate('', new EmailDomain());
+
+        $this->assertNoViolation();
+    }
+
+    public function testDomainNotFound(): void
+    {
+        $user = new User();
+        $user->setEmail('user@example.com');
+        $this->validator->validate($user, new EmailDomain());
+
+        $this->buildViolation('form.missing-domain')
+            ->assertRaised();
+    }
+}
diff --git a/tests/Validator/Constraints/PasswordPolicyValidatorTest.php b/tests/Validator/Constraints/PasswordPolicyValidatorTest.php
@@ -0,0 +1,46 @@
+<?php
+
+namespace App\Tests\Validator\Constraints;
+
+use App\Handler\PasswordStrengthHandler;
+use App\Validator\Constraints\PasswordPolicy;
+use App\Validator\PasswordPolicyValidator;
+use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
+
+class PasswordPolicyValidatorTest extends ConstraintValidatorTestCase
+{
+    protected function createValidator(): PasswordPolicyValidator
+    {
+        $passwordStrengthHandler = new PasswordStrengthHandler();
+        return new PasswordPolicyValidator($passwordStrengthHandler);
+    }
+
+    public function testNullIsValid(): void
+    {
+        $this->validator->validate(null, new PasswordPolicy());
+
+        $this->assertNoViolation();
+    }
+
+    public function testEmptyStringIsValid(): void
+    {
+        $this->validator->validate('', new PasswordPolicy());
+
+        $this->assertNoViolation();
+    }
+
+    public function testValidPassword(): void
+    {
+        $this->validator->validate('Password123!', new PasswordPolicy());
+
+        $this->assertNoViolation();
+    }
+
+    public function testInvalidPassword(): void
+    {
+        $this->validator->validate('password', new PasswordPolicy());
+
+        $this->buildViolation('form.weak_password')
+            ->assertRaised();
+    }
+}