Skip to content

Commit 63e62e0

Browse files
DinahK-2SODinahK-2SO
authored
Fix FileTypeChoices in Storage Pickers to preserve insertion order (#5948)
**Description** This is a fix for issue: - #5827 The existing FileTypeChoices property was built on an unordered map, which does not preserve the user-defined order. However, in the FileOpenPicker and FileSavePicker, the FileTypeChoices need to be displayed in the order they were inserted, rather than in a random order. This is important for developers' and end-users' experience because: - Developers expect to see their file type options in the logical order they added them - The first added option is usually the default selection - Maintaining consistent ordering helps end users quickly locate the desired file type Additionaly, the insertion order is respected in the legacy UWP FileSavePicker. **Fix** This is a backward-compatible fix. The goal of this fix is to maintain the existing Map type API contract and its good performance while ensuring that the display order of FileTypeChoices meets expectations. This pull request refactors the implementation of the `FileTypeChoicesMap` to ensure that the insertion order of keys is preserved and provides efficient key lookups. It replaces the previous unordered map-based implementation with the implementation backed by a vector. The update also introduces custom iterator and view classes to support this ordered behavior and modifies related tests to verify the new insertion order.
1 parent 2b9dff7 commit 63e62e0

File tree

3 files changed

+427
-27
lines changed

3 files changed

+427
-27
lines changed

‎dev/Interop/StoragePickers/FileTypeChoicesMap.cpp‎

Lines changed: 166 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,34 @@
55
#include "FileTypeChoicesMap.h"
66
#include "FileTypeFilterVector.h"
77
#include "TerminalVelocityFeatures-StoragePickers2.h"
8+
#include <memory>
9+
#include <ranges>
810

911
namespace winrt::Microsoft::Windows::Storage::Pickers::implementation
1012
{
1113
FileTypeChoicesMap::FileTypeChoicesMap()
14+
: m_orderedMap(std::make_shared<FileTypeChoiceVector>())
1215
{
16+
// Pre-allocate capacity for typical file type choices (usually 2-8 items)
17+
m_orderedMap->reserve(8);
18+
}
19+
20+
FileTypeChoiceVector::const_iterator FileTypeChoicesMap::FindKey(hstring const& key) const noexcept
21+
{
22+
return std::ranges::find(
23+
m_orderedMap->cbegin(),
24+
m_orderedMap->cend(),
25+
key,
26+
&FileTypeChoiceVector::value_type::first);
27+
}
28+
29+
FileTypeChoiceVector::iterator FileTypeChoicesMap::FindKey(hstring const& key) noexcept
30+
{
31+
return std::ranges::find(
32+
m_orderedMap->begin(),
33+
m_orderedMap->end(),
34+
key,
35+
&FileTypeChoiceVector::value_type::first);
1336
}
1437

1538
bool FileTypeChoicesMap::Insert(hstring const& key, winrt::Windows::Foundation::Collections::IVector<hstring> const& value)
@@ -20,8 +43,8 @@ namespace winrt::Microsoft::Windows::Storage::Pickers::implementation
2043
}
2144

2245
// Create a new FileTypeFilterVector and copy all values from the input vector
23-
auto validatingVector = make<FileTypeFilterVector>();
24-
46+
auto validatingVector{ make<FileTypeFilterVector>() };
47+
2548
if (value)
2649
{
2750
// Each Append call will validate the extension
@@ -30,42 +53,172 @@ namespace winrt::Microsoft::Windows::Storage::Pickers::implementation
3053
validatingVector.as<winrt::Windows::Foundation::Collections::IVector<hstring>>().Append(item);
3154
}
3255
}
33-
34-
return m_innerMap.Insert(key, validatingVector);
56+
57+
// Check if key already exists; vector is small so linear scan is sufficient
58+
auto existing{ FindKey(key) };
59+
if (existing != m_orderedMap->end())
60+
{
61+
// Key exists, update the value
62+
existing->second = validatingVector;
63+
return false; // Indicates replacement, not insertion
64+
}
65+
else
66+
{
67+
// Key doesn't exist, insert new pair at the end to maintain insertion order
68+
m_orderedMap->emplace_back(key, validatingVector);
69+
return true; // Indicates new insertion
70+
}
3571
}
3672

3773
winrt::Windows::Foundation::Collections::IVector<hstring> FileTypeChoicesMap::Lookup(hstring const& key) const
3874
{
39-
return m_innerMap.Lookup(key);
75+
auto it{ FindKey(key) };
76+
if (it != m_orderedMap->cend())
77+
{
78+
return it->second;
79+
}
80+
throw winrt::hresult_out_of_bounds(L"Key not found");
4081
}
4182

42-
uint32_t FileTypeChoicesMap::Size() const
83+
uint32_t FileTypeChoicesMap::Size() const noexcept
4384
{
44-
return m_innerMap.Size();
85+
return static_cast<uint32_t>(m_orderedMap->size());
4586
}
4687

