Browse Source

Merge branch '5.x' into 5.next

Mark Story 1 year ago
parent
commit
cd35ec5b5f

+ 1 - 1
.github/workflows/ci.yml

@@ -46,7 +46,7 @@ jobs:
           - 11211/tcp
 
     steps:
-    - name: Setup MySQL
+    - name: Setup MySQL 8.0
       if: matrix.db-type == 'mysql'
       run: |
         sudo service mysql start

+ 1 - 0
composer.json

@@ -83,6 +83,7 @@
         "psr/simple-cache-implementation": "^3.0"
     },
     "config": {
+        "lock": false,
         "process-timeout": 900,
         "sort-packages": true,
         "allow-plugins": {

+ 6 - 3
src/Command/I18nExtractCommand.php

@@ -185,10 +185,13 @@ class I18nExtractCommand extends Command
         }
         if ($args->getOption('paths')) {
             $this->_paths = explode(',', (string)$args->getOption('paths'));
-        } elseif ($args->getOption('plugin')) {
+        }
+        if ($args->getOption('plugin')) {
             $plugin = Inflector::camelize((string)$args->getOption('plugin'));
-            $this->_paths = [Plugin::classPath($plugin), Plugin::templatePath($plugin)];
-        } else {
+            if (empty($this->_paths)) {
+                $this->_paths = [Plugin::classPath($plugin), Plugin::templatePath($plugin)];
+            }
+        } elseif (!$args->getOption('paths')) {
             $this->_getPaths($io);
         }
 

+ 1 - 0
src/Database/Query.php

@@ -1847,6 +1847,7 @@ abstract class Query implements ExpressionInterface, Stringable
                 '(help)' => 'This is a Query object, to get the results execute or iterate it.',
                 'sql' => $sql,
                 'params' => $params,
+                'role' => $this->connectionRole,
                 'defaultTypes' => $this->getDefaultTypes(),
                 'executed' => (bool)$this->_statement,
             ];

+ 1 - 1
src/Database/Query/SelectQuery.php

@@ -147,7 +147,7 @@ class SelectQuery extends Query implements IteratorAggregate
      * $query->select('id', true); // Resets the list: SELECT id
      * $query->select(['total' => $countQuery]); // SELECT id, (SELECT ...) AS total
      * $query->select(function ($query) {
-     *     return ['article_id', 'total' => $query->count('*')];
+     *     return ['article_id', 'total' => $query->func()->count('*')];
      * })
      * ```
      *

+ 1 - 1
src/Database/README.md

@@ -218,7 +218,7 @@ Generating conditions:
 // WHERE id = 1
 $query->where(['id' => 1]);
 
-// WHERE id > 2
+// WHERE id > 1
 $query->where(['id >' => 1]);
 ```
 

+ 8 - 4
src/Error/Renderer/WebExceptionRenderer.php

