Skip to content

Commit

Permalink
Refactor to avoid instanceof checks
Browse files Browse the repository at this point in the history
  • Loading branch information
theodorejb committed Oct 13, 2024
1 parent d582e33 commit 51a820f
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 49 deletions.
7 changes: 7 additions & 0 deletions lib/BaseOptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ abstract class BaseOptions
*/
public int $maxInsertRows = 0;

public string $insertIdSelector = '';

/**
* Escapes a table or column name, and validates that it isn't blank
*/
Expand All @@ -38,4 +40,9 @@ public function escapeIdentifier(string $identifier): string
$qualifiedIdentifiers = array_map($escaper, explode('.', $identifier));
return implode('.', $qualifiedIdentifiers);
}

public function buildPagination(int $limit, int $offset): string
{
return "LIMIT {$limit} OFFSET {$offset}";
}
}
8 changes: 1 addition & 7 deletions lib/QueryBuilder/Insert.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,7 @@ public function buildQuery(string $table, array $colVals): SqlParams
$valStr = ' VALUES' . substr_replace(str_repeat($valSetStr, count($colVals)), '', -1); // remove trailing comma
$params = array_merge(...array_map('array_values', $colVals));

if ($this->options instanceof \PeachySQL\SqlServer\Options) {
$selStr = '; SELECT SCOPE_IDENTITY() AS RowID;';
} else {
$selStr = '';
}

return new SqlParams($insert . $valStr . $selStr, $params);
return new SqlParams($insert . $valStr . $this->options->insertIdSelector, $params);
}

/**
Expand Down
9 changes: 0 additions & 9 deletions lib/QueryBuilder/Select.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,4 @@ public function buildOrderByClause(array $orderBy): string

return substr_replace($sql, '', -2); // remove trailing comma and space
}

public function buildPagination(int $limit, int $offset): string
{
if ($this->options instanceof \PeachySQL\SqlServer\Options) {
return "OFFSET {$offset} ROWS FETCH NEXT {$limit} ROWS ONLY";
} else {
return "LIMIT {$limit} OFFSET {$offset}";
}
}
}
2 changes: 1 addition & 1 deletion lib/QueryBuilder/Selector.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public function getSqlParams(): SqlParams
throw new \Exception('Results must be sorted to use an offset');
}

$sql .= ' ' . $select->buildPagination($this->limit, $this->offset);
$sql .= ' ' . $this->options->buildPagination($this->limit, $this->offset);
}

return new SqlParams($sql, [...$this->query->params, ...$where->params]);
Expand Down
6 changes: 6 additions & 0 deletions lib/SqlServer/Options.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,10 @@ class Options extends BaseOptions
// https://learn.microsoft.com/en-us/sql/sql-server/maximum-capacity-specifications-for-sql-server
public int $maxBoundParams = 2100 - 1;
public int $maxInsertRows = 1000;
public string $insertIdSelector = '; SELECT SCOPE_IDENTITY() AS RowID;';

public function buildPagination(int $limit, int $offset): string
{
return "OFFSET {$offset} ROWS FETCH NEXT {$limit} ROWS ONLY";
}
}
23 changes: 9 additions & 14 deletions test/DbTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace PeachySQL\Test;

use PeachySQL\{PeachySql, SqlException, SqlServer};
use PeachySQL\{PeachySql, SqlException};
use PeachySQL\QueryBuilder\SqlParams;
use PHPUnit\Framework\TestCase;
use Ramsey\Uuid\Uuid;
Expand All @@ -22,6 +22,8 @@ abstract class DbTestCase extends TestCase
*/
abstract public static function dbProvider(): array;

abstract protected function getExpectedBadSyntaxCode(): int;

/**
* @dataProvider dbProvider
*/
Expand Down Expand Up @@ -93,17 +95,9 @@ public function testException(PeachySql $peachySql): void
$this->assertSame($badQuery, $e->query);
$this->assertSame([], $e->params);
$this->assertSame('42000', $e->getSqlState());

