Bug 274330 - [ui] Inconsistent swapping of repository contents
Summary: [ui] Inconsistent swapping of repository contents
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: p2 (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5 RC1   Edit
Assignee: Susan McCourt CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 267965 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-04-29 11:49 EDT by Matthew Piggott CLA
Modified: 2009-05-13 17:13 EDT (History)
6 users (show)

See Also:
dj.houghton: review+


Attachments
patch (1.43 KB, patch)
2009-05-11 18:35 EDT, Susan McCourt CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Piggott CLA 2009-04-29 11:49:50 EDT
I can't get this to occur consistently, but twice while testing build I20090429-0100 I've typed the address for fullmoon's 3.5-I-build repository, and software from another site is displayed.  The other site is associated with the contents of the 3.5-I-build repository.  Once this happens the contents of the two sites remain swapped until leaving the dialog.

The 3.5-I-builds repo is a composite repository.

I've had this occur once while in the Install New Software dialog, and once when using the Target Definition.
Comment 1 John Arthorne CLA 2009-04-29 14:16:55 EDT
I have seen this as well, but was waiting to see if I could create steps to reproduce it before entering a bug. I believe Pascal has seen this too.
Comment 2 Susan McCourt CLA 2009-04-29 19:25:07 EDT
*** Bug 267965 has been marked as a duplicate of this bug. ***
Comment 3 Susan McCourt CLA 2009-04-29 19:28:56 EDT
I had hoped this was fixed because I had a lot of changes in this space to be more forgiving when looking for combo matches, but it's still a problem.  If anyone can come up with a repeatable test case, that'd be great.

My suspicion is that it happens the first time the repo is loaded (when the child repos are added)?  I sometimes adjust the repo combo based on repo events, but the intent is to ignore non-user generated events.

So far everyone reports seeing this when using the I-build repo.  I don't know if that is meaningful (event flow for composite repos) or coincidence (because that's one of the first sites we add in the test plan and in upgrading our I-builds).

If anyone sees this with a different repo, please report here.
I'm marking 3.5 to keep on the radar for now.
Comment 4 Susan McCourt CLA 2009-04-30 01:31:36 EDT
Thinking about this a bit:
I have a list of known URI's and a list of strings based on the URI list.  I map back and forth between those lists to map combo selections to the associated URI that should be used. 

It's possible that there's some case where one or the other is being set. They should always be updated together, or else you might see something like this.

Another possible factor could be when a repo gets loaded, has no nickname, and the nickname is set to the provider name.  In that case we would get a change event, and if we aren't resetting the combo string then maybe we are unable to map from a location back to the combo string.  

This might explain bug 274334 and bug 272894.
Comment 5 Susan McCourt CLA 2009-04-30 01:46:18 EDT
Note to self: 
> Another possible factor could be when a repo gets loaded, has no nickname, and
> the nickname is set to the provider name.  In that case we would get a change
> event, and if we aren't resetting the combo string then maybe we are unable to
> map from a location back to the combo string.  


This looks like it could cause the mismatch (not sure about the swapping)...
protected void repositoryAdded(RepositoryEvent e) {
    if (e instanceof UIRepositoryEvent) {  		 fillRepoCombo(getSiteString(e.getRepositoryLocation()));
     }
}

