From 13d4510e12be3486336f4098a1ee356ffc9cdc45 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 28 Jul 2011 22:28:17 +0200 Subject: [PATCH 1/7] Rename send to send_queue in network.Connection --- mopidy/utils/network.py | 4 ++-- tests/frontends/mpd/protocol/__init__.py | 2 +- tests/utils/network/connection_test.py | 10 +++++----- tests/utils/network/lineprotocol_test.py | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 9306ccd7..50b7d239 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -161,7 +161,7 @@ class Connection(object): except socket.error: pass - def send(self, data): + def queue_send(self, data): """Send data to client exactly as is.""" self.send_lock.acquire(True) self.send_buffer += data @@ -383,4 +383,4 @@ class LineProtocol(ThreadingActor): return data = self.join_lines(lines) - self.connection.send(self.encode(data)) + self.connection.queue_send(self.encode(data)) diff --git a/tests/frontends/mpd/protocol/__init__.py b/tests/frontends/mpd/protocol/__init__.py index 8cd91d60..078153b5 100644 --- a/tests/frontends/mpd/protocol/__init__.py +++ b/tests/frontends/mpd/protocol/__init__.py @@ -14,7 +14,7 @@ class MockConnetion(mock.Mock): self.port = mock.sentinel.port self.response = [] - def send(self, data): + def queue_send(self, data): lines = (line for line in data.split('\n') if line) self.response.extend(lines) diff --git a/tests/utils/network/connection_test.py b/tests/utils/network/connection_test.py index 6e68f250..7e7ff05a 100644 --- a/tests/utils/network/connection_test.py +++ b/tests/utils/network/connection_test.py @@ -318,7 +318,7 @@ class ConnectionTest(unittest.TestCase): self.mock.send_lock = Mock() self.mock.send_buffer = '' - network.Connection.send(self.mock, 'data') + network.Connection.queue_send(self.mock, 'data') self.mock.send_lock.acquire.assert_called_once_with(True) self.mock.send_lock.release.assert_called_once_with() @@ -326,20 +326,20 @@ class ConnectionTest(unittest.TestCase): self.mock.send_lock = Mock() self.mock.send_buffer = '' - network.Connection.send(self.mock, 'abc') + network.Connection.queue_send(self.mock, 'abc') self.assertEqual('abc', self.mock.send_buffer) - network.Connection.send(self.mock, 'def') + network.Connection.queue_send(self.mock, 'def') self.assertEqual('abcdef', self.mock.send_buffer) - network.Connection.send(self.mock, '') + network.Connection.queue_send(self.mock, '') self.assertEqual('abcdef', self.mock.send_buffer) def test_send_calls_enable_send(self): self.mock.send_lock = Mock() self.mock.send_buffer = '' - network.Connection.send(self.mock, 'data') + network.Connection.queue_send(self.mock, 'data') self.mock.enable_send.assert_called_once_with() def test_recv_callback_respects_io_err(self): diff --git a/tests/utils/network/lineprotocol_test.py b/tests/utils/network/lineprotocol_test.py index 3d16f81c..4e7df132 100644 --- a/tests/utils/network/lineprotocol_test.py +++ b/tests/utils/network/lineprotocol_test.py @@ -196,7 +196,7 @@ class LineProtocolTest(unittest.TestCase): network.LineProtocol.send_lines(self.mock, []) self.assertEqual(0, self.mock.encode.call_count) - self.assertEqual(0, self.mock.connection.send.call_count) + self.assertEqual(0, self.mock.connection.queue_send.call_count) def test_send_lines_calls_join_lines(self): self.mock.connection = Mock(spec=network.Connection) @@ -218,7 +218,7 @@ class LineProtocolTest(unittest.TestCase): self.mock.encode.return_value = sentinel.data network.LineProtocol.send_lines(self.mock, sentinel.lines) - self.mock.connection.send.assert_called_once_with(sentinel.data) + self.mock.connection.queue_send.assert_called_once_with(sentinel.data) def test_join_lines_returns_empty_string_for_no_lines(self): self.assertEqual(u'', network.LineProtocol.join_lines(self.mock, [])) From 43f4f1537ef2b07346d46c351c4850c7c2279c95 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 28 Jul 2011 22:34:47 +0200 Subject: [PATCH 2/7] Extract send to seperate method --- mopidy/utils/network.py | 15 ++++++++++----- tests/utils/network/connection_test.py | 16 +++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 50b7d239..7da31f84 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -253,18 +253,23 @@ class Connection(object): return True try: - sent = self.sock.send(self.send_buffer) - self.send_buffer = self.send_buffer[sent:] + self.send_buffer = self.send(self.send_buffer) if not self.send_buffer: self.disable_send() - except socket.error as e: - if e.errno not in (errno.EWOULDBLOCK, errno.EINTR): - self.stop(u'Unexpected client error: %s' % e) finally: self.send_lock.release() return True + def send(self, data): + try: + sent = self.sock.send(data) + return data[sent:] + except socket.error as e: + if e.errno in (errno.EWOULDBLOCK, errno.EINTR): + return data + self.stop(u'Unexpected client error: %s' % e) + def timeout_callback(self): self.stop(u'Client timeout out after %s seconds' % self.timeout) return False diff --git a/tests/utils/network/connection_test.py b/tests/utils/network/connection_test.py index 7e7ff05a..6f22aeb3 100644 --- a/tests/utils/network/connection_test.py +++ b/tests/utils/network/connection_test.py @@ -8,7 +8,7 @@ import unittest from mopidy.utils import network from mock import patch, sentinel, Mock -from tests import any_int, any_unicode +from tests import any_int, any_unicode, SkipTest class ConnectionTest(unittest.TestCase): def setUp(self): @@ -473,32 +473,30 @@ class ConnectionTest(unittest.TestCase): self.mock.send_lock = Mock() self.mock.send_lock.acquire.return_value = True self.mock.send_buffer = 'data' - self.mock.sock = Mock(spec=socket.SocketType) - self.mock.sock.send.return_value = 4 + self.mock.send.return_value = '' self.assertTrue(network.Connection.send_callback( self.mock, sentinel.fd, gobject.IO_IN)) self.mock.disable_send.assert_called_once_with() - self.mock.sock.send.assert_called_once_with('data') + self.mock.send.assert_called_once_with('data') self.assertEqual('', self.mock.send_buffer) def test_send_callback_sends_partial_data(self): self.mock.send_lock = Mock() self.mock.send_lock.acquire.return_value = True self.mock.send_buffer = 'data' - self.mock.sock = Mock(spec=socket.SocketType) - self.mock.sock.send.return_value = 2 + self.mock.send.return_value = 'ta' self.assertTrue(network.Connection.send_callback( self.mock, sentinel.fd, gobject.IO_IN)) - self.mock.sock.send.assert_called_once_with('data') + self.mock.send.assert_called_once_with('data') self.assertEqual('ta', self.mock.send_buffer) + @SkipTest def test_send_callback_recoverable_error(self): self.mock.send_lock = Mock() self.mock.send_lock.acquire.return_value = True self.mock.send_buffer = 'data' - self.mock.sock = Mock(spec=socket.SocketType) for error in (errno.EWOULDBLOCK, errno.EINTR): self.mock.sock.send.side_effect = socket.error(error, '') @@ -506,11 +504,11 @@ class ConnectionTest(unittest.TestCase): self.mock, sentinel.fd, gobject.IO_IN)) self.assertEqual(0, self.mock.stop.call_count) + @SkipTest def test_send_callback_unrecoverable_error(self): self.mock.send_lock = Mock() self.mock.send_lock.acquire.return_value = True self.mock.send_buffer = 'data' - self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.send.side_effect = socket.error self.assertTrue(network.Connection.send_callback( From 4f6ddd3532876c43b23b54029271d1b4851bd7a5 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 28 Jul 2011 22:41:53 +0200 Subject: [PATCH 3/7] Add error handling tests for new send method --- mopidy/utils/network.py | 1 + tests/utils/network/connection_test.py | 23 ++++++++--------------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 7da31f84..5267afd5 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -269,6 +269,7 @@ class Connection(object): if e.errno in (errno.EWOULDBLOCK, errno.EINTR): return data self.stop(u'Unexpected client error: %s' % e) + return '' def timeout_callback(self): self.stop(u'Client timeout out after %s seconds' % self.timeout) diff --git a/tests/utils/network/connection_test.py b/tests/utils/network/connection_test.py index 6f22aeb3..7241dc5a 100644 --- a/tests/utils/network/connection_test.py +++ b/tests/utils/network/connection_test.py @@ -492,27 +492,20 @@ class ConnectionTest(unittest.TestCase): self.mock.send.assert_called_once_with('data') self.assertEqual('ta', self.mock.send_buffer) - @SkipTest - def test_send_callback_recoverable_error(self): - self.mock.send_lock = Mock() - self.mock.send_lock.acquire.return_value = True - self.mock.send_buffer = 'data' + def test_send_recoverable_error(self): + self.mock.sock = Mock(spec=socket.SocketType) for error in (errno.EWOULDBLOCK, errno.EINTR): self.mock.sock.send.side_effect = socket.error(error, '') - self.assertTrue(network.Connection.send_callback( - self.mock, sentinel.fd, gobject.IO_IN)) + + network.Connection.send(self.mock, 'data') self.assertEqual(0, self.mock.stop.call_count) - @SkipTest - def test_send_callback_unrecoverable_error(self): - self.mock.send_lock = Mock() - self.mock.send_lock.acquire.return_value = True - self.mock.send_buffer = 'data' - + def test_send_unrecoverable_error(self): + self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.send.side_effect = socket.error - self.assertTrue(network.Connection.send_callback( - self.mock, sentinel.fd, gobject.IO_IN)) + + self.assertEqual('', network.Connection.send(self.mock, 'data')) self.mock.stop.assert_called_once_with(any_unicode) def test_timeout_callback(self): From 93c16cc2cdc723344e1792828a42c3db78a17780 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 28 Jul 2011 22:43:54 +0200 Subject: [PATCH 4/7] Add tests for socket sending --- tests/utils/network/connection_test.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/utils/network/connection_test.py b/tests/utils/network/connection_test.py index 7241dc5a..16ec3979 100644 --- a/tests/utils/network/connection_test.py +++ b/tests/utils/network/connection_test.py @@ -501,6 +501,20 @@ class ConnectionTest(unittest.TestCase): network.Connection.send(self.mock, 'data') self.assertEqual(0, self.mock.stop.call_count) + def test_send_calls_socket_send(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.send.return_value = 4 + + self.assertEqual('', network.Connection.send(self.mock, 'data')) + self.mock.sock.send.assert_called_once_with('data') + + def test_send_calls_socket_send_partial_send(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.send.return_value = 2 + + self.assertEqual('ta', network.Connection.send(self.mock, 'data')) + self.mock.sock.send.assert_called_once_with('data') + def test_send_unrecoverable_error(self): self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.send.side_effect = socket.error From 3195476421b031a6b8516404a56a1a75dc91acbb Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 28 Jul 2011 22:44:43 +0200 Subject: [PATCH 5/7] Rename old send tests to queue_send --- tests/utils/network/connection_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/utils/network/connection_test.py b/tests/utils/network/connection_test.py index 16ec3979..e96d1852 100644 --- a/tests/utils/network/connection_test.py +++ b/tests/utils/network/connection_test.py @@ -314,7 +314,7 @@ class ConnectionTest(unittest.TestCase): self.assertEqual(0, gobject.source_remove.call_count) self.assertEqual(None, self.mock.timeout_id) - def test_send_acquires_and_releases_lock(self): + def test_queue_send_acquires_and_releases_lock(self): self.mock.send_lock = Mock() self.mock.send_buffer = '' @@ -322,7 +322,7 @@ class ConnectionTest(unittest.TestCase): self.mock.send_lock.acquire.assert_called_once_with(True) self.mock.send_lock.release.assert_called_once_with() - def test_send_appends_to_send_buffer(self): + def test_queue_send_appends_to_send_buffer(self): self.mock.send_lock = Mock() self.mock.send_buffer = '' @@ -335,7 +335,7 @@ class ConnectionTest(unittest.TestCase): network.Connection.queue_send(self.mock, '') self.assertEqual('abcdef', self.mock.send_buffer) - def test_send_calls_enable_send(self): + def test_queue_send_calls_enable_send(self): self.mock.send_lock = Mock() self.mock.send_buffer = '' From cb4f32cb5846d55d9db76140b90646837f0e5edd Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 28 Jul 2011 22:51:41 +0200 Subject: [PATCH 6/7] Try to send directly in quene_send when we can to prevent uneeded context switches --- mopidy/utils/network.py | 5 ++-- tests/utils/network/connection_test.py | 38 ++++++++++++++++---------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 5267afd5..84fe49bd 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -164,9 +164,10 @@ class Connection(object): def queue_send(self, data): """Send data to client exactly as is.""" self.send_lock.acquire(True) - self.send_buffer += data + self.send_buffer = self.send(self.send_buffer + data) self.send_lock.release() - self.enable_send() + if self.send_buffer: + self.enable_send() def enable_timeout(self): """Reactivate timeout mechanism.""" diff --git a/tests/utils/network/connection_test.py b/tests/utils/network/connection_test.py index e96d1852..1bda0af5 100644 --- a/tests/utils/network/connection_test.py +++ b/tests/utils/network/connection_test.py @@ -322,25 +322,35 @@ class ConnectionTest(unittest.TestCase): self.mock.send_lock.acquire.assert_called_once_with(True) self.mock.send_lock.release.assert_called_once_with() - def test_queue_send_appends_to_send_buffer(self): - self.mock.send_lock = Mock() + def test_queue_send_calls_send(self): self.mock.send_buffer = '' - - network.Connection.queue_send(self.mock, 'abc') - self.assertEqual('abc', self.mock.send_buffer) - - network.Connection.queue_send(self.mock, 'def') - self.assertEqual('abcdef', self.mock.send_buffer) - - network.Connection.queue_send(self.mock, '') - self.assertEqual('abcdef', self.mock.send_buffer) - - def test_queue_send_calls_enable_send(self): self.mock.send_lock = Mock() - self.mock.send_buffer = '' + self.mock.send.return_value = '' network.Connection.queue_send(self.mock, 'data') + self.mock.send.assert_called_once_with('data') + self.assertEqual(0, self.mock.enable_send.call_count) + self.assertEqual('', self.mock.send_buffer) + + def test_queue_send_calls_enable_send_for_partial_send(self): + self.mock.send_buffer = '' + self.mock.send_lock = Mock() + self.mock.send.return_value = 'ta' + + network.Connection.queue_send(self.mock, 'data') + self.mock.send.assert_called_once_with('data') self.mock.enable_send.assert_called_once_with() + self.assertEqual('ta', self.mock.send_buffer) + + def test_queue_send_calls_send_with_existing_buffer(self): + self.mock.send_buffer = 'foo' + self.mock.send_lock = Mock() + self.mock.send.return_value = '' + + network.Connection.queue_send(self.mock, 'bar') + self.mock.send.assert_called_once_with('foobar') + self.assertEqual(0, self.mock.enable_send.call_count) + self.assertEqual('', self.mock.send_buffer) def test_recv_callback_respects_io_err(self): self.mock.sock = Mock(spec=socket.SocketType) From 34594e40e83ec7b0065c9b3e0ea4f47789cf3fa1 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 28 Jul 2011 22:55:54 +0200 Subject: [PATCH 7/7] Move method and cleanup docstrings --- mopidy/utils/network.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 84fe49bd..5079fe7c 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -162,13 +162,24 @@ class Connection(object): pass def queue_send(self, data): - """Send data to client exactly as is.""" + """Try to send data to client exactly as is and queue rest.""" self.send_lock.acquire(True) self.send_buffer = self.send(self.send_buffer + data) self.send_lock.release() if self.send_buffer: self.enable_send() + def send(self, data): + """Send data to client, return any unsent data.""" + try: + sent = self.sock.send(data) + return data[sent:] + except socket.error as e: + if e.errno in (errno.EWOULDBLOCK, errno.EINTR): + return data + self.stop(u'Unexpected client error: %s' % e) + return '' + def enable_timeout(self): """Reactivate timeout mechanism.""" if self.timeout <= 0: @@ -262,16 +273,6 @@ class Connection(object): return True - def send(self, data): - try: - sent = self.sock.send(data) - return data[sent:] - except socket.error as e: - if e.errno in (errno.EWOULDBLOCK, errno.EINTR): - return data - self.stop(u'Unexpected client error: %s' % e) - return '' - def timeout_callback(self): self.stop(u'Client timeout out after %s seconds' % self.timeout) return False