Bug 207100 - [api] Event to notify listener of a completed file upload.
Summary: [api] Event to notify listener of a completed file upload.
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 2.0.1   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.0 M3   Edit
Assignee: David McKnight CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2007-10-22 19:52 EDT by Ankit Pasricha CLA
Modified: 2007-10-31 12:31 EDT (History)
3 users (show)

See Also:


Attachments
patch to fire event when file uploaded or downloaded (8.79 KB, patch)
2007-10-24 11:06 EDT, David McKnight CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ankit Pasricha CLA 2007-10-22 19:52:19 EDT
I have a plugin that needs to do some processing AFTER a file has been uploaded completely from the local to a remote system. The upload might be performed in the UI (e.g. copying and pasting from local to remote) or programmatically.
I would like RSE to let me know through an event when a file upload finishes. The event should notify me when my plugin, RSE or any other plugin causes an upload to happen. I would like this to be supported for dstore as well as other connection mechanisms (if possible).
Comment 1 Martin Oberhuber CLA 2007-10-23 06:22:14 EDT
The request sounds reasonable to me.
Dave, what do you think?
Comment 2 David McKnight CLA 2007-10-23 08:59:01 EDT
Yes, I agree this would be a good enhancement.
Comment 3 David McKnight CLA 2007-10-24 11:06:31 EDT
Created attachment 81068 [details]
patch to fire event when file uploaded or downloaded

In the patch I've provided, there are two new ISystemRemoteChangeEvents:

/**
	 * Event Type: a remote resource was uploaded
	 * 
	 * The event stores the following event parameters:
	 * <ul> 
	 *  <li>resource - the remote resource object, or absolute name of the resource as would be given by calling getAbsoluteName on its remote adapter
	 *  <li>resourceParent - the remote resource's parent object, or absolute name, if that is known. If it is non-null, this will aid in refreshing occurrences of that parent.
	 *  <li>subsystem - the subsystem which contains this remote resource. This allows the search for impacts to be 
	 *   limited to subsystems of the same parent factory, and to connections with the same hostname as the subsystem's connection.
	 * </ul>
	 */
	public static final int SYSTEM_REMOTE_RESOURCE_UPLOADED = 20;
	
	/**
	 * Event Type: a remote resource was downloaded
	 * 
	 * The event stores the following event parameters:
	 * <ul> 
	 *  <li>resource - the remote resource object, or absolute name of the resource as would be given by calling getAbsoluteName on its remote adapter
	 *  <li>resourceParent - the remote resource's parent object, or absolute name, if that is known. If it is non-null, this will aid in refreshing occurrences of that parent.
	 *  <li>subsystem - the subsystem which contains this remote resource. This allows the search for impacts to be 
	 *   limited to subsystems of the same parent factory, and to connections with the same hostname as the subsystem's connection.
	 * </ul>
	 */
	public static final int SYSTEM_REMOTE_RESOURCE_DOWNLOADED = 24;
	
I've put code in the FileServiceSubSystem to fire these events after download and after upload.  

Ankit, will this solve your requirements? 

Martin, do you see any issues with this approach?
Comment 4 Martin Oberhuber CLA 2007-10-24 11:12:50 EDT
I find it odd that events which relate to the files subsystem only are defined in a Core module.

Would it make sense to re-interpret the events to a general concept of upload/download that's not necessarily tied to files but any kind of resource?

