Merge lp:~ralsina/ubuntuone-windows-installer/fix-810053 into lp:ubuntuone-windows-installer

[Roberto Alsina]

Use SSO the right way, make the UI be in the ussoc process, connect callbacks so the ping url is used.

Use SSO the right way, make the UI be in the ussoc process, connect callbacks so the ping url is used.

To test IRL(windows):

This works by making ussoc import the UI from the installer (which imports from control panel), you need to run ussoc with the installer in the PYTHONPATH, something like this (maybe you need other things):

set PYTHONPATH=..\fix-810053;..\ubuntuone-control-panel;.
python bin\windows-ubuntu-sso-client

And then you run the installer:

set PYTHONPATH=..\ubuntuone-client;..\ubuntuone-control-panel;..\ubuntu-sso-client;.
python bin\ubuntuone-installer-qt

[lp:~ralsina/ubuntuone-windows-installer/fix-810053]

18. By Roberto Alsina on 2011-07-13

Re-add appname, which SSO uses

19. By Roberto Alsina on 2011-07-14

More tests

20. By Roberto Alsina on 2011-07-14

missing docstrings

21. By Roberto Alsina on 2011-07-14

another test

22. By Roberto Alsina on 2011-07-14

stylefix

23. By Roberto Alsina on 2011-07-14

removed unused imports

24. By Roberto Alsina on 2011-07-14

test fixes

25. By Roberto Alsina on 2011-07-15

removed unused code, fixed comment

26. By Roberto Alsina on 2011-07-15

Add help_text, emit userCancellation

27. By Roberto Alsina on 2011-07-15

Close the app when the user cancels

[Natalia Bidart (nataliabidart)]

A couple of needs fixing:

* the 'found' callback should be called 'stop' or something similar.
* the signal CredentialsError should be connected as well, probably by defining a callback for on_credentials_error.
* the code needs to unregister_from_signals, otherwise the SSO process shows something like this when the installer finishes:

2011-07-15 22:53:43,746:746.000051498 - ubuntu_sso.main.windows - WARNING - Could not emit signal due to [Failure instance: Traceback (failure with no frames):
<class 'twisted.spread.pb.PBConnectionLost'>: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.error.ConnectionDone'>: Connection was closed cleanly.]
]
*--- Failure #6 ---
Failure: twisted.spread.pb.PBConnectionLost: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.error.ConnectionDone'>: Connectionwas closed cleanly.]
*--- End of Failure #6 ---
2011-07-15 22:53:43,760:760.999917984 - ubuntu_sso.main.windows - WARNING - Traceback is:
None

* I had the following issue, but this may be related to SSO itself and not to the installer:
 - I ran SSO as specified
 - I ran the installer as specified
 - I got the wizard UI and I clicked 'Disagree and cancel' -> 'Yes I want to cancel'
 - SSO printed out the trace above and re-opened the wizard UI...

Let me know if you can reproduce what I detailed in the last point.
Thanks!

review: Needs Fixing

[lp:~ralsina/ubuntuone-windows-installer/fix-810053]

28. By Roberto Alsina on 2011-07-16

more reasonable name

29. By Roberto Alsina on 2011-07-16

Connect to credentials error signal

[Roberto Alsina (ralsina)]

Pushed revision 29.

* I can reproduce that error (BTW, what kind of error is "connection closed cleanly"? ;-)
* I can't reproduce the wizard restarting before or after revision 29
* I added the connection to on_credentials_error (does nothing interesting yet, I can add that in another
  branch)
* I added the unregister_to_signals call (hope I did it right)

After these changes, the trace is still there. Looks harmless enough, even if it's annoying.

review: Approve

[Roberto Alsina (ralsina)]

review: Abstain

[lp:~ralsina/ubuntuone-windows-installer/fix-810053]

30. By Roberto Alsina on 2011-07-18

Use the right constants in main

[Natalia Bidart (nataliabidart)]

Test are not passing in windows.

review: Needs Fixing

[lp:~ralsina/ubuntuone-windows-installer/fix-810053]

31. By Roberto Alsina on 2011-07-18

test fixes

32. By Roberto Alsina on 2011-07-18

pep8 fix

33. By Roberto Alsina on 2011-07-18

oops

[Natalia Bidart (nataliabidart)]

All green now.

review: Approve

[Alejandro J. Cura (alecu)]

In order to review, I had to add the ubuntuone-client to the sso path, and I needed to remove the credentials from the windows credentials manager.

