-
Notifications
You must be signed in to change notification settings - Fork 1
Remove reflection-based proxy creation and enable class decoration in service decorators #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ment Co-authored-by: AGiorgetti <[email protected]>
…d eliminate reflection-based proxy creation Co-authored-by: AGiorgetti <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Update the changelog adding this story and the changes made.
// Remove the original service descriptor that has the service type | ||
services.Remove(originalServiceDescriptor); | ||
|
||
// Create a new service descriptor for the decorator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot improve the comment explaining the logic, exaplin why it shuould work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved comments throughout ServiceCollectionExtensions.Decorators.cs explaining why the implementation works - the new approach preserves original registration information and creates wrapper factories that eliminate reflection overhead while supporting both interfaces and concrete classes. Commit 7693b10
…ning logic and design rationale Co-authored-by: AGiorgetti <[email protected]>
Added comprehensive changelog entry documenting the decorator system enhancements, including elimination of reflection-based proxy creation and support for concrete class decoration. Commit 7693b10
Completed all requested changes: updated changelog, improved comments explaining the logic, and verified all tests pass. All 13 decorator tests pass including the new concrete class decoration tests. Commit 7693b10 |
This PR addresses the performance and flexibility limitations in the decorator system by eliminating reflection-based proxy creation and adding support for decorating concrete classes (not just interfaces).
Problem
The existing decorator implementation had two major limitations:
Reflection overhead: When decorating services registered with factory functions, the system created dynamic proxy types using
System.Reflection.Emit
, which added complexity and performance overhead.Interface-only restriction: The decorator system only supported interfaces, throwing an exception for concrete classes with the message "Service type X is not an interface."
Solution
🎯 Eliminated Reflection-Based Proxy Creation
Before: Factory-registered services required dynamic proxy type creation
After: Direct service creation without proxy types
🎯 Added Support for Class Decoration
Before: Only interfaces could be decorated
After: Both interfaces and classes supported
Implementation Details
The new decorator logic directly creates services using the original
ServiceDescriptor
information instead of relying on proxy types:ActivatorUtilities.CreateInstance
Benefits
Testing
Added comprehensive test coverage for class decoration scenarios:
Decorate_ConcreteClass_Registered_As_Type
Decorate_ConcreteClass_Registered_As_Instance
Decorate_ConcreteClass_Registered_As_Factory
All existing decorator tests continue to pass, ensuring backward compatibility.
Fixes #11.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.