feat: make url encode default, add encoding for delete and get entity set (#243)

* add option to not encode path for get and update requests
* doc: update encode path description
* tests: rename path not encoded tests

Author: linda-sap

CHANGELOG.md

@@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
+- Service: Encode paths with entity keys with option to disable it - linda-sap
 
 ## [1.10.1]
 

docs/usage/deleting.rst

@@ -41,4 +41,16 @@ or by passing the EntityKey object
 
     key = EntityKey(service.schema.entity_type('Employee'), ID=23)
     request = service.entity_sets.Employees.delete_entity(key=key)
-    request.execute()
+    request.execute()
+
+
+Encode OData URL Path
+-------------------------------------------
+
+By default the resource paths of requests are percent encoded. However if this is not what your API expects, 
+you can disable the encoding with the variable encode_path by setting it to False.
+
+
+.. code-block:: python
+
+   request = service.entity_sets.Employees.delete_entity(key=key, encode_path=False)

docs/usage/querying.rst

@@ -154,7 +154,15 @@ these parameters by the method `custom(name: str, value: str)`.
 
     employees = northwind.entity_sets.Employees.get_entities().custom('sap-client', '100').custom('$skiptoken', 'ABCD').top(10).execute() 
     
+Encode OData URL Path
+-------------------------------------------
+
+By default the resource paths of requests are percent encoded. However if this is not what your API expects, 
+you can disable the encoding with the variable encode_path by setting it to False.
+
+.. code-block:: python
 
+    employee = northwind.entity_sets.Employees.get_entity(1, encode_path=False).execute()
 
 (Experimental) Query server-side paginations using the __next field
 -------------------------------------------------------------------

docs/usage/updating.rst

@@ -34,3 +34,14 @@ then you can consider setting the default service's update method to *PUT*.
 .. code-block:: python
 
     northwind.config['http']['update_method'] = 'PUT'
+
+Encode OData URL Path
+-------------------------------------------
+
+By default the resource paths of requests are percent encoded. However if this is not what your API expects, 
+you can disable the encoding with the variable encode_path by setting it to False.
+
+
+.. code-block:: python
+
+    update_request = northwind.entity_sets.Customers.update_entity(CustomerID='ALFKI', encode_path=False)

pyodata/v2/service.py

@@ -13,7 +13,7 @@
 from email.parser import Parser
 from http.client import HTTPResponse
 from io import BytesIO
-from urllib.parse import urlencode
+from urllib.parse import urlencode, quote
 
 
 from pyodata.exceptions import HttpError, PyODataException, ExpressionError, ProgramError
@@ -371,14 +371,15 @@ def custom(self, name, value):
 class EntityGetRequest(ODataHttpRequest):
     """Used for GET operations of a single entity"""
 
-    def __init__(self, handler, entity_key, entity_set_proxy):
+    def __init__(self, handler, entity_key, entity_set_proxy, encode_path=True):
         super(EntityGetRequest, self).__init__(entity_set_proxy.service.url, entity_set_proxy.service.connection,
                                                handler)
         self._logger = logging.getLogger(LOGGER_NAME)
         self._entity_key = entity_key
         self._entity_set_proxy = entity_set_proxy
         self._select = None
         self._expand = None
+        self._encode_path = encode_path
 
         self._logger.debug('New instance of EntityGetRequest for last segment: %s', self._entity_set_proxy.last_segment)
 
@@ -403,6 +404,8 @@ def expand(self, expand):
         return self
 
     def get_path(self):
+        if self.get_encode_path():
+            return quote(self._entity_set_proxy.last_segment + self._entity_key.to_key_string())
         return self._entity_set_proxy.last_segment + self._entity_key.to_key_string()
 
     def get_default_headers(self):
@@ -439,6 +442,10 @@ def stream_handler(response):
             connection,
             stream_handler)
 
+    def get_encode_path(self):
+        """Getter for encode path flag"""
+        return self._encode_path
+
 
 class NavEntityGetRequest(EntityGetRequest):
     """Used for GET operations of a single entity accessed via a Navigation property"""
@@ -545,22 +552,30 @@ def set(self, **kwargs):
 class EntityDeleteRequest(ODataHttpRequest):
     """Used for deleting entity (DELETE operations on a single entity)"""
 
-    def __init__(self, url, connection, handler, entity_set, entity_key):
+    def __init__(self, url, connection, handler, entity_set, entity_key, encode_path=True):
         super(EntityDeleteRequest, self).__init__(url, connection, handler)
         self._logger = logging.getLogger(LOGGER_NAME)
         self._entity_set = entity_set
         self._entity_key = entity_key
+        self._encode_path = encode_path
 
         self._logger.debug('New instance of EntityDeleteRequest for entity type: %s', entity_set.entity_type.name)
 
     def get_path(self):
+        if self.get_encode_path():
+            return quote(self._entity_set.name + self._entity_key.to_key_string())
         return self._entity_set.name + self._entity_key.to_key_string()
 
+    def get_encode_path(self):
+        """Getter for encode path flag"""
+        return self._encode_path
+
     def get_method(self):
         # pylint: disable=no-self-use
         return 'DELETE'
 
 
