From: aticig Date: Thu, 31 Mar 2022 20:07:21 +0000 (+0300) Subject: Code Cleanup and adding unit tests X-Git-Tag: v12.0.0rc1~1 X-Git-Url: https://osm.etsi.org/gitweb/?a=commitdiff_plain;h=refs%2Fchanges%2F16%2F11816%2F6;p=osm%2Fcommon.git 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 --- 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 @@ 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 --- 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 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 @@ 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 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.