Deduplicate settings migration handling

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<int>();
-
-    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(), 

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<int>();
-
-    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 ) )

common/settings/common_settings.cpp

@@ -176,25 +176,8 @@ COMMON_SETTINGS::COMMON_SETTINGS() :
 
     m_params.emplace_back( new PARAM<bool>( "session.remember_open_files",
         &m_Session.remember_open_files, false ) );
-}
 
-
+    registerMigration( 0, 1, std::bind( &COMMON_SETTINGS::migrateSchema0to1, this ) );
-bool COMMON_SETTINGS::Migrate()
-{
-    bool ret = true;
-    int  filever = at( PointerFromString( "meta.version" ) ).get<int>();
-
-    if( filever == 0 )
-    {
-        ret &= migrateSchema0to1();
-
-        if( ret )
-        {
-            ( *this )[PointerFromString( "meta.version" )] = 1;
-        }
-    }
-
-    return ret;
 }
 
 

common/settings/json_settings.cpp

@@ -420,10 +420,46 @@ OPT<nlohmann::json> JSON_SETTINGS::GetJson( const std::string& aPath ) const
 }
 
 
+void JSON_SETTINGS::registerMigration( int aOldSchemaVersion, int aNewSchemaVersion,
+                                       std::function<bool()> 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() );
+    int filever = at( PointerFromString( "meta.version" ) ).get<int>();
-    return false;
+
+    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<int, std::function<bool()>> 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;
 }
 
 

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 );

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;

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

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 );
 

include/settings/common_settings.h

@@ -103,8 +103,6 @@ public:
 
     virtual bool MigrateFromLegacy( wxConfigBase* aLegacyConfig ) override;
 
-    bool Migrate() override;
-
 private:
     bool migrateSchema0to1();
 

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<bool(void)> 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<nlohmann::json::json_pointer> m_preserved_paths;
+
+    /// A map of starting schema version to a pair of <ending version, migrator function>
+    std::map<int, std::pair<int, std::function<bool()>>> m_migrators;
 };
 
 // Specializations to allow conversion between wxString and std::string via JSON_SETTINGS API

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<bool>( "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<int>();
-
-    if( filever == 0 )
-    {
-        ret &= migrateSchema0to1();
-
-        if( ret )
-        {
-            ( *this )[PointerFromString( "meta.version" )] = 1;
-        }
-    }
-
-    return ret;
-}
-
-
 bool BOARD_DESIGN_SETTINGS::migrateSchema0to1()
 {
     /**

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<int>();
-
-    if( filever == 0 )
-    {
-        ret &= migrateSchema0to1();
-
-        if( ret )
-        {
-            ( *this )[PointerFromString( "meta.version" )] = 1;
-        }
-    }
-
-    return ret;
-}
-
-
 bool FOOTPRINT_EDITOR_SETTINGS::migrateSchema0to1()
 {
     /**