From 387d72ef67b1203a831124e1b6288bb405864012 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 7 Jun 2011 16:05:26 +0200 Subject: [PATCH 01/72] Remove self.set_reuse_addr from asyncore code --- mopidy/frontends/mpd/server.py | 1 - mopidy/utils/network.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/frontends/mpd/server.py b/mopidy/frontends/mpd/server.py index 927e2a00..b3aa0481 100644 --- a/mopidy/frontends/mpd/server.py +++ b/mopidy/frontends/mpd/server.py @@ -21,7 +21,6 @@ class MpdServer(asyncore.dispatcher): """Start MPD server.""" try: self.socket = network.create_socket() - self.set_reuse_addr() hostname = network.format_hostname(settings.MPD_SERVER_HOSTNAME) port = settings.MPD_SERVER_PORT logger.debug(u'MPD server is binding to [%s]:%s', hostname, port) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 1dedf7d7..80a51c77 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -27,6 +27,7 @@ def create_socket(): sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 0) else: sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) return sock def format_hostname(hostname): From 24dbba2fa9d3f2dc76657a2d631e8f6f15232a7f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 16 Jun 2011 23:17:37 +0200 Subject: [PATCH 02/72] Switch to async globject based loop --- mopidy/frontends/mpd/server.py | 53 +++++++++---- mopidy/frontends/mpd/session.py | 58 --------------- mopidy/utils/network.py | 127 ++++++++++++++++++++++++++++++++ 3 files changed, 167 insertions(+), 71 deletions(-) delete mode 100644 mopidy/frontends/mpd/session.py diff --git a/mopidy/frontends/mpd/server.py b/mopidy/frontends/mpd/server.py index 62e443fb..1e5f5d4a 100644 --- a/mopidy/frontends/mpd/server.py +++ b/mopidy/frontends/mpd/server.py @@ -1,14 +1,17 @@ -import asyncore import logging import sys +import gobject + from mopidy import settings from mopidy.utils import network -from .session import MpdSession +from mopidy.frontends.mpd.dispatcher import MpdDispatcher +from mopidy.frontends.mpd.protocol import ENCODING, LINE_TERMINATOR, VERSION +from mopidy.utils.log import indent logger = logging.getLogger('mopidy.frontends.mpd.server') -class MpdServer(asyncore.dispatcher): +class MpdServer(object): """ The MPD server. Creates a :class:`mopidy.frontends.mpd.session.MpdSession` for each client connection. @@ -17,22 +20,46 @@ class MpdServer(asyncore.dispatcher): def start(self): """Start MPD server.""" try: - self.set_socket(network.create_socket()) - self.set_reuse_addr() hostname = network.format_hostname(settings.MPD_SERVER_HOSTNAME) port = settings.MPD_SERVER_PORT logger.debug(u'MPD server is binding to [%s]:%s', hostname, port) - self.bind((hostname, port)) - self.listen(1) + network.Listener((hostname, port), handler=MpdHandler) logger.info(u'MPD server running at [%s]:%s', hostname, port) except IOError, e: logger.error(u'MPD server startup failed: %s' % str(e).decode('utf-8')) sys.exit(1) - def handle_accept(self): - """Called by asyncore when a new client connects.""" - (client_socket, client_socket_address) = self.accept() - logger.info(u'MPD client connection from [%s]:%s', - client_socket_address[0], client_socket_address[1]) - MpdSession(self, client_socket, client_socket_address) + +class MpdHandler(network.BaseHandler): + """ + The MPD client session. Keeps track of a single client session. Any + requests from the client is passed on to the MPD request dispatcher. + """ + + terminator = LINE_TERMINATOR + + def __init__(self, (sock, addr)): + super(MpdHandler, self).__init__((sock, addr)) + self.dispatcher = MpdDispatcher(session=self) + self.send_response([u'OK MPD %s' % VERSION]) + + def recv(self, line): + """Handle the request using the MPD command handlers.""" + request = line.decode(ENCODING) + logger.debug(u'Request from [%s]:%s: %s', self.addr[0], + self.addr[1], indent(request)) + self.send_response(self.dispatcher.handle_request(request)) + + def send_response(self, response): + """ + Format a response from the MPD command handlers and send it to the + client. + """ + if response: + response = LINE_TERMINATOR.join(response) + logger.debug(u'Response to [%s]:%s: %s', self.addr[0], + self.addr[1], indent(response)) + response = u'%s%s' % (response, LINE_TERMINATOR) + data = response.encode(ENCODING) + self.send(data) diff --git a/mopidy/frontends/mpd/session.py b/mopidy/frontends/mpd/session.py deleted file mode 100644 index ce5d3be7..00000000 --- a/mopidy/frontends/mpd/session.py +++ /dev/null @@ -1,58 +0,0 @@ -import asynchat -import logging - -from mopidy.frontends.mpd.dispatcher import MpdDispatcher -from mopidy.frontends.mpd.protocol import ENCODING, LINE_TERMINATOR, VERSION -from mopidy.utils.log import indent - -logger = logging.getLogger('mopidy.frontends.mpd.session') - -class MpdSession(asynchat.async_chat): - """ - The MPD client session. Keeps track of a single client session. Any - requests from the client is passed on to the MPD request dispatcher. - """ - - def __init__(self, server, client_socket, client_socket_address): - asynchat.async_chat.__init__(self, sock=client_socket) - self.server = server - self.client_address = client_socket_address[0] - self.client_port = client_socket_address[1] - self.input_buffer = [] - self.authenticated = False - self.set_terminator(LINE_TERMINATOR.encode(ENCODING)) - self.dispatcher = MpdDispatcher(session=self) - self.send_response([u'OK MPD %s' % VERSION]) - - def collect_incoming_data(self, data): - """Called by asynchat when new data arrives.""" - self.input_buffer.append(data) - - def found_terminator(self): - """Called by asynchat when a terminator is found in incoming data.""" - data = ''.join(self.input_buffer).strip() - self.input_buffer = [] - try: - self.send_response(self.handle_request(data)) - except UnicodeDecodeError as e: - logger.warning(u'Received invalid data: %s', e) - - def handle_request(self, request): - """Handle the request using the MPD command handlers.""" - request = request.decode(ENCODING) - logger.debug(u'Request from [%s]:%s: %s', self.client_address, - self.client_port, indent(request)) - return self.dispatcher.handle_request(request) - - def send_response(self, response): - """ - Format a response from the MPD command handlers and send it to the - client. - """ - if response: - response = LINE_TERMINATOR.join(response) - logger.debug(u'Response to [%s]:%s: %s', self.client_address, - self.client_port, indent(response)) - response = u'%s%s' % (response, LINE_TERMINATOR) - data = response.encode(ENCODING) - self.push(data) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 80a51c77..a5680466 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -1,6 +1,7 @@ import logging import re import socket +import gobject logger = logging.getLogger('mopidy.utils.server') @@ -35,3 +36,129 @@ def format_hostname(hostname): if (has_ipv6 and re.match('\d+.\d+.\d+.\d+', hostname) is not None): hostname = '::ffff:%s' % hostname return hostname + +class BaseHandler(object): + """Buffered lined based client, subclass for use.""" + + #: Line terminator to use in parse_line, can be overridden by subclasses. + terminator = '\n' + + def __init__(self, (sock, addr)): + logger.debug('Established connection from %s', addr) + + self.sock, self.addr = sock, addr + self.receiver = None + self.sender = None + self.recv_buffer = '' + self.send_buffer = '' + + self.sock.setblocking(0) + self.add_recv_watch() + + def add_recv_watch(self): + """Register recv and error handling of socket.""" + if self.receiver is None: + self.receiver = gobject.io_add_watch(self.sock.fileno(), gobject.IO_IN + | gobject.IO_ERR | gobject.IO_HUP, self.handle) + + def clear_recv_watch(self): + if self.receiver is not None: + gobject.source_remove(self.receiver) + self.receiver = None + + def add_send_watch(self): + """Register send handling if it has not already been done.""" + if self.sender is None: + self.sender = gobject.io_add_watch(self.sock.fileno(), + gobject.IO_OUT, self.handle) + + def clear_send_watch(self): + """Remove send watcher if it is set.""" + if self.sender is not None: + gobject.source_remove(self.sender) + self.sender = None + + def handle(self, fd, flags): + """Dispatch based on current flags.""" + if flags & (gobject.IO_ERR | gobject.IO_HUP): + return self.close() + if flags & gobject.IO_IN: + return self.io_in() + if flags & gobject.IO_OUT: + return self.io_out() + logger.error('Unknown flag: %s', flags) + return False + + def io_in(self): + """Record any incoming data to buffer and parse lines.""" + data = self.sock.recv(1024) + self.recv_buffer += data # XXX limit buffer size? + if data: + return self.parse_lines() + else: + return self.close() + + def io_out(self): + """Send as much of outgoing buffer as possible.""" + if self.send_buffer: + sent = self.sock.send(self.send_buffer) + self.send_buffer = self.send_buffer[sent:] + if not self.send_buffer: + self.clear_send_watch() + return True + + def close(self): + """Close connection.""" + logger.debug('Closing connection from %s', self.addr) + self.clear_send_watch() + self.sock.close() + return False + + def release(self): + """Forget about socket so that other loop can take over FD. + + Note that other code will still need to keep a ref to the socket in + order to prevent GC cleanup closing it. + """ + self.clear_recv_watch() + self.clear_send_watch() + return self.sock + + def send(self, data): + """Add raw data to send to outbound buffer.""" + self.add_send_watch() + self.send_buffer += data # XXX limit buffer size? + + def recv(self, line): + """Recv one and one line of request. Must be sub-classed.""" + raise NotImplementedError + + def parse_lines(self): + """Parse lines by splitting at terminator.""" + while self.terminator in self.recv_buffer: + line, self.recv_buffer = self.recv_buffer.split(self.terminator, 1) + self.recv(line) + return True + +class EchoHandler(BaseHandler): + """Basic handler used for debuging of Listener and Handler code itself.""" + def recv(self, line): + print repr(line) + self.send(line) + +class Listener(object): + """Setup listener and register it with gobject loop.""" + def __init__(self, addr, handler=EchoHandler): + self.handler = handler + self.sock = create_socket() + self.sock.setblocking(0) + self.sock.bind(addr) + self.sock.listen(5) + + gobject.io_add_watch(self.sock.fileno(), gobject.IO_IN, self.handle) + logger.debug('Listening on %s using %s', addr, self.handler) + + def handle(self, fd, flags): + self.handler(self.sock.accept()) + return True + From 9df16e071630a3e1f9af94e624d539a05e0343dc Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 17 Jun 2011 00:01:55 +0200 Subject: [PATCH 03/72] Get rid of custom async client code in favour of blocking IOChannel in ThreadingActors --- mopidy/frontends/mpd/__init__.py | 1 - mopidy/frontends/mpd/server.py | 39 ++++++---- mopidy/utils/network.py | 119 ++----------------------------- 3 files changed, 31 insertions(+), 128 deletions(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 175aa0ee..b6088a41 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -44,4 +44,3 @@ class MpdThread(BaseThread): logger.debug(u'Starting MPD server thread') server = MpdServer() server.start() - asyncore.loop() diff --git a/mopidy/frontends/mpd/server.py b/mopidy/frontends/mpd/server.py index 1e5f5d4a..12e6a92a 100644 --- a/mopidy/frontends/mpd/server.py +++ b/mopidy/frontends/mpd/server.py @@ -3,6 +3,8 @@ import sys import gobject +from pykka.actor import ThreadingActor + from mopidy import settings from mopidy.utils import network from mopidy.frontends.mpd.dispatcher import MpdDispatcher @@ -23,7 +25,7 @@ class MpdServer(object): hostname = network.format_hostname(settings.MPD_SERVER_HOSTNAME) port = settings.MPD_SERVER_PORT logger.debug(u'MPD server is binding to [%s]:%s', hostname, port) - network.Listener((hostname, port), handler=MpdHandler) + network.Listener((hostname, port), session=MpdSession) logger.info(u'MPD server running at [%s]:%s', hostname, port) except IOError, e: logger.error(u'MPD server startup failed: %s' % @@ -31,25 +33,33 @@ class MpdServer(object): sys.exit(1) -class MpdHandler(network.BaseHandler): +class MpdSession(ThreadingActor): """ The MPD client session. Keeps track of a single client session. Any requests from the client is passed on to the MPD request dispatcher. """ - terminator = LINE_TERMINATOR - - def __init__(self, (sock, addr)): - super(MpdHandler, self).__init__((sock, addr)) + def __init__(self, sock, addr): + self.sock = sock # Prevent premature GC + self.addr = addr + self.channel = gobject.IOChannel(sock.fileno()) self.dispatcher = MpdDispatcher(session=self) - self.send_response([u'OK MPD %s' % VERSION]) - def recv(self, line): - """Handle the request using the MPD command handlers.""" - request = line.decode(ENCODING) - logger.debug(u'Request from [%s]:%s: %s', self.addr[0], - self.addr[1], indent(request)) - self.send_response(self.dispatcher.handle_request(request)) + def on_start(self): + try: + self.send_response([u'OK MPD %s' % VERSION]) + self.request_loop() + except gobject.GError, e: + self.stop() + + def request_loop(self): + while True: + logger.debug('Trying to readline') + request = self.channel.readline()[:-1].decode(ENCODING) + logger.debug(u'Request from [%s]:%s: %s', self.addr[0], + self.addr[1], indent(request)) + response = self.dispatcher.handle_request(request) + self.send_response(response) def send_response(self, response): """ @@ -62,4 +72,5 @@ class MpdHandler(network.BaseHandler): self.addr[1], indent(response)) response = u'%s%s' % (response, LINE_TERMINATOR) data = response.encode(ENCODING) - self.send(data) + self.channel.write(data) + self.channel.flush() diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index a5680466..df4f9292 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -37,128 +37,21 @@ def format_hostname(hostname): hostname = '::ffff:%s' % hostname return hostname -class BaseHandler(object): - """Buffered lined based client, subclass for use.""" - - #: Line terminator to use in parse_line, can be overridden by subclasses. - terminator = '\n' - - def __init__(self, (sock, addr)): - logger.debug('Established connection from %s', addr) - - self.sock, self.addr = sock, addr - self.receiver = None - self.sender = None - self.recv_buffer = '' - self.send_buffer = '' - - self.sock.setblocking(0) - self.add_recv_watch() - - def add_recv_watch(self): - """Register recv and error handling of socket.""" - if self.receiver is None: - self.receiver = gobject.io_add_watch(self.sock.fileno(), gobject.IO_IN - | gobject.IO_ERR | gobject.IO_HUP, self.handle) - - def clear_recv_watch(self): - if self.receiver is not None: - gobject.source_remove(self.receiver) - self.receiver = None - - def add_send_watch(self): - """Register send handling if it has not already been done.""" - if self.sender is None: - self.sender = gobject.io_add_watch(self.sock.fileno(), - gobject.IO_OUT, self.handle) - - def clear_send_watch(self): - """Remove send watcher if it is set.""" - if self.sender is not None: - gobject.source_remove(self.sender) - self.sender = None - - def handle(self, fd, flags): - """Dispatch based on current flags.""" - if flags & (gobject.IO_ERR | gobject.IO_HUP): - return self.close() - if flags & gobject.IO_IN: - return self.io_in() - if flags & gobject.IO_OUT: - return self.io_out() - logger.error('Unknown flag: %s', flags) - return False - - def io_in(self): - """Record any incoming data to buffer and parse lines.""" - data = self.sock.recv(1024) - self.recv_buffer += data # XXX limit buffer size? - if data: - return self.parse_lines() - else: - return self.close() - - def io_out(self): - """Send as much of outgoing buffer as possible.""" - if self.send_buffer: - sent = self.sock.send(self.send_buffer) - self.send_buffer = self.send_buffer[sent:] - if not self.send_buffer: - self.clear_send_watch() - return True - - def close(self): - """Close connection.""" - logger.debug('Closing connection from %s', self.addr) - self.clear_send_watch() - self.sock.close() - return False - - def release(self): - """Forget about socket so that other loop can take over FD. - - Note that other code will still need to keep a ref to the socket in - order to prevent GC cleanup closing it. - """ - self.clear_recv_watch() - self.clear_send_watch() - return self.sock - - def send(self, data): - """Add raw data to send to outbound buffer.""" - self.add_send_watch() - self.send_buffer += data # XXX limit buffer size? - - def recv(self, line): - """Recv one and one line of request. Must be sub-classed.""" - raise NotImplementedError - - def parse_lines(self): - """Parse lines by splitting at terminator.""" - while self.terminator in self.recv_buffer: - line, self.recv_buffer = self.recv_buffer.split(self.terminator, 1) - self.recv(line) - return True - -class EchoHandler(BaseHandler): - """Basic handler used for debuging of Listener and Handler code itself.""" - def recv(self, line): - print repr(line) - self.send(line) - class Listener(object): """Setup listener and register it with gobject loop.""" - def __init__(self, addr, handler=EchoHandler): - self.handler = handler + def __init__(self, addr, session): + self.session = session self.sock = create_socket() self.sock.setblocking(0) self.sock.bind(addr) self.sock.listen(5) gobject.io_add_watch(self.sock.fileno(), gobject.IO_IN, self.handle) - logger.debug('Listening on %s using %s', addr, self.handler) + logger.debug('Listening on %s using %s', addr, self.session) def handle(self, fd, flags): - self.handler(self.sock.accept()) + sock, addr = self.sock.accept() + logger.debug('Got connection from %s', addr) + self.session.start(sock, addr) return True From 74aa96b300f9e498529fc9323f12c96bfb0aa6cc Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 17 Jun 2011 01:44:22 +0200 Subject: [PATCH 04/72] Moved mpd session to mopidy.frontends.mpd --- mopidy/frontends/mpd/__init__.py | 78 +++++++++++++++++----- mopidy/frontends/mpd/server.py | 76 --------------------- mopidy/utils/network.py | 6 +- tests/frontends/mpd/authentication_test.py | 2 +- tests/frontends/mpd/connection_test.py | 2 +- tests/frontends/mpd/server_test.py | 23 ------- 6 files changed, 66 insertions(+), 121 deletions(-) delete mode 100644 mopidy/frontends/mpd/server.py delete mode 100644 tests/frontends/mpd/server_test.py diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index b6088a41..d0ca761e 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -1,14 +1,19 @@ -import asyncore +import gobject import logging +import sys from pykka.actor import ThreadingActor from mopidy.frontends.base import BaseFrontend -from mopidy.frontends.mpd.server import MpdServer -from mopidy.utils.process import BaseThread +from mopidy import settings +from mopidy.utils import network +from mopidy.frontends.mpd.dispatcher import MpdDispatcher +from mopidy.frontends.mpd.protocol import ENCODING, VERSION, LINE_TERMINATOR +from mopidy.utils.log import indent logger = logging.getLogger('mopidy.frontends.mpd') +# FIXME no real need for frontend to be threading actor class MpdFrontend(ThreadingActor, BaseFrontend): """ The MPD frontend. @@ -25,22 +30,61 @@ class MpdFrontend(ThreadingActor, BaseFrontend): """ def __init__(self): - self._thread = None + hostname = network.format_hostname(settings.MPD_SERVER_HOSTNAME) + port = settings.MPD_SERVER_PORT + + try: + network.Listener(hostname, port, session=MpdSession) + except IOError, e: + logger.error(u'MPD server startup failed: %s', e) + sys.exit(1) + + logger.info(u'MPD server running at [%s]:%s', hostname, port) + + +class MpdSession(ThreadingActor): + """ + The MPD client session. Keeps track of a single client session. Any + requests from the client is passed on to the MPD request dispatcher. + """ + + def __init__(self, sock, addr): + self.sock = sock # Prevent premature GC of socket closing it + self.addr = addr + self.channel = gobject.IOChannel(sock.fileno()) + self.dispatcher = MpdDispatcher() def on_start(self): - self._thread = MpdThread() - self._thread.start() + try: + self.send_response([u'OK MPD %s' % VERSION]) + self.request_loop() + except gobject.GError: + self.stop() - def on_receive(self, message): - pass # Ignore any messages + def close(self): + self.channel.close() + def request_loop(self): + while True: + data = self.channel.readline() + if not data: + return self.close() + request = data.rstrip(LINE_TERMINATOR).decode(ENCODING) + logger.debug(u'Request from [%s]:%s: %s', self.addr[0], + self.addr[1], indent(request)) + response = self.dispatcher.handle_request(request) + self.send_response(response) -class MpdThread(BaseThread): - def __init__(self): - super(MpdThread, self).__init__() - self.name = u'MpdThread' - - def run_inside_try(self): - logger.debug(u'Starting MPD server thread') - server = MpdServer() - server.start() + def send_response(self, response): + """ + Format a response from the MPD command handlers and send it to the + client. + """ + if response: + response = LINE_TERMINATOR.join(response) + logger.debug(u'Response to [%s]:%s: %s', self.addr[0], + self.addr[1], indent(response)) + response = u'%s%s' % (response, LINE_TERMINATOR) + data = response.encode(ENCODING) + self.channel.write(data) + self.channel.flush() diff --git a/mopidy/frontends/mpd/server.py b/mopidy/frontends/mpd/server.py deleted file mode 100644 index 12e6a92a..00000000 --- a/mopidy/frontends/mpd/server.py +++ /dev/null @@ -1,76 +0,0 @@ -import logging -import sys - -import gobject - -from pykka.actor import ThreadingActor - -from mopidy import settings -from mopidy.utils import network -from mopidy.frontends.mpd.dispatcher import MpdDispatcher -from mopidy.frontends.mpd.protocol import ENCODING, LINE_TERMINATOR, VERSION -from mopidy.utils.log import indent - -logger = logging.getLogger('mopidy.frontends.mpd.server') - -class MpdServer(object): - """ - The MPD server. Creates a :class:`mopidy.frontends.mpd.session.MpdSession` - for each client connection. - """ - - def start(self): - """Start MPD server.""" - try: - hostname = network.format_hostname(settings.MPD_SERVER_HOSTNAME) - port = settings.MPD_SERVER_PORT - logger.debug(u'MPD server is binding to [%s]:%s', hostname, port) - network.Listener((hostname, port), session=MpdSession) - logger.info(u'MPD server running at [%s]:%s', hostname, port) - except IOError, e: - logger.error(u'MPD server startup failed: %s' % - str(e).decode('utf-8')) - sys.exit(1) - - -class MpdSession(ThreadingActor): - """ - The MPD client session. Keeps track of a single client session. Any - requests from the client is passed on to the MPD request dispatcher. - """ - - def __init__(self, sock, addr): - self.sock = sock # Prevent premature GC - self.addr = addr - self.channel = gobject.IOChannel(sock.fileno()) - self.dispatcher = MpdDispatcher(session=self) - - def on_start(self): - try: - self.send_response([u'OK MPD %s' % VERSION]) - self.request_loop() - except gobject.GError, e: - self.stop() - - def request_loop(self): - while True: - logger.debug('Trying to readline') - request = self.channel.readline()[:-1].decode(ENCODING) - logger.debug(u'Request from [%s]:%s: %s', self.addr[0], - self.addr[1], indent(request)) - response = self.dispatcher.handle_request(request) - self.send_response(response) - - def send_response(self, response): - """ - Format a response from the MPD command handlers and send it to the - client. - """ - if response: - response = LINE_TERMINATOR.join(response) - logger.debug(u'Response to [%s]:%s: %s', self.addr[0], - self.addr[1], indent(response)) - response = u'%s%s' % (response, LINE_TERMINATOR) - data = response.encode(ENCODING) - self.channel.write(data) - self.channel.flush() diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index df4f9292..d1536afb 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -39,15 +39,15 @@ def format_hostname(hostname): class Listener(object): """Setup listener and register it with gobject loop.""" - def __init__(self, addr, session): + def __init__(self, host, port, session): self.session = session self.sock = create_socket() self.sock.setblocking(0) - self.sock.bind(addr) + self.sock.bind((host, port)) self.sock.listen(5) gobject.io_add_watch(self.sock.fileno(), gobject.IO_IN, self.handle) - logger.debug('Listening on %s using %s', addr, self.session) + logger.debug('Listening on [%s]:%s using %s', host, port, self.session) def handle(self, fd, flags): sock, addr = self.sock.accept() diff --git a/tests/frontends/mpd/authentication_test.py b/tests/frontends/mpd/authentication_test.py index d795d726..7d340071 100644 --- a/tests/frontends/mpd/authentication_test.py +++ b/tests/frontends/mpd/authentication_test.py @@ -3,7 +3,7 @@ import unittest from mopidy import settings from mopidy.frontends.mpd.dispatcher import MpdDispatcher -from mopidy.frontends.mpd.session import MpdSession +from mopidy.frontends.mpd import MpdSession class AuthenticationTest(unittest.TestCase): def setUp(self): diff --git a/tests/frontends/mpd/connection_test.py b/tests/frontends/mpd/connection_test.py index bc995a5e..3f6b00f9 100644 --- a/tests/frontends/mpd/connection_test.py +++ b/tests/frontends/mpd/connection_test.py @@ -4,7 +4,7 @@ import unittest from mopidy import settings from mopidy.backends.dummy import DummyBackend from mopidy.frontends.mpd.dispatcher import MpdDispatcher -from mopidy.frontends.mpd.session import MpdSession +from mopidy.frontends.mpd import MpdSession from mopidy.mixers.dummy import DummyMixer class ConnectionHandlerTest(unittest.TestCase): diff --git a/tests/frontends/mpd/server_test.py b/tests/frontends/mpd/server_test.py deleted file mode 100644 index b2e27559..00000000 --- a/tests/frontends/mpd/server_test.py +++ /dev/null @@ -1,23 +0,0 @@ -import unittest - -from mopidy import settings -from mopidy.backends.dummy import DummyBackend -from mopidy.frontends.mpd import server -from mopidy.mixers.dummy import DummyMixer - -class MpdSessionTest(unittest.TestCase): - def setUp(self): - self.backend = DummyBackend.start().proxy() - self.mixer = DummyMixer.start().proxy() - self.session = server.MpdSession(None, None, (None, None)) - - def tearDown(self): - self.backend.stop().get() - self.mixer.stop().get() - settings.runtime.clear() - - def test_found_terminator_catches_decode_error(self): - # Pressing Ctrl+C in a telnet session sends a 0xff byte to the server. - self.session.input_buffer = ['\xff'] - self.session.found_terminator() - self.assertEqual(len(self.session.input_buffer), 0) From 54f09b0157ea9747b8b9bc8b623fa47748b46231 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 22 Jun 2011 02:49:02 +0200 Subject: [PATCH 05/72] Rewrite of client part of listener - takes into account that we will be implementing idle --- mopidy/frontends/mpd/__init__.py | 50 +++---------- mopidy/utils/network.py | 116 +++++++++++++++++++++++++++---- 2 files changed, 114 insertions(+), 52 deletions(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index d0ca761e..3b6b5db1 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -1,4 +1,3 @@ -import gobject import logging import sys @@ -9,7 +8,6 @@ from mopidy import settings from mopidy.utils import network from mopidy.frontends.mpd.dispatcher import MpdDispatcher from mopidy.frontends.mpd.protocol import ENCODING, VERSION, LINE_TERMINATOR -from mopidy.utils.log import indent logger = logging.getLogger('mopidy.frontends.mpd') @@ -34,7 +32,7 @@ class MpdFrontend(ThreadingActor, BaseFrontend): port = settings.MPD_SERVER_PORT try: - network.Listener(hostname, port, session=MpdSession) + network.Listener(hostname, port, MpdSession) except IOError, e: logger.error(u'MPD server startup failed: %s', e) sys.exit(1) @@ -42,49 +40,21 @@ class MpdFrontend(ThreadingActor, BaseFrontend): logger.info(u'MPD server running at [%s]:%s', hostname, port) -class MpdSession(ThreadingActor): +class MpdSession(network.LineProtocol): """ The MPD client session. Keeps track of a single client session. Any requests from the client is passed on to the MPD request dispatcher. """ + terminator = LINE_TERMINATOR + encoding = ENCODING + def __init__(self, sock, addr): - self.sock = sock # Prevent premature GC of socket closing it - self.addr = addr - self.channel = gobject.IOChannel(sock.fileno()) - self.dispatcher = MpdDispatcher() + super(MpdSession, self).__init__(sock, addr) + self.dispatcher = MpdDispatcher(self) def on_start(self): - try: - self.send_response([u'OK MPD %s' % VERSION]) - self.request_loop() - except gobject.GError: - self.stop() + self.send_lines([u'OK MPD %s' % VERSION]) - def close(self): - self.channel.close() - - def request_loop(self): - while True: - data = self.channel.readline() - if not data: - return self.close() - request = data.rstrip(LINE_TERMINATOR).decode(ENCODING) - logger.debug(u'Request from [%s]:%s: %s', self.addr[0], - self.addr[1], indent(request)) - response = self.dispatcher.handle_request(request) - self.send_response(response) - - def send_response(self, response): - """ - Format a response from the MPD command handlers and send it to the - client. - """ - if response: - response = LINE_TERMINATOR.join(response) - logger.debug(u'Response to [%s]:%s: %s', self.addr[0], - self.addr[1], indent(response)) - response = u'%s%s' % (response, LINE_TERMINATOR) - data = response.encode(ENCODING) - self.channel.write(data) - self.channel.flush() + def on_line_recieved(self, line): + self.send_lines(self.dispatcher.handle_request(line)) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index d1536afb..3b597e36 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -3,6 +3,10 @@ import re import socket import gobject +from pykka.actor import ThreadingActor + +from mopidy.utils.log import indent + logger = logging.getLogger('mopidy.utils.server') def _try_ipv6_socket(): @@ -39,19 +43,107 @@ def format_hostname(hostname): class Listener(object): """Setup listener and register it with gobject loop.""" - def __init__(self, host, port, session): - self.session = session - self.sock = create_socket() - self.sock.setblocking(0) - self.sock.bind((host, port)) - self.sock.listen(5) - gobject.io_add_watch(self.sock.fileno(), gobject.IO_IN, self.handle) - logger.debug('Listening on [%s]:%s using %s', host, port, self.session) + def __init__(self, host, port, protcol): + self.protcol = protcol + self.listener = create_socket() + self.listener.setblocking(False) + self.listener.bind((host, port)) + self.listener.listen(1) + + gobject.io_add_watch( + self.listener.fileno(), gobject.IO_IN, self.handle_accept) + logger.debug('Listening on [%s]:%s using %s as protcol handler', + host, port, self.protcol.__name__) + + def handle_accept(self, fd, flags): + sock, addr = self.listener.accept() + sock.setblocking(False) + + actor_ref = self.protcol.start(sock, addr) + gobject.io_add_watch(sock.fileno(), gobject.IO_IN | gobject.IO_ERR | + gobject.IO_HUP, self.handle_client, sock, actor_ref) - def handle(self, fd, flags): - sock, addr = self.sock.accept() - logger.debug('Got connection from %s', addr) - self.session.start(sock, addr) return True + def handle_client(self, fd, flags, sock, actor_ref): + if flags & (gobject.IO_ERR | gobject.IO_HUP): + data = '' + else: + data = sock.recv(1024) + + if not data: + actor_ref.stop() + return False + + actor_ref.send_one_way({'recvieved': data}) + return True + + +class LineProtocol(ThreadingActor): + terminator = '\n' + encoding = 'utf-8' + + def __init__(self, sock, addr): + self.sock = sock + self.host, self.port = addr + self.recv_buffer = '' + + def on_line_recieved(self, line): + raise NotImplemented + + def on_receive(self, message): + if 'recvieved' not in message: + return + + for line in self.parse_lines(message['recvieved']): + line = self.encode(line) + self.log_request(line) + self.on_line_recieved(line) + + def on_stop(self): + try: + self.sock.close() + except socket.error as e: + pass + + def parse_lines(self, new_data=None): + if new_data: + self.recv_buffer += new_data + while self.terminator in self.recv_buffer: + line, self.recv_buffer = self.recv_buffer.split(self.terminator, 1) + yield line + + def log_request(self, request): + logger.debug(u'Request from [%s]:%s: %s', + self.host, self.port, indent(request)) + + def log_response(self, response): + logger.debug(u'Response to [%s]:%s: %s', + self.host, self.port, indent(response)) + + def encode(self, line): + if self.encoding: + return line.encode(self.encoding) + return line + + def decode(self, line): + if self.encoding: + return line.decode(self.encoding) + return line + + def send_lines(self, lines): + if not lines: + return + + data = self.terminator.join(lines) + self.log_response(data) + self.send_raw(self.encode(data + self.terminator)) + + def send_raw(self, data): + try: + sent = self.sock.send(data) + assert len(data) == sent, 'All data was not sent' # FIXME + except socket.error as e: # FIXME + logger.debug('send() failed with: %s', e) + self.stop() From 14a7a9bd80a07c498a7165e736165f1024b18d85 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 22 Jun 2011 03:00:30 +0200 Subject: [PATCH 06/72] Get rid of GObjectEventThread --- mopidy/core.py | 16 +++++++--------- mopidy/utils/process.py | 25 ------------------------- 2 files changed, 7 insertions(+), 34 deletions(-) diff --git a/mopidy/core.py b/mopidy/core.py index 65472a29..4420c319 100644 --- a/mopidy/core.py +++ b/mopidy/core.py @@ -2,7 +2,9 @@ import logging import optparse import signal import sys -import time + +import gobject +gobject.threads_init() # Extract any non-GStreamer arguments, and leave the GStreamer arguments for # processing by GStreamer. This needs to be done before GStreamer is imported, @@ -23,25 +25,23 @@ from mopidy.gstreamer import GStreamer from mopidy.utils import get_class from mopidy.utils.log import setup_logging from mopidy.utils.path import get_or_create_folder, get_or_create_file -from mopidy.utils.process import (GObjectEventThread, exit_handler, - stop_all_actors) +from mopidy.utils.process import exit_handler, stop_all_actors from mopidy.utils.settings import list_settings_optparse_callback logger = logging.getLogger('mopidy.core') def main(): signal.signal(signal.SIGTERM, exit_handler) + loop = gobject.MainLoop() try: options = parse_options() setup_logging(options.verbosity_level, options.save_debug_log) setup_settings(options.interactive) - setup_gobject_loop() setup_gstreamer() setup_mixer() setup_backend() setup_frontends() - while True: - time.sleep(1) + loop.run() except SettingsError as e: logger.error(e.message) except KeyboardInterrupt: @@ -49,6 +49,7 @@ def main(): except Exception as e: logger.exception(e) finally: + loop.quit() stop_all_actors() def parse_options(): @@ -82,9 +83,6 @@ def setup_settings(interactive): logger.error(e.message) sys.exit(1) -def setup_gobject_loop(): - GObjectEventThread().start() - def setup_gstreamer(): GStreamer.start() diff --git a/mopidy/utils/process.py b/mopidy/utils/process.py index c1d1c9f5..f9577496 100644 --- a/mopidy/utils/process.py +++ b/mopidy/utils/process.py @@ -3,9 +3,6 @@ import signal import thread import threading -import gobject -gobject.threads_init() - from pykka import ActorDeadError from pykka.registry import ActorRegistry @@ -60,25 +57,3 @@ class BaseThread(threading.Thread): def run_inside_try(self): raise NotImplementedError - - -class GObjectEventThread(BaseThread): - """ - A GObject event loop which is shared by all Mopidy components that uses - libraries that need a GObject event loop, like GStreamer and D-Bus. - - Should be started by Mopidy's core and used by - :mod:`mopidy.output.gstreamer`, :mod:`mopidy.frontend.mpris`, etc. - """ - - def __init__(self): - super(GObjectEventThread, self).__init__() - self.name = u'GObjectEventThread' - self.loop = None - - def run_inside_try(self): - self.loop = gobject.MainLoop().run() - - def destroy(self): - self.loop.quit() - super(GObjectEventThread, self).destroy() From 742ecf10ae2989e177cd97526ed32f31f336b218 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 22 Jun 2011 03:04:46 +0200 Subject: [PATCH 07/72] Fix minor test regresion --- mopidy/frontends/mpd/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 3b6b5db1..89180d0a 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -58,3 +58,6 @@ class MpdSession(network.LineProtocol): def on_line_recieved(self, line): self.send_lines(self.dispatcher.handle_request(line)) + + def close(self): + self.stop() From 34203e2ba18d3572807faf98e6cd183925135339 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 23 Jun 2011 21:40:32 +0200 Subject: [PATCH 08/72] Reignore info sent to frontend --- mopidy/frontends/mpd/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 89180d0a..0432850d 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -39,6 +39,9 @@ class MpdFrontend(ThreadingActor, BaseFrontend): logger.info(u'MPD server running at [%s]:%s', hostname, port) + def on_receive(self, message): + pass # Ignore state info that is sent to frontend. + class MpdSession(network.LineProtocol): """ From b3e2e13a8db67030a44a05f2f012a81b27c6cd59 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 3 Jul 2011 22:37:45 +0200 Subject: [PATCH 09/72] sock.accept() on IPv6 systems is different --- mopidy/utils/network.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 3b597e36..04efac86 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -86,7 +86,7 @@ class LineProtocol(ThreadingActor): def __init__(self, sock, addr): self.sock = sock - self.host, self.port = addr + self.host, self.port = addr[:2] self.recv_buffer = '' def on_line_recieved(self, line): From 77c140f466023a1b26d9992444bcc1ca0c7073d3 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 3 Jul 2011 22:38:16 +0200 Subject: [PATCH 10/72] More low-level debug logging --- mopidy/utils/network.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 04efac86..e23e1d2f 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -96,6 +96,9 @@ class LineProtocol(ThreadingActor): if 'recvieved' not in message: return + logger.debug('Got %s from event-loop in %s', + repr(message['recvieved']), self.actor_urn) + for line in self.parse_lines(message['recvieved']): line = self.encode(line) self.log_request(line) @@ -115,12 +118,12 @@ class LineProtocol(ThreadingActor): yield line def log_request(self, request): - logger.debug(u'Request from [%s]:%s: %s', - self.host, self.port, indent(request)) + logger.debug(u'Request from [%s]:%s %s: %s', + self.host, self.port, self.actor_urn, indent(request)) def log_response(self, response): - logger.debug(u'Response to [%s]:%s: %s', - self.host, self.port, indent(response)) + logger.debug(u'Response to [%s]:%s %s: %s', + self.host, self.port, self.actor_urn, indent(response)) def encode(self, line): if self.encoding: From a3d72351d93cc89407df3470420179edd3604c30 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 4 Jul 2011 00:34:06 +0200 Subject: [PATCH 11/72] Stop all sesions when mpd frontend is asked to stop --- mopidy/frontends/mpd/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 1e18f6e2..3e6080e5 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -5,6 +5,7 @@ from pykka.actor import ThreadingActor from mopidy import settings from mopidy.utils import network +from mopidy.utils.process import stop_actors_by_class from mopidy.frontends.mpd.dispatcher import MpdDispatcher from mopidy.frontends.mpd.protocol import ENCODING, VERSION, LINE_TERMINATOR @@ -40,6 +41,9 @@ class MpdFrontend(ThreadingActor): def on_receive(self, message): pass # Ignore state info that is sent to frontend. + def on_stop(self): + stop_actors_by_class(MpdSession) + class MpdSession(network.LineProtocol): """ From 6e0d9905ed06bf25ea907f5885baca6d0dcc601f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 4 Jul 2011 00:40:50 +0200 Subject: [PATCH 12/72] Sort imports --- mopidy/frontends/mpd/__init__.py | 4 ++-- tests/frontends/mpd/authentication_test.py | 2 +- tests/frontends/mpd/connection_test.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 3e6080e5..3feb7478 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -4,10 +4,10 @@ import sys from pykka.actor import ThreadingActor from mopidy import settings -from mopidy.utils import network -from mopidy.utils.process import stop_actors_by_class from mopidy.frontends.mpd.dispatcher import MpdDispatcher from mopidy.frontends.mpd.protocol import ENCODING, VERSION, LINE_TERMINATOR +from mopidy.utils import network +from mopidy.utils.process import stop_actors_by_class logger = logging.getLogger('mopidy.frontends.mpd') diff --git a/tests/frontends/mpd/authentication_test.py b/tests/frontends/mpd/authentication_test.py index 7d340071..fb32ea54 100644 --- a/tests/frontends/mpd/authentication_test.py +++ b/tests/frontends/mpd/authentication_test.py @@ -2,8 +2,8 @@ import mock import unittest from mopidy import settings -from mopidy.frontends.mpd.dispatcher import MpdDispatcher from mopidy.frontends.mpd import MpdSession +from mopidy.frontends.mpd.dispatcher import MpdDispatcher class AuthenticationTest(unittest.TestCase): def setUp(self): diff --git a/tests/frontends/mpd/connection_test.py b/tests/frontends/mpd/connection_test.py index 3f6b00f9..82debabb 100644 --- a/tests/frontends/mpd/connection_test.py +++ b/tests/frontends/mpd/connection_test.py @@ -3,8 +3,8 @@ import unittest from mopidy import settings from mopidy.backends.dummy import DummyBackend -from mopidy.frontends.mpd.dispatcher import MpdDispatcher from mopidy.frontends.mpd import MpdSession +from mopidy.frontends.mpd.dispatcher import MpdDispatcher from mopidy.mixers.dummy import DummyMixer class ConnectionHandlerTest(unittest.TestCase): From e0ecc76e98068928677d0a6e2920cd8885cbbb25 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 4 Jul 2011 00:42:39 +0200 Subject: [PATCH 13/72] Import modules --- mopidy/frontends/mpd/__init__.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 3feb7478..64591e6a 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -4,10 +4,10 @@ import sys from pykka.actor import ThreadingActor from mopidy import settings -from mopidy.frontends.mpd.dispatcher import MpdDispatcher -from mopidy.frontends.mpd.protocol import ENCODING, VERSION, LINE_TERMINATOR +from mopidy.frontends.mpd import dispatcher +from mopidy.frontends.mpd import protocol from mopidy.utils import network -from mopidy.utils.process import stop_actors_by_class +from mopidy.utils import process logger = logging.getLogger('mopidy.frontends.mpd') @@ -42,7 +42,7 @@ class MpdFrontend(ThreadingActor): pass # Ignore state info that is sent to frontend. def on_stop(self): - stop_actors_by_class(MpdSession) + process.stop_actors_by_class(MpdSession) class MpdSession(network.LineProtocol): @@ -51,15 +51,15 @@ class MpdSession(network.LineProtocol): requests from the client is passed on to the MPD request dispatcher. """ - terminator = LINE_TERMINATOR - encoding = ENCODING + terminator = protocol.LINE_TERMINATOR + encoding = protocol.ENCODING def __init__(self, sock, addr): super(MpdSession, self).__init__(sock, addr) - self.dispatcher = MpdDispatcher(self) + self.dispatcher = dispatcher.MpdDispatcher(self) def on_start(self): - self.send_lines([u'OK MPD %s' % VERSION]) + self.send_lines([u'OK MPD %s' % protocol.VERSION]) def on_line_recieved(self, line): self.send_lines(self.dispatcher.handle_request(line)) From cfd48f8a5b8aaabaa01edc82501603ca404cd203 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 4 Jul 2011 00:46:46 +0200 Subject: [PATCH 14/72] Code style fix --- mopidy/frontends/mpd/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 64591e6a..00d60b00 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -6,8 +6,7 @@ from pykka.actor import ThreadingActor from mopidy import settings from mopidy.frontends.mpd import dispatcher from mopidy.frontends.mpd import protocol -from mopidy.utils import network -from mopidy.utils import process +from mopidy.utils import network, process logger = logging.getLogger('mopidy.frontends.mpd') From 224a0d1247301eb318bba81679a5bb0d4620efee Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 4 Jul 2011 00:56:57 +0200 Subject: [PATCH 15/72] Remove on_recieve from frontend --- mopidy/frontends/mpd/__init__.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 00d60b00..d6ff0a86 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -37,9 +37,6 @@ class MpdFrontend(ThreadingActor): logger.info(u'MPD server running at [%s]:%s', hostname, port) - def on_receive(self, message): - pass # Ignore state info that is sent to frontend. - def on_stop(self): process.stop_actors_by_class(MpdSession) From ab653b7539fbb4999da219f408e29e60096a5630 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 4 Jul 2011 10:33:59 +0200 Subject: [PATCH 16/72] Fix comments from pull-request --- mopidy/frontends/mpd/__init__.py | 3 +- mopidy/utils/network.py | 49 +++++++++++++++++++++----------- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index d6ff0a86..4599b31a 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -4,8 +4,7 @@ import sys from pykka.actor import ThreadingActor from mopidy import settings -from mopidy.frontends.mpd import dispatcher -from mopidy.frontends.mpd import protocol +from mopidy.frontends.mpd import dispatcher, protocol from mopidy.utils import network, process logger = logging.getLogger('mopidy.frontends.mpd') diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index e23e1d2f..1366e6b1 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -44,8 +44,8 @@ def format_hostname(hostname): class Listener(object): """Setup listener and register it with gobject loop.""" - def __init__(self, host, port, protcol): - self.protcol = protcol + def __init__(self, host, port, protocol): + self.protocol = protocol self.listener = create_socket() self.listener.setblocking(False) self.listener.bind((host, port)) @@ -53,14 +53,14 @@ class Listener(object): gobject.io_add_watch( self.listener.fileno(), gobject.IO_IN, self.handle_accept) - logger.debug('Listening on [%s]:%s using %s as protcol handler', - host, port, self.protcol.__name__) + logger.debug(u'Listening on [%s]:%s using %s as protocol handler', + host, port, self.protocol.__name__) def handle_accept(self, fd, flags): sock, addr = self.listener.accept() sock.setblocking(False) - actor_ref = self.protcol.start(sock, addr) + actor_ref = self.protocol.start(sock, addr) gobject.io_add_watch(sock.fileno(), gobject.IO_IN | gobject.IO_ERR | gobject.IO_HUP, self.handle_client, sock, actor_ref) @@ -76,7 +76,7 @@ class Listener(object): actor_ref.stop() return False - actor_ref.send_one_way({'recvieved': data}) + actor_ref.send_one_way({'received': data}) return True @@ -93,13 +93,13 @@ class LineProtocol(ThreadingActor): raise NotImplemented def on_receive(self, message): - if 'recvieved' not in message: + if 'received' not in message: return - logger.debug('Got %s from event-loop in %s', - repr(message['recvieved']), self.actor_urn) + logger.debug(u'Got %s from eventloop in %s', + repr(message['received']), self.actor_urn) - for line in self.parse_lines(message['recvieved']): + for line in self.parse_lines(message['received']): line = self.encode(line) self.log_request(line) self.on_line_recieved(line) @@ -107,7 +107,7 @@ class LineProtocol(ThreadingActor): def on_stop(self): try: self.sock.close() - except socket.error as e: + except socket.error: pass def parse_lines(self, new_data=None): @@ -118,11 +118,11 @@ class LineProtocol(ThreadingActor): yield line def log_request(self, request): - logger.debug(u'Request from [%s]:%s %s: %s', + logger.debug(u'Request from [%s]:%s to %s: %s', self.host, self.port, self.actor_urn, indent(request)) def log_response(self, response): - logger.debug(u'Response to [%s]:%s %s: %s', + logger.debug(u'Response to [%s]:%s %s: from %s', self.host, self.port, self.actor_urn, indent(response)) def encode(self, line): @@ -146,7 +146,24 @@ class LineProtocol(ThreadingActor): def send_raw(self, data): try: sent = self.sock.send(data) - assert len(data) == sent, 'All data was not sent' # FIXME - except socket.error as e: # FIXME - logger.debug('send() failed with: %s', e) + # FIXME we are assuming that sock send will not fail as the OS send + # buffer is big enough compared to our need. This can of course + # fail and will be caught and handeled fairly poorly with the + # following assert. + # + # Safer, and more complex way of handling this would be to ensure + # that data can be send by putting a data sender in the event loop + # and appending to its buffer. Once the buffer is empty the sender + # must be removed from the loop. This option is doable, but adds + # extra complexity. + # + # The other simpler option would be to try and recall raw_send with + # remaining data. Probably with a decrementing retry counter to + # prevent an inf. loop. + assert len(data) == sent, u'All data was not sent' + except socket.error as e: + # FIXME should this be handeled in a better maner, for instance + # retry? For instance would block errors and interupted system call + # errors would warrant a retry. + logger.debug(u'send() failed with: %s', e) self.stop() From c7617e150ade8dacb793b5d9a7f2c7dbc5e413f0 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 4 Jul 2011 21:08:49 +0200 Subject: [PATCH 17/72] Decode incomming data --- mopidy/utils/network.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 1366e6b1..7ded3e27 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -100,7 +100,7 @@ class LineProtocol(ThreadingActor): repr(message['received']), self.actor_urn) for line in self.parse_lines(message['received']): - line = self.encode(line) + line = self.decode(line) self.log_request(line) self.on_line_recieved(line) From 9a8f3a141ca25e374461c580689fe50a478d5ded Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 4 Jul 2011 21:09:21 +0200 Subject: [PATCH 18/72] Misplaced text in log message --- mopidy/utils/network.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 7ded3e27..69eaac2a 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -122,7 +122,7 @@ class LineProtocol(ThreadingActor): self.host, self.port, self.actor_urn, indent(request)) def log_response(self, response): - logger.debug(u'Response to [%s]:%s %s: from %s', + logger.debug(u'Response to [%s]:%s from %s: %s', self.host, self.port, self.actor_urn, indent(response)) def encode(self, line): From 3053ba09e003bb7a40988637ff5f04488b21984e Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 4 Jul 2011 21:13:46 +0200 Subject: [PATCH 19/72] Typo fix :) --- mopidy/frontends/mpd/__init__.py | 2 +- mopidy/utils/network.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 4599b31a..65299e9d 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -56,7 +56,7 @@ class MpdSession(network.LineProtocol): def on_start(self): self.send_lines([u'OK MPD %s' % protocol.VERSION]) - def on_line_recieved(self, line): + def on_line_received(self, line): self.send_lines(self.dispatcher.handle_request(line)) def close(self): diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 69eaac2a..50c7bfab 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -89,7 +89,7 @@ class LineProtocol(ThreadingActor): self.host, self.port = addr[:2] self.recv_buffer = '' - def on_line_recieved(self, line): + def on_line_received(self, line): raise NotImplemented def on_receive(self, message): @@ -102,7 +102,7 @@ class LineProtocol(ThreadingActor): for line in self.parse_lines(message['received']): line = self.decode(line) self.log_request(line) - self.on_line_recieved(line) + self.on_line_received(line) def on_stop(self): try: From 094850fe2071c5d83627a9f27dcac8be89a06db9 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 4 Jul 2011 21:24:09 +0200 Subject: [PATCH 20/72] Fix reference to mopidy.frontends.mpd.MpdSession --- mopidy/frontends/mpd/dispatcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/frontends/mpd/dispatcher.py b/mopidy/frontends/mpd/dispatcher.py index 18f994de..0f0f0299 100644 --- a/mopidy/frontends/mpd/dispatcher.py +++ b/mopidy/frontends/mpd/dispatcher.py @@ -178,7 +178,7 @@ class MpdContext(object): #: The current :class:`MpdDispatcher`. dispatcher = None - #: The current :class:`mopidy.frontends.mpd.session.MpdSession`. + #: The current :class:`mopidy.frontends.mpd.MpdSession`. session = None def __init__(self, dispatcher, session=None): From b311e4284054c3828d41559cfb6dbb042d142c30 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 5 Jul 2011 00:55:35 +0200 Subject: [PATCH 21/72] Try to document new server helper --- mopidy/utils/network.py | 63 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 50c7bfab..f89867ba 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -57,7 +57,7 @@ class Listener(object): host, port, self.protocol.__name__) def handle_accept(self, fd, flags): - sock, addr = self.listener.accept() + sock, addr = self.listener.accept() # FIXME this might fail is some rare cases. sock.setblocking(False) actor_ref = self.protocol.start(sock, addr) @@ -67,10 +67,16 @@ class Listener(object): return True def handle_client(self, fd, flags, sock, actor_ref): + """ + Read client data when possible. + + Returns false when reading failed in order to deregister with the event + loop. + """ if flags & (gobject.IO_ERR | gobject.IO_HUP): data = '' else: - data = sock.recv(1024) + data = sock.recv(1024) # FIXME there are cases where this might fail. if not data: actor_ref.stop() @@ -81,6 +87,16 @@ class Listener(object): class LineProtocol(ThreadingActor): + """ + Base class for handling line based protocols. + + Takes care of receiving new data from listener's client code, decoding and + then splitting data along line boundaries. + + Attributes ``terminator``and ``encoding`` can be set in case subclasses + want to split by another terminator or use another encoding. + """ + terminator = '\n' encoding = 'utf-8' @@ -90,13 +106,19 @@ class LineProtocol(ThreadingActor): self.recv_buffer = '' def on_line_received(self, line): + """ + Called whenever a new line is found. + + Should be implemented by subclasses. + """ raise NotImplemented def on_receive(self, message): + """Handle messages with new data from listener.""" if 'received' not in message: return - logger.debug(u'Got %s from eventloop in %s', + logger.debug(u'Got %s from event loop in %s', repr(message['received']), self.actor_urn) for line in self.parse_lines(message['received']): @@ -105,12 +127,14 @@ class LineProtocol(ThreadingActor): self.on_line_received(line) def on_stop(self): + """Ensure that socket is closed when actor stops.""" try: self.sock.close() except socket.error: pass def parse_lines(self, new_data=None): + """Consume new data and yield any lines found.""" if new_data: self.recv_buffer += new_data while self.terminator in self.recv_buffer: @@ -118,24 +142,50 @@ class LineProtocol(ThreadingActor): yield line def log_request(self, request): + """ + Log request for debug purposes. + + Can be overridden by subclasses to change logging behaviour. + """ logger.debug(u'Request from [%s]:%s to %s: %s', self.host, self.port, self.actor_urn, indent(request)) def log_response(self, response): + """ + Log response for debug purposes. + + Can be overridden by subclasses to change logging behaviour. + """ logger.debug(u'Response to [%s]:%s from %s: %s', self.host, self.port, self.actor_urn, indent(response)) def encode(self, line): + """ + Handle encoding of line. + + Can be overridden by subclasses to change encoding behaviour. + """ if self.encoding: return line.encode(self.encoding) return line def decode(self, line): + """ + Handle decoding of line. + + Can be overridden by subclasses to change decoding behaviour. + """ if self.encoding: return line.decode(self.encoding) return line def send_lines(self, lines): + """ + Send array of lines to client. + + Join lines using the terminator that is set for this class, encode it + and send it to the client. + """ if not lines: return @@ -144,11 +194,12 @@ class LineProtocol(ThreadingActor): self.send_raw(self.encode(data + self.terminator)) def send_raw(self, data): + """Send data to client exactly as is.""" try: sent = self.sock.send(data) # FIXME we are assuming that sock send will not fail as the OS send # buffer is big enough compared to our need. This can of course - # fail and will be caught and handeled fairly poorly with the + # fail and will be caught and handled fairly poorly with the # following assert. # # Safer, and more complex way of handling this would be to ensure @@ -162,8 +213,8 @@ class LineProtocol(ThreadingActor): # prevent an inf. loop. assert len(data) == sent, u'All data was not sent' except socket.error as e: - # FIXME should this be handeled in a better maner, for instance - # retry? For instance would block errors and interupted system call + # FIXME should this be handled in a better manner, for instance + # retry? For instance would block errors and interrupted system call # errors would warrant a retry. logger.debug(u'send() failed with: %s', e) self.stop() From 79e46ab4fa4eb3b293e85516a2283212416e725c Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 5 Jul 2011 00:56:29 +0200 Subject: [PATCH 22/72] Rename listemer to server --- mopidy/frontends/mpd/__init__.py | 2 +- mopidy/utils/network.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 65299e9d..8dbbf3db 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -29,7 +29,7 @@ class MpdFrontend(ThreadingActor): port = settings.MPD_SERVER_PORT try: - network.Listener(hostname, port, MpdSession) + network.Server(hostname, port, protocol=MpdSession) except IOError, e: logger.error(u'MPD server startup failed: %s', e) sys.exit(1) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index f89867ba..6e8b5b44 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -41,7 +41,7 @@ def format_hostname(hostname): hostname = '::ffff:%s' % hostname return hostname -class Listener(object): +class Server(object): """Setup listener and register it with gobject loop.""" def __init__(self, host, port, protocol): From cb2f0df5d6a0d88af1d8a49a59e279583eb7f2fe Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 5 Jul 2011 01:13:12 +0200 Subject: [PATCH 23/72] Extract logging of raw data to method --- mopidy/utils/network.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 6e8b5b44..1fed885a 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -118,8 +118,7 @@ class LineProtocol(ThreadingActor): if 'received' not in message: return - logger.debug(u'Got %s from event loop in %s', - repr(message['received']), self.actor_urn) + self.log_raw_data(message['received']) for line in self.parse_lines(message['received']): line = self.decode(line) @@ -141,6 +140,15 @@ class LineProtocol(ThreadingActor): line, self.recv_buffer = self.recv_buffer.split(self.terminator, 1) yield line + def log_raw_data(self, data): + """ + Log raw data from event loopfor debug purposes. + + Can be overridden by subclasses to change logging behaviour. + """ + logger.debug(u'Got %s from event loop in %s', + repr(data), self.actor_urn) + def log_request(self, request): """ Log request for debug purposes. From a12e9779e39ae9075299df3f0d2dd7679e0f1dec Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 5 Jul 2011 23:47:23 +0200 Subject: [PATCH 24/72] Add some better error handling for accept call --- mopidy/utils/network.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 1fed885a..13bbd2bf 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -57,7 +57,13 @@ class Server(object): host, port, self.protocol.__name__) def handle_accept(self, fd, flags): - sock, addr = self.listener.accept() # FIXME this might fail is some rare cases. + try: + sock, addr = self.listener.accept() + except socket.error as e: + if e.errno in (errno.EAGAIN, errno.EINTR): + return True # i.e. retry + raise + sock.setblocking(False) actor_ref = self.protocol.start(sock, addr) From 52087bd5b4cf47cb15927e0a4ff197ed527d4c40 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 6 Jul 2011 00:23:18 +0200 Subject: [PATCH 25/72] Cleanup recv code a bit --- mopidy/utils/network.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 13bbd2bf..542baaa0 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -80,9 +80,16 @@ class Server(object): loop. """ if flags & (gobject.IO_ERR | gobject.IO_HUP): - data = '' - else: - data = sock.recv(1024) # FIXME there are cases where this might fail. + actor_ref.stop() + return False + + try: + data = sock.recv(4096) + except socket.error as e: + if e.errno in (errno.EWOULDBLOCK, errno.EAGAIN): + return True + actor_ref.stop() + return False if not data: actor_ref.stop() @@ -148,7 +155,7 @@ class LineProtocol(ThreadingActor): def log_raw_data(self, data): """ - Log raw data from event loopfor debug purposes. + Log raw data from event loop for debug purposes. Can be overridden by subclasses to change logging behaviour. """ From a0f6ba7dc451e552628ff493ba3b1b39fdef66ac Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 6 Jul 2011 00:23:54 +0200 Subject: [PATCH 26/72] Switch to thread safe send queue and use event loop to send data --- mopidy/utils/network.py | 49 +++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 542baaa0..89d573a3 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -2,6 +2,7 @@ import logging import re import socket import gobject +import Queue as queue from pykka.actor import ThreadingActor @@ -115,8 +116,9 @@ class LineProtocol(ThreadingActor): def __init__(self, sock, addr): self.sock = sock - self.host, self.port = addr[:2] + self.host, self.port = addr[:2] # IPv6 has larger addr self.recv_buffer = '' + self.send_queue = queue.Queue() def on_line_received(self, line): """ @@ -216,26 +218,31 @@ class LineProtocol(ThreadingActor): def send_raw(self, data): """Send data to client exactly as is.""" + start_sender = self.send_queue.empty() + + self.send_queue.put(data) + + if start_sender: + gobject.io_add_watch(self.sock.fileno(), gobject.IO_OUT | + gobject.IO_ERR | gobject.IO_HUP, self._send) + + def _send(self, fd, flags): + # NOTE: This code is _not_ run in the actor's thread, but in the same + # one as the event loop. If this blocks, rest of gobject code will + # likely be blocked as well... try: + data = self.send_queue.get_nowait() sent = self.sock.send(data) - # FIXME we are assuming that sock send will not fail as the OS send - # buffer is big enough compared to our need. This can of course - # fail and will be caught and handled fairly poorly with the - # following assert. - # - # Safer, and more complex way of handling this would be to ensure - # that data can be send by putting a data sender in the event loop - # and appending to its buffer. Once the buffer is empty the sender - # must be removed from the loop. This option is doable, but adds - # extra complexity. - # - # The other simpler option would be to try and recall raw_send with - # remaining data. Probably with a decrementing retry counter to - # prevent an inf. loop. - assert len(data) == sent, u'All data was not sent' + except queue.Empty: + return False # No more data to send, remove callback except socket.error as e: - # FIXME should this be handled in a better manner, for instance - # retry? For instance would block errors and interrupted system call - # errors would warrant a retry. - logger.debug(u'send() failed with: %s', e) - self.stop() + if e.errno in (errno.EAGAIN, errno.EWOULDBLOCK): + self.send_queue.put(data) + return True + self.actor_ref.stop() + return False + + if len(data) != sent: # Retry remaining data + self.send_queue.put(data[sent:]) + + return not self.send_queue.empty() From 4cd6f5f66cb5d0a0854438cf054739116b921057 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 7 Jul 2011 21:50:03 +0200 Subject: [PATCH 27/72] Switch to lock based protection of send buffer, queue use was flawed --- mopidy/utils/network.py | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 89d573a3..76bebc1d 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -1,8 +1,8 @@ +import gobject import logging import re import socket -import gobject -import Queue as queue +import threading from pykka.actor import ThreadingActor @@ -117,8 +117,9 @@ class LineProtocol(ThreadingActor): def __init__(self, sock, addr): self.sock = sock self.host, self.port = addr[:2] # IPv6 has larger addr + self.send_lock = threading.Lock() self.recv_buffer = '' - self.send_queue = queue.Queue() + self.send_buffer = '' def on_line_received(self, line): """ @@ -218,11 +219,12 @@ class LineProtocol(ThreadingActor): def send_raw(self, data): """Send data to client exactly as is.""" - start_sender = self.send_queue.empty() + self.send_lock.acquire(True) + should_register_sender = len(self.send_buffer) == 0 + self.send_buffer += data + self.send_lock.release() - self.send_queue.put(data) - - if start_sender: + if should_register_sender: gobject.io_add_watch(self.sock.fileno(), gobject.IO_OUT | gobject.IO_ERR | gobject.IO_HUP, self._send) @@ -230,19 +232,20 @@ class LineProtocol(ThreadingActor): # NOTE: This code is _not_ run in the actor's thread, but in the same # one as the event loop. If this blocks, rest of gobject code will # likely be blocked as well... + + # If with can't get the lock, simply try again next time socket is + # ready for sending. + if not self.send_lock.acquire(False): + return True + try: - data = self.send_queue.get_nowait() - sent = self.sock.send(data) - except queue.Empty: - return False # No more data to send, remove callback + sent = self.sock.send(self.send_buffer) + self.send_buffer = self.send_buffer[sent:] + return bool(self.send_buffer) except socket.error as e: if e.errno in (errno.EAGAIN, errno.EWOULDBLOCK): - self.send_queue.put(data) return True self.actor_ref.stop() return False - - if len(data) != sent: # Retry remaining data - self.send_queue.put(data[sent:]) - - return not self.send_queue.empty() + finally: + self.send_lock.release() From 2449308758b27dda3b83adf11151553dc54c83e6 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 7 Jul 2011 23:47:28 +0200 Subject: [PATCH 28/72] Use re for finding terminator and splitting as it does not assume unicode --- mopidy/utils/network.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 76bebc1d..a9cde77d 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -120,6 +120,7 @@ class LineProtocol(ThreadingActor): self.send_lock = threading.Lock() self.recv_buffer = '' self.send_buffer = '' + self.terminator_re = re.compile(self.terminator) def on_line_received(self, line): """ @@ -152,8 +153,9 @@ class LineProtocol(ThreadingActor): """Consume new data and yield any lines found.""" if new_data: self.recv_buffer += new_data - while self.terminator in self.recv_buffer: - line, self.recv_buffer = self.recv_buffer.split(self.terminator, 1) + while self.terminator_re.search(self.recv_buffer): + line, self.recv_buffer = self.terminator_re.split( + self.recv_buffer, 1) yield line def log_raw_data(self, data): From 12633e0d4ab94671f8958b14c67dc1b94346ed11 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 7 Jul 2011 23:52:38 +0200 Subject: [PATCH 29/72] Add log_error to LineProtocol --- mopidy/utils/network.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index a9cde77d..1de582b8 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -185,6 +185,15 @@ class LineProtocol(ThreadingActor): logger.debug(u'Response to [%s]:%s from %s: %s', self.host, self.port, self.actor_urn, indent(response)) + def log_error(self, error): + """ + Log error for debug purposes. + + Can be overridden by subclasses to change logging behaviour. + """ + logger.warning('Problem with connection to [%s]:%s in %s: %s', + self.host, self.port, self.actor_urn, error) + def encode(self, line): """ Handle encoding of line. @@ -247,6 +256,7 @@ class LineProtocol(ThreadingActor): except socket.error as e: if e.errno in (errno.EAGAIN, errno.EWOULDBLOCK): return True + self.log_error(e) self.actor_ref.stop() return False finally: From 7c2ccbbaa13299e5858cfc066c63add22fa1a016 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 7 Jul 2011 23:55:54 +0200 Subject: [PATCH 30/72] Document attributes correctly --- mopidy/utils/network.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 1de582b8..1bee3ba1 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -106,12 +106,12 @@ class LineProtocol(ThreadingActor): Takes care of receiving new data from listener's client code, decoding and then splitting data along line boundaries. - - Attributes ``terminator``and ``encoding`` can be set in case subclasses - want to split by another terminator or use another encoding. """ + #: What terminator to use to split lines. terminator = '\n' + + #: What encoding to expect incomming data to be in, can be :class:`None`. encoding = 'utf-8' def __init__(self, sock, addr): From 26155d1d402f05b915e7831df1c35b6c65372d4f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 7 Jul 2011 23:57:20 +0200 Subject: [PATCH 31/72] Fix s/listener/server/ in docs --- mopidy/utils/network.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 1bee3ba1..69033cea 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -43,7 +43,7 @@ def format_hostname(hostname): return hostname class Server(object): - """Setup listener and register it with gobject loop.""" + """Setup listener and register it with gobject's event loop.""" def __init__(self, host, port, protocol): self.protocol = protocol @@ -104,7 +104,7 @@ class LineProtocol(ThreadingActor): """ Base class for handling line based protocols. - Takes care of receiving new data from listener's client code, decoding and + Takes care of receiving new data from server's client code, decoding and then splitting data along line boundaries. """ @@ -131,7 +131,7 @@ class LineProtocol(ThreadingActor): raise NotImplemented def on_receive(self, message): - """Handle messages with new data from listener.""" + """Handle messages with new data from server.""" if 'received' not in message: return From 8cc1aa07d97e2894214c8fe8790f794907c7b679 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 8 Jul 2011 00:07:14 +0200 Subject: [PATCH 32/72] Forgot to include errno --- mopidy/utils/network.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 69033cea..f539b84d 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -1,3 +1,4 @@ +import errno import gobject import logging import re From 63244b9af8dcdf34bf3071dab9b230a6105585c1 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 8 Jul 2011 00:08:07 +0200 Subject: [PATCH 33/72] Limit number of allowed connections --- mopidy/utils/network.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index f539b84d..4497c479 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -6,6 +6,7 @@ import socket import threading from pykka.actor import ThreadingActor +from pykka.registry import ActorRegistry from mopidy.utils.log import indent @@ -46,8 +47,9 @@ def format_hostname(hostname): class Server(object): """Setup listener and register it with gobject's event loop.""" - def __init__(self, host, port, protocol): + def __init__(self, host, port, protocol, max_connections=15): self.protocol = protocol + self.max_connections = max_connections self.listener = create_socket() self.listener.setblocking(False) self.listener.bind((host, port)) @@ -66,6 +68,15 @@ class Server(object): return True # i.e. retry raise + num_connections = len(ActorRegistry.get_by_class(self.protocol)) + if self.max_connections and num_connections >= self.max_connections: + logger.warning(u'Rejected connection from [%s]:%s', addr[0], addr[1]) + try: + sock.close() + except socket.error: + pass + return True + sock.setblocking(False) actor_ref = self.protocol.start(sock, addr) @@ -192,7 +203,7 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change logging behaviour. """ - logger.warning('Problem with connection to [%s]:%s in %s: %s', + logger.warning(u'Problem with connection to [%s]:%s in %s: %s', self.host, self.port, self.actor_urn, error) def encode(self, line): From 7f77fe38d5fd5ba05e39476b5f10e27fae5d0740 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 8 Jul 2011 00:28:01 +0200 Subject: [PATCH 34/72] Add timeout support to LineProtocol --- mopidy/utils/network.py | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 4497c479..fb4b6f4d 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -126,6 +126,10 @@ class LineProtocol(ThreadingActor): #: What encoding to expect incomming data to be in, can be :class:`None`. encoding = 'utf-8' + #: How long to wait before disconnecting client due to inactivity in + #: milliseconds. + timeout = 30000 + def __init__(self, sock, addr): self.sock = sock self.host, self.port = addr[:2] # IPv6 has larger addr @@ -133,6 +137,9 @@ class LineProtocol(ThreadingActor): self.recv_buffer = '' self.send_buffer = '' self.terminator_re = re.compile(self.terminator) + self.timeout_id = None + + self.enable_timeout() def on_line_received(self, line): """ @@ -147,6 +154,7 @@ class LineProtocol(ThreadingActor): if 'received' not in message: return + self.disable_timeout() self.log_raw_data(message['received']) for line in self.parse_lines(message['received']): @@ -154,13 +162,26 @@ class LineProtocol(ThreadingActor): self.log_request(line) self.on_line_received(line) + self.enable_timeout() + def on_stop(self): - """Ensure that socket is closed when actor stops.""" + """Ensure that cleanup when actor stops.""" + self.disable_timeout() try: self.sock.close() except socket.error: pass + def disable_timeout(self): + """Deactivate timeout mechanism.""" + if self.timeout_id: + gobject.source_remove(self.timeout_id) + + def enable_timeout(self): + """Reactivate timeout mechanism.""" + self.disable_timeout() + self.timeout_id = gobject.timeout_add(self.timeout, self._timeout) + def parse_lines(self, new_data=None): """Consume new data and yield any lines found.""" if new_data: @@ -206,6 +227,15 @@ class LineProtocol(ThreadingActor): logger.warning(u'Problem with connection to [%s]:%s in %s: %s', self.host, self.port, self.actor_urn, error) + def log_timeout(self): + """ + Log timeout for debug purposes. + + Can be overridden by subclasses to change logging behaviour. + """ + logger.debug(u'Closing connection to [%s]:%s in %s due to timeout.', + self.host, self.port, self.actor_urn) + def encode(self, line): """ Handle encoding of line. @@ -273,3 +303,9 @@ class LineProtocol(ThreadingActor): return False finally: self.send_lock.release() + + def _timeout(self): + # NOTE: This code is _not_ run in the actor's thread... + self.log_timeout() + self.actor_ref.stop() + return False From cdb68d61f5b92e38f72cab9b957ef0c6c7d5f67c Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 9 Jul 2011 00:46:56 +0200 Subject: [PATCH 35/72] Use timeout_add_seconds which is less accurate but more efficient --- mopidy/utils/network.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index fb4b6f4d..ed9c7161 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -127,8 +127,8 @@ class LineProtocol(ThreadingActor): encoding = 'utf-8' #: How long to wait before disconnecting client due to inactivity in - #: milliseconds. - timeout = 30000 + #: seconds. + timeout = 30 def __init__(self, sock, addr): self.sock = sock @@ -180,7 +180,8 @@ class LineProtocol(ThreadingActor): def enable_timeout(self): """Reactivate timeout mechanism.""" self.disable_timeout() - self.timeout_id = gobject.timeout_add(self.timeout, self._timeout) + self.timeout_id = gobject.timeout_add_seconds(self.timeout, + self._timeout) def parse_lines(self, new_data=None): """Consume new data and yield any lines found.""" From 22ebb1bc297bbbb37f7cb3c31bd259129d042412 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 9 Jul 2011 14:28:35 +0200 Subject: [PATCH 36/72] Move recv code to LineProtocol and add source removal Fixes problem where timed out sockets where not being removed from event loop causing excess CPU usage. --- mopidy/utils/network.py | 99 +++++++++++++++++++++++++---------------- 1 file changed, 60 insertions(+), 39 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index ed9c7161..d949eb40 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -77,38 +77,7 @@ class Server(object): pass return True - sock.setblocking(False) - - actor_ref = self.protocol.start(sock, addr) - gobject.io_add_watch(sock.fileno(), gobject.IO_IN | gobject.IO_ERR | - gobject.IO_HUP, self.handle_client, sock, actor_ref) - - return True - - def handle_client(self, fd, flags, sock, actor_ref): - """ - Read client data when possible. - - Returns false when reading failed in order to deregister with the event - loop. - """ - if flags & (gobject.IO_ERR | gobject.IO_HUP): - actor_ref.stop() - return False - - try: - data = sock.recv(4096) - except socket.error as e: - if e.errno in (errno.EWOULDBLOCK, errno.EAGAIN): - return True - actor_ref.stop() - return False - - if not data: - actor_ref.stop() - return False - - actor_ref.send_one_way({'received': data}) + self.protocol.start(sock, addr) return True @@ -131,14 +100,21 @@ class LineProtocol(ThreadingActor): timeout = 30 def __init__(self, sock, addr): + sock.setblocking(False) + self.sock = sock self.host, self.port = addr[:2] # IPv6 has larger addr self.send_lock = threading.Lock() self.recv_buffer = '' self.send_buffer = '' self.terminator_re = re.compile(self.terminator) + self.send_id = None + self.recv_id = None self.timeout_id = None + self.sock.setblocking(False) + + self.enable_recv() self.enable_timeout() def on_line_received(self, line): @@ -167,6 +143,8 @@ class LineProtocol(ThreadingActor): def on_stop(self): """Ensure that cleanup when actor stops.""" self.disable_timeout() + self.disable_recv() + self.disable_send() try: self.sock.close() except socket.error: @@ -174,8 +152,33 @@ class LineProtocol(ThreadingActor): def disable_timeout(self): """Deactivate timeout mechanism.""" - if self.timeout_id: + if self.timeout_id is not None: gobject.source_remove(self.timeout_id) + self.timeout_id = None + + def disable_recv(self): + """Deactivate recv mechanism.""" + if self.recv_id is not None: + gobject.source_remove(self.recv_id) + self.recv_id = None + + def disable_send(self): + """Deactivate send mechanism.""" + if self.send_id: + gobject.source_remove(self.send_id) + self.send_id = None + + def enable_recv(self): + """Reactivate recv mechanism.""" + if self.recv_id is None: + self.recv_id = gobject.io_add_watch(self.sock.fileno(), gobject.IO_IN | + gobject.IO_ERR | gobject.IO_HUP, self._recv) + + def enable_send(self): + """Reactivate send mechanism.""" + if self.send_id is None: + self.send_id = gobject.io_add_watch(self.sock.fileno(), + gobject.IO_OUT | gobject.IO_ERR | gobject.IO_HUP, self._send) def enable_timeout(self): """Reactivate timeout mechanism.""" @@ -274,19 +277,37 @@ class LineProtocol(ThreadingActor): def send_raw(self, data): """Send data to client exactly as is.""" self.send_lock.acquire(True) - should_register_sender = len(self.send_buffer) == 0 self.send_buffer += data self.send_lock.release() + self.enable_send() - if should_register_sender: - gobject.io_add_watch(self.sock.fileno(), gobject.IO_OUT | - gobject.IO_ERR | gobject.IO_HUP, self._send) - - def _send(self, fd, flags): + def _recv(self, fd, flags): # NOTE: This code is _not_ run in the actor's thread, but in the same # one as the event loop. If this blocks, rest of gobject code will # likely be blocked as well... + if flags & (gobject.IO_ERR | gobject.IO_HUP): + actor_ref.stop() + return False + + try: + data = self.sock.recv(4096) + except socket.error as e: + if e.errno in (errno.EWOULDBLOCK, errno.EAGAIN): + return True + self.actor_ref.stop() + return False + + if not data: + self.actor_ref.stop() + return False + + self.actor_ref.send_one_way({'received': data}) + return True + + def _send(self, fd, flags): + # NOTE: This code is _not_ run in the actor's thread... + # If with can't get the lock, simply try again next time socket is # ready for sending. if not self.send_lock.acquire(False): From 34cd3008d9c21213039327d18645442311fbd2da Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 9 Jul 2011 22:44:11 +0200 Subject: [PATCH 37/72] Extract gobject/network code to new Client class This implies that the Server class is in charge of just listening and starting up new clients. Clients are expected to run in the event loop thread, so they only deal with minimal IO/network concerns. Each client has a protocol actor that does the actual work. --- mopidy/frontends/mpd/__init__.py | 4 +- mopidy/utils/network.py | 280 ++++++++++++++++--------------- 2 files changed, 148 insertions(+), 136 deletions(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 8dbbf3db..e0ad28fc 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -49,8 +49,8 @@ class MpdSession(network.LineProtocol): terminator = protocol.LINE_TERMINATOR encoding = protocol.ENCODING - def __init__(self, sock, addr): - super(MpdSession, self).__init__(sock, addr) + def __init__(self, client): + super(MpdSession, self).__init__(client) self.dispatcher = dispatcher.MpdDispatcher(self) def on_start(self): diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index d949eb40..15ddb98e 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -47,9 +47,11 @@ def format_hostname(hostname): class Server(object): """Setup listener and register it with gobject's event loop.""" - def __init__(self, host, port, protocol, max_connections=15): + def __init__(self, host, port, protocol, max_connections=5, timeout=30): self.protocol = protocol self.max_connections = max_connections + self.timeout = timeout + self.listener = create_socket() self.listener.setblocking(False) self.listener.bind((host, port)) @@ -57,6 +59,7 @@ class Server(object): gobject.io_add_watch( self.listener.fileno(), gobject.IO_IN, self.handle_accept) + logger.debug(u'Listening on [%s]:%s using %s as protocol handler', host, port, self.protocol.__name__) @@ -77,10 +80,135 @@ class Server(object): pass return True - self.protocol.start(sock, addr) + client = Client(self.protocol, sock, addr, self.timeout) + client.start() + return True +class Client(object): + def __init__(self, protocol, sock, addr, timeout): + sock.setblocking(False) + + self._sock = sock + self.host, self.port = addr[:2] # IPv6 has larger addr + self._protocol = protocol + self._timeout_time = timeout + + self._send_lock = threading.Lock() + self._send_buffer = '' + + self._actor_ref = None + + self._recv_id = None + self._send_id = None + self._timeout_id = None + + def start(self): + self._actor_ref = self._protocol.start(self) + self._enable_recv() + self.enable_timeout() + + def stop(self): + self._actor_ref.stop() + self.disable_timeout() + self._disable_recv() + self._disable_send() + try: + self._sock.close() + except socket.error: + pass + return False + + def send(self, data): + """Send data to client exactly as is.""" + self._send_lock.acquire(True) + self._send_buffer += data + self._send_lock.release() + self._enable_send() + + def enable_timeout(self): + """Reactivate timeout mechanism.""" + self.disable_timeout() + if self._timeout_time > 0: + self._timeout_id = gobject.timeout_add_seconds( + self._timeout_time, self._timeout) + + def disable_timeout(self): + """Deactivate timeout mechanism.""" + if self._timeout_id is not None: + gobject.source_remove(self._timeout_id) + self._timeout_id = None + + def _enable_recv(self): + if self._recv_id is None: + self._recv_id = gobject.io_add_watch(self._sock.fileno(), + gobject.IO_IN | gobject.IO_ERR | gobject.IO_HUP, self._recv) + + def _disable_recv(self): + if self._recv_id is not None: + gobject.source_remove(self._recv_id) + self._recv_id = None + + def _enable_send(self): + if self._send_id is None: + self._send_id = gobject.io_add_watch(self._sock.fileno(), + gobject.IO_OUT | gobject.IO_ERR | gobject.IO_HUP, self._send) + + def _disable_send(self): + if self._send_id: + gobject.source_remove(self._send_id) + self._send_id = None + + def _recv(self, fd, flags): + # NOTE: This code is _not_ run in the actor's thread, but in the same + # one as the event loop. If this blocks, rest of gobject code will + # likely be blocked as well... + + if flags & (gobject.IO_ERR | gobject.IO_HUP): + return self.stop() + + try: + data = self._sock.recv(4096) + except socket.error as e: + if e.errno in (errno.EWOULDBLOCK, errno.EAGAIN): + return True + return self.stop() + + if not data: + return self.stop() + + self._actor_ref.send_one_way({'received': data}) + return True + + def _send(self, fd, flags): + # NOTE: This code is _not_ run in the actor's thread... + + # If with can't get the lock, simply try again next time socket is + # ready for sending. + if not self._send_lock.acquire(False): + return True + + try: + sent = self._sock.send(self._send_buffer) + self._send_buffer = self._send_buffer[sent:] + if not self._send_buffer: + self._disable_send() + except socket.error as e: + if e.errno not in (errno.EAGAIN, errno.EWOULDBLOCK): + #self.log_error(e) # FIXME log error + return self.stop() + finally: + self._send_lock.release() + + return True + + def _timeout(self): + # NOTE: This code is _not_ run in the actor's thread... + #self.log_timeout() # FIXME log this + return self.stop() + + class LineProtocol(ThreadingActor): """ Base class for handling line based protocols. @@ -95,27 +223,11 @@ class LineProtocol(ThreadingActor): #: What encoding to expect incomming data to be in, can be :class:`None`. encoding = 'utf-8' - #: How long to wait before disconnecting client due to inactivity in - #: seconds. - timeout = 30 + def __init__(self, client): + self.client = client - def __init__(self, sock, addr): - sock.setblocking(False) - - self.sock = sock - self.host, self.port = addr[:2] # IPv6 has larger addr - self.send_lock = threading.Lock() self.recv_buffer = '' - self.send_buffer = '' self.terminator_re = re.compile(self.terminator) - self.send_id = None - self.recv_id = None - self.timeout_id = None - - self.sock.setblocking(False) - - self.enable_recv() - self.enable_timeout() def on_line_received(self, line): """ @@ -130,7 +242,7 @@ class LineProtocol(ThreadingActor): if 'received' not in message: return - self.disable_timeout() + self.client.disable_timeout() self.log_raw_data(message['received']) for line in self.parse_lines(message['received']): @@ -138,53 +250,11 @@ class LineProtocol(ThreadingActor): self.log_request(line) self.on_line_received(line) - self.enable_timeout() + self.client.enable_timeout() def on_stop(self): """Ensure that cleanup when actor stops.""" - self.disable_timeout() - self.disable_recv() - self.disable_send() - try: - self.sock.close() - except socket.error: - pass - - def disable_timeout(self): - """Deactivate timeout mechanism.""" - if self.timeout_id is not None: - gobject.source_remove(self.timeout_id) - self.timeout_id = None - - def disable_recv(self): - """Deactivate recv mechanism.""" - if self.recv_id is not None: - gobject.source_remove(self.recv_id) - self.recv_id = None - - def disable_send(self): - """Deactivate send mechanism.""" - if self.send_id: - gobject.source_remove(self.send_id) - self.send_id = None - - def enable_recv(self): - """Reactivate recv mechanism.""" - if self.recv_id is None: - self.recv_id = gobject.io_add_watch(self.sock.fileno(), gobject.IO_IN | - gobject.IO_ERR | gobject.IO_HUP, self._recv) - - def enable_send(self): - """Reactivate send mechanism.""" - if self.send_id is None: - self.send_id = gobject.io_add_watch(self.sock.fileno(), - gobject.IO_OUT | gobject.IO_ERR | gobject.IO_HUP, self._send) - - def enable_timeout(self): - """Reactivate timeout mechanism.""" - self.disable_timeout() - self.timeout_id = gobject.timeout_add_seconds(self.timeout, - self._timeout) + self.client.stop() def parse_lines(self, new_data=None): """Consume new data and yield any lines found.""" @@ -201,8 +271,8 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change logging behaviour. """ - logger.debug(u'Got %s from event loop in %s', - repr(data), self.actor_urn) + logger.debug(u'Got %s from event loop in %s', repr(data), + self.actor_urn) def log_request(self, request): """ @@ -210,8 +280,8 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change logging behaviour. """ - logger.debug(u'Request from [%s]:%s to %s: %s', - self.host, self.port, self.actor_urn, indent(request)) + logger.debug(u'Request from %s to %s: %s', self.client, self.actor_urn, + indent(request)) def log_response(self, response): """ @@ -219,8 +289,8 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change logging behaviour. """ - logger.debug(u'Response to [%s]:%s from %s: %s', - self.host, self.port, self.actor_urn, indent(response)) + logger.debug(u'Response to %s from %s: %s', self.client, + self.actor_urn, indent(response)) def log_error(self, error): """ @@ -228,8 +298,8 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change logging behaviour. """ - logger.warning(u'Problem with connection to [%s]:%s in %s: %s', - self.host, self.port, self.actor_urn, error) + logger.warning(u'Problem with connection to %s in %s: %s', + self.client, self.actor_urn, error) def log_timeout(self): """ @@ -237,8 +307,8 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change logging behaviour. """ - logger.debug(u'Closing connection to [%s]:%s in %s due to timeout.', - self.host, self.port, self.actor_urn) + logger.debug(u'Closing connection to %s in %s due to timeout.', + self.client, self.actor_urn) def encode(self, line): """ @@ -272,62 +342,4 @@ class LineProtocol(ThreadingActor): data = self.terminator.join(lines) self.log_response(data) - self.send_raw(self.encode(data + self.terminator)) - - def send_raw(self, data): - """Send data to client exactly as is.""" - self.send_lock.acquire(True) - self.send_buffer += data - self.send_lock.release() - self.enable_send() - - def _recv(self, fd, flags): - # NOTE: This code is _not_ run in the actor's thread, but in the same - # one as the event loop. If this blocks, rest of gobject code will - # likely be blocked as well... - - if flags & (gobject.IO_ERR | gobject.IO_HUP): - actor_ref.stop() - return False - - try: - data = self.sock.recv(4096) - except socket.error as e: - if e.errno in (errno.EWOULDBLOCK, errno.EAGAIN): - return True - self.actor_ref.stop() - return False - - if not data: - self.actor_ref.stop() - return False - - self.actor_ref.send_one_way({'received': data}) - return True - - def _send(self, fd, flags): - # NOTE: This code is _not_ run in the actor's thread... - - # If with can't get the lock, simply try again next time socket is - # ready for sending. - if not self.send_lock.acquire(False): - return True - - try: - sent = self.sock.send(self.send_buffer) - self.send_buffer = self.send_buffer[sent:] - return bool(self.send_buffer) - except socket.error as e: - if e.errno in (errno.EAGAIN, errno.EWOULDBLOCK): - return True - self.log_error(e) - self.actor_ref.stop() - return False - finally: - self.send_lock.release() - - def _timeout(self): - # NOTE: This code is _not_ run in the actor's thread... - self.log_timeout() - self.actor_ref.stop() - return False + self.client.send(self.encode(data + self.terminator)) From 91270ef535920530a7a65056cbda0d916c859587 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 11 Jul 2011 18:53:49 +0200 Subject: [PATCH 38/72] Refactor network.Server to improve testability --- mopidy/utils/network.py | 103 ++++++++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 41 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 15ddb98e..9280e772 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -12,6 +12,9 @@ from mopidy.utils.log import indent logger = logging.getLogger('mopidy.utils.server') +class ShouldRetrySocketCall(Exception): + """Indicate that attempted socket call should be retried""" + def _try_ipv6_socket(): """Determine if system really supports IPv6""" if not socket.has_ipv6: @@ -51,61 +54,79 @@ class Server(object): self.protocol = protocol self.max_connections = max_connections self.timeout = timeout + self.server_socket = self.create_server_socket(host, port) - self.listener = create_socket() - self.listener.setblocking(False) - self.listener.bind((host, port)) - self.listener.listen(1) - - gobject.io_add_watch( - self.listener.fileno(), gobject.IO_IN, self.handle_accept) + self.register_server_socket(self.server_socket.fileno()) logger.debug(u'Listening on [%s]:%s using %s as protocol handler', - host, port, self.protocol.__name__) + host, port, self.protocol) - def handle_accept(self, fd, flags): + def create_server_socket(self, host, port): + sock = create_socket() + sock.setblocking(False) + sock.bind((host, port)) + sock.listen(1) + return sock + + def register_server_socket(self, fileno): + gobject.io_add_watch(fileno, gobject.IO_IN, self.handle_connection) + + def handle_connection(self, fd, flags): try: - sock, addr = self.listener.accept() - except socket.error as e: - if e.errno in (errno.EAGAIN, errno.EINTR): - return True # i.e. retry - raise - - num_connections = len(ActorRegistry.get_by_class(self.protocol)) - if self.max_connections and num_connections >= self.max_connections: - logger.warning(u'Rejected connection from [%s]:%s', addr[0], addr[1]) - try: - sock.close() - except socket.error: - pass + sock, addr = self.accept_connection() + except ShouldRetrySocketCall: return True - client = Client(self.protocol, sock, addr, self.timeout) - client.start() - + if self.maximum_connections_exceeded(): + self.reject_connection(sock, addr) + else: + self.init_connection(sock, addr) return True + def accept_connection(self): + try: + return self.server_socket.accept() + except socket.error as e: + if e.errno in (errno.EAGAIN, errno.EINTR): + raise ShouldRetrySocketCall + raise -class Client(object): + def maximum_connections_exceeded(self): + return (self.max_connections is not None and + self.number_of_connections() >= self.max_connections) + + def number_of_connections(self): + return len(ActorRegistry.get_by_class(self.protocol)) + + def reject_connection(self, sock, addr): + logger.warning(u'Rejected connection from [%s]:%s', addr[0], addr[1]) + try: + sock.close() + except socket.error: + pass + + def init_connection(self, sock, addr): + Connection(self.protocol, sock, addr, self.timeout) + +class Connection(object): def __init__(self, protocol, sock, addr, timeout): sock.setblocking(False) - self._sock = sock self.host, self.port = addr[:2] # IPv6 has larger addr + + self._sock = sock self._protocol = protocol self._timeout_time = timeout self._send_lock = threading.Lock() self._send_buffer = '' - self._actor_ref = None - self._recv_id = None self._send_id = None self._timeout_id = None - def start(self): self._actor_ref = self._protocol.start(self) + self._enable_recv() self.enable_timeout() @@ -223,8 +244,8 @@ class LineProtocol(ThreadingActor): #: What encoding to expect incomming data to be in, can be :class:`None`. encoding = 'utf-8' - def __init__(self, client): - self.client = client + def __init__(self, connection): + self.connection = connection self.recv_buffer = '' self.terminator_re = re.compile(self.terminator) @@ -242,7 +263,7 @@ class LineProtocol(ThreadingActor): if 'received' not in message: return - self.client.disable_timeout() + self.connection.disable_timeout() self.log_raw_data(message['received']) for line in self.parse_lines(message['received']): @@ -250,11 +271,11 @@ class LineProtocol(ThreadingActor): self.log_request(line) self.on_line_received(line) - self.client.enable_timeout() + self.connection.enable_timeout() def on_stop(self): """Ensure that cleanup when actor stops.""" - self.client.stop() + self.connection.stop() def parse_lines(self, new_data=None): """Consume new data and yield any lines found.""" @@ -280,7 +301,7 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change logging behaviour. """ - logger.debug(u'Request from %s to %s: %s', self.client, self.actor_urn, + logger.debug(u'Request from %s to %s: %s', self.connection, self.actor_urn, indent(request)) def log_response(self, response): @@ -289,7 +310,7 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change logging behaviour. """ - logger.debug(u'Response to %s from %s: %s', self.client, + logger.debug(u'Response to %s from %s: %s', self.connection, self.actor_urn, indent(response)) def log_error(self, error): @@ -299,7 +320,7 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change logging behaviour. """ logger.warning(u'Problem with connection to %s in %s: %s', - self.client, self.actor_urn, error) + self.connection, self.actor_urn, error) def log_timeout(self): """ @@ -308,7 +329,7 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change logging behaviour. """ logger.debug(u'Closing connection to %s in %s due to timeout.', - self.client, self.actor_urn) + self.connection, self.actor_urn) def encode(self, line): """ @@ -332,7 +353,7 @@ class LineProtocol(ThreadingActor): def send_lines(self, lines): """ - Send array of lines to client. + Send array of lines to client via connection. Join lines using the terminator that is set for this class, encode it and send it to the client. @@ -342,4 +363,4 @@ class LineProtocol(ThreadingActor): data = self.terminator.join(lines) self.log_response(data) - self.client.send(self.encode(data + self.terminator)) + self.connection.send(self.encode(data + self.terminator)) From 471ab6802adbcb3b1282447eaddfbcdb4bb6b874 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 11 Jul 2011 18:54:08 +0200 Subject: [PATCH 39/72] Add tests that backed the network.Server refactor --- tests/utils/network_test.py | 179 +++++++++++++++++++++++++++++++++--- 1 file changed, 166 insertions(+), 13 deletions(-) diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index 66229036..6fee59d1 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -1,52 +1,54 @@ -import mock +import errno +import gobject import socket import unittest +from mock import patch, sentinel, Mock from mopidy.utils import network from tests import SkipTest class FormatHostnameTest(unittest.TestCase): - @mock.patch('mopidy.utils.network.has_ipv6', True) + @patch('mopidy.utils.network.has_ipv6', True) def test_format_hostname_prefixes_ipv4_addresses_when_ipv6_available(self): network.has_ipv6 = True self.assertEqual(network.format_hostname('0.0.0.0'), '::ffff:0.0.0.0') self.assertEqual(network.format_hostname('1.0.0.1'), '::ffff:1.0.0.1') - @mock.patch('mopidy.utils.network.has_ipv6', False) + @patch('mopidy.utils.network.has_ipv6', False) def test_format_hostname_does_nothing_when_only_ipv4_available(self): network.has_ipv6 = False self.assertEquals(network.format_hostname('0.0.0.0'), '0.0.0.0') class TryIPv6SocketTest(unittest.TestCase): - @mock.patch('socket.has_ipv6', False) + @patch('socket.has_ipv6', False) def test_system_that_claims_no_ipv6_support(self): self.assertFalse(network._try_ipv6_socket()) - @mock.patch('socket.has_ipv6', True) - @mock.patch('socket.socket') + @patch('socket.has_ipv6', True) + @patch('socket.socket') def test_system_with_broken_ipv6(self, socket_mock): socket_mock.side_effect = IOError() self.assertFalse(network._try_ipv6_socket()) - @mock.patch('socket.has_ipv6', True) - @mock.patch('socket.socket') + @patch('socket.has_ipv6', True) + @patch('socket.socket') def test_with_working_ipv6(self, socket_mock): - socket_mock.return_value = mock.Mock() + socket_mock.return_value = Mock() self.assertTrue(network._try_ipv6_socket()) class CreateSocketTest(unittest.TestCase): - @mock.patch('mopidy.utils.network.has_ipv6', False) - @mock.patch('socket.socket') + @patch('mopidy.utils.network.has_ipv6', False) + @patch('socket.socket') def test_ipv4_socket(self, socket_mock): network.create_socket() self.assertEqual(socket_mock.call_args[0], (socket.AF_INET, socket.SOCK_STREAM)) - @mock.patch('mopidy.utils.network.has_ipv6', True) - @mock.patch('socket.socket') + @patch('mopidy.utils.network.has_ipv6', True) + @patch('socket.socket') def test_ipv6_socket(self, socket_mock): network.create_socket() self.assertEqual(socket_mock.call_args[0], @@ -55,3 +57,154 @@ class CreateSocketTest(unittest.TestCase): @SkipTest def test_ipv6_only_is_set(self): pass + +class ServerTest(unittest.TestCase): + def setUp(self): + self.protocol = network.LineProtocol + self.addr = (sentinel.host, sentinel.port) + self.host, self.port = self.addr + + self.create_server_socket_patchter = patch.object( + network.Server, 'create_server_socket', new=Mock()) + self.register_server_socket_patcher = patch.object( + network.Server, 'register_server_socket', new=Mock()) + + self.create_server_socket_patchter.start() + self.register_server_socket_patcher.start() + + def tearDown(self): + self.create_server_socket_patchter.stop() + self.register_server_socket_patcher.stop() + + def create_server(self): + return network.Server(sentinel.host, sentinel.port, self.protocol) + + def test_init_creates_socket_and_registers_it(self): + server = self.create_server() + sock = server.create_server_socket.return_value + fileno = sock.fileno.return_value + + server.create_server_socket.assert_called_once_with(self.host, self.port) + server.register_server_socket.assert_called_once_with(fileno) + + @patch.object(network, 'create_socket', spec=socket.SocketType) + def test_create_server_socket_sets_up_listener(self, create_socket): + self.create_server_socket_patchter.stop() + + try: + server = self.create_server() + sock = create_socket.return_value + + sock.setblocking.assert_called_once_with(False) + sock.bind.assert_called_once_with(self.addr) + self.assertEqual(1, sock.listen.call_count) + self.assertEqual(sock, server.server_socket) + finally: + self.create_server_socket_patchter.start() + + @SkipTest + def test_create_server_socket_fails(self): + # FIXME define what should happen in this case, let the error propegate + # or do something else? + pass + + @patch.object(gobject, 'io_add_watch', new=Mock()) + def test_register_server_socket_sets_up_io_watch(self): + self.register_server_socket_patcher.stop() + + try: + server = self.create_server() + sock = server.create_server_socket.return_value + fileno = sock.fileno.return_value + + gobject.io_add_watch.assert_called_once_with( + fileno, gobject.IO_IN, server.handle_connection) + finally: + self.register_server_socket_patcher.start() + + @patch.object(network.Server, 'accept_connection', new=Mock()) + @patch.object(network.Server, 'maximum_connections_exceeded', new=Mock()) + @patch.object(network.Server, 'reject_connection', new=Mock()) + @patch.object(network.Server, 'init_connection', new=Mock()) + def test_handle_connection(self): + server = self.create_server() + server.accept_connection.return_value = (sentinel.sock, self.addr) + server.maximum_connections_exceeded.return_value = False + + server.handle_connection(sentinel.fileno, gobject.IO_IN) + + server.accept_connection.assert_called_once_with() + server.maximum_connections_exceeded.assert_called_once_with() + server.init_connection.assert_called_once_with(sentinel.sock, self.addr) + self.assertEquals(0, server.reject_connection.call_count) + + @patch.object(network.Server, 'accept_connection', new=Mock()) + @patch.object(network.Server, 'maximum_connections_exceeded', new=Mock()) + @patch.object(network.Server, 'reject_connection', new=Mock()) + @patch.object(network.Server, 'init_connection', new=Mock()) + def test_handle_connection_exceeded_connections(self): + server = self.create_server() + server.accept_connection.return_value = (sentinel.sock, self.addr) + server.maximum_connections_exceeded.return_value = True + + server.handle_connection(sentinel.fileno, gobject.IO_IN) + + server.accept_connection.assert_called_once_with() + server.maximum_connections_exceeded.assert_called_once_with() + server.reject_connection.assert_called_once_with(sentinel.sock, self.addr) + self.assertEquals(0, server.init_connection.call_count) + + def test_accept_connection(self): + server = self.create_server() + sock = server.create_server_socket.return_value + sock.accept.return_value = (sentinel.sock, self.addr) + + self.assertEquals((sentinel.sock, self.addr), server.accept_connection()) + + def test_accept_connection_recoverable_error(self): + server = self.create_server() + sock = server.create_server_socket.return_value + + sock.accept.side_effect = socket.error(errno.EAGAIN, '') + self.assertRaises(network.ShouldRetrySocketCall, server.accept_connection) + + sock.accept.side_effect = socket.error(errno.EINTR, '') + self.assertRaises(network.ShouldRetrySocketCall, server.accept_connection) + + def test_accept_connection_recoverable_error(self): + server = self.create_server() + sock = server.create_server_socket.return_value + + sock.accept.side_effect = socket.error() + self.assertRaises(socket.error, server.accept_connection) + + @patch.object(network.Server, 'number_of_connections', new=Mock()) + def test_maximum_connections_exceeded(self): + server = self.create_server() + maximum_connections = server.max_connections + + server.number_of_connections.return_value = maximum_connections + 1 + self.assertTrue(server.maximum_connections_exceeded()) + + server.number_of_connections.return_value = maximum_connections + self.assertTrue(server.maximum_connections_exceeded()) + + server.number_of_connections.return_value = maximum_connections - 1 + self.assertFalse(server.maximum_connections_exceeded()) + + @patch('pykka.registry.ActorRegistry.get_by_class') + def test_number_of_connections(self, get_by_class): + server = self.create_server() + + get_by_class.return_value = [1, 2, 3] + self.assertEqual(3, server.number_of_connections()) + + get_by_class.return_value = [] + self.assertEqual(0, server.number_of_connections()) + + @patch.object(network, 'Connection', new=Mock()) + def test_init_connection(self): + server = self.create_server() + server.init_connection(sentinel.sock, self.addr) + + network.Connection.assert_called_once_with(server.protocol, sentinel.sock, self.addr, server.timeout) From cef53b9e7d55b971fdf77d6ea1f2077160c1a675 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 11 Jul 2011 19:07:22 +0200 Subject: [PATCH 40/72] Cleanup of connection class --- mopidy/utils/network.py | 114 ++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 58 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 9280e772..6d6c9472 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -109,88 +109,89 @@ class Server(object): Connection(self.protocol, sock, addr, self.timeout) class Connection(object): + # NOTE: the callback code is _not_ run in the actor's thread, but in the + # same one as the event loop. If code in the callbacks blocks, the rest of + # gobject code will likely be blocked as well... + def __init__(self, protocol, sock, addr, timeout): sock.setblocking(False) self.host, self.port = addr[:2] # IPv6 has larger addr - self._sock = sock - self._protocol = protocol - self._timeout_time = timeout + self.sock = sock + self.protocol = protocol + self.timeout = timeout - self._send_lock = threading.Lock() - self._send_buffer = '' + self.send_lock = threading.Lock() + self.send_buffer = '' - self._recv_id = None - self._send_id = None - self._timeout_id = None + self.recv_id = None + self.send_id = None + self.timeout_id = None - self._actor_ref = self._protocol.start(self) + self.actor_ref = self.protocol.start(self) - self._enable_recv() + self.enable_recv() self.enable_timeout() def stop(self): - self._actor_ref.stop() + self.actor_ref.stop() self.disable_timeout() - self._disable_recv() - self._disable_send() + self.disable_recv() + self.disable_send() try: - self._sock.close() + self.sock.close() except socket.error: pass return False def send(self, data): """Send data to client exactly as is.""" - self._send_lock.acquire(True) - self._send_buffer += data - self._send_lock.release() - self._enable_send() + self.send_lock.acquire(True) + self.send_buffer += data + self.send_lock.release() + self.enable_send() def enable_timeout(self): """Reactivate timeout mechanism.""" self.disable_timeout() - if self._timeout_time > 0: - self._timeout_id = gobject.timeout_add_seconds( - self._timeout_time, self._timeout) + if self.timeout > 0: + self.timeout_id = gobject.timeout_add_seconds( + self.timeout, self.timeout_callback) def disable_timeout(self): """Deactivate timeout mechanism.""" - if self._timeout_id is not None: - gobject.source_remove(self._timeout_id) - self._timeout_id = None + if self.timeout_id is not None: + gobject.source_remove(self.timeout_id) + self.timeout_id = None - def _enable_recv(self): - if self._recv_id is None: - self._recv_id = gobject.io_add_watch(self._sock.fileno(), - gobject.IO_IN | gobject.IO_ERR | gobject.IO_HUP, self._recv) + def enable_recv(self): + if self.recv_id is None: + self.recv_id = gobject.io_add_watch(self.sock.fileno(), + gobject.IO_IN | gobject.IO_ERR | gobject.IO_HUP, self.recv_callback) - def _disable_recv(self): - if self._recv_id is not None: - gobject.source_remove(self._recv_id) - self._recv_id = None + def disable_recv(self): + if self.recv_id is not None: + gobject.source_remove(self.recv_id) + self.recv_id = None - def _enable_send(self): - if self._send_id is None: - self._send_id = gobject.io_add_watch(self._sock.fileno(), - gobject.IO_OUT | gobject.IO_ERR | gobject.IO_HUP, self._send) + def enable_send(self): + if self.send_id is None: + self.send_id = gobject.io_add_watch(self.sock.fileno(), + gobject.IO_OUT | gobject.IO_ERR | gobject.IO_HUP, + self.send_callback) - def _disable_send(self): - if self._send_id: - gobject.source_remove(self._send_id) - self._send_id = None - - def _recv(self, fd, flags): - # NOTE: This code is _not_ run in the actor's thread, but in the same - # one as the event loop. If this blocks, rest of gobject code will - # likely be blocked as well... + def disable_send(self): + if self.send_id is not None: + gobject.source_remove(self.send_id) + self.send_id = None + def recv_callback(self, fd, flags): if flags & (gobject.IO_ERR | gobject.IO_HUP): return self.stop() try: - data = self._sock.recv(4096) + data = self.sock.recv(4096) except socket.error as e: if e.errno in (errno.EWOULDBLOCK, errno.EAGAIN): return True @@ -199,33 +200,30 @@ class Connection(object): if not data: return self.stop() - self._actor_ref.send_one_way({'received': data}) + self.actor_ref.send_one_way({'received': data}) return True - def _send(self, fd, flags): - # NOTE: This code is _not_ run in the actor's thread... - + def send_callback(self, fd, flags): # If with can't get the lock, simply try again next time socket is # ready for sending. - if not self._send_lock.acquire(False): + if not self.send_lock.acquire(False): return True try: - sent = self._sock.send(self._send_buffer) - self._send_buffer = self._send_buffer[sent:] - if not self._send_buffer: - self._disable_send() + sent = self.sock.send(self.send_buffer) + self.send_buffer = self.send_buffer[sent:] + if not self.send_buffer: + self.disable_send() except socket.error as e: if e.errno not in (errno.EAGAIN, errno.EWOULDBLOCK): #self.log_error(e) # FIXME log error return self.stop() finally: - self._send_lock.release() + self.send_lock.release() return True - def _timeout(self): - # NOTE: This code is _not_ run in the actor's thread... + def timeout_callback(self): #self.log_timeout() # FIXME log this return self.stop() From 9b41eb17c5ebc8534bda627ad4a0891c4f3fad2e Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 11 Jul 2011 23:55:33 +0200 Subject: [PATCH 41/72] Lint fixing --- mopidy/utils/network.py | 13 +++++++------ requirements/tests.txt | 2 +- tests/utils/network_test.py | 25 ++++++++++++++++--------- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 6d6c9472..7767377b 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -15,7 +15,7 @@ logger = logging.getLogger('mopidy.utils.server') class ShouldRetrySocketCall(Exception): """Indicate that attempted socket call should be retried""" -def _try_ipv6_socket(): +def try_ipv6_socket(): """Determine if system really supports IPv6""" if not socket.has_ipv6: return False @@ -28,7 +28,7 @@ def _try_ipv6_socket(): return False #: Boolean value that indicates if creating an IPv6 socket will succeed. -has_ipv6 = _try_ipv6_socket() +has_ipv6 = try_ipv6_socket() def create_socket(): """Create a TCP socket with or without IPv6 depending on system support""" @@ -168,7 +168,8 @@ class Connection(object): def enable_recv(self): if self.recv_id is None: self.recv_id = gobject.io_add_watch(self.sock.fileno(), - gobject.IO_IN | gobject.IO_ERR | gobject.IO_HUP, self.recv_callback) + gobject.IO_IN | gobject.IO_ERR | gobject.IO_HUP, + self.recv_callback) def disable_recv(self): if self.recv_id is not None: @@ -254,7 +255,7 @@ class LineProtocol(ThreadingActor): Should be implemented by subclasses. """ - raise NotImplemented + raise NotImplementedError def on_receive(self, message): """Handle messages with new data from server.""" @@ -299,8 +300,8 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change logging behaviour. """ - logger.debug(u'Request from %s to %s: %s', self.connection, self.actor_urn, - indent(request)) + logger.debug(u'Request from %s to %s: %s', self.connection, + self.actor_urn, indent(request)) def log_response(self, response): """ diff --git a/requirements/tests.txt b/requirements/tests.txt index f8cf2eb3..0bc8380f 100644 --- a/requirements/tests.txt +++ b/requirements/tests.txt @@ -1,4 +1,4 @@ coverage -mock +mock >= 0.7 nose tox diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index 6fee59d1..e7767689 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -24,19 +24,19 @@ class FormatHostnameTest(unittest.TestCase): class TryIPv6SocketTest(unittest.TestCase): @patch('socket.has_ipv6', False) def test_system_that_claims_no_ipv6_support(self): - self.assertFalse(network._try_ipv6_socket()) + self.assertFalse(network.try_ipv6_socket()) @patch('socket.has_ipv6', True) @patch('socket.socket') def test_system_with_broken_ipv6(self, socket_mock): socket_mock.side_effect = IOError() - self.assertFalse(network._try_ipv6_socket()) + self.assertFalse(network.try_ipv6_socket()) @patch('socket.has_ipv6', True) @patch('socket.socket') def test_with_working_ipv6(self, socket_mock): socket_mock.return_value = Mock() - self.assertTrue(network._try_ipv6_socket()) + self.assertTrue(network.try_ipv6_socket()) class CreateSocketTest(unittest.TestCase): @@ -84,7 +84,8 @@ class ServerTest(unittest.TestCase): sock = server.create_server_socket.return_value fileno = sock.fileno.return_value - server.create_server_socket.assert_called_once_with(self.host, self.port) + server.create_server_socket.assert_called_once_with( + self.host, self.port) server.register_server_socket.assert_called_once_with(fileno) @patch.object(network, 'create_socket', spec=socket.SocketType) @@ -151,7 +152,8 @@ class ServerTest(unittest.TestCase): server.accept_connection.assert_called_once_with() server.maximum_connections_exceeded.assert_called_once_with() - server.reject_connection.assert_called_once_with(sentinel.sock, self.addr) + server.reject_connection.assert_called_once_with( + sentinel.sock, self.addr) self.assertEquals(0, server.init_connection.call_count) def test_accept_connection(self): @@ -159,17 +161,20 @@ class ServerTest(unittest.TestCase): sock = server.create_server_socket.return_value sock.accept.return_value = (sentinel.sock, self.addr) - self.assertEquals((sentinel.sock, self.addr), server.accept_connection()) + self.assertEquals((sentinel.sock, self.addr), + server.accept_connection()) def test_accept_connection_recoverable_error(self): server = self.create_server() sock = server.create_server_socket.return_value sock.accept.side_effect = socket.error(errno.EAGAIN, '') - self.assertRaises(network.ShouldRetrySocketCall, server.accept_connection) + self.assertRaises(network.ShouldRetrySocketCall, + server.accept_connection) sock.accept.side_effect = socket.error(errno.EINTR, '') - self.assertRaises(network.ShouldRetrySocketCall, server.accept_connection) + self.assertRaises(network.ShouldRetrySocketCall, + server.accept_connection) def test_accept_connection_recoverable_error(self): server = self.create_server() @@ -207,4 +212,6 @@ class ServerTest(unittest.TestCase): server = self.create_server() server.init_connection(sentinel.sock, self.addr) - network.Connection.assert_called_once_with(server.protocol, sentinel.sock, self.addr, server.timeout) + network.Connection.assert_called_once_with(server.protocol, + sentinel.sock, self.addr, server.timeout) + From 6d7575a2dbf0c3152b5fb76846019433e216225a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 Jul 2011 01:32:14 +0200 Subject: [PATCH 42/72] Changed test strategy to use mocks in better way, i.e. rewrote ServerTest --- tests/utils/network_test.py | 175 ++++++++++++++++-------------------- 1 file changed, 76 insertions(+), 99 deletions(-) diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index e7767689..b686e20c 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -60,48 +60,44 @@ class CreateSocketTest(unittest.TestCase): class ServerTest(unittest.TestCase): def setUp(self): - self.protocol = network.LineProtocol - self.addr = (sentinel.host, sentinel.port) - self.host, self.port = self.addr + self.mock = Mock(spec=network.Server) - self.create_server_socket_patchter = patch.object( - network.Server, 'create_server_socket', new=Mock()) - self.register_server_socket_patcher = patch.object( - network.Server, 'register_server_socket', new=Mock()) + def test_init_calls_create_server_socket(self): + network.Server.__init__(self.mock, sentinel.host, + sentinel.port, sentinel.protocol) + self.mock.create_server_socket.assert_called_once_with( + sentinel.host, sentinel.port) - self.create_server_socket_patchter.start() - self.register_server_socket_patcher.start() + def test_init_calls_register_server(self): + sock = Mock(spec=socket.SocketType) + sock.fileno.return_value = sentinel.fileno + self.mock.create_server_socket.return_value = sock - def tearDown(self): - self.create_server_socket_patchter.stop() - self.register_server_socket_patcher.stop() + network.Server.__init__(self.mock, sentinel.host, + sentinel.port, sentinel.protocol) + self.mock.register_server_socket.assert_called_once_with(sentinel.fileno) - def create_server(self): - return network.Server(sentinel.host, sentinel.port, self.protocol) + def test_init_stores_values_in_attributes(self): + sock = Mock(spec=socket.SocketType) + self.mock.create_server_socket.return_value = sock - def test_init_creates_socket_and_registers_it(self): - server = self.create_server() - sock = server.create_server_socket.return_value - fileno = sock.fileno.return_value - - server.create_server_socket.assert_called_once_with( - self.host, self.port) - server.register_server_socket.assert_called_once_with(fileno) + network.Server.__init__(self.mock, sentinel.host, sentinel.port, + sentinel.protocol, max_connections=sentinel.max_connections, + timeout=sentinel.timeout) + self.assertEqual(sentinel.protocol, self.mock.protocol) + self.assertEqual(sentinel.max_connections, self.mock.max_connections) + self.assertEqual(sentinel.timeout, self.mock.timeout) + self.assertEqual(sock, self.mock.server_socket) @patch.object(network, 'create_socket', spec=socket.SocketType) def test_create_server_socket_sets_up_listener(self, create_socket): - self.create_server_socket_patchter.stop() + sock = create_socket.return_value - try: - server = self.create_server() - sock = create_socket.return_value - - sock.setblocking.assert_called_once_with(False) - sock.bind.assert_called_once_with(self.addr) - self.assertEqual(1, sock.listen.call_count) - self.assertEqual(sock, server.server_socket) - finally: - self.create_server_socket_patchter.start() + network.Server.create_server_socket(self.mock, + sentinel.host, sentinel.port) + sock.setblocking.assert_called_once_with(False) + sock.bind.assert_called_once_with((sentinel.host, sentinel.port)) + self.assertEqual(1, sock.listen.call_count) @SkipTest def test_create_server_socket_fails(self): @@ -111,107 +107,88 @@ class ServerTest(unittest.TestCase): @patch.object(gobject, 'io_add_watch', new=Mock()) def test_register_server_socket_sets_up_io_watch(self): - self.register_server_socket_patcher.stop() + network.Server.register_server_socket(self.mock, sentinel.fileno) + gobject.io_add_watch.assert_called_once_with(sentinel.fileno, + gobject.IO_IN, self.mock.handle_connection) - try: - server = self.create_server() - sock = server.create_server_socket.return_value - fileno = sock.fileno.return_value - - gobject.io_add_watch.assert_called_once_with( - fileno, gobject.IO_IN, server.handle_connection) - finally: - self.register_server_socket_patcher.start() - - @patch.object(network.Server, 'accept_connection', new=Mock()) - @patch.object(network.Server, 'maximum_connections_exceeded', new=Mock()) - @patch.object(network.Server, 'reject_connection', new=Mock()) - @patch.object(network.Server, 'init_connection', new=Mock()) def test_handle_connection(self): - server = self.create_server() - server.accept_connection.return_value = (sentinel.sock, self.addr) - server.maximum_connections_exceeded.return_value = False + self.mock.accept_connection.return_value = (sentinel.sock, sentinel.addr) + self.mock.maximum_connections_exceeded.return_value = False - server.handle_connection(sentinel.fileno, gobject.IO_IN) + network.Server.handle_connection(self.mock, sentinel.fileno, gobject.IO_IN) + self.mock.accept_connection.assert_called_once_with() + self.mock.maximum_connections_exceeded.assert_called_once_with() + self.mock.init_connection.assert_called_once_with(sentinel.sock, sentinel.addr) + self.assertEquals(0, self.mock.reject_connection.call_count) - server.accept_connection.assert_called_once_with() - server.maximum_connections_exceeded.assert_called_once_with() - server.init_connection.assert_called_once_with(sentinel.sock, self.addr) - self.assertEquals(0, server.reject_connection.call_count) - - @patch.object(network.Server, 'accept_connection', new=Mock()) - @patch.object(network.Server, 'maximum_connections_exceeded', new=Mock()) - @patch.object(network.Server, 'reject_connection', new=Mock()) - @patch.object(network.Server, 'init_connection', new=Mock()) def test_handle_connection_exceeded_connections(self): - server = self.create_server() - server.accept_connection.return_value = (sentinel.sock, self.addr) - server.maximum_connections_exceeded.return_value = True + self.mock.accept_connection.return_value = (sentinel.sock, sentinel.addr) + self.mock.maximum_connections_exceeded.return_value = True - server.handle_connection(sentinel.fileno, gobject.IO_IN) - - server.accept_connection.assert_called_once_with() - server.maximum_connections_exceeded.assert_called_once_with() - server.reject_connection.assert_called_once_with( - sentinel.sock, self.addr) - self.assertEquals(0, server.init_connection.call_count) + network.Server.handle_connection(self.mock, sentinel.fileno, gobject.IO_IN) + self.mock.accept_connection.assert_called_once_with() + self.mock.maximum_connections_exceeded.assert_called_once_with() + self.mock.reject_connection.assert_called_once_with(sentinel.sock, sentinel.addr) + self.assertEquals(0, self.mock.init_connection.call_count) def test_accept_connection(self): - server = self.create_server() - sock = server.create_server_socket.return_value - sock.accept.return_value = (sentinel.sock, self.addr) + sock = Mock(spec=socket.SocketType) + sock.accept.return_value = (sentinel.sock, sentinel.addr) + self.mock.server_socket = sock - self.assertEquals((sentinel.sock, self.addr), - server.accept_connection()) + sock, addr = network.Server.accept_connection(self.mock) + self.assertEquals(sentinel.sock, sock) + self.assertEquals(sentinel.addr, addr) def test_accept_connection_recoverable_error(self): - server = self.create_server() - sock = server.create_server_socket.return_value + sock = Mock(spec=socket.SocketType) + self.mock.server_socket = sock sock.accept.side_effect = socket.error(errno.EAGAIN, '') self.assertRaises(network.ShouldRetrySocketCall, - server.accept_connection) + network.Server.accept_connection, self.mock) sock.accept.side_effect = socket.error(errno.EINTR, '') self.assertRaises(network.ShouldRetrySocketCall, - server.accept_connection) + network.Server.accept_connection, self.mock) - def test_accept_connection_recoverable_error(self): - server = self.create_server() - sock = server.create_server_socket.return_value + def test_accept_connection_unrecoverable_error(self): + sock = Mock(spec=socket.SocketType) + self.mock.server_socket = sock sock.accept.side_effect = socket.error() - self.assertRaises(socket.error, server.accept_connection) + self.assertRaises(socket.error, + network.Server.accept_connection, self.mock) @patch.object(network.Server, 'number_of_connections', new=Mock()) def test_maximum_connections_exceeded(self): - server = self.create_server() - maximum_connections = server.max_connections + self.mock.max_connections = 10 - server.number_of_connections.return_value = maximum_connections + 1 - self.assertTrue(server.maximum_connections_exceeded()) + self.mock.number_of_connections.return_value = 11 + self.assertTrue(network.Server.maximum_connections_exceeded(self.mock)) - server.number_of_connections.return_value = maximum_connections - self.assertTrue(server.maximum_connections_exceeded()) + self.mock.number_of_connections.return_value = 10 + self.assertTrue(network.Server.maximum_connections_exceeded(self.mock)) - server.number_of_connections.return_value = maximum_connections - 1 - self.assertFalse(server.maximum_connections_exceeded()) + self.mock.number_of_connections.return_value = 9 + self.assertFalse(network.Server.maximum_connections_exceeded(self.mock)) @patch('pykka.registry.ActorRegistry.get_by_class') def test_number_of_connections(self, get_by_class): - server = self.create_server() + self.mock.protocol = sentinel.protocol get_by_class.return_value = [1, 2, 3] - self.assertEqual(3, server.number_of_connections()) + self.assertEqual(3, network.Server.number_of_connections(self.mock)) get_by_class.return_value = [] - self.assertEqual(0, server.number_of_connections()) + self.assertEqual(0, network.Server.number_of_connections(self.mock)) @patch.object(network, 'Connection', new=Mock()) def test_init_connection(self): - server = self.create_server() - server.init_connection(sentinel.sock, self.addr) + self.mock.protocol = sentinel.protocol + self.mock.timeout = sentinel.timeout - network.Connection.assert_called_once_with(server.protocol, - sentinel.sock, self.addr, server.timeout) + network.Server.init_connection(self.mock, sentinel.sock, sentinel.addr) + network.Connection.assert_called_once_with(sentinel.protocol, + sentinel.sock, sentinel.addr, sentinel.timeout) From d9406420e3ce6462e2eb0b878b00dc06a135ef9b Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 Jul 2011 01:34:02 +0200 Subject: [PATCH 43/72] Add missing reject_connection_test --- tests/utils/network_test.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index b686e20c..00032884 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -192,3 +192,17 @@ class ServerTest(unittest.TestCase): network.Connection.assert_called_once_with(sentinel.protocol, sentinel.sock, sentinel.addr, sentinel.timeout) + def test_reject_connection(self): + sock = Mock(spec=socket.SocketType) + + network.Server.reject_connection(self.mock, sock, + (sentinel.host, sentinel.port)) + sock.close.assert_called_once_with() + + def test_reject_connection_error(self): + sock = Mock(spec=socket.SocketType) + sock.close.side_effect = socket.error() + + network.Server.reject_connection(self.mock, sock, + (sentinel.host, sentinel.port)) + sock.close.assert_called_once_with() From b14e019d98b953fe88df4745cb68e04cdb285bb0 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 Jul 2011 02:10:42 +0200 Subject: [PATCH 44/72] Write up most of ConnectionTest, only callbacks to go --- tests/utils/network_test.py | 189 ++++++++++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+) diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index 00032884..23ed0247 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -206,3 +206,192 @@ class ServerTest(unittest.TestCase): network.Server.reject_connection(self.mock, sock, (sentinel.host, sentinel.port)) sock.close.assert_called_once_with() + +class ConnectionTest(unittest.TestCase): + def setUp(self): + self.mock = Mock(spec=network.Connection) + + def test_init_ensure_nonblocking_io(self): + sock = Mock(spec=socket.SocketType) + + network.Connection.__init__(self.mock, Mock(), sock, + (sentinel.host, sentinel.port), sentinel.timeout) + sock.setblocking.assert_called_once_with(False) + + def test_init_starts_actor(self): + protocol = Mock(spec=network.LineProtocol) + + network.Connection.__init__(self.mock, protocol, Mock(), + (sentinel.host, sentinel.port), sentinel.timeout) + protocol.start.assert_called_once_with(self.mock) + + def test_init_enables_recv_and_timeout(self): + network.Connection.__init__(self.mock, Mock(), Mock(), + (sentinel.host, sentinel.port), sentinel.timeout) + self.mock.enable_recv.assert_called_once_with() + self.mock.enable_timeout.assert_called_once_with() + + def test_init_stores_values_in_attributes(self): + protocol = Mock(spec=network.LineProtocol) + sock = Mock(spec=socket.SocketType) + + network.Connection.__init__(self.mock, protocol, sock, + (sentinel.host, sentinel.port), sentinel.timeout) + self.assertEqual(sock, self.mock.sock) + self.assertEqual(protocol, self.mock.protocol) + self.assertEqual(sentinel.timeout, self.mock.timeout) + self.assertEqual(sentinel.host, self.mock.host) + self.assertEqual(sentinel.port, self.mock.port) + + def test_stop_disables_recv_send_and_timeout(self): + network.Connection.stop(self.mock) + self.mock.disable_timeout.assert_called_once_with() + self.mock.disable_recv.assert_called_once_with() + self.mock.disable_timeout.assert_called_once_with() + + def test_stop_closes_socket(self): + sock = Mock(spec=socket.SocketType) + self.mock.sock = sock + + network.Connection.stop(self.mock) + sock.close.assert_called_once_with() + + def test_stop_closes_socket_error(self): + sock = Mock(spec=socket.SocketType) + sock.close.side_effect = socket.error() + self.mock.sock = sock + + network.Connection.stop(self.mock) + sock.close.assert_called_once_with() + + def test_stop_return_false(self): + self.assertFalse(network.Connection.stop(self.mock)) + + @patch.object(gobject, 'io_add_watch', new=Mock()) + def test_enable_recv_registers_with_gobject(self): + self.mock.recv_id = None + self.mock.sock.fileno.return_value = sentinel.fileno + gobject.io_add_watch.return_value = sentinel.tag + + network.Connection.enable_recv(self.mock) + gobject.io_add_watch.assert_called_once_with(sentinel.fileno, + gobject.IO_IN | gobject.IO_ERR | gobject.IO_HUP, + self.mock.recv_callback) + self.assertEqual(sentinel.tag, self.mock.recv_id) + + @patch.object(gobject, 'io_add_watch', new=Mock()) + def test_enable_recv_already_registered(self): + self.mock.recv_id = sentinel.tag + + network.Connection.enable_recv(self.mock) + self.assertEqual(0, gobject.io_add_watch.call_count) + + @patch.object(gobject, 'source_remove', new=Mock()) + def test_disable_recv_deregisters(self): + self.mock.recv_id = sentinel.tag + + network.Connection.disable_recv(self.mock) + gobject.source_remove.assert_called_once_with(sentinel.tag) + self.assertEqual(None, self.mock.recv_id) + + @patch.object(gobject, 'source_remove', new=Mock()) + def test_disable_recv_already_deregistered(self): + self.mock.recv_id = None + + network.Connection.disable_recv(self.mock) + self.assertEqual(0, gobject.source_remove.call_count) + self.assertEqual(None, self.mock.recv_id) + + @patch.object(gobject, 'io_add_watch', new=Mock()) + def test_enable_send_registers_with_gobject(self): + self.mock.send_id = None + self.mock.sock.fileno.return_value = sentinel.fileno + gobject.io_add_watch.return_value = sentinel.tag + + network.Connection.enable_send(self.mock) + gobject.io_add_watch.assert_called_once_with(sentinel.fileno, + gobject.IO_OUT | gobject.IO_ERR | gobject.IO_HUP, + self.mock.send_callback) + self.assertEqual(sentinel.tag, self.mock.send_id) + + @patch.object(gobject, 'io_add_watch', new=Mock()) + def test_enable_send_already_registered(self): + self.mock.send_id = sentinel.tag + + network.Connection.enable_send(self.mock) + self.assertEqual(0, gobject.io_add_watch.call_count) + + @patch.object(gobject, 'source_remove', new=Mock()) + def test_disable_send_deregisters(self): + self.mock.send_id = sentinel.tag + + network.Connection.disable_send(self.mock) + gobject.source_remove.assert_called_once_with(sentinel.tag) + self.assertEqual(None, self.mock.send_id) + + @patch.object(gobject, 'source_remove', new=Mock()) + def test_disable_send_already_deregistered(self): + self.mock.send_id = None + + network.Connection.disable_send(self.mock) + self.assertEqual(0, gobject.source_remove.call_count) + self.assertEqual(None, self.mock.send_id) + + @patch.object(gobject, 'timeout_add_seconds', new=Mock()) + def test_enable_timeout_clears_existing_timeouts(self): + self.mock.timeout = 10 + + network.Connection.enable_timeout(self.mock) + self.mock.disable_timeout.assert_called_once_with() + + @patch.object(gobject, 'timeout_add_seconds', new=Mock()) + def test_enable_timeout_add_gobject_timeout(self): + self.mock.timeout = 10 + gobject.timeout_add_seconds.return_value = sentinel.tag + + network.Connection.enable_timeout(self.mock) + gobject.timeout_add_seconds.assert_called_once_with(10, + self.mock.timeout_callback) + self.assertEqual(sentinel.tag, self.mock.timeout_id) + + @patch.object(gobject, 'timeout_add_seconds', new=Mock()) + def test_enable_timeout_does_not_add_timeout(self): + self.mock.timeout = 0 + network.Connection.enable_timeout(self.mock) + self.assertEqual(0, gobject.timeout_add_seconds.call_count) + + self.mock.timeout = -1 + network.Connection.enable_timeout(self.mock) + self.assertEqual(0, gobject.timeout_add_seconds.call_count) + + self.mock.timeout = None + network.Connection.enable_timeout(self.mock) + self.assertEqual(0, gobject.timeout_add_seconds.call_count) + + @patch.object(gobject, 'source_remove', new=Mock()) + def test_disable_timeout_deregisters(self): + self.mock.timeout_id = sentinel.tag + + network.Connection.disable_timeout(self.mock) + gobject.source_remove.assert_called_once_with(sentinel.tag) + self.assertEqual(None, self.mock.timeout_id) + + @patch.object(gobject, 'source_remove', new=Mock()) + def test_disable_timeout_already_deregistered(self): + self.mock.timeout_id = None + + network.Connection.disable_timeout(self.mock) + self.assertEqual(0, gobject.source_remove.call_count) + self.assertEqual(None, self.mock.timeout_id) + + @SkipTest + def test_recv_callback(self): + pass + + @SkipTest + def test_send_callback(self): + pass + + @SkipTest + def test_timeout_callback(self): + pass From 8c9fc735503ca383768f8b1eaaf0fe4175b1bd01 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 Jul 2011 03:15:30 +0200 Subject: [PATCH 45/72] Implement rest of connection tests --- mopidy/utils/network.py | 16 +-- tests/utils/network_test.py | 201 +++++++++++++++++++++++++++++++----- 2 files changed, 183 insertions(+), 34 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 7767377b..27eb3ed8 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -143,7 +143,6 @@ class Connection(object): self.sock.close() except socket.error: pass - return False def send(self, data): """Send data to client exactly as is.""" @@ -189,17 +188,20 @@ class Connection(object): def recv_callback(self, fd, flags): if flags & (gobject.IO_ERR | gobject.IO_HUP): - return self.stop() + self.stop() + return False try: data = self.sock.recv(4096) except socket.error as e: - if e.errno in (errno.EWOULDBLOCK, errno.EAGAIN): + if e.errno in (errno.EWOULDBLOCK, errno.EINTR): return True - return self.stop() + self.stop() + return False if not data: - return self.stop() + self.stop() + return False self.actor_ref.send_one_way({'received': data}) return True @@ -216,9 +218,9 @@ class Connection(object): if not self.send_buffer: self.disable_send() except socket.error as e: - if e.errno not in (errno.EAGAIN, errno.EWOULDBLOCK): + if e.errno not in (errno.EWOULDBLOCK, errno.EINTR): #self.log_error(e) # FIXME log error - return self.stop() + self.stop() finally: self.send_lock.release() diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index 23ed0247..8fd345c6 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -18,7 +18,7 @@ class FormatHostnameTest(unittest.TestCase): @patch('mopidy.utils.network.has_ipv6', False) def test_format_hostname_does_nothing_when_only_ipv4_available(self): network.has_ipv6 = False - self.assertEquals(network.format_hostname('0.0.0.0'), '0.0.0.0') + self.assertEqual(network.format_hostname('0.0.0.0'), '0.0.0.0') class TryIPv6SocketTest(unittest.TestCase): @@ -119,7 +119,7 @@ class ServerTest(unittest.TestCase): self.mock.accept_connection.assert_called_once_with() self.mock.maximum_connections_exceeded.assert_called_once_with() self.mock.init_connection.assert_called_once_with(sentinel.sock, sentinel.addr) - self.assertEquals(0, self.mock.reject_connection.call_count) + self.assertEqual(0, self.mock.reject_connection.call_count) def test_handle_connection_exceeded_connections(self): self.mock.accept_connection.return_value = (sentinel.sock, sentinel.addr) @@ -129,7 +129,7 @@ class ServerTest(unittest.TestCase): self.mock.accept_connection.assert_called_once_with() self.mock.maximum_connections_exceeded.assert_called_once_with() self.mock.reject_connection.assert_called_once_with(sentinel.sock, sentinel.addr) - self.assertEquals(0, self.mock.init_connection.call_count) + self.assertEqual(0, self.mock.init_connection.call_count) def test_accept_connection(self): sock = Mock(spec=socket.SocketType) @@ -137,20 +137,17 @@ class ServerTest(unittest.TestCase): self.mock.server_socket = sock sock, addr = network.Server.accept_connection(self.mock) - self.assertEquals(sentinel.sock, sock) - self.assertEquals(sentinel.addr, addr) + self.assertEqual(sentinel.sock, sock) + self.assertEqual(sentinel.addr, addr) def test_accept_connection_recoverable_error(self): sock = Mock(spec=socket.SocketType) self.mock.server_socket = sock - sock.accept.side_effect = socket.error(errno.EAGAIN, '') - self.assertRaises(network.ShouldRetrySocketCall, - network.Server.accept_connection, self.mock) - - sock.accept.side_effect = socket.error(errno.EINTR, '') - self.assertRaises(network.ShouldRetrySocketCall, - network.Server.accept_connection, self.mock) + for error in (errno.EAGAIN, errno.EINTR): + sock.accept.side_effect = socket.error(error, '') + self.assertRaises(network.ShouldRetrySocketCall, + network.Server.accept_connection, self.mock) def test_accept_connection_unrecoverable_error(self): sock = Mock(spec=socket.SocketType) @@ -244,32 +241,40 @@ class ConnectionTest(unittest.TestCase): self.assertEqual(sentinel.port, self.mock.port) def test_stop_disables_recv_send_and_timeout(self): + self.mock.actor_ref = Mock() + self.mock.sock = Mock(spec=socket.SocketType) + network.Connection.stop(self.mock) self.mock.disable_timeout.assert_called_once_with() self.mock.disable_recv.assert_called_once_with() self.mock.disable_timeout.assert_called_once_with() def test_stop_closes_socket(self): - sock = Mock(spec=socket.SocketType) - self.mock.sock = sock + self.mock.actor_ref = Mock() + self.mock.sock = Mock(spec=socket.SocketType) network.Connection.stop(self.mock) - sock.close.assert_called_once_with() + self.mock.sock.close.assert_called_once_with() def test_stop_closes_socket_error(self): - sock = Mock(spec=socket.SocketType) - sock.close.side_effect = socket.error() - self.mock.sock = sock + self.mock.actor_ref = Mock() + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.close.side_effect = socket.error() network.Connection.stop(self.mock) - sock.close.assert_called_once_with() + self.mock.sock.close.assert_called_once_with() - def test_stop_return_false(self): - self.assertFalse(network.Connection.stop(self.mock)) + def test_stop_stops_actor(self): + self.mock.actor_ref = Mock() + self.mock.sock = Mock(spec=socket.SocketType) + + network.Connection.stop(self.mock) + self.mock.actor_ref.stop.assert_called_once_with() @patch.object(gobject, 'io_add_watch', new=Mock()) def test_enable_recv_registers_with_gobject(self): self.mock.recv_id = None + self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.fileno.return_value = sentinel.fileno gobject.io_add_watch.return_value = sentinel.tag @@ -305,6 +310,7 @@ class ConnectionTest(unittest.TestCase): @patch.object(gobject, 'io_add_watch', new=Mock()) def test_enable_send_registers_with_gobject(self): self.mock.send_id = None + self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.fileno.return_value = sentinel.fileno gobject.io_add_watch.return_value = sentinel.tag @@ -384,14 +390,155 @@ class ConnectionTest(unittest.TestCase): self.assertEqual(0, gobject.source_remove.call_count) self.assertEqual(None, self.mock.timeout_id) - @SkipTest - def test_recv_callback(self): - pass + def test_send_acquires_and_releases_lock(self): + self.mock.send_lock = Mock() + self.mock.send_buffer = '' + + network.Connection.send(self.mock, 'data') + 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): + self.mock.send_lock = Mock() + self.mock.send_buffer = '' + + network.Connection.send(self.mock, 'abc') + self.assertEqual('abc', self.mock.send_buffer) + + network.Connection.send(self.mock, 'def') + self.assertEqual('abcdef', self.mock.send_buffer) + + network.Connection.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') + self.mock.enable_send.assert_called_once_with() + + def test_recv_callback_respects_io_err(self): + self.assertFalse(network.Connection.recv_callback(self.mock, + sentinel.fd, gobject.IO_IN | gobject.IO_ERR)) + self.mock.stop.assert_called_once_with() + + def test_recv_callback_respects_io_hup(self): + self.assertFalse(network.Connection.recv_callback(self.mock, + sentinel.fd, gobject.IO_IN | gobject.IO_HUP)) + self.mock.stop.assert_called_once_with() + + def test_recv_callback_respects_io_hup_and_io_err(self): + self.assertFalse(network.Connection.recv_callback(self.mock, + sentinel.fd, gobject.IO_IN | gobject.IO_HUP | gobject.IO_ERR)) + self.mock.stop.assert_called_once_with() + + def test_recv_callback_gets_data(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.recv.return_value = 'data' + self.mock.actor_ref = Mock() + + self.assertTrue(network.Connection.recv_callback( + self.mock, sentinel.fd, gobject.IO_IN)) + self.mock.actor_ref.send_one_way.assert_called_once_with( + {'received': 'data'}) + + def test_recv_callback_gets_no_data(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.recv.return_value = '' + + self.assertFalse(network.Connection.recv_callback( + self.mock, sentinel.fd, gobject.IO_IN)) + self.mock.stop.assert_called_once_with() + + def test_recv_callback_recoverable_error(self): + self.mock.sock = Mock(spec=socket.SocketType) + + for error in (errno.EWOULDBLOCK, errno.EINTR): + self.mock.sock.recv.side_effect = socket.error(error, '') + self.assertTrue(network.Connection.recv_callback( + self.mock, sentinel.fd, gobject.IO_IN)) + + def test_recv_callback_unrecoverable_error(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.recv.side_effect = socket.error() + + self.assertFalse(network.Connection.recv_callback( + self.mock, sentinel.fd, gobject.IO_IN)) + self.mock.stop.assert_called_once_with() @SkipTest - def test_send_callback(self): + def test_send_callback_respects_flags(self): + # stop self pass - @SkipTest + def test_send_callback_acquires_and_releases_lock(self): + self.mock.send_lock = Mock() + self.mock.send_lock.acquire.return_value = True + self.mock.send_buffer = '' + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.send.return_value = 0 + + self.assertTrue(network.Connection.send_callback( + self.mock, sentinel.fd, gobject.IO_IN)) + self.mock.send_lock.acquire.assert_called_once_with(False) + self.mock.send_lock.release.assert_called_once_with() + + def test_send_callback_fails_to_acquire_lock(self): + self.mock.send_lock = Mock() + self.mock.send_lock.acquire.return_value = False + + self.assertTrue(network.Connection.send_callback( + self.mock, sentinel.fd, gobject.IO_IN)) + self.mock.send_lock.acquire.assert_called_once_with(False) + + def test_send_callback_sends_all_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 = 4 + + 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.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.assertTrue(network.Connection.send_callback( + self.mock, sentinel.fd, gobject.IO_IN)) + self.mock.sock.send.assert_called_once_with('data') + self.assertEqual('ta', self.mock.send_buffer) + + 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, '') + self.assertTrue(network.Connection.send_callback( + self.mock, sentinel.fd, gobject.IO_IN)) + + 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( + self.mock, sentinel.fd, gobject.IO_IN)) + self.mock.stop.assert_called_once_with() + def test_timeout_callback(self): - pass + network.Connection.timeout_callback(self.mock) + self.mock.stop.assert_called_once_with() From b5c6bc044263f280a742be35998e7cf729864b90 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 Jul 2011 03:18:07 +0200 Subject: [PATCH 46/72] Allways return true from recv_callback, rely on activly removing sources instead --- mopidy/utils/network.py | 14 ++++++-------- tests/utils/network_test.py | 10 +++++----- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 27eb3ed8..c56690a3 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -189,21 +189,19 @@ class Connection(object): def recv_callback(self, fd, flags): if flags & (gobject.IO_ERR | gobject.IO_HUP): self.stop() - return False + return True try: data = self.sock.recv(4096) except socket.error as e: - if e.errno in (errno.EWOULDBLOCK, errno.EINTR): - return True - self.stop() - return False + if e.errno not in (errno.EWOULDBLOCK, errno.EINTR): + self.stop() + return True if not data: self.stop() - return False - - self.actor_ref.send_one_way({'received': data}) + else: + self.actor_ref.send_one_way({'received': data}) return True def send_callback(self, fd, flags): diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index 8fd345c6..f280c0d3 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -419,17 +419,17 @@ class ConnectionTest(unittest.TestCase): self.mock.enable_send.assert_called_once_with() def test_recv_callback_respects_io_err(self): - self.assertFalse(network.Connection.recv_callback(self.mock, + self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_ERR)) self.mock.stop.assert_called_once_with() def test_recv_callback_respects_io_hup(self): - self.assertFalse(network.Connection.recv_callback(self.mock, + self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_HUP)) self.mock.stop.assert_called_once_with() def test_recv_callback_respects_io_hup_and_io_err(self): - self.assertFalse(network.Connection.recv_callback(self.mock, + self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_HUP | gobject.IO_ERR)) self.mock.stop.assert_called_once_with() @@ -447,7 +447,7 @@ class ConnectionTest(unittest.TestCase): self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.recv.return_value = '' - self.assertFalse(network.Connection.recv_callback( + self.assertTrue(network.Connection.recv_callback( self.mock, sentinel.fd, gobject.IO_IN)) self.mock.stop.assert_called_once_with() @@ -463,7 +463,7 @@ class ConnectionTest(unittest.TestCase): self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.recv.side_effect = socket.error() - self.assertFalse(network.Connection.recv_callback( + self.assertTrue(network.Connection.recv_callback( self.mock, sentinel.fd, gobject.IO_IN)) self.mock.stop.assert_called_once_with() From b9286fb9ee190520276b706a17f505077316a889 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 Jul 2011 03:29:04 +0200 Subject: [PATCH 47/72] Log why we are stopping --- mopidy/utils/network.py | 18 ++++++++------- tests/utils/network_test.py | 44 +++++++++++++++++++++++++++---------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index c56690a3..26565577 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -10,6 +10,7 @@ from pykka.registry import ActorRegistry from mopidy.utils.log import indent +# FIXME setup logger with extra={...} logger = logging.getLogger('mopidy.utils.server') class ShouldRetrySocketCall(Exception): @@ -134,11 +135,14 @@ class Connection(object): self.enable_recv() self.enable_timeout() - def stop(self): + def stop(self, reason, level=logging.DEBUG): + logger.log(level, reason) + self.actor_ref.stop() self.disable_timeout() self.disable_recv() self.disable_send() + try: self.sock.close() except socket.error: @@ -188,18 +192,18 @@ class Connection(object): def recv_callback(self, fd, flags): if flags & (gobject.IO_ERR | gobject.IO_HUP): - self.stop() + self.stop(u'Bad client flags: %s' % flags) return True try: data = self.sock.recv(4096) except socket.error as e: if e.errno not in (errno.EWOULDBLOCK, errno.EINTR): - self.stop() + self.stop(u'Unexpected client error: %s' % e) return True if not data: - self.stop() + self.stop(u'Client most likely disconnected.') else: self.actor_ref.send_one_way({'received': data}) return True @@ -217,16 +221,14 @@ class Connection(object): self.disable_send() except socket.error as e: if e.errno not in (errno.EWOULDBLOCK, errno.EINTR): - #self.log_error(e) # FIXME log error - self.stop() + self.stop(u'Unexpected client error: %s' % e) finally: self.send_lock.release() return True def timeout_callback(self): - #self.log_timeout() # FIXME log this - return self.stop() + return self.stop(u'Client timeout out after %s seconds' % self.timeout) class LineProtocol(ThreadingActor): diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index f280c0d3..e4dcec70 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -2,6 +2,7 @@ import errno import gobject import socket import unittest +import logging from mock import patch, sentinel, Mock from mopidy.utils import network @@ -244,7 +245,7 @@ class ConnectionTest(unittest.TestCase): self.mock.actor_ref = Mock() self.mock.sock = Mock(spec=socket.SocketType) - network.Connection.stop(self.mock) + network.Connection.stop(self.mock, sentinel.reason) self.mock.disable_timeout.assert_called_once_with() self.mock.disable_recv.assert_called_once_with() self.mock.disable_timeout.assert_called_once_with() @@ -253,7 +254,7 @@ class ConnectionTest(unittest.TestCase): self.mock.actor_ref = Mock() self.mock.sock = Mock(spec=socket.SocketType) - network.Connection.stop(self.mock) + network.Connection.stop(self.mock, sentinel.reason) self.mock.sock.close.assert_called_once_with() def test_stop_closes_socket_error(self): @@ -261,16 +262,35 @@ class ConnectionTest(unittest.TestCase): self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.close.side_effect = socket.error() - network.Connection.stop(self.mock) + network.Connection.stop(self.mock, sentinel.reason) self.mock.sock.close.assert_called_once_with() def test_stop_stops_actor(self): self.mock.actor_ref = Mock() self.mock.sock = Mock(spec=socket.SocketType) - network.Connection.stop(self.mock) + network.Connection.stop(self.mock, sentinel.reason) self.mock.actor_ref.stop.assert_called_once_with() + @patch.object(network.logger, 'log', new=Mock()) + def test_stop_logs_reason(self): + self.mock.actor_ref = Mock() + self.mock.sock = Mock(spec=socket.SocketType) + + network.Connection.stop(self.mock, sentinel.reason) + network.logger.log.assert_called_once_with( + logging.DEBUG, sentinel.reason) + + @patch.object(network.logger, 'log', new=Mock()) + def test_stop_logs_reason_with_level(self): + self.mock.actor_ref = Mock() + self.mock.sock = Mock(spec=socket.SocketType) + + network.Connection.stop(self.mock, sentinel.reason, + level=sentinel.level) + network.logger.log.assert_called_once_with( + sentinel.level, sentinel.reason) + @patch.object(gobject, 'io_add_watch', new=Mock()) def test_enable_recv_registers_with_gobject(self): self.mock.recv_id = None @@ -421,17 +441,17 @@ class ConnectionTest(unittest.TestCase): def test_recv_callback_respects_io_err(self): self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_ERR)) - self.mock.stop.assert_called_once_with() + self.assertEqual(1, self.mock.stop.call_count) def test_recv_callback_respects_io_hup(self): self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_HUP)) - self.mock.stop.assert_called_once_with() + self.assertEqual(1, self.mock.stop.call_count) def test_recv_callback_respects_io_hup_and_io_err(self): self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_HUP | gobject.IO_ERR)) - self.mock.stop.assert_called_once_with() + self.assertEqual(1, self.mock.stop.call_count) def test_recv_callback_gets_data(self): self.mock.sock = Mock(spec=socket.SocketType) @@ -449,7 +469,7 @@ class ConnectionTest(unittest.TestCase): self.assertTrue(network.Connection.recv_callback( self.mock, sentinel.fd, gobject.IO_IN)) - self.mock.stop.assert_called_once_with() + self.assertEqual(1, self.mock.stop.call_count) def test_recv_callback_recoverable_error(self): self.mock.sock = Mock(spec=socket.SocketType) @@ -465,7 +485,7 @@ class ConnectionTest(unittest.TestCase): self.assertTrue(network.Connection.recv_callback( self.mock, sentinel.fd, gobject.IO_IN)) - self.mock.stop.assert_called_once_with() + self.assertEqual(1, self.mock.stop.call_count) @SkipTest def test_send_callback_respects_flags(self): @@ -537,8 +557,10 @@ class ConnectionTest(unittest.TestCase): self.mock.sock.send.side_effect = socket.error() self.assertTrue(network.Connection.send_callback( self.mock, sentinel.fd, gobject.IO_IN)) - self.mock.stop.assert_called_once_with() + self.assertEqual(1, self.mock.stop.call_count) def test_timeout_callback(self): + self.mock.timeout = 10 + network.Connection.timeout_callback(self.mock) - self.mock.stop.assert_called_once_with() + self.assertEqual(1, self.mock.stop.call_count) From 2f1d32ba80816e3880a464a63d8f3f549a2be9e2 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 Jul 2011 22:02:50 +0200 Subject: [PATCH 48/72] Add IsA helper to tests to provde any_int, any_str and any_unicode --- tests/__init__.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/__init__.py b/tests/__init__.py index 1d4d2e3d..663b89ec 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -22,3 +22,22 @@ def path_to_data_dir(name): path = os.path.abspath(path) return os.path.join(path, name) +class IsA(object): + def __init__(self, klass): + self.klass = klass + + def __eq__(self, rhs): + try: + return isinstance(rhs, self.klass) + except TypeError: + return type(rhs) == type(self.klass) + + def __ne__(self, rhs): + return not self.__eq__(rhs) + + def __repr__(self): + return str(self.klass) + +any_int = IsA(int) +any_str = IsA(str) +any_unicode = IsA(unicode) From 51190c510a089d73bdb216e4a17fd94807fc2dae Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 Jul 2011 22:02:56 +0200 Subject: [PATCH 49/72] Switch to more robust checking of stop calls --- tests/utils/network_test.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index e4dcec70..e1f1bbed 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -7,7 +7,7 @@ from mock import patch, sentinel, Mock from mopidy.utils import network -from tests import SkipTest +from tests import SkipTest, any_int, any_unicode class FormatHostnameTest(unittest.TestCase): @patch('mopidy.utils.network.has_ipv6', True) @@ -98,7 +98,7 @@ class ServerTest(unittest.TestCase): sentinel.host, sentinel.port) sock.setblocking.assert_called_once_with(False) sock.bind.assert_called_once_with((sentinel.host, sentinel.port)) - self.assertEqual(1, sock.listen.call_count) + sock.listen.assert_called_once_with(any_int) @SkipTest def test_create_server_socket_fails(self): @@ -441,17 +441,17 @@ class ConnectionTest(unittest.TestCase): def test_recv_callback_respects_io_err(self): self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_ERR)) - self.assertEqual(1, self.mock.stop.call_count) + self.mock.stop.assert_called_once_with(any_unicode) def test_recv_callback_respects_io_hup(self): self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_HUP)) - self.assertEqual(1, self.mock.stop.call_count) + self.mock.stop.assert_called_once_with(any_unicode) def test_recv_callback_respects_io_hup_and_io_err(self): self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_HUP | gobject.IO_ERR)) - self.assertEqual(1, self.mock.stop.call_count) + self.mock.stop.assert_called_once_with(any_unicode) def test_recv_callback_gets_data(self): self.mock.sock = Mock(spec=socket.SocketType) @@ -469,7 +469,7 @@ class ConnectionTest(unittest.TestCase): self.assertTrue(network.Connection.recv_callback( self.mock, sentinel.fd, gobject.IO_IN)) - self.assertEqual(1, self.mock.stop.call_count) + self.mock.stop.assert_called_once_with(any_unicode) def test_recv_callback_recoverable_error(self): self.mock.sock = Mock(spec=socket.SocketType) @@ -485,7 +485,7 @@ class ConnectionTest(unittest.TestCase): self.assertTrue(network.Connection.recv_callback( self.mock, sentinel.fd, gobject.IO_IN)) - self.assertEqual(1, self.mock.stop.call_count) + self.mock.stop.assert_called_once_with(any_unicode) @SkipTest def test_send_callback_respects_flags(self): @@ -557,10 +557,10 @@ class ConnectionTest(unittest.TestCase): self.mock.sock.send.side_effect = socket.error() self.assertTrue(network.Connection.send_callback( self.mock, sentinel.fd, gobject.IO_IN)) - self.assertEqual(1, self.mock.stop.call_count) + self.mock.stop.assert_called_once_with(any_unicode) def test_timeout_callback(self): self.mock.timeout = 10 network.Connection.timeout_callback(self.mock) - self.assertEqual(1, self.mock.stop.call_count) + self.mock.stop.assert_called_once_with(any_unicode) From 05b169930fa2b85f47dfebfbb5abb403e1846953 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 13 Jul 2011 11:48:22 +0200 Subject: [PATCH 50/72] Add missing stop explanation --- mopidy/utils/network.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 26565577..52fc04eb 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -276,7 +276,7 @@ class LineProtocol(ThreadingActor): def on_stop(self): """Ensure that cleanup when actor stops.""" - self.connection.stop() + self.connection.stop(u'Actor is shuting down.') def parse_lines(self, new_data=None): """Consume new data and yield any lines found.""" From a49855abfa5431711369cf3a94c0a213a0b92e92 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 13 Jul 2011 22:32:35 +0200 Subject: [PATCH 51/72] Improve error handling in connection code --- mopidy/utils/network.py | 28 ++++++++++++++++++++---- tests/utils/network_test.py | 43 ++++++++++++++++++++++++++++++++++--- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 52fc04eb..1d63f6e6 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -5,6 +5,7 @@ import re import socket import threading +from pykka import ActorDeadError from pykka.actor import ThreadingActor from pykka.registry import ActorRegistry @@ -138,7 +139,11 @@ class Connection(object): def stop(self, reason, level=logging.DEBUG): logger.log(level, reason) - self.actor_ref.stop() + try: + self.actor_ref.stop() + except ActorDeadError: + pass + self.disable_timeout() self.disable_recv() self.disable_send() @@ -169,10 +174,15 @@ class Connection(object): self.timeout_id = None def enable_recv(self): - if self.recv_id is None: + if self.recv_id is not None: + return + + try: self.recv_id = gobject.io_add_watch(self.sock.fileno(), gobject.IO_IN | gobject.IO_ERR | gobject.IO_HUP, self.recv_callback) + except socket.error as e: + self.stop(u'Problem with connection: %s' % e) def disable_recv(self): if self.recv_id is not None: @@ -180,10 +190,15 @@ class Connection(object): self.recv_id = None def enable_send(self): - if self.send_id is None: + if self.send_id is not None: + return + + try: self.send_id = gobject.io_add_watch(self.sock.fileno(), gobject.IO_OUT | gobject.IO_ERR | gobject.IO_HUP, self.send_callback) + except socket.error as e: + self.stop(u'Problem with connection: %s' % e) def disable_send(self): if self.send_id is not None: @@ -204,8 +219,13 @@ class Connection(object): if not data: self.stop(u'Client most likely disconnected.') - else: + return True + + try: self.actor_ref.send_one_way({'received': data}) + except ActorDeadError: + self.stop(u'Actor is dead.') + return True def send_callback(self, fd, flags): diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index e1f1bbed..9e41472f 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -1,12 +1,13 @@ import errno import gobject +import logging +import pykka import socket import unittest -import logging -from mock import patch, sentinel, Mock from mopidy.utils import network +from mock import patch, sentinel, Mock from tests import SkipTest, any_int, any_unicode class FormatHostnameTest(unittest.TestCase): @@ -59,6 +60,7 @@ class CreateSocketTest(unittest.TestCase): def test_ipv6_only_is_set(self): pass + class ServerTest(unittest.TestCase): def setUp(self): self.mock = Mock(spec=network.Server) @@ -205,6 +207,7 @@ class ServerTest(unittest.TestCase): (sentinel.host, sentinel.port)) sock.close.assert_called_once_with() + class ConnectionTest(unittest.TestCase): def setUp(self): self.mock = Mock(spec=network.Connection) @@ -272,6 +275,13 @@ class ConnectionTest(unittest.TestCase): network.Connection.stop(self.mock, sentinel.reason) self.mock.actor_ref.stop.assert_called_once_with() + def test_stop_handles_actor_already_being_stopped(self): + self.mock.actor_ref = Mock() + self.mock.actor_ref.stop.side_effect = pykka.ActorDeadError() + self.mock.sock = Mock(spec=socket.SocketType) + + network.Connection.stop(self.mock, sentinel.reason) + @patch.object(network.logger, 'log', new=Mock()) def test_stop_logs_reason(self): self.mock.actor_ref = Mock() @@ -327,6 +337,15 @@ class ConnectionTest(unittest.TestCase): self.assertEqual(0, gobject.source_remove.call_count) self.assertEqual(None, self.mock.recv_id) + def test_enable_recv_on_closed_socket(self): + self.mock.recv_id = None + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.fileno.side_effect = socket.error(errno.EBADF, '') + + network.Connection.enable_recv(self.mock) + self.mock.stop.assert_called_once_with(any_unicode) + self.assertEqual(None, self.mock.recv_id) + @patch.object(gobject, 'io_add_watch', new=Mock()) def test_enable_send_registers_with_gobject(self): self.mock.send_id = None @@ -363,6 +382,14 @@ class ConnectionTest(unittest.TestCase): self.assertEqual(0, gobject.source_remove.call_count) self.assertEqual(None, self.mock.send_id) + def test_enable_send_on_closed_socket(self): + self.mock.send_id = None + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.fileno.side_effect = socket.error(errno.EBADF, '') + + network.Connection.enable_send(self.mock) + self.assertEqual(None, self.mock.send_id) + @patch.object(gobject, 'timeout_add_seconds', new=Mock()) def test_enable_timeout_clears_existing_timeouts(self): self.mock.timeout = 10 @@ -453,7 +480,7 @@ class ConnectionTest(unittest.TestCase): sentinel.fd, gobject.IO_IN | gobject.IO_HUP | gobject.IO_ERR)) self.mock.stop.assert_called_once_with(any_unicode) - def test_recv_callback_gets_data(self): + def test_recv_callback_sends_data_to_actor(self): self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.recv.return_value = 'data' self.mock.actor_ref = Mock() @@ -463,6 +490,16 @@ class ConnectionTest(unittest.TestCase): self.mock.actor_ref.send_one_way.assert_called_once_with( {'received': 'data'}) + def test_recv_callback_handles_dead_actors(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.recv.return_value = 'data' + self.mock.actor_ref = Mock() + self.mock.actor_ref.send_one_way.side_effect = pykka.ActorDeadError() + + self.assertTrue(network.Connection.recv_callback( + self.mock, sentinel.fd, gobject.IO_IN)) + self.mock.stop.assert_called_once_with(any_unicode) + def test_recv_callback_gets_no_data(self): self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.recv.return_value = '' From ee6f5a651bf5061683558a524e1640fab2f0a02e Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 13 Jul 2011 22:43:57 +0200 Subject: [PATCH 52/72] Try to prevent recursive calls to stop --- mopidy/utils/network.py | 8 ++++++++ tests/utils/network_test.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 1d63f6e6..acb074d1 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -127,6 +127,8 @@ class Connection(object): self.send_lock = threading.Lock() self.send_buffer = '' + self.stopping = False + self.recv_id = None self.send_id = None self.timeout_id = None @@ -137,6 +139,12 @@ class Connection(object): self.enable_timeout() def stop(self, reason, level=logging.DEBUG): + if self.stopping: + logger.log(level, 'Already stopping: %s' % reason) + return + else: + self.stopping = True + logger.log(level, reason) try: diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index 9e41472f..52d8c51b 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -245,6 +245,7 @@ class ConnectionTest(unittest.TestCase): self.assertEqual(sentinel.port, self.mock.port) def test_stop_disables_recv_send_and_timeout(self): + self.mock.stopping = False self.mock.actor_ref = Mock() self.mock.sock = Mock(spec=socket.SocketType) @@ -254,6 +255,7 @@ class ConnectionTest(unittest.TestCase): self.mock.disable_timeout.assert_called_once_with() def test_stop_closes_socket(self): + self.mock.stopping = False self.mock.actor_ref = Mock() self.mock.sock = Mock(spec=socket.SocketType) @@ -261,6 +263,7 @@ class ConnectionTest(unittest.TestCase): self.mock.sock.close.assert_called_once_with() def test_stop_closes_socket_error(self): + self.mock.stopping = False self.mock.actor_ref = Mock() self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.close.side_effect = socket.error() @@ -269,6 +272,7 @@ class ConnectionTest(unittest.TestCase): self.mock.sock.close.assert_called_once_with() def test_stop_stops_actor(self): + self.mock.stopping = False self.mock.actor_ref = Mock() self.mock.sock = Mock(spec=socket.SocketType) @@ -276,14 +280,42 @@ class ConnectionTest(unittest.TestCase): self.mock.actor_ref.stop.assert_called_once_with() def test_stop_handles_actor_already_being_stopped(self): + self.mock.stopping = False self.mock.actor_ref = Mock() self.mock.actor_ref.stop.side_effect = pykka.ActorDeadError() self.mock.sock = Mock(spec=socket.SocketType) network.Connection.stop(self.mock, sentinel.reason) + def test_stop_sets_stopping_to_true(self): + self.mock.stopping = False + self.mock.actor_ref = Mock() + self.mock.sock = Mock(spec=socket.SocketType) + + network.Connection.stop(self.mock, sentinel.reason) + self.assertEqual(True, self.mock.stopping) + + def test_stop_does_not_proceed_when_already_stopping(self): + self.mock.stopping = True + self.mock.actor_ref = Mock() + self.mock.sock = Mock(spec=socket.SocketType) + + network.Connection.stop(self.mock, sentinel.reason) + self.assertEqual(0, self.mock.actor_ref.stop.call_count) + self.assertEqual(0, self.mock.sock.close.call_count) + + @patch.object(network.logger, 'log', new=Mock()) + def test_stop_logs_that_it_is_calling_itself(self): + self.mock.stopping = True + self.mock.actor_ref = Mock() + self.mock.sock = Mock(spec=socket.SocketType) + + network.Connection.stop(self.mock, sentinel.reason) + network.logger.log(any_int, any_unicode) + @patch.object(network.logger, 'log', new=Mock()) def test_stop_logs_reason(self): + self.mock.stopping = False self.mock.actor_ref = Mock() self.mock.sock = Mock(spec=socket.SocketType) @@ -293,6 +325,7 @@ class ConnectionTest(unittest.TestCase): @patch.object(network.logger, 'log', new=Mock()) def test_stop_logs_reason_with_level(self): + self.mock.stopping = False self.mock.actor_ref = Mock() self.mock.sock = Mock(spec=socket.SocketType) From 66a89918c8480430541fa93eaa998393fdc8d9f2 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 13 Jul 2011 23:11:28 +0200 Subject: [PATCH 53/72] Add LineProtocol tests --- mopidy/utils/network.py | 30 +++--- tests/utils/network_test.py | 203 ++++++++++++++++++++++++++++++++++++ 2 files changed, 217 insertions(+), 16 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index acb074d1..7233e65b 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -275,9 +275,7 @@ class LineProtocol(ThreadingActor): def __init__(self, connection): self.connection = connection - self.recv_buffer = '' - self.terminator_re = re.compile(self.terminator) def on_line_received(self, line): """ @@ -294,8 +292,9 @@ class LineProtocol(ThreadingActor): self.connection.disable_timeout() self.log_raw_data(message['received']) + self.recv_buffer += message['received'] - for line in self.parse_lines(message['received']): + for line in self.parse_lines(): line = self.decode(line) self.log_request(line) self.on_line_received(line) @@ -306,12 +305,10 @@ class LineProtocol(ThreadingActor): """Ensure that cleanup when actor stops.""" self.connection.stop(u'Actor is shuting down.') - def parse_lines(self, new_data=None): + def parse_lines(self): """Consume new data and yield any lines found.""" - if new_data: - self.recv_buffer += new_data - while self.terminator_re.search(self.recv_buffer): - line, self.recv_buffer = self.terminator_re.split( + while re.search(self.terminator, self.recv_buffer): + line, self.recv_buffer = re.split(self.terminator, self.recv_buffer, 1) yield line @@ -366,9 +363,7 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change encoding behaviour. """ - if self.encoding: - return line.encode(self.encoding) - return line + return line.encode(self.encoding) def decode(self, line): """ @@ -376,9 +371,12 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change decoding behaviour. """ - if self.encoding: - return line.decode(self.encoding) - return line + return line.decode(self.encoding) + + def join_lines(self, lines): + if not lines: + return u'' + return self.terminator.join(lines) + self.terminator def send_lines(self, lines): """ @@ -390,6 +388,6 @@ class LineProtocol(ThreadingActor): if not lines: return - data = self.terminator.join(lines) + data = self.join_lines(lines) self.log_response(data) - self.connection.send(self.encode(data + self.terminator)) + self.connection.send(self.encode(data)) diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index 52d8c51b..5553af1a 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -1,3 +1,5 @@ +#encoding: utf-8 + import errno import gobject import logging @@ -634,3 +636,204 @@ class ConnectionTest(unittest.TestCase): network.Connection.timeout_callback(self.mock) self.mock.stop.assert_called_once_with(any_unicode) + + +class LineProtocolTest(unittest.TestCase): + def setUp(self): + self.mock = Mock(spec=network.LineProtocol) + self.mock.terminator = network.LineProtocol.terminator + self.mock.encoding = network.LineProtocol.encoding + + def test_init_stores_values_in_attributes(self): + network.LineProtocol.__init__(self.mock, sentinel.connection) + self.assertEqual(sentinel.connection, self.mock.connection) + self.assertEqual('', self.mock.recv_buffer) + + def test_on_receive_no_new_lines_adds_to_recv_buffer(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = [] + + network.LineProtocol.on_receive(self.mock, {'received': 'data'}) + self.assertEqual('data', self.mock.recv_buffer) + self.mock.parse_lines.assert_called_once_with() + self.assertEqual(0, self.mock.on_line_received.call_count) + + def test_on_receive_no_new_lines_toggles_timeout(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = [] + + network.LineProtocol.on_receive(self.mock, {'received': 'data'}) + self.mock.connection.disable_timeout.assert_called_once_with() + self.mock.connection.enable_timeout.assert_called_once_with() + + def test_on_receive_no_new_lines_calls_parse_lines(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = [] + + network.LineProtocol.on_receive(self.mock, {'received': 'data'}) + self.mock.parse_lines.assert_called_once_with() + self.assertEqual(0, self.mock.on_line_received.call_count) + + def test_on_receive_with_new_line_calls_decode(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = [sentinel.line] + + network.LineProtocol.on_receive(self.mock, {'received': 'data\n'}) + self.mock.parse_lines.assert_called_once_with() + self.mock.decode.assert_called_once_with(sentinel.line) + + def test_on_receive_with_new_line_calls_on_recieve(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = [sentinel.line] + self.mock.decode.return_value = sentinel.decoded + + network.LineProtocol.on_receive(self.mock, {'received': 'data\n'}) + self.mock.on_line_received.assert_called_once_with(sentinel.decoded) + + def test_parse_lines_emtpy_buffer(self): + self.mock.recv_buffer = '' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertRaises(StopIteration, lines.next) + + def test_parse_lines_no_terminator(self): + self.mock.recv_buffer = 'data' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertRaises(StopIteration, lines.next) + + def test_parse_lines_termintor(self): + self.mock.recv_buffer = 'data\n' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual('data', lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('', self.mock.recv_buffer) + + def test_parse_lines_no_data_before_terminator(self): + self.mock.recv_buffer = '\n' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual('', lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('', self.mock.recv_buffer) + + def test_parse_lines_extra_data_after_terminator(self): + self.mock.recv_buffer = 'data1\ndata2' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual('data1', lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('data2', self.mock.recv_buffer) + + def test_parse_lines_unicode(self): + self.mock.recv_buffer = u'æøå\n'.encode('utf-8') + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual(u'æøå'.encode('utf-8'), lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('', self.mock.recv_buffer) + + def test_parse_lines_multiple_lines(self): + self.mock.recv_buffer = 'abc\ndef\nghi\njkl' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual('abc', lines.next()) + self.assertEqual('def', lines.next()) + self.assertEqual('ghi', lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('jkl', self.mock.recv_buffer) + + def test_parse_lines_multiple_calls(self): + self.mock.recv_buffer = 'data' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('data', self.mock.recv_buffer) + + self.mock.recv_buffer += '\n' + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual('data', lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('', self.mock.recv_buffer) + + def test_send_lines_called_with_no_lines(self): + self.mock.connection = Mock(spec=network.Connection) + + network.LineProtocol.send_lines(self.mock, []) + self.assertEqual(0, self.mock.encode.call_count) + self.assertEqual(0, self.mock.connection.send.call_count) + + def test_send_lines_calls_join_lines(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.join_lines.return_value = 'lines' + + network.LineProtocol.send_lines(self.mock, sentinel.lines) + self.mock.join_lines.assert_called_once_with(sentinel.lines) + + def test_send_line_encodes_joined_lines_with_final_terminator(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.join_lines.return_value = u'lines\n' + + network.LineProtocol.send_lines(self.mock, sentinel.lines) + self.mock.encode.assert_called_once_with(u'lines\n') + + def test_send_lines_sends_encoded_string(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.join_lines.return_value = 'lines' + 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) + + def test_join_lines_returns_empty_string_for_no_lines(self): + self.assertEqual(u'', network.LineProtocol.join_lines(self.mock, [])) + + def test_join_lines_returns_joined_lines(self): + self.assertEqual(u'1\n2\n', network.LineProtocol.join_lines( + self.mock, [u'1', u'2'])) + + def test_decode_calls_decode_on_string(self): + string = Mock() + + network.LineProtocol.decode(self.mock, string) + string.decode.assert_called_once_with(self.mock.encoding) + + def test_decode_plain_ascii(self): + self.assertEqual(u'abc', network.LineProtocol.decode(self.mock, 'abc')) + + def test_decode_utf8(self): + self.assertEqual(u'æøå', network.LineProtocol.decode( + self.mock, u'æøå'.encode('utf-8'))) + + @SkipTest + def test_decode_invalid_data(self): + string = Mock() + string.decode.side_effect = UnicodeError + + network.LineProtocol.decode(self.mock, string) + + def test_encode_calls_encode_on_string(self): + string = Mock() + + network.LineProtocol.encode(self.mock, string) + string.encode.assert_called_once_with(self.mock.encoding) + + def test_encode_plain_ascii(self): + self.assertEqual('abc', network.LineProtocol.encode(self.mock, u'abc')) + + def test_encode_utf8(self): + self.assertEqual(u'æøå'.encode('utf-8'), + network.LineProtocol.encode(self.mock, u'æøå')) + + @SkipTest + def test_encode_invalid_data(self): + string = Mock() + string.encode.side_effect = UnicodeError + + network.LineProtocol.encode(self.mock, string) From cf48faad986ea57c0bd3542f7a22a11ce5333838 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 13 Jul 2011 23:57:00 +0200 Subject: [PATCH 54/72] Remove log_* method from LineProtocol --- mopidy/frontends/mpd/__init__.py | 15 ++++++-- mopidy/utils/network.py | 59 +++++--------------------------- tests/utils/network_test.py | 8 +++++ 3 files changed, 29 insertions(+), 53 deletions(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index e0ad28fc..37877386 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -5,7 +5,7 @@ from pykka.actor import ThreadingActor from mopidy import settings from mopidy.frontends.mpd import dispatcher, protocol -from mopidy.utils import network, process +from mopidy.utils import network, process, log logger = logging.getLogger('mopidy.frontends.mpd') @@ -54,10 +54,21 @@ class MpdSession(network.LineProtocol): self.dispatcher = dispatcher.MpdDispatcher(self) def on_start(self): + logger.info(u'New connection from [%s]:%s', self.host, self.port) self.send_lines([u'OK MPD %s' % protocol.VERSION]) def on_line_received(self, line): - self.send_lines(self.dispatcher.handle_request(line)) + logger.debug(u'Request from [%s]:%s to %s: %s', self.host, self.port, + self.actor_urn, line) + + response = self.dispatcher.handle_request(line) + if not response: + return + + logger.debug(u'Response to [%s]:%s from %s: %s', self.host, self.port, + self.actor_urn, log.indent(self.terminator.join(response))) + + self.send_lines(response) def close(self): self.stop() diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 7233e65b..936aab41 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -60,9 +60,6 @@ class Server(object): self.register_server_socket(self.server_socket.fileno()) - logger.debug(u'Listening on [%s]:%s using %s as protocol handler', - host, port, self.protocol) - def create_server_socket(self, host, port): sock = create_socket() sock.setblocking(False) @@ -277,6 +274,14 @@ class LineProtocol(ThreadingActor): self.connection = connection self.recv_buffer = '' + @property + def host(self): + return self.connection.host + + @property + def port(self): + return self.connection.port + def on_line_received(self, line): """ Called whenever a new line is found. @@ -291,12 +296,10 @@ class LineProtocol(ThreadingActor): return self.connection.disable_timeout() - self.log_raw_data(message['received']) self.recv_buffer += message['received'] for line in self.parse_lines(): line = self.decode(line) - self.log_request(line) self.on_line_received(line) self.connection.enable_timeout() @@ -312,51 +315,6 @@ class LineProtocol(ThreadingActor): self.recv_buffer, 1) yield line - def log_raw_data(self, data): - """ - Log raw data from event loop for debug purposes. - - Can be overridden by subclasses to change logging behaviour. - """ - logger.debug(u'Got %s from event loop in %s', repr(data), - self.actor_urn) - - def log_request(self, request): - """ - Log request for debug purposes. - - Can be overridden by subclasses to change logging behaviour. - """ - logger.debug(u'Request from %s to %s: %s', self.connection, - self.actor_urn, indent(request)) - - def log_response(self, response): - """ - Log response for debug purposes. - - Can be overridden by subclasses to change logging behaviour. - """ - logger.debug(u'Response to %s from %s: %s', self.connection, - self.actor_urn, indent(response)) - - def log_error(self, error): - """ - Log error for debug purposes. - - Can be overridden by subclasses to change logging behaviour. - """ - logger.warning(u'Problem with connection to %s in %s: %s', - self.connection, self.actor_urn, error) - - def log_timeout(self): - """ - Log timeout for debug purposes. - - Can be overridden by subclasses to change logging behaviour. - """ - logger.debug(u'Closing connection to %s in %s due to timeout.', - self.connection, self.actor_urn) - def encode(self, line): """ Handle encoding of line. @@ -389,5 +347,4 @@ class LineProtocol(ThreadingActor): return data = self.join_lines(lines) - self.log_response(data) self.connection.send(self.encode(data)) diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index 5553af1a..369d3a44 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -837,3 +837,11 @@ class LineProtocolTest(unittest.TestCase): string.encode.side_effect = UnicodeError network.LineProtocol.encode(self.mock, string) + + @SkipTest + def test_host_property(self): + pass + + @SkipTest(self): + def test_port_property + pass From fe6e4a65f5764c6cf2b55897f56d10677c8f726f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 14 Jul 2011 11:09:04 +0200 Subject: [PATCH 55/72] Fix syntax error --- mopidy/utils/network.py | 2 -- tests/utils/network_test.py | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 936aab41..33d8efc0 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -9,8 +9,6 @@ from pykka import ActorDeadError from pykka.actor import ThreadingActor from pykka.registry import ActorRegistry -from mopidy.utils.log import indent - # FIXME setup logger with extra={...} logger = logging.getLogger('mopidy.utils.server') diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index 369d3a44..f1b23418 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -842,6 +842,6 @@ class LineProtocolTest(unittest.TestCase): def test_host_property(self): pass - @SkipTest(self): - def test_port_property + @SkipTest + def test_port_property(self): pass From 96ebb4eed45eba08b8a571673f25d1a910094732 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 14 Jul 2011 14:02:36 +0200 Subject: [PATCH 56/72] Indicate connection type in log message --- mopidy/frontends/mpd/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 37877386..4deb7b89 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -54,7 +54,7 @@ class MpdSession(network.LineProtocol): self.dispatcher = dispatcher.MpdDispatcher(self) def on_start(self): - logger.info(u'New connection from [%s]:%s', self.host, self.port) + logger.info(u'New MPD connection from [%s]:%s', self.host, self.port) self.send_lines([u'OK MPD %s' % protocol.VERSION]) def on_line_received(self, line): From e23476cc6fdc54e65a5fa382caadc238866b305d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 14 Jul 2011 23:05:08 +0200 Subject: [PATCH 57/72] Cleanup some tests --- tests/utils/network_test.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index f1b23418..f3619314 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -80,7 +80,16 @@ class ServerTest(unittest.TestCase): network.Server.__init__(self.mock, sentinel.host, sentinel.port, sentinel.protocol) - self.mock.register_server_socket.assert_called_once_with(sentinel.fileno) + self. mock.register_server_socket.assert_called_once_with(sentinel.fileno) + + @SkipTest + def test_init_fails_on_fileno_call(self): + sock = Mock(spec=socket.SocketType) + sock.fileno.side_effect = socket.error + self.mock.create_server_socket.return_value = sock + + network.Server.__init__(self.mock, sentinel.host, + sentinel.port, sentinel.protocol) def test_init_stores_values_in_attributes(self): sock = Mock(spec=socket.SocketType) @@ -105,10 +114,11 @@ class ServerTest(unittest.TestCase): sock.listen.assert_called_once_with(any_int) @SkipTest + @patch.object(network, 'create_socket') def test_create_server_socket_fails(self): - # FIXME define what should happen in this case, let the error propegate - # or do something else? - pass + create_socket.side_effect = socket.error + network.Server.create_server_socket(self.mock, + sentinel.host, sentinel.port) @patch.object(gobject, 'io_add_watch', new=Mock()) def test_register_server_socket_sets_up_io_watch(self): @@ -157,12 +167,10 @@ class ServerTest(unittest.TestCase): def test_accept_connection_unrecoverable_error(self): sock = Mock(spec=socket.SocketType) self.mock.server_socket = sock - sock.accept.side_effect = socket.error() self.assertRaises(socket.error, network.Server.accept_connection, self.mock) - @patch.object(network.Server, 'number_of_connections', new=Mock()) def test_maximum_connections_exceeded(self): self.mock.max_connections = 10 @@ -755,8 +763,8 @@ class LineProtocolTest(unittest.TestCase): lines = network.LineProtocol.parse_lines(self.mock) self.assertRaises(StopIteration, lines.next) self.assertEqual('data', self.mock.recv_buffer) - self.mock.recv_buffer += '\n' + lines = network.LineProtocol.parse_lines(self.mock) self.assertEqual('data', lines.next()) self.assertRaises(StopIteration, lines.next) From 805a6fefd0015cfc6f35ca0d889e06bf82772a30 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 14 Jul 2011 23:14:51 +0200 Subject: [PATCH 58/72] Lint fixing --- mopidy/utils/network.py | 1 - tests/utils/network_test.py | 23 +++++++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 33d8efc0..e1f2d8c4 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -9,7 +9,6 @@ from pykka import ActorDeadError from pykka.actor import ThreadingActor from pykka.registry import ActorRegistry -# FIXME setup logger with extra={...} logger = logging.getLogger('mopidy.utils.server') class ShouldRetrySocketCall(Exception): diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index f3619314..9b7abff3 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -80,7 +80,8 @@ class ServerTest(unittest.TestCase): network.Server.__init__(self.mock, sentinel.host, sentinel.port, sentinel.protocol) - self. mock.register_server_socket.assert_called_once_with(sentinel.fileno) + self.mock.register_server_socket.assert_called_once_with( + sentinel.fileno) @SkipTest def test_init_fails_on_fileno_call(self): @@ -116,7 +117,7 @@ class ServerTest(unittest.TestCase): @SkipTest @patch.object(network, 'create_socket') def test_create_server_socket_fails(self): - create_socket.side_effect = socket.error + network.create_socket.side_effect = socket.error network.Server.create_server_socket(self.mock, sentinel.host, sentinel.port) @@ -127,23 +128,29 @@ class ServerTest(unittest.TestCase): gobject.IO_IN, self.mock.handle_connection) def test_handle_connection(self): - self.mock.accept_connection.return_value = (sentinel.sock, sentinel.addr) + self.mock.accept_connection.return_value = ( + sentinel.sock, sentinel.addr) self.mock.maximum_connections_exceeded.return_value = False - network.Server.handle_connection(self.mock, sentinel.fileno, gobject.IO_IN) + network.Server.handle_connection( + self.mock, sentinel.fileno, gobject.IO_IN) self.mock.accept_connection.assert_called_once_with() self.mock.maximum_connections_exceeded.assert_called_once_with() - self.mock.init_connection.assert_called_once_with(sentinel.sock, sentinel.addr) + self.mock.init_connection.assert_called_once_with( + sentinel.sock, sentinel.addr) self.assertEqual(0, self.mock.reject_connection.call_count) def test_handle_connection_exceeded_connections(self): - self.mock.accept_connection.return_value = (sentinel.sock, sentinel.addr) + self.mock.accept_connection.return_value = ( + sentinel.sock, sentinel.addr) self.mock.maximum_connections_exceeded.return_value = True - network.Server.handle_connection(self.mock, sentinel.fileno, gobject.IO_IN) + network.Server.handle_connection( + self.mock, sentinel.fileno, gobject.IO_IN) self.mock.accept_connection.assert_called_once_with() self.mock.maximum_connections_exceeded.assert_called_once_with() - self.mock.reject_connection.assert_called_once_with(sentinel.sock, sentinel.addr) + self.mock.reject_connection.assert_called_once_with( + sentinel.sock, sentinel.addr) self.assertEqual(0, self.mock.init_connection.call_count) def test_accept_connection(self): From e6781135ba0cd7770a89f6612774855fb89f1ed1 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 15 Jul 2011 00:48:47 +0200 Subject: [PATCH 59/72] Doubled checked most network.Server/Connection/LineProtocol tests --- mopidy/utils/network.py | 37 +++++--- tests/utils/network_test.py | 168 +++++++++++++++++++++++++++++------- 2 files changed, 162 insertions(+), 43 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index e1f2d8c4..c0dd5043 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -95,6 +95,7 @@ class Server(object): return len(ActorRegistry.get_by_class(self.protocol)) def reject_connection(self, sock, addr): + # FIXME provide more context in logging? logger.warning(u'Rejected connection from [%s]:%s', addr[0], addr[1]) try: sock.close() @@ -104,6 +105,7 @@ class Server(object): def init_connection(self, sock, addr): Connection(self.protocol, sock, addr, self.timeout) + class Connection(object): # NOTE: the callback code is _not_ run in the actor's thread, but in the # same one as the event loop. If code in the callbacks blocks, the rest of @@ -164,16 +166,19 @@ class Connection(object): def enable_timeout(self): """Reactivate timeout mechanism.""" + if self.timeout <= 0: + return + self.disable_timeout() - if self.timeout > 0: - self.timeout_id = gobject.timeout_add_seconds( - self.timeout, self.timeout_callback) + self.timeout_id = gobject.timeout_add_seconds( + self.timeout, self.timeout_callback) def disable_timeout(self): """Deactivate timeout mechanism.""" - if self.timeout_id is not None: - gobject.source_remove(self.timeout_id) - self.timeout_id = None + if self.timeout_id is None: + return + gobject.source_remove(self.timeout_id) + self.timeout_id = None def enable_recv(self): if self.recv_id is not None: @@ -181,15 +186,16 @@ class Connection(object): try: self.recv_id = gobject.io_add_watch(self.sock.fileno(), - gobject.IO_IN | gobject.IO_ERR | gobject.IO_HUP, + gobject.IO_IN | gobject.IO_ERR | gobject.IO_HUP, self.recv_callback) except socket.error as e: self.stop(u'Problem with connection: %s' % e) def disable_recv(self): - if self.recv_id is not None: - gobject.source_remove(self.recv_id) - self.recv_id = None + if self.recv_id is None: + return + gobject.source_remove(self.recv_id) + self.recv_id = None def enable_send(self): if self.send_id is not None: @@ -203,9 +209,11 @@ class Connection(object): self.stop(u'Problem with connection: %s' % e) def disable_send(self): - if self.send_id is not None: - gobject.source_remove(self.send_id) - self.send_id = None + if self.send_id is None: + return + + gobject.source_remove(self.send_id) + self.send_id = None def recv_callback(self, fd, flags): if flags & (gobject.IO_ERR | gobject.IO_HUP): @@ -250,7 +258,8 @@ class Connection(object): return True def timeout_callback(self): - return self.stop(u'Client timeout out after %s seconds' % self.timeout) + self.stop(u'Client timeout out after %s seconds' % self.timeout) + return False class LineProtocol(ThreadingActor): diff --git a/tests/utils/network_test.py b/tests/utils/network_test.py index 9b7abff3..41dacda1 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network_test.py @@ -93,6 +93,7 @@ class ServerTest(unittest.TestCase): sentinel.port, sentinel.protocol) def test_init_stores_values_in_attributes(self): + # This need to be a mock and no a sentinel as fileno() is called on it sock = Mock(spec=socket.SocketType) self.mock.create_server_socket.return_value = sock @@ -114,13 +115,31 @@ class ServerTest(unittest.TestCase): sock.bind.assert_called_once_with((sentinel.host, sentinel.port)) sock.listen.assert_called_once_with(any_int) - @SkipTest + @SkipTest # FIXME decide behaviour @patch.object(network, 'create_socket') def test_create_server_socket_fails(self): network.create_socket.side_effect = socket.error network.Server.create_server_socket(self.mock, sentinel.host, sentinel.port) + @SkipTest # FIXME decide behaviour + @patch.object(network, 'create_socket', spec=socket.SocketType) + def test_create_server_bind_fails(self): + sock = create_socket.return_value + sock.bind.side_effect = socket.error + + network.Server.create_server_socket(self.mock, + sentinel.host, sentinel.port) + + @SkipTest # FIXME decide behaviour + @patch.object(network, 'create_socket', spec=socket.SocketType) + def test_create_server_listen_fails(self): + sock = create_socket.return_value + sock.listen.side_effect = socket.error + + network.Server.create_server_socket(self.mock, + sentinel.host, sentinel.port) + @patch.object(gobject, 'io_add_watch', new=Mock()) def test_register_server_socket_sets_up_io_watch(self): network.Server.register_server_socket(self.mock, sentinel.fileno) @@ -132,8 +151,8 @@ class ServerTest(unittest.TestCase): sentinel.sock, sentinel.addr) self.mock.maximum_connections_exceeded.return_value = False - network.Server.handle_connection( - self.mock, sentinel.fileno, gobject.IO_IN) + self.assertTrue(network.Server.handle_connection( + self.mock, sentinel.fileno, gobject.IO_IN)) self.mock.accept_connection.assert_called_once_with() self.mock.maximum_connections_exceeded.assert_called_once_with() self.mock.init_connection.assert_called_once_with( @@ -145,8 +164,8 @@ class ServerTest(unittest.TestCase): sentinel.sock, sentinel.addr) self.mock.maximum_connections_exceeded.return_value = True - network.Server.handle_connection( - self.mock, sentinel.fileno, gobject.IO_IN) + self.assertTrue(network.Server.handle_connection( + self.mock, sentinel.fileno, gobject.IO_IN)) self.mock.accept_connection.assert_called_once_with() self.mock.maximum_connections_exceeded.assert_called_once_with() self.mock.reject_connection.assert_called_once_with( @@ -171,10 +190,11 @@ class ServerTest(unittest.TestCase): self.assertRaises(network.ShouldRetrySocketCall, network.Server.accept_connection, self.mock) + # FIXME decide if this should be allowed to propegate def test_accept_connection_unrecoverable_error(self): sock = Mock(spec=socket.SocketType) self.mock.server_socket = sock - sock.accept.side_effect = socket.error() + sock.accept.side_effect = socket.error self.assertRaises(socket.error, network.Server.accept_connection, self.mock) @@ -218,7 +238,7 @@ class ServerTest(unittest.TestCase): def test_reject_connection_error(self): sock = Mock(spec=socket.SocketType) - sock.close.side_effect = socket.error() + sock.close.side_effect = socket.error network.Server.reject_connection(self.mock, sock, (sentinel.host, sentinel.port)) @@ -250,17 +270,29 @@ class ConnectionTest(unittest.TestCase): self.mock.enable_timeout.assert_called_once_with() def test_init_stores_values_in_attributes(self): + addr = (sentinel.host, sentinel.port) protocol = Mock(spec=network.LineProtocol) sock = Mock(spec=socket.SocketType) - network.Connection.__init__(self.mock, protocol, sock, - (sentinel.host, sentinel.port), sentinel.timeout) + network.Connection.__init__( + self.mock, protocol, sock, addr, sentinel.timeout) self.assertEqual(sock, self.mock.sock) self.assertEqual(protocol, self.mock.protocol) self.assertEqual(sentinel.timeout, self.mock.timeout) self.assertEqual(sentinel.host, self.mock.host) self.assertEqual(sentinel.port, self.mock.port) + def test_init_handles_ipv6_addr(self): + addr = (sentinel.host, sentinel.port, + sentinel.flowinfo, sentinel.scopeid) + protocol = Mock(spec=network.LineProtocol) + sock = Mock(spec=socket.SocketType) + + network.Connection.__init__( + self.mock, protocol, sock, addr, sentinel.timeout) + self.assertEqual(sentinel.host, self.mock.host) + self.assertEqual(sentinel.port, self.mock.port) + def test_stop_disables_recv_send_and_timeout(self): self.mock.stopping = False self.mock.actor_ref = Mock() @@ -269,7 +301,7 @@ class ConnectionTest(unittest.TestCase): network.Connection.stop(self.mock, sentinel.reason) self.mock.disable_timeout.assert_called_once_with() self.mock.disable_recv.assert_called_once_with() - self.mock.disable_timeout.assert_called_once_with() + self.mock.disable_send.assert_called_once_with() def test_stop_closes_socket(self): self.mock.stopping = False @@ -283,7 +315,7 @@ class ConnectionTest(unittest.TestCase): self.mock.stopping = False self.mock.actor_ref = Mock() self.mock.sock = Mock(spec=socket.SocketType) - self.mock.sock.close.side_effect = socket.error() + self.mock.sock.close.side_effect = socket.error network.Connection.stop(self.mock, sentinel.reason) self.mock.sock.close.assert_called_once_with() @@ -303,6 +335,7 @@ class ConnectionTest(unittest.TestCase): self.mock.sock = Mock(spec=socket.SocketType) network.Connection.stop(self.mock, sentinel.reason) + self.mock.actor_ref.stop.assert_called_once_with() def test_stop_sets_stopping_to_true(self): self.mock.stopping = False @@ -321,15 +354,6 @@ class ConnectionTest(unittest.TestCase): self.assertEqual(0, self.mock.actor_ref.stop.call_count) self.assertEqual(0, self.mock.sock.close.call_count) - @patch.object(network.logger, 'log', new=Mock()) - def test_stop_logs_that_it_is_calling_itself(self): - self.mock.stopping = True - self.mock.actor_ref = Mock() - self.mock.sock = Mock(spec=socket.SocketType) - - network.Connection.stop(self.mock, sentinel.reason) - network.logger.log(any_int, any_unicode) - @patch.object(network.logger, 'log', new=Mock()) def test_stop_logs_reason(self): self.mock.stopping = False @@ -351,6 +375,15 @@ class ConnectionTest(unittest.TestCase): network.logger.log.assert_called_once_with( sentinel.level, sentinel.reason) + @patch.object(network.logger, 'log', new=Mock()) + def test_stop_logs_that_it_is_calling_itself(self): + self.mock.stopping = True + self.mock.actor_ref = Mock() + self.mock.sock = Mock(spec=socket.SocketType) + + network.Connection.stop(self.mock, sentinel.reason) + network.logger.log(any_int, any_unicode) + @patch.object(gobject, 'io_add_watch', new=Mock()) def test_enable_recv_registers_with_gobject(self): self.mock.recv_id = None @@ -366,11 +399,19 @@ class ConnectionTest(unittest.TestCase): @patch.object(gobject, 'io_add_watch', new=Mock()) def test_enable_recv_already_registered(self): + self.mock.sock = Mock(spec=socket.SocketType) self.mock.recv_id = sentinel.tag network.Connection.enable_recv(self.mock) self.assertEqual(0, gobject.io_add_watch.call_count) + def test_enable_recv_does_not_change_tag(self): + self.mock.recv_id = sentinel.tag + self.mock.sock = Mock(spec=socket.SocketType) + + network.Connection.enable_recv(self.mock) + self.assertEqual(sentinel.tag, self.mock.recv_id) + @patch.object(gobject, 'source_remove', new=Mock()) def test_disable_recv_deregisters(self): self.mock.recv_id = sentinel.tag @@ -411,11 +452,19 @@ class ConnectionTest(unittest.TestCase): @patch.object(gobject, 'io_add_watch', new=Mock()) def test_enable_send_already_registered(self): + self.mock.sock = Mock(spec=socket.SocketType) self.mock.send_id = sentinel.tag network.Connection.enable_send(self.mock) self.assertEqual(0, gobject.io_add_watch.call_count) + def test_enable_send_does_not_change_tag(self): + self.mock.send_id = sentinel.tag + self.mock.sock = Mock(spec=socket.SocketType) + + network.Connection.enable_send(self.mock) + self.assertEqual(sentinel.tag, self.mock.send_id) + @patch.object(gobject, 'source_remove', new=Mock()) def test_disable_send_deregisters(self): self.mock.send_id = sentinel.tag @@ -471,6 +520,19 @@ class ConnectionTest(unittest.TestCase): network.Connection.enable_timeout(self.mock) self.assertEqual(0, gobject.timeout_add_seconds.call_count) + def test_enable_timeout_does_not_call_disable_for_invalid_timeout(self): + self.mock.timeout = 0 + network.Connection.enable_timeout(self.mock) + self.assertEqual(0, self.mock.disable_timeout.call_count) + + self.mock.timeout = -1 + network.Connection.enable_timeout(self.mock) + self.assertEqual(0, self.mock.disable_timeout.call_count) + + self.mock.timeout = None + network.Connection.enable_timeout(self.mock) + self.assertEqual(0, self.mock.disable_timeout.call_count) + @patch.object(gobject, 'source_remove', new=Mock()) def test_disable_timeout_deregisters(self): self.mock.timeout_id = sentinel.tag @@ -516,16 +578,25 @@ class ConnectionTest(unittest.TestCase): self.mock.enable_send.assert_called_once_with() def test_recv_callback_respects_io_err(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.actor_ref = Mock() + self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_ERR)) self.mock.stop.assert_called_once_with(any_unicode) def test_recv_callback_respects_io_hup(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.actor_ref = Mock() + self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_HUP)) self.mock.stop.assert_called_once_with(any_unicode) def test_recv_callback_respects_io_hup_and_io_err(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.actor_ref = Mock() + self.assertTrue(network.Connection.recv_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_HUP | gobject.IO_ERR)) self.mock.stop.assert_called_once_with(any_unicode) @@ -553,6 +624,7 @@ class ConnectionTest(unittest.TestCase): def test_recv_callback_gets_no_data(self): self.mock.sock = Mock(spec=socket.SocketType) self.mock.sock.recv.return_value = '' + self.mock.actor_ref = Mock() self.assertTrue(network.Connection.recv_callback( self.mock, sentinel.fd, gobject.IO_IN)) @@ -565,19 +637,42 @@ class ConnectionTest(unittest.TestCase): self.mock.sock.recv.side_effect = socket.error(error, '') self.assertTrue(network.Connection.recv_callback( self.mock, sentinel.fd, gobject.IO_IN)) + self.assertEqual(0, self.mock.stop.call_count) def test_recv_callback_unrecoverable_error(self): self.mock.sock = Mock(spec=socket.SocketType) - self.mock.sock.recv.side_effect = socket.error() + self.mock.sock.recv.side_effect = socket.error self.assertTrue(network.Connection.recv_callback( self.mock, sentinel.fd, gobject.IO_IN)) self.mock.stop.assert_called_once_with(any_unicode) - @SkipTest - def test_send_callback_respects_flags(self): - # stop self - pass + @SkipTest # FIXME decide behaviour + def test_send_callback_respects_io_err(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.actor_ref = Mock() + + self.assertTrue(network.Connection.send_callback(self.mock, + sentinel.fd, gobject.IO_IN | gobject.IO_ERR)) + self.mock.stop.assert_called_once_with(any_unicode) + + @SkipTest # FIXME decide behaviour + def test_send_callback_respects_io_hup(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.actor_ref = Mock() + + self.assertTrue(network.Connection.send_callback(self.mock, + sentinel.fd, gobject.IO_IN | gobject.IO_HUP)) + self.mock.stop.assert_called_once_with(any_unicode) + + @SkipTest # FIXME decide behaviour + def test_send_callback_respects_io_hup_and_io_err(self): + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.actor_ref = Mock() + + self.assertTrue(network.Connection.send_callback(self.mock, + sentinel.fd, gobject.IO_IN | gobject.IO_HUP | gobject.IO_ERR)) + self.mock.stop.assert_called_once_with(any_unicode) def test_send_callback_acquires_and_releases_lock(self): self.mock.send_lock = Mock() @@ -594,10 +689,14 @@ class ConnectionTest(unittest.TestCase): def test_send_callback_fails_to_acquire_lock(self): self.mock.send_lock = Mock() self.mock.send_lock.acquire.return_value = False + self.mock.send_buffer = '' + self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.send.return_value = 0 self.assertTrue(network.Connection.send_callback( self.mock, sentinel.fd, gobject.IO_IN)) self.mock.send_lock.acquire.assert_called_once_with(False) + self.assertEqual(0, self.mock.sock.send.call_count) def test_send_callback_sends_all_data(self): self.mock.send_lock = Mock() @@ -634,6 +733,7 @@ class ConnectionTest(unittest.TestCase): self.mock.sock.send.side_effect = socket.error(error, '') self.assertTrue(network.Connection.send_callback( self.mock, sentinel.fd, gobject.IO_IN)) + self.assertEqual(0, self.mock.stop.call_count) def test_send_callback_unrecoverable_error(self): self.mock.send_lock = Mock() @@ -641,7 +741,7 @@ class ConnectionTest(unittest.TestCase): self.mock.send_buffer = 'data' self.mock.sock = Mock(spec=socket.SocketType) - self.mock.sock.send.side_effect = socket.error() + self.mock.sock.send.side_effect = socket.error self.assertTrue(network.Connection.send_callback( self.mock, sentinel.fd, gobject.IO_IN)) self.mock.stop.assert_called_once_with(any_unicode) @@ -649,7 +749,7 @@ class ConnectionTest(unittest.TestCase): def test_timeout_callback(self): self.mock.timeout = 10 - network.Connection.timeout_callback(self.mock) + self.assertFalse(network.Connection.timeout_callback(self.mock)) self.mock.stop.assert_called_once_with(any_unicode) @@ -710,6 +810,16 @@ class LineProtocolTest(unittest.TestCase): network.LineProtocol.on_receive(self.mock, {'received': 'data\n'}) self.mock.on_line_received.assert_called_once_with(sentinel.decoded) + def test_on_receive_with_new_lines_calls_on_recieve(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = ['line1', 'line2'] + self.mock.decode.return_value = sentinel.decoded + + network.LineProtocol.on_receive(self.mock, + {'received': 'line1\nline2\n'}) + self.assertEqual(2, self.mock.on_line_received.call_count) + def test_parse_lines_emtpy_buffer(self): self.mock.recv_buffer = '' @@ -826,7 +936,7 @@ class LineProtocolTest(unittest.TestCase): self.assertEqual(u'æøå', network.LineProtocol.decode( self.mock, u'æøå'.encode('utf-8'))) - @SkipTest + @SkipTest # FIXME decide behaviour def test_decode_invalid_data(self): string = Mock() string.decode.side_effect = UnicodeError @@ -846,7 +956,7 @@ class LineProtocolTest(unittest.TestCase): self.assertEqual(u'æøå'.encode('utf-8'), network.LineProtocol.encode(self.mock, u'æøå')) - @SkipTest + @SkipTest # FIXME decide behaviour def test_encode_invalid_data(self): string = Mock() string.encode.side_effect = UnicodeError From 1b46dade83ee2e8ba55371e9071200d036696c55 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 15 Jul 2011 00:54:14 +0200 Subject: [PATCH 60/72] Note why source_remove return values are ignored --- mopidy/utils/network.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index c0dd5043..b08a12d6 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -110,6 +110,10 @@ class Connection(object): # NOTE: the callback code is _not_ run in the actor's thread, but in the # same one as the event loop. If code in the callbacks blocks, the rest of # gobject code will likely be blocked as well... + # + # Also note that source_remove() return values are ignored on purpose, a + # false return value would only tell us that what we thought was registered + # is already gone, there is really nothing more we can do. def __init__(self, protocol, sock, addr, timeout): sock.setblocking(False) From a1c382666f70a035691a6b6a6355a03ec486071f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 15 Jul 2011 01:08:29 +0200 Subject: [PATCH 61/72] Split up tests into multiple files --- tests/utils/network/__init__.py | 0 .../connection_test.py} | 454 ------------------ tests/utils/network/lineprotocol_test.py | 226 +++++++++ tests/utils/network/server_test.py | 190 ++++++++ tests/utils/network/utils_test.py | 57 +++ 5 files changed, 473 insertions(+), 454 deletions(-) create mode 100644 tests/utils/network/__init__.py rename tests/utils/{network_test.py => network/connection_test.py} (53%) create mode 100644 tests/utils/network/lineprotocol_test.py create mode 100644 tests/utils/network/server_test.py create mode 100644 tests/utils/network/utils_test.py diff --git a/tests/utils/network/__init__.py b/tests/utils/network/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/utils/network_test.py b/tests/utils/network/connection_test.py similarity index 53% rename from tests/utils/network_test.py rename to tests/utils/network/connection_test.py index 41dacda1..090d7e3c 100644 --- a/tests/utils/network_test.py +++ b/tests/utils/network/connection_test.py @@ -1,5 +1,3 @@ -#encoding: utf-8 - import errno import gobject import logging @@ -12,239 +10,6 @@ from mopidy.utils import network from mock import patch, sentinel, Mock from tests import SkipTest, any_int, any_unicode -class FormatHostnameTest(unittest.TestCase): - @patch('mopidy.utils.network.has_ipv6', True) - def test_format_hostname_prefixes_ipv4_addresses_when_ipv6_available(self): - network.has_ipv6 = True - self.assertEqual(network.format_hostname('0.0.0.0'), '::ffff:0.0.0.0') - self.assertEqual(network.format_hostname('1.0.0.1'), '::ffff:1.0.0.1') - - @patch('mopidy.utils.network.has_ipv6', False) - def test_format_hostname_does_nothing_when_only_ipv4_available(self): - network.has_ipv6 = False - self.assertEqual(network.format_hostname('0.0.0.0'), '0.0.0.0') - - -class TryIPv6SocketTest(unittest.TestCase): - @patch('socket.has_ipv6', False) - def test_system_that_claims_no_ipv6_support(self): - self.assertFalse(network.try_ipv6_socket()) - - @patch('socket.has_ipv6', True) - @patch('socket.socket') - def test_system_with_broken_ipv6(self, socket_mock): - socket_mock.side_effect = IOError() - self.assertFalse(network.try_ipv6_socket()) - - @patch('socket.has_ipv6', True) - @patch('socket.socket') - def test_with_working_ipv6(self, socket_mock): - socket_mock.return_value = Mock() - self.assertTrue(network.try_ipv6_socket()) - - -class CreateSocketTest(unittest.TestCase): - @patch('mopidy.utils.network.has_ipv6', False) - @patch('socket.socket') - def test_ipv4_socket(self, socket_mock): - network.create_socket() - self.assertEqual(socket_mock.call_args[0], - (socket.AF_INET, socket.SOCK_STREAM)) - - @patch('mopidy.utils.network.has_ipv6', True) - @patch('socket.socket') - def test_ipv6_socket(self, socket_mock): - network.create_socket() - self.assertEqual(socket_mock.call_args[0], - (socket.AF_INET6, socket.SOCK_STREAM)) - - @SkipTest - def test_ipv6_only_is_set(self): - pass - - -class ServerTest(unittest.TestCase): - def setUp(self): - self.mock = Mock(spec=network.Server) - - def test_init_calls_create_server_socket(self): - network.Server.__init__(self.mock, sentinel.host, - sentinel.port, sentinel.protocol) - self.mock.create_server_socket.assert_called_once_with( - sentinel.host, sentinel.port) - - def test_init_calls_register_server(self): - sock = Mock(spec=socket.SocketType) - sock.fileno.return_value = sentinel.fileno - self.mock.create_server_socket.return_value = sock - - network.Server.__init__(self.mock, sentinel.host, - sentinel.port, sentinel.protocol) - self.mock.register_server_socket.assert_called_once_with( - sentinel.fileno) - - @SkipTest - def test_init_fails_on_fileno_call(self): - sock = Mock(spec=socket.SocketType) - sock.fileno.side_effect = socket.error - self.mock.create_server_socket.return_value = sock - - network.Server.__init__(self.mock, sentinel.host, - sentinel.port, sentinel.protocol) - - def test_init_stores_values_in_attributes(self): - # This need to be a mock and no a sentinel as fileno() is called on it - sock = Mock(spec=socket.SocketType) - self.mock.create_server_socket.return_value = sock - - network.Server.__init__(self.mock, sentinel.host, sentinel.port, - sentinel.protocol, max_connections=sentinel.max_connections, - timeout=sentinel.timeout) - self.assertEqual(sentinel.protocol, self.mock.protocol) - self.assertEqual(sentinel.max_connections, self.mock.max_connections) - self.assertEqual(sentinel.timeout, self.mock.timeout) - self.assertEqual(sock, self.mock.server_socket) - - @patch.object(network, 'create_socket', spec=socket.SocketType) - def test_create_server_socket_sets_up_listener(self, create_socket): - sock = create_socket.return_value - - network.Server.create_server_socket(self.mock, - sentinel.host, sentinel.port) - sock.setblocking.assert_called_once_with(False) - sock.bind.assert_called_once_with((sentinel.host, sentinel.port)) - sock.listen.assert_called_once_with(any_int) - - @SkipTest # FIXME decide behaviour - @patch.object(network, 'create_socket') - def test_create_server_socket_fails(self): - network.create_socket.side_effect = socket.error - network.Server.create_server_socket(self.mock, - sentinel.host, sentinel.port) - - @SkipTest # FIXME decide behaviour - @patch.object(network, 'create_socket', spec=socket.SocketType) - def test_create_server_bind_fails(self): - sock = create_socket.return_value - sock.bind.side_effect = socket.error - - network.Server.create_server_socket(self.mock, - sentinel.host, sentinel.port) - - @SkipTest # FIXME decide behaviour - @patch.object(network, 'create_socket', spec=socket.SocketType) - def test_create_server_listen_fails(self): - sock = create_socket.return_value - sock.listen.side_effect = socket.error - - network.Server.create_server_socket(self.mock, - sentinel.host, sentinel.port) - - @patch.object(gobject, 'io_add_watch', new=Mock()) - def test_register_server_socket_sets_up_io_watch(self): - network.Server.register_server_socket(self.mock, sentinel.fileno) - gobject.io_add_watch.assert_called_once_with(sentinel.fileno, - gobject.IO_IN, self.mock.handle_connection) - - def test_handle_connection(self): - self.mock.accept_connection.return_value = ( - sentinel.sock, sentinel.addr) - self.mock.maximum_connections_exceeded.return_value = False - - self.assertTrue(network.Server.handle_connection( - self.mock, sentinel.fileno, gobject.IO_IN)) - self.mock.accept_connection.assert_called_once_with() - self.mock.maximum_connections_exceeded.assert_called_once_with() - self.mock.init_connection.assert_called_once_with( - sentinel.sock, sentinel.addr) - self.assertEqual(0, self.mock.reject_connection.call_count) - - def test_handle_connection_exceeded_connections(self): - self.mock.accept_connection.return_value = ( - sentinel.sock, sentinel.addr) - self.mock.maximum_connections_exceeded.return_value = True - - self.assertTrue(network.Server.handle_connection( - self.mock, sentinel.fileno, gobject.IO_IN)) - self.mock.accept_connection.assert_called_once_with() - self.mock.maximum_connections_exceeded.assert_called_once_with() - self.mock.reject_connection.assert_called_once_with( - sentinel.sock, sentinel.addr) - self.assertEqual(0, self.mock.init_connection.call_count) - - def test_accept_connection(self): - sock = Mock(spec=socket.SocketType) - sock.accept.return_value = (sentinel.sock, sentinel.addr) - self.mock.server_socket = sock - - sock, addr = network.Server.accept_connection(self.mock) - self.assertEqual(sentinel.sock, sock) - self.assertEqual(sentinel.addr, addr) - - def test_accept_connection_recoverable_error(self): - sock = Mock(spec=socket.SocketType) - self.mock.server_socket = sock - - for error in (errno.EAGAIN, errno.EINTR): - sock.accept.side_effect = socket.error(error, '') - self.assertRaises(network.ShouldRetrySocketCall, - network.Server.accept_connection, self.mock) - - # FIXME decide if this should be allowed to propegate - def test_accept_connection_unrecoverable_error(self): - sock = Mock(spec=socket.SocketType) - self.mock.server_socket = sock - sock.accept.side_effect = socket.error - self.assertRaises(socket.error, - network.Server.accept_connection, self.mock) - - def test_maximum_connections_exceeded(self): - self.mock.max_connections = 10 - - self.mock.number_of_connections.return_value = 11 - self.assertTrue(network.Server.maximum_connections_exceeded(self.mock)) - - self.mock.number_of_connections.return_value = 10 - self.assertTrue(network.Server.maximum_connections_exceeded(self.mock)) - - self.mock.number_of_connections.return_value = 9 - self.assertFalse(network.Server.maximum_connections_exceeded(self.mock)) - - @patch('pykka.registry.ActorRegistry.get_by_class') - def test_number_of_connections(self, get_by_class): - self.mock.protocol = sentinel.protocol - - get_by_class.return_value = [1, 2, 3] - self.assertEqual(3, network.Server.number_of_connections(self.mock)) - - get_by_class.return_value = [] - self.assertEqual(0, network.Server.number_of_connections(self.mock)) - - @patch.object(network, 'Connection', new=Mock()) - def test_init_connection(self): - self.mock.protocol = sentinel.protocol - self.mock.timeout = sentinel.timeout - - network.Server.init_connection(self.mock, sentinel.sock, sentinel.addr) - network.Connection.assert_called_once_with(sentinel.protocol, - sentinel.sock, sentinel.addr, sentinel.timeout) - - def test_reject_connection(self): - sock = Mock(spec=socket.SocketType) - - network.Server.reject_connection(self.mock, sock, - (sentinel.host, sentinel.port)) - sock.close.assert_called_once_with() - - def test_reject_connection_error(self): - sock = Mock(spec=socket.SocketType) - sock.close.side_effect = socket.error - - network.Server.reject_connection(self.mock, sock, - (sentinel.host, sentinel.port)) - sock.close.assert_called_once_with() - - class ConnectionTest(unittest.TestCase): def setUp(self): self.mock = Mock(spec=network.Connection) @@ -751,222 +516,3 @@ class ConnectionTest(unittest.TestCase): self.assertFalse(network.Connection.timeout_callback(self.mock)) self.mock.stop.assert_called_once_with(any_unicode) - - -class LineProtocolTest(unittest.TestCase): - def setUp(self): - self.mock = Mock(spec=network.LineProtocol) - self.mock.terminator = network.LineProtocol.terminator - self.mock.encoding = network.LineProtocol.encoding - - def test_init_stores_values_in_attributes(self): - network.LineProtocol.__init__(self.mock, sentinel.connection) - self.assertEqual(sentinel.connection, self.mock.connection) - self.assertEqual('', self.mock.recv_buffer) - - def test_on_receive_no_new_lines_adds_to_recv_buffer(self): - self.mock.connection = Mock(spec=network.Connection) - self.mock.recv_buffer = '' - self.mock.parse_lines.return_value = [] - - network.LineProtocol.on_receive(self.mock, {'received': 'data'}) - self.assertEqual('data', self.mock.recv_buffer) - self.mock.parse_lines.assert_called_once_with() - self.assertEqual(0, self.mock.on_line_received.call_count) - - def test_on_receive_no_new_lines_toggles_timeout(self): - self.mock.connection = Mock(spec=network.Connection) - self.mock.recv_buffer = '' - self.mock.parse_lines.return_value = [] - - network.LineProtocol.on_receive(self.mock, {'received': 'data'}) - self.mock.connection.disable_timeout.assert_called_once_with() - self.mock.connection.enable_timeout.assert_called_once_with() - - def test_on_receive_no_new_lines_calls_parse_lines(self): - self.mock.connection = Mock(spec=network.Connection) - self.mock.recv_buffer = '' - self.mock.parse_lines.return_value = [] - - network.LineProtocol.on_receive(self.mock, {'received': 'data'}) - self.mock.parse_lines.assert_called_once_with() - self.assertEqual(0, self.mock.on_line_received.call_count) - - def test_on_receive_with_new_line_calls_decode(self): - self.mock.connection = Mock(spec=network.Connection) - self.mock.recv_buffer = '' - self.mock.parse_lines.return_value = [sentinel.line] - - network.LineProtocol.on_receive(self.mock, {'received': 'data\n'}) - self.mock.parse_lines.assert_called_once_with() - self.mock.decode.assert_called_once_with(sentinel.line) - - def test_on_receive_with_new_line_calls_on_recieve(self): - self.mock.connection = Mock(spec=network.Connection) - self.mock.recv_buffer = '' - self.mock.parse_lines.return_value = [sentinel.line] - self.mock.decode.return_value = sentinel.decoded - - network.LineProtocol.on_receive(self.mock, {'received': 'data\n'}) - self.mock.on_line_received.assert_called_once_with(sentinel.decoded) - - def test_on_receive_with_new_lines_calls_on_recieve(self): - self.mock.connection = Mock(spec=network.Connection) - self.mock.recv_buffer = '' - self.mock.parse_lines.return_value = ['line1', 'line2'] - self.mock.decode.return_value = sentinel.decoded - - network.LineProtocol.on_receive(self.mock, - {'received': 'line1\nline2\n'}) - self.assertEqual(2, self.mock.on_line_received.call_count) - - def test_parse_lines_emtpy_buffer(self): - self.mock.recv_buffer = '' - - lines = network.LineProtocol.parse_lines(self.mock) - self.assertRaises(StopIteration, lines.next) - - def test_parse_lines_no_terminator(self): - self.mock.recv_buffer = 'data' - - lines = network.LineProtocol.parse_lines(self.mock) - self.assertRaises(StopIteration, lines.next) - - def test_parse_lines_termintor(self): - self.mock.recv_buffer = 'data\n' - - lines = network.LineProtocol.parse_lines(self.mock) - self.assertEqual('data', lines.next()) - self.assertRaises(StopIteration, lines.next) - self.assertEqual('', self.mock.recv_buffer) - - def test_parse_lines_no_data_before_terminator(self): - self.mock.recv_buffer = '\n' - - lines = network.LineProtocol.parse_lines(self.mock) - self.assertEqual('', lines.next()) - self.assertRaises(StopIteration, lines.next) - self.assertEqual('', self.mock.recv_buffer) - - def test_parse_lines_extra_data_after_terminator(self): - self.mock.recv_buffer = 'data1\ndata2' - - lines = network.LineProtocol.parse_lines(self.mock) - self.assertEqual('data1', lines.next()) - self.assertRaises(StopIteration, lines.next) - self.assertEqual('data2', self.mock.recv_buffer) - - def test_parse_lines_unicode(self): - self.mock.recv_buffer = u'æøå\n'.encode('utf-8') - - lines = network.LineProtocol.parse_lines(self.mock) - self.assertEqual(u'æøå'.encode('utf-8'), lines.next()) - self.assertRaises(StopIteration, lines.next) - self.assertEqual('', self.mock.recv_buffer) - - def test_parse_lines_multiple_lines(self): - self.mock.recv_buffer = 'abc\ndef\nghi\njkl' - - lines = network.LineProtocol.parse_lines(self.mock) - self.assertEqual('abc', lines.next()) - self.assertEqual('def', lines.next()) - self.assertEqual('ghi', lines.next()) - self.assertRaises(StopIteration, lines.next) - self.assertEqual('jkl', self.mock.recv_buffer) - - def test_parse_lines_multiple_calls(self): - self.mock.recv_buffer = 'data' - - lines = network.LineProtocol.parse_lines(self.mock) - self.assertRaises(StopIteration, lines.next) - self.assertEqual('data', self.mock.recv_buffer) - self.mock.recv_buffer += '\n' - - lines = network.LineProtocol.parse_lines(self.mock) - self.assertEqual('data', lines.next()) - self.assertRaises(StopIteration, lines.next) - self.assertEqual('', self.mock.recv_buffer) - - def test_send_lines_called_with_no_lines(self): - self.mock.connection = Mock(spec=network.Connection) - - network.LineProtocol.send_lines(self.mock, []) - self.assertEqual(0, self.mock.encode.call_count) - self.assertEqual(0, self.mock.connection.send.call_count) - - def test_send_lines_calls_join_lines(self): - self.mock.connection = Mock(spec=network.Connection) - self.mock.join_lines.return_value = 'lines' - - network.LineProtocol.send_lines(self.mock, sentinel.lines) - self.mock.join_lines.assert_called_once_with(sentinel.lines) - - def test_send_line_encodes_joined_lines_with_final_terminator(self): - self.mock.connection = Mock(spec=network.Connection) - self.mock.join_lines.return_value = u'lines\n' - - network.LineProtocol.send_lines(self.mock, sentinel.lines) - self.mock.encode.assert_called_once_with(u'lines\n') - - def test_send_lines_sends_encoded_string(self): - self.mock.connection = Mock(spec=network.Connection) - self.mock.join_lines.return_value = 'lines' - 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) - - def test_join_lines_returns_empty_string_for_no_lines(self): - self.assertEqual(u'', network.LineProtocol.join_lines(self.mock, [])) - - def test_join_lines_returns_joined_lines(self): - self.assertEqual(u'1\n2\n', network.LineProtocol.join_lines( - self.mock, [u'1', u'2'])) - - def test_decode_calls_decode_on_string(self): - string = Mock() - - network.LineProtocol.decode(self.mock, string) - string.decode.assert_called_once_with(self.mock.encoding) - - def test_decode_plain_ascii(self): - self.assertEqual(u'abc', network.LineProtocol.decode(self.mock, 'abc')) - - def test_decode_utf8(self): - self.assertEqual(u'æøå', network.LineProtocol.decode( - self.mock, u'æøå'.encode('utf-8'))) - - @SkipTest # FIXME decide behaviour - def test_decode_invalid_data(self): - string = Mock() - string.decode.side_effect = UnicodeError - - network.LineProtocol.decode(self.mock, string) - - def test_encode_calls_encode_on_string(self): - string = Mock() - - network.LineProtocol.encode(self.mock, string) - string.encode.assert_called_once_with(self.mock.encoding) - - def test_encode_plain_ascii(self): - self.assertEqual('abc', network.LineProtocol.encode(self.mock, u'abc')) - - def test_encode_utf8(self): - self.assertEqual(u'æøå'.encode('utf-8'), - network.LineProtocol.encode(self.mock, u'æøå')) - - @SkipTest # FIXME decide behaviour - def test_encode_invalid_data(self): - string = Mock() - string.encode.side_effect = UnicodeError - - network.LineProtocol.encode(self.mock, string) - - @SkipTest - def test_host_property(self): - pass - - @SkipTest - def test_port_property(self): - pass diff --git a/tests/utils/network/lineprotocol_test.py b/tests/utils/network/lineprotocol_test.py new file mode 100644 index 00000000..836c3109 --- /dev/null +++ b/tests/utils/network/lineprotocol_test.py @@ -0,0 +1,226 @@ +#encoding: utf-8 + +import unittest + +from mopidy.utils import network + +from mock import sentinel, Mock +from tests import SkipTest + +class LineProtocolTest(unittest.TestCase): + def setUp(self): + self.mock = Mock(spec=network.LineProtocol) + self.mock.terminator = network.LineProtocol.terminator + self.mock.encoding = network.LineProtocol.encoding + + def test_init_stores_values_in_attributes(self): + network.LineProtocol.__init__(self.mock, sentinel.connection) + self.assertEqual(sentinel.connection, self.mock.connection) + self.assertEqual('', self.mock.recv_buffer) + + def test_on_receive_no_new_lines_adds_to_recv_buffer(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = [] + + network.LineProtocol.on_receive(self.mock, {'received': 'data'}) + self.assertEqual('data', self.mock.recv_buffer) + self.mock.parse_lines.assert_called_once_with() + self.assertEqual(0, self.mock.on_line_received.call_count) + + def test_on_receive_no_new_lines_toggles_timeout(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = [] + + network.LineProtocol.on_receive(self.mock, {'received': 'data'}) + self.mock.connection.disable_timeout.assert_called_once_with() + self.mock.connection.enable_timeout.assert_called_once_with() + + def test_on_receive_no_new_lines_calls_parse_lines(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = [] + + network.LineProtocol.on_receive(self.mock, {'received': 'data'}) + self.mock.parse_lines.assert_called_once_with() + self.assertEqual(0, self.mock.on_line_received.call_count) + + def test_on_receive_with_new_line_calls_decode(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = [sentinel.line] + + network.LineProtocol.on_receive(self.mock, {'received': 'data\n'}) + self.mock.parse_lines.assert_called_once_with() + self.mock.decode.assert_called_once_with(sentinel.line) + + def test_on_receive_with_new_line_calls_on_recieve(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = [sentinel.line] + self.mock.decode.return_value = sentinel.decoded + + network.LineProtocol.on_receive(self.mock, {'received': 'data\n'}) + self.mock.on_line_received.assert_called_once_with(sentinel.decoded) + + def test_on_receive_with_new_lines_calls_on_recieve(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.recv_buffer = '' + self.mock.parse_lines.return_value = ['line1', 'line2'] + self.mock.decode.return_value = sentinel.decoded + + network.LineProtocol.on_receive(self.mock, + {'received': 'line1\nline2\n'}) + self.assertEqual(2, self.mock.on_line_received.call_count) + + def test_parse_lines_emtpy_buffer(self): + self.mock.recv_buffer = '' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertRaises(StopIteration, lines.next) + + def test_parse_lines_no_terminator(self): + self.mock.recv_buffer = 'data' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertRaises(StopIteration, lines.next) + + def test_parse_lines_termintor(self): + self.mock.recv_buffer = 'data\n' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual('data', lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('', self.mock.recv_buffer) + + def test_parse_lines_no_data_before_terminator(self): + self.mock.recv_buffer = '\n' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual('', lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('', self.mock.recv_buffer) + + def test_parse_lines_extra_data_after_terminator(self): + self.mock.recv_buffer = 'data1\ndata2' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual('data1', lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('data2', self.mock.recv_buffer) + + def test_parse_lines_unicode(self): + self.mock.recv_buffer = u'æøå\n'.encode('utf-8') + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual(u'æøå'.encode('utf-8'), lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('', self.mock.recv_buffer) + + def test_parse_lines_multiple_lines(self): + self.mock.recv_buffer = 'abc\ndef\nghi\njkl' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual('abc', lines.next()) + self.assertEqual('def', lines.next()) + self.assertEqual('ghi', lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('jkl', self.mock.recv_buffer) + + def test_parse_lines_multiple_calls(self): + self.mock.recv_buffer = 'data' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('data', self.mock.recv_buffer) + self.mock.recv_buffer += '\n' + + lines = network.LineProtocol.parse_lines(self.mock) + self.assertEqual('data', lines.next()) + self.assertRaises(StopIteration, lines.next) + self.assertEqual('', self.mock.recv_buffer) + + def test_send_lines_called_with_no_lines(self): + self.mock.connection = Mock(spec=network.Connection) + + network.LineProtocol.send_lines(self.mock, []) + self.assertEqual(0, self.mock.encode.call_count) + self.assertEqual(0, self.mock.connection.send.call_count) + + def test_send_lines_calls_join_lines(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.join_lines.return_value = 'lines' + + network.LineProtocol.send_lines(self.mock, sentinel.lines) + self.mock.join_lines.assert_called_once_with(sentinel.lines) + + def test_send_line_encodes_joined_lines_with_final_terminator(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.join_lines.return_value = u'lines\n' + + network.LineProtocol.send_lines(self.mock, sentinel.lines) + self.mock.encode.assert_called_once_with(u'lines\n') + + def test_send_lines_sends_encoded_string(self): + self.mock.connection = Mock(spec=network.Connection) + self.mock.join_lines.return_value = 'lines' + 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) + + def test_join_lines_returns_empty_string_for_no_lines(self): + self.assertEqual(u'', network.LineProtocol.join_lines(self.mock, [])) + + def test_join_lines_returns_joined_lines(self): + self.assertEqual(u'1\n2\n', network.LineProtocol.join_lines( + self.mock, [u'1', u'2'])) + + def test_decode_calls_decode_on_string(self): + string = Mock() + + network.LineProtocol.decode(self.mock, string) + string.decode.assert_called_once_with(self.mock.encoding) + + def test_decode_plain_ascii(self): + self.assertEqual(u'abc', network.LineProtocol.decode(self.mock, 'abc')) + + def test_decode_utf8(self): + self.assertEqual(u'æøå', network.LineProtocol.decode( + self.mock, u'æøå'.encode('utf-8'))) + + @SkipTest # FIXME decide behaviour + def test_decode_invalid_data(self): + string = Mock() + string.decode.side_effect = UnicodeError + + network.LineProtocol.decode(self.mock, string) + + def test_encode_calls_encode_on_string(self): + string = Mock() + + network.LineProtocol.encode(self.mock, string) + string.encode.assert_called_once_with(self.mock.encoding) + + def test_encode_plain_ascii(self): + self.assertEqual('abc', network.LineProtocol.encode(self.mock, u'abc')) + + def test_encode_utf8(self): + self.assertEqual(u'æøå'.encode('utf-8'), + network.LineProtocol.encode(self.mock, u'æøå')) + + @SkipTest # FIXME decide behaviour + def test_encode_invalid_data(self): + string = Mock() + string.encode.side_effect = UnicodeError + + network.LineProtocol.encode(self.mock, string) + + @SkipTest + def test_host_property(self): + pass + + @SkipTest + def test_port_property(self): + pass diff --git a/tests/utils/network/server_test.py b/tests/utils/network/server_test.py new file mode 100644 index 00000000..c844a487 --- /dev/null +++ b/tests/utils/network/server_test.py @@ -0,0 +1,190 @@ +import errno +import gobject +import socket +import unittest + +from mopidy.utils import network + +from mock import patch, sentinel, Mock +from tests import SkipTest, any_int + +class ServerTest(unittest.TestCase): + def setUp(self): + self.mock = Mock(spec=network.Server) + + def test_init_calls_create_server_socket(self): + network.Server.__init__(self.mock, sentinel.host, + sentinel.port, sentinel.protocol) + self.mock.create_server_socket.assert_called_once_with( + sentinel.host, sentinel.port) + + def test_init_calls_register_server(self): + sock = Mock(spec=socket.SocketType) + sock.fileno.return_value = sentinel.fileno + self.mock.create_server_socket.return_value = sock + + network.Server.__init__(self.mock, sentinel.host, + sentinel.port, sentinel.protocol) + self.mock.register_server_socket.assert_called_once_with( + sentinel.fileno) + + @SkipTest + def test_init_fails_on_fileno_call(self): + sock = Mock(spec=socket.SocketType) + sock.fileno.side_effect = socket.error + self.mock.create_server_socket.return_value = sock + + network.Server.__init__(self.mock, sentinel.host, + sentinel.port, sentinel.protocol) + + def test_init_stores_values_in_attributes(self): + # This need to be a mock and no a sentinel as fileno() is called on it + sock = Mock(spec=socket.SocketType) + self.mock.create_server_socket.return_value = sock + + network.Server.__init__(self.mock, sentinel.host, sentinel.port, + sentinel.protocol, max_connections=sentinel.max_connections, + timeout=sentinel.timeout) + self.assertEqual(sentinel.protocol, self.mock.protocol) + self.assertEqual(sentinel.max_connections, self.mock.max_connections) + self.assertEqual(sentinel.timeout, self.mock.timeout) + self.assertEqual(sock, self.mock.server_socket) + + @patch.object(network, 'create_socket', spec=socket.SocketType) + def test_create_server_socket_sets_up_listener(self, create_socket): + sock = create_socket.return_value + + network.Server.create_server_socket(self.mock, + sentinel.host, sentinel.port) + sock.setblocking.assert_called_once_with(False) + sock.bind.assert_called_once_with((sentinel.host, sentinel.port)) + sock.listen.assert_called_once_with(any_int) + + @SkipTest # FIXME decide behaviour + @patch.object(network, 'create_socket') + def test_create_server_socket_fails(self): + network.create_socket.side_effect = socket.error + network.Server.create_server_socket(self.mock, + sentinel.host, sentinel.port) + + @SkipTest # FIXME decide behaviour + @patch.object(network, 'create_socket', spec=socket.SocketType) + def test_create_server_bind_fails(self): + sock = create_socket.return_value + sock.bind.side_effect = socket.error + + network.Server.create_server_socket(self.mock, + sentinel.host, sentinel.port) + + @SkipTest # FIXME decide behaviour + @patch.object(network, 'create_socket', spec=socket.SocketType) + def test_create_server_listen_fails(self): + sock = create_socket.return_value + sock.listen.side_effect = socket.error + + network.Server.create_server_socket(self.mock, + sentinel.host, sentinel.port) + + @patch.object(gobject, 'io_add_watch', new=Mock()) + def test_register_server_socket_sets_up_io_watch(self): + network.Server.register_server_socket(self.mock, sentinel.fileno) + gobject.io_add_watch.assert_called_once_with(sentinel.fileno, + gobject.IO_IN, self.mock.handle_connection) + + def test_handle_connection(self): + self.mock.accept_connection.return_value = ( + sentinel.sock, sentinel.addr) + self.mock.maximum_connections_exceeded.return_value = False + + self.assertTrue(network.Server.handle_connection( + self.mock, sentinel.fileno, gobject.IO_IN)) + self.mock.accept_connection.assert_called_once_with() + self.mock.maximum_connections_exceeded.assert_called_once_with() + self.mock.init_connection.assert_called_once_with( + sentinel.sock, sentinel.addr) + self.assertEqual(0, self.mock.reject_connection.call_count) + + def test_handle_connection_exceeded_connections(self): + self.mock.accept_connection.return_value = ( + sentinel.sock, sentinel.addr) + self.mock.maximum_connections_exceeded.return_value = True + + self.assertTrue(network.Server.handle_connection( + self.mock, sentinel.fileno, gobject.IO_IN)) + self.mock.accept_connection.assert_called_once_with() + self.mock.maximum_connections_exceeded.assert_called_once_with() + self.mock.reject_connection.assert_called_once_with( + sentinel.sock, sentinel.addr) + self.assertEqual(0, self.mock.init_connection.call_count) + + def test_accept_connection(self): + sock = Mock(spec=socket.SocketType) + sock.accept.return_value = (sentinel.sock, sentinel.addr) + self.mock.server_socket = sock + + sock, addr = network.Server.accept_connection(self.mock) + self.assertEqual(sentinel.sock, sock) + self.assertEqual(sentinel.addr, addr) + + def test_accept_connection_recoverable_error(self): + sock = Mock(spec=socket.SocketType) + self.mock.server_socket = sock + + for error in (errno.EAGAIN, errno.EINTR): + sock.accept.side_effect = socket.error(error, '') + self.assertRaises(network.ShouldRetrySocketCall, + network.Server.accept_connection, self.mock) + + # FIXME decide if this should be allowed to propegate + def test_accept_connection_unrecoverable_error(self): + sock = Mock(spec=socket.SocketType) + self.mock.server_socket = sock + sock.accept.side_effect = socket.error + self.assertRaises(socket.error, + network.Server.accept_connection, self.mock) + + def test_maximum_connections_exceeded(self): + self.mock.max_connections = 10 + + self.mock.number_of_connections.return_value = 11 + self.assertTrue(network.Server.maximum_connections_exceeded(self.mock)) + + self.mock.number_of_connections.return_value = 10 + self.assertTrue(network.Server.maximum_connections_exceeded(self.mock)) + + self.mock.number_of_connections.return_value = 9 + self.assertFalse(network.Server.maximum_connections_exceeded(self.mock)) + + @patch('pykka.registry.ActorRegistry.get_by_class') + def test_number_of_connections(self, get_by_class): + self.mock.protocol = sentinel.protocol + + get_by_class.return_value = [1, 2, 3] + self.assertEqual(3, network.Server.number_of_connections(self.mock)) + + get_by_class.return_value = [] + self.assertEqual(0, network.Server.number_of_connections(self.mock)) + + @patch.object(network, 'Connection', new=Mock()) + def test_init_connection(self): + self.mock.protocol = sentinel.protocol + self.mock.timeout = sentinel.timeout + + network.Server.init_connection(self.mock, sentinel.sock, sentinel.addr) + network.Connection.assert_called_once_with(sentinel.protocol, + sentinel.sock, sentinel.addr, sentinel.timeout) + + def test_reject_connection(self): + sock = Mock(spec=socket.SocketType) + + network.Server.reject_connection(self.mock, sock, + (sentinel.host, sentinel.port)) + sock.close.assert_called_once_with() + + def test_reject_connection_error(self): + sock = Mock(spec=socket.SocketType) + sock.close.side_effect = socket.error + + network.Server.reject_connection(self.mock, sock, + (sentinel.host, sentinel.port)) + sock.close.assert_called_once_with() diff --git a/tests/utils/network/utils_test.py b/tests/utils/network/utils_test.py new file mode 100644 index 00000000..ada1de01 --- /dev/null +++ b/tests/utils/network/utils_test.py @@ -0,0 +1,57 @@ +import socket +import unittest + +from mopidy.utils import network + +from mock import patch, Mock +from tests import SkipTest + +class FormatHostnameTest(unittest.TestCase): + @patch('mopidy.utils.network.has_ipv6', True) + def test_format_hostname_prefixes_ipv4_addresses_when_ipv6_available(self): + network.has_ipv6 = True + self.assertEqual(network.format_hostname('0.0.0.0'), '::ffff:0.0.0.0') + self.assertEqual(network.format_hostname('1.0.0.1'), '::ffff:1.0.0.1') + + @patch('mopidy.utils.network.has_ipv6', False) + def test_format_hostname_does_nothing_when_only_ipv4_available(self): + network.has_ipv6 = False + self.assertEqual(network.format_hostname('0.0.0.0'), '0.0.0.0') + + +class TryIPv6SocketTest(unittest.TestCase): + @patch('socket.has_ipv6', False) + def test_system_that_claims_no_ipv6_support(self): + self.assertFalse(network.try_ipv6_socket()) + + @patch('socket.has_ipv6', True) + @patch('socket.socket') + def test_system_with_broken_ipv6(self, socket_mock): + socket_mock.side_effect = IOError() + self.assertFalse(network.try_ipv6_socket()) + + @patch('socket.has_ipv6', True) + @patch('socket.socket') + def test_with_working_ipv6(self, socket_mock): + socket_mock.return_value = Mock() + self.assertTrue(network.try_ipv6_socket()) + + +class CreateSocketTest(unittest.TestCase): + @patch('mopidy.utils.network.has_ipv6', False) + @patch('socket.socket') + def test_ipv4_socket(self, socket_mock): + network.create_socket() + self.assertEqual(socket_mock.call_args[0], + (socket.AF_INET, socket.SOCK_STREAM)) + + @patch('mopidy.utils.network.has_ipv6', True) + @patch('socket.socket') + def test_ipv6_socket(self, socket_mock): + network.create_socket() + self.assertEqual(socket_mock.call_args[0], + (socket.AF_INET6, socket.SOCK_STREAM)) + + @SkipTest + def test_ipv6_only_is_set(self): + pass From 91b450bd6ba03ba2a9bc5af51734e097697dd92d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 16 Jul 2011 22:56:43 +0200 Subject: [PATCH 62/72] Add tests for line protocol host and port properties --- tests/utils/network/lineprotocol_test.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/utils/network/lineprotocol_test.py b/tests/utils/network/lineprotocol_test.py index 836c3109..d339890c 100644 --- a/tests/utils/network/lineprotocol_test.py +++ b/tests/utils/network/lineprotocol_test.py @@ -217,10 +217,16 @@ class LineProtocolTest(unittest.TestCase): network.LineProtocol.encode(self.mock, string) - @SkipTest def test_host_property(self): - pass + mock = Mock(spec=network.Connection) + mock.host = sentinel.host + + lineprotocol = network.LineProtocol(mock) + self.assertEqual(sentinel.host, lineprotocol.host) - @SkipTest def test_port_property(self): - pass + mock = Mock(spec=network.Connection) + mock.port = sentinel.port + + lineprotocol = network.LineProtocol(mock) + self.assertEqual(sentinel.port, lineprotocol.port) From ffd4ae5045fd783472fedfab41bffdefb1ce416d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 17 Jul 2011 00:06:10 +0200 Subject: [PATCH 63/72] Some more test cleanup and improvement --- tests/utils/network/lineprotocol_test.py | 32 +++++++++++++++--------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/tests/utils/network/lineprotocol_test.py b/tests/utils/network/lineprotocol_test.py index d339890c..360b5c68 100644 --- a/tests/utils/network/lineprotocol_test.py +++ b/tests/utils/network/lineprotocol_test.py @@ -28,7 +28,7 @@ class LineProtocolTest(unittest.TestCase): self.mock.parse_lines.assert_called_once_with() self.assertEqual(0, self.mock.on_line_received.call_count) - def test_on_receive_no_new_lines_toggles_timeout(self): + def test_on_receive_toggles_timeout(self): self.mock.connection = Mock(spec=network.Connection) self.mock.recv_buffer = '' self.mock.parse_lines.return_value = [] @@ -129,17 +129,18 @@ class LineProtocolTest(unittest.TestCase): self.assertEqual('jkl', self.mock.recv_buffer) def test_parse_lines_multiple_calls(self): - self.mock.recv_buffer = 'data' + self.mock.recv_buffer = 'data1' lines = network.LineProtocol.parse_lines(self.mock) self.assertRaises(StopIteration, lines.next) - self.assertEqual('data', self.mock.recv_buffer) - self.mock.recv_buffer += '\n' + self.assertEqual('data1', self.mock.recv_buffer) + + self.mock.recv_buffer += '\ndata2' lines = network.LineProtocol.parse_lines(self.mock) - self.assertEqual('data', lines.next()) + self.assertEqual('data1', lines.next()) self.assertRaises(StopIteration, lines.next) - self.assertEqual('', self.mock.recv_buffer) + self.assertEqual('data2', self.mock.recv_buffer) def test_send_lines_called_with_no_lines(self): self.mock.connection = Mock(spec=network.Connection) @@ -184,11 +185,15 @@ class LineProtocolTest(unittest.TestCase): string.decode.assert_called_once_with(self.mock.encoding) def test_decode_plain_ascii(self): - self.assertEqual(u'abc', network.LineProtocol.decode(self.mock, 'abc')) + result = network.LineProtocol.decode(self.mock, 'abc') + self.assertEqual(u'abc', result) + self.assertEqual(unicode, type(result)) def test_decode_utf8(self): - self.assertEqual(u'æøå', network.LineProtocol.decode( - self.mock, u'æøå'.encode('utf-8'))) + result = network.LineProtocol.decode( + self.mock, u'æøå'.encode('utf-8')) + self.assertEqual(u'æøå', result) + self.assertEqual(unicode, type(result)) @SkipTest # FIXME decide behaviour def test_decode_invalid_data(self): @@ -204,11 +209,14 @@ class LineProtocolTest(unittest.TestCase): string.encode.assert_called_once_with(self.mock.encoding) def test_encode_plain_ascii(self): - self.assertEqual('abc', network.LineProtocol.encode(self.mock, u'abc')) + result = network.LineProtocol.encode(self.mock, u'abc') + self.assertEqual('abc', result) + self.assertEqual(str, type(result)) def test_encode_utf8(self): - self.assertEqual(u'æøå'.encode('utf-8'), - network.LineProtocol.encode(self.mock, u'æøå')) + result = network.LineProtocol.encode(self.mock, u'æøå') + self.assertEqual(u'æøå'.encode('utf-8'), result) + self.assertEqual(str, type(result)) @SkipTest # FIXME decide behaviour def test_encode_invalid_data(self): From c773998fd8f9187423216ca93acbd232f167585a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 17 Jul 2011 00:26:33 +0200 Subject: [PATCH 64/72] Stop actor if decode or encode fails --- mopidy/utils/network.py | 10 ++++++++-- tests/utils/network/lineprotocol_test.py | 5 ++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index b08a12d6..9de688cc 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -331,7 +331,10 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change encoding behaviour. """ - return line.encode(self.encoding) + try: + return line.encode(self.encoding) + except UnicodeError: # FIXME log this? + self.stop() def decode(self, line): """ @@ -339,7 +342,10 @@ class LineProtocol(ThreadingActor): Can be overridden by subclasses to change decoding behaviour. """ - return line.decode(self.encoding) + try: + return line.decode(self.encoding) + except UnicodeError: # FIXME log this? + self.stop() def join_lines(self, lines): if not lines: diff --git a/tests/utils/network/lineprotocol_test.py b/tests/utils/network/lineprotocol_test.py index 360b5c68..a87f461c 100644 --- a/tests/utils/network/lineprotocol_test.py +++ b/tests/utils/network/lineprotocol_test.py @@ -5,7 +5,6 @@ import unittest from mopidy.utils import network from mock import sentinel, Mock -from tests import SkipTest class LineProtocolTest(unittest.TestCase): def setUp(self): @@ -195,12 +194,12 @@ class LineProtocolTest(unittest.TestCase): self.assertEqual(u'æøå', result) self.assertEqual(unicode, type(result)) - @SkipTest # FIXME decide behaviour def test_decode_invalid_data(self): string = Mock() string.decode.side_effect = UnicodeError network.LineProtocol.decode(self.mock, string) + self.mock.stop.assert_called_once_with() def test_encode_calls_encode_on_string(self): string = Mock() @@ -218,12 +217,12 @@ class LineProtocolTest(unittest.TestCase): self.assertEqual(u'æøå'.encode('utf-8'), result) self.assertEqual(str, type(result)) - @SkipTest # FIXME decide behaviour def test_encode_invalid_data(self): string = Mock() string.encode.side_effect = UnicodeError network.LineProtocol.encode(self.mock, string) + self.mock.stop.assert_called_once_with() def test_host_property(self): mock = Mock(spec=network.Connection) From d2a9e3d1ecda18c454d1253638c62154487b567b Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 17 Jul 2011 00:32:50 +0200 Subject: [PATCH 65/72] Make send_callback respect flags from gobject --- mopidy/utils/network.py | 4 ++++ tests/utils/network/connection_test.py | 14 ++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 9de688cc..852b6ca6 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -243,6 +243,10 @@ class Connection(object): return True def send_callback(self, fd, flags): + if flags & (gobject.IO_ERR | gobject.IO_HUP): + self.stop(u'Bad client flags: %s' % flags) + return True + # If with can't get the lock, simply try again next time socket is # ready for sending. if not self.send_lock.acquire(False): diff --git a/tests/utils/network/connection_test.py b/tests/utils/network/connection_test.py index 090d7e3c..6e68f250 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 SkipTest, any_int, any_unicode +from tests import any_int, any_unicode class ConnectionTest(unittest.TestCase): def setUp(self): @@ -412,28 +412,34 @@ class ConnectionTest(unittest.TestCase): self.mock, sentinel.fd, gobject.IO_IN)) self.mock.stop.assert_called_once_with(any_unicode) - @SkipTest # FIXME decide behaviour def test_send_callback_respects_io_err(self): self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.send.return_value = 1 + self.mock.send_lock = Mock() self.mock.actor_ref = Mock() + self.mock.send_buffer = '' self.assertTrue(network.Connection.send_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_ERR)) self.mock.stop.assert_called_once_with(any_unicode) - @SkipTest # FIXME decide behaviour def test_send_callback_respects_io_hup(self): self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.send.return_value = 1 + self.mock.send_lock = Mock() self.mock.actor_ref = Mock() + self.mock.send_buffer = '' self.assertTrue(network.Connection.send_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_HUP)) self.mock.stop.assert_called_once_with(any_unicode) - @SkipTest # FIXME decide behaviour def test_send_callback_respects_io_hup_and_io_err(self): self.mock.sock = Mock(spec=socket.SocketType) + self.mock.sock.send.return_value = 1 + self.mock.send_lock = Mock() self.mock.actor_ref = Mock() + self.mock.send_buffer = '' self.assertTrue(network.Connection.send_callback(self.mock, sentinel.fd, gobject.IO_IN | gobject.IO_HUP | gobject.IO_ERR)) From d07a758f68d38fcfc36956020ba7863f7e226098 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 17 Jul 2011 00:42:13 +0200 Subject: [PATCH 66/72] Update tests to reflect that server's socket errors should simply not be handeled --- tests/utils/network/server_test.py | 32 +++++++++++++----------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/tests/utils/network/server_test.py b/tests/utils/network/server_test.py index c844a487..75b33d61 100644 --- a/tests/utils/network/server_test.py +++ b/tests/utils/network/server_test.py @@ -6,7 +6,7 @@ import unittest from mopidy.utils import network from mock import patch, sentinel, Mock -from tests import SkipTest, any_int +from tests import any_int class ServerTest(unittest.TestCase): def setUp(self): @@ -28,14 +28,13 @@ class ServerTest(unittest.TestCase): self.mock.register_server_socket.assert_called_once_with( sentinel.fileno) - @SkipTest def test_init_fails_on_fileno_call(self): sock = Mock(spec=socket.SocketType) sock.fileno.side_effect = socket.error self.mock.create_server_socket.return_value = sock - network.Server.__init__(self.mock, sentinel.host, - sentinel.port, sentinel.protocol) + self.assertRaises(socket.error, network.Server.__init__, + self.mock, sentinel.host, sentinel.port, sentinel.protocol) def test_init_stores_values_in_attributes(self): # This need to be a mock and no a sentinel as fileno() is called on it @@ -60,30 +59,27 @@ class ServerTest(unittest.TestCase): sock.bind.assert_called_once_with((sentinel.host, sentinel.port)) sock.listen.assert_called_once_with(any_int) - @SkipTest # FIXME decide behaviour - @patch.object(network, 'create_socket') + @patch.object(network, 'create_socket', new=Mock()) def test_create_server_socket_fails(self): network.create_socket.side_effect = socket.error - network.Server.create_server_socket(self.mock, - sentinel.host, sentinel.port) + self.assertRaises(socket.error, network.Server.create_server_socket, + self.mock, sentinel.host, sentinel.port) - @SkipTest # FIXME decide behaviour - @patch.object(network, 'create_socket', spec=socket.SocketType) + @patch.object(network, 'create_socket', new=Mock()) def test_create_server_bind_fails(self): - sock = create_socket.return_value + sock = network.create_socket.return_value sock.bind.side_effect = socket.error - network.Server.create_server_socket(self.mock, - sentinel.host, sentinel.port) + self.assertRaises(socket.error, network.Server.create_server_socket, + self.mock, sentinel.host, sentinel.port) - @SkipTest # FIXME decide behaviour - @patch.object(network, 'create_socket', spec=socket.SocketType) + @patch.object(network, 'create_socket', new=Mock()) def test_create_server_listen_fails(self): - sock = create_socket.return_value + sock = network.create_socket.return_value sock.listen.side_effect = socket.error - network.Server.create_server_socket(self.mock, - sentinel.host, sentinel.port) + self.assertRaises(socket.error, network.Server.create_server_socket, + self.mock, sentinel.host, sentinel.port) @patch.object(gobject, 'io_add_watch', new=Mock()) def test_register_server_socket_sets_up_io_watch(self): From 0f976fe4e03ca9c85edd61b6a6a3232df1521ad9 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 17 Jul 2011 02:38:41 +0200 Subject: [PATCH 67/72] Add changes notes about network code changes --- docs/changes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index 5d2ab57d..21011139 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -20,6 +20,9 @@ v0.6.0 (in development) - Add Listener API, :mod:`mopidy.listeners`, to be implemented by actors wanting to receive events from the backend. This is a formalization of the ad hoc events the Last.fm scrobbler has already been using for some time. +- Replaced all of the MPD network code that was provided by asyncore with + custom stack. This change was made to facilitate the future support of the + `idle` command, and to reduce the number of event loops being used. v0.5.0 (2011-06-15) From 83ccee0bf58dee09632b11ad2f13095e1ec79125 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 17 Jul 2011 03:11:02 +0200 Subject: [PATCH 68/72] Set metadata to ' ' when data is mising (fixes: #122) --- docs/changes.rst | 1 + mopidy/gstreamer.py | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index 5d2ab57d..5506bfb0 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -20,6 +20,7 @@ v0.6.0 (in development) - Add Listener API, :mod:`mopidy.listeners`, to be implemented by actors wanting to receive events from the backend. This is a formalization of the ad hoc events the Last.fm scrobbler has already been using for some time. +- Fix metadata update in Shoutcast streaming (Fixes: :issue:`122`) v0.5.0 (2011-06-15) diff --git a/mopidy/gstreamer.py b/mopidy/gstreamer.py index 166c487e..b5e38b92 100644 --- a/mopidy/gstreamer.py +++ b/mopidy/gstreamer.py @@ -277,10 +277,18 @@ class GStreamer(ThreadingActor): taglist = gst.TagList() artists = [a for a in (track.artists or []) if a.name] + # Default to blank data to trick shoutcast into clearing any previous + # values it might have. + taglist[gst.TAG_ARTIST] = u' ' + taglist[gst.TAG_TITLE] = u' ' + taglist[gst.TAG_ALBUM] = u' ' + if artists: taglist[gst.TAG_ARTIST] = u', '.join([a.name for a in artists]) + if track.name: taglist[gst.TAG_TITLE] = track.name + if track.album and track.album.name: taglist[gst.TAG_ALBUM] = track.album.name From bc6162ca05b81267687cf6fa2cae107bc509eedd Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 17 Jul 2011 03:16:48 +0200 Subject: [PATCH 69/72] Remove outdated refrence to mopidy.utils.process.GObjectEventThread --- mopidy/gstreamer.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/mopidy/gstreamer.py b/mopidy/gstreamer.py index 166c487e..43c5ea2b 100644 --- a/mopidy/gstreamer.py +++ b/mopidy/gstreamer.py @@ -43,9 +43,6 @@ class GStreamer(ThreadingActor): self._handlers = {} def on_start(self): - # **Warning:** :class:`GStreamer` requires - # :class:`mopidy.utils.process.GObjectEventThread` to be running. This - # is not enforced by :class:`GStreamer` itself. self._setup_pipeline() self._setup_outputs() self._setup_message_processor() From 292d0e26cf88b29d0d168bd1028e8e9ab7026635 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 17 Jul 2011 04:18:24 +0200 Subject: [PATCH 70/72] Fix minor issue in get_class bug caused by bad user input. --- mopidy/utils/__init__.py | 4 +++- tests/utils/init_test.py | 9 +++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/mopidy/utils/__init__.py b/mopidy/utils/__init__.py index acbb4664..9d7532a0 100644 --- a/mopidy/utils/__init__.py +++ b/mopidy/utils/__init__.py @@ -18,9 +18,11 @@ def import_module(name): return sys.modules[name] def get_class(name): + logger.debug('Loading: %s', name) + if '.' not in name: + raise ImportError("Couldn't load: %s" % name) module_name = name[:name.rindex('.')] class_name = name[name.rindex('.') + 1:] - logger.debug('Loading: %s', name) try: module = import_module(module_name) class_object = getattr(module, class_name) diff --git a/tests/utils/init_test.py b/tests/utils/init_test.py index fb38e2ea..70dd7e36 100644 --- a/tests/utils/init_test.py +++ b/tests/utils/init_test.py @@ -4,12 +4,13 @@ from mopidy.utils import get_class class GetClassTest(unittest.TestCase): def test_loading_module_that_does_not_exist(self): - test = lambda: get_class('foo.bar.Baz') - self.assertRaises(ImportError, test) + self.assertRaises(ImportError, get_class, 'foo.bar.Baz') def test_loading_class_that_does_not_exist(self): - test = lambda: get_class('unittest.FooBarBaz') - self.assertRaises(ImportError, test) + self.assertRaises(ImportError, get_class, 'unittest.FooBarBaz') + + def test_loading_incorrect_class_path(self): + self.assertRaises(ImportError, get_class, 'foobarbaz') def test_import_error_message_contains_complete_class_path(self): try: From 23775dfe1add8f00d4095ab71687a0394f9b286f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 17 Jul 2011 23:44:00 +0200 Subject: [PATCH 71/72] Fix up last comments regarding typo and more logging --- mopidy/utils/network.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 852b6ca6..b7f808c9 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -320,7 +320,7 @@ class LineProtocol(ThreadingActor): def on_stop(self): """Ensure that cleanup when actor stops.""" - self.connection.stop(u'Actor is shuting down.') + self.connection.stop(u'Actor is shutting down.') def parse_lines(self): """Consume new data and yield any lines found.""" @@ -337,7 +337,9 @@ class LineProtocol(ThreadingActor): """ try: return line.encode(self.encoding) - except UnicodeError: # FIXME log this? + except UnicodeError: + logger.warning(u'Stoping actor due to encode problem, data ' + 'supplied by client was not valid %s', self.encoding) self.stop() def decode(self, line): @@ -348,7 +350,9 @@ class LineProtocol(ThreadingActor): """ try: return line.decode(self.encoding) - except UnicodeError: # FIXME log this? + except UnicodeError: + logger.warning(u'Stoping actor due to decode problem, data ' + 'supplied by client was not valid %s', self.encoding) self.stop() def join_lines(self, lines): From 6cf5deb21651b403d1719f18d2af77ee26537b8f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 17 Jul 2011 23:52:32 +0200 Subject: [PATCH 72/72] Typo fix :) --- mopidy/utils/network.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index b7f808c9..b7cc144d 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -338,7 +338,7 @@ class LineProtocol(ThreadingActor): try: return line.encode(self.encoding) except UnicodeError: - logger.warning(u'Stoping actor due to encode problem, data ' + logger.warning(u'Stopping actor due to encode problem, data ' 'supplied by client was not valid %s', self.encoding) self.stop() @@ -351,7 +351,7 @@ class LineProtocol(ThreadingActor): try: return line.decode(self.encoding) except UnicodeError: - logger.warning(u'Stoping actor due to decode problem, data ' + logger.warning(u'Stopping actor due to decode problem, data ' 'supplied by client was not valid %s', self.encoding) self.stop()