From f08ab1c8b4809c343e038ec98f1e1784e6506b34 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Tue, 10 Oct 2023 11:40:45 +0100 Subject: [PATCH 01/19] Fix duplicated cache key --- .github/workflows/toolshed.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/toolshed.yaml b/.github/workflows/toolshed.yaml index f6c06663da3c..21303063345a 100644 --- a/.github/workflows/toolshed.yaml +++ b/.github/workflows/toolshed.yaml @@ -68,7 +68,6 @@ jobs: with: path: 'galaxy root/.venv' key: gxy-venv-${{ runner.os }}-${{ steps.full-python-version.outputs.version }}-${{ hashFiles('galaxy root/requirements.txt') }}-toolshed - key: gxy-venv-${{ runner.os }}-${{ steps.full-python-version.outputs.version }}-${{ hashFiles('galaxy_root/requirements.txt') }}-toolshed - name: Install dependencies run: ./scripts/common_startup.sh --skip-client-build working-directory: 'galaxy root' From 72629a4f8edc95858a110dca8cc1c1911af66ed0 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Tue, 10 Oct 2023 11:43:02 +0100 Subject: [PATCH 02/19] Expand matrix to test all combinations on Python 3.7 and 3.11 --- .github/workflows/toolshed.yaml | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/.github/workflows/toolshed.yaml b/.github/workflows/toolshed.yaml index 21303063345a..ce985340dd0a 100644 --- a/.github/workflows/toolshed.yaml +++ b/.github/workflows/toolshed.yaml @@ -19,24 +19,11 @@ jobs: name: Test runs-on: ubuntu-latest strategy: + fail-fast: false matrix: - include: - - test-install-client: 'galaxy_api' - python-version: '3.7' - shed-api: 'v1' - shed-browser: 'twill' - - test-install-client: 'standalone' - python-version: '3.8' - shed-api: 'v1' - shed-browser: 'twill' - - test-install-client: 'galaxy_api' - python-version: '3.9' - shed-api: 'v2' - shed-browser: 'playwright' - - test-install-client: 'standalone' - python-version: '3.10' - shed-api: 'v2' - shed-browser: 'playwright' + python-version: ['3.7', '3.11'] + shed-api: ['v1', 'v2'] + test-install-client: ['galaxy_api', 'standalone'] services: postgres: image: postgres:13 @@ -82,10 +69,10 @@ jobs: env: TOOL_SHED_TEST_INSTALL_CLIENT: ${{ matrix.test-install-client }} TOOL_SHED_API_VERSION: ${{ matrix.shed-api }} - TOOL_SHED_TEST_BROWSER: ${{ matrix.shed-browser }} + TOOL_SHED_TEST_BROWSER: ${{ matrix.shed-api == 'v1' && 'twill' || 'playwright' }} working-directory: 'galaxy root' - uses: actions/upload-artifact@v3 if: failure() with: - name: Toolshed test results (${{ matrix.python-version }}, ${{ matrix.test-install-client }}) + name: Toolshed test results (${{ matrix.python-version }}, ${{ matrix.shed-api }}, ${{ matrix.test-install-client }}) path: 'galaxy root/run_toolshed_tests.html' From 60aca1657d4ac01d3a1873529c09984709393ed8 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Tue, 10 Oct 2023 11:43:56 +0100 Subject: [PATCH 03/19] Expand ``run:`` values to multiple lines --- .github/workflows/toolshed.yaml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/toolshed.yaml b/.github/workflows/toolshed.yaml index ce985340dd0a..2b047a4676be 100644 --- a/.github/workflows/toolshed.yaml +++ b/.github/workflows/toolshed.yaml @@ -59,13 +59,19 @@ jobs: run: ./scripts/common_startup.sh --skip-client-build working-directory: 'galaxy root' - name: Build Frontend - run: '. .venv/bin/activate && cd lib/tool_shed/webapp/frontend && yarn && make client' + run: | + . .venv/bin/activate + cd lib/tool_shed/webapp/frontend + yarn + make client working-directory: 'galaxy root' - name: Install playwright - run: '. .venv/bin/activate && playwright install' + run: | + . .venv/bin/activate + playwright install working-directory: 'galaxy root' - name: Run tests - run: './run_tests.sh -toolshed' + run: ./run_tests.sh -toolshed env: TOOL_SHED_TEST_INSTALL_CLIENT: ${{ matrix.test-install-client }} TOOL_SHED_API_VERSION: ${{ matrix.shed-api }} From 9a5a552ca74ae4b6f0680e4de5b8a80ac3043b3b Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Tue, 10 Oct 2023 12:07:23 +0100 Subject: [PATCH 04/19] Fix issues reported by ``yarn build`` Introduced in https://github.com/galaxyproject/galaxy/pull/16787 . --- lib/tool_shed/webapp/frontend/src/apiUtil.ts | 2 +- .../webapp/frontend/src/components/pages/RepositoryPage.vue | 2 +- lib/tool_shed/webapp/frontend/src/stores/users.store.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/tool_shed/webapp/frontend/src/apiUtil.ts b/lib/tool_shed/webapp/frontend/src/apiUtil.ts index f14774767f97..9dfa3df9109d 100644 --- a/lib/tool_shed/webapp/frontend/src/apiUtil.ts +++ b/lib/tool_shed/webapp/frontend/src/apiUtil.ts @@ -2,7 +2,7 @@ import axios from "axios" import { RawAxiosRequestConfig } from "axios" import { components } from "@/schema" -type User = components["schemas"]["User"] +type User = components["schemas"]["UserV2"] export async function getCurrentUser(): Promise { const conf: RawAxiosRequestConfig = {} diff --git a/lib/tool_shed/webapp/frontend/src/components/pages/RepositoryPage.vue b/lib/tool_shed/webapp/frontend/src/components/pages/RepositoryPage.vue index 51929fc44d31..d337877ce8bd 100644 --- a/lib/tool_shed/webapp/frontend/src/components/pages/RepositoryPage.vue +++ b/lib/tool_shed/webapp/frontend/src/components/pages/RepositoryPage.vue @@ -242,7 +242,7 @@ const toolsYaml = computed( diff --git a/lib/tool_shed/webapp/frontend/src/stores/users.store.ts b/lib/tool_shed/webapp/frontend/src/stores/users.store.ts index 13cb403f801f..5e32f2042178 100644 --- a/lib/tool_shed/webapp/frontend/src/stores/users.store.ts +++ b/lib/tool_shed/webapp/frontend/src/stores/users.store.ts @@ -3,7 +3,7 @@ import { defineStore } from "pinia" import { fetcher, components } from "@/schema" const usersFetcher = fetcher.path("/api/users").method("get").create() -type User = components["schemas"]["User"] +type User = components["schemas"]["UserV2"] export const useUsersStore = defineStore({ id: "users", From 7dc9be43bf0e9e36d6f6e67ff273739dd5993136 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Tue, 10 Oct 2023 12:47:56 +0100 Subject: [PATCH 05/19] Explicitly add playwright to dev dependencies --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 4794d781629d..f0729a813c1f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -140,6 +140,7 @@ lxml = "!=4.2.2" markdown-it-reporter = "*" myst-parser = "*" pkce = "*" +playwright = "*" pytest = "*" pytest-asyncio = "*" pytest-celery = "*" From b1d86d4bad7a96c9c9b0ac0de1f97b189b41f993 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Tue, 10 Oct 2023 18:15:01 +0100 Subject: [PATCH 06/19] Check that TOOL_SHED_TEST_BROWSER is a valid value --- lib/tool_shed/test/functional/conftest.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/tool_shed/test/functional/conftest.py b/lib/tool_shed/test/functional/conftest.py index 9798b868a212..2b547bfe4b51 100644 --- a/lib/tool_shed/test/functional/conftest.py +++ b/lib/tool_shed/test/functional/conftest.py @@ -10,13 +10,12 @@ Browser, BrowserContext, ) -from typing_extensions import Literal from ..base.browser import ShedBrowser from ..base.playwrightbrowser import PlaywrightShedBrowser from ..base.twillbrowser import TwillShedBrowser -DEFAULT_BROWSER: Literal["twill", "playwright"] = "playwright" +DEFAULT_BROWSER = "playwright" def twill_browser() -> Generator[ShedBrowser, None, None]: @@ -28,10 +27,13 @@ def playwright_browser(class_context: BrowserContext) -> Generator[ShedBrowser, yield PlaywrightShedBrowser(page) -if os.environ.get("TOOL_SHED_TEST_BROWSER", DEFAULT_BROWSER) == "twill": +test_browser = os.environ.get("TOOL_SHED_TEST_BROWSER", DEFAULT_BROWSER) +if test_browser == "twill": shed_browser = pytest.fixture(scope="class")(twill_browser) -else: +elif test_browser == "playwright": shed_browser = pytest.fixture(scope="class")(playwright_browser) +else: + raise ValueError(f"Unrecognized value for TOOL_SHED_TEST_BROWSER: {test_browser}") @pytest.fixture(scope="class") From ed0c72e3c4b32b4b998218b5196622a72d8fd461 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 11 Oct 2023 10:44:54 +0100 Subject: [PATCH 07/19] Make owner and name parameters of ``get_or_create_repository()`` not optional --- lib/tool_shed/test/base/twilltestcase.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/tool_shed/test/base/twilltestcase.py b/lib/tool_shed/test/base/twilltestcase.py index 2d9b5e25c86f..d03a5041fee2 100644 --- a/lib/tool_shed/test/base/twilltestcase.py +++ b/lib/tool_shed/test/base/twilltestcase.py @@ -764,7 +764,7 @@ def login( ) # v2 doesn't log you in on account creation... so force a login here if previously_created or self.is_v2: - # The acount has previously been created, so just login. + # The account has previously been created, so just login. # HACK: don't use panels because late_javascripts() messes up the twill browser and it # can't find form fields (and hence user can't be logged in). params = {"use_panels": False} @@ -1402,21 +1402,20 @@ def get_repositories_category_api( self.check_for_strings(strings_displayed, strings_not_displayed) def get_or_create_repository( - self, category: Category, owner=None, strings_displayed=None, strings_not_displayed=None, **kwd + self, category: Category, owner: str, name: str, strings_displayed=None, strings_not_displayed=None, **kwd ) -> Optional[Repository]: # If not checking for a specific string, it should be safe to assume that # we expect repository creation to be successful. if strings_displayed is None: - strings_displayed = ["Repository", kwd["name"], "has been created"] + strings_displayed = ["Repository", name, "has been created"] if strings_not_displayed is None: strings_not_displayed = [] - name = kwd["name"] repository = self.populator.get_repository_for(owner, name) - category_id = category.id - assert category_id if repository is None: + category_id = category.id + assert category_id self.visit_url("/repository/create_repository") - self.submit_form(button="create_repository_button", category_id=category_id, **kwd) + self.submit_form(button="create_repository_button", name=name, category_id=category_id, **kwd) self.check_for_strings(strings_displayed, strings_not_displayed) repository = self.populator.get_repository_for(owner, name) return repository From 219bd20320df5adfc491860725484f3ea3091f19 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 18 Oct 2023 15:20:43 +0100 Subject: [PATCH 08/19] Fix imports Broken in commit 3238b212805f35aa17906ee51296a08b491e0e4c . --- lib/tool_shed/webapp/controllers/repository.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/tool_shed/webapp/controllers/repository.py b/lib/tool_shed/webapp/controllers/repository.py index ce313c9d0821..110838cbabae 100644 --- a/lib/tool_shed/webapp/controllers/repository.py +++ b/lib/tool_shed/webapp/controllers/repository.py @@ -16,10 +16,6 @@ null, select, ) -from toolshed.webapp.model import ( - Repository, - RepositoryMetadata, -) import tool_shed.grids.repository_grids as repository_grids import tool_shed.grids.util as grids_util @@ -62,7 +58,9 @@ from tool_shed.webapp.framework.decorators import require_login from tool_shed.webapp.model import ( Category, + Repository, RepositoryCategoryAssociation, + RepositoryMetadata, ) from tool_shed.webapp.util import ratings_util From bbb239a76be0d6fbda3cd1e1927a34e1000647fd Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 18 Oct 2023 15:29:46 +0100 Subject: [PATCH 09/19] Install node from setup-node action --- .github/workflows/toolshed.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/toolshed.yaml b/.github/workflows/toolshed.yaml index 2b047a4676be..4e3a2b2cac8c 100644 --- a/.github/workflows/toolshed.yaml +++ b/.github/workflows/toolshed.yaml @@ -37,6 +37,11 @@ jobs: - uses: actions/checkout@v3 with: path: 'galaxy root' + - uses: actions/setup-node@v3 + with: + node-version: '18.12.1' + cache: 'yarn' + cache-dependency-path: 'galaxy root/client/yarn.lock' - uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} From 14e3ae32ed43c3e57ea69e5a571dedf2d2a92477 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 18 Oct 2023 13:34:10 +0100 Subject: [PATCH 10/19] Restore GET /api/categories/ in ToolShed 2.0 and the deleted field in the response. Rationale: it is not particularly useful, but both: GET /api/categories GET /api/categories//repositories work on TS 2.0, so it's weird that this one is missing. Also, it was breaking BioBlend's https://bioblend.readthedocs.io/en/latest/api_docs/toolshed/all.html#bioblend.toolshed.categories.ToolShedCategoryClient.show_category --- lib/tool_shed/managers/categories.py | 4 +++ lib/tool_shed/test/base/populators.py | 7 ++-- .../test/functional/test_shed_categories.py | 13 ++++++- lib/tool_shed/webapp/api2/categories.py | 12 +++++++ .../webapp/frontend/src/schema/schema.ts | 35 +++++++++++++++++++ lib/tool_shed_client/schema/__init__.py | 1 + 6 files changed, 69 insertions(+), 3 deletions(-) diff --git a/lib/tool_shed/managers/categories.py b/lib/tool_shed/managers/categories.py index 5341c4dea450..1d418f1aa8fa 100644 --- a/lib/tool_shed/managers/categories.py +++ b/lib/tool_shed/managers/categories.py @@ -26,6 +26,9 @@ class CategoryManager: def __init__(self, app: ToolShedApp): self.app = app + def get(self, encoded_category_id: str) -> Category: + return suc.get_category(self.app, encoded_category_id) + def create(self, trans: ProvidesUserContext, category_request: CreateCategoryRequest) -> Category: name = category_request.name description = category_request.description or name @@ -71,6 +74,7 @@ def to_model(self, category: Category) -> CategoryResponse: name=as_dict["name"], description=as_dict["description"], repositories=as_dict["repositories"], + deleted=as_dict["deleted"], ) diff --git a/lib/tool_shed/test/base/populators.py b/lib/tool_shed/test/base/populators.py index 9b63e1f7a559..19b6b697b46e 100644 --- a/lib/tool_shed/test/base/populators.py +++ b/lib/tool_shed/test/base/populators.py @@ -266,9 +266,12 @@ def get_categories(self) -> List[Category]: response.raise_for_status() return [Category(**c) for c in response.json()] - def get_category_with_name(self, name: str) -> Optional[Category]: - response = self._api_interactor.get("categories") + def get_category_with_id(self, category_id: str) -> Category: + response = self._api_interactor.get(f"categories/{category_id}") response.raise_for_status() + return Category(**response.json()) + + def get_category_with_name(self, name: str) -> Optional[Category]: categories = [c for c in self.get_categories() if c.name == name] return categories[0] if categories else None diff --git a/lib/tool_shed/test/functional/test_shed_categories.py b/lib/tool_shed/test/functional/test_shed_categories.py index 63120b72d83c..3f9dd14fa516 100644 --- a/lib/tool_shed/test/functional/test_shed_categories.py +++ b/lib/tool_shed/test/functional/test_shed_categories.py @@ -10,7 +10,18 @@ def test_create_requires_name(self): def test_create_okay(self): name = random_name(prefix="createokay") - body = {"name": name, "description": "testcreateokaydescript"} + description = "testcreateokaydescript" + body = {"name": name, "description": description} response = self.admin_api_interactor.post("categories", json=body) assert response.status_code == 200 assert response.json()["name"] == name + + category = self.populator.get_category_with_name(name) + assert category is not None + assert category.name == name + assert category.description == description + + category_id = category.id + category = self.populator.get_category_with_id(category_id) + assert category.name == name + assert category.description == description diff --git a/lib/tool_shed/webapp/api2/categories.py b/lib/tool_shed/webapp/api2/categories.py index 023242469dd1..cd8696f65cfb 100644 --- a/lib/tool_shed/webapp/api2/categories.py +++ b/lib/tool_shed/webapp/api2/categories.py @@ -56,6 +56,18 @@ def index(self, trans: SessionRequestContext = DependsOnTrans) -> List[CategoryR categories = self.category_manager.index_db(trans, deleted) return [self.category_manager.to_model(c) for c in categories] + @router.get( + "/api/categories/{encoded_category_id}", + description="show category", + operation_id="categories__show", + ) + def show(self, encoded_category_id: str = CategoryIdPathParam) -> CategoryResponse: + """ + Return a list of dictionaries that contain information about each Category. + """ + category = self.category_manager.get(encoded_category_id) + return self.category_manager.to_model(category) + @router.get( "/api/categories/{encoded_category_id}/repositories", description="display repositories by category", diff --git a/lib/tool_shed/webapp/frontend/src/schema/schema.ts b/lib/tool_shed/webapp/frontend/src/schema/schema.ts index 91fd9d54ea0c..c49abf299742 100644 --- a/lib/tool_shed/webapp/frontend/src/schema/schema.ts +++ b/lib/tool_shed/webapp/frontend/src/schema/schema.ts @@ -20,6 +20,13 @@ export interface paths { */ post: operations["categories__create"] } + "/api/categories/{encoded_category_id}": { + /** + * Show + * @description show category + */ + get: operations["categories__show"] + } "/api/categories/{encoded_category_id}/repositories": { /** * Repositories @@ -261,6 +268,8 @@ export interface components { } /** Category */ Category: { + /** Deleted */ + deleted: boolean /** Description */ description: string /** Id */ @@ -1128,6 +1137,32 @@ export interface operations { } } } + categories__show: { + /** + * Show + * @description show category + */ + parameters: { + /** @description The encoded database identifier of the category. */ + path: { + encoded_category_id: string + } + } + responses: { + /** @description Successful Response */ + 200: { + content: { + "application/json": components["schemas"]["Category"] + } + } + /** @description Validation Error */ + 422: { + content: { + "application/json": components["schemas"]["HTTPValidationError"] + } + } + } + } categories__repositories: { /** * Repositories diff --git a/lib/tool_shed_client/schema/__init__.py b/lib/tool_shed_client/schema/__init__.py index a97ca4f3401b..2f4de2cc198f 100644 --- a/lib/tool_shed_client/schema/__init__.py +++ b/lib/tool_shed_client/schema/__init__.py @@ -69,6 +69,7 @@ class Category(BaseModel): id: str name: str description: str + deleted: bool repositories: int From b85f2f10faeb7481ee312a8871c8ed7f19587cc9 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 18 Oct 2023 16:44:38 +0100 Subject: [PATCH 11/19] Fix `get_users_with_repo_alert()` call Introduced in commit 1711e96dacf02724ae7d7f73d8cbc02201e88477 . --- lib/tool_shed/util/shed_util_common.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/tool_shed/util/shed_util_common.py b/lib/tool_shed/util/shed_util_common.py index 3f107c652ab3..016183204139 100644 --- a/lib/tool_shed/util/shed_util_common.py +++ b/lib/tool_shed/util/shed_util_common.py @@ -11,6 +11,7 @@ select, true, ) +from sqlalchemy.orm import scoped_session from galaxy import util from galaxy.tool_shed.util.shed_util_common import ( @@ -331,7 +332,7 @@ def handle_email_alerts(app, host, repository, content_alert_str="", new_repo_al subject = f"Galaxy tool shed alert for new repository named {str(repository.name)}" subject = subject[:80] email_alerts = [] - for user in get_users_with_repo_alert(sa_session.query, app.model.User): + for user in get_users_with_repo_alert(sa_session, app.model.User): if admin_only: if user.email in admin_users: email_alerts.append(user.email) @@ -463,6 +464,6 @@ def tool_shed_is_this_tool_shed(toolshed_base_url, trans=None): ) -def get_users_with_repo_alert(session, user_model): +def get_users_with_repo_alert(session: scoped_session, user_model): stmt = select(user_model).where(user_model.deleted == false()).where(user_model.new_repo_alert == true()) return session.scalars(stmt) From 4e7763d8468c34d013d17658ccb1a226840eb358 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 18 Oct 2023 17:05:39 +0100 Subject: [PATCH 12/19] Add type annotations. Drop unused functions and re-exports. --- lib/tool_shed/util/shed_util_common.py | 117 ++++++------------------- 1 file changed, 27 insertions(+), 90 deletions(-) diff --git a/lib/tool_shed/util/shed_util_common.py b/lib/tool_shed/util/shed_util_common.py index 016183204139..ed4d160effda 100644 --- a/lib/tool_shed/util/shed_util_common.py +++ b/lib/tool_shed/util/shed_util_common.py @@ -3,7 +3,10 @@ import os import socket import string -from typing import TYPE_CHECKING +from typing import ( + List, + TYPE_CHECKING, +) from sqlalchemy import ( false, @@ -14,17 +17,7 @@ from sqlalchemy.orm import scoped_session from galaxy import util -from galaxy.tool_shed.util.shed_util_common import ( - can_eliminate_repository_dependency, - clean_dependency_relationships, - generate_tool_guid, - get_ctx_rev, - get_next_prior_import_or_install_required_dict_entry, - get_tool_panel_config_tool_path_install_dir, - get_user, - have_shed_tool_conf_for_install, - set_image_paths, -) +from galaxy.tool_shed.util.shed_util_common import get_user from galaxy.util import ( checkers, unicodify, @@ -39,6 +32,7 @@ if TYPE_CHECKING: from tool_shed.structured_app import ToolShedApp + from tool_shed.webapp.model import Repository log = logging.getLogger(__name__) @@ -118,16 +112,7 @@ def get_category_by_name(app: "ToolShedApp", name: str): return sa_session.scalars(stmt).first() -def get_repository_categories(app, id): - """Get categories of a repository on the tool shed side from the database via id""" - sa_session = app.model.session - stmt = select(app.model.RepositoryCategoryAssociation).where( - app.model.RepositoryCategoryAssociation.repository_id == app.security.decode_id(id) - ) - return sa_session.scalars(stmt).all() - - -def get_repository_file_contents(app, file_path, repository_id, is_admin=False): +def get_repository_file_contents(app: "ToolShedApp", file_path: str, repository_id: str, is_admin: bool = False) -> str: """Return the display-safe contents of a repository file for display in a browser.""" safe_str = "" if not _is_path_browsable(app, file_path, repository_id, is_admin): @@ -188,7 +173,7 @@ def get_repository_files(folder_path): return contents -def get_repository_from_refresh_on_change(app, **kwd): +def get_repository_from_refresh_on_change(app: "ToolShedApp", **kwd): # The changeset_revision_select_field in several grids performs a refresh_on_change which sends in request parameters like # changeset_revison_1, changeset_revision_2, etc. One of the many select fields on the grid performed the refresh_on_change, # so we loop through all of the received values to see which value is not the repository tip. If we find it, we know the @@ -206,52 +191,14 @@ def get_repository_from_refresh_on_change(app, **kwd): return v, None -def get_repository_type_from_tool_shed(app, tool_shed_url, name, owner): - """ - Send a request to the tool shed to retrieve the type for a repository defined by the - combination of a name and owner. - """ - tool_shed_url = common_util.get_tool_shed_url_from_tool_shed_registry(app, tool_shed_url) - params = dict(name=name, owner=owner) - pathspec = ["repository", "get_repository_type"] - repository_type = util.url_get( - tool_shed_url, auth=app.tool_shed_registry.url_auth(tool_shed_url), pathspec=pathspec, params=params - ) - return repository_type - - -def get_tool_dependency_definition_metadata_from_tool_shed(app, tool_shed_url, name, owner): - """ - Send a request to the tool shed to retrieve the current metadata for a - repository of type tool_dependency_definition defined by the combination - of a name and owner. - """ - tool_shed_url = common_util.get_tool_shed_url_from_tool_shed_registry(app, tool_shed_url) - params = dict(name=name, owner=owner) - pathspec = ["repository", "get_tool_dependency_definition_metadata"] - metadata = util.url_get( - tool_shed_url, auth=app.tool_shed_registry.url_auth(tool_shed_url), pathspec=pathspec, params=params - ) - return metadata - - -def get_tool_path_by_shed_tool_conf_filename(app, shed_tool_conf): - """ - Return the tool_path config setting for the received shed_tool_conf file by searching the tool box's in-memory list of shed_tool_confs for the - dictionary whose config_filename key has a value matching the received shed_tool_conf. - """ - for shed_tool_conf_dict in app.toolbox.dynamic_confs(include_migrated_tool_conf=True): - config_filename = shed_tool_conf_dict["config_filename"] - if config_filename == shed_tool_conf: - return shed_tool_conf_dict["tool_path"] - else: - file_name = basic_util.strip_path(config_filename) - if file_name == shed_tool_conf: - return shed_tool_conf_dict["tool_path"] - return None - - -def handle_email_alerts(app, host, repository, content_alert_str="", new_repo_alert=False, admin_only=False): +def handle_email_alerts( + app: "ToolShedApp", + host: str, + repository: "Repository", + content_alert_str: str = "", + new_repo_alert: bool = False, + admin_only: bool = False, +) -> None: """ There are 2 complementary features that enable a tool shed user to receive email notification: @@ -353,7 +300,7 @@ def handle_email_alerts(app, host, repository, content_alert_str="", new_repo_al log.exception("An error occurred sending a tool shed repository update alert by email.") -def _is_path_browsable(app, path, repository_id, is_admin=False): +def _is_path_browsable(app: "ToolShedApp", path: str, repository_id: str, is_admin: bool = False) -> bool: """ Detects whether the given path is browsable i.e. is within the allowed repository folders. Admins can additionaly browse folders @@ -364,7 +311,7 @@ def _is_path_browsable(app, path, repository_id, is_admin=False): return is_path_within_repo(app, path, repository_id) -def is_path_within_dependency_dir(app, path): +def is_path_within_dependency_dir(app: "ToolShedApp", path: str) -> bool: """ Detect whether the given path is within the tool_dependency_dir folder on the disk. (Specified by the config option). Use to filter malicious symlinks targeting outside paths. @@ -378,7 +325,7 @@ def is_path_within_dependency_dir(app, path): return allowed -def is_path_within_repo(app, path, repository_id): +def is_path_within_repo(app: "ToolShedApp", path: str, repository_id: str) -> bool: """ Detect whether the given path is within the repository folder on the disk. Use to filter malicious symlinks targeting outside paths. @@ -388,7 +335,9 @@ def is_path_within_repo(app, path, repository_id): return os.path.commonprefix([repo_path, resolved_path]) == repo_path -def open_repository_files_folder(app, folder_path, repository_id, is_admin=False): +def open_repository_files_folder( + app: "ToolShedApp", folder_path: str, repository_id: str, is_admin: bool = False +) -> List: """ Return a list of dictionaries, each of which contains information for a file or directory contained within a directory in a repository file hierarchy. @@ -437,33 +386,21 @@ def tool_shed_is_this_tool_shed(toolshed_base_url, trans=None): return cleaned_toolshed_base_url == cleaned_tool_shed +def get_users_with_repo_alert(session: scoped_session, user_model): + stmt = select(user_model).where(user_model.deleted == false()).where(user_model.new_repo_alert == true()) + return session.scalars(stmt) + + __all__ = ( - "can_eliminate_repository_dependency", - "clean_dependency_relationships", "count_repositories_in_category", - "generate_tool_guid", "get_categories", "get_category", "get_category_by_name", - "get_ctx_rev", - "get_next_prior_import_or_install_required_dict_entry", - "get_repository_categories", "get_repository_file_contents", - "get_repository_type_from_tool_shed", - "get_tool_dependency_definition_metadata_from_tool_shed", - "get_tool_panel_config_tool_path_install_dir", - "get_tool_path_by_shed_tool_conf_filename", "get_user", "handle_email_alerts", - "have_shed_tool_conf_for_install", "is_path_within_dependency_dir", "is_path_within_repo", "open_repository_files_folder", - "set_image_paths", "tool_shed_is_this_tool_shed", ) - - -def get_users_with_repo_alert(session: scoped_session, user_model): - stmt = select(user_model).where(user_model.deleted == false()).where(user_model.new_repo_alert == true()) - return session.scalars(stmt) From 806dfae4b3ea70428c729cd9bb77d45c4703a1a8 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 18 Oct 2023 17:59:47 +0100 Subject: [PATCH 13/19] Fix `index_repositories()` method Broken in commit 77df9c9a73e6efd5233ec881a555855e913b3aaf . --- lib/tool_shed/managers/repositories.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/tool_shed/managers/repositories.py b/lib/tool_shed/managers/repositories.py index 596aca642b0e..9d83144c258e 100644 --- a/lib/tool_shed/managers/repositories.py +++ b/lib/tool_shed/managers/repositories.py @@ -20,6 +20,7 @@ false, select, ) +from sqlalchemy.orm import scoped_session from galaxy import web from galaxy.exceptions import ( @@ -254,7 +255,9 @@ def index_tool_ids(app: ToolShedApp, tool_ids: List[str]) -> Dict[str, Any]: def index_repositories(app: ToolShedApp, name: Optional[str], owner: Optional[str], deleted: bool): - return list(_get_repository_by_name_and_owner_and_deleted(app.model.context, name, owner, deleted, app.model.User)) + return list( + _get_repositories_by_name_and_owner_and_deleted(app.model.context, name, owner, deleted, app.model.User) + ) def can_manage_repo(trans: ProvidesUserContext, repository: Repository) -> bool: @@ -579,7 +582,7 @@ def upload_tar_and_set_metadata( return message -def _get_repository_by_name_and_owner(session, name, owner, user_model): +def _get_repository_by_name_and_owner(session: scoped_session, name: str, owner: str, user_model): stmt = ( select(Repository) .where(Repository.deprecated == false()) @@ -592,12 +595,14 @@ def _get_repository_by_name_and_owner(session, name, owner, user_model): return session.scalars(stmt).first() -def _get_repository_by_name_and_owner_and_deleted(session, name, owner, deleted, user_model): +def _get_repositories_by_name_and_owner_and_deleted( + session: scoped_session, name: Optional[str], owner: Optional[str], deleted: bool, user_model +): stmt = select(Repository).where(Repository.deprecated == false()).where(Repository.deleted == deleted) if owner is not None: stmt = stmt.where(user_model.username == owner) stmt = stmt.where(Repository.user_id == user_model.id) if name is not None: stmt = stmt.where(Repository.name == name) - stmt = stmt.order_by(Repository.name).limit(1) - return session.scalars(stmt).first() + stmt = stmt.order_by(Repository.name) + return session.scalars(stmt) From e5bec6ad71494034a90926e5efbc4103e6490aea Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 19 Oct 2023 11:07:15 +0100 Subject: [PATCH 14/19] Fix typo in query Introduced in commit 3238b212805f35aa17906ee51296a08b491e0e4c . --- lib/tool_shed/webapp/controllers/repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tool_shed/webapp/controllers/repository.py b/lib/tool_shed/webapp/controllers/repository.py index 110838cbabae..baee2f6123ba 100644 --- a/lib/tool_shed/webapp/controllers/repository.py +++ b/lib/tool_shed/webapp/controllers/repository.py @@ -2659,7 +2659,7 @@ def validate_changeset_revision(self, trans, changeset_revision, repository_id): def get_first_repository_metadata(session): stmt = select(RepositoryMetadata).limit(1) - return session.select(stmt).first() + return session.scalars(stmt).first() def get_current_repositories(session): From b5013aa57a4199537ef09a4427968ec3f365605f Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 19 Oct 2023 11:54:43 +0100 Subject: [PATCH 15/19] Fix another typo Introduced in commit 70c7f03be9ef02bcdd405820ad68f1772f43a25d . --- lib/tool_shed/test/base/twilltestcase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tool_shed/test/base/twilltestcase.py b/lib/tool_shed/test/base/twilltestcase.py index d03a5041fee2..9568aac837df 100644 --- a/lib/tool_shed/test/base/twilltestcase.py +++ b/lib/tool_shed/test/base/twilltestcase.py @@ -2092,7 +2092,7 @@ def get_installed_repository(session, name, owner, changeset): if owner is not None: stmt = stmt.where(ToolShedRepository.owner == owner) if changeset is not None: - stmt = stmt.wehre(ToolShedRepository.changeset_revision == changeset) + stmt = stmt.where(ToolShedRepository.changeset_revision == changeset) stmt = stmt.where(ToolShedRepository.deleted == false()) stmt = stmt.where(ToolShedRepository.uninstalled == false()) return session.scalars(stmt).one_or_none() From bab9753aebe6c16ac0466217cda36f4b1dacf382 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 19 Oct 2023 12:45:59 +0100 Subject: [PATCH 16/19] Fix another typo Introduced in commit 68831ddcfc661ca13ce7b26cc7383589f185c65a . --- lib/tool_shed/metadata/repository_metadata_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tool_shed/metadata/repository_metadata_manager.py b/lib/tool_shed/metadata/repository_metadata_manager.py index 03c5562d5c02..2bc5e756a0d0 100644 --- a/lib/tool_shed/metadata/repository_metadata_manager.py +++ b/lib/tool_shed/metadata/repository_metadata_manager.py @@ -1097,7 +1097,7 @@ def get_current_repositories(session, order=False): def get_filtered_repositories(session, repo_ids, order): - stmt = select(Repository).where(Repository.in_(repo_ids)) + stmt = select(Repository).where(Repository.id.in_(repo_ids)) if order: stmt = stmt.order_by(Repository.name, Repository.user_id) return session.scalars(stmt) From e34089513411032d28721fd063653e485998841f Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 19 Oct 2023 16:52:22 +0100 Subject: [PATCH 17/19] Type annotation fixes --- lib/tool_shed/test/base/twilltestcase.py | 3 +- .../test_0300_reset_all_metadata.py | 34 +++++++++++++++++-- .../test/functional/test_shed_repositories.py | 1 + 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/lib/tool_shed/test/base/twilltestcase.py b/lib/tool_shed/test/base/twilltestcase.py index 9568aac837df..e7d32c23e153 100644 --- a/lib/tool_shed/test/base/twilltestcase.py +++ b/lib/tool_shed/test/base/twilltestcase.py @@ -1040,7 +1040,7 @@ def create_category(self, **kwd) -> Category: def create_repository_dependency( self, - repository: Optional[Repository] = None, + repository: Repository, repository_tuples=None, filepath=None, prior_installation_required=False, @@ -1050,7 +1050,6 @@ def create_repository_dependency( strings_displayed=None, strings_not_displayed=None, ): - assert repository repository_tuples = repository_tuples or [] repository_names = [] if complex: diff --git a/lib/tool_shed/test/functional/test_0300_reset_all_metadata.py b/lib/tool_shed/test/functional/test_0300_reset_all_metadata.py index f20c29b08cbd..e28d653c4c41 100644 --- a/lib/tool_shed/test/functional/test_0300_reset_all_metadata.py +++ b/lib/tool_shed/test/functional/test_0300_reset_all_metadata.py @@ -1,3 +1,5 @@ +from typing import Dict + from ..base.twilltestcase import ( common, ShedTwillTestCase, @@ -63,6 +65,7 @@ def test_0005_create_filtering_repository(self): owner=common.test_user_1_name, category=category_0000, ) + assert repository if self.repository_is_new(repository): running_standalone = True self.commit_tar_to_repository( @@ -90,6 +93,7 @@ def test_0010_create_freebayes_repository(self): strings_displayed=[], ) if running_standalone: + assert repository self.setup_freebayes_0010_repo(repository) def test_0015_create_datatypes_0020_repository(self): @@ -110,6 +114,7 @@ def test_0015_create_datatypes_0020_repository(self): category=category_0020, strings_displayed=[], ) + assert repository self.commit_tar_to_repository( repository, "column_maker/column_maker.tar", @@ -134,6 +139,7 @@ def test_0020_create_emboss_0020_repository(self): category=category_0020, strings_displayed=[], ) + assert repository self.commit_tar_to_repository( repository, "emboss/emboss.tar", @@ -158,6 +164,7 @@ def test_0025_create_emboss_datatypes_0030_repository(self): category=category_0030, strings_displayed=[], ) + assert column_maker_repository self.commit_tar_to_repository( column_maker_repository, "column_maker/column_maker.tar", @@ -182,6 +189,7 @@ def test_0030_create_emboss_5_repository(self): category=category_0030, strings_displayed=[], ) + assert emboss_5_repository self.commit_tar_to_repository( emboss_5_repository, "emboss/emboss.tar", @@ -206,6 +214,7 @@ def test_0035_create_emboss_6_repository(self): category=category_0030, strings_displayed=[], ) + assert emboss_6_repository self.commit_tar_to_repository( emboss_6_repository, "emboss/emboss.tar", @@ -230,6 +239,7 @@ def test_0040_create_emboss_0030_repository(self): category=category_0030, strings_displayed=[], ) + assert emboss_repository self.commit_tar_to_repository( emboss_repository, "emboss/emboss.tar", @@ -243,9 +253,13 @@ def test_0045_create_repository_dependencies_for_0030(self): column_maker_repository = self._get_repository_by_name_and_owner( "column_maker_0030", common.test_user_1_name ) + assert column_maker_repository emboss_repository = self._get_repository_by_name_and_owner("emboss_0030", common.test_user_1_name) + assert emboss_repository emboss_5_repository = self._get_repository_by_name_and_owner("emboss_5_0030", common.test_user_1_name) + assert emboss_5_repository emboss_6_repository = self._get_repository_by_name_and_owner("emboss_6_0030", common.test_user_1_name) + assert emboss_6_repository repository_dependencies_path = self.generate_temp_path("test_0330", additional_paths=["emboss"]) column_maker_tuple = ( self.url, @@ -299,6 +313,7 @@ def test_0050_create_freebayes_repository(self): category=category_0040, strings_displayed=[], ) + assert repository if running_standalone: self.commit_tar_to_repository( repository, @@ -324,6 +339,7 @@ def test_0055_create_filtering_repository(self): category=category_0040, strings_displayed=[], ) + assert repository self.commit_tar_to_repository( repository, "filtering/filtering_1.1.0.tar", @@ -335,7 +351,9 @@ def test_0060_create_dependency_structure(self): global running_standalone if running_standalone: freebayes_repository = self._get_repository_by_name_and_owner("freebayes_0040", common.test_user_1_name) + assert freebayes_repository filtering_repository = self._get_repository_by_name_and_owner("filtering_0040", common.test_user_1_name) + assert filtering_repository repository_dependencies_path = self.generate_temp_path("test_0340", additional_paths=["dependencies"]) freebayes_tuple = ( self.url, @@ -375,6 +393,7 @@ def test_0065_create_convert_repository(self): category=category, strings_displayed=[], ) + assert repository self.commit_tar_to_repository( repository, "convert_chars/convert_chars.tar", @@ -396,6 +415,7 @@ def test_0070_create_column_repository(self): category=category, strings_displayed=[], ) + assert repository self.commit_tar_to_repository( repository, "column_maker/column_maker.tar", @@ -420,6 +440,7 @@ def test_0080_create_emboss_repository(self): category=category, strings_displayed=[], ) + assert repository self.commit_tar_to_repository( repository, "emboss/emboss.tar", @@ -441,6 +462,7 @@ def test_0085_create_filtering_repository(self): category=category, strings_displayed=[], ) + assert filtering_repository self.commit_tar_to_repository( filtering_repository, "filtering/filtering_1.1.0.tar", @@ -462,6 +484,7 @@ def test_0090_create_freebayes_repository(self): category=category, strings_displayed=[], ) + assert repository self.commit_tar_to_repository( repository, "freebayes/freebayes.tar", @@ -483,6 +506,7 @@ def test_0095_create_bismark_repository(self): category=category, strings_displayed=[], ) + assert repository self.user_populator().setup_bismark_repo(repository, end=1) def test_0100_create_and_upload_dependency_definitions(self): @@ -491,19 +515,25 @@ def test_0100_create_and_upload_dependency_definitions(self): if running_standalone: self.login(email=common.test_user_1_email, username=common.test_user_1_name) column_repository = self._get_repository_by_name_and_owner(column_repository_name, common.test_user_1_name) + assert column_repository convert_repository = self._get_repository_by_name_and_owner( convert_repository_name, common.test_user_1_name ) + assert convert_repository emboss_repository = self._get_repository_by_name_and_owner(emboss_repository_name, common.test_user_1_name) + assert emboss_repository filtering_repository = self._get_repository_by_name_and_owner( filtering_repository_name, common.test_user_1_name ) + assert filtering_repository freebayes_repository = self._get_repository_by_name_and_owner( freebayes_repository_name, common.test_user_1_name ) + assert freebayes_repository bismark_repository = self._get_repository_by_name_and_owner( bismark_repository_name, common.test_user_1_name ) + assert bismark_repository dependency_xml_path = self.generate_temp_path("test_0050", additional_paths=["freebayes"]) # convert_chars depends on column_maker # column_maker depends on convert_chars @@ -562,8 +592,8 @@ def test_0100_create_and_upload_dependency_definitions(self): def test_0110_reset_metadata_on_all_repositories(self): """Reset metadata on all repositories, then verify that it has not changed.""" self.login(email=common.admin_email, username=common.admin_username) - old_metadata = dict() - new_metadata = dict() + old_metadata: Dict[str, Dict] = dict() + new_metadata: Dict[str, Dict] = dict() repositories = self.test_db_util.get_all_repositories() for repository in repositories: old_metadata[self.security.encode_id(repository.id)] = dict() diff --git a/lib/tool_shed/test/functional/test_shed_repositories.py b/lib/tool_shed/test/functional/test_shed_repositories.py index 90e9b8134059..ac58f44d07d6 100644 --- a/lib/tool_shed/test/functional/test_shed_repositories.py +++ b/lib/tool_shed/test/functional/test_shed_repositories.py @@ -83,6 +83,7 @@ def test_index_simple(self): assert repository_id in repository_ids repository = self.populator.get_repository_for(repo.owner, repo.name) + assert repository assert repository.owner == repo.owner assert repository.name == repo.name From bdcc33195a54391f09012a429f0369f56fdbe4ab Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Fri, 20 Oct 2023 11:14:05 +0100 Subject: [PATCH 18/19] Remove unused test config option --- lib/galaxy_test/driver/driver_util.py | 1 - lib/tool_shed/test/base/driver.py | 1 - 2 files changed, 2 deletions(-) diff --git a/lib/galaxy_test/driver/driver_util.py b/lib/galaxy_test/driver/driver_util.py index ef9d6c4988bc..6bd109e76039 100644 --- a/lib/galaxy_test/driver/driver_util.py +++ b/lib/galaxy_test/driver/driver_util.py @@ -227,7 +227,6 @@ def setup_galaxy_config( template_cache_path=template_cache_path, tool_config_file=tool_config_file, tool_data_table_config_path=tool_data_table_config_path, - tool_parse_help=False, tool_path=tool_path, update_integrated_tool_panel=update_integrated_tool_panel, use_tasked_jobs=True, diff --git a/lib/tool_shed/test/base/driver.py b/lib/tool_shed/test/base/driver.py index 7dc778643e81..6f56751f39cc 100644 --- a/lib/tool_shed/test/base/driver.py +++ b/lib/tool_shed/test/base/driver.py @@ -103,7 +103,6 @@ def _setup_local(self): shed_tool_data_table_config=shed_tool_data_table_conf_file, smtp_server="smtp.dummy.string.tld", email_from="functional@localhost", - tool_parse_help=False, use_heartbeat=False, ) kwargs.update(toolshed_database_conf) From 048d79242f3c2619423be59a82f4503b9c4d1044 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Fri, 20 Oct 2023 11:23:35 +0100 Subject: [PATCH 19/19] Configure temporary whoosh_index_dir for tests Otherwise a `/database/toolshed_whoosh_indexes` would be reused, leading to failing tests when running tests locally. --- lib/tool_shed/test/base/driver.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/tool_shed/test/base/driver.py b/lib/tool_shed/test/base/driver.py index 6f56751f39cc..b342a749f591 100644 --- a/lib/tool_shed/test/base/driver.py +++ b/lib/tool_shed/test/base/driver.py @@ -80,11 +80,11 @@ def _setup_local(self): os.environ["GALAXY_TEST_TOOL_DATA_PATH"] = tool_data_path galaxy_db_path = driver_util.database_files_path(tool_shed_test_tmp_dir) shed_file_path = os.path.join(shed_db_path, "files") - hgweb_config_file_path = tempfile.mkdtemp(dir=tool_shed_test_tmp_dir) + whoosh_index_dir = os.path.join(shed_db_path, "toolshed_whoosh_indexes") + hgweb_config_dir = tempfile.mkdtemp(dir=tool_shed_test_tmp_dir) new_repos_path = tempfile.mkdtemp(dir=tool_shed_test_tmp_dir) galaxy_shed_tool_path = tempfile.mkdtemp(dir=tool_shed_test_tmp_dir) galaxy_migrated_tool_path = tempfile.mkdtemp(dir=tool_shed_test_tmp_dir) - hgweb_config_dir = hgweb_config_file_path os.environ["TEST_HG_WEB_CONFIG_DIR"] = hgweb_config_dir print("Directory location for hgweb.config:", hgweb_config_dir) toolshed_database_conf = driver_util.database_conf(shed_db_path, prefix="TOOL_SHED") @@ -104,6 +104,7 @@ def _setup_local(self): smtp_server="smtp.dummy.string.tld", email_from="functional@localhost", use_heartbeat=False, + whoosh_index_dir=whoosh_index_dir, ) kwargs.update(toolshed_database_conf) # Generate the tool_data_table_conf.xml file.