Merge lp:~brian.curtin/ubuntuone-control-panel/autostart-clean into lp:ubuntuone-control-panel

[Brian Curtin]

- Add Ubuntu One to the Windows auto-start registry key

Introduce add_to_autostart (from ubuntuone-windows-installer) and add it to the install wizard for new users as well as ensure it gets called for users who already have credentials.

[Natalia Bidart (nataliabidart)]

If we're a already calling add_to_autostart from ubuntuone/controlpanel/gui/qt/gui.py, why would you also add it in the wizard code?

Unless I'm missing something (please let me know), there is no need to do that. Also, semantically, the wizard is a 're usable' widget, and we would not like to leak thing like adding to autostart when using it.

review: Needs Fixing

[Manuel de la Peña (mandel)]

Adding tho the comments of natalia, I would call the parents setup before I do any patching or operations. Also I would not add the setup of we only have one test, but im lazy :)

review: Needs Fixing

[lp:~brian.curtin/ubuntuone-control-panel/autostart-clean]

301. By Brian Curtin on 2012-03-23

Removed an add_to_autostart call which is no longer needed.

[Brian Curtin (brian.curtin)]

Rev 301 removes the call Natalia was talking about.

Manuel: I need that ordering in the setUp because I need add_to_autostart patched before anything else up the chain runs. The add_to_autostart is called when MainWindow is created, and it's created within BaseTestCase, so I have to get patched in before then.

[lp:~brian.curtin/ubuntuone-control-panel/autostart-clean]

302. By Brian Curtin on 2012-03-23

Add docstrings to two test classes

[Natalia Bidart (nataliabidart)]

ubuntuone/controlpanel/gui/qt/tests/test_gui.py:
    130: [C0111, AutoStartTestCase.test_add_to_autostart] Missing docstring

ubuntuone/controlpanel/utils/tests/test_windows.py:
    162: [W0622, AutostartTestCase.test_add_syncdaemon_to_autostart] Redefining built-in 'dir'

ubuntuone/controlpanel/utils/windows.py:
    22: [F0401] Unable to import '_winreg'

review: Needs Fixing

[lp:~brian.curtin/ubuntuone-control-panel/autostart-clean]

303. By Brian Curtin on 2012-03-23

fix lint errors

[Brian Curtin (brian.curtin)]

Rev 303 fixes the remaining lint issues.

[Natalia Bidart (nataliabidart)]

Lookis at the logic inside add_to_autostart, is there any reason to have the "else: return" in the middle of the code? Why not just:

def add_to_autostart():
    """Add syncdaemon to the session's autostart."""
    if getattr(sys, "frozen", False):
        sd_path = '"%s"' % os.path.join(os.path.dirname(
            os.path.abspath(sys.executable)),
            "ubuntuone-syncdaemon.exe")
        u1cp_path = '"%s"' % os.path.join(os.path.dirname(
            os.path.abspath(sys.executable)),
            "ubuntuone-control-panel-qt.exe")

        with _winreg.OpenKey(_winreg.HKEY_CURRENT_USER, AUTORUN_KEY,
                             0, _winreg.KEY_ALL_ACCESS) as key:
            # pylint: disable=E0602
            _winreg.SetValueEx(key, "Ubuntu One", 0, _winreg.REG_SZ, sd_path)
            _winreg.SetValueEx(key, "Ubuntu One Icon", 0, _winreg.REG_SZ,
                               u1cp_path + " --minimized --with-icon")

[Brian Curtin (brian.curtin)]

That was an artifact of the old implementation from ubuntuone-windows-installer. Removed.

[lp:~brian.curtin/ubuntuone-control-panel/autostart-clean]

304. By Brian Curtin on 2012-03-23

Remove unnecessary else/return block

[Manuel de la Peña (mandel)]

> Rev 301 removes the call Natalia was talking about.
>
> Manuel: I need that ordering in the setUp because I need add_to_autostart
> patched before anything else up the chain runs. The add_to_autostart is called
> when MainWindow is created, and it's created within BaseTestCase, so I have to
> get patched in before then.

Very well then.. I would have added a comment to state so to avoid stupid reviewers :)

Few things:

8 +from ubuntuone.controlpanel.utils import add_to_autostart

I know we do take a lot of effort in doing alphabetical ordered imports, can you move the import a few lines? I'm the first one to ignore this rule, but is a style we try to keep.

Besides that every thing looks good so I'll approve assuming you will order the imports!

