| Summary: | SystemRemoteFileDialog shows Local contents even when specifying a SystemType | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] Target Management | Reporter: | Kevin Doyle <kjdoyle> | ||||||
| Component: | RSE | Assignee: | David McKnight <dmcknigh> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | Martin Oberhuber <mober.at+eclipse> | ||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | kjdoyle, rdibugs | ||||||
| Version: | 3.0.1 | ||||||||
| Target Milestone: | 3.1 M3 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 253769 | ||||||||
| Attachments: |
|
||||||||
Created attachment 116654 [details]
patch to determine valid connection based on system types
This patch checks the system types to determine whether a connection is valid or not.
Kevin, does this help?
No, I still get the contents of the Local file subsystem when it displays the dialog even though it's not an iseries connection. (In reply to comment #2) > No, I still get the contents of the Local file subsystem when it displays the > dialog even though it's not an iseries connection. > Do you call setSystemType() on the action? For example: IRSESystemType systemType = RSECorePlugin.getTheCoreRegistry().getSystemTypeById(IRSESystemType.SYSTEMTYPE_ISERIES_ID); action.setSystemType(systemType); (In reply to comment #3) > Do you call setSystemType() on the action? For example: > > IRSESystemType systemType = > RSECorePlugin.getTheCoreRegistry().getSystemTypeById(IRSESystemType.SYSTEMTYPE_ISERIES_ID); > action.setSystemType(systemType); > Basically, we extend SystemSelectRemoteFileAction and in our constructor after calling super() we do: IRSESystemType systemType = RSECorePlugin.getTheCoreRegistry().getSystemTypeById(IRSESystemType.SYSTEMTYPE_ISERIES_ID); super.setSystemType(systemType); Created attachment 116975 [details]
updated patch
Kevin, can you try again with this patch?
Works well and don't see any problems with the patch diff. Thanks Dave. I've committed the fix to cvs. Does this need to be backported? Yes please. We are still going through the list of which ones we want backported, but this is on the list. (In reply to comment #8) > Yes please. We are still going through the list of which ones we want > backported, but this is on the list. > I've created bug 253769 for the backport. In SystemResourceSelectionInputProvider line 62, please do NOT compare IRSESystemType objects with ==:
hostType == type
Somebody could wand to implement a proxy to a systemType, which has the same ID but returns different Properties for some reason. PLEASE, compare with .equals()!! FindBugs should issue an error for == comparisons, and I think even Eclipse can issue an error for this.
In getSystemViewRoots() -- line 143 -- you can run into an NPE if none of the hosts are valid. You should return
new Object[0]
in that case.
Reopening to get fixed.
(In reply to comment #10) > In SystemResourceSelectionInputProvider line 62, please do NOT compare > IRSESystemType objects with ==: > hostType == type > > Somebody could wand to implement a proxy to a systemType, which has the same ID > but returns different Properties for some reason. PLEASE, compare with > .equals()!! FindBugs should issue an error for == comparisons, and I think even > Eclipse can issue an error for this. > > In getSystemViewRoots() -- line 143 -- you can run into an NPE if none of the > hosts are valid. You should return > new Object[0] > in that case. > > Reopening to get fixed. > I wouldn't have expected the proxy to systemType. Anyway, I've updated the fix in cvs. expected or not, equals() should be the standard in Java. Use == only in very special cases and with precautions. What about the potential NPE in line 140 of SystemResourceSelectionInputProvider ? Note that an NPE here is a potential regression because before your fix, the "restrict to" would not have been honored. (In reply to comment #13) > What about the potential NPE in line 140 of > SystemResourceSelectionInputProvider ? Note that an NPE here is a potential > regression because before your fix, the "restrict to" would not have been > honored. > Updated to return new Object[0] in the case where no connections are available. |
We are using SystemSelectRemoteFileAction and setting the systemType to IRSESystemType.SYSTEMTYPE_ISERIES_ID. However when the dialog loads up it shows the Local connection filters. It should be showing the filters for the connection specified in the combo box. SystemSelectRemoteFileAction.createDialog(Shell) does: SystemRemoteFileDialog dlg = null; if (dlgTitle == null) dlg = new SystemRemoteFileDialog(shell); else dlg = new SystemRemoteFileDialog(shell, dlgTitle); SystemRemoteFileDialog does: new SystemRemoteFileSelectionInputProvider() Which does: // choose random host ISystemRegistry registry = RSECorePlugin.getTheSystemRegistry(); IHost[] hosts = registry.getHosts(); if (hosts != null && hosts.length>0) { _connection = hosts[0]; } hosts[0] is the Local connection.