From fdd9849d0169dc5633cc1b08f8ac988b6dfa3b95 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Sun, 2 Aug 2020 22:31:42 -0700 Subject: [PATCH 1/8] Split off InvokeAndClearCommands call --- src/DependencyManagement/PowerShellGalleryModuleProvider.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs index feb581d6..4e84c74f 100644 --- a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs +++ b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs @@ -87,8 +87,9 @@ public void SaveModule(PowerShell pwsh, string moduleName, string version, strin .AddParameter("AllowPrerelease", Utils.BoxedTrue) .AddParameter("Path", path) .AddParameter("Force", Utils.BoxedTrue) - .AddParameter("ErrorAction", "Stop") - .InvokeAndClearCommands(); + .AddParameter("ErrorAction", "Stop"); + + pwsh.InvokeAndClearCommands(); } /// From a8d98c9735bcceb66af4c785346c3198610cd7a1 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Sun, 2 Aug 2020 22:34:01 -0700 Subject: [PATCH 2/8] Extract _moduleProvider field --- .../PowerShellGalleryModuleProviderTests.cs | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/test/Unit/DependencyManagement/PowerShellGalleryModuleProviderTests.cs b/test/Unit/DependencyManagement/PowerShellGalleryModuleProviderTests.cs index d72a0406..814408e0 100644 --- a/test/Unit/DependencyManagement/PowerShellGalleryModuleProviderTests.cs +++ b/test/Unit/DependencyManagement/PowerShellGalleryModuleProviderTests.cs @@ -13,12 +13,18 @@ public class PowerShellGalleryModuleProviderTests private readonly Mock _mockSearchInvoker = new Mock(MockBehavior.Strict); + private PowerShellGalleryModuleProvider _moduleProvider; + + public PowerShellGalleryModuleProviderTests() + { + _moduleProvider = new PowerShellGalleryModuleProvider(_mockSearchInvoker.Object); + } + [Fact] public void ReturnsNullIfSearchInvokerReturnsNull() { _mockSearchInvoker.Setup(_ => _.Invoke(It.IsAny())).Returns(default(Stream)); - var moduleProvider = new PowerShellGalleryModuleProvider(_mockSearchInvoker.Object); - var actualVersion = moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); + var actualVersion = _moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); Assert.Null(actualVersion); } @@ -43,8 +49,7 @@ public void ReturnsSingleVersion() using (var responseStream = new MemoryStream(Encoding.UTF8.GetBytes(ResponseText))) { _mockSearchInvoker.Setup(_ => _.Invoke(It.IsAny())).Returns(responseStream); - var moduleProvider = new PowerShellGalleryModuleProvider(_mockSearchInvoker.Object); - var actualVersion = moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); + var actualVersion = _moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); Assert.Equal("1.2.3.4", actualVersion); } } @@ -82,8 +87,7 @@ public void FindsLatestVersionRegardlessOfResponseOrder() using (var responseStream = new MemoryStream(Encoding.UTF8.GetBytes(ResponseText))) { _mockSearchInvoker.Setup(_ => _.Invoke(It.IsAny())).Returns(responseStream); - var moduleProvider = new PowerShellGalleryModuleProvider(_mockSearchInvoker.Object); - var actualVersion = moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); + var actualVersion = _moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); Assert.Equal("1.2.3.6", actualVersion); } } @@ -121,8 +125,7 @@ public void IgnoresPrereleaseVersions() using (var responseStream = new MemoryStream(Encoding.UTF8.GetBytes(ResponseText))) { _mockSearchInvoker.Setup(_ => _.Invoke(It.IsAny())).Returns(responseStream); - var moduleProvider = new PowerShellGalleryModuleProvider(_mockSearchInvoker.Object); - var actualVersion = moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); + var actualVersion = _moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); Assert.Equal("1.2.3.5", actualVersion); } } @@ -166,8 +169,7 @@ public void IgnoresNotMatchingMajorVersions() using (var responseStream = new MemoryStream(Encoding.UTF8.GetBytes(ResponseText))) { _mockSearchInvoker.Setup(_ => _.Invoke(It.IsAny())).Returns(responseStream); - var moduleProvider = new PowerShellGalleryModuleProvider(_mockSearchInvoker.Object); - var actualVersion = moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); + var actualVersion = _moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); Assert.Equal("1.2.3.6", actualVersion); } } @@ -207,8 +209,7 @@ public void ComparesVersionsCorrectly(string lowerVersion, string higherVersion) using (var responseStream = new MemoryStream(Encoding.UTF8.GetBytes(responseText))) { _mockSearchInvoker.Setup(_ => _.Invoke(It.IsAny())).Returns(responseStream); - var moduleProvider = new PowerShellGalleryModuleProvider(_mockSearchInvoker.Object); - var actualVersion = moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "0"); + var actualVersion = _moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "0"); Assert.Equal(higherVersion, actualVersion); } } @@ -228,8 +229,7 @@ public void ReturnsNullIfNoVersionFound() using (var responseStream = new MemoryStream(Encoding.UTF8.GetBytes(ResponseText))) { _mockSearchInvoker.Setup(_ => _.Invoke(It.IsAny())).Returns(responseStream); - var moduleProvider = new PowerShellGalleryModuleProvider(_mockSearchInvoker.Object); - var actualVersion = moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); + var actualVersion = _moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); Assert.Null(actualVersion); } } @@ -315,8 +315,7 @@ public void FindsLatestVersionAcrossMultiplePages() _mockSearchInvoker.Setup(_ => _.Invoke(new Uri("https://NextLink2"))) .Returns(responseStream3); - var moduleProvider = new PowerShellGalleryModuleProvider(_mockSearchInvoker.Object); - var actualVersion = moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); + var actualVersion = _moduleProvider.GetLatestPublishedModuleVersion("ModuleName", "1"); Assert.Equal("1.2.3.6", actualVersion); } } From ceb61bd977ed0c610a890e58b7f7875750fa718e Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Sun, 2 Aug 2020 23:21:30 -0700 Subject: [PATCH 3/8] Pass ILogger to PowerShellGalleryModuleProvider --- src/DependencyManagement/DependencyManager.cs | 5 +++-- .../PowerShellGalleryModuleProvider.cs | 5 ++++- src/RequestProcessor.cs | 2 +- .../DependencyManagement/DependencyManagementTests.cs | 10 +++++----- .../PowerShellGalleryModuleProviderTests.cs | 5 ++++- 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/DependencyManagement/DependencyManager.cs b/src/DependencyManagement/DependencyManager.cs index 41075b82..fda9407b 100644 --- a/src/DependencyManagement/DependencyManager.cs +++ b/src/DependencyManagement/DependencyManager.cs @@ -48,12 +48,13 @@ public DependencyManager( IInstalledDependenciesLocator installedDependenciesLocator = null, IDependencySnapshotInstaller installer = null, INewerDependencySnapshotDetector newerSnapshotDetector = null, - IBackgroundDependencySnapshotMaintainer maintainer = null) + IBackgroundDependencySnapshotMaintainer maintainer = null, + ILogger logger = null) { _storage = storage ?? new DependencyManagerStorage(GetFunctionAppRootPath(requestMetadataDirectory)); _installedDependenciesLocator = installedDependenciesLocator ?? new InstalledDependenciesLocator(_storage); _installer = installer ?? new DependencySnapshotInstaller( - moduleProvider ?? new PowerShellGalleryModuleProvider(), + moduleProvider ?? new PowerShellGalleryModuleProvider(logger), _storage, new PowerShellModuleSnapshotComparer(), new PowerShellModuleSnapshotLogger()); diff --git a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs index 4e84c74f..0f5892f0 100644 --- a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs +++ b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs @@ -14,10 +14,13 @@ namespace Microsoft.Azure.Functions.PowerShellWorker.DependencyManagement internal class PowerShellGalleryModuleProvider : IModuleProvider { + private readonly ILogger _logger; + private readonly IPowerShellGallerySearchInvoker _searchInvoker; - public PowerShellGalleryModuleProvider(IPowerShellGallerySearchInvoker searchInvoker = null) + public PowerShellGalleryModuleProvider(ILogger logger, IPowerShellGallerySearchInvoker searchInvoker = null) { + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _searchInvoker = searchInvoker ?? new PowerShellGallerySearchInvoker(); } diff --git a/src/RequestProcessor.cs b/src/RequestProcessor.cs index 3f4b51b2..e0492626 100644 --- a/src/RequestProcessor.cs +++ b/src/RequestProcessor.cs @@ -190,7 +190,7 @@ internal StreamingMessage ProcessFunctionLoadRequest(StreamingMessage request) var rpcLogger = new RpcLogger(_msgStream); rpcLogger.SetContext(request.RequestId, null); - _dependencyManager = new DependencyManager(request.FunctionLoadRequest.Metadata.Directory); + _dependencyManager = new DependencyManager(request.FunctionLoadRequest.Metadata.Directory, logger: rpcLogger); var managedDependenciesPath = _dependencyManager.Initialize(request, rpcLogger); // Setup the FunctionApp root path and module path. diff --git a/test/Unit/DependencyManagement/DependencyManagementTests.cs b/test/Unit/DependencyManagement/DependencyManagementTests.cs index 80f1a22b..e176ed79 100644 --- a/test/Unit/DependencyManagement/DependencyManagementTests.cs +++ b/test/Unit/DependencyManagement/DependencyManagementTests.cs @@ -102,7 +102,7 @@ public void TestManagedDependencyBasicRequirements() var functionLoadRequest = GetFuncLoadRequest(functionFolderPath, true); // Create DependencyManager and process the requirements.psd1 file at the function app root. - using (var dependencyManager = new DependencyManager(functionLoadRequest.Metadata.Directory)) + using (var dependencyManager = new DependencyManager(functionLoadRequest.Metadata.Directory, logger: _testLogger)) { var currentDependenciesPath = dependencyManager.Initialize(_testLogger); @@ -132,7 +132,7 @@ public void TestManagedDependencyEmptyHashtableRequirement() var functionLoadRequest = GetFuncLoadRequest(functionFolderPath, true); // Create DependencyManager and process the requirements.psd1 file at the function app root. - using (var dependencyManager = new DependencyManager(functionLoadRequest.Metadata.Directory)) + using (var dependencyManager = new DependencyManager(functionLoadRequest.Metadata.Directory, logger: _testLogger)) { var currentDependenciesPath = dependencyManager.Initialize(_testLogger); @@ -153,7 +153,7 @@ public void TestManagedDependencyNoHashtableRequirementShouldThrow() var functionFolderPath = Path.Combine(_dependencyManagementDirectory, requirementsDirectoryName, "FunctionDirectory"); var functionLoadRequest = GetFuncLoadRequest(functionFolderPath, true); - using (var dependencyManager = new DependencyManager(functionLoadRequest.Metadata.Directory)) + using (var dependencyManager = new DependencyManager(functionLoadRequest.Metadata.Directory, logger: _testLogger)) { // Trying to set the functionApp dependencies should throw since requirements.psd1 is not a hash table. var exception = Assert.Throws( @@ -172,7 +172,7 @@ public void TestManagedDependencyInvalidRequirementsFormatShouldThrow() var functionFolderPath = Path.Combine(_dependencyManagementDirectory, requirementsDirectoryName, "FunctionDirectory"); var functionLoadRequest = GetFuncLoadRequest(functionFolderPath, true); - using (var dependencyManager = new DependencyManager(functionLoadRequest.Metadata.Directory)) + using (var dependencyManager = new DependencyManager(functionLoadRequest.Metadata.Directory, logger: _testLogger)) { // Trying to set the functionApp dependencies should throw since the module version // in requirements.psd1 is not in a valid format. @@ -193,7 +193,7 @@ public void TestManagedDependencyNoRequirementsFileShouldThrow() var functionFolderPath = Path.Combine(_dependencyManagementDirectory, requirementsDirectoryName, "FunctionDirectory"); var functionLoadRequest = GetFuncLoadRequest(functionFolderPath, true); - using (var dependencyManager = new DependencyManager(functionLoadRequest.Metadata.Directory)) + using (var dependencyManager = new DependencyManager(functionLoadRequest.Metadata.Directory, logger: _testLogger)) { // Trying to set the functionApp dependencies should throw since no // requirements.psd1 is found at the function app root. diff --git a/test/Unit/DependencyManagement/PowerShellGalleryModuleProviderTests.cs b/test/Unit/DependencyManagement/PowerShellGalleryModuleProviderTests.cs index 814408e0..a8633e51 100644 --- a/test/Unit/DependencyManagement/PowerShellGalleryModuleProviderTests.cs +++ b/test/Unit/DependencyManagement/PowerShellGalleryModuleProviderTests.cs @@ -7,9 +7,12 @@ namespace Microsoft.Azure.Functions.PowerShellWorker.Test.DependencyManagement using Xunit; using PowerShellWorker.DependencyManagement; + using PowerShellWorker.Utility; public class PowerShellGalleryModuleProviderTests { + private readonly Mock _mockLogger = new Mock(); + private readonly Mock _mockSearchInvoker = new Mock(MockBehavior.Strict); @@ -17,7 +20,7 @@ public class PowerShellGalleryModuleProviderTests public PowerShellGalleryModuleProviderTests() { - _moduleProvider = new PowerShellGalleryModuleProvider(_mockSearchInvoker.Object); + _moduleProvider = new PowerShellGalleryModuleProvider(_mockLogger.Object, _mockSearchInvoker.Object); } [Fact] From 75f8cfa0879828cb2ca486a977729687cf41d7aa Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Sun, 2 Aug 2020 23:25:03 -0700 Subject: [PATCH 4/8] Inline InvokeAndClearCommands call --- .../PowerShellGalleryModuleProvider.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs index 0f5892f0..f8ef0a34 100644 --- a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs +++ b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs @@ -92,7 +92,15 @@ public void SaveModule(PowerShell pwsh, string moduleName, string version, strin .AddParameter("Force", Utils.BoxedTrue) .AddParameter("ErrorAction", "Stop"); - pwsh.InvokeAndClearCommands(); + try + { + pwsh.Invoke(); + } + finally + { + pwsh.Streams.ClearStreams(); + pwsh.Commands.Clear(); + } } /// From 044e2a6ed65e1e3827cb0f31a01ec2573f6a5654 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Tue, 4 Aug 2020 11:34:28 -0700 Subject: [PATCH 5/8] Log errors and warnings from Save-Module --- .../PowerShellGalleryModuleProvider.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs index f8ef0a34..b44ea3c9 100644 --- a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs +++ b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs @@ -11,6 +11,7 @@ namespace Microsoft.Azure.Functions.PowerShellWorker.DependencyManagement using Microsoft.Azure.Functions.PowerShellWorker.PowerShell; using Microsoft.Azure.Functions.PowerShellWorker.Utility; + using LogLevel = Microsoft.Azure.WebJobs.Script.Grpc.Messages.RpcLog.Types.Level; internal class PowerShellGalleryModuleProvider : IModuleProvider { @@ -98,6 +99,16 @@ public void SaveModule(PowerShell pwsh, string moduleName, string version, strin } finally { + foreach (var item in pwsh.Streams.Error) + { + _logger.Log(isUserOnlyLog: false, LogLevel.Error, $"Save-Module: {item.Exception.Message}"); + } + + foreach (var item in pwsh.Streams.Warning) + { + _logger.Log(isUserOnlyLog: false, LogLevel.Warning, $"Save-Module: {item.Message}"); + } + pwsh.Streams.ClearStreams(); pwsh.Commands.Clear(); } From 2a851f84501d6b97fc014e95481b0f1de1c3c567 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Tue, 4 Aug 2020 11:44:03 -0700 Subject: [PATCH 6/8] Extract LogSaveModuleErrorsAndWarnings --- .../PowerShellGalleryModuleProvider.cs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs index b44ea3c9..1260b284 100644 --- a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs +++ b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs @@ -99,16 +99,7 @@ public void SaveModule(PowerShell pwsh, string moduleName, string version, strin } finally { - foreach (var item in pwsh.Streams.Error) - { - _logger.Log(isUserOnlyLog: false, LogLevel.Error, $"Save-Module: {item.Exception.Message}"); - } - - foreach (var item in pwsh.Streams.Warning) - { - _logger.Log(isUserOnlyLog: false, LogLevel.Warning, $"Save-Module: {item.Message}"); - } - + LogSaveModuleErrorsAndWarnings(pwsh); pwsh.Streams.ClearStreams(); pwsh.Commands.Clear(); } @@ -127,6 +118,19 @@ public void Cleanup(PowerShell pwsh) .InvokeAndClearCommands(); } + private void LogSaveModuleErrorsAndWarnings(PowerShell pwsh) + { + foreach (var item in pwsh.Streams.Error) + { + _logger.Log(isUserOnlyLog: false, LogLevel.Error, $"Save-Module: {item.Exception.Message}"); + } + + foreach (var item in pwsh.Streams.Warning) + { + _logger.Log(isUserOnlyLog: false, LogLevel.Warning, $"Save-Module: {item.Message}"); + } + } + private static Version GetLatestVersion( XmlNode root, XmlNamespaceManager namespaceManager, string expectedVersionStart, Version latestVersion) { From 20316122f362986814e59d981843bfe90ee47f79 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Tue, 4 Aug 2020 12:55:22 -0700 Subject: [PATCH 7/8] Log module name and version --- .../PowerShellGalleryModuleProvider.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs index 1260b284..611d5f73 100644 --- a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs +++ b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs @@ -99,7 +99,7 @@ public void SaveModule(PowerShell pwsh, string moduleName, string version, strin } finally { - LogSaveModuleErrorsAndWarnings(pwsh); +Lo LogSaveModuleErrorsAndWarnings(pwsh, moduleName, version); pwsh.Streams.ClearStreams(); pwsh.Commands.Clear(); } @@ -118,16 +118,18 @@ public void Cleanup(PowerShell pwsh) .InvokeAndClearCommands(); } - private void LogSaveModuleErrorsAndWarnings(PowerShell pwsh) + private void LogSaveModuleErrorsAndWarnings(PowerShell pwsh, string moduleName, string version) { + var prefix = $"Save-Module('{moduleName}', '{version}'): "; + foreach (var item in pwsh.Streams.Error) { - _logger.Log(isUserOnlyLog: false, LogLevel.Error, $"Save-Module: {item.Exception.Message}"); + _logger.Log(isUserOnlyLog: false, LogLevel.Error, $"{prefix}{item.Exception.Message}"); } foreach (var item in pwsh.Streams.Warning) { - _logger.Log(isUserOnlyLog: false, LogLevel.Warning, $"Save-Module: {item.Message}"); + _logger.Log(isUserOnlyLog: false, LogLevel.Warning, $"{prefix}{item.Message}"); } } From ea40fab64c3aeae9d7d4304ac764642bc90b0825 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Tue, 4 Aug 2020 13:26:37 -0700 Subject: [PATCH 8/8] Fix a typo --- src/DependencyManagement/PowerShellGalleryModuleProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs index 611d5f73..1e442067 100644 --- a/src/DependencyManagement/PowerShellGalleryModuleProvider.cs +++ b/src/DependencyManagement/PowerShellGalleryModuleProvider.cs @@ -99,7 +99,7 @@ public void SaveModule(PowerShell pwsh, string moduleName, string version, strin } finally { -Lo LogSaveModuleErrorsAndWarnings(pwsh, moduleName, version); + LogSaveModuleErrorsAndWarnings(pwsh, moduleName, version); pwsh.Streams.ClearStreams(); pwsh.Commands.Clear(); }