From a51a9bbaafb06c0a62a705c19c11d01e2ea7e6d0 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Tue, 12 Feb 2019 16:08:21 +0100 Subject: [PATCH] Avoid comparing bytes and str in Python 3 --- src/olympia/addons/tests/test_cron.py | 6 ++---- src/olympia/files/models.py | 17 +++++++---------- src/olympia/files/tests/test_models.py | 25 +++++++++++++++++++++---- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/olympia/addons/tests/test_cron.py b/src/olympia/addons/tests/test_cron.py index a8aa66f76310..6ef836e3b526 100644 --- a/src/olympia/addons/tests/test_cron.py +++ b/src/olympia/addons/tests/test_cron.py @@ -160,14 +160,12 @@ def test_move_disabled_addon_ioerror(self, mv_mock, storage_exists): # Check that we called `move_stored_file` for f2 properly f2 = self.f2 - mv_mock.assert_called_with( - force_bytes(f2.file_path), force_bytes(f2.guarded_file_path)) + mv_mock.assert_called_with(f2.file_path, f2.guarded_file_path) # Check that we called `move_stored_file` for f1 properly f1 = self.f1 mv_mock.call_args = mv_mock.call_args_list[0] - mv_mock.assert_called_with( - force_bytes(f1.file_path), force_bytes(f1.guarded_file_path)) + mv_mock.assert_called_with(f1.file_path, f1.guarded_file_path) # Make sure we called `mv` twice despite an `IOError` for the first # file diff --git a/src/olympia/files/models.py b/src/olympia/files/models.py index e63aa01436e5..e68e8d1115b2 100644 --- a/src/olympia/files/models.py +++ b/src/olympia/files/models.py @@ -154,13 +154,13 @@ def from_upload(cls, upload, version, platform, parsed_data=None): assert parsed_data is not None file_ = cls(version=version, platform=platform) - upload.path = force_bytes(nfd_str(upload.path)) - ext = force_text(os.path.splitext(upload.path)[1]) + upload_path = force_text(nfd_str(upload.path)) + ext = force_text(os.path.splitext(upload_path)[1]) if ext == '.jar': ext = '.xpi' file_.filename = file_.generate_filename(extension=ext or '.xpi') # Size in bytes. - file_.size = storage.size(upload.path) + file_.size = storage.size(upload_path) file_.is_restart_required = parsed_data.get( 'is_restart_required', False) file_.strict_compatibility = parsed_data.get( @@ -171,7 +171,7 @@ def from_upload(cls, upload, version, platform, parsed_data=None): file_.is_mozilla_signed_extension = parsed_data.get( 'is_mozilla_signed_extension', False) - file_.hash = file_.generate_hash(upload.path) + file_.hash = file_.generate_hash(upload_path) file_.original_hash = file_.hash if upload.validation: @@ -192,7 +192,7 @@ def from_upload(cls, upload, version, platform, parsed_data=None): log.debug('New file: %r from %r' % (file_, upload)) # Move the uploaded file from the temp location. - copy_stored_file(upload.path, file_.current_file_path) + copy_stored_file(upload_path, file_.current_file_path) if upload.validation: FileValidation.from_json(file_, validation) @@ -290,17 +290,14 @@ def extension(self): def move_file(self, source, destination, log_message): """Move a file from `source` to `destination`.""" - # Make sure we are passing bytes to Python's io system. - source, destination = force_bytes(source), force_bytes(destination) - + log_message = force_text(log_message) try: if storage.exists(source): log.info(log_message.format( source=source, destination=destination)) move_stored_file(source, destination) except (UnicodeEncodeError, IOError): - msg = 'Move Failure: {} {}'.format( - force_bytes(source), force_bytes(destination)) + msg = u'Move Failure: {} {}'.format(source, destination) log.exception(msg) def hide_disabled_file(self): diff --git a/src/olympia/files/tests/test_models.py b/src/olympia/files/tests/test_models.py index 4e7cb2ba2496..c699113e9a71 100644 --- a/src/olympia/files/tests/test_models.py +++ b/src/olympia/files/tests/test_models.py @@ -156,6 +156,7 @@ def test_unhide_on_enable(self, unhide_mock): def test_unhide_disabled_files(self): f = File.objects.get(pk=67442) f.status = amo.STATUS_PUBLIC + f.filename = u'test_unhide_disabled_filés.xpi' with storage.open(f.guarded_file_path, 'wb') as fp: fp.write(b'some data\n') f.unhide_disabled_file() @@ -714,7 +715,7 @@ def setUp(self): def upload(self, **params): # The data should be in chunks. data = [bytes(bytearray(s)) for s in chunked(self.data, 3)] - return FileUpload.from_post(data, 'filename.xpi', + return FileUpload.from_post(data, u'filenamé.xpi', len(self.data), **params) def test_from_post_write_file(self): @@ -723,11 +724,12 @@ def test_from_post_write_file(self): def test_from_post_filename(self): upload = self.upload() assert upload.uuid - assert upload.name == '{0}_filename.xpi'.format(upload.uuid.hex) + assert upload.name == u'{0}_filenamé.xpi'.format( + force_text(upload.uuid.hex)) def test_from_post_hash(self): - hash = hashlib.sha256(self.data).hexdigest() - assert self.upload().hash == 'sha256:%s' % hash + hashdigest = hashlib.sha256(self.data).hexdigest() + assert self.upload().hash == 'sha256:%s' % hashdigest def test_from_post_extra_params(self): upload = self.upload(automated_signing=True, addon_id=3615) @@ -1092,6 +1094,21 @@ def test_utf8(self): upload, self.version, self.platform, parsed_data={}) assert file_.filename == u'jets-0.1.xpi' + @mock.patch('olympia.files.models.copy_stored_file') + def test_dont_send_both_bytes_and_str_to_copy_stored_file( + self, copy_stored_file_mock): + upload = self.upload(u'jétpack') + self.version.addon.name = u'jéts!' + file_ = File.from_upload( + upload, self.version, self.platform, parsed_data={}) + assert file_.filename == u'jets-0.1.xpi' + expected_path_orig = force_text(upload.path) + expected_path_dest = force_text(file_.current_file_path) + assert copy_stored_file_mock.call_count == 1 + assert copy_stored_file_mock.call_args_list[0][0] == ( + expected_path_orig, expected_path_dest + ) + def test_size(self): upload = self.upload('extension') file_ = File.from_upload(