Bug 32434 - FileTransfer: add a way to pass the outgoing file to the Handler
Summary: FileTransfer: add a way to pass the outgoing file to the Handler
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-spec (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/ca...
Whiteboard: review+ with wording tweaks
Keywords: patch
: 27423 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-12-16 03:40 UTC by Guillaume Desmottes
Modified: 2011-01-28 06:02 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Guillaume Desmottes 2010-12-16 03:40:18 UTC
Currently, the process requesting an outgoing FT channel has to be the one handling it as it has to stream the file to the send. But it would be handy to be able to just request the channel and let an handler doing the actual sending for us. For example, empathy-chat is now able to request initiate FT but it would be nice to let empathy doing the sending.

This could be done by defining a request Hints (org.freedesktop.Telepathy.
Channel.Type.FileTransfer.LocalFileURI?) containing the URI of the file to send.

This would also have the nice side effect to let Observers known about the
file being sent (which is something Zeitgeist seem to want).

But they'd also like to be able to get the URI/path where *incoming* files are
saved.
One way to do that  could be to add a LocalFileURI rw D-Bus property on the FT
channel. We could then maybe reuse it for outgoing channels as well.
Comment 1 Guillaume Desmottes 2011-01-25 00:22:43 UTC
(In reply to comment #0)
> But they'd also like to be able to get the URI/path where *incoming* files are
> saved.
> One way to do that  could be to add a LocalFileURI rw D-Bus property on the FT
> channel. We could then maybe reuse it for outgoing channels as well.

We certainly don't want adding a change signal for this, so we should define when this property is defined. Forcing the Handler to set the property *before* calling AcceptFile() could be an option, so Observer would be sure that if the property has not been set when the FT becomes Open, then it won't be set at all.
Comment 2 Guillaume Desmottes 2011-01-25 01:07:31 UTC
For the record, this blocks https://bugzilla.gnome.org/show_bug.cgi?id=640513
Comment 3 Will Thompson 2011-01-25 02:10:15 UTC
So you're proposing:

prop LocalFilePath (rw):
    tp:requestable
    tp:immutable='sometimes'

    For outgoing file transfers, this requestable property allows the channel requester to inform observers (and the handler, if it is not the requester itself) of the full path to the local file being transferred. Note that the connection manager SHOULD NOT read this local file directly; the handler streams the file into the CM through the socket negotiated using ProvideFile.

    On outgoing file transfers, this property MUST NOT change after the channel is requested.

      tp:rationale
        Historically, the handler for outgoing file transfers had to be the same process as the requester: there was no way for the requester to tell the handler which file should be sent. This made it impossible to write tp_send_file (TpContact *foo, const gchar *file_path) without reimplementing the user interface for file transfers.

    For incoming file transfers, this property MAY be set by the channel handler before calling AcceptFile to inform observers where the incoming file will be saved locally. Setting this property once AcceptFile has been called MUST fail. Blah blah observers should check this property's value when 
FileTransferStateChanged(Open) is emitted.

There's a race:

• Handler sets LocalFileURI
• Handler calls AcceptFile(); CM returns really quickly, and emits FileTransferStateChanged(Open)
• Observer calls Get(LocalFileURI), but due to a quirk of the scheduler the message doesn't get sent for a while.
• Handler gets the entire file.
• CM emits FileTransferStateChanged(Completed)
• Handler calls Close()
• The observer's method call finally gets delivered, and fails.

So I think we need change notification. LocalFileURISet(s: Local_File_URI) doesn't seem so bad.
Comment 4 Will Thompson 2011-01-25 02:13:20 UTC
(In reply to comment #3)
> So I think we need change notification. LocalFileURISet(s: Local_File_URI)
> doesn't seem so bad.

Of course in Telepathy 1.0 we can use org.freedesktop.DBus.Properties.PropertiesChanged and there won't be a proliferation of signals.

Also, if a CM supported a hypothetical Address_Type_Local_File, the handler could just pass that and the file path as the Access_Control_Param to Accept/Provide… ;-)
Comment 5 Guillaume Desmottes 2011-01-25 03:39:33 UTC
*** Bug 27423 has been marked as a duplicate of this bug. ***
Comment 6 Guillaume Desmottes 2011-01-25 03:47:15 UTC
(In reply to comment #3)
> So you're proposing:
> 
> prop LocalFilePath (rw):
>     tp:requestable
>     tp:immutable='sometimes'

Maybe FileURI so we can use gio magic to send/receive files from/to remote
locations?
Note that would make things more complex if we use this property as a file
transfer method with the CM (a CM may not support, say, ftp://);

> There's a race:
> 
> • Handler sets LocalFileURI
> • Handler calls AcceptFile(); CM returns really quickly, and emits
> FileTransferStateChanged(Open)
> • Observer calls Get(LocalFileURI), but due to a quirk of the scheduler the
> message doesn't get sent for a while.
> • Handler gets the entire file.
> • CM emits FileTransferStateChanged(Completed)
> • Handler calls Close()
> • The observer's method call finally gets delivered, and fails.
> 
> So I think we need change notification. LocalFileURISet(s: Local_File_URI)
> doesn't seem so bad.

That's a bit of a corner case but yeah, ok. Adding this signal is trivial any
way.

(In reply to comment #4)
> (In reply to comment #3)
> Also, if a CM supported a hypothetical Address_Type_Local_File, the handler
> could just pass that and the file path as the Access_Control_Param to
> Accept/Provide… ;-)

That's bug #18527, I added a comment.
Comment 7 Will Thompson 2011-01-25 06:22:48 UTC
(In reply to comment #6)
> (In reply to comment #3)
> > So you're proposing:
> > 
> > prop LocalFilePath (rw):
> >     tp:requestable
> >     tp:immutable='sometimes'
> 
> Maybe FileURI so we can use gio magic to send/receive files from/to remote
> locations?
> Note that would make things more complex if we use this property as a file
> transfer method with the CM (a CM may not support, say, ftp://);

We can always say “don't do that”. :)
Comment 8 Guillaume Desmottes 2011-01-25 07:00:39 UTC
Here we go:
http://git.collabora.co.uk/?p=user/cassidy/telepathy-spec;a=shortlog;h=refs/heads/ft-uri-32434

I named the signal FileURIDefined as we already have InitialOffsetDefined.
Comment 9 Guillaume Desmottes 2011-01-25 08:17:01 UTC
Maybe just "URI" would be enough actally? We already have Size or Date, not FileSize or FileDate.
Comment 10 Guillaume Desmottes 2011-01-26 02:24:11 UTC
I implemented this API in Gabble: bug #33471
Comment 11 Will Thompson 2011-01-26 03:02:28 UTC
+        property has been set. This signal MAY only be fired on incoming file

That should be “This signal MUST only be emitted on incoming file transfers, and only if the handler sets the <tp:member-ref>FileURI</tp:member-ref> property before accepting the file.”

+        <tp:member-ref>FileURIDefined</tp:member-ref> is fired.</p>

emitted plz.
Comment 12 Will Thompson 2011-01-26 03:04:36 UTC
(In reply to comment #9)
> Maybe just "URI" would be enough actally? We already have Size or Date, not
> FileSize or FileDate.

URI, maybe.

You should mention that in general this will be a file: URI, but it MAY be something else if we're streaming a web page to a contact or something. I can't see why it would ever not be a file: URI for incoming files: gvfs has mount points under .gvfs (IIRC) for stuff you can write to, and knows how to translate to and fro.
Comment 13 Guillaume Desmottes 2011-01-26 04:09:14 UTC
(In reply to comment #11)
> +        property has been set. This signal MAY only be fired on incoming file
> 
> That should be “This signal MUST only be emitted on incoming file transfers,
> and only if the handler sets the <tp:member-ref>FileURI</tp:member-ref>
> property before accepting the file.”

changed.

> +        <tp:member-ref>FileURIDefined</tp:member-ref> is fired.</p>
> 
> emitted plz.

changed.

(In reply to comment #12)
> (In reply to comment #9)
> > Maybe just "URI" would be enough actally? We already have Size or Date, not
> > FileSize or FileDate.
> 
> URI, maybe.

I think that's better. changed.

> You should mention that in general this will be a file: URI, but it MAY be
> something else if we're streaming a web page to a contact or something. I can't
> see why it would ever not be a file: URI for incoming files: gvfs has mount
> points under .gvfs (IIRC) for stuff you can write to, and knows how to
> translate to and fro.

I added two small paragraphs explaining that.

HTML:
http://people.freedesktop.org/~gdesmott/telepathy-spec-ft_uri_32434/spec/Channel_Type_File_Transfer.html#Property:URI
http://people.freedesktop.org/~gdesmott/telepathy-spec-ft_uri_32434/spec/Channel_Type_File_Transfer.html#Signal:URIDefined
Comment 14 Will Thompson 2011-01-28 05:48:31 UTC
+        <p>For outgoing file transfers, most of the time this URI will be a
+        file but it MAY be something else; if we're streaming
+        a web page to a contact for example.</p>

+        <p>For incoming file transfers, this URI MUST be a file.</p>

I'd replace both of these with a single paragraph:

    If set, this URL SHOULD generally point to a file on the local system, as defined by <a href='http://www.apps.ietf.org/rfc/rfc1738.html#sec-3.10'>RFC 1738 §3.10</a>; that is, it should be of the form <tt>file:///path/to/file</tt> or <tt>file://localhost/path/to/file</tt>. For outgoing files, this URL MAY use a different scheme, such as <tt>http:</tt>, if a remote resource is being transferred to a contact.

Pedantically, this property should be URL, not URI: we want this to be a locator, not an identifier. See <http://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Relationship_to_URL_and_URN>: file:, http: and friends are the kinds of URI we want, not urn:xmpp:jingle:1, no matter how much I love that protocol.

But besides that I think this is good to go. I don't mind *that* much about URL vs. URI.
Comment 15 Guillaume Desmottes 2011-01-28 06:02:40 UTC
Merged to master; will be in 0.21.9


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.