Skip to content

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

Open
jbbqqf wants to merge 1 commit into
marshmallow-code:devfrom
jbbqqf:fix/938-datetime-invalid-format-null
Open

Fix: stop emitting null format/pattern for DateTime fields (#938)#1048
jbbqqf wants to merge 1 commit into
marshmallow-code:devfrom
jbbqqf:fix/938-datetime-invalid-format-null

Conversation

@jbbqqf
Copy link
Copy Markdown

@jbbqqf jbbqqf commented May 22, 2026

Summary

MarshmallowPlugin currently emits "format": null (and, in the custom-strftime case, "pattern": null) for DateTime fields that use any non-iso format. The OpenAPI 3 schema requires those keywords to be strings when present, so the rendered document does not validate against openapi-spec-validator.

This PR makes the datetime2properties attribute function drop the inherited date-time value from the accumulator rather than overwriting it with null, and only emits "pattern" when the user actually supplied a metadata pattern.

Fixes #938.

Context

PR #815 (released in v6.4) introduced datetime2properties and explicitly returned "format": None to "override" the default ("string", "date-time") mapping from DEFAULT_FIELD_MAPPING. That answers the question @lafrech raised on the abandoned PR #937 ("any idea why it stopped working as expected in version 6.4?"): the regression is in 1ac6f55 itself. The same PR is also the source of the "pattern": None for custom formats.

The OpenAPI 3.0 schema (and openapi-spec-validator) rejects both of those keys when their value is null:

> {'type': 'string', 'format': None, 'pattern': None} is not valid under any of the given schemas

A previous attempt (#937) tried to give every non-iso format its own canonical format value. That triggered a long discussion that never converged. The minimal change in this PR avoids that debate entirely: it just stops emitting the invalid null keys, which is what reviewer @bhperry already pointed out in #937 (comment).

Changes

  • src/apispec/ext/marshmallow/field_converter.py: datetime2properties now receives the accumulator dict ret and calls ret.pop("format", None) for the rfc/rfc822 and custom-format branches. For custom formats, pattern is only added when present in field.metadata. The function still returns the per-format example/pattern/min so existing behavior is otherwise preserved.
  • tests/test_ext_marshmallow_field.py:
    • Updated test_datetime2property_rfc, test_datetime2property_rfc822, test_datetime2property_custom_format, and test_datetime2property_custom_format_missing_regex to assert the absence of the keys instead of None values.
    • Added test_datetime2property_emits_valid_openapi_spec, a regression test that builds a spec with all three previously-broken cases, asserts no key is set to None, and runs the full validate_spec() round-trip.
  • CHANGELOG.rst: added a bug-fix entry under unreleased.
  • AUTHORS.rst: added myself.

Reproduce BEFORE/AFTER yourself (copy-paste)

# --- one-time setup ---
rm -rf /tmp/apispec-938-repro
git clone https://github.com/marshmallow-code/apispec.git /tmp/apispec-938-repro
cd /tmp/apispec-938-repro
uv sync --all-extras --group tests >/dev/null 2>&1
REPRO='
import sys; sys.path.insert(0, "tests")
from apispec import APISpec
from apispec.ext.marshmallow import MarshmallowPlugin
from marshmallow import Schema, fields
from utils import validate_spec

class S(Schema):
    rfc = fields.DateTime(format="rfc")
    custom = fields.DateTime(format="%Y-%m-%dT%H:%M:%S")

spec = APISpec(title="t", version="v", openapi_version="3.0.2",
               plugins=[MarshmallowPlugin()])
spec.components.schema("S", schema=S)

props = spec.to_dict()["components"]["schemas"]["S"]["properties"]
print("rfc.format    =", repr(props["rfc"].get("format")))
print("custom.format =", repr(props["custom"].get("format")))
print("custom.pattern=", repr(props["custom"].get("pattern")))
try:
    validate_spec(spec)
    print("spec validation: PASS")
except Exception as e:
    print("spec validation: FAIL ->", str(e)[:80])
'

# --- BEFORE: origin/dev, expect null keys and validation failure ---
git checkout origin/dev
uv run python -c "$REPRO"
# Expected:
#   rfc.format    = None
#   custom.format = None
#   custom.pattern= None
#   spec validation: FAIL -> ... is not valid under any of the given schemas ...

# --- AFTER: this PR branch, expect no null keys and validation pass ---
git fetch https://github.com/jbbqqf/apispec.git fix/938-datetime-invalid-format-null
git checkout FETCH_HEAD
uv run python -c "$REPRO"
# Expected:
#   rfc.format    = None        # absent from the dict, .get() returns None
#   custom.format = None        # absent from the dict, .get() returns None
#   custom.pattern= None        # absent from the dict, .get() returns None
#   spec validation: PASS

Note the BEFORE prints format: None because the key is literally None in the dict (and validation fails); AFTER prints None because .get() falls through (the keys are not in the dict, and validation passes). The wider repro is in the new regression test, which differentiates the two cases properly.

What I ran locally

$ uv run pytest tests/test_ext_marshmallow_field.py -v -k datetime
...
26 passed, 161 deselected in 0.15s

$ uv run pytest
...
620 passed, 6 skipped in 0.49s

$ uvx ruff check src/apispec/ext/marshmallow/field_converter.py tests/test_ext_marshmallow_field.py
All checks passed!

$ uvx ruff format --check src/apispec/ext/marshmallow/field_converter.py tests/test_ext_marshmallow_field.py
2 files already formatted

I also verified that the new regression test (test_datetime2property_emits_valid_openapi_spec) fails on origin/dev when only the source file is reverted:

$ git stash push -- src/apispec/ext/marshmallow/field_converter.py
$ uv run pytest tests/test_ext_marshmallow_field.py::test_datetime2property_emits_valid_openapi_spec -v
...
FAILED tests/test_ext_marshmallow_field.py::test_datetime2property_emits_valid_openapi_spec[2.0]
FAILED tests/test_ext_marshmallow_field.py::test_datetime2property_emits_valid_openapi_spec[3.0.0]
E    AssertionError: rfc emits null format
$ git stash pop

Edge cases

# Scenario Field declaration Before After
1 ISO 8601 (default) fields.DateTime() {type: string, format: date-time} unchanged
2 Explicit iso/iso8601 fields.DateTime(format="iso") {type: string, format: date-time} unchanged
3 rfc / rfc822 fields.DateTime(format="rfc") {type: string, format: null, example, pattern} {type: string, example, pattern}
4 timestamp fields.DateTime(format="timestamp") {type: number, format: float, example, min} unchanged
5 timestamp_ms fields.DateTime(format="timestamp_ms") {type: number, format: float, example, min} unchanged
6 Custom strftime, with metadata pattern fields.DateTime(format=..., metadata={"pattern": ...}) {type: string, format: null, pattern: <user>} {type: string, pattern: <user>}
7 Custom strftime, no metadata pattern fields.DateTime(format=...) {type: string, format: null, pattern: null} {type: string}

Cases 1, 2, 4, and 5 already produced valid OpenAPI and are untouched. Cases 3, 6, and 7 are the ones the issue is about and are now schema-valid.

Risk / blast radius

The change is local to datetime2properties (~20 lines). The function signature gains a default-valued ret keyword (kept default-None) and remains callable as datetime2properties(field), so any user code that calls it directly is unaffected. Custom attribute functions that previously inspected the returned dict for format/pattern set to None would need to update — but as far as I can tell, None was never a useful sentinel for downstream code and the original intent was clearly "this isn't a real format". Date fields are unaffected because the function explicitly only triggers on marshmallow.fields.DateTime (and Date is not a DateTime subclass in marshmallow 3+).

Release note

MarshmallowPlugin: stop emitting "format": null and "pattern": null for DateTime fields with "rfc"/"rfc822" or a custom strftime format; the resulting OpenAPI 3 document now validates against openapi-spec-validator (#938).

PR drafted with assistance from Claude Code (Anthropic). The change was reviewed manually against apispec's source — including the original commit 1ac6f55 (PR #815) that introduced the null-emitting branches in v6.4 and the discussion on the abandoned PR #937 — and the reproducer above is the one I used during development; reviewers can paste it verbatim.

…ow-code#938)

DateTime fields declared with the "rfc"/"rfc822" format or a custom
strftime format string were rendered with `"format": null` and, for
custom formats without a metadata pattern, `"pattern": null`. The
OpenAPI 3 schema requires those keywords to be strings when present, so
the resulting spec failed openapi-spec-validator.

Pop the inherited `date-time` format from the accumulator dict for the
non-iso branches and only emit `pattern` when the user supplied one via
metadata. A regression test exercises the three affected combinations
and runs the full spec through `validate_spec` so any future null
leakage is caught immediately.

Fixes marshmallow-code#938.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Date and DateTime with format argument not rendered correctly

1 participant