Skip to content

Commit 37d2989

Browse files
n0nag0nn0nag0n
committed
Major changes to fix wrap() bug
1 parent f6a8e34 commit 37d2989

File tree

6 files changed

+318
-58
lines changed

6 files changed

+318
-58
lines changed

‎src/ActiveRecord.php‎

Lines changed: 94 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,14 @@ abstract class ActiveRecord extends Base implements JsonSerializable
7171
public const HAS_ONE = 'has_one';
7272

7373
/**
74-
* @var array Stored the SQL Expressions of the SQL.
74+
* @var array Store the SQL expressions inside wrapped (parentheses).
7575
*/
76-
protected array $expressions = [];
76+
protected array $wrapExpressions = [];
77+
78+
/**
79+
* @var boolean if a statement is being wrapped in parentheses
80+
*/
81+
protected bool $wrap = false;
7782

7883
/**
7984
* @var array Stored the Expressions of the SQL.
@@ -83,7 +88,7 @@ abstract class ActiveRecord extends Base implements JsonSerializable
8388
/**
8489
* @var string SQL that is built to be used by execute()
8590
*/
86-
protected string $built_sql = '';
91+
protected string $builtSql = '';
8792

8893
/**
8994
* Captures all the joins that are made
@@ -196,6 +201,9 @@ public function __call($name, $args)
196201
$operator = ActiveRecordData::OPERATORS[$name];
197202
$value = isset($args[1]) ? $args[1] : null;
198203
$last_arg = end($args);
204+
205+
// If the last arg is "OR" make this an OR condition
206+
// e.g. $this->eq('name', 'John', 'or')->eq('age', 25);
199207
$and_or_or = is_string($last_arg) && strtolower($last_arg) === 'or' ? 'OR' : 'AND';
200208

201209
$this->addCondition($field, $operator, $value, $and_or_or);
@@ -648,39 +656,47 @@ protected function &getRelation(string $name)
648656
}
649657
/**
650658
* helper function to build SQL with sql parts.
651-
* @param string $sql_statement The SQL part will be build.
659+
* @param string $sqlStatement The SQL part will be build.
652660
* @param ActiveRecord $o The reference to $this
653661
* @return string
654662
*/
655-
protected function buildSqlCallback(string $sql_statement, ActiveRecord $object): string
663+
protected function buildSqlCallback(string $sqlStatement, ActiveRecord $object): string
656664
{
657-
if ('select' === $sql_statement && null == $object->$sql_statement) {
658-
$sql_statement = strtoupper($sql_statement) . ' ' . $this->escapeIdentifier($object->table) . '.*';
659-
} elseif (('update' === $sql_statement || 'from' === $sql_statement) && null == $object->$sql_statement) {
660-
$sql_statement = strtoupper($sql_statement) . ' ' . $this->escapeIdentifier($object->table);
661-
} elseif ('delete' === $sql_statement) {
662-
$sql_statement = strtoupper($sql_statement) . ' ';
665+
// First add the SELECT table.*
666+
if ('select' === $sqlStatement && null == $object->$sqlStatement) {
667+
$sqlStatement = strtoupper($sqlStatement) . ' ' . $this->escapeIdentifier($object->table) . '.*';
668+
} elseif (('update' === $sqlStatement || 'from' === $sqlStatement) && null == $object->$sqlStatement) {
669+
$sqlStatement = strtoupper($sqlStatement) . ' ' . $this->escapeIdentifier($object->table);
670+
} elseif ('delete' === $sqlStatement) {
671+
$sqlStatement = strtoupper($sqlStatement);
663672
} else {
664-
$sql_statement = (null !== $object->$sql_statement) ? $object->$sql_statement . ' ' : '';
673+
$sqlStatement = (null !== $object->$sqlStatement) ? (string) $object->$sqlStatement : '';
665674
}
666675

667-
return $sql_statement;
676+
return $sqlStatement;
668677
}
669678

670679
/**
671680
* helper function to build SQL with sql parts.
672-
* @param array $sql_statements The SQL part will be build.
681+
* @param array $sqlStatements The SQL part will be build.
673682
* @return string
674683
*/
675-
protected function buildSql(array $sql_statements = []): string
684+
protected function buildSql(array $sqlStatements = []): string
676685
{
677-
foreach ($sql_statements as &$sql) {
678-
$sql = $this->buildSqlCallback($sql, $this);
686+
$finalSql = [];
687+
foreach ($sqlStatements as $sql) {
688+
$statement = $this->buildSqlCallback($sql, $this);
689+
if ($statement !== '') {
690+
$finalSql[] = $statement;
691+
}
679692
}
680693
//this code to debug info.
681-
//echo 'SQL: ', implode(' ', $sql_statements), "\n", "PARAMS: ", implode(', ', $this->params), "\n";
682-
$this->built_sql = implode(' ', $sql_statements);
683-
return $this->built_sql;
694+
//echo 'SQL: ', implode(' ', $sqlStatements), "\n", "PARAMS: ", implode(', ', $this->params), "\n";
695+
$this->builtSql = implode(' ', $finalSql);
696+
697+
// get rid of multiple spaces in the query for prettiness
698+
$this->builtSql = preg_replace('/\s+/', ' ', $this->builtSql);
699+
return $this->builtSql;
684700
}
685701

686702
/**
@@ -690,30 +706,46 @@ protected function buildSql(array $sql_statements = []): string
690706
*/
691707
public function getBuiltSql(): string
692708
{
693-
return $this->built_sql;
709+
return $this->builtSql;
710+
}
711+
712+
public function startWrap(): self
713+
{
714+
$this->wrap = true;
715+
return $this;
716+
}
717+
718+
/**
719+
* Alias to encWrap
720+
*
721+
* @deprecated 0.6.0
722+
* @param string|null $op If give this param will build one WrapExpressions include the stored expressions add into WHERE. Otherwise will stored the expressions into array.
723+
* @return self
724+
*/
725+
public function wrap(?string $op = null): self
726+
{
727+
return $op === null ? $this->startWrap() : $this->endWrap($op);
694728
}
729+
695730
/**
696731
* make wrap when build the SQL expressions of WHERE.
697732
* @param string $op If give this param will build one WrapExpressions include the stored expressions add into WHERE. Otherwise will stored the expressions into array.
698733
* @return ActiveRecord
699734
*/
700-
public function wrap(?string $op = null)
735+
public function endWrap(string $op): self
701736
{
702-
if ($op !== null) {
703-
$this->wrap = false;
704-
if (is_array($this->expressions) && count($this->expressions) > 0) {
705-
$this->addConditionGroup(new WrapExpressions([
706-
'delimiter' => ' ',
707-
'target' => $this->expressions
708-
]), 'or' === strtolower($op) ? 'OR' : 'AND');
709-
}
710-
$this->expressions = [];
711-
} else {
712-
$this->wrap = true;
737+
$this->wrap = false;
738+
if (is_array($this->wrapExpressions) === true && count($this->wrapExpressions) > 0) {
739+
$this->addCondition(new WrapExpressions([
740+
'delimiter' => ('or' === strtolower($op) ? ' OR ' : ' AND '),
741+
'target' => $this->wrapExpressions
742+
]), null, null);
713743
}
744+
$this->wrapExpressions = [];
714745
return $this;
715746
}
716747

748+
717749
/**
718750
* helper function to build place holder when making SQL expressions.
719751
* @param mixed $value the value will bind to SQL, just store it in $this->params.
@@ -738,25 +770,31 @@ protected function filterParam($value)
738770
/**
739771
* helper function to add condition into WHERE.
740772
* create the SQL Expressions.
741-
* @param string $field The field name, the source of Expressions
742-
* @param string $operator
773+
* @param string|Expressions $field The field name, the source of Expressions
774+
* @param ?string $operator
743775
* @param mixed $value the target of the Expressions
744776
* @param string $delimiter the operator to concat this Expressions into WHERE or SET statement.
745777
* @param string $name The Expression will contact to.
746778
*/
747-
public function addCondition(string $field, string $operator, $value, string $delimiter = 'AND', string $name = 'where')
779+
public function addCondition($field, ?string $operator, $value, string $delimiter = 'AND', string $name = 'where')
748780
{
749781
// This will catch unique conditions such as IS NULL, IS NOT NULL, etc
750782
// You only need to filter by a param if there's a param to really filter by
751-
if (stripos($operator, 'NULL') === false) {
783+
// A true null value is passed in from a endWrap() method to skip the param.
784+
if ($operator !== null && stripos((string) $operator, 'NULL') === false) {
752785
$value = $this->filterParam($value);
753786
}
787+
788+
// This is used for wrapped expressions so extra statements aren't printed out.
789+
if ($operator === null) {
790+
$operator = '';
791+
}
754792
$name = strtolower($name);
755793

756794
// skip adding the `table.` prefix if it's already there or a function is being supplied.
757-
$skip_table_prefix = (strpos($field, '.') !== false || strpos($field, '(') !== false);
795+
$skipTablePrefix = $field instanceof WrapExpressions || is_string($field) === true && (strpos($field, '.') !== false || strpos($field, '(') !== false);
758796
$expressions = new Expressions([
759-
'source' => ('where' === $name && $skip_table_prefix === false ? $this->escapeIdentifier($this->table) . '.' : '') . $this->escapeIdentifier($field),
797+
'source' => ('where' === $name && $skipTablePrefix === false ? $this->escapeIdentifier($this->table) . '.' : '') . (is_string($field) === true ? $this->escapeIdentifier($field) : $field),
760798
'operator' => $operator,
761799
'target' => (
762800
is_array($value)
@@ -769,10 +807,11 @@ public function addCondition(string $field, string $operator, $value, string $de
769807
)
770808
]);
771809
if ($expressions) {
772-
if (!$this->wrap) {
810+
if ($this->wrap === false) {
773811
$this->addConditionGroup($expressions, $delimiter, $name);
774812
} else {
775-
$this->addExpression($expressions, $delimiter);
813+
// This method is only for wrapping conditions in parentheses
814+
$this->addExpression($expressions);
776815
}
777816
}
778817
}
@@ -805,12 +844,13 @@ public function join(string $table, string $on, string $type = 'LEFT')
805844
* @param Expressions $exp The expression will be stored.
806845
* @param string $delimiter The operator to concat this Expressions into WHERE statement.
807846
*/
808-
protected function addExpression(Expressions $expressions, string $delimiter)
847+
protected function addExpression(Expressions $expressions)
809848
{
810-
if (!is_array($this->expressions) || count($this->expressions) == 0) {
811-
$this->expressions = [ $expressions ];
849+
$wrapExpressions =& $this->wrapExpressions;
850+
if (is_array($wrapExpressions) === false || count($wrapExpressions) === 0) {
851+
$wrapExpressions = [ $expressions ];
812852
} else {
813-
$this->expressions[] = new Expressions(['operator' => $delimiter, 'target' => $expressions]);
853+
$wrapExpressions[] = new Expressions([ 'operator' => '', 'target' => $expressions ]);
814854
}
815855
}
816856

@@ -823,9 +863,16 @@ protected function addExpression(Expressions $expressions, string $delimiter)
823863
protected function addConditionGroup(Expressions $expressions, string $operator, string $name = 'where')
824864
{
825865
if (!$this->{$name}) {
826-
$this->{$name} = new Expressions(['operator' => strtoupper($name) , 'target' => $expressions]);
866+
$this->{$name} = new Expressions([
867+
'operator' => strtoupper($name),
868+
'target' => $expressions
869+
]);
827870
} else {
828-
$this->{$name}->target = new Expressions(['source' => $this->$name->target, 'operator' => $operator, 'target' => $expressions]);
871+
$this->{$name}->target = new Expressions([
872+
'source' => $this->$name->target,
873+
'operator' => $operator,
874+
'target' => $expressions
875+
]);
829876
}
830877
}
831878

‎src/WrapExpressions.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ class WrapExpressions extends Expressions
1616

1717
public function __toString()
1818
{
19-
return $this->start . implode(($this->delimiter ? $this->delimiter : ','), $this->target) . $this->end;
19+
if (is_array($this->target) === true) {
20+
$this->target = array_map(function ($target) {
21+
return $target instanceof Expressions ? $target->__toString() : $target;
22+
}, $this->target);
23+
}
24+
return $this->start . implode($this->delimiter, $this->target) . $this->end;
2025
}
2126
}

‎tests/ActiveRecordPdoIntegrationTest.php‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ public function testEdit()
8686
$this->assertEquals($original_id, $user->id);
8787

8888
$sql = $user->getBuiltSql();
89-
$this->assertStringContainsString('UPDATE "user" SET "name" = :ph3 , "password" = :ph4 WHERE "user"."id" = :ph5', $sql);
89+
$this->assertStringContainsString('UPDATE "user" SET "name" = :ph3 , "password" = :ph4 WHERE "user"."id" = :ph5', $sql);
9090
}
9191

9292
public function testUpdateNoChanges()
@@ -235,7 +235,7 @@ public function getDirty()
235235

236236
$user->isNotNull('id')->eq('id', 1)->lt('id', 2)->gt('id', 0)->find();
237237
$sql = $user->getBuiltSql();
238-
$this->assertStringContainsString('SELECT "user".* FROM "user" WHERE "user"."id" IS NOT NULL AND "user"."id" = 1 AND "user"."id" < 2 AND "user"."id" > 0', $sql);
238+
$this->assertStringContainsString('SELECT "user".* FROM "user" WHERE "user"."id" IS NOT NULL AND "user"."id" = 1 AND "user"."id" < 2 AND "user"."id" > 0', $sql);
239239
$this->assertGreaterThan(0, $user->id);
240240
$this->assertSame([], $user->getDirty());
241241
$user->name = 'testname';
@@ -245,7 +245,7 @@ public function getDirty()
245245
unset($user->name);
246246
$this->assertSame([], $user->getDirty());
247247
$user->isNotNull('id')->eq('id', 'aaa"')->wrap()->lt('id', 2)->gt('id', 0)->wrap('OR')->find();
248-
$this->assertGreaterThan(0, $user->id);
248+
$this->assertEmpty($user->id);
249249
$user->isNotNull('id')->between('id', [0, 2])->find();
250250
$this->assertGreaterThan(0, $user->id);
251251
}
@@ -260,7 +260,7 @@ public function testWrapWithArrays()
260260
$user->password = md5('demo1');
261261
$user->insert();
262262

263-
$users = $user->isNotNull('id')->wrap('OR')->in('name', [ 'demo', 'demo1' ])->wrap('AND')->lt('id', 3)->gt('id', 0)->wrap('OR')->findAll();
263+
$users = $user->isNotNull('id')->wrap()->in('name', [ 'demo', 'demo1' ])->wrap('OR')->lt('id', 3)->gt('id', 0)->findAll();
264264
$this->assertGreaterThan(0, $users[0]->id);
265265
$this->assertGreaterThan(0, $users[1]->id);
266266
}
@@ -293,7 +293,7 @@ public function testDelete()
293293
$this->assertEmpty($new_contact->id);
294294
$this->assertInstanceOf(User::class, $new_user->find($uid));
295295
$this->assertEmpty($new_user->id);
296-
$this->assertStringContainsString('DELETE FROM "contact" WHERE "contact"."id" = :ph4', $sql);
296+
$this->assertStringContainsString('DELETE FROM "contact" WHERE "contact"."id" = :ph4', $sql);
297297
}
298298

299299
public function testDeleteWithConditions()
@@ -671,4 +671,37 @@ public function testCallMethodPassingToPdoConnection()
671671
$this->assertInstanceOf(PdoStatementAdapter::class, $result);
672672
$this->assertNotInstanceOf(ActiveRecord::class, $result);
673673
}
674+
675+
public function testWrapMultipleOrStatements()
676+
{
677+
$record = new User(new PDO('sqlite:test.db'));
678+
$record
679+
->notNull('name')
680+
->startWrap()
681+
->eq('name', 'John')
682+
->eq('id', 1)
683+
->gte('password', '123')
684+
->endWrap('OR')
685+
->eq('id', 2)
686+
->find();
687+
$sql = $record->getBuiltSql();
688+
$this->assertEquals('SELECT "user".* FROM "user" WHERE "user"."name" IS NOT NULL AND ("user"."name" = :ph1 OR "user"."id" = 1 OR "user"."password" >= :ph2) AND "user"."id" = 2 LIMIT 1', $sql);
689+
}
690+
691+
public function testWrapWithComplexLogic()
692+
{
693+
$record = new User(new PDO('sqlite:test.db'));
694+
$record
695+
->startWrap()
696+
->eq('name', 'John')
697+
->in('id', [ 1,5,9 ])
698+
->eq('id', 1)
699+
->endWrap('OR')
700+
->notNull('name')
701+
->between('id', [ 1, 2 ])
702+
->join('contact', '"contact"."user_id" = "user"."id"')
703+
->find();
704+
$sql = $record->getBuiltSql();
705+
$this->assertEquals('SELECT "user".* FROM "user" LEFT JOIN contact ON "contact"."user_id" = "user"."id" WHERE ("user"."name" = :ph1 OR "user"."id" IN (:ph2,:ph3,:ph4) OR "user"."id" = 1) AND "user"."name" IS NOT NULL AND "user"."id" BETWEEN :ph5 AND :ph6 LIMIT 1', $sql);
706+
}
674707
}

