Code improvements + added tests for vacation date regexps handling

3 files changed, 146 insertions, 72 deletions

diff --git a/plugins/managesieve/lib/Roundcube/rcube_sieve_vacation.php b/plugins/managesieve/lib/Roundcube/rcube_sieve_vacation.php
index 879c58783..9ba52b03b 100644
--- a/plugins/managesieve/lib/Roundcube/rcube_sieve_vacation.php
+++ b/plugins/managesieve/lib/Roundcube/rcube_sieve_vacation.php
@@ -130,6 +130,16 @@ class rcube_sieve_vacation extends rcube_sieve_engine
             $error = 'managesieve.forbiddenchars';
         }
 
+        // find and remove existing date/regex/true rules
+        foreach ((array) $vacation_tests as $idx => $t) {
+            if (($t['test'] == 'currentdate' && $t['part'] == 'date' && $t['type'] == $type)
+                || ($t['test'] == 'header' && $t['type'] == 'regex' && $t['arg1'] == 'received')
+                || ($t['test'] == 'true')
+            ) {
+                unset($vacation_tests[$idx]);
+            }
+        }
+
         if ($date_extension) {
             foreach (array('date_from', 'date_to') as $var) {
                 $date = $$var;
@@ -142,68 +152,15 @@ class rcube_sieve_vacation extends rcube_sieve_engine
                         'arg'  => $dt->format('Y-m-d'),
                     );
 
-                    // find existing date rule
-                    foreach ((array) $vacation_tests as $idx => $t) {
-                        if ($t['test'] == 'currentdate' && $t['part'] == 'date' && $t['type'] == $type) {
-                            $vacation_tests[$idx] = $test;
-                            continue 2;
-                        }
-                    }
-
                     $vacation_tests[] = $test;
                 }
             }
         }
         else if ($regex_extension) {
-            // Sieve 'date' extension not available, use RegEx based rules instead
-
-            // clear any existing date rules in tests array
-            foreach ((array) $vacation_tests as $idx => $t) {
-                if ($t['test'] == 'header' && $t['type'] == 'regex' && $t['arg1'] == 'received') {
-                    unset($vacation_tests[$idx]);
-                }
-
-                if ($t['test'] == 'true') {
-                    unset($vacation_tests[$idx]);
-                }
-            }
-
-            $vacation_tests = array();
-
             // Add date range rules if range specified
             if ($date_from && $date_to) {
-                $dt_from  = rcube_utils::anytodatetime($date_from);
-                $dt_to    = rcube_utils::anytodatetime($date_to);
-                $interval = $dt_from->diff($dt_to);
-
-                if ($interval->invert || $interval->days > 365) {
-                   $error = 'managesieve.invaliddateformat';
-                }
-
-                $dt_i     = $dt_from;
-                $interval = new DateInterval('P1D');
-                $matchexp = '';
-
-                while (!$dt_i->diff($dt_to)->invert) {
-                    $days     = (int) $dt_i->format('d');
-                    $matchexp .= $days < 10 ? "[ 0]$days" : $days;
-
-                    if ($days == $dt_i->format('t') || $dt_i->diff($dt_to)->days == 0) {
-                        $test = array(
-                            'test' => 'header',
-                            'type' => 'regex',
-                            'arg1' => 'received',
-                            'arg2' => '('.$matchexp.') '.$dt_i->format('M Y')
-                        );
-
-                        $vacation_tests[] = $test;
-                        $matchexp         = '';
-                    }
-                    else {
-                        $matchexp .= '|';
-                    }
-
-                    $dt_i->add($interval);
+                if ($tests = self::build_regexp_tests($date_from, $date_to, $error)) {
+                    $vacation_tests = array_merge($vacation_tests, $tests);
                 }
             }
         }
@@ -359,25 +316,11 @@ class rcube_sieve_vacation extends rcube_sieve_engine
             }
         }
         else if ($regex_extension) {
-            $rx1 = '/^\(([0-9][0-9]).*\)\s([A-Za-z]*)\s([0-9][0-9][0-9][0-9])/';
-            $rx2 = '/^\(.*([0-9][0-9])\)\s([A-Za-z]*)\s([0-9][0-9][0-9][0-9])/';
             // Sieve 'date' extension not available, read start/end from RegEx based rules instead
-            foreach ((array) $this->vacation['tests'] as $test) {
-                if ($test['test'] == 'header' && $test['type'] == 'regex' && $test['arg1'] == 'received') {
-                    $textexp = preg_replace('/\[ ([^\]]*)\]/', '0', $test['arg2']);
-
-                    if (!$date_value['from'] && preg_match($rx1, $textexp, $matches)) {
-                        $date_value['from'] = $matches[1]." ".$matches[2]." ".$matches[3];
-                    }
-
-                    if (preg_match($rx2, $textexp, $matches)) {
-                        $date_value['to'] = $matches[1]." ".$matches[2]." ".$matches[3];
-                    }
-                }
+            if ($date_tests = self::parse_regexp_tests($this->vacation['tests'])) {
+                $date_value['from'] = $this->rc->format_date($date_tests['from'], $date_format, false);
+                $date_value['to']   = $this->rc->format_date($date_tests['to'], $date_format, false);
             }
-
-            $date_value['from'] = $this->rc->format_date($date_value['from'], $date_format, false);
-            $date_value['to']   = $this->rc->format_date($date_value['to'], $date_format, false);
         }
 
         // force domain selection in redirect email input
@@ -452,4 +395,68 @@ class rcube_sieve_vacation extends rcube_sieve_engine
 
         return $out;
     }
