[PR Feedback]:Refactor error handling and add admin/system check func…

…tions - Added IsRunningAsAdminOrSystem() in Runtime.h and Runtime.cpp. - Added COM class entry for RemovePackageCatalogOptions in Microsoft.Management.Deployment.InProc.dll.manifest. - Corrected NameCLSIDPair syntax in Factory.cpp. - Added and declared new status mapping functions in Converters.cpp and Converters.h. - Encapsulated GetRefreshPackageCatalogResult in PackageCatalogReference.cpp. - Encapsulated package catalog operation functions in PackageManager.cpp. - Removed ResetPackageCatalogResult class and its implementation. - Updated error codes in PackageCatalogInterop.cs.
Madhusudhan-MSFT · Oct 23, 2024 · 4dcdc2a · 4dcdc2a
1 parent ea785f3
commit 4dcdc2a
Show file tree

Hide file tree

Showing 11 changed files with 188 additions and 236 deletions.
diff --git a/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs b/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs
@@ -78,10 +78,10 @@ public async Task AddRemovePackageCatalog()
         public async Task AddPackageCatalogWithInvalidOptions()
         {
             // Add package catalog with null options.
-            await this.AddAndValidatePackageCatalogAsync(null, AddPackageCatalogStatus.InvalidOptions, Constants.ErrorCode.ERROR_INVALID_CL_ARGUMENTS);
+            await this.AddAndValidatePackageCatalogAsync(null, AddPackageCatalogStatus.InvalidOptions, Constants.ErrorCode.E_INVALIDARG);
 
             // Add package catalog with empty options.
-            await this.AddAndValidatePackageCatalogAsync(this.TestFactory.CreateAddPackageCatalogOptions(), AddPackageCatalogStatus.InvalidOptions, Constants.ErrorCode.ERROR_INVALID_CL_ARGUMENTS);
+            await this.AddAndValidatePackageCatalogAsync(this.TestFactory.CreateAddPackageCatalogOptions(), AddPackageCatalogStatus.InvalidOptions, Constants.ErrorCode.E_INVALIDARG);
         }
 
         /// <summary>
@@ -237,10 +237,10 @@ public async Task AddRemovePackageCatalogWithPreserveDataFiledSet()
         public async Task RemovePackageCatalogWithInvalidOptions()
         {
             // Remove package catalog with null options.
-            await this.RemoveAndValidatePackageCatalogAsync(null, RemovePackageCatalogStatus.InvalidOptions, Constants.ErrorCode.ERROR_INVALID_CL_ARGUMENTS);
+            await this.RemoveAndValidatePackageCatalogAsync(null, RemovePackageCatalogStatus.InvalidOptions, Constants.ErrorCode.E_INVALIDARG);
 
             // Remove package catalog with empty options.
-            await this.RemoveAndValidatePackageCatalogAsync(this.TestFactory.CreateRemovePackageCatalogOptions(), RemovePackageCatalogStatus.InvalidOptions, Constants.ErrorCode.ERROR_INVALID_CL_ARGUMENTS);
+            await this.RemoveAndValidatePackageCatalogAsync(this.TestFactory.CreateRemovePackageCatalogOptions(), RemovePackageCatalogStatus.InvalidOptions, Constants.ErrorCode.E_INVALIDARG);
         }
 
         /// <summary>

diff --git a/src/AppInstallerSharedLib/Public/winget/Runtime.h b/src/AppInstallerSharedLib/Public/winget/Runtime.h
@@ -40,6 +40,9 @@ namespace AppInstaller::Runtime
     // Determines whether the process is running with local system context.
     bool IsRunningAsSystem();
 
+    // Determines whether the process is running with administrator or system privileges.
+    bool IsRunningAsAdminOrSystem();
+
     // Returns true if this is a release build; false if not.
     inline constexpr bool IsReleaseBuild()
     {

diff --git a/src/AppInstallerSharedLib/Runtime.cpp b/src/AppInstallerSharedLib/Runtime.cpp
@@ -208,4 +208,9 @@ namespace AppInstaller::Runtime
     {
         return wil::test_token_membership(nullptr, SECURITY_NT_AUTHORITY, SECURITY_LOCAL_SYSTEM_RID);
     }
+
+    bool IsRunningAsAdminOrSystem()
+    {
+        return IsRunningAsAdmin() || IsRunningAsSystem();
+    }
 }
diff --git a/...icrosoft.Management.Deployment.InProc/Microsoft.Management.Deployment.InProc.dll.manifest b/...icrosoft.Management.Deployment.InProc/Microsoft.Management.Deployment.InProc.dll.manifest
@@ -51,5 +51,9 @@
         clsid="{24E6F1FA-E4C3-4ACD-965D-DF213FD58F15}"
         threadingModel="Both"
         description="AddPackageCatalogOptions"/>
