Fix handling of temporary files

When we use self._set_filename in self._set_fileobj, the file already
exists, so we need to force "overwriting" it.

Also, move temporary file handling to a dedicated _set_tempfile method,
so we can officially claim not supporting _set_fileobj with QtWebEngine
instead of supporting it with a hack.
This commit is contained in:
Florian Bruhin 2016-11-08 07:21:21 +01:00
parent 53e360ec4b
commit 54db4255b1
3 changed files with 18 additions and 6 deletions

View File

@ -482,17 +482,21 @@ class AbstractDownloadItem(QObject):
def _set_fileobj(self, fileobj): def _set_fileobj(self, fileobj):
"""Set a file object to save the download to. """Set a file object to save the download to.
Note that some backends (QtWebEngine) will simply access the .name Not supported by QtWebEngine.
attribute and not actually use the file object directly.
""" """
raise NotImplementedError raise NotImplementedError
def _set_filename(self, filename): def _set_tempfile(self, fileobj):
"""Set a temporary file when opening the download."""
raise NotImplementedError
def _set_filename(self, filename, *, force_overwrite=False):
"""Set the filename to save the download to. """Set the filename to save the download to.
Args: Args:
filename: The full filename to save the download to. filename: The full filename to save the download to.
None: special value to stop the download. None: special value to stop the download.
force_overwrite: Force overwriting existing files.
""" """
global last_used_directory global last_used_directory
filename = os.path.expanduser(filename) filename = os.path.expanduser(filename)
@ -525,7 +529,9 @@ class AbstractDownloadItem(QObject):
last_used_directory = os.path.dirname(self._filename) last_used_directory = os.path.dirname(self._filename)
log.downloads.debug("Setting filename to {}".format(filename)) log.downloads.debug("Setting filename to {}".format(filename))
if os.path.isfile(self._filename): if force_overwrite:
self._after_set_filename()
elif os.path.isfile(self._filename):
# The file already exists, so ask the user if it should be # The file already exists, so ask the user if it should be
# overwritten. # overwritten.
txt = "<b>{}</b> already exists. Overwrite?".format( txt = "<b>{}</b> already exists. Overwrite?".format(
@ -573,7 +579,7 @@ class AbstractDownloadItem(QObject):
return return
self.finished.connect( self.finished.connect(
functools.partial(self._open_if_successful, target.cmdline)) functools.partial(self._open_if_successful, target.cmdline))
self._set_fileobj(fobj) self._set_tempfile(fobj)
else: # pragma: no cover else: # pragma: no cover
raise ValueError("Unsupported download target: {}".format(target)) raise ValueError("Unsupported download target: {}".format(target))

View File

@ -214,6 +214,9 @@ class DownloadItem(downloads.AbstractDownloadItem):
except OSError as e: except OSError as e:
self._die(e.strerror) self._die(e.strerror)
def _set_tempfile(self, fileobj):
self._set_fileobj(fileobj)
def _finish_download(self): def _finish_download(self):
"""Write buffered data to disk and finish the QNetworkReply.""" """Write buffered data to disk and finish the QNetworkReply."""
log.downloads.debug("Finishing download...") log.downloads.debug("Finishing download...")

View File

@ -88,7 +88,10 @@ class DownloadItem(downloads.AbstractDownloadItem):
return self._filename return self._filename
def _set_fileobj(self, fileobj): def _set_fileobj(self, fileobj):
self._set_filename(fileobj.name) raise downloads.UnsupportedOperationError
def _set_tempfile(self, fileobj):
self._set_filename(fileobj.name, force_overwrite=True)
def _ensure_can_set_filename(self, filename): def _ensure_can_set_filename(self, filename):
state = self._qt_item.state() state = self._qt_item.state()