Fixed Criteria::limit and Builder::limit

CHANGELOG.md

@@ -6,6 +6,7 @@
 - Fixed `Phalcon\Mvc\Model\Resultset\Simple` to save snapshot when caching
 - Fixed `Phalcon\Http\Request::getHeaders` to handle auth headers correctly [#12480](https://github.com/phalcon/cphalcon/issues/12480)
 - Fixed `Phalcon\Http\Request::getMethod` to handle `X-HTTP-Method-Override` header correctly [#12478](https://github.com/phalcon/cphalcon/issues/12478)
+- Fixed `Phalcon\Mvc\Model\Criteria::limit` and `Phalcon\Mvc\Model\Query\Builder::limit` to work with `limit` and `offset` properly [#12419](https://github.com/phalcon/cphalcon/issues/12419)
 
 # [3.0.2](https://github.com/phalcon/cphalcon/releases/tag/v3.0.2) (2016-11-26)
 - Fixed saving snapshot data while caching model [#12170](https://github.com/phalcon/cphalcon/issues/12170), [#12000](https://github.com/phalcon/cphalcon/issues/12000)

phalcon/mvc/model/criteria.zep

@@ -470,10 +470,9 @@ class Criteria implements CriteriaInterface, InjectionAwareInterface
 	/**
 	 * Adds the order-by parameter to the criteria (deprecated)
 	 *
-	 * @deprecated 1.2.1
 	 * @see \Phalcon\Mvc\Model\Criteria::orderBy()
 	 */
-	public function order(string! orderColumns) -> <Criteria>
+	deprecated public function order(string! orderColumns) -> <Criteria>
 	{
 		let this->_params["order"] = orderColumns;
 		return this;
@@ -507,14 +506,27 @@ class Criteria implements CriteriaInterface, InjectionAwareInterface
 	}
 
 	/**
-	 * Adds the limit parameter to the criteria
+	 * Adds the limit parameter to the criteria.
+	 *
+	 * <code>
+	 * $criteria->limit(100);
+	 * $criteria->limit(100, 200);
+	 * $criteria->limit("100", "200");
+	 * </code>
 	 */
-	public function limit(var limit, var offset = null) -> <Criteria>
+	public function limit(int limit, var offset = null) -> <Criteria>
 	{
-		if typeof offset == "null" {
-			let this->_params["limit"] = limit;
-		} else {
+		let limit = abs(limit);
+
+		if unlikely limit == 0 {
+			return this;
+		}
+
+		if is_numeric(offset) {
+			let offset = abs((int) offset);
 			let this->_params["limit"] = ["number": limit, "offset": offset];
+		} else {
+			let this->_params["limit"] = limit;
 		}
 
 		return this;

phalcon/mvc/model/query/builder.zep

@@ -2,7 +2,7 @@
  +------------------------------------------------------------------------+
  | Phalcon Framework                                                      |
  +------------------------------------------------------------------------+
- | Copyright (c) 2011-2016 Phalcon Team (https://phalconphp.com)       |
+ | Copyright (c) 2011-2016 Phalcon Team (https://phalconphp.com)          |
  +------------------------------------------------------------------------+
  | This source file is subject to the New BSD License that is bundled     |
  | with this package in the file docs/LICENSE.txt.                        |
@@ -933,17 +933,26 @@ class Builder implements BuilderInterface, InjectionAwareInterface
 	/**
 	 * Sets a LIMIT clause, optionally an offset clause
 	 *
-	 *<code>
+	 * <code>
 	 * $builder->limit(100);
 	 * $builder->limit(100, 20);
-	 *</code>
+	 * $builder->limit("100", "20");
+	 * </code>
 	 */
-	public function limit(var limit = null, var offset = null) -> <Builder>
+	public function limit(int limit, var offset = null) -> <Builder>
 	{
+		let limit = abs(limit);
+
+		if unlikely limit == 0 {
+			return this;
+		}
+
 		let this->_limit = limit;
+
 		if is_numeric(offset) {
-			let this->_offset = (int)offset;
+			let this->_offset = abs((int) offset);
 		}
+
 		return this;
 	}
 

phalcon/mvc/model/query/builderinterface.zep

@@ -220,7 +220,7 @@ interface BuilderInterface
 	 * @param int offset
 	 * @return \Phalcon\Mvc\Model\Query\BuilderInterface
 	 */
-	public function limit(limit, offset = null);
+	public function limit(int limit, offset = null);
 
 	/**
 	 * Returns the current LIMIT clause

tests/unit/Mvc/Model/CriteriaTest.php

@@ -51,11 +51,12 @@ protected function _before()
     /**
      * Tests Criteria::inWhere with empty array.
      *
+     * @test
      * @issue  10676
      * @author Serghei Iakovlev <serghei@phalconphp.com>
      * @since  2016-08-11
      */
-    public function testShouldExecuteInWhereQueryWithEmptyArray()
+    public function shouldExecuteInWhereQueryWithEmptyArray()
     {
         $this->specify(
             'The Criteria::inWhere with empty array does not work as expected',
@@ -67,4 +68,42 @@ function () {
             }
         );
     }
+
+    /**
+     * Tests work with limit / offset
+     *
+     * @test
+     * @issue  12419
+     * @author Serghei Iakovelv <serghei@phalconphp.com>
+     * @since  2016-12-18
+     */
+    public function shouldCorrectHandleLimitAndOffset()
+    {
+        $this->specify(
+            'The criteria object works with limit / offset incorrectly',
+            function ($limit, $offset, $expected) {
+                /** @var \Phalcon\Mvc\Model\Criteria $query */
+                $query = Users::query();
+
+                $query->limit($limit, $offset);
+
+                expect($query->getLimit())->equals($expected);
+            },
+            ['examples' => $this->limitOffsetProvider()]
+        );
+    }
+
+    protected function limitOffsetProvider()
+    {
+        return [
+            [-7,      null,  7                                  ],
+            ["-7234", null,  7234                               ],
+            ["18",    null,  18                                 ],
+            ["18",    2,     ['number' => 18, 'offset' => 2]    ],
+            ["-1000", -200,  ['number' => 1000, 'offset' => 200]],
+            ["1000", "-200", ['number' => 1000, 'offset' => 200]],
+            ["0",    "-200", null                               ],
+            ["%3CMETA%20HTTP-EQUIV%3D%22refresh%22%20CONT ENT%3D%220%3Burl%3Djavascript%3Aqss%3D7%22%3E", 50, null],
+        ];
+    }
 }

tests/unit/Mvc/Model/Query/BuilderTest.php

@@ -542,4 +542,45 @@ function () {
             }
         );
     }
+
+    /**
+     * Tests work with limit / offset
+     *
+     * @test
+     * @issue  12419
+     * @author Serghei Iakovelv <serghei@phalconphp.com>
+     * @since  2016-12-18
+     */
+    public function shouldCorrectHandleLimitAndOffset()
+    {
+        $this->specify(
+            'The builder object works with limit / offset incorrectly',
+            function ($limit, $offset, $expected) {
+                $builder = new Builder(null, $this->di);
+                $phql = $builder
+                    ->columns(['name'])
+                    ->from(Robots::class)
+                    ->limit($limit, $offset)
+                    ->getPhql();
+
+                /** Just prevent IDE to highlight this as not valid SQL dialect */
+                expect($phql)->equals('SELECT name ' . "FROM {$expected}");
+            },
+            ['examples' => $this->limitOffsetProvider()]
+        );
+    }
+
+    protected function limitOffsetProvider()
+    {
+        return [
+            [-7,      null,  "[Phalcon\\Test\\Models\\Robots] LIMIT :APL0:"              ],
+            ["-7234", null,  "[Phalcon\\Test\\Models\\Robots] LIMIT :APL0:"              ],
+            ["18",    null,  "[Phalcon\\Test\\Models\\Robots] LIMIT :APL0:"              ],
+            ["18",    2,     "[Phalcon\\Test\\Models\\Robots] LIMIT :APL0: OFFSET :APL1:"],
+            ["-1000", -200,  "[Phalcon\\Test\\Models\\Robots] LIMIT :APL0: OFFSET :APL1:"],
+            ["1000", "-200", "[Phalcon\\Test\\Models\\Robots] LIMIT :APL0: OFFSET :APL1:"],
+            ["0",    "-200", "[Phalcon\\Test\\Models\\Robots]"                           ],
+            ["%3CMETA%20HTTP-EQUIV%3D%22refresh%22%20CONT ENT%3D%220%3Burl%3Djavascript%3Aqss%3D7%22%3E", 50, "[Phalcon\\Test\\Models\\Robots]"],
+        ];
+    }
 }

tests/unit/Mvc/Model/QueryTest.php

@@ -50,9 +50,6 @@ protected function _before()
         $this->di = $app->getDI();
     }
 
-    /**
-     * @medium
-     */
     public function testSelectParsing()
     {
         $this->specify(
@@ -6923,8 +6920,6 @@ function ($phql, $expected) {
         );
     }
 
-
-
     public function testInsertParsing()
     {
         $this->specify(