Browse Source

Merge branch 'master' into 4.next

Corey Taylor 5 years ago
parent
commit
178ffc879f

+ 5 - 4
.github/workflows/ci.yml

@@ -30,6 +30,10 @@ jobs:
         image: redis
         ports:
           - 6379/tcp
+      memcached:
+        image: memcached
+        ports:
+          - 11211/tcp
 
     steps:
     - name: Setup MySQL latest
@@ -82,10 +86,6 @@ jobs:
 
     - name: Install packages
       run: |
-        # memcached installation fails without updating packages.
-        sudo apt update
-        sudo apt install memcached
-
         sudo locale-gen da_DK.UTF-8
         sudo locale-gen de_DE.UTF-8
 
@@ -104,6 +104,7 @@ jobs:
     - name: Run PHPUnit
       env:
         REDIS_PORT: ${{ job.services.redis.ports['6379'] }}
+        MEMCACHED_PORT: ${{ job.services.memcached.ports['11211'] }}
       run: |
         if [[ ${{ matrix.db-type }} == 'sqlite' ]]; then export DB_URL='sqlite:///:memory:'; fi
         if [[ ${{ matrix.db-type }} == 'mysql' && ${{ matrix.php-version }} != '7.2' ]]; then export DB_URL='mysql://root:root@127.0.0.1/cakephp'; fi

+ 4 - 3
src/Database/Driver/Sqlserver.php

@@ -19,6 +19,7 @@ namespace Cake\Database\Driver;
 use Cake\Database\Driver;
 use Cake\Database\Expression\FunctionExpression;
 use Cake\Database\Expression\OrderByExpression;
+use Cake\Database\Expression\OrderClauseExpression;
 use Cake\Database\Expression\TupleComparison;
 use Cake\Database\Expression\UnaryExpression;
 use Cake\Database\ExpressionInterface;
@@ -29,7 +30,6 @@ use Cake\Database\Schema\SqlserverSchemaDialect;
 use Cake\Database\SqlserverCompiler;
 use Cake\Database\Statement\SqlserverStatement;
 use Cake\Database\StatementInterface;
-use Cake\Database\ValueBinder;
 use InvalidArgumentException;
 use PDO;
 
@@ -349,9 +349,10 @@ class Sqlserver extends Driver
                         isset($select[$orderBy]) &&
                         $select[$orderBy] instanceof ExpressionInterface
                     ) {
-                        $key = $select[$orderBy]->sql(new ValueBinder());
+                        $order->add(new OrderClauseExpression($select[$orderBy], $direction));
+                    } else {
+                        $order->add([$key => $direction]);
                     }
-                    $order->add([$key => $direction]);
 
                     // Leave original order clause unchanged.
                     return $orderBy;

+ 7 - 3
src/Database/Expression/OrderByExpression.php

@@ -72,9 +72,13 @@ class OrderByExpression extends QueryExpression
                 !in_array(strtoupper($val), ['ASC', 'DESC'], true)
             ) {
                 throw new RuntimeException(
-                    'Passing extra expressions by associative array is not ' .
-                    'allowed to avoid potential SQL injection. ' .
-                    'Use QueryExpression or numeric array instead.'
+                    sprintf(
+                        'Passing extra expressions by associative array (`\'%s\' => \'%s\'`) ' .
+                        'is not allowed to avoid potential SQL injection. ' .
+                        'Use QueryExpression or numeric array instead.',
+                        $key,
+                        $val
+                    )
                 );
             }
         }

+ 4 - 1
src/Database/Expression/OrderClauseExpression.php

@@ -17,6 +17,7 @@ declare(strict_types=1);
 namespace Cake\Database\Expression;
 
 use Cake\Database\ExpressionInterface;
+use Cake\Database\Query;
 use Cake\Database\ValueBinder;
 use Closure;
 
