From 3403609f22352f079d09dba4809b316a5be49e7f Mon Sep 17 00:00:00 2001 From: Dan1elRoos Date: Wed, 25 Sep 2024 08:09:36 +0200 Subject: [PATCH] apply requested changes --- src/helper/filesystem.py | 4 +- ..._devices.py => import_devices_from_csv.py} | 45 +++--- ...ces.py => test_import_devices_from_csv.py} | 129 ++++++++++-------- .../test_helper/test_helper_filesystem.py | 8 +- 4 files changed, 103 insertions(+), 83 deletions(-) rename src/nac/management/commands/{import_devices.py => import_devices_from_csv.py} (85%) rename src/tests/test_commands/{test_import_devices.py => test_import_devices_from_csv.py} (76%) diff --git a/src/helper/filesystem.py b/src/helper/filesystem.py index 7f767de..6c1ee8b 100644 --- a/src/helper/filesystem.py +++ b/src/helper/filesystem.py @@ -28,5 +28,5 @@ def get_resources_directory(): return get_src_directory().parent / 'resources' -def get_absolute_path(path): - return get_src_directory().parent / path +def get_existing_path(path): + return Path(path) if Path(path).exists() else None diff --git a/src/nac/management/commands/import_devices.py b/src/nac/management/commands/import_devices_from_csv.py similarity index 85% rename from src/nac/management/commands/import_devices.py rename to src/nac/management/commands/import_devices_from_csv.py index 7de5e55..d7b4907 100644 --- a/src/nac/management/commands/import_devices.py +++ b/src/nac/management/commands/import_devices_from_csv.py @@ -1,16 +1,16 @@ import logging from os import stat from csv import DictReader, DictWriter -from django.core.management.base import BaseCommand +from django.core.management.base import BaseCommand, CommandError from django.db import transaction from django.core.exceptions import ValidationError, ObjectDoesNotExist from nac.models import Device, AuthorizationGroup, DeviceRoleProd, DeviceRoleInst from nac.forms import DeviceForm from helper.logging import setup_console_logger -from helper.filesystem import get_resources_directory, get_absolute_path +from helper.filesystem import get_resources_directory, get_existing_path -DEFAULT_SOURCE_CSV = get_resources_directory() / 'ldapObjects.csv' -CSV_SAVE_FILE = get_resources_directory() / "invalid_devices.csv" +DEFAULT_SOURCE_FILE = get_resources_directory() / 'ldapObjects.csv' +SAVE_FILE = get_resources_directory() / "invalid_devices.csv" class Command(BaseCommand): @@ -20,7 +20,7 @@ class Command(BaseCommand): def add_arguments(self, parser): parser.add_argument( '-f', '--csv_file', - default=DEFAULT_SOURCE_CSV, + default=DEFAULT_SOURCE_FILE, help='use a specific csv file [src/ldapObjects.csv]' ) parser.add_argument( @@ -31,12 +31,21 @@ def add_arguments(self, parser): def handle(self, *args, **options): setup_console_logger(options['verbosity']) - self.source_file = get_absolute_path(options['csv_file']) + self.source_file = get_existing_path(options['csv_file']) + + if not self.source_file: + logging.error( + f"The path '{options['csv_file']}'does not exist.") + raise CommandError( + f"The path '{options['csv_file']}' does not exist.") self.auth_group = self.check_valid_auth_group(options['auth_group']) if not self.auth_group: - return - self.clear_invalid_devices_file() + logging.error( + f"Invalid auth group '{options['auth_group']}'.") + raise CommandError( + f"Invalid auth group '{options['auth_group']}'.") + self.clear_invalid_devices_file() self.read_csv() def check_valid_auth_group(self, auth_group): @@ -48,11 +57,11 @@ def check_valid_auth_group(self, auth_group): def clear_invalid_devices_file(self): try: - with open(CSV_SAVE_FILE, "w"): - logging.info(f"Removing all entries in {CSV_SAVE_FILE}") + with open(SAVE_FILE, "w"): + logging.info(f"Removing all entries in {SAVE_FILE}") except Exception as e: logging.error( - f"Removing all entries in {CSV_SAVE_FILE} FAILED -> {e}" + f"Removing all entries in {SAVE_FILE} FAILED -> {e}" ) def read_csv(self): @@ -72,8 +81,8 @@ def handle_deviceObject(self, deviceObject): self.add_device_to_db(device) except ValidationError: self.save_invalid_devices(deviceObject) - except Exception: - pass + except Exception as e: + logging.error(f"Error: Handling device Object failed -> {e}") def check_device(self, deviceObject): logging.info(f"Checking validity of device" @@ -198,20 +207,20 @@ def add_device_to_db(self, deviceObject_valid): def save_invalid_devices(self, deviceObject_invalid): try: column_header = deviceObject_invalid.keys() - with open(CSV_SAVE_FILE, 'a', newline="") as csvfile: - logging.info(f"Writing invalid device to {CSV_SAVE_FILE}") + with open(SAVE_FILE, 'a', newline="") as csvfile: + logging.info(f"Writing invalid device to {SAVE_FILE}") writer = DictWriter( csvfile, fieldnames=column_header, delimiter=";" ) - if stat(CSV_SAVE_FILE).st_size == 0: + if stat(SAVE_FILE).st_size == 0: writer.writeheader() writer.writerows([deviceObject_invalid]) logging.debug( - f"Writing invalid device to {CSV_SAVE_FILE}: " + f"Writing invalid device to {SAVE_FILE}: " f"SUCCESSFUL" ) except Exception as e: logging.error( f"Writing invalid device to " - f"{CSV_SAVE_FILE}: FAILED -> {e}" + f"{SAVE_FILE}: FAILED -> {e}" ) diff --git a/src/tests/test_commands/test_import_devices.py b/src/tests/test_commands/test_import_devices_from_csv.py similarity index 76% rename from src/tests/test_commands/test_import_devices.py rename to src/tests/test_commands/test_import_devices_from_csv.py index 46dcaeb..4f5c807 100644 --- a/src/tests/test_commands/test_import_devices.py +++ b/src/tests/test_commands/test_import_devices_from_csv.py @@ -1,8 +1,9 @@ import pytest -from nac.management.commands.import_devices import Command, \ - DEFAULT_SOURCE_CSV, CSV_SAVE_FILE +from nac.management.commands.import_devices_from_csv import Command, \ + DEFAULT_SOURCE_FILE, SAVE_FILE from unittest.mock import patch, mock_open, MagicMock from django.core.exceptions import ValidationError +from django.core.management.base import CommandError from nac.models import AuthorizationGroup, DeviceRoleProd, DeviceRoleInst @@ -33,8 +34,8 @@ def command(): (True, True, "test", True, "001122334455", False, None, None) ] ) -@patch('nac.management.commands.import_devices.transaction.atomic') -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.transaction.atomic') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_check_device(mock_logging, mock_atomic, appl_NAC_ForceDot1X, appl_NAC_AllowAccessVPN, appl_NAC_Certificate, appl_NAC_AllowAccessAIR, @@ -79,10 +80,10 @@ def test_check_device(mock_logging, mock_atomic, appl_NAC_ForceDot1X, @pytest.mark.django_db -@patch('nac.management.commands.import_devices.transaction.atomic') -@patch('nac.management.commands.import_devices.Command.str_to_bool') -@patch('nac.management.commands.import_devices.logging') -@patch('nac.management.commands.import_devices.DeviceForm') +@patch('nac.management.commands.import_devices_from_csv.transaction.atomic') +@patch('nac.management.commands.import_devices_from_csv.Command.str_to_bool') +@patch('nac.management.commands.import_devices_from_csv.logging') +@patch('nac.management.commands.import_devices_from_csv.DeviceForm') def test_check_device_exceptions( mock_device_form, mock_logging, mock_str_to_bool, mock_atomic, command): @@ -187,8 +188,8 @@ def test_check_device_exceptions( @pytest.mark.django_db -@patch('nac.management.commands.import_devices.Device.objects.create') -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.Device.objects.create') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_add_device_to_db(mock_logging, mock_create, command): device = {"name": "test"} mock_create.return_value = None @@ -200,9 +201,9 @@ def test_add_device_to_db(mock_logging, mock_create, command): @pytest.mark.django_db -@patch('nac.management.commands.import_devices.transaction.atomic') -@patch('nac.management.commands.import_devices.Device.objects.create') -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.transaction.atomic') +@patch('nac.management.commands.import_devices_from_csv.Device.objects.create') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_add_device_to_db_exception(mock_logging, mock_create, mock_atomic, command): device = {"name": "test"} @@ -231,9 +232,9 @@ def test_str_to_bool(input_value, expected_output, command): @patch('builtins.open', new_callable=mock_open) -@patch('nac.management.commands.import_devices.DictWriter') -@patch('nac.management.commands.import_devices.stat') -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.DictWriter') +@patch('nac.management.commands.import_devices_from_csv.stat') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_save_invalid_devices_empty_file(mock_logging, mock_stat, mock_writer, mocked_file, command): device = {"name": "test"} @@ -241,7 +242,7 @@ def test_save_invalid_devices_empty_file(mock_logging, mock_stat, mock_writer_instance = MagicMock() mock_writer.return_value = mock_writer_instance command.save_invalid_devices(device) - mocked_file.assert_called_once_with(CSV_SAVE_FILE, + mocked_file.assert_called_once_with(SAVE_FILE, 'a', newline="") mock_writer.assert_called_once_with(mocked_file(), fieldnames=device.keys(), @@ -249,14 +250,14 @@ def test_save_invalid_devices_empty_file(mock_logging, mock_stat, mock_writer_instance.writeheader.assert_called_once() mock_writer_instance.writerows.assert_called_once_with([device]) mock_logging.info.assert_called_once_with( - f"Writing invalid device to {CSV_SAVE_FILE}") + f"Writing invalid device to {SAVE_FILE}") mock_logging.debug.assert_called_once_with( - f"Writing invalid device to {CSV_SAVE_FILE}: SUCCESSFUL") + f"Writing invalid device to {SAVE_FILE}: SUCCESSFUL") @patch('builtins.open', new_callable=mock_open) -@patch('nac.management.commands.import_devices.DictWriter') -@patch('nac.management.commands.import_devices.stat') +@patch('nac.management.commands.import_devices_from_csv.DictWriter') +@patch('nac.management.commands.import_devices_from_csv.stat') def test_save_invalid_devices_non_empty_file(mock_stat, mock_writer, mock_file, command): device = {"name": "test"} @@ -267,20 +268,20 @@ def test_save_invalid_devices_non_empty_file(mock_stat, mock_writer, mock_writer_instance.writeheader.assert_not_called() -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.logging') @patch('builtins.open', side_effect=Exception("Failed")) def test_save_invalid_devices_exception(mock_file, mock_logging, command): device = {"name": "test"} command.save_invalid_devices(device) mock_logging.error.assert_called_once_with( f"Writing invalid device to " - f"{CSV_SAVE_FILE}: FAILED -> Failed") + f"{SAVE_FILE}: FAILED -> Failed") @patch('builtins.open', new_callable=mock_open, read_data="name\n;test") -@patch('nac.management.commands.import_devices.DictReader') -@patch('nac.management.commands.import_devices.Command.handle_deviceObject') -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.DictReader') +@patch('nac.management.commands.import_devices_from_csv.Command.handle_deviceObject') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_read_csv(mock_logging, mock_handler, mock_reader, mock_file, command): command.source_file = "test.csv" mock_reader.return_value = [{"name": "test"}] @@ -295,9 +296,9 @@ def test_read_csv(mock_logging, mock_handler, mock_reader, mock_file, command): @patch('builtins.open', side_effect=Exception("Failed")) -@patch('nac.management.commands.import_devices.DictReader') -@patch('nac.management.commands.import_devices.Command.handle_deviceObject') -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.DictReader') +@patch('nac.management.commands.import_devices_from_csv.Command.handle_deviceObject') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_read_csv_exception(mock_logging, mock_handler, mock_reader, mock_file, command): command.source_file = "test.csv" @@ -310,30 +311,30 @@ def test_read_csv_exception(mock_logging, mock_handler, @patch('builtins.open', new_callable=mock_open) -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_clear_invalid_devices_file(mock_logging, mock_file, command): command.clear_invalid_devices_file() - mock_file.assert_called_once_with(CSV_SAVE_FILE, "w") + mock_file.assert_called_once_with(SAVE_FILE, "w") mock_logging.info.assert_called_once_with( - f"Removing all entries in {CSV_SAVE_FILE}") + f"Removing all entries in {SAVE_FILE}") @patch('builtins.open', side_effect=Exception("Failed")) -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_clear_invalid_devices_file_exception(mock_logging, mock_file, command): command.clear_invalid_devices_file() - mock_file.assert_called_once_with(CSV_SAVE_FILE, "w") + mock_file.assert_called_once_with(SAVE_FILE, "w") mock_logging.error.assert_called_once_with( - f"Removing all entries in {CSV_SAVE_FILE} FAILED -> Failed" + f"Removing all entries in {SAVE_FILE} FAILED -> Failed" ) @pytest.mark.django_db -@patch('nac.management.commands.import_devices.Command.check_device') -@patch('nac.management.commands.import_devices.Command.add_device_to_db') -@patch('nac.management.commands.import_devices.Command.save_invalid_devices') -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.Command.check_device') +@patch('nac.management.commands.import_devices_from_csv.Command.add_device_to_db') +@patch('nac.management.commands.import_devices_from_csv.Command.save_invalid_devices') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_handle_deviceObject( mock_logging, mock_save_invalid_devices, mock_add_device_to_db, mock_check_device, command): @@ -350,10 +351,10 @@ def test_handle_deviceObject( @pytest.mark.django_db -@patch('nac.management.commands.import_devices.Command.check_device') -@patch('nac.management.commands.import_devices.Command.add_device_to_db') -@patch('nac.management.commands.import_devices.Command.save_invalid_devices') -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.Command.check_device') +@patch('nac.management.commands.import_devices_from_csv.Command.add_device_to_db') +@patch('nac.management.commands.import_devices_from_csv.Command.save_invalid_devices') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_handle_deviceObject_invalid_Device( mock_logging, mock_save_invalid_devices, mock_add_device_to_db, mock_check_device, command): @@ -369,10 +370,10 @@ def test_handle_deviceObject_invalid_Device( @pytest.mark.django_db -@patch('nac.management.commands.import_devices.Command.check_device') -@patch('nac.management.commands.import_devices.Command.add_device_to_db') -@patch('nac.management.commands.import_devices.Command.save_invalid_devices') -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.Command.check_device') +@patch('nac.management.commands.import_devices_from_csv.Command.add_device_to_db') +@patch('nac.management.commands.import_devices_from_csv.Command.save_invalid_devices') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_handle_deviceObject_exception( mock_logging, mock_save_invalid_devices, mock_add_device_to_db, mock_check_device, command): @@ -380,7 +381,7 @@ def test_handle_deviceObject_exception( mock_check_device.side_effect = Exception("Invalid device") command.handle_deviceObject(device) - + mock_logging.error.assert_called_once_with("Error: Handling device Object failed -> Invalid device") mock_check_device.assert_called_once_with(device) mock_add_device_to_db.assert_not_called() mock_save_invalid_devices.assert_not_called() @@ -391,7 +392,7 @@ def test_add_arguments(command): command.add_arguments(mock_parser) mock_parser.add_argument.assert_any_call( '-f', '--csv_file', - default=DEFAULT_SOURCE_CSV, + default=DEFAULT_SOURCE_FILE, help='use a specific csv file [src/ldapObjects.csv]' ) mock_parser.add_argument.assert_any_call( @@ -402,31 +403,41 @@ def test_add_arguments(command): @pytest.mark.django_db -@patch('nac.management.commands.import_devices.Command.check_valid_auth_group') -@patch('nac.management.commands.import_devices.setup_console_logger') -@patch('nac.management.commands.import_devices.get_absolute_path') +@patch('nac.management.commands.import_devices_from_csv.Command.check_valid_auth_group') +@patch('nac.management.commands.import_devices_from_csv.setup_console_logger') +@patch('nac.management.commands.import_devices_from_csv.get_existing_path') @patch( - 'nac.management.commands.import_devices.Command.clear_invalid_devices_file') -@patch('nac.management.commands.import_devices.Command.read_csv') + 'nac.management.commands.import_devices_from_csv.Command.clear_invalid_devices_file') +@patch('nac.management.commands.import_devices_from_csv.Command.read_csv') def test_handle(mock_read_csv, mock_clear_invalid_devices_file, - mock_get_absolute_path, mock_setup_console_logger, + mock_get_existing_path, mock_setup_console_logger, mock_check_valid_auth_group, command): options = { 'verbosity': 0, 'csv_file': 'test.csv', 'auth_group': 'testag' } - mock_get_absolute_path.return_value = 'mockpath/to/test.csv' + mock_get_existing_path.return_value = 'mockpath/to/test.csv' command.handle(**options) mock_setup_console_logger.assert_called_once_with(0) mock_check_valid_auth_group.assert_called_once_with('testag') - mock_get_absolute_path.assert_called_once_with('test.csv') + mock_get_existing_path.assert_called_once_with('test.csv') mock_clear_invalid_devices_file.assert_called_once() mock_read_csv.assert_called_once() assert command.source_file == 'mockpath/to/test.csv' + mock_clear_invalid_devices_file.reset_mock() mock_check_valid_auth_group.return_value = None - command.handle(**options) + with pytest.raises(CommandError, + match="Invalid auth group 'testag'."): + command.handle(**options) + mock_clear_invalid_devices_file.assert_not_called() + + mock_clear_invalid_devices_file.reset_mock() + mock_get_existing_path.return_value = None + with pytest.raises(CommandError, + match="The path 'test.csv' does not exist."): + command.handle(**options) mock_clear_invalid_devices_file.assert_not_called() @@ -438,7 +449,7 @@ def test_check_valid_auth_group_exists(command): @pytest.mark.django_db -@patch('nac.management.commands.import_devices.logging') +@patch('nac.management.commands.import_devices_from_csv.logging') def test_check_valid_auth_group_not_exists(mock_logging, command): result = command.check_valid_auth_group('dummy') mock_logging.error.assert_called_once_with( diff --git a/src/tests/test_helper/test_helper_filesystem.py b/src/tests/test_helper/test_helper_filesystem.py index 8040d29..541ad3c 100644 --- a/src/tests/test_helper/test_helper_filesystem.py +++ b/src/tests/test_helper/test_helper_filesystem.py @@ -1,4 +1,4 @@ -from helper.filesystem import get_src_directory, get_config_directory, get_resources_directory, get_absolute_path +from helper.filesystem import get_src_directory, get_config_directory, get_resources_directory, get_existing_path def test_get_src_directory(): @@ -13,6 +13,6 @@ def test_get_resources_directory(): assert (get_resources_directory() / 'appl-NAC.schema').exists() -def test_get_absolute_path(): - path = "src/manage.py" - assert (get_absolute_path(path)).exists() +def test_get_existing_path(): + path = str(get_src_directory() / 'manage.py') + assert (get_existing_path(path)) is not None