47-
bool FileTypeChoicesMap::HasKey(hstring const& key) const
88+
bool FileTypeChoicesMap::HasKey(hstring const& key) const noexcept
4889
{
49-
return m_innerMap.HasKey(key);
90+
return FindKey(key) != m_orderedMap->cend();
5091
}
5192

5293
winrt::Windows::Foundation::Collections::IMapView<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>> FileTypeChoicesMap::GetView() const
5394
{
54-
return m_innerMap.GetView();
95+
return make<OrderedMapView>(m_orderedMap);
5596
}
5697

5798
void FileTypeChoicesMap::Remove(hstring const& key)
5899
{
59-
m_innerMap.Remove(key);
100+
auto it{ FindKey(key) };
101+
if (it != m_orderedMap->end())
102+
{
103+
// Remove from vector
104+
m_orderedMap->erase(it);
105+
}
60106
}
61107

62108
void FileTypeChoicesMap::Clear()
63109
{
64-
m_innerMap.Clear();
110+
m_orderedMap->clear();
65111
}
66112

67113
winrt::Windows::Foundation::Collections::IIterator<winrt::Windows::Foundation::Collections::IKeyValuePair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>> FileTypeChoicesMap::First() const
68114
{
69-
return m_innerMap.First();
115+
return make<OrderedMapIterator>(m_orderedMap);
116+
}
117+
118+
// OrderedMapIterator implementation
119+
OrderedMapIterator::OrderedMapIterator(std::shared_ptr<FileTypeChoiceVector const> const& map)
120+
: m_map(map), m_current(0)
121+
{
122+
}
123+
124+
winrt::Windows::Foundation::Collections::IKeyValuePair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>> OrderedMapIterator::Current() const
125+
{
126+
if (!m_map || m_current >= m_map->size())
127+
{
128+
throw winrt::hresult_out_of_bounds();
129+
}
130+
131+
// Create a simple key-value pair
132+
auto tempMap{ single_threaded_map<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>() };
133+
tempMap.Insert((*m_map)[m_current].first, (*m_map)[m_current].second);
134+
return tempMap.First().Current();
135+
}
136+
137+
bool OrderedMapIterator::HasCurrent() const noexcept
138+
{
139+
return m_map && m_current < m_map->size();
140+
}
141+
142+
bool OrderedMapIterator::MoveNext() noexcept
143+
{
144+
if (m_map && m_current < m_map->size())
145+
{
146+
++m_current;
147+
return m_current < m_map->size();
148+
}
149+
return false;
150+
}
151+
152+
uint32_t OrderedMapIterator::GetMany(array_view<winrt::Windows::Foundation::Collections::IKeyValuePair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>> items)
153+
{
154+
uint32_t copied{};
155+
while (copied < items.size() && HasCurrent())
156+
{
157+
items[copied] = Current();
158+
MoveNext();
159+
++copied;
160+
}
161+
return copied;
162+
}
163+
164+
// OrderedMapView implementation
165+
OrderedMapView::OrderedMapView(std::shared_ptr<FileTypeChoiceVector const> const& map)
166+
: m_map(map)
167+
{
168+
}
169+
170+
FileTypeChoiceVector::const_iterator OrderedMapView::FindKey(hstring const& key) const noexcept
171+
{
172+
if (!m_map)
173+
{
174+
return FileTypeChoiceVector::const_iterator{};
175+
}
176+
return std::ranges::find(
177+
m_map->cbegin(),
178+
m_map->cend(),
179+
key,
180+
&FileTypeChoiceVector::value_type::first);
181+
}
182+
183+
winrt::Windows::Foundation::Collections::IVector<hstring> OrderedMapView::Lookup(hstring const& key) const
184+
{
185+
if (!m_map)
186+
{
187+
throw winrt::hresult_error(E_ILLEGAL_METHOD_CALL, L"Map view is no longer valid");
188+
}
189+
190+
auto it{ FindKey(key) };
191+
if (it != m_map->end())
192+
{
193+
return it->second;
194+
}
195+
throw winrt::hresult_out_of_bounds(L"Key not found");
196+
}
197+
198+
uint32_t OrderedMapView::Size() const
199+
{
200+
return m_map ? static_cast<uint32_t>(m_map->size()) : 0;
201+
}
202+
203+
bool OrderedMapView::HasKey(hstring const& key) const noexcept
204+
{
205+
if (!m_map)
206+
{
207+
return false;
208+
}
209+
return FindKey(key) != m_map->end();
210+
}
211+
212+
void OrderedMapView::Split(winrt::Windows::Foundation::Collections::IMapView<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>& first,
213+
winrt::Windows::Foundation::Collections::IMapView<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>& second) const noexcept
214+
{
215+
// For simplicity, not implementing split
216+
first = nullptr;
217+
second = nullptr;
218+
}
219+
220+
winrt::Windows::Foundation::Collections::IIterator<winrt::Windows::Foundation::Collections::IKeyValuePair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>> OrderedMapView::First() const
221+
{
222+
return make<OrderedMapIterator>(m_map);
70223
}
71224
}

