Bug 410596 - user name shows as 'null' when not configured in Gerrit
Summary: user name shows as 'null' when not configured in Gerrit
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: Macintosh Mac OS X
: P2 normal (vote)
Target Milestone: 2.0.1   Edit
Assignee: Steffen Pingel CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 412098 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-06-12 09:34 EDT by Steffen Pingel CLA
Modified: 2013-07-31 12:24 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Steffen Pingel CLA 2013-06-12 09:34:21 EDT
Steps:
1. Create Gerrit repository as anonymous
2. Update repository configuration
3. Open repository settings and enter credentials
4. Validate

The message is "Logged in as null".
Comment 1 Miles Parker CLA 2013-06-12 14:48:30 EDT
Related / cause-of bug 410597?
Comment 2 Steffen Pingel CLA 2013-06-12 15:19:46 EDT
(In reply to comment #1)
> Related / cause-of bug 410597?

I don't know. I am assuming that cached state in the connector is not properly discarded when repository properties change which is causing this defect. It doesn't necessarily need to be related to bug 410597.
Comment 3 Miles Parker CLA 2013-06-24 18:28:55 EDT
Postponing to 2.1 for now.
Comment 4 Tomasz Zarna CLA 2013-07-02 07:47:36 EDT
See also bug 412098 for a different issue with repo validation.
Comment 5 Miles Parker CLA 2013-07-04 21:01:23 EDT
*** Bug 412098 has been marked as a duplicate of this bug. ***
Comment 6 Miles Parker CLA 2013-07-04 21:03:47 EDT
Fixed by: https://git.eclipse.org/r/#/c/14294/ 
Please verify. Candidate for possible inclusion in 2.0.1.

Actually, I think it was the same cause. I'm not sure given description, but it's even possible that Steffen had a valid connection, but it wasn't being reported because the Account full name was returning null even though a user exists for the Gerrit Tests. In addition to fixing that issue, this change clarifies and simplifies the interface, allowing explicit resetting of connection.
Comment 7 Steffen Pingel CLA 2013-07-12 13:25:38 EDT
Miles, what in review 14429 would fix this problem? Could isolate that particular change and provide a test case?
Comment 9 Steffen Pingel CLA 2013-07-12 18:19:44 EDT
We can improve GerritSystemInfo.getFullName() with the change you are proposing and maybe you are right that that was the actual problem. 

The client tests look good but they don't really test the validation scenario since the life-cycle of the client is managed in the connector in that case where I was suspecting the bug but from looking at the code I don't see anything obviously wrong.

Are you if I take your change and rebase the two classes you pointed out against the latest master?
Comment 10 Miles Parker CLA 2013-07-15 13:04:52 EDT
(In reply to comment #9)
> Are you if I take your change and rebase the two classes you pointed out against
> the latest master?

Totally. :) I can do that to. I'm not wedded to any particular approach here, it just probably doesn't make sense for Tomek and I to be in the same code at the same time.
Comment 11 Miles Parker CLA 2013-07-15 13:07:32 EDT
(In reply to comment #9)
> We can improve GerritSystemInfo.getFullName() with the change you are proposing
> and maybe you are right that that was the actual problem.
> 
> The client tests look good but they don't really test the validation scenario
> since the life-cycle of the client is managed in the connector in that case
> where I was suspecting the bug but from looking at the code I don't see anything
> obviously wrong.

What I was seeing and the reports that we were getting were that we were having difficulties when users first logged in (or didn't) and then subsequently made a change. It seemed that configuraiton changes weren't taking. The tests were designed to ensure that the configuration change actually got made. I found the code as written quite opaque to the extent that I couldn't understand even what was suppossed to be happening, which is what drove me to refactor it as I did.
Comment 12 Miles Parker CLA 2013-07-15 13:29:28 EDT
I'm reassinging to default. Let's discuss how we want to cover this one and bug 410597.
Comment 13 Steffen Pingel CLA 2013-07-31 12:24:18 EDT
I committed a fix for the username problem in 604edce1d17845034a9f10dcd19a698d772bde30 and updated the summary accordingly. It's not clear to me whether there are other problems beyond that. Please open separate bugs for any outstanding issues.