Repository navigation
Complete formatter serialization cleanup (#3596) - #3601
Merged
Merged
Conversation
Remove the obsolete BinaryFormatter round-trip test and its legacy and .NET Core project references. The formatter constructors, GetObjectData overrides, Serializable attributes, warning suppressions, and public API entries were already removed from dev-9.x by #3334. Remove the remaining NonSerialized annotations from response fields now that their containing exception types no longer support formatter serialization.
There was a problem hiding this comment.
Pull request overview
This PR completes the cleanup of legacy formatter-based (e.g., BinaryFormatter) serialization remnants in the OData Client test suite and exception types, aligning with the removal of serialization constructors/GetObjectData/[Serializable] work previously done in #3334 and the goals in issue #3596.
Changes:
- Removes the obsolete
BinaryFormatterround-trip unit test forDataServiceClientException. - Cleans up test project
.csprojreferences that previously included/excluded that test file. - Removes now-inert
[NonSerialized]annotations from response fields in client exception types that no longer support formatter serialization.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/FunctionalTests/Tests/DataServices/UnitTests/Client.TDD.Tests/Tests/DataServiceClientExceptionSerializationTests.cs | Deletes the obsolete BinaryFormatter-based serialization test. |
| test/FunctionalTests/Tests/DataServices/UnitTests/Client.TDD.Tests/Microsoft.OData.Client.TDDUnitTests.NetCore.csproj | Removes now-unneeded compile-removal entry for the deleted test file. |
| test/FunctionalTests/Tests/DataServices/UnitTests/Client.TDD.Tests/Microsoft.OData.Client.TDDUnitTests.csproj | Removes compile include for the deleted test file to avoid build break. |
| src/Microsoft.OData.Client/DataServiceRequestException.cs | Removes inert [NonSerialized] from the response field. |
| src/Microsoft.OData.Client/DataServiceQueryException.cs | Removes inert [NonSerialized] from the response field. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Author
|
/AzurePipelines run |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove the obsolete BinaryFormatter round-trip test and its legacy and .NET Core project references. The formatter constructors, GetObjectData overrides, Serializable attributes, warning suppressions, and public API entries were already removed from dev-9.x by #3334.
Remove the remaining NonSerialized annotations from response fields now that their containing exception types no longer support formatter serialization.
The reason to remove [NonSerialized] is cleanup and accuracy, not because the attribute itself is unsafe:
• [NonSerialized] tells formatter-based serializers to exclude a field from serialization.
• These exception classes are no longer [Serializable] and no longer expose serialization constructors or GetObjectData .
• Therefore, the response fields cannot participate in formatter serialization, making [NonSerialized] functionally inert.
• Leaving it suggests that formatter serialization is still supported and that the field requires special serialization treatment.
• Removing it completes the cleanup described in #3596 without changing runtime behavior or public APIs.
References:
• Issue: #3596
• [NonSerialized] documentation: https://learn.microsoft.com/dotnet/api/system.nonserializedattribute
• Microsoft BinaryFormatter guidance: https://learn.microsoft.com/dotnet/standard/serialization/binaryformatter-security-guide
Issues
This pull request fixes #xxx.
Description
Briefly describe the changes of this pull request.
Checklist (Uncheck if it is not completed)
Additional work necessary
If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.
Repository notes
Team members can start a CI build by adding a comment with the text
/AzurePipelines runto a PR. A bot may respond indicating that there is no pipeline associated with the pull request. This can be ignored if the build is triggered.Team members should not trigger a build this way for pull requests coming from forked repositories. They should instead trigger the build manually by setting the "branch" to
refs/pull/{prId}/mergewhere{prId}is the ID of the PR.