Merge pull request #91 from LM-Development/dev/fix-code-smells

Fixing a bunch of code smells

Author: InDieTasten

Samples/PublicSamples/RecordingBot/deploy/teams-recording-bot/Chart.yaml

@@ -16,12 +16,12 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 1.4.5
+version: 1.4.6
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
-appVersion: 1.3.4
+appVersion: 1.3.5
 
 dependencies:
   - name: ingress-nginx

Samples/PublicSamples/RecordingBot/src/RecordingBot.Model/Extension/HttpRequestExtensions.cs

@@ -12,8 +12,8 @@ public static class HttpRequestExtensions
 
         public static Uri GetUri(this HttpRequest request)
         {
-            ArgumentNullException.ThrowIfNull(request, nameof(request));
-            ArgumentException.ThrowIfNullOrWhiteSpace(request.Scheme, nameof(request.Scheme));
+            ArgumentNullException.ThrowIfNull(request);
+            ArgumentException.ThrowIfNullOrWhiteSpace(request.Scheme);
 
             return new Uri(GetUrl(request));
         }

Samples/PublicSamples/RecordingBot/src/RecordingBot.Model/Models/JoinURLResponse.cs

@@ -3,7 +3,7 @@
 
 namespace RecordingBot.Model.Models
 {
-    public partial class JoinURLResponse
+    public partial class JoinUrlResponse
     {
         [JsonProperty("callId")]
         public object CallId { get; set; }

Samples/PublicSamples/RecordingBot/src/RecordingBot.Services/Authentication/AuthenticationProvider.cs

@@ -129,13 +129,6 @@ public async Task<RequestValidationResult> ValidateInboundRequestAsync(HttpReque
                 JwtSecurityTokenHandler handler = new();
                 claimsPrincipal = handler.ValidateToken(token, validationParameters, out _);
             }
-
-            // Token expired... should somehow return 401 (Unauthorized)
-            // catch (SecurityTokenExpiredException ex)
-            // Tampered token
-            // catch (SecurityTokenInvalidSignatureException ex)
-            // Some other validation error
-            // catch (SecurityTokenValidationException ex)
             catch (Exception ex)
             {
                 // Some other error

Samples/PublicSamples/RecordingBot/src/RecordingBot.Services/Authentication/UserPasswordAuthenticationProvider.cs

@@ -6,22 +6,21 @@
 using System.Diagnostics;
 using System.Net.Http;
 using System.Net.Http.Headers;
+using System.Security.Authentication;
 using System.Threading.Tasks;
 
 namespace RecordingBot.Services.Authentication
 {
     public class UserPasswordAuthenticationProvider : ObjectRoot, IRequestAuthenticationProvider
     {
-        private readonly string _appName;
         private readonly string _appId;
         private readonly string _appSecret;
         private readonly string _userName;
         private readonly string _password;
 
-        public UserPasswordAuthenticationProvider(string appName, string appId, string appSecret, string userName, string password, IGraphLogger logger)
+        public UserPasswordAuthenticationProvider(string appId, string appSecret, string userName, string password, IGraphLogger logger)
             : base(logger.NotNull(nameof(logger)).CreateShim(nameof(UserPasswordAuthenticationProvider)))
         {
-            _appName = appName.NotNullOrWhitespace(nameof(appName));
             _appId = appId.NotNullOrWhitespace(nameof(appId));
             _appSecret = appSecret.NotNullOrWhitespace(nameof(appSecret));
 
@@ -30,16 +29,16 @@ public UserPasswordAuthenticationProvider(string appName, string appId, string a
         }
 
         /// <inheritdoc />
-        public async Task AuthenticateOutboundRequestAsync(HttpRequestMessage request, string tenantId)
+        public async Task AuthenticateOutboundRequestAsync(HttpRequestMessage request, string tenant)
         {
-            Debug.Assert(!string.IsNullOrWhiteSpace(tenantId), $"Invalid {nameof(tenantId)}.");
+            Debug.Assert(!string.IsNullOrWhiteSpace(tenant), $"Invalid {nameof(tenant)}.");
 
             const string BearerPrefix = "Bearer";
             const string ReplaceString = "{tenant}";
             const string TokenAuthorityMicrosoft = "https://login.microsoftonline.com/{tenant}/oauth2/v2.0/token";
             const string Resource = @"https://graph.microsoft.com/.default";
 
-            var tokenLink = TokenAuthorityMicrosoft.Replace(ReplaceString, tenantId);
+            var tokenLink = TokenAuthorityMicrosoft.Replace(ReplaceString, tenant);
             OAuthResponse authResult = null;
 
             try
@@ -57,7 +56,7 @@ public async Task AuthenticateOutboundRequestAsync(HttpRequestMessage request, s
 
                 if (!result.IsSuccessStatusCode)
                 {
-                    throw new Exception("Failed to generate user token.");
+                    throw new AuthenticationException("Failed to generate user token.");
                 }
 
                 var content = await result.Content.ReadAsStringAsync().ConfigureAwait(false);
@@ -81,10 +80,11 @@ public Task<RequestValidationResult> ValidateInboundRequestAsync(HttpRequestMess
             throw new NotImplementedException();
         }
 
-        private class OAuthResponse
+        [Serializable]
+        private sealed class OAuthResponse
         {
-            public string Access_Token { get; set; }
-            public int Expires_In { get; set; }
+            public string Access_Token { get; }
+            public int Expires_In { get; }
         }
     }
 }

Samples/PublicSamples/RecordingBot/src/RecordingBot.Services/Bot/CallHandler.cs

@@ -115,13 +115,10 @@ private async void CallOnUpdated(ICall sender, ResourceEventArgs<Call> e)
             // Event - Recording update e.g established/updated/start/ended
             _eventPublisher.Publish($"Call{e.NewResource.State}", $"Call.ID {Call.Id} Sender.Id {sender.Id} status updated to {e.NewResource.State} - {e.NewResource.ResultInfo?.Message}");
 
-            if (e.OldResource.State != e.NewResource.State && e.NewResource.State == CallState.Established)
+            if (e.OldResource.State != e.NewResource.State && e.NewResource.State == CallState.Established && !_isDisposed)
             {
-                if (!_isDisposed)
-                {
-                    // Call is established. We should start receiving Audio, we can inform clients that we have started recording.
-                    OnRecordingStatusFlip(sender);
-                }
+                // Call is established. We should start receiving Audio, we can inform clients that we have started recording.
+                OnRecordingStatusFlip(sender);
             }
 
             if ((e.OldResource.State == CallState.Established) && (e.NewResource.State == CallState.Terminated))
@@ -184,12 +181,9 @@ private void UpdateParticipants(ICollection<IParticipant> eventArgs, bool added
                 {
                     json = UpdateParticipant(BotMediaStream.participants, participant, added, participantDetails.DisplayName);
                 }
-                else if (participant.Resource.Info.Identity.AdditionalData?.Count > 0)
+                else if (participant.Resource.Info.Identity.AdditionalData?.Count > 0 && CheckParticipantIsUsable(participant))
                 {
-                    if (CheckParticipantIsUsable(participant))
-                    {
-                        json = UpdateParticipant(BotMediaStream.participants, participant, added);
-                    }
+                    json = UpdateParticipant(BotMediaStream.participants, participant, added);
                 }
 
                 if (json.Length > 0)

Samples/PublicSamples/RecordingBot/src/RecordingBot.Services/Bot/HeartbeatHandler.cs

@@ -10,7 +10,7 @@ public abstract class HeartbeatHandler : ObjectRootDisposable
     {
         private readonly Timer _heartbeatTimer;
 
-        public HeartbeatHandler(TimeSpan frequency, IGraphLogger logger)
+        protected HeartbeatHandler(TimeSpan frequency, IGraphLogger logger)
             : base(logger)
         {
             // initialize the timer

Samples/PublicSamples/RecordingBot/src/RecordingBot.Services/Contract/IEventPublisher.cs

@@ -2,6 +2,6 @@ namespace RecordingBot.Services.Contract
 {
     public interface IEventPublisher
     {
-        void Publish(string Subject, string Message, string TopicName = "");
+        void Publish(string subject, string message, string topicName = "");
     }
 }

Samples/PublicSamples/RecordingBot/src/RecordingBot.Services/Contract/IMediaStream.cs

@@ -7,7 +7,7 @@ namespace RecordingBot.Services.Contract
 {
     public interface IMediaStream
     {
-        Task AppendAudioBuffer(AudioMediaBuffer buffer, List<IParticipant> participant);
+        Task AppendAudioBuffer(AudioMediaBuffer buffer, List<IParticipant> participants);
         Task End();
     }
 }

Samples/PublicSamples/RecordingBot/src/RecordingBot.Services/Http/Controllers/DemoController.cs

@@ -5,6 +5,7 @@
 using RecordingBot.Services.ServiceSetup;
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading.Tasks;
 
 namespace RecordingBot.Services.Http.Controllers
@@ -37,9 +38,8 @@ public IActionResult OnGetCalls()
             _eventPublisher.Publish("GetCalls", "Getting calls");
 
             List<Dictionary<string, string>> calls = [];
-            foreach (var callHandler in _botService.CallHandlers.Values)
+            foreach (var call in _botService.CallHandlers.Values.Select(handler => handler.Call))
             {
-                var call = callHandler.Call;
                 var callPath = "/" + HttpRouteConstants.CALL_ROUTE.Replace("{callLegId}", call.Id);
                 var callUri = new Uri(_settings.CallControlBaseUrl, callPath).AbsoluteUri;
                 var values = new Dictionary<string, string>