Repository: fetchai/tools-pocketbook

Issue:
Add feature to rename account
Take this example:
```
Wallet2                  key  9.0            0.0   
Wallet 3                 key  15.0           0.0   
```
... opps. Wallet 3 was meant to be Wallet3. Propose:

```
pocketbook rename "Wallet 3" "Wallet3"
```


Discussion:

Patch:
diff --git a/pocketbook/address_book.py b/pocketbook/address_book.py
index f39fd4c..aba8bbb 100644
--- a/pocketbook/address_book.py
+++ b/pocketbook/address_book.py
@@ -20,6 +20,24 @@ class AddressBook:
         self._address_book[name] = str(address)
         self._save()
 
+    def rename(self, old: str, new: str) -> bool:
+        """
+        Rename an address from one name to another
+
+        :param old: The old name for the address
+        :param new: The new name for the address
+        :return: True if successful, otherwise False
+        """
+        success = False
+
+        if old in self._address_book and new not in self._address_book:
+            self._address_book[new] = self._address_book[old]
+            del self._address_book[old]
+            self._save()
+            success = True
+
+        return success
+
     def keys(self):
         return self._address_book.keys()
 
diff --git a/pocketbook/cli.py b/pocketbook/cli.py
index c12b74c..bf1de02 100644
--- a/pocketbook/cli.py
+++ b/pocketbook/cli.py
@@ -9,6 +9,7 @@ from .commands.create import run_create
 from .commands.display import run_display
 from .commands.list import run_list
 from .commands.transfer import run_transfer
+from .commands.rename import run_rename
 from .disclaimer import display_disclaimer
 from .utils import NetworkUnavailableError
 
@@ -43,6 +44,11 @@ def parse_commandline():
     parser_transfer.add_argument('signers', nargs='+', help='The series of key names needed to sign the transaction')
     parser_transfer.set_defaults(handler=run_transfer)
 
+    parser_rename = subparsers.add_parser('rename', aliases=['mv'])
+    parser_rename.add_argument('old', help='The name of the old account name')
+    parser_rename.add_argument('new', help='The new name of the account')
+    parser_rename.set_defaults(handler=run_rename)
+
     return parser, parser.parse_args()
 
 
@@ -65,9 +71,13 @@ def main():
         try:
 
             # execute the handler
-            handler(args)
+            ret = handler(args)
 
-            exit_code = 0
+            # update the exit code based on the return value
+            if isinstance(ret, int):
+                exit_code = ret
+            else:
+                exit_code = 0
 
         except NetworkUnavailableError:
             print('The network appears to be unavailable at the moment. Please try again later')
diff --git a/pocketbook/commands/rename.py b/pocketbook/commands/rename.py
new file mode 100644
index 0000000..e395c0f
--- /dev/null
+++ b/pocketbook/commands/rename.py
@@ -0,0 +1,33 @@
+def run_rename(args):
+    from pocketbook.address_book import AddressBook
+    from pocketbook.key_store import KeyStore
+
+    address_book = AddressBook()
+    key_store = KeyStore()
+
+    # make sure that the new name is not present either as a key, or as an address
+    new_present = args.new in address_book.keys() or args.new in key_store.list_keys()
+    if new_present:
+        print('{} is already present, please choose a different destination name'.format(args.new))
+        return 1
+
+    # check the old address or key name
+    old_is_address = args.old in address_book.keys()
+    old_is_key = args.old in key_store.list_keys()
+
+    success = False
+    if old_is_address and old_is_key:
+        raise RuntimeError('Data store corrupting, key looks like an address + key')
+    elif old_is_address:
+        success = address_book.rename(args.old, args.new)
+    elif old_is_key:
+        success = key_store.rename_key(args.old, args.new)
+    else:
+        print('{} doesn\'t appear to be a valid key or address name, please check and try again'.format(args.old))
+        return 1
+
+    if not success:
+        print('Failed to rename {} to {}'.format(args.old, args.new))
+        return 1
+
+    return 0
diff --git a/pocketbook/key_store.py b/pocketbook/key_store.py
index 7f8b942..88cec46 100644
--- a/pocketbook/key_store.py
+++ b/pocketbook/key_store.py
@@ -1,4 +1,5 @@
 import os
