apache
Avro 3603 dotnet reflect make properties and method virtual in ClassCache, DotnetClass, DotnetProperty
What is the purpose of the change
This PR is my next attempt to brings flexibility and opportunity to override default logic for .NET reflect reader and writer. Please find more detail in https://issues.apache.org/jira/browse/AVRO-3603
As there is argument against using interfaces (https://github.com/apache/avro/pull/1940) so I made all properties, fields and methods in ClassCache, DotnetClass, DotnetProperty virtual. Also I update all private to protected to be able use and override in child implementation. This should allow similar flexibility as interface.
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Documentation
- Does this pull request introduce a new feature? NO
- If yes, how is the feature documented? N/A
My biggest concern is calling the virtual functions from the constructors (see CodeQL complains). It must be addressed before merging.
Personally , I feel interfaces are a better solution here, but I understand the concerns about them as well.
virtual functions from the constructors
I think those are not introduced with this PR.
Hi @zcsizmadia , I remove "virtual" from methods that are called from constructor. Also, I made fields readonly as CodeQL suggested. Not sure why this changes are not reflected in scan result.
Hi @martin-g , Current PR does not update serialization/deserialization or other core avro functionality. It is only about classes that parse/map dotnet type to avro schema. Also, event though this changes can be user to quickly handle errors and add desired functionality, this is not the main purpose.
Users/projects can have custom requirements that are not applicable for everyone and that should not be added to Apache.Avro. For example, at my project we use camel case naming convention for classes and properties (MyClass, MyProperty) for .NET, and there is requirement to use snake case naming for avro schemas across company (different languages) (my_class, my_property). Currently I use AvroField attribute to override property name. But this mean, each micro service should reefer to Apache.Avro and I should not forget to add that attribute to each property and not forget to update it when schema is changed (what happens often before release). After this PR I will be able to update property - field mapping to add implement custom requirements once.
Hi @vivere-dally , This PR and https://github.com/apache/avro/pull/1940 are different approaches to resolve the same task. Only one should be merged.
I prefer https://github.com/apache/avro/pull/1940