Fixed|FS|libcore: Problem populating subfolders

The folder population method assumes that the folder structure is created outside the operation. If folders are returned among the populated files, they will be discarded as pre-existing and will end up destroyed. Reverted back to the correct population procedure in DirectoryFeed.
skyjake · Sep 1, 2019 · 1ebcb21 · 1ebcb21
1 parent c69d80d
commit 1ebcb21
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 32 deletions.
diff --git a/doomsday/libs/core/include/de/filesys/directoryfeed.h b/doomsday/libs/core/include/de/filesys/directoryfeed.h
@@ -130,8 +130,8 @@ class DE_PUBLIC DirectoryFeed : public Feed
     static File &manuallyPopulateSingleFile(NativePath const &nativePath, Folder &parentFolder);
 
 protected:
-    Folder *populateSubFolder(const Folder &folder, const String &entryName);
-    void    populateFile(const Folder &folder, const String &entryName, PopulatedFiles &populated);
+    void populateSubFolder(const Folder &folder, const String &entryName);
+    void populateFile(const Folder &folder, const String &entryName, PopulatedFiles &populated);
 
 private:
     DE_PRIVATE(d)

diff --git a/doomsday/libs/core/src/filesys/directoryfeed.cpp b/doomsday/libs/core/src/filesys/directoryfeed.cpp
@@ -107,10 +107,7 @@ Feed::PopulatedFiles DirectoryFeed::populate(Folder const &folder)
 
         if (isDirectory_FileInfo(i.value))
         {
-            if (auto *f = populateSubFolder(folder, name))
-            {
-                populated << f;
-            }
+            populateSubFolder(folder, name);
         }
         else
         {
@@ -123,7 +120,7 @@ Feed::PopulatedFiles DirectoryFeed::populate(Folder const &folder)
     return populated;
 }
 
-Folder *DirectoryFeed::populateSubFolder(const Folder &folder, const String &entryName)
+void DirectoryFeed::populateSubFolder(const Folder &folder, const String &entryName)
 {
     LOG_AS("DirectoryFeed::populateSubFolder");
 
@@ -132,25 +129,11 @@ Folder *DirectoryFeed::populateSubFolder(const Folder &folder, const String &ent
         Folder *subFolder = folder.tryLocate<Folder>(entryName);
         if (!subFolder)
         {
-            subFolder = new Folder(entryName);
-
-            // Technically folders could be interpreted, but that would require knowing
-            // their source data at this time, and we only know the file name.
-            //subFolder = &folder.fileSystem().interpret(subFolder)->as<Folder>();
-
-            subFolder->attach(newSubFeed(entryName));
-            //folder.add(subFolder);
-//            subFolder = &folder.fileSystem()
-//                    .makeFolderWithFeed(folder.path() / entryName,
-//                                        newSubFeed(entryName),
-//                                        Folder::PopulateFullTree,
-//                                        FS::DontInheritFeeds);
+            subFolder = &FS::get().makeFolderWithFeed(folder.path() / entryName,
+                                                      newSubFeed(entryName),
+                                                      Folder::PopulateFullTree,
+                                                      FS::DontInheritFeeds);
         }
-//        else
-//        {
-//            // Use the previously populated subfolder.
-//            subFolder = &folder.locate<Folder>(entryName);
-//        }
         if (d->mode & AllowWrite)
         {
             subFolder->setMode(File::Write);
@@ -159,9 +142,7 @@ Folder *DirectoryFeed::populateSubFolder(const Folder &folder, const String &ent
         {
             subFolder->setMode(File::ReadOnly);
         }
-        return subFolder;
     }
-    return nullptr;
 }
 
 void DirectoryFeed::populateFile(Folder const &folder, String const &entryName,

diff --git a/doomsday/libs/core/src/filesys/folder.cpp b/doomsday/libs/core/src/filesys/folder.cpp
@@ -210,6 +210,9 @@ void Folder::clear()
 
 void Folder::populate(PopulationBehaviors behavior)
 {
+    // Only folders in the file system tree can be populated.
+    DE_ASSERT(parent() || this == &FS::get().root());
+
     if (!behavior.testFlag(DisableIndexing))
     {
         fileSystem().changeBusyLevel(+1);
@@ -260,7 +263,7 @@ void Folder::populate(PopulationBehaviors behavior)
             }
 
             if (mustPrune)
-            {
+            {               
                 // It needs to go.
                 file->setParent(nullptr);
                 iter = d->contents.erase(iter);
@@ -289,7 +292,7 @@ void Folder::populate(PopulationBehaviors behavior)
             {
                 if (i)
                 {
-                    std::unique_ptr<File> file(i);
+                    std::unique_ptr<File> file(i);                    
                     if (!d->contents.contains(i->name().lower()))
                     {
                         d->add(file.release());
@@ -562,19 +565,19 @@ void Folder::waitForPopulation(WaitBehavior waitBehavior)
     }
 }
 
-AsyncTask *Folder::afterPopulation(std::function<void ()> func)
+AsyncTask *Folder::afterPopulation(std::function<void()> func)
 {
     if (!isPopulatingAsync())
     {
         func();
         return nullptr;
     }
-    return async([] ()
+    return async([]()
     {
         waitForPopulation();
         return 0;
     },
-    [func] (int)
+    [func](int)
     {
         func();
     });