Merge lp:~trb143/openlp/themecleanup into lp:openlp

[Tim Bentley]

Finally got round to finishing the Theme clean up from 2,2!

Themes now save to JSON and read XML or JSON so fully compatible with 2.4.

 lp:~trb143/openlp/themecleanup (revision 2742)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/2028/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1938/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1869/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Code_Analysis/1249/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Test_Coverage/1104/
[FAILURE] https://ci.openlp.io/job/Branch-04c-Code_Analysis2/233/
Stopping after failure

openlp/core/ui/thememanager.py:576: [E0601(used-before-assignment), ThemeManager.unzip_theme] Using variable 'theme' before assignment

 theme = Theme()
 theme.load_theme(theme_zip.read(json_file[0]).decode("utf-8"))

who has been feeding Jenkins bad ideas!

[Tomas Groth (tomasgroth)]

I have no idea why, but this patch fixes the pylint error: https://bin.snyman.info/mmmc7cmv

review: Needs Fixing

[Phill (phill-ridout)]

I presume at some point we will drop support for XML themes altogether? I'd like to see a TODO, comment suggesting when we can drop it. i.e. # TODO: xml theme loading code can be removed after OpenLP 2.8 has been released.

review: Needs Information

[Phill (phill-ridout)]

> I presume at some point we will drop support for XML themes altogether? I'd
> like to see a TODO, comment suggesting when we can drop it. i.e. # TODO: xml
> theme loading code can be removed after OpenLP 2.8 has been released.

But otherwise this looks good. Not tested tho!

[lp:~trb143/openlp/themecleanup]

2743. By Tim Bentley on 2017-05-30

Minor tweeks

Unmerged revisions

=== modified file 'openlp/core/lib/theme.py'
--- openlp/core/lib/theme.py	2017-05-12 21:05:50 +0000
+++ openlp/core/lib/theme.py	2017-05-30 13:56:07 +0000
@@ -26,7 +26,6 @@
 import logging
 import json
 
-from xml.dom.minidom import Document
 from lxml import etree, objectify
 from openlp.core.common import AppLocation, de_hump
 
@@ -150,7 +149,7 @@
                 'horizontal_align', 'vertical_align', 'wrap_style']
 
 
-class ThemeXML(object):
+class Theme(object):
     """
     A class to encapsulate the Theme XML.
     """
@@ -195,184 +194,6 @@
                 self.background_filename = self.background_filename.strip()
                 self.background_filename = os.path.join(path, self.theme_name, self.background_filename)
 
