Use project IDs internaly to manage projects #3884

Withalion · 2025-05-06T13:33:23Z

Fixes #1969

This PR adds foundational work for supporting project IDs in mobile app. Besides issue mentioned above this is preliminary work for #1008, #1102, #576, project renaming etc.

Outline of the changes done:

core
- CoreUtils
  - refactor
  - getFullProjectName(), extractProjectName() has been moved from MerginApi
- Project
  - LocalProject supports UUID type of IDs + new generateProjectId()
  - MerginProject supports UUID type of IDs + new fullname()
  - LocalProjectsList changed to HashMap LocalProjectsDict with ID as key
- LocalProjectsManager
  - keeps projects in HashMap
  - delete projectFromMerginName(), it was used only by tests
  - new updateProjectId() necessary function for creating new projects on server ( server returns new ID for already existing projects, app needs to adapt to it)
  - general refactor and move from project fullnames to project IDs as parameters in functions
- MerginApi
  - general refactor, a lot of the functions are never called from QML for example
  - listProjectsByName() stays until there is v2 endpoint for IDs
  - delete migrateProjectToMergin() just a wrapper for createProject() ( i think it was used just by tests)
  - delete projectDiffableFiles() not used anywhere
  - move getFullProjectName() & extractProjectName() to utils
  - change deleteProject() to use immediate deletion instead of scheduled, we use it only internally for project management
  - remove topic parameter from networkErrorOccured() it was only used by tests (they use http error codes now)
  - new projectIdChanged() signal to trigger update of local project ID to server generated ID
  - new getProjectDetails() it's leaner version of getProjectInfo(), the response won't contain versioning history
  - Transactions now keep projectId as key
  - generally everything what could be, was moved to use project IDs, the synchronization workflow couldn't, so for communication with API we use project names but internally the project ID is propagated and used where possible
  - when a local project is "migrated" to mergin there is new request for new project ID generated on server that is further processed and local ID is changed to match server ID
app
- QML
  - just refactor to support project IDs
- C++
  - ActiveProject
    - new projectId() to get project ID of active project
  - Main
    - rework to project IDs
  - ProjectsModel
    - replaced listProjectsByName() by fetchProjectsById()
    - reworked slots to use project IDs
    - removed porjectNames(), it was used internally and is not necessary since we moved to IDs
  - ProjectWizard
    - refactor, most importantly moved from deprecated QVariant::type to QMetaType
  - SynchronizationManager
    - keeps synchronization processes by project ID
    - reworked to use project IDs instead of project names
- Tests
  - mostly refactor & minor changes
  - replaced findProjectByName() with template function
  - moved descriptive inline test function comments to header file or written new ones

github-actions · 2025-05-15T09:56:22Z

Pull Request Test Coverage Report for Build 15041670829

Details

0 of 0 changed or added relevant lines in 0 files are covered.
634 unchanged lines in 14 files lost coverage.
Overall coverage increased (+0.3%) to 60.661%

Files with Coverage Reduction	New Missed Lines	%
input/app/projectwizard.h	1	50.0%
input/core/merginapi.h	1	97.56%
input/core/merginprojectstatusmodel.h	1	0.0%
input/core/project.cpp	1	97.22%
input/app/projectsmodel.h	3	25.0%
input/core/project.h	16	46.15%
input/app/variablesmanager.cpp	19	60.28%
input/core/localprojectsmanager.cpp	21	78.74%
input/core/coreutils.cpp	31	82.61%
input/app/projectsmodel.cpp	41	69.52%

Totals
Change from base Build 15018608869:	0.3%
Covered Lines:	8202
Relevant Lines:	13521

💛 - Coveralls

tomasMizera

Managed to go through the app folder so far, will continue later!

tomasMizera · 2025-05-30T17:28:23Z

app/projectsmodel.cpp

-  // In theory we could send that request only for this one project.
-  listProjectsByName();
+  // To ensure project will be in sync with server, send fetchProjectsByProjectId request.
+  fetchProjectsByProjectId( { projectId } );


If you fetch state only of this one project, won't other local projects states get unknown (not calculated)? When requesting this API (in fetchProjectsByProjectId) we clear the projects model and so the resulting state of the other projects is now likely unknown (I did not test this, just assumption from the code).

Sounds plausible from the look of it. Will test it later again when the review is finished.

tomasMizera · 2025-05-30T17:44:39Z

app/projectsmodel.h


    //! Stops running project upload or update
-    Q_INVOKABLE void stopProjectSync( const QString &projectId );
+    Q_INVOKABLE void stopProjectSync( const QString &projectId ) const;


This one should not be const, it alters things (maybe not directly, but it does)!

I agree with that it changes some things. We should probably have some discussion about using const and in what cases is it intended to use.

tomasMizera · 2025-05-30T17:46:43Z

app/projectsmodel.h


    //! Forwards call to LocalProjectsManager to remove local project
-    Q_INVOKABLE void removeLocalProject( const QString &projectId );
+    Q_INVOKABLE void removeLocalProject( const QString &projectId ) const;


