Code Cleanup and adding unit tests

Cleaning code to allow only URI type MongoClient connection,
adding unit tests

Change-Id: Iacca44c00006a1072ff70989b0220e2b27bc5fd8
Signed-off-by: aticig <gulsum.atici@canonical.com>

diff --git a/osm_common/dbmongo.py b/osm_common/dbmongo.py
index 8561e96..f64949d 100644 (file)
--- a/osm_common/dbmongo.py
+++ b/osm_common/dbmongo.py
@@ -105,13 +105,6 @@ class DbMongo(DbBase):
                 self.client = MongoClient(
                     config["uri"], replicaSet=config.get("replicaset", None)
                 )
-            else:
-                self.client = MongoClient(
-                    config["host"],
-                    config["port"],
-                    replicaSet=config.get("replicaset", None),
-                )
-            # TODO add as parameters also username=config.get("user"), password=config.get("password"))
             # when all modules are ready
             self.db = self.client[config["name"]]
             if "loglevel" in config:

diff --git a/osm_common/fsmongo.py b/osm_common/fsmongo.py
index 1ce5909..727410e 100644 (file)
--- a/osm_common/fsmongo.py
+++ b/osm_common/fsmongo.py
@@ -275,14 +275,11 @@ class FsMongo(FsBase):
             if all(key in config.keys() for key in ["uri", "collection"]):
                 self.client = MongoClient(config["uri"])
                 self.fs = GridFSBucket(self.client[config["collection"]])
-            elif all(key in config.keys() for key in ["host", "port", "collection"]):
-                self.client = MongoClient(config["host"], config["port"])
-                self.fs = GridFSBucket(self.client[config["collection"]])
             else:
                 if "collection" not in config.keys():
                     raise FsException('Missing parameter "collection"')
                 else:
-                    raise FsException('Missing parameters: "uri" or "host" + "port"')
+                    raise FsException('Missing parameters: "uri"')
         except FsException:
             raise
         except Exception as e:  # TODO refine

