Merge "relinker: Accept policy names, too"

swift/cli/relinker.py

@@ -37,9 +37,8 @@ EXIT_NO_APPLICABLE_POLICY = 2
 EXIT_ERROR = 1
 
 
-def policy(policy_index):
-    value = non_negative_int(policy_index)
-    value = POLICIES.get_by_index(value)
+def policy(policy_name_or_index):
+    value = POLICIES.get_by_name_or_index(policy_name_or_index)
     if value is None:
         raise ValueError
     return value

swift/common/storage_policy.py

@@ -819,6 +819,15 @@ class StoragePolicyCollection(object):
                 return None
         return self.by_index.get(index)
 
+    def get_by_name_or_index(self, name_or_index):
+        by_name = self.get_by_name(name_or_index)
+        by_index = self.get_by_index(name_or_index)
+        if by_name and by_index and by_name != by_index:
+            raise PolicyError(
+                "Found different polices when searching by "
+                "name (%s) and by index (%s)" % (by_name, by_index))
+        return by_name or by_index
+
     @property
     def legacy(self):
         return self.get_by_index(None)

test/unit/cli/test_relinker.py

@@ -1007,27 +1007,26 @@ class TestRelinker(unittest.TestCase):
         # invalid policy
         with mock.patch('sys.stdout'), mock.patch('sys.stderr'):
             with self.assertRaises(SystemExit) as cm:
-                self.assertEqual(0, relinker.main([
+                relinker.main([
                     'relink',
                     '--swift-dir', self.testdir,
                     '--policy', '9',
                     '--skip-mount',
                     '--devices', self.devices,
                     '--device', self.existing_device,
-                ]))
+                ])
         self.assertEqual(2, cm.exception.code)
 
-        # policy must be index not name
         with mock.patch('sys.stdout'), mock.patch('sys.stderr'):
             with self.assertRaises(SystemExit) as cm:
-                self.assertEqual(0, relinker.main([
+                relinker.main([
                     'relink',
                     '--swift-dir', self.testdir,
-                    '--policy', 'gold',
+                    '--policy', 'pewter',
                     '--skip-mount',
                     '--devices', self.devices,
                     '--device', self.existing_device,
-                ]))
+                ])
             self.assertEqual(2, cm.exception.code)
 
         # policy with no object
@@ -1074,6 +1073,30 @@ class TestRelinker(unittest.TestCase):
              '0 removed, 0 errors)'],
             self.logger.get_lines_for_level('info'))
 
+        # policy name works, too
+        self.logger.clear()
+        with mock.patch.object(relinker.logging, 'getLogger',
+                               return_value=self.logger):
+            self.assertEqual(0, relinker.main([
+                'relink',
+                '--swift-dir', self.testdir,
+                '--policy', 'gold',
+                '--skip-mount',
+                '--devices', self.devices,
+                '--device', self.existing_device,
+            ]))
+        self.assertTrue(os.path.isdir(self.expected_dir))
+        self.assertTrue(os.path.isfile(self.expected_file))
+        stat_old = os.stat(os.path.join(self.objdir, self.object_fname))
+        stat_new = os.stat(self.expected_file)
+        self.assertEqual(stat_old.st_ino, stat_new.st_ino)
+        self.assertEqual(
+            ['Processing files for policy gold under %s/%s (cleanup=False)'
+             % (self.devices, self.existing_device),
+             '0 hash dirs processed (cleanup=False) '
+             '(0 files, 0 linked, 0 removed, 0 errors)'],
+            self.logger.get_lines_for_level('info'))
+
     @patch_policies(
         [StoragePolicy(0, name='gold', is_default=True),
          ECStoragePolicy(1, name='platinum', ec_type=DEFAULT_TEST_EC_TYPE,
@@ -1105,7 +1128,7 @@ class TestRelinker(unittest.TestCase):
         self.assertEqual([mock.call(POLICIES[1])], mocked.call_args_list)
         self.assertEqual(0, res)
 
-        res, mocked = do_relink(['--policy', 0])
+        res, mocked = do_relink(['--policy', '0'])
         self.assertEqual([], mocked.call_args_list)
         self.assertEqual(2, res)
 

test/unit/common/test_storage_policy.py

@@ -367,6 +367,25 @@ class TestStoragePolicies(unittest.TestCase):
             self.assertEqual(pol1, policies.get_by_name(name))
             self.assertEqual(policies.get_by_name(name).name, 'One')
 
+    def test_wacky_int_names(self):
+        # checking duplicate on insert
+        test_policies = [StoragePolicy(0, '1', True, aliases='-1'),
+                         StoragePolicy(1, '0', False)]
+        policies = StoragePolicyCollection(test_policies)
+
+        with self.assertRaises(PolicyError):
+            policies.get_by_name_or_index('0')
+        self.assertEqual(policies.get_by_name('1'), test_policies[0])
+        self.assertEqual(policies.get_by_index(0), test_policies[0])
+
+        with self.assertRaises(PolicyError):
+            policies.get_by_name_or_index('1')
+        self.assertEqual(policies.get_by_name('0'), test_policies[1])
+        self.assertEqual(policies.get_by_index(1), test_policies[1])
+
+        self.assertIsNone(policies.get_by_index(-1))
+        self.assertEqual(policies.get_by_name_or_index('-1'), test_policies[0])
+
     def test_multiple_names(self):
         # checking duplicate on insert
         test_policies = [StoragePolicy(0, 'zero', True),