From 5683ed5163367cd4bc49fbf7069566f77f762eb4 Mon Sep 17 00:00:00 2001
From: Rodolphe Marques <marques.rodolphe@gmail.com>
Date: Tue, 24 Jan 2017 15:59:02 +0100
Subject: [PATCH 1/7] Added mongodb admin commands to add and remove members
 from the replicaset

---
 bigchaindb/backend/admin.py         | 10 +++++
 bigchaindb/backend/mongodb/admin.py | 59 +++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)
 create mode 100644 bigchaindb/backend/mongodb/admin.py

diff --git a/bigchaindb/backend/admin.py b/bigchaindb/backend/admin.py
index 057b5995..1a57c8a4 100644
--- a/bigchaindb/backend/admin.py
+++ b/bigchaindb/backend/admin.py
@@ -20,3 +20,13 @@ def set_shards(connection, *, shards):
 @singledispatch
 def set_replicas(connection, *, replicas):
     raise NotImplementedError
+
+
+@singledispatch
+def add_replicas(connection, *, replicas):
+    raise NotImplementedError
+
+
+@singledispatch
+def remove_replicas(connection, *, replicas):
+    raise NotImplementedError
diff --git a/bigchaindb/backend/mongodb/admin.py b/bigchaindb/backend/mongodb/admin.py
new file mode 100644
index 00000000..b41021e9
--- /dev/null
+++ b/bigchaindb/backend/mongodb/admin.py
@@ -0,0 +1,59 @@
+"""Database configuration functions."""
+import logging
+
+from bigchaindb.backend import admin
+from bigchaindb.backend.utils import module_dispatch_registrar
+from bigchaindb.backend.mongodb.connection import MongoDBConnection
+
+logger = logging.getLogger(__name__)
+
+register_admin = module_dispatch_registrar(admin)
+
+
+@register_admin(MongoDBConnection)
+def add_replicas(connection, replicas):
+    """Add a set of replicas to the replicaset
+
+    Args:
+        replicas list of strings: of the form "hostname:port".
+    """
+    # get current configuration
+    conf = connection.conn.admin.command('replSetGetConfig')
+
+    # MongoDB does not automatically add and id for the members so we need
+    # to chose one that does not exists yet. The safest way is to use
+    # incrementing ids, so we first check what is the highest id already in
+    # the set and continue from there.
+    cur_id = max([member['_id'] for member in conf['config']['members']])
+
+    # add the nodes to the members list of the replica set
+    for replica in replicas:
+        cur_id += 1
+        conf['config']['members'].append({'_id': cur_id, 'host': replica})
+
+    # increase the configuration version number
+    conf['config']['version'] += 1
+
+    # apply new configuration
+    return connection.conn.admin.command('replSetReconfig', conf['config'])
+
+
+@register_admin(MongoDBConnection)
+def remove_replicas(connection, replicas):
+    """Remove a set of replicas from the replicaset
+
+    """
+    # get the current configuration
+    conf = connection.conn.admin.command('replSetGetConfig')
+
+    # remove the nodes from the members list in the replica set
+    conf['config']['members'] = list(
+        filter(lambda member: member['host'] not in replicas,
+               conf['config']['members'])
+    )
+
+    # increase the configuration version number
+    conf['config']['version'] += 1
+
+    # apply new configuration
+    return connection.conn.admin.command('replSetReconfig', conf['config'])

From 69505a366baf76db045ac9e61f9723b1a95ae714 Mon Sep 17 00:00:00 2001
From: Rodolphe Marques <marques.rodolphe@gmail.com>
Date: Tue, 24 Jan 2017 17:55:06 +0100
Subject: [PATCH 2/7] Added bigchaindb commands to add and remove nodes from
 replicaset

---
 bigchaindb/backend/admin.py            |  4 +-
 bigchaindb/backend/mongodb/__init__.py |  2 +-
 bigchaindb/backend/mongodb/admin.py    | 13 +++++-
 bigchaindb/commands/bigchain.py        | 55 +++++++++++++++++++++++++-
 bigchaindb/commands/utils.py           | 31 ++++++++++++++-
 5 files changed, 98 insertions(+), 7 deletions(-)

diff --git a/bigchaindb/backend/admin.py b/bigchaindb/backend/admin.py
index 1a57c8a4..da54397b 100644
--- a/bigchaindb/backend/admin.py
+++ b/bigchaindb/backend/admin.py
@@ -23,10 +23,10 @@ def set_replicas(connection, *, replicas):
 
 
 @singledispatch
-def add_replicas(connection, *, replicas):
+def add_replicas(connection, replicas):
     raise NotImplementedError
 
 
 @singledispatch
-def remove_replicas(connection, *, replicas):
+def remove_replicas(connection, replicas):
     raise NotImplementedError
