Implement cache typing and versioning

While the new cache architecture has a type and version field for the global
header, I totally forgot to add the same thing to mark the type and version
of the actual cached data.
There was a version field for Package/Intent/ApplicationInfo, but this was
badly implemented and could crash the system trying to load an incompatible
cache file, because the version check was done too late.

Change-Id: I755cc5101213b60996541ea7e872e3017550ec37
Reviewed-by: Dominik Holland <dominik.holland@qt.io>

13 files changed, 123 insertions, 51 deletions

diff --git a/src/application-lib/applicationinfo.cpp b/src/application-lib/applicationinfo.cpp
index c3f5ec0d..e8282f15 100644
--- a/src/application-lib/applicationinfo.cpp
+++ b/src/application-lib/applicationinfo.cpp
@@ -51,8 +51,6 @@
 
 QT_BEGIN_NAMESPACE_AM
 
-static constexpr quint32 ApplicationInfoDataStreamVersion = 2;
-
 //TODO Make this really unique
 static int uniqueCounter = 0;
 static int nextUniqueNumber() {
@@ -94,10 +92,15 @@ QVariantMap ApplicationInfo::allAppProperties() const
     return m_allAppProperties;
 }
 
+
+const quint32 ApplicationInfo::DataStreamVersion = 3;
+
+
 void ApplicationInfo::writeToDataStream(QDataStream &ds) const
 {
-    ds << ApplicationInfoDataStreamVersion
-       << m_id
+    //NOTE: increment DataStreamVersion above, if you make any changes here
+
+    ds << m_id
        << m_uniqueNumber
        << m_sysAppProperties
        << m_allAppProperties
@@ -113,11 +116,11 @@ void ApplicationInfo::writeToDataStream(QDataStream &ds) const
 
 ApplicationInfo *ApplicationInfo::readFromDataStream(PackageInfo *pkg, QDataStream &ds)
 {
+    //NOTE: increment DataStreamVersion above, if you make any changes here
+
     QScopedPointer<ApplicationInfo> app(new ApplicationInfo(pkg));
-    auto dataStreamVersion = ApplicationInfoDataStreamVersion;
 
-    ds >> dataStreamVersion
-       >> app->m_id
+    ds >> app->m_id
        >> app->m_uniqueNumber
        >> app->m_sysAppProperties
        >> app->m_allAppProperties
@@ -130,9 +133,6 @@ ApplicationInfo *ApplicationInfo::readFromDataStream(PackageInfo *pkg, QDataStre
        >> app->m_dltConfiguration
        >> app->m_supportedMimeTypes;
 
-    if (dataStreamVersion != ApplicationInfoDataStreamVersion)
-        return nullptr;
-
     uniqueCounter = qMax(uniqueCounter, app->m_uniqueNumber);
     app->m_capabilities.sort();
 
diff --git a/src/application-lib/applicationinfo.h b/src/application-lib/applicationinfo.h
index 58451145..04f96d56 100644
--- a/src/application-lib/applicationinfo.h
+++ b/src/application-lib/applicationinfo.h
@@ -62,6 +62,8 @@ class ApplicationInfo
 public:
     ApplicationInfo(PackageInfo *packageInfo);
 
+    static const quint32 DataStreamVersion;
+
     PackageInfo *packageInfo() const;
 
     QVariantMap toVariantMap() const;
diff --git a/src/application-lib/intentinfo.cpp b/src/application-lib/intentinfo.cpp
index 1097f526..3395d7f5 100644
--- a/src/application-lib/intentinfo.cpp
+++ b/src/application-lib/intentinfo.cpp
@@ -47,8 +47,6 @@
 
 QT_BEGIN_NAMESPACE_AM
 
-static constexpr quint32 IntentInfoDataStreamVersion = 1;
-
 
 IntentInfo::IntentInfo(PackageInfo *packageInfo)
     : m_packageInfo(packageInfo)
@@ -112,10 +110,15 @@ QString IntentInfo::icon() const
     return m_icon.isEmpty() ? m_packageInfo->icon() : m_icon;
 }
 
+
+const quint32 IntentInfo::DataStreamVersion = 2;
+
+
 void IntentInfo::writeToDataStream(QDataStream &ds) const
 {
-    ds << IntentInfoDataStreamVersion
-       << m_id
+    //NOTE: increment DataStreamVersion above, if you make any changes here
+
+    ds << m_id
        << (m_visibility == Public ? qSL("public") : qSL("private"))
        << m_requiredCapabilities
        << m_parameterMatch
@@ -128,12 +131,12 @@ void IntentInfo::writeToDataStream(QDataStream &ds) const
 
 IntentInfo *IntentInfo::readFromDataStream(PackageInfo *pkg, QDataStream &ds)
 {
+    //NOTE: increment DataStreamVersion above, if you make any changes here
+
     QScopedPointer<IntentInfo> intent(new IntentInfo(pkg));
     QString visibilityStr;
-    auto dataStreamVersion = IntentInfoDataStreamVersion;
 
-    ds >> dataStreamVersion
-       >> intent->m_id
+    ds >> intent->m_id
        >> visibilityStr
        >> intent->m_requiredCapabilities
        >> intent->m_parameterMatch
@@ -143,9 +146,6 @@ IntentInfo *IntentInfo::readFromDataStream(PackageInfo *pkg, QDataStream &ds)
        >> intent->m_descriptions
        >> intent->m_icon;
 
-    if (dataStreamVersion != IntentInfoDataStreamVersion)
-        return nullptr;
-
     intent->m_visibility = (visibilityStr == qSL("public")) ? Public : Private;
     intent->m_categories.sort();
 
diff --git a/src/application-lib/intentinfo.h b/src/application-lib/intentinfo.h
index 9834e42c..1f8cdb7c 100644
--- a/src/application-lib/intentinfo.h
+++ b/src/application-lib/intentinfo.h
@@ -63,6 +63,8 @@ public:
     IntentInfo(PackageInfo *packageInfo);
     ~IntentInfo();
 
+    static const quint32 DataStreamVersion;
+
     enum Visibility {
         Public,
         Private
diff --git a/src/application-lib/packagedatabase.cpp b/src/application-lib/packagedatabase.cpp
index c7622fbf..8dfdba06 100644
--- a/src/application-lib/packagedatabase.cpp
+++ b/src/application-lib/packagedatabase.cpp
@@ -194,7 +194,8 @@ void PackageDatabase::parse()
         if (!m_loadFromCache && !m_saveToCache)
             cacheOptions |= AbstractConfigCache::NoCache;
 
-        ConfigCache<PackageInfo> cache(manifestFiles, qSL("appdb"), cacheOptions);
+        ConfigCache<PackageInfo> cache(manifestFiles, qSL("appdb"), "MANI",
+                                       PackageInfo::DataStreamVersion, cacheOptions);
         cache.parse();
 
         for (int i = 0; i < manifestFiles.size(); ++i) {
diff --git a/src/application-lib/packageinfo.cpp b/src/application-lib/packageinfo.cpp
index fddf268d..05946096 100644
--- a/src/application-lib/packageinfo.cpp
+++ b/src/application-lib/packageinfo.cpp
@@ -53,8 +53,6 @@
 
 QT_BEGIN_NAMESPACE_AM
 
-static constexpr quint32 PackageInfoDataStreamVersion = 1;
-
 
 PackageInfo::PackageInfo()
 { }
@@ -167,8 +165,16 @@ void PackageInfo::setInstallationReport(InstallationReport *report)
     m_installationReport.reset(report);
 }
 
+
+const quint32 PackageInfo::DataStreamVersion = 2 \
+        + (ApplicationInfo::DataStreamVersion << 8) \
+        + (IntentInfo::DataStreamVersion << 16);
+
+
 void PackageInfo::writeToDataStream(QDataStream &ds) const
 {
+    //NOTE: increment DataStreamVersion above, if you make any changes here
+
     QByteArray serializedReport;
 
     if (auto report = installationReport()) {
@@ -177,8 +183,7 @@ void PackageInfo::writeToDataStream(QDataStream &ds) const
         report->serialize(&buffer);
     }
 
-    ds << PackageInfoDataStreamVersion
-       << m_id
+    ds << m_id
        << m_names
        << m_icon
        << m_descriptions
@@ -200,14 +205,14 @@ void PackageInfo::writeToDataStream(QDataStream &ds) const
 
 PackageInfo *PackageInfo::readFromDataStream(QDataStream &ds)
 {
+    //NOTE: increment DataStreamVersion above, if you make any changes here
+
     QScopedPointer<PackageInfo> pkg(new PackageInfo);
 
     QString baseDir;
     QByteArray serializedReport;
-    auto dataStreamVersion = PackageInfoDataStreamVersion;
 
-    ds >> dataStreamVersion
-       >> pkg->m_id
+    ds >> pkg->m_id
        >> pkg->m_names
        >> pkg->m_icon
        >> pkg->m_descriptions
@@ -218,9 +223,6 @@ PackageInfo *PackageInfo::readFromDataStream(QDataStream &ds)
        >> baseDir
        >> serializedReport;
 
-    if (dataStreamVersion != PackageInfoDataStreamVersion)
-        return nullptr;
-
     pkg->m_baseDir.setPath(baseDir);
 
     if (!serializedReport.isEmpty()) {
diff --git a/src/application-lib/packageinfo.h b/src/application-lib/packageinfo.h
index 1c59f64e..795d1eef 100644
--- a/src/application-lib/packageinfo.h
+++ b/src/application-lib/packageinfo.h
@@ -67,6 +67,8 @@ class PackageInfo
 public:
     ~PackageInfo();
 
+    static const quint32 DataStreamVersion;
+
     void validate() const Q_DECL_NOEXCEPT_EXPR(false);
 
     QString id() const;
diff --git a/src/common-lib/configcache.cpp b/src/common-lib/configcache.cpp
index 6eec593b..fb90564e 100644
--- a/src/common-lib/configcache.cpp
+++ b/src/common-lib/configcache.cpp
@@ -79,13 +79,13 @@ QDataStream &operator<<(QDataStream &ds, const ConfigCacheEntry &ce)
 
 QDataStream &operator>>(QDataStream &ds, CacheHeader &ch)
 {
-    ds >> ch.magic >> ch.version >> ch.globalId >> ch.baseName >> ch.entries;
+    ds >> ch.magic >> ch.version >> ch.typeId >> ch.typeVersion >> ch.baseName >> ch.entries;
     return ds;
 }
 
 QDataStream &operator<<(QDataStream &ds, const CacheHeader &ch)
 {
-    ds << ch.magic << ch.version << ch.globalId << ch.baseName << ch.entries;
+    ds << ch.magic << ch.version << ch.typeId << ch.typeVersion << ch.baseName << ch.entries;
     return ds;
 }
 
@@ -98,23 +98,34 @@ QDebug operator<<(QDebug dbg, const ConfigCacheEntry &ce)
 }
 
 
-// this could be used in the future to have multiple AM instances that each have their own cache
-quint64 CacheHeader::s_globalId = 0;
+static quint32 makeTypeId(const char typeIdStr[4])
+{
+    if (typeIdStr) {
+        return (quint32(typeIdStr[0])) | (quint32(typeIdStr[1]) << 8)
+                | (quint32(typeIdStr[2]) << 16) | (quint32(typeIdStr[3]) << 24);
+    } else {
+        return 0;
+    }
+}
 
-bool CacheHeader::isValid(const QString &baseName) const
+bool CacheHeader::isValid(const QString &baseName, quint32 typeId, quint32 typeVersion) const
 {
     return magic == Magic
             && version == Version
-            && globalId == s_globalId
+            && this->typeId == typeId
+            && this->typeVersion == typeVersion
             && this->baseName == baseName
             && entries < 1000;
 }
 
 
-AbstractConfigCache::AbstractConfigCache(const QStringList &configFiles, const QString &cacheBaseName, Options options)
+AbstractConfigCache::AbstractConfigCache(const QStringList &configFiles, const QString &cacheBaseName,
+                                         const char typeId[4], quint32 version, Options options)
     : d(new ConfigCachePrivate)
 {
     d->options = options;
+    d->typeId = makeTypeId(typeId);
+    d->typeVersion = version;
     d->rawFiles = configFiles;
     d->cacheBaseName = cacheBaseName;
 }
@@ -190,7 +201,7 @@ void AbstractConfigCache::parse()
 
                 if (ds.status() != QDataStream::Ok)
                     throw Exception("failed to read cache header");
-                if (!cacheHeader.isValid(d->cacheBaseName))
+                if (!cacheHeader.isValid(d->cacheBaseName, d->typeId, d->typeVersion))
                     throw Exception("failed to parse cache header");
 
                 cache.resize(int(cacheHeader.entries));
@@ -226,7 +237,7 @@ void AbstractConfigCache::parse()
                     }
                     cacheIsComplete = true;
                 }
-
+                d->cacheWasRead = true;
 
             } catch (const Exception &e) {
                 qWarning(LogCache) << "Failed to read cache:" << e.what();
@@ -360,6 +371,8 @@ void AbstractConfigCache::parse()
                 QDataStream ds(&cacheFile);
                 CacheHeader cacheHeader;
                 cacheHeader.baseName = d->cacheBaseName;
+                cacheHeader.typeId = d->typeId;
+                cacheHeader.typeVersion = d->typeVersion;
                 cacheHeader.entries = quint32(cache.size());
                 ds << cacheHeader;
 
@@ -376,6 +389,8 @@ void AbstractConfigCache::parse()
 
                 if (ds.status() != QDataStream::Ok)
                     throw Exception("error writing content");
+
+                d->cacheWasWritten = true;
             } catch (const Exception &e) {
                 qCWarning(LogCache) << "Failed to write Cache:" << e.what();
             }
@@ -400,6 +415,18 @@ void AbstractConfigCache::clear()
     d->cacheIndex.clear();
     destruct(d->mergedContent);
     d->mergedContent = nullptr;
+    d->cacheWasRead = false;
+    d->cacheWasWritten = false;
+}
+
+bool AbstractConfigCache::parseReadFromCache() const
+{
+    return d->cacheWasRead;
+}
+
+bool AbstractConfigCache::parseWroteToCache() const
+{
+    return d->cacheWasWritten;
 }
 
 QT_END_NAMESPACE_AM
diff --git a/src/common-lib/configcache.h b/src/common-lib/configcache.h
index afcfd00f..dd6996e1 100644
--- a/src/common-lib/configcache.h
+++ b/src/common-lib/configcache.h
@@ -75,7 +75,8 @@ public:
     };
     Q_DECLARE_FLAGS(Options, Option)
 
-    AbstractConfigCache(const QStringList &configFiles, const QString &cacheBaseName, Options options = None);
+    AbstractConfigCache(const QStringList &configFiles, const QString &cacheBaseName,
+                        const char typeId[4] = nullptr, quint32 version = 0, Options options = None);
     virtual ~AbstractConfigCache();
 
     virtual void parse();
@@ -86,6 +87,10 @@ public:
 
     void clear();
 
+    // mainly for debugging and auto tests
+    bool parseReadFromCache() const;
+    bool parseWroteToCache() const;
+
 protected:
     virtual void *loadFromSource(QIODevice *source, const QString &fileName) = 0;
     virtual void preProcessSourceContent(QByteArray &sourceContent, const QString &fileName) = 0;
@@ -106,8 +111,9 @@ public:
     using AbstractConfigCache::Option;
     using AbstractConfigCache::Options;
 
-    ConfigCache(const QStringList &configFiles, const QString &cacheBaseName, Options options = None)
-        : AbstractConfigCache(configFiles, cacheBaseName, options)
+    ConfigCache(const QStringList &configFiles, const QString &cacheBaseName, const char typeId[4],
+            qint32 typeVersion = 0, Options options = None)
+        : AbstractConfigCache(configFiles, cacheBaseName, typeId, typeVersion, options)
     { }
 
     ~ConfigCache()
diff --git a/src/common-lib/configcache_p.h b/src/common-lib/configcache_p.h
index 2bd4eb27..4a6e52e3 100644
--- a/src/common-lib/configcache_p.h
+++ b/src/common-lib/configcache_p.h
@@ -58,27 +58,31 @@ struct ConfigCacheEntry
 struct CacheHeader
 {
     enum { Magic = 0x23d39366, // dd if=/dev/random bs=4 count=1 status=none | xxd -p
-           Version = 1 };
-    static quint64 s_globalId;
+           Version = 2 };
 
     quint32 magic = Magic;
     quint32 version = Version;
-    quint64 globalId = 0;
+    quint32 typeId = 0;
+    quint32 typeVersion = 0;
     QString baseName;
     quint32 entries = 0;
 
-    bool isValid(const QString &baseName) const;
+    bool isValid(const QString &baseName, quint32 typeId = 0, quint32 typeVersion = 0) const;
 };
 
 class ConfigCachePrivate
 {
 public:
     AbstractConfigCache::Options options;
+    quint32 typeId;
+    quint32 typeVersion;
     QStringList rawFiles;
     QString cacheBaseName;
     QVector<ConfigCacheEntry> cache;
     QMap<QString, int> cacheIndex;
     void *mergedContent = nullptr;
+    bool cacheWasRead = false;
+    bool cacheWasWritten = false;
 };
 
 QT_END_NAMESPACE_AM
diff --git a/src/main-lib/configuration.cpp b/src/main-lib/configuration.cpp
index 6fd6bea6..4249a588 100644
--- a/src/main-lib/configuration.cpp
+++ b/src/main-lib/configuration.cpp
@@ -329,7 +329,8 @@ void Configuration::parseWithArguments(const QStringList &arguments)
     if (configFilePaths.isEmpty()) {
         m_data = new ConfigurationData();
     } else {
-        ConfigCache<ConfigurationData> cache(configFilePaths, qSL("config"), cacheOptions);
+        ConfigCache<ConfigurationData> cache(configFilePaths, qSL("config"), "CFGD",
+                                             ConfigurationData::DataStreamVersion, cacheOptions);
 
         try {
             cache.parse();
@@ -395,8 +396,14 @@ void Configuration::parseWithArguments(const QStringList &arguments)
         qCDebug(LogDeployment) << "ignoring '--start-session-dbus'";
 }
 
+
+const quint32 ConfigurationData::DataStreamVersion = 2;
+
+
 ConfigurationData *ConfigurationData::loadFromCache(QDataStream &ds)
 {
+    // NOTE: increment DataStreamVersion above, if you make any changes here
+
     // IMPORTANT: when doing changes to ConfigurationData, remember to adjust all of
     //            loadFromCache(), saveToCache() and mergeFrom() at the same time!
 
@@ -453,6 +460,8 @@ ConfigurationData *ConfigurationData::loadFromCache(QDataStream &ds)
 
 void ConfigurationData::saveToCache(QDataStream &ds) const
 {
+    // NOTE: increment DataStreamVersion above, if you make any changes here
+
     // IMPORTANT: when doing changes to ConfigurationData, remember to adjust all of
     //            loadFromCache(), saveToCache() and mergeFrom() at the same time!
 
diff --git a/src/main-lib/configuration_p.h b/src/main-lib/configuration_p.h
index b0897704..08771523 100644
--- a/src/main-lib/configuration_p.h
+++ b/src/main-lib/configuration_p.h
@@ -59,6 +59,8 @@ QT_BEGIN_NAMESPACE_AM
 
 struct ConfigurationData
 {
+    static const quint32 DataStreamVersion;
+
     static ConfigurationData *loadFromSource(QIODevice *source, const QString &fileName);
     static QByteArray substituteVars(const QByteArray &sourceContent, const QString &fileName);
     static ConfigurationData *loadFromCache(QDataStream &ds);
diff --git a/tests/yaml/tst_yaml.cpp b/tests/yaml/tst_yaml.cpp
index fdd03032..448dd690 100644
--- a/tests/yaml/tst_yaml.cpp
+++ b/tests/yaml/tst_yaml.cpp
@@ -291,9 +291,12 @@ void tst_Yaml::cache()
 
     for (int step = 0; step < 2; ++step) {
         try {
-            ConfigCache<CacheTest> cache(files, "cache-test", step == 0 ? AbstractConfigCache::ClearCache
-                                                                        : AbstractConfigCache::None);
+            ConfigCache<CacheTest> cache(files, "cache-test", "CTST", 1,
+                                         step == 0 ? AbstractConfigCache::ClearCache
+                                                   : AbstractConfigCache::None);
             cache.parse();
+            QVERIFY(cache.parseReadFromCache() == (step == 1));
+            QVERIFY(cache.parseWroteToCache() == (step == 0));
             CacheTest *ct1 = cache.takeResult(0);
             QVERIFY(ct1);
             QCOMPARE(ct1->name, "cache1");
@@ -306,6 +309,16 @@ void tst_Yaml::cache()
             QVERIFY2(false, e.what());
         }
     }
+
+    ConfigCache<CacheTest> wrongVersion(files, "cache-test", "CTST", 2, AbstractConfigCache::None);
+    QTest::ignoreMessage(QtWarningMsg, "Failed to read cache: failed to parse cache header");
+    wrongVersion.parse();
+    QVERIFY(!wrongVersion.parseReadFromCache());
+
+    ConfigCache<CacheTest> wrongType(files, "cache-test", "XTST", 1, AbstractConfigCache::None);
+    QTest::ignoreMessage(QtWarningMsg, "Failed to read cache: failed to parse cache header");
+    wrongType.parse();
+    QVERIFY(!wrongType.parseReadFromCache());
 }
 
 void tst_Yaml::mergedCache()
@@ -320,8 +333,10 @@ void tst_Yaml::mergedCache()
             std::reverse(files.begin(), files.end());
 
         try {
-            ConfigCache<CacheTest> cache(files, "cache-test", options);
+            ConfigCache<CacheTest> cache(files, "cache-test", "MTST", 1, options);
             cache.parse();
+            QVERIFY(cache.parseReadFromCache() == (step % 2 == 1));
+            QVERIFY(cache.parseWroteToCache() == (step % 2 == 0));
             CacheTest *ct = cache.takeMergedResult();
             QVERIFY(ct);
             QCOMPARE(ct->name, QFileInfo(files.last()).baseName());