From 1248492c1d873547c23dc1bc86cc0e536d0b49db Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 15 Aug 2024 18:27:54 +0200 Subject: [PATCH 01/18] Init Visualizations FastAPI --- lib/galaxy/schema/visualization.py | 79 ++++ .../webapps/galaxy/api/visualizations.py | 228 +++------- lib/galaxy/webapps/galaxy/buildapp.py | 1 - .../webapps/galaxy/services/visualizations.py | 389 ++++++++++++++++++ 4 files changed, 514 insertions(+), 183 deletions(-) diff --git a/lib/galaxy/schema/visualization.py b/lib/galaxy/schema/visualization.py index 0488bf249c9e..8f6316701b73 100644 --- a/lib/galaxy/schema/visualization.py +++ b/lib/galaxy/schema/visualization.py @@ -100,3 +100,82 @@ class VisualizationSummaryList(RootModel): default=[], title="List with detailed information of Visualizations.", ) + +class VisualizationShow(RootModel): + model_class: str = Field( + "Visualization", + title="Model Class", + description="The model class name for this object.", + ) + id: EncodedDatabaseIdField = Field( + ..., + title="ID", + description="Encoded ID of the Visualization.", + ) + title: str = Field( + ..., + title="Title", + description="The name of the visualization.", + ) + type: str = Field( + ..., + title="Type", + description="The type of the visualization.", + ) + user_id: DecodedDatabaseIdField = Field( + ..., + title="User ID", + description="The ID of the user owning this Visualization.", + ) + dbkey: Optional[str] = Field( + default=None, + title="DbKey", + description="The database key of the visualization.", + ) + slug: Optional[str] = Field( + default=None, + title="Slug", + description="The slug of the visualization.", + ) + # should be a VisualizationRevision Model + latest_revision: Model = Field( + ..., + title="Latest Revision", + description="The latest revision of this Visualization.", + ) + revisions: List[EncodedDatabaseIdField] = Field( + default=[], + title="Revisions", + description="A list of encoded IDs of the revisions of this Visualization.", + ) + url: Optional[str] = Field( + default=None, + title="URL", + description="The URL of the visualization.", + ) + username: str = Field( + ..., + title="Username", + description="The name of the user owning this Visualization.", + ) + email_hash: Optional[str] = Field( + default=None, + title="Email Hash", + description="The hash of the email of the user owning this Visualization.", + ) + tags: TagCollection = Field( + default=[], + title="Tags", + description="A list of tags to add to this item.", + ) + annotation: Optional[str] = Field( + default=None, + title="Annotation", + description="The annotation of this Visualization.", + ) + # should be a Plugin Model + plugin: Model = Field( + ..., + title="Plugin", + description="The plugin of this Visualization.", + ) diff --git a/lib/galaxy/webapps/galaxy/api/visualizations.py b/lib/galaxy/webapps/galaxy/api/visualizations.py index 2e8283b03dcd..65f5b40c4edf 100644 --- a/lib/galaxy/webapps/galaxy/api/visualizations.py +++ b/lib/galaxy/webapps/galaxy/api/visualizations.py @@ -5,7 +5,6 @@ may change often. """ -import json import logging from typing import Optional @@ -18,14 +17,7 @@ ) from typing_extensions import Annotated -from galaxy import ( - exceptions, - util, - web, -) from galaxy.managers.context import ProvidesUserContext -from galaxy.model.base import transaction -from galaxy.model.item_attrs import UsesAnnotations from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.schema import ( SetSlugPayload, @@ -36,14 +28,10 @@ from galaxy.schema.visualization import ( VisualizationIndexQueryPayload, VisualizationSortByEnum, + VisualizationShow, VisualizationSummaryList, ) -from galaxy.util.hash_util import md5_hash_str -from galaxy.web import expose_api -from galaxy.webapps.base.controller import UsesVisualizationMixin -from galaxy.webapps.base.webapp import GalaxyWebTransaction from galaxy.webapps.galaxy.api import ( - BaseGalaxyAPIController, depends, DependsOnTrans, IndexQueryTag, @@ -239,175 +227,51 @@ def set_slug( self.service.shareable_service.set_slug(trans, id, payload) return Response(status_code=status.HTTP_204_NO_CONTENT) + @router.get( + "/api/visualizations/{id}", + summary="Get a visualization by ID.", + ) + def show( + self, + id: VisualizationIdPathParam, + trans: ProvidesUserContext = DependsOnTrans, + ) -> VisualizationShow: + """Return the visualization.""" + return self.service.show(trans, id) -class VisualizationsController(BaseGalaxyAPIController, UsesVisualizationMixin, UsesAnnotations): - """ - RESTful controller for interactions with visualizations. - """ + @router.post( + "/api/visualizations", + summary="Create a new visualization.", + ) + def create( + self, + trans: ProvidesUserContext = DependsOnTrans, + ) -> VisualizationShow: + payload = {} + return self.service.create(trans, payload) - service: VisualizationsService = depends(VisualizationsService) + @router.post( + "/api/visualizations?import_id={id}", + summary="Import a visualization.", + ) + def create_import( + self, + id: VisualizationIdPathParam, + trans: ProvidesUserContext = DependsOnTrans, + ) -> VisualizationShow: + return self.service.create(trans, id) - @expose_api - def show(self, trans: GalaxyWebTransaction, id: str, **kwargs): - """ - GET /api/visualizations/{viz_id} - """ - # TODO: revisions should be a contents/nested controller like viz/xxx/r/xxx)? - # the important thing is the config - # TODO:?? /api/visualizations/registry -> json of registry.listings? - visualization = self.get_visualization(trans, id, check_ownership=False, check_accessible=True) - dictionary = trans.security.encode_dict_ids(self.get_visualization_dict(visualization)) - dictionary["url"] = web.url_for( - controller="visualization", - action="display_by_username_and_slug", - username=visualization.user.username, - slug=visualization.slug, - ) - dictionary["username"] = visualization.user.username - dictionary["email_hash"] = md5_hash_str(visualization.user.email) - dictionary["tags"] = visualization.make_tag_string_list() - dictionary["annotation"] = self.get_item_annotation_str(trans.sa_session, trans.user, visualization) - # need to encode ids in revisions as well - encoded_revisions = [] - for revision in dictionary["revisions"]: - # NOTE: does not encode ids inside the configs - encoded_revisions.append(trans.security.encode_id(revision)) - dictionary["revisions"] = encoded_revisions - dictionary["latest_revision"] = trans.security.encode_dict_ids(dictionary["latest_revision"]) - if trans.app.visualizations_registry: - visualization = trans.app.visualizations_registry.get_plugin(dictionary["type"]) - dictionary["plugin"] = visualization.to_dict() - return dictionary - - @expose_api - def create(self, trans: GalaxyWebTransaction, payload: dict, **kwargs): - """ - POST /api/visualizations - creates a new visualization using the given payload - - POST /api/visualizations?import_id={encoded_visualization_id} - imports a copy of an existing visualization into the user's workspace - """ - rval = None - - if "import_id" in payload: - import_id = payload["import_id"] - visualization = self.import_visualization(trans, import_id, user=trans.user) - - else: - payload = self._validate_and_parse_payload(payload) - # must have a type (I've taken this to be the visualization name) - if "type" not in payload: - raise exceptions.RequestParameterMissingException("key/value 'type' is required") - vis_type = payload.pop("type", False) - - payload["save"] = True - try: - # generate defaults - this will err if given a weird key? - visualization = self.create_visualization(trans, vis_type, **payload) - except ValueError as val_err: - raise exceptions.RequestParameterMissingException(str(val_err)) - - rval = {"id": trans.security.encode_id(visualization.id)} - - return rval - - @expose_api - def update(self, trans: GalaxyWebTransaction, id: str, payload: dict, **kwargs): - """ - PUT /api/visualizations/{encoded_visualization_id} - """ - rval = None - payload = self._validate_and_parse_payload(payload) - - # there's a differentiation here between updating the visualization and creating a new revision - # that needs to be handled clearly here or alternately, using a different controller - # like e.g. PUT /api/visualizations/{id}/r/{id} - - # TODO: consider allowing direct alteration of revisions title (without a new revision) - # only create a new revsion on a different config - - # only update owned visualizations - visualization = self.get_visualization(trans, id, check_ownership=True) - title = payload.get("title", visualization.latest_revision.title) - dbkey = payload.get("dbkey", visualization.latest_revision.dbkey) - deleted = payload.get("deleted", visualization.deleted) - config = payload.get("config", visualization.latest_revision.config) - - latest_config = visualization.latest_revision.config - if ( - (title != visualization.latest_revision.title) - or (dbkey != visualization.latest_revision.dbkey) - or (json.dumps(config) != json.dumps(latest_config)) - ): - revision = self.add_visualization_revision(trans, visualization, config, title, dbkey) - rval = {"id": id, "revision": revision.id} - - # allow updating vis title - visualization.title = title - visualization.deleted = deleted - with transaction(trans.sa_session): - trans.sa_session.commit() - - return rval - - def _validate_and_parse_payload(self, payload): - """ - Validate and parse incomming data payload for a visualization. - """ - # This layer handles (most of the stricter idiot proofing): - # - unknown/unallowed keys - # - changing data keys from api key to attribute name - # - protection against bad data form/type - # - protection against malicious data content - # all other conversions and processing (such as permissions, etc.) should happen down the line - - # keys listed here don't error when attempting to set, but fail silently - # this allows PUT'ing an entire model back to the server without attribute errors on uneditable attrs - valid_but_uneditable_keys = ( - "id", - "model_class", - # TODO: fill out when we create to_dict, get_dict, whatevs - ) - # TODO: importable - ValidationError = exceptions.RequestParameterInvalidException - - validated_payload = {} - for key, val in payload.items(): - # TODO: validate types in VALID_TYPES/registry names at the mixin/model level? - if key == "type": - if not isinstance(val, str): - raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") - val = util.sanitize_html.sanitize_html(val) - elif key == "config": - if not isinstance(val, dict): - raise ValidationError(f"{key} must be a dictionary: {str(type(val))}") - elif key == "annotation": - if not isinstance(val, str): - raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") - val = util.sanitize_html.sanitize_html(val) - elif key == "deleted": - if not isinstance(val, bool): - raise ValidationError(f"{key} must be a bool: {str(type(val))}") - - # these are keys that actually only be *updated* at the revision level and not here - # (they are still valid for create, tho) - elif key == "title": - if not isinstance(val, str): - raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") - val = util.sanitize_html.sanitize_html(val) - elif key == "slug": - if not isinstance(val, str): - raise ValidationError(f"{key} must be a string: {str(type(val))}") - val = util.sanitize_html.sanitize_html(val) - elif key == "dbkey": - if not isinstance(val, str): - raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") - val = util.sanitize_html.sanitize_html(val) - - elif key not in valid_but_uneditable_keys: - continue - # raise AttributeError( 'unknown key: %s' %( str( key ) ) ) - - validated_payload[key] = val - return validated_payload + @router.put( + "/api/visualizations/{id}", + summary="Update a visualization.", + ) + def update( + self, + id: VisualizationIdPathParam, + trans: ProvidesUserContext = DependsOnTrans, + ) -> VisualizationShow: + payload = {} + return self.service.update(trans, id, payload) + +# schema and payloads needs to be configured right +# check for the correct fields and types \ No newline at end of file diff --git a/lib/galaxy/webapps/galaxy/buildapp.py b/lib/galaxy/webapps/galaxy/buildapp.py index 8b13885b45d6..26d3d1c1f411 100644 --- a/lib/galaxy/webapps/galaxy/buildapp.py +++ b/lib/galaxy/webapps/galaxy/buildapp.py @@ -588,7 +588,6 @@ def populate_api_routes(webapp, app): conditions=dict(method=["POST"]), ) - webapp.mapper.resource("visualization", "visualizations", path_prefix="/api") webapp.mapper.resource("plugins", "plugins", path_prefix="/api") webapp.mapper.connect("/api/workflows/build_module", action="build_module", controller="workflows") webapp.mapper.connect( diff --git a/lib/galaxy/webapps/galaxy/services/visualizations.py b/lib/galaxy/webapps/galaxy/services/visualizations.py index 0138de78d9ff..c33ea2f07ab6 100644 --- a/lib/galaxy/webapps/galaxy/services/visualizations.py +++ b/lib/galaxy/webapps/galaxy/services/visualizations.py @@ -1,16 +1,36 @@ +import json import logging from typing import Tuple from galaxy import exceptions +from galaxy.managers.base import ( + is_valid_slug, + security_check, +) +from galaxy.managers.context import ProvidesAppContext +from galaxy.managers.sharable import ( + slug_exists, + SlugBuilder, +) from galaxy.managers.visualizations import ( VisualizationManager, VisualizationSerializer, ) +from galaxy.model import Visualization, VisualizationRevision +from galaxy.model.base import transaction +from galaxy.model.item_attrs import ( + add_item_annotation, + get_item_annotation_str, +) +from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.visualization import ( VisualizationIndexQueryPayload, + VisualizationShow, VisualizationSummaryList, ) from galaxy.security.idencoding import IdEncodingHelper +from galaxy.util.hash_util import md5_hash_str +from galaxy.util.sanitize_html import sanitize_html from galaxy.webapps.galaxy.services.base import ServiceBase from galaxy.webapps.galaxy.services.notifications import NotificationService from galaxy.webapps.galaxy.services.sharable import ShareableService @@ -60,3 +80,372 @@ def index( VisualizationSummaryList(root=[entry.to_dict() for entry in entries]), total_matches, ) + + def show( + self, + trans: ProvidesAppContext, + visualization_id: DecodedDatabaseIdField, + ) -> VisualizationShow: + """Return a dictionary containing the Visualization's details + + :rtype: dictionary + :returns: Visualization details + """ + # TODO: revisions should be a contents/nested controller like viz/xxx/r/xxx)? + # the important thing is the config + # TODO:?? /api/visualizations/registry -> json of registry.listings? + visualization = self._get_visualization(trans, visualization_id, check_ownership=False, check_accessible=True) + dictionary = trans.security.encode_dict_ids(self._get_visualization_dict(visualization)) + dictionary["url"] = trans.url_builder( + controller="show_group", + action="display_by_username_and_slug", + username=visualization.user.username, + slug=visualization.slug, + ) + dictionary["username"] = visualization.user.username + dictionary["email_hash"] = md5_hash_str(visualization.user.email) + dictionary["tags"] = visualization.make_tag_string_list() + dictionary["annotation"] = get_item_annotation_str(trans.sa_session, trans.user, visualization) + # need to encode ids in revisions as well + encoded_revisions = [] + for revision in dictionary["revisions"]: + # NOTE: does not encode ids inside the configs + encoded_revisions.append(trans.security.encode_id(revision)) + dictionary["revisions"] = encoded_revisions + dictionary["latest_revision"] = trans.security.encode_dict_ids(dictionary["latest_revision"]) + if trans.app.visualizations_registry: + visualization = trans.app.visualizations_registry.get_plugin(dictionary["type"]) + dictionary["plugin"] = visualization.to_dict() + return dictionary + + def create( + self, + trans: ProvidesAppContext, + payload: VisualizationIndexQueryPayload, + ) -> VisualizationShow: + """Returns a dictionary of the created visualization + + :rtype: dictionary + :returns: dictionary containing Visualization details + """ + payload = self._validate_and_parse_payload(payload) + # must have a type (I've taken this to be the visualization name) + if "type" not in payload: + raise exceptions.RequestParameterMissingException("key/value 'type' is required") + type = payload.pop("type", False) + + payload["save"] = True + title = payload.get("title", "Untitled Visualization") + slug = payload.get("slug", None) + dbkey = payload.get("dbkey", None) + annotation = payload.get("annotation", None) + config = payload.get("config", {}) + save = payload.get("save", True) + + # generate defaults - this will err if given a weird key? + visualization = self._create_visualization(trans, title, type, dbkey, slug, annotation, save) + # TODO: handle this error structure better either in _create or here + if isinstance(visualization, dict): + err_dict = visualization + val_err = str(err_dict["title_err"] or err_dict["slug_err"]) + raise exceptions.RequestParameterMissingException(val_err) + + # Create and save first visualization revision + revision = trans.model.VisualizationRevision( + visualization=visualization, title=title, config=config, dbkey=dbkey + ) + visualization.latest_revision = revision + + if save: + session = trans.sa_session + session.add(revision) + with transaction(session): + session.commit() + + rval = {"id": trans.security.encode_id(visualization.id)} + + return rval + + def import_visualization( + self, + trans: ProvidesAppContext, + visualization_id: DecodedDatabaseIdField, + ) -> VisualizationShow: + """ + Returns a dictionary of the created visualization + + :rtype: dictionary + :returns: dictionary containing Visualization details + + Copy the visualization with the given id and associate the copy + with the given user (defaults to trans.user). + + Raises `ItemAccessibilityException` if `user` is not passed and + the current user is anonymous, and if the visualization is not `importable`. + Raises `ItemDeletionException` if the visualization has been deleted. + """ + # default to trans.user, error if anon + if not trans.user: + raise exceptions.ItemAccessibilityException("You must be logged in to import Galaxy visualizations") + user = trans.user + + # check accessibility + visualization = self._get_visualization(trans, visualization_id, check_ownership=False) + if not visualization.importable: + raise exceptions.ItemAccessibilityException( + "The owner of this visualization has disabled imports via this link." + ) + if visualization.deleted: + raise exceptions.ItemDeletionException("You can't import this visualization because it has been deleted.") + + # copy vis and alter title + # TODO: need to handle custom db keys. + imported_visualization = visualization.copy(user=user, title=f"imported: {visualization.title}") + trans.sa_session.add(imported_visualization) + with transaction(trans.sa_session): + trans.sa_session.commit() + + rval = {"id": trans.security.encode_id(imported_visualization.id)} + + return rval + + def update( + self, + trans: ProvidesAppContext, + visualization_id: DecodedDatabaseIdField, + payload: VisualizationIndexQueryPayload, + ) -> VisualizationShow: + """ + Update a visualization + + :rtype: dictionary + :returns: dictionary containing Visualization details + """ + rval = None + payload = self._validate_and_parse_payload(payload) + + # there's a differentiation here between updating the visualization and creating a new revision + # that needs to be handled clearly here or alternately, using a different controller + # like e.g. PUT /api/visualizations/{visualization_id}/r/{revision_id} + + # TODO: consider allowing direct alteration of revisions title (without a new revision) + # only create a new revsion on a different config + + # only update owned visualizations + visualization = self._get_visualization(trans, visualization_id, check_ownership=True) + title = payload.get("title", visualization.latest_revision.title) + dbkey = payload.get("dbkey", visualization.latest_revision.dbkey) + deleted = payload.get("deleted", visualization.deleted) + config = payload.get("config", visualization.latest_revision.config) + + latest_config = visualization.latest_revision.config + if ( + (title != visualization.latest_revision.title) + or (dbkey != visualization.latest_revision.dbkey) + or (json.dumps(config) != json.dumps(latest_config)) + ): + revision = self._add_visualization_revision(trans, visualization, config, title, dbkey) + rval = {"id": visualization_id, "revision": revision.id} + + # allow updating vis title + visualization.title = title + visualization.deleted = deleted + with transaction(trans.sa_session): + trans.sa_session.commit() + + return rval + + def _validate_and_parse_payload( + self, + payload: VisualizationIndexQueryPayload, + ) -> VisualizationShow: + """ + Validate and parse incomming data payload for a visualization. + """ + # This layer handles (most of the stricter idiot proofing): + # - unknown/unallowed keys + # - changing data keys from api key to attribute name + # - protection against bad data form/type + # - protection against malicious data content + # all other conversions and processing (such as permissions, etc.) should happen down the line + + # keys listed here don't error when attempting to set, but fail silently + # this allows PUT'ing an entire model back to the server without attribute errors on uneditable attrs + valid_but_uneditable_keys = ( + "id", + "model_class", + # TODO: fill out when we create to_dict, get_dict, whatevs + ) + # TODO: importable + ValidationError = exceptions.RequestParameterInvalidException + + validated_payload = {} + for key, val in payload.items(): + # TODO: validate types in VALID_TYPES/registry names at the mixin/model level? + if key == "type": + if not isinstance(val, str): + raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") + val = sanitize_html(val) + elif key == "config": + if not isinstance(val, dict): + raise ValidationError(f"{key} must be a dictionary: {str(type(val))}") + elif key == "annotation": + if not isinstance(val, str): + raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") + val = sanitize_html(val) + elif key == "deleted": + if not isinstance(val, bool): + raise ValidationError(f"{key} must be a bool: {str(type(val))}") + + # these are keys that actually only be *updated* at the revision level and not here + # (they are still valid for create, tho) + elif key == "title": + if not isinstance(val, str): + raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") + val = sanitize_html(val) + elif key == "slug": + if not isinstance(val, str): + raise ValidationError(f"{key} must be a string: {str(type(val))}") + val = sanitize_html(val) + elif key == "dbkey": + if not isinstance(val, str): + raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") + val = sanitize_html(val) + + elif key not in valid_but_uneditable_keys: + continue + # raise AttributeError( 'unknown key: %s' %( str( key ) ) ) + + validated_payload[key] = val + return validated_payload + + def _get_visualization( + self, + trans: ProvidesAppContext, + visualization_id: DecodedDatabaseIdField, + check_ownership=True, + check_accessible=False, + ) -> Visualization: + """ + Get a Visualization from the database by id, verifying ownership. + """ + # Load workflow from database + try: + visualization = trans.sa_session.get(Visualization, trans.security.decode_id(visualization_id)) + except TypeError: + visualization = None + if not visualization: + raise exceptions.ObjectNotFound("Visualization not found") + else: + return security_check(trans, visualization, check_ownership, check_accessible) + + def _get_visualization_dict( + self, + visualization: Visualization, + ) -> dict: + """ + Return a set of detailed attributes for a visualization in dictionary form. + The visualization's latest_revision is returned in its own sub-dictionary. + NOTE: that encoding ids isn't done here should happen at the caller level. + """ + return { + "model_class": "Visualization", + "id": visualization.id, + "title": visualization.title, + "type": visualization.type, + "user_id": visualization.user.id, + "dbkey": visualization.dbkey, + "slug": visualization.slug, + # to_dict only the latest revision (allow older to be fetched elsewhere) + "latest_revision": self._get_visualization_revision_dict(visualization.latest_revision), + "revisions": [r.id for r in visualization.revisions], + } + + def _get_visualization_revision_dict( + self, + revision: VisualizationRevision, + ) -> dict: + """ + Return a set of detailed attributes for a visualization in dictionary form. + NOTE: that encoding ids isn't done here should happen at the caller level. + """ + return { + "model_class": "VisualizationRevision", + "id": revision.id, + "visualization_id": revision.visualization.id, + "title": revision.title, + "dbkey": revision.dbkey, + "config": revision.config, + } + + def _add_visualization_revision( + self, + trans: ProvidesAppContext, + visualization: Visualization, + config: dict, + title: str, + dbkey: str, + ) -> VisualizationRevision: + """ + Adds a new `VisualizationRevision` to the given `visualization` with + the given parameters and set its parent visualization's `latest_revision` + to the new revision. + """ + # precondition: only add new revision on owned vis's + # TODO:?? should we default title, dbkey, config? to which: visualization or latest_revision? + revision = trans.model.VisualizationRevision( + visualization=visualization, title=title, dbkey=dbkey, config=config + ) + + visualization.latest_revision = revision + # TODO:?? does this automatically add revision to visualzation.revisions? + trans.sa_session.add(revision) + with transaction(trans.sa_session): + trans.sa_session.commit() + return revision + + def _create_visualization( + self, + trans: ProvidesAppContext, + title: str, + type: str, + dbkey: str = None, + slug: str = None, + annotation: str = None, + save: bool = True, + ) -> Visualization: + """Create visualization but not first revision. Returns Visualization object.""" + user = trans.get_user() + + # Error checking. + title_err = slug_err = "" + if not title: + title_err = "visualization name is required" + elif slug and not is_valid_slug(slug): + slug_err = "visualization identifier must consist of only lowercase letters, numbers, and the '-' character" + elif slug and slug_exists(trans.sa_session, trans.model.Visualization, user, slug, ignore_deleted=True): + slug_err = "visualization identifier must be unique" + + if title_err or slug_err: + return {"title_err": title_err, "slug_err": slug_err} + + # Create visualization + visualization = trans.model.Visualization(user=user, title=title, dbkey=dbkey, type=type) + if slug: + visualization.slug = slug + else: + slug_builder = SlugBuilder() + slug_builder.create_item_slug(trans.sa_session, visualization) + if annotation: + annotation = sanitize_html(annotation) + # TODO: if this is to stay in the mixin, UsesAnnotations should be added to the superclasses + # right now this is depending on the classes that include this mixin to have UsesAnnotations + add_item_annotation(trans.sa_session, trans.user, visualization, annotation) + + if save: + session = trans.sa_session + session.add(visualization) + with transaction(session): + session.commit() + + return visualization From 4ba092ef53ca733a85b47420dde93fb90d7e077f Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 16 Aug 2024 12:50:50 +0200 Subject: [PATCH 02/18] Add Visualization schema and payloads & joining create and import together again --- lib/galaxy/schema/visualization.py | 106 ++++++++++++- .../webapps/galaxy/api/visualizations.py | 46 ++++-- .../webapps/galaxy/services/visualizations.py | 146 +++++++++--------- 3 files changed, 201 insertions(+), 97 deletions(-) diff --git a/lib/galaxy/schema/visualization.py b/lib/galaxy/schema/visualization.py index 8f6316701b73..a1920856badc 100644 --- a/lib/galaxy/schema/visualization.py +++ b/lib/galaxy/schema/visualization.py @@ -1,5 +1,6 @@ from datetime import datetime from typing import ( + Dict, List, Optional, ) @@ -101,7 +102,104 @@ class VisualizationSummaryList(RootModel): title="List with detailed information of Visualizations.", ) -class VisualizationShow(RootModel): + +class VisualizationRevision(Model): + model_class: str = Field( + "VisualizationRevision", + title="Model Class", + description="The model class name for this object.", + ) + id: EncodedDatabaseIdField = Field( + ..., + title="ID", + description="Encoded ID of the Visualization Revision.", + ) + visualization_id: EncodedDatabaseIdField = Field( + ..., + title="Visualization ID", + description="Encoded ID of the Visualization.", + ) + title: str = Field( + ..., + title="Title", + description="The name of the visualization revision.", + ) + dbkey: Optional[str] = Field( + default=None, + title="DbKey", + description="The database key of the visualization.", + ) + config: Dict = Field( + ..., + title="Config", + description="The config of the visualization revision.", + ) + + +class VisualizationPlugin(Model): + name: str = Field( + ..., + title="Name", + description="The name of the plugin.", + ) + html: str = Field( + ..., + title="HTML", + description="The HTML of the plugin.", + ) + description: str = Field( + ..., + title="Description", + description="The description of the plugin.", + ) + logo: str = Field( + ..., + title="Logo", + description="The logo of the plugin.", + ) + title: Optional[str] = Field( + default=None, + title="Title", + description="The title of the plugin.", + ) + target: str = Field( + ..., + title="Target", + description="The target of the plugin.", + ) + embeddable: bool = Field( + ..., + title="Embeddable", + description="Whether the plugin is embeddable.", + ) + entry_point: Dict = Field( + ..., + title="Entry Point", + description="The entry point of the plugin.", + ) + settings: List[Dict] = Field( + ..., + title="Settings", + description="The settings of the plugin.", + ) + groups: List[Dict] = Field( + ..., + title="Groups", + description="The groups of the plugin.", + ) + specs: Dict = Field( + ..., + title="Specs", + description="The specs of the plugin.", + ) + href: str = Field( + ..., + title="Href", + description="The href of the plugin.", + ) + + +class VisualizationShow(Model): model_class: str = Field( "Visualization", title="Model Class", @@ -137,8 +235,7 @@ class VisualizationShow(RootModel): title="Slug", description="The slug of the visualization.", ) - # should be a VisualizationRevision Model - latest_revision: Model = Field( + latest_revision: VisualizationRevision = Field( ..., title="Latest Revision", description="The latest revision of this Visualization.", @@ -173,8 +270,7 @@ class VisualizationShow(RootModel): title="Annotation", description="The annotation of this Visualization.", ) - # should be a Plugin Model - plugin: Model = Field( + plugin: VisualizationPlugin = Field( ..., title="Plugin", description="The plugin of this Visualization.", diff --git a/lib/galaxy/webapps/galaxy/api/visualizations.py b/lib/galaxy/webapps/galaxy/api/visualizations.py index 65f5b40c4edf..eb80358a2b0e 100644 --- a/lib/galaxy/webapps/galaxy/api/visualizations.py +++ b/lib/galaxy/webapps/galaxy/api/visualizations.py @@ -27,8 +27,8 @@ ) from galaxy.schema.visualization import ( VisualizationIndexQueryPayload, - VisualizationSortByEnum, VisualizationShow, + VisualizationSortByEnum, VisualizationSummaryList, ) from galaxy.webapps.galaxy.api import ( @@ -245,22 +245,28 @@ def show( ) def create( self, + import_id: Optional[VisualizationIdPathParam] = None, + type: str = Body(...), + title: str = Body(...), + dbkey: Optional[str] = Body(None), + slug: Optional[str] = Body(None), + annotation: Optional[str] = Body(None), + config: Optional[dict] = Body(None), + save: bool = Body(True), trans: ProvidesUserContext = DependsOnTrans, ) -> VisualizationShow: - payload = {} + payload = { + "import_id": import_id, + "type": type, + "title": title, + "dbkey": dbkey, + "slug": slug, + "annotation": annotation, + "config": config, + "save": save, + } return self.service.create(trans, payload) - @router.post( - "/api/visualizations?import_id={id}", - summary="Import a visualization.", - ) - def create_import( - self, - id: VisualizationIdPathParam, - trans: ProvidesUserContext = DependsOnTrans, - ) -> VisualizationShow: - return self.service.create(trans, id) - @router.put( "/api/visualizations/{id}", summary="Update a visualization.", @@ -268,10 +274,16 @@ def create_import( def update( self, id: VisualizationIdPathParam, + title: Optional[str] = Body(None), + dbkey: Optional[str] = Body(None), + deleted: Optional[bool] = Body(None), + config: Optional[dict] = Body(None), trans: ProvidesUserContext = DependsOnTrans, ) -> VisualizationShow: - payload = {} + payload = { + "type": type, + "title": title, + "dbkey": dbkey, + "config": config, + } return self.service.update(trans, id, payload) - -# schema and payloads needs to be configured right -# check for the correct fields and types \ No newline at end of file diff --git a/lib/galaxy/webapps/galaxy/services/visualizations.py b/lib/galaxy/webapps/galaxy/services/visualizations.py index c33ea2f07ab6..566988e7d615 100644 --- a/lib/galaxy/webapps/galaxy/services/visualizations.py +++ b/lib/galaxy/webapps/galaxy/services/visualizations.py @@ -16,7 +16,10 @@ VisualizationManager, VisualizationSerializer, ) -from galaxy.model import Visualization, VisualizationRevision +from galaxy.model import ( + Visualization, + VisualizationRevision, +) from galaxy.model.base import transaction from galaxy.model.item_attrs import ( add_item_annotation, @@ -128,84 +131,45 @@ def create( :rtype: dictionary :returns: dictionary containing Visualization details """ - payload = self._validate_and_parse_payload(payload) - # must have a type (I've taken this to be the visualization name) - if "type" not in payload: - raise exceptions.RequestParameterMissingException("key/value 'type' is required") - type = payload.pop("type", False) - - payload["save"] = True - title = payload.get("title", "Untitled Visualization") - slug = payload.get("slug", None) - dbkey = payload.get("dbkey", None) - annotation = payload.get("annotation", None) - config = payload.get("config", {}) - save = payload.get("save", True) - - # generate defaults - this will err if given a weird key? - visualization = self._create_visualization(trans, title, type, dbkey, slug, annotation, save) - # TODO: handle this error structure better either in _create or here - if isinstance(visualization, dict): - err_dict = visualization - val_err = str(err_dict["title_err"] or err_dict["slug_err"]) - raise exceptions.RequestParameterMissingException(val_err) - - # Create and save first visualization revision - revision = trans.model.VisualizationRevision( - visualization=visualization, title=title, config=config, dbkey=dbkey - ) - visualization.latest_revision = revision - - if save: - session = trans.sa_session - session.add(revision) - with transaction(session): - session.commit() - - rval = {"id": trans.security.encode_id(visualization.id)} - - return rval - - def import_visualization( - self, - trans: ProvidesAppContext, - visualization_id: DecodedDatabaseIdField, - ) -> VisualizationShow: - """ - Returns a dictionary of the created visualization - - :rtype: dictionary - :returns: dictionary containing Visualization details - - Copy the visualization with the given id and associate the copy - with the given user (defaults to trans.user). - - Raises `ItemAccessibilityException` if `user` is not passed and - the current user is anonymous, and if the visualization is not `importable`. - Raises `ItemDeletionException` if the visualization has been deleted. - """ - # default to trans.user, error if anon - if not trans.user: - raise exceptions.ItemAccessibilityException("You must be logged in to import Galaxy visualizations") - user = trans.user + rval = None - # check accessibility - visualization = self._get_visualization(trans, visualization_id, check_ownership=False) - if not visualization.importable: - raise exceptions.ItemAccessibilityException( - "The owner of this visualization has disabled imports via this link." + if "import_id" in payload: + import_id = payload["import_id"] + visualization = self._import_visualization(trans, import_id, user=trans.user) + else: + payload = self._validate_and_parse_payload(payload) + # must have a type (I've taken this to be the visualization name) + if "type" not in payload: + raise exceptions.RequestParameterMissingException("key/value 'type' is required") + type = payload.pop("type", False) + title = payload.get("title", "Untitled Visualization") + slug = payload.get("slug", None) + dbkey = payload.get("dbkey", None) + annotation = payload.get("annotation", None) + config = payload.get("config", {}) + save = payload.get("save", True) + + # generate defaults - this will err if given a weird key? + visualization = self._create_visualization(trans, title, type, dbkey, slug, annotation, save) + # TODO: handle this error structure better either in _create or here + if isinstance(visualization, dict): + err_dict = visualization + val_err = str(err_dict["title_err"] or err_dict["slug_err"]) + raise exceptions.RequestParameterMissingException(val_err) + + # Create and save first visualization revision + revision = trans.model.VisualizationRevision( + visualization=visualization, title=title, config=config, dbkey=dbkey ) - if visualization.deleted: - raise exceptions.ItemDeletionException("You can't import this visualization because it has been deleted.") + visualization.latest_revision = revision - # copy vis and alter title - # TODO: need to handle custom db keys. - imported_visualization = visualization.copy(user=user, title=f"imported: {visualization.title}") - trans.sa_session.add(imported_visualization) - with transaction(trans.sa_session): - trans.sa_session.commit() + if save: + session = trans.sa_session + session.add(revision) + with transaction(session): + session.commit() - rval = {"id": trans.security.encode_id(imported_visualization.id)} + rval = {"id": trans.security.encode_id(visualization.id)} return rval @@ -449,3 +413,35 @@ def _create_visualization( session.commit() return visualization + + def _import_visualization(self, trans, id, user=None): + """ + Copy the visualization with the given id and associate the copy + with the given user (defaults to trans.user). + + Raises `ItemAccessibilityException` if `user` is not passed and + the current user is anonymous, and if the visualization is not `importable`. + Raises `ItemDeletionException` if the visualization has been deleted. + """ + # default to trans.user, error if anon + if not user: + if not trans.user: + raise exceptions.ItemAccessibilityException("You must be logged in to import Galaxy visualizations") + user = trans.user + + # check accessibility + visualization = self.get_visualization(trans, id, check_ownership=False) + if not visualization.importable: + raise exceptions.ItemAccessibilityException( + "The owner of this visualization has disabled imports via this link." + ) + if visualization.deleted: + raise exceptions.ItemDeletionException("You can't import this visualization because it has been deleted.") + + # copy vis and alter title + # TODO: need to handle custom db keys. + imported_visualization = visualization.copy(user=user, title=f"imported: {visualization.title}") + trans.sa_session.add(imported_visualization) + with transaction(trans.sa_session): + trans.sa_session.commit() + return imported_visualization From 544421b047903ee6ee98104d8635c5d25890ac41 Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 16 Aug 2024 12:56:21 +0200 Subject: [PATCH 03/18] Fix missed payloads --- lib/galaxy/webapps/galaxy/api/visualizations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/api/visualizations.py b/lib/galaxy/webapps/galaxy/api/visualizations.py index eb80358a2b0e..6c21c413ab0e 100644 --- a/lib/galaxy/webapps/galaxy/api/visualizations.py +++ b/lib/galaxy/webapps/galaxy/api/visualizations.py @@ -281,9 +281,9 @@ def update( trans: ProvidesUserContext = DependsOnTrans, ) -> VisualizationShow: payload = { - "type": type, "title": title, "dbkey": dbkey, + "deleted": deleted, "config": config, } return self.service.update(trans, id, payload) From be2e56ab8c861e503a6ffea218977451f4fb618d Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 16 Aug 2024 12:58:46 +0200 Subject: [PATCH 04/18] Update import_id parameter in create method of FastAPIVisualizations --- lib/galaxy/webapps/galaxy/api/visualizations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/api/visualizations.py b/lib/galaxy/webapps/galaxy/api/visualizations.py index 6c21c413ab0e..f3f485572a79 100644 --- a/lib/galaxy/webapps/galaxy/api/visualizations.py +++ b/lib/galaxy/webapps/galaxy/api/visualizations.py @@ -245,7 +245,7 @@ def show( ) def create( self, - import_id: Optional[VisualizationIdPathParam] = None, + import_id: Optional[VisualizationIdPathParam] = Body(None), type: str = Body(...), title: str = Body(...), dbkey: Optional[str] = Body(None), From 2880a296318d84cd685c7ec92dd60dccc3b35e69 Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 16 Aug 2024 14:00:11 +0200 Subject: [PATCH 05/18] Remove the old Visualization mixin controller --- lib/galaxy/webapps/base/controller.py | 171 -------------------------- 1 file changed, 171 deletions(-) diff --git a/lib/galaxy/webapps/base/controller.py b/lib/galaxy/webapps/base/controller.py index 83c3ae620560..59bb189d1952 100644 --- a/lib/galaxy/webapps/base/controller.py +++ b/lib/galaxy/webapps/base/controller.py @@ -613,139 +613,6 @@ class UsesVisualizationMixin(UsesLibraryMixinItems): slug_builder = SlugBuilder() - def get_visualization(self, trans, id, check_ownership=True, check_accessible=False): - """ - Get a Visualization from the database by id, verifying ownership. - """ - # Load workflow from database - try: - visualization = trans.sa_session.get(model.Visualization, trans.security.decode_id(id)) - except TypeError: - visualization = None - if not visualization: - error("Visualization not found") - else: - return self.security_check(trans, visualization, check_ownership, check_accessible) - - def get_visualization_dict(self, visualization): - """ - Return a set of detailed attributes for a visualization in dictionary form. - The visualization's latest_revision is returned in its own sub-dictionary. - NOTE: that encoding ids isn't done here should happen at the caller level. - """ - return { - "model_class": "Visualization", - "id": visualization.id, - "title": visualization.title, - "type": visualization.type, - "user_id": visualization.user.id, - "dbkey": visualization.dbkey, - "slug": visualization.slug, - # to_dict only the latest revision (allow older to be fetched elsewhere) - "latest_revision": self.get_visualization_revision_dict(visualization.latest_revision), - "revisions": [r.id for r in visualization.revisions], - } - - def get_visualization_revision_dict(self, revision): - """ - Return a set of detailed attributes for a visualization in dictionary form. - NOTE: that encoding ids isn't done here should happen at the caller level. - """ - return { - "model_class": "VisualizationRevision", - "id": revision.id, - "visualization_id": revision.visualization.id, - "title": revision.title, - "dbkey": revision.dbkey, - "config": revision.config, - } - - def import_visualization(self, trans, id, user=None): - """ - Copy the visualization with the given id and associate the copy - with the given user (defaults to trans.user). - - Raises `ItemAccessibilityException` if `user` is not passed and - the current user is anonymous, and if the visualization is not `importable`. - Raises `ItemDeletionException` if the visualization has been deleted. - """ - # default to trans.user, error if anon - if not user: - if not trans.user: - raise exceptions.ItemAccessibilityException("You must be logged in to import Galaxy visualizations") - user = trans.user - - # check accessibility - visualization = self.get_visualization(trans, id, check_ownership=False) - if not visualization.importable: - raise exceptions.ItemAccessibilityException( - "The owner of this visualization has disabled imports via this link." - ) - if visualization.deleted: - raise exceptions.ItemDeletionException("You can't import this visualization because it has been deleted.") - - # copy vis and alter title - # TODO: need to handle custom db keys. - imported_visualization = visualization.copy(user=user, title=f"imported: {visualization.title}") - trans.sa_session.add(imported_visualization) - with transaction(trans.sa_session): - trans.sa_session.commit() - return imported_visualization - - def create_visualization( - self, - trans, - type, - title="Untitled Visualization", - slug=None, - dbkey=None, - annotation=None, - config=None, - save=True, - ): - """ - Create visualiation and first revision. - """ - config = config or {} - visualization = self._create_visualization(trans, title, type, dbkey, slug, annotation, save) - # TODO: handle this error structure better either in _create or here - if isinstance(visualization, dict): - err_dict = visualization - raise ValueError(err_dict["title_err"] or err_dict["slug_err"]) - - # Create and save first visualization revision - revision = trans.model.VisualizationRevision( - visualization=visualization, title=title, config=config, dbkey=dbkey - ) - visualization.latest_revision = revision - - if save: - session = trans.sa_session - session.add(revision) - with transaction(session): - session.commit() - - return visualization - - def add_visualization_revision(self, trans, visualization, config, title, dbkey): - """ - Adds a new `VisualizationRevision` to the given `visualization` with - the given parameters and set its parent visualization's `latest_revision` - to the new revision. - """ - # precondition: only add new revision on owned vis's - # TODO:?? should we default title, dbkey, config? to which: visualization or latest_revision? - revision = trans.model.VisualizationRevision( - visualization=visualization, title=title, dbkey=dbkey, config=config - ) - - visualization.latest_revision = revision - # TODO:?? does this automatically add revision to visualzation.revisions? - trans.sa_session.add(revision) - with transaction(trans.sa_session): - trans.sa_session.commit() - return revision - def save_visualization(self, trans, config, type, id=None, title=None, dbkey=None, slug=None, annotation=None): session = trans.sa_session @@ -1016,44 +883,6 @@ def get_hda(self, trans, dataset_id, check_ownership=True, check_accessible=Fals ) return data - # -- Helper functions -- - - def _create_visualization(self, trans, title, type, dbkey=None, slug=None, annotation=None, save=True): - """Create visualization but not first revision. Returns Visualization object.""" - user = trans.get_user() - - # Error checking. - title_err = slug_err = "" - if not title: - title_err = "visualization name is required" - elif slug and not managers_base.is_valid_slug(slug): - slug_err = "visualization identifier must consist of only lowercase letters, numbers, and the '-' character" - elif slug and slug_exists(trans.sa_session, trans.model.Visualization, user, slug, ignore_deleted=True): - slug_err = "visualization identifier must be unique" - - if title_err or slug_err: - return {"title_err": title_err, "slug_err": slug_err} - - # Create visualization - visualization = trans.model.Visualization(user=user, title=title, dbkey=dbkey, type=type) - if slug: - visualization.slug = slug - else: - self.slug_builder.create_item_slug(trans.sa_session, visualization) - if annotation: - annotation = sanitize_html(annotation) - # TODO: if this is to stay in the mixin, UsesAnnotations should be added to the superclasses - # right now this is depending on the classes that include this mixin to have UsesAnnotations - self.add_item_annotation(trans.sa_session, trans.user, visualization, annotation) - - if save: - session = trans.sa_session - session.add(visualization) - with transaction(session): - session.commit() - - return visualization - def _get_genome_data(self, trans, dataset, dbkey=None): """ Returns genome-wide data for dataset if available; if not, message is returned. From bffd8d34e60f65519197e7effe295b8eecfa2a3b Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 16 Aug 2024 14:01:58 +0200 Subject: [PATCH 06/18] Add comment for creating/importing visualization --- lib/galaxy/webapps/galaxy/api/visualizations.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/galaxy/webapps/galaxy/api/visualizations.py b/lib/galaxy/webapps/galaxy/api/visualizations.py index f3f485572a79..66f4eb9d5005 100644 --- a/lib/galaxy/webapps/galaxy/api/visualizations.py +++ b/lib/galaxy/webapps/galaxy/api/visualizations.py @@ -255,6 +255,13 @@ def create( save: bool = Body(True), trans: ProvidesUserContext = DependsOnTrans, ) -> VisualizationShow: + """ + POST /api/visualizations + creates a new visualization using the given payload + + POST /api/visualizations?import_id={encoded_visualization_id} + imports a copy of an existing visualization into the user's workspace + """ payload = { "import_id": import_id, "type": type, From 7c962b077032051931ea6a06ade1f192e4c5fcf8 Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 16 Aug 2024 14:48:46 +0200 Subject: [PATCH 07/18] Fix Schema --- lib/galaxy/schema/visualization.py | 117 +++++++++++++++--- .../webapps/galaxy/api/visualizations.py | 46 ++----- .../webapps/galaxy/services/visualizations.py | 51 ++++---- 3 files changed, 136 insertions(+), 78 deletions(-) diff --git a/lib/galaxy/schema/visualization.py b/lib/galaxy/schema/visualization.py index a1920856badc..7c899397b16f 100644 --- a/lib/galaxy/schema/visualization.py +++ b/lib/galaxy/schema/visualization.py @@ -103,7 +103,7 @@ class VisualizationSummaryList(RootModel): ) -class VisualizationRevision(Model): +class VisualizationRevisionResponse(Model): model_class: str = Field( "VisualizationRevision", title="Model Class", @@ -136,7 +136,7 @@ class VisualizationRevision(Model): ) -class VisualizationPlugin(Model): +class VisualizationPluginResponse(Model): name: str = Field( ..., title="Name", @@ -199,7 +199,7 @@ class VisualizationPlugin(Model): ) -class VisualizationShow(Model): +class VisualizationShowResponse(Model): model_class: str = Field( "Visualization", title="Model Class", @@ -226,27 +226,27 @@ class VisualizationShow(Model): description="The ID of the user owning this Visualization.", ) dbkey: Optional[str] = Field( - default=None, + None, title="DbKey", description="The database key of the visualization.", ) slug: Optional[str] = Field( - default=None, + None, title="Slug", description="The slug of the visualization.", ) - latest_revision: VisualizationRevision = Field( + latest_revision: VisualizationRevisionResponse = Field( ..., title="Latest Revision", description="The latest revision of this Visualization.", ) revisions: List[EncodedDatabaseIdField] = Field( - default=[], + ..., title="Revisions", description="A list of encoded IDs of the revisions of this Visualization.", ) - url: Optional[str] = Field( - default=None, + url: str = Field( + ..., title="URL", description="The URL of the visualization.", ) @@ -255,23 +255,110 @@ class VisualizationShow(Model): title="Username", description="The name of the user owning this Visualization.", ) - email_hash: Optional[str] = Field( - default=None, + email_hash: str = Field( + ..., title="Email Hash", description="The hash of the email of the user owning this Visualization.", ) - tags: TagCollection = Field( - default=[], + tags: Optional[TagCollection] = Field( + [], title="Tags", description="A list of tags to add to this item.", ) annotation: Optional[str] = Field( - default=None, + None, title="Annotation", description="The annotation of this Visualization.", ) - plugin: VisualizationPlugin = Field( + plugin: VisualizationPluginResponse = Field( ..., title="Plugin", description="The plugin of this Visualization.", ) + + +class VisualizationCreateResponse(Model): + id: EncodedDatabaseIdField = Field( + ..., + title="ID", + description="Encoded ID of the Visualization.", + ) + + +class VisualizationUpdateResponse(Model): + id: EncodedDatabaseIdField = Field( + ..., + title="ID", + description="Encoded ID of the Visualization.", + ) + revision: EncodedDatabaseIdField = Field( + ..., + title="Revision", + description="Encoded ID of the Visualization Revision.", + ) + + +class VisualizationCreatePayload(Model): + import_id: Optional[DecodedDatabaseIdField] = Field( + default=None, + title="Import ID", + description="The ID of the imported visualization.", + ) + type: Optional[str] = Field( + default=None, + title="Type", + description="The type of the visualization.", + ) + title: Optional[str] = Field( + default=None, + title="Title", + description="The name of the visualization.", + ) + dbkey: Optional[str] = Field( + default=None, + title="DbKey", + description="The database key of the visualization.", + ) + slug: Optional[str] = Field( + default=None, + title="Slug", + description="The slug of the visualization.", + ) + annotation: Optional[str] = Field( + default=None, + title="Annotation", + description="The annotation of the visualization.", + ) + config: Optional[dict] = Field( + default={}, + title="Config", + description="The config of the visualization.", + ) + save: Optional[bool] = Field( + default=True, + title="Save", + description="Whether to save the visualization.", + ) + + +class VisualizationUpdatePayload(Model): + title: Optional[str] = Field( + default=None, + title="Title", + description="The name of the visualization.", + ) + dbkey: Optional[str] = Field( + default=None, + title="DbKey", + description="The database key of the visualization.", + ) + deleted: Optional[bool] = Field( + default=False, + title="Deleted", + description="Whether this Visualization has been deleted.", + ) + config: Optional[dict] = Field( + default={}, + title="Config", + description="The config of the visualization.", + ) diff --git a/lib/galaxy/webapps/galaxy/api/visualizations.py b/lib/galaxy/webapps/galaxy/api/visualizations.py index 66f4eb9d5005..7f7b7d494183 100644 --- a/lib/galaxy/webapps/galaxy/api/visualizations.py +++ b/lib/galaxy/webapps/galaxy/api/visualizations.py @@ -26,10 +26,14 @@ SharingStatus, ) from galaxy.schema.visualization import ( + VisualizationCreatePayload, + VisualizationCreateResponse, VisualizationIndexQueryPayload, - VisualizationShow, + VisualizationShowResponse, VisualizationSortByEnum, VisualizationSummaryList, + VisualizationUpdatePayload, + VisualizationUpdateResponse, ) from galaxy.webapps.galaxy.api import ( depends, @@ -235,7 +239,7 @@ def show( self, id: VisualizationIdPathParam, trans: ProvidesUserContext = DependsOnTrans, - ) -> VisualizationShow: + ) -> VisualizationShowResponse: """Return the visualization.""" return self.service.show(trans, id) @@ -245,33 +249,16 @@ def show( ) def create( self, - import_id: Optional[VisualizationIdPathParam] = Body(None), - type: str = Body(...), - title: str = Body(...), - dbkey: Optional[str] = Body(None), - slug: Optional[str] = Body(None), - annotation: Optional[str] = Body(None), - config: Optional[dict] = Body(None), - save: bool = Body(True), + payload: VisualizationCreatePayload = Body(...), trans: ProvidesUserContext = DependsOnTrans, - ) -> VisualizationShow: + ) -> VisualizationCreateResponse: """ POST /api/visualizations - creates a new visualization using the given payload + creates a new visualization using the given payload and does not require the import_id field POST /api/visualizations?import_id={encoded_visualization_id} - imports a copy of an existing visualization into the user's workspace + imports a copy of an existing visualization into the user's workspace and does not require the rest of the payload """ - payload = { - "import_id": import_id, - "type": type, - "title": title, - "dbkey": dbkey, - "slug": slug, - "annotation": annotation, - "config": config, - "save": save, - } return self.service.create(trans, payload) @router.put( @@ -281,16 +268,7 @@ def create( def update( self, id: VisualizationIdPathParam, - title: Optional[str] = Body(None), - dbkey: Optional[str] = Body(None), - deleted: Optional[bool] = Body(None), - config: Optional[dict] = Body(None), + payload: VisualizationUpdatePayload = Body(...), trans: ProvidesUserContext = DependsOnTrans, - ) -> VisualizationShow: - payload = { - "title": title, - "dbkey": dbkey, - "deleted": deleted, - "config": config, - } + ) -> VisualizationUpdateResponse: return self.service.update(trans, id, payload) diff --git a/lib/galaxy/webapps/galaxy/services/visualizations.py b/lib/galaxy/webapps/galaxy/services/visualizations.py index 566988e7d615..0e83d73a5ea3 100644 --- a/lib/galaxy/webapps/galaxy/services/visualizations.py +++ b/lib/galaxy/webapps/galaxy/services/visualizations.py @@ -27,9 +27,12 @@ ) from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.visualization import ( + VisualizationCreateResponse, VisualizationIndexQueryPayload, - VisualizationShow, + VisualizationRevisionResponse, + VisualizationShowResponse, VisualizationSummaryList, + VisualizationUpdateResponse, ) from galaxy.security.idencoding import IdEncodingHelper from galaxy.util.hash_util import md5_hash_str @@ -88,7 +91,7 @@ def show( self, trans: ProvidesAppContext, visualization_id: DecodedDatabaseIdField, - ) -> VisualizationShow: + ) -> VisualizationShowResponse: """Return a dictionary containing the Visualization's details :rtype: dictionary @@ -98,7 +101,19 @@ def show( # the important thing is the config # TODO:?? /api/visualizations/registry -> json of registry.listings? visualization = self._get_visualization(trans, visualization_id, check_ownership=False, check_accessible=True) - dictionary = trans.security.encode_dict_ids(self._get_visualization_dict(visualization)) + visualization_dict = { + "model_class": "Visualization", + "id": visualization.id, + "title": visualization.title, + "type": visualization.type, + "user_id": visualization.user.id, + "dbkey": visualization.dbkey, + "slug": visualization.slug, + # to_dict only the latest revision (allow older to be fetched elsewhere) + "latest_revision": self._get_visualization_revision_dict(visualization.latest_revision), + "revisions": [r.id for r in visualization.revisions], + } + dictionary = trans.security.encode_dict_ids(visualization_dict) dictionary["url"] = trans.url_builder( controller="show_group", action="display_by_username_and_slug", @@ -125,7 +140,7 @@ def create( self, trans: ProvidesAppContext, payload: VisualizationIndexQueryPayload, - ) -> VisualizationShow: + ) -> VisualizationCreateResponse: """Returns a dictionary of the created visualization :rtype: dictionary @@ -178,7 +193,7 @@ def update( trans: ProvidesAppContext, visualization_id: DecodedDatabaseIdField, payload: VisualizationIndexQueryPayload, - ) -> VisualizationShow: + ) -> VisualizationUpdateResponse: """ Update a visualization @@ -222,7 +237,7 @@ def update( def _validate_and_parse_payload( self, payload: VisualizationIndexQueryPayload, - ) -> VisualizationShow: + ) -> VisualizationIndexQueryPayload: """ Validate and parse incomming data payload for a visualization. """ @@ -303,32 +318,10 @@ def _get_visualization( else: return security_check(trans, visualization, check_ownership, check_accessible) - def _get_visualization_dict( - self, - visualization: Visualization, - ) -> dict: - """ - Return a set of detailed attributes for a visualization in dictionary form. - The visualization's latest_revision is returned in its own sub-dictionary. - NOTE: that encoding ids isn't done here should happen at the caller level. - """ - return { - "model_class": "Visualization", - "id": visualization.id, - "title": visualization.title, - "type": visualization.type, - "user_id": visualization.user.id, - "dbkey": visualization.dbkey, - "slug": visualization.slug, - # to_dict only the latest revision (allow older to be fetched elsewhere) - "latest_revision": self._get_visualization_revision_dict(visualization.latest_revision), - "revisions": [r.id for r in visualization.revisions], - } - def _get_visualization_revision_dict( self, revision: VisualizationRevision, - ) -> dict: + ) -> VisualizationRevisionResponse: """ Return a set of detailed attributes for a visualization in dictionary form. NOTE: that encoding ids isn't done here should happen at the caller level. From f8c01b283c8709e93dc17d2d5aafa50603e9ac03 Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 16 Aug 2024 19:33:21 +0200 Subject: [PATCH 08/18] Update --- lib/galaxy/schema/visualization.py | 30 ++++---- .../webapps/galaxy/services/visualizations.py | 77 +++++++++++-------- lib/galaxy_test/api/test_visualizations.py | 4 +- 3 files changed, 63 insertions(+), 48 deletions(-) diff --git a/lib/galaxy/schema/visualization.py b/lib/galaxy/schema/visualization.py index 7c899397b16f..b665bf4f89fa 100644 --- a/lib/galaxy/schema/visualization.py +++ b/lib/galaxy/schema/visualization.py @@ -125,7 +125,7 @@ class VisualizationRevisionResponse(Model): description="The name of the visualization revision.", ) dbkey: Optional[str] = Field( - default=None, + None, title="DbKey", description="The database key of the visualization.", ) @@ -158,7 +158,7 @@ class VisualizationPluginResponse(Model): description="The logo of the plugin.", ) title: Optional[str] = Field( - default=None, + None, title="Title", description="The title of the plugin.", ) @@ -299,43 +299,43 @@ class VisualizationUpdateResponse(Model): class VisualizationCreatePayload(Model): - import_id: Optional[DecodedDatabaseIdField] = Field( - default=None, + import_id: Optional[EncodedDatabaseIdField] = Field( + None, title="Import ID", description="The ID of the imported visualization.", ) type: Optional[str] = Field( - default=None, + None, title="Type", description="The type of the visualization.", ) title: Optional[str] = Field( - default=None, + "Untitled Visualization", title="Title", description="The name of the visualization.", ) dbkey: Optional[str] = Field( - default=None, + None, title="DbKey", description="The database key of the visualization.", ) slug: Optional[str] = Field( - default=None, + None, title="Slug", description="The slug of the visualization.", ) annotation: Optional[str] = Field( - default=None, + None, title="Annotation", description="The annotation of the visualization.", ) config: Optional[dict] = Field( - default={}, + {}, title="Config", description="The config of the visualization.", ) save: Optional[bool] = Field( - default=True, + True, title="Save", description="Whether to save the visualization.", ) @@ -343,22 +343,22 @@ class VisualizationCreatePayload(Model): class VisualizationUpdatePayload(Model): title: Optional[str] = Field( - default=None, + None, title="Title", description="The name of the visualization.", ) dbkey: Optional[str] = Field( - default=None, + None, title="DbKey", description="The database key of the visualization.", ) deleted: Optional[bool] = Field( - default=False, + False, title="Deleted", description="Whether this Visualization has been deleted.", ) config: Optional[dict] = Field( - default={}, + {}, title="Config", description="The config of the visualization.", ) diff --git a/lib/galaxy/webapps/galaxy/services/visualizations.py b/lib/galaxy/webapps/galaxy/services/visualizations.py index 0e83d73a5ea3..05bcd7d264cf 100644 --- a/lib/galaxy/webapps/galaxy/services/visualizations.py +++ b/lib/galaxy/webapps/galaxy/services/visualizations.py @@ -1,6 +1,9 @@ import json import logging -from typing import Tuple +from typing import ( + Tuple, + Union, +) from galaxy import exceptions from galaxy.managers.base import ( @@ -27,11 +30,13 @@ ) from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.visualization import ( + VisualizationCreatePayload, VisualizationCreateResponse, VisualizationIndexQueryPayload, VisualizationRevisionResponse, VisualizationShowResponse, VisualizationSummaryList, + VisualizationUpdatePayload, VisualizationUpdateResponse, ) from galaxy.security.idencoding import IdEncodingHelper @@ -115,7 +120,8 @@ def show( } dictionary = trans.security.encode_dict_ids(visualization_dict) dictionary["url"] = trans.url_builder( - controller="show_group", + name="visualization", + controller="visualization", action="display_by_username_and_slug", username=visualization.user.username, slug=visualization.slug, @@ -139,7 +145,7 @@ def show( def create( self, trans: ProvidesAppContext, - payload: VisualizationIndexQueryPayload, + payload: VisualizationCreatePayload, ) -> VisualizationCreateResponse: """Returns a dictionary of the created visualization @@ -148,21 +154,20 @@ def create( """ rval = None - if "import_id" in payload: - import_id = payload["import_id"] - visualization = self._import_visualization(trans, import_id, user=trans.user) + if payload.import_id: + visualization = self._import_visualization(trans, payload.import_id) else: payload = self._validate_and_parse_payload(payload) # must have a type (I've taken this to be the visualization name) - if "type" not in payload: + if not payload.type: raise exceptions.RequestParameterMissingException("key/value 'type' is required") - type = payload.pop("type", False) - title = payload.get("title", "Untitled Visualization") - slug = payload.get("slug", None) - dbkey = payload.get("dbkey", None) - annotation = payload.get("annotation", None) - config = payload.get("config", {}) - save = payload.get("save", True) + type = payload.type + title = payload.title + slug = payload.slug + dbkey = payload.dbkey + annotation = payload.annotation + config = payload.config + save = payload.save or True # generate defaults - this will err if given a weird key? visualization = self._create_visualization(trans, title, type, dbkey, slug, annotation, save) @@ -186,13 +191,13 @@ def create( rval = {"id": trans.security.encode_id(visualization.id)} - return rval + return VisualizationCreateResponse(**rval) def update( self, trans: ProvidesAppContext, visualization_id: DecodedDatabaseIdField, - payload: VisualizationIndexQueryPayload, + payload: VisualizationUpdatePayload, ) -> VisualizationUpdateResponse: """ Update a visualization @@ -212,10 +217,10 @@ def update( # only update owned visualizations visualization = self._get_visualization(trans, visualization_id, check_ownership=True) - title = payload.get("title", visualization.latest_revision.title) - dbkey = payload.get("dbkey", visualization.latest_revision.dbkey) - deleted = payload.get("deleted", visualization.deleted) - config = payload.get("config", visualization.latest_revision.config) + title = payload.title or visualization.latest_revision.title + dbkey = payload.dbkey or visualization.latest_revision.dbkey + deleted = payload.deleted or visualization.deleted + config = payload.config or visualization.latest_revision.config latest_config = visualization.latest_revision.config if ( @@ -232,12 +237,12 @@ def update( with transaction(trans.sa_session): trans.sa_session.commit() - return rval + return VisualizationUpdateResponse(**rval) def _validate_and_parse_payload( self, - payload: VisualizationIndexQueryPayload, - ) -> VisualizationIndexQueryPayload: + payload: Union[VisualizationCreatePayload, VisualizationUpdatePayload], + ) -> Union[VisualizationCreatePayload, VisualizationUpdatePayload]: """ Validate and parse incomming data payload for a visualization. """ @@ -259,7 +264,7 @@ def _validate_and_parse_payload( ValidationError = exceptions.RequestParameterInvalidException validated_payload = {} - for key, val in payload.items(): + for key, val in payload.model_dump().items(): # TODO: validate types in VALID_TYPES/registry names at the mixin/model level? if key == "type": if not isinstance(val, str): @@ -296,7 +301,10 @@ def _validate_and_parse_payload( # raise AttributeError( 'unknown key: %s' %( str( key ) ) ) validated_payload[key] = val - return validated_payload + try: + return VisualizationCreatePayload(**validated_payload) + except TypeError: + return VisualizationUpdatePayload(**validated_payload) def _get_visualization( self, @@ -310,7 +318,7 @@ def _get_visualization( """ # Load workflow from database try: - visualization = trans.sa_session.get(Visualization, trans.security.decode_id(visualization_id)) + visualization = trans.sa_session.get(Visualization, visualization_id) except TypeError: visualization = None if not visualization: @@ -407,7 +415,11 @@ def _create_visualization( return visualization - def _import_visualization(self, trans, id, user=None): + def _import_visualization( + self, + trans: ProvidesAppContext, + id: DecodedDatabaseIdField, + ) -> Visualization: """ Copy the visualization with the given id and associate the copy with the given user (defaults to trans.user). @@ -417,10 +429,9 @@ def _import_visualization(self, trans, id, user=None): Raises `ItemDeletionException` if the visualization has been deleted. """ # default to trans.user, error if anon - if not user: - if not trans.user: - raise exceptions.ItemAccessibilityException("You must be logged in to import Galaxy visualizations") - user = trans.user + if not trans.user: + raise exceptions.ItemAccessibilityException("You must be logged in to import Galaxy visualizations") + user = trans.user # check accessibility visualization = self.get_visualization(trans, id, check_ownership=False) @@ -438,3 +449,7 @@ def _import_visualization(self, trans, id, user=None): with transaction(trans.sa_session): trans.sa_session.commit() return imported_visualization + + +# ToDo check for the encode-decode conflicts in the code +# ToDo check for name problem in url_builder \ No newline at end of file diff --git a/lib/galaxy_test/api/test_visualizations.py b/lib/galaxy_test/api/test_visualizations.py index 989fc25f2489..4e463f6871ed 100644 --- a/lib/galaxy_test/api/test_visualizations.py +++ b/lib/galaxy_test/api/test_visualizations.py @@ -101,7 +101,7 @@ def _create_viz(self, **kwds): return viz["id"], viz def _index(self, params): - index_response = self._get("visualizations", data=params or {}) + index_response = self._get("visualizations", json=params or {}) return index_response.json() def _index_ids(self, params=None): @@ -129,7 +129,7 @@ def _new_viz(self, title=None, slug=None, config=None): "annotation": "this is a test of the emergency visualization system", "config": config, } - response = self._post("visualizations", data=create_payload) + response = self._post("visualizations", json=create_payload) return response def _publish_viz(self, id): From 1d7142631ad7a313d3ccd8e5cc0ca8b66f33e796 Mon Sep 17 00:00:00 2001 From: Arash Date: Mon, 19 Aug 2024 18:40:37 +0200 Subject: [PATCH 09/18] Fix bugs with pydantic models, add schema.ts, fix tests --- client/src/api/schema/schema.ts | 443 ++++++++++++++++++ lib/galaxy/schema/visualization.py | 10 +- .../webapps/galaxy/api/visualizations.py | 7 +- .../webapps/galaxy/services/visualizations.py | 58 +-- lib/galaxy_test/api/test_visualizations.py | 16 +- 5 files changed, 484 insertions(+), 50 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 6f1ed33cf314..2e28f1c53f5c 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -4687,6 +4687,35 @@ export interface paths { /** Returns visualizations for the current user. */ get: operations["index_api_visualizations_get"]; put?: never; + /** + * Create a new visualization. + * @description POST /api/visualizations + * creates a new visualization using the given payload and does not require the import_id field + * + * POST /api/visualizations?import_id={encoded_visualization_id} + * imports a copy of an existing visualization into the user's workspace and does not require the rest of the payload + */ + post: operations["create_api_visualizations_post"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/visualizations/{id}": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** + * Get a visualization by ID. + * @description Return the visualization. + */ + get: operations["show_api_visualizations__id__get"]; + /** Update a visualization. */ + put: operations["update_api_visualizations__id__put"]; post?: never; delete?: never; options?: never; @@ -16649,6 +16678,243 @@ export interface components { }; /** Visualization */ Visualization: Record; + /** VisualizationCreatePayload */ + VisualizationCreatePayload: { + /** + * Annotation + * @description The annotation of the visualization. + */ + annotation?: string | null; + /** + * Config + * @description The config of the visualization. + * @default {} + */ + config: Record | null; + /** + * DbKey + * @description The database key of the visualization. + */ + dbkey?: string | null; + /** + * Import ID + * @description The ID of the imported visualization. + */ + import_id?: string | null; + /** + * Save + * @description Whether to save the visualization. + * @default true + */ + save: boolean | null; + /** + * Slug + * @description The slug of the visualization. + */ + slug?: string | null; + /** + * Title + * @description The name of the visualization. + * @default Untitled Visualization + */ + title: string | null; + /** + * Type + * @description The type of the visualization. + */ + type?: string | null; + }; + /** VisualizationCreateResponse */ + VisualizationCreateResponse: { + /** + * ID + * @description Encoded ID of the Visualization. + * @example 0123456789ABCDEF + */ + id: string; + }; + /** VisualizationPluginResponse */ + VisualizationPluginResponse: { + /** + * Description + * @description The description of the plugin. + */ + description: string; + /** + * Embeddable + * @description Whether the plugin is embeddable. + */ + embeddable: boolean; + /** + * Entry Point + * @description The entry point of the plugin. + */ + entry_point: Record; + /** + * Groups + * @description The groups of the plugin. + */ + groups: Record[]; + /** + * Href + * @description The href of the plugin. + */ + href: string; + /** + * HTML + * @description The HTML of the plugin. + */ + html: string; + /** + * Logo + * @description The logo of the plugin. + */ + logo: string; + /** + * Name + * @description The name of the plugin. + */ + name: string; + /** + * Settings + * @description The settings of the plugin. + */ + settings: Record[]; + /** + * Specs + * @description The specs of the plugin. + */ + specs?: Record | null; + /** + * Target + * @description The target of the plugin. + */ + target: string; + /** + * Title + * @description The title of the plugin. + */ + title?: string | null; + }; + /** VisualizationRevisionResponse */ + VisualizationRevisionResponse: { + /** + * Config + * @description The config of the visualization revision. + */ + config: Record; + /** + * DbKey + * @description The database key of the visualization. + */ + dbkey?: string | null; + /** + * ID + * @description Encoded ID of the Visualization Revision. + * @example 0123456789ABCDEF + */ + id: string; + /** + * Model Class + * @description The model class name for this object. + * @default VisualizationRevision + */ + model_class: string; + /** + * Title + * @description The name of the visualization revision. + */ + title: string; + /** + * Visualization ID + * @description Encoded ID of the Visualization. + * @example 0123456789ABCDEF + */ + visualization_id: string; + }; + /** VisualizationShowResponse */ + VisualizationShowResponse: { + /** + * Annotation + * @description The annotation of this Visualization. + */ + annotation?: string | null; + /** + * DbKey + * @description The database key of the visualization. + */ + dbkey?: string | null; + /** + * Email Hash + * @description The hash of the email of the user owning this Visualization. + */ + email_hash: string; + /** + * ID + * @description Encoded ID of the Visualization. + * @example 0123456789ABCDEF + */ + id: string; + /** + * Latest Revision + * @description The latest revision of this Visualization. + */ + latest_revision: components["schemas"]["VisualizationRevisionResponse"]; + /** + * Model Class + * @description The model class name for this object. + * @default Visualization + */ + model_class: string; + /** + * Plugin + * @description The plugin of this Visualization. + * @default {} + */ + plugin: components["schemas"]["VisualizationPluginResponse"] | null; + /** + * Revisions + * @description A list of encoded IDs of the revisions of this Visualization. + */ + revisions: string[]; + /** + * Slug + * @description The slug of the visualization. + */ + slug?: string | null; + /** + * Tags + * @description A list of tags to add to this item. + * @default [] + */ + tags: components["schemas"]["TagCollection"] | null; + /** + * Title + * @description The name of the visualization. + */ + title: string; + /** + * Type + * @description The type of the visualization. + */ + type: string; + /** + * URL + * @description The URL of the visualization. + */ + url: string; + /** + * User ID + * @description The ID of the user owning this Visualization. + * @example 0123456789ABCDEF + */ + user_id: string; + /** + * Username + * @description The name of the user owning this Visualization. + */ + username: string; + }; /** VisualizationSummary */ VisualizationSummary: { /** @@ -16720,6 +16986,46 @@ export interface components { * @default [] */ VisualizationSummaryList: components["schemas"]["VisualizationSummary"][]; + /** VisualizationUpdatePayload */ + VisualizationUpdatePayload: { + /** + * Config + * @description The config of the visualization. + * @default {} + */ + config: Record | null; + /** + * DbKey + * @description The database key of the visualization. + */ + dbkey?: string | null; + /** + * Deleted + * @description Whether this Visualization has been deleted. + * @default false + */ + deleted: boolean | null; + /** + * Title + * @description The name of the visualization. + */ + title?: string | null; + }; + /** VisualizationUpdateResponse */ + VisualizationUpdateResponse: { + /** + * ID + * @description Encoded ID of the Visualization. + * @example 0123456789ABCDEF + */ + id: string; + /** + * Revision + * @description Encoded ID of the Visualization Revision. + * @example 0123456789ABCDEF + */ + revision: string; + }; /** WorkflowInput */ WorkflowInput: { /** @@ -32135,6 +32441,143 @@ export interface operations { }; }; }; + create_api_visualizations_post: { + parameters: { + query?: never; + header?: { + /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ + "run-as"?: string | null; + }; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["VisualizationCreatePayload"]; + }; + }; + responses: { + /** @description Successful Response */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["VisualizationCreateResponse"]; + }; + }; + /** @description Request Error */ + "4XX": { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["MessageExceptionModel"]; + }; + }; + /** @description Server Error */ + "5XX": { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["MessageExceptionModel"]; + }; + }; + }; + }; + show_api_visualizations__id__get: { + parameters: { + query?: never; + header?: { + /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ + "run-as"?: string | null; + }; + path: { + /** @description The encoded database identifier of the Visualization. */ + id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Successful Response */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["VisualizationShowResponse"]; + }; + }; + /** @description Request Error */ + "4XX": { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["MessageExceptionModel"]; + }; + }; + /** @description Server Error */ + "5XX": { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["MessageExceptionModel"]; + }; + }; + }; + }; + update_api_visualizations__id__put: { + parameters: { + query?: never; + header?: { + /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ + "run-as"?: string | null; + }; + path: { + /** @description The encoded database identifier of the Visualization. */ + id: string; + }; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["VisualizationUpdatePayload"]; + }; + }; + responses: { + /** @description Successful Response */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["VisualizationUpdateResponse"] | null; + }; + }; + /** @description Request Error */ + "4XX": { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["MessageExceptionModel"]; + }; + }; + /** @description Server Error */ + "5XX": { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["MessageExceptionModel"]; + }; + }; + }; + }; disable_link_access_api_visualizations__id__disable_link_access_put: { parameters: { query?: never; diff --git a/lib/galaxy/schema/visualization.py b/lib/galaxy/schema/visualization.py index b665bf4f89fa..f189260e517f 100644 --- a/lib/galaxy/schema/visualization.py +++ b/lib/galaxy/schema/visualization.py @@ -187,8 +187,8 @@ class VisualizationPluginResponse(Model): title="Groups", description="The groups of the plugin.", ) - specs: Dict = Field( - ..., + specs: Optional[Dict] = Field( + None, title="Specs", description="The specs of the plugin.", ) @@ -220,7 +220,7 @@ class VisualizationShowResponse(Model): title="Type", description="The type of the visualization.", ) - user_id: DecodedDatabaseIdField = Field( + user_id: EncodedDatabaseIdField = Field( ..., title="User ID", description="The ID of the user owning this Visualization.", @@ -270,8 +270,8 @@ class VisualizationShowResponse(Model): title="Annotation", description="The annotation of this Visualization.", ) - plugin: VisualizationPluginResponse = Field( - ..., + plugin: Optional[VisualizationPluginResponse] = Field( + {}, title="Plugin", description="The plugin of this Visualization.", ) diff --git a/lib/galaxy/webapps/galaxy/api/visualizations.py b/lib/galaxy/webapps/galaxy/api/visualizations.py index 7f7b7d494183..e76ed8a94e4b 100644 --- a/lib/galaxy/webapps/galaxy/api/visualizations.py +++ b/lib/galaxy/webapps/galaxy/api/visualizations.py @@ -6,7 +6,10 @@ """ import logging -from typing import Optional +from typing import ( + Optional, + Union, +) from fastapi import ( Body, @@ -270,5 +273,5 @@ def update( id: VisualizationIdPathParam, payload: VisualizationUpdatePayload = Body(...), trans: ProvidesUserContext = DependsOnTrans, - ) -> VisualizationUpdateResponse: + ) -> Union[VisualizationUpdateResponse, None]: return self.service.update(trans, id, payload) diff --git a/lib/galaxy/webapps/galaxy/services/visualizations.py b/lib/galaxy/webapps/galaxy/services/visualizations.py index 05bcd7d264cf..9a54a452832c 100644 --- a/lib/galaxy/webapps/galaxy/services/visualizations.py +++ b/lib/galaxy/webapps/galaxy/services/visualizations.py @@ -42,6 +42,7 @@ from galaxy.security.idencoding import IdEncodingHelper from galaxy.util.hash_util import md5_hash_str from galaxy.util.sanitize_html import sanitize_html +from galaxy.web import url_for from galaxy.webapps.galaxy.services.base import ServiceBase from galaxy.webapps.galaxy.services.notifications import NotificationService from galaxy.webapps.galaxy.services.sharable import ShareableService @@ -106,7 +107,7 @@ def show( # the important thing is the config # TODO:?? /api/visualizations/registry -> json of registry.listings? visualization = self._get_visualization(trans, visualization_id, check_ownership=False, check_accessible=True) - visualization_dict = { + dictionary = { "model_class": "Visualization", "id": visualization.id, "title": visualization.title, @@ -116,11 +117,12 @@ def show( "slug": visualization.slug, # to_dict only the latest revision (allow older to be fetched elsewhere) "latest_revision": self._get_visualization_revision_dict(visualization.latest_revision), + # need to encode ids in revisions as well + # NOTE: does not encode ids inside the configs "revisions": [r.id for r in visualization.revisions], } - dictionary = trans.security.encode_dict_ids(visualization_dict) - dictionary["url"] = trans.url_builder( - name="visualization", + # replace with trans.url_builder if possible + dictionary["url"] = url_for( controller="visualization", action="display_by_username_and_slug", username=visualization.user.username, @@ -130,17 +132,9 @@ def show( dictionary["email_hash"] = md5_hash_str(visualization.user.email) dictionary["tags"] = visualization.make_tag_string_list() dictionary["annotation"] = get_item_annotation_str(trans.sa_session, trans.user, visualization) - # need to encode ids in revisions as well - encoded_revisions = [] - for revision in dictionary["revisions"]: - # NOTE: does not encode ids inside the configs - encoded_revisions.append(trans.security.encode_id(revision)) - dictionary["revisions"] = encoded_revisions - dictionary["latest_revision"] = trans.security.encode_dict_ids(dictionary["latest_revision"]) if trans.app.visualizations_registry: - visualization = trans.app.visualizations_registry.get_plugin(dictionary["type"]) - dictionary["plugin"] = visualization.to_dict() - return dictionary + dictionary["plugin"] = trans.app.visualizations_registry.get_plugin(dictionary["type"]).to_dict() + return VisualizationShowResponse(**dictionary) def create( self, @@ -152,7 +146,6 @@ def create( :rtype: dictionary :returns: dictionary containing Visualization details """ - rval = None if payload.import_id: visualization = self._import_visualization(trans, payload.import_id) @@ -167,15 +160,10 @@ def create( dbkey = payload.dbkey annotation = payload.annotation config = payload.config - save = payload.save or True + save = payload.save # generate defaults - this will err if given a weird key? visualization = self._create_visualization(trans, title, type, dbkey, slug, annotation, save) - # TODO: handle this error structure better either in _create or here - if isinstance(visualization, dict): - err_dict = visualization - val_err = str(err_dict["title_err"] or err_dict["slug_err"]) - raise exceptions.RequestParameterMissingException(val_err) # Create and save first visualization revision revision = trans.model.VisualizationRevision( @@ -189,7 +177,7 @@ def create( with transaction(session): session.commit() - rval = {"id": trans.security.encode_id(visualization.id)} + rval = {"id": visualization.id} return VisualizationCreateResponse(**rval) @@ -198,7 +186,7 @@ def update( trans: ProvidesAppContext, visualization_id: DecodedDatabaseIdField, payload: VisualizationUpdatePayload, - ) -> VisualizationUpdateResponse: + ) -> Union[VisualizationUpdateResponse, None]: """ Update a visualization @@ -237,7 +225,7 @@ def update( with transaction(trans.sa_session): trans.sa_session.commit() - return VisualizationUpdateResponse(**rval) + return VisualizationUpdateResponse(**rval) if rval else None def _validate_and_parse_payload( self, @@ -265,6 +253,9 @@ def _validate_and_parse_payload( validated_payload = {} for key, val in payload.model_dump().items(): + # By adding the pydatnic model there will be some variables that are not set and should be ignored in the validation + if val is None: + continue # TODO: validate types in VALID_TYPES/registry names at the mixin/model level? if key == "type": if not isinstance(val, str): @@ -301,9 +292,9 @@ def _validate_and_parse_payload( # raise AttributeError( 'unknown key: %s' %( str( key ) ) ) validated_payload[key] = val - try: + if isinstance(payload, VisualizationCreatePayload): return VisualizationCreatePayload(**validated_payload) - except TypeError: + elif isinstance(payload, VisualizationUpdatePayload): return VisualizationUpdatePayload(**validated_payload) def _get_visualization( @@ -334,7 +325,7 @@ def _get_visualization_revision_dict( Return a set of detailed attributes for a visualization in dictionary form. NOTE: that encoding ids isn't done here should happen at the caller level. """ - return { + revision_dict = { "model_class": "VisualizationRevision", "id": revision.id, "visualization_id": revision.visualization.id, @@ -342,6 +333,7 @@ def _get_visualization_revision_dict( "dbkey": revision.dbkey, "config": revision.config, } + return VisualizationRevisionResponse(**revision_dict) def _add_visualization_revision( self, @@ -392,7 +384,9 @@ def _create_visualization( slug_err = "visualization identifier must be unique" if title_err or slug_err: - return {"title_err": title_err, "slug_err": slug_err} + # TODO: handle this error structure better + val_err = str(title_err or slug_err) + raise exceptions.RequestParameterMissingException(val_err) # Create visualization visualization = trans.model.Visualization(user=user, title=title, dbkey=dbkey, type=type) @@ -418,7 +412,7 @@ def _create_visualization( def _import_visualization( self, trans: ProvidesAppContext, - id: DecodedDatabaseIdField, + visualization_id: DecodedDatabaseIdField, ) -> Visualization: """ Copy the visualization with the given id and associate the copy @@ -434,7 +428,7 @@ def _import_visualization( user = trans.user # check accessibility - visualization = self.get_visualization(trans, id, check_ownership=False) + visualization = self._get_visualization(trans, visualization_id, check_ownership=False) if not visualization.importable: raise exceptions.ItemAccessibilityException( "The owner of this visualization has disabled imports via this link." @@ -449,7 +443,3 @@ def _import_visualization( with transaction(trans.sa_session): trans.sa_session.commit() return imported_visualization - - -# ToDo check for the encode-decode conflicts in the code -# ToDo check for name problem in url_builder \ No newline at end of file diff --git a/lib/galaxy_test/api/test_visualizations.py b/lib/galaxy_test/api/test_visualizations.py index 4e463f6871ed..6a9e9350c5a5 100644 --- a/lib/galaxy_test/api/test_visualizations.py +++ b/lib/galaxy_test/api/test_visualizations.py @@ -89,7 +89,7 @@ def test_sharing(self): def test_update_title(self): viz_id, viz = self._create_viz() update_url = self._api_url(f"visualizations/{viz_id}", use_key=True) - response = put(update_url, {"title": "New Name"}) + response = self._put(update_url, {"title": "New Name"}, json=True) self._assert_status_code_is(response, 200) updated_viz = self._show_viz(viz_id) assert updated_viz["title"] == "New Name" @@ -101,7 +101,7 @@ def _create_viz(self, **kwds): return viz["id"], viz def _index(self, params): - index_response = self._get("visualizations", json=params or {}) + index_response = self._get("visualizations", data=params or {}) return index_response.json() def _index_ids(self, params=None): @@ -114,12 +114,10 @@ def _new_viz(self, title=None, slug=None, config=None): config = ( config if config is not None - else json.dumps( - { - "x": 10, - "y": 12, - } - ) + else { + "x": 10, + "y": 12, + } ) create_payload = { "title": title, @@ -129,7 +127,7 @@ def _new_viz(self, title=None, slug=None, config=None): "annotation": "this is a test of the emergency visualization system", "config": config, } - response = self._post("visualizations", json=create_payload) + response = self._post("visualizations", data=create_payload, json=True) return response def _publish_viz(self, id): From da230383015ad107e9b9cf842ed7f521f6019e01 Mon Sep 17 00:00:00 2001 From: Arash Date: Mon, 19 Aug 2024 18:50:03 +0200 Subject: [PATCH 10/18] remove unused import --- lib/galaxy_test/api/test_visualizations.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/galaxy_test/api/test_visualizations.py b/lib/galaxy_test/api/test_visualizations.py index 6a9e9350c5a5..0a2577c086ca 100644 --- a/lib/galaxy_test/api/test_visualizations.py +++ b/lib/galaxy_test/api/test_visualizations.py @@ -1,8 +1,5 @@ -import json import uuid -from requests import put - from galaxy_test.api.sharable import SharingApiTests from galaxy_test.base.api_asserts import assert_has_keys from ._framework import ApiTestCase From 1c908d60174ca1c385bd14b3b8c66da0fec86045 Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 20 Aug 2024 11:16:36 +0200 Subject: [PATCH 11/18] Fix mypy errors --- lib/galaxy/schema/visualization.py | 6 +-- .../webapps/galaxy/services/visualizations.py | 39 +++++++++++-------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/lib/galaxy/schema/visualization.py b/lib/galaxy/schema/visualization.py index f189260e517f..9efd344eb4f0 100644 --- a/lib/galaxy/schema/visualization.py +++ b/lib/galaxy/schema/visualization.py @@ -261,7 +261,7 @@ class VisualizationShowResponse(Model): description="The hash of the email of the user owning this Visualization.", ) tags: Optional[TagCollection] = Field( - [], + None, title="Tags", description="A list of tags to add to this item.", ) @@ -271,7 +271,7 @@ class VisualizationShowResponse(Model): description="The annotation of this Visualization.", ) plugin: Optional[VisualizationPluginResponse] = Field( - {}, + None, title="Plugin", description="The plugin of this Visualization.", ) @@ -299,7 +299,7 @@ class VisualizationUpdateResponse(Model): class VisualizationCreatePayload(Model): - import_id: Optional[EncodedDatabaseIdField] = Field( + import_id: Optional[DecodedDatabaseIdField] = Field( None, title="Import ID", description="The ID of the imported visualization.", diff --git a/lib/galaxy/webapps/galaxy/services/visualizations.py b/lib/galaxy/webapps/galaxy/services/visualizations.py index 9a54a452832c..562b9ed289de 100644 --- a/lib/galaxy/webapps/galaxy/services/visualizations.py +++ b/lib/galaxy/webapps/galaxy/services/visualizations.py @@ -1,6 +1,7 @@ import json import logging from typing import ( + Optional, Tuple, Union, ) @@ -10,7 +11,7 @@ is_valid_slug, security_check, ) -from galaxy.managers.context import ProvidesAppContext +from galaxy.managers.context import ProvidesUserContext from galaxy.managers.sharable import ( slug_exists, SlugBuilder, @@ -40,8 +41,11 @@ VisualizationUpdateResponse, ) from galaxy.security.idencoding import IdEncodingHelper +from galaxy.structured_app import StructuredApp from galaxy.util.hash_util import md5_hash_str from galaxy.util.sanitize_html import sanitize_html +from galaxy.visualization.plugins.plugin import VisualizationPlugin +from galaxy.visualization.plugins.registry import VisualizationsRegistry from galaxy.web import url_for from galaxy.webapps.galaxy.services.base import ServiceBase from galaxy.webapps.galaxy.services.notifications import NotificationService @@ -73,7 +77,7 @@ def __init__( def index( self, - trans, + trans: ProvidesUserContext, payload: VisualizationIndexQueryPayload, include_total_count: bool = False, ) -> Tuple[VisualizationSummaryList, int]: @@ -95,7 +99,7 @@ def index( def show( self, - trans: ProvidesAppContext, + trans: ProvidesUserContext, visualization_id: DecodedDatabaseIdField, ) -> VisualizationShowResponse: """Return a dictionary containing the Visualization's details @@ -132,13 +136,16 @@ def show( dictionary["email_hash"] = md5_hash_str(visualization.user.email) dictionary["tags"] = visualization.make_tag_string_list() dictionary["annotation"] = get_item_annotation_str(trans.sa_session, trans.user, visualization) - if trans.app.visualizations_registry: - dictionary["plugin"] = trans.app.visualizations_registry.get_plugin(dictionary["type"]).to_dict() + app: StructuredApp = trans.app + if app.visualizations_registry: + visualizations_registry: VisualizationsRegistry = app.visualizations_registry + visualization_plugin: VisualizationPlugin = visualizations_registry.get_plugin(dictionary["type"]) + dictionary["plugin"] = visualization_plugin.to_dict() return VisualizationShowResponse(**dictionary) def create( self, - trans: ProvidesAppContext, + trans: ProvidesUserContext, payload: VisualizationCreatePayload, ) -> VisualizationCreateResponse: """Returns a dictionary of the created visualization @@ -163,7 +170,7 @@ def create( save = payload.save # generate defaults - this will err if given a weird key? - visualization = self._create_visualization(trans, title, type, dbkey, slug, annotation, save) + visualization = self._create_visualization(trans, type, title, dbkey, slug, annotation, save) # Create and save first visualization revision revision = trans.model.VisualizationRevision( @@ -183,7 +190,7 @@ def create( def update( self, - trans: ProvidesAppContext, + trans: ProvidesUserContext, visualization_id: DecodedDatabaseIdField, payload: VisualizationUpdatePayload, ) -> Union[VisualizationUpdateResponse, None]: @@ -299,7 +306,7 @@ def _validate_and_parse_payload( def _get_visualization( self, - trans: ProvidesAppContext, + trans: ProvidesUserContext, visualization_id: DecodedDatabaseIdField, check_ownership=True, check_accessible=False, @@ -337,7 +344,7 @@ def _get_visualization_revision_dict( def _add_visualization_revision( self, - trans: ProvidesAppContext, + trans: ProvidesUserContext, visualization: Visualization, config: dict, title: str, @@ -363,12 +370,12 @@ def _add_visualization_revision( def _create_visualization( self, - trans: ProvidesAppContext, - title: str, + trans: ProvidesUserContext, type: str, - dbkey: str = None, - slug: str = None, - annotation: str = None, + title: Optional[str] = None, + dbkey: Optional[str] = None, + slug: Optional[str] = None, + annotation: Optional[str] = None, save: bool = True, ) -> Visualization: """Create visualization but not first revision. Returns Visualization object.""" @@ -411,7 +418,7 @@ def _create_visualization( def _import_visualization( self, - trans: ProvidesAppContext, + trans: ProvidesUserContext, visualization_id: DecodedDatabaseIdField, ) -> Visualization: """ From c2a1014725535158b25ad49983ba436669044557 Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 20 Aug 2024 12:23:39 +0200 Subject: [PATCH 12/18] Fix mypy errors --- lib/galaxy/schema/visualization.py | 3 +- .../webapps/galaxy/services/visualizations.py | 44 ++++++++++--------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/lib/galaxy/schema/visualization.py b/lib/galaxy/schema/visualization.py index 9efd344eb4f0..fe859a34b204 100644 --- a/lib/galaxy/schema/visualization.py +++ b/lib/galaxy/schema/visualization.py @@ -3,6 +3,7 @@ Dict, List, Optional, + Union, ) from pydantic import ( @@ -357,7 +358,7 @@ class VisualizationUpdatePayload(Model): title="Deleted", description="Whether this Visualization has been deleted.", ) - config: Optional[dict] = Field( + config: Optional[Union[dict, bytes]] = Field( {}, title="Config", description="The config of the visualization.", diff --git a/lib/galaxy/webapps/galaxy/services/visualizations.py b/lib/galaxy/webapps/galaxy/services/visualizations.py index 562b9ed289de..07f782629184 100644 --- a/lib/galaxy/webapps/galaxy/services/visualizations.py +++ b/lib/galaxy/webapps/galaxy/services/visualizations.py @@ -1,6 +1,7 @@ import json import logging from typing import ( + cast, Optional, Tuple, Union, @@ -120,7 +121,11 @@ def show( "dbkey": visualization.dbkey, "slug": visualization.slug, # to_dict only the latest revision (allow older to be fetched elsewhere) - "latest_revision": self._get_visualization_revision_dict(visualization.latest_revision), + "latest_revision": ( + self._get_visualization_revision_dict(visualization.latest_revision) + if visualization.latest_revision + else None + ), # need to encode ids in revisions as well # NOTE: does not encode ids inside the configs "revisions": [r.id for r in visualization.revisions], @@ -136,10 +141,10 @@ def show( dictionary["email_hash"] = md5_hash_str(visualization.user.email) dictionary["tags"] = visualization.make_tag_string_list() dictionary["annotation"] = get_item_annotation_str(trans.sa_session, trans.user, visualization) - app: StructuredApp = trans.app + app = cast(StructuredApp, trans.app) if app.visualizations_registry: - visualizations_registry: VisualizationsRegistry = app.visualizations_registry - visualization_plugin: VisualizationPlugin = visualizations_registry.get_plugin(dictionary["type"]) + visualizations_registry = cast(VisualizationsRegistry, app.visualizations_registry) + visualization_plugin = cast(VisualizationPlugin, visualizations_registry.get_plugin(dictionary["type"])) dictionary["plugin"] = visualization_plugin.to_dict() return VisualizationShowResponse(**dictionary) @@ -157,7 +162,7 @@ def create( if payload.import_id: visualization = self._import_visualization(trans, payload.import_id) else: - payload = self._validate_and_parse_payload(payload) + payload = cast(VisualizationCreatePayload, self._validate_and_parse_payload(payload)) # must have a type (I've taken this to be the visualization name) if not payload.type: raise exceptions.RequestParameterMissingException("key/value 'type' is required") @@ -184,9 +189,7 @@ def create( with transaction(session): session.commit() - rval = {"id": visualization.id} - - return VisualizationCreateResponse(**rval) + return VisualizationCreateResponse(id=str(visualization.id)) def update( self, @@ -201,7 +204,7 @@ def update( :returns: dictionary containing Visualization details """ rval = None - payload = self._validate_and_parse_payload(payload) + payload = cast(VisualizationUpdatePayload, self._validate_and_parse_payload(payload)) # there's a differentiation here between updating the visualization and creating a new revision # that needs to be handled clearly here or alternately, using a different controller @@ -212,19 +215,20 @@ def update( # only update owned visualizations visualization = self._get_visualization(trans, visualization_id, check_ownership=True) - title = payload.title or visualization.latest_revision.title - dbkey = payload.dbkey or visualization.latest_revision.dbkey + latest_revision = cast(VisualizationRevision, visualization.latest_revision) + title = payload.title or latest_revision.title + dbkey = payload.dbkey or latest_revision.dbkey deleted = payload.deleted or visualization.deleted - config = payload.config or visualization.latest_revision.config + config = payload.config or latest_revision.config - latest_config = visualization.latest_revision.config + latest_config = latest_revision.config if ( - (title != visualization.latest_revision.title) - or (dbkey != visualization.latest_revision.dbkey) + (title != latest_revision.title) + or (dbkey != latest_revision.dbkey) or (json.dumps(config) != json.dumps(latest_config)) ): revision = self._add_visualization_revision(trans, visualization, config, title, dbkey) - rval = {"id": visualization_id, "revision": revision.id} + rval = {"id": str(visualization_id), "revision": str(revision.id)} # allow updating vis title visualization.title = title @@ -346,9 +350,9 @@ def _add_visualization_revision( self, trans: ProvidesUserContext, visualization: Visualization, - config: dict, - title: str, - dbkey: str, + config: Optional[Union[dict, bytes]], + title: Optional[str], + dbkey: Optional[str], ) -> VisualizationRevision: """ Adds a new `VisualizationRevision` to the given `visualization` with @@ -376,7 +380,7 @@ def _create_visualization( dbkey: Optional[str] = None, slug: Optional[str] = None, annotation: Optional[str] = None, - save: bool = True, + save: Optional[bool] = True, ) -> Visualization: """Create visualization but not first revision. Returns Visualization object.""" user = trans.get_user() From d36c835259aee88e7bf697d06ef430d43a8aaa7b Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 20 Aug 2024 12:29:11 +0200 Subject: [PATCH 13/18] update openapi schema --- client/src/api/schema/schema.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 2e28f1c53f5c..e46624f381ca 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -16869,9 +16869,8 @@ export interface components { /** * Plugin * @description The plugin of this Visualization. - * @default {} */ - plugin: components["schemas"]["VisualizationPluginResponse"] | null; + plugin?: components["schemas"]["VisualizationPluginResponse"] | null; /** * Revisions * @description A list of encoded IDs of the revisions of this Visualization. @@ -16885,9 +16884,8 @@ export interface components { /** * Tags * @description A list of tags to add to this item. - * @default [] */ - tags: components["schemas"]["TagCollection"] | null; + tags?: components["schemas"]["TagCollection"] | null; /** * Title * @description The name of the visualization. @@ -16993,7 +16991,7 @@ export interface components { * @description The config of the visualization. * @default {} */ - config: Record | null; + config: Record | string | null; /** * DbKey * @description The database key of the visualization. From a7e260bd2c74c639500cb6e1c792b922975c4642 Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 20 Aug 2024 16:06:37 +0200 Subject: [PATCH 14/18] Update plugin schema --- client/src/api/schema/schema.ts | 4 ++-- lib/galaxy/schema/visualization.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index e46624f381ca..d0061ecb292b 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -16754,7 +16754,7 @@ export interface components { * Groups * @description The groups of the plugin. */ - groups: Record[]; + groups?: Record[] | null; /** * Href * @description The href of the plugin. @@ -16769,7 +16769,7 @@ export interface components { * Logo * @description The logo of the plugin. */ - logo: string; + logo?: string | null; /** * Name * @description The name of the plugin. diff --git a/lib/galaxy/schema/visualization.py b/lib/galaxy/schema/visualization.py index fe859a34b204..67f06f2c884c 100644 --- a/lib/galaxy/schema/visualization.py +++ b/lib/galaxy/schema/visualization.py @@ -153,8 +153,8 @@ class VisualizationPluginResponse(Model): title="Description", description="The description of the plugin.", ) - logo: str = Field( - ..., + logo: Optional[str] = Field( + None, title="Logo", description="The logo of the plugin.", ) @@ -183,8 +183,8 @@ class VisualizationPluginResponse(Model): title="Settings", description="The settings of the plugin.", ) - groups: List[Dict] = Field( - ..., + groups: Optional[List[Dict]] = Field( + None, title="Groups", description="The groups of the plugin.", ) From b73333a74a8095c7abc56ffa99ece2dd6eafedc7 Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 20 Aug 2024 17:27:58 +0200 Subject: [PATCH 15/18] Fix TestVisualizationTags and NotificationsIntegrationBase to send data as json to Visualization api --- lib/galaxy_test/api/test_tags.py | 12 +++++------- test/integration/test_notifications.py | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/galaxy_test/api/test_tags.py b/lib/galaxy_test/api/test_tags.py index 776f95db147d..035f675aecb5 100644 --- a/lib/galaxy_test/api/test_tags.py +++ b/lib/galaxy_test/api/test_tags.py @@ -155,12 +155,10 @@ def create_item(self) -> str: title = f"Test Visualization {uuid_str}" slug = f"test-visualization-{uuid_str}" - config = json.dumps( - { - "x": 10, - "y": 12, - } - ) + config = { + "x": 10, + "y": 12, + } create_payload = { "title": title, "slug": slug, @@ -169,7 +167,7 @@ def create_item(self) -> str: "annotation": "this is a test visualization for tags", "config": config, } - response = self._post("visualizations", data=create_payload) + response = self._post("visualizations", data=create_payload, json=True) self._assert_status_code_is(response, 200) viz = response.json() return viz["id"] diff --git a/test/integration/test_notifications.py b/test/integration/test_notifications.py index d44fcaea8da6..89077357f824 100644 --- a/test/integration/test_notifications.py +++ b/test/integration/test_notifications.py @@ -356,7 +356,7 @@ def test_sharing_items_creates_notifications_when_expected(self): "type": "example", "dbkey": "hg17", } - response = self._post("visualizations", data=create_payload).json() + response = self._post("visualizations", data=create_payload, json=True).json() visualization_id = response["id"] payload = {"user_ids": user_ids} sharing_response = self._put(f"visualizations/{visualization_id}/share_with_users", data=payload, json=True) From b441f979a72c6d320f4891a9f435f8bd66ca9209 Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 21 Aug 2024 14:33:48 +0200 Subject: [PATCH 16/18] Addressing the comments --- client/src/api/schema/schema.ts | 28 +++--- lib/galaxy/schema/visualization.py | 42 ++++---- .../webapps/galaxy/api/visualizations.py | 12 +-- .../webapps/galaxy/services/visualizations.py | 98 +++---------------- 4 files changed, 57 insertions(+), 123 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index d0061ecb292b..71172c780780 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -16696,11 +16696,6 @@ export interface components { * @description The database key of the visualization. */ dbkey?: string | null; - /** - * Import ID - * @description The ID of the imported visualization. - */ - import_id?: string | null; /** * Save * @description Whether to save the visualization. @@ -16815,11 +16810,12 @@ export interface components { */ id: string; /** - * Model Class - * @description The model class name for this object. - * @default VisualizationRevision + * Model class + * @description The name of the database model class. + * @constant + * @enum {string} */ - model_class: string; + model_class: "VisualizationRevision"; /** * Title * @description The name of the visualization revision. @@ -16861,11 +16857,12 @@ export interface components { */ latest_revision: components["schemas"]["VisualizationRevisionResponse"]; /** - * Model Class - * @description The model class name for this object. - * @default Visualization + * Model class + * @description The name of the database model class. + * @constant + * @enum {string} */ - model_class: string; + model_class: "Visualization"; /** * Plugin * @description The plugin of this Visualization. @@ -32441,7 +32438,10 @@ export interface operations { }; create_api_visualizations_post: { parameters: { - query?: never; + query?: { + /** @description The encoded database identifier of the Visualization to import. */ + import_id?: string | null; + }; header?: { /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ "run-as"?: string | null; diff --git a/lib/galaxy/schema/visualization.py b/lib/galaxy/schema/visualization.py index 67f06f2c884c..8b628a4843e8 100644 --- a/lib/galaxy/schema/visualization.py +++ b/lib/galaxy/schema/visualization.py @@ -9,6 +9,7 @@ from pydantic import ( ConfigDict, Field, + field_validator, RootModel, ) from typing_extensions import Literal @@ -16,16 +17,22 @@ from galaxy.schema.fields import ( DecodedDatabaseIdField, EncodedDatabaseIdField, + ModelClassField, ) from galaxy.schema.schema import ( CreateTimeField, Model, TagCollection, UpdateTimeField, + WithModelClass, ) +from galaxy.util.sanitize_html import sanitize_html VisualizationSortByEnum = Literal["create_time", "title", "update_time", "username"] +VISUALIZATION_MODEL_CLASS = Literal["Visualization"] +VISUALIZATION_REVISION_MODEL_CLASS = Literal["VisualizationRevision"] + class VisualizationIndexQueryPayload(Model): deleted: bool = False @@ -104,12 +111,8 @@ class VisualizationSummaryList(RootModel): ) -class VisualizationRevisionResponse(Model): - model_class: str = Field( - "VisualizationRevision", - title="Model Class", - description="The model class name for this object.", - ) +class VisualizationRevisionResponse(Model, WithModelClass): + model_class: VISUALIZATION_REVISION_MODEL_CLASS = ModelClassField(VISUALIZATION_REVISION_MODEL_CLASS) id: EncodedDatabaseIdField = Field( ..., title="ID", @@ -200,12 +203,8 @@ class VisualizationPluginResponse(Model): ) -class VisualizationShowResponse(Model): - model_class: str = Field( - "Visualization", - title="Model Class", - description="The model class name for this object.", - ) +class VisualizationShowResponse(Model, WithModelClass): + model_class: VISUALIZATION_MODEL_CLASS = ModelClassField(VISUALIZATION_MODEL_CLASS) id: EncodedDatabaseIdField = Field( ..., title="ID", @@ -300,11 +299,6 @@ class VisualizationUpdateResponse(Model): class VisualizationCreatePayload(Model): - import_id: Optional[DecodedDatabaseIdField] = Field( - None, - title="Import ID", - description="The ID of the imported visualization.", - ) type: Optional[str] = Field( None, title="Type", @@ -341,6 +335,13 @@ class VisualizationCreatePayload(Model): description="Whether to save the visualization.", ) + @field_validator("type", "title", "dbkey", "slug", "annotation", mode="before") + @classmethod + def sanitize_html_fields(cls, v): + if isinstance(v, str): + return sanitize_html(v) + return v + class VisualizationUpdatePayload(Model): title: Optional[str] = Field( @@ -363,3 +364,10 @@ class VisualizationUpdatePayload(Model): title="Config", description="The config of the visualization.", ) + + @field_validator("title", "dbkey", mode="before") + @classmethod + def sanitize_html_fields(cls, v): + if isinstance(v, str): + return sanitize_html(v) + return v diff --git a/lib/galaxy/webapps/galaxy/api/visualizations.py b/lib/galaxy/webapps/galaxy/api/visualizations.py index e76ed8a94e4b..39c53b2a6c8b 100644 --- a/lib/galaxy/webapps/galaxy/api/visualizations.py +++ b/lib/galaxy/webapps/galaxy/api/visualizations.py @@ -6,10 +6,7 @@ """ import logging -from typing import ( - Optional, - Union, -) +from typing import Optional from fastapi import ( Body, @@ -253,6 +250,9 @@ def show( def create( self, payload: VisualizationCreatePayload = Body(...), + import_id: Optional[DecodedDatabaseIdField] = Query( + None, title="Import ID", description="The encoded database identifier of the Visualization to import." + ), trans: ProvidesUserContext = DependsOnTrans, ) -> VisualizationCreateResponse: """ @@ -262,7 +262,7 @@ def create( POST /api/visualizations?import_id={encoded_visualization_id} imports a copy of an existing visualization into the user's workspace and does not require the rest of the payload """ - return self.service.create(trans, payload) + return self.service.create(trans, import_id, payload) @router.put( "/api/visualizations/{id}", @@ -273,5 +273,5 @@ def update( id: VisualizationIdPathParam, payload: VisualizationUpdatePayload = Body(...), trans: ProvidesUserContext = DependsOnTrans, - ) -> Union[VisualizationUpdateResponse, None]: + ) -> Optional[VisualizationUpdateResponse]: return self.service.update(trans, id, payload) diff --git a/lib/galaxy/webapps/galaxy/services/visualizations.py b/lib/galaxy/webapps/galaxy/services/visualizations.py index 07f782629184..c7536dd5323b 100644 --- a/lib/galaxy/webapps/galaxy/services/visualizations.py +++ b/lib/galaxy/webapps/galaxy/services/visualizations.py @@ -122,7 +122,7 @@ def show( "slug": visualization.slug, # to_dict only the latest revision (allow older to be fetched elsewhere) "latest_revision": ( - self._get_visualization_revision_dict(visualization.latest_revision) + self._get_visualization_revision(visualization.latest_revision) if visualization.latest_revision else None ), @@ -151,6 +151,7 @@ def show( def create( self, trans: ProvidesUserContext, + import_id: Optional[DecodedDatabaseIdField], payload: VisualizationCreatePayload, ) -> VisualizationCreateResponse: """Returns a dictionary of the created visualization @@ -159,10 +160,11 @@ def create( :returns: dictionary containing Visualization details """ - if payload.import_id: - visualization = self._import_visualization(trans, payload.import_id) + if import_id: + visualization = self._import_visualization(trans, import_id) else: - payload = cast(VisualizationCreatePayload, self._validate_and_parse_payload(payload)) + # custom validator to sanitize the HTML, and assign that type to those fields that require it: type, annotation, title, slug, dbkey + # must have a type (I've taken this to be the visualization name) if not payload.type: raise exceptions.RequestParameterMissingException("key/value 'type' is required") @@ -178,9 +180,7 @@ def create( visualization = self._create_visualization(trans, type, title, dbkey, slug, annotation, save) # Create and save first visualization revision - revision = trans.model.VisualizationRevision( - visualization=visualization, title=title, config=config, dbkey=dbkey - ) + revision = VisualizationRevision(visualization=visualization, title=title, config=config, dbkey=dbkey) visualization.latest_revision = revision if save: @@ -196,7 +196,7 @@ def update( trans: ProvidesUserContext, visualization_id: DecodedDatabaseIdField, payload: VisualizationUpdatePayload, - ) -> Union[VisualizationUpdateResponse, None]: + ) -> Optional[VisualizationUpdateResponse]: """ Update a visualization @@ -204,7 +204,6 @@ def update( :returns: dictionary containing Visualization details """ rval = None - payload = cast(VisualizationUpdatePayload, self._validate_and_parse_payload(payload)) # there's a differentiation here between updating the visualization and creating a new revision # that needs to be handled clearly here or alternately, using a different controller @@ -238,76 +237,6 @@ def update( return VisualizationUpdateResponse(**rval) if rval else None - def _validate_and_parse_payload( - self, - payload: Union[VisualizationCreatePayload, VisualizationUpdatePayload], - ) -> Union[VisualizationCreatePayload, VisualizationUpdatePayload]: - """ - Validate and parse incomming data payload for a visualization. - """ - # This layer handles (most of the stricter idiot proofing): - # - unknown/unallowed keys - # - changing data keys from api key to attribute name - # - protection against bad data form/type - # - protection against malicious data content - # all other conversions and processing (such as permissions, etc.) should happen down the line - - # keys listed here don't error when attempting to set, but fail silently - # this allows PUT'ing an entire model back to the server without attribute errors on uneditable attrs - valid_but_uneditable_keys = ( - "id", - "model_class", - # TODO: fill out when we create to_dict, get_dict, whatevs - ) - # TODO: importable - ValidationError = exceptions.RequestParameterInvalidException - - validated_payload = {} - for key, val in payload.model_dump().items(): - # By adding the pydatnic model there will be some variables that are not set and should be ignored in the validation - if val is None: - continue - # TODO: validate types in VALID_TYPES/registry names at the mixin/model level? - if key == "type": - if not isinstance(val, str): - raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") - val = sanitize_html(val) - elif key == "config": - if not isinstance(val, dict): - raise ValidationError(f"{key} must be a dictionary: {str(type(val))}") - elif key == "annotation": - if not isinstance(val, str): - raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") - val = sanitize_html(val) - elif key == "deleted": - if not isinstance(val, bool): - raise ValidationError(f"{key} must be a bool: {str(type(val))}") - - # these are keys that actually only be *updated* at the revision level and not here - # (they are still valid for create, tho) - elif key == "title": - if not isinstance(val, str): - raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") - val = sanitize_html(val) - elif key == "slug": - if not isinstance(val, str): - raise ValidationError(f"{key} must be a string: {str(type(val))}") - val = sanitize_html(val) - elif key == "dbkey": - if not isinstance(val, str): - raise ValidationError(f"{key} must be a string or unicode: {str(type(val))}") - val = sanitize_html(val) - - elif key not in valid_but_uneditable_keys: - continue - # raise AttributeError( 'unknown key: %s' %( str( key ) ) ) - - validated_payload[key] = val - if isinstance(payload, VisualizationCreatePayload): - return VisualizationCreatePayload(**validated_payload) - elif isinstance(payload, VisualizationUpdatePayload): - return VisualizationUpdatePayload(**validated_payload) - def _get_visualization( self, trans: ProvidesUserContext, @@ -318,7 +247,6 @@ def _get_visualization( """ Get a Visualization from the database by id, verifying ownership. """ - # Load workflow from database try: visualization = trans.sa_session.get(Visualization, visualization_id) except TypeError: @@ -328,7 +256,7 @@ def _get_visualization( else: return security_check(trans, visualization, check_ownership, check_accessible) - def _get_visualization_revision_dict( + def _get_visualization_revision( self, revision: VisualizationRevision, ) -> VisualizationRevisionResponse: @@ -361,9 +289,7 @@ def _add_visualization_revision( """ # precondition: only add new revision on owned vis's # TODO:?? should we default title, dbkey, config? to which: visualization or latest_revision? - revision = trans.model.VisualizationRevision( - visualization=visualization, title=title, dbkey=dbkey, config=config - ) + revision = VisualizationRevision(visualization=visualization, title=title, dbkey=dbkey, config=config) visualization.latest_revision = revision # TODO:?? does this automatically add revision to visualzation.revisions? @@ -391,7 +317,7 @@ def _create_visualization( title_err = "visualization name is required" elif slug and not is_valid_slug(slug): slug_err = "visualization identifier must consist of only lowercase letters, numbers, and the '-' character" - elif slug and slug_exists(trans.sa_session, trans.model.Visualization, user, slug, ignore_deleted=True): + elif slug and slug_exists(trans.sa_session, Visualization, user, slug, ignore_deleted=True): slug_err = "visualization identifier must be unique" if title_err or slug_err: @@ -400,7 +326,7 @@ def _create_visualization( raise exceptions.RequestParameterMissingException(val_err) # Create visualization - visualization = trans.model.Visualization(user=user, title=title, dbkey=dbkey, type=type) + visualization = Visualization(user=user, title=title, dbkey=dbkey, type=type) if slug: visualization.slug = slug else: From 0c6106c89ef90a779ad0841493d5585d07c56e91 Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 21 Aug 2024 14:35:59 +0200 Subject: [PATCH 17/18] Remove unnecessary comment --- lib/galaxy/webapps/galaxy/services/visualizations.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/services/visualizations.py b/lib/galaxy/webapps/galaxy/services/visualizations.py index c7536dd5323b..5eb9469609e0 100644 --- a/lib/galaxy/webapps/galaxy/services/visualizations.py +++ b/lib/galaxy/webapps/galaxy/services/visualizations.py @@ -163,8 +163,6 @@ def create( if import_id: visualization = self._import_visualization(trans, import_id) else: - # custom validator to sanitize the HTML, and assign that type to those fields that require it: type, annotation, title, slug, dbkey - # must have a type (I've taken this to be the visualization name) if not payload.type: raise exceptions.RequestParameterMissingException("key/value 'type' is required") From 33f744f33adae6b59093b9c33cdea0ac9e2b916c Mon Sep 17 00:00:00 2001 From: Arash Date: Mon, 2 Sep 2024 14:10:48 +0200 Subject: [PATCH 18/18] Using SanitizedString Type --- lib/galaxy/schema/schema.py | 22 ++++++++++++++++++++ lib/galaxy/schema/visualization.py | 33 ++++++++---------------------- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index e782cdb82092..1adc4a334a90 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -28,6 +28,7 @@ RootModel, UUID4, ) +from pydantic_core import core_schema from typing_extensions import ( Annotated, Literal, @@ -48,6 +49,7 @@ OffsetNaiveDatetime, RelativeUrl, ) +from galaxy.util.sanitize_html import sanitize_html USER_MODEL_CLASS = Literal["User"] GROUP_MODEL_CLASS = Literal["Group"] @@ -3805,3 +3807,23 @@ class PageSummaryList(RootModel): class MessageExceptionModel(BaseModel): err_msg: str err_code: int + + +class SanitizedString(str): + @classmethod + def __get_validators__(cls): + yield cls.validate + + @classmethod + def validate(cls, value): + if isinstance(value, str): + return cls(sanitize_html(value)) + raise TypeError("string required") + + @classmethod + def __get_pydantic_core_schema__(cls, source_type, handler): + return core_schema.no_info_after_validator_function( + cls.validate, + core_schema.str_schema(), + serialization=core_schema.to_string_ser_schema(), + ) diff --git a/lib/galaxy/schema/visualization.py b/lib/galaxy/schema/visualization.py index 8b628a4843e8..e1dcc0d50147 100644 --- a/lib/galaxy/schema/visualization.py +++ b/lib/galaxy/schema/visualization.py @@ -9,7 +9,6 @@ from pydantic import ( ConfigDict, Field, - field_validator, RootModel, ) from typing_extensions import Literal @@ -22,11 +21,11 @@ from galaxy.schema.schema import ( CreateTimeField, Model, + SanitizedString, TagCollection, UpdateTimeField, WithModelClass, ) -from galaxy.util.sanitize_html import sanitize_html VisualizationSortByEnum = Literal["create_time", "title", "update_time", "username"] @@ -299,27 +298,27 @@ class VisualizationUpdateResponse(Model): class VisualizationCreatePayload(Model): - type: Optional[str] = Field( + type: Optional[SanitizedString] = Field( None, title="Type", description="The type of the visualization.", ) - title: Optional[str] = Field( - "Untitled Visualization", + title: Optional[SanitizedString] = Field( + SanitizedString("Untitled Visualization"), title="Title", description="The name of the visualization.", ) - dbkey: Optional[str] = Field( + dbkey: Optional[SanitizedString] = Field( None, title="DbKey", description="The database key of the visualization.", ) - slug: Optional[str] = Field( + slug: Optional[SanitizedString] = Field( None, title="Slug", description="The slug of the visualization.", ) - annotation: Optional[str] = Field( + annotation: Optional[SanitizedString] = Field( None, title="Annotation", description="The annotation of the visualization.", @@ -335,21 +334,14 @@ class VisualizationCreatePayload(Model): description="Whether to save the visualization.", ) - @field_validator("type", "title", "dbkey", "slug", "annotation", mode="before") - @classmethod - def sanitize_html_fields(cls, v): - if isinstance(v, str): - return sanitize_html(v) - return v - class VisualizationUpdatePayload(Model): - title: Optional[str] = Field( + title: Optional[SanitizedString] = Field( None, title="Title", description="The name of the visualization.", ) - dbkey: Optional[str] = Field( + dbkey: Optional[SanitizedString] = Field( None, title="DbKey", description="The database key of the visualization.", @@ -364,10 +356,3 @@ class VisualizationUpdatePayload(Model): title="Config", description="The config of the visualization.", ) - - @field_validator("title", "dbkey", mode="before") - @classmethod - def sanitize_html_fields(cls, v): - if isinstance(v, str): - return sanitize_html(v) - return v