[plugin_installer] Further Refactoring

- Rename some methods more appropriate
- Move some methods to more appropriate places
- Dont emit GTK events to signal GTK, use GLib.idle_add
- Remove option to set the plugin repo URL, use global vars instead

plugin_installer/plugin_installer.py

@@ -23,7 +23,6 @@ from gi.repository import Gtk
 from gi.repository import GdkPixbuf
 from gi.repository import Pango
 from gi.repository import GLib
-from gi.repository import GObject
 
 import io
 import threading
@@ -36,7 +35,6 @@ import logging
 import posixpath
 
 from urllib.request import urlopen
-from urllib.parse import urlparse, urljoin
 from common import gajim
 from plugins import GajimPlugin
 from plugins.helpers import log_calls, log
@@ -44,9 +42,14 @@ from htmltextview import HtmlTextView
 from dialogs import WarningDialog, HigDialog, YesNoDialog
 from plugins.gui import GajimPluginConfigDialog
 from enum import IntEnum
+from gtkgui_helpers import get_action
 
 log = logging.getLogger('gajim.plugin_system.plugin_installer')
 
+PLUGINS_URL = 'https://ftp.gajim.org/plugins_1/'
+MANIFEST_URL = 'https://ftp.gajim.org/plugins_1/manifests.zip'
+MANIFEST_IMAGE_URL = 'https://ftp.gajim.org/plugins_1/manifests_images.zip'
+
 
 class Column(IntEnum):
     PIXBUF = 0
@@ -60,6 +63,12 @@ class Column(IntEnum):
     HOMEPAGE = 8
 
 
+def get_plugin_version(plugin_name):
+    for plugin in gajim.plugin_manager.plugins:
+        if plugin.name == plugin_name:
+            return plugin.version
+
+
 def convert_version_to_list(version_str):
     version_list = version_str.split('.')
     l = []
@@ -73,53 +82,53 @@ class PluginInstaller(GajimPlugin):
     def init(self):
         self.description = _('Install and Upgrade Plugins')
         self.config_dialog = PluginInstallerPluginConfigDialog(self)
-        self.config_default_values = {'http_server': ('https://ftp.gajim.org', ''),
+        self.config_default_values = {'check_update': (True, '')}
-                                      'check_update': (True, ''),
-                                      }
         self.window = None
         self.progressbar = None
         self.available_plugins_model = None
-        self.upgrading = False # True when opened from upgrade popup dialog
         self.timeout_id = 0
         self.connected_ids = {}
         icon = Gtk.Image()
         self.def_icon = icon.render_icon(Gtk.STOCK_PREFERENCES,
             Gtk.IconSize.MENU)
-        self.server_folder = 'plugins_1'
 
     @log_calls('PluginInstallerPlugin')
     def activate(self):
         if self.config['check_update']:
             self.timeout_id = GLib.timeout_add_seconds(30, self.check_update)
+        if 'plugins' in gajim.interface.instances:
+            self.on_activate(gajim.interface.instances['plugins'])
 
     @log_calls('PluginInstallerPlugin')
     def warn_update(self, plugins):
         def open_update(dummy):
-            self.upgrading = True
+            get_action('plugins').activate()
-            self.pl_menuitem.activate()
+            page = self.notebook.page_num(self.paned)
-            nb = gajim.interface.instances['plugins'].plugins_notebook
+            self.notebook.set_current_page(page)
-            page = nb.page_num(self.hpaned)
-            GLib.idle_add(nb.set_current_page, page)
         if plugins:
-            plugins_str = '\n'.join(plugins)
+            plugins_str = '\n' + '\n'.join(plugins)
             YesNoDialog(_('Plugins updates'), _('Some updates are available for'
                 ' your installer plugins. Do you want to update those plugins:'
                 '\n%s') % plugins_str, on_response_yes=open_update)
+        else:
+            if hasattr(self, 'thread'):
+                del self.thread
 
     def check_update(self):
-        self.thread = download_async(self, check_update=True)
+        if hasattr(self, 'thread'):
+            return
+        self.thread = DownloadAsync(self, check_update=True)
         self.thread.start()
         self.timeout_id = 0
 
     @log_calls('PluginInstallerPlugin')
     def deactivate(self):
