Multithreading UrlRequestInterceptor

- Add parse() free function to UrlRequestInterceptor
- hostlists are loaded in parallel via QtConcurrent

9 files changed, 141 insertions, 63 deletions

diff --git a/BUGS.md b/BUGS.md
index 53d77dd..635b39b 100644
--- a/BUGS.md
+++ b/BUGS.md
@@ -1,24 +1,29 @@
 ## Reporting bugs
 
-Please include the following when reporting bugs:
+### What to include
+A bug report should contain these 4 things:
 
-* Operating system used
-* Version of Qt used, any relevant information if you built it yourself
-* Detailed explanation of the bug: 
-  * what was done
-  * what was expected
-  * what happened
+#### Environment
+This includes program version; operating system used; version of Qt or other
+libraries; any relevant information if you built from source.
 
-An example of a bad bug report is:
+#### Steps to reproduce
+Give detailed steps on how to reproduce the problem. Start from the beginning,
+mention the actions involved, and be verbose.
 
-> Something happened, and when I pressed the button it broke.
+#### Expected result
+It's important to include the result you were expecting, as it might differ
+from the way the program was designed to work.
 
-An exmaple of a good bug report is:
-
-> When I opened xyz menu and pressed abc, nothing happened.
+#### Actual result
+It is also important to include a good description of the buggy behaviour
+itself as well, because it's possible that following your steps on a different
+system doesn't reproduce the issue.
 
 Send bug reports to _aqua at iserlohn dash fortress dot net_.
 
+_Adapted from the guide in the texmate repository._
+
 ## Known bugs
 
 ### Search terms in address bar
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1b9e25d..f582600 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -8,6 +8,7 @@ option(MercurialRepo "Get version information from .hg" ON)
 option(CompilerWarnings "Compiler warnings" ON)
 option(QtDeprecatedWarnings "Qt deprecated warnings" ON)
 option(UseLibCpp "Use libc++ over stdlibc++ (requires clang)" OFF)
+option(Tests "Enable/disable some basic autotests" ON)
 
 # Libraries
 find_package(Qt5 COMPONENTS Core Widgets Concurrent REQUIRED)
@@ -61,4 +62,10 @@ add_subdirectory(src)
 # configuration program
 add_subdirectory(config)
 
+if (Tests)
+    enable_testing()
+    find_package(Qt5 COMPONENTS Test REQUIRED)
+    add_subdirectory(test)
+endif()
+
 feature_summary(WHAT ALL)
diff --git a/src/webengine/urlinterceptor.cpp b/src/webengine/urlinterceptor.cpp
index 91919ea..e88e5b7 100644
--- a/src/webengine/urlinterceptor.cpp
+++ b/src/webengine/urlinterceptor.cpp
@@ -9,76 +9,76 @@
 #include "urlinterceptor.h"
 #include <QDir>
 #include <QTextStream>
+#include <QtConcurrent>
 
 UrlRequestInterceptor::UrlRequestInterceptor(const QString &path, QObject *parent)
     : QWebEngineUrlRequestInterceptor(parent)
 {
-#ifdef QT_DEBUG
-    qDebug("Reading request blocklist: %s", qUtf8Printable(path));
-#endif
-
     QDir hostsD(path);
     const QStringList hostFiles = hostsD.entryList(QDir::Files);
     for(const QString &file : hostFiles) {
-        qDebug("Parsing hosts.d/%s: %i", qUtf8Printable(file), parseHostfile(hostsD.absoluteFilePath(file)));
-    }
+        const QString absPath = hostsD.absoluteFilePath(file);
+        QtConcurrent::run([this, absPath]() {
+            auto r = parse(absPath);
+#ifdef QT_DEBUG
+            qDebug("Parsed %i rules from %s", r.count(), qUtf8Printable(absPath));
+#endif
 
-    qDebug("Total number of rules: %i", m_rules.count());
+            rulesLock.lock();
+            for(const auto &k : r.keys()) {
+                if(rules.contains(k)) {
+                    //
+                } else {
+                    rules.insert(k, r.value(k));
+                }
+            }
+            rulesLock.unlock();
+        });
+    }
 }
 
