From e0c626e63d12a39431d5e626eb8a58beebe9b019 Mon Sep 17 00:00:00 2001
From: JohnMcPMS <johnmcp@microsoft.com>
Date: Mon, 18 Sep 2023 12:04:43 -0700
Subject: [PATCH] Read value data on demand

---
 src/AppInstallerSharedLib/GroupPolicy.cpp     | 17 ++++++++++----
 .../Public/winget/Registry.h                  |  9 ++++++--
 src/AppInstallerSharedLib/Registry.cpp        | 23 +++++++++++--------
 3 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/src/AppInstallerSharedLib/GroupPolicy.cpp b/src/AppInstallerSharedLib/GroupPolicy.cpp
index 3250dee63e..6777d000b0 100644
--- 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
index fcbae166f2..82bc2a3605 100644
--- 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
index 402d090020..cac167136b 100644
--- 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