changeset 249:6a7eb102716d

Remove code duplication by unifying the certificatelist. You should now check for isInstallCert to determine wether this certificate should be installed or removed. Leaving the getInstallCertificates and getRemoveCertificates in place for compatibilty would have been easier to keep the tests stable.
author Andre Heinecke <aheinecke@intevation.de>
date Mon, 31 Mar 2014 08:06:17 +0000
parents 9f0865dc8b14
children 1e112cf41e92
files ui/certificatelist.cpp ui/certificatelist.h ui/listupdatedialog.cpp ui/mainwindow.cpp ui/tests/certlistparsertest.cpp ui/tests/cinstprocesstest.cpp ui/tests/windowsstoretest.cpp
diffstat 7 files changed, 53 insertions(+), 74 deletions(-) [+]
line wrap: on
line diff
--- a/ui/certificatelist.cpp	Mon Mar 31 08:03:20 2014 +0000
+++ b/ui/certificatelist.cpp	Mon Mar 31 08:06:17 2014 +0000
@@ -13,10 +13,10 @@
     char *data = NULL;
     size_t size = 0;
 
-    mCertificatesRemove.clear();
-    mCertificatesInstall.clear();
+    mCertificates.clear();
     mDate = QDateTime();
     mData = QString();
+    mFileName = QString::fromUtf8(fileName);
 
     mStatus = read_and_verify_list(fileName, &data, &size);
 
@@ -53,9 +53,9 @@
 
             mDate = QDateTime::fromMSecsSinceEpoch(timestamp * 1000);
         } else if (curLine.startsWith("I:")) {
-            mCertificatesInstall << Certificate(curLine);
+            mCertificates << Certificate(curLine);
         } else if (curLine.startsWith("R:")) {
-            mCertificatesRemove << Certificate(curLine);
+            mCertificates << Certificate(curLine);
         } else if (curLine.startsWith("S:")) {
             // Signature is verified in read_and_verify_list
             continue;
@@ -71,10 +71,7 @@
     readList(fileName);
 }
 
-const QList<Certificate>& CertificateList::getInstallCertificates() const {
-    return mCertificatesInstall;
+const QList<Certificate>& CertificateList::getCertificates() const
+{
+    return mCertificates;
 }
-
-const QList<Certificate>& CertificateList::getRemoveCertificates() const {
-    return mCertificatesRemove;
-}
--- a/ui/certificatelist.h	Mon Mar 31 08:03:20 2014 +0000
+++ b/ui/certificatelist.h	Mon Mar 31 08:06:17 2014 +0000
@@ -44,19 +44,16 @@
      */
     bool isValid() const {return mStatus == Valid;}
 
-    /* @brief get a list of certificate objects that should be installed */
-    const QList<Certificate>& getInstallCertificates() const;
-
-    /* @brief get a list of certificate objects that should be removed */
-    const QList<Certificate>& getRemoveCertificates() const;
+    /* @brief get a list of certificate objects in this list */
+    const QList<Certificate>& getCertificates() const;
 
     /* @brief get the raw certificate list */
     const QString& rawData() const {return mData;}
 
 private:
-    QList<Certificate> mCertificatesInstall;
-    QList<Certificate> mCertificatesRemove;
+    QList<Certificate> mCertificates;
     QString mData;
+    QString mFileName;
     list_status_t mStatus;
     QDateTime mDate;
 };
--- a/ui/listupdatedialog.cpp	Mon Mar 31 08:03:20 2014 +0000
+++ b/ui/listupdatedialog.cpp	Mon Mar 31 08:06:17 2014 +0000
@@ -59,7 +59,7 @@
     QGroupBox *certGroup = new QGroupBox(tr("Select certificates"));
     certGroup->setLayout(listLayout);
 