-UrlRequestInterceptor::~UrlRequestInterceptor()
-{
-    qDeleteAll(m_rules);
-}
+UrlRequestInterceptor::~UrlRequestInterceptor() = default;
 
 void UrlRequestInterceptor::interceptRequest(QWebEngineUrlRequestInfo &info)
 {
-    if(m_rules.contains(info.requestUrl().host())) {
-        if(m_rules.value(info.requestUrl().host())->isBlocking) {
-            info.block(true);
-        }
+    rulesLock.lock();
+    if(rules.contains(info.requestUrl().host())) {
+        info.block(rules.value(info.requestUrl().host()).isBlocking);
     }
+    rulesLock.unlock();
 }
 
-int UrlRequestInterceptor::parseHostfile(const QString &filename)
+QHash<QString, UrlRequestInterceptor::HostRule> parse(const QString &filename)
 {
-    int numRules = 0;
-    QFile file(filename);
+    QHash<QString, UrlRequestInterceptor::HostRule> rules;
 
-    // try to open the file
-    if(file.open(QIODevice::ReadOnly | QIODevice::Text)) {
+    QFile hostfile(filename);
+    if(hostfile.open(QIODevice::ReadOnly | QIODevice::Text)) {
 
         // with a QTextStream we can read lines without getting linebreaks at the end
-        QTextStream out(&file);
-        while(!out.atEnd()) {
-            const QString &line = out.readLine().trimmed();
+        QTextStream hostfile_stream(&hostfile);
+        while(!hostfile_stream.atEnd()) {
+            // read line and remove any whitespace at the end
+            const QString &line = hostfile_stream.readLine().trimmed();
 
             // skip comments and empty lines
-            if(!line.startsWith('#') && !line.isEmpty()) {
-                // format is <redirect> <host>
-                //0.0.0.0 host
-                QStringList parts = line.split(' ');
-                QString redirect = parts.at(0);
-                QString host = parts.at(1);
+            // everything else should be a rule
+            // format is <redirect> <host>
+            // 0.0.0.0 hostname
+            const QStringList &parts = line.split(' ');
+            const QString &redirect = parts.at(0);
 
-                if(m_rules.contains(host)) {
-                    qWarning("Duplicate rule %s", qUtf8Printable(line));
-                }
-
-                if(redirect == "0.0.0.0") {
-                    HostRule *rule = new HostRule;
-                    rule->isBlocking = true;
-                    m_rules.insert(host, rule);
-
-                    ++numRules;
-                } else {
-                    qDebug("Ignoring rule %s", qUtf8Printable(line));
+            for(const QString &host : parts.mid(1)) {
+                if(!rules.contains(host)) {
+                    UrlRequestInterceptor::HostRule rule{};
+                    rule.isBlocking = redirect == "0.0.0.0";
+                    rules.insert(host, rule);
                 }
             }
         }
 
-        // close once we're done with it
-        file.close();
+        hostfile.close();
     }
-    return numRules;
-}
+
+    return rules;
+};
\ No newline at end of file
diff --git a/src/webengine/urlinterceptor.h b/src/webengine/urlinterceptor.h
index b34d9ff..af15f99 100644
--- a/src/webengine/urlinterceptor.h
+++ b/src/webengine/urlinterceptor.h
@@ -10,6 +10,7 @@
 #define URLREQUESTINTERCEPTOR_H
 
 #include <QWebEngineUrlRequestInterceptor>
+#include <QMutex>
 
 class UrlRequestInterceptor : public QWebEngineUrlRequestInterceptor
 {
@@ -20,15 +21,15 @@ public:
     };
 
     explicit UrlRequestInterceptor(const QString &path, QObject *parent = nullptr);
-    ~UrlRequestInterceptor();
+    ~UrlRequestInterceptor() override;
 
-    void interceptRequest(QWebEngineUrlRequestInfo &info);
-
-public slots:
-    int parseHostfile(const QString &filename);
+    void interceptRequest(QWebEngineUrlRequestInfo &info) override;
 
 private:
-    QHash<QString, HostRule *> m_rules;
+    QHash<QString, HostRule> rules;
+    QMutex rulesLock;
 };
 
+QHash<QString, UrlRequestInterceptor::HostRule> parse(const QString &filename);
+
 #endif // URLREQUESTINTERCEPTOR_H
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
new file mode 100644
index 0000000..29c95ff
--- /dev/null
+++ b/test/CMakeLists.txt
@@ -0,0 +1,17 @@
+set(CMAKE_INCLUDE_CURRENT_DIR ON)
+set(CMAKE_AUTOMOC ON)
+set(CMAKE_AUTORCC ON)
+
+macro(create_test testname)
+    add_executable(${testname}
+            ${testname}.cpp ${testname}.h
+            autotests.qrc
+            ${ARGN})
+
+    target_include_directories(${testname} PRIVATE ../src)
+    target_link_libraries(${testname} Qt5::Test Qt5::Concurrent Qt5::WebEngineWidgets)
+
+    add_test(NAME smolbote-${testname} COMMAND ${testname})
+endmacro()
+
+create_test(HostlistTest ../src/webengine/urlinterceptor.cpp ../src/webengine/urlinterceptor.h)
\ No newline at end of file
diff --git a/test/HostlistTest.cpp b/test/HostlistTest.cpp
new file mode 100644
index 0000000..31ae11c
--- /dev/null
+++ b/test/HostlistTest.cpp
@@ -0,0 +1,25 @@
+#include "HostlistTest.h"
+
+void HostlistTest::initTestCase()
+{
+    rules = parse(":/autotests/data/hostlist.txt");
+}
+
+void HostlistTest::parse_ruleCount()
+{
+    QVERIFY(rules.count() == 3);
+}
+
+void HostlistTest::parse_blockSomehost()
+{
+    QVERIFY(rules.contains("somehost.org"));
+    QVERIFY(rules.value("somehost.org").isBlocking);
+}
+
+void HostlistTest::parse_blockHost2()
+{
+    QVERIFY(rules.contains("host2.org"));
+    QVERIFY(rules.value("host2.org").isBlocking);
+}
+
+QTEST_MAIN(HostlistTest)
diff --git a/test/HostlistTest.h b/test/HostlistTest.h
new file mode 100644
index 0000000..dcfd5a3
--- /dev/null
+++ b/test/HostlistTest.h
@@ -0,0 +1,16 @@
+#include <QtTest/QtTest>
+#include "webengine/urlinterceptor.h"
+
+class HostlistTest : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void initTestCase();
+    void parse_ruleCount();
+    void parse_blockSomehost();
+    void parse_blockHost2();
+
+private:
+    QHash<QString, UrlRequestInterceptor::HostRule> rules;
+};
diff --git a/test/autotests.qrc b/test/autotests.qrc
new file mode 100644
index 0000000..5817c00
--- /dev/null
+++ b/test/autotests.qrc
@@ -0,0 +1,5 @@
+<RCC>
+    <qresource prefix="/autotests">
+        <file>data/hostlist.txt</file>
+    </qresource>
+</RCC>
\ No newline at end of file
diff --git a/test/data/hostlist.txt b/test/data/hostlist.txt
new file mode 100644
index 0000000..d228e1d
--- /dev/null
+++ b/test/data/hostlist.txt
@@ -0,0 +1,2 @@
+0.0.0.0 somehost.org
+0.0.0.0 host1.org host2.org