Add delay to File System watchers to prevent Calibre installation issue (#6821)

* add a separate task to dequeue and create an app on installation

* Added tests to validate the behavior of the event handler

* release unmanaged memory
This commit is contained in:
Alekhya
2020-09-25 10:36:38 -07:00
committed by GitHub
parent 9e1711cbb9
commit f61db8ed3f
5 changed files with 186 additions and 47 deletions

View File

@@ -0,0 +1,106 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
using System.Collections.Concurrent;
using Microsoft.Plugin.Program.Storage;
using NUnit.Framework;
namespace Microsoft.Plugin.Program.UnitTests.Storage
{
[TestFixture]
public class ConcurrentQueueEventHandlerTest
{
[TestCase]
public void EventHandlerMustReturnEmptyPathForEmptyQueue()
{
// Arrange
int dequeueDelay = 0;
ConcurrentQueue<string> eventHandlingQueue = new ConcurrentQueue<string>();
// Act
string appPath = EventHandler.GetAppPathFromQueue(eventHandlingQueue, dequeueDelay);
// Assert
Assert.IsEmpty(appPath);
}
[TestCase(1)]
[TestCase(10)]
public void EventHandlerMustReturnPathForConcurrentQueueWithSameFilePaths(int itemCount)
{
// Arrange
int dequeueDelay = 0;
string appPath = "appPath";
ConcurrentQueue<string> eventHandlingQueue = new ConcurrentQueue<string>();
for (int i = 0; i < itemCount; i++)
{
eventHandlingQueue.Enqueue(appPath);
}
// Act
string pathFromQueue = EventHandler.GetAppPathFromQueue(eventHandlingQueue, dequeueDelay);
// Assert
Assert.AreEqual(pathFromQueue, appPath);
Assert.AreEqual(eventHandlingQueue.Count, 0);
}
[TestCase(5)]
public void EventHandlerMustReturnPathAndRetainDifferentFilePathsInQueue(int itemCount)
{
// Arrange
int dequeueDelay = 0;
string firstAppPath = "appPath1";
string secondAppPath = "appPath2";
ConcurrentQueue<string> eventHandlingQueue = new ConcurrentQueue<string>();
for (int i = 0; i < itemCount; i++)
{
eventHandlingQueue.Enqueue(firstAppPath);
}
for (int i = 0; i < itemCount; i++)
{
eventHandlingQueue.Enqueue(secondAppPath);
}
// Act
string pathFromQueue = EventHandler.GetAppPathFromQueue(eventHandlingQueue, dequeueDelay);
// Assert
Assert.AreEqual(pathFromQueue, firstAppPath);
Assert.AreEqual(eventHandlingQueue.Count, itemCount);
}
[TestCase(5)]
public void EventHandlerMustReturnPathAndRetainAllPathsAfterEncounteringADifferentPath(int itemCount)
{
// Arrange
int dequeueDelay = 0;
string firstAppPath = "appPath1";
string secondAppPath = "appPath2";
ConcurrentQueue<string> eventHandlingQueue = new ConcurrentQueue<string>();
for (int i = 0; i < itemCount; i++)
{
eventHandlingQueue.Enqueue(firstAppPath);
}
for (int i = 0; i < itemCount; i++)
{
eventHandlingQueue.Enqueue(secondAppPath);
}
for (int i = 0; i < itemCount; i++)
{
eventHandlingQueue.Enqueue(firstAppPath);
}
// Act
string pathFromQueue = EventHandler.GetAppPathFromQueue(eventHandlingQueue, dequeueDelay);
// Assert
Assert.AreEqual(pathFromQueue, firstAppPath);
Assert.AreEqual(eventHandlingQueue.Count, itemCount * 2);
}
}
}

View File

@@ -199,26 +199,6 @@ namespace Microsoft.Plugin.Program.UnitTests.Storage
Assert.IsFalse(win32ProgramRepository.Contains(olditem));
}
[TestCase("path.url")]
public void Win32ProgramRepositoryMustCallOnAppChangedForUrlAppsWhenChangedEventIsRaised(string path)
{
// Arrange
Win32ProgramRepository win32ProgramRepository = new Win32ProgramRepository(_fileSystemWatchers, new BinaryStorage<IList<Win32Program>>("Win32"), _settings, _pathsToWatch);
FileSystemEventArgs e = new FileSystemEventArgs(WatcherChangeTypes.Changed, "directory", path);
// File.ReadAllLines must be mocked for url applications
var mockFile = new Mock<IFileWrapper>();
mockFile.Setup(m => m.ReadAllLines(It.IsAny<string>())).Returns(new string[] { "URL=steam://rungameid/1258080", "IconFile=iconFile" });
Win32Program.FileWrapper = mockFile.Object;
// Act
_fileSystemMocks[0].Raise(m => m.Changed += null, e);
// Assert
Assert.AreEqual(win32ProgramRepository.Count(), 1);
Assert.AreEqual(win32ProgramRepository.ElementAt(0).AppType, Win32Program.ApplicationType.InternetShortcutApplication); // Internet Shortcut Application
}
[TestCase("path.url")]
public void Win32ProgramRepositoryMustNotCreateUrlAppWhenCreatedEventIsRaised(string path)
{
@@ -319,26 +299,6 @@ namespace Microsoft.Plugin.Program.UnitTests.Storage
Assert.IsFalse(win32ProgramRepository.Contains(olditem));
}
[TestCase("path.lnk")]
public void Win32ProgramRepositoryMustCallOnAppCreatedForLnkAppsWhenCreatedEventIsRaised(string path)
{
// Arrange
Win32ProgramRepository win32ProgramRepository = new Win32ProgramRepository(_fileSystemWatchers, new BinaryStorage<IList<Win32Program>>("Win32"), _settings, _pathsToWatch);
FileSystemEventArgs e = new FileSystemEventArgs(WatcherChangeTypes.Created, "directory", path);
// ShellLinkHelper must be mocked for lnk applications
var mockShellLink = new Mock<IShellLinkHelper>();
mockShellLink.Setup(m => m.RetrieveTargetPath(It.IsAny<string>())).Returns(string.Empty);
Win32Program.Helper = mockShellLink.Object;
// Act
_fileSystemMocks[0].Raise(m => m.Created += null, e);
// Assert
Assert.AreEqual(win32ProgramRepository.Count(), 1);
Assert.AreEqual(win32ProgramRepository.ElementAt(0).AppType, Win32Program.ApplicationType.Win32Application);
}
[TestCase("directory", "path.lnk")]
public void Win32ProgramRepositoryMustCallOnAppDeletedForLnkAppsWhenDeletedEventIsRaised(string directory, string path)
{

View File

@@ -174,6 +174,9 @@ namespace Microsoft.Plugin.Program.Programs
}
}
// To release unmanaged memory
Marshal.ReleaseComObject(link);
return target;
}
}

