Fix bug in Date::addPeriod(N, 'month') where max number of days in a month was not respected. So if subtracting one month from July 31, result would be July 1. Now result is June 30th. Note: there appears to be little to no change in speed, tested both in integration tests and the code by itself.

3 files changed, 82 insertions, 6 deletions

diff --git a/core/Date.php b/core/Date.php
index 7af59e1f2a..c5b4cd7af6 100644
--- a/core/Date.php
+++ b/core/Date.php
@@ -41,6 +41,26 @@ class Date
     const DATE_TIME_FORMAT = 'Y-m-d H:i:s';
 
     /**
+     * Max days for months (non-leap-year). See {@link addPeriod()} implementation.
+     *
+     * @var int[]
+     */
+    private static $maxDaysInMonth = array(
+        '1' => 31,
+        '2' => 28,
+        '3' => 31,
+        '4' => 30,
+        '5' => 31,
+        '6' => 30,
+        '7' => 31,
+        '8' => 31,
+        '9' => 30,
+        '10' => 31,
+        '11' => 30,
+        '12' => 31
+    );
+
+    /**
      * The stored timestamp is always UTC based.
      * The returned timestamp via getTimestamp() will have the conversion applied
      * @var int|null
@@ -673,14 +693,41 @@ class Date
      */
     public function addPeriod($n, $period)
     {
-        if ($n < 0) {
-            $ts = strtotime("$n $period", $this->timestamp);
+        if (strtolower($period) == 'month') { // TODO: comments
+            $dateInfo = getdate($this->timestamp);
+
+            $ts = mktime(
+                $dateInfo['hours'],
+                $dateInfo['minutes'],
+                $dateInfo['seconds'],
+                $dateInfo['mon'] + (int)$n,
+                1,
+                $dateInfo['year']
+            );
+
+            $daysToAdd = min($dateInfo['mday'], self::getMaxDaysInMonth($ts)) - 1;
+            $ts += self::NUM_SECONDS_IN_DAY * $daysToAdd;
         } else {
-            $ts = strtotime("+$n $period", $this->timestamp);
+            $time = $n < 0 ? "$n $period" : "+$n $period";
+
+            $ts = strtotime($time, $this->timestamp);
         }
+
         return new Date($ts, $this->timezone);
     }
 
+    private static function getMaxDaysInMonth($timestamp)
+    {
+        $month = (int)date('m', $timestamp);
+        if (date('L', $timestamp) == 1
+            && $month == 2
+        ) {
+            return 29;
+        } else {
+            return self::$maxDaysInMonth[$month];
+        }
+    }
+
     /**
      * Subtracts a period from `$this` date and returns the result in a new Date instance.
      *
diff --git a/tests/PHPUnit/Core/DateTest.php b/tests/PHPUnit/Core/DateTest.php
index 2193d10674..84041f53fd 100644
--- a/tests/PHPUnit/Core/DateTest.php
+++ b/tests/PHPUnit/Core/DateTest.php
@@ -8,6 +8,9 @@
 use Piwik\Date;
 use Piwik\SettingsServer;
 
+/**
+ * @group Core_DateTest
+ */
 class DateTest extends PHPUnit_Framework_TestCase
 {
     /**
@@ -241,6 +244,28 @@ class DateTest extends PHPUnit_Framework_TestCase
     /**
      * @group Core
      */
+    public function testAddPeriodMonthRespectsMaxDaysInMonth()
+    {
+        $date = Date::factory('2014-07-31');
+        $dateExpected = Date::factory('2014-06-30');
+        $dateActual = $date->subPeriod(1, 'month');
+        $this->assertEquals($dateExpected->toString(), $dateActual->toString());
+
+        // test leap year
+        $date = Date::factory('2000-03-31');
+        $dateExpected = Date::factory('2000-02-29');
+        $dateActual = $date->subPeriod(1, 'month');
+        $this->assertEquals($dateExpected->toString(), $dateActual->toString());
+
+        $date = Date::factory('2000-01-31');
+        $dateExpected = Date::factory('2000-02-29');
+        $dateActual = $date->addPeriod(1, 'month');
+        $this->assertEquals($dateExpected->toString(), $dateActual->toString());
+    }
+
+    /**
+     * @group Core
+     */
     public function testIsLeapYear()
     {
         $date = Date::factory('2011-03-01');
@@ -260,8 +285,9 @@ class DateTest extends PHPUnit_Framework_TestCase
         $date = Date::factory('2013-12-31');
         $this->assertFalse($date->isLeapYear());
 
-        $date = Date::factory('2052-01-01'); // dates after 19/01/2038 03:14:07 fail on 32-bit arch
-        $this->assertTrue($date->isLeapYear());
-
+        if (PHP_INT_SIZE > 4) { // dates after 19/01/2038 03:14:07 fail on 32-bit arch
+            $date = Date::factory('2052-01-01');
+            $this->assertTrue($date->isLeapYear());
+        }
     }
 }
diff --git a/tests/PHPUnit/Core/Period/RangeTest.php b/tests/PHPUnit/Core/Period/RangeTest.php
index 380c69383d..b2114a4372 100644
--- a/tests/PHPUnit/Core/Period/RangeTest.php
+++ b/tests/PHPUnit/Core/Period/RangeTest.php
@@ -12,6 +12,9 @@ use Piwik\Period\Week;
 use Piwik\Period\Year;
 use Piwik\Translate;
 
+/**
+ * @group Core_Period_RangeTest
+ */
 class Period_RangeTest extends PHPUnit_Framework_TestCase
 {
     // test range 1