From 0411b8ca58715cdca093e9390c22a52800edaa5f Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 6 Aug 2025 16:43:11 +1000 Subject: [PATCH] [api] Settings cast (#10133) * Fix value type for settings API endpoints - Cast to 'native' value * Check for numerical type casting * Additional unit tests * Tweak unit tests * Fix 'to_native_value' funcs * Refactor native value casting --------- Co-authored-by: Matthias Mair --- src/backend/InvenTree/common/models.py | 12 +- src/backend/InvenTree/common/serializers.py | 21 ++- src/backend/InvenTree/common/tests.py | 139 +++++++++++++++++++- src/backend/InvenTree/machine/models.py | 6 + src/backend/InvenTree/plugin/models.py | 4 + src/backend/InvenTree/plugin/test_api.py | 4 +- 6 files changed, 171 insertions(+), 15 deletions(-) diff --git a/src/backend/InvenTree/common/models.py b/src/backend/InvenTree/common/models.py index f172ca2026..df7f60e461 100644 --- a/src/backend/InvenTree/common/models.py +++ b/src/backend/InvenTree/common/models.py @@ -814,13 +814,13 @@ class BaseInvenTreeSetting(models.Model): # Encode as native values if self.is_int(): - self.value = self.as_int() + self.value = self.as_int(raise_error=True) elif self.is_bool(): self.value = self.as_bool() elif self.is_float(): - self.value = self.as_float() + self.value = self.as_float(raise_error=True) validator = self.__class__.get_setting_validator( self.key, **self.get_filters_for_instance() @@ -1120,7 +1120,7 @@ class BaseInvenTreeSetting(models.Model): return False - def as_float(self): + def as_float(self, raise_error: bool = False) -> float: """Return the value of this setting converted to a float value. If an error occurs, return the default value @@ -1128,6 +1128,8 @@ class BaseInvenTreeSetting(models.Model): try: value = float(self.value) except (ValueError, TypeError): + if raise_error: + raise ValidationError('Provided value is not a valid float') value = self.default_value return value @@ -1153,7 +1155,7 @@ class BaseInvenTreeSetting(models.Model): return False - def as_int(self): + def as_int(self, raise_error: bool = False) -> int: """Return the value of this setting converted to a boolean value. If an error occurs, return the default value @@ -1161,6 +1163,8 @@ class BaseInvenTreeSetting(models.Model): try: value = int(self.value) except (ValueError, TypeError): + if raise_error: + raise ValidationError('Provided value is not a valid integer') value = self.default_value return value diff --git a/src/backend/InvenTree/common/serializers.py b/src/backend/InvenTree/common/serializers.py index 27a4c14e78..34af2a0fd3 100644 --- a/src/backend/InvenTree/common/serializers.py +++ b/src/backend/InvenTree/common/serializers.py @@ -38,17 +38,30 @@ class SettingsValueField(serializers.Field): """Return the object instance, not the attribute value.""" return instance - def to_representation(self, instance) -> str: + def to_representation(self, instance: common_models.InvenTreeSetting) -> str: """Return the value of the setting. Protected settings are returned as '***' """ if instance.protected: return '***' - elif instance.value is None: - return '' else: - return str(instance.value) + value = instance.value + + if value is None: + value = '' + + # Attempt to coerce the value to a native type + if instance.is_int(): + value = instance.as_int() + + elif instance.is_float(): + value = instance.as_float() + + elif instance.is_bool(): + value = instance.as_bool() + + return value def to_internal_value(self, data) -> str: """Return the internal value of the setting.""" diff --git a/src/backend/InvenTree/common/tests.py b/src/backend/InvenTree/common/tests.py index 44f1840735..6d89322fa3 100644 --- a/src/backend/InvenTree/common/tests.py +++ b/src/backend/InvenTree/common/tests.py @@ -659,6 +659,33 @@ class GlobalSettingsApiTest(InvenTreeAPITestCase): self.assertEqual(response.data['value'], 'My new title') + def test_cast(self): + """Test that values are cast to the correct type.""" + key = 'INVENTREE_RESTRICT_ABOUT' + + # Delete the associated setting object + InvenTreeSetting.objects.filter(key=key).delete() + + # Fetch all settings + response = self.get(reverse('api-global-setting-list'), max_query_count=50) + + # Find the associated setting + setting = next((s for s in response.data if s['key'] == key), None) + + # Check default value (should be False, not 'False') + self.assertIsNotNone(setting) + self.assertFalse(setting['value']) + + # Check that we can manually set the value + for v in [True, False]: + set_global_setting(key, v) + + # Check the 'detail' API endpoint + response = self.get( + reverse('api-global-setting-detail', kwargs={'key': key}) + ) + self.assertEqual(response.data['value'], v) + class UserSettingsApiTest(InvenTreeAPITestCase): """Tests for the user settings API.""" @@ -701,7 +728,7 @@ class UserSettingsApiTest(InvenTreeAPITestCase): response = self.get(url, expected_code=200) - self.assertEqual(response.data['value'], 'True') + self.assertEqual(response.data['value'], True) self.patch(url, {'value': 'False'}, expected_code=200) @@ -798,7 +825,7 @@ class UserSettingsApiTest(InvenTreeAPITestCase): response = self.get(url) - self.assertEqual(response.data['value'], str(v)) + self.assertEqual(response.data['value'], v) # Set valid options via the api for v in [5, 15, 25]: @@ -812,6 +839,37 @@ class UserSettingsApiTest(InvenTreeAPITestCase): for v in [0, -1, -5]: response = self.patch(url, {'value': v}, expected_code=400) + def test_cast(self): + """Test numerical typecast for user settings.""" + key = 'SEARCH_PREVIEW_RESULTS' + + # Delete the associated setting object + InvenTreeUserSetting.objects.filter(key=key, user=self.user).delete() + + # Fetch all settings + response = self.get(reverse('api-user-setting-list')) + + # Find the associated setting + setting = next((s for s in response.data if s['key'] == key), None) + + # Check default value (should be 10, not '10') + self.assertIsNotNone(setting) + self.assertEqual(setting['value'], 10) + + # Check that writing an invalid value returns an error + url = reverse('api-user-setting-detail', kwargs={'key': key}) + + self.patch(url, {'value': 'not a number'}, expected_code=400) + self.patch(url, {'value': 0}, expected_code=400) + + # Check that we can manually set the value + for v in [1, 2, 3]: + InvenTreeUserSetting.set_setting(key, v, None, user=self.user) + + # Check the 'detail' API endpoint + response = self.get(reverse('api-user-setting-detail', kwargs={'key': key})) + self.assertEqual(response.data['value'], v) + class PluginSettingsApiTest(PluginMixin, InvenTreeAPITestCase): """Tests for the plugin settings API.""" @@ -874,11 +932,82 @@ class PluginSettingsApiTest(PluginMixin, InvenTreeAPITestCase): "Plugin 'sample' has no setting matching 'doesnotexist'", str(response.data) ) - def test_invalid_setting_key(self): - """Test that an invalid setting key returns a 404.""" - def test_uninitialized_setting(self): """Test that requesting an uninitialized setting creates the setting.""" + from plugin.models import PluginSetting + + slug = 'sample' + key = 'PROTECTED_SETTING' + + registry.set_plugin_state(slug, True) + + plugin = registry.get_plugin(slug) + + # Ensure that the setting does not exist + PluginSetting.objects.filter(plugin=plugin.pk, key=key).delete() + self.assertFalse( + PluginSetting.objects.filter(plugin=plugin.pk, key=key).exists() + ) + + url = reverse('api-plugin-setting-detail', kwargs={'plugin': slug, 'key': key}) + + data = self.get(url, expected_code=200).data + + self.assertEqual(data['key'], key) + self.assertEqual(data['plugin'], slug) + self.assertEqual(data['value'], '***') # Protected setting should return '***' + + # Check that the setting has been created + self.assertTrue( + PluginSetting.objects.filter(plugin=plugin.pk, key=key).exists() + ) + + def test_cast(self): + """Test type casting for plugin settings.""" + slug = 'sample' + key = 'NUMERICAL_SETTING' + + registry.set_plugin_state(slug, True) + url = reverse('api-plugin-setting-detail', kwargs={'plugin': slug, 'key': key}) + + for value in ['-1', '0', '7777']: + response = self.patch(url, {'value': value}, expected_code=200) + + # Check that the returned response is correctly cast to an integer + self.assertEqual(response.data['value'], int(value)) + + +class PluginUserSettingsApiTest(PluginMixin, InvenTreeAPITestCase): + """Tests for the plugin user settings API.""" + + def setUp(self): + """Ensure plugin is activated.""" + registry.set_plugin_state('sample', True) + + super().setUp() + + def test_user_setting_list(self): + """Test the plugin user setting list API.""" + url = reverse('api-plugin-user-setting-list', kwargs={'plugin': 'sample'}) + + response = self.get(url, expected_code=200) + self.assertEqual(len(response.data), 3) + + def test_cast(self): + """Test the plugin values are cast appropriately.""" + slug = 'sample' + key = 'USER_SETTING_2' + + url = reverse( + 'api-plugin-user-setting-detail', kwargs={'plugin': slug, 'key': key} + ) + + for value in [True, False]: + response = self.patch(url, {'value': str(value)}) + + self.assertEqual(response.data['value'], value) + self.assertEqual(response.data['key'], key) + self.assertEqual(response.data['name'], 'User Setting 2') class ErrorReportTest(InvenTreeAPITestCase): diff --git a/src/backend/InvenTree/machine/models.py b/src/backend/InvenTree/machine/models.py index 0e4852c64e..e22d32741b 100755 --- a/src/backend/InvenTree/machine/models.py +++ b/src/backend/InvenTree/machine/models.py @@ -145,6 +145,12 @@ class MachineSetting(common.models.BaseInvenTreeSetting): MACHINE = 'M', _('Machine') DRIVER = 'D', _('Driver') + def to_native_value(self): + """Return the 'native' value of this setting.""" + return self.__class__.get_setting( + self.key, machine_config=self.machine_config, config_type=self.config_type + ) + machine_config = models.ForeignKey( MachineConfig, related_name='settings', diff --git a/src/backend/InvenTree/plugin/models.py b/src/backend/InvenTree/plugin/models.py index 69538cc104..2d6cc836ca 100644 --- a/src/backend/InvenTree/plugin/models.py +++ b/src/backend/InvenTree/plugin/models.py @@ -285,6 +285,10 @@ class PluginSetting(common.models.BaseInvenTreeSetting): unique_together = [('plugin', 'key')] + def to_native_value(self): + """Return the 'native' value of this setting.""" + return self.__class__.get_setting(self.key, plugin=self.plugin) + plugin = models.ForeignKey( PluginConfig, related_name='settings', diff --git a/src/backend/InvenTree/plugin/test_api.py b/src/backend/InvenTree/plugin/test_api.py index a37dfbef81..91576e4d58 100644 --- a/src/backend/InvenTree/plugin/test_api.py +++ b/src/backend/InvenTree/plugin/test_api.py @@ -389,7 +389,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase): expected_code=200, ) - self.assertEqual(response.data['value'], '456') + self.assertEqual(response.data['value'], 456) # Retrieve the value again response = self.get( @@ -400,7 +400,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase): expected_code=200, ) - self.assertEqual(response.data['value'], '456') + self.assertEqual(response.data['value'], 456) def test_plugin_user_settings(self): """Test the PluginUserSetting API endpoints."""