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.

Sign up for GitHub

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

Revoke public interface change. #876

Merged
merged 2 commits into from
Apr 17, 2020
+13 −21
Merged

Revoke public interface change. #876

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd47204
Revoke public interface change.
terencefan Apr 17, 2020
037dbe9
Merge branch 'dev' into revoke
vicancy Apr 17, 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
16 changes: 8 additions & 8 deletions src/Microsoft.Azure.SignalR.Management/IServiceManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ public interface IServiceManager
/// <returns>The task object representing the asynchronous operation.</returns>
Task<IServiceHubContext> CreateHubContextAsync(string hubName, ILoggerFactory loggerFactory = null, CancellationToken cancellationToken = default);

/// <summary>
/// Creates a client access token for SignalR hub connections to connect to Azure SignalR Service.
/// </summary>
/// <param name="hubName">The hub name.</param>
/// <param name="userId">The user ID.</param>
/// <param name="claims">The claim list to be put into access token.</param>
/// <param name="lifeTime">The lifetime of the token. The default value is one hour.</param>
/// <summary>
/// Creates a client access token for SignalR hub connections to connect to Azure SignalR Service.
/// </summary>
/// <param name="hubName">The hub name.</param>
/// <param name="userId">The user ID.</param>
/// <param name="claims">The claim list to be put into access token.</param>
/// <param name="lifeTime">The lifetime of the token. The default value is one hour.</param>
/// <returns>Client access token to Azure SignalR Service.</returns>
Task<string> GenerateClientAccessToken(string hubName, string userId = null, IList<Claim> claims = null, TimeSpan? lifeTime = null);
string GenerateClientAccessToken(string hubName, string userId = null, IList<Claim> claims = null, TimeSpan? lifeTime = null);

/// <summary>
/// Creates an client endpoint for SignalR hub connections to connect to Azure SignalR Service
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Azure.SignalR.Management/ServiceManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public async Task<IServiceHubContext> CreateHubContextAsync(string hubName, ILog
}
}

public Task<string> GenerateClientAccessToken(string hubName, string userId = null, IList<Claim> claims = null, TimeSpan? lifeTime = null)
public string GenerateClientAccessToken(string hubName, string userId = null, IList<Claim> claims = null, TimeSpan? lifeTime = null)
{
var claimsWithUserId = new List<Claim>();
if (userId != null)
Expand All @@ -143,7 +143,7 @@ public Task<string> GenerateClientAccessToken(string hubName, string userId = nu
{
claimsWithUserId.AddRange(claims);
}
return _endpointProvider.GenerateClientAccessTokenAsync(hubName, claimsWithUserId, lifeTime);
return _endpointProvider.GenerateClientAccessTokenAsync(hubName, claimsWithUserId, lifeTime).Result;
Copy link
Member Author

@terencefan terencefan Apr 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is a quick fix of #861 . We don't actually have this method awaitable at this moment.

But it would probably cause a deadlock if you embed a Task.Wait inside an async method.
(Task.Result does things that similar to Task.Wait)
https://stackoverflow.com/questions/13140523/await-vs-task-wait-deadlock

So try avoiding doing something like this:

async Task Get(){ var token = manager.GenerateClientAccessToken; }

We will provide an async version of this API later on another PR.

}

public string GetClientEndpoint(string hubName) => _endpointProvider.GetClientEndpoint(hubName, null, null);
Expand Down
10 changes: 1 addition & 9 deletions test/Microsoft.Azure.SignalR.E2ETests/Management/ServiceHubContextE2EFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,16 +331,8 @@ private static string[] GetTestStringList(string prefix, int count)
var serviceHubContext = await serviceManager.CreateHubContextAsync(HubName, loggerFactory);

var clientEndpoint = serviceManager.GetClientEndpoint(HubName);
var tasks = from userName in _userNames
var tokens = from userName in _userNames
select serviceManager.GenerateClientAccessToken(HubName, userName);

await Task.WhenAll(tasks);

var tokens = new List<string>();
foreach (var task in tasks)
{
tokens.Add(task.Result);
}
return (clientEndpoint, tokens, serviceHubContext);
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/Microsoft.Azure.SignalR.Management.Tests/ServiceManagerFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ from appName in _appNames

[Theory]
[MemberData(nameof(TestGenerateAccessTokenData))]
internal async Task GenerateClientAccessTokenTest(string userId, Claim[] claims, string appName)
internal void GenerateClientAccessTokenTest(string userId, Claim[] claims, string appName)
{
var manager = new ServiceManager(new ServiceManagerOptions() { ConnectionString = _testConnectionString, ApplicationName = appName }, null);
var tokenString = await manager.GenerateClientAccessToken(HubName, userId, claims, _tokenLifeTime);
var tokenString = manager.GenerateClientAccessToken(HubName, userId, claims, _tokenLifeTime);
var token = JwtTokenHelper.JwtHandler.ReadJwtToken(tokenString);

string expectedToken = JwtTokenHelper.GenerateExpectedAccessToken(token, GetExpectedClientEndpoint(appName), AccessKey, claims);
Expand Down