Merge pull request #11386 from davelopez/refactor_display_applications_api

Refactor display_applications API logic into DisplayApplicationsManager

lib/galaxy/datatypes/registry.py

@@ -6,6 +6,7 @@ import imp
 import logging
 import os
 from string import Template
+from typing import Dict
 
 import yaml
 
@@ -57,7 +58,7 @@ class Registry:
         # tool shed repositories that contain display applications.
         self.proprietary_display_app_containers = []
         # Map a display application id to a display application
-        self.display_applications = {}
+        self.display_applications: Dict[str, DisplayApplication] = {}
         # The following 2 attributes are used in the to_xml_file()
         # method to persist the current state into an xml file.
         self.display_path_attr = None

lib/galaxy/managers/display_applications.py

@@ -0,0 +1,61 @@
+import logging
+from typing import (
+    Any, Dict,
+    List,
+)
+
+from galaxy.datatypes.registry import Registry
+
+log = logging.getLogger(__name__)
+
+
+class DisplayApplicationsManager:
+    """Interface/service object for sharing logic between controllers."""
+
+    def __init__(self, app):
+        self._app = app
+
+    @property
+    def datatypes_registry(self) -> Registry:
+        return self._app.datatypes_registry
+
+    def index(self) -> List[Any]:
+        """
+        Returns the list of display applications.
+
+        :returns:   list of available display applications
+        :rtype:     list
+        """
+        rval = []
+        for display_app in self.datatypes_registry.display_applications.values():
+            rval.append({
+                'id': display_app.id,
+                'name': display_app.name,
+                'version': display_app.version,
+                'filename_': display_app._filename,
+                'links': [{'name': link.name} for link in display_app.links.values()]
+            })
+        return rval
+
+    def reload(self, ids: List[str]) -> Dict[str, Any]:
+        """
+        Reloads the list of display applications.
+
+        :param  ids:  list containing ids of display to be reloaded
+        :type   ids:  list
+        """
+        self._app.queue_worker.send_control_task(
+            'reload_display_application',
+            noop_self=True,
+            kwargs={'display_application_ids': ids}
+        )
+        reloaded, failed = self.datatypes_registry.reload_display_applications(ids)
+        if not reloaded and failed:
+            message = 'Unable to reload any of the %i requested display applications ("%s").' % (len(failed), '", "'.join(failed))
+        elif failed:
+            message = 'Reloaded %i display applications ("%s"), but failed to reload %i display applications ("%s").' % (len(reloaded), '", "'.join(reloaded), len(failed), '", "'.join(failed))
+        elif not reloaded:
+            message = 'You need to request at least one display application to reload.'
+        else:
+            message = 'Reloaded %i requested display applications ("%s").' % (len(reloaded), '", "'.join(reloaded))
+        return {'message': message, 'reloaded': reloaded, 'failed': failed}

lib/galaxy/webapps/galaxy/api/display_applications.py

@@ -3,7 +3,12 @@ API operations on annotations.
 """
 import logging
 
-from galaxy.web import legacy_expose_api, require_admin
+from galaxy.app import StructuredApp
+from galaxy.managers.display_applications import DisplayApplicationsManager
+from galaxy.web import (
+    expose_api,
+    require_admin,
+)
 from galaxy.webapps.base.controller import BaseAPIController
 
 log = logging.getLogger(__name__)
@@ -11,7 +16,11 @@ log = logging.getLogger(__name__)
 
 class DisplayApplicationsController(BaseAPIController):
 
-    @legacy_expose_api
+    def __init__(self, app: StructuredApp):
+        super().__init__(app)
+        self.manager = DisplayApplicationsManager(app)
+
+    @expose_api
     def index(self, trans, **kwd):
         """
         GET /api/display_applications/
@@ -21,19 +30,10 @@ class DisplayApplicationsController(BaseAPIController):
         :returns:   list of available display applications
         :rtype:     list
         """
-        response = []
-        for display_app in trans.app.datatypes_registry.display_applications.values():
-            response.append({
-                'id': display_app.id,
-                'name': display_app.name,
-                'version': display_app.version,
-                'filename_': display_app._filename,
-                'links': [{'name': link.name} for link in display_app.links.values()]
-            })
-        return response
+        return self.manager.index()
 
+    @expose_api
     @require_admin
-    @legacy_expose_api
     def reload(self, trans, payload=None, **kwd):
         """
         POST /api/display_applications/reload
@@ -44,19 +44,5 @@ class DisplayApplicationsController(BaseAPIController):
         :type   ids:  list
         """
         payload = payload or {}
-        ids = payload.get('ids')
-        trans.app.queue_worker.send_control_task(
-            'reload_display_application',
-            noop_self=True,
-            kwargs={'display_application_ids': ids}
-        )
-        reloaded, failed = trans.app.datatypes_registry.reload_display_applications(ids)
-        if not reloaded and failed:
-            message = 'Unable to reload any of the %i requested display applications ("%s").' % (len(failed), '", "'.join(failed))
-        elif failed:
-            message = 'Reloaded %i display applications ("%s"), but failed to reload %i display applications ("%s").' % (len(reloaded), '", "'.join(reloaded), len(failed), '", "'.join(failed))
-        elif not reloaded:
-            message = 'You need to request at least one display application to reload.'
-        else:
-            message = 'Reloaded %i requested display applications ("%s").' % (len(reloaded), '", "'.join(reloaded))
-        return {'message': message, 'reloaded': reloaded, 'failed': failed}
+        ids = payload.get('ids', [])
+        return self.manager.reload(ids)

lib/galaxy_test/api/test_display_applications.py

@@ -0,0 +1,53 @@
+import random
+from typing import List
+
+from ._framework import ApiTestCase
+
+
+class DisplayApplicationsApiTestCase(ApiTestCase):
+
+    def test_index(self):
+        response = self._get("display_applications")
+        self._assert_status_code_is(response, 200)
+        as_list = response.json()
+        assert isinstance(as_list, list)
+        assert len(as_list) > 0
+        for display_app in as_list:
+            self._assert_has_keys(display_app, "id", "name", "version", "filename_", "links")
+
+    def test_reload_as_admin(self):
+        response = self._post("display_applications/reload", admin=True)
+        self._assert_status_code_is(response, 200)
+
+    def test_reload_with_some_ids(self):
+        response = self._get("display_applications")
+        self._assert_status_code_is(response, 200)
+        display_apps = response.json()
+        all_ids = [display_app["id"] for display_app in display_apps]
+        input_ids = self._get_half_random_items(all_ids)
+        payload = {'ids': input_ids}
+        response = self._post("display_applications/reload", payload, admin=True)
+        self._assert_status_code_is(response, 200)
+        reloaded = response.json()["reloaded"]
+        assert len(reloaded) == len(input_ids)
+        assert all(elem in reloaded for elem in input_ids)
+
+    def test_reload_unknow_returns_as_failed(self):
+        unknown_id = "unknown"
+        payload = {'ids': [unknown_id]}
+        response = self._post("display_applications/reload", payload, admin=True)
+        self._assert_status_code_is(response, 200)
+        reloaded = response.json()["reloaded"]
+        failed = response.json()["failed"]
+        assert len(reloaded) == 0
+        assert len(failed) == 1
+        assert unknown_id in failed
+
+    def test_reload_as_non_admin_returns_403(self):
+        response = self._post("display_applications/reload")
+        self._assert_status_code_is(response, 403)
+
+    def _get_half_random_items(self, collection: List[str]) -> List[str]:
+        half_num_items = int(len(collection) / 2)
+        rval = random.sample(collection, half_num_items)
+        return rval