-        self.pl_menuitem.disconnect(self.id_)
+        if hasattr(self, 'available_page'):
-        if hasattr(self, 'page_num'):
+            self.notebook.remove_page(self.notebook.page_num(self.paned))
-            self.notebook.remove_page(self.notebook.page_num(self.hpaned))
             self.notebook.set_current_page(0)
             for id_, widget in list(self.connected_ids.items()):
                 widget.disconnect(id_)
-            del self.page_num
+            del self.available_page
         if hasattr(self, 'thread'):
             del self.thread
         if self.timeout_id > 0:
@@ -127,9 +136,11 @@ class PluginInstaller(GajimPlugin):
             self.timeout_id = 0
 
     def on_activate(self, plugin_win):
-        if hasattr(self, 'page_num'):
+        if hasattr(self, 'available_page'):
             # 'Available' tab exists
             return
+        if hasattr(self, 'thread'):
+            del self.thread
         self.installed_plugins_model = plugin_win.installed_plugins_model
         self.notebook = plugin_win.plugins_notebook
         id_ = self.notebook.connect('switch-page', self.on_notebook_switch_page)
@@ -159,7 +170,7 @@ class PluginInstaller(GajimPlugin):
         context.add_provider(style_provider,
                              Gtk.STYLE_PROVIDER_PRIORITY_USER)
 
