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

[Geoff Teale]

The second part of the fix for bug #931937,

This branch does the following:

 1. Removes the TestedGoodState and TestedBadState to reflect the decision not to tackne testing of configurations in this phase.
 2. Performs 3 stages of data setup:
     i. use code level defaults in the VirginState
     ii. overwrite user visible values with current values in GSettings (VirginState -> Initialised)
     iii. overwrite either hosted or local values from the configuration file values (VirginState -> Initialised)
 3. Persist data in two places:
     i. write all user visible data back to GSettings.
     ii. write either hosted or local values to the configuration file.

[Free Ekanayaka (free.ekanayaka)]

Great stuff, +1!

[1]

+class UISettings(object):

Please add a class docstring here.

[2]

+ def get(self, *args):

and

+ def set(self, *args):

Please add method docstrings explaining what these methods do in detail, as they seem to have non trivial logic.

[3]

+ settings = FakeGSettings(data=self.default_data)
+ uisettings = UISettings(settings)
+ model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)

There are a lot of these, I guess they could be moved to setUp, unless you prefer to have them there because they need slightly different parameters for each test or you like to have less context for a test.

review: Approve

[Alberto Donato (ack)]

Looks great! +1

Just minor stylistic notes:

#1:
+ if self._computer_title in ("", None):

you could use "if not self._computer_title" here. (there are a few of these).

#2:
+ self.assertEqual(True, state.get(IS_HOSTED))

please use assertTrue here.

#3:
+ self.config_string = "[client]\n" \

please use parentheses rather than breaking with \

+ assert(self.schema.attrib["id"] == \
+ "com.canonical.landscape-client-settings")

same here

Thanks!

review: Approve

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

483. By Geoff Teale on 2012-02-29

removed unused GSettings event handlers

484. By Geoff Teale on 2012-02-29

