From 524d8f3acbe36f8cc6ce9978427c88039c7251fa Mon Sep 17 00:00:00 2001 From: VirBinarus Date: Tue, 13 Jun 2017 16:49:48 +0100 Subject: [PATCH 01/18] fixed bug #1238385, but no tests Fixes: https://launchpad.net/bugs/1238385 --- openlp/core/ui/servicemanager.py | 37 ++++++++++++++++++ .../openlp_core_ui/test_servicemanager.py | 39 ++++++++++++++++++- 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index cf30245bf..8e6c6fd6b 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -66,6 +66,12 @@ class ServiceManagerList(QtWidgets.QTreeWidget): elif event.key() == QtCore.Qt.Key_Down: self.service_manager.on_move_selection_down() event.accept() + elif event.key() == QtCore.Qt.Key_Right: + self.service_manager.on_expand_selection() + event.accept() + elif event.key() == QtCore.Qt.Key_Left: + self.service_manager.on_collapse_selection() + event.accept() elif event.key() == QtCore.Qt.Key_Delete: self.service_manager.on_delete_from_service() event.accept() @@ -1119,6 +1125,37 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa return self.service_manager_list.setCurrentItem(item_after) + def on_expand_selection(self): + """ + Expands cursor selection on the window. Called by the right arrow + """ + item = self.service_manager_list.currentItem() + + if item.childCount(): # Since we only have 2 levels we find them by checking for children + if not self.service_manager_list.isExpanded(self.service_manager_list.currentIndex()): + self.service_manager_list.expandItem(item) + self.service_manager.expanded(item) + # If not expanded, Expand it + + self.service_manager_list.setCurrentItem(self.service_manager_list.itemBelow(item)) + # Then move selection down to child whether it needed to be expanded or not + + def on_collapse_selection(self): + """ + Collapses cursor selection on the window Called by the left arrow + """ + item = self.service_manager_list.currentItem() + + if item.childCount(): # Since we only have 2 levels we find them by checking for children + if self.service_manager_list.isExpanded(self.service_manager_list.currentIndex()): + self.service_manager_list.collapseItem(item) + self.service_manager.collapsed(item) + + else: # If selection is lower level + self.service_manager_list.collapseItem(item.parent()) + self.service_manager.collapsed(item.parent()) + self.service_manager_list.setCurrentItem(item.parent()) + def on_collapse_all(self, field=None): """ Collapse all the service items. diff --git a/tests/interfaces/openlp_core_ui/test_servicemanager.py b/tests/interfaces/openlp_core_ui/test_servicemanager.py index 10e928bc2..4e84e4035 100644 --- a/tests/interfaces/openlp_core_ui/test_servicemanager.py +++ b/tests/interfaces/openlp_core_ui/test_servicemanager.py @@ -28,9 +28,11 @@ from unittest.mock import MagicMock, patch from openlp.core.common import Registry from openlp.core.lib import ScreenList, ServiceItem, ItemCapabilities from openlp.core.ui.mainwindow import MainWindow +from openlp.core.ui.servicemanager import ServiceManagerList +from openlp.core.lib.serviceitem import ServiceItem from tests.helpers.testmixin import TestMixin - +from PyQt5 import QtCore, QtGui, QtTest class TestServiceManager(TestCase, TestMixin): @@ -351,3 +353,38 @@ class TestServiceManager(TestCase, TestMixin): new_service.trigger() assert mocked_event.call_count == 1, 'The on_new_service_clicked method should have been called once' + + def test_keyboard_expand_selection(self): + """ + Test on on_expand_selection method caused by keyboard + """ + # GIVEN A collapsed song selected on the service manager. + self.service_manager.setup_ui(self.service_manager) + ServiceManagerList(self.service_manager) + + item = ServiceItem() + item.title = "test" + item.add_from_text("slide 1") + item.add_from_text("slide 2") + item.add_icon(":/plugins/plugin_songs.png") + #SongMediaItem.generate_slide_data(item) + self.service_manager.add_service_item(item) + + print(item._raw_frames) + + song_to_expand = self.service_manager.service_manager_list.topLevelItem(0) + #print(song_to_expand) + self.service_manager.service_manager_list.setCurrentItem(song_to_expand) + #print(self.service_manager.service_manager_list.currentItem()) + #print(self.service_manager.service_manager_list.topLevelItemCount()) + + # WHEN Pressing the right arrow key + #QtTest.QTest.keyPress(self.service_manager.service_manager_list, QtCore.Qt.Key_Right) + + event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress,QtCore.Qt.Key_Right,QtCore.Qt.NoModifier) + ServiceManagerList.keyPressEvent(self.service_manager,event) + + # THEN Should be expanded + selected_index = self.service_manager.service_manager_list.currentIndex() + above_selection_index = self.service_manager.service_manager_list.indexAbove(selected_index) + self.assertTrue(self.service_manager.service_manager_list.isExpanded(above_selection_index), 'Item should have been expanded') \ No newline at end of file From 9aef1cdf5dbd96d9624da76d375e77b300f41589 Mon Sep 17 00:00:00 2001 From: VirBinarus Date: Thu, 15 Jun 2017 19:05:38 +0100 Subject: [PATCH 02/18] made tests --- openlp/core/ui/servicemanager.py | 6 +- .../openlp_core_ui/test_servicemanager.py | 164 +++++++++++++++--- 2 files changed, 141 insertions(+), 29 deletions(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 8e6c6fd6b..777fc51b0 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -1131,7 +1131,8 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa """ item = self.service_manager_list.currentItem() - if item.childCount(): # Since we only have 2 levels we find them by checking for children + # Since we only have 2 levels we find them by checking for children + if item.childCount(): if not self.service_manager_list.isExpanded(self.service_manager_list.currentIndex()): self.service_manager_list.expandItem(item) self.service_manager.expanded(item) @@ -1146,7 +1147,8 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa """ item = self.service_manager_list.currentItem() - if item.childCount(): # Since we only have 2 levels we find them by checking for children + # Since we only have 2 levels we find them by checking for children + if item.childCount(): if self.service_manager_list.isExpanded(self.service_manager_list.currentIndex()): self.service_manager_list.collapseItem(item) self.service_manager.collapsed(item) diff --git a/tests/interfaces/openlp_core_ui/test_servicemanager.py b/tests/interfaces/openlp_core_ui/test_servicemanager.py index 4e84e4035..b93f75a88 100644 --- a/tests/interfaces/openlp_core_ui/test_servicemanager.py +++ b/tests/interfaces/openlp_core_ui/test_servicemanager.py @@ -32,7 +32,8 @@ from openlp.core.ui.servicemanager import ServiceManagerList from openlp.core.lib.serviceitem import ServiceItem from tests.helpers.testmixin import TestMixin -from PyQt5 import QtCore, QtGui, QtTest + +from PyQt5 import QtCore, QtGui, QtWidgets class TestServiceManager(TestCase, TestMixin): @@ -354,37 +355,146 @@ class TestServiceManager(TestCase, TestMixin): assert mocked_event.call_count == 1, 'The on_new_service_clicked method should have been called once' - def test_keyboard_expand_selection(self): - """ - Test on on_expand_selection method caused by keyboard - """ - # GIVEN A collapsed song selected on the service manager. + def test_expand_selection_on_right_arrow(self): + #GIVEN a mocked expand function + self.service_manager.on_expand_selection = MagicMock() + + #WHEN the right arrow key event is called self.service_manager.setup_ui(self.service_manager) - ServiceManagerList(self.service_manager) + event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Right, QtCore.Qt.NoModifier) + self.service_manager.service_manager_list.keyPressEvent(event) - item = ServiceItem() - item.title = "test" - item.add_from_text("slide 1") - item.add_from_text("slide 2") - item.add_icon(":/plugins/plugin_songs.png") - #SongMediaItem.generate_slide_data(item) - self.service_manager.add_service_item(item) + #THEN the on_expand_selection function should have been called. + self.service_manager.on_expand_selection.assert_called_once_with() - print(item._raw_frames) + def test_collapse_selection_on_left_arrow(self): + #GIVEN a mocked collapse function + self.service_manager.on_collapse_selection = MagicMock() - song_to_expand = self.service_manager.service_manager_list.topLevelItem(0) - #print(song_to_expand) - self.service_manager.service_manager_list.setCurrentItem(song_to_expand) - #print(self.service_manager.service_manager_list.currentItem()) - #print(self.service_manager.service_manager_list.topLevelItemCount()) + #WHEN the left arrow key event is called + self.service_manager.setup_ui(self.service_manager) + event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Left, QtCore.Qt.NoModifier) + self.service_manager.service_manager_list.keyPressEvent(event) - # WHEN Pressing the right arrow key - #QtTest.QTest.keyPress(self.service_manager.service_manager_list, QtCore.Qt.Key_Right) + #THEN the on_collapse_selection function should have been called. + self.service_manager.on_collapse_selection.assert_called_once_with() - event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress,QtCore.Qt.Key_Right,QtCore.Qt.NoModifier) - ServiceManagerList.keyPressEvent(self.service_manager,event) + def test_move_selection_down_on_down_arrow(self): + #GIVEN a mocked move down function + self.service_manager.on_move_selection_down = MagicMock() + + #WHEN the down arrow key event is called + self.service_manager.setup_ui(self.service_manager) + event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Down, QtCore.Qt.NoModifier) + self.service_manager.service_manager_list.keyPressEvent(event) + + #THEN the on_move_selection_down function should have been called. + self.service_manager.on_move_selection_down.assert_called_once_with() + + def test_move_selection_up_on_up_arrow(self): + #GIVEN a mocked move up function + self.service_manager.on_move_selection_up = MagicMock() + + #WHEN the up arrow key event is called + self.service_manager.setup_ui(self.service_manager) + event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Up, QtCore.Qt.NoModifier) + self.service_manager.service_manager_list.keyPressEvent(event) + + #THEN the on_move_selection_up function should have been called. + self.service_manager.on_move_selection_up.assert_called_once_with() + + def test_on_expand_selection(self): + + #GIVEN a mocked servicemanager list + self.service_manager.expanded = MagicMock() + + verse_1 = QtWidgets.QTreeWidgetItem(0) + verse_2 = QtWidgets.QTreeWidgetItem(0) + song_item = QtWidgets.QTreeWidgetItem(0) + song_item.addChild(verse_1) + song_item.addChild(verse_2) + + self.service_manager.setup_ui(self.service_manager) + self.service_manager.service_manager_list.addTopLevelItem(song_item) + + self.service_manager.service_manager_list.setCurrentItem(song_item) + + #Reset expanded function in case it has been called and/or changed in initialisation of the service manager. + self.service_manager.expanded = MagicMock() + + #WHEN on_expand_selection is called + self.service_manager.on_expand_selection() + + #THEN selection should be expanded - # THEN Should be expanded selected_index = self.service_manager.service_manager_list.currentIndex() - above_selection_index = self.service_manager.service_manager_list.indexAbove(selected_index) - self.assertTrue(self.service_manager.service_manager_list.isExpanded(above_selection_index), 'Item should have been expanded') \ No newline at end of file + above_selected_index = self.service_manager.service_manager_list.indexAbove(selected_index) + self.assertTrue(self.service_manager.service_manager_list.isExpanded(above_selected_index), + 'Item should have been expanded') + self.service_manager.expanded.assert_called_once() + + def test_on_collapse_selection_with_parent_selected(self): + # GIVEN a mocked servicemanager list + self.service_manager.expanded = MagicMock() + self.service_manager.collapsed = MagicMock() + + verse_1 = QtWidgets.QTreeWidgetItem(0) + verse_2 = QtWidgets.QTreeWidgetItem(0) + song_item = QtWidgets.QTreeWidgetItem(0) + song_item.addChild(verse_1) + song_item.addChild(verse_2) + + self.service_manager.setup_ui(self.service_manager) + self.service_manager.service_manager_list.addTopLevelItem(song_item) + + self.service_manager.service_manager_list.setCurrentItem(song_item) + self.service_manager.service_manager_list.expandItem(song_item) + + # Reset collapsed function in case it has been called and/or changed in initialisation of the service manager. + self.service_manager.collapsed = MagicMock() + + # WHEN on_expand_selection is called + self.service_manager.on_collapse_selection() + + # THEN selection should be expanded + + selected_index = self.service_manager.service_manager_list.currentIndex() + self.assertFalse(self.service_manager.service_manager_list.isExpanded(selected_index), + 'Item should have been collapsed') + self.assertTrue(self.service_manager.service_manager_list.currentItem() == song_item, + 'Top item should have been selected') + self.service_manager.collapsed.assert_called_once() + + def test_on_collapse_selection_with_child_selected(self): + # GIVEN a mocked servicemanager list + self.service_manager.expanded = MagicMock() + self.service_manager.collapsed = MagicMock() + + verse_1 = QtWidgets.QTreeWidgetItem(0) + verse_2 = QtWidgets.QTreeWidgetItem(0) + song_item = QtWidgets.QTreeWidgetItem(0) + song_item.addChild(verse_1) + song_item.addChild(verse_2) + + self.service_manager.setup_ui(self.service_manager) + self.service_manager.service_manager_list.addTopLevelItem(song_item) + + self.service_manager.service_manager_list.setCurrentItem(verse_2) + self.service_manager.service_manager_list.expandItem(song_item) + + # Reset collapsed function in case it has been called and/or changed in initialisation of the service manager. + self.service_manager.collapsed = MagicMock() + + # WHEN on_expand_selection is called + self.service_manager.on_collapse_selection() + + # THEN selection should be expanded + + selected_index = self.service_manager.service_manager_list.currentIndex() + self.assertFalse(self.service_manager.service_manager_list.isExpanded(selected_index), + 'Item should have been collapsed') + self.assertTrue(self.service_manager.service_manager_list.currentItem() == song_item, + 'Top item should have been selected') + self.service_manager.collapsed.assert_called_once() + + \ No newline at end of file From 26275566eafec065541c9febf4611b7b1fd356f5 Mon Sep 17 00:00:00 2001 From: VirBinarus Date: Thu, 15 Jun 2017 19:10:09 +0100 Subject: [PATCH 03/18] bzr thinks I've modified it so this is to make sure --- tests/interfaces/openlp_core_ui/test_servicemanager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/interfaces/openlp_core_ui/test_servicemanager.py b/tests/interfaces/openlp_core_ui/test_servicemanager.py index b93f75a88..2c30e251a 100644 --- a/tests/interfaces/openlp_core_ui/test_servicemanager.py +++ b/tests/interfaces/openlp_core_ui/test_servicemanager.py @@ -497,4 +497,3 @@ class TestServiceManager(TestCase, TestMixin): 'Top item should have been selected') self.service_manager.collapsed.assert_called_once() - \ No newline at end of file From 462bc6e051cc3a89f6eb4694c3ff97553e21e2db Mon Sep 17 00:00:00 2001 From: VirBinarus Date: Thu, 22 Jun 2017 21:24:43 +0100 Subject: [PATCH 04/18] fixed tests --- tests/interfaces/openlp_core_ui/test_servicemanager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/interfaces/openlp_core_ui/test_servicemanager.py b/tests/interfaces/openlp_core_ui/test_servicemanager.py index 2c30e251a..6c2f3a83a 100644 --- a/tests/interfaces/openlp_core_ui/test_servicemanager.py +++ b/tests/interfaces/openlp_core_ui/test_servicemanager.py @@ -431,7 +431,7 @@ class TestServiceManager(TestCase, TestMixin): above_selected_index = self.service_manager.service_manager_list.indexAbove(selected_index) self.assertTrue(self.service_manager.service_manager_list.isExpanded(above_selected_index), 'Item should have been expanded') - self.service_manager.expanded.assert_called_once() + self.service_manager.expanded.assert_called_once_with(song_item) def test_on_collapse_selection_with_parent_selected(self): # GIVEN a mocked servicemanager list @@ -463,7 +463,7 @@ class TestServiceManager(TestCase, TestMixin): 'Item should have been collapsed') self.assertTrue(self.service_manager.service_manager_list.currentItem() == song_item, 'Top item should have been selected') - self.service_manager.collapsed.assert_called_once() + self.service_manager.collapsed.assert_called_once_with(song_item) def test_on_collapse_selection_with_child_selected(self): # GIVEN a mocked servicemanager list @@ -495,5 +495,5 @@ class TestServiceManager(TestCase, TestMixin): 'Item should have been collapsed') self.assertTrue(self.service_manager.service_manager_list.currentItem() == song_item, 'Top item should have been selected') - self.service_manager.collapsed.assert_called_once() + self.service_manager.collapsed.assert_called_once_with(song_item) From 36b965325f4d5fe9c8a3440b865e0bfc8769effc Mon Sep 17 00:00:00 2001 From: VirBinarus Date: Thu, 22 Jun 2017 21:42:46 +0100 Subject: [PATCH 05/18] formatted test_servicemanager.py to make jenkins happy --- .../openlp_core_ui/test_servicemanager.py | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/interfaces/openlp_core_ui/test_servicemanager.py b/tests/interfaces/openlp_core_ui/test_servicemanager.py index 6c2f3a83a..6ae6b2a24 100644 --- a/tests/interfaces/openlp_core_ui/test_servicemanager.py +++ b/tests/interfaces/openlp_core_ui/test_servicemanager.py @@ -35,6 +35,7 @@ from tests.helpers.testmixin import TestMixin from PyQt5 import QtCore, QtGui, QtWidgets + class TestServiceManager(TestCase, TestMixin): def setUp(self): @@ -356,6 +357,9 @@ class TestServiceManager(TestCase, TestMixin): assert mocked_event.call_count == 1, 'The on_new_service_clicked method should have been called once' def test_expand_selection_on_right_arrow(self): + """ + Test that a right arrow key press event calls the on_expand_selection function + """ #GIVEN a mocked expand function self.service_manager.on_expand_selection = MagicMock() @@ -368,6 +372,9 @@ class TestServiceManager(TestCase, TestMixin): self.service_manager.on_expand_selection.assert_called_once_with() def test_collapse_selection_on_left_arrow(self): + """ + Test that a left arrow key press event calls the on_collapse_selection function + """ #GIVEN a mocked collapse function self.service_manager.on_collapse_selection = MagicMock() @@ -380,6 +387,9 @@ class TestServiceManager(TestCase, TestMixin): self.service_manager.on_collapse_selection.assert_called_once_with() def test_move_selection_down_on_down_arrow(self): + """ + Test that a down arrow key press event calls the on_move_selection_down function + """ #GIVEN a mocked move down function self.service_manager.on_move_selection_down = MagicMock() @@ -392,6 +402,9 @@ class TestServiceManager(TestCase, TestMixin): self.service_manager.on_move_selection_down.assert_called_once_with() def test_move_selection_up_on_up_arrow(self): + """ + Test that an up arrow key press event calls the on_move_selection_up function + """ #GIVEN a mocked move up function self.service_manager.on_move_selection_up = MagicMock() @@ -404,7 +417,9 @@ class TestServiceManager(TestCase, TestMixin): self.service_manager.on_move_selection_up.assert_called_once_with() def test_on_expand_selection(self): - + """ + Test that the on_expand_selection function successfully expands an item and moves to its first child + """ #GIVEN a mocked servicemanager list self.service_manager.expanded = MagicMock() @@ -434,6 +449,10 @@ class TestServiceManager(TestCase, TestMixin): self.service_manager.expanded.assert_called_once_with(song_item) def test_on_collapse_selection_with_parent_selected(self): + """ + Test that the on_collapse_selection function successfully collapses an item + """ + # GIVEN a mocked servicemanager list self.service_manager.expanded = MagicMock() self.service_manager.collapsed = MagicMock() @@ -466,6 +485,10 @@ class TestServiceManager(TestCase, TestMixin): self.service_manager.collapsed.assert_called_once_with(song_item) def test_on_collapse_selection_with_child_selected(self): + """ + Test that the on_collapse_selection function successfully collapses child's parent item + and moves selection to its parent. + """ # GIVEN a mocked servicemanager list self.service_manager.expanded = MagicMock() self.service_manager.collapsed = MagicMock() From 82b7a568a514dad1e4c87ab7afbd104bc1e2b1b9 Mon Sep 17 00:00:00 2001 From: VirBinarus Date: Thu, 22 Jun 2017 21:49:14 +0100 Subject: [PATCH 06/18] formatted test_servicemanager.py more to make jenkins more happy --- .../openlp_core_ui/test_servicemanager.py | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/tests/interfaces/openlp_core_ui/test_servicemanager.py b/tests/interfaces/openlp_core_ui/test_servicemanager.py index 6ae6b2a24..fb9551444 100644 --- a/tests/interfaces/openlp_core_ui/test_servicemanager.py +++ b/tests/interfaces/openlp_core_ui/test_servicemanager.py @@ -360,67 +360,67 @@ class TestServiceManager(TestCase, TestMixin): """ Test that a right arrow key press event calls the on_expand_selection function """ - #GIVEN a mocked expand function + # GIVEN a mocked expand function self.service_manager.on_expand_selection = MagicMock() - #WHEN the right arrow key event is called + # WHEN the right arrow key event is called self.service_manager.setup_ui(self.service_manager) event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Right, QtCore.Qt.NoModifier) self.service_manager.service_manager_list.keyPressEvent(event) - #THEN the on_expand_selection function should have been called. + # THEN the on_expand_selection function should have been called. self.service_manager.on_expand_selection.assert_called_once_with() def test_collapse_selection_on_left_arrow(self): """ Test that a left arrow key press event calls the on_collapse_selection function """ - #GIVEN a mocked collapse function + # GIVEN a mocked collapse function self.service_manager.on_collapse_selection = MagicMock() - #WHEN the left arrow key event is called + # WHEN the left arrow key event is called self.service_manager.setup_ui(self.service_manager) event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Left, QtCore.Qt.NoModifier) self.service_manager.service_manager_list.keyPressEvent(event) - #THEN the on_collapse_selection function should have been called. + # THEN the on_collapse_selection function should have been called. self.service_manager.on_collapse_selection.assert_called_once_with() def test_move_selection_down_on_down_arrow(self): """ Test that a down arrow key press event calls the on_move_selection_down function """ - #GIVEN a mocked move down function + # GIVEN a mocked move down function self.service_manager.on_move_selection_down = MagicMock() - #WHEN the down arrow key event is called + # WHEN the down arrow key event is called self.service_manager.setup_ui(self.service_manager) event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Down, QtCore.Qt.NoModifier) self.service_manager.service_manager_list.keyPressEvent(event) - #THEN the on_move_selection_down function should have been called. + # THEN the on_move_selection_down function should have been called. self.service_manager.on_move_selection_down.assert_called_once_with() def test_move_selection_up_on_up_arrow(self): """ Test that an up arrow key press event calls the on_move_selection_up function """ - #GIVEN a mocked move up function + # GIVEN a mocked move up function self.service_manager.on_move_selection_up = MagicMock() - #WHEN the up arrow key event is called + # WHEN the up arrow key event is called self.service_manager.setup_ui(self.service_manager) event = QtGui.QKeyEvent(QtCore.QEvent.KeyPress, QtCore.Qt.Key_Up, QtCore.Qt.NoModifier) self.service_manager.service_manager_list.keyPressEvent(event) - #THEN the on_move_selection_up function should have been called. + # THEN the on_move_selection_up function should have been called. self.service_manager.on_move_selection_up.assert_called_once_with() def test_on_expand_selection(self): """ Test that the on_expand_selection function successfully expands an item and moves to its first child """ - #GIVEN a mocked servicemanager list + # GIVEN a mocked servicemanager list self.service_manager.expanded = MagicMock() verse_1 = QtWidgets.QTreeWidgetItem(0) @@ -434,13 +434,13 @@ class TestServiceManager(TestCase, TestMixin): self.service_manager.service_manager_list.setCurrentItem(song_item) - #Reset expanded function in case it has been called and/or changed in initialisation of the service manager. + # Reset expanded function in case it has been called and/or changed in initialisation of the service manager. self.service_manager.expanded = MagicMock() - #WHEN on_expand_selection is called + # WHEN on_expand_selection is called self.service_manager.on_expand_selection() - #THEN selection should be expanded + # THEN selection should be expanded selected_index = self.service_manager.service_manager_list.currentIndex() above_selected_index = self.service_manager.service_manager_list.indexAbove(selected_index) @@ -518,5 +518,4 @@ class TestServiceManager(TestCase, TestMixin): 'Item should have been collapsed') self.assertTrue(self.service_manager.service_manager_list.currentItem() == song_item, 'Top item should have been selected') - self.service_manager.collapsed.assert_called_once_with(song_item) - + self.service_manager.collapsed.assert_called_once_with(song_item) \ No newline at end of file From 4d6a3ac5d5a11d71ebd1a5026082be6def5d7a34 Mon Sep 17 00:00:00 2001 From: VirBinarus Date: Thu, 22 Jun 2017 21:57:36 +0100 Subject: [PATCH 07/18] formatted test_servicemanager.py even more to make jenkins even more happy --- tests/interfaces/openlp_core_ui/test_servicemanager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/interfaces/openlp_core_ui/test_servicemanager.py b/tests/interfaces/openlp_core_ui/test_servicemanager.py index fb9551444..e9202020f 100644 --- a/tests/interfaces/openlp_core_ui/test_servicemanager.py +++ b/tests/interfaces/openlp_core_ui/test_servicemanager.py @@ -518,4 +518,4 @@ class TestServiceManager(TestCase, TestMixin): 'Item should have been collapsed') self.assertTrue(self.service_manager.service_manager_list.currentItem() == song_item, 'Top item should have been selected') - self.service_manager.collapsed.assert_called_once_with(song_item) \ No newline at end of file + self.service_manager.collapsed.assert_called_once_with(song_item) From 0e2019faf296e4856f87af60c873377da18594ad Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Sat, 24 Jun 2017 19:21:07 -0700 Subject: [PATCH 08/18] PJLink update F --- openlp/core/lib/projector/db.py | 2 +- openlp/core/lib/projector/pjlink1.py | 118 ++++++++++++++++-- openlp/core/lib/projector/upgrade.py | 10 +- setup.cfg | 10 ++ .../test_projector_constants.py | 2 +- ...st_projectordb.py => test_projector_db.py} | 8 +- .../openlp_core_lib/test_projector_pjlink1.py | 116 ++++++++++++++++- 7 files changed, 241 insertions(+), 25 deletions(-) rename tests/functional/openlp_core_lib/{test_projectordb.py => test_projector_db.py} (99%) diff --git a/openlp/core/lib/projector/db.py b/openlp/core/lib/projector/db.py index 89b807f21..a0c7c009e 100644 --- a/openlp/core/lib/projector/db.py +++ b/openlp/core/lib/projector/db.py @@ -303,7 +303,7 @@ class ProjectorDB(Manager): :param ip: Host IP/Name :returns: Projector() instance """ - log.debug('get_projector_by_ip(ip="%s")' % ip) + log.debug('get_projector_by_ip(ip="{ip}")'.format(ip=ip)) projector = self.get_object_filtered(Projector, Projector.ip == ip) if projector is None: # Not found diff --git a/openlp/core/lib/projector/pjlink1.py b/openlp/core/lib/projector/pjlink1.py index eb2753cbf..d71ce7c99 100644 --- a/openlp/core/lib/projector/pjlink1.py +++ b/openlp/core/lib/projector/pjlink1.py @@ -44,6 +44,8 @@ log.debug('pjlink1 loaded') __all__ = ['PJLink'] +import re + from codecs import decode from PyQt5 import QtCore, QtNetwork @@ -53,9 +55,12 @@ from openlp.core.lib.projector.constants import CONNECTION_ERRORS, CR, ERROR_MSG E_AUTHENTICATION, E_CONNECTION_REFUSED, E_GENERAL, E_INVALID_DATA, E_NETWORK, E_NOT_CONNECTED, \ E_PARAMETER, E_PROJECTOR, E_SOCKET_TIMEOUT, E_UNAVAILABLE, E_UNDEFINED, PJLINK_ERRORS, \ PJLINK_ERST_STATUS, PJLINK_MAX_PACKET, PJLINK_PORT, PJLINK_POWR_STATUS, PJLINK_VALID_CMD, \ - PJLINK_DEFAULT_CODES, STATUS_STRING, S_CONNECTED, S_CONNECTING, S_NETWORK_RECEIVED, S_NETWORK_SENDING, \ + STATUS_STRING, S_CONNECTED, S_CONNECTING, S_NETWORK_RECEIVED, S_NETWORK_SENDING, \ S_NOT_CONNECTED, S_OFF, S_OK, S_ON, S_STATUS +# Possible future imports +# from openlp.core.lib.projector.constants import PJLINK_DEFAULT_CODES + # Shortcuts SocketError = QtNetwork.QAbstractSocket.SocketError SocketSTate = QtNetwork.QAbstractSocket.SocketState @@ -113,8 +118,13 @@ class PJLink(QtNetwork.QTcpSocket): self.port = port self.pin = pin super().__init__() + self.model_lamp = None + self.model_filter = None self.mac_adx = kwargs.get('mac_adx') + self.serial_no = None + self.serial_no_received = None # Used only if saved serial number is different than received serial number self.dbid = None + self.db_update = False # Use to check if db needs to be updated prior to exiting self.location = None self.notes = None self.dbid = kwargs.get('dbid') @@ -158,7 +168,9 @@ class PJLink(QtNetwork.QTcpSocket): 'LAMP': self.process_lamp, 'NAME': self.process_name, 'PJLINK': self.check_login, - 'POWR': self.process_powr + 'POWR': self.process_powr, + 'SNUM': self.process_snum, + 'SVER': self.process_sver } def reset_information(self): @@ -166,12 +178,16 @@ class PJLink(QtNetwork.QTcpSocket): Reset projector-specific information to default """ log.debug('({ip}) reset_information() connect status is {state}'.format(ip=self.ip, state=self.state())) + self.send_queue = [] self.power = S_OFF self.pjlink_name = None self.manufacturer = None self.model = None self.serial_no = None + self.serial_no_received = None self.sw_version = None + self.sw_version_received = None + self.mac_adx = None self.shutter = None self.mute = None self.lamp = None @@ -188,7 +204,6 @@ class PJLink(QtNetwork.QTcpSocket): if hasattr(self, 'socket_timer'): log.debug('({ip}): Calling socket_timer.stop()'.format(ip=self.ip)) self.socket_timer.stop() - self.send_queue = [] self.send_busy = False def thread_started(self): @@ -249,7 +264,10 @@ class PJLink(QtNetwork.QTcpSocket): # Restart timer self.timer.start() # These commands may change during connetion - for command in ['POWR', 'ERST', 'LAMP', 'AVMT', 'INPT']: + check_list = ['POWR', 'ERST', 'LAMP', 'AVMT', 'INPT'] + if self.pjlink_class == '2': + check_list.extend(['FILT', 'FREZ']) + for command in check_list: self.send_command(command, queue=True) # The following commands do not change, so only check them once if self.power == S_ON and self.source_available is None: @@ -262,6 +280,38 @@ class PJLink(QtNetwork.QTcpSocket): self.send_command('INF2', queue=True) if self.pjlink_name is None: self.send_command('NAME', queue=True) + if self.pjlink_class == '2': + # Class 2 specific checks + if self.serial_no is None: + self.send_command('SNUM', queue=True) + if self.sw_version is None: + self.send_command('SVER', queue=True) + if self.model_filter is None: + self.send_command('RFIL', queue=True) + if self.model_lamp is None: + self.send_command('RLMP', queue=True) + + def process_rfil(self, data): + """ + Process replacement filter type + """ + if self.model_filter is None: + self.model_filter = data + else: + log.warn("({ip}) Filter model already set".format(ip=self.ip)) + log.warn("({ip}) Saved model: '{old}'".format(ip=self.ip, old=self.model_filter)) + log.warn("({ip}) New model: '{new}'".format(ip=self.ip, new=data)) + + def process_rlmp(self, data): + """ + Process replacement lamp type + """ + if self.model_lamp is None: + self.model_lamp = data + else: + log.warn("({ip}) Lamp model already set".format(ip=self.ip)) + log.warn("({ip}) Saved lamp: '{old}'".format(ip=self.ip, old=self.model_lamp)) + log.warn("({ip}) New lamp: '{new}'".format(ip=self.ip, new=data)) def _get_status(self, status): """ @@ -341,7 +391,7 @@ class PJLink(QtNetwork.QTcpSocket): data = decode(read, 'utf-8') # Possibility of extraneous data on input when reading. # Clean out extraneous characters in buffer. - dontcare = self.readLine(self.max_size) + dontcare = self.readLine(self.max_size) # noqa: F841 log.debug('({ip}) check_login() read "{data}"'.format(ip=self.ip, data=data.strip())) # At this point, we should only have the initial login prompt with # possible authentication @@ -440,7 +490,8 @@ class PJLink(QtNetwork.QTcpSocket): return data_split = data.split('=') try: - (prefix, version, cmd, data) = (data_split[0][0], data_split[0][1], data_split[0][2:], data_split[1]) + (prefix, version, cmd, data) = (data_split[0][0], data_split[0][1], # noqa: F841 + data_split[0][2:], data_split[1]) except ValueError as e: log.warning('({ip}) get_data(): Invalid packet - expected header + command + data'.format(ip=self.ip)) log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in.strip())) @@ -729,11 +780,22 @@ class PJLink(QtNetwork.QTcpSocket): :param data: Class that projector supports. """ # bug 1550891: Projector returns non-standard class response: - # : Expected: %1CLSS=1 - # : Received: %1CLSS=Class 1 + # : Expected: '%1CLSS=1' + # : Received: '%1CLSS=Class 1' (Optoma) + # : Received: '%1CLSS=Version1' (BenQ) if len(data) > 1: - # Split non-standard information from response - clss = data.split()[-1] + log.warn("({ip}) Non-standard CLSS reply: '{data}'".format(ip=self.ip, data=data)) + # Due to stupid projectors not following standards (Optoma, BenQ comes to mind), + # AND the different responses that can be received, the semi-permanent way to + # fix the class reply is to just remove all non-digit characters. + try: + clss = re.findall('\d', data)[0] # Should only be the first match + except IndexError: + log.error("({ip}) No numbers found in class version reply - defaulting to class '1'".format(ip=self.ip)) + clss = '1' + elif not data.isdigit(): + log.error("({ip}) NAN class version reply - defaulting to class '1'".format(ip=self.ip)) + clss = '1' else: clss = data self.pjlink_class = clss @@ -845,6 +907,42 @@ class PJLink(QtNetwork.QTcpSocket): PJLINK_ERST_STATUS[data[5]] return + def process_snum(self, data): + """ + Serial number of projector. + + :param data: Serial number from projector. + """ + if self.serial_no is None: + log.debug("({ip}) Setting projector serial number to '{data}'".format(ip=self.ip, data=data)) + self.serial_no = data + self.db_update = False + else: + # Compare serial numbers and see if we got the same projector + if self.serial_no != data: + log.warn("({ip}) Projector serial number does not match saved serial number".format(ip=self.ip)) + log.warn("({ip}) Saved: '{old}'".format(ip=self.ip, old=self.serial_no)) + log.warn("({ip}) Received: '{new}'".format(ip=self.ip, new=data)) + log.warn("({ip}) NOT saving serial number".format(ip=self.ip)) + self.serial_no_received = data + + def process_sver(self, data): + """ + Software version of projector + """ + if self.sw_version is None: + log.debug("({ip}) Setting projector software version to '{data}'".format(ip=self.ip, data=data)) + self.sw_version = data + self.db_update = True + else: + # Compare software version and see if we got the same projector + if self.serial_no != data: + log.warn("({ip}) Projector software version does not match saved software version".format(ip=self.ip)) + log.warn("({ip}) Saved: '{old}'".format(ip=self.ip, old=self.sw_version)) + log.warn("({ip}) Received: '{new}'".format(ip=self.ip, new=data)) + log.warn("({ip}) NOT saving serial number".format(ip=self.ip)) + self.sw_version_received = data + def connect_to_host(self): """ Initiate connection to projector. diff --git a/openlp/core/lib/projector/upgrade.py b/openlp/core/lib/projector/upgrade.py index 913d54d2d..178ab2be9 100644 --- a/openlp/core/lib/projector/upgrade.py +++ b/openlp/core/lib/projector/upgrade.py @@ -25,16 +25,18 @@ backend for the projector setup. """ import logging -# Not all imports used at this time, but keep for future upgrades -from sqlalchemy import Table, Column, types, inspect -from sqlalchemy.exc import NoSuchTableError +from sqlalchemy import Table, Column, types from sqlalchemy.sql.expression import null -from openlp.core.common.db import drop_columns from openlp.core.lib.db import get_upgrade_op log = logging.getLogger(__name__) +# Possible future imports +# from sqlalchemy.exc import NoSuchTableError +# from sqlalchemy import inspect +# from openlp.core.common.db import drop_columns + # Initial projector DB was unversioned __version__ = 2 diff --git a/setup.cfg b/setup.cfg index 0ecc03ae8..e7e2651c0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,4 +1,14 @@ +# E402 module level import not at top of file +# E722 do not use bare except, specify exception instead +# F841 local variable '' is assigned to but never used + [pep8] exclude=resources.py,vlc.py max-line-length = 120 ignore = E402,E722 + +[flake8] +exclude=resources.py,vlc.py +max-line-length = 120 +ignore = E402,E722 + diff --git a/tests/functional/openlp_core_lib/test_projector_constants.py b/tests/functional/openlp_core_lib/test_projector_constants.py index 019c18888..61a93eb10 100644 --- a/tests/functional/openlp_core_lib/test_projector_constants.py +++ b/tests/functional/openlp_core_lib/test_projector_constants.py @@ -22,7 +22,7 @@ """ Package to test the openlp.core.lib.projector.constants package. """ -from unittest import TestCase, skip +from unittest import TestCase, skip # noqa: F401 class TestProjectorConstants(TestCase): diff --git a/tests/functional/openlp_core_lib/test_projectordb.py b/tests/functional/openlp_core_lib/test_projector_db.py similarity index 99% rename from tests/functional/openlp_core_lib/test_projectordb.py rename to tests/functional/openlp_core_lib/test_projector_db.py index d4ff4e75c..fd2852acf 100644 --- a/tests/functional/openlp_core_lib/test_projectordb.py +++ b/tests/functional/openlp_core_lib/test_projector_db.py @@ -29,8 +29,8 @@ import os import shutil from tempfile import mkdtemp -from unittest import TestCase, skip -from unittest.mock import MagicMock, patch +from unittest import TestCase, skip # noqa: F401 +from unittest.mock import MagicMock, patch # noqa: F401 from openlp.core.lib.projector import upgrade from openlp.core.lib.db import upgrade_db @@ -413,7 +413,7 @@ class TestProjectorDB(TestCase): Test add_projector() fail """ # GIVEN: Test entry in the database - ignore_result = self.projector.add_projector(Projector(**TEST1_DATA)) + ignore_result = self.projector.add_projector(Projector(**TEST1_DATA)) # noqa: F841 # WHEN: Attempt to add same projector entry results = self.projector.add_projector(Projector(**TEST1_DATA)) @@ -439,7 +439,7 @@ class TestProjectorDB(TestCase): Test update_projector() when entry not in database """ # GIVEN: Projector entry in database - ignore_result = self.projector.add_projector(Projector(**TEST1_DATA)) + ignore_result = self.projector.add_projector(Projector(**TEST1_DATA)) # noqa: F841 projector = Projector(**TEST2_DATA) # WHEN: Attempt to update data with a different ID diff --git a/tests/functional/openlp_core_lib/test_projector_pjlink1.py b/tests/functional/openlp_core_lib/test_projector_pjlink1.py index e5fb7566f..3ff47d85a 100644 --- a/tests/functional/openlp_core_lib/test_projector_pjlink1.py +++ b/tests/functional/openlp_core_lib/test_projector_pjlink1.py @@ -59,9 +59,101 @@ class TestPJLink(TestCase): self.assertTrue(mock_qmd5_hash.called_with(TEST_PIN, "Connection request should have been called with TEST_PIN")) - def test_projector_class(self): + def test_projector_process_rfil_save(self): """ - Test class version from projector + Test saving filter type + """ + # GIVEN: Test object + pjlink = pjlink_test + pjlink.model_filter = None + filter_model = 'Filter Type Test' + + # WHEN: Filter model is received + pjlink.process_rfil(data=filter_model) + + # THEN: Filter model number should be saved + self.assertEquals(pjlink.model_filter, filter_model, 'Filter type should have been saved') + + def test_projector_process_rfil_nosave(self): + """ + Test saving filter type previously saved + """ + # GIVEN: Test object + pjlink = pjlink_test + pjlink.model_filter = 'Old filter type' + filter_model = 'Filter Type Test' + + # WHEN: Filter model is received + pjlink.process_rfil(data=filter_model) + + # THEN: Filter model number should be saved + self.assertNotEquals(pjlink.model_filter, filter_model, 'Filter type should NOT have been saved') + + def test_projector_process_rlmp_save(self): + """ + Test saving lamp type + """ + # GIVEN: Test object + pjlink = pjlink_test + pjlink.model_lamp = None + lamp_model = 'Lamp Type Test' + + # WHEN: Filter model is received + pjlink.process_rlmp(data=lamp_model) + + # THEN: Filter model number should be saved + self.assertEquals(pjlink.model_lamp, lamp_model, 'Lamp type should have been saved') + + def test_projector_process_rlmp_nosave(self): + """ + Test saving lamp type previously saved + """ + # GIVEN: Test object + pjlink = pjlink_test + pjlink.model_lamp = 'Old lamp type' + lamp_model = 'Filter Type Test' + + # WHEN: Filter model is received + pjlink.process_rlmp(data=lamp_model) + + # THEN: Filter model number should be saved + self.assertNotEquals(pjlink.model_lamp, lamp_model, 'Lamp type should NOT have been saved') + + def test_projector_process_snum_set(self): + """ + Test saving serial number from projector + """ + # GIVEN: Test object + pjlink = pjlink_test + pjlink.serial_no = None + test_number = 'Test Serial Number' + + # WHEN: No serial number is set and we receive serial number command + pjlink.process_snum(data=test_number) + + # THEN: Serial number should be set + self.assertEquals(pjlink.serial_no, test_number, + 'Projector serial number should have been set') + + def test_projector_process_snum_different(self): + """ + Test projector serial number different than saved serial number + """ + # GIVEN: Test object + pjlink = pjlink_test + pjlink.serial_no = 'Previous serial number' + test_number = 'Test Serial Number' + + # WHEN: No serial number is set and we receive serial number command + pjlink.process_snum(data=test_number) + + # THEN: Serial number should be set + self.assertNotEquals(pjlink.serial_no, test_number, + 'Projector serial number should NOT have been set') + + def test_projector_clss_one(self): + """ + Test class 1 sent from projector """ # GIVEN: Test object pjlink = pjlink_test @@ -73,9 +165,23 @@ class TestPJLink(TestCase): self.assertEquals(pjlink.pjlink_class, '1', 'Projector should have returned class=1') - def test_non_standard_class_reply(self): + def test_projector_clss_two(self): """ - Bugfix 1550891: CLSS request returns non-standard 'Class N' reply + Test class 2 sent from projector + """ + # GIVEN: Test object + pjlink = pjlink_test + + # WHEN: Process class response + pjlink.process_clss('2') + + # THEN: Projector class should be set to 1 + self.assertEquals(pjlink.pjlink_class, '2', + 'Projector should have returned class=2') + + def test_bug_1550891_non_standard_class_reply(self): + """ + Bugfix 1550891: CLSS request returns non-standard reply """ # GIVEN: Test object pjlink = pjlink_test @@ -85,7 +191,7 @@ class TestPJLink(TestCase): # THEN: Projector class should be set with proper value self.assertEquals(pjlink.pjlink_class, '1', - 'Non-standard class reply should have set proper class') + 'Non-standard class reply should have set class=1') @patch.object(pjlink_test, 'change_status') def test_status_change(self, mock_change_status): From 1ce9a53d3b9cf636b8ab5a3eba4b580fad46374d Mon Sep 17 00:00:00 2001 From: VirBinarus Date: Tue, 27 Jun 2017 18:42:54 +0100 Subject: [PATCH 09/18] removed blank lines, split up some test functions for readablilty. --- openlp/core/ui/servicemanager.py | 4 -- .../openlp_core_ui/test_servicemanager.py | 48 +++++-------------- 2 files changed, 11 insertions(+), 41 deletions(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 777fc51b0..eb304b8c1 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -1130,14 +1130,12 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa Expands cursor selection on the window. Called by the right arrow """ item = self.service_manager_list.currentItem() - # Since we only have 2 levels we find them by checking for children if item.childCount(): if not self.service_manager_list.isExpanded(self.service_manager_list.currentIndex()): self.service_manager_list.expandItem(item) self.service_manager.expanded(item) # If not expanded, Expand it - self.service_manager_list.setCurrentItem(self.service_manager_list.itemBelow(item)) # Then move selection down to child whether it needed to be expanded or not @@ -1146,13 +1144,11 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa Collapses cursor selection on the window Called by the left arrow """ item = self.service_manager_list.currentItem() - # Since we only have 2 levels we find them by checking for children if item.childCount(): if self.service_manager_list.isExpanded(self.service_manager_list.currentIndex()): self.service_manager_list.collapseItem(item) self.service_manager.collapsed(item) - else: # If selection is lower level self.service_manager_list.collapseItem(item.parent()) self.service_manager.collapsed(item.parent()) diff --git a/tests/interfaces/openlp_core_ui/test_servicemanager.py b/tests/interfaces/openlp_core_ui/test_servicemanager.py index e9202020f..635ca1774 100644 --- a/tests/interfaces/openlp_core_ui/test_servicemanager.py +++ b/tests/interfaces/openlp_core_ui/test_servicemanager.py @@ -416,24 +416,25 @@ class TestServiceManager(TestCase, TestMixin): # THEN the on_move_selection_up function should have been called. self.service_manager.on_move_selection_up.assert_called_once_with() - def test_on_expand_selection(self): - """ - Test that the on_expand_selection function successfully expands an item and moves to its first child - """ - # GIVEN a mocked servicemanager list + def _setup_service_manager_list(self): self.service_manager.expanded = MagicMock() - + self.service_manager.collapsed = MagicMock() verse_1 = QtWidgets.QTreeWidgetItem(0) verse_2 = QtWidgets.QTreeWidgetItem(0) song_item = QtWidgets.QTreeWidgetItem(0) song_item.addChild(verse_1) song_item.addChild(verse_2) - self.service_manager.setup_ui(self.service_manager) self.service_manager.service_manager_list.addTopLevelItem(song_item) + return verse_1, verse_2, song_item + def test_on_expand_selection(self): + """ + Test that the on_expand_selection function successfully expands an item and moves to its first child + """ + # GIVEN a mocked servicemanager list + verse_1, verse_2, song_item = self._setup_service_manager_list() self.service_manager.service_manager_list.setCurrentItem(song_item) - # Reset expanded function in case it has been called and/or changed in initialisation of the service manager. self.service_manager.expanded = MagicMock() @@ -441,7 +442,6 @@ class TestServiceManager(TestCase, TestMixin): self.service_manager.on_expand_selection() # THEN selection should be expanded - selected_index = self.service_manager.service_manager_list.currentIndex() above_selected_index = self.service_manager.service_manager_list.indexAbove(selected_index) self.assertTrue(self.service_manager.service_manager_list.isExpanded(above_selected_index), @@ -452,20 +452,8 @@ class TestServiceManager(TestCase, TestMixin): """ Test that the on_collapse_selection function successfully collapses an item """ - # GIVEN a mocked servicemanager list - self.service_manager.expanded = MagicMock() - self.service_manager.collapsed = MagicMock() - - verse_1 = QtWidgets.QTreeWidgetItem(0) - verse_2 = QtWidgets.QTreeWidgetItem(0) - song_item = QtWidgets.QTreeWidgetItem(0) - song_item.addChild(verse_1) - song_item.addChild(verse_2) - - self.service_manager.setup_ui(self.service_manager) - self.service_manager.service_manager_list.addTopLevelItem(song_item) - + verse_1, verse_2, song_item = self._setup_service_manager_list() self.service_manager.service_manager_list.setCurrentItem(song_item) self.service_manager.service_manager_list.expandItem(song_item) @@ -476,7 +464,6 @@ class TestServiceManager(TestCase, TestMixin): self.service_manager.on_collapse_selection() # THEN selection should be expanded - selected_index = self.service_manager.service_manager_list.currentIndex() self.assertFalse(self.service_manager.service_manager_list.isExpanded(selected_index), 'Item should have been collapsed') @@ -490,21 +477,9 @@ class TestServiceManager(TestCase, TestMixin): and moves selection to its parent. """ # GIVEN a mocked servicemanager list - self.service_manager.expanded = MagicMock() - self.service_manager.collapsed = MagicMock() - - verse_1 = QtWidgets.QTreeWidgetItem(0) - verse_2 = QtWidgets.QTreeWidgetItem(0) - song_item = QtWidgets.QTreeWidgetItem(0) - song_item.addChild(verse_1) - song_item.addChild(verse_2) - - self.service_manager.setup_ui(self.service_manager) - self.service_manager.service_manager_list.addTopLevelItem(song_item) - + verse_1, verse_2, song_item = self._setup_service_manager_list() self.service_manager.service_manager_list.setCurrentItem(verse_2) self.service_manager.service_manager_list.expandItem(song_item) - # Reset collapsed function in case it has been called and/or changed in initialisation of the service manager. self.service_manager.collapsed = MagicMock() @@ -512,7 +487,6 @@ class TestServiceManager(TestCase, TestMixin): self.service_manager.on_collapse_selection() # THEN selection should be expanded - selected_index = self.service_manager.service_manager_list.currentIndex() self.assertFalse(self.service_manager.service_manager_list.isExpanded(selected_index), 'Item should have been collapsed') From 44fb799c7abb2367e3936ac970790ea95352977a Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Wed, 28 Jun 2017 19:58:08 -0700 Subject: [PATCH 10/18] noqa cleanlups --- openlp/core/lib/projector/pjlink1.py | 21 ++--- openlp/core/ui/projector/manager.py | 9 ++- openlp/core/ui/projector/sourceselectform.py | 4 +- .../test_projector_constants.py | 2 +- .../openlp_core_lib/test_projector_db.py | 8 +- .../openlp_core_lib/test_projector_pjlink1.py | 80 +++++++++---------- 6 files changed, 64 insertions(+), 60 deletions(-) diff --git a/openlp/core/lib/projector/pjlink1.py b/openlp/core/lib/projector/pjlink1.py index d71ce7c99..107e79179 100644 --- a/openlp/core/lib/projector/pjlink1.py +++ b/openlp/core/lib/projector/pjlink1.py @@ -381,7 +381,7 @@ class PJLink(QtNetwork.QTcpSocket): self.change_status(E_SOCKET_TIMEOUT) return read = self.readLine(self.max_size) - dontcare = self.readLine(self.max_size) # Clean out the trailing \r\n + _ = self.readLine(self.max_size) # Clean out the trailing \r\n if read is None: log.warning('({ip}) read is None - socket error?'.format(ip=self.ip)) return @@ -391,7 +391,7 @@ class PJLink(QtNetwork.QTcpSocket): data = decode(read, 'utf-8') # Possibility of extraneous data on input when reading. # Clean out extraneous characters in buffer. - dontcare = self.readLine(self.max_size) # noqa: F841 + _ = self.readLine(self.max_size) log.debug('({ip}) check_login() read "{data}"'.format(ip=self.ip, data=data.strip())) # At this point, we should only have the initial login prompt with # possible authentication @@ -413,24 +413,24 @@ class PJLink(QtNetwork.QTcpSocket): # Authentication error self.disconnect_from_host() self.change_status(E_AUTHENTICATION) - log.debug('({ip}) emitting projectorAuthentication() signal'.format(ip=self.name)) + log.debug('({ip}) emitting projectorAuthentication() signal'.format(ip=self.ip)) return elif data_check[1] == '0' and self.pin is not None: # Pin set and no authentication needed log.warning('({ip}) Regular connection but PIN set'.format(ip=self.name)) self.disconnect_from_host() self.change_status(E_AUTHENTICATION) - log.debug('({ip}) Emitting projectorNoAuthentication() signal'.format(ip=self.name)) + log.debug('({ip}) Emitting projectorNoAuthentication() signal'.format(ip=self.ip)) self.projectorNoAuthentication.emit(self.name) return elif data_check[1] == '1': # Authenticated login with salt if self.pin is None: - log.warning('({ip}) Authenticated connection but no pin set'.format(ip=self.name)) + log.warning('({ip}) Authenticated connection but no pin set'.format(ip=self.ip)) self.disconnect_from_host() self.change_status(E_AUTHENTICATION) - log.debug('({ip}) Emitting projectorAuthentication() signal'.format(ip=self.name)) - self.projectorAuthentication.emit(self.name) + log.debug('({ip}) Emitting projectorAuthentication() signal'.format(ip=self.ip)) + self.projectorAuthentication.emit(self.ip) return else: log.debug('({ip}) Setting hash with salt="{data}"'.format(ip=self.ip, data=data_check[2])) @@ -488,10 +488,13 @@ class PJLink(QtNetwork.QTcpSocket): self.check_login(data) self.receive_data_signal() return + elif data[0] != PJLINK_PREFIX: + log.debug("({ip}) get_data(): Invalid packet - prefix not equal to '{prefix}'".format(ip=self.ip, + prefix=PJLINK_PREFIX)) + return data_split = data.split('=') try: - (prefix, version, cmd, data) = (data_split[0][0], data_split[0][1], # noqa: F841 - data_split[0][2:], data_split[1]) + (version, cmd, data) = (data_split[0][1], data_split[0][2:], data_split[1]) except ValueError as e: log.warning('({ip}) get_data(): Invalid packet - expected header + command + data'.format(ip=self.ip)) log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in.strip())) diff --git a/openlp/core/ui/projector/manager.py b/openlp/core/ui/projector/manager.py index fdaf0c577..abb916eee 100644 --- a/openlp/core/ui/projector/manager.py +++ b/openlp/core/ui/projector/manager.py @@ -527,7 +527,7 @@ class ProjectorManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, UiProjecto self.projector_list = new_list list_item = self.projector_list_widget.takeItem(self.projector_list_widget.currentRow()) list_item = None - deleted = self.projectordb.delete_projector(projector.db_item) + _ = self.projectordb.delete_projector(projector.db_item) for item in self.projector_list: log.debug('New projector list - item: {ip} {name}'.format(ip=item.link.ip, name=item.link.name)) @@ -662,9 +662,10 @@ class ProjectorManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, UiProjecto data=translate('OpenLP.ProjectorManager', 'Closed') if projector.link.shutter else translate('OpenLP', 'Open')) - message = '%s%s: %s
' % (message, - translate('OpenLP.ProjectorManager', 'Current source input is'), - projector.link.source) + message = '{msg}{source}/b>: {selected}
'.format(msg=message, + source=translate('OpenLP.ProjectorManager', + 'Current source input is'), + selected=projector.link.source) if projector.link.pjlink_class == '2': # Information only available for PJLink Class 2 projectors message += '{title}: {data}

