From 7f4c5f7410b08134bb737ea15a96d625ba62861a Mon Sep 17 00:00:00 2001
From: Sunil Mohan Adapa <sunil@medhas.org>
Date: Thu, 11 Feb 2016 09:45:09 +0530
Subject: [PATCH] Make app names as module identifiers

- The last part of the module import path is the module name.  This also
  becomes the Django app name.  Apps names have to be unique.  Hence,
  there is no scope for two different modules with same name but
  different load path to exist in the project.

- Most uses of list of loaded modules are dealing with app names instead
  of full module load path.  This is due to the fact that Django deals
  with app names and not module paths.

- It is also somewhat clumsy to access a loaded module as we are
  re-importing every time to get access module.

- Simplify all of the above by using app names are module identifiers
  and maintaing an ordered dictionary of app names to loadded modules.

- Remove unused imports.

- Minor styling fixes.
---
 plinth/__main__.py                        |  4 +--
 plinth/module_loader.py                   | 38 ++++++++++++-----------
 plinth/modules/avahi/__init__.py          |  4 +--
 plinth/modules/config/__init__.py         |  4 +--
 plinth/modules/config/urls.py             |  2 +-
 plinth/modules/datetime/__init__.py       |  3 +-
 plinth/modules/deluge/__init__.py         |  3 +-
 plinth/modules/diagnostics/__init__.py    |  2 +-
 plinth/modules/diagnostics/diagnostics.py | 27 ++++++----------
 plinth/modules/dynamicdns/__init__.py     |  2 +-
 plinth/modules/firewall/__init__.py       |  2 +-
 plinth/modules/ikiwiki/__init__.py        |  3 +-
 plinth/modules/letsencrypt/__init__.py    |  8 +----
 plinth/modules/monkeysphere/__init__.py   |  2 +-
 plinth/modules/mumble/__init__.py         |  3 +-
 plinth/modules/names/__init__.py          |  2 +-
 plinth/modules/networks/__init__.py       |  2 +-
 plinth/modules/openvpn/__init__.py        |  2 +-
 plinth/modules/owncloud/__init__.py       |  2 +-
 plinth/modules/pagekite/__init__.py       |  2 +-
 plinth/modules/power/__init__.py          |  2 +-
 plinth/modules/privoxy/__init__.py        |  7 ++---
 plinth/modules/quassel/__init__.py        |  2 +-
 plinth/modules/repro/__init__.py          |  2 +-
 plinth/modules/restore/__init__.py        |  2 +-
 plinth/modules/roundcube/__init__.py      |  4 +--
 plinth/modules/shaarli/__init__.py        |  2 +-
 plinth/modules/tor/__init__.py            |  2 +-
 plinth/modules/transmission/__init__.py   |  3 +-
 plinth/modules/upgrades/__init__.py       |  2 +-
 plinth/modules/users/__init__.py          |  4 +--
 plinth/modules/xmpp/__init__.py           |  3 +-
 32 files changed, 61 insertions(+), 91 deletions(-)

diff --git a/plinth/__main__.py b/plinth/__main__.py
index 37e62ea4b..b3f31ce3c 100644
--- a/plinth/__main__.py
+++ b/plinth/__main__.py
@@ -132,9 +132,7 @@ def setup_server():
     cherrypy.tree.mount(None, manual_url, config)
     logger.debug('Serving manual images %s on %s', manual_dir, manual_url)
 
-    for module_import_path in module_loader.loaded_modules:
-        module = importlib.import_module(module_import_path)
-        module_name = module_import_path.split('.')[-1]
+    for module_name, module in module_loader.loaded_modules.items():
         module_path = os.path.dirname(module.__file__)
         static_dir = os.path.join(module_path, 'static')
         if not os.path.isdir(static_dir):
