From 8dc867476f9b46f714e6595389a0cba5653b6a6d Mon Sep 17 00:00:00 2001
From: Arjan Molenaar <gaphor@gmail.com>
Date: Mon, 13 Nov 2023 23:16:04 +0100
Subject: [PATCH] fix: clean up duplicate code

---
 backend/capellacollab/sessions/routes.py      | 211 ++++++++----------
 .../tests/sessions/test_sessions_routes.py    |   6 +-
 2 files changed, 104 insertions(+), 113 deletions(-)

diff --git a/backend/capellacollab/sessions/routes.py b/backend/capellacollab/sessions/routes.py
index b7f889dde3..3b1d04d873 100644
--- a/backend/capellacollab/sessions/routes.py
+++ b/backend/capellacollab/sessions/routes.py
@@ -293,13 +293,71 @@ def request_persistent_session(
 
     raise_if_conflicting_persistent_sessions(tool, user)
 
-    response = start_persistent_session(user, tool, version, operator, db)
+    docker_image = get_image_for_tool_version(db, version.id)
+    response = start_persistent_session(
+        db=db,
+        user=user,
+        tool=tool,
+        version=version,
+        operator=operator,
+        docker_image=docker_image,
+        environment={},
+    )
 
     return response
 
 
-def start_persistent_session(user, tool, version, operator, db):
-    environment: dict[str, str] = {}
+def raise_if_conflicting_persistent_sessions(
+    tool: tools_models.DatabaseTool,
+    user: users_models.DatabaseUser,
+) -> None:
+    existing_user_sessions = user.sessions
+
+    # This is a temporary workaround until we can define a proper locking of workspaces
+    # Currently, all tools share one workspace. Eclipse based tools lock the workspace.
+    # We can only run one Eclipse-based tool at a time.
+    # Status tracked in https://github.com/DSD-DBS/capella-collab-manager/issues/847
+    if tool.integrations.jupyter:
+        # Check if there is already an Jupyter session running.
+        if True in [
+            session.tool.integrations.jupyter
+            for session in existing_user_sessions
+        ]:
+            raise fastapi.HTTPException(
+                status_code=status.HTTP_409_CONFLICT,
+                detail={
+                    "err_code": "EXISTING_SESSION",
+                    "reason": "You already have a Jupyter session. Please navigate to 'Active Sessions' to connect.",
+                },
+            )
+    else:
+        if models.WorkspaceType.PERSISTENT in [
+            session.type
+            for session in existing_user_sessions
+            if session.tool.name != "Jupyter"
+        ]:
+            raise fastapi.HTTPException(
+                status_code=status.HTTP_409_CONFLICT,
+                detail={
+                    "err_code": "EXISTING_SESSION",
+                    "reason": (
+                        "You already have a open persistent Eclipse-based session.",
+                        "Currently, we can only run one Eclipse-based session at a time.",
+                    ),
+                },
+            )
+
+
+def start_persistent_session(
+    db,
+    user,
+    tool,
+    version,
+    operator,
+    docker_image,
+    environment,
+    session_type="persistent",
+):
     volumes: list[operators_models.Volume] = []
     warnings: list[core_models.Message] = []
 
@@ -316,8 +374,6 @@ def start_persistent_session(user, tool, version, operator, db):
         volumes += hook_volumes
         warnings += hook_warnings
 
-    docker_image = get_image_for_tool_version(db, version.id)
-
     if tool.integrations and tool.integrations.jupyter:
         response = start_persistent_jupyter_session(
             db=db,
@@ -328,6 +384,7 @@ def start_persistent_session(user, tool, version, operator, db):
             volumes=volumes,
             environment=environment,
             docker_image=docker_image,
+            session_type=session_type,
         )
     else:
         response = start_persistent_guacamole_session(
@@ -340,6 +397,7 @@ def start_persistent_session(user, tool, version, operator, db):
             volumes=volumes,
             environment=environment,
             docker_image=docker_image,
+            session_type=session_type,
         )
 
     for hook in hooks.get_activated_integration_hooks(tool):
@@ -352,47 +410,6 @@ def start_persistent_session(user, tool, version, operator, db):
     return response
 
 
-def raise_if_conflicting_persistent_sessions(
-    tool: tools_models.DatabaseTool,
-    user: users_models.DatabaseUser,
-) -> None:
-    existing_user_sessions = user.sessions
-
-    # This is a temporary workaround until we can define a proper locking of workspaces
-    # Currently, all tools share one workspace. Eclipse based tools lock the workspace.
-    # We can only run one Eclipse-based tool at a time.
-    # Status tracked in https://github.com/DSD-DBS/capella-collab-manager/issues/847
-    if tool.integrations.jupyter:
-        # Check if there is already an Jupyter session running.
-        if True in [
-            session.tool.integrations.jupyter
-            for session in existing_user_sessions
-        ]:
-            raise fastapi.HTTPException(
-                status_code=status.HTTP_409_CONFLICT,
-                detail={
-                    "err_code": "EXISTING_SESSION",
-                    "reason": "You already have a Jupyter session. Please navigate to 'Active Sessions' to connect.",
-                },
-            )
-    else:
-        if models.WorkspaceType.PERSISTENT in [
-            session.type
-            for session in existing_user_sessions
-            if session.tool.name != "Jupyter"
-        ]:
-            raise fastapi.HTTPException(
-                status_code=status.HTTP_409_CONFLICT,
-                detail={
-                    "err_code": "EXISTING_SESSION",
-                    "reason": (
-                        "You already have a open persistent Eclipse-based session.",
-                        "Currently, we can only run one Eclipse-based session at a time.",
-                    ),
-                },
-            )
-
-
 def start_persistent_jupyter_session(
     db: orm.Session,
     operator: k8s.KubernetesOperator,
@@ -609,7 +626,6 @@ def request_provision_workspace(
     launched_sessions = []
     for model_group in grouped_models:
         model = model_group[0]
-        tool = model.version.tool
         warnings: list[core_models.Message] = []
 
         # Start sessions for each tool and tell it to load models, same as readonly sessions
@@ -631,72 +647,19 @@ def request_provision_workspace(
             )
             continue
 
-        rdp_password = credentials.generate_password(length=64)
-
-        environment: dict[str, str] = {
-            "GIT_REPOS_JSON": json.dumps(
-                [
-                    git_model_as_json(
-                        git_model=git_model,
-                        revision=git_model.revision,
-                        deep_clone=False,
-                    )
-                    for m in model_group
-                    for git_model in m.git_models
-                    if git_model.primary
-                ]
-            ),
-            "RMT_PASSWORD": rdp_password,
-            "WORKSPACE_DIR": f"/workspace/projects/{project.slug}/{tool.slug}/{model.version.slug}",
-        }
-
-        volumes: list[operators_models.Volume] = []
-
-        for hook in hooks.get_activated_integration_hooks(tool):
-            hook_env, hook_volumes, hook_warnings = hook.configuration_hook(
-                db=db,
-                user=user,
-                tool_version=model.version,
-                tool=tool,
-                username=user.name,
-                operator=operator,
-            )
-            environment |= hook_env
-            volumes += hook_volumes
-            warnings += hook_warnings
-
-        if tool.integrations and tool.integrations.jupyter:
-            response = start_persistent_jupyter_session(
-                db=db,
-                operator=operator,
-                owner=user.name,
-                tool=tool,
-                version=model.version,
-                volumes=volumes,
-                environment=environment,
-                docker_image=docker_image,
-                session_type="readonly",
-            )
-        else:
-            response = start_persistent_guacamole_session(
-                db=db,
-                operator=operator,
-                user=user,
-                owner=user.name,
-                tool=tool,
-                version=model.version,
-                volumes=volumes,
-                environment=environment,
-                docker_image=docker_image,
-                session_type="readonly",
-            )
-
-        for hook in hooks.get_activated_integration_hooks(tool):
-            hook.post_session_creation_hook(
-                session_id=response.id, operator=operator, user=user
-            )
+        environment = create_environment(model_group)
+        response = start_persistent_session(
+            db=db,
+            user=user,
+            tool=model.version.tool,
+            version=model.version,
+            operator=operator,
+            docker_image=docker_image,
+            environment=environment,
+            session_type="readonly",
+        )
+        response.warnings += warnings
 
-        response.warnings = warnings
         launched_sessions.append(response)
 
     return launched_sessions
@@ -713,6 +676,30 @@ def group_models_by_tool_version(
     )
 
 
+def create_environment(
+    model_group: list[toolmodels_models.DatabaseCapellaModel],
+) -> dict[str, str]:
+    model = model_group[0]
+    rdp_password = credentials.generate_password(length=64)
+
+    return {
+        "GIT_REPOS_JSON": json.dumps(
+            [
+                git_model_as_json(
+                    git_model=git_model,
+                    revision=git_model.revision,
+                    deep_clone=False,
+                )
+                for m in model_group
+                for git_model in m.git_models
+                if git_model.primary
+            ]
+        ),
+        "RMT_PASSWORD": rdp_password,
+        "WORKSPACE_DIR": f"/workspace/projects/{model.project.slug}/{model.version.tool.slug}/{model.version.slug}",
+    }
+
+
 @router.delete("/{session_id}", status_code=204)
 def end_session(
     db: orm.Session = fastapi.Depends(database.get_db),
diff --git a/backend/tests/sessions/test_sessions_routes.py b/backend/tests/sessions/test_sessions_routes.py
index 5d558066f8..7dc61283bd 100644
--- a/backend/tests/sessions/test_sessions_routes.py
+++ b/backend/tests/sessions/test_sessions_routes.py
@@ -309,7 +309,11 @@ def setup_model(db, project, version):
         db,
         model,
         PostGitModel(
-            path=git_path, entrypoint="", revision="", username="", password=""
+            path=git_path,
+            entrypoint="",
+            revision="main",
+            username="",
+            password="",
         ),
     )
     return model, git_model