‎tests/ActiveRecordTest.php‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@
1010

1111
class ActiveRecordTest extends \PHPUnit\Framework\TestCase
1212
{
13+
protected function createEmptyRecord()
14+
{
15+
return new class (null, 'test_table') extends ActiveRecord {
16+
};
17+
}
18+
1319
public function testMagicSet()
1420
{
1521
$pdo_mock = $this->createStub(PDO::class);
@@ -117,17 +123,15 @@ public function testIsDirty()
117123

118124
public function testCopyFrom()
119125
{
120-
$record = new class (null, 'test_table') extends ActiveRecord {
121-
};
126+
$record = $this->createEmptyRecord();
122127
$record->copyFrom(['name' => 'John']);
123128
$this->assertEquals('John', $record->name);
124129
$this->assertEquals(['name' => 'John'], $record->getData());
125130
}
126131

127132
public function testIsset()
128133
{
129-
$record = new class (null, 'test_table') extends ActiveRecord {
130-
};
134+
$record = $this->createEmptyRecord();
131135
$record->name = 'John';
132136
$this->assertTrue(isset($record->name));
133137
$this->assertFalse(isset($record->email));
@@ -144,7 +148,7 @@ public function query(string $sql, array $param = [], ?ActiveRecord $obj = null,
144148
$record->join('table1', 'table1.some_id = test_table.id');
145149
$record->join('table2', 'table2.some_id = table1.id');
146150
$result = $record->find()->getBuiltSql();
147-
$this->assertEquals('SELECT test_table.* FROM test_table LEFT JOIN table1 ON table1.some_id = test_table.id LEFT JOIN table2 ON table2.some_id = table1.id LIMIT 1 ', $result);
151+
$this->assertEquals('SELECT test_table.* FROM test_table LEFT JOIN table1 ON table1.some_id = test_table.id LEFT JOIN table2 ON table2.some_id = table1.id LIMIT 1', $result);
148152
}
149153

150154
public function testEscapeIdentifierSqlSrv()

0 commit comments

Comments
 (0)