Merge lp:~alecu/ubuntu-sso-client/gsettings-at-sign into lp:ubuntu-sso-client

[Alejandro J. Cura]

- Account for g_variant_print type annotations (LP: #1007109).

[Manuel de la Peña (mandel)]

review: Approve

[Natalia Bidart (nataliabidart)]

The branch looks ok, I'm a little afraid we won't have more settings with 'annotations'.

Would you please improve a bit the parsing so the "default" case is not a cast to int()? I would suggest something like:

elif value.isdigit():
    parsed_value = int(valued)
else:
    logger.warning('Can not parse value from gsettings %r, ignoring.', value)

Does that make sense?

review: Needs Information

[lp:~alecu/ubuntu-sso-client/gsettings-at-sign]

977. By Alejandro J. Cura on 2012-06-25

Handle default parsing case as suggested on reviews

978. By Alejandro J. Cura on 2012-06-25

Fixes requested on review

[Natalia Bidart (nataliabidart)]

Works great!

Thanks

review: Approve

=== modified file 'ubuntu_sso/gtk/tests/test_gui.py'
--- ubuntu_sso/gtk/tests/test_gui.py	2012-06-22 16:51:06 +0000
+++ ubuntu_sso/gtk/tests/test_gui.py	2012-06-25 16:38:20 +0000
@@ -168,6 +168,7 @@
         self.memento = MementoHandler()
         self.memento.setLevel(logging.DEBUG)
         gui.logger.addHandler(self.memento)
+        self.addCleanup(gui.logger.removeHandler, self.memento)
 
     def _set_called(self, *args, **kwargs):
         """Set _called to True."""

=== modified file 'ubuntu_sso/tests/test_credentials.py'
--- ubuntu_sso/tests/test_credentials.py	2012-06-22 16:51:06 +0000
+++ ubuntu_sso/tests/test_credentials.py	2012-06-25 16:38:20 +0000
@@ -122,6 +122,7 @@
         self.memento = MementoHandler()
         self.memento.setLevel(logging.DEBUG)
         credentials.logger.addHandler(self.memento)
+        self.addCleanup(credentials.logger.removeHandler, self.memento)
 
         self.patch(credentials, 'get_bin_dir', lambda: self.bin_dir)
 

=== modified file 'ubuntu_sso/utils/tests/test_common.py'
--- ubuntu_sso/utils/tests/test_common.py	2012-04-17 20:27:21 +0000
+++ ubuntu_sso/utils/tests/test_common.py	2012-06-25 16:38:20 +0000
@@ -106,6 +106,7 @@
         self.memento = MementoHandler()
         self.memento.setLevel(logging.DEBUG)
         utils.logger.addHandler(self.memento)
+        self.addCleanup(utils.logger.removeHandler, self.memento)
 
         self.get_dir = getattr(utils, self.DIR_GETTER)
 

=== modified file 'ubuntu_sso/utils/webclient/gsettings.py'
--- ubuntu_sso/utils/webclient/gsettings.py	2012-04-09 17:38:24 +0000
+++ ubuntu_sso/utils/webclient/gsettings.py	2012-06-25 16:38:20 +0000
@@ -30,7 +30,11 @@
 
 import subprocess
 
+from ubuntu_sso.logger import setup_logging
+
+logger = setup_logging("ubuntu_sso.utils.webclient.gsettings")
 GSETTINGS_CMDLINE = "gsettings list-recursively org.gnome.system.proxy"
+CANNOT_PARSE_WARNING = "Cannot parse gsettings value: %r"
 
 
 def parse_proxy_host(hostname):
@@ -80,12 +84,15 @@
             continue
         if value.startswith("'"):
             parsed_value = value[1:-1]
-        elif value.startswith('['):
+        elif value.startswith(('[', '@')):
             parsed_value = value
         elif value in ('true', 'false'):
             parsed_value = (value == 'true')
+        elif value.isdigit():
+            parsed_value = int(value)
         else:
-            parsed_value = int(value)
+            logger.warning(CANNOT_PARSE_WARNING, value)
+            parsed_value = value
         relative_key = (path + "." + key)[base_len:]
         gsettings[relative_key] = parsed_value
     mode = gsettings["mode"]

=== modified file 'ubuntu_sso/utils/webclient/tests/test_gsettings.py'
--- ubuntu_sso/utils/webclient/tests/test_gsettings.py	2012-04-09 17:38:24 +0000
+++ ubuntu_sso/utils/webclient/tests/test_gsettings.py	2012-06-25 16:38:20 +0000
@@ -28,7 +28,10 @@
 # files in the program, then also delete it here.
 """Test the gsettings parser."""
 
+import logging
+
 from twisted.trial.unittest import TestCase
+from ubuntuone.devtools.handlers import MementoHandler
 
 from ubuntu_sso.utils.webclient import gsettings
 from ubuntu_sso.utils.webclient.tests import (
@@ -91,6 +94,35 @@
         """Test a parser of gsettings."""
         self._assert_parser_anonymous('https')
 
+    def test_gsettings_empty_ignore_hosts(self):
+        """Missing values in the ignore hosts."""
+        troublesome_value = "@as []"
+        template_values = dict(BASE_GSETTINGS_VALUES)
+        template_values["ignore_hosts"] = troublesome_value
+        fake_output = TEMPLATE_GSETTINGS_OUTPUT.format(**template_values)
+        self.patch(gsettings.subprocess, "check_output",
+                   lambda _: fake_output)
+        ps = gsettings.get_proxy_settings()
+        self.assertEqual(ps, {})
+
+    def test_gsettings_cannot_parse(self):
+        """Some weird setting that cannot be parsed is logged with warning."""
+        memento = MementoHandler()
+        memento.setLevel(logging.DEBUG)
+        gsettings.logger.addHandler(memento)
+        self.addCleanup(gsettings.logger.removeHandler, memento)
+
+        troublesome_value = "#bang"
+        template_values = dict(BASE_GSETTINGS_VALUES)
+        template_values["ignore_hosts"] = troublesome_value
+        fake_output = TEMPLATE_GSETTINGS_OUTPUT.format(**template_values)
+        self.patch(gsettings.subprocess, "check_output",
+                   lambda _: fake_output)
+        ps = gsettings.get_proxy_settings()
+        self.assertTrue(memento.check_warning(gsettings.CANNOT_PARSE_WARNING %
+                                              troublesome_value))
+        self.assertEqual(ps, {})
+
     def test_gsettings_parser_http_authenticated(self):
         """Test a parser of gsettings."""
         template_values = dict(BASE_GSETTINGS_VALUES)