Merge pull request #16 from vmware/feature-select-related

Use select_related if query contains foreign key references

bridgeql/__init__.py

@@ -14,7 +14,7 @@
 """
 
 __title__ = 'BridgeQL'
-__version__ = '0.1.12'
+__version__ = '0.1.13'
 __license__ = 'BSD 2-Clause'
 __copyright__ = 'Copyright © 2023 VMware, Inc.  All rights reserved.'
 

bridgeql/django/bridge.py

@@ -36,20 +36,3 @@ def read_django_model(request):
     except Exception as e:
         res = {'data': [], 'message': str(e), 'success': False}
         return JSONResponse(res, status=500)
-
-    """
-    args = {
-        selector: {
-            and: [1,2,3],
-            or: [1,2,3],
-        },
-        values: [],
-        orderby: [],
-        exclude: {
-            buildid: 111
-        },
-        extras: {
-            buildtree_url: []
-        }
-    }
-    """

bridgeql/django/exceptions.py

@@ -3,10 +3,6 @@
 # SPDX-License-Identifier: BSD-2-Clause
 
 
-class InvalidQueryException(Exception):
-    pass
-
-
 class ForbiddenModelOrField(Exception):
     pass
 
@@ -23,5 +19,9 @@ class InvalidModelFieldName(InvalidRequest):
     pass
 
 
+class InvalidQueryException(InvalidRequest):
+    pass
+
+
 class InvalidBridgeQLSettings(Exception):
     pass

bridgeql/django/models.py

@@ -12,7 +12,8 @@
     ForbiddenModelOrField,
     InvalidRequest,
     InvalidAppOrModelName,
-    InvalidModelFieldName
+    InvalidModelFieldName,
+    InvalidQueryException,
 )
 from bridgeql.django.query import construct_query, extract_keys
 from bridgeql.django.settings import bridgeql_settings
@@ -159,11 +160,11 @@ def validate_fields(self, query_fields):
 
 class ModelBuilder(object):
     _QUERYSET_OPTS = [
-        ('exclude', 'exclude'),  # dict
-        ('distinct', 'distinct'),  # bool
-        ('order_by', 'order_by'),  # list
-        ('fields', 'values'),  # list
-        ('count', 'count'),  # bool
+        ('exclude', 'exclude', dict),  # dict
+        ('distinct', 'distinct', bool),  # bool
+        ('order_by', 'order_by', list),  # list
+        ('fields', 'values', list),  # list
+        ('count', 'count', bool),  # bool
     ]
 
     def __init__(self, params):
@@ -180,11 +181,15 @@ def __init__(self, params):
         self.model_config.validate_fields(set(requested_fields))
 
     def _apply_opts(self):
-        for opt, qset_opt in ModelBuilder._QUERYSET_OPTS:
+        for opt, qset_opt, opt_type in ModelBuilder._QUERYSET_OPTS:
             func = getattr(self.qset, qset_opt)
             value = getattr(self.params, opt, None)
             if not value:
                 continue
+            if not isinstance(value, opt_type):
+                raise InvalidQueryException('Invalid type %s for %s'
+                                            ' expected %s'
+                                            % (type(value), opt, opt_type))
             if isinstance(value, dict):
                 self.qset = func(**value)
             elif isinstance(value, list):
@@ -194,7 +199,7 @@ def _apply_opts(self):
                     self.qset = self._add_fields()
                 else:
                     self.qset = func(*value)
-            else:
+            elif value:
                 self.qset = func()
 
     def query_has_properties(self):
@@ -205,17 +210,21 @@ def query_has_properties(self):
 
     def _add_fields(self):
         qset_values = DBRows()
-        self.print_db_query_log()
+        self.qset = self.qset.select_related()
+        logger.debug('Request parameters: %s \nQuery: %s\n',
+                     self.params.params, self.qset.query)
         for row in self.qset:
             model_fields = {}
             for field in self.params.fields:
                 attr = row
                 for ref in field.split('__'):
                     try:
                         attr = getattr(attr, ref)
+                        if attr is None:
+                            break
                     except AttributeError:
                         raise InvalidModelFieldName(
-                            'Invalid query for field %s.' % ref)
+                            'Invalid query for field %s in %s.' % (ref, attr))
                 model_fields[field] = attr
             qset_values.append(model_fields)
         return qset_values
@@ -231,17 +240,15 @@ def queryset(self):
         else:
             self.qset = self.model_config.model.objects.filter(query)
         self._apply_opts()
-        # handle limit and offset seperately
+        # handle limit and offset separately
         if self.params.limit:
             self.qset = self.qset[self.params.offset:
                                   self.params.offset + self.params.limit]
         if isinstance(self.qset, QuerySet):
-            self.print_db_query_log()
+            logger.debug('Request parameters: %s \nQuery: %s\n',
+                         self.params.params, self.qset.query)
             return list(self.qset)
         return self.qset
 
-    def print_db_query_log(self):
-        logger.debug('Request parameters: %s \nQuery: %s',
-                     self.params.params, self.qset.query)
 
 # query -> normal fields, foreign key reference

tests/server/machine/tests/test_api_reader.py

@@ -113,6 +113,21 @@ def test_count_query(self):
         res_json = resp.json()
         self.assertEqual(10, res_json['data'])
 
+    def test_count_false_query(self):
+        self.params = {
+            'app_name': 'machine',
+            'model_name': 'Machine',
+            'filter': {
+                'os__name': 'os-name-5'
+            },
+            'fields': ['name'],
+            'count': False
+        }
+        resp = self.client.get(self.url, {'payload': json.dumps(self.params)})
+        self.assertEqual(resp.status_code, 200)
+        res_json = resp.json()
+        self.assertEqual(10, len(res_json['data']))
+
     def test_count_distinct_query(self):
         self.params = {
             'app_name': 'machine',
@@ -301,6 +316,20 @@ def test_model_isnull(self):
         resp = self.client.get(self.url, {'payload': json.dumps(self.params)})
         self.assertEqual(resp.status_code, 200)
 
+    def test_invalid_query_type(self):
+        self.params = {
+            'app_name': 'machine',
+            'model_name': 'Machine',
+            'filter': {
+                'os__isnull': True,
+            },
+            'fields': ['name', 'arch'],
+            'count': 'yes_invalid'
+        }
+        resp = self.client.get(self.url, {'payload': json.dumps(self.params)})
+        self.assertEqual(resp.status_code, 400)
+        self.assertFalse(resp.json()['success'])
+
     @override_settings(BRIDGEQL_AUTHENTICATION_DECORATOR='server.auth.localtest')
     def test_custom_auth_decorator(self):
         self.params = {

tests/server/machine/tests/test_views.py

@@ -19,16 +19,19 @@ def test_page_index(self):
         self.assertEqual(resp.status_code, 200)
 
     def test_page_schema_should_fail(self):
-        testuser_without_staff = User.objects.create(username='testuser_without_staff')
+        testuser_without_staff = User.objects.create(
+            username='testuser_without_staff')
         testuser_without_staff.set_password('password')
         testuser_without_staff.save()
         _url = reverse('generate_bridgeql_schema')
-        self.client.login(username="testuser_without_staff", password="password")
+        self.client.login(username="testuser_without_staff",
+                          password="password")
         resp = self.client.get(_url)
         self.assertEqual(resp.status_code, 302)
 
     def test_page_schema_should_pass(self):
-        testuser_with_staff = User.objects.create(username='testuser_with_staff')
+        testuser_with_staff = User.objects.create(
+            username='testuser_with_staff')
         testuser_with_staff.set_password('password')
         testuser_with_staff.is_staff = True
         testuser_with_staff.save()

tests/server/server/settings.py

@@ -29,6 +29,38 @@
 # SECURITY WARNING: don't run with debug turned on in production!
 DEBUG = True
 
+LOGGING = {
+    "version": 1,
+    "disable_existing_loggers": False,
+    "formatters": {
+        'standard': {
+            'format': "[%(asctime)s] %(levelname)s [pid:%(process)d] "
+                      "[%(name)s.%(module)s:%(funcName)s] %(message)s",
+            'datefmt': "%d/%b/%Y %H:%M:%S",
+        },
+        'simple': {
+            'format': '%(levelname)s %(message)s',
+        },
+    },
+    "handlers": {
+        "console": {
+            "class": "logging.StreamHandler",
+            "formatter": "standard",
+        },
+    },
+    "root": {
+        "handlers": ["console"],
+        "level": "WARNING",
+    },
+    "loggers": {
+        "bridgeql.django": {
+            "handlers": ["console"],
+            "level": "DEBUG" if DEBUG else os.getenv("DJANGO_LOG_LEVEL", "INFO"),
+            "propagate": False,
+        },
+    },
+}
+
 ALLOWED_HOSTS = []