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
--- a/osm_common/dbmongo.py
+++ b/osm_common/dbmongo.py
@@ -105,13 +105,6 @@
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
--- a/osm_common/fsmongo.py
+++ b/osm_common/fsmongo.py
@@ -275,14 +275,11 @@
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
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
--- a/osm_common/tests/test_fsmongo.py
+++ b/osm_common/tests/test_fsmongo.py
@@ -44,7 +44,7 @@
@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 @@
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
@@ -143,17 +141,6 @@
(
{
"logger_name": "fs_mongo",
- "path": valid_path(),
- "host": "mongo",
- "port": 27017,
- "collection": "files",
- },
- "fs_mongo",
- valid_path(),
- ),
- (
- {
- "logger_name": "fs_mongo",
"path": valid_path()[:-1],
"uri": "mongo:27017",
"collection": "files",
@@ -162,46 +149,15 @@
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):
@@ -228,16 +184,6 @@
(
{
"logger_name": "fs_mongo",
- "path": invalid_path(),
- "host": "mongo",
- "port": 27017,
- "collection": "files",
- },
- fs_connect_exception_message(invalid_path()),
- ),
- (
- {
- "logger_name": "fs_mongo",
"path": invalid_path()[:-1],
"uri": "mongo:27017",
"collection": "files",
@@ -245,43 +191,15 @@
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"
),
@@ -303,49 +221,13 @@
'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 @@
},
"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 @@
},
"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 @@
},
"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
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.