diff --git a/plinth/module_loader.py b/plinth/module_loader.py
index e8cfd6dc9..c8e58cac1 100644
--- a/plinth/module_loader.py
+++ b/plinth/module_loader.py
@@ -19,6 +19,7 @@
 Discover, load and manage Plinth modules
 """
 
+import collections
 import django
 import importlib
 import logging
@@ -29,9 +30,9 @@ from plinth import cfg
 from plinth import urls
 from plinth.signals import pre_module_loading, post_module_loading
 
-LOGGER = logging.getLogger(__name__)
+logger = logging.getLogger(__name__)
 
-loaded_modules = []
+loaded_modules = collections.OrderedDict()
 _modules_to_load = None
 
 
@@ -42,16 +43,18 @@ def load_modules():
     """
     pre_module_loading.send_robust(sender="module_loader")
     modules = {}
-    for module_name in get_modules_to_load():
-        LOGGER.info('Importing %s', module_name)
+    for module_import_path in get_modules_to_load():
+        logger.info('Importing %s', module_import_path)
+        module_name = module_import_path.split('.')[-1]
         try:
-            modules[module_name] = importlib.import_module(module_name)
+            modules[module_name] = importlib.import_module(module_import_path)
         except Exception as exception:
-            LOGGER.exception('Could not import %s: %s', module_name, exception)
+            logger.exception('Could not import %s: %s', module_import_path,
+                             exception)
             if cfg.debug:
                 raise
 
-        _include_module_urls(module_name)
+        _include_module_urls(module_import_path, module_name)
 
     ordered_modules = []
     remaining_modules = dict(modules)  # Make a copy
@@ -64,14 +67,14 @@ def load_modules():
             _insert_modules(module_name, module, remaining_modules,
                             ordered_modules)
         except KeyError:
-            LOGGER.error('Unsatified dependency for module - %s',
+            logger.error('Unsatified dependency for module - %s',
                          module_name)
 
-    LOGGER.debug('Module load order - %s', ordered_modules)
+    logger.debug('Module load order - %s', ordered_modules)
 
     for module_name in ordered_modules:
         _initialize_module(modules[module_name])
-        loaded_modules.append(module_name)
+        loaded_modules[module_name] = modules[module_name]
 
     post_module_loading.send_robust(sender="module_loader")
 
@@ -94,7 +97,7 @@ def _insert_modules(module_name, module, remaining_modules, ordered_modules):
         try:
             module = remaining_modules.pop(dependency)
         except KeyError:
-            LOGGER.error('Not found or circular dependency - %s, %s',
+            logger.error('Not found or circular dependency - %s, %s',
                          module_name, dependency)
             raise
 
@@ -103,16 +106,15 @@ def _insert_modules(module_name, module, remaining_modules, ordered_modules):
     ordered_modules.append(module_name)
 
 
-def _include_module_urls(module_name):
+def _include_module_urls(module_import_path, module_name):
     """Include the module's URLs in global project URLs list"""
-    namespace = module_name.split('.')[-1]
-    url_module = module_name + '.urls'
+    url_module = module_import_path + '.urls'
     try:
         urls.urlpatterns += [
             django.conf.urls.url(
-                r'', django.conf.urls.include(url_module, namespace))]
+                r'', django.conf.urls.include(url_module, module_name))]
     except ImportError:
-        LOGGER.debug('No URLs for %s', module_name)
+        logger.debug('No URLs for %s', module_name)
         if cfg.debug:
             raise
 
@@ -122,13 +124,13 @@ def _initialize_module(module):
     try:
         init = module.init
     except AttributeError:
-        LOGGER.debug('No init() for module - %s', module.__name__)
+        logger.debug('No init() for module - %s', module.__name__)
         return
 
     try:
         init()
     except Exception as exception:
