diff --git a/samcli/commands/deploy/guided_config.py b/samcli/commands/deploy/guided_config.py index b9d8ea59b5..10b80189db 100644 --- a/samcli/commands/deploy/guided_config.py +++ b/samcli/commands/deploy/guided_config.py @@ -7,6 +7,7 @@ from samcli.cli.context import get_cmd_names from samcli.commands.deploy.exceptions import GuidedDeployFailedError +from samcli.lib.config.exceptions import SamConfigFileReadException from samcli.lib.config.samconfig import DEFAULT_CONFIG_FILE_NAME, DEFAULT_ENV, SamConfig @@ -26,13 +27,17 @@ def get_config_ctx(self, config_file=None): return ctx, samconfig def read_config_showcase(self, config_file=None): - _, samconfig = self.get_config_ctx(config_file) - - status = "Found" if samconfig.exists() else "Not found" msg = ( "Syntax invalid in samconfig.toml; save values " "through sam deploy --guided to overwrite file with a valid set of values." ) + try: + _, samconfig = self.get_config_ctx(config_file) + except SamConfigFileReadException: + raise GuidedDeployFailedError(msg) + + status = "Found" if samconfig.exists() else "Not found" + config_sanity = samconfig.sanity_check() click.secho("\nConfiguring SAM deploy\n======================", fg="yellow") click.echo(f"\n\tLooking for config file [{config_file}] : {status}") diff --git a/samcli/lib/config/exceptions.py b/samcli/lib/config/exceptions.py index 50297ce722..c179b4a13c 100644 --- a/samcli/lib/config/exceptions.py +++ b/samcli/lib/config/exceptions.py @@ -4,4 +4,12 @@ class SamConfigVersionException(Exception): - pass + """Exception for the `samconfig` file being not present or in unrecognized format""" + + +class FileParseException(Exception): + """Exception when a file is incorrectly parsed by a FileManager object.""" + + +class SamConfigFileReadException(Exception): + """Exception when a `samconfig` file is read incorrectly.""" diff --git a/samcli/lib/config/file_manager.py b/samcli/lib/config/file_manager.py new file mode 100644 index 0000000000..146946056f --- /dev/null +++ b/samcli/lib/config/file_manager.py @@ -0,0 +1,161 @@ +""" +Class to represent the parsing of different file types into Python objects. +""" + + +import logging +from abc import ABC, abstractmethod +from pathlib import Path +from typing import Any + +import tomlkit + +from samcli.lib.config.exceptions import FileParseException + +LOG = logging.getLogger(__name__) +COMMENT_KEY = "__comment__" + + +class FileManager(ABC): + """ + Abstract class to be overridden by file managers for specific file extensions. + """ + + @staticmethod + @abstractmethod + def read(filepath: Path) -> Any: + """ + Read a file at a given path. + + Parameters + ---------- + filepath: Path + The Path object that points to the file to be read. + + Returns + ------- + Any + A dictionary-like representation of the contents at the filepath location, along with a specialized + representation of the file that was read, if there is a specialization of it. + """ + raise NotImplementedError("Read method not implemented.") + + @staticmethod + @abstractmethod + def write(document: dict, filepath: Path): + """ + Write a dictionary or dictionary-like object to a given file. + + Parameters + ---------- + document: dict + The object to write. + filepath: Path + The final location for the document to be written. + """ + raise NotImplementedError("Write method not implemented.") + + @staticmethod + @abstractmethod + def put_comment(document: Any, comment: str) -> Any: + """ + Put a comment in a document object. + + Parameters + ---------- + document: Any + The object to write + comment: str + The comment to include in the document. + + Returns + ------- + Any + The new document, with the comment added to it. + """ + raise NotImplementedError("Put comment method not implemented.") + + +class TomlFileManager(FileManager): + """ + Static class to read and write toml files. + """ + + file_format = "TOML" + + @staticmethod + def read(filepath: Path) -> Any: + """ + Read a TOML file at the given path. + + Parameters + ---------- + filepath: Path + The Path object that points to the file to be read. + + Returns + ------- + Any + A dictionary-like tomlkit.TOMLDocument object, which represents the contents of the TOML file at the + provided location. + """ + toml_doc = tomlkit.document() + try: + txt = filepath.read_text() + toml_doc = tomlkit.loads(txt) + except OSError as e: + LOG.debug(f"OSError occurred while reading {TomlFileManager.file_format} file: {str(e)}") + except tomlkit.exceptions.TOMLKitError as e: + raise FileParseException(e) from e + + return toml_doc + + @staticmethod + def write(document: dict, filepath: Path): + """ + Write the contents of a dictionary or tomlkit.TOMLDocument to a TOML file at the provided location. + + Parameters + ---------- + document: dict + The object to write. + filepath: Path + The final location for the TOML file to be written. + """ + if not document: + LOG.debug("Nothing for TomlFileManager to write.") + return + + toml_document = TomlFileManager._to_toml(document) + + if toml_document.get(COMMENT_KEY, None): # Remove dunder comments that may be residue from other formats + toml_document.add(tomlkit.comment(toml_document[COMMENT_KEY])) + toml_document.pop(COMMENT_KEY) + + filepath.write_text(tomlkit.dumps(toml_document)) + + @staticmethod + def put_comment(document: dict, comment: str) -> Any: + """ + Put a comment in a document object. + + Parameters + ---------- + document: Any + The tomlkit.TOMLDocument object to write + comment: str + The comment to include in the document. + + Returns + ------- + Any + The new TOMLDocument, with the comment added to it. + """ + document = TomlFileManager._to_toml(document) + document.add(tomlkit.comment(comment)) + return document + + @staticmethod + def _to_toml(document: dict) -> tomlkit.TOMLDocument: + """Ensure that a dictionary-like object is a TOMLDocument.""" + return tomlkit.parse(tomlkit.dumps(document)) diff --git a/samcli/lib/config/samconfig.py b/samcli/lib/config/samconfig.py index e48e53d625..d70b8fe1bb 100644 --- a/samcli/lib/config/samconfig.py +++ b/samcli/lib/config/samconfig.py @@ -5,27 +5,28 @@ import logging import os from pathlib import Path -from typing import Any, Iterable +from typing import Any, Dict, Iterable, Type -import tomlkit - -from samcli.lib.config.exceptions import SamConfigVersionException +from samcli.lib.config.exceptions import FileParseException, SamConfigFileReadException, SamConfigVersionException +from samcli.lib.config.file_manager import FileManager, TomlFileManager from samcli.lib.config.version import SAM_CONFIG_VERSION, VERSION_KEY LOG = logging.getLogger(__name__) -DEFAULT_CONFIG_FILE_EXTENSION = "toml" -DEFAULT_CONFIG_FILE_NAME = f"samconfig.{DEFAULT_CONFIG_FILE_EXTENSION}" +DEFAULT_CONFIG_FILE_EXTENSION = ".toml" +DEFAULT_CONFIG_FILE_NAME = f"samconfig{DEFAULT_CONFIG_FILE_EXTENSION}" DEFAULT_ENV = "default" DEFAULT_GLOBAL_CMDNAME = "global" class SamConfig: """ - Class to interface with `samconfig.toml` file. + Class to represent `samconfig` config options. """ - document = None + FILE_MANAGER_MAPPER: Dict[str, Type[FileManager]] = { + ".toml": TomlFileManager, + } def __init__(self, config_dir, filename=None): """ @@ -39,11 +40,21 @@ def __init__(self, config_dir, filename=None): Optional. Name of the configuration file. It is recommended to stick with default so in the future we could automatically support auto-resolving multiple config files within same directory. """ + self.document = {} self.filepath = Path(config_dir, filename or DEFAULT_CONFIG_FILE_NAME) + self.file_manager = self.FILE_MANAGER_MAPPER.get(self.filepath.suffix, None) + if not self.file_manager: + LOG.warning( + f"The config file extension '{self.filepath.suffix}' is not supported. " + f"Supported formats are: [{'|'.join(self.FILE_MANAGER_MAPPER.keys())}]" + ) + raise SamConfigFileReadException( + f"The config file {self.filepath} uses an unsupported extension, and cannot be read." + ) + self._read() def get_stage_configuration_names(self): - self._read() - if isinstance(self.document, dict): + if self.document: return [stage for stage, value in self.document.items() if isinstance(value, dict)] return [] @@ -69,23 +80,19 @@ def get_all(self, cmd_names, section, env=DEFAULT_ENV): ------ KeyError If the config file does *not* have the specific section - - tomlkit.exceptions.TOMLKitError - If the configuration file is invalid """ env = env or DEFAULT_ENV - self._read() - if isinstance(self.document, dict): - toml_content = self.document.get(env, {}) - params = toml_content.get(self._to_key(cmd_names), {}).get(section, {}) - if DEFAULT_GLOBAL_CMDNAME in toml_content: - global_params = toml_content.get(DEFAULT_GLOBAL_CMDNAME, {}).get(section, {}) - global_params.update(params.copy()) - params = global_params.copy() - return params - return {} + self.document = self._read() + + config_content = self.document.get(env, {}) + params = config_content.get(self._to_key(cmd_names), {}).get(section, {}) + if DEFAULT_GLOBAL_CMDNAME in config_content: + global_params = config_content.get(DEFAULT_GLOBAL_CMDNAME, {}).get(section, {}) + global_params.update(params.copy()) + params = global_params.copy() + return params def put(self, cmd_names, section, key, value, env=DEFAULT_ENV): """ @@ -102,20 +109,10 @@ def put(self, cmd_names, section, key, value, env=DEFAULT_ENV): key : str Key to write the data under value : Any - Value to write. Could be any of the supported TOML types. + Value to write. Could be any of the supported types. env : str Optional, Name of the environment - - Raises - ------ - tomlkit.exceptions.TOMLKitError - If the data is invalid """ - - if self.document is None: - # Checking for None here since a TOMLDocument can include a - # 'body' property but still be falsy without a 'value' property - self._read() # Empty document prepare the initial structure. # self.document is a nested dict, we need to check each layer and add new tables, otherwise duplicated key # in parent layer will override the whole child layer @@ -144,20 +141,12 @@ def put_comment(self, comment): comment: str A comment to write to the samconfg file """ - if self.document is None: - self._read() - self.document.add(tomlkit.comment(comment)) + self.document = self.file_manager.put_comment(self.document, comment) def flush(self): """ Write the data back to file - - Raises - ------ - tomlkit.exceptions.TOMLKitError - If the data is invalid - """ self._write() @@ -167,7 +156,7 @@ def sanity_check(self): """ try: self._read() - except tomlkit.exceptions.TOMLKitError: + except SamConfigFileReadException: return False else: return True @@ -196,13 +185,10 @@ def config_dir(template_file_path=None): def _read(self): if not self.document: try: - txt = self.filepath.read_text() - self.document = tomlkit.loads(txt) - self._version_sanity_check(self._version()) - except OSError: - self.document = tomlkit.document() - - if self.document.body: + self.document = self.file_manager.read(self.filepath) + except FileParseException as e: + raise SamConfigFileReadException(e) from e + if self.document: self._version_sanity_check(self._version()) return self.document @@ -213,12 +199,9 @@ def _write(self): self._ensure_exists() current_version = self._version() if self._version() else SAM_CONFIG_VERSION - try: - self.document.add(VERSION_KEY, current_version) - except tomlkit.exceptions.KeyAlreadyPresent: - # NOTE(TheSriram): Do not attempt to re-write an existing version - pass - self.filepath.write_text(tomlkit.dumps(self.document)) + self.document.update({VERSION_KEY: current_version}) + + self.file_manager.write(self.document, self.filepath) def _version(self): return self.document.get(VERSION_KEY, None) diff --git a/tests/unit/cli/test_cli_config_file.py b/tests/unit/cli/test_cli_config_file.py index 606e0a004e..3395c12786 100644 --- a/tests/unit/cli/test_cli_config_file.py +++ b/tests/unit/cli/test_cli_config_file.py @@ -5,9 +5,12 @@ from unittest import TestCase, skipIf from unittest.mock import MagicMock, patch +import tomlkit + from samcli.commands.exceptions import ConfigException from samcli.cli.cli_config_file import TomlProvider, configuration_option, configuration_callback, get_ctx_defaults -from samcli.lib.config.samconfig import DEFAULT_ENV +from samcli.lib.config.exceptions import FileParseException, SamConfigFileReadException, SamConfigVersionException +from samcli.lib.config.samconfig import DEFAULT_ENV, TomlFileManager from tests.testing_utils import IS_WINDOWS @@ -40,14 +43,14 @@ def test_toml_valid_with_no_version(self): config_dir = tempfile.gettempdir() config_path = Path(config_dir, "samconfig.toml") config_path.write_text("[config_env.topic.parameters]\nword='clarity'\n") - with self.assertRaises(ConfigException): + with self.assertRaises(SamConfigVersionException): TomlProvider(section=self.parameters)(config_path, self.config_env, [self.cmd_name]) def test_toml_valid_with_invalid_version(self): config_dir = tempfile.gettempdir() config_path = Path(config_dir, "samconfig.toml") config_path.write_text("version='abc'\n[config_env.topic.parameters]\nword='clarity'\n") - with self.assertRaises(ConfigException): + with self.assertRaises(SamConfigVersionException): TomlProvider(section=self.parameters)(config_path, self.config_env, [self.cmd_name]) def test_toml_invalid_empty_dict(self): @@ -55,7 +58,8 @@ def test_toml_invalid_empty_dict(self): config_path = Path(config_dir, "samconfig.toml") config_path.write_text("[topic]\nword=clarity\n") - self.assertEqual(self.toml_provider(config_dir, self.config_env, [self.cmd_name]), {}) + with self.assertRaises(SamConfigFileReadException): + self.toml_provider(config_path, self.config_env, [self.cmd_name]) def test_toml_invalid_file_name(self): config_dir = tempfile.gettempdir() @@ -63,7 +67,7 @@ def test_toml_invalid_file_name(self): config_path.write_text("version=0.1\n[config_env.topic.parameters]\nword='clarity'\n") config_path_invalid = Path(config_dir, "samconfig.toml") - with self.assertRaises(ConfigException): + with self.assertRaises(SamConfigFileReadException): self.toml_provider(config_path_invalid, self.config_env, [self.cmd_name]) def test_toml_invalid_syntax(self): @@ -71,7 +75,7 @@ def test_toml_invalid_syntax(self): config_path = Path(config_dir, "samconfig.toml") config_path.write_text("version=0.1\n[config_env.topic.parameters]\nword=_clarity'\n") - with self.assertRaises(ConfigException): + with self.assertRaises(SamConfigFileReadException): self.toml_provider(config_path, self.config_env, [self.cmd_name]) diff --git a/tests/unit/commands/samconfig/test_samconfig.py b/tests/unit/commands/samconfig/test_samconfig.py index 1ff1217138..c9d7e442fc 100644 --- a/tests/unit/commands/samconfig/test_samconfig.py +++ b/tests/unit/commands/samconfig/test_samconfig.py @@ -8,14 +8,15 @@ import tempfile from pathlib import Path from contextlib import contextmanager -from samcli.lib.config.samconfig import SamConfig, DEFAULT_ENV from click.testing import CliRunner from unittest import TestCase from unittest.mock import patch, ANY import logging +from samcli.lib.config.exceptions import SamConfigFileReadException +from samcli.lib.config.samconfig import SamConfig, DEFAULT_ENV, TomlFileManager from samcli.lib.utils.packagetype import ZIP, IMAGE LOG = logging.getLogger() @@ -1245,6 +1246,23 @@ def test_secondary_option_name_template_validate(self, do_cli_mock): do_cli_mock.assert_called_with(ANY, str(Path(os.getcwd(), "mytemplate.yaml")), False) +class TestSamConfigFileManager(TestCase): + def test_file_manager_not_declared(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig") + + with self.assertRaises(SamConfigFileReadException): + SamConfig(config_path, filename="samconfig") + + def test_file_manager_toml(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + + samconfig = SamConfig(config_path, filename="samconfig.toml") + + self.assertIs(samconfig.file_manager, TomlFileManager) + + @contextmanager def samconfig_parameters(cmd_names, config_dir=None, env=None, **kwargs): """ diff --git a/tests/unit/lib/samconfig/test_file_manager.py b/tests/unit/lib/samconfig/test_file_manager.py new file mode 100644 index 0000000000..0973637023 --- /dev/null +++ b/tests/unit/lib/samconfig/test_file_manager.py @@ -0,0 +1,101 @@ +from pathlib import Path +import tempfile +from unittest import TestCase + +import tomlkit +from samcli.lib.config.exceptions import FileParseException + +from samcli.lib.config.file_manager import COMMENT_KEY, TomlFileManager + + +class TestTomlFileManager(TestCase): + def test_read_toml(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + config_path.write_text("version=0.1\n[config_env.topic1.parameters]\nword='clarity'\n") + config_doc = TomlFileManager.read(config_path) + self.assertEqual( + config_doc, + {"version": 0.1, "config_env": {"topic1": {"parameters": {"word": "clarity"}}}}, + ) + + def test_read_toml_invalid_toml(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + config_path.write_text("fake='not real'\nimproper toml file\n") + with self.assertRaises(FileParseException): + TomlFileManager.read(config_path) + + def test_read_toml_file_path_not_valid(self): + config_dir = "path/that/doesnt/exist" + config_path = Path(config_dir, "samconfig.toml") + config_doc = TomlFileManager.read(config_path) + self.assertEqual(config_doc, tomlkit.document()) + + def test_write_toml(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + toml = { + "version": 0.1, + "config_env": {"topic2": {"parameters": {"word": "clarity"}}}, + COMMENT_KEY: "This is a comment", + } + + TomlFileManager.write(toml, config_path) + + txt = config_path.read_text() + self.assertIn("version = 0.1", txt) + self.assertIn("[config_env.topic2.parameters]", txt) + self.assertIn('word = "clarity"', txt) + self.assertIn("# This is a comment", txt) + + def test_dont_write_toml_if_empty(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + config_path.write_text("nothing to see here\n") + toml = {} + + TomlFileManager.write(toml, config_path) + + self.assertEqual(config_path.read_text(), "nothing to see here\n") + + def test_write_toml_bad_path(self): + config_path = Path("path/to/some", "file_that_doesnt_exist.toml") + with self.assertRaises(FileNotFoundError): + TomlFileManager.write({"key": "some value"}, config_path) + + def test_write_toml_file(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + toml = tomlkit.parse('# This is a comment\nversion = 0.1\n[config_env.topic2.parameters]\nword = "clarity"\n') + + TomlFileManager.write(toml, config_path) + + txt = config_path.read_text() + self.assertIn("version = 0.1", txt) + self.assertIn("[config_env.topic2.parameters]", txt) + self.assertIn('word = "clarity"', txt) + self.assertIn("# This is a comment", txt) + + def test_dont_write_toml_file_if_empty(self): + config_dir = tempfile.gettempdir() + config_path = Path(config_dir, "samconfig.toml") + config_path.write_text("nothing to see here\n") + toml = tomlkit.document() + + TomlFileManager.write(toml, config_path) + + self.assertEqual(config_path.read_text(), "nothing to see here\n") + + def test_write_toml_file_bad_path(self): + config_path = Path("path/to/some", "file_that_doesnt_exist.toml") + with self.assertRaises(FileNotFoundError): + TomlFileManager.write(tomlkit.parse('key = "some value"'), config_path) + + def test_toml_put_comment(self): + toml_doc = tomlkit.loads('version = 0.1\n[config_env.topic2.parameters]\nword = "clarity"\n') + + toml_doc = TomlFileManager.put_comment(toml_doc, "This is a comment") + + txt = tomlkit.dumps(toml_doc) + self.assertIn("# This is a comment", txt) diff --git a/tests/unit/lib/samconfig/test_samconfig.py b/tests/unit/lib/samconfig/test_samconfig.py index 7a86e6f97d..346353501b 100644 --- a/tests/unit/lib/samconfig/test_samconfig.py +++ b/tests/unit/lib/samconfig/test_samconfig.py @@ -182,27 +182,27 @@ def test_check_sanity(self): def test_check_version_non_supported_type(self): self._setup_config() - self.samconfig.document.remove(VERSION_KEY) - self.samconfig.document.add(VERSION_KEY, "aadeff") + self.samconfig.document.pop(VERSION_KEY) + self.samconfig.document.update({VERSION_KEY: "aadeff"}) with self.assertRaises(SamConfigVersionException): self.samconfig.sanity_check() def test_check_version_no_version_exists(self): self._setup_config() - self.samconfig.document.remove(VERSION_KEY) + self.samconfig.document.pop(VERSION_KEY) with self.assertRaises(SamConfigVersionException): self.samconfig.sanity_check() def test_check_version_float(self): self._setup_config() - self.samconfig.document.remove(VERSION_KEY) - self.samconfig.document.add(VERSION_KEY, 0.2) + self.samconfig.document.pop(VERSION_KEY) + self.samconfig.document.update({VERSION_KEY: 0.2}) self.samconfig.sanity_check() def test_write_config_file_non_standard_version(self): self._setup_config() - self.samconfig.document.remove(VERSION_KEY) - self.samconfig.document.add(VERSION_KEY, 0.2) + self.samconfig.document.pop(VERSION_KEY) + self.samconfig.document.update({VERSION_KEY: 0.2}) self.samconfig.put(cmd_names=["local", "start", "api"], section="parameters", key="skip_pull_image", value=True) self.samconfig.sanity_check() self.assertEqual(self.samconfig.document.get(VERSION_KEY), 0.2) @@ -210,7 +210,7 @@ def test_write_config_file_non_standard_version(self): def test_write_config_file_will_create_the_file_if_not_exist(self): with osutils.mkdir_temp(ignore_errors=True) as tempdir: non_existing_dir = os.path.join(tempdir, "non-existing-dir") - non_existing_file = "non-existing-file" + non_existing_file = "non-existing-file.toml" samconfig = SamConfig(config_dir=non_existing_dir, filename=non_existing_file) self.assertFalse(samconfig.exists())