Merge branch 'memoryfs-iter' into window-id

Fix bug in wxMemoryFSHandler iteration.

See https://github.com/wxWidgets/wxWidgets/pull/1833

src/common/fs_mem.cpp

@@ -123,6 +123,10 @@ wxFSFile * wxMemoryFSHandlerBase::OpenFile(wxFileSystem& WXUNUSED(fs),
 
 wxString wxMemoryFSHandlerBase::FindFirst(const wxString& url, int flags)
 {
+    // Make sure to reset the find iterator, so that calling FindNext() doesn't
+    // reuse its value from the last search that could well be invalid.
+    m_findIter = m_Hash.end();
+
     if ( (flags & wxDIR) && !(flags & wxFILE) )
     {
         // we only store files, not directories, so we don't risk finding

tests/filesys/filesystest.cpp

@@ -24,6 +24,8 @@
 
 #if wxUSE_FILESYSTEM
 
+#include "wx/fs_mem.h"
+
 // ----------------------------------------------------------------------------
 // helpers
 // ----------------------------------------------------------------------------
@@ -48,35 +50,10 @@ public:
 
 
 // ----------------------------------------------------------------------------
-// test class
+// tests themselves
 // ----------------------------------------------------------------------------
 
-class FileSystemTestCase : public CppUnit::TestCase
+TEST_CASE("wxFileSystem::URLParsing", "[filesys][url][parse]")
-{
-public:
-    FileSystemTestCase() { }
-
-private:
-    CPPUNIT_TEST_SUITE( FileSystemTestCase );
-        CPPUNIT_TEST( UrlParsing );
-        CPPUNIT_TEST( FileNameToUrlConversion );
-        CPPUNIT_TEST( UnicodeFileNameToUrlConversion );
-    CPPUNIT_TEST_SUITE_END();
-
-    void UrlParsing();
-    void FileNameToUrlConversion();
-    void UnicodeFileNameToUrlConversion();
-
-    wxDECLARE_NO_COPY_CLASS(FileSystemTestCase);
-};
-
-// register in the unnamed registry so that these tests are run by default
-CPPUNIT_TEST_SUITE_REGISTRATION( FileSystemTestCase );
-
-// also include in its own registry so that these tests can be run alone
-CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( FileSystemTestCase, "FileSystemTestCase" );
-
-void FileSystemTestCase::UrlParsing()
 {
     static const struct Data
     {
@@ -151,14 +128,15 @@ void FileSystemTestCase::UrlParsing()
     for ( size_t n = 0; n < WXSIZEOF(data); n++ )
     {
         const Data& d = data[n];
-        CPPUNIT_ASSERT_EQUAL( d.protocol, tst.Protocol(d.url) );
+        INFO( "Testing URL " << wxString(d.url) );
-        CPPUNIT_ASSERT_EQUAL( d.left, tst.LeftLocation(d.url) );
+        CHECK( tst.Protocol(d.url) == d.protocol );
-        CPPUNIT_ASSERT_EQUAL( d.right, tst.RightLocation(d.url) );
+        CHECK( tst.LeftLocation(d.url) == d.left );
-        CPPUNIT_ASSERT_EQUAL( d.anchor, tst.Anchor(d.url) );
+        CHECK( tst.RightLocation(d.url) == d.right );
+        CHECK( tst.Anchor(d.url) == d.anchor );
     }
 }
 
-void FileSystemTestCase::FileNameToUrlConversion()
+TEST_CASE("wxFileSystem::FileNameToUrlConversion", "[filesys][url][filename]")
 {
     const static struct Data {
         const wxChar *input, *expected;
@@ -179,12 +157,13 @@ void FileSystemTestCase::FileNameToUrlConversion()
         wxFileName fn1(d.input);
         wxString url1 = wxFileSystem::FileNameToURL(fn1);
 
-        CPPUNIT_ASSERT_EQUAL( d.expected, url1 );
+        INFO( "Testing URL " << wxString(d.input) );
-        CPPUNIT_ASSERT( fn1.SameAs(wxFileName::URLToFileName(url1)) );
+        CHECK( url1 == d.expected );
+        CHECK( fn1.SameAs(wxFileName::URLToFileName(url1)) );
     }
 }
 
-void FileSystemTestCase::UnicodeFileNameToUrlConversion()
+TEST_CASE("wxFileSystem::UnicodeFileNameToUrlConversion", "[filesys][url][filename][unicode]")
 {
     const unsigned char filename_utf8[] = {
               0x4b, 0x72, 0xc3, 0xa1, 0x73, 0x79, 0x50, 0xc5,
@@ -196,7 +175,55 @@ void FileSystemTestCase::UnicodeFileNameToUrlConversion()
 
     wxString url = wxFileSystem::FileNameToURL(filename);
 
-    CPPUNIT_ASSERT( filename.SameAs(wxFileName::URLToFileName(url)) );
+    CHECK( filename.SameAs(wxFileName::URLToFileName(url)) );
+}
+
+// Test that using FindFirst() after removing a previously found URL works:
+// this used to be broken, see https://trac.wxwidgets.org/ticket/18744
+TEST_CASE("wxFileSystem::MemoryFSHandler", "[filesys][memoryfshandler][find]")
+{
+    // Install wxMemoryFSHandler just for the duration of this test.
+    class AutoMemoryFSHandler
+    {
+    public:
+        AutoMemoryFSHandler()
+            : m_handler(new wxMemoryFSHandler())
+        {
+            wxFileSystem::AddHandler(m_handler);
+        }
+
+        ~AutoMemoryFSHandler()
+        {
+            wxFileSystem::RemoveHandler(m_handler);
+        }
+
+    private:
+        wxMemoryFSHandler* const m_handler;
+    } autoMemoryFSHandler;
+
+    wxMemoryFSHandler::AddFile("foo.txt", "foo contents");
+    wxMemoryFSHandler::AddFile("bar.txt", "bar contents");
+    wxFileSystem fs;
+
+    wxString const url = fs.FindFirst("memory:*.txt");
+    INFO("Found URL was: " << url);
+
+    wxString filename;
+    REQUIRE( url.StartsWith("memory:", &filename) );
+
+    // We don't know which of the two files will be found, this depends on the
+    // details of the hash table implementation and varies across builds, so
+    // handle both cases and remove the other file, which should certainly
+    // invalidate the iterator pointing to it.
+    if ( filename == "foo.txt" )
+        wxMemoryFSHandler::RemoveFile("bar.txt");
+    else if ( filename == "bar.txt" )
+        wxMemoryFSHandler::RemoveFile("foo.txt");
+    else
+        FAIL("Unexpected filename: " << filename);
+
+    CHECK( fs.FindFirst(url) == url );
+    CHECK( fs.FindNext() == "" );
 }
 
 #endif // wxUSE_FILESYSTEM