Feature/query encoding (#27)

* Adding class _QueryUtil to bundle query encoding related utilities.

* Adding tests for special character encoding.

* Fixed handling and documentation of inventory API for querying by name. Added query parameter for specification of custom queries.

Author: chsou

CHANGELOG.md

@@ -3,6 +3,13 @@
 
 ## Work in progress
 
+* Adding class _QueryUtil, bundling query encoding related functionality.
+
+* Added tests for special character parsing.
+
+* Fixed handling and documentation of inventory API for querying by name. 
+  Added query parameter for specification of custom queries.
+
 * Reverted changes in ComplexObject - a ComplexObject is not a dictionary-like class, it only   
   supports some dictionary-like access functions. But, for instance, updating a ComplexObject
   is very different from updating a dictionary. Hence, it no longer inherits MutableMapping.

c8y_api/model/_util.py

@@ -6,6 +6,18 @@
 
 from datetime import datetime, timedelta, timezone
 from dateutil import parser
+from re import sub
+
+
+class _QueryUtil(object):
+
+    @staticmethod
+    def encode_odata_query_value(value):
+        """Encode value strings according to OData query rules.
+        http://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#sec_URLParsing
+        http://docs.oasis-open.org/odata/odata/v4.01/cs01/abnf/odata-abnf-construction-rules.txt """
+        # single quotes escaped through single quote
+        return sub('\'', '\'\'', value)
 
 
 class _DateUtil(object):

c8y_api/model/inventory.py

@@ -10,6 +10,7 @@
 from typing import Any, Generator, List
 
 from c8y_api.model._base import CumulocityResource
+from c8y_api.model._util import _QueryUtil
 from c8y_api.model.managedobjects import ManagedObjectUtil, ManagedObject, Device, DeviceGroup
 
 
@@ -52,10 +53,10 @@ def get_all(self, type: str = None, fragment: str = None, name: str = None, owne
         Returns:
             List of ManagedObject instances
         """
-        return list(self.select(type=type, fragment=fragment, name=name, limit=limit, page_size=page_size))
+        return list(self.select(type=type, fragment=fragment, name=name, owner=owner, limit=limit, page_size=page_size))
 
     def select(self, type: str = None, fragment: str = None, name: str = None, owner: str = None,  # noqa (type)
-               limit: int = None, page_size: int = 1000) -> Generator[ManagedObject]:
+               query: str = None, limit: int = None, page_size: int = 1000) -> Generator[ManagedObject]:
         """ Query the database for managed objects and iterate over the
         results.
 
@@ -70,7 +71,12 @@ def select(self, type: str = None, fragment: str = None, name: str = None, owner
             type (str):  Managed object type
             fragment (str):  Name of a present custom/standard fragment
             name (str):  Name of the managed object
+                Note: The Cumulocity REST API does not support filtering for
+                names directly; this is a convenience parameter which will
+                translate all filters into a query string.
             owner (str):  Username of the object owner
+            query (str):  Complex query to execute; all other filters are
+                ignored if such a custom query is provided
             limit (int): Limit the number of results to this number.
             page_size (int): Define the number of events which are read (and
                 parsed in one chunk). This is a performance related setting.
@@ -79,12 +85,37 @@ def select(self, type: str = None, fragment: str = None, name: str = None, owner
             Generator for ManagedObject instances
         """
         return self._select(ManagedObject.from_json, type=type, fragment=fragment, name=name, owner=owner,
-                            limit=limit, page_size=page_size)
+                            query=None, limit=limit, page_size=page_size)
+
+    def _select(self, jsonyfy_func, type: str = None, fragment: str = None, name: str = None,  # noqa
+                owner: str = None, query: str = None, limit: int = None, page_size: int = 1000) -> Generator[Any]:
+
+        query_filters = []
+
+        # if there is no custom query, we check whether standard filters need to
+        # be translated into a query
+        if not query and name:
+
+            # A name filter can only be expressed as a query, which then
+            # triggers "query mode" (all filters are translated into a query)
+            query_filters.append(f"name eq '{_QueryUtil.encode_odata_query_value(name)}'")
+
+            if type:
+                query_filters.append(f"type eq '{type}'")
+            if owner:
+                query_filters.append(f"owner eq '{owner}'")
+            if fragment:
+                query_filters.append(f"has({fragment})")
+            if len(query_filters) == 1:
+                query = query_filters[0]
+            else:
+                query = '$filter=(' + ' and '.join(query_filters) + ')'
+
+        if query:
+            base_query = self._build_base_query(query=query, page_size=page_size)
+        else:
+            base_query = self._build_base_query(type=type, fragment=fragment, owner=owner, page_size=page_size)
 
-    def _select(self, jsonyfy_func, type: str = None, fragment: str = None, name: str = None,
-                owner: str = None,  limit: int = None, page_size: int = 1000) -> Generator[Any]:
-        base_query = self._build_base_query(type=type, fragment=fragment, owner=owner,
-                                            query=f"name eq '{name}'" if name else None, page_size=page_size)
         return super()._iterate(base_query, limit, jsonyfy_func)
 
     def create(self, *objects: ManagedObject):
@@ -166,7 +197,7 @@ def get(self, id: str) -> Device:  # noqa (id)
         return device
 
     def select(self, type: str = None, name: str = None, owner: str = None,  # noqa (type, args)
-               limit: int = None, page_size: int = 100) -> Generator[Device]:
+               query: str = None, limit: int = None, page_size: int = 100) -> Generator[Device]:
         # pylint: disable=arguments-differ
         """ Query the database for devices and iterate over the results.
 
@@ -180,7 +211,12 @@ def select(self, type: str = None, name: str = None, owner: str = None,  # noqa
         Args:
             type (str):  Device type
             name (str):  Name of the device
+                Note: The Cumulocity REST API does not support filtering for
+                names directly; this is a convenience parameter which will
+                translate all filters into a query string.
             owner (str):  Username of the object owner
+            query (str):  Complex query to execute; all other filters are
+                ignored if such a custom query is provided
             limit (int): Limit the number of results to this number.
             page_size (int): Define the number of events which are read (and
                 parsed in one chunk). This is a performance related setting.
@@ -189,7 +225,7 @@ def select(self, type: str = None, name: str = None, owner: str = None,  # noqa
             Generator for Device objects
         """
         return self._select(ManagedObject.from_json, type=type, fragment='c8y_IsDevice', name=name, owner=owner,
-                            limit=limit, page_size=page_size)
+                            query=query, limit=limit, page_size=page_size)
 
     def get_all(self, type: str = None, name: str = None, owner: str = None,   # noqa (type, parameters)
                 page_size: int = 100) -> List[Device]:
@@ -244,8 +280,8 @@ def get(self, group_id):
         group.c8y = self.c8y
         return group
 
-    def select(self, type: str = DeviceGroup.ROOT_TYPE, parent: str | int = None, fragment: str = None,
-               name: str = None, owner: str = None, page_size: int = 100) -> Generator[DeviceGroup]:
+    def select(self, type: str = DeviceGroup.ROOT_TYPE, parent: str | int = None, fragment: str = None,  # noqa
+               name: str = None, owner: str = None, query: str = None, page_size: int = 100) -> Generator[DeviceGroup]:
         # pylint: disable=arguments-differ, arguments-renamed
         """ Select device groups by various parameters.
 
@@ -263,41 +299,55 @@ def select(self, type: str = DeviceGroup.ROOT_TYPE, parent: str | int = None, fr
                 match device groups only you need to use the fragment filter.
             parent (str): ID of the parent device group
                 Note: this forces the `type` filter to be c8y_DeviceSubGroup
+                Like the `name` parameter, this is a convenience parameter
+                which will translate all filters into a query string.
             fragment (str): Additional fragment present within the objects
-            name (str): Name string of the groups to select; no partial
-                matching/patterns are supported
+            name (str): Name string of the groups to select
+                Note:  he Cumulocity REST API does not support filtering for
+                names directly; this is a convenience parameter which will
+                translate all filters into a query string.
+                No partial matching/patterns are supported
             owner (str): Username of the group owner
+            query (str):  Complex query to execute; all other filters are
+                ignored if such a custom query is provided
             page_size (int): Define the number of events which are read (and
                 parsed in one chunk). This is a performance related setting.
 
         Returns:
             Generator of DeviceGroup instances
         """
         query_filters = []
-        if name:
-            query_filters.append(f"name eq '{name}'")
-        if parent:
-            query_filters.append(f"bygroupid({parent})")
-            type = DeviceGroup.CHILD_TYPE
-
-        # if any query was defined, all filters must be put into the query
-        if query_filters:
-            query_filters.append(f"type eq {type}")
-            # all other filters must be set as well
-            if fragment:
-                query_filters.append(f"has({fragment})")
-            if owner:
-                query_filters.append(f"owner eq '{owner}'")
-            query = '$filter=' + ' and '.join(query_filters)
 
+        # if there is no custom query, we check whether standard filters need to
+        # be translated into a query
+        if not query:
+
+            # Both name and parent filters can only be expressed as a query,
+            # which then triggers "query mode"
+            if name:
+                query_filters.append(f"name eq '{_QueryUtil.encode_odata_query_value(name)}'")
+            if parent:
+                query_filters.append(f"bygroupid({parent})")
+                type = DeviceGroup.CHILD_TYPE  # noqa
+
+            # if any query was defined, all filters must be put into the query
+            if query_filters:
+                if type:
+                    query_filters.append(f"type eq '{type}'")
+                if owner:
+                    query_filters.append(f"owner eq '{owner}'")
+                if fragment:
+                    query_filters.append(f"has({fragment}")
+                query = '$filter=' + ' and '.join(query_filters)
+
+        if query:
             base_query = self._build_base_query(query=query, page_size=page_size)
-        # otherwise we can just build the regular query
         else:
             base_query = self._build_base_query(type=type, fragment=fragment, owner=owner, page_size=page_size)
 
         return super()._iterate(base_query, limit=9999, parse_func=DeviceGroup.from_json)
 
-    def get_all(self, type: str = DeviceGroup.ROOT_TYPE, parent: str | int = None, fragment: str = None,
+    def get_all(self, type: str = DeviceGroup.ROOT_TYPE, parent: str | int = None, fragment: str = None, # noqa
                 name: str = None, owner: str = None, page_size: int = 100):  # noqa
         # pylint: disable=arguments-differ, arguments-renamed
         """ Select managed objects by various parameters.
@@ -328,9 +378,9 @@ def assign_children(self, root_id, *child_ids):
         # adding multiple references at once is not (yet) supported
         # refs = {'references': [InventoryUtil.build_managed_object_reference(id) for id in child_ids]}
         # self.c8y.post(self.build_object_path(root_id) + '/childAssets', json=refs, accept='')
-        for id in child_ids:
+        for child_id in child_ids:
             self.c8y.post(self.build_object_path(root_id) + '/childAssets',
-                          json=ManagedObjectUtil.build_managed_object_reference(id), accept='')
+                          json=ManagedObjectUtil.build_managed_object_reference(child_id), accept='')
 
     def unassign_children(self, root_id, *child_ids):
         """Unlink child groups from this device group.
@@ -339,7 +389,7 @@ def unassign_children(self, root_id, *child_ids):
             root_id (str|int): ID of the root device group
             child_ids (*str|int): ID of the child device groups
         """
-        refs = {'references': [ManagedObjectUtil.build_managed_object_reference(id) for id in child_ids]}
+        refs = {'references': [ManagedObjectUtil.build_managed_object_reference(i) for i in child_ids]}
         self.c8y.delete(self.build_object_path(root_id) + '/childAssets', json=refs)
 
     def delete(self, *groups: DeviceGroup | str | int):

integration_tests/test_inventory.py

@@ -87,6 +87,7 @@ def similar_objects(object_factory) -> List[ManagedObject]:
 
 @pytest.mark.parametrize('key, value_fun', [
     ('type', lambda mo: mo.type),
+    ('name', lambda mo: mo.type + '*'),
     ('fragment', lambda mo: mo.type + '_fragment')
 ])
 def test_get_by_something(live_c8y: CumulocityApi, similar_objects: List[ManagedObject], key, value_fun):

tests/model/test_inventory.py

@@ -0,0 +1,69 @@
+# Copyright (c) 2020 Software AG,
+# Darmstadt, Germany and/or Software AG USA Inc., Reston, VA, USA,
+# and/or its subsidiaries and/or its affiliates and/or their licensors.
+# Use, reproduction, transfer, publication or disclosure is prohibited except
+# as specifically provided for in your License Agreement with Software AG.
+from unittest.mock import Mock
+
+import pytest
+from urllib import parse
+
+from c8y_api import CumulocityRestApi
+from c8y_api.model import Inventory
+from c8y_api.model._util import _QueryUtil
+from tests.utils import isolate_last_call_arg
+
+
+@pytest.mark.parametrize('test, expected', [
+    ('string', 'string'),
+    ('with spaces', 'with spaces'),
+    ('quote\'s', 'quote\'\'s')
+])
+def test_encode_odata_query_value(test, expected):
+    """Verify that the query value encoding works as expected."""
+    assert _QueryUtil.encode_odata_query_value(test) == expected
+
+
+@pytest.mark.parametrize('name, expected', [
+    ('some name', 'query=name eq \'some name\''),
+    ('some\'s name', 'query=name eq \'some\'\'s name\'')
+])
+def test_select_by_name(name, expected):
+    """Verify that the inventory's select function can filter by name."""
+
+    # In the end, the select function should result in a GET request; the
+    # result of this is not important, we simulate an empty result set.
+    c8y: CumulocityRestApi = Mock()
+    c8y.get = Mock(return_value={'managedObjects': []})
+
+    inventory = Inventory(c8y)
+    inventory.get_all(name=name)
+
+    assert c8y.get.call_count == 1
+    url = parse.unquote_plus(isolate_last_call_arg(c8y.get, 'resource', 0))
+    assert expected in url
+
+
+def test_select_by_name_plus():
+    """Verify that the inventory's select function will put all filters
+    as parts of a complex query."""
+
+    c8y: CumulocityRestApi = Mock()
+    c8y.get = Mock(return_value={'managedObjects': []})
+
+    inventory = Inventory(c8y)
+    inventory.get_all(name='NAME', fragment='FRAGMENT', type='TYPE', owner='OWNER')
+
+    # we expect that the following strings are part of the resource string
+    expected = [
+        'query=$filter=(',
+        'has(FRAGMENT)',
+        'name eq \'NAME\'',
+        'owner eq \'OWNER\'',
+        'type eq \'TYPE\'']
+
+    assert c8y.get.call_count == 1
+    url = parse.unquote_plus(isolate_last_call_arg(c8y.get, 'resource', 0))
+
+    for e in expected:
+        assert e in url

tests/model/test_parser.py

@@ -30,7 +30,8 @@ def __init__(self):
     mapping = {
         'string_field': 'db_string',
         'int_field': 'db_int',
-        'boolean_field': 'db_boolean'}
+        'boolean_field': 'db_boolean',
+        'special_field': 'db_special'}
 
     return TestClass(), mapping
 
@@ -47,13 +48,15 @@ def test_from_json_simple(simple_object_and_mapping):
         'db_int': random.randint(100, 200),
         'db_string': RandomNameGenerator.random_name(),
         'db_boolean': True,
+        'db_special': "Special chars: ,._#'`\"?$%7{",
         'to_be_ignored_fragment': {'level': 2}}
 
     parsed_obj = parser.from_json(source_json, obj)
 
     assert parsed_obj.int_field == source_json['db_int']
     assert parsed_obj.string_field == source_json['db_string']
     assert parsed_obj.boolean_field == source_json['db_boolean']
+    assert parsed_obj.special_field == source_json['db_special']
 
 
 def test_from_json_simple_skip(simple_object_and_mapping):