From daad2824c5c982671a3bebb68f0e636c6ac23e87 Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Sat, 30 May 2020 12:56:16 -0400 Subject: [PATCH] Add ability to skip JSON writes if the params aren't modified --- common/settings/json_settings.cpp | 60 ++++++++++---- common/settings/nested_settings.cpp | 24 +++++- common/settings/settings_manager.cpp | 9 ++- include/settings/color_settings.h | 18 ++++- include/settings/json_settings.h | 21 +++-- include/settings/nested_settings.h | 7 +- include/settings/parameters.h | 115 ++++++++++++++++++++++++++- 7 files changed, 218 insertions(+), 36 deletions(-) diff --git a/common/settings/json_settings.cpp b/common/settings/json_settings.cpp index 9304e6cb5c..48b91ae5dd 100644 --- a/common/settings/json_settings.cpp +++ b/common/settings/json_settings.cpp @@ -36,13 +36,14 @@ extern const char* traceSettings; JSON_SETTINGS::JSON_SETTINGS( const std::string& aFilename, SETTINGS_LOC aLocation, - int aSchemaVersion, bool aCreateIfMissing, bool aWriteFile, - nlohmann::json aDefault ) : - nlohmann::json( std::move( aDefault ) ), + int aSchemaVersion, bool aCreateIfMissing, bool aCreateIfDefault, + bool aWriteFile ) : + nlohmann::json(), m_filename( aFilename ), m_legacy_filename( "" ), m_location( aLocation ), m_createIfMissing( aCreateIfMissing ), + m_createIfDefault( aCreateIfDefault ), m_writeFile( aWriteFile ), m_schemaVersion( aSchemaVersion ), m_manager( nullptr ) @@ -206,10 +207,17 @@ void JSON_SETTINGS::LoadFromFile( const std::string& aDirectory ) } -void JSON_SETTINGS::Store() +bool JSON_SETTINGS::Store() { + bool modified = false; + for( auto param : m_params ) + { + modified |= !param->MatchesFile( this ); param->Store( this ); + } + + return modified; } @@ -220,29 +228,49 @@ void JSON_SETTINGS::ResetToDefaults() } -void JSON_SETTINGS::SaveToFile( const std::string& aDirectory ) +bool JSON_SETTINGS::SaveToFile( const std::string& aDirectory, bool aForce ) { if( !m_writeFile ) - return; - - wxLogTrace( traceSettings, "Saving %s", m_filename ); + return false; wxFileName path( aDirectory, m_filename, "json" ); if( !m_createIfMissing && !path.FileExists() ) - return; + { + wxLogTrace( traceSettings, + "File for %s doesn't exist and m_createIfMissing == false; not saving", + m_filename ); + return false; + } + + bool modified = false; + + for( auto settings : m_nested_settings ) + modified |= settings->SaveToFile(); + + modified |= Store(); + + if( !modified && !aForce && path.FileExists() ) + { + wxLogTrace( traceSettings, "%s contents not modified, skipping save", m_filename ); + return false; + } + else if( !modified && !aForce && !m_createIfDefault ) + { + wxLogTrace( traceSettings, + "%s contents still default and m_createIfDefault == false; not saving", + m_filename ); + return false; + } if( !path.DirExists() && !path.Mkdir() ) { wxLogTrace( traceSettings, "Warning: could not create path %s, can't save %s", - path.GetPath(), m_filename ); - return; + path.GetPath(), m_filename ); + return false; } - for( auto settings : m_nested_settings ) - settings->SaveToFile(); - - Store(); + wxLogTrace( traceSettings, "Saving %s", m_filename ); LOCALE_IO dummy; @@ -258,6 +286,8 @@ void JSON_SETTINGS::SaveToFile( const std::string& aDirectory ) catch( ... ) { } + + return true; } diff --git a/common/settings/nested_settings.cpp b/common/settings/nested_settings.cpp index be2dbce9a8..29179afe4a 100644 --- a/common/settings/nested_settings.cpp +++ b/common/settings/nested_settings.cpp @@ -26,8 +26,8 @@ extern const char* traceSettings; NESTED_SETTINGS::NESTED_SETTINGS( const std::string& aName, int aVersion, JSON_SETTINGS* aParent, - const std::string& aPath, nlohmann::json aDefault ) : - JSON_SETTINGS( aName, SETTINGS_LOC::NESTED, aVersion, std::move( aDefault ) ), + const std::string& aPath ) : + JSON_SETTINGS( aName, SETTINGS_LOC::NESTED, aVersion ), m_parent( aParent ), m_path( aPath ) { if( m_parent ) @@ -70,9 +70,23 @@ void NESTED_SETTINGS::LoadFromFile( const std::string& aDirectory ) } -void NESTED_SETTINGS::SaveToFile( const std::string& aDirectory ) +bool NESTED_SETTINGS::SaveToFile( const std::string& aDirectory, bool aForce ) { - Store(); + bool modified = Store(); + + try + { + nlohmann::json patch = + nlohmann::json::diff( *this, ( *m_parent )[PointerFromString( m_path )] ); + modified |= !patch.empty(); + } + catch( ... ) + { + modified = true; + } + + if( !modified && !aForce ) + return false; try { @@ -86,4 +100,6 @@ void NESTED_SETTINGS::SaveToFile( const std::string& aDirectory ) wxLogTrace( traceSettings, "NESTED_SETTINGS %s: Could not store to %s at %s", m_filename, m_parent->GetFilename(), m_path ); } + + return modified; } diff --git a/common/settings/settings_manager.cpp b/common/settings/settings_manager.cpp index 8e4410ca64..68540d5026 100644 --- a/common/settings/settings_manager.cpp +++ b/common/settings/settings_manager.cpp @@ -290,7 +290,12 @@ void SETTINGS_MANAGER::SaveColorSettings( COLOR_SETTINGS* aSettings, const std:: nlohmann::json::json_pointer ptr = JSON_SETTINGS::PointerFromString( aNamespace ); - aSettings->Store(); + if( !aSettings->Store() ) + { + wxLogTrace( traceSettings, "Color scheme %s not modified; skipping save", + aSettings->GetFilename(), aNamespace ); + return; + } wxASSERT( aSettings->contains( ptr ) ); @@ -305,7 +310,7 @@ void SETTINGS_MANAGER::SaveColorSettings( COLOR_SETTINGS* aSettings, const std:: ( *aSettings )[ptr].update( backup ); aSettings->Load(); - aSettings->SaveToFile( path ); + aSettings->SaveToFile( path, true ); } diff --git a/include/settings/color_settings.h b/include/settings/color_settings.h index ff17267b3e..98afc21551 100644 --- a/include/settings/color_settings.h +++ b/include/settings/color_settings.h @@ -133,14 +133,30 @@ public: return m_default; } - virtual void SetDefault() override + void SetDefault() override { ( *m_map )[ m_key ] = m_default; } + bool IsDefault() const override + { + return ( *m_map )[ m_key ] == m_default; + } + + bool MatchesFile( JSON_SETTINGS* aSettings ) const override + { + if( OPT optval = aSettings->Get( m_path ) ) + return m_map->count( m_key ) && ( *optval == m_map->at( m_key ) ); + + // If the JSON doesn't exist, the map shouldn't exist either + return !m_map->count( m_key ); + } + private: int m_key; + COLOR4D m_default; + std::unordered_map* m_map; }; diff --git a/include/settings/json_settings.h b/include/settings/json_settings.h index 42b2a02caf..71c64c87cf 100644 --- a/include/settings/json_settings.h +++ b/include/settings/json_settings.h @@ -44,13 +44,11 @@ enum class SETTINGS_LOC { class JSON_SETTINGS : public nlohmann::json { public: - JSON_SETTINGS( const std::string& aFilename, SETTINGS_LOC aLocation, int aSchemaVersion, - nlohmann::json aDefaultJson = nlohmann::json( {} ) ) : - JSON_SETTINGS( aFilename, aLocation, aSchemaVersion, true, true, aDefaultJson ) {} + JSON_SETTINGS( const std::string& aFilename, SETTINGS_LOC aLocation, int aSchemaVersion ) : + JSON_SETTINGS( aFilename, aLocation, aSchemaVersion, true, true, true ) {} JSON_SETTINGS( const std::string& aFilename, SETTINGS_LOC aLocation, int aSchemaVersion, - bool aCreateIfMissing, bool aWriteFile, - nlohmann::json aDefaultJson = nlohmann::json( {} ) ); + bool aCreateIfMissing, bool aCreateIfDefault, bool aWriteFile ); virtual ~JSON_SETTINGS(); @@ -68,8 +66,9 @@ public: /** * Stores the current parameters into the JSON document represented by this object * Note: this doesn't do any writing to disk; that's handled by SETTINGS_MANAGER + * @return true if any part of the JSON document was updated */ - virtual void Store(); + virtual bool Store(); /** * Loads the backing file from disk and then calls Load() @@ -80,8 +79,10 @@ public: /** * Calls Store() and then writes the contents of the JSON document to a file * @param aDirectory is the directory to save to, including trailing separator + * @param aForce if true will always save, even if contents are not modified + * @return true if the file was saved */ - virtual void SaveToFile( const std::string& aDirectory ); + virtual bool SaveToFile( const std::string& aDirectory, bool aForce = false ); /** * Resets all parameters to default values. Does NOT write to file or update underlying JSON. @@ -228,6 +229,12 @@ protected: /// Whether or not the backing store file should be created it if doesn't exist bool m_createIfMissing; + /** + * Whether or not the backing store file should be created if all parameters are still + * at their default values. Ignored if m_createIfMissing is false or m_writeFile is false. + */ + bool m_createIfDefault; + /// Whether or not the backing store file should be written bool m_writeFile; diff --git a/include/settings/nested_settings.h b/include/settings/nested_settings.h index 717d586929..a500bf09c3 100644 --- a/include/settings/nested_settings.h +++ b/include/settings/nested_settings.h @@ -32,7 +32,7 @@ class NESTED_SETTINGS : public JSON_SETTINGS { public: NESTED_SETTINGS( const std::string& aName, int aSchemaVersion, JSON_SETTINGS* aParent, - const std::string& aPath, nlohmann::json aDefault = nlohmann::json( {} ) ); + const std::string& aPath ); virtual ~NESTED_SETTINGS(); @@ -40,13 +40,14 @@ public: * Loads the JSON document from the parent and then calls Load() * @param aDirectory */ - virtual void LoadFromFile( const std::string& aDirectory = "" ) override; + void LoadFromFile( const std::string& aDirectory = "" ) override; /** * Calls Store() and then saves the JSON document contents into the parent JSON_SETTINGS * @param aDirectory is ignored + * @return true if the document contents were updated */ - virtual void SaveToFile( const std::string& aDirectory = "" ) override; + bool SaveToFile( const std::string& aDirectory = "", bool aForce = false ) override; private: diff --git a/include/settings/parameters.h b/include/settings/parameters.h index 3fd16f44de..5bd4d92aae 100644 --- a/include/settings/parameters.h +++ b/include/settings/parameters.h @@ -53,6 +53,19 @@ public: virtual void SetDefault() = 0; + /** + * Checks whether or not this param has been changed from its default value + * @return true if the parameter in memory matches its default value + */ + virtual bool IsDefault() const = 0; + + /** + * Checks whether the parameter in memory matches the one in a given JSON file + * @param aSettings is a JSON_SETTINGS to check the JSON file contents of + * @return true if the parameter in memory matches its value in the file + */ + virtual bool MatchesFile( JSON_SETTINGS* aSettings ) const = 0; + /** * @return the path name of the parameter used to store it in the json file * mainly usefull in error messages @@ -115,7 +128,7 @@ public: *m_ptr = val; } - void Store( JSON_SETTINGS* aSettings) const override + void Store( JSON_SETTINGS* aSettings ) const override { aSettings->Set( m_path, *m_ptr ); } @@ -125,11 +138,24 @@ public: return m_default; } - virtual void SetDefault() override + void SetDefault() override { *m_ptr = m_default; } + bool IsDefault() const override + { + return *m_ptr == m_default; + } + + bool MatchesFile( JSON_SETTINGS* aSettings ) const override + { + if( OPT optval = aSettings->Get( m_path ) ) + return *optval == *m_ptr; + + return false; + } + private: ValueType* m_ptr; ValueType m_default; @@ -193,11 +219,33 @@ public: return m_default; } - virtual void SetDefault() override + void SetDefault() override { m_setter( m_default ); } + bool IsDefault() const override + { + return m_getter() == m_default; + } + + bool MatchesFile( JSON_SETTINGS* aSettings ) const override + { + if( std::is_same::value ) + { + if( OPT optval = aSettings->GetJson( m_path ) ) + return *optval == m_default; + } + else + { + if( OPT optval = aSettings->Get( m_path ) ) + return *optval == m_default; + } + + // Not in file + return false; + } + private: ValueType m_default; @@ -275,6 +323,19 @@ public: *m_ptr = m_default; } + bool IsDefault() const override + { + return *m_ptr == m_default; + } + + bool MatchesFile( JSON_SETTINGS* aSettings ) const override + { + if( OPT optval = aSettings->Get( m_path ) ) + return *optval == ( *m_ptr * m_scale ); + + return false; + } + private: ValueType* m_ptr; ValueType m_default; @@ -333,11 +394,34 @@ public: aSettings->Set( m_path, js ); } - virtual void SetDefault() override + void SetDefault() override { *m_ptr = m_default; } + bool IsDefault() const override + { + return *m_ptr == m_default; + } + + bool MatchesFile( JSON_SETTINGS* aSettings ) const override + { + if( OPT js = aSettings->GetJson( m_path ) ) + { + if( js->is_array() ) + { + std::vector val; + + for( const auto& el : js->items() ) + val.emplace_back( el.value().get() ); + + return val == *m_ptr; + } + } + + return false; + } + private: std::vector* m_ptr; @@ -405,6 +489,29 @@ public: *m_ptr = m_default; } + bool IsDefault() const override + { + return *m_ptr == m_default; + } + + bool MatchesFile( JSON_SETTINGS* aSettings ) const override + { + if( OPT js = aSettings->GetJson( m_path ) ) + { + if( js->is_object() ) + { + std::map val; + + for( const auto& el : js->items() ) + val[ el.key() ] = el.value().get(); + + return val == *m_ptr; + } + } + + return false; + } + private: std::map* m_ptr;