fix(db): Deprecate `IExpressionBuilder::or()` and `IExpressionBuilder::and()` without parameters

Signed-off-by: Joas Schilling <coding@schilljs.com>
pull/46605/head
Joas Schilling 2 years ago
parent e45465781f
commit eeb6ddb176
No known key found for this signature in database
GPG Key ID: 74434EFE0D2E2205
  1. 20
      apps/contactsinteraction/lib/Db/CardSearchDao.php
  2. 14
      apps/contactsinteraction/lib/Db/RecentContactMapper.php
  3. 64
      apps/dav/lib/CalDAV/CalDavBackend.php
  4. 12
      apps/dav/lib/CardDAV/CardDavBackend.php
  5. 8
      lib/private/BackgroundJob/JobList.php
  6. 12
      lib/private/DB/Connection.php
  7. 19
      lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php
  8. 12
      lib/private/DB/QueryBuilder/ExpressionBuilder/MySqlExpressionBuilder.php
  9. 15
      lib/private/DB/QueryBuilder/QueryBuilder.php
  10. 10
      lib/private/TaskProcessing/Db/TaskMapper.php
  11. 2
      lib/public/DB/QueryBuilder/IExpressionBuilder.php
  12. 7
      lib/public/DB/QueryBuilder/IQueryBuilder.php
  13. 7
      tests/lib/DB/QueryBuilder/ExpressionBuilderTest.php