+from copy import deepcopy
 
 import toml
 from fetchai.ledger.crypto import Entity, Address
@@ -87,6 +88,35 @@ class KeyStore:
         })
         self._flush_index()
 
+    def rename_key(self, old_name: str, new_name: str) -> bool:
+
+        # do some basic checks
+        if old_name not in self.list_keys():
+            return False
+        if new_name in self.list_keys():
+            return False
+
+        # extract the old key metadata
+        old_metadata = self._lookup_meta_data(old_name)
+        if old_metadata is None:
+            return False
+
+        # move the key file
+        old_key_file_path = self._format_key_path(old_name)
+        new_key_file_path = self._format_key_path(new_name)
+        os.rename(old_key_file_path, new_key_file_path)
+
+        # remove the old entries to the metadata
+        self._index['key'] = list(filter(lambda x: x['name'] != old_name, self._index['key']))
+
+        # update and insert the new meta data into place
+        new_metadata = deepcopy(old_metadata)
+        new_metadata['name'] = new_name
+        self._index['key'].append(new_metadata)
+        self._flush_index()
+
+        return True
+
     def _format_key_path(self, name: str):
         return os.path.join(self._root, '{}.key'.format(name))
 


Test patch:
diff --git a/tests/commands/test_rename.py b/tests/commands/test_rename.py
new file mode 100644
index 0000000..f9515c7
--- /dev/null
+++ b/tests/commands/test_rename.py
@@ -0,0 +1,147 @@
+import unittest
+from unittest.mock import patch, Mock
+
+from pocketbook.address_book import AddressBook
+from pocketbook.key_store import KeyStore
+
+
+class RenameCommandTests(unittest.TestCase):
+
+    @patch('pocketbook.address_book.AddressBook', spec=AddressBook)
+    @patch('pocketbook.key_store.KeyStore', spec=KeyStore)
+    def test_normal_key_rename(self, MockKeyStore, MockAddressBook):
+        key_store = MockKeyStore()
+        key_store.list_keys.return_value = ['old-name']
+
+        address_book = MockAddressBook()
+        address_book.keys.return_value = []
+
+        args = Mock()
+        args.old = 'old-name'
+        args.new = 'new-name'
+
+        from pocketbook.commands.rename import run_rename
+        self.assertEqual(run_rename(args), 0)
+
+        key_store.rename_key.assert_called_once_with('old-name', 'new-name')
+
+    @patch('pocketbook.address_book.AddressBook', spec=AddressBook)
+    @patch('pocketbook.key_store.KeyStore', spec=KeyStore)
+    def test_normal_address_rename(self, MockKeyStore, MockAddressBook):
+        key_store = MockKeyStore()
+        key_store.list_keys.return_value = []
+
+        address_book = MockAddressBook()
+        address_book.keys.return_value = ['old-name']
+
+        args = Mock()
+        args.old = 'old-name'
+        args.new = 'new-name'
+
+        from pocketbook.commands.rename import run_rename
+        self.assertEqual(run_rename(args), 0)
+
+        address_book.rename.assert_called_once_with('old-name', 'new-name')
+
+    @patch('pocketbook.address_book.AddressBook', spec=AddressBook)
+    @patch('pocketbook.key_store.KeyStore', spec=KeyStore)
+    def test_doesnt_exits(self, MockKeyStore, MockAddressBook):
+        key_store = MockKeyStore()
+        key_store.list_keys.return_value = []
+
+        address_book = MockAddressBook()
+        address_book.keys.return_value = []
+
+        args = Mock()
+        args.old = 'old-name'
+        args.new = 'new-name'
+
+        from pocketbook.commands.rename import run_rename
+        self.assertEqual(run_rename(args), 1)
+
+    @patch('pocketbook.address_book.AddressBook', spec=AddressBook)
+    @patch('pocketbook.key_store.KeyStore', spec=KeyStore)
+    def test_duplicate_destination_key_name(self, MockKeyStore, MockAddressBook):
+        key_store = MockKeyStore()
+        key_store.list_keys.return_value = ['new-name']
+
+        address_book = MockAddressBook()
+        address_book.keys.return_value = []
+
+        args = Mock()
+        args.old = 'old-name'
+        args.new = 'new-name'
+
+        from pocketbook.commands.rename import run_rename
+        self.assertEqual(run_rename(args), 1)
+
+    @patch('pocketbook.address_book.AddressBook', spec=AddressBook)
+    @patch('pocketbook.key_store.KeyStore', spec=KeyStore)
+    def test_duplicate_destination_address_name(self, MockKeyStore, MockAddressBook):
+        key_store = MockKeyStore()
+        key_store.list_keys.return_value = []
+
+        address_book = MockAddressBook()
+        address_book.keys.return_value = ['new-name']
+
+        args = Mock()
+        args.old = 'old-name'
+        args.new = 'new-name'
+
+        from pocketbook.commands.rename import run_rename
+        self.assertEqual(run_rename(args), 1)
+
+    @patch('pocketbook.address_book.AddressBook', spec=AddressBook)
+    @patch('pocketbook.key_store.KeyStore', spec=KeyStore)
+    def test_corrupted_db_duplicate_address_key(self, MockKeyStore, MockAddressBook):
+        key_store = MockKeyStore()
+        key_store.list_keys.return_value = ['old-name']
+
+        address_book = MockAddressBook()
+        address_book.keys.return_value = ['old-name']
+
+        args = Mock()
+        args.old = 'old-name'
+        args.new = 'new-name'
+
+        from pocketbook.commands.rename import run_rename
+        with self.assertRaises(RuntimeError):
+            run_rename(args)
+
+    @patch('pocketbook.address_book.AddressBook', spec=AddressBook)
+    @patch('pocketbook.key_store.KeyStore', spec=KeyStore)
+    def test_handle_address_rename_failure(self, MockKeyStore, MockAddressBook):
+        key_store = MockKeyStore()
+        key_store.list_keys.return_value = []
+
+        address_book = MockAddressBook()
+        address_book.keys.return_value = ['old-name']
+        address_book.rename.return_value = False
+
+        args = Mock()
+        args.old = 'old-name'
+        args.new = 'new-name'
+
+        from pocketbook.commands.rename import run_rename
+        self.assertEqual(run_rename(args), 1)
+
+        address_book.rename.assert_called_once_with('old-name', 'new-name')
+
+    @patch('pocketbook.address_book.AddressBook', spec=AddressBook)
+    @patch('pocketbook.key_store.KeyStore', spec=KeyStore)
+    def test_handle_key_rename_failure(self, MockKeyStore, MockAddressBook):
+        key_store = MockKeyStore()
+        key_store.list_keys.return_value = ['old-name']
+        key_store.rename_key.return_value = False
+
+        address_book = MockAddressBook()
+        address_book.keys.return_value = []
+
+        args = Mock()
+        args.old = 'old-name'
+        args.new = 'new-name'
+
+        from pocketbook.commands.rename import run_rename
+        self.assertEqual(run_rename(args), 1)
+
+        key_store.rename_key.assert_called_once_with('old-name', 'new-name')
diff --git a/tests/test_address_book.py b/tests/test_address_book.py
index 10e616f..694db76 100644
--- a/tests/test_address_book.py
+++ b/tests/test_address_book.py
@@ -77,3 +77,24 @@ class AddressBookTests(unittest.TestCase):
 
             address_book2 = AddressBook(root=ctx.root)
             self.assertEqual(set(address_book1.items()), set(address_book2.items()))
