From 61345dfa4502679b2caf726f8e9a223bbe426d67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Fri, 25 Sep 2026 16:51:53 +0200 Subject: [PATCH 1/4] feat(serverdb): Allow to redirect attributes via alias To ease renaming attributes this feature allows to set up an alias for an attribute so that requests still using the old name continue working and allow a graceful migration. Do not resolve redirect in the Servershell as humans should pick up the new name immediately otherwise it could increase the risk to become a permanent thing because we humans are lazy --- packages/serveradmin/serveradmin/api/views.py | 6 ++- .../serveradmin/serveradmin/serverdb/admin.py | 14 ++++-- .../migrations/0026_attributeredirect.py | 21 +++++++++ .../serveradmin/serverdb/models.py | 43 +++++++++++++++++++ 4 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 packages/serveradmin/serveradmin/serverdb/migrations/0026_attributeredirect.py diff --git a/packages/serveradmin/serveradmin/api/views.py b/packages/serveradmin/serveradmin/api/views.py index 8d084427..ce1e11f9 100644 --- a/packages/serveradmin/serveradmin/api/views.py +++ b/packages/serveradmin/serveradmin/api/views.py @@ -18,7 +18,7 @@ from serveradmin.api.decorators import api_view from serveradmin.dataset import Query from serveradmin.querylog.utils import log_query -from serveradmin.serverdb.models import Attribute +from serveradmin.serverdb.models import Attribute, AttributeRedirect from serveradmin.serverdb.query_committer import commit_query from serveradmin.serverdb.query_executer import execute_query from serveradmin.serverdb.query_materializer import ( @@ -67,6 +67,10 @@ def dataset_query(request, app, data): order_by = data.get('order_by') start = monotonic() + + # Resolve alias attributes to real attributes + filters, restrict, order_by = AttributeRedirect.resolve_aliases(filters, restrict, order_by) + result = execute_query(filters, restrict, order_by) duration_seconds = monotonic() - start diff --git a/packages/serveradmin/serveradmin/serverdb/admin.py b/packages/serveradmin/serveradmin/serverdb/admin.py index 0edb6a7c..52e0a087 100644 --- a/packages/serveradmin/serveradmin/serverdb/admin.py +++ b/packages/serveradmin/serveradmin/serverdb/admin.py @@ -1,6 +1,6 @@ """Serveradmin - Django Admin Setup -Copyright (c) 2019 InnoGames GmbH +Copyright (c) 2026 InnoGames GmbH """ from django.contrib import admin @@ -14,9 +14,8 @@ Servertype, Attribute, ServertypeAttribute, - Server, ServerRelationAttribute, - ServerStringAttribute, + ServerStringAttribute, AttributeRedirect, ) @@ -106,5 +105,14 @@ def get_hovertext(self, obj): ) +class AttributeRedirectAdmin(admin.ModelAdmin): + model = AttributeRedirect + + list_display = ['alias', 'target', ] + search_fields = ['alias', 'target', ] + list_filter = ['alias', 'target', ] + + admin.site.register(Servertype, ServertypeAdmin) admin.site.register(Attribute, AttributeAdmin) +admin.site.register(AttributeRedirect, AttributeRedirectAdmin) diff --git a/packages/serveradmin/serveradmin/serverdb/migrations/0026_attributeredirect.py b/packages/serveradmin/serveradmin/serverdb/migrations/0026_attributeredirect.py new file mode 100644 index 00000000..7a03fb3a --- /dev/null +++ b/packages/serveradmin/serveradmin/serverdb/migrations/0026_attributeredirect.py @@ -0,0 +1,21 @@ +# Generated by Django 5.2.17 on 2026-09-25 14:14 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('serverdb', '0025_rename_serverbooleanattribute_attribute_server_bool_attribu_25fb6c_idx_and_more'), + ] + + operations = [ + migrations.CreateModel( + name='AttributeRedirect', + fields=[ + ('alias', models.CharField(help_text="The 'virtual' attribute name (e.g. old attribute)", max_length=32, primary_key=True, serialize=False)), + ('target', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='serverdb.attribute')), + ], + ), + ] diff --git a/packages/serveradmin/serveradmin/serverdb/models.py b/packages/serveradmin/serveradmin/serverdb/models.py index 882e2731..08d3c275 100644 --- a/packages/serveradmin/serveradmin/serverdb/models.py +++ b/packages/serveradmin/serveradmin/serverdb/models.py @@ -402,6 +402,49 @@ def clean(self): super(Attribute, self).clean() +class AttributeRedirect(models.Model): + """Redirect alias to an existing attribute + + Purpose of this is to allow graceful renaming of attributes. + + One can delete and attribute create a new one and set up a redirect + from the old name to the new name. + """ + + alias = models.CharField( + max_length=32, + help_text="The 'virtual' attribute name (e.g. old attribute)", + primary_key=True, + ) + target = models.ForeignKey(Attribute, on_delete=models.CASCADE) + + def clean(self): + super().clean() + + if Attribute.objects.filter(attribute_id=self.alias).exists(): + raise ValidationError({ + "alias": "Creating a alias that matches an existing attribute not allowed!" + }) + + @classmethod + def resolve_aliases(cls, *objs): + """Resolve alias names in the given objects to real attribute_ids.""" + mapping = dict(cls.objects.values_list('alias', 'target_id')) + return tuple(_apply_aliases(mapping, obj) for obj in objs) + + +def _apply_aliases(mapping, obj): + if obj is None: + return None + if isinstance(obj, str): + return mapping.get(obj, obj) + if isinstance(obj, list): + return [mapping.get(item, item) for item in obj] + if isinstance(obj, dict): + return {mapping.get(key, key): value for key, value in obj.items()} + raise TypeError(f'Unsupported type {type(obj).__name__}') + + class ServerTableSpecial(object): def __init__(self, field, unique=False): self.field = field From 953015dbc81c0481aa8ba486b4b188ce0038540d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Mon, 28 Sep 2026 13:40:28 +0200 Subject: [PATCH 2/4] Fix FieldError when searching --- packages/serveradmin/serveradmin/serverdb/admin.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/serveradmin/serveradmin/serverdb/admin.py b/packages/serveradmin/serveradmin/serverdb/admin.py index 52e0a087..9f94bdb7 100644 --- a/packages/serveradmin/serveradmin/serverdb/admin.py +++ b/packages/serveradmin/serveradmin/serverdb/admin.py @@ -108,9 +108,9 @@ def get_hovertext(self, obj): class AttributeRedirectAdmin(admin.ModelAdmin): model = AttributeRedirect - list_display = ['alias', 'target', ] - search_fields = ['alias', 'target', ] - list_filter = ['alias', 'target', ] + list_display = ['alias', 'target__attribute_id', ] + search_fields = ['alias', 'target__attribute_id', ] + list_filter = ['alias', 'target__attribute_id', ] admin.site.register(Servertype, ServertypeAdmin) From 02a0f9198eda8e358facebc2e5ce8540a2630644 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Mon, 28 Sep 2026 14:07:42 +0200 Subject: [PATCH 3/4] Fix TypeError: unhashable type: 'dict' Forgot about the joined queries. Added a test with Claude --- .../serveradmin/serverdb/models.py | 25 +++++++++- .../serverdb/tests/test_attribute_redirect.py | 49 +++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 packages/serveradmin/serveradmin/serverdb/tests/test_attribute_redirect.py diff --git a/packages/serveradmin/serveradmin/serverdb/models.py b/packages/serveradmin/serveradmin/serverdb/models.py index 08d3c275..4294dba5 100644 --- a/packages/serveradmin/serveradmin/serverdb/models.py +++ b/packages/serveradmin/serveradmin/serverdb/models.py @@ -434,14 +434,35 @@ def resolve_aliases(cls, *objs): def _apply_aliases(mapping, obj): + """Recursively replace alias attribute names in a query argument + + Supported shapes are the ones passed to a query: + + * ``filters``: ``{attribute_id: filter}``, only keys are attribute names, + the filter objects are left untouched. + * ``restrict``: ``[attribute_id, {attribute_id: [restrict, ...]}, ...]``, + where a dictionary item is a join into a related object whose value is + again a restrict clause. + * ``order_by``: ``[attribute_id, ...]`` + """ if obj is None: return None if isinstance(obj, str): return mapping.get(obj, obj) if isinstance(obj, list): - return [mapping.get(item, item) for item in obj] + return [ + _apply_aliases(mapping, item) + if isinstance(item, (str, list, dict)) else item + for item in obj + ] if isinstance(obj, dict): - return {mapping.get(key, key): value for key, value in obj.items()} + # Only keys are attribute names. A value is either a nested restrict + # clause of a join (list) or a filter value, which must stay as is. + return { + mapping.get(key, key): _apply_aliases(mapping, value) + if isinstance(value, (list, dict)) else value + for key, value in obj.items() + } raise TypeError(f'Unsupported type {type(obj).__name__}') diff --git a/packages/serveradmin/serveradmin/serverdb/tests/test_attribute_redirect.py b/packages/serveradmin/serveradmin/serverdb/tests/test_attribute_redirect.py new file mode 100644 index 00000000..0cd102d2 --- /dev/null +++ b/packages/serveradmin/serveradmin/serverdb/tests/test_attribute_redirect.py @@ -0,0 +1,49 @@ +from django.test import SimpleTestCase + +from adminapi.filters import Regexp +from serveradmin.serverdb.models import _apply_aliases + + +class ApplyAliasesTestCase(SimpleTestCase): + mapping = {'old_name': 'new_name', 'old_code': 'short_code'} + + def test_none(self): + self.assertIsNone(_apply_aliases(self.mapping, None)) + + def test_string(self): + self.assertEqual(_apply_aliases(self.mapping, 'old_name'), 'new_name') + self.assertEqual(_apply_aliases(self.mapping, 'hostname'), 'hostname') + + def test_flat_restrict(self): + self.assertEqual( + _apply_aliases(self.mapping, ['hostname', 'old_name']), + ['hostname', 'new_name'], + ) + + def test_restrict_with_joins(self): + restrict = [{'old_name': ['old_code', 'object_id']}, 'hostname'] + self.assertEqual( + _apply_aliases(self.mapping, restrict), + [{'new_name': ['short_code', 'object_id']}, 'hostname'], + ) + + def test_restrict_with_nested_joins(self): + restrict = [{'project': [{'old_name': ['old_code']}, 'hostname']}] + self.assertEqual( + _apply_aliases(self.mapping, restrict), + [{'project': [{'new_name': ['short_code']}, 'hostname']}], + ) + + def test_filters_keep_filter_objects(self): + regexp = Regexp('^foo') + # A plain string value is a filter value, not an attribute name, + # so it must not be rewritten even if it matches an alias. + filters = {'old_name': regexp, 'hostname': 'old_code'} + result = _apply_aliases(self.mapping, filters) + self.assertEqual(set(result), {'new_name', 'hostname'}) + self.assertIs(result['new_name'], regexp) + self.assertEqual(result['hostname'], 'old_code') + + def test_unsupported_type(self): + with self.assertRaises(TypeError): + _apply_aliases(self.mapping, 42) From 097d386399085400bc3c9c1681a918cf7eb80441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Tue, 29 Sep 2026 16:00:55 +0200 Subject: [PATCH 4/4] Fix KeyError when displaying results When querying objects and accessing them by the alias we must ensure the results are present via the alias attribute again. --- packages/serveradmin/serveradmin/api/views.py | 5 +- .../serveradmin/serverdb/models.py | 73 +++++++++- .../serverdb/tests/test_attribute_redirect.py | 128 +++++++++++++++++- 3 files changed, 202 insertions(+), 4 deletions(-) diff --git a/packages/serveradmin/serveradmin/api/views.py b/packages/serveradmin/serveradmin/api/views.py index ce1e11f9..bd451ec1 100644 --- a/packages/serveradmin/serveradmin/api/views.py +++ b/packages/serveradmin/serveradmin/api/views.py @@ -68,10 +68,13 @@ def dataset_query(request, app, data): start = monotonic() - # Resolve alias attributes to real attributes + # Resolve alias attributes to real attributes. The requested restrict + # is kept to rename the attributes in the results back to the aliases. + requested_restrict = restrict filters, restrict, order_by = AttributeRedirect.resolve_aliases(filters, restrict, order_by) result = execute_query(filters, restrict, order_by) + result = AttributeRedirect.restore_aliases(requested_restrict, result) duration_seconds = monotonic() - start # Query(...) is instantiated here only for its repr(); it is never diff --git a/packages/serveradmin/serveradmin/serverdb/models.py b/packages/serveradmin/serveradmin/serverdb/models.py index 4294dba5..c3926faf 100644 --- a/packages/serveradmin/serveradmin/serverdb/models.py +++ b/packages/serveradmin/serveradmin/serverdb/models.py @@ -426,12 +426,34 @@ def clean(self): "alias": "Creating a alias that matches an existing attribute not allowed!" }) + @classmethod + def get_mapping(cls): + """Return a dictionary mapping alias names to real attribute_ids""" + return dict(cls.objects.values_list('alias', 'target_id')) + @classmethod def resolve_aliases(cls, *objs): """Resolve alias names in the given objects to real attribute_ids.""" - mapping = dict(cls.objects.values_list('alias', 'target_id')) + mapping = cls.get_mapping() return tuple(_apply_aliases(mapping, obj) for obj in objs) + @classmethod + def restore_aliases(cls, restrict, results): + """Rename attributes in query results back to the requested aliases + + ``restrict`` must be the clause as the client sent it, before the + aliases were resolved. ``results`` are modified in place and + returned for convenience. + """ + if restrict is None: + return results + + mapping = cls.get_mapping() + if mapping: + _restore_aliases(mapping, restrict, results) + + return results + def _apply_aliases(mapping, obj): """Recursively replace alias attribute names in a query argument @@ -466,6 +488,55 @@ def _apply_aliases(mapping, obj): raise TypeError(f'Unsupported type {type(obj).__name__}') +def _restore_aliases(mapping, restrict, results): + """Rename real attribute_ids in results back to the names in restrict + + The query is executed with the resolved attribute_ids, so the results + are keyed by the real names. The client however expects the names it + asked for. Joins are followed recursively. If both the alias and the + real attribute are requested, the value ends up under both names. + + The results are dictionaries, usually DatasetObjects. Their base dict + methods are used on purpose, so the objects are not marked as changed. + """ + # real attribute_id -> [(requested name, sub restrict or None), ...] + requested = {} + for item in restrict: + if isinstance(item, dict): + for name, sub_restrict in item.items(): + real_id = mapping.get(name, name) + requested.setdefault(real_id, []).append((name, sub_restrict)) + else: + real_id = mapping.get(item, item) + requested.setdefault(real_id, []).append((item, None)) + + # Nothing to do for attributes requested by their real name without join + requested = { + real_id: names for real_id, names in requested.items() + if any(name != real_id or sub is not None for name, sub in names) + } + if not requested: + return + + for obj in results: + for real_id, names in requested.items(): + if real_id not in obj: + continue + + if any(name == real_id for name, _ in names): + value = dict.__getitem__(obj, real_id) + else: + value = dict.pop(obj, real_id) + + for name, sub_restrict in names: + if sub_restrict is not None and value is not None: + if isinstance(value, (set, frozenset, list, tuple)): + _restore_aliases(mapping, sub_restrict, value) + else: + _restore_aliases(mapping, sub_restrict, [value]) + dict.__setitem__(obj, name, value) + + class ServerTableSpecial(object): def __init__(self, field, unique=False): self.field = field diff --git a/packages/serveradmin/serveradmin/serverdb/tests/test_attribute_redirect.py b/packages/serveradmin/serveradmin/serverdb/tests/test_attribute_redirect.py index 0cd102d2..6d7dce7d 100644 --- a/packages/serveradmin/serveradmin/serverdb/tests/test_attribute_redirect.py +++ b/packages/serveradmin/serveradmin/serverdb/tests/test_attribute_redirect.py @@ -1,7 +1,16 @@ -from django.test import SimpleTestCase +from django.contrib.auth.models import User +from django.test import SimpleTestCase, TransactionTestCase +from adminapi.dataset import DatasetObject from adminapi.filters import Regexp -from serveradmin.serverdb.models import _apply_aliases +from serveradmin.api.views import dataset_query +from serveradmin.apps.models import Application +from serveradmin.serverdb.models import ( + Attribute, + AttributeRedirect, + _apply_aliases, + _restore_aliases, +) class ApplyAliasesTestCase(SimpleTestCase): @@ -47,3 +56,118 @@ def test_filters_keep_filter_objects(self): def test_unsupported_type(self): with self.assertRaises(TypeError): _apply_aliases(self.mapping, 42) + + +class RestoreAliasesTestCase(SimpleTestCase): + mapping = {'operating_system': 'os', 'hv': 'hypervisor', 'name': 'hostname'} + + def test_flat_alias_is_renamed(self): + results = [{'hostname': 'test0', 'os': 'wheezy'}] + _restore_aliases(self.mapping, ['hostname', 'operating_system'], results) + self.assertEqual(results, [{'hostname': 'test0', 'operating_system': 'wheezy'}]) + + def test_real_name_is_untouched(self): + results = [{'hostname': 'test0', 'os': 'wheezy'}] + _restore_aliases(self.mapping, ['hostname', 'os'], results) + self.assertEqual(results, [{'hostname': 'test0', 'os': 'wheezy'}]) + + def test_alias_and_real_name_both_requested(self): + results = [{'os': 'wheezy'}] + _restore_aliases(self.mapping, ['os', 'operating_system'], results) + self.assertEqual(results, [{'os': 'wheezy', 'operating_system': 'wheezy'}]) + + def test_missing_attribute_is_not_added(self): + results = [{'hostname': 'test0'}] + _restore_aliases(self.mapping, ['hostname', 'operating_system'], results) + self.assertEqual(results, [{'hostname': 'test0'}]) + + def test_join_single_relation(self): + results = [{'hostname': 'vm-1', 'hypervisor': {'hostname': 'hv-1', 'os': 'wheezy'}}] + _restore_aliases(self.mapping, ['hostname', {'hv': ['name', 'operating_system']}], results) + self.assertEqual(results, [ + {'hostname': 'vm-1', 'hv': {'name': 'hv-1', 'operating_system': 'wheezy'}}, + ]) + + def test_join_with_none_value(self): + results = [{'hostname': 'vm-1', 'hypervisor': None}] + _restore_aliases(self.mapping, [{'hv': ['name']}], results) + self.assertEqual(results, [{'hostname': 'vm-1', 'hv': None}]) + + def test_join_multi_relation(self): + vm1 = DatasetObject({'hostname': 'vm-1', 'os': 'wheezy'}, 7) + vm2 = DatasetObject({'hostname': 'vm-2', 'os': 'squeeze'}, 8) + results = [DatasetObject({'hostname': 'hv-1', 'vms': [vm1, vm2]}, 6)] + _restore_aliases(self.mapping, [{'vms': ['name', 'operating_system']}], results) + + self.assertEqual(set(results[0]), {'hostname', 'vms'}) + self.assertEqual( + sorted(sorted(vm.items()) for vm in results[0]['vms']), + [ + [('name', 'vm-1'), ('operating_system', 'wheezy')], + [('name', 'vm-2'), ('operating_system', 'squeeze')], + ], + ) + + def test_join_real_name_with_nested_alias(self): + results = [{'hypervisor': {'os': 'wheezy'}}] + _restore_aliases(self.mapping, [{'hypervisor': ['operating_system']}], results) + self.assertEqual(results, [{'hypervisor': {'operating_system': 'wheezy'}}]) + + def test_dataset_objects_stay_clean(self): + nested = DatasetObject({'hostname': 'hv-1', 'os': 'wheezy'}, 6) + obj = DatasetObject({'hostname': 'vm-1', 'os': 'squeeze', 'hypervisor': nested}, 7) + _restore_aliases( + self.mapping, + ['operating_system', {'hv': ['operating_system']}], + [obj], + ) + self.assertEqual( + obj, {'hostname': 'vm-1', 'operating_system': 'squeeze', 'hv': nested}, + ) + self.assertEqual(nested, {'hostname': 'hv-1', 'operating_system': 'wheezy'}) + self.assertFalse(obj.is_dirty()) + self.assertFalse(nested.is_dirty()) + + +class DatasetQueryAliasTestCase(TransactionTestCase): + fixtures = ['test_dataset.json'] + + def setUp(self): + super().setUp() + user = User.objects.create_user('alice') + self.app = Application.objects.create(name='app', owner=user, location='') + AttributeRedirect.objects.create( + alias='operating_system', target=Attribute.objects.get(pk='os'), + ) + AttributeRedirect.objects.create( + alias='hv', target=Attribute.objects.get(pk='hypervisor'), + ) + + def _query(self, filters, restrict): + response = dataset_query.__wrapped__( + None, self.app, {'filters': filters, 'restrict': restrict}, + ) + self.assertEqual(response['status'], 'success') + return response['result'] + + def test_restrict_alias_is_returned_as_alias(self): + result = self._query({'hostname': 'test0'}, ['hostname', 'operating_system']) + self.assertEqual(result, [{'hostname': 'test0', 'operating_system': 'wheezy'}]) + + def test_restrict_alias_and_real_name(self): + result = self._query({'hostname': 'test0'}, ['os', 'operating_system']) + self.assertEqual(result, [{'os': 'wheezy', 'operating_system': 'wheezy'}]) + + def test_restrict_join_alias(self): + result = self._query({'hostname': 'vm-1'}, ['hostname', {'hv': ['hostname']}]) + self.assertEqual(result, [{'hostname': 'vm-1', 'hv': {'hostname': 'hv-1'}}]) + + def test_filter_alias_resolves_objects(self): + result = self._query({'operating_system': 'wheezy'}, ['hostname']) + self.assertEqual(result, [{'hostname': 'test0'}]) + + def test_no_restrict_returns_real_names(self): + result = self._query({'hostname': 'test0'}, []) + self.assertEqual(len(result), 1) + self.assertIn('os', result[0]) + self.assertNotIn('operating_system', result[0])