commit aec31b87bbacc37693a270f1e002f4ceac3ca73c parent 6bbe990d102a705d2040aa4468b339c5e7534c9e Author: JohnMcPMS <johnmcp@microsoft.com> Date: Tue, 25 Feb 2020 16:31:17 -0800 Move from future to progress (#43) Diffstat:
21 files changed, 304 insertions(+), 568 deletions(-)
diff --git a/src/AppInstallerCLICore/Workflows/InstallerHandlerBase.cpp b/src/AppInstallerCLICore/Workflows/InstallerHandlerBase.cpp @@ -1,11 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. - #include "pch.h" #include "Common.h" #include "InstallerHandlerBase.h" -using namespace AppInstaller::Utility; using namespace AppInstaller::Manifest; namespace AppInstaller::Workflow @@ -14,18 +12,15 @@ namespace AppInstaller::Workflow { // Todo: Rework the path logic. The new path logic should work with MOTW. std::filesystem::path tempInstallerPath = Runtime::GetPathToTemp(); - tempInstallerPath /= SHA256::ConvertToString(m_manifestInstallerRef.Sha256); + tempInstallerPath /= Utility::SHA256::ConvertToString(m_manifestInstallerRef.Sha256); AICLI_LOG(CLI, Info, << "Generated temp download path: " << tempInstallerPath); - auto future = DownloadAsync( + auto hash = m_reporterRef.ExecuteWithProgress(std::bind(Utility::Download, m_manifestInstallerRef.Url, tempInstallerPath, - true); - - future.SetProgressReceiver(&m_reporterRef); - - auto hash = future.Get(); + std::placeholders::_1, + true)); if (!hash) { @@ -40,9 +35,9 @@ namespace AppInstaller::Workflow { AICLI_LOG(CLI, Error, << "Package hash verification failed. SHA256 in manifest: " - << SHA256::ConvertToString(m_manifestInstallerRef.Sha256) + << Utility::SHA256::ConvertToString(m_manifestInstallerRef.Sha256) << " SHA256 from download: " - << SHA256::ConvertToString(hash.value())); + << Utility::SHA256::ConvertToString(hash.value())); if (!m_reporterRef.PromptForBoolResponse(WorkflowReporter::Level::Warning, "Package hash verification failed. Continue?")) { diff --git a/src/AppInstallerCLICore/Workflows/MsixInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/MsixInstallerHandler.cpp @@ -65,19 +65,16 @@ namespace AppInstaller::Workflow Uri target = m_useStreaming ? Uri(Utility::ConvertToUTF16(m_manifestInstallerRef.Url)) : Uri(m_downloadedInstaller.c_str()); - auto installTask = ExecuteInstallerAsync(target); - installTask.SetProgressReceiver(&m_reporterRef); - m_reporterRef.ShowMsg(WorkflowReporter::Level::Info, "Starting package install..."); - installTask.Get(); + ExecuteInstallerAsync(target); m_reporterRef.ShowMsg(WorkflowReporter::Level::Info, "Successfully installed."); } - Future<void> MsixInstallerHandler::ExecuteInstallerAsync(const winrt::Windows::Foundation::Uri& uri) + void MsixInstallerHandler::ExecuteInstallerAsync(const winrt::Windows::Foundation::Uri& uri) { DeploymentOptions deploymentOptions = DeploymentOptions::ForceApplicationShutdown | DeploymentOptions::ForceTargetApplicationShutdown; - return Deployment::RequestAddPackageAsync(uri, deploymentOptions); + m_reporterRef.ExecuteWithProgress(std::bind(Deployment::RequestAddPackageAsync, uri, deploymentOptions, std::placeholders::_1)); } } \ No newline at end of file diff --git a/src/AppInstallerCLICore/Workflows/MsixInstallerHandler.h b/src/AppInstallerCLICore/Workflows/MsixInstallerHandler.h @@ -2,7 +2,6 @@ // Licensed under the MIT License. #pragma once #include "InstallerHandlerBase.h" -#include <AppInstallerFuture.h> namespace AppInstaller::Workflow { @@ -27,6 +26,6 @@ namespace AppInstaller::Workflow // If use streaming install vs download install. bool m_useStreaming = true; - virtual Future<void> ExecuteInstallerAsync(const winrt::Windows::Foundation::Uri& uri); + virtual void ExecuteInstallerAsync(const winrt::Windows::Foundation::Uri& uri); }; } \ No newline at end of file diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -24,13 +24,16 @@ namespace AppInstaller::Workflow RenameDownloadedInstaller(); - Future<DWORD> installTask = ExecuteInstallerAsync(m_downloadedInstaller, installerArgs); - installTask.SetProgressReceiver(&m_reporterRef); - auto installResult = installTask.Get(); + auto installResult = m_reporterRef.ExecuteWithProgress( + std::bind(ExecuteInstaller, + m_downloadedInstaller, + installerArgs, + m_argsRef.Contains(CLI::ARG_INTERACTIVE), + std::placeholders::_1)); if (!installResult) { - m_reporterRef.ShowMsg(WorkflowReporter::Level::Error, "Installation cancelled"); + m_reporterRef.ShowMsg(WorkflowReporter::Level::Error, "Installation abandoned"); } else if (installResult.value() != 0) { @@ -45,49 +48,48 @@ namespace AppInstaller::Workflow } } - Future<DWORD> ShellExecuteInstallerHandler::ExecuteInstallerAsync(const std::filesystem::path& filePath, const std::string& args) + std::optional<DWORD> ShellExecuteInstallerHandler::ExecuteInstaller(const std::filesystem::path& filePath, const std::string& args, bool interactive, IProgressCallback& progress) { AICLI_LOG(CLI, Info, << "Staring installer. Path: " << filePath); - int showValue = m_argsRef.Contains(CLI::ARG_INTERACTIVE) ? SW_SHOW : SW_HIDE; - return Future<DWORD>([filePath, args, showValue] (IPromiseKeeperProgress* progress) - { - SHELLEXECUTEINFOA execInfo = { 0 }; - execInfo.cbSize = sizeof(SHELLEXECUTEINFO); - execInfo.fMask = SEE_MASK_NOCLOSEPROCESS; - std::string filePathUTF8Str = Utility::ConvertToUTF8(filePath.c_str()); - execInfo.lpFile = filePathUTF8Str.c_str(); - execInfo.lpParameters = args.c_str(); - execInfo.nShow = showValue; - if (!ShellExecuteExA(&execInfo) || !execInfo.hProcess) - { - return GetLastError(); - } + + SHELLEXECUTEINFOA execInfo = { 0 }; + execInfo.cbSize = sizeof(SHELLEXECUTEINFO); + execInfo.fMask = SEE_MASK_NOCLOSEPROCESS; + std::string filePathUTF8Str = Utility::ConvertToUTF8(filePath.c_str()); + execInfo.lpFile = filePathUTF8Str.c_str(); + execInfo.lpParameters = args.c_str(); + execInfo.nShow = interactive ? SW_SHOW : SW_HIDE; + if (!ShellExecuteExA(&execInfo) || !execInfo.hProcess) + { + return GetLastError(); + } - wil::unique_process_handle process{ execInfo.hProcess }; - - // Wait for installation to finish - while (!progress->IsCancelled()) - { - DWORD waitResult = WaitForSingleObject(process.get(), 250); - if (waitResult == WAIT_OBJECT_0) - { - break; - } - if (waitResult != WAIT_TIMEOUT) - { - THROW_LAST_ERROR_MSG("Unexpected WaitForSingleObjectResult: %d", waitResult); - } - } - - DWORD exitCode = 0; - - if (!progress->IsCancelled()) - { - GetExitCodeProcess(process.get(), &exitCode); - } - - return exitCode; - }); + wil::unique_process_handle process{ execInfo.hProcess }; + + // Wait for installation to finish + while (!progress.IsCancelled()) + { + DWORD waitResult = WaitForSingleObject(process.get(), 250); + if (waitResult == WAIT_OBJECT_0) + { + break; + } + if (waitResult != WAIT_TIMEOUT) + { + THROW_LAST_ERROR_MSG("Unexpected WaitForSingleObjectResult: %d", waitResult); + } + } + + if (progress.IsCancelled()) + { + return {}; + } + else + { + DWORD exitCode = 0; + GetExitCodeProcess(process.get(), &exitCode); + return exitCode; + } } std::string ShellExecuteInstallerHandler::GetInstallerArgsTemplate() diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.h b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.h @@ -2,7 +2,9 @@ // Licensed under the MIT License. #pragma once #include "InstallerHandlerBase.h" -#include <AppInstallerFuture.h> +#include <AppInstallerProgress.h> + +#include <optional> namespace AppInstaller::Workflow { @@ -21,11 +23,7 @@ namespace AppInstaller::Workflow void Install() override; protected: - Future<DWORD> ExecuteInstallerAsync(const std::filesystem::path& filePath, const std::string& args); - - // The known default arg format if the corresponding arg is not specified in the manifest - // i.e. If silent switch is not specified in manifest and installer type is msi, /quiet will be returned. - std::string GetDefaultArg(std::string_view argType); + static std::optional<DWORD> ExecuteInstaller(const std::filesystem::path& filePath, const std::string& args, bool interactive, IProgressCallback& progress); // Construct the installer arg string from appropriate source(known args, manifest) according to command line args. // Token is not replaced with actual values yet. diff --git a/src/AppInstallerCLICore/Workflows/WorkflowReporter.cpp b/src/AppInstallerCLICore/Workflows/WorkflowReporter.cpp @@ -28,6 +28,10 @@ namespace AppInstaller::Workflow { char spinnerChars[] = { '-', '\\', '|', '/' }; + // First wait for a small amount of time to enable a fast task to skip + // showing anything, or a progress task to skip straight to progress. + Sleep(100); + for (int i = 0; !m_canceled; i++) { out << '\b' << spinnerChars[i] << std::flush; @@ -36,7 +40,7 @@ namespace AppInstaller::Workflow i = -1; } - Sleep(300); + Sleep(250); } out << '\b'; @@ -105,23 +109,10 @@ namespace AppInstaller::Workflow } } - // TODO: Better handling of generic progress facility - void WorkflowReporter::OnStarted() - { - ShowIndefiniteProgress(true); - } - - void WorkflowReporter::OnProgress(uint64_t current, uint64_t maximum, FutureProgressType type) + void WorkflowReporter::OnProgress(uint64_t current, uint64_t maximum, ProgressType type) { UNREFERENCED_PARAMETER(type); ShowIndefiniteProgress(false); ShowProgress(true, (maximum ? static_cast<uint64_t>((static_cast<double>(current) / maximum) * 100) : current)); } - - void WorkflowReporter::OnCompleted(bool cancelled) - { - UNREFERENCED_PARAMETER(cancelled); - ShowIndefiniteProgress(false); - ShowProgress(false, 0); - } } diff --git a/src/AppInstallerCLICore/Workflows/WorkflowReporter.h b/src/AppInstallerCLICore/Workflows/WorkflowReporter.h @@ -1,7 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once -#include "AppInstallerFuture.h" +#include "AppInstallerProgress.h" + +#include <wil/resource.h> #include <atomic> #include <future> @@ -44,7 +46,7 @@ namespace AppInstaller::Workflow // WorkflowReporter should be the central place to show workflow status to user. // Todo: need to implement actual console output to show color, progress bar, etc - struct WorkflowReporter : public IFutureProgress + struct WorkflowReporter : public IProgressCallback { enum class Level { @@ -70,10 +72,24 @@ namespace AppInstaller::Workflow // running: shows indefinite progress if set to true, stops indefinite progress if set to false void ShowIndefiniteProgress(bool running); - // IFutureProgress - void OnStarted() override; - void OnProgress(uint64_t current, uint64_t maximum, FutureProgressType type) override; - void OnCompleted(bool cancelled) override; + // IProgressCallback + void OnProgress(uint64_t current, uint64_t maximum, ProgressType type) override; + bool IsCancelled() override { return false; } + + // Runs the given callable of type: auto(IProgressCallback&) + template <typename F> + auto ExecuteWithProgress(F&& f) + { + ProgressCallback callback(this); + ShowIndefiniteProgress(true); + + auto hideProgress = wil::scope_exit([this]() + { + ShowIndefiniteProgress(false); + ShowProgress(false, 0); + }); + return f(callback); + } private: std::ostream& out; diff --git a/src/AppInstallerCLICore/pch.h b/src/AppInstallerCLICore/pch.h @@ -13,11 +13,13 @@ #include <iostream> #include <fstream> +#include <future> +#include <functional> #include <memory> +#include <optional> #include <sstream> -#include <vector> -#include <future> #include <string_view> +#include <vector> #include <yaml-cpp\yaml.h> diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -152,7 +152,6 @@ </ItemGroup> <ItemGroup> <ClCompile Include="Downloader.cpp" /> - <ClCompile Include="Future.cpp" /> <ClCompile Include="WorkFlow.cpp" /> <ClCompile Include="LanguageUtilities.cpp" /> <ClCompile Include="main.cpp"> diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -65,9 +65,6 @@ <ClCompile Include="Synchronization.cpp"> <Filter>Source Files</Filter> </ClCompile> - <ClCompile Include="Future.cpp"> - <Filter>Source Files</Filter> - </ClCompile> </ItemGroup> <ItemGroup> <None Include="PropertySheet.props" /> diff --git a/src/AppInstallerCLITests/Downloader.cpp b/src/AppInstallerCLITests/Downloader.cpp @@ -5,6 +5,7 @@ #include "AppInstallerDownloader.h" #include "AppInstallerSHA256.h" +using namespace AppInstaller; using namespace AppInstaller::Utility; using namespace std::string_literals; @@ -14,9 +15,8 @@ TEST_CASE("DownloadValidFileAndVerifyHash", "[Downloader]") INFO("Using temporary file named: " << tempFile.GetPath()); // Todo: point to files from our repo when the repo goes public - auto future = DownloadAsync("https://raw.githubusercontent.com/microsoft/msix-packaging/master/LICENSE", tempFile.GetPath(), true); - - auto result = future.Get(); + ProgressCallback callback; + auto result = Download("https://raw.githubusercontent.com/microsoft/msix-packaging/master/LICENSE", tempFile.GetPath(), callback, true); REQUIRE(result.has_value()); auto resultHash = result.value(); @@ -35,12 +35,15 @@ TEST_CASE("DownloadValidFileAndCancel", "[Downloader]") TestCommon::TempFile tempFile("downloader_test"s, ".test"s); INFO("Using temporary file named: " << tempFile.GetPath()); - auto future = DownloadAsync("https://aka.ms/win32-x64-user-stable", tempFile.GetPath(), true); + ProgressCallback callback; std::optional<std::vector<BYTE>> waitResult; - std::thread waitThread([&future, &waitResult] { waitResult = future.Get(); }); + std::thread waitThread([&] + { + waitResult = Download("https://aka.ms/win32-x64-user-stable", tempFile.GetPath(), callback, true); + }); - future.Cancel(); + callback.Cancel(); waitThread.join(); @@ -52,7 +55,7 @@ TEST_CASE("DownloadUnreachableUrl", "[Downloader]") TestCommon::TempFile tempFile("downloader_test"s, ".test"s); INFO("Using temporary file named: " << tempFile.GetPath()); - auto future = DownloadAsync("https://does_not_exist.com/", tempFile.GetPath(), true); + ProgressCallback callback; - REQUIRE_THROWS_HR(future.Get(), WININET_E_NAME_NOT_RESOLVED); + REQUIRE_THROWS_HR(Download("https://does_not_exist.com/", tempFile.GetPath(), callback, true), WININET_E_NAME_NOT_RESOLVED); } diff --git a/src/AppInstallerCLITests/Future.cpp b/src/AppInstallerCLITests/Future.cpp @@ -1,140 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. -#include "pch.h" -#include "TestCommon.h" -#include <AppInstallerFuture.h> - -using namespace AppInstaller; - -struct FutureProgress : public IFutureProgress -{ - // IFutureProgress - void OnStarted() override - { - if (Started) - { - Started(); - } - } - - void OnProgress(uint64_t current, uint64_t maximum, FutureProgressType type) override - { - if (Progress) - { - Progress(current, maximum, type); - } - } - - void OnCompleted(bool cancelled) override - { - if (Completed) - { - Completed(cancelled); - } - } - - std::function<void()> Started; - std::function<void(uint64_t, uint64_t, FutureProgressType)> Progress; - std::function<void(bool)> Completed; -}; - -TEST_CASE("Future_Basic", "[Future]") -{ - int input = 42; - - Future<int> f{ std::packaged_task<int(IPromiseKeeperProgress*)>{ [input](IPromiseKeeperProgress*) { return input; } } }; - int result = f.Get().value(); - - REQUIRE(input == result); -} - -TEST_CASE("Future_Optional", "[Future]") -{ - int input = 42; - - Future<std::optional<int>> f{ std::packaged_task<std::optional<int>(IPromiseKeeperProgress*)>{ [input](IPromiseKeeperProgress*) { return input; } } }; - int result = f.Get().value(); - - REQUIRE(input == result); -} - -TEST_CASE("Future_Callbacks", "[Future]") -{ - int input = 42; - uint64_t progress = 110; - uint64_t maximum = 100; - FutureProgressType type = FutureProgressType::Percent; - - bool startedCalled = false; - uint64_t progressActual = 0; - uint64_t maximumActual = 0; - FutureProgressType typeActual = FutureProgressType::None; - bool cancelledActual = false; - - FutureProgress futureProgress; - futureProgress.Started = [&]() { startedCalled = true; }; - futureProgress.Progress = [&](uint64_t p, uint64_t m, FutureProgressType t) { progressActual = p; maximumActual = m; typeActual = t; }; - futureProgress.Completed = [&](bool c) { cancelledActual = c; }; - - Future<std::optional<int>> f{ std::packaged_task<std::optional<int>(IPromiseKeeperProgress*)>{ - [&](IPromiseKeeperProgress* p) { - p->OnProgress(progress, maximum, type); - return input; - } - } }; - f.SetProgressReceiver(&futureProgress); - int result = f.Get().value(); - - REQUIRE(input == result); - REQUIRE(startedCalled); - REQUIRE(progress == progressActual); - REQUIRE(maximum == maximumActual); - REQUIRE(type == typeActual); - REQUIRE(!cancelledActual); -} - -TEST_CASE("Future_Cancelled", "[Future]") -{ - int input = 42; - uint64_t progress = 110; - uint64_t maximum = 100; - FutureProgressType type = FutureProgressType::Percent; - - bool startedCalled = false; - uint64_t progressActual = 0; - uint64_t maximumActual = 0; - FutureProgressType typeActual = FutureProgressType::None; - bool cancelledActual = false; - - Future<std::optional<int>> f{ std::packaged_task<std::optional<int>(IPromiseKeeperProgress*)>{ - [&](IPromiseKeeperProgress* p) { - p->OnProgress(progress, maximum, type); - if (p->IsCancelled()) - { - return 0; - } - else - { - return input; - } - } - } }; - - FutureProgress futureProgress; - futureProgress.Started = [&]() { startedCalled = true; }; - futureProgress.Progress = [&](uint64_t p, uint64_t m, FutureProgressType t) { - progressActual = p; maximumActual = m; typeActual = t; - f.Cancel(); - }; - futureProgress.Completed = [&](bool c) { cancelledActual = c; }; - f.SetProgressReceiver(&futureProgress); - - auto result = f.Get(); - - REQUIRE(!result.has_value()); - REQUIRE(startedCalled); - REQUIRE(progress == progressActual); - REQUIRE(maximum == maximumActual); - REQUIRE(type == typeActual); - REQUIRE(cancelledActual); -} diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp @@ -32,7 +32,7 @@ public: protected: - AppInstaller::Future<void> ExecuteInstallerAsync(const Uri& uri) override + void ExecuteInstallerAsync(const Uri& uri) override { std::filesystem::path temp = std::filesystem::temp_directory_path(); temp /= "TestMsixInstalled.txt"; @@ -41,8 +41,6 @@ protected: file << AppInstaller::Utility::ConvertToUTF8(uri.ToString()); file.close(); - - return AppInstaller::Future<void>([](AppInstaller::IPromiseKeeperProgress*) {}); } }; diff --git a/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj b/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj @@ -177,7 +177,7 @@ <ClInclude Include="Public\AppInstallerDownloader.h" /> <ClInclude Include="Public\AppInstallerErrors.h" /> <ClInclude Include="Public\AppInstallerFileLogger.h" /> - <ClInclude Include="Public\AppInstallerFuture.h" /> + <ClInclude Include="Public\AppInstallerProgress.h" /> <ClInclude Include="Public\AppInstallerLanguageUtilities.h" /> <ClInclude Include="Public\AppInstallerMsixInfo.h" /> <ClInclude Include="Public\AppInstallerRuntime.h" /> diff --git a/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj.filters b/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj.filters @@ -84,10 +84,10 @@ <ClInclude Include="Public\AppInstallerSynchronization.h"> <Filter>Public</Filter> </ClInclude> - <ClInclude Include="Public\AppInstallerFuture.h"> + <ClInclude Include="Public\AppInstallerDeployment.h"> <Filter>Public</Filter> </ClInclude> - <ClInclude Include="Public\AppInstallerDeployment.h"> + <ClInclude Include="Public\AppInstallerProgress.h"> <Filter>Public</Filter> </ClInclude> </ItemGroup> diff --git a/src/AppInstallerCommonCore/Deployment.cpp b/src/AppInstallerCommonCore/Deployment.cpp @@ -17,51 +17,49 @@ namespace AppInstaller::Deployment } } - Future<void> RequestAddPackageAsync( + void RequestAddPackageAsync( const winrt::Windows::Foundation::Uri& uri, - winrt::Windows::Management::Deployment::DeploymentOptions options) + winrt::Windows::Management::Deployment::DeploymentOptions options, + IProgressCallback& callback) { - return Future<void>([uri, options](IPromiseKeeperProgress* pkp) - { - using namespace winrt::Windows::Foundation; - using namespace winrt::Windows::Management::Deployment; + using namespace winrt::Windows::Foundation; + using namespace winrt::Windows::Management::Deployment; - size_t id = GetDeploymentOperationId(); - AICLI_LOG(Core, Info, << "Starting RequestAddPackage operation #" << id << ": " << Utility::ConvertToUTF8(uri.AbsoluteUri().c_str())); + size_t id = GetDeploymentOperationId(); + AICLI_LOG(Core, Info, << "Starting RequestAddPackage operation #" << id << ": " << Utility::ConvertToUTF8(uri.AbsoluteUri().c_str())); - PackageManager packageManager; + PackageManager packageManager; - // RequestAddPackageAsync will invoke smart screen. - auto deployOperation = packageManager.RequestAddPackageAsync( - uri, - nullptr, /*dependencyPackageUris*/ - options, - nullptr, /*targetVolume*/ - nullptr, /*optionalAndRelatedPackageFamilyNames*/ - nullptr /*relatedPackageUris*/); + // RequestAddPackageAsync will invoke smart screen. + auto deployOperation = packageManager.RequestAddPackageAsync( + uri, + nullptr, /*dependencyPackageUris*/ + options, + nullptr, /*targetVolume*/ + nullptr, /*optionalAndRelatedPackageFamilyNames*/ + nullptr /*relatedPackageUris*/); - AsyncOperationProgressHandler<DeploymentResult, DeploymentProgress> progressCallback( - [pkp](const IAsyncOperationWithProgress<DeploymentResult, DeploymentProgress>&, DeploymentProgress progress) - { - pkp->OnProgress(progress.percentage, 100, FutureProgressType::Percent); - } - ); + AsyncOperationProgressHandler<DeploymentResult, DeploymentProgress> progressCallback( + [&callback](const IAsyncOperationWithProgress<DeploymentResult, DeploymentProgress>&, DeploymentProgress progress) + { + callback.OnProgress(progress.percentage, 100, ProgressType::Percent); + } + ); - // Set progress callback. - deployOperation.Progress(progressCallback); + // Set progress callback. + deployOperation.Progress(progressCallback); - auto deployResult = deployOperation.GetResults(); + auto deployResult = deployOperation.GetResults(); - if (!SUCCEEDED(deployResult.ExtendedErrorCode())) - { - AICLI_LOG(Core, Error, << "Deployment failed #" << id << ": " << Utility::ConvertToUTF8(deployResult.ErrorText())); + if (!SUCCEEDED(deployResult.ExtendedErrorCode())) + { + AICLI_LOG(Core, Error, << "Deployment failed #" << id << ": " << Utility::ConvertToUTF8(deployResult.ErrorText())); - THROW_HR_MSG(deployResult.ExtendedErrorCode(), "Install failed: %s", Utility::ConvertToUTF8(deployResult.ErrorText()).c_str()); - } - else - { - AICLI_LOG(Core, Info, << "Successfully deployed #" << id); - } - }); + THROW_HR_MSG(deployResult.ExtendedErrorCode(), "Install failed: %s", Utility::ConvertToUTF8(deployResult.ErrorText()).c_str()); + } + else + { + AICLI_LOG(Core, Info, << "Successfully deployed #" << id); + } } } diff --git a/src/AppInstallerCommonCore/Downloader.cpp b/src/AppInstallerCommonCore/Downloader.cpp @@ -11,126 +11,116 @@ using namespace AppInstaller::Runtime; namespace AppInstaller::Utility { - namespace + std::optional<std::vector<BYTE>> Download( + const std::string& url, + const std::filesystem::path& dest, + IProgressCallback& progress, + bool computeHash) { - std::vector<BYTE> DownloadAsyncInternal( - IPromiseKeeperProgress* progress, - const std::string& url, - const std::filesystem::path& dest, - bool computeHash) + THROW_HR_IF(E_INVALIDARG, url.empty()); + THROW_HR_IF(E_INVALIDARG, dest.empty()); + + AICLI_LOG(CLI, Info, << "Downloading url: " << url << " , dest: " << dest); + + wil::unique_hinternet session(InternetOpenA( + "appinstaller-cli", + INTERNET_OPEN_TYPE_PRECONFIG, + NULL, + NULL, + 0)); + THROW_LAST_ERROR_IF_NULL_MSG(session, "InternetOpen() failed."); + + wil::unique_hinternet urlFile(InternetOpenUrlA( + session.get(), + url.c_str(), + NULL, + 0, + INTERNET_FLAG_IGNORE_REDIRECT_TO_HTTPS, // This allows http->https redirection + 0)); + THROW_LAST_ERROR_IF_NULL_MSG(urlFile, "InternetOpenUrl() failed."); + + // Check http return status + DWORD requestStatus = 0; + DWORD cbRequestStatus = sizeof(requestStatus); + + THROW_LAST_ERROR_IF_MSG(!HttpQueryInfoA(urlFile.get(), + HTTP_QUERY_STATUS_CODE | HTTP_QUERY_FLAG_NUMBER, + &requestStatus, + &cbRequestStatus, + nullptr), "Query download request status failed."); + + if (requestStatus != HTTP_STATUS_OK) { - AICLI_LOG(CLI, Info, << "Downloading url: " << url << " , dest: " << dest); - - wil::unique_hinternet session(InternetOpenA( - "appinstaller-cli", - INTERNET_OPEN_TYPE_PRECONFIG, - NULL, - NULL, - 0)); - THROW_LAST_ERROR_IF_NULL_MSG(session, "InternetOpen() failed."); - - wil::unique_hinternet urlFile(InternetOpenUrlA( - session.get(), - url.c_str(), - NULL, - 0, - INTERNET_FLAG_IGNORE_REDIRECT_TO_HTTPS, // This allows http->https redirection - 0)); - THROW_LAST_ERROR_IF_NULL_MSG(urlFile, "InternetOpenUrl() failed."); - - // Check http return status - DWORD requestStatus = 0; - DWORD cbRequestStatus = sizeof(requestStatus); - - THROW_LAST_ERROR_IF_MSG(!HttpQueryInfoA(urlFile.get(), - HTTP_QUERY_STATUS_CODE | HTTP_QUERY_FLAG_NUMBER, - &requestStatus, - &cbRequestStatus, - nullptr), "Query download request status failed."); - - if (requestStatus != HTTP_STATUS_OK) - { - AICLI_LOG(CLI, Error, << "Download request failed. Returned status: " << requestStatus); - THROW_HR_MSG(MAKE_HRESULT(SEVERITY_ERROR, FACILITY_HTTP, requestStatus), "Download request status is not success."); - } + AICLI_LOG(CLI, Error, << "Download request failed. Returned status: " << requestStatus); + THROW_HR_MSG(MAKE_HRESULT(SEVERITY_ERROR, FACILITY_HTTP, requestStatus), "Download request status is not success."); + } - AICLI_LOG(CLI, Verbose, << "Download request status success."); + AICLI_LOG(CLI, Verbose, << "Download request status success."); - // Get content length. Don't fail the download if failed. - LONGLONG contentLength = 0; - DWORD cbContentLength = sizeof(contentLength); + // Get content length. Don't fail the download if failed. + LONGLONG contentLength = 0; + DWORD cbContentLength = sizeof(contentLength); - HttpQueryInfoA( - urlFile.get(), - HTTP_QUERY_CONTENT_LENGTH | HTTP_QUERY_FLAG_NUMBER64, - &contentLength, - &cbContentLength, - nullptr); - AICLI_LOG(CLI, Verbose, << "Download size: " << contentLength); + HttpQueryInfoA( + urlFile.get(), + HTTP_QUERY_CONTENT_LENGTH | HTTP_QUERY_FLAG_NUMBER64, + &contentLength, + &cbContentLength, + nullptr); + AICLI_LOG(CLI, Verbose, << "Download size: " << contentLength); - std::ofstream outfile(dest, std::ofstream::binary); + std::ofstream outfile(dest, std::ofstream::binary); - // Setup hash engine - SHA256 hashEngine; - std::string contentHash; + // Setup hash engine + SHA256 hashEngine; + std::string contentHash; - const int bufferSize = 1024 * 1024; // 1MB - auto buffer = std::make_unique<BYTE[]>(bufferSize); + const int bufferSize = 1024 * 1024; // 1MB + auto buffer = std::make_unique<BYTE[]>(bufferSize); - BOOL readSuccess = true; - DWORD bytesRead = 0; - LONGLONG bytesDownloaded = 0; + BOOL readSuccess = true; + DWORD bytesRead = 0; + LONGLONG bytesDownloaded = 0; - do + do + { + if (progress.IsCancelled()) { - if (progress->IsCancelled()) - { - AICLI_LOG(CLI, Info, << "Download cancelled."); - return {}; - } - - readSuccess = InternetReadFile(urlFile.get(), buffer.get(), bufferSize, &bytesRead); - - THROW_LAST_ERROR_IF_MSG(!readSuccess, "InternetReadFile() failed."); - - if (computeHash) - { - hashEngine.Add(buffer.get(), bytesRead); - } + AICLI_LOG(CLI, Info, << "Download cancelled."); + return {}; + } - outfile.write((char*)buffer.get(), bytesRead); + readSuccess = InternetReadFile(urlFile.get(), buffer.get(), bufferSize, &bytesRead); - bytesDownloaded += bytesRead; + THROW_LAST_ERROR_IF_MSG(!readSuccess, "InternetReadFile() failed."); - if (bytesRead != 0) - { - progress->OnProgress(bytesDownloaded, contentLength, FutureProgressType::Bytes); - } + if (computeHash) + { + hashEngine.Add(buffer.get(), bytesRead); + } - } while (bytesRead != 0); + outfile.write((char*)buffer.get(), bytesRead); - outfile.flush(); + bytesDownloaded += bytesRead; - std::vector<BYTE> result; - if (computeHash) + if (bytesRead != 0) { - result = hashEngine.Get(); - AICLI_LOG(CLI, Info, << "Download hash: " << SHA256::ConvertToString(result)); + progress.OnProgress(bytesDownloaded, contentLength, ProgressType::Bytes); } - AICLI_LOG(CLI, Info, << "Download completed."); + } while (bytesRead != 0); - return result; + outfile.flush(); + + std::vector<BYTE> result; + if (computeHash) + { + result = hashEngine.Get(); + AICLI_LOG(CLI, Info, << "Download hash: " << SHA256::ConvertToString(result)); } - } - Future<std::vector<BYTE>> DownloadAsync( - const std::string& url, - const std::filesystem::path& dest, - bool computeHash) - { - THROW_HR_IF(E_INVALIDARG, url.empty()); - THROW_HR_IF(E_INVALIDARG, dest.empty()); - return Future<std::vector<BYTE>>(std::bind(DownloadAsyncInternal, std::placeholders::_1, url, dest, computeHash)); + AICLI_LOG(CLI, Info, << "Download completed."); + + return result; } } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Public/AppInstallerDeployment.h b/src/AppInstallerCommonCore/Public/AppInstallerDeployment.h @@ -1,14 +1,15 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once -#include <AppInstallerFuture.h> +#include <AppInstallerProgress.h> #include <winrt/Windows.Foundation.h> #include <winrt/Windows.Management.Deployment.h> namespace AppInstaller::Deployment { // Calls winrt::Windows::Management::Deployment::PackageManager::RequestAddPackageAsync as a Future. - Future<void> RequestAddPackageAsync( + void RequestAddPackageAsync( const winrt::Windows::Foundation::Uri& uri, - winrt::Windows::Management::Deployment::DeploymentOptions options); + winrt::Windows::Management::Deployment::DeploymentOptions options, + IProgressCallback& callback); } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerDownloader.h b/src/AppInstallerCommonCore/Public/AppInstallerDownloader.h @@ -1,7 +1,12 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once -#include <AppInstallerFuture.h> +#include <AppInstallerProgress.h> + +#include <filesystem> +#include <optional> +#include <string> +#include <vector> namespace AppInstaller::Utility { @@ -9,8 +14,9 @@ namespace AppInstaller::Utility // url: The url to be downloaded from. http->https redirection is allowed. // dest: The path to local file to be downloaded to. // computeHash: Optional. Indicates if SHA256 hash should be calculated when downloading. - Future<std::vector<BYTE>> DownloadAsync( + std::optional<std::vector<BYTE>> Download( const std::string& url, const std::filesystem::path& dest, + IProgressCallback& progress, bool computeHash = false); } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Public/AppInstallerFuture.h b/src/AppInstallerCommonCore/Public/AppInstallerFuture.h @@ -1,179 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. -#pragma once -#include <wil/resource.h> - -#include <atomic> -#include <future> -#include <optional> -#include <type_traits> -#include <utility> - -namespace AppInstaller -{ - // The semantic meaning of the progress values. - enum class FutureProgressType - { - // Progress will not be sent. - None, - Bytes, - Percent, - }; - - // Callback interface for receiving progress data. - struct IFutureProgress - { - // Called when the future has started processing. - virtual void OnStarted() = 0; - - // Called as progress is made. - // If maximum is 0, the maximum is unknown. - virtual void OnProgress(uint64_t current, uint64_t maximum, FutureProgressType type) = 0; - - // Called when the future is done processing. - virtual void OnCompleted(bool cancelled) = 0; - }; - - // Callback interface given to the promise keeper to work with. - struct IPromiseKeeperProgress - { - // Called as progress is made. - // If maximum is 0, the maximum is unknown. - virtual void OnProgress(uint64_t current, uint64_t maximum, FutureProgressType type) = 0; - - // Returns a value indicating if the future has been cancelled. - virtual bool IsCancelled() = 0; - }; - - namespace details - { - struct PromiseKeeperProgress : public IPromiseKeeperProgress - { - void OnStarted() - { - IFutureProgress* receiver = GetFutureProgress(); - if (receiver) - { - receiver->OnStarted(); - } - } - - void OnProgress(uint64_t current, uint64_t maximum, FutureProgressType type) override - { - IFutureProgress* receiver = GetFutureProgress(); - if (receiver) - { - receiver->OnProgress(current, maximum, type); - } - } - - void OnCompleted(bool cancelled) - { - IFutureProgress* receiver = GetFutureProgress(); - if (receiver) - { - receiver->OnCompleted(cancelled); - } - } - - bool IsCancelled() override - { - return m_cancelled.load(); - } - - IFutureProgress* GetFutureProgress() - { - return m_receiver.load(); - } - - std::atomic<IFutureProgress*> m_receiver = nullptr; - std::atomic_bool m_cancelled = false; - }; - - // Helper to determine template type info. - template <typename T> - struct TemplateDeduction - { - using t = T; - }; - - template <template<typename> typename T, typename U> - struct TemplateDeduction<T<U>> - { - using t = T<void>; - }; - - template <typename T> - using TemplateDeduction_t = typename TemplateDeduction<T>::t; - } - - // Future wrapper that enables progress to be hooked up by caller. - template <typename Result> - struct Future - { - using Task = std::packaged_task<Result(IPromiseKeeperProgress*)>; - // If the incoming Result type is void or std::optional, just use that as the return type for Get. - // Otherwise, wrap it in std::optional. - using GetResult = std::conditional_t<std::is_same_v<void, Result> || std::is_same_v<std::optional<void>, details::TemplateDeduction_t<Result>>, Result, std::optional<Result>>; - - Future(Task&& task) : m_task(std::move(task)) {} - - template <typename F> - explicit Future(F&& f) : m_task(std::move(f)) {} - - Future(const Future&) = delete; - Future& operator=(const Future&) = delete; - - Future(Future&&) = default; - Future& operator=(Future&&) = default; - - // Cancel the processing of the future, if possible. - void Cancel() { m_progress.m_cancelled = true; } - - // Gets the result, waiting as required. - // If cancelled, result depends on promise keeper. - GetResult Get() - { - m_progress.OnStarted(); - - std::future future = m_task.get_future(); - m_task(&m_progress); - - auto scopeExit = wil::scope_exit([&]() - { - if (m_progress.IsCancelled()) - { - m_progress.OnCompleted(true); - } - else - { - m_progress.OnCompleted(false); - } - }); - - future.wait(); - - if (m_progress.IsCancelled()) - { - return HandleDefaultReturn<GetResult>::Return(); - } - else - { - return future.get(); - } - } - - // Sets the progress receiver. - void SetProgressReceiver(IFutureProgress* receiver) { m_progress.m_receiver = receiver; } - - private: - template <typename T> - struct HandleDefaultReturn { static T Return() { return T{}; } }; - - template <> - struct HandleDefaultReturn<void> { static void Return() {} }; - - Task m_task; - details::PromiseKeeperProgress m_progress; - }; -} diff --git a/src/AppInstallerCommonCore/Public/AppInstallerProgress.h b/src/AppInstallerCommonCore/Public/AppInstallerProgress.h @@ -0,0 +1,63 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include <atomic> + +namespace AppInstaller +{ + // The semantic meaning of the progress values. + enum class ProgressType + { + // Progress will not be sent. + None, + Bytes, + Percent, + }; + + // Callback interface given to the worker to report to. + // Also enables the caller to request cancellation. + struct IProgressCallback + { + // Called as progress is made. + // If maximum is 0, the maximum is unknown. + virtual void OnProgress(uint64_t current, uint64_t maximum, ProgressType type) = 0; + + // Returns a value indicating if the future has been cancelled. + virtual bool IsCancelled() = 0; + }; + + // Implementation of IProgressCallback. + struct ProgressCallback : public IProgressCallback + { + ProgressCallback() = default; + ProgressCallback(IProgressCallback* callback) : m_callback(callback) {} + + void OnProgress(uint64_t current, uint64_t maximum, ProgressType type) override + { + IProgressCallback* callback = GetCallback(); + if (callback) + { + callback->OnProgress(current, maximum, type); + } + } + + bool IsCancelled() override + { + return m_cancelled.load(); + } + + void Cancel() + { + m_cancelled = true; + } + + IProgressCallback* GetCallback() + { + return m_callback.load(); + } + + private: + std::atomic<IProgressCallback*> m_callback = nullptr; + std::atomic_bool m_cancelled = false; + }; +}