Apply suggested style changes to C# code (#18)

This PR applies suggested style changes to C# code from the official .NET tooling. This also adds a step to the .NET CI workflow to perform these checks

.github/workflows/dotnet-ci.yml

@@ -8,16 +8,19 @@ on:
     branches:
       - main
 
+env:
+  DOTNET_VERSION: 6.0.119
+
 jobs:
   static_code_analysis:
     runs-on: ubuntu-20.04
     steps:
       - name: Checkout repository and submodules
         uses: actions/checkout@v3
-      - name: Install .NET 6.0.119
+      - name: Install .NET ${{ env.DOTNET_VERSION }}
         uses: actions/setup-dotnet@v3
         with:
-          dotnet-version: 6.0.119
+          dotnet-version: ${{ env.DOTNET_VERSION }}
           global-json-file: cloud_connectors/azure/digital_twins_connector/global.json
       - name: Cache NuGet dependencies
         uses: actions/cache@v3
@@ -30,7 +33,9 @@ jobs:
       - name: Build Digital Twins Connector
         run: ./cloud_connectors/azure/digital_twins_connector/build.sh
       - name: Build MQTT Connector's Azure Function
-        run: dotnet build cloud_connectors/azure/mqtt_connector/azure_function/src/function.csproj
+        run: |
+          dotnet build cloud_connectors/azure/mqtt_connector/azure_function/src/function.csproj -warnaserror
+          dotnet build cloud_connectors/azure/mqtt_connector/azure_function/tests/MQTTConnectorAzureFunction.Tests.csproj -warnaserror
       - name: Digital Twins Connector Tests
         run: dotnet test cloud_connectors/azure/digital_twins_connector/tests/**/*.csproj
       - name: MQTT Connector's Azure Function Tests

cloud_connectors/azure/.globalconfig

@@ -0,0 +1,6 @@
+# Global analyzer config for .NET projects
+is_global = true
+
+# The way to address this rule is not well documented and appears to be very complex,
+# so it's left as a suggestion for now
+dotnet_diagnostic.CA1848.severity = suggestion

cloud_connectors/azure/digital_twins_connector/build.sh

@@ -7,6 +7,6 @@
 # Set the current directory to the directory of this script.
 cd "$(dirname "$0")"
 
-dotnet build src/core/DigitalTwinsConnector.csproj
-dotnet build src/DigitalTwinsClientWrapper/DigitalTwinsClientWrapper.csproj
-dotnet build tests/DigitalTwinsClientWrapper.Tests/DigitalTwinsClientWrapper.Tests.csproj
+dotnet build src/core/DigitalTwinsConnector.csproj -warnaserror
+dotnet build src/DigitalTwinsClientWrapper/DigitalTwinsClientWrapper.csproj -warnaserror
+dotnet build tests/DigitalTwinsClientWrapper.Tests/DigitalTwinsClientWrapper.Tests.csproj -warnaserror

cloud_connectors/azure/digital_twins_connector/src/DigitalTwinsClientWrapper/DigitalTwinsClientWrapper.cs

@@ -28,7 +28,7 @@ public class DigitalTwinsClientWrapper
         /// </summary>
         /// <param name="path">the path.</param>
         /// <returns>Returns true if the path starts with a slash, otherwise false.</returns>
-        private bool DoesPathStartsWithSlash(string path)
+        private static bool DoesPathStartsWithSlash(string path)
         {
             return path.StartsWith('/');
         }
