Skip to content
This repository has been archived by the owner on Nov 26, 2017. It is now read-only.

Fix for omition of the glue on WHERE after the first call in query builder #956

Closed
wants to merge 13 commits into from
Closed
98 changes: 84 additions & 14 deletions libraries/joomla/database/query.php
Expand Up @@ -30,28 +30,50 @@ class JDatabaseQueryElement
*/
protected $elements = null;

/**
* @var array An array of elements with glues.
* @since 11.1
*/
protected $elements_with_glues = null;

/**
* @var string Glue piece.
* @since 11.1
*/
protected $glue = null;

/**
* @var string Start of a nesting of element
* @since 12.1
*/
protected $nesting_start = null;

/**
* @var string End of a nesting of element
* @since 12.1
*/
protected $nesting_end = null;

/**
* Constructor.
*
* @param string $name The name of the element.
* @param mixed $elements String or array.
* @param string $glue The glue for elements.
*
* @param string $name The name of the element.
* @param mixed $elements String or array.
* @param string $glue The default glue for elements.
* @param string $nesting_start Starting character for nesting
* @param string $nesting_end Ending character for nesting
* @param bool $nested If there is nesting for first added elements
* @since 11.1
*/
public function __construct($name, $elements, $glue = ',')
public function __construct($name, $elements, $glue = ',',$nesting_start = '',$nesting_end = '',$nested = false)
{
$this->elements = array();
$this->name = $name;
$this->glue = $glue;
$this->nesting_start = $nesting_start;
$this->nesting_end = $nesting_end;

$this->append($elements);
$this->append($elements,$glue,$nested);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need spaces after commas in a bunch of places.

}

/**
Expand All @@ -65,32 +87,80 @@ public function __toString()
{
if (substr($this->name, -2) == '()')
{
return PHP_EOL . substr($this->name, 0, -2) . '(' . implode($this->glue, $this->elements) . ')';
return PHP_EOL . substr($this->name, 0, -2) . '(' . implode('', $this->elements_with_glues) . ')';
}
else
{
return PHP_EOL . $this->name . ' ' . implode($this->glue, $this->elements);
return PHP_EOL . $this->name . ' ' . implode('', $this->elements_with_glues);
}
}

/**
* Appends element parts to the internal list.
*
* @param mixed $elements String or array.
* @param mixed $elements String or array.
* @param string $glue A glue that will replace the default one if different from null.
* @param boolean $nesting In case of an array, do we need to nest it ?
* @param boolean $nesting_start To notify the function that the last action done was the starting of a nesting
*
* @return void
*
* @since 11.1
*/
public function append($elements)
public function append($elements, $glue = null, $nesting = false, $nesting_start = false)
{
if($glue === null)
{
$glue = $this->glue;
}

if (is_array($elements))
{
$this->elements = array_merge($this->elements, $elements);
//Won't add the glue on the first element of the list or of a nesting
if (count($this->elements) > 0 && $nesting_start == false)
{
$this->elements_with_glues[] = $glue;
}

if ($nesting)
$this->elements_with_glues[] = $this->nesting_start;

$firstelement = true;
foreach ($elements as $element)
{
if (is_array($element) && isset($element["glue"]))
{
$this->append(
$element[0],
$element['glue'],
$nesting,
$firstelement?true:false
);
}
else
{
$this->append(
$element,
$glue,
$nesting,
$firstelement?true:false
);
}
$firstelement = false;
}

if ($nesting)
$this->elements_with_glues[] = $this->nesting_end;
}
else
{
$this->elements = array_merge($this->elements, array($elements));
//Won't add the glue on the first element of the list or of a nesting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put a space after the //.

if (count($this->elements) > 0 && $nesting_start == false)
{
$this->elements_with_glues[] = $glue;
}
$this->elements[] = $elements;
$this->elements_with_glues[] = $elements;
}
}

Expand Down Expand Up @@ -1458,11 +1528,11 @@ public function where($conditions, $glue = 'AND')
if (is_null($this->where))
{
$glue = strtoupper($glue);
$this->where = new JDatabaseQueryElement('WHERE', $conditions, " $glue ");
$this->where = new JDatabaseQueryElement('WHERE', $conditions, " $glue ", "(", ")", true);
}
else
{
$this->where->append($conditions);
$this->where->append($conditions," $glue ", true);
}

return $this;
Expand Down
109 changes: 105 additions & 4 deletions tests/suite/joomla/database/JDatabaseQueryElementTest.php
Expand Up @@ -127,25 +127,25 @@ public function dataTestToString()
'FROM',
'table1',
',',
"\nFROM table1"
PHP_EOL . "FROM table1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you put the all the PHP_EOL changes into a separate pull? I think that will fix up some problems we are having with windows testing in general and I'd like to get those in while your pull is being reviewed. Thanks!

),
array(
'SELECT',
array('column1', 'column2'),
',',
"\nSELECT column1,column2"
PHP_EOL . "SELECT column1,column2"
),
array(
'()',
array('column1', 'column2'),
',',
"\n(column1,column2)"
PHP_EOL . "(column1,column2)"
),
array(
'CONCAT()',
array('column1', 'column2'),
',',
"\nCONCAT(column1,column2)"
PHP_EOL . "CONCAT(column1,column2)"
),
);
}
Expand Down Expand Up @@ -265,4 +265,105 @@ public function test__clone_object()
$this->assertFalse($baseElement === $cloneElement);
$this->assertFalse($baseElement->testObject === $cloneElement->testObject);
}

/**
* Tests case for nesting of elements.
*
* @return array
*
* @since 12.1
*/
public function dataTestNested()
{
return array(
array(
'WHERE',
array(
"a=1",
"b=1"
),
'AND',
PHP_EOL . "WHERE (a=1 AND b=1)"
),
array(
'WHERE',
array(
"a=1",
"b=1",
array(
"c=1",
array(
"c=2",
"glue" => " OR "
)
)
),
'AND',
PHP_EOL . "WHERE (a=1 AND b=1 AND (c=1 OR c=2))"
),
array(
'WHERE',
array(
"a=1",
"b=1",
array(
"c=1",
array(
array(
"c=2",
"d=1"
),
"glue" => " OR "
)
)
),
'AND',
PHP_EOL . "WHERE (a=1 AND b=1 AND (c=1 OR (c=2 OR d=1)))"
),
array(
'WHERE',
array(
"a=1",
"b=1",
array(
"c=1",
array(
array(
"c=2",
array(
"d=1",
"glue" => " AND "
)
),
"glue" => " OR "
)
)
),
'AND',
PHP_EOL . "WHERE (a=1 AND b=1 AND (c=1 OR (c=2 AND d=1)))"
)
);
}

/**
* Tests JDatabaseQueryElement::__constructor for nesting of elements.
*
* @param string $name name of the element
* @param mixed $elements append elements value
* @param string $glue default glue
* @param string $expected expected string generated by __toString
*
* @dataProvider dataTestNested
*
* @since 12.1
*/
public function testNested($name, $elements, $glue, $expected)
{
$e = new JDatabaseQueryElement($name, $elements, " $glue ","(",")", true);

$this->assertThat(
(string) $e,
$this->equalTo($expected)
);
}
}