network: Use uuids for identifying connections

- Name for a network connection is not unique.  UUIDs are much less likely to
  change compared to name of the connection.  No necessity to perform URL
  encoding too.

- Move UI error message strings to view module instead of the network module
  for proper separation.

- Minor refactoring.

plinth/modules/networks/networks.py

@@ -54,19 +54,18 @@ def index(request):
 
 
 @login_required
-def edit(request, conn_id):
+def edit(request, uuid):
     """Serve connection editing form."""
-    form = None
+    try:
-    name = urllib.parse.unquote_plus(conn_id)
+        connection = network.get_connection(uuid)
-    form_data = {'name': name}
+    except network.ConnectionNotFound:
-
+        messages.error(request, _('Cannot edit connection: '
-    conn = network.get_connection(name)
+                                  'Connection not found.'))
-    if not conn:
-        messages.error(
-            request,
-            _('Cannot edit connection: %s not found.') % name)
         return redirect(reverse_lazy('networks:index'))
-    settings = conn.GetSettings()
+
+    form = None
+    settings = connection.GetSettings()
+    form_data = {'name': settings['connection']['id']}
 
     if request.method == 'POST':
         if settings['connection']['type'] == '802-11-wireless':
@@ -81,7 +80,7 @@ def edit(request, conn_id):
 
             if settings['connection']['type'] == '802-3-ethernet':
                 network.edit_ethernet_connection(
-                    conn,
+                    connection,
                     name, zone,
                     ipv4_method, ipv4_address)
             elif settings['connection']['type'] == '802-11-wireless':
@@ -91,7 +90,7 @@ def edit(request, conn_id):
                 passphrase = form.cleaned_data['passphrase']
 
                 network.edit_wifi_connection(
-                    conn, name, zone,
+                    connection, name, zone,
                     ssid, mode, auth_mode, passphrase,
                     ipv4_method, ipv4_address)
             else:
@@ -125,7 +124,7 @@ def edit(request, conn_id):
                     wifi_sec = settings['802-11-wireless-security']
                     if wifi_sec['key-mgmt'] == 'wpa-psk':
                         form_data['auth_mode'] = 'wpa'
-                        secret = conn.GetSecrets()
+                        secret = connection.GetSecrets()
                         psk = secret['802-11-wireless-security']['psk']
                         form_data['passphrase'] = psk
                 else:
@@ -144,32 +143,34 @@ def edit(request, conn_id):
 
 
 @login_required
-def activate(request, conn_id):
+def activate(request, uuid):
     """Activate the connection."""
-    name = urllib.parse.unquote_plus(conn_id)
-
     try:
-        network.activate_connection(name)
+        connection = network.activate_connection(uuid)
-    except network.ConnectionNotFound as cnf:
+        name = connection.GetSettings()['connection']['id']
-        messages.error(request, cnf)
+        messages.success(request, _('Activated connection %s.') % name)
-        return redirect(reverse_lazy('networks:index'))
+    except network.ConnectionNotFound:
-    except network.DeviceNotFound as dnf:
+        messages.error(request, _('Failed to activate connection: '
-        messages.error(request, dnf)
+                                  'Connection not found.'))
-        return redirect(reverse_lazy('networks:index'))
+    except network.DeviceNotFound as exception:
+        name = exception.args[0].GetSettings()['connection']['id']
+        messages.error(request, _('Failed to activate connection %s: '
+                                  'No suitable device is available.') % name)
 
-    messages.success(request, _('Activated connection %s.') % name)
     return redirect(reverse_lazy('networks:index'))
 
 
 @login_required
-def deactivate(request, conn_id):
+def deactivate(request, uuid):
     """Deactivate the connection."""
-    name = urllib.parse.unquote_plus(conn_id)
     try:
-        network.deactivate_connection(name)
+        active_connection = network.deactivate_connection(uuid)
+        name = active_connection.Connection.GetSettings()['connection']['id']
         messages.success(request, _('Deactivated connection %s.') % name)
-    except network.ConnectionNotFound as cnf:
+    except network.ConnectionNotFound:
-        messages.error(request, cnf)
+        messages.error(request, _('Failed to de-activate connection: '
+                                  'Connection not found.'))
+
     return redirect(reverse_lazy('networks:index'))
 
 
@@ -300,20 +301,28 @@ def add_wifi(request):
 
 
 @login_required
-def delete(request, conn_id):
+def delete(request, uuid):
     """Handle deleting connections, showing a confirmation dialog first.
 
     On GET, display a confirmation page.
     On POST, delete the connection.
     """
-    name = urllib.parse.unquote_plus(conn_id)
     if request.method == 'POST':
         try:
-            network.delete_connection(name)
+            name = network.delete_connection(uuid)
-        except network.ConnectionNotFound as cnf:
-            messages.error(request, cnf)
-        else:
             messages.success(request, _('Connection %s deleted.') % name)