Same here, I believe this one should not be const.

Same thing as mentioned above.

tomasMizera · 2025-06-26T10:13:27Z

From our discussion earlier today:

For sync:

For listing downloaded projects:

Also refactor some clang-tidy warnings in MerginApi

tomasMizera · 2025-08-07T15:42:11Z

core/merginapi.cpp

-    projectNamesToRequest.erase( projectNamesToRequest.begin() + listProjectsByNameApiLimit, projectNamesToRequest.end() );
-    Q_ASSERT( projectNamesToRequest.count() == listProjectsByNameApiLimit );
+    projectNamesToRequest.erase( projectNamesToRequest.begin() + maxProjectRequests, projectNamesToRequest.end() );
+    Q_ASSERT( projectNamesToRequest.count() == maxProjectRequests );


I guess

Suggested change

Q_ASSERT( projectNamesToRequest.count() == maxProjectRequests );

Q_ASSERT( projectNamesToRequest.count() <= maxProjectRequests );

tomasMizera · 2025-08-08T11:56:10Z

core/merginapi.cpp

  Q_ASSERT( r == transaction.replyPullProjectInfo );

  if ( r->error() == QNetworkReply::NoError )
  {
-    QByteArray data = r->readAll();
+    const QByteArray data = r->readAll();
    CoreUtils::log( "pull " + projectFullName, QStringLiteral( "Downloaded project info." ) );



We need to compare if the server project ID is the same as the local project ID - in case remote project was renamed and some other project now bears the same name. We would try to sync a completely different project

tomasMizera · 2025-08-08T11:56:28Z

core/merginapi.cpp

@@ -2817,7 +2949,7 @@ void MerginApi::pushInfoReplyFinished()
    transaction.replyPushProjectInfo->deleteLater();


We need to compare if the server project ID is the same as the local project ID - in case remote project was renamed and some other project now bears the same name. We would try to sync a completely different project

tomasMizera · 2025-08-08T11:57:51Z

core/merginapi.h

     * \return true when sync has started, false otherwise (e.g. due to a missing authorization or invalid server)
     */
-    Q_INVOKABLE bool pushProject( const QString &projectNamespace, const QString &projectName, bool isInitialPush = false );
+    bool pushProject( const QString &projectFullName, const QString &projectId, bool isInitialPush = false );


This and other API definitions should not need to accept both projectName and projectId. The ID should be enough - we can lookup the name via mLocalProjectsManager

Withalion force-pushed the move-to-project-ids branch 2 times, most recently from 8bf8a20 to 2d54ae3 Compare May 15, 2025 09:36

Withalion force-pushed the move-to-project-ids branch from 2d54ae3 to 61b99d1 Compare May 15, 2025 10:46

Withalion marked this pull request as ready for review May 15, 2025 10:47

Withalion requested a review from tomasMizera May 15, 2025 10:47

tomasMizera reviewed May 30, 2025

View reviewed changes

tomasMizera added the FROZEN 🥶 do not merge before upcoming release label Jun 5, 2025

tomasMizera removed the FROZEN 🥶 do not merge before upcoming release label Jul 21, 2025

Withalion added 7 commits July 24, 2025 17:48

WIP use project IDs internaly to manage projects

83142ba

Rework MerginAPI tests

6bbc46b

Fix compilation errors

a0df7d5

Fix error signal in createWorkspace

d2fc016

Also refactor some clang-tidy warnings in MerginApi

Fix unit tests

eceb06a

Add fallback for getProjectInfo during sync

920fef0

Add fallback for listProjectsByName

9ab3d46

Withalion force-pushed the move-to-project-ids branch from 61b99d1 to 9ab3d46 Compare July 24, 2025 14:52

Fix rebase issues

9f0c742

tomasMizera self-requested a review July 25, 2025 07:58

tomasMizera requested changes Aug 8, 2025

View reviewed changes

Withalion added the blocked Not possible to continue until other task is resolved label Sep 30, 2025

tomasMizera changed the base branch from master to dev/2025.7.0 October 3, 2025 11:55

tomasMizera changed the base branch from dev/2025.7.0 to master October 31, 2025 12:53

	Q_ASSERT( projectNamesToRequest.count() == maxProjectRequests );
	Q_ASSERT( projectNamesToRequest.count() <= maxProjectRequests );

		@@ -2817,7 +2949,7 @@ void MerginApi::pushInfoReplyFinished()
		transaction.replyPushProjectInfo->deleteLater();

Use project IDs internaly to manage projects #3884

Are you sure you want to change the base?

Use project IDs internaly to manage projects #3884

Uh oh!

Conversation

Withalion commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented May 15, 2025

Pull Request Test Coverage Report for Build 15041670829

Details

💛 - Coveralls

Uh oh!

tomasMizera left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomasMizera commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Withalion commented May 6, 2025 •

edited

Loading

tomasMizera commented Jun 26, 2025 •

edited

Loading