Merge lp:~tealeg/landscape-client/settings-ui into lp:~landscape/landscape-client/trunk

[Geoff Teale]

Firstly see: https://wiki.canonical.com/Landscape/EEP/GUIPrefs

This branch adds a script "landscape-client-settings-ui" that displays a dialog allowing the user to:

   * Selected either a hosted or dedicated server to connect to.
   * Set the account name and registration password for a hosted instance.
   * Set the server host name for a dedicated server.

In the event that the user is unhappy with their changes they are able to revert them to the set the dialog was initialised with, otherwise closing the dialog will result in the data being written back to the configuration file.

You can run unit test for this code using:

trial landscape.ui

You can run the dialog thus:

./scripts/landscape-client-settings-ui

This branch does *not* do the following:

  * Attempt to re-register the client following changes (this needs discussion)
  * Package the program for installation
  * Install the program into the settings panel.
  * Run in secure context

...all of these things will follow once this branch is merged.

[lp:~tealeg/landscape-client/settings-ui]

452. By Geoff Teale on 2012-01-03

Resolved divergance and conflicts

453. By Geoff Teale on 2012-01-04

Fixed typo: configfuration -> configuration

[Fernando Correa Neto (fcorrea)]

Nice work Geoff.

The code looks good, just marking as "Needs fixing" as there are some aesthetics that could be worked out.

[1]

82 + def __init__(self, configuration):
83 + self.__lock_out = True
84 + self.__lock = threading.Lock()
85 + self.__initial_server_host_name = self.DEFAULT_SERVER_HOST_NAME
86 + self.__configuration = configuration
87 + self.__configuration
88 + self.__configuration.load([])
89 + self.__load_data_from_config()
90 + self.__modified = False
91 + self.unlock()

We don't need that self.__configuration in 87.

[2]

93 + def default_dedicated(self):
94 + """
95 + Set L{server_host_name} to something sane when switching from hosted to
96 + dedicated
97 + """

We need a period at the end of line.

[3]

160 + def __derive_server_host_name_from_url(self, url):
161 + "Extract the hostname part from a url"
162 + try:
163 + without_protocol = url[url.index("://") + 3:]
164 + except ValueError:
165 + without_protocol = url
166 + try:
167 + return without_protocol[:without_protocol.index("/")]
168 + except ValueError:
169 + return without_protocol

Maybe we could store the result of urlparse.urlparse(url) and reuse it whenever we need it.
Also, the docstring must be a sentence ending with a period. I've observed it in some other methods as well.

[4]

354 + def setUp(self):
355 + super(LandscapeSettingsApplicationControllerUISetupTest, self).setUp()
356 +
357 + def fake_run(obj):
358 + """
359 + Retard X11 mapping.
360 + """
361 + pass
362 + self._real_run = Gtk.Dialog.run
363 + Gtk.Dialog.run = fake_run

Do you think the test suite will evolve much more for GUI? Maybe we could have another iteration to create some GUI specific base class and add stuff like this in it.

[5]

365 + def get_config():
366 + configdata = """
367 +[client]
368 +data_path = %s
369 +http_proxy = http://proxy.localdomain:3192
370 +tags = a_tag
371 +url = https://landscape.canonical.com/message-system
372 +account_name = foo
373 +registration_password = bar
374 +computer_title = baz
375 +https_proxy = https://proxy.localdomain:6192
376 +ping_url = http://landscape.canonical.com/ping
377 +
378 +""" % sys.path[0]

Maybe we could make it less unindented:

    def get_config():
        configdata = ("[client]"
                     "data_path = %s"
                     "http_proxy = http://proxy.localdomain:3192"
                      "tags = a_tag"
                      "url = https://landscape.canonical.com/message-system"
                      "account_name = foo"
                      "registration_password = bar"
                      "computer_title = baz"
                      "https_proxy = https://proxy.localdomain:6192"
                      "ping_url = http://landscape.canonical.com/ping") % sys.path[0]

[6]

381 + class MyLandscapeSettingsConfiguration(
382 + LandscapeSettingsConfiguration):
383 + default_config_filenames = [config_filena...

review: Needs Fixing

[Free Ekanayaka (free.ekanayaka)]

Hi Geoff, since the changeset is somehow large, please consider it splitting in smaller branches next time. It will speed up reviewing, and possibly let you get early comments.

[1]

-ping_url = http://localhost:8081/ping
+ping_url = http://localhost:8080/ping

Why this? This configuration is supposed to work locally against ./dev/advicedog, I believe this change breaks it.

[2]

landscape/ui/controller/configuration.py:124: redefinition of function 'server_host_name' from line 120
landscape/ui/controller/configuration.py:159: redefinition of function 'account_name' from line 155
landscape/ui/controller/configuration.py:172: redefinition of function 'registration_password' from line 168
landscape/ui/controller/app.py:3: 'register' imported but unused

[3]

+ # register(config)

Is this a leftover?

[4]

+ self.__configuration = configuration
+ self.__configuration
+ self.__configuration.load([])

Second line seems a leftover too.

[5]

I think we could safely drop all "Landscape" prefixes from the class names, as the namespace is clear from the modules path, and probably we don't have to disambiguate anything. This will make things a tad more terse/readable.

[6]

+ return LandscapeSettingsConfiguration([])

and

+class LandscapeSettingsConfiguration(LandscapeSetupConfiguration):

As discussed on IRC, passing [] to the constructor is probably broken, and it'd be good to use LandscapeSetupConfiguration directly. For this you could refactor LandscapeSetupConfiguration like this:

class LandscapeSetupConfiguration(BrokerConfiguration):

    fetch_import_url = fetch_import_url # You will need to move fetch_import_url to the top,
                                        # or even better make it a method of the class if it's
                                        # not use elsewhere, then tests can overwrite it.

and LandscapeSetupConfiguration.__init__ can be dropped entirely.

review: Needs Fixing

[Geoff Teale (tealeg)]

OK, so I have addressed those issues. Can you re-review please.

[lp:~tealeg/landscape-client/settings-ui]

454. By Geoff Teale on 2012-01-04

Convert double underscore prefixed symbols to single underscore prefixed symbols at fcorrea's suggestion

455. By Geoff Teale on 2012-01-04

Skip tests if GObject Introspection (Gtk3) is not available

456. By Geoff Teale on 2012-01-04

Remove leftover line

457. By Geoff Teale on 2012-01-04

Add full stop to comment

458. By Geoff Teale on 2012-01-04

Fixed some docstrings according to fcorreas suggestions.

459. By Geoff Teale on 2012-01-04

- Let LandscapeSettingsConfiguration pass None to it's superclass __init__ instead of passing this to each new instance.
- Remove instatiation with empty list from get_config

460. By Geoff Teale on 2012-01-05

Tidying up in line with comments from fcorrea

461. By Geoff Teale on 2012-01-05

Tidy up setUp in line with comments from fcorrea

462. By Geoff Teale on 2012-01-05

Reformat setUp code in-line with comments from fcorrea

463. By Geoff Teale on 2012-01-05

- Reset the landscape-client.conf to trunk values after accidental check-in

464. By Geoff Teale on 2012-01-05

Removed Landscape prefix from class names in line with review comments from free.ekanayaka

465. By Geoff Teale on 2012-01-05

Fixed lint issues

466. By Geoff Teale on 2012-01-05

- Removed fetch_import_url from __init__ of LandscapeSetupConfiguration
- Made fetch_import_url a method of LandscapeSetupConfiguration
- Fixed tests.

467. By Geoff Teale on 2012-01-05

Fixed failing tests caused by bad call to __init__ in superclass (old signature).

468. By Geoff Teale on 2012-01-05

- Fixed more failing tests caused by old signatures

[Fernando Correa Neto (fcorrea)]

Looks great, +1!

review: Approve

[lp:~tealeg/landscape-client/settings-ui]

469. By Geoff Teale on 2012-01-09

Derive Glade file location from module location.

470. By Geoff Teale on 2012-01-09

Make broker configuration default URL if it is not provided instead of exiting

471. By Geoff Teale on 2012-01-09

Remove SettingsConfiguration
 - Remove landscape.ui.model.configuration
 - Replace references to SettingsConfiguration with LandscapeSetupConfiguration

[Free Ekanayaka (free.ekanayaka)]

Hi Geoff, some more comments.

[7]

+if os.path.dirname(os.path.abspath(sys.argv[0])) == script_dir:
+ sys.path.insert(0, "./")
+ data_path = os.path.abspath(os.path.join(script_dir, os.path.pardir))
+ print data_path
+else:
+ from landscape.lib.warning import hide_warnings
+ hide_warnings()
+ data_path = None
+ print "B"

I suspect the print statements here are a leftover.

[8]

+ # self.select_landscape_hosting()

This line is commented.

[9]

+ if data_path is None:
+ self._ui_path = os.path.join(
+ controller.data_path, "ui",
+ ClientSettingsDialog.GLADE_FILE)

As discussed on IRC, data_path is for run-time data, better to put the glade file in the same directory as the python module.

[10]

+class SettingsConfiguration(LandscapeSetupConfiguration):
+ required_options = []

As mentioned, instead of subclassing it'd be better to drop the required "url" option from the base class and add a sensible default for it instead (the url of hosted).

[11]

+ with self._lock:

This will make the module fail to import from the tests on Python 2.5.

[12]

+ self._configuration.load([])

As mentioned, this should be passed argv, so we can have:

./scripts/landscape-client-settings-ui -c root-client.conf

(root-client.conf is the default file that you would use for testing a branch against a local server, like ./scripts/landscape-client -c root-client.conf)

[13]

+ @server_host_name.setter
+ def server_host_name(self, value):

I'd rename it to _set_server_host_name, to clearness and to make lint tools happy (there's are redefinition warning). Same for:

+ @account_name.setter
+ def account_name(self, value):

[14]

+ self._configuration.load([])
+ self._load_data_from_config()

This is minor, but I general I find __init__ methods that end up reading files or querying databases or similar "big" side effects a bit odd, and they often make the class tad less testable (indeed you have to use 2 test case classes to, with one to test the empty config file case). They should usually just set some attributes.

What do you think of moving the loading logic to a "load" method (or some other appropriate name), that you can drive from the tests? So we could have a single test like this:

class ConfigControllerTest(LandscapeTest):

    def setUp(self):
        super(ConfigControllerTest, self).setUp()
        config = "[client]"
        config += "data_path = /var/lib/landscape/client\n"
        config += "http_proxy = http://proxy.localdomain:3192\n"
        config += "tags = a_tag\n"
        config += "url = https://landscape.canonical.com/message-system\n"
        config += "account_name = foo\n"
        config += "registration_password = bar\n"
        config += "computer_title = baz\n"
        config += "https_proxy = https://proxy.localdomain:6192\n"
        config += "ping_url = http://landscape.canonical.com/ping\n"
        self.config_filename = self.makeFile(config)

        class MySettingsConfiguration(SettingsConfiguration):
            default_config_filenames = [self.config_filename]

        self.config = MySettingsConfiguration()
...

[lp:~tealeg/landscape-client/settings-ui]

472. By Geoff Teale on 2012-01-09

Allow landscape-client-settings-ui to accept a configuration file on the command line using the -c switch

473. By Geoff Teale on 2012-01-09

Replace with statements with explicit lock.acquire() and lock.release() statements to avoid problems with python < 2.6

474. By Geoff Teale on 2012-01-09

Fixed lint issues

475. By Geoff Teale on 2012-01-09

Restructured __init__ and tests in line with Free's comments

[Geoff Teale (tealeg)]

Hi Free.

OK, everything on the above list is now either done, or we have discussed an alternative fix on IRC which I have implemented. Please can you check again.

[Free Ekanayaka (free.ekanayaka)]

Great work, thanks for bearing with me and addressing all comments :)

+1!

[15]

+ Test that is we don't explicitly pass a URL that this value is

Typos/phrasing, "Test that is" -> "Test that if" and "URL that this" -> "URL, then this"

[16]

landscape/ui/model is now emtpy and can be dropped.

[17]

+ def __init__(self, args=[], data_path=None):

I believe the data_path argument is now unused, and can be dropped (all the way down the stack till ClientSettingsDialog).

[18]

+ def __init__(self, controller, data_path=None, *args, **kwargs):

Are *args and **kwargs necessary? If not, I'd drop them.

review: Approve

=== modified file 'landscape/broker/config.py'
--- landscape/broker/config.py	2011-11-11 16:45:32 +0000
+++ landscape/broker/config.py	2012-01-09 11:38:25 +0000
@@ -11,7 +11,7 @@
     @cvar required_options: C{["url"]}
     """
 
-    required_options = ["url"]
+    DEFAULT_URL = "https://landscape.canonical.com/message-system"
 
     def __init__(self):
         super(BrokerConfiguration, self).__init__()
@@ -115,3 +115,6 @@
             os.environ["https_proxy"] = self.https_proxy
         elif self._original_https_proxy:
             os.environ["https_proxy"] = self._original_https_proxy
+
+        if self.url is None:
+            self.url = self.DEFAULT_URL
 
=== modified file 'landscape/broker/tests/test_config.py'
--- landscape/broker/tests/test_config.py	2011-08-19 15:47:27 +0000
+++ landscape/broker/tests/test_config.py	2012-01-09 11:38:25 +0000
@@ -78,3 +78,16 @@
         self.assertEqual(configuration.urgent_exchange_interval, 12)
         self.assertEqual(configuration.exchange_interval, 34)
         self.assertEqual(configuration.ping_interval, 6)
+
+    def test_missing_url_is_defaulted(self):
+        """
+        Test that is we don't explicitly pass a URL that this value is
+        defaulted.
+        """
+        filename = self.makeFile("[client]\n")
+
+        configuration = BrokerConfiguration()
+        configuration.load(["--config", filename])
+
+        self.assertEqual(configuration.url,
+                         "https://landscape.canonical.com/message-system")
 
=== modified file 'landscape/configuration.py'
--- landscape/configuration.py	2011-12-01 14:01:29 +0000
+++ landscape/configuration.py	2012-01-09 11:38:25 +0000
@@ -68,9 +68,8 @@
     unsaved_options = ("no_start", "disable", "silent", "ok_no_register",
                        "import_from")
 
-    def __init__(self, fetch_import_url):
+    def __init__(self):
         super(LandscapeSetupConfiguration, self).__init__()
-        self._fetch_import_url = fetch_import_url
 
     def _load_external_options(self):
         """Handle the --import parameter.
@@ -89,7 +88,7 @@
                         os.environ["http_proxy"] = self.http_proxy
                     if self.https_proxy:
                         os.environ["https_proxy"] = self.https_proxy
-                    content = self._fetch_import_url(self.import_from)
+                    content = self.fetch_import_url(self.import_from)
                     parser.readfp(StringIO(content))
                 elif not os.path.isfile(self.import_from):
                     raise ImportOptionError("File %s doesn't exist." %
@@ -109,6 +108,21 @@
             options.update(self._command_line_options)
             self._command_line_options = options
 
+    def fetch_import_url(self, url):
+        """Handle fetching of URLs passed to --url."""
+
+        print_text("Fetching configuration from %s..." % url)
+        error_message = None
+        try:
+            content = fetch(url)
+        except FetchError, error:
+            error_message = str(error)
+        if error_message is not None:
+            raise ImportOptionError(
+                "Couldn't download configuration from %s: %s" %
+                (url, error_message))
+        return content
+
     def make_parser(self):
         """
         Specialize the parser, adding configure-specific options.
@@ -584,28 +598,8 @@
     return result
 
 
-def fetch_import_url(url):
-    """Handle fetching of URLs passed to --url.
-
-    This is done out of LandscapeSetupConfiguration since it has to deal
-    with interaction with the user and downloading of files.
-    """
-
-    print_text("Fetching configuration from %s..." % url)
-    error_message = None
-    try:
-        content = fetch(url)
-    except FetchError, error:
-        error_message = str(error)
-    if error_message is not None:
-        raise ImportOptionError(
-            "Couldn't download configuration from %s: %s" %
-            (url, error_message))
-    return content
-
-
 def main(args):
-    config = LandscapeSetupConfiguration(fetch_import_url)
+    config = LandscapeSetupConfiguration()
     if args in (["-h"], ["--help"]):
         # We let landscape-config --help to be run as normal user
         config.load(args)
 
=== modified file 'landscape/tests/test_configuration.py'
--- landscape/tests/test_configuration.py	2011-12-01 14:01:29 +0000
+++ landscape/tests/test_configuration.py	2012-01-09 11:38:25 +0000
@@ -11,7 +11,7 @@
     print_text, LandscapeSetupScript, LandscapeSetupConfiguration,
     register, setup, main, setup_init_script_and_start_client,
     stop_client_and_disable_init_script, ConfigurationError,
-    fetch_import_url, ImportOptionError, store_public_key_data)
+    ImportOptionError, store_public_key_data)
 from landscape.broker.registration import InvalidCredentialsError
 from landscape.sysvconfig import SysVConfig, ProcessError
 from landscape.tests.helpers import (
@@ -27,7 +27,7 @@
 url = https://landscape.canonical.com/message-system
 """)
         args.extend(["--config", filename, "--data-path", self.makeDir()])
-    config = LandscapeSetupConfiguration(fetch_import_url)
+    config = LandscapeSetupConfiguration()
     config.load(args)
     return config
 
@@ -91,7 +91,7 @@
 
         class MyLandscapeSetupConfiguration(LandscapeSetupConfiguration):
             default_config_filenames = [self.config_filename]
-        self.config = MyLandscapeSetupConfiguration(None)
+        self.config = MyLandscapeSetupConfiguration()
         self.script = LandscapeSetupScript(self.config)
 
     def test_show_help(self):
