Fixes bug in AppConfigHelper

I forgot to test actually not specifying a key for mp3splt in 28edc79 because I had the value set for the Windows settings. Also ensures only one warning is output. When Mp3splt not specified in settings.
QutEcoacoustics · Jun 21, 2018 · e33e445 · e33e445
1 parent 265fa0d
commit e33e445
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 18 deletions.
diff --git a/src/Acoustics.Shared/AppConfigHelper.cs b/src/Acoustics.Shared/AppConfigHelper.cs
@@ -17,6 +17,7 @@ namespace Acoustics.Shared
     using System.IO;
     using System.Linq;
     using System.Reflection;
+    using ConfigFile;
     using log4net;
 
     public static class AppConfigHelper
@@ -39,7 +40,6 @@ public static class AppConfigHelper
         private static readonly string ExecutingAssemblyPath =
             (Assembly.GetEntryAssembly() ?? Assembly.GetExecutingAssembly()).Location;
 
-
         private static readonly KeyValueConfigurationCollection SharedSettings;
 
         private static readonly ILog Log = LogManager.GetLogger(nameof(AppConfigHelper));
@@ -57,7 +57,6 @@ static AppConfigHelper()
 
             IsMono = Type.GetType("Mono.Runtime") != null;
             CheckOs(ref IsWindowsValue, ref IsLinuxValue, ref IsMacOsXValue);
-
         }
 
         public static string ExecutingAssemblyDirectory { get; } = Path.GetDirectoryName(ExecutingAssemblyPath);
@@ -190,7 +189,7 @@ public static string Mp3SpltExe
         {
             get
             {
-                return GetExeFile("AudioUtilityMp3SpltExe");
+                return GetExeFile("AudioUtilityMp3SpltExe", required: false);
             }
         }
 
@@ -201,7 +200,7 @@ public static string ShntoolExe
         {
             get
             {
-                return GetExeFile("AudioUtilityShntoolExe");
+                return GetExeFile("AudioUtilityShntoolExe", required: false);
             }
         }
 
@@ -334,14 +333,13 @@ public static DirectoryInfo AssemblyDir
 
         public static string GetString(string key)
         {
-            if (!Contains(key))
+            var found = TryGetString(key, out var value);
+
+            if (!found)
             {
-                //throw new ConfigurationErrorsException("Could not find key: " + key);
-                return null;
+                throw new ConfigurationErrorsException("Could not find key: " + key);
             }
 
-            var value = SharedSettings[key].Value;
-
             if (string.IsNullOrEmpty(value))
             {
                 throw new ConfigurationErrorsException("Found key, but it did not have a value: " + key);
@@ -350,6 +348,13 @@ public static string GetString(string key)
             return value;
         }
 
+        public static bool TryGetString(string key, out string value)
+        {
+            var found = Contains(key);
+            value = found ? SharedSettings[key].Value : null;
+            return found;
+        }
+
         public static bool Contains(string key)
         {
             return SharedSettings.AllKeys.Contains(key);
@@ -540,30 +545,38 @@ public static IEnumerable<DirectoryInfo> GetDirs(string key, bool checkAnyExist,
             return dirs;
         }
 
-        private static string GetExeFile(string appConfigKey)
+        private static string GetExeFile(string appConfigKey, bool required = true)
         {
-            string path = null;
-            string key = null;
+            string key;
 
             if (IsMacOsX)
             {
                 key = appConfigKey + "MacOsX";
-                path = GetString(key);
             }
             else if (IsLinux)
             {
                 key = appConfigKey + "Linux";
-                path = GetString(key);
             }
             else
             {
                 key = appConfigKey;
-                path = GetString(appConfigKey);
+            }
+
+            var found = TryGetString(key, out var path);
+
+            if (!found && required)
+            {
+                throw new ConfigFileException($"An exe path for `{key}` was not found in AP.Settings.config");
             }
 
             if (path.IsNullOrEmpty())
             {
-                Log.Warn($"No key found for `{key}` in the App.Config. This program may fail if this binary is needed.");
+                if (required)
+                {
+                    throw new ConfigFileException($"An exe path for `{key}` has an empty value set and it is required (in AP.Settings.config)");
+                }
+
+                Log.Debug($"No key found for `{key}` in the AP.Settings.config. This program may fail if this binary is needed.");
                 return null;
             }
 

diff --git a/src/Acoustics.Tools/Audio/MasterAudioUtility.cs b/src/Acoustics.Tools/Audio/MasterAudioUtility.cs
@@ -13,7 +13,7 @@ namespace Acoustics.Tools.Audio
     using System.Collections.Generic;
     using System.Globalization;
     using System.IO;
-
+    using System.Threading;
     using Shared;
     using Shared.Contracts;
 
@@ -22,6 +22,8 @@ namespace Acoustics.Tools.Audio
     /// </summary>
     public class MasterAudioUtility : AbstractAudioUtility, IAudioUtility
     {
+        private static bool missingMp3SpltWarned = false;
+
         private readonly WavPackAudioUtility wvunpackUtility;
 
         private readonly FfmpegAudioUtility ffmpegUtility;
@@ -108,8 +110,9 @@ private void Validate()
             Contract.RequiresNotNull(this.soxUtility, nameof(this.soxUtility));
             Contract.RequiresNotNull(this.TemporaryFilesDirectory, nameof(this.TemporaryFilesDirectory));
 
-            if (this.mp3SpltUtility == null)
+            if (this.mp3SpltUtility == null && !missingMp3SpltWarned)
             {
+                missingMp3SpltWarned = true;   
                 this.Log.Warn("No Mp3Splt utility provided. If you try to segment MP3 the program will fail.");
             }
         }