From 36c42974d6746a306002f8f8e1b3dae664c783d6 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Thu, 22 Feb 2018 22:48:55 +0000 Subject: [PATCH] py3: Port more CLI tools Bring under test - test/unit/cli/test_dispersion_report.py - test/unit/cli/test_info.py and - test/unit/cli/test_relinker.py I've verified that swift-*-info (at least) behave reasonably under py3, even swift-object-info when there's non-utf8 metadata on the data/meta file. Change-Id: Ifed4b8059337c395e56f5e9f8d939c34fe4ff8dd --- bin/swift-object-info | 8 ++++++ swift/cli/info.py | 4 +-- swift/common/db.py | 23 +++++++++++------ swift/common/swob.py | 4 +-- swift/obj/diskfile.py | 47 ++++++++++++++++++++++++++-------- test/unit/__init__.py | 2 +- test/unit/cli/test_info.py | 6 ++--- test/unit/cli/test_recon.py | 2 +- test/unit/cli/test_relinker.py | 4 +-- test/unit/obj/test_diskfile.py | 6 ++--- tox.ini | 3 +++ 11 files changed, 76 insertions(+), 33 deletions(-) diff --git a/bin/swift-object-info b/bin/swift-object-info index fc8a13e4b7..8e80b0eda7 100755 --- a/bin/swift-object-info +++ b/bin/swift-object-info @@ -14,15 +14,23 @@ # See the License for the specific language governing permissions and # limitations under the License. +import codecs import sys from optparse import OptionParser +import six + from swift.common.storage_policy import reload_storage_policies from swift.common.utils import set_swift_dir from swift.cli.info import print_obj, InfoSystemExit if __name__ == '__main__': + if not six.PY2: + # Make stdout able to write escaped bytes + sys.stdout = codecs.getwriter("utf-8")( + sys.stdout.detach(), errors='surrogateescape') + parser = OptionParser('%prog [options] OBJECT_FILE') parser.add_option( '-n', '--no-check-etag', default=True, diff --git a/swift/cli/info.py b/swift/cli/info.py index e6f3042c77..c2c46e7d56 100644 --- a/swift/cli/info.py +++ b/swift/cli/info.py @@ -352,8 +352,8 @@ def print_obj_metadata(metadata): def print_metadata(title, items): print(title) if items: - for meta_key in sorted(items): - print(' %s: %s' % (meta_key, items[meta_key])) + for key, value in sorted(items.items()): + print(' %s: %s' % (key, value)) else: print(' No metadata found') diff --git a/swift/common/db.py b/swift/common/db.py index 048d5e308c..b05eeb8d11 100644 --- a/swift/common/db.py +++ b/swift/common/db.py @@ -56,12 +56,19 @@ def utf8encode(*args): for s in args] -def utf8encodekeys(metadata): - uni_keys = [k for k in metadata if isinstance(k, six.text_type)] - for k in uni_keys: - sv = metadata[k] - del metadata[k] - metadata[k.encode('utf-8')] = sv +def native_str_keys(metadata): + if six.PY2: + uni_keys = [k for k in metadata if isinstance(k, six.text_type)] + for k in uni_keys: + sv = metadata[k] + del metadata[k] + metadata[k.encode('utf-8')] = sv + else: + bin_keys = [k for k in metadata if isinstance(k, six.binary_type)] + for k in bin_keys: + sv = metadata[k] + del metadata[k] + metadata[k.decode('utf-8')] = sv def _db_timeout(timeout, db_file, call): @@ -741,7 +748,7 @@ class DatabaseBroker(object): metadata = self.get_raw_metadata() if metadata: metadata = json.loads(metadata) - utf8encodekeys(metadata) + native_str_keys(metadata) else: metadata = {} return metadata @@ -803,7 +810,7 @@ class DatabaseBroker(object): self.db_type) md = row[0] md = json.loads(md) if md else {} - utf8encodekeys(md) + native_str_keys(md) except sqlite3.OperationalError as err: if 'no such column: metadata' not in str(err): raise diff --git a/swift/common/swob.py b/swift/common/swob.py index 6cfbc810d3..9b4842d63d 100644 --- a/swift/common/swob.py +++ b/swift/common/swob.py @@ -290,8 +290,8 @@ def _resp_status_property(): else: if isinstance(value, six.text_type): value = value.encode('utf-8') - self.status_int = int(value.split(' ', 1)[0]) - self.explanation = self.title = value.split(' ', 1)[1] + self.status_int = int(value.split(b' ', 1)[0]) + self.explanation = self.title = value.split(b' ', 1)[1] return property(getter, setter, doc="Retrieve and set the Response status, e.g. '200 OK'") diff --git a/swift/obj/diskfile.py b/swift/obj/diskfile.py index 9a2e9ff2a4..f7e22161b8 100644 --- a/swift/obj/diskfile.py +++ b/swift/obj/diskfile.py @@ -83,8 +83,8 @@ PICKLE_PROTOCOL = 2 DEFAULT_RECLAIM_AGE = timedelta(weeks=1).total_seconds() HASH_FILE = 'hashes.pkl' HASH_INVALIDATIONS_FILE = 'hashes.invalid' -METADATA_KEY = 'user.swift.metadata' -METADATA_CHECKSUM_KEY = 'user.swift.metadata_checksum' +METADATA_KEY = b'user.swift.metadata' +METADATA_CHECKSUM_KEY = b'user.swift.metadata_checksum' DROP_CACHE_WINDOW = 1024 * 1024 # These are system-set metadata keys that cannot be changed with a POST. # They should be lowercase. @@ -131,6 +131,26 @@ def _encode_metadata(metadata): return dict(((encode_str(k), encode_str(v)) for k, v in metadata.items())) +def _decode_metadata(metadata): + """ + Given a metadata dict from disk, convert keys and values to native strings. + + :param metadata: a dict + """ + if six.PY2: + def to_str(item): + if isinstance(item, six.text_type): + return item.encode('utf8') + return item + else: + def to_str(item): + if isinstance(item, six.binary_type): + return item.decode('utf8', 'surrogateescape') + return item + + return dict(((to_str(k), to_str(v)) for k, v in metadata.items())) + + def read_metadata(fd, add_missing_checksum=False): """ Helper function to read the pickled metadata from an object file. @@ -144,8 +164,8 @@ def read_metadata(fd, add_missing_checksum=False): key = 0 try: while True: - metadata += xattr.getxattr(fd, '%s%s' % (METADATA_KEY, - (key or ''))) + metadata += xattr.getxattr( + fd, METADATA_KEY + str(key or '').encode('ascii')) key += 1 except (IOError, OSError) as e: if errno.errorcode.get(e.errno) in ('ENOTSUP', 'EOPNOTSUPP'): @@ -173,7 +193,7 @@ def read_metadata(fd, add_missing_checksum=False): logging.error("Error adding metadata: %s" % e) if metadata_checksum: - computed_checksum = hashlib.md5(metadata).hexdigest() + computed_checksum = hashlib.md5(metadata).hexdigest().encode('ascii') if metadata_checksum != computed_checksum: raise DiskFileBadMetadataChecksum( "Metadata checksum mismatch for %s: " @@ -183,7 +203,11 @@ def read_metadata(fd, add_missing_checksum=False): # strings are utf-8 encoded when written, but have not always been # (see https://bugs.launchpad.net/swift/+bug/1678018) so encode them again # when read - return _encode_metadata(pickle.loads(metadata)) + if six.PY2: + metadata = pickle.loads(metadata) + else: + metadata = pickle.loads(metadata, encoding='bytes') + return _decode_metadata(metadata) def write_metadata(fd, metadata, xattr_size=65536): @@ -194,11 +218,11 @@ def write_metadata(fd, metadata, xattr_size=65536): :param metadata: metadata to write """ metastr = pickle.dumps(_encode_metadata(metadata), PICKLE_PROTOCOL) - metastr_md5 = hashlib.md5(metastr).hexdigest() + metastr_md5 = hashlib.md5(metastr).hexdigest().encode('ascii') key = 0 try: while metastr: - xattr.setxattr(fd, '%s%s' % (METADATA_KEY, key or ''), + xattr.setxattr(fd, METADATA_KEY + str(key or '').encode('ascii'), metastr[:xattr_size]) metastr = metastr[xattr_size:] key += 1 @@ -368,9 +392,10 @@ def invalidate_hash(suffix_dir): suffix = basename(suffix_dir) partition_dir = dirname(suffix_dir) invalidations_file = join(partition_dir, HASH_INVALIDATIONS_FILE) - with lock_path(partition_dir): - with open(invalidations_file, 'ab') as inv_fh: - inv_fh.write(suffix + "\n") + if not isinstance(suffix, bytes): + suffix = suffix.encode('utf-8') + with lock_path(partition_dir), open(invalidations_file, 'ab') as inv_fh: + inv_fh.write(suffix + b"\n") def relink_paths(target_path, new_target_path, check_existing=False): diff --git a/test/unit/__init__.py b/test/unit/__init__.py index a7c9d25965..4ecf902063 100644 --- a/test/unit/__init__.py +++ b/test/unit/__init__.py @@ -1292,7 +1292,7 @@ def xattr_supported_check(): # assume the worst -- xattrs aren't supported supports_xattr_cached_val = False - big_val = 'x' * (4096 + 1) # more than 4k of metadata + big_val = b'x' * (4096 + 1) # more than 4k of metadata try: fd, tmppath = mkstemp() xattr.setxattr(fd, 'user.swift.testing_key', big_val) diff --git a/test/unit/cli/test_info.py b/test/unit/cli/test_info.py index 3a111d98d8..5f6d8d202f 100644 --- a/test/unit/cli/test_info.py +++ b/test/unit/cli/test_info.py @@ -42,8 +42,8 @@ class TestCliInfoBase(unittest.TestCase): def setUp(self): skip_if_no_xattrs() self.orig_hp = utils.HASH_PATH_PREFIX, utils.HASH_PATH_SUFFIX - utils.HASH_PATH_PREFIX = 'info' - utils.HASH_PATH_SUFFIX = 'info' + utils.HASH_PATH_PREFIX = b'info' + utils.HASH_PATH_SUFFIX = b'info' self.testdir = os.path.join(mkdtemp(), 'tmp_test_cli_info') utils.mkdirs(self.testdir) rmtree(self.testdir) @@ -875,7 +875,7 @@ class TestPrintObj(TestCliInfoBase): self.assertRaises(InfoSystemExit, print_obj, datafile) with open(datafile, 'wb') as fp: - fp.write('1234') + fp.write(b'1234') out = StringIO() with mock.patch('sys.stdout', out): diff --git a/test/unit/cli/test_recon.py b/test/unit/cli/test_recon.py index 05a6cc1598..36e69c496c 100644 --- a/test/unit/cli/test_recon.py +++ b/test/unit/cli/test_recon.py @@ -511,7 +511,7 @@ aliases = %s self.recon_instance.umount_check(hosts) output = stdout.getvalue() - r = re.compile("\Not mounted:|Device errors: .*") + r = re.compile("^Not mounted:|Device errors: .*") lines = output.splitlines() self.assertTrue(lines) for line in lines: diff --git a/test/unit/cli/test_relinker.py b/test/unit/cli/test_relinker.py index a2116f68c3..35c6721bc1 100644 --- a/test/unit/cli/test_relinker.py +++ b/test/unit/cli/test_relinker.py @@ -62,7 +62,7 @@ class TestRelinker(unittest.TestCase): self.object_fname = "1278553064.00000.data" self.objname = os.path.join(self.objdir, self.object_fname) with open(self.objname, "wb") as dummy: - dummy.write("Hello World!") + dummy.write(b"Hello World!") write_metadata(dummy, {'name': '/a/c/o', 'Content-Length': '12'}) test_policies = [StoragePolicy(0, 'platin', True)] @@ -164,7 +164,7 @@ class TestRelinker(unittest.TestCase): self._common_test_cleanup() # Pretend the object in the new place got corrupted with open(self.expected_file, "wb") as obj: - obj.write('trash') + obj.write(b'trash') self.assertEqual( 1, relinker.cleanup(self.testdir, self.devices, True, self.logger)) diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py index 8b05efc603..76bd385224 100644 --- a/test/unit/obj/test_diskfile.py +++ b/test/unit/obj/test_diskfile.py @@ -326,7 +326,7 @@ class TestDiskFileModuleMethods(unittest.TestCase): check_metadata() # simulate a legacy diskfile that might have persisted unicode metadata - with mock.patch.object(diskfile, '_encode_metadata', lambda x: x): + with mock.patch.object(diskfile, '_decode_metadata', lambda x: x): with open(path, 'wb') as fd: diskfile.write_metadata(fd, metadata) # sanity check, while still mocked, that we did persist unicode @@ -334,8 +334,8 @@ class TestDiskFileModuleMethods(unittest.TestCase): actual = diskfile.read_metadata(fd) for k, v in actual.items(): if k == u'X-Object-Meta-Strange': - self.assertIsInstance(k, six.text_type) - self.assertIsInstance(v, six.text_type) + self.assertIsInstance(k, str) + self.assertIsInstance(v, str) break else: self.fail('Did not find X-Object-Meta-Strange') diff --git a/tox.ini b/tox.ini index 0984488217..9da8180366 100644 --- a/tox.ini +++ b/tox.ini @@ -29,6 +29,9 @@ setenv = VIRTUAL_ENV={envdir} [testenv:py34] commands = nosetests \ + test/unit/cli/test_dispersion_report.py \ + test/unit/cli/test_info.py \ + test/unit/cli/test_relinker.py \ test/unit/cli/test_ring_builder_analyzer.py \ test/unit/cli/test_ringbuilder.py \ test/unit/common/ring \