-    def _new_document(self, name):
-        """
-        Create a new theme XML document.
-        """
-        self.theme_xml = Document()
-        self.theme = self.theme_xml.createElement('theme')
-        self.theme_xml.appendChild(self.theme)
-        self.theme.setAttribute('version', '2.0')
-        self.name = self.theme_xml.createElement('name')
-        text_node = self.theme_xml.createTextNode(name)
-        self.name.appendChild(text_node)
-        self.theme.appendChild(self.name)
-
-    def add_background_transparent(self):
-        """
-        Add a transparent background.
-        """
-        background = self.theme_xml.createElement('background')
-        background.setAttribute('type', 'transparent')
-        self.theme.appendChild(background)
-
-    def add_background_solid(self, bkcolor):
-        """
-        Add a Solid background.
-
-        :param bkcolor: The color of the background.
-        """
-        background = self.theme_xml.createElement('background')
-        background.setAttribute('type', 'solid')
-        self.theme.appendChild(background)
-        self.child_element(background, 'color', str(bkcolor))
-
-    def add_background_gradient(self, startcolor, endcolor, direction):
-        """
-        Add a gradient background.
-
-        :param startcolor: The gradient's starting colour.
-        :param endcolor: The gradient's ending colour.
-        :param direction: The direction of the gradient.
-        """
-        background = self.theme_xml.createElement('background')
-        background.setAttribute('type', 'gradient')
-        self.theme.appendChild(background)
-        # Create startColor element
-        self.child_element(background, 'startColor', str(startcolor))
-        # Create endColor element
-        self.child_element(background, 'endColor', str(endcolor))
-        # Create direction element
-        self.child_element(background, 'direction', str(direction))
-
-    def add_background_image(self, filename, border_color):
-        """
-        Add a image background.
-
-        :param filename: The file name of the image.
-        :param border_color:
-        """
-        background = self.theme_xml.createElement('background')
-        background.setAttribute('type', 'image')
-        self.theme.appendChild(background)
-        # Create Filename element
-        self.child_element(background, 'filename', filename)
-        # Create endColor element
-        self.child_element(background, 'borderColor', str(border_color))
-
-    def add_background_video(self, filename, border_color):
-        """
-        Add a video background.
-
-        :param filename: The file name of the video.
-        :param border_color:
-        """
-        background = self.theme_xml.createElement('background')
-        background.setAttribute('type', 'video')
-        self.theme.appendChild(background)
-        # Create Filename element
-        self.child_element(background, 'filename', filename)
-        # Create endColor element
-        self.child_element(background, 'borderColor', str(border_color))
-
-    def add_font(self, name, color, size, override, fonttype='main', bold='False', italics='False',
-                 line_adjustment=0, xpos=0, ypos=0, width=0, height=0, outline='False', outline_color='#ffffff',
-                 outline_pixel=2, shadow='False', shadow_color='#ffffff', shadow_pixel=5):
-        """
-        Add a Font.
-
-        :param name: The name of the font.
-        :param color: The colour of the font.
-        :param size: The size of the font.
-        :param override: Whether or not to override the default positioning of the theme.
-        :param fonttype: The type of font, ``main`` or ``footer``. Defaults to ``main``.
-        :param bold:
-        :param italics: The weight of then font Defaults to 50 Normal
-        :param line_adjustment: Does the font render to italics Defaults to 0 Normal
-        :param xpos: The X position of the text block.
-        :param ypos: The Y position of the text block.
-        :param width: The width of the text block.
-        :param height: The height of the text block.
-        :param outline: Whether or not to show an outline.
-        :param outline_color: The colour of the outline.
-        :param outline_pixel:  How big the Shadow is
-        :param shadow: Whether or not to show a shadow.
-        :param shadow_color: The colour of the shadow.
-        :param shadow_pixel: How big the Shadow is
-        """
-        background = self.theme_xml.createElement('font')
-        background.setAttribute('type', fonttype)
-        self.theme.appendChild(background)
-        # Create Font name element
-        self.child_element(background, 'name', name)
-        # Create Font color element
-        self.child_element(background, 'color', str(color))
-        # Create Proportion name element
-        self.child_element(background, 'size', str(size))
-        # Create weight name element
-        self.child_element(background, 'bold', str(bold))
-        # Create italics name element
-        self.child_element(background, 'italics', str(italics))
-        # Create indentation name element
-        self.child_element(background, 'line_adjustment', str(line_adjustment))
-        # Create Location element
-        element = self.theme_xml.createElement('location')
-        element.setAttribute('override', str(override))
-        element.setAttribute('x', str(xpos))
-        element.setAttribute('y', str(ypos))
-        element.setAttribute('width', str(width))
-        element.setAttribute('height', str(height))
-        background.appendChild(element)
-        # Shadow
-        element = self.theme_xml.createElement('shadow')
-        element.setAttribute('shadowColor', str(shadow_color))
-        element.setAttribute('shadowSize', str(shadow_pixel))
-        value = self.theme_xml.createTextNode(str(shadow))
-        element.appendChild(value)
-        background.appendChild(element)
-        # Outline
-        element = self.theme_xml.createElement('outline')
-        element.setAttribute('outlineColor', str(outline_color))
-        element.setAttribute('outlineSize', str(outline_pixel))
-        value = self.theme_xml.createTextNode(str(outline))
-        element.appendChild(value)
-        background.appendChild(element)
-
-    def add_display(self, horizontal, vertical, transition):
-        """
-        Add a Display options.
-
-        :param horizontal: The horizontal alignment of the text.
-        :param vertical: The vertical alignment of the text.
-        :param transition: Whether the slide transition is active.
-        """
-        background = self.theme_xml.createElement('display')
-        self.theme.appendChild(background)
-        # Horizontal alignment
-        element = self.theme_xml.createElement('horizontalAlign')
-        value = self.theme_xml.createTextNode(str(horizontal))
-        element.appendChild(value)
-        background.appendChild(element)
-        # Vertical alignment
-        element = self.theme_xml.createElement('verticalAlign')
-        value = self.theme_xml.createTextNode(str(vertical))
-        element.appendChild(value)
-        background.appendChild(element)
-        # Slide Transition
-        element = self.theme_xml.createElement('slideTransition')
-        value = self.theme_xml.createTextNode(str(transition))
-        element.appendChild(value)
-        background.appendChild(element)
-
-    def child_element(self, element, tag, value):
-        """
-        Generic child element creator.
-        """
-        child = self.theme_xml.createElement(tag)
-        child.appendChild(self.theme_xml.createTextNode(value))
-        element.appendChild(child)
-        return child
-
     def set_default_header_footer(self):
         """
         Set the header and footer size into the current primary screen.
@@ -386,25 +207,24 @@
         self.font_footer_y = current_screen['size'].height() * 9 / 10
         self.font_footer_height = current_screen['size'].height() / 10
 
-    def dump_xml(self):
-        """
-        Dump the XML to file used for debugging
-        """
-        return self.theme_xml.toprettyxml(indent='  ')
-
-    def extract_xml(self):
-        """
-        Print out the XML string.
-        """
-        self._build_xml_from_attrs()
-        return self.theme_xml.toxml('utf-8').decode('utf-8')
-
-    def extract_formatted_xml(self):
-        """
-        Pull out the XML string formatted for human consumption
-        """
-        self._build_xml_from_attrs()
-        return self.theme_xml.toprettyxml(indent='    ', newl='\n', encoding='utf-8')
+    def load_theme(self, theme):
+        """
+        Convert the JSON file and expand it.
+
+        :param theme: the theme string
+        """
+        jsn = json.loads(theme)
+        self.expand_json(jsn)
+
+    def export_theme(self):
+        """
+        Loop through the fields and build a dictionary of them
+
+        """
+        theme_data = {}
+        for attr, value in self.__dict__.items():
+            theme_data["{attr}".format(attr=attr)] = value
+        return json.dumps(theme_data)
 
     def parse(self, xml):
         """
