diff --git a/openlp/core/lib/imagemanager.py b/openlp/core/lib/imagemanager.py index e85f38d64..0854b6599 100644 --- a/openlp/core/lib/imagemanager.py +++ b/openlp/core/lib/imagemanager.py @@ -36,6 +36,7 @@ import logging import os import time import queue +import re from PyQt4 import QtCore @@ -105,7 +106,7 @@ class Image(object): """ secondary_priority = 0 - def __init__(self, path, source, background): + def __init__(self, path, source, background, dimensions=''): """ Create an image for the :class:`ImageManager`'s cache. @@ -127,6 +128,15 @@ class Image(object): self.source = source self.background = background self.timestamp = 0 + match = re.search('(\d+)x(\d+)', dimensions) + if match: + # let's make sure that the dimensions are within reason + self.width = sorted([10, int(match.group(1)), 1000])[1] + self.height = sorted([10, int(match.group(2)), 1000])[1] + else: + # -1 means use the default dimension in ImageManager + self.width = -1 + self.height = -1 # FIXME: We assume that the path exist. The caller has to take care that it exists! if os.path.exists(path): self.timestamp = os.stat(path).st_mtime @@ -217,13 +227,13 @@ class ImageManager(QtCore.QObject): image.background = background self._reset_image(image) - def update_image_border(self, path, source, background): + def update_image_border(self, path, source, background, dimensions=''): """ Border has changed so update the image affected. """ log.debug('update_image_border') # Mark the image as dirty for a rebuild by setting the image and byte stream to None. - image = self._cache[(path, source)] + image = self._cache[(path, source, dimensions)] if image.source == source: image.background = background self._reset_image(image) @@ -244,12 +254,12 @@ class ImageManager(QtCore.QObject): if not self.image_thread.isRunning(): self.image_thread.start() - def get_image(self, path, source): + def get_image(self, path, source, dimensions=''): """ Return the ``QImage`` from the cache. If not present wait for the background thread to process it. """ log.debug('getImage %s' % path) - image = self._cache[(path, source)] + image = self._cache[(path, source, dimensions)] if image.image is None: self._conversion_queue.modify_priority(image, Priority.High) # make sure we are running and if not give it a kick @@ -264,12 +274,12 @@ class ImageManager(QtCore.QObject): self._conversion_queue.modify_priority(image, Priority.Low) return image.image - def get_image_bytes(self, path, source): + def get_image_bytes(self, path, source, dimensions=''): """ Returns the byte string for an image. If not present wait for the background thread to process it. """ log.debug('get_image_bytes %s' % path) - image = self._cache[(path, source)] + image = self._cache[(path, source, dimensions)] if image.image_bytes is None: self._conversion_queue.modify_priority(image, Priority.Urgent) # make sure we are running and if not give it a kick @@ -279,14 +289,14 @@ class ImageManager(QtCore.QObject): time.sleep(0.1) return image.image_bytes - def add_image(self, path, source, background): + def add_image(self, path, source, background, dimensions=''): """ Add image to cache if it is not already there. """ log.debug('add_image %s' % path) - if not (path, source) in self._cache: - image = Image(path, source, background) - self._cache[(path, source)] = image + if not (path, source, dimensions) in self._cache: + image = Image(path, source, background, dimensions) + self._cache[(path, source, dimensions)] = image self._conversion_queue.put((image.priority, image.secondary_priority, image)) # Check if the there are any images with the same path and check if the timestamp has changed. for image in list(self._cache.values()): @@ -315,7 +325,10 @@ class ImageManager(QtCore.QObject): image = self._conversion_queue.get()[2] # Generate the QImage for the image. if image.image is None: - image.image = resize_image(image.path, self.width, self.height, image.background) + # Let's see if the image was requested with specific dimensions + width = self.width if image.width == -1 else image.width + height = self.height if image.height == -1 else image.height + image.image = resize_image(image.path, width, height, image.background) # Set the priority to Lowest and stop here as we need to process more important images first. if image.priority == Priority.Normal: self._conversion_queue.modify_priority(image, Priority.Lowest) diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index 209472c8f..8931867c2 100644 --- a/openlp/core/lib/serviceitem.py +++ b/openlp/core/lib/serviceitem.py @@ -452,8 +452,8 @@ class ServiceItem(object): if path: self.has_original_files = False self.add_from_command(path, text_image['title'], - text_image['image'], text_image['display_title'], - text_image['notes']) + text_image['image'], text_image.get('display_title',''), + text_image.get('notes', '')) else: self.add_from_command(text_image['path'], text_image['title'], text_image['image']) diff --git a/openlp/plugins/remotes/lib/httprouter.py b/openlp/plugins/remotes/lib/httprouter.py index c820e126c..d5b95e68a 100644 --- a/openlp/plugins/remotes/lib/httprouter.py +++ b/openlp/plugins/remotes/lib/httprouter.py @@ -402,31 +402,25 @@ class HttpRouter(object): log.debug('serve thumbnail %s/thumbnails%s/%s' % (controller_name, dimensions, file_name)) supported_controllers = ['presentations'] + if not dimensions: + dimensions = '' content = '' if controller_name and file_name: if controller_name in supported_controllers: full_path = urllib.parse.unquote(file_name) if not '..' in full_path: # no hacking please - width = 80 - height = 80 - if dimensions: - match = re.search('(\d+)x(\d+)', - dimensions) - if match: - width = int(match.group(1)) - height = int(match.group(2)) - # let's make sure that the dimensions are within reason - width = min(width,1000) - width = max(width,10) - height = min(height,1000) - height = max(height,10) full_path = os.path.normpath(os.path.join( AppLocation.get_section_data_path(controller_name), 'thumbnails/' + full_path)) if os.path.exists(full_path): + path, just_file_name = os.path.split(full_path) + image_manager = Registry().get('image_manager') + image_manager.add_image(full_path, just_file_name, None, + dimensions) ext = self.send_appropriate_header(full_path) - content = image_to_byte(resize_image(full_path, width, - height),False) + content = image_to_byte( + image_manager.get_image(full_path, + just_file_name, dimensions), False) if len(content)==0: content = self.do_not_found() return content diff --git a/tests/functional/openlp_core_lib/test_image_manager.py b/tests/functional/openlp_core_lib/test_image_manager.py index 472011884..912a6c2a2 100644 --- a/tests/functional/openlp_core_lib/test_image_manager.py +++ b/tests/functional/openlp_core_lib/test_image_manager.py @@ -61,16 +61,17 @@ class TestImageManager(TestCase): Test the Image Manager setup basic functionality """ # GIVEN: the an image add to the image manager - self.image_manager.add_image(TEST_PATH, 'church.jpg', None) + full_path = os.path.normpath(os.path.join(TEST_PATH, 'church.jpg')) + self.image_manager.add_image(full_path, 'church.jpg', None) # WHEN the image is retrieved - image = self.image_manager.get_image(TEST_PATH, 'church.jpg') + image = self.image_manager.get_image(full_path, 'church.jpg') # THEN returned record is a type of image self.assertEqual(isinstance(image, QtGui.QImage), True, 'The returned object should be a QImage') # WHEN: The image bytes are requested. - byte_array = self.image_manager.get_image_bytes(TEST_PATH, 'church.jpg') + byte_array = self.image_manager.get_image_bytes(full_path, 'church.jpg') # THEN: Type should be a str. self.assertEqual(isinstance(byte_array, str), True, 'The returned object should be a str') @@ -80,3 +81,40 @@ class TestImageManager(TestCase): with self.assertRaises(KeyError) as context: self.image_manager.get_image(TEST_PATH, 'church1.jpg') self.assertNotEquals(context.exception, '', 'KeyError exception should have been thrown for missing image') + + def different_dimension_image_test(self): + """ + Test the Image Manager with dimensions + """ + # GIVEN: add an image with specific dimensions + full_path = os.path.normpath(os.path.join(TEST_PATH, 'church.jpg')) + self.image_manager.add_image(full_path, 'church.jpg', None, '80x80') + + # WHEN: the image is retrieved + image = self.image_manager.get_image(full_path, 'church.jpg', '80x80') + + # THEN: The return should be of type image + self.assertEqual(isinstance(image, QtGui.QImage), True, + 'The returned object should be a QImage') + #print(len(self.image_manager._cache)) + + # WHEN: adding the same image with different dimensions + self.image_manager.add_image(full_path, 'church.jpg', None, '100x100') + + # THEN: the cache should contain two pictures + self.assertEqual(len(self.image_manager._cache), 2, + 'Image manager should consider two dimensions of the same picture as different') + + # WHEN: adding the same image with first dimensions + self.image_manager.add_image(full_path, 'church.jpg', None, '80x80') + + # THEN: the cache should still contain only two pictures + self.assertEqual(len(self.image_manager._cache), 2, + 'Same dimensions should not be added again') + + # WHEN: calling with correct image, but wrong dimensions + with self.assertRaises(KeyError) as context: + self.image_manager.get_image(full_path, 'church.jpg', '120x120') + self.assertNotEquals(context.exception, '', + 'KeyError exception should have been thrown for missing dimension') + 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 e36c421b1..9f2668948 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -36,6 +36,7 @@ from tempfile import mkstemp from PyQt4 import QtGui +from openlp.core.lib import Registry from openlp.core.common import Settings from openlp.plugins.remotes.lib.httpserver import HttpRouter from mock import MagicMock, patch, mock_open @@ -183,6 +184,9 @@ class TestRouter(TestCase): self.router.send_header = MagicMock() self.router.end_headers = MagicMock() self.router.wfile = MagicMock() + mocked_image_manager = MagicMock() + Registry.create() + Registry().register('image_manager',mocked_image_manager) file_name = 'another%20test/slide1.png' full_path = os.path.normpath(os.path.join('thumbnails',file_name)) width = 120 @@ -191,8 +195,6 @@ class TestRouter(TestCase): patch('builtins.open', mock_open(read_data='123')), \ patch('openlp.plugins.remotes.lib.httprouter.AppLocation') \ as mocked_location, \ - patch('openlp.plugins.remotes.lib.httprouter.resize_image') \ - as mocked_resize, \ patch('openlp.plugins.remotes.lib.httprouter.image_to_byte')\ as mocked_image_to_byte: mocked_exists.return_value = True @@ -205,8 +207,11 @@ class TestRouter(TestCase): # THEN: a file should be returned self.assertEqual(self.router.send_header.call_count, 1, 'One header') - self.assertEqual(result, '123', 'The content should match \'123\'') mocked_exists.assert_called_with(urllib.parse.unquote(full_path)) self.assertEqual(mocked_image_to_byte.call_count, 1, 'Called once') - mocked_resize.assert_called_once_with( - urllib.parse.unquote(full_path), width, height) + mocked_image_manager.assert_called_any( + os.path.normpath('thumbnails\\another test'), 'slide1.png', + None, '120x90') + mocked_image_manager.assert_called_any( + os.path.normpath('thumbnails\\another test'),'slide1.png', + '120x90')