v1.2.0-stable (#122)

* from_http bug None and non dict data bug fixes (#119)

* resolving from_http bugs

Signed-off-by: Curtis Mason <[email protected]>

* resolved from_http bugs

Signed-off-by: Curtis Mason <[email protected]>

* nit fix

Signed-off-by: Curtis Mason <[email protected]>

* Exceptions general class (#120)

* More edgecase testing

Signed-off-by: Curtis Mason <[email protected]>

* Tested empty object edge cases

Signed-off-by: Curtis Mason <[email protected]>

* test-coverage

Signed-off-by: Curtis Mason <[email protected]>

* Changelog update (#121)

Signed-off-by: Curtis Mason <[email protected]>

Author: cumason123

CHANGELOG.md

@@ -4,11 +4,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.2.0]
+### Added
+- Added GenericException, DataMarshallingError and DataUnmarshallingError ([#120])
+
 ## [1.1.0]
 ### Changed
 - Changed from_http to now expect headers argument before data ([#110])
 - Renamed exception names ([#111])
 
+### Fixed
+- Fixed from_http bugs with data of type None, or not dict-like ([#119])
+
 ### Deprecated
 - Renamed to_binary_http and to_structured_http. ([#108])
 
@@ -105,3 +112,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#108]: https://github.com/cloudevents/sdk-python/pull/108
 [#110]: https://github.com/cloudevents/sdk-python/pull/110
 [#111]: https://github.com/cloudevents/sdk-python/pull/111
+[#119]: https://github.com/cloudevents/sdk-python/pull/119
+[#120]: https://github.com/cloudevents/sdk-python/pull/120

cloudevents/__init__.py

@@ -1 +1 @@
-__version__ = "1.1.0"
+__version__ = "1.2.0"

cloudevents/exceptions.py

@@ -11,17 +11,29 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-class MissingRequiredFields(Exception):
+class GenericException(Exception):
     pass
 
 
-class InvalidRequiredFields(Exception):
+class MissingRequiredFields(GenericException):
     pass
 
 
-class InvalidStructuredJSON(Exception):
+class InvalidRequiredFields(GenericException):
     pass
 
 
-class InvalidHeadersFormat(Exception):
+class InvalidStructuredJSON(GenericException):
+    pass
+
+
+class InvalidHeadersFormat(GenericException):
+    pass
+
+
+class DataMarshallerError(GenericException):
+    pass
+
+
+class DataUnmarshallerError(GenericException):
     pass

cloudevents/http/event.py

@@ -59,14 +59,14 @@ def __init__(
 
         if self._attributes["specversion"] not in _required_by_version:
             raise cloud_exceptions.MissingRequiredFields(
-                f"Invalid specversion: {self._attributes['specversion']}. "
+                f"Invalid specversion: {self._attributes['specversion']}"
             )
         # There is no good way to default 'source' and 'type', so this
         # checks for those (or any new required attributes).
         required_set = _required_by_version[self._attributes["specversion"]]
         if not required_set <= self._attributes.keys():
             raise cloud_exceptions.MissingRequiredFields(
-                f"Missing required keys: {required_set - self._attributes.keys()}. "
+                f"Missing required keys: {required_set - self._attributes.keys()}"
             )
 
     def __eq__(self, other):

cloudevents/http/http_methods.py

@@ -20,19 +20,21 @@ def from_http(
     Unwrap a CloudEvent (binary or structured) from an HTTP request.
     :param headers: the HTTP headers
     :type headers: typing.Dict[str, str]
-    :param data: the HTTP request body
+    :param data: the HTTP request body. If set to None, "" or b'', the returned
+        event's data field will be set to None
     :type data: typing.IO
     :param data_unmarshaller: Callable function to map data to a python object
         e.g. lambda x: x or lambda x: json.loads(x)
     :type data_unmarshaller: types.UnmarshallerType
     """
-    if data is None:
+    if data is None or data == b"":
+        # Empty string will cause data to be marshalled into None
         data = ""
 
     if not isinstance(data, (str, bytes, bytearray)):
         raise cloud_exceptions.InvalidStructuredJSON(
             "Expected json of type (str, bytes, bytearray), "
-            f"but instead found {type(data)}. "
+            f"but instead found type {type(data)}"
         )
 
     headers = {key.lower(): value for key, value in headers.items()}
@@ -47,22 +49,28 @@ def from_http(
         try:
             raw_ce = json.loads(data)
         except json.decoder.JSONDecodeError:
-            raise cloud_exceptions.InvalidStructuredJSON(
-                "Failed to read fields from structured event. "
-                f"The following can not be parsed as json: {data}. "
+            raise cloud_exceptions.MissingRequiredFields(
+                "Failed to read specversion from both headers and data. "
+                f"The following can not be parsed as json: {data}"
+            )
+        if hasattr(raw_ce, "get"):
+            specversion = raw_ce.get("specversion", None)
+        else:
+            raise cloud_exceptions.MissingRequiredFields(
+                "Failed to read specversion from both headers and data. "
+                f"The following deserialized data has no 'get' method: {raw_ce}"
             )
-        specversion = raw_ce.get("specversion", None)
 
     if specversion is None:
         raise cloud_exceptions.MissingRequiredFields(
-            "Failed to find specversion in HTTP request. "
+            "Failed to find specversion in HTTP request"
         )
 
     event_handler = _obj_by_version.get(specversion, None)
 
     if event_handler is None:
         raise cloud_exceptions.InvalidRequiredFields(
-            f"Found invalid specversion {specversion}. "
+            f"Found invalid specversion {specversion}"
         )
 
     event = marshall.FromRequest(
@@ -73,7 +81,13 @@ def from_http(
     attrs.pop("extensions", None)
     attrs.update(**event.extensions)
 
-    return CloudEvent(attrs, event.data)
+    if event.data == "" or event.data == b"":
+        # TODO: Check binary unmarshallers to debug why setting data to ""
+        # returns an event with data set to None, but structured will return ""
+        data = None
+    else:
+        data = event.data
+    return CloudEvent(attrs, data)
 
 
 def _to_http(
@@ -96,7 +110,7 @@ def _to_http(
 
     if event._attributes["specversion"] not in _obj_by_version:
         raise cloud_exceptions.InvalidRequiredFields(
-            f"Unsupported specversion: {event._attributes['specversion']}. "
+            f"Unsupported specversion: {event._attributes['specversion']}"
         )
 
     event_handler = _obj_by_version[event._attributes["specversion"]]()

cloudevents/http/util.py

@@ -3,7 +3,7 @@
 
 
 def default_marshaller(content: any):
-    if content is None or len(content) == 0:
+    if content is None:
         return None
     try:
         return json.dumps(content)
@@ -12,7 +12,7 @@ def default_marshaller(content: any):
 
 
 def _json_or_string(content: typing.Union[str, bytes]):
-    if content is None or len(content) == 0:
+    if content is None:
         return None
     try:
         return json.loads(content)

cloudevents/sdk/event/base.py

@@ -201,7 +201,14 @@ def MarshalJSON(self, data_marshaller: types.MarshallerType) -> str:
             data_marshaller = lambda x: x  # noqa: E731
         props = self.Properties()
         if "data" in props:
-            data = data_marshaller(props.pop("data"))
+            data = props.pop("data")
+            try:
+                data = data_marshaller(data)
+            except Exception as e:
+                raise cloud_exceptions.DataMarshallerError(
+                    "Failed to marshall data with error: "
+                    f"{type(e).__name__}('{e}')"
+                )
             if isinstance(data, (bytes, bytes, memoryview)):
                 props["data_base64"] = base64.b64encode(data).decode("ascii")
             else:
@@ -225,14 +232,23 @@ def UnmarshalJSON(
             )
 
         for name, value in raw_ce.items():
+            decoder = lambda x: x
             if name == "data":
                 # Use the user-provided serializer, which may have customized
                 # JSON decoding
-                value = data_unmarshaller(json.dumps(value))
+                decoder = lambda v: data_unmarshaller(json.dumps(v))
             if name == "data_base64":
-                value = data_unmarshaller(base64.b64decode(value))
+                decoder = lambda v: data_unmarshaller(base64.b64decode(v))
                 name = "data"
-            self.Set(name, value)
+
+            try:
+                set_value = decoder(value)
+            except Exception as e:
+                raise cloud_exceptions.DataUnmarshallerError(
+                    "Failed to unmarshall data with error: "
+                    f"{type(e).__name__}('{e}')"
+                )
+            self.Set(name, set_value)
 
     def UnmarshalBinary(
         self,
@@ -256,7 +272,15 @@ def UnmarshalBinary(
                 self.SetContentType(value)
             elif header.startswith("ce-"):
                 self.Set(header[3:], value)
-        self.Set("data", data_unmarshaller(body))
+
+        try:
+            raw_ce = data_unmarshaller(body)
+        except Exception as e:
+            raise cloud_exceptions.DataUnmarshallerError(
+                "Failed to unmarshall data with error: "
+                f"{type(e).__name__}('{e}')"
+            )
+        self.Set("data", raw_ce)
 
     def MarshalBinary(
         self, data_marshaller: types.MarshallerType
@@ -276,7 +300,13 @@ def MarshalBinary(
             headers["ce-{0}".format(key)] = value
 
         data, _ = self.Get("data")
-        data = data_marshaller(data)
+        try:
+            data = data_marshaller(data)
+        except Exception as e:
+            raise cloud_exceptions.DataMarshallerError(
+                "Failed to marshall data with error: "
+                f"{type(e).__name__}('{e}')"
+            )
         if isinstance(data, str):  # Convenience method for json.dumps
             data = data.encode("utf-8")
         return headers, data

cloudevents/tests/test_http_cloudevent.py

@@ -2,6 +2,7 @@
 
 import cloudevents.exceptions as cloud_exceptions
 from cloudevents.http import CloudEvent
+from cloudevents.http.util import _json_or_string
 
 
 @pytest.mark.parametrize("specversion", ["0.3", "1.0"])
@@ -75,19 +76,19 @@ def test_http_cloudevent_mutates_equality(specversion):
 def test_cloudevent_missing_specversion():
     attributes = {"specversion": "0.2", "source": "s", "type": "t"}
     with pytest.raises(cloud_exceptions.MissingRequiredFields) as e:
-        event = CloudEvent(attributes, None)
+        _ = CloudEvent(attributes, None)
     assert "Invalid specversion: 0.2" in str(e.value)
 
 
 def test_cloudevent_missing_minimal_required_fields():
     attributes = {"type": "t"}
     with pytest.raises(cloud_exceptions.MissingRequiredFields) as e:
-        event = CloudEvent(attributes, None)
+        _ = CloudEvent(attributes, None)
     assert f"Missing required keys: {set(['source'])}" in str(e.value)
 
     attributes = {"source": "s"}
     with pytest.raises(cloud_exceptions.MissingRequiredFields) as e:
-        event = CloudEvent(attributes, None)
+        _ = CloudEvent(attributes, None)
     assert f"Missing required keys: {set(['type'])}" in str(e.value)
 
 
@@ -114,3 +115,7 @@ def test_cloudevent_general_overrides():
         assert attribute in event
         del event[attribute]
     assert len(event) == 0
+
+
+def test_none_json_or_string():
+    assert _json_or_string(None) is None