Refactor ApiSettings (stratisproject#993)

* Refactor ApiSettings

* Add comment

* Fix comment

* Fix test

* Fix tests

* Simplify

Author: quantumagi

src/Stratis.Bitcoin.Features.Api/ApiFeature.cs

@@ -7,6 +7,7 @@
 using NBitcoin;
 using Stratis.Bitcoin.Builder;
 using Stratis.Bitcoin.Builder.Feature;
+using Stratis.Bitcoin.Configuration.Settings;
 
 namespace Stratis.Bitcoin.Features.Api
 {
@@ -83,7 +84,7 @@ public override Task InitializeAsync()
         /// <param name="network">The network to extract values from.</param>
         public static void PrintHelp(Network network)
         {
-            ApiSettings.PrintHelp(network);
+            BaseSettings.PrintHelp(typeof(ApiSettings), network);
         }
 
         /// <summary>
@@ -93,7 +94,7 @@ public static void PrintHelp(Network network)
         /// <param name="network">The network to base the defaults off.</param>
         public static void BuildDefaultConfigurationFile(StringBuilder builder, Network network)
         {
-            ApiSettings.BuildDefaultConfigurationFile(builder, network);
+            BaseSettings.BuildDefaultConfigurationFile(typeof(ApiSettings), builder, network);
         }
 
         /// <inheritdoc />

src/Stratis.Bitcoin.Features.Api/ApiSettings.cs

@@ -1,31 +1,35 @@
 using System;
-using System.Text;
 using System.Timers;
-using Microsoft.Extensions.Logging;
-using NBitcoin;
 using Stratis.Bitcoin.Configuration;
-using Stratis.Bitcoin.Utilities;
+using Stratis.Bitcoin.Configuration.Settings;
 
 namespace Stratis.Bitcoin.Features.Api
 {
     /// <summary>
     /// Configuration related to the API interface.
     /// </summary>
-    public class ApiSettings
+    public class ApiSettings : BaseSettings
     {
         /// <summary>The default port used by the API when the node runs on the Stratis network.</summary>
         public const string DefaultApiHost = "http://localhost";
 
-        /// <summary>Instance logger.</summary>
-        private readonly ILogger logger;
-
         /// <summary>URI to node's API interface.</summary>
-        public Uri ApiUri { get; set; }
+        [CommandLineOption("apiuri", "URI to node's API interface.")]
+        private string ApiHost { get { return this.UseHttps ? this.apiHost.Replace(@"http://", @"https://") : this.apiHost; } set { this.apiHost = value; } }
+        private string apiHost = DefaultApiHost;
+
+        // If a port is set in the -apiuri, it takes precedence over the default port or the port passed in -apiport.
+        public Uri ApiUri { get { Uri uri = new Uri(this.ApiHost); return uri.IsDefaultPort ? new Uri($"{this.ApiHost}:{this.apiPort ?? this.nodeSettings.Network.DefaultAPIPort}") : uri; } }
 
         /// <summary>Port of node's API interface.</summary>
-        public int ApiPort { get; set; }
+        [CommandLineOption("apiport", "Port of node's API interface.")]
+        public int ApiPort { get { return this.ApiUri.Port; } set { this.apiPort = value; } }
+        private int? apiPort = null;
+
+        /// <summary>Sets the keepalive interval (set in seconds).</summary>
+        [CommandLineOption("keepalive", "Keep Alive interval (set in seconds).")]
+        private int KeepAlive { get; set; } = 0;
 
-        /// <summary>URI to node's API interface.</summary>
         public Timer KeepaliveTimer { get; private set; }
 
         /// <summary>
@@ -35,99 +39,31 @@ public class ApiSettings
         /// Password protected certificates are not supported. On MacOs, only p12 certificates can be used without password.
         /// Please refer to .Net Core documentation for usage: <seealso cref="https://docs.microsoft.com/en-us/dotnet/api/system.security.cryptography.x509certificates.x509certificate2.-ctor?view=netcore-2.1#System_Security_Cryptography_X509Certificates_X509Certificate2__ctor_System_Byte___" />.
         /// </remarks>
-        public string HttpsCertificateFilePath { get; set; }
+        [CommandLineOption("certificatefilepath", "Path to the certificate used for https traffic encryption. Password protected files are not supported. On MacOs, only p12 certificates can be used without password.", false)]
+        public string HttpsCertificateFilePath { get; set; } = null;
 
         /// <summary>Use HTTPS or not.</summary>
-        public bool UseHttps { get; set; }
+        [CommandLineOption("usehttps", "Use https protocol on the API.")]
+        public bool UseHttps { get; set; } = false;
 
         /// <summary>
         /// Initializes an instance of the object from the node configuration.
         /// </summary>
         /// <param name="nodeSettings">The node configuration.</param>
-        public ApiSettings(NodeSettings nodeSettings)
+        public ApiSettings(NodeSettings nodeSettings) : base(nodeSettings)
         {
-            Guard.NotNull(nodeSettings, nameof(nodeSettings));
-
-            this.logger = nodeSettings.LoggerFactory.CreateLogger(typeof(ApiSettings).FullName);
-
-            TextFileConfiguration config = nodeSettings.ConfigReader;
-
-            this.UseHttps = config.GetOrDefault("usehttps", false);
-            this.HttpsCertificateFilePath = config.GetOrDefault("certificatefilepath", (string)null);
-
             if (this.UseHttps && string.IsNullOrWhiteSpace(this.HttpsCertificateFilePath))
                 throw new ConfigurationException("The path to a certificate needs to be provided when using https. Please use the argument 'certificatefilepath' to provide it.");
 
-            var defaultApiHost = this.UseHttps
-                ? DefaultApiHost.Replace(@"http://", @"https://")
-                : DefaultApiHost;
-
-            string apiHost = config.GetOrDefault("apiuri", defaultApiHost, this.logger);
-            var apiUri = new Uri(apiHost);
-
-            // Find out which port should be used for the API.
-            int apiPort = config.GetOrDefault("apiport", nodeSettings.Network.DefaultAPIPort, this.logger);
-
-            // If no port is set in the API URI.
-            if (apiUri.IsDefaultPort)
-            {
-                this.ApiUri = new Uri($"{apiHost}:{apiPort}");
-                this.ApiPort = apiPort;
-            }
-            // If a port is set in the -apiuri, it takes precedence over the default port or the port passed in -apiport.
-            else
-            {
-                this.ApiUri = apiUri;
-                this.ApiPort = apiUri.Port;
-            }
-
             // Set the keepalive interval (set in seconds).
-            int keepAlive = config.GetOrDefault("keepalive", 0, this.logger);
-            if (keepAlive > 0)
+            if (this.KeepAlive > 0)
             {
                 this.KeepaliveTimer = new Timer
                 {
                     AutoReset = false,
-                    Interval = keepAlive * 1000
+                    Interval = this.KeepAlive * 1000
                 };
             }
         }
-
-        /// <summary>Prints the help information on how to configure the API settings to the logger.</summary>
-        /// <param name="network">The network to use.</param>
-        public static void PrintHelp(Network network)
-        {
-            var builder = new StringBuilder();
-
-            builder.AppendLine($"-apiuri=<string>               URI to node's API interface. Defaults to '{ DefaultApiHost }'.");
-            builder.AppendLine($"-apiport=<0-65535>             Port of node's API interface. Defaults to { network.DefaultAPIPort }.");
-            builder.AppendLine($"-keepalive=<seconds>           Keep Alive interval (set in seconds). Default: 0 (no keep alive).");
-            builder.AppendLine($"-usehttps=<bool>               Use https protocol on the API. Defaults to false.");
-            builder.AppendLine($"-certificatefilepath=<string>  Path to the certificate used for https traffic encryption. Defaults to <null>. Password protected files are not supported. On MacOs, only p12 certificates can be used without password.");
-
-            var logger = NodeSettings.Default(network).LoggerFactory.CreateLogger(typeof(ApiSettings).FullName);
-            logger.LogInformation(builder.ToString());            
-        }
-
-        /// <summary>
-        /// Get the default configuration.
-        /// </summary>
-        /// <param name="builder">The string builder to add the settings to.</param>
-        /// <param name="network">The network to base the defaults off.</param>
-        public static void BuildDefaultConfigurationFile(StringBuilder builder, Network network)
-        {
-            builder.AppendLine("####API Settings####");
-            builder.AppendLine($"#URI to node's API interface. Defaults to '{ DefaultApiHost }'.");
-            builder.AppendLine($"#apiuri={ DefaultApiHost }");
-            builder.AppendLine($"#Port of node's API interface. Defaults to { network.DefaultAPIPort }.");
-            builder.AppendLine($"#apiport={ network.DefaultAPIPort }");
-            builder.AppendLine($"#Keep Alive interval (set in seconds). Default: 0 (no keep alive).");
-            builder.AppendLine($"#keepalive=0");
-            builder.AppendLine($"#Use HTTPS protocol on the API. Default is false.");
-            builder.AppendLine($"#usehttps=false");
-            builder.AppendLine($"#Path to the file containing the certificate to use for https traffic encryption. Password protected files are not supported. On MacOs, only p12 certificates can be used without password.");
-            builder.AppendLine(@"#Please refer to .Net Core documentation for usage: 'https://docs.microsoft.com/en-us/dotnet/api/system.security.cryptography.x509certificates.x509certificate2.-ctor?view=netcore-2.1#System_Security_Cryptography_X509Certificates_X509Certificate2__ctor_System_Byte___'.");
-            builder.AppendLine($"#certificatefilepath=");
-        }
     }
 }

src/Stratis.Bitcoin/Configuration/Settings/BaseSettings.cs

@@ -56,6 +56,8 @@ private static string DefaultValue(object value, bool raw = false)
 
         private static Dictionary<Type, MethodInfo> typeGetter = new Dictionary<Type, MethodInfo>();
 
+        private static BindingFlags bindingFlags = BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public;
+
         public BaseSettings(NodeSettings nodeSettings)
         {
             Guard.NotNull(nodeSettings, nameof(nodeSettings));
@@ -66,8 +68,6 @@ public BaseSettings(NodeSettings nodeSettings)
 
             TextFileConfiguration config = nodeSettings.ConfigReader;
 
-            BindingFlags bindingFlags = BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public;
-
             foreach (PropertyInfo pi in this.GetType().GetProperties(bindingFlags))
             {
                 CommandLineOptionAttribute attr = Attribute.GetCustomAttributes(pi).OfType<CommandLineOptionAttribute>().FirstOrDefault();
@@ -100,7 +100,7 @@ public static void PrintHelp(Type settingsType, Network network)
             var builder = new StringBuilder();
             var defaultValues = Activator.CreateInstance(settingsType, new object[] { defaultSettings });
 
-            foreach (PropertyInfo pi in settingsType.GetProperties())
+            foreach (PropertyInfo pi in settingsType.GetProperties(bindingFlags))
             {
                 CommandLineOptionAttribute attr = Attribute.GetCustomAttributes(pi).OfType<CommandLineOptionAttribute>().FirstOrDefault();
                 if (attr == null)
@@ -130,7 +130,7 @@ public static void BuildDefaultConfigurationFile(Type settingsType, StringBuilde
             NodeSettings defaultSettings = NodeSettings.Default(network);
             var defaultValues = Activator.CreateInstance(settingsType, new object[] { defaultSettings });
 
-            foreach (PropertyInfo pi in settingsType.GetProperties())
+            foreach (PropertyInfo pi in settingsType.GetProperties(bindingFlags))
             {
                 CommandLineOptionAttribute attr = Attribute.GetCustomAttributes(pi).OfType<CommandLineOptionAttribute>().FirstOrDefault();
                 if (attr == null)

src/Stratis.Features.Unity3dApi/Unity3dApiFeature.cs

@@ -8,6 +8,7 @@
 using Stratis.Bitcoin;
 using Stratis.Bitcoin.Builder;
 using Stratis.Bitcoin.Builder.Feature;
+using Stratis.Bitcoin.Configuration.Settings;
 using Stratis.Bitcoin.Features.Api;
 using Stratis.Bitcoin.Features.Wallet.Controllers;
 using Stratis.Features.Unity3dApi.Controllers;
@@ -95,7 +96,7 @@ public override Task InitializeAsync()
         /// <param name="network">The network to extract values from.</param>
         public static void PrintHelp(Network network)
         {
-            ApiSettings.PrintHelp(network);
+            BaseSettings.PrintHelp(typeof(ApiSettings), network);
         }
 
         /// <summary>
@@ -105,7 +106,7 @@ public static void PrintHelp(Network network)
         /// <param name="network">The network to base the defaults off.</param>
         public static void BuildDefaultConfigurationFile(StringBuilder builder, Network network)
         {
-            ApiSettings.BuildDefaultConfigurationFile(builder, network);
+            BaseSettings.BuildDefaultConfigurationFile(typeof(ApiSettings), builder, network);
         }
 
         /// <inheritdoc />