Added docstring (from Free's review)

485. By Geoff Teale on 2012-02-29

Docstring and comments on complex get/set machinary (from Free's review)

486. By Geoff Teale on 2012-02-29

Simplify empty value comparisons (from Ack's review).

487. By Geoff Teale on 2012-02-29

Replace assertEqual(True, ... with assertTrue(... (from Ack's review).

488. By Geoff Teale on 2012-02-29

Use brackets instead of line continuation (from Ack's review).

=== added directory 'glib-2.0'
=== added directory 'glib-2.0/schemas'
=== added file 'glib-2.0/schemas/com.canonical.landscape-client-settings.gschema.xml'
--- glib-2.0/schemas/com.canonical.landscape-client-settings.gschema.xml	1970-01-01 00:00:00 +0000
+++ glib-2.0/schemas/com.canonical.landscape-client-settings.gschema.xml	2012-02-29 16:50:20 +0000
@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="utf-8"?>
+<schemalist>
+  <schema id="com.canonical.landscape-client-settings" path="/com/canonical/landscape-client-settings/">
+    <key type="b" name="is-hosted">
+      <default>true</default>
+      <summary>Whether the client settings UI currently set to a hosted client configuration.</summary>
+    </key>
+    <key type="s" name="computer-title">
+      <default></default>
+      <summary>The title to register the machine with.</summary>
+    </key>
+    <key type="s" name="hosted-landscape-host">
+      <default>canonical.landscape.com</default>
+      <summary>The hostname of the Canonical hosted landscape system.</summary>
+    </key>
+    <key type="s" name="hosted-account-name">
+      <default></default>
+      <summary>An account name for use with the Canonical hosted landscape system.</summary>
+    </key>
+    <key type="s" name="hosted-password">
+      <default></default>
+      <summary>A password for use with the Canonical hosted landscape system</summary>
+    </key>
+    <key type="s" name="local-landscape-host">
+      <default></default>
+      <summary>The hostname of the local landscape system.</summary>
+    </key>
+    <key type="s" name="local-account-name">
+      <default></default>
+      <summary>An account name for use with the local landscape system.</summary>
+    </key>
+    <key type="s" name="local-password">
+      <default></default>
+      <summary>A password for use with the local landscape system</summary>
+    </key>
+  </schema>
+</schemalist>
 
=== modified file 'landscape/ui/controller/configuration.py'
--- landscape/ui/controller/configuration.py	2012-02-03 18:10:57 +0000
+++ landscape/ui/controller/configuration.py	2012-02-29 16:50:20 +0000
@@ -1,6 +1,9 @@
 import socket
 import threading
 from landscape.ui.model.registration.proxy import RegistrationProxy
+from landscape.ui.model.configuration.state import (
+    derive_url_from_host_name, derive_ping_url_from_host_name,
+    derive_server_host_name_from_url)
 
 
 class ConfigControllerLockError(Exception):
@@ -65,7 +68,7 @@
         """
         Default machine name to FQDN.
         """
-        if self._computer_title is None:
+        if self._computer_title in ("", None):
             self._computer_title = self.getfqdn()
 
     def default_dedicated(self):
@@ -78,9 +81,8 @@
             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._url = derive_url_from_host_name(self._server_host_name)
+            self._ping_url = derive_ping_url_from_host_name(
                 self._server_host_name)
         self.modify()
 
@@ -91,10 +93,8 @@
         """
         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._url = derive_url_from_host_name(self._server_host_name)
+        self._ping_url = derive_ping_url_from_host_name(self._server_host_name)
         self._account_name = self._initial_account_name
         self.modify()
 
@@ -117,7 +117,7 @@
         self._ping_url = self._configuration.ping_url
         if self._url:
             self._server_host_name = \
-                self._derive_server_host_name_from_url(self._url)
+                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
@@ -143,29 +143,6 @@
         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
 
@@ -178,9 +155,8 @@
             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._url = derive_url_from_host_name(self._server_host_name)
+            self._ping_url = derive_ping_url_from_host_name(
                 self._server_host_name)
             self.modify()
             self._lock.release()
 
=== modified file 'landscape/ui/controller/tests/test_configuration.py'
--- landscape/ui/controller/tests/test_configuration.py	2012-02-06 22:29:12 +0000
+++ landscape/ui/controller/tests/test_configuration.py	2012-02-29 16:50:20 +0000
@@ -224,19 +224,19 @@
         dedicated.
         """
         self.controller.load()
-        self.assertEqual(None, self.controller.account_name)
-        self.assertEqual(None, self.controller.registration_password)
+        self.assertEqual("", self.controller.account_name)
+        self.assertEqual("", self.controller.registration_password)
         self.assertEqual("landscape.canonical.com",
                          self.controller.server_host_name)
         self.controller.account_name = "Bungle"
         self.controller.default_dedicated()
         self.assertEqual("standalone", self.controller.account_name)
-        self.assertEqual(None, self.controller.registration_password)
+        self.assertEqual("", self.controller.registration_password)
         self.assertEqual("landscape.localdomain",
                          self.controller.server_host_name)
         self.controller.default_hosted()
-        self.assertEqual(None, self.controller.account_name)
-        self.assertEqual(None, self.controller.registration_password)
+        self.assertEqual("", self.controller.account_name)
+        self.assertEqual("", self.controller.registration_password)
         self.assertEqual("landscape.canonical.com",
                          self.controller.server_host_name)
         self.controller.default_dedicated()
 
=== modified file 'landscape/ui/lib/polkit.py'
--- landscape/ui/lib/polkit.py	2012-01-23 09:52:49 +0000
+++ landscape/ui/lib/polkit.py	2012-02-29 16:50:20 +0000
@@ -1,7 +1,8 @@
 import dbus
 import dbus.service
 import dbus.glib
-import gobject
+
+from gi.repository import GObject
 
 
 class PolicyKitMechanism(dbus.service.Object):
@@ -111,5 +112,5 @@
     """
     Invoke a L{gobject.MainLoop} to process incoming DBus events.
     """
-    mainloop = gobject.MainLoop()
+    mainloop = GObject.MainLoop()
     mainloop.run()
 
=== modified file 'landscape/ui/model/configuration/mechanism.py'
--- landscape/ui/model/configuration/mechanism.py	2012-01-24 10:12:49 +0000
+++ landscape/ui/model/configuration/mechanism.py	2012-02-29 16:50:20 +0000
@@ -81,8 +81,9 @@
             except AttributeError:
                 return ""
             if value is None:
-                return None
+                return ""
             return str(value)
+        return ""
 
     @dbus.service.method(INTERFACE_NAME,
                          in_signature="ss",
 
=== modified file 'landscape/ui/model/configuration/proxy.py'
--- landscape/ui/model/configuration/proxy.py	2012-01-24 10:10:31 +0000
+++ landscape/ui/model/configuration/proxy.py	2012-02-29 16:50:20 +0000
@@ -22,7 +22,8 @@
     The canonical case for this is L{landscape-client-settings-ui}.
     """
 
-    def __init__(self, bus=None, interface=None):
+    def __init__(self, bus=None, interface=None, loadargs=[]):
+        self._loadargs = loadargs
         if bus is None:
             self._bus = dbus.SystemBus()
         else:
@@ -34,7 +35,9 @@
             self._interface = interface
 
     def load(self, arglist):
-        if arglist is None or len(arglist) == 0:
+        if arglist is None:
+            arglist = self._loadargs
+        if len(arglist) == 0:
             al = ""
         else:
             al = chr(0x1e).join(arglist)
 
=== modified file 'landscape/ui/model/configuration/state.py'
--- landscape/ui/model/configuration/state.py	2012-02-23 11:38:49 +0000
+++ landscape/ui/model/configuration/state.py	2012-02-29 16:50:20 +0000
@@ -1,3 +1,74 @@
+import copy
+import socket
+
+from landscape.ui.model.configuration.proxy import ConfigurationProxy
+
+
+HOSTED_LANDSCAPE_HOST = "landscape.canonical.com"
+LOCAL_LANDSCAPE_HOST = ""
+
+HOSTED_ACCOUNT_NAME = ""
+LOCAL_ACCOUNT_NAME = ""
+
+HOSTED_PASSWORD = ""
+LOCAL_PASSWORD = ""
+
+HOSTED = "hosted"
+LOCAL = "local"
+IS_HOSTED = "is-hosted"
+COMPUTER_TITLE = "computer-title"
+LANDSCAPE_HOST = "landscape-host"
+ACCOUNT_NAME = "account-name"
+PASSWORD = "password"
+
+
+def get_fqdn():
+    """
+    Wrap socket.getfqdn so we can test reliably.
+    """
+    return socket.getfqdn()
+
+
+DEFAULT_DATA = {
+    IS_HOSTED: True,
+    COMPUTER_TITLE: get_fqdn(),
+    HOSTED: {
+        LANDSCAPE_HOST: HOSTED_LANDSCAPE_HOST,
+        ACCOUNT_NAME: HOSTED_ACCOUNT_NAME,
+        PASSWORD: HOSTED_PASSWORD,
+        },
+    LOCAL: {
+        LANDSCAPE_HOST: LOCAL_LANDSCAPE_HOST,
+        ACCOUNT_NAME: LOCAL_ACCOUNT_NAME,
+        PASSWORD: LOCAL_PASSWORD,
+        }
+}
+
+
+def derive_server_host_name_from_url(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(host_name):
+    "Extrapolate a url from a host name."
+    #Reuse this code to make sure it's a proper host name
+    host_name = derive_server_host_name_from_url(host_name)
+    return "https://" + host_name + "/message-system"
+
+
+def derive_ping_url_from_host_name(host_name):
+    "Extrapolate a ping_url from a host name."
+    #Reuse this code to make sure it's a proper host name
+    host_name = derive_server_host_name_from_url(host_name)
+    return "http://" + host_name + "/ping"
 
 
 class StateError(Exception):
@@ -9,15 +80,68 @@
 
 class ConfigurationState(object):
     """
-    Abstract base class for states used in the L{ConfigurationModel}.
+    Base class for states used in the L{ConfigurationModel}.
     """
+    def __init__(self, data, proxy, uisettings):
+        self._data = copy.deepcopy(data)
+        self._proxy = proxy
+        self._uisettings = uisettings
+
+    def get(self, *args):
+        """
+        Retrieve only valid values from two level dictionary based tree.
+
+        This mainly served to pick up programming errors and could easily be
+        replaced with a simpler scheme.
+        """
+        arglen = len(args)
+        if arglen > 2 or arglen == 0:
+            raise TypeError(
+                "get() takes either 1 or 2 keys (%d given)" % arglen)
+        if arglen == 2:  # We're looking for a leaf on a branch
+            sub_dict = None
+            if args[0] in [HOSTED, LOCAL]:
+                sub_dict = self._data.get(args[0], {})
+            sub_dict = self._data[args[0]]
+            if not isinstance(sub_dict, dict):
+                raise KeyError(
+                    "Compound key [%s][%s] is invalid. The data type " +
+                    "returned from the first index was %s." %
+                    sub_dict.__class__.__name__)
+            return sub_dict.get(args[1], None)
+        else: # we looking for a leaf at the root
+            if args[0] in (IS_HOSTED, COMPUTER_TITLE):
+                return self._data.get(args[0], None)
+            else:
+                raise KeyError("Key [%s] is invalid. " % args[0])
+
+    def set(self, *args):
+        """
+        Set only valid values from two level dictionary based tree.
+
+        This mainly served to pick up programming errors and could easily be
+        replaced with a simpler scheme.
+        """
+        arglen = len(args)
+        if arglen < 2 or arglen > 3:
+            raise TypeError("set() takes either 1 or 2 keys and exactly 1 " +
+                            "value (%d arguments given)" % arglen)
+        if arglen == 2: # We're setting a leaf attached to the root
+            self._data[args[0]] = args[1]
+        else: # We're setting a leaf on a branch
+            sub_dict = None
+            if args[0] in [HOSTED, LOCAL]:
+                sub_dict = self._data.get(args[0], {})
+            if not isinstance(sub_dict, dict):
+                raise KeyError("Compound key [%s][%s] is invalid. The data " +
+                               "type returned from the first index was %s."
+                               % sub_dict.__class__.__name__)
+            sub_dict[args[1]] = args[2]
+            self._data[args[0]] = sub_dict
 
     def load_data(self):
         raise NotImplementedError
 
-    def test(self, test_method):
-        raise NotImplementedError
-
     def modify(self):
         raise NotImplementedError
 
@@ -28,16 +152,32 @@
         raise NotImplementedError
 
 
-class ModifiableHelper(object):
+class Helper(object):
+    """
+    Base class for all state transition helpers.
+
+    It is assumed that the Helper classes are "friends" of the
+    L{ConfigurationState} classes and can have some knowledge of their
+    internals.  They shouldn't be visible to users of the
+    L{ConfigurationState}s and in general we should avoid seeing the
+    L{ConfigurationState}'s _data attribute outside this module.
+    """
+
+    def __init__(self, state):
+        self._state = state
+
+
+class ModifiableHelper(Helper):
     """
     Allow a L{ConfigurationState}s to be modified.
     """
 
     def modify(self):
-        return ModifiedState()
-
-
-class UnloadableHelper(object):
+        return ModifiedState(self._state._data, self._state._proxy,
+                             self._state._uisettings)
+
+
+class UnloadableHelper(Helper):
     """
     Disallow loading of data into a L{ConfigurationModel}.
     """
@@ -48,7 +188,7 @@
                          " cannot be transitioned via load_data()")
 
 
-class UnmodifiableHelper(object):
+class UnmodifiableHelper(Helper):
     """
     Disallow modification of a L{ConfigurationState}.
     """
@@ -59,39 +199,17 @@
                          " cannot transition via modify()")
 
 
-class TestableHelper(object):
-    """
-    Allow testing of a L{ConfigurationModel}.
-    """
-
-    def test(self, test_method):
-        if test_method():
-            return TestedGoodState()
-        else:
-            return TestedBadState()
-
-
-class UntestableHelper(object):
-    """
-    Disallow testing of a L{ConfigurationModel}.
-    """
-
-    def test(self, test_method):
-        raise StateError("A ConfigurationModel in " +
-                         self.__class__.__name__ +
-                         " cannot transition via test()")
-
-
-class RevertableHelper(object):
+class RevertableHelper(Helper):
     """
     Allow reverting of a L{ConfigurationModel}.
     """
 
     def revert(self):
-        return InitialisedState()
-
-
-class UnrevertableHelper(object):
+        return InitialisedState(self._state._data, self._state._proxy,
+                                self._state._uisettings)
+
+
+class UnrevertableHelper(Helper):
     """
     Disallow reverting of a L{ConfigurationModel}.
     """
@@ -102,16 +220,50 @@
                          " cannot transition via revert()")
 
 
-class PersistableHelper(object):
+class PersistableHelper(Helper):
     """
     Allow a L{ConfigurationModel} to persist.
     """
 
+    def _save_to_uisettings(self):
+        self._state._uisettings.set_is_hosted(self._state.get(IS_HOSTED))
+        self._state._uisettings.set_computer_title(
+            self._state.get(COMPUTER_TITLE))
+        self._state._uisettings.set_hosted_account_name(
+            self._state.get(HOSTED, ACCOUNT_NAME))
+        self._state._uisettings.set_hosted_password(
+            self._state.get(HOSTED, PASSWORD))
+        self._state._uisettings.set_local_landscape_host(
+            self._state.get(LOCAL, LANDSCAPE_HOST))
+        self._state._uisettings.set_local_account_name(
+            self._state.get(LOCAL, ACCOUNT_NAME))
+        self._state._uisettings.set_local_password(
+            self._state.get(LOCAL, PASSWORD))
+
+    def _save_to_config(self):
+        if self._state.get(IS_HOSTED):
+            first_key = HOSTED
+        else:
+            first_key = LOCAL
+        self._state._proxy.url = derive_url_from_host_name(
+            self._state.get(first_key, LANDSCAPE_HOST))
+        self._state._proxy.ping_url = derive_ping_url_from_host_name(
+            self._state.get(first_key, LANDSCAPE_HOST))
+        self._state._proxy.account_name = \
+            self._state.get(first_key, ACCOUNT_NAME)
+        self._state._proxy.registration_password = \
+            self._state.get(first_key, PASSWORD)
+        self._state._proxy.computer_title = self._state.get(COMPUTER_TITLE)
+        self._state._proxy.write()
+
     def persist(self):
-        return InitialisedState()
-
-
-class UnpersistableHelper(object):
+        self._save_to_uisettings()
+        self._save_to_config()
+        return InitialisedState(self._state._data, self._state._proxy,
+                                self._state._uisettings)
+
+
+class UnpersistableHelper(Helper):
     """
     Disallow persistence of a L{ConfigurationModel}.
     """
@@ -125,70 +277,20 @@
 class ModifiedState(ConfigurationState):
     """
     The state of a L{ConfigurationModel} whenever the user has modified some
-    data but hasn't yet L{test}ed or L{revert}ed.
-    """
-
-    _modifiable_helper = ModifiableHelper()
-    _revertable_helper = RevertableHelper()
-    _testable_helper = TestableHelper()
-    _unpersistable_helper = UnpersistableHelper()
-
-    def modify(self):
-        return self._modifiable_helper.modify()
-
-    def revert(self):
-        return self._revertable_helper.revert()
-
-    def test(self, test_method):
-        return self._testable_helper.test(test_method)
-
-    def persist(self):
-        return self._unpersistable_helper.persist()
-
-
-class TestedState(ConfigurationState):
-    """
-    A superclass for the two possible L{TestedStates} (L{TestedGoodState} and
-    L{TestedBadState}).
-    """
-
-    _untestable_helper = UntestableHelper()
-    _unloadable_helper = UnloadableHelper()
-    _modifiable_helper = ModifiableHelper()
-    _revertable_helper = RevertableHelper()
-
-    def test(self, test_method):
-        return self._untestable_helper.test(test_method)
-
-    def load_data(self):
-        return self._unloadable_helper.load_data()
-
-    def modify(self):
-        return self._modifiable_helper.modify()
-
-    def revert(self):
-        return self._revertable_helper.revert()
-
-
-class TestedBadState(TestedState):
-    """
-    The state of a L{ConfigurationModel} after it has been L{test}ed but that
-    L{test} has failed for some reason.
-    """
-
-    _unpersistable_helper = UnpersistableHelper()
-
-    def persist(self):
-        return self._unpersistable_helper.persist()
-
-
-class TestedGoodState(TestedState):
-    """
-    The state of a L{ConfigurationModel} after it has been L{test}ed
-    successfully.
-    """
-
-    _persistable_helper = PersistableHelper()
+    data but hasn't yet L{persist}ed or L{revert}ed.
+    """
+
+    def __init__(self, data, proxy, uisettings):
+        super(ModifiedState, self).__init__(data, proxy, uisettings)
+        self._modifiable_helper = ModifiableHelper(self)
+        self._revertable_helper = RevertableHelper(self)
+        self._persistable_helper = PersistableHelper(self)
+
+    def modify(self):
+        return self._modifiable_helper.modify()
+
+    def revert(self):
+        return self._revertable_helper.revert()
 
     def persist(self):
         return self._persistable_helper.persist()
@@ -202,10 +304,45 @@
     finally defaults should be applied where necessary.
     """
 
-    _modifiable_helper = ModifiableHelper()
-    _unrevertable_helper = UnrevertableHelper()
-    _testable_helper = TestableHelper()
-    _unpersistable_helper = UnpersistableHelper()
+    def __init__(self, data, proxy, uisettings):
+        super(InitialisedState, self).__init__(data, proxy, uisettings)
+        self._modifiable_helper = ModifiableHelper(self)
+        self._unrevertable_helper = UnrevertableHelper(self)
+        self._unpersistable_helper = UnpersistableHelper(self)
+        self._load_uisettings_data()
+        self._load_live_data()
+
+    def _load_uisettings_data(self):
+        self.set(IS_HOSTED, self._uisettings.get_is_hosted())
+        computer_title = self._uisettings.get_computer_title()
+        if computer_title:
+            self.set(COMPUTER_TITLE, computer_title)
+        self.set(HOSTED, LANDSCAPE_HOST,
+                 self._uisettings.get_hosted_landscape_host())
+        self.set(HOSTED, ACCOUNT_NAME,
+                 self._uisettings.get_hosted_account_name())
+        self.set(HOSTED, PASSWORD, self._uisettings.get_hosted_password())
+        self.set(LOCAL, LANDSCAPE_HOST,
+                 self._uisettings.get_local_landscape_host())
+        self.set(LOCAL, ACCOUNT_NAME,
+                 self._uisettings.get_local_account_name())
+        self.set(LOCAL, PASSWORD, self._uisettings.get_local_password())
+
+    def _load_live_data(self):
+        self._proxy.load(None)
+        computer_title = self._proxy.computer_title
+        if computer_title:
+            self.set(COMPUTER_TITLE, computer_title)
+        url = self._proxy.url
+        if url.find(HOSTED_LANDSCAPE_HOST) > -1:
+            self.set(IS_HOSTED, True)
+            self.set(HOSTED, ACCOUNT_NAME, self._proxy.account_name)
+            self.set(HOSTED, PASSWORD, self._proxy.registration_password)
+        else:
+            self.set(IS_HOSTED, False)
+            self.set(LOCAL, LANDSCAPE_HOST,
+                     derive_server_host_name_from_url(url))
+            self.set(LOCAL, ACCOUNT_NAME, self._proxy.account_name)
 
     def load_data(self):
         return self
@@ -216,9 +353,6 @@
     def revert(self):
         return self._unrevertable_helper.revert()
 
-    def test(self, test_method):
-        return self._testable_helper.test(test_method)
-
     def persist(self):
         return self._unpersistable_helper.persist()
 
@@ -229,16 +363,14 @@
     upon it.
     """
 
-    _untestable_helper = UntestableHelper()
-    _unmodifiable_helper = UnmodifiableHelper()
-    _unrevertable_helper = UnrevertableHelper()
-    _unpersistable_helper = UnpersistableHelper()
+    def __init__(self, proxy, uisettings):
+        super(VirginState, self).__init__(DEFAULT_DATA, proxy, uisettings)
+        self._unmodifiable_helper = UnmodifiableHelper(self)
+        self._unrevertable_helper = UnrevertableHelper(self)
+        self._unpersistable_helper = UnpersistableHelper(self)
 
     def load_data(self):
-        return InitialisedState()
-
-    def test(self, test_method):
-        return self._untestable_helper.test(test_method)
+        return InitialisedState(self._data, self._proxy, self._uisettings)
 
     def modify(self):
         return self._unmodifiable_helper.modify()
@@ -266,29 +398,15 @@
 
        VirginState      --(load_data)--> InitialisedState
        InitialisedState --(modify)-----> ModifiedState
-       InitialisedState --(test)-------> TestedGoodState
-       InitialisedState --(test)-------> TestedBadState
        ModifiedState    --(revert)-----> InitialisedState
        ModifiedState    --(modify)-----> ModifiedState
-       ModifiedState    --(test)-------> TestedGoodState
-       ModifiedState    --(test)-------> TestedBadState
-       TestedGoodState  --(revert)-----> InitialisedState
-       TestedGoodState  --(persist)----> InitialisedState
-       TestedGoodState  --(modify)-----> ModifiedState
-       TestedBadState   --(revert)-----> InitialisedState
-       TestedBadState   --(modify)-----> ModifiedState
+       ModifiedState    --(persist)----> InitialisedState
     """
 
-    def __init__(self, test_method=None):
-        self._current_state = VirginState()
-        if test_method:
-            self._test_method = test_method
-        else:
-            self._test_method = self._test
-
-    def _test(self):
-        # TODO, dump this and use something real
-        return True
+    def __init__(self, proxy=None, proxy_loadargs=[], uisettings=None):
+        if not proxy:
+            proxy = ConfigurationProxy(loadargs=proxy_loadargs)
+        self._current_state = VirginState(proxy, uisettings)
 
     def get_state(self):
         """
@@ -299,9 +417,6 @@
     def load_data(self):
         self._current_state = self._current_state.load_data()
 
-    def test(self):
-        self._current_state = self._current_state.test(self._test_method)
-
     def modify(self):
         self._current_state = self._current_state.modify()
 
@@ -310,3 +425,73 @@
 
     def persist(self):
         self._current_state = self._current_state.persist()
+
+    def _get_is_hosted(self):
+        return self._current_state.get(IS_HOSTED)
+
+    def _set_is_hosted(self, value):
+        self._current_state.set(IS_HOSTED, value)
+
+    is_hosted = property(_get_is_hosted, _set_is_hosted)
+
+    def _get_computer_title(self):
+        return self._current_state.get(COMPUTER_TITLE)
+
+    def _set_computer_title(self, value):
+        self._current_state.set(COMPUTER_TITLE, value)
+
+    computer_title = property(_get_computer_title, _set_computer_title)
+
+    def _get_hosted_landscape_host(self):
+        return self._current_state.get(HOSTED, LANDSCAPE_HOST)
+
+    def _set_hosted_landscape_host(self, value):
+        self._current_state.set(HOSTED, LANDSCAPE_HOST, value)
+
+    hosted_landscape_host = property(_get_hosted_landscape_host,
+                                     _set_hosted_landscape_host)
+
+    def _get_local_landscape_host(self):
+        return self._current_state.get(LOCAL, LANDSCAPE_HOST)
+
+    def _set_local_landscape_host(self, value):
+        self._current_state.set(LOCAL, LANDSCAPE_HOST, value)
+
+    local_landscape_host = property(_get_local_landscape_host,
+                                    _set_local_landscape_host)
+
+    def _get_hosted_account_name(self):
+        return self._current_state.get(HOSTED, ACCOUNT_NAME)
+
+    def _set_hosted_account_name(self, value):
+        self._current_state.set(HOSTED, ACCOUNT_NAME, value)
+
+    hosted_account_name = property(_get_hosted_account_name,
+                                   _set_hosted_account_name)
+
+    def _get_local_account_name(self):
+        return self._current_state.get(LOCAL, ACCOUNT_NAME)
+
+    def _set_local_account_name(self, value):
+        self._current_state.set(LOCAL, ACCOUNT_NAME, value)
+
+    local_account_name = property(_get_local_account_name,
+                                   _set_local_account_name)
+
+    def _get_hosted_password(self):
+        return self._current_state.get(HOSTED, PASSWORD)
+
+    def _set_hosted_password(self, value):
+        self._current_state.set(HOSTED, PASSWORD, value)
+
+    hosted_password = property(_get_hosted_password,
+                               _set_hosted_password)
+
+    def _get_local_password(self):
+        return self._current_state.get(LOCAL, PASSWORD)
+
+    def _set_local_password(self, value):
+        self._current_state.set(LOCAL, PASSWORD, value)
+
+    local_password = property(_get_local_password,
+                              _set_local_password)
 
=== modified file 'landscape/ui/model/configuration/tests/test_state.py'
--- landscape/ui/model/configuration/tests/test_state.py	2012-02-21 10:37:58 +0000
+++ landscape/ui/model/configuration/tests/test_state.py	2012-02-29 16:50:20 +0000
@@ -1,7 +1,293 @@
 from landscape.tests.helpers import LandscapeTest
+from landscape.ui.model.configuration.uisettings import UISettings
+import landscape.ui.model.configuration.state
 from landscape.ui.model.configuration.state import (
     ConfigurationModel, StateError, VirginState, InitialisedState,
-    TestedGoodState, TestedBadState, ModifiedState)
+    ModifiedState, IS_HOSTED, HOSTED, LOCAL, HOSTED_LANDSCAPE_HOST,
+    LANDSCAPE_HOST, COMPUTER_TITLE)
+from landscape.ui.tests.helpers import ConfigurationProxyHelper, FakeGSettings
+
+
+class ConfigurationModelTest(LandscapeTest):
+    """
+    Test the internal data handling of the L{ConfigurationModel} without
+    loading external data.
+    """
+
+    helpers = [ConfigurationProxyHelper]
+
+    default_data = {"is-hosted": True,
+                    "computer-title": "",
+                    "hosted-landscape-host": "",
+                    "hosted-account-name": "",
+                    "hosted-password": "",
+                    "local-landscape-host": "",
+                    "local-account-name": "",
+                    "local-password": ""
+                    }
+
+    def setUp(self):
+        self.config_string = ""
+        landscape.ui.model.configuration.state.DEFAULT_DATA[COMPUTER_TITLE] \
+            = "bound.to.lose"
+        super(ConfigurationModelTest, self).setUp()
+
+    def tearDown(self):
+        super(ConfigurationModelTest, self).tearDown()
+        self.proxy = None
+
+    def test_get(self):
+        """
+        Test that L{get} correctly extracts data from the internal data storage
+        of the L{ConfigurationState}s associated with a L{ConfigurationModel}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        state = model.get_state()
+        self.assertTrue(state.get(IS_HOSTED))
+        self.assertEqual(HOSTED_LANDSCAPE_HOST,
+                         state.get(HOSTED, LANDSCAPE_HOST))
+        self.assertRaises(TypeError, state.get, IS_HOSTED, HOSTED,
+                          LANDSCAPE_HOST)
+        self.assertRaises(KeyError, state.get, LANDSCAPE_HOST)
+        self.assertRaises(KeyError, state.get, IS_HOSTED, LANDSCAPE_HOST)
+
+    def test_set(self):
+        """
+        Test that L{set} correctly sets data in the internal data storage of
+        the L{ConfigurationState}s associated with a L{ConfigurationModel}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        state = model.get_state()
+        state.set(IS_HOSTED, True)
+        self.assertTrue(state.get(IS_HOSTED))
+        state.set(IS_HOSTED, False)
+        self.assertFalse(state.get(IS_HOSTED))
+        self.assertEqual("", state.get(LOCAL, LANDSCAPE_HOST))
+        state.set(LOCAL, LANDSCAPE_HOST, "goodison.park")
+        self.assertEqual("goodison.park", state.get(LOCAL, LANDSCAPE_HOST))
+
+    def test_virginal(self):
+        """
+        Test that the L{ConfigurationModel} is created with default data.  This
+        should be managed via L{VirginState} (hence the name), but this should
+        not be exposed and is not explicitly tested here (see
+        L{StateTransitionTest}).
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        self.assertTrue(model.is_hosted)
+        self.assertEqual(HOSTED_LANDSCAPE_HOST, model.hosted_landscape_host)
+        self.assertEqual("bound.to.lose", model.computer_title)
+        self.assertEqual("", model.local_landscape_host)
+        self.assertEqual("", model.hosted_account_name)
+        self.assertEqual("", model.local_account_name)
+        self.assertEqual("", model.hosted_password)
+
+    def test_is_hosted_property(self):
+        """
+        Test we can use the L{is_hosted} property to set and get that data on
+        the current L{ConfigurationState}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        model.load_data()
+        self.assertTrue(model.is_hosted)
+        model.is_hosted = False
+        self.assertFalse(model.is_hosted)
+
+    def test_computer_title_property(self):
+        """
+        Test that we can use the L{computer_title} property to set and get that
+        data on the current L{ConfigurationState}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        model.load_data()
+        self.assertEqual("bound.to.lose", model.computer_title)
+        model.computer_title = "bound.to.win"
+        self.assertEqual("bound.to.win", model.computer_title)
+
+    def test_hosted_landscape_host_property(self):
+        """
+        Test we can use the L{hosted_landscape_host} property to set and get
+        that data on the current L{ConfigurationState}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        self.assertEqual(HOSTED_LANDSCAPE_HOST, model.hosted_landscape_host)
+        model.hosted_landscape_host = "foo"
+        self.assertEqual("foo", model.hosted_landscape_host)
+
+    def test_hosted_account_name_property(self):
+        """
+        Test we can use the L{hosted_account_name} property to set and get
+        that data on the current L{ConfigurationState}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        self.assertEqual("", model.hosted_account_name)
+        model.hosted_account_name = "foo"
+        self.assertEqual("foo", model.hosted_account_name)
+
+    def test_hosted_password_property(self):
+        """
+        Test we can use the L{hosted_password} property to set and get
+        that data on the current L{ConfigurationState}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        self.assertEqual("", model.hosted_password)
+        model.hosted_password = "foo"
+        self.assertEqual("foo", model.hosted_password)
+
+    def test_local_landscape_host_property(self):
+        """
+        Test we can use the L{local_landscape_host} property to set and get
+        that data on the current L{ConfigurationState}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        self.assertEqual("", model.local_landscape_host)
+        model.local_landscape_host = "foo"
+        self.assertEqual("foo", model.local_landscape_host)
+
+    def test_local_account_name_property(self):
+        """
+        Test we can use the L{local_account_name} property to set and get
+        that data on the current L{ConfigurationState}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        self.assertEqual("", model.local_account_name)
+        model.local_account_name = "foo"
+        self.assertEqual("foo", model.local_account_name)
+
+    def test_local_password_property(self):
+        """
+        Test we can use the L{local_password} property to set and get
+        that data on the current L{ConfigurationState}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        self.assertEqual("", model.local_password)
+        model.local_password = "foo"
+        self.assertEqual("foo", model.local_password)
+
+
+class ConfigurationModelHostedTest(LandscapeTest):
+    """
+    Test the L{ConfigurationModel} is correctly initialised when the live
+    configuration is set for a hosted account.
+
+    Note the multilayer data loading:
+
+         1. Internal state is defaulted.
+         2. UISettings data is loaded.
+         3. Live configuration is loaded.
+    """
+
+    helpers = [ConfigurationProxyHelper]
+
+    default_data = {"is-hosted": True,
+                    "computer-title": "bound.to.lose",
+                    "hosted-landscape-host": "landscape.canonical.com",
+                    "hosted-account-name": "Sparklehorse",
+                    "hosted-password": "Vivadixiesubmarinetransmissionplot",
+                    "local-landscape-host": "the.local.machine",
+                    "local-account-name": "CrazyHorse",
+                    "local-password": "RustNeverSleeps"
+                    }
+
+    def setUp(self):
+        self.config_string = "[client]\n" \
+            "data_path = /var/lib/landscape/client/\n" \
+            "http_proxy = http://proxy.localdomain:3192\n" \
+            "tags = a_tag\n" \
+            "url = https://landscape.canonical.com/message-system\n" \
+            "account_name = foo\n" \
+            "registration_password = boink\n" \
+            "computer_title = baz\n" \
+            "https_proxy = https://proxy.localdomain:6192\n" \
+            "ping_url = http://landscape.canonical.com/ping\n"
+
+        super(ConfigurationModelHostedTest, self).setUp()
+
+    def test_initialised_hosted(self):
+        """
+        Test the L{ConfigurationModel} is correctly initialised from a proxy
+        and defaults with hosted data.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        model.load_data()
+        self.assertTrue(model.is_hosted)
+        self.assertEqual("landscape.canonical.com",
+                         model.hosted_landscape_host)
+        self.assertEqual("the.local.machine", model.local_landscape_host)
+        self.assertEqual("foo", model.hosted_account_name)
+        self.assertEqual("CrazyHorse", model.local_account_name)
+        self.assertEqual("boink", model.hosted_password)
+
+
+class ConfigurationModelLocalTest(LandscapeTest):
+
+    helpers = [ConfigurationProxyHelper]
+
+    default_data = {"is-hosted": True,
+                    "computer-title": "bound.to.lose",
+                    "hosted-landscape-host": "landscape.canonical.com",
+                    "hosted-account-name": "Sparklehorse",
+                    "hosted-password": "Vivadixiesubmarinetransmissionplot",
+                    "local-landscape-host": "the.local.machine",
+                    "local-account-name": "CrazyHorse",
+                    "local-password": "RustNeverSleeps"
+                    }
+
+    def setUp(self):
+        self.config_string = "[client]\n" \
+            "data_path = /var/lib/landscape/client/\n" \
+            "http_proxy = http://proxy.localdomain:3192\n" \
+            "tags = a_tag\n" \
+            "url = https://landscape.localdomain/message-system\n" \
+            "account_name = foo\n" \
+            "registration_password = boink\n" \
+            "computer_title = baz\n" \
+            "https_proxy = \n" \
+            "ping_url = http://landscape.localdomain/ping\n"
+
+        super(ConfigurationModelLocalTest, self).setUp()
+
+    def test_initialised_local(self):
+        """
+        Test the L{ConfigurationModel} is correctly initialised from a proxy
+        and defaults with local data.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        model.load_data()
+        self.assertFalse(model.is_hosted)
+        self.assertEqual("landscape.canonical.com",
+                         model.hosted_landscape_host)
+        self.assertEqual("landscape.localdomain", model.local_landscape_host)
+        self.assertEqual("Sparklehorse", model.hosted_account_name)
+        self.assertEqual("foo", model.local_account_name)
+        self.assertEqual("Vivadixiesubmarinetransmissionplot",
+                         model.hosted_password)
 
 
 class StateTransitionTest(LandscapeTest):
@@ -10,12 +296,30 @@
     L{ConfigurationModel}.
     """
 
+    helpers = [ConfigurationProxyHelper]
+
+    def setUp(self):
+        self.config_string = ""
+        self.default_data = {
+            "is-hosted": True,
+            "computer-title": "bound.to.lose",
+            "hosted-landscape-host": "landscape.canonical.com",
+            "hosted-account-name": "Sparklehorse",
+            "hosted-password": "Vivadixiesubmarinetransmissionplot",
+            "local-landscape-host": "the.local.machine",
+            "local-account-name": "CrazyHorse",
+            "local-password": "RustNeverSleeps"
+            }
+        super(StateTransitionTest, self).setUp()
+
     def test_load_data_transitions(self):
         """
         Test that the L{ConfigurationModel} correctly changes state as we call
         L{load_data}.
         """
-        model = ConfigurationModel()
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
         self.assertTrue(isinstance(model.get_state(), VirginState))
         model.load_data()
         self.assertTrue(isinstance(model.get_state(), InitialisedState))
@@ -24,54 +328,14 @@
         self.assertTrue(isinstance(model.get_state(), InitialisedState))
         self.assertIs(initialised, model.get_state())
 
-    def test_testing_a_virgin_raises(self):
-        """
-        Test that calling L{test} on a L{ConfigurationModel} in L{VirginState}
-        raises an error.
-        """
-        model = ConfigurationModel()
-        self.assertTrue(isinstance(model.get_state(), VirginState))
-        self.assertRaises(StateError, model.test)
-
-    def test_load_data_on_tested_state_raises(self):
-        """
-        Test that calling L{load_data} on a L{ConfigurationModel} in either one
-        of the two L{TestedState} subclasses (L{TestedGoodState} or
-        L{TestedBadState}) will raise a L{StateError}.
-        """
-        test_succeed = lambda: True
-        test_fail = lambda: False
-        model = ConfigurationModel(test_method=test_succeed)
-        model.load_data()
-        model.test()
-        self.assertRaises(StateError, model.load_data)
-        model = ConfigurationModel(test_method=test_fail)
-        model.load_data()
-        model.test()
-        self.assertRaises(StateError, model.load_data)
-
-    def test_test_transition(self):
-        """
-        Test that the L{ConfigurationModel} transitions to a L{TestedGoodState}
-        or a L{TestedBadState} when L{test} is called.
-        """
-        test_succeed = lambda: True
-        test_fail = lambda: False
-        model = ConfigurationModel(test_method=test_succeed)
-        model.load_data()
-        model.test()
-        self.assertTrue(isinstance(model.get_state(), TestedGoodState))
-        model = ConfigurationModel(test_method=test_fail)
-        model.load_data()
-        model.test()
-        self.assertTrue(isinstance(model.get_state(), TestedBadState))
-
     def test_modifying_a_virgin_raises(self):
         """
         Test that attempting a L{modify} a L{ConfigurationModel} in
         L{VirginState} raises a L{StateError}.
         """
-        model = ConfigurationModel()
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
         self.assertRaises(StateError, model.modify)
 
     def test_initialised_state_is_modifiable(self):
@@ -79,50 +343,28 @@
         Test that the L{ConfigurationModel} transitions to L{ModifiedState}
         whenever L{modify} is called on it in L{InitialisedState}.
         """
-        model = ConfigurationModel()
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
         model.load_data()
+        self.assertTrue(model.is_hosted)
+        model.is_hosted = False
+        self.assertFalse(model.is_hosted)
         model.modify()
         self.assertTrue(isinstance(model.get_state(), ModifiedState))
+        self.assertFalse(model.is_hosted)
 
     def test_modified_state_is_modifiable(self):
         """
         Test that the L{ConfigurationModel} transitions to L{ModifiedState}
         whenever L{modify} is called on it in L{ModifiedState}.
         """
-        model = ConfigurationModel()
-        model.load_data()
-        model.modify()
-        self.assertTrue(isinstance(model.get_state(), ModifiedState))
-        model.modify()
-        self.assertTrue(isinstance(model.get_state(), ModifiedState))
-
-    def test_modified_state_is_testable(self):
-        """
-        Test that the L{ConfigurationModel} can be transitioned via L{test}
-        when it is in the L{ModifiedState}.
-        """
-        model = ConfigurationModel()
-        model.load_data()
-        model.modify()
-        model.test()
-        self.assertTrue(isinstance(model.get_state(), TestedGoodState))
-
-    def test_tested_states_are_modifiable(self):
-        """
-        Test that the L{ConfigurationModel} transitions to L{ModifiedState}
-        whenever L{modify} is called on it in a subclass of L{TestedState}
-        (L{TestedGoodState} or L{TestedBadState}).
-        """
-        test_succeed = lambda: True
-        test_fail = lambda: False
-        model = ConfigurationModel(test_method=test_succeed)
-        model.load_data()
-        model.test()
-        model.modify()
-        self.assertTrue(isinstance(model.get_state(), ModifiedState))
-        model = ConfigurationModel(test_method=test_fail)
-        model.load_data()
-        model.test()
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        model.load_data()
+        model.modify()
+        self.assertTrue(isinstance(model.get_state(), ModifiedState))
         model.modify()
         self.assertTrue(isinstance(model.get_state(), ModifiedState))
 
@@ -131,7 +373,9 @@
         Test that calling L{revert} on a L{ConfigurationModel} in
         L{VirginState} raises a L{StateError}.
         """
-        model = ConfigurationModel()
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
         self.assertRaises(StateError, model.revert)
 
     def test_initialiased_state_is_unrevertable(self):
@@ -139,7 +383,9 @@
         Test that calling L{revert} on a L{ConfigurationModel} in
         L{InitialisedState} raises a L{StateError}.
         """
-        model = ConfigurationModel()
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
         model.load_data()
         self.assertRaises(StateError, model.revert)
 
@@ -148,36 +394,43 @@
         Test that a L{ConfigurationModel} in L{ModifiedState} can be
         transitioned via L{revert} to L{InitialisedState}.
         """
-        model = ConfigurationModel()
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
         model.load_data()
         model.modify()
         model.revert()
         self.assertTrue(isinstance(model.get_state(), InitialisedState))
 
-    def test_tested_states_are_revertable(self):
-        """
-        Test that a L{ConfigurationModel} in one of the two L{TestedState}s can
-        be transitioned via L{revert} to L{InitialisedState}.
-        """
-        test_succeed = lambda: True
-        test_fail = lambda: False
-        model = ConfigurationModel(test_method=test_succeed)
-        model.load_data()
-        model.test()
-        model.revert()
-        self.assertTrue(isinstance(model.get_state(), InitialisedState))
-        model = ConfigurationModel(test_method=test_fail)
-        model.load_data()
-        model.test()
-        model.revert()
-        self.assertTrue(isinstance(model.get_state(), InitialisedState))
+    def test_reverting_reverts_data(self):
+        """
+        Test that transitioning via L{revert} causes the original
+        L{InitialisedState} to be restored.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        model.load_data()
+        self.assertEqual(HOSTED_LANDSCAPE_HOST, model.hosted_landscape_host)
+        self.assertEqual("CrazyHorse", model.local_account_name)
+        model.hosted_landscape_host = "foo"
+        model.local_account_name = "bar"
+        model.modify()
+        self.assertEqual("foo", model.hosted_landscape_host)
+        self.assertEqual("bar", model.local_account_name)
+        model.revert()
+        self.assertTrue(isinstance(model.get_state(), InitialisedState))
+        self.assertEqual(HOSTED_LANDSCAPE_HOST, model.hosted_landscape_host)
+        self.assertEqual("CrazyHorse", model.local_account_name)
 
     def test_persisting_a_virgin_raises(self):
         """
         Test that a L{ConfigurationModel} in L{VirginState} will raise a
         L{StateError} when you attempt to transition it with L{persist}.
         """
-        model = ConfigurationModel()
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
         self.assertRaises(StateError, model.persist)
 
     def test_persisting_initialised_state_raises(self):
@@ -185,39 +438,107 @@
         Test that a L{ConfigurationModel} in L{IntialisedState} will raise a
         L{StateError} when you attempt to transition it with L{persist}.
         """
-        model = ConfigurationModel()
-        model.load_data()
-        self.assertRaises(StateError, model.persist)
-
-    def test_persisting_modified_state_raises(self):
-        """
-        Test that a L{ConfigurationModel} in L{InitialisedState} will raise a
-        L{StateError} when you attempt to transition it with L{persist}.
-        """
-        model = ConfigurationModel()
-        model.load_data()
-        model.modify()
-        self.assertRaises(StateError, model.persist)
-
-    def test_persisting_tested_bad_state_raises(self):
-        """
-        Test that a L{ConfigurationModel} in L{TestedBadState} will raise a
-        L{StateError} when you attempt to transition it with L{persist}.
-        """
-        test_fail = lambda: False
-        model = ConfigurationModel(test_method=test_fail)
-        model.load_data()
-        model.test()
-        self.assertRaises(StateError, model.persist)
-
-    def test_persist_tested_good_state(self):
-        """
-        Test that a L{ConfigurationModel} in L{TestedGoodState} can be
-        transitioned via L{persist} to a L{IntialisedState}.
-        """
-        test_succeed = lambda: True
-        model = ConfigurationModel(test_method=test_succeed)
-        model.load_data()
-        model.test()
-        model.persist()
-        self.assertTrue(isinstance(model.get_state(), InitialisedState))
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        model.load_data()
+        self.assertRaises(StateError, model.persist)
+
+    def test_persisting_modified_is_allowed(self):
+        """
+        Test that a L{ConfigurationModel} in L{ModifiedState} will allow itself
+        to be transitioned with L{persist}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        model.load_data()
+        model.modify()
+        model.persist()
+        self.assertTrue(isinstance(model.get_state(), InitialisedState))
+
+    def test_persisting_saves_data_to_uisettings(self):
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        model.load_data()
+        self.assertTrue(uisettings.get_is_hosted())
+        self.assertEqual("Sparklehorse", uisettings.get_hosted_account_name())
+        self.assertEqual("Vivadixiesubmarinetransmissionplot",
+                        uisettings.get_hosted_password())
+        self.assertEqual("the.local.machine",
+                         uisettings.get_local_landscape_host())
+        self.assertEqual("CrazyHorse", uisettings.get_local_account_name())
+        self.assertEqual("RustNeverSleeps", uisettings.get_local_password())
+        model.is_hosted = False
+        model.hosted_account_name = "ThomasPaine"
+        model.hosted_password = "TheAgeOfReason"
+        model.local_landscape_host = "another.local.machine"
+        model.local_account_name = "ThomasHobbes"
+        model.local_password = "TheLeviathan"
+        model.modify()
+        self.assertTrue(isinstance(model.get_state(), ModifiedState))
+        model.persist()
+        self.assertTrue(isinstance(model.get_state(), InitialisedState))
+        self.assertFalse(uisettings.get_is_hosted())
+        self.assertEqual("ThomasPaine", uisettings.get_hosted_account_name())
+        self.assertEqual("TheAgeOfReason", uisettings.get_hosted_password())
+        self.assertEqual("another.local.machine",
+                         uisettings.get_local_landscape_host())
+        self.assertEqual("ThomasHobbes", uisettings.get_local_account_name())
+        self.assertEqual("TheLeviathan", uisettings.get_local_password())
+
+
+class StateTransitionWithExistingConfigTest(LandscapeTest):
+    """
+    Test that we handle existing configuration data correctly when
+    transitioning through L{ConfigurationModel} states.
+    """
+
+    helpers = [ConfigurationProxyHelper]
+
+    def setUp(self):
+        self.config_string = (
+            "[client]\n"
+            "data_path = /var/lib/landscape/client/\n"
+            "http_proxy = http://proxy.localdomain:3192\n"
+            "tags = a_tag\n"
+            "url = https://landscape.canonical.com/message-system\n"
+            "account_name = Sparklehorse\n"
+            "registration_password = Vivadixiesubmarinetransmissionplot\n"
+            "computer_title = baz\n"
+            "https_proxy = https://proxy.localdomain:6192\n"
+            "ping_url = http://landscape.canonical.com/ping\n")
+        self.default_data = {
+            "is-hosted": True,
+            "computer-title": "bound.to.lose",
+            "hosted-landscape-host": "landscape.canonical.com",
+            "hosted-account-name": "Sparklehorse",
+            "hosted-password": "Vivadixiesubmarinetransmissionplot",
+            "local-landscape-host": "the.local.machine",
+            "local-account-name": "CrazyHorse",
+            "local-password": "RustNeverSleeps"
+            }
+        super(StateTransitionWithExistingConfigTest, self).setUp()
+
+    def test_persisting_saves_data_to_proxy(self):
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        model = ConfigurationModel(proxy=self.proxy, uisettings=uisettings)
+        model.load_data()
+        self.assertEqual("Sparklehorse", self.proxy.account_name)
+        self.assertEqual("Vivadixiesubmarinetransmissionplot",
+                        self.proxy.registration_password)
+        model.is_hosted = False
+        model.local_account_name = "ThomasPaine"
+        model.local_password = "TheAgeOfReason"
+        model.modify()
+        self.assertTrue(isinstance(model.get_state(), ModifiedState))
+        model.persist()
+        self.assertTrue(isinstance(model.get_state(), InitialisedState))
+        self.assertFalse(model.is_hosted)
+        self.assertEqual("https://the.local.machine/message-system",
+                         self.proxy.url)
+        self.assertEqual("http://the.local.machine/ping", self.proxy.ping_url)
+        self.assertEqual("ThomasPaine", self.proxy.account_name)
+        self.assertEqual("TheAgeOfReason", self.proxy.registration_password)
 
=== added file 'landscape/ui/model/configuration/tests/test_uisettings.py'
--- landscape/ui/model/configuration/tests/test_uisettings.py	1970-01-01 00:00:00 +0000
+++ landscape/ui/model/configuration/tests/test_uisettings.py	2012-02-29 16:50:20 +0000
@@ -0,0 +1,189 @@
+from landscape.tests.helpers import LandscapeTest
+from landscape.ui.tests.helpers import FakeGSettings
+from landscape.ui.model.configuration.uisettings import UISettings
+
+
+class UISettingsTest(LandscapeTest):
+
+    default_data = {"is-hosted": True,
+                    "computer-title": "bound.to.lose",
+                    "hosted-landscape-host": "landscape.canonical.com",
+                    "hosted-account-name": "Sparklehorse",
+                    "hosted-password": "Vivadixiesubmarinetransmissionplot",
+                    "local-landscape-host": "the.local.machine",
+                    "local-account-name": "CrazyHorse",
+                    "local-password": "RustNeverSleeps"
+                    }
+
+    def test_setup(self):
+        """
+        Test that the L{GSettings.Client} is correctly initialised.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        UISettings(settings)
+        self.assertTrue(settings.was_called_with_args(
+                "new", UISettings.BASE_KEY))
+
+    def test_get_is_hosted(self):
+        """
+        Test that the L{get_is_hosted} value is correctly fetched from the
+        L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertTrue(uisettings.get_is_hosted())
+
+    def test_set_is_hosted(self):
+        """
+        Test that we can correctly use L{set_is_hosted} to write the
+        L{is_hosted} value to the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertTrue(uisettings.get_is_hosted())
+        uisettings.set_is_hosted(False)
+        self.assertFalse(uisettings.get_is_hosted())
+        self.assertTrue(settings.was_called_with_args(
+                "set_boolean", "is-hosted", False))
+
+    def test_get_computer_title(self):
+        """
+        Test that the L{get_computer_title} value is correctly fetched
+        from the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("bound.to.lose",
+                         uisettings.get_computer_title())
+
+    def test_set_computer_title(self):
+        """
+        Test that L{set_computer_title} correctly sets the value of
+        L{computer_title} in the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("bound.to.lose", uisettings.get_computer_title())
+        uisettings.set_computer_title("Bang")
+        self.assertEqual("Bang", uisettings.get_computer_title())
+
+    def test_get_hosted_landscape_host(self):
+        """
+        Test that the L{get_hosted_landscape_host} value is correctly fetched
+        from the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("landscape.canonical.com",
+                         uisettings.get_hosted_landscape_host())
+
+    # NOTE: There is no facility to set the hosted-landscape-host
+
+    def test_get_hosted_account_name(self):
+        """
+        Test that the L{get_hosted_account_name} value is correctly fetched
+        from the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("Sparklehorse",
+                         uisettings.get_hosted_account_name())
+
+    def test_set_hosted_account_name(self):
+        """
+        Test that L{set_hosted_account_name} correctly sets the value of
+        L{hosted_account_name} in the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("Sparklehorse", uisettings.get_hosted_account_name())
+        uisettings.set_hosted_account_name("Bang")
+        self.assertEqual("Bang", uisettings.get_hosted_account_name())
+
+    def test_get_hosted_password(self):
+        """
+        Test that the L{get_hosted_password} value is correctly fetched
+        from the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("Vivadixiesubmarinetransmissionplot",
+                         uisettings.get_hosted_password())
+
+    def test_set_hosted_password(self):
+        """
+        Test that L{set_hosted_password} correctly sets the value of
+        L{hosted_password} in the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("Vivadixiesubmarinetransmissionplot",
+                         uisettings.get_hosted_password())
+        uisettings.set_hosted_password("Bang")
+        self.assertEqual("Bang", uisettings.get_hosted_password())
+
+    def test_get_local_landscape_host(self):
+        """
+        Test that the L{get_local_landscape_host} value is correctly fetched
+        from the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("the.local.machine",
+                         uisettings.get_local_landscape_host())
+
+    def test_set_local_landscape_host(self):
+        """
+        Test that L{set_local_landscape_host} correctly sets the value of
+        L{local_landscape_host} in the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("the.local.machine",
+                         uisettings.get_local_landscape_host())
+        uisettings.set_local_landscape_host("Bang")
+        self.assertEqual("Bang", uisettings.get_local_landscape_host())
+
+    def test_get_local_account_name(self):
+        """
+        Test that the L{get_local_account_name} value is correctly fetched
+        from the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("CrazyHorse",
+                         uisettings.get_local_account_name())
+
+    def test_set_local_account_name(self):
+        """
+        Test that L{set_local_account_name} correctly sets the value of
+        L{local_account_name} in the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("CrazyHorse",
+                         uisettings.get_local_account_name())
+        uisettings.set_local_account_name("Bang")
+        self.assertEqual("Bang", uisettings.get_local_account_name())
+
+    def test_get_local_password(self):
+        """
+        Test that the L{get_local_password} value is correctly fetched
+        from the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("RustNeverSleeps",
+                         uisettings.get_local_password())
+
+    def test_set_local_password(self):
+        """
+        Test that L{set_local_password} correctly sets the value of
+        L{local_password} in the L{GSettings.Client}.
+        """
+        settings = FakeGSettings(data=self.default_data)
+        uisettings = UISettings(settings)
+        self.assertEqual("RustNeverSleeps",
+                         uisettings.get_local_password())
+        uisettings.set_local_password("Bang")
+        self.assertEqual("Bang", uisettings.get_local_password())
 
=== added file 'landscape/ui/model/configuration/uisettings.py'
--- landscape/ui/model/configuration/uisettings.py	1970-01-01 00:00:00 +0000
+++ landscape/ui/model/configuration/uisettings.py	2012-02-29 16:50:20 +0000
@@ -0,0 +1,57 @@
+class UISettings(object):
+    """
+    A very thin wrapper around L{GSettings} to avoid having to know the
+    L{BaseKey} and type information elsewhere.  In some future version it would
+    be right to bind to change events here so we can react to people changing
+    the settings in dconf, for now that is overkill.
+    """
+
+    BASE_KEY = "com.canonical.landscape-client-settings"
+
+    def __init__(self, settings):
+        self.settings = settings.new(self.BASE_KEY)
+
+    def get_is_hosted(self):
+        return self.settings.get_boolean("is-hosted")
+
+    def set_is_hosted(self, value):
+        self.settings.set_boolean("is-hosted", value)
+
+    def get_computer_title(self):
+        return self.settings.get_string("computer-title")
+
+    def set_computer_title(self, value):
+        self.settings.set_string("computer-title", value)
+
+    def get_hosted_landscape_host(self):
+        return self.settings.get_string("hosted-landscape-host")
+
+    def get_hosted_account_name(self):
+        return self.settings.get_string("hosted-account-name")
+
+    def set_hosted_account_name(self, value):
+        self.settings.set_string("hosted-account-name", value)
+
+    def get_hosted_password(self):
+        return self.settings.get_string("hosted-password")
+
+    def set_hosted_password(self, value):
+        self.settings.set_string("hosted-password", value)
+
+    def get_local_landscape_host(self):
+        return self.settings.get_string("local-landscape-host")
+
+    def set_local_landscape_host(self, value):
+        self.settings.set_string("local-landscape-host", value)
+
+    def get_local_account_name(self):
+        return self.settings.get_string("local-account-name")
+
+    def set_local_account_name(self, value):
+        self.settings.set_string("local-account-name", value)
+
+    def get_local_password(self):
+        return self.settings.get_string("local-password")
+
+    def set_local_password(self, value):
+        self.settings.set_string("local-password", value)
 
=== modified file 'landscape/ui/tests/helpers.py'
--- landscape/ui/tests/helpers.py	2012-01-26 12:40:16 +0000
+++ landscape/ui/tests/helpers.py	2012-02-29 16:50:20 +0000
@@ -1,3 +1,6 @@
+import os
+
+from lxml import etree
 import dbus
 
 from landscape.configuration import LandscapeSetupConfiguration
@@ -50,3 +53,95 @@
     def tear_down(self, test_case):
         if not dbus_test_should_skip:
             test_case.mechanism.remove_from_connection()
+
+
+class FakeGSettings(object):
+    """
+    This class impersonates a real L{gi.repostiroy.Gio.GSettings}
+    object to allow for testing code that utilises it without setting values in
+    the live DConf.
+    """
+
+    calls = {}
+
+    def __init__(self, data={}):
+        self.set_data(data)
+        tree = etree.parse(
+            os.path.join(
+                os.path.dirname(os.path.abspath(__file__)),
+                "../../../",
+                "glib-2.0/schemas/",
+                "com.canonical.landscape-client-settings.gschema.xml"))
+        root = tree.getroot()
+        self.schema = root.find("schema")
+        assert(self.schema.attrib["id"] ==
+               "com.canonical.landscape-client-settings")
+        self.keys = {}
+        for key in self.schema.findall("key"):
+            self.keys[key.attrib["name"]] = key.attrib["type"]
+
+    def check_key_data(self, name, gstype):
+        if name in self.keys:
+            if self.keys[name] == gstype:
+                return True
+            else:
+                raise ValueError("The GSchema file says %s is a %s, " +
+                                 "but you asked for a %s" %
+                                 (name, self.keys[name], gstype))
+        else:
+            raise KeyError("Can't find %s in the GSchema file!" % name)
+
+    def get_value(self, name, gstype):
+        if self.check_key_data(name, gstype):
+            return self.data[name]
+
+    def set_value(self, name, gstype, value):
+        if self.check_key_data(name, gstype):
+            self.data[name] = value
+
+    def set_data(self, data):
+        self.data = data
+
+    def _call(self, name, *args):
+        [count, arglist] = self.calls.get(name, (0, []))
+        count += 1
+        arglist.append(self._args_to_string(*args))
+        self.calls[name] = [count, arglist]
+
+    def _args_to_string(self, *args):
+        return "|".join([str(arg) for arg in args])
+
+    def new(self, key):
+        self._call("new", key)
+        return self
+
+    def connect(self, signal, callback, *args):
+        self._call("connect", signal, callback, *args)
+
+    def get_boolean(self, name):
+        self._call("get_boolean", name)
+        return self.get_value(name, "b")
+
+    def set_boolean(self, name, value):
+        self._call("set_boolean", name, value)
+        self.set_value(name, "b", value)
+
+    def get_string(self, name):
+        self._call("get_string", name)
+        return self.get_value(name, "s")
+
+    def set_string(self, name, value):
+        self._call("set_string", name, value)
+        self.set_value(name, "s", value)
+
+    def was_called(self, name):
+        return self.calls.haskey(name)
+
+    def was_called_with_args(self, name, *args):
+        try:
+            [count, arglist] = self.calls.get(name, (0, []))
+        except KeyError:
+            return False
+
+        expected_args = self._args_to_string(*args)
+        return expected_args in arglist
 
=== modified file 'scripts/landscape-client-registration-mechanism' (properties changed: -x to +x)
=== modified file 'scripts/landscape-client-settings-mechanism' (properties changed: -x to +x)
=== modified file 'setupui.py'
--- setupui.py	2012-01-19 14:06:45 +0000
+++ setupui.py	2012-02-29 16:50:20 +0000
@@ -39,6 +39,7 @@
                 ff = open(service_path, "w")
                 ff.write(output)
                 ff.close()
+        os.system("glib-compile-schemas /usr/share/glib-2.0/schemas/")
 
 
 pkit_description = \
@@ -76,7 +77,10 @@
         ('/usr/share/applications/',
          ['applications/landscape-client-settings.desktop']),
         ('/usr/share/icons/hicolor/scalable/apps/',
-         ['icons/preferences-management-service.svg'])],
+         ['icons/preferences-management-service.svg']),
+        ('/usr/share/glib-2.0/schemas/',
+         ['glib-2.0/schemas/com.canonical.landscape-client-settings.gschema.xml'])
+        ],
       scripts=['scripts/landscape-client-settings-mechanism',
                'scripts/landscape-client-registration-mechanism',
                "scripts/landscape-client-settings-ui"],