Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set up options and track changes in management SDK. #1081

Merged
merged 31 commits into from
Nov 9, 2020
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
f1d6d2c
set up ServiceManagerContext and ServiceOptions; track changes
Y-Sindo Oct 27, 2020
ef79564
fix error in ServiceManagerBuilder as a temporary solution
Y-Sindo Oct 27, 2020
04d9b10
test for options changes detected
Y-Sindo Oct 27, 2020
1840a4d
make sure product info has default value
Y-Sindo Oct 27, 2020
8cb960c
fix default product info error
Y-Sindo Oct 28, 2020
fa833d2
remove limitation that AddSignalRServiceManager must be added after S…
Y-Sindo Oct 28, 2020
f9b358e
offer choices to configure with configration instance or action
Y-Sindo Oct 30, 2020
c4f2030
fix
Y-Sindo Oct 30, 2020
32636be
fix
Y-Sindo Oct 30, 2020
c83f4c9
fix
Y-Sindo Oct 30, 2020
17b6585
Enable service builder to build dynamically configurable service manager
Y-Sindo Oct 30, 2020
52dc94b
add config hot reload to ServiceManagerBuilder, fix productInfo error…
Y-Sindo Nov 2, 2020
5315a43
Update DiExtensionFacts.cs
Y-Sindo Nov 2, 2020
d3a3b68
fix
Y-Sindo Nov 2, 2020
1399157
rename & skip fail test
Y-Sindo Nov 3, 2020
d32b810
fix
Y-Sindo Nov 3, 2020
80874da
fix
Y-Sindo Nov 3, 2020
6786043
fix
Y-Sindo Nov 3, 2020
3d6a3ec
merge ServiceManagerOptions section key into one
Y-Sindo Nov 3, 2020
37947ab
rename service provider
Y-Sindo Nov 3, 2020
e3edf86
tracks change directly from IConfiguration; Add InMemoryConfigHotRelo…
Y-Sindo Nov 3, 2020
9a5c796
add memory config hot reload test to DependencyInjectionExtensionFact…
Y-Sindo Nov 3, 2020
a9a6153
Implement options changes tracking chain with self-creating IChangeTo…
Y-Sindo Nov 4, 2020
f6b7d91
dispose serviceProvider from serviceManager; delete unnecessary tests…
Y-Sindo Nov 4, 2020
2532535
fix
Y-Sindo Nov 4, 2020
514355a
fix
Y-Sindo Nov 4, 2020
13ec165
do not dispose ServiceProvider if ServiceManager is built from global…
Y-Sindo Nov 5, 2020
35c083e
change ServiceProvider to IServiceProvider in ServiceManager ctor and…
Y-Sindo Nov 5, 2020
4b73ac4
fix
Y-Sindo Nov 5, 2020
903f1c2
read from context
Y-Sindo Nov 9, 2020
383ad4b
Merge branch 'chained-options' of https://github.com/Y-Sindo/azure-si…
Y-Sindo Nov 9, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build/dependencies.props
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
<MicrosoftConfigurationUserSecretVersion>2.2.0</MicrosoftConfigurationUserSecretVersion>
<MicrosoftConfigurationEnvironmentVariablesVersion>2.2.0</MicrosoftConfigurationEnvironmentVariablesVersion>
<MicrosoftAspNetCoreSignalRProtocolsMessagePackPackageVersion>1.0.11</MicrosoftAspNetCoreSignalRProtocolsMessagePackPackageVersion>
<MicrosoftExtensionsOptionsConfigurationExtensionsVersion>2.1.0</MicrosoftExtensionsOptionsConfigurationExtensionsVersion>

