Address SonarQube issues (#66)

* gh-65 address SonarQube issues

Signed-off-by: Victor Chang <[email protected]>

Author: mocsharp

.editorconfig

@@ -4,10 +4,9 @@
 root = true
 
 # Copyright File Header
-file_header_template                = Copyright [year file created] - [last year file modified], MONAI Consortium\nSPDX-License-Identifier: Apache License 2.0
+file_header_template                = SPDX-FileCopyrightText: © [year file created] - [last year file modified], MONAI Consortium\nSPDX-License-Identifier: Apache License 2.0
 dotnet_diagnostic.IDE0073.severity  = error
 
-
 # Default settings:
 # A newline ending every file
 # Use 4 spaces as indentation

.github/workflows/ci.yml

@@ -9,9 +9,6 @@ on:
   push:
     paths-ignore:
       - 'demos/**'
-  pull_request:
-    paths-ignore:
-      - 'demos/**'
 
   # Allows you to run this workflow manually from the Actions tab
   workflow_dispatch:
@@ -90,19 +87,15 @@ jobs:
 
     - name: Perform CodeQL Analysis
       uses: github/codeql-action/analyze@v1
+      
 
   unit-test:
-    runs-on: ${{ matrix.os }}
-    strategy:
-      matrix:
-        os: [ubuntu-latest]
-      fail-fast: true
-
+    runs-on: windows-latest
     steps:
-      - name: Checkout repository
-        uses: actions/checkout@v2
+      - name: Set up JDK 11
+        uses: actions/setup-java@v1
         with:
-          fetch-depth: 0
+          java-version: 1.11
 
       - uses: actions/setup-dotnet@v1
         with:
@@ -116,67 +109,57 @@ jobs:
           restore-keys: |
             ${{ runner.os }}-nuget
 
-      - name: Restore dependencies
-        run: dotnet restore
-        working-directory: ./src
-
-      - name: Build Solution
-        run: dotnet build -c ${{ env.BUILD_CONFIG }} --nologo ${{ env.SOLUTION }}
-        working-directory: ./src
-
-      - name: Run Unit Test
-        run: dotnet test --filter FullyQualifiedName\!~Monai.Deploy.InformaticsGateway.Integration.Test  -c ${{ env.BUILD_CONFIG }} -v=minimal --results-directory "${{ env.TEST_RESULTS }}" --collect:"XPlat Code Coverage" --settings coverlet.runsettings ${{ env.SOLUTION }}
-        working-directory: ./src
-
-      - uses: codecov/codecov-action@v2
-        with:
-          token: ${{ secrets.CODECOV_TOKEN }}
-          directory: "src/${{ env.TEST_RESULTS }}"
-          files: "**/coverage.opencover.xml"
-          flags: unittests
-          name: codecov-umbrella
-          fail_ci_if_error: true
-          verbose: true
-
-  sonar-qube:
-    runs-on: windows-latest
-    steps:
-      - name: Set up JDK 11
-        uses: actions/setup-java@v1
-        with:
-          java-version: 1.11
       - uses: actions/checkout@v2
         with:
           fetch-depth: 0
+
       - name: Cache SonarCloud packages
         uses: actions/cache@v1
         with:
           path: ~\sonar\cache
           key: ${{ runner.os }}-sonar
           restore-keys: ${{ runner.os }}-sonar
+
       - name: Cache SonarCloud scanner
         id: cache-sonar-scanner
         uses: actions/cache@v1
         with:
           path: .\.sonar\scanner
           key: ${{ runner.os }}-sonar-scanner
           restore-keys: ${{ runner.os }}-sonar-scanner
+
       - name: Install SonarCloud scanner
         if: steps.cache-sonar-scanner.outputs.cache-hit != 'true'
         shell: powershell
         run: |
           New-Item -Path .\.sonar\scanner -ItemType Directory
-          dotnet tool update dotnet-sonarscanner --tool-path .\.sonar\scanner
+          dotnet tool update dotnet-sonarscanner --tool-path .\.sonar\scanner          
+
+      - name: Restore dependencies
+        run: dotnet restore
+        working-directory: ./src
+
       - name: Build and analyze
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
         shell: powershell
         run: |
-          .\.sonar\scanner\dotnet-sonarscanner begin /k:"Project-MONAI_monai-deploy-informatics-gateway" /o:"project-monai" /d:sonar.login="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="https://sonarcloud.io"
+          .\.sonar\scanner\dotnet-sonarscanner begin /k:"Project-MONAI_monai-deploy-informatics-gateway" /o:"project-monai" /d:sonar.login="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.cs.opencover.reportsPaths="src\${{ env.TEST_RESULTS }}\**\*.xml"
           dotnet build -c ${{ env.BUILD_CONFIG }} --nologo "src\${{ env.SOLUTION }}"
+          dotnet test --no-build --filter FullyQualifiedName\!~Monai.Deploy.InformaticsGateway.Integration.Test -c ${{ env.BUILD_CONFIG }} -v=minimal --results-directory "src\${{ env.TEST_RESULTS }}" --collect:"XPlat Code Coverage" --settings src\coverlet.runsettings "src\${{ env.SOLUTION }}"
           .\.sonar\scanner\dotnet-sonarscanner end /d:sonar.login="${{ secrets.SONAR_TOKEN }}"
 
+      - uses: codecov/codecov-action@v2
+        with:
+          token: ${{ secrets.CODECOV_TOKEN }}
+          directory: "src/${{ env.TEST_RESULTS }}"
+          files: "**/coverage.opencover.xml"
+          flags: unittests
+          name: codecov-umbrella
+          fail_ci_if_error: true
+          verbose: true
+
   build:
     runs-on: ${{ matrix.os }}
     needs: [calc-version]

src/Api/LoggingDataDictionary.cs

@@ -2,13 +2,24 @@
 // SPDX-FileCopyrightText: © 2021 NVIDIA Corporation
 // SPDX-License-Identifier: Apache License 2.0
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Runtime.Serialization;
 
 namespace Monai.Deploy.InformaticsGateway.Api
 {
+    [Serializable]
     public class LoggingDataDictionary<K, V> : Dictionary<K, V>
     {
+        public LoggingDataDictionary()
+        {
+        }
+
+        protected LoggingDataDictionary(SerializationInfo info, StreamingContext context) : base(info, context)
+        {
+        }
+
         public override string ToString()
         {
             var pairs = this.Select(x => string.Format("{0}={1}", x.Key, x.Value));

src/Api/MessageBroker/JsonMessage.cs

@@ -9,8 +9,6 @@ namespace Monai.Deploy.InformaticsGateway.Api.MessageBroker
 {
     public sealed class JsonMessage<T> : MessageBase
     {
-        public static readonly string JsonApplicationType = "application/json";
-
         /// <summary>
         /// Body of the message.
         /// </summary>
@@ -65,7 +63,7 @@ public JsonMessage(T body,
             MessageDescription = bodyDescription;
             MessageId = messageId;
             ApplicationId = applicationId;
-            ContentType = JsonApplicationType;
+            ContentType = MessageContentTypes.JsonApplicationType;
             CorrelationId = correlationId;
             CreationDateTime = creationDateTime;
             DeliveryTag = deliveryTag;

src/Api/MessageBroker/MessageContentTypes.cs

@@ -0,0 +1,10 @@
+// SPDX-FileCopyrightText: © 2021-2022 MONAI Consortium
+// SPDX-License-Identifier: Apache License 2.0
+
+namespace Monai.Deploy.InformaticsGateway.Api.MessageBroker
+{
+    public static class MessageContentTypes
+    {
+        public static readonly string JsonApplicationType = "application/json";
+    }
+}

src/Api/Rest/InferenceRequest.cs

@@ -215,17 +215,23 @@ private bool Validate(out string details)
                 errors.Add("'transactionId' is required.");
             }
 
-            if (InputResources.IsNullOrEmpty() ||
-                InputResources.Count(predicate => predicate.Interface != InputInterfaceType.Algorithm) == 0)
-            {
-                errors.Add("No 'inputResources' specified.");
-            }
-
             if (Application is null)
             {
                 errors.Add("No algorithm defined or more than one algorithms defined in 'inputResources'.  'inputResources' must include one algorithm/pipeline for the inference request.");
             }
 
+            ValidateInputResources(errors);
+            ValidateInputMetadata(errors);
+            ValidateOUtputResources(errors);
+
+            details = string.Join(" ", errors);
+            return errors.Count == 0;
+        }
+
+        private void ValidateOUtputResources(List<string> errors)
+        {
+            Guard.Against.Null(errors, nameof(errors));
+
             if (InputMetadata.Inputs.IsNullOrEmpty())
             {
                 errors.Add("Request has no `inputMetadata` defined. At least one `inputs` or `inputMetadata` required.");
@@ -237,18 +243,11 @@ private bool Validate(out string details)
                     CheckInputMetadataDetails(inputDetails, errors);
                 }
             }
+        }
 
-            foreach (var input in InputResources)
-            {
-                if (input.Interface == InputInterfaceType.DicomWeb)
-                {
-                    CheckDicomWebConnectionDetails("inputResources", errors, input.ConnectionDetails);
-                }
-                else if (input.Interface == InputInterfaceType.Fhir)
-                {
-                    CheckFhirConnectionDetails("inputResources", errors, input.ConnectionDetails);
-                }
-            }
+        private void ValidateInputMetadata(List<string> errors)
+        {
+            Guard.Against.Null(errors, nameof(errors));
 
             foreach (var output in OutputResources)
             {
@@ -261,51 +260,37 @@ private bool Validate(out string details)
                     CheckFhirConnectionDetails("outputResources", errors, output.ConnectionDetails);
                 }
             }
+        }
 
-            details = string.Join(" ", errors);
-            return errors.Count == 0;
+        private void ValidateInputResources(List<string> errors)
+        {
+            Guard.Against.Null(errors, nameof(errors));
+
+            if (InputResources.IsNullOrEmpty() ||
+                !InputResources.Any(predicate => predicate.Interface != InputInterfaceType.Algorithm))
+            {
+                errors.Add("No 'inputResources' specified.");
+            }
+
+            foreach (var input in InputResources)
+            {
+                if (input.Interface == InputInterfaceType.DicomWeb)
+                {
+                    CheckDicomWebConnectionDetails("inputResources", errors, input.ConnectionDetails);
+                }
+                else if (input.Interface == InputInterfaceType.Fhir)
+                {
+                    CheckFhirConnectionDetails("inputResources", errors, input.ConnectionDetails);
+                }
+            }
         }
 
         private void CheckInputMetadataDetails(InferenceRequestDetails details, List<string> errors)
         {
             switch (details.Type)
             {
                 case InferenceRequestType.DicomUid:
-                    if (details.Studies.IsNullOrEmpty())
-                    {
-                        errors.Add("Request type is set to `DICOM_UID` but no `studies` defined.");
-                    }
-                    else
-                    {
-                        foreach (var study in details.Studies)
-                        {
-                            if (string.IsNullOrWhiteSpace(study.StudyInstanceUid))
-                            {
-                                errors.Add("`StudyInstanceUID` cannot be empty.");
-                            }
-
-                            if (study.Series is null) continue;
-
-                            foreach (var series in study.Series)
-                            {
-                                if (string.IsNullOrWhiteSpace(series.SeriesInstanceUid))
-                                {
-                                    errors.Add("`SeriesInstanceUID` cannot be empty.");
-                                }
-
-                                if (series.Instances is null) continue;
-
-                                foreach (var instance in series.Instances)
-                                {
-                                    if (instance.SopInstanceUid.IsNullOrEmpty() ||
-                                        instance.SopInstanceUid.Any(p => string.IsNullOrWhiteSpace(p)))
-                                    {
-                                        errors.Add("`SOPInstanceUID` cannot be empty.");
-                                    }
-                                }
-                            }
-                        }
-                    }
+                    CheckInputMetadataWithTypDeicomUid(details, errors);
                     break;
 
                 case InferenceRequestType.DicomPatientId:
@@ -323,25 +308,66 @@ private void CheckInputMetadataDetails(InferenceRequestDetails details, List<str
                     break;
 
                 case InferenceRequestType.FhireResource:
-                    if (details.Resources.IsNullOrEmpty())
+                    CheckInputMetadataWithTypeFhirResource(details, errors);
+                    break;
+
+                default:
+                    errors.Add($"'inputMetadata' does not yet support type '{details.Type}'.");
+                    break;
+            }
+        }
+
+        private static void CheckInputMetadataWithTypeFhirResource(InferenceRequestDetails details, List<string> errors)
+        {
+            Guard.Against.Null(details, nameof(details));
+            Guard.Against.Null(errors, nameof(errors));
+
+            if (details.Resources.IsNullOrEmpty())
+            {
+                errors.Add("Request type is set to `FHIR_RESOURCE` but no FHIR `resources` defined.");
+            }
+            else
+            {
+                errors.AddRange(details.Resources.Where(resource => string.IsNullOrWhiteSpace(resource.Type)).Select(resource => "A FHIR resource type cannot be empty."));
+            }
+        }
+
+        private static void CheckInputMetadataWithTypDeicomUid(InferenceRequestDetails details, List<string> errors)
+        {
+            Guard.Against.Null(details, nameof(details));
+            Guard.Against.Null(errors, nameof(errors));
+
+            if (details.Studies.IsNullOrEmpty())
+            {
+                errors.Add("Request type is set to `DICOM_UID` but no `studies` defined.");
+            }
+            else
+            {
+                foreach (var study in details.Studies)
+                {
+                    if (string.IsNullOrWhiteSpace(study.StudyInstanceUid))
                     {
-                        errors.Add("Request type is set to `FHIR_RESOURCE` but no FHIR `resources` defined.");
+                        errors.Add("`StudyInstanceUID` cannot be empty.");
                     }
-                    else
+
+                    if (study.Series is null) continue;
+
+                    foreach (var series in study.Series)
                     {
-                        foreach (var resource in details.Resources)
+                        if (string.IsNullOrWhiteSpace(series.SeriesInstanceUid))
                         {
-                            if (string.IsNullOrWhiteSpace(resource.Type))
-                            {
-                                errors.Add("A FHIR resource type cannot be empty.");
-                            }
+                            errors.Add("`SeriesInstanceUID` cannot be empty.");
                         }
-                    }
-                    break;
 
-                default:
-                    errors.Add($"'inputMetadata' does not yet support type '{details.Type}'.");
-                    break;
+                        if (series.Instances is null) continue;
+                        errors.AddRange(
+                            series.Instances
+                                .Where(
+                                    instance => instance.SopInstanceUid.IsNullOrEmpty() ||
+                                    instance.SopInstanceUid.Any(p => string.IsNullOrWhiteSpace(p)))
+                                .Select(instance => "`SOPInstanceUID` cannot be empty."));
+                    }
+                }
             }
         }