Merge pull request #17 from abimus/fix/flags

Fix behavior on Server::setFlag()
tedious · Jul 12, 2013 · 12f147b · 12f147b
2 parents 5403184 + 7972b0c
commit 12f147b
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 9 deletions.
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<phpunit backupGlobals="false"
+         backupStaticAttributes="false"
+         colors="true"
+         convertErrorsToExceptions="true"
+         convertNoticesToExceptions="true"
+         convertWarningsToExceptions="true"
+         processIsolation="false"
+         stopOnFailure="false"
+         syntaxCheck="false"
+         bootstrap="tests/bootstrap.php"
+>
+
+    <testsuites>
+        <testsuite name="Fetch Test Suite">
+            <directory>./tests</directory>
+        </testsuite>
+    </testsuites>
+</phpunit>
diff --git a/src/Fetch/Server.php b/src/Fetch/Server.php
@@ -179,21 +179,27 @@ public function setFlag($flag, $value = null)
             return;
 
         if (isset(self::$exclusiveFlags[$flag])) {
-            $kill = $flag;
+            $kill = self::$exclusiveFlags[$flag];
         } elseif ($index = array_search($flag, self::$exclusiveFlags)) {
             $kill = $index;
         }
 
-        if (isset($kill) && isset($this->flags[$kill]))
-            unset($this->flags[$kill]);
+        if (isset($kill) && false !== $index = array_search($kill, $this->flags))
+            unset($this->flags[$index]);
 
+        $index = array_search($flag, $this->flags);
         if (isset($value) && $value !== true) {
-            if ($value == false) {
-                unset($this->flags[$flag]);
-            } else {
-                $this->flags[] = $flag . '=' . $value;
+            if ($value == false && $index !== false) {
+                unset($this->flags[$index]);
+            } elseif ($value != false) {
+                $match = preg_grep('/' . $flag . '/', $this->flags);
+                if (reset($match)) {
+                    $this->flags[key($match)] = $flag . '=' . $value;
+                } else {
+                    $this->flags[] = $flag . '=' . $value;
+                }
             }
-        } else {
+        } elseif ($index === false) {
             $this->flags[] = $flag;
         }
     }

diff --git a/tests/Fetch/Test/ServerTest.php b/tests/Fetch/Test/ServerTest.php
@@ -11,12 +11,47 @@
 
 namespace Fetch\Test;
 
+use Fetch\Server;
 
 /**
  * @package Fetch
  * @author  Robert Hafner <tedivm@tedivm.com>
  */
 class ServerTest extends \PHPUnit_Framework_TestCase
 {
-
+    /**
+     * @dataProvider flagsDataProvider
+     * @param string $expected server string with %host% placeholder
+     * @param integer $port to use (needed to test behavior on port 143 and 993 from constructor)
+     * @param array $flags to set/unset ($flag => $value)
+     */
+    public function testFlags($expected, $port, $flags)
+    {
+        $host = 'example.com';
+        $server = new Server($host, $port);
+
+        foreach ($flags as $flag => $value) {
+            $server->setFlag($flag, $value);
+        }
+
+        $this->assertEquals(str_replace('%host%', $host, $expected), $server->getServerString());
+    }
+
+    public function flagsDataProvider() {
+        return array(
+                array('{%host%:143/novalidate-cert}', 143, array()),
+                array('{%host%:143/validate-cert}', 143, array('validate-cert' => true)),
+                array('{%host%:143}', 143, array('novalidate-cert' => false)),
+                array('{%host%:993/ssl}', 993, array()),
+                array('{%host%:993}', 993, array('ssl' => false)),
+                array('{%host%:100/tls}', 100, array('tls' => true)),
+                array('{%host%:100/tls}', 100, array('tls' => true, 'tls' => true)),
+                array('{%host%:100/notls}', 100, array('tls' => true, 'notls' => true)),
+                array('{%host%:100}', 100, array('ssl' => true, 'ssl' => false)),
+                array('{%host%:100/user=foo}', 100, array('user' => 'foo')),
+                array('{%host%:100/user=foo}', 100, array('user' => 'foo', 'user' => 'foo')),
+                array('{%host%:100/user=bar}', 100, array('user' => 'foo', 'user' => 'bar')),
+                array('{%host%:100}', 100, array('user' => 'foo', 'user' => false)),
+        );
+    }
 }