@@ -56,10 +56,12 @@ public DigitalTwinsClientWrapper(DigitalTwinsClient client, ILogger<DigitalTwins
         /// <param name="instanceID">the digital twin instance ID.</param>
         /// <param name="instancePropertyPath">the property path of a digital twin instance to update.</param>
         /// <param name="data">the data used to update a digital twin instance's property.</param>
+        /// <exception cref="Azure.RequestFailedException">Rethrown if the client throws this exception</exception>
+        /// <exception cref="NotSupportedException">Thrown if the data parameter could not be parsed</exception>
         /// <returns>Returns a task for updating a digital twin instance.</returns>
         public async Task UpdateDigitalTwinAsync(string modelID, string instanceID, string instancePropertyPath, string data)
         {
-            List<Type> dataTypes = new List<Type>() { typeof(Double), typeof(Boolean), typeof(Int32) };
+            List<Type> dataTypes = new() { typeof(double), typeof(bool), typeof(int) };
             var jsonPatchDocument = new JsonPatchDocument();
 
             foreach (Type type in dataTypes)
@@ -73,19 +75,31 @@ public async Task UpdateDigitalTwinAsync(string modelID, string instanceID, stri
                     {
                         instancePropertyPath = "$/{instancePropertyPath}";
                     }
+
                     // Once we're able to parse the data string to a type
                     // we append it to the jsonPatchDocument
                     jsonPatchDocument.AppendAdd(instancePropertyPath, value);
 
                     // First UpdateDigitalTwinAsync call may block due to initial authorization.
                     await _client.UpdateDigitalTwinAsync(instanceID, jsonPatchDocument);
-                    _logger.LogInformation($"Successfully set instance {instanceID}{instancePropertyPath} based on model {modelID} to {data}");
+                    _logger.LogInformation(
+                        "Successfully set instance {InstanceID}{InstancePropertyPath} based on model {ModelID} to {Data}",
+                        instanceID,
+                        instancePropertyPath,
+                        modelID,
+                        data);
                     return;
                 }
                 catch (RequestFailedException ex)
                 {
-                    _logger.LogError($"Cannot set instance {instanceID}{instancePropertyPath} based on model {modelID} to {data} due to {ex.Message}");
-                    throw ex;
+                    _logger.LogError(
+                        "Cannot set instance {InstanceID}{InstancePropertyPath} based on model {ModelID} to {Data} due to {Message}",
+                        instanceID,
+                        instancePropertyPath,
+                        modelID,
+                        data,
+                        ex.Message);
+                    throw;
                 }
                 // Try to parse string data with the next type if we're unsuccessful.
                 catch (Exception ex) when (ex is NotSupportedException || ex is ArgumentException || ex is FormatException)
@@ -94,9 +108,13 @@ public async Task UpdateDigitalTwinAsync(string modelID, string instanceID, stri
                 }
             }
 
-            string errorMessage = $"Failed to parse {data}. Cannot set instance {instanceID}{instancePropertyPath} based on model {modelID} to {data}";
-            _logger.LogError(errorMessage);
-            throw new NotSupportedException(errorMessage);
+            _logger.LogError(
+                "Failed to parse data. Cannot set instance {InstanceID}{InstancePropertyPath} based on model {ModelID} to {Data}",
+                instanceID,
+                instancePropertyPath,
+                modelID,
+                data);
+            throw new NotSupportedException();
         }
     }
 }

cloud_connectors/azure/digital_twins_connector/src/DigitalTwinsClientWrapper/DigitalTwinsClientWrapper.csproj

@@ -3,6 +3,7 @@
  <PropertyGroup>
     <TargetFramework>net6.0</TargetFramework>
     <ImplicitUsings>enable</ImplicitUsings>
+    <AnalysisLevel>latest-recommended</AnalysisLevel>
   </PropertyGroup>
 
   <ItemGroup>

cloud_connectors/azure/digital_twins_connector/src/core/DigitalTwinsConnector.csproj

@@ -4,6 +4,7 @@
     <OutputType>Exe</OutputType>
     <TargetFramework>net6.0</TargetFramework>
     <ImplicitUsings>enable</ImplicitUsings>
+    <AnalysisLevel>latest-recommended</AnalysisLevel>
   </PropertyGroup>
 
   <ItemGroup>

cloud_connectors/azure/digital_twins_connector/src/core/DigitalTwinsConnectorService.cs

