Abstract LIB_TABLE IO to allow non-file-based tables

Mostly intended right now for allowing testing of library tables
to help with testing chained loading, but it also decouples the
idea of a library table from on-disk files in general.

All current (real) lib table implementations continue to use the
file-based IO.

This could be made more general (not just for tables) if really
needed.

common/lib_table_base.cpp

@@ -40,6 +40,55 @@
 using namespace LIB_TABLE_T;
 
 
+std::unique_ptr<LINE_READER> FILE_LIB_TABLE_IO::GetReader( const wxString& aURI ) const
+{
+    const wxFileName fn( aURI );
+
+    if( !fn.IsOk() || !fn.IsFileReadable() )
+        return nullptr;
+
+    return std::make_unique<FILE_LINE_READER>( aURI );
+}
+
+bool FILE_LIB_TABLE_IO::CanSaveToUri( const wxString& aURI ) const
+{
+    const wxFileName fn( aURI );
+
+    if( !fn.IsOk() )
+        return false;
+
+    return fn.IsFileWritable();
+}
+
+bool FILE_LIB_TABLE_IO::UrisAreEquivalent( const wxString& aURI1, const wxString& aURI2 ) const
+{
+    // Avoid comparing filenames as wxURIs
+    if( aURI1.Find( "://" ) != wxNOT_FOUND )
+    {
+        // found as full path
+        return aURI1 == aURI2;
+    }
+    else
+    {
+        const wxFileName fn1( aURI1 );
+        const wxFileName fn2( aURI2 );
+
+        // This will also test if the file is a symlink so if we are comparing
+        // a symlink to the same real file, the comparison will be true.  See
+        // wxFileName::SameAs() in the wxWidgets source.
+
+        // found as full path and file name
+        return fn1 == fn2;
+    }
+}
+
+std::unique_ptr<OUTPUTFORMATTER> FILE_LIB_TABLE_IO::GetWriter( const wxString& aURI ) const
+{
+    const wxFileName fn( aURI );
+    return std::make_unique<FILE_OUTPUTFORMATTER>( aURI );
+}
+
+
 LIB_TABLE_ROW* new_clone( const LIB_TABLE_ROW& aRow )
 {
     return aRow.clone();
@@ -114,9 +163,15 @@ void LIB_TABLE_ROW::SetOptions( const wxString& aOptions )
 }
 
 
-LIB_TABLE::LIB_TABLE( LIB_TABLE* aFallBackTable ) :
-    m_fallBack( aFallBackTable ), m_version( 0 )
+LIB_TABLE::LIB_TABLE( LIB_TABLE* aFallBackTable, std::unique_ptr<LIB_TABLE_IO> aTableIo ) :
+        m_io( std::move( aTableIo ) ), m_fallBack( aFallBackTable ), m_version( 0 )
 {
+    // If not given, use the default file-based I/O.
+    if( !m_io )
+    {
+        m_io = std::make_unique<FILE_LIB_TABLE_IO>();
+    }
+
     // not copying fall back, simply search aFallBackTable separately
     // if "nickName not found".
 }
