Replace from_com and into_com with From implementations (#126)

Given that these from_com and into_com methods are all safe anyway,
there's no good reason not to use From instead

Rewrite them as ```impl From<ITYPE> for TYPE``` and ```impl From<ITYPE>
for TYPE```.
In a few cases, the second (which would have been into_com before) did
not exist, but made sense, so I added them.
As per general Rust guidance, implementing From, as that also results in
the blanket ```Into``` implementation.

In the vast majority of cases, ownership was required, so I went with
```impl From<ITYPE> for TYPE``` instead of ```impl From<&ITYPE> for
TYPE```, as in some cases, this will save a reference count increment,
and in the other case, it just moves where .clone is called, and those
implementations won't be called frequently outside the library.

This also saved a few unnecessary clones inside the library that we
appear to have had by accident.

Note in a few cases this makes it possible to construct the individual
client wrappers outside the library (where we didn't have pub on
from_com before). This is in my view a feature.

This will require any code outside this library to replace from_com with
from and into_com with .into, but that shouldn't be a big deal.

I left ```FabricClient::from_com``` alone because the comment says the
method is intended to be private eventually.

crates/libs/core/src/client/connection.rs

@@ -27,8 +27,8 @@ pub struct GatewayInformationResult {
     pub node_name: crate::WString,
 }
 
-impl GatewayInformationResult {
-    fn from_com(com: &IFabricGatewayInformationResult) -> Self {
+impl From<&IFabricGatewayInformationResult> for GatewayInformationResult {
+    fn from(com: &IFabricGatewayInformationResult) -> Self {
         let info = unsafe { com.get_GatewayInformation().as_ref().unwrap() };
         Self {
             node_address: WStringWrap::from(info.NodeAddress).into(),
@@ -69,15 +69,15 @@ where
         &self,
         gw_info: windows_core::Ref<IFabricGatewayInformationResult>,
     ) -> windows_core::Result<()> {
-        let info = GatewayInformationResult::from_com(gw_info.unwrap());
+        let info = GatewayInformationResult::from(gw_info.unwrap());
         self.inner.on_connected(&info)
     }
 
     fn OnDisconnected(
         &self,
         gw_info: windows_core::Ref<IFabricGatewayInformationResult>,
     ) -> windows_core::Result<()> {
-        let info = GatewayInformationResult::from_com(gw_info.unwrap());
+        let info = GatewayInformationResult::from(gw_info.unwrap());
         self.inner.on_disconnected(&info)
     }
 }

crates/libs/core/src/client/health_client.rs

@@ -31,16 +31,20 @@ pub struct HealthClient {
     com: IFabricHealthClient4,
 }
 
-// Public implementation block
-impl HealthClient {
-    pub fn get_com(&self) -> IFabricHealthClient4 {
-        self.com.clone()
+impl From<IFabricHealthClient4> for HealthClient {
+    fn from(value: IFabricHealthClient4) -> Self {
+        Self { com: value }
     }
+}
 
-    pub fn from_com(com: IFabricHealthClient4) -> Self {
-        Self { com: com.clone() }
+impl From<HealthClient> for IFabricHealthClient4 {
+    fn from(value: HealthClient) -> Self {
+        value.com
     }
+}
 
+// Public implementation block
+impl HealthClient {
     /// Reports health on a Service Fabric entity. See C# API [here](https://docs.microsoft.com/en-us/dotnet/api/system.fabric.fabricclient.healthclient.reporthealth?view=azure-dotnet).
     ///
     /// Remarks:

crates/libs/core/src/client/mod.rs

@@ -226,10 +226,10 @@ impl FabricClient {
         let com_query_client = com.clone().cast::<IFabricQueryClient10>().unwrap();
         let com_health_client = com.clone().cast::<IFabricHealthClient4>().unwrap();
         Self {
-            property_client: PropertyManagementClient::from_com(com_property_client),
-            service_client: ServiceManagementClient::from_com(com_service_client),
-            query_client: QueryClient::from_com(com_query_client),
-            health_client: HealthClient::from_com(com_health_client),
+            property_client: PropertyManagementClient::from(com_property_client),
+            service_client: ServiceManagementClient::from(com_service_client),
+            query_client: QueryClient::from(com_query_client),
+            health_client: HealthClient::from(com_health_client),
         }
     }
 
@@ -259,13 +259,14 @@ pub struct PropertyManagementClient {
     com: IFabricPropertyManagementClient2,
 }
 
-impl PropertyManagementClient {
-    /// Get a copy of COM object
-    pub fn get_com(&self) -> IFabricPropertyManagementClient2 {
-        self.com.clone()
+impl From<IFabricPropertyManagementClient2> for PropertyManagementClient {
+    fn from(com: IFabricPropertyManagementClient2) -> Self {
+        Self { com }
     }
+}
 
-    fn from_com(com: IFabricPropertyManagementClient2) -> Self {
-        Self { com }
+impl From<PropertyManagementClient> for IFabricPropertyManagementClient2 {
+    fn from(value: PropertyManagementClient) -> Self {
+        value.com
     }
 }

crates/libs/core/src/client/notification.rs

@@ -37,8 +37,8 @@ pub struct ServiceNotification {
     com: IFabricServiceNotification,
 }
 
-impl ServiceNotification {
-    fn from_com(com: IFabricServiceNotification) -> Self {
+impl From<IFabricServiceNotification> for ServiceNotification {
+    fn from(com: IFabricServiceNotification) -> Self {
         // SF guarantees this is not null.
         let raw = unsafe { com.get_Notification().as_ref().unwrap() };
         Self {
@@ -56,10 +56,12 @@ impl ServiceNotification {
             com,
         }
     }
+}
 
+impl ServiceNotification {
     pub fn get_version(&self) -> crate::Result<ServiceEndpointsVersion> {
         let version = unsafe { self.com.GetVersion() }?;
-        Ok(ServiceEndpointsVersion::from_com(version))
+        Ok(ServiceEndpointsVersion::from(version))
     }
 }
 
@@ -96,11 +98,19 @@ pub struct ServiceEndpointsVersion {
     com: IFabricServiceEndpointsVersion,
 }
 
-impl ServiceEndpointsVersion {
-    fn from_com(com: IFabricServiceEndpointsVersion) -> Self {
+impl From<IFabricServiceEndpointsVersion> for ServiceEndpointsVersion {
+    fn from(com: IFabricServiceEndpointsVersion) -> Self {
         Self { com }
     }
+}
 
+impl From<ServiceEndpointsVersion> for IFabricServiceEndpointsVersion {
+    fn from(value: ServiceEndpointsVersion) -> Self {
+        value.com
+    }
+}
+
+impl ServiceEndpointsVersion {
     /// CSharp doc: Zero if this and other are equivalent,
     /// a negative value if this is less than other, and a positive value if this is greater than other.
     ///
@@ -167,7 +177,7 @@ where
         notification: windows_core::Ref<IFabricServiceNotification>,
     ) -> crate::Result<()> {
         let com = notification.unwrap();
-        let msg = ServiceNotification::from_com(com.to_owned());
+        let msg = ServiceNotification::from(com.clone());
         self.inner.on_notification(&msg)
     }
 }

crates/libs/core/src/client/query_client.rs

@@ -113,13 +113,15 @@ impl QueryClient {
     }
 }
 
-impl QueryClient {
-    pub fn get_com(&self) -> IFabricQueryClient10 {
-        self.com.clone()
+impl From<IFabricQueryClient10> for QueryClient {
+    fn from(com: IFabricQueryClient10) -> Self {
+        Self { com }
     }
+}
 
-    pub fn from_com(com: IFabricQueryClient10) -> Self {
-        Self { com: com.clone() }
+impl From<QueryClient> for IFabricQueryClient10 {
+    fn from(value: QueryClient) -> Self {
+        value.com
     }
 }
 
@@ -161,7 +163,7 @@ impl QueryClient {
             )
         }
         .await??;
-        Ok(NodeList::from_com(com))
+        Ok(NodeList::from(com))
     }
 
     pub async fn get_partition_list(

crates/libs/core/src/client/svc_mgmt_client.rs

@@ -150,10 +150,15 @@ impl ServiceManagementClient {
     }
 }
 
-// public implementation block
-impl ServiceManagementClient {
-    pub fn from_com(com: IFabricServiceManagementClient6) -> Self {
-        Self { com: com.clone() }
+impl From<IFabricServiceManagementClient6> for ServiceManagementClient {
+    fn from(com: IFabricServiceManagementClient6) -> Self {
+        Self { com }
+    }
+}
+
+impl From<ServiceManagementClient> for IFabricServiceManagementClient6 {
+    fn from(value: ServiceManagementClient) -> Self {
+        value.com
     }
 }
 
@@ -186,7 +191,7 @@ impl ServiceManagementClient {
             )
         }
         .await??;
-        let res = ResolvedServicePartition::from_com(com);
+        let res = ResolvedServicePartition::from(com);
         Ok(res)
     }
 
@@ -377,8 +382,8 @@ pub struct ResolvedServicePartition {
     com: IFabricResolvedServicePartitionResult,
 }
 
-impl ResolvedServicePartition {
-    fn from_com(com: IFabricResolvedServicePartitionResult) -> Self {
+impl From<IFabricResolvedServicePartitionResult> for ResolvedServicePartition {
+    fn from(com: IFabricResolvedServicePartitionResult) -> Self {
         Self { com }
     }
 }
@@ -408,7 +413,7 @@ impl ResolvedServicePartition {
 
     // Get the list of endpoints
     pub fn get_endpoint_list(&self) -> ResolvedServiceEndpointList {
-        ResolvedServiceEndpointList::from_com(self.com.clone())
+        ResolvedServiceEndpointList::from(self.com.clone())
     }
 
     // If compared with different partition error is returned.
@@ -472,10 +477,13 @@ pub struct ResolvedServiceEndpointList {
     com: IFabricResolvedServicePartitionResult,
 }
 
-impl ResolvedServiceEndpointList {
-    fn from_com(com: IFabricResolvedServicePartitionResult) -> Self {
+impl From<IFabricResolvedServicePartitionResult> for ResolvedServiceEndpointList {
+    fn from(com: IFabricResolvedServicePartitionResult) -> Self {
         Self { com }
     }
+}
+
+impl ResolvedServiceEndpointList {
     // Get iterator for the list
     pub fn iter(&self) -> ResolvedServiceEndpointListIter {
         ResolvedServiceEndpointListIter::new(self, self)

crates/libs/core/src/runtime/activation_context.rs

@@ -78,7 +78,7 @@ impl CodePackageActivationContext {
             self.com_impl
                 .GetConfigurationPackage(configpackagename.as_pcwstr())
         }?;
-        Ok(ConfigurationPackage::from_com(c))
+        Ok(ConfigurationPackage::from(c))
     }
 
     pub fn get_code_package_info(&self) -> CodePackageInfo {
@@ -162,7 +162,7 @@ impl CodePackageActivationContext {
                 .RegisterConfigurationPackageChangeHandler(&callback)
         }?;
         // SAFETY: raw_handle is a configuration package change handler id, not some other id.
-        Ok(unsafe { ConfigurationPackageChangeCallbackHandle::from_com(raw_handle) })
+        Ok(unsafe { ConfigurationPackageChangeCallbackHandle::from(raw_handle) })
     }
 
     pub fn unregister_configuration_package_change_handler(

crates/libs/core/src/runtime/config.rs

@@ -62,11 +62,19 @@ impl FabricListAccessor<FABRIC_CONFIGURATION_SECTION> for ConfigurationSectionLi
     }
 }
 
-impl ConfigurationPackage {
-    pub fn from_com(com: IFabricConfigurationPackage) -> Self {
+impl From<IFabricConfigurationPackage> for ConfigurationPackage {
+    fn from(com: IFabricConfigurationPackage) -> Self {
         Self { com }
     }
+}
+
+impl From<ConfigurationPackage> for IFabricConfigurationPackage {
+    fn from(value: ConfigurationPackage) -> Self {
+        value.com
+    }
+}
 
+impl ConfigurationPackage {
     pub fn get_description(&self) -> ConfigurationPackageDesc {
         let raw = unsafe { self.com.get_Description().as_ref().unwrap() };
 

crates/libs/core/src/runtime/package_change/config.rs

@@ -47,7 +47,7 @@ where
         _source: windows_core::Ref<mssf_com::FabricRuntime::IFabricCodePackageActivationContext>,
         configpackage: windows_core::Ref<mssf_com::FabricRuntime::IFabricConfigurationPackage>,
     ) {
-        let new_package = ConfigurationPackage::from_com(configpackage.unwrap().clone());
+        let new_package = ConfigurationPackage::from(configpackage.unwrap().clone());
         let event = ConfigurationPackageChangeEvent::Addition { new_package };
         self.inner.on_change(&event)
     }
@@ -57,7 +57,7 @@ where
         _source: windows_core::Ref<mssf_com::FabricRuntime::IFabricCodePackageActivationContext>,
         configpackage: windows_core::Ref<mssf_com::FabricRuntime::IFabricConfigurationPackage>,
     ) {
-        let previous_package = ConfigurationPackage::from_com(configpackage.unwrap().clone());
+        let previous_package = ConfigurationPackage::from(configpackage.unwrap().clone());
         let event = ConfigurationPackageChangeEvent::Removal { previous_package };
         self.inner.on_change(&event)
     }
@@ -70,9 +70,8 @@ where
         >,
         configpackage: windows_core::Ref<mssf_com::FabricRuntime::IFabricConfigurationPackage>,
     ) {
-        let new_package = ConfigurationPackage::from_com(configpackage.unwrap().clone());
-        let previous_package =
-            ConfigurationPackage::from_com(previousconfigpackage.unwrap().clone());
+        let new_package = ConfigurationPackage::from(configpackage.unwrap().clone());
+        let previous_package = ConfigurationPackage::from(previousconfigpackage.unwrap().clone());
         let event = ConfigurationPackageChangeEvent::Modification {
             previous_package,
             new_package,
@@ -117,7 +116,7 @@ pub struct ConfigurationPackageChangeCallbackHandle(pub(crate) i64);
 impl ConfigurationPackageChangeCallbackHandle {
     /// # Safety
     /// Caller ensures this is a registered callback id
-    pub const unsafe fn from_com(com: i64) -> Self {
+    pub const unsafe fn from(com: i64) -> Self {
         Self(com)
     }
 }

crates/libs/core/src/types/client/node.rs

@@ -85,13 +85,17 @@ impl FabricListAccessor<FABRIC_NODE_QUERY_RESULT_ITEM> for NodeList {
     }
 }
 
-impl NodeList {
-    pub fn from_com(com: IFabricGetNodeListResult2) -> Self {
+impl From<IFabricGetNodeListResult2> for NodeList {
+    fn from(com: IFabricGetNodeListResult2) -> Self {
         Self { com }
     }
+}
+
+impl NodeList {
     pub fn iter(&self) -> NodeListIter {
         NodeListIter::new(self, self)
     }
+
     pub fn get_paging_status(&self) -> Option<PagingStatus> {
         // If there is no more entries there is no paging status returned.
         let raw = unsafe { self.com.get_PagingStatus().as_ref() }?;