From 6cd3c74ad9448e4f49857dde8c61d5422818c85c Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Mon, 6 May 2024 09:13:25 +0200 Subject: [PATCH 1/7] [#8] Add management commands for generating + checking configuration docs --- django_setup_configuration/__init__.py | 5 + django_setup_configuration/base.py | 166 ++++++++++++++++++ django_setup_configuration/constants.py | 31 ++++ django_setup_configuration/exceptions.py | 13 ++ .../management/commands/check_config_docs.py | 43 +++++ .../commands/generate_config_docs.py | 108 ++++++++++++ django_setup_configuration/registry.py | 47 +++++ 7 files changed, 413 insertions(+) create mode 100644 django_setup_configuration/base.py create mode 100644 django_setup_configuration/constants.py create mode 100644 django_setup_configuration/management/commands/check_config_docs.py create mode 100644 django_setup_configuration/management/commands/generate_config_docs.py create mode 100644 django_setup_configuration/registry.py diff --git a/django_setup_configuration/__init__.py b/django_setup_configuration/__init__.py index e69de29..4807d27 100644 --- a/django_setup_configuration/__init__.py +++ b/django_setup_configuration/__init__.py @@ -0,0 +1,5 @@ +from .base import ConfigSettingsModel + +__all__ = [ + "ConfigSettingsModel", +] diff --git a/django_setup_configuration/base.py b/django_setup_configuration/base.py new file mode 100644 index 0000000..50b24ea --- /dev/null +++ b/django_setup_configuration/base.py @@ -0,0 +1,166 @@ +from dataclasses import dataclass, field +from typing import Iterator, Mapping, Sequence + +from django.contrib.postgres.fields import ArrayField +from django.db import models +from django.db.models.fields import NOT_PROVIDED +from django.db.models.fields.json import JSONField +from django.db.models.fields.related import ForeignKey, OneToOneField + +from .constants import BasicFieldDescription + + +@dataclass(frozen=True, slots=True) +class ConfigField: + name: str + verbose_name: str + description: str + default_value: str + values: str + + +@dataclass +class Fields: + all: set[ConfigField] = field(default_factory=set) + required: set[ConfigField] = field(default_factory=set) + + +class ConfigSettingsModel: + model: models.Model + display_name: str + namespace: str + required_fields = tuple() + all_fields = tuple() + excluded_fields = ("id",) + + def __init__(self): + self.config_fields = Fields() + + self.create_config_fields( + require=self.required_fields, + exclude=self.excluded_fields, + include=self.all_fields, + model=self.model, + ) + + @classmethod + def get_setting_name(cls, field: ConfigField) -> str: + return f"{cls.namespace}_" + field.name.upper() + + @staticmethod + def get_default_value(field: models.Field) -> str: + default = field.default + + if default is NOT_PROVIDED: + return "No default" + + # needed to make `generate_config_docs` idempotent + # because UUID's are randomly generated + if isinstance(field, models.UUIDField): + return "random UUID string" + + # if default is a function, call the function to retrieve the value; + # we don't immediately return because we need to check the type first + # and cast to another type if necessary (e.g. list is unhashable) + if callable(default): + default = default() + + if isinstance(default, Mapping): + return str(default) + + # check for field type as well to avoid splitting values from CharField + if isinstance(field, (JSONField, ArrayField)) and isinstance(default, Sequence): + try: + return ", ".join(str(item) for item in default) + except TypeError: + return str(default) + + return default + + @staticmethod + def get_example_values(field: models.Field) -> str: + # fields with choices + if choices := field.choices: + values = [choice[0] for choice in choices] + return ", ".join(values) + + # other fields + field_type = field.get_internal_type() + match field_type: + case item if item in BasicFieldDescription.names: + return getattr(BasicFieldDescription, field_type) + case _: + return "No information available" + + def get_concrete_model_fields(self, model) -> Iterator[models.Field]: + """ + Get all concrete fields for a given `model`, skipping over backreferences like + `OneToOneRel` and fields that are blacklisted + """ + return ( + field + for field in model._meta.concrete_fields + if field.name not in self.excluded_fields + ) + + def create_config_fields( + self, + require: tuple[str, ...], + exclude: tuple[str, ...], + include: tuple[str, ...], + model: models.Model, + relating_field: models.Field | None = None, + ) -> None: + """ + Create a `ConfigField` instance for each field of the given `model` and + add it to `self.fields.all` and `self.fields.required` + + Basic fields (`CharField`, `IntegerField` etc) constitute the base case, + relations (`ForeignKey`, `OneToOneField`) are handled recursively + """ + + model_fields = self.get_concrete_model_fields(model) + + for model_field in model_fields: + if isinstance(model_field, (ForeignKey, OneToOneField)): + self.create_config_fields( + require=require, + exclude=exclude, + include=include, + model=model_field.related_model, + relating_field=model_field, + ) + else: + if model_field.name in self.excluded_fields: + continue + + # model field name could be "api_root", + # but we need "xyz_service_api_root" (or similar) for consistency + if relating_field: + name = f"{relating_field.name}_{model_field.name}" + else: + name = model_field.name + + config_field = ConfigField( + name=name, + verbose_name=model_field.verbose_name, + description=model_field.help_text, + default_value=self.get_default_value(model_field), + values=self.get_example_values(model_field), + ) + + if config_field.name in self.required_fields: + self.config_fields.required.add(config_field) + + # if all_fields is empty, that means we're filtering by blacklist, + # hence the config_field is included by default + if not self.all_fields or config_field.name in self.all_fields: + self.config_fields.all.add(config_field) + + def get_required_settings(self) -> tuple[str, ...]: + return tuple( + self.get_setting_name(field) for field in self.config_fields.required + ) + + def get_config_mapping(self) -> dict[str, ConfigField]: + return {self.get_setting_name(field): field for field in self.config_fields.all} diff --git a/django_setup_configuration/constants.py b/django_setup_configuration/constants.py new file mode 100644 index 0000000..60d294d --- /dev/null +++ b/django_setup_configuration/constants.py @@ -0,0 +1,31 @@ +from django.db import models + + +class BasicFieldDescription(models.TextChoices): + """ + Description of the values for basic Django model fields + """ + + ArrayField = "string, comma-delimited ('foo,bar,baz')" + BooleanField = "True, False" + CharField = "string" + FileField = ( + "string represeting the (absolute) path to a file, " + "including file extension: {example}".format( + example="/absolute/path/to/file.xml" + ) + ) + ImageField = ( + "string represeting the (absolute) path to an image file, " + "including file extension: {example}".format( + example="/absolute/path/to/image.png" + ) + ) + IntegerField = "string representing an integer" + JSONField = "Mapping: {example}".format(example="{'some_key': 'Some value'}") + PositiveIntegerField = "string representing a positive integer" + TextField = "text (string)" + URLField = "string (URL)" + UUIDField = "UUID string {example}".format( + example="(e.g. f6b45142-0c60-4ec7-b43d-28ceacdc0b34)" + ) diff --git a/django_setup_configuration/exceptions.py b/django_setup_configuration/exceptions.py index d8aac32..f7efa4f 100644 --- a/django_setup_configuration/exceptions.py +++ b/django_setup_configuration/exceptions.py @@ -20,3 +20,16 @@ class SelfTestFailed(ConfigurationException): """ Raises an error for failed configuration self-tests. """ + + +class ImproperlyConfigured(ConfigurationException): + """ + Raised when the library is not properly configured + """ + + +class DocumentationCheckFailed(ConfigurationException): + """ + Raised when the documentation based on the configuration models + is not up to date + """ diff --git a/django_setup_configuration/management/commands/check_config_docs.py b/django_setup_configuration/management/commands/check_config_docs.py new file mode 100644 index 0000000..f03d0a2 --- /dev/null +++ b/django_setup_configuration/management/commands/check_config_docs.py @@ -0,0 +1,43 @@ +from django.conf import settings + +from ...exceptions import DocumentationCheckFailed +from .generate_config_docs import ConfigDocBaseCommand + +SOURCE_DIR = settings.DJANGO_SETUP_CONFIG_DOC_DIR + + +class Command(ConfigDocBaseCommand): + help = "Check that changes to configuration setup classes are reflected in the docs" + + def check_doc(self, config_option: str) -> None: + source_path = f"{SOURCE_DIR}/{config_option}.rst" + + try: + with open(source_path, "r") as file: + file_content = file.read() + except FileNotFoundError as exc: + msg = ( + "\nNo documentation was found for {config}\n" + "Did you forget to run generate_config_docs?\n".format( + config=self.get_config(config_option, class_name_only=True) + ) + ) + raise DocumentationCheckFailed(msg) from exc + else: + rendered_content = self.render_doc(config_option) + + if rendered_content != file_content: + raise DocumentationCheckFailed( + "Class {config} has changes which are not reflected in the " + "documentation ({source_path}). " + "Did you forget to run generate_config_docs?\n".format( + config=self.get_config(config_option, class_name_only=True), + source_path=f"{SOURCE_DIR}/{config_option}.rst", + ) + ) + + def handle(self, *args, **kwargs) -> None: + supported_options = self.registry.field_names + + for option in supported_options: + self.check_doc(option) diff --git a/django_setup_configuration/management/commands/generate_config_docs.py b/django_setup_configuration/management/commands/generate_config_docs.py new file mode 100644 index 0000000..ef8d948 --- /dev/null +++ b/django_setup_configuration/management/commands/generate_config_docs.py @@ -0,0 +1,108 @@ +import pathlib + +from django.conf import settings +from django.core.management.base import BaseCommand +from django.template import loader + +from ...base import ConfigSettingsModel +from ...exceptions import ConfigurationException +from ...registry import ConfigurationRegistry + +TEMPLATE_NAME = settings.DJANGO_SETUP_CONFIG_TEMPLATE_NAME +TARGET_DIR = settings.DJANGO_SETUP_CONFIG_DOC_DIR + + +class ConfigDocBaseCommand(BaseCommand): + registry = ConfigurationRegistry() + + def get_config( + self, config_option: str, class_name_only=False + ) -> ConfigSettingsModel: + config_model = getattr(self.registry, config_option, None) + if class_name_only: + return config_model.__name__ + + config_instance = config_model() + return config_instance + + def get_detailed_info(self, config: ConfigSettingsModel) -> list[list[str]]: + ret = [] + for field in config.config_fields.all: + part = [] + part.append(f"{'Variable':<20}{config.get_setting_name(field)}") + part.append(f"{'Setting':<20}{field.verbose_name}") + part.append(f"{'Description':<20}{field.description or 'No description'}") + part.append(f"{'Possible values':<20}{field.values}") + part.append(f"{'Default value':<20}{field.default_value}") + ret.append(part) + return ret + + def format_display_name(self, display_name): + """Surround title with '=' to display as heading in rst file""" + + heading_bar = "=" * len(display_name) + display_name_formatted = f"{heading_bar}\n{display_name}\n{heading_bar}" + return display_name_formatted + + def render_doc(self, config_option: str) -> None: + config = self.get_config(config_option) + + required_settings = [ + config.get_setting_name(field) for field in config.config_fields.required + ] + required_settings.sort() + + all_settings = [ + config.get_setting_name(field) for field in config.config_fields.all + ] + all_settings.sort() + + detailed_info = self.get_detailed_info(config) + detailed_info.sort() + + template_variables = { + "enable_settings": f"{config.namespace}_CONFIG_ENABLE", + "required_settings": required_settings, + "all_settings": all_settings, + "detailed_info": detailed_info, + "link": f".. _{config_option}:", + "title": self.format_display_name(config.display_name), + } + + template = loader.get_template(TEMPLATE_NAME) + rendered = template.render(template_variables) + + return rendered + + +class Command(ConfigDocBaseCommand): + help = "Create documentation for configuration setup steps" + + def add_arguments(self, parser): + parser.add_argument("config_option", nargs="?") + + def write_doc(self, config_option: str) -> None: + rendered = self.render_doc(config_option) + + pathlib.Path(TARGET_DIR).mkdir(parents=True, exist_ok=True) + + output_path = f"{TARGET_DIR}/{config_option}.rst" + + with open(output_path, "w+") as output: + output.write(rendered) + + def handle(self, *args, **kwargs) -> None: + config_option = kwargs["config_option"] + + supported_options = self.registry.field_names + + if config_option and config_option not in supported_options: + raise ConfigurationException( + f"Unsupported config option ({config_option})\n" + f"Supported: {', '.join(supported_options)}" + ) + elif config_option: + self.write_doc(config_option) + else: + for option in supported_options: + self.write_doc(option) diff --git a/django_setup_configuration/registry.py b/django_setup_configuration/registry.py new file mode 100644 index 0000000..1a11573 --- /dev/null +++ b/django_setup_configuration/registry.py @@ -0,0 +1,47 @@ +from django.conf import settings +from django.utils.module_loading import import_string + +from .base import ConfigSettingsModel +from .exceptions import ImproperlyConfigured + + +class ConfigurationRegistry: + def __init__(self): + if not getattr(settings, "DJANGO_SETUP_CONFIG_REGISTER", None): + raise ImproperlyConfigured("DJANGO_SETUP_CONFIG_REGISTER is not defined") + + if not all( + (entry.get("model") for entry in settings.DJANGO_SETUP_CONFIG_REGISTER) + ): + raise ImproperlyConfigured( + "Each entry for the DJANGO_SETUP_CONFIG_REGISTER setting " + "must specify a configuration model" + ) + + self.register_config_models() + + def register_config_models(self) -> None: + """ + Load config models specified in settings and set them as attributes on + the instance + """ + for mapping in settings.DJANGO_SETUP_CONFIG_REGISTER: + file_name = mapping.get("file_name") or mapping["model"].split(".")[-1] + + try: + model = import_string(mapping["model"]) + except ImportError as exc: + exc.add_note( + "\nHint: check your settings for django-setup-configuration" + ) + raise + else: + setattr(self, file_name, model) + + @property + def fields(self) -> tuple[ConfigSettingsModel, ...]: + return tuple(getattr(self, key) for key in vars(self).keys()) + + @property + def field_names(self) -> tuple[str, ...]: + return tuple(key for key in vars(self).keys()) From 67dd087f66b50897e43f968601b7f710dac773e2 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Fri, 10 May 2024 09:44:54 +0200 Subject: [PATCH 2/7] [#8] Add tests for documentation management commands --- .github/workflows/ci.yml | 16 ++++++ django_setup_configuration/base.py | 28 +++++----- django_setup_configuration/constants.py | 37 ++++++------- .../management/commands/check_config_docs.py | 8 +-- .../commands/generate_config_docs.py | 25 +++++---- django_setup_configuration/registry.py | 13 +++-- pyproject.toml | 5 ++ testapp/config_models.py | 34 ++++++++++++ testapp/configuration.py | 11 ++++ testapp/docs/configuration/product.rst | 55 +++++++++++++++++++ testapp/settings.py | 16 +++++- testapp/templates/testapp/config_doc.rst | 46 ++++++++++++++++ tests/mocks.py | 1 + tests/test_config_docs.py | 50 +++++++++++++++++ tests/test_config_registry.py | 46 ++++++++++++++++ 15 files changed, 336 insertions(+), 55 deletions(-) create mode 100644 testapp/config_models.py create mode 100644 testapp/docs/configuration/product.rst create mode 100644 testapp/templates/testapp/config_doc.rst create mode 100644 tests/mocks.py create mode 100644 tests/test_config_docs.py create mode 100644 tests/test_config_registry.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f4f5e31..8828c35 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,20 @@ jobs: name: Run the test suite (Python ${{ matrix.python }}, Django ${{ matrix.django }}) + services: + postgres: + image: postgres + env: + POSTGRES_HOST_AUTH_METHOD: trust + ports: + - 5432:5432 + # needed because the postgres container does not provide a healthcheck + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 @@ -34,6 +48,8 @@ jobs: env: PYTHON_VERSION: ${{ matrix.python }} DJANGO: ${{ matrix.django }} + POSTGRES_USER: postgres + POSTGRES_HOST: localhost - name: Publish coverage report uses: codecov/codecov-action@v3 diff --git a/django_setup_configuration/base.py b/django_setup_configuration/base.py index 50b24ea..1b64271 100644 --- a/django_setup_configuration/base.py +++ b/django_setup_configuration/base.py @@ -5,9 +5,9 @@ from django.db import models from django.db.models.fields import NOT_PROVIDED from django.db.models.fields.json import JSONField -from django.db.models.fields.related import ForeignKey, OneToOneField +from django.db.models.fields.related import OneToOneField -from .constants import BasicFieldDescription +from .constants import basic_field_description @dataclass(frozen=True, slots=True) @@ -16,7 +16,7 @@ class ConfigField: verbose_name: str description: str default_value: str - values: str + field_description: str @dataclass @@ -78,17 +78,16 @@ def get_default_value(field: models.Field) -> str: return default @staticmethod - def get_example_values(field: models.Field) -> str: + def get_field_description(field: models.Field) -> str: # fields with choices if choices := field.choices: - values = [choice[0] for choice in choices] - return ", ".join(values) + example_values = [choice[0] for choice in choices] + return ", ".join(example_values) - # other fields - field_type = field.get_internal_type() + field_type = type(field) match field_type: - case item if item in BasicFieldDescription.names: - return getattr(BasicFieldDescription, field_type) + case item if item in basic_field_description.keys(): + return basic_field_description.get(item) case _: return "No information available" @@ -116,13 +115,16 @@ def create_config_fields( add it to `self.fields.all` and `self.fields.required` Basic fields (`CharField`, `IntegerField` etc) constitute the base case, - relations (`ForeignKey`, `OneToOneField`) are handled recursively + one-to-one relations (`OneToOneField`) are handled recursively + + `ForeignKey` and `ManyToManyField` are currently not supported (these require + special care to avoid recursion errors) """ model_fields = self.get_concrete_model_fields(model) for model_field in model_fields: - if isinstance(model_field, (ForeignKey, OneToOneField)): + if isinstance(model_field, OneToOneField): self.create_config_fields( require=require, exclude=exclude, @@ -146,7 +148,7 @@ def create_config_fields( verbose_name=model_field.verbose_name, description=model_field.help_text, default_value=self.get_default_value(model_field), - values=self.get_example_values(model_field), + field_description=self.get_field_description(model_field), ) if config_field.name in self.required_fields: diff --git a/django_setup_configuration/constants.py b/django_setup_configuration/constants.py index 60d294d..13e453b 100644 --- a/django_setup_configuration/constants.py +++ b/django_setup_configuration/constants.py @@ -1,31 +1,28 @@ +from django.contrib import postgres from django.db import models - -class BasicFieldDescription(models.TextChoices): - """ - Description of the values for basic Django model fields - """ - - ArrayField = "string, comma-delimited ('foo,bar,baz')" - BooleanField = "True, False" - CharField = "string" - FileField = ( +basic_field_description = { + postgres.fields.ArrayField: "string, comma-delimited ('foo,bar,baz')", + models.BooleanField: "True, False", + models.CharField: "string", + models.FileField: ( "string represeting the (absolute) path to a file, " "including file extension: {example}".format( example="/absolute/path/to/file.xml" ) - ) - ImageField = ( + ), + models.ImageField: ( "string represeting the (absolute) path to an image file, " "including file extension: {example}".format( example="/absolute/path/to/image.png" ) - ) - IntegerField = "string representing an integer" - JSONField = "Mapping: {example}".format(example="{'some_key': 'Some value'}") - PositiveIntegerField = "string representing a positive integer" - TextField = "text (string)" - URLField = "string (URL)" - UUIDField = "UUID string {example}".format( + ), + models.IntegerField: "string representing an integer", + models.JSONField: "Mapping: {example}".format(example="{'some_key': 'Some value'}"), + models.PositiveIntegerField: "string representing a positive integer", + models.TextField: "text (string)", + models.URLField: "string (URL)", + models.UUIDField: "UUID string {example}".format( example="(e.g. f6b45142-0c60-4ec7-b43d-28ceacdc0b34)" - ) + ), +} diff --git a/django_setup_configuration/management/commands/check_config_docs.py b/django_setup_configuration/management/commands/check_config_docs.py index f03d0a2..fc7df10 100644 --- a/django_setup_configuration/management/commands/check_config_docs.py +++ b/django_setup_configuration/management/commands/check_config_docs.py @@ -3,13 +3,13 @@ from ...exceptions import DocumentationCheckFailed from .generate_config_docs import ConfigDocBaseCommand -SOURCE_DIR = settings.DJANGO_SETUP_CONFIG_DOC_DIR - class Command(ConfigDocBaseCommand): help = "Check that changes to configuration setup classes are reflected in the docs" def check_doc(self, config_option: str) -> None: + SOURCE_DIR = settings.DJANGO_SETUP_CONFIG_DOC_DIR + source_path = f"{SOURCE_DIR}/{config_option}.rst" try: @@ -29,7 +29,7 @@ def check_doc(self, config_option: str) -> None: if rendered_content != file_content: raise DocumentationCheckFailed( "Class {config} has changes which are not reflected in the " - "documentation ({source_path}). " + "documentation ({source_path})." "Did you forget to run generate_config_docs?\n".format( config=self.get_config(config_option, class_name_only=True), source_path=f"{SOURCE_DIR}/{config_option}.rst", @@ -37,7 +37,7 @@ def check_doc(self, config_option: str) -> None: ) def handle(self, *args, **kwargs) -> None: - supported_options = self.registry.field_names + supported_options = self.registry.config_model_keys for option in supported_options: self.check_doc(option) diff --git a/django_setup_configuration/management/commands/generate_config_docs.py b/django_setup_configuration/management/commands/generate_config_docs.py index ef8d948..a1fa08a 100644 --- a/django_setup_configuration/management/commands/generate_config_docs.py +++ b/django_setup_configuration/management/commands/generate_config_docs.py @@ -8,12 +8,12 @@ from ...exceptions import ConfigurationException from ...registry import ConfigurationRegistry -TEMPLATE_NAME = settings.DJANGO_SETUP_CONFIG_TEMPLATE_NAME -TARGET_DIR = settings.DJANGO_SETUP_CONFIG_DOC_DIR - class ConfigDocBaseCommand(BaseCommand): - registry = ConfigurationRegistry() + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.registry = ConfigurationRegistry() def get_config( self, config_option: str, class_name_only=False @@ -25,14 +25,14 @@ def get_config( config_instance = config_model() return config_instance - def get_detailed_info(self, config: ConfigSettingsModel) -> list[list[str]]: + def get_detailed_info(self, config=ConfigSettingsModel) -> list[list[str]]: ret = [] for field in config.config_fields.all: part = [] part.append(f"{'Variable':<20}{config.get_setting_name(field)}") part.append(f"{'Setting':<20}{field.verbose_name}") part.append(f"{'Description':<20}{field.description or 'No description'}") - part.append(f"{'Possible values':<20}{field.values}") + part.append(f"{'Possible values':<20}{field.field_description}") part.append(f"{'Default value':<20}{field.default_value}") ret.append(part) return ret @@ -69,7 +69,7 @@ def render_doc(self, config_option: str) -> None: "title": self.format_display_name(config.display_name), } - template = loader.get_template(TEMPLATE_NAME) + template = loader.get_template(settings.DJANGO_SETUP_CONFIG_TEMPLATE_NAME) rendered = template.render(template_variables) return rendered @@ -84,6 +84,8 @@ def add_arguments(self, parser): def write_doc(self, config_option: str) -> None: rendered = self.render_doc(config_option) + TARGET_DIR = settings.DJANGO_SETUP_CONFIG_DOC_DIR + pathlib.Path(TARGET_DIR).mkdir(parents=True, exist_ok=True) output_path = f"{TARGET_DIR}/{config_option}.rst" @@ -91,10 +93,12 @@ def write_doc(self, config_option: str) -> None: with open(output_path, "w+") as output: output.write(rendered) + return rendered + def handle(self, *args, **kwargs) -> None: config_option = kwargs["config_option"] - supported_options = self.registry.field_names + supported_options = self.registry.config_model_keys if config_option and config_option not in supported_options: raise ConfigurationException( @@ -102,7 +106,8 @@ def handle(self, *args, **kwargs) -> None: f"Supported: {', '.join(supported_options)}" ) elif config_option: - self.write_doc(config_option) + rendered = self.write_doc(config_option) else: for option in supported_options: - self.write_doc(option) + rendered = self.write_doc(option) + return rendered diff --git a/django_setup_configuration/registry.py b/django_setup_configuration/registry.py index 1a11573..0c23ad0 100644 --- a/django_setup_configuration/registry.py +++ b/django_setup_configuration/registry.py @@ -31,17 +31,18 @@ def register_config_models(self) -> None: try: model = import_string(mapping["model"]) except ImportError as exc: - exc.add_note( - "\nHint: check your settings for django-setup-configuration" - ) - raise + raise ImproperlyConfigured( + "\n\nThe class testapp.models.bogus was not found\n" + "Check the DJANGO_SETUP_CONFIG_REGISTER setting for " + "django-setup-configuration" + ) from exc else: setattr(self, file_name, model) @property - def fields(self) -> tuple[ConfigSettingsModel, ...]: + def config_models(self) -> tuple[ConfigSettingsModel, ...]: return tuple(getattr(self, key) for key in vars(self).keys()) @property - def field_names(self) -> tuple[str, ...]: + def config_model_keys(self) -> tuple[str, ...]: return tuple(key for key in vars(self).keys()) diff --git a/pyproject.toml b/pyproject.toml index 1cc2eee..0f78018 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,7 @@ Documentation = "http://django-setup-configuration.readthedocs.io/en/latest/" [project.optional-dependencies] tests = [ + "psycopg2", "pytest", "pytest-django", "pytest-mock", @@ -75,6 +76,10 @@ sections=["FUTURE", "STDLIB", "DJANGO", "THIRDPARTY", "FIRSTPARTY", "LOCALFOLDER testpaths = ["tests"] DJANGO_SETTINGS_MODULE = "testapp.settings" +[tool.pytest] +testpaths = ["tests"] +DJANGO_SETTINGS_MODULE = "testapp.settings" + [tool.bumpversion] current_version = "0.1.0" files = [ diff --git a/testapp/config_models.py b/testapp/config_models.py new file mode 100644 index 0000000..3ab418c --- /dev/null +++ b/testapp/config_models.py @@ -0,0 +1,34 @@ +from django.contrib.postgres.fields import ArrayField +from django.db import models + + +class Service(models.Model): + url = models.URLField( + verbose_name="Service url", + help_text="The url of the service", + ) + bogus = models.CharField( + verbose_name="Bogus service field", help_text="Should not be included in docs" + ) + + +class ProductConfig(models.Model): + name = models.CharField( + verbose_name="Name", + help_text="The name of the product", + ) + service = models.OneToOneField( + to=Service, + verbose_name="Service", + default=None, + on_delete=models.SET_NULL, + help_text="API service of the product", + ) + tags = ArrayField( + base_field=models.CharField("Product tag"), + default=["example_tag"], + help_text="Tags for the product", + ) + bogus = models.CharField( + help_text="Should be excluded", + ) diff --git a/testapp/configuration.py b/testapp/configuration.py index f56dee9..1a6fb4b 100644 --- a/testapp/configuration.py +++ b/testapp/configuration.py @@ -2,9 +2,20 @@ from django.contrib.auth import authenticate from django.contrib.auth.models import User +from django_setup_configuration.base import ConfigSettingsModel from django_setup_configuration.configuration import BaseConfigurationStep from django_setup_configuration.exceptions import SelfTestFailed +from .config_models import ProductConfig + + +class ProductConfigurationSettings(ConfigSettingsModel): + model = ProductConfig + display_name = "Product Configuration" + namespace = "PRODUCT" + required_fields = ("name", "service_url") + all_fields = required_fields + ("tags",) + class UserConfigurationStep(BaseConfigurationStep): """ diff --git a/testapp/docs/configuration/product.rst b/testapp/docs/configuration/product.rst new file mode 100644 index 0000000..b242abf --- /dev/null +++ b/testapp/docs/configuration/product.rst @@ -0,0 +1,55 @@ +.. _product: + +===================== +Product Configuration +===================== + +Settings Overview +================= + +Enable/Disable configuration: +""""""""""""""""""""""""""""" + +:: + + PRODUCT_CONFIG_ENABLE + +Required: +""""""""" + +:: + + PRODUCT_NAME + PRODUCT_SERVICE_URL + +All settings: +""""""""""""" + +:: + + PRODUCT_NAME + PRODUCT_SERVICE_URL + PRODUCT_TAGS + +Detailed Information +==================== + +:: + + Variable PRODUCT_NAME + Setting Name + Description The name of the product + Possible values string + Default value No default + + Variable PRODUCT_SERVICE_URL + Setting Service url + Description The url of the service + Possible values string (URL) + Default value No default + + Variable PRODUCT_TAGS + Setting tags + Description Tags for the product + Possible values string, comma-delimited ('foo,bar,baz') + Default value example_tag diff --git a/testapp/settings.py b/testapp/settings.py index fef34af..dc3b19b 100644 --- a/testapp/settings.py +++ b/testapp/settings.py @@ -9,8 +9,11 @@ DATABASES = { "default": { - "ENGINE": "django.db.backends.sqlite3", - "NAME": BASE_DIR / "django_setup_configuration.db", + "ENGINE": "django.db.backends.postgresql", + "NAME": "postgres", + "USER": "postgres", + "PASSWORD": "postgres", + "HOST": "localhost", } } @@ -64,3 +67,12 @@ USER_CONFIGURATION_ENABLED = os.getenv("USER_CONFIGURATION_ENABLED", True) USER_CONFIGURATION_USERNAME = os.getenv("USER_CONFIGURATION_USERNAME", "demo") USER_CONFIGURATION_PASSWORD = os.getenv("USER_CONFIGURATION_PASSWORD", "secret") + +DJANGO_SETUP_CONFIG_REGISTER = [ + { + "model": "testapp.configuration.ProductConfigurationSettings", + "file_name": "product", + } +] +DJANGO_SETUP_CONFIG_TEMPLATE_NAME = "testapp/config_doc.rst" +DJANGO_SETUP_CONFIG_DOC_DIR = "testapp/docs/configuration" diff --git a/testapp/templates/testapp/config_doc.rst b/testapp/templates/testapp/config_doc.rst new file mode 100644 index 0000000..b9de9c5 --- /dev/null +++ b/testapp/templates/testapp/config_doc.rst @@ -0,0 +1,46 @@ +{% block link %}{{ link }}{% endblock %} + +{% block title %}{{ title }}{% endblock %} + +Settings Overview +================= + +Enable/Disable configuration: +""""""""""""""""""""""""""""" + +:: + + {% spaceless %} + {{ enable_settings }} + {% endspaceless %} + +Required: +""""""""" + +:: + + {% spaceless %} + {% for setting in required_settings %}{{ setting }} + {% endfor %} + {% endspaceless %} + +All settings: +""""""""""""" + +:: + + {% spaceless %} + {% for setting in all_settings %}{{ setting }} + {% endfor %} + {% endspaceless %} + +Detailed Information +==================== + +:: + + {% spaceless %} + {% for detail in detailed_info %} + {% for part in detail %}{{ part|safe }} + {% endfor %}{% endfor %} + {% endspaceless %} diff --git a/tests/mocks.py b/tests/mocks.py new file mode 100644 index 0000000..0d02d68 --- /dev/null +++ b/tests/mocks.py @@ -0,0 +1 @@ +mock_product_doc = '.. _product:\n\n=====================\nProduct Configuration\n=====================\n\nSettings Overview\n=================\n\nEnable/Disable configuration:\n"""""""""""""""""""""""""""""\n\n::\n\n PRODUCT_CONFIG_ENABLE\n\nRequired:\n"""""""""\n\n::\n\n PRODUCT_NAME\n PRODUCT_SERVICE_URL\n\nAll settings:\n"""""""""""""\n\n::\n\n PRODUCT_NAME\n PRODUCT_SERVICE_URL\n PRODUCT_TAGS\n\nDetailed Information\n====================\n\n::\n\n Variable PRODUCT_NAME\n Setting Name\n Description The name of the product\n Possible values string\n Default value No default\n \n Variable PRODUCT_SERVICE_URL\n Setting Service url\n Description The url of the service\n Possible values string (URL)\n Default value No default\n \n Variable PRODUCT_TAGS\n Setting tags\n Description Tags for the product\n Possible values string, comma-delimited (\'foo,bar,baz\')\n Default value example_tag\n' # noqa diff --git a/tests/test_config_docs.py b/tests/test_config_docs.py new file mode 100644 index 0000000..2cf3999 --- /dev/null +++ b/tests/test_config_docs.py @@ -0,0 +1,50 @@ +from unittest import mock + +from django.core.management import call_command + +import pytest + +from django_setup_configuration.exceptions import DocumentationCheckFailed + +from .mocks import mock_product_doc + + +def test_generate_config_docs(settings): + content = call_command("generate_config_docs") + + assert "PRODUCT_CONFIG_ENABLE" in content + # 3 occurrences of PRODUCT_NAME: required, all settings, description + assert content.count("PRODUCT_NAME") == 3 + + assert "PRODUCT_SERVICE_URL" in content + # 3 occurrences of PRODUCT_SERVICE_URL: required, all settings, description + assert content.count("PRODUCT_SERVICE_URL") == 3 + + assert "PRODUCT_TAGS" in content + # 2 occurrences of PRODUCT_TAGS: all settings, description + assert content.count("PRODUCT_TAGS") == 2 + + assert "PRODUCT_BOGUS" not in content + assert "PRODUCT_SERVICE_BOGUS" not in content + + +def test_check_config_docs_ok(settings): + with mock.patch("builtins.open", mock.mock_open(read_data=mock_product_doc)): + call_command("check_config_docs") + + +def test_check_config_docs_fail_missing_docs(settings): + mock_open = mock.mock_open(read_data=mock_product_doc) + mock_open.side_effect = FileNotFoundError + + with mock.patch("builtins.open", mock_open): + with pytest.raises(DocumentationCheckFailed): + call_command("check_config_docs") + + +@mock.patch("testapp.configuration.ProductConfigurationSettings.get_setting_name") +def test_check_config_docs_fail_content_mismatch(m, settings): + m.return_value = "" + + with pytest.raises(DocumentationCheckFailed): + call_command("check_config_docs") diff --git a/tests/test_config_registry.py b/tests/test_config_registry.py new file mode 100644 index 0000000..6b11868 --- /dev/null +++ b/tests/test_config_registry.py @@ -0,0 +1,46 @@ +from django.core.management import call_command + +import pytest + +from django_setup_configuration.exceptions import ImproperlyConfigured +from django_setup_configuration.registry import ConfigurationRegistry +from testapp.configuration import ProductConfigurationSettings + + +def test_missing_registry(settings): + del settings.DJANGO_SETUP_CONFIG_REGISTER + + with pytest.raises(ImproperlyConfigured): + call_command("generate_config_docs") + + +def test_registry_missing_model_spec(settings): + settings.DJANGO_SETUP_CONFIG_REGISTER = [{"file_name": "test"}] + + with pytest.raises(ImproperlyConfigured): + call_command("generate_config_docs") + + +def test_registry_model_not_found(settings): + settings.DJANGO_SETUP_CONFIG_REGISTER = [{"model": "testapp.models.bogus"}] + + with pytest.raises(ImproperlyConfigured): + call_command("generate_config_docs") + + +def test_registry_property_models(): + registry = ConfigurationRegistry() + + models = registry.config_models + + assert len(models) == 1 + assert models[0] == ProductConfigurationSettings + + +def test_registry_property_model_keys(): + registry = ConfigurationRegistry() + + model_keys = registry.config_model_keys + + assert len(model_keys) == 1 + assert model_keys[0] == "product" From b7a5383292b565f05f8f65e83ff2771bb607dfdb Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Tue, 14 May 2024 09:35:20 +0200 Subject: [PATCH 3/7] [#8] Add documentation for generation of config docs --- docs/config_docs.rst | 143 +++++++++++++++++++++++++++++++++++++++++++ docs/index.rst | 8 ++- 2 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 docs/config_docs.rst diff --git a/docs/config_docs.rst b/docs/config_docs.rst new file mode 100644 index 0000000..53cd8cb --- /dev/null +++ b/docs/config_docs.rst @@ -0,0 +1,143 @@ +Configuration documentation +=========================== + +The library can generate (and check) the documentation for model fields which are being +configured, containing information about the names of the fields, possible values, +default values, and a short description. This helps clients determine what can be +configured with the help of the library and how. + + +Setup +""""" + +Start by defining the target directory of the docs and the template that will be used +to render them. For example: + +:: + + DJANGO_SETUP_CONFIG_TEMPLATE_NAME = "configurations/config_doc.rst" + DJANGO_SETUP_CONFIG_DOC_DIR = "docs/configuration" + +Next, create a template: + +:: + + {% block link %}{{ link }}{% endblock %} + + {% block title %}{{ title }}{% endblock %} + + Settings Overview + ================= + + Enable/Disable configuration: + """"""""""""""""""""""""""""" + + :: + + {% spaceless %} + {{ enable_settings }} + {% endspaceless %} + + Required: + """"""""" + + :: + + {% spaceless %} + {% for setting in required_settings %}{{ setting }} + {% endfor %} + {% endspaceless %} + + All settings: + """"""""""""" + + :: + + {% spaceless %} + {% for setting in all_settings %}{{ setting }} + {% endfor %} + {% endspaceless %} + + Detailed Information + ==================== + + :: + + {% spaceless %} + {% for detail in detailed_info %} + {% for part in detail %}{{ part|safe }} + {% endfor %}{% endfor %} + {% endspaceless %} + +You're free to choose a different layout, of course, but this should give you an idea +of what's available, and some of the nuisances (like spacing) you might run into. + +The value for ``link`` is automatically create based on the ``file_name`` you defined +in the first step. Similarly, ``enable_settings`` is automatically created. Values for +the other template variables are determined based on the model your client is configuring, +and a related settings model you need to define. + +Generally, when adding support for configuring a model ``FooConfiguration`` with +``django-setup-configuration``, you will have a class ``FooConfigurationStep(BaseConfigurationStep)`` +that carries out the configuration. In the same file, define a class with information about +the fields from ``FooConfiguration`` that are required, included, or excluded, and +meta-information required for creating the docs. For example: + + +:: + + from django-setup-configuration import ConfigSettingsModel + + class FooConfigurationSettings(ConfigSettingsModel): + model = FooConfiguration + display_name = "Foo Configuration" + namespace = "FOO" + required_fields = ( + "bar", + "baz", + ) + included_fields = required_fields + ( + "foobar", + ) + excluded_fields = ( + "bogus", + ) + + + class FooConfigurationStep(BaseConfigurationStep): + ... + +``display_name`` provides the value for ``title`` in the template above. ``namespace`` +tells you how config variables for different models are namespaced. In your settings, +you would define ``FOO_BAR``, ``FOO_BAZ``, and ``FOO_FOOBAR``. + +Finally, you need to register you configuration settings class with the library: + +:: + + DJANGO_SETUP_CONFIG_REGISTER = [ + { + "model": "example_project.path.to.FooConfigurationSettings", + "file_name": "foo_config", + } + ] + + +Usage +""""" + +The library provides two management commands: + +:: + + manage.py generate_config_docs [CONFIG_OPTION] + manage.py check_config_docs + +The optional ``CONFIG_OPTION`` should be a ``file_name`` (without extension) that +corresponds to a settings model (e.g. ``foo_config``). When given, +``generate_config_docs`` will create the docs for the corresponding model. Otherwise +the command creates docs for all supported models. ``check_config_docs`` is similar +to ``manage.py makemigrations --check --dry-run``: it tests that documentation for your +configuration setup steps exists and is accurate (if you make changes to +``FooConfiguration`` or ``FooConfigurationSettings`` without re-creating the docs, +``check_config_docs`` will raise an exception). diff --git a/docs/index.rst b/docs/index.rst index a88b709..4e7f5f9 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -22,7 +22,13 @@ Features :maxdepth: 2 :caption: Contents: - quickstart + quickstart.rst + + config_docs.rst + +`Quickstart <./quickstart.rst>`_ + +`Configuration docs <./config_docs.rst>`_ From fe6e7980756d94c91313e810eb2a870f9f733913 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Wed, 15 May 2024 11:00:59 +0200 Subject: [PATCH 4/7] [#8] Integrate cration of docs with config steps + other feedback - delete registry, store config_settings as class_attributes on configuration steps - add static method to update model field descriptions with custom fields - modify template to show required settings section only if required settings are present --- .github/workflows/ci.yml | 2 - django_setup_configuration/base.py | 113 +++++++++-------- django_setup_configuration/configuration.py | 2 + django_setup_configuration/constants.py | 9 +- .../management/commands/check_config_docs.py | 43 ------- .../commands/generate_config_docs.py | 115 +++++++++--------- django_setup_configuration/registry.py | 48 -------- .../django_setup_configuration/config_doc.rst | 50 ++++++++ pyproject.toml | 1 + setup.cfg | 2 +- testapp/config_models.py | 34 ------ testapp/configuration.py | 21 ++-- testapp/docs/configuration/product.rst | 55 --------- testapp/settings.py | 15 +-- testapp/templates/testapp/config_doc.rst | 2 + tests/mocks.py | 3 +- tests/test_config_docs.py | 65 +++++----- tests/test_config_registry.py | 46 ------- 18 files changed, 227 insertions(+), 399 deletions(-) delete mode 100644 django_setup_configuration/management/commands/check_config_docs.py delete mode 100644 django_setup_configuration/registry.py create mode 100644 django_setup_configuration/templates/django_setup_configuration/config_doc.rst delete mode 100644 testapp/config_models.py delete mode 100644 testapp/docs/configuration/product.rst delete mode 100644 tests/test_config_registry.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8828c35..aff535b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,8 +48,6 @@ jobs: env: PYTHON_VERSION: ${{ matrix.python }} DJANGO: ${{ matrix.django }} - POSTGRES_USER: postgres - POSTGRES_HOST: localhost - name: Publish coverage report uses: codecov/codecov-action@v3 diff --git a/django_setup_configuration/base.py b/django_setup_configuration/base.py index 1b64271..1ba6202 100644 --- a/django_setup_configuration/base.py +++ b/django_setup_configuration/base.py @@ -1,13 +1,16 @@ -from dataclasses import dataclass, field -from typing import Iterator, Mapping, Sequence +from dataclasses import dataclass +from typing import Iterator, Mapping, Sequence, Type +from django.conf import settings from django.contrib.postgres.fields import ArrayField from django.db import models from django.db.models.fields import NOT_PROVIDED from django.db.models.fields.json import JSONField from django.db.models.fields.related import OneToOneField +from django.utils.module_loading import import_string -from .constants import basic_field_description +from .constants import basic_field_descriptions +from .exceptions import ImproperlyConfigured @dataclass(frozen=True, slots=True) @@ -19,33 +22,31 @@ class ConfigField: field_description: str -@dataclass -class Fields: - all: set[ConfigField] = field(default_factory=set) - required: set[ConfigField] = field(default_factory=set) - - class ConfigSettingsModel: - model: models.Model + models: list[Type[models.Model]] display_name: str namespace: str - required_fields = tuple() - all_fields = tuple() - excluded_fields = ("id",) - - def __init__(self): - self.config_fields = Fields() - - self.create_config_fields( - require=self.required_fields, - exclude=self.excluded_fields, - include=self.all_fields, - model=self.model, - ) + excluded_fields = ["id"] + + def __init__(self, *args, **kwargs): + self.config_fields: list = [] + + for key, value in kwargs.items(): + setattr(self, key, value) - @classmethod - def get_setting_name(cls, field: ConfigField) -> str: - return f"{cls.namespace}_" + field.name.upper() + self.update_field_descriptions() + + if not self.models: + return + + for model in self.models: + self.create_config_fields( + exclude=self.excluded_fields, + model=model, + ) + + def get_setting_name(self, field: ConfigField) -> str: + return f"{self.namespace}_" + field.name.upper() @staticmethod def get_default_value(field: models.Field) -> str: @@ -77,6 +78,30 @@ def get_default_value(field: models.Field) -> str: return default + @staticmethod + def update_field_descriptions() -> None: + """ + Add custom fields + descriptions defined in settings to + `basic_field_descriptions` + """ + custom_fields = getattr(settings, "DJANGO_SETUP_CONFIG_CUSTOM_FIELDS", None) + if not custom_fields: + return + + for mapping in custom_fields: + try: + field = import_string(mapping["field"]) + except ImportError as exc: + raise ImproperlyConfigured( + "\n\nSomething went wrong when importing {field}.\n" + "Check your settings for django-setup-configuration".format( + field=mapping["field"] + ) + ) from exc + else: + description = mapping["description"] + basic_field_descriptions[field] = description + @staticmethod def get_field_description(field: models.Field) -> str: # fields with choices @@ -84,12 +109,11 @@ def get_field_description(field: models.Field) -> str: example_values = [choice[0] for choice in choices] return ", ".join(example_values) + # other fields field_type = type(field) - match field_type: - case item if item in basic_field_description.keys(): - return basic_field_description.get(item) - case _: - return "No information available" + if field_type in basic_field_descriptions.keys(): + return basic_field_descriptions.get(field_type) + return "No information available" def get_concrete_model_fields(self, model) -> Iterator[models.Field]: """ @@ -104,15 +128,13 @@ def get_concrete_model_fields(self, model) -> Iterator[models.Field]: def create_config_fields( self, - require: tuple[str, ...], - exclude: tuple[str, ...], - include: tuple[str, ...], - model: models.Model, + exclude: list[str], + model: Type[models.Model], relating_field: models.Field | None = None, ) -> None: """ Create a `ConfigField` instance for each field of the given `model` and - add it to `self.fields.all` and `self.fields.required` + add it to `self.fields` Basic fields (`CharField`, `IntegerField` etc) constitute the base case, one-to-one relations (`OneToOneField`) are handled recursively @@ -126,9 +148,7 @@ def create_config_fields( for model_field in model_fields: if isinstance(model_field, OneToOneField): self.create_config_fields( - require=require, exclude=exclude, - include=include, model=model_field.related_model, relating_field=model_field, ) @@ -151,18 +171,7 @@ def create_config_fields( field_description=self.get_field_description(model_field), ) - if config_field.name in self.required_fields: - self.config_fields.required.add(config_field) - - # if all_fields is empty, that means we're filtering by blacklist, - # hence the config_field is included by default - if not self.all_fields or config_field.name in self.all_fields: - self.config_fields.all.add(config_field) - - def get_required_settings(self) -> tuple[str, ...]: - return tuple( - self.get_setting_name(field) for field in self.config_fields.required - ) + self.config_fields.append(config_field) - def get_config_mapping(self) -> dict[str, ConfigField]: - return {self.get_setting_name(field): field for field in self.config_fields.all} + def get_required_settings(self) -> list[str]: + return [self.get_setting_name(field) for field in self.config_fields.required] diff --git a/django_setup_configuration/configuration.py b/django_setup_configuration/configuration.py index dc8a4cf..6ce91d5 100644 --- a/django_setup_configuration/configuration.py +++ b/django_setup_configuration/configuration.py @@ -2,6 +2,7 @@ from django.conf import settings +from .base import ConfigSettingsModel from .exceptions import PrerequisiteFailed @@ -9,6 +10,7 @@ class BaseConfigurationStep(ABC): verbose_name: str required_settings: list[str] = [] enable_setting: str = "" + config_settings: ConfigSettingsModel def __repr__(self): return self.verbose_name diff --git a/django_setup_configuration/constants.py b/django_setup_configuration/constants.py index 13e453b..fa33504 100644 --- a/django_setup_configuration/constants.py +++ b/django_setup_configuration/constants.py @@ -1,18 +1,17 @@ -from django.contrib import postgres from django.db import models -basic_field_description = { - postgres.fields.ArrayField: "string, comma-delimited ('foo,bar,baz')", +basic_field_descriptions = { models.BooleanField: "True, False", models.CharField: "string", + models.EmailField: "string representing an Email address (foo@bar.com)", models.FileField: ( - "string represeting the (absolute) path to a file, " + "string representing the (absolute) path to a file, " "including file extension: {example}".format( example="/absolute/path/to/file.xml" ) ), models.ImageField: ( - "string represeting the (absolute) path to an image file, " + "string representing the (absolute) path to an image file, " "including file extension: {example}".format( example="/absolute/path/to/image.png" ) diff --git a/django_setup_configuration/management/commands/check_config_docs.py b/django_setup_configuration/management/commands/check_config_docs.py deleted file mode 100644 index fc7df10..0000000 --- a/django_setup_configuration/management/commands/check_config_docs.py +++ /dev/null @@ -1,43 +0,0 @@ -from django.conf import settings - -from ...exceptions import DocumentationCheckFailed -from .generate_config_docs import ConfigDocBaseCommand - - -class Command(ConfigDocBaseCommand): - help = "Check that changes to configuration setup classes are reflected in the docs" - - def check_doc(self, config_option: str) -> None: - SOURCE_DIR = settings.DJANGO_SETUP_CONFIG_DOC_DIR - - source_path = f"{SOURCE_DIR}/{config_option}.rst" - - try: - with open(source_path, "r") as file: - file_content = file.read() - except FileNotFoundError as exc: - msg = ( - "\nNo documentation was found for {config}\n" - "Did you forget to run generate_config_docs?\n".format( - config=self.get_config(config_option, class_name_only=True) - ) - ) - raise DocumentationCheckFailed(msg) from exc - else: - rendered_content = self.render_doc(config_option) - - if rendered_content != file_content: - raise DocumentationCheckFailed( - "Class {config} has changes which are not reflected in the " - "documentation ({source_path})." - "Did you forget to run generate_config_docs?\n".format( - config=self.get_config(config_option, class_name_only=True), - source_path=f"{SOURCE_DIR}/{config_option}.rst", - ) - ) - - def handle(self, *args, **kwargs) -> None: - supported_options = self.registry.config_model_keys - - for option in supported_options: - self.check_doc(option) diff --git a/django_setup_configuration/management/commands/generate_config_docs.py b/django_setup_configuration/management/commands/generate_config_docs.py index a1fa08a..88ad863 100644 --- a/django_setup_configuration/management/commands/generate_config_docs.py +++ b/django_setup_configuration/management/commands/generate_config_docs.py @@ -3,31 +3,22 @@ from django.conf import settings from django.core.management.base import BaseCommand from django.template import loader +from django.utils.module_loading import import_string from ...base import ConfigSettingsModel -from ...exceptions import ConfigurationException -from ...registry import ConfigurationRegistry -class ConfigDocBaseCommand(BaseCommand): +class ConfigDocBase: + """ + Base class encapsulating the functionality for generating + checking documentation. - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.registry = ConfigurationRegistry() + Defined independently of `BaseCommand` for more flexibility (the class could be + used without running a Django management command). + """ - def get_config( - self, config_option: str, class_name_only=False - ) -> ConfigSettingsModel: - config_model = getattr(self.registry, config_option, None) - if class_name_only: - return config_model.__name__ - - config_instance = config_model() - return config_instance - - def get_detailed_info(self, config=ConfigSettingsModel) -> list[list[str]]: + def get_detailed_info(self, config: ConfigSettingsModel) -> list[list[str]]: ret = [] - for field in config.config_fields.all: + for field in config.config_fields: part = [] part.append(f"{'Variable':<20}{config.get_setting_name(field)}") part.append(f"{'Setting':<20}{field.verbose_name}") @@ -37,77 +28,83 @@ def get_detailed_info(self, config=ConfigSettingsModel) -> list[list[str]]: ret.append(part) return ret - def format_display_name(self, display_name): + def format_display_name(self, display_name: str) -> str: """Surround title with '=' to display as heading in rst file""" heading_bar = "=" * len(display_name) display_name_formatted = f"{heading_bar}\n{display_name}\n{heading_bar}" return display_name_formatted - def render_doc(self, config_option: str) -> None: - config = self.get_config(config_option) + def render_doc(self, config_settings: ConfigSettingsModel, config_step) -> None: + # 1. + enable_setting = getattr(config_step, "enable_setting", None) - required_settings = [ - config.get_setting_name(field) for field in config.config_fields.required - ] - required_settings.sort() + # 2. + required_settings = getattr(config_step, "required_settings", None) + if required_settings: + required_settings.sort() + # 3. all_settings = [ - config.get_setting_name(field) for field in config.config_fields.all + config_settings.get_setting_name(field) + for field in config_settings.config_fields ] all_settings.sort() - detailed_info = self.get_detailed_info(config) + # 4. + detailed_info = self.get_detailed_info(config_settings) detailed_info.sort() + # 5. + title = self.format_display_name(config_step.verbose_name) + template_variables = { - "enable_settings": f"{config.namespace}_CONFIG_ENABLE", + "enable_settings": enable_setting, "required_settings": required_settings, "all_settings": all_settings, "detailed_info": detailed_info, - "link": f".. _{config_option}:", - "title": self.format_display_name(config.display_name), + "link": f".. _{config_settings.file_name}:", + "title": title, } - template = loader.get_template(settings.DJANGO_SETUP_CONFIG_TEMPLATE_NAME) + template = loader.get_template(settings.DJANGO_SETUP_CONFIG_TEMPLATE) rendered = template.render(template_variables) return rendered -class Command(ConfigDocBaseCommand): - help = "Create documentation for configuration setup steps" +class Command(ConfigDocBase, BaseCommand): + help = "Generate documentation for configuration setup steps" - def add_arguments(self, parser): - parser.add_argument("config_option", nargs="?") + def content_is_up_to_date(self, rendered_content: str, doc_path: str) -> bool: + """ + Check that documentation at `doc_path` exists and that its content matches + that of `rendered_content` + """ + try: + with open(doc_path, "r") as file: + file_content = file.read() + except FileNotFoundError: + return False - def write_doc(self, config_option: str) -> None: - rendered = self.render_doc(config_option) + if not file_content == rendered_content: + return False - TARGET_DIR = settings.DJANGO_SETUP_CONFIG_DOC_DIR + return True - pathlib.Path(TARGET_DIR).mkdir(parents=True, exist_ok=True) + def handle(self, *args, **kwargs) -> str: + target_dir = settings.DJANGO_SETUP_CONFIG_DOC_PATH - output_path = f"{TARGET_DIR}/{config_option}.rst" + # create directory for docs if it doesn't exist + pathlib.Path(target_dir).mkdir(parents=True, exist_ok=True) - with open(output_path, "w+") as output: - output.write(rendered) + for config_string in settings.SETUP_CONFIGURATION_STEPS: + config_step = import_string(config_string) - return rendered + if config_settings := getattr(config_step, "config_settings", None): + doc_path = f"{target_dir}/{config_settings.file_name}.rst" + rendered_content = self.render_doc(config_settings, config_step) - def handle(self, *args, **kwargs) -> None: - config_option = kwargs["config_option"] - - supported_options = self.registry.config_model_keys - - if config_option and config_option not in supported_options: - raise ConfigurationException( - f"Unsupported config option ({config_option})\n" - f"Supported: {', '.join(supported_options)}" - ) - elif config_option: - rendered = self.write_doc(config_option) - else: - for option in supported_options: - rendered = self.write_doc(option) - return rendered + if not self.content_is_up_to_date(rendered_content, doc_path): + with open(doc_path, "w+") as output: + output.write(rendered_content) diff --git a/django_setup_configuration/registry.py b/django_setup_configuration/registry.py deleted file mode 100644 index 0c23ad0..0000000 --- a/django_setup_configuration/registry.py +++ /dev/null @@ -1,48 +0,0 @@ -from django.conf import settings -from django.utils.module_loading import import_string - -from .base import ConfigSettingsModel -from .exceptions import ImproperlyConfigured - - -class ConfigurationRegistry: - def __init__(self): - if not getattr(settings, "DJANGO_SETUP_CONFIG_REGISTER", None): - raise ImproperlyConfigured("DJANGO_SETUP_CONFIG_REGISTER is not defined") - - if not all( - (entry.get("model") for entry in settings.DJANGO_SETUP_CONFIG_REGISTER) - ): - raise ImproperlyConfigured( - "Each entry for the DJANGO_SETUP_CONFIG_REGISTER setting " - "must specify a configuration model" - ) - - self.register_config_models() - - def register_config_models(self) -> None: - """ - Load config models specified in settings and set them as attributes on - the instance - """ - for mapping in settings.DJANGO_SETUP_CONFIG_REGISTER: - file_name = mapping.get("file_name") or mapping["model"].split(".")[-1] - - try: - model = import_string(mapping["model"]) - except ImportError as exc: - raise ImproperlyConfigured( - "\n\nThe class testapp.models.bogus was not found\n" - "Check the DJANGO_SETUP_CONFIG_REGISTER setting for " - "django-setup-configuration" - ) from exc - else: - setattr(self, file_name, model) - - @property - def config_models(self) -> tuple[ConfigSettingsModel, ...]: - return tuple(getattr(self, key) for key in vars(self).keys()) - - @property - def config_model_keys(self) -> tuple[str, ...]: - return tuple(key for key in vars(self).keys()) diff --git a/django_setup_configuration/templates/django_setup_configuration/config_doc.rst b/django_setup_configuration/templates/django_setup_configuration/config_doc.rst new file mode 100644 index 0000000..48deaf8 --- /dev/null +++ b/django_setup_configuration/templates/django_setup_configuration/config_doc.rst @@ -0,0 +1,50 @@ +{% block link %}{{ link }}{% endblock %} + +{% block title %}{{ title }}{% endblock %} + +Settings Overview +================= + +Enable/Disable configuration: +""""""""""""""""""""""""""""" + +{% if required_settings %} +:: + + {% spaceless %} + {{ enable_settings }} + {% endspaceless %} +{% endif %} + +{% if required_settings %} +Required: +""""""""" + +:: + + {% spaceless %} + {% for setting in required_settings %}{{ setting }} + {% endfor %} + {% endspaceless %} +{% endif %} + +All settings: +""""""""""""" + +:: + + {% spaceless %} + {% for setting in all_settings %}{{ setting }} + {% endfor %} + {% endspaceless %} + +Detailed Information +==================== + +:: + + {% spaceless %} + {% for detail in detailed_info %} + {% for part in detail %}{{ part|safe }} + {% endfor %}{% endfor %} + {% endspaceless %} diff --git a/pyproject.toml b/pyproject.toml index 0f78018..fadbcca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,7 @@ tests = [ "isort", "black", "flake8", + "python-decouple" ] coverage = [ "pytest-cov", diff --git a/setup.cfg b/setup.cfg index 8c5866b..343df37 100644 --- a/setup.cfg +++ b/setup.cfg @@ -2,4 +2,4 @@ [flake8] max-line-length=88 -exclude=env,.tox,doc +exclude=env,.tox,doc,mocks.py diff --git a/testapp/config_models.py b/testapp/config_models.py deleted file mode 100644 index 3ab418c..0000000 --- a/testapp/config_models.py +++ /dev/null @@ -1,34 +0,0 @@ -from django.contrib.postgres.fields import ArrayField -from django.db import models - - -class Service(models.Model): - url = models.URLField( - verbose_name="Service url", - help_text="The url of the service", - ) - bogus = models.CharField( - verbose_name="Bogus service field", help_text="Should not be included in docs" - ) - - -class ProductConfig(models.Model): - name = models.CharField( - verbose_name="Name", - help_text="The name of the product", - ) - service = models.OneToOneField( - to=Service, - verbose_name="Service", - default=None, - on_delete=models.SET_NULL, - help_text="API service of the product", - ) - tags = ArrayField( - base_field=models.CharField("Product tag"), - default=["example_tag"], - help_text="Tags for the product", - ) - bogus = models.CharField( - help_text="Should be excluded", - ) diff --git a/testapp/configuration.py b/testapp/configuration.py index 1a6fb4b..aef0fb1 100644 --- a/testapp/configuration.py +++ b/testapp/configuration.py @@ -6,16 +6,6 @@ from django_setup_configuration.configuration import BaseConfigurationStep from django_setup_configuration.exceptions import SelfTestFailed -from .config_models import ProductConfig - - -class ProductConfigurationSettings(ConfigSettingsModel): - model = ProductConfig - display_name = "Product Configuration" - namespace = "PRODUCT" - required_fields = ("name", "service_url") - all_fields = required_fields + ("tags",) - class UserConfigurationStep(BaseConfigurationStep): """ @@ -26,8 +16,17 @@ class UserConfigurationStep(BaseConfigurationStep): """ verbose_name = "User Configuration" - required_settings = ["USER_CONFIGURATION_USERNAME", "USER_CONFIGURATION_PASSWORD"] enable_setting = "USER_CONFIGURATION_ENABLED" + required_settings = [ + "USER_CONFIGURATION_USERNAME", + "USER_CONFIGURATION_PASSWORD", + ] + config_settings = ConfigSettingsModel( + models=[User], + file_name="user", + namespace="USER_CONFIGURATION", + excluded_fields=["id", "date_joined", "is_active", "last_login"], + ) def is_configured(self) -> bool: return User.objects.filter( diff --git a/testapp/docs/configuration/product.rst b/testapp/docs/configuration/product.rst deleted file mode 100644 index b242abf..0000000 --- a/testapp/docs/configuration/product.rst +++ /dev/null @@ -1,55 +0,0 @@ -.. _product: - -===================== -Product Configuration -===================== - -Settings Overview -================= - -Enable/Disable configuration: -""""""""""""""""""""""""""""" - -:: - - PRODUCT_CONFIG_ENABLE - -Required: -""""""""" - -:: - - PRODUCT_NAME - PRODUCT_SERVICE_URL - -All settings: -""""""""""""" - -:: - - PRODUCT_NAME - PRODUCT_SERVICE_URL - PRODUCT_TAGS - -Detailed Information -==================== - -:: - - Variable PRODUCT_NAME - Setting Name - Description The name of the product - Possible values string - Default value No default - - Variable PRODUCT_SERVICE_URL - Setting Service url - Description The url of the service - Possible values string (URL) - Default value No default - - Variable PRODUCT_TAGS - Setting tags - Description Tags for the product - Possible values string, comma-delimited ('foo,bar,baz') - Default value example_tag diff --git a/testapp/settings.py b/testapp/settings.py index dc3b19b..9a4fb68 100644 --- a/testapp/settings.py +++ b/testapp/settings.py @@ -1,6 +1,8 @@ import os from pathlib import Path +from decouple import config + BASE_DIR = Path(__file__).resolve(strict=True).parent SECRET_KEY = "so-secret-i-cant-believe-you-are-looking-at-this" @@ -13,7 +15,8 @@ "NAME": "postgres", "USER": "postgres", "PASSWORD": "postgres", - "HOST": "localhost", + # set DB_HOST="" in .env to connect via socket + "HOST": config("DB_HOST", "localhost"), } } @@ -68,11 +71,5 @@ USER_CONFIGURATION_USERNAME = os.getenv("USER_CONFIGURATION_USERNAME", "demo") USER_CONFIGURATION_PASSWORD = os.getenv("USER_CONFIGURATION_PASSWORD", "secret") -DJANGO_SETUP_CONFIG_REGISTER = [ - { - "model": "testapp.configuration.ProductConfigurationSettings", - "file_name": "product", - } -] -DJANGO_SETUP_CONFIG_TEMPLATE_NAME = "testapp/config_doc.rst" -DJANGO_SETUP_CONFIG_DOC_DIR = "testapp/docs/configuration" +DJANGO_SETUP_CONFIG_TEMPLATE = "testapp/config_doc.rst" +DJANGO_SETUP_CONFIG_DOC_PATH = "testapp/docs/configuration" diff --git a/testapp/templates/testapp/config_doc.rst b/testapp/templates/testapp/config_doc.rst index b9de9c5..5c3ce56 100644 --- a/testapp/templates/testapp/config_doc.rst +++ b/testapp/templates/testapp/config_doc.rst @@ -14,6 +14,7 @@ Enable/Disable configuration: {{ enable_settings }} {% endspaceless %} +{% if required_settings %} Required: """"""""" @@ -23,6 +24,7 @@ Required: {% for setting in required_settings %}{{ setting }} {% endfor %} {% endspaceless %} +{% endif %} All settings: """"""""""""" diff --git a/tests/mocks.py b/tests/mocks.py index 0d02d68..b352a3f 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -1 +1,2 @@ -mock_product_doc = '.. _product:\n\n=====================\nProduct Configuration\n=====================\n\nSettings Overview\n=================\n\nEnable/Disable configuration:\n"""""""""""""""""""""""""""""\n\n::\n\n PRODUCT_CONFIG_ENABLE\n\nRequired:\n"""""""""\n\n::\n\n PRODUCT_NAME\n PRODUCT_SERVICE_URL\n\nAll settings:\n"""""""""""""\n\n::\n\n PRODUCT_NAME\n PRODUCT_SERVICE_URL\n PRODUCT_TAGS\n\nDetailed Information\n====================\n\n::\n\n Variable PRODUCT_NAME\n Setting Name\n Description The name of the product\n Possible values string\n Default value No default\n \n Variable PRODUCT_SERVICE_URL\n Setting Service url\n Description The url of the service\n Possible values string (URL)\n Default value No default\n \n Variable PRODUCT_TAGS\n Setting tags\n Description Tags for the product\n Possible values string, comma-delimited (\'foo,bar,baz\')\n Default value example_tag\n' # noqa +mock_user_doc = '.. _user:\n\n==================\nUser Configuration\n==================\n\nSettings Overview\n=================\n\nEnable/Disable configuration:\n"""""""""""""""""""""""""""""\n\n::\n\n USER_CONFIGURATION_ENABLED\n\n\nRequired:\n"""""""""\n\n::\n\n USER_CONFIGURATION_PASSWORD\n USER_CONFIGURATION_USERNAME\n\n\nAll settings:\n"""""""""""""\n\n::\n\n USER_CONFIGURATION_EMAIL\n USER_CONFIGURATION_FIRST_NAME\n USER_CONFIGURATION_IS_STAFF\n USER_CONFIGURATION_IS_SUPERUSER\n USER_CONFIGURATION_LAST_NAME\n USER_CONFIGURATION_PASSWORD\n USER_CONFIGURATION_USERNAME\n\nDetailed Information\n====================\n\n::\n\n Variable USER_CONFIGURATION_EMAIL\n Setting email address\n Description No description\n Possible values string representing an Email address (foo@bar.com)\n Default value No default\n \n Variable USER_CONFIGURATION_FIRST_NAME\n Setting first name\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_IS_STAFF\n Setting staff status\n Description Designates whether the user can log into this admin site.\n Possible values True, False\n Default value False\n \n Variable USER_CONFIGURATION_IS_SUPERUSER\n Setting superuser status\n Description Designates that this user has all permissions without explicitly assigning them.\n Possible values True, False\n Default value False\n \n Variable USER_CONFIGURATION_LAST_NAME\n Setting last name\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_PASSWORD\n Setting password\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_USERNAME\n Setting username\n Description Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.\n Possible values string\n Default value No default\n' # noqua +mock_user_doc_mismatch = "hello world" diff --git a/tests/test_config_docs.py b/tests/test_config_docs.py index 2cf3999..7a3b992 100644 --- a/tests/test_config_docs.py +++ b/tests/test_config_docs.py @@ -2,49 +2,48 @@ from django.core.management import call_command -import pytest +from .mocks import mock_user_doc, mock_user_doc_mismatch -from django_setup_configuration.exceptions import DocumentationCheckFailed +open_func = "django_setup_configuration.management.commands.generate_config_docs.open" -from .mocks import mock_product_doc +def test_generate_config_docs_new_file(settings): + """ + Assert that file with correct content is written if no docs exist + """ + open_mock = mock.mock_open() -def test_generate_config_docs(settings): - content = call_command("generate_config_docs") + with mock.patch(open_func, open_mock): + call_command("generate_config_docs") - assert "PRODUCT_CONFIG_ENABLE" in content - # 3 occurrences of PRODUCT_NAME: required, all settings, description - assert content.count("PRODUCT_NAME") == 3 + open_mock.assert_called_with("testapp/docs/configuration/user.rst", "w+") + open_mock.return_value.write.assert_called_once_with(mock_user_doc) - assert "PRODUCT_SERVICE_URL" in content - # 3 occurrences of PRODUCT_SERVICE_URL: required, all settings, description - assert content.count("PRODUCT_SERVICE_URL") == 3 - assert "PRODUCT_TAGS" in content - # 2 occurrences of PRODUCT_TAGS: all settings, description - assert content.count("PRODUCT_TAGS") == 2 +def test_generate_config_docs_content_mismatch(settings): + """ + Assert that file with updated content is written if the content read by `open` + is different + """ + open_mock = mock.mock_open(read_data=mock_user_doc_mismatch) - assert "PRODUCT_BOGUS" not in content - assert "PRODUCT_SERVICE_BOGUS" not in content + with mock.patch(open_func, open_mock): + call_command("generate_config_docs") + open_mock.assert_called_with("testapp/docs/configuration/user.rst", "w+") + open_mock.return_value.write.assert_called_once_with(mock_user_doc) -def test_check_config_docs_ok(settings): - with mock.patch("builtins.open", mock.mock_open(read_data=mock_product_doc)): - call_command("check_config_docs") +def test_docs_up_to_date(settings): + """ + Assert that no file is written if the content read by `open` is up to date + """ + open_mock = mock.mock_open(read_data=mock_user_doc) -def test_check_config_docs_fail_missing_docs(settings): - mock_open = mock.mock_open(read_data=mock_product_doc) - mock_open.side_effect = FileNotFoundError + with mock.patch(open_func, open_mock): + call_command("generate_config_docs") - with mock.patch("builtins.open", mock_open): - with pytest.raises(DocumentationCheckFailed): - call_command("check_config_docs") - - -@mock.patch("testapp.configuration.ProductConfigurationSettings.get_setting_name") -def test_check_config_docs_fail_content_mismatch(m, settings): - m.return_value = "" - - with pytest.raises(DocumentationCheckFailed): - call_command("check_config_docs") + assert ( + mock.call("testapp/docs/configuration/user.rst", "w+") + not in open_mock.mock_calls + ) diff --git a/tests/test_config_registry.py b/tests/test_config_registry.py deleted file mode 100644 index 6b11868..0000000 --- a/tests/test_config_registry.py +++ /dev/null @@ -1,46 +0,0 @@ -from django.core.management import call_command - -import pytest - -from django_setup_configuration.exceptions import ImproperlyConfigured -from django_setup_configuration.registry import ConfigurationRegistry -from testapp.configuration import ProductConfigurationSettings - - -def test_missing_registry(settings): - del settings.DJANGO_SETUP_CONFIG_REGISTER - - with pytest.raises(ImproperlyConfigured): - call_command("generate_config_docs") - - -def test_registry_missing_model_spec(settings): - settings.DJANGO_SETUP_CONFIG_REGISTER = [{"file_name": "test"}] - - with pytest.raises(ImproperlyConfigured): - call_command("generate_config_docs") - - -def test_registry_model_not_found(settings): - settings.DJANGO_SETUP_CONFIG_REGISTER = [{"model": "testapp.models.bogus"}] - - with pytest.raises(ImproperlyConfigured): - call_command("generate_config_docs") - - -def test_registry_property_models(): - registry = ConfigurationRegistry() - - models = registry.config_models - - assert len(models) == 1 - assert models[0] == ProductConfigurationSettings - - -def test_registry_property_model_keys(): - registry = ConfigurationRegistry() - - model_keys = registry.config_model_keys - - assert len(model_keys) == 1 - assert model_keys[0] == "product" From 33a4532d5634bcbb2cd68b87e68bbe0ea7058d67 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Thu, 23 May 2024 10:48:14 +0200 Subject: [PATCH 5/7] [#8] Refactor ConfigSettings and management command - replace blacklist (`exclude`) with explicit whitelist for determining target model fields - avoid use of private Django API for skipping relational fields (made possible by avoiding blacklist) - add support for manually adding documentation for settings which are not associated with any model field --- django_setup_configuration/__init__.py | 4 +- .../{base.py => config_settings.py} | 144 ++++++++++++------ django_setup_configuration/configuration.py | 6 +- .../commands/generate_config_docs.py | 86 +++++++++-- .../django_setup_configuration/config_doc.rst | 4 +- testapp/configuration.py | 20 ++- tests/mocks.py | 2 +- 7 files changed, 193 insertions(+), 73 deletions(-) rename django_setup_configuration/{base.py => config_settings.py} (52%) diff --git a/django_setup_configuration/__init__.py b/django_setup_configuration/__init__.py index 4807d27..588b1bb 100644 --- a/django_setup_configuration/__init__.py +++ b/django_setup_configuration/__init__.py @@ -1,5 +1,5 @@ -from .base import ConfigSettingsModel +from .config_settings import ConfigSettings __all__ = [ - "ConfigSettingsModel", + "ConfigSettings", ] diff --git a/django_setup_configuration/base.py b/django_setup_configuration/config_settings.py similarity index 52% rename from django_setup_configuration/base.py rename to django_setup_configuration/config_settings.py index 1ba6202..0e4b344 100644 --- a/django_setup_configuration/base.py +++ b/django_setup_configuration/config_settings.py @@ -1,12 +1,12 @@ from dataclasses import dataclass -from typing import Iterator, Mapping, Sequence, Type +from typing import Mapping, Sequence, Type from django.conf import settings from django.contrib.postgres.fields import ArrayField from django.db import models from django.db.models.fields import NOT_PROVIDED from django.db.models.fields.json import JSONField -from django.db.models.fields.related import OneToOneField +from django.db.models.fields.related import ForeignKey, OneToOneField from django.utils.module_loading import import_string from .constants import basic_field_descriptions @@ -22,31 +22,74 @@ class ConfigField: field_description: str -class ConfigSettingsModel: - models: list[Type[models.Model]] - display_name: str +class ConfigSettings: + """ + Settings for configuration steps, used to generate documentation. + + Attributes: + namespace (`str`): the namespace of configuration variables for a given + configuration + file_name (`str`): the name of the file where the documentation is stored + models (`list`): a list of models from which documentation is retrieved + update_field_descriptions (`bool`): if `True`, custom model fields + (along with their descriptions) are loaded via the settings variable + `DJANGO_SETUP_CONFIG_CUSTOM_FIELDS` + required_settings (`list`): required settings for a configuration step + optional_settings (`list`): optional settings for a configuration step + detailed_info (`dict`): information for configuration settings which are + not associated with a particular model field + + Example: + Given a configuration step `FooConfigurationStep`: :: + + FooConfigurationStep(BaseConfigurationStep): + verbose_name = "Configuration step for Foo" + enable_setting = "FOO_CONFIG_ENABLE" + config_settings = ConfigSettings( + namespace="FOO", + file_name="foo", + models=["FooConfigurationModel"], + required_settings=[ + "FOO_SOME_SETTING", + "FOO_SOME_OTHER_SETTING", + ], + optional_settings=[ + "FOO_SOME_OPT_SETTING", + "FOO_SOME_OTHER_OPT_SETTING", + ], + detailed_info={ + "example_non_model_field": { + "variable": "FOO_EXAMPLE_NON_MODEL_FIELD", + "description": "Documentation for a field that could not + be retrievend from a model", + "possible_values": "string (URL)", + }, + }, + ) + """ + namespace: str - excluded_fields = ["id"] + file_name: str + models: list[Type[models.Model]] | None + required_settings: list[str] = [] + optional_settings: list[str] = [] + detailed_info: dict[str, dict[str, str]] | None - def __init__(self, *args, **kwargs): - self.config_fields: list = [] + def __init__(self, *args, update_field_descriptions: bool = False, **kwargs): + self.config_fields: list[ConfigField] = [] for key, value in kwargs.items(): setattr(self, key, value) - self.update_field_descriptions() - - if not self.models: + if not getattr(self, "models", None): return - for model in self.models: - self.create_config_fields( - exclude=self.excluded_fields, - model=model, - ) + # add support for custom fields like PrivateMediaField + if update_field_descriptions: + self.update_field_descriptions() - def get_setting_name(self, field: ConfigField) -> str: - return f"{self.namespace}_" + field.name.upper() + for model in self.models: + self.create_config_fields(model=model) @staticmethod def get_default_value(field: models.Field) -> str: @@ -113,22 +156,11 @@ def get_field_description(field: models.Field) -> str: field_type = type(field) if field_type in basic_field_descriptions.keys(): return basic_field_descriptions.get(field_type) - return "No information available" - def get_concrete_model_fields(self, model) -> Iterator[models.Field]: - """ - Get all concrete fields for a given `model`, skipping over backreferences like - `OneToOneRel` and fields that are blacklisted - """ - return ( - field - for field in model._meta.concrete_fields - if field.name not in self.excluded_fields - ) + return "No information available" def create_config_fields( self, - exclude: list[str], model: Type[models.Model], relating_field: models.Field | None = None, ) -> None: @@ -137,41 +169,59 @@ def create_config_fields( add it to `self.fields` Basic fields (`CharField`, `IntegerField` etc) constitute the base case, - one-to-one relations (`OneToOneField`) are handled recursively - - `ForeignKey` and `ManyToManyField` are currently not supported (these require - special care to avoid recursion errors) + relations (`ForeignKey`, `OneToOneField`) are handled recursively """ - model_fields = self.get_concrete_model_fields(model) + for model_field in model._meta.fields: + if isinstance(model_field, (ForeignKey, OneToOneField)): + # avoid recursion error when following ForeignKey + if model_field.name in ("parent", "owner"): + continue - for model_field in model_fields: - if isinstance(model_field, OneToOneField): self.create_config_fields( - exclude=exclude, model=model_field.related_model, relating_field=model_field, ) else: - if model_field.name in self.excluded_fields: - continue - # model field name could be "api_root", # but we need "xyz_service_api_root" (or similar) for consistency + # when dealing with relations if relating_field: - name = f"{relating_field.name}_{model_field.name}" + config_field_name = f"{relating_field.name}_{model_field.name}" else: - name = model_field.name + config_field_name = model_field.name + + config_setting = self.get_config_variable(config_field_name) + + if not ( + config_setting in self.required_settings + or config_setting in self.optional_settings + ): + continue config_field = ConfigField( - name=name, + name=config_field_name, verbose_name=model_field.verbose_name, description=model_field.help_text, default_value=self.get_default_value(model_field), field_description=self.get_field_description(model_field), ) - self.config_fields.append(config_field) - def get_required_settings(self) -> list[str]: - return [self.get_setting_name(field) for field in self.config_fields.required] + # + # convenience methods/properties for formatting + # + def get_config_variable(self, setting: str) -> str: + return f"{self.namespace}_" + setting.upper() + + @property + def file_name(self) -> str: + """ + Use `self.namespace` in lower case as default file name of the documentation + if `file_name` is not provided when instantiating the class + """ + return getattr(self, "_file_name", None) or self.namespace.lower() + + @file_name.setter + def file_name(self, val) -> None: + self._file_name = val diff --git a/django_setup_configuration/configuration.py b/django_setup_configuration/configuration.py index 6ce91d5..4c94ada 100644 --- a/django_setup_configuration/configuration.py +++ b/django_setup_configuration/configuration.py @@ -2,7 +2,7 @@ from django.conf import settings -from .base import ConfigSettingsModel +from .config_settings import ConfigSettings from .exceptions import PrerequisiteFailed @@ -10,7 +10,7 @@ class BaseConfigurationStep(ABC): verbose_name: str required_settings: list[str] = [] enable_setting: str = "" - config_settings: ConfigSettingsModel + config_settings: ConfigSettings def __repr__(self): return self.verbose_name @@ -24,7 +24,7 @@ def validate_requirements(self) -> None: """ missing = [ var - for var in self.required_settings + for var in self.config_settings.required_settings if getattr(settings, var, None) in [None, ""] ] if missing: diff --git a/django_setup_configuration/management/commands/generate_config_docs.py b/django_setup_configuration/management/commands/generate_config_docs.py index 88ad863..00e8afc 100644 --- a/django_setup_configuration/management/commands/generate_config_docs.py +++ b/django_setup_configuration/management/commands/generate_config_docs.py @@ -5,7 +5,7 @@ from django.template import loader from django.utils.module_loading import import_string -from ...base import ConfigSettingsModel +from ...config_settings import ConfigSettings class ConfigDocBase: @@ -16,16 +16,56 @@ class ConfigDocBase: used without running a Django management command). """ - def get_detailed_info(self, config: ConfigSettingsModel) -> list[list[str]]: + @staticmethod + def _add_detailed_info(config_settings: ConfigSettings, result: list[str]) -> None: + """Convenience/helper function to retrieve additional documentation info""" + + if not (info := getattr(config_settings, "detailed_info", None)): + return + + for key, value in info.items(): + part = [] + part.append(f"{'Variable':<20}{value['variable']}") + part.append( + f"{'Description':<20}{value['description'] or 'No description'}" + ) + part.append( + f"{'Possible values':<20}" + f"{value.get('possible_values') or 'No information available'}" + ) + part.append( + f"{'Default value':<20}{value.get('default_value') or 'No default'}" + ) + result.append(part) + + def get_detailed_info( + self, + config: ConfigSettings, + config_step, + related_steps: list, + ) -> list[list[str]]: + """ + Get information about the configuration settings: + 1. from model fields associated with the `ConfigSettings` + 2. from information provided manually in the `ConfigSettings` + 3. from information provided manually in the `ConfigSettings` of related + configuration steps + """ ret = [] for field in config.config_fields: part = [] - part.append(f"{'Variable':<20}{config.get_setting_name(field)}") + part.append(f"{'Variable':<20}{config.get_config_variable(field.name)}") part.append(f"{'Setting':<20}{field.verbose_name}") part.append(f"{'Description':<20}{field.description or 'No description'}") part.append(f"{'Possible values':<20}{field.field_description}") part.append(f"{'Default value':<20}{field.default_value}") ret.append(part) + + self._add_detailed_info(config, ret) + + for step in related_steps: + self._add_detailed_info(step.config_settings, ret) + return ret def format_display_name(self, display_name: str) -> str: @@ -35,31 +75,55 @@ def format_display_name(self, display_name: str) -> str: display_name_formatted = f"{heading_bar}\n{display_name}\n{heading_bar}" return display_name_formatted - def render_doc(self, config_settings: ConfigSettingsModel, config_step) -> None: + def render_doc(self, config_settings: ConfigSettings, config_step) -> None: + """ + Render a `ConfigSettings` documentation template with the following variables: + 1. enable_setting + 2. required_settings + 3. all_settings (required_settings + optional_settings) + 4. detailed_info + 5. title + 6. link (for crossreference across different files) + """ # 1. enable_setting = getattr(config_step, "enable_setting", None) # 2. - required_settings = getattr(config_step, "required_settings", None) - if required_settings: - required_settings.sort() + required_settings = [ + name for name in getattr(config_settings, "required_settings", []) + ] + + # additional requirements from related configuration steps + related_steps = [step for step in getattr(config_step, "related_steps", [])] + related_requirements_lists = [ + step.config_settings.required_settings for step in related_steps + ] + related_requirements = set( + item for row in related_requirements_lists for item in row + ) + + required_settings.extend(list(related_requirements)) + required_settings.sort() # 3. all_settings = [ - config_settings.get_setting_name(field) - for field in config_settings.config_fields + setting + for setting in config_settings.required_settings + + config_settings.optional_settings ] all_settings.sort() # 4. - detailed_info = self.get_detailed_info(config_settings) + detailed_info = self.get_detailed_info( + config_settings, config_step, related_steps + ) detailed_info.sort() # 5. title = self.format_display_name(config_step.verbose_name) template_variables = { - "enable_settings": enable_setting, + "enable_setting": enable_setting, "required_settings": required_settings, "all_settings": all_settings, "detailed_info": detailed_info, diff --git a/django_setup_configuration/templates/django_setup_configuration/config_doc.rst b/django_setup_configuration/templates/django_setup_configuration/config_doc.rst index 48deaf8..ae6d494 100644 --- a/django_setup_configuration/templates/django_setup_configuration/config_doc.rst +++ b/django_setup_configuration/templates/django_setup_configuration/config_doc.rst @@ -5,14 +5,14 @@ Settings Overview ================= +{% if enable_setting %} Enable/Disable configuration: """"""""""""""""""""""""""""" -{% if required_settings %} :: {% spaceless %} - {{ enable_settings }} + {{ enable_setting }} {% endspaceless %} {% endif %} diff --git a/testapp/configuration.py b/testapp/configuration.py index aef0fb1..eac493b 100644 --- a/testapp/configuration.py +++ b/testapp/configuration.py @@ -2,7 +2,7 @@ from django.contrib.auth import authenticate from django.contrib.auth.models import User -from django_setup_configuration.base import ConfigSettingsModel +from django_setup_configuration.config_settings import ConfigSettings from django_setup_configuration.configuration import BaseConfigurationStep from django_setup_configuration.exceptions import SelfTestFailed @@ -17,15 +17,21 @@ class UserConfigurationStep(BaseConfigurationStep): verbose_name = "User Configuration" enable_setting = "USER_CONFIGURATION_ENABLED" - required_settings = [ - "USER_CONFIGURATION_USERNAME", - "USER_CONFIGURATION_PASSWORD", - ] - config_settings = ConfigSettingsModel( + config_settings = ConfigSettings( models=[User], file_name="user", namespace="USER_CONFIGURATION", - excluded_fields=["id", "date_joined", "is_active", "last_login"], + required_settings=[ + "USER_CONFIGURATION_USERNAME", + "USER_CONFIGURATION_PASSWORD", + ], + optional_settings=[ + "USER_CONFIGURATION_EMAIL", + "USER_CONFIGURATION_FIRST_NAME", + "USER_CONFIGURATION_LAST_NAME", + "USER_CONFIGURATION_IS_SUPERUSER", + "USER_CONFIGURATION_IS_STAFF", + ], ) def is_configured(self) -> bool: diff --git a/tests/mocks.py b/tests/mocks.py index b352a3f..b2a99aa 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -1,2 +1,2 @@ -mock_user_doc = '.. _user:\n\n==================\nUser Configuration\n==================\n\nSettings Overview\n=================\n\nEnable/Disable configuration:\n"""""""""""""""""""""""""""""\n\n::\n\n USER_CONFIGURATION_ENABLED\n\n\nRequired:\n"""""""""\n\n::\n\n USER_CONFIGURATION_PASSWORD\n USER_CONFIGURATION_USERNAME\n\n\nAll settings:\n"""""""""""""\n\n::\n\n USER_CONFIGURATION_EMAIL\n USER_CONFIGURATION_FIRST_NAME\n USER_CONFIGURATION_IS_STAFF\n USER_CONFIGURATION_IS_SUPERUSER\n USER_CONFIGURATION_LAST_NAME\n USER_CONFIGURATION_PASSWORD\n USER_CONFIGURATION_USERNAME\n\nDetailed Information\n====================\n\n::\n\n Variable USER_CONFIGURATION_EMAIL\n Setting email address\n Description No description\n Possible values string representing an Email address (foo@bar.com)\n Default value No default\n \n Variable USER_CONFIGURATION_FIRST_NAME\n Setting first name\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_IS_STAFF\n Setting staff status\n Description Designates whether the user can log into this admin site.\n Possible values True, False\n Default value False\n \n Variable USER_CONFIGURATION_IS_SUPERUSER\n Setting superuser status\n Description Designates that this user has all permissions without explicitly assigning them.\n Possible values True, False\n Default value False\n \n Variable USER_CONFIGURATION_LAST_NAME\n Setting last name\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_PASSWORD\n Setting password\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_USERNAME\n Setting username\n Description Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.\n Possible values string\n Default value No default\n' # noqua +mock_user_doc = '.. _user:\n\n==================\nUser Configuration\n==================\n\nSettings Overview\n=================\n\nEnable/Disable configuration:\n"""""""""""""""""""""""""""""\n\n::\n\n \n\n\nRequired:\n"""""""""\n\n::\n\n USER_CONFIGURATION_PASSWORD\n USER_CONFIGURATION_USERNAME\n\n\nAll settings:\n"""""""""""""\n\n::\n\n USER_CONFIGURATION_EMAIL\n USER_CONFIGURATION_FIRST_NAME\n USER_CONFIGURATION_IS_STAFF\n USER_CONFIGURATION_IS_SUPERUSER\n USER_CONFIGURATION_LAST_NAME\n USER_CONFIGURATION_PASSWORD\n USER_CONFIGURATION_USERNAME\n\nDetailed Information\n====================\n\n::\n\n Variable USER_CONFIGURATION_EMAIL\n Setting email address\n Description No description\n Possible values string representing an Email address (foo@bar.com)\n Default value No default\n \n Variable USER_CONFIGURATION_FIRST_NAME\n Setting first name\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_IS_STAFF\n Setting staff status\n Description Designates whether the user can log into this admin site.\n Possible values True, False\n Default value False\n \n Variable USER_CONFIGURATION_IS_SUPERUSER\n Setting superuser status\n Description Designates that this user has all permissions without explicitly assigning them.\n Possible values True, False\n Default value False\n \n Variable USER_CONFIGURATION_LAST_NAME\n Setting last name\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_PASSWORD\n Setting password\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_USERNAME\n Setting username\n Description Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.\n Possible values string\n Default value No default\n' # noqua mock_user_doc_mismatch = "hello world" From 9911b821f2321f0073a3eb8918d7dd4fd5ed66a3 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Fri, 28 Jun 2024 11:52:16 +0200 Subject: [PATCH 6/7] Update documentation, type annotations + other PR feedback --- django_setup_configuration/config_settings.py | 69 +++---- django_setup_configuration/configuration.py | 6 +- django_setup_configuration/constants.py | 12 +- .../commands/generate_config_docs.py | 20 +- docs/config_docs.rst | 179 +++++++----------- testapp/configuration.py | 6 +- 6 files changed, 120 insertions(+), 172 deletions(-) diff --git a/django_setup_configuration/config_settings.py b/django_setup_configuration/config_settings.py index 0e4b344..b7ce0dc 100644 --- a/django_setup_configuration/config_settings.py +++ b/django_setup_configuration/config_settings.py @@ -24,28 +24,32 @@ class ConfigField: class ConfigSettings: """ - Settings for configuration steps, used to generate documentation. + Settings for configuration steps, also used to generate documentation. Attributes: + enable_setting (`str`): the setting for enabling the associated configuration + step namespace (`str`): the namespace of configuration variables for a given configuration file_name (`str`): the name of the file where the documentation is stored models (`list`): a list of models from which documentation is retrieved + required_settings (`list`): required settings for a configuration step + optional_settings (`list`): optional settings for a configuration step update_field_descriptions (`bool`): if `True`, custom model fields (along with their descriptions) are loaded via the settings variable `DJANGO_SETUP_CONFIG_CUSTOM_FIELDS` - required_settings (`list`): required settings for a configuration step - optional_settings (`list`): optional settings for a configuration step - detailed_info (`dict`): information for configuration settings which are + additional_info (`dict`): information for configuration settings which are not associated with a particular model field + config_fields (`list`): a list of `ConfigField` objects containing information + about Django model fields Example: Given a configuration step `FooConfigurationStep`: :: FooConfigurationStep(BaseConfigurationStep): verbose_name = "Configuration step for Foo" - enable_setting = "FOO_CONFIG_ENABLE" config_settings = ConfigSettings( + enable_setting = "FOO_CONFIG_ENABLE" namespace="FOO", file_name="foo", models=["FooConfigurationModel"], @@ -57,10 +61,10 @@ class ConfigSettings: "FOO_SOME_OPT_SETTING", "FOO_SOME_OTHER_OPT_SETTING", ], - detailed_info={ + additional_info={ "example_non_model_field": { "variable": "FOO_EXAMPLE_NON_MODEL_FIELD", - "description": "Documentation for a field that could not + "description": "Documentation for a field that cannot be retrievend from a model", "possible_values": "string (URL)", }, @@ -68,23 +72,32 @@ class ConfigSettings: ) """ - namespace: str - file_name: str - models: list[Type[models.Model]] | None - required_settings: list[str] = [] - optional_settings: list[str] = [] - detailed_info: dict[str, dict[str, str]] | None - - def __init__(self, *args, update_field_descriptions: bool = False, **kwargs): + def __init__( + self, + *args, + enable_setting: str, + namespace: str, + file_name: str | None = None, + models: list[Type[models.Model]] | None = None, + required_settings: list[str], + optional_settings: list[str] | None = None, + additional_info: dict[str, dict[str, str]] | None = None, + update_field_descriptions: bool = False, + **kwargs, + ): + self.enable_setting = enable_setting + self.namespace = namespace + self.file_name = file_name or self.namespace.lower() + self.models = models + self.required_settings = required_settings + self.optional_settings = optional_settings or [] + self.additional_info = additional_info or {} + self.update_field_descriptions = update_field_descriptions self.config_fields: list[ConfigField] = [] - for key, value in kwargs.items(): - setattr(self, key, value) - - if not getattr(self, "models", None): + if not self.models: return - # add support for custom fields like PrivateMediaField if update_field_descriptions: self.update_field_descriptions() @@ -155,7 +168,7 @@ def get_field_description(field: models.Field) -> str: # other fields field_type = type(field) if field_type in basic_field_descriptions.keys(): - return basic_field_descriptions.get(field_type) + return basic_field_descriptions.get(field_type, "") return "No information available" @@ -212,16 +225,4 @@ def create_config_fields( # convenience methods/properties for formatting # def get_config_variable(self, setting: str) -> str: - return f"{self.namespace}_" + setting.upper() - - @property - def file_name(self) -> str: - """ - Use `self.namespace` in lower case as default file name of the documentation - if `file_name` is not provided when instantiating the class - """ - return getattr(self, "_file_name", None) or self.namespace.lower() - - @file_name.setter - def file_name(self, val) -> None: - self._file_name = val + return f"{self.namespace}_{setting.upper()}" diff --git a/django_setup_configuration/configuration.py b/django_setup_configuration/configuration.py index 4c94ada..9b3e8ca 100644 --- a/django_setup_configuration/configuration.py +++ b/django_setup_configuration/configuration.py @@ -8,8 +8,6 @@ class BaseConfigurationStep(ABC): verbose_name: str - required_settings: list[str] = [] - enable_setting: str = "" config_settings: ConfigSettings def __repr__(self): @@ -38,10 +36,10 @@ def is_enabled(self) -> bool: By default all steps are enabled """ - if not self.enable_setting: + if not self.config_settings.enable_setting: return True - return getattr(settings, self.enable_setting, True) + return getattr(settings, self.config_settings.enable_setting, True) @abstractmethod def is_configured(self) -> bool: diff --git a/django_setup_configuration/constants.py b/django_setup_configuration/constants.py index fa33504..d36e57d 100644 --- a/django_setup_configuration/constants.py +++ b/django_setup_configuration/constants.py @@ -6,22 +6,16 @@ models.EmailField: "string representing an Email address (foo@bar.com)", models.FileField: ( "string representing the (absolute) path to a file, " - "including file extension: {example}".format( - example="/absolute/path/to/file.xml" - ) + "including file extension: /absolute/path/to/file.xml" ), models.ImageField: ( "string representing the (absolute) path to an image file, " - "including file extension: {example}".format( - example="/absolute/path/to/image.png" - ) + "including file extension: /absolute/path/to/image.png" ), models.IntegerField: "string representing an integer", models.JSONField: "Mapping: {example}".format(example="{'some_key': 'Some value'}"), models.PositiveIntegerField: "string representing a positive integer", models.TextField: "text (string)", models.URLField: "string (URL)", - models.UUIDField: "UUID string {example}".format( - example="(e.g. f6b45142-0c60-4ec7-b43d-28ceacdc0b34)" - ), + models.UUIDField: "UUID string (e.g. f6b45142-0c60-4ec7-b43d-28ceacdc0b34)", } diff --git a/django_setup_configuration/management/commands/generate_config_docs.py b/django_setup_configuration/management/commands/generate_config_docs.py index 00e8afc..99acd0a 100644 --- a/django_setup_configuration/management/commands/generate_config_docs.py +++ b/django_setup_configuration/management/commands/generate_config_docs.py @@ -17,13 +17,14 @@ class ConfigDocBase: """ @staticmethod - def _add_detailed_info(config_settings: ConfigSettings, result: list[str]) -> None: + def _add_additional_info( + config_settings: ConfigSettings, result: list[str] + ) -> None: """Convenience/helper function to retrieve additional documentation info""" - if not (info := getattr(config_settings, "detailed_info", None)): - return + additional_info = config_settings.additional_info - for key, value in info.items(): + for key, value in additional_info.items(): part = [] part.append(f"{'Variable':<20}{value['variable']}") part.append( @@ -61,10 +62,10 @@ def get_detailed_info( part.append(f"{'Default value':<20}{field.default_value}") ret.append(part) - self._add_detailed_info(config, ret) + self._add_additional_info(config, ret) for step in related_steps: - self._add_detailed_info(step.config_settings, ret) + self._add_additional_info(step.config_settings, ret) return ret @@ -75,7 +76,7 @@ def format_display_name(self, display_name: str) -> str: display_name_formatted = f"{heading_bar}\n{display_name}\n{heading_bar}" return display_name_formatted - def render_doc(self, config_settings: ConfigSettings, config_step) -> None: + def render_doc(self, config_settings: ConfigSettings, config_step) -> str: """ Render a `ConfigSettings` documentation template with the following variables: 1. enable_setting @@ -93,7 +94,8 @@ def render_doc(self, config_settings: ConfigSettings, config_step) -> None: name for name in getattr(config_settings, "required_settings", []) ] - # additional requirements from related configuration steps + # additional requirements from related configuration steps to embed + # the documentation of several steps into one related_steps = [step for step in getattr(config_step, "related_steps", [])] related_requirements_lists = [ step.config_settings.required_settings for step in related_steps @@ -156,7 +158,7 @@ def content_is_up_to_date(self, rendered_content: str, doc_path: str) -> bool: return True - def handle(self, *args, **kwargs) -> str: + def handle(self, *args, **kwargs) -> None: target_dir = settings.DJANGO_SETUP_CONFIG_DOC_PATH # create directory for docs if it doesn't exist diff --git a/docs/config_docs.rst b/docs/config_docs.rst index 53cd8cb..681c068 100644 --- a/docs/config_docs.rst +++ b/docs/config_docs.rst @@ -10,134 +10,87 @@ configured with the help of the library and how. Setup """"" -Start by defining the target directory of the docs and the template that will be used -to render them. For example: +Start by defining the target directory where the docs will be saved and the template used +to render them (a default template is included, but you can specify your own). For example: :: - DJANGO_SETUP_CONFIG_TEMPLATE_NAME = "configurations/config_doc.rst" - DJANGO_SETUP_CONFIG_DOC_DIR = "docs/configuration" + DJANGO_SETUP_CONFIG_TEMPLATE = "django-setup-configuration/config_doc.rst" + DJANGO_SETUP_CONFIG_DOC_PATH = "your-project/docs/configuration" -Next, create a template: +Only a subset of Django model fields (CharField, TextField...) is supported out of the box. +The reason for this is two-fold. On the one hand, there are many custom model fields from various +third-party libraries, and it is impossible to support documentation for all of them. +Secondly, the exact way how certain fields are configured via strings (the values of configuration +variables) inevitably depends on the implementation. For example, in order to configure a +``DateTimeField`` via a string provided by the end user, the string has to be converted to a Python +``datetime`` object. How this is done (in particular, what format of string is tolerated) depends on +the implementation of the configuration step and cannot be anticipated by the library. For a list of +currently supported fields, see the list of constants defined in the source code: +`link `_ -:: - - {% block link %}{{ link }}{% endblock %} - - {% block title %}{{ title }}{% endblock %} - - Settings Overview - ================= - - Enable/Disable configuration: - """"""""""""""""""""""""""""" - - :: - - {% spaceless %} - {{ enable_settings }} - {% endspaceless %} - - Required: - """"""""" - - :: - - {% spaceless %} - {% for setting in required_settings %}{{ setting }} - {% endfor %} - {% endspaceless %} - - All settings: - """"""""""""" - - :: - - {% spaceless %} - {% for setting in all_settings %}{{ setting }} - {% endfor %} - {% endspaceless %} - - Detailed Information - ==================== - - :: - - {% spaceless %} - {% for detail in detailed_info %} - {% for part in detail %}{{ part|safe }} - {% endfor %}{% endfor %} - {% endspaceless %} - -You're free to choose a different layout, of course, but this should give you an idea -of what's available, and some of the nuisances (like spacing) you might run into. - -The value for ``link`` is automatically create based on the ``file_name`` you defined -in the first step. Similarly, ``enable_settings`` is automatically created. Values for -the other template variables are determined based on the model your client is configuring, -and a related settings model you need to define. - -Generally, when adding support for configuring a model ``FooConfiguration`` with -``django-setup-configuration``, you will have a class ``FooConfigurationStep(BaseConfigurationStep)`` -that carries out the configuration. In the same file, define a class with information about -the fields from ``FooConfiguration`` that are required, included, or excluded, and -meta-information required for creating the docs. For example: - - -:: - - from django-setup-configuration import ConfigSettingsModel - - class FooConfigurationSettings(ConfigSettingsModel): - model = FooConfiguration - display_name = "Foo Configuration" - namespace = "FOO" - required_fields = ( - "bar", - "baz", - ) - included_fields = required_fields + ( - "foobar", - ) - excluded_fields = ( - "bogus", - ) - - - class FooConfigurationStep(BaseConfigurationStep): - ... - -``display_name`` provides the value for ``title`` in the template above. ``namespace`` -tells you how config variables for different models are namespaced. In your settings, -you would define ``FOO_BAR``, ``FOO_BAZ``, and ``FOO_FOOBAR``. - -Finally, you need to register you configuration settings class with the library: +You can add support for additional fields like ``FileField`` or custom fields from third-party +libraries (or your own) as follows: :: - DJANGO_SETUP_CONFIG_REGISTER = [ + DJANGO_SETUP_CONFIG_CUSTOM_FIELDS = [ { - "model": "example_project.path.to.FooConfigurationSettings", - "file_name": "foo_config", - } + "field": "django_jsonform.models.fields.ArrayField", + "description": "string, comma-delimited ('foo,bar,baz')", + }, + { + "field": "django.db.models.fields.files.FileField", + "description": "string represeting the (absolute) path to a file, including file extension", + }, ] Usage """"" -The library provides two management commands: +For every configuration step, define a ``ConfigSetting`` model with the appropriate settings as +attribute on the class: + +:: + + from django-setup-configuration.config_settings import ConfigSettingsModel + from django-setup-configuration.models import BaseConfigurationStep + + FooConfigurationStep(BaseConfigurationStep): + verbose_name = "Configuration step for Foo" + enable_setting = "FOO_CONFIG_ENABLE" + config_settings = ConfigSettings( + namespace="FOO", + file_name="foo", + models=["FooConfigurationModel"], + required_settings=[ + "FOO_SOME_SETTING", + "FOO_SOME_OTHER_SETTING", + ], + optional_settings=[ + "FOO_SOME_OPT_SETTING", + "FOO_SOME_OTHER_OPT_SETTING", + ], + additional_info={ + "example_non_model_field": { + "variable": "FOO_EXAMPLE_NON_MODEL_FIELD", + "description": "Documentation for a field that could not + be retrievend from a model", + "possible_values": "string (URL)", + }, + }, + ) + + def configure(): ... + +The documentation for settings used to configure Django model fields is pulled from the help +text of the relevant fields. You merely have to specify the models used in the configuration +step and which settings are required/optional. ``additional_info`` is used to manually document +configuration settings which are not associated with any model field. + +With everything set up, you can generate the docs with the following command: :: - manage.py generate_config_docs [CONFIG_OPTION] - manage.py check_config_docs - -The optional ``CONFIG_OPTION`` should be a ``file_name`` (without extension) that -corresponds to a settings model (e.g. ``foo_config``). When given, -``generate_config_docs`` will create the docs for the corresponding model. Otherwise -the command creates docs for all supported models. ``check_config_docs`` is similar -to ``manage.py makemigrations --check --dry-run``: it tests that documentation for your -configuration setup steps exists and is accurate (if you make changes to -``FooConfiguration`` or ``FooConfigurationSettings`` without re-creating the docs, -``check_config_docs`` will raise an exception). + python manage.py generate_config_docs diff --git a/testapp/configuration.py b/testapp/configuration.py index eac493b..4e5550e 100644 --- a/testapp/configuration.py +++ b/testapp/configuration.py @@ -16,11 +16,11 @@ class UserConfigurationStep(BaseConfigurationStep): """ verbose_name = "User Configuration" - enable_setting = "USER_CONFIGURATION_ENABLED" config_settings = ConfigSettings( - models=[User], - file_name="user", + enable_setting="USER_CONFIGURATION_ENABLED", namespace="USER_CONFIGURATION", + file_name="user", + models=[User], required_settings=[ "USER_CONFIGURATION_USERNAME", "USER_CONFIGURATION_PASSWORD", From 5629f83d6b85078f4d9a66421541d5f625dd21b5 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Fri, 5 Jul 2024 17:09:17 +0200 Subject: [PATCH 7/7] [#8] Define related_settings on ConfigSettings --- django_setup_configuration/__init__.py | 5 -- django_setup_configuration/config_settings.py | 24 ++++-- .../commands/generate_config_docs.py | 77 +++++++++++-------- docs/config_docs.rst | 13 +++- testapp/settings.py | 2 +- testapp/templates/testapp/config_doc.rst | 48 ------------ tests/mocks.py | 2 +- 7 files changed, 79 insertions(+), 92 deletions(-) delete mode 100644 testapp/templates/testapp/config_doc.rst diff --git a/django_setup_configuration/__init__.py b/django_setup_configuration/__init__.py index 588b1bb..e69de29 100644 --- a/django_setup_configuration/__init__.py +++ b/django_setup_configuration/__init__.py @@ -1,5 +0,0 @@ -from .config_settings import ConfigSettings - -__all__ = [ - "ConfigSettings", -] diff --git a/django_setup_configuration/config_settings.py b/django_setup_configuration/config_settings.py index b7ce0dc..8de6f93 100644 --- a/django_setup_configuration/config_settings.py +++ b/django_setup_configuration/config_settings.py @@ -35,7 +35,12 @@ class ConfigSettings: models (`list`): a list of models from which documentation is retrieved required_settings (`list`): required settings for a configuration step optional_settings (`list`): optional settings for a configuration step - update_field_descriptions (`bool`): if `True`, custom model fields + independent (`bool`): if `True` (the default), the documentation will be + created in its own file; set to `False` if you want to avoid this and + plan to embed the docs in another file (see `related_config_settings`) + related_config_settings (`list`): optional list of `ConfigSettings` from + related configuration steps; used for embedding documentation + update_fields (`bool`): if `True`, custom model fields (along with their descriptions) are loaded via the settings variable `DJANGO_SETUP_CONFIG_CUSTOM_FIELDS` additional_info (`dict`): information for configuration settings which are @@ -61,6 +66,9 @@ class ConfigSettings: "FOO_SOME_OPT_SETTING", "FOO_SOME_OTHER_OPT_SETTING", ], + related_config_settings=[ + "BarRelatedConfigurationStep.config_settings", + ], additional_info={ "example_non_model_field": { "variable": "FOO_EXAMPLE_NON_MODEL_FIELD", @@ -81,8 +89,10 @@ def __init__( models: list[Type[models.Model]] | None = None, required_settings: list[str], optional_settings: list[str] | None = None, + independent: bool = True, + related_config_settings: list["ConfigSettings"] | None = None, additional_info: dict[str, dict[str, str]] | None = None, - update_field_descriptions: bool = False, + update_fields: bool = False, **kwargs, ): self.enable_setting = enable_setting @@ -91,15 +101,17 @@ def __init__( self.models = models self.required_settings = required_settings self.optional_settings = optional_settings or [] + self.independent = independent + self.related_config_settings = related_config_settings or [] self.additional_info = additional_info or {} - self.update_field_descriptions = update_field_descriptions + self.update_fields = update_fields self.config_fields: list[ConfigField] = [] if not self.models: return - if update_field_descriptions: - self.update_field_descriptions() + if update_fields: + self.load_additional_fields() for model in self.models: self.create_config_fields(model=model) @@ -135,7 +147,7 @@ def get_default_value(field: models.Field) -> str: return default @staticmethod - def update_field_descriptions() -> None: + def load_additional_fields() -> None: """ Add custom fields + descriptions defined in settings to `basic_field_descriptions` diff --git a/django_setup_configuration/management/commands/generate_config_docs.py b/django_setup_configuration/management/commands/generate_config_docs.py index 99acd0a..06c61b1 100644 --- a/django_setup_configuration/management/commands/generate_config_docs.py +++ b/django_setup_configuration/management/commands/generate_config_docs.py @@ -1,3 +1,4 @@ +import itertools import pathlib from django.conf import settings @@ -17,8 +18,16 @@ class ConfigDocBase: """ @staticmethod - def _add_additional_info( - config_settings: ConfigSettings, result: list[str] + def extract_unique_settings(settings: list[list[str]]) -> list[str]: + """ + Flatten `settings` (a list of lists with settings) and remove dupes + """ + unique_settings = set(itertools.chain.from_iterable(settings)) + return list(unique_settings) + + @staticmethod + def add_additional_info( + config_settings: ConfigSettings, result: list[list[str]] ) -> None: """Convenience/helper function to retrieve additional documentation info""" @@ -41,9 +50,8 @@ def _add_additional_info( def get_detailed_info( self, - config: ConfigSettings, - config_step, - related_steps: list, + config_settings: ConfigSettings, + related_config_settings: list[ConfigSettings], ) -> list[list[str]]: """ Get information about the configuration settings: @@ -52,22 +60,24 @@ def get_detailed_info( 3. from information provided manually in the `ConfigSettings` of related configuration steps """ - ret = [] - for field in config.config_fields: + result = [] + for field in config_settings.config_fields: part = [] - part.append(f"{'Variable':<20}{config.get_config_variable(field.name)}") + part.append( + f"{'Variable':<20}{config_settings.get_config_variable(field.name)}" + ) part.append(f"{'Setting':<20}{field.verbose_name}") part.append(f"{'Description':<20}{field.description or 'No description'}") part.append(f"{'Possible values':<20}{field.field_description}") part.append(f"{'Default value':<20}{field.default_value}") - ret.append(part) + result.append(part) - self._add_additional_info(config, ret) + self.add_additional_info(config_settings, result) - for step in related_steps: - self._add_additional_info(step.config_settings, ret) + for config_settings in related_config_settings: + self.add_additional_info(config_settings, result) - return ret + return result def format_display_name(self, display_name: str) -> str: """Surround title with '=' to display as heading in rst file""" @@ -81,43 +91,47 @@ def render_doc(self, config_settings: ConfigSettings, config_step) -> str: Render a `ConfigSettings` documentation template with the following variables: 1. enable_setting 2. required_settings - 3. all_settings (required_settings + optional_settings) + 3. all_settings (required_settings + optional_settings + related settings) 4. detailed_info 5. title 6. link (for crossreference across different files) """ # 1. - enable_setting = getattr(config_step, "enable_setting", None) + enable_setting = getattr(config_settings, "enable_setting", None) # 2. required_settings = [ name for name in getattr(config_settings, "required_settings", []) ] - # additional requirements from related configuration steps to embed + # additional settings from related configuration steps to embed # the documentation of several steps into one - related_steps = [step for step in getattr(config_step, "related_steps", [])] - related_requirements_lists = [ - step.config_settings.required_settings for step in related_steps + related_config_settings = [ + config for config in getattr(config_settings, "related_config_settings", []) ] - related_requirements = set( - item for row in related_requirements_lists for item in row + required_settings_related = self.extract_unique_settings( + [config.required_settings for config in related_config_settings] + ) + optional_settings_related = self.extract_unique_settings( + [config.optional_settings for config in related_config_settings] ) - required_settings.extend(list(related_requirements)) + required_settings.extend(required_settings_related) required_settings.sort() # 3. all_settings = [ setting - for setting in config_settings.required_settings + for setting in required_settings + config_settings.optional_settings + + optional_settings_related ] all_settings.sort() # 4. detailed_info = self.get_detailed_info( - config_settings, config_step, related_steps + config_settings, + related_config_settings, ) detailed_info.sort() @@ -167,10 +181,13 @@ def handle(self, *args, **kwargs) -> None: for config_string in settings.SETUP_CONFIGURATION_STEPS: config_step = import_string(config_string) - if config_settings := getattr(config_step, "config_settings", None): - doc_path = f"{target_dir}/{config_settings.file_name}.rst" - rendered_content = self.render_doc(config_settings, config_step) + config_settings = getattr(config_step, "config_settings", None) + if not config_settings or not config_settings.independent: + continue + + doc_path = f"{target_dir}/{config_settings.file_name}.rst" + rendered_content = self.render_doc(config_settings, config_step) - if not self.content_is_up_to_date(rendered_content, doc_path): - with open(doc_path, "w+") as output: - output.write(rendered_content) + if not self.content_is_up_to_date(rendered_content, doc_path): + with open(doc_path, "w+") as output: + output.write(rendered_content) diff --git a/docs/config_docs.rst b/docs/config_docs.rst index 681c068..223b0f4 100644 --- a/docs/config_docs.rst +++ b/docs/config_docs.rst @@ -59,8 +59,8 @@ attribute on the class: FooConfigurationStep(BaseConfigurationStep): verbose_name = "Configuration step for Foo" - enable_setting = "FOO_CONFIG_ENABLE" config_settings = ConfigSettings( + enable_setting = "FOO_CONFIG_ENABLE" namespace="FOO", file_name="foo", models=["FooConfigurationModel"], @@ -72,6 +72,10 @@ attribute on the class: "FOO_SOME_OPT_SETTING", "FOO_SOME_OTHER_OPT_SETTING", ], + independent=True, + related_config_settings=[ + "BarRelatedConfigurationStep.config_settings", + ], additional_info={ "example_non_model_field": { "variable": "FOO_EXAMPLE_NON_MODEL_FIELD", @@ -89,6 +93,13 @@ text of the relevant fields. You merely have to specify the models used in the c step and which settings are required/optional. ``additional_info`` is used to manually document configuration settings which are not associated with any model field. +In certain cases, you may want to avoid creating a separate documentation file for some +configuration steps. For example, you may want to include the documentation for API services +associated with ``FOO`` in the documentation for ``FOO``, instead of having a separate file +for each. In this case, you set ``independent`` to ``False`` on the ``ConfigSettings`` that you +want to embed, and include the relevant ``ConfigSettings`` under ``related_config_settings`` +on your main config. + With everything set up, you can generate the docs with the following command: :: diff --git a/testapp/settings.py b/testapp/settings.py index 9a4fb68..4c4fe03 100644 --- a/testapp/settings.py +++ b/testapp/settings.py @@ -71,5 +71,5 @@ USER_CONFIGURATION_USERNAME = os.getenv("USER_CONFIGURATION_USERNAME", "demo") USER_CONFIGURATION_PASSWORD = os.getenv("USER_CONFIGURATION_PASSWORD", "secret") -DJANGO_SETUP_CONFIG_TEMPLATE = "testapp/config_doc.rst" +DJANGO_SETUP_CONFIG_TEMPLATE = "django_setup_configuration/config_doc.rst" DJANGO_SETUP_CONFIG_DOC_PATH = "testapp/docs/configuration" diff --git a/testapp/templates/testapp/config_doc.rst b/testapp/templates/testapp/config_doc.rst deleted file mode 100644 index 5c3ce56..0000000 --- a/testapp/templates/testapp/config_doc.rst +++ /dev/null @@ -1,48 +0,0 @@ -{% block link %}{{ link }}{% endblock %} - -{% block title %}{{ title }}{% endblock %} - -Settings Overview -================= - -Enable/Disable configuration: -""""""""""""""""""""""""""""" - -:: - - {% spaceless %} - {{ enable_settings }} - {% endspaceless %} - -{% if required_settings %} -Required: -""""""""" - -:: - - {% spaceless %} - {% for setting in required_settings %}{{ setting }} - {% endfor %} - {% endspaceless %} -{% endif %} - -All settings: -""""""""""""" - -:: - - {% spaceless %} - {% for setting in all_settings %}{{ setting }} - {% endfor %} - {% endspaceless %} - -Detailed Information -==================== - -:: - - {% spaceless %} - {% for detail in detailed_info %} - {% for part in detail %}{{ part|safe }} - {% endfor %}{% endfor %} - {% endspaceless %} diff --git a/tests/mocks.py b/tests/mocks.py index b2a99aa..1f7e0ce 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -1,2 +1,2 @@ -mock_user_doc = '.. _user:\n\n==================\nUser Configuration\n==================\n\nSettings Overview\n=================\n\nEnable/Disable configuration:\n"""""""""""""""""""""""""""""\n\n::\n\n \n\n\nRequired:\n"""""""""\n\n::\n\n USER_CONFIGURATION_PASSWORD\n USER_CONFIGURATION_USERNAME\n\n\nAll settings:\n"""""""""""""\n\n::\n\n USER_CONFIGURATION_EMAIL\n USER_CONFIGURATION_FIRST_NAME\n USER_CONFIGURATION_IS_STAFF\n USER_CONFIGURATION_IS_SUPERUSER\n USER_CONFIGURATION_LAST_NAME\n USER_CONFIGURATION_PASSWORD\n USER_CONFIGURATION_USERNAME\n\nDetailed Information\n====================\n\n::\n\n Variable USER_CONFIGURATION_EMAIL\n Setting email address\n Description No description\n Possible values string representing an Email address (foo@bar.com)\n Default value No default\n \n Variable USER_CONFIGURATION_FIRST_NAME\n Setting first name\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_IS_STAFF\n Setting staff status\n Description Designates whether the user can log into this admin site.\n Possible values True, False\n Default value False\n \n Variable USER_CONFIGURATION_IS_SUPERUSER\n Setting superuser status\n Description Designates that this user has all permissions without explicitly assigning them.\n Possible values True, False\n Default value False\n \n Variable USER_CONFIGURATION_LAST_NAME\n Setting last name\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_PASSWORD\n Setting password\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_USERNAME\n Setting username\n Description Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.\n Possible values string\n Default value No default\n' # noqua +mock_user_doc = '.. _user:\n\n==================\nUser Configuration\n==================\n\nSettings Overview\n=================\n\n\nEnable/Disable configuration:\n"""""""""""""""""""""""""""""\n\n::\n\n USER_CONFIGURATION_ENABLED\n\n\n\nRequired:\n"""""""""\n\n::\n\n USER_CONFIGURATION_PASSWORD\n USER_CONFIGURATION_USERNAME\n\n\nAll settings:\n"""""""""""""\n\n::\n\n USER_CONFIGURATION_EMAIL\n USER_CONFIGURATION_FIRST_NAME\n USER_CONFIGURATION_IS_STAFF\n USER_CONFIGURATION_IS_SUPERUSER\n USER_CONFIGURATION_LAST_NAME\n USER_CONFIGURATION_PASSWORD\n USER_CONFIGURATION_USERNAME\n\nDetailed Information\n====================\n\n::\n\n Variable USER_CONFIGURATION_EMAIL\n Setting email address\n Description No description\n Possible values string representing an Email address (foo@bar.com)\n Default value No default\n \n Variable USER_CONFIGURATION_FIRST_NAME\n Setting first name\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_IS_STAFF\n Setting staff status\n Description Designates whether the user can log into this admin site.\n Possible values True, False\n Default value False\n \n Variable USER_CONFIGURATION_IS_SUPERUSER\n Setting superuser status\n Description Designates that this user has all permissions without explicitly assigning them.\n Possible values True, False\n Default value False\n \n Variable USER_CONFIGURATION_LAST_NAME\n Setting last name\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_PASSWORD\n Setting password\n Description No description\n Possible values string\n Default value No default\n \n Variable USER_CONFIGURATION_USERNAME\n Setting username\n Description Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.\n Possible values string\n Default value No default\n' # noqua mock_user_doc_mismatch = "hello world"