diff --git a/osm_common/tests/test_dbmongo.py b/osm_common/tests/test_dbmongo.py
new file mode 100644 (file)
index 0000000..52c9c55
--- /dev/null
+++ b/osm_common/tests/test_dbmongo.py
@@ -0,0 +1,501 @@
+#######################################################################################
+# Copyright ETSI Contributors and Others.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#######################################################################################
+
+import logging
+from urllib.parse import quote
+
+from osm_common.dbbase import DbException, FakeLock
+from osm_common.dbmongo import DbMongo
+from pymongo import MongoClient
+import pytest
+
+
+def db_status_exception_message():
+    return "database exception Wrong database status"
+
+
+def db_version_exception_message():
+    return "database exception Invalid database version"
+
+
+def mock_get_one_status_not_enabled(a, b, c, fail_on_empty=False, fail_on_more=True):
+    return {"status": "ERROR", "version": "", "serial": ""}
+
+
+def mock_get_one_wrong_db_version(a, b, c, fail_on_empty=False, fail_on_more=True):
+    return {"status": "ENABLED", "version": "4.0", "serial": "MDY4OA=="}
+
+
+def db_generic_exception(exception):
+    return exception
+
+
+def db_generic_exception_message(message):
+    return f"database exception {message}"
+
+
+def test_constructor():
+    db = DbMongo(lock=True)
+    assert db.logger == logging.getLogger("db")
+    assert db.db is None
+    assert db.client is None
+    assert db.database_key is None
+    assert db.secret_obtained is False
+    assert db.lock.acquire() is True
+
+
+def test_constructor_with_logger():
+    logger_name = "db_mongo"
+    db = DbMongo(logger_name=logger_name, lock=False)
+    assert db.logger == logging.getLogger(logger_name)
+    assert db.db is None
+    assert db.client is None
+    assert db.database_key is None
+    assert db.secret_obtained is False
+    assert type(db.lock) == FakeLock
+
+
+@pytest.mark.parametrize(
+    "config, target_version, serial, lock",
+    [
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": "mongo:27017",
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "5.0",
+            "MDY=",
+            True,
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "masterpassword": "master",
+                "uri": "mongo:27017",
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "5.0",
+            "MDY=",
+            False,
+        ),
+        (
+            {
+                "logger_name": "logger",
+                "uri": "mongo:27017",
+                "name": "newdb",
+                "commonkey": "common",
+            },
+            "3.6",
+            "",
+            True,
+        ),
+        (
+            {
+                "uri": "mongo:27017",
+                "commonkey": "common",
+                "name": "newdb",
+            },
+            "5.0",
+            "MDIy",
+            False,
+        ),
+        (
+            {
+                "uri": "mongo:27017",
+                "masterpassword": "common",
+                "name": "newdb",
+                "loglevel": "CRITICAL",
+            },
+            "4.4",
+            "OTA=",
+            False,
+        ),
+        (
+            {
+                "uri": "mongo",
+                "masterpassword": "common",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "4.4",
+            "OTA=",
+            True,
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": quote("user4:password4@mongo"),
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "5.0",
+            "NTM=",
+            True,
+        ),
+        (
+            {
+                "logger_name": "logger",
+                "uri": quote("user3:password3@mongo:27017"),
+                "name": "newdb",
+                "commonkey": "common",
+            },
+            "4.0",
+            "NjEx",
+            False,
+        ),
+        (
+            {
+                "uri": quote("user2:password2@mongo:27017"),
+                "commonkey": "common",
+                "name": "newdb",
+            },
+            "5.0",
+            "cmV0MzI=",
+            False,
+        ),
+        (
+            {
+                "uri": quote("user1:password1@mongo:27017"),
+                "commonkey": "common",
+                "masterpassword": "master",
+                "name": "newdb",
+                "loglevel": "CRITICAL",
+            },
+            "4.0",
+            "MjMyNQ==",
+            False,
+        ),
+        (
+            {
+                "uri": quote("user1:password1@mongo"),
+                "masterpassword": "common",
+                "name": "newdb",
+                "loglevel": "CRITICAL",
+            },
+            "4.0",
+            "MjMyNQ==",
+            True,
+        ),
+    ],
+)
+def test_db_connection_with_valid_config(
+    config, target_version, serial, lock, monkeypatch
+):
+    def mock_get_one(a, b, c, fail_on_empty=False, fail_on_more=True):
+        return {"status": "ENABLED", "version": target_version, "serial": serial}
+
+    monkeypatch.setattr(DbMongo, "get_one", mock_get_one)
+    db = DbMongo(lock=lock)
+    db.db_connect(config, target_version)
+    assert (
+        db.logger == logging.getLogger(config.get("logger_name"))
+        if config.get("logger_name")
+        else logging.getLogger("db")
+    )
+    assert type(db.client) == MongoClient
+    assert db.database_key == "common"
+    assert db.logger.getEffectiveLevel() == 50 if config.get("loglevel") else 20
+
+
+@pytest.mark.parametrize(
+    "config, target_version, version_data, expected_exception_message",
+    [
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": "mongo:27017",
+                "replicaset": "rs2",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "4.0",
+            mock_get_one_status_not_enabled,
+            db_status_exception_message(),
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": "mongo:27017",
+                "replicaset": "rs4",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "5.0",
+            mock_get_one_wrong_db_version,
+            db_version_exception_message(),
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": quote("user2:pa@word2@mongo:27017"),
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "DEBUG",
+            },
+            "4.0",
+            mock_get_one_status_not_enabled,
+            db_status_exception_message(),
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": quote("username:pass1rd@mongo:27017"),
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "DEBUG",
+            },
+            "5.0",
+            mock_get_one_wrong_db_version,
+            db_version_exception_message(),
+        ),
+    ],
+)
+def test_db_connection_db_status_error(
+    config, target_version, version_data, expected_exception_message, monkeypatch
+):
+    monkeypatch.setattr(DbMongo, "get_one", version_data)
+    db = DbMongo(lock=False)
+    with pytest.raises(DbException) as exception_info:
+        db.db_connect(config, target_version)
+    assert str(exception_info.value).startswith(expected_exception_message)
+
+
+@pytest.mark.parametrize(
+    "config, target_version, lock, expected_exception",
+    [
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": "27017@/:",
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "4.0",
+            True,
+            db_generic_exception(DbException),
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": "user@pass",
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "4.0",
+            False,
+            db_generic_exception(DbException),
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": "user@pass:27017",
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "4.0",
+            True,
+            db_generic_exception(DbException),
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": "",
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "5.0",
+            False,
+            db_generic_exception(TypeError),
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": "user2::@mon:27017",
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "DEBUG",
+            },
+            "4.0",
+            True,
+            db_generic_exception(ValueError),
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "replicaset": 33,
+                "uri": "user2@@mongo:27017",
+                "name": "osmdb",
+                "loglevel": "DEBUG",
+            },
+            "5.0",
+            False,
+            db_generic_exception(TypeError),
+        ),
+    ],
+)
+def test_db_connection_with_invalid_uri(
+    config, target_version, lock, expected_exception, monkeypatch
+):
+    def mock_get_one(a, b, c, fail_on_empty=False, fail_on_more=True):
+        pass
+
+    monkeypatch.setattr(DbMongo, "get_one", mock_get_one)
+    db = DbMongo(lock=lock)
+    with pytest.raises(expected_exception) as exception_info:
+        db.db_connect(config, target_version)
+    assert type(exception_info.value) == expected_exception
+
+
+@pytest.mark.parametrize(
+    "config, target_version, expected_exception",
+    [
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "",
+            db_generic_exception(TypeError),
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "uri": "mongo:27017",
+                "replicaset": "rs0",
+                "loglevel": "CRITICAL",
+            },
+            "4.0",
+            db_generic_exception(KeyError),
+        ),
+        (
+            {
+                "replicaset": "rs0",
+                "loglevel": "CRITICAL",
+            },
+            None,
+            db_generic_exception(KeyError),
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": "",
+                "replicaset": "rs0",
+                "name": "osmdb",
+                "loglevel": "CRITICAL",
+            },
+            "5.0",
+            db_generic_exception(TypeError),
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "name": "osmdb",
+            },
+            "4.0",
+            db_generic_exception(TypeError),
+        ),
+        (
+            {
+                "logger_name": "logger",
+                "replicaset": "",
+                "uri": "user2@@mongo:27017",
+            },
+            "5.0",
+            db_generic_exception(KeyError),
+        ),
+    ],
+)
+def test_db_connection_with_missing_parameters(
+    config, target_version, expected_exception, monkeypatch
+):
+    def mock_get_one(a, b, c, fail_on_empty=False, fail_on_more=True):
+        return
+
+    monkeypatch.setattr(DbMongo, "get_one", mock_get_one)
+    db = DbMongo(lock=False)
+    with pytest.raises(expected_exception) as exception_info:
+        db.db_connect(config, target_version)
+    assert type(exception_info.value) == expected_exception
+
+
+@pytest.mark.parametrize(
+    "config, expected_exception_message",
+    [
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": "mongo:27017",
+                "replicaset": "rs0",
+                "name": "osmdb1",
+                "loglevel": "CRITICAL",
+            },
+            "MongoClient crashed",
+        ),
+        (
+            {
+                "logger_name": "mongo_logger",
+                "commonkey": "common",
+                "uri": "username:pas1ed@mongo:27017",
+                "replicaset": "rs1",
+                "name": "osmdb2",
+                "loglevel": "DEBUG",
+            },
+            "MongoClient crashed",
+        ),
+    ],
+)
+def test_db_connection_with_invalid_mongoclient(
+    config, expected_exception_message, monkeypatch
+):
+    def generate_exception(a, b, replicaSet=None):
+        raise DbException(expected_exception_message)
+
+    monkeypatch.setattr(MongoClient, "__init__", generate_exception)
+    db = DbMongo()
+    with pytest.raises(DbException) as exception_info:
+        db.db_connect(config)
+    assert str(exception_info.value) == db_generic_exception_message(
+        expected_exception_message
+    )

