Merge lp:~mandel/ubuntu-sso-client/use-qt4-webclient-backend into lp:ubuntu-sso-client

Proposed by Manuel de la Peña
Status: Merged
Approved by: Roberto Alsina
Approved revision: 923
Merged at revision: 924
Proposed branch: lp:~mandel/ubuntu-sso-client/use-qt4-webclient-backend
Merge into: lp:ubuntu-sso-client
Diff against target: 162 lines (+46/-34)
4 files modified
ubuntu_sso/utils/webclient/__init__.py (+15/-2)
ubuntu_sso/utils/webclient/common.py (+1/-0)
ubuntu_sso/utils/webclient/qtnetwork.py (+10/-25)
ubuntu_sso/utils/webclient/tests/test_webclient.py (+20/-7)
To merge this branch: bzr merge lp:~mandel/ubuntu-sso-client/use-qt4-webclient-backend
Reviewer Review Type Date Requested Status
Roberto Alsina (community) Approve
Alejandro J. Cura (community) Approve
Review via email: mp+97934@code.launchpad.net

Commit message

- Stopped listening to the proxyAuthenticationRequired to avoid the dialog showing more than once (LP: #957170)
- Made changes in the way the webclient is selected to ensure that qt is used when possible (LP: #957169)

Description of the change

- Stopped listening to the proxyAuthenticationRequired to avoid the dialog showing more than once (LP: #957170)
- Made changes in the way the webclient is selected to ensure that qt is used when possible (LP: #957169)

Some tests have been skipped because Qt sucks in some areas which has been filled as bug #957313

To post a comment you must log in.
920. By Manuel de la Peña

Do not skip tests that do work

Revision history for this message
Alejandro J. Cura (alecu) wrote :

Tested IRL, seems to work both with authenticated proxies, with non-authenticated proxies and with no proxies.

review: Approve
921. By Manuel de la Peña

Merged with changes in trunk.

Revision history for this message
Roberto Alsina (ralsina) wrote :

Two little problems:

* Typo "cres" => "creds"
* There is no need to check QCoreApplication.instance() and QApplication.instance(), QCoreApplication is enough.

922. By Manuel de la Peña

Fixed according to reviews.

923. By Manuel de la Peña

Merged with trunk.

Revision history for this message
Roberto Alsina (ralsina) wrote :

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ubuntu_sso/utils/webclient/__init__.py'
2--- ubuntu_sso/utils/webclient/__init__.py 2012-03-08 12:35:25 +0000
3+++ ubuntu_sso/utils/webclient/__init__.py 2012-03-19 15:27:18 +0000
4@@ -27,8 +27,21 @@
5
6 def is_qt4reactor_installed():
7 """Check if the qt4reactor is installed."""
8- reactor = sys.modules.get("twisted.internet.reactor")
9- return reactor and getattr(reactor, "qApp", None)
10+ result = False
11+
12+ if not 'PyQt4' in sys.modules:
13+ return result
14+
15+ try:
16+ from PyQt4.QtCore import QCoreApplication
17+
18+ # we could be running a process with or without ui, and those are diff
19+ # apps.
20+ result = QCoreApplication.instance() is not None
21+ except ImportError:
22+ pass
23+
24+ return result
25
26
27 def webclient_module():
28
29=== modified file 'ubuntu_sso/utils/webclient/common.py'
30--- ubuntu_sso/utils/webclient/common.py 2012-03-09 12:04:31 +0000
31+++ ubuntu_sso/utils/webclient/common.py 2012-03-19 15:27:18 +0000
32@@ -187,6 +187,7 @@
33 self.proxy_username = creds['username']
34 self.proxy_password = creds['password']
35 defer.returnValue(True)
36+ logger.debug('Proxy creds not in keyring.')
37 defer.returnValue(False)
38
39 def _launch_proxy_creds_dialog(self, domain, retry):
40
41=== modified file 'ubuntu_sso/utils/webclient/qtnetwork.py'
42--- ubuntu_sso/utils/webclient/qtnetwork.py 2012-03-16 21:03:30 +0000
43+++ ubuntu_sso/utils/webclient/qtnetwork.py 2012-03-19 15:27:18 +0000
44@@ -118,6 +118,12 @@
45 schemes.append(scheme)
46 return schemes
47
48+ def find_proxy_for_scheme(self, scheme):
49+ """Return the proxy used for the scheme or default otherwise."""
50+ if scheme in self._proxies:
51+ return self._proxies[scheme]
52+ return self._proxies['default']
53+
54
55 class WebClient(BaseWebClient):
56 """A webclient with a qtnetwork backend."""
57@@ -128,8 +134,6 @@
58 self.nam = QNetworkAccessManager(QCoreApplication.instance())
59 self.nam.finished.connect(self._handle_finished)
60 self.nam.authenticationRequired.connect(self._handle_authentication)
61- self.nam.proxyAuthenticationRequired.connect(
62- self._handle_bad_proxy_authentication)
63 self.nam.sslErrors.connect(self._handle_ssl_errors)
64 self.replies = {}
65 self.proxy_factory = None
66@@ -155,7 +159,8 @@
67 reply = self.nam.sendCustomRequest(request, method, post_buffer)
68 # deal with auth errors in case we get
69 # ProxyAuthenticationRequiredError
70- d.addErrback(self._handle_proxy_authentication, method, reply,
71+ scheme = str(request.url().scheme())
72+ d.addErrback(self._handle_proxy_authentication, scheme, method, reply,
73 post_buffer)
74 self.replies[reply] = d
75 return d
76@@ -194,31 +199,11 @@
77 authenticator.setPassword(self.password)
78
79 @defer.inlineCallbacks
80- def _handle_bad_proxy_authentication(self, proxy, authenticator):
81- """Handle when we have a bad proxy."""
82- host = unicode(proxy.hostName()).encode('utf8')
83- port = proxy.port()
84- settings = dict(host=host, port=port)
85- # HACK: work around for reported bugs: QTBUG-16728 and #QTBUG-14850
86- got_creds = yield self.request_proxy_auth_credentials(host, True)
87- if got_creds:
88- logger.debug('Got proxy credentials from user.')
89- settings.update({'username': self.proxy_username,
90- 'password': self.proxy_password})
91- if (settings and 'username' in settings
92- and 'password' in settings):
93- authenticator.setUser(self.proxy_username)
94- authenticator.setPassword(self.proxy_password)
95- schemes = self.proxy_factory.find_proxy_schemes(proxy)
96- for scheme in schemes:
97- self.proxy_factory.update_proxy(scheme, settings)
98-
99- @defer.inlineCallbacks
100- def _handle_proxy_authentication(self, failure, method, reply,
101+ def _handle_proxy_authentication(self, failure, scheme, method, reply,
102 post_buffer=None, retry=False):
103 """The reply needs authentication."""
104 failure.trap(ProxyUnauthorizedError)
105- current_proxy = self.nam.proxy()
106+ current_proxy = self.proxy_factory.find_proxy_for_scheme(scheme)
107 host = unicode(current_proxy.hostName()).encode('utf8')
108 port = current_proxy.port()
109 settings = dict(host=host, port=port)
110
111=== modified file 'ubuntu_sso/utils/webclient/tests/test_webclient.py'
112--- ubuntu_sso/utils/webclient/tests/test_webclient.py 2012-03-16 21:03:30 +0000
113+++ ubuntu_sso/utils/webclient/tests/test_webclient.py 2012-03-19 15:27:18 +0000
114@@ -199,9 +199,13 @@
115 return root
116
117
118-class FakeReactor(object):
119- """A fake reactor object."""
120- qApp = "Sample qapp"
121+class FakeQApplication(object):
122+ """A fake Qt module."""
123+
124+ @classmethod
125+ def instance(cls):
126+ """Return the instance."""
127+ return cls
128
129
130 class ModuleSelectionTestCase(TestCase):
131@@ -212,10 +216,11 @@
132 self.patch(sys, "modules", {})
133 self.assertFalse(webclient.is_qt4reactor_installed())
134
135- def test_is_qt4reactor_installed_installed(self):
136+ def test_is_qt4reactor_installed_installed_core(self):
137 """When the qt4reactor is installed, it returns true."""
138- fake_sysmodules = {"twisted.internet.reactor": FakeReactor()}
139- self.patch(sys, "modules", fake_sysmodules)
140+ from PyQt4 import QtCore
141+
142+ self.patch(QtCore, 'QCoreApplication', FakeQApplication)
143 self.assertTrue(webclient.is_qt4reactor_installed())
144
145 def assert_module_name(self, module, expected_name):
146@@ -583,7 +588,15 @@
147 if WEBCLIENT_MODULE_NAME.endswith(".txweb"):
148 reason = "txweb does not support proxies."
149 test_anonymous_proxy_is_used.skip = reason
150- test_authenticated_proxy_is_used.skip = reason
151+ test_authenticated_proxy_is_used.kip = reason
152+ test_auth_proxy_is_used_creds_requested.skip = reason
153+ test_auth_proxy_is_requested_creds_bad_details.skip = reason
154+ test_auth_proxy_is_requested_creds_bad_details_user.skip = reason
155+ test_auth_proxy_is_requested_creds_bad_details_everywhere.skip = reason
156+ test_auth_proxy_is_requested_user_cancels.skip = reason
157+
158+ if WEBCLIENT_MODULE_NAME.endswith(".qtnetwork"):
159+ reason = 'QNetworkAccessManager is buggy.'
160 test_auth_proxy_is_used_creds_requested.skip = reason
161 test_auth_proxy_is_requested_creds_bad_details.skip = reason
162 test_auth_proxy_is_requested_creds_bad_details_user.skip = reason

Subscribers

People subscribed via source and target branches