@@ -461,7 +281,8 @@
                 if element.tag == 'name':
                     self._create_attr('theme', element.tag, element.text)
 
-    def _translate_tags(self, master, element, value):
+    @staticmethod
+    def _translate_tags(master, element, value):
         """
         Clean up XML removing and redefining tags
         """
@@ -514,71 +335,5 @@
         theme_strings = []
         for key in dir(self):
             if key[0:1] != '_':
-                # TODO: Due to bound methods returned, I don't know how to write a proper test
                 theme_strings.append('{key:>30}: {value}'.format(key=key, value=getattr(self, key)))
         return '\n'.join(theme_strings)
-
-    def _build_xml_from_attrs(self):
-        """
-        Build the XML from the varables in the object
-        """
-        self._new_document(self.theme_name)
-        if self.background_type == BackgroundType.to_string(BackgroundType.Solid):
-            self.add_background_solid(self.background_color)
-        elif self.background_type == BackgroundType.to_string(BackgroundType.Gradient):
-            self.add_background_gradient(
-                self.background_start_color,
-                self.background_end_color,
-                self.background_direction
-            )
-        elif self.background_type == BackgroundType.to_string(BackgroundType.Image):
-            filename = os.path.split(self.background_filename)[1]
-            self.add_background_image(filename, self.background_border_color)
-        elif self.background_type == BackgroundType.to_string(BackgroundType.Video):
-            filename = os.path.split(self.background_filename)[1]
-            self.add_background_video(filename, self.background_border_color)
-        elif self.background_type == BackgroundType.to_string(BackgroundType.Transparent):
-            self.add_background_transparent()
-        self.add_font(
-            self.font_main_name,
-            self.font_main_color,
-            self.font_main_size,
-            self.font_main_override, 'main',
-            self.font_main_bold,
-            self.font_main_italics,
-            self.font_main_line_adjustment,
-            self.font_main_x,
-            self.font_main_y,
-            self.font_main_width,
-            self.font_main_height,
-            self.font_main_outline,
-            self.font_main_outline_color,
-            self.font_main_outline_size,
-            self.font_main_shadow,
-            self.font_main_shadow_color,
-            self.font_main_shadow_size
-        )
-        self.add_font(
-            self.font_footer_name,
-            self.font_footer_color,
-            self.font_footer_size,
-            self.font_footer_override, 'footer',
-            self.font_footer_bold,
-            self.font_footer_italics,
-            0,  # line adjustment
-            self.font_footer_x,
-            self.font_footer_y,
-            self.font_footer_width,
-            self.font_footer_height,
-            self.font_footer_outline,
-            self.font_footer_outline_color,
-            self.font_footer_outline_size,
-            self.font_footer_shadow,
-            self.font_footer_shadow_color,
-            self.font_footer_shadow_size
-        )
-        self.add_display(
-            self.display_horizontal_align,
-            self.display_vertical_align,
-            self.display_slide_transition
-        )
 