+
+    def test_rename(self):
+        with TemporaryPocketBookRoot() as ctx:
+            address_book1 = AddressBook(root=ctx.root)
+            address_book1.add('sample', SAMPLE_ADDRESS)
+            self.assertTrue(address_book1.rename('sample', 'sample2'))
+            self.assertIn('sample2', address_book1.keys())
+            self.assertNotIn('sample', address_book1.keys())
+
+    def test_rename_occurs_on_disk(self):
+        with TemporaryPocketBookRoot() as ctx:
+            address_book1 = AddressBook(root=ctx.root)
+            address_book1.add('sample', SAMPLE_ADDRESS)
+
+            self.assertTrue(address_book1.rename('sample', 'sample2'))
+            self.assertAddressIsPresentOnDisk('sample2', SAMPLE_ADDRESS, ctx)
+
+    def test_rename_fails(self):
+        with TemporaryPocketBookRoot() as ctx:
+            address_book1 = AddressBook(root=ctx.root)
+            self.assertFalse(address_book1.rename('sample', 'sample2'))
diff --git a/tests/test_key_store.py b/tests/test_key_store.py
index a3b035e..b0ed8b7 100644
--- a/tests/test_key_store.py
+++ b/tests/test_key_store.py
@@ -1,5 +1,6 @@
 import os
 import unittest