View File

@@ -0,0 +1,45 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
using System;
using System.Collections.Concurrent;
using System.Threading;
namespace Microsoft.Plugin.Program.Storage
{
public static class EventHandler
{
// To obtain the path of the app when multiple events are added to the Concurrent queue across multiple threads.
// On the first occurence of a different file path, the existing app path is to be returned without removing any more elements from the queue.
public static string GetAppPathFromQueue(ConcurrentQueue<string> eventHandlingQueue, int dequeueDelay)
{
if (eventHandlingQueue == null)
{
throw new ArgumentNullException(nameof(eventHandlingQueue));
}
string previousAppPath = string.Empty;
// To obtain the last event associated with a particular app.
while (eventHandlingQueue.TryPeek(out string currentAppPath))
{
if (string.IsNullOrEmpty(previousAppPath) || previousAppPath.Equals(currentAppPath, StringComparison.OrdinalIgnoreCase))
{
// To dequeue a path only if it is the first one in the queue or if the path was the same as thre previous one (to avoid trying to create apps on duplicate events)
previousAppPath = currentAppPath;
eventHandlingQueue.TryDequeue(out _);
}
else
{
break;
}
// This delay has been added to account for the delay in events being triggered during app installation.
Thread.Sleep(dequeueDelay);
}
return previousAppPath;
}
}
}

