Browse Source

Merge branch 'master' into 3.next

ADmad 9 years ago
parent
commit
c3549daa4b

+ 37 - 11
src/Database/SqlDialectTrait.php

@@ -178,6 +178,43 @@ trait SqlDialectTrait
         if (!$hadAlias) {
             return $query;
         }
+
+        return $this->_removeAliasesFromConditions($query);
+    }
+
+    /**
+     * Apply translation steps to update queries.
+     *
+     * Chops out aliases on update query conditions as not all database dialects do support
+     * aliases in update queries.
+     *
+     * Just like for delete queries, joins are currently not supported for update queries.
+     *
+     * @param \Cake\Database\Query $query The query to translate
+     * @return \Cake\Database\Query The modified query
+     */
+    protected function _updateQueryTranslator($query)
+    {
+        return $this->_removeAliasesFromConditions($query);
+    }
+
+    /**
+     * Removes aliases from the `WHERE` clause of a query.
+     *
+     * @param \Cake\Database\Query $query The query to process.
+     * @return \Cake\Database\Query The modified query.
+     * @throws \RuntimeException In case the processed query contains any joins, as removing
+     *  aliases from the conditions can break references to the joined tables.
+     */
+    protected function _removeAliasesFromConditions($query)
+    {
+        if ($query->clause('join')) {
+            throw new \RuntimeException(
+                'Aliases are being removed from conditions for UPDATE/DELETE queries, ' .
+                'this can break references to joined tables.'
+            );
+        }
+
         $conditions = $query->clause('where');
         if ($conditions) {
             $conditions->traverse(function ($condition) {
@@ -201,17 +238,6 @@ trait SqlDialectTrait
     }
 
     /**
-     * Apply translation steps to update queries.
-     *
-     * @param \Cake\Database\Query $query The query to translate
-     * @return \Cake\Database\Query The modified query
-     */
-    protected function _updateQueryTranslator($query)
-    {
-        return $query;
-    }
-
-    /**
      * Apply translation steps to insert queries.
      *
      * @param \Cake\Database\Query $query The query to translate

+ 2 - 0
src/ORM/Association/HasMany.php

@@ -494,12 +494,14 @@ class HasMany extends Association
                 return $ok;
             }
 
+            $conditions = array_merge($conditions, $this->conditions());
             $target->deleteAll($conditions);
 
             return true;
         }
 
         $updateFields = array_fill_keys($foreignKey, null);
+        $conditions = array_merge($conditions, $this->conditions());
         $target->updateAll($updateFields, $conditions);
 
         return true;

+ 37 - 10
src/View/Helper/TextHelper.php

@@ -117,9 +117,21 @@ class TextHelper extends Helper
         $this->_placeholders = [];
         $options += ['escape' => true];
 
-        $pattern = '#(?<!href="|src="|">)((?:https?|ftp|nntp)://[\p{L}0-9.\-_:]+' .
-            '(?:[^\s()<>]+|\(([^\s()<>]+|(\([^\s()<>]+\)))*\))+' .
-            '(?:\(([^\s()<>]+|(\([^\s()<>]+\)))*\)|[^\s`!()\[\]{};:\'".,<>?«»“”‘’]))#i';
+        $pattern = '/(?:(?<!href="|src="|">)
+            (?>
+                (
+                    (?<left>[\[<(]) # left paren,brace
+                    (?>
+                        # Lax match URL
+                        (?<url>(?:https?|ftp|nntp):\/\/[\p{L}0-9.\-_:]+(?:[\/?][\p{L}0-9.\-_:\/?=&>\[\]()#@]+)?)
+                        (?<right>[\])>]) # right paren,brace
+                    )
+                )
+                |
+                (?<url_bare>(?P>url)) # A bare URL. Use subroutine
+            )
+            )/ixu';
+
         $text = preg_replace_callback(
             $pattern,
             [&$this, '_insertPlaceHolder'],
@@ -146,8 +158,20 @@ class TextHelper extends Helper
      */
     protected function _insertPlaceHolder($matches)
     {
-        $key = md5($matches[0]);
-        $this->_placeholders[$key] = $matches[0];
+        $match = $matches[0];
+        $envelope = ['', ''];
+        if (isset($matches['url'])) {
+            $match = $matches['url'];
+            $envelope = [$matches['left'], $matches['right']];
+        }
+        if (isset($matches['url_bare'])) {
+            $match = $matches['url_bare'];
+        }
+        $key = md5($match);
+        $this->_placeholders[$key] = [
+            'content' => $match,
+            'envelope' => $envelope
+        ];
 
         return $key;
     }
@@ -162,12 +186,13 @@ class TextHelper extends Helper
     protected function _linkUrls($text, $htmlOptions)
     {
         $replace = [];
-        foreach ($this->_placeholders as $hash => $url) {
-            $link = $url;
+        foreach ($this->_placeholders as $hash => $content) {
+            $link = $url = $content['content'];
+            $envelope = $content['envelope'];
             if (!preg_match('#^[a-z]+\://#i', $url)) {
                 $url = 'http://' . $url;
             }
-            $replace[$hash] = $this->Html->link($link, $url, $htmlOptions);
+            $replace[$hash] = $envelope[0] . $this->Html->link($link, $url, $htmlOptions) . $envelope[1];
         }
 
         return strtr($text, $replace);
@@ -184,8 +209,10 @@ class TextHelper extends Helper
     protected function _linkEmails($text, $options)
     {
         $replace = [];
-        foreach ($this->_placeholders as $hash => $url) {
-            $replace[$hash] = $this->Html->link($url, 'mailto:' . $url, $options);
+        foreach ($this->_placeholders as $hash => $content) {
+            $url = $content['content'];
+            $envelope = $content['envelope'];
+            $replace[$hash] = $envelope[0] . $this->Html->link($url, 'mailto:' . $url, $options) . $envelope[1];
         }
 
         return strtr($text, $replace);

+ 85 - 0
tests/TestCase/Database/QueryTest.php

@@ -2671,6 +2671,29 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Tests that delete queries that contain joins do trigger a notice,
+     * warning about possible incompatibilities with aliases being removed
+     * from the conditions.
+     *
+     *
+     * @expectedException \RuntimeException
+     * @expectedExceptionMessage Aliases are being removed from conditions for UPDATE/DELETE queries, this can break references to joined tables.
+     * @return void
+     */
+    public function testDeleteRemovingAliasesCanBreakJoins()
+    {
+        $query = new Query($this->connection);
+
+        $query
+            ->delete('authors')
+            ->from(['a ' => 'authors'])
+            ->innerJoin('articles')
+            ->where(['a.id' => 1]);
+
+        $query->sql();
+    }
+
+    /**
      * Test setting select() & delete() modes.
      *
      * @return void
@@ -2854,6 +2877,68 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Tests that aliases are stripped from update query conditions
+     * where possible.
+     *
+     * @return void
+     */
+    public function testUpdateStripAliasesFromConditions()
+    {
+        $query = new Query($this->connection);
+
+        $query
+            ->update('authors')
+            ->set(['name' => 'name'])
+            ->where([
+                'OR' => [
+                    'a.id' => 1,
+                    'AND' => [
+                        'b.name NOT IN' => ['foo', 'bar'],
+                        'OR' => [
+                            $query->newExpr()->eq(new IdentifierExpression('c.name'), 'zap'),
+                            'd.name' => 'baz',
+                            (new Query($this->connection))->select(['e.name'])->where(['e.name' => 'oof'])
+                        ]
+                    ]
+                ],
+            ]);
+
+        $this->assertQuotedQuery(
+            'UPDATE <authors> SET <name> = :c0 WHERE \(' .
+                '<id> = :c1 OR \(' .
+                    '<name> not in \(:c2,:c3\) AND \(' .
+                        '\(<c>\.<name>\) = :c4 OR <name> = :c5 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c6\)' .
+                    '\)' .
+                '\)' .
+            '\)',
+            $query->sql(),
+            !$this->autoQuote
+        );
+    }
+
+    /**
+     * Tests that update queries that contain joins do trigger a notice,
+     * warning about possible incompatibilities with aliases being removed
+     * from the conditions.
+     *
+     * @expectedException \RuntimeException
+     * @expectedExceptionMessage Aliases are being removed from conditions for UPDATE/DELETE queries, this can break references to joined tables.
+     * @return void
+     */
+    public function testUpdateRemovingAliasesCanBreakJoins()
+    {
+        $query = new Query($this->connection);
+
+        $query
+            ->update('authors')
+            ->set(['name' => 'name'])
+            ->innerJoin('articles')
+            ->where(['a.id' => 1]);
+
+        $query->sql();
+    }
+
+    /**
      * You cannot call values() before insert() it causes all sorts of pain.
      *
      * @expectedException \Cake\Database\Exception

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

@@ -2071,6 +2071,131 @@ class TableTest extends TestCase
     }
 
     /**
+     * Tests that dependent, non-cascading deletes are using the association
+     * conditions for deleting associated records.
+     *
+     * @return void
+     */
+    public function testHasManyNonCascadingUnlinkDeleteUsesAssociationConditions()
+    {
+        $Articles = TableRegistry::get('Articles');
+        $Comments = $Articles->hasMany('Comments', [
+            'dependent' => true,
+            'cascadeCallbacks' => false,
+            'saveStrategy' => HasMany::SAVE_REPLACE,
+            'conditions' => [
+                'Comments.published' => 'Y'
+            ]
+        ]);
+
+        $article = $Articles->newEntity([
+            'title' => 'Title',
+            'body' => 'Body',
+            'comments' => [
+                [
+                    'user_id' => 1,
+                    'comment' => 'First comment',
+                    'published' => 'Y'
+                ],
+                [
+                    'user_id' => 1,
+                    'comment' => 'Second comment',
+                    'published' => 'Y'
+                ]
+            ]
+        ]);
+        $article = $Articles->save($article);
+        $this->assertNotEmpty($article);
+
+        $comment3 = $Comments->target()->newEntity([
+            'article_id' => $article->get('id'),
+            'user_id' => 1,
+            'comment' => 'Third comment',
+            'published' => 'N'
+        ]);
+        $comment3 = $Comments->target()->save($comment3);
+        $this->assertNotEmpty($comment3);
+
+        $this->assertEquals(3, $Comments->target()->find()->where(['Comments.article_id' => $article->get('id')])->count());
+
+        unset($article->comments[1]);
+        $article->dirty('comments', true);
+
+        $article = $Articles->save($article);
+        $this->assertNotEmpty($article);
+
+        // Given the association condition of `'Comments.published' => 'Y'`,
+        // it is expected that only one of the three linked comments are
+        // actually being deleted, as only one of them matches the
+        // association condition.
+        $this->assertEquals(2, $Comments->target()->find()->where(['Comments.article_id' => $article->get('id')])->count());
+    }
+
+    /**
+     * Tests that non-dependent, non-cascading deletes are using the association
+     * conditions for updating associated records.
+     *
+     * @return void
+     */
+    public function testHasManyNonDependentNonCascadingUnlinkUpdateUsesAssociationConditions()
+    {
+        $Authors = TableRegistry::get('Authors');
+        $Authors->associations()->removeAll();
+        $Articles = $Authors->hasMany('Articles', [
+            'dependent' => false,
+            'cascadeCallbacks' => false,
+            'saveStrategy' => HasMany::SAVE_REPLACE,
+            'conditions' => [
+                'Articles.published' => 'Y'
+            ]
+        ]);
+
+        $author = $Authors->newEntity([
+            'name' => 'Name',
+            'articles' => [
+                [
+                    'title' => 'First article',
+                    'body' => 'First article',
+                    'published' => 'Y'
+                ],
+                [
+                    'title' => 'Second article',
+                    'body' => 'Second article',
+                    'published' => 'Y'
+                ]
+            ]
+        ]);
+        $author = $Authors->save($author);
+        $this->assertNotEmpty($author);
+
+        $article3 = $Articles->target()->newEntity([
+            'author_id' => $author->get('id'),
+            'title' => 'Third article',
+            'body' => 'Third article',
+            'published' => 'N'
+        ]);
+        $article3 = $Articles->target()->save($article3);
+        $this->assertNotEmpty($article3);
+
+        $this->assertEquals(3, $Articles->target()->find()->where(['Articles.author_id' => $author->get('id')])->count());
+
+        $article2 = $author->articles[1];
+        unset($author->articles[1]);
+        $author->dirty('articles', true);
+
+        $author = $Authors->save($author);
+        $this->assertNotEmpty($author);
+
+        // Given the association condition of `'Articles.published' => 'Y'`,
+        // it is expected that only one of the three linked articles are
+        // actually being unlinked (nulled), as only one of them matches the
+        // association condition.
+        $this->assertEquals(2, $Articles->target()->find()->where(['Articles.author_id' => $author->get('id')])->count());
+        $this->assertNull($Articles->get($article2->get('id'))->get('author_id'));
+        $this->assertEquals($author->get('id'), $Articles->get($article3->get('id'))->get('author_id'));
+    }
+
+    /**
      * Test that saving a new entity with a Primary Key set does not call exists when checkExisting is false.
      *
      * @group save

+ 4 - 0
tests/TestCase/View/Helper/TextHelperTest.php

@@ -352,6 +352,10 @@ class TextHelperTest extends TestCase
             [
                 "Text with partial www.cakephp.org\r\nwww.cakephp.org urls and CRLF",
                 "Text with partial <a href=\"http://www.cakephp.org\">www.cakephp.org</a>\r\n<a href=\"http://www.cakephp.org\">www.cakephp.org</a> urls and CRLF"
+            ],
+            [
+                'https://nl.wikipedia.org/wiki/Exploit_(computerbeveiliging)',
+                '<a href="https://nl.wikipedia.org/wiki/Exploit_(computerbeveiliging)">https://nl.wikipedia.org/wiki/Exploit_(computerbeveiliging)</a>'
             ]
         ];
     }