Merge branch 'logging_fixes' into 'master'

Logging and Powerpoint fixes

See merge request openlp/openlp!177

openlp/core/api/versions/v2/controller.py

@@ -18,6 +18,7 @@
 # You should have received a copy of the GNU General Public License      #
 # along with this program.  If not, see <https://www.gnu.org/licenses/>. #
 ##########################################################################
+import logging
 import os
 import urllib.request
 from pathlib import Path
@@ -32,6 +33,7 @@ from openlp.core.lib.serviceitem import ItemCapabilities
 from flask import jsonify, request, abort, Blueprint
 
 controller_views = Blueprint('controller', __name__)
+log = logging.getLogger(__name__)
 
 
 @controller_views.route('/live-item')
@@ -85,6 +87,7 @@ def controller_text_api():
 def controller_set():
     data = request.json
     if not data:
+        log.error('Missing request data')
         abort(400)
     num = data.get('id', -1)
     Registry().get('live_controller').slidecontroller_live_set.emit([num])
@@ -97,9 +100,11 @@ def controller_direction():
     ALLOWED_ACTIONS = ['next', 'previous']
     data = request.json
     if not data:
+        log.error('Missing request data')
         abort(400)
     action = data.get('action', '').lower()
     if action not in ALLOWED_ACTIONS:
+        log.error('Invalid action passed ' + action)
         abort(400)
     getattr(Registry().get('live_controller'), 'slidecontroller_live_{action}'.
             format(action=action)).emit()
@@ -110,14 +115,12 @@ def controller_direction():
 @login_required
 def get_theme_level():
     theme_level = Registry().get('settings').value('themes/theme level')
-
     if theme_level == ThemeLevel.Global:
         theme_level = 'global'
     elif theme_level == ThemeLevel.Service:
         theme_level = 'service'
     elif theme_level == ThemeLevel.Song:
         theme_level = 'song'
-
     return jsonify(theme_level)
 
 
@@ -126,14 +129,14 @@ def get_theme_level():
 def set_theme_level():
     data = request.json
     if not data:
+        log.error('Missing request data')
         abort(400)
-
     theme_level = ''
     try:
         theme_level = str(data.get("level"))
     except ValueError:
+        log.error('Invalid data passed ' + theme_level)
         abort(400)
-
     if theme_level == 'global':
         Registry().get('settings').setValue('themes/theme level', 1)
     elif theme_level == 'service':
@@ -141,8 +144,8 @@ def set_theme_level():
     elif theme_level == 'song':
         Registry().get('settings').setValue('theme/theme level', 3)
     else:
+        log.error('Unsupported data passed ' + theme_level)
         abort(400)
-
     return '', 204
 
 
@@ -152,12 +155,10 @@ def get_themes():
     theme_level = Registry().get('settings').value('themes/theme level')
     theme_list = []
     current_theme = ''
-
     if theme_level == ThemeLevel.Global:
         current_theme = Registry().get('theme_manager').global_theme
     if theme_level == ThemeLevel.Service:
         current_theme = Registry().get('service_manager').service_theme
-
     # Gets and appends theme list
     themes = Registry().execute('get_theme_names')
     try:
@@ -170,8 +171,8 @@ def get_themes():
             if i["name"] == current_theme:
                 i["selected"] = True
     except IndexError:
+        log.error('Missing theme passed ' + str(themes))
         pass
-
     return jsonify(theme_list)
 
 
@@ -181,14 +182,14 @@ def set_theme():
     data = request.json
     theme = ''
     theme_level = Registry().get('settings').value('themes/theme level')
-
     if not data:
+        log.error('Missing request data')
         abort(400)
     try:
         theme = str(data.get('theme'))
     except ValueError:
+        log.error('Invalid data passed ' + theme)
         abort(400)
-
     if theme_level == ThemeLevel.Global:
         Registry().get('settings').setValue('themes/global theme', theme)
         Registry().execute('theme_update_global')
@@ -196,6 +197,6 @@ def set_theme():
         Registry().get('settings').setValue('servicemanager/service theme', theme)
         Registry().execute('theme_update_service')
     elif theme_level == ThemeLevel.Song:
+        log.error('Unimplemented method')
         return '', 501
-
     return '', 204

openlp/core/api/versions/v2/core.py

@@ -18,6 +18,7 @@
 # You should have received a copy of the GNU General Public License      #
 # along with this program.  If not, see <https://www.gnu.org/licenses/>. #
 ##########################################################################
+import logging
 from openlp.core.api.lib import login_required
 from openlp.core.common.registry import Registry
 from openlp.core.lib import image_to_byte
@@ -27,6 +28,7 @@ from openlp.core.state import State
 from flask import jsonify, request, abort, Blueprint
 
 core = Blueprint('core', __name__)
+log = logging.getLogger(__name__)
 
 
 @core.route('/poll')