'.format(title=translate('OpenLP.ProjectorManager', diff --git a/openlp/core/ui/projector/sourceselectform.py b/openlp/core/ui/projector/sourceselectform.py index 1d17c07cd..dc231d3e9 100644 --- a/openlp/core/ui/projector/sourceselectform.py +++ b/openlp/core/ui/projector/sourceselectform.py @@ -393,9 +393,9 @@ class SourceSelectSingle(QtWidgets.QDialog): QtCore.Qt.WindowCloseButtonHint) self.edit = edit if self.edit: - title = translate('OpenLP.SourceSelectForm', 'Edit Projector Source Text') + self.title = translate('OpenLP.SourceSelectForm', 'Edit Projector Source Text') else: - title = translate('OpenLP.SourceSelectForm', 'Select Projector Source') + self.title = translate('OpenLP.SourceSelectForm', 'Select Projector Source') self.setObjectName('source_select_single') self.setWindowIcon(build_icon(':/icon/openlp-log.svg')) self.setModal(True) diff --git a/tests/functional/openlp_core_lib/test_projector_constants.py b/tests/functional/openlp_core_lib/test_projector_constants.py index 61a93eb10..019c18888 100644 --- a/tests/functional/openlp_core_lib/test_projector_constants.py +++ b/tests/functional/openlp_core_lib/test_projector_constants.py @@ -22,7 +22,7 @@ """ Package to test the openlp.core.lib.projector.constants package. """ -from unittest import TestCase, skip # noqa: F401 +from unittest import TestCase, skip class TestProjectorConstants(TestCase): diff --git a/tests/functional/openlp_core_lib/test_projector_db.py b/tests/functional/openlp_core_lib/test_projector_db.py index fd2852acf..d4ff4e75c 100644 --- a/tests/functional/openlp_core_lib/test_projector_db.py +++ b/tests/functional/openlp_core_lib/test_projector_db.py @@ -29,8 +29,8 @@ import os import shutil from tempfile import mkdtemp -from unittest import TestCase, skip # noqa: F401 -from unittest.mock import MagicMock, patch # noqa: F401 +from unittest import TestCase, skip +from unittest.mock import MagicMock, patch from openlp.core.lib.projector import upgrade from openlp.core.lib.db import upgrade_db @@ -413,7 +413,7 @@ class TestProjectorDB(TestCase): Test add_projector() fail """ # GIVEN: Test entry in the database - ignore_result = self.projector.add_projector(Projector(**TEST1_DATA)) # noqa: F841 + ignore_result = self.projector.add_projector(Projector(**TEST1_DATA)) # WHEN: Attempt to add same projector entry results = self.projector.add_projector(Projector(**TEST1_DATA)) @@ -439,7 +439,7 @@ class TestProjectorDB(TestCase): Test update_projector() when entry not in database """ # GIVEN: Projector entry in database - ignore_result = self.projector.add_projector(Projector(**TEST1_DATA)) # noqa: F841 + ignore_result = self.projector.add_projector(Projector(**TEST1_DATA)) projector = Projector(**TEST2_DATA) # WHEN: Attempt to update data with a different ID diff --git a/tests/functional/openlp_core_lib/test_projector_pjlink1.py b/tests/functional/openlp_core_lib/test_projector_pjlink1.py index 3ff47d85a..969ad72e1 100644 --- a/tests/functional/openlp_core_lib/test_projector_pjlink1.py +++ b/tests/functional/openlp_core_lib/test_projector_pjlink1.py @@ -72,7 +72,7 @@ class TestPJLink(TestCase): pjlink.process_rfil(data=filter_model) # THEN: Filter model number should be saved - self.assertEquals(pjlink.model_filter, filter_model, 'Filter type should have been saved') + self.assertEqual(pjlink.model_filter, filter_model, 'Filter type should have been saved') def test_projector_process_rfil_nosave(self): """ @@ -102,7 +102,7 @@ class TestPJLink(TestCase): pjlink.process_rlmp(data=lamp_model) # THEN: Filter model number should be saved - self.assertEquals(pjlink.model_lamp, lamp_model, 'Lamp type should have been saved') + self.assertEqual(pjlink.model_lamp, lamp_model, 'Lamp type should have been saved') def test_projector_process_rlmp_nosave(self): """ @@ -132,8 +132,8 @@ class TestPJLink(TestCase): pjlink.process_snum(data=test_number) # THEN: Serial number should be set - self.assertEquals(pjlink.serial_no, test_number, - 'Projector serial number should have been set') + self.assertEqual(pjlink.serial_no, test_number, + 'Projector serial number should have been set') def test_projector_process_snum_different(self): """ @@ -162,8 +162,8 @@ class TestPJLink(TestCase): pjlink.process_clss('1') # THEN: Projector class should be set to 1 - self.assertEquals(pjlink.pjlink_class, '1', - 'Projector should have returned class=1') + self.assertEqual(pjlink.pjlink_class, '1', + 'Projector should have returned class=1') def test_projector_clss_two(self): """ @@ -176,8 +176,8 @@ class TestPJLink(TestCase): pjlink.process_clss('2') # THEN: Projector class should be set to 1 - self.assertEquals(pjlink.pjlink_class, '2', - 'Projector should have returned class=2') + self.assertEqual(pjlink.pjlink_class, '2', + 'Projector should have returned class=2') def test_bug_1550891_non_standard_class_reply(self): """ @@ -190,8 +190,8 @@ class TestPJLink(TestCase): pjlink.process_clss('Class 1') # THEN: Projector class should be set with proper value - self.assertEquals(pjlink.pjlink_class, '1', - 'Non-standard class reply should have set class=1') + self.assertEqual(pjlink.pjlink_class, '1', + 'Non-standard class reply should have set class=1') @patch.object(pjlink_test, 'change_status') def test_status_change(self, mock_change_status): @@ -237,10 +237,10 @@ class TestPJLink(TestCase): pjlink.process_command('LAMP', '22222 1') # THEN: Lamp should have been set with status=ON and hours=22222 - self.assertEquals(pjlink.lamp[0]['On'], True, - 'Lamp power status should have been set to TRUE') - self.assertEquals(pjlink.lamp[0]['Hours'], 22222, - 'Lamp hours should have been set to 22222') + self.assertEqual(pjlink.lamp[0]['On'], True, + 'Lamp power status should have been set to TRUE') + self.assertEqual(pjlink.lamp[0]['Hours'], 22222, + 'Lamp hours should have been set to 22222') @patch.object(pjlink_test, 'projectorReceivedData') def test_projector_process_multiple_lamp(self, mock_projectorReceivedData): @@ -254,20 +254,20 @@ class TestPJLink(TestCase): pjlink.process_command('LAMP', '11111 1 22222 0 33333 1') # THEN: Lamp should have been set with proper lamp status - self.assertEquals(len(pjlink.lamp), 3, - 'Projector should have 3 lamps specified') - self.assertEquals(pjlink.lamp[0]['On'], True, - 'Lamp 1 power status should have been set to TRUE') - self.assertEquals(pjlink.lamp[0]['Hours'], 11111, - 'Lamp 1 hours should have been set to 11111') - self.assertEquals(pjlink.lamp[1]['On'], False, - 'Lamp 2 power status should have been set to FALSE') - self.assertEquals(pjlink.lamp[1]['Hours'], 22222, - 'Lamp 2 hours should have been set to 22222') - self.assertEquals(pjlink.lamp[2]['On'], True, - 'Lamp 3 power status should have been set to TRUE') - self.assertEquals(pjlink.lamp[2]['Hours'], 33333, - 'Lamp 3 hours should have been set to 33333') + self.assertEqual(len(pjlink.lamp), 3, + 'Projector should have 3 lamps specified') + self.assertEqual(pjlink.lamp[0]['On'], True, + 'Lamp 1 power status should have been set to TRUE') + self.assertEqual(pjlink.lamp[0]['Hours'], 11111, + 'Lamp 1 hours should have been set to 11111') + self.assertEqual(pjlink.lamp[1]['On'], False, + 'Lamp 2 power status should have been set to FALSE') + self.assertEqual(pjlink.lamp[1]['Hours'], 22222, + 'Lamp 2 hours should have been set to 22222') + self.assertEqual(pjlink.lamp[2]['On'], True, + 'Lamp 3 power status should have been set to TRUE') + self.assertEqual(pjlink.lamp[2]['Hours'], 33333, + 'Lamp 3 hours should have been set to 33333') @patch.object(pjlink_test, 'projectorReceivedData') @patch.object(pjlink_test, 'projectorUpdateIcons') @@ -288,9 +288,9 @@ class TestPJLink(TestCase): pjlink.process_command('POWR', PJLINK_POWR_STATUS[S_ON]) # THEN: Power should be set to ON - self.assertEquals(pjlink.power, S_ON, 'Power should have been set to ON') + self.assertEqual(pjlink.power, S_ON, 'Power should have been set to ON') mock_send_command.assert_called_once_with('INST') - self.assertEquals(mock_UpdateIcons.emit.called, True, 'projectorUpdateIcons should have been called') + self.assertEqual(mock_UpdateIcons.emit.called, True, 'projectorUpdateIcons should have been called') @patch.object(pjlink_test, 'projectorReceivedData') @patch.object(pjlink_test, 'projectorUpdateIcons') @@ -311,9 +311,9 @@ class TestPJLink(TestCase): pjlink.process_command('POWR', PJLINK_POWR_STATUS[S_STANDBY]) # THEN: Power should be set to STANDBY - self.assertEquals(pjlink.power, S_STANDBY, 'Power should have been set to STANDBY') - self.assertEquals(mock_send_command.called, False, 'send_command should not have been called') - self.assertEquals(mock_UpdateIcons.emit.called, True, 'projectorUpdateIcons should have been called') + self.assertEqual(pjlink.power, S_STANDBY, 'Power should have been set to STANDBY') + self.assertEqual(mock_send_command.called, False, 'send_command should not have been called') + self.assertEqual(mock_UpdateIcons.emit.called, True, 'projectorUpdateIcons should have been called') @patch.object(pjlink_test, 'projectorUpdateIcons') def test_projector_process_avmt_closed_unmuted(self, mock_projectorReceivedData): @@ -395,7 +395,7 @@ class TestPJLink(TestCase): pjlink.process_inpt('1') # THEN: Input selected should reflect current input - self.assertEquals(pjlink.source, '1', 'Input source should be set to "1"') + self.assertEqual(pjlink.source, '1', 'Input source should be set to "1"') def test_projector_reset_information(self): """ @@ -424,7 +424,7 @@ class TestPJLink(TestCase): pjlink.reset_information() # THEN: All information should be reset and timers stopped - self.assertEquals(pjlink.power, S_OFF, 'Projector power should be OFF') + self.assertEqual(pjlink.power, S_OFF, 'Projector power should be OFF') self.assertIsNone(pjlink.pjlink_name, 'Projector pjlink_name should be None') self.assertIsNone(pjlink.manufacturer, 'Projector manufacturer should be None') self.assertIsNone(pjlink.model, 'Projector model should be None') @@ -435,7 +435,7 @@ class TestPJLink(TestCase): self.assertIsNone(pjlink.source_available, 'Projector source_available should be None') self.assertIsNone(pjlink.source, 'Projector source should be None') self.assertIsNone(pjlink.other_info, 'Projector other_info should be None') - self.assertEquals(pjlink.send_queue, [], 'Projector send_queue should be an empty list') + self.assertEqual(pjlink.send_queue, [], 'Projector send_queue should be an empty list') self.assertFalse(pjlink.send_busy, 'Projector send_busy should be False') self.assertTrue(mock_timer.called, 'Projector timer.stop() should have been called') self.assertTrue(mock_socket_timer.called, 'Projector socket_timer.stop() should have been called') @@ -461,8 +461,8 @@ class TestPJLink(TestCase): # WHEN: call with authentication request and pin not set pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE) - # THEN: No Authentication signal should have been sent - mock_authentication.emit.assert_called_with(pjlink.name) + # THEN: 'No Authentication' signal should have been sent + mock_authentication.emit.assert_called_with(pjlink.ip) @patch.object(pjlink_test, 'waitForReadyRead') @patch.object(pjlink_test, 'state') @@ -487,8 +487,8 @@ class TestPJLink(TestCase): pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE) # THEN: send_command should have the proper authentication - self.assertEquals("{test}".format(test=mock_send_command.call_args), - "call(data='{hash}%1CLSS ?\\r')".format(hash=TEST_HASH)) + self.assertEqual("{test}".format(test=mock_send_command.call_args), + "call(data='{hash}%1CLSS ?\\r')".format(hash=TEST_HASH)) @patch.object(pjlink_test, 'disconnect_from_host') def socket_abort_test(self, mock_disconnect): From d072dd4063bbc7bb5d6ae6136dde29464736f987 Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Wed, 28 Jun 2017 20:19:13 -0700 Subject: [PATCH 11/18] Drop future import comments --- openlp/core/lib/projector/pjlink1.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/openlp/core/lib/projector/pjlink1.py b/openlp/core/lib/projector/pjlink1.py index 107e79179..02e8466be 100644 --- a/openlp/core/lib/projector/pjlink1.py +++ b/openlp/core/lib/projector/pjlink1.py @@ -58,9 +58,6 @@ from openlp.core.lib.projector.constants import CONNECTION_ERRORS, CR, ERROR_MSG STATUS_STRING, S_CONNECTED, S_CONNECTING, S_NETWORK_RECEIVED, S_NETWORK_SENDING, \ S_NOT_CONNECTED, S_OFF, S_OK, S_ON, S_STATUS -# Possible future imports -# from openlp.core.lib.projector.constants import PJLINK_DEFAULT_CODES - # Shortcuts SocketError = QtNetwork.QAbstractSocket.SocketError SocketSTate = QtNetwork.QAbstractSocket.SocketState From e277c804f97fa6234bdf089fbf30bd0014db0fe5 Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Fri, 7 Jul 2017 16:43:50 -0700 Subject: [PATCH 12/18] Cleanups part 3 --- openlp/core/lib/projector/pjlink1.py | 76 +++++++++++++--------------- openlp/core/lib/projector/pjlink2.py | 4 +- openlp/core/lib/projector/upgrade.py | 5 -- openlp/core/ui/projector/manager.py | 39 +++++--------- setup.cfg | 4 +- 5 files changed, 53 insertions(+), 75 deletions(-) diff --git a/openlp/core/lib/projector/pjlink1.py b/openlp/core/lib/projector/pjlink1.py index 02e8466be..a76f82217 100644 --- a/openlp/core/lib/projector/pjlink1.py +++ b/openlp/core/lib/projector/pjlink1.py @@ -45,7 +45,6 @@ log.debug('pjlink1 loaded') __all__ = ['PJLink'] import re - from codecs import decode from PyQt5 import QtCore, QtNetwork @@ -252,6 +251,7 @@ class PJLink(QtNetwork.QTcpSocket): Normally called by timer(). """ if self.state() != self.ConnectedState: + log.warn("({ip}) poll_loop(): Not connected - returning".format(ip=self.ip)) return log.debug('({ip}) Updating projector status'.format(ip=self.ip)) # Reset timer in case we were called from a set command @@ -260,7 +260,7 @@ class PJLink(QtNetwork.QTcpSocket): self.timer.setInterval(self.poll_time) # Restart timer self.timer.start() - # These commands may change during connetion + # These commands may change during connection check_list = ['POWR', 'ERST', 'LAMP', 'AVMT', 'INPT'] if self.pjlink_class == '2': check_list.extend(['FILT', 'FREZ']) @@ -378,7 +378,8 @@ class PJLink(QtNetwork.QTcpSocket): self.change_status(E_SOCKET_TIMEOUT) return read = self.readLine(self.max_size) - _ = self.readLine(self.max_size) # Clean out the trailing \r\n + # _ = self.readLine(self.max_size) # Clean out the trailing \r\n + self.readLine(self.max_size) # Clean out the trailing \r\n if read is None: log.warning('({ip}) read is None - socket error?'.format(ip=self.ip)) return @@ -388,7 +389,8 @@ class PJLink(QtNetwork.QTcpSocket): data = decode(read, 'utf-8') # Possibility of extraneous data on input when reading. # Clean out extraneous characters in buffer. - _ = self.readLine(self.max_size) + # _ = self.readLine(self.max_size) + self.readLine(self.max_size) log.debug('({ip}) check_login() read "{data}"'.format(ip=self.ip, data=data.strip())) # At this point, we should only have the initial login prompt with # possible authentication @@ -447,6 +449,20 @@ class PJLink(QtNetwork.QTcpSocket): self.timer.setInterval(2000) # Set 2 seconds for initial information self.timer.start() + def _trash_buffer(self, msg=None): + """ + Clean out extraneous stuff in the buffer. + """ + log.warning("({ip}) {message}".format(ip=self.ip, message='Invalid packet' if msg is None else msg)) + self.send_busy = False + trash_count = 0 + while self.bytesAvailable() > 0: + trash = self.read(self.max_size) + trash_count += len(trash) + log.debug("({ip}) Finished cleaning buffer - {count} bytes dropped".format(ip=self.ip, + count=trash_count)) + return + @QtCore.pyqtSlot() def get_data(self): """ @@ -461,47 +477,27 @@ class PJLink(QtNetwork.QTcpSocket): if read == -1: # No data available log.debug('({ip}) get_data(): No data available (-1)'.format(ip=self.ip)) - self.send_busy = False - self.projectorReceivedData.emit() - return + return self.receive_data_signal() self.socket_timer.stop() self.projectorNetwork.emit(S_NETWORK_RECEIVED) # NOTE: Class2 has changed to some values being UTF-8 data_in = decode(read, 'utf-8') data = data_in.strip() - if len(data) < 7: - # Not enough data for a packet - log.debug('({ip}) get_data(): Packet length < 7: "{data}"'.format(ip=self.ip, data=data)) - self.receive_data_signal() - return + if (len(data) < 7) or (not data.startswith(PJLINK_PREFIX)): + return self._trash_buffer(msg='get_data(): Invalid packet - length or prefix') elif '=' not in data: - log.warning('({ip}) get_data(): Invalid packet received'.format(ip=self.ip)) - self.receive_data_signal() - return + return self._trash_buffer(msg='get_data(): Invalid packet does not have equal') log.debug('({ip}) get_data(): Checking new data "{data}"'.format(ip=self.ip, data=data)) - # At this point, we should have something to work with - if data.upper().startswith('PJLINK'): - # Reconnected from remote host disconnect ? - self.check_login(data) - self.receive_data_signal() - return - elif data[0] != PJLINK_PREFIX: - log.debug("({ip}) get_data(): Invalid packet - prefix not equal to '{prefix}'".format(ip=self.ip, - prefix=PJLINK_PREFIX)) - return - data_split = data.split('=') + header, data = data.split('=') try: - (version, cmd, data) = (data_split[0][1], data_split[0][2:], data_split[1]) + version, cmd = header[1], header[2:] except ValueError as e: - log.warning('({ip}) get_data(): Invalid packet - expected header + command + data'.format(ip=self.ip)) - log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in.strip())) self.change_status(E_INVALID_DATA) - self.receive_data_signal() - return + log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in.strip())) + return self._trash_buffer('get_data(): Expected header + command + data') if cmd not in PJLINK_VALID_CMD: log.warning('({ip}) get_data(): Invalid packet - unknown command "{data}"'.format(ip=self.ip, data=cmd)) - self.receive_data_signal() - return + return self._trash_buffer(msg='get_data(): Unknown command "{data}"'.format(data=cmd)) if int(self.pjlink_class) < int(version): log.warn('({ip}) get_data(): Projector returned class reply higher ' 'than projector stated class'.format(ip=self.ip)) @@ -557,7 +553,7 @@ class PJLink(QtNetwork.QTcpSocket): salt='' if salt is None else ' with hash')) cmd_ver = PJLINK_VALID_CMD[cmd]['version'] - if self.pjlink_class in cmd_ver: + if self.pjlink_class in PJLINK_VALID_CMD[cmd]['version']: header = PJLINK_HEADER.format(linkclass=self.pjlink_class) elif len(cmd_ver) == 1 and (int(cmd_ver[0]) < int(self.pjlink_class)): # Typically a class 1 only command @@ -626,6 +622,7 @@ class PJLink(QtNetwork.QTcpSocket): self.waitForBytesWritten(2000) # 2 seconds should be enough if sent == -1: # Network error? + log.warning("({ip}) _send_command(): -1 received".format(ip=self.ip)) self.change_status(E_NETWORK, translate('OpenLP.PJLink', 'Error while sending data to projector')) @@ -668,20 +665,17 @@ class PJLink(QtNetwork.QTcpSocket): elif data.upper() == 'ERR4': # Projector/display error self.change_status(E_PROJECTOR) - self.send_busy = False - self.projectorReceivedData.emit() + self.receive_data_signal() return # Command succeeded - no extra information elif data.upper() == 'OK': log.debug('({ip}) Command returned OK'.format(ip=self.ip)) - # A command returned successfully, recheck data - self.send_busy = False - self.projectorReceivedData.emit() + # A command returned successfully + self.receive_data_signal() return # Command checks already passed log.debug('({ip}) Calling function for {cmd}'.format(ip=self.ip, cmd=cmd)) - self.send_busy = False - self.projectorReceivedData.emit() + self.receive_data_signal() self.pjlink_functions[cmd](data) def process_lamp(self, data): diff --git a/openlp/core/lib/projector/pjlink2.py b/openlp/core/lib/projector/pjlink2.py index 65f2de336..caa3a0c6b 100644 --- a/openlp/core/lib/projector/pjlink2.py +++ b/openlp/core/lib/projector/pjlink2.py @@ -71,10 +71,10 @@ log = logging.getLogger(__name__) log.debug('pjlink2 loaded') -from PyQt5 import QtCore, QtNetwork +from PyQt5 import QtNetwork -class PJLinkUDP(QtNetwork.QTcpSocket): +class PJLinkUDP(QtNetwork.QUdpSocket): """ Socket service for handling datagram (UDP) sockets. """ diff --git a/openlp/core/lib/projector/upgrade.py b/openlp/core/lib/projector/upgrade.py index 178ab2be9..acb3c1b0b 100644 --- a/openlp/core/lib/projector/upgrade.py +++ b/openlp/core/lib/projector/upgrade.py @@ -32,11 +32,6 @@ from openlp.core.lib.db import get_upgrade_op log = logging.getLogger(__name__) -# Possible future imports -# from sqlalchemy.exc import NoSuchTableError -# from sqlalchemy import inspect -# from openlp.core.common.db import drop_columns - # Initial projector DB was unversioned __version__ = 2 diff --git a/openlp/core/ui/projector/manager.py b/openlp/core/ui/projector/manager.py index abb916eee..7eb5451ad 100644 --- a/openlp/core/ui/projector/manager.py +++ b/openlp/core/ui/projector/manager.py @@ -420,9 +420,7 @@ class ProjectorManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, UiProjecto :param opt: Needed by PyQt5 """ try: - ip = opt.link.ip - projector = opt - projector.link.set_shutter_closed() + opt.link.set_shutter_closed() except AttributeError: for list_item in self.projector_list_widget.selectedItems(): if list_item is None: @@ -455,9 +453,7 @@ class ProjectorManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, UiProjecto :param opt: Needed by PyQt5 """ try: - ip = opt.link.ip - projector = opt - projector.link.connect_to_host() + opt.link.connect_to_host() except AttributeError: for list_item in self.projector_list_widget.selectedItems(): if list_item is None: @@ -527,7 +523,8 @@ class ProjectorManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, UiProjecto self.projector_list = new_list list_item = self.projector_list_widget.takeItem(self.projector_list_widget.currentRow()) list_item = None - _ = self.projectordb.delete_projector(projector.db_item) + if not self.projectordb.delete_projector(projector.db_item): + log.warning('Delete projector {item} failed'.format(item=projector.db_item)) for item in self.projector_list: log.debug('New projector list - item: {ip} {name}'.format(ip=item.link.ip, name=item.link.name)) @@ -538,9 +535,7 @@ class ProjectorManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, UiProjecto :param opt: Needed by PyQt5 """ try: - ip = opt.link.ip - projector = opt - projector.link.disconnect_from_host() + opt.link.disconnect_from_host() except AttributeError: for list_item in self.projector_list_widget.selectedItems(): if list_item is None: @@ -573,9 +568,7 @@ class ProjectorManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, UiProjecto :param opt: Needed by PyQt5 """ try: - ip = opt.link.ip - projector = opt - projector.link.set_power_off() + opt.link.set_power_off() except AttributeError: for list_item in self.projector_list_widget.selectedItems(): if list_item is None: @@ -593,9 +586,7 @@ class ProjectorManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, UiProjecto :param opt: Needed by PyQt5 """ try: - ip = opt.link.ip - projector = opt - projector.link.set_power_on() + opt.link.set_power_on() except AttributeError: for list_item in self.projector_list_widget.selectedItems(): if list_item is None: @@ -613,9 +604,7 @@ class ProjectorManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, UiProjecto :param opt: Needed by PyQt5 """ try: - ip = opt.link.ip - projector = opt - projector.link.set_shutter_open() + opt.link.set_shutter_open() except AttributeError: for list_item in self.projector_list_widget.selectedItems(): if list_item is None: @@ -662,10 +651,10 @@ class ProjectorManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, UiProjecto data=translate('OpenLP.ProjectorManager', 'Closed') if projector.link.shutter else translate('OpenLP', 'Open')) - message = '{msg}{source}/b>: {selected}
'.format(msg=message, - source=translate('OpenLP.ProjectorManager', - 'Current source input is'), - selected=projector.link.source) + message = '{msg}{source}: {selected}
'.format(msg=message, + source=translate('OpenLP.ProjectorManager', + 'Current source input is'), + selected=projector.link.source) if projector.link.pjlink_class == '2': # Information only available for PJLink Class 2 projectors message += '{title}: {data}

