pr comments

yao-msft · May 9, 2024 · 58305d7 · 58305d7
1 parent 6bc5acf
commit 58305d7
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 41 deletions.
diff --git a/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp b/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp
@@ -245,13 +245,11 @@ namespace AppInstaller::CLI::Workflow
                 if (installerDownloadOnly)
                 {
                     context <<
+                        EnsureFeatureEnabled(Settings::ExperimentalFeature::Feature::StoreDownload) <<
                         MSStoreDownload <<
                         ExportManifest;
                 }
-                else
-                {
-                    // Nothing to do here
-                }
+
                 return;
             default:
                 THROW_HR(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED));

diff --git a/src/AppInstallerCLICore/Workflows/MSStoreInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/MSStoreInstallerHandler.cpp
@@ -264,11 +264,6 @@ namespace AppInstaller::CLI::Workflow
 
     void MSStoreDownload(Execution::Context& context)
     {
-        if (!Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::StoreDownload))
-        {
-            THROW_HR(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED));
-        }
-
         if (context.Args.Contains(Execution::Args::Type::Rename))
         {
             context.Reporter.Warn() << Resource::String::MSStoreDownloadRenameNotSupported << std::endl;
@@ -324,7 +319,6 @@ namespace AppInstaller::CLI::Workflow
         bool skipDependencies = context.Args.Contains(Execution::Args::Type::SkipDependencies);
 
         // Prepare directories
-        THROW_HR_IF(E_UNEXPECTED, !context.Contains(Execution::Data::DownloadDirectory));
         std::filesystem::path downloadDirectory = context.Get<Execution::Data::DownloadDirectory>();
         std::filesystem::path dependenciesDirectory = downloadDirectory / L"Dependencies";
 
@@ -352,11 +346,10 @@ namespace AppInstaller::CLI::Workflow
         }
 
         // Download main packages
+        AICLI_LOG(CLI, Info, << "Downloading MSStore main packages");
         context.Reporter.Info() << Resource::String::MSStoreDownloadMainPackages << std::endl;
         for (auto const& mainPackage : downloadInfo.MainPackages)
         {
-            AICLI_LOG(CLI, Info, << "Downloading MSStore main packages");
-
             THROW_IF_FAILED(DownloadMSStorePackageFile(mainPackage, downloadDirectory, context));
         }
 
@@ -371,7 +364,7 @@ namespace AppInstaller::CLI::Workflow
             std::vector<BYTE> licenseContent;
             try
             {
-                licenseContent = downloadContext.GetLicense();
+                licenseContent = downloadContext.GetLicense(downloadInfo.ContentId);
             }
             catch (const wil::ResultException& re)
             {

diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
@@ -2893,10 +2893,10 @@ Please specify one of them using the --source option to proceed.</value>
     <value>No applicable Microsoft Store package found from Microsoft Store package catalog.</value>
   </data>
   <data name="APPINSTALLER_CLI_ERROR_SFSCLIENT_API_FAILED" xml:space="preserve">
-    <value>Failed to get Microsoft Store package download infomation.</value>
+    <value>Failed to get Microsoft Store package download information.</value>
   </data>
   <data name="APPINSTALLER_CLI_ERROR_NO_APPLICABLE_SFSCLIENT_PACKAGE" xml:space="preserve">
-    <value>No applicable Microsoft Store package download infomation found.</value>
+    <value>No applicable Microsoft Store package download information found.</value>
   </data>
   <data name="APPINSTALLER_CLI_ERROR_LICENSING_API_FAILED" xml:space="preserve">
     <value>Failed to retrieve Microsoft Store package license.</value>
@@ -2922,10 +2922,10 @@ Please specify one of them using the --source option to proceed.</value>
     <value>Microsoft Store package download completed</value>
   </data>
   <data name="MSStoreDownloadMainPackages" xml:space="preserve">
-    <value>Downloading main packages...</value>
+    <value>Downloading main packages from Microsoft Store...</value>
   </data>
   <data name="MSStoreDownloadDependencyPackages" xml:space="preserve">
-    <value>Downloading dependency packages...</value>
+    <value>Downloading dependency packages from Microsoft Store...</value>
   </data>
   <data name="MSStoreDownloadGetDownloadInfo" xml:space="preserve">
     <value>Retrieving Microsoft Store package download information</value>
@@ -2944,7 +2944,7 @@ Please specify one of them using the --source option to proceed.</value>
     <value>Failed to retrieve Microsoft Store package license</value>
   </data>
   <data name="MSStoreDownloadRenameNotSupported" xml:space="preserve">
-    <value>Microsoft Store package download does not support --rename argument. Microsoft Store package will use names provided by Microsoft Store catalog</value>
+    <value>Microsoft Store package download does not support --rename argument. Microsoft Store package will use names provided by Microsoft Store catalog.</value>
     <comment>{Locked="--rename"}</comment>
   </data>
   <data name="MSStoreDownloadAuthenticationNotice" xml:space="preserve">

diff --git a/src/AppInstallerCommonCore/MSStoreDownload.cpp b/src/AppInstallerCommonCore/MSStoreDownload.cpp
@@ -4,7 +4,6 @@
 #include <AppInstallerStrings.h>
 #include <AppInstallerErrors.h>
 #include <AppinstallerLogging.h>
-#include <AppInstallerSHA256.h>
 #include "AppInstallerMsixInfo.h"
 #include "AppInstallerRuntime.h"
 #include "winget/Locale.h"
@@ -862,8 +861,8 @@ namespace AppInstaller::MSStore
                 appContents = (*TestHooks::s_SfsClient_AppContents_Override)(wuCategoryId);
             }
             else