=== modified file 'openlp/core/ui/lib/pathedit.py'
--- openlp/core/ui/lib/pathedit.py	2017-05-22 18:22:43 +0000
+++ openlp/core/ui/lib/pathedit.py	2017-05-30 13:56:07 +0000
@@ -46,16 +46,16 @@
 
         :param parent: The parent of the widget. This is just passed to the super method.
         :type parent: QWidget or None
-        
+
         :param dialog_caption: Used to customise the caption in the QFileDialog.
         :param dialog_caption: str
-        
+
         :param default_path: The default path. This is set as the path when the revert button is clicked
         :type default_path: str
 
         :param show_revert: Used to determin if the 'revert button' should be visible.
         :type show_revert: bool
-        
+
         :return: None
         :rtype: None
         """
@@ -72,7 +72,7 @@
         Set up the widget
         :param show_revert: Show or hide the revert button
         :type show_revert: bool
-        
+
         :return: None
         :rtype: None
         """
 
=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py	2016-12-31 11:01:36 +0000
+++ openlp/core/ui/servicemanager.py	2017-05-30 13:56:07 +0000
@@ -698,7 +698,7 @@
                 translate('OpenLP.ServiceManager',
                           'OpenLP Service Files (*.osz);; OpenLP Service Files - lite (*.oszl)'))
         else:
