From 1afe2c7bbb571004736daf347f5178a27128704c Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Tue, 19 Aug 2014 10:01:36 -0400 Subject: [PATCH] 2800: Improve spec conformance of Python SDK KeepLocator. * Require size to immediately follow digest. * Accept all valid hints. --- sdk/python/arvados/keep.py | 22 +++++++++--------- sdk/python/tests/test_collections.py | 2 +- sdk/python/tests/test_keep_locator.py | 32 ++++++++++++++++++++------- 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/sdk/python/arvados/keep.py b/sdk/python/arvados/keep.py index bffb02a147..36678291d8 100644 --- a/sdk/python/arvados/keep.py +++ b/sdk/python/arvados/keep.py @@ -31,28 +31,30 @@ import arvados.util class KeepLocator(object): EPOCH_DATETIME = datetime.datetime.utcfromtimestamp(0) + HINT_RE = re.compile(r'^[A-Z][A-Za-z0-9@_-]+$') def __init__(self, locator_str): - self.size = None - self.loc_hint = None + self.hints = [] self._perm_sig = None self._perm_expiry = None pieces = iter(locator_str.split('+')) self.md5sum = next(pieces) + try: + self.size = int(next(pieces)) + except StopIteration: + self.size = None for hint in pieces: - if hint.startswith('A'): + if self.HINT_RE.match(hint) is None: + raise ValueError("unrecognized hint data {}".format(hint)) + elif hint.startswith('A'): self.parse_permission_hint(hint) - elif hint.startswith('K'): - self.loc_hint = hint # FIXME - elif hint.isdigit(): - self.size = int(hint) else: - raise ValueError("unrecognized hint data {}".format(hint)) + self.hints.append(hint) def __str__(self): return '+'.join( - str(s) for s in [self.md5sum, self.size, self.loc_hint, - self.permission_hint()] + str(s) for s in [self.md5sum, self.size, + self.permission_hint()] + self.hints if s is not None) def _make_hex_prop(name, length): diff --git a/sdk/python/tests/test_collections.py b/sdk/python/tests/test_collections.py index 5354f3a9f7..b1af723635 100644 --- a/sdk/python/tests/test_collections.py +++ b/sdk/python/tests/test_collections.py @@ -586,7 +586,7 @@ class ArvadosCollectionsTest(ArvadosKeepLocalStoreTestCase): state = cwriter.current_state() # Add an expired locator to the state. state['_current_stream_locators'].append(''.join([ - 'a' * 32, '+A', 'b' * 40, '@', '10000000'])) + 'a' * 32, '+1+A', 'b' * 40, '@', '10000000'])) self.assertRaises(arvados.errors.StaleWriterStateError, TestResumableWriter.from_state, state) diff --git a/sdk/python/tests/test_keep_locator.py b/sdk/python/tests/test_keep_locator.py index e9d635673c..a7e9cb1bc3 100644 --- a/sdk/python/tests/test_keep_locator.py +++ b/sdk/python/tests/test_keep_locator.py @@ -8,7 +8,7 @@ import unittest from arvados.keep import KeepLocator -class ArvadosPutResumeCacheTest(unittest.TestCase): +class ArvadosKeepLocatorTest(unittest.TestCase): DEFAULT_TEST_COUNT = 10 def numstrs(fmtstr, base, exponent): @@ -22,13 +22,17 @@ class ArvadosPutResumeCacheTest(unittest.TestCase): signatures = numstrs('{:040x}', 16, 40) timestamps = numstrs('{:08x}', 16, 8) + def base_locators(self, count=DEFAULT_TEST_COUNT): + return ('+'.join(pair) for pair in + itertools.izip(self.checksums(count), self.sizes(count))) + def perm_hints(self, count=DEFAULT_TEST_COUNT): for sig, ts in itertools.izip(self.signatures(count), self.timestamps(count)): yield 'A{}@{}'.format(sig, ts) def test_good_locators_returned(self): - for hint_gens in [(), (self.sizes(),), (self.perm_hints(),), + for hint_gens in [(), (self.sizes(),), (self.sizes(), self.perm_hints())]: for loc_data in itertools.izip(self.checksums(), *hint_gens): locator = '+'.join(loc_data) @@ -40,24 +44,36 @@ class ArvadosPutResumeCacheTest(unittest.TestCase): '3+8f9e68d957b504a29ba76c526c3145d9']: self.assertRaises(ValueError, KeepLocator, badstr) + def test_unknown_hints_accepted(self): + base = next(self.base_locators(1)) + for weirdhint in ['Zfoo', 'Ybar234', 'Xa@b_c-372', 'W99']: + locator = '+'.join([base, weirdhint]) + self.assertEquals(locator, str(KeepLocator(locator))) + def test_bad_hints_rejected(self): - checksum = next(self.checksums(1)) - for badhint in ['', 'nonsense', '+32', checksum]: + base = next(self.base_locators(1)) + for badhint in ['', 'A', 'lowercase', '+32']: self.assertRaises(ValueError, KeepLocator, - '+'.join([checksum, badhint])) + '+'.join([base, badhint])) + + def test_multiple_locator_hints_accepted(self): + base = next(self.base_locators(1)) + for loc_hints in itertools.permutations(['Kab1cd', 'Kef2gh', 'Kij3kl']): + locator = '+'.join((base,) + loc_hints) + self.assertEquals(locator, str(KeepLocator(locator))) def test_expiry_passed(self): - checksum = next(self.checksums(1)) + base = next(self.base_locators(1)) signature = next(self.signatures(1)) dt1980 = datetime.datetime(1980, 1, 1) dt2000 = datetime.datetime(2000, 2, 2) dt2080 = datetime.datetime(2080, 3, 3) - locator = KeepLocator(checksum) + locator = KeepLocator(base) self.assertFalse(locator.permission_expired()) self.assertFalse(locator.permission_expired(dt1980)) self.assertFalse(locator.permission_expired(dt2080)) # Timestamped to 1987-01-05 18:48:32. - locator = KeepLocator('{}+A{}@20000000'.format(checksum, signature)) + locator = KeepLocator('{}+A{}@20000000'.format(base, signature)) self.assertTrue(locator.permission_expired()) self.assertTrue(locator.permission_expired(dt2000)) self.assertFalse(locator.permission_expired(dt1980)) -- 2.30.2