Merge pull request mautic#2206 from acquia/MAUT-11541

Adding regex operator validation
escopecz · Jun 28, 2024 · f47d4d6 · f47d4d6
1 parent 83d84d0
commit f47d4d6
Show file tree

Hide file tree

Showing 6 changed files with 140 additions and 9 deletions.
diff --git a/app/bundles/LeadBundle/Config/config.php b/app/bundles/LeadBundle/Config/config.php
@@ -403,6 +403,11 @@
                 'tag'       => 'validator.constraint_validator',
                 'alias'     => 'uniqueleadlist',
             ],
+            'mautic.lead.constraint.dbregex' => [
+                'class'     => \Mautic\LeadBundle\Form\Validator\Constraints\DbRegexValidator::class,
+                'arguments' => ['doctrine.dbal.default_connection'],
+                'tag'       => 'validator.constraint_validator',
+            ],
             'mautic.lead.validator.custom_field' => [
                 'class'     => Mautic\LeadBundle\Validator\CustomFieldValidator::class,
                 'arguments' => ['mautic.lead.model.field', 'translator'],

diff --git a/app/bundles/LeadBundle/EventListener/TypeOperatorSubscriber.php b/app/bundles/LeadBundle/EventListener/TypeOperatorSubscriber.php
@@ -15,6 +15,7 @@
 use Mautic\LeadBundle\Event\FormAdjustmentEvent;
 use Mautic\LeadBundle\Event\ListFieldChoicesEvent;
 use Mautic\LeadBundle\Event\TypeOperatorsEvent;
+use Mautic\LeadBundle\Form\Validator\Constraints\DbRegex;
 use Mautic\LeadBundle\Helper\FormFieldHelper;
 use Mautic\LeadBundle\LeadEvents;
 use Mautic\LeadBundle\Model\LeadModel;
@@ -277,16 +278,22 @@ public function onSegmentFilterFormHandleSelect(FormAdjustmentEvent $event): voi
 
     public function onSegmentFilterFormHandleDefault(FormAdjustmentEvent $event): void
     {
-        $form = $event->getForm();
+        $form        = $event->getForm();
+        $constraints = [];
+
+        if (in_array($event->getOperator(), [OperatorOptions::REGEXP, OperatorOptions::NOT_REGEXP], true)) {
+            $constraints[] = new DbRegex();
+        }
 
         $form->add(
             'filter',
             TextType::class,
             [
-                'label'    => false,
-                'attr'     => ['class' => 'form-control'],
-                'disabled' => $event->filterShouldBeDisabled(),
-                'data'     => $form->getData()['filter'] ?? '',
+                'label'       => false,
+                'attr'        => ['class' => 'form-control'],
+                'disabled'    => $event->filterShouldBeDisabled(),
+                'data'        => $form->getData()['filter'] ?? '',
+                'constraints' => $constraints,
             ]
         );
         $this->showOperatorsBasedAlertMessages($event);
@@ -297,6 +304,7 @@ private function showOperatorsBasedAlertMessages(FormAdjustmentEvent $event): vo
     {
         switch ($event->getOperator()) {
             case OperatorOptions::REGEXP:
+            case OperatorOptions::NOT_REGEXP:
                 $alertText = $this->translator->trans('mautic.lead_list.filter.alert.regexp');
                 break;
             case OperatorOptions::ENDS_WITH:
@@ -306,6 +314,7 @@ private function showOperatorsBasedAlertMessages(FormAdjustmentEvent $event): vo
                 $alertText = $this->translator->trans('mautic.lead_list.filter.alert.contain');
                 break;
             case OperatorOptions::LIKE:
+            case OperatorOptions::NOT_LIKE:
                 $alertText = $this->translator->trans('mautic.lead_list.filter.alert.like');
                 break;
             default:

diff --git a/app/bundles/LeadBundle/Form/Validator/Constraints/DbRegex.php b/app/bundles/LeadBundle/Form/Validator/Constraints/DbRegex.php
@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Mautic\LeadBundle\Form\Validator\Constraints;
+
+use Symfony\Component\Validator\Constraint;
+
+final class DbRegex extends Constraint
+{
+}
diff --git a/app/bundles/LeadBundle/Form/Validator/Constraints/DbRegexValidator.php b/app/bundles/LeadBundle/Form/Validator/Constraints/DbRegexValidator.php
@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Mautic\LeadBundle\Form\Validator\Constraints;
+
+use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Exception;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
+
+final class DbRegexValidator extends ConstraintValidator
+{
+    public function __construct(private Connection $connection)
+    {
+    }
+
+    public function validate($regex, Constraint $constraint): void
+    {
+        if (!$constraint instanceof DbRegex) {
+            throw new UnexpectedTypeException($constraint, DbRegex::class);
+        }
+
+        try {
+            $this->connection->executeQuery('SELECT "" REGEXP ? AS is_valid', [$regex]);
+        } catch (Exception $e) {
+            $this->context->buildViolation(
+                $this->stripUglyPartOfTheErrorMessage($e->getPrevious()->getMessage())
+            )->addViolation();
+        }
+    }
+
+    private function stripUglyPartOfTheErrorMessage(string $message): string
+    {
+        return preg_replace('/^SQLSTATE\[HY000\]: General error: \d+ /', '', $message);
+    }
+}
diff --git a/app/bundles/LeadBundle/Tests/Controller/Api/ListApiControllerFunctionalTest.php b/app/bundles/LeadBundle/Tests/Controller/Api/ListApiControllerFunctionalTest.php
@@ -7,6 +7,7 @@
 use Mautic\LeadBundle\Entity\LeadList;
 use Mautic\LeadBundle\Model\ListModel;
 use PHPUnit\Framework\Assert;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
@@ -27,6 +28,72 @@ protected function setUp(): void
         $this->translator = static::getContainer()->get('translator');
     }
 
+    protected function beforeBeginTransaction(): void
+    {
+        $this->resetAutoincrement(['categories']);
+    }
+
+    /**
+     * @return iterable<array<string|int|null>>
+     */
+    public function regexOperatorProvider(): iterable
+    {
+        yield [
+            'regexp',
+            '^{Test|Test string)', // invalid regex: the first parantheses should not be curly
+            Response::HTTP_BAD_REQUEST,
+            'filter: Incorrect description of a {min,max} interval.',
+        ];
+
+        yield [
+            '!regexp',
+            '^(Test|Test string))', // invalid regex: 2 ending parantheses
+            Response::HTTP_BAD_REQUEST,
+            'filter: Mismatched parenthesis in regular expression.',
+        ];
+
+        yield [
+            'regexp',
+            '^(Test|Test string)', // valid regex
+            Response::HTTP_CREATED,
+            null,
+        ];
+    }
+
+    /**
+     * @dataProvider regexOperatorProvider
+     */
+    public function testRegexOperatorValidation(string $operator, string $regex, int $expectedResponseCode, ?string $expectedErrorMessage): void
+    {
+        $this->client->request(
+            Request::METHOD_POST,
+            '/api/segments/new',
+            [
+                'name'    => 'Regex test',
+                'filters' => [
+                    [
+                        'glue'       => 'and',
+                        'field'      => 'city',
+                        'object'     => 'lead',
+                        'type'       => 'text',
+                        'operator'   => $operator,
+                        'properties' => ['filter' => $regex],
+                    ],
+                ],
+            ]
+        );
+
+        Assert::assertSame($expectedResponseCode, $this->client->getResponse()->getStatusCode());
+
+        if ($expectedErrorMessage) {
+            Assert::assertSame(
+                $expectedErrorMessage,
+                json_decode($this->client->getResponse()->getContent(), true)['errors'][0]['message'],
+                $this->client->getResponse()->getContent()
+            );
+        }
+    }
+
     public function testSingleSegmentWorkflow(): void
     {
         $payload = [

diff --git a/app/bundles/LeadBundle/Tests/EventListener/TypeOperatorSubscriberTest.php b/app/bundles/LeadBundle/Tests/EventListener/TypeOperatorSubscriberTest.php
@@ -508,10 +508,11 @@ public function testOnSegmentFilterFormHandleDefault(): void
                 'filter',
                 TextType::class,
                 [
-                    'label'    => false,
-                    'attr'     => ['class' => 'form-control'],
-                    'disabled' => false,
-                    'data'     => '',
+                    'label'       => false,
+                    'attr'        => ['class' => 'form-control'],
+                    'disabled'    => false,
+                    'data'        => '',
+                    'constraints' => [],
                 ]
             );