Merge pull request #2514 from kbase/fix-backend-tests

Fix ws.list_objects requiring ws ids

RELEASE_NOTES.md

@@ -5,6 +5,10 @@ The Narrative Interface allows users to craft KBase Narratives using a combinati
 This is built on the Jupyter Notebook v6.0.2 (more notes will follow).
 
 ### Version 4.5.1
+Code changes
+- DATAUP-599 - Adjusted the kernel code and tests to account for a Workspace service update.
+
+Dependency Changes
 - Python dependency updates
   - pillow 8.3.1 -> 8.3.2
   - plotly 5.1.0 -> 5.3.1

src/biokbase/narrative/contents/narrativeio.py

@@ -24,7 +24,7 @@
 # The list_workspace_objects method has been deprecated, the
 # list_objects method is the current primary method for fetching
 # objects, and has a different field list
-list_objects_fields = [
+LIST_OBJECTS_FIELDS = [
     "objid",
     "name",
     "type",
@@ -37,10 +37,11 @@
     "size",
     "meta",
 ]
-obj_field = dict(zip(list_objects_fields, range(len(list_objects_fields))))
+obj_field = dict(zip(LIST_OBJECTS_FIELDS, range(len(LIST_OBJECTS_FIELDS))))
 
 obj_ref_regex = re.compile(r"^(?P<wsid>\d+)\/(?P<objid>\d+)(\/(?P<ver>\d+))?$")
 
+MAX_WORKSPACES = 10000  # from ws.list_objects
 MAX_METADATA_STRING_BYTES = 900
 MAX_METADATA_SIZE_BYTES = 16000
 WORKSPACE_TIMEOUT = 30  # seconds
@@ -55,7 +56,6 @@ class KBaseWSManagerMixin(object):
     """
 
     ws_uri = URLS.workspace
-    nar_type = "KBaseNarrative.Narrative"
 
     def __init__(self, *args, **kwargs):
         if not self.ws_uri:
@@ -201,7 +201,7 @@ def write_narrative(self, ref, nb, cur_user):
             if "creator" not in meta:
                 meta["creator"] = cur_user
             if "type" not in meta:
-                meta["type"] = self.nar_type
+                meta["type"] = NARRATIVE_TYPE
             if "description" not in meta:
                 meta["description"] = ""
             if "data_dependencies" not in meta:
@@ -253,7 +253,7 @@ def write_narrative(self, ref, nb, cur_user):
         # Now we can save the Narrative object.
         try:
             ws_save_obj = {
-                "type": self.nar_type,
+                "type": NARRATIVE_TYPE,
                 "data": nb,
                 "objid": obj_id,
                 "meta": nb["metadata"].copy(),
@@ -520,20 +520,30 @@ def list_narratives(self, ws_id=None):
         numeric.
         """
         log_event(g_log, "list_narratives start", {"ws_id": ws_id})
-        list_obj_params = {"type": self.nar_type, "includeMetadata": 1}
+
+        ws = self.ws_client()
         if ws_id:
-            try:
-                int(ws_id)  # will throw an exception if ws_id isn't an int
-                list_obj_params["ids"] = [ws_id]
-            except ValueError:
-                raise
+            ws_ids = [int(ws_id)]  # will throw an exception if ws_id isn't an int
+        else:
+            ret = ws.list_workspace_ids(
+                {"perm": "r", "onlyGlobal": 0, "excludeGlobal": 0}
+            )
+            ws_ids = ret.get("workspaces", []) + ret.get("pub", [])
 
         try:
-            ws = self.ws_client()
-            res = ws.list_objects(list_obj_params)
+            res = []
+            for i in range(0, len(ws_ids), MAX_WORKSPACES):
+                res += ws.list_objects(
+                    {
+                        "ids": ws_ids[i:i + MAX_WORKSPACES],
+                        "type": NARRATIVE_TYPE,
+                        "includeMetadata": 1
+                    }
+                )
         except ServerError as err:
-            raise WorkspaceError(err, ws_id)
-        my_narratives = [dict(zip(list_objects_fields, obj)) for obj in res]
+            raise WorkspaceError(err, ws_ids)
+
+        my_narratives = [dict(zip(LIST_OBJECTS_FIELDS, obj)) for obj in res]
         for nar in my_narratives:
             # Look first for the name in the object metadata. if it's not there, use
             # the object's name. If THAT'S not there, use Untitled.

src/biokbase/narrative/tests/narrative_mock/mockclients.py

@@ -2,6 +2,22 @@
 from biokbase.workspace.baseclient import ServerError
 
 
+def get_nar_obj(i):
+    return [
+        i,
+        "My_Test_Narrative",
+        "KBaseNarrative.Narrative",
+        "2017-03-31T23:42:59+0000",
+        1,
+        "wjriehl",
+        18836,
+        "wjriehl:1490995018528",
+        "278abf8f0dbf8ab5ce349598a8674a6e",
+        109180038,
+        {},
+    ]
+
+
 class MockClients:
     """
     Mock KBase service clients as needed for Narrative backend tests.
@@ -216,6 +232,10 @@ def get_object_info3(self, params):
         num_objects = len(params.get("objects", [0]))
         return {"infos": infos * num_objects, "paths": paths * num_objects}
 
+    def list_objects(self, params):
+        ws_ids = params["ids"]
+        return [get_nar_obj(int(id)) for id in ws_ids]  # assert int
+
     # ----- Narrative Job Service functions -----
 
     def run_job(self, params):

src/biokbase/narrative/tests/test_narrativeio.py

@@ -3,11 +3,13 @@
 Narrative and workspace service.
 """
 import unittest
-from biokbase.narrative.contents.narrativeio import KBaseWSManagerMixin
+from unittest.mock import patch
+from biokbase.narrative.contents.narrativeio import KBaseWSManagerMixin, LIST_OBJECTS_FIELDS
 from biokbase.narrative.common.exceptions import WorkspaceError
 import biokbase.auth
 from tornado.web import HTTPError
 from . import util
+from .narrative_mock.mockclients import get_mock_client, MockClients, get_nar_obj
 from biokbase.narrative.common.url_config import URLS
 from biokbase.narrative.common.narrative_ref import NarrativeRef
 import biokbase.narrative.clients as clients
@@ -32,6 +34,10 @@
 HAS_TEST_TOKEN = False
 
 
+def get_exp_nar(i):
+    return dict(zip(LIST_OBJECTS_FIELDS, get_nar_obj(i)))
+
+
 def skipUnlessToken():
     global HAS_TEST_TOKEN
     if not HAS_TEST_TOKEN:
@@ -71,9 +77,10 @@ def setUpClass(self):
         if self.test_token is None or self.private_token is None:
             print("Skipping most narrativeio.py tests due to missing tokens.")
             print(
-                "To enable these, place a valid auth token in files\n{}\nand\n{}".format(
-                    config.get_path("token_files", "test_user"),
-                    config.get_path("token_files", "private_user"),
+                "To enable these, update {} and place a valid auth token in files\n{}\nand\n{}".format(
+                    config.config_file_path,
+                    config.get_path("token_files", "test_user", from_root=True),
+                    config.get_path("token_files", "private_user", from_root=True),
                 )
             )
             print("Note that these should belong to different users.")
@@ -512,6 +519,46 @@ def test_list_narrative_ws_valid_login(self):
         self.validate_narrative_list(res)
         self.logout()
 
+    @patch("biokbase.narrative.clients.get", get_mock_client)
+    def test_list_narratives__no_ws_id__0_ws_ids(self):
+        ws_ids = {"workspaces": [], "pub": []}
+
+        with patch.object(MockClients, "list_workspace_ids", create=True, return_value=ws_ids):
+            nar_l = self.mixin.list_narratives()
+
+        self.assertEqual([], nar_l)
+        self.validate_narrative_list(nar_l)
+
+    @patch("biokbase.narrative.clients.get", get_mock_client)
+    def test_list_narratives__no_ws_id__9999_ws_ids(self):
+        ws_ids = {"workspaces": list(range(9999)), "pub": []}
+
+        with patch.object(MockClients, "list_workspace_ids", create=True, return_value=ws_ids):
+            nar_l = self.mixin.list_narratives()
+
+        self.assertEqual([get_exp_nar(i) for i in range(9999)], nar_l)
+        self.validate_narrative_list(nar_l)
+
+    @patch("biokbase.narrative.clients.get", get_mock_client)
+    def test_list_narratives__no_ws_id__10000_ws_ids(self):
+        ws_ids = {"workspaces": list(range(10000)), "pub": []}
+
+        with patch.object(MockClients, "list_workspace_ids", create=True, return_value=ws_ids):
+            nar_l = self.mixin.list_narratives()
+
+        self.assertEqual([get_exp_nar(i) for i in range(10000)], nar_l)
+        self.validate_narrative_list(nar_l)
+
+    @patch("biokbase.narrative.clients.get", get_mock_client)
+    def test_list_narratives__no_ws_id__10001_ws_ids(self):
+        ws_ids = {"workspaces": list(range(10000)), "pub": [10000]}
+
+        with patch.object(MockClients, "list_workspace_ids", create=True, return_value=ws_ids):
+            nar_l = self.mixin.list_narratives()
+
+        self.assertEqual([get_exp_nar(i) for i in range(10001)], nar_l)
+        self.validate_narrative_list(nar_l)
+
     def test_list_narrative_ws_invalid(self):
         with self.assertRaises(WorkspaceError) as err:
             self.mixin.list_narratives(ws_id=self.invalid_ws_id)

src/biokbase/narrative/tests/util.py

@@ -43,9 +43,9 @@ def __init__(self):
             os.environ["NARRATIVE_DIR"], "src", "biokbase", "narrative", "tests"
         )
         self._path_root = os.path.join(os.environ["NARRATIVE_DIR"])
-        config_file_path = self.file_path(_config_file)
+        self.config_file_path = self.file_path(_config_file)
         self._config = configparser.ConfigParser()
-        self._config.read(config_file_path)
+        self._config.read(self.config_file_path)
 
     def get(self, *args, **kwargs):
         return self._config.get(*args, **kwargs)

src/biokbase/workspace/client.py

@@ -2942,6 +2942,35 @@ def list_workspace_info(self, params, context=None):
             "Workspace.list_workspace_info", [params], self._service_ver, context
         )
 
