[Telepathy] File transfer spec

[Simon McVittie]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wed, 06 Jun 2007 at 21:32:21 +0200, Marco Barisione wrote:
> I'm working on telepathy-salut to add file transfer capabilities using
> the proposed file transfer spec and I have some notes about it.
> I read all the messages I found in this list about this spec but maybe I
> missed something so I may write something stupid :)

For reference, the current spec draft is here:

http://projects.collabora.co.uk/~smcv/file-transfer-spec-20070606.html

(generated from the non-spec extensions in the Salut source tree, so please
ignore the OLPC bits for the purposes of this discussion :-)

In addition to Marco's criticisms:

* We should define some well-known keys for the optional info dict. In
  particular, UInt64: size (omitted if not known).

* IIRC, we wanted to be able to support "groups" of files as well as
  single files, so we could support Google Talk in future? Or do we not
  care about maintaining the difference between offering a bunch of
  files together vs offering them one after the other?

> First of all there isn't a way to get the initiator, direction, state,
> etc. of a single file transfer, so if you want the state you have to do:
> 
> for ft in channel.ListFileTransfers():
>     if ft[0] == id:
>         state = ft[3]
>         break
> 
> instead of something as simple as:
> 
> state = channel.GetFileTransfer(id)[3]
> 
> So I propose to add a GetFileTransfer() method (maybe you can find a
> better name).

You're meant to watch all three signals and maintain a client-side copy
of the table of file transfers, probably as a hash table. This avoids
making a D-Bus round-trip every time you want to know something about a
file transfer. This "get an initial snapshot and update it on signals"
model is common throughout Telepathy.

Example (assume this is all in a Python class):

def __init__(...):
    ...
    self._file_transfers = {}
    # it's important to connect the signals before making the method
    # call, for race-free operation
    self._ft_iface.connect_to_signal('NewFileTransfer',
                                     self._on_new_file_transfer)
    self._ft_iface.connect_to_signal('FileTransferStateChanged',
                                     self._on_ft_state_changed)
    self._ft_iface.connect_to_signal('FileTransferClosed',
                                     self._on_ft_closed)
    self._ft_iface.ListFileTransfers(reply_handler=_listed_file_transfers,
                                     error_handler=...)

def _listed_file_transfers(fts):
    self._file_transfers = {}
    for (ft_id, initiator, direction, state, filename, props) in fts:
        # FIXME: why are these not in the same order as for
        # NewFileTransfer?!
        self._on_new_file_transfer(ft_id, initiator, direction,
                                   filename, props, state)


def _on_new_file_transfer(ft_id, initiator, direction, filename, props,
                          state):
    self._file_transfers[ft_id] = [initiator, direction, filename,
                                   props, state]

def _on_ft_state_changed(ft_id, state):
    self._file_transfers[ft_id][-1] = state

def _on_ft_closed(ft_id):
    del self._file_transfers[ft_id]

> There is no way to inform the user of the failure of a transfer, i.e.
> the file transfer interface is lacking something like
> Channel.Type.Text.SendError. I would like to add a FileTransferError
> signal:
> 
> FileTransferError (u: error, u: id, u: initiator, u: direction,
>                    u: state, s: filename, a{sv}: information)
> 
> or just:
> 
> FileTransferError (u: id, u: error)
> 
> The ChannelFileTransferError enumeration could have some generic errors
> similar to the ones in the ChannelTextSendError enumeration and some
> specific errors such as ChannelFileTransferErrorRemoteClosed when the
> remote user stops a transfer.

A reasonable criticism. Or we could change the signature of
CloseFileTransfer and FileTransferClosed, like so:

CloseFileTransfer(u: id, u: reason)
FileTransferClosed(u: id, u: reason)

with one of the reasons being FILE_TRANSFER_CLOSE_REASON_SUCCESS.

> Can the state of a file transfer change from open to pending? I think
> that a (local or remote) pending transfer can only become open, so why
> FileTransferStateChanged and not FileTransferOpened or
> FileTransferStarted?

You could be right there, yes.

> The order of the arguments to NewFileTransfer and the values returned by
> ListFileTransfers() are different, it's not a big problem but I would
> like to move state before filename in NewFileTransfer.

Or vice versa. I agree that the inconsistency is silly.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: OpenPGP key: http://www.pseudorandom.co.uk/2003/contact/ or pgp.net

iD8DBQFGZ+nYWSc8zVUw7HYRAqBoAKDVw7n+gr1MUhHmsiunYSLXUD3QHwCfYxa9
p08Z5DW/AJmJh9e7Ko0a7Oo=
=X29g
-----END PGP SIGNATURE-----