Skip to content

Do not modify json_schema_extra when building JSON Schemas - #155

Open
hygorcluz wants to merge 1 commit into
scrapy:masterfrom
hygorcluz:json-schema-extra-deepcopy
Open

hygorcluz wants to merge 1 commit into
scrapy:masterfrom
hygorcluz:json-schema-extra-deepcopy

Conversation

@hygorcluz

Copy link
Copy Markdown
Contributor

get_json_schema() made shallow copies of json_schema_extra, then filled in nested items and additionalProperties dicts in place, so those writes ended up in the user's metadata. If two fields share one dict, e.g. {"items": {"description": "Tag."}} on a list[str] field and on a list[int] field of another class, the first schema adds "type": "string" to the shared dict and the second schema then reports string items too. This happened for dataclass, attrs, TypedDict, Pydantic and Scrapy items, and in one edge case for a class-level __json_schema_extra__ (a dataclass with no fields whose properties got attribute docstrings written into them).

This switches every copy() of user-provided schema data in _json_schema.py to deepcopy(), and adds tests for a shared field dict on each item type and for the class-level case. The new tests fail without the change. The py, extra, min-extra, docs, mypy, pylint and twinecheck tox envs and pre-commit pass.

@codecov

codecov Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.88%. Comparing base (4983811) to head (33dbea5).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #155   +/-   ##
=======================================
  Coverage   99.88%   99.88%           
=======================================
  Files           6        6           
  Lines         844      844           
  Branches      161      161           
=======================================
  Hits          843      843           
  Partials        1        1           

☔ View full report in Codecov by Harness.
📢 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.

@codspeed

codspeed Bot commented Nov 13, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 24 untouched benchmarks
⏩ 4 skipped benchmarks1


Comparing hygorcluz:json-schema-extra-deepcopy (33dbea5) with master (4983811)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@hygorcluz

Copy link
Copy Markdown
Contributor Author

The Test: py311, macos-latest job was cancelled because no macOS runner picked it up, unrelated to this change. Could a maintainer re-run it?

This branch has not been deployed

No deployments
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.

1 participant