-        self.page_num = self.notebook.append_page(
+        self.available_page = self.notebook.append_page(
             self.paned, Gtk.Label.new(_('Available')))
 
         self.available_plugins_model = self.xml.get_object('plugin_store')
@@ -167,20 +178,6 @@ class PluginInstaller(GajimPlugin):
 
         self.progressbar.set_property('no-show-all', True)
 
-        if GObject.signal_lookup('error_signal', self.window) is 0:
-            GObject.signal_new('error_signal', self.window,
-                GObject.SignalFlags.RUN_LAST, GObject.TYPE_STRING,
-                (GObject.TYPE_STRING,))
-            GObject.signal_new('plugin_downloaded', self.window,
-                GObject.SignalFlags.RUN_LAST, GObject.TYPE_STRING,
-                (GObject.TYPE_PYOBJECT,))
-
-        id_ = self.window.connect('error_signal', self.on_some_ftp_error)
-        self.connected_ids[id_] = self.window
-        id_ = self.window.connect('plugin_downloaded',
-            self.on_plugin_downloaded)
-        self.connected_ids[id_] = self.window
-
         selection = self.available_treeview.get_selection()
         selection.connect(
             'changed', self.available_plugins_treeview_selection_changed)
@@ -198,8 +195,8 @@ class PluginInstaller(GajimPlugin):
     def on_win_destroy(self, widget):
         if hasattr(self, 'thread'):
             del self.thread
-        if hasattr(self, 'page_num'):
+        if hasattr(self, 'available_page'):
-            del self.page_num
+            del self.available_page
 
     def available_plugins_toggled_cb(self, cell, path):
         is_active = self.available_plugins_model[path][Column.UPGRADE]
@@ -208,19 +205,15 @@ class PluginInstaller(GajimPlugin):
         for i in range(len(self.available_plugins_model)):
             if self.available_plugins_model[i][Column.UPGRADE]:
                 dir_list.append(self.available_plugins_model[i][Column.DIR])
-        if not dir_list:
+        self.install_button.set_property('sensitive', bool(dir_list))
-            self.install_button.set_property('sensitive', False)
-        else:
-            self.install_button.set_property('sensitive', True)
 
     def on_notebook_switch_page(self, widget, page, page_num):
-        tab_label_text = self.notebook.get_tab_label_text(self.paned)
+        tab_label_text = self.notebook.get_tab_label_text(page)
         if tab_label_text != (_('Available')):
             return
         if not hasattr(self, 'thread'):
             self.available_plugins_model.clear()
-            self.progressbar.show()
+            self.thread = DownloadAsync(self, upgrading=True)
-            self.thread = download_async(self, upgrading=True)
             self.thread.start()
 
     def on_install_upgrade_clicked(self, widget):
@@ -230,18 +223,18 @@ class PluginInstaller(GajimPlugin):
             if self.available_plugins_model[i][Column.UPGRADE]:
                 dir_list.append(self.available_plugins_model[i][Column.DIR])
 
-        self.thread = download_async(self, remote=dir_list)
+        self.thread = DownloadAsync(self, remote_dirs=dir_list)
         self.thread.start()
 
-    def on_some_ftp_error(self, widget, error_text):
+    def on_error(self, error_text):
-        for i in range(len(self.available_plugins_model)):
+        if self.available_plugins_model:
-            self.available_plugins_model[i][Column.UPGRADE] = False
+            for i in range(len(self.available_plugins_model)):
-        self.progressbar.hide()
+                self.available_plugins_model[i][Column.UPGRADE] = False
-        def warn():
+            self.progressbar.hide()
-            WarningDialog(_('Ftp error'), error_text, self.window)
+        text = GLib.markup_escape_text(error_text)
-        GLib.idle_add(warn)
+        WarningDialog(_('Error'), text, self.window)
 
-    def on_plugin_downloaded(self, widget, plugin_dirs):
+    def on_plugin_downloaded(self, plugin_dirs):
         dialog = HigDialog(None, Gtk.MessageType.INFO, Gtk.ButtonsType.OK,
             '', _('All selected plugins downloaded'))
         dialog.set_modal(False)
@@ -288,7 +281,6 @@ class PluginInstaller(GajimPlugin):
                 plugin.activatable = False
             row = [plugin, plugin.name, is_active, plugin.activatable, icon]
             self.installed_plugins_model.append(row)
-
         dialog.popup()
 
     def available_plugins_treeview_selection_changed(self, treeview_selection):
@@ -396,7 +388,7 @@ class PluginInstaller(GajimPlugin):
         return plugins_found
 
     def select_root_iter(self):
-        if hasattr(self, 'page_num'):
+        if hasattr(self, 'available_page'):
             selection = self.available_treeview.get_selection()
             if selection.count_selected_rows() == 0:
                 root_iter = self.available_plugins_model.get_iter_first()
@@ -407,7 +399,7 @@ class PluginInstaller(GajimPlugin):
             vadjustment.set_value(0)
 
 
-class download_async(threading.Thread):
+class DownloadAsync(threading.Thread):
     def __init__(self, plugin, remote_dirs=None,
                  upgrading=False, check_update=False):
         threading.Thread.__init__(self)
@@ -430,16 +422,15 @@ class download_async(threading.Thread):
         self.progressbar.pulse()
         return True
 
-    def get_plugin_version(self, plugin_name):
-        for plugin in gajim.plugin_manager.plugins:
-            if plugin.name == plugin_name:
-                return plugin.version
-
     def run(self):
-        if self.check_update:
+        try:
-            self.run_check_update()
+            if self.check_update:
-        else:
+                self.run_check_update()
-            self.run_download_plugin_list()
+            else:
+                self.run_download_plugin_list()
+        except Exception as e:
+            GLib.idle_add(self.plugin.on_error, str(e))
+            log.exception('Error fetching plugin list')
 
     def parse_manifest(self, buf):
         '''
@@ -458,129 +449,92 @@ class download_async(threading.Thread):
             plugins.append(config)
         return plugins
 
-    def retrieve_path(self, directory, fname):
+    def download_url(self, url):
-        server = self.plugin.config['http_server']
+        log.debug('Fetching {}'.format(url))
-        if not server:
+        request = urlopen(url)
-            server = self.plugin.config_default_values['http_server'][0]
+        return io.BytesIO(request.read())
-        if not urlparse(server).scheme:
-            server = 'https://' + server
-        if urlparse(server).scheme != 'https':
-            log.warn('Warning: not using HTTPS is a '
-                     'very serious security issue!')
-        location = posixpath.join(directory, fname)
-        uri = urljoin(server, location)
-        log.debug('Fetching {}'.format(uri))
-        request = urlopen(uri)
-
-        manifest_buffer = io.BytesIO(request.read())
-
-        return manifest_buffer
-
-    def retrieve_manifest(self):
-        return self.retrieve_path(self.plugin.server_folder, 'manifests.zip')
 
     def run_check_update(self):
-        try:
+        to_update = []
-            to_update = []
+        zipbuf = self.download_url(MANIFEST_URL)
-            zipbuf = self.retrieve_manifest()
+        plugin_manifests = self.parse_manifest(zipbuf)
-            plugin_manifests = self.parse_manifest(zipbuf)
+        for config in plugin_manifests:
-            for config in plugin_manifests:
+            opts = config.options('info')
+            if 'name' not in opts or 'version' not in opts or \
+            'description' not in opts or 'authors' not in opts or \
+            'homepage' not in opts:
+                continue
+            local_version = get_plugin_version(config.get(
+                'info', 'name'))
+            if local_version:
+                local = convert_version_to_list(local_version)
+                remote = convert_version_to_list(config.get('info',
+                    'version'))
+                if remote > local:
+                    to_update.append(config.get('info', 'name'))
+        GLib.idle_add(self.plugin.warn_update, to_update)
+
+    def run_download_plugin_list(self):
+        GLib.idle_add(self.progressbar.show)
+        self.pulse = GLib.timeout_add(150, self.progressbar_pulse)
+        if not self.remote_dirs:
+            buf = self.download_url(MANIFEST_IMAGE_URL)
+            zip_file = zipfile.ZipFile(buf)
+            manifest_list = zip_file.namelist()
+            for filename in manifest_list:
+                if not filename.endswith('manifest.ini'):
+                    continue
+                dir_ = filename.split('/')[0]
+                config = configparser.ConfigParser()
+                conf_file = zip_file.open(filename)
+                config.read_file(io.TextIOWrapper(conf_file, encoding='utf-8'))
+                conf_file.close()
+                if not config.has_section('info'):
+                    continue
                 opts = config.options('info')
                 if 'name' not in opts or 'version' not in opts or \
                 'description' not in opts or 'authors' not in opts or \
                 'homepage' not in opts:
                     continue
-                local_version = self.get_plugin_version(config.get(
+
-                    'info', 'name'))
+                local_version = get_plugin_version(
-                if local_version:
+                    config.get('info', 'name'))
+                upgrade = False
+                if self.upgrading and local_version:
                     local = convert_version_to_list(local_version)
                     remote = convert_version_to_list(config.get('info',
                         'version'))
                     if remote > local:
-                        to_update.append(config.get('info', 'name'))
+                        upgrade = True
-            GLib.idle_add(self.warn_update, to_update)
+                        GLib.idle_add(
-        except Exception as e:
+                            self.plugin.install_button.set_property,
-            log.error('Ftp error when check updates: %s' % str(e), exc_info=True)
+                            'sensitive', True)
-
+                png_filename = dir_ + '/' + dir_ + '.png'
-    def run_download_plugin_list(self):
+                if png_filename in manifest_list:
-        try:
+                    data = zip_file.open(png_filename).read()
-            GLib.idle_add(self.progressbar.set_text,
+                    pbl = GdkPixbuf.PixbufLoader()
-                _('Connecting to server'))
+                    pbl.set_size(16, 16)
-            if not self.remote_dirs:
+                    pbl.write(data)
-                GLib.idle_add(self.progressbar.set_text,
+                    pbl.close()
-                                 _('Scan files on the server'))
+                    def_icon = pbl.get_pixbuf()
-                try:
+                else:
-                    buf = self.retrieve_path(self.plugin.server_folder, 'manifests_images.zip')
+                    def_icon = self.def_icon
-                except:
+                if local_version:
-                    log.exception("Error fetching plugin list")
+                    base_dir, user_dir = gajim.PLUGINS_DIRS
-                    return
+                    local_dir = os.path.join(user_dir, dir_)
-                zip_file = zipfile.ZipFile(buf)
+                GLib.idle_add(self.model_append, [def_icon, dir_,
-                manifest_list = zip_file.namelist()
+                    config.get('info', 'name'), local_version,
-                progress_step = 1.0 / len(manifest_list)
+                    config.get('info', 'version'), upgrade,
-                for filename in manifest_list:
+                    config.get('info', 'description'),
-                    if not filename.endswith('manifest.ini'):
+                    config.get('info', 'authors'),
-                        continue
+                    config.get('info', 'homepage'), ])
-                    dir_ = filename.split('/')[0]
+        else:
-                    fract = self.progressbar.get_fraction() + progress_step
+            self.download_plugin()
-                    GLib.idle_add(self.progressbar.set_fraction, fract)
+        GLib.source_remove(self.pulse)
-                    GLib.idle_add(self.progressbar.set_text,
+        GLib.idle_add(self.progressbar.hide)
-                        _('Reading "%s"') % dir_)
+        GLib.idle_add(self.plugin.select_root_iter)
-
-                    config = configparser.ConfigParser()
-                    conf_file = zip_file.open(filename)
-                    config.read_file(io.TextIOWrapper(conf_file, encoding='utf-8'))
-                    conf_file.close()
-                    if not config.has_section('info'):
-                        continue
-                    opts = config.options('info')
-                    if 'name' not in opts or 'version' not in opts or \
-                    'description' not in opts or 'authors' not in opts or \
-                    'homepage' not in opts:
-                        continue
-
-                    local_version = self.get_plugin_version(
-                        config.get('info', 'name'))
-                    upgrade = False
-                    if self.upgrading and local_version:
-                        local = convert_version_to_list(local_version)
-                        remote = convert_version_to_list(config.get('info',
-                            'version'))
-                        if remote > local:
-                            upgrade = True
-                            GLib.idle_add(
-                                self.plugin.install_button.set_property,
-                                'sensitive', True)
-                    png_filename = dir_ + '/' + dir_ + '.png'
-                    if png_filename in manifest_list:
-                        data = zip_file.open(png_filename).read()
-                        pbl = GdkPixbuf.PixbufLoader()
-                        pbl.set_size(16, 16)
-                        pbl.write(data)
-                        pbl.close()
-                        def_icon = pbl.get_pixbuf()
-                    else:
-                        def_icon = self.def_icon
-                    if local_version:
-                        base_dir, user_dir = gajim.PLUGINS_DIRS
-                        local_dir = os.path.join(user_dir, dir_)
-                    GLib.idle_add(self.model_append, [def_icon, dir_,
-                        config.get('info', 'name'), local_version,
-                        config.get('info', 'version'), upgrade,
-                        config.get('info', 'description'),
-                        config.get('info', 'authors'),
-                        config.get('info', 'homepage'), ])
-            GLib.idle_add(self.progressbar.set_fraction, 0)
-            if self.remote_dirs:
-                self.download_plugin()
-            GLib.idle_add(self.progressbar.hide)
-            GLib.idle_add(self.plugin.select_root_iter)
-        except Exception as e:
-            self.window.emit('error_signal', str(e))
 
     def download_plugin(self):
-        GLib.idle_add(self.progressbar.show)
-        self.pulse = GLib.timeout_add(150, self.progressbar_pulse)
-        GLib.idle_add(self.progressbar.set_text, _('Creating a list of files'))
         for remote_dir in self.remote_dirs:
             filename = remote_dir + '.zip'
             base_dir, user_dir = gajim.PLUGINS_DIRS
@@ -592,18 +546,15 @@ class download_async(threading.Thread):
             local_dir = os.path.split(user_dir)[0]
 
             # downloading zip file
-            GLib.idle_add(self.progressbar.set_text,
-                             _('Downloading "%s"') % filename)
             try:
-                buf = self.plugin.retrieve_path(self.plugin.server_folder,
+                plugin = posixpath.join(PLUGINS_URL, filename)
-                                                filename)
+                buf = self.download_url(plugin)
             except:
                 log.exception("Error downloading plugin %s" % filename)
                 continue
             with zipfile.ZipFile(buf) as zip_file:
                 zip_file.extractall(os.path.join(local_dir, 'plugins'))
-        GLib.idle_add(self.window.emit, 'plugin_downloaded', self.remote_dirs)
+        GLib.idle_add(self.plugin.on_plugin_downloaded, self.remote_dirs)
-        GLib.source_remove(self.pulse)
 
 
 class PluginInstallerPluginConfigDialog(GajimPluginConfigDialog):