-            file_name, filter_uesd = QtWidgets.QFileDialog.getSaveFileName(
+            file_name, filter_used = QtWidgets.QFileDialog.getSaveFileName(
                 self.main_window, UiStrings().SaveService, path,
                 translate('OpenLP.ServiceManager', 'OpenLP Service Files (*.osz);;'))
         if not file_name:
 
=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py	2016-12-31 11:01:36 +0000
+++ openlp/core/ui/thememanager.py	2017-05-30 13:56:07 +0000
@@ -22,6 +22,7 @@
 """
 The Theme Manager manages adding, deleteing and modifying of themes.
 """
+import json
 import os
 import zipfile
 import shutil
@@ -33,7 +34,7 @@
     check_directory_exists, UiStrings, translate, is_win, get_filesystem_encoding, delete_file
 from openlp.core.lib import FileDialog, ImageSource, ValidationError, get_text_file_string, build_icon, \
     check_item_selected, create_thumb, validate_thumb
-from openlp.core.lib.theme import ThemeXML, BackgroundType
+from openlp.core.lib.theme import Theme, BackgroundType
 from openlp.core.lib.ui import critical_error_message_box, create_widget_action
 from openlp.core.ui import FileRenameForm, ThemeForm
 from openlp.core.ui.lib import OpenLPToolbar
@@ -245,7 +246,7 @@
         their customisations.
         :param field:
         """
-        theme = ThemeXML()
+        theme = Theme()
         theme.set_default_header_footer()
         self.theme_form.theme = theme
         self.theme_form.exec()
@@ -378,11 +379,12 @@
             critical_error_message_box(message=translate('OpenLP.ThemeManager', 'You have not selected a theme.'))
             return
         theme = item.data(QtCore.Qt.UserRole)
-        path = QtWidgets.QFileDialog.getExistingDirectory(self,
-                                                          translate('OpenLP.ThemeManager',
-                                                                    'Save Theme - ({name})').format(name=theme),
-                                                          Settings().value(self.settings_section +
-                                                                           '/last directory export'))
+        path, filter_used = \
+            QtWidgets.QFileDialog.getSaveFileName(self.main_window,
+                                                  translate('OpenLP.ThemeManager', 'Save Theme - ({name})').
+                                                  format(name=theme),
+                                                  Settings().value(self.settings_section + '/last directory export'),
+                                                  translate('OpenLP.ThemeManager', 'OpenLP Themes (*.otz)'))
         self.application.set_busy_cursor()
         if path:
             Settings().setValue(self.settings_section + '/last directory export', path)
@@ -393,13 +395,12 @@
                                                             'Your theme has been successfully exported.'))
         self.application.set_normal_cursor()
 
-    def _export_theme(self, path, theme):
+    def _export_theme(self, theme_path, theme):
         """
         Create the zipfile with the theme contents.
-        :param path: Location where the zip file will be placed
+        :param theme_path: Location where the zip file will be placed
         :param theme: The name of the theme to be exported
         """
-        theme_path = os.path.join(path, theme + '.otz')
         theme_zip = None
         try:
             theme_zip = zipfile.ZipFile(theme_path, 'w')
@@ -452,7 +453,7 @@
         files = AppLocation.get_files(self.settings_section, '.png')
         # No themes have been found so create one
         if not files:
-            theme = ThemeXML()
+            theme = Theme()
             theme.theme_name = UiStrings().Default
             self._write_theme(theme, None, None)
             Settings().setValue(self.settings_section + '/global theme', theme.theme_name)
@@ -505,19 +506,27 @@
 
     def get_theme_data(self, theme_name):
         """
-        Returns a theme object from an XML file
+        Returns a theme object from an XML or JSON file
 
         :param theme_name: Name of the theme to load from file
         :return: The theme object.
         """
         self.log_debug('get theme data for theme {name}'.format(name=theme_name))
-        xml_file = os.path.join(self.path, str(theme_name), str(theme_name) + '.xml')
-        xml = get_text_file_string(xml_file)
-        if not xml:
+        theme_file = os.path.join(self.path, str(theme_name), str(theme_name) + '.json')
+        theme_data = get_text_file_string(theme_file)
+        jsn = True
+        if not theme_data:
+            theme_file = os.path.join(self.path, str(theme_name), str(theme_name) + '.xml')
+            theme_data = get_text_file_string(theme_file)
+            jsn = False
+        if not theme_data:
             self.log_debug('No theme data - using default theme')
-            return ThemeXML()
+            return Theme()
         else:
-            return self._create_theme_from_xml(xml, self.path)
+            if jsn:
+                return self._create_theme_from_json(theme_data, self.path)
+            else:
+                return self._create_theme_from_xml(theme_data, self.path)
 
     def over_write_message_box(self, theme_name):
         """
@@ -547,18 +556,28 @@
         out_file = None
         file_xml = None
         abort_import = True
+        json_theme = False
+        theme_name = ""
         try:
             theme_zip = zipfile.ZipFile(file_name)
-            xml_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.xml']
-            if len(xml_file) != 1:
-                self.log_error('Theme contains "{val:d}" XML files'.format(val=len(xml_file)))
-                raise ValidationError
-            xml_tree = ElementTree(element=XML(theme_zip.read(xml_file[0]))).getroot()
-            theme_version = xml_tree.get('version', default=None)
-            if not theme_version or float(theme_version) < 2.0:
-                self.log_error('Theme version is less than 2.0')
-                raise ValidationError
-            theme_name = xml_tree.find('name').text.strip()
+            json_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.json']
+            if len(json_file) != 1:
+                # TODO: remove XML handling at some point but would need a auto conversion to run first.
+                xml_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.xml']
+                if len(xml_file) != 1:
+                    self.log_error('Theme contains "{val:d}" theme files'.format(val=len(xml_file)))
+                    raise ValidationError
+                xml_tree = ElementTree(element=XML(theme_zip.read(xml_file[0]))).getroot()
+                theme_version = xml_tree.get('version', default=None)
+                if not theme_version or float(theme_version) < 2.0:
+                    self.log_error('Theme version is less than 2.0')
+                    raise ValidationError
+                theme_name = xml_tree.find('name').text.strip()
+            else:
+                new_theme = Theme()
+                new_theme.load_theme(theme_zip.read(json_file[0]).decode("utf-8"))
+                theme_name = new_theme.theme_name
+                json_theme = True
             theme_folder = os.path.join(directory, theme_name)
             theme_exists = os.path.exists(theme_folder)
             if theme_exists and not self.over_write_message_box(theme_name):
@@ -574,7 +593,7 @@
                     continue
                 full_name = os.path.join(directory, out_name)
                 check_directory_exists(os.path.dirname(full_name))
-                if os.path.splitext(name)[1].lower() == '.xml':
+                if os.path.splitext(name)[1].lower() == '.xml' or os.path.splitext(name)[1].lower() == '.json':
                     file_xml = str(theme_zip.read(name), 'utf-8')
                     out_file = open(full_name, 'w', encoding='utf-8')
                     out_file.write(file_xml)
@@ -597,7 +616,10 @@
             if not abort_import:
                 # As all files are closed, we can create the Theme.
                 if file_xml:
-                    theme = self._create_theme_from_xml(file_xml, self.path)
+                    if json_theme:
+                        theme = self._create_theme_from_json(file_xml, self.path)
+                    else:
+                        theme = self._create_theme_from_xml(file_xml, self.path)
                     self.generate_and_save_image(theme_name, theme)
                 # Only show the error message, when IOError was not raised (in
                 # this case the error message has already been shown).
@@ -646,16 +668,16 @@
         :param image_to: Where the Theme Image is to be saved to
         """
         name = theme.theme_name
-        theme_pretty_xml = theme.extract_formatted_xml()
+        theme_pretty = theme.export_theme()
         theme_dir = os.path.join(self.path, name)
         check_directory_exists(theme_dir)
-        theme_file = os.path.join(theme_dir, name + '.xml')
+        theme_file = os.path.join(theme_dir, name + '.json')
         if self.old_background_image and image_to != self.old_background_image:
             delete_file(self.old_background_image)
         out_file = None
         try:
             out_file = open(theme_file, 'w', encoding='utf-8')
-            out_file.write(theme_pretty_xml.decode('utf-8'))
+            out_file.write(theme_pretty)
         except IOError:
             self.log_exception('Saving theme to file failed')
         finally:
@@ -717,7 +739,8 @@
         """
         return os.path.join(self.path, theme + '.png')
 
-    def _create_theme_from_xml(self, theme_xml, image_path):
+    @staticmethod
+    def _create_theme_from_xml(theme_xml, image_path):
         """
         Return a theme object using information parsed from XML
 
@@ -725,11 +748,25 @@
         :param image_path: Where the theme image is stored
         :return: Theme data.
         """
-        theme = ThemeXML()
+        theme = Theme()
         theme.parse(theme_xml)
         theme.extend_image_filename(image_path)
         return theme
 
+    @staticmethod
+    def _create_theme_from_json(theme_json, image_path):
+        """
+        Return a theme object using information parsed from JSON
+
+        :param theme_json: The Theme data object.
+        :param image_path: Where the theme image is stored
+        :return: Theme data.
+        """
+        theme = Theme()
+        theme.load_theme(theme_json)
+        theme.extend_image_filename(image_path)
+        return theme
+
     def _validate_theme_action(self, select_text, confirm_title, confirm_text, test_plugin=True, confirm=True):
         """
         Check to see if theme has been selected and the destructive action is allowed.
 
=== modified file 'openlp/plugins/presentations/presentationplugin.py'
--- openlp/plugins/presentations/presentationplugin.py	2017-05-22 18:27:40 +0000
+++ openlp/plugins/presentations/presentationplugin.py	2017-05-30 13:56:07 +0000
@@ -1,4 +1,4 @@
-    # -*- coding: utf-8 -*-
+# -*- coding: utf-8 -*-
 # vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
 
 ###############################################################################
 
=== modified file 'openlp/plugins/songusage/forms/songusagedetaildialog.py'
--- openlp/plugins/songusage/forms/songusagedetaildialog.py	2017-05-22 18:22:43 +0000
+++ openlp/plugins/songusage/forms/songusagedetaildialog.py	2017-05-30 13:56:07 +0000
@@ -69,7 +69,7 @@
         self.file_horizontal_layout.setSpacing(8)
         self.file_horizontal_layout.setContentsMargins(8, 8, 8, 8)
         self.file_horizontal_layout.setObjectName('file_horizontal_layout')
-        self.report_path_edit = PathEdit(self.file_group_box, path_type = PathType.Directories, show_revert=False)
+        self.report_path_edit = PathEdit(self.file_group_box, path_type=PathType.Directories, show_revert=False)
         self.file_horizontal_layout.addWidget(self.report_path_edit)
         self.vertical_layout.addWidget(self.file_group_box)
         self.button_box = create_button_box(song_usage_detail_dialog, 'button_box', ['cancel', 'ok'])
 
=== modified file 'tests/functional/openlp_core_lib/test_renderer.py'
--- tests/functional/openlp_core_lib/test_renderer.py	2017-04-24 05:17:55 +0000
+++ tests/functional/openlp_core_lib/test_renderer.py	2017-05-30 13:56:07 +0000
@@ -30,7 +30,7 @@
 from openlp.core.common import Registry
 from openlp.core.lib import Renderer, ScreenList, ServiceItem, FormattingTags
 from openlp.core.lib.renderer import words_split, get_start_tags
-from openlp.core.lib.theme import ThemeXML
+from openlp.core.lib.theme import Theme
 
 
 SCREEN = {
@@ -189,7 +189,7 @@
         # GIVEN: test object and data
         mock_lyrics_css.return_value = ' FORMAT CSS; '
         mock_outline_css.return_value = ' OUTLINE CSS; '
-        theme_data = ThemeXML()
+        theme_data = Theme()
         theme_data.font_main_name = 'Arial'
         theme_data.font_main_size = 20
         theme_data.font_main_color = '#FFFFFF'
 
=== modified file 'tests/functional/openlp_core_lib/test_theme.py'
--- tests/functional/openlp_core_lib/test_theme.py	2016-12-31 11:01:36 +0000
+++ tests/functional/openlp_core_lib/test_theme.py	2017-05-30 13:56:07 +0000
@@ -25,36 +25,30 @@
 from unittest import TestCase
 import os
 
-from openlp.core.lib.theme import ThemeXML
-
-
-class TestThemeXML(TestCase):
+from openlp.core.lib.theme import Theme
+
+
+class TestTheme(TestCase):
     """
-    Test the ThemeXML class
+    Test the Theme class
     """
     def test_new_theme(self):
         """
-        Test the ThemeXML constructor
+        Test the Theme constructor
         """
-        # GIVEN: The ThemeXML class
+        # GIVEN: The Theme class
         # WHEN: A theme object is created
-        default_theme = ThemeXML()
+        default_theme = Theme()
 
         # THEN: The default values should be correct
-        self.assertEqual('#000000', default_theme.background_border_color,
-                         'background_border_color should be "#000000"')
-        self.assertEqual('solid', default_theme.background_type, 'background_type should be "solid"')
-        self.assertEqual(0, default_theme.display_vertical_align, 'display_vertical_align should be 0')
-        self.assertEqual('Arial', default_theme.font_footer_name, 'font_footer_name should be "Arial"')
-        self.assertFalse(default_theme.font_main_bold, 'font_main_bold should be False')
-        self.assertEqual(47, len(default_theme.__dict__), 'The theme should have 47 attributes')
+        self.check_theme(default_theme)
 
     def test_expand_json(self):
         """
         Test the expand_json method
         """
-        # GIVEN: A ThemeXML object and some JSON to "expand"
-        theme = ThemeXML()
+        # GIVEN: A Theme object and some JSON to "expand"
+        theme = Theme()
         theme_json = {
             'background': {
                 'border_color': '#000000',
@@ -73,31 +67,48 @@
             }
         }
 
-        # WHEN: ThemeXML.expand_json() is run
+        # WHEN: Theme.expand_json() is run
         theme.expand_json(theme_json)
 
         # THEN: The attributes should be set on the object
-        self.assertEqual('#000000', theme.background_border_color, 'background_border_color should be "#000000"')
-        self.assertEqual('solid', theme.background_type, 'background_type should be "solid"')
-        self.assertEqual(0, theme.display_vertical_align, 'display_vertical_align should be 0')
-        self.assertFalse(theme.font_footer_bold, 'font_footer_bold should be False')
-        self.assertEqual('Arial', theme.font_main_name, 'font_main_name should be "Arial"')
+        self.check_theme(theme)
 
     def test_extend_image_filename(self):
         """
         Test the extend_image_filename method
         """
         # GIVEN: A theme object
-        theme = ThemeXML()
+        theme = Theme()
         theme.theme_name = 'MyBeautifulTheme   '
         theme.background_filename = '    video.mp4'
         theme.background_type = 'video'
         path = os.path.expanduser('~')
 
-        # WHEN: ThemeXML.extend_image_filename is run
+        # WHEN: Theme.extend_image_filename is run
         theme.extend_image_filename(path)
 
         # THEN: The filename of the background should be correct
         expected_filename = os.path.join(path, 'MyBeautifulTheme', 'video.mp4')
         self.assertEqual(expected_filename, theme.background_filename)
         self.assertEqual('MyBeautifulTheme', theme.theme_name)
+
+    def test_save_retrieve(self):
+        """
+        Load a dummy theme, save it and reload it
+        """
+        # GIVEN: The default Theme class
+        # WHEN: A theme object is created
+        default_theme = Theme()
+        # THEN: The default values should be correct
+        save_theme_json = default_theme.export_theme()
+        lt = Theme()
+        lt.load_theme(save_theme_json)
+        self.check_theme(lt)
+
+    def check_theme(self, theme):
+        self.assertEqual('#000000', theme.background_border_color, 'background_border_color should be "#000000"')
+        self.assertEqual('solid', theme.background_type, 'background_type should be "solid"')
+        self.assertEqual(0, theme.display_vertical_align, 'display_vertical_align should be 0')
+        self.assertFalse(theme.font_footer_bold, 'font_footer_bold should be False')
+        self.assertEqual('Arial', theme.font_main_name, 'font_main_name should be "Arial"')
+        self.assertEqual(47, len(theme.__dict__), 'The theme should have 47 attributes')
 
=== modified file 'tests/functional/openlp_core_ui/test_thememanager.py'
--- tests/functional/openlp_core_ui/test_thememanager.py	2017-05-08 19:04:14 +0000
+++ tests/functional/openlp_core_ui/test_thememanager.py	2017-05-30 13:56:07 +0000
@@ -63,7 +63,7 @@
             mocked_zipfile_init.return_value = None
 
             # WHEN: The theme is exported
-            theme_manager._export_theme(os.path.join('some', 'path'), 'Default')
+            theme_manager._export_theme(os.path.join('some', 'path', 'Default.otz'), 'Default')
 
             # THEN: The zipfile should be created at the given path
             mocked_zipfile_init.assert_called_with(os.path.join('some', 'path', 'Default.otz'), 'w')
@@ -126,8 +126,9 @@
             theme_manager.path = ''
             mocked_theme = MagicMock()
             mocked_theme.theme_name = 'themename'
-            mocked_theme.extract_formatted_xml = MagicMock()
-            mocked_theme.extract_formatted_xml.return_value = 'fake_theme_xml'.encode()
+            mocked_theme.filename = "filename"
+            # mocked_theme.extract_formatted_xml = MagicMock()
+            # mocked_theme.extract_formatted_xml.return_value = 'fake_theme_xml'.encode()
 
             # WHEN: Calling _write_theme with path to different images
             file_name1 = os.path.join(TEST_RESOURCES_PATH, 'church.jpg')
@@ -148,14 +149,13 @@
         theme_manager.path = self.temp_folder
         mocked_theme = MagicMock()
         mocked_theme.theme_name = 'theme 愛 name'
-        mocked_theme.extract_formatted_xml = MagicMock()
-        mocked_theme.extract_formatted_xml.return_value = 'fake theme 愛 XML'.encode()
+        mocked_theme.export_theme.return_value = "{}"
 
         # WHEN: Calling _write_theme with a theme with a name with special characters in it
         theme_manager._write_theme(mocked_theme, None, None)
 
         # THEN: It should have been created
-        self.assertTrue(os.path.exists(os.path.join(self.temp_folder, 'theme 愛 name', 'theme 愛 name.xml')),
+        self.assertTrue(os.path.exists(os.path.join(self.temp_folder, 'theme 愛 name', 'theme 愛 name.json')),
                         'Theme with special characters should have been created!')
 
     def test_over_write_message_box_yes(self):