Propagate changes on backup tests + fixes bugs found in the process

data/hooks/restore/21-conf_ynh_certs

@@ -3,6 +3,5 @@ backup_dir="$1/conf/ynh/certs"
 sudo mkdir -p /etc/yunohost/certs/
 
 sudo cp -a $backup_dir/. /etc/yunohost/certs/
-sudo yunohost app ssowatconf
 sudo service nginx reload
 sudo service metronome reload

src/yunohost/backup.py

@@ -1134,6 +1134,8 @@ class RestoreManager():
 
             self._restore_system()
             self._restore_apps()
+        except Exception as e:
+            logger.error("The following critical error happened during restoration: %s" % e)
         finally:
             self.clean()
 
@@ -1186,11 +1188,12 @@ class RestoreManager():
         if system_targets == []:
             return
 
-        from yunohost.permission import permission_create, user_permission_update, user_permission_list
+        from yunohost.user import user_group_list
+        from yunohost.permission import permission_create, permission_delete, user_permission_update, user_permission_list
 
         # Backup old permission for apps
         # We need to do that because in case of an app is installed we can't remove the permission for this app
-        old_apps_permission = user_permission_list(ignore_system_perms=True)["permissions"]
+        old_apps_permission = user_permission_list(ignore_system_perms=True, full=True)["permissions"]
 
         # Start register change on system
         operation_logger = OperationLogger('backup_restore_system')
@@ -1232,7 +1235,7 @@ class RestoreManager():
         # do the migration 0011 : setup group and permission
         #
         # Legacy code
-        if not user_group_list["groups"]:
+        if not "all_users" in user_group_list()["groups"].keys():
             from yunohost.tools import _get_migration_by_name
             setup_group_permission = _get_migration_by_name("setup_group_permission")
             # Update LDAP schema restart slapd
@@ -1251,14 +1254,12 @@ class RestoreManager():
                 permission_create(permission_name, urls=permission_infos["urls"], sync_perm=False)
                 user_permission_update(permission_name, remove="all_users", add=permission_infos["allowed"])
 
-
     def _restore_apps(self):
         """Restore all apps targeted"""
 
         apps_targets = self.targets.list("apps", exclude=["Skipped"])
 
         for app in apps_targets:
-            print(app)
             self._restore_app(app)
 
     def _restore_app(self, app_instance_name):
@@ -1359,11 +1360,11 @@ class RestoreManager():
                 permissions = read_yaml('%s/permissions.yml' % app_settings_new_path)
                 existing_groups = user_group_list()['groups']
 
-                for permission_name, permission_infos in permissions:
+                for permission_name, permission_infos in permissions.items():
 
                     permission_create(permission_name, urls=permission_infos.get("urls", []))
 
-                    if "allowed" not in permissions_infos:
+                    if "allowed" not in permission_infos:
                         logger.warning("'allowed' key corresponding to allowed groups for permission %s not found when restoring app %s ... You might need to reconfigure permissions yourself!" % (permission_name, app_instance_name))
                     else:
                         groups = [g for g in permission_infos["allowed"] if g in existing_groups]

src/yunohost/data_migrations/0011_setup_group_permission.py

@@ -89,6 +89,11 @@ class MyMigration(Migration):
             app_setting(app, 'allowed_users', delete=True)
 
     def run(self):
+
+        # FIXME : what do we really want to do here ...
+        # Imho we should just force-regen the conf in all case, and maybe
+        # just display a warning if we detect that the conf was manually modified
+
         # Check if the migration can be processed
         ldap_regen_conf_status = regen_conf(names=['slapd'], dry_run=True)
         # By this we check if the have been customized

src/yunohost/tests/test_backuprestore.py

@@ -38,10 +38,10 @@ def setup_function(function):
         add_archive_wordpress_from_2p4()
         assert len(backup_list()["archives"]) == 1
 
-    if "with_backup_legacy_app_installed" in markers:
+    if "with_legacy_app_installed" in markers:
-        assert not app_is_installed("backup_legacy_app")
+        assert not app_is_installed("legacy_app")
-        install_app("backup_legacy_app_ynh", "/yolo")
+        install_app("legacy_app_ynh", "/yolo")
-        assert app_is_installed("backup_legacy_app")
+        assert app_is_installed("legacy_app")
 
     if "with_backup_recommended_app_installed" in markers:
         assert not app_is_installed("backup_recommended_app")
@@ -105,7 +105,7 @@ def backup_test_dependencies_are_met():
 
     # Dummy test apps (or backup archives)
     assert os.path.exists("./tests/apps/backup_wordpress_from_2p4")
-    assert os.path.exists("./tests/apps/backup_legacy_app_ynh")
+    assert os.path.exists("./tests/apps/legacy_app_ynh")
     assert os.path.exists("./tests/apps/backup_recommended_app_ynh")
 
     return True
@@ -155,8 +155,8 @@ def delete_all_backups():
 
 def uninstall_test_apps_if_needed():
 
-    if _is_installed("backup_legacy_app"):
+    if _is_installed("legacy_app"):
-        app_remove("backup_legacy_app")
+        app_remove("legacy_app")
 
     if _is_installed("backup_recommended_app"):
         app_remove("backup_recommended_app")
@@ -497,10 +497,10 @@ def test_restore_app_already_installed(mocker):
     assert _is_installed("wordpress")
 
 
-@pytest.mark.with_backup_legacy_app_installed
+@pytest.mark.with_legacy_app_installed
 def test_backup_and_restore_legacy_app():
 
-    _test_backup_and_restore_app("backup_legacy_app")
+    _test_backup_and_restore_app("legacy_app")
 
 
 @pytest.mark.with_backup_recommended_app_installed
@@ -531,7 +531,7 @@ def _test_backup_and_restore_app(app):
     # Uninstall the app
     app_remove(app)
     assert not app_is_installed(app)
-    assert app not in user_permission_list()['permissions']
+    assert app+".main" not in user_permission_list()['permissions']
 
     # Restore the app
     backup_restore(system=None, name=archives[0],
@@ -541,8 +541,7 @@ def _test_backup_and_restore_app(app):
 
     # Check permission
     per_list = user_permission_list()['permissions']
-    assert app in per_list
+    assert app+".main" in per_list
-    assert "main" in per_list[app]
 
 #
 # Some edge cases                                                            #