@@ -53,7 +54,9 @@ class OrderClauseExpression implements ExpressionInterface, FieldInterface
     {
         /** @var string|\Cake\Database\ExpressionInterface $field */
         $field = $this->_field;
-        if ($field instanceof ExpressionInterface) {
+        if ($field instanceof Query) {
+            $field = sprintf('(%s)', $field->sql($binder));
+        } elseif ($field instanceof ExpressionInterface) {
             $field = $field->sql($binder);
         }
 

+ 4 - 2
src/Database/Expression/QueryExpression.php

@@ -738,7 +738,7 @@ class QueryExpression implements ExpressionInterface, Countable
             [$expression, $operator] = $parts;
             $operator = strtolower(trim($operator));
         }
-        $type = $this->getTypeMap() ->type($expression);
+        $type = $this->getTypeMap()->type($expression);
 
         $typeMultiple = (is_string($type) && strpos($type, '[]') !== false);
         if (in_array($operator, ['in', 'not in']) || $typeMultiple) {
@@ -780,7 +780,9 @@ class QueryExpression implements ExpressionInterface, Countable
         }
 
         if ($value === null && $this->_conjunction !== ',') {
-            throw new InvalidArgumentException('Expression is missing operator (IS, IS NOT) with `null` value.');
+            throw new InvalidArgumentException(
+                sprintf('Expression `%s` is missing operator (IS, IS NOT) with `null` value.', $expression)
+            );
         }
 
         return new ComparisonExpression($expression, $value, $type, $operator);

+ 14 - 0
src/Http/README.md

@@ -41,6 +41,9 @@ namespace App;
 
 use Cake\Core\HttpApplicationInterface;
 use Cake\Http\MiddlewareQueue;
+use Cake\Http\Response;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
 
 class Application implements HttpApplicationInterface
 {
@@ -66,6 +69,17 @@ class Application implements HttpApplicationInterface
         // Add middleware for your application.
         return $middlewareQueue;
     }
+
+    /**
+     * Handle incoming server request and return a response.
+     *
+     * @param \Psr\Http\Message\ServerRequestInterface $request The request
+     * @return \Psr\Http\Message\ResponseInterface
+     */
+    public function handle(ServerRequestInterface $request): ResponseInterface
+    {
+        return new Response(['body'=>'Hello World!']);
+    }
 }
 ```
 

+ 4 - 0
src/ORM/Table.php

@@ -125,6 +125,8 @@ use RuntimeException;
  * lifecycle methods below:
  *
  * - `beforeFind(EventInterface $event, Query $query, ArrayObject $options, boolean $primary)`
+ * - `beforeMarshal(EventInterface $event, ArrayObject $data, ArrayObject $options)`
+ * - `afterMarshal(EventInterface $event, EntityInterface $entity, ArrayObject $options)`
  * - `buildValidator(EventInterface $event, Validator $validator, string $name)`
  * - `buildRules(RulesChecker $rules)`
  * - `beforeRules(EventInterface $event, EntityInterface $entity, ArrayObject $options, string $operation)`
@@ -134,8 +136,10 @@ use RuntimeException;
  * - `afterSaveCommit(EventInterface $event, EntityInterface $entity, ArrayObject $options)`
  * - `beforeDelete(EventInterface $event, EntityInterface $entity, ArrayObject $options)`
  * - `afterDelete(EventInterface $event, EntityInterface $entity, ArrayObject $options)`
+ * - `afterDeleteCommit(EventInterface $event, EntityInterface $entity, ArrayObject $options)`
  *
  * @see \Cake\Event\EventManager for reference on the events system.
+ * @link https://book.cakephp.org/4/en/orm/table-objects.html#event-list
  */
 class Table implements RepositoryInterface, EventListenerInterface, EventDispatcherInterface, ValidatorAwareInterface
 {

+ 1 - 1
src/TestSuite/StringCompareTrait.php

@@ -64,6 +64,6 @@ trait StringCompareTrait
         }
 
         $expected = file_get_contents($path);
-        $this->assertTextEquals($expected, $result);
+        $this->assertTextEquals($expected, $result, 'Content does not match file ' . $path);
     }
 }

+ 63 - 0
tests/TestCase/Database/Driver/SqlserverTest.php

@@ -368,6 +368,69 @@ class SqlserverTest extends TestCase
             'FROM articles WHERE title = :c0) _cake_paging_ ' .
             'WHERE (_cake_paging_._cake_page_rownum_ > 50 AND _cake_paging_._cake_page_rownum_ <= 60)';
         $this->assertSame($expected, $query->sql());
+
+        $query = new Query($connection);
+        $subquery = new Query($connection);
+        $subquery->select(1);
+        $query
+            ->select([
+                'id',
+                'computed' => $subquery,
+            ])
+            ->from('articles')
+            ->order([
+                'computed' => 'ASC',
+            ])
+            ->offset(10);
+        $expected =
+            'SELECT * FROM (' .
+                'SELECT id, (SELECT 1) AS computed, ' .
+                '(ROW_NUMBER() OVER (ORDER BY (SELECT 1) ASC)) AS _cake_page_rownum_ FROM articles' .
+            ') _cake_paging_ ' .
+            'WHERE _cake_paging_._cake_page_rownum_ > 10';
+        $this->assertSame($expected, $query->sql());
+
+        $subqueryA = new Query($connection);
+        $subqueryA
+            ->select('count(*)')
+            ->from(['a' => 'articles'])
+            ->where([
+                'a.id = articles.id',
+                'a.published' => 'Y',
+            ]);
+
+        $subqueryB = new Query($connection);
+        $subqueryB
+            ->select('count(*)')
+            ->from(['b' => 'articles'])
+            ->where([
+                'b.id = articles.id',
+                'b.published' => 'N',
+            ]);
+
+        $query = new Query($connection);
+        $query
+            ->select([
+                'id',
+                'computedA' => $subqueryA,
+                'computedB' => $subqueryB,
+            ])
+            ->from('articles')
+            ->order([
+                'computedA' => 'ASC',
+            ])
+            ->offset(10);
+
+        $this->assertSame(
+            'SELECT * FROM (' .
+                'SELECT id, ' .
+                '(SELECT count(*) FROM articles a WHERE (a.id = articles.id AND a.published = :c0)) AS computedA, ' .
+                '(SELECT count(*) FROM articles b WHERE (b.id = articles.id AND b.published = :c1)) AS computedB, ' .
+                '(ROW_NUMBER() OVER (ORDER BY (SELECT count(*) FROM articles a WHERE (a.id = articles.id AND a.published = :c2)) ASC)) AS _cake_page_rownum_ FROM articles' .
+            ') _cake_paging_ ' .
+            'WHERE _cake_paging_._cake_page_rownum_ > 10',
+            $query->sql()
+        );
     }
 
     /**

+ 177 - 3
tests/TestCase/Database/QueryTest.php

@@ -1870,8 +1870,8 @@ class QueryTest extends TestCase
     {
         $this->expectException('RuntimeException');
         $this->expectExceptionMessage(
-            'Passing extra expressions by associative array is not ' .
-            'allowed to avoid potential SQL injection. ' .
+            'Passing extra expressions by associative array (`\'id\' => \'desc -- Comment\'`) ' .
+            'is not allowed to avoid potential SQL injection. ' .
             'Use QueryExpression or numeric array instead.'
         );
 
