Skip to content

Commit

Permalink
Speed improvements of home page / sync preparation speed (#2820)
Browse files Browse the repository at this point in the history
* fetch only info about 50 projects in listProjectsByNameApi

* cache calculation of local files checksums

* do not resave cache file if there are no modifications

* merge ProjectStatuses to NeedsSync

* first check if we need sync because of server version, after that because of local changes

* create optimized compare function
  • Loading branch information
PeterPetrik authored Oct 5, 2023
1 parent 897285b commit 31608a1
Show file tree
Hide file tree
Showing 17 changed files with 536 additions and 104 deletions.
2 changes: 2 additions & 0 deletions app/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ if (ENABLE_TESTS)
test/testutilsfunctions.cpp
test/testvariablesmanager.cpp
test/testactiveproject.cpp
test/testprojectchecksumcache.cpp
)

set(MM_HDRS
Expand All @@ -220,6 +221,7 @@ if (ENABLE_TESTS)
test/testutilsfunctions.h
test/testvariablesmanager.h
test/testactiveproject.h
test/testprojectchecksumcache.h
)
endif ()

Expand Down
3 changes: 3 additions & 0 deletions app/projectsmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,9 @@ void ProjectsModel::mergeProjects( const MerginProjectsList &merginProjects, Mer
else if ( project.local.hasMerginMetadata() )
{
// App is starting - loads all local projects from a device
// OR
// We do not have server info because the project was ignored
// (listProjectsByName API limits response to max 50 projects)
project.mergin.projectName = project.local.projectName;
project.mergin.projectNamespace = project.local.projectNamespace;
project.mergin.status = ProjectStatus::projectStatus( project );
Expand Down
6 changes: 2 additions & 4 deletions app/qml/components/ProjectDelegateItem.qml
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ Rectangle {

if ( projectIsLocal && projectIsMergin ) {
// downloaded mergin projects
if ( projectStatus === ProjectStatus.OutOfDate ||
projectStatus === ProjectStatus.Modified ) {
if ( projectStatus === ProjectStatus.NeedsSync ) {
return InputStyle.syncIcon
}
return "" // no icon if this project does not have changes
Expand All @@ -76,8 +75,7 @@ Rectangle {
function getMoreMenuItems() {
if ( projectIsMergin && projectIsLocal )
{
if ( ( projectStatus === ProjectStatus.OutOfDate ||
projectStatus === ProjectStatus.Modified ) )
if ( ( projectStatus === ProjectStatus.NeedsSync ) )
return "sync,changes,remove"

return "changes,remove"
Expand Down
6 changes: 6 additions & 0 deletions app/test/inputtests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "test/testmaptools.h"
#include "test/testlayertree.h"
#include "test/testactiveproject.h"
#include "test/testprojectchecksumcache.h"

#if not defined APPLE_PURCHASING
#include "test/testpurchasing.h"
Expand Down Expand Up @@ -181,6 +182,11 @@ int InputTests::runTest() const
TestActiveProject activeProjectTest( mApi );
nFailed = QTest::qExec( &activeProjectTest, mTestArgs );
}
else if ( mTestRequested == "--testProjectChecksumCache" )
{
TestProjectChecksumCache projectChecksumTest;
nFailed = QTest::qExec( &projectChecksumTest, mTestArgs );
}
#if not defined APPLE_PURCHASING
else if ( mTestRequested == "--testPurchasing" )
{
Expand Down
35 changes: 26 additions & 9 deletions app/test/testmerginapi.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <QtTest/QtTest>
#include <QtCore/QObject>

#include "projectchecksumcache.h"
#include "testmerginapi.h"
#include "inpututils.h"
#include "coreutils.h"
Expand Down Expand Up @@ -533,7 +534,7 @@ void TestMerginApi::testMultiChunkUploadDownload()
bigFile.write( QByteArray( 1024 * 1024, static_cast<char>( 'A' + i ) ) ); // AAAA.....BBBB.....CCCC.....
bigFile.close();

QByteArray checksum = MerginApi::getChecksum( bigFilePath );
QByteArray checksum = CoreUtils::calculateChecksum( bigFilePath );
QVERIFY( !checksum.isEmpty() );

// upload
Expand All @@ -545,7 +546,7 @@ void TestMerginApi::testMultiChunkUploadDownload()
downloadRemoteProject( mApi, mUsername, projectName );

// verify it's there and with correct content
QByteArray checksum2 = MerginApi::getChecksum( bigFilePath );
QByteArray checksum2 = CoreUtils::calculateChecksum( bigFilePath );
QVERIFY( QFileInfo::exists( bigFilePath ) );
QCOMPARE( checksum, checksum2 );
}
Expand All @@ -566,7 +567,7 @@ void TestMerginApi::testEmptyFileUploadDownload()
QFile::copy( mTestDataPath + "/" + TEST_EMPTY_FILE_NAME, emptyFileDestinationPath );
QVERIFY( QFileInfo::exists( emptyFileDestinationPath ) );

QByteArray checksum = MerginApi::getChecksum( emptyFileDestinationPath );
QByteArray checksum = CoreUtils::calculateChecksum( emptyFileDestinationPath );
QVERIFY( !checksum.isEmpty() );

//upload
Expand All @@ -578,7 +579,7 @@ void TestMerginApi::testEmptyFileUploadDownload()
downloadRemoteProject( mApi, mUsername, projectName );

// verify it's there and with correct content
QByteArray checksum2 = MerginApi::getChecksum( emptyFileDestinationPath );
QByteArray checksum2 = CoreUtils::calculateChecksum( emptyFileDestinationPath );
QVERIFY( QFileInfo::exists( emptyFileDestinationPath ) );
QCOMPARE( checksum, checksum2 );
}
Expand Down Expand Up @@ -612,7 +613,7 @@ void TestMerginApi::testPushAddedFile()
QVERIFY( project1.isLocal() && project1.isMergin() );
QCOMPARE( project1.local.localVersion, 1 );
QCOMPARE( project1.mergin.serverVersion, 1 );
QCOMPARE( project1.mergin.status, ProjectStatus::Modified );
QCOMPARE( project1.mergin.status, ProjectStatus::NeedsSync );

// upload
uploadRemoteProject( mApi, mUsername, projectName );
Expand Down Expand Up @@ -670,7 +671,7 @@ void TestMerginApi::testPushRemovedFile()
QVERIFY( project1.isLocal() && project1.isMergin() );
QCOMPARE( project1.local.localVersion, 1 );
QCOMPARE( project1.mergin.serverVersion, 1 );
QCOMPARE( project1.mergin.status, ProjectStatus::Modified );
QCOMPARE( project1.mergin.status, ProjectStatus::NeedsSync );

// upload changes

Expand Down Expand Up @@ -726,7 +727,7 @@ void TestMerginApi::testPushModifiedFile()
QVERIFY( project1.isLocal() && project1.isMergin() );
QCOMPARE( project1.local.localVersion, 1 );
QCOMPARE( project1.mergin.serverVersion, 1 );
QCOMPARE( project1.mergin.status, ProjectStatus::Modified );
QCOMPARE( project1.mergin.status, ProjectStatus::NeedsSync );

// upload
uploadRemoteProject( mApi, mUsername, projectName );
Expand Down Expand Up @@ -784,6 +785,7 @@ void TestMerginApi::testPushNoChanges()
QCOMPARE( project2.mergin.status, ProjectStatus::UpToDate );

QCOMPARE( MerginApi::localProjectChanges( projectDir ), ProjectDiff() );
QVERIFY( !MerginApi::hasLocalProjectChanges( projectDir ) );
}

void TestMerginApi::testUpdateAddedFile()
Expand Down Expand Up @@ -821,7 +823,7 @@ void TestMerginApi::testUpdateAddedFile()
QVERIFY( project1.isLocal() && project1.isMergin() );
QCOMPARE( project1.local.localVersion, 1 );
QCOMPARE( project1.mergin.serverVersion, 2 );
QCOMPARE( project1.mergin.status, ProjectStatus::OutOfDate );
QCOMPARE( project1.mergin.status, ProjectStatus::NeedsSync );

// now try to update
downloadRemoteProject( mApi, mUsername, projectName );
Expand Down Expand Up @@ -1122,6 +1124,7 @@ void TestMerginApi::testDiffUpload()
QVERIFY( QFileInfo::exists( projectDir + "/.mergin/base.gpkg" ) );

QCOMPARE( MerginApi::localProjectChanges( projectDir ), ProjectDiff() ); // no local changes expected
QVERIFY( !MerginApi::hasLocalProjectChanges( projectDir ) );

// replace gpkg with a new version with a modified geometry
QFile::remove( projectDir + "/base.gpkg" );
Expand All @@ -1131,6 +1134,7 @@ void TestMerginApi::testDiffUpload()
ProjectDiff expectedDiff;
expectedDiff.localUpdated = QSet<QString>() << "base.gpkg";
QCOMPARE( diff, expectedDiff );
QVERIFY( MerginApi::hasLocalProjectChanges( projectDir ) );

GeodiffUtils::ChangesetSummary expectedSummary;
expectedSummary["simple"] = GeodiffUtils::TableSummary( 0, 1, 0 );
Expand All @@ -1141,6 +1145,7 @@ void TestMerginApi::testDiffUpload()
uploadRemoteProject( mApi, mUsername, projectName );

QCOMPARE( MerginApi::localProjectChanges( projectDir ), ProjectDiff() ); // no local changes expected
QVERIFY( !MerginApi::hasLocalProjectChanges( projectDir ) );
}

void TestMerginApi::testDiffSubdirsUpload()
Expand All @@ -1156,7 +1161,7 @@ void TestMerginApi::testDiffSubdirsUpload()
QVERIFY( QFileInfo::exists( projectDir + "/.mergin/" + base ) );

QCOMPARE( MerginApi::localProjectChanges( projectDir ), ProjectDiff() ); // no local changes expected

QVERIFY( !MerginApi::hasLocalProjectChanges( projectDir ) );

// replace gpkg with a new version with a modified geometry
QFile::remove( projectDir + "/" + base );
Expand All @@ -1166,6 +1171,7 @@ void TestMerginApi::testDiffSubdirsUpload()
ProjectDiff expectedDiff;
expectedDiff.localUpdated = QSet<QString>() << base ;
QCOMPARE( diff, expectedDiff );
QVERIFY( MerginApi::hasLocalProjectChanges( projectDir ) );

GeodiffUtils::ChangesetSummary expectedSummary;
expectedSummary["simple"] = GeodiffUtils::TableSummary( 0, 1, 0 );
Expand All @@ -1176,6 +1182,7 @@ void TestMerginApi::testDiffSubdirsUpload()
uploadRemoteProject( mApi, mUsername, projectName );

QCOMPARE( MerginApi::localProjectChanges( projectDir ), ProjectDiff() ); // no local changes expected
QVERIFY( !MerginApi::hasLocalProjectChanges( projectDir ) );
}

void TestMerginApi::testDiffUpdateBasic()
Expand All @@ -1193,6 +1200,7 @@ void TestMerginApi::testDiffUpdateBasic()

QVERIFY( QFileInfo::exists( projectDir + "/.mergin/base.gpkg" ) );
QCOMPARE( MerginApi::localProjectChanges( projectDir ), ProjectDiff() ); // no local changes expected
QVERIFY( !MerginApi::hasLocalProjectChanges( projectDir ) );

QgsVectorLayer *vl0 = new QgsVectorLayer( projectDir + "/base.gpkg|layername=simple", "base", "ogr" );
QVERIFY( vl0->isValid() );
Expand Down Expand Up @@ -1222,6 +1230,7 @@ void TestMerginApi::testDiffUpdateBasic()
delete vl;

QCOMPARE( MerginApi::localProjectChanges( projectDir ), ProjectDiff() ); // no local changes expected
QVERIFY( !MerginApi::hasLocalProjectChanges( projectDir ) );

QVERIFY( !GeodiffUtils::hasPendingChanges( projectDir, "base.gpkg" ) );
}
Expand All @@ -1241,6 +1250,7 @@ void TestMerginApi::testDiffUpdateWithRebase()

QVERIFY( QFileInfo::exists( projectDir + "/.mergin/base.gpkg" ) );
QCOMPARE( MerginApi::localProjectChanges( projectDir ), ProjectDiff() ); // no local changes expected
QVERIFY( !MerginApi::hasLocalProjectChanges( projectDir ) );

//
// download with mApiExtra + modify + upload
Expand Down Expand Up @@ -1274,6 +1284,7 @@ void TestMerginApi::testDiffUpdateWithRebase()
ProjectDiff expectedDiff;
expectedDiff.localUpdated = QSet<QString>() << "base.gpkg";
QCOMPARE( diff, expectedDiff );
QVERIFY( MerginApi::hasLocalProjectChanges( projectDir ) );

// check that geodiff knows there was one added feature
GeodiffUtils::ChangesetSummary expectedSummary;
Expand All @@ -1297,6 +1308,7 @@ void TestMerginApi::testDiffUpdateWithRebase()
// like before the update - there should be locally modified base.gpkg with the changes we did
QCOMPARE( MerginApi::localProjectChanges( projectDir ), expectedDiff );
QCOMPARE( GeodiffUtils::parseChangesetSummary( changes ), expectedSummary );
QVERIFY( MerginApi::hasLocalProjectChanges( projectDir ) );
}

void TestMerginApi::testDiffUpdateWithRebaseFailed()
Expand All @@ -1317,6 +1329,7 @@ void TestMerginApi::testDiffUpdateWithRebaseFailed()

QVERIFY( QFileInfo::exists( projectDir + "/.mergin/base.gpkg" ) );
QCOMPARE( MerginApi::localProjectChanges( projectDir ), ProjectDiff() ); // no local changes expected
QVERIFY( !MerginApi::hasLocalProjectChanges( projectDir ) );

//
// download with mApiExtra + modify + upload
Expand All @@ -1342,6 +1355,7 @@ void TestMerginApi::testDiffUpdateWithRebaseFailed()
expectedDiff.localUpdated = QSet<QString>() << "base.gpkg";
qDebug() << diff.dump();
QCOMPARE( diff, expectedDiff );
QVERIFY( MerginApi::hasLocalProjectChanges( projectDir ) );

// check that geodiff knows there was one added feature
QString changes = GeodiffUtils::diffableFilePendingChanges( projectDir, "base.gpkg", true );
Expand All @@ -1367,6 +1381,7 @@ void TestMerginApi::testDiffUpdateWithRebaseFailed()
ProjectDiff expectedDiffFinal;
expectedDiffFinal.localAdded = QSet<QString>() << conflictFilename;
QCOMPARE( MerginApi::localProjectChanges( projectDir ), expectedDiffFinal );
QVERIFY( MerginApi::hasLocalProjectChanges( projectDir ) );
}

void TestMerginApi::testUpdateWithDiffs()
Expand All @@ -1384,6 +1399,7 @@ void TestMerginApi::testUpdateWithDiffs()

QVERIFY( QFileInfo::exists( projectDir + "/.mergin/base.gpkg" ) );
QCOMPARE( MerginApi::localProjectChanges( projectDir ), ProjectDiff() ); // no local changes expected
QVERIFY( !MerginApi::hasLocalProjectChanges( projectDir ) );

//
// download with mApiExtra + modify + upload
Expand Down Expand Up @@ -1414,6 +1430,7 @@ void TestMerginApi::testUpdateWithDiffs()
delete vl;

QCOMPARE( MerginApi::localProjectChanges( projectDir ), ProjectDiff() );
QVERIFY( !MerginApi::hasLocalProjectChanges( projectDir ) );
QVERIFY( !GeodiffUtils::hasPendingChanges( projectDir, "base.gpkg" ) );
}

Expand Down
Loading

0 comments on commit 31608a1

Please sign in to comment.