@@ -1889,7 +1889,7 @@
         When registration fails because of an unknown error, a message is
         printed and the program exits.
         """
-        configuration = LandscapeSetupConfiguration(None)
+        configuration = LandscapeSetupConfiguration()
 
         # We'll just mock the remote here to have it raise an exception.
         connector_factory = self.mocker.replace(
 
=== added directory 'landscape/ui'
=== added file 'landscape/ui/__init__.py'
=== added directory 'landscape/ui/controller'
=== added file 'landscape/ui/controller/__init__.py'
=== added file 'landscape/ui/controller/app.py'
--- landscape/ui/controller/app.py	1970-01-01 00:00:00 +0000
+++ landscape/ui/controller/app.py	2012-01-09 11:38:25 +0000
@@ -0,0 +1,32 @@
+from gi.repository import Gtk
+
+from landscape.configuration import LandscapeSetupConfiguration
+from landscape.ui.view.configuration import ClientSettingsDialog
+from landscape.ui.controller.configuration import ConfigController
+
+
+APPLICATION_ID = "com.canonical.landscape-client.settings.ui"
+
+
+class SettingsApplicationController(Gtk.Application):
+    """
+    Core application controller for the landscape settings application.
+    """
+
+    def __init__(self, args=[], data_path=None):
+        super(SettingsApplicationController, self).__init__(
+            application_id=APPLICATION_ID)
+        self._args = args
+        self.data_path = data_path
+        self.connect("activate", self.setup_ui)
+
+    def get_config(self):
+        return LandscapeSetupConfiguration()
+
+    def setup_ui(self, data=None):
+        config = self.get_config()
+        controller = ConfigController(config, args=self._args)
+        controller.load()
+        self.settings_dialog = ClientSettingsDialog(controller,
+                                                    data_path=self.data_path)
+        self.settings_dialog.run()
 
=== added file 'landscape/ui/controller/configuration.py'
--- landscape/ui/controller/configuration.py	1970-01-01 00:00:00 +0000
+++ landscape/ui/controller/configuration.py	2012-01-09 11:38:25 +0000
@@ -0,0 +1,232 @@
+import threading
+
+
+class ConfigControllerLockError(Exception):
+    pass
+
+
+class ConfigController(object):
+    """
+    L{ConfigContoller} defines actions to take against a configuration object,
+    providing starting values from the file, allowing them to be changed
+    transiently, reverted or committed.
+    """
+
+    HOSTED_HOST_NAME = "landscape.canonical.com"
+    DEFAULT_SERVER_HOST_NAME = "landscape.localdomain"
+
+    def __init__(self, configuration, args=[]):
+        self._initial_server_host_name = self.DEFAULT_SERVER_HOST_NAME
+        self._configuration = configuration
+        self._args = args
+        self._lock_out = False
+        self._lock = threading.Lock()
+
+    def load(self):
+        "Load the initial data from the configuration"
+        self.lock()
+        self._configuration.load(self._args)
+        self._pull_data_from_config()
+        self._modified = False
+        self.unlock()
+
+    def default_dedicated(self):
+        """
+        Set L{server_host_name} to something sane when switching from hosted to
+        dedicated.
+        """
+        if self._initial_server_host_name != self.HOSTED_HOST_NAME:
+            self._server_host_name = self._initial_server_host_name
+        else:
+            self._server_host_name = self.DEFAULT_SERVER_HOST_NAME
+            self._url = self._derive_url_from_host_name(
+                self._server_host_name)
+            self._ping_url = self._derive_ping_url_from_host_name(
+                self._server_host_name)
+        self._modified = True
+
+    def default_hosted(self):
+        """
+        Set L{server_host_name} in a recoverable fashion when switching from
+        dedicated to hosted.
+        """
+        if self._server_host_name != self.HOSTED_HOST_NAME:
+            self._server_host_name = self.HOSTED_HOST_NAME
+        self._url = self._derive_url_from_host_name(
+            self._server_host_name)
+        self._ping_url = self._derive_ping_url_from_host_name(
+            self._server_host_name)
+        self._modified = True
+
+    def _pull_data_from_config(self):
+        """
+        Pull in data set from configuration class.
+        """
+        self._lock.acquire()
+        self._data_path = self._configuration.data_path
+        self._http_proxy = self._configuration.http_proxy
+        self._tags = self._configuration.tags
+        self._url = self._configuration.url
+        self._ping_url = self._configuration.ping_url
+        self._account_name = self._configuration.account_name
+        self._registration_password = \
+            self._configuration.registration_password
+        self._computer_title = self._configuration.computer_title
+        self._https_proxy = self._configuration.https_proxy
+        self._ping_url = self._configuration.ping_url
+        if self._url:
+            self._server_host_name = \
+                self._derive_server_host_name_from_url(self._url)
+        else:
+            self._server_host_name = self.HOSTED_HOST_NAME
+        self._initial_server_host_name = self._server_host_name
+        self._modified = False
+        self._lock.release()
+
+    def lock(self):
+        "Block updates to the data set."
+        self._lock.acquire()
+        self._lock_out = True
+        self._lock.release()
+
+    def unlock(self):
+        "Allow updates to the data set."
+        self._lock.acquire()
+        self._lock_out = False
+        self._lock.release()
+
+    def is_locked(self):
+        "Check if updates are locked out."
+        self._lock.acquire()
+        lock_state = self._lock_out
+        self._lock.release()
+        return lock_state
+
+    def _derive_server_host_name_from_url(self, url):
+        "Extract the hostname part from a URL."
+        try:
+            without_protocol = url[url.index("://") + 3:]
+        except ValueError:
+            without_protocol = url
+        try:
+            return without_protocol[:without_protocol.index("/")]
+        except ValueError:
+            return without_protocol
+
+    def _derive_url_from_host_name(self, host_name):
+        "Extrapolate a url from a host name."
+        #Reuse this code to make sure it's a proper host name
+        host_name = self._derive_server_host_name_from_url(host_name)
+        return "https://" + host_name + "/message-system"
+
+    def _derive_ping_url_from_host_name(self, host_name):
+        "Extrapolate a ping_url from a host name."
+        #Reuse this code to make sure it's a proper host name
+        host_name = self._derive_server_host_name_from_url(host_name)
+        return "http://" + host_name + "/ping"
+
+    def _get_server_host_name(self):
+        return self._server_host_name
+
+    def _set_server_host_name(self, value):
+        self._lock.acquire()
+        if self._lock_out:
+            self._lock.release()
+            raise ConfigControllerLockError
+        else:
+            if value != self.HOSTED_HOST_NAME:
+                self._initial_server_host_name = value
+            self._server_host_name = value
+            self._url = self._derive_url_from_host_name(
+                self._server_host_name)
+            self._ping_url = self._derive_ping_url_from_host_name(
+                self._server_host_name)
+            self._modified = True
+            self._lock.release()
+    server_host_name = property(_get_server_host_name, _set_server_host_name)
+
+    @property
+    def data_path(self):
+        return self._data_path
+
+    @property
+    def url(self):
+        return self._url
+
+    @property
+    def http_proxy(self):
+        return self._http_proxy
+
+    @property
+    def tags(self):
+        return self._tags
+
+    def _get_account_name(self):
+        return self._account_name
+
+    def _set_account_name(self, value):
+        self._lock.acquire()
+        if self._lock_out:
+            self._lock.release()
+            raise ConfigControllerLockError
+        else:
+            self._account_name = value
+            self._modified = True
+            self._lock.release()
+    account_name = property(_get_account_name, _set_account_name)
+
+    def _get_registration_password(self):
+        return self._registration_password
+
+    def _set_registration_password(self, value):
+        self._lock.acquire()
+        if self._lock_out:
+            self._lock.release()
+            raise ConfigControllerLockError
+        else:
+            self._registration_password = value
+            self._modified = True
+            self._lock.release()
+    registration_password = property(_get_registration_password,
+                                     _set_registration_password)
+
+    @property
+    def computer_title(self):
+        return self._computer_title
+
+    @property
+    def https_proxy(self):
+        return self._https_proxy
+
+    @property
+    def ping_url(self):
+        return self._ping_url
+
+    @property
+    def hosted(self):
+        return self.server_host_name == self.HOSTED_HOST_NAME
+
+    @property
+    def is_modified(self):
+        return self._modified
+
+    def revert(self):
+        "Revert settings to those the configuration object originally found."
+        self._configuration.reload()
+        self._pull_data_from_config()
+
+    def commit(self):
+        "Persist settings via the configuration object."
+        self._configuration.data_path = self._data_path
+        self._configuration.http_proxy = self._http_proxy
+        self._configuration.tags = self._tags
+        self._configuration.url = self._url
+        self._configuration.ping_url = self._ping_url
+        self._configuration.account_name = self._account_name
+        self._configuration.registration_password = \
+            self._registration_password
+        self._configuration.computer_title = self._computer_title
+        self._configuration.https_proxy = self._https_proxy
+        self._configuration.ping_url = self._ping_url
+        self._configuration.write()
+        self._modified = False
 
=== added directory 'landscape/ui/controller/tests'
=== added file 'landscape/ui/controller/tests/__init__.py'
=== added file 'landscape/ui/controller/tests/test_app.py'
--- landscape/ui/controller/tests/test_app.py	1970-01-01 00:00:00 +0000
+++ landscape/ui/controller/tests/test_app.py	2012-01-09 11:38:25 +0000
@@ -0,0 +1,124 @@
+import sys
+
+try:
+    from gi.repository import Gtk
+    got_gobject_introspection = True
+except ImportError:
+    got_gobject_introspection = False
+    gobject_skip_message = "GObject Introspection module unavailable"
+
+
+from landscape.tests.helpers import LandscapeTest
+from landscape.ui.controller.app import SettingsApplicationController
+from landscape.ui.controller.configuration import ConfigController
+from landscape.ui.view.configuration import ClientSettingsDialog
+from landscape.configuration import LandscapeSetupConfiguration
+
+
+class ConnectionRecordingSettingsApplicationController(
+    SettingsApplicationController):
+
+    _connections = set()
+    _connection_args = {}
+    _connection_kwargs = {}
+
+    def __init__(self, get_config_f=None):
+        super(ConnectionRecordingSettingsApplicationController,
+              self).__init__()
+        if get_config_f:
+            self.get_config = get_config_f
+
+    def _make_connection_name(self, signal, func):
+        return signal + ">" + func.__name__
+
+    def _record_connection(self, signal, func, *args, **kwargs):
+        connection_name = self._make_connection_name(signal, func)
+        self._connections.add(connection_name)
+        signal_connection_args = self._connection_args.get(
+            connection_name, [])
+        signal_connection_args.append(repr(args))
+        self._connection_args = signal_connection_args
+        signal_connection_kwargs = self._connection_kwargs.get(
+            connection_name, [])
+        signal_connection_kwargs.append(repr(kwargs))
+        self._connection_kwargs = signal_connection_kwargs
+
+    def is_connected(self, signal, func):
+        connection_name = self._make_connection_name(signal, func)
+        return self._connections.issuperset(set([connection_name]))
+
+    def connect(self, signal, func, *args, **kwargs):
+        self._record_connection(signal, func)
+
+
+class SettingsApplicationControllerInitTest(LandscapeTest):
+
+    def setUp(self):
+        super(SettingsApplicationControllerInitTest, self).setUp()
+
+    def test_init(self):
+        """
+        Test we connect activate to something useful on application
+        initialisation.
+        """
+        app = ConnectionRecordingSettingsApplicationController()
+        self.assertTrue(app.is_connected("activate", app.setup_ui))
+
+    if not got_gobject_introspection:
+        test_init.skip = gobject_skip_message
+
+
+class SettingsApplicationControllerUISetupTest(LandscapeTest):
+
+    def setUp(self):
+        super(SettingsApplicationControllerUISetupTest, self).setUp()
+
+        def fake_run(obj):
+            """
+            Retard X11 mapping.
+            """
+            pass
+
+        self._real_run = Gtk.Dialog.run
+        Gtk.Dialog.run = fake_run
+
+        def get_config():
+            configdata = "[client]\n"
+            configdata += "data_path = %s\n" % sys.path[0]
+            configdata += "http_proxy = http://proxy.localdomain:3192\n"
+            configdata += "tags = a_tag\n"
+            configdata += \
+                "url = https://landscape.canonical.com/message-system\n"
+            configdata += "account_name = foo\n"
+            configdata += "registration_password = bar\n"
+            configdata += "computer_title = baz\n"
+            configdata += "https_proxy = https://proxy.localdomain:6192\n"
+            configdata += "ping_url = http://landscape.canonical.com/ping\n"
+            config_filename = self.makeFile(configdata)
+
+            class MyLandscapeSetupConfiguration(LandscapeSetupConfiguration):
+                default_config_filenames = [config_filename]
+
+            config = MyLandscapeSetupConfiguration()
+            return config
+
+        self.app = ConnectionRecordingSettingsApplicationController(
+            get_config_f=get_config)
+
+    def tearDown(self):
+        Gtk.Dialog.run = self._real_run
+        super(
+            SettingsApplicationControllerUISetupTest, self).tearDown()
+
+    def test_setup_ui(self):
+        """
+        Test that we correctly setup the L{ClientSettingsDialog} with
+        the config object and correct data
+        """
+        self.app.setup_ui(data=None)
+        self.assertIsInstance(self.app.settings_dialog, ClientSettingsDialog)
+        self.assertIsInstance(self.app.settings_dialog.controller,
+                              ConfigController)
+
+    if not got_gobject_introspection:
+        test_setup_ui.skip = gobject_skip_message
 
=== added file 'landscape/ui/controller/tests/test_configuration.py'
--- landscape/ui/controller/tests/test_configuration.py	1970-01-01 00:00:00 +0000
+++ landscape/ui/controller/tests/test_configuration.py	2012-01-09 11:38:25 +0000
@@ -0,0 +1,223 @@
+from landscape.tests.helpers import LandscapeTest
+from landscape.ui.controller.configuration import (
+    ConfigController, ConfigControllerLockError)
+from landscape.configuration import LandscapeSetupConfiguration
+
+
+class ConfigControllerTest(LandscapeTest):
+
+    def setUp(self):
+        super(ConfigControllerTest, self).setUp()
+        config = "[client]"
+        config += "data_path = /var/lib/landscape/client\n"
+        config += "http_proxy = http://proxy.localdomain:3192\n"
+        config += "tags = a_tag\n"
+        config += "url = https://landscape.canonical.com/message-system\n"
+        config += "account_name = foo\n"
+        config += "registration_password = bar\n"
+        config += "computer_title = baz\n"
+        config += "https_proxy = https://proxy.localdomain:6192\n"
+        config += "ping_url = http://landscape.canonical.com/ping\n"
+        self.config_filename = self.makeFile(config)
+
+        class MyLandscapeSetupConfiguration(LandscapeSetupConfiguration):
+            default_config_filenames = [self.config_filename]
+
+        self.config = MyLandscapeSetupConfiguration()
+        self.controller = ConfigController(self.config)
+
+    def test_init(self):
+        """
+        Test that when we create a controller it has initial state read in
+        directly from the configuration file.
+        """
+        self.controller.load()
+        self.assertEqual(self.controller.data_path,
+                         "/var/lib/landscape/client/")
+        self.assertEqual(self.controller.http_proxy,
+                         "http://proxy.localdomain:3192")
+        self.assertEqual(self.controller.tags, "a_tag")
+        self.assertEqual(self.controller.url,
+                         "https://landscape.canonical.com/message-system")
+        self.assertEqual(self.controller.account_name, "foo")
+        self.assertEqual(self.controller.registration_password, "bar")
+        self.assertEqual(self.controller.computer_title, "baz")
+        self.assertEqual(self.controller.https_proxy,
+                         "https://proxy.localdomain:6192")
+        self.assertEqual(self.controller.ping_url,
+                         "http://landscape.canonical.com/ping")
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.canonical.com")
+
+    def test_set_server_hostname(self):
+        """
+        Test we can set the server_hostname correctly, and derive L{url} and
+        L{ping_url} from it.
+        """
+        self.controller.load()
+        self.assertEqual(self.controller.url,
+                         "https://landscape.canonical.com/message-system")
+        self.assertEqual(self.controller.ping_url,
+                         "http://landscape.canonical.com/ping")
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.canonical.com")
+        new_server_host_name = "landscape.localdomain"
+        self.controller.server_host_name = new_server_host_name
+        self.assertEqual(self.controller.server_host_name,
+                         new_server_host_name)
+        self.assertEqual(self.controller.url,
+                         "https://landscape.localdomain/message-system")
+        self.assertEqual(self.controller.ping_url,
+                         "http://landscape.localdomain/ping")
+
+    def test_setting_server_host_name_also_sets_hosted(self):
+        """
+        Test that when we set the L{server_host_name} the L{hosted} value is
+        also derived.
+        """
+        self.controller.load()
+        self.assertTrue(self.controller.hosted)
+        self.controller.server_host_name = "landscape.localdomain"
+        self.assertFalse(self.controller.hosted)
+        self.controller.server_host_name = "landscape.canonical.com"
+        self.assertTrue(self.controller.hosted)
+
+    def test_set_account_name(self):
+        """
+        Test that we can set the L{account_name} property.
+        """
+        self.controller.load()
+        self.assertEqual(self.controller.account_name, "foo")
+        self.controller.account_name = "shoe"
+        self.assertEqual(self.controller.account_name, "shoe")
+
+    def test_set_registration_password(self):
+        """
+        Test that we can set the L{registration_password} property.
+        """
+        self.controller.load()
+        self.assertEquals(self.controller.registration_password, "bar")
+        self.controller.registration_password = "nucker"
+        self.assertEquals(self.controller.registration_password, "nucker")
+
+    def test_revert(self):
+        """
+        Test that we can revert the controller to it's initial state.
+        """
+        self.controller.load()
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.canonical.com")
+        self.controller.server_host_name = "landscape.localdomain"
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.localdomain")
+        self.controller.revert()
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.canonical.com")
+
+    def test_is_modified(self):
+        """
+        Test that we can determine when something has been modified.
+        """
+        self.controller.load()
+        self.assertFalse(self.controller.is_modified)
+        self.controller.server_host_name = "bing.bang.a.bang"
+        self.assertTrue(self.controller.is_modified)
+        self.controller.revert()
+        self.assertFalse(self.controller.is_modified)
+        self.controller.account_name = "soldierBlue"
+        self.assertTrue(self.controller.is_modified)
+        self.controller.revert()
+        self.assertFalse(self.controller.is_modified)
+        self.controller.registration_password = "HesAnIndianCowboyInTheRodeo"
+        self.assertTrue(self.controller.is_modified)
+
+    def test_commit(self):
+        """
+        Test that we can write configuration settings back to the config file.
+        """
+        self.controller.load()
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.canonical.com")
+        self.controller.server_host_name = "landscape.localdomain"
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.localdomain")
+        self.controller.commit()
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.localdomain")
+        self.controller.revert()
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.localdomain")
+
+    def test_lock(self):
+        """
+        Test that we can lock out updates.
+        """
+        self.controller.load()
+        self.controller.lock()
+        self.assertRaises(ConfigControllerLockError, setattr, self.controller,
+                          "server_host_name", "faily.fail.com")
+        self.assertFalse(self.controller.is_modified)
+        self.controller.unlock()
+        self.controller.server_host_name = "successy.success.org"
+        self.assertTrue(self.controller.is_modified)
+
+        self.controller.revert()
+        self.assertFalse(self.controller.is_modified)
+
+        self.controller.lock()
+        self.assertRaises(ConfigControllerLockError,
+                          setattr,
+                          self.controller,
+                          "account_name",
+                          "Failbert")
+        self.assertFalse(self.controller.is_modified)
+        self.controller.unlock()
+        self.assertFalse(self.controller.is_modified)
+        self.controller.account_name = "Winbob"
+        self.assertTrue(self.controller.is_modified)
+
+        self.controller.revert()
+        self.assertFalse(self.controller.is_modified)
+
+        self.controller.lock()
+        self.assertRaises(ConfigControllerLockError,
+                          setattr,
+                          self.controller,
+                          "registration_password",
+                          "I Fail")
+        self.assertFalse(self.controller.is_modified)
+        self.controller.unlock()
+        self.assertFalse(self.controller.is_modified)
+        self.controller.registration_password = "I Win"
+        self.assertTrue(self.controller.is_modified)
+
+    def test_defaulting(self):
+        """
+        Test we set the correct values when switching between hosted and
+        dedicated.
+        """
+        self.makeFile("", path=self.config_filename)  # empty the config file
+        self.controller.load()
+        self.assertEqual(self.controller.account_name, None)
+        self.assertEqual(self.controller.registration_password, None)
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.canonical.com")
+        self.controller.default_dedicated()
+        self.assertEqual(self.controller.account_name, None)
+        self.assertEqual(self.controller.registration_password, None)
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.localdomain")
+        self.controller.default_hosted()
+        self.assertEqual(self.controller.account_name, None)
+        self.assertEqual(self.controller.registration_password, None)
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.canonical.com")
+        self.controller.default_dedicated()
+        self.controller.server_host_name = "test.machine"
+        self.controller.default_dedicated()
+        self.assertEqual(self.controller.server_host_name, "test.machine")
+        self.controller.default_hosted()
+        self.assertEqual(self.controller.server_host_name,
+                         "landscape.canonical.com")
+        self.controller.default_dedicated()
+        self.assertEqual(self.controller.server_host_name, "test.machine")
 
=== added directory 'landscape/ui/model'
=== added file 'landscape/ui/model/__init__.py'
=== added directory 'landscape/ui/view'
=== added file 'landscape/ui/view/__init__.py'
=== added file 'landscape/ui/view/configuration.py'
--- landscape/ui/view/configuration.py	1970-01-01 00:00:00 +0000
+++ landscape/ui/view/configuration.py	2012-01-09 11:38:25 +0000
@@ -0,0 +1,152 @@
+import os
+
+from gi.repository import Gtk
+
+
+class ClientSettingsDialog(Gtk.Dialog):
+
+    GLADE_FILE = "landscape-client-settings.glade"
+
+    def __init__(self, controller, data_path=None, *args, **kwargs):
+        super(ClientSettingsDialog, self).__init__(*args, **kwargs)
+        self.controller = controller
+        self._ui_path = os.path.join(
+            os.path.dirname(__file__), "ui",
+            ClientSettingsDialog.GLADE_FILE)
+        self._builder = Gtk.Builder()
+        self._builder.add_from_file(self._ui_path)
+        self._setup_ui()
+        self._hosted_toggle = None
+        self._dedicated_toggle = None
+        self.revert(self._revert_button)
+
+    def _setup_window(self):
+        """
+        Configure the dialog window and pack content from the Glade UI file
+        into the main content area.
+        """
+        self.set_title("Client Settings")
+        content_area = self.get_content_area()
+        vbox = self._builder.get_object(
+            "landscape-client-settings-dialog-vbox")
+        vbox.unparent()
+        content_area.pack_start(vbox, expand=True, fill=True, padding=0)
+
+    def _setup_action_buttons(self):
+        """
+        Obtain handles for action buttons and connect them to handlers.
+        """
+        self._close_button = self._builder.get_object("close-button")
+        self._revert_button = self._builder.get_object("revert-button")
+        self._revert_button.connect("clicked", self.revert)
+        self._close_button.connect("clicked", self._possibly_save_and_exit)
+
+    def _setup_entries(self):
+        """
+        Obtain handles for entry widgets, set initial state and connect them to
+        handlers.
+        """
+        self._account_entry = self._builder.get_object("account-name-entry")
+        self._password_entry = self._builder.get_object(
+            "reigstered-password-entry")
+        self._server_host_name_entry = self._builder.get_object(
+            "server-host-name-entry")
+        self._account_entry.set_sensitive(False)
+        self._password_entry.set_sensitive(False)
+        self._server_host_name_entry.set_sensitive(False)
+        self._account_entry.connect("changed", self._update_account)
+        self._password_entry.connect("changed", self._update_password)
+        self._server_host_name_entry.connect("changed",
+                                             self._update_server_host_name)
+
+    def _setup_radiobuttons(self):
+        """
+        Obtain handles on radiobuttons and connect them to handler.
+        """
+        self._hosted_radiobutton = self._builder.get_object(
+            "hosted-radiobutton")
+        self._dedicated_radiobutton = self._builder.get_object(
+            "dedicated-radiobutton")
+
+    def _setup_ui(self):
+        self._setup_window()
+        self._setup_radiobuttons()
+        self._setup_entries()
+        self._setup_action_buttons()
+
+    def _load_data(self):
+        self.controller.lock()
+        if not self.controller.account_name is None:
+            self._account_entry.set_text(self.controller.account_name)
+        if not self.controller.registration_password is None:
+            self._password_entry.set_text(
+                self.controller.registration_password)
+        if not self.controller.server_host_name is None:
+            self._server_host_name_entry.set_text(
+                self.controller.server_host_name)
+        self.controller.unlock()
+
+    def _update_account(self, event):
+        if not self.controller.is_locked():
+            self.controller.account_name = self._account_entry.get_text()
+
+    def _update_password(self, event):
+        if not self.controller.is_locked():
+            self.controller.registration_password = \
+                self._password_entry.get_text()
+
+    def _update_server_host_name(self, event):
+        if not self.controller.is_locked():
+            self.controller.server_host_name = \
+                self._server_host_name_entry.get_text()
+
+    def _set_entry_sensitivity(self, hosted):
+        if hosted:
+            self._account_entry.set_sensitive(True)
+            self._password_entry.set_sensitive(True)
+            self._server_host_name_entry.set_sensitive(False)
+        else:
+            self._account_entry.set_sensitive(False)
+            self._password_entry.set_sensitive(False)
+            self._server_host_name_entry.set_sensitive(True)
+
+    def select_landscape_hosting(self):
+        hosted = self._hosted_radiobutton.get_active()
+        self._set_entry_sensitivity(hosted)
+        if hosted:
+            self.controller.default_hosted()
+            self._load_data()
+        else:
+            self.controller.default_dedicated()
+            self._load_data()
+
+    def _on_toggle_server_type_radiobutton(self, radiobutton):
+        self.select_landscape_hosting()
+        return True
+
+    def revert(self, button):
+        self.controller.revert()
+        if self._hosted_toggle:
+            self._hosted_radiobutton.disconnect(self._hosted_toggle)
+        if self._dedicated_toggle:
+            self._dedicated_radiobutton.disconnect(self._dedicated_toggle)
+        if self.controller.hosted:
+            self._hosted_radiobutton.set_active(True)
+        else:
+            self._dedicated_radiobutton.set_active(True)
+        self._set_entry_sensitivity(self.controller.hosted)
+        self._load_data()
+        self._hosted_toggle = self._hosted_radiobutton.connect(
+            "toggled",
+            self._on_toggle_server_type_radiobutton)
+        self._dedicated_toggle = self._dedicated_radiobutton.connect(
+            "toggled",
+            self._on_toggle_server_type_radiobutton)
+
+    def _write_back(self):
+        self.controller.commit()
+
+    def _possibly_save_and_exit(self, button):
+        if self.controller.is_modified:
+            self._write_back()
+        self.destroy()
 
=== added directory 'landscape/ui/view/tests'
=== added file 'landscape/ui/view/tests/__init__.py'
=== added file 'landscape/ui/view/tests/test_configuration.py'
--- landscape/ui/view/tests/test_configuration.py	1970-01-01 00:00:00 +0000
+++ landscape/ui/view/tests/test_configuration.py	2012-01-09 11:38:25 +0000
@@ -0,0 +1,270 @@
+import sys
+
+try:
+    from gi.repository import Gtk
+    got_gobject_introspection = True
+except ImportError:
+    got_gobject_introspection = False
+    gobject_skip_message = "GObject Introspection module unavailable"
+
+from landscape.tests.helpers import LandscapeTest
+from landscape.configuration import LandscapeSetupConfiguration
+from landscape.ui.view.configuration import ClientSettingsDialog
+from landscape.ui.controller.configuration import ConfigController
+
+
+class ConfigurationViewTest(LandscapeTest):
+
+    def setUp(self):
+        super(ConfigurationViewTest, self).setUp()
+        config = "[client]\n"
+        config += "data_path = %s\n" % sys.path[0]
+        config += "http_proxy = http://proxy.localdomain:3192\n"
+        config += "tags = a_tag\n"
+        config += "url = https://landscape.canonical.com/message-system\n"
+        config += "account_name = foo\n"
+        config += "registration_password = bar\n"
+        config += "computer_title = baz\n"
+        config += "https_proxy = https://proxy.localdomain:6192\n"
+        config += "ping_url = http://landscape.canonical.com/ping\n"
+
+        self.config_filename = self.makeFile(config)
+
+        class MySetupConfiguration(LandscapeSetupConfiguration):
+            default_config_filenames = [self.config_filename]
+
+        self.config = MySetupConfiguration()
+
+    def test_init(self):
+        """
+        Test that we correctly initialise the L{ConfigurationView} correctly
+        from the controller.
+        """
+        controller = ConfigController(self.config)
+        dialog = ClientSettingsDialog(controller)
+        content_area = dialog.get_content_area()
+        children = content_area.get_children()
+        self.assertEqual(len(children), 2)
+        box = children[0]
+        self.assertIsInstance(box, Gtk.Box)
+        self.assertTrue(dialog._hosted_radiobutton.get_active())
+        self.assertFalse(dialog._dedicated_radiobutton.get_active())
+        self.assertTrue(dialog._account_entry.get_sensitive())
+        self.assertTrue(dialog._password_entry.get_sensitive())
+        self.assertFalse(dialog._server_host_name_entry.get_sensitive())
+
+    def test_toggle_radio_button(self):
+        """
+        Test that we disable and enable the correct entries when we toggle the
+        dialog radiobuttons.
+        """
+        controller = ConfigController(self.config)
+        dialog = ClientSettingsDialog(controller)
+        self.assertTrue(dialog._hosted_radiobutton.get_active())
+        self.assertFalse(dialog._dedicated_radiobutton.get_active())
+        self.assertTrue(dialog._account_entry.get_sensitive())
+        self.assertTrue(dialog._password_entry.get_sensitive())
+        self.assertFalse(dialog._server_host_name_entry.get_sensitive())
+        dialog._dedicated_radiobutton.set_active(True)
+        self.assertFalse(dialog._hosted_radiobutton.get_active())
+        self.assertTrue(dialog._dedicated_radiobutton.get_active())
+        self.assertFalse(dialog._account_entry.get_sensitive())
+        self.assertFalse(dialog._password_entry.get_sensitive())
+        self.assertTrue(dialog._server_host_name_entry.get_sensitive())
+        dialog._hosted_radiobutton.set_active(True)
+        self.assertTrue(dialog._hosted_radiobutton.get_active())
+        self.assertFalse(dialog._dedicated_radiobutton.get_active())
+        self.assertTrue(dialog._account_entry.get_sensitive())
+        self.assertTrue(dialog._password_entry.get_sensitive())
+        self.assertFalse(dialog._server_host_name_entry.get_sensitive())
+
+    def test_load_data_from_config(self):
+        """
+        Test that we load data into the appropriate entries from the
+        configuration file.
+        """
+        controller = ConfigController(self.config)
+        dialog = ClientSettingsDialog(controller)
+        self.assertEqual(dialog._account_entry.get_text(), "foo")
+        self.assertEqual(dialog._password_entry.get_text(), "bar")
+        self.assertEqual(dialog._server_host_name_entry.get_text(),
+                         "landscape.canonical.com")
+
+    def test_revert(self):
+        """
+        Test that we can revert the UI values using the controller.
+        """
+        controller = ConfigController(self.config)
+        dialog = ClientSettingsDialog(controller)
+        self.assertEqual(dialog._account_entry.get_text(), "foo")
+        self.assertEqual(dialog._password_entry.get_text(), "bar")
+        self.assertEqual(dialog._server_host_name_entry.get_text(),
+                         "landscape.canonical.com")
+        dialog._dedicated_radiobutton.set_active(True)
+        dialog._server_host_name_entry.set_text("more.barn")
+        self.assertEqual(dialog._account_entry.get_text(), "foo")
+        self.assertEqual(dialog._password_entry.get_text(), "bar")
+        self.assertEqual(dialog._server_host_name_entry.get_text(),
+                         "more.barn")
+        self.assertTrue(dialog._dedicated_radiobutton.get_active())
+        self.assertFalse(dialog._hosted_radiobutton.get_active())
+        dialog.revert(None)
+        self.assertEqual(dialog._account_entry.get_text(), "foo")
+        self.assertEqual(dialog._password_entry.get_text(), "bar")
+        self.assertEqual(dialog._server_host_name_entry.get_text(),
+                         "landscape.canonical.com")
+        self.assertFalse(dialog._dedicated_radiobutton.get_active())
+        self.assertTrue(dialog._hosted_radiobutton.get_active())
+
+    if not got_gobject_introspection:
+        test_revert.skip = gobject_skip_message
+        test_load_data_from_config.skip = gobject_skip_message
+        test_toggle_radio_button.skip = gobject_skip_message
+        test_init.skip = gobject_skip_message
+
+
+class ConfigurationViewCommitTest(LandscapeTest):
+
+    def setUp(self):
+        super(ConfigurationViewCommitTest, self).setUp()
+        config = "[client]\n"
+        config += "data_path = %s\n" % sys.path[0]
+        config += "http_proxy = http://proxy.localdomain:3192\n"
+        config += "tags = a_tag\n"
+        config += "url = https://landscape.canonical.com/message-system\n"
+        config += "account_name = foo\n"
+        config += "registration_password = bar\n"
+        config += "computer_title = baz\n"
+        config += "https_proxy = https://proxy.localdomain:6192\n"
+        config += "ping_url = http://landscape.canonical.com/ping\n"
+        self.config_filename = self.makeFile(config)
+
+        class MySetupConfiguration(LandscapeSetupConfiguration):
+            default_config_filenames = [self.config_filename]
+
+        self.config = MySetupConfiguration()
+        self.real_write_back = ClientSettingsDialog._write_back
+        self.write_back_called = False
+
+        def fake_write_back(obj):
+            self.write_back_called = True
+
+        ClientSettingsDialog._write_back = fake_write_back
+        self.controller = ConfigController(self.config)
+        self.dialog = ClientSettingsDialog(self.controller)
+
+    def tearDown(self):
+        self.controller = None
+        self.dialog.destroy()
+        self.dialog = None
+        while Gtk.events_pending():
+            Gtk.main_iteration()
+        self.write_back_called = False
+        ClientSettingsDialog._write_back = self.real_write_back
+        super(ConfigurationViewCommitTest, self).tearDown()
+
+    def test_commit_fresh_dialog(self):
+        """
+        Test that we don't save anything from an untouched dialog on exit
+        """
+        while Gtk.events_pending():
+            Gtk.main_iteration()
+        self.dialog._possibly_save_and_exit(None)
+        self.assertFalse(self.write_back_called)
+
+    def test_commit_hosted_account_name_change(self):
+        """
+        Test that we do save changes when we set a new account name for hosted
+        accounts.
+        """
+        self.dialog._hosted_radiobutton.set_active(True)
+        while Gtk.events_pending():
+            Gtk.main_iteration()
+        self.dialog._account_entry.set_text("glow")
+        while Gtk.events_pending():
+            Gtk.main_iteration()
+        self.dialog._possibly_save_and_exit(None)
+        self.assertTrue(self.write_back_called)
+
+    def test_commit_hosted_password_change(self):
+        """
+        Test that we do save changes when we set a new account name for hosted
+        accounts.
+        """
+        self.dialog._hosted_radiobutton.set_active(True)
+        self.dialog._password_entry.set_text("sticks")
+        while Gtk.events_pending():
+            Gtk.main_iteration()
+        self.dialog._possibly_save_and_exit(None)
+        self.assertTrue(self.write_back_called)
+
+    def test_commit_dedicated_server_host_name_change(self):
+        """
+        Test that we do save changes when we set a new server host name for
+        a dedicated server.
+        """
+        self.dialog._dedicated_radiobutton.set_active(True)
+        self.dialog._server_host_name_entry.set_text(
+            "that.isolated.geographic")
+        while Gtk.events_pending():
+            Gtk.main_iteration()
+        self.dialog._possibly_save_and_exit(None)
+        self.assertTrue(self.write_back_called)
+
+    if not got_gobject_introspection:
+        test_commit_dedicated_server_host_name_change.skip = \
+            gobject_skip_message
+        test_commit_hosted_password_change.skip = gobject_skip_message
+        test_commit_hosted_account_name_change.skip = gobject_skip_message
+        test_commit_fresh_dialog.skip = gobject_skip_message
+
+
+class DedicatedConfigurationViewTest(LandscapeTest):
+
+    def setUp(self):
+        super(DedicatedConfigurationViewTest, self).setUp()
+        config = "[client]\n"
+        config += "data_path = %s\n" % sys.path[0]
+        config += "url = https://landscape.localdomain/message-system\n"
+        config += "computer_title = baz\n"
+        config += "ping_url = http://landscape.localdomain/ping\n"
+        self.config_filename = self.makeFile(config)
+
+        class MySetupConfiguration(LandscapeSetupConfiguration):
+            default_config_filenames = [self.config_filename]
+
+        self.config = MySetupConfiguration()
+
+    def test_init(self):
+        """
+        Test that we correctly initialise the L{ConfigurationView} correctly
+        from the controller.
+        """
+        controller = ConfigController(self.config)
+        dialog = ClientSettingsDialog(controller)
+        content_area = dialog.get_content_area()
+        children = content_area.get_children()
+        self.assertEqual(len(children), 2)
+        box = children[0]
+        self.assertIsInstance(box, Gtk.Box)
+        self.assertFalse(dialog._hosted_radiobutton.get_active())
+        self.assertTrue(dialog._dedicated_radiobutton.get_active())
+        self.assertFalse(dialog._account_entry.get_sensitive())
+        self.assertFalse(dialog._password_entry.get_sensitive())
+        self.assertTrue(dialog._server_host_name_entry.get_sensitive())
+
+    def test_load_data_from_config(self):
+        """
+        Test that we load data into the appropriate entries from the
+        configuration file.
+        """
+        controller = ConfigController(self.config)
+        dialog = ClientSettingsDialog(controller)
+        self.assertEqual(dialog._account_entry.get_text(), "")
+        self.assertEqual(dialog._password_entry.get_text(), "")
+        self.assertEqual(dialog._server_host_name_entry.get_text(),
+                         "landscape.localdomain")
+
+    if not got_gobject_introspection:
+        test_load_data_from_config.skip = gobject_skip_message
+        test_init.skip = gobject_skip_message
 
=== added directory 'landscape/ui/view/ui'
=== added file 'landscape/ui/view/ui/landscape-client-settings.glade'
--- landscape/ui/view/ui/landscape-client-settings.glade	1970-01-01 00:00:00 +0000
+++ landscape/ui/view/ui/landscape-client-settings.glade	2012-01-09 11:38:25 +0000
@@ -0,0 +1,407 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<interface>
+  <!-- interface-requires gtk+ 3.0 -->
+  <object class="GtkDialog" id="landscape-client-settings-dialog">
+    <property name="can_focus">False</property>
+    <property name="border_width">5</property>
+    <property name="title" translatable="yes">Landscape Client Settings</property>
+    <property name="resizable">False</property>
+    <property name="modal">True</property>
+    <property name="window_position">center</property>
+    <property name="icon_name">landscape-client-settings</property>
+    <property name="type_hint">dialog</property>
+    <property name="has_resize_grip">False</property>
+    <child internal-child="vbox">
+      <object class="GtkBox" id="landscape-client-settings-dialog-vbox">
+        <property name="can_focus">False</property>
+        <property name="orientation">vertical</property>
+        <property name="spacing">2</property>
+        <child internal-child="action_area">
+          <object class="GtkButtonBox" id="dialog-action_area1">
+            <property name="can_focus">False</property>
+            <property name="layout_style">end</property>
+            <child>
+              <object class="GtkButton" id="revert-button">
+                <property name="label">gtk-revert-to-saved</property>
+                <property name="visible">True</property>
+                <property name="can_focus">True</property>
+                <property name="receives_default">True</property>
+                <property name="use_action_appearance">False</property>
+                <property name="use_stock">True</property>
+              </object>
+              <packing>
+                <property name="expand">False</property>
+                <property name="fill">True</property>
+                <property name="position">0</property>
+              </packing>
+            </child>
+            <child>
+              <object class="GtkButton" id="close-button">
+                <property name="label">gtk-close</property>
+                <property name="visible">True</property>
+                <property name="can_focus">True</property>
+                <property name="receives_default">True</property>
+                <property name="use_action_appearance">False</property>
+                <property name="use_stock">True</property>
+              </object>
+              <packing>
+                <property name="expand">False</property>
+                <property name="fill">True</property>
+                <property name="position">1</property>
+              </packing>
+            </child>
+          </object>
+          <packing>
+            <property name="expand">False</property>
+            <property name="fill">True</property>
+            <property name="pack_type">end</property>
+            <property name="position">0</property>
+          </packing>
+        </child>
+        <child>
+          <object class="GtkBox" id="vbox">
+            <property name="visible">True</property>
+            <property name="can_focus">False</property>
+            <property name="orientation">vertical</property>
+            <property name="spacing">6</property>
+            <child>
+              <object class="GtkFrame" id="landscape-server-type-frame">
+                <property name="visible">True</property>
+                <property name="can_focus">False</property>
+                <property name="label_xalign">0</property>
+                <property name="shadow_type">none</property>
+                <child>
+                  <object class="GtkAlignment" id="landscape-server-type-frame-alignment">
+                    <property name="visible">True</property>
+                    <property name="can_focus">False</property>
+                    <property name="hexpand">True</property>
+                    <property name="vexpand">True</property>
+                    <property name="top_padding">6</property>
+                    <property name="left_padding">12</property>
+                    <child>
+                      <object class="GtkBox" id="server-type-options-vbox">
+                        <property name="visible">True</property>
+                        <property name="can_focus">False</property>
+                        <property name="hexpand">True</property>
+                        <property name="vexpand">True</property>
+                        <property name="orientation">vertical</property>
+                        <property name="spacing">12</property>
+                        <child>
+                          <object class="GtkBox" id="hosted-hbox">
+                            <property name="visible">True</property>
+                            <property name="can_focus">False</property>
+                            <property name="hexpand">True</property>
+                            <property name="vexpand">True</property>
+                            <child>
+                              <object class="GtkAlignment" id="hosted-radiobutton-alignment">
+                                <property name="visible">True</property>
+                                <property name="can_focus">False</property>
+                                <property name="yalign">0</property>
+                                <child>
+                                  <object class="GtkRadioButton" id="hosted-radiobutton">
+                                    <property name="visible">True</property>
+                                    <property name="can_focus">True</property>
+                                    <property name="receives_default">False</property>
+                                    <property name="halign">start</property>
+                                    <property name="valign">start</property>
+                                    <property name="use_action_appearance">False</property>
+                                    <property name="xalign">0</property>
+                                    <property name="active">True</property>
+                                    <property name="draw_indicator">True</property>
+                                    <property name="group">dedicated-radiobutton</property>
+                                    <child>
+                                      <object class="GtkLabel" id="hosted-radiobutton-label">
+                                        <property name="visible">True</property>
+                                        <property name="can_focus">False</property>
+                                        <property name="xalign">0</property>
+                                        <attributes>
+                                          <attribute name="weight" value="bold"/>
+                                        </attributes>
+                                      </object>
+                                    </child>
+                                  </object>
+                                </child>
+                              </object>
+                              <packing>
+                                <property name="expand">False</property>
+                                <property name="fill">True</property>
+                                <property name="position">0</property>
+                              </packing>
+                            </child>
+                            <child>
+                              <object class="GtkFrame" id="hosted-frame">
+                                <property name="visible">True</property>
+                                <property name="can_focus">False</property>
+                                <property name="hexpand">True</property>
+                                <property name="label_xalign">0</property>
+                                <property name="shadow_type">none</property>
+                                <child>
+                                  <object class="GtkAlignment" id="hosted-frame-alignment">
+                                    <property name="visible">True</property>
+                                    <property name="can_focus">False</property>
+                                    <property name="left_padding">12</property>
+                                    <child>
+                                      <object class="GtkBox" id="hosted-vbox">
+                                        <property name="visible">True</property>
+                                        <property name="can_focus">False</property>
+                                        <property name="hexpand">True</property>
+                                        <property name="vexpand">True</property>
+                                        <property name="orientation">vertical</property>
+                                        <property name="spacing">6</property>
+                                        <child>
+                                          <object class="GtkBox" id="account-hbox">
+                                            <property name="visible">True</property>
+                                            <property name="can_focus">False</property>
+                                            <property name="hexpand">True</property>
+                                            <property name="spacing">12</property>
+                                            <child>
+                                              <object class="GtkLabel" id="account-label">
+                                                <property name="visible">True</property>
+                                                <property name="can_focus">False</property>
+                                                <property name="xalign">0</property>
+                                                <property name="label" translatable="yes">Account name: </property>
+                                              </object>
+                                              <packing>
+                                                <property name="expand">False</property>
+                                                <property name="fill">True</property>
+                                                <property name="position">0</property>
+                                              </packing>
+                                            </child>
+                                            <child>
+                                              <object class="GtkEntry" id="account-name-entry">
+                                                <property name="visible">True</property>
+                                                <property name="can_focus">True</property>
+                                                <property name="hexpand">True</property>
+                                                <property name="invisible_char">•</property>
+                                                <property name="invisible_char_set">True</property>
+                                              </object>
+                                              <packing>
+                                                <property name="expand">True</property>
+                                                <property name="fill">True</property>
+                                                <property name="position">1</property>
+                                              </packing>
+                                            </child>
+                                          </object>
+                                          <packing>
+                                            <property name="expand">True</property>
+                                            <property name="fill">True</property>
+                                            <property name="position">0</property>
+                                          </packing>
+                                        </child>
+                                        <child>
+                                          <object class="GtkBox" id="password-hbox">
+                                            <property name="visible">True</property>
+                                            <property name="can_focus">False</property>
+                                            <property name="spacing">12</property>
+                                            <child>
+                                              <object class="GtkLabel" id="password-label">
+                                                <property name="visible">True</property>
+                                                <property name="can_focus">False</property>
+                                                <property name="xalign">0</property>
+                                                <property name="label" translatable="yes">Password:</property>
+                                              </object>
+                                              <packing>
+                                                <property name="expand">False</property>
+                                                <property name="fill">True</property>
+                                                <property name="position">0</property>
+                                              </packing>
+                                            </child>
+                                            <child>
+                                              <object class="GtkEntry" id="reigstered-password-entry">
+                                                <property name="visible">True</property>
+                                                <property name="can_focus">True</property>
+                                                <property name="hexpand">True</property>
+                                                <property name="visibility">False</property>
+                                                <property name="invisible_char">•</property>
+                                                <property name="invisible_char_set">True</property>
+                                              </object>
+                                              <packing>
+                                                <property name="expand">True</property>
+                                                <property name="fill">True</property>
+                                                <property name="position">1</property>
+                                              </packing>
+                                            </child>
+                                          </object>
+                                          <packing>
+                                            <property name="expand">True</property>
+                                            <property name="fill">True</property>
+                                            <property name="position">1</property>
+                                          </packing>
+                                        </child>
+                                      </object>
+                                    </child>
+                                  </object>
+                                </child>
+                                <child type="label">
+                                  <object class="GtkLabel" id="hosted-frame-label">
+                                    <property name="visible">True</property>
+                                    <property name="can_focus">False</property>
+                                    <property name="margin_top">4</property>
+                                    <property name="label" translatable="yes">&lt;b&gt;Hosted&lt;/b&gt;</property>
+                                    <property name="use_markup">True</property>
+                                  </object>
+                                </child>
+                              </object>
+                              <packing>
+                                <property name="expand">False</property>
+                                <property name="fill">True</property>
+                                <property name="position">1</property>
+                              </packing>
+                            </child>
+                          </object>
+                          <packing>
+                            <property name="expand">True</property>
+                            <property name="fill">True</property>
+                            <property name="position">0</property>
+                          </packing>
+                        </child>
+                        <child>
+                          <object class="GtkBox" id="dedicated-hbox">
+                            <property name="visible">True</property>
+                            <property name="can_focus">False</property>
+                            <child>
+                              <object class="GtkRadioButton" id="dedicated-radiobutton">
+                                <property name="visible">True</property>
+                                <property name="can_focus">True</property>
+                                <property name="receives_default">False</property>
+                                <property name="valign">start</property>
+                                <property name="use_action_appearance">False</property>
+                                <property name="xalign">0</property>
+                                <property name="active">True</property>
+                                <property name="draw_indicator">True</property>
+                                <child>
+                                  <object class="GtkAlignment" id="alignment3">
+                                    <property name="visible">True</property>
+                                    <property name="can_focus">False</property>
+                                    <child>
+                                      <object class="GtkLabel" id="dedicated-radiobutton-label">
+                                        <property name="visible">True</property>
+                                        <property name="can_focus">False</property>
+                                        <property name="xalign">0</property>
+                                        <attributes>
+                                          <attribute name="weight" value="bold"/>
+                                        </attributes>
+                                      </object>
+                                    </child>
+                                  </object>
+                                </child>
+                              </object>
+                              <packing>
+                                <property name="expand">False</property>
+                                <property name="fill">True</property>
+                                <property name="position">0</property>
+                              </packing>
+                            </child>
+                            <child>
+                              <object class="GtkFrame" id="dedicated-frame">
+                                <property name="visible">True</property>
+                                <property name="can_focus">False</property>
+                                <property name="margin_top">4</property>
+                                <property name="hexpand">True</property>
+                                <property name="label_xalign">0</property>
+                                <property name="shadow_type">none</property>
+                                <child>
+                                  <object class="GtkAlignment" id="dedicated-frame-alignment">
+                                    <property name="visible">True</property>
+                                    <property name="can_focus">False</property>
+                                    <property name="left_padding">12</property>
+                                    <child>
+                                      <object class="GtkBox" id="host-name-hbox">
+                                        <property name="visible">True</property>
+                                        <property name="can_focus">False</property>
+                                        <property name="hexpand">True</property>
+                                        <property name="spacing">12</property>
+                                        <child>
+                                          <object class="GtkLabel" id="server-host-name-label">
+                                            <property name="visible">True</property>
+                                            <property name="can_focus">False</property>
+                                            <property name="xalign">0</property>
+                                            <property name="label" translatable="yes">Server host name:</property>
+                                          </object>
+                                          <packing>
+                                            <property name="expand">False</property>
+                                            <property name="fill">True</property>
+                                            <property name="position">0</property>
+                                          </packing>
+                                        </child>
+                                        <child>
+                                          <object class="GtkEntry" id="server-host-name-entry">
+                                            <property name="visible">True</property>
+                                            <property name="can_focus">True</property>
+                                            <property name="hexpand">True</property>
+                                            <property name="invisible_char">•</property>
+                                            <property name="invisible_char_set">True</property>
+                                          </object>
+                                          <packing>
+                                            <property name="expand">True</property>
+                                            <property name="fill">True</property>
+                                            <property name="position">1</property>
+                                          </packing>
+                                        </child>
+                                      </object>
+                                    </child>
+                                  </object>
+                                </child>
+                                <child type="label">
+                                  <object class="GtkLabel" id="dedicated-frame-label">
+                                    <property name="visible">True</property>
+                                    <property name="can_focus">False</property>
+                                    <property name="label" translatable="yes">&lt;b&gt;Dedicated server&lt;/b&gt;</property>
+                                    <property name="use_markup">True</property>
+                                  </object>
+                                </child>
+                              </object>
+                              <packing>
+                                <property name="expand">False</property>
+                                <property name="fill">True</property>
+                                <property name="position">1</property>
+                              </packing>
+                            </child>
+                          </object>
+                          <packing>
+                            <property name="expand">True</property>
+                            <property name="fill">True</property>
+                            <property name="position">1</property>
+                          </packing>
+                        </child>
+                      </object>
+                    </child>
+                  </object>
+                </child>
+                <child type="label">
+                  <object class="GtkLabel" id="landscape-server-type-label">
+                    <property name="visible">True</property>
+                    <property name="can_focus">False</property>
+                    <property name="label" translatable="yes">&lt;b&gt;Landscape Server Type:&lt;/b&gt;</property>
+                    <property name="use_markup">True</property>
+                  </object>
+                </child>
+              </object>
+              <packing>
+                <property name="expand">False</property>
+                <property name="fill">True</property>
+                <property name="position">0</property>
+              </packing>
+            </child>
+          </object>
+          <packing>
+            <property name="expand">False</property>
+            <property name="fill">True</property>
+            <property name="position">1</property>
+          </packing>
+        </child>
+      </object>
+    </child>
+    <action-widgets>
+      <action-widget response="1">revert-button</action-widget>
+      <action-widget response="0">close-button</action-widget>
+    </action-widgets>
+  </object>
+  <object class="GtkSizeGroup" id="property-label-sizegroup">
+    <widgets>
+      <widget name="server-host-name-label"/>
+      <widget name="password-label"/>
+      <widget name="account-label"/>
+    </widgets>
+  </object>
+</interface>
 
=== added file 'scripts/landscape-client-settings-ui'
--- scripts/landscape-client-settings-ui	1970-01-01 00:00:00 +0000
+++ scripts/landscape-client-settings-ui	2012-01-09 11:38:25 +0000
@@ -0,0 +1,17 @@
+#!/usr/bin/python 
+import os
+import sys
+
+script_dir = os.path.abspath("scripts")
+if os.path.dirname(os.path.abspath(sys.argv[0])) == script_dir:
+    sys.path.insert(0, "./")
+else:
+    from landscape.lib.warning import hide_warnings
+    hide_warnings()
+
+from landscape.ui.controller.app import SettingsApplicationController
+
+
+if __name__ == "__main__":
+    app = SettingsApplicationController(args=sys.argv[1:])
+    app.run(None)