Or, would it make sense to define these events in a different class inside org.eclipse.rse.subsystems.files.core ?
Comment 5 Martin Oberhuber CLA 2007-10-24 11:16:19 EDT
Also - is there really a need to have the 1st parameter specified as "the remote object OR its absolute name"? 
Such an alternative makes it harder for event receivers to handle the event. It would be easier if it were clearly defined what the argument should be.
Comment 6 David McKnight CLA 2007-10-24 11:43:55 EDT
(In reply to comment #4)
> I find it odd that events which relate to the files subsystem only are defined
> in a Core module.
> Would it make sense to re-interpret the events to a general concept of
> upload/download that's not necessarily tied to files but any kind of resource?
> Or, would it make sense to define these events in a different class inside
> org.eclipse.rse.subsystems.files.core ?

I thought about these issues too - whether we should have separate events for files or whether these events are general enough to apply beyond just files.   I decided that upload and download, like rename and delete, are not necessarily file-system specific and therefore it would be okay to leave this in core.  Do you think the upload and download are too file-specific?   

Comment 7 David McKnight CLA 2007-10-24 11:47:22 EDT
(In reply to comment #5)
> Also - is there really a need to have the 1st parameter specified as "the
> remote object OR its absolute name"? 
> Such an alternative makes it harder for event receivers to handle the event. It
> would be easier if it were clearly defined what the argument should be.


There are a couple reasons for this.  First, all the other remote events allow for objects OR strings and, in doing so here, we're being consistent.  Secondly, there are cases where we don't have a remote object to use, such as when we're in this method:

	public void upload(String source, String srcEncoding, String remotePath, String rmtEncoding, IProgressMonitor monitor);


In that case, I fire the event using the strings, although I could have done a getRemoteFileObject() if we really wanted the objects.

Comment 8 Martin Oberhuber CLA 2007-10-24 11:50:39 EDT
(In reply to comment #6)
No. I agree that they can be generic. I was just confused by the patch's
comment -- the APIDoc in the patch seems to be generic already.

Would it make sense to let an event listener also know about the LOCAL source
or destination of the upload/download? Currently it can only find out the
remote source or destination. This might be an optional field for the case
where a Stream is opened or the download is done from memory / to memory.

Talking about Streams, I think the event should also be fired when a client has
opened an InputStream / OutputStream on the remote, at the time the Stream is
closed.

Ok for the absoluteName - or - Object question.
Comment 9 David McKnight CLA 2007-10-24 12:19:59 EDT
(In reply to comment #8)
> Would it make sense to let an event listener also know about the LOCAL source
> or destination of the upload/download? Currently it can only find out the
> remote source or destination. 

I wondered about that too.  Currently SystemRemoteChangeEvent has no field that would be appropriate for the local source.  Since this is generic, I'd want to avoid adding any field that is too specific to this operation.  How would you suggest that we store the local source? 

> Talking about Streams, I think the event should also be fired when a client has
> opened an InputStream / OutputStream on the remote, at the time the Stream is
> closed.

Oh right...that gets a little tricky since the streams are created at the service layer (which is a layer below the events).  I suppose we could construct a thread in the file service subsystem to wait for a stream to close.  Hopefully, polling wouldn't be required.  Any ideas on how to handle that?


Comment 10 Martin Oberhuber CLA 2007-10-24 13:20:52 EDT
When I'm not mistaken, there's a Streams API at the subsystem layer as well. That API could return decorated Streams, which fire the event when close() is called.

I'm not quite sure what to do with Streams that are ONLY created at the Service layer. Note that if a client chooses to use only the Service layer for upload or download, the events would also not be fired. 

So we either live with not having events in case a client just uses the Service layer, or we invent a separate notification mechanism at the Service layer that the services need to implement so we can have our events fired. In terms of backward compatibility, however, a service that's implemented against 2.0 would of course not make these notifications.
Comment 11 David McKnight CLA 2007-10-25 15:00:12 EDT
(In reply to comment #10)
> When I'm not mistaken, there's a Streams API at the subsystem layer as well.
> That API could return decorated Streams, which fire the event when close() is
> called.
> 


At the subsystem layer (i.e. FileServiceSubSystem) we have this:

	/**
	 * Defers to the file service.
	 * @see org.eclipse.rse.subsystems.files.core.subsystems.RemoteFileSubSystem#getInputStream(java.lang.String, java.lang.String, boolean, org.eclipse.core.runtime.IProgressMonitor)
	 */
	public InputStream getInputStream(String remoteParent, String remoteFile, boolean isBinary, IProgressMonitor monitor) throws SystemMessageException {
		return getFileService().getInputStream(remoteParent, remoteFile, isBinary, monitor);
	}

	/**
	 * Defers to the file service.
	 * @see org.eclipse.rse.subsystems.files.core.subsystems.RemoteFileSubSystem#getOutputStream(java.lang.String, java.lang.String, boolean, org.eclipse.core.runtime.IProgressMonitor)
	 */
	public OutputStream getOutputStream(String remoteParent, String remoteFile, boolean isBinary, IProgressMonitor monitor) throws SystemMessageException {
		return getFileService().getOutputStream(remoteParent, remoteFile, isBinary, monitor);
	}

Where else would there be streams created at the subsystem layer?

> I'm not quite sure what to do with Streams that are ONLY created at the Service
> layer. Note that if a client chooses to use only the Service layer for upload
> or download, the events would also not be fired. 
> 

If a stream is only created at the service layer, then I think it's okay to avoid the events since, at that layer we're doing the bare minimum.
Comment 12 Ankit Pasricha CLA 2007-10-25 16:08:59 EDT
Dave,

Will it be possible to include ISystemRegistry.isRegisteredSystemRemoteChangeListener similar to ISystemRegistry.isRegisteredSystemResourceChangeListener? I can use this to check when to add my listener.
Comment 13 David McKnight CLA 2007-10-25 17:05:49 EDT
(In reply to comment #12)
> Dave,
> Will it be possible to include
> ISystemRegistry.isRegisteredSystemRemoteChangeListener similar to
> ISystemRegistry.isRegisteredSystemResourceChangeListener? I can use this to
> check when to add my listener.

I've added ISystemRegistry.isRegisteredSystemRemoteChangeListener as requested.  I've also committed the changes for notifying after an upload.  The remaining item is to deal with the streams.
Comment 14 David McKnight CLA 2007-10-26 10:56:26 EDT
I've created two new classes:

FileSubSystemInputStream and 
FileSubSystemOutputStream 

These are used to wrapper the input and output streams that the service layer returns to the subsystem layer.  The upload and download events are fired on the close method.

These changes are in cvs now so I'll mark this as fixed.