View File

@@ -3,10 +3,13 @@
// See the LICENSE file in the project root for more information.
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Globalization;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Wox.Infrastructure.Logger;
using Wox.Infrastructure.Storage;
using Win32Program = Microsoft.Plugin.Program.Programs.Win32Program;
@@ -25,6 +28,8 @@ namespace Microsoft.Plugin.Program.Storage
private int _numberOfPathsToWatch;
private Collection<string> extensionsToWatch = new Collection<string> { "*.exe", $"*{LnkExtension}", "*.appref-ms", $"*{UrlExtension}" };
private static ConcurrentQueue<string> commonEventHandlingQueue = new ConcurrentQueue<string>();
public Win32ProgramRepository(IList<IFileSystemWatcherWrapper> fileSystemWatcherHelpers, IStorage<IList<Win32Program>> storage, ProgramPluginSettings settings, string[] pathsToWatch)
{
_fileSystemWatcherHelpers = fileSystemWatcherHelpers;
@@ -33,6 +38,28 @@ namespace Microsoft.Plugin.Program.Storage
_pathsToWatch = pathsToWatch;
_numberOfPathsToWatch = pathsToWatch.Length;
InitializeFileSystemWatchers();
// This task would always run in the background trying to dequeue file paths from the queue at regular intervals.
Task.Run(() =>
{
while (true)
{
int dequeueDelay = 500;
string appPath = EventHandler.GetAppPathFromQueue(commonEventHandlingQueue, dequeueDelay);
// To allow for the installation process to finish.
Thread.Sleep(5000);
if (!string.IsNullOrEmpty(appPath))
{
Programs.Win32Program app = Programs.Win32Program.GetAppFromPath(appPath);
if (app != null)
{
Add(app);
}
}
}
}).ConfigureAwait(false);
}
private void InitializeFileSystemWatchers()
@@ -175,7 +202,7 @@ namespace Microsoft.Plugin.Program.Storage
private void OnAppCreated(object sender, FileSystemEventArgs e)
{
string path = e.FullPath;
if (!Path.GetExtension(path).Equals(UrlExtension, StringComparison.CurrentCultureIgnoreCase))
if (!Path.GetExtension(path).Equals(UrlExtension, StringComparison.CurrentCultureIgnoreCase) && !Path.GetExtension(path).Equals(LnkExtension, StringComparison.CurrentCultureIgnoreCase))
{
Programs.Win32Program app = Programs.Win32Program.GetAppFromPath(path);
if (app != null)
@@ -188,13 +215,11 @@ namespace Microsoft.Plugin.Program.Storage
private void OnAppChanged(object sender, FileSystemEventArgs e)
{
string path = e.FullPath;
if (Path.GetExtension(path).Equals(UrlExtension, StringComparison.CurrentCultureIgnoreCase))
if (Path.GetExtension(path).Equals(UrlExtension, StringComparison.CurrentCultureIgnoreCase) || Path.GetExtension(path).Equals(LnkExtension, StringComparison.CurrentCultureIgnoreCase))
{
Programs.Win32Program app = Programs.Win32Program.GetAppFromPath(path);
if (app != null)
{
Add(app);
}
// When a url or lnk app is installed, multiple created and changed events are triggered.
// To prevent the code from acting on the first such event (which may still be during app installation), the events are added a common queue and dequeued by a background task at regular intervals - https://github.com/microsoft/PowerToys/issues/6429.
commonEventHandlingQueue.Enqueue(path);
}
}