‎dev/Interop/StoragePickers/FileTypeChoicesMap.h‎

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,19 @@
33

44
#pragma once
55
#include <winrt/Windows.Foundation.Collections.h>
6+
#include <vector>
7+
#include <utility>
8+
#include <memory>
69
#include "FileTypeFilterVector.h"
710

811
namespace winrt::Microsoft::Windows::Storage::Pickers::implementation
912
{
13+
// Forward declarations
14+
struct OrderedMapView;
15+
struct OrderedMapIterator;
16+
17+
using FileTypeChoiceVector = std::vector<std::pair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>>;
18+
1019
struct FileTypeChoicesMap : implements<FileTypeChoicesMap,
1120
winrt::Windows::Foundation::Collections::IMap<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>,
1221
winrt::Windows::Foundation::Collections::IIterable<winrt::Windows::Foundation::Collections::IKeyValuePair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>>>
@@ -17,8 +26,8 @@ namespace winrt::Microsoft::Windows::Storage::Pickers::implementation
1726

1827
// IMap<hstring, IVector<hstring>>
1928
winrt::Windows::Foundation::Collections::IVector<hstring> Lookup(hstring const& key) const;
20-
uint32_t Size() const;
21-
bool HasKey(hstring const& key) const;
29+
uint32_t Size() const noexcept;
30+
bool HasKey(hstring const& key) const noexcept;
2231
winrt::Windows::Foundation::Collections::IMapView<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>> GetView() const;
2332
bool Insert(hstring const& key, winrt::Windows::Foundation::Collections::IVector<hstring> const& value);
2433
void Remove(hstring const& key);
@@ -28,7 +37,49 @@ namespace winrt::Microsoft::Windows::Storage::Pickers::implementation
2837
winrt::Windows::Foundation::Collections::IIterator<winrt::Windows::Foundation::Collections::IKeyValuePair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>> First() const;
2938

3039
private:
31-
winrt::Windows::Foundation::Collections::IMap<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>> m_innerMap{
32-
single_threaded_map<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>() };
40+
// Shared ownership keeps insertion-ordered entries alive for any outstanding views/iterators.
41+
std::shared_ptr<FileTypeChoiceVector> m_orderedMap;
42+
43+
// Helper methods
44+
FileTypeChoiceVector::const_iterator FindKey(hstring const& key) const noexcept;
45+
FileTypeChoiceVector::iterator FindKey(hstring const& key) noexcept;
46+
};
47+
48+
// Custom iterator to maintain insertion order
49+
struct OrderedMapIterator : implements<OrderedMapIterator,
50+
winrt::Windows::Foundation::Collections::IIterator<winrt::Windows::Foundation::Collections::IKeyValuePair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>>>
51+
{
52+
OrderedMapIterator(std::shared_ptr<FileTypeChoiceVector const> const& map);
53+
54+
winrt::Windows::Foundation::Collections::IKeyValuePair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>> Current() const;
55+
bool HasCurrent() const noexcept;
56+
bool MoveNext() noexcept;
57+
uint32_t GetMany(array_view<winrt::Windows::Foundation::Collections::IKeyValuePair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>> items);
58+
59+
private:
60+
std::shared_ptr<FileTypeChoiceVector const> m_map;
61+
size_t m_current{};
62+
};
63+
64+
// Custom view to maintain insertion order
65+
struct OrderedMapView : implements<OrderedMapView,
66+
winrt::Windows::Foundation::Collections::IMapView<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>,
67+
winrt::Windows::Foundation::Collections::IIterable<winrt::Windows::Foundation::Collections::IKeyValuePair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>>>
68+
{
69+
OrderedMapView(std::shared_ptr<FileTypeChoiceVector const> const& map);
70+
71+
// IMapView
72+
winrt::Windows::Foundation::Collections::IVector<hstring> Lookup(hstring const& key) const;
73+
uint32_t Size() const;
74+
bool HasKey(hstring const& key) const noexcept;
75+
void Split(winrt::Windows::Foundation::Collections::IMapView<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>& first,
76+
winrt::Windows::Foundation::Collections::IMapView<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>& second) const noexcept;
77+
78+
// IIterable
79+
winrt::Windows::Foundation::Collections::IIterator<winrt::Windows::Foundation::Collections::IKeyValuePair<hstring, winrt::Windows::Foundation::Collections::IVector<hstring>>> First() const;
80+
81+
private:
82+
std::shared_ptr<FileTypeChoiceVector const> m_map;
83+
FileTypeChoiceVector::const_iterator FindKey(hstring const& key) const noexcept;
3384
};
3485
}

0 commit comments

Comments
 (0)