From 7ebbc475fc1a04731b3f0e8d3565a88f97cc7cf6 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 8 Dec 2015 22:39:02 -0800 Subject: [PATCH] put user defined userAgentString to the beginning of userAgentString --- src/csharp/Grpc.Core.Tests/ChannelTest.cs | 11 ++ .../Grpc.Core.Tests/ClientServerTest.cs | 14 +-- .../Grpc.Core.Tests/Grpc.Core.Tests.csproj | 1 + .../Grpc.Core.Tests/MockServiceHelper.cs | 7 +- .../Grpc.Core.Tests/UserAgentStringTest.cs | 101 ++++++++++++++++++ src/csharp/Grpc.Core/Channel.cs | 42 +++++--- src/csharp/Grpc.Core/ChannelOptions.cs | 7 +- 7 files changed, 152 insertions(+), 31 deletions(-) create mode 100644 src/csharp/Grpc.Core.Tests/UserAgentStringTest.cs diff --git a/src/csharp/Grpc.Core.Tests/ChannelTest.cs b/src/csharp/Grpc.Core.Tests/ChannelTest.cs index f4ae9abefd8..ed0ec14df5c 100644 --- a/src/csharp/Grpc.Core.Tests/ChannelTest.cs +++ b/src/csharp/Grpc.Core.Tests/ChannelTest.cs @@ -47,6 +47,17 @@ namespace Grpc.Core.Tests Assert.Throws(typeof(ArgumentNullException), () => new Channel(null, ChannelCredentials.Insecure)); } + [Test] + public void Constructor_RejectsDuplicateOptions() + { + var options = new ChannelOption[] + { + new ChannelOption(ChannelOptions.PrimaryUserAgentString, "ABC"), + new ChannelOption(ChannelOptions.PrimaryUserAgentString, "XYZ") + }; + Assert.Throws(typeof(ArgumentException), () => new Channel("127.0.0.1", ChannelCredentials.Insecure, options)); + } + [Test] public void State_IdleAfterCreation() { diff --git a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs index 25a5a27c8e3..b683751bc03 100644 --- a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs +++ b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs @@ -201,7 +201,7 @@ namespace Grpc.Core.Tests Assert.AreEqual(headers[1].Key, trailers[1].Key); CollectionAssert.AreEqual(headers[1].ValueBytes, trailers[1].ValueBytes); } - + [Test] public void UnknownMethodHandler() { @@ -218,18 +218,6 @@ namespace Grpc.Core.Tests Assert.AreEqual(StatusCode.Unimplemented, ex.Status.StatusCode); } - [Test] - public void UserAgentStringPresent() - { - helper.UnaryHandler = new UnaryServerMethod(async (request, context) => - { - return context.RequestHeaders.Where(entry => entry.Key == "user-agent").Single().Value; - }); - - string userAgent = Calls.BlockingUnaryCall(helper.CreateUnaryCall(), "abc"); - Assert.IsTrue(userAgent.StartsWith("grpc-csharp/")); - } - [Test] public void PeerInfoPresent() { diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj index e5ffa319895..70b83f7fb14 100644 --- a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj +++ b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj @@ -64,6 +64,7 @@ Version.cs + diff --git a/src/csharp/Grpc.Core.Tests/MockServiceHelper.cs b/src/csharp/Grpc.Core.Tests/MockServiceHelper.cs index 567e04eddcc..30473143452 100644 --- a/src/csharp/Grpc.Core.Tests/MockServiceHelper.cs +++ b/src/csharp/Grpc.Core.Tests/MockServiceHelper.cs @@ -32,6 +32,7 @@ #endregion using System; +using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Threading; @@ -52,6 +53,7 @@ namespace Grpc.Core.Tests readonly string host; readonly ServerServiceDefinition serviceDefinition; + readonly IEnumerable channelOptions; readonly Method unaryMethod; readonly Method clientStreamingMethod; @@ -66,9 +68,10 @@ namespace Grpc.Core.Tests Server server; Channel channel; - public MockServiceHelper(string host = null, Marshaller marshaller = null) + public MockServiceHelper(string host = null, Marshaller marshaller = null, IEnumerable channelOptions = null) { this.host = host ?? "localhost"; + this.channelOptions = channelOptions; marshaller = marshaller ?? Marshallers.StringMarshaller; unaryMethod = new Method( @@ -154,7 +157,7 @@ namespace Grpc.Core.Tests { if (channel == null) { - channel = new Channel(Host, GetServer().Ports.Single().BoundPort, ChannelCredentials.Insecure); + channel = new Channel(Host, GetServer().Ports.Single().BoundPort, ChannelCredentials.Insecure, channelOptions); } return channel; } diff --git a/src/csharp/Grpc.Core.Tests/UserAgentStringTest.cs b/src/csharp/Grpc.Core.Tests/UserAgentStringTest.cs new file mode 100644 index 00000000000..cc830086a67 --- /dev/null +++ b/src/csharp/Grpc.Core.Tests/UserAgentStringTest.cs @@ -0,0 +1,101 @@ +#region Copyright notice and license + +// Copyright 2015, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#endregion + +using System; +using System.Diagnostics; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Grpc.Core; +using Grpc.Core.Internal; +using Grpc.Core.Profiling; +using Grpc.Core.Utils; +using NUnit.Framework; + +namespace Grpc.Core.Tests +{ + public class UserAgentStringTest + { + const string Host = "127.0.0.1"; + + MockServiceHelper helper; + Server server; + Channel channel; + + [TearDown] + public void Cleanup() + { + channel.ShutdownAsync().Wait(); + server.ShutdownAsync().Wait(); + } + + [Test] + public void DefaultUserAgentString() + { + helper = new MockServiceHelper(Host); + server = helper.GetServer(); + server.Start(); + channel = helper.GetChannel(); + + helper.UnaryHandler = new UnaryServerMethod((request, context) => + { + var userAgentString = context.RequestHeaders.First(m => (m.Key == "user-agent")).Value; + var parts = userAgentString.Split(new [] {' '}, 2); + Assert.AreEqual(string.Format("grpc-csharp/{0}", VersionInfo.CurrentVersion), parts[0]); + Assert.IsTrue(parts[1].StartsWith("grpc-c/")); + return Task.FromResult("PASS"); + }); + Assert.AreEqual("PASS", Calls.BlockingUnaryCall(helper.CreateUnaryCall(), "")); + } + + [Test] + public void ApplicationUserAgentString() + { + helper = new MockServiceHelper(Host, + channelOptions: new[] { new ChannelOption(ChannelOptions.PrimaryUserAgentString, "XYZ") }); + server = helper.GetServer(); + server.Start(); + channel = helper.GetChannel(); + + channel = helper.GetChannel(); + helper.UnaryHandler = new UnaryServerMethod((request, context) => + { + var userAgentString = context.RequestHeaders.First(m => (m.Key == "user-agent")).Value; + var parts = userAgentString.Split(new[] { ' ' }, 3); + Assert.AreEqual("XYZ", parts[0]); + return Task.FromResult("PASS"); + }); + Assert.AreEqual("PASS", Calls.BlockingUnaryCall(helper.CreateUnaryCall(), "")); + } + } +} diff --git a/src/csharp/Grpc.Core/Channel.cs b/src/csharp/Grpc.Core/Channel.cs index ec60354639e..d8d43c7998d 100644 --- a/src/csharp/Grpc.Core/Channel.cs +++ b/src/csharp/Grpc.Core/Channel.cs @@ -32,8 +32,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Runtime.InteropServices; -using System.Threading; using System.Threading.Tasks; using Grpc.Core.Internal; @@ -57,7 +55,7 @@ namespace Grpc.Core readonly string target; readonly GrpcEnvironment environment; readonly ChannelSafeHandle handle; - readonly List options; + readonly Dictionary options; bool shutdownRequested; @@ -71,12 +69,12 @@ namespace Grpc.Core public Channel(string target, ChannelCredentials credentials, IEnumerable options = null) { this.target = Preconditions.CheckNotNull(target, "target"); + this.options = CreateOptionsDictionary(options); + EnsureUserAgentChannelOption(this.options); this.environment = GrpcEnvironment.AddRef(); - this.options = options != null ? new List(options) : new List(); - EnsureUserAgentChannelOption(this.options); using (var nativeCredentials = credentials.ToNativeCredentials()) - using (var nativeChannelArgs = ChannelOptions.CreateChannelArgs(this.options)) + using (var nativeChannelArgs = ChannelOptions.CreateChannelArgs(this.options.Values)) { if (nativeCredentials != null) { @@ -233,18 +231,36 @@ namespace Grpc.Core activeCallCounter.Decrement(); } - private static void EnsureUserAgentChannelOption(List options) + private static void EnsureUserAgentChannelOption(Dictionary options) { - if (!options.Any((option) => option.Name == ChannelOptions.PrimaryUserAgentString)) + var key = ChannelOptions.PrimaryUserAgentString; + var userAgentString = ""; + + ChannelOption option; + if (options.TryGetValue(key, out option)) { - options.Add(new ChannelOption(ChannelOptions.PrimaryUserAgentString, GetUserAgentString())); - } + // user-provided userAgentString needs to be at the beginning + userAgentString = option.StringValue + " "; + }; + + // TODO(jtattermusch): it would be useful to also provide .NET/mono version. + userAgentString += string.Format("grpc-csharp/{0}", VersionInfo.CurrentVersion); + + options[ChannelOptions.PrimaryUserAgentString] = new ChannelOption(key, userAgentString); } - private static string GetUserAgentString() + private static Dictionary CreateOptionsDictionary(IEnumerable options) { - // TODO(jtattermusch): it would be useful to also provide .NET/mono version. - return string.Format("grpc-csharp/{0}", VersionInfo.CurrentVersion); + var dict = new Dictionary(); + if (options == null) + { + return dict; + } + foreach (var option in options) + { + dict.Add(option.Name, option); + } + return dict; } } } diff --git a/src/csharp/Grpc.Core/ChannelOptions.cs b/src/csharp/Grpc.Core/ChannelOptions.cs index f5ef63af544..d70673cf781 100644 --- a/src/csharp/Grpc.Core/ChannelOptions.cs +++ b/src/csharp/Grpc.Core/ChannelOptions.cs @@ -169,7 +169,7 @@ namespace Grpc.Core /// Creates native object for a collection of channel options. /// /// The native channel arguments. - internal static ChannelArgsSafeHandle CreateChannelArgs(List options) + internal static ChannelArgsSafeHandle CreateChannelArgs(ICollection options) { if (options == null || options.Count == 0) { @@ -179,9 +179,9 @@ namespace Grpc.Core try { nativeArgs = ChannelArgsSafeHandle.Create(options.Count); - for (int i = 0; i < options.Count; i++) + int i = 0; + foreach (var option in options) { - var option = options[i]; if (option.Type == ChannelOption.OptionType.Integer) { nativeArgs.SetInteger(i, option.Name, option.IntValue); @@ -194,6 +194,7 @@ namespace Grpc.Core { throw new InvalidOperationException("Unknown option type"); } + i++; } return nativeArgs; }