Merge pull request #4365 from jtattermusch/user_agent_str

Put user defined userAgentString to the beginning of userAgentString

7 files changed, 152 insertions, 31 deletions

diff --git a/src/csharp/Grpc.Core.Tests/ChannelTest.cs b/src/csharp/Grpc.Core.Tests/ChannelTest.cs
index f4ae9abefd..ed0ec14df5 100644
--- a/src/csharp/Grpc.Core.Tests/ChannelTest.cs
+++ b/src/csharp/Grpc.Core.Tests/ChannelTest.cs
@@ -48,6 +48,17 @@ namespace Grpc.Core.Tests
         }
 
         [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()
         {
             var channel = new Channel("localhost", ChannelCredentials.Insecure);
diff --git a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs
index 25a5a27c8e..b683751bc0 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()
         {
@@ -219,18 +219,6 @@ namespace Grpc.Core.Tests
         }
 
         [Test]
-        public void UserAgentStringPresent()
-        {
-            helper.UnaryHandler = new UnaryServerMethod<string, string>(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()
         {
             helper.UnaryHandler = new UnaryServerMethod<string, string>(async (request, context) =>
diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj
index e5ffa31989..70b83f7fb1 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 @@
       <Link>Version.cs</Link>
     </Compile>
     <Compile Include="CallCredentialsTest.cs" />
+    <Compile Include="UserAgentStringTest.cs" />
     <Compile Include="FakeCredentials.cs" />
     <Compile Include="MarshallingErrorsTest.cs" />
     <Compile Include="ChannelCredentialsTest.cs" />
diff --git a/src/csharp/Grpc.Core.Tests/MockServiceHelper.cs b/src/csharp/Grpc.Core.Tests/MockServiceHelper.cs
index 567e04eddc..3047314345 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<ChannelOption> channelOptions;
 
         readonly Method<string, string> unaryMethod;
         readonly Method<string, string> clientStreamingMethod;
@@ -66,9 +68,10 @@ namespace Grpc.Core.Tests
         Server server;
         Channel channel;
 
-        public MockServiceHelper(string host = null, Marshaller<string> marshaller = null)
+        public MockServiceHelper(string host = null, Marshaller<string> marshaller = null, IEnumerable<ChannelOption> channelOptions = null)
         {
             this.host = host ?? "localhost";
+            this.channelOptions = channelOptions;
             marshaller = marshaller ?? Marshallers.StringMarshaller;
 
             unaryMethod = new Method<string, string>(
@@ -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 0000000000..cc830086a6
--- /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<string, string>((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<string, string>((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 ec60354639..d8d43c7998 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<ChannelOption> options;
+        readonly Dictionary<string, ChannelOption> options;
 
         bool shutdownRequested;
 
@@ -71,12 +69,12 @@ namespace Grpc.Core
         public Channel(string target, ChannelCredentials credentials, IEnumerable<ChannelOption> 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<ChannelOption>(options) : new List<ChannelOption>();
 
-            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<ChannelOption> options)
+        private static void EnsureUserAgentChannelOption(Dictionary<string, ChannelOption> 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<string, ChannelOption> CreateOptionsDictionary(IEnumerable<ChannelOption> 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<string, ChannelOption>();
+            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 f5ef63af54..d70673cf78 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.
         /// </summary>
         /// <returns>The native channel arguments.</returns>
-        internal static ChannelArgsSafeHandle CreateChannelArgs(List<ChannelOption> options)
+        internal static ChannelArgsSafeHandle CreateChannelArgs(ICollection<ChannelOption> 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;
             }