Bug 149280 - Ordering of subsystems is not always consistent
Summary: Ordering of subsystems is not always consistent
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P3 minor (vote)
Target Milestone: 2.0   Edit
Assignee: Kushal Munir CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-06-30 03:54 EDT by Martin Oberhuber CLA
Modified: 2008-08-13 13:21 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Oberhuber CLA 2006-06-30 03:54:01 EDT
The ordering of subsystems is not always consistent:
* In the New Connection Wizard, I get "ssh.shells" before "ssh.files" on Windows;
  but on Linux with the ssh and local runtimes only installed, I get it the other
  way round (files before shells). For dstore, the ordering is the same as always.
* In the RSE Tree, I also see the ssh.shells subsystem before ssh.files; on Linux
  it is the other way round.

This seems to be a regression since the "Wizard completely Replaceable" changes checked in between 2006-06-28 and 2006-60-30.
Comment 1 Kushal Munir CLA 2006-08-17 16:55:13 EDT
There is no way to guarantee the order of subsystems since the plugin manifest files may not be read in the same order on all platforms (or even on different invocations with a new workspace).

I'd like to add a "priority" attribute so that there is a way to order the subsystems. This attribute will be an integer value. Subsystem configurations with higher priority would appear earlier in the New Connection wizard and the subsystems would appear above subsystems with lower priority in the Remote Systems tree view.

This attribute will be documented in the extension point exsd. It will be optional . Subsystem configurations without the priority attribute will be treated as having the lowest priority (i.e. any subsystem configuration with a priority specified will appear before/above subsystem configurations without the priority attribute).
Comment 2 Martin Oberhuber CLA 2006-08-17 17:31:28 EDT
Sounds good.
I'd suggest priority 100=files, 200=processes, 300=shells;
Subsystems without priority should be added at the end (i.e. like priority=MAXINT)

I see a potential problem with multiple subsystems of the same category -- ssh.files, dstore.files, ftp.files are all in the same category and can be exchanged at runtime. In the wizard, they all share the same wizard page. What if priorities are different for each of these by programming error? e.g. dstore.files=100, dstore.processes=200, ssh.files=300? Would subsystems be re-ordered at runtime? Or should we take the minimum priority of a list of subsystems in the same category?

Another option might be to sort alphabetically by "category" entry -- this would allow to keep the existing extension point, and the existing order (files, processes, shells).

Comment 3 Kushal Munir CLA 2006-08-18 03:07:13 EDT
I considered sorting alphabetically by category, but some categories are always higher priority than others in my opinion. For example, I would say that the files category is higher priority than the daytime category in the examples plugin.

The priority attribute seems to be the best solution at the moment and this is what I'm going with. The values are what was suggested for the various categories. There are a few scenarios to consider. Suppose a new subsystem configuration extension belonging to the "files" category is added and the value is something other than 100. Let's say it's 400.

In the New Connection wizard, this will not cause a problem as long as one of the default subsystem configurations belonging to the "files" category is available (i.e. one with priority = 100). However, if no default subsystem configuration belonging to the "files" category is available, but default configurations belonging to the "processes" category are (i.e. ones with priority = 200) then the wizard page for the "processes" category will come up before the wizard page for the "files" category.

Now, let's say that the default configurations belonging to the "files" category are available, but when creating the connection, the user selects the new "files" subsystem configuration (with priority = 400) and a default "processes" subsystem configuration (with priority = 200). In the Remote Systems tree view, and other RSE views, the subsystem belonging to the "processes" category will be shown before the subsystem belonging to the "files" category.

One way to work around these problems would be to have the priority attribute apply to the category. However, right now, a category is an attribute itself, and it's redundant to have to specify it for each subsystem configuration. What would be nice is if we allowed a category to be defined in the subsystemConfigurations extension point with a priority attribute and have any subsystem configuration refer to the category id. So, in effect, we would allow categories to be defined separately from the subsystem configurations. This would also allow us to define categories in more generic plugins. For example, the "files" category could be defined in org.eclipse.rse.subsystems.files.core. The benefit of this is that the "files" category along with its priority would be available to anyone providing a new subsystem configuration as long as they pick up this base plugin (even if they don't pick up any of the default subsystem configurations). Alternatively, we can define the default categories in the lowest level RSE plugin where the subsystemConfigurations extension point is defined (currently, that's the rse.ui plugin), and this would ensure that RSE extenders will know the priority values of the default categories. 

I won't mark the defect as fixed yet, because I'd like to pursue this post M4 (subject to further discussion), but I will lower the priority of the defect since the problem is fixed for the default subsystem configurations provided in RSE.
Comment 4 Kushal Munir CLA 2006-08-31 10:27:21 EDT
Martin, I'd like to get your opinion on this. Is what I'm suggesting above something we should consider for 1.0? It should be easy to do and I think is very low risk.
Comment 5 Martin Oberhuber CLA 2006-08-31 11:07:49 EDT
Few points:

1. I dont think considering the "category" attribute is a good idea since it is
   optional. RSE uses the getServiceType() interface in order to associate
   subsystems of the same class, and not the category -- see 
   SystemRegistry.getSubSystemConfigurationsBySystemType(String,boolean).

2. I think we can expect from users that they honor the pre-defined priorities
   per subsystem configuration, we do not need to enforce consistency. So
   basically, your current approach is sufficient I think.

3. For the (unlikely) case that we do have subsystems of the same service type
   but with different priorities, we should do something deterministic. I 
   would suggest that the integer minimum of all registered configurations is
   taken and applied to the system.
   When the wizard assembles its pages, it needs to iterate over the configured
   subsystems anyway, so computing the minimum of all priorities should not
   be a problem.

Does this answer your questions?
Comment 6 Martin Oberhuber CLA 2006-08-31 11:10:38 EDT
BTW, I think you could also document with the extension point docs for the "priority" attribute that the user MUST ensure that all subsystem configurations which use the same service type in getServiceType() also have the same priority.

If you should run into any issues with implementing the "minimum" strategy, you could also consider the bug fixed just by documentation, since we are telling users that they must not fuss with the priority.
Comment 7 Martin Oberhuber CLA 2006-10-24 14:58:27 EDT
Kushal - didnt you fix this for M5 already by implementing the "priority" attribute? If yes, please set the Target Milestone and set it FIXED.
Comment 8 Martin Oberhuber CLA 2006-11-23 11:21:43 EST
I didn't observe incorrectly ordered subsystems since a while now, so I guess this was fixed with M5 indeed.

What I did observe was some different behavior in the New Connection Wizard, which I've recorded in bug 165674.

Kushal if you do not plan to make any more changes to the subsystem priorities (I do not see any need to), please close this bug as fixed per 1.0M5.
Comment 9 Kushal Munir CLA 2007-05-30 09:11:53 EDT
Docs need to be updated.
Comment 10 Kushal Munir CLA 2007-06-20 15:56:17 EDT
Extension point doc is already updated.
Comment 11 Martin Oberhuber CLA 2008-08-13 13:21:00 EDT
[target cleanup] 2.0 RC3 was the original target milestone for this bug