Repository: jdkandersson/OpenAlchemy

Issue:
Don't return null column values unless they are required for to_dict and to_str
`null` values for non-required properties are not valid JSON schema dictionaries unless they are nullable. Change the rule for null values to:

- [ ] If the value is required, return it.
- [ ] If the value is nullable, return it.
- [ ] If the value is not nullable and not required, don't return it.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index adf69203..71d90b35 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 - Add support for `readOnly`.
 - Fix bug where TypedDIct types for `binary`, `date` and `date-time` string formats mapped to the incorrect python types.
+- Fix bug where `to_dict` and `to_str` returned `null` for values that are not required and not nullable.
 
 ## Version 1.1.0 - 2020-04-05
 
diff --git a/open_alchemy/utility_base/__init__.py b/open_alchemy/utility_base/__init__.py
index 70181b32..76846c7a 100644
--- a/open_alchemy/utility_base/__init__.py
+++ b/open_alchemy/utility_base/__init__.py
@@ -221,6 +221,15 @@ class UtilityBase:
         return_dict: typing.Dict[str, typing.Any] = {}
         for name, property_schema in properties.items():
             value = getattr(instance, name, None)
+
+            # Handle none value
+            if value is None:
+                return_none = to_dict.return_none(schema=schema, property_name=name)
+                if return_none is True:
+                    return_dict[name] = None
+                # Don't consider for coverage due to coverage bug
+                continue  # pragma: no cover
+
             try:
                 return_dict[name] = to_dict.convert(schema=property_schema, value=value)
             except exceptions.BaseError as exc:
diff --git a/open_alchemy/utility_base/to_dict/__init__.py b/open_alchemy/utility_base/to_dict/__init__.py
index 67079c56..41cd0680 100644
--- a/open_alchemy/utility_base/to_dict/__init__.py
+++ b/open_alchemy/utility_base/to_dict/__init__.py
@@ -31,3 +31,36 @@ def convert(*, schema: oa_types.Schema, value: typing.Any) -> types.TAnyDict:
     if type_ in {"integer", "number", "string", "boolean"}:
         return simple.convert(value, schema=schema)
     raise exceptions.FeatureNotImplementedError(f"Type {type_} is not supported.")
+
+
+def return_none(*, schema: oa_types.Schema, property_name: str) -> bool:
+    """
+    Check whether a null value for a property should be returned.
+
+    Assume the schema has properties and that it has a schema for the property.
+    Assume that any $ref and allOf has already been resolved.
+
+    The rules are:
+    1. if the property is required, return it,
+    2. if the property is nullable, return it and
+    3. else, don't return it.
+
+    Args:
+        schema: The schema for the model.
+        property_name: The name of the property to check for.
+
+    Returns:
+        Whether the none value should be returned for the property.
+
+    """
+    # Retrieve input
+    required_array = schema.get("required", None)
+    property_schema = schema["properties"][property_name]
+
+    # Check for required and nullable
+    if required_array is not None and property_name in set(required_array):
+        return True
+    nullable_value = helpers.peek.nullable(schema=property_schema, schemas={})
+    if nullable_value is True:
+        return True
+    return False


Test patch:
diff --git a/tests/open_alchemy/utility_base/test_to_dict.py b/tests/open_alchemy/utility_base/test_to_dict.py
index eba6f58c..51713a44 100644
--- a/tests/open_alchemy/utility_base/test_to_dict.py
+++ b/tests/open_alchemy/utility_base/test_to_dict.py
@@ -41,14 +41,31 @@ def test_to_dict_no_properties(__init__):
 @pytest.mark.parametrize(
     "schema, value, expected_value",
     [
-        ({"properties": {"key": {"type": "integer"}}}, {"key": 1}, {"key": 1}),
-        (
+        pytest.param(
+            {"properties": {"key": {"type": "integer"}}},
+            {"key": 1},
+            {"key": 1},
+            id="single",
+        ),
+        pytest.param(
+            {"properties": {"key": {"type": "integer"}}},
+            {"key": None},
+            {},
+            id="single null value not return",
+        ),
+        pytest.param(
+            {"properties": {"key": {"type": "integer", "nullable": True}}},
+            {"key": None},
+            {"key": None},
+            id="single null value return",
+        ),
+        pytest.param(
             {"properties": {"key_1": {"type": "integer"}, "key_2": {"type": "string"}}},
             {"key_1": 1, "key_2": "value 2"},
             {"key_1": 1, "key_2": "value 2"},
+            id="multiple",
         ),
     ],
-    ids=["single", "multiple"],
 )
 @pytest.mark.utility_base
 def test_valid(__init__, schema, value, expected_value):
diff --git a/tests/open_alchemy/utility_base/to_dict/test_to_dict.py b/tests/open_alchemy/utility_base/to_dict/test_to_dict.py
new file mode 100644
index 00000000..fe056fd8
--- /dev/null
+++ b/tests/open_alchemy/utility_base/to_dict/test_to_dict.py
@@ -0,0 +1,74 @@
+"""Tests for to_dict."""
+
+import pytest
+
+from open_alchemy.utility_base import to_dict
+
+
+@pytest.mark.parametrize(
+    "schema, expected_result",
+    [
+        pytest.param(
+            {"properties": {"prop_1": {"key": "value"}}},
+            False,
+            id="nullable not set required not set expect false",
+        ),
+        pytest.param(
+            {"properties": {"prop_1": {"key": "value"}}, "required": []},
+            False,
+            id="nullable not set required empty expect false",
+        ),
+        pytest.param(
+            {"properties": {"prop_1": {"key": "value"}}, "required": ["prop_2"]},
+            False,
+            id="nullable not set required different property expect false",
+        ),
+        pytest.param(
+            {"properties": {"prop_1": {"key": "value"}}, "required": ["prop_1"]},
+            True,
+            id="nullable not set required has property expect true",
+        ),
+        pytest.param(
+            {
+                "properties": {"prop_1": {"key": "value", "nullable": False}},
+                "required": [],
+            },
+            False,
+            id="nullable false required empty expect false",
+        ),
+        pytest.param(
+            {
+                "properties": {"prop_1": {"key": "value", "nullable": False}},
+                "required": ["prop_1"],
+            },
+            True,
+            id="nullable false required has property expect true",
+        ),
+        pytest.param(
+            {
+                "properties": {"prop_1": {"key": "value", "nullable": True}},
+                "required": [],
+            },
+            True,
+            id="nullable true required empty expect true",
+        ),
+        pytest.param(
+            {
+                "properties": {"prop_1": {"key": "value", "nullable": True}},
+                "required": ["prop_1"],
+            },
+            True,
+            id="nullable true required has property expect true",
+        ),
+    ],
+)
+@pytest.mark.utility_base
+def test_return_none(schema, expected_result):
+    """
+    GIVEN schema with property and expected result
+    WHEN return_none is called with the schema and property name
+    THEN the expected result is returned.
+    """
+    result = to_dict.return_none(schema=schema, property_name="prop_1")
+
+    assert result == expected_result