From 759363b5f4d4b79b0ebb7533c9b03430b97abd84 Mon Sep 17 00:00:00 2001
From: Amit Kumar <aktech@users.noreply.github.com>
Date: Mon, 20 May 2024 16:32:52 +0100
Subject: [PATCH] Fix filtering for shared servers (#285)

* Fix filtering for shared servers

* remove unused params

* add tests and minor refactoring
---
 jhub_apps/service/routes.py | 16 +---------------
 jhub_apps/service/utils.py  | 19 +++++++++++++++++++
 jhub_apps/tests/test_api.py | 38 +++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/jhub_apps/service/routes.py b/jhub_apps/service/routes.py
index 3078b26d..4cd8a798 100644
--- a/jhub_apps/service/routes.py
+++ b/jhub_apps/service/routes.py
@@ -34,7 +34,7 @@
     get_conda_envs,
     get_jupyterhub_config,
     get_spawner_profiles,
-    get_thumbnail_data_url,
+    get_thumbnail_data_url, get_shared_servers,
 )
 from jhub_apps.spawner.types import FRAMEWORKS
 from jhub_apps.version import get_version
@@ -94,20 +94,6 @@ async def login(request: Request):
     return RedirectResponse(authorization_url, status_code=302)
 
 
-def get_shared_servers(user_servers, current_hub_user):
-    user_servers_without_default_jlab = list(filter(lambda server: server["name"] != "", user_servers.values()))
-    # Filter servers shared with the user
-    hub_client = HubClient(username=current_hub_user["name"])
-    shared_servers = hub_client.get_shared_servers()
-    shared_server_names = {shared_server["server"]["name"] for shared_server in shared_servers}
-    shared_servers_rich = [
-        server for server in user_servers_without_default_jlab
-        if server["name"] in shared_server_names
-    ]
-    return shared_servers_rich
-
-
-
 @router.get("/server/", description="Get all servers")
 @router.get("/server/{server_name}", description="Get a server by server name")
 async def get_server(user: User = Depends(get_current_user), server_name=None):
diff --git a/jhub_apps/service/utils.py b/jhub_apps/service/utils.py
index bff1dba5..7cf18657 100644
--- a/jhub_apps/service/utils.py
+++ b/jhub_apps/service/utils.py
@@ -1,4 +1,5 @@
 import base64
+import itertools
 
 import structlog
 import os
@@ -9,6 +10,7 @@
 from jupyterhub.app import JupyterHub
 from traitlets.config import LazyConfigValue
 
+from jhub_apps.hub_client.hub_client import HubClient
 from jhub_apps.spawner.types import FrameworkConf, FRAMEWORKS_MAPPING
 from slugify import slugify
 
@@ -145,3 +147,20 @@ def get_theme(config):
         return config.JupyterHub.template_vars
     else:
         return None
+
+
+def get_shared_servers(current_hub_user):
+    # Filter servers shared with the user
+    hub_client_service = HubClient()
+    all_users_servers = list(itertools.chain.from_iterable([
+        list(user['servers'].values()) for user in hub_client_service.get_users()
+    ]))
+    user_servers_without_default_jlab = list(filter(lambda server: server["name"] != "", all_users_servers))
+    hub_client_user = HubClient(username=current_hub_user['name'])
+    shared_servers = hub_client_user.get_shared_servers()
+    shared_server_names = {shared_server["server"]["name"] for shared_server in shared_servers}
+    shared_servers_rich = [
+        server for server in user_servers_without_default_jlab
+        if server["name"] in shared_server_names
+    ]
+    return shared_servers_rich
diff --git a/jhub_apps/tests/test_api.py b/jhub_apps/tests/test_api.py
index 039aea8c..c7cc3543 100644
--- a/jhub_apps/tests/test_api.py
+++ b/jhub_apps/tests/test_api.py
@@ -5,6 +5,7 @@
 import pytest
 
 from jhub_apps.hub_client.hub_client import HubClient
+from jhub_apps.service.utils import get_shared_servers
 from jhub_apps.spawner.types import FRAMEWORKS
 from jhub_apps.tests.constants import MOCK_USER
 
@@ -144,6 +145,43 @@ def test_api_update_server(edit_server, client):
     assert response.json() == create_server_response
 
 
+@patch.object(HubClient, "get_users")
+@patch.object(HubClient, "get_shared_servers")
+def test_shared_server_filtering(hub_get_shared_servers, get_users):
+    current_hub_user = {"name": "fakeuser"}
+    get_users.return_value = [
+        {
+            "servers": {
+                '': {'name': ''},
+                "panel-12": {"name": "panel-12"}
+            }
+        },
+        {
+            "servers": {
+                '': {'name': ''},
+                "panel-34": {"name": "panel-34", "fullname": "panel shared 34"}
+            }
+        },
+        {
+            "servers": {
+                '': {'name': ''},
+                "panel-56": {"name": "panel-56", "fullname": "panel shared server"}
+            }
+        }
+    ]
+    hub_get_shared_servers.return_value = [
+        {"server": {"name": "panel-56"}},
+        {"server": {"name": "panel-34"}},
+    ]
+    shared_servers = get_shared_servers(current_hub_user)
+    assert shared_servers == [
+        {"name": "panel-34", "fullname": "panel shared 34"},
+        {"name": "panel-56", "fullname": "panel shared server"}
+    ]
+    hub_get_shared_servers.assert_called_once_with()
+    get_users.assert_called_once_with()
+
+
 def test_api_frameworks(client):
     response = client.get(
         "/frameworks",