Merge pull request #128 from jaredmoo/MultiShardConnectionMultipleEnu…

…meration

Refactored MultiShardConnection validation and evaluate enumerables once

Src/ElasticScale.Client/Query/MultiShardConnection.cs

@@ -66,19 +66,24 @@ public MultiShardConnection(IEnumerable<Shard> shards, string connectionString)
             {
                 throw new ArgumentNullException("connectionString");
             }
+            if (shards == null)
+            {
+                throw new ArgumentNullException("shards");
+            }
 
             // Enhance the ApplicationName with this library's name as a suffix
             // Devnote: If connection string specifies Active Directory authentication and runtime is not
             // .NET 4.6 or higher, then below call will throw.
             SqlConnectionStringBuilder connectionStringBuilder = new SqlConnectionStringBuilder(
-                connectionString).WithApplicationNameSuffix(ApplicationNameSuffix); 
+                connectionString).WithApplicationNameSuffix(ApplicationNameSuffix);
+            ValidateConnectionString(connectionStringBuilder);
 
-            ValidateConnectionArguments(shards, "shards", connectionStringBuilder);
+            // Force evaluation of the input enumerable so that we don't evaluate it multiple times later
+            this.Shards = shards.ToList();
+            ValidateNotEmpty(this.Shards, "shards");
 
-            this.Shards = shards;
-            this.ShardConnections = shards.Select(
-                s => (CreateDbConnectionForLocation(s.Location, connectionStringBuilder))
-                ).ToList();
+            this.ShardConnections = this.Shards.Select(
+                s => CreateDbConnectionForLocation(s.Location, connectionStringBuilder)).ToList();
         }
 
         /// <summary>
@@ -99,21 +104,27 @@ public MultiShardConnection(IEnumerable<ShardLocation> shardLocations, string co
             {
                 throw new ArgumentNullException("connectionString");
             }
+            if (shardLocations == null)
+            {
+                throw new ArgumentNullException("shardLocations");
+            }
 
             // Enhance the ApplicationName with this library's name as a suffix
             // Devnote: If connection string specifies Active Directory authentication and runtime is not
             // .NET 4.6 or higher, then below call will throw.
             SqlConnectionStringBuilder connectionStringBuilder = new SqlConnectionStringBuilder(
-                connectionString).WithApplicationNameSuffix(ApplicationNameSuffix); 
+                connectionString).WithApplicationNameSuffix(ApplicationNameSuffix);
+            ValidateConnectionString(connectionStringBuilder);
 
-            ValidateConnectionArguments(shardLocations, "shardLocations", connectionStringBuilder);
+            // Force evaluation of the input enumerable so that we don't evaluate it multiple times later
+            IList<ShardLocation> shardLocationsList = shardLocations.ToList();
+            ValidateNotEmpty(shardLocationsList, "shardLocations");
 
             this.Shards = null;
-            this.ShardConnections = shardLocations.Select(
-                s => (CreateDbConnectionForLocation(s, connectionStringBuilder))
-                ).ToList();
+            this.ShardConnections = shardLocationsList.Select(
+                s => CreateDbConnectionForLocation(s, connectionStringBuilder)).ToList();
         }
-        
+
         /// <summary>
         /// Creates an instance of this class 
         /// /* TEST ONLY */
@@ -196,21 +207,19 @@ public void Dispose()
 
 #region Helpers
 
-        private static void ValidateConnectionArguments<T>(
+        private static void ValidateNotEmpty<T>(
             IEnumerable<T> namedCollection,
-            string collectionName,
-            SqlConnectionStringBuilder connectionStringBuilder)
+            string collectionName)
         {
-            if (namedCollection == null)
-            {
-                throw new ArgumentNullException(collectionName);
-            }
-
-            if (0 == namedCollection.Count())
+            if (!namedCollection.Any())
             {
                 throw new ArgumentException(string.Format("No {0} provided.", collectionName));
             }
+        }
 