+     <comClass
+         clsid="{1125D3A6-E2CE-479A-91D5-71A3F6F8B00B}"
+        threadingModel="Both"
+        description="RemovePackageCatalogOptions"/>
   </file>
 </assembly>
diff --git a/src/Microsoft.Management.Deployment.OutOfProc/Factory.cpp b/src/Microsoft.Management.Deployment.OutOfProc/Factory.cpp
@@ -57,7 +57,7 @@ namespace Microsoft::Management::Deployment::OutOfProc
             NameCLSIDPair{ L"Microsoft.Management.Deployment.AuthenticationArguments"sv, CLSID_AuthenticationArguments },
             NameCLSIDPair{ L"Microsoft.Management.Deployment.RepairOptions"sv, CLSID_RepairOptions },
             NameCLSIDPair{ L"Microsoft.Management.Deployment.AddPackageCatalogOptions"sv, CLSID_AddPackageCatalogOptions },
-            NameCLSIDPair{ L"Microsoft.Management.Deployment.RemovePackageCatalogOptions"sv, CLSID_RemovePackageCatalogOptions }
+            NameCLSIDPair{ L"Microsoft.Management.Deployment.RemovePackageCatalogOptions"sv, CLSID_RemovePackageCatalogOptions },
         };
 
         bool IsCLSIDPresent(const GUID& clsid)

diff --git a/src/Microsoft.Management.Deployment/Converters.cpp b/src/Microsoft.Management.Deployment/Converters.cpp
@@ -497,4 +497,37 @@ namespace winrt::Microsoft::Management::Deployment::implementation
 
         return result;
     }
+
+    AddPackageCatalogStatus GetAddPackageCatalogOperationStatus(winrt::hresult hresult)
+    {
+        switch (hresult)
+        {
+        case APPINSTALLER_CLI_ERROR_AUTHENTICATION_TYPE_NOT_SUPPORTED:
+            return AddPackageCatalogStatus::AuthenticationError;
+        case APPINSTALLER_CLI_ERROR_SOURCE_NOT_SECURE:
+        case APPINSTALLER_CLI_ERROR_INVALID_SOURCE_TYPE:
+        case APPINSTALLER_CLI_ERROR_SOURCE_NOT_REMOTE:
+        case APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS:
+        case APPINSTALLER_CLI_ERROR_SOURCE_ARG_ALREADY_EXISTS:
+            return AddPackageCatalogStatus::InvalidOptions;
+        case APPINSTALLER_CLI_ERROR_SOURCE_OPEN_FAILED:
+            return AddPackageCatalogStatus::CatalogError;
+        default:
+            return HandleCommonCatalogOperationStatus<AddPackageCatalogStatus>(hresult);
+        }
+    }
+
+    RemovePackageCatalogStatus GetRemovePackageCatalogOperationStatus(winrt::hresult hresult)
+    {
+        switch (hresult)
+        {
+        case APPINSTALLER_CLI_ERROR_SOURCE_NAME_DOES_NOT_EXIST:
+            return RemovePackageCatalogStatus::InvalidOptions;
+        case APPINSTALLER_CLI_ERROR_INVALID_SOURCE_TYPE:
+            return RemovePackageCatalogStatus::CatalogError;
+        default:
+            return HandleCommonCatalogOperationStatus<RemovePackageCatalogStatus>(hresult);
+        }
+    }
+
 }
diff --git a/src/Microsoft.Management.Deployment/Converters.h b/src/Microsoft.Management.Deployment/Converters.h
@@ -31,6 +31,8 @@ namespace winrt::Microsoft::Management::Deployment::implementation
     ::AppInstaller::Authentication::AuthenticationMode GetAuthenticationMode(winrt::Microsoft::Management::Deployment::AuthenticationMode authMode);
     ::AppInstaller::Authentication::AuthenticationArguments GetAuthenticationArguments(winrt::Microsoft::Management::Deployment::AuthenticationArguments authArgs);
     ::AppInstaller::Manifest::ScopeEnum GetManifestRepairScope(winrt::Microsoft::Management::Deployment::PackageRepairScope scope);
+    winrt::Microsoft::Management::Deployment::AddPackageCatalogStatus GetAddPackageCatalogOperationStatus(winrt::hresult hresult);
+    winrt::Microsoft::Management::Deployment::RemovePackageCatalogStatus GetRemovePackageCatalogOperationStatus(winrt::hresult hresult);
 
 #define WINGET_GET_OPERATION_RESULT_STATUS(_installResultStatus_, _uninstallResultStatus_, _downloadResultStatus_, _repairResultStatus_) \
     if constexpr (std::is_same_v<TStatus, winrt::Microsoft::Management::Deployment::InstallResultStatus>) \
@@ -176,50 +178,16 @@ namespace winrt::Microsoft::Management::Deployment::implementation
         }
     }
 
