diff --git a/sanic/config.py b/sanic/config.py index 506233bb65..8f653c6ef5 100644 --- a/sanic/config.py +++ b/sanic/config.py @@ -1,14 +1,8 @@ +from inspect import isclass from os import environ +from pathlib import Path from typing import Any, Union -# NOTE(tomaszdrozdz): remove in version: 21.3 -# We replace from_envvar(), from_object(), from_pyfile() config object methods -# with one simpler update_config() method. -# We also replace "loading module from file code" in from_pyfile() -# in a favour of load_module_from_file_location(). -# Please see pull request: 1903 -# and issue: 1895 -from .deprecated import from_envvar, from_object, from_pyfile # noqa from .utils import load_module_from_file_location, str_to_bool @@ -69,17 +63,6 @@ def __getattr__(self, attr): def __setattr__(self, attr, value): self[attr] = value - # NOTE(tomaszdrozdz): remove in version: 21.3 - # We replace from_envvar(), from_object(), from_pyfile() config object - # methods with one simpler update_config() method. - # We also replace "loading module from file code" in from_pyfile() - # in a favour of load_module_from_file_location(). - # Please see pull request: 1903 - # and issue: 1895 - from_envvar = from_envvar - from_pyfile = from_pyfile - from_object = from_object - def load_environment_vars(self, prefix=SANIC_PREFIX): """ Looks for prefixed environment variables and applies @@ -100,18 +83,23 @@ def load_environment_vars(self, prefix=SANIC_PREFIX): self[config_key] = v def update_config(self, config: Union[bytes, str, dict, Any]): - """Update app.config. + """ + Update app.config. Note:: only upper case settings are considered. You can upload app config by providing path to py file holding settings. + .. code-block:: python + # /some/py/file A = 1 B = 2 - config.update_config("${some}/py/file") + .. code-block:: python + + config.update_config("${some}/py/file") Yes you can put environment variable here, but they must be provided in format: ${some_env_var}, and mark that $some_env_var is treated @@ -119,23 +107,41 @@ def update_config(self, config: Union[bytes, str, dict, Any]): You can upload app config by providing dict holding settings. + .. code-block:: python + d = {"A": 1, "B": 2} config.update_config(d) You can upload app config by providing any object holding settings, but in such case config.__dict__ will be used as dict holding settings. + .. code-block:: python + class C: A = 1 B = 2 - config.update_config(C)""" - if isinstance(config, (bytes, str)): + config.update_config(C) + """ + + if isinstance(config, (bytes, str, Path)): config = load_module_from_file_location(location=config) if not isinstance(config, dict): - config = config.__dict__ + cfg = {} + if not isclass(config): + cfg.update( + { + key: getattr(config, key) + for key in config.__class__.__dict__.keys() + } + ) + + config = dict(config.__dict__) + config.update(cfg) config = dict(filter(lambda i: i[0].isupper(), config.items())) self.update(config) + + load = update_config diff --git a/sanic/cookies.py b/sanic/cookies.py index ed672fba1a..5387fcc540 100644 --- a/sanic/cookies.py +++ b/sanic/cookies.py @@ -109,7 +109,7 @@ def __setitem__(self, key, value): if value is not False: if key.lower() == "max-age": if not str(value).isdigit(): - value = DEFAULT_MAX_AGE + raise ValueError("Cookie max-age must be an integer") elif key.lower() == "expires": if not isinstance(value, datetime): raise TypeError( diff --git a/sanic/deprecated.py b/sanic/deprecated.py deleted file mode 100644 index c8c95be00b..0000000000 --- a/sanic/deprecated.py +++ /dev/null @@ -1,106 +0,0 @@ -# NOTE(tomaszdrozdz): remove in version: 21.3 -# We replace from_envvar(), from_object(), from_pyfile() config object methods -# with one simpler update_config() method. -# We also replace "loading module from file code" in from_pyfile() -# in a favour of load_module_from_file_location(). -# Please see pull request: 1903 -# and issue: 1895 -import types - -from os import environ -from typing import Any -from warnings import warn - -from sanic.exceptions import PyFileError -from sanic.helpers import import_string - - -def from_envvar(self, variable_name: str) -> bool: - """Load a configuration from an environment variable pointing to - a configuration file. - - :param variable_name: name of the environment variable - :return: bool. ``True`` if able to load config, ``False`` otherwise. - """ - - warn( - "Using `from_envvar` method is deprecated and will be removed in " - "v21.3, use `app.update_config` method instead.", - DeprecationWarning, - stacklevel=2, - ) - - config_file = environ.get(variable_name) - if not config_file: - raise RuntimeError( - f"The environment variable {variable_name} is not set and " - f"thus configuration could not be loaded." - ) - return self.from_pyfile(config_file) - - -def from_pyfile(self, filename: str) -> bool: - """Update the values in the config from a Python file. - Only the uppercase variables in that module are stored in the config. - - :param filename: an absolute path to the config file - """ - - warn( - "Using `from_pyfile` method is deprecated and will be removed in " - "v21.3, use `app.update_config` method instead.", - DeprecationWarning, - stacklevel=2, - ) - - module = types.ModuleType("config") - module.__file__ = filename - try: - with open(filename) as config_file: - exec( # nosec - compile(config_file.read(), filename, "exec"), - module.__dict__, - ) - except IOError as e: - e.strerror = "Unable to load configuration file (e.strerror)" - raise - except Exception as e: - raise PyFileError(filename) from e - - self.from_object(module) - return True - - -def from_object(self, obj: Any) -> None: - """Update the values from the given object. - Objects are usually either modules or classes. - - Just the uppercase variables in that object are stored in the config. - Example usage:: - - from yourapplication import default_config - app.config.from_object(default_config) - - or also: - app.config.from_object('myproject.config.MyConfigClass') - - You should not use this function to load the actual configuration but - rather configuration defaults. The actual config should be loaded - with :meth:`from_pyfile` and ideally from a location not within the - package because the package might be installed system wide. - - :param obj: an object holding the configuration - """ - - warn( - "Using `from_object` method is deprecated and will be removed in " - "v21.3, use `app.update_config` method instead.", - DeprecationWarning, - stacklevel=2, - ) - - if isinstance(obj, str): - obj = import_string(obj) - for key in dir(obj): - if key.isupper(): - self[key] = getattr(obj, key) diff --git a/sanic/utils.py b/sanic/utils.py index e34a8b66a9..dce6161900 100644 --- a/sanic/utils.py +++ b/sanic/utils.py @@ -1,9 +1,13 @@ +import types + from importlib.util import module_from_spec, spec_from_file_location from os import environ as os_environ +from pathlib import Path from re import findall as re_findall from typing import Union -from .exceptions import LoadFileException +from sanic.exceptions import LoadFileException, PyFileError +from sanic.helpers import import_string def str_to_bool(val: str) -> bool: @@ -39,7 +43,7 @@ def str_to_bool(val: str) -> bool: def load_module_from_file_location( - location: Union[bytes, str], encoding: str = "utf8", *args, **kwargs + location: Union[bytes, str, Path], encoding: str = "utf8", *args, **kwargs ): """Returns loaded module provided as a file path. @@ -67,33 +71,61 @@ def load_module_from_file_location( "/some/path/${some_env_var}" ) """ - - # 1) Parse location. if isinstance(location, bytes): location = location.decode(encoding) - # A) Check if location contains any environment variables - # in format ${some_env_var}. - env_vars_in_location = set(re_findall(r"\${(.+?)}", location)) - - # B) Check these variables exists in environment. - not_defined_env_vars = env_vars_in_location.difference(os_environ.keys()) - if not_defined_env_vars: - raise LoadFileException( - "The following environment variables are not set: " - f"{', '.join(not_defined_env_vars)}" - ) - - # C) Substitute them in location. - for env_var in env_vars_in_location: - location = location.replace("${" + env_var + "}", os_environ[env_var]) - - # 2) Load and return module. - name = location.split("/")[-1].split(".")[ - 0 - ] # get just the file name without path and .py extension - _mod_spec = spec_from_file_location(name, location, *args, **kwargs) - module = module_from_spec(_mod_spec) - _mod_spec.loader.exec_module(module) # type: ignore - - return module + if isinstance(location, Path) or "/" in location or "$" in location: + + if not isinstance(location, Path): + # A) Check if location contains any environment variables + # in format ${some_env_var}. + env_vars_in_location = set(re_findall(r"\${(.+?)}", location)) + + # B) Check these variables exists in environment. + not_defined_env_vars = env_vars_in_location.difference( + os_environ.keys() + ) + if not_defined_env_vars: + raise LoadFileException( + "The following environment variables are not set: " + f"{', '.join(not_defined_env_vars)}" + ) + + # C) Substitute them in location. + for env_var in env_vars_in_location: + location = location.replace( + "${" + env_var + "}", os_environ[env_var] + ) + + location = str(location) + if ".py" in location: + name = location.split("/")[-1].split(".")[ + 0 + ] # get just the file name without path and .py extension + _mod_spec = spec_from_file_location( + name, location, *args, **kwargs + ) + module = module_from_spec(_mod_spec) + _mod_spec.loader.exec_module(module) # type: ignore + + else: + module = types.ModuleType("config") + module.__file__ = str(location) + try: + with open(location) as config_file: + exec( # nosec + compile(config_file.read(), location, "exec"), + module.__dict__, + ) + except IOError as e: + e.strerror = "Unable to load configuration file (e.strerror)" + raise + except Exception as e: + raise PyFileError(location) from e + + return module + else: + try: + return import_string(location) + except ValueError: + raise IOError("Unable to load configuration %s" % str(location)) diff --git a/setup.py b/setup.py index fc19e20a5c..02649b57fb 100644 --- a/setup.py +++ b/setup.py @@ -104,7 +104,6 @@ def open_local(paths, mode="r", encoding="utf8"): "pytest-sanic", "pytest-sugar", "pytest-benchmark", - "pytest-dependency", ] docs_require = [ diff --git a/tests/test_config.py b/tests/test_config.py index 7c232d7556..2a54aa9f68 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -22,24 +22,41 @@ class ConfigTest: not_for_config = "should not be used" CONFIG_VALUE = "should be used" + @property + def ANOTHER_VALUE(self): + return self.CONFIG_VALUE + + @property + def another_not_for_config(self): + return self.not_for_config + def test_load_from_object(app): - app.config.from_object(ConfigTest) + app.config.load(ConfigTest) assert "CONFIG_VALUE" in app.config assert app.config.CONFIG_VALUE == "should be used" assert "not_for_config" not in app.config def test_load_from_object_string(app): - app.config.from_object("test_config.ConfigTest") + app.config.load("test_config.ConfigTest") + assert "CONFIG_VALUE" in app.config + assert app.config.CONFIG_VALUE == "should be used" + assert "not_for_config" not in app.config + + +def test_load_from_instance(app): + app.config.load(ConfigTest()) assert "CONFIG_VALUE" in app.config assert app.config.CONFIG_VALUE == "should be used" + assert app.config.ANOTHER_VALUE == "should be used" assert "not_for_config" not in app.config + assert "another_not_for_config" not in app.config def test_load_from_object_string_exception(app): with pytest.raises(ImportError): - app.config.from_object("test_config.Config.test") + app.config.load("test_config.Config.test") def test_auto_load_env(): @@ -52,7 +69,7 @@ def test_auto_load_env(): def test_auto_load_bool_env(): environ["SANIC_TEST_ANSWER"] = "True" app = Sanic(name=__name__) - assert app.config.TEST_ANSWER == True + assert app.config.TEST_ANSWER is True del environ["SANIC_TEST_ANSWER"] @@ -95,7 +112,7 @@ def test_load_from_file(app): ) with temp_path() as config_path: config_path.write_text(config) - app.config.from_pyfile(str(config_path)) + app.config.load(str(config_path)) assert "VALUE" in app.config assert app.config.VALUE == "some value" assert "CONDITIONAL" in app.config @@ -105,7 +122,7 @@ def test_load_from_file(app): def test_load_from_missing_file(app): with pytest.raises(IOError): - app.config.from_pyfile("non-existent file") + app.config.load("non-existent file") def test_load_from_envvar(app): @@ -113,14 +130,14 @@ def test_load_from_envvar(app): with temp_path() as config_path: config_path.write_text(config) environ["APP_CONFIG"] = str(config_path) - app.config.from_envvar("APP_CONFIG") + app.config.load("${APP_CONFIG}") assert "VALUE" in app.config assert app.config.VALUE == "some value" def test_load_from_missing_envvar(app): - with pytest.raises(RuntimeError) as e: - app.config.from_envvar("non-existent variable") + with pytest.raises(IOError) as e: + app.config.load("non-existent variable") assert str(e.value) == ( "The environment variable 'non-existent " "variable' is not set and thus configuration " @@ -134,7 +151,7 @@ def test_load_config_from_file_invalid_syntax(app): config_path.write_text(config) with pytest.raises(PyFileError): - app.config.from_pyfile(config_path) + app.config.load(config_path) def test_overwrite_exisiting_config(app): @@ -143,7 +160,7 @@ def test_overwrite_exisiting_config(app): class Config: DEFAULT = 2 - app.config.from_object(Config) + app.config.load(Config) assert app.config.DEFAULT == 2 @@ -153,14 +170,12 @@ def test_overwrite_exisiting_config_ignore_lowercase(app): class Config: default = 2 - app.config.from_object(Config) + app.config.load(Config) assert app.config.default == 1 def test_missing_config(app): - with pytest.raises( - AttributeError, match="Config has no 'NON_EXISTENT'" - ) as e: + with pytest.raises(AttributeError, match="Config has no 'NON_EXISTENT'"): _ = app.config.NON_EXISTENT @@ -175,7 +190,8 @@ def test_config_defaults(): def test_config_custom_defaults(): """ - we should have all the variables from defaults rewriting them with custom defaults passed in + we should have all the variables from defaults rewriting them with + custom defaults passed in Config """ custom_defaults = { @@ -192,7 +208,8 @@ def test_config_custom_defaults(): def test_config_custom_defaults_with_env(): """ - test that environment variables has higher priority than DEFAULT_CONFIG and passed defaults dict + test that environment variables has higher priority than DEFAULT_CONFIG + and passed defaults dict """ custom_defaults = { "REQUEST_MAX_SIZE123": 1, @@ -226,22 +243,22 @@ def test_config_custom_defaults_with_env(): def test_config_access_log_passing_in_run(app): - assert app.config.ACCESS_LOG == True + assert app.config.ACCESS_LOG is True @app.listener("after_server_start") async def _request(sanic, loop): app.stop() app.run(port=1340, access_log=False) - assert app.config.ACCESS_LOG == False + assert app.config.ACCESS_LOG is False app.run(port=1340, access_log=True) - assert app.config.ACCESS_LOG == True + assert app.config.ACCESS_LOG is True @pytest.mark.asyncio async def test_config_access_log_passing_in_create_server(app): - assert app.config.ACCESS_LOG == True + assert app.config.ACCESS_LOG is True @app.listener("after_server_start") async def _request(sanic, loop): @@ -250,24 +267,51 @@ async def _request(sanic, loop): await app.create_server( port=1341, access_log=False, return_asyncio_server=True ) - assert app.config.ACCESS_LOG == False + assert app.config.ACCESS_LOG is False await app.create_server( port=1342, access_log=True, return_asyncio_server=True ) - assert app.config.ACCESS_LOG == True + assert app.config.ACCESS_LOG is True def test_config_rewrite_keep_alive(): config = Config() assert config.KEEP_ALIVE == DEFAULT_CONFIG["KEEP_ALIVE"] config = Config(keep_alive=True) - assert config.KEEP_ALIVE == True + assert config.KEEP_ALIVE is True config = Config(keep_alive=False) - assert config.KEEP_ALIVE == False + assert config.KEEP_ALIVE is False # use defaults config = Config(defaults={"KEEP_ALIVE": False}) - assert config.KEEP_ALIVE == False + assert config.KEEP_ALIVE is False config = Config(defaults={"KEEP_ALIVE": True}) - assert config.KEEP_ALIVE == True + assert config.KEEP_ALIVE is True + + +_test_setting_as_dict = {"TEST_SETTING_VALUE": 1} +_test_setting_as_class = type("C", (), {"TEST_SETTING_VALUE": 1}) +_test_setting_as_module = str( + Path(__file__).parent / "static/app_test_config.py" +) + + +@pytest.mark.parametrize( + "conf_object", + [ + _test_setting_as_dict, + _test_setting_as_class, + _test_setting_as_module, + ], + ids=["from_dict", "from_class", "from_file"], +) +def test_update(app, conf_object): + app.update_config(conf_object) + assert app.config["TEST_SETTING_VALUE"] == 1 + + +def test_update_from_lowercase_key(app): + d = {"test_setting_value": 1} + app.update_config(d) + assert "test_setting_value" not in app.config diff --git a/tests/test_cookies.py b/tests/test_cookies.py index 1c29c551aa..22ce938730 100644 --- a/tests/test_cookies.py +++ b/tests/test_cookies.py @@ -162,7 +162,7 @@ def handler(request): assert response.cookies["test"] == "pass" -@pytest.mark.parametrize("max_age", ["0", 30, 30.0, 30.1, "30", "test"]) +@pytest.mark.parametrize("max_age", ["0", 30, "30"]) def test_cookie_max_age(app, max_age): cookies = {"test": "wait"} @@ -204,6 +204,23 @@ def handler(request): assert cookie is None +@pytest.mark.parametrize("max_age", [30.0, 30.1, "test"]) +def test_cookie_bad_max_age(app, max_age): + cookies = {"test": "wait"} + + @app.get("/") + def handler(request): + response = text("pass") + response.cookies["test"] = "pass" + response.cookies["test"]["max-age"] = max_age + return response + + request, response = app.test_client.get( + "/", cookies=cookies, raw_cookies=True + ) + assert response.status == 500 + + @pytest.mark.parametrize( "expires", [datetime.utcnow() + timedelta(seconds=60)] ) diff --git a/tests/test_load_module_from_file_location.py b/tests/test_load_module_from_file_location.py deleted file mode 100644 index c47913dd9a..0000000000 --- a/tests/test_load_module_from_file_location.py +++ /dev/null @@ -1,38 +0,0 @@ -from pathlib import Path -from types import ModuleType - -import pytest - -from sanic.exceptions import LoadFileException -from sanic.utils import load_module_from_file_location - - -@pytest.fixture -def loaded_module_from_file_location(): - return load_module_from_file_location( - str(Path(__file__).parent / "static" / "app_test_config.py") - ) - - -@pytest.mark.dependency(name="test_load_module_from_file_location") -def test_load_module_from_file_location(loaded_module_from_file_location): - assert isinstance(loaded_module_from_file_location, ModuleType) - - -@pytest.mark.dependency(depends=["test_load_module_from_file_location"]) -def test_loaded_module_from_file_location_name( - loaded_module_from_file_location, -): - name = loaded_module_from_file_location.__name__ - if "C:\\" in name: - name = name.split("\\")[-1] - assert name == "app_test_config" - - -def test_load_module_from_file_location_with_non_existing_env_variable(): - with pytest.raises( - LoadFileException, - match="The following environment variables are not set: MuuMilk", - ): - - load_module_from_file_location("${MuuMilk}") diff --git a/tests/test_update_config.py b/tests/test_update_config.py deleted file mode 100644 index 8f30445829..0000000000 --- a/tests/test_update_config.py +++ /dev/null @@ -1,36 +0,0 @@ -from pathlib import Path - -import pytest - - -_test_setting_as_dict = {"TEST_SETTING_VALUE": 1} -_test_setting_as_class = type("C", (), {"TEST_SETTING_VALUE": 1}) -_test_setting_as_module = str( - Path(__file__).parent / "static/app_test_config.py" -) - - -@pytest.mark.parametrize( - "conf_object", - [ - _test_setting_as_dict, - _test_setting_as_class, - pytest.param( - _test_setting_as_module, - marks=pytest.mark.dependency( - depends=["test_load_module_from_file_location"], - scope="session", - ), - ), - ], - ids=["from_dict", "from_class", "from_file"], -) -def test_update(app, conf_object): - app.update_config(conf_object) - assert app.config["TEST_SETTING_VALUE"] == 1 - - -def test_update_from_lowercase_key(app): - d = {"test_setting_value": 1} - app.update_config(d) - assert "test_setting_value" not in app.config diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 0000000000..3744b3887c --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,50 @@ +from os import environ +from pathlib import Path +from types import ModuleType + +import pytest + +from sanic.exceptions import LoadFileException +from sanic.utils import load_module_from_file_location + + +@pytest.mark.parametrize( + "location", + ( + Path(__file__).parent / "static" / "app_test_config.py", + str(Path(__file__).parent / "static" / "app_test_config.py"), + str(Path(__file__).parent / "static" / "app_test_config.py").encode(), + ), +) +def test_load_module_from_file_location(location): + module = load_module_from_file_location(location) + + assert isinstance(module, ModuleType) + + +def test_loaded_module_from_file_location_name(): + module = load_module_from_file_location( + str(Path(__file__).parent / "static" / "app_test_config.py") + ) + + name = module.__name__ + if "C:\\" in name: + name = name.split("\\")[-1] + assert name == "app_test_config" + + +def test_load_module_from_file_location_with_non_existing_env_variable(): + with pytest.raises( + LoadFileException, + match="The following environment variables are not set: MuuMilk", + ): + + load_module_from_file_location("${MuuMilk}") + + +def test_load_module_from_file_location_using_env(): + environ["APP_TEST_CONFIG"] = "static/app_test_config.py" + location = str(Path(__file__).parent / "${APP_TEST_CONFIG}") + module = load_module_from_file_location(location) + + assert isinstance(module, ModuleType) diff --git a/tox.ini b/tox.ini index 49998c984e..e365c9fc6f 100644 --- a/tox.ini +++ b/tox.ini @@ -13,7 +13,6 @@ deps = pytest-sanic pytest-sugar pytest-benchmark - pytest-dependency httpcore==0.11.* httpx==0.15.4 chardet==3.*