diff --git a/osm_common/tests/test_fsmongo.py b/osm_common/tests/test_fsmongo.py
index 01a8ef2..7e1e47c 100644 (file)
--- a/osm_common/tests/test_fsmongo.py
+++ b/osm_common/tests/test_fsmongo.py
@@ -44,7 +44,7 @@ def invalid_path():
 
 @pytest.fixture(scope="function", params=[True, False])
 def fs_mongo(request, monkeypatch):
-    def mock_mongoclient_constructor(a, b, c):
+    def mock_mongoclient_constructor(a, b):
         pass
 
     def mock_mongoclient_getitem(a, b):
@@ -57,9 +57,7 @@ def fs_mongo(request, monkeypatch):
     monkeypatch.setattr(MongoClient, "__getitem__", mock_mongoclient_getitem)
     monkeypatch.setattr(GridFSBucket, "__init__", mock_gridfs_constructor)
     fs = FsMongo(lock=request.param)
-    fs.fs_connect(
-        {"path": valid_path(), "host": "mongo", "port": 27017, "collection": "files"}
-    )
+    fs.fs_connect({"path": valid_path(), "uri": "mongo:27017", "collection": "files"})
     return fs
 
 
@@ -140,17 +138,6 @@ def test_get_params(fs_mongo, monkeypatch):
             "fs_mongo",
             valid_path(),
         ),
