Skip to content
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

WmiLight integral types inconsistent with System.Management types #9

Closed
rastrac-joel opened this issue Jun 5, 2023 · 9 comments
Closed

Comments

@rastrac-joel
Copy link

rastrac-joel commented Jun 5, 2023

I've attached a screenshot of query output from both System.Management WMI interaction and WmiLight WMI interaction and you can see that WmiLight uses int for all integral types while System.Management uses other types like uint, ushort, etc. This means you get an InvalidCastException when casting and if you use the .GetPropertyValue<T> overload, the same thing occurs.

I looked at the code and it's not obvious to me where the int is materializing, but it does mean that WmiLight is difficult to drop in as a System.Management replacement. Additionally, in a case like int vs uint or int vs long, it may cause semantic problems - though it seems like the library simply uses string for long. I'd like to use this library for its support for trimming; Microsoft.Management.Infrastructure sucks in not only the same ways that System.Management does, but also requires a very specific RuntimeIdentifier.

wmilight_types

@MartinKuschnik
Copy link
Owner

Hi,

I implemtned WmiLight that is uses the types provided by the COM interface itself. System.Management teached me that i have to interprete the provided type identifie (CimType).

Changing the implementation is not so easy. It would break the usage of old version.

I have to think about an solution ....

@MartinKuschnik
Copy link
Owner

Could you please have a look to Version 4.0.0-pre?

This version should behave like System.Management but also allows the "wrong" types to avoid breaking changes.

@rastrac-joel
Copy link
Author

rastrac-joel commented Jun 12, 2023

It does seem to be able to be used as a drop-in replacement now. Thank you!

I played around with the backward-compatibility functionality. I have some code that looks like e.g.
result.Speed = Frequency.From(Magnitude.Mega, (uint?)obj.GetPropertyValue("Speed"));

If I change that (uint?) cast to an (int?) - or (int) for that matter - cast, an exception is thrown. It has to go through the GetPropertyValue<T> overload in order to function. In that case, it also doesn't allow the same kind of cast, only the raw type works. That is, .GetPropertyValue<int?>("") throws the same exception that the cast does. .GetPropertyValue<int>("") appears to work.

So it will support backward compatibility to some degree, but certain usages won't work.

@MartinKuschnik
Copy link
Owner

I only adjusted the generic version of the GetPropertyValue'method to rerurn the correct types by default.

Another option would be to introduce new methods (eg. GetValue and GetValue<TResult>) and mark the GetPropertyValue metods as obsolete.

I tend to the second options, because all other would be a compromise :-|

What do you think?

@rastrac-joel
Copy link
Author

I don't think there's a way to cleanly support all use cases in a backward-compatible fashion.

There's the use case of being a drop-in replacement. System.Management has object this[string] to get properties and object GetPropertyValue(string). Because WmiObject has object GetPropertyValue(string), if that's marked obsolete, it'll give either warnings or errors to users that are dropping it in to replace System.Management. There'll be type errors, of course, but for the most part, WmiLight is API-compatible as long as you rename the types. WmiLight users could have used the library in this same way, but they would've had different types on their casts.

In order to still be a drop-in replacement and support WmiLight users that might have been using this API, you would you have to return a proxy object that can be casted to either of the types. That is, since it returns object, you can return WmiPropertyProxy which has methods to cast to int, int?, and then the appropriate type as well. To enforce valid casting, I guess you would need a type-specific proxy object so something like WmiPropertyProxyUInt, WmiPropertyProxyUShort etc. The long or ulong versions would need to allow casting to string, the rest would allow casting to int or int?.

The only issue with this would be a separate use case where a caller simply used the object that gets returned without casting it. I'm not sure how common that usage is. If users use WMI like I do, the proxy object scheme works. If they're taking the result regardless of its type and doing things with it, like the WmiTool does, then returning a proxy object would possibly break those usages.

A separate use case is the convenience accessor T GetPropertyValue<T>(string) which will need backward compatibility and doesn't appear to be difficult to support. The presence of a possible null value on any WMI property means that e.g. int? casts are useful. A null has no type in WMI. It would be better to support those if possible. Only users of older WmiLight versions will be using this function. To be clear, since the implementation before was to simply do a cast, functioning int? casts etc. will be needed for backward compatibility.

You could introduce a class WmiObjectTyped or similar which has the new behavior. The old behavior would be maintained for the old users and then the new users - having to do a rename anyway - can simply use the new name. Though they'd have to understand the differences between the different WmiObject types. This would support everything at the cost of having essentially duplicated code with minor differences and be confusing to somebody that wasn't aware of the underlying issues. You would also have to overload functions that operate on or with WmiObject unless you gave them an inheritance relationship, which is its own set of problems.

You could make a separate library that's intended to be a complete drop-in replacement. That is, the only change you'd need to make is changing using System.Management; to using WmiLightManagement; or similar. There are some libraries that do this for other deprecated Microsoft technologies. I'm not sure how much of the API is directly compatible, so maybe this wouldn't be feasible.

It seems like there's a compromise somewhere at any level that you pick. Some kind of costs or strange behavior.

@MartinKuschnik
Copy link
Owner

I alos had the idea with the proxy object but did not like the fact that it also does not work in all cases and introduces aditional complexety and reduces the performence.

Therefore I decided to introduce a Version 4 with breaking changes.

To get an idea how WmiLight is used, I searched on GitHub and found some usages:

Please have a log to version 4.0.0. and if it wokes as drop-in replacement.

@MartinKuschnik
Copy link
Owner

@rastrac-joel I'm working on a new verion thats compatible with AOT. I have reimplement a lot of code and like to test it. Could you provide the WmiTool from your screenshots?

@rastrac-joel
Copy link
Author

It's owned by my employer, I'm currently looking into whether I have permission to release the source or the tool. I'll give you an update next week.

@MartinKuschnik
Copy link
Owner

I already wrote a small compare tool 😁

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants