Skip to content

Commit

Permalink
FIX Use OR conjuctive in filterAny aggregate queries
Browse files Browse the repository at this point in the history
  • Loading branch information
emteknetnz committed May 16, 2023
1 parent 7210ac8 commit 696fe79
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 20 deletions.
20 changes: 17 additions & 3 deletions src/ORM/DataList.php
Expand Up @@ -12,6 +12,7 @@
use LogicException;
use BadMethodCallException;
use Traversable;
use SilverStripe\ORM\DataQuery;

/**
* Implements a "lazy loading" DataObjectSet.
Expand Down Expand Up @@ -525,14 +526,27 @@ public function filterAny()
throw new InvalidArgumentException('Incorrect number of arguments passed to filterAny()');
}

return $this->alterDataQuery(function (DataQuery $query) use ($whereArguments) {
$subquery = $query->disjunctiveGroup();

$list = $this->alterDataQuery(function (DataQuery $query) use ($whereArguments) {
$subquery = $this->getFilterAnySubquery($query, $whereArguments);
foreach ($whereArguments as $field => $value) {
$filter = $this->createSearchFilter($field, $value);
$filter->apply($subquery);
}
});

return $list;
}

private function getFilterAnySubquery(DataQuery $query, array $whereArguments): DataQuery_SubGroup
{
$clause = 'WHERE';
foreach (array_keys($whereArguments) as $field) {
if (preg_match('#\.(COUNT|SUM|AVG|MIN|MAX)\(#', strtoupper($field))) {
$clause = 'HAVING';
break;
}
}
return $query->disjunctiveGroup($clause);
}

/**
Expand Down
22 changes: 20 additions & 2 deletions src/ORM/DataQuery.php
Expand Up @@ -658,23 +658,41 @@ public function having($having)
*
* That is a subgroup joined by OR
*
* @param string $clause
* @return DataQuery_SubGroup
*/
public function disjunctiveGroup()
{
return new DataQuery_SubGroup($this, 'OR');
// using func_get_args to add a new param while retaining BC
// @deprecated - add a new param for CMS 6 - string $clause = 'WHERE'
$clause = 'WHERE';
$args = func_get_args();
if (count($args) > 0) {
$clause = $args[0];
}
return new DataQuery_SubGroup($this, 'OR', $clause);
}



/**
* Create a conjunctive subgroup
*
* That is a subgroup joined by AND
*
* @param string $clause
* @return DataQuery_SubGroup
*/
public function conjunctiveGroup()
{
return new DataQuery_SubGroup($this, 'AND');
// using func_get_args to add a new param while retaining BC
// @deprecated - add a new param for CMS 6 - string $clause = 'WHERE'
$clause = 'WHERE';
$args = func_get_args();
if (count($args) > 0) {
$clause = $args[0];
}
return new DataQuery_SubGroup($this, 'AND', $clause);
}

