diff --git a/common/dialog_shim.cpp b/common/dialog_shim.cpp index c1ca16e500..6bdc8bf77d 100644 --- a/common/dialog_shim.cpp +++ b/common/dialog_shim.cpp @@ -29,6 +29,9 @@ #include #include #include +#include +#include +#include #include #include @@ -40,8 +43,19 @@ #include #include #include +#include +#include +#include +#include +#include +#include +#include +#include #include +#include +#include +#include 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 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 #include -static std::unordered_map 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::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::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 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( win ) ) + dlgMap[ key ] = combo->GetSelection(); + else if( wxChoice* choice = dynamic_cast( win ) ) + dlgMap[ key ] = choice->GetSelection(); + else if( wxCheckBox* check = dynamic_cast( win ) ) + dlgMap[ key ] = check->GetValue(); + else if( wxSpinCtrl* spin = dynamic_cast( win ) ) + dlgMap[ key ] = spin->GetValue(); + else if( wxRadioButton* radio = dynamic_cast( win ) ) + dlgMap[ key ] = radio->GetValue(); + else if( wxRadioBox* radioBox = dynamic_cast( win ) ) + dlgMap[ key ] = radioBox->GetSelection(); + else if( wxSplitterWindow* splitter = dynamic_cast( win ) ) + dlgMap[ key ] = splitter->GetSashPosition(); + else if( wxScrolledWindow* scrolled = dynamic_cast( win ) ) + dlgMap[ key ] = scrolled->GetScrollPos( wxVERTICAL ); + else if( wxNotebook* notebook = dynamic_cast( 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 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( win ) ) + { + if( j.is_string() ) + combo->SetValue( wxString::FromUTF8( j.get().c_str() ) ); + } + else if( wxChoice* choice = dynamic_cast( win ) ) + { + if( j.is_number_integer() ) + choice->SetSelection( j.get() ); + } + else if( wxCheckBox* check = dynamic_cast( win ) ) + { + if( j.is_boolean() ) + check->SetValue( j.get() ); + } + else if( wxSpinCtrl* spin = dynamic_cast( win ) ) + { + if( j.is_number_integer() ) + spin->SetValue( j.get() ); + } + else if( wxRadioButton* radio = dynamic_cast( win ) ) + { + if( j.is_boolean() ) + radio->SetValue( j.get() ); + } + else if( wxRadioBox* radioBox = dynamic_cast( win ) ) + { + if( j.is_number_integer() ) + radioBox->SetSelection( j.get() ); + } + else if( wxSplitterWindow* splitter = dynamic_cast( win ) ) + { + if( j.is_number_integer() ) + splitter->SetSashPosition( j.get() ); + } + else if( wxScrolledWindow* scrolled = dynamic_cast( win ) ) + { + if( j.is_number_integer() ) + scrolled->SetScrollPos( wxVERTICAL, j.get() ); + } + else if( wxNotebook* notebook = dynamic_cast( win ) ) + { + if( j.is_number_integer() ) + notebook->SetSelection( j.get() ); + } + } + } + + 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. diff --git a/common/settings/common_settings.cpp b/common/settings/common_settings.cpp index 190761a2f4..75b339f809 100644 --- a/common/settings/common_settings.cpp +++ b/common/settings/common_settings.cpp @@ -435,6 +435,35 @@ COMMON_SETTINGS::COMMON_SETTINGS() : m_params.emplace_back( new PARAM( "api.enable_server", &m_Api.enable_server, false ) ); + m_params.emplace_back( new PARAM_LAMBDA( "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 ) ); diff --git a/include/dialog_shim.h b/include/dialog_shim.h index cfb8eccd76..46a3789fd6 100644 --- a/include/dialog_shim.h +++ b/include/dialog_shim.h @@ -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: *
subclass name = DIALOG_SHIM *
subclass header = dialog_shim.h *
@@ -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. diff --git a/include/property_holder.h b/include/property_holder.h index e8e16de2d2..182fea10b2 100644 --- a/include/property_holder.h +++ b/include/property_holder.h @@ -78,16 +78,165 @@ concept PropertyValueType = std::copy_constructible && std::destructible; 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( 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( 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 - void SetProperty(const std::string& key, T&& value) + template + bool SetProperty( const std::string& aKey, T&& aValue ) { - m_properties[key] = std::forward(value); + if( !IsValid() ) + return false; + + m_properties[aKey] = std::forward( aValue ); + return true; } /** @@ -96,18 +245,24 @@ public: * @param key The property key * @return std::optional containing the value if found and type matches, nullopt otherwise */ - template - std::optional GetProperty(const std::string& key) const + template + std::optional 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(it->second); + try + { + return std::any_cast( 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 - T GetPropertyOr(const std::string& key, T&& defaultValue) const + T GetPropertyOr(const std::string& aKey, T&& aDefaultValue) const { - if (auto value = GetProperty(key)) { - return *value; - } - return std::forward(defaultValue); + if( auto aValue = GetProperty( aKey ) ) + return *aValue; + + return std::forward(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 GetKeys() const { + if( !IsValid() ) + return {}; + std::vector 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 containing type info if property exists + * @return std::optional containing type info if property exists and object is valid */ - std::optional> GetPropertyType(const std::string& key) const + std::optional> 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 - bool HasPropertyOfType(const std::string& key) const + template + 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 - void SetTypedProperty(const std::string& key, T&& value) + bool SetTypedProperty(const std::string& aKey, T&& aValue) { - SetProperty(key, std::forward(value)); + return SetProperty(aKey, std::forward(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 m_properties; }; @@ -261,37 +450,37 @@ public: // Convenience methods that delegate to the property holder template - void SetProperty(const std::string& key, T&& value) + void SetProperty(const std::string& aKey, T&& aValue) { - m_propertyHolder.SetProperty(key, std::forward(value)); + m_propertyHolder.SetProperty(aKey, std::forward(aValue)); } template - std::optional GetProperty(const std::string& key) const + std::optional GetProperty(const std::string& aKey) const { - return m_propertyHolder.GetProperty(key); + return m_propertyHolder.GetProperty(aKey); } template - 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(defaultValue)); + return m_propertyHolder.GetPropertyOr(aKey, std::forward(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 - bool HasPropertyOfType(const std::string& key) const + bool HasPropertyOfType(const std::string& aKey) const { - return m_propertyHolder.HasPropertyOfType(key); + return m_propertyHolder.HasPropertyOfType(aKey); } private: diff --git a/include/settings/common_settings.h b/include/settings/common_settings.h index 046e592303..a2b19e6ae0 100644 --- a/include/settings/common_settings.h +++ b/include/settings/common_settings.h @@ -233,6 +233,9 @@ public: GIT m_Git; API m_Api; + + /// Persistent dialog control values + std::map> m_dialogControlValues; }; #endif diff --git a/qa/tests/common/test_property_holder.cpp b/qa/tests/common/test_property_holder.cpp index 78d83c2e93..ba0405aba9 100644 --- a/qa/tests/common/test_property_holder.cpp +++ b/qa/tests/common/test_property_holder.cpp @@ -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_prop" ) ); - BOOST_CHECK_EQUAL( true, m_populatedHolder.HasPropertyOfType( "int_prop" ) ); - BOOST_CHECK_EQUAL( true, m_populatedHolder.HasPropertyOfType( "double_prop" ) ); - BOOST_CHECK_EQUAL( true, m_populatedHolder.HasPropertyOfType( "string_prop" ) ); + BOOST_CHECK( m_populatedHolder.HasPropertyOfType( "bool_prop" ) ); + BOOST_CHECK( m_populatedHolder.HasPropertyOfType( "int_prop" ) ); + BOOST_CHECK( m_populatedHolder.HasPropertyOfType( "double_prop" ) ); + BOOST_CHECK( m_populatedHolder.HasPropertyOfType( "string_prop" ) ); // Check wrong type detection BOOST_CHECK_EQUAL( false, m_populatedHolder.HasPropertyOfType( "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( 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( 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( nullptr ) ) ); + BOOST_CHECK_EQUAL( nullptr, PROPERTY_HOLDER::SafeCast( static_cast( 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( 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( 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( 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 fake_properties; + }; + + FakePropertyHolder fake; + PROPERTY_HOLDER* fakeHolder = reinterpret_cast( &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( "anything" ) ); + + // GetProperty should return nullopt + auto result = fakeHolder->GetProperty( "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() \ No newline at end of file