+
+    public static function build_regexp_tests($date_from, $date_to, &$error)
+    {
+        $tests    = array();
+        $dt_from  = rcube_utils::anytodatetime($date_from);
+        $dt_to    = rcube_utils::anytodatetime($date_to);
+        $interval = $dt_from->diff($dt_to);
+
+        if ($interval->invert || $interval->days > 365) {
+            $error = 'managesieve.invaliddateformat';
+            return;
+        }
+
+        $dt_i     = $dt_from;
+        $interval = new DateInterval('P1D');
+        $matchexp = '';
+
+        while (!$dt_i->diff($dt_to)->invert) {
+            $days     = (int) $dt_i->format('d');
+            $matchexp .= $days < 10 ? "[ 0]$days" : $days;
+
+            if ($days == $dt_i->format('t') || $dt_i->diff($dt_to)->days == 0) {
+                $test = array(
+                    'test' => 'header',
+                    'type' => 'regex',
+                    'arg1' => 'received',
+                    'arg2' => '('.$matchexp.') '.$dt_i->format('M Y')
+                );
+
+                $tests[]  = $test;
+                $matchexp = '';
+            }
+            else {
+                $matchexp .= '|';
+            }
+
+            $dt_i->add($interval);
+        }
+
+        return $tests;
+    }
+
+    public static function parse_regexp_tests($tests)
+    {
+        $rx_from = '/^\(([0-9]{2}).*\)\s([A-Za-z]+)\s([0-9]{4})/';
+        $rx_to   = '/^\(.*([0-9]{2})\)\s([A-Za-z]+)\s([0-9]{4})/';
+        $result  = array();
+
+        foreach ((array) $tests as $test) {
+            if ($test['test'] == 'header' && $test['type'] == 'regex' && $test['arg1'] == 'received') {
+                $textexp = preg_replace('/\[ ([^\]]*)\]/', '0', $test['arg2']);
+
+                if (!$result['from'] && preg_match($rx_from, $textexp, $matches)) {
+                    $result['from'] = $matches[1]." ".$matches[2]." ".$matches[3];
+                }
+
+                if (preg_match($rx_to, $textexp, $matches)) {
+                    $result['to'] = $matches[1]." ".$matches[2]." ".$matches[3];
+                }
+            }
+        }
+
+        return $result;
+    }
 }
diff --git a/plugins/managesieve/tests/Vacation.php b/plugins/managesieve/tests/Vacation.php
new file mode 100644
index 000000000..e34eb7aa2
--- /dev/null
+++ b/plugins/managesieve/tests/Vacation.php
@@ -0,0 +1,66 @@
+<?php
+
+class Managesieve_Vacation extends PHPUnit_Framework_TestCase
+{
+
+    function setUp()
+    {
+        include_once dirname(__FILE__) . '/../lib/Roundcube/rcube_sieve_engine.php';
+        include_once dirname(__FILE__) . '/../lib/Roundcube/rcube_sieve_vacation.php';
+    }
+
+    /**
+     * Plugin object construction test
+     */
+    function test_constructor()
+    {
+        $vacation = new rcube_sieve_vacation(true);
+
+        $this->assertInstanceOf('rcube_sieve_vacation', $vacation);
+    }
+
+    function test_build_regexp_tests()
+    {
+        $tests = rcube_sieve_vacation::build_regexp_tests('2014-02-20', '2014-03-05', $error);
+
+        $this->assertCount(2, $tests);
+        $this->assertSame('header', $tests[0]['test']);
+        $this->assertSame('regex', $tests[0]['type']);
+        $this->assertSame('received', $tests[0]['arg1']);
+        $this->assertSame('(20|21|22|23|24|25|26|27|28) Feb 2014', $tests[0]['arg2']);
+        $this->assertSame('header', $tests[1]['test']);
+        $this->assertSame('regex', $tests[1]['type']);
+        $this->assertSame('received', $tests[1]['arg1']);
+        $this->assertSame('([ 0]1|[ 0]2|[ 0]3|[ 0]4|[ 0]5) Mar 2014', $tests[1]['arg2']);
+
+        $tests = rcube_sieve_vacation::build_regexp_tests('2014-02-20', '2014-01-05', $error);
+
+        $this->assertSame(null, $tests);
+        $this->assertSame('managesieve.invaliddateformat', $error);
+    }
+
+    function test_parse_regexp_tests()
+    {
+        $tests = array(
+            array(
+                'test' => 'header',
+                'type' => 'regex',
+                'arg1' => 'received',
+                'arg2' => '(20|21|22|23|24|25|26|27|28) Feb 2014',
+            ),
+            array(
+                'test' => 'header',
+                'type' => 'regex',
+                'arg1' => 'received',
+                'arg2' => '([ 0]1|[ 0]2|[ 0]3|[ 0]4|[ 0]5) Mar 2014',
+            )
+        );
+
+        $result = rcube_sieve_vacation::parse_regexp_tests($tests);
+
+        $this->assertCount(2, $result);
+        $this->assertSame('20 Feb 2014', $result['from']);
+        $this->assertSame('05 Mar 2014', $result['to']);
+    }
+}
+
diff --git a/tests/phpunit.xml b/tests/phpunit.xml
index f3df53521..4d50ad6a0 100644
--- a/tests/phpunit.xml
+++ b/tests/phpunit.xml
@@ -67,6 +67,7 @@
             <file>./../plugins/managesieve/tests/Managesieve.php</file>
             <file>./../plugins/managesieve/tests/Parser.php</file>
             <file>./../plugins/managesieve/tests/Tokenizer.php</file>
+            <file>./../plugins/managesieve/tests/Vacation.php</file>
             <file>./../plugins/markasjunk/tests/Markasjunk.php</file>
             <file>./../plugins/new_user_dialog/tests/NewUserDialog.php</file>
             <file>./../plugins/new_user_identity/tests/NewUserIdentity.php</file>