Symbol library manager object changes.

* Fix issue handling symbols with multiple inheritance.
* Remove unused code from the symbol library manager object.
* Splits out the library buffer and symbol buffer object so the can be unit
  tested without having to mock the symbol library manager object.
* Add unit tests for library buffer and symbol buffer objects.

eeschema/symbol_library_manager.cpp

@@ -188,7 +188,7 @@ bool SYMBOL_LIBRARY_MANAGER::SaveLibrary( const wxString& aLibrary, const wxStri
 
         const auto& symbolBuffers = libBuf.GetBuffers();
 
-        for( const auto& symbolBuf : symbolBuffers )
+        for( const std::shared_ptr<SYMBOL_BUFFER>& symbolBuf : symbolBuffers )
         {
             if( !libBuf.SaveBuffer( symbolBuf, aFileName, &*pi, true ) )
             {
@@ -287,13 +287,12 @@ bool SYMBOL_LIBRARY_MANAGER::IsSymbolModified( const wxString& aAlias,
         return false;
 
     const LIB_BUFFER& buf = libIt->second;
-    const std::shared_ptr<SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER> symbolBuf = buf.GetBuffer( aAlias );
+    const std::shared_ptr<SYMBOL_BUFFER> symbolBuf = buf.GetBuffer( aAlias );
     return symbolBuf ? symbolBuf->IsModified() : false;
 }
 
 
-void SYMBOL_LIBRARY_MANAGER::SetSymbolModified( const wxString& aAlias,
+void SYMBOL_LIBRARY_MANAGER::SetSymbolModified( const wxString& aAlias, const wxString& aLibrary )
-                                                const wxString& aLibrary )
 {
     auto libIt = m_libs.find( aLibrary );
 
@@ -301,7 +300,7 @@ void SYMBOL_LIBRARY_MANAGER::SetSymbolModified( const wxString& aAlias,
         return;
 
     const LIB_BUFFER& buf = libIt->second;
-    std::shared_ptr<SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER> symbolBuf = buf.GetBuffer( aAlias );
+    std::shared_ptr<SYMBOL_BUFFER> symbolBuf = buf.GetBuffer( aAlias );
 
     wxCHECK( symbolBuf, /* void */ );
 
@@ -513,20 +512,6 @@ bool SYMBOL_LIBRARY_MANAGER::UpdateSymbolAfterRename( LIB_SYMBOL* aSymbol, const
 }
 
 
-bool SYMBOL_LIBRARY_MANAGER::FlushSymbol( const wxString& aAlias, const wxString& aLibrary )
-{
-    auto it = m_libs.find( aLibrary );
-
-    if( it == m_libs.end() )    // no items to flush
-        return true;
-
-    auto symbolBuf = it->second.GetBuffer( aAlias );
-    wxCHECK( symbolBuf, false );
-
-    return it->second.SaveBuffer( symbolBuf, symTable() );
-}
-
-
 LIB_ID SYMBOL_LIBRARY_MANAGER::RevertSymbol( const wxString& aAlias, const wxString& aLibrary )
 {
     auto it = m_libs.find( aLibrary );
@@ -800,8 +785,7 @@ std::set<LIB_SYMBOL*> SYMBOL_LIBRARY_MANAGER::getOriginalSymbols( const wxString
 }
 
 
-SYMBOL_LIBRARY_MANAGER::LIB_BUFFER& SYMBOL_LIBRARY_MANAGER::getLibraryBuffer(
+LIB_BUFFER& SYMBOL_LIBRARY_MANAGER::getLibraryBuffer( const wxString& aLibrary )
-        const wxString& aLibrary )
 {
     auto it = m_libs.find( aLibrary );
 
@@ -875,8 +859,7 @@ bool SYMBOL_LIBRARY_MANAGER::UpdateLibraryBuffer( const wxString& aLibrary )
 }
 
 
-SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::SYMBOL_BUFFER( LIB_SYMBOL* aSymbol,
+SYMBOL_BUFFER::SYMBOL_BUFFER( LIB_SYMBOL* aSymbol, std::unique_ptr<SCH_SCREEN> aScreen ) :
-                                                      std::unique_ptr<SCH_SCREEN> aScreen ) :
     m_screen( std::move( aScreen ) ),
     m_symbol( aSymbol )
 {
@@ -884,14 +867,14 @@ SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::SYMBOL_BUFFER( LIB_SYMBOL* aSymbol,
 }
 
 
-SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::~SYMBOL_BUFFER()
+SYMBOL_BUFFER::~SYMBOL_BUFFER()
 {
     delete m_symbol;
     delete m_original;
 }
 
 
-void SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::SetSymbol( LIB_SYMBOL* aSymbol )
+void SYMBOL_BUFFER::SetSymbol( LIB_SYMBOL* aSymbol )
 {
     wxCHECK( m_symbol != aSymbol, /* void */ );
     wxASSERT( aSymbol );
@@ -907,7 +890,7 @@ void SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::SetSymbol( LIB_SYMBOL* aSymbol )
 }
 
 
-void SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::SetOriginal( LIB_SYMBOL* aSymbol )
+void SYMBOL_BUFFER::SetOriginal( LIB_SYMBOL* aSymbol )
 {
     wxCHECK( m_original != aSymbol, /* void */ );
     wxASSERT( aSymbol );
@@ -923,13 +906,13 @@ void SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::SetOriginal( LIB_SYMBOL* aSymbol )
 }
 
 
-bool SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER::IsModified() const
+bool SYMBOL_BUFFER::IsModified() const
 {
     return m_screen && m_screen->IsContentModified();
 }
 
 
-LIB_SYMBOL* SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetSymbol( const wxString& aAlias ) const
+LIB_SYMBOL* LIB_BUFFER::GetSymbol( const wxString& aAlias ) const
 {
     auto buf = GetBuffer( aAlias );
 
@@ -944,7 +927,7 @@ LIB_SYMBOL* SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetSymbol( const wxString& aAlia
 }
 
 
-bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::CreateBuffer( LIB_SYMBOL* aCopy, SCH_SCREEN* aScreen )
+bool LIB_BUFFER::CreateBuffer( LIB_SYMBOL* aCopy, SCH_SCREEN* aScreen )
 {
     wxASSERT( aCopy );
     wxASSERT( aCopy->GetLib() == nullptr );
@@ -963,8 +946,7 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::CreateBuffer( LIB_SYMBOL* aCopy, SCH_SC
 }
 
 
-bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::UpdateBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf,
+bool LIB_BUFFER::UpdateBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf, LIB_SYMBOL* aCopy )
-                                                       LIB_SYMBOL* aCopy )
 {
     wxCHECK( aCopy && aSymbolBuf, false );
 
@@ -979,7 +961,7 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::UpdateBuffer( std::shared_ptr<SYMBOL_BU
 }
 
 
-bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::DeleteBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf )
+bool LIB_BUFFER::DeleteBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf )
 {
     auto symbolBufIt = std::find( m_symbols.begin(), m_symbols.end(), aSymbolBuf );
     wxCHECK( symbolBufIt != m_symbols.end(), false );
@@ -1001,124 +983,24 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::DeleteBuffer( std::shared_ptr<SYMBOL_BU
 }
 
 
-bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::SaveBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf,
+bool LIB_BUFFER::SaveBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf,
-                                                     SYMBOL_LIB_TABLE* aLibTable )
+                             const wxString& aFileName, SCH_PLUGIN* aPlugin, bool aBuffer )
 {
     wxCHECK( aSymbolBuf, false );
-    LIB_SYMBOL* libSymbol = aSymbolBuf->GetSymbol();
-    LIB_SYMBOL* originalSymbol = aSymbolBuf->GetOriginal();
-    wxCHECK( libSymbol && originalSymbol, false );
-    SYMBOL_LIB_TABLE::SAVE_T result;
-    STRING_UTF8_MAP          properties;
-    properties.emplace( SCH_LEGACY_PLUGIN::PropBuffering, "" );
-
-    wxString errorMsg = _( "Error saving symbol %s to library '%s'." ) + wxS( "\n%s" );
-
-    // Delete the original symbol if the symbol name has been changed.
-    if( libSymbol->GetName() != originalSymbol->GetName() )
-    {
-        // DeleteSymbol may throw
-        try
-        {
-            if( aLibTable->LoadSymbol( m_libName, originalSymbol->GetName() ) )
-                aLibTable->DeleteSymbol( m_libName, originalSymbol->GetName() );
-        }
-        catch( const IO_ERROR& ioe )
-        {
-            wxLogError( errorMsg, UnescapeString( originalSymbol->GetName() ), m_libName,
-                        ioe.What() );
-            return false;
-        }
-    }
-
-    if( libSymbol->IsAlias() )
-    {
-        LIB_SYMBOL* newCachedSymbol = new LIB_SYMBOL( *libSymbol );
-        std::shared_ptr< LIB_SYMBOL > bufferedParent = libSymbol->GetParent().lock();
-
-        wxCHECK( bufferedParent, false );
-
-        LIB_SYMBOL* cachedParent = aLibTable->LoadSymbol( m_libName, bufferedParent->GetName() );
-
-        if( !cachedParent )
-        {
-            cachedParent = new LIB_SYMBOL( *bufferedParent.get() );
-            newCachedSymbol->SetParent( cachedParent );
-            result = aLibTable->SaveSymbol( m_libName, cachedParent );
-            wxCHECK( result == SYMBOL_LIB_TABLE::SAVE_OK, false );
-            result = aLibTable->SaveSymbol( m_libName, newCachedSymbol );
-            wxCHECK( result == SYMBOL_LIB_TABLE::SAVE_OK, false );
-
-            LIB_SYMBOL* originalParent = new LIB_SYMBOL( *bufferedParent.get() );
-            aSymbolBuf->SetOriginal( originalParent );
-            originalSymbol = new LIB_SYMBOL( *libSymbol );
-            originalSymbol->SetParent( originalParent );
-            aSymbolBuf->SetOriginal( originalSymbol );
-        }
-        else
-        {
-            newCachedSymbol->SetParent( cachedParent );
-            result = aLibTable->SaveSymbol( m_libName, newCachedSymbol );
-            wxCHECK( result == SYMBOL_LIB_TABLE::SAVE_OK, false );
-
-            auto originalBufferedParent = GetBuffer( bufferedParent->GetName() );
-            wxCHECK( originalBufferedParent, false );
-            originalSymbol = new LIB_SYMBOL( *libSymbol );
-            originalSymbol->SetParent( originalBufferedParent->GetSymbol() );
-            aSymbolBuf->SetOriginal( originalSymbol );
-        }
-    }
-    else
-    {
-        wxArrayString derivedSymbols;
-
-        if( GetDerivedSymbolNames( libSymbol->GetName(), derivedSymbols ) == 0 )
-        {
-            result = aLibTable->SaveSymbol( m_libName, new LIB_SYMBOL( *libSymbol ) );
-            wxCHECK( result == SYMBOL_LIB_TABLE::SAVE_OK, false );
-            aSymbolBuf->SetOriginal( new LIB_SYMBOL( *libSymbol ) );
-        }
-        else
-        {
-            LIB_SYMBOL* parentSymbol = new LIB_SYMBOL( *libSymbol );
-
-            aLibTable->SaveSymbol( m_libName, parentSymbol );
-
-            for( auto& entry : derivedSymbols )
-            {
-                std::shared_ptr<SYMBOL_BUFFER> symbol = GetBuffer( entry );
-
-                wxCHECK2( symbol, continue );
-
-                LIB_SYMBOL* derivedSymbol = new LIB_SYMBOL( *symbol->GetSymbol() );
-                derivedSymbol->SetParent( parentSymbol );
-                result = aLibTable->SaveSymbol( m_libName, derivedSymbol );
-                wxCHECK( result == SYMBOL_LIB_TABLE::SAVE_OK, false );
-            }
-        }
-    }
-
-    ++m_hash;
-    return true;
-}
-
-
-bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::SaveBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf,
-                                                     const wxString& aFileName,
-        SCH_PLUGIN* aPlugin, bool aBuffer )
-{
-    wxCHECK( aSymbolBuf, false );
-    LIB_SYMBOL* libSymbol = aSymbolBuf->GetSymbol();
-    LIB_SYMBOL* originalSymbol = aSymbolBuf->GetOriginal();
-    wxCHECK( libSymbol && originalSymbol, false );
     wxCHECK( !aFileName.IsEmpty(), false );
 
     wxString errorMsg = _( "Error saving symbol %s to library '%s'." ) + wxS( "\n%s" );
 
-    // set properties to prevent save file on every symbol save
+    // Set properties to prevent saving the file on every symbol save.
     STRING_UTF8_MAP properties;
     properties.emplace( SCH_LEGACY_PLUGIN::PropBuffering, "" );
 
+    std::shared_ptr<SYMBOL_BUFFER>& symbolBuf = aSymbolBuf;
+    LIB_SYMBOL* libSymbol = symbolBuf->GetSymbol();
+    LIB_SYMBOL* originalSymbol = symbolBuf->GetOriginal();
+
+    wxCHECK( libSymbol && originalSymbol, false );
+
     // Delete the original symbol if the symbol name has been changed.
     if( libSymbol->GetName() != originalSymbol->GetName() )
     {
@@ -1135,10 +1017,13 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::SaveBuffer( std::shared_ptr<SYMBOL_BUFF
         }
     }
 
+    LIB_SYMBOL* parentSymbol = nullptr;
+
     if( libSymbol->IsAlias() )
     {
         LIB_SYMBOL* newCachedSymbol = new LIB_SYMBOL( *libSymbol );
         std::shared_ptr< LIB_SYMBOL > bufferedParent = libSymbol->GetParent().lock();
+        parentSymbol = newCachedSymbol;
 
         wxCHECK( bufferedParent, false );
 
@@ -1210,29 +1095,8 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::SaveBuffer( std::shared_ptr<SYMBOL_BUFF
     }
     else
     {
-        wxArrayString derivedSymbols;
+        parentSymbol = new LIB_SYMBOL( *libSymbol );
 
-        if( GetDerivedSymbolNames( libSymbol->GetName(), derivedSymbols ) == 0 )
-        {
-            try
-            {
-                aPlugin->SaveSymbol( aFileName, new LIB_SYMBOL( *libSymbol ),
-                                     aBuffer ? &properties : nullptr );
-            }
-            catch( const IO_ERROR& ioe )
-            {
-                wxLogError( errorMsg, UnescapeString( libSymbol->GetName() ), aFileName,
-                            ioe.What() );
-                return false;
-            }
-
-            aSymbolBuf->SetOriginal( new LIB_SYMBOL( *libSymbol ) );
-        }
-        else
-        {
-            LIB_SYMBOL* parentSymbol = new LIB_SYMBOL( *libSymbol );
-
-            // Save the modified root symbol.
         try
         {
             aPlugin->SaveSymbol( aFileName, parentSymbol, aBuffer ? &properties : nullptr );
@@ -1245,7 +1109,13 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::SaveBuffer( std::shared_ptr<SYMBOL_BUFF
         }
 
         aSymbolBuf->SetOriginal( new LIB_SYMBOL( *libSymbol ) );
+    }
 
+    wxArrayString derivedSymbols;
+
+    // Reparent all symbols derived from the saved symbol.
+    if( GetDerivedSymbolNames( libSymbol->GetName(), derivedSymbols ) != 0 )
+    {
         // Save the derived symbols.
         for( const wxString& entry : derivedSymbols )
         {
@@ -1269,17 +1139,15 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::SaveBuffer( std::shared_ptr<SYMBOL_BUFF
             }
         }
     }
-    }
 
     ++m_hash;
     return true;
 }
 
 
-std::shared_ptr<SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER>
+std::shared_ptr<SYMBOL_BUFFER> LIB_BUFFER::GetBuffer( const wxString& aAlias ) const
-SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetBuffer( const wxString& aAlias ) const
 {
-    for( std::shared_ptr<SYMBOL_LIBRARY_MANAGER::SYMBOL_BUFFER> entry : m_symbols )
+    for( std::shared_ptr<SYMBOL_BUFFER> entry : m_symbols )
     {
         if( entry->GetSymbol()->GetName() == aAlias )
             return entry;
@@ -1289,9 +1157,9 @@ SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetBuffer( const wxString& aAlias ) const
 }
 
 
-bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::HasDerivedSymbols( const wxString& aParentName ) const
+bool LIB_BUFFER::HasDerivedSymbols( const wxString& aParentName ) const
 {
-    for( auto& entry : m_symbols )
+    for( const std::shared_ptr<SYMBOL_BUFFER>& entry : m_symbols )
     {
         if( entry->GetSymbol()->IsAlias() )
         {
@@ -1309,10 +1177,9 @@ bool SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::HasDerivedSymbols( const wxString& aPar
 }
 
 
-void SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetSymbolNames( wxArrayString& aSymbolNames,
+void LIB_BUFFER::GetSymbolNames( wxArrayString& aSymbolNames, SYMBOL_NAME_FILTER aFilter )
-                                                         SYMBOL_NAME_FILTER aFilter )
 {
-    for( auto& entry : m_symbols )
+    for( std::shared_ptr<SYMBOL_BUFFER>& entry : m_symbols )
     {
         if( ( entry->GetSymbol()->IsAlias() && ( aFilter == SYMBOL_NAME_FILTER::ROOT_ONLY ) )
           || ( entry->GetSymbol()->IsRoot() && ( aFilter == SYMBOL_NAME_FILTER::DERIVED_ONLY ) ) )
@@ -1323,12 +1190,11 @@ void SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetSymbolNames( wxArrayString& aSymbolN
 }
 
 
-size_t SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetDerivedSymbolNames( const wxString& aSymbolName,
+size_t LIB_BUFFER::GetDerivedSymbolNames( const wxString& aSymbolName, wxArrayString& aList )
-                                                                  wxArrayString& aList )
 {
     wxCHECK( !aSymbolName.IsEmpty(), 0 );
 
-    for( auto& entry : m_symbols )
+    for( std::shared_ptr<SYMBOL_BUFFER>& entry : m_symbols )
     {
         if( entry->GetSymbol()->IsAlias() )
         {
@@ -1350,7 +1216,7 @@ size_t SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::GetDerivedSymbolNames( const wxString
 }
 
 
-int SYMBOL_LIBRARY_MANAGER::LIB_BUFFER::removeChildSymbols( std::shared_ptr<SYMBOL_BUFFER>& aSymbolBuf )
+int LIB_BUFFER::removeChildSymbols( std::shared_ptr<SYMBOL_BUFFER>& aSymbolBuf )
 {
     wxCHECK( aSymbolBuf, 0 );
 

eeschema/symbol_library_manager.h

@@ -54,6 +54,121 @@ enum class SYMBOL_NAME_FILTER
 };
 
 
+class SYMBOL_BUFFER
+{
+public:
+    SYMBOL_BUFFER( LIB_SYMBOL* aSymbol = nullptr, std::unique_ptr<SCH_SCREEN> aScreen = nullptr );
+    ~SYMBOL_BUFFER();
+
+    LIB_SYMBOL* GetSymbol() const { return m_symbol; }
+    void        SetSymbol( LIB_SYMBOL* aSymbol );
+
+    LIB_SYMBOL* GetOriginal() const { return m_original; }
+    void        SetOriginal( LIB_SYMBOL* aSymbol );
+
+    bool        IsModified() const;
+    SCH_SCREEN* GetScreen() const { return m_screen.get(); }
+
+private:
+    std::unique_ptr<SCH_SCREEN> m_screen;
+
+    LIB_SYMBOL* m_symbol;   // Working copy
+    LIB_SYMBOL* m_original; // Initial state of the symbol
+};
+
+
+///< Store a working copy of a library.
+class LIB_BUFFER
+{
+public:
+    LIB_BUFFER( const wxString& aLibrary ) : m_libName( aLibrary ), m_hash( 1 ) {}
+
+    bool IsModified() const
+    {
+        if( !m_deleted.empty() )
+            return true;
+
+        for( const std::shared_ptr<SYMBOL_BUFFER>& symbolBuf : m_symbols )
+        {
+            if( symbolBuf->IsModified() )
+                return true;
+        }
+
+        return false;
+    }
+
+    int GetHash() const { return m_hash; }
+
+    ///< Return the working copy of a LIB_SYMBOL root object with specified alias.
+    LIB_SYMBOL* GetSymbol( const wxString& aAlias ) const;
+
+    ///< Create a new buffer to store a symbol. LIB_BUFFER takes ownership of aCopy.
+    bool CreateBuffer( LIB_SYMBOL* aCopy, SCH_SCREEN* aScreen );
+
+    ///< Update the buffered symbol with the contents of \a aCopy.
+    bool UpdateBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf, LIB_SYMBOL* aCopy );
+
+    bool DeleteBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf );
+
+    void ClearDeletedBuffer() { m_deleted.clear(); }
+
+    ///< Save stored modifications using a plugin. aBuffer decides whether the changes
+    ///< should be cached or stored directly to the disk (for SCH_LEGACY_PLUGIN).
+    bool SaveBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf, const wxString& aFileName,
+                     SCH_PLUGIN* aPlugin, bool aBuffer );
+
+    ///< Return a symbol buffer with LIB_SYMBOL holding a symbolicular alias
+    std::shared_ptr<SYMBOL_BUFFER> GetBuffer( const wxString& aAlias ) const;
+
+    ///< Return all buffered symbols
+    const std::deque<std::shared_ptr<SYMBOL_BUFFER>>& GetBuffers() const { return m_symbols; }
+
+    /**
+     * Check to see any symbols in the buffer are derived from a parent named \a aParentName.
+     *
+     * @param aParentName is the name of the parent to test.
+     * @return true if any symbols are found derived from a symbol named \a aParent, otherwise
+     *         false.
+     */
+    bool HasDerivedSymbols( const wxString& aParentName ) const;
+
+    /**
+     * Fetch a list of root symbols names from the library buffer.
+     *
+     * @param aRootSymbolNames is a reference to a list to populate with root symbol names.
+     * @param aFilter is the symbol derivation type.
+     */
+    void GetSymbolNames( wxArrayString&     aSymbolNames,
+                         SYMBOL_NAME_FILTER aFilter = SYMBOL_NAME_FILTER::ALL );
+
+    /**
+     * Fetch all of the symbols derived from a \a aSymbolName into \a aList.
+     *
+     * @param aSymbolName is the name of the symbol to search for derived symbols in this
+     *                    buffer.
+     * @param aList is the list of symbols names derived from \a aSymbolName.
+     * @return a size_t count of the number of symbols derived from \a aSymbolName.
+     */
+    size_t GetDerivedSymbolNames( const wxString& aSymbolName, wxArrayString& aList );
+
+private:
+    /**
+     * Remove all symbols derived from \a aParent from the library buffer.
+     *
+     * @param aParent is the #SYMBOL_BUFFER to check against.
+     * @return the count of #SYMBOL_BUFFER objects removed from the library.
+     */
+    int removeChildSymbols( std::shared_ptr<SYMBOL_BUFFER>& aSymbolBuf );
+
+    std::deque<std::shared_ptr<SYMBOL_BUFFER>> m_symbols;
+
+    ///< Buffer for deleted symbols until library is saved.
+    std::deque<std::shared_ptr<SYMBOL_BUFFER>> m_deleted;
+    const wxString                             m_libName; // Buffered library name
+    int                                        m_hash;
+};
+
+
 /**
  * Class to handle modifications to the symbol libraries.
  */
@@ -200,14 +315,6 @@ public:
      */
     bool IsLibraryReadOnly( const wxString& aLibrary ) const;
 
-    /**
-     * Save symbol changes to the library copy used by the schematic editor. Not it is not
-     * necessarily saved to the file.
-     *
-     * @return True on success, false otherwise.
-     */
-    bool FlushSymbol( const wxString& aAlias, const wxString& aLibrary );
-
     /**
      * Save library to a file, including unsaved changes.
      *
@@ -273,144 +380,6 @@ protected:
     SYMBOL_LIB_TABLE* symTable() const;
 
     ///< Class to store a working copy of a LIB_SYMBOL object and editor context.
-    class SYMBOL_BUFFER
-    {
-    public:
-        SYMBOL_BUFFER( LIB_SYMBOL* aSymbol = nullptr,
-                       std::unique_ptr<SCH_SCREEN> aScreen = nullptr );
-        ~SYMBOL_BUFFER();
-
-        LIB_SYMBOL* GetSymbol() const { return m_symbol; }
-        void SetSymbol( LIB_SYMBOL* aSymbol );
-
-        LIB_SYMBOL* GetOriginal() const { return m_original; }
-        void SetOriginal( LIB_SYMBOL* aSymbol );
-
-        bool IsModified() const;
-        SCH_SCREEN* GetScreen() const { return m_screen.get(); }
-
-        ///< Transfer the screen ownership
-        std::unique_ptr<SCH_SCREEN> RemoveScreen()
-        {
-            return std::move( m_screen );
-        }
-
-        bool SetScreen( std::unique_ptr<SCH_SCREEN> aScreen )
-        {
-            bool ret = !!m_screen;
-            m_screen = std::move( aScreen );
-            return ret;
-        }
-
-    private:
-        std::unique_ptr<SCH_SCREEN> m_screen;
-
-        LIB_SYMBOL* m_symbol;      // Working copy
-        LIB_SYMBOL* m_original;    // Initial state of the symbol
-    };
-
-
-    ///< Store a working copy of a library.
-    class LIB_BUFFER
-    {
-    public:
-        LIB_BUFFER( const wxString& aLibrary ) :
-                m_libName( aLibrary ),
-                m_hash( 1 )
-        { }
-
-        bool IsModified() const
-        {
-            if( !m_deleted.empty() )
-                return true;
-
-            for( const std::shared_ptr<SYMBOL_BUFFER>& symbolBuf : m_symbols )
-            {
-                if( symbolBuf->IsModified() )
-                    return true;
-            }
-
-            return false;
-        }
-
-        int GetHash() const { return m_hash; }
-
-        ///< Return the working copy of a LIB_SYMBOL root object with specified alias.
-        LIB_SYMBOL* GetSymbol( const wxString& aAlias ) const;
-
-        ///< Create a new buffer to store a symbol. LIB_BUFFER takes ownership of aCopy.
-        bool CreateBuffer( LIB_SYMBOL* aCopy, SCH_SCREEN* aScreen );
-
-        ///< Update the buffered symbol with the contents of \a aCopy.
-        bool UpdateBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf, LIB_SYMBOL* aCopy );
-
-        bool DeleteBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf );
-
-        void ClearDeletedBuffer()
-        {
-            m_deleted.clear();
-        }
-
-        ///< Save stored modifications to Symbol Lib Table. It may result in saving the symbol
-        ///< to disk as well, depending on the row properties.
-        bool SaveBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf, SYMBOL_LIB_TABLE* aLibTable );
-
-        ///< Save stored modifications using a plugin. aBuffer decides whether the changes
-        ///< should be cached or stored directly to the disk (for SCH_LEGACY_PLUGIN).
-        bool SaveBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf, const wxString& aFileName,
-                         SCH_PLUGIN* aPlugin, bool aBuffer );
-
-        ///< Return a symbol buffer with LIB_SYMBOL holding a symbolicular alias
-        std::shared_ptr<SYMBOL_BUFFER> GetBuffer( const wxString& aAlias ) const;
-
-        ///< Return all buffered symbols
-        const std::deque< std::shared_ptr<SYMBOL_BUFFER> >& GetBuffers() const { return m_symbols; }
-
-        /**
-         * Check to see any symbols in the buffer are derived from a parent named \a aParentName.
-         *
-         * @param aParentName is the name of the parent to test.
-         * @return true if any symbols are found derived from a symbol named \a aParent, otherwise
-         *         false.
-         */
-        bool HasDerivedSymbols( const wxString& aParentName ) const;
-
-        /**
-         * Fetch a list of root symbols names from the library buffer.
-         *
-         * @param aRootSymbolNames is a reference to a list to populate with root symbol names.
-         * @param aFilter is the symbol derivation type.
-         */
-        void GetSymbolNames( wxArrayString& aSymbolNames,
-                             SYMBOL_NAME_FILTER aFilter = SYMBOL_NAME_FILTER::ALL );
-
-        /**
-         * Fetch all of the symbols derived from a \a aSymbolName into \a aList.
-         *
-         * @param aSymbolName is the name of the symbol to search for derived symbols in this
-         *                    buffer.
-         * @param aList is the list of symbols names derived from \a aSymbolName.
-         * @return a size_t count of the number of symbols derived from \a aSymbolName.
-         */
-        size_t GetDerivedSymbolNames( const wxString& aSymbolName, wxArrayString& aList );
-
-    private:
-        /**
-         * Remove all symbols derived from \a aParent from the library buffer.
-         *
-         * @param aParent is the #SYMBOL_BUFFER to check against.
-         * @return the count of #SYMBOL_BUFFER objects removed from the library.
-         */
-        int removeChildSymbols( std::shared_ptr<SYMBOL_BUFFER>& aSymbolBuf );
-
-        std::deque< std::shared_ptr<SYMBOL_BUFFER> > m_symbols;
-
-        ///< Buffer for deleted symbols until library is saved.
-        std::deque< std::shared_ptr<SYMBOL_BUFFER> > m_deleted;
-        const wxString                               m_libName;  // Buffered library name
-        int                                          m_hash;
-    };
-
     /**
      * Return a set of #LIB_SYMBOL objects belonging to the original library.
      */

qa/tests/eeschema/CMakeLists.txt

@@ -3,7 +3,7 @@
 #
 # Copyright (C) 2017 CERN
 # @author Alejandro García Montoro <alejandro.garciamontoro@gmail.com>
-# Copyright (C) 2022 KiCad Developers, see AUTHORS.txt for contributors.
+# Copyright (C) 2022-2023 KiCad Developers, see AUTHORS.txt for contributors.
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -71,11 +71,13 @@ set( QA_EESCHEMA_SRCS
     test_sch_sheet_path.cpp
     test_sch_sheet_list.cpp
     test_sch_symbol.cpp
+    test_symbol_library_manager.cpp
 )
 
 if( WIN32 )
     # We want to declare a resource manifest on Windows to enable UTF8 mode
-    # Without UTF8 mode, some random IO tests may fail, we set the active code page on normal kicad to UTF8 as well
+    # Without UTF8 mode, some random IO tests may fail, we set the active code page on normal
+    # kicad to UTF8 as well.
     if( MINGW )
         # QA_EESCHEMA_RESOURCES variable is set by the macro.
         mingw_resource_compiler( qa_eeschema )

qa/tests/eeschema/test_symbol_library_manager.cpp

@@ -0,0 +1,141 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2023 Wayne Stambaugh <stambaughw@gmail.com>
+ * Copyright (C) 2023 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 3 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/**
+ * @file
+ * Test suite for SYMBOL_LIBRARY_MANAGER object.
+ */
+
+#include <qa_utils/wx_utils/unit_test_utils.h>
+
+// Code under test
+#include <symbol_library_manager.h>
+
+class SYMBOL_LIBRARY_MANAGER_TEST_FIXTURE
+{
+public:
+    SYMBOL_LIBRARY_MANAGER_TEST_FIXTURE()
+    {
+    }
+};
+
+
+BOOST_FIXTURE_TEST_SUITE( SymbolLibraryManager, SYMBOL_LIBRARY_MANAGER_TEST_FIXTURE )
+
+
+/**
+ * Test the SYMBOL_BUFFER object.
+ */
+BOOST_AUTO_TEST_CASE( SymbolBuffer )
+{
+    LIB_SYMBOL* symbol = new LIB_SYMBOL( wxS( "Symbol" ) );
+    std::unique_ptr<SCH_SCREEN> screen( new SCH_SCREEN() );
+
+    SYMBOL_BUFFER buffer( symbol, std::move( screen ) );
+
+    BOOST_CHECK( !buffer.IsModified() );
+    BOOST_CHECK( buffer.GetSymbol() == symbol );
+    BOOST_CHECK( *buffer.GetOriginal() == *symbol );
+
+    buffer.GetScreen()->SetContentModified();
+    BOOST_CHECK( buffer.IsModified() );
+
+    LIB_SYMBOL* originalSymbol = new LIB_SYMBOL( wxS( "OriginalSymbol" ) );
+    buffer.SetOriginal( originalSymbol );
+
+    BOOST_CHECK( buffer.GetOriginal() == originalSymbol );
+    BOOST_CHECK( *buffer.GetOriginal() == *originalSymbol );
+    BOOST_CHECK( *buffer.GetSymbol() != *buffer.GetOriginal() );
+
+    // Allocated memory is cleaned up by the SYMBOL_BUFFER object dtor.
+}
+
+
+/**
+ * Test the LIB_BUFFER object.
+ */
+BOOST_AUTO_TEST_CASE( LibBuffer )
+{
+    wxArrayString symbolNames;
+    LIB_BUFFER libBuffer( wxS( "TestLibrary" ) );
+
+    BOOST_CHECK( !libBuffer.IsModified() );
+    BOOST_CHECK_EQUAL( libBuffer.GetHash(), 1 );
+
+    LIB_SYMBOL* parentSymbol1 = new LIB_SYMBOL( wxS( "Parent1" ) );
+
+    BOOST_CHECK_EQUAL( libBuffer.GetSymbol( parentSymbol1->GetName() ), nullptr );
+
+    parentSymbol1->GetValueField().SetText( parentSymbol1->GetName() );
+    libBuffer.CreateBuffer( parentSymbol1, new SCH_SCREEN() );
+    BOOST_CHECK( libBuffer.GetSymbol( parentSymbol1->GetName() ) == parentSymbol1 );
+    BOOST_CHECK( !libBuffer.HasDerivedSymbols( parentSymbol1->GetName() ) );
+    BOOST_CHECK_EQUAL( libBuffer.GetHash(), 2 );
+
+    libBuffer.GetSymbolNames( symbolNames );
+    BOOST_CHECK_EQUAL( symbolNames.GetCount(), 1 );
+    BOOST_CHECK_EQUAL( symbolNames[0], parentSymbol1->GetName() );
+
+    symbolNames.Clear();
+    libBuffer.GetSymbolNames( symbolNames, SYMBOL_NAME_FILTER::ROOT_ONLY );
+    BOOST_CHECK_EQUAL( symbolNames.GetCount(), 1 );
+    BOOST_CHECK_EQUAL( symbolNames[0], parentSymbol1->GetName() );
+
+    symbolNames.Clear();
+    libBuffer.GetSymbolNames( symbolNames, SYMBOL_NAME_FILTER::DERIVED_ONLY );
+    BOOST_CHECK_EQUAL( symbolNames.GetCount(), 0 );
+
+    LIB_SYMBOL* childSymbol1 = new LIB_SYMBOL( wxS( "Child1" ) );
+    childSymbol1->SetParent( parentSymbol1 );
+    childSymbol1->GetValueField().SetText( childSymbol1->GetName() );
+    libBuffer.CreateBuffer( childSymbol1, new SCH_SCREEN() );
+    BOOST_CHECK( libBuffer.GetSymbol( childSymbol1->GetName() ) == childSymbol1 );
+    BOOST_CHECK( libBuffer.HasDerivedSymbols( parentSymbol1->GetName() ) );
+    BOOST_CHECK_EQUAL( libBuffer.GetHash(), 3 );
+
+    symbolNames.Clear();
+    libBuffer.GetSymbolNames( symbolNames );
+    BOOST_CHECK_EQUAL( symbolNames.GetCount(), 2 );
+    BOOST_CHECK_EQUAL( symbolNames[0], parentSymbol1->GetName() );
+    BOOST_CHECK_EQUAL( symbolNames[1], childSymbol1->GetName() );
+
+    symbolNames.Clear();
+    libBuffer.GetSymbolNames( symbolNames, SYMBOL_NAME_FILTER::DERIVED_ONLY );
+    BOOST_CHECK_EQUAL( symbolNames.GetCount(), 1 );
+    BOOST_CHECK_EQUAL( symbolNames[0], childSymbol1->GetName() );
+
+    symbolNames.Clear();
+    BOOST_CHECK_EQUAL( libBuffer.GetDerivedSymbolNames( parentSymbol1->GetName(),
+                                                        symbolNames ), 1 );
+    BOOST_CHECK_EQUAL( symbolNames[0], childSymbol1->GetName() );
+
+    std::shared_ptr<SYMBOL_BUFFER> buf = libBuffer.GetBuffer( parentSymbol1->GetName() );
+    LIB_SYMBOL* tmp = new LIB_SYMBOL( *parentSymbol1 );
+    tmp->GetDescriptionField().SetText( wxS( "Description" ) );
+    libBuffer.UpdateBuffer( buf, tmp );
+    BOOST_CHECK_EQUAL( libBuffer.GetHash(), 4 );
+    BOOST_CHECK( *libBuffer.GetSymbol( parentSymbol1->GetName() ) == *tmp );
+
+    libBuffer.DeleteBuffer( buf );
+    BOOST_CHECK( libBuffer.GetBuffers().empty() );
+}
+
+
+BOOST_AUTO_TEST_SUITE_END()