Merge pull request #103 from sjaensch/parameter_nullable

Refactor nullable-related tests, add support for marshalling

CHANGELOG.rst

@@ -1,5 +1,9 @@
 Changelog
 =========
+4.3.0 (2016-08-01)
+------------------
+- Add support for ``x-nullable``. Thanks to Andreas Hug for the patch!
+
 4.2.5 (2016-07-27)
 ------------------
 - Add basepython python2.7 for flake8, docs, and coverage tox commands

bravado_core/marshal.py

@@ -5,6 +5,7 @@
 from bravado_core.model import is_model, MODEL_MARKER
 from bravado_core.schema import is_dict_like
 from bravado_core.schema import is_list_like
+from bravado_core.schema import is_prop_nullable
 from bravado_core.schema import SWAGGER_PRIMITIVES
 from bravado_core.schema import get_spec_for_prop
 
@@ -125,13 +126,14 @@ def marshal_object(swagger_spec, object_spec, object_value):
     result = {}
     for k, v in iteritems(object_value):
 
-        # Values cannot be None - skip them entirely!
-        if v is None:
-            continue
-
         prop_spec = get_spec_for_prop(
             swagger_spec, deref(object_spec), object_value, k)
 
+        # Values cannot be None unless x-nullable is set
+        is_nullable = prop_spec and is_prop_nullable(swagger_spec, prop_spec)
+        if v is None and not is_nullable:
+            continue
+
         if prop_spec:
             result[k] = marshal_schema_object(swagger_spec, prop_spec, v)
         else:

docs/source/models.rst

@@ -78,6 +78,29 @@ always disable it if necessary.
     swagger_dict = {..}
     spec = Spec.from_dict(swagger_dict, config={'use_models': False})
 
+Allowing null values for properties
+-----------------------------------
+Typically, bravado-core will complain during validation if it encounters fields with ``null`` values.
+This can be problematic, especially when you're adding Swagger support to pre-existing
+APIs. In that case, declare your model properties as ``x-nullable``:
+
+.. code-block:: json
+
+    {
+        "Pet": {
+            "type": "object",
+            "properties": {
+                "breed": {
+                    "type": "string",
+                    "x-nullable": true
+                }
+            }
+        }
+    }
+
+``x-nullable`` is an extension to the Swagger 2.0 spec. A ``nullable`` attribute is being
+`considered <https://github.com/OAI/OpenAPI-Specification/pull/741>`_ for the next major
+version of Swagger.
 
 Model Discovery
 ---------------

tests/marshal/marshal_object_test.py

@@ -239,3 +239,20 @@ def test_recursive_ref_with_depth_n(recursive_swagger_spec):
         }
     }
     assert result == expected
+
+
+def test_marshal_with_object(empty_swagger_spec):
+        param_spec = {
+            'type': 'object',
+            'required': ['x'],
+            'properties': {
+                'x': {
+                    'type': 'string',
+                    'x-nullable': True,
+                }
+            }
+        }
+        value = {'x': None}
+
+        result = marshal_object(empty_swagger_spec, param_spec, value)
+        assert result == value

tests/marshal/marshal_primitive_test.py

@@ -66,3 +66,11 @@ def test_ref(minimal_swagger_dict):
     ref_spec = {'$ref': '#/definitions/Integer'}
     swagger_spec = Spec(minimal_swagger_dict)
     assert 10 == marshal_primitive(swagger_spec, ref_spec, 10)
+
+
+def test_nullable(empty_swagger_spec):
+    string_spec = {
+        'type': 'string',
+        'x-nullable': True,
+    }
+    assert None is marshal_primitive(empty_swagger_spec, string_spec, None)

tests/unmarshal/unmarshal_object_test.py

@@ -236,3 +236,44 @@ def test_recursive_ref_with_depth_n(recursive_swagger_spec):
         }
     }
     assert result == expected
+
+
+def nullable_spec_factory(required, nullable):
+    return {
+        'type': 'object',
+        'required': ['x'] if required else [],
+        'properties': {
+            'x': {
+                'type': 'string',
+                'x-nullable': nullable,
+            }
+        }
+    }
+
+
+@pytest.mark.parametrize('nullable', [True, False])
+@pytest.mark.parametrize('required', [True, False])
+def test_nullable_with_value(empty_swagger_spec, nullable, required):
+    content_spec = nullable_spec_factory(required, nullable)
+    value = {'x': 'y'}
+
+    result = unmarshal_object(empty_swagger_spec, content_spec, value)
+    assert result == value
+
+
+@pytest.mark.parametrize('nullable', [True, False])
+def test_nullable_no_value(empty_swagger_spec, nullable):
+    content_spec = nullable_spec_factory(False, nullable=nullable)
+    value = {}
+
+    result = unmarshal_object(empty_swagger_spec, content_spec, value)
+    assert result == {'x': None}  # Missing parameters are re-introduced
+
+
+@pytest.mark.parametrize('required', [True, False])
+def test_nullable_none_value(empty_swagger_spec, required):
+    content_spec = nullable_spec_factory(required, True)
+    value = {'x': None}
+
+    result = unmarshal_object(empty_swagger_spec, content_spec, value)
+    assert result == {'x': None}

tests/unmarshal/unmarshal_primitive_test.py

@@ -72,3 +72,14 @@ def test_ref(minimal_swagger_dict):
     special_integer_spec = {'$ref': '#/definitions/SpecialInteger'}
     swagger_spec = Spec.from_dict(minimal_swagger_dict)
     assert 10 == unmarshal_primitive(swagger_spec, special_integer_spec, 10)
+
+
+@pytest.mark.parametrize(['nullable', 'value'],
+                         [(False, 'x'), (True, 'x'), (True, None)])
+def test_nullable(minimal_swagger_spec, value, nullable):
+    string_spec = {
+        'type': 'string',
+        'x-nullable': nullable,
+    }
+    result = unmarshal_primitive(minimal_swagger_spec, string_spec, value)
+    assert value == result

tests/validate/validate_object_test.py

@@ -168,3 +168,93 @@ def test_recursive_ref_depth_n_failure(recursive_swagger_spec):
             {'$ref': '#/definitions/Node'},
             value)
     assert "'name' is a required property" in str(excinfo.value)
+
+
+# x-nullable validation
+# ---------------------
+# If the value is an object, validation should pass if
+# `x-nullable` is `True` and the value is `None`. `required` doesn't
+# have an influence.
+#
+# +---------------------+-------------------------+--------------------------+
+# |                     | required == False       | required == True         |
+# +---------------------+-------------------------+--------------------------+
+# | x-nullable == False | {}          -> pass (1) | {}          -> fail  (4) |
+# |                     | {'x': 'y'}  -> pass (2) | {'x': 'y'}  -> pass  (5) |
+# |                     | {'x': None} -> fail (3) | {'x': None} -> fail  (6) |
+# +---------------------+-------------------------+--------------------------+
+# | x-nullable == True  | {}          -> pass (7) | {}          -> fail (10) |
+# |                     | {'x': 'y'}  -> pass (8) | {'x': 'y'}  -> pass (11) |
+# |                     | {'x': None} -> pass (9) | {'x': None} -> pass (12) |
+# +---------------------+-------------------------+--------------------------+
+
+
+def content_spec_factory(required, nullable):
+    return {
+        'type': 'object',
+        'required': ['x'] if required else [],
+        'properties': {
+            'x': {
+                'type': 'string',
+                'x-nullable': nullable,
+            }
+        }
+    }
+
+
+@pytest.mark.parametrize('nullable', [True, False])
+@pytest.mark.parametrize('required', [True, False])
+def test_nullable_with_value(empty_swagger_spec, nullable, required):
+    """With a value set, validation should always pass: (2), (5), (8), (11)"""
+    content_spec = content_spec_factory(required, nullable)
+    value = {'x': 'y'}
+
+    validate_object(empty_swagger_spec, content_spec, value)
+
+
+@pytest.mark.parametrize('nullable', [True, False])
+def test_nullable_required_no_value(empty_swagger_spec, nullable):
+    """When the value is required but not set at all, validation
+    should fail: (4), (10)
+    """
+    content_spec = content_spec_factory(True, nullable)
+    value = {}
+
+    with pytest.raises(ValidationError) as excinfo:
+        validate_object(empty_swagger_spec, content_spec, value)
+    assert excinfo.value.message == "'x' is a required property"
+
+
+@pytest.mark.parametrize('nullable', [True, False])
+def test_nullable_no_value(empty_swagger_spec, nullable):
+    """When the value is not required and not set at all, validation
+    should pass: (1), (7)
+    """
+    content_spec = content_spec_factory(False, nullable=nullable)
+    value = {}
+
+    validate_object(empty_swagger_spec, content_spec, value)
+
+
+@pytest.mark.parametrize('required', [True, False])
+def test_nullable_false_value_none(empty_swagger_spec, required):
+    """When nullable is `False` and the value is set to `None`, validation
+    should fail: (3), (6)
+    """
+    content_spec = content_spec_factory(required, False)
+    value = {'x': None}
+
+    with pytest.raises(ValidationError) as excinfo:
+        validate_object(empty_swagger_spec, content_spec, value)
+    assert excinfo.value.message == "None is not of type 'string'"
+
+
+@pytest.mark.parametrize('required', [True, False])
+def test_nullable_none_value(empty_swagger_spec, required):
+    """When nullable is `True` and the value is set to `None`, validation
+    should pass: (9), (12)
+    """
+    content_spec = content_spec_factory(required, True)
+    value = {'x': None}
+
+    validate_object(empty_swagger_spec, content_spec, value)