diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index d1ce87e..dd2a0d0 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -4,7 +4,7 @@ on: pull_request: push: branches: - - "1.4" + - "2.0" jobs: coding-quality: @@ -18,7 +18,7 @@ jobs: steps: - name: "Checkout" - uses: "actions/checkout@v2" + uses: "actions/checkout@v4" - name: "Install PHP" uses: "shivammathur/setup-php@v2" @@ -28,7 +28,7 @@ jobs: tools: "cs2pr" - name: "Cache dependencies installed with composer" - uses: "actions/cache@v1" + uses: "actions/cache@v4" with: path: "~/.composer/cache" key: "php-${{ matrix.php-version }}-composer-locked-${{ hashFiles('composer.lock') }}" @@ -60,7 +60,7 @@ jobs: steps: - name: "Checkout" - uses: "actions/checkout@v2" + uses: "actions/checkout@v4" with: fetch-depth: 2 @@ -71,7 +71,7 @@ jobs: extensions: "" - name: "Cache dependencies installed with composer" - uses: "actions/cache@v1" + uses: "actions/cache@v4" with: path: "~/.composer/cache" key: "php-${{ matrix.php-version }}-composer-locked-${{ hashFiles('composer.lock') }}" @@ -109,7 +109,7 @@ jobs: steps: - name: "Checkout" - uses: "actions/checkout@v2" + uses: "actions/checkout@v4" with: fetch-depth: 2 @@ -121,7 +121,7 @@ jobs: coverage: "pcov" - name: "Cache dependencies installed with composer" - uses: "actions/cache@v1" + uses: "actions/cache@v4" with: path: "~/.composer/cache" key: "php-${{ matrix.php-version }}-composer-locked-${{ hashFiles('composer.lock') }}" @@ -140,7 +140,9 @@ jobs: run: "vendor/bin/phpunit" - name: "Run Coveralls" - run: "vendor/bin/coveralls -v" + run: "vendor/bin/php-coveralls -v" + env: + COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }} phpunit: name: "PHPUnit" @@ -149,7 +151,7 @@ jobs: strategy: matrix: php-version: - - "8.0" + - "8.2" services: mysql: @@ -162,7 +164,7 @@ jobs: steps: - name: "Checkout" - uses: "actions/checkout@v2" + uses: "actions/checkout@v4" with: fetch-depth: 2 @@ -173,7 +175,7 @@ jobs: extensions: "" - name: "Cache dependencies installed with composer" - uses: "actions/cache@v1" + uses: "actions/cache@v4" with: path: "~/.composer/cache" key: "php-${{ matrix.php-version }}-composer-locked-${{ hashFiles('composer.lock') }}" diff --git a/.gitignore b/.gitignore index 8fe7799..a008000 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ vendor/ composer.lock phpunit.xml -build/ \ No newline at end of file +build/ +.phpunit.result.cache \ No newline at end of file diff --git a/composer.json b/composer.json index e4b93e2..5a62b47 100644 --- a/composer.json +++ b/composer.json @@ -18,15 +18,15 @@ "mouf/utils.value.value-interface": "~1.0", "mouf/utils.common.paginable-interface": "~1.0", "mouf/utils.common.sortable-interface": "~1.0", - "mouf/schema-analyzer": "~1.0", + "mouf/schema-analyzer": "^2.0", "twig/twig": "^2.11 || ^3", "greenlion/php-sql-parser": "^4.3", "doctrine/cache": "^1.5" }, "require-dev": { "phpunit/phpunit": "^9.5", - "satooshi/php-coveralls": "~1.0", - "doctrine/dbal": "~2.5", + "php-coveralls/php-coveralls": "^2.0.0", + "doctrine/dbal": "^3.0", "phpstan/phpstan": "^0.12.82" }, "suggest": { diff --git a/phpstan.neon b/phpstan.neon index c85f7c3..f7010a7 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -8,6 +8,7 @@ parameters: inferPrivatePropertyTypeFromConstructor: true checkMissingIterableValueType: false checkGenericClassInNonGenericObjectType: false + reportUnmatchedIgnoredErrors: false ignoreErrors: - "#Mouf\\\\MoufManager#" - "#Mouf\\\\MoufInstanceDescriptor#" diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 61da686..e7699e8 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,37 +1,37 @@ - - - - - - - - - - - - - - ./tests/ - - - - - - src/ - - - - - - + + + src/ + + + + + + + + + + + + + + + + ./tests/ + + + diff --git a/src/Mouf/Database/MagicQuery.php b/src/Mouf/Database/MagicQuery.php index 7dbdb8e..a9c04f6 100644 --- a/src/Mouf/Database/MagicQuery.php +++ b/src/Mouf/Database/MagicQuery.php @@ -10,6 +10,7 @@ use function array_filter; use function array_keys; use Doctrine\Common\Cache\VoidCache; +use Doctrine\DBAL\Platforms\MySQL80Platform; use function hash; use function implode; use Mouf\Database\MagicQuery\Twig\SqlTwigEnvironmentFactory; @@ -54,7 +55,7 @@ class MagicQuery public function __construct($connection = null, $cache = null, SchemaAnalyzer $schemaAnalyzer = null) { $this->connection = $connection; - $this->platform = $connection ? $connection->getDatabasePlatform() : new MySqlPlatform(); + $this->platform = $connection ? $connection->getDatabasePlatform() : new MySQL80Platform(); if ($cache) { $this->cache = $cache; } else { @@ -91,7 +92,7 @@ public function setOutputDialect(?AbstractPlatform $platform): self if ($platform !== null) { $this->platform = $platform; } else { - $this->platform = $this->connection ? $this->connection->getDatabasePlatform() : new MySqlPlatform(); + $this->platform = $this->connection ? $this->connection->getDatabasePlatform() : new MySQL80Platform(); } return $this; @@ -304,7 +305,7 @@ private function getSchemaAnalyzer() throw new MagicQueryMissingConnectionException('In order to use MagicJoin, you need to configure a DBAL connection.'); } - $this->schemaAnalyzer = new SchemaAnalyzer($this->connection->getSchemaManager(), $this->cache, $this->getConnectionUniqueId($this->connection)); + $this->schemaAnalyzer = new SchemaAnalyzer($this->connection->createSchemaManager(), $this->cache, $this->getConnectionUniqueId($this->connection)); } return $this->schemaAnalyzer; @@ -312,7 +313,10 @@ private function getSchemaAnalyzer() private function getConnectionUniqueId(Connection $connection): string { - return hash('md4', $connection->getHost().'-'.$connection->getPort().'-'.$connection->getDatabase().'-'.$connection->getDriver()->getName()); + $connectionParams = $connection->getParams(); + \assert(\array_key_exists('host', $connectionParams)); + \assert(\array_key_exists('port', $connectionParams)); + return hash('md4', $connectionParams['host'].'-'.$connectionParams['port'].'-'.$connection->getDatabase().'-'.$connection->getDriver()->getDatabasePlatform()->getName()); } /** diff --git a/src/Mouf/Database/QueryWriter/CountNbResult.php b/src/Mouf/Database/QueryWriter/CountNbResult.php index 0de2238..106425d 100644 --- a/src/Mouf/Database/QueryWriter/CountNbResult.php +++ b/src/Mouf/Database/QueryWriter/CountNbResult.php @@ -50,6 +50,6 @@ public function val() { $sql = 'SELECT count(*) as cnt FROM ('.$this->queryResult->toSql().') tmp'; - return $this->connection->fetchColumn($sql); + return $this->connection->fetchOne($sql); } } diff --git a/src/Mouf/Database/QueryWriter/QueryResult.php b/src/Mouf/Database/QueryWriter/QueryResult.php index d0287b3..3eab17a 100644 --- a/src/Mouf/Database/QueryWriter/QueryResult.php +++ b/src/Mouf/Database/QueryWriter/QueryResult.php @@ -77,9 +77,9 @@ public function setParameters($parameters): void public function val() { $parameters = ValueUtils::val($this->parameters); - $pdoStatement = $this->connection->query($this->select->toSql($parameters, $this->connection->getDatabasePlatform()).DbHelper::getFromLimitString($this->offset, $this->limit)); + $pdoStatement = $this->connection->prepare($this->select->toSql($parameters, $this->connection->getDatabasePlatform()).DbHelper::getFromLimitString($this->offset, $this->limit)); - return new ResultSet($pdoStatement); + return new ResultSet($pdoStatement->getWrappedStatement()); } /** diff --git a/src/Mouf/Database/QueryWriter/ResultSet.php b/src/Mouf/Database/QueryWriter/ResultSet.php index 5cffdec..85c41ca 100644 --- a/src/Mouf/Database/QueryWriter/ResultSet.php +++ b/src/Mouf/Database/QueryWriter/ResultSet.php @@ -2,7 +2,6 @@ namespace Mouf\Database\QueryWriter; -use Doctrine\DBAL\FetchMode; use Doctrine\DBAL\Driver\Statement; /** @@ -16,7 +15,7 @@ class ResultSet implements \Iterator private $statement; /** @var int */ private $key = 0; - /** @var array|false */ + /** @var array */ private $result; /** @var bool */ private $fetched = false; @@ -28,7 +27,7 @@ public function __construct(Statement $statement) $this->statement = $statement; } - public function rewind() + public function rewind(): void { ++$this->rewindCalls; if ($this->rewindCalls == 2) { @@ -36,10 +35,7 @@ public function rewind() } } - /** - * @return array|false - */ - public function current() + public function current(): array { if (!$this->fetched) { $this->fetch(); @@ -48,15 +44,12 @@ public function current() return $this->result; } - /** - * @return int - */ - public function key() + public function key(): int { return $this->key; } - public function next() + public function next(): void { ++$this->key; $this->fetched = false; @@ -65,11 +58,11 @@ public function next() private function fetch(): void { - $this->result = $this->statement->fetch(FetchMode::ASSOCIATIVE); + $this->result = $this->statement->execute()->fetchAllAssociative(); $this->fetched = true; } - public function valid() + public function valid(): bool { if (!$this->fetched) { $this->fetch(); diff --git a/src/SQLParser/Node/NodeFactory.php b/src/SQLParser/Node/NodeFactory.php index 54df8f9..ecb37f1 100644 --- a/src/SQLParser/Node/NodeFactory.php +++ b/src/SQLParser/Node/NodeFactory.php @@ -653,7 +653,9 @@ public static function simplify($nodes) if (empty($operand->getBaseExpression())) { $subTree = $operand->getSubTree(); if (is_array($subTree) && count($subTree) === 1) { - $newNodes = array_merge($newNodes, self::simplify($subTree)); + $simplifiedSubTree = self::simplify($subTree); + \assert(is_array($simplifiedSubTree)); + $newNodes = array_merge($newNodes, $simplifiedSubTree); } else { $newNodes[] = $operand; } diff --git a/src/SQLParser/Query/StatementFactory.php b/src/SQLParser/Query/StatementFactory.php index e5cebaf..9447f17 100644 --- a/src/SQLParser/Query/StatementFactory.php +++ b/src/SQLParser/Query/StatementFactory.php @@ -92,14 +92,27 @@ public static function toObject(array $desc) } return $select; - } elseif (isset($desc['UNION'])) { + } + // UNION and UNION DISTINCT have similar behavior + if (isset($desc['UNION']) || isset($desc['UNION ALL']) || isset($desc['UNION DISTINCT'])) { + $isUnionAll = isset($desc['UNION ALL']); + $unionStatement = $desc['UNION'] ?? ($desc['UNION ALL'] ?? $desc['UNION DISTINCT']); + /** @var Select[] $selects */ - $selects = array_map([self::class, 'toObject'], $desc['UNION']); + $selects = array_map([self::class, 'toObject'], $unionStatement); - return new Union($selects); - } else { - throw new \BadMethodCallException('Unknown query'); + $union = new Union($selects, $isUnionAll); + + if (isset($desc['0']) && isset($desc['0']['ORDER'])) { + $order = NodeFactory::mapArrayToNodeObjectList($desc['0']['ORDER']); + $order = NodeFactory::simplify($order); + $union->setOrder($order); + } + + return $union; } + + throw new \BadMethodCallException('Unknown query'); } /** diff --git a/src/SQLParser/Query/Union.php b/src/SQLParser/Query/Union.php index 255ecf3..5478c5a 100644 --- a/src/SQLParser/Query/Union.php +++ b/src/SQLParser/Query/Union.php @@ -25,13 +25,42 @@ class Union implements StatementInterface, NodeInterface */ private $selects; + /** + * @var bool + */ + private $isUnionAll; + /** * Union constructor. * @param Select[] $selects */ - public function __construct(array $selects) + public function __construct(array $selects, bool $isUnionAll) { $this->selects = $selects; + $this->isUnionAll = $isUnionAll; + } + + /** @var NodeInterface[]|NodeInterface */ + private $order; + + /** + * Returns the list of order statements. + * + * @return NodeInterface[]|NodeInterface + */ + public function getOrder() + { + return $this->order; + } + + /** + * Sets the list of order statements. + * + * @param NodeInterface[]|NodeInterface $order + */ + public function setOrder($order): void + { + $this->order = $order; } /** @@ -43,6 +72,7 @@ public function toInstanceDescriptor(MoufManager $moufManager) { $instanceDescriptor = $moufManager->createInstance(get_called_class()); $instanceDescriptor->getProperty('selects')->setValue(NodeFactory::nodeToInstanceDescriptor($this->selects, $moufManager)); + $instanceDescriptor->getProperty('order')->setValue(NodeFactory::nodeToInstanceDescriptor($this->order, $moufManager)); return $instanceDescriptor; } @@ -59,6 +89,7 @@ public function overwriteInstanceDescriptor($name, MoufManager $moufManager) //$name = $moufManager->findInstanceName($this); $instanceDescriptor = $moufManager->getInstanceDescriptor($name); $instanceDescriptor->getProperty('selects')->setValue(NodeFactory::nodeToInstanceDescriptor($this->selects, $moufManager)); + $instanceDescriptor->getProperty('order')->setValue(NodeFactory::nodeToInstanceDescriptor($this->order, $moufManager)); return $instanceDescriptor; } @@ -80,7 +111,16 @@ public function toSql(array $parameters, AbstractPlatform $platform, int $indent return $select->toSql($parameters, $platform, $indent, $conditionsMode, $extrapolateParameters); }, $this->selects); - $sql = implode(' UNION ', $selectsSql); + $unionStatement = $this->isUnionAll ? 'UNION ALL' : 'UNION'; + + $sql = '(' . implode(') ' . $unionStatement . ' (', $selectsSql) . ')'; + + if (!empty($this->order)) { + $order = NodeFactory::toSql($this->order, $platform, $parameters, ',', false, $indent + 2, $conditionsMode, $extrapolateParameters); + if ($order) { + $sql .= "\nORDER BY ".$order; + } + } return $sql; } @@ -99,27 +139,37 @@ public function walk(VisitorInterface $visitor) } if ($result !== NodeTraverser::DONT_TRAVERSE_CHILDREN) { $this->walkChildren($this->selects, $visitor); + $this->walkChildren($this->order, $visitor); } return $visitor->leaveNode($node); } /** - * @param array $children + * @param array|NodeInterface|null $children * @param VisitorInterface $visitor */ - private function walkChildren(array &$children, VisitorInterface $visitor): void + private function walkChildren(&$children, VisitorInterface $visitor): void { if ($children) { - foreach ($children as $key => $operand) { - if ($operand) { - $result2 = $operand->walk($visitor); - if ($result2 === NodeTraverser::REMOVE_NODE) { - unset($children[$key]); - } elseif ($result2 instanceof NodeInterface) { - $children[$key] = $result2; + if (is_array($children)) { + foreach ($children as $key => $operand) { + if ($operand) { + $result2 = $operand->walk($visitor); + if ($result2 === NodeTraverser::REMOVE_NODE) { + unset($children[$key]); + } elseif ($result2 instanceof NodeInterface) { + $children[$key] = $result2; + } } } + } else { + $result2 = $children->walk($visitor); + if ($result2 === NodeTraverser::REMOVE_NODE) { + $children = null; + } elseif ($result2 instanceof NodeInterface) { + $children = $result2; + } } } } diff --git a/tests/Mouf/Database/MagicQuery/Twig/SqlTwigEnvironmentFactoryTest.php b/tests/Mouf/Database/MagicQuery/Twig/SqlTwigEnvironmentFactoryTest.php index 7d2fdc5..c72bba2 100644 --- a/tests/Mouf/Database/MagicQuery/Twig/SqlTwigEnvironmentFactoryTest.php +++ b/tests/Mouf/Database/MagicQuery/Twig/SqlTwigEnvironmentFactoryTest.php @@ -1,6 +1,6 @@ assertEquals('SELECT COUNT(DISTINCT a, b) FROM users', self::simplifySql($magicQuery->build($sql))); $sql = 'SELECT a FROM users UNION SELECT a FROM users'; - $this->assertEquals('SELECT a FROM users UNION SELECT a FROM users', self::simplifySql($magicQuery->build($sql))); + $this->assertEquals('(SELECT a FROM users) UNION (SELECT a FROM users)', self::simplifySql($magicQuery->build($sql))); + + $sql = 'SELECT a FROM users UNION ALL SELECT a FROM users'; + $this->assertEquals('(SELECT a FROM users) UNION ALL (SELECT a FROM users)', self::simplifySql($magicQuery->build($sql))); + + $sql = 'SELECT a FROM users UNION DISTINCT SELECT a FROM users'; + $this->assertEquals('(SELECT a FROM users) UNION (SELECT a FROM users)', self::simplifySql($magicQuery->build($sql))); $sql = 'SELECT a FROM users u, users u2'; $this->assertEquals('SELECT a FROM users u CROSS JOIN users u2', self::simplifySql($magicQuery->build($sql))); @@ -459,7 +466,7 @@ public function testBuildPreparedStatement() public function testSetOutputDialect() { $magicQuery = new MagicQuery(null, new ArrayCache()); - $magicQuery->setOutputDialect(new PostgreSqlPlatform()); + $magicQuery->setOutputDialect(new PostgreSQL100Platform()); $sql = 'SELECT id FROM users'; $this->assertEquals('SELECT "id" FROM "users"', self::simplifySql($magicQuery->buildPreparedStatement($sql))); @@ -478,4 +485,34 @@ public function testPrepareStatementCache(): void $this->assertEquals('SELECT * FROM users WHERE 0 <> 0', self::simplifySql($magicQuery->buildPreparedStatement('SELECT * FROM users WHERE id IN :users', ['users' => []]))); $this->assertEquals('SELECT * FROM users WHERE id IN (:users)', self::simplifySql($magicQuery->buildPreparedStatement('SELECT * FROM users WHERE id IN :users', ['users' => [1]]))); } + + public function testUnionAndOrderBy(): void + { + $magicQuery = new MagicQuery(null, new ArrayCache()); + + $query = '(SELECT id FROM users) UNION (SELECT id FROM users) ORDER BY users.id ASC'; + + $this->assertEquals( + '(SELECT id FROM users) UNION (SELECT id FROM users) ORDER BY users.id ASC', + self::simplifySql($magicQuery->buildPreparedStatement($query)) + ); + + $query = '(SELECT id FROM users) UNION (SELECT id FROM users ORDER BY users.id ASC)'; + $this->assertEquals( + '(SELECT id FROM users) UNION (SELECT id FROM users ORDER BY users.id ASC)', + self::simplifySql($magicQuery->buildPreparedStatement($query)) + ); + + $query = '(SELECT id FROM users ORDER BY users.id ASC) UNION (SELECT id FROM users)'; + $this->assertEquals( + '(SELECT id FROM users ORDER BY users.id ASC) UNION (SELECT id FROM users)', + self::simplifySql($magicQuery->buildPreparedStatement($query)) + ); + + $query = 'SELECT id FROM users UNION SELECT id FROM users ORDER BY users.id ASC'; + $this->assertEquals( + '(SELECT id FROM users) UNION (SELECT id FROM users) ORDER BY users.id ASC', + self::simplifySql($magicQuery->buildPreparedStatement($query)) + ); + } } diff --git a/tests/phpunit-mysql8.sh b/tests/phpunit-mysql8.sh new file mode 100755 index 0000000..cd41775 --- /dev/null +++ b/tests/phpunit-mysql8.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash + +# Use this file to start a MySQL8 database using Docker and then run the test suite on the MySQL8 database. + +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +cd $DIR +cd .. + +docker run --rm --name mysql8-magic-query-test -p 3306:3306 -p 33060:33060 -e MYSQL_ROOT_PASSWORD= -e MYSQL_ALLOW_EMPTY_PASSWORD=1 -d mysql:8 mysqld --default-authentication-plugin=mysql_native_password + +# Let's wait for MySQL 8 to start +sleep 20 + +vendor/bin/phpunit -c phpunit.xml.dist $NO_COVERAGE +RESULT_CODE=$? + +docker stop mysql8-magic-query-test + +exit $RESULT_CODE