diff --git a/bigchaindb/backend/mongodb/__init__.py b/bigchaindb/backend/mongodb/__init__.py
index af5293ac..e3746fa3 100644
--- a/bigchaindb/backend/mongodb/__init__.py
+++ b/bigchaindb/backend/mongodb/__init__.py
@@ -16,7 +16,7 @@ generic backend interfaces to the implementations in this module.
 """
 
 # Register the single dispatched modules on import.
-from bigchaindb.backend.mongodb import schema, query, changefeed  # noqa
+from bigchaindb.backend.mongodb import admin, schema, query, changefeed  # noqa
 
 # MongoDBConnection should always be accessed via
 # ``bigchaindb.backend.connect()``.
diff --git a/bigchaindb/backend/mongodb/admin.py b/bigchaindb/backend/mongodb/admin.py
index b41021e9..3c2001d5 100644
--- a/bigchaindb/backend/mongodb/admin.py
+++ b/bigchaindb/backend/mongodb/admin.py
@@ -1,8 +1,11 @@
 """Database configuration functions."""
 import logging
 
+from pymongo.errors import OperationFailure
+
 from bigchaindb.backend import admin
 from bigchaindb.backend.utils import module_dispatch_registrar
+from bigchaindb.backend.exceptions import DatabaseOpFailedError
 from bigchaindb.backend.mongodb.connection import MongoDBConnection
 
 logger = logging.getLogger(__name__)
@@ -35,7 +38,10 @@ def add_replicas(connection, replicas):
     conf['config']['version'] += 1
 
     # apply new configuration
-    return connection.conn.admin.command('replSetReconfig', conf['config'])
+    try:
+        return connection.conn.admin.command('replSetReconfig', conf['config'])
+    except OperationFailure as exc:
+        raise DatabaseOpFailedError(exc.details['errmsg'])
 
 
 @register_admin(MongoDBConnection)
@@ -56,4 +62,7 @@ def remove_replicas(connection, replicas):
     conf['config']['version'] += 1
 
     # apply new configuration
-    return connection.conn.admin.command('replSetReconfig', conf['config'])
+    try:
+        return connection.conn.admin.command('replSetReconfig', conf['config'])
+    except OperationFailure as exc:
+        raise DatabaseOpFailedError(exc.details['errmsg'])
diff --git a/bigchaindb/commands/bigchain.py b/bigchaindb/commands/bigchain.py
index 2fc8df70..78b3b745 100644
--- a/bigchaindb/commands/bigchain.py
+++ b/bigchaindb/commands/bigchain.py
@@ -22,7 +22,8 @@ from bigchaindb.models import Transaction
 from bigchaindb.utils import ProcessGroup
 from bigchaindb import backend
 from bigchaindb.backend import schema
-from bigchaindb.backend.admin import set_replicas, set_shards
+from bigchaindb.backend.admin import (set_replicas, set_shards, add_replicas,
+                                      remove_replicas)
 from bigchaindb.backend.exceptions import DatabaseOpFailedError
 from bigchaindb.commands import utils
 from bigchaindb import processes
@@ -269,6 +270,32 @@ def run_set_replicas(args):
         logger.warn(e)
 
 
+def run_add_replicas(args):
+    # Note: This command is specific to MongoDB
+    bigchaindb.config_utils.autoconfigure(filename=args.config, force=True)
+    conn = backend.connect()
+
+    try:
+        add_replicas(conn, args.replicas)
+    except DatabaseOpFailedError as e:
+        logger.warn(e)
+    else:
+        logger.info('Added {} to the replicaset.'.format(args.replicas))
+
+
+def run_remove_replicas(args):
+    # Note: This command is specific to MongoDB
+    bigchaindb.config_utils.autoconfigure(filename=args.config, force=True)
+    conn = backend.connect()
+
+    try:
+        remove_replicas(conn, args.replicas)
+    except DatabaseOpFailedError as e:
+        logger.warn(e)
+    else:
+        logger.info('Removed {} from the replicaset.'.format(args.replicas))
+
+
 def create_parser():
     parser = argparse.ArgumentParser(
         description='Control your BigchainDB node.',
@@ -334,6 +361,32 @@ def create_parser():
                                  type=int, default=1,
                                  help='Number of replicas (i.e. the replication factor)')
 
+    # parser for adding nodes to the replica set
+    add_replicas_parser = subparsers.add_parser('add-replicas',
+                                                help='Add a set of nodes to the '
+                                                     'replica set. This command '
+                                                     'is specific to the MongoDB'
+                                                     ' backend.')
+
+    add_replicas_parser.add_argument('replicas', nargs='+',
+                                     type=utils.mongodb_host,
+                                     help='A list of space separated hosts to '
+                                          'add to the replicaset. Each host '
+                                          'should be in the form `host:port`.')
+
+    # parser for removing nodes from the replica set
+    rm_replicas_parser = subparsers.add_parser('remove-replicas',
+                                               help='Remove a set of nodes from the '
+                                                    'replica set. This command '
+                                                    'is specific to the MongoDB'
+                                                    ' backend.')
+
+    rm_replicas_parser.add_argument('replicas', nargs='+',
+                                    type=utils.mongodb_host,
+                                    help='A list of space separated hosts to '
+                                         'remove from the replicaset. Each host '
+                                         'should be in the form `host:port`.')
+
     load_parser = subparsers.add_parser('load',
                                         help='Write transactions to the backlog')
 
diff --git a/bigchaindb/commands/utils.py b/bigchaindb/commands/utils.py
index 510eb2f6..7b662308 100644
--- a/bigchaindb/commands/utils.py
+++ b/bigchaindb/commands/utils.py
@@ -3,14 +3,15 @@ for ``argparse.ArgumentParser``.
 """
 
 import argparse
