# HG changeset patch
# User Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
# Date 1630526925 -7200
# Node ID ee1da602b5905cd51f9ce8364d3bdb6fe606732d
# Parent  2ef4f7c0031e7470cc20e83cd4589bba2aa5e6c6
repo_groups: fix deletion of subgroups

Deletion of a repository group that has a parent group (i.e. is not at the
root of the repository group tree) failed as follows:

Traceback (most recent call last):
  [...]
  File ".../lib/python3.9/site-packages/tg/configurator/components/dispatch.py", line 114, in _call_controller
    return controller(*remainder, **params)
  File "<decorator-gen-5>", line 2, in delete

  File "/home/tdescham/repo/contrib/kallithea/kallithea-release/kallithea/lib/auth.py", line 572, in __wrapper
    return func(*fargs, **fkwargs)
  File "/home/tdescham/repo/contrib/kallithea/kallithea-release/kallithea/controllers/admin/repo_groups.py", line 271, in delete
    if gr.parent_group:
  File ".../lib/python3.9/site-packages/sqlalchemy/orm/attributes.py", line 294, in __get__
    return self.impl.get(instance_state(instance), dict_)
  File ".../lib/python3.9/site-packages/sqlalchemy/orm/attributes.py", line 730, in get
    value = self.callable_(state, passive)
  File ".../lib/python3.9/site-packages/sqlalchemy/orm/strategies.py", line 717, in _load_for_state
    raise orm_exc.DetachedInstanceError(
sqlalchemy.orm.exc.DetachedInstanceError: Parent instance <RepoGroup at 0x7f1f2664f4c0> is not bound to a Session; lazy load operation of attribute 'parent_group' cannot proceed (Background on this error at: http://sqlalche.me/e/13/bhk3)


In the reference 'gr.parent_group', 'gr' is an SQLAlchemy object referring
to the group being deleted, and 'gr.parent_group' is a lazy reference to its
parent group. The 'lazy' means that the parent group object is not loaded
automatically when 'gr' is assigned, but instead will be loaded on-the-fly
when the parent group is actually accessed. See [1] and [2] for more
information.

The problem was that the lazy 'parent_group' attribute was accessed _after_
deleting the database object it was part of.

Fix this by obtaining a handle to the parent group _before_ deleting the
subgroup.

Reported-by: André Klitzing (via mailing list)


[1] https://docs.sqlalchemy.org/en/13/errors.html#error-bhk3
[2] https://docs.sqlalchemy.org/en/13/orm/loading_relationships.html

diff -r 2ef4f7c0031e -r ee1da602b590 kallithea/controllers/admin/repo_groups.py
--- a/kallithea/controllers/admin/repo_groups.py	Wed Sep 01 22:44:49 2021 +0200
+++ b/kallithea/controllers/admin/repo_groups.py	Wed Sep 01 22:08:45 2021 +0200
@@ -245,6 +245,7 @@
     @HasRepoGroupPermissionLevelDecorator('admin')
     def delete(self, group_name):
         gr = c.repo_group = db.RepoGroup.guess_instance(group_name)
+        parent_group = gr.parent_group
         repos = gr.repositories.all()
         if repos:
             webutils.flash(_('This group contains %s repositories and cannot be '
@@ -268,8 +269,8 @@
             webutils.flash(_('Error occurred during deletion of repository group %s')
                     % group_name, category='error')
 
-        if gr.parent_group:
-            raise HTTPFound(location=url('repos_group_home', group_name=gr.parent_group.group_name))
+        if parent_group:
+            raise HTTPFound(location=url('repos_group_home', group_name=parent_group.group_name))
         raise HTTPFound(location=url('repos_groups'))
 
     def show_by_name(self, group_name):
diff -r 2ef4f7c0031e -r ee1da602b590 kallithea/tests/functional/test_admin_repo_groups.py
--- a/kallithea/tests/functional/test_admin_repo_groups.py	Wed Sep 01 22:44:49 2021 +0200
+++ b/kallithea/tests/functional/test_admin_repo_groups.py	Wed Sep 01 22:08:45 2021 +0200
@@ -1,4 +1,4 @@
-from kallithea.model import meta
+from kallithea.model import db, meta
 from kallithea.model.repo_group import RepoGroupModel
 from kallithea.tests import base
 from kallithea.tests.fixture import Fixture
@@ -97,3 +97,54 @@
 
         RepoGroupModel().delete(group_name)
         meta.Session().commit()
+
+    def test_subgroup_deletion(self):
+        self.log_user()
+        parent = None
+        parent_name = 'parent'
+        sub = None
+        sub_name = 'sub'
+        sub_path = 'parent/sub'
+
+        try:
+            # create parent group
+            assert db.RepoGroup.guess_instance(parent_name) is None
+            response = self.app.post(
+                base.url('repos_groups'),
+                fixture._get_repo_group_create_params(
+                    group_name=parent_name,
+                    _session_csrf_secret_token=self.session_csrf_secret_token()
+                )
+            )
+            parent = db.RepoGroup.guess_instance(parent_name)
+            assert parent is not None
+
+            # create sub group
+            assert db.RepoGroup.guess_instance(sub_path) is None
+            response = self.app.post(
+                base.url('repos_groups'),
+                fixture._get_repo_group_create_params(
+                    group_name=sub_name,
+                    parent_group_id=parent.group_id,
+                    _session_csrf_secret_token=self.session_csrf_secret_token()
+                )
+            )
+            sub = db.RepoGroup.guess_instance(sub_path)
+            assert sub is not None
+
+            # delete sub group
+            response = self.app.post(
+                base.url('delete_repo_group', group_name=sub_path),
+                params={
+                    '_session_csrf_secret_token': self.session_csrf_secret_token()
+                },
+            )
+            sub = db.RepoGroup.guess_instance(sub_path)
+            assert sub is None
+
+        finally:
+            if sub:
+                RepoGroupModel().delete(sub)
+            if parent:
+                RepoGroupModel().delete(parent)
+            meta.Session().commit()