Design review: Always instantiate via DI

This commit is contained in:
SteveSandersonMS
2016-09-01 15:51:53 +01:00
parent 61fd900974
commit f04fb8c421
9 changed files with 97 additions and 91 deletions

View File

@@ -3,6 +3,7 @@ using System.Diagnostics;
using System.IO;
using System.Threading.Tasks;
using Microsoft.AspNetCore.NodeServices;
using Microsoft.Extensions.DependencyInjection;
namespace ConsoleApplication
{
@@ -12,7 +13,17 @@ namespace ConsoleApplication
public class Program
{
public static void Main(string[] args) {
using (var nodeServices = CreateNodeServices(NodeServicesOptions.DefaultNodeHostingModel)) {
// Set up the DI system
var services = new ServiceCollection();
services.AddNodeServices(new NodeServicesOptions {
HostingModel = NodeServicesOptions.DefaultNodeHostingModel,
ProjectPath = Directory.GetCurrentDirectory(),
WatchFileExtensions = new string[] {} // Don't watch anything
});
var serviceProvider = services.BuildServiceProvider();
// Now instantiate an INodeServices and use it
using (var nodeServices = serviceProvider.GetRequiredService<INodeServices>()) {
MeasureLatency(nodeServices).Wait();
}
}
@@ -34,13 +45,5 @@ namespace ConsoleApplication
Console.WriteLine("\nTotal time: {0:F2} milliseconds", 1000 * elapsedSeconds);
Console.WriteLine("\nTime per invocation: {0:F2} milliseconds", 1000 * elapsedSeconds / requestCount);
}
private static INodeServices CreateNodeServices(NodeHostingModel hostingModel) {
return Configuration.CreateNodeServices(new NodeServicesOptions {
HostingModel = hostingModel,
ProjectPath = Directory.GetCurrentDirectory(),
WatchFileExtensions = new string[] {} // Don't watch anything
});
}
}
}

View File

