Merge pull request #1030 from jtattermusch/csharp_immutablecollections

Started using immutable collections and other code cleanup

src/csharp/Grpc.Core/Calls.cs

@@ -38,8 +38,6 @@ using Grpc.Core.Internal;
 
 namespace Grpc.Core
 {
-    // NOTE: this class is work-in-progress
-
     /// <summary>
     /// Helper methods for generated stubs to make RPC calls.
     /// </summary>

src/csharp/Grpc.Core/Channel.cs

@@ -36,6 +36,9 @@ using Grpc.Core.Internal;
 
 namespace Grpc.Core
 {
+    /// <summary>
+    /// gRPC Channel
+    /// </summary>
     public class Channel : IDisposable
     {
         readonly ChannelSafeHandle handle;

src/csharp/Grpc.Core/ChannelArgs.cs

@@ -30,6 +30,7 @@
 #endregion
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
@@ -37,33 +38,18 @@ using Grpc.Core.Internal;
 
 namespace Grpc.Core
 {
-    // TODO: should we be using the builder pattern?
+    /// <summary>
+    /// gRPC channel options.
+    /// </summary>
     public class ChannelArgs
     {
         public const string SslTargetNameOverrideKey = "grpc.ssl_target_name_override";
 
-        public class Builder
-        {
-            Dictionary<string, string> stringArgs = new Dictionary<string, string>();
-            // TODO: AddInteger not supported yet.
-            public Builder AddString(string key, string value)
-            {
-                stringArgs.Add(key, value);
-                return this;
-            }
+        readonly ImmutableDictionary<string, string> stringArgs;
 
-            public ChannelArgs Build()
-            {
-                return new ChannelArgs(stringArgs);
-            }
-        }
-
-        Dictionary<string, string> stringArgs;
-
-        private ChannelArgs(Dictionary<string, string> stringArgs)
+        private ChannelArgs(ImmutableDictionary<string, string> stringArgs)
         {
-            // TODO: use immutable dict?
-            this.stringArgs = new Dictionary<string, string>(stringArgs);
+            this.stringArgs = stringArgs;
         }
 
         public string GetSslTargetNameOverride()
@@ -76,11 +62,28 @@ namespace Grpc.Core
             return null;
         }
 
-        public static Builder NewBuilder()
+        public static Builder CreateBuilder()
         {
             return new Builder();
         }
 
+        public class Builder
+        {
+            readonly Dictionary<string, string> stringArgs = new Dictionary<string, string>();
+
+            // TODO: AddInteger not supported yet.
+            public Builder AddString(string key, string value)
+            {
+                stringArgs.Add(key, value);
+                return this;
+            }
+
+            public ChannelArgs Build()
+            {
+                return new ChannelArgs(stringArgs.ToImmutableDictionary());
+            }
+        }
+
         /// <summary>
         /// Creates native object for the channel arguments.
         /// </summary>

src/csharp/Grpc.Core/Credentials.cs

@@ -36,6 +36,9 @@ using Grpc.Core.Internal;
 
 namespace Grpc.Core
 {
+    /// <summary>
+    /// Client-side credentials.
+    /// </summary>
     public abstract class Credentials
     {
         /// <summary>

src/csharp/Grpc.Core/Grpc.Core.csproj

@@ -31,6 +31,9 @@
   </PropertyGroup>
   <ItemGroup>
     <Reference Include="System" />
+    <Reference Include="System.Collections.Immutable">
+      <HintPath>..\packages\System.Collections.Immutable.1.1.34-rc\lib\portable-net45+win8+wp8+wpa81\System.Collections.Immutable.dll</HintPath>
+    </Reference>
   </ItemGroup>
   <ItemGroup>
     <Compile Include="Internal\GrpcLog.cs" />
@@ -54,7 +57,7 @@
     <Compile Include="Internal\ServerSafeHandle.cs" />
     <Compile Include="Method.cs" />
     <Compile Include="ServerCalls.cs" />
-    <Compile Include="ServerCallHandler.cs" />
+    <Compile Include="Internal\ServerCallHandler.cs" />
     <Compile Include="Marshaller.cs" />
     <Compile Include="ServerServiceDefinition.cs" />
     <Compile Include="Utils\RecordingObserver.cs" />
@@ -77,6 +80,9 @@
     <Compile Include="Internal\ServerCredentialsSafeHandle.cs" />
     <Compile Include="ServerCredentials.cs" />
   </ItemGroup>
+  <ItemGroup>
+    <None Include="packages.config" />
+  </ItemGroup>
   <Choose>
     <!-- Under older versions of Monodevelop, Choose is not supported and is just
          ignored, which gives us the desired effect. -->

src/csharp/Grpc.Core/ServerCallHandler.cs → src/csharp/Grpc.Core/Internal/ServerCallHandler.cs

@@ -36,7 +36,7 @@ using System.Linq;
 using Grpc.Core.Internal;
 using Grpc.Core.Utils;
 
-namespace Grpc.Core
+namespace Grpc.Core.Internal
 {
     internal interface IServerCallHandler
     {

src/csharp/Grpc.Core/Marshaller.cs

@@ -66,6 +66,9 @@ namespace Grpc.Core
         }
     }
 
+    /// <summary>
+    /// Utilities for creating marshallers.
+    /// </summary>
     public static class Marshallers
     {
         public static Marshaller<T> Create<T>(Func<T, byte[]> serializer, Func<byte[], T> deserializer)

src/csharp/Grpc.Core/RpcException.cs

@@ -35,6 +35,9 @@ using System;
 
 namespace Grpc.Core
 {
+    /// <summary>
+    /// Thrown when remote procedure call fails.
+    /// </summary>
     public class RpcException : Exception
     {
         private readonly Status status;

src/csharp/Grpc.Core/ServerCalls.cs

@@ -32,6 +32,7 @@
 #endregion
 
 using System;
+using Grpc.Core.Internal;
 
 namespace Grpc.Core
 {

src/csharp/Grpc.Core/ServerCredentials.cs

@@ -33,10 +33,14 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using Grpc.Core.Internal;
 
 namespace Grpc.Core
 {
+    /// <summary>
+    /// Server side credentials.
+    /// </summary>
     public abstract class ServerCredentials
     {
         /// <summary>
@@ -51,8 +55,8 @@ namespace Grpc.Core
     /// </summary>
     public class KeyCertificatePair
     {
-        string certChain;
-        string privateKey;
+        readonly string certChain;
+        readonly string privateKey;
 
         public KeyCertificatePair(string certChain, string privateKey)
         {
@@ -82,10 +86,9 @@ namespace Grpc.Core
     /// </summary>
     public class SslServerCredentials : ServerCredentials
     {
-        // TODO: immutable list...
-        List<KeyCertificatePair> keyCertPairs;
+        ImmutableList<KeyCertificatePair> keyCertPairs;
 
-        public SslServerCredentials(List<KeyCertificatePair> keyCertPairs)
+        public SslServerCredentials(ImmutableList<KeyCertificatePair> keyCertPairs)
         {
             this.keyCertPairs = keyCertPairs;
         }

src/csharp/Grpc.Core/ServerServiceDefinition.cs

@@ -33,22 +33,26 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
+using Grpc.Core.Internal;
 
 namespace Grpc.Core
 {
+    /// <summary>
+    /// Mapping of method names to server call handlers.
+    /// </summary>
     public class ServerServiceDefinition
     {
         readonly string serviceName;
-        // TODO: we would need an immutable dictionary here...
-        readonly Dictionary<string, IServerCallHandler> callHandlers;
+        readonly ImmutableDictionary<string, IServerCallHandler> callHandlers;
 
-        private ServerServiceDefinition(string serviceName, Dictionary<string, IServerCallHandler> callHandlers)
+        private ServerServiceDefinition(string serviceName, ImmutableDictionary<string, IServerCallHandler> callHandlers)
         {
             this.serviceName = serviceName;
-            this.callHandlers = new Dictionary<string, IServerCallHandler>(callHandlers);
+            this.callHandlers = callHandlers;
         }
 
-        internal Dictionary<string, IServerCallHandler> CallHandlers
+        internal ImmutableDictionary<string, IServerCallHandler> CallHandlers
         {
             get
             {
@@ -89,7 +93,7 @@ namespace Grpc.Core
 
             public ServerServiceDefinition Build()
             {
-                return new ServerServiceDefinition(serviceName, callHandlers);
+                return new ServerServiceDefinition(serviceName, callHandlers.ToImmutableDictionary());
             }
         }
     }

src/csharp/Grpc.Core/StatusCode.cs

@@ -35,9 +35,9 @@ using System;
 
 namespace Grpc.Core
 {
-    // TODO: element names should changed to comply with C# naming conventions.
     /// <summary>
-    /// based on grpc_status_code from grpc/status.h
+    /// Result of a remote procedure call.
+    /// Based on grpc_status_code from grpc/status.h
     /// </summary>
     public enum StatusCode
     {

src/csharp/Grpc.Core/packages.config

@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>
+<packages>
+  <package id="System.Collections.Immutable" version="1.1.34-rc" targetFramework="net45" />
+</packages>

src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj

@@ -39,6 +39,10 @@
     <Reference Include="Google.ProtocolBuffers">
       <HintPath>..\packages\Google.ProtocolBuffers.2.4.1.521\lib\net40\Google.ProtocolBuffers.dll</HintPath>
     </Reference>
+    <Reference Include="System.Collections.Immutable, Version=1.1.34.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
+      <SpecificVersion>False</SpecificVersion>
+      <HintPath>..\packages\System.Collections.Immutable.1.1.34-rc\lib\portable-net45+win8+wp8+wpa81\System.Collections.Immutable.dll</HintPath>
+    </Reference>
   </ItemGroup>
   <ItemGroup>
     <Compile Include="Properties\AssemblyInfo.cs" />
@@ -76,8 +80,5 @@
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
   </ItemGroup>
-  <ItemGroup>
-    <Folder Include="proto\" />
-    <Folder Include="data\" />
-  </ItemGroup>
-</Project>
+  <ItemGroup />
+</Project>

src/csharp/Grpc.IntegrationTesting/InteropClient.cs

@@ -109,7 +109,7 @@ namespace Grpc.IntegrationTesting
             ChannelArgs channelArgs = null;
             if (!string.IsNullOrEmpty(options.serverHostOverride))
             {
-                channelArgs = ChannelArgs.NewBuilder()
+                channelArgs = ChannelArgs.CreateBuilder()
                     .AddString(ChannelArgs.SslTargetNameOverrideKey, options.serverHostOverride).Build();
             }
 

src/csharp/Grpc.IntegrationTesting/InteropClientServerTest.cs

@@ -62,7 +62,7 @@ namespace Grpc.IntegrationTesting
             int port = server.AddPort(host + ":0", TestCredentials.CreateTestServerCredentials());
             server.Start();
 
-            var channelArgs = ChannelArgs.NewBuilder()
+            var channelArgs = ChannelArgs.CreateBuilder()
                 .AddString(ChannelArgs.SslTargetNameOverrideKey, TestCredentials.DefaultHostOverride).Build();
 
             channel = new Channel(host + ":" + port, TestCredentials.CreateTestClientCredentials(true), channelArgs);

src/csharp/Grpc.IntegrationTesting/TestCredentials.cs

@@ -33,6 +33,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Diagnostics;
 using System.IO;
 using System.Text.RegularExpressions;
@@ -77,7 +78,7 @@ namespace Grpc.IntegrationTesting
             var keyCertPair = new KeyCertificatePair(
                 File.ReadAllText(ServerCertChainPath),
                 File.ReadAllText(ServerPrivateKeyPath));
-            return new SslServerCredentials(new List<KeyCertificatePair> { keyCertPair });
+            return new SslServerCredentials(ImmutableList.Create(keyCertPair));
         }
     }
 }

src/csharp/Grpc.IntegrationTesting/packages.config

@@ -2,4 +2,5 @@
 <packages>
   <package id="Google.ProtocolBuffers" version="2.4.1.521" targetFramework="net45" />
   <package id="NUnit" version="2.6.4" targetFramework="net45" />
+  <package id="System.Collections.Immutable" version="1.1.34-rc" targetFramework="net45" />
 </packages>