Fix sequential presentations - proper fix

openlp/plugins/presentations/lib/messagelistener.py

@@ -28,6 +28,7 @@ from openlp.core.common.registry import Registry
 from openlp.core.lib import ServiceItemContext
 from openlp.core.ui import HideMode
 from openlp.plugins.presentations.lib.pdfcontroller import PDF_CONTROLLER_FILETYPES
+from openlp.plugins.presentations.lib.presentationcontroller import PresentationList
 
 
 log = logging.getLogger(__name__)
@@ -49,21 +50,20 @@ class Controller(object):
         self.hide_mode = None
         log.info('{name} controller loaded'.format(name=live))
 
-    def add_handler(self, controller, file, hide_mode, slide_no):
+    def add_handler(self, controller, file, hide_mode, slide_no, unique_id):
         """
         Add a handler, which is an instance of a presentation and slidecontroller combination. If the slidecontroller
         has a display then load the presentation.
         """
         log.debug('Live = {live}, add_handler {handler}'.format(live=self.is_live, handler=file))
         self.controller = controller
-        if self.doc is not None:
-            self.shutdown()
         self.doc = self.controller.add_document(file)
         if not self.doc.load_presentation():
             # Display error message to user
             # Inform slidecontroller that the action failed?
             self.doc.slidenumber = 0
             return
+        PresentationList().add(self.doc, unique_id)
         self.doc.slidenumber = slide_no
         self.hide_mode = hide_mode
         log.debug('add_handler, slide_number: {slide:d}'.format(slide=slide_no))
@@ -205,15 +205,15 @@ class Controller(object):
         self.poll()
         return ret
 
-    def shutdown(self):
+    def shutdown(self, unique_id):
         """
         Based on the handler passed at startup triggers slide show to shut down.
         """
         log.debug('Live = {live}, shutdown'.format(live=self.is_live))
-        if not self.doc:
-            return
-        self.doc.close_presentation()
-        self.doc = None
+        presentation_to_close = PresentationList().get_presentation_by_id(unique_id)
+        if presentation_to_close:
+            presentation_to_close.close_presentation()
+            PresentationList().remove(unique_id)
 
     def blank(self, hide_mode):
         """
@@ -365,7 +365,8 @@ class MessageListener(object):
         if self.handler is None:
             self.controller = controller
         else:
-            controller.add_handler(self.controllers[self.handler], file_path, hide_mode, message[3])
+            controller.add_handler(self.controllers[self.handler], file_path, hide_mode, message[3],
+                                   message[0].unique_identifier)
             self.timer.start()
 
     def slide(self, message):
@@ -443,10 +444,10 @@ class MessageListener(object):
         """
         is_live = message[1]
         if is_live:
-            self.live_handler.shutdown()
+            self.live_handler.shutdown(message[0].unique_identifier)
             self.timer.stop()
         else:
-            self.preview_handler.shutdown()
+            self.preview_handler.shutdown(message[0].unique_identifier)
 
     def hide(self, message):
         """

openlp/plugins/presentations/lib/presentationcontroller.py

@@ -24,7 +24,7 @@ from pathlib import Path
 
 from PyQt5 import QtCore
 
-from openlp.core.common import md5_hash, sha256_file_hash
+from openlp.core.common import Singleton, md5_hash, sha256_file_hash
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.path import create_paths
 from openlp.core.common.registry import Registry
@@ -386,6 +386,37 @@ class PresentationDocument(object):
         return self._sha256_file_hash
 
 
+class PresentationList(metaclass=Singleton):
+    """
+    This is a singleton class which maintains a list of instances for presentations
+    which have been started.
+    The document load_presentation() method is called several times - for example, when the
+    presentation files are being loaded into the library - but a document is included in this
+    PresentationList only when the presentation is actually displayed.
+    In this case the loading is initiated by a Registry 'presentation_start' event, the message
+    includes the service item, and the unique_identifier from the service item is used as the id
+    to differentiate the presentation document instances within this PresentationList.
+    The purpose of this is so that the 'presentation_stop' event, which also includes the service
+    item and its unique identifier, can result in the correct presentation being stopped.
+    This fixes issue #700
+    """
+
+    def __init__(self):
+        self._presentations = {}
+
+    def add(self, document, unique_id):
+        self._presentations[unique_id] = document
+
+    def remove(self, unique_id):
+        del self._presentations[unique_id]
+
+    def get_presentation_by_id(self, unique_id):
+        if unique_id in self._presentations:
+            return self._presentations[unique_id]
+        else:
+            return None
+
+
 class PresentationController(object):
     """
     This class is used to control interactions with presentation applications by creating a runtime environment.

tests/openlp_plugins/presentations/test_messagelistener.py

@@ -122,7 +122,7 @@ def test_add_handler_failure():
     mocked_doc_controller.add_document.return_value = mocked_doc
 
     # WHEN: calling add_handler that fails
-    controller.add_handler(mocked_doc_controller, MagicMock(), True, 0)
+    controller.add_handler(mocked_doc_controller, MagicMock(), True, 0, "uuid")
 
     # THEN: slidenumber should be 0
     assert controller.doc.slidenumber == 0, 'doc.slidenumber should be 0'

tests/openlp_plugins/presentations/test_presentationcontroller.py

@@ -26,7 +26,8 @@ import pytest
 from pathlib import Path
 from unittest.mock import MagicMock, call, patch
 
-from openlp.plugins.presentations.lib.presentationcontroller import PresentationController, PresentationDocument
+from openlp.plugins.presentations.lib.presentationcontroller import PresentationController, PresentationDocument, \
+    PresentationList
 
 
 FOLDER_TO_PATCH = 'openlp.plugins.presentations.lib.presentationcontroller.PresentationDocument.get_thumbnail_folder'
@@ -215,3 +216,45 @@ def test_load_presentation(get_thumbnail_folder):
 
     # THEN: load_presentation should return false
     assert result is False, "PresentationDocument.load_presentation should return false."
+
+
+def test_presentation_list_is_singleton():
+    """
+    Test PresentationList is a singleton class
+    """
+    # GIVEN: a PresentationList
+    presentation_list = PresentationList()
+
+    # WHEN: I try to create another instance
+    presentation_list_2 = PresentationList()
+
+    # THEN: I get the same instance returned
+    assert presentation_list_2 is presentation_list
+
+
+def test_presentation_list_add_and_retrieve(document):
+    """
+    Test adding a presentation document and later retrieving it
+    """
+    # GIVEN: a fixture with a mocked document which is added to the PresentationList
+    PresentationList().add(document, "unique id")
+
+    # WHEN: I retrieve the presentation document
+    retrieved_presentation = PresentationList().get_presentation_by_id("unique id")
+
+    # THEN: I get the same instance returned
+    retrieved_presentation is document
+
+
+def test_presentation_list_remove(document):
+    """
+    Test removing a presentation document from the list
+    """
+    # GIVEN: a fixture with a mocked document which is added to the PresentationList
+    PresentationList().add(document, "unique id")
+
+    # WHEN: I remove the presentation document
+    PresentationList().remove("unique id")
+
+    # THEN: That document shouldn't be in the list
+    PresentationList().get_presentation_by_id("unique id") is None