@@ -8,7 +8,8 @@
"version": "1.0.0",
"type": "platform"
},
"Microsoft.AspNetCore.NodeServices": "1.0.0-*"
"Microsoft.AspNetCore.NodeServices": "1.0.0-*",
"Microsoft.Extensions.DependencyInjection": "1.0.0"
},
"frameworks": {
"netcoreapp1.0": {

View File

@@ -10,7 +10,7 @@ module.exports = merge({
},
module: {
loaders: [
{ test: /\.ts(x?)$/, exclude: /node_modules/, loader: 'ts-loader' }
{ test: /\.ts(x?)$/, exclude: /node_modules/, loader: 'ts-loader?silent' }
],
},
entry: {

View File

@@ -4,6 +4,7 @@ using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.NodeServices.HostingModels;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Console;
using System.Collections.Generic;
namespace Microsoft.AspNetCore.NodeServices
{
@@ -16,48 +17,18 @@ namespace Microsoft.AspNetCore.NodeServices
public static void AddNodeServices(this IServiceCollection serviceCollection, NodeServicesOptions options)
{
serviceCollection.AddSingleton(typeof(INodeServices), serviceProvider =>
{
// Since this instance is being created through DI, we can access the IHostingEnvironment
// to populate options.ProjectPath if it wasn't explicitly specified.
var hostEnv = serviceProvider.GetRequiredService<IHostingEnvironment>();
if (string.IsNullOrEmpty(options.ProjectPath))
{
options.ProjectPath = hostEnv.ContentRootPath;
serviceCollection.AddSingleton(
typeof(INodeServices),
serviceProvider => CreateNodeServices(serviceProvider, options));
}
// Similarly, we can determine the 'is development' value from the hosting environment
options.AddDefaultEnvironmentVariables(hostEnv.IsDevelopment());
// Likewise, if no logger was specified explicitly, we should use the one from DI.
// If it doesn't provide one, CreateNodeInstance will set up a default.
if (options.NodeInstanceOutputLogger == null)
public static INodeServices CreateNodeServices(IServiceProvider serviceProvider, NodeServicesOptions options)
{
var loggerFactory = serviceProvider.GetService<ILoggerFactory>();
if (loggerFactory != null)
{
options.NodeInstanceOutputLogger = loggerFactory.CreateLogger(LogCategoryName);
}
return new NodeServicesImpl(() => CreateNodeInstance(serviceProvider, options));
}
return new NodeServicesImpl(options, () => CreateNodeInstance(options));
});
}
public static INodeServices CreateNodeServices(NodeServicesOptions options)
private static INodeInstance CreateNodeInstance(IServiceProvider serviceProvider, NodeServicesOptions options)
{
return new NodeServicesImpl(options, () => CreateNodeInstance(options));
}
private static INodeInstance CreateNodeInstance(NodeServicesOptions options)
{
// If you've specified no logger, fall back on a default console logger
var logger = options.NodeInstanceOutputLogger;
if (logger == null)
{
logger = new ConsoleLogger(LogCategoryName, null, false);
}
if (options.NodeInstanceFactory != null)
{
// If you've explicitly supplied an INodeInstance factory, we'll use that. This is useful for
@@ -66,17 +37,51 @@ namespace Microsoft.AspNetCore.NodeServices
}
else
{
// Otherwise we'll construct the type of INodeInstance specified by the HostingModel property,
// which itself has a useful default value.
// Otherwise we'll construct the type of INodeInstance specified by the HostingModel property
// (which itself has a useful default value), plus obtain config information from the DI system.
var projectPath = options.ProjectPath;
var envVars = options.EnvironmentVariables == null
? new Dictionary<string, string>()
: new Dictionary<string, string>(options.EnvironmentVariables);
var hostEnv = serviceProvider.GetService<IHostingEnvironment>();
if (hostEnv != null)
{
// In an ASP.NET environment, we can use the IHostingEnvironment data to auto-populate a few
// things that you'd otherwise have to specify manually
if (string.IsNullOrEmpty(projectPath))
{
projectPath = hostEnv.ContentRootPath;
}
// Similarly, we can determine the 'is development' value from the hosting environment
if (!envVars.ContainsKey("NODE_ENV"))
{
// These strings are a de-facto standard in Node
envVars["NODE_ENV"] = hostEnv.IsDevelopment() ? "development" : "production";
}
}
// If no logger was specified explicitly, we should use the one from DI.
// If it doesn't provide one, we'll set up a default one.
var logger = options.NodeInstanceOutputLogger;
if (logger == null)
{
var loggerFactory = serviceProvider.GetService<ILoggerFactory>();
logger = loggerFactory != null
? loggerFactory.CreateLogger(LogCategoryName)
: new ConsoleLogger(LogCategoryName, null, false);
}
switch (options.HostingModel)
{
case NodeHostingModel.Http:
return new HttpNodeInstance(options.ProjectPath, options.WatchFileExtensions, logger,
options.EnvironmentVariables, options.LaunchWithDebugging, options.DebuggingPort, /* port */ 0);
return new HttpNodeInstance(projectPath, options.WatchFileExtensions, logger,
envVars, options.LaunchWithDebugging, options.DebuggingPort, /* port */ 0);
case NodeHostingModel.Socket:
var pipeName = "pni-" + Guid.NewGuid().ToString("D"); // Arbitrary non-clashing string
return new SocketNodeInstance(options.ProjectPath, options.WatchFileExtensions, pipeName, logger,
options.EnvironmentVariables, options.LaunchWithDebugging, options.DebuggingPort);
return new SocketNodeInstance(projectPath, options.WatchFileExtensions, pipeName, logger,
envVars, options.LaunchWithDebugging, options.DebuggingPort);
default:
throw new ArgumentException("Unknown hosting model: " + options.HostingModel);
}

View File

@@ -25,21 +25,5 @@ namespace Microsoft.AspNetCore.NodeServices
public bool LaunchWithDebugging { get; set; }
public IDictionary<string, string> EnvironmentVariables { get; set; }
public int? DebuggingPort { get; set; }
public NodeServicesOptions AddDefaultEnvironmentVariables(bool isDevelopmentMode)
{
if (EnvironmentVariables == null)
{
EnvironmentVariables = new Dictionary<string, string>();
}
if (!EnvironmentVariables.ContainsKey("NODE_ENV"))
{
// These strings are a de-facto standard in Node
EnvironmentVariables["NODE_ENV"] = isDevelopmentMode ? "development" : "production";
}
return this;
}
}
}

View File

@@ -19,15 +19,13 @@ namespace Microsoft.AspNetCore.NodeServices
internal class NodeServicesImpl : INodeServices
{
private static TimeSpan ConnectionDrainingTimespan = TimeSpan.FromSeconds(15);
private NodeServicesOptions _options;
private Func<INodeInstance> _nodeInstanceFactory;
private INodeInstance _currentNodeInstance;
private object _currentNodeInstanceAccessLock = new object();
private Exception _instanceDelayedDisposalException;
internal NodeServicesImpl(NodeServicesOptions options, Func<INodeInstance> nodeInstanceFactory)
internal NodeServicesImpl(Func<INodeInstance> nodeInstanceFactory)
{
_options = options;
_nodeInstanceFactory = nodeInstanceFactory;
}

View File

@@ -90,18 +90,33 @@ If you want to put `addNumber.js` inside a subfolder rather than the root of you
## For non-ASP.NET apps
In other types of .NET app where you don't have ASP.NET Core's DI system, you can get an instance of `NodeServices` as follows:
In other types of .NET Core app, where you don't have ASP.NET supplying an `IServiceCollection` to you, you'll need to instantiate your own DI container. For example, add a reference to the .NET package `Microsoft.Extensions.DependencyInjection`, and then you can construct an `IServiceCollection`, then register NodeServices as usual:
```csharp
// Remember to add 'using Microsoft.AspNetCore.NodeServices;' at the top of your file
var services = new ServiceCollection();
services.AddNodeServices(new NodeServicesOptions { /* your options here */ });
```
var nodeServices = Configuration.CreateNodeServices(new NodeServicesOptions());
Now you can ask it to supply the shared `INodeServices` instance:
```csharp
var serviceProvider = services.BuildServiceProvider();
var nodeServices = serviceProvider.GetRequiredService<INodeServices>();
```
Or, if you want to obtain a separate (non-shared) `INodeServices` instance:
```csharp
var options = new NodeServicesOptions { /* your options here */ };
var nodeServices = Microsoft.AspNetCore.NodeServices.Configuration.CreateNodeServices(serviceProvider, options);
```
Besides this, the usage is the same as described for ASP.NET above, so you can now call `nodeServices.InvokeAsync<T>(...)` etc.
You can dispose the `nodeServices` object whenever you are done with it (and it will shut down the associated Node.js instance), but because these instances are expensive to create, you should whenever possible retain and reuse instances. They are thread-safe - you can call `InvokeAsync<T>` simultaneously from multiple threads. Also, `NodeServices` instances are smart enough to detect if the associated Node instance has died and will automatically start a new Node instance if needed.
You can dispose the `nodeServices` object whenever you are done with it (and it will shut down the associated Node.js instance), but because these instances are expensive to create, you should whenever possible retain and reuse instances. Don't dispose the shared instance returned from `serviceProvider.GetRequiredService` (except perhaps if you know your application is shutting down, although .NET's finalizers will dispose it anyway if the shutdown is graceful).
NodeServices instances are thread-safe - you can call `InvokeAsync<T>` simultaneously from multiple threads. Also, they are smart enough to detect if the associated Node instance has died and will automatically start a new Node instance if needed.
# API Reference
@@ -158,21 +173,22 @@ If no `options` is passed, the default `WatchFileExtensions` array includes `.js
**Signature:**
```csharp
CreateNodeServices(NodeServicesOptions options)
CreateNodeServices(IServiceProvider serviceProvider, NodeServicesOptions options)
```
Directly supplies an instance of `NodeServices` without using ASP.NET's DI system.
Supplies a new (non-shared) instance of `NodeServices`. It takes configuration from the .NET DI system (hence requiring an `IServiceProvider`), though some aspects of configuration can be overridden via the `options` parameter.
**Example**
```csharp
var nodeServices = Configuration.CreateNodeServices(new NodeServicesOptions {
var nodeServices = Configuration.CreateNodeServices(serviceProvider, new NodeServicesOptions {
HostingModel = NodeHostingModel.Socket
});
```
**Parameters**
* `serviceProvider` - type: `IServiceProvider`
* An instance of .NET's standard DI service provider. You can get an instance of this by calling `BuildServiceProvider` on an `IServiceCollection` object. See the example usage of `CreateNodeServices` earlier in this document.
* `options` - type: `NodeServicesOptions`.
* Configures the returned `NodeServices` instance.
* Properties:

View File

@@ -33,10 +33,9 @@ namespace Microsoft.AspNetCore.SpaServices.Prerendering
// in your startup file, but then again it might be confusing that you don't need to.
if (_nodeServices == null)
{
_nodeServices = _fallbackNodeServices = Configuration.CreateNodeServices(new NodeServicesOptions
{
ProjectPath = _applicationBasePath
}.AddDefaultEnvironmentVariables(hostEnv.IsDevelopment()));
_nodeServices = _fallbackNodeServices = Configuration.CreateNodeServices(
serviceProvider,
new NodeServicesOptions());
}
}

View File

@@ -35,19 +35,17 @@ namespace Microsoft.AspNetCore.Builder
"To enable ReactHotModuleReplacement, you must also enable HotModuleReplacement.");
}
var hostEnv = (IHostingEnvironment)appBuilder.ApplicationServices.GetService(typeof(IHostingEnvironment));
var projectPath = options.ProjectPath ?? hostEnv.ContentRootPath;
// Unlike other consumers of NodeServices, WebpackDevMiddleware dosen't share Node instances, nor does it
// use your DI configuration. It's important for WebpackDevMiddleware to have its own private Node instance
// because it must *not* restart when files change (if it did, you'd lose all the benefits of Webpack
// middleware). And since this is a dev-time-only feature, it doesn't matter if the default transport isn't
// as fast as some theoretical future alternative.
var nodeServices = Configuration.CreateNodeServices(new NodeServicesOptions
var nodeServices = Configuration.CreateNodeServices(
appBuilder.ApplicationServices,
new NodeServicesOptions
{
ProjectPath = projectPath,
WatchFileExtensions = new string[] { } // Don't watch anything
}.AddDefaultEnvironmentVariables(hostEnv.IsDevelopment()));
});
// Get a filename matching the middleware Node script
var script = EmbeddedResourceReader.Read(typeof(WebpackDevMiddleware),
@@ -55,6 +53,8 @@ namespace Microsoft.AspNetCore.Builder
var nodeScript = new StringAsTempFile(script); // Will be cleaned up on process exit
// Tell Node to start the server hosting webpack-dev-middleware
var hostEnv = (IHostingEnvironment)appBuilder.ApplicationServices.GetService(typeof(IHostingEnvironment));
var projectPath = options.ProjectPath ?? hostEnv.ContentRootPath;
var devServerOptions = new
{
webpackConfigPath = Path.Combine(projectPath, options.ConfigFile ?? DefaultConfigFile),