PS: I had no idea _winreg could be a context manager!

review: Approve

[lp:~brian.curtin/ubuntuone-control-panel/autostart-clean]

305. By Brian Curtin on 2012-03-26

Alphabetical import ordering

306. By Brian Curtin on 2012-03-26

Add comment explaining patching order

[Natalia Bidart (nataliabidart)]

Looks good!

review: Approve

[Roberto Alsina (ralsina)]

+1

review: Approve

=== modified file 'ubuntuone/controlpanel/gui/qt/gui.py'
--- ubuntuone/controlpanel/gui/qt/gui.py	2012-03-21 19:17:21 +0000
+++ ubuntuone/controlpanel/gui/qt/gui.py	2012-03-26 13:36:23 +0000
@@ -20,6 +20,7 @@
 
 from ubuntuone.controlpanel.gui.qt.systray import TrayIcon
 from ubuntuone.controlpanel.gui.qt.ui import mainwindow_ui
+from ubuntuone.controlpanel.utils import add_to_autostart
 
 # pylint: disable=E0611
 try:
@@ -49,6 +50,7 @@
         self.installer = installer
         if installer:
             self.ui.control_panel.start_from_license()
+        add_to_autostart()
         if USE_LIBUNITY:
             self.entry = Unity.LauncherEntry.get_for_desktop_id(U1_DOTDESKTOP)
         else:

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_gui.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_gui.py	2012-03-09 01:05:49 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_gui.py	2012-03-26 13:36:23 +0000
@@ -18,6 +18,8 @@
 
 """Tests for the Qt UI."""
 
+from twisted.internet import defer
+
 from ubuntuone.controlpanel.gui.qt import gui
 from ubuntuone.controlpanel.gui.qt.tests import BaseTestCase
 
@@ -115,3 +117,17 @@
         self.patch(self.ui, "entry", entry)
         self.ui.set_urgent("foo")
         self.assertEqual(entry.called, (('urgent', "foo"), {}))
+
+
+class AutoStartTestCase(MainWindowTestCase):
+    """Test the add_to_autostart call."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        # Be sure to patch add_to_autostart *before* class_ui creation occurs.
+        self.patch(gui, "add_to_autostart", self._set_called)
+        yield super(AutoStartTestCase, self).setUp()
+
+    def test_add_to_autostart(self):
+        """Test that the add_to_autostart function is called when CP opens."""
+        self.assertTrue(self._called)

=== modified file 'ubuntuone/controlpanel/utils/__init__.py'
--- ubuntuone/controlpanel/utils/__init__.py	2012-03-16 16:40:27 +0000
+++ ubuntuone/controlpanel/utils/__init__.py	2012-03-26 13:36:23 +0000
@@ -38,11 +38,13 @@
     are_updates_present = windows.are_updates_present
     default_folders = windows.default_folders
     perform_update = windows.perform_update
+    add_to_autostart = windows.add_to_autostart
 else:
     from ubuntuone.controlpanel.utils import linux
     are_updates_present = lambda *args, **kwargs: False
     default_folders = linux.default_folders
     perform_update = lambda *args, **kwargs: None
+    add_to_autostart = lambda *args, **kwargs: None
 
 # pylint: enable=C0103
 

=== modified file 'ubuntuone/controlpanel/utils/tests/test_windows.py'
--- ubuntuone/controlpanel/utils/tests/test_windows.py	2012-03-16 19:55:22 +0000
+++ ubuntuone/controlpanel/utils/tests/test_windows.py	2012-03-26 13:36:23 +0000
@@ -18,6 +18,7 @@
 """Test the windows utils functions."""
 
 import os
+import sys
 
 from twisted.internet import defer
 
@@ -87,6 +88,93 @@
         self.assertEqual(0, self._called[0][5])
 
 
+class FakeOpenKey(object):
+
+    """A Fake OpenKey class.
+
+    This class becomes a method-like callable on FakeRegistry, allowing
+    FakeRegistry.OpenKey to operate as a context manager."""
+
+    def __init__(self):
+        self.openkey_args = None
+        super(FakeOpenKey, self).__init__()
+
+    def __call__(self, *args, **kwargs):
+        self.openkey_args = (args, kwargs)
+        return self
+
+    def __enter__(self, *args, **kwargs):
+        #self.openkey_args = (args, kwargs)
+        return self
+
+    def __exit__(self, *args):
+        pass
+
+
+class FakeRegistry(object):
+
+    """A fake registry."""
+
+    # pylint: disable=C0103
+    HKEY_CURRENT_USER = 2
+    KEY_ALL_ACCESS = 4
+    REG_SZ = 1
+
+    def __init__(self):
+        self.HKEY_CURRENT_USER = 2
+        self.KEY_ALL_ACCESS = 4
+        self.openkey_args = None
+        self.query_args = None
+        self.set_args = []
+
+        self.OpenKey = FakeOpenKey()
+
+    def QueryValueEx(self, *args, **kwargs):
+        """Fake QueryValueEx."""
+        self.query_args = (args, kwargs)
+
+    def SetValueEx(self, *args, **kwargs):
+        """Fake SetValueEx."""
+        self.set_args.append((args, kwargs))
+
+
+class AutostartTestCase(TestCase):
+
+    """Test add_syncdaemon_to_autostart."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Initialize this test instance."""
+        self.registry = FakeRegistry()
+        self.patch(utils.windows, "_winreg", self.registry)
+        yield super(AutostartTestCase, self).setUp()
+
+    def test_add_syncdaemon_to_autostart(self):
+        """Check that the registry is updated correctly."""
+        # I can't patch sys because frozen is not there by default
+        sys.frozen = True
+        self.addCleanup(delattr, sys, 'frozen')
+        utils.windows.add_to_autostart()
+        self.assertEqual(self.registry.OpenKey.openkey_args,
+            ((self.registry.HKEY_CURRENT_USER, utils.windows.AUTORUN_KEY, 0,
+              self.registry.KEY_ALL_ACCESS), {}))
+        self.assertEqual(self.registry.query_args, None)
+        path = os.path.dirname(os.path.abspath(sys.executable))
+        self.assertEqual(self.registry.set_args,
+             [((self.registry.OpenKey, 'Ubuntu One', 0, 1,
+             '"%s\\ubuntuone-syncdaemon.exe"' % path), {}),
+             ((self.registry.OpenKey, 'Ubuntu One Icon', 0, 1,
+             '"%s\\ubuntuone-control-panel-qt.exe" --minimized' % path),
+             {})])
+
+    def test_not_added_if_not_frozen(self):
+        """Not frozen binaries are not added to the registry."""
+        utils.windows.add_to_autostart()
+        self.assertEqual(self.registry.openkey_args, None)
+        self.assertEqual(self.registry.query_args, None)
+        self.assertEqual(self.registry.set_args, [])
+
+
 class DefaultFoldersTestCase(TestCase):
     """Test the default_folders method."""
 