@ -29,24 +29,24 @@ class CardSearchDao {
$cardQuery = $this->db->getQueryBuilder();
$propQuery = $this->db->getQueryBuilder();
$propOr = $propQuery->expr()->orX();
$additionalWheres = [];
if ($uid !== null) {
$propOr->add($propQuery->expr()->andX(
$additionalWheres[] = $propQuery->expr()->andX(
$propQuery->expr()->eq('name', $cardQuery->createNamedParameter('UID')),
$propQuery->expr()->eq('value', $cardQuery->createNamedParameter($uid))
));
);
}
if ($email !== null) {
$propOr->add($propQuery->expr()->andX(
$additionalWheres[] = $propQuery->expr()->andX(
$propQuery->expr()->eq('name', $cardQuery->createNamedParameter('EMAIL')),
$propQuery->expr()->eq('value', $cardQuery->createNamedParameter($email))
));
);
}
if ($cloudId !== null) {
$propOr->add($propQuery->expr()->andX(
$additionalWheres[] = $propQuery->expr()->andX(
$propQuery->expr()->eq('name', $cardQuery->createNamedParameter('CLOUD')),
$propQuery->expr()->eq('value', $cardQuery->createNamedParameter($cloudId))
));
);
}
$addressbooksQuery->selectDistinct('id')
->from('addressbooks')
@ -54,8 +54,12 @@ class CardSearchDao {
$propQuery->selectDistinct('cardid')
->from('cards_properties')
->where($propQuery->expr()->in('addressbookid', $propQuery->createFunction($addressbooksQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY))
->andWhere($propOr)
->groupBy('cardid');
if (!empty($additionalWheres)) {
$propQuery->andWhere($propQuery->expr()->orX(...$additionalWheres));
}
$cardQuery->select('carddata')
->from('cards')
->where($cardQuery->expr()->in('id', $cardQuery->createFunction($propQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY))

@ -61,23 +61,25 @@ class RecentContactMapper extends QBMapper {
?string $cloudId): array {
$qb = $this->db->getQueryBuilder();
$or = $qb->expr()->orX();
$additionalWheres = [];
if ($uid !== null) {
$or->add($qb->expr()->eq('uid', $qb->createNamedParameter($uid)));
$additionalWheres[] = $qb->expr()->eq('uid', $qb->createNamedParameter($uid));
}
if ($email !== null) {
$or->add($qb->expr()->eq('email', $qb->createNamedParameter($email)));
$additionalWheres[] = $qb->expr()->eq('email', $qb->createNamedParameter($email));
}
if ($cloudId !== null) {
$or->add($qb->expr()->eq('federated_cloud_id', $qb->createNamedParameter($cloudId)));
$additionalWheres[] = $qb->expr()->eq('federated_cloud_id', $qb->createNamedParameter($cloudId));
}
$select = $qb
->select('*')
->from($this->getTableName())
->where($or)
->andWhere($qb->expr()->eq('actor_uid', $qb->createNamedParameter($user->getUID())));
->where($qb->expr()->eq('actor_uid', $qb->createNamedParameter($user->getUID())));
if (!empty($additionalWheres)) {
$select->andWhere($select->expr()->orX(...$additionalWheres));
}
return $this->findEntities($select);
}

@ -1875,12 +1875,12 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
}
if (!empty($searchProperties)) {
$or = $innerQuery->expr()->orX();
$or = [];
foreach ($searchProperties as $searchProperty) {
$or->add($innerQuery->expr()->eq('op.name',
$outerQuery->createNamedParameter($searchProperty)));
$or[] = $innerQuery->expr()->eq('op.name',
$outerQuery->createNamedParameter($searchProperty));
}
$innerQuery->andWhere($or);
$innerQuery->andWhere($innerQuery->expr()->orX(...$or));
}
if ($pattern !== '') {
@ -1924,12 +1924,12 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
}
if (!empty($options['types'])) {
$or = $outerQuery->expr()->orX();
$or = [];
foreach ($options['types'] as $type) {
$or->add($outerQuery->expr()->eq('componenttype',
$outerQuery->createNamedParameter($type)));
$or[] = $outerQuery->expr()->eq('componenttype',
$outerQuery->createNamedParameter($type));
}
$outerQuery->andWhere($or);
$outerQuery->andWhere($outerQuery->expr()->orX(...$or));
}
$outerQuery->andWhere($outerQuery->expr()->in('c.id', $outerQuery->createFunction($innerQuery->getSQL())));
@ -2150,16 +2150,17 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
$escapePattern = !\array_key_exists('escape_like_param', $options) || $options['escape_like_param'] !== false;
$calendarObjectIdQuery = $this->db->getQueryBuilder();
$calendarOr = $calendarObjectIdQuery->expr()->orX();
$searchOr = $calendarObjectIdQuery->expr()->orX();
$calendarOr = [];
$searchOr = [];
// Fetch calendars and subscription
$calendars = $this->getCalendarsForUser($principalUri);
$subscriptions = $this->getSubscriptionsForUser($principalUri);
foreach ($calendars as $calendar) {
$calendarAnd = $calendarObjectIdQuery->expr()->andX();
$calendarAnd->add($calendarObjectIdQuery->expr()->eq('cob.calendarid', $calendarObjectIdQuery->createNamedParameter((int)$calendar['id'])));
$calendarAnd->add($calendarObjectIdQuery->expr()->eq('cob.calendartype', $calendarObjectIdQuery->createNamedParameter(self::CALENDAR_TYPE_CALENDAR)));
$calendarAnd = $calendarObjectIdQuery->expr()->andX(
$calendarObjectIdQuery->expr()->eq('cob.calendarid', $calendarObjectIdQuery->createNamedParameter((int)$calendar['id'])),
$calendarObjectIdQuery->expr()->eq('cob.calendartype', $calendarObjectIdQuery->createNamedParameter(self::CALENDAR_TYPE_CALENDAR)),
);
// If it's shared, limit search to public events
if (isset($calendar['{http://owncloud.org/ns}owner-principal'])
@ -2167,12 +2168,13 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
$calendarAnd->add($calendarObjectIdQuery->expr()->eq('co.classification', $calendarObjectIdQuery->createNamedParameter(self::CLASSIFICATION_PUBLIC)));
}
$calendarOr->add($calendarAnd);
$calendarOr[] = $calendarAnd;
}
foreach ($subscriptions as $subscription) {
$subscriptionAnd = $calendarObjectIdQuery->expr()->andX();
$subscriptionAnd->add($calendarObjectIdQuery->expr()->eq('cob.calendarid', $calendarObjectIdQuery->createNamedParameter((int)$subscription['id'])));
$subscriptionAnd->add($calendarObjectIdQuery->expr()->eq('cob.calendartype', $calendarObjectIdQuery->createNamedParameter(self::CALENDAR_TYPE_SUBSCRIPTION)));
$subscriptionAnd = $calendarObjectIdQuery->expr()->andX(
$calendarObjectIdQuery->expr()->eq('cob.calendarid', $calendarObjectIdQuery->createNamedParameter((int)$subscription['id'])),
$calendarObjectIdQuery->expr()->eq('cob.calendartype', $calendarObjectIdQuery->createNamedParameter(self::CALENDAR_TYPE_SUBSCRIPTION)),
);
// If it's shared, limit search to public events
if (isset($subscription['{http://owncloud.org/ns}owner-principal'])
@ -2180,28 +2182,30 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
$subscriptionAnd->add($calendarObjectIdQuery->expr()->eq('co.classification', $calendarObjectIdQuery->createNamedParameter(self::CLASSIFICATION_PUBLIC)));
}
$calendarOr->add($subscriptionAnd);
$calendarOr[] = $subscriptionAnd;
}
foreach ($searchProperties as $property) {
$propertyAnd = $calendarObjectIdQuery->expr()->andX();
$propertyAnd->add($calendarObjectIdQuery->expr()->eq('cob.name', $calendarObjectIdQuery->createNamedParameter($property, IQueryBuilder::PARAM_STR)));
$propertyAnd->add($calendarObjectIdQuery->expr()->isNull('cob.parameter'));
$propertyAnd = $calendarObjectIdQuery->expr()->andX(
$calendarObjectIdQuery->expr()->eq('cob.name', $calendarObjectIdQuery->createNamedParameter($property, IQueryBuilder::PARAM_STR)),
$calendarObjectIdQuery->expr()->isNull('cob.parameter'),
);
$searchOr->add($propertyAnd);
$searchOr[] = $propertyAnd;
}
foreach ($searchParameters as $property => $parameter) {
$parameterAnd = $calendarObjectIdQuery->expr()->andX();
$parameterAnd->add($calendarObjectIdQuery->expr()->eq('cob.name', $calendarObjectIdQuery->createNamedParameter($property, IQueryBuilder::PARAM_STR)));
$parameterAnd->add($calendarObjectIdQuery->expr()->eq('cob.parameter', $calendarObjectIdQuery->createNamedParameter($parameter, IQueryBuilder::PARAM_STR_ARRAY)));
$parameterAnd = $calendarObjectIdQuery->expr()->andX(
$calendarObjectIdQuery->expr()->eq('cob.name', $calendarObjectIdQuery->createNamedParameter($property, IQueryBuilder::PARAM_STR)),
$calendarObjectIdQuery->expr()->eq('cob.parameter', $calendarObjectIdQuery->createNamedParameter($parameter, IQueryBuilder::PARAM_STR_ARRAY)),
);
$searchOr->add($parameterAnd);
$searchOr[] = $parameterAnd;
}
if ($calendarOr->count() === 0) {
if (empty($calendarOr)) {
return [];
}
if ($searchOr->count() === 0) {
if (empty($searchOr)) {
return [];
}
@ -2209,8 +2213,8 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
->from($this->dbObjectPropertiesTable, 'cob')
->leftJoin('cob', 'calendarobjects', 'co', $calendarObjectIdQuery->expr()->eq('co.id', 'cob.objectid'))
->andWhere($calendarObjectIdQuery->expr()->in('co.componenttype', $calendarObjectIdQuery->createNamedParameter($componentTypes, IQueryBuilder::PARAM_STR_ARRAY)))
->andWhere($calendarOr)
->andWhere($searchOr)
->andWhere($calendarObjectIdQuery->expr()->orX(...$calendarOr))
->andWhere($calendarObjectIdQuery->expr()->orX(...$searchOr))
->andWhere($calendarObjectIdQuery->expr()->isNull('deleted_at'));
if ($pattern !== '') {

@ -1148,20 +1148,20 @@ class CardDavBackend implements BackendInterface, SyncSupport {
/**
* FIXME Find a way to match only 4 last digits
* BDAY can be --1018 without year or 20001019 with it
* $bDayOr = $query2->expr()->orX();
* $bDayOr = [];
* if ($options['since'] instanceof DateTimeFilter) {
* $bDayOr->add(
* $bDayOr[] =
* $query2->expr()->gte('SUBSTR(cp_bday.value, -4)',
* $query2->createNamedParameter($options['since']->get()->format('md')))
* $query2->createNamedParameter($options['since']->get()->format('md'))
* );
* }
* if ($options['until'] instanceof DateTimeFilter) {
* $bDayOr->add(
* $bDayOr[] =
* $query2->expr()->lte('SUBSTR(cp_bday.value, -4)',
* $query2->createNamedParameter($options['until']->get()->format('md')))
* $query2->createNamedParameter($options['until']->get()->format('md'))
* );
* }
* $query2->andWhere($bDayOr);
* $query2->andWhere($query2->expr()->orX(...$bDayOr));
*/
}

@ -203,12 +203,12 @@ class JobList implements IJobList {
$query->andWhere($query->expr()->eq('time_sensitive', $query->createNamedParameter(IJob::TIME_SENSITIVE, IQueryBuilder::PARAM_INT)));
}
if ($jobClasses !== null && count($jobClasses) > 0) {
$orClasses = $query->expr()->orx();
if (!empty($jobClasses)) {
$orClasses = [];
foreach ($jobClasses as $jobClass) {
$orClasses->add($query->expr()->eq('class', $query->createNamedParameter($jobClass, IQueryBuilder::PARAM_STR)));
$orClasses[] = $query->expr()->eq('class', $query->createNamedParameter($jobClass, IQueryBuilder::PARAM_STR));
}
$query->andWhere($orClasses);
$query->andWhere($query->expr()->orX(...$orClasses));
}
$result = $query->executeQuery();

@ -471,22 +471,22 @@ class Connection extends PrimaryReadReplicaConnection {
foreach ($values as $name => $value) {
$updateQb->set($name, $updateQb->createNamedParameter($value, $this->getType($value)));
}
$where = $updateQb->expr()->andX();
$where = [];
$whereValues = array_merge($keys, $updatePreconditionValues);
foreach ($whereValues as $name => $value) {
if ($value === '') {
$where->add($updateQb->expr()->emptyString(
$where[] = $updateQb->expr()->emptyString(
$name
));
);
} else {
$where->add($updateQb->expr()->eq(
$where[] = $updateQb->expr()->eq(
$name,
$updateQb->createNamedParameter($value, $this->getType($value)),
$this->getType($value)
));
);
}
}
$updateQb->where($where);
$updateQb->where($updateQb->expr()->andX(...$where));
$affected = $updateQb->executeStatement();
if ($affected === 0 && !empty($updatePreconditionValues)) {

@ -21,6 +21,7 @@ use OCP\DB\QueryBuilder\IParameter;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\DB\QueryBuilder\IQueryFunction;
use OCP\IDBConnection;
use Psr\Log\LoggerInterface;
class ExpressionBuilder implements IExpressionBuilder {
/** @var \Doctrine\DBAL\Query\Expression\ExpressionBuilder */
@ -32,17 +33,15 @@ class ExpressionBuilder implements IExpressionBuilder {
/** @var IDBConnection */
protected $connection;
/** @var LoggerInterface */
protected $logger;
/** @var FunctionBuilder */
protected $functionBuilder;
/**
* Initializes a new <tt>ExpressionBuilder</tt>.
*
* @param ConnectionAdapter $connection
* @param IQueryBuilder $queryBuilder
*/
public function __construct(ConnectionAdapter $connection, IQueryBuilder $queryBuilder) {
public function __construct(ConnectionAdapter $connection, IQueryBuilder $queryBuilder, LoggerInterface $logger) {
$this->connection = $connection;
$this->logger = $logger;
$this->helper = new QuoteHelper();
$this->expressionBuilder = new DoctrineExpressionBuilder($connection->getInner());
$this->functionBuilder = $queryBuilder->func();
@ -63,6 +62,9 @@ class ExpressionBuilder implements IExpressionBuilder {
* @return \OCP\DB\QueryBuilder\ICompositeExpression
*/
public function andX(...$x): ICompositeExpression {
if (empty($x)) {
$this->logger->debug('Calling ' . IQueryBuilder::class . '::' . __FUNCTION__ . ' without parameters is deprecated and will throw soon.', ['exception' => new \Exception('No parameters in call to ' . __METHOD__)]);
}
return new CompositeExpression(CompositeExpression::TYPE_AND, $x);
}
@ -81,6 +83,9 @@ class ExpressionBuilder implements IExpressionBuilder {
* @return \OCP\DB\QueryBuilder\ICompositeExpression
*/
public function orX(...$x): ICompositeExpression {
if (empty($x)) {
$this->logger->debug('Calling ' . IQueryBuilder::class . '::' . __FUNCTION__ . ' without parameters is deprecated and will throw soon.', ['exception' => new \Exception('No parameters in call to ' . __METHOD__)]);
}
return new CompositeExpression(CompositeExpression::TYPE_OR, $x);
}

@ -11,17 +11,13 @@ use OC\DB\ConnectionAdapter;
use OC\DB\QueryBuilder\QueryFunction;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\DB\QueryBuilder\IQueryFunction;
use Psr\Log\LoggerInterface;
class MySqlExpressionBuilder extends ExpressionBuilder {
/** @var string */
protected $collation;
protected string $collation;
/**
* @param ConnectionAdapter $connection
* @param IQueryBuilder $queryBuilder
*/
public function __construct(ConnectionAdapter $connection, IQueryBuilder $queryBuilder) {
parent::__construct($connection, $queryBuilder);
public function __construct(ConnectionAdapter $connection, IQueryBuilder $queryBuilder, LoggerInterface $logger) {
parent::__construct($connection, $queryBuilder, $logger);
$params = $connection->getInner()->getParams();
$this->collation = $params['collation'] ?? (($params['charset'] ?? 'utf8') . '_general_ci');

@ -92,10 +92,10 @@ class QueryBuilder implements IQueryBuilder {
*/
public function expr() {
return match($this->connection->getDatabaseProvider()) {
IDBConnection::PLATFORM_ORACLE => new OCIExpressionBuilder($this->connection, $this),
IDBConnection::PLATFORM_POSTGRES => new PgSqlExpressionBuilder($this->connection, $this),
IDBConnection::PLATFORM_MYSQL => new MySqlExpressionBuilder($this->connection, $this),
IDBConnection::PLATFORM_SQLITE => new SqliteExpressionBuilder($this->connection, $this),
IDBConnection::PLATFORM_ORACLE => new OCIExpressionBuilder($this->connection, $this, $this->logger),
IDBConnection::PLATFORM_POSTGRES => new PgSqlExpressionBuilder($this->connection, $this, $this->logger),
IDBConnection::PLATFORM_MYSQL => new MySqlExpressionBuilder($this->connection, $this, $this->logger),
IDBConnection::PLATFORM_SQLITE => new SqliteExpressionBuilder($this->connection, $this, $this->logger),
};
}
@ -815,9 +815,10 @@ class QueryBuilder implements IQueryBuilder {
* // You can optionally programmatically build and/or expressions
* $qb = $conn->getQueryBuilder();
*
* $or = $qb->expr()->orx();
* $or->add($qb->expr()->eq('u.id', 1));
* $or->add($qb->expr()->eq('u.id', 2));
* $or = $qb->expr()->orx(
* $qb->expr()->eq('u.id', 1),
* $qb->expr()->eq('u.id', 2),
* );
*
* $qb->update('users', 'u')
* ->set('u.password', md5('password'))

@ -59,16 +59,16 @@ class TaskMapper extends QBMapper {
->setMaxResults(1)
->orderBy('last_updated', 'ASC');
if (count($taskTypes) > 0) {
$filter = $qb->expr()->orX();
if (!empty($taskTypes)) {
$filter = [];
foreach ($taskTypes as $taskType) {
$filter->add($qb->expr()->eq('type', $qb->createPositionalParameter($taskType)));
$filter[] = $qb->expr()->eq('type', $qb->createPositionalParameter($taskType));
}
$qb->andWhere($filter);
$qb->andWhere($qb->expr()->orX(...$filter));
}
if (count($taskIdsToIgnore) > 0) {
if (!empty($taskIdsToIgnore)) {
$qb->andWhere($qb->expr()->notIn('id', $qb->createNamedParameter($taskIdsToIgnore, IQueryBuilder::PARAM_INT_ARRAY)));
}

@ -55,6 +55,7 @@ interface IExpressionBuilder {
*
* @return \OCP\DB\QueryBuilder\ICompositeExpression
* @since 8.2.0
* @since 30.0.0 Calling the method without any arguments is deprecated and will throw with the next Doctrine/DBAL update
*
* @psalm-taint-sink sql $x
*/
@ -74,6 +75,7 @@ interface IExpressionBuilder {
*
* @return \OCP\DB\QueryBuilder\ICompositeExpression
* @since 8.2.0
* @since 30.0.0 Calling the method without any arguments is deprecated and will throw with the next Doctrine/DBAL update
*
* @psalm-taint-sink sql $x
*/

@ -614,9 +614,10 @@ interface IQueryBuilder {
* // You can optionally programmatically build and/or expressions
* $qb = $conn->getQueryBuilder();
*
* $or = $qb->expr()->orx();
* $or->add($qb->expr()->eq('u.id', 1));
* $or->add($qb->expr()->eq('u.id', 2));
* $or = $qb->expr()->orx(
* $qb->expr()->eq('u.id', 1),
* $qb->expr()->eq('u.id', 2),
* );
*
* $qb->update('users', 'u')
* ->set('u.password', md5('password'))

@ -10,6 +10,7 @@ namespace Test\DB\QueryBuilder;
use Doctrine\DBAL\Query\Expression\ExpressionBuilder as DoctrineExpressionBuilder;
use OC\DB\QueryBuilder\ExpressionBuilder\ExpressionBuilder;
use OCP\DB\QueryBuilder\IQueryBuilder;
use Psr\Log\LoggerInterface;
use Test\TestCase;
/**
@ -32,15 +33,19 @@ class ExpressionBuilderTest extends TestCase {
/** @var \Doctrine\DBAL\Connection */
protected $internalConnection;
/** @var LoggerInterface */
protected $logger;
protected function setUp(): void {
parent::setUp();
$this->connection = \OC::$server->getDatabaseConnection();
$this->internalConnection = \OC::$server->get(\OC\DB\Connection::class);
$this->logger = $this->createMock(LoggerInterface::class);
$queryBuilder = $this->createMock(IQueryBuilder::class);
$this->expressionBuilder = new ExpressionBuilder($this->connection, $queryBuilder);
$this->expressionBuilder = new ExpressionBuilder($this->connection, $queryBuilder, $this->logger);
$this->doctrineExpressionBuilder = new DoctrineExpressionBuilder($this->internalConnection);
}

Loading…
Cancel
Save