-            {
 #endif
+            {
                 SFS::RequestParams sfsClientRequest;
                 sfsClientRequest.productRequests = { {std::string{ wuCategoryId }, {}} };
 
@@ -873,9 +872,7 @@ namespace AppInstaller::MSStore
                     AICLI_LOG(Core, Error, << "Failed to call SfsClient GetLatestAppDownloadInfo. Error code: " << requestResult.GetCode() << " Message: " << requestResult.GetMsg());
                     THROW_HR_MSG(APPINSTALLER_CLI_ERROR_SFSCLIENT_API_FAILED, "Failed to call SfsClient GetLatestAppDownloadInfo. ErrorCode: %lu Message: %hs", requestResult.GetCode(), requestResult.GetMsg().c_str());
                 }
-#ifndef AICLI_DISABLE_TEST_HOOKS
             }
-#endif
 
             THROW_HR_IF(E_UNEXPECTED, appContents.empty());
 
@@ -927,9 +924,9 @@ namespace AppInstaller::MSStore
         //     ]
         //   }
         // }
-        std::vector<BYTE> GetLicencing(std::string_view contentId, const Http::HttpClientHelper::HttpRequestHeaders& authHeaders)
+        std::vector<BYTE> GetLicensing(std::string_view contentId, const Http::HttpClientHelper::HttpRequestHeaders& authHeaders)
         {
-            AICLI_LOG(Core, Error, << "GetLicencing with ContentId: " << contentId);
+            AICLI_LOG(Core, Error, << "GetLicensing with ContentId: " << contentId);
 
             AppInstaller::Http::HttpClientHelper httpClientHelper;
 
@@ -1012,8 +1009,8 @@ namespace AppInstaller::MSStore
     {
 #ifndef AICLI_DISABLE_TEST_HOOKS
         if (!TestHooks::s_Licensing_HttpPipelineStage_Override)
-        {
 #endif
+        {
             Authentication::MicrosoftEntraIdAuthenticationInfo licensingMicrosoftEntraIdAuthInfo;
             licensingMicrosoftEntraIdAuthInfo.Resource = "c5e1cb0d-5d24-4b1a-b291-ec684152b2ba";
             Authentication::AuthenticationInfo licensingAuthInfo;
@@ -1022,27 +1019,23 @@ namespace AppInstaller::MSStore
 
             // Not moving authArgs because we'll have auth for display catalog and sfs client in the near future.
             m_licensingAuthenticator = std::make_unique<Authentication::Authenticator>(std::move(licensingAuthInfo), authArgs);
-#ifndef AICLI_DISABLE_TEST_HOOKS
         }
-#endif
     }
 
     MSStoreDownloadInfo MSStoreDownloadContext::GetDownloadInfo()
     {
 #ifndef WINGET_DISABLE_FOR_FUZZING
         auto displayCatalogPackage = DisplayCatalogDetails::CallDisplayCatalogAndGetPreferredPackage(m_productId, m_locale, m_architecture);
         auto downloadInfo = SfsClientDetails::CallSfsClientAndGetMSStoreDownloadInfo(displayCatalogPackage.WuCategoryId, m_architecture, m_platform);
-        m_contentId = displayCatalogPackage.ContentId;
+        downloadInfo.ContentId = displayCatalogPackage.ContentId;
         return downloadInfo;
 #else
         return {};
 #endif
     }
 
-    std::vector<BYTE> MSStoreDownloadContext::GetLicense()
+    std::vector<BYTE> MSStoreDownloadContext::GetLicense(std::string_view contentId)
     {
-        THROW_HR_IF_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), m_contentId.empty(), "GetDownloadInfo() must be called before GetLicense()");
-
-        return LicensingDetails::GetLicencing(m_contentId, GetAuthHeaders(m_licensingAuthenticator));
+        return LicensingDetails::GetLicensing(contentId, GetAuthHeaders(m_licensingAuthenticator));
     }
 }
