jenkins-bot has submitted this change and it was merged.
Change subject: [FEAT] upload: Get stash info before continuing
......................................................................
[FEAT] upload: Get stash info before continuing
If the response does contain warnings it won't contain the offset for the
continuation (T73737). So it requests the stash info for the given file key to
determine the offset. This also allows to properly reuse the file key as it
will create a new file key instance when the offset is 0. It also prevents
that the API shows an error if the offset is neither 0 nor the actual size
uploaded.
And if the offset is given it can verify that it matches the stashed data. It
also supports comparing the SHA1 of the already uploaded part to minimize the
chance that a different file is uploaded for continuation. It now also verifies
that the offset is not larger than the file size. In case the file is large and
doing an SHA1 would be time consuming it's possible to not verify them.
The offset is not present if the warning occurred in the first chunk which
would have caused that the first chunk is discarded when continuing to upload.
To do that the file key must be defined before it issues the warnings.
Also when uploading in the unchunked mode and a file key is provided it did
upload the complete file even though the offset was set to `False` to indicate
the file is completely uploaded.
One test is an expected failure due to T112416.
Change-Id: I610a0744d74dc5a2523d96eabde818f76dd6c5e8
---
M pywikibot/site.py
M tests/upload_tests.py
2 files changed, 221 insertions(+), 19 deletions(-)
Approvals:
John Vandenberg: Looks good to me, approved
jenkins-bot: Verified
diff --git a/pywikibot/site.py b/pywikibot/site.py
index 8cb66a8..9e7dde9 100644
--- a/pywikibot/site.py
+++ b/pywikibot/site.py
@@ -16,6 +16,7 @@
#
import datetime
+import hashlib
import itertools
import os
import re
@@ -5177,10 +5178,20 @@
raise
return self._uploaddisabled
+ def stash_info(self, file_key, props=False):
+ """Get the stash info for a given file key."""
+ if not props:
+ props = False
+ req = self._simple_request(
+ action='query', prop='stashimageinfo', siifilekey=file_key,
+ siiprop=props)
+ return req.submit()['query']['stashimageinfo'][0]
+
@deprecate_arg('imagepage', 'filepage')
def upload(self, filepage, source_filename=None, source_url=None,
comment=None, text=None, watch=False, ignore_warnings=False,
- chunk_size=0, _file_key=None, _offset=0, report_success=None):
+ chunk_size=0, _file_key=None, _offset=0, _verify_stash=None,
+ report_success=None):
"""Upload a file to the wiki.
Either source_filename or source_url, but not both, must be provided.
@@ -5215,8 +5226,15 @@
@type _file_key: str or None
@param _offset: When file_key is not None this can be an integer to
continue a previously canceled chunked upload. If False it treats
- that as a finished upload. By default starts at 0.
+ that as a finished upload. If True it requests the stash info from
+ the server to determine the offset. By default starts at 0.
@type _offset: int or bool
+ @param _verify_stash: Requests the SHA1 and file size uploaded and
+ compares it to the local file. Also verifies that _offset is
+ matching the file size if the _offset is an int. If _offset is
False
+ if verifies that the file size match with the local file. If None
+ it'll verifies the stash when a file key and offset is given.
+ @type _verify_stash: bool or None
@param report_success: If the upload was successful it'll print a
success message and if ignore_warnings is set to False it'll
raise an UploadWarning if a warning occurred. If it's None
(default)
@@ -5231,7 +5249,7 @@
api.UploadWarning(
warning,
upload_warnings.get(warning, '%(msg)s') % {'msg': data},
- _file_key, response.get('offset', 0))
+ _file_key, response['offset'])
for warning, data in response['warnings'].items()]
upload_warnings = {
@@ -5287,7 +5305,76 @@
token = self.tokens['edit']
result = None
file_page_title = filepage.title(withNamespace=False)
- if _file_key and _offset is False:
+ file_size = None
+ offset = _offset
+ # make sure file actually exists
+ if source_filename:
+ if os.path.isfile(source_filename):
+ file_size = os.path.getsize(source_filename)
+ elif offset is not False:
+ raise ValueError("File '%s' does not exist."
+ % source_filename)
+
+ if source_filename and _file_key:
+ assert offset is False or file_size is not None
+ if _verify_stash is None:
+ _verify_stash = True
+ if (offset is not False and offset is not True and
+ offset > file_size):
+ raise ValueError(
+ 'For the file key "{0}" the offset was set to {1} '
+ 'while the file is only {2} bytes large.'.format(
+ _file_key, offset, file_size))
+
+ if _verify_stash or offset is True:
+ if not _file_key:
+ raise ValueError('Without a file key it cannot request the '
+ 'stash information')
+ if not source_filename:
+ raise ValueError('Can request stash information only when '
+ 'using a file name.')
+ props = ['size']
+ if _verify_stash:
+ props += ['sha1']
+ stash_info = self.stash_info(_file_key, props)
+ if offset is True:
+ offset = stash_info['size']
+ elif offset is False:
+ if file_size != stash_info['size']:
+ raise ValueError(
+ 'For the file key "{0}" the server reported a size {1}
'
+ 'while the file size is {2}'.format(
+ _file_key, stash_info['size'], file_size))
+ elif offset is not False and offset != stash_info['size']:
+ raise ValueError(
+ 'For the file key "{0}" the server reported a size {1} '
+ 'while the offset was {2}'.format(
+ _file_key, stash_info['size'], offset))
+
+ if _verify_stash:
+ # The SHA1 was also requested so calculate and compare it
+ assert 'sha1' in stash_info, \
+ 'sha1 not in stash info: {0}'.format(stash_info)
+ sha1 = hashlib.sha1()
+ bytes_to_read = offset
+ with open(source_filename, 'rb') as f:
+ while bytes_to_read > 0:
+ read_bytes = f.read(min(bytes_to_read, 1 << 20))
+ assert read_bytes # make sure we actually read bytes
+ bytes_to_read -= len(read_bytes)
+ sha1.update(read_bytes)
+ sha1 = sha1.hexdigest()
+ if sha1 != stash_info['sha1']:
+ raise ValueError(
+ 'The SHA1 of {0} bytes of the stashed "{1}" is {2} '
+ 'while the local file is {3}'.format(
+ offset, _file_key, stash_info['sha1'], sha1))
+
+ assert offset is not True
+ if _file_key and file_size is None:
+ assert offset is False
+
+ if _file_key and offset is False or offset == file_size:
pywikibot.log('Reused already upload file using '
'filekey "{0}"'.format(_file_key))
# TODO: Use sessionkey instead of filekey if necessary
@@ -5299,10 +5386,6 @@
# TODO: Dummy value to allow also Unicode names, see bug 73661
mime_filename = 'FAKE-NAME'
# upload local file
- # make sure file actually exists
- if not os.path.isfile(source_filename):
- raise ValueError("File '%s' does not exist."
- % source_filename)
throttle = True
filesize = os.path.getsize(source_filename)
chunked_upload = (chunk_size > 0 and chunk_size < filesize and
@@ -5313,7 +5396,6 @@
'action': 'upload', 'token': token, 'text': text,
'filename': file_page_title, 'comment': comment})
if chunked_upload:
- offset = _offset
if offset > 0:
pywikibot.log('Continuing upload from byte '
'{0}'.format(offset))
@@ -5343,13 +5425,16 @@
if error.code == u'uploaddisabled':
self._uploaddisabled = True
raise error
+ _file_key = data['filekey']
if 'warnings' in data and not ignore_all_warnings:
if callable(ignore_warnings):
+ if 'offset' not in data:
+ data['offset'] = True
if ignore_warnings(create_warnings_list(data)):
# Future warnings of this run can be
ignored
ignore_warnings = True
ignore_all_warnings = True
- offset = result.get('offset', 0)
+ offset = data['offset']
continue
else:
return False
@@ -5357,7 +5442,6 @@
if 'offset' not in result:
result['offset'] = 0
break
- _file_key = data['filekey']
throttle = False
if 'offset' in data:
new_offset = int(data['offset'])
@@ -5376,13 +5460,16 @@
final_request['filekey'] = _file_key
break
else: # not chunked upload
- file_contents = f.read()
- filetype = (mimetypes.guess_type(source_filename)[0] or
- 'application/octet-stream')
- final_request.mime_params = {
- 'file': (file_contents, filetype.split('/'),
- {'filename': mime_filename})
- }
+ if _file_key:
+ final_request['filekey'] = _file_key
+ else:
+ file_contents = f.read()
+ filetype = (mimetypes.guess_type(source_filename)[0] or
+ 'application/octet-stream')
+ final_request.mime_params = {
+ 'file': (file_contents, filetype.split('/'),
+ {'filename': mime_filename})
+ }
else:
# upload by URL
if "upload_by_url" not in self.userinfo["rights"]:
@@ -5417,11 +5504,13 @@
_file_key = None
pywikibot.warning('No filekey defined.')
if not report_success:
+ if 'offset' not in result:
+ result['offset'] = True
if ignore_warnings(create_warnings_list(result)):
return self.upload(
filepage, source_filename, source_url, comment, text,
watch, True, chunk_size, _file_key,
- result.get('offset', False), report_success=False)
+ result['offset'], report_success=False)
else:
return False
warn('When ignore_warnings=False in APISite.upload will change '
diff --git a/tests/upload_tests.py b/tests/upload_tests.py
index 4d1b1ea..aed4a18 100644
--- a/tests/upload_tests.py
+++ b/tests/upload_tests.py
@@ -17,6 +17,8 @@
import pywikibot
+from pywikibot.data.api import APIError
+
from tests import _images_dir
from tests.aspects import unittest, TestCase
@@ -29,6 +31,9 @@
family = 'wikipedia'
code = 'test'
+
+ sounds_png = os.path.join(_images_dir, 'MP_sounds.png')
+ arrow_png = os.path.join(_images_dir, '1rightarrow.png')
def test_png(self):
"""Test uploading a png using Site.upload."""
@@ -46,6 +51,114 @@
comment='pywikibot test',
ignore_warnings=True, chunk_size=1024)
+ def _init_upload(self, chunk_size):
+ """Do an initial upload causing an abort because of warnings."""
+ def warn_callback(warnings):
+ """A simple callback not automatically finishing the upload."""
+ self.assertCountEqual([w.code for w in warnings], expected_warns)
+ # by now we know there are only two but just make sure
+ assert len(warnings) == len(expected_warns)
+ assert len(expected_warns) in [1, 2]
+ if len(expected_warns) == 2:
+ self.assertEqual(warnings[0].file_key, warnings[1].file_key)
+ self.assertEqual(warnings[0].offset, warnings[1].offset)
+ self._file_key = warnings[0].file_key
+ self._offset = warnings[0].offset
+
+ if chunk_size:
+ expected_warns = ['exists']
+ else:
+ expected_warns = ['duplicate', 'exists']
+
+ # First upload the warning with warnings enabled
+ page = pywikibot.FilePage(self.site, 'MP_sounds-pwb.png')
+ self.assertFalse(hasattr(self, '_file_key'))
+ self.site.upload(page, source_filename=self.sounds_png,
+ comment='pywikibot test', chunk_size=chunk_size,
+ ignore_warnings=warn_callback)
+
+ # Check that the warning happened and it's cached
+ self.assertTrue(hasattr(self, '_file_key'))
+ self.assertIs(self._offset, True)
+ self.assertRegex(self._file_key, r'[0-9a-z]+.[0-9a-z]+.\d+.png')
+ self._verify_stash()
+
+ def _verify_stash(self):
+ info = self.site.stash_info(self._file_key, ['size', 'sha1'])
+ if info['size'] == 1024:
+ self.assertEqual('3503db342c8dfb0a38db0682b7370ddd271fa163',
+ info['sha1'])
+ else:
+ self.assertEqual('0408a0f6a5e057e701f3aed96b0d1fb913c3d9d0',
+ info['sha1'])
+
+ def _finish_upload(self, chunk_size, file_name):
+ """Finish the upload."""
+ # Finish/continue upload with the given file key
+ page = pywikibot.FilePage(self.site, 'MP_sounds-pwb.png')
+ self.site.upload(page, source_filename=file_name,
+ comment='pywikibot test', chunk_size=chunk_size,
+ ignore_warnings=True, report_success=False,
+ _file_key=self._file_key, _offset=self._offset)
+
+ def _test_continue_filekey(self, chunk_size):
+ """Test uploading a chunk first and finish in a separate upload."""
+ self._init_upload(chunk_size)
+ self._finish_upload(chunk_size, self.sounds_png)
+
+ # Check if it's still cached
+ with self.assertRaises(APIError) as cm:
+ self.site.stash_info(self._file_key)
+ self.assertEqual(cm.exception.code, 'siiinvalidsessiondata')
+ self.assertTrue(cm.exception.info.startswith('File not found'),
+ 'info ({0}) did not start with '
+ '"File not found"'.format(cm.exception.info))
+
+ def test_continue_filekey_once(self):
+ """Test continuing to upload a file without using chunked mode."""
+ self._test_continue_filekey(0)
+
+ @unittest.expectedFailure # see T112416
+ def test_continue_filekey_chunked(self):
+ """Test continuing to upload a file with using chunked mode."""
+ self._test_continue_filekey(1024)
+
+ def test_sha1_missmatch(self):
+ """Test trying to continue with a different file."""
+ self._init_upload(1024)
+ with self.assertRaises(ValueError) as cm:
+ self._finish_upload(1024, self.arrow_png)
+ self.assertEqual(
+ str(cm.exception),
+ 'The SHA1 of 1024 bytes of the stashed "{0}" is '
+ '3503db342c8dfb0a38db0682b7370ddd271fa163 while the local file is '
+ '3dd334f11aa1e780d636416dc0649b96b67588b6'.format(self._file_key))
+ self._verify_stash()
+
+ def test_offset_missmatch(self):
+ """Test trying to continue with a different offset."""
+ self._init_upload(1024)
+ self._offset = 0
+ with self.assertRaises(ValueError) as cm:
+ self._finish_upload(1024, self.sounds_png)
+ self.assertEqual(
+ str(cm.exception),
+ 'For the file key "{0}" the server reported a size 1024 while the '
+ 'offset was 0'.format(self._file_key))
+ self._verify_stash()
+
+ def test_offset_oversize(self):
+ """Test trying to continue with an offset which is to large."""
+ self._init_upload(1024)
+ self._offset = 2000
+ with self.assertRaises(ValueError) as cm:
+ self._finish_upload(1024, self.sounds_png)
+ self.assertEqual(
+ str(cm.exception),
+ 'For the file key "{0}" the offset was set to 2000 while the file '
+ 'is only 1276 bytes large.'.format(self._file_key))
+ self._verify_stash()
+
if __name__ == '__main__':
try:
--
To view, visit https://gerrit.wikimedia.org/r/234851
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I610a0744d74dc5a2523d96eabde818f76dd6c5e8
Gerrit-PatchSet: 7
Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Owner: XZise <[email protected]>
Gerrit-Reviewer: John Vandenberg <[email protected]>
Gerrit-Reviewer: Ladsgroup <[email protected]>
Gerrit-Reviewer: Mpaa <[email protected]>
Gerrit-Reviewer: XZise <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