-        (
-            {
-                "logger_name": "fs_mongo",
-                "path": valid_path(),
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            "fs_mongo",
-            valid_path(),
-        ),
         (
             {
                 "logger_name": "fs_mongo",
@@ -161,47 +148,16 @@ def test_get_params(fs_mongo, monkeypatch):
             "fs_mongo",
             valid_path(),
         ),
-        (
-            {
-                "logger_name": "fs_mongo",
-                "path": valid_path()[:-1],
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            "fs_mongo",
-            valid_path(),
-        ),
         (
             {"path": valid_path(), "uri": "mongo:27017", "collection": "files"},
             "fs",
             valid_path(),
         ),
-        (
-            {
-                "path": valid_path(),
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            "fs",
-            valid_path(),
-        ),
         (
             {"path": valid_path()[:-1], "uri": "mongo:27017", "collection": "files"},
             "fs",
             valid_path(),
         ),
-        (
-            {
-                "path": valid_path()[:-1],
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            "fs",
-            valid_path(),
-        ),
     ],
 )
 def test_fs_connect_with_valid_config(config, exp_logger, exp_path):
@@ -225,16 +181,6 @@ def test_fs_connect_with_valid_config(config, exp_logger, exp_path):
             },
             fs_connect_exception_message(invalid_path()),
         ),
-        (
-            {
-                "logger_name": "fs_mongo",
-                "path": invalid_path(),
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            fs_connect_exception_message(invalid_path()),
-        ),
         (
             {
                 "logger_name": "fs_mongo",
@@ -244,44 +190,16 @@ def test_fs_connect_with_valid_config(config, exp_logger, exp_path):
             },
             fs_connect_exception_message(invalid_path()[:-1]),
         ),
-        (
-            {
-                "logger_name": "fs_mongo",
-                "path": invalid_path()[:-1],
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            fs_connect_exception_message(invalid_path()[:-1]),
-        ),
         (
             {"path": invalid_path(), "uri": "mongo:27017", "collection": "files"},
             fs_connect_exception_message(invalid_path()),
         ),
-        (
-            {
-                "path": invalid_path(),
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            fs_connect_exception_message(invalid_path()),
-        ),
         (
             {"path": invalid_path()[:-1], "uri": "mongo:27017", "collection": "files"},
             fs_connect_exception_message(invalid_path()[:-1]),
         ),
         (
-            {
-                "path": invalid_path()[:-1],
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            fs_connect_exception_message(invalid_path()[:-1]),
-        ),
-        (
-            {"path": "/", "host": "mongo", "port": 27017, "collection": "files"},
+            {"path": "/", "uri": "mongo:27017", "collection": "files"},
             generic_fs_exception_message(
                 "Invalid configuration param at '[storage]': path '/' is not writable"
             ),
@@ -302,50 +220,14 @@ def test_fs_connect_with_invalid_path(config, exp_exception_message):
             {"logger_name": "fs_mongo", "uri": "mongo:27017", "collection": "files"},
             'Missing parameter "path"',
         ),
-        (
-            {
-                "logger_name": "fs_mongo",
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            'Missing parameter "path"',
-        ),
         (
             {"logger_name": "fs_mongo", "path": valid_path(), "collection": "files"},
-            'Missing parameters: "uri" or "host" + "port"',
-        ),
-        (
-            {
-                "logger_name": "fs_mongo",
-                "path": valid_path(),
-                "port": 27017,
-                "collection": "files",
-            },
-            'Missing parameters: "uri" or "host" + "port"',
-        ),
-        (
-            {
-                "logger_name": "fs_mongo",
-                "path": valid_path(),
-                "host": "mongo",
-                "collection": "files",
-            },
-            'Missing parameters: "uri" or "host" + "port"',
+            'Missing parameters: "uri"',
         ),
         (
             {"logger_name": "fs_mongo", "path": valid_path(), "uri": "mongo:27017"},
             'Missing parameter "collection"',
         ),
-        (
-            {
-                "logger_name": "fs_mongo",
-                "path": valid_path(),
-                "host": "mongo",
-                "port": 27017,
-            },
-            'Missing parameter "collection"',
-        ),
     ],
 )
 def test_fs_connect_with_missing_parameters(config, exp_exception_message):
@@ -367,22 +249,12 @@ def test_fs_connect_with_missing_parameters(config, exp_exception_message):
             },
             "MongoClient crashed",
         ),
-        (
-            {
-                "logger_name": "fs_mongo",
-                "path": valid_path(),
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            "MongoClient crashed",
-        ),
     ],
 )
 def test_fs_connect_with_invalid_mongoclient(
     config, exp_exception_message, monkeypatch
 ):
-    def generate_exception(a, b, c=None):
+    def generate_exception(a, b=None):
         raise Exception(exp_exception_message)
 
     monkeypatch.setattr(MongoClient, "__init__", generate_exception)
@@ -405,22 +277,12 @@ def test_fs_connect_with_invalid_mongoclient(
             },
             "Collection unavailable",
         ),