-    template <typename TStatus>
-    TStatus GetAddPackageCatalogOperationStatus(winrt::hresult hresult)
-    {
-        switch (hresult)
-        {
-        case APPINSTALLER_CLI_ERROR_AUTHENTICATION_TYPE_NOT_SUPPORTED:
-            return TStatus::AuthenticationError;
-        case APPINSTALLER_CLI_ERROR_SOURCE_NOT_SECURE:
-        case APPINSTALLER_CLI_ERROR_INVALID_SOURCE_TYPE:
-        case APPINSTALLER_CLI_ERROR_SOURCE_NOT_REMOTE:
-        case APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS:
-        case APPINSTALLER_CLI_ERROR_SOURCE_ARG_ALREADY_EXISTS:
-            return TStatus::InvalidOptions;
-        case APPINSTALLER_CLI_ERROR_SOURCE_OPEN_FAILED:
-            return TStatus::CatalogError;
-        default:
-            return HandleCommonCatalogOperationStatus<TStatus>(hresult);
-        }
-    }
-
-    template <typename TStatus>
-    TStatus GetRemovePackageCatalogOperationStatus(winrt::hresult hresult)
-    {
-        switch (hresult)
-        {
-        case APPINSTALLER_CLI_ERROR_SOURCE_NAME_DOES_NOT_EXIST:
-            return TStatus::InvalidOptions;
-        case APPINSTALLER_CLI_ERROR_INVALID_SOURCE_TYPE:
-            return TStatus::CatalogError;
-        default:
-            return HandleCommonCatalogOperationStatus<TStatus>(hresult);
-        }
-    }
-
     template <typename TStatus>
     TStatus GetPackageCatalogOperationStatus(winrt::hresult hresult)
     {
         if constexpr (std::is_same_v<TStatus, winrt::Microsoft::Management::Deployment::AddPackageCatalogStatus>)
         {
-            return GetAddPackageCatalogOperationStatus<AddPackageCatalogStatus>(hresult);
+            return GetAddPackageCatalogOperationStatus(hresult);
         }
         else if constexpr (std::is_same_v<TStatus, winrt::Microsoft::Management::Deployment::RemovePackageCatalogStatus>)
         {
-            return GetRemovePackageCatalogOperationStatus<RemovePackageCatalogStatus>(hresult);
+            return GetRemovePackageCatalogOperationStatus(hresult);
         }
         else if constexpr (std::is_same_v<TStatus, winrt::Microsoft::Management::Deployment::RefreshPackageCatalogStatus>)
         {

diff --git a/src/Microsoft.Management.Deployment/PackageCatalogReference.cpp b/src/Microsoft.Management.Deployment/PackageCatalogReference.cpp
@@ -23,6 +23,17 @@
 
 namespace winrt::Microsoft::Management::Deployment::implementation
 {
+    namespace
+    {
+        winrt::Microsoft::Management::Deployment::RefreshPackageCatalogResult GetRefreshPackageCatalogResult(winrt::hresult terminationStatus)
+        {
+            winrt::Microsoft::Management::Deployment::RefreshPackageCatalogStatus status = GetPackageCatalogOperationStatus<RefreshPackageCatalogStatus>(terminationStatus);
+            auto updateResult = winrt::make_self<wil::details::module_count_wrapper<winrt::Microsoft::Management::Deployment::implementation::RefreshPackageCatalogResult>>();
+            updateResult->Initialize(status, terminationStatus);
+            return *updateResult;
+        }
+    }
+
     void PackageCatalogReference::Initialize(winrt::Microsoft::Management::Deployment::PackageCatalogInfo packageCatalogInfo, ::AppInstaller::Repository::Source sourceReference)
     {
         m_info = packageCatalogInfo;
@@ -292,14 +303,6 @@ namespace winrt::Microsoft::Management::Deployment::implementation
         return m_authenticationInfo;
     }
 
-    winrt::Microsoft::Management::Deployment::RefreshPackageCatalogResult GetRefreshPackageCatalogResult(winrt::hresult terminationStatus)
-    {
-        winrt::Microsoft::Management::Deployment::RefreshPackageCatalogStatus status = GetPackageCatalogOperationStatus<RefreshPackageCatalogStatus>(terminationStatus);
-        auto updateResult = winrt::make_self<wil::details::module_count_wrapper<winrt::Microsoft::Management::Deployment::implementation::RefreshPackageCatalogResult>>();
-        updateResult->Initialize(status, terminationStatus);
-        return *updateResult;
-    }
-
     winrt::Windows::Foundation::IAsyncOperationWithProgress<winrt::Microsoft::Management::Deployment::RefreshPackageCatalogResult, double> PackageCatalogReference::RefreshPackageCatalogAsync()
     {
         HRESULT terminationHR = S_OK;