<!-- Signing -->
<MicroBuildCorePackageVersion>0.3.0</MicroBuildCorePackageVersion>
Expand Down
3 changes: 2 additions & 1 deletion src/Microsoft.Azure.SignalR.Common/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ internal static class Constants
{
public static class Keys
{
public const string AzureSignalRSectionKey = "Azure:SignalR";
public const string ServerStickyModeDefaultKey = "Azure:SignalR:ServerStickyMode";
public const string ConnectionStringDefaultKey = "Azure:SignalR:ConnectionString";
public const string ApplicationNameDefaultKey = "Azure:SignalR:ApplicationName";
Expand Down Expand Up @@ -85,4 +86,4 @@ public static class Protocol
public const string BlazorPack = "blazorpack";
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Threading;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Primitives;

namespace Microsoft.Azure.SignalR.Management.Configuration
{
/// <summary>
/// Sets up TargetOptions from SourceOptions and tracks changes .
/// </summary>
internal abstract class CascadeOptionsSetup<TargetOptions, SourceOptions> : IConfigureOptions<TargetOptions>, IOptionsChangeTokenSource<TargetOptions>, IDisposable
where SourceOptions : class
where TargetOptions : class
{
private readonly IDisposable _registration;
private readonly IOptionsMonitor<SourceOptions> _sourceMonitor;
private ConfigurationReloadToken _changeToken;

public CascadeOptionsSetup(IOptionsMonitor<SourceOptions> sourceMonitor)
{
_registration = sourceMonitor.OnChange(RaiseChange);
_changeToken = new ConfigurationReloadToken();
_sourceMonitor = sourceMonitor;
}

public string Name => Options.DefaultName;

public void Configure(TargetOptions target) => Convert(target, _sourceMonitor.CurrentValue);

protected abstract void Convert(TargetOptions target, SourceOptions source);

public IChangeToken GetChangeToken() => _changeToken;

private void RaiseChange(SourceOptions sourceOptions)
{
var previousToken = Interlocked.Exchange(ref _changeToken, new ConfigurationReloadToken());
previousToken.OnReload();
}

public void Dispose()
{
_registration.Dispose();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,6 @@ internal class ServiceManagerContext

public ServiceTransportType ServiceTransportType { get; set; } = ServiceTransportType.Transient;

public void SetValueFromOptions(ServiceManagerOptions options)
{
ServiceEndpoints = options.ServiceEndpoints ?? (options.ServiceEndpoint != null
? (new ServiceEndpoint[] { options.ServiceEndpoint })
: (new ServiceEndpoint[] { new ServiceEndpoint(options.ConnectionString) }));
ApplicationName = options.ApplicationName;
ConnectionCount = options.ConnectionCount;
Proxy = options.Proxy;
ServiceTransportType = options.ServiceTransportType;
}
public bool DisposeServiceProvider { get; set; } = false;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Azure.SignalR.Management.Configuration;
using Microsoft.Extensions.Options;

namespace Microsoft.Azure.SignalR.Management
{
internal class ServiceManagerContextSetup : CascadeOptionsSetup<ServiceManagerContext, ServiceManagerOptions>
{
public ServiceManagerContextSetup(IOptionsMonitor<ServiceManagerOptions> sourceMonitor) : base(sourceMonitor)
{
}

protected override void Convert(ServiceManagerContext target, ServiceManagerOptions source)
{
target.ServiceEndpoints = source.ServiceEndpoints ?? (source.ServiceEndpoint != null
? (new ServiceEndpoint[] { source.ServiceEndpoint })
: (new ServiceEndpoint[] { new ServiceEndpoint(source.ConnectionString) }));
target.ApplicationName = source.ApplicationName;
target.ConnectionCount = source.ConnectionCount;
target.Proxy = source.Proxy;
target.ServiceTransportType = source.ServiceTransportType;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.FileProviders;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Primitives;

namespace Microsoft.Azure.SignalR.Management.Configuration
{
internal class ServiceManagerOptionsSetup : IConfigureOptions<ServiceManagerOptions>, IOptionsChangeTokenSource<ServiceManagerOptions>
{
private readonly IConfiguration _configuration;

public ServiceManagerOptionsSetup(IConfiguration configuration = null)
{
_configuration = configuration;
}

public string Name => Options.DefaultName;

public void Configure(ServiceManagerOptions options)
{
if (_configuration != null)
{
_configuration.GetSection(Constants.Keys.AzureSignalRSectionKey).Bind(options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding services.Configure<ServiceManagerOptions>(Configuration.GetSection(Constants.Keys.AzureSignalRSectionKey)); is enough, we don't need this setup class as we don't have additional setups? Emulator uses such configure and hotreload works, see https://github.com/Azure/azure-signalr/blob/dev/src/Microsoft.Azure.SignalR.Emulator/Startup.cs#L45

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In here, https://github.com/Azure/azure-signalr/blob/dev/src/Microsoft.Azure.SignalR.Emulator/Startup.cs#L45, the Configuration variable is actually an IConfiguration type variable. However, in the extension methods of ServiceCollection, we don't have IConfiguration. Do you think we should add it as a parameter?

}
}

public IChangeToken GetChangeToken()
{
return _configuration?.GetReloadToken() ?? NullChangeToken.Singleton;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Azure.SignalR.Management.Configuration;
using Microsoft.Extensions.Options;

namespace Microsoft.Azure.SignalR.Management
{
internal class ServiceOptionsSetup : CascadeOptionsSetup<ServiceOptions, ServiceManagerContext>
{
public ServiceOptionsSetup(IOptionsMonitor<ServiceManagerContext> sourceMonitor) : base(sourceMonitor)
{
}

protected override void Convert(ServiceOptions target, ServiceManagerContext source)
{
target.ApplicationName = source.ApplicationName;
target.Endpoints = source.ServiceEndpoints;
target.Proxy = source.Proxy;
target.ConnectionCount = source.ConnectionCount;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
using System;
using System.ComponentModel;
using System.Reflection;
using Microsoft.Azure.SignalR.Management.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;

namespace Microsoft.Azure.SignalR.Management
{
internal static class DependencyInjectionExtensions //TODO: not ready for public use
{
/// <summary>
/// Adds the essential SignalR Service Manager services to the specified services collection and configures <see cref="ServiceManagerOptions"/> with configuration instance registered in service collection.
/// </summary>
public static IServiceCollection AddSignalRServiceManager(this IServiceCollection services)
{
services.AddSingleton<ServiceManagerOptionsSetup>()
.AddSingleton<IConfigureOptions<ServiceManagerOptions>>(sp => sp.GetService<ServiceManagerOptionsSetup>())
.AddSingleton<IOptionsChangeTokenSource<ServiceManagerOptions>>(sp => sp.GetService<ServiceManagerOptionsSetup>());
services.PostConfigure<ServiceManagerOptions>(o => o.ValidateOptions());
services.AddSingleton<ServiceManagerContextSetup>()
.AddSingleton<IConfigureOptions<ServiceManagerContext>>(sp => sp.GetService<ServiceManagerContextSetup>())
.AddSingleton<IOptionsChangeTokenSource<ServiceManagerContext>>(sp => sp.GetService<ServiceManagerContextSetup>());
services.AddSingleton<ServiceOptionsSetup>()
.AddSingleton<IConfigureOptions<ServiceOptions>>(sp => sp.GetService<ServiceOptionsSetup>())
.AddSingleton<IOptionsChangeTokenSource<ServiceOptions>>(sp => sp.GetService<ServiceOptionsSetup>());
return services.TrySetProductInfo();
}

/// <summary>
/// Adds the essential SignalR Service Manager services to the specified services collection and registers an action used to configure <see cref="ServiceManagerOptions"/>
/// </summary>
public static IServiceCollection AddSignalRServiceManager(this IServiceCollection services, Action<ServiceManagerOptions> configure)
{
services.Configure(configure);
return services.AddSignalRServiceManager();
}

/// <summary>
/// Adds product info to <see cref="ServiceManagerContext"/>
/// </summary>
/// <returns></returns>
[EditorBrowsable(EditorBrowsableState.Never)]
public static IServiceCollection WithAssembly(this IServiceCollection services, Assembly assembly)
{
var productInfo = ProductInfo.GetProductInfo(assembly);
return services.Configure<ServiceManagerContext>(o => o.ProductInfo = productInfo);
}

private static IServiceCollection TrySetProductInfo(this IServiceCollection services)
{
var assembly = Assembly.GetExecutingAssembly();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this anyway executes even if ProductInfo is already set?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but if ProductInfo is not null, then the value would not be overwritten.

var productInfo = ProductInfo.GetProductInfo(assembly);
return services.Configure<ServiceManagerContext>(o => o.ProductInfo = o.ProductInfo ?? productInfo);
}
}
}
2 changes: 1 addition & 1 deletion src/Microsoft.Azure.SignalR.Management/IServiceManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.Azure.SignalR.Management
/// <summary>
/// A manager abstraction for managing Azure SignalR Service.
/// </summary>
public interface IServiceManager
public interface IServiceManager : IDisposable
{
/// <summary>
/// Creates an instance of <see cref="IServiceHubContext"/> asynchronously.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0'">
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="$(MicrosoftExtensionsDependencyInjectionPackageVersion)" />
<PackageReference Include="Microsoft.Extensions.Http" Version="$(MicrosoftExtensionHttpVersion)" />
<PackageReference Include="Microsoft.Extensions.Http" Version="$(MicrosoftExtensionHttpVersion)" />
<PackageReference Include="Microsoft.AspNetCore.SignalR" Version="$(MicrosoftAspNetCoreSignalRPackageVersion)" />
<PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" Version="$(MicrosoftExtensionsOptionsConfigurationExtensionsVersion)" />
</ItemGroup>
</Project>
12 changes: 11 additions & 1 deletion src/Microsoft.Azure.SignalR.Management/ServiceManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ internal class ServiceManager : IServiceManager
private readonly string _productInfo;
private readonly ServiceManagerContext _context;
private readonly RestClientFactory _restClientFactory;
private readonly IServiceProvider _serviceProvider;

internal ServiceManager(ServiceManagerContext context, RestClientFactory restClientFactory)
internal ServiceManager(ServiceManagerContext context, RestClientFactory restClientFactory, IServiceProvider serviceProvider)
{
_endpoint = context.ServiceEndpoints.Single();//temp solution

Expand All @@ -46,6 +47,7 @@ internal ServiceManager(ServiceManagerContext context, RestClientFactory restCli
_productInfo = context.ProductInfo;
_context = context;
_restClientFactory = restClientFactory;
_serviceProvider = serviceProvider;
}

public async Task<IServiceHubContext> CreateHubContextAsync(string hubName, ILoggerFactory loggerFactory = null, CancellationToken cancellationToken = default)
Expand Down Expand Up @@ -141,6 +143,14 @@ public async Task<IServiceHubContext> CreateHubContextAsync(string hubName, ILog
}
}

public void Dispose()
{
if (_context.DisposeServiceProvider)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is passing some ownServiceProvider parameter in .ctor doable?

{
(_serviceProvider as IDisposable).Dispose();
}
}

public string GenerateClientAccessToken(string hubName, string userId = null, IList<Claim> claims = null, TimeSpan? lifeTime = null)
{
var claimsWithUserId = new List<Claim>();
Expand Down
34 changes: 23 additions & 11 deletions src/Microsoft.Azure.SignalR.Management/ServiceManagerBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
using System;
using System.ComponentModel;
using System.Reflection;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;

namespace Microsoft.Azure.SignalR.Management
{
Expand All @@ -12,24 +15,36 @@ namespace Microsoft.Azure.SignalR.Management
/// </summary>
public class ServiceManagerBuilder : IServiceManagerBuilder
{
private readonly ServiceManagerOptions _options = new ServiceManagerOptions();
private readonly IServiceCollection _services = new ServiceCollection();
private Assembly _assembly;

/// <summary>
/// Configures the <see cref="IServiceManager"/> instances.
/// Registers an action used to configure <see cref="IServiceManager"/>.
/// </summary>
/// <param name="configure">A callback to configure the <see cref="IServiceManager"/>.</param>
/// <returns>The same instance of the <see cref="ServiceManagerBuilder"/> for chaining.</returns>
public ServiceManagerBuilder WithOptions(Action<ServiceManagerOptions> configure)
{
configure?.Invoke(_options);
_services.Configure(configure);
return this;
}

/// <summary>
/// Registers a configuration instance to configure <see cref="IServiceManager"/>
/// </summary>
/// <param name="config">The configuration instance.</param>
/// <returns>The same instance of the <see cref="ServiceManagerBuilder"/> for chaining.</returns>
internal ServiceManagerBuilder WithConfiguration(IConfiguration config)
{
_services.AddSingleton(config);
return this;
}

[EditorBrowsable(EditorBrowsableState.Never)]
public ServiceManagerBuilder WithCallingAssembly()
{
_assembly = Assembly.GetCallingAssembly();
_services.WithAssembly(_assembly);
return this;
}

Expand All @@ -39,16 +54,13 @@ public ServiceManagerBuilder WithCallingAssembly()
/// <returns>The instance of the <see cref="IServiceManager"/>.</returns>
public IServiceManager Build()
{
_options.ValidateOptions();

_services.AddSignalRServiceManager();
_services.Configure<ServiceManagerContext>(c => c.DisposeServiceProvider = true);
var serviceProvider = _services.BuildServiceProvider();
var context = serviceProvider.GetRequiredService<IOptions<ServiceManagerContext>>().Value;
var productInfo = ProductInfo.GetProductInfo(_assembly);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

read from context?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

var context = new ServiceManagerContext()
{
ProductInfo = productInfo
};
context.SetValueFromOptions(_options);
var restClientBuilder = new RestClientFactory(productInfo);
return new ServiceManager(context, restClientBuilder);
return new ServiceManager(context, restClientBuilder, serviceProvider);
}
}
}
Loading