Comment 1 Martin Oberhuber 2007-07-16 05:47:15 EDT

Kevin you've been most exposed to search, could you investigate?

Comment 2 Kevin Doyle 2007-07-23 15:22:24 EDT

Created attachment 74390 [details]
Changed mentions of Vector to List so ArrayList is handled.

Another case where trying to cast an ArrayList to Vector.  Changed it to List's.  Also changed getRemoteResourceAbsoluteName(Object) to use an ISystemViewElementAdapter instead of IRemoteObjectIdentifier.

Legal Message: I, Kevin Doyle, declare that I developed attached code from
scratch, without referencing any 3rd party materials except material licensed
under the EPL. I am authorized by my employer, IBM Canada Ltd. to make this
contribution under the EPL.

Comment 3 David McKnight 2007-07-23 15:30:51 EDT

I've committed this to cvs.

Comment 4 Martin Oberhuber 2007-07-23 18:28:39 EDT

(In reply to comment #2)
> Another case where trying to cast an ArrayList to Vector.

Have you ever analyzed why we keep getting ArrayList passed into methods that expect a Vector? - I'm all in favor of moving away from Vector to the more general List interface, but in the light of getting bugs fixed for 2.0.1 we should also fix the source of the issue, e.g.
    SystemCommonDeleteAction
which fires remoteChangedEvents with an ArrayList. We should also consider updating API docs to read what our API really expects -- ISystemRemoteChangeEvents.SYSTEM_REMOTE_RESOURCE_DELETED talks about "List" being expected although ISystemResourceChangeEvents.EVENT_REFRESH_REMOTE talks about "Vector" being expected. 

This is inconsistent, and for the sake of being API compatible in 2.0.1 I'd like to see us fire events with the less general "Vector" but expect events with the more general "List". With respect to the API docs, I'm not sure but there might even already exist a bug report asking for this to be sanitized, and if it is not yet there it should be created (I'm in favor of moving to "List" everywhere).

> Also changed getRemoteResourceAbsoluteName(Object) to use an
> ISystemViewElementAdapter instead of IRemoteObjectIdentifier.

Why? This change makes no sense. ISystemViewElementAdapter is a subclass of IRemoteObjectIdentifier, and by moving to a less general interface you're essentially breaking API (somebody who registers an adapter that's just an IRemoteObjectIdentifier but not an ISystemViewElementAdapter would no longer get his adapter called.

Reopening to get the previous version of getting the adapter restored, and getting the internal ArrayList replaced by a Vector in SystemCommonDeleteAction.

Comment 5 Kevin Doyle 2007-07-23 23:00:16 EDT

I do not see a bug open for fixing up the API docs in regards to what is expected.  Though I do remember talking about Lists instead of Vector's, but maybe that was for the Validator's only.

In regards to making the change to ISystemViewElementAdapter.  I think I understood this wrong before.  Since the name isn't view specific and ISystemViewElementAdapter is a subclass of IRemoteObjectIdentifier it would be okay to use IRemoteObjectIdentifier for getting the absolute name, but for anything relating to the view itself we should be using ISystemViewElementAdapter.  

SystemView, SystemScratchpadView, and SystemTableView all use ISystemViewElementAdapter for getRemoteResourceAbsoluteName().  I'm not sure why we are doing it differently in this case.  It's weird seeing things done differently in different spots.

Though looking back at this I noticed that the code for getting an IRemoteObjectIdentifier adapter doesn't work.  It returns null.

return (IRemoteObjectIdentifier) ((IAdaptable)o).getAdapter(IRemoteObjectIdentifier.class);

Also SystemView has a protected getRemoteObjectIdentifier(Object) method that is never even used.  With that same line of code.

Comment 6 Martin Oberhuber 2007-07-24 01:30:01 EDT

(In reply to comment #5)
> SystemView, SystemScratchpadView, and SystemTableView all use
> ISystemViewElementAdapter for getRemoteResourceAbsoluteName().  I'm not sure
> why we are doing it differently in this case.  It's weird seeing things done
> differently in different spots.

That's right.

> Though looking back at this I noticed that the code for getting an
> IRemoteObjectIdentifier adapter doesn't work.  It returns null.

I now see why -- the plugin.xml for files.ui has:
<!-- TODO Check: These are not in the adapter factories 
     getAdapterList() Although the adapter implements them
   <adapter type="org.eclipse.rse.core.subsystems.IRemoteObjectIdentifier"/>
-->

The comments would need to be removed, and the factory class updated, to make it work. I want to think a little bit more about this, but now that it didn't ever work your change certainly is not a regression. Let's keep your code in the repository for now.

> Also SystemView has a protected getRemoteObjectIdentifier(Object) method that
> is never even used.  With that same line of code.

We should mark that deprecated right away since it does not work in 2.0.1 as of today.

Comment 7 Kevin Doyle 2007-07-24 10:56:47 EDT

Created attachment 74471 [details]
SystemCommonDeleteAction now pass's Vector instead of ArrayList

This patch updates SystemCommonDeleteAction, so Vector's are passed instead of ArrayList's.  Also went through all class's implementing ISystemRemoteChangeListener to make sure they used the more general List.  SystemMonitorViewPart was the only one that wasn't and it's updated in the patch.

Legal Message: I, Kevin Doyle, declare that I developed attached code from
scratch, without referencing any 3rd party materials except material licensed
under the EPL. I am authorized by my employer, IBM Canada Ltd. to make this
contribution under the EPL.

Comment 8 Kevin Doyle 2007-08-08 11:38:01 EDT

Created attachment 75681 [details]
Updated to deprecate getRemoteObjectIdentifier

Forgot to deprecate getRemoteObjectIdentifier in the previous patch.  Updated patch deprecates it.

Legal Message: I, Kevin Doyle, declare that I developed attached code from
scratch, without referencing any 3rd party materials except material licensed
under the EPL. I am authorized by my employer, IBM Canada Ltd. to make this
contribution under the EPL.

Comment 9 Xuan Chen 2007-08-08 17:02:27 EDT

The patch looks good.  

Comment 10 Xuan Chen 2007-08-08 17:08:01 EDT

You forgot to update the Contributor list in SystemView.java.  Please add it in.  Thanks.

Comment 11 Kevin Doyle 2007-08-08 17:30:41 EDT

Created attachment 75712 [details]
Added Contributer Line

Legal Message: I, Kevin Doyle, declare that I developed attached code from
scratch, without referencing any 3rd party materials except material licensed
under the EPL. I am authorized by my employer, IBM Canada Ltd. to make this
contribution under the EPL.

Comment 12 Xuan Chen 2007-08-08 23:45:08 EDT

I've committed the patch.

Comment 13 Kevin Doyle 2007-09-13 13:57:51 EDT

Verified fixed in 2.0.1 RC1.