=== modified file 'ubuntuone/controlpanel/utils/windows.py'
--- ubuntuone/controlpanel/utils/windows.py	2012-03-16 19:55:22 +0000
+++ ubuntuone/controlpanel/utils/windows.py	2012-03-26 13:36:23 +0000
@@ -23,6 +23,7 @@
 # Avoid pylint error on Linux
 # pylint: disable=F0401
 import win32api
+import _winreg
 
 from win32com.shell import shell, shellcon
 # pylint: enable=F0401
@@ -33,6 +34,7 @@
 
 logger = setup_logging('utils.windows')
 AUTOUPDATE_EXE_NAME = 'autoupdate-windows.exe'
+AUTORUN_KEY = r"Software\Microsoft\Windows\CurrentVersion\Run"
 
 
 def _get_update_path():
@@ -96,3 +98,21 @@
         win32api.ShellExecute(None, 'runas',
             update_path,
             '--unattendedmodeui none', '', 0)
+
+
+def add_to_autostart():
+    """Add syncdaemon to the session's autostart."""
+    if getattr(sys, "frozen", False):
+        sd_path = '"%s"' % os.path.join(os.path.dirname(
+            os.path.abspath(sys.executable)),
+            "ubuntuone-syncdaemon.exe")
+        u1cp_path = '"%s"' % os.path.join(os.path.dirname(
+            os.path.abspath(sys.executable)),
+            "ubuntuone-control-panel-qt.exe")
+
+        with _winreg.OpenKey(_winreg.HKEY_CURRENT_USER, AUTORUN_KEY,
+                             0, _winreg.KEY_ALL_ACCESS) as key:
+            # pylint: disable=E0602
+            _winreg.SetValueEx(key, "Ubuntu One", 0, _winreg.REG_SZ, sd_path)
+            _winreg.SetValueEx(key, "Ubuntu One Icon", 0, _winreg.REG_SZ,
+                               u1cp_path + " --minimized --with-icon")