-    foreach (const Certificate& cert, mCertificateList.getInstallCertificates()) {
+    foreach (const Certificate& cert, mCertificateList.getCertificates()) {
         if (!cert.isValid()) {
             qWarning() << "Invalid certificate in list";
             continue;
@@ -69,27 +69,13 @@
         item->setData(Qt::ToolTipRole, cert.details());
         item->setData(Qt::UserRole, cert.base64Line());
         item->setCheckState(Qt::Checked);
-        QIcon *certIcon = new QIcon(":/img/list-add.png");
+        QIcon *certIcon = cert.isInstallCert() ? new QIcon(":/img/list-add.png") :
+                                                 new QIcon(":/img/list-remove.png");
         item->setIcon(*certIcon);
+
         mCertListWidget->addItem(item);
     }
 
-    foreach (const Certificate& cert, mCertificateList.getRemoveCertificates()) {
-        if (!cert.isValid()) {
-            qWarning() << "Invalid certificate in list";
-            continue;
-        }
-        QListWidgetItem* item = new QListWidgetItem(cert.shortDescription());
-        item->setFlags(item->flags() | Qt::ItemIsUserCheckable);
-        item->setData(Qt::ToolTipRole, cert.details());
-        item->setData(Qt::UserRole, cert.base64Line());
-        item->setCheckState(Qt::Checked);
-        QIcon *certIcon = new QIcon(":/img/list-remove.png");
-        item->setIcon(*certIcon);
-        mCertListWidget->addItem(item);
-    }
-
-
     /* Fill top level layout */
     topLayout->addLayout(headerLayout);
     topLayout->addWidget(certGroup);
--- a/ui/mainwindow.cpp	Mon Mar 31 08:03:20 2014 +0000
+++ b/ui/mainwindow.cpp	Mon Mar 31 08:06:17 2014 +0000
@@ -283,25 +283,15 @@
 {
     qDebug() << "display certificates";
     certificateList->clear();
-    foreach (const Certificate &cert, mListToInstall.getInstallCertificates()) {
+    foreach (const Certificate &cert, mListToInstall.getCertificates()) {
         if (!cert.isValid()) {
             qWarning() << "Invalid certificate in list";
             continue;
         }
         QListWidgetItem* item = new QListWidgetItem(cert.shortDescription());
         item->setData(Qt::UserRole, cert.details());
-        QIcon *certIcon = new QIcon(":/img/list-add.png");
-        item->setIcon(*certIcon);
-        certificateList->addItem(item);
-    }
-    foreach (const Certificate& cert, mListToInstall.getRemoveCertificates()) {
-        if (!cert.isValid()) {
-            qWarning() << "Invalid certificate in list";
-            continue;
-        }
-        QListWidgetItem* item = new QListWidgetItem(cert.shortDescription());
-        item->setData(Qt::UserRole, cert.details());
-        QIcon *certIcon = new QIcon(":/img/list-remove.png");
+        QIcon *certIcon = cert.isInstallCert() ? new QIcon(":/img/list-add.png"):
+                                                 new QIcon(":/img/list-remove.png");
         item->setIcon(*certIcon);
         certificateList->addItem(item);
     }
--- a/ui/tests/certlistparsertest.cpp	Mon Mar 31 08:03:20 2014 +0000
+++ b/ui/tests/certlistparsertest.cpp	Mon Mar 31 08:06:17 2014 +0000
@@ -23,8 +23,7 @@
 
     QVERIFY(certList->rawData() == validData.toLatin1());
 
-    const QList<Certificate> instList = certList->getInstallCertificates();
-    const QList<Certificate> remoList = certList->getRemoveCertificates();
+    const QList<Certificate> cList = certList->getCertificates();
 
     foreach (QString act, validData.split("\r\n")) {
         if (act.startsWith("I:")) {
@@ -37,20 +36,24 @@
         }
     }
 
-    foreach (const Certificate& cert, instList) {
+    int instCnt = 0,
+        remoCnt = 0;
+    foreach (const Certificate& cert, cList) {
         QVERIFY(cert.isValid());
-        QVERIFY(instLines.contains(cert.base64Line()));
-    }
-    foreach (const Certificate& cert, remoList) {
-        QVERIFY(cert.isValid());
-        QVERIFY(remoLines.contains(cert.base64Line()));
+        if (cert.isInstallCert()) {
+            QVERIFY(instLines.contains(cert.base64Line()));
+            instCnt++;
+        } else {
+            QVERIFY(remoLines.contains(cert.base64Line()));
+            remoCnt++;
+        }
     }
 
     /* Be variable if test data changes later */
-    QVERIFY(instList.size() >= 1);
-    QVERIFY(remoList.size() >= 1);
-    QVERIFY(instLines.size() == instList.size());
-    QVERIFY(remoLines.size() == remoList.size());
+    QVERIFY(instCnt >= 1);
+    QVERIFY(remoCnt >= 1);
+    QVERIFY(instLines.size() == instCnt);
+    QVERIFY(remoLines.size() == remoCnt);
 
     /* Check that a default certificate is invalid */
     Certificate cert;
@@ -58,10 +61,8 @@
 
     certList->readList(fileName.toLocal8Bit().data());
 
-    const QList<Certificate> instList2 = certList->getInstallCertificates();
-    const QList<Certificate> remoList2 = certList->getRemoveCertificates();
-    QVERIFY(instLines.size() == instList2.size());
-    QVERIFY(remoLines.size() == remoList2.size());
+    const QList<Certificate> cList2 = certList->getCertificates();
+    QVERIFY(instLines.size() + remoLines.size() == cList2.size());
 
     delete certList;
 }
--- a/ui/tests/cinstprocesstest.cpp	Mon Mar 31 08:03:20 2014 +0000
+++ b/ui/tests/cinstprocesstest.cpp	Mon Mar 31 08:06:17 2014 +0000
@@ -41,7 +41,7 @@
     installerProcess->write(validList.rawData().toLatin1());
     installerProcess->write("-----END CERTIFICATE LIST-----\r\n");
 
-    foreach (const Certificate &cert, validList.getInstallCertificates()) {
+    foreach (const Certificate &cert, validList.getCertificates()) {
         installerProcess->write(cert.base64Line().toLatin1());
         installerProcess->write("\r\n");
     }
@@ -62,7 +62,7 @@
     installerProcess->write("-----BEGIN CERTIFICATE LIST-----\r\n");
     installerProcess->write("-----END CERTIFICATE LIST-----\r\n");
 
-    foreach (const Certificate &cert, validList.getInstallCertificates()) {
+    foreach (const Certificate &cert, validList.getCertificates()) {
         installerProcess->write(cert.base64Line().toLatin1());
         installerProcess->write("\r\n");
     }
--- a/ui/tests/windowsstoretest.cpp	Mon Mar 31 08:03:20 2014 +0000
+++ b/ui/tests/windowsstoretest.cpp	Mon Mar 31 08:06:17 2014 +0000
@@ -46,16 +46,24 @@
     PCCERT_CONTEXT pCert = NULL;
     size_t i = 0;
 
-    foreach (const Certificate &cert, validList.getInstallCertificates()) {
+    QList<Certificate> instList;
+
+    foreach (const Certificate &cert, validList.getCertificates()) {
+        if (cert.isInstallCert())
+            instList << cert;
+    }
+
+
+    foreach (const Certificate &cert, instList) {
         strv_append (&to_install, cert.base64Line().toLatin1().constData() + 2,
                 cert.base64Line().size() - 2);
     }
 
     /* Just a quick check for str_append_str functionality */
-    QVERIFY((size_t) validList.getInstallCertificates().size() == strv_length(to_install));
+    QVERIFY((size_t) instList.size() == strv_length(to_install));
     for (i = 0; i < strv_length(to_install); i++) {
-        QVERIFY (validList.getInstallCertificates()[i].base64Line().right(
-                    validList.getInstallCertificates()[i].base64Line().size() - 2) ==
+        QVERIFY (instList[i].base64Line().right(
+                    instList[i].base64Line().size() - 2) ==
                 QString::fromLatin1(to_install[i]));
     }
 
@@ -66,7 +74,7 @@
         bool certFound = false;
         QByteArray data = QByteArray::fromRawData ((const char *)pCert->pbCertEncoded,
                 pCert->cbCertEncoded);
-        foreach (const Certificate &cert, validList.getInstallCertificates()) {
+        foreach (const Certificate &cert, instList) {
             QByteArray asn1data = QByteArray::fromBase64(
                     cert.base64Line().right(cert.base64Line().size() - 2).toLatin1());
             if (asn1data == data) {
@@ -76,7 +84,7 @@
         QVERIFY(certFound);
         i++;
     }
-    QVERIFY ((size_t)validList.getInstallCertificates().size() == i);
+    QVERIFY ((size_t)instList.size() == i);
 
     /* Remove all except one */
     for (i = 0; i < strv_length(to_install) - 1; i++) {
@@ -116,8 +124,8 @@
         QByteArray data = QByteArray::fromRawData((const char*) pCert->pbCertEncoded,
                 pCert->cbCertEncoded);
         QVERIFY (data.toBase64() != to_remove[0]);
-        for (int j = 0; j < validList.getInstallCertificates().size() - 1; j++) {
-            const Certificate &cert = validList.getInstallCertificates()[j];
+        for (int j = 0; j < instList.size() - 1; j++) {
+            const Certificate &cert = instList[j];
             QByteArray asn1data = QByteArray::fromBase64(
                     cert.base64Line().right(cert.base64Line().size() - 2).toLatin1());
             if (asn1data == data) {

http://wald.intevation.org/projects/trustbridge/