MessagePack serializer for vs-mef types #478
Merged
Conversation
This file contains 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
In this commit, several changes were made to improve code readability and efficiency. In `ImportSatisfiabilityConstraintFormatter.cs`, type checking and casting were simplified using pattern matching. Null-forgiving post-fixes were added in `MessagePackOptionsExtensions.cs` for better null safety. The deserialization approach in `MetadataDictionaryFormatter.cs` was modified. A switch statement in `TypeRefObjectFormatter.cs` was replaced with a switch expression for conciseness. Additionally, some lines were removed and added back without changes in `ComposablePartDefinitionFormatter.cs` and `MemberRefFormatter.cs`, possibly due to formatting changes.
Updated the access modifiers of several classes in the `Microsoft.VisualStudio.Composition` namespace from `internal` to `public`. Removed a block of commented-out code in `TypeRefObjectFormatter.cs` related to a switch statement for handling different counts of `TypeRef`. Cleaned up `AssemblyInfo.cs` by removing several assembly attributes and using directives. Also, removed several `AssemblyAttribute` items from an `ItemGroup` in `Microsoft.VisualStudio.Composition.Tests.csproj` that were making internal types visible to other assemblies.
In `AssembliesLazyLoadedTests.cs` and `CacheResiliencyTests.cs`, removed lines of code that were creating unused variables. This should not affect the functionality. Also removed a line of code in `CacheResiliencyTests.cs` that was assigning a value to `metadataToken` variable, which might affect the functionality if used later in the code. In `ExportMetadataTests.cs`, changed the access modifier of `MetadataEnumNonPublic` enum from `public` to `internal`, restricting its visibility to the current assembly. Also, removed comment lines in `ExportMetadataTests.cs`, `TestUtilities.cs`, and `Mef3DiscoveryTestCaseRunner.cs`.
This commit includes a significant refactoring of namespaces, changing `Microsoft.VisualStudio.Composition` to `Microsoft.VisualStudio.Composition.Formatter` in multiple files. The `using Microsoft.VisualStudio.Composition.Formatter;` directive has been added to several files. The order of `using MessagePack;` and `using MessagePack.Resolvers;` directives has been changed in `CachedCatalog.cs`. The `[MessagePackObject(true)]` attribute has been removed from `ComposablePartDefinition.cs`. The return value of `PartCreationPolicyConstraint.GetRequiredCreationPolicyConstraint(creationPolicy)` has been marked as non-nullable in `ImportSatisfiabilityConstraintFormatter.cs`. Several classes and methods have been updated to handle null values, including `MemberRefFormatter`, `MessagePackOptionsExtensions`, `MessagePackSerializerContext`, `MetadataDictionaryFormatter`, `ParameterRefFormatter`, and others. The `MessagePackSerializerContext` class has been marked as `internal`. The `RuntimeExportFormatter`, `RuntimeImportFormatter`, `RuntimePartFormatter`, `StrongAssemblyIdentityFormatter`, and `TypeRefObjectFormatter` classes have been updated to handle nullable types, and the `Deserialize` and `Serialize` methods in these classes have been updated accordingly. The `#pragma warning disable CS3001` directive has been added to these classes to suppress the CS3001 warning. Some commented out code related to MessagePack unions and formatters has been removed. A new file `GlobalSuppressions.cs` has been added to suppress a specific warning across the entire project. The copyright notice has been updated in multiple files.
This commit includes several changes to the Microsoft.VisualStudio.Composition and Microsoft.VisualStudio.Composition.Tests namespaces. The `AssemblyLoader` and `AssemblyLoaderWrapper` classes have been changed from `internal` to `private`, restricting their visibility. The `MessagePackObject` attribute has been removed from `AssemblyLoaderWrapper` and `StandardAssemblyLoader` classes, disabling their serialization using MessagePack. In `TestUtilities.cs`, the assertion comparing `runtimeComposition` and `deserializedRuntimeComposition` has been removed. Lastly, the code resetting the position of MemoryStream `ms` in `Mef3DiscoveryTestCaseRunner.cs` has been removed, which may affect catalog loading.
AArnott
requested changes
May 21, 2024
src/Microsoft.VisualStudio.Composition/Microsoft.VisualStudio.Composition.csproj
Outdated
Show resolved
Hide resolved
src/Microsoft.VisualStudio.Composition/Microsoft.VisualStudio.Composition.csproj
Outdated
Show resolved
Hide resolved
...VisualStudio.Composition/Configuration/Formatters/ImportSatisfiabilityConstraintFormatter.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.VisualStudio.Composition/Configuration/Formatters/ParameterRefFormatter.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.VisualStudio.Composition/Configuration/Formatters/MessagePackSerializerContext.cs
Outdated
Show resolved
Hide resolved
AArnott
reviewed
May 21, 2024
BertanAygun
reviewed
May 22, 2024
BertanAygun
reviewed
May 22, 2024
BertanAygun
reviewed
May 22, 2024
src/Microsoft.VisualStudio.Composition/Configuration/Formatters/RuntimePartFormatter.cs
Outdated
Show resolved
Hide resolved
- Updated `MessagePackSerializerContext` across `CachedCatalog.cs`, `CachedComposition.cs`, and `MessagePackSerializerContext.cs` to use `StandardResolverAllowPrivate.Instance` for enabling serialization of private members. - Changed `MetadataDictionaryFormatter` visibility from `public` to `internal` in `MetadataDictionaryFormatter.cs` to limit access within the assembly and prevent external misuse. - Added `IgnoreMember` attribute to various properties and fields in `ImportDefinition.cs`, `FieldRef.cs`, `MemberRef.cs`, `MethodRef.cs`, `PropertyRef.cs`, and `RuntimeComposition.cs` to exclude them from serialization, aiming to reduce serialized data size and avoid serialization of non-essential or recalculable members. - Modified `SerializationConstructor` attribute in `FieldRef.cs`, `MethodRef.cs`, and `PropertyRef.cs` to mark constructors as `private`, enhancing encapsulation by restricting their use to the serialization framework. - The addition of `IgnoreMember` to members related to debugging or internal state in several classes indicates a focus on serialization efficiency and reliability by excluding non-critical state information.
The `MessagePackSerializerContext` constructor and the `GetIFormatterResolver` method have been updated to support a more flexible specification of the formatter resolver. This is achieved by allowing an `IFormatterResolver` instance to be passed as a parameter, enabling the use of custom formatter resolvers. Additionally, the internal usage of the resolver in the `GetIFormatterResolver` method has been modified to utilize the passed-in `formatterResolver` instead of the default `StandardResolverAllowPrivate.Instance`, further enhancing the configurability of the serialization process.
Removed unused `using` directives in `CachedCatalog.cs` and `CachedComposition.cs`, streamlining the codebase and potentially improving compile-time performance. This cleanup involved namespaces such as `System`, `System.Collections.Generic`, and others, indicating a significant effort to remove unnecessary dependencies. Both `CachedCatalog.cs` and `CachedComposition.cs` saw the removal of their `SerializationContext` classes, signaling a shift in the serialization strategy, likely towards using a more standardized or efficient method like MessagePack. This change suggests a move away from custom serialization solutions, which could result in improved performance, maintainability, and possibly security of the serialization process. The commit does not detail changes to `SerializationContextBase.cs`, but given the context, it's reasonable to infer that any modifications there would align with the overarching goals of performance improvement, bug fixes, feature enhancements, API changes, security enhancements, or documentation updates to support the new serialization approach. Overall, this commit represents a significant step in simplifying the codebase and enhancing the serialization strategy, which could have wide-ranging impacts on the project's performance and maintainability.
This commit introduces the `MetadataObjectFormatter` class, implementing the `IMessagePackFormatter<object?>` interface to centralize serialization and deserialization logic for a wide range of types, including special cases like `Enum32Substitution`. It significantly refactors the `MetadataDictionaryFormatter`, extracting redundant logic into the new class to adhere to the single responsibility principle. Unused usings and obsolete serialization/deserialization code have been removed from `MetadataDictionaryFormatter.cs` for better readability. Additionally, `MetadataObjectFormatter.Instance` is now utilized in both `MetadataDictionaryFormatter` and `MessagePackSerializerContext` for serialization tasks, ensuring a unified approach to object formatting. The handling of `null` values during serialization and deserialization has been improved. The `MessagePackSerializerContext` class has been updated to include more types in its `DedupingTypes` set and to incorporate `MetadataObjectFormatter.Instance` in its resolver setup, optimizing serialization efficiency and consistency across the application.
Removed the [MessagePackFormatter(typeof(MetadataDictionaryFormatter))] attribute from the Metadata properties in ExportDefinition.cs, ImportDefinition.cs, and RuntimeComposition.cs to standardize serialization behavior. Additionally, introduced a new private field `exportedValueTypeRef` in RuntimeComposition.cs with the [IgnoreMember] attribute for internal type reference handling enhancements.
AArnott
requested changes
Jun 17, 2024
src/Microsoft.VisualStudio.Composition/Configuration/Formatters/MetadataDictionaryFormatter.cs
Outdated
Show resolved
Hide resolved
This commit encompasses a wide range of changes aimed at improving the codebase's organization, readability, maintainability, and performance. Key changes include the reorganization of namespace declarations and using directives across multiple files to enhance code clarity and reduce potential conflicts. Serialization support has been significantly enhanced through the adoption of MessagePack, with several classes now annotated for MessagePack serialization, ensuring efficient serialization/deserialization processes. Additionally, custom formatters have been introduced for certain classes to handle complex types more effectively. Refactoring efforts have also focused on improving code safety, with added null and range checks to ensure robustness. Debugging experiences have been enhanced through the use of the DebuggerDisplay attribute, providing more informative string representations of objects. Furthermore, equality checks and hash code generation have been refined across various classes to ensure accurate comparisons and efficient collection operations. Code documentation has seen improvements, with added summaries and comments to provide better context and understanding of the code's functionality. General code clean-up efforts, such as the removal of unnecessary comments and redundant code, contribute to the overall quality and maintainability of the codebase. Overall, these changes aim to make the codebase more maintainable, efficient, and compatible with modern serialization frameworks, while also enhancing developer experience through better documentation and debugging support.
Added a new package reference for `MessagePack` in the `Microsoft.VisualStudio.Composition.Tests.csproj` file. Introduced `SerializationFormatterTests` class in `SerializationFormatterTests.cs` to validate serialization of various types using `MessagePack`. Implemented multiple test methods for objects like `AssemblyName`, `MetaDataDictionaryFormatter`, `MetaDataObject`, `MemberRef`, `TypeRef`, `ComposableCatalog`, `ComposablePartDefinition`, `ImportDefinitionBinding`, `ImportMetadataViewConstraint`, `PartCreationPolicyConstraint`, and `RuntimeComposition`. Created a private abstract class `TypeSerializerTest<TObjectType>` for generic serialization tests. Added nested classes for specific type tests, each overriding `CreateObjectToSerialize` method. Included necessary using directives, namespace declarations, and license information.
Updated PartDiscoveryException to use a custom MessagePackFormatter: - Added internal field `StackTraceInternal` and overridden `StackTrace`. - Implemented `PartDiscoveryExceptionFormatter` for serialization. Enhanced PartDiscoveryExceptionTests: - Removed .NET Core conditional compilation. - Added tests for serialization/deserialization with MessagePack. Refactored SerializationFormatterTests: - Renamed methods for consistency and clarity. - Added `PartDiscoveryExceptionSerializeTest` for new serialization logic. - Introduced `PartDiscoveryExceptionSerialize` class for test preparation.
Instead, we have formatters whose instances are created with the MEF `Resolver` they need.
Instead of copying the magic number into an error message that can go stale, I defined a method that will read, assert, and throw a message with the number all from one argument.
AArnott
approved these changes
Jun 25, 2024
| using Xunit; | ||
|
|
||
| public class PartDiscoveryExceptionTests | ||
| { | ||
| #if !NETCOREAPP | ||
| [Fact] | ||
| public void ExceptionIsSerializable() |
There was a problem hiding this comment.
This test used to test BinaryFormatter serializability of our exception. I don't want us to lose that. Instead of changing this test, I restored it and just added a new one for the scenario you're testing.
AArnott
approved these changes
Jun 25, 2024
Modified AssemblyNameFormatter to support serialization and deserialization of the CodeBase field in AssemblyName objects. Updated deserialization to expect two strings (FullName and CodeBase) and adjusted serialization to include both fields.
Merged
ankitvarmait
added a commit
that referenced
this pull request
Jul 31, 2024
This reverts commit ddee4d9.
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.
MessagePack serializer for vs-mef types