Fix: stop emitting null format/pattern for DateTime fields (#938)

AUTHORS.rst

@@ -91,3 +91,4 @@ Contributors (chronological)
 - Felix Claessen `@Flix6x <https://github.com/Flix6x>`_
 - Karthik Ramadugu `@karthiksai109 <https://github.com/karthiksai109>`_
 - Amir Kahriman `@kingdomOfIT <https://github.com/kingdomOfIT>`_
+- Jean-Baptiste Braun `@jbbqqf <https://github.com/jbbqqf>`_

CHANGELOG.rst

@@ -4,6 +4,14 @@ Changelog
 unreleased
 **********
 
+Bug fixes:
+
+- ``MarshmallowPlugin``: stop emitting ``"format": null`` and ``"pattern": null``
+  for ``DateTime`` fields declared with the ``"rfc"``/``"rfc822"`` format or a
+  custom ``strftime`` format string. The OpenAPI 3 schema rejects null values
+  for these keywords, so the resulting document failed
+  ``openapi-spec-validator`` (:issue:`938`).
+
 Other changes:
 
 - Drop support for marshmallow 3, which is EOL.

src/apispec/ext/marshmallow/field_converter.py

@@ -566,52 +566,63 @@ def enum2properties(self, field, **kwargs: typing.Any) -> dict:
                 ret["enum"].append(None)
         return ret
 
-    def datetime2properties(self, field, **kwargs: typing.Any) -> dict:
+    def datetime2properties(self, field, ret=None, **kwargs: typing.Any) -> dict:
         """Return a dictionary of properties from :class:`DateTime <marshmallow.fields.DateTime` fields.
 
+        For non-ISO formats the OpenAPI ``format`` keyword from the default
+        field mapping (``date-time`` for DateTime fields) does not apply, so
+        we drop it from the accumulator ``ret`` rather than emitting an
+        explicit ``"format": null``: the OpenAPI 3 schema rejects null values
+        for ``format`` and ``pattern`` (#938).
+
         :param Field field: A marshmallow field.
         :rtype: dict
         """
-        ret = {}
+        attributes: dict = {}
         if isinstance(field, marshmallow.fields.DateTime):
             if field.format in ("iso", "iso8601") or field.format is None:
                 # Will return { "type": "string", "format": "date-time" }
                 # as specified inside DEFAULT_FIELD_MAPPING
                 pass
             elif field.format in ("rfc", "rfc822"):
-                ret = {
+                # rfc822 strings do not match the OpenAPI ``date-time`` format
+                # (RFC 3339 / ISO 8601); drop the inherited format and describe
+                # the value with an example and pattern instead.
+                if ret is not None:
+                    ret.pop("format", None)
+                attributes = {
                     "type": "string",
-                    "format": None,
                     "example": "Wed, 02 Oct 2002 13:00:00 GMT",
                     "pattern": r"((Mon|Tue|Wed|Thu|Fri|Sat|Sun), ){0,1}\d{2} "
                     + r"(Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec) \d{4} \d{2}:\d{2}:\d{2} "
                     + r"(UT|GMT|EST|EDT|CST|CDT|MST|MDT|PST|PDT|(Z|A|M|N)|(\+|-)\d{4})",
                 }
             elif field.format == "timestamp":
-                ret = {
+                attributes = {
                     "type": "number",
                     "format": "float",
                     "example": "1676451245.596",
                     "min": "0",
                 }
             elif field.format == "timestamp_ms":
-                ret = {
+                attributes = {
                     "type": "number",
                     "format": "float",
                     "example": "1676451277514.654",
                     "min": "0",
                 }
             else:
-                ret = {
-                    "type": "string",
-                    "format": None,
-                    "pattern": (
-                        field.metadata["pattern"]
-                        if field.metadata.get("pattern")
-                        else None
-                    ),
-                }
-        return ret
+                # Custom strftime format string: there is no standard OpenAPI
+                # ``format`` value for an arbitrary user pattern, so drop the
+                # inherited ``date-time`` and only emit ``pattern`` when the
+                # user supplied one in metadata.
+                if ret is not None:
+                    ret.pop("format", None)
+                attributes = {"type": "string"}
+                pattern = field.metadata.get("pattern")
+                if pattern:
+                    attributes["pattern"] = pattern
+        return attributes
 
 
 def make_type_list(types):

tests/test_ext_marshmallow_field.py

@@ -564,7 +564,6 @@ def test_datetime2property_rfc(spec_fixture):
     res = spec_fixture.openapi.field2property(field)
     assert res == {
         "type": "string",
-        "format": None,
         "example": "Wed, 02 Oct 2002 13:00:00 GMT",
         "pattern": r"((Mon|Tue|Wed|Thu|Fri|Sat|Sun), ){0,1}\d{2} "
         + r"(Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec) \d{4} \d{2}:\d{2}:\d{2} "
@@ -577,7 +576,6 @@ def test_datetime2property_rfc822(spec_fixture):
     res = spec_fixture.openapi.field2property(field)
     assert res == {
         "type": "string",
-        "format": None,
         "example": "Wed, 02 Oct 2002 13:00:00 GMT",
         "pattern": r"((Mon|Tue|Wed|Thu|Fri|Sat|Sun), ){0,1}\d{2} "
         + r"(Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec) \d{4} \d{2}:\d{2}:\d{2} "
@@ -617,7 +615,6 @@ def test_datetime2property_custom_format(spec_fixture):
     res = spec_fixture.openapi.field2property(field)
     assert res == {
         "type": "string",
-        "format": None,
         "pattern": r"^((?:(\d{4}-\d{2}-\d{2})T(\d{2}:\d{2}:\d{2}(?:\.\d+)?))(Z|[\+-]\d{2}:\d{2})?)$",
     }
 
@@ -627,11 +624,51 @@ def test_datetime2property_custom_format_missing_regex(spec_fixture):
     res = spec_fixture.openapi.field2property(field)
     assert res == {
         "type": "string",
-        "format": None,
-        "pattern": None,
     }
 
 
+def test_datetime2property_emits_valid_openapi_spec(spec_fixture):
+    """Regression test for #938: DateTime fields with non-iso/non-default
+    formats used to emit ``"format": null`` and/or ``"pattern": null`` in the
+    rendered OpenAPI document, which is rejected by the OpenAPI 3.0 schema
+    (both keywords MUST be strings when present). The full spec is now
+    validated to make sure no null-valued keywords leak through.
+    """
+    from apispec import APISpec
+    from apispec.ext.marshmallow import MarshmallowPlugin
+
+    from .utils import validate_spec
+
+    class S(Schema):
+        rfc = fields.DateTime(format="rfc")
+        custom_no_pattern = fields.DateTime(format="%Y-%m-%dT%H:%M:%S")
+        custom_with_pattern = fields.DateTime(
+            format="%Y-%m-%dT%H:%M:%S",
+            metadata={"pattern": r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}$"},
+        )
+
+    spec = APISpec(
+        title="t",
+        version="v",
+        openapi_version="3.0.2",
+        plugins=[MarshmallowPlugin()],
+    )
+    spec.components.schema("S", schema=S)
+    # Round-trip through to_dict so any null leakage is captured. The
+    # validation call below would raise OpenAPIError("...is not valid under
+    # any of the given schemas...") if `format` or `pattern` were emitted
+    # as null on any of the three fields.
+    props = spec.to_dict()["components"]["schemas"]["S"]["properties"]
+    for name, prop in props.items():
+        assert prop.get("format") is not None or "format" not in prop, (
+            f"{name} emits null format"
+        )
+        assert prop.get("pattern") is not None or "pattern" not in prop, (
+            f"{name} emits null pattern"
+        )
+    validate_spec(spec)
+
+
 class TestField2PropertyPluck:
     @pytest.fixture(autouse=True)
     def _setup(self, spec_fixture):