From 20a1a2ad70d0be6f1341c3464596a847e4d41b7b Mon Sep 17 00:00:00 2001 From: Felipe Polo-Wood Date: Fri, 8 Nov 2013 13:26:27 -0500 Subject: [PATCH 1/4] Fixed router so response and headers are sent correctly Fixed tests so they run on Windows Added unit test for new method get_content_type --- openlp/plugins/remotes/lib/httprouter.py | 50 +++++++++++-------- .../openlp_plugins/remotes/test_remotetab.py | 3 +- .../openlp_plugins/remotes/test_router.py | 20 +++++++- 3 files changed, 48 insertions(+), 25 deletions(-) diff --git a/openlp/plugins/remotes/lib/httprouter.py b/openlp/plugins/remotes/lib/httprouter.py index ce13ed812..eb2b68e6b 100644 --- a/openlp/plugins/remotes/lib/httprouter.py +++ b/openlp/plugins/remotes/lib/httprouter.py @@ -346,30 +346,14 @@ class HttpRouter(object): path = os.path.normpath(os.path.join(self.html_dir, file_name)) if not path.startswith(self.html_dir): return self.do_not_found() - ext = os.path.splitext(file_name)[1] - html = None - if ext == '.html': - self.send_header('Content-type', 'text/html') - variables = self.template_vars - html = Template(filename=path, input_encoding='utf-8', output_encoding='utf-8').render(**variables) - elif ext == '.css': - self.send_header('Content-type', 'text/css') - elif ext == '.js': - self.send_header('Content-type', 'application/javascript') - elif ext == '.jpg': - self.send_header('Content-type', 'image/jpeg') - elif ext == '.gif': - self.send_header('Content-type', 'image/gif') - elif ext == '.ico': - self.send_header('Content-type', 'image/x-icon') - elif ext == '.png': - self.send_header('Content-type', 'image/png') - else: - self.send_header('Content-type', 'text/plain') + content = None + ext, content_type = self.get_content_type(path) file_handle = None try: - if html: - content = html + if ext == '.html': + variables = self.template_vars + content = Template(filename=path, input_encoding='utf-8', + output_encoding='utf-8').render(**variables) else: file_handle = open(path, 'rb') log.debug('Opened %s' % path) @@ -380,8 +364,30 @@ class HttpRouter(object): finally: if file_handle: file_handle.close() + self.send_response(200) + self.send_header('Content-type', content_type) + self.end_headers() return content + def get_content_type(self, file_name): + """ + Examines the extension of the file and determines + what header to send back + Returns the extension found + """ + content_type = 'text/plain' + file_types = {'.html': 'text/html', + '.css': 'text/css', + '.js': 'application/javascript', + '.jpg': 'image/jpeg', + '.gif': 'image/gif', + '.ico': 'image/x-icon', + '.png': 'image/png' + } + ext = os.path.splitext(file_name)[1] + content_type = file_types.get(ext, 'text/plain') + return ext, content_type + def poll(self): """ Poll OpenLP to determine the current slide number and item name. diff --git a/tests/functional/openlp_plugins/remotes/test_remotetab.py b/tests/functional/openlp_plugins/remotes/test_remotetab.py index 067c5cff1..52aeeee99 100644 --- a/tests/functional/openlp_plugins/remotes/test_remotetab.py +++ b/tests/functional/openlp_plugins/remotes/test_remotetab.py @@ -62,7 +62,7 @@ class TestRemoteTab(TestCase): """ Create the UI """ - fd, self.ini_file = mkstemp('.ini') + self.fd, self.ini_file = mkstemp('.ini') Settings().set_filename(self.ini_file) self.application = QtGui.QApplication.instance() Settings().extend_default_settings(__default_settings__) @@ -76,6 +76,7 @@ class TestRemoteTab(TestCase): del self.application del self.parent del self.form + os.close(self.fd) os.unlink(self.ini_file) def get_ip_address_default_test(self): diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index a9ba16bf8..6762f7643 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -59,7 +59,7 @@ class TestRouter(TestCase): """ Create the UI """ - fd, self.ini_file = mkstemp('.ini') + self.fd, self.ini_file = mkstemp('.ini') Settings().set_filename(self.ini_file) self.application = QtGui.QApplication.instance() Settings().extend_default_settings(__default_settings__) @@ -70,6 +70,7 @@ class TestRouter(TestCase): Delete all the C++ objects at the end so that we don't have a segfault """ del self.application + os.close(self.fd) os.unlink(self.ini_file) def password_encrypter_test(self): @@ -109,4 +110,19 @@ class TestRouter(TestCase): assert function['function'] == mocked_function, \ 'The mocked function should match defined value.' assert function['secure'] == False, \ - 'The mocked function should not require any security.' \ No newline at end of file + 'The mocked function should not require any security.' + + def get_content_type_test(self): + """ + Test the get_content_type logic + """ + headers = [ ['test.html', 'text/html'], ['test.css', 'text/css'], + ['test.js', 'application/javascript'], ['test.jpg', 'image/jpeg'], + ['test.gif', 'image/gif'], ['test.ico', 'image/x-icon'], + ['test.png', 'image/png'], ['test.whatever', 'text/plain'], + ['test', 'text/plain'], ['', 'text/plain'], + ['/test/test.html', 'text/html'], + ['c:\\test\\test.html', 'text/html']] + for header in headers: + ext, content_type = self.router.get_content_type(header[0]) + self.assertEqual(content_type, header[1], 'Mismatch of content type') From 45bcbe92728fd5d822c3d86d9c3612827e3de4fe Mon Sep 17 00:00:00 2001 From: Felipe Polo-Wood Date: Sun, 10 Nov 2013 19:13:03 -0500 Subject: [PATCH 2/4] Moved the file_types to module level so they get processed once --- openlp/plugins/remotes/lib/httprouter.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/openlp/plugins/remotes/lib/httprouter.py b/openlp/plugins/remotes/lib/httprouter.py index eb2b68e6b..b3652d4df 100644 --- a/openlp/plugins/remotes/lib/httprouter.py +++ b/openlp/plugins/remotes/lib/httprouter.py @@ -128,6 +128,15 @@ from openlp.core.common import AppLocation, Settings, translate from openlp.core.lib import Registry, PluginStatus, StringContent, image_to_byte log = logging.getLogger(__name__) +file_types = { + '.html': 'text/html', + '.css': 'text/css', + '.js': 'application/javascript', + '.jpg': 'image/jpeg', + '.gif': 'image/gif', + '.ico': 'image/x-icon', + '.png': 'image/png' +} class HttpRouter(object): @@ -372,18 +381,10 @@ class HttpRouter(object): def get_content_type(self, file_name): """ Examines the extension of the file and determines - what header to send back - Returns the extension found + what the content_type should be, defaults to text/plain + Returns the extension and the content_type """ content_type = 'text/plain' - file_types = {'.html': 'text/html', - '.css': 'text/css', - '.js': 'application/javascript', - '.jpg': 'image/jpeg', - '.gif': 'image/gif', - '.ico': 'image/x-icon', - '.png': 'image/png' - } ext = os.path.splitext(file_name)[1] content_type = file_types.get(ext, 'text/plain') return ext, content_type From 649494589a0cf38694203b0f321ad08afb27cc7a Mon Sep 17 00:00:00 2001 From: Felipe Polo-Wood Date: Sun, 10 Nov 2013 19:55:06 -0500 Subject: [PATCH 3/4] Added unit tests for serve_file --- .../openlp_plugins/remotes/test_router.py | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index 6762f7643..5475405a0 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -37,7 +37,7 @@ from PyQt4 import QtGui from openlp.core.common import Settings from openlp.plugins.remotes.lib.httpserver import HttpRouter -from tests.functional import MagicMock +from mock import MagicMock, patch, mock_open __default_settings__ = { 'remotes/twelve hour': True, @@ -116,6 +116,7 @@ class TestRouter(TestCase): """ Test the get_content_type logic """ + # GIVEN: a set of files and their corresponding types headers = [ ['test.html', 'text/html'], ['test.css', 'text/css'], ['test.js', 'application/javascript'], ['test.jpg', 'image/jpeg'], ['test.gif', 'image/gif'], ['test.ico', 'image/x-icon'], @@ -123,6 +124,50 @@ class TestRouter(TestCase): ['test', 'text/plain'], ['', 'text/plain'], ['/test/test.html', 'text/html'], ['c:\\test\\test.html', 'text/html']] + # WHEN: calling each file type for header in headers: ext, content_type = self.router.get_content_type(header[0]) + # THEN: all types should match self.assertEqual(content_type, header[1], 'Mismatch of content type') + + def serve_file_without_params_test(self): + """ + Test the serve_file method without params + """ + # GIVEN: mocked environment + self.router.send_response = MagicMock() + self.router.send_header = MagicMock() + self.router.end_headers = MagicMock() + self.router.wfile = MagicMock() + self.router.html_dir = os.path.normpath('test/dir') + self.router.template_vars = MagicMock() + # WHEN: call serve_file with no file_name + self.router.serve_file() + # THEN: it should return a 404 + self.router.send_response.assert_called_once_with(404) + self.router.send_header.assert_called_once_with('Content-type','text/html') + self.assertEqual(self.router.end_headers.call_count, 1, + 'end_headers called once') + + def serve_file_with_valid_params_test(self): + """ + Test the serve_file method with an existing file + """ + # GIVEN: mocked environment + self.router.send_response = MagicMock() + self.router.send_header = MagicMock() + self.router.end_headers = MagicMock() + self.router.wfile = MagicMock() + self.router.html_dir = os.path.normpath('test/dir') + self.router.template_vars = MagicMock() + with patch('openlp.core.lib.os.path.exists') as mocked_exists, \ + patch('builtins.open', mock_open(read_data='123')): + mocked_exists.return_value = True + # WHEN: call serve_file with an existing html file + self.router.serve_file(os.path.normpath('test/dir/test.html')) + # THEN: it should return a 200 and the file + self.router.send_response.assert_called_once_with(200) + self.router.send_header.assert_called_once_with( + 'Content-type','text/html') + self.assertEqual(self.router.end_headers.call_count, 1, + 'end_headers called once') From e56414e22c3c82855e4eec426afb29acd6b535cb Mon Sep 17 00:00:00 2001 From: Felipe Polo-Wood Date: Thu, 14 Nov 2013 15:33:46 -0500 Subject: [PATCH 4/4] Fixed OS specific string Coding standard fixes --- openlp/plugins/remotes/lib/httprouter.py | 7 +++---- tests/functional/openlp_plugins/remotes/test_router.py | 7 ++++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/openlp/plugins/remotes/lib/httprouter.py b/openlp/plugins/remotes/lib/httprouter.py index b3652d4df..deba92c10 100644 --- a/openlp/plugins/remotes/lib/httprouter.py +++ b/openlp/plugins/remotes/lib/httprouter.py @@ -128,7 +128,7 @@ from openlp.core.common import AppLocation, Settings, translate from openlp.core.lib import Registry, PluginStatus, StringContent, image_to_byte log = logging.getLogger(__name__) -file_types = { +FILE_TYPES = { '.html': 'text/html', '.css': 'text/css', '.js': 'application/javascript', @@ -361,8 +361,7 @@ class HttpRouter(object): try: if ext == '.html': variables = self.template_vars - content = Template(filename=path, input_encoding='utf-8', - output_encoding='utf-8').render(**variables) + content = Template(filename=path, input_encoding='utf-8', output_encoding='utf-8').render(**variables) else: file_handle = open(path, 'rb') log.debug('Opened %s' % path) @@ -386,7 +385,7 @@ class HttpRouter(object): """ content_type = 'text/plain' ext = os.path.splitext(file_name)[1] - content_type = file_types.get(ext, 'text/plain') + content_type = FILE_TYPES.get(ext, 'text/plain') return ext, content_type def poll(self): diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index 5475405a0..4d7da2b91 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -37,7 +37,8 @@ from PyQt4 import QtGui from openlp.core.common import Settings from openlp.plugins.remotes.lib.httpserver import HttpRouter -from mock import MagicMock, patch, mock_open +from tests.functional import MagicMock, patch +from mock import mock_open __default_settings__ = { 'remotes/twelve hour': True, @@ -50,6 +51,7 @@ __default_settings__ = { 'remotes/ip address': '0.0.0.0' } +TEST_PATH = os.path.abspath(os.path.dirname(__file__)) class TestRouter(TestCase): """ @@ -122,8 +124,7 @@ class TestRouter(TestCase): ['test.gif', 'image/gif'], ['test.ico', 'image/x-icon'], ['test.png', 'image/png'], ['test.whatever', 'text/plain'], ['test', 'text/plain'], ['', 'text/plain'], - ['/test/test.html', 'text/html'], - ['c:\\test\\test.html', 'text/html']] + [os.path.join(TEST_PATH,'test.html'), 'text/html']] # WHEN: calling each file type for header in headers: ext, content_type = self.router.get_content_type(header[0])