From 728c207105e1291e43a6f182729ee64079223870 Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Mon, 5 Oct 2020 23:21:57 -0400 Subject: [PATCH] Deduplicate settings migration handling --- common/project/project_local_settings.cpp | 69 +++++++++-------------- common/settings/color_settings.cpp | 53 +++++------------ common/settings/common_settings.cpp | 19 +------ common/settings/json_settings.cpp | 40 ++++++++++++- include/board_design_settings.h | 2 - include/footprint_editor_settings.h | 2 - include/project/project_local_settings.h | 5 -- include/settings/color_settings.h | 3 - include/settings/common_settings.h | 2 - include/settings/json_settings.h | 16 +++++- pcbnew/board_design_settings.cpp | 21 +------ pcbnew/footprint_editor_settings.cpp | 21 +------ 12 files changed, 98 insertions(+), 155 deletions(-) diff --git a/common/project/project_local_settings.cpp b/common/project/project_local_settings.cpp index 053e19b09c..88e78b00c5 100644 --- a/common/project/project_local_settings.cpp +++ b/common/project/project_local_settings.cpp @@ -219,6 +219,32 @@ PROJECT_LOCAL_SETTINGS::PROJECT_LOCAL_SETTINGS( PROJECT* aProject, const wxStrin }, { } ) ); + + registerMigration( 1, 2, + [&]() + { + /** + * Schema version 1 to 2: + * LAYER_PADS and LAYER_ZONES added to visibility controls + */ + + nlohmann::json::json_pointer ptr( "/board/visible_items"_json_pointer ); + + if( contains( ptr ) ) + { + if( ( *this )[ptr].is_array() ) + { + ( *this )[ptr].push_back( LAYER_PADS ); + ( *this )[ptr].push_back( LAYER_ZONES ); + } + else + { + at( "board" ).erase( "visible_items" ); + } + } + + return true; + } ); } @@ -244,49 +270,6 @@ bool PROJECT_LOCAL_SETTINGS::SaveToFile( const wxString& aDirectory, bool aForce } -bool PROJECT_LOCAL_SETTINGS::Migrate() -{ - bool ret = true; - int filever = at( PointerFromString( "meta.version" ) ).get(); - - if( filever == 1 ) - { - ret &= migrateSchema1to2(); - - if( ret ) - ( *this )[PointerFromString( "meta.version" )] = 2; - } - - return ret; -} - - -bool PROJECT_LOCAL_SETTINGS::migrateSchema1to2() -{ - /** - * Schema version 1 to 2: - * LAYER_PADS and LAYER_ZONES added to visibility controls - */ - - nlohmann::json::json_pointer ptr( "/board/visible_items"_json_pointer ); - - if( contains( ptr ) ) - { - if( ( *this )[ptr].is_array() ) - { - ( *this )[ptr].push_back( LAYER_PADS ); - ( *this )[ptr].push_back( LAYER_ZONES ); - } - else - { - at( "board" ).erase( "visible_items" ); - } - } - - return true; -} - - const PROJECT_FILE_STATE* PROJECT_LOCAL_SETTINGS::GetFileState( const wxString& aFileName ) { auto it = std::find_if( m_files.begin(), m_files.end(), diff --git a/common/settings/color_settings.cpp b/common/settings/color_settings.cpp index 6284e9ba8d..50a665b2bc 100644 --- a/common/settings/color_settings.cpp +++ b/common/settings/color_settings.cpp @@ -214,6 +214,19 @@ COLOR_SETTINGS::COLOR_SETTINGS( wxString aFilename ) : CLR( "3d_viewer.silkscreen_top", LAYER_3D_SILKSCREEN_TOP, COLOR4D( 0.9, 0.9, 0.9, 1.0 ) ); CLR( "3d_viewer.soldermask", LAYER_3D_SOLDERMASK, COLOR4D( 0.08, 0.2, 0.14, 0.83 ) ); CLR( "3d_viewer.solderpaste", LAYER_3D_SOLDERPASTE, COLOR4D( 0.5, 0.5, 0.5, 1.0 ) ); + + registerMigration( 0, 1, std::bind( &COLOR_SETTINGS::migrateSchema0to1, this ) ); + + registerMigration( 1, 2, + [&]() + { + // Fix LAYER_VIAS_HOLES color - before version 2, this setting had no effect + nlohmann::json::json_pointer ptr( "/board/via_hole"); + + ( *this )[ptr] = COLOR4D( 0.5, 0.4, 0, 0.8 ).ToWxString( wxC2S_CSS_SYNTAX ); + + return true; + } ); } @@ -256,33 +269,6 @@ bool COLOR_SETTINGS::MigrateFromLegacy( wxConfigBase* aCfg ) } -bool COLOR_SETTINGS::Migrate() -{ - bool ret = true; - int filever = at( PointerFromString( "meta.version" ) ).get(); - - if( filever == 0 ) - { - ret &= migrateSchema0to1(); - - if( ret ) - filever = 1; - } - - if( filever == 1 ) - { - ret &= migrateSchema1to2(); - - if( ret ) - filever = 2; - } - - ( *this )[PointerFromString( "meta.version" )] = filever; - - return ret; -} - - bool COLOR_SETTINGS::migrateSchema0to1() { /** @@ -304,7 +290,7 @@ bool COLOR_SETTINGS::migrateSchema0to1() if( !contains( fpedit ) ) { wxLogTrace( traceSettings, "migrateSchema0to1: %s doesn't have fpedit settings; skipping.", - m_filename ); + m_filename ); return true; } @@ -331,17 +317,6 @@ bool COLOR_SETTINGS::migrateSchema0to1() } -bool COLOR_SETTINGS::migrateSchema1to2() -{ - // Fix LAYER_VIAS_HOLES color - before version 2, this setting had no effect - nlohmann::json::json_pointer ptr( "/board/via_hole"); - - ( *this )[ptr] = COLOR4D( 0.5, 0.4, 0, 0.8 ).ToWxString( wxC2S_CSS_SYNTAX ); - - return true; -} - - COLOR4D COLOR_SETTINGS::GetColor( int aLayer ) const { if( m_colors.count( aLayer ) ) diff --git a/common/settings/common_settings.cpp b/common/settings/common_settings.cpp index 1831cbbf95..136b181676 100644 --- a/common/settings/common_settings.cpp +++ b/common/settings/common_settings.cpp @@ -176,25 +176,8 @@ COMMON_SETTINGS::COMMON_SETTINGS() : m_params.emplace_back( new PARAM( "session.remember_open_files", &m_Session.remember_open_files, false ) ); -} - -bool COMMON_SETTINGS::Migrate() -{ - bool ret = true; - int filever = at( PointerFromString( "meta.version" ) ).get(); - - if( filever == 0 ) - { - ret &= migrateSchema0to1(); - - if( ret ) - { - ( *this )[PointerFromString( "meta.version" )] = 1; - } - } - - return ret; + registerMigration( 0, 1, std::bind( &COMMON_SETTINGS::migrateSchema0to1, this ) ); } diff --git a/common/settings/json_settings.cpp b/common/settings/json_settings.cpp index b282b57bae..77bdc14f60 100644 --- a/common/settings/json_settings.cpp +++ b/common/settings/json_settings.cpp @@ -420,10 +420,46 @@ OPT JSON_SETTINGS::GetJson( const std::string& aPath ) const } +void JSON_SETTINGS::registerMigration( int aOldSchemaVersion, int aNewSchemaVersion, + std::function aMigrator ) +{ + wxASSERT( aNewSchemaVersion > aOldSchemaVersion ); + wxASSERT( aNewSchemaVersion <= m_schemaVersion ); + m_migrators[aOldSchemaVersion] = std::make_pair( aNewSchemaVersion, aMigrator ); +} + + bool JSON_SETTINGS::Migrate() { - wxLogTrace( traceSettings, "Migrate() not implemented for %s", typeid( *this ).name() ); - return false; + int filever = at( PointerFromString( "meta.version" ) ).get(); + + while( filever < m_schemaVersion ) + { + if( !m_migrators.count( filever ) ) + { + wxLogTrace( traceSettings, "Migrator missing for %s version %d!", + typeid( *this ).name(), filever ); + return false; + } + + std::pair> pair = m_migrators.at( filever ); + + if( pair.second() ) + { + wxLogTrace( traceSettings, "Migrated %s from %d to %d", typeid( *this ).name(), + filever, pair.first ); + filever = pair.first; + ( *this )[PointerFromString( "meta.version" )] = filever; + } + else + { + wxLogTrace( traceSettings, "Migration failed for %s from %d to %d", + typeid( *this ).name(), filever, pair.first ); + return false; + } + } + + return true; } diff --git a/include/board_design_settings.h b/include/board_design_settings.h index 298861f5f8..bfd133802f 100644 --- a/include/board_design_settings.h +++ b/include/board_design_settings.h @@ -362,8 +362,6 @@ public: bool LoadFromFile( const wxString& aDirectory = "" ) override; - bool Migrate() override; - BOARD_STACKUP& GetStackupDescriptor() { return m_stackup; } int GetSeverity( int aDRCErrorCode ); diff --git a/include/footprint_editor_settings.h b/include/footprint_editor_settings.h index 6de2586fef..1fefab4b1f 100644 --- a/include/footprint_editor_settings.h +++ b/include/footprint_editor_settings.h @@ -43,8 +43,6 @@ public: virtual bool MigrateFromLegacy( wxConfigBase* aLegacyConfig ) override; - bool Migrate() override; - /// Only some of these settings are actually used for footprint editing // TODO: factor out the relevant stuff so the whole BDS doesn't have to be here BOARD_DESIGN_SETTINGS m_DesignSettings; diff --git a/include/project/project_local_settings.h b/include/project/project_local_settings.h index 6075003f86..cf7b2b2918 100644 --- a/include/project/project_local_settings.h +++ b/include/project/project_local_settings.h @@ -58,8 +58,6 @@ public: bool MigrateFromLegacy( wxConfigBase* aLegacyConfig ) override; - bool Migrate() override; - bool SaveToFile( const wxString& aDirectory = "", bool aForce = false ) override; void SetProject( PROJECT* aProject ) @@ -67,9 +65,6 @@ public: m_project = aProject; } -private: - bool migrateSchema1to2(); - protected: wxString getFileExt() const override diff --git a/include/settings/color_settings.h b/include/settings/color_settings.h index 426ec26c95..f082dea68a 100644 --- a/include/settings/color_settings.h +++ b/include/settings/color_settings.h @@ -74,8 +74,6 @@ public: bool MigrateFromLegacy( wxConfigBase* aCfg ) override; - bool Migrate() override; - COLOR4D GetColor( int aLayer ) const; COLOR4D GetDefaultColor( int aLayer ); @@ -90,7 +88,6 @@ public: private: bool migrateSchema0to1(); - bool migrateSchema1to2(); void initFromOther( const COLOR_SETTINGS& aOther ); diff --git a/include/settings/common_settings.h b/include/settings/common_settings.h index 32b19a4754..d2b0133116 100644 --- a/include/settings/common_settings.h +++ b/include/settings/common_settings.h @@ -103,8 +103,6 @@ public: virtual bool MigrateFromLegacy( wxConfigBase* aLegacyConfig ) override; - bool Migrate() override; - private: bool migrateSchema0to1(); diff --git a/include/settings/json_settings.h b/include/settings/json_settings.h index 15b7d12634..6ec0db7720 100644 --- a/include/settings/json_settings.h +++ b/include/settings/json_settings.h @@ -156,7 +156,7 @@ c * @return true if the file was saved * * @return true if migration was successful */ - virtual bool Migrate(); + bool Migrate(); /** * Migrates from wxConfig to JSON-based configuration. Should be implemented by any subclasses @@ -229,6 +229,17 @@ c * @return true if the file was saved unsigned int& aTarget ); protected: + /** + * Registers a migration from one schema version to another. If the schema version in the file + * loaded from disk is less than the schema version of the JSON_SETTINGS class, migration + * functions will be called one after the other until the data is up-to-date. + * @param aOldSchemaVersion is the starting schema version for this migration + * @param aNewSchemaVersion is the ending schema version for this migration + * @param aMigrator is a function that performs the migration and returns true if successful + */ + void registerMigration( int aOldSchemaVersion, int aNewSchemaVersion, + std::function aMigrator ); + /** * Translates a legacy wxConfig value to a given JSON pointer value * @tparam ValueType is the basic type of the value @@ -308,6 +319,9 @@ protected: /// A list of JSON pointers that are preserved during a read-update-write to disk std::vector m_preserved_paths; + + /// A map of starting schema version to a pair of + std::map>> m_migrators; }; // Specializations to allow conversion between wxString and std::string via JSON_SETTINGS API diff --git a/pcbnew/board_design_settings.cpp b/pcbnew/board_design_settings.cpp index 50853b29a3..b808d5528e 100644 --- a/pcbnew/board_design_settings.cpp +++ b/pcbnew/board_design_settings.cpp @@ -596,6 +596,8 @@ BOARD_DESIGN_SETTINGS::BOARD_DESIGN_SETTINGS( JSON_SETTINGS* aParent, const std: m_params.emplace_back( new PARAM( "zones_allow_external_fillets", &m_ZoneKeepExternalFillets, false ) ); + + registerMigration( 0, 1, std::bind( &BOARD_DESIGN_SETTINGS::migrateSchema0to1, this ) ); } @@ -707,25 +709,6 @@ void BOARD_DESIGN_SETTINGS::initFromOther( const BOARD_DESIGN_SETTINGS& aOther ) } -bool BOARD_DESIGN_SETTINGS::Migrate() -{ - bool ret = true; - int filever = at( PointerFromString( "meta.version" ) ).get(); - - if( filever == 0 ) - { - ret &= migrateSchema0to1(); - - if( ret ) - { - ( *this )[PointerFromString( "meta.version" )] = 1; - } - } - - return ret; -} - - bool BOARD_DESIGN_SETTINGS::migrateSchema0to1() { /** diff --git a/pcbnew/footprint_editor_settings.cpp b/pcbnew/footprint_editor_settings.cpp index c9260fb987..81a954fd56 100644 --- a/pcbnew/footprint_editor_settings.cpp +++ b/pcbnew/footprint_editor_settings.cpp @@ -273,6 +273,8 @@ FOOTPRINT_EDITOR_SETTINGS::FOOTPRINT_EDITOR_SETTINGS() : { "dimensions", true }, { "otherItems", true } } ) ); + + registerMigration( 0, 1, std::bind( &FOOTPRINT_EDITOR_SETTINGS::migrateSchema0to1, this ) ); } @@ -394,25 +396,6 @@ bool FOOTPRINT_EDITOR_SETTINGS::MigrateFromLegacy( wxConfigBase* aCfg ) } -bool FOOTPRINT_EDITOR_SETTINGS::Migrate() -{ - bool ret = true; - int filever = at( PointerFromString( "meta.version" ) ).get(); - - if( filever == 0 ) - { - ret &= migrateSchema0to1(); - - if( ret ) - { - ( *this )[PointerFromString( "meta.version" )] = 1; - } - } - - return ret; -} - - bool FOOTPRINT_EDITOR_SETTINGS::migrateSchema0to1() { /**