-        LOGGER.exception('Exception while running init for %s: %s',
+        logger.exception('Exception while running init for %s: %s',
                          module, exception)
         if cfg.debug:
             raise
diff --git a/plinth/modules/avahi/__init__.py b/plinth/modules/avahi/__init__.py
index 7c8f5592c..84987cdc7 100644
--- a/plinth/modules/avahi/__init__.py
+++ b/plinth/modules/avahi/__init__.py
@@ -20,16 +20,14 @@ Plinth module for service discovery.
 """
 
 from django.utils.translation import ugettext_lazy as _
-import subprocess
 
-from plinth import actions
 from plinth import action_utils
 from plinth import cfg
 from plinth import service as service_module
 
 # pylint: disable=C0103
 
-depends = ['plinth.modules.system']
+depends = ['system']
 
 service = None
 
diff --git a/plinth/modules/config/__init__.py b/plinth/modules/config/__init__.py
index 12cb341ff..f4f34c4e3 100644
--- a/plinth/modules/config/__init__.py
+++ b/plinth/modules/config/__init__.py
@@ -24,6 +24,4 @@ from .config import init
 
 __all__ = ['config', 'init']
 
-depends = ['plinth.modules.system',
-           'plinth.modules.firewall',
-           'plinth.modules.names']
+depends = ['system', 'firewall', 'names']
diff --git a/plinth/modules/config/urls.py b/plinth/modules/config/urls.py
index 35f392f31..846f6e19f 100644
--- a/plinth/modules/config/urls.py
+++ b/plinth/modules/config/urls.py
@@ -19,7 +19,7 @@
 URLs for the Configuration module
 """
 
-from django.conf.urls import patterns, url
+from django.conf.urls import url
 
 from . import config as views
 
diff --git a/plinth/modules/datetime/__init__.py b/plinth/modules/datetime/__init__.py
index 147b95146..1ca8f6244 100644
--- a/plinth/modules/datetime/__init__.py
+++ b/plinth/modules/datetime/__init__.py
@@ -22,13 +22,12 @@ Plinth module to configure system date and time
 from django.utils.translation import ugettext_lazy as _
 import subprocess
 
-from plinth import actions
 from plinth import action_utils
 from plinth import cfg
 from plinth import service as service_module
 
 
-depends = ['plinth.modules.system']
+depends = ['system']
 
 service = None
 
diff --git a/plinth/modules/deluge/__init__.py b/plinth/modules/deluge/__init__.py
index 7038d3696..40fc0fd40 100644
--- a/plinth/modules/deluge/__init__.py
+++ b/plinth/modules/deluge/__init__.py
@@ -21,13 +21,12 @@ Plinth module to configure a Deluge web client.
 
 from django.utils.translation import ugettext_lazy as _
 
-from plinth import actions
 from plinth import action_utils
 from plinth import cfg
 from plinth import service as service_module
 
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 service = None
 
diff --git a/plinth/modules/diagnostics/__init__.py b/plinth/modules/diagnostics/__init__.py
index 8005882b3..294d3f884 100644
--- a/plinth/modules/diagnostics/__init__.py
+++ b/plinth/modules/diagnostics/__init__.py
@@ -25,7 +25,7 @@ from plinth import action_utils
 
 __all__ = ['diagnostics', 'init']
 
-depends = ['plinth.modules.system']
+depends = ['system']
 
 
 def diagnose():
diff --git a/plinth/modules/diagnostics/diagnostics.py b/plinth/modules/diagnostics/diagnostics.py
index 19ab23e7d..4ce058e09 100644
--- a/plinth/modules/diagnostics/diagnostics.py
+++ b/plinth/modules/diagnostics/diagnostics.py
@@ -60,19 +60,14 @@ def index(request):
 @require_POST
 def module(request, module_name):
     """Return diagnostics for a particular module."""
-    found = False
-    for module_import_path in module_loader.loaded_modules:
-        if module_name == module_import_path.split('.')[-1]:
-            found = True
-            break
-
-    if not found:
+    try:
+        module = module_loader.loaded_modules[module_name]
+    except KeyError:
         raise Http404('Module does not exist or not loaded')
 
-    loaded_module = importlib.import_module(module_import_path)
     results = []
-    if hasattr(loaded_module, 'diagnose'):
-        results = loaded_module.diagnose()
+    if hasattr(module, 'diagnose'):
+        results = module.diagnose()
 
     return TemplateResponse(request, 'diagnostics_module.html',
                             {'title': _('Diagnostic Test'),
@@ -110,17 +105,15 @@ def run_on_all_modules():
                        'progress_percentage': 0}
 
     modules = []
-    for module_import_path in module_loader.loaded_modules:
-        loaded_module = importlib.import_module(module_import_path)
-        if not hasattr(loaded_module, 'diagnose'):
+    for module_name, module in module_loader.loaded_modules.items():
+        if not hasattr(module, 'diagnose'):
             continue
 
-        module_name = module_import_path.split('.')[-1]
-        modules.append((module_name, loaded_module))
+        modules.append((module_name, module))
         current_results['results'][module_name] = None
 
     current_results['modules'] = modules
-    for current_index, (module_name, loaded_module) in enumerate(modules):
-        current_results['results'][module_name] = loaded_module.diagnose()
+    for current_index, (module_name, module) in enumerate(modules):
+        current_results['results'][module_name] = module.diagnose()
         current_results['progress_percentage'] = \
             int((current_index + 1) * 100 / len(modules))
diff --git a/plinth/modules/dynamicdns/__init__.py b/plinth/modules/dynamicdns/__init__.py
index 747482e9d..7b4bc398d 100644
--- a/plinth/modules/dynamicdns/__init__.py
+++ b/plinth/modules/dynamicdns/__init__.py
@@ -24,4 +24,4 @@ from .dynamicdns import init
 
 __all__ = ['dynamicdns', 'init']
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
diff --git a/plinth/modules/firewall/__init__.py b/plinth/modules/firewall/__init__.py
index 27daf85a0..3abc58c06 100644
--- a/plinth/modules/firewall/__init__.py
+++ b/plinth/modules/firewall/__init__.py
@@ -24,4 +24,4 @@ from .firewall import init
 
 __all__ = ['firewall', 'init']
 
-depends = ['plinth.modules.system']
+depends = ['system']
diff --git a/plinth/modules/ikiwiki/__init__.py b/plinth/modules/ikiwiki/__init__.py
index 402f4b6ae..8830af010 100644
--- a/plinth/modules/ikiwiki/__init__.py
+++ b/plinth/modules/ikiwiki/__init__.py
@@ -21,13 +21,12 @@ Plinth module to configure ikiwiki
 
 from django.utils.translation import ugettext_lazy as _
 
-from plinth import actions
 from plinth import action_utils
 from plinth import cfg
 from plinth import service as service_module
 
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 service = None
 
diff --git a/plinth/modules/letsencrypt/__init__.py b/plinth/modules/letsencrypt/__init__.py
index 565d78bed..1375ed176 100644
--- a/plinth/modules/letsencrypt/__init__.py
+++ b/plinth/modules/letsencrypt/__init__.py
@@ -20,19 +20,13 @@ Plinth module for using Let's Encrypt.
 """
 
 from django.utils.translation import ugettext_lazy as _
-import json
 
-from plinth import actions
 from plinth import action_utils
 from plinth import cfg
-from plinth import service as service_module
 from plinth.modules import names
 
 
-depends = [
-    'plinth.modules.apps',
-    'plinth.modules.names'
-]
+depends = ['apps', 'names']
 
 service = None
 
diff --git a/plinth/modules/monkeysphere/__init__.py b/plinth/modules/monkeysphere/__init__.py
index d9501870d..b7a8329f3 100644
--- a/plinth/modules/monkeysphere/__init__.py
+++ b/plinth/modules/monkeysphere/__init__.py
@@ -23,7 +23,7 @@ from django.utils.translation import ugettext_lazy as _
 
 from plinth import cfg
 
-depends = ['plinth.modules.system']
+depends = ['system']
 
 
 def init():
diff --git a/plinth/modules/mumble/__init__.py b/plinth/modules/mumble/__init__.py
index 570a57cc8..846681b29 100644
--- a/plinth/modules/mumble/__init__.py
+++ b/plinth/modules/mumble/__init__.py
@@ -21,13 +21,12 @@ Plinth module to configure Mumble server
 
 from django.utils.translation import ugettext_lazy as _
 
-from plinth import actions
 from plinth import action_utils
 from plinth import cfg
 from plinth import service as service_module
 
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 service = None
 
diff --git a/plinth/modules/names/__init__.py b/plinth/modules/names/__init__.py
index 841b95f7f..8f9fd8488 100644
--- a/plinth/modules/names/__init__.py
+++ b/plinth/modules/names/__init__.py
@@ -31,7 +31,7 @@ SERVICES = (
     ('ssh', _('SSH'), 22),
 )
 
-depends = ['plinth.modules.system']
+depends = ['system']
 
 domain_types = {}
 domains = {}
diff --git a/plinth/modules/networks/__init__.py b/plinth/modules/networks/__init__.py
index a3a679823..c3c93a17b 100644
--- a/plinth/modules/networks/__init__.py
+++ b/plinth/modules/networks/__init__.py
@@ -31,7 +31,7 @@ from plinth import network
 
 __all__ = ['networks', 'init']
 
-depends = ['plinth.modules.system']
+depends = ['system']
 
 logger = Logger(__name__)
 
diff --git a/plinth/modules/openvpn/__init__.py b/plinth/modules/openvpn/__init__.py
index 20be7aabe..688b1507f 100644
--- a/plinth/modules/openvpn/__init__.py
+++ b/plinth/modules/openvpn/__init__.py
@@ -27,7 +27,7 @@ from plinth import cfg
 from plinth import service as service_module
 
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 service = None
 
diff --git a/plinth/modules/owncloud/__init__.py b/plinth/modules/owncloud/__init__.py
index e48fcae2d..543909e21 100644
--- a/plinth/modules/owncloud/__init__.py
+++ b/plinth/modules/owncloud/__init__.py
@@ -26,7 +26,7 @@ from plinth import action_utils
 
 __all__ = ['owncloud', 'init']
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 
 def diagnose():
diff --git a/plinth/modules/pagekite/__init__.py b/plinth/modules/pagekite/__init__.py
index b83c7e5ae..7e5f689ba 100644
--- a/plinth/modules/pagekite/__init__.py
+++ b/plinth/modules/pagekite/__init__.py
@@ -26,7 +26,7 @@ from . import utils
 
 __all__ = ['init']
 
-depends = ['plinth.modules.apps', 'plinth.modules.names']
+depends = ['apps', 'names']
 
 
 def init():
diff --git a/plinth/modules/power/__init__.py b/plinth/modules/power/__init__.py
index f8f4c4e56..32cd2af14 100644
--- a/plinth/modules/power/__init__.py
+++ b/plinth/modules/power/__init__.py
@@ -23,7 +23,7 @@ from django.utils.translation import ugettext_lazy as _
 
 from plinth import cfg
 
-depends = ['plinth.modules.system']
+depends = ['system']
 
 
 def init():
diff --git a/plinth/modules/privoxy/__init__.py b/plinth/modules/privoxy/__init__.py
index 545020a0f..4c3e07627 100644
--- a/plinth/modules/privoxy/__init__.py
+++ b/plinth/modules/privoxy/__init__.py
@@ -20,7 +20,6 @@ Plinth module to configure Privoxy.
 """
 
 from django.utils.translation import ugettext_lazy as _
-import json
 
 from plinth import actions
 from plinth import action_utils
@@ -28,7 +27,7 @@ from plinth import cfg
 from plinth import service as service_module
 
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 service = None
 
@@ -84,9 +83,7 @@ def diagnose_url_with_proxy():
 
         result = action_utils.diagnose_url(url, kind=address['kind'], env=env)
         result[0] = _('Access {url} with proxy {proxy} on tcp{kind}') \
-                    .format(url=url, proxy=proxy, kind=address['kind'])
+            .format(url=url, proxy=proxy, kind=address['kind'])
         results.append(result)
 
     return results
-
-
diff --git a/plinth/modules/quassel/__init__.py b/plinth/modules/quassel/__init__.py
index 3e8c1b03a..54a2ec45b 100644
--- a/plinth/modules/quassel/__init__.py
+++ b/plinth/modules/quassel/__init__.py
@@ -25,7 +25,7 @@ from plinth import action_utils
 from plinth import cfg
 from plinth import service as service_module
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 service = None
 
diff --git a/plinth/modules/repro/__init__.py b/plinth/modules/repro/__init__.py
index 7517052f8..5cba93082 100644
--- a/plinth/modules/repro/__init__.py
+++ b/plinth/modules/repro/__init__.py
@@ -25,7 +25,7 @@ from plinth import action_utils
 from plinth import cfg
 from plinth import service as service_module
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 service = None
 
diff --git a/plinth/modules/restore/__init__.py b/plinth/modules/restore/__init__.py
index 21bb030d4..45404a27a 100644
--- a/plinth/modules/restore/__init__.py
+++ b/plinth/modules/restore/__init__.py
@@ -27,7 +27,7 @@ service = None
 
 __all__ = ['init']
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 
 def init():
diff --git a/plinth/modules/roundcube/__init__.py b/plinth/modules/roundcube/__init__.py
index 5d9bd1e40..c170c6f2b 100644
--- a/plinth/modules/roundcube/__init__.py
+++ b/plinth/modules/roundcube/__init__.py
@@ -21,13 +21,11 @@ Plinth module to configure Roundcube.
 
 from django.utils.translation import ugettext_lazy as _
 
-from plinth import actions
 from plinth import action_utils
 from plinth import cfg
-from plinth import service as service_module
 
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 
 def init():
diff --git a/plinth/modules/shaarli/__init__.py b/plinth/modules/shaarli/__init__.py
index e23af9186..ac2e07df1 100644
--- a/plinth/modules/shaarli/__init__.py
+++ b/plinth/modules/shaarli/__init__.py
@@ -26,7 +26,7 @@ from plinth import cfg
 from plinth import service as service_module
 
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 service = None
 
diff --git a/plinth/modules/tor/__init__.py b/plinth/modules/tor/__init__.py
index bd7ef694e..6933e9348 100644
--- a/plinth/modules/tor/__init__.py
+++ b/plinth/modules/tor/__init__.py
@@ -32,7 +32,7 @@ from plinth.modules.names import SERVICES
 from plinth.signals import domain_added
 
 
-depends = ['plinth.modules.apps', 'plinth.modules.names']
+depends = ['apps', 'names']
 
 socks_service = None
 bridge_service = None
diff --git a/plinth/modules/transmission/__init__.py b/plinth/modules/transmission/__init__.py
index b57797b99..4206c1985 100644
--- a/plinth/modules/transmission/__init__.py
+++ b/plinth/modules/transmission/__init__.py
@@ -21,13 +21,12 @@ Plinth module to configure Transmission server
 
 from django.utils.translation import ugettext_lazy as _
 
-from plinth import actions
 from plinth import action_utils
 from plinth import cfg
 from plinth import service as service_module
 
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 service = None
 
diff --git a/plinth/modules/upgrades/__init__.py b/plinth/modules/upgrades/__init__.py
index 28b9f7c4a..2e5110a16 100644
--- a/plinth/modules/upgrades/__init__.py
+++ b/plinth/modules/upgrades/__init__.py
@@ -24,7 +24,7 @@ from django.utils.translation import ugettext_lazy as _
 from plinth import cfg
 
 
-depends = ['plinth.modules.system']
+depends = ['system']
 
 
 def init():
diff --git a/plinth/modules/users/__init__.py b/plinth/modules/users/__init__.py
index 7232efaa1..7cb738071 100644
--- a/plinth/modules/users/__init__.py
+++ b/plinth/modules/users/__init__.py
@@ -20,14 +20,12 @@ Plinth module to manage users
 """
 
 from django.utils.translation import ugettext_lazy as _
-import json
 import subprocess
 
 from plinth import cfg
-from plinth import actions
 from plinth import action_utils
 
-depends = ['plinth.modules.system']
+depends = ['system']
 
 
 def init():
diff --git a/plinth/modules/xmpp/__init__.py b/plinth/modules/xmpp/__init__.py
index d0851c24a..65e8a3c19 100644
--- a/plinth/modules/xmpp/__init__.py
+++ b/plinth/modules/xmpp/__init__.py
@@ -20,7 +20,6 @@ Plinth module to configure XMPP server
 """
 
 from django.utils.translation import ugettext_lazy as _
-import json
 
 from plinth import actions
 from plinth import action_utils
@@ -30,7 +29,7 @@ from plinth.signals import pre_hostname_change, post_hostname_change
 from plinth.signals import domainname_change
 
 
-depends = ['plinth.modules.apps']
+depends = ['apps']
 
 service = None