/**
Expand Down
79 changes: 65 additions & 14 deletions src/ORM/DataQuery_SubGroup.php
Expand Up @@ -4,6 +4,8 @@

use SilverStripe\ORM\Queries\SQLConditionGroup;
use SilverStripe\ORM\Queries\SQLSelect;
use InvalidArgumentException;
use LogicException;

/**
* Represents a subgroup inside a WHERE clause in a {@link DataQuery}
Expand All @@ -14,26 +16,54 @@
*/
class DataQuery_SubGroup extends DataQuery implements SQLConditionGroup
{
private string $clause;

/**
*
* @var SQLSelect
*/
protected $whereQuery;

/**
* @var SQLSelect
*/
protected $havingQuery;

/**
* @param DataQuery $base
* @param string $connective
* @param string $clause
*/
public function __construct(DataQuery $base, $connective)
{
// using func_get_args to add a 3rd param while retaining BC
// @deprecated - add a 3rd param for CMS 6 - string $clause = 'WHERE'
$clause = 'WHERE';
$args = func_get_args();
if (count($args) > 2) {
$clause = $args[2];
}
parent::__construct($base->dataClass);
$this->query = $base->query;
$this->whereQuery = new SQLSelect();
$this->whereQuery->setConnective($connective);

$base->where($this);
$this->clause = strtoupper($clause);
if ($this->clause === 'WHERE') {
$this->whereQuery = new SQLSelect();
$this->whereQuery->setConnective($connective);
$base->where($this);
} elseif ($this->clause === 'HAVING') {
$this->havingQuery = new SQLSelect();
$this->havingQuery->setConnective($connective);
$base->having($this);
} else {
throw new InvalidArgumentException('$clause must be either WHERE or HAVING');
}
}

public function where($filter)
{
if ($filter) {
if ($this->clause === 'HAVING') {
throw new LogicException('Cannot call where() when clause is set to HAVING');
}
if ($filter && $this->whereQuery) {
$this->whereQuery->addWhere($filter);
}

Expand All @@ -42,25 +72,46 @@ public function where($filter)

public function whereAny($filter)
{
if ($filter) {
if ($this->clause === 'HAVING') {
throw new LogicException('Cannot call whereAny() when clause is set to HAVING');
}
if ($filter && $this->whereQuery) {
$this->whereQuery->addWhereAny($filter);
}

return $this;
}

public function having($filter)
{
if ($this->clause === 'WHERE') {
throw new LogicException('Cannot call having() when clause is set to WHERE');
}
if ($filter && $this->havingQuery) {
$this->havingQuery->addHaving($filter);
}

return $this;
}

public function conditionSQL(&$parameters)
{
$parameters = [];

// Ignore empty conditions
$where = $this->whereQuery->getWhere();
if (empty($where)) {
return null;
if ($this->clause === 'WHERE') {
$where = $this->whereQuery->getWhere();
if (!empty($where)) {
$sql = DB::get_conn()->getQueryBuilder()->buildWhereFragment($this->whereQuery, $parameters);
return preg_replace('/^\s*WHERE\s*/i', '', $sql ?? '');
}
} elseif ($this->clause === 'HAVING') {
$having = $this->havingQuery->getHaving();
if (!empty($having)) {
$sql = DB::get_conn()->getQueryBuilder()->buildHavingFragment($this->havingQuery, $parameters);
return preg_replace('/^\s*HAVING\s*/i', '', $sql ?? '');
}
}

// Allow database to manage joining of conditions
$sql = DB::get_conn()->getQueryBuilder()->buildWhereFragment($this->whereQuery, $parameters);
return preg_replace('/^\s*WHERE\s*/i', '', $sql ?? '');
return null;
}
}
1 change: 0 additions & 1 deletion src/ORM/ManyManyList.php
Expand Up @@ -15,7 +15,6 @@
*/
class ManyManyList extends RelationList
{

/**
* @var string $joinTable
*/
Expand Down
30 changes: 30 additions & 0 deletions tests/php/ORM/DataListTest.php
Expand Up @@ -1073,6 +1073,36 @@ public function testFilterAnyArrayInArray()
);
}

private function createTeam(int $playerCount)
{
$team = Team::create();
$team->write();
for ($i = 0; $i < $playerCount; $i++) {
$player = Player::create();
$player->write();
$team->Players()->add($player);
}
return $team;
}

public function testFilterAnyManyManyAggregate()
{
Team::get()->removeAll();
$team1 = $this->createTeam(1);
$team2 = $this->createTeam(2);
$team3 = $this->createTeam(3);
$list = Team::get()->filterAny([
'Players.Count():LessThan' => 2,
'Players.Count():GreaterThan' => 2,
]);
$match = 'HAVING ((COUNT("players_Member"."ID") < ?) OR (COUNT("players_Member"."ID") > ?))';
$sql = str_replace("\n", '', $list->sql());
$this->assertTrue(str_contains($sql, $match));
$ids = $list->column('ID');
sort($ids);
$this->assertSame([$team1->ID, $team3->ID], $ids);
}

public function testFilterOnJoin()
{
$list = TeamComment::get()
Expand Down
45 changes: 45 additions & 0 deletions tests/php/ORM/DataQuery_SubGroupTest.php
@@ -0,0 +1,45 @@
<?php

namespace SilverStripe\ORM\Tests;

use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\DataQuery;
use SilverStripe\ORM\DataQuery_SubGroup;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\Tests\DataObjectTest\Team;
use LogicException;
use InvalidArgumentException;

class DataQuery_SubGroupTest extends SapphireTest
{
public function testConstructorException()
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('$clause must be either WHERE or HAVING');
new DataQuery_SubGroup(new DataQuery(Team::class), 'AND', 'INVALID');
}

public function testWhereException()
{
$this->expectException(LogicException::class);
$this->expectExceptionMessage('Cannot call where() when clause is set to HAVING');
$query = new DataQuery_SubGroup(new DataQuery(Team::class), 'AND', 'HAVING');
$query->where([]);
}

public function testWhereAnyException()
{
$this->expectException(LogicException::class);
$this->expectExceptionMessage('Cannot call whereAny() when clause is set to HAVING');
$query = new DataQuery_SubGroup(new DataQuery(Team::class), 'AND', 'HAVING');
$query->whereAny([]);
}

public function testHavingException()
{
$this->expectException(LogicException::class);
$this->expectExceptionMessage('Cannot call having() when clause is set to WHERE');
$query = new DataQuery_SubGroup(new DataQuery(Team::class), 'AND', 'WHERE');
$query->having([]);
}
}

0 comments on commit 696fe79

Please sign in to comment.