Merge pull request #735 from cbcoutinho/fix/pydantic-model-drift-728-704

fix(models): coerce Contact.birthday + relax Table.owner_display_name (fixes #728, #704)
This commit is contained in:
Chris Coutinho
2026-04-27 00:45:24 +02:00
committed by GitHub
3 changed files with 98 additions and 3 deletions
+17 -1
View File
@@ -1,8 +1,9 @@
"""Pydantic models for Contacts app responses."""
from datetime import date, datetime
from typing import Any, Dict, List, Optional
from pydantic import BaseModel, Field
from pydantic import BaseModel, Field, field_validator
from .base import BaseResponse, StatusResponse
@@ -57,6 +58,21 @@ class Contact(BaseModel):
)
etag: Optional[str] = Field(None, description="ETag for versioning")
@field_validator("birthday", mode="before")
@classmethod
def _coerce_birthday(cls, value: Any) -> Any:
"""Accept ``date``/``datetime`` from vobject and serialise to ISO string.
vobject parses BDAY into ``datetime.date`` (or ``datetime.datetime``);
the Pydantic model stores it as ``str``. Without this coercion any
contact with a populated BDAY blew up the entire ``list_contacts``
response (#704, #672). Strings and ``None`` pass through unchanged so
callers can still construct contacts with raw ISO input.
"""
if isinstance(value, (date, datetime)):
return value.isoformat()
return value
@property
def primary_email(self) -> Optional[str]:
"""Get the primary email address."""
+6 -1
View File
@@ -72,7 +72,12 @@ class Table(BaseModel):
title: str = Field(description="Table title")
emoji: Optional[str] = Field(None, description="Table emoji")
ownership: str = Field(description="Table ownership")
owner_display_name: str = Field(description="Display name of table owner")
# Tables app v2.0.1 stopped emitting owner_display_name at the top level
# (still present inside views via get_schema). Optional avoids a 100% failure
# rate on list_tables — see #728.
owner_display_name: Optional[str] = Field(
None, description="Display name of table owner"
)
created_by: Optional[str] = Field(None, description="User who created the table")
created_at: Optional[str] = Field(None, description="Table creation timestamp")
last_edit_by: Optional[str] = Field(
+75 -1
View File
@@ -1,6 +1,6 @@
"""Unit tests for Pydantic response models."""
from datetime import date
from datetime import date, datetime
import pytest
@@ -18,6 +18,7 @@ from nextcloud_mcp_server.models.semantic import (
SamplingSearchResponse,
SemanticSearchResult,
)
from nextcloud_mcp_server.models.tables import Table
from nextcloud_mcp_server.server.calendar import _event_dict_to_summary
from nextcloud_mcp_server.server.contacts import _raw_contact_to_model
@@ -677,3 +678,76 @@ def test_event_dict_to_summary_calendar_name_without_display_name():
assert summary.calendar_name == "personal"
assert summary.calendar_display_name == "personal"
# ----------------------------------------------------------------------------
# Direct Contact() construction with date-typed birthday — pins #704 / #672
# ----------------------------------------------------------------------------
# These bypass _raw_contact_to_model and hit the Pydantic validator directly,
# so any future code path that constructs Contact from raw vobject output is
# covered without depending on the upstream coercion in server/contacts.py.
@pytest.mark.unit
def test_contact_model_coerces_date_birthday_to_iso():
"""Regression for #704: a datetime.date birthday must coerce to ISO str
rather than raise a Pydantic ValidationError.
"""
contact = Contact(uid="c1", fn="Date BDay", birthday=date(2000, 1, 1))
assert contact.birthday == "2000-01-01"
@pytest.mark.unit
def test_contact_model_coerces_datetime_birthday_to_iso():
"""A datetime.datetime input is also valid in vobject output and must coerce."""
contact = Contact(
uid="c2", fn="DateTime BDay", birthday=datetime(2000, 1, 1, 12, 30)
)
assert contact.birthday == "2000-01-01T12:30:00"
@pytest.mark.unit
def test_contact_model_string_birthday_passes_through():
"""ISO strings must round-trip unchanged — the validator should be a no-op."""
contact = Contact(uid="c3", fn="String BDay", birthday="1990-05-15")
assert contact.birthday == "1990-05-15"
@pytest.mark.unit
def test_contact_model_none_birthday_is_allowed():
contact = Contact(uid="c4", fn="No BDay")
assert contact.birthday is None
# ----------------------------------------------------------------------------
# Table model parses without owner_display_name — pins #728
# ----------------------------------------------------------------------------
@pytest.mark.unit
def test_table_parses_without_owner_display_name():
"""Tables app v2.0.1 dropped owner_display_name from the top-level payload.
Parsing must succeed (#728) — anything else 100%-fails nc_tables_list_tables.
"""
raw = {
"id": 1,
"title": "Welcome to Nextcloud Tables!",
"ownership": "alice",
# owner_display_name intentionally absent
}
table = Table(**raw)
assert table.id == 1
assert table.owner_display_name is None
@pytest.mark.unit
def test_table_parses_with_owner_display_name():
"""When the field is present we still capture it — Optional doesn't drop data."""
raw = {
"id": 2,
"title": "Old API Table",
"ownership": "bob",
"owner_display_name": "Bob the Builder",
}
table = Table(**raw)
assert table.owner_display_name == "Bob the Builder"