Avoid simple text replacements in SQL if possible

MAILPOET-4986
This commit is contained in:
John Oleksowicz
2023-03-09 12:25:13 -06:00
committed by Aschepikov
parent 1cab4a944a
commit 03da98e5df
4 changed files with 83 additions and 9 deletions

View File

@@ -3,6 +3,7 @@
namespace MailPoet\Segments\DynamicSegments\Filters;
use MailPoet\Entities\SubscriberEntity;
use MailPoet\Util\Security;
use MailPoetVendor\Doctrine\DBAL\Query\QueryBuilder;
use MailPoetVendor\Doctrine\ORM\EntityManager;
@@ -34,4 +35,27 @@ class FilterHelper {
->getClassMetadata(SubscriberEntity::class)
->getTableName();
}
public function getInterpolatedSQL(QueryBuilder $query): string {
$sql = $query->getSQL();
$params = $query->getParameters();
$search = array_map(function($key) {
return ":$key";
}, array_keys($params));
$replace = array_map(function($value) use ($query) {
if (is_array($value)) {
$quotedValues = array_map(function($arrayValue) use ($query) {
return $query->expr()->literal($arrayValue);
}, $value);
return implode(',', $quotedValues);
}
return $query->expr()->literal($value);
}, array_values($params));
return str_replace($search, $replace, $sql);
}
public function getUniqueParameterName(string $parameter): string {
$suffix = Security::generateRandomString();
return sprintf("%s_%s", $parameter, $suffix);
}
}

View File

@@ -37,7 +37,7 @@ class WooCommercePurchaseDate implements Filter {
if (in_array($operator, [DateFilterHelper::NOT_ON, DateFilterHelper::NOT_IN_THE_LAST])) {
$subQuery = $this->filterHelper->getNewSubscribersQueryBuilder();
$this->applyConditionsToQueryBuilder($operator, $date, $subQuery);
$queryBuilder->andWhere($queryBuilder->expr()->notIn("{$subscribersTable}.id", $subQuery->getSQL()));
$queryBuilder->andWhere($queryBuilder->expr()->notIn("{$subscribersTable}.id", $this->filterHelper->getInterpolatedSQL($subQuery)));
} else {
$this->applyConditionsToQueryBuilder($operator, $date, $queryBuilder);
}
@@ -47,27 +47,29 @@ class WooCommercePurchaseDate implements Filter {
private function applyConditionsToQueryBuilder(string $operator, string $date, QueryBuilder $queryBuilder): QueryBuilder {
$orderStatsAlias = $this->wooFilterHelper->applyOrderStatusFilter($queryBuilder);
$quotedDate = $queryBuilder->expr()->literal($date);
$dateParam = $this->filterHelper->getUniqueParameterName('date');
switch ($operator) {
case DateFilterHelper::BEFORE:
$queryBuilder->andWhere("DATE($orderStatsAlias.date_created) < $quotedDate");
$queryBuilder->andWhere("DATE($orderStatsAlias.date_created) < :$dateParam");
break;
case DateFilterHelper::AFTER:
$queryBuilder->andWhere("DATE($orderStatsAlias.date_created) > $quotedDate");
$queryBuilder->andWhere("DATE($orderStatsAlias.date_created) > :$dateParam");
break;
case DateFilterHelper::IN_THE_LAST:
case DateFilterHelper::NOT_IN_THE_LAST:
$queryBuilder->andWhere("DATE($orderStatsAlias.date_created) >= $quotedDate");
$queryBuilder->andWhere("DATE($orderStatsAlias.date_created) >= :$dateParam");
break;
case DateFilterHelper::ON:
case DateFilterHelper::NOT_ON:
$queryBuilder->andWhere("DATE($orderStatsAlias.date_created) = $quotedDate");
$queryBuilder->andWhere("DATE($orderStatsAlias.date_created) = :$dateParam");
break;
default:
throw new InvalidFilterException('Incorrect value for operator', InvalidFilterException::MISSING_VALUE);
}
$queryBuilder->setParameter($dateParam, $date);
return $queryBuilder;
}
}

View File

@@ -3,6 +3,7 @@
namespace MailPoet\Segments\DynamicSegments\Filters;
use MailPoet\Util\DBCollationChecker;
use MailPoetVendor\Doctrine\DBAL\Connection;
use MailPoetVendor\Doctrine\DBAL\Query\QueryBuilder;
class WooFilterHelper {
@@ -76,10 +77,10 @@ class WooFilterHelper {
$allowedStatuses = $this->defaultIncludedStatuses();
}
$statusParam = $this->filterHelper->getUniqueParameterName('status');
$orderStatsAlias = $this->applyCustomerOrderJoin($queryBuilder);
$quotedStatus = array_map([$queryBuilder->expr(), 'literal'], $allowedStatuses);
$queryBuilder->andWhere($queryBuilder->expr()->in("$orderStatsAlias.status", $quotedStatus));
$queryBuilder->andWhere("$orderStatsAlias.status IN (:$statusParam)");
$queryBuilder->setParameter($statusParam, $allowedStatuses, Connection::PARAM_STR_ARRAY);
return $orderStatsAlias;
}

View File

@@ -0,0 +1,47 @@
<?php declare(strict_types = 1);
namespace MailPoet\Segments\DynamicSegments\Filters;
use MailPoet\Entities\SubscriberEntity;
use MailPoetVendor\Doctrine\DBAL\Query\QueryBuilder;
class FilterHelperTest extends \MailPoetTest {
/** @var FilterHelper */
private $filterHelper;
/** @var string */
private $subscribersTable;
public function _before() {
parent::_before();
$this->filterHelper = $this->diContainer->get(FilterHelper::class);
$this->subscribersTable = $this->entityManager
->getClassMetadata(SubscriberEntity::class)
->getTableName();
}
public function testItCanReturnSQLThatDoesNotIncludeParams(): void {
$queryBuilder = $this->getSubscribersQueryBuilder();
$defaultResult = $queryBuilder->getSQL();
expect($defaultResult)->equals("SELECT id FROM $this->subscribersTable");
expect($this->filterHelper->getInterpolatedSQL($queryBuilder))->equals($defaultResult);
}
public function testItCanReturnInterpolatedSQL(): void {
$queryBuilder = $this->getSubscribersQueryBuilder();
$queryBuilder->where("$this->subscribersTable.created_at < :date");
$queryBuilder->setParameter('date', '2023-03-09');
expect($this->filterHelper->getInterpolatedSQL($queryBuilder))->equals("SELECT id FROM $this->subscribersTable WHERE $this->subscribersTable.created_at < '2023-03-09'");
}
public function testItProperlyInterpolatesArrayValues(): void {
$queryBuilder = $this->getSubscribersQueryBuilder();
$queryBuilder->where("$this->subscribersTable.status IN (:statuses)");
$queryBuilder->setParameter('statuses', ['subscribed', 'inactive']);
expect($this->filterHelper->getInterpolatedSQL($queryBuilder))->equals("SELECT id FROM $this->subscribersTable WHERE $this->subscribersTable.status IN ('subscribed','inactive')");
}
private function getSubscribersQueryBuilder(): QueryBuilder {
return $this->entityManager->getConnection()->createQueryBuilder()->select('id')->from($this->subscribersTable);
}
}