The branch works as expected, but I still don't understand why we are making the UI be a part of the ussoc process. The dependency graph of module import looks very weird after this change.

review: Needs Information

[Roberto Alsina (ralsina)]

> In order to review, I had to add the ubuntuone-client to the sso path, and I
> needed to remove the credentials from the windows credentials manager.
>
> The branch works as expected, but I still don't understand why we are making
> the UI be a part of the ussoc process. The dependency graph of module import
> looks very weird after this change.

This is how ussoc is meant to be used, and using it differently did not work.
I agree it complicates things.

[Alejandro J. Cura (alecu)]

I finally understood how this all is supposed to work, and while I'm not thrilled about it, I think it works and the UI looks perfect.
Thanks for taking the time to explain it on IRC :-)

review: Approve

=== modified file 'bin/ubuntuone-installer-qt'
--- bin/ubuntuone-installer-qt	2011-06-22 14:53:52 +0000
+++ bin/ubuntuone-installer-qt	2011-07-18 18:59:29 +0000
@@ -27,6 +27,8 @@
 
 from optparse import OptionParser
 
+from twisted.internet import reactor
+
 from ubuntuone_installer import TRANSLATION_DOMAIN
 
 gettext.textdomain(TRANSLATION_DOMAIN)
@@ -46,3 +48,4 @@
     parser = parser_options()
     (options, args) = parser.parse_args(sys.argv)
     main.main()
+    reactor.run()

=== modified file 'run-tests.bat'
--- run-tests.bat	2011-07-05 20:07:40 +0000
+++ run-tests.bat	2011-07-18 18:59:29 +0000
@@ -55,7 +55,7 @@
 set USE_PYFLAKES=1
 "%PYTHONEXEPATH%\python.exe" "%PYTHONEXEPATH%\Scripts\u1lint" ubuntuone_installer
 :: test for style if we can, if pep8 is not present, move to the end
-IF EXIST "%PYTHONEXEPATH%\Scripts\pep8.exe" "%PYTHONEXEPATH%\Scripts\pep8.exe" --repeat ubuntuone_installer
+IF EXIST "%PYTHONEXEPATH%\Scripts\pep8.exe" "%PYTHONEXEPATH%\Scripts\pep8.exe" --repeat --exclude="*_ui.py,*_rc.py" .
 :: Delete the temp folders
 RMDIR /s /q _trial_temp
 RMDIR /s /q .coverage

=== modified file 'ubuntuone_installer/gui/qt/embedded_sso.py'
--- ubuntuone_installer/gui/qt/embedded_sso.py	2011-06-27 09:59:34 +0000
+++ ubuntuone_installer/gui/qt/embedded_sso.py	2011-07-18 18:59:29 +0000
@@ -18,10 +18,10 @@
 
 """The user interface for the Ubuntu One Installer's SSO client."""
 
-from PyQt4 import QtCore
-
 import ubuntu_sso.qt.gui as sso_gui
 
+from ubuntuone_installer.gui.qt.gui import MainWindow
+
 
 class UbuntuSSOClientGUI(object):
     """A custom Client GUI for SSO."""
@@ -31,5 +31,31 @@
 
         # create the controller and the ui, then set the cb and call the show
         # method so that we can work
-        self.controller = sso_gui.UbuntuSSOWizardController(app_name)
-        self.view = QtCore.QCoreApplication.instance().window
+        self.controller = sso_gui.UbuntuSSOWizardController(
+            app_name,
+            user_cancellation_callback=self._user_cancellation_callback
+        )
+        self.view = MainWindow()
+
+        self.login_success_callback = lambda x, y: None
+        self.registration_success_callback = lambda x, y: None
+        self.user_cancellation_callback = lambda x: None
+
+        self.view.loginSuccess.connect(self._login_success_callback)
+        self.view.registrationSuccess.connect(
+            self._registration_success_callback)
+        self.view.userCancellation.connect(self._user_cancellation_callback)
+
+        self.view.show()
+
+    def _login_success_callback(self, app_name, email):
+        """Call the real callback, set by the Credentials object"""
+        self.login_success_callback(str(app_name), str(email))
+
+    def _registration_success_callback(self, app_name, email):
+        """Call the real callback, set by the Credentials object"""
+        self.registration_success_callback(str(app_name), str(email))
+
+    def _user_cancellation_callback(self, app_name):
+        """Call the real callback, set by the Credentials object"""
+        self.user_cancellation_callback(str(app_name))

