From cb8ae10280d61a4d055e565e166920230fab243f Mon Sep 17 00:00:00 2001 From: Lukas <76838159+wolflu05@users.noreply.github.com> Date: Fri, 19 May 2023 01:51:30 +0200 Subject: [PATCH] Refactor: BaseInvenTreeSetting (#4834) * Added typing for settings * Refactored common.BaseInvenTreeSetting model to make it more generic * Use older syntax for union types * Added protected option to typing * Remove now unused code * Remove old 'get_kwargs' method as it is replaced by 'get_filters_for_instance' * Trigger ci --- InvenTree/common/models.py | 163 ++++++++---------- .../part/templatetags/inventree_extras.py | 3 + InvenTree/plugin/api.py | 2 +- .../plugin/base/integration/SettingsMixin.py | 13 +- InvenTree/plugin/models.py | 19 +- 5 files changed, 93 insertions(+), 107 deletions(-) diff --git a/InvenTree/common/models.py b/InvenTree/common/models.py index 16545e5914..7a26b0c11d 100644 --- a/InvenTree/common/models.py +++ b/InvenTree/common/models.py @@ -16,6 +16,7 @@ import uuid from datetime import datetime, timedelta from enum import Enum from secrets import compare_digest +from typing import Any, Callable, Dict, List, Tuple, TypedDict, Union from django.apps import apps from django.conf import settings @@ -112,10 +113,45 @@ class ProjectCode(InvenTree.models.MetadataMixin, models.Model): ) -class BaseInvenTreeSetting(models.Model): - """An base InvenTreeSetting object is a key:value pair used for storing single values (e.g. one-off settings values).""" +class SettingsKeyType(TypedDict, total=False): + """Type definitions for a SettingsKeyType - SETTINGS = {} + Attributes: + name: Translatable string name of the setting (required) + description: Translatable string description of the setting (required) + units: Units of the particular setting (optional) + validator: Validation function/list of functions for the setting (optional, default: None, e.g: bool, int, str, MinValueValidator, ...) + default: Default value or function that returns default value (optional) + choices: (Function that returns) Tuple[str: key, str: display value] (optional) + hidden: Hide this setting from settings page (optional) + before_save: Function that gets called after save with *args, **kwargs (optional) + after_save: Function that gets called after save with *args, **kwargs (optional) + protected: Protected values are not returned to the client, instead "***" is returned (optional, default: False) + """ + + name: str + description: str + units: str + validator: Union[Callable, List[Callable], Tuple[Callable]] + default: Union[Callable, Any] + choices: Union[Tuple[str, str], Callable[[], Tuple[str, str]]] + hidden: bool + before_save: Callable[..., None] + after_save: Callable[..., None] + protected: bool + + +class BaseInvenTreeSetting(models.Model): + """An base InvenTreeSetting object is a key:value pair used for storing single values (e.g. one-off settings values). + + Attributes: + SETTINGS: definition of all available settings + extra_unique_fields: List of extra fields used to be unique, e.g. for PluginConfig -> plugin + """ + + SETTINGS: Dict[str, SettingsKeyType] = {} + + extra_unique_fields: List[str] = [] class Meta: """Meta options for BaseInvenTreeSetting -> abstract stops creation of database entry.""" @@ -129,7 +165,7 @@ class BaseInvenTreeSetting(models.Model): do_cache = kwargs.pop('cache', True) self.clean(**kwargs) - self.validate_unique(**kwargs) + self.validate_unique() # Execute before_save action self._call_settings_function('before_save', args, kwargs) @@ -162,7 +198,7 @@ class BaseInvenTreeSetting(models.Model): @property def cache_key(self): """Generate a unique cache key for this settings object""" - return self.__class__.create_cache_key(self.key, **self.get_kwargs()) + return self.__class__.create_cache_key(self.key, **self.get_filters_for_instance()) def save_to_cache(self): """Save this setting object to cache""" @@ -199,7 +235,16 @@ class BaseInvenTreeSetting(models.Model): return key.replace(" ", "") @classmethod - def allValues(cls, user=None, exclude_hidden=False): + def get_filters(cls, **kwargs): + """Enable to filter by other kwargs defined in cls.extra_unique_fields""" + return {key: value for key, value in kwargs.items() if key in cls.extra_unique_fields} + + def get_filters_for_instance(self): + """Enable to filter by other fields defined in self.extra_unique_fields""" + return {key: getattr(self, key, None) for key in self.extra_unique_fields if hasattr(self, key)} + + @classmethod + def allValues(cls, exclude_hidden=False, **kwargs): """Return a dict of "all" defined global settings. This performs a single database lookup, @@ -212,9 +257,8 @@ class BaseInvenTreeSetting(models.Model): # Keys which start with an undersore are used for internal functionality results = results.exclude(key__startswith='_') - # Optionally filter by user - if user is not None: - results = results.filter(user=user) + # Optionally filter by other keys + results = results.filter(**cls.get_filters(**kwargs)) # Query the database settings = {} @@ -253,16 +297,6 @@ class BaseInvenTreeSetting(models.Model): return settings - def get_kwargs(self): - """Construct kwargs for doing class-based settings lookup, depending on *which* class we are. - - This is necessary to abtract the settings object - from the implementing class (e.g plugins) - - Subclasses should override this function to ensure the kwargs are correctly set. - """ - return {} - @classmethod def get_setting_definition(cls, key, **kwargs): """Return the 'definition' of a particular settings value, as a dict object. @@ -361,32 +395,11 @@ class BaseInvenTreeSetting(models.Model): filters = { 'key__iexact': key, + + # Optionally filter by other keys + **cls.get_filters(**kwargs), } - # Filter by user - user = kwargs.get('user', None) - - if user is not None: - filters['user'] = user - - # Filter by plugin - plugin = kwargs.get('plugin', None) - - if plugin is not None: - from plugin import InvenTreePlugin - - if issubclass(plugin.__class__, InvenTreePlugin): - plugin = plugin.plugin_config() - - filters['plugin'] = plugin - kwargs['plugin'] = plugin - - # Filter by method - method = kwargs.get('method', None) - - if method is not None: - filters['method'] = method - # Perform cache lookup by default do_cache = kwargs.pop('cache', True) @@ -493,22 +506,11 @@ class BaseInvenTreeSetting(models.Model): filters = { 'key__iexact': key, + + # Optionally filter by other keys + **cls.get_filters(**kwargs), } - user = kwargs.get('user', None) - plugin = kwargs.get('plugin', None) - - if user is not None: - filters['user'] = user - - if plugin is not None: - from plugin import InvenTreePlugin - - if issubclass(plugin.__class__, InvenTreePlugin): - filters['plugin'] = plugin.plugin_config() - else: - filters['plugin'] = plugin - try: setting = cls.objects.get(**filters) except cls.DoesNotExist: @@ -532,22 +534,22 @@ class BaseInvenTreeSetting(models.Model): @property def name(self): """Return name for setting.""" - return self.__class__.get_setting_name(self.key, **self.get_kwargs()) + return self.__class__.get_setting_name(self.key, **self.get_filters_for_instance()) @property def default_value(self): """Return default_value for setting.""" - return self.__class__.get_setting_default(self.key, **self.get_kwargs()) + return self.__class__.get_setting_default(self.key, **self.get_filters_for_instance()) @property def description(self): """Return description for setting.""" - return self.__class__.get_setting_description(self.key, **self.get_kwargs()) + return self.__class__.get_setting_description(self.key, **self.get_filters_for_instance()) @property def units(self): """Return units for setting.""" - return self.__class__.get_setting_units(self.key, **self.get_kwargs()) + return self.__class__.get_setting_units(self.key, **self.get_filters_for_instance()) def clean(self, **kwargs): """If a validator (or multiple validators) are defined for a particular setting key, run them against the 'value' field.""" @@ -615,7 +617,7 @@ class BaseInvenTreeSetting(models.Model): validator(value) - def validate_unique(self, exclude=None, **kwargs): + def validate_unique(self, exclude=None): """Ensure that the key:value pair is unique. In addition to the base validators, this ensures that the 'key' is unique, using a case-insensitive comparison. Note that sub-classes (UserSetting, PluginSetting) use other filters @@ -625,17 +627,11 @@ class BaseInvenTreeSetting(models.Model): filters = { 'key__iexact': self.key, + + # Optionally filter by other keys + **self.get_filters_for_instance(), } - user = getattr(self, 'user', None) - plugin = getattr(self, 'plugin', None) - - if user is not None: - filters['user'] = user - - if plugin is not None: - filters['plugin'] = plugin - try: # Check if a duplicate setting already exists setting = self.__class__.objects.filter(**filters).exclude(id=self.id) @@ -648,7 +644,7 @@ class BaseInvenTreeSetting(models.Model): def choices(self): """Return the available choices for this setting (or None if no choices are defined).""" - return self.__class__.get_setting_choices(self.key, **self.get_kwargs()) + return self.__class__.get_setting_choices(self.key, **self.get_filters_for_instance()) def valid_options(self): """Return a list of valid options for this setting.""" @@ -661,7 +657,7 @@ class BaseInvenTreeSetting(models.Model): def is_choice(self): """Check if this setting is a "choice" field.""" - return self.__class__.get_setting_choices(self.key, **self.get_kwargs()) is not None + return self.__class__.get_setting_choices(self.key, **self.get_filters_for_instance()) is not None def as_choice(self): """Render this setting as the "display" value of a choice field. @@ -671,7 +667,7 @@ class BaseInvenTreeSetting(models.Model): and the value is 'A4', then display 'A4 paper' """ - choices = self.get_setting_choices(self.key, **self.get_kwargs()) + choices = self.get_setting_choices(self.key, **self.get_filters_for_instance()) if not choices: return self.value @@ -688,7 +684,7 @@ class BaseInvenTreeSetting(models.Model): def model_name(self): """Return the model name associated with this setting.""" - setting = self.get_setting_definition(self.key, **self.get_kwargs()) + setting = self.get_setting_definition(self.key, **self.get_filters_for_instance()) return setting.get('model', None) @@ -752,7 +748,7 @@ class BaseInvenTreeSetting(models.Model): def is_bool(self): """Check if this setting is required to be a boolean value.""" - validator = self.__class__.get_setting_validator(self.key, **self.get_kwargs()) + validator = self.__class__.get_setting_validator(self.key, **self.get_filters_for_instance()) return self.__class__.validator_is_bool(validator) @@ -792,7 +788,7 @@ class BaseInvenTreeSetting(models.Model): def is_int(self,): """Check if the setting is required to be an integer value.""" - validator = self.__class__.get_setting_validator(self.key, **self.get_kwargs()) + validator = self.__class__.get_setting_validator(self.key, **self.get_filters_for_instance()) return self.__class__.validator_is_int(validator) @@ -831,7 +827,7 @@ class BaseInvenTreeSetting(models.Model): @property def protected(self): """Returns if setting is protected from rendering.""" - return self.__class__.is_protected(self.key, **self.get_kwargs()) + return self.__class__.is_protected(self.key, **self.get_filters_for_instance()) def settings_group_options(): @@ -2105,6 +2101,7 @@ class InvenTreeUserSetting(BaseInvenTreeSetting): } typ = 'user' + extra_unique_fields = ['user'] key = models.CharField( max_length=50, @@ -2121,20 +2118,10 @@ class InvenTreeUserSetting(BaseInvenTreeSetting): help_text=_('User'), ) - def validate_unique(self, exclude=None, **kwargs): - """Return if the setting (including key) is unique.""" - return super().validate_unique(exclude=exclude, user=self.user) - def to_native_value(self): """Return the "pythonic" value, e.g. convert "True" to True, and "1" to 1.""" return self.__class__.get_setting(self.key, user=self.user) - def get_kwargs(self): - """Explicit kwargs required to uniquely identify a particular setting object, in addition to the 'key' parameter.""" - return { - 'user': self.user, - } - class PriceBreak(MetaMixin): """Represents a PriceBreak model.""" diff --git a/InvenTree/part/templatetags/inventree_extras.py b/InvenTree/part/templatetags/inventree_extras.py index d264dff2e9..b90c61e588 100644 --- a/InvenTree/part/templatetags/inventree_extras.py +++ b/InvenTree/part/templatetags/inventree_extras.py @@ -19,6 +19,7 @@ from common.settings import currency_code_default from InvenTree import settings, version from plugin import registry from plugin.models import NotificationUserSetting, PluginSetting +from plugin.plugin import InvenTreePlugin register = template.Library() @@ -325,6 +326,8 @@ def setting_object(key, *args, **kwargs): # Note, 'plugin' is an instance of an InvenTreePlugin class plugin = kwargs['plugin'] + if issubclass(plugin.__class__, InvenTreePlugin): + plugin = plugin.plugin_config() return PluginSetting.get_setting_object(key, plugin=plugin, cache=cache) diff --git a/InvenTree/plugin/api.py b/InvenTree/plugin/api.py index 7491b13587..4246fe21d5 100644 --- a/InvenTree/plugin/api.py +++ b/InvenTree/plugin/api.py @@ -232,7 +232,7 @@ class PluginSettingDetail(RetrieveUpdateAPI): if key not in settings: raise NotFound(detail=f"Plugin '{plugin.slug}' has no setting matching '{key}'") - return PluginSetting.get_setting_object(key, plugin=plugin) + return PluginSetting.get_setting_object(key, plugin=plugin.plugin_config()) # Staff permission required permission_classes = [ diff --git a/InvenTree/plugin/base/integration/SettingsMixin.py b/InvenTree/plugin/base/integration/SettingsMixin.py index d7937e9087..5f14dc063d 100644 --- a/InvenTree/plugin/base/integration/SettingsMixin.py +++ b/InvenTree/plugin/base/integration/SettingsMixin.py @@ -1,14 +1,25 @@ """Plugin mixin class for SettingsMixin.""" import logging +from typing import TYPE_CHECKING, Dict from django.db.utils import OperationalError, ProgrammingError logger = logging.getLogger('inventree') +# import only for typechecking, otherwise this throws a model is unready error +if TYPE_CHECKING: + from common.models import SettingsKeyType +else: + class SettingsKeyType: + """Dummy class, so that python throws no error""" + pass + class SettingsMixin: """Mixin that enables global settings for the plugin.""" + SETTINGS: Dict[str, SettingsKeyType] = {} + class MixinMeta: """Meta for mixin.""" MIXIN_NAME = 'Settings' @@ -56,7 +67,7 @@ class SettingsMixin: """ from plugin.models import PluginSetting - return PluginSetting.get_setting(key, plugin=self, cache=cache) + return PluginSetting.get_setting(key, plugin=self.plugin_config(), cache=cache) def set_setting(self, key, value, user=None): """Set plugin setting value by key.""" diff --git a/InvenTree/plugin/models.py b/InvenTree/plugin/models.py index b34585fb97..464def533c 100644 --- a/InvenTree/plugin/models.py +++ b/InvenTree/plugin/models.py @@ -133,6 +133,7 @@ class PluginSetting(common.models.BaseInvenTreeSetting): """This model represents settings for individual plugins.""" typ = 'plugin' + extra_unique_fields = ['plugin'] class Meta: """Meta for PluginSetting.""" @@ -165,27 +166,18 @@ class PluginSetting(common.models.BaseInvenTreeSetting): plugin = kwargs.pop('plugin', None) if plugin: - - if issubclass(plugin.__class__, InvenTreePlugin): - plugin = plugin.plugin_config() - mixin_settings = getattr(registry, 'mixins_settings') if mixin_settings: kwargs['settings'] = mixin_settings.get(plugin.key, {}) return super().get_setting_definition(key, **kwargs) - def get_kwargs(self): - """Explicit kwargs required to uniquely identify a particular setting object, in addition to the 'key' parameter.""" - return { - 'plugin': self.plugin, - } - class NotificationUserSetting(common.models.BaseInvenTreeSetting): """This model represents notification settings for a user.""" typ = 'notification' + extra_unique_fields = ['method', 'user'] class Meta: """Meta for NotificationUserSetting.""" @@ -202,13 +194,6 @@ class NotificationUserSetting(common.models.BaseInvenTreeSetting): return super().get_setting_definition(key, **kwargs) - def get_kwargs(self): - """Explicit kwargs required to uniquely identify a particular setting object, in addition to the 'key' parameter.""" - return { - 'method': self.method, - 'user': self.user, - } - method = models.CharField( max_length=255, verbose_name=_('Method'),