3278 Kusto Unit Tests - Part 2 (#1063)

* 3278 Added unit tests in MetadataFactoryTests and Microsoft.Kusto.ServiceLayer.UnitTests project

* 3278 Removed todo and changed unit test to validate megabytes

* 3278 Added file and unit tests in AutoCompleteHelperTests.cs

* 3278 Removed unused functions from Kusto > ScriptAsScriptingOperation

* 3278 Added unit tests for DataSourceFactory

* 3278 Refactored AdminService to pass in ConnectionService rather than through instance variable. Added unit test for AdminServiceTests

* 3278 Refactored DataSourceFactory to not have static functions for future unit tests

* 3278 Re-added properties that were flagged as unused but are being used by ADS in ReliableDataSourceConnection.cs

* 3278 Re-added properties that were flagged as unused but are being used by ADS in ReliableDataSourceConnection.cs

* adding pipeline to execute tests (#1062)

* 3278 Converted GetDefaultAutoComplete and GetDefaultSemanticMarkers to static functions in DataSourceFactory. Removed unused constructor in ScriptFile. Added positive unit tests for both functions

* undoing release version bump

* adding additional configs

* 3278 Minor refactors in ConnectionInfo, BindingQueue, DiagnosticsHelper, MetadataService, and HostLoader. Changed AssemblyInfo to only allow Kusto Unit Tests for internal access. Added lots of unit tests.

* 3278 Commented out bindingContext.IsConnected in AddConnectionContext_Sets_BindingContext

* 3278 Reversed order of unit tests in ConnectedBindingQueueTests and added throw to Catch block.

* 3278 Reverted change to ConnectedBindingQueue. Removed unit test from AddConnectionContext for NeedsMetaData True

Co-authored-by: Jorge Berumen <52225468+joberume@users.noreply.github.com>
Co-authored-by: joberume <jberumen3@miners.utep.edu>

src/Microsoft.Kusto.ServiceLayer/Connection/ConnectionInfo.cs

@@ -6,7 +6,6 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
-using System.Data.Common;
 using Microsoft.Kusto.ServiceLayer.Connection.Contracts;
 using Microsoft.SqlTools.Utility;
 
@@ -27,6 +26,7 @@ namespace Microsoft.Kusto.ServiceLayer.Connection
             ConnectionDetails = details;
             ConnectionId = Guid.NewGuid();
             IntellisenseMetrics = new InteractionMetrics<double>(new int[] {50, 100, 200, 500, 1000, 2000});
+            _connectionTypeToConnectionMap = new ConcurrentDictionary<string, ReliableDataSourceConnection>();
         }
 
         /// <summary>
@@ -54,13 +54,12 @@ namespace Microsoft.Kusto.ServiceLayer.Connection
         /// this ConnectionInfo's OwnerUri.
         /// This is internal for testing access only
         /// </summary>
-        internal readonly ConcurrentDictionary<string, ReliableDataSourceConnection> ConnectionTypeToConnectionMap =
-            new ConcurrentDictionary<string, ReliableDataSourceConnection>();
+        private readonly ConcurrentDictionary<string, ReliableDataSourceConnection> _connectionTypeToConnectionMap;
 
         /// <summary>
         /// Intellisense Metrics
         /// </summary>
-        public InteractionMetrics<double> IntellisenseMetrics { get; private set; }
+        public InteractionMetrics<double> IntellisenseMetrics { get; }
 
         /// <summary>
         /// Returns true if the db connection is to any cloud instance
@@ -79,7 +78,7 @@ namespace Microsoft.Kusto.ServiceLayer.Connection
         {
             get
             {
-                return ConnectionTypeToConnectionMap.Values;
+                return _connectionTypeToConnectionMap.Values;
             }
         }
 
@@ -91,14 +90,14 @@ namespace Microsoft.Kusto.ServiceLayer.Connection
         {
             get
             {
-                return ConnectionTypeToConnectionMap.Keys;
+                return _connectionTypeToConnectionMap.Keys;
             }
         }
 
         public bool HasConnectionType(string connectionType)
         {
             connectionType = connectionType ?? ConnectionType.Default;
-            return ConnectionTypeToConnectionMap.ContainsKey(connectionType);
+            return _connectionTypeToConnectionMap.ContainsKey(connectionType);
         }
 
         /// <summary>
@@ -108,7 +107,7 @@ namespace Microsoft.Kusto.ServiceLayer.Connection
         {
             get
             {
-                return ConnectionTypeToConnectionMap.Count;
+                return _connectionTypeToConnectionMap.Count;
             }
         }
 
@@ -121,7 +120,7 @@ namespace Microsoft.Kusto.ServiceLayer.Connection
         public bool TryGetConnection(string connectionType, out ReliableDataSourceConnection connection)
         {
             Validate.IsNotNullOrEmptyString("Connection Type", connectionType);
-            return ConnectionTypeToConnectionMap.TryGetValue(connectionType, out connection);
+            return _connectionTypeToConnectionMap.TryGetValue(connectionType, out connection);
         }
 
         /// <summary>
@@ -133,7 +132,7 @@ namespace Microsoft.Kusto.ServiceLayer.Connection
         public void AddConnection(string connectionType, ReliableDataSourceConnection connection)
         {
             Validate.IsNotNullOrEmptyString("Connection Type", connectionType);
-            ConnectionTypeToConnectionMap.TryAdd(connectionType, connection);
+            _connectionTypeToConnectionMap.TryAdd(connectionType, connection);
         }
 
         /// <summary>
@@ -144,7 +143,7 @@ namespace Microsoft.Kusto.ServiceLayer.Connection
         {
             Validate.IsNotNullOrEmptyString("Connection Type", connectionType);
             ReliableDataSourceConnection connection;
-            ConnectionTypeToConnectionMap.TryRemove(connectionType, out connection);
+            _connectionTypeToConnectionMap.TryRemove(connectionType, out connection);
         }
 
         /// <summary>
@@ -155,7 +154,7 @@ namespace Microsoft.Kusto.ServiceLayer.Connection
             foreach (var type in AllConnectionTypes)
             {
                 ReliableDataSourceConnection connection;
-                ConnectionTypeToConnectionMap.TryRemove(type, out connection);
+                _connectionTypeToConnectionMap.TryRemove(type, out connection);
             }
         } 
     }