-        (
-            {
-                "logger_name": "fs_mongo",
-                "path": valid_path(),
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            "Collection unavailable",
-        ),
     ],
 )
 def test_fs_connect_with_invalid_mongo_collection(
     config, exp_exception_message, monkeypatch
 ):
-    def mock_mongoclient_constructor(a, b, c=None):
+    def mock_mongoclient_constructor(a, b=None):
         pass
 
     def generate_exception(a, b):
@@ -447,22 +309,12 @@ def test_fs_connect_with_invalid_mongo_collection(
             },
             "GridFsBucket crashed",
         ),
-        (
-            {
-                "logger_name": "fs_mongo",
-                "path": valid_path(),
-                "host": "mongo",
-                "port": 27017,
-                "collection": "files",
-            },
-            "GridFsBucket crashed",
-        ),
     ],
 )
 def test_fs_connect_with_invalid_gridfsbucket(
     config, exp_exception_message, monkeypatch
 ):
-    def mock_mongoclient_constructor(a, b, c=None):
+    def mock_mongoclient_constructor(a, b=None):
         pass
 
     def mock_mongoclient_getitem(a, b):

diff --git a/releasenotes/notes/code_cleanup-ee340441905782bf.yaml b/releasenotes/notes/code_cleanup-ee340441905782bf.yaml
new file mode 100644 (file)
index 0000000..c026c54
--- /dev/null
+++ b/releasenotes/notes/code_cleanup-ee340441905782bf.yaml
@@ -0,0 +1,21 @@
+#######################################################################################
+# Copyright ETSI Contributors and Others.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#######################################################################################
+---
+other:
+  - |
+    Only uri type authentication is enabled in MongoClient, parameter based (host, port..)
+    authentication is removed. Unit tests are added in dbmongo, changed in fsmongo.