diff --git a/src/AppInstallerCommonCore/MsixInfo.cpp b/src/AppInstallerCommonCore/MsixInfo.cpp
@@ -479,17 +479,16 @@ namespace AppInstaller::Msix
 
         THROW_HR_IF(E_UNEXPECTED, length == 0);
 
-        std::vector<BYTE> packageIdContent;
-        packageIdContent.resize(length);
+        std::unique_ptr<BYTE[]> packageIdContent = std::make_unique<BYTE[]>(length);
 
-        returnVal = PackageIdFromFullName(fullNameWide.c_str(), PACKAGE_INFORMATION_BASIC, &length, packageIdContent.data());
+        returnVal = PackageIdFromFullName(fullNameWide.c_str(), PACKAGE_INFORMATION_BASIC, &length, packageIdContent.get());
         if (returnVal != ERROR_SUCCESS)
         {
             LOG_WIN32(returnVal);
             return 0;
         }
 
-        PACKAGE_ID* packageId = (PACKAGE_ID*)packageIdContent.data();
+        PACKAGE_ID* packageId = (PACKAGE_ID*)packageIdContent.get();
 
         return packageId->version.Version;
     }

diff --git a/src/AppInstallerCommonCore/Public/winget/MSStoreDownload.h b/src/AppInstallerCommonCore/Public/winget/MSStoreDownload.h
@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 #pragma once
 #include <AppInstallerArchitecture.h>
+#include <AppInstallerSHA256.h>
 #include <AppInstallerVersions.h>
 #include "winget/Authentication.h"
 #include "winget/ManifestCommon.h"
@@ -18,7 +19,7 @@ namespace AppInstaller::MSStore
     struct MSStoreDownloadFile
     {
         std::string Url;
-        std::vector<BYTE> Sha256;
+        AppInstaller::Utility::SHA256::HashBuffer Sha256;
         std::string FileName;
         AppInstaller::Utility::UInt64Version Version = 0;
     };
@@ -27,6 +28,8 @@ namespace AppInstaller::MSStore
     {
         std::vector<MSStoreDownloadFile> MainPackages;
         std::vector<MSStoreDownloadFile> DependencyPackages;
+
+        std::string ContentId;
     };
 
     struct MSStoreDownloadContext
@@ -41,16 +44,14 @@ namespace AppInstaller::MSStore
         // Calls display catalog API and sfs-client to get download info.
         MSStoreDownloadInfo GetDownloadInfo();
 
-        // Gets license for the corresponding package returned by previous GetDownloadInfo().
-        // GetDownloadInfo() must be called before calling this method.
-        std::vector<BYTE> GetLicense();
+        // Gets license for the corresponding packages
+        std::vector<BYTE> GetLicense(std::string_view contentId);
 
     private:
         std::string m_productId;
         AppInstaller::Utility::Architecture m_architecture = AppInstaller::Utility::Architecture::Unknown;
         AppInstaller::Manifest::PlatformEnum m_platform = AppInstaller::Manifest::PlatformEnum::Unknown;
         std::string m_locale;
         std::unique_ptr<AppInstaller::Authentication::Authenticator> m_licensingAuthenticator;
-        std::string m_contentId;
     };
 }