From cb8b0a5c25ed4c084c7138e1298599632acc02ba Mon Sep 17 00:00:00 2001 From: Douglas Rioux Date: Tue, 17 Jan 2023 12:03:54 -0500 Subject: [PATCH 1/7] Add pipelines view + tests --- src/encoded/tests/test_types_base.py | 17 +++++++++++++ src/encoded/types/base.py | 38 ++++++++++++++++++---------- src/encoded/types/sample.py | 1 + src/encoded/types/utils.py | 8 ++++++ 4 files changed, 51 insertions(+), 13 deletions(-) create mode 100644 src/encoded/tests/test_types_base.py create mode 100644 src/encoded/types/utils.py diff --git a/src/encoded/tests/test_types_base.py b/src/encoded/tests/test_types_base.py new file mode 100644 index 0000000000..605e63d62e --- /dev/null +++ b/src/encoded/tests/test_types_base.py @@ -0,0 +1,17 @@ +class TestPipelineDisplay: + + PIPELINE_DISPLAY_VIEW = "@@pipelines" + + def assert_pipeline_display_status(self, testapp, item_to_get, status): + item_pipeline_url = self.get_item_pipeline_display_url(item_to_get) + testapp.get(item_pipeline_url, status=status) + + def get_item_pipeline_display_url(self, item_to_get): + item_atid = item_to_get["@id"] + return item_atid + self.PIPELINE_DISPLAY_VIEW + + def test_pipeline_display_success(self, testapp, sample_proc_fam): + self.assert_pipeline_display_status(testapp, sample_proc_fam, 200) + + def test_pipeline_display_failure(self, testapp, project): + self.assert_pipeline_display_status(testapp, project, 405) diff --git a/src/encoded/types/base.py b/src/encoded/types/base.py index ee7c79085a..d894d2664c 100644 --- a/src/encoded/types/base.py +++ b/src/encoded/types/base.py @@ -2,23 +2,17 @@ import re import snovault import string +from requests import Request +from typing import Any, List, Tuple, Union -# from datetime import date -# from functools import lru_cache -# from jsonschema_serialize_fork import NO_DEFAULT +from pyramid.httpexceptions import HTTPMethodNotAllowed from pyramid.security import ( - # ALL_PERMISSIONS, Allow, Authenticated, Deny, - # DENY_ALL, Everyone, ) -# from pyramid.traversal import find_root -from pyramid.view import ( - view_config, -) -# from pyramid.httpexceptions import HTTPUnprocessableEntity +from pyramid.view import view_config from snovault.util import debug_log # import snovault default post / patch stuff so we can overwrite it in this file from snovault.validators import ( @@ -28,7 +22,7 @@ validate_item_content_in_place, no_validate_item_content_post, no_validate_item_content_put, - no_validate_item_content_patch + no_validate_item_content_patch, ) # We will extend the following functions with CGAP-specific actions from snovault.crud_views import ( @@ -36,8 +30,8 @@ item_edit as sno_item_edit, ) from snovault.interfaces import CONNECTION -from typing import Any, List, Tuple, Union -# from ..schema_formats import is_accession + +from .utils import display_pipelines from ..server_defaults import get_userid, add_last_modified @@ -498,3 +492,21 @@ def item_edit(context, request, render=None): # This works # Probably don't need to extend re: institution + project since if editing, assuming these have previously existed. return sno_item_edit(context, request, render) + + +def validate_item_pipelines_get(context: Item, request: Request) -> None: + display_pipelines = getattr(context, "display_pipelines", False) + if not display_pipelines: + raise HTTPMethodNotAllowed(detail="Item cannot display pipelines") + + +@view_config( + context=Item, + permission="view", + name="pipelines", + request_method="GET", + validators=[validate_item_pipelines_get], +) +@debug_log +def pipelines(context, request): + return display_pipelines(context, request) diff --git a/src/encoded/types/sample.py b/src/encoded/types/sample.py index 4420080b1a..d5d06d6b7b 100644 --- a/src/encoded/types/sample.py +++ b/src/encoded/types/sample.py @@ -1094,6 +1094,7 @@ class SampleProcessing(Item): schema = load_schema("encoded:schemas/sample_processing.json") embedded_list = _build_sample_processing_embedded_list() rev = {"case": ("Case", "sample_processing")} + display_pipelines = True QC_VALUE_SCHEMA = { "title": "Value", diff --git a/src/encoded/types/utils.py b/src/encoded/types/utils.py new file mode 100644 index 0000000000..aa70ddb61f --- /dev/null +++ b/src/encoded/types/utils.py @@ -0,0 +1,8 @@ +from typing import Mapping + +from pyramid.request import Request + + +def display_pipelines(context: Mapping, request: Request): + import pdb; pdb.set_trace() + return {} From b225d01187095e7629d6246eb5ebee1ddb039c7a Mon Sep 17 00:00:00 2001 From: Douglas Rioux Date: Mon, 23 Jan 2023 13:16:38 -0500 Subject: [PATCH 2/7] Create basic API infrastructure Also, refactor custom_embed some to create constants to share across modules. --- src/encoded/custom_embed.py | 15 +++-- src/encoded/tests/datafixtures.py | 49 +++++++++++++- src/encoded/tests/test_types_base.py | 99 ++++++++++++++++++++++++++-- src/encoded/types/base.py | 88 +++++++++++++++++++++++-- src/encoded/types/sample.py | 2 +- src/encoded/types/utils.py | 8 --- 6 files changed, 233 insertions(+), 28 deletions(-) delete mode 100644 src/encoded/types/utils.py diff --git a/src/encoded/custom_embed.py b/src/encoded/custom_embed.py index 7687405996..bbdaf92ae6 100644 --- a/src/encoded/custom_embed.py +++ b/src/encoded/custom_embed.py @@ -8,7 +8,10 @@ from pyramid.view import view_config from snovault.util import debug_log + ATID_PATTERN = re.compile("/[a-zA-Z-]+/[a-zA-Z0-9-_:]+/") +EMBED_ALL_FIELDS_MARKER = "*" +PROPERTY_SPLITTER = "." GENELIST_ATID = re.compile("/gene-lists/[a-zA-Z0-9-]+/") MINIMAL_EMBEDS = ["projects", "institutions", "users"] MINIMAL_EMBED_ATID = re.compile("/(" + "|".join(MINIMAL_EMBEDS) + ")/[a-zA-Z0-9-_:]+/") @@ -26,6 +29,7 @@ ] FORBIDDEN_MSG = {"error": "no view permissions"} DATABASE_ITEM_KEY = "@type" # Key specific to JSON objects that are CGAP items +REQUESTED_FIELDS = "requested_fields" def includeme(config): @@ -43,7 +47,7 @@ def __init__(self, request, item, embed_props): self.ignored_embeds = embed_props.get("ignored_embeds", []) self.desired_embeds = embed_props.get("desired_embeds", []) self.embed_depth = embed_props.get("embed_depth", 4) - self.requested_fields = embed_props.get("requested_fields", []) + self.requested_fields = embed_props.get(REQUESTED_FIELDS, []) self.cache = {} self.invalid_ids = [] @@ -55,6 +59,9 @@ def __init__(self, request, item, embed_props): depth = -1 self.result = self.embed(item, depth) + def get_embedded_fields(self) -> dict: + return self.result + def add_actions(self, item): """ Add the "actions" field to an item according to the request's @@ -241,7 +248,7 @@ def fields_to_nested_dict(self): """ field_dict = {} for field in self.requested_fields: - field_keys = field.split(".") + field_keys = field.split(PROPERTY_SPLITTER) field_keys = [x for x in field_keys if x] field_dict = self.build_nested_dict(field_dict, field_keys) return field_dict @@ -306,7 +313,7 @@ def field_embed(self, item, field_dict, initial_item=False): "The 'actions' field was requested for a JSON object" " that is not a database item." ) - if "*" not in fields_to_keep: + if EMBED_ALL_FIELDS_MARKER not in fields_to_keep: culled_item = {} for field in fields_to_keep: try: @@ -380,7 +387,7 @@ def embed(context, request): "ignored_embeds": ignored_embeds, "desired_embeds": desired_embeds, "embed_depth": embed_depth, - "requested_fields": requested_fields, + REQUESTED_FIELDS: requested_fields, } for item_id in ids: item_embed = CustomEmbed(request, item_id, embed_props) diff --git a/src/encoded/tests/datafixtures.py b/src/encoded/tests/datafixtures.py index baf822d47d..f763dd04f8 100644 --- a/src/encoded/tests/datafixtures.py +++ b/src/encoded/tests/datafixtures.py @@ -351,6 +351,7 @@ def mother_bam_file(testapp, project, institution, file_formats, mother_bam_qc): } return testapp.post_json('/file_processed', item).json['@graph'][0] + @pytest.fixture def mother_sample(testapp, project, institution, mother_bam_file): item = { @@ -708,7 +709,7 @@ def fam(testapp, project, female_individual, institution, grandpa, mother, fathe @pytest.fixture -def sample_proc_fam(testapp, project, institution, fam): +def sample_proc_fam(testapp, project, institution, fam, meta_workflow_run): data = { 'project': project['@id'], 'institution': institution['@id'], @@ -724,7 +725,8 @@ def sample_proc_fam(testapp, project, institution, fam): "GAPSAUNCLE01", "GAPSACOUSIN1" ], - 'families': [fam['@id']] + 'families': [fam['@id']], + "meta_workflow_runs": [meta_workflow_run["@id"]], } res = testapp.post_json('/sample_processing', data).json['@graph'][0] return res @@ -1062,6 +1064,49 @@ def workflow_run_awsem_json(testapp, institution, project, workflow_bam): } +@pytest.fixture +def meta_workflow(testapp, project, institution, workflow_bam): + item = { + "project": project["@id"], + "institution": institution["@id"], + "title": "A beautiful pipeline", + "name": "Beautiful Pipeline", + "workflows": [ + { + "name": "BAM processing", + "workflow": workflow_bam["@id"], + }, + ], + } + return testapp.post_json("/meta_workflow", item, status=201).json["@graph"][0] + + +@pytest.fixture +def meta_workflow_run( + testapp, project, institution, meta_workflow, workflow_run_awsem, mother_bam_file +): + item = { + "project": project["@id"], + "institution": institution["@id"], + "final_status": "completed", + "title": "A beautiful pipeline run", + "meta_workflow": meta_workflow["@id"], + "workflow_runs": [ + { + "name": "BAM processing", + "status": "completed", + "workflow_run": workflow_run_awsem["@id"], + "output": [ + { + "file": mother_bam_file["@id"], + }, + ], + }, + ], + } + return testapp.post_json("/meta_workflow_run", item, status=201).json["@graph"][0] + + @pytest.fixture def software_bam(testapp, institution, project): # TODO: ASK_ANDY do we want software_type to be an array? diff --git a/src/encoded/tests/test_types_base.py b/src/encoded/tests/test_types_base.py index 605e63d62e..732680e353 100644 --- a/src/encoded/tests/test_types_base.py +++ b/src/encoded/tests/test_types_base.py @@ -1,17 +1,102 @@ -class TestPipelineDisplay: +from requests import Request +from typing import Any, List, Mapping, Optional, Sequence +from unittest import mock - PIPELINE_DISPLAY_VIEW = "@@pipelines" +import pytest +from webtest import TestApp - def assert_pipeline_display_status(self, testapp, item_to_get, status): +from ..types.base import Item, PipelineRetriever + + +class Mocks: + + @staticmethod + def mock_item(attributes: Optional[Mapping[str, Any]] = None) -> mock.NonCallableMagicMock: + item = mock.create_autospec(Item, instance=True) + if attributes: + for key, value in attributes.items(): + setattr(item, key, value) + return item + + @staticmethod + def mock_request() -> mock.NonCallableMagicMock: + return mock.create_autospec(Request, instance=True) + + +class TestPipelineView: + + PIPELINE_VIEW_ADDON = "@@pipelines" + + def assert_pipeline_display_status( + self, testapp: TestApp, item_to_get: Mapping, status: int + ) -> None: item_pipeline_url = self.get_item_pipeline_display_url(item_to_get) testapp.get(item_pipeline_url, status=status) - def get_item_pipeline_display_url(self, item_to_get): + def get_item_pipeline_display_url(self, item_to_get: Mapping) -> str: item_atid = item_to_get["@id"] - return item_atid + self.PIPELINE_DISPLAY_VIEW + return item_atid + self.PIPELINE_VIEW_ADDON - def test_pipeline_display_success(self, testapp, sample_proc_fam): + def test_pipeline_display_success( + self, testapp: TestApp, sample_proc_fam: Mapping + ) -> None: self.assert_pipeline_display_status(testapp, sample_proc_fam, 200) - def test_pipeline_display_failure(self, testapp, project): + def test_pipeline_display_failure(self, testapp: TestApp, project: Mapping) -> None: self.assert_pipeline_display_status(testapp, project, 405) + + +# @pytest.mark.parametrize( +# "item_attributes,expected", +# [ +# ({}, []), +# ({"foo": "bar"}, []), +# ({"pipeline_properties": ["foo", "bar"]}, ["foo", "bar"]), +# ] +# ) +# def test_get_pipeline_properties( +# item_attributes: Mapping[str, Any], expected: Sequence[str] +# ) -> None: +# item = Mocks.mock_item(item_attributes) +# assert get_pipeline_properties(item) == expected + + +class TestPipelineRetriever: + + REQUEST = Mocks.mock_request() + CONTEXT = Mocks.mock_item() + PIPELINE_PROPERTIES = ["foo", "bar.buz"] + + def pipeline_retriever(self) -> PipelineRetriever: + return PipelineRetriever(self.CONTEXT, self.REQUEST) + + + @pytest.mark.parametrize( + "pipeline_properties,expected", + [ + ([], []), + (["foo"], ["foo.*"]), + (["foo", "bar.buz"], ["foo.*", "bar.*", "bar.buz.*"]), + ] + ) + def test_get_properties_to_embed( + self, pipeline_properties: Sequence[str], expected: Sequence[str] + ) -> None: + retriever = self.pipeline_retriever() + assert retriever.get_properties_to_embed() == expected + + @pytest.mark.parametrize( + "pipeline_property,expected", + [ + ("", []), + ("foo", ["foo.*"]), + ("foo.bar", ["foo.*", "foo.bar.*"]), + ] + ) + def test_get_properties_to_embed_from_pipeline_property( + self, pipeline_property: str, expected: List[str] + ) -> None: + pipeline_retriever = self.pipeline_retriever() + assert pipeline_retriever.get_properties_to_embed_from_pipeline_property( + pipeline_property + ) == expected diff --git a/src/encoded/types/base.py b/src/encoded/types/base.py index d894d2664c..dcf008c725 100644 --- a/src/encoded/types/base.py +++ b/src/encoded/types/base.py @@ -2,8 +2,9 @@ import re import snovault import string +from dataclasses import dataclass from requests import Request -from typing import Any, List, Tuple, Union +from typing import Any, Iterable, List, Optional, Sequence, Tuple, Union from pyramid.httpexceptions import HTTPMethodNotAllowed from pyramid.security import ( @@ -31,7 +32,7 @@ ) from snovault.interfaces import CONNECTION -from .utils import display_pipelines +from .. import custom_embed from ..server_defaults import get_userid, add_last_modified @@ -495,8 +496,8 @@ def item_edit(context, request, render=None): def validate_item_pipelines_get(context: Item, request: Request) -> None: - display_pipelines = getattr(context, "display_pipelines", False) - if not display_pipelines: + pipeline_properties = getattr(context, "pipeline_properties", []) + if not pipeline_properties: raise HTTPMethodNotAllowed(detail="Item cannot display pipelines") @@ -508,5 +509,80 @@ def validate_item_pipelines_get(context: Item, request: Request) -> None: validators=[validate_item_pipelines_get], ) @debug_log -def pipelines(context, request): - return display_pipelines(context, request) +def pipelines(context: Item, request: Request) -> dict: + pipeline_retriever = PipelineRetriever(context, request) + pipelines = pipeline_retriever.get_pipelines() + return PipelineDisplayer(pipelines).get_display() + + +@dataclass(frozen=True) +class PipelineRetriever: + + PIPELINE_PROPERTIES = "pipeline_properties" + UUID = "uuid" + + context: Item + request: Request + + def get_pipeline_properties(self) -> List[str]: + return getattr(self.context, self.PIPELINE_PROPERTIES, []) + + def get_pipelines(self) -> List[dict]: + item_with_embeds = self.get_item_with_embeds() + return self.get_pipelines_from_embeds(item_with_embeds) + + def get_item_with_embeds(self) -> List[dict]: + item_identifier = self.get_item_identifier() + custom_embed_parameters = self.get_custom_embed_parameters() + return custom_embed.CustomEmbed( + self.request, item_identifier, custom_embed_parameters + ).get_embedded_fields() + + def get_item_identifier(self) -> str: + return str(getattr(self.context, self.UUID, "")) + + def get_custom_embed_parameters(self) -> dict: + return {custom_embed.REQUESTED_FIELDS: self.get_properties_to_embed()} + + def get_properties_to_embed(self) -> List[str]: + result = [] + for pipeline_property in self.get_pipeline_properties(): + result.extend( + self.get_properties_to_embed_from_pipeline_property(pipeline_property) + ) + return result + + def get_properties_to_embed_from_pipeline_property( + self, + pipeline_property: str + ) -> List[str]: + split_properties = [ + term for term in pipeline_property.split(custom_embed.PROPERTY_SPLITTER) + if term + ] + return [ + self.make_embed_property( + custom_embed.PROPERTY_SPLITTER.join(split_properties[:idx + 1]) + ) + for idx in range(len(split_properties)) + ] + + @staticmethod + def make_embed_property(property_to_embed: str) -> str: + return ( + property_to_embed + + custom_embed.PROPERTY_SPLITTER + + custom_embed.EMBED_ALL_FIELDS_MARKER + ) + + def get_pipelines_from_item(self, item: dict) -> List[dict]: + pass + + +@dataclass(frozen=True) +class PipelineDisplayer: + + pipelines: Sequence[dict] + + def get_display(self) -> dict: + pass diff --git a/src/encoded/types/sample.py b/src/encoded/types/sample.py index d5d06d6b7b..f15c5b5071 100644 --- a/src/encoded/types/sample.py +++ b/src/encoded/types/sample.py @@ -1094,7 +1094,7 @@ class SampleProcessing(Item): schema = load_schema("encoded:schemas/sample_processing.json") embedded_list = _build_sample_processing_embedded_list() rev = {"case": ("Case", "sample_processing")} - display_pipelines = True + pipeline_properties = ["meta_workflow_runs", "samples.meta_workflow_runs"] QC_VALUE_SCHEMA = { "title": "Value", diff --git a/src/encoded/types/utils.py b/src/encoded/types/utils.py deleted file mode 100644 index aa70ddb61f..0000000000 --- a/src/encoded/types/utils.py +++ /dev/null @@ -1,8 +0,0 @@ -from typing import Mapping - -from pyramid.request import Request - - -def display_pipelines(context: Mapping, request: Request): - import pdb; pdb.set_trace() - return {} From aff11fcc99719340cb1a54051cb1de613c3ac8b8 Mon Sep 17 00:00:00 2001 From: Douglas Rioux Date: Sat, 4 Feb 2023 12:32:58 -0500 Subject: [PATCH 3/7] Add recursive property getter Increase test coverage and add display classes as well. --- src/encoded/tests/test_types_base.py | 482 +++++++++++++++++++++++++-- src/encoded/types/base.py | 193 ++++++++++- 2 files changed, 627 insertions(+), 48 deletions(-) diff --git a/src/encoded/tests/test_types_base.py b/src/encoded/tests/test_types_base.py index 732680e353..a5f0a9213c 100644 --- a/src/encoded/tests/test_types_base.py +++ b/src/encoded/tests/test_types_base.py @@ -1,26 +1,128 @@ +from contextlib import contextmanager from requests import Request -from typing import Any, List, Mapping, Optional, Sequence +from typing import Any, List, Mapping, Optional, Sequence, Union from unittest import mock +from uuid import uuid4 import pytest from webtest import TestApp -from ..types.base import Item, PipelineRetriever +from ..types import base as base_module +from ..types.base import Item, PipelineRetriever, PipelineToDisplay, RecursivePipelineRetriever class Mocks: @staticmethod - def mock_item(attributes: Optional[Mapping[str, Any]] = None) -> mock.NonCallableMagicMock: - item = mock.create_autospec(Item, instance=True) + def autospec_instance_with_attributes( + item_to_mock: object, attributes: Optional[Mapping[str, Any]] = None + ) -> mock.NonCallableMagicMock: + mocked_item = mock.create_autospec(item_to_mock, instance=True) if attributes: - for key, value in attributes.items(): - setattr(item, key, value) - return item + for attribute, value in attributes.items(): + setattr(mocked_item, attribute, value) + return mocked_item + + @staticmethod + def mock_context(attributes: Optional[Mapping[str, Any]] = None) -> mock.NonCallableMagicMock: + return Mocks.autospec_instance_with_attributes(Item, attributes) + + @staticmethod + def mock_request(attributes: Optional[Mapping[str, Any]] = None) -> mock.NonCallableMagicMock: + return Mocks.autospec_instance_with_attributes(Request, attributes) + + @staticmethod + def mock_pipeline_to_display(attributes: Optional[Mapping[str, Any]] = None) -> mock.NonCallableMagicMock: + return Mocks.autospec_instance_with_attributes(PipelineToDisplay, attributes) + + +class Patches: @staticmethod - def mock_request() -> mock.NonCallableMagicMock: - return mock.create_autospec(Request, instance=True) + @contextmanager + def patch_context( + object_to_patch: object, + attribute_to_patch: str, + return_value: Optional[Any] = None, + ) -> mock.MagicMock: + with mock.patch.object(object_to_patch, attribute_to_patch) as mocked_item: + if return_value: + mocked_item.return_value = return_value + yield mocked_item + + @staticmethod + @contextmanager + def get_item_with_embeds(**kwargs) -> mock.MagicMock: + with Patches.patch_context( + base_module.PipelineRetriever, "get_item_with_embeds", **kwargs + ) as mocked_item: + yield mocked_item + + @staticmethod + @contextmanager + def get_pipelines(**kwargs) -> mock.MagicMock: + with Patches.patch_context( + base_module.PipelineRetriever, "get_pipelines", **kwargs + ) as mocked_item: + yield mocked_item + + @staticmethod + @contextmanager + def get_pipeline_properties(**kwargs) -> mock.MagicMock: + with Patches.patch_context( + PipelineRetriever, 'get_pipeline_properties', **kwargs + ) as result: + yield result + + @staticmethod + @contextmanager + def custom_embed(**kwargs) -> mock.MagicMock: + with Patches.patch_context( + base_module.custom_embed, "CustomEmbed", **kwargs + ) as result: + yield result + + @staticmethod + @contextmanager + def get_pipelines_for_pipeline_property(**kwargs) -> mock.MagicMock: + with Patches.patch_context( + base_module.PipelineRetriever, "get_pipelines_for_pipeline_property", **kwargs + ) as result: + yield result + + @staticmethod + @contextmanager + def recursive_pipeline_retriever(**kwargs) -> mock.MagicMock: + with Patches.patch_context( + base_module, "RecursivePipelineRetriever", **kwargs + ) as result: + yield result + + @staticmethod + @contextmanager + def pipeline_to_display(**kwargs) -> mock.MagicMock: + with Patches.patch_context( + base_module, "PipelineToDisplay", **kwargs + ) as result: + yield result + + @staticmethod + @contextmanager + def recursive_get_pipelines_from_dict(**kwargs) -> mock.MagicMock: + with Patches.patch_context( + base_module.RecursivePipelineRetriever, "recursive_get_pipelines_from_dict", **kwargs + ) as result: + yield result + + @staticmethod + @contextmanager + def recursive_get_pipelines_from_item(**kwargs) -> mock.MagicMock: + with Patches.patch_context( + base_module.RecursivePipelineRetriever, + "recursive_get_pipelines_from_item", + **kwargs + ) as result: + yield result class TestPipelineView: @@ -46,44 +148,82 @@ def test_pipeline_display_failure(self, testapp: TestApp, project: Mapping) -> N self.assert_pipeline_display_status(testapp, project, 405) -# @pytest.mark.parametrize( -# "item_attributes,expected", -# [ -# ({}, []), -# ({"foo": "bar"}, []), -# ({"pipeline_properties": ["foo", "bar"]}, ["foo", "bar"]), -# ] -# ) -# def test_get_pipeline_properties( -# item_attributes: Mapping[str, Any], expected: Sequence[str] -# ) -> None: -# item = Mocks.mock_item(item_attributes) -# assert get_pipeline_properties(item) == expected - - class TestPipelineRetriever: + SOME_UUID = uuid4() + SOME_IDENTIFIER = str(SOME_UUID) + SOME_PIPELINE_PROPERTIES = ["foo", "bar"] + SOME_ITEM = {"uuid": SOME_IDENTIFIER, "foo": "bar"} + SOME_EMBED_PARAMETERS = {"requested_fields": ["*", "foo.*", "bar.*"]} + SOME_PIPELINE_TO_DISPLAY = Mocks.mock_pipeline_to_display() + SOME_PIPELINES_TO_DISPLAY = [SOME_PIPELINE_TO_DISPLAY, SOME_PIPELINE_TO_DISPLAY] REQUEST = Mocks.mock_request() - CONTEXT = Mocks.mock_item() - PIPELINE_PROPERTIES = ["foo", "bar.buz"] + CONTEXT = Mocks.mock_context( + {"uuid": SOME_UUID, "pipeline_properties": SOME_PIPELINE_PROPERTIES} + ) + + def pipeline_retriever( + self, request: Optional[Request] = None, context: Optional[Item] = None + ) -> PipelineRetriever: + return PipelineRetriever(context or self.CONTEXT, request or self.REQUEST) - def pipeline_retriever(self) -> PipelineRetriever: - return PipelineRetriever(self.CONTEXT, self.REQUEST) + def test_get_pipelines_to_display(self) -> None: + with Patches.get_item_with_embeds(return_value=self.SOME_ITEM): + with Patches.get_pipelines() as mocked_get_pipelines: + pipeline_retriever = self.pipeline_retriever() + result = pipeline_retriever.get_pipelines_to_display() + mocked_get_pipelines.assert_called_once_with(self.SOME_ITEM) + assert result == mocked_get_pipelines.return_value + def test_get_item_with_embeds(self) -> None: + with Patches.custom_embed() as mock_custom_embed: + retriever = self.pipeline_retriever() + result = retriever.get_item_with_embeds() + assert result == mock_custom_embed.return_value.get_embedded_fields() + mock_custom_embed.assert_called_once_with( + self.REQUEST, self.SOME_IDENTIFIER, self.SOME_EMBED_PARAMETERS + ) + + def test_get_item_identifier(self) -> None: + retriever = self.pipeline_retriever() + assert retriever.get_item_identifier() == self.SOME_IDENTIFIER + + def test_get_custom_embed_parameters(self) -> None: + with Patches.get_pipeline_properties( + return_value=self.SOME_PIPELINE_PROPERTIES + ): + retriever = self.pipeline_retriever() + assert retriever.get_custom_embed_parameters() == self.SOME_EMBED_PARAMETERS @pytest.mark.parametrize( "pipeline_properties,expected", [ - ([], []), - (["foo"], ["foo.*"]), - (["foo", "bar.buz"], ["foo.*", "bar.*", "bar.buz.*"]), + ([], ["*"]), + (["foo"], ["*", "foo.*"]), + (["foo", "bar.buz"], ["*", "foo.*", "bar.*", "bar.buz.*"]), ] ) def test_get_properties_to_embed( self, pipeline_properties: Sequence[str], expected: Sequence[str] ) -> None: - retriever = self.pipeline_retriever() - assert retriever.get_properties_to_embed() == expected + with Patches.get_pipeline_properties(return_value=pipeline_properties): + retriever = self.pipeline_retriever() + assert retriever.get_properties_to_embed() == expected + + @pytest.mark.parametrize( + "item_attributes,expected", + [ + ({}, []), + ({"foo": "bar"}, []), + ({"pipeline_properties": ["foo", "bar"]}, ["foo", "bar"]), + ] + ) + def test_get_pipeline_properties( + self, item_attributes: Mapping[str, Any], expected: Sequence[str] + ) -> None: + context = Mocks.mock_context(item_attributes) + retriever = self.pipeline_retriever(context=context) + assert retriever.get_pipeline_properties() == expected @pytest.mark.parametrize( "pipeline_property,expected", @@ -100,3 +240,279 @@ def test_get_properties_to_embed_from_pipeline_property( assert pipeline_retriever.get_properties_to_embed_from_pipeline_property( pipeline_property ) == expected + + def test_make_embed_property(self) -> None: + pipeline_retriever = self.pipeline_retriever() + assert pipeline_retriever.make_embed_property("foo") == "foo.*" + + def test_get_pipelines(self) -> None: + item_with_pipelines = self.SOME_ITEM + pipeline_property_count = len(self.SOME_PIPELINE_PROPERTIES) + with Patches.get_pipeline_properties( + return_value=self.SOME_PIPELINE_PROPERTIES + ): + with Patches.get_pipelines_for_pipeline_property( + return_value=self.SOME_PIPELINES_TO_DISPLAY + ) as mock_get_pipelines_for_pipeline_property: + retriever = self.pipeline_retriever() + result = retriever.get_pipelines(item_with_pipelines) + expected = self.SOME_PIPELINES_TO_DISPLAY * pipeline_property_count + assert result == expected + assert len( + mock_get_pipelines_for_pipeline_property.mock_calls + ) == pipeline_property_count + for idx in range(pipeline_property_count): + mock_get_pipelines_for_pipeline_property.assert_any_call( + item_with_pipelines, self.SOME_PIPELINE_PROPERTIES[idx] + ) + + def test_get_pipelines_for_pipeline_property(self) -> None: + item_properties = self.SOME_ITEM + pipeline_property = "foo.bar" + expected_properties_to_get = ["foo", "bar"] + expected_call = [item_properties, item_properties, expected_properties_to_get] + with Patches.recursive_pipeline_retriever() as mock_recursive_retriever: + pipeline_retriever = self.pipeline_retriever() + result = pipeline_retriever.get_pipelines_for_pipeline_property( + item_properties, pipeline_property + ) + mock_recursive_retriever.assert_called_once_with(*expected_call) + assert result == mock_recursive_retriever.return_value.get_pipelines() + + + @pytest.mark.parametrize( + "pipeline_property,expected", + [ + ("", []), + ("foo", ["foo"]), + ("foo.bar", ["foo", "bar"]), + ] + ) + def test_split_pipeline_property( + self, pipeline_property: str, expected: Sequence[str] + ) -> None: + pipeline_retriever = self.pipeline_retriever() + assert pipeline_retriever.split_pipeline_property(pipeline_property) == expected + + +class TestRecursivePipelineRetriever: + + SOME_NON_ITEM = {"foo": "bar"} + SOME_ITEM = {"foo": "bar", "@type": ["Item"]} + ANOTHER_NON_ITEM = [SOME_ITEM] + SOME_PIPELINE = {"@id": "/pipelines/1/", "@type": ["MetaWorkflowRun", "Item"]} + ANOTHER_PIPELINE = {"@id": "/pipelines/2/", "@type": ["MetaWorkflowRun", "Item"]} + SOME_ITEM_WITH_PIPELINE = {"foo": {"bar": SOME_PIPELINE}, "@type": ["Item"]} + SOME_LIST_WITH_ITEM_WITH_PIPELINE = [SOME_ITEM_WITH_PIPELINE, SOME_ITEM] + SOME_PIPELINE_PROPERTIES = ["foo", "bar"] + + def default_arg(self, arg: Any, default: Any) -> Any: + if arg is None: + return default + return arg + + def recursive_pipeline_retriever( + self, + parent_item: Optional[Mapping] = None, + item: Optional[any] = None, + pipeline_properties: Optional[Sequence[str]] = None, + ) -> RecursivePipelineRetriever: + return RecursivePipelineRetriever( + self.default_arg(parent_item, self.SOME_ITEM), + self.default_arg(item, self.SOME_NON_ITEM), + self.default_arg(pipeline_properties, self.SOME_PIPELINE_PROPERTIES), + ) + + @pytest.mark.parametrize( + "item,pipeline_properties,expected", + [ + ("", None, []), + (5, None, []), + (None, None, []), + (SOME_NON_ITEM, None, []), + ( + SOME_ITEM_WITH_PIPELINE, + None, + [PipelineToDisplay(SOME_ITEM, SOME_PIPELINE)] + ), + (SOME_ITEM_WITH_PIPELINE, ["foo"], []), + (SOME_PIPELINE, None, []), + (SOME_PIPELINE, [], [PipelineToDisplay(SOME_ITEM, SOME_PIPELINE)]), + ( + SOME_LIST_WITH_ITEM_WITH_PIPELINE, + None, + [PipelineToDisplay(SOME_ITEM_WITH_PIPELINE, SOME_PIPELINE)] + ), + ] + ) + def test_get_pipelines( + self, + item: Any, + pipeline_properties: Union[Sequence[str], None], + expected: Sequence[PipelineToDisplay] + ) -> None: + retriever = self.recursive_pipeline_retriever( + item=item, pipeline_properties=pipeline_properties + ) + assert retriever.get_pipelines() == expected + + @pytest.mark.parametrize( + "item,pipeline_properties,expected_recursive_call,expected_pipeline_to_display", + [ + (SOME_NON_ITEM, SOME_PIPELINE_PROPERTIES, True, False), + (SOME_ITEM, SOME_PIPELINE_PROPERTIES, True, False), + (SOME_ITEM, [], False, False), + (SOME_PIPELINE, SOME_PIPELINE_PROPERTIES, True, False), + (SOME_PIPELINE, [], False, True), + ] + ) + def test_get_pipelines_from_dict( + self, + item: Mapping, + pipeline_properties: Sequence[str], + expected_recursive_call: bool, + expected_pipeline_to_display: bool, + ) -> None: + with Patches.recursive_get_pipelines_from_dict( + return_value=[self.SOME_PIPELINE] + ) as mock_recursive_get_pipelines_from_dict: + with Patches.pipeline_to_display() as mock_pipeline_to_display: + recursive_pipeline_retriever = self.recursive_pipeline_retriever( + item=item, pipeline_properties=pipeline_properties + ) + result = recursive_pipeline_retriever.get_pipelines_from_dict() + if expected_recursive_call: + mock_recursive_get_pipelines_from_dict.assert_called_once_with() + assert result == [self.SOME_PIPELINE] + mock_pipeline_to_display.assert_not_called() + elif expected_pipeline_to_display: + mock_recursive_get_pipelines_from_dict.assert_not_called() + assert result == [mock_pipeline_to_display.return_value] + mock_pipeline_to_display.assert_called_once_with(self.SOME_ITEM, item) + else: + mock_recursive_get_pipelines_from_dict.assert_not_called() + mock_pipeline_to_display.assert_not_called() + assert result == [] + + @pytest.mark.parametrize( + "item,expected", + [ + (SOME_NON_ITEM, False), + (SOME_ITEM, False), + (SOME_PIPELINE, True), + ] + ) + def test_is_pipeline_item(self, item: Mapping, expected: bool) -> None: + retriever = self.recursive_pipeline_retriever(item=item) + assert retriever.is_pipeline_item() == expected + + @pytest.mark.parametrize( + "item,properties_to_get,expected_call", + [ + (SOME_PIPELINE, SOME_PIPELINE_PROPERTIES, []), + (SOME_NON_ITEM, SOME_PIPELINE_PROPERTIES, ["bar", ["bar"]]), + (SOME_NON_ITEM, ["foo"], ["bar", []]), + ] + ) + def test_recursive_get_pipelines_from_dict( + self, + item: Mapping, + properties_to_get: Sequence[str], + expected_call: Sequence, + ) -> None: + retriever = self.recursive_pipeline_retriever( + item=item, pipeline_properties=properties_to_get + ) + with Patches.recursive_get_pipelines_from_item( + return_value=[self.SOME_PIPELINE] + ) as mocked_get_pipelines_from_item: + result = retriever.get_pipelines_from_dict() + if expected_call: + assert result == [self.SOME_PIPELINE] + mocked_get_pipelines_from_item.assert_called_once_with(*expected_call) + else: + assert result == [] + mocked_get_pipelines_from_item.assert_not_called() + + @pytest.mark.parametrize( + "item", + [ + (ANOTHER_NON_ITEM), + (SOME_PIPELINE_PROPERTIES), + ] + ) + def test_recursive_get_pipelines_from_list( + self, + item: Sequence, + ) -> None: + with Patches.recursive_get_pipelines_from_item( + return_value=[self.SOME_PIPELINE] + ) as mocked_get_pipelines_from_item: + recursive_pipeline_retriever = self.recursive_pipeline_retriever(item=item) + result = recursive_pipeline_retriever.recursive_get_pipelines_from_list() + for sub_item in item: + mocked_get_pipelines_from_item.assert_any_call(sub_item) + assert mocked_get_pipelines_from_item.call_count == len(item) + assert len(result) == len(item) + for pipeline in result: + assert pipeline == self.SOME_PIPELINE + + @pytest.mark.parametrize( + "item,properties_to_get,expected_call_args", + [ + (SOME_NON_ITEM, None, [SOME_ITEM, SOME_NON_ITEM, SOME_PIPELINE_PROPERTIES]), + (SOME_NON_ITEM, ["foo"], [SOME_ITEM, SOME_NON_ITEM, ["foo"]]), + (SOME_PIPELINE, None, [SOME_PIPELINE, SOME_PIPELINE, SOME_PIPELINE_PROPERTIES]), + (SOME_PIPELINE, [], [SOME_ITEM, SOME_PIPELINE, []]), + (SOME_PIPELINE, ["foo"], [SOME_PIPELINE, SOME_PIPELINE, ["foo"]]), + ] + ) + def test_recursive_get_pipelines_from_item( + self, + item: Any, + properties_to_get: Union[Sequence[str], None], + expected_call_args: Sequence, + ) -> None: + """Test that method creates new parent class recursively. + + Since recursive, mock the class only after intialization. + """ + recursive_pipeline_retriever = self.recursive_pipeline_retriever() + with Patches.recursive_pipeline_retriever() as mock_recursive_pipeline_retriever: + result = recursive_pipeline_retriever.recursive_get_pipelines_from_item( + item, properties_to_get + ) + mock_recursive_pipeline_retriever.assert_called_once_with( + *expected_call_args + ) + assert result == mock_recursive_pipeline_retriever.return_value.get_pipelines() + + @pytest.mark.parametrize( + "item,properties_to_get,expected", + [ + (SOME_NON_ITEM, [], SOME_ITEM), + (SOME_NON_ITEM, SOME_PIPELINE_PROPERTIES, SOME_ITEM), + (SOME_PIPELINE, [], SOME_ITEM), + (SOME_PIPELINE, SOME_PIPELINE_PROPERTIES, SOME_PIPELINE), + ] + ) + def test_get_parent_item_to_pass( + self, item: Any, properties_to_get: Sequence[str], expected: Mapping + ) -> None: + recursive_pipeline_retriever = self.recursive_pipeline_retriever() + assert recursive_pipeline_retriever.get_parent_item_to_pass( + item, properties_to_get + ) == expected + + + @pytest.mark.parametrize( + "item,expected", + [ + (SOME_NON_ITEM, False), + (ANOTHER_NON_ITEM, False), + (SOME_ITEM, True), + ] + ) + def test_is_item(self, item: Any, expected: bool) -> None: + retriever = self.recursive_pipeline_retriever() + assert retriever.is_item(item) == expected diff --git a/src/encoded/types/base.py b/src/encoded/types/base.py index dcf008c725..6423d85668 100644 --- a/src/encoded/types/base.py +++ b/src/encoded/types/base.py @@ -4,7 +4,7 @@ import string from dataclasses import dataclass from requests import Request -from typing import Any, Iterable, List, Optional, Sequence, Tuple, Union +from typing import Any, Dict, Iterable, List, Mapping, Optional, Sequence, Tuple, Union from pyramid.httpexceptions import HTTPMethodNotAllowed from pyramid.security import ( @@ -511,8 +511,70 @@ def validate_item_pipelines_get(context: Item, request: Request) -> None: @debug_log def pipelines(context: Item, request: Request) -> dict: pipeline_retriever = PipelineRetriever(context, request) - pipelines = pipeline_retriever.get_pipelines() - return PipelineDisplayer(pipelines).get_display() + pipelines_to_display = pipeline_retriever.get_pipelines_to_display() + return PipelineDisplayer(pipelines_to_display).get_display() + + +@dataclass(frozen=True) +class PipelineToDisplay: + + ATID = "@id" + COMPLETED = "completed" + DISPLAY_TITLE = "display_title" + FINAL_STATUS = "final_status" + FINAL_STATUS_COMPLETED = "completed" + FINAL_STATUS_STOPPED = "stopped" + FINAL_STATUS_QC_ERROR = "quality metric failed" + NAME = "name" + RUN_STATUS = "run_status" + RUNNING = "running" + STOPPED = "stopped" + STOPPED_FINAL_STATUSES = [FINAL_STATUS_STOPPED, FINAL_STATUS_QC_ERROR] + VERSION = "version" + + parent_item: Mapping[str, Any] + pipeline: Mapping[str, Any] + + def get_parent_item_display(self) -> str: + return { + self.ATID: self.get_parent_item_atid(), + self.NAME: self.get_parent_item_name(), + } + + def get_parent_item_atid(self) -> str: + return self.parent_item.get(self.ATID, "") + + def get_parent_item_name(self) -> str: + return self.parent_item.get(self.DISPLAY_TITLE, "") + + def get_pipeline_display(self) -> Mapping[str, Any]: + return { + self.ATID: self.get_pipeline_atid(), + self.RUN_STATUS: self.get_pipeline_run_status(), + self.NAME: self.get_pipeline_name(), + self.VERSION: self.get_pipeline_version(), + } + + def get_pipeline_run_status(self) -> str: + final_status = self.get_pipeline_final_status() + if final_status == self.FINAL_STATUS_COMPLETED: + return self.COMPLETED + elif final_status in self.STOPPED_FINAL_STATUSES: + return self.STOPPED + else: + return self.RUNNING + + def get_pipeline_final_status(self) -> str: + return self.pipeline.get(self.FINAL_STATUS, "") + + def get_pipeline_name(self) -> str: + return self.pipeline.get(self.NAME, "") + + def get_pipeline_version(self) -> str: + return self.pipeline.get(self.VERSION, "") + + def get_pipeline_atid(self) -> str: + return self.pipeline.get(self.ATID, "") @dataclass(frozen=True) @@ -524,12 +586,9 @@ class PipelineRetriever: context: Item request: Request - def get_pipeline_properties(self) -> List[str]: - return getattr(self.context, self.PIPELINE_PROPERTIES, []) - - def get_pipelines(self) -> List[dict]: + def get_pipelines_to_display(self) -> List[PipelineToDisplay]: item_with_embeds = self.get_item_with_embeds() - return self.get_pipelines_from_embeds(item_with_embeds) + return self.get_pipelines(item_with_embeds) def get_item_with_embeds(self) -> List[dict]: item_identifier = self.get_item_identifier() @@ -545,13 +604,16 @@ def get_custom_embed_parameters(self) -> dict: return {custom_embed.REQUESTED_FIELDS: self.get_properties_to_embed()} def get_properties_to_embed(self) -> List[str]: - result = [] + result = ["*"] for pipeline_property in self.get_pipeline_properties(): result.extend( self.get_properties_to_embed_from_pipeline_property(pipeline_property) ) return result + def get_pipeline_properties(self) -> List[str]: + return getattr(self.context, self.PIPELINE_PROPERTIES, []) + def get_properties_to_embed_from_pipeline_property( self, pipeline_property: str @@ -575,14 +637,115 @@ def make_embed_property(property_to_embed: str) -> str: + custom_embed.EMBED_ALL_FIELDS_MARKER ) - def get_pipelines_from_item(self, item: dict) -> List[dict]: - pass + def get_pipelines(self, embedded_properties: Mapping) -> List[PipelineToDisplay]: + result = [] + for pipeline_property in self.get_pipeline_properties(): + result.extend( + self.get_pipelines_for_pipeline_property( + embedded_properties, pipeline_property + ) + ) + return result + + def get_pipelines_for_pipeline_property( + self, + embedded_properties: Mapping, + pipeline_property: str + ) -> List[PipelineToDisplay]: + properties_to_get = self.split_pipeline_property(pipeline_property) + return RecursivePipelineRetriever( + embedded_properties, embedded_properties, properties_to_get + ).get_pipelines() + + @staticmethod + def split_pipeline_property(pipeline_property: str) -> List[str]: + return [ + term for term in pipeline_property.split(custom_embed.PROPERTY_SPLITTER) + if term + ] @dataclass(frozen=True) -class PipelineDisplayer: +class RecursivePipelineRetriever: - pipelines: Sequence[dict] + TYPES = "@type" + META_WORKFLOW_RUN_TYPE = "MetaWorkflowRun" - def get_display(self) -> dict: - pass + parent_item: Mapping + item_to_get_from: Any + properties_to_get: List[str] + + def get_pipelines(self) -> List[PipelineToDisplay]: + result = [] + if isinstance(self.item_to_get_from, dict): + result.extend(self.get_pipelines_from_dict()) + elif isinstance(self.item_to_get_from, list): + result.extend(self.recursive_get_pipelines_from_list()) + return result + + def get_pipelines_from_dict(self) -> List[PipelineToDisplay]: + result = [] + if self.properties_to_get: + result.extend(self.recursive_get_pipelines_from_dict()) + elif self.is_pipeline_item(): + result.append(PipelineToDisplay(self.parent_item, self.item_to_get_from)) + return result + + def is_pipeline_item(self) -> bool: + return self.META_WORKFLOW_RUN_TYPE in self.item_to_get_from.get(self.TYPES, []) + + def recursive_get_pipelines_from_dict(self) -> List[PipelineToDisplay]: + result = [] + [property_to_get, *remaining_properties_to_get] = self.properties_to_get + new_item_to_get_from = self.item_to_get_from.get(property_to_get, {}) + if new_item_to_get_from: + result.extend( + self.recursive_get_pipelines_from_item( + new_item_to_get_from, remaining_properties_to_get + ) + ) + return result + + def recursive_get_pipelines_from_list(self) -> List[PipelineToDisplay]: + result = [] + for item in self.item_to_get_from: + result.extend(self.recursive_get_pipelines_from_item(item)) + return result + + def recursive_get_pipelines_from_item( + self, item: Any, properties_to_get: Optional[List[str]] = None + ) -> List[PipelineToDisplay]: + if properties_to_get is None: + properties_to_get = self.properties_to_get + parent_item_to_pass = self.get_parent_item_to_pass(item, properties_to_get) + return RecursivePipelineRetriever( + parent_item_to_pass, item, properties_to_get + ).get_pipelines() + + def get_parent_item_to_pass(self, item: Any, properties_to_get: List[str]) -> Dict: + if self.is_item(item) and properties_to_get: + return item + return self.parent_item + + def is_item(self, item: Any) -> bool: + if isinstance(item, dict) and item.get(self.TYPES): + return True + return False + + +@dataclass(frozen=True) +class PipelineDisplayer: + + pipelines_to_display: Sequence[PipelineToDisplay] + + def get_display(self) -> Dict[str, List[Dict]]: + result = {} + for pipeline_to_display in self.pipelines_to_display: + parent_atid = pipeline_to_display.get_parent_item_atid() + pipeline_display = pipeline_to_display.get_pipeline_display() + existing_pipeline_displays = result.get(parent_atid) + if existing_pipeline_displays is None: + result[parent_atid] = [pipeline_display] + else: + existing_pipeline_displays.append(pipeline_display) + return result From 970efe85224190f1b9f3e337e289cc864a11beee Mon Sep 17 00:00:00 2001 From: Douglas Rioux Date: Sat, 4 Feb 2023 12:44:36 -0500 Subject: [PATCH 4/7] Move pipeline view to own module --- src/encoded/pipeline_view.py | 266 ++++++++++++++++++ ...st_types_base.py => test_pipeline_view.py} | 6 +- src/encoded/types/base.py | 256 ----------------- 3 files changed, 270 insertions(+), 258 deletions(-) create mode 100644 src/encoded/pipeline_view.py rename src/encoded/tests/{test_types_base.py => test_pipeline_view.py} (99%) diff --git a/src/encoded/pipeline_view.py b/src/encoded/pipeline_view.py new file mode 100644 index 0000000000..7b421277c6 --- /dev/null +++ b/src/encoded/pipeline_view.py @@ -0,0 +1,266 @@ +from dataclasses import dataclass +from requests import Request +from typing import Any, Dict, List, Mapping, Optional, Sequence + +from pyramid.httpexceptions import HTTPMethodNotAllowed +from pyramid.view import view_config +from snovault.util import debug_log + +from . import custom_embed +from .types.base import Item + + +def validate_item_pipelines_get(context: Item, request: Request) -> None: + pipeline_properties = getattr(context, "pipeline_properties", []) + if not pipeline_properties: + raise HTTPMethodNotAllowed(detail="Item cannot display pipelines") + + +@view_config( + context=Item, + permission="view", + name="pipelines", + request_method="GET", + validators=[validate_item_pipelines_get], +) +@debug_log +def pipelines(context: Item, request: Request) -> dict: + pipeline_retriever = PipelineRetriever(context, request) + pipelines_to_display = pipeline_retriever.get_pipelines_to_display() + return PipelineDisplayer(pipelines_to_display).get_display() + + +@dataclass(frozen=True) +class PipelineToDisplay: + + ATID = "@id" + COMPLETED = "completed" + DISPLAY_TITLE = "display_title" + FINAL_STATUS = "final_status" + FINAL_STATUS_COMPLETED = "completed" + FINAL_STATUS_STOPPED = "stopped" + FINAL_STATUS_QC_ERROR = "quality metric failed" + NAME = "name" + RUN_STATUS = "run_status" + RUNNING = "running" + STOPPED = "stopped" + STOPPED_FINAL_STATUSES = [FINAL_STATUS_STOPPED, FINAL_STATUS_QC_ERROR] + VERSION = "version" + + parent_item: Mapping[str, Any] + pipeline: Mapping[str, Any] + + def get_parent_item_display(self) -> str: + return { + self.ATID: self.get_parent_item_atid(), + self.NAME: self.get_parent_item_name(), + } + + def get_parent_item_atid(self) -> str: + return self.parent_item.get(self.ATID, "") + + def get_parent_item_name(self) -> str: + return self.parent_item.get(self.DISPLAY_TITLE, "") + + def get_pipeline_display(self) -> Mapping[str, Any]: + return { + self.ATID: self.get_pipeline_atid(), + self.RUN_STATUS: self.get_pipeline_run_status(), + self.NAME: self.get_pipeline_name(), + self.VERSION: self.get_pipeline_version(), + } + + def get_pipeline_run_status(self) -> str: + final_status = self.get_pipeline_final_status() + if final_status == self.FINAL_STATUS_COMPLETED: + return self.COMPLETED + elif final_status in self.STOPPED_FINAL_STATUSES: + return self.STOPPED + else: + return self.RUNNING + + def get_pipeline_final_status(self) -> str: + return self.pipeline.get(self.FINAL_STATUS, "") + + def get_pipeline_name(self) -> str: + return self.pipeline.get(self.NAME, "") + + def get_pipeline_version(self) -> str: + return self.pipeline.get(self.VERSION, "") + + def get_pipeline_atid(self) -> str: + return self.pipeline.get(self.ATID, "") + + +@dataclass(frozen=True) +class PipelineRetriever: + + PIPELINE_PROPERTIES = "pipeline_properties" + UUID = "uuid" + + context: Item + request: Request + + def get_pipelines_to_display(self) -> List[PipelineToDisplay]: + item_with_embeds = self.get_item_with_embeds() + return self.get_pipelines(item_with_embeds) + + def get_item_with_embeds(self) -> List[dict]: + item_identifier = self.get_item_identifier() + custom_embed_parameters = self.get_custom_embed_parameters() + return custom_embed.CustomEmbed( + self.request, item_identifier, custom_embed_parameters + ).get_embedded_fields() + + def get_item_identifier(self) -> str: + return str(getattr(self.context, self.UUID, "")) + + def get_custom_embed_parameters(self) -> dict: + return {custom_embed.REQUESTED_FIELDS: self.get_properties_to_embed()} + + def get_properties_to_embed(self) -> List[str]: + result = ["*"] + for pipeline_property in self.get_pipeline_properties(): + result.extend( + self.get_properties_to_embed_from_pipeline_property(pipeline_property) + ) + return result + + def get_pipeline_properties(self) -> List[str]: + return getattr(self.context, self.PIPELINE_PROPERTIES, []) + + def get_properties_to_embed_from_pipeline_property( + self, + pipeline_property: str + ) -> List[str]: + split_properties = [ + term for term in pipeline_property.split(custom_embed.PROPERTY_SPLITTER) + if term + ] + return [ + self.make_embed_property( + custom_embed.PROPERTY_SPLITTER.join(split_properties[:idx + 1]) + ) + for idx in range(len(split_properties)) + ] + + @staticmethod + def make_embed_property(property_to_embed: str) -> str: + return ( + property_to_embed + + custom_embed.PROPERTY_SPLITTER + + custom_embed.EMBED_ALL_FIELDS_MARKER + ) + + def get_pipelines(self, embedded_properties: Mapping) -> List[PipelineToDisplay]: + result = [] + for pipeline_property in self.get_pipeline_properties(): + result.extend( + self.get_pipelines_for_pipeline_property( + embedded_properties, pipeline_property + ) + ) + return result + + def get_pipelines_for_pipeline_property( + self, + embedded_properties: Mapping, + pipeline_property: str + ) -> List[PipelineToDisplay]: + properties_to_get = self.split_pipeline_property(pipeline_property) + return RecursivePipelineRetriever( + embedded_properties, embedded_properties, properties_to_get + ).get_pipelines() + + @staticmethod + def split_pipeline_property(pipeline_property: str) -> List[str]: + return [ + term for term in pipeline_property.split(custom_embed.PROPERTY_SPLITTER) + if term + ] + + +@dataclass(frozen=True) +class RecursivePipelineRetriever: + + TYPES = "@type" + META_WORKFLOW_RUN_TYPE = "MetaWorkflowRun" + + parent_item: Mapping + item_to_get_from: Any + properties_to_get: List[str] + + def get_pipelines(self) -> List[PipelineToDisplay]: + result = [] + if isinstance(self.item_to_get_from, dict): + result.extend(self.get_pipelines_from_dict()) + elif isinstance(self.item_to_get_from, list): + result.extend(self.recursive_get_pipelines_from_list()) + return result + + def get_pipelines_from_dict(self) -> List[PipelineToDisplay]: + result = [] + if self.properties_to_get: + result.extend(self.recursive_get_pipelines_from_dict()) + elif self.is_pipeline_item(): + result.append(PipelineToDisplay(self.parent_item, self.item_to_get_from)) + return result + + def is_pipeline_item(self) -> bool: + return self.META_WORKFLOW_RUN_TYPE in self.item_to_get_from.get(self.TYPES, []) + + def recursive_get_pipelines_from_dict(self) -> List[PipelineToDisplay]: + result = [] + [property_to_get, *remaining_properties_to_get] = self.properties_to_get + new_item_to_get_from = self.item_to_get_from.get(property_to_get, {}) + if new_item_to_get_from: + result.extend( + self.recursive_get_pipelines_from_item( + new_item_to_get_from, remaining_properties_to_get + ) + ) + return result + + def recursive_get_pipelines_from_list(self) -> List[PipelineToDisplay]: + result = [] + for item in self.item_to_get_from: + result.extend(self.recursive_get_pipelines_from_item(item)) + return result + + def recursive_get_pipelines_from_item( + self, item: Any, properties_to_get: Optional[List[str]] = None + ) -> List[PipelineToDisplay]: + if properties_to_get is None: + properties_to_get = self.properties_to_get + parent_item_to_pass = self.get_parent_item_to_pass(item, properties_to_get) + return RecursivePipelineRetriever( + parent_item_to_pass, item, properties_to_get + ).get_pipelines() + + def get_parent_item_to_pass(self, item: Any, properties_to_get: List[str]) -> Dict: + if self.is_item(item) and properties_to_get: + return item + return self.parent_item + + def is_item(self, item: Any) -> bool: + if isinstance(item, dict) and item.get(self.TYPES): + return True + return False + + +@dataclass(frozen=True) +class PipelineDisplayer: + + pipelines_to_display: Sequence[PipelineToDisplay] + + def get_display(self) -> Dict[str, List[Dict]]: + result = {} + for pipeline_to_display in self.pipelines_to_display: + parent_atid = pipeline_to_display.get_parent_item_atid() + pipeline_display = pipeline_to_display.get_pipeline_display() + existing_pipeline_displays = result.get(parent_atid) + if existing_pipeline_displays is None: + result[parent_atid] = [pipeline_display] + else: + existing_pipeline_displays.append(pipeline_display) + return result diff --git a/src/encoded/tests/test_types_base.py b/src/encoded/tests/test_pipeline_view.py similarity index 99% rename from src/encoded/tests/test_types_base.py rename to src/encoded/tests/test_pipeline_view.py index a5f0a9213c..15245a3159 100644 --- a/src/encoded/tests/test_types_base.py +++ b/src/encoded/tests/test_pipeline_view.py @@ -7,8 +7,10 @@ import pytest from webtest import TestApp -from ..types import base as base_module -from ..types.base import Item, PipelineRetriever, PipelineToDisplay, RecursivePipelineRetriever +from .. import pipeline_view as pipeline_view_module +from ..pipeline_view import ( + Item, PipelineRetriever, PipelineToDisplay, RecursivePipelineRetriever +) class Mocks: diff --git a/src/encoded/types/base.py b/src/encoded/types/base.py index 6423d85668..b909ab295f 100644 --- a/src/encoded/types/base.py +++ b/src/encoded/types/base.py @@ -493,259 +493,3 @@ def item_edit(context, request, render=None): # This works # Probably don't need to extend re: institution + project since if editing, assuming these have previously existed. return sno_item_edit(context, request, render) - - -def validate_item_pipelines_get(context: Item, request: Request) -> None: - pipeline_properties = getattr(context, "pipeline_properties", []) - if not pipeline_properties: - raise HTTPMethodNotAllowed(detail="Item cannot display pipelines") - - -@view_config( - context=Item, - permission="view", - name="pipelines", - request_method="GET", - validators=[validate_item_pipelines_get], -) -@debug_log -def pipelines(context: Item, request: Request) -> dict: - pipeline_retriever = PipelineRetriever(context, request) - pipelines_to_display = pipeline_retriever.get_pipelines_to_display() - return PipelineDisplayer(pipelines_to_display).get_display() - - -@dataclass(frozen=True) -class PipelineToDisplay: - - ATID = "@id" - COMPLETED = "completed" - DISPLAY_TITLE = "display_title" - FINAL_STATUS = "final_status" - FINAL_STATUS_COMPLETED = "completed" - FINAL_STATUS_STOPPED = "stopped" - FINAL_STATUS_QC_ERROR = "quality metric failed" - NAME = "name" - RUN_STATUS = "run_status" - RUNNING = "running" - STOPPED = "stopped" - STOPPED_FINAL_STATUSES = [FINAL_STATUS_STOPPED, FINAL_STATUS_QC_ERROR] - VERSION = "version" - - parent_item: Mapping[str, Any] - pipeline: Mapping[str, Any] - - def get_parent_item_display(self) -> str: - return { - self.ATID: self.get_parent_item_atid(), - self.NAME: self.get_parent_item_name(), - } - - def get_parent_item_atid(self) -> str: - return self.parent_item.get(self.ATID, "") - - def get_parent_item_name(self) -> str: - return self.parent_item.get(self.DISPLAY_TITLE, "") - - def get_pipeline_display(self) -> Mapping[str, Any]: - return { - self.ATID: self.get_pipeline_atid(), - self.RUN_STATUS: self.get_pipeline_run_status(), - self.NAME: self.get_pipeline_name(), - self.VERSION: self.get_pipeline_version(), - } - - def get_pipeline_run_status(self) -> str: - final_status = self.get_pipeline_final_status() - if final_status == self.FINAL_STATUS_COMPLETED: - return self.COMPLETED - elif final_status in self.STOPPED_FINAL_STATUSES: - return self.STOPPED - else: - return self.RUNNING - - def get_pipeline_final_status(self) -> str: - return self.pipeline.get(self.FINAL_STATUS, "") - - def get_pipeline_name(self) -> str: - return self.pipeline.get(self.NAME, "") - - def get_pipeline_version(self) -> str: - return self.pipeline.get(self.VERSION, "") - - def get_pipeline_atid(self) -> str: - return self.pipeline.get(self.ATID, "") - - -@dataclass(frozen=True) -class PipelineRetriever: - - PIPELINE_PROPERTIES = "pipeline_properties" - UUID = "uuid" - - context: Item - request: Request - - def get_pipelines_to_display(self) -> List[PipelineToDisplay]: - item_with_embeds = self.get_item_with_embeds() - return self.get_pipelines(item_with_embeds) - - def get_item_with_embeds(self) -> List[dict]: - item_identifier = self.get_item_identifier() - custom_embed_parameters = self.get_custom_embed_parameters() - return custom_embed.CustomEmbed( - self.request, item_identifier, custom_embed_parameters - ).get_embedded_fields() - - def get_item_identifier(self) -> str: - return str(getattr(self.context, self.UUID, "")) - - def get_custom_embed_parameters(self) -> dict: - return {custom_embed.REQUESTED_FIELDS: self.get_properties_to_embed()} - - def get_properties_to_embed(self) -> List[str]: - result = ["*"] - for pipeline_property in self.get_pipeline_properties(): - result.extend( - self.get_properties_to_embed_from_pipeline_property(pipeline_property) - ) - return result - - def get_pipeline_properties(self) -> List[str]: - return getattr(self.context, self.PIPELINE_PROPERTIES, []) - - def get_properties_to_embed_from_pipeline_property( - self, - pipeline_property: str - ) -> List[str]: - split_properties = [ - term for term in pipeline_property.split(custom_embed.PROPERTY_SPLITTER) - if term - ] - return [ - self.make_embed_property( - custom_embed.PROPERTY_SPLITTER.join(split_properties[:idx + 1]) - ) - for idx in range(len(split_properties)) - ] - - @staticmethod - def make_embed_property(property_to_embed: str) -> str: - return ( - property_to_embed - + custom_embed.PROPERTY_SPLITTER - + custom_embed.EMBED_ALL_FIELDS_MARKER - ) - - def get_pipelines(self, embedded_properties: Mapping) -> List[PipelineToDisplay]: - result = [] - for pipeline_property in self.get_pipeline_properties(): - result.extend( - self.get_pipelines_for_pipeline_property( - embedded_properties, pipeline_property - ) - ) - return result - - def get_pipelines_for_pipeline_property( - self, - embedded_properties: Mapping, - pipeline_property: str - ) -> List[PipelineToDisplay]: - properties_to_get = self.split_pipeline_property(pipeline_property) - return RecursivePipelineRetriever( - embedded_properties, embedded_properties, properties_to_get - ).get_pipelines() - - @staticmethod - def split_pipeline_property(pipeline_property: str) -> List[str]: - return [ - term for term in pipeline_property.split(custom_embed.PROPERTY_SPLITTER) - if term - ] - - -@dataclass(frozen=True) -class RecursivePipelineRetriever: - - TYPES = "@type" - META_WORKFLOW_RUN_TYPE = "MetaWorkflowRun" - - parent_item: Mapping - item_to_get_from: Any - properties_to_get: List[str] - - def get_pipelines(self) -> List[PipelineToDisplay]: - result = [] - if isinstance(self.item_to_get_from, dict): - result.extend(self.get_pipelines_from_dict()) - elif isinstance(self.item_to_get_from, list): - result.extend(self.recursive_get_pipelines_from_list()) - return result - - def get_pipelines_from_dict(self) -> List[PipelineToDisplay]: - result = [] - if self.properties_to_get: - result.extend(self.recursive_get_pipelines_from_dict()) - elif self.is_pipeline_item(): - result.append(PipelineToDisplay(self.parent_item, self.item_to_get_from)) - return result - - def is_pipeline_item(self) -> bool: - return self.META_WORKFLOW_RUN_TYPE in self.item_to_get_from.get(self.TYPES, []) - - def recursive_get_pipelines_from_dict(self) -> List[PipelineToDisplay]: - result = [] - [property_to_get, *remaining_properties_to_get] = self.properties_to_get - new_item_to_get_from = self.item_to_get_from.get(property_to_get, {}) - if new_item_to_get_from: - result.extend( - self.recursive_get_pipelines_from_item( - new_item_to_get_from, remaining_properties_to_get - ) - ) - return result - - def recursive_get_pipelines_from_list(self) -> List[PipelineToDisplay]: - result = [] - for item in self.item_to_get_from: - result.extend(self.recursive_get_pipelines_from_item(item)) - return result - - def recursive_get_pipelines_from_item( - self, item: Any, properties_to_get: Optional[List[str]] = None - ) -> List[PipelineToDisplay]: - if properties_to_get is None: - properties_to_get = self.properties_to_get - parent_item_to_pass = self.get_parent_item_to_pass(item, properties_to_get) - return RecursivePipelineRetriever( - parent_item_to_pass, item, properties_to_get - ).get_pipelines() - - def get_parent_item_to_pass(self, item: Any, properties_to_get: List[str]) -> Dict: - if self.is_item(item) and properties_to_get: - return item - return self.parent_item - - def is_item(self, item: Any) -> bool: - if isinstance(item, dict) and item.get(self.TYPES): - return True - return False - - -@dataclass(frozen=True) -class PipelineDisplayer: - - pipelines_to_display: Sequence[PipelineToDisplay] - - def get_display(self) -> Dict[str, List[Dict]]: - result = {} - for pipeline_to_display in self.pipelines_to_display: - parent_atid = pipeline_to_display.get_parent_item_atid() - pipeline_display = pipeline_to_display.get_pipeline_display() - existing_pipeline_displays = result.get(parent_atid) - if existing_pipeline_displays is None: - result[parent_atid] = [pipeline_display] - else: - existing_pipeline_displays.append(pipeline_display) - return result From 4dbee2c65978894edd702505acee530f8dd2a15d Mon Sep 17 00:00:00 2001 From: Douglas Rioux Date: Mon, 6 Feb 2023 12:01:09 -0500 Subject: [PATCH 5/7] Refactor for separate module to pass tests --- src/encoded/__init__.py | 1 + src/encoded/pipeline_view.py | 4 ++++ src/encoded/tests/test_pipeline_view.py | 16 ++++++++-------- src/encoded/types/base.py | 6 +----- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/encoded/__init__.py b/src/encoded/__init__.py index 08f36e8d55..896f662c7a 100644 --- a/src/encoded/__init__.py +++ b/src/encoded/__init__.py @@ -192,6 +192,7 @@ def main(global_config, **local_config): config.include('.visualization') config.include('.ingestion_listener') config.include('.custom_embed') + config.include('.pipeline_view') if 'elasticsearch.server' in config.registry.settings: config.include('snovault.elasticsearch') diff --git a/src/encoded/pipeline_view.py b/src/encoded/pipeline_view.py index 7b421277c6..6d6cfd41b9 100644 --- a/src/encoded/pipeline_view.py +++ b/src/encoded/pipeline_view.py @@ -10,6 +10,10 @@ from .types.base import Item +def includeme(config): + config.scan(__name__) + + def validate_item_pipelines_get(context: Item, request: Request) -> None: pipeline_properties = getattr(context, "pipeline_properties", []) if not pipeline_properties: diff --git a/src/encoded/tests/test_pipeline_view.py b/src/encoded/tests/test_pipeline_view.py index 15245a3159..e9313fdc00 100644 --- a/src/encoded/tests/test_pipeline_view.py +++ b/src/encoded/tests/test_pipeline_view.py @@ -56,7 +56,7 @@ def patch_context( @contextmanager def get_item_with_embeds(**kwargs) -> mock.MagicMock: with Patches.patch_context( - base_module.PipelineRetriever, "get_item_with_embeds", **kwargs + pipeline_view_module.PipelineRetriever, "get_item_with_embeds", **kwargs ) as mocked_item: yield mocked_item @@ -64,7 +64,7 @@ def get_item_with_embeds(**kwargs) -> mock.MagicMock: @contextmanager def get_pipelines(**kwargs) -> mock.MagicMock: with Patches.patch_context( - base_module.PipelineRetriever, "get_pipelines", **kwargs + pipeline_view_module.PipelineRetriever, "get_pipelines", **kwargs ) as mocked_item: yield mocked_item @@ -80,7 +80,7 @@ def get_pipeline_properties(**kwargs) -> mock.MagicMock: @contextmanager def custom_embed(**kwargs) -> mock.MagicMock: with Patches.patch_context( - base_module.custom_embed, "CustomEmbed", **kwargs + pipeline_view_module.custom_embed, "CustomEmbed", **kwargs ) as result: yield result @@ -88,7 +88,7 @@ def custom_embed(**kwargs) -> mock.MagicMock: @contextmanager def get_pipelines_for_pipeline_property(**kwargs) -> mock.MagicMock: with Patches.patch_context( - base_module.PipelineRetriever, "get_pipelines_for_pipeline_property", **kwargs + pipeline_view_module.PipelineRetriever, "get_pipelines_for_pipeline_property", **kwargs ) as result: yield result @@ -96,7 +96,7 @@ def get_pipelines_for_pipeline_property(**kwargs) -> mock.MagicMock: @contextmanager def recursive_pipeline_retriever(**kwargs) -> mock.MagicMock: with Patches.patch_context( - base_module, "RecursivePipelineRetriever", **kwargs + pipeline_view_module, "RecursivePipelineRetriever", **kwargs ) as result: yield result @@ -104,7 +104,7 @@ def recursive_pipeline_retriever(**kwargs) -> mock.MagicMock: @contextmanager def pipeline_to_display(**kwargs) -> mock.MagicMock: with Patches.patch_context( - base_module, "PipelineToDisplay", **kwargs + pipeline_view_module, "PipelineToDisplay", **kwargs ) as result: yield result @@ -112,7 +112,7 @@ def pipeline_to_display(**kwargs) -> mock.MagicMock: @contextmanager def recursive_get_pipelines_from_dict(**kwargs) -> mock.MagicMock: with Patches.patch_context( - base_module.RecursivePipelineRetriever, "recursive_get_pipelines_from_dict", **kwargs + pipeline_view_module.RecursivePipelineRetriever, "recursive_get_pipelines_from_dict", **kwargs ) as result: yield result @@ -120,7 +120,7 @@ def recursive_get_pipelines_from_dict(**kwargs) -> mock.MagicMock: @contextmanager def recursive_get_pipelines_from_item(**kwargs) -> mock.MagicMock: with Patches.patch_context( - base_module.RecursivePipelineRetriever, + pipeline_view_module.RecursivePipelineRetriever, "recursive_get_pipelines_from_item", **kwargs ) as result: diff --git a/src/encoded/types/base.py b/src/encoded/types/base.py index b909ab295f..ecc826df02 100644 --- a/src/encoded/types/base.py +++ b/src/encoded/types/base.py @@ -2,11 +2,8 @@ import re import snovault import string -from dataclasses import dataclass -from requests import Request -from typing import Any, Dict, Iterable, List, Mapping, Optional, Sequence, Tuple, Union +from typing import Any, List, Tuple, Union -from pyramid.httpexceptions import HTTPMethodNotAllowed from pyramid.security import ( Allow, Authenticated, @@ -32,7 +29,6 @@ ) from snovault.interfaces import CONNECTION -from .. import custom_embed from ..server_defaults import get_userid, add_last_modified From 4ae7d367aaf91060a267faaa1799982f34715d94 Mon Sep 17 00:00:00 2001 From: Douglas Rioux Date: Mon, 6 Feb 2023 12:25:04 -0500 Subject: [PATCH 6/7] Format with black --- src/encoded/pipeline_view.py | 19 +++-- src/encoded/tests/test_pipeline_view.py | 104 +++++++++++++++--------- 2 files changed, 74 insertions(+), 49 deletions(-) diff --git a/src/encoded/pipeline_view.py b/src/encoded/pipeline_view.py index 6d6cfd41b9..335eef2bd4 100644 --- a/src/encoded/pipeline_view.py +++ b/src/encoded/pipeline_view.py @@ -36,6 +36,7 @@ def pipelines(context: Item, request: Request) -> dict: @dataclass(frozen=True) class PipelineToDisplay: + """TODO: Finalize display properties once front-end settles.""" ATID = "@id" COMPLETED = "completed" @@ -134,16 +135,16 @@ def get_pipeline_properties(self) -> List[str]: return getattr(self.context, self.PIPELINE_PROPERTIES, []) def get_properties_to_embed_from_pipeline_property( - self, - pipeline_property: str + self, pipeline_property: str ) -> List[str]: split_properties = [ - term for term in pipeline_property.split(custom_embed.PROPERTY_SPLITTER) + term + for term in pipeline_property.split(custom_embed.PROPERTY_SPLITTER) if term ] return [ self.make_embed_property( - custom_embed.PROPERTY_SPLITTER.join(split_properties[:idx + 1]) + custom_embed.PROPERTY_SPLITTER.join(split_properties[: idx + 1]) ) for idx in range(len(split_properties)) ] @@ -167,9 +168,7 @@ def get_pipelines(self, embedded_properties: Mapping) -> List[PipelineToDisplay] return result def get_pipelines_for_pipeline_property( - self, - embedded_properties: Mapping, - pipeline_property: str + self, embedded_properties: Mapping, pipeline_property: str ) -> List[PipelineToDisplay]: properties_to_get = self.split_pipeline_property(pipeline_property) return RecursivePipelineRetriever( @@ -179,7 +178,8 @@ def get_pipelines_for_pipeline_property( @staticmethod def split_pipeline_property(pipeline_property: str) -> List[str]: return [ - term for term in pipeline_property.split(custom_embed.PROPERTY_SPLITTER) + term + for term in pipeline_property.split(custom_embed.PROPERTY_SPLITTER) if term ] @@ -254,10 +254,11 @@ def is_item(self, item: Any) -> bool: @dataclass(frozen=True) class PipelineDisplayer: + """TODO: Finalize display once front-end settles.""" pipelines_to_display: Sequence[PipelineToDisplay] - def get_display(self) -> Dict[str, List[Dict]]: + def get_display(self) -> Dict[str, List[Dict]]: result = {} for pipeline_to_display in self.pipelines_to_display: parent_atid = pipeline_to_display.get_parent_item_atid() diff --git a/src/encoded/tests/test_pipeline_view.py b/src/encoded/tests/test_pipeline_view.py index e9313fdc00..81823cae73 100644 --- a/src/encoded/tests/test_pipeline_view.py +++ b/src/encoded/tests/test_pipeline_view.py @@ -9,12 +9,14 @@ from .. import pipeline_view as pipeline_view_module from ..pipeline_view import ( - Item, PipelineRetriever, PipelineToDisplay, RecursivePipelineRetriever + Item, + PipelineRetriever, + PipelineToDisplay, + RecursivePipelineRetriever, ) class Mocks: - @staticmethod def autospec_instance_with_attributes( item_to_mock: object, attributes: Optional[Mapping[str, Any]] = None @@ -24,22 +26,27 @@ def autospec_instance_with_attributes( for attribute, value in attributes.items(): setattr(mocked_item, attribute, value) return mocked_item - + @staticmethod - def mock_context(attributes: Optional[Mapping[str, Any]] = None) -> mock.NonCallableMagicMock: + def mock_context( + attributes: Optional[Mapping[str, Any]] = None + ) -> mock.NonCallableMagicMock: return Mocks.autospec_instance_with_attributes(Item, attributes) @staticmethod - def mock_request(attributes: Optional[Mapping[str, Any]] = None) -> mock.NonCallableMagicMock: + def mock_request( + attributes: Optional[Mapping[str, Any]] = None + ) -> mock.NonCallableMagicMock: return Mocks.autospec_instance_with_attributes(Request, attributes) @staticmethod - def mock_pipeline_to_display(attributes: Optional[Mapping[str, Any]] = None) -> mock.NonCallableMagicMock: + def mock_pipeline_to_display( + attributes: Optional[Mapping[str, Any]] = None + ) -> mock.NonCallableMagicMock: return Mocks.autospec_instance_with_attributes(PipelineToDisplay, attributes) class Patches: - @staticmethod @contextmanager def patch_context( @@ -72,7 +79,7 @@ def get_pipelines(**kwargs) -> mock.MagicMock: @contextmanager def get_pipeline_properties(**kwargs) -> mock.MagicMock: with Patches.patch_context( - PipelineRetriever, 'get_pipeline_properties', **kwargs + PipelineRetriever, "get_pipeline_properties", **kwargs ) as result: yield result @@ -88,7 +95,9 @@ def custom_embed(**kwargs) -> mock.MagicMock: @contextmanager def get_pipelines_for_pipeline_property(**kwargs) -> mock.MagicMock: with Patches.patch_context( - pipeline_view_module.PipelineRetriever, "get_pipelines_for_pipeline_property", **kwargs + pipeline_view_module.PipelineRetriever, + "get_pipelines_for_pipeline_property", + **kwargs ) as result: yield result @@ -112,7 +121,9 @@ def pipeline_to_display(**kwargs) -> mock.MagicMock: @contextmanager def recursive_get_pipelines_from_dict(**kwargs) -> mock.MagicMock: with Patches.patch_context( - pipeline_view_module.RecursivePipelineRetriever, "recursive_get_pipelines_from_dict", **kwargs + pipeline_view_module.RecursivePipelineRetriever, + "recursive_get_pipelines_from_dict", + **kwargs ) as result: yield result @@ -203,7 +214,7 @@ def test_get_custom_embed_parameters(self) -> None: ([], ["*"]), (["foo"], ["*", "foo.*"]), (["foo", "bar.buz"], ["*", "foo.*", "bar.*", "bar.buz.*"]), - ] + ], ) def test_get_properties_to_embed( self, pipeline_properties: Sequence[str], expected: Sequence[str] @@ -218,7 +229,7 @@ def test_get_properties_to_embed( ({}, []), ({"foo": "bar"}, []), ({"pipeline_properties": ["foo", "bar"]}, ["foo", "bar"]), - ] + ], ) def test_get_pipeline_properties( self, item_attributes: Mapping[str, Any], expected: Sequence[str] @@ -233,15 +244,18 @@ def test_get_pipeline_properties( ("", []), ("foo", ["foo.*"]), ("foo.bar", ["foo.*", "foo.bar.*"]), - ] + ], ) def test_get_properties_to_embed_from_pipeline_property( - self, pipeline_property: str, expected: List[str] + self, pipeline_property: str, expected: List[str] ) -> None: pipeline_retriever = self.pipeline_retriever() - assert pipeline_retriever.get_properties_to_embed_from_pipeline_property( - pipeline_property - ) == expected + assert ( + pipeline_retriever.get_properties_to_embed_from_pipeline_property( + pipeline_property + ) + == expected + ) def test_make_embed_property(self) -> None: pipeline_retriever = self.pipeline_retriever() @@ -260,9 +274,10 @@ def test_get_pipelines(self) -> None: result = retriever.get_pipelines(item_with_pipelines) expected = self.SOME_PIPELINES_TO_DISPLAY * pipeline_property_count assert result == expected - assert len( - mock_get_pipelines_for_pipeline_property.mock_calls - ) == pipeline_property_count + assert ( + len(mock_get_pipelines_for_pipeline_property.mock_calls) + == pipeline_property_count + ) for idx in range(pipeline_property_count): mock_get_pipelines_for_pipeline_property.assert_any_call( item_with_pipelines, self.SOME_PIPELINE_PROPERTIES[idx] @@ -281,14 +296,13 @@ def test_get_pipelines_for_pipeline_property(self) -> None: mock_recursive_retriever.assert_called_once_with(*expected_call) assert result == mock_recursive_retriever.return_value.get_pipelines() - @pytest.mark.parametrize( "pipeline_property,expected", [ ("", []), ("foo", ["foo"]), ("foo.bar", ["foo", "bar"]), - ] + ], ) def test_split_pipeline_property( self, pipeline_property: str, expected: Sequence[str] @@ -335,7 +349,7 @@ def recursive_pipeline_retriever( ( SOME_ITEM_WITH_PIPELINE, None, - [PipelineToDisplay(SOME_ITEM, SOME_PIPELINE)] + [PipelineToDisplay(SOME_ITEM, SOME_PIPELINE)], ), (SOME_ITEM_WITH_PIPELINE, ["foo"], []), (SOME_PIPELINE, None, []), @@ -343,15 +357,15 @@ def recursive_pipeline_retriever( ( SOME_LIST_WITH_ITEM_WITH_PIPELINE, None, - [PipelineToDisplay(SOME_ITEM_WITH_PIPELINE, SOME_PIPELINE)] + [PipelineToDisplay(SOME_ITEM_WITH_PIPELINE, SOME_PIPELINE)], ), - ] + ], ) def test_get_pipelines( self, item: Any, pipeline_properties: Union[Sequence[str], None], - expected: Sequence[PipelineToDisplay] + expected: Sequence[PipelineToDisplay], ) -> None: retriever = self.recursive_pipeline_retriever( item=item, pipeline_properties=pipeline_properties @@ -366,7 +380,7 @@ def test_get_pipelines( (SOME_ITEM, [], False, False), (SOME_PIPELINE, SOME_PIPELINE_PROPERTIES, True, False), (SOME_PIPELINE, [], False, True), - ] + ], ) def test_get_pipelines_from_dict( self, @@ -390,7 +404,9 @@ def test_get_pipelines_from_dict( elif expected_pipeline_to_display: mock_recursive_get_pipelines_from_dict.assert_not_called() assert result == [mock_pipeline_to_display.return_value] - mock_pipeline_to_display.assert_called_once_with(self.SOME_ITEM, item) + mock_pipeline_to_display.assert_called_once_with( + self.SOME_ITEM, item + ) else: mock_recursive_get_pipelines_from_dict.assert_not_called() mock_pipeline_to_display.assert_not_called() @@ -402,7 +418,7 @@ def test_get_pipelines_from_dict( (SOME_NON_ITEM, False), (SOME_ITEM, False), (SOME_PIPELINE, True), - ] + ], ) def test_is_pipeline_item(self, item: Mapping, expected: bool) -> None: retriever = self.recursive_pipeline_retriever(item=item) @@ -414,7 +430,7 @@ def test_is_pipeline_item(self, item: Mapping, expected: bool) -> None: (SOME_PIPELINE, SOME_PIPELINE_PROPERTIES, []), (SOME_NON_ITEM, SOME_PIPELINE_PROPERTIES, ["bar", ["bar"]]), (SOME_NON_ITEM, ["foo"], ["bar", []]), - ] + ], ) def test_recursive_get_pipelines_from_dict( self, @@ -441,7 +457,7 @@ def test_recursive_get_pipelines_from_dict( [ (ANOTHER_NON_ITEM), (SOME_PIPELINE_PROPERTIES), - ] + ], ) def test_recursive_get_pipelines_from_list( self, @@ -464,10 +480,14 @@ def test_recursive_get_pipelines_from_list( [ (SOME_NON_ITEM, None, [SOME_ITEM, SOME_NON_ITEM, SOME_PIPELINE_PROPERTIES]), (SOME_NON_ITEM, ["foo"], [SOME_ITEM, SOME_NON_ITEM, ["foo"]]), - (SOME_PIPELINE, None, [SOME_PIPELINE, SOME_PIPELINE, SOME_PIPELINE_PROPERTIES]), + ( + SOME_PIPELINE, + None, + [SOME_PIPELINE, SOME_PIPELINE, SOME_PIPELINE_PROPERTIES], + ), (SOME_PIPELINE, [], [SOME_ITEM, SOME_PIPELINE, []]), (SOME_PIPELINE, ["foo"], [SOME_PIPELINE, SOME_PIPELINE, ["foo"]]), - ] + ], ) def test_recursive_get_pipelines_from_item( self, @@ -487,7 +507,9 @@ def test_recursive_get_pipelines_from_item( mock_recursive_pipeline_retriever.assert_called_once_with( *expected_call_args ) - assert result == mock_recursive_pipeline_retriever.return_value.get_pipelines() + assert ( + result == mock_recursive_pipeline_retriever.return_value.get_pipelines() + ) @pytest.mark.parametrize( "item,properties_to_get,expected", @@ -496,24 +518,26 @@ def test_recursive_get_pipelines_from_item( (SOME_NON_ITEM, SOME_PIPELINE_PROPERTIES, SOME_ITEM), (SOME_PIPELINE, [], SOME_ITEM), (SOME_PIPELINE, SOME_PIPELINE_PROPERTIES, SOME_PIPELINE), - ] + ], ) def test_get_parent_item_to_pass( self, item: Any, properties_to_get: Sequence[str], expected: Mapping ) -> None: recursive_pipeline_retriever = self.recursive_pipeline_retriever() - assert recursive_pipeline_retriever.get_parent_item_to_pass( - item, properties_to_get - ) == expected + assert ( + recursive_pipeline_retriever.get_parent_item_to_pass( + item, properties_to_get + ) + == expected + ) - @pytest.mark.parametrize( "item,expected", [ (SOME_NON_ITEM, False), (ANOTHER_NON_ITEM, False), (SOME_ITEM, True), - ] + ], ) def test_is_item(self, item: Any, expected: bool) -> None: retriever = self.recursive_pipeline_retriever() From f7d625766362151e3fe8792cbb6f8d6731b1aaa2 Mon Sep 17 00:00:00 2001 From: Douglas Rioux Date: Mon, 6 Feb 2023 14:52:07 -0500 Subject: [PATCH 7/7] Clean up typing + minor refactor --- src/encoded/pipeline_view.py | 39 +++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/encoded/pipeline_view.py b/src/encoded/pipeline_view.py index 335eef2bd4..e0b49b196c 100644 --- a/src/encoded/pipeline_view.py +++ b/src/encoded/pipeline_view.py @@ -10,12 +10,15 @@ from .types.base import Item +PIPELINE_PROPERTIES = "pipeline_properties" + + def includeme(config): config.scan(__name__) def validate_item_pipelines_get(context: Item, request: Request) -> None: - pipeline_properties = getattr(context, "pipeline_properties", []) + pipeline_properties = getattr(context, PIPELINE_PROPERTIES, []) if not pipeline_properties: raise HTTPMethodNotAllowed(detail="Item cannot display pipelines") @@ -28,9 +31,10 @@ def validate_item_pipelines_get(context: Item, request: Request) -> None: validators=[validate_item_pipelines_get], ) @debug_log -def pipelines(context: Item, request: Request) -> dict: - pipeline_retriever = PipelineRetriever(context, request) - pipelines_to_display = pipeline_retriever.get_pipelines_to_display() +def pipelines(context: Item, request: Request) -> Dict: + pipelines_to_display = PipelineRetriever( + context, request + ).get_pipelines_to_display() return PipelineDisplayer(pipelines_to_display).get_display() @@ -55,7 +59,7 @@ class PipelineToDisplay: parent_item: Mapping[str, Any] pipeline: Mapping[str, Any] - def get_parent_item_display(self) -> str: + def get_parent_item_display(self) -> Dict[str, str]: return { self.ATID: self.get_parent_item_atid(), self.NAME: self.get_parent_item_name(), @@ -67,7 +71,7 @@ def get_parent_item_atid(self) -> str: def get_parent_item_name(self) -> str: return self.parent_item.get(self.DISPLAY_TITLE, "") - def get_pipeline_display(self) -> Mapping[str, Any]: + def get_pipeline_display(self) -> Dict[str, Any]: return { self.ATID: self.get_pipeline_atid(), self.RUN_STATUS: self.get_pipeline_run_status(), @@ -100,7 +104,6 @@ def get_pipeline_atid(self) -> str: @dataclass(frozen=True) class PipelineRetriever: - PIPELINE_PROPERTIES = "pipeline_properties" UUID = "uuid" context: Item @@ -124,7 +127,7 @@ def get_custom_embed_parameters(self) -> dict: return {custom_embed.REQUESTED_FIELDS: self.get_properties_to_embed()} def get_properties_to_embed(self) -> List[str]: - result = ["*"] + result = [custom_embed.EMBED_ALL_FIELDS_MARKER] for pipeline_property in self.get_pipeline_properties(): result.extend( self.get_properties_to_embed_from_pipeline_property(pipeline_property) @@ -132,25 +135,25 @@ def get_properties_to_embed(self) -> List[str]: return result def get_pipeline_properties(self) -> List[str]: - return getattr(self.context, self.PIPELINE_PROPERTIES, []) + return getattr(self.context, PIPELINE_PROPERTIES, []) def get_properties_to_embed_from_pipeline_property( self, pipeline_property: str ) -> List[str]: - split_properties = [ - term - for term in pipeline_property.split(custom_embed.PROPERTY_SPLITTER) - if term + split_properties = self.split_pipeline_property(pipeline_property) + properties_to_embed = self.get_all_possible_embeds(split_properties) + return [ + self.make_embed_property(property_to_embed) + for property_to_embed in properties_to_embed ] + + def get_all_possible_embeds(self, split_properties: List[str]) -> List[str]: return [ - self.make_embed_property( - custom_embed.PROPERTY_SPLITTER.join(split_properties[: idx + 1]) - ) + custom_embed.PROPERTY_SPLITTER.join(split_properties[:idx + 1]) for idx in range(len(split_properties)) ] - @staticmethod - def make_embed_property(property_to_embed: str) -> str: + def make_embed_property(self, property_to_embed: str) -> str: return ( property_to_embed + custom_embed.PROPERTY_SPLITTER