Skip to content

Commit

Permalink
Merge pull request galaxyproject#18773 from nsoranzo/ld_update
Browse files Browse the repository at this point in the history
Refactor ``LibraryDatasetsManager`` and fix type annotation issue
  • Loading branch information
mvdbeek committed Sep 7, 2024
2 parents 4e47fce + c065fa9 commit b669011
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 101 deletions.
6 changes: 3 additions & 3 deletions lib/galaxy/managers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,18 +503,18 @@ def copy(self, item, **kwargs) -> U:
"""
raise exceptions.NotImplemented("Abstract method")

def update(self, item, new_values, flush=True, **kwargs) -> U:
def update(self, item: U, new_values: Dict[str, Any], flush: bool = True, **kwargs) -> U:
"""
Given a dictionary of new values, update `item` and return it.
..note: NO validation or deserialization occurs here.
"""
self.session().add(item)
for key, value in new_values.items():
if hasattr(item, key):
setattr(item, key, value)
session = self.session()
session.add(item)
if flush:
session = self.session()
with transaction(session):
session.commit()
return item
Expand Down
85 changes: 43 additions & 42 deletions lib/galaxy/managers/datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from galaxy.model import (
Dataset,
DatasetHash,
DatasetInstance,
)
from galaxy.model.base import transaction
from galaxy.schema.tasks import (
Expand All @@ -45,12 +46,12 @@
T = TypeVar("T")


class DatasetManager(base.ModelManager[model.Dataset], secured.AccessibleManagerMixin, deletable.PurgableManagerMixin):
class DatasetManager(base.ModelManager[Dataset], secured.AccessibleManagerMixin, deletable.PurgableManagerMixin):
"""
Manipulate datasets: the components contained in DatasetAssociations/DatasetInstances/HDAs/LDDAs
"""

model_class = model.Dataset
model_class = Dataset
foreign_key_name = "dataset"
app: MinimalManagerApp

Expand All @@ -62,7 +63,6 @@ def __init__(self, app: MinimalManagerApp):
# needed for admin test
self.user_manager = users.UserManager(app)
self.quota_agent = app.quota_agent
self.security_agent = app.model.security_agent

def copy(self, item, **kwargs):
raise exceptions.NotImplemented("Datasets cannot be copied")
Expand Down Expand Up @@ -142,7 +142,7 @@ def update_object_store_id(self, trans, dataset, object_store_id: str):
"Cannot swap object store IDs for object stores that don't share a device ID."
)

if not self.security_agent.can_change_object_store_id(trans.user, dataset):
if not self.app.security_agent.can_change_object_store_id(trans.user, dataset):
# TODO: probably want separate exceptions for doesn't own the dataset and dataset
# has been shared.
raise exceptions.InsufficientPermissionsException("Cannot change dataset permissions...")
Expand All @@ -152,7 +152,7 @@ def update_object_store_id(self, trans, dataset, object_store_id: str):
new_label = quota_source_map.get_quota_source_label(new_object_store_id)
if old_label != new_label:
self.quota_agent.relabel_quota_for_dataset(dataset, old_label, new_label)
sa_session = self.app.model.context
sa_session = self.session()
with transaction(sa_session):
dataset.object_store_id = new_object_store_id
sa_session.add(dataset)
Expand Down Expand Up @@ -321,7 +321,7 @@ def serialize_permissions(self, item, key, user=None, **context):

# ============================================================================= AKA DatasetInstanceManager
class DatasetAssociationManager(
base.ModelManager[model.DatasetInstance],
base.ModelManager[DatasetInstance],
secured.AccessibleManagerMixin,
secured.OwnableManagerMixin,
deletable.PurgableManagerMixin,
Expand All @@ -338,7 +338,7 @@ class DatasetAssociationManager(

# NOTE: model_manager_class should be set in HDA/LDA subclasses

def __init__(self, app):
def __init__(self, app: MinimalManagerApp):
super().__init__(app)
self.dataset_manager = DatasetManager(app)

Expand Down Expand Up @@ -404,7 +404,7 @@ def stop_creating_job(self, dataset_assoc, flush=False):

# Optimize this to skip other checks if this dataset is terminal - we can infer the
# job is already complete.
if dataset_assoc.state in model.Dataset.terminal_states:
if dataset_assoc.state in Dataset.terminal_states:
return False

if dataset_assoc.parent_id is None and len(dataset_assoc.creating_job_associations) > 0:
Expand Down Expand Up @@ -438,7 +438,7 @@ def extra_files(self, dataset_assoc):
return []
return glob.glob(os.path.join(dataset_assoc.dataset.extra_files_path, "*"))

def serialize_dataset_association_roles(self, trans, dataset_assoc):
def serialize_dataset_association_roles(self, dataset_assoc):
if hasattr(dataset_assoc, "library_dataset_dataset_association"):
library_dataset = dataset_assoc
dataset = library_dataset.library_dataset_dataset_association.dataset
Expand All @@ -447,25 +447,24 @@ def serialize_dataset_association_roles(self, trans, dataset_assoc):
dataset = dataset_assoc.dataset

# Omit duplicated roles by converting to set
security_agent = trans.app.security_agent
access_roles = set(dataset.get_access_roles(security_agent))
manage_roles = set(dataset.get_manage_permissions_roles(security_agent))
access_roles = set(dataset.get_access_roles(self.app.security_agent))
manage_roles = set(dataset.get_manage_permissions_roles(self.app.security_agent))

access_dataset_role_list = [
(access_role.name, trans.security.encode_id(access_role.id)) for access_role in access_roles
(access_role.name, self.app.security.encode_id(access_role.id)) for access_role in access_roles
]
manage_dataset_role_list = [
(manage_role.name, trans.security.encode_id(manage_role.id)) for manage_role in manage_roles
(manage_role.name, self.app.security.encode_id(manage_role.id)) for manage_role in manage_roles
]
rval = dict(access_dataset_roles=access_dataset_role_list, manage_dataset_roles=manage_dataset_role_list)
if library_dataset is not None:
modify_roles = set(
security_agent.get_roles_for_action(
library_dataset, trans.app.security_agent.permitted_actions.LIBRARY_MODIFY
self.app.security_agent.get_roles_for_action(
library_dataset, self.app.security_agent.permitted_actions.LIBRARY_MODIFY
)
)
modify_item_role_list = [
(modify_role.name, trans.security.encode_id(modify_role.id)) for modify_role in modify_roles
(modify_role.name, self.app.security.encode_id(modify_role.id)) for modify_role in modify_roles
]
rval["modify_item_roles"] = modify_item_role_list
return rval
Expand Down Expand Up @@ -493,7 +492,7 @@ def ensure_dataset_on_disk(self, trans, dataset):
"The dataset you are attempting to view is in paused state. One of the inputs for the job that creates this dataset has failed."
)

def ensure_can_change_datatype(self, dataset: model.DatasetInstance, raiseException: bool = True) -> bool:
def ensure_can_change_datatype(self, dataset: DatasetInstance, raiseException: bool = True) -> bool:
if not dataset.datatype.is_datatype_change_allowed():
if not raiseException:
return False
Expand All @@ -502,7 +501,7 @@ def ensure_can_change_datatype(self, dataset: model.DatasetInstance, raiseExcept
)
return True

def ensure_can_set_metadata(self, dataset: model.DatasetInstance, raiseException: bool = True) -> bool:
def ensure_can_set_metadata(self, dataset: DatasetInstance, raiseException: bool = True) -> bool:
if not dataset.ok_to_edit_metadata():
if not raiseException:
return False
Expand All @@ -511,29 +510,31 @@ def ensure_can_set_metadata(self, dataset: model.DatasetInstance, raiseException
)
return True

def detect_datatype(self, trans, dataset_assoc):
def detect_datatype(self, trans, dataset_assoc: DatasetInstance):
"""Sniff and assign the datatype to a given dataset association (ldda or hda)"""
data = trans.sa_session.get(self.model_class, dataset_assoc.id)
self.ensure_can_change_datatype(data)
self.ensure_can_set_metadata(data)
path = data.dataset.get_file_name()
datatype = sniff.guess_ext(path, trans.app.datatypes_registry.sniff_order)
trans.app.datatypes_registry.change_datatype(data, datatype)
with transaction(trans.sa_session):
trans.sa_session.commit()
session = self.session()
self.ensure_can_change_datatype(dataset_assoc)
self.ensure_can_set_metadata(dataset_assoc)
assert dataset_assoc.dataset
path = dataset_assoc.dataset.get_file_name()
datatype = sniff.guess_ext(path, self.app.datatypes_registry.sniff_order)
self.app.datatypes_registry.change_datatype(dataset_assoc, datatype)
with transaction(session):
session.commit()
self.set_metadata(trans, dataset_assoc)

def set_metadata(self, trans, dataset_assoc, overwrite=False, validate=True):
def set_metadata(
self, trans, dataset_assoc: DatasetInstance, overwrite: bool = False, validate: bool = True
) -> None:
"""Trigger a job that detects and sets metadata on a given dataset association (ldda or hda)"""
data = trans.sa_session.get(self.model_class, dataset_assoc.id)
self.ensure_can_set_metadata(data)
self.ensure_can_set_metadata(dataset_assoc)
if overwrite:
self.overwrite_metadata(data)
self.overwrite_metadata(dataset_assoc)

job, *_ = self.app.datatypes_registry.set_external_metadata_tool.tool_action.execute_via_trans(
self.app.datatypes_registry.set_external_metadata_tool,
trans,
incoming={"input1": data, "validate": validate},
incoming={"input1": dataset_assoc, "validate": validate},
overwrite=overwrite,
)
self.app.job_manager.enqueue(job, tool=self.app.datatypes_registry.set_external_metadata_tool)
Expand All @@ -560,26 +561,26 @@ def update_permissions(self, trans, dataset_assoc, **kwd):
dataset = dataset_assoc.dataset

current_user_roles = trans.get_current_user_roles()
can_manage = trans.app.security_agent.can_manage_dataset(current_user_roles, dataset) or trans.user_is_admin
can_manage = self.app.security_agent.can_manage_dataset(current_user_roles, dataset) or trans.user_is_admin
if not can_manage:
raise exceptions.InsufficientPermissionsException(
"You do not have proper permissions to manage permissions on this dataset."
)

if action == "remove_restrictions":
trans.app.security_agent.make_dataset_public(dataset)
if not trans.app.security_agent.dataset_is_public(dataset):
self.app.security_agent.make_dataset_public(dataset)
if not self.app.security_agent.dataset_is_public(dataset):
raise exceptions.InternalServerError("An error occurred while making dataset public.")
elif action == "make_private":
if not trans.app.security_agent.dataset_is_private_to_user(trans, dataset):
private_role = trans.app.security_agent.get_private_user_role(trans.user)
dp = trans.app.model.DatasetPermissions(
trans.app.security_agent.permitted_actions.DATASET_ACCESS.action, dataset, private_role
if not self.app.security_agent.dataset_is_private_to_user(trans, dataset):
private_role = self.app.security_agent.get_private_user_role(trans.user)
dp = self.app.model.DatasetPermissions(
self.app.security_agent.permitted_actions.DATASET_ACCESS.action, dataset, private_role
)
trans.sa_session.add(dp)
with transaction(trans.sa_session):
trans.sa_session.commit()
if not trans.app.security_agent.dataset_is_private_to_user(trans, dataset):
if not self.app.security_agent.dataset_is_private_to_user(trans, dataset):
# Check again and inform the user if dataset is not private.
raise exceptions.InternalServerError("An error occurred and the dataset is NOT private.")
elif action == "set_permissions":
Expand Down Expand Up @@ -869,7 +870,7 @@ def deserialize_datatype(self, item, key, val, **context):
job, *_ = self.app.datatypes_registry.set_external_metadata_tool.tool_action.execute_via_trans(
self.app.datatypes_registry.set_external_metadata_tool, trans, incoming={"input1": item}, overwrite=False
) # overwrite is False as per existing behavior
trans.app.job_manager.enqueue(job, tool=trans.app.datatypes_registry.set_external_metadata_tool)
self.app.job_manager.enqueue(job, tool=self.app.datatypes_registry.set_external_metadata_tool)
return item.datatype


Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/managers/folders.py
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ def _get_contained_datasets_statement(
stmt = stmt.where(
or_(
func.lower(ldda.name).contains(search_text, autoescape=True),
func.lower(ldda.message).contains(search_text, autoescape=True), # type:ignore[attr-defined]
func.lower(ldda.message).contains(search_text, autoescape=True),
)
)
sort_column = LDDA_SORT_COLUMN_MAP[payload.order_by](ldda, associated_dataset)
Expand Down
Loading

0 comments on commit b669011

Please sign in to comment.