+    def list_workspace_ids(self, params, context=None):
+        """
+        List workspace IDs to which the user has access.
+        This function returns a subset of the information in the
+        list_workspace_info method and should be substantially faster.
+        :param params: instance of type "ListWorkspaceIDsParams" (Input
+           parameters for the "list_workspace_ids" function. Optional
+           parameters: permission perm - filter workspaces by minimum
+           permission level. 'None' and 'readable' are ignored. boolean
+           onlyGlobal - if onlyGlobal is true only include world readable
+           workspaces. Defaults to false. If true, excludeGlobal is ignored.
+           boolean excludeGlobal - if excludeGlobal is true exclude world
+           readable workspaces. Defaults to true.) -> structure: parameter
+           "perm" of type "permission" (Represents the permissions a user or
+           users have to a workspace: 'a' - administrator. All operations
+           allowed. 'w' - read/write. 'r' - read. 'n' - no permissions.),
+           parameter "excludeGlobal" of type "boolean" (A boolean. 0 = false,
+           other = true.), parameter "onlyGlobal" of type "boolean" (A
+           boolean. 0 = false, other = true.)
+        :returns: instance of type "ListWorkspaceIDsResults" (Results of the
+           "list_workspace_ids" function. list<int> workspaces - the
+           workspaces to which the user has explicit access. list<int> pub -
+           the workspaces to which the user has access because they're
+           globally readable.) -> structure: parameter "workspaces" of list
+           of Long, parameter "pub" of list of Long
+        """
+        return self._client.call_method('Workspace.list_workspace_ids',
+                                        [params], self._service_ver, context)
+
     def list_workspace_objects(self, params, context=None):
         """
         Lists the metadata of all objects in the specified workspace with the