From 08db1e6fac984bb889c15db4780c443a963a9890 Mon Sep 17 00:00:00 2001
From: Chris Boden <cboden@gmail.com>
Date: Tue, 15 Nov 2011 08:22:01 -0500
Subject: [PATCH] Minor Refactoring

---
 lib/Ratchet/Application/Server/App.php        | 117 +++++++++---------
 lib/Ratchet/Exception.php                     |   3 +
 lib/Ratchet/Resource/Connection.php           |  17 ++-
 tests/Ratchet/Tests/Mock/Application.php      |  20 +++
 .../Ratchet/Tests/Resource/ConnectionTest.php |  37 +++++-
 5 files changed, 132 insertions(+), 62 deletions(-)

diff --git a/lib/Ratchet/Application/Server/App.php b/lib/Ratchet/Application/Server/App.php
index d0ab70f..5ecd404 100644
--- a/lib/Ratchet/Application/Server/App.php
+++ b/lib/Ratchet/Application/Server/App.php
@@ -49,8 +49,6 @@ class App implements ApplicationInterface {
         $this->_connections[$host->getResource()] = new Connection($host);
         $this->_resources[] = $host->getResource();
 
-        $recv_bytes = 1024;
-
         set_time_limit(0);
         ob_implicit_flush();
 
@@ -61,64 +59,69 @@ class App implements ApplicationInterface {
         $host->listen();
 
         do {
-            $changed = $this->_resources;
-
-            try {
-                $num_changed = $host->select($changed, $write = null, $except = null, null);
-            } catch (Exception $e) {
-                // master had a problem?...what to do?
-                continue;
-            }
-
-			foreach($changed as $resource) {
-                try {
-                    $conn = $this->_connections[$resource];
-
-                    if ($host->getResource() === $resource) {
-                        $res = $this->onOpen($conn);
-                    } else {
-                        $data  = $buf = '';
-                        $bytes = $conn->getSocket()->recv($buf, $recv_bytes, 0);
-                        if ($bytes > 0) {
-                            $data = $buf;
-
-                            // This idea works* but...
-                            // 1) A single DDOS attack will block the entire application (I think)
-                            // 2) What if the last message in the frame is equal to $recv_bytes?  Would loop until another msg is sent
-                            // 3) This failed...an intermediary can set their buffer lower and this still propagates a fragment
-                            // Need to 1) proc_open the recv() calls.  2) ???
-/*
-                            while ($bytes === $recv_bytes) {
-                                $bytes = $conn->recv($buf, $recv_bytes, 0);
-                                $data .= $buf;
-                            }
-*/
-
-                            $res = $this->onRecv($conn, $data);
-                        } else {
-                            $res = $this->onClose($conn);
-                        }
-                    }
-                } catch (\Exception $e) {
-                    $res = $this->onError($conn, $e);
-                }
-
-                while ($res instanceof CommandInterface) {
-                    try {
-                        $new_res = $res->execute($this);
-                    } catch (\Exception $e) {
-                        break;
-                        // trigger new error
-                        // $new_res = $this->onError($e->getSocket()); ???
-                        // this is dangerous territory...could get in an infinte loop...Exception might not be Ratchet\Exception...$new_res could be ActionInterface|Composite|NULL...
-                    }
-
-                    $res = $new_res;
-                }
-            }
+            $this->loop($host);
         } while (true);
     }
 
+    protected function loop(SocketInterface $host, $recv_bytes = 1024) {
+        $changed = $this->_resources;
+
+        try {
+            $num_changed = $host->select($changed, $write = null, $except = null, null);
+        } catch (Exception $e) {
+            // master had a problem?...what to do?
+            return;
+        }
+
+        foreach($changed as $resource) {
+            try {
+                $conn = $this->_connections[$resource];
+
+                if ($host->getResource() === $resource) {
+                    $res = $this->onOpen($conn);
+                } else {
+                    $data  = $buf = '';
+                    $bytes = $conn->getSocket()->recv($buf, $recv_bytes, 0);
+                    if ($bytes > 0) {
+                        $data = $buf;
+
+                        // This idea works* but...
+                        // 1) A single DDOS attack will block the entire application (I think)
+                        // 2) What if the last message in the frame is equal to $recv_bytes?  Would loop until another msg is sent
+                        // 3) This failed...an intermediary can set their buffer lower and this still propagates a fragment
+                        // Need to 1) proc_open the recv() calls.  2) ???
+
+                        /*
+                        while ($bytes === $recv_bytes) {
+                            $bytes = $conn->recv($buf, $recv_bytes, 0);
+                            $data .= $buf;
+                        }
+                        */
+
+                        $res = $this->onRecv($conn, $data);
+                    } else {
+                        $res = $this->onClose($conn);
+                    }
+                }
+            } catch (\Exception $e) {
+                $res = $this->onError($conn, $e);
+            }
+
+            while ($res instanceof CommandInterface) {
+                try {
+                    $new_res = $res->execute($this);
+                } catch (\Exception $e) {
+                    break;
+                    // trigger new error
+                    // $new_res = $this->onError($e->getSocket()); ???
+                    // this is dangerous territory...could get in an infinte loop...Exception might not be Ratchet\Exception...$new_res could be ActionInterface|Composite|NULL...
+                }
+
+                $res = $new_res;
+            }
+        }
+    }
+
     public function onOpen(Connection $conn) {
         $new_socket     = clone $conn->getSocket();
         $new_connection = new Connection($new_socket);
diff --git a/lib/Ratchet/Exception.php b/lib/Ratchet/Exception.php
index 9d0087f..3fc2b16 100644
--- a/lib/Ratchet/Exception.php
+++ b/lib/Ratchet/Exception.php
@@ -5,6 +5,9 @@ namespace Ratchet;
  * Uses internal php methods to fill an Exception class (no parameters required)
  */
 class Exception extends \Exception {
+    /**
+     * @var SocketInterface
+     */
     protected $_socket;
 
     public function __construct(SocketInterface $socket) {
diff --git a/lib/Ratchet/Resource/Connection.php b/lib/Ratchet/Resource/Connection.php
index 64592cb..3c47865 100644
--- a/lib/Ratchet/Resource/Connection.php
+++ b/lib/Ratchet/Resource/Connection.php
@@ -50,7 +50,7 @@ class Connection {
      * @throws \InvalidArgumentException
      */
     public function &__get($name) {
-        if (!isset($this->_data[$name])) {
+        if (!$this->__isset($name)) {
             throw new \InvalidArgumentException("Attribute '{$name}' not found in Connection {$this->getID()}");
         }
 
@@ -60,4 +60,19 @@ class Connection {
             return $this->_data[$name];
         }
     }
+
+    /**
+     * @param mixed
+     * @return bool
+     */
+    public function __isset($name) {
+        return isset($this->_data[$name]);
+    }
+
+    /**
+     * @param mixed
+     */
+    public function __unset($name) {
+        unset($this->_data[$name]);
+    }
 }
\ No newline at end of file
diff --git a/tests/Ratchet/Tests/Mock/Application.php b/tests/Ratchet/Tests/Mock/Application.php
index 9515264..0b76dc6 100644
--- a/tests/Ratchet/Tests/Mock/Application.php
+++ b/tests/Ratchet/Tests/Mock/Application.php
@@ -5,18 +5,38 @@ use Ratchet\Tests\Mock\Socket as MockSocket;
 use Ratchet\Resource\Connection;
 
 class Application implements ApplicationInterface {
+    public $_app;
+
+    public $_conn_open;
+
+    public $_conn_recv;
+    public $_msg_recv;
+
+    public $_conn_close;
+
+    public $_conn_error;
+    public $_excep_error;
+
     public function __construct(ApplicationInterface $app = null) {
+        // probably should make this null app
+        $this->_app = $app;
     }
 
     public function onOpen(Connection $conn) {
+        $this->_conn_open = $conn;
     }
 
     public function onRecv(Connection $from, $msg) {
+        $this->_conn_recv = $from;
+        $this->_msg_recv  = $msg;
     }
 
     public function onClose(Connection $conn) {
+        $this->_conn_close = $conn;
     }
 
     public function onError(Connection $conn, \Exception $e) {
+        $this->_conn_error  = $conn;
+        $this->_excep_error = $e;
     }
 }
\ No newline at end of file
diff --git a/tests/Ratchet/Tests/Resource/ConnectionTest.php b/tests/Ratchet/Tests/Resource/ConnectionTest.php
index bfce598..490e3e2 100644
--- a/tests/Ratchet/Tests/Resource/ConnectionTest.php
+++ b/tests/Ratchet/Tests/Resource/ConnectionTest.php
@@ -7,16 +7,28 @@ use Ratchet\Tests\Mock\FakeSocket;
  * @covers Ratchet\Resource\Connection
  */
 class ConnectionTest extends \PHPUnit_Framework_TestCase {
+    protected $_fs;
     protected $_c;
 
     public function setUp() {
-        $this->_c = new Connection(new FakeSocket);
+        $this->_fs = new FakeSocket;
+        $this->_c  = new Connection($this->_fs);
     }
 
-    public function testCanGetWhatIsSet() {
-        $key = 'hello';
-        $val = 'world';
+    public static function keyAndValProvider() {
+        return array(
+            array('hello', 'world')
+        );
+    }
 
+    public function testGetSocketReturnsWhatIsSetInConstruct() {
+        $this->assertSame($this->_fs, $this->_c->getSocket());
+    }
+
+    /**
+     * @dataProvider keyAndValProvider
+     */
+    public function testCanGetWhatIsSet($key, $val) {
         $this->_c->{$key} = $val;
         $this->assertEquals($val, $this->_c->{$key});
     }
@@ -29,4 +41,21 @@ class ConnectionTest extends \PHPUnit_Framework_TestCase {
     public function testLambdaReturnValueOnGet() {
         $this->markTestIncomplete();
     }
+
+    /**
+     * @dataProvider keyAndValProvider
+     */
+    public function testIssetWorksOnOverloadedVariables($key, $val) {
+        $this->_c->{$key} = $val;
+        $this->assertTrue(isset($this->_c->{$key}));
+    }
+
+    /**
+     * @dataProvider keyAndValProvider
+     */
+    public function testUnsetMakesIssetReturnFalse($key, $val) {
+        $this->_c->{$key} = $val;
+        unset($this->_c->{$key});
+        $this->assertFalse(isset($this->_c->{$key}));
+    }
 }
\ No newline at end of file