@@ -43,7 +43,7 @@ public override async Task<UpdateDigitalTwinResponse> UpdateDigitalTwin(UpdateDi
             }
             catch (Exception ex)
             {
-                _logger.LogError(ex.Message);
+                _logger.LogError("Error updating digital twin: {ExceptionType}: {Message}", ex.GetType(), ex.Message);
                 throw;
             }
 

cloud_connectors/azure/digital_twins_connector/src/core/Program.cs

@@ -29,13 +29,14 @@ static void Main(string[] args)
 
             string adtInstanceUrl = adtInstanceConfig.AzureDigitalTwinsInstanceUrl;
             var credential = new DefaultAzureCredential();
-            DigitalTwinsClient client = new DigitalTwinsClient(new Uri(adtInstanceUrl), credential);
+            DigitalTwinsClient client = new(new Uri(adtInstanceUrl), credential);
 
             ILoggerFactory loggerFactory = LoggerFactory.Create(builder => builder.AddSimpleConsole(c =>
             {
                 c.TimestampFormat = "[yyyy-MM-ddTHH:mm::ssZ] ";
                 c.UseUtcTimestamp = true;
             }));
+
             loggerFactory.CreateLogger("Main").LogInformation("Started the Azure Digital Twins Connector");
 
             // Instantiate the DigitalTwinClient first before adding it as a service for dependency injection.

cloud_connectors/azure/digital_twins_connector/tests/DigitalTwinsClientWrapper.Tests/DigitalTwinsClientWrapper.Tests.cs

@@ -23,7 +23,7 @@ public void Setup()
         }
 
         [Test]
-        public async Task UpdateDigitalTwinAsync_ShouldSucceed()
+        public async Task UpdateDigitalTwinAsyncShouldSucceed()
         {
             const string modelID = "some-model";
             const string instanceID = "some-instance";
@@ -33,7 +33,7 @@ public async Task UpdateDigitalTwinAsync_ShouldSucceed()
         }
 
         [Test]
