Bug 41332

Summary:	[patch] Support resuming file transfer
Product:	Telepathy	Reporter:	Daniele E. Domenichelli <daniele.domenichelli>
Component:	tp-qt	Assignee:	Daniele E. Domenichelli <daniele.domenichelli>
Status:	RESOLVED INVALID	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	CC:	daniele.domenichelli, ollisal
Version:	git master
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:

Description Daniele E. Domenichelli 2011-09-29 05:38:15 UTC

Currently if you set the offset parameter in IncomingFileTransferChannel::acceptFile(qulonglong offset, QIODevice *output), output is opened in write only mode, therefore the file is overwritten.

The patch here[1] opens the file in append mode if offset is set, and uses seek to move to the offset.


[1]https://gitorious.org/drdanz-telepathy-kde/telepathy-qt4/commits/resume_filetransfer

Comment 1 Olli Salli 2011-10-04 08:15:28 UTC

A few problems with the patch:

1) The output stream is not necessarily sequential. It might, for example, be a socket for relaying the data to some other process, or some custom QIODevice for e.g. storing the data to a database or alike

2) It's not documented that a seek is now made by acceptFile()

3) It changes existing, public, exported library API to have additional side effects for objects passed to it from the user. This is an API/ABI break and hence unacceptable.

Solving 1 would be possible by treating sequential (e.g. QFile) and other QIODevices differently via the isSequential accessor. This would, however, be confusing in my opinion.

Solving 2 would of course be easy.

3 however can only be solved by retaining the behavior of the current function and adding a new one (an overload if params differ, or otherwise one with a new name) for easy resumes. You could add an overload:

acceptFile(QFile *file)

which would be documented as the one to use when you have an existing (but partial) file the user wants to complete by this file transfer, that you can directly access. It would take the size of the file, open if not open already (in append mode to preserve the already received part) and pass the file's size as the requested offset to acceptFile(qulonglong, QIODevice *).

Of course, one could do that sequence in their own code, but clearly it's not intuitive that this is required. Hence, I'd very warmly welcome a patch adding that "resume to complete this file" overload.

Also, maybe instead of being an overload it'd be better to call that new function resumeToFile() or something like that. Ideas welcome.

I'm assigning to you, but if you don't feel like cooking this patch, feel free to reset assignee to default, and turn this into a feature request (prio: enhancement) to have a nice convenience overload for resuming incoming FTs.

Comment 2 Daniele E. Domenichelli 2011-10-04 09:45:14 UTC

Having a better look at the code I realized that this patch is completely unnecessary, what is probably missing is a short note to the documentation that documents that in order to append to a QIODevice, it should be opened and seeked to the right position, otherwise the "offset" parameter might be misleading.

I will do it later!

Comment 3 Olli Salli 2011-12-29 07:59:22 UTC

Here goes a belated "very well".

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.