if ($peachySql instanceof SqlServer) {
$this->assertSame(102, $e->getCode());
$this->assertStringEndsWith("Incorrect syntax near 'WHERE'.", $e->getMessage());
} else {
$this->assertSame(1064, $e->getCode());
$expectedMessage = "Failed to prepare statement: You have an error in your"
. " SQL syntax; check the manual that corresponds to your MySQL server"
. " version for the right syntax to use near '' at line 1";
$this->assertSame($expectedMessage, $e->getMessage());
}
$this->assertSame($this->getExpectedBadSyntaxCode(), $e->getCode());
$this->assertStringContainsString(' syntax ', $e->getMessage());
$this->assertStringContainsString(" near '", $e->getMessage());
}
}

Expand Down Expand Up @@ -188,13 +182,14 @@ public function testInsertBulk(PeachySql $peachySql): void
}

$insertColVals = [];
$expectedQueries = ($peachySql instanceof SqlServer) ? 2 : 1;

foreach ($colVals as $row) {
$row['uuid'] = $peachySql->makeBinaryParam($row['uuid']);
$insertColVals[] = $row;
}

$totalBoundParams = count($insertColVals[0]) * $rowCount;
$expectedQueries = ($totalBoundParams > $peachySql->options->maxBoundParams) ? 2 : 1;

$result = $peachySql->insertRows($this->table, $insertColVals);
$this->assertSame($expectedQueries, $result->queryCount);
$this->assertSame($rowCount, $result->affected);
Expand Down
5 changes: 5 additions & 0 deletions test/Mysql/MysqlDbTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ class MysqlDbTest extends DbTestCase
{
private static ?Mysql $db = null;

protected function getExpectedBadSyntaxCode(): int
{
return 1064;
}

public static function dbProvider(): array
{
if (!self::$db) {
Expand Down
11 changes: 11 additions & 0 deletions test/Mysql/OptionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,15 @@ public function testEscapeIdentifier(): void
$this->assertSame('Identifier cannot be blank', $e->getMessage());
}
}

public function testBuildPagination(): void
{
$options = new Options();

$page1 = $options->buildPagination(25, 0);
$this->assertSame('LIMIT 25 OFFSET 0', $page1);

$page3 = $options->buildPagination(100, 200);
$this->assertSame('LIMIT 100 OFFSET 200', $page3);
}
}
18 changes: 0 additions & 18 deletions test/QueryBuilder/SelectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,24 +125,6 @@ public function testInvalidOrderBy(): void
$select->buildOrderByClause(['testcol' => 'nonsense']);
}

public function testBuildPagination(): void
{
$mySqlSelect = new Select(new MysqlOptions());
$sqlsrvSelect = new Select(new Options());

$mySqlPage1 = $mySqlSelect->buildPagination(25, 0);
$this->assertSame('LIMIT 25 OFFSET 0', $mySqlPage1);

$sqlsrvPage1 = $sqlsrvSelect->buildPagination(25, 0);
$this->assertSame('OFFSET 0 ROWS FETCH NEXT 25 ROWS ONLY', $sqlsrvPage1);

$mySqlPage3 = $mySqlSelect->buildPagination(100, 200);
$this->assertSame('LIMIT 100 OFFSET 200', $mySqlPage3);

$sqlsrvPage3 = $sqlsrvSelect->buildPagination(100, 200);
$this->assertSame('OFFSET 200 ROWS FETCH NEXT 100 ROWS ONLY', $sqlsrvPage3);
}

public function testGetSqlParams(): void
{
$query = "SELECT * FROM MyTable a INNER JOIN AnotherTable b ON b.id = a.id";
Expand Down
5 changes: 5 additions & 0 deletions test/SqlServer/MssqlDbTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ class MssqlDbTest extends DbTestCase
{
private static ?SqlServer $db = null;

protected function getExpectedBadSyntaxCode(): int
{
return 102;
}

public static function dbProvider(): array
{
if (!self::$db) {
Expand Down
11 changes: 11 additions & 0 deletions test/SqlServer/OptionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,15 @@ public function testEscapeIdentifier(): void
$this->assertSame('Identifier cannot be blank', $e->getMessage());
}
}

public function testBuildPagination(): void
{
$options = new Options();

$page1 = $options->buildPagination(25, 0);
$this->assertSame('OFFSET 0 ROWS FETCH NEXT 25 ROWS ONLY', $page1);

$page3 = $options->buildPagination(100, 200);
$this->assertSame('OFFSET 200 ROWS FETCH NEXT 100 ROWS ONLY', $page3);
}
}

0 comments on commit 51a820f

Please sign in to comment.