Rewrite list/news/help download and backup file handling

6 files changed, 66 insertions, 190 deletions

diff --git a/src/dialog.cpp b/src/dialog.cpp
index d87753a..388290d 100644
--- a/src/dialog.cpp
+++ b/src/dialog.cpp
@@ -26,6 +26,8 @@
 
 static bool isProcessRunning(const QString &binary);
 
+static QDomDocument toDomDocument(const QString& what, const QByteArray& data, const QString& backupFile, const QString& mandatoryChild);
+
 Dialog::Dialog(QWidget *parent)
     : QDialog(parent), ui(new Ui::Dialog) {
 
@@ -460,44 +462,21 @@ void Dialog::downloadData(const QString& locationIds) {
     //
     // Download lecture XML
     FileDownloader::download(listUrl, [this](QNetworkReply::NetworkError err, const QByteArray& data) {
-        QString temp_filename;
-
-        if (err != QNetworkReply::NoError) {
-            qDebug() << "Error reading from URL: " << err;
-
-            QFile backup_file(TEMP_PATH_XML_LIST);
+        QList<Session*> sessions;
+        QDomDocument doc = toDomDocument(QStringLiteral("lecture list"), data, TEMP_PATH_XML_LIST, QStringLiteral("settings"));
+        sessions = VSession::loadFromXmlDocument(doc);
 
-            if (!backup_file.open(QIODevice::ReadOnly)) {
-                qDebug() << "Cannot read backup file " << TEMP_PATH_XML_LIST << " either";
-                this->removeStatusString(STR_LOADING);
+        this->removeStatusString(STR_LOADING);
+        if (sessions.isEmpty()) {
+            if (err == QNetworkReply::NoError) {
+                this->addStatusString(STR_NO_ITEMS);
+            } else {
                 this->addStatusString(STR_URL_ERROR);
-                return;
             }
-            qDebug() << "Using backup file " << TEMP_PATH_XML_LIST;
-            backup_file.close();
-
-            temp_filename = TEMP_PATH_XML_LIST;
         } else {
-            // write xml to temporary file
-            temp_filename = QDir::tempPath() + "/vmchooser2/vmchooser-XXXXXX.xml";
-            QTemporaryFile tmpfile(temp_filename);
-            if (!tmpfile.open() || tmpfile.write(data) == -1) {
-                return;
-            }
-            tmpfile.close();
-            tmpfile.setAutoRemove(false);
-            temp_filename = tmpfile.fileName();
-        }
-
-        QList<Session*> sessions = VSession::readXmlFile(temp_filename);
-
-        this->removeStatusString(STR_LOADING);
-
-        if (!sessions.isEmpty()) {
-            qSort(sessions.begin(), sessions.end(), myLessThan);
+            qSort(sessions.begin(), sessions.end(), sessionComparator);
             this->addItems(sessions, TAB_ALL_VMS);
-            // TODO: Filter user's classes and add to tab[TAB_MY_COURSES]
-            bool showEdit = false;
+            bool showEdit = false; // Only show edit button if at least one lecture is editable
             for (QList<Session*>::const_iterator it = sessions.begin(); it != sessions.end(); ++it) {
                 if (reinterpret_cast<VSession*>(*it)->canEdit()) {
                     showEdit = true;
@@ -507,8 +486,6 @@ void Dialog::downloadData(const QString& locationIds) {
             if (showEdit) {
                 ui->chkAdminMode->setVisible(true);
             }
-        } else {
-            this->addStatusString(STR_NO_ITEMS);
         }
 
         checkAutostart();
@@ -522,133 +499,46 @@ void Dialog::downloadData(const QString& locationIds) {
     FileDownloader::download(QUrl(Config::isSet(Config::URL_NEWS)
                                   ? Config::get(Config::URL_NEWS)
                                   : Config::get(Config::URL_BASE).append("/news")), [this](QNetworkReply::NetworkError err, const QByteArray& data) {
-        if (err != QNetworkReply::NoError) {
-            if (g_debugMode) {
-                qDebug() << "Could not get news. Try to get cached news.";
-            }
-
-            // try to get cached news
-            QFile backup_file(TEMP_PATH_NEWS);
-
-            if (!backup_file.open(QIODevice::ReadOnly)) {
-                if (g_debugMode) {
-                    qDebug() << "Cannot read backup file " << TEMP_PATH_NEWS << " either";
-                }
-                this->ui->newsTextBrowser->setText(QCoreApplication::instance()->translate("Dialog", "Could not get news."));
-                return;
-            }
-            if (g_debugMode) {
-                qDebug() << "Used backup file " << TEMP_PATH_NEWS;
-            }
-            backup_file.close();
-            return;
-        }
-        QDomDocument doc;
-        if (!doc.setContent(data)) {
-            qDebug() << "News XML contains errors.";
-            return;
-        }
+        QDomDocument doc = toDomDocument(QStringLiteral("news"), data, TEMP_PATH_NEWS, QStringLiteral("news"));
         QDomElement newsNode = doc.firstChildElement("news");
         QDomElement timeNode = newsNode.firstChildElement("date");
         QDomElement infoNode = newsNode.firstChildElement("info");
+        QDomElement headlineNode = newsNode.firstChildElement("headline");
         QDateTime timestamp;
         timestamp.setTime_t(timeNode.text().toUInt());
 
         if (timeNode.isNull() || infoNode.isNull()) {
+            qDebug() << "Could not load news. Network:" << err;
             ui->newsTextBrowser->setText(QCoreApplication::instance()->translate("Dialog", "Could not get news. (//news/date or //news/info missing)"));
         } else {
             // format and print news
-            ui->newsTextBrowser->setText(QString("<p style='font-size:16px; margin-bottom: 2px;'>" + newsNode.firstChildElement("headline").text() + "</p> <small>"
+            ui->newsTextBrowser->setText(QString("<p style='font-size:16px; margin-bottom: 2px;'>" + headlineNode.text() + "</p> <small>"
                     + timestamp.toString(Qt::SystemLocaleShortDate) + "</small><p>"
                     + infoNode.text() + "</p>"));
         }
 
-        if (ui->helpBox->isHidden() && UserConfig::getLastNewsTime() < timestamp.toTime_t()) {
+        if (ui->helpBox->isHidden()
+                && (UserConfig::getLastNewsTime() < timestamp.toTime_t() || UserConfig::getLastNewsTime() > QDateTime::currentMSecsSinceEpoch() / 1000)) {
             // show news if not seen before
             on_helpNewsButton_clicked();
         }
 
         // update ini
         UserConfig::setLastNewsTime(timestamp.toTime_t());
-
-        // make backup
-        QFile file(TEMP_PATH_NEWS);
-        if (!file.open(QIODevice::WriteOnly)) {
-            if (g_debugMode) {
-                qDebug() << "Could not write XML to " << TEMP_PATH_NEWS;
-            }
-            return;
-        }
-
-        if (file.write(data) != data.length()) {
-            return;
-        }
-        if (!file.setPermissions(QFile::ReadUser | QFile::ReadGroup | QFile::ReadOther | QFile::WriteUser | QFile::WriteGroup | QFile::WriteOther)) {
-            if (g_debugMode) {
-                qDebug() << "Could not change permissions of file: " << TEMP_PATH_NEWS;
-            }
-        }
-        file.close();
     });
     //
     // Download help
     FileDownloader::download(QUrl(Config::isSet(Config::URL_HELP)
                                   ? Config::get(Config::URL_HELP)
                                   : Config::get(Config::URL_BASE).append("/help")), [this](QNetworkReply::NetworkError err, const QByteArray& data) {
-        if (err != QNetworkReply::NoError) {
-            if (g_debugMode) {
-                qDebug() << "Could not get help xml. Try to get cached help...";
-            }
-
-            // try to get cached news
-            QFile backup_file(TEMP_PATH_HELP);
-
-            if (!backup_file.open(QIODevice::ReadOnly)) {
-                if (g_debugMode) {
-                    qDebug() << "Cannot read backup file " << TEMP_PATH_HELP << " either";
-                }
-                this->ui->helpTextBrowser->setText(QCoreApplication::instance()->translate("Dialog", "Could not get help."));
-                return;
-            }
-            if (g_debugMode) {
-                qDebug() << "Used backup file " << TEMP_PATH_HELP;
-            }
-            backup_file.close();
-            return;
-        }
-        QDomDocument doc;
-        if (!doc.setContent(data)) {
-            if (g_debugMode) {
-                qDebug() << "Help file contains errors.";
-            }
-            return;
-        }
-
-        QDomElement newsNode = doc.firstChildElement("news").firstChildElement("info");
-        if (newsNode.isNull()) {
+        QDomDocument doc = toDomDocument(QStringLiteral("help"), data, TEMP_PATH_HELP, QStringLiteral("news"));
+        QDomElement helpTextNode = doc.firstChildElement("news").firstChildElement("info");
+        if (helpTextNode.isNull()) {
+            qDebug() << "Could not load help. Network:" << err;
             ui->helpTextBrowser->setText(QCoreApplication::instance()->translate("Dialog", "Could not get help (XML has no //news/info)"));
         } else {
-            ui->helpTextBrowser->setText(newsNode.text());
-        }
-
-        // make backup
-        QFile file(TEMP_PATH_HELP);
-        if (!file.open(QIODevice::WriteOnly)) {
-            if (g_debugMode) {
-                qDebug() << "Could not write XML to " << TEMP_PATH_HELP;
-            }
-            return;
-        }
-
-        if (file.write(data) != data.length()) {
-            return;
-        }
-        if (!file.setPermissions(QFile::ReadUser | QFile::ReadGroup | QFile::ReadOther | QFile::WriteUser | QFile::WriteGroup | QFile::WriteOther)) {
-            if (g_debugMode) {
-                qDebug() << "Could not change permissions of file: " << TEMP_PATH_HELP;
-            }
+            ui->helpTextBrowser->setText(helpTextNode.text());
         }
-        file.close();
     });
     //
 }
@@ -950,3 +840,38 @@ static bool isProcessRunning(const QString &binary)
 	}
 	return false;
 }
+
+static QDomDocument toDomDocument(const QString& what, const QByteArray& data, const QString& backupFile, const QString& mandatoryChild)
+{
+    QDomDocument doc;
+    if (!data.isEmpty()) {
+        if (!doc.setContent(data)) {
+            qDebug() << "Downloaded" << what << "XML contains errors.";
+        }
+    } else {
+        qDebug() << "No content downloaded for" << what;
+    }
+    QFile backup(backupFile);
+    if (doc.isNull() || !doc.hasChildNodes()) {
+        if (backup.open(QFile::ReadOnly)) {
+            if (!doc.setContent(&backup)) {
+                qDebug() << "Could not load" << what << "backup.";
+            }
+        }
+    }
+    if (doc.isNull() || !doc.hasChildNodes())
+        return QDomDocument(); // Above methods failed
+    if (doc.firstChildElement(mandatoryChild).isNull()) {
+        qDebug() << "Downloaded" << what << "xml doesn't contain mandatory root node" << mandatoryChild;
+        return QDomDocument();
+    }
+    if (!backup.isOpen()) {
+        // If it were already open we'd have read the backup, so no need to write it out again...
+        if (!backup.open(QFile::WriteOnly)) {
+            qDebug() << "Cannot open" << what << "backup file" << backupFile << "for writing";
+        } else {
+            backup.write(data);
+        }
+    }
+    return doc;
+}
diff --git a/src/globals.cpp b/src/globals.cpp
index 7780e8a..a20f52c 100644
--- a/src/globals.cpp
+++ b/src/globals.cpp
@@ -21,6 +21,6 @@ int g_forLocationHandling = LOCATION_BUMP;
 
 bool g_noVtx = false;
 
-bool myLessThan(Session* a, Session* b) {
+bool sessionComparator(Session* a, Session* b) {
     return *a < *b;
 }
diff --git a/src/globals.h b/src/globals.h
index 3b4f344..a95ca59 100644
--- a/src/globals.h
+++ b/src/globals.h
@@ -42,6 +42,6 @@ extern int g_forLocationHandling;
 
 extern bool g_noVtx;
 
-bool myLessThan(Session* a, Session* b);
+bool sessionComparator(Session* a, Session* b);
 
 #endif
diff --git a/src/main.cpp b/src/main.cpp
index 9874f00..df7309c 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -117,7 +117,7 @@ int main(int argc, char *argv[]) {
     w.resize(width, height);
 
     if (xsessions.size()) {
-        qSort(xsessions.begin(), xsessions.end(), myLessThan);
+        qSort(xsessions.begin(), xsessions.end(), sessionComparator);
         w.addItems(xsessions, 0);
     }
 
diff --git a/src/vsession.cpp b/src/vsession.cpp
index 78185e4..b86f314 100644
--- a/src/vsession.cpp
+++ b/src/vsession.cpp
@@ -285,64 +285,15 @@ int VSession::type() const {
     return Session::VSESSION;
 }
 
-QList<Session*> VSession::readXmlFile(const QString& filepath) {
+QList<Session*> VSession::loadFromXmlDocument(const QDomDocument& doc) {
     QList<Session*> sessionList;
-
-    QDomDocument doc;
-    QFile file(filepath);
-    QFile backup_file(TEMP_PATH_XML_LIST);
-
-    if (!file.open(QIODevice::ReadOnly)) {
-        if (g_debugMode) {
-            qDebug() << "Cannot read file: " << file.fileName();
-        }
+    if (doc.isNull())
         return sessionList;
-    }
-
-    if (!doc.setContent(&file)) {
-        if (g_debugMode) {
-            qDebug() << "XML file not valid: " << file.fileName();
-        }
-
-        file.close();
-
-        // try to use backup file
-        if (!backup_file.open(QIODevice::ReadOnly)) {
-            if (g_debugMode) {
-                qDebug() << "Cannot read backup file " << TEMP_PATH_XML_LIST << " either";
-            }
-            return sessionList;
-        }
-
-        if (!doc.setContent(&backup_file)) {
-            if (g_debugMode) {
-                qDebug() << "XML file not valid: " << backup_file.fileName();
-            }
-            backup_file.close();
-            return sessionList;
-        }
-
-        if (g_debugMode) {
-            qDebug() << "Used backup file " << TEMP_PATH_XML_LIST;
-        }
-
-        backup_file.close();
-    } else {
-        file.close();
-
-        // file is valid --> create backup file
-        QFile::remove(TEMP_PATH_XML_LIST);
-        QFile::rename(filepath, TEMP_PATH_XML_LIST);
-        if (!QFile::setPermissions(TEMP_PATH_XML_LIST, QFile::ReadUser | QFile::ReadGroup | QFile::ReadOther | QFile::WriteUser | QFile::WriteGroup | QFile::WriteOther)) {
-            if (g_debugMode) {
-                qDebug() << "Could not change permissions of file: " << TEMP_PATH_NEWS;
-            }
-        }
-    }
 
     UserLdapData::init();
-
     QDomElement settingsNode = doc.firstChildElement(QStringLiteral("settings"));
+    if (settingsNode.isNull())
+        return sessionList;
     for (QDomElement el(settingsNode.firstChildElement(QStringLiteral("eintrag")));
             !el.isNull();
             el = el.nextSiblingElement(QStringLiteral("eintrag"))) {
diff --git a/src/vsession.h b/src/vsession.h
index fa3014d..69ecf13 100644
--- a/src/vsession.h
+++ b/src/vsession.h
@@ -83,7 +83,7 @@ class VSession : public Session {
 
     bool operator<(const Session& other) const;
 
-    static QList<Session*> readXmlFile(const QString& filepath);
+    static QList<Session*> loadFromXmlDocument(const QDomDocument& doc);
 
 protected:
     virtual QString checkCanRunInternal() const;