fix: multi space systems are now properly detected and handled (#12062)

* fix: multi space systems are now properly detected and handled

* Revert "[test-only][full-ci] Fix GUI tests (#11963)"

This reverts commit b056cf0.

* fix: refactor dependency between SpaceManager and Space

Author: DeepDiver1975

src/libsync/graphapi/space.cpp

@@ -32,10 +32,11 @@ const auto personalC = QLatin1String("personal");
 const auto sharesIdC = QLatin1String("a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668");
 }
 
-Space::Space(SpacesManager *spacesManager, const OpenAPI::OAIDrive &drive)
+Space::Space(SpacesManager *spacesManager, const OpenAPI::OAIDrive &drive, const bool hasManyPersonalSpaces)
     : QObject(spacesManager)
     , _spaceManager(spacesManager)
     , _image(new SpaceImage(this))
+    , _hasManyPersonalSpaces(hasManyPersonalSpaces)
 {
     setDrive(drive);
     connect(_image, &SpaceImage::imageChanged, this, &Space::imageChanged);
@@ -97,14 +98,15 @@ void SpaceImage::update()
 
 QString Space::displayName() const
 {
-    auto hasManyPersonalSpaces = _spaceManager->account()->capabilities().spacesSupport().enabled;
-    if (hasManyPersonalSpaces) {
+    if (_hasManyPersonalSpaces) {
         return _drive.getName();
     }
 
+    // other systems like oCIS have one personal and one shared space and their names are hard coded
     if (_drive.getDriveType() == personalC) {
         return tr("Personal");
-    } else if (_drive.getId() == sharesIdC) {
+    }
+    if (_drive.getId() == sharesIdC) {
         // don't call it ShareJail
         return tr("Shares");
     }

src/libsync/graphapi/space.h

@@ -77,7 +77,7 @@ namespace GraphApi {
         OpenAPI::OAIDrive drive() const;
 
         /***
-         * Asign a priority to a drive, used for sorting
+         * Assign a priority to a drive, used for sorting
          */
         uint32_t priority() const;
 
@@ -94,14 +94,16 @@ namespace GraphApi {
         void imageChanged();
 
     private:
-        Space(SpacesManager *spaceManager, const OpenAPI::OAIDrive &drive);
+        Space(SpacesManager *spaceManager, const OpenAPI::OAIDrive &drive, bool hasManyPersonalSpaces);
         void setDrive(const OpenAPI::OAIDrive &drive);
 
         SpacesManager *_spaceManager;
         OpenAPI::OAIDrive _drive;
 
         SpaceImage *_image;
 
+        bool _hasManyPersonalSpaces;
+
         friend class SpacesManager;
         friend class SpaceImage;
     };

src/libsync/graphapi/spacesmanager.cpp

@@ -55,18 +55,22 @@ void SpacesManager::refresh()
     if (!_account->credentials()->ready()) {
         return;
     }
-    // TODO: leak the job until we fixed the onwership https://github.com/owncloud/client/issues/11203
+
+    // TODO: leak the job until we fixed the ownership https://github.com/owncloud/client/issues/11203
     auto drivesJob = new Drives(_account->sharedFromThis(), nullptr);
     drivesJob->setTimeout(refreshTimeoutC);
     connect(drivesJob, &Drives::finishedSignal, this, [drivesJob, this] {
+        // a system which provides multiple personal spaces the name of the drive is always used as display name
+        auto hasManyPersonalSpaces = this->account()->capabilities().spacesSupport().hasMultiplePersonalSpaces;
+
         drivesJob->deleteLater();
         if (drivesJob->httpStatusCode() == 200) {
             auto oldKeys = _spacesMap.keys();
             for (const auto &dr : drivesJob->drives()) {
                 auto *space = this->space(dr.getId());
                 oldKeys.removeAll(dr.getId());
                 if (!space) {
-                    space = new Space(this, dr);
+                    space = new Space(this, dr, hasManyPersonalSpaces);
                     _spacesMap.insert(dr.getId(), space);
                 } else {
                     space->setDrive(dr);

test/gui/shared/scripts/bdd_hooks.py

@@ -112,10 +112,7 @@ def hook(context):
         os.makedirs(tmp_dir)
 
     # sync connection folder display name
-    # For oCIS, it will be the user's display name
-    # and will be set while adding the account or creating resources
-    if not get_config("ocis"):
-        set_config("syncConnectionName", "ownCloud")
+    set_config("syncConnectionName", "Personal" if get_config("ocis") else "ownCloud")
 
 
 # determines if the test scenario failed or not

test/gui/shared/scripts/helpers/ConfigHelper.py

@@ -100,7 +100,6 @@ def get_default_home_dir():
     'ocis': False,
     'record_video_on_failure': False,
     'files_for_upload': os.path.join(CURRENT_DIR.parent.parent, 'files-for-upload'),
-    'syncConnectionName': '',
 }
 CONFIG.update(DEFAULT_PATH_CONFIG)
 

test/gui/shared/scripts/helpers/SetupClientHelper.py

@@ -74,18 +74,11 @@ def set_current_user_sync_path(sync_path):
 
 def get_resource_path(resource='', user='', space=''):
     sync_path = get_config('currentUserSyncPath')
-    if not sync_path.startswith(get_config('tempFolderPath')):
-        if user:
-            sync_path = user
-        else:
-            user = parse_username_from_sync_path(sync_path)
-        if get_config('ocis'):
-            space = (
-                space
-                or get_config('syncConnectionName')
-                or get_displayname_for_user(user)
-            )
-            sync_path = join(sync_path, space)
+    if user:
+        sync_path = user
+    if get_config('ocis'):
+        space = space or get_config('syncConnectionName')
+        sync_path = join(sync_path, space)
     sync_path = join(get_config('clientRootSyncPath'), sync_path)
     resource = resource.replace(sync_path, '').strip('/').strip('\\')
     if is_windows():
@@ -96,10 +89,6 @@ def get_resource_path(resource='', user='', space=''):
     )
 
 
-def parse_username_from_sync_path(sync_path):
-    return sync_path.split('/').pop()
-
-
 def get_temp_resource_path(resource_name):
     return join(get_config('tempFolderPath'), resource_name)
 
@@ -168,10 +157,12 @@ def generate_account_config(users, space='Personal'):
         server_url = get_config('localBackendUrl')
 
         if is_ocis := get_config('ocis'):
-            if space == 'Personal':
-                space = get_displayname_for_user(username)
+            set_config('syncConnectionName', space)
             sync_path = create_space_path(space)
-            dav_endpoint = url_join('dav/spaces', get_space_id(space, username))
+            space_name = space
+            if space == 'Personal':
+                space_name = get_displayname_for_user(username)
+            dav_endpoint = url_join('dav/spaces', get_space_id(space_name, username))
 
         args = {
             'url': url_join(server_url, dav_endpoint, ''),
@@ -199,10 +190,7 @@ def generate_account_config(users, space='Personal'):
     return sync_paths
 
 
-def setup_client(username, space=None):
-    if not space or space == 'Personal':
-        space = get_displayname_for_user(username)
-        set_config('syncConnectionName', space)
+def setup_client(username, space='Personal'):
     sync_paths = generate_account_config([username], space)
     start_client()
     for _, sync_path in sync_paths.items():

test/gui/shared/steps/account_context.py

@@ -17,13 +17,12 @@
     wait_for_initial_sync_to_complete,
     listen_sync_status_for_item,
 )
-from helpers.ConfigHelper import get_config, set_config, is_windows, is_linux
+from helpers.ConfigHelper import get_config, is_windows, is_linux
 
 
 @When('the user adds the following user credentials:')
 def step(context):
     account_details = get_client_details(context)
-    set_config('syncConnectionName', get_displayname_for_user(account_details['user']))
     AccountConnectionWizard.add_user_credentials(
         account_details['user'], account_details['password']
     )
@@ -106,7 +105,6 @@ def step(context):
 @When('the user adds the following account:')
 def step(context):
     account_details = get_client_details(context)
-    set_config('syncConnectionName', get_displayname_for_user(account_details['user']))
     AccountConnectionWizard.add_account(account_details)
     # wait for files to sync
     wait_for_initial_sync_to_complete(get_resource_path('/', account_details['user']))
@@ -115,10 +113,6 @@ def step(context):
 @Given('the user has entered the following account information:')
 def step(context):
     account_details = get_client_details(context)
-    if account_details['user']:
-        set_config(
-            'syncConnectionName', get_displayname_for_user(account_details['user'])
-        )
     AccountConnectionWizard.add_account_information(account_details)
 
 

test/gui/shared/steps/spaces_context.py

@@ -11,7 +11,7 @@
     get_file_content,
     resource_exists,
 )
-from helpers.ConfigHelper import get_config, set_config
+from helpers.ConfigHelper import get_config
 
 
 @Given('the administrator has created a space "|any|"')
@@ -43,7 +43,6 @@ def step(context, user, space_name):
     enter_password = EnterPassword()
     if get_config('ocis'):
         enter_password.accept_certificate()
-        set_config('syncConnectionName', space_name)
     enter_password.login_after_setup(user, password)
     # wait for files to sync
     wait_for_initial_sync_to_complete(get_resource_path('/', user, space_name))

test/gui/tst_addAccount/test.feature

@@ -76,7 +76,7 @@ Feature: adding accounts
             | user     | Alice          |
             | password | 1234           |
         When the user selects manual sync folder option in advanced section
-        And the user syncs the "Alice Hansen" space
+        And the user syncs the "Personal" space
         Then the folder "simple-folder" should exist on the file system
 
     @skipOnOCIS

test/gui/tst_syncing/test.feature

@@ -68,7 +68,7 @@ Feature: Syncing files
             | user     | Alice          |
             | password | 1234           |
         When the user selects manual sync folder option in advanced section
-        And the user selects "Alice Hansen" space in sync connection wizard
+        And the user selects "Personal" space in sync connection wizard
         And the user sets the sync path in sync connection wizard
         And the user navigates back in the sync connection wizard
         And the user sets the temp folder "localSyncFolder" as local sync path in sync connection wizard
@@ -97,7 +97,7 @@ Feature: Syncing files
             | user     | Alice          |
             | password | 1234           |
         When the user selects manual sync folder option in advanced section
-        And the user selects "Alice Hansen" space in sync connection wizard
+        And the user selects "Personal" space in sync connection wizard
         And the user sets the sync path in sync connection wizard
         And the user selects "ownCloud" as a remote destination folder
         And the user disables VFS support for Windows
@@ -133,7 +133,7 @@ Feature: Syncing files
             | user     | Alice          |
             | password | 1234           |
         When the user selects manual sync folder option in advanced section
-        And the user selects "Alice Hansen" space in sync connection wizard
+        And the user selects "Personal" space in sync connection wizard
         And the user sets the sync path in sync connection wizard
         And the user selects "ownCloud" as a remote destination folder
         And the user disables VFS support for Windows
@@ -471,7 +471,7 @@ Feature: Syncing files
             | user     | Alice          |
             | password | 1234           |
         When the user selects manual sync folder option in advanced section
-        And the user selects "Alice Hansen" space in sync connection wizard
+        And the user selects "Personal" space in sync connection wizard
         And the user sets the temp folder "~`!@#$^&()-_=+{[}];',)PRN%" as local sync path in sync connection wizard
         And the user selects "ownCloud" as a remote destination folder
         And the user disables VFS support for Windows