@@ -189,7 +189,8 @@ class WebExceptionRenderer implements ExceptionRendererInterface
         } catch (Throwable $e) {
             Log::warning(
                 "Failed to construct or call startup() on the resolved controller class of `$class`. " .
-                    "Using Fallback Controller instead. Error {$e->getMessage()}",
+                    "Using Fallback Controller instead. Error {$e->getMessage()}" .
+                    "\nStack Trace\n: {$e->getTraceAsString()}",
                 'cake.error'
             );
             $controller = null;
@@ -420,7 +421,8 @@ class WebExceptionRenderer implements ExceptionRendererInterface
             return $this->_shutdown();
         } catch (MissingTemplateException $e) {
             Log::warning(
-                "MissingTemplateException - Failed to render error template `{$template}`. Error: {$e->getMessage()}",
+                "MissingTemplateException - Failed to render error template `{$template}` . Error: {$e->getMessage()}" .
+                    "\nStack Trace\n: {$e->getTraceAsString()}",
                 'cake.error'
             );
             $attributes = $e->getAttributes();
@@ -434,7 +436,8 @@ class WebExceptionRenderer implements ExceptionRendererInterface
             return $this->_outputMessage('error500');
         } catch (MissingPluginException $e) {
             Log::warning(
-                "MissingPluginException - Failed to render error template `{$template}`. Error: {$e->getMessage()}",
+                "MissingPluginException - Failed to render error template `{$template}`. Error: {$e->getMessage()}" .
+                    "\nStack Trace\n: {$e->getTraceAsString()}",
                 'cake.error'
             );
             $attributes = $e->getAttributes();
@@ -445,7 +448,8 @@ class WebExceptionRenderer implements ExceptionRendererInterface
             return $this->_outputMessageSafe('error500');
         } catch (Throwable $outer) {
             Log::warning(
-                "Throwable - Failed to render error template `{$template}`. Error: {$outer->getMessage()}",
+                "Throwable - Failed to render error template `{$template}`. Error: {$outer->getMessage()}" .
+                    "\nStack Trace\n: {$outer->getTraceAsString()}",
                 'cake.error'
             );
             try {

+ 2 - 1
src/ORM/Association/Loader/SelectLoader.php

@@ -178,7 +178,8 @@ class SelectLoader
             ->select($options['fields'])
             ->where($options['conditions'])
             ->eagerLoaded(true)
-            ->enableHydration($selectQuery->isHydrationEnabled());
+            ->enableHydration($selectQuery->isHydrationEnabled())
+            ->setConnectionRole($selectQuery->getConnectionRole());
         if ($selectQuery->isResultsCastingEnabled()) {
             $fetchQuery->enableResultsCasting();
         } else {

+ 2 - 3
src/ORM/EagerLoader.php

@@ -557,10 +557,9 @@ class EagerLoader
     protected function _correctStrategy(EagerLoadable $loadable): void
     {
         $config = $loadable->getConfig();
-        $currentStrategy = $config['strategy'] ??
-            'join';
+        $currentStrategy = $config['strategy'] ?? Association::STRATEGY_JOIN;
 
-        if (!$loadable->canBeJoined() || $currentStrategy !== 'join') {
+        if (!$loadable->canBeJoined() || $currentStrategy !== Association::STRATEGY_JOIN) {
             return;
         }
 

+ 40 - 31
src/ORM/Table.php

@@ -2669,45 +2669,54 @@ class Table implements RepositoryInterface, EventListenerInterface, EventDispatc
         $reflected = new ReflectionFunction($callable);
         $params = $reflected->getParameters();
         $secondParam = $params[1] ?? null;
-        $secondParamType = null;
 
-        if ($args === [] || isset($args[0])) {
-            $secondParamType = $secondParam?->getType();
-            $secondParamTypeName = $secondParamType instanceof ReflectionNamedType ? $secondParamType->getName() : null;
-            // Backwards compatibility of 4.x style finders with signature `findFoo(SelectQuery $query, array $options)`
-            // called as `find('foo')` or `find('foo', [..])`
-            if (
-                count($params) === 2 &&
-                $secondParam?->name === 'options' &&
-                !$secondParam->isVariadic() &&
-                ($secondParamType === null || $secondParamTypeName === 'array')
-            ) {
-                if (isset($args[0])) {
-                    deprecationWarning(
-                        '5.0.0',
-                        'Using options array for the `find()` call is deprecated.'
-                        . ' Use named arguments instead.'
-                    );
-
-                    $args = $args[0];
-                }
+        $secondParamType = $secondParam?->getType();
+        $secondParamTypeName = $secondParamType instanceof ReflectionNamedType ? $secondParamType->getName() : null;
 
-                $query->applyOptions($args);
-
-                return $callable($query, $query->getOptions());
-            }
+        $secondParamIsOptions = (
+            count($params) === 2 &&
+            $secondParam?->name === 'options' &&
+            !$secondParam->isVariadic() &&
+            ($secondParamType === null || $secondParamTypeName === 'array')
+        );
 
-            // Backwards compatibility for core finders like `findList()` called in 4.x style
-            // with an array `find('list', ['valueField' => 'foo'])` instead of `find('list', valueField: 'foo')`
-            if (isset($args[0]) && is_array($args[0]) && $secondParamTypeName !== 'array') {
+        if (($args === [] || isset($args[0])) && $secondParamIsOptions) {
+            // Backwards compatibility of 4.x style finders
+            // with signature `findFoo(SelectQuery $query, array $options)`
+            // called as `find('foo')` or `find('foo', [..])`
+            if (isset($args[0])) {
                 deprecationWarning(
                     '5.0.0',
-                    "Calling `{$reflected->getName()}` finder with options array is deprecated."
-                     . ' Use named arguments instead.'
+                    'Calling finders with options arrays is deprecated.'
+                    . ' Update your finder methods to used named arguments instead.'
                 );
-
                 $args = $args[0];
             }
+            $query->applyOptions($args);
+
+            return $callable($query, $query->getOptions());
+        }
+
+        // Backwards compatibility for 4.x style finders with signatures like
+        // `findFoo(SelectQuery $query, array $options)` called as
+        // `find('foo', key: $value)`.
+        if (!isset($args[0]) && $secondParamIsOptions) {
+            $query->applyOptions($args);
+
+            return $callable($query, $query->getOptions());
+        }
+
+        // Backwards compatibility for core finders like `findList()` called in 4.x
+        // style with an array `find('list', ['valueField' => 'foo'])` instead of
+        // `find('list', valueField: 'foo')`
+        if (isset($args[0]) && is_array($args[0]) && $secondParamTypeName !== 'array') {
+            deprecationWarning(
+                '5.0.0',
+                "Calling `{$reflected->getName()}` finder with options array is deprecated."
+                 . ' Use named arguments instead.'
+            );
+
+            $args = $args[0];
         }
 
         if ($args) {

+ 3 - 0
tests/TestCase/Database/Query/SelectQueryTest.php

@@ -17,6 +17,7 @@ declare(strict_types=1);
 namespace Cake\Test\TestCase\Database\Query;
 
 use ArrayIterator;
+use Cake\Database\Connection;
 use Cake\Database\Driver\Mysql;
 use Cake\Database\Driver\Postgres;
 use Cake\Database\Driver\Sqlite;
@@ -3265,6 +3266,7 @@ class SelectQueryTest extends TestCase
             'params' => [
                 ':c0' => ['value' => '1', 'type' => 'integer', 'placeholder' => 'c0'],
             ],
+            'role' => Connection::ROLE_WRITE,
             'defaultTypes' => ['id' => 'integer'],
             'decorators' => 0,
             'executed' => false,
@@ -3279,6 +3281,7 @@ class SelectQueryTest extends TestCase
             'params' => [
                 ':c0' => ['value' => '1', 'type' => 'integer', 'placeholder' => 'c0'],
             ],
+            'role' => Connection::ROLE_WRITE,
             'defaultTypes' => ['id' => 'integer'],
             'decorators' => 0,
             'executed' => true,

+ 74 - 0
tests/TestCase/ORM/Association/BelongsToManyTest.php

@@ -18,6 +18,7 @@ namespace Cake\Test\TestCase\ORM\Association;
 
 use Cake\Database\Connection;
 use Cake\Database\Driver;
+use Cake\Database\Driver\Sqlite;
 use Cake\Database\Exception\DatabaseException;
 use Cake\Database\Expression\OrderClauseExpression;
 use Cake\Database\Expression\QueryExpression;
@@ -25,6 +26,7 @@ use Cake\Datasource\ConnectionManager;
 use Cake\Datasource\EntityInterface;
 use Cake\Event\EventInterface;
 use Cake\I18n\DateTime;
+use Cake\Log\Log;
 use Cake\ORM\Association\BelongsTo;
 use Cake\ORM\Association\BelongsToMany;
 use Cake\ORM\Association\HasMany;
@@ -102,6 +104,13 @@ class BelongsToManyTest extends TestCase
         ]);
     }
 
+    public function tearDown(): void
+    {
+        parent::tearDown();
+        ConnectionManager::drop('test_read_write');
+        Log::drop('queries');
+    }
+
     /**
      * Tests setForeignKey()
      */
@@ -1702,4 +1711,69 @@ class BelongsToManyTest extends TestCase
         // 4 records in the junction table.
         $this->assertCount(4, $results);
     }
+
+    public function testEagerLoaderConnectionRole(): void
+    {
+        $this->skipIf(!extension_loaded('pdo_sqlite'), 'Skipping as SQLite extension is missing');
+
+        Log::setConfig('queries', [
+            'className' => 'Array',
+            'scopes' => ['queriesLog'],
+        ]);
+
+        ConnectionManager::setConfig('test_read_write', [
+            'className' => Connection::class,
+            'driver' => Sqlite::class,
+            'write' => [
+                'database' => ':memory:',
+                'cached' => 'shared', // used so role configs are unique
+                'log' => true,
+            ],
+            'read' => [
+                'database' => ':memory:',
+                'log' => true,
+            ],
+        ]);
+
+        $connection = ConnectionManager::get('test_read_write');
+        $this->assertNotSame($connection->getDriver(Connection::ROLE_READ), $connection->getDriver(Connection::ROLE_WRITE));
+
+        // Create belongs to many relationships with unique table names
+        $driver = $connection->getDriver(Connection::ROLE_WRITE);
+        $driver->execute('CREATE TABLE unique_items (id int PRIMARY KEY);');
+        $driver->execute('CREATE TABLE articles (id int PRIMARY KEY);');
+        $driver->execute('CREATE TABLE articles_unique_items (unique_item_id int, article_id int);');
+
+        $driver = $connection->getDriver(Connection::ROLE_READ);
+        $driver->execute('CREATE TABLE unique_items (id int PRIMARY KEY);');
+        $driver->execute('CREATE TABLE articles (id int PRIMARY KEY);');
+        $driver->execute('CREATE TABLE articles_unique_items (unique_item_id int, article_id int);');
+        $driver->execute('INSERT INTO unique_items (id) VALUES (1)');
+        $driver->execute('INSERT INTO articles (id) VALUES (1)');
+        $driver->execute('INSERT INTO articles_unique_items (unique_item_id, article_id) VALUES (1, 1)');
+
+        $articles = $this->getTableLocator()->get('Articles')->setConnection($connection);
+        $articles->belongsToMany('UniqueItems')->getTarget()->setConnection($connection);
+
+        $query = $articles->find();
+        $this->assertSame(Connection::ROLE_WRITE, $query->getConnectionRole(), 'This test assumes select queries still default to write role');
+
+        $results = $query->contain('UniqueItems')->useReadRole()->toArray();
+        $this->assertCount(1, $results);
+        $this->assertCount(1, $results[0]->unique_items);
+        $this->assertSame(1, $results[0]->unique_items[0]->id);
+
+        $logs = Log::engine('queries')->read();
+        $this->assertNotEmpty($logs);
+
+        foreach ($logs as $log) {
+            if (
+                str_contains($log, 'FROM articles') ||
+                str_contains($log, 'FROM articles_unique_items') ||
+                str_contains($log, 'FROM unique_items')
+            ) {
+                $this->assertStringContainsString('role=read', $log);
+            }
+        }
+    }
 }

+ 71 - 0
tests/TestCase/ORM/Association/HasManyTest.php

@@ -16,6 +16,8 @@ declare(strict_types=1);
  */
 namespace Cake\Test\TestCase\ORM\Association;
 
+use Cake\Database\Connection;
+use Cake\Database\Driver\Sqlite;
 use Cake\Database\Driver\Sqlserver;
 use Cake\Database\Expression\OrderByExpression;
 use Cake\Database\Expression\OrderClauseExpression;
@@ -23,6 +25,7 @@ use Cake\Database\Expression\QueryExpression;
 use Cake\Database\Expression\TupleComparison;
 use Cake\Database\TypeMap;
 use Cake\Datasource\ConnectionManager;
+use Cake\Log\Log;
 use Cake\ORM\Association;
 use Cake\ORM\Association\HasMany;
 use Cake\ORM\Entity;
@@ -116,6 +119,13 @@ class HasManyTest extends TestCase
         $this->autoQuote = $connection->getDriver()->isAutoQuotingEnabled();
     }
 
+    public function tearDown(): void
+    {
+        parent::tearDown();
+        ConnectionManager::drop('test_read_write');
+        Log::drop('queries');
+    }
+
     /**
      * Tests that foreignKey() returns the correct configured value
      */
@@ -1565,4 +1575,65 @@ class HasManyTest extends TestCase
         $this->assertNull($Articles->get($article2->get('id'))->get('author_id'));
         $this->assertEquals($author->get('id'), $Articles->get($article3->get('id'))->get('author_id'));
     }
+
+    public function testEagerLoaderConnectionRole(): void
+    {
+        $this->skipIf(!extension_loaded('pdo_sqlite'), 'Skipping as SQLite extension is missing');
+
+        Log::setConfig('queries', [
+            'className' => 'Array',
+            'scopes' => ['queriesLog'],
+        ]);
+
+        ConnectionManager::setConfig('test_read_write', [
+            'className' => Connection::class,
+            'driver' => Sqlite::class,
+            'write' => [
+                'database' => ':memory:',
+                'cached' => 'shared', // used so role configs are unique
+                'log' => true,
+            ],
+            'read' => [
+                'database' => ':memory:',
+                'log' => true,
+            ],
+        ]);
+
+        $connection = ConnectionManager::get('test_read_write');
+        $this->assertNotSame($connection->getDriver(Connection::ROLE_READ), $connection->getDriver(Connection::ROLE_WRITE));
+
+        // Create belongs to many relationships with unique table names
+        $driver = $connection->getDriver(Connection::ROLE_WRITE);
+        $driver->execute('CREATE TABLE unique_items (id int PRIMARY KEY, article_id int);');
+        $driver->execute('CREATE TABLE articles (id int PRIMARY KEY);');
+
+        $driver = $connection->getDriver(Connection::ROLE_READ);
+        $driver->execute('CREATE TABLE unique_items (id int PRIMARY KEY, article_id int);');
+        $driver->execute('CREATE TABLE articles (id int PRIMARY KEY);');
+        $driver->execute('INSERT INTO unique_items (id, article_id) VALUES (1, 1)');
+        $driver->execute('INSERT INTO articles (id) VALUES (1)');
+
+        $articles = $this->getTableLocator()->get('Articles')->setConnection($connection);
+        $articles->hasMany('UniqueItems')->setStrategy('select')->getTarget()->setConnection($connection);
+
+        $query = $articles->find();
+        $this->assertSame(Connection::ROLE_WRITE, $query->getConnectionRole(), 'This test assumes select queries still default to write role');
+
+        $results = $query->contain('UniqueItems')->useReadRole()->toArray();
+        $this->assertCount(1, $results);
+        $this->assertCount(1, $results[0]->unique_items);
+        $this->assertSame(1, $results[0]->unique_items[0]->id);
+
+        $logs = Log::engine('queries')->read();
+        $this->assertNotEmpty($logs);
+
+        foreach ($logs as $log) {
+            if (
+                str_contains($log, 'FROM articles') ||
+                str_contains($log, 'FROM unique_items')
+            ) {
+                $this->assertStringContainsString('role=read', $log);
+            }
+        }
+    }
 }

