Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with generating slug for sharing #18986

Merged
Merged
2 changes: 1 addition & 1 deletion client/src/components/Sharing/SlugInput.vue
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const initialSlug = props.slug;
function onChange(value: string) {
const slugFormatted = value
.replace(/\s+/g, "-")
.replace(/[^a-zA-Z0-9-]/g, "")
.replace(/[/:?#]/g, "")
.toLowerCase();
emit("change", slugFormatted);
Expand Down
1 change: 1 addition & 0 deletions lib/galaxy/dependencies/pinned-typecheck-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ types-html5lib==1.1.11.20241018
types-markdown==3.7.0.20240822
types-paramiko==3.5.0.20240928
types-python-dateutil==2.9.0.20241003
types-python-slugify==8.0.2.20240310
types-pyyaml==6.0.12.20240917
types-requests==2.31.0.6
types-s3transfer==0.10.3
Expand Down
7 changes: 0 additions & 7 deletions lib/galaxy/managers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1294,13 +1294,6 @@ def raise_filter_err(attr, op, val, msg):
raise exceptions.RequestParameterInvalidException(msg, column=attr, operation=op, val=val)


def is_valid_slug(slug):
"""Returns true iff slug is valid."""

VALID_SLUG_RE = re.compile(r"^[a-z0-9\-]+$")
return VALID_SLUG_RE.match(slug)


class SortableManager:
"""A manager interface for parsing order_by strings into actual 'order by' queries."""

Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/managers/pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ def create_page(self, trans, payload: CreatePagePayload):
raise exceptions.ObjectAttributeMissingException("Page name is required")
elif not payload.slug:
raise exceptions.ObjectAttributeMissingException("Page id is required")
elif not base.is_valid_slug(payload.slug):
elif not sharable.SlugBuilder.is_valid_slug(payload.slug):
raise exceptions.ObjectAttributeInvalidException(
"Page identifier must consist of only lowercase letters, numbers, and the '-' character"
)
Expand Down
28 changes: 8 additions & 20 deletions lib/galaxy/managers/sharable.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"""

import logging
import re
from typing import (
Any,
List,
Expand All @@ -20,6 +19,7 @@
Type,
)

from slugify import slugify
from sqlalchemy import (
exists,
false,
Expand Down Expand Up @@ -291,7 +291,7 @@ def set_slug(self, item, new_slug, user, flush=True):
Validate and set the new slug for `item`.
"""
# precondition: has been validated
if not self.is_valid_slug(new_slug):
if not SlugBuilder.is_valid_slug(new_slug):
raise exceptions.RequestParameterInvalidException("Invalid slug", slug=new_slug)

if item.slug == new_slug:
Expand All @@ -309,23 +309,6 @@ def set_slug(self, item, new_slug, user, flush=True):
session.commit()
return item

def is_valid_slug(self, slug):
"""
Returns true if `slug` is valid.
"""
VALID_SLUG_RE = re.compile(r"^[a-z0-9\-]+$")
return VALID_SLUG_RE.match(slug)

def _slugify(self, start_with):
# Replace whitespace with '-'
slug_base = re.sub(r"\s+", "-", start_with)
# Remove all non-alphanumeric characters.
slug_base = re.sub(r"[^a-zA-Z0-9\-]", "", slug_base)
# Remove trailing '-'.
if slug_base.endswith("-"):
slug_base = slug_base[:-1]
return slug_base

def _default_slug_base(self, item):
# override in subclasses
if hasattr(item, "title"):
Expand All @@ -341,7 +324,7 @@ def get_unique_slug(self, item):

# Setup slug base.
if cur_slug is None or cur_slug == "":
slug_base = self._slugify(self._default_slug_base(item))
slug_base = slugify(self._default_slug_base(item), allow_unicode=True)
else:
slug_base = cur_slug

Expand Down Expand Up @@ -580,6 +563,11 @@ def create_item_slug(self, sa_session, item) -> bool:
item.slug = new_slug
return item.slug == cur_slug

@classmethod
def is_valid_slug(self, slug):
"""Returns true if slug is valid."""
return slugify(slug, allow_unicode=True) == slug


def slug_exists(session, model_class, user, slug, ignore_deleted=False):
stmt = select(exists().where(model_class.user == user).where(model_class.slug == slug))
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/schema/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -3678,7 +3678,7 @@ class PageSummaryBase(Model):
..., # Required
title="Identifier",
description="The title slug for the page URL, must be unique.",
pattern=r"^[a-z0-9\-]+$",
pattern=r"^[^/:?#]+$",
)


Expand Down
7 changes: 6 additions & 1 deletion lib/galaxy/webapps/base/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -1090,7 +1090,7 @@ class SharableMixin:

def _is_valid_slug(self, slug):
"""Returns true if slug is valid."""
return managers_base.is_valid_slug(slug)
return SlugBuilder.is_valid_slug(slug)

@web.expose
@web.require_login("modify Galaxy items")
Expand Down Expand Up @@ -1123,6 +1123,11 @@ def share(self, trans, id=None, email="", **kwd):
@web.expose
def display_by_username_and_slug(self, trans, username, slug, **kwargs):
"""Display item by username and slug."""
# Ensure slug is in the correct format.
slug = slug.encode("latin1").decode("utf-8")
self._display_by_username_and_slug(trans, username, slug, **kwargs)

def _display_by_username_and_slug(self, trans, username, slug, **kwargs):
raise NotImplementedError()

def get_item(self, trans, id):
Expand Down
3 changes: 1 addition & 2 deletions lib/galaxy/webapps/galaxy/controllers/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,8 +431,7 @@ def _get_dataset_for_edit(self, trans, dataset_id):
trans.app.security_agent.set_dataset_permission(data.dataset, permissions)
return data, None

@web.expose
def display_by_username_and_slug(self, trans, username, slug, filename=None, preview=True, **kwargs):
def _display_by_username_and_slug(self, trans, username, slug, filename=None, preview=True, **kwargs):
"""Display dataset by username and slug; because datasets do not yet have slugs, the slug is the dataset's id."""
dataset = self._check_dataset(trans, slug)
# Filename used for composite types.
Expand Down
3 changes: 1 addition & 2 deletions lib/galaxy/webapps/galaxy/controllers/history.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ def view(self, trans, id=None, show_deleted=False, show_hidden=False, use_panels
"allow_user_dataset_purge": trans.app.config.allow_user_dataset_purge,
}

@web.expose
def display_by_username_and_slug(self, trans, username, slug, **kwargs):
def _display_by_username_and_slug(self, trans, username, slug, **kwargs):
"""
Display history based on a username and slug.
"""
Expand Down
3 changes: 1 addition & 2 deletions lib/galaxy/webapps/galaxy/controllers/page.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,7 @@ def display(self, trans, id, **kwargs):
raise web.httpexceptions.HTTPNotFound()
return self.display_by_username_and_slug(trans, page.user.username, page.slug)

@web.expose
def display_by_username_and_slug(self, trans, username, slug, **kwargs):
def _display_by_username_and_slug(self, trans, username, slug, **kwargs):
"""Display page based on a username and slug."""

# Get page.
Expand Down
3 changes: 1 addition & 2 deletions lib/galaxy/webapps/galaxy/controllers/visualization.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ def imp(self, trans, id, **kwargs):
use_panels=True,
)

@web.expose
def display_by_username_and_slug(self, trans, username, slug, **kwargs):
def _display_by_username_and_slug(self, trans, username, slug, **kwargs):
"""Display visualization based on a username and slug."""

# Get visualization.
Expand Down
3 changes: 1 addition & 2 deletions lib/galaxy/webapps/galaxy/controllers/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ class WorkflowController(BaseUIController, SharableMixin, UsesStoredWorkflowMixi
def __init__(self, app: StructuredApp):
super().__init__(app)

@web.expose
def display_by_username_and_slug(self, trans, username, slug, format="html", **kwargs):
def _display_by_username_and_slug(self, trans, username, slug, format="html", **kwargs):
"""
Display workflow based on a username and slug. Format can be html, json, or json-download.
"""
Expand Down
7 changes: 2 additions & 5 deletions lib/galaxy/webapps/galaxy/services/visualizations.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@
)

from galaxy import exceptions
from galaxy.managers.base import (
is_valid_slug,
security_check,
)
from galaxy.managers.base import security_check
from galaxy.managers.context import ProvidesUserContext
from galaxy.managers.sharable import (
slug_exists,
Expand Down Expand Up @@ -302,7 +299,7 @@ def _create_visualization(
# Error checking.
if slug:
slug_err = ""
if not is_valid_slug(slug):
if not SlugBuilder.is_valid_slug(slug):
slug_err = (
"visualization identifier must consist of only lowercase letters, numbers, and the '-' character"
)
Expand Down
6 changes: 6 additions & 0 deletions lib/galaxy_test/api/test_histories.py
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,12 @@ def test_anonymous_can_import_published(self):
}
self.dataset_populator.import_history(import_data)

def test_publish_non_alphanumeric(self):
history_name = "تاریخچه"
history_id = self.dataset_populator.new_history(name=history_name)
response = self.dataset_populator.make_public(history_id)
assert history_name in response["username_and_slug"]

def test_immutable_history_update_fails(self):
history_id = self._create_history("TestHistoryForImmutability")["id"]

Expand Down
1 change: 1 addition & 0 deletions packages/app/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ install_requires =
pulsar-galaxy-lib>=0.15.0.dev0
pydantic>=2.7.4
pysam>=0.21
python-slugify
PyJWT
PyYAML
refgenconf>=0.12.0
Expand Down
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ dependencies = [
"python-dateutil",
"python-magic",
"python-multipart", # required to support form parsing in FastAPI/Starlette
"python-slugify",
"PyYAML",
"refgenconf>=0.12.0",
"regex",
Expand Down Expand Up @@ -172,6 +173,7 @@ typecheck = [
"types-Markdown",
"types-paramiko",
"types-python-dateutil",
"types-python-slugify",
"types-PyYAML",
"types-requests",
"types-setuptools",
Expand Down
Loading