diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index cf89ca8410..10ce6a3bdd 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -7,7 +7,7 @@ This is built on the Jupyter Notebook v6.4.12 and IPython 8.5.0 (more notes will ## Unreleased - PTV-1875 - fix public data paging issue by removing paging from workspace data sources - UIP-28 - update Google Analytics tags to GA4 properties -- PTV-1810 - address object name display issues in the View Configure tab of app cells. +- PTV-1810 - address object name display issues in the View Configure tab of app cells. This now saves all app inputs as UPAs in the cell. It also includes an update to input transforms to properly convert from UPAs <-> names or references as appropriate before starting the app. - PTV-1877 - fix app descriptions to replace the documentation link for the upload / download guide - PTV-1878 - fix some failing front end unit tests diff --git a/src/biokbase/narrative/app_util.py b/src/biokbase/narrative/app_util.py index 5c95a31f8e..d8b2bd8536 100644 --- a/src/biokbase/narrative/app_util.py +++ b/src/biokbase/narrative/app_util.py @@ -1,7 +1,9 @@ import json import re +from typing import Any, Optional import biokbase.narrative.clients as clients +import biokbase.narrative.upa as upa from biokbase.narrative.system import system_variable """ @@ -10,6 +12,7 @@ app_version_tags = ["release", "beta", "dev"] + def check_tag(tag, raise_exception=False): """ Checks if the given tag is one of "release", "beta", or "dev". @@ -25,7 +28,6 @@ def check_tag(tag, raise_exception=False): return tag_exists - def map_inputs_from_job(job_inputs, app_spec): """ Unmaps the actual list of job inputs back to the @@ -93,7 +95,7 @@ def _untransform(transform_type, value): if slash == -1: return value else: - return value[slash + 1 :] + return value[slash + 1:] else: return value @@ -113,7 +115,8 @@ def app_param(p): "type": string "is_output": boolean "allow_multiple": boolean - "allowed_values": present if type == "dropdown" or "checkbox", a list of allowed dropdown values + "allowed_values": present if type == "dropdown" or "checkbox", a list of allowed dropdown + values "default": None, singleton, or list if "allow_multiple" is True "checkbox_map": [checked_value, unchecked_value] if type == checkbox The following only apply if the "text_options" field is present in the original spec @@ -259,7 +262,8 @@ def get_result_sub_path(result, path): So it recursively works through. look at path[pos] - that's a 0, so get the 0th element of result (the dict that's there.) - since result is a list (it almost always is, to start with, since that's what run_job returns), we get: + since result is a list (it almost always is, to start with, since that's what run_job + returns), we get: result[path[0]] = {'report': 'asdf', 'report_ref': 'xyz'} path = ['0', 'report_ref'] pos = 1 @@ -685,12 +689,8 @@ def resolve_single_ref(workspace, value): for path_item in path_items: if len(path_item.split("/")) > 3: raise ValueError( - "Object reference {} has too many slashes - should be workspace/object/version(optional)".format( - value - ) + f"Object reference {value} has too many slashes - should be ws/object/version" ) - # return (ws_ref, 'Data reference named {} does not have the right format - # - should be workspace/object/version(optional)') info = clients.get("workspace").get_object_info_new( {"objects": [{"ref": value}]} )[0] @@ -727,7 +727,9 @@ def resolve_ref_if_typed(value, spec_param): return value -def transform_param_value(transform_type, value, spec_param): +def transform_param_value( + transform_type: Optional[str], value: Any, spec_param: Optional[dict] +) -> Any: """ Transforms an input according to the rules given in NarrativeMethodStore.ServiceMethodInputMapping @@ -738,7 +740,62 @@ def transform_param_value(transform_type, value, spec_param): (4.) none or None - doesn't transform. Returns a transformed (or not) value. + + Rules and logic, esp for objects being sent. + 1. Test if current transform applies. (None is a valid transform) + A. Check if input is an object - valid transforms are ref, resolved-ref, list, + list, None + B. If not object, int, list, and None are allowed. + 2. If object and not output field, apply transform as follows: + A. None -> returns only object name + B. ref -> returns workspace_name/object_name + C. resolved-ref -> returns UPA + D. (not in docs yet) upa -> returns UPA + E. any of the above can be applied in list + 3. Exception: if the input is an UPA path or reference path, it should only get transformed + to an UPA path. + + This function will attempt to transform value according to the above rules. If value looks + like a ref (ws_name/obj_name) and transform_type is None, then obj_name will be returned. + Likewise, if resolved-ref is given, and value looks like an UPA already, then the already + resolved-ref will be returned. + + Parameters: + transform_type - str/None - should be one of the following, if not None: + * string + * int + * ref + * resolved-ref + * upa + * list where X is any of the above + value - anything or None. Parameter values are expected, by the KBase app stack, to + generally be either a singleton or a list of singletons. In practice, they're usually + strings, ints, floats, None, or a list of those. + spec_param - either None or a spec parameter dictionary as defined by + SpecManager.app_params. That is: + { + optional = boolean, + is_constant = boolean, + value = (whatever, optional), + type = [text|int|float|list|textsubdata], + is_output = boolean, + short_hint = string, + description = string, + allowed_values = list (optional), + } """ + if transform_type is not None: + transform_type = transform_type.lower() + + is_input_object_param = False + if ( + spec_param is not None + and spec_param["type"] == "text" + and not spec_param["is_output"] + and len(spec_param.get("allowed_types", [])) + ): + is_input_object_param = True + if ( transform_type is None and spec_param is not None @@ -746,60 +803,105 @@ def transform_param_value(transform_type, value, spec_param): ): transform_type = "string" - if ( - transform_type is None - or transform_type == "none" - or transform_type == "object-name" + if not is_input_object_param and ( + transform_type is None or transform_type == "none" ): return value - elif transform_type == "ref" or transform_type == "unresolved-ref": - # make unresolved workspace ref (like 'ws-name/obj-name') - if (value is not None) and ("/" not in value): - value = system_variable("workspace") + "/" + value - return value - - elif transform_type == "resolved-ref": - # make a workspace ref - if value is not None: - value = resolve_ref(system_variable("workspace"), value) - return value - - elif transform_type == "future-default": - # let's guess base on spec_param - if spec_param is None: - return value - else: - if value is not None: - value = resolve_ref_if_typed(value, spec_param) - return value + if transform_type in ["ref", "unresolved-ref", "resolved-ref", "upa"] or ( + is_input_object_param and transform_type is None + ): + if isinstance(value, list): + return [transform_object_value(transform_type, v) for v in value] + return transform_object_value(transform_type, value) - elif transform_type == "int": + if transform_type == "int": # make it an integer, OR 0. if value is None or len(str(value).strip()) == 0: return None return int(value) - elif transform_type == "string": + if transform_type == "string": if value is None: return value - elif isinstance(value, list): + if isinstance(value, list): return ",".join(value) - elif isinstance(value, dict): - return ",".join([key + "=" + str(value[key]) for key in value]) - else: - return str(value) + if isinstance(value, dict): + return ",".join([f"{key}={value[key]}" for key in value]) + return str(value) - elif transform_type.startswith("list<") and transform_type.endswith(">"): + if transform_type.startswith("list<") and transform_type.endswith(">"): # make it a list of transformed types. list_type = transform_type[5:-1] if isinstance(value, list): - ret = [] - for pos in range(0, len(value)): - ret.append(transform_param_value(list_type, value[pos], None)) - return ret - else: - return [transform_param_value(list_type, value, None)] + return [transform_param_value(list_type, v, None) for v in value] + return [transform_param_value(list_type, value, None)] else: raise ValueError("Unsupported Transformation type: " + transform_type) + + +def transform_object_value(transform_type: Optional[str], value: Optional[str]) -> str: + """ + Cases: + transform = ref or unresolved-ref: + - should return wsname / object name + transform = upa or resolved-ref: + - should return UPA + transform = None: + - should return object name + Note that if it is a reference path, it was always get returned as an UPA-path + for the best compatibility. + + value can be either object name, ref, upa, or ref-path + can tell by testing with UPA api + + If we can't find any object info on the value, just return the value as-is + """ + if value is None: + return None + + # 1. get object info + is_upa = upa.is_upa(value) + is_ref = upa.is_ref(value) + is_path = (is_upa or is_ref) and ";" in value + + # simple cases: + # 1. if is_upa and we want resolved-ref or upa, return the value + # 2. if is_ref and not is_upa and we want ref or unresolved-ref, return the value + # 3. if is neither upa or ref and transform is None, then we want the string, so return that + # 4. Otherwise, look up the object and return what's desired from there. + + if is_upa and transform_type in ["upa", "resolved-ref"]: + return value + if is_ref and not is_upa and transform_type in ["ref", "unresolved-ref"]: + return value + if not is_upa and not is_ref and transform_type is None: + return value + + + search_ref = value + if not is_upa and not is_ref: + search_ref = f"{system_variable('workspace')}/{value}" + try: + obj_info = clients.get("workspace").get_object_info3( + {"objects": [{"ref": search_ref}]} + ) + except Exception: + transform = transform_type + if transform is None: + transform = "object name" + raise ValueError( + f"Unable to find object reference '{search_ref}' to transform as {transform}" + ) + + if is_path: + return ";".join(obj_info["paths"][0]) + if transform_type in ["ref", "unresolved-ref"]: + obj = obj_info["infos"][0] + return f"{obj[7]}/{obj[1]}" + if transform_type in ["resolved-ref", "upa"]: + return ";".join(obj_info["paths"][0]) + if transform_type is None: + return obj_info["infos"][0][1] + return value diff --git a/src/biokbase/narrative/tests/data/ee2_job_test_data.json b/src/biokbase/narrative/tests/data/ee2_job_test_data.json index 94db803aeb..44d4b9223b 100644 --- a/src/biokbase/narrative/tests/data/ee2_job_test_data.json +++ b/src/biokbase/narrative/tests/data/ee2_job_test_data.json @@ -292,7 +292,7 @@ "widget_info": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/2/12", "report_name": "kb_sra_upload_report_af468a02-4278-40af-9536-0bdef1f050db", "report_ref": "62425/18/1", "report_window_line_height": "16", @@ -512,7 +512,7 @@ "widget_info": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/17/1", "report_name": "kb_sra_upload_report_4ffc6219-0260-4f1d-8b4d-5083e0595150", "report_ref": "62425/19/1", "report_window_line_height": "16", diff --git a/src/biokbase/narrative/tests/data/response_data.json b/src/biokbase/narrative/tests/data/response_data.json index d5a85ff521..ddd42028aa 100644 --- a/src/biokbase/narrative/tests/data/response_data.json +++ b/src/biokbase/narrative/tests/data/response_data.json @@ -401,7 +401,7 @@ "widget_info": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/2/12", "report_name": "kb_sra_upload_report_af468a02-4278-40af-9536-0bdef1f050db", "report_ref": "62425/18/1", "report_window_line_height": "16", @@ -414,7 +414,7 @@ "outputWidgetInfo": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/2/12", "report_name": "kb_sra_upload_report_af468a02-4278-40af-9536-0bdef1f050db", "report_ref": "62425/18/1", "report_window_line_height": "16", @@ -502,7 +502,7 @@ "widget_info": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/17/1", "report_name": "kb_sra_upload_report_4ffc6219-0260-4f1d-8b4d-5083e0595150", "report_ref": "62425/19/1", "report_window_line_height": "16", @@ -515,7 +515,7 @@ "outputWidgetInfo": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/17/1", "report_name": "kb_sra_upload_report_4ffc6219-0260-4f1d-8b4d-5083e0595150", "report_ref": "62425/19/1", "report_window_line_height": "16", @@ -773,7 +773,7 @@ "widget_info": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/2/12", "report_name": "kb_sra_upload_report_af468a02-4278-40af-9536-0bdef1f050db", "report_ref": "62425/18/1", "report_window_line_height": "16", @@ -786,7 +786,7 @@ "outputWidgetInfo": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/2/12", "report_name": "kb_sra_upload_report_af468a02-4278-40af-9536-0bdef1f050db", "report_ref": "62425/18/1", "report_window_line_height": "16", @@ -913,7 +913,7 @@ "widget_info": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/17/1", "report_name": "kb_sra_upload_report_4ffc6219-0260-4f1d-8b4d-5083e0595150", "report_ref": "62425/19/1", "report_window_line_height": "16", @@ -926,7 +926,7 @@ "outputWidgetInfo": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/17/1", "report_name": "kb_sra_upload_report_4ffc6219-0260-4f1d-8b4d-5083e0595150", "report_ref": "62425/19/1", "report_window_line_height": "16", @@ -1065,7 +1065,7 @@ "widget_info": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/17/1", "report_name": "kb_sra_upload_report_4ffc6219-0260-4f1d-8b4d-5083e0595150", "report_ref": "62425/19/1", "report_window_line_height": "16", @@ -1078,7 +1078,7 @@ "outputWidgetInfo": { "name": "no-display", "params": { - "obj_ref": "18836/5/1", + "obj_ref": "62425/17/1", "report_name": "kb_sra_upload_report_4ffc6219-0260-4f1d-8b4d-5083e0595150", "report_ref": "62425/19/1", "report_window_line_height": "16", diff --git a/src/biokbase/narrative/tests/narrative_mock/mockclients.py b/src/biokbase/narrative/tests/narrative_mock/mockclients.py index b7a2341438..e86e912af8 100644 --- a/src/biokbase/narrative/tests/narrative_mock/mockclients.py +++ b/src/biokbase/narrative/tests/narrative_mock/mockclients.py @@ -232,9 +232,19 @@ def get_object_info3(self, params): None, ] ] - paths = [["18836/5/1"]] + upa = "18836/5/1" num_objects = len(params.get("objects", [0])) - return {"infos": infos * num_objects, "paths": paths * num_objects} + paths = [] + for obj_ident in params.get("objects", []): + ref_path = [] + if "ref" in obj_ident and ";" in obj_ident["ref"]: + num_steps = len(obj_ident["ref"].split(";")) + for step in range(num_steps - 1): + i = step + 1 + ref_path.append(f"{i}/{i}/{i}") + ref_path.append(upa) + paths.append(ref_path) + return {"infos": infos * num_objects, "paths": paths} def list_objects(self, params): ws_ids = params["ids"] diff --git a/src/biokbase/narrative/tests/test_app_util.py b/src/biokbase/narrative/tests/test_app_util.py index d039741356..c53aa19ac7 100644 --- a/src/biokbase/narrative/tests/test_app_util.py +++ b/src/biokbase/narrative/tests/test_app_util.py @@ -3,17 +3,19 @@ """ import copy import os -import pytest from unittest import mock +import pytest + from biokbase.narrative.app_util import ( app_param, check_tag, get_result_sub_path, map_inputs_from_job, map_outputs_from_state, - transform_param_value + transform_param_value, ) +from biokbase.narrative.upa import is_upa from . import util from .narrative_mock.mockclients import get_mock_client @@ -21,7 +23,9 @@ user_token = "SOME_TOKEN" config = util.ConfigTests() user_id = config.get("users", "test_user") -user_token = util.read_token_file(config.get_path("token_files", "test_user", from_root=True)) +user_token = util.read_token_file( + config.get_path("token_files", "test_user", from_root=True) +) good_tag = "release" bad_tag = "not_a_tag" @@ -30,59 +34,48 @@ workspace = "valid_workspace" bad_variable = "not a variable" + def test_check_tag_good(): assert check_tag(good_tag) is True + def test_check_tag_bad(): assert check_tag(bad_tag) is False + def test_check_tag_bad_except(): with pytest.raises(ValueError): check_tag(bad_tag, raise_exception=True) + @pytest.fixture() def workspace_name(): def set_ws_name(ws_name): os.environ["KB_WORKSPACE_ID"] = ws_name + yield set_ws_name del os.environ["KB_WORKSPACE_ID"] + get_result_sub_path_cases = [ ( [{"report": "this_is_a_report", "report_ref": "123/456/7"}], [0, "report_ref"], - "123/456/7" - ), - ( - ["foo", "bar", "baz"], - [2], - "baz" - ), - ( - ["foo", {"bar": "baz"}, "foobar"], - [1, "bar"], - "baz" + "123/456/7", ), - ( - ["foo", 0, {"bar": {"baz": [10, 11, 12, 13]}}], - [2, "bar", "baz", 3], - 13 - ), - ( - ["foo"], - [2], - None - ), - ( - {"foo": "bar"}, - ["baz"], - None - ) + (["foo", "bar", "baz"], [2], "baz"), + (["foo", {"bar": "baz"}, "foobar"], [1, "bar"], "baz"), + (["foo", 0, {"bar": {"baz": [10, 11, 12, 13]}}], [2, "bar", "baz", 3], 13), + (["foo"], [2], None), + ({"foo": "bar"}, ["baz"], None), ] + + @pytest.mark.parametrize("result,path,expected", get_result_sub_path_cases) def test_get_result_sub_path(result, path, expected): assert get_result_sub_path(result, path) == expected + def test_map_inputs_from_job(): inputs = [ "input1", @@ -138,13 +131,12 @@ def test_map_outputs_from_state_simple(workspace_name): workspace_name(workspace) app_spec = { "parameters": [], - "behavior": { - "output_mapping": [{"narrative_system_variable": "workspace"}] - }, + "behavior": {"output_mapping": [{"narrative_system_variable": "workspace"}]}, } expected = ("kbaseDefaultNarrativeOutput", workspace) assert map_outputs_from_state(None, None, app_spec) == expected + def test_map_outputs_from_state(workspace_name): workspace_name(workspace) app_spec = { @@ -175,6 +167,7 @@ def test_map_outputs_from_state(workspace_name): ) assert map_outputs_from_state(state, params, app_spec) == expected + def test_map_outputs_from_state_bad_spec(workspace_name): workspace_name(workspace) app_spec = {"not": "really"} @@ -184,17 +177,16 @@ def test_map_outputs_from_state_bad_spec(workspace_name): map_outputs_from_state(state, params, app_spec) - # app_param tests base_app_param = { - 'id': 'my_param', - 'ui_name': 'My Param', - 'short_hint': 'Short Info', - 'description': 'Longer description', - 'allow_multiple': 0, - 'optional': 0, - 'ui_class': 'input', - 'default_values': [''], + "id": "my_param", + "ui_name": "My Param", + "short_hint": "Short Info", + "description": "Longer description", + "allow_multiple": 0, + "optional": 0, + "ui_class": "input", + "default_values": [""], } base_expect = { "id": base_app_param["id"], @@ -206,136 +198,80 @@ def test_map_outputs_from_state_bad_spec(workspace_name): "allow_multiple": False, "default": None, } -app_param_cases = [( - "text", {}, {} -), ( - "text", - { - "text_options": { - "is_output_name": 1 - } - }, - { - "is_output": True - } -), ( - "text", - { - "optional": 1, - }, - { - "optional": True - } -), ( - "text", - { - "allow_multiple": 1, - "default_values": ["foo", "", "bar"] - }, - { - "allow_multiple": True, - "default": ["foo", "bar"] - } -), ( - "text", - { - "text_options": { - "validate_as": "int", - "min_int": -1, - "max_int": 1000 - } - }, - { - "type": "int", - "min_val": -1, - "max_val": 1000 - } -), ( - "text", - { - "text_options": { - "validate_as": "float", - "min_float": -1.2, - "max_float": 1000.4 - } - }, - { - "type": "float", - "min_val": -1.2, - "max_val": 1000.4 - } -), ( - "dropdown", - { - "dropdown_options": { - "options": [ - {"display": "a", "value": "b"}, - {"display": "c", "value": "d"} - ] - } - }, - { - "allowed_values": ["b", "d"] - } -), ( - "checkbox", - { - "checkbox_options": { - "checked_value": 1, - "unchecked_value": 0 - } - }, - { - "checkbox_map": [1, 0], - "allowed_values": [True, False] - } -), ( - "checkbox", - {}, - { - "allowed_values": [True, False] - } -), ( - "checkbox", - { - "checkbox_options": { - "checked_value": 2, - "unchecked_value": 1, - "kinda_checked_value": 0 - } - }, - { - "allowed_values": [True, False] - } -), ( - "text", - { - "text_options": { - "valid_ws_types": [] - } - }, - {} -), ( - "text", - { - "text_options": { - "valid_ws_types": ["KBaseGenomes.Genome", "FooBar.Baz"] - } - }, - { - "allowed_types": ["KBaseGenomes.Genome", "FooBar.Baz"] - } -), ( - "text", - { - "text_options": { - "regex_constraint": "/\\d+/" - } - }, - { - "regex_constraint": "/\\d+/" - } -)] +app_param_cases = [ + ("text", {}, {}), + ("text", {"text_options": {"is_output_name": 1}}, {"is_output": True}), + ( + "text", + { + "optional": 1, + }, + {"optional": True}, + ), + ( + "text", + {"allow_multiple": 1, "default_values": ["foo", "", "bar"]}, + {"allow_multiple": True, "default": ["foo", "bar"]}, + ), + ( + "text", + {"text_options": {"validate_as": "int", "min_int": -1, "max_int": 1000}}, + {"type": "int", "min_val": -1, "max_val": 1000}, + ), + ( + "text", + { + "text_options": { + "validate_as": "float", + "min_float": -1.2, + "max_float": 1000.4, + } + }, + {"type": "float", "min_val": -1.2, "max_val": 1000.4}, + ), + ( + "dropdown", + { + "dropdown_options": { + "options": [ + {"display": "a", "value": "b"}, + {"display": "c", "value": "d"}, + ] + } + }, + {"allowed_values": ["b", "d"]}, + ), + ( + "checkbox", + {"checkbox_options": {"checked_value": 1, "unchecked_value": 0}}, + {"checkbox_map": [1, 0], "allowed_values": [True, False]}, + ), + ("checkbox", {}, {"allowed_values": [True, False]}), + ( + "checkbox", + { + "checkbox_options": { + "checked_value": 2, + "unchecked_value": 1, + "kinda_checked_value": 0, + } + }, + {"allowed_values": [True, False]}, + ), + ("text", {"text_options": {"valid_ws_types": []}}, {}), + ( + "text", + {"text_options": {"valid_ws_types": ["KBaseGenomes.Genome", "FooBar.Baz"]}}, + {"allowed_types": ["KBaseGenomes.Genome", "FooBar.Baz"]}, + ), + ( + "text", + {"text_options": {"regex_constraint": "/\\d+/"}}, + {"regex_constraint": "/\\d+/"}, + ), +] + + @pytest.mark.parametrize("field_type,spec_add,expect_add", app_param_cases) def test_app_param(field_type, spec_add, expect_add): spec_param = copy.deepcopy(base_app_param) @@ -350,64 +286,154 @@ def test_app_param(field_type, spec_add, expect_add): # transform_param_value tests transform_param_value_simple_cases = [ - ( "string", None, None, None ), - ( "string", "foo", None, "foo" ), - ( "string", 123, None, "123" ), - ( "string", ["a", "b", "c"], None, "a,b,c"), - ( "string", {"a": "b", "c": "d"}, None, "a=b,c=d"), - ( "string", [], None, "" ), - ( "string", {}, None, "" ), - ( "int", "1", None, 1 ), - ( "int", None, None, None ), - ( "int", "", None, None ), - ( "list", [1, 2, 3], None, ["1", "2", "3"] ), - ( "list", ["1", "2", "3"], None, [1, 2, 3] ), - ( "list", "asdf", None, ["asdf"] ), - ( "list", "1", None, [1] ) + ("string", None, None, None), + ("string", "foo", None, "foo"), + ("string", 123, None, "123"), + ("string", ["a", "b", "c"], None, "a,b,c"), + ("string", {"a": "b", "c": "d"}, None, "a=b,c=d"), + ("string", [], None, ""), + ("string", {}, None, ""), + ("int", "1", None, 1), + ("int", None, None, None), + ("int", "", None, None), + ("list", [1, 2, 3], None, ["1", "2", "3"]), + ("list", ["1", "2", "3"], None, [1, 2, 3]), + ("list", "asdf", None, ["asdf"]), + ("list", "1", None, [1]), ] -@pytest.mark.parametrize("transform_type,value,spec_param,expected", transform_param_value_simple_cases) + + +@pytest.mark.parametrize( + "transform_type,value,spec_param,expected", transform_param_value_simple_cases +) def test_transform_param_value_simple(transform_type, value, spec_param, expected): assert transform_param_value(transform_type, value, spec_param) == expected + def test_transform_param_value_fail(): ttype = "foobar" with pytest.raises(ValueError, match=f"Unsupported Transformation type: {ttype}"): transform_param_value(ttype, "foo", None) + textsubdata_cases = [ (None, None), ("asdf", "asdf"), (123, "123"), (["1", "2", "3"], "1,2,3"), - ({"a":"b", "c": "d"}, "a=b,c=d") + ({"a": "b", "c": "d"}, "a=b,c=d"), ] + + @pytest.mark.parametrize("value,expected", textsubdata_cases) def test_transform_param_value_textsubdata(value, expected): - spec = { - "type": "textsubdata" - } + spec = {"type": "textsubdata"} assert transform_param_value(None, value, spec) == expected + +mock_obj_ref = "wjriehl:1490995018528/Sbicolor2" ref_cases = [ (None, None), ("foo/bar", "foo/bar"), - ("1/2/3", "1/2/3"), - ("name", workspace + "/name") + ("1/2/3", mock_obj_ref), + ("Sbicolor2", mock_obj_ref), ] + + +@mock.patch("biokbase.narrative.app_util.clients.get", get_mock_client) @pytest.mark.parametrize("value,expected", ref_cases) def test_transform_param_value_simple_ref(value, expected, workspace_name): workspace_name(workspace) for tf_type in ["ref", "unresolved-ref"]: - assert transform_param_value(tf_type, value, None) == expected + tf_value = transform_param_value(tf_type, value, None) + assert tf_value == expected + mock_upa = "18836/5/1" upa_cases = [ ("some_name", mock_upa), (["multiple", "names"], [mock_upa, mock_upa]), - ("already/ref", mock_upa) + ("already/ref", mock_upa), ] + + @mock.patch("biokbase.narrative.app_util.clients.get", get_mock_client) @pytest.mark.parametrize("value,expected", upa_cases) def test_transform_param_value_resolved_ref(value, expected, workspace_name): workspace_name(workspace) assert transform_param_value("resolved-ref", value, None) == expected + + +class RefChainWorkspace: + def __init__(self): + pass + + def get_object_info3(self, params): + """ + Makes quite a few assumptions about input, as it's used for a specific test. + 1. params = {"objects": [{"ref": ref}, {"ref": ref}, ...]} + 2. ref is a ;-separated chain of either wsname/objid or upas, not a mix. + 3. we don't really care about names or object info here in responses + + # TODO something like this should go into the MockClient workspace call + """ + + def make_obj_info(wsid, objid, ver): + return [ + objid, + "objname", + "Object.Type-1.0", + "date", + ver, + "user_name", + wsid, + "SomeWorkspace", + "checksum", + 12345, + None, + ] + + obj_infos = [] + obj_paths = [] + for obj_ident in params["objects"]: + ref_chain = obj_ident["ref"].split(";") + obj_path = [] + cur_ws = 1 + cur_obj = 1 + cur_ver = 1 + for ref in ref_chain: + if is_upa(ref): + obj_path.append(ref) + else: + obj_path.append(f"{cur_ws}/{cur_obj}/{cur_ver}") + cur_ws += 1 + cur_obj += 1 + cur_ver += 1 + if is_upa(obj_ident["ref"]): + obj_info = make_obj_info(*(ref_chain[-1].split("/"))) + else: + obj_info = make_obj_info(cur_ws, cur_obj, cur_ver) + obj_infos.append(obj_info) + obj_paths.append(obj_path) + return {"infos": obj_infos, "paths": obj_paths} + + +def get_ref_path_mock_ws(name="workspace"): + return RefChainWorkspace() + + +@mock.patch("biokbase.narrative.app_util.clients.get", get_ref_path_mock_ws) +def test_transform_param_value_upa_path(): + upa_path = "123/456/789;234/345/456" + for tf_type in ["ref", "unresolved-ref", "upa", "resolved-ref"]: + assert transform_param_value(tf_type, upa_path, None) == upa_path + + +@mock.patch("biokbase.narrative.app_util.clients.get", get_ref_path_mock_ws) +def test_transform_param_value_ref_path(): + ref_path = "some_ws/some_obj;some_other_ws/some_other_obj" + for tf_type in ["ref", "unresolved-ref"]: + assert transform_param_value(tf_type, ref_path, None) == ref_path + expected_upa = "1/1/1;2/2/2" + for tf_type in ["upa", "resolved-ref"]: + assert transform_param_value(tf_type, ref_path, None) == expected_upa diff --git a/src/biokbase/narrative/tests/test_appmanager.py b/src/biokbase/narrative/tests/test_appmanager.py index b07a8f2517..5b83c232b5 100644 --- a/src/biokbase/narrative/tests/test_appmanager.py +++ b/src/biokbase/narrative/tests/test_appmanager.py @@ -1072,8 +1072,9 @@ def test_input_mapping(self): ref_path = ( ws_name + "/MyReadsSet; " + ws_name + "/rhodobacterium.art.q10.PE.reads" ) + # ref_paths get mocked as 1/1/1;2/2/2;...N/N/N;18836/5/1 ret = app_util.transform_param_value("resolved-ref", ref_path, None) - self.assertEqual(ret, ws_name + "/MyReadsSet;18836/5/1") + self.assertEqual(ret, "1/1/1;18836/5/1") @mock.patch(CLIENTS_AM_SM, get_mock_client) def test_generate_input(self): @@ -1096,19 +1097,19 @@ def test_transform_input_good(self): ws_name = self.public_ws test_data = [ { - "value": "input_value", + "value": READS_OBJ_1, "type": "ref", - "expected": ws_name + "/" + "input_value", + "expected": ws_name + "/" + READS_OBJ_1, }, { - "value": ws_name + "/input_value", + "value": ws_name + "/" + READS_OBJ_1, "type": "ref", - "expected": ws_name + "/" + "input_value", + "expected": ws_name + "/" + READS_OBJ_1, }, { - "value": "input_value", + "value": READS_OBJ_1, "type": "unresolved-ref", - "expected": ws_name + "/" + "input_value", + "expected": ws_name + "/" + READS_OBJ_1, }, { "value": READS_OBJ_1, @@ -1123,9 +1124,9 @@ def test_transform_input_good(self): {"value": None, "type": "int", "expected": None}, {"value": "5", "type": "int", "expected": 5}, { - "value": ["a", "b", "c"], + "value": [READS_OBJ_1, READS_OBJ_2], "type": "list", - "expected": [ws_name + "/a", ws_name + "/b", ws_name + "/c"], + "expected": [ws_name + "/" + READS_OBJ_1, ws_name + "/" + READS_OBJ_2], }, { "value": [ @@ -1135,15 +1136,13 @@ def test_transform_input_good(self): "type": "list", "expected": ["11635/9/1", "11635/10/1"], }, - {"value": "foo", "type": "list", "expected": [ws_name + "/foo"]}, - {"value": ["1", "2", 3], "type": "list", "expected": [1, 2, 3]}, - {"value": "bar", "type": None, "expected": "bar"}, { "value": READS_OBJ_1, - "type": "future-default", - "spec": {"is_output": 0, "allowed_types": ["Some.KnownType"]}, - "expected": "11635/9/1", + "type": "list", + "expected": [ws_name + "/" + READS_OBJ_1], }, + {"value": ["1", "2", 3], "type": "list", "expected": [1, 2, 3]}, + {"value": "bar", "type": None, "expected": "bar"}, {"value": [123, 456], "type": None, "expected": [123, 456]}, {"value": 123, "type": "string", "expected": "123"}, {