From 2223a562689aa4fc8080c6b2925ad397ad2ea556 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sat, 2 May 2020 18:53:56 +0000 Subject: [PATCH] Logging and Powerpoint fixes --- openlp/core/api/versions/v2/controller.py | 23 ++++++++++--------- openlp/core/api/versions/v2/core.py | 6 ++++- openlp/core/app.py | 3 ++- openlp/core/ui/mainwindow.py | 5 ++++ openlp/plugins/presentations/lib/mediaitem.py | 4 ++-- .../presentations/test_mediaitem.py | 16 ++++++------- 6 files changed, 34 insertions(+), 23 deletions(-) diff --git a/openlp/core/api/versions/v2/controller.py b/openlp/core/api/versions/v2/controller.py index 0b4394040..e94a55d84 100644 --- a/openlp/core/api/versions/v2/controller.py +++ b/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 . # ########################################################################## +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 diff --git a/openlp/core/api/versions/v2/core.py b/openlp/core/api/versions/v2/core.py index 28ef99d9e..650d58fb3 100644 --- a/openlp/core/api/versions/v2/core.py +++ b/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 . # ########################################################################## +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 diff --git a/openlp/core/app.py b/openlp/core/app.py index 7fa4cc06c..11308c853 100644 --- a/openlp/core/app.py +++ b/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): diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 63490dc49..a13f8494a 100644 --- a/openlp/core/ui/mainwindow.py +++ b/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() diff --git a/openlp/plugins/presentations/lib/mediaitem.py b/openlp/plugins/presentations/lib/mediaitem.py index 8c7d0fefc..4d1c967ba 100644 --- a/openlp/plugins/presentations/lib/mediaitem.py +++ b/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() - self.service_manager.supported_suffixes(file_type_string.split(' ')) + file_type_string = file_type_string.strip() + self.service_manager.supported_suffixes(file_type_string.split(' ')) self.on_new_file_masks = translate('PresentationPlugin.MediaItem', 'Presentations ({text})').format(text=file_type_string) diff --git a/tests/functional/openlp_plugins/presentations/test_mediaitem.py b/tests/functional/openlp_plugins/presentations/test_mediaitem.py index cc7ac2f96..79d93c970 100644 --- a/tests/functional/openlp_plugins/presentations/test_mediaitem.py +++ b/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 '*.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 '*.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 '*.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 ' *.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 ' *.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 ' *.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 ' *.fb2' in media_item.on_new_file_masks, 'The file mask should contain the fb2 extension' def test_clean_up_thumbnails(media_item):