@@ -40,11 +42,11 @@ def toggle_display():
     ALLOWED_ACTIONS = ['hide', 'show', 'blank', 'theme', 'desktop']
     data = request.json
     if not data:
+        log.error('Missing request data')
         abort(400)
     display = data.get('display', '').lower()
     if display not in ALLOWED_ACTIONS:
         abort(400)
-
     Registry().get('live_controller').slidecontroller_toggle_display.emit(display)
     return '', 204
 
@@ -70,6 +72,7 @@ def system_information():
 def login():
     data = request.json
     if not data:
+        log.error('Missing request data')
         abort(400)
     username = data.get('username', '')
     password = data.get('password', '')
@@ -77,6 +80,7 @@ def login():
             password == Registry().get('settings_thread').value('api/password'):
         return jsonify({'token': Registry().get('authentication_token')})
     else:
+        log.error('Unauthorised Request for ' + username)
         return '', 401
 
 

openlp/core/app.py

@@ -31,6 +31,7 @@ import os
 import sys
 import time
 from datetime import datetime
+from logging.handlers import RotatingFileHandler
 from pathlib import Path
 from shutil import copytree
 from traceback import format_exception
@@ -303,7 +304,7 @@ def set_up_logging(log_path):
     """
     create_paths(log_path, do_not_log=True)
     file_path = log_path / 'openlp.log'
-    logfile = logging.FileHandler(file_path, 'w', encoding='UTF-8')
+    logfile = RotatingFileHandler(str(file_path), 'w', encoding='UTF-8', backupCount=2)
     logfile.setFormatter(logging.Formatter('%(asctime)s %(threadName)s %(name)-55s %(levelname)-8s %(message)s'))
     log.addHandler(logfile)
     if log.isEnabledFor(logging.DEBUG):

openlp/core/ui/mainwindow.py

@@ -1056,6 +1056,11 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert
                     event.ignore()
             else:
                 event.accept()
+        # Included here as it is only needed to help force the logging rollover not for general logging use.
+        # Rollover require as when WebSocket exits having been used it will destroy the application log on exit.
+        import logging
+        log_man = logging.getLogger()
+        log_man.handlers[0].doRollover()
         if event.isAccepted():
             # Wait for all the threads to complete
             self._wait_for_threads()

openlp/plugins/presentations/lib/mediaitem.py

@@ -87,8 +87,8 @@ class PresentationMediaItem(MediaManagerItem):
                 for file_type in file_types:
                     if file_type not in file_type_string:
                         file_type_string += '*.{text} '.format(text=file_type)
-                file_type_string = file_type_string.strip()
+        file_type_string = file_type_string.strip()
-                self.service_manager.supported_suffixes(file_type_string.split(' '))
+        self.service_manager.supported_suffixes(file_type_string.split(' '))
         self.on_new_file_masks = translate('PresentationPlugin.MediaItem',
                                            'Presentations ({text})').format(text=file_type_string)
 

tests/functional/openlp_plugins/presentations/test_mediaitem.py

@@ -60,15 +60,15 @@ def test_build_file_mask_string(media_item):
         mocked_translate.side_effect = lambda module, string_to_translate: string_to_translate
         media_item.build_file_mask_string()
 
-    # THEN: The file mask should be generated correctly
+    # THEN: The file mask should be generated correctly with a space before all bar the first.
     assert '*.odp' in media_item.on_new_file_masks, 'The file mask should contain the odp extension'
-    assert '*.ppt' in media_item.on_new_file_masks, 'The file mask should contain the ppt extension'
+    assert ' *.ppt' in media_item.on_new_file_masks, 'The file mask should contain the ppt extension'
-    assert '*.pdf' in media_item.on_new_file_masks, 'The file mask should contain the pdf extension'
+    assert ' *.pdf' in media_item.on_new_file_masks, 'The file mask should contain the pdf extension'
-    assert '*.xps' in media_item.on_new_file_masks, 'The file mask should contain the xps extension'
+    assert ' *.xps' in media_item.on_new_file_masks, 'The file mask should contain the xps extension'
-    assert '*.oxps' in media_item.on_new_file_masks, 'The file mask should contain the oxps extension'
+    assert ' *.oxps' in media_item.on_new_file_masks, 'The file mask should contain the oxps extension'
-    assert '*.epub' in media_item.on_new_file_masks, 'The file mask should contain the epub extension'
+    assert ' *.epub' in media_item.on_new_file_masks, 'The file mask should contain the epub extension'
-    assert '*.cbz' in media_item.on_new_file_masks, 'The file mask should contain the cbz extension'
+    assert ' *.cbz' in media_item.on_new_file_masks, 'The file mask should contain the cbz extension'
-    assert '*.fb2' in media_item.on_new_file_masks, 'The file mask should contain the fb2 extension'
+    assert ' *.fb2' in media_item.on_new_file_masks, 'The file mask should contain the fb2 extension'
 
 
 def test_clean_up_thumbnails(media_item):