Community
Participate
Working Groups
Instead of login-request-logout for each call, just login on startup (and on a lost login). this would save 2 requests and could speed up the connector.
Created attachment 137242 [details] Patch Not sure if this is enough for this task, tests do pass - with some limitation: I had to tweak the JiraClient Tests because with this change they failed sometimes. It seems to be some race condition (for which there already is a very ugly Thread.sleep(500) in the Test. I tried to get rid of it by calling a getIssueByKey instead, but it still kept failing sometimes. Also, I did some Tests with different setups (network connection as well as with this patch or without), here are the results: 500ms sleep: no patch: 0 /35 (WLAN) no patch: 0/35 (LAN) with patch: 3/35 (WLAN) with patch: 6/35 (LAN) 750ms sleep: with patch: 0/35 ( LAN) The numbers being the single assert that could sometimes fail (times failed / times invoked). The test that fails adds comments to an issue (calling client.updateIssue) and even though I call it twice with 500ms in between (or with a client.getIssueByKey in between) sometimes the second comment shows up as first comment. I do not see why. Any idea Steffen?
Created attachment 137243 [details] mylyn/context/zip
It's a good start and I have committed the changes. There are still a bunch of things missing though such as handling of authentication failures and test cases. I'll take this bug for now.
I have reverted the changes for now (bug 278819).
Created attachment 145218 [details] patch v2 There are a couple of things that needed fixing here. Bug 278819 broke because of missing detection of invalid sessions. This patch now introduces detection (which should, in theory, be a "user not logged in" site) and tries a second time with a login. In theory, because there is at least one scenario (requesting the XML representaiton of an issue) where the site is gzipped but the HTTP header type is html/text, so a workaround has been added to cover that. Finally for the test it turns out that the logout does not do a complete logout: after calling logout, calls still worked. A .clear() on the httpclient state fixed that issue. Steffen, was that what you were talking about in comment 3, or are there other things we need to take care of and other tests necessary?
Created attachment 145219 [details] mylyn/context/zip
Created attachment 145308 [details] patch v3 Great stuff! Patch applied with a minor modification: I extracted the method that checks for the authentication exception.
Marking resolved.
As descried on bug 277254 comment 6-8 there is still a scenario when this is failing: If a user switches networks while being logged in the session may expire and subsequent requests will be processed without being logged in which can have unexpected effects, e.g. query returning the wrong results. I couldn't find an obvious way to verify that a login session is valid. Thomas, can you investigate if there is a cheap request that we can make prior to each request to validate the session?
Seems like for now we have enable login vor every request again, see bug 277254 comment 10 ( https://bugs.eclipse.org/bugs/show_bug.cgi?id=277254#c10 ) for details
Created attachment 148362 [details] patch v4 This patch adds login for every request again. The previously added testcase should make sure this works, since a logout purges all cookies. Next step is to find out if there is a cheap way to check if we are still logged in.
Created attachment 148363 [details] mylyn/context/zip
Let me know when you want me to apply the patch.
well, I do not think there will be time to do the more elegant solution (checking for login), so for now this patch is all I can do I fear.
Have you tried a HEAD request for a protected page, e.g. http://mylyn.eclipse.org/jira-enterprise-3.13.5/secure/UpdateUserPreferences!default.jspa ? If you are not logged in that should return a redirect and a 200 otherwise.
Created attachment 148584 [details] patch that uses head request
Created attachment 148585 [details] patch that uses head request
Created attachment 148586 [details] mylyn/context/zip
I have committed a change as suggested in comment 15.
cool stuff, thanks Steffen