+# pylint: disable=too-many-instance-attributes
 class EntityModifyRequest(ODataHttpRequest):
     """Used for modyfing entities (UPDATE/MERGE operations on a single entity)
 
@@ -569,12 +584,14 @@ class EntityModifyRequest(ODataHttpRequest):
 
     ALLOWED_HTTP_METHODS = ['PATCH', 'PUT', 'MERGE']
 
-    def __init__(self, url, connection, handler, entity_set, entity_key, method="PATCH"):
+    # pylint: disable=too-many-arguments
+    def __init__(self, url, connection, handler, entity_set, entity_key, method="PATCH", encode_path=True):
         super(EntityModifyRequest, self).__init__(url, connection, handler)
         self._logger = logging.getLogger(LOGGER_NAME)
         self._entity_set = entity_set
         self._entity_type = entity_set.entity_type
         self._entity_key = entity_key
+        self._encode_path = encode_path
 
         self._method = method.upper()
         if self._method not in EntityModifyRequest.ALLOWED_HTTP_METHODS:
@@ -589,6 +606,8 @@ def __init__(self, url, connection, handler, entity_set, entity_key, method="PAT
         self._logger.debug('New instance of EntityModifyRequest for entity type: %s', self._entity_type.name)
 
     def get_path(self):
+        if self.get_encode_path():
+            return quote(self._entity_set.name + self._entity_key.to_key_string())
         return self._entity_set.name + self._entity_key.to_key_string()
 
     def get_method(self):
@@ -605,6 +624,10 @@ def get_body(self):
     def get_default_headers(self):
         return {'Accept': 'application/json', 'Content-Type': 'application/json'}
 
+    def get_encode_path(self):
+        """Getter for encode path flag"""
+        return self._encode_path
+
     def set(self, **kwargs):
         """Set properties to be changed."""
 
@@ -1307,10 +1330,11 @@ def __str__(self):
 class GetEntitySetRequest(QueryRequest):
     """GET on EntitySet"""
 
-    def __init__(self, url, connection, handler, last_segment, entity_type):
+    def __init__(self, url, connection, handler, last_segment, entity_type, encode_path=True):
         super(GetEntitySetRequest, self).__init__(url, connection, handler, last_segment)
 
         self._entity_type = entity_type
+        self._encode_path = encode_path
 
     def __getattr__(self, name):
         proprty = self._entity_type.proprty(name)
@@ -1329,6 +1353,20 @@ def filter(self, *args, **kwargs):
 
         return self
 
+    def get_path(self):
+        if self.get_encode_path():
+            path = quote(self._last_segment)
+        else:
+            path = self._last_segment
+
+        if self._count:
+            return urljoin(path, '/$count')
+        return path
+
+    def get_encode_path(self):
+        """Getter for encode path flag"""
+        return self._encode_path
+
 
 class ListWithTotalCount(list):
     """
@@ -1452,7 +1490,7 @@ def get_entity_handler(parent, nav_property, navigation_entity_set, response):
             self,
             nav_property)
 
-    def get_entity(self, key=None, **args):
+    def get_entity(self, key=None, encode_path=True, **args):
         """Get entity based on provided key properties"""
 
         def get_entity_handler(response):
@@ -1474,9 +1512,9 @@ def get_entity_handler(response):
 
         self._logger.info('Getting entity %s for key %s and args %s', self._entity_set.entity_type.name, key, args)
 
-        return EntityGetRequest(get_entity_handler, entity_key, self)
+        return EntityGetRequest(get_entity_handler, entity_key, self, encode_path=encode_path)
 
-    def get_entities(self):
+    def get_entities(self, encode_path=True):
         """Get some, potentially all entities"""
 
         def get_entities_handler(response):
@@ -1513,7 +1551,8 @@ def get_entities_handler(response):
 
         entity_set_name = self._alias if self._alias is not None else self._entity_set.name
         return GetEntitySetRequest(self._service.url, self._service.connection, get_entities_handler,
-                                   self._parent_last_segment + entity_set_name, self._entity_set.entity_type)
+                                   self._parent_last_segment + entity_set_name, self._entity_set.entity_type,
+                                   encode_path=encode_path)
 
     def create_entity(self, return_code=HTTP_CODE_CREATED):
         """Creates a new entity in the given entity-set."""
@@ -1533,7 +1572,7 @@ def create_entity_handler(response):
         return EntityCreateRequest(self._service.url, self._service.connection, create_entity_handler, self._entity_set,
                                    self.last_segment)
 
-    def update_entity(self, key=None, method=None, **kwargs):
+    def update_entity(self, key=None, method=None, encode_path=True, **kwargs):
         """Updates an existing entity in the given entity-set."""
 
         def update_entity_handler(response):
@@ -1554,9 +1593,9 @@ def update_entity_handler(response):
             method = self._service.config['http']['update_method']
 
         return EntityModifyRequest(self._service.url, self._service.connection, update_entity_handler, self._entity_set,
-                                   entity_key, method=method)
+                                   entity_key, method=method, encode_path=encode_path)
 
-    def delete_entity(self, key: EntityKey = None, **kwargs):
+    def delete_entity(self, key: EntityKey = None, encode_path=True, **kwargs):
         """Delete the entity"""
 
         def delete_entity_handler(response):
@@ -1573,7 +1612,7 @@ def delete_entity_handler(response):
             entity_key = EntityKey(self._entity_set.entity_type, key, **kwargs)
 
         return EntityDeleteRequest(self._service.url, self._service.connection, delete_entity_handler, self._entity_set,
-                                   entity_key)
+                                   entity_key, encode_path=encode_path)
 
 
 # pylint: disable=too-few-public-methods