nicoriff / ORMi

A Light-ORM for accesing WMI
MIT License
209 stars 28 forks source link

Conversion fails for Nullable ValueType #3

Closed Maverik closed 5 years ago

Maverik commented 5 years ago

It seems Convert.ChangeType at L76 of TypeHelper chokes on a nullable value type when the value is null. Since a null value will result in default value of property anyway, we can do an early return out of _SetPropertyValue and not run into an InvalidCastException.

Property tested: IPConnectionMetric as unit? on NetworkAdapterConfiguration

StackTrace:

   at System.Convert.ChangeType(Object value, Type conversionType, IFormatProvider provider)
   at ORMi.Helpers.TypeHelper.LoadObject(ManagementObject mo, Type t)
   at ORMi.WMIHelper.Query[T]()
   at UserQuery.Main()
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()

Proposed fix @L76: Simplify to p.SetValue(o, a, null);

Is there a specific scenario that you're using Convert.ChangeType for? IMHO, if there's a mismatch of types between what WMI was expecting and what end-user wanted, end-user should be doing an explicit cast rather than the library.

It might be worth looking into a Converter attribute for more custom behaviour like going from a string IPAddress to IPAddress. I've done something similar in my powershell-converter repo for the same reason: Serialize & Deserialize

Thank you very much for your work. This makes working with WMI much more pleasant.

nicoriff commented 5 years ago

Hi @Venomed. Thanks for using ORMi in first place.

I´ve already solved the issue. You can download or pull the new version of the repo.

SetValue method aims to make simpler the development. Datetime format in WMI is a pain. SetValue solves that so you not have to try to guess about WMI DateTime format.

It is a good idea indeed to make a Converter for other class of types such as IpAddress. I will take that to backlog.

Regards!.

Maverik commented 5 years ago

Perfect! thank you for such a quick fix!

Re: Converter, Just to be clear: I meant a generic IValueConverter<TInput, TOutput> sort of thing that we could feed via a ConverterAttribute and not literally create multiple converters (since that'd just be fair bit of work and bloat the library).