Fix file browser cancel/connetion issues (#528)

* clean bindingqueue after cancel

* fileborwser fixes

* code cleanup

* re-initialize filetree whenever filter is changed

* fix test issues

* address pr comments
This commit is contained in:
Kate Shin
2017-10-26 10:54:22 -07:00
committed by Karl Burtram
parent 4d04bff810
commit cc9beed835
8 changed files with 195 additions and 125 deletions

View File

@@ -42,4 +42,14 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser.Contracts
RequestType<FileBrowserCloseParams, FileBrowserCloseResponse> Type =
RequestType<FileBrowserCloseParams, FileBrowserCloseResponse>.Create("filebrowser/close");
}
/// <summary>
/// Notification for close completion
/// </summary>
public class FileBrowserClosedNotification
{
public static readonly
EventType<FileBrowserCloseResponse> Type =
EventType<FileBrowserCloseResponse>.Create("filebrowser/closecomplete");
}
}

View File

@@ -30,7 +30,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
public abstract class FileBrowserBase
{
private Enumerator enumerator = null;
protected SqlConnection sqlConnection = null;
protected ServerConnection connection = null;
protected Enumerator Enumerator
{
@@ -49,7 +49,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
/// Returns the PathSeparator values of the Server.
/// </summary>
/// <returns>PathSeparator</returns>
internal static char GetPathSeparator(Enumerator enumerator, SqlConnection connection)
internal static char GetPathSeparator(Enumerator enumerator, ServerConnection connection)
{
var req = new Request();
req.Urn = "Server";
@@ -78,7 +78,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
/// <param name="enumerator"></param>
/// <param name="connection"></param>
/// <returns></returns>
internal static IEnumerable<FileInfo> EnumerateDrives(Enumerator enumerator, SqlConnection connection)
internal static IEnumerable<FileInfo> EnumerateDrives(Enumerator enumerator, ServerConnection connection)
{
// if not supplied, server name will be obtained from urn
Request req = new Request();
@@ -145,7 +145,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
/// <param name="connection"></param>
/// <param name="path"></param>
/// <returns></returns>
internal static IEnumerable<FileInfo> EnumerateFilesInFolder(Enumerator enumerator, SqlConnection connection, string path)
internal static IEnumerable<FileInfo> EnumerateFilesInFolder(Enumerator enumerator, ServerConnection connection, string path)
{
var request = new Request
{

View File

@@ -5,10 +5,10 @@
using System;
using System.Collections.Generic;
using System.Data.SqlClient;
using System.Globalization;
using System.Text.RegularExpressions;
using System.Threading;
using Microsoft.SqlServer.Management.Common;
using Microsoft.SqlTools.ServiceLayer.FileBrowser.Contracts;
namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
@@ -30,21 +30,13 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
/// <summary>
/// Initializes a new instance of the <see cref="FileBrowser"/> class.
/// </summary>
public FileBrowserOperation()
/// <param name="connection">The connection object</param>
/// <param name="fileFilters">The file extension filters</param>
public FileBrowserOperation(ServerConnection connection, string expandPath, string[] fileFilters = null)
{
this.fileTree = new FileTree();
this.cancelSource = new CancellationTokenSource();
this.cancelToken = cancelSource.Token;
}
/// <summary>
/// Initializes a new instance of the <see cref="FileBrowser"/> class.
/// </summary>
/// <param name="connectionInfo">The connection info</param>
/// <param name="fileFilters">The file extension filters</param>
public FileBrowserOperation(SqlConnection connectionInfo, string expandPath, string[] fileFilters = null): this()
{
this.sqlConnection = connectionInfo;
this.connection = connection;
this.Initialize(expandPath, fileFilters);
}
@@ -76,14 +68,6 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
}
}
public SqlConnection SqlConnection
{
get
{
return this.sqlConnection;
}
}
public bool IsCancellationRequested
{
get
@@ -100,6 +84,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
public void Initialize(string expandPath, string[] fileFilters)
{
this.fileTree = new FileTree();
this.expandPath = expandPath;
if (fileFilters == null)
{
@@ -113,9 +98,9 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
public void Dispose()
{
if (this.sqlConnection != null)
if (this.connection != null)
{
this.sqlConnection.Close();
this.connection.Disconnect();
}
this.cancelSource.Dispose();
}
@@ -123,7 +108,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
public void PopulateFileTree()
{
this.fileTreeCreated = false;
this.PathSeparator = GetPathSeparator(this.Enumerator, this.sqlConnection);
this.PathSeparator = GetPathSeparator(this.Enumerator, this.connection);
PopulateDrives();
ExpandSelectedNode(this.expandPath);
this.fileTreeCreated = true;
@@ -191,7 +176,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
bool first = true;
if (!cancelToken.IsCancellationRequested)
{
foreach (var fileInfo in EnumerateDrives(Enumerator, sqlConnection))
foreach (var fileInfo in EnumerateDrives(Enumerator, connection))
{
if (cancelToken.IsCancellationRequested)
{
@@ -221,7 +206,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
public List<FileTreeNode> GetChildren(string filePath)
{
List<FileTreeNode> children = new List<FileTreeNode>();
foreach (var file in EnumerateFilesInFolder(Enumerator, sqlConnection, filePath))
foreach (var file in EnumerateFilesInFolder(Enumerator, connection, filePath))
{
bool isFile = !string.IsNullOrEmpty(file.fileName);
FileTreeNode treeNode = new FileTreeNode();

View File

@@ -13,6 +13,7 @@ using Microsoft.SqlTools.ServiceLayer.FileBrowser.Contracts;
using Microsoft.SqlTools.ServiceLayer.Hosting;
using Microsoft.SqlTools.ServiceLayer.LanguageServices;
using Microsoft.SqlTools.ServiceLayer.Utility;
using Microsoft.SqlTools.Utility;
namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
{
@@ -28,8 +29,8 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
private readonly ConcurrentDictionary<string, FileBrowserOperation> ownerToFileBrowserMap = new ConcurrentDictionary<string, FileBrowserOperation>();
private readonly ConcurrentDictionary<string, ValidatePathsCallback> validatePathsCallbackMap = new ConcurrentDictionary<string, ValidatePathsCallback>();
private ConnectionService connectionService;
private ConnectedBindingQueue expandNodeQueue = new ConnectedBindingQueue(needsMetadata: false);
private static int DefaultExpandTimeout = 120000;
private ConnectedBindingQueue fileBrowserQueue = new ConnectedBindingQueue(needsMetadata: false);
private static int DefaultTimeout = 120000;
private string serviceName = "FileBrowser";
/// <summary>
@@ -45,7 +46,7 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
if (connectionService == null)
{
connectionService = ConnectionService.Instance;
connectionService.RegisterConnectedQueue(this.serviceName, this.expandNodeQueue);
connectionService.RegisterConnectedQueue(this.serviceName, this.fileBrowserQueue);
}
return connectionService;
}
@@ -94,8 +95,9 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
.ContinueWithOnFaulted(null);
await requestContext.SendResult(true);
}
catch
catch (Exception ex)
{
Logger.Write(LogLevel.Error, "Unexpected exception while handling file browser open request: " + ex.Message);
await requestContext.SendResult(false);
}
}
@@ -108,8 +110,9 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
.ContinueWithOnFaulted(null);
await requestContext.SendResult(true);
}
catch
catch (Exception ex)
{
Logger.Write(LogLevel.Error, "Unexpected exception while handling file browser expand request: " + ex.Message);
await requestContext.SendResult(false);
}
}
@@ -122,8 +125,9 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
.ContinueWithOnFaulted(null);
await requestContext.SendResult(true);
}
catch
catch (Exception ex)
{
Logger.Write(LogLevel.Error, "Unexpected exception while handling file browser validate request: " + ex.Message);
await requestContext.SendResult(false);
}
}
@@ -132,90 +136,139 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
FileBrowserCloseParams fileBrowserParams,
RequestContext<FileBrowserCloseResponse> requestContext)
{
FileBrowserCloseResponse response = new FileBrowserCloseResponse();
FileBrowserOperation removedOperation;
response.Succeeded = ownerToFileBrowserMap.TryRemove(fileBrowserParams.OwnerUri, out removedOperation);
if (removedOperation != null && this.expandNodeQueue != null)
try
{
bool hasPendingQueueItems = this.expandNodeQueue.HasPendingQueueItems;
if (removedOperation.FileTreeCreated && !hasPendingQueueItems)
{
removedOperation.Dispose();
this.expandNodeQueue.CloseConnections(removedOperation.SqlConnection.DataSource, removedOperation.SqlConnection.Database, DefaultExpandTimeout);
}
else if (!removedOperation.FileTreeCreated)
{
removedOperation.Cancel();
}
else if (hasPendingQueueItems)
{
this.expandNodeQueue.StopQueueProcessor(DefaultExpandTimeout);
}
var task = Task.Run(() => RunFileBrowserCloseTask(fileBrowserParams, requestContext))
.ContinueWithOnFaulted(null);
await requestContext.SendResult(new FileBrowserCloseResponse() { Succeeded = true });
}
catch (Exception ex)
{
Logger.Write(LogLevel.Error, "Unexpected exception while handling file browser close request: " + ex.Message);
await requestContext.SendResult(new FileBrowserCloseResponse() { Message = ex.Message });
}
await requestContext.SendResult(response);
}
#endregion
public void Dispose()
{
this.expandNodeQueue.Dispose();
this.fileBrowserQueue.Dispose();
}
internal async Task RunFileBrowserCloseTask(FileBrowserCloseParams fileBrowserParams, RequestContext<FileBrowserCloseResponse> requestContext)
{
FileBrowserCloseResponse result = new FileBrowserCloseResponse();
try
{
FileBrowserOperation operation = null;
ConnectionInfo connInfo;
ownerToFileBrowserMap.TryGetValue(fileBrowserParams.OwnerUri, out operation);
this.ConnectionServiceInstance.TryFindConnection(fileBrowserParams.OwnerUri, out connInfo);
if (operation != null && connInfo != null)
{
if (!operation.FileTreeCreated)
{
operation.Cancel();
}
// Clear queued items
this.fileBrowserQueue.ClearQueuedItems();
// Queue operation to clean up resources
QueueItem queueItem = fileBrowserQueue.QueueBindingOperation(
key: fileBrowserQueue.AddConnectionContext(connInfo, this.serviceName),
bindingTimeout: DefaultTimeout,
waitForLockTimeout: DefaultTimeout,
bindOperation: (bindingContext, cancelToken) =>
{
FileBrowserOperation removedOperation = null;
ownerToFileBrowserMap.TryRemove(fileBrowserParams.OwnerUri, out removedOperation);
if (removedOperation != null)
{
removedOperation.Dispose();
}
result.Succeeded = true;
return result;
});
queueItem.ItemProcessed.WaitOne();
if (queueItem.GetResultAsT<FileBrowserCloseResponse>() != null)
{
result = queueItem.GetResultAsT<FileBrowserCloseResponse>();
}
this.fileBrowserQueue.CloseConnections(connInfo.ConnectionDetails.ServerName, connInfo.ConnectionDetails.DatabaseName, DefaultTimeout);
}
}
catch (Exception ex)
{
Logger.Write(LogLevel.Error, "Unexpected exception while closing file browser: " + ex.Message);
result.Message = ex.Message;
}
await requestContext.SendEvent(FileBrowserClosedNotification.Type, result);
}
internal async Task RunFileBrowserOpenTask(FileBrowserOpenParams fileBrowserParams, RequestContext<bool> requestContext)
{
FileBrowserOpenedParams result = new FileBrowserOpenedParams();
SqlConnection conn = null;
FileBrowserOperation browser = null;
bool isCancelRequested = false;
if (this.expandNodeQueue.IsCancelRequested)
{
this.expandNodeQueue.StartQueueProcessor();
}
try
{
if (!fileBrowserParams.ChangeFilter)
ConnectionInfo connInfo;
this.ConnectionServiceInstance.TryFindConnection(fileBrowserParams.OwnerUri, out connInfo);
if (connInfo != null)
{
ConnectionInfo connInfo;
this.ConnectionServiceInstance.TryFindConnection(fileBrowserParams.OwnerUri, out connInfo);
if (connInfo != null)
{
// Open new connection for each Open request
conn = ConnectionService.OpenSqlConnection(connInfo, this.serviceName);
browser = new FileBrowserOperation(conn, fileBrowserParams.ExpandPath, fileBrowserParams.FileFilters);
}
}
else
{
ownerToFileBrowserMap.TryGetValue(fileBrowserParams.OwnerUri, out browser);
if (browser != null)
{
browser.Initialize(fileBrowserParams.ExpandPath, fileBrowserParams.FileFilters);
}
}
QueueItem queueItem = fileBrowserQueue.QueueBindingOperation(
key: fileBrowserQueue.AddConnectionContext(connInfo, this.serviceName),
bindingTimeout: DefaultTimeout,
waitForLockTimeout: DefaultTimeout,
bindOperation: (bindingContext, cancelToken) =>
{
if (!fileBrowserParams.ChangeFilter)
{
browser = new FileBrowserOperation(bindingContext.ServerConnection, fileBrowserParams.ExpandPath, fileBrowserParams.FileFilters);
}
else
{
ownerToFileBrowserMap.TryGetValue(fileBrowserParams.OwnerUri, out browser);
if (browser != null)
{
browser.Initialize(fileBrowserParams.ExpandPath, fileBrowserParams.FileFilters);
}
}
if (browser != null)
{
ownerToFileBrowserMap.AddOrUpdate(fileBrowserParams.OwnerUri, browser, (key, value) => browser);
if (browser != null)
{
ownerToFileBrowserMap.AddOrUpdate(fileBrowserParams.OwnerUri, browser, (key, value) => browser);
// Create file browser tree
browser.PopulateFileTree();
// Create file browser tree
browser.PopulateFileTree();
// Check if cancel was requested
if (browser.IsCancellationRequested)
if (browser.IsCancellationRequested)
{
isCancelRequested = true;
}
else
{
result.OwnerUri = fileBrowserParams.OwnerUri;
result.FileTree = browser.FileTree;
result.Succeeded = true;
}
}
return result;
});
queueItem.ItemProcessed.WaitOne();
if (queueItem.GetResultAsT<FileBrowserOpenedParams>() != null)
{
browser.Dispose();
isCancelRequested = true;
}
else
{
result.OwnerUri = fileBrowserParams.OwnerUri;
result.FileTree = browser.FileTree;
result.Succeeded = true;
result = queueItem.GetResultAsT<FileBrowserOpenedParams>();
}
}
}
@@ -235,32 +288,29 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
FileBrowserExpandedParams result = new FileBrowserExpandedParams();
try
{
FileBrowserOperation operation;
FileBrowserOperation operation = null;
ConnectionInfo connInfo;
result.Succeeded = ownerToFileBrowserMap.TryGetValue(fileBrowserParams.OwnerUri, out operation);
ownerToFileBrowserMap.TryGetValue(fileBrowserParams.OwnerUri, out operation);
this.ConnectionServiceInstance.TryFindConnection(fileBrowserParams.OwnerUri, out connInfo);
if (result.Succeeded && operation != null && connInfo != null)
if (operation != null && connInfo != null)
{
QueueItem queueItem = expandNodeQueue.QueueBindingOperation(
key: expandNodeQueue.AddConnectionContext(connInfo, this.serviceName),
bindingTimeout: DefaultExpandTimeout,
waitForLockTimeout: DefaultExpandTimeout,
QueueItem queueItem = fileBrowserQueue.QueueBindingOperation(
key: fileBrowserQueue.AddConnectionContext(connInfo, this.serviceName),
bindingTimeout: DefaultTimeout,
waitForLockTimeout: DefaultTimeout,
bindOperation: (bindingContext, cancelToken) =>
{
result.ExpandPath = fileBrowserParams.ExpandPath;
result.Children = operation.GetChildren(fileBrowserParams.ExpandPath).ToArray();
result.OwnerUri = fileBrowserParams.OwnerUri;
result.Succeeded = true;
return result;
});
queueItem.ItemProcessed.WaitOne();
if (this.expandNodeQueue.IsCancelRequested)
{
this.expandNodeQueue.CloseConnections(operation.SqlConnection.DataSource, operation.SqlConnection.Database, DefaultExpandTimeout);
}
else if (queueItem.GetResultAsT<FileBrowserExpandedParams>() != null)
if (queueItem.GetResultAsT<FileBrowserExpandedParams>() != null)
{
result = queueItem.GetResultAsT<FileBrowserExpandedParams>();
}
@@ -268,7 +318,6 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
}
catch (Exception ex)
{
result.Succeeded = false;
result.Message = ex.Message;
}
@@ -282,22 +331,36 @@ namespace Microsoft.SqlTools.ServiceLayer.FileBrowser
try
{
ValidatePathsCallback callback;
ConnectionInfo connInfo;
this.ConnectionServiceInstance.TryFindConnection(fileBrowserParams.OwnerUri, out connInfo);
if (validatePathsCallbackMap.TryGetValue(fileBrowserParams.ServiceType, out callback)
&& callback != null
&& connInfo != null
&& fileBrowserParams.SelectedFiles != null
&& fileBrowserParams.SelectedFiles.Length > 0)
{
string errorMessage;
result.Succeeded = callback(new FileBrowserValidateEventArgs
{
ServiceType = fileBrowserParams.ServiceType,
OwnerUri = fileBrowserParams.OwnerUri,
FilePaths = fileBrowserParams.SelectedFiles
}, out errorMessage);
QueueItem queueItem = fileBrowserQueue.QueueBindingOperation(
key: fileBrowserQueue.AddConnectionContext(connInfo, this.serviceName),
bindingTimeout: DefaultTimeout,
waitForLockTimeout: DefaultTimeout,
bindOperation: (bindingContext, cancelToken) =>
{
string errorMessage;
result.Succeeded = callback(new FileBrowserValidateEventArgs
{
ServiceType = fileBrowserParams.ServiceType,
OwnerUri = fileBrowserParams.OwnerUri,
FilePaths = fileBrowserParams.SelectedFiles
}, out errorMessage);
result.Message = errorMessage;
return result;
});
if (!string.IsNullOrEmpty(errorMessage))
queueItem.ItemProcessed.WaitOne();
if (queueItem.GetResultAsT<FileBrowserValidatedParams>() != null)
{
result.Message = errorMessage;
result = queueItem.GetResultAsT<FileBrowserValidatedParams>();
}
}
else

View File

@@ -388,6 +388,20 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
}
}
/// <summary>
/// Clear queued items
/// </summary>
public void ClearQueuedItems()
{
lock (this.bindingQueueLock)
{
if (this.bindingQueue.Count > 0)
{
this.bindingQueue.Clear();
}
}
}
public void Dispose()
{
if (this.processQueueCancelToken != null)

View File

@@ -76,6 +76,6 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
/// <summary>
/// Gets or sets the database compatibility level
/// </summary>
DatabaseCompatibilityLevel DatabaseCompatibilityLevel { get; }
DatabaseCompatibilityLevel DatabaseCompatibilityLevel { get; }
}
}

View File

@@ -91,9 +91,7 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.FileBrowser
};
await service.HandleFileBrowserCloseRequest(inputParams, requestContext.Object);
// Result should return false since it's trying to close a filebrowser that was never opened
requestContext.Verify(x => x.SendResult(It.Is<FileBrowserCloseResponse>(p => p.Succeeded == false)));
requestContext.Verify(x => x.SendResult(It.Is<FileBrowserCloseResponse>(p => p.Succeeded == true)));
}
#endregion

View File

@@ -28,7 +28,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.FileBrowser
[Fact]
public void FilterFilesTest()
{
FileBrowserOperation operation = new FileBrowserOperation();
FileBrowserOperation operation = new FileBrowserOperation(null, "", null);
string[] supportedFilePaths = new string[] {"te\\s/t1.txt", "te!s.t2.bak" };
string[] unsupportedFilePaths = new string[] { "te.s*/t3.jpg", "t_est4.trn" };
string[] filters = new string[] { "*.txt", "*.bak"};
@@ -47,7 +47,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.FileBrowser
[Fact]
public void ExpandNodeShouldThrowExceptionForInvalidPath()
{
FileBrowserOperation operation = new FileBrowserOperation();
FileBrowserOperation operation = new FileBrowserOperation(null, "", null);
Exception exception = null;
try