winget-cli

commit 5a6134a966447ef7b3977fe034f8e8a48e767ac6
parent 04ccec385f842590d02e94fe784ee162648d8257
Author: JohnMcPMS <johnmcp@microsoft.com>
Date:   Tue, 19 Sep 2023 15:31:41 -0700

Read value data on demand (#3642)

When enumerating registry keys, move the code that reads the registry value data to be on request rather than during the iterator advancement.  This enables code that doesn't need the data to simply skip it, and code that does to better handle the error if desired.
Diffstat:
M
src/AppInstallerSharedLib/GroupPolicy.cpp
 | 
17
+++++++++++++----
M
src/AppInstallerSharedLib/Public/winget/Registry.h
 | 
9
+++++++--
M
src/AppInstallerSharedLib/Registry.cpp
 | 
23
++++++++++++++---------

3 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/src/AppInstallerSharedLib/GroupPolicy.cpp b/src/AppInstallerSharedLib/GroupPolicy.cpp
@@ -136,14 +136,23 @@ namespace AppInstaller::Settings
             typename Mapping::value_t items;
             for (const auto& value : listKey->Values())
             {
-                auto item = Mapping::ReadAndValidateItem(value);
-                if (item.has_value())
+                std::optional<Registry::Value> potentialValue = value.Value();
+
+                if (potentialValue)
                 {
-                    items.emplace_back(std::move(item.value()));
+                    auto item = Mapping::ReadAndValidateItem(potentialValue.value());
+                    if (item.has_value())
+                    {
+                        items.emplace_back(std::move(item.value()));
+                    }
+                    else
+                    {
+                        AICLI_LOG(Core, Warning, << "Failed to read Group Policy list value. Policy [" << Mapping::KeyName << "], Value [" << value.Name() << ']');
+                    }
                 }
                 else
                 {
-                    AICLI_LOG(Core, Warning, << "Failed to read Group Policy list value. Policy [" << Mapping::KeyName << "], Value [" << value.Name() << ']');
+                    AICLI_LOG(Core, Verbose, << "Group Policy list value not found. Policy [" << Mapping::KeyName << "], Value [" << value.Name() << ']');
                 }
             }
 
diff --git a/src/AppInstallerSharedLib/Public/winget/Registry.h b/src/AppInstallerSharedLib/Public/winget/Registry.h
@@ -148,16 +148,21 @@ namespace AppInstaller::Registry
 
         struct const_iterator;
 
-        struct ValueRef : Value
+        struct ValueRef
         {
             friend const_iterator;
 
             // Gets the name of the value.
             std::string Name() const;
 
+            // Gets the actual value of the value.
+            // The optional allows for the potential race with the value being removed.
+            std::optional<Value> Value() const;
+
         private:
-            ValueRef(std::wstring&& valueName, DWORD type, std::vector<BYTE>&& data);
+            ValueRef(wil::shared_hkey key, std::wstring&& valueName);
 
+            wil::shared_hkey m_key;
             std::wstring m_valueName;
         };
 
diff --git a/src/AppInstallerSharedLib/Registry.cpp b/src/AppInstallerSharedLib/Registry.cpp
@@ -188,13 +188,25 @@ namespace AppInstaller::Registry
         return m_type == type;
     }
 
-    ValueList::ValueRef::ValueRef(std::wstring&& valueName, DWORD type, std::vector<BYTE>&& data) : Value(type, std::move(data)), m_valueName(std::move(valueName)) {}
+    ValueList::ValueRef::ValueRef(wil::shared_hkey key, std::wstring&& valueName) : m_key(std::move(key)), m_valueName(std::move(valueName)) {}
 
     std::string ValueList::ValueRef::Name() const
     {
         return Utility::ConvertToUTF8(m_valueName);
     }
 
+    std::optional<Value> ValueList::ValueRef::Value() const
+    {
+        DWORD type;
+        std::vector<BYTE> data;
+        if (!TryGetRegistryValueData(m_key, m_valueName, type, data))
+        {
+            return std::nullopt;
+        }
+
+        return Registry::Value{ type, std::move(data) };
+    }
+
     ValueList::const_iterator& ValueList::const_iterator::operator++()
     {
         ++m_index;
@@ -232,14 +244,7 @@ namespace AppInstaller::Registry
             return;
         }
 
-        DWORD type;
-        std::vector<BYTE> data;
-        if (!TryGetRegistryValueData(m_key, valueName, type, data))
-        {
-            THROW_HR(E_UNEXPECTED);
-        }
-
-        m_value = ValueRef{ std::move(valueName), type, std::move(data) };
+        m_value = ValueRef{ m_key, std::move(valueName) };
     }
 
     const ValueList::ValueRef& ValueList::const_iterator::operator*() const