Make all windows remember their preferences

Hook into DIALOG_SHIM to remember stylistic preferences for windows such
as position, size, sash position, notebook selection, etc.

Dialogs and controls can opt out of this by setting a client data user
property "persist" to false.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/19992

Fixes https://gitlab.com/kicad/code/kicad/-/issues/10756

Fixes https://gitlab.com/kicad/code/kicad/-/issues/21108
This commit is contained in:
Seth Hillbrand 2025-08-03 20:10:44 -07:00
parent 11d0bb466b
commit bf16dcfbe8
6 changed files with 770 additions and 146 deletions

View File

@ -29,6 +29,9 @@
#include <kiway_player.h>
#include <kiway.h>
#include <pgm_base.h>
#include <project/project_local_settings.h>
#include <property_holder.h>
#include <settings/settings_manager.h>
#include <tool/tool_manager.h>
#include <kiplatform/ui.h>
@ -40,8 +43,19 @@
#include <wx/bmpbuttn.h>
#include <wx/textctrl.h>
#include <wx/stc/stc.h>
#include <wx/combobox.h>
#include <wx/choice.h>
#include <wx/checkbox.h>
#include <wx/spinctrl.h>
#include <wx/splitter.h>
#include <wx/radiobox.h>
#include <wx/radiobut.h>
#include <wx/variant.h>
#include <algorithm>
#include <functional>
#include <nlohmann/json.hpp>
#include <typeinfo>
BEGIN_EVENT_TABLE( DIALOG_SHIM, wxDialog )
EVT_CHAR_HOOK( DIALOG_SHIM::OnCharHook )
@ -106,6 +120,7 @@ DIALOG_SHIM::DIALOG_SHIM( wxWindow* aParent, wxWindowID id, const wxString& titl
Bind( wxEVT_BUTTON, &DIALOG_SHIM::OnButton, this );
Bind( wxEVT_SIZE, &DIALOG_SHIM::OnSize, this );
Bind( wxEVT_MOVE, &DIALOG_SHIM::OnMove, this );
Bind( wxEVT_INIT_DIALOG, &DIALOG_SHIM::onInitDialog, this );
#ifdef __WINDOWS__
// On Windows, the app top windows can be brought to the foreground (at least temporarily)
@ -126,11 +141,14 @@ DIALOG_SHIM::~DIALOG_SHIM()
{
m_isClosing = true;
SaveControlState();
Unbind( wxEVT_CLOSE_WINDOW, &DIALOG_SHIM::OnCloseWindow, this );
Unbind( wxEVT_BUTTON, &DIALOG_SHIM::OnButton, this );
Unbind( wxEVT_PAINT, &DIALOG_SHIM::OnPaint, this );
Unbind( wxEVT_SIZE, &DIALOG_SHIM::OnSize, this );
Unbind( wxEVT_MOVE, &DIALOG_SHIM::OnMove, this );
Unbind( wxEVT_INIT_DIALOG, &DIALOG_SHIM::onInitDialog, this );
std::function<void( wxWindowList& )> disconnectFocusHandlers =
[&]( wxWindowList& children )
@ -169,6 +187,13 @@ DIALOG_SHIM::~DIALOG_SHIM()
}
void DIALOG_SHIM::onInitDialog( wxInitDialogEvent& aEvent )
{
LoadControlState();
aEvent.Skip();
}
void DIALOG_SHIM::finishDialogSettings()
{
// must be called from the constructor of derived classes,
@ -205,55 +230,18 @@ int DIALOG_SHIM::vertPixelsFromDU( int y ) const
#include <hashtables.h>
#include <typeinfo>
static std::unordered_map<std::string, wxRect> class_map;
void DIALOG_SHIM::SetPosition( const wxPoint& aNewPosition )
{
wxDialog::SetPosition( aNewPosition );
// Now update the stored position:
const char* hash_key;
if( m_hash_key.size() )
{
// a special case like EDA_LIST_DIALOG, which has multiple uses.
hash_key = m_hash_key.c_str();
}
else
{
hash_key = typeid(*this).name();
}
std::unordered_map<std::string, wxRect>::iterator it = class_map.find( hash_key );
if( it == class_map.end() )
return;
wxRect rect = it->second;
rect.SetPosition( aNewPosition );
class_map[ hash_key ] = rect;
}
bool DIALOG_SHIM::Show( bool show )
{
bool ret;
const char* hash_key;
bool ret;
if( m_hash_key.size() )
{
// a special case like EDA_LIST_DIALOG, which has multiple uses.
hash_key = m_hash_key.c_str();
}
else
{
hash_key = typeid(*this).name();
}
// Show or hide the window. If hiding, save current position and size.
// If showing, use previous position and size.
if( show )
{
#ifndef __WINDOWS__
@ -261,8 +249,23 @@ bool DIALOG_SHIM::Show( bool show )
#endif
ret = wxDialog::Show( show );
// classname is key, returns a zeroed-out default wxRect if none existed before.
wxRect savedDialogRect = class_map[ hash_key ];
wxRect savedDialogRect;
COMMON_SETTINGS* settings = Pgm().GetSettingsManager().GetCommonSettings();
std::string key = m_hash_key.empty() ? GetTitle().ToStdString() : m_hash_key;
auto dlgIt = settings->m_dialogControlValues.find( key );
if( dlgIt != settings->m_dialogControlValues.end() )
{
auto geoIt = dlgIt->second.find( "__geometry" );
if( geoIt != dlgIt->second.end() && geoIt->second.is_object() )
{
const nlohmann::json& g = geoIt->second;
savedDialogRect.SetPosition( wxPoint( g.value( "x", 0 ), g.value( "y", 0 ) ) );
savedDialogRect.SetSize( wxSize( g.value( "w", 500 ), g.value( "h", 300 ) ) );
}
}
if( savedDialogRect.GetSize().x != 0 && savedDialogRect.GetSize().y != 0 )
{
@ -294,9 +297,6 @@ bool DIALOG_SHIM::Show( bool show )
Centre();
}
// Be sure that the dialog appears in a visible area
// (the dialog position might have been stored at the time when it was
// shown on another display)
if( wxDisplay::GetFromWindow( this ) == wxNOT_FOUND )
Centre();
@ -305,18 +305,6 @@ bool DIALOG_SHIM::Show( bool show )
}
else
{
// Save the dialog's position & size before hiding, using classname as key.
// Be careful of rounding errors: only re-save if the user modified the value or
// it has not yet been saved.
wxRect rect = class_map[ hash_key ];
if( m_userPositioned || rect.GetPosition() == wxPoint() )
rect.SetPosition( wxDialog::GetPosition() );
if( m_userResized || rect.GetSize() == wxSize() )
rect.SetSize( wxDialog::GetSize() );
class_map[ hash_key ] = rect;
#ifdef __WXMAC__
if ( m_eventLoop )
@ -335,26 +323,15 @@ bool DIALOG_SHIM::Show( bool show )
void DIALOG_SHIM::resetSize()
{
const char* hash_key;
COMMON_SETTINGS* settings = Pgm().GetSettingsManager().GetCommonSettings();
std::string key = m_hash_key.empty() ? GetTitle().ToStdString() : m_hash_key;
if( m_hash_key.size() )
{
// a special case like EDA_LIST_DIALOG, which has multiple uses.
hash_key = m_hash_key.c_str();
}
else
{
hash_key = typeid(*this).name();
}
auto dlgIt = settings->m_dialogControlValues.find( key );
std::unordered_map<std::string, wxRect>::iterator it = class_map.find( hash_key );
if( it == class_map.end() )
if( dlgIt == settings->m_dialogControlValues.end() )
return;
wxRect rect = it->second;
rect.SetSize( wxSize( 0, 0 ) );
class_map[ hash_key ] = rect;
dlgIt->second.erase( "__geometry" );
}
@ -379,6 +356,196 @@ bool DIALOG_SHIM::Enable( bool enable )
}
std::string DIALOG_SHIM::generateKey( const wxWindow* aWin ) const
{
wxString className = aWin->GetClassInfo()->GetClassName();
int siblingIndex = 0;
for( const wxWindow* parent = aWin->GetParent(); parent && parent != this; parent = parent->GetParent() )
{
wxString parentClassName = parent->GetClassInfo()->GetClassName();
if( !parent->GetParent() )
break;
for( auto& sibling : parent->GetParent()->GetChildren() )
{
if( sibling->GetClassInfo()->GetClassName() != parentClassName )
continue;
if( sibling == parent )
break;
siblingIndex++;
}
className = parentClassName + "_" + std::to_string( siblingIndex ) + ":" + className;
}
siblingIndex = 0;
if( aWin->GetParent() )
{
for( auto& sibling : aWin->GetParent()->GetChildren() )
{
if( sibling->GetClassInfo()->GetClassName() != className )
continue;
if( sibling == aWin )
break;
siblingIndex++;
}
}
std::string key = className.ToStdString() + "_" + std::to_string( siblingIndex );
return key;
}
void DIALOG_SHIM::SaveControlState()
{
COMMON_SETTINGS* settings = Pgm().GetSettingsManager().GetCommonSettings();
std::string dialogKey = m_hash_key.empty() ? GetTitle().ToStdString() : m_hash_key;
auto& dlgMap = settings->m_dialogControlValues[ dialogKey ];
wxRect rect( GetPosition(), GetSize() );
nlohmann::json geom;
geom[ "x" ] = rect.GetX();
geom[ "y" ] = rect.GetY();
geom[ "w" ] = rect.GetWidth();
geom[ "h" ] = rect.GetHeight();
dlgMap[ "__geometry" ] = geom;
std::function<void( wxWindow* )> saveFn = [&]( wxWindow* win )
{
if( PROPERTY_HOLDER* props = PROPERTY_HOLDER::SafeCast( win->GetClientData() ) )
{
if( !props->GetPropertyOr( "persist", false ) )
return;
}
std::string key = generateKey( win );
if( !key.empty() )
{
if( wxComboBox* combo = dynamic_cast<wxComboBox*>( win ) )
dlgMap[ key ] = combo->GetSelection();
else if( wxChoice* choice = dynamic_cast<wxChoice*>( win ) )
dlgMap[ key ] = choice->GetSelection();
else if( wxCheckBox* check = dynamic_cast<wxCheckBox*>( win ) )
dlgMap[ key ] = check->GetValue();
else if( wxSpinCtrl* spin = dynamic_cast<wxSpinCtrl*>( win ) )
dlgMap[ key ] = spin->GetValue();
else if( wxRadioButton* radio = dynamic_cast<wxRadioButton*>( win ) )
dlgMap[ key ] = radio->GetValue();
else if( wxRadioBox* radioBox = dynamic_cast<wxRadioBox*>( win ) )
dlgMap[ key ] = radioBox->GetSelection();
else if( wxSplitterWindow* splitter = dynamic_cast<wxSplitterWindow*>( win ) )
dlgMap[ key ] = splitter->GetSashPosition();
else if( wxScrolledWindow* scrolled = dynamic_cast<wxScrolledWindow*>( win ) )
dlgMap[ key ] = scrolled->GetScrollPos( wxVERTICAL );
else if( wxNotebook* notebook = dynamic_cast<wxNotebook*>( win ) )
dlgMap[ key ] = notebook->GetSelection();
}
for( wxWindow* child : win->GetChildren() )
saveFn( child );
};
for( wxWindow* child : GetChildren() )
saveFn( child );
}
void DIALOG_SHIM::LoadControlState()
{
COMMON_SETTINGS* settings = Pgm().GetSettingsManager().GetCommonSettings();
std::string dialogKey = m_hash_key.empty() ? GetTitle().ToStdString() : m_hash_key;
auto dlgIt = settings->m_dialogControlValues.find( dialogKey );
if( dlgIt == settings->m_dialogControlValues.end() )
return;
const auto& dlgMap = dlgIt->second;
std::function<void( wxWindow* )> loadFn = [&]( wxWindow* win )
{
if( PROPERTY_HOLDER* props = PROPERTY_HOLDER::SafeCast( win->GetClientData() ) )
{
if( !props->GetPropertyOr( "persist", false ) )
return;
}
std::string key = generateKey( win );
if( !key.empty() )
{
auto it = dlgMap.find( key );
if( it != dlgMap.end() )
{
const nlohmann::json& j = it->second;
if( wxComboBox* combo = dynamic_cast<wxComboBox*>( win ) )
{
if( j.is_string() )
combo->SetValue( wxString::FromUTF8( j.get<std::string>().c_str() ) );
}
else if( wxChoice* choice = dynamic_cast<wxChoice*>( win ) )
{
if( j.is_number_integer() )
choice->SetSelection( j.get<int>() );
}
else if( wxCheckBox* check = dynamic_cast<wxCheckBox*>( win ) )
{
if( j.is_boolean() )
check->SetValue( j.get<bool>() );
}
else if( wxSpinCtrl* spin = dynamic_cast<wxSpinCtrl*>( win ) )
{
if( j.is_number_integer() )
spin->SetValue( j.get<int>() );
}
else if( wxRadioButton* radio = dynamic_cast<wxRadioButton*>( win ) )
{
if( j.is_boolean() )
radio->SetValue( j.get<bool>() );
}
else if( wxRadioBox* radioBox = dynamic_cast<wxRadioBox*>( win ) )
{
if( j.is_number_integer() )
radioBox->SetSelection( j.get<int>() );
}
else if( wxSplitterWindow* splitter = dynamic_cast<wxSplitterWindow*>( win ) )
{
if( j.is_number_integer() )
splitter->SetSashPosition( j.get<int>() );
}
else if( wxScrolledWindow* scrolled = dynamic_cast<wxScrolledWindow*>( win ) )
{
if( j.is_number_integer() )
scrolled->SetScrollPos( wxVERTICAL, j.get<int>() );
}
else if( wxNotebook* notebook = dynamic_cast<wxNotebook*>( win ) )
{
if( j.is_number_integer() )
notebook->SetSelection( j.get<int>() );
}
}
}
for( wxWindow* child : win->GetChildren() )
loadFn( child );
};
for( wxWindow* child : GetChildren() )
loadFn( child );
}
// Recursive descent doing a SelectAll() in wxTextCtrls.
// MacOS User Interface Guidelines state that when tabbing to a text control all its
// text should be selected. Since wxWidgets fails to implement this, we do it here.

View File

@ -435,6 +435,35 @@ COMMON_SETTINGS::COMMON_SETTINGS() :
m_params.emplace_back( new PARAM<bool>( "api.enable_server",
&m_Api.enable_server, false ) );
m_params.emplace_back( new PARAM_LAMBDA<nlohmann::json>( "dialog.controls",
[&]() -> nlohmann::json
{
nlohmann::json ret = nlohmann::json::object();
for( const auto& dlg : m_dialogControlValues )
ret[ dlg.first ] = dlg.second;
return ret;
},
[&]( const nlohmann::json& aVal )
{
m_dialogControlValues.clear();
if( !aVal.is_object() )
return;
for( auto& [dlgKey, dlgVal] : aVal.items() )
{
if( !dlgVal.is_object() )
continue;
for( auto& [ctrlKey, ctrlVal] : dlgVal.items() )
m_dialogControlValues[ dlgKey ][ ctrlKey ] = ctrlVal;
}
},
nlohmann::json::object() ) );
registerMigration( 0, 1, std::bind( &COMMON_SETTINGS::migrateSchema0to1, this ) );
registerMigration( 1, 2, std::bind( &COMMON_SETTINGS::migrateSchema1to2, this ) );
registerMigration( 2, 3, std::bind( &COMMON_SETTINGS::migrateSchema2to3, this ) );

View File

@ -36,13 +36,21 @@ class EDA_BASE_FRAME;
class wxGridEvent;
class wxGUIEventLoop;
class wxInitDialogEvent;
/**
* Dialog helper object to sit in the inheritance tree between wxDialog and any class written
* by wxFormBuilder.
*
* To put it there, use wxFormBuilder tool and set:
* In addition to common dialog utilities, DIALOG_SHIM will persist the state of
* all child controls and the dialog geometry in the current project's local
* settings. The dialog's type name is used as the default key, but dialogs may
* override @ref m_hash_key to store settings under a custom name. Any control
* can opt out of persistence by setting the boolean window property "persist" to
* false. Geometry is stored under the special key "__geometry".
*
* To use it in wxFormBuilder set:
* <br> subclass name = DIALOG_SHIM
* <br> subclass header = dialog_shim.h
* <br>
@ -120,6 +128,18 @@ public:
e.ShiftDown() && !e.MetaDown();
}
/**
* Load persisted control values from the current project's local settings.
* Controls may opt out by setting the boolean window property "persist" to
* false. Dialog geometry is stored under the special key "__geometry".
*/
void LoadControlState();
/**
* Save control values and geometry to the current project's local settings.
*/
void SaveControlState();
protected:
/**
* In all dialogs, we must call the same functions to fix minimal dlg size, the default
@ -190,11 +210,15 @@ private:
void onChildSetFocus( wxFocusEvent& aEvent );
void onInitDialog( wxInitDialogEvent& aEvent );
std::string generateKey( const wxWindow* aWin ) const;
DECLARE_EVENT_TABLE();
protected:
EDA_UNITS m_units; // userUnits for display and parsing
std::string m_hash_key; // alternate for class_map when classname re-used
std::string m_hash_key; // optional custom key for persistence
// The following disables the storing of a user size. It is used primarily for dialogs
// with conditional content which don't need user sizing.

View File

@ -78,16 +78,165 @@ concept PropertyValueType = std::copy_constructible<T> && std::destructible<T>;
class PROPERTY_HOLDER
{
public:
/// Magic value for memory validation (ASCII: "PROP" + "HLDR")
static constexpr uint64_t MAGIC_VALUE = 0x50524F5048444C52ULL;
/**
* @brief Default constructor - initializes magic value
*/
PROPERTY_HOLDER() :
m_magic( MAGIC_VALUE )
{
}
/**
* @brief Copy constructor - maintains magic value
*/
PROPERTY_HOLDER( const PROPERTY_HOLDER& other ) :
m_magic( MAGIC_VALUE ),
m_properties( other.m_properties )
{
}
/**
* @brief Move constructor - maintains magic value
*/
PROPERTY_HOLDER( PROPERTY_HOLDER&& other ) noexcept :
m_magic( MAGIC_VALUE ),
m_properties( std::move( other.m_properties ) )
{
}
/**
* @brief Copy assignment operator
*/
PROPERTY_HOLDER& operator=( const PROPERTY_HOLDER& other )
{
if( this != &other )
m_properties = other.m_properties;
return *this;
}
/**
* @brief Move assignment operator
*/
PROPERTY_HOLDER& operator=( PROPERTY_HOLDER&& other ) noexcept
{
if( this != &other )
m_properties = std::move( other.m_properties );
return *this;
}
/**
* @brief Destructor - clears magic value to detect use-after-free
*/
~PROPERTY_HOLDER()
{
m_magic = 0xDEADBEEFDEADBEEFULL; // Clear magic to detect use-after-free
}
/**
* @brief Check if this instance has a valid magic value
* @return true if magic value is valid, false otherwise
*/
bool IsValid() const noexcept { return m_magic == MAGIC_VALUE; }
/**
* @brief Safely cast a void pointer to PROPERTY_HOLDER*
* @param ptr Pointer to validate and cast
* @return PROPERTY_HOLDER* if valid, nullptr otherwise
*/
static PROPERTY_HOLDER* SafeCast( void* aPtr ) noexcept
{
if( !aPtr )
return nullptr;
try
{
PROPERTY_HOLDER* aCandidate = reinterpret_cast<PROPERTY_HOLDER*>( aPtr );
if( aCandidate->m_magic == MAGIC_VALUE )
{
return aCandidate;
}
}
catch( ... )
{
// Any exception means invalid memory
}
return nullptr;
}
/**
* @brief Safely cast a const void pointer to const PROPERTY_HOLDER*
* @param ptr Pointer to validate and cast
* @return const PROPERTY_HOLDER* if valid, nullptr otherwise
*/
static const PROPERTY_HOLDER* SafeCast( const void* aPtr ) noexcept
{
if( !aPtr )
return nullptr;
try
{
const PROPERTY_HOLDER* aCandidate = reinterpret_cast<const PROPERTY_HOLDER*>( aPtr );
if( aCandidate->m_magic == MAGIC_VALUE )
{
return aCandidate;
}
}
catch( ... )
{
// Any exception means invalid memory
}
return nullptr;
}
/**
* @brief Safely delete a PROPERTY_HOLDER from client data
* @param ptr Pointer from client data to validate and delete
* @return true if successfully deleted, false if invalid pointer
*/
static bool SafeDelete( void* aPtr ) noexcept
{
PROPERTY_HOLDER* aHolder = SafeCast( aPtr );
if( aHolder )
{
delete aHolder;
return true;
}
return false;
}
static bool SafeDelete( PROPERTY_HOLDER* aHolder ) noexcept
{
if( aHolder )
{
delete aHolder;
return true;
}
return false;
}
/**
* @brief Set a property with the given key and value.
* @tparam T The type of the value to store
* @param key The property key
* @param value The value to store
* @return true if property was set, false if object is invalid
*/
template<typename T>
void SetProperty(const std::string& key, T&& value)
template <typename T>
bool SetProperty( const std::string& aKey, T&& aValue )
{
m_properties[key] = std::forward<T>(value);
if( !IsValid() )
return false;
m_properties[aKey] = std::forward<T>( aValue );
return true;
}
/**
@ -96,18 +245,24 @@ public:
* @param key The property key
* @return std::optional<T> containing the value if found and type matches, nullopt otherwise
*/
template<typename T>
std::optional<T> GetProperty(const std::string& key) const
template <typename T>
std::optional<T> GetProperty( const std::string& aKey ) const
{
auto it = m_properties.find(key);
if (it == m_properties.end()) {
if( !IsValid() )
return std::nullopt;
auto it = m_properties.find( aKey );
if( it == m_properties.end() )
{
return std::nullopt;
}
try {
return std::any_cast<T>(it->second);
try
{
return std::any_cast<T>( it->second );
}
catch (const std::bad_any_cast&) {
catch( const std::bad_any_cast& )
{
return std::nullopt;
}
}
@ -120,114 +275,148 @@ public:
* @return The property value or the default value
*/
template<typename T>
T GetPropertyOr(const std::string& key, T&& defaultValue) const
T GetPropertyOr(const std::string& aKey, T&& aDefaultValue) const
{
if (auto value = GetProperty<T>(key)) {
return *value;
}
return std::forward<T>(defaultValue);
if( auto aValue = GetProperty<T>( aKey ) )
return *aValue;
return std::forward<T>(aDefaultValue);
}
/**
* @brief Check if a property exists.
* @param key The property key
* @return true if the property exists, false otherwise
* @return true if the property exists and object is valid, false otherwise
*/
bool HasProperty(const std::string& key) const
bool HasProperty( const std::string& aKey ) const
{
return m_properties.find(key) != m_properties.end();
if( !IsValid() )
return false;
return m_properties.find( aKey ) != m_properties.end();
}
/**
* @brief Remove a property.
* @param key The property key
* @return true if the property was removed, false if it didn't exist
* @return true if the property was removed, false if it didn't exist or object is invalid
*/
bool RemoveProperty(const std::string& key)
bool RemoveProperty( const std::string& aKey )
{
return m_properties.erase(key) > 0;
if( !IsValid() )
return false;
return m_properties.erase( aKey ) > 0;
}
/**
* @brief Clear all properties.
* @return true if cleared successfully, false if object is invalid
*/
void Clear()
bool Clear()
{
if( !IsValid() )
return false;
m_properties.clear();
return true;
}
/**
* @brief Get the number of stored properties.
* @return The number of properties
* @return The number of properties, or 0 if object is invalid
*/
size_t Size() const
{
if( !IsValid() )
return 0;
return m_properties.size();
}
/**
* @brief Check if there are no properties stored.
* @return true if empty, false otherwise
* @return true if empty or object is invalid, false otherwise
*/
bool Empty() const
{
if( !IsValid() )
return true;
return m_properties.empty();
}
/**
* @brief Get all property keys.
* @return A vector of all property keys
* @return A vector of all property keys (empty if object is invalid)
*/
std::vector<std::string> GetKeys() const
{
if( !IsValid() )
return {};
std::vector<std::string> keys;
keys.reserve(m_properties.size());
for (const auto& [key, value] : m_properties) {
keys.push_back(key);
}
keys.reserve( m_properties.size() );
for( const auto& [key, value] : m_properties )
keys.push_back( key );
return keys;
}
/**
* @brief Get the type information for a property.
* @param key The property key
* @return std::optional<std::type_info> containing type info if property exists
* @return std::optional<std::type_info> containing type info if property exists and object is valid
*/
std::optional<std::reference_wrapper<const std::type_info>> GetPropertyType(const std::string& key) const
std::optional<std::reference_wrapper<const std::type_info>> GetPropertyType( const std::string& aKey ) const
{
auto it = m_properties.find(key);
if (it == m_properties.end()) {
if( !IsValid() )
return std::nullopt;
}
return std::cref(it->second.type());
auto it = m_properties.find( aKey );
if( it == m_properties.end() )
return std::nullopt;
return std::cref( it->second.type() );
}
/**
* @brief Check if a property exists and has the expected type.
* @tparam T The expected type
* @param key The property key
* @return true if property exists and has type T
* @return true if property exists, has type T, and object is valid
*/
template<typename T>
bool HasPropertyOfType(const std::string& key) const
template <typename T>
bool HasPropertyOfType( const std::string& aKey ) const
{
auto it = m_properties.find(key);
if (it == m_properties.end()) {
if( !IsValid() )
return false;
}
return it->second.type() == typeid(T);
auto it = m_properties.find( aKey );
if( it == m_properties.end() )
return false;
return it->second.type() == typeid( T );
}
/**
* @brief Type-safe property setter that only accepts valid property types
*/
template<PropertyValueType T>
void SetTypedProperty(const std::string& key, T&& value)
bool SetTypedProperty(const std::string& aKey, T&& aValue)
{
SetProperty(key, std::forward<T>(value));
return SetProperty(aKey, std::forward<T>(aValue));
}
private:
uint64_t m_magic; ///< Magic value for memory validation
/**
* @brief Internal storage for properties using string keys and any values.
*
* This uses std::any to allow storing any type of value, with type safety
* provided by the GetProperty and SetProperty methods.
*/
std::unordered_map<std::string, std::any> m_properties;
};
@ -261,37 +450,37 @@ public:
// Convenience methods that delegate to the property holder
template<typename T>
void SetProperty(const std::string& key, T&& value)
void SetProperty(const std::string& aKey, T&& aValue)
{
m_propertyHolder.SetProperty(key, std::forward<T>(value));
m_propertyHolder.SetProperty(aKey, std::forward<T>(aValue));
}
template<typename T>
std::optional<T> GetProperty(const std::string& key) const
std::optional<T> GetProperty(const std::string& aKey) const
{
return m_propertyHolder.GetProperty<T>(key);
return m_propertyHolder.GetProperty<T>(aKey);
}
template<typename T>
T GetPropertyOr(const std::string& key, T&& defaultValue) const
T GetPropertyOr(const std::string& aKey, T&& aDefaultValue) const
{
return m_propertyHolder.GetPropertyOr(key, std::forward<T>(defaultValue));
return m_propertyHolder.GetPropertyOr(aKey, std::forward<T>(aDefaultValue));
}
bool HasProperty(const std::string& key) const
bool HasProperty(const std::string& aKey) const
{
return m_propertyHolder.HasProperty(key);
return m_propertyHolder.HasProperty(aKey);
}
bool RemoveProperty(const std::string& key)
bool RemoveProperty(const std::string& aKey)
{
return m_propertyHolder.RemoveProperty(key);
return m_propertyHolder.RemoveProperty(aKey);
}
template<typename T>
bool HasPropertyOfType(const std::string& key) const
bool HasPropertyOfType(const std::string& aKey) const
{
return m_propertyHolder.HasPropertyOfType<T>(key);
return m_propertyHolder.HasPropertyOfType<T>(aKey);
}
private:

View File

@ -233,6 +233,9 @@ public:
GIT m_Git;
API m_Api;
/// Persistent dialog control values
std::map<std::string, std::map<std::string, nlohmann::json>> m_dialogControlValues;
};
#endif

View File

@ -200,9 +200,9 @@ BOOST_AUTO_TEST_CASE( ExistenceChecking )
BOOST_CHECK_EQUAL( false, m_emptyHolder.HasProperty( "any_key" ) );
// Populated holder
BOOST_CHECK_EQUAL( true, m_populatedHolder.HasProperty( "bool_prop" ) );
BOOST_CHECK_EQUAL( true, m_populatedHolder.HasProperty( "int_prop" ) );
BOOST_CHECK_EQUAL( true, m_populatedHolder.HasProperty( "string_prop" ) );
BOOST_CHECK( m_populatedHolder.HasProperty( "bool_prop" ) );
BOOST_CHECK( m_populatedHolder.HasProperty( "int_prop" ) );
BOOST_CHECK( m_populatedHolder.HasProperty( "string_prop" ) );
BOOST_CHECK_EQUAL( false, m_populatedHolder.HasProperty( "missing_prop" ) );
}
@ -212,10 +212,10 @@ BOOST_AUTO_TEST_CASE( ExistenceChecking )
BOOST_AUTO_TEST_CASE( TypeSpecificExistence )
{
// Check correct type detection
BOOST_CHECK_EQUAL( true, m_populatedHolder.HasPropertyOfType<bool>( "bool_prop" ) );
BOOST_CHECK_EQUAL( true, m_populatedHolder.HasPropertyOfType<int>( "int_prop" ) );
BOOST_CHECK_EQUAL( true, m_populatedHolder.HasPropertyOfType<double>( "double_prop" ) );
BOOST_CHECK_EQUAL( true, m_populatedHolder.HasPropertyOfType<std::string>( "string_prop" ) );
BOOST_CHECK( m_populatedHolder.HasPropertyOfType<bool>( "bool_prop" ) );
BOOST_CHECK( m_populatedHolder.HasPropertyOfType<int>( "int_prop" ) );
BOOST_CHECK( m_populatedHolder.HasPropertyOfType<double>( "double_prop" ) );
BOOST_CHECK( m_populatedHolder.HasPropertyOfType<std::string>( "string_prop" ) );
// Check wrong type detection
BOOST_CHECK_EQUAL( false, m_populatedHolder.HasPropertyOfType<int>( "bool_prop" ) );
@ -235,12 +235,12 @@ BOOST_AUTO_TEST_CASE( PropertyRemoval )
holder.SetProperty( "temp_prop", 42 );
// Property should exist
BOOST_CHECK_EQUAL( true, holder.HasProperty( "temp_prop" ) );
BOOST_CHECK( holder.HasProperty( "temp_prop" ) );
BOOST_CHECK_EQUAL( 1, holder.Size() );
// Remove existing property
bool removed = holder.RemoveProperty( "temp_prop" );
BOOST_CHECK_EQUAL( true, removed );
BOOST_CHECK( removed );
BOOST_CHECK_EQUAL( false, holder.HasProperty( "temp_prop" ) );
BOOST_CHECK_EQUAL( 0, holder.Size() );
@ -265,7 +265,7 @@ BOOST_AUTO_TEST_CASE( ClearProperties )
holder.Clear();
BOOST_CHECK_EQUAL( 0, holder.Size() );
BOOST_CHECK_EQUAL( true, holder.Empty() );
BOOST_CHECK( holder.Empty() );
BOOST_CHECK_EQUAL( false, holder.HasProperty( "prop1" ) );
BOOST_CHECK_EQUAL( false, holder.HasProperty( "prop2" ) );
BOOST_CHECK_EQUAL( false, holder.HasProperty( "prop3" ) );
@ -278,7 +278,7 @@ BOOST_AUTO_TEST_CASE( SizeAndEmpty )
{
// Empty holder
BOOST_CHECK_EQUAL( 0, m_emptyHolder.Size() );
BOOST_CHECK_EQUAL( true, m_emptyHolder.Empty() );
BOOST_CHECK( m_emptyHolder.Empty() );
// Populated holder (from fixture: 6 properties)
BOOST_CHECK_EQUAL( 7, m_populatedHolder.Size() );
@ -426,9 +426,9 @@ BOOST_AUTO_TEST_CASE( BasicMixin )
BOOST_CHECK_EQUAL( widget.GetPropertyOr( "runtime_prop", 0 ), 42 );
// Should be able to check existence
BOOST_CHECK_EQUAL( true, widget.HasProperty( "widget_name" ) );
BOOST_CHECK_EQUAL( true, widget.HasProperty( "default_enabled" ) );
BOOST_CHECK_EQUAL( true, widget.HasProperty( "runtime_prop" ) );
BOOST_CHECK( widget.HasProperty( "widget_name" ) );
BOOST_CHECK( widget.HasProperty( "default_enabled" ) );
BOOST_CHECK( widget.HasProperty( "runtime_prop" ) );
BOOST_CHECK_EQUAL( false, widget.HasProperty( "missing_prop" ) );
}
@ -471,7 +471,7 @@ BOOST_AUTO_TEST_CASE( PropertyHolderAccess )
BOOST_CHECK_EQUAL( widget.GetPropertyOr( "direct_access", 0 ), 999 );
// Should be able to use holder methods
BOOST_CHECK_EQUAL( true, constHolder.HasProperty( "widget_name" ) );
BOOST_CHECK( constHolder.HasProperty( "widget_name" ) );
auto keys = constHolder.GetKeys();
BOOST_CHECK( keys.size() >= 2 ); // At least widget_name and default_enabled
}
@ -492,13 +492,13 @@ BOOST_AUTO_TEST_CASE( EmptyKeys )
// Empty key should work (though not recommended)
holder.SetProperty( "", 42 );
BOOST_CHECK_EQUAL( true, holder.HasProperty( "" ) );
BOOST_CHECK( holder.HasProperty( "" ) );
BOOST_CHECK_EQUAL( holder.GetPropertyOr( "", 0 ), 42 );
// Very long key should work
std::string longKey( 1000, 'x' );
holder.SetProperty( longKey, "long key value" );
BOOST_CHECK_EQUAL( true, holder.HasProperty( longKey ) );
BOOST_CHECK( holder.HasProperty( longKey ) );
}
/**
@ -542,4 +542,216 @@ BOOST_AUTO_TEST_CASE( NullScenarios )
BOOST_CHECK_EQUAL( false, holder.RemoveProperty( "non_existing" ) );
}
BOOST_AUTO_TEST_SUITE_END()
/**
* Test suite for magic value validation and memory safety
*/
BOOST_AUTO_TEST_SUITE( PropertyHolderMagicValue )
/**
* Test magic value validation
*/
BOOST_AUTO_TEST_CASE( MagicValueValidation )
{
PROPERTY_HOLDER holder;
// New holder should be valid
BOOST_CHECK( holder.IsValid() );
BOOST_CHECK_EQUAL( PROPERTY_HOLDER::MAGIC_VALUE, 0x50524F5048444C52ULL );
// Should be able to use all functions when valid
BOOST_CHECK( holder.SetProperty( "test", 42 ) );
BOOST_CHECK( holder.HasProperty( "test" ) );
BOOST_CHECK_EQUAL( holder.GetPropertyOr( "test", 0 ), 42 );
}
/**
* Test safe casting functionality
*/
BOOST_AUTO_TEST_CASE( SafeCasting )
{
// Create a PROPERTY_HOLDER
PROPERTY_HOLDER* original = new PROPERTY_HOLDER();
original->SetProperty( "test_prop", 123 );
// Cast to void* (simulating client data storage)
void* clientData = static_cast<void*>( original );
// Safe cast should succeed
PROPERTY_HOLDER* casted = PROPERTY_HOLDER::SafeCast( clientData );
BOOST_CHECK_NE( nullptr, casted );
BOOST_CHECK( casted->IsValid() );
BOOST_CHECK_EQUAL( casted->GetPropertyOr( "test_prop", 0 ), 123 );
// Should be the same object
BOOST_CHECK_EQUAL( original, casted );
// Const version should also work
const void* constClientData = static_cast<const void*>( original );
const PROPERTY_HOLDER* constCasted = PROPERTY_HOLDER::SafeCast( constClientData );
BOOST_CHECK_NE( nullptr, constCasted );
BOOST_CHECK( constCasted->IsValid() );
delete original;
}
/**
* Test safe casting with invalid pointers
*/
BOOST_AUTO_TEST_CASE( SafeCastingInvalid )
{
// Null pointer should return null
BOOST_CHECK_EQUAL( nullptr, PROPERTY_HOLDER::SafeCast( static_cast<void*>( nullptr ) ) );
BOOST_CHECK_EQUAL( nullptr, PROPERTY_HOLDER::SafeCast( static_cast<const void*>( nullptr ) ) );
// Random memory should return null
int randomInt = 12345;
void* randomPtr = &randomInt;
BOOST_CHECK_EQUAL( nullptr, PROPERTY_HOLDER::SafeCast( randomPtr ) );
// Memory with wrong magic value should return null
struct FakeHolder {
uint64_t wrong_magic = 0x1234567890ABCDEFULL;
int some_data = 42;
};
FakeHolder fake;
void* fakePtr = &fake;
BOOST_CHECK_EQUAL( nullptr, PROPERTY_HOLDER::SafeCast( fakePtr ) );
}
/**
* Test use-after-free detection
*/
BOOST_AUTO_TEST_CASE( UseAfterFreeDetection )
{
PROPERTY_HOLDER* holder = new PROPERTY_HOLDER();
holder->SetProperty( "test", 42 );
// Should be valid before deletion
BOOST_CHECK( holder->IsValid() );
void* ptr = static_cast<void*>( holder );
BOOST_CHECK_NE( nullptr, PROPERTY_HOLDER::SafeCast( ptr ) );
// Delete the holder
delete holder;
// Now safe cast should fail (magic value was cleared in destructor)
BOOST_CHECK_EQUAL( nullptr, PROPERTY_HOLDER::SafeCast( ptr ) );
}
/**
* Test client data creation and deletion helpers
*/
BOOST_AUTO_TEST_CASE( ClientDataHelpers )
{
PROPERTY_HOLDER* holder = new PROPERTY_HOLDER();
BOOST_CHECK_NE( nullptr, holder );
BOOST_CHECK( holder->IsValid() );
// Should be able to use it
BOOST_CHECK( holder->SetProperty( "client_test", 999 ) );
BOOST_CHECK_EQUAL( holder->GetPropertyOr( "client_test", 0 ), 999 );
// Safe delete should succeed
BOOST_CHECK( PROPERTY_HOLDER::SafeDelete( reinterpret_cast<void*>( holder ) ) );
// Safe delete of invalid pointer should fail
int randomData = 42;
BOOST_CHECK_EQUAL( false, PROPERTY_HOLDER::SafeDelete( &randomData ) );
BOOST_CHECK_EQUAL( false, PROPERTY_HOLDER::SafeDelete( static_cast<void*>( nullptr ) ) );
}
/**
* Test copy and move constructors preserve magic value
*/
BOOST_AUTO_TEST_CASE( CopyMoveConstructors )
{
PROPERTY_HOLDER original;
original.SetProperty( "test", 42 );
BOOST_CHECK( original.IsValid() );
// Copy constructor
PROPERTY_HOLDER copied( original );
BOOST_CHECK( copied.IsValid() );
BOOST_CHECK_EQUAL( copied.GetPropertyOr( "test", 0 ), 42 );
// Move constructor
PROPERTY_HOLDER moved( std::move( original ) );
BOOST_CHECK( moved.IsValid() );
BOOST_CHECK_EQUAL( moved.GetPropertyOr( "test", 0 ), 42 );
// Original should still be valid (magic value preserved)
BOOST_CHECK( original.IsValid() );
}
/**
* Test assignment operators preserve magic value
*/
BOOST_AUTO_TEST_CASE( AssignmentOperators )
{
PROPERTY_HOLDER source;
source.SetProperty( "source_prop", 123 );
PROPERTY_HOLDER target;
target.SetProperty( "target_prop", 456 );
BOOST_CHECK( source.IsValid() );
BOOST_CHECK( target.IsValid() );
// Copy assignment
target = source;
BOOST_CHECK( target.IsValid() );
BOOST_CHECK_EQUAL( target.GetPropertyOr( "source_prop", 0 ), 123 );
BOOST_CHECK_EQUAL( false, target.HasProperty( "target_prop" ) ); // Should be overwritten
// Move assignment
PROPERTY_HOLDER another;
another.SetProperty( "another_prop", 789 );
target = std::move( another );
BOOST_CHECK( target.IsValid() );
BOOST_CHECK_EQUAL( target.GetPropertyOr( "another_prop", 0 ), 789 );
BOOST_CHECK( another.IsValid() ); // Magic value preserved
}
/**
* Test behavior of methods when object is invalid
*/
BOOST_AUTO_TEST_CASE( InvalidObjectBehavior )
{
// Create a fake PROPERTY_HOLDER with wrong magic value
struct FakePropertyHolder {
uint64_t wrong_magic = 0xDEADBEEF;
std::unordered_map<std::string, std::any> fake_properties;
};
FakePropertyHolder fake;
PROPERTY_HOLDER* fakeHolder = reinterpret_cast<PROPERTY_HOLDER*>( &fake );
// All methods should fail gracefully
BOOST_CHECK_EQUAL( false, fakeHolder->IsValid() );
BOOST_CHECK_EQUAL( false, fakeHolder->SetProperty( "test", 42 ) );
BOOST_CHECK_EQUAL( false, fakeHolder->HasProperty( "anything" ) );
BOOST_CHECK_EQUAL( false, fakeHolder->RemoveProperty( "anything" ) );
BOOST_CHECK_EQUAL( false, fakeHolder->Clear() );
BOOST_CHECK_EQUAL( 0, fakeHolder->Size() );
BOOST_CHECK( fakeHolder->Empty() ); // Returns true for invalid objects
BOOST_CHECK_EQUAL( 0, fakeHolder->GetKeys().size() );
BOOST_CHECK_EQUAL( false, fakeHolder->HasPropertyOfType<int>( "anything" ) );
// GetProperty should return nullopt
auto result = fakeHolder->GetProperty<int>( "anything" );
BOOST_CHECK( !result.has_value() );
// GetPropertyOr should return default
BOOST_CHECK_EQUAL( fakeHolder->GetPropertyOr( "anything", 999 ), 999 );
// GetPropertyType should return nullopt
auto typeInfo = fakeHolder->GetPropertyType( "anything" );
BOOST_CHECK( !typeInfo.has_value() );
}
BOOST_AUTO_TEST_SUITE_END()