Skip to content

Commit f0d954b

Browse files
SteveSandersonMSSteveSandersonMS
committed
Design review: Change AddNodeServices to take an Action<NodeServicesOptions> like other aspects of MVC DI config
1 parent f04fb8c commit f0d954b

11 files changed

Lines changed: 150 additions & 141 deletions

File tree

‎samples/misc/LatencyTest/Program.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ public class Program
1515
public static void Main(string[] args) {
1616
// Set up the DI system
1717
var services = new ServiceCollection();
18-
services.AddNodeServices(new NodeServicesOptions {
19-
HostingModel = NodeServicesOptions.DefaultNodeHostingModel,
20-
ProjectPath = Directory.GetCurrentDirectory(),
21-
WatchFileExtensions = new string[] {} // Don't watch anything
18+
services.AddNodeServices(options => {
19+
options.HostingModel = NodeServicesOptions.DefaultNodeHostingModel;
20+
options.ProjectPath = Directory.GetCurrentDirectory();
21+
options.WatchFileExtensions = new string[] {}; // Don't watch anything
2222
});
2323
var serviceProvider = services.BuildServiceProvider();
2424

‎src/Microsoft.AspNetCore.NodeServices/Configuration/Configuration.cs‎

Lines changed: 0 additions & 91 deletions
This file was deleted.

‎src/Microsoft.AspNetCore.NodeServices/Configuration/NodeServicesFactory.cs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
using System;
2+
using Microsoft.AspNetCore.NodeServices.HostingModels;
3+
4+
namespace Microsoft.AspNetCore.NodeServices
5+
{
6+
public static class NodeServicesFactory
7+
{
8+
public static INodeServices CreateNodeServices(NodeServicesOptions options)
9+
{
10+
if (options == null)
11+
{
12+
throw new ArgumentNullException(nameof (options));
13+
}
14+
15+
return new NodeServicesImpl(() => CreateNodeInstance(options));
16+
}
17+
18+
private static INodeInstance CreateNodeInstance(NodeServicesOptions options)
19+
{
20+
if (options.NodeInstanceFactory != null)
21+
{
22+
// If you've explicitly supplied an INodeInstance factory, we'll use that. This is useful for
23+
// custom INodeInstance implementations.
24+
return options.NodeInstanceFactory();
25+
}
26+
else
27+
{
28+
switch (options.HostingModel)
29+
{
30+
case NodeHostingModel.Http:
31+
return new HttpNodeInstance(options.ProjectPath, options.WatchFileExtensions, options.NodeInstanceOutputLogger,
32+
options.EnvironmentVariables, options.LaunchWithDebugging, options.DebuggingPort, /* port */ 0);
33+
case NodeHostingModel.Socket:
34+
var pipeName = "pni-" + Guid.NewGuid().ToString("D"); // Arbitrary non-clashing string
35+
return new SocketNodeInstance(options.ProjectPath, options.WatchFileExtensions, pipeName, options.NodeInstanceOutputLogger,
36+
options.EnvironmentVariables, options.LaunchWithDebugging, options.DebuggingPort);
37+
default:
38+
throw new ArgumentException("Unknown hosting model: " + options.HostingModel);
39+
}
40+
}
41+
}
42+
}
43+
}

‎src/Microsoft.AspNetCore.NodeServices/Configuration/NodeServicesOptions.cs‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,28 +2,52 @@
22
using System.Collections.Generic;
33
using Microsoft.AspNetCore.NodeServices.HostingModels;
44
using Microsoft.Extensions.Logging;
5+
using Microsoft.Extensions.DependencyInjection;
6+
using Microsoft.AspNetCore.Hosting;
7+
using Microsoft.Extensions.Logging.Console;
58

69
namespace Microsoft.AspNetCore.NodeServices
710
{
811
public class NodeServicesOptions
912
{
1013
public const NodeHostingModel DefaultNodeHostingModel = NodeHostingModel.Http;
11-
14+
private const string LogCategoryName = "Microsoft.AspNetCore.NodeServices";
1215
private static readonly string[] DefaultWatchFileExtensions = { ".js", ".jsx", ".ts", ".tsx", ".json", ".html" };
1316

14-
public NodeServicesOptions()
17+
public NodeServicesOptions(IServiceProvider serviceProvider)
1518
{
19+
if (serviceProvider == null)
20+
{
21+
throw new ArgumentNullException(nameof (serviceProvider));
22+
}
23+
24+
EnvironmentVariables = new Dictionary<string, string>();
1625
HostingModel = DefaultNodeHostingModel;
1726
WatchFileExtensions = (string[])DefaultWatchFileExtensions.Clone();
27+
28+
// In an ASP.NET environment, we can use the IHostingEnvironment data to auto-populate a few
29+
// things that you'd otherwise have to specify manually
30+
var hostEnv = serviceProvider.GetService<IHostingEnvironment>();
31+
if (hostEnv != null)
32+
{
33+
ProjectPath = hostEnv.ContentRootPath;
34+
EnvironmentVariables["NODE_ENV"] = hostEnv.IsDevelopment() ? "development" : "production"; // De-facto standard values for Node
35+
}
36+
37+
// If the DI system gives us a logger, use it. Otherwise, set up a default one.
38+
var loggerFactory = serviceProvider.GetService<ILoggerFactory>();
39+
NodeInstanceOutputLogger = loggerFactory != null
40+
? loggerFactory.CreateLogger(LogCategoryName)
41+
: new ConsoleLogger(LogCategoryName, null, false);
1842
}
19-
public Action<System.Diagnostics.ProcessStartInfo> OnBeforeStartExternalProcess { get; set; }
43+
2044
public NodeHostingModel HostingModel { get; set; }
2145
public Func<INodeInstance> NodeInstanceFactory { get; set; }
2246
public string ProjectPath { get; set; }
2347
public string[] WatchFileExtensions { get; set; }
2448
public ILogger NodeInstanceOutputLogger { get; set; }
2549
public bool LaunchWithDebugging { get; set; }
2650
public IDictionary<string, string> EnvironmentVariables { get; set; }
27-
public int? DebuggingPort { get; set; }
51+
public int DebuggingPort { get; set; }
2852
}
2953
}

‎src/Microsoft.AspNetCore.NodeServices/Configuration/NodeServicesServiceCollectionExtensions.cs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
using System;
2+
using Microsoft.AspNetCore.NodeServices;
3+
4+
namespace Microsoft.Extensions.DependencyInjection
5+
{
6+
/// <summary>
7+
/// Extension methods for setting up NodeServices in an <see cref="IServiceCollection" />.
8+
/// </summary>
9+
public static class NodeServicesServiceCollectionExtensions
10+
{
11+
public static void AddNodeServices(this IServiceCollection serviceCollection)
12+
=> AddNodeServices(serviceCollection, _ => {});
13+
14+
[Obsolete("Use the AddNodeServices(Action<NodeServicesOptions> setupAction) overload instead.")]
15+
public static void AddNodeServices(this IServiceCollection serviceCollection, NodeServicesOptions options)
16+
{
17+
serviceCollection.AddSingleton(typeof (INodeServices), _ =>
18+
{
19+
return NodeServicesFactory.CreateNodeServices(options);
20+
});
21+
}
22+
23+
public static void AddNodeServices(this IServiceCollection serviceCollection, Action<NodeServicesOptions> setupAction)
24+
{
25+
if (setupAction == null)
26+
{
27+
throw new ArgumentNullException(nameof (setupAction));
28+
}
29+
30+
serviceCollection.AddSingleton(typeof(INodeServices), serviceProvider =>
31+
{
32+
// First we let NodeServicesOptions take its defaults from the IServiceProvider,
33+
// then we let the developer override those options
34+
var options = new NodeServicesOptions(serviceProvider);
35+
setupAction(options);
36+
37+
return NodeServicesFactory.CreateNodeServices(options);
38+
});
39+
}
40+
}
41+
}

‎src/Microsoft.AspNetCore.NodeServices/HostingModels/HttpNodeInstance.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ internal class HttpNodeInstance : OutOfProcessNodeInstance
3535
private int _portNumber;
3636

3737
public HttpNodeInstance(string projectPath, string[] watchFileExtensions, ILogger nodeInstanceOutputLogger,
38-
IDictionary<string, string> environmentVars, bool launchWithDebugging, int? debuggingPort, int port = 0)
38+
IDictionary<string, string> environmentVars, bool launchWithDebugging, int debuggingPort, int port = 0)
3939
: base(
4040
EmbeddedResourceReader.Read(
4141
typeof(HttpNodeInstance),

‎src/Microsoft.AspNetCore.NodeServices/HostingModels/OutOfProcessNodeInstance.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public OutOfProcessNodeInstance(
4949
ILogger nodeOutputLogger,
5050
IDictionary<string, string> environmentVars,
5151
bool launchWithDebugging,
52-
int? debuggingPort)
52+
int debuggingPort)
5353
{
5454
if (nodeOutputLogger == null)
5555
{
@@ -101,12 +101,12 @@ public void Dispose()
101101
// This method is virtual, as it provides a way to override the NODE_PATH or the path to node.exe
102102
protected virtual ProcessStartInfo PrepareNodeProcessStartInfo(
103103
string entryPointFilename, string projectPath, string commandLineArguments,
104-
IDictionary<string, string> environmentVars, bool launchWithDebugging, int? debuggingPort)
104+
IDictionary<string, string> environmentVars, bool launchWithDebugging, int debuggingPort)
105105
{
106106
string debuggingArgs;
107107
if (launchWithDebugging)
108108
{
109-
debuggingArgs = debuggingPort.HasValue ? $"--debug={debuggingPort.Value} " : "--debug ";
109+
debuggingArgs = debuggingPort != default(int) ? $"--debug={debuggingPort} " : "--debug ";
110110
_nodeDebuggingPort = debuggingPort;
111111
}
112112
else

‎src/Microsoft.AspNetCore.NodeServices/HostingModels/SocketNodeInstance.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ internal class SocketNodeInstance : OutOfProcessNodeInstance
4040

4141
public SocketNodeInstance(string projectPath, string[] watchFileExtensions, string socketAddress,
4242
ILogger nodeInstanceOutputLogger, IDictionary<string, string> environmentVars,
43-
bool launchWithDebugging, int? debuggingPort)
43+
bool launchWithDebugging, int debuggingPort)
4444
: base(
4545
EmbeddedResourceReader.Read(
4646
typeof(SocketNodeInstance),

0 commit comments

Comments
 (0)