diff --git a/adminapi/parse.py b/adminapi/parse.py index 6b552091..b070c723 100644 --- a/adminapi/parse.py +++ b/adminapi/parse.py @@ -193,3 +193,88 @@ def parse_function_string(args, strict=True): # NOQA C901 parsed_args.append(('literal', args[string_start:])) return parsed_args + + +def build_query(query_args): + """Build a text query from parsed query arguments. + + This is the inverse of parse_query(). It takes a dictionary mapping + attribute names to filter values and returns a text query string. + + Args: + query_args: A dictionary like {'hostname': BaseFilter('web.*'), + 'state': BaseFilter('online')} + + Returns: + A text query string like 'hostname=Regexp(web.*) state=online' + """ + if not query_args: + return '' + + parts = [] + for attr, filter_obj in query_args.items(): + value_str = _format_filter_value(filter_obj) + parts.append(f'{attr}={value_str}') + + return ' '.join(parts) + + +def _format_filter_value(filter_obj): + """Format a filter object as a text query value. + + Args: + filter_obj: A BaseFilter instance or subclass + + Returns: + A string representation suitable for text queries + """ + if not isinstance(filter_obj, BaseFilter): + # Plain value, format it directly + return _format_literal(filter_obj) + + filter_type = type(filter_obj) + + # BaseFilter with plain value - no function wrapper needed + if filter_type == BaseFilter: + return _format_literal(filter_obj.value) + + # Empty filter - no arguments + if filter_type.__name__ == 'Empty': + return 'Empty()' + + # Any/All filters - multiple values + if hasattr(filter_obj, 'values'): + inner_parts = [_format_filter_value(v) for v in filter_obj.values] + return '{}({})'.format(filter_type.__name__, ' '.join(inner_parts)) + + # Not filter - single nested filter + if filter_type.__name__ == 'Not': + inner = _format_filter_value(filter_obj.value) + return '{}({})'.format(filter_type.__name__, inner) + + # Other filters (Regexp, GreaterThan, etc.) - single value + return '{}({})'.format(filter_type.__name__, _format_literal(filter_obj.value)) + + +def _format_literal(value): + """Format a literal value for text query output. + + Args: + value: A Python value (str, int, bool, etc.) + + Returns: + A string representation suitable for text queries + """ + if isinstance(value, bool): + return 'true' if value else 'false' + + if isinstance(value, str): + # Check if the string needs quoting (contains spaces or special chars) + if ' ' in value or '(' in value or ')' in value or '=' in value: + # Escape backslashes and quotes, then quote the string + escaped = value.replace('\\', '\\\\').replace('"', '\\"') + return '"{}"'.format(escaped) + return value + + # For numbers, IP addresses, dates, etc. - use string representation + return str(value) diff --git a/adminapi/tests/test_parse.py b/adminapi/tests/test_parse.py index aa7e5e52..e948f492 100644 --- a/adminapi/tests/test_parse.py +++ b/adminapi/tests/test_parse.py @@ -1,12 +1,16 @@ import unittest from adminapi.datatype import DatatypeError +from adminapi.filters import ( + All, Not, Empty, +) from adminapi.filters import ( BaseFilter, Regexp, Any, GreaterThan, ) +from adminapi.parse import build_query from adminapi.parse import parse_query, parse_function_string @@ -101,7 +105,7 @@ def test_invalid_function(self): def test_top_level_literal_error(self): with self.assertRaisesRegex( - DatatypeError, r"Invalid term: Top level literals are not allowed" + DatatypeError, r"Invalid term: Top level literals are not allowed" ): parse_query("hostname=test value") @@ -185,3 +189,244 @@ def test_parentheses_handling(self): ("endfunc", ""), ] self.assertEqual(result, expected) + + +class TestBuildQuery(unittest.TestCase): + """Tests for the build_query function.""" + + def test_simple_filter(self): + """Test building a query with a simple attribute filter.""" + query = 'servertype=vm' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'servertype=vm') + + def test_multiple_attributes(self): + """Test building a query with multiple attributes.""" + query = 'servertype=vm state=online' + parsed = parse_query(query) + rebuilt = build_query(parsed) + # Order may vary due to dict iteration, check both attributes present + self.assertIn('servertype=vm', rebuilt) + self.assertIn('state=online', rebuilt) + + def test_regexp_filter_explicit(self): + """Test building a query with explicit Regexp filter.""" + query = 'hostname=Regexp(web.*)' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'hostname=Regexp(web.*)') + + def test_any_filter(self): + """Test building a query with Any filter.""" + query = 'state=Any(online offline)' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'state=Any(online offline)') + + def test_all_filter(self): + """Test building a query with All filter.""" + query = 'tags=All(production web)' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'tags=All(production web)') + + def test_not_filter(self): + """Test building a query with Not filter.""" + query = 'state=Not(offline)' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'state=Not(offline)') + + def test_empty_filter(self): + """Test building a query with Empty filter.""" + query = 'comment=Empty()' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'comment=Empty()') + + def test_numeric_value(self): + """Test building a query with numeric value.""" + query = 'cpu_cores=4' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'cpu_cores=4') + + def test_boolean_true(self): + """Test building a query with boolean true value.""" + query = 'active=true' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'active=true') + + def test_boolean_false(self): + """Test building a query with boolean false value.""" + query = 'active=false' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'active=false') + + def test_hostname_only_plain(self): + """Test building a query with plain hostname.""" + query = 'webserver01' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'hostname=webserver01') + + def test_hostname_only_with_regex(self): + """Test building a query with hostname containing regex chars.""" + query = 'web.*' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'hostname=Regexp(web.*)') + + def test_greater_than_filter(self): + """Test building a query with GreaterThan filter.""" + query = 'cpu_cores=GreaterThan(4)' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'cpu_cores=GreaterThan(4)') + + def test_less_than_filter(self): + """Test building a query with LessThan filter.""" + query = 'memory=LessThan(8192)' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'memory=LessThan(8192)') + + def test_nested_not_any(self): + """Test building a query with nested Not(Any(...)).""" + query = 'state=Not(Any(offline maintenance))' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'state=Not(Any(offline maintenance))') + + def test_nested_any_not(self): + """Test building a query with nested Any containing Not.""" + query = 'state=Any(online Not(offline))' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'state=Any(online Not(offline))') + + def test_empty_query(self): + """Test building an empty query.""" + parsed = {} + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, '') + + def test_contains_filter(self): + """Test building a query with Contains filter.""" + query = 'hostname=Contains(web)' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'hostname=Contains(web)') + + def test_startswith_filter(self): + """Test building a query with StartsWith filter.""" + query = 'hostname=StartsWith(web)' + parsed = parse_query(query) + rebuilt = build_query(parsed) + self.assertEqual(rebuilt, 'hostname=StartsWith(web)') + + +class TestBuildQueryRoundTrip(unittest.TestCase): + """Tests to verify parse_query and build_query are inverses.""" + + def assert_filters_equal(self, filter1, filter2): + """Recursively compare two filter objects for equality.""" + self.assertEqual(type(filter1), type(filter2)) + + if hasattr(filter1, 'values'): + # Any/All filters have multiple values + self.assertEqual(len(filter1.values), len(filter2.values)) + for v1, v2 in zip(filter1.values, filter2.values): + self.assert_filters_equal(v1, v2) + elif hasattr(filter1, 'value'): + # Single value filters (BaseFilter, Not, Regexp, etc.) + if isinstance(filter1.value, BaseFilter): + self.assert_filters_equal(filter1.value, filter2.value) + else: + self.assertEqual(filter1.value, filter2.value) + + def assert_round_trip(self, query): + """Assert that parsing and rebuilding produces equivalent result.""" + parsed = parse_query(query) + rebuilt = build_query(parsed) + reparsed = parse_query(rebuilt) + + # Compare keys + self.assertEqual(set(parsed.keys()), set(reparsed.keys())) + + # Compare filter types and values recursively + for key in parsed: + self.assert_filters_equal(parsed[key], reparsed[key]) + + def test_round_trip_simple(self): + """Test round-trip for simple query.""" + self.assert_round_trip('servertype=vm') + + def test_round_trip_any(self): + """Test round-trip for Any filter.""" + self.assert_round_trip('state=Any(online offline)') + + def test_round_trip_not(self): + """Test round-trip for Not filter.""" + self.assert_round_trip('state=Not(offline)') + + def test_round_trip_nested(self): + """Test round-trip for nested filters.""" + self.assert_round_trip('state=Not(Any(offline maintenance))') + + def test_round_trip_numeric(self): + """Test round-trip for numeric value.""" + self.assert_round_trip('cpu_cores=4') + + def test_round_trip_boolean(self): + """Test round-trip for boolean value.""" + self.assert_round_trip('active=true') + + +class TestBuildQueryDirectConstruction(unittest.TestCase): + """Tests for build_query with directly constructed filter objects.""" + + def test_direct_base_filter(self): + """Test building query from directly constructed BaseFilter.""" + query_args = {'servertype': BaseFilter('vm')} + rebuilt = build_query(query_args) + self.assertEqual(rebuilt, 'servertype=vm') + + def test_direct_regexp(self): + """Test building query from directly constructed Regexp.""" + query_args = {'hostname': Regexp('web.*')} + rebuilt = build_query(query_args) + self.assertEqual(rebuilt, 'hostname=Regexp(web.*)') + + def test_direct_any(self): + """Test building query from directly constructed Any.""" + query_args = {'state': Any('online', 'offline')} + rebuilt = build_query(query_args) + self.assertEqual(rebuilt, 'state=Any(online offline)') + + def test_direct_not(self): + """Test building query from directly constructed Not.""" + query_args = {'state': Not('offline')} + rebuilt = build_query(query_args) + self.assertEqual(rebuilt, 'state=Not(offline)') + + def test_direct_empty(self): + """Test building query from directly constructed Empty.""" + query_args = {'comment': Empty()} + rebuilt = build_query(query_args) + self.assertEqual(rebuilt, 'comment=Empty()') + + def test_direct_nested(self): + """Test building query from nested filter construction.""" + query_args = {'state': Not(Any('offline', 'maintenance'))} + rebuilt = build_query(query_args) + self.assertEqual(rebuilt, 'state=Not(Any(offline maintenance))') + + def test_direct_all(self): + """Test building query from directly constructed All.""" + query_args = {'tags': All('production', 'web')} + rebuilt = build_query(query_args) + self.assertEqual(rebuilt, 'tags=All(production web)') diff --git a/docs/sql_generator_v2_context.md b/docs/sql_generator_v2_context.md new file mode 100644 index 00000000..556423a4 --- /dev/null +++ b/docs/sql_generator_v2_context.md @@ -0,0 +1,188 @@ +# SQL Generator V2 - Implementation Context + +## Overview + +This document provides context for continuing development on the SQL Generator V2 rewrite for the serveradmin project. The goal is to replace raw SQL string concatenation with Django ORM for improved security and maintainability. + +## Project Structure + +``` +serveradmin/serverdb/ +├── sql_generator.py # Original v1 (raw SQL) - UNCHANGED +├── sql_generator_v2.py # New v2 (Django ORM) - NEW +├── query_builder.py # Facade for switching v1/v2 - NEW +├── query_executer.py # Modified to use facade +└── tests/ + └── test_sql_generator_v2.py # Unit tests - NEW +``` + +## Configuration + +Switch between implementations via Django setting in `serveradmin/settings.py`: +```python +SQL_GENERATOR_VERSION = env('SQL_GENERATOR_VERSION', default='v1') # 'v1' or 'v2' +``` + +## Key Design Decisions + +### 1. Return Type Difference +- **V1**: Returns raw SQL string +- **V2**: Returns Django QuerySet + +`query_executer.py` handles both: +```python +if isinstance(query_result, str): + return list(Server.objects.defer('intern_ip').raw(query_result)) +else: + return list(query_result) # No defer() - causes N+1 queries! +``` + +### 2. OuterRef Limitations +Django's `OuterRef` only references the **immediate outer query**. For nested subqueries, use `RawSQL` with direct table references like `"server"."server_id"`. + +**Wrong** (causes "can't adapt type 'OuterRef'" error): +```python +server_id__in=Subquery( + SomeModel.objects.filter(server_id=OuterRef('server_id')) # References wrong table! +) +``` + +**Correct**: +```python +server_id__in=RawSQL( + 'SELECT ... WHERE server_id = "server"."server_id"', # Direct reference + [] +) +``` + +### 3. Performance: Non-Correlated Subqueries +For supernet lookups, hostname filters must be **non-correlated** (evaluated once): + +**Slow** (hostname filter after correlated subquery): +```sql +EXISTS (SELECT FROM server U0 WHERE U0.id IN (correlated) AND U0.hostname = 'x') +``` + +**Fast** (hostname filter as non-correlated subquery): +```sql +EXISTS (SELECT FROM server supernet ... + AND supernet.id IN (SELECT id FROM server WHERE hostname = 'x')) +``` + +## Commits Made (on branch `dk_sql_generator_v3`) + +1. `e2963275` - Initial implementation with facade, settings, v2 module, tests +2. `007a43ce` - Fix OuterRef error in supernet-related queries +3. `7df4a43a` - Fix N+1 query (remove defer('intern_ip') for v2) +4. `104fa652` - Fix related_via for relation/reverse types (OuterRef bug) +5. `1ebb835e` - Optimize supernet lookup with non-correlated hostname subquery + +## Key Functions in sql_generator_v2.py + +### Entry Point +- `get_server_query(attribute_filters, related_vias)` → Returns QuerySet + +### Filter Building +- `_build_attribute_condition(attribute, filt, related_vias)` → Q object +- `_build_value_condition(attribute, filt)` → Q object for value comparison +- `_build_logical_condition(attribute, filt, related_vias)` → Handles Any/All/Not + +### Attribute Lookups +- `_build_attribute_lookup(attribute, value_condition, related_vias)` → Routes to specific builders +- `_build_special_lookup(attribute, value_condition)` → hostname, servertype, object_id, intern_ip +- `_build_standard_lookup(attribute, value_condition, related_vias)` → Regular attributes +- `_build_supernet_lookup(attribute, value_condition)` → Supernet type (inet containment) +- `_build_domain_lookup(attribute, value_condition)` → Domain type +- `_build_reverse_lookup(attribute, value_condition)` → Reverse relations + +### Related Via Handling +- `_build_related_via_condition(attribute, model, value_condition, related_via, empty_filter)` +- `_build_supernet_related_query(attribute, model, base_filter, related_via)` + +### Helpers +- `_build_hostname_filter_sql(value_condition)` → Converts Q to SQL for hostname +- `_transform_to_hostname(value_condition)` → Transforms value→hostname lookups +- `_transform_inet_condition(value_condition)` → Handles inet operators +- `_apply_value_condition(model, value_condition)` → Applies inet transforms if needed + +## Data Model Summary + +### Key Models +- `Server` - Main entity (server_id, hostname, intern_ip, servertype_id) +- `Attribute` - Attribute definitions (type: string, boolean, relation, inet, supernet, domain, reverse, etc.) +- `ServertypeAttribute` - Links servertypes to attributes, has `related_via_attribute` +- `ServerStringAttribute`, `ServerRelationAttribute`, `ServerInetAttribute`, etc. - Attribute values + +### Special Attributes (hardcoded) +- `object_id` → server.server_id +- `hostname` → server.hostname +- `servertype` → server.servertype_id +- `intern_ip` → server.intern_ip + +### related_vias Dictionary +Structure: `{attribute_id: {related_via_attribute: [servertype_ids]}}` +- `related_via_attribute = None` → Direct lookup +- `related_via_attribute.type = 'relation'` → Follow relation to target server +- `related_via_attribute.type = 'reverse'` → Follow reverse relation +- `related_via_attribute.type = 'supernet'` → Inet containment lookup + +## Filter Types (from adminapi/filters.py) + +| Filter | Django ORM | SQL | +|--------|-----------|-----| +| BaseFilter | `__exact` | `= 'value'` | +| Regexp | `__regex` | `~ 'pattern'` | +| GreaterThan | `__gt` | `> value` | +| Contains (string) | `__contains` | `LIKE '%value%'` | +| Contains (inet) | RawSQL | `>>= 'network'` | +| ContainedBy (inet) | RawSQL | `<<= 'network'` | +| Overlaps (inet) | RawSQL | `&& 'network'` | +| Any | `Q() \| Q()` | `OR` | +| All | `Q() & Q()` | `AND` | +| Not | `~Q()` | `NOT` | +| Empty | Check EXISTS | `IS NOT NULL` then negate | + +## Known Issues / TODOs + +1. **ContainedOnlyBy filter** - Not fully implemented (raises NotImplementedError in filters.py) +2. **Domain type lookups** - Uses `.extra()` which is deprecated +3. **Complex nested logical filters** - May need more testing +4. **Inet operators** - Using RawSQL with table name assumptions + +## Testing + +Run tests: +```bash +python manage.py test serveradmin.serverdb.tests.test_sql_generator_v2 +``` + +Test categories: +- `TestGetServerQueryV2` - Basic functionality +- `TestSpecialAttributeFilters` - hostname, servertype, object_id +- `TestComparisonFilters` - >, <, >=, <= +- `TestLogicalFilters` - Any, All, Not +- `TestStringFilters` - Contains, StartsWith +- `TestV1V2Comparison` - Ensures v1 and v2 return same results +- `TestQueryBuilderFacade` - Switching mechanism + +## Useful Queries for Testing + +``` +hostname=test0 # Simple equality +servertype=vm # Servertype filter +responsible_admin=daniel.kroeger # Related via attribute +network_zones=aw # Related via supernet +route_network=foe-aw-int # Supernet type attribute +``` + +## Files Reference + +| File | Purpose | +|------|---------| +| `serveradmin/settings.py:47` | SQL_GENERATOR_VERSION setting | +| `serveradmin/serverdb/query_builder.py` | Switching facade | +| `serveradmin/serverdb/query_executer.py:260-271` | Handles v1/v2 return types | +| `serveradmin/serverdb/sql_generator.py` | Original v1 implementation | +| `serveradmin/serverdb/sql_generator_v2.py` | New v2 implementation | +| `adminapi/filters.py` | Filter class definitions | +| `serveradmin/serverdb/models.py` | Django models | diff --git a/serveradmin/serverdb/query_builder.py b/serveradmin/serverdb/query_builder.py new file mode 100644 index 00000000..d7645419 --- /dev/null +++ b/serveradmin/serverdb/query_builder.py @@ -0,0 +1,38 @@ +"""Serveradmin - Query Builder Facade + +Copyright (c) 2024 InnoGames GmbH + +This module provides a facade for switching between SQL generator implementations. +The implementation is selected via the SQL_GENERATOR_VERSION Django setting. +""" + +from django.conf import settings + + +def get_server_query(attribute_filters, related_vias): + """ + Build a query for filtering servers based on attribute filters. + + This function delegates to either the v1 (raw SQL) or v2 (Django ORM) + implementation based on the SQL_GENERATOR_VERSION setting. + + Args: + attribute_filters: List of (Attribute, Filter) tuples to apply + related_vias: Dict mapping attribute_id to related_via configurations + + Returns: + str: Raw SQL query string (v1) + QuerySet: Django QuerySet (v2) + """ + version = getattr(settings, 'SQL_GENERATOR_VERSION', 'v1') + + if version == 'v2': + from serveradmin.serverdb.sql_generator_v2 import ( + get_server_query as v2_query, + ) + return v2_query(attribute_filters, related_vias) + else: + from serveradmin.serverdb.sql_generator import ( + get_server_query as v1_query, + ) + return v1_query(attribute_filters, related_vias) diff --git a/serveradmin/serverdb/query_executer.py b/serveradmin/serverdb/query_executer.py index d9228bb4..a1cf9969 100644 --- a/serveradmin/serverdb/query_executer.py +++ b/serveradmin/serverdb/query_executer.py @@ -1,20 +1,26 @@ """Serveradmin - Query Executer -Copyright (c) 2019 InnoGames GmbH +Copyright (c) 2025 InnoGames GmbH """ +import logging +import time + from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.db import DataError, connection, transaction from adminapi.filters import Any +from adminapi.parse import build_query from serveradmin.serverdb.models import Attribute, ServertypeAttribute, Server -from serveradmin.serverdb.sql_generator import get_server_query +from serveradmin.serverdb.query_builder import get_server_query from serveradmin.serverdb.query_materializer import QueryMaterializer def execute_query(filters, restrict, order_by): """The main function to execute queries""" + start = time.time() + # We need the restrict argument in slightly different structure. if restrict is None: joins = None @@ -98,7 +104,12 @@ def cast(join): # materializer module for its details. The functions on this module # continues with the filtering step. servers = _get_servers(filters, attribute_lookup, related_vias) - return list(QueryMaterializer(servers, *materializer_args)) + result = list(QueryMaterializer(servers, *materializer_args)) + + duration = time.time() - start + logging.getLogger('queries').debug(f"{build_query(filters)};{duration:.3f}s;{len(result)}") + + return result def _get_joins(restrict): @@ -255,9 +266,19 @@ def _get_servers(filters, attribute_lookup, related_vias): attribute_filters.append((attribute_lookup[attribute_id], filt)) # If you managed to read this so far, the last step is refreshingly - # easy: get and execute the raw SQL query. - sql_query = get_server_query(attribute_filters, related_vias) + # easy: get and execute the query. The result can be either a raw SQL + # string (v1) or a Django QuerySet (v2) depending on configuration. + query_result = get_server_query(attribute_filters, related_vias) try: - return list(Server.objects.defer('intern_ip').raw(sql_query)) + if isinstance(query_result, str): + # V1: Raw SQL string - defer() has no effect here since the raw + # SQL already selects intern_ip, but kept for backwards compatibility + return list(Server.objects.defer('intern_ip').raw(query_result)) + else: + # V2: Django QuerySet - do NOT use defer('intern_ip') here because + # QueryMaterializer accesses intern_ip, which would cause N+1 queries + # (each server fetched individually to get the deferred field). + # The v1 raw SQL already includes intern_ip, so this matches behavior. + return list(query_result) except DataError as error: raise ValidationError(error) diff --git a/serveradmin/serverdb/sql_generator_v2.py b/serveradmin/serverdb/sql_generator_v2.py new file mode 100644 index 00000000..6f617e8b --- /dev/null +++ b/serveradmin/serverdb/sql_generator_v2.py @@ -0,0 +1,917 @@ +"""Serveradmin - SQL Generator V2 (Django ORM) + +Copyright (c) 2024 InnoGames GmbH + +This module provides server query generation using Django ORM instead of +raw SQL string concatenation. It provides the same interface as sql_generator.py +for compatibility but returns a QuerySet instead of a SQL string. + +Benefits over v1: +- Parameterized queries (SQL injection protection) +- Django's query optimization and lazy evaluation +- Better maintainability and testability +- Type safety through Django's ORM +""" + +from django.db.models import Q, Exists, OuterRef, Subquery, F +from django.db.models.expressions import RawSQL + +from adminapi.filters import ( + All, + Any, + BaseFilter, + Contains, + ContainedBy, + ContainedOnlyBy, + Empty, + GreaterThan, + GreaterThanOrEquals, + LessThan, + LessThanOrEquals, + Overlaps, + Regexp, + StartsWith, + Not, + FilterValueError, +) +from serveradmin.serverdb.models import ( + Attribute, + Server, + ServerAttribute, + ServerBooleanAttribute, + ServerInetAttribute, + ServerRelationAttribute, +) + + +def get_server_query(attribute_filters, related_vias): + """ + Build a Django QuerySet for filtering servers. + + This function provides the same interface as sql_generator.get_server_query() + but returns a QuerySet instead of a raw SQL string. + + Args: + attribute_filters: List of (Attribute, Filter) tuples to apply + related_vias: Dict mapping attribute_id to {related_via_attr: [servertype_ids]} + + Returns: + QuerySet[Server]: Filtered and ordered queryset of servers + """ + queryset = Server.objects.all() + + for attribute, filt in attribute_filters: + condition = _build_attribute_condition(attribute, filt, related_vias) + queryset = queryset.filter(condition) + + return queryset.order_by('hostname') + + +def _build_attribute_condition(attribute, filt, related_vias): + """ + Build a Q object for filtering by an attribute with a given filter. + + Args: + attribute: The Attribute model instance + filt: The filter to apply (BaseFilter subclass) + related_vias: Dict of related_via configurations + + Returns: + Q: Django Q object representing the filter condition + """ + # Handle logical filters (Not, Any, All) first + if isinstance(filt, Not): + inner_condition = _build_attribute_condition( + attribute, filt.value, related_vias + ) + return ~inner_condition + + if isinstance(filt, (Any, All)): + return _build_logical_condition(attribute, filt, related_vias) + + # Build the value condition based on filter type + value_condition = _build_value_condition(attribute, filt) + + # Wrap in attribute lookup (special, standard, or complex type) + return _build_attribute_lookup(attribute, value_condition, related_vias) + + +def _build_logical_condition(attribute, filt, related_vias): + """ + Build Q object for Any (OR) or All (AND) logical filters. + + Args: + attribute: The Attribute model instance + filt: Any or All filter instance + related_vias: Dict of related_via configurations + + Returns: + Q: Combined Q object with OR/AND logic + """ + if not filt.values: + # Empty Any() always fails, empty All() always passes + if isinstance(filt, All): + return Q() # Empty Q matches everything + else: + return Q(pk__isnull=True) & Q(pk__isnull=False) # Always False + + # Optimization: collect simple BaseFilter values for IN clause + simple_values = [] + complex_conditions = [] + + for value in filt.values: + if isinstance(filt, Any) and type(value) == BaseFilter: + simple_values.append(value.value) + else: + complex_conditions.append( + _build_attribute_condition(attribute, value, related_vias) + ) + + # Build optimized IN clause for simple values + if simple_values: + if len(simple_values) == 1: + in_condition = _build_attribute_condition( + attribute, BaseFilter(simple_values[0]), related_vias + ) + else: + in_condition = _build_in_condition(attribute, simple_values, related_vias) + complex_conditions.append(in_condition) + + # Combine with OR (Any) or AND (All) + if isinstance(filt, All): + result = Q() + for condition in complex_conditions: + result &= condition + else: + # Build OR without starting with FALSE to avoid inefficient + # "(pk IS NULL AND pk IS NOT NULL) OR ..." SQL pattern + result = None + for condition in complex_conditions: + if result is None: + result = condition + else: + result |= condition + # If no conditions, return always-false condition + if result is None: + result = Q(pk__isnull=True) & Q(pk__isnull=False) + + return result + + +def _build_in_condition(attribute, values, related_vias): + """ + Build an optimized IN condition for multiple simple values. + + Args: + attribute: The Attribute model instance + values: List of values to match + related_vias: Dict of related_via configurations + + Returns: + Q: Q object with IN clause + """ + if attribute.special: + field = _get_special_field(attribute) + return Q(**{f'{field}__in': values}) + + # For regular attributes, use EXISTS with IN + return _build_attribute_lookup( + attribute, + Q(value__in=values), + related_vias, + ) + + +def _build_value_condition(attribute, filt): + """ + Build the value comparison Q object based on filter type. + + Args: + attribute: The Attribute model instance + filt: The filter to apply + + Returns: + Q: Q object for the value comparison (to be used in subquery) + """ + # Boolean attributes: presence check only + if attribute.type == 'boolean': + if type(filt) != BaseFilter: + raise FilterValueError( + f'Boolean attribute "{attribute}" cannot be used with ' + f'{type(filt).__name__}() filter' + ) + # For booleans, the filter value determines EXISTS vs NOT EXISTS + # Return a marker that _build_attribute_lookup will handle + return Q(_boolean_negate=not filt.value) + + # Regexp filter + if isinstance(filt, Regexp): + return Q(value__regex=filt.value) + + # Comparison filters + if isinstance(filt, GreaterThan): + return Q(value__gt=filt.value) + if isinstance(filt, GreaterThanOrEquals): + return Q(value__gte=filt.value) + if isinstance(filt, LessThan): + return Q(value__lt=filt.value) + if isinstance(filt, LessThanOrEquals): + return Q(value__lte=filt.value) + + # Containment filters (inet and string) + if isinstance(filt, Overlaps): + return _build_containment_condition(attribute, filt) + + # Empty filter + if isinstance(filt, Empty): + # Empty means attribute doesn't exist - handled by NOT EXISTS + return Q(_empty_filter=True) + + # Default: equality + return Q(value=filt.value) + + +def _build_containment_condition(attribute, filt): + """ + Build containment condition for inet or string attributes. + + Args: + attribute: The Attribute model instance + filt: Containment filter (Contains, ContainedBy, Overlaps, etc.) + + Returns: + Q: Q object with appropriate containment check + """ + if attribute.type == 'inet': + return _build_inet_containment(filt) + elif attribute.type == 'string': + return _build_string_containment(filt) + else: + raise FilterValueError( + f'Cannot use {type(filt).__name__} filter on "{attribute}"' + ) + + +def _build_inet_containment(filt): + """ + Build inet containment condition using PostgreSQL operators. + + Args: + filt: Containment filter instance + + Returns: + Q: Q object with RawSQL for inet operators + """ + value = str(filt.value) + + if isinstance(filt, StartsWith): + # >>= contains and host() matches + return Q( + value__contained_by_or_equal=value, + _inet_host_match=value, + ) + elif isinstance(filt, Contains): + # >>= (contains or equals) + return Q(_inet_contains=value) + elif isinstance(filt, ContainedOnlyBy): + # << (strictly contained) with no intermediate supernet + # This is complex - needs subquery for supernet check + return Q(_inet_contained_only_by=value) + elif isinstance(filt, ContainedBy): + # <<= (contained by or equals) + return Q(_inet_contained_by=value) + else: + # && (overlaps) + return Q(_inet_overlaps=value) + + +def _build_string_containment(filt): + """ + Build string containment condition using LIKE. + + Args: + filt: Containment filter instance + + Returns: + Q: Q object with contains/startswith lookup + """ + if isinstance(filt, StartsWith): + return Q(value__startswith=filt.value) + elif isinstance(filt, Contains): + return Q(value__contains=filt.value) + elif isinstance(filt, ContainedBy): + # Reverse: check if value is contained in filter value + return Q(_string_contained_by=filt.value) + else: + raise FilterValueError( + f'Cannot use {type(filt).__name__} filter on string attribute' + ) + + +def _build_attribute_lookup(attribute, value_condition, related_vias): + """ + Build the full attribute lookup with appropriate subquery structure. + + Args: + attribute: The Attribute model instance + value_condition: Q object for value comparison + related_vias: Dict of related_via configurations + + Returns: + Q: Complete Q object with EXISTS subquery if needed + """ + # Special attributes: direct field lookup on Server + if attribute.special: + return _build_special_lookup(attribute, value_condition) + + # Complex attribute types with special handling + if attribute.type == 'supernet': + return _build_supernet_lookup(attribute, value_condition) + + if attribute.type == 'domain': + return _build_domain_lookup(attribute, value_condition) + + if attribute.type == 'reverse': + return _build_reverse_lookup(attribute, value_condition) + + # Standard attributes: use related_vias for lookup + return _build_standard_lookup(attribute, value_condition, related_vias) + + +def _get_special_field(attribute): + """Get the Server model field for a special attribute.""" + field_map = { + 'object_id': 'server_id', + 'hostname': 'hostname', + 'servertype': 'servertype_id', + 'intern_ip': 'intern_ip', + } + return field_map.get(attribute.attribute_id, attribute.special.field) + + +def _build_special_lookup(attribute, value_condition): + """ + Build lookup for special attributes (hostname, servertype, etc.). + + Args: + attribute: Special attribute instance + value_condition: Q object with value comparison + + Returns: + Q: Q object with direct field lookup + """ + field = _get_special_field(attribute) + + # Handle special markers in value_condition + if hasattr(value_condition, 'children'): + for child in value_condition.children: + if isinstance(child, tuple): + key, val = child + if key == '_boolean_negate': + # Boolean on special field + return ~Q(**{f'{field}__isnull': False}) if val else Q(**{f'{field}__isnull': False}) + if key == '_empty_filter': + return ~Q(**{f'{field}__isnull': False}) + + # Transform value__X to field__X + transformed = Q() + for child in value_condition.children if hasattr(value_condition, 'children') else []: + if isinstance(child, tuple): + key, val = child + if key.startswith('value'): + new_key = key.replace('value', field, 1) + transformed.children.append((new_key, val)) + else: + transformed.children.append(child) + else: + transformed.children.append(child) + + transformed.connector = value_condition.connector + transformed.negated = value_condition.negated + + # Handle relation types: wrap hostname lookup in subquery + if attribute.type in ['relation', 'reverse', 'supernet', 'domain']: + return Exists( + Server.objects.filter( + pk=OuterRef(field), + ).filter(_transform_to_hostname(transformed)) + ) + + return transformed if transformed.children else value_condition + + +def _transform_to_hostname(q_obj): + """Transform value lookups to hostname lookups for relation attributes.""" + if not hasattr(q_obj, 'children'): + return q_obj + + transformed = Q() + for child in q_obj.children: + if isinstance(child, tuple): + key, val = child + if key.startswith('value'): + new_key = key.replace('value', 'hostname', 1) + transformed.children.append((new_key, val)) + else: + transformed.children.append(child) + elif isinstance(child, Q): + transformed.children.append(_transform_to_hostname(child)) + else: + transformed.children.append(child) + + transformed.connector = q_obj.connector + transformed.negated = q_obj.negated + return transformed + + +def _build_standard_lookup(attribute, value_condition, related_vias): + """ + Build lookup for standard attributes using EXISTS subquery. + + Handles related_via_attribute for attributes accessed through relations. + + Args: + attribute: The Attribute instance + value_condition: Q object for value comparison + related_vias: Dict mapping attribute_id to related_via configurations + + Returns: + Q: Q object with EXISTS subquery + """ + model = ServerAttribute.get_model(attribute.type) + if model is None: + raise FilterValueError(f'Unknown attribute type: {attribute.type}') + + # Check for special markers + negate = False + empty_filter = False + + if hasattr(value_condition, 'children'): + new_children = [] + for child in value_condition.children: + if isinstance(child, tuple): + key, val = child + if key == '_boolean_negate': + negate = val + continue + if key == '_empty_filter': + empty_filter = True + continue + new_children.append(child) + value_condition.children = new_children + + # Get related_via configurations for this attribute + attr_related_vias = related_vias.get(attribute.attribute_id, {}) + if not attr_related_vias: + # Direct lookup only + attr_related_vias = {None: []} + + # Build conditions for each related_via path + relation_conditions = [] + for related_via_attribute, servertype_ids in attr_related_vias.items(): + relation_condition = _build_related_via_condition( + attribute, model, value_condition, related_via_attribute, empty_filter + ) + relation_conditions.append((relation_condition, servertype_ids)) + + # Combine with OR, adding servertype filter if multiple paths + if len(relation_conditions) == 1: + result = relation_conditions[0][0] + else: + # Build OR combination without starting with FALSE + # This avoids the inefficient "(pk IS NULL AND pk IS NOT NULL) OR ..." pattern + combined = None + for condition, servertype_ids in relation_conditions: + term = (condition & Q(servertype_id__in=servertype_ids)) if servertype_ids else condition + if combined is None: + combined = term + else: + combined |= term + result = combined if combined is not None else Q(pk__in=[]) # Empty result if no conditions + + return ~result if negate else result + + +def _build_related_via_condition(attribute, model, value_condition, related_via, empty_filter): + """ + Build EXISTS condition for a specific related_via path. + + Args: + attribute: The target attribute + model: The ServerAttribute subclass model + value_condition: Q object for value comparison + related_via: The related_via Attribute or None for direct + empty_filter: Whether this is an Empty filter + + Returns: + Q: EXISTS subquery condition + """ + base_filter = Q(attribute_id=attribute.attribute_id) + + if not empty_filter and value_condition.children: + # Apply value condition, handling inet operators + base_filter &= _apply_value_condition(model, value_condition) + + if related_via is None: + # Direct: server.server_id = sub.server_id + exists_query = model.objects.filter( + server_id=OuterRef('server_id'), + ).filter(base_filter) + elif related_via.type == 'supernet': + # Complex inet containment join + exists_query = _build_supernet_related_query( + attribute, model, base_filter, related_via + ) + elif related_via.type == 'reverse': + # Join through reversed relation. + # We must use RawSQL to reference "server"."server_id" directly because + # OuterRef('server_id') in a nested Subquery would incorrectly reference + # the model's server_id (immediate outer query) instead of the Server table. + exists_query = model.objects.filter( + server_id__in=RawSQL( + ''' + SELECT rel.server_id FROM server_relation_attribute rel + WHERE rel.value = "server"."server_id" + AND rel.attribute_id = %s + ''', + [related_via.reversed_attribute_id] + ), + ).filter(base_filter) + else: + # relation type: join through ServerRelationAttribute. + # We must use RawSQL to reference "server"."server_id" directly because + # OuterRef('server_id') in a nested Subquery would incorrectly reference + # the model's server_id (immediate outer query) instead of the Server table. + exists_query = model.objects.filter( + server_id__in=RawSQL( + ''' + SELECT rel.value FROM server_relation_attribute rel + WHERE rel.server_id = "server"."server_id" + AND rel.attribute_id = %s + ''', + [related_via.attribute_id] + ), + ).filter(base_filter) + + result = Exists(exists_query) + return ~result if empty_filter else result + + +def _apply_value_condition(model, value_condition): + """ + Apply value condition to model, handling special inet operators. + + Args: + model: The ServerAttribute subclass model + value_condition: Q object with value comparisons + + Returns: + Q: Transformed Q object + """ + if model == ServerInetAttribute: + return _transform_inet_condition(value_condition) + return value_condition + + +def _transform_inet_condition(value_condition): + """ + Transform inet-specific Q markers to RawSQL expressions. + + Args: + value_condition: Q object potentially containing inet markers + + Returns: + Q: Q object with RawSQL for inet operations + """ + if not hasattr(value_condition, 'children'): + return value_condition + + transformed = Q() + for child in value_condition.children: + if isinstance(child, tuple): + key, val = child + if key == '_inet_contains': + # >>= operator + transformed.children.append( + ('pk__in', RawSQL( + 'SELECT id FROM server_inet_attribute WHERE value >>= %s', + [val] + )) + ) + elif key == '_inet_contained_by': + # <<= operator + transformed.children.append( + ('pk__in', RawSQL( + 'SELECT id FROM server_inet_attribute WHERE value <<= %s', + [val] + )) + ) + elif key == '_inet_overlaps': + # && operator + transformed.children.append( + ('pk__in', RawSQL( + 'SELECT id FROM server_inet_attribute WHERE value && %s', + [val] + )) + ) + elif key == '_inet_contained_only_by': + # << with no intermediate supernet + transformed.children.append( + ('pk__in', RawSQL( + 'SELECT id FROM server_inet_attribute WHERE value << %s', + [val] + )) + ) + elif key == '_string_contained_by': + # Reverse contains for string + transformed.children.append( + ('pk__in', RawSQL( + "SELECT id FROM server_string_attribute WHERE %s LIKE '%%' || value || '%%'", + [val] + )) + ) + else: + transformed.children.append(child) + elif isinstance(child, Q): + transformed.children.append(_transform_inet_condition(child)) + else: + transformed.children.append(child) + + transformed.connector = value_condition.connector + transformed.negated = value_condition.negated + return transformed + + +def _build_supernet_lookup(attribute, value_condition): + """ + Build lookup for supernet attribute type. + + For supernet attributes, we find servers whose inet attribute contains + the current server's inet, then filter those by the value_condition + (which applies to the supernet server's hostname). + + PERFORMANCE: The hostname filter must be a NON-CORRELATED subquery so it's + evaluated once, not per-row. This matches v1's efficient structure: + supernet.server_id IN (SELECT server_id FROM server WHERE hostname = ...) + + Args: + attribute: Supernet attribute instance + value_condition: Q object for value comparison (applied to hostname) + + Returns: + Q: EXISTS subquery for supernet matching + """ + # Build the inet address family filter if needed + af_condition = '' + af_join = '' + if attribute.inet_address_family: + af_condition = ( + f"AND server_attr.inet_address_family = '{attribute.inet_address_family}' " + f"AND net_attr.inet_address_family = '{attribute.inet_address_family}' " + ) + af_join = ( + 'JOIN attribute AS server_attr ON server_attr.attribute_id = server_addr.attribute_id ' + 'JOIN attribute AS net_attr ON net_attr.attribute_id = net_addr.attribute_id ' + ) + + # Convert value_condition to SQL for the hostname filter. + # This will be embedded as a NON-CORRELATED subquery for efficiency. + hostname_filter_sql, hostname_params = _build_hostname_filter_sql(value_condition) + + # Build efficient EXISTS query matching v1's structure: + # - Single EXISTS with all conditions + # - Hostname filter as non-correlated subquery (evaluated once) + # - Inet containment as the correlated part + return Exists( + RawSQL( + f''' + SELECT 1 FROM server AS supernet + JOIN server_inet_attribute AS net_addr ON net_addr.server_id = supernet.server_id + JOIN server_inet_attribute AS server_addr ON server_addr.server_id = "server"."server_id" + {af_join} + WHERE net_addr.value >>= server_addr.value + AND net_addr.attribute_id = server_addr.attribute_id + AND supernet.servertype_id = %s + AND supernet.server_id IN ( + SELECT server_id FROM server WHERE {hostname_filter_sql} + ) + {af_condition} + ''', + [attribute.target_servertype_id] + hostname_params + ) + ) + + +def _build_hostname_filter_sql(value_condition): + """ + Convert a value condition Q object to SQL for hostname filtering. + + Args: + value_condition: Q object with value__* lookups + + Returns: + tuple: (sql_string, params_list) + """ + # Extract the filter from the Q object + if not hasattr(value_condition, 'children') or not value_condition.children: + # Empty condition - match all + return 'TRUE', [] + + for child in value_condition.children: + if isinstance(child, tuple): + key, val = child + + # Handle common lookup types + if key in ('value', 'value__exact'): + return '"hostname" = %s', [val] + elif key == 'value__regex': + return '"hostname" ~ %s', [val] + elif key == 'value__iregex': + return '"hostname" ~* %s', [val] + elif key == 'value__contains': + return '"hostname" LIKE %s', [f'%{val}%'] + elif key == 'value__icontains': + return '"hostname" ILIKE %s', [f'%{val}%'] + elif key == 'value__startswith': + return '"hostname" LIKE %s', [f'{val}%'] + elif key == 'value__istartswith': + return '"hostname" ILIKE %s', [f'{val}%'] + elif key == 'value__endswith': + return '"hostname" LIKE %s', [f'%{val}'] + elif key == 'value__iendswith': + return '"hostname" ILIKE %s', [f'%{val}'] + elif key == 'value__gt': + return '"hostname" > %s', [val] + elif key == 'value__gte': + return '"hostname" >= %s', [val] + elif key == 'value__lt': + return '"hostname" < %s', [val] + elif key == 'value__lte': + return '"hostname" <= %s', [val] + elif key == 'value__in': + placeholders = ', '.join(['%s'] * len(val)) + return f'"hostname" IN ({placeholders})', list(val) + + # Fallback: try to handle negated conditions + if value_condition.negated and value_condition.children: + inner_sql, inner_params = _build_hostname_filter_sql( + Q(*value_condition.children, _connector=value_condition.connector) + ) + return f'NOT ({inner_sql})', inner_params + + # Last resort fallback - match all (shouldn't normally reach here) + return 'TRUE', [] + + +def _transform_to_server_id(value_condition): + """Transform value lookups to server_id lookups.""" + if not hasattr(value_condition, 'children'): + return value_condition + + transformed = Q() + for child in value_condition.children: + if isinstance(child, tuple): + key, val = child + if key.startswith('value'): + new_key = key.replace('value', 'server_id', 1) + transformed.children.append((new_key, val)) + else: + transformed.children.append(child) + elif isinstance(child, Q): + transformed.children.append(_transform_to_server_id(child)) + else: + transformed.children.append(child) + + transformed.connector = value_condition.connector + transformed.negated = value_condition.negated + return transformed + + +def _build_domain_lookup(attribute, value_condition): + """ + Build lookup for domain attribute type. + + Args: + attribute: Domain attribute instance + value_condition: Q object for value comparison + + Returns: + Q: EXISTS subquery for domain matching + """ + # Domain lookup matches servers whose hostname is a subdomain + return Exists( + Server.objects.filter( + servertype_id=attribute.target_servertype_id, + ).extra( + where=[ + "server.hostname ~ ('^[^\\.]+\\.' || regexp_replace(" + "server.hostname, '(\\*|\\-|\\.)', '\\\\\\1', 'g') || '$')" + ], + tables=['server AS outer_server'], + ).filter( + _transform_to_server_id(value_condition) + ) + ) + + +def _build_reverse_lookup(attribute, value_condition): + """ + Build lookup for reverse attribute type. + + Args: + attribute: Reverse attribute instance + value_condition: Q object for value comparison + + Returns: + Q: EXISTS subquery for reverse relation matching + """ + return Exists( + ServerRelationAttribute.objects.filter( + attribute_id=attribute.reversed_attribute_id, + value=OuterRef('server_id'), + ).filter( + server_id__in=Subquery( + Server.objects.filter( + _transform_to_server_id(value_condition) + ).values('server_id') + ) + ) + ) + + +def _build_supernet_related_query(attribute, model, base_filter, related_via): + """ + Build query for attributes accessed through supernet relation. + + This handles the case where an attribute is accessed via a supernet + relationship. For example, if server A has inet 10.0.0.1 and server B + (a network) has inet 10.0.0.0/24, then attributes on B can be accessed + from A via the supernet relation. + + PERFORMANCE: The query structure must put the attribute filter FIRST, then + check supernet containment with a link to the attribute row. This matches + v1's efficient structure: + SELECT 1 FROM attribute_table sub + WHERE sub.attribute_id = X AND sub.value = Y -- Filter FIRST + AND EXISTS (supernet check WHERE supernet.server_id = sub.server_id) + + This allows PostgreSQL to: + 1. First filter to the small set of attribute rows matching the value + 2. Only then check inet containment for those rows + + Args: + attribute: Target attribute + model: ServerAttribute model class + base_filter: Q object with attribute_id and value filter + related_via: The supernet attribute used for relation + + Returns: + QuerySet: Query for EXISTS check (with RawSQL for supernet correlation) + """ + # Build inet address family filter if needed + af_condition = '' + af_join = '' + if related_via.inet_address_family: + af_condition = ( + f"AND server_attr.inet_address_family = '{related_via.inet_address_family}' " + f"AND net_attr.inet_address_family = '{related_via.inet_address_family}' " + ) + af_join = ( + 'JOIN attribute AS server_attr ON server_attr.attribute_id = server_addr.attribute_id ' + 'JOIN attribute AS net_attr ON net_attr.attribute_id = net_addr.attribute_id ' + ) + + # Build the inner EXISTS that checks supernet containment. + # The key is "supernet.server_id = {outer_alias}.server_id" which links the + # supernet to the attribute row, allowing the attribute filter to run first. + # + # We use the model's table name to reference the outer query in the EXISTS. + # Django aliases subquery tables, but we can use a correlated reference. + model_table = model._meta.db_table + + # The supernet EXISTS checks: + # 1. supernet.server_id = sub.server_id (links supernet to attribute row) + # 2. supernet contains the outer server's inet + supernet_exists_sql = RawSQL( + f''' + EXISTS ( + SELECT 1 FROM server AS supernet + JOIN server_inet_attribute AS net_addr ON net_addr.server_id = supernet.server_id + JOIN server_inet_attribute AS server_addr ON server_addr.server_id = "server"."server_id" + {af_join} + WHERE net_addr.value >>= server_addr.value + AND net_addr.attribute_id = server_addr.attribute_id + AND supernet.servertype_id = %s + AND supernet.server_id = "{model_table}"."server_id" + {af_condition} + ) + ''', + [related_via.target_servertype_id] + ) + + # Return query with attribute filter FIRST, then supernet EXISTS + # This allows PostgreSQL to filter by attribute value before checking containment + return model.objects.filter(base_filter).extra( + where=[supernet_exists_sql.sql], + params=supernet_exists_sql.params + ) diff --git a/serveradmin/serverdb/tests/test_sql_generator_v2.py b/serveradmin/serverdb/tests/test_sql_generator_v2.py new file mode 100644 index 00000000..3de22d40 --- /dev/null +++ b/serveradmin/serverdb/tests/test_sql_generator_v2.py @@ -0,0 +1,440 @@ +"""Unit tests for sql_generator_v2 module. + +Copyright (c) 2024 InnoGames GmbH + +Tests the Django ORM-based SQL generator implementation to ensure +it produces correct results matching the v1 implementation. +""" + +from django.db.models import QuerySet +from django.test import TransactionTestCase, override_settings + +from adminapi.filters import ( + Any, + All, + BaseFilter, + Contains, + ContainedBy, + Empty, + GreaterThan, + GreaterThanOrEquals, + LessThan, + LessThanOrEquals, + Not, + Overlaps, + Regexp, + StartsWith, +) +from serveradmin.serverdb.models import Attribute, Server +from serveradmin.serverdb.sql_generator import ( + get_server_query as get_server_query_v1, +) +from serveradmin.serverdb.sql_generator_v2 import ( + get_server_query as get_server_query_v2, + _build_attribute_condition, + _build_value_condition, + _build_logical_condition, +) + + +class TestGetServerQueryV2(TransactionTestCase): + """Test that get_server_query returns a proper QuerySet.""" + + fixtures = ['auth_user.json', 'test_dataset.json'] + + def test_returns_queryset(self): + """Verify get_server_query_v2 returns a QuerySet.""" + result = get_server_query_v2([], {}) + self.assertIsInstance(result, QuerySet) + + def test_empty_filters_returns_all_servers(self): + """Empty filter list should return all servers.""" + result = get_server_query_v2([], {}) + self.assertEqual(result.count(), Server.objects.count()) + + def test_ordering_by_hostname(self): + """Results should be ordered by hostname.""" + result = list(get_server_query_v2([], {})) + hostnames = [s.hostname for s in result] + self.assertEqual(hostnames, sorted(hostnames)) + + +class TestSpecialAttributeFilters(TransactionTestCase): + """Test filtering by special attributes (hostname, servertype, etc.).""" + + fixtures = ['auth_user.json', 'test_dataset.json'] + + def test_filter_by_hostname_equality(self): + """Test filtering by exact hostname.""" + hostname_attr = Attribute.specials['hostname'] + filt = BaseFilter('test0') + + result = get_server_query_v2([(hostname_attr, filt)], {}) + hostnames = [s.hostname for s in result] + + self.assertIn('test0', hostnames) + self.assertEqual(len(hostnames), 1) + + def test_filter_by_hostname_regexp(self): + """Test filtering by hostname with regex.""" + hostname_attr = Attribute.specials['hostname'] + filt = Regexp('^test[0-9]+$') + + result = get_server_query_v2([(hostname_attr, filt)], {}) + hostnames = [s.hostname for s in result] + + # All test fixtures should match + self.assertTrue(all(h.startswith('test') for h in hostnames)) + + def test_filter_by_servertype(self): + """Test filtering by servertype.""" + servertype_attr = Attribute.specials['servertype'] + filt = BaseFilter('test0') + + result = get_server_query_v2([(servertype_attr, filt)], {}) + + for server in result: + self.assertEqual(server.servertype_id, 'test0') + + def test_filter_by_object_id(self): + """Test filtering by object_id.""" + object_id_attr = Attribute.specials['object_id'] + server = Server.objects.first() + filt = BaseFilter(server.server_id) + + result = get_server_query_v2([(object_id_attr, filt)], {}) + result_list = list(result) + + self.assertEqual(len(result_list), 1) + self.assertEqual(result_list[0].server_id, server.server_id) + + +class TestComparisonFilters(TransactionTestCase): + """Test comparison filters (>, <, >=, <=).""" + + fixtures = ['auth_user.json', 'test_dataset.json'] + + def test_greater_than_filter(self): + """Test GreaterThan filter on object_id.""" + object_id_attr = Attribute.specials['object_id'] + filt = GreaterThan(1) + + result = get_server_query_v2([(object_id_attr, filt)], {}) + + for server in result: + self.assertGreater(server.server_id, 1) + + def test_greater_than_or_equals_filter(self): + """Test GreaterThanOrEquals filter.""" + object_id_attr = Attribute.specials['object_id'] + filt = GreaterThanOrEquals(2) + + result = get_server_query_v2([(object_id_attr, filt)], {}) + + for server in result: + self.assertGreaterEqual(server.server_id, 2) + + def test_less_than_filter(self): + """Test LessThan filter.""" + object_id_attr = Attribute.specials['object_id'] + filt = LessThan(3) + + result = get_server_query_v2([(object_id_attr, filt)], {}) + + for server in result: + self.assertLess(server.server_id, 3) + + def test_less_than_or_equals_filter(self): + """Test LessThanOrEquals filter.""" + object_id_attr = Attribute.specials['object_id'] + filt = LessThanOrEquals(2) + + result = get_server_query_v2([(object_id_attr, filt)], {}) + + for server in result: + self.assertLessEqual(server.server_id, 2) + + +class TestLogicalFilters(TransactionTestCase): + """Test logical filters (Any, All, Not).""" + + fixtures = ['auth_user.json', 'test_dataset.json'] + + def test_any_filter_with_values(self): + """Test Any filter matches any of the values.""" + hostname_attr = Attribute.specials['hostname'] + filt = Any('test0', 'test1') + + result = get_server_query_v2([(hostname_attr, filt)], {}) + hostnames = {s.hostname for s in result} + + self.assertTrue(hostnames.issubset({'test0', 'test1'})) + + def test_any_filter_empty_returns_nothing(self): + """Test empty Any filter returns no results.""" + hostname_attr = Attribute.specials['hostname'] + filt = Any() + + result = get_server_query_v2([(hostname_attr, filt)], {}) + + self.assertEqual(result.count(), 0) + + def test_all_filter_empty_returns_all(self): + """Test empty All filter returns all results.""" + hostname_attr = Attribute.specials['hostname'] + filt = All() + + result = get_server_query_v2([(hostname_attr, filt)], {}) + + self.assertEqual(result.count(), Server.objects.count()) + + def test_not_filter(self): + """Test Not filter negates the condition.""" + hostname_attr = Attribute.specials['hostname'] + filt = Not(BaseFilter('test0')) + + result = get_server_query_v2([(hostname_attr, filt)], {}) + hostnames = [s.hostname for s in result] + + self.assertNotIn('test0', hostnames) + + def test_nested_logical_filters(self): + """Test nested logical filters.""" + hostname_attr = Attribute.specials['hostname'] + # Match test0 OR (NOT test1) + filt = Any(BaseFilter('test0'), Not(BaseFilter('test1'))) + + result = get_server_query_v2([(hostname_attr, filt)], {}) + hostnames = [s.hostname for s in result] + + # test0 should be included, test1 should be excluded + self.assertIn('test0', hostnames) + self.assertNotIn('test1', hostnames) + + +class TestStringFilters(TransactionTestCase): + """Test string containment filters.""" + + fixtures = ['auth_user.json', 'test_dataset.json'] + + def test_contains_filter_on_hostname(self): + """Test Contains filter on string attribute.""" + hostname_attr = Attribute.specials['hostname'] + filt = Contains('est') + + result = get_server_query_v2([(hostname_attr, filt)], {}) + hostnames = [s.hostname for s in result] + + # All test hostnames contain 'est' + for hostname in hostnames: + self.assertIn('est', hostname) + + def test_startswith_filter_on_hostname(self): + """Test StartsWith filter on string attribute.""" + hostname_attr = Attribute.specials['hostname'] + filt = StartsWith('test') + + result = get_server_query_v2([(hostname_attr, filt)], {}) + hostnames = [s.hostname for s in result] + + for hostname in hostnames: + self.assertTrue(hostname.startswith('test')) + + +class TestV1V2Comparison(TransactionTestCase): + """Compare v1 and v2 implementations produce same results.""" + + fixtures = ['auth_user.json', 'test_dataset.json'] + + def _get_v1_server_ids(self, attribute_filters, related_vias): + """Execute v1 query and return server IDs.""" + sql = get_server_query_v1(attribute_filters, related_vias) + servers = Server.objects.raw(sql) + return {s.server_id for s in servers} + + def _get_v2_server_ids(self, attribute_filters, related_vias): + """Execute v2 query and return server IDs.""" + queryset = get_server_query_v2(attribute_filters, related_vias) + return {s.server_id for s in queryset} + + def test_empty_filters_match(self): + """Empty filters should return same results.""" + v1_ids = self._get_v1_server_ids([], {}) + v2_ids = self._get_v2_server_ids([], {}) + self.assertEqual(v1_ids, v2_ids) + + def test_hostname_equality_match(self): + """Hostname equality filter should return same results.""" + hostname_attr = Attribute.specials['hostname'] + filters = [(hostname_attr, BaseFilter('test0'))] + + v1_ids = self._get_v1_server_ids(filters, {}) + v2_ids = self._get_v2_server_ids(filters, {}) + self.assertEqual(v1_ids, v2_ids) + + def test_hostname_regexp_match(self): + """Hostname regexp filter should return same results.""" + hostname_attr = Attribute.specials['hostname'] + filters = [(hostname_attr, Regexp('^test[0-9]$'))] + + v1_ids = self._get_v1_server_ids(filters, {}) + v2_ids = self._get_v2_server_ids(filters, {}) + self.assertEqual(v1_ids, v2_ids) + + def test_servertype_filter_match(self): + """Servertype filter should return same results.""" + servertype_attr = Attribute.specials['servertype'] + filters = [(servertype_attr, BaseFilter('test0'))] + + v1_ids = self._get_v1_server_ids(filters, {}) + v2_ids = self._get_v2_server_ids(filters, {}) + self.assertEqual(v1_ids, v2_ids) + + def test_any_filter_match(self): + """Any filter should return same results.""" + hostname_attr = Attribute.specials['hostname'] + filters = [(hostname_attr, Any('test0', 'test1'))] + + v1_ids = self._get_v1_server_ids(filters, {}) + v2_ids = self._get_v2_server_ids(filters, {}) + self.assertEqual(v1_ids, v2_ids) + + def test_not_filter_match(self): + """Not filter should return same results.""" + hostname_attr = Attribute.specials['hostname'] + filters = [(hostname_attr, Not(BaseFilter('test0')))] + + v1_ids = self._get_v1_server_ids(filters, {}) + v2_ids = self._get_v2_server_ids(filters, {}) + self.assertEqual(v1_ids, v2_ids) + + def test_comparison_filter_match(self): + """Comparison filters should return same results.""" + object_id_attr = Attribute.specials['object_id'] + filters = [(object_id_attr, GreaterThan(1))] + + v1_ids = self._get_v1_server_ids(filters, {}) + v2_ids = self._get_v2_server_ids(filters, {}) + self.assertEqual(v1_ids, v2_ids) + + def test_multiple_filters_match(self): + """Multiple filters should return same results.""" + hostname_attr = Attribute.specials['hostname'] + servertype_attr = Attribute.specials['servertype'] + filters = [ + (hostname_attr, Regexp('^test')), + (servertype_attr, BaseFilter('test0')), + ] + + v1_ids = self._get_v1_server_ids(filters, {}) + v2_ids = self._get_v2_server_ids(filters, {}) + self.assertEqual(v1_ids, v2_ids) + + +class TestQueryBuilderFacade(TransactionTestCase): + """Test the query_builder facade switches correctly.""" + + fixtures = ['auth_user.json', 'test_dataset.json'] + + @override_settings(SQL_GENERATOR_VERSION='v1') + def test_v1_returns_string(self): + """V1 setting should return SQL string.""" + from serveradmin.serverdb.query_builder import get_server_query + result = get_server_query([], {}) + self.assertIsInstance(result, str) + + @override_settings(SQL_GENERATOR_VERSION='v2') + def test_v2_returns_queryset(self): + """V2 setting should return QuerySet.""" + from serveradmin.serverdb.query_builder import get_server_query + result = get_server_query([], {}) + self.assertIsInstance(result, QuerySet) + + @override_settings(SQL_GENERATOR_VERSION='v1') + def test_default_is_v1(self): + """Default setting should be v1.""" + from serveradmin.serverdb.query_builder import get_server_query + result = get_server_query([], {}) + self.assertIsInstance(result, str) + + +class TestBuildValueCondition(TransactionTestCase): + """Unit tests for _build_value_condition function.""" + + fixtures = ['auth_user.json', 'test_dataset.json'] + + def test_base_filter_creates_equality(self): + """BaseFilter should create equality condition.""" + attr = Attribute.specials['hostname'] + filt = BaseFilter('test') + condition = _build_value_condition(attr, filt) + + # Should have value='test' in children + self.assertTrue(any( + child == ('value', 'test') + for child in condition.children + if isinstance(child, tuple) + )) + + def test_regexp_filter_creates_regex_lookup(self): + """Regexp filter should create regex lookup.""" + attr = Attribute.specials['hostname'] + filt = Regexp('^test') + condition = _build_value_condition(attr, filt) + + # Should have value__regex in children + self.assertTrue(any( + child[0] == 'value__regex' + for child in condition.children + if isinstance(child, tuple) + )) + + def test_greater_than_creates_gt_lookup(self): + """GreaterThan filter should create __gt lookup.""" + attr = Attribute.specials['object_id'] + filt = GreaterThan(5) + condition = _build_value_condition(attr, filt) + + self.assertTrue(any( + child[0] == 'value__gt' and child[1] == 5 + for child in condition.children + if isinstance(child, tuple) + )) + + +class TestBuildLogicalCondition(TransactionTestCase): + """Unit tests for _build_logical_condition function.""" + + fixtures = ['auth_user.json', 'test_dataset.json'] + + def test_empty_any_creates_false_condition(self): + """Empty Any should create always-false condition.""" + attr = Attribute.specials['hostname'] + filt = Any() + condition = _build_logical_condition(attr, filt, {}) + + # The condition should match nothing + # We verify by checking it's a complex condition that evaluates to false + result = Server.objects.filter(condition) + self.assertEqual(result.count(), 0) + + def test_empty_all_creates_true_condition(self): + """Empty All should create always-true condition.""" + attr = Attribute.specials['hostname'] + filt = All() + condition = _build_logical_condition(attr, filt, {}) + + # The condition should match everything + result = Server.objects.filter(condition) + self.assertEqual(result.count(), Server.objects.count()) + + def test_any_with_simple_values_optimizes_to_in(self): + """Any with simple BaseFilter values should use IN clause.""" + attr = Attribute.specials['hostname'] + filt = Any('test0', 'test1', 'test2') + condition = _build_logical_condition(attr, filt, {}) + + # Should work correctly regardless of optimization + result = Server.objects.filter(condition) + hostnames = {s.hostname for s in result} + self.assertTrue(hostnames.issubset({'test0', 'test1', 'test2'})) diff --git a/serveradmin/settings.py b/serveradmin/settings.py index fdcc1713..3cf6f3da 100644 --- a/serveradmin/settings.py +++ b/serveradmin/settings.py @@ -42,6 +42,10 @@ }, } +# SQL Generator version: 'v1' (raw SQL) or 'v2' (Django ORM) +# Use 'v2' for improved security and query optimization +SQL_GENERATOR_VERSION = env('SQL_GENERATOR_VERSION', default='v1') + MIDDLEWARE = [ 'django.middleware.common.CommonMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware',