fixes based on MD review

app/test/testmerginapi.cpp

@@ -1,7 +1,7 @@
 #include <QtTest/QtTest>
 #include <QtCore/QObject>
 
-#include "checksum.h"
+#include "projectchecksumcache.h"
 #include "testmerginapi.h"
 #include "inpututils.h"
 #include "coreutils.h"
@@ -534,7 +534,7 @@ void TestMerginApi::testMultiChunkUploadDownload()
     bigFile.write( QByteArray( 1024 * 1024, static_cast<char>( 'A' + i ) ) );   // AAAA.....BBBB.....CCCC.....
   bigFile.close();
 
-  QByteArray checksum = Checksum::calculate( bigFilePath );
+  QByteArray checksum = CoreUtils::calculate( bigFilePath );
   QVERIFY( !checksum.isEmpty() );
 
   // upload
@@ -546,7 +546,7 @@ void TestMerginApi::testMultiChunkUploadDownload()
   downloadRemoteProject( mApi, mUsername, projectName );
 
   // verify it's there and with correct content
-  QByteArray checksum2 = Checksum::calculate( bigFilePath );
+  QByteArray checksum2 = CoreUtils::calculate( bigFilePath );
   QVERIFY( QFileInfo::exists( bigFilePath ) );
   QCOMPARE( checksum, checksum2 );
 }
@@ -567,7 +567,7 @@ void TestMerginApi::testEmptyFileUploadDownload()
   QFile::copy( mTestDataPath + "/" + TEST_EMPTY_FILE_NAME, emptyFileDestinationPath );
   QVERIFY( QFileInfo::exists( emptyFileDestinationPath ) );
 
-  QByteArray checksum = Checksum::calculate( emptyFileDestinationPath );
+  QByteArray checksum = CoreUtils::calculate( emptyFileDestinationPath );
   QVERIFY( !checksum.isEmpty() );
 
   //upload
@@ -579,7 +579,7 @@ void TestMerginApi::testEmptyFileUploadDownload()
   downloadRemoteProject( mApi, mUsername, projectName );
 
   // verify it's there and with correct content
-  QByteArray checksum2 = Checksum::calculate( emptyFileDestinationPath );
+  QByteArray checksum2 = CoreUtils::calculate( emptyFileDestinationPath );
   QVERIFY( QFileInfo::exists( emptyFileDestinationPath ) );
   QCOMPARE( checksum, checksum2 );
 }

core/CMakeLists.txt

@@ -14,7 +14,7 @@ set(MM_CORE_SRCS
     merginprojectmetadata.cpp
     project.cpp
     geodiffutils.cpp
-    checksum.cpp
+    projectchecksumcache.cpp
 )
 
 set(MM_CORE_HDRS
@@ -33,7 +33,7 @@ set(MM_CORE_HDRS
     merginprojectmetadata.h
     project.h
     geodiffutils.h
-    checksum.h
+    projectchecksumcache.h
 )
 
 if (USE_MM_SERVER_API_KEY)

core/coreutils.cpp

@@ -23,6 +23,7 @@
 const QString CoreUtils::LOG_TO_DEVNULL = QStringLiteral();
 const QString CoreUtils::LOG_TO_STDOUT = QStringLiteral( "TO_STDOUT" );
 QString CoreUtils::sLogFile = CoreUtils::LOG_TO_DEVNULL;
+int CoreUtils::CHECKSUM_CHUNK_SIZE = 65536;
 
 QString CoreUtils::appInfo()
 {
@@ -162,6 +163,25 @@ QString CoreUtils::findUniquePath( const QString &path )
   return uniquePath;
 }
 
+QByteArray CoreUtils::calculate( const QString &filePath )
+{
+  QFile f( filePath );
+  if ( f.open( QFile::ReadOnly ) )
+  {
+    QCryptographicHash hash( QCryptographicHash::Sha1 );
+    QByteArray chunk = f.read( CHECKSUM_CHUNK_SIZE );
+    while ( !chunk.isEmpty() )
+    {
+      hash.addData( chunk );
+      chunk = f.read( CHECKSUM_CHUNK_SIZE );
+    }
+    f.close();
+    return hash.result().toHex();
+  }
+
+  return QByteArray();
+}
+
 QString CoreUtils::createUniqueProjectDirectory( const QString &baseDataDir, const QString &projectName )
 {
   QString projectDirPath = findUniquePath( baseDataDir + "/" + projectName );

core/coreutils.h

@@ -39,6 +39,13 @@ class CoreUtils
 
     static QString uuidWithoutBraces( const QUuid &uuid );
 
+    /**
+     * Returns Sha1 checksum of file (no-caching)
+     * This is potentially resourcing-costly operation
+     * \param filePath full path to the file on disk
+     */
+    static QByteArray calculate( const QString &filePath );
+
     /**
     * Returns given path if it does not exist yet, otherwise adds a number to the path in format:
     *  - if path is a directory: "folder" -> "folder (1)"
@@ -88,6 +95,8 @@ class CoreUtils
 
   private:
     static QString sLogFile;
+    static int CHECKSUM_CHUNK_SIZE;
+
     static void appendLog( const QByteArray &data, const QString &path );
 };
 

core/merginapi.cpp

@@ -20,7 +20,7 @@
 #include <QtMath>
 #include <QElapsedTimer>
 
-#include "checksum.h"
+#include "projectchecksumcache.h"
 #include "coreutils.h"
 #include "geodiffutils.h"
 #include "localprojectsmanager.h"
@@ -35,7 +35,7 @@ const QString MerginApi::sMarketingPageRoot = QStringLiteral( "https://merginmap
 const QString MerginApi::sDefaultApiRoot = QStringLiteral( "https://app.merginmaps.com/" );
 const QSet<QString> MerginApi::sIgnoreExtensions = QSet<QString>() << "gpkg-shm" << "gpkg-wal" << "qgs~" << "qgz~" << "pyc" << "swap";
 const QSet<QString> MerginApi::sIgnoreImageExtensions = QSet<QString>() << "jpg" << "jpeg" << "png";
-const QSet<QString> MerginApi::sIgnoreFiles = QSet<QString>() << "mergin.json" << Checksum::sCacheFile << ".DS_Store";
+const QSet<QString> MerginApi::sIgnoreFiles = QSet<QString>() << "mergin.json" << ".DS_Store";
 const int MerginApi::UPLOAD_CHUNK_SIZE = 10 * 1024 * 1024; // Should be the same as on Mergin server
 
 
@@ -219,7 +219,7 @@ QString MerginApi::listProjectsByName( const QStringList &projectNames )
   {
     CoreUtils::log( "list projects by name", QStringLiteral( "Too many local projects: " ) + QString::number( projectNames.count(), 'f', 0 ) );
     const int projectsToRemoveCount = projectNames.count() - listProjectsByNameApiLimit;
-    QString msg = tr( "Reached limit of local projects.\nRemove %1 projects from your device!" ).arg( projectsToRemoveCount );
+    QString msg = tr( "Please remove some projects as the app currently\nonly allows up to %1 downloaded projects." ).arg( projectsToRemoveCount );
     notify( msg );
     projectNamesToRequest.erase( projectNamesToRequest.begin() + listProjectsByNameApiLimit, projectNamesToRequest.end() );
     Q_ASSERT( projectNamesToRequest.count() == listProjectsByNameApiLimit );
@@ -1611,23 +1611,20 @@ QList<MerginFile> MerginApi::getLocalProjectFiles( const QString &projectPath )
   timer.start();
 
   QList<MerginFile> merginFiles;
-  Checksum checkSums( projectPath );
+  ProjectChecksumCache checksumCache( projectPath );
 
-  checkSums.load();
   QSet<QString> localFiles = listFiles( projectPath );
   for ( QString p : localFiles )
   {
     MerginFile file;
-    file.checksum = checkSums.get( p );
+    file.checksum = checksumCache.get( p );
     file.path = p;
     QFileInfo info( projectPath + p );
     file.size = info.size();
     file.mtime = info.lastModified();
     merginFiles.append( file );
   }
 
-  checkSums.save();
-
   qint64 elapsed = timer.elapsed();
   if ( elapsed > 100 )
   {
@@ -2543,7 +2540,7 @@ void MerginApi::pushInfoReplyFinished()
 
         if ( geodiffRes == GEODIFF_SUCCESS )
         {
-          QByteArray checksumDiff = Checksum::calculate( diffPath );
+          QByteArray checksumDiff = CoreUtils::calculate( diffPath );
 
           // TODO: this is ugly. our basefile may not need to have the same checksum as the server's
           // basefile (because each of them have applied the diff independently) so we have to fake it
@@ -2820,7 +2817,7 @@ bool MerginApi::hasLocalChanges(
   const QString &projectDir
 )
 {
-  if (localFiles.count() != oldServerFiles.count())
+  if ( localFiles.count() != oldServerFiles.count() )
   {
     return true;
   }

core/project.h

@@ -25,7 +25,7 @@ namespace ProjectStatus
   {
     NoVersion,  //!< the project is not downloaded
     UpToDate,   //!< both server and local copy are in sync with no extra modifications
-    NeedsSync,  //!< server has newer version than what is available locally and/or the project is not modified locally
+    NeedsSync,  //!< server has newer version than what is available locally and/or the project is modified locally
   };
   Q_ENUM_NS( Status )
 

core/checksum.cpp → core/projectchecksumcache.cpp

@@ -12,23 +12,21 @@
 #include <QFileInfo>
 #include <QDataStream>
 
-#include "checksum.h"
+#include "projectchecksumcache.h"
+#include "coreutils.h"
+#include "merginapi.h"
 
-static const int CHUNK_SIZE = 65536;
-const QString Checksum::sCacheFile = QStringLiteral( ".checksum.cache" );
+const QString ProjectChecksumCache::sCacheFile = QStringLiteral( "checksum.cache" );
 
-Checksum::Checksum( const QString &projectDir )
-  : mProjectDir( projectDir )
+QString ProjectChecksumCache::cacheFilePath() const
 {
-
+  return mProjectDir + "/" + MerginApi::sMetadataFolder + "/" + sCacheFile;
 }
 
-void Checksum::load()
+ProjectChecksumCache::ProjectChecksumCache( const QString &projectDir )
+  : mProjectDir( projectDir )
 {
-  mCache.clear();
-  mCacheModified = false;
-
-  QFile f( mProjectDir + "/" + sCacheFile );
+  QFile f( cacheFilePath() );
 
   if ( f.open( QIODevice::ReadOnly ) )
   {
@@ -49,12 +47,12 @@ void Checksum::load()
   }
 }
 
-void Checksum::save()
+ProjectChecksumCache::~ProjectChecksumCache()
 {
   if ( !mCacheModified )
     return;
 
-  QFile f( mProjectDir + "/" + sCacheFile );
+  QFile f( cacheFilePath() );
 
   if ( f.open( QIODevice::WriteOnly ) ) // implies Truncate
   {
@@ -66,11 +64,15 @@ void Checksum::save()
       stream << it.key() << it.value().checksum << it.value().mtime;
     }
   }
+  else
+  {
+    CoreUtils::log( "projectchecksumcache", QStringLiteral( "Unable to save cache %1" ).arg( cacheFilePath() ) );
+  }
 }
 
-QString Checksum::get( const QString &path )
+QString ProjectChecksumCache::get( const QString &path )
 {
-  QDateTime localLastModified = QFileInfo( mProjectDir + path ).lastModified();
+  QDateTime localLastModified = QFileInfo( mProjectDir + "/" + path ).lastModified();
 
   auto match = mCache.find( path );
 
@@ -80,14 +82,9 @@ QString Checksum::get( const QString &path )
     {
       return match.value().checksum;
     }
-    else
-    {
-      // invalid entry - remove from cache and recalculate
-      mCache.remove( path );
-    }
   }
 
-  QByteArray localChecksumBytes = calculate( mProjectDir + path );
+  QByteArray localChecksumBytes = CoreUtils::calculate( mProjectDir + "/" + path );
   QString localChecksum = QString::fromLatin1( localChecksumBytes.data(), localChecksumBytes.size() );
 
   CacheValue entry;
@@ -98,22 +95,3 @@ QString Checksum::get( const QString &path )
 
   return localChecksum;
 }
-
-QByteArray Checksum::calculate( const QString &filePath )
-{
-  QFile f( filePath );
-  if ( f.open( QFile::ReadOnly ) )
-  {
-    QCryptographicHash hash( QCryptographicHash::Sha1 );
-    QByteArray chunk = f.read( CHUNK_SIZE );
-    while ( !chunk.isEmpty() )
-    {
-      hash.addData( chunk );
-      chunk = f.read( CHUNK_SIZE );
-    }
-    f.close();
-    return hash.result().toHex();
-  }
-
-  return QByteArray();
-}

core/checksum.h → core/projectchecksumcache.h

@@ -7,8 +7,8 @@
  *                                                                         *
  ***************************************************************************/
 
-#ifndef CHECKSUM_H
-#define CHECKSUM_H
+#ifndef PROJECTCHECKSUMCACHE_H
+#define PROJECTCHECKSUMCACHE_H
 
 #include <QByteArray>
 #include <QString>
@@ -18,32 +18,25 @@
 /**
  * Calculates the checksums of local files and store the results in the local binary file
  */
-class Checksum
+class ProjectChecksumCache
 {
   public:
-    Checksum( const QString &projectDir );
-
-    //! Loads cache from mProjectDir/sCacheFile
-    void load();
-    //! Saves cache to mProjectDir/sCacheFile
-    void save();
+    ProjectChecksumCache( const QString &projectDir );
+    ~ProjectChecksumCache();
 
     /**
      * Returns Sha1 checksum of file (with-caching)
-     * Recalculates checksum for all entries not in cache
+     * Recalculates checksum for an entry not in cache
+     * \param path relative path of the file to mProjectDir
      */
     QString get( const QString &path );
 
-    /**
-     * Returns Sha1 checksum of file (no-caching)
-     * This is potentially resourcing-costly operation
-     */
-    static QByteArray calculate( const QString &filePath );
-
     //! Name of the file in which the cache for the project is stored
     static const QString sCacheFile;
 
   private:
+    QString cacheFilePath() const;
+
     struct CacheValue
     {
       QDateTime mtime; //!< associated file modification date when checksum was calculated
@@ -55,4 +48,4 @@ class Checksum
     bool mCacheModified = false;
 };
 
-#endif
+#endif // PROJECTCHECKSUMCACHE_H