At the time the repo combo is filled the nickname is different than the next time we go to look it up.  We should watch for nickname changes and update the combo accordingly.  Check senders of getSiteString(URI)
Comment 6 John Arthorne CLA 2009-04-30 13:31:36 EDT
When I saw this, I had a feeling it was more of an "off by one" error where selecting entry N in the combo would show the contents of N+1 in the pane below. However I'm not sure about this, and I'll confirm if I can reproduce again.
Comment 7 Susan McCourt CLA 2009-04-30 14:05:04 EDT
(In reply to comment #6)
> When I saw this, I had a feeling it was more of an "off by one" error where
> selecting entry N in the combo would show the contents of N+1 in the pane
> below. However I'm not sure about this, and I'll confirm if I can reproduce
> again.
> 
thanks, that's helpful.
Any random clues people have might help me examine the code and find the error, then backtrack to when it happens.

Comment 8 Susan McCourt CLA 2009-05-07 19:32:08 EDT
Henrik & Remy - any help reproducing this would be great
Comment 9 Remy Suen CLA 2009-05-08 10:12:33 EDT
(In reply to comment #8)
> Henrik & Remy - any help reproducing this would be great

I would like to try and help but I don't work for IBM so I don't know the fullmoon URLs nor can I access them. :o Is there another URL that can be used?
Comment 10 John Arthorne CLA 2009-05-08 11:21:04 EDT
Fullmoon is just our local mirror of:

http://download.eclipse.org/eclipse/updates/3.5-I-builds
Comment 11 Remy Suen CLA 2009-05-08 11:31:37 EDT
(In reply to comment #10)
> Fullmoon is just our local mirror of:
> 
> http://download.eclipse.org/eclipse/updates/3.5-I-builds

Ah...I got confused by "The other site is associated with the contents of the 3.5-I-build repository.". It sounded like "I had two 3.5 I builds sites and the public and private mirrors got their content swapped". I guess it's actually "The 3.5 I builds site content got swapped with the content of another random site".
Comment 12 Susan McCourt CLA 2009-05-11 18:35:42 EDT
Created attachment 135236 [details]
patch

Studying the code, I see how this could happen.  Now I just have to find the circumstance to reproduce it and show that this code fixes the problem.

When the repo combo box is filled, two parallel arrays are created.  One holds the URI's, one holds the strings.  These are always created together and one is never updated without the other.  So far, so good.

However...these arrays are then sorted.  The intention is they should both be sorted so that the index in one array still maps to the same item in the other array.

The sort comparator for the URI was using a method to obtain the sort string, rather than referencing the original matching string.  This means if a URI's site string (Name - Location) had changed since the two arrays were created, the URI array would sort differently than the strings in the combo.  This would most certainly cause a "swapping" effect whereby selecting one name in the combo would consistently choose a different site.

So it can happen when a site's nickname or name changes while the sort is occurring.  I think this happens for sites with no user names on a repo load.  Once the repo loads, it has a name, and if the load completed after the list was made and before or during sorting, this could happen.
Comment 13 Susan McCourt CLA 2009-05-12 17:08:38 EDT
Adding John for review.
I spent a good bit of time trying to reproduce this in the UI and could not reliably reproduce it.  I think my connection to the I-build site is too slow to easily reproduce it, as the problem can occur when a repo gets loaded (and therefore gets its nickname set by ProvisioningUtil.loadMetadataRepository(...) before the combo sorting is done.

I also spent a good deal of time trying to automate a test case but I had to change so much of the real code to make the data structures visible that it was not going to prove much.

The only way I could reproduce this was by adding breakpoints in 
RepositorySelectionGroup.sortRepoItems(...)

I added a breakpoint just before the last statement (where the URI array is sorted).  I also added a breakpoint in the URI collator.  I was able to reproduce a case while stepping through the code where the combo string array had (in position 2 and 3)

Galileo	- http://download.eclipse.org/releases/galileo
http://fullmoon.ottawa.ibm.com/updates/3.5-I-builds

and the URI after sorting had (in position 2 and 3)
http://fullmoon.ottawa.ibm.com/updates/3.5-I-builds
http://download.eclipse.org/releases/galileo

The net effect was the swapping of these two repos.  The reason is that by the time the URI array was sorted, the name of the I-build repo had been set to 
"Eclipse Project Test Site" and it sorted higher in the array than it did when it had no name.

With the patch applied, I did not observe this effect.

It's a little hard to consistently reproduce because it involves multiple calls to the sort method, caused by the handling of the repo add events as the child repos are loaded.

I am confident after observing it once in the debugger that this was the cause of the swapping.

Comment 14 Susan McCourt CLA 2009-05-13 15:52:56 EDT
changing reviewer to DJ, John is backed up with other reviews
Comment 15 DJ Houghton CLA 2009-05-13 17:13:22 EDT
Released.