From b6efb9a8d5f1948c23b36172f2f4a074d9a9b619 Mon Sep 17 00:00:00 2001 From: Grzegorz Sadowski Date: Wed, 6 Oct 2021 13:01:01 +0200 Subject: [PATCH] [CatalogPromotion] Refactor scope validators to do an actual validation --- .../Resources/config/services/validators.xml | 5 +- .../ForTaxonsScopeValidator.php | 13 +++-- .../ForVariantsScopeValidator.php | 13 +++-- .../ScopeValidatorInterface.php | 3 +- .../CatalogPromotionScopeValidator.php | 13 +++-- .../ForTaxonsScopeValidatorSpec.php | 40 ++++++++----- .../ForVariantsScopeValidatorSpec.php | 56 ++++++++++++------- .../CatalogPromotionScopeValidatorSpec.php | 26 +++------ 8 files changed, 98 insertions(+), 71 deletions(-) diff --git a/src/Sylius/Bundle/CoreBundle/Resources/config/services/validators.xml b/src/Sylius/Bundle/CoreBundle/Resources/config/services/validators.xml index dabfbc091d1..b95ecf06ff2 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services/validators.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services/validators.xml @@ -66,8 +66,8 @@ - for_taxons - for_variants + Sylius\Component\Core\Model\CatalogPromotionScopeInterface::TYPE_FOR_TAXONS + Sylius\Component\Core\Model\CatalogPromotionScopeInterface::TYPE_FOR_VARIANTS @@ -78,7 +78,6 @@ - diff --git a/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForTaxonsScopeValidator.php b/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForTaxonsScopeValidator.php index 5ac5261abbb..705e7d2864f 100644 --- a/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForTaxonsScopeValidator.php +++ b/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForTaxonsScopeValidator.php @@ -16,6 +16,7 @@ use Sylius\Bundle\CoreBundle\Validator\Constraints\CatalogPromotionScope; use Sylius\Component\Taxonomy\Repository\TaxonRepositoryInterface; use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Context\ExecutionContextInterface; use Webmozart\Assert\Assert; final class ForTaxonsScopeValidator implements ScopeValidatorInterface @@ -27,21 +28,23 @@ public function __construct(TaxonRepositoryInterface $taxonRepository) $this->taxonRepository = $taxonRepository; } - public function validate(array $configuration, Constraint $constraint): array + public function validate(array $configuration, Constraint $constraint, ExecutionContextInterface $context): void { /** @var CatalogPromotionScope $constraint */ Assert::isInstanceOf($constraint, CatalogPromotionScope::class); if (!isset($configuration['taxons']) || empty($configuration['taxons'])) { - return ['configuration.taxons' => $constraint->taxonsNotEmpty]; + $context->buildViolation($constraint->taxonsNotEmpty)->atPath('configuration.taxons')->addViolation(); + + return; } foreach ($configuration['taxons'] as $taxonCode) { if (null === $this->taxonRepository->findOneBy(['code' => $taxonCode])) { - return ['configuration.taxons' => $constraint->invalidTaxons]; + $context->buildViolation($constraint->invalidTaxons)->atPath('configuration.taxons')->addViolation(); + + return; } } - - return []; } } diff --git a/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForVariantsScopeValidator.php b/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForVariantsScopeValidator.php index 2c91b59d369..4e2b5bc8095 100644 --- a/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForVariantsScopeValidator.php +++ b/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForVariantsScopeValidator.php @@ -16,6 +16,7 @@ use Sylius\Bundle\CoreBundle\Validator\Constraints\CatalogPromotionScope; use Sylius\Component\Core\Repository\ProductVariantRepositoryInterface; use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Context\ExecutionContextInterface; use Webmozart\Assert\Assert; final class ForVariantsScopeValidator implements ScopeValidatorInterface @@ -27,21 +28,23 @@ public function __construct(ProductVariantRepositoryInterface $variantRepository $this->variantRepository = $variantRepository; } - public function validate(array $configuration, Constraint $constraint): array + public function validate(array $configuration, Constraint $constraint, ExecutionContextInterface $context): void { /** @var CatalogPromotionScope $constraint */ Assert::isInstanceOf($constraint, CatalogPromotionScope::class); if (!array_key_exists('variants', $configuration) || empty($configuration['variants'])) { - return ['configuration.variants' => $constraint->variantsNotEmpty]; + $context->buildViolation($constraint->variantsNotEmpty)->atPath('configuration.variants')->addViolation(); + + return; } foreach ($configuration['variants'] as $variantCode) { if (null === $this->variantRepository->findOneBy(['code' => $variantCode])) { - return ['configuration.variants' => $constraint->invalidVariants]; + $context->buildViolation($constraint->invalidVariants)->atPath('configuration.variants')->addViolation(); + + return; } } - - return []; } } diff --git a/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ScopeValidatorInterface.php b/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ScopeValidatorInterface.php index 4e35b5ced44..015529707a5 100644 --- a/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ScopeValidatorInterface.php +++ b/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ScopeValidatorInterface.php @@ -14,8 +14,9 @@ namespace Sylius\Bundle\CoreBundle\Validator\CatalogPromotionScope; use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Context\ExecutionContextInterface; interface ScopeValidatorInterface { - public function validate(array $configuration, Constraint $constraint): array; + public function validate(array $configuration, Constraint $constraint, ExecutionContextInterface $context): void; } diff --git a/src/Sylius/Bundle/CoreBundle/Validator/Constraints/CatalogPromotionScopeValidator.php b/src/Sylius/Bundle/CoreBundle/Validator/Constraints/CatalogPromotionScopeValidator.php index a874e31c46c..b03fdc1322f 100644 --- a/src/Sylius/Bundle/CoreBundle/Validator/Constraints/CatalogPromotionScopeValidator.php +++ b/src/Sylius/Bundle/CoreBundle/Validator/Constraints/CatalogPromotionScopeValidator.php @@ -43,14 +43,15 @@ public function validate($value, Constraint $constraint): void return; } + $type = $value->getType(); + if (!key_exists($type, $this->scopeValidators)) { + return; + } + $configuration = $value->getConfiguration(); /** @var ScopeValidatorInterface $validator */ - $validator = $this->scopeValidators[$value->getType()]; - $violations = $validator->validate($configuration, $constraint); - - foreach ($violations as $path => $message) { - $this->context->buildViolation($message)->atPath($path)->addViolation(); - } + $validator = $this->scopeValidators[$type]; + $validator->validate($configuration, $constraint, $this->context); } } diff --git a/src/Sylius/Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForTaxonsScopeValidatorSpec.php b/src/Sylius/Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForTaxonsScopeValidatorSpec.php index 2b6d1003365..f038c65792e 100644 --- a/src/Sylius/Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForTaxonsScopeValidatorSpec.php +++ b/src/Sylius/Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForTaxonsScopeValidatorSpec.php @@ -18,6 +18,8 @@ use Sylius\Bundle\CoreBundle\Validator\Constraints\CatalogPromotionScope; use Sylius\Component\Core\Model\TaxonInterface; use Sylius\Component\Taxonomy\Repository\TaxonRepositoryInterface; +use Symfony\Component\Validator\Context\ExecutionContextInterface; +use Symfony\Component\Validator\Violation\ConstraintViolationBuilderInterface; final class ForTaxonsScopeValidatorSpec extends ObjectBehavior { @@ -31,31 +33,41 @@ function it_is_a_scope_validator(): void $this->shouldHaveType(ScopeValidatorInterface::class); } - function it_prepares_array_with_violation_if_catalog_promotion_scope_does_not_have_taxons_key_configured(): void - { - $this - ->validate([], new CatalogPromotionScope()) - ->shouldReturn(['configuration.taxons' => 'sylius.catalog_promotion_scope.for_taxons.not_empty']) - ; + function it_adds_violation_if_catalog_promotion_scope_does_not_have_taxons_key_configured( + ExecutionContextInterface $executionContext, + ConstraintViolationBuilderInterface $constraintViolationBuilder + ): void { + $executionContext->buildViolation('sylius.catalog_promotion_scope.for_taxons.not_empty')->willReturn($constraintViolationBuilder); + $constraintViolationBuilder->atPath('configuration.taxons')->willReturn($constraintViolationBuilder); + $constraintViolationBuilder->addViolation()->shouldBeCalled(); + + $this->validate([], new CatalogPromotionScope(), $executionContext); } - function it_prepares_array_with_violation_if_catalog_promotion_scope_has_not_existing_taxons_configured( - TaxonRepositoryInterface $taxonRepository + function it_adds_violation_if_catalog_promotion_scope_has_not_existing_taxons_configured( + TaxonRepositoryInterface $taxonRepository, + ExecutionContextInterface $executionContext, + ConstraintViolationBuilderInterface $constraintViolationBuilder ): void { $taxonRepository->findOneBy(['code' => 'not_existing_taxon'])->willReturn(null); - $this - ->validate(['taxons' => ['not_existing_taxon']], new CatalogPromotionScope()) - ->shouldReturn(['configuration.taxons' => 'sylius.catalog_promotion_scope.for_taxons.invalid_taxons']) - ; + $executionContext->buildViolation('sylius.catalog_promotion_scope.for_taxons.invalid_taxons')->willReturn($constraintViolationBuilder); + $constraintViolationBuilder->atPath('configuration.taxons')->willReturn($constraintViolationBuilder); + $constraintViolationBuilder->addViolation()->shouldBeCalled(); + + $this->validate(['taxons' => ['not_existing_taxon']], new CatalogPromotionScope(), $executionContext); } - function it_returns_an_empty_array_if_catalog_promotion_scope_is_valid( + function it_does_nothing_if_catalog_promotion_scope_is_valid( TaxonRepositoryInterface $taxonRepository, + ExecutionContextInterface $executionContext, TaxonInterface $taxon ): void { $taxonRepository->findOneBy(['code' => 'taxon'])->willReturn($taxon); - $this->validate(['taxons' => ['taxon']], new CatalogPromotionScope())->shouldReturn([]); + $executionContext->buildViolation('sylius.catalog_promotion_scope.for_taxons.not_empty')->shouldNotBeCalled(); + $executionContext->buildViolation('sylius.catalog_promotion_scope.for_taxons.invalid_taxons')->shouldNotBeCalled(); + + $this->validate(['taxons' => ['taxon']], new CatalogPromotionScope(), $executionContext); } } diff --git a/src/Sylius/Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForVariantsScopeValidatorSpec.php b/src/Sylius/Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForVariantsScopeValidatorSpec.php index 6ec864f0d16..f6d7f6ffd6b 100644 --- a/src/Sylius/Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForVariantsScopeValidatorSpec.php +++ b/src/Sylius/Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForVariantsScopeValidatorSpec.php @@ -18,6 +18,9 @@ use Sylius\Bundle\CoreBundle\Validator\Constraints\CatalogPromotionScope; use Sylius\Component\Core\Model\ProductVariantInterface; use Sylius\Component\Core\Repository\ProductVariantRepositoryInterface; +use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Context\ExecutionContextInterface; +use Symfony\Component\Validator\Violation\ConstraintViolationBuilderInterface; final class ForVariantsScopeValidatorSpec extends ObjectBehavior { @@ -31,41 +34,54 @@ function it_is_a_scope_validator(): void $this->shouldHaveType(ScopeValidatorInterface::class); } - function it_prepares_array_with_violation_if_catalog_promotion_scope_does_not_have_variants_key_configured(): void - { - $this - ->validate([], new CatalogPromotionScope()) - ->shouldReturn(['configuration.variants' => 'sylius.catalog_promotion_scope.for_variants.not_empty']) - ; + function it_adds_violation_if_catalog_promotion_scope_does_not_have_variants_key_configured( + ExecutionContextInterface $executionContext, + ConstraintViolationBuilderInterface $constraintViolationBuilder + ): void { + $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.not_empty')->willReturn($constraintViolationBuilder); + $constraintViolationBuilder->atPath('configuration.variants')->willReturn($constraintViolationBuilder); + $constraintViolationBuilder->addViolation()->shouldBeCalled(); + + $this->validate([], new CatalogPromotionScope(), $executionContext); } - function it_prepares_array_with_violation_if_catalog_promotion_scope_has_empty_variants_configured(): void - { - $this - ->validate(['variants' => []], new CatalogPromotionScope()) - ->shouldReturn(['configuration.variants' => 'sylius.catalog_promotion_scope.for_variants.not_empty']) - ; + function it_adds_violation_if_catalog_promotion_scope_has_empty_variants_configured( + ExecutionContextInterface $executionContext, + ConstraintViolationBuilderInterface $constraintViolationBuilder + ): void { + $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.not_empty')->willReturn($constraintViolationBuilder); + $constraintViolationBuilder->atPath('configuration.variants')->willReturn($constraintViolationBuilder); + $constraintViolationBuilder->addViolation()->shouldBeCalled(); + + $this->validate(['variants' => []], new CatalogPromotionScope(), $executionContext); } - function it_prepares_array_with_violation_if_catalog_promotion_scope_has_not_existing_variants_configured( - ProductVariantRepositoryInterface $variantRepository + function it_adds_violation_if_catalog_promotion_scope_has_not_existing_variants_configured( + ProductVariantRepositoryInterface $variantRepository, + ExecutionContextInterface $executionContext, + ConstraintViolationBuilderInterface $constraintViolationBuilder ): void { $variantRepository->findOneBy(['code' => 'not_existing_variant'])->willReturn(null); - $this - ->validate(['variants' => ['not_existing_variant']], new CatalogPromotionScope()) - ->shouldReturn(['configuration.variants' => 'sylius.catalog_promotion_scope.for_variants.invalid_variants']) - ; + $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.invalid_variants')->willReturn($constraintViolationBuilder); + $constraintViolationBuilder->atPath('configuration.variants')->willReturn($constraintViolationBuilder); + $constraintViolationBuilder->addViolation()->shouldBeCalled(); + + $this->validate(['variants' => ['not_existing_variant']], new CatalogPromotionScope(), $executionContext); } - function it_returns_an_empty_array_if_catalog_promotion_scope_is_valid( + function it_does_nothing_if_catalog_promotion_scope_is_valid( ProductVariantRepositoryInterface $variantRepository, + ExecutionContextInterface $executionContext, ProductVariantInterface $firstVariant, ProductVariantInterface $secondVariant ): void { $variantRepository->findOneBy(['code' => 'first_variant'])->willReturn($firstVariant); $variantRepository->findOneBy(['code' => 'second_variant'])->willReturn($secondVariant); - $this->validate(['variants' => ['first_variant', 'second_variant']], new CatalogPromotionScope())->shouldReturn([]); + $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.not_empty')->shouldNotBeCalled(); + $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.invalid_variants')->shouldNotBeCalled(); + + $this->validate(['variants' => ['first_variant', 'second_variant']], new CatalogPromotionScope(), $executionContext); } } diff --git a/src/Sylius/Bundle/CoreBundle/spec/Validator/Constraints/CatalogPromotionScopeValidatorSpec.php b/src/Sylius/Bundle/CoreBundle/spec/Validator/Constraints/CatalogPromotionScopeValidatorSpec.php index ec5d9bc4583..b93bda0d928 100644 --- a/src/Sylius/Bundle/CoreBundle/spec/Validator/Constraints/CatalogPromotionScopeValidatorSpec.php +++ b/src/Sylius/Bundle/CoreBundle/spec/Validator/Constraints/CatalogPromotionScopeValidatorSpec.php @@ -50,37 +50,29 @@ function it_is_a_constraint_validator(): void function it_adds_violation_if_catalog_promotion_scope_has_invalid_type( ExecutionContextInterface $executionContext, ConstraintViolationBuilderInterface $constraintViolationBuilder, - CatalogPromotionScopeInterface $rule + CatalogPromotionScopeInterface $scope ): void { - $rule->getType()->willReturn('wrong_type'); + $scope->getType()->willReturn('wrong_type'); $executionContext->buildViolation('sylius.catalog_promotion_scope.invalid_type')->willReturn($constraintViolationBuilder); $constraintViolationBuilder->atPath('type')->willReturn($constraintViolationBuilder); $constraintViolationBuilder->addViolation()->shouldBeCalled(); - $this->validate($rule, new CatalogPromotionScope()); + $this->validate($scope, new CatalogPromotionScope()); } - function it_adds_violation_if_scope_validator_returns_one( + function it_calls_a_proper_validator_to_validate_the_configuration( ExecutionContextInterface $executionContext, - ConstraintViolationBuilderInterface $constraintViolationBuilder, - CatalogPromotionScopeInterface $rule, + CatalogPromotionScopeInterface $scope, ScopeValidatorInterface $forVariantsValidator ): void { $constraint = new CatalogPromotionScope(); - $rule->getType()->willReturn(CatalogPromotionScopeInterface::TYPE_FOR_VARIANTS); - $rule->getConfiguration()->willReturn([]); - - $forVariantsValidator - ->validate([], $constraint) - ->willReturn(['configuration.variants' => 'sylius.catalog_promotion_scope.for_variants.not_empty']) - ; + $scope->getType()->willReturn(CatalogPromotionScopeInterface::TYPE_FOR_VARIANTS); + $scope->getConfiguration()->willReturn([]); - $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.not_empty')->willReturn($constraintViolationBuilder); - $constraintViolationBuilder->atPath('configuration.variants')->willReturn($constraintViolationBuilder); - $constraintViolationBuilder->addViolation()->shouldBeCalled(); + $forVariantsValidator->validate([], $constraint, $executionContext)->shouldBeCalled(); - $this->validate($rule, $constraint); + $this->validate($scope, $constraint); } }