'.format(title=translate('OpenLP.ProjectorManager', @@ -686,10 +675,10 @@ class ProjectorManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, UiProjecto 'Lamp'), count=count, status=translate('OpenLP.ProjectorManager', - ' is on') + 'ON') if item['On'] else translate('OpenLP.ProjectorManager', - 'is off')) + 'OFF')) message += '{title}: {hours}
'.format(title=translate('OpenLP.ProjectorManager', 'Hours'), hours=item['Hours']) diff --git a/setup.cfg b/setup.cfg index e7e2651c0..bddcc5291 100644 --- a/setup.cfg +++ b/setup.cfg @@ -5,10 +5,10 @@ [pep8] exclude=resources.py,vlc.py max-line-length = 120 -ignore = E402,E722 +ignore = E402 [flake8] exclude=resources.py,vlc.py max-line-length = 120 -ignore = E402,E722 +ignore = E402 From f333d8d7a02e22775b8c33337ae50e7102527f85 Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Wed, 12 Jul 2017 06:01:46 -0700 Subject: [PATCH 13/18] Ooops - removed commented code no longer needed --- openlp/core/lib/projector/pjlink1.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/openlp/core/lib/projector/pjlink1.py b/openlp/core/lib/projector/pjlink1.py index a76f82217..074272d2a 100644 --- a/openlp/core/lib/projector/pjlink1.py +++ b/openlp/core/lib/projector/pjlink1.py @@ -378,7 +378,6 @@ class PJLink(QtNetwork.QTcpSocket): self.change_status(E_SOCKET_TIMEOUT) return read = self.readLine(self.max_size) - # _ = self.readLine(self.max_size) # Clean out the trailing \r\n self.readLine(self.max_size) # Clean out the trailing \r\n if read is None: log.warning('({ip}) read is None - socket error?'.format(ip=self.ip)) @@ -389,7 +388,6 @@ class PJLink(QtNetwork.QTcpSocket): data = decode(read, 'utf-8') # Possibility of extraneous data on input when reading. # Clean out extraneous characters in buffer. - # _ = self.readLine(self.max_size) self.readLine(self.max_size) log.debug('({ip}) check_login() read "{data}"'.format(ip=self.ip, data=data.strip())) # At this point, we should only have the initial login prompt with From 08848a14e746a0d472ee1f7ededd4aeed419bcab Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Thu, 20 Jul 2017 08:31:50 -0700 Subject: [PATCH 14/18] Added calls to new functions --- openlp/core/lib/projector/pjlink1.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openlp/core/lib/projector/pjlink1.py b/openlp/core/lib/projector/pjlink1.py index 074272d2a..b2b1a4af1 100644 --- a/openlp/core/lib/projector/pjlink1.py +++ b/openlp/core/lib/projector/pjlink1.py @@ -166,7 +166,9 @@ class PJLink(QtNetwork.QTcpSocket): 'PJLINK': self.check_login, 'POWR': self.process_powr, 'SNUM': self.process_snum, - 'SVER': self.process_sver + 'SVER': self.process_sver, + 'RFIL': self.process_rfil, + 'RLMP': self.process_rlmp } def reset_information(self): From 63bd98372a9a87e8b1450cfc71b092f1711457a4 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Tue, 1 Aug 2017 21:59:41 +0100 Subject: [PATCH 15/18] Move applocation over to using pathlib --- openlp/core/__init__.py | 8 +- openlp/core/common/__init__.py | 6 +- openlp/core/common/applocation.py | 108 ++++++++++-------- openlp/core/common/languagemanager.py | 4 +- openlp/core/common/versionchecker.py | 2 +- openlp/core/lib/db.py | 4 +- openlp/core/lib/path.py | 61 ++++++++++ openlp/core/lib/pluginmanager.py | 2 +- openlp/core/lib/serviceitem.py | 2 +- openlp/core/lib/theme.py | 2 +- openlp/core/ui/advancedtab.py | 8 +- openlp/core/ui/firsttimeform.py | 4 +- openlp/core/ui/maindisplay.py | 2 +- openlp/core/ui/mainwindow.py | 8 +- openlp/core/ui/printserviceform.py | 2 +- openlp/core/ui/servicemanager.py | 2 +- openlp/core/ui/thememanager.py | 5 +- .../plugins/bibles/forms/bibleimportform.py | 2 +- openlp/plugins/bibles/lib/db.py | 4 +- openlp/plugins/bibles/lib/manager.py | 4 +- openlp/plugins/images/lib/mediaitem.py | 2 +- openlp/plugins/media/lib/mediaitem.py | 2 +- openlp/plugins/media/mediaplugin.py | 2 +- .../presentations/lib/pdfcontroller.py | 10 +- .../presentations/lib/pptviewcontroller.py | 2 +- .../lib/presentationcontroller.py | 5 +- openlp/plugins/remotes/lib/httprouter.py | 10 +- openlp/plugins/songs/forms/editsongform.py | 2 +- openlp/plugins/songs/lib/__init__.py | 2 +- .../plugins/songs/lib/importers/songimport.py | 2 +- openlp/plugins/songs/lib/mediaitem.py | 6 +- .../openlp_core_common/test_applocation.py | 62 +++++----- .../openlp_core_common/test_common.py | 23 ++-- tests/functional/openlp_core_lib/test_path.py | 88 ++++++++++++++ 34 files changed, 312 insertions(+), 146 deletions(-) create mode 100644 openlp/core/lib/path.py create mode 100644 tests/functional/openlp_core_lib/test_path.py diff --git a/openlp/core/__init__.py b/openlp/core/__init__.py index 49553513c..350f5fe6d 100644 --- a/openlp/core/__init__.py +++ b/openlp/core/__init__.py @@ -181,7 +181,7 @@ class OpenLP(OpenLPMixin, QtWidgets.QApplication): """ Check if the data folder path exists. """ - data_folder_path = AppLocation.get_data_path() + data_folder_path = str(AppLocation.get_data_path()) if not os.path.exists(data_folder_path): log.critical('Database was not found in: ' + data_folder_path) status = QtWidgets.QMessageBox.critical(None, translate('OpenLP', 'Data Directory Error'), @@ -253,7 +253,7 @@ class OpenLP(OpenLPMixin, QtWidgets.QApplication): 'a backup of the old data folder?'), defaultButton=QtWidgets.QMessageBox.Yes) == QtWidgets.QMessageBox.Yes: # Create copy of data folder - data_folder_path = AppLocation.get_data_path() + data_folder_path = str(AppLocation.get_data_path()) timestamp = time.strftime("%Y%m%d-%H%M%S") data_folder_backup_path = data_folder_path + '-' + timestamp try: @@ -390,7 +390,7 @@ def main(args=None): application.setApplicationName('OpenLPPortable') Settings.setDefaultFormat(Settings.IniFormat) # Get location OpenLPPortable.ini - application_path = AppLocation.get_directory(AppLocation.AppDir) + application_path = str(AppLocation.get_directory(AppLocation.AppDir)) set_up_logging(os.path.abspath(os.path.join(application_path, '..', '..', 'Other'))) log.info('Running portable') portable_settings_file = os.path.abspath(os.path.join(application_path, '..', '..', 'Data', 'OpenLP.ini')) @@ -407,7 +407,7 @@ def main(args=None): portable_settings.sync() else: application.setApplicationName('OpenLP') - set_up_logging(AppLocation.get_directory(AppLocation.CacheDir)) + set_up_logging(str(AppLocation.get_directory(AppLocation.CacheDir))) Registry.create() Registry().register('application', application) application.setApplicationVersion(get_application_version()['version']) diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index fde02506d..52da5af2c 100644 --- a/openlp/core/common/__init__.py +++ b/openlp/core/common/__init__.py @@ -95,9 +95,9 @@ def extension_loader(glob_pattern, excluded_files=[]): :return: None :rtype: None """ - app_dir = Path(AppLocation.get_directory(AppLocation.AppDir)).parent - for extension_path in app_dir.glob(glob_pattern): - extension_path = extension_path.relative_to(app_dir) + base_dir_path = AppLocation.get_directory(AppLocation.AppDir).parent + for extension_path in base_dir_path.glob(glob_pattern): + extension_path = extension_path.relative_to(base_dir_path) if extension_path.name in excluded_files: continue module_name = path_to_module(extension_path) diff --git a/openlp/core/common/applocation.py b/openlp/core/common/applocation.py index 126014014..ad81e36ad 100644 --- a/openlp/core/common/applocation.py +++ b/openlp/core/common/applocation.py @@ -25,6 +25,7 @@ The :mod:`openlp.core.common.applocation` module provides an utility for OpenLP import logging import os import sys +from pathlib import Path from openlp.core.common import Settings, is_win, is_macosx @@ -42,6 +43,9 @@ from openlp.core.common import check_directory_exists, get_frozen_path log = logging.getLogger(__name__) +FROZEN_APP_PATH = Path(sys.argv[0]).parent +APP_PATH = Path(openlp.__file__).parent + class AppLocation(object): """ @@ -63,20 +67,19 @@ class AppLocation(object): Return the appropriate directory according to the directory type. :param dir_type: The directory type you want, for instance the data directory. Default *AppLocation.AppDir* + :type dir_type: AppLocation Enum + + :return: The requested path + :rtype: pathlib.Path """ - if dir_type == AppLocation.AppDir: - return get_frozen_path(os.path.abspath(os.path.dirname(sys.argv[0])), os.path.dirname(openlp.__file__)) + if dir_type == AppLocation.AppDir or dir_type == AppLocation.VersionDir: + return get_frozen_path(FROZEN_APP_PATH, APP_PATH) elif dir_type == AppLocation.PluginsDir: - app_path = os.path.abspath(os.path.dirname(sys.argv[0])) - return get_frozen_path(os.path.join(app_path, 'plugins'), - os.path.join(os.path.dirname(openlp.__file__), 'plugins')) - elif dir_type == AppLocation.VersionDir: - return get_frozen_path(os.path.abspath(os.path.dirname(sys.argv[0])), os.path.dirname(openlp.__file__)) + return get_frozen_path(FROZEN_APP_PATH, APP_PATH) / 'plugins' elif dir_type == AppLocation.LanguageDir: - app_path = get_frozen_path(os.path.abspath(os.path.dirname(sys.argv[0])), _get_os_dir_path(dir_type)) - return os.path.join(app_path, 'i18n') + return get_frozen_path(FROZEN_APP_PATH, _get_os_dir_path(dir_type)) / 'i18n' elif dir_type == AppLocation.DataDir and AppLocation.BaseDir: - return os.path.join(AppLocation.BaseDir, 'data') + return Path(AppLocation.BaseDir, 'data') else: return _get_os_dir_path(dir_type) @@ -84,84 +87,97 @@ class AppLocation(object): def get_data_path(): """ Return the path OpenLP stores all its data under. + + :return: The data path to use. + :rtype: pathlib.Path """ # Check if we have a different data location. if Settings().contains('advanced/data path'): - path = Settings().value('advanced/data path') + path = Path(Settings().value('advanced/data path')) else: path = AppLocation.get_directory(AppLocation.DataDir) - check_directory_exists(path) - return os.path.normpath(path) + check_directory_exists(str(path)) + return path @staticmethod - def get_files(section=None, extension=None): + def get_files(section=None, extension=''): """ Get a list of files from the data files path. + + :param section: Defaults to *None*. The section of code getting the files - used to load from a section's data + subdirectory. + :type section: None | str - :param section: Defaults to *None*. The section of code getting the files - used to load from a section's - data subdirectory. - :param extension: - Defaults to *None*. The extension to search for. For example:: - - '.png' + :param extension: Defaults to ''. The extension to search for. For example:: + '.png' + :type extension: str + + :return: List of files found. + :rtype: list[pathlib.Path] """ path = AppLocation.get_data_path() if section: - path = os.path.join(path, section) + path = path / section try: - files = os.listdir(path) + file_paths = path.glob('*' + extension) + return [file_path.relative_to(path) for file_path in file_paths] # TODO: Could/should this be an iterator? except OSError: return [] - if extension: - return [filename for filename in files if extension == os.path.splitext(filename)[1]] - else: - # no filtering required - return files @staticmethod def get_section_data_path(section): """ Return the path a particular module stores its data under. + + :type section: str + + :rtype: pathlib.Path """ - data_path = AppLocation.get_data_path() - path = os.path.join(data_path, section) - check_directory_exists(path) + path = AppLocation.get_data_path() / section + check_directory_exists(str(path)) return path def _get_os_dir_path(dir_type): """ Return a path based on which OS and environment we are running in. + + :param dir_type: AppLocation Enum of the requested path type + :type dir_type: AppLocation Enum + + :return: The requested path + :rtype: pathlib.Path """ # If running from source, return the language directory from the source directory if dir_type == AppLocation.LanguageDir: - directory = os.path.abspath(os.path.join(os.path.dirname(openlp.__file__), '..', 'resources')) - if os.path.exists(directory): + directory = Path(os.path.abspath(os.path.join(os.path.dirname(openlp.__file__), '..', 'resources'))) + if directory.exists(): return directory if is_win(): + openlp_folder_path = Path(os.getenv('APPDATA'), 'openlp') if dir_type == AppLocation.DataDir: - return os.path.join(str(os.getenv('APPDATA')), 'openlp', 'data') + return openlp_folder_path / 'data' elif dir_type == AppLocation.LanguageDir: return os.path.dirname(openlp.__file__) - return os.path.join(str(os.getenv('APPDATA')), 'openlp') + return openlp_folder_path elif is_macosx(): + openlp_folder_path = Path(os.getenv('HOME'), 'Library', 'Application Support', 'openlp') if dir_type == AppLocation.DataDir: - return os.path.join(str(os.getenv('HOME')), 'Library', 'Application Support', 'openlp', 'Data') + return openlp_folder_path / 'Data' elif dir_type == AppLocation.LanguageDir: return os.path.dirname(openlp.__file__) - return os.path.join(str(os.getenv('HOME')), 'Library', 'Application Support', 'openlp') + return openlp_folder_path else: if dir_type == AppLocation.LanguageDir: - for prefix in ['/usr/local', '/usr']: - directory = os.path.join(prefix, 'share', 'openlp') - if os.path.exists(directory): - return directory - return os.path.join('/usr', 'share', 'openlp') + directory = Path('/usr', 'local', 'share', 'openlp') + if directory.exists(): + return directory + return Path('/usr', 'share', 'openlp') if XDG_BASE_AVAILABLE: - if dir_type == AppLocation.DataDir: - return os.path.join(str(BaseDirectory.xdg_data_home), 'openlp') + if dir_type == AppLocation.DataDir or dir_type == AppLocation.CacheDir: + return Path(BaseDirectory.xdg_data_home, 'openlp') elif dir_type == AppLocation.CacheDir: - return os.path.join(str(BaseDirectory.xdg_cache_home), 'openlp') + return Path(BaseDirectory.xdg_cache_home, 'openlp') if dir_type == AppLocation.DataDir: - return os.path.join(str(os.getenv('HOME')), '.openlp', 'data') - return os.path.join(str(os.getenv('HOME')), '.openlp') + return Path(os.getenv('HOME'), '.openlp', 'data') + return Path(os.getenv('HOME'), '.openlp') diff --git a/openlp/core/common/languagemanager.py b/openlp/core/common/languagemanager.py index 026a6ddda..35b195031 100644 --- a/openlp/core/common/languagemanager.py +++ b/openlp/core/common/languagemanager.py @@ -53,7 +53,7 @@ class LanguageManager(object): """ if LanguageManager.auto_language: language = QtCore.QLocale.system().name() - lang_path = AppLocation.get_directory(AppLocation.LanguageDir) + lang_path = str(AppLocation.get_directory(AppLocation.LanguageDir)) app_translator = QtCore.QTranslator() app_translator.load(language, lang_path) # A translator for buttons and other default strings provided by Qt. @@ -72,7 +72,7 @@ class LanguageManager(object): Find all available language files in this OpenLP install """ log.debug('Translation files: {files}'.format(files=AppLocation.get_directory(AppLocation.LanguageDir))) - trans_dir = QtCore.QDir(AppLocation.get_directory(AppLocation.LanguageDir)) + trans_dir = QtCore.QDir(str(AppLocation.get_directory(AppLocation.LanguageDir))) file_names = trans_dir.entryList(['*.qm'], QtCore.QDir.Files, QtCore.QDir.Name) # Remove qm files from the list which start with "qt". file_names = [file_ for file_ in file_names if not file_.startswith('qt')] diff --git a/openlp/core/common/versionchecker.py b/openlp/core/common/versionchecker.py index 25479884f..fcb6c7e1e 100644 --- a/openlp/core/common/versionchecker.py +++ b/openlp/core/common/versionchecker.py @@ -95,7 +95,7 @@ def get_application_version(): full_version = '{tag}-bzr{tree}'.format(tag=tag_version.strip(), tree=tree_revision.strip()) else: # We're not running the development version, let's use the file. - file_path = AppLocation.get_directory(AppLocation.VersionDir) + file_path = str(AppLocation.get_directory(AppLocation.VersionDir)) file_path = os.path.join(file_path, '.version') version_file = None try: diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py index 6ce8811d7..0021a41e7 100644 --- a/openlp/core/lib/db.py +++ b/openlp/core/lib/db.py @@ -274,9 +274,9 @@ def delete_database(plugin_name, db_file_name=None): :param db_file_name: The database file name. Defaults to None resulting in the plugin_name being used. """ if db_file_name: - db_file_path = os.path.join(AppLocation.get_section_data_path(plugin_name), db_file_name) + db_file_path = os.path.join(str(AppLocation.get_section_data_path(plugin_name)), db_file_name) else: - db_file_path = os.path.join(AppLocation.get_section_data_path(plugin_name), plugin_name) + db_file_path = os.path.join(str(AppLocation.get_section_data_path(plugin_name)), plugin_name) return delete_file(db_file_path) diff --git a/openlp/core/lib/path.py b/openlp/core/lib/path.py new file mode 100644 index 000000000..3d08df48e --- /dev/null +++ b/openlp/core/lib/path.py @@ -0,0 +1,61 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2017 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### + +from pathlib import Path + + +def path_to_str(path): + """ + A utility function to convert a Path object or NoneType to a string equivalent. + + :param path: The value to convert to a string + :type: pathlib.Path or None + + :return: An empty string if :param:`path` is None, else a string representation of the :param:`path` + :rtype: str + """ + if not isinstance(path, Path) and path is not None: + raise TypeError('parameter \'path\' must be of type Path or NoneType') + if path is None: + return '' + else: + return str(path) + + +def str_to_path(string): + """ + A utility function to convert a str object to a Path or NoneType. + + This function is of particular use because initating a Path object with an empty string causes the Path object to + point to the current working directory. + + :param string: The string to convert + :type string: str + + :return: None if :param:`string` is empty, or a Path object representation of :param:`string` + :rtype: pathlib.Path or None + """ + if not isinstance(string, str): + raise TypeError('parameter \'string\' must be of type str') + if string == '': + return None + return Path(string) \ No newline at end of file diff --git a/openlp/core/lib/pluginmanager.py b/openlp/core/lib/pluginmanager.py index bd2b7b9e1..5e4bf4e87 100644 --- a/openlp/core/lib/pluginmanager.py +++ b/openlp/core/lib/pluginmanager.py @@ -40,7 +40,7 @@ class PluginManager(RegistryMixin, OpenLPMixin, RegistryProperties): """ super(PluginManager, self).__init__(parent) self.log_info('Plugin manager Initialising') - self.base_path = os.path.abspath(AppLocation.get_directory(AppLocation.PluginsDir)) + self.base_path = os.path.abspath(str(AppLocation.get_directory(AppLocation.PluginsDir))) self.log_debug('Base path {path}'.format(path=self.base_path)) self.plugins = [] self.log_info('Plugin manager Initialised') diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index c8040aa58..3d561ea84 100644 --- a/openlp/core/lib/serviceitem.py +++ b/openlp/core/lib/serviceitem.py @@ -335,7 +335,7 @@ class ServiceItem(RegistryProperties): if image and not self.has_original_files and self.name == 'presentations': file_location = os.path.join(path, file_name) file_location_hash = md5_hash(file_location.encode('utf-8')) - image = os.path.join(AppLocation.get_section_data_path(self.name), 'thumbnails', + image = os.path.join(str(AppLocation.get_section_data_path(self.name)), 'thumbnails', file_location_hash, ntpath.basename(image)) self._raw_frames.append({'title': file_name, 'image': image, 'path': path, 'display_title': display_title, 'notes': notes}) diff --git a/openlp/core/lib/theme.py b/openlp/core/lib/theme.py index fb78f7443..78a2c240d 100644 --- a/openlp/core/lib/theme.py +++ b/openlp/core/lib/theme.py @@ -158,7 +158,7 @@ class Theme(object): Initialise the theme object. """ # basic theme object with defaults - json_dir = os.path.join(AppLocation.get_directory(AppLocation.AppDir), 'core', 'lib', 'json') + json_dir = os.path.join(str(AppLocation.get_directory(AppLocation.AppDir)), 'core', 'lib', 'json') json_file = os.path.join(json_dir, 'theme.json') jsn = get_text_file_string(json_file) jsn = json.loads(jsn) diff --git a/openlp/core/ui/advancedtab.py b/openlp/core/ui/advancedtab.py index 8d64bfecc..bf7294ca8 100644 --- a/openlp/core/ui/advancedtab.py +++ b/openlp/core/ui/advancedtab.py @@ -156,7 +156,7 @@ class AdvancedTab(SettingsTab): self.data_directory_new_label = QtWidgets.QLabel(self.data_directory_group_box) self.data_directory_new_label.setObjectName('data_directory_current_label') self.data_directory_path_edit = PathEdit(self.data_directory_group_box, path_type=PathType.Directories, - default_path=AppLocation.get_directory(AppLocation.DataDir)) + default_path=str(AppLocation.get_directory(AppLocation.DataDir))) self.data_directory_layout.addRow(self.data_directory_new_label, self.data_directory_path_edit) self.new_data_directory_has_files_label = QtWidgets.QLabel(self.data_directory_group_box) self.new_data_directory_has_files_label.setObjectName('new_data_directory_has_files_label') @@ -373,7 +373,7 @@ class AdvancedTab(SettingsTab): self.new_data_directory_has_files_label.hide() self.data_directory_cancel_button.hide() # Since data location can be changed, make sure the path is present. - self.data_directory_path_edit.path = AppLocation.get_data_path() + self.data_directory_path_edit.path = str(AppLocation.get_data_path()) # Don't allow data directory move if running portable. if settings.value('advanced/is portable'): self.data_directory_group_box.hide() @@ -497,7 +497,7 @@ class AdvancedTab(SettingsTab): 'closed.').format(path=new_data_path), defaultButton=QtWidgets.QMessageBox.No) if answer != QtWidgets.QMessageBox.Yes: - self.data_directory_path_edit.path = AppLocation.get_data_path() + self.data_directory_path_edit.path = str(AppLocation.get_data_path()) return # Check if data already exists here. self.check_data_overwrite(new_data_path) @@ -550,7 +550,7 @@ class AdvancedTab(SettingsTab): """ Cancel the data directory location change """ - self.data_directory_path_edit.path = AppLocation.get_data_path() + self.data_directory_path_edit.path = str(AppLocation.get_data_path()) self.data_directory_copy_check_box.setChecked(False) self.main_window.set_new_data_path(None) self.main_window.set_copy_data(False) diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index 0e95235fb..b6c90e2f3 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -554,8 +554,8 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties): """ # Build directories for downloads songs_destination = os.path.join(gettempdir(), 'openlp') - bibles_destination = AppLocation.get_section_data_path('bibles') - themes_destination = AppLocation.get_section_data_path('themes') + bibles_destination = str(AppLocation.get_section_data_path('bibles')) + themes_destination = str(AppLocation.get_section_data_path('themes')) missed_files = [] # Download songs for i in range(self.songs_list_widget.count()): diff --git a/openlp/core/ui/maindisplay.py b/openlp/core/ui/maindisplay.py index 23151395c..e846898b5 100644 --- a/openlp/core/ui/maindisplay.py +++ b/openlp/core/ui/maindisplay.py @@ -484,7 +484,7 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties): service_item = ServiceItem() service_item.title = 'webkit' service_item.processor = 'webkit' - path = os.path.join(AppLocation.get_section_data_path('themes'), + path = os.path.join(str(AppLocation.get_section_data_path('themes')), self.service_item.theme_data.theme_name) service_item.add_from_command(path, self.service_item.theme_data.background_filename, diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 1ea61b5a5..43da129b5 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -305,9 +305,9 @@ class Ui_MainWindow(object): # Give QT Extra Hint that this is an About Menu Item self.about_item.setMenuRole(QtWidgets.QAction.AboutRole) if is_win(): - self.local_help_file = os.path.join(AppLocation.get_directory(AppLocation.AppDir), 'OpenLP.chm') + self.local_help_file = os.path.join(str(AppLocation.get_directory(AppLocation.AppDir)), 'OpenLP.chm') elif is_macosx(): - self.local_help_file = os.path.join(AppLocation.get_directory(AppLocation.AppDir), + self.local_help_file = os.path.join(str(AppLocation.get_directory(AppLocation.AppDir)), '..', 'Resources', 'OpenLP.help') self.user_manual_item = create_action(main_window, 'userManualItem', icon=':/system/system_help_contents.png', can_shortcuts=True, category=UiStrings().Help, @@ -788,7 +788,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): """ Open data folder """ - path = AppLocation.get_data_path() + path = str(AppLocation.get_data_path()) QtGui.QDesktopServices.openUrl(QtCore.QUrl.fromLocalFile(path)) def on_update_theme_images(self): @@ -1438,7 +1438,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): settings = QtCore.QSettings() settings.setValue('advanced/data path', self.new_data_path) # Check if the new data path is our default. - if self.new_data_path == AppLocation.get_directory(AppLocation.DataDir): + if self.new_data_path == str(AppLocation.get_directory(AppLocation.DataDir)): settings.remove('advanced/data path') self.application.set_normal_cursor() diff --git a/openlp/core/ui/printserviceform.py b/openlp/core/ui/printserviceform.py index 5a26a001d..c9e1d5a8a 100644 --- a/openlp/core/ui/printserviceform.py +++ b/openlp/core/ui/printserviceform.py @@ -176,7 +176,7 @@ class PrintServiceForm(QtWidgets.QDialog, Ui_PrintServiceDialog, RegistryPropert html_data = self._add_element('html') self._add_element('head', parent=html_data) self._add_element('title', self.title_line_edit.text(), html_data.head) - css_path = os.path.join(AppLocation.get_data_path(), 'serviceprint', 'service_print.css') + css_path = os.path.join(str(AppLocation.get_data_path()), 'serviceprint', 'service_print.css') custom_css = get_text_file_string(css_path) if not custom_css: custom_css = DEFAULT_CSS diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index eb304b8c1..93fa7f31b 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -223,7 +223,7 @@ class Ui_ServiceManager(object): self.service_manager_list.itemExpanded.connect(self.expanded) # Last little bits of setting up self.service_theme = Settings().value(self.main_window.service_manager_settings_section + '/service theme') - self.service_path = AppLocation.get_section_data_path('servicemanager') + self.service_path = str(AppLocation.get_section_data_path('servicemanager')) # build the drag and drop context menu self.dnd_menu = QtWidgets.QMenu() self.new_action = self.dnd_menu.addAction(translate('OpenLP.ServiceManager', '&Add New Item')) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 969d676a8..00e9ed6e8 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -159,7 +159,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage """ Set up the theme path variables """ - self.path = AppLocation.get_section_data_path(self.settings_section) + self.path = str(AppLocation.get_section_data_path(self.settings_section)) check_directory_exists(self.path) self.thumb_path = os.path.join(self.path, 'thumbnails') check_directory_exists(self.thumb_path) @@ -445,7 +445,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage self.application.set_busy_cursor() files = AppLocation.get_files(self.settings_section, '.otz') for theme_file in files: - theme_file = os.path.join(self.path, theme_file) + theme_file = os.path.join(self.path, str(theme_file)) self.unzip_theme(theme_file, self.path) delete_file(theme_file) files = AppLocation.get_files(self.settings_section, '.png') @@ -470,6 +470,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage files.sort(key=lambda file_name: get_locale_key(str(file_name))) # now process the file list of png files for name in files: + name = str(name) # check to see file is in theme root directory theme = os.path.join(self.path, name) if os.path.exists(theme): diff --git a/openlp/plugins/bibles/forms/bibleimportform.py b/openlp/plugins/bibles/forms/bibleimportform.py index a6a2a4299..fd270fced 100644 --- a/openlp/plugins/bibles/forms/bibleimportform.py +++ b/openlp/plugins/bibles/forms/bibleimportform.py @@ -584,7 +584,7 @@ class BibleImportForm(OpenLPWizard): elif self.currentPage() == self.license_details_page: license_version = self.field('license_version') license_copyright = self.field('license_copyright') - path = AppLocation.get_section_data_path('bibles') + path = str(AppLocation.get_section_data_path('bibles')) if not license_version: critical_error_message_box( UiStrings().EmptyField, diff --git a/openlp/plugins/bibles/lib/db.py b/openlp/plugins/bibles/lib/db.py index 65460c92e..64744113b 100644 --- a/openlp/plugins/bibles/lib/db.py +++ b/openlp/plugins/bibles/lib/db.py @@ -470,7 +470,7 @@ class BiblesResourcesDB(QtCore.QObject, Manager): Return the cursor object. Instantiate one if it doesn't exist yet. """ if BiblesResourcesDB.cursor is None: - file_path = os.path.join(AppLocation.get_directory(AppLocation.PluginsDir), + file_path = os.path.join(str(AppLocation.get_directory(AppLocation.PluginsDir)), 'bibles', 'resources', 'bibles_resources.sqlite') conn = sqlite3.connect(file_path) BiblesResourcesDB.cursor = conn.cursor() @@ -759,7 +759,7 @@ class AlternativeBookNamesDB(QtCore.QObject, Manager): """ if AlternativeBookNamesDB.cursor is None: file_path = os.path.join( - AppLocation.get_directory(AppLocation.DataDir), 'bibles', 'alternative_book_names.sqlite') + str(AppLocation.get_directory(AppLocation.DataDir)), 'bibles', 'alternative_book_names.sqlite') if not os.path.exists(file_path): # create new DB, create table alternative_book_names AlternativeBookNamesDB.conn = sqlite3.connect(file_path) diff --git a/openlp/plugins/bibles/lib/manager.py b/openlp/plugins/bibles/lib/manager.py index 899d659b6..4b7b78e08 100644 --- a/openlp/plugins/bibles/lib/manager.py +++ b/openlp/plugins/bibles/lib/manager.py @@ -111,7 +111,7 @@ class BibleManager(OpenLPMixin, RegistryProperties): self.settings_section = 'bibles' self.web = 'Web' self.db_cache = None - self.path = AppLocation.get_section_data_path(self.settings_section) + self.path = str(AppLocation.get_section_data_path(self.settings_section)) self.proxy_name = Settings().value(self.settings_section + '/proxy name') self.suffix = '.sqlite' self.import_wizard = None @@ -124,7 +124,7 @@ class BibleManager(OpenLPMixin, RegistryProperties): of HTTPBible is loaded instead of the BibleDB class. """ log.debug('Reload bibles') - files = AppLocation.get_files(self.settings_section, self.suffix) + files = [str(file) for file in AppLocation.get_files(self.settings_section, self.suffix)] if 'alternative_book_names.sqlite' in files: files.remove('alternative_book_names.sqlite') log.debug('Bible Files {text}'.format(text=files)) diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index 97a9dd956..5856b9ccb 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -98,7 +98,7 @@ class ImageMediaItem(MediaManagerItem): self.list_view.setIconSize(QtCore.QSize(88, 50)) self.list_view.setIndentation(self.list_view.default_indentation) self.list_view.allow_internal_dnd = True - self.service_path = os.path.join(AppLocation.get_section_data_path(self.settings_section), 'thumbnails') + self.service_path = os.path.join(str(AppLocation.get_section_data_path(self.settings_section)), 'thumbnails') check_directory_exists(self.service_path) # Load images from the database self.load_full_list( diff --git a/openlp/plugins/media/lib/mediaitem.py b/openlp/plugins/media/lib/mediaitem.py index b52fa4134..f3f5495c7 100644 --- a/openlp/plugins/media/lib/mediaitem.py +++ b/openlp/plugins/media/lib/mediaitem.py @@ -300,7 +300,7 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties): Initialize media item. """ self.list_view.clear() - self.service_path = os.path.join(AppLocation.get_section_data_path(self.settings_section), 'thumbnails') + self.service_path = os.path.join(str(AppLocation.get_section_data_path(self.settings_section)), 'thumbnails') check_directory_exists(self.service_path) self.load_list(Settings().value(self.settings_section + '/media files')) self.rebuild_players() diff --git a/openlp/plugins/media/mediaplugin.py b/openlp/plugins/media/mediaplugin.py index 8bd7882dd..6a9ff5bae 100644 --- a/openlp/plugins/media/mediaplugin.py +++ b/openlp/plugins/media/mediaplugin.py @@ -75,7 +75,7 @@ class MediaPlugin(Plugin): exists = process_check_binary('mediainfo') # If mediainfo is not in the path, try to find it in the application folder if not exists: - exists = process_check_binary(os.path.join(AppLocation.get_directory(AppLocation.AppDir), 'mediainfo')) + exists = process_check_binary(os.path.join(str(AppLocation.get_directory(AppLocation.AppDir)), 'mediainfo')) return exists def app_startup(self): diff --git a/openlp/plugins/presentations/lib/pdfcontroller.py b/openlp/plugins/presentations/lib/pdfcontroller.py index d59fd83a4..128d940d9 100644 --- a/openlp/plugins/presentations/lib/pdfcontroller.py +++ b/openlp/plugins/presentations/lib/pdfcontroller.py @@ -122,10 +122,10 @@ class PdfController(PresentationController): self.mutoolbin = pdf_program else: # Fallback to autodetection - application_path = AppLocation.get_directory(AppLocation.AppDir) + application_path = str(AppLocation.get_directory(AppLocation.AppDir)) if is_win(): # for windows we only accept mudraw.exe or mutool.exe in the base folder - application_path = AppLocation.get_directory(AppLocation.AppDir) + application_path = str(AppLocation.get_directory(AppLocation.AppDir)) if os.path.isfile(os.path.join(application_path, 'mudraw.exe')): self.mudrawbin = os.path.join(application_path, 'mudraw.exe') elif os.path.isfile(os.path.join(application_path, 'mutool.exe')): @@ -142,7 +142,7 @@ class PdfController(PresentationController): self.gsbin = which('gs') # Last option: check if mudraw or mutool is placed in OpenLP base folder if not self.mudrawbin and not self.mutoolbin and not self.gsbin: - application_path = AppLocation.get_directory(AppLocation.AppDir) + application_path = str(AppLocation.get_directory(AppLocation.AppDir)) if os.path.isfile(os.path.join(application_path, 'mudraw')): self.mudrawbin = os.path.join(application_path, 'mudraw') elif os.path.isfile(os.path.join(application_path, 'mutool')): @@ -199,8 +199,8 @@ class PdfDocument(PresentationDocument): :return: The resolution dpi to be used. """ # Use a postscript script to get size of the pdf. It is assumed that all pages have same size - gs_resolution_script = AppLocation.get_directory( - AppLocation.PluginsDir) + '/presentations/lib/ghostscript_get_resolution.ps' + gs_resolution_script = str(AppLocation.get_directory( + AppLocation.PluginsDir)) + '/presentations/lib/ghostscript_get_resolution.ps' # Run the script on the pdf to get the size runlog = [] try: diff --git a/openlp/plugins/presentations/lib/pptviewcontroller.py b/openlp/plugins/presentations/lib/pptviewcontroller.py index 0c33d1559..645bef053 100644 --- a/openlp/plugins/presentations/lib/pptviewcontroller.py +++ b/openlp/plugins/presentations/lib/pptviewcontroller.py @@ -85,7 +85,7 @@ class PptviewController(PresentationController): if self.process: return log.debug('start PPTView') - dll_path = os.path.join(AppLocation.get_directory(AppLocation.AppDir), + dll_path = os.path.join(str(AppLocation.get_directory(AppLocation.AppDir)), 'plugins', 'presentations', 'lib', 'pptviewlib', 'pptviewlib.dll') self.process = cdll.LoadLibrary(dll_path) if log.isEnabledFor(logging.DEBUG): diff --git a/openlp/plugins/presentations/lib/presentationcontroller.py b/openlp/plugins/presentations/lib/presentationcontroller.py index 74028c983..ce0e5dd8b 100644 --- a/openlp/plugins/presentations/lib/presentationcontroller.py +++ b/openlp/plugins/presentations/lib/presentationcontroller.py @@ -415,8 +415,9 @@ class PresentationController(object): self.document_class = document_class self.settings_section = self.plugin.settings_section self.available = None - self.temp_folder = os.path.join(AppLocation.get_section_data_path(self.settings_section), name) - self.thumbnail_folder = os.path.join(AppLocation.get_section_data_path(self.settings_section), 'thumbnails') + self.temp_folder = os.path.join(str(AppLocation.get_section_data_path(self.settings_section)), name) + self.thumbnail_folder = os.path.join( + str(AppLocation.get_section_data_path(self.settings_section)), 'thumbnails') self.thumbnail_prefix = 'slide' check_directory_exists(self.thumbnail_folder) check_directory_exists(self.temp_folder) diff --git a/openlp/plugins/remotes/lib/httprouter.py b/openlp/plugins/remotes/lib/httprouter.py index b3b45b03f..fcaeb6f21 100644 --- a/openlp/plugins/remotes/lib/httprouter.py +++ b/openlp/plugins/remotes/lib/httprouter.py @@ -171,8 +171,8 @@ class HttpRouter(RegistryProperties): ] self.settings_section = 'remotes' self.translate() - self.html_dir = os.path.join(AppLocation.get_directory(AppLocation.PluginsDir), 'remotes', 'html') - self.config_dir = os.path.join(AppLocation.get_data_path(), 'stages') + self.html_dir = os.path.join(str(AppLocation.get_directory(AppLocation.PluginsDir)), 'remotes', 'html') + self.config_dir = os.path.join(str(AppLocation.get_data_path()), 'stages') def do_post_processor(self): """ @@ -456,7 +456,7 @@ class HttpRouter(RegistryProperties): if controller_name in supported_controllers: full_path = urllib.parse.unquote(file_name) if '..' not in full_path: # no hacking please - full_path = os.path.normpath(os.path.join(AppLocation.get_section_data_path(controller_name), + full_path = os.path.normpath(os.path.join(str(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) @@ -565,7 +565,7 @@ class HttpRouter(RegistryProperties): elif current_item.is_image() and not frame.get('image', '') and Settings().value('remotes/thumbnails'): item['tag'] = str(index + 1) thumbnail_path = os.path.join('images', 'thumbnails', frame['title']) - full_thumbnail_path = os.path.join(AppLocation.get_data_path(), thumbnail_path) + full_thumbnail_path = os.path.join(str(AppLocation.get_data_path()), thumbnail_path) # Create thumbnail if it doesn't exists if not os.path.exists(full_thumbnail_path): create_thumb(current_item.get_frame_path(index), full_thumbnail_path, False) @@ -582,7 +582,7 @@ class HttpRouter(RegistryProperties): if current_item.is_capable(ItemCapabilities.HasThumbnails) and \ Settings().value('remotes/thumbnails'): # If the file is under our app directory tree send the portion after the match - data_path = AppLocation.get_data_path() + data_path = str(AppLocation.get_data_path()) if frame['image'][0:len(data_path)] == data_path: item['img'] = urllib.request.pathname2url(frame['image'][len(data_path):]) item['text'] = str(frame['title']) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index dd965f733..61d5b1ddd 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -1065,7 +1065,7 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): self.manager.save_object(self.song) audio_files = [a.file_name for a in self.song.media_files] log.debug(audio_files) - save_path = os.path.join(AppLocation.get_section_data_path(self.media_item.plugin.name), 'audio', + save_path = os.path.join(str(AppLocation.get_section_data_path(self.media_item.plugin.name)), 'audio', str(self.song.id)) check_directory_exists(save_path) self.song.media_files = [] diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py index 6798a39c0..b342edc19 100644 --- a/openlp/plugins/songs/lib/__init__.py +++ b/openlp/plugins/songs/lib/__init__.py @@ -538,7 +538,7 @@ def delete_song(song_id, song_plugin): except OSError: log.exception('Could not remove file: {name}'.format(name=media_file.file_name)) try: - save_path = os.path.join(AppLocation.get_section_data_path(song_plugin.name), 'audio', str(song_id)) + save_path = os.path.join(str(AppLocation.get_section_data_path(song_plugin.name)), 'audio', str(song_id)) if os.path.exists(save_path): os.rmdir(save_path) except OSError: diff --git a/openlp/plugins/songs/lib/importers/songimport.py b/openlp/plugins/songs/lib/importers/songimport.py index 070919c44..0b82d01af 100644 --- a/openlp/plugins/songs/lib/importers/songimport.py +++ b/openlp/plugins/songs/lib/importers/songimport.py @@ -421,7 +421,7 @@ class SongImport(QtCore.QObject): :param filename: The file to copy. """ if not hasattr(self, 'save_path'): - self.save_path = os.path.join(AppLocation.get_section_data_path(self.import_wizard.plugin.name), + self.save_path = os.path.join(str(AppLocation.get_section_data_path(self.import_wizard.plugin.name)), 'audio', str(song_id)) check_directory_exists(self.save_path) if not filename.startswith(self.save_path): diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 6d2847444..f8260a748 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -88,9 +88,9 @@ class SongMediaItem(MediaManagerItem): song.media_files = [] for i, bga in enumerate(item.background_audio): dest_file = os.path.join( - AppLocation.get_section_data_path(self.plugin.name), 'audio', str(song.id), os.path.split(bga)[1]) + str(AppLocation.get_section_data_path(self.plugin.name)), 'audio', str(song.id), os.path.split(bga)[1]) check_directory_exists(os.path.split(dest_file)[0]) - shutil.copyfile(os.path.join(AppLocation.get_section_data_path('servicemanager'), bga), dest_file) + shutil.copyfile(os.path.join(str(AppLocation.get_section_data_path('servicemanager')), bga), dest_file) song.media_files.append(MediaFile.populate(weight=i, file_name=dest_file)) self.plugin.manager.save_object(song, True) @@ -533,7 +533,7 @@ class SongMediaItem(MediaManagerItem): 'copy', 'For song cloning')) # Copy audio files from the old to the new song if len(old_song.media_files) > 0: - save_path = os.path.join(AppLocation.get_section_data_path(self.plugin.name), 'audio', str(new_song.id)) + save_path = os.path.join(str(AppLocation.get_section_data_path(self.plugin.name)), 'audio', str(new_song.id)) check_directory_exists(save_path) for media_file in old_song.media_files: new_media_file_name = os.path.join(save_path, os.path.basename(media_file.file_name)) diff --git a/tests/functional/openlp_core_common/test_applocation.py b/tests/functional/openlp_core_common/test_applocation.py index bb8d58b9a..34ff861df 100644 --- a/tests/functional/openlp_core_common/test_applocation.py +++ b/tests/functional/openlp_core_common/test_applocation.py @@ -24,8 +24,9 @@ Functional tests to test the AppLocation class and related methods. """ import copy import os +from pathlib import Path from unittest import TestCase -from unittest.mock import patch +from unittest.mock import MagicMock, patch from openlp.core.common import AppLocation, get_frozen_path @@ -64,56 +65,51 @@ class TestAppLocation(TestCase): """ Test the AppLocation.get_data_path() method when a custom location is set in the settings """ - with patch('openlp.core.common.applocation.Settings') as mocked_class,\ - patch('openlp.core.common.applocation.os') as mocked_os: - # GIVEN: A mocked out Settings class which returns a custom data location - mocked_settings = mocked_class.return_value - mocked_settings.contains.return_value = True - mocked_settings.value.return_value.toString.return_value = 'custom/dir' - mocked_os.path.normpath.return_value = 'custom/dir' + # GIVEN: A mocked out Settings class which returns a custom data location + mocked_settings_instance = MagicMock( + **{'contains.return_value': True, 'value.return_value': Path('custom', 'dir')}) + with patch('openlp.core.common.applocation.Settings', return_value=mocked_settings_instance): # WHEN: we call AppLocation.get_data_path() data_path = AppLocation.get_data_path() # THEN: the mocked Settings methods were called and the value returned was our set up value - mocked_settings.contains.assert_called_with('advanced/data path') - mocked_settings.value.assert_called_with('advanced/data path') - self.assertEqual('custom/dir', data_path, 'Result should be "custom/dir"') + mocked_settings_instance.contains.assert_called_with('advanced/data path') + mocked_settings_instance.value.assert_called_with('advanced/data path') + self.assertEqual(Path('custom', 'dir'), data_path, 'Result should be "custom/dir"') def test_get_files_no_section_no_extension(self): """ Test the AppLocation.get_files() method with no parameters passed. """ - with patch('openlp.core.common.AppLocation.get_data_path') as mocked_get_data_path, \ - patch('openlp.core.common.applocation.os.listdir') as mocked_listdir: - # GIVEN: Our mocked modules/methods. - mocked_get_data_path.return_value = 'test/dir' - mocked_listdir.return_value = copy.deepcopy(FILE_LIST) + # GIVEN: Our mocked modules/methods. + with patch.object(Path, 'glob', return_value=[Path('/dir/file5.mp3'), Path('/dir/file6.mp3')]) as mocked_glob, \ + patch('openlp.core.common.AppLocation.get_data_path', return_value=Path('/dir')): # When: Get the list of files. result = AppLocation.get_files() + # Then: Check if the section parameter was used correctly, and the glob argument was passed. + mocked_glob.assert_called_once_with('*') + # Then: check if the file lists are identical. - self.assertListEqual(FILE_LIST, result, 'The file lists should be identical.') + self.assertListEqual([Path('file5.mp3'), Path('file6.mp3')], result, 'The file lists should be identical.') def test_get_files(self): """ Test the AppLocation.get_files() method with all parameters passed. """ - with patch('openlp.core.common.AppLocation.get_data_path') as mocked_get_data_path, \ - patch('openlp.core.common.applocation.os.listdir') as mocked_listdir: - # GIVEN: Our mocked modules/methods. - mocked_get_data_path.return_value = os.path.join('test', 'dir') - mocked_listdir.return_value = copy.deepcopy(FILE_LIST) + # GIVEN: Our mocked modules/methods. + with patch.object(Path, 'glob', return_value=[Path('/dir/section/file5.mp3'), Path('/dir/section/file6.mp3')]) \ + as mocked_glob, \ + patch('openlp.core.common.AppLocation.get_data_path', return_value=Path('/dir')): # When: Get the list of files. result = AppLocation.get_files('section', '.mp3') - # Then: Check if the section parameter was used correctly. - mocked_listdir.assert_called_with(os.path.join('test', 'dir', 'section')) - - # Then: check if the file lists are identical. - self.assertListEqual(['file5.mp3', 'file6.mp3'], result, 'The file lists should be identical.') + # Then: The section parameter was used correctly, and the glob argument was passed.. + mocked_glob.assert_called_once_with('*.mp3') + self.assertListEqual([Path('file5.mp3'), Path('file6.mp3')], result, 'The file lists should be identical.') def test_get_section_data_path(self): """ @@ -122,7 +118,7 @@ class TestAppLocation(TestCase): with patch('openlp.core.common.AppLocation.get_data_path') as mocked_get_data_path, \ patch('openlp.core.common.applocation.check_directory_exists') as mocked_check_directory_exists: # GIVEN: A mocked out AppLocation.get_data_path() - mocked_get_data_path.return_value = os.path.join('test', 'dir') + mocked_get_data_path.return_value = Path('test', 'dir') mocked_check_directory_exists.return_value = True # WHEN: we call AppLocation.get_data_path() @@ -130,7 +126,7 @@ class TestAppLocation(TestCase): # THEN: check that all the correct methods were called, and the result is correct mocked_check_directory_exists.assert_called_with(os.path.join('test', 'dir', 'section')) - self.assertEqual(os.path.join('test', 'dir', 'section'), data_path, 'Result should be "test/dir/section"') + self.assertEqual(Path('test', 'dir', 'section'), data_path, 'Result should be "test/dir/section"') def test_get_directory_for_app_dir(self): """ @@ -138,13 +134,13 @@ class TestAppLocation(TestCase): """ # GIVEN: A mocked out _get_frozen_path function with patch('openlp.core.common.applocation.get_frozen_path') as mocked_get_frozen_path: - mocked_get_frozen_path.return_value = os.path.join('app', 'dir') + mocked_get_frozen_path.return_value = Path('app', 'dir') # WHEN: We call AppLocation.get_directory directory = AppLocation.get_directory(AppLocation.AppDir) # THEN: check that the correct directory is returned - self.assertEqual(os.path.join('app', 'dir'), directory, 'Directory should be "app/dir"') + self.assertEqual(Path('app', 'dir'), directory, 'Directory should be "app/dir"') def test_get_directory_for_plugins_dir(self): """ @@ -157,7 +153,7 @@ class TestAppLocation(TestCase): patch('openlp.core.common.applocation.sys') as mocked_sys: mocked_abspath.return_value = os.path.join('plugins', 'dir') mocked_split.return_value = ['openlp'] - mocked_get_frozen_path.return_value = os.path.join('plugins', 'dir') + mocked_get_frozen_path.return_value = Path('dir') mocked_sys.frozen = 1 mocked_sys.argv = ['openlp'] @@ -165,7 +161,7 @@ class TestAppLocation(TestCase): directory = AppLocation.get_directory(AppLocation.PluginsDir) # THEN: The correct directory should be returned - self.assertEqual(os.path.join('plugins', 'dir'), directory, 'Directory should be "plugins/dir"') + self.assertEqual(Path('dir', 'plugins'), directory, 'Directory should be "dir/plugins"') def test_get_frozen_path_in_unfrozen_app(self): """ diff --git a/tests/functional/openlp_core_common/test_common.py b/tests/functional/openlp_core_common/test_common.py index e70a82328..78072ddd0 100644 --- a/tests/functional/openlp_core_common/test_common.py +++ b/tests/functional/openlp_core_common/test_common.py @@ -79,7 +79,7 @@ class TestCommonFunctions(TestCase): Test the `extension_loader` function when no files are found """ # GIVEN: A mocked `Path.glob` method which does not match any files - with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \ + with patch('openlp.core.common.AppLocation.get_directory', return_value=Path('/', 'app', 'dir', 'openlp')), \ patch.object(common.Path, 'glob', return_value=[]), \ patch('openlp.core.common.importlib.import_module') as mocked_import_module: @@ -94,11 +94,12 @@ class TestCommonFunctions(TestCase): Test the `extension_loader` function when it successfully finds and loads some files """ # GIVEN: A mocked `Path.glob` method which returns a list of files - with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \ - patch.object(common.Path, 'glob', return_value=[Path('/app/dir/openlp/import_dir/file1.py'), - Path('/app/dir/openlp/import_dir/file2.py'), - Path('/app/dir/openlp/import_dir/file3.py'), - Path('/app/dir/openlp/import_dir/file4.py')]), \ + with patch('openlp.core.common.AppLocation.get_directory', return_value=Path('/', 'app', 'dir', 'openlp')), \ + patch.object(common.Path, 'glob', return_value=[ + Path('/', 'app', 'dir', 'openlp', 'import_dir', 'file1.py'), + Path('/', 'app', 'dir', 'openlp', 'import_dir', 'file2.py'), + Path('/', 'app', 'dir', 'openlp', 'import_dir', 'file3.py'), + Path('/', 'app', 'dir', 'openlp', 'import_dir', 'file4.py')]), \ patch('openlp.core.common.importlib.import_module') as mocked_import_module: # WHEN: Calling `extension_loader` with a list of files to exclude @@ -113,8 +114,9 @@ class TestCommonFunctions(TestCase): Test the `extension_loader` function when `SourceFileLoader` raises a `ImportError` """ # GIVEN: A mocked `import_module` which raises an `ImportError` - with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \ - patch.object(common.Path, 'glob', return_value=[Path('/app/dir/openlp/import_dir/file1.py')]), \ + with patch('openlp.core.common.AppLocation.get_directory', return_value=Path('/', 'app', 'dir', 'openlp')), \ + patch.object(common.Path, 'glob', return_value=[ + Path('/', 'app', 'dir', 'openlp', 'import_dir', 'file1.py')]), \ patch('openlp.core.common.importlib.import_module', side_effect=ImportError()), \ patch('openlp.core.common.log') as mocked_logger: @@ -129,8 +131,9 @@ class TestCommonFunctions(TestCase): Test the `extension_loader` function when `import_module` raises a `ImportError` """ # GIVEN: A mocked `SourceFileLoader` which raises an `OSError` - with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \ - patch.object(common.Path, 'glob', return_value=[Path('/app/dir/openlp/import_dir/file1.py')]), \ + with patch('openlp.core.common.AppLocation.get_directory', return_value=Path('/', 'app', 'dir', 'openlp')), \ + patch.object(common.Path, 'glob', return_value=[ + Path('/', 'app', 'dir', 'openlp', 'import_dir', 'file1.py')]), \ patch('openlp.core.common.importlib.import_module', side_effect=OSError()), \ patch('openlp.core.common.log') as mocked_logger: diff --git a/tests/functional/openlp_core_lib/test_path.py b/tests/functional/openlp_core_lib/test_path.py new file mode 100644 index 000000000..9af3673ff --- /dev/null +++ b/tests/functional/openlp_core_lib/test_path.py @@ -0,0 +1,88 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2017 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +Package to test the openlp.core.lib.path package. +""" +import os +from pathlib import Path +from unittest import TestCase + +from openlp.core.lib.path import path_to_str, str_to_path + + +class TestPath(TestCase): + """ + Tests for the :mod:`openlp.core.lib.path` module + """ + + def test_path_to_str_type_error(self): + """ + Test that `path_to_str` raises a type error when called with an invalid type + """ + # GIVEN: The `path_to_str` function + # WHEN: Calling `path_to_str` with an invalid Type + # THEN: A TypeError should have been raised + with self.assertRaises(TypeError): + path_to_str(str()) + + def test_path_to_str_none(self): + """ + Test that `path_to_str` correctly converts the path parameter when passed with None + """ + # GIVEN: The `path_to_str` function + # WHEN: Calling the `path_to_str` function with None + result = path_to_str(None) + + # THEN: `path_to_str` should return an empty string + self.assertEqual(result, '') + + def test_path_to_str_path_object(self): + """ + Test that `path_to_str` correctly converts the path parameter when passed a Path object + """ + # GIVEN: The `path_to_str` function + # WHEN: Calling the `path_to_str` function with a Path object + result = path_to_str(Path('test/path')) + + # THEN: `path_to_str` should return a string representation of the Path object + self.assertEqual(result, os.path.join('test', 'path')) + + def test_str_to_path_type_error(self): + """ + Test that `str_to_path` raises a type error when called with an invalid type + """ + # GIVEN: The `str_to_path` function + # WHEN: Calling `str_to_path` with an invalid Type + # THEN: A TypeError should have been raised + with self.assertRaises(TypeError): + str_to_path(Path()) + + def test_str_to_path_empty_str(self): + """ + Test that `str_to_path` correctly converts the string parameter when passed with and empty string + """ + # GIVEN: The `str_to_path` function + # WHEN: Calling the `str_to_path` function with None + result = str_to_path('') + + # THEN: `path_to_str` should return None + self.assertEqual(result, None) \ No newline at end of file From 17fb6ba835d88774407f1438c20679187dfb7005 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Tue, 1 Aug 2017 22:06:52 +0100 Subject: [PATCH 16/18] Convert applocation over to pathlab. Add utility functions for future use. --- openlp/core/common/applocation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/common/applocation.py b/openlp/core/common/applocation.py index ad81e36ad..a6c137f01 100644 --- a/openlp/core/common/applocation.py +++ b/openlp/core/common/applocation.py @@ -120,7 +120,7 @@ class AppLocation(object): path = path / section try: file_paths = path.glob('*' + extension) - return [file_path.relative_to(path) for file_path in file_paths] # TODO: Could/should this be an iterator? + return [file_path.relative_to(path) for file_path in file_paths] except OSError: return [] From e787896931de4f1deec91630b4aab06b349e527e Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Wed, 2 Aug 2017 07:09:38 +0100 Subject: [PATCH 17/18] test fixes --- openlp/core/common/applocation.py | 12 ++++++------ openlp/core/lib/path.py | 2 +- openlp/plugins/bibles/lib/importers/http.py | 2 +- openlp/plugins/songs/lib/mediaitem.py | 3 ++- tests/functional/openlp_core_lib/test_path.py | 2 +- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/openlp/core/common/applocation.py b/openlp/core/common/applocation.py index a6c137f01..d361c54d4 100644 --- a/openlp/core/common/applocation.py +++ b/openlp/core/common/applocation.py @@ -87,7 +87,7 @@ class AppLocation(object): def get_data_path(): """ Return the path OpenLP stores all its data under. - + :return: The data path to use. :rtype: pathlib.Path """ @@ -103,15 +103,15 @@ class AppLocation(object): def get_files(section=None, extension=''): """ Get a list of files from the data files path. - - :param section: Defaults to *None*. The section of code getting the files - used to load from a section's data + + :param section: Defaults to *None*. The section of code getting the files - used to load from a section's data subdirectory. :type section: None | str :param extension: Defaults to ''. The extension to search for. For example:: '.png' :type extension: str - + :return: List of files found. :rtype: list[pathlib.Path] """ @@ -141,10 +141,10 @@ class AppLocation(object): def _get_os_dir_path(dir_type): """ Return a path based on which OS and environment we are running in. - + :param dir_type: AppLocation Enum of the requested path type :type dir_type: AppLocation Enum - + :return: The requested path :rtype: pathlib.Path """ diff --git a/openlp/core/lib/path.py b/openlp/core/lib/path.py index 3d08df48e..12bef802d 100644 --- a/openlp/core/lib/path.py +++ b/openlp/core/lib/path.py @@ -58,4 +58,4 @@ def str_to_path(string): raise TypeError('parameter \'string\' must be of type str') if string == '': return None - return Path(string) \ No newline at end of file + return Path(string) diff --git a/openlp/plugins/bibles/lib/importers/http.py b/openlp/plugins/bibles/lib/importers/http.py index 4f032784d..6abb85d35 100644 --- a/openlp/plugins/bibles/lib/importers/http.py +++ b/openlp/plugins/bibles/lib/importers/http.py @@ -773,7 +773,7 @@ def get_soup_for_bible_ref(reference_url, header=None, pre_parse_regex=None, pre return None try: page = get_web_page(reference_url, header, True) - except: + except Exception as e: page = None if not page: send_error_message('download') diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index f8260a748..5a27b3033 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -533,7 +533,8 @@ class SongMediaItem(MediaManagerItem): 'copy', 'For song cloning')) # Copy audio files from the old to the new song if len(old_song.media_files) > 0: - save_path = os.path.join(str(AppLocation.get_section_data_path(self.plugin.name)), 'audio', str(new_song.id)) + save_path = os.path.join( + str(AppLocation.get_section_data_path(self.plugin.name)), 'audio', str(new_song.id)) check_directory_exists(save_path) for media_file in old_song.media_files: new_media_file_name = os.path.join(save_path, os.path.basename(media_file.file_name)) diff --git a/tests/functional/openlp_core_lib/test_path.py b/tests/functional/openlp_core_lib/test_path.py index 9af3673ff..cf93e634c 100644 --- a/tests/functional/openlp_core_lib/test_path.py +++ b/tests/functional/openlp_core_lib/test_path.py @@ -85,4 +85,4 @@ class TestPath(TestCase): result = str_to_path('') # THEN: `path_to_str` should return None - self.assertEqual(result, None) \ No newline at end of file + self.assertEqual(result, None) From d895365ff814a5d1f139fc3344a1b9444455414e Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Wed, 2 Aug 2017 07:10:26 +0100 Subject: [PATCH 18/18] Bible gateway fix --- openlp/plugins/bibles/lib/importers/http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/bibles/lib/importers/http.py b/openlp/plugins/bibles/lib/importers/http.py index 6abb85d35..4ed5fa844 100644 --- a/openlp/plugins/bibles/lib/importers/http.py +++ b/openlp/plugins/bibles/lib/importers/http.py @@ -325,7 +325,7 @@ class BGExtract(RegistryProperties): returns a list in the form [(biblename, biblekey, language_code)] """ log.debug('BGExtract.get_bibles_from_http') - bible_url = 'https://biblegateway.com/versions/' + bible_url = 'https://www.biblegateway.com/versions/' soup = get_soup_for_bible_ref(bible_url) if not soup: return None