@@ -243,23 +298,10 @@ const LIB_TABLE_ROW* LIB_TABLE::FindRowByURI( const wxString& aURI )
     {
         for( unsigned i = 0; i < cur->m_rows.size(); i++ )
         {
-            wxString tmp = cur->m_rows[i].GetFullURI( true );
+            const wxString tmp = cur->m_rows[i].GetFullURI( true );
 
-            if( tmp.Find( "://" ) != wxNOT_FOUND )
-            {
-                if( tmp == aURI )
-                    return &cur->m_rows[i];  // found as URI
-            }
-            else
-            {
-                wxFileName fn = aURI;
-
-                // This will also test if the file is a symlink so if we are comparing
-                // a symlink to the same real file, the comparison will be true.  See
-                // wxFileName::SameAs() in the wxWidgets source.
-                if( fn == wxFileName( tmp ) )
-                    return &cur->m_rows[i];  // found as full path and file name
-            }
+            if( m_io->UrisAreEquivalent( tmp, aURI ) )
+                return &cur->m_rows[i];
         }
 
         // not found, search fall back table(s), if any
@@ -464,15 +506,16 @@ void LIB_TABLE::Load( const wxString& aFileName )
     std::lock_guard<std::shared_mutex> lock( m_mutex );
     clear();
 
+    std::unique_ptr<LINE_READER> reader = m_io->GetReader( aFileName );
+
     // It's OK if footprint library tables are missing.
-    if( wxFileName::IsFileReadable( aFileName ) )
+    if( reader )
     {
-        FILE_LINE_READER reader( aFileName );
-        LIB_TABLE_LEXER  lexer( &reader );
+        LIB_TABLE_LEXER lexer( reader.get() );
 
         Parse( &lexer );
 
-        if( m_version != 7 && migrate() && wxFileName::IsFileWritable( aFileName ) )
+        if( m_version != 7 && migrate() && m_io->CanSaveToUri( aFileName ) )
             Save( aFileName );
 
         reindex();
@@ -482,11 +525,16 @@ void LIB_TABLE::Load( const wxString& aFileName )
 
 void LIB_TABLE::Save( const wxString& aFileName ) const
 {
-    FILE_OUTPUTFORMATTER sf( aFileName );
+    std::unique_ptr<OUTPUTFORMATTER> sf = m_io->GetWriter( aFileName );
+
+    if( !sf )
+    {
+        THROW_IO_ERROR( wxString::Format( _( "Failed to get writer for %s" ), aFileName ) );
+    }
 
     // Force the lib table version to 7 before saving
     m_version = 7;
-    Format( &sf, 0 );
+    Format( sf.get(), 0 );
 }
 
 

include/lib_table_base.h

@@ -58,6 +58,65 @@ typedef LIB_TABLE_ROWS::const_iterator     LIB_TABLE_ROWS_CITER;
 KICOMMON_API LIB_TABLE_ROW* new_clone( const LIB_TABLE_ROW& aRow );
 
 
+/**
+ * LIB_TABLE_IO abstracts the file I/O operations for the library table
+ * loading and saving.
+ *
+ * Normally, this is file-based-reading, but that's not a requirement.
+ */
+class KICOMMON_API LIB_TABLE_IO
+{
+public:
+    virtual ~LIB_TABLE_IO() = default;
+
+    /**
+     * Create a reader for the given URI.
+     *
+     * This can return nullptr, for example if the URI is not a file or
+     * is not readable.
+     */
+    virtual std::unique_ptr<LINE_READER> GetReader( const wxString& aURI ) const = 0;
+
+    /**
+     * Check if the given URI is writable.
+     */
+    virtual bool CanSaveToUri( const wxString& aURI ) const = 0;
+
+    /**
+     * Compare two URIs for equivalence.
+     *
+     * For example, two URIs that point to the same file should be considered equivalent,
+     * even if they are not string-wise equal (e.g. symlinks)
+     */
+    virtual bool UrisAreEquivalent( const wxString& aURI1, const wxString& aURI2 ) const = 0;
+
+    /**
+     * Save the given table to the given URI.
+     */
+    virtual std::unique_ptr<OUTPUTFORMATTER> GetWriter( const wxString& aURI ) const = 0;
+};
+
+
+/**
+ * Implementations of LIB_TABLE_IO for file-based I/O.
+ *
+ * This is the default implementation for real usage.
+ */
+class KICOMMON_API FILE_LIB_TABLE_IO : public LIB_TABLE_IO
+{
+public:
+    FILE_LIB_TABLE_IO() = default;
+
+    std::unique_ptr<LINE_READER> GetReader( const wxString& aURI ) const override;
+
+    bool CanSaveToUri( const wxString& aURI ) const override;
+
+    bool UrisAreEquivalent( const wxString& aURI1, const wxString& aURI2 ) const override;
+
+    std::unique_ptr<OUTPUTFORMATTER> GetWriter( const wxString& aURI ) const override;
+};
+
+
 /**
  * Hold a record identifying a library accessed by the appropriate plug in object in the
  * #LIB_TABLE.  This is an abstract base class from which to derive library specific rows.
@@ -328,8 +387,11 @@ public:
      * @param aFallBackTable is another LIB_TABLE which is searched only when
      *                       a row is not found in this table.  No ownership is
      *                       taken of aFallBackTable.
+     * @param aTableIo is the I/O object to use for the table data. nullptr
+     *                 means use the default file-based I/O.
      */
-    LIB_TABLE( LIB_TABLE* aFallBackTable = nullptr );
+    LIB_TABLE( LIB_TABLE*                    aFallBackTable = nullptr,
+               std::unique_ptr<LIB_TABLE_IO> aTableIo = nullptr );
 
     virtual ~LIB_TABLE();
 
@@ -560,6 +622,9 @@ protected:
     void reindex();
 
 protected:
+    // Injected I/O interface
+    std::unique_ptr<LIB_TABLE_IO> m_io;
+
     LIB_TABLE* m_fallBack;
 
     /// Versioning to handle importing old tables

include/richio.h

@@ -328,8 +328,6 @@ protected:
         quoteChar[1] = '\0';
     }
 
-    virtual ~OUTPUTFORMATTER() {}
-
     /**
      * Perform quote character need determination according to the Specctra DSN specification.
 
@@ -363,6 +361,11 @@ protected:
 #endif
 
 public:
+    /**
+     * This is a polymorphic class that can validly be handled by a pointer to the base class.
+     */
+    virtual ~OUTPUTFORMATTER() {}
+
     /**
      * Format and write text to the output stream.
      *

qa/tests/common/test_lib_table.cpp

@@ -35,6 +35,40 @@
 // Code under test
 #include <lib_table_base.h>
 
+namespace
+{
+
+/**
+ * A very simple implementation of #LIB_TABLE_IO that does nothing.
+ *
+ * If needed, this could be extended to provide some basic functionality
+ * like providing test data to be read.
+ */
+class DUMMY_LIB_TABLE_IO : public LIB_TABLE_IO
+{
+public:
+    std::unique_ptr<LINE_READER> GetReader( const wxString& aURI ) const override
+    {
+        return std::make_unique<STRING_LINE_READER>( "", "DUMMY_LIB_TABLE_IO Data" );
+    }
+
+    bool CanSaveToUri( const wxString& aURI ) const override
+    {
+        // Always return true, it'll just write to a dummy string
+        return true;
+    }
+
+    bool UrisAreEquivalent( const wxString& aURI1, const wxString& aURI2 ) const override
+    {
+        return aURI1 == aURI2;
+    }
+
+    std::unique_ptr<OUTPUTFORMATTER> GetWriter( const wxString& aURI ) const override
+    {
+        return std::make_unique<STRING_FORMATTER>();
+    }
+};
+
 
 /**
  * A concrete implementation of #LIB_TABLE_ROW that implements
@@ -79,7 +113,8 @@ private:
 class TEST_LIB_TABLE : public LIB_TABLE
 {
 public:
-    TEST_LIB_TABLE( LIB_TABLE* aFallback = nullptr ) : LIB_TABLE( aFallback )
+    TEST_LIB_TABLE( LIB_TABLE* aFallback = nullptr ) :
+            LIB_TABLE( aFallback, std::make_unique<DUMMY_LIB_TABLE_IO>() )
     {
     }
 
@@ -199,6 +234,8 @@ struct LIB_TABLE_TEST_FIXTURE
     TEST_LIB_TABLE m_fallbackTable;
 };
 
+} // namespace
+
 /**
  * Declare the test suite
  */