Added caching for thumbnail images

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:
+        if not (path, source, dimensions) in self._cache:
-            image = Image(path, source, background)
+            image = Image(path, source, background, dimensions)
-            self._cache[(path, source)] = image
+            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)

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['image'], text_image.get('display_title',''),
-                        text_image['notes'])
+                        text_image.get('notes', ''))
                 else:
                     self.add_from_command(text_image['path'], 
                         text_image['title'], text_image['image'])

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,
+                        content = image_to_byte(
-                            height),False)
+                            image_manager.get_image(full_path,
+                                just_file_name, dimensions), False)
         if len(content)==0:
             content = self.do_not_found()
         return content

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')
+

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):

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(
+            mocked_image_manager.assert_called_any(
-                urllib.parse.unquote(full_path), width, height)
+                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')