-        public void UpdateDigitalTwinAsync_ThrowNotSupported()
+        public void UpdateDigitalTwinAsyncThrowNotSupported()
         {
             const string modelID = "some-model";
             const string instanceID = "some-instance";

cloud_connectors/azure/digital_twins_connector/tests/DigitalTwinsClientWrapper.Tests/DigitalTwinsClientWrapper.Tests.csproj

@@ -4,6 +4,7 @@
     <TargetFramework>net6.0</TargetFramework>
     <ImplicitUsings>enable</ImplicitUsings>
     <IsPackable>false</IsPackable>
+    <AnalysisLevel>latest-recommended</AnalysisLevel>
   </PropertyGroup>
 
   <ItemGroup>

cloud_connectors/azure/mqtt_connector/azure_function/src/function.csproj

@@ -2,6 +2,7 @@
  <PropertyGroup>
     <TargetFramework>net6.0</TargetFramework>
     <AzureFunctionsVersion>v4</AzureFunctionsVersion>
+    <AnalysisLevel>latest-recommended</AnalysisLevel>
   </PropertyGroup>
 
   <ItemGroup>

cloud_connectors/azure/mqtt_connector/azure_function/src/run.cs

@@ -4,7 +4,8 @@
 
 using System;
 using System.Collections.Generic;
-using System.ComponentModel;
+using System.Globalization;
+using System.Text.Json.Serialization;
 using System.Threading.Tasks;
 
 using Azure;
@@ -19,25 +20,63 @@ namespace Microsoft.ESDV.CloudConnector.Azure {
     /// <summary>
     /// This class contains the info to target an Azure Digital Twin instance.
     /// </summary>
-    public class DigitalTwinsInstance {
-        public string model_id { get; set; }
-        public string instance_id { get; set; }
-        public string instance_property_path { get; set; }
-        public string data { get; set; }
+    public class DigitalTwinsInstance
+    {
+        /// <summary>
+        /// The Azure Digital Twins model ID
+        /// </summary>
+        [JsonPropertyName("model_id")]
+        public string ModelId { get; set; }
+
+        /// <summary>
+        /// The Azure Digital Twins instance ID
+        /// </summary>
+        [JsonPropertyName("instance_id")]
+        public string InstanceId { get; set; }
+
+        /// <summary>
+        /// The Azure Digital Twins instance property path
+        /// </summary>
+        [JsonPropertyName("instance_property_path")]
+        public string InstancePropertyPath { get; set; }
+
+        /// <summary>
+        /// The data to synchronize
+        /// </summary>
+        [JsonPropertyName("data")]
+        public string Data { get; set; }
     }
 
-    public class MQTTConnectorAzureFunction {
+    /// <summary>
+    /// Azure function for use with the MQTT connector.
+    /// Reads data from an event grid and forwards it to Azure Digital Twins.
+    /// </summary>
+    public class MQTTConnectorAzureFunction
+    {
+        /// <summary>
+        /// The logger for this function
+        /// </summary>
         private readonly ILogger _logger;
 
+        /// <summary>
+        /// The environment variable name for the keyvault settings
+        /// </summary>
         private const string KEYVAULT_SETTINGS = "KEYVAULT_SETTINGS";
 
-        // Maps a string data type name to its concrete data type.
-        private static readonly Dictionary<string, Type> dataTypeNameToConverterMap = new Dictionary<string, Type> {
+        /// <summary>
+        /// Maps a string data type name to its concrete data type.
+        /// </summary>
+        private static readonly Dictionary<string, Type> dataTypeNameToConverterMap = new()
+        {
             { "int", typeof(int) },
             { "double", typeof(double) },
             { "boolean", typeof(bool) }
         };
 
+        /// <summary>
+        /// Create a new MQTTConnectorAzureFunction
+        /// </summary>
+        /// <param name="logger">The logger to use</param>
         public MQTTConnectorAzureFunction(ILogger<MQTTConnectorAzureFunction> logger)
         {
             _logger = logger;
@@ -48,54 +87,64 @@ public MQTTConnectorAzureFunction(ILogger<MQTTConnectorAzureFunction> logger)
         /// </summary>
         /// <param name="path">the path.</param>
         /// <returns>Returns true if the path starts with a slash, otherwise false.</returns>
-        public static bool DoesPathStartsWithSlash(string path) {
+        public static bool DoesPathStartsWithSlash(string path)
+        {
             return path.StartsWith('/');
         }
 
         /// <summary>
         /// Gets the data type from a data type name.
         /// </summary>
-        /// <param name="dataTypeName">the name of the data type.
+        /// <param name="dataTypeName">the name of the data type.</param>
+        /// <exception cref="NotSupportedException">Thrown if the data type is not supported.</exception>
         /// <returns>Returns a task for updating a digital twin instance.</returns>
-        public Type GetDataTypeFromString(string dataTypeName) {
-            if (!dataTypeNameToConverterMap.ContainsKey(dataTypeName)) {
+        public static Type GetDataTypeFromString(string dataTypeName)
+        {
+            if (!dataTypeNameToConverterMap.TryGetValue(dataTypeName, out Type value))
+            {
                 throw new NotSupportedException($"No conversion for {dataTypeName}");
             }
-            return dataTypeNameToConverterMap[dataTypeName];
+
+            return value;
         }
 
         /// <summary>
         /// Updates a digital twin's property.
         /// </summary>
         /// <param name="client">the Azure Digital Twins client.</param>
         /// <param name="instance">the digital twin instance to update.</param>
-        /// <param name="dataTypeName">the name of the data type.
+        /// <param name="dataTypeName">the name of the data type. Defaults to "double".</param>
         /// <returns>Returns a task for updating a digital twin instance.</returns>
-        public async Task UpdateDigitalTwinAsync(DigitalTwinsClient client, DigitalTwinsInstance instance, string dataTypeName = "double") {
-            JsonPatchDocument jsonPatchDocument = new JsonPatchDocument();
+        public static async Task UpdateDigitalTwinAsync(DigitalTwinsClient client, DigitalTwinsInstance instance, string dataTypeName = "double")
+        {
+            JsonPatchDocument jsonPatchDocument = new();
 
-            try {
+            try
+            {
                 // Get the concrete data type of an instance's data based on its string data type name
                 // then uses that concrete data type to change the data from string to its concrete data type.
                 Type dataType = GetDataTypeFromString(dataTypeName);
-                dynamic convertedDataToType = Convert.ChangeType(instance.data, dataType);
+                dynamic convertedDataToType = Convert.ChangeType(instance.Data, dataType, CultureInfo.InvariantCulture);
 
-                if (!DoesPathStartsWithSlash(instance.instance_property_path))
+                if (!DoesPathStartsWithSlash(instance.InstancePropertyPath))
                 {
-                    instance.instance_property_path = $"/{instance.instance_property_path}";
+                    instance.InstancePropertyPath = $"/{instance.InstancePropertyPath}";
                 }
-                jsonPatchDocument.AppendAdd(instance.instance_property_path, convertedDataToType);
+                jsonPatchDocument.AppendAdd(instance.InstancePropertyPath, convertedDataToType);
             }
-            catch (Exception ex) when (ex is NotSupportedException || ex is InvalidCastException || ex is FormatException) {
-                throw new NotSupportedException($"Cannot convert {instance.data}. {ex.Message}");
+            catch (Exception ex) when (ex is NotSupportedException || ex is InvalidCastException || ex is FormatException)
+            {
+                throw new NotSupportedException($"Cannot convert {instance.Data}. {ex.Message}");
             }
 
-            try {
-                await client.UpdateDigitalTwinAsync(instance.instance_id, jsonPatchDocument);
+            try
+            {
+                await client.UpdateDigitalTwinAsync(instance.InstanceId, jsonPatchDocument);
             }
-            catch(RequestFailedException ex) {
-                string errorMessage = @$"Cannot set instance {instance.instance_id}{instance.instance_property_path}
-                    based on model {instance.model_id} to {instance.data} due to {ex.Message}";
+            catch(RequestFailedException ex)
+            {
+                string errorMessage = @$"Cannot set instance {instance.InstanceId}{instance.InstancePropertyPath}
+                    based on model {instance.ModelId} to {instance.Data} due to {ex.Message}";
                 throw new NotSupportedException(errorMessage);
             }
         }
@@ -108,18 +157,21 @@ public async Task UpdateDigitalTwinAsync(DigitalTwinsClient client, DigitalTwins
         /// <exception>An exception is thrown if the Azure Digital Twin client cannot update an instance.</exception>
         /// <returns></returns>
         [FunctionName("MQTTConnectorAzureFunction")]
-        public async Task Run([EventGridTrigger] CloudEvent cloudEvent) {
+        public async Task Run([EventGridTrigger] CloudEvent cloudEvent)
+        {
             DigitalTwinsInstance instance = cloudEvent.Data.ToObjectFromJson<DigitalTwinsInstance>();
 
-            try {
-                DefaultAzureCredential credential = new DefaultAzureCredential();
+            try
+            {
+                DefaultAzureCredential credential = new();
                 string adt_instance_url = Environment.GetEnvironmentVariable(KEYVAULT_SETTINGS, EnvironmentVariableTarget.Process);
-                DigitalTwinsClient client = new DigitalTwinsClient(new Uri(adt_instance_url), credential);
+                DigitalTwinsClient client = new(new Uri(adt_instance_url), credential);
                 await UpdateDigitalTwinAsync(client, instance);
-                _logger.LogInformation(@$"Successfully set instance {instance.instance_id}{instance.instance_property_path}
-                    based on model {instance.model_id} to {instance.data}");
+                _logger.LogInformation(@$"Successfully set instance {instance.InstanceId}{instance.InstancePropertyPath}
+                    based on model {instance.ModelId} to {instance.Data}");
             }
-            catch (Exception ex) {
+            catch (Exception ex)
+            {
                 _logger.LogError(ex.Message);
                 throw;
             }