Skip to content

ENG-3828: Fix Pydantic serializer warning for AnyHttpUrlStringRemovesSlash#8193

Merged
erosselli merged 3 commits into
mainfrom
ENG-fix-anyhttpurl-serializer
May 14, 2026
Merged

ENG-3828: Fix Pydantic serializer warning for AnyHttpUrlStringRemovesSlash#8193
erosselli merged 3 commits into
mainfrom
ENG-fix-anyhttpurl-serializer

Conversation

@mikeGarifullin
Copy link
Copy Markdown
Contributor

@mikeGarifullin mikeGarifullin commented May 14, 2026

Ticket https://ethyca.atlassian.net/browse/ENG-3828

Description Of Changes

AnyHttpUrlStringRemovesSlash was declared as Annotated[AnyHttpUrl, AfterValidator(...)], but the AfterValidator returns a plain str. At serialization time Pydantic still expected an AnyHttpUrl instance and emitted a PydanticSerializationUnexpectedValue warning whenever the field held a real value.

Concretely surfaced in the celery task Saving record(s) of current preferences and historical preferences for consent reporting after ENG-2488 switched url_recorded from SafeStr to AnyHttpUrlStringRemovesSlash:

UserWarning: Pydantic serializer warnings:
  PydanticSerializationUnexpectedValue(Expected `<class 'pydantic.networks.AnyHttpUrl'>` but got `<class 'str'>` with value `'https://suscripciones.condenastamericas.com'` - serialized value may not be as expected.)

Attach a PlainSerializer(return_type=str) so the serializer type matches what the validator actually produces. No behavioral change to the output value (still str with trailing slash stripped), but the warning is gone and the field now round-trips cleanly.

Code Changes

  • src/fides/api/custom_types.py — added PlainSerializer to AnyHttpUrlStringRemovesSlash.
  • tests/lib/test_custom_types.py — new TestAnyHttpUrlStringRemovesSlash covering validation, trailing-slash stripping, invalid-URL rejection, None passthrough, and a regression test that promotes the serializer warning to an error.

Steps to Confirm

  1. Run nox -s "pytest(lib)" (or docker exec fides /opt/fides/bin/python -m pytest tests/lib/test_custom_types.py) — all 208 tests pass, including the 11 new ones.
  2. Verify regression coverage: revert only the custom_types.py change and re-run TestAnyHttpUrlStringRemovesSlash::test_serialization_does_not_warn — it fails with the exact PydanticSerializationUnexpectedValue warning quoted above.
  3. In a Fidesplus environment, trigger the consent-reporting celery task with a non-null url_recorded — the warning no longer appears in worker logs.

Pre-Merge Checklist

  • All CI pipelines succeeded
  • CHANGELOG.md updated (follow-up commit on this PR)
  • No migrations
  • No documentation updates required

The annotation declared `AnyHttpUrl` but `AfterValidator` returned a
`str`, causing `PydanticSerializationUnexpectedValue` warnings whenever
fields of this type were serialized with a populated value. Most call
sites were unaffected because their values were typically `None`, but
`url_recorded` on the privacy-preference celery payload (set from the
HTTP referer header) consistently triggered the warning.

Attach a `PlainSerializer` so the serializer expects `str` to match what
the validator actually produces.
@mikeGarifullin mikeGarifullin requested a review from a team as a code owner May 14, 2026 21:11
@mikeGarifullin mikeGarifullin requested review from nreyes-dev and removed request for a team May 14, 2026 21:11
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
fides-plus-nightly Ignored Ignored Preview May 14, 2026 9:12pm
fides-privacy-center Ignored Ignored May 14, 2026 9:12pm

Request Review

@mikeGarifullin mikeGarifullin changed the title Fix Pydantic serializer warning for AnyHttpUrlStringRemovesSlash EGN-2488: Fix Pydantic serializer warning for AnyHttpUrlStringRemovesSlash May 14, 2026
@mikeGarifullin mikeGarifullin changed the title EGN-2488: Fix Pydantic serializer warning for AnyHttpUrlStringRemovesSlash ENG-3828: Fix Pydantic serializer warning for AnyHttpUrlStringRemovesSlash May 14, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.59%. Comparing base (b3374c0) to head (c451054).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8193   +/-   ##
=======================================
  Coverage   85.59%   85.59%           
=======================================
  Files         658      658           
  Lines       42847    42847           
  Branches     5018     5018           
=======================================
+ Hits        36675    36676    +1     
  Misses       5064     5064           
+ Partials     1108     1107    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@erosselli erosselli added this pull request to the merge queue May 14, 2026
Merged via the queue into main with commit 2c980f8 May 14, 2026
69 checks passed
@erosselli erosselli deleted the ENG-fix-anyhttpurl-serializer branch May 14, 2026 21:34
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.

2 participants