@@ -4211,7 +4211,7 @@ class QueryTest extends TestCase
     public function testIsNullInvalid()
     {
         $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionMessage('Expression is missing operator (IS, IS NOT) with `null` value.');
+        $this->expectExceptionMessage('Expression `name` is missing operator (IS, IS NOT) with `null` value.');
 
         $this->loadFixtures('Authors');
         (new Query($this->connection))
@@ -4998,6 +4998,180 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Tests that query expressions can be used for ordering.
+     *
+     * @return void
+     */
+    public function testOrderBySubquery()
+    {
+        $this->autoQuote = true;
+        $this->connection->getDriver()->enableAutoQuoting($this->autoQuote);
+
+        $this->loadFixtures('Articles');
+        $connection = $this->connection;
+
+        $query = new Query($connection);
+
+        $stmt = $connection->update('articles', ['published' => 'N'], ['id' => 3]);
+        $stmt->closeCursor();
+
+        $subquery = new Query($connection);
+        $subquery
+            ->select(
+                $subquery->newExpr()->addCase(
+                    [$subquery->newExpr()->add(['a.published' => 'N'])],
+                    [1, 0],
+                    ['integer', 'integer']
+                )
+            )
+            ->from(['a' => 'articles'])
+            ->where([
+                'a.id = articles.id',
+            ]);
+
+        $query
+            ->select(['id'])
+            ->from('articles')
+            ->orderDesc($subquery)
+            ->orderAsc('id')
+            ->setSelectTypeMap(new TypeMap([
+                'id' => 'integer',
+            ]));
+
+        $this->assertQuotedQuery(
+            'SELECT <id> FROM <articles> ORDER BY \(' .
+                'SELECT \(CASE WHEN <a>\.<published> = \:c0 THEN \:param1 ELSE \:param2 END\) ' .
+                'FROM <articles> <a> ' .
+                'WHERE a\.id = articles\.id' .
+            '\) DESC, <id> ASC',
+            $query->sql(),
+            !$this->autoQuote
+        );
+
+        $this->assertEquals(
+            [
+                [
+                    'id' => 3,
+                ],
+                [
+                    'id' => 1,
+                ],
+                [
+                    'id' => 2,
+                ],
+            ],
+            $query->execute()->fetchAll('assoc')
+        );
+    }
+
+    /**
+     * Test that reusing expressions will duplicate bindings and run successfully.
+     *
+     * This replicates what the SQL Server driver would do for <= SQL Server 2008
+     * when ordering on fields that are expressions.
+     *
+     * @return void
+     * @see \Cake\Database\Driver\Sqlserver::_pagingSubquery()
+     */
+    public function testReusingExpressions()
+    {
+        $this->loadFixtures('Articles');
+        $connection = $this->connection;
+
+        $query = new Query($connection);
+
+        $stmt = $connection->update('articles', ['published' => 'N'], ['id' => 3]);
+        $stmt->closeCursor();
+
+        $subqueryA = new Query($connection);
+        $subqueryA
+            ->select('count(*)')
+            ->from(['a' => 'articles'])
+            ->where([
+                'a.id = articles.id',
+                'a.published' => 'Y',
+            ]);
+
+        $subqueryB = new Query($connection);
+        $subqueryB
+            ->select('count(*)')
+            ->from(['b' => 'articles'])
+            ->where([
+                'b.id = articles.id',
+                'b.published' => 'N',
+            ]);
+
+        $query
+            ->select([
+                'id',
+                'computedA' => $subqueryA,
+                'computedB' => $subqueryB,
+            ])
+            ->from('articles')
+            ->orderDesc($subqueryB)
+            ->orderAsc('id')
+            ->setSelectTypeMap(new TypeMap([
+                'id' => 'integer',
+                'computedA' => 'integer',
+                'computedB' => 'integer',
+            ]));
+
+        $this->assertQuotedQuery(
+            'SELECT <id>, ' .
+                '\(SELECT count\(\*\) FROM <articles> <a> WHERE \(a\.id = articles\.id AND <a>\.<published> = :c0\)\) AS <computedA>, ' .
+                '\(SELECT count\(\*\) FROM <articles> <b> WHERE \(b\.id = articles\.id AND <b>\.<published> = :c1\)\) AS <computedB> ' .
+            'FROM <articles> ' .
+            'ORDER BY \(' .
+                'SELECT count\(\*\) FROM <articles> <b> WHERE \(b\.id = articles\.id AND <b>\.<published> = :c2\)' .
+            '\) DESC, <id> ASC',
+            $query->sql(),
+            !$this->autoQuote
+        );
+
+        $this->assertSame(
+            [
+                [
+                    'id' => 3,
+                    'computedA' => 0,
+                    'computedB' => 1,
+                ],
+                [
+                    'id' => 1,
+                    'computedA' => 1,
+                    'computedB' => 0,
+                ],
+                [
+                    'id' => 2,
+                    'computedA' => 1,
+                    'computedB' => 0,
+                ],
+            ],
+            $query->execute()->fetchAll('assoc')
+        );
+
+        $this->assertSame(
+            [
+                ':c0' => [
+                    'value' => 'Y',
+                    'type' => null,
+                    'placeholder' => 'c0',
+                ],
+                ':c1' => [
+                    'value' => 'N',
+                    'type' => null,
+                    'placeholder' => 'c1',
+                ],
+                ':c2' => [
+                    'value' => 'N',
+                    'type' => null,
+                    'placeholder' => 'c2',
+                ],
+            ],
+            $query->getValueBinder()->bindings()
+        );
+    }
+
+    /**
      * Tests creating StringExpression.
      *
      * @return void

+ 1 - 1
tests/TestCase/ORM/Behavior/TranslateBehaviorShadowTableTest.php

@@ -669,7 +669,7 @@ class TranslateBehaviorShadowTableTest extends TranslateBehaviorTest
      *
      * The parent test expects description translations in only some of the records
      * that's incompatible with the shadow-translate behavior, since the schema
-     * dictates what fields to expect to be translated and doesnt permit any EAV
+     * dictates what fields to expect to be translated and doesn't permit any EAV
      * style translations
      *
      * @return void

+ 17 - 0
tests/TestCase/TestSuite/IntegrationTestTraitTest.php

@@ -721,6 +721,21 @@ class IntegrationTestTraitTest extends TestCase
     }
 
     /**
+     * Test flash assertions stored with enableRememberFlashMessages() after a
+     * redirect.
+     *
+     * @return void
+     */
+    public function testFlashAssertionsAfterRedirect()
+    {
+        $this->get('/posts/someRedirect');
+
+        $this->assertResponseCode(302);
+
+        $this->assertSession('An error message', 'Flash.flash.0.message');
+    }
+
+    /**
      * Test flash assertions stored with enableRememberFlashMessages() after they
      * are rendered
      *
@@ -731,6 +746,8 @@ class IntegrationTestTraitTest extends TestCase
         $this->enableRetainFlashMessages();
         $this->get('/posts/index/with_flash');
 
+        $this->assertResponseCode(200);
+
         $this->assertSession('An error message', 'Flash.flash.0.message');
     }
 

+ 14 - 2
tests/test_app/TestApp/Controller/PostsController.php

@@ -24,6 +24,9 @@ use Cake\Http\Cookie\Cookie;
  */
 class PostsController extends AppController
 {
+    /**
+     * @return void
+     */
     public function initialize(): void
     {
         $this->loadComponent('Flash');
@@ -32,8 +35,7 @@ class PostsController extends AppController
     }
 
     /**
-     * beforeFilter
-     *
+     * @param \Cake\Event\EventInterface $event
      * @return \Cake\Http\Response|null|void
      */
     public function beforeFilter(EventInterface $event)
@@ -60,6 +62,16 @@ class PostsController extends AppController
     }
 
     /**
+     * @return \Cake\Http\Response|null
+     */
+    public function someRedirect()
+    {
+        $this->Flash->error('An error message');
+
+        return $this->redirect('/somewhere');
+    }
+
+    /**
      * Sets a flash message and redirects (no rendering)
      *
      * @return \Cake\Http\Response