Skip to content

fix(json): preserve empty property names during serialization - #741

Merged
Vincent Biret (baywet) merged 3 commits into
microsoft:mainfrom
Shubham-Padkonde:fix/json-empty-property-names
Sep 21, 2026
Merged

Vincent Biret (baywet) merged 3 commits into
microsoft:mainfrom
Shubham-Padkonde:fix/json-empty-property-names

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown
Contributor

Overview

Preserve empty-string JSON property names instead of treating them as root values. For example, additional data {"properties": {"": {}}} currently serializes as {"properties": {}}. The writer now distinguishes None (a root value) from "" (a property name), consistently across the JSON writer methods.

Related Issue

Fixes #746

Testing Instructions

From packages/serialization/json, install requirements-dev.txt and run pytest.

  • 168 JSON package tests pass on Windows, Python 3.12.
  • The new tests cover empty and ordinary property names, root values, nested additional data (including null, false, zero, empty objects and lists), enums and model collections. Against the unchanged source, 22 cases fail and 26 controls pass; all 48 pass with the fix.
  • Mypy passes for all 6 package source files; YAPF and isort checks pass.
  • Pylint exits successfully with a 10.00/10 score, but reports an existing unrecognized suggestion-mode setting in .pylintrc.

Notes

AI-assisted implementation and validation. The full multi-package/Python-version CI matrix and a generated client against a live API were not run locally. Tests exercise the serializer directly, including the nested payload reported in the issue.

@baywet Vincent Biret (baywet) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is failing due to unit tests failing. Please have a look.

@github-project-automation github-project-automation Bot moved this to In Progress 🚧 in Kiota Sep 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Root falsy values are still dropped during serialization.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

This PR preserves empty-string JSON property names during serialization and adds regression coverage. A critical issue remains: falsy root values can still serialize incorrectly.

Changes:

  • Distinguishes None keys from empty-string property names.
  • Adds tests for nested data, primitives, enums, and model collections.
File Summary
packages/​serialization/​json/​tests/​unit/​test_empty_property_names.py Adds regression tests for empty property names.
packages/​serialization/​json/​kiota_serialization_json/​json_serialization_writer.py Updates serialization handling for empty property names.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Shubham-Padkonde

Copy link
Copy Markdown
Contributor Author

The multipart CI failures were caused by an internal caller passing an empty string for a JSON root value. With this PR, an empty string correctly means an empty property name, so that caller produced an unwanted wrapper.

Fixed in 79455e0 by passing None from MultipartBody._write_parsable. The three existing multipart regressions fail before this follow-up and pass after it. On Windows/Python 3.13: multipart 13 passed, JSON 168 passed, abstractions 141 passed; abstractions mypy passes all 48 source files, and the changed file passes YAPF/isort checks. The hosted multi-version matrix still needs to rerun.

Prepared and validated with Codex assistance.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The only remaining feedback is a non-blocking test-coverage nit.

Review effort: Lite
Findings: None

Resolved since last review (1)

@sonarqubecloud

Copy link
Copy Markdown

@baywet
Vincent Biret (baywet) merged commit 6071049 into microsoft:main Sep 21, 2026
53 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress 🚧 to Done ✔️ in Kiota Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done ✔️

Development

Successfully merging this pull request may close these issues.

Empty-string JSON property names are dropped during request serialization

3 participants