From 4645f0d299ebbaa49ab59df250fd4f7bdea3b4de Mon Sep 17 00:00:00 2001 From: Christopher Warrington Date: Fri, 1 Feb 2019 19:45:39 -0800 Subject: [PATCH 1/4] Add UserState dictionary to C# ServerCallContext This commit adds a IDictionary UserState member to the ServerCallContext. Interceptors and call handlers can use this member to pass per-call state between themselves. Like other members of ServerCallContext, UserState is not thread-safe. UserState is initialized on demand so that calls that don't use UserState don't need to pay for it. Closes https://github.com/grpc/grpc/issues/17759 --- src/csharp/Grpc.Core.Api/ServerCallContext.cs | 11 ++++- .../TestServerCallContext.cs | 15 ++++++ .../Interceptors/ServerInterceptorTest.cs | 47 +++++++++++++++++++ .../Internal/DefaultServerCallContext.cs | 15 ++++++ 4 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/csharp/Grpc.Core.Api/ServerCallContext.cs b/src/csharp/Grpc.Core.Api/ServerCallContext.cs index 90b6e9419f0..7149ba283fd 100644 --- a/src/csharp/Grpc.Core.Api/ServerCallContext.cs +++ b/src/csharp/Grpc.Core.Api/ServerCallContext.cs @@ -17,6 +17,7 @@ #endregion using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -113,6 +114,12 @@ namespace Grpc.Core /// public AuthContext AuthContext => AuthContextCore; + /// + /// Gets a dictionary that can be used by the various interceptors and handlers of this + /// call to store arbitrary state. + /// + public IDictionary UserSate => UserStateCore; + /// Provides implementation of a non-virtual public member. protected abstract Task WriteResponseHeadersAsyncCore(Metadata responseHeaders); /// Provides implementation of a non-virtual public member. @@ -135,7 +142,9 @@ namespace Grpc.Core protected abstract Status StatusCore { get; set; } /// Provides implementation of a non-virtual public member. protected abstract WriteOptions WriteOptionsCore { get; set; } - /// Provides implementation of a non-virtual public member. + /// Provides implementation of a non-virtual public member. protected abstract AuthContext AuthContextCore { get; } + /// Provides implementation of a non-virtual public member. + protected abstract IDictionary UserStateCore { get; } } } diff --git a/src/csharp/Grpc.Core.Testing/TestServerCallContext.cs b/src/csharp/Grpc.Core.Testing/TestServerCallContext.cs index e6297e61226..54134fdb25b 100644 --- a/src/csharp/Grpc.Core.Testing/TestServerCallContext.cs +++ b/src/csharp/Grpc.Core.Testing/TestServerCallContext.cs @@ -17,6 +17,7 @@ #endregion using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -50,6 +51,7 @@ namespace Grpc.Core.Testing private Status status; private readonly string peer; private readonly AuthContext authContext; + private Dictionary userState; private readonly ContextPropagationToken contextPropagationToken; private readonly Func writeHeadersFunc; private readonly Func writeOptionsGetter; @@ -93,6 +95,19 @@ namespace Grpc.Core.Testing protected override AuthContext AuthContextCore => authContext; + protected override IDictionary UserStateCore + { + get + { + if (userState == null) + { + userState = new Dictionary(); + } + + return userState; + } + } + protected override ContextPropagationToken CreatePropagationTokenCore(ContextPropagationOptions options) { return contextPropagationToken; diff --git a/src/csharp/Grpc.Core.Tests/Interceptors/ServerInterceptorTest.cs b/src/csharp/Grpc.Core.Tests/Interceptors/ServerInterceptorTest.cs index e76f21d0985..f5e42f2312d 100644 --- a/src/csharp/Grpc.Core.Tests/Interceptors/ServerInterceptorTest.cs +++ b/src/csharp/Grpc.Core.Tests/Interceptors/ServerInterceptorTest.cs @@ -77,6 +77,53 @@ namespace Grpc.Core.Interceptors.Tests Assert.AreEqual("CB1B2B3A", stringBuilder.ToString()); } + [Test] + public void UserStateVisibleToAllInterceptors() + { + object key1 = new object(); + object value1 = new object(); + const string key2 = "Interceptor #2"; + const string value2 = "Important state"; + + var interceptor1 = new ServerCallContextInterceptor(ctx => { + // state starts off empty + Assert.AreEqual(0, ctx.UserSate.Count); + + ctx.UserSate.Add(key1, value1); + }); + + var interceptor2 = new ServerCallContextInterceptor(ctx => { + // second interceptor can see state set by the first + bool found = ctx.UserSate.TryGetValue(key1, out object storedValue1); + Assert.IsTrue(found); + Assert.AreEqual(value1, storedValue1); + + ctx.UserSate.Add(key2, value2); + }); + + var helper = new MockServiceHelper(Host); + helper.UnaryHandler = new UnaryServerMethod((request, context) => { + // call handler can see all the state + bool found = context.UserSate.TryGetValue(key1, out object storedValue1); + Assert.IsTrue(found); + Assert.AreEqual(value1, storedValue1); + + found = context.UserSate.TryGetValue(key2, out object storedValue2); + Assert.IsTrue(found); + Assert.AreEqual(value2, storedValue2); + + return Task.FromResult("PASS"); + }); + helper.ServiceDefinition = helper.ServiceDefinition + .Intercept(interceptor2) + .Intercept(interceptor1); + + var server = helper.GetServer(); + server.Start(); + var channel = helper.GetChannel(); + Assert.AreEqual("PASS", Calls.BlockingUnaryCall(helper.CreateUnaryCall(), "")); + } + [Test] public void CheckNullInterceptorRegistrationFails() { diff --git a/src/csharp/Grpc.Core/Internal/DefaultServerCallContext.cs b/src/csharp/Grpc.Core/Internal/DefaultServerCallContext.cs index b33cb631e26..15089caf5f4 100644 --- a/src/csharp/Grpc.Core/Internal/DefaultServerCallContext.cs +++ b/src/csharp/Grpc.Core/Internal/DefaultServerCallContext.cs @@ -17,6 +17,7 @@ #endregion using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -39,6 +40,7 @@ namespace Grpc.Core private Status status; private readonly IServerResponseStream serverResponseStream; private readonly Lazy authContext; + private Dictionary userState; /// /// Creates a new instance of ServerCallContext. @@ -99,6 +101,19 @@ namespace Grpc.Core protected override AuthContext AuthContextCore => authContext.Value; + protected override IDictionary UserStateCore + { + get + { + if (userState == null) + { + userState = new Dictionary(); + } + + return userState; + } + } + private AuthContext GetAuthContextEager() { using (var authContextNative = callHandle.GetAuthContext()) From 3421c9c4f9643f024290a4c8ded13c146406cffb Mon Sep 17 00:00:00 2001 From: Christopher Warrington Date: Sat, 16 Feb 2019 18:35:52 -0800 Subject: [PATCH 2/4] Make ServerCallContext.UserData a virtual property --- src/csharp/Grpc.Core.Api/ServerCallContext.cs | 17 ++++++++++++++--- .../Grpc.Core.Testing/TestServerCallContext.cs | 15 --------------- .../Internal/DefaultServerCallContext.cs | 15 --------------- 3 files changed, 14 insertions(+), 33 deletions(-) diff --git a/src/csharp/Grpc.Core.Api/ServerCallContext.cs b/src/csharp/Grpc.Core.Api/ServerCallContext.cs index 7149ba283fd..c37aa0f2c0f 100644 --- a/src/csharp/Grpc.Core.Api/ServerCallContext.cs +++ b/src/csharp/Grpc.Core.Api/ServerCallContext.cs @@ -28,6 +28,8 @@ namespace Grpc.Core /// public abstract class ServerCallContext { + private Dictionary userState; + /// /// Creates a new instance of ServerCallContext. /// @@ -118,7 +120,18 @@ namespace Grpc.Core /// Gets a dictionary that can be used by the various interceptors and handlers of this /// call to store arbitrary state. /// - public IDictionary UserSate => UserStateCore; + public virtual IDictionary UserSate + { + get + { + if (userState == null) + { + userState = new Dictionary(); + } + + return userState; + } + } /// Provides implementation of a non-virtual public member. protected abstract Task WriteResponseHeadersAsyncCore(Metadata responseHeaders); @@ -144,7 +157,5 @@ namespace Grpc.Core protected abstract WriteOptions WriteOptionsCore { get; set; } /// Provides implementation of a non-virtual public member. protected abstract AuthContext AuthContextCore { get; } - /// Provides implementation of a non-virtual public member. - protected abstract IDictionary UserStateCore { get; } } } diff --git a/src/csharp/Grpc.Core.Testing/TestServerCallContext.cs b/src/csharp/Grpc.Core.Testing/TestServerCallContext.cs index 54134fdb25b..e6297e61226 100644 --- a/src/csharp/Grpc.Core.Testing/TestServerCallContext.cs +++ b/src/csharp/Grpc.Core.Testing/TestServerCallContext.cs @@ -17,7 +17,6 @@ #endregion using System; -using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -51,7 +50,6 @@ namespace Grpc.Core.Testing private Status status; private readonly string peer; private readonly AuthContext authContext; - private Dictionary userState; private readonly ContextPropagationToken contextPropagationToken; private readonly Func writeHeadersFunc; private readonly Func writeOptionsGetter; @@ -95,19 +93,6 @@ namespace Grpc.Core.Testing protected override AuthContext AuthContextCore => authContext; - protected override IDictionary UserStateCore - { - get - { - if (userState == null) - { - userState = new Dictionary(); - } - - return userState; - } - } - protected override ContextPropagationToken CreatePropagationTokenCore(ContextPropagationOptions options) { return contextPropagationToken; diff --git a/src/csharp/Grpc.Core/Internal/DefaultServerCallContext.cs b/src/csharp/Grpc.Core/Internal/DefaultServerCallContext.cs index 15089caf5f4..b33cb631e26 100644 --- a/src/csharp/Grpc.Core/Internal/DefaultServerCallContext.cs +++ b/src/csharp/Grpc.Core/Internal/DefaultServerCallContext.cs @@ -17,7 +17,6 @@ #endregion using System; -using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -40,7 +39,6 @@ namespace Grpc.Core private Status status; private readonly IServerResponseStream serverResponseStream; private readonly Lazy authContext; - private Dictionary userState; /// /// Creates a new instance of ServerCallContext. @@ -101,19 +99,6 @@ namespace Grpc.Core protected override AuthContext AuthContextCore => authContext.Value; - protected override IDictionary UserStateCore - { - get - { - if (userState == null) - { - userState = new Dictionary(); - } - - return userState; - } - } - private AuthContext GetAuthContextEager() { using (var authContextNative = callHandle.GetAuthContext()) From 2adb48acf0b8e302e81cba61b7db53ff421672e6 Mon Sep 17 00:00:00 2001 From: Christopher Warrington Date: Sat, 16 Feb 2019 23:24:07 -0800 Subject: [PATCH 3/4] Fix typo in ServerCallContext.UserState name --- src/csharp/Grpc.Core.Api/ServerCallContext.cs | 2 +- .../Interceptors/ServerInterceptorTest.cs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/csharp/Grpc.Core.Api/ServerCallContext.cs b/src/csharp/Grpc.Core.Api/ServerCallContext.cs index c37aa0f2c0f..7cc03cb3a0b 100644 --- a/src/csharp/Grpc.Core.Api/ServerCallContext.cs +++ b/src/csharp/Grpc.Core.Api/ServerCallContext.cs @@ -120,7 +120,7 @@ namespace Grpc.Core /// Gets a dictionary that can be used by the various interceptors and handlers of this /// call to store arbitrary state. /// - public virtual IDictionary UserSate + public virtual IDictionary UserState { get { diff --git a/src/csharp/Grpc.Core.Tests/Interceptors/ServerInterceptorTest.cs b/src/csharp/Grpc.Core.Tests/Interceptors/ServerInterceptorTest.cs index f5e42f2312d..66990832124 100644 --- a/src/csharp/Grpc.Core.Tests/Interceptors/ServerInterceptorTest.cs +++ b/src/csharp/Grpc.Core.Tests/Interceptors/ServerInterceptorTest.cs @@ -87,28 +87,28 @@ namespace Grpc.Core.Interceptors.Tests var interceptor1 = new ServerCallContextInterceptor(ctx => { // state starts off empty - Assert.AreEqual(0, ctx.UserSate.Count); + Assert.AreEqual(0, ctx.UserState.Count); - ctx.UserSate.Add(key1, value1); + ctx.UserState.Add(key1, value1); }); var interceptor2 = new ServerCallContextInterceptor(ctx => { // second interceptor can see state set by the first - bool found = ctx.UserSate.TryGetValue(key1, out object storedValue1); + bool found = ctx.UserState.TryGetValue(key1, out object storedValue1); Assert.IsTrue(found); Assert.AreEqual(value1, storedValue1); - ctx.UserSate.Add(key2, value2); + ctx.UserState.Add(key2, value2); }); var helper = new MockServiceHelper(Host); helper.UnaryHandler = new UnaryServerMethod((request, context) => { // call handler can see all the state - bool found = context.UserSate.TryGetValue(key1, out object storedValue1); + bool found = context.UserState.TryGetValue(key1, out object storedValue1); Assert.IsTrue(found); Assert.AreEqual(value1, storedValue1); - found = context.UserSate.TryGetValue(key2, out object storedValue2); + found = context.UserState.TryGetValue(key2, out object storedValue2); Assert.IsTrue(found); Assert.AreEqual(value2, storedValue2); From 1232f60ac2dfc46f314cbe35326d7508cbf5a4e9 Mon Sep 17 00:00:00 2001 From: Christopher Warrington Date: Thu, 21 Feb 2019 17:26:54 -0800 Subject: [PATCH 4/4] Make UserState non-virtual; add protected impl Makes the public UserState property non-virtual and adds a protected virtual UserStateCore that can be overridden. This follows the pattern of the other members. --- src/csharp/Grpc.Core.Api/ServerCallContext.cs | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/csharp/Grpc.Core.Api/ServerCallContext.cs b/src/csharp/Grpc.Core.Api/ServerCallContext.cs index 7cc03cb3a0b..8d7dbf544d6 100644 --- a/src/csharp/Grpc.Core.Api/ServerCallContext.cs +++ b/src/csharp/Grpc.Core.Api/ServerCallContext.cs @@ -120,18 +120,7 @@ namespace Grpc.Core /// Gets a dictionary that can be used by the various interceptors and handlers of this /// call to store arbitrary state. /// - public virtual IDictionary UserState - { - get - { - if (userState == null) - { - userState = new Dictionary(); - } - - return userState; - } - } + public IDictionary UserState => UserStateCore; /// Provides implementation of a non-virtual public member. protected abstract Task WriteResponseHeadersAsyncCore(Metadata responseHeaders); @@ -157,5 +146,18 @@ namespace Grpc.Core protected abstract WriteOptions WriteOptionsCore { get; set; } /// Provides implementation of a non-virtual public member. protected abstract AuthContext AuthContextCore { get; } + /// Provides implementation of a non-virtual public member. + protected virtual IDictionary UserStateCore + { + get + { + if (userState == null) + { + userState = new Dictionary(); + } + + return userState; + } + } } }