+        private static void ValidateConnectionString(
+            SqlConnectionStringBuilder connectionStringBuilder)
+        {
             // Datasource must not be set
             if (!string.IsNullOrEmpty(connectionStringBuilder.DataSource))
             {
@@ -228,7 +237,7 @@ private static void ValidateConnectionArguments<T>(
         //
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")]
         private static Tuple<ShardLocation, DbConnection> CreateDbConnectionForLocation(
-            ShardLocation shardLocation, 
+            ShardLocation shardLocation,
             SqlConnectionStringBuilder connectionStringBuilder)
         {
             return new Tuple<ShardLocation, DbConnection>

Test/ElasticScale.Query.UnitTests/EnumerableHelpers.cs

@@ -0,0 +1,49 @@
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Threading;
+
+namespace Microsoft.Azure.SqlDatabase.ElasticScale.Query.UnitTests
+{
+    public static class EnumerableHelpers
+    {
+        public static IEnumerable<T> ToConsumable<T>(this IEnumerable<T> source)
+        {
+            return new ConsumingEnumerable<T>(source);
+        }
+
+        /// <summary>
+        /// IEnumerable wrapper that can only be enumerated once.
+        /// </summary>
+        /// <typeparam name="T"></typeparam>
+        public class ConsumingEnumerable<T> : IEnumerable<T>
+        {
+            private IEnumerable<T> _source;
+            private int _consumed;
+
+            public ConsumingEnumerable(IEnumerable<T> source)
+            {
+                _source = source;
+                _consumed = 0;
+            }
+
+            public IEnumerator<T> GetEnumerator()
+            {
+                int wasConsumed = Interlocked.Exchange(ref _consumed, 1);
+                if (wasConsumed == 0)
+                {
+                    return _source.GetEnumerator();
+                }
+                else
+                {
+                    throw new InvalidOperationException("GetEnumerator() has already been called. Cannot enumerate more than once");
+                }
+            }
+
+            IEnumerator IEnumerable.GetEnumerator()
+            {
+                return GetEnumerator();
+            }
+        }
+    }
+}

Test/ElasticScale.Query.UnitTests/MultiShardConnectionTests.cs

@@ -0,0 +1,71 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Microsoft.Azure.SqlDatabase.ElasticScale.ShardManagement;
+using Microsoft.Azure.SqlDatabase.ElasticScale.Test.Common;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Microsoft.Azure.SqlDatabase.ElasticScale.Query.UnitTests
+{
+    [TestClass]
+    public class MultiShardConnectionTests
+    {
+        private const string dummyConnectionString = "User ID=x;Password=x";
+
+        /// <summary>
+        /// Verifies that <see cref="MultiShardConnection.MultiShardConnection(IEnumerable{ShardLocation}, string)"/>
+        /// throws when the input shardlocations are null
+        /// </summary>
+        [TestMethod]
+        public void TestMultiShardConnectionConstructorThrowsNullShardLocations()
+        {
+            AssertExtensions.AssertThrows<ArgumentNullException>(
+                () => new MultiShardConnection((IEnumerable<ShardLocation>)null, dummyConnectionString));
+        }
+
+        /// <summary>
+        /// Verifies that <see cref="MultiShardConnection.MultiShardConnection(IEnumerable{ShardLocation}, string)"/>
+        /// throws when the input shards are null
+        /// </summary>
+        [TestMethod]
+        public void TestMultiShardConnectionConstructorThrowsNullShards()
+        {
+            AssertExtensions.AssertThrows<ArgumentNullException>(
+                () => new MultiShardConnection((IEnumerable<Shard>)null, dummyConnectionString));
+        }
+
+        /// <summary>
+        /// Verifies that <see cref="MultiShardConnection.MultiShardConnection(IEnumerable{ShardLocation}, string)"/>
+        /// does not multiply evaluate the input enumerable
+        /// </summary>
+        [TestMethod]
+        public void TestMultiShardConnectionConstructorEvaluatesShardLocations()
+        {
+            List<ShardLocation> shardLocations = new List<ShardLocation>
+            {
+                new ShardLocation("server1", "db1"),
+                new ShardLocation("server2", "db2"),
+                new ShardLocation("server3", "db3")
+            };
+
+            MultiShardConnection conn = new MultiShardConnection(shardLocations.ToConsumable(), dummyConnectionString);
+            AssertExtensions.AssertSequenceEqual(shardLocations, conn.ShardLocations);
+        }
+
+        /// <summary>
+        /// Verifies that <see cref="MultiShardConnection.MultiShardConnection(IEnumerable{Shard}, string)"/>
+        /// does not multiply evaluate the input enumerable
+        /// </summary>
+        [TestMethod]
+        public void TestMultiShardConnectionConstructorEvaluatesShards()
+        {
+            MultiShardTestUtils.DropAndCreateDatabases();
+            ShardMap shardMap = MultiShardTestUtils.CreateAndGetTestShardMap();
+
+            List<Shard> shards = shardMap.GetShards().ToList();
+
+            MultiShardConnection conn = new MultiShardConnection(shards.ToConsumable(), dummyConnectionString);
+            AssertExtensions.AssertSequenceEqual(shards, conn.Shards);
+        }
+    }
+}