+ 2 - 0
tests/TestCase/ORM/Query/SelectQueryTest.php

@@ -17,6 +17,7 @@ declare(strict_types=1);
 namespace Cake\Test\TestCase\ORM\Query;
 
 use Cake\Cache\Engine\FileEngine;
+use Cake\Database\Connection;
 use Cake\Database\Driver\Mysql;
 use Cake\Database\Driver\Sqlite;
 use Cake\Database\DriverFeatureEnum;
@@ -2603,6 +2604,7 @@ class SelectQueryTest extends TestCase
             '(help)' => 'This is a Query object, to get the results execute or iterate it.',
             'sql' => $query->sql(),
             'params' => $query->getValueBinder()->bindings(),
+            'role' => Connection::ROLE_WRITE,
             'defaultTypes' => [
                 'authors__id' => 'integer',
                 'authors.id' => 'integer',

+ 19 - 0
tests/TestCase/ORM/TableTest.php

@@ -1288,6 +1288,25 @@ class TableTest extends TestCase
         $this->assertSame(2, $author->id);
     }
 
+    public function testFindTypedParameterCompatibility(): void
+    {
+        $articles = $this->fetchTable('Articles');
+        $article = $articles->find('titled')->first();
+        $this->assertNotEmpty($article);
+
+        // Options arrays are deprecated but should work
+        $this->deprecated(function () use ($articles) {
+            $article = $articles->find('titled', ['title' => 'Second Article'])->first();
+            $this->assertNotEmpty($article);
+            $this->assertEquals('Second Article', $article->title);
+        });
+
+        // Named parameters should be compatible with options finders
+        $article = $articles->find('titled', title: 'Second Article')->first();
+        $this->assertNotEmpty($article);
+        $this->assertEquals('Second Article', $article->title);
+    }
+
     public function testFindForFinderVariadic(): void
     {
         $testTable = $this->fetchTable('Test');

+ 12 - 0
tests/test_app/TestApp/Model/Table/ArticlesTable.php

@@ -58,6 +58,18 @@ class ArticlesTable extends Table
     }
 
     /**
+     * Finder for testing named parameter compatibility
+     */
+    public function findTitled(SelectQuery $query, array $options): SelectQuery
+    {
+        if (!empty($options['title'])) {
+            $query->where(['Articles.title' => $options['title']]);
+        }
+
+        return $query;
+    }
+
+    /**
      * Example public method
      */
     public function doSomething(): void