src/Microsoft.Kusto.ServiceLayer/HostLoader.cs

@@ -97,7 +97,7 @@ namespace Microsoft.Kusto.ServiceLayer
             AdminService.Instance.InitializeService(serviceHost, ConnectionService.Instance);
             serviceProvider.RegisterSingleService(AdminService.Instance);
 
-            MetadataService.Instance.InitializeService(serviceHost);
+            MetadataService.Instance.InitializeService(serviceHost, ConnectionService.Instance);
             serviceProvider.RegisterSingleService(MetadataService.Instance);
             
             InitializeHostedServices(serviceProvider, serviceHost);

src/Microsoft.Kusto.ServiceLayer/LanguageServices/BindingQueue.cs

@@ -22,19 +22,17 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
     /// </summary>
     public class BindingQueue<T> : IDisposable where T : IBindingContext, new()
     {
-        internal const int QueueThreadStackSize = 5 * 1024 * 1024;
+        private CancellationTokenSource _processQueueCancelToken;
 
-        private CancellationTokenSource processQueueCancelToken = null;
+        private readonly ManualResetEvent _itemQueuedEvent;
 
-        private ManualResetEvent itemQueuedEvent = new ManualResetEvent(initialState: false);
+        private readonly object _bindingQueueLock;
 
-        private object bindingQueueLock = new object();
+        private readonly LinkedList<QueueItem> _bindingQueue;
 
-        private LinkedList<QueueItem> bindingQueue = new LinkedList<QueueItem>();
+        private readonly object _bindingContextLock ;
 
-        private object bindingContextLock = new object();
-
-        private Task queueProcessorTask;
+        private Task _queueProcessorTask;
 
         public delegate void UnhandledExceptionDelegate(string connectionKey, Exception ex);
 
@@ -44,22 +42,27 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// Map from context keys to binding context instances
         /// Internal for testing purposes only
         /// </summary>
-        internal Dictionary<string, IBindingContext> BindingContextMap { get; set; }
+        internal Dictionary<string, IBindingContext> BindingContextMap { get; }
 
-        internal Dictionary<IBindingContext, Task> BindingContextTasks { get; set; } = new Dictionary<IBindingContext, Task>();
+        private Dictionary<IBindingContext, Task> BindingContextTasks { get; }
 
         /// <summary>
         /// Constructor for a binding queue instance
         /// </summary>
-        public BindingQueue()
+        internal BindingQueue()
         {
-            this.BindingContextMap = new Dictionary<string, IBindingContext>();
-            this.StartQueueProcessor();
+            BindingContextMap = new Dictionary<string, IBindingContext>();
+            _itemQueuedEvent = new ManualResetEvent(initialState: false);
+            _bindingQueueLock = new object();
+            _bindingQueue = new LinkedList<QueueItem>();
+            _bindingContextLock = new object();
+            BindingContextTasks = new Dictionary<IBindingContext, Task>();
+            StartQueueProcessor();
         }
 
-        public void StartQueueProcessor()
+        private void StartQueueProcessor()
         {
-            this.queueProcessorTask = StartQueueProcessorAsync();
+            this._queueProcessorTask = StartQueueProcessorAsync();
         }
 
         /// <summary>
@@ -68,8 +71,8 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// <param name="timeout"></param>
         public bool StopQueueProcessor(int timeout)
         {
-            this.processQueueCancelToken.Cancel();
-            return this.queueProcessorTask.Wait(timeout);
+            this._processQueueCancelToken.Cancel();
+            return this._queueProcessorTask.Wait(timeout);
         }
 
         /// <summary>
@@ -80,14 +83,14 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         {
             get
             {
-                return this.processQueueCancelToken.IsCancellationRequested;
+                return this._processQueueCancelToken.IsCancellationRequested;
             }
         }
 
         /// <summary>
         /// Queue a binding request item
         /// </summary>
-        public virtual QueueItem QueueBindingOperation(
+        public QueueItem QueueBindingOperation(
             string key,
             Func<IBindingContext, CancellationToken, object> bindOperation,
             Func<IBindingContext, object> timeoutOperation = null,
@@ -111,12 +114,12 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
                 WaitForLockTimeout = waitForLockTimeout
             };
 
-            lock (this.bindingQueueLock)
+            lock (this._bindingQueueLock)
             {
-                this.bindingQueue.AddLast(queueItem);
+                this._bindingQueue.AddLast(queueItem);
             }
 
-            this.itemQueuedEvent.Set();
+            this._itemQueuedEvent.Set();
 
             return queueItem;
         }
@@ -127,7 +130,7 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// <param name="key"></param>
         public bool IsBindingContextConnected(string key)
         {
-            lock (this.bindingContextLock)
+            lock (this._bindingContextLock)
             {
                 IBindingContext context;
                 if (this.BindingContextMap.TryGetValue(key, out context))
@@ -142,7 +145,7 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// Gets or creates a binding context for the provided context key
         /// </summary>
         /// <param name="key"></param>
-        protected IBindingContext GetOrCreateBindingContext(string key)
+        internal IBindingContext GetOrCreateBindingContext(string key)
         {
             // use a default binding context for disconnected requests
             if (string.IsNullOrWhiteSpace(key))
@@ -150,7 +153,7 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
                 key = "disconnected_binding_context";
             }
                         
-            lock (this.bindingContextLock)
+            lock (this._bindingContextLock)
             {
                 if (!this.BindingContextMap.ContainsKey(key))
                 {
@@ -171,7 +174,7 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
                 keyPrefix = "disconnected_binding_context";
             }
 
-            lock (this.bindingContextLock)
+            lock (this._bindingContextLock)
             {
                 return this.BindingContextMap.Where(x => x.Key.StartsWith(keyPrefix)).Select(v => v.Value);
             }
@@ -182,7 +185,7 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// </summary>
         protected bool BindingContextExists(string key)
         {
-            lock (this.bindingContextLock)
+            lock (this._bindingContextLock)
             {
                 return this.BindingContextMap.ContainsKey(key);
             }
@@ -193,7 +196,7 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// </summary>
         protected void RemoveBindingContext(string key)
         {
-            lock (this.bindingContextLock)
+            lock (this._bindingContextLock)
             {
                 if (this.BindingContextMap.ContainsKey(key))
                 {
@@ -216,13 +219,13 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
             }
         }
 
-        public bool HasPendingQueueItems
+        private bool HasPendingQueueItems
         {
             get
             {
-                lock (this.bindingQueueLock)
+                lock (this._bindingQueueLock)
                 {
-                    return this.bindingQueue.Count > 0;
+                    return this._bindingQueue.Count > 0;
                 }
             }
         }
@@ -232,15 +235,15 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// </summary>
         private QueueItem GetNextQueueItem()
         {
-            lock (this.bindingQueueLock)
+            lock (this._bindingQueueLock)
             {
-                if (this.bindingQueue.Count == 0)
+                if (this._bindingQueue.Count == 0)
                 {
                     return null;
                 }
 
-                QueueItem queueItem = this.bindingQueue.First.Value;
-                this.bindingQueue.RemoveFirst();
+                QueueItem queueItem = this._bindingQueue.First.Value;
+                this._bindingQueue.RemoveFirst();
                 return queueItem;
             }
         }
@@ -250,15 +253,15 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// </summary>        
         private Task StartQueueProcessorAsync()
         {
-            if (this.processQueueCancelToken != null)
+            if (this._processQueueCancelToken != null)
             {
-                this.processQueueCancelToken.Dispose();
+                this._processQueueCancelToken.Dispose();
             }
-            this.processQueueCancelToken = new CancellationTokenSource();
+            this._processQueueCancelToken = new CancellationTokenSource();
 
             return Task.Factory.StartNew(
                 ProcessQueue,
-                this.processQueueCancelToken.Token,
+                this._processQueueCancelToken.Token,
                 TaskCreationOptions.LongRunning,
                 TaskScheduler.Default);
         }
@@ -266,13 +269,12 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// <summary>
         /// The core queue processing method
         /// </summary>
-        /// <param name="state"></param>
         private void ProcessQueue()
         {
-            CancellationToken token = this.processQueueCancelToken.Token;
+            CancellationToken token = this._processQueueCancelToken.Token;
             WaitHandle[] waitHandles = new WaitHandle[2]
             {
-                this.itemQueuedEvent,
+                this._itemQueuedEvent,
                 token.WaitHandle
             };
     
@@ -443,13 +445,13 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
                 }
                 finally
                 {
-                    lock (this.bindingQueueLock)
+                    lock (this._bindingQueueLock)
                     {
                         // verify the binding queue is still empty
-                        if (this.bindingQueue.Count == 0)
+                        if (this._bindingQueue.Count == 0)
                         {
                             // reset the item queued event since we've processed all the pending items
-                            this.itemQueuedEvent.Reset();
+                            this._itemQueuedEvent.Reset();
                         }
                     }
                 }
@@ -461,25 +463,25 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// </summary>
         public void ClearQueuedItems()
         {
-            lock (this.bindingQueueLock)
+            lock (this._bindingQueueLock)
             {
-                if (this.bindingQueue.Count > 0)
+                if (this._bindingQueue.Count > 0)
                 {
-                    this.bindingQueue.Clear();
+                    this._bindingQueue.Clear();
                 }
             }
         }
 
         public void Dispose()
         {
-            if (this.processQueueCancelToken != null)
+            if (this._processQueueCancelToken != null)
             {
-                this.processQueueCancelToken.Dispose();
+                this._processQueueCancelToken.Dispose();
             }
 
-            if (itemQueuedEvent != null)
+            if (_itemQueuedEvent != null)
             {
-                itemQueuedEvent.Dispose();
+                _itemQueuedEvent.Dispose();
             }
 
             if (this.BindingContextMap != null)

src/Microsoft.Kusto.ServiceLayer/LanguageServices/DiagnosticsHelper.cs

@@ -77,7 +77,7 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// </summary>
         /// <param name="scriptFileMarker"></param>
         /// <returns></returns>
-        internal static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMarker)
+        private static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMarker)
         {
             return new Diagnostic
             {
@@ -103,7 +103,7 @@ namespace Microsoft.Kusto.ServiceLayer.LanguageServices
         /// Map ScriptFileMarker severity to Diagnostic severity
         /// </summary>
         /// <param name="markerLevel"></param>        
-        internal static DiagnosticSeverity MapDiagnosticSeverity(ScriptFileMarkerLevel markerLevel)
+        private static DiagnosticSeverity MapDiagnosticSeverity(ScriptFileMarkerLevel markerLevel)
         {
             switch (markerLevel)
             {

src/Microsoft.Kusto.ServiceLayer/Metadata/MetadataService.cs

@@ -24,35 +24,18 @@ namespace Microsoft.Kusto.ServiceLayer.Metadata
 
         public static MetadataService Instance => LazyInstance.Value;
 
-        private static ConnectionService connectionService = null;
-
-        /// <summary>
-        /// Internal for testing purposes only
-        /// </summary>
-        internal static ConnectionService ConnectionServiceInstance
-        {
-            get
-            {
-                if (connectionService == null)
-                {
-                    connectionService = ConnectionService.Instance;
-                }
-                return connectionService;
-            }
-
-            set
-            {
-                connectionService = value;
-            }
-        }
+        private static ConnectionService _connectionService;
+        
+        internal Task MetadataListTask { get; private set; }
 
         /// <summary>
         /// Initializes the Metadata Service instance
         /// </summary>
         /// <param name="serviceHost"></param>
-        /// <param name="metadataFactory"></param>
-        public void InitializeService(IProtocolEndpoint serviceHost)
+        /// <param name="connectionService"></param>
+        public void InitializeService(IProtocolEndpoint serviceHost, ConnectionService connectionService)
         {
+            _connectionService = connectionService;
             serviceHost.SetRequestHandler(MetadataListRequest.Type, HandleMetadataListRequest);
         }
 
@@ -68,7 +51,7 @@ namespace Microsoft.Kusto.ServiceLayer.Metadata
                 Func<Task> requestHandler = async () =>
                 {
                     ConnectionInfo connInfo;
-                    MetadataService.ConnectionServiceInstance.TryFindConnection(metadataParams.OwnerUri, out connInfo);
+                    _connectionService.TryFindConnection(metadataParams.OwnerUri, out connInfo);
 
                     var metadata = new List<ObjectMetadata>();
                     if (connInfo != null)
@@ -102,6 +85,6 @@ namespace Microsoft.Kusto.ServiceLayer.Metadata
             }
         }
 
-        internal Task MetadataListTask { get; set; }
+        
     }
 }

src/Microsoft.Kusto.ServiceLayer/Properties/AssemblyInfo.cs

@@ -41,10 +41,7 @@ using System.Runtime.InteropServices;
 [assembly: AssemblyFileVersion("1.0.0.0")]
 [assembly: AssemblyInformationalVersion("1.0.0.0")]
 
-[assembly: InternalsVisibleTo("Microsoft.SqlTools.ServiceLayer.UnitTests")]
-[assembly: InternalsVisibleTo("Microsoft.SqlTools.ServiceLayer.IntegrationTests")]
-[assembly: InternalsVisibleTo("Microsoft.SqlTools.ManagedBatchParser.UnitTests")]
-[assembly: InternalsVisibleTo("Microsoft.SqlTools.ServiceLayer.Test.Common")]
+[assembly: InternalsVisibleTo("Microsoft.Kusto.ServiceLayer.UnitTests")]
 
 // Allowing internals visible access to Moq library to help testing
 [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]