Code review feedback changes

Author: ahmelsayed

src/WebJobs.Script.WebHost/App_Start/AutofacBootstrap.cs

@@ -1,9 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
-using System.Web.Http;
 using Autofac;
-using Autofac.Integration.WebApi;
 using Microsoft.Azure.WebJobs.Host;
 using Microsoft.Azure.WebJobs.Script.Config;
 using Microsoft.Azure.WebJobs.Script.WebHost.WebHooks;
@@ -12,7 +10,7 @@ namespace Microsoft.Azure.WebJobs.Script.WebHost
 {
     public static class AutofacBootstrap
     {
-        internal static void Initialize(ScriptSettingsManager settingsManager, ContainerBuilder builder, WebHostSettings settings, HttpConfiguration config)
+        internal static void Initialize(ScriptSettingsManager settingsManager, ContainerBuilder builder, WebHostSettings settings)
         {
             builder.RegisterInstance(settingsManager);
 
@@ -26,7 +24,6 @@ internal static void Initialize(ScriptSettingsManager settingsManager, Container
             builder.Register<WebScriptHostManager>(ct => ct.Resolve<WebHostResolver>().GetWebScriptHostManager(settings)).ExternallyOwned();
             builder.Register<WebHookReceiverManager>(ct => ct.Resolve<WebHostResolver>().GetWebHookReceiverManager(settings)).ExternallyOwned();
             builder.RegisterInstance(settings);
-            builder.RegisterHttpRequestMessage(config);
         }
     }
 }

src/WebJobs.Script.WebHost/App_Start/WebApiConfig.cs

@@ -40,7 +40,7 @@ public static void Register(HttpConfiguration config, ScriptSettingsManager sett
 
             var builder = new ContainerBuilder();
             builder.RegisterApiControllers(typeof(FunctionsController).Assembly);
-            AutofacBootstrap.Initialize(settingsManager, builder, settings, config);
+            AutofacBootstrap.Initialize(settingsManager, builder, settings);
 
             // Invoke registration callback
             dependencyCallback?.Invoke(builder, settings);
@@ -93,7 +93,6 @@ private static WebHostSettings GetDefaultSettings(ScriptSettingsManager settings
                 settings.ScriptPath = settingsManager.GetSetting(EnvironmentSettingNames.AzureWebJobsScriptRoot);
                 settings.LogPath = Path.Combine(Path.GetTempPath(), @"Functions");
                 settings.SecretsPath = HttpContext.Current.Server.MapPath("~/App_Data/Secrets");
-                settings.IsSelfHost = true;
             }
             else
             {

src/WebJobs.Script.WebHost/App_Start/WebHostResolver.cs

@@ -151,14 +151,8 @@ private static ScriptHostConfiguration CreateScriptHostConfiguration(WebHostSett
             };
 
             // If running on Azure Web App, derive the host ID from the default subdomain
-            // Otherwise, derive it from machine name and folder name
             string hostId = _settingsManager.AzureWebsiteDefaultSubdomain;
 
-            if (string.IsNullOrEmpty(hostId))
-            {
-                hostId = MakeValidHostId($"{Environment.MachineName}-{Path.GetFileName(Environment.CurrentDirectory)}");
-            }
-
             if (!String.IsNullOrEmpty(hostId))
             {
                 // Truncate to the max host name length if needed
@@ -177,51 +171,6 @@ private static ScriptHostConfiguration CreateScriptHostConfiguration(WebHostSett
             return scriptHostConfig;
         }
 
-        private static string MakeValidHostId(string id)
-        {
-            var sb = new StringBuilder();
-
-            //filter for valid characters
-            foreach (var c in id)
-            {
-                if (c == '-')
-                {
-                    //dashes are valid
-                    //but it cannot start with one
-                    //nor can it have consecutive dashes
-                    if (sb.Length != 0 && sb[sb.Length - 1] != '-')
-                    {
-                        sb.Append(c);
-                    }
-                }
-                else if (char.IsDigit(c))
-                {
-                    //digits are valid
-                    sb.Append(c);
-                }
-                else if (char.IsLetter(c))
-                {
-                    //letters are valid but must be lowercase
-                    sb.Append(char.ToLowerInvariant(c));
-                }
-            }
-
-            //it cannot end with a dash
-            if (sb.Length > 0 && sb[sb.Length - 1] == '-')
-            {
-                sb.Length -= 1;
-            }
-
-            //length cannot exceed 32
-            const int MaximumHostIdLength = 32;
-            if (sb.Length > MaximumHostIdLength)
-            {
-                sb.Length = MaximumHostIdLength;
-            }
-
-            return sb.ToString();
-        }
-
         private static void InitializeFileSystem(string scriptPath)
         {
             if (ScriptSettingsManager.Instance.IsAzureEnvironment)

src/WebJobs.Script.WebHost/App_Start/WebHostSettings.cs

@@ -2,28 +2,22 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using Microsoft.Azure.WebJobs.Host;
-using Newtonsoft.Json;
 
 namespace Microsoft.Azure.WebJobs.Script.WebHost
 {
     public class WebHostSettings
     {
-        [JsonProperty("isSelfHost")]
         public bool IsSelfHost { get; set; }
 
-        [JsonProperty("scriptPath")]
         public string ScriptPath { get; set; }
 
-        [JsonProperty("logPath")]
         public string LogPath { get; set; }
 
-        [JsonProperty("secretsPath")]
         public string SecretsPath { get; set; }
 
-        [JsonProperty("nodeDebugPort")]
-        public int NodeDebugPort { get; set; }
+        // Used by Cli to disable auth when running locally.
+        public bool IsAuthDisabled { get; set; } = false;
 
-        [JsonIgnore]
         public TraceWriter TraceWriter { get; set; }
     }
 }

src/WebJobs.Script.WebHost/Controllers/AdminController.cs

@@ -62,12 +62,8 @@ public HttpResponseMessage Invoke(string name, [FromBody] FunctionInvocation inv
         [Route("admin/functions/{name}/status")]
         public FunctionStatus GetFunctionStatus(string name)
         {
+            FunctionStatus status = new FunctionStatus();
             Collection<string> functionErrors = null;
-            FunctionDescriptor function = _scriptHostManager.Instance.Functions.FirstOrDefault(p => p.Name.ToLowerInvariant() == name.ToLowerInvariant());
-            FunctionStatus status = new FunctionStatus
-            {
-                Metadata = function?.Metadata
-            };
 
             // first see if the function has any errors
             if (_scriptHostManager.Instance.FunctionErrors.TryGetValue(name, out functionErrors))
@@ -78,6 +74,7 @@ public FunctionStatus GetFunctionStatus(string name)
             {
                 // if we don't have any errors registered, make sure the function exists
                 // before returning empty errors
+                FunctionDescriptor function = _scriptHostManager.Instance.Functions.FirstOrDefault(p => p.Name.ToLowerInvariant() == name.ToLowerInvariant());
                 if (function == null)
                 {
                     throw new HttpResponseException(HttpStatusCode.NotFound);
@@ -93,10 +90,7 @@ public HostStatus GetHostStatus()
         {
             HostStatus status = new HostStatus
             {
-                Id = _scriptHostManager.Instance?.ScriptConfig.HostConfig.HostId,
-                WebHostSettings = _webHostSettings,
-                ProcessId = Process.GetCurrentProcess().Id,
-                IsDebuggerAttached = Debugger.IsAttached
+                Id = _scriptHostManager.Instance?.ScriptConfig.HostConfig.HostId
             };
 
             var lastError = _scriptHostManager.LastError;
@@ -111,13 +105,21 @@ public HostStatus GetHostStatus()
 
         [HttpPost]
         [Route("admin/host/debug")]
-        public bool LaunchDebugger()
+        public HttpResponseMessage LaunchDebugger()
         {
             if (_webHostSettings.IsSelfHost)
             {
-                return Debugger.Launch();
+                // If debugger is already running, this will be a no-op returning true.
+                if (Debugger.Launch())
+                {
+                    return new HttpResponseMessage(HttpStatusCode.OK);
+                }
+                else
+                {
+                    return new HttpResponseMessage(HttpStatusCode.Conflict);
+                }
             }
-            return false;
+            return new HttpResponseMessage(HttpStatusCode.NotImplemented);
         }
 
         public override Task<HttpResponseMessage> ExecuteAsync(HttpControllerContext controllerContext, CancellationToken cancellationToken)

src/WebJobs.Script.WebHost/Controllers/FunctionsController.cs

@@ -44,7 +44,7 @@ public override async Task<HttpResponseMessage> ExecuteAsync(HttpControllerConte
             // Determine the authorization level of the request
             ISecretManager secretManager = controllerContext.Configuration.DependencyResolver.GetService<ISecretManager>();
             var settings = controllerContext.Configuration.DependencyResolver.GetService<WebHostSettings>();
-            var authorizationLevel = settings.IsSelfHost
+            var authorizationLevel = settings.IsAuthDisabled
                 ? AuthorizationLevel.Admin
                 : await AuthorizationLevelAttribute.GetAuthorizationLevelAsync(request, secretManager, functionName: function.Name);
 

src/WebJobs.Script.WebHost/Filters/AuthorizationLevelAttribute.cs

@@ -34,7 +34,7 @@ public async override Task OnAuthorizationAsync(HttpActionContext actionContext,
             ISecretManager secretManager = actionContext.ControllerContext.Configuration.DependencyResolver.GetService<ISecretManager>();
             var settings = actionContext.ControllerContext.Configuration.DependencyResolver.GetService<WebHostSettings>();
 
-            if (!settings.IsSelfHost && !await IsAuthorizedAsync(actionContext.Request, Level, secretManager))
+            if (!settings.IsAuthDisabled && !await IsAuthorizedAsync(actionContext.Request, Level, secretManager))
             {
                 actionContext.Response = new HttpResponseMessage(HttpStatusCode.Unauthorized);
             }

src/WebJobs.Script.WebHost/Models/FunctionStatus.cs

@@ -14,8 +14,5 @@ public class FunctionStatus
         /// </summary>
         [JsonProperty(PropertyName = "errors", DefaultValueHandling = DefaultValueHandling.Ignore)]
         public Collection<string> Errors { get; set; }
-
-        [JsonProperty(PropertyName = "metadata", DefaultValueHandling = DefaultValueHandling.Ignore)]
-        public FunctionMetadata Metadata { get; set; }
     }
 }

src/WebJobs.Script.WebHost/Models/HostStatus.cs

@@ -31,29 +31,5 @@ public HostStatus()
         /// </summary>
         [JsonProperty(PropertyName = "errors", DefaultValueHandling = DefaultValueHandling.Ignore)]
         public Collection<string> Errors { get; set; }
-
-        /// <summary>
-        /// Gets or sets the web host settings.
-        /// </summary>
-        [JsonProperty(PropertyName = "webHostSettings", DefaultValueHandling = DefaultValueHandling.Ignore)]
-        public WebHostSettings WebHostSettings { get; set; }
-
-        /// <summary>
-        /// Gets or sets the processId for the host.
-        /// </summary>
-        [JsonProperty(PropertyName = "processId", DefaultValueHandling = DefaultValueHandling.Ignore)]
-        public int ProcessId { get; set; }
-
-        /// <summary>
-        /// Gets or sets whether the process has a debugger attached or not.
-        /// </summary>
-        [JsonProperty(PropertyName = "isDebuggerAttached", DefaultValueHandling = DefaultValueHandling.Ignore)]
-        public bool IsDebuggerAttached { get; set; }
-
-        internal static string GetAssemblyFileVersion(Assembly assembly)
-        {
-            AssemblyFileVersionAttribute fileVersionAttr = assembly.GetCustomAttribute<AssemblyFileVersionAttribute>();
-            return fileVersionAttr?.Version ?? "Unknown";
-        }
     }
 }

src/WebJobs.Script/Host/ScriptHost.cs

@@ -406,9 +406,9 @@ private void RestartHost()
 #endif
         }
 
-            /// <summary>
-            /// Whenever the debug marker file changes we update our debug timeout
-            /// </summary>
+        /// <summary>
+        /// Whenever the debug marker file changes we update our debug timeout
+        /// </summary>
         private void OnDebugModeFileChanged(object sender, FileSystemEventArgs e)
         {
             LastDebugNotify = DateTime.UtcNow;