Issue 6477 - separate done and error callbacks in IFileSystem::Read

test/BaseJsTest.h

Index: test/BaseJsTest.h
diff --git a/test/BaseJsTest.h b/test/BaseJsTest.h
index 7762ab22cd10f741d304637a4eb6ebecb8c664d8..8ac8920ba86166d48718234748c6489b85d60440 100644
--- a/test/BaseJsTest.h
+++ b/test/BaseJsTest.h
@@ -108,7 +108,7 @@ public:
 class ThrowingFileSystem : public AdblockPlus::IFileSystem
 {
 public:
-  void Read(const std::string& fileName, const ReadCallback& callback) const override
+  void Read(const std::string& fileName, const ReadCallback& callback, const Callback& errorCallback) const override
   {
     throw std::runtime_error("Not implemented");
   }
@@ -160,20 +160,22 @@ public:
   {
   }
 
-  void Read(const std::string& fileName, const ReadCallback& callback) const override
+  void Read(const std::string& fileName, const ReadCallback& callback, const Callback& errorCallback) const override
   {
-    scheduler([fileName, callback]
+    scheduler([fileName, callback, errorCallback]
     {

[sergei, 2018/03/23 11:27:19]

Strictly speaking, now in accordance with the interface one should catch
exceptions and call errorCallback if there is some, e.g. see
https://codereview.adblockplus.org/29731562/diff/29731589/test/FileSystemJsOb...,
MockFileSystem::Read.

I left it without try-catch because it's not supposed that anything results in
an exception here, so it seems rather a dead-code, so IMO it should be rather
simple and one should find out what is happening earlier (because of less
conditions) in tests. Please let me know if you think that every implementation
of IFileSystem::Read, including all tests, should contain that try-catch, namely
calling errorCallback if there is an exception.

       if (fileName == "patterns.ini")
       {
         std::string dummyData = "# Adblock Plus preferences\n[Subscription]\nurl=~user~0000";
-        callback(IOBuffer(dummyData.cbegin(), dummyData.cend()), "");
+        callback(IOBuffer(dummyData.cbegin(), dummyData.cend()));
       }
       else if (fileName == "prefs.json")
       {
         std::string dummyData = "{}";
-        callback(IOBuffer(dummyData.cbegin(), dummyData.cend()), "");
+        callback(IOBuffer(dummyData.cbegin(), dummyData.cend()));
       }
+      else
+        errorCallback("File not found, " + fileName);
     });
   }
 
@@ -213,17 +215,15 @@ class InMemoryFileSystem : public LazyFileSystem
   std::map<std::string, IOBuffer> files;
 public:
   using LazyFileSystem::LazyFileSystem;
-  void Read(const std::string& fileName, const ReadCallback& callback) const override
+  void Read(const std::string& fileName, const ReadCallback& callback, const Callback& errorCallback) const override
   {
-    scheduler([this, fileName, callback]()
+    scheduler([this, fileName, callback, errorCallback]()
     {
       auto ii_file = files.find(fileName);
-      if (ii_file == files.end())
-      {
-        callback(IOBuffer(), "File not found, " + fileName);
-        return;
-      }
-      callback(IOBuffer(ii_file->second), "");
+      if (ii_file != files.end())
+        callback(IOBuffer(ii_file->second));
+      else
+        errorCallback("File not found, " + fileName);
     });
   }