+from unittest.mock import patch
 
 import toml
 from fetchai.ledger.crypto import Entity, Address
@@ -8,8 +9,7 @@ from pocketbook.key_store import KeyStore, DuplicateKeyNameError, KeyNotFoundErr
 from .utils import TemporaryPocketBookRoot, SUPER_SECURE_PASSWORD
 
 
-
-class AddressBookTests(unittest.TestCase):
+class KeyStoreTests(unittest.TestCase):
     def assertFlushedToDisk(self, name: str, password: str, entity: Entity, ctx: TemporaryPocketBookRoot):
         self.assertTrue(os.path.isdir(ctx.root))
 
@@ -126,4 +126,49 @@ class AddressBookTests(unittest.TestCase):
 
     def test_unable_to_decode_key_error_message(self):
         error = UnableToDecodeKeyError()
-        self.assertEqual(str(error), 'Unable to decode key from key store')
\ No newline at end of file
+        self.assertEqual(str(error), 'Unable to decode key from key store')
+
+    def test_rename(self):
+        with TemporaryPocketBookRoot() as ctx:
+            entity = Entity()
+            key_store = KeyStore(root=ctx.root)
+            key_store.add_key('sample', SUPER_SECURE_PASSWORD, entity)
+
+            self.assertTrue(key_store.rename_key('sample', 'sample2'))
+            self.assertIn('sample2', key_store.list_keys())
+            self.assertNotIn('sample', key_store.list_keys())
+
+    def test_rename_is_persisted_to_disk(self):
+        with TemporaryPocketBookRoot() as ctx:
+            entity = Entity()
+            key_store = KeyStore(root=ctx.root)
+            key_store.add_key('sample', SUPER_SECURE_PASSWORD, entity)
+
+            self.assertTrue(key_store.rename_key('sample', 'sample2'))
+            self.assertFlushedToDisk('sample2', SUPER_SECURE_PASSWORD, entity, ctx)
+
+    def test_rename_failure_doesnt_exist(self):
+        with TemporaryPocketBookRoot() as ctx:
+            key_store = KeyStore(root=ctx.root)
+            self.assertFalse(key_store.rename_key('sample', 'sample2'))
+
+    def test_rename_failure_already_exists(self):
+        with TemporaryPocketBookRoot() as ctx:
+            entity1 = Entity()
+            entity2 = Entity()
+
+            key_store = KeyStore(root=ctx.root)
+            key_store.add_key('sample1', SUPER_SECURE_PASSWORD, entity1)
+            key_store.add_key('sample2', SUPER_SECURE_PASSWORD, entity2)
+
+            self.assertFalse(key_store.rename_key('sample1', 'sample2'))
+
+    def test_corrupted_metadata(self):
+        with TemporaryPocketBookRoot() as ctx:
+            entity1 = Entity()
+
+            key_store = KeyStore(root=ctx.root)
+            key_store.add_key('sample1', SUPER_SECURE_PASSWORD, entity1)
+
+            with patch.object(key_store, '_lookup_meta_data', return_value=None):
+                self.assertFalse(key_store.rename_key('sample1', 'sample2'))
\ No newline at end of file