=== modified file 'ubuntuone_installer/gui/qt/gui.py'
--- ubuntuone_installer/gui/qt/gui.py	2011-07-15 18:45:17 +0000
+++ ubuntuone_installer/gui/qt/gui.py	2011-07-18 18:59:29 +0000
@@ -25,8 +25,6 @@
 
 from PyQt4 import QtGui, QtCore
 
-from ubuntu_sso.credentials import Credentials
-
 from ubuntu_sso.qt.gui import (
     EmailVerificationPage,
     ErrorPage,
@@ -57,9 +55,8 @@
 
 from ubuntuone.credentials import (
     APP_NAME,
+    DESCRIPTION,
     TC_URL,
-    PING_URL,
-    DESCRIPTION,
 )
 
 from ubuntuone_installer.logger import setup_logging
@@ -276,6 +273,7 @@
     recoverableError = QtCore.pyqtSignal('QString', 'QString')
     loginSuccess = QtCore.pyqtSignal('QString', 'QString')
     registrationSuccess = QtCore.pyqtSignal('QString', 'QString')
+    userCancellation = QtCore.pyqtSignal('QString')
 
     def __init__(self, close_callback=None):
         """Initialize this instance."""
@@ -284,9 +282,9 @@
 
         # Data for SSO
         self.app_name = APP_NAME
-        self.help_text = DESCRIPTION
         self.tc_url = TC_URL
-        self.ping_url = PING_URL
+        # help_text is not displayed in the UI from design
+        self.help_text = ""
 
         super(MainWindow, self).__init__()
         self.setWizardStyle(self.ModernStyle)
@@ -294,13 +292,6 @@
 
         self.setSideWidget(SideWidget())
 
-        self.creds = Credentials(
-            app_name=self.app_name,
-            ui_module="ubuntuone_installer.gui.qt.embedded_sso",
-            tc_url=self.tc_url,
-            ping_url=self.ping_url,
-            )
-
         self.setOption(self.NoBackButtonOnStartPage, True)
 
         # PyQt doesn't support the (int, page) version of addPage, so
@@ -400,6 +391,7 @@
         if self.close_callback is not None:
             self.close_callback()
         QtGui.QWizard.done(self, result)
+        self.userCancellation.emit(self.app_name)
 
     def on_currentIdChanged(self, page_id):
         """The current page changed."""

=== added directory 'ubuntuone_installer/gui/qt/main/tests'
=== added file 'ubuntuone_installer/gui/qt/main/tests/__init__.py'
--- ubuntuone_installer/gui/qt/main/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ ubuntuone_installer/gui/qt/main/tests/__init__.py	2011-07-18 18:59:29 +0000
@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+
+# Authors: Roberto Alsina <[e-mail]>
+#
+# Copyright 2011 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""The test suite for the Qt UI of the Ubuntu One Installer."""

=== added file 'ubuntuone_installer/gui/qt/main/tests/test_main.py'
--- ubuntuone_installer/gui/qt/main/tests/test_main.py	1970-01-01 00:00:00 +0000
+++ ubuntuone_installer/gui/qt/main/tests/test_main.py	2011-07-18 18:59:29 +0000
@@ -0,0 +1,97 @@
+# -*- coding: utf-8 -*-
+
+# Authors: Roberto Alsina <[e-mail]>
+#
+# Copyright 2011 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""The test suite for the Qt UI of the Ubuntu One Installer."""
+
+from ubuntuone_installer.tests import TestCase
+
+import ubuntuone_installer.gui.qt.main.windows as main_module
+
+# pylint: disable=W0212
+
+from ubuntuone.credentials import (
+    APP_NAME,
+    DESCRIPTION,
+    TC_URL,
+)
+
+
+class FakeSSOCred(object):
+    """A fake sso_cred."""
+
+    _registered_to_signal = False
+    _login_args = ()
+    on_credentials_found = None
+
+    def register_to_signals(self):
+        """Fake method."""
+        self._registered_to_signal = True
+
+    def login_or_register_to_get_credentials(self, *args, **kwargs):
+        """Fake method."""
+        self._login_args = (args, kwargs)
+
+
+class FakeSSOClient(object):
+
+    """Fake UbuntuSSOClient."""
+
+    sso_cred = FakeSSOCred()
+    testcase = None
+
+    def connect(self):
+        """Fake method."""
+        self.testcase._connected = True
+        return self
+
+
+class MainTestCase(TestCase):
+
+    """Tests for the main() function."""
+
+    _connected = False
+
+    def setUp(self):
+        """Prepare tests."""
+        self.patch(main_module, "UbuntuSSOClient", FakeSSOClient)
+        main_module.UbuntuSSOClient.testcase = self
+
+    def test_connect(self):
+        """Test that we instantiate the SSO Client and connect."""
+        main_module.main()
+        self.assertEqual(FakeSSOClient.testcase._connected, True)
+
+    def test_credentials_found_cb(self):
+        """Test that the credentials found signal is connected."""
+        main_module.main()
+        self.assertEqual(
+            FakeSSOClient.sso_cred.on_credentials_found_cb.__name__,
+            "stop")
+
+    def test_registered(self):
+        """Test that the credentials registers to signals."""
+        main_module.main()
+        self.assertEqual(FakeSSOClient.sso_cred._registered_to_signal,
+            True)
+
+    def test_check_register_args(self):
+        """Test that the credentials registers to signals."""
+        main_module.main()
+        self.assertEqual(FakeSSOClient.sso_cred._login_args,
+            ((APP_NAME, TC_URL, DESCRIPTION, 0),
+            {'ui_module': 'ubuntuone_installer.gui.qt.embedded_sso'}))

=== modified file 'ubuntuone_installer/gui/qt/main/windows.py'
--- ubuntuone_installer/gui/qt/main/windows.py	2011-06-30 17:53:23 +0000
+++ ubuntuone_installer/gui/qt/main/windows.py	2011-07-18 18:59:29 +0000
@@ -17,32 +17,36 @@
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Reactor installation for windows."""
 
-import sys
-
-from PyQt4 import Qt
-
 # pylint: disable=E1101, F0401, W0404
 
-
+from twisted.internet.defer import inlineCallbacks
+from ubuntu_sso.main.windows import UbuntuSSOClient
+
+from ubuntuone.credentials import (
+    APP_NAME,
+    DESCRIPTION,
+    TC_URL,
+)
+
+
+@inlineCallbacks
 def main():
-    """Start the txnamedpipes reactor and open the main window."""
-
-    # The following cannot be imported outside this function
-    # because u1trial already provides a reactor.
-    # pylint: disable=W0404, F0401
-
-    app = Qt.QApplication(sys.argv)
-    import qtreactor.qt4reactor
-    qtreactor.qt4reactor.install()
-
-    from ubuntuone_installer.gui.qt.gui import MainWindow
-    from twisted.internet import reactor
-
-    def really_quit():
-        """Close the application and cleanup."""
+    """Perform a client request to be logged in."""
+    client = UbuntuSSOClient()
+    client = yield client.connect()
+
+    @inlineCallbacks
+    def stop(*args):
+        """End the process."""
+        from twisted.internet import reactor
+        yield client.sso_cred.unregister_to_signals()
         reactor.stop()
 
-    window = MainWindow(close_callback=really_quit)
-    app.window = window
-    window.show()
-    reactor.run()
+    client.sso_cred.on_credentials_found_cb = stop
+    client.sso_cred.on_authorization_denied_cb = stop
+    client.sso_cred.on_credentials_error_cb = stop
+    yield client.sso_cred.register_to_signals()
+    yield client.sso_cred.login_or_register_to_get_credentials(APP_NAME,
+        TC_URL,
+        DESCRIPTION, 0,
+        ui_module="ubuntuone_installer.gui.qt.embedded_sso")

=== modified file 'ubuntuone_installer/gui/qt/tests/test_gui.py'
--- ubuntuone_installer/gui/qt/tests/test_gui.py	2011-07-15 18:47:24 +0000
+++ ubuntuone_installer/gui/qt/tests/test_gui.py	2011-07-18 18:59:29 +0000
@@ -29,14 +29,13 @@
 from ubuntuone.credentials import (
     APP_NAME,
     TC_URL,
-    PING_URL,
 )
 
 import ubuntu_sso
 
 from ubuntuone_installer.gui.qt import gui
 from ubuntuone_installer.gui.qt.tests import BaseTestCase
-from ubuntuone_installer.gui.qt.embedded_sso import UbuntuSSOClientGUI
+from ubuntuone_installer.gui.qt import embedded_sso
 from ubuntuone_installer.gui.qt import local_folders
 from ubuntuone_installer.gui.qt import setup_account
 from ubuntuone_installer.gui.qt.ui import (
@@ -51,15 +50,6 @@
          u'token_secret': u'qFYImEtlczPbsCnYyuwLoPDlPEnvNcIktZphPQklAWrvyfFMV'}
 
 
-class FakeCredentials(object):
-
-    """A very dummy Credentials object."""
-
-    def __init__(self, *a, **kw):
-        self.args = a
-        self.kwargs = kw
-
-
 class FakeController(object):
 
     """A fake controller for the tests."""
@@ -99,7 +89,6 @@
         """Initialize this test instance."""
         # Faking each SSO object instead of doing it lower
         # so we don't rely on any SSO behaviour
-        self.patch(gui, "Credentials", FakeCredentials)
         self.patch(gui, "SetUpAccountController", FakeController)
         self.patch(gui, "TosController", FakeController)
         self.patch(gui, "EmailVerificationController", FakeController)
@@ -196,15 +185,6 @@
         self.assertEqual(
             congrats_page.ui.progressContainer.isVisible(), False)
 
-    def test_credential_parameters(self):
-        """Compare credential parameters with control panel's."""
-        self.assertEqual(self.ui.creds.kwargs, {
-            'app_name': APP_NAME,
-            'ui_module': "ubuntuone_installer.gui.qt.embedded_sso",
-            'tc_url': TC_URL,
-            'ping_url': PING_URL,
-        })
-
     def test_current_user_controller_parameters(self):
         """Compare controller parameters with expected values."""
         self.assertEqual(self.ui.current_user_controller.args,
@@ -283,10 +263,37 @@
         self.assertEqual(self.ui.result(), self.ui.Rejected)
 
 
+class FakeSignal(object):
+
+    """A fake PyQt signal."""
+
+    def connect(self, target):
+        """Fake connect."""
+        self.target = target
+
+    def emit(self, *args):
+        """Fake emit."""
+        self.target(*args)
+
+
+class FakeMainWindow(object):
+
+    """A fake MainWindow."""
+
+    loginSuccess = FakeSignal()
+    registrationSuccess = FakeSignal()
+    userCancellation = FakeSignal()
+    shown = False
+
+    def show(self):
+        """Fake method."""
+        self.shown = True
+
+
 class SSOGuiTestCase(BaseTestCase):
     """Test the custom SSO GUI."""
 
-    class_ui = UbuntuSSOClientGUI
+    class_ui = embedded_sso.UbuntuSSOClientGUI
 
     def setUp(self):
         """Initialize this test instance."""
@@ -295,15 +302,37 @@
             'tc_url': TC_URL,
             'help_text': '',
         }
-        QtCore.QCoreApplication.instance().window = True
+        self.patch(embedded_sso, "MainWindow", FakeMainWindow)
         super(SSOGuiTestCase, self).setUp()
 
     def test_sso_client_gui(self):
-        """Ensure the class stores the right parameters."""
-        self.assertEqual(isinstance(
+        """Ensure the class instantiates correctly."""
+        self.assertIsInstance(
             self.ui.controller,
-            ubuntu_sso.qt.controllers.UbuntuSSOWizardController), True)
-        self.assertEqual(self.ui.view, True)
+            ubuntu_sso.qt.controllers.UbuntuSSOWizardController)
+        self.assertIsInstance(self.ui.view, FakeMainWindow)
+
+    def test_login_callback(self):
+        """Test that the login success callback is correctly handled."""
+        self.ui.login_success_callback = self._set_called
+        self.ui.view.loginSuccess.emit("app", "email")
+        self.assertEqual(self._called, (('app', 'email'), {}))
+
+    def test_registration_callback(self):
+        """Test that the registration success callback is correctly handled."""
+        self.ui.registration_success_callback = self._set_called
+        self.ui.view.registrationSuccess.emit("app", "email")
+        self.assertEqual(self._called, (('app', 'email'), {}))
+
+    def test_cancellation_callback(self):
+        """Test that the cancellation callback is called right."""
+        self.ui.user_cancellation_callback = self._set_called
+        self.ui.view.userCancellation.emit("app")
+        self.assertEqual(self._called, (('app',), {}))
+
+    def test_view_is_shown(self):
+        """Test that the view is actually shown."""
+        self.assertTrue(self.ui.view.shown)
 
 
 class LocalFoldersTestCase(BaseTestCase):