+        except network.ConnectionNotFound:
+            messages.error(request, _('Failed to delete connection: '
+                                      'Connection not found.'))
+
+        return redirect(reverse_lazy('networks:index'))
+
+    try:
+        connection = network.get_connection(uuid)
+        name = connection.GetSettings()['connection']['id']
+    except network.ConnectionNotFound:
+        messages.error(request, _('Failed to delete connection: '
+                                  'Connection not found.'))
         return redirect(reverse_lazy('networks:index'))
 
     return TemplateResponse(request, 'connections_delete.html',

plinth/modules/networks/urls.py

@@ -25,11 +25,11 @@ from django.conf.urls import patterns, url
 urlpatterns = patterns(
     'plinth.modules.networks.networks',
     url(r'^sys/networks/$', 'index', name='index'),
-    url(r'^sys/networks/(?P<conn_id>[\w.@+-]+)/edit/$',
+    url(r'^sys/networks/(?P<uuid>[\w.@+-]+)/edit/$',
         'edit', name='edit'),
-    url(r'^sys/networks/(?P<conn_id>[\w.@+-]+)/activate/$',
+    url(r'^sys/networks/(?P<uuid>[\w.@+-]+)/activate/$',
         'activate', name='activate'),
-    url(r'^sys/networks/(?P<conn_id>[\w.@+-]+)/deactivate/$',
+    url(r'^sys/networks/(?P<uuid>[\w.@+-]+)/deactivate/$',
         'deactivate', name='deactivate'),
     url(r'^sys/networks/scan/$', 'scan', name='scan'),
     url(r'^sys/networks/connect/(?P<connect_path>[\w.@+-]+)/$',
@@ -37,6 +37,6 @@ urlpatterns = patterns(
     url(r'^sys/networks/add/$', 'add', name='add'),
     url(r'^sys/networks/add/ethernet/$', 'add_ethernet', name='add_ethernet'),
     url(r'^sys/networks/add/wifi/$', 'add_wifi', name='add_wifi'),
-    url(r'^sys/networks/(?P<conn_id>[\w.@+-]+)/delete/$',
+    url(r'^sys/networks/(?P<uuid>[\w.@+-]+)/delete/$',
         'delete', name='delete'),
 )

plinth/network.py

@@ -20,12 +20,14 @@ Helper functions for working with network manager.
 """
 
 from dbus.exceptions import DBusException
-from gettext import gettext as _
+import logging
 import NetworkManager
 import uuid
 import urllib
 
 
+logger = logging.getLogger(__name__)
+
 CONNECTION_TYPE_NAMES = {
     '802-3-ethernet': 'Ethernet',
     '802-11-wireless': 'Wi-Fi',
@@ -33,68 +35,71 @@ CONNECTION_TYPE_NAMES = {
 
 
 class ConnectionNotFound(Exception):
-    def __init__(self, msg):
+    """Network connection with a given name could not be found."""
-        self.msg = msg
+    pass
-
-    def __str__(self):
-        return self.msg
 
 
 class DeviceNotFound(Exception):
-    def __init__(self, msg):
+    """Network device for specified operation could not be found."""
-        self.msg = msg
+    pass
-
-    def __str__(self):
-        return self.msg
 
 
 def get_connection_list():
     """Get a list of active and available connections."""
-    connections = []
+    active_uuids = []
-    active = []
+    for connection in NetworkManager.NetworkManager.ActiveConnections:
-
-    for conn in NetworkManager.NetworkManager.ActiveConnections:
         try:
-            settings = conn.Connection.GetSettings()['connection']
+            settings = connection.Connection.GetSettings()['connection']
         except DBusException:
             # DBusException can be thrown here if the connection list is loaded
             # quickly after a connection is deactivated.
             continue
-        active.append(settings['id'])
 
-    for conn in NetworkManager.Settings.ListConnections():
+        active_uuids.append(settings['uuid'])
-        settings = conn.GetSettings()['connection']
+
+    connections = []
+    for connection in NetworkManager.Settings.ListConnections():
+        settings = connection.GetSettings()['connection']
         # Display a friendly type name if known.
-        conn_type = CONNECTION_TYPE_NAMES.get(settings['type'],
+        connection_type = CONNECTION_TYPE_NAMES.get(settings['type'],
-                                              settings['type'])
+                                                    settings['type'])
         connections.append({
             'name': settings['id'],
-            'id': urllib.parse.quote_plus(settings['id']),
+            'uuid': settings['uuid'],
-            'type': conn_type,
+            'type': connection_type,
-            'is_active': settings['id'] in active,
+            'is_active': settings['uuid'] in active_uuids,
         })
-    connections.sort(key=lambda x: x['is_active'], reverse=True)
+    connections.sort(key=lambda connection: connection['is_active'],
+                     reverse=True)
     return connections
 
 
-def get_connection(name):
+def get_connection(uuid):
-    """Returns connection with id matching name.
+    """Return connection with matching uuid.
-    Returns None if not found.
+
+    Raise ConnectionNotFound if a connection with that uuid is not found.
     """
     connections = NetworkManager.Settings.ListConnections()
-    connections = dict([(x.GetSettings()['connection']['id'], x)
+    connections = {connection.GetSettings()['connection']['uuid']: connection
-                        for x in connections])
+                   for connection in connections}
-    return connections.get(name)
+    try:
+        return connections[uuid]
+    except KeyError:
+        raise ConnectionNotFound(uuid)
 
 
-def get_active_connection(name):
+def get_active_connection(uuid):
-    """Returns active connection with id matching name.
+    """Returns active connection with matching uuid.
-    Returns None if not found.
+
+    Raise ConnectionNotFound if a connection with that uuid is not found.
     """
     connections = NetworkManager.NetworkManager.ActiveConnections
-    connections = dict([(x.Connection.GetSettings()['connection']['id'], x)
+    connections = {connection.Connection.GetSettings()['connection']['uuid']:
-                        for x in connections])
+                   connection for connection in connections}
-    return connections.get(name)
+    try:
+        return connections[uuid]
+    except KeyError:
+        raise ConnectionNotFound(uuid)
 
 
 def edit_ethernet_connection(conn, name, zone, ipv4_method, ipv4_address):
@@ -154,25 +159,20 @@ def edit_wifi_connection(conn, name, zone,
     conn.Update(new_settings)
 
 
-def activate_connection(name):
+def activate_connection(uuid):
+    """Find and activate a network connection."""
     # Find the connection
-    conn = get_connection(name)
+    connection = get_connection(uuid)
-    if not conn:
-        raise ConnectionNotFound(
-            _('Failed to activate connection %s: '
-              'Connection not found.') % name)
 
     # Find a suitable device
-    ctype = conn.GetSettings()['connection']['type']
+    ctype = connection.GetSettings()['connection']['type']
     if ctype == 'vpn':
-        for dev in NetworkManager.NetworkManager.GetDevices():
+        for device in NetworkManager.NetworkManager.GetDevices():
-            if (dev.State == NetworkManager.NM_DEVICE_STATE_ACTIVATED
+            if device.State == NetworkManager.NM_DEVICE_STATE_ACTIVATED and \
-                and dev.Managed):
+               device.Managed:
                 break
         else:
-            raise DeviceNotFound(
+            raise DeviceNotFound(connection)
-                _('Failed to activate connection %s: '
-                  'No suitable device is available.') % name)
     else:
         dtype = {
             '802-11-wireless': NetworkManager.NM_DEVICE_TYPE_WIFI,
@@ -180,26 +180,22 @@ def activate_connection(name):
             'gsm': NetworkManager.NM_DEVICE_TYPE_MODEM,
         }.get(ctype, ctype)
 
-        for dev in NetworkManager.NetworkManager.GetDevices():
+        for device in NetworkManager.NetworkManager.GetDevices():
-            if (dev.DeviceType == dtype and
+            if device.DeviceType == dtype and \
-                dev.State == NetworkManager.NM_DEVICE_STATE_DISCONNECTED):
+               device.State == NetworkManager.NM_DEVICE_STATE_DISCONNECTED:
                 break
         else:
-            raise DeviceNotFound(
+            raise DeviceNotFound(connection)
-                _('Failed to activate connection %s: '
-                  'No suitable device is available.') % name)
 
-    NetworkManager.NetworkManager.ActivateConnection(conn, dev, "/")
+    NetworkManager.NetworkManager.ActivateConnection(connection, device, "/")
+    return connection
 
 
 def deactivate_connection(name):
-    active = get_active_connection(name)
+    """Find and de-activate a network connection."""
-    if active:
+    active_connection = get_active_connection(name)
-        NetworkManager.NetworkManager.DeactivateConnection(active)
+    NetworkManager.NetworkManager.DeactivateConnection(active_connection)
-    else:
+    return active_connection
-        raise ConnectionNotFound(
-            _('Failed to deactivate connection %s: '
-              'Connection not found.') % name)
 
 
 def add_ethernet_connection(name, zone, ipv4_method, ipv4_address):
@@ -257,12 +253,10 @@ def add_wifi_connection(name, zone,
 
 
 def delete_connection(name):
-    conn = get_connection(name)
+    connection = get_connection(name)
-    if not conn:
+    name = connection.GetSettings()['connection']['id']
-        raise ConnectionNotFound(
+    connection.Delete()
-            _('Failed to delete connection %s: '
+    return name
-              'Connection not found.') % name)
-    conn.Delete()
 
 
 def wifi_scan():