-from bigchaindb.common.exceptions import StartupError
 import multiprocessing as mp
 import subprocess
 
 import rethinkdb as r
+from pymongo import uri_parser
 
 import bigchaindb
 from bigchaindb import backend
+from bigchaindb.common.exceptions import StartupError
 from bigchaindb.version import __version__
 
 
@@ -95,6 +96,34 @@ def start(parser, argv, scope):
     return func(args)
 
 
+def mongodb_host(host):
+    """Utility function that works as a type for mongodb ``host`` args.
+
+    This function validates the ``host`` args provided by to the
+    ``add-replicas`` and ``remove-replicas`` commands and checks if each arg
+    is in the form "host:port"
+
+    Args:
+        host (str): A string containing hostname and port (e.g. "host:port")
+
+    Raises:
+        ArgumentTypeError: if it fails to parse the argument
+    """
+    # check if mongodb can parse the host
+    try:
+        hostname, port = uri_parser.parse_host(host, default_port=None)
+    except ValueError as exc:
+        raise argparse.ArgumentTypeError(exc.args[0])
+
+    # we do require the port to be provided.
+    if port is None:
+        raise argparse.ArgumentTypeError('expected host in the form '
+                                         '`host:port`. Got `{}` instead.'
+                                         .format(host))
+
+    return host
+
+
 base_parser = argparse.ArgumentParser(add_help=False, prog='bigchaindb')
 
 base_parser.add_argument('-c', '--config',

From 391da2cf604287acf217051acd2743f65a8b94a8 Mon Sep 17 00:00:00 2001
From: Rodolphe Marques <marques.rodolphe@gmail.com>
Date: Wed, 25 Jan 2017 12:36:08 +0100
Subject: [PATCH 3/7] Added tests

---
 bigchaindb/backend/admin.py         |   6 +-
 bigchaindb/commands/bigchain.py     |   4 +-
 tests/backend/mongodb/test_admin.py | 118 ++++++++++++++++++++++++++++
 tests/backend/test_generics.py      |   2 +
 tests/commands/test_commands.py     |  54 ++++++++++++-
 5 files changed, 179 insertions(+), 5 deletions(-)
 create mode 100644 tests/backend/mongodb/test_admin.py

diff --git a/bigchaindb/backend/admin.py b/bigchaindb/backend/admin.py
index da54397b..f0ea62fd 100644
--- a/bigchaindb/backend/admin.py
+++ b/bigchaindb/backend/admin.py
@@ -24,9 +24,11 @@ def set_replicas(connection, *, replicas):
 
 @singledispatch
 def add_replicas(connection, replicas):
-    raise NotImplementedError
+    raise NotImplementedError('This command is specific to the '
+                              'MongoDB backend.')
 
 
 @singledispatch
 def remove_replicas(connection, replicas):
-    raise NotImplementedError
+    raise NotImplementedError('This command is specific to the '
+                              'MongoDB backend.')
diff --git a/bigchaindb/commands/bigchain.py b/bigchaindb/commands/bigchain.py
index 78b3b745..98e9d81c 100644
--- a/bigchaindb/commands/bigchain.py
+++ b/bigchaindb/commands/bigchain.py
@@ -277,7 +277,7 @@ def run_add_replicas(args):
 
     try:
         add_replicas(conn, args.replicas)
-    except DatabaseOpFailedError as e:
+    except (DatabaseOpFailedError, NotImplementedError) as e:
         logger.warn(e)
     else:
         logger.info('Added {} to the replicaset.'.format(args.replicas))
@@ -290,7 +290,7 @@ def run_remove_replicas(args):
 
     try:
         remove_replicas(conn, args.replicas)
-    except DatabaseOpFailedError as e:
+    except (DatabaseOpFailedError, NotImplementedError) as e:
         logger.warn(e)
     else:
         logger.info('Removed {} from the replicaset.'.format(args.replicas))
diff --git a/tests/backend/mongodb/test_admin.py b/tests/backend/mongodb/test_admin.py
new file mode 100644
index 00000000..138bc616
--- /dev/null
+++ b/tests/backend/mongodb/test_admin.py
@@ -0,0 +1,118 @@
+"""Tests for the :mod:`bigchaindb.backend.mongodb.admin` module."""
+import copy
+from unittest import mock
+
+import pytest
+from pymongo.database import Database
+from pymongo.errors import OperationFailure
+
+
+@pytest.fixture
+def mock_replicaset_config():
+    return {
+        'config': {
+            '_id': 'bigchain-rs',
+            'members': [
+                {
+                    '_id': 0,
+                    'arbiterOnly': False,
+                    'buildIndexes': True,
+                    'hidden': False,
+                    'host': 'localhost:27017',
+                    'priority': 1.0,
+                    'slaveDelay': 0,
+                    'tags': {},
+                    'votes': 1
+                }
+            ],
+            'version': 1
+        }
+    }
+
+
+def test_add_replicas(mock_replicaset_config):
+    from bigchaindb.backend import connect
+    from bigchaindb.backend.admin import add_replicas
+
+    connection = connect()
+    # force the connection object to setup a connection to the database
+    # before we mock `Database.command`
+    connection.conn
+
+    expected_config = copy.deepcopy(mock_replicaset_config)
+    expected_config['config']['members'] += [
+        {'_id': 1, 'host': 'localhost:27018'},
+        {'_id': 2, 'host': 'localhost:27019'}
+    ]
+    expected_config['config']['version'] += 1
+
+    with mock.patch.object(Database, 'command') as mock_command:
+        mock_command.return_value = mock_replicaset_config
+        add_replicas(connection, ['localhost:27018', 'localhost:27019'])
+
+    mock_command.assert_called_with('replSetReconfig',
+                                    expected_config['config'])
+
+
+def test_add_replicas_raises(mock_replicaset_config):
+    from bigchaindb.backend import connect
+    from bigchaindb.backend.admin import add_replicas
+    from bigchaindb.backend.exceptions import DatabaseOpFailedError
+
+    connection = connect()
+    # force the connection object to setup a connection to the database
+    # before we mock `Database.command`
+    connection.conn
+
+    with mock.patch.object(Database, 'command') as mock_command:
+        mock_command.side_effect = [
+            mock_replicaset_config,
+            OperationFailure(error=1, details={'errmsg': ''})
+        ]
+        with pytest.raises(DatabaseOpFailedError):
+            add_replicas(connection, ['localhost:27018'])
+
+
+def test_remove_replicas(mock_replicaset_config):
+    from bigchaindb.backend import connect
+    from bigchaindb.backend.admin import remove_replicas
+
+    connection = connect()
+    # force the connection object to setup a connection to the database
+    # before we mock `Database.command`
+    connection.conn
+
+    expected_config = copy.deepcopy(mock_replicaset_config)
+    expected_config['config']['version'] += 1
+
+    # add some hosts to the configuration to remove
+    mock_replicaset_config['config']['members'] += [
+        {'_id': 1, 'host': 'localhost:27018'},
+        {'_id': 2, 'host': 'localhost:27019'}
+    ]
+
+    with mock.patch.object(Database, 'command') as mock_command:
+        mock_command.return_value = mock_replicaset_config
+        remove_replicas(connection, ['localhost:27018', 'localhost:27019'])
+
+    mock_command.assert_called_with('replSetReconfig',
+                                    expected_config['config'])
+
+
+def test_remove_replicas_raises(mock_replicaset_config):
+    from bigchaindb.backend import connect
+    from bigchaindb.backend.admin import remove_replicas
+    from bigchaindb.backend.exceptions import DatabaseOpFailedError
+
+    connection = connect()
+    # force the connection object to setup a connection to the database
+    # before we mock `Database.command`
+    connection.conn
+
+    with mock.patch.object(Database, 'command') as mock_command:
+        mock_command.side_effect = [
+            mock_replicaset_config,
+            OperationFailure(error=1, details={'errmsg': ''})
+        ]
+        with pytest.raises(DatabaseOpFailedError):
+            remove_replicas(connection, ['localhost:27018'])
diff --git a/tests/backend/test_generics.py b/tests/backend/test_generics.py
index 2049d72b..0dd2637f 100644
--- a/tests/backend/test_generics.py
+++ b/tests/backend/test_generics.py
@@ -100,6 +100,8 @@ def test_init_database(mock_create_database, mock_create_tables,
     ('reconfigure', {'table': None, 'shards': None, 'replicas': None}),
     ('set_shards', {'shards': None}),
     ('set_replicas', {'replicas': None}),
+    ('add_replicas', {'replicas': None}),
+    ('remove_replicas', {'replicas': None}),
 ))
 def test_admin(admin_func_name, kwargs):
     from bigchaindb.backend import admin
diff --git a/tests/commands/test_commands.py b/tests/commands/test_commands.py
index 1a1291e3..f61c4c6f 100644
--- a/tests/commands/test_commands.py
+++ b/tests/commands/test_commands.py
@@ -1,6 +1,6 @@
 import json
 from unittest.mock import Mock, patch
-from argparse import Namespace
+from argparse import Namespace, ArgumentTypeError
 import copy
 
 import pytest
@@ -376,3 +376,55 @@ def test_calling_main(start_mock, base_parser_mock, parse_args_mock,
                                                'distributed equally to all '
                                                'the processes')
     assert start_mock.called is True
+
+
+@patch('bigchaindb.backend.admin.add_replicas')
+def test_run_add_replicas(mock_add_replicas):
+    from bigchaindb.commands.bigchain import run_add_replicas
+    from bigchaindb.backend.exceptions import DatabaseOpFailedError
+
+    args = Namespace(config=None, replicas=['localhost:27017'])
+
+    # test add_replicas no raises
+    mock_add_replicas.return_value = None
+    assert run_add_replicas(args) is None
+
+    # test add_replicas with `DatabaseOpFailedError`
+    mock_add_replicas.side_effect = DatabaseOpFailedError()
+    assert run_add_replicas(args) is None
+
+    # test add_replicas with `NotImplementedError`
+    mock_add_replicas.side_effect = NotImplementedError()
+    assert run_add_replicas(args) is None
+
+
+@patch('bigchaindb.backend.admin.remove_replicas')
+def test_run_remove_replicas(mock_remove_replicas):
+    from bigchaindb.commands.bigchain import run_remove_replicas
+    from bigchaindb.backend.exceptions import DatabaseOpFailedError
+
+    args = Namespace(config=None, replicas=['localhost:27017'])
+
+    # test add_replicas no raises
+    mock_remove_replicas.return_value = None
+    assert run_remove_replicas(args) is None
+
+    # test add_replicas with `DatabaseOpFailedError`
+    mock_remove_replicas.side_effect = DatabaseOpFailedError()
+    assert run_remove_replicas(args) is None
+
+    # test add_replicas with `NotImplementedError`
+    mock_remove_replicas.side_effect = NotImplementedError()
+    assert run_remove_replicas(args) is None
+
+
+def test_mongodb_host_type():
+    from bigchaindb.commands.utils import mongodb_host
+
+    # bad port provided
+    with pytest.raises(ArgumentTypeError):
+        mongodb_host('localhost:11111111111')
+
+    # no port information provided
+    with pytest.raises(ArgumentTypeError):
+        mongodb_host('localhost')

From 8e8a60a5399a43804c153a2209f4e5431d069335 Mon Sep 17 00:00:00 2001
From: Rodolphe Marques <marques.rodolphe@gmail.com>
Date: Wed, 25 Jan 2017 13:58:35 +0100
Subject: [PATCH 4/7] Get the correct configuration if backend is set by envs

---
 bigchaindb/__init__.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/bigchaindb/__init__.py b/bigchaindb/__init__.py
index 315774e5..79118e23 100644
--- a/bigchaindb/__init__.py
+++ b/bigchaindb/__init__.py
@@ -5,6 +5,12 @@ import os
 # PORT_NUMBER = reduce(lambda x, y: x * y, map(ord, 'BigchainDB')) % 2**16
 # basically, the port number is 9984
 
+
+def _get_database_from_env():
+    return globals()['_database_' + os.environ.get(
+        'BIGCHAINDB_DATABASE_BACKEND', 'rethinkdb')]
+
+
 _database_rethinkdb = {
     'backend': os.environ.get('BIGCHAINDB_DATABASE_BACKEND', 'rethinkdb'),
     'host': os.environ.get('BIGCHAINDB_DATABASE_HOST', 'localhost'),
@@ -28,7 +34,7 @@ config = {
         'workers': None,  # if none, the value will be cpu_count * 2 + 1
         'threads': None,  # if none, the value will be cpu_count * 2 + 1
     },
-    'database': _database_rethinkdb,
+    'database': _get_database_from_env(),
     'keypair': {
         'public': None,
         'private': None,

From 9d03aeb72a8debea42e7775e8b77983620efa4d3 Mon Sep 17 00:00:00 2001
From: Rodolphe Marques <marques.rodolphe@gmail.com>
Date: Thu, 26 Jan 2017 15:02:48 +0100
Subject: [PATCH 5/7] fixed tests

---
 tests/commands/test_commands.py |  2 ++
 tests/test_config_utils.py      | 27 +++++++++++++++++++++------
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/tests/commands/test_commands.py b/tests/commands/test_commands.py
index f61c4c6f..16b615eb 100644
--- a/tests/commands/test_commands.py
+++ b/tests/commands/test_commands.py
@@ -378,6 +378,7 @@ def test_calling_main(start_mock, base_parser_mock, parse_args_mock,
     assert start_mock.called is True
 
 
+@pytest.mark.usefixtures('ignore_local_config_file')
 @patch('bigchaindb.backend.admin.add_replicas')
 def test_run_add_replicas(mock_add_replicas):
     from bigchaindb.commands.bigchain import run_add_replicas
@@ -398,6 +399,7 @@ def test_run_add_replicas(mock_add_replicas):
     assert run_add_replicas(args) is None
 
 
+@pytest.mark.usefixtures('ignore_local_config_file')
 @patch('bigchaindb.backend.admin.remove_replicas')
 def test_run_remove_replicas(mock_remove_replicas):
     from bigchaindb.commands.bigchain import run_remove_replicas
diff --git a/tests/test_config_utils.py b/tests/test_config_utils.py
index c1f63742..7ebe5579 100644
--- a/tests/test_config_utils.py
+++ b/tests/test_config_utils.py
@@ -131,6 +131,26 @@ def test_autoconfigure_read_both_from_file_and_env(monkeypatch, request):
     from bigchaindb import config_utils
     config_utils.autoconfigure()
 
+    backend = request.config.getoption('--database-backend')
+    database_rethinkdb = {
+        'backend': 'rethinkdb',
+        'host': 'test-host',
+        'port': 4242,
+        'name': 'test-dbname',
+    }
+    database_mongodb = {
+        'backend': 'mongodb',
+        'host': 'test-host',
+        'port': 4242,
+        'name': 'test-dbname',
+        'replicaset': 'bigchain-rs',
+    }
+
+    # default
+    database = database_rethinkdb
+    if backend == 'mongodb':
+        database = database_mongodb
+
     assert bigchaindb.config == {
         'CONFIGURED': True,
         'server': {
@@ -138,12 +158,7 @@ def test_autoconfigure_read_both_from_file_and_env(monkeypatch, request):
             'workers': None,
             'threads': None,
         },
-        'database': {
-            'backend': request.config.getoption('--database-backend'),
-            'host': 'test-host',
-            'port': 4242,
-            'name': 'test-dbname',
-        },
+        'database': database,
         'keypair': {
             'public': None,
             'private': None,

From f0e298bcd7d436ffd42a9562ac1495952a64dc8e Mon Sep 17 00:00:00 2001
From: Rodolphe Marques <marques.rodolphe@gmail.com>
Date: Tue, 31 Jan 2017 14:54:36 +0100
Subject: [PATCH 6/7] Added docstrings. Removed unnecessary returns.

Created fixture to simplify the tests.
Better comments.
---
 bigchaindb/backend/mongodb/admin.py | 28 ++++++++++++++----
 tests/backend/mongodb/test_admin.py | 44 +++++++++++------------------
 2 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/bigchaindb/backend/mongodb/admin.py b/bigchaindb/backend/mongodb/admin.py
index 3c2001d5..afe909ac 100644
--- a/bigchaindb/backend/mongodb/admin.py
+++ b/bigchaindb/backend/mongodb/admin.py
@@ -18,13 +18,20 @@ def add_replicas(connection, replicas):
     """Add a set of replicas to the replicaset
 
     Args:
-        replicas list of strings: of the form "hostname:port".
+        connection (:class:`~bigchaindb.backend.connection.Connection`):
+            A connection to the database.
+        replicas (:obj:`list` of :obj:`str`): replica addresses in the
+            form "hostname:port".
+
+    Raises:
+        DatabaseOpFailedError: If the reconfiguration fails due to a MongoDB
+            :exc:`OperationFailure`
     """
     # get current configuration
     conf = connection.conn.admin.command('replSetGetConfig')
 
-    # MongoDB does not automatically add and id for the members so we need
-    # to chose one that does not exists yet. The safest way is to use
+    # MongoDB does not automatically add an id for the members so we need
+    # to choose one that does not exists yet. The safest way is to use
     # incrementing ids, so we first check what is the highest id already in
     # the set and continue from there.
     cur_id = max([member['_id'] for member in conf['config']['members']])
@@ -35,11 +42,13 @@ def add_replicas(connection, replicas):
         conf['config']['members'].append({'_id': cur_id, 'host': replica})
 
     # increase the configuration version number
+    # when reconfiguring, mongodb expects a version number higher than the one
+    # it currently has
     conf['config']['version'] += 1
 
     # apply new configuration
     try:
-        return connection.conn.admin.command('replSetReconfig', conf['config'])
+        connection.conn.admin.command('replSetReconfig', conf['config'])
     except OperationFailure as exc:
         raise DatabaseOpFailedError(exc.details['errmsg'])
 
@@ -48,6 +57,15 @@ def add_replicas(connection, replicas):
 def remove_replicas(connection, replicas):
     """Remove a set of replicas from the replicaset
 
+    Args:
+        connection (:class:`~bigchaindb.backend.connection.Connection`):
+            A connection to the database.
+        replicas (:obj:`list` of :obj:`str`): replica addresses in the
+            form "hostname:port".
+
+    Raises:
+        DatabaseOpFailedError: If the reconfiguration fails due to a MongoDB
+            :exc:`OperationFailure`
     """
     # get the current configuration
     conf = connection.conn.admin.command('replSetGetConfig')
@@ -63,6 +81,6 @@ def remove_replicas(connection, replicas):
 
     # apply new configuration
     try:
-        return connection.conn.admin.command('replSetReconfig', conf['config'])
+        connection.conn.admin.command('replSetReconfig', conf['config'])
     except OperationFailure as exc:
         raise DatabaseOpFailedError(exc.details['errmsg'])
diff --git a/tests/backend/mongodb/test_admin.py b/tests/backend/mongodb/test_admin.py
index 138bc616..a7784369 100644
--- a/tests/backend/mongodb/test_admin.py
+++ b/tests/backend/mongodb/test_admin.py
@@ -30,14 +30,22 @@ def mock_replicaset_config():
     }
 
 
-def test_add_replicas(mock_replicaset_config):
+@pytest.fixture
+def connection():
     from bigchaindb.backend import connect
-    from bigchaindb.backend.admin import add_replicas
-
     connection = connect()
-    # force the connection object to setup a connection to the database
-    # before we mock `Database.command`
-    connection.conn
+    # connection is a lazy object. It only actually creates a connection to
+    # the database when its first used.
+    # During the setup of a MongoDBConnection some `Database.command` are
+    # executed to make sure that the replica set is correctly initialized.
+    # Here we force the the connection setup so that all required
+    # `Database.command` are executed before we mock them it in the tests.
+    connection._connect()
+    return connection
+
+
+def test_add_replicas(mock_replicaset_config, connection):
+    from bigchaindb.backend.admin import add_replicas
 
     expected_config = copy.deepcopy(mock_replicaset_config)
     expected_config['config']['members'] += [
@@ -54,16 +62,10 @@ def test_add_replicas(mock_replicaset_config):
                                     expected_config['config'])
 
 
-def test_add_replicas_raises(mock_replicaset_config):
-    from bigchaindb.backend import connect
+def test_add_replicas_raises(mock_replicaset_config, connection):
     from bigchaindb.backend.admin import add_replicas
     from bigchaindb.backend.exceptions import DatabaseOpFailedError
 
-    connection = connect()
-    # force the connection object to setup a connection to the database
-    # before we mock `Database.command`
-    connection.conn
-
     with mock.patch.object(Database, 'command') as mock_command:
         mock_command.side_effect = [
             mock_replicaset_config,
@@ -73,15 +75,9 @@ def test_add_replicas_raises(mock_replicaset_config):
             add_replicas(connection, ['localhost:27018'])
 
 
-def test_remove_replicas(mock_replicaset_config):
-    from bigchaindb.backend import connect
+def test_remove_replicas(mock_replicaset_config, connection):
     from bigchaindb.backend.admin import remove_replicas
 
-    connection = connect()
-    # force the connection object to setup a connection to the database
-    # before we mock `Database.command`
-    connection.conn
-
     expected_config = copy.deepcopy(mock_replicaset_config)
     expected_config['config']['version'] += 1
 
@@ -99,16 +95,10 @@ def test_remove_replicas(mock_replicaset_config):
                                     expected_config['config'])
 
 
-def test_remove_replicas_raises(mock_replicaset_config):
-    from bigchaindb.backend import connect
+def test_remove_replicas_raises(mock_replicaset_config, connection):
     from bigchaindb.backend.admin import remove_replicas
     from bigchaindb.backend.exceptions import DatabaseOpFailedError
 
-    connection = connect()
-    # force the connection object to setup a connection to the database
-    # before we mock `Database.command`
-    connection.conn
-
     with mock.patch.object(Database, 'command') as mock_command:
         mock_command.side_effect = [
             mock_replicaset_config,

From 84626b6e327369c3e3ee521638ecac6fff8b7598 Mon Sep 17 00:00:00 2001
From: Rodolphe Marques <marques.rodolphe@gmail.com>
Date: Tue, 31 Jan 2017 16:23:09 +0100
Subject: [PATCH 7/7] Improved tests

Fixed typo
Add extra validation to hostnames to make sure host is not empty
---
 bigchaindb/backend/mongodb/admin.py |  2 +-
 bigchaindb/commands/utils.py        |  2 +-
 tests/commands/test_commands.py     | 22 ++++++++++++++++++++--
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/bigchaindb/backend/mongodb/admin.py b/bigchaindb/backend/mongodb/admin.py
index afe909ac..7d72c3a4 100644
--- a/bigchaindb/backend/mongodb/admin.py
+++ b/bigchaindb/backend/mongodb/admin.py
@@ -31,7 +31,7 @@ def add_replicas(connection, replicas):
     conf = connection.conn.admin.command('replSetGetConfig')
 
     # MongoDB does not automatically add an id for the members so we need
-    # to choose one that does not exists yet. The safest way is to use
+    # to choose one that does not exist yet. The safest way is to use
     # incrementing ids, so we first check what is the highest id already in
     # the set and continue from there.
     cur_id = max([member['_id'] for member in conf['config']['members']])
diff --git a/bigchaindb/commands/utils.py b/bigchaindb/commands/utils.py
index 7b662308..80ee7a6b 100644
--- a/bigchaindb/commands/utils.py
+++ b/bigchaindb/commands/utils.py
@@ -116,7 +116,7 @@ def mongodb_host(host):
         raise argparse.ArgumentTypeError(exc.args[0])
 
     # we do require the port to be provided.
-    if port is None:
+    if port is None or hostname == '':
         raise argparse.ArgumentTypeError('expected host in the form '
                                          '`host:port`. Got `{}` instead.'
                                          .format(host))
diff --git a/tests/commands/test_commands.py b/tests/commands/test_commands.py
index 16b615eb..95bb0db7 100644
--- a/tests/commands/test_commands.py
+++ b/tests/commands/test_commands.py
@@ -22,6 +22,8 @@ def test_make_sure_we_dont_remove_any_command():
     assert parser.parse_args(['set-shards', '1']).command
     assert parser.parse_args(['set-replicas', '1']).command
     assert parser.parse_args(['load']).command
+    assert parser.parse_args(['add-replicas', 'localhost:27017']).command
+    assert parser.parse_args(['remove-replicas', 'localhost:27017']).command
 
 
 def test_start_raises_if_command_not_implemented():
@@ -379,7 +381,7 @@ def test_calling_main(start_mock, base_parser_mock, parse_args_mock,
 
 
 @pytest.mark.usefixtures('ignore_local_config_file')
-@patch('bigchaindb.backend.admin.add_replicas')
+@patch('bigchaindb.commands.bigchain.add_replicas')
 def test_run_add_replicas(mock_add_replicas):
     from bigchaindb.commands.bigchain import run_add_replicas
     from bigchaindb.backend.exceptions import DatabaseOpFailedError
@@ -389,18 +391,24 @@ def test_run_add_replicas(mock_add_replicas):
     # test add_replicas no raises
     mock_add_replicas.return_value = None
     assert run_add_replicas(args) is None
+    assert mock_add_replicas.call_count == 1
+    mock_add_replicas.reset_mock()
 
     # test add_replicas with `DatabaseOpFailedError`
     mock_add_replicas.side_effect = DatabaseOpFailedError()
     assert run_add_replicas(args) is None
+    assert mock_add_replicas.call_count == 1
+    mock_add_replicas.reset_mock()
 
     # test add_replicas with `NotImplementedError`
     mock_add_replicas.side_effect = NotImplementedError()
     assert run_add_replicas(args) is None
+    assert mock_add_replicas.call_count == 1
+    mock_add_replicas.reset_mock()
 
 
 @pytest.mark.usefixtures('ignore_local_config_file')
-@patch('bigchaindb.backend.admin.remove_replicas')
+@patch('bigchaindb.commands.bigchain.remove_replicas')
 def test_run_remove_replicas(mock_remove_replicas):
     from bigchaindb.commands.bigchain import run_remove_replicas
     from bigchaindb.backend.exceptions import DatabaseOpFailedError
@@ -410,14 +418,20 @@ def test_run_remove_replicas(mock_remove_replicas):
     # test add_replicas no raises
     mock_remove_replicas.return_value = None
     assert run_remove_replicas(args) is None
+    assert mock_remove_replicas.call_count == 1
+    mock_remove_replicas.reset_mock()
 
     # test add_replicas with `DatabaseOpFailedError`
     mock_remove_replicas.side_effect = DatabaseOpFailedError()
     assert run_remove_replicas(args) is None
+    assert mock_remove_replicas.call_count == 1
+    mock_remove_replicas.reset_mock()
 
     # test add_replicas with `NotImplementedError`
     mock_remove_replicas.side_effect = NotImplementedError()
     assert run_remove_replicas(args) is None
+    assert mock_remove_replicas.call_count == 1
+    mock_remove_replicas.reset_mock()
 
 
 def test_mongodb_host_type():
@@ -430,3 +444,7 @@ def test_mongodb_host_type():
     # no port information provided
     with pytest.raises(ArgumentTypeError):
         mongodb_host('localhost')
+
+    # bad host provided
+    with pytest.raises(ArgumentTypeError):
+        mongodb_host(':27017')