From 06585380c6ab3a5f54f5c2d718f0d667d3839ad1 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Sep 2023 10:32:25 +0200 Subject: [PATCH 01/50] Narrow down history_content_type in schema --- client/src/schema/schema.ts | 28 ++++++++++++++++------------ lib/galaxy/schema/schema.py | 32 ++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/client/src/schema/schema.ts b/client/src/schema/schema.ts index 54b20f389e31..4f89f5262cc4 100644 --- a/client/src/schema/schema.ts +++ b/client/src/schema/schema.ts @@ -4807,10 +4807,11 @@ export interface components { */ hid: number; /** - * Content Type - * @description The type of this item. + * History Content Type + * @description This is always `dataset` for datasets. + * @enum {string} */ - history_content_type: components["schemas"]["HistoryContentType"]; + history_content_type: "dataset"; /** * History ID * @description The encoded ID of the history associated with this item. @@ -5021,10 +5022,11 @@ export interface components { */ hid: number; /** - * Content Type - * @description The type of this item. + * History Content Type + * @description This is always `dataset` for datasets. + * @enum {string} */ - history_content_type: components["schemas"]["HistoryContentType"]; + history_content_type: "dataset"; /** * History ID * @description The encoded ID of the history associated with this item. @@ -5136,10 +5138,11 @@ export interface components { */ hid: number; /** - * Content Type - * @description The type of this item. + * History Content Type + * @description This is always `dataset_collection` for dataset collections. + * @enum {string} */ - history_content_type: components["schemas"]["HistoryContentType"]; + history_content_type: "dataset_collection"; /** * History ID * @description The encoded ID of the history associated with this item. @@ -5270,10 +5273,11 @@ export interface components { */ hid: number; /** - * Content Type - * @description The type of this item. + * History Content Type + * @description This is always `dataset_collection` for dataset collections. + * @enum {string} */ - history_content_type: components["schemas"]["HistoryContentType"]; + history_content_type: "dataset_collection"; /** * History ID * @description The encoded ID of the history associated with this item. diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index f9c45f9a4d13..ef3e83b2c5b0 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -585,11 +585,6 @@ class HistoryItemBase(Model): title="HID", description="The index position of this item in the History.", ) - history_content_type: HistoryContentType = Field( - ..., - title="Content Type", - description="The type of this item.", - ) deleted: bool = Field( ..., title="Deleted", @@ -625,7 +620,17 @@ class Config: tags: TagCollection -class HDASummary(HistoryItemCommon): +class HDACommon(HistoryItemCommon): + history_content_type: Annotated[ + Literal["dataset"], + Field( + title="History Content Type", + description="This is always `dataset` for datasets.", + ), + ] + + +class HDASummary(HDACommon): """History Dataset Association summary information.""" dataset_id: DecodedDatabaseIdField = Field( @@ -647,7 +652,7 @@ class HDASummary(HistoryItemCommon): ) -class HDAInaccessible(HistoryItemBase): +class HDAInaccessible(HDACommon): """History Dataset Association information when the user can not access it.""" accessible: bool = AccessibleField @@ -974,7 +979,17 @@ class HDCJobStateSummary(Model): ) -class HDCASummary(HistoryItemCommon): +class HDCACommon(HistoryItemCommon): + history_content_type: Annotated[ + Literal["dataset_collection"], + Field( + title="History Content Type", + description="This is always `dataset_collection` for dataset collections.", + ), + ] + + +class HDCASummary(HDCACommon): """History Dataset Collection Association summary information.""" model_class: HDCA_MODEL_CLASS = ModelClassField(HDCA_MODEL_CLASS) @@ -985,6 +1000,7 @@ class HDCASummary(HistoryItemCommon): description="This is always `collection` for dataset collections.", ), ] = "collection" + collection_type: CollectionType = CollectionTypeField populated_state: DatasetCollectionPopulatedState = PopulatedStateField populated_state_message: Optional[str] = PopulatedStateMessageField From 8f91199fdd575466d6dd0354aaed0dd500b12373 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Sep 2023 14:46:13 +0200 Subject: [PATCH 02/50] Add dataset collection services using Fetcher --- .../services/datasetCollection.service.ts | 39 +++++++++++++++++++ client/src/stores/services/index.ts | 3 ++ 2 files changed, 42 insertions(+) create mode 100644 client/src/stores/services/datasetCollection.service.ts diff --git a/client/src/stores/services/datasetCollection.service.ts b/client/src/stores/services/datasetCollection.service.ts new file mode 100644 index 000000000000..474752989f18 --- /dev/null +++ b/client/src/stores/services/datasetCollection.service.ts @@ -0,0 +1,39 @@ +import { fetcher } from "@/schema"; + +import { DCESummary, HDCASummary } from "."; + +const DEFAULT_LIMIT = 50; + +const getCollectionContents = fetcher + .path("/api/dataset_collections/{hdca_id}/contents/{parent_id}") + .method("get") + .create(); + +export async function fetchCollectionElements(params: { + hdcaId: string; + collectionId: string; + offset?: number; + limit?: number; +}): Promise { + const { data } = await getCollectionContents({ + instance_type: "history", + hdca_id: params.hdcaId, + parent_id: params.collectionId, + offset: params.offset, + limit: params.limit, + }); + return data; +} + +export async function fetchElementsFromHDCA(params: { + hdca: HDCASummary; + offset?: number; + limit?: number; +}): Promise { + return fetchCollectionElements({ + hdcaId: params.hdca.id, + collectionId: params.hdca.collection_id, + offset: params.offset ?? 0, + limit: params.limit ?? DEFAULT_LIMIT, + }); +} diff --git a/client/src/stores/services/index.ts b/client/src/stores/services/index.ts index 7427757c9723..49d8cc27d665 100644 --- a/client/src/stores/services/index.ts +++ b/client/src/stores/services/index.ts @@ -2,6 +2,9 @@ import { components } from "@/schema"; export type DatasetSummary = components["schemas"]["HDASummary"]; export type DatasetDetails = components["schemas"]["HDADetailed"]; +export type DCESummary = components["schemas"]["DCESummary"]; +export type HDCASummary = components["schemas"]["HDCASummary"]; +export type HDCADetailed = components["schemas"]["HDCADetailed"]; export type HistoryContentItemBase = components["schemas"]["EncodedHistoryContentItem"]; From 25491c7759001c50b4674e9e5d4f967dec8f5f57 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Sep 2023 16:11:28 +0200 Subject: [PATCH 03/50] Add new collection elements Pinia store --- client/src/stores/collectionElementsStore.ts | 70 ++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 client/src/stores/collectionElementsStore.ts diff --git a/client/src/stores/collectionElementsStore.ts b/client/src/stores/collectionElementsStore.ts new file mode 100644 index 000000000000..3bd0e0e8d4ac --- /dev/null +++ b/client/src/stores/collectionElementsStore.ts @@ -0,0 +1,70 @@ +import { defineStore } from "pinia"; +import Vue, { computed, ref } from "vue"; + +import { DCESummary, HDCASummary, HistoryContentItemBase } from "./services"; +import * as Service from "./services/datasetCollection.service"; + +export const useCollectionElementsStore = defineStore("collectionElementsStore", () => { + const storedCollections = ref<{ [key: string]: HDCASummary }>({}); + const loadingCollectionElements = ref<{ [key: string]: boolean }>({}); + const storedCollectionElements = ref<{ [key: string]: DCESummary[] }>({}); + + const getCollectionElements = computed(() => { + return (collection: HDCASummary, offset = 0, limit = 50) => { + const elements = storedCollectionElements.value[collection.id] ?? []; + fetchMissingElements({ collection, offset, limit }); + return elements ?? null; + }; + }); + + const isLoadingCollectionElements = computed(() => { + return (collection: HDCASummary) => { + return loadingCollectionElements.value[collection.id] ?? false; + }; + }); + + async function fetchMissingElements(params: { collection: HDCASummary; offset: number; limit: number }) { + try { + const maxElementCountInCollection = params.collection.element_count ?? 0; + const storedElements = storedCollectionElements.value[params.collection.id] ?? []; + // Collections are immutable, so there is no need to fetch elements if the range we want is already stored + if (params.offset + params.limit <= storedElements.length) { + return; + } + // If we already have items at the offset, we can start fetching from the next offset + params.offset = storedElements.length; + + if (params.offset >= maxElementCountInCollection) { + return; + } + + Vue.set(loadingCollectionElements.value, params.collection.id, true); + const fetchedElements = await Service.fetchElementsFromHDCA({ + hdca: params.collection, + offset: params.offset, + limit: params.limit, + }); + const updatedElements = [...storedElements, ...fetchedElements]; + Vue.set(storedCollectionElements.value, params.collection.id, updatedElements); + } finally { + Vue.delete(loadingCollectionElements.value, params.collection.id); + } + } + + function saveCollectionObjects(historyContentsPayload: HistoryContentItemBase[]) { + const collectionsInHistory = historyContentsPayload.filter( + (entry) => entry.history_content_type === "dataset_collection" + ) as HDCASummary[]; + for (const collection of collectionsInHistory) { + Vue.set(storedCollections.value, collection.id, collection); + } + } + + return { + storedCollections, + storedCollectionElements, + getCollectionElements, + isLoadingCollectionElements, + saveCollectionObjects, + }; +}); From 6e8f95aedefef29c6253f78b85fc75925b6ae12a Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Sep 2023 16:11:41 +0200 Subject: [PATCH 04/50] Add some unit tests for the store --- .../stores/collectionElementsStore.test.ts | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 client/src/stores/collectionElementsStore.test.ts diff --git a/client/src/stores/collectionElementsStore.test.ts b/client/src/stores/collectionElementsStore.test.ts new file mode 100644 index 000000000000..afcae357795d --- /dev/null +++ b/client/src/stores/collectionElementsStore.test.ts @@ -0,0 +1,154 @@ +import flushPromises from "flush-promises"; +import { createPinia, setActivePinia } from "pinia"; + +import { mockFetcher } from "@/schema/__mocks__"; +import { useCollectionElementsStore } from "@/stores/collectionElementsStore"; +import { DCESummary, HDCASummary } from "@/stores/services"; + +jest.mock("@/schema"); + +const collection1: HDCASummary = mockCollection("1"); +const collection2: HDCASummary = mockCollection("2"); +const collections: HDCASummary[] = [collection1, collection2]; + +describe("useCollectionElementsStore", () => { + beforeEach(() => { + setActivePinia(createPinia()); + mockFetcher + .path("/api/dataset_collections/{hdca_id}/contents/{parent_id}") + .method("get") + .mock(fetchCollectionElements); + }); + + it("should save collections", async () => { + const store = useCollectionElementsStore(); + expect(store.storedCollections).toEqual({}); + + store.saveCollectionObjects(collections); + + expect(store.storedCollections).toEqual({ + "1": collection1, + "2": collection2, + }); + }); + + it("should fetch collection elements if they are not yet in the store", async () => { + const store = useCollectionElementsStore(); + expect(store.storedCollectionElements).toEqual({}); + expect(store.isLoadingCollectionElements(collection1)).toEqual(false); + + const offset = 0; + const limit = 5; + // Getting collection elements should trigger a fetch + store.getCollectionElements(collection1, offset, limit); + expect(store.isLoadingCollectionElements(collection1)).toEqual(true); + await flushPromises(); + expect(store.isLoadingCollectionElements(collection1)).toEqual(false); + expect(fetchCollectionElements).toHaveBeenCalled(); + + const elements = store.storedCollectionElements[collection1.id]; + expect(elements).toBeDefined(); + expect(elements).toHaveLength(limit); + }); + + it("should not fetch collection elements if they are already in the store", async () => { + const store = useCollectionElementsStore(); + const storedCount = 5; + const expectedStoredElements = Array.from({ length: storedCount }, (_, i) => mockElement(collection1.id, i)); + store.storedCollectionElements[collection1.id] = expectedStoredElements; + expect(store.storedCollectionElements[collection1.id]).toHaveLength(storedCount); + + const offset = 0; + const limit = 5; + // Getting collection elements should not trigger a fetch in this case + store.getCollectionElements(collection1, offset, limit); + expect(store.isLoadingCollectionElements(collection1)).toEqual(false); + expect(fetchCollectionElements).not.toHaveBeenCalled(); + }); + + it("should fetch only missing elements if the requested range is not already stored", async () => { + const store = useCollectionElementsStore(); + const storedCount = 3; + const expectedStoredElements = Array.from({ length: storedCount }, (_, i) => mockElement(collection1.id, i)); + store.storedCollectionElements[collection1.id] = expectedStoredElements; + expect(store.storedCollectionElements[collection1.id]).toHaveLength(storedCount); + + const offset = 2; + const limit = 5; + // Getting collection elements should trigger a fetch in this case + store.getCollectionElements(collection1, offset, limit); + expect(store.isLoadingCollectionElements(collection1)).toEqual(true); + await flushPromises(); + expect(store.isLoadingCollectionElements(collection1)).toEqual(false); + expect(fetchCollectionElements).toHaveBeenCalled(); + + const elements = store.storedCollectionElements[collection1.id]; + expect(elements).toBeDefined(); + // The offset was overlapping with the stored elements, so it was increased by the number of stored elements + // so it fetches the next "limit" number of elements + expect(elements).toHaveLength(storedCount + limit); + }); +}); + +function mockCollection(id: string, numElements = 10): HDCASummary { + return { + id: id, + element_count: numElements, + collection_type: "list", + populated_state: "ok", + populated_state_message: "", + collection_id: id, + name: `collection ${id}`, + deleted: false, + contents_url: "", + hid: 1, + history_content_type: "dataset_collection", + history_id: "1", + model_class: "HistoryDatasetCollectionAssociation", + tags: [], + visible: true, + create_time: "2021-05-25T14:00:00.000Z", + update_time: "2021-05-25T14:00:00.000Z", + type_id: "dataset_collection", + url: "", + }; +} + +function mockElement(collectionId: string, i: number): DCESummary { + const fakeID = `${collectionId}-${i}`; + return { + id: fakeID, + element_index: i, + element_identifier: `element ${i}`, + element_type: "hda", + model_class: "DatasetCollectionElement", + object: { + id: fakeID, + model_class: "HistoryDatasetAssociation", + state: "ok", + hda_ldda: "hda", + history_id: "1", + tags: [], + }, + }; +} + +interface ApiRequest { + hdca_id: string; + offset: number; + limit: number; +} + +const fetchCollectionElements = jest.fn(fakeCollectionElementsApiResponse); + +function fakeCollectionElementsApiResponse(params: ApiRequest) { + const elements: DCESummary[] = []; + const startIndex = params.offset ?? 0; + const endIndex = startIndex + (params.limit ?? 10); + for (let i = startIndex; i < endIndex; i++) { + elements.push(mockElement(params.hdca_id, i)); + } + return { + data: elements, + }; +} From de22a751d49bc7e41b9133e3c915bdf9bbe3f404 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Sep 2023 16:15:00 +0200 Subject: [PATCH 05/50] Drop CollectionElementsProvider --- .../CurrentCollection/CollectionPanel.vue | 87 +++++++++---------- .../components/providers/storeProviders.js | 1 - 2 files changed, 40 insertions(+), 48 deletions(-) diff --git a/client/src/components/History/CurrentCollection/CollectionPanel.vue b/client/src/components/History/CurrentCollection/CollectionPanel.vue index 8e3b18008aca..758378f3a9f4 100644 --- a/client/src/components/History/CurrentCollection/CollectionPanel.vue +++ b/client/src/components/History/CurrentCollection/CollectionPanel.vue @@ -1,44 +1,35 @@ + - - diff --git a/client/src/stores/collectionElementsStore.ts b/client/src/stores/collectionElementsStore.ts index bdc622073848..426f6a61aea7 100644 --- a/client/src/stores/collectionElementsStore.ts +++ b/client/src/stores/collectionElementsStore.ts @@ -51,6 +51,11 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore", } } + async function loadCollectionElements(collection: HDCASummary) { + const elements = await Service.fetchElementsFromHDCA({ hdca: collection }); + Vue.set(storedCollectionElements.value, collection.id, elements); + } + function saveCollections(historyContentsPayload: HistoryContentItemBase[]) { const collectionsInHistory = historyContentsPayload.filter( (entry) => entry.history_content_type === "dataset_collection" @@ -65,6 +70,7 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore", storedCollectionElements, getCollectionElements, isLoadingCollectionElements, + loadCollectionElements, saveCollections, }; }); From 60d3d05979671bf10dcd92dd498745607e1ff8f1 Mon Sep 17 00:00:00 2001 From: John Davis Date: Wed, 20 Sep 2023 17:14:40 -0400 Subject: [PATCH 11/50] Fix SA2.0 ORM usage in webapps.galaxy.api Move data access method to managers.workflows (count_stored_workflow_user_assocs) Move data access method to managers.pages (get_page_revisions) --- lib/galaxy/managers/pages.py | 12 +++++++++- lib/galaxy/managers/workflows.py | 14 ++++++++++- lib/galaxy/webapps/galaxy/api/forms.py | 10 +++++--- lib/galaxy/webapps/galaxy/api/job_files.py | 4 ++-- lib/galaxy/webapps/galaxy/api/job_tokens.py | 7 +++--- .../webapps/galaxy/api/library_contents.py | 23 ++++--------------- .../webapps/galaxy/api/page_revisions.py | 7 ++++-- .../webapps/galaxy/api/tool_entry_points.py | 4 ++-- lib/galaxy/webapps/galaxy/api/users.py | 21 ++++++++--------- lib/galaxy/webapps/galaxy/api/workflows.py | 21 ++++++++--------- 10 files changed, 66 insertions(+), 57 deletions(-) diff --git a/lib/galaxy/managers/pages.py b/lib/galaxy/managers/pages.py index 9b4f4d8f4552..af5a2b6d491b 100644 --- a/lib/galaxy/managers/pages.py +++ b/lib/galaxy/managers/pages.py @@ -18,9 +18,13 @@ from sqlalchemy import ( false, or_, + select, true, ) -from sqlalchemy.orm import aliased +from sqlalchemy.orm import ( + aliased, + Session, +) from galaxy import ( exceptions, @@ -38,6 +42,7 @@ ready_galaxy_markdown_for_export, ready_galaxy_markdown_for_import, ) +from galaxy.model import PageRevision from galaxy.model.base import transaction from galaxy.model.index_filter_util import ( append_user_filter, @@ -621,3 +626,8 @@ def placeholderRenderForSave(trans: ProvidesHistoryContext, item_class, item_id, item_id=item_id, item_name=item_name, ) + + +def get_page_revision(session: Session, page_id: int): + stmt = select(PageRevision).filter_by(page_id=page_id) + return session.scalars(stmt) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 4d84e834cd85..5a81ba4df14d 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -27,13 +27,16 @@ and_, desc, false, + func, or_, + select, true, ) from sqlalchemy.orm import ( aliased, joinedload, Query, + Session, subqueryload, ) @@ -50,7 +53,10 @@ from galaxy.managers.base import decode_id from galaxy.managers.context import ProvidesUserContext from galaxy.managers.executables import artifact_class -from galaxy.model import StoredWorkflow +from galaxy.model import ( + StoredWorkflow, + StoredWorkflowUserShareAssociation, +) from galaxy.model.base import transaction from galaxy.model.index_filter_util import ( append_user_filter, @@ -1972,3 +1978,9 @@ def import_workflow(self, workflow, **kwds): raise NotImplementedError( "Direct format 2 import of nested workflows is not yet implemented, use bioblend client." ) + + +def count_stored_workflow_user_assocs(session: Session, user, stored_workflow) -> int: + stmt = select(StoredWorkflowUserShareAssociation).filter_by(user=user, stored_workflow=stored_workflow) + stmt = select(func.count()).select_from(stmt) + return session.scalar(stmt) diff --git a/lib/galaxy/webapps/galaxy/api/forms.py b/lib/galaxy/webapps/galaxy/api/forms.py index c3313fb46cce..3b4a0dbd9702 100644 --- a/lib/galaxy/webapps/galaxy/api/forms.py +++ b/lib/galaxy/webapps/galaxy/api/forms.py @@ -3,8 +3,11 @@ """ import logging +from sqlalchemy import select + from galaxy import web from galaxy.forms.forms import form_factory +from galaxy.model import FormDefinition from galaxy.model.base import transaction from galaxy.util import XML from galaxy.webapps.base.controller import url_for @@ -23,9 +26,10 @@ def index(self, trans, **kwd): if not trans.user_is_admin: trans.response.status = 403 return "You are not authorized to view the list of forms." - query = trans.sa_session.query(trans.app.model.FormDefinition) + rval = [] - for form_definition in query: + form_defs = trans.sa_session.scalars(select(FormDefinition)) + for form_definition in form_defs: item = form_definition.to_dict( value_mapper={"id": trans.security.encode_id, "form_definition_current_id": trans.security.encode_id} ) @@ -46,7 +50,7 @@ def show(self, trans, id, **kwd): trans.response.status = 400 return f"Malformed form definition id ( {str(form_definition_id)} ) specified, unable to decode." try: - form_definition = trans.sa_session.query(trans.app.model.FormDefinition).get(decoded_form_definition_id) + form_definition = trans.sa_session.get(FormDefinition, decoded_form_definition_id) except Exception: form_definition = None if not form_definition or not trans.user_is_admin: diff --git a/lib/galaxy/webapps/galaxy/api/job_files.py b/lib/galaxy/webapps/galaxy/api/job_files.py index e6ec29d640f9..ea52c5a59a49 100644 --- a/lib/galaxy/webapps/galaxy/api/job_files.py +++ b/lib/galaxy/webapps/galaxy/api/job_files.py @@ -7,9 +7,9 @@ from galaxy import ( exceptions, - model, util, ) +from galaxy.model import Job from galaxy.web import ( expose_api_anonymous_and_sessionless, expose_api_raw_anonymous_and_sessionless, @@ -126,7 +126,7 @@ def __authorize_job_access(self, trans, encoded_job_id, **kwargs): raise exceptions.ItemAccessibilityException("Invalid job_key supplied.") # Verify job is active. Don't update the contents of complete jobs. - job = trans.sa_session.query(model.Job).get(job_id) + job = trans.sa_session.get(Job, job_id) if job.finished: error_message = "Attempting to read or modify the files of a job that has already completed." raise exceptions.ItemAccessibilityException(error_message) diff --git a/lib/galaxy/webapps/galaxy/api/job_tokens.py b/lib/galaxy/webapps/galaxy/api/job_tokens.py index 7c654709ece7..63907f662cd9 100644 --- a/lib/galaxy/webapps/galaxy/api/job_tokens.py +++ b/lib/galaxy/webapps/galaxy/api/job_tokens.py @@ -7,11 +7,11 @@ from galaxy import ( exceptions, - model, util, ) from galaxy.authnz.util import provider_name_to_backend from galaxy.managers.context import ProvidesAppContext +from galaxy.model import Job from galaxy.schema.fields import EncodedDatabaseIdField from galaxy.webapps.galaxy.api import ( DependsOnTrans, @@ -52,8 +52,9 @@ def get_token( return tokens["id"] def __authorize_job_access(self, trans, encoded_job_id, job_key): + session = trans.sa_session job_id = trans.security.decode_id(encoded_job_id) - job = trans.sa_session.query(model.Job).get(job_id) + job = session.get(Job, job_id) secret = job.destination_params.get("job_secret_base", "jobs_token") job_key_internal = trans.security.encode_id(job_id, kind=secret) @@ -61,7 +62,7 @@ def __authorize_job_access(self, trans, encoded_job_id, job_key): raise exceptions.AuthenticationFailed("Invalid job_key supplied.") # Verify job is active - job = trans.sa_session.query(model.Job).get(job_id) + job = session.get(Job, job_id) if job.finished: error_message = "Attempting to get oidc token for a job that has already completed." raise exceptions.ItemAccessibilityException(error_message) diff --git a/lib/galaxy/webapps/galaxy/api/library_contents.py b/lib/galaxy/webapps/galaxy/api/library_contents.py index a0e21e1ad6d6..06b74eda6edb 100644 --- a/lib/galaxy/webapps/galaxy/api/library_contents.py +++ b/lib/galaxy/webapps/galaxy/api/library_contents.py @@ -4,11 +4,6 @@ import logging from typing import Optional -from sqlalchemy.orm.exc import ( - MultipleResultsFound, - NoResultFound, -) - from galaxy import ( exceptions, managers, @@ -25,7 +20,9 @@ from galaxy.model import ( ExtendedMetadata, ExtendedMetadataIndex, + Library, LibraryDataset, + LibraryFolder, tags, ) from galaxy.model.base import transaction @@ -101,19 +98,7 @@ def traverse(folder): rval.append(ld) return rval - decoded_library_id = self.decode_id(library_id) - try: - library = ( - trans.sa_session.query(trans.app.model.Library) - .filter(trans.app.model.Library.table.c.id == decoded_library_id) - .one() - ) - except MultipleResultsFound: - raise exceptions.InconsistentDatabase("Multiple libraries found with the same id.") - except NoResultFound: - raise exceptions.RequestParameterInvalidException("No library found with the id provided.") - except Exception as e: - raise exceptions.InternalServerError(f"Error loading from the database.{util.unicodify(e)}") + library = trans.sa_session.get(Library, self.decode_id(library_id)) if not (trans.user_is_admin or trans.app.security_agent.can_access_library(current_user_roles, library)): raise exceptions.RequestParameterInvalidException("No library found with the id provided.") encoded_id = f"F{trans.security.encode_id(library.root_folder.id)}" @@ -348,7 +333,7 @@ def _upload_library_dataset(self, trans, folder_id: int, **kwd): roles = kwd.get("roles", "") is_admin = trans.user_is_admin current_user_roles = trans.get_current_user_roles() - folder = trans.sa_session.query(trans.app.model.LibraryFolder).get(folder_id) + folder = trans.sa_session.get(LibraryFolder, folder_id) self._check_access(trans, is_admin, folder, current_user_roles) self._check_add(trans, is_admin, folder, current_user_roles) library = folder.parent_library diff --git a/lib/galaxy/webapps/galaxy/api/page_revisions.py b/lib/galaxy/webapps/galaxy/api/page_revisions.py index 85121077e53f..815b177d77fb 100644 --- a/lib/galaxy/webapps/galaxy/api/page_revisions.py +++ b/lib/galaxy/webapps/galaxy/api/page_revisions.py @@ -4,7 +4,10 @@ import logging from galaxy.managers.base import get_object -from galaxy.managers.pages import PageManager +from galaxy.managers.pages import ( + get_page_revision, + PageManager, +) from galaxy.web import expose_api from . import ( BaseGalaxyAPIController, @@ -30,7 +33,7 @@ def index(self, trans, page_id, **kwd): :returns: dictionaries containing different revisions of the page """ page = get_object(trans, page_id, "Page", check_ownership=False, check_accessible=True) - r = trans.sa_session.query(trans.app.model.PageRevision).filter_by(page_id=page.id) + r = get_page_revision(trans.sa_session, page.id) out = [] for page in r: as_dict = self.encode_all_ids(trans, page.to_dict(), True) diff --git a/lib/galaxy/webapps/galaxy/api/tool_entry_points.py b/lib/galaxy/webapps/galaxy/api/tool_entry_points.py index f6590ef07344..d2d43031dfb3 100644 --- a/lib/galaxy/webapps/galaxy/api/tool_entry_points.py +++ b/lib/galaxy/webapps/galaxy/api/tool_entry_points.py @@ -51,7 +51,7 @@ def index(self, trans: ProvidesUserContext, running=False, job_id=None, **kwd): ) if job_id is not None: - job = trans.sa_session.query(Job).get(self.decode_id(job_id)) + job = trans.sa_session.get(Job, self.decode_id(job_id)) if not self.interactivetool_manager.can_access_job(trans, job): raise exceptions.ItemAccessibilityException() entry_points = job.interactivetool_entry_points @@ -94,7 +94,7 @@ def stop_entry_point(self, trans: ProvidesUserContext, id, **kwds): raise exceptions.RequestParameterMissingException("Must supply entry point id") try: entry_point_id = self.decode_id(id) - entry_point = trans.sa_session.query(InteractiveToolEntryPoint).get(entry_point_id) + entry_point = trans.sa_session.get(InteractiveToolEntryPoint, entry_point_id) except Exception: raise exceptions.RequestParameterInvalidException("entry point invalid") if self.app.interactivetool_manager.can_access_entry_point(trans, entry_point): diff --git a/lib/galaxy/webapps/galaxy/api/users.py b/lib/galaxy/webapps/galaxy/api/users.py index 20a594f33ac4..14b8160bcdd0 100644 --- a/lib/galaxy/webapps/galaxy/api/users.py +++ b/lib/galaxy/webapps/galaxy/api/users.py @@ -34,6 +34,9 @@ ProvidesUserContext, ) from galaxy.model import ( + FormDefinition, + HistoryDatasetAssociation, + Role, UserAddress, UserQuotaUsage, ) @@ -491,7 +494,7 @@ def add_custom_builds( build_dict["count"] = str(counter) else: build_dict["fasta"] = trans.security.decode_id(len_value) - dataset = trans.sa_session.query(trans.app.model.HistoryDatasetAssociation).get(build_dict["fasta"]) + dataset = trans.sa_session.get(HistoryDatasetAssociation, int(build_dict["fasta"])) try: new_len = dataset.get_converted_dataset(trans, "len") new_linecount = new_len.get_converted_dataset(trans, "linecount") @@ -519,9 +522,7 @@ def get_custom_builds( update = False for key, dbkey in dbkeys.items(): if "count" not in dbkey and "linecount" in dbkey: - chrom_count_dataset = trans.sa_session.query(trans.app.model.HistoryDatasetAssociation).get( - dbkey["linecount"] - ) + chrom_count_dataset = trans.sa_session.get(HistoryDatasetAssociation, dbkey["linecount"]) if ( chrom_count_dataset and not chrom_count_dataset.deleted @@ -779,7 +780,7 @@ def get_information(self, trans, id, **kwd): } ) info_form_models = self.get_all_forms( - trans, filter=dict(deleted=False), form_type=trans.app.model.FormDefinition.types.USER_INFO + trans, filter=dict(deleted=False), form_type=FormDefinition.types.USER_INFO ) if info_form_models: info_form_id = trans.security.encode_id(user.values.form_definition.id) if user.values else None @@ -909,9 +910,7 @@ def set_information(self, trans, id, payload=None, **kwd): user_info_form_id = payload.get("info|form_id") if user_info_form_id: prefix = "info|" - user_info_form = trans.sa_session.query(trans.app.model.FormDefinition).get( - trans.security.decode_id(user_info_form_id) - ) + user_info_form = trans.sa_session.get(FormDefinition, trans.security.decode_id(user_info_form_id)) user_info_values = {} for item in payload: if item.startswith(prefix): @@ -962,7 +961,7 @@ def set_information(self, trans, id, payload=None, **kwd): d = address_dicts[index] if d.get("id"): try: - user_address = trans.sa_session.query(UserAddress).get(trans.security.decode_id(d["id"])) + user_address = trans.sa_session.get(UserAddress, trans.security.decode_id(d["id"])) except Exception as e: raise exceptions.ObjectNotFound(f"Failed to access user address ({d['id']}). {e}") else: @@ -1042,9 +1041,7 @@ def set_permissions(self, trans, id, payload=None, **kwd): permissions = {} for index, action in trans.app.model.Dataset.permitted_actions.items(): action_id = trans.app.security_agent.get_action(action.action).action - permissions[action_id] = [ - trans.sa_session.query(trans.app.model.Role).get(x) for x in (payload.get(index) or []) - ] + permissions[action_id] = [trans.sa_session.get(Role, x) for x in (payload.get(index) or [])] trans.app.security_agent.user_set_default_permissions(user, permissions) return {"message": "Permissions have been saved."} diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index f3f94d81fda9..7971d1115773 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -40,6 +40,7 @@ invocation_job_source_iter, ) from galaxy.managers.workflows import ( + count_stored_workflow_user_assocs, MissingToolsException, RefactorRequest, WorkflowCreateOptions, @@ -156,13 +157,12 @@ def set_workflow_menu(self, trans: GalaxyWebTransaction, payload=None, **kwd): # Decode the encoded workflow ids for ids in workflow_ids: workflow_ids_decoded.append(trans.security.decode_id(ids)) - sess = trans.sa_session + session = trans.sa_session # This explicit remove seems like a hack, need to figure out # how to make the association do it automatically. for m in user.stored_workflow_menu_entries: - sess.delete(m) + session.delete(m) user.stored_workflow_menu_entries = [] - q = sess.query(model.StoredWorkflow) # To ensure id list is unique seen_workflow_ids = set() for wf_id in workflow_ids_decoded: @@ -171,10 +171,11 @@ def set_workflow_menu(self, trans: GalaxyWebTransaction, payload=None, **kwd): else: seen_workflow_ids.add(wf_id) m = model.StoredWorkflowMenuEntry() - m.stored_workflow = q.get(wf_id) + m.stored_workflow = session.get(model.StoredWorkflow, wf_id) + user.stored_workflow_menu_entries.append(m) - with transaction(sess): - sess.commit() + with transaction(session): + session.commit() message = "Menu updated." trans.set_message(message) return {"message": message, "status": "done"} @@ -192,12 +193,8 @@ def show(self, trans: GalaxyWebTransaction, id, **kwd): """ stored_workflow = self.__get_stored_workflow(trans, id, **kwd) if stored_workflow.importable is False and stored_workflow.user != trans.user and not trans.user_is_admin: - if ( - trans.sa_session.query(model.StoredWorkflowUserShareAssociation) - .filter_by(user=trans.user, stored_workflow=stored_workflow) - .count() - == 0 - ): + wf_count = count_stored_workflow_user_assocs(trans.sa_session, trans.user, stored_workflow) + if wf_count == 0: message = "Workflow is neither importable, nor owned by or shared with current user" raise exceptions.ItemAccessibilityException(message) if kwd.get("legacy", False): From b58d741f4514b8719bd182f2fb38fa8aeb103da4 Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 21 Jul 2023 16:08:53 -0400 Subject: [PATCH 12/50] Fix SA2.0 ORM usage in galaxy.actions --- lib/galaxy/actions/library.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/actions/library.py b/lib/galaxy/actions/library.py index 2311adeb2575..b1f3a8bbfae8 100644 --- a/lib/galaxy/actions/library.py +++ b/lib/galaxy/actions/library.py @@ -16,7 +16,10 @@ ObjectNotFound, RequestParameterInvalidException, ) -from galaxy.model import LibraryDataset +from galaxy.model import ( + LibraryDataset, + LibraryFolder, +) from galaxy.model.base import transaction from galaxy.tools.actions import upload_common from galaxy.tools.parameters import populate_state @@ -307,12 +310,12 @@ def _make_library_uploaded_dataset(self, trans, params, name, path, type, librar def _create_folder(self, trans, parent_id: int, **kwd): is_admin = trans.user_is_admin current_user_roles = trans.get_current_user_roles() - parent_folder = trans.sa_session.query(trans.app.model.LibraryFolder).get(parent_id) + parent_folder = trans.sa_session.get(LibraryFolder, parent_id) # Check the library which actually contains the user-supplied parent folder, not the user-supplied # library, which could be anything. self._check_access(trans, is_admin, parent_folder, current_user_roles) self._check_add(trans, is_admin, parent_folder, current_user_roles) - new_folder = trans.app.model.LibraryFolder(name=kwd.get("name", ""), description=kwd.get("description", "")) + new_folder = LibraryFolder(name=kwd.get("name", ""), description=kwd.get("description", "")) # We are associating the last used genome build with folders, so we will always # initialize a new folder with the first dbkey in genome builds list which is currently # ? unspecified (?) @@ -347,7 +350,7 @@ def _check_access(self, trans, is_admin, item, current_user_roles): ): if isinstance(item, trans.model.Library): item_type = "data library" - elif isinstance(item, trans.model.LibraryFolder): + elif isinstance(item, LibraryFolder): item_type = "folder" else: item_type = "(unknown item type)" From 5547675e2e6a1fac6545b1b9a796e431fe73c69f Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 21 Jul 2023 17:02:56 -0400 Subject: [PATCH 13/50] Fix SA2.0 ORM usage in galaxy.celery --- lib/galaxy/celery/tasks.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/celery/tasks.py b/lib/galaxy/celery/tasks.py index 23d726a06229..64e251fa6e38 100644 --- a/lib/galaxy/celery/tasks.py +++ b/lib/galaxy/celery/tasks.py @@ -34,6 +34,10 @@ from galaxy.managers.notification import NotificationManager from galaxy.managers.tool_data import ToolDataImportManager from galaxy.metadata.set_metadata import set_metadata_portable +from galaxy.model import ( + Job, + User, +) from galaxy.model.base import transaction from galaxy.model.scoped_session import galaxy_scoped_session from galaxy.objectstore import BaseObjectStore @@ -81,7 +85,7 @@ def recalculate_user_disk_usage( session: galaxy_scoped_session, object_store: BaseObjectStore, task_user_id: Optional[int] = None ): if task_user_id: - user = session.query(model.User).get(task_user_id) + user = session.get(User, task_user_id) if user: user.calculate_and_set_disk_usage(object_store) else: @@ -165,7 +169,8 @@ def touch( ): if model_class != "HistoryDatasetCollectionAssociation": raise NotImplementedError(f"touch method not implemented for '{model_class}'") - item = sa_session.query(model.HistoryDatasetCollectionAssociation).filter_by(id=item_id).one() + stmt = select(model.HistoryDatasetCollectionAssociation).filter_by(id=item_id) + item = sa_session.execute(stmt).scalar_one() item.touch() with transaction(sa_session): sa_session.commit() @@ -221,7 +226,7 @@ def setup_fetch_data( task_user_id: Optional[int] = None, ): tool = cached_create_tool_from_representation(app=app, raw_tool_source=raw_tool_source) - job = sa_session.query(model.Job).get(job_id) + job = sa_session.get(Job, job_id) # self.request.hostname is the actual worker name given by the `-n` argument, not the hostname as you might think. job.handler = self.request.hostname job.job_runner_name = "celery" @@ -253,7 +258,7 @@ def finish_job( task_user_id: Optional[int] = None, ): tool = cached_create_tool_from_representation(app=app, raw_tool_source=raw_tool_source) - job = sa_session.query(model.Job).get(job_id) + job = sa_session.get(Job, job_id) # TODO: assert state ? mini_job_wrapper = MinimalJobWrapper(job=job, app=app, tool=tool) mini_job_wrapper.finish("", "") @@ -313,7 +318,7 @@ def fetch_data( sa_session: galaxy_scoped_session, task_user_id: Optional[int] = None, ) -> str: - job = sa_session.query(model.Job).get(job_id) + job = sa_session.get(Job, job_id) mini_job_wrapper = MinimalJobWrapper(job=job, app=app) mini_job_wrapper.change_state(model.Job.states.RUNNING, flush=True, job=job) return abort_when_job_stops(_fetch_data, session=sa_session, job_id=job_id, setup_return=setup_return) From 9bfc041a47bf21578486b740527faa8f6a91402b Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 21 Jul 2023 17:40:43 -0400 Subject: [PATCH 14/50] Fix SA2.0 ORM usage in galaxy.quota --- lib/galaxy/quota/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/quota/__init__.py b/lib/galaxy/quota/__init__.py index e047aa76e10a..dfe9ba4acb22 100644 --- a/lib/galaxy/quota/__init__.py +++ b/lib/galaxy/quota/__init__.py @@ -2,6 +2,7 @@ import logging from typing import Optional +from sqlalchemy import select from sqlalchemy.sql import text import galaxy.util @@ -209,11 +210,10 @@ def set_default_quota(self, default_type, quota): self.sa_session.delete(gqa) # Find the old default, assign the new quota if it exists label = quota.quota_source_label - dqas = ( - self.sa_session.query(self.model.DefaultQuotaAssociation) - .filter(self.model.DefaultQuotaAssociation.table.c.type == default_type) - .all() + stmt = select(self.model.DefaultQuotaAssociation).filter( + self.model.DefaultQuotaAssociation.type == default_type ) + dqas = self.sa_session.scalars(stmt).all() target_default = None for dqa in dqas: if dqa.quota.quota_source_label == label and not dqa.quota.deleted: From eeba6d6d5aa91d10019ea9d3234bd32ccdc9bc95 Mon Sep 17 00:00:00 2001 From: John Davis Date: Mon, 7 Aug 2023 15:21:23 -0400 Subject: [PATCH 15/50] Fix SA2.0 ORM usage in galaxy.security --- lib/galaxy/security/validate_user_input.py | 18 +++++++++--------- lib/galaxy/security/vault.py | 9 +++++++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/galaxy/security/validate_user_input.py b/lib/galaxy/security/validate_user_input.py index 86d890cdf3cb..a5400bd0ee02 100644 --- a/lib/galaxy/security/validate_user_input.py +++ b/lib/galaxy/security/validate_user_input.py @@ -10,7 +10,10 @@ import dns.resolver from dns.exception import DNSException -from sqlalchemy import func +from sqlalchemy import ( + func, + select, +) from typing_extensions import LiteralString from galaxy.objectstore import ObjectStore @@ -78,13 +81,8 @@ def validate_email(trans, email, user=None, check_dup=True, allow_empty=False, v domain = extract_domain(email) message = validate_email_domain_name(domain) - if ( - not message - and check_dup - and trans.sa_session.query(trans.app.model.User) - .filter(func.lower(trans.app.model.User.table.c.email) == email.lower()) - .first() - ): + stmt = select(trans.app.model.User).filter(func.lower(trans.app.model.User.email) == email.lower()).limit(1) + if not message and check_dup and trans.sa_session.scalars(stmt).first(): message = f"User with email '{email}' already exists." if not message: @@ -134,7 +132,9 @@ def validate_publicname(trans, publicname, user=None): message = validate_publicname_str(publicname) if message: return message - if trans.sa_session.query(trans.app.model.User).filter_by(username=publicname).first(): + + stmt = select(trans.app.model.User).filter_by(username=publicname).limit(1) + if trans.sa_session.scalars(stmt).first(): return "Public name is taken; please choose another." return "" diff --git a/lib/galaxy/security/vault.py b/lib/galaxy/security/vault.py index 5a1d4a557b62..ee57aa2d1100 100644 --- a/lib/galaxy/security/vault.py +++ b/lib/galaxy/security/vault.py @@ -12,6 +12,7 @@ Fernet, MultiFernet, ) +from sqlalchemy import select try: from custos.clients.resource_secret_management_client import ResourceSecretManagementClient @@ -130,7 +131,7 @@ def _get_multi_fernet(self) -> MultiFernet: return MultiFernet(self.fernet_keys) def _update_or_create(self, key: str, value: Optional[str]) -> model.Vault: - vault_entry = self.sa_session.query(model.Vault).filter_by(key=key).first() + vault_entry = self._get_vault_value(key) if vault_entry: if value: vault_entry.value = value @@ -149,7 +150,7 @@ def _update_or_create(self, key: str, value: Optional[str]) -> model.Vault: return vault_entry def read_secret(self, key: str) -> Optional[str]: - key_obj = self.sa_session.query(model.Vault).filter_by(key=key).first() + key_obj = self._get_vault_value(key) if key_obj and key_obj.value: f = self._get_multi_fernet() return f.decrypt(key_obj.value.encode("utf-8")).decode("utf-8") @@ -163,6 +164,10 @@ def write_secret(self, key: str, value: str) -> None: def list_secrets(self, key: str) -> List[str]: raise NotImplementedError() + def _get_vault_value(self, key): + stmt = select(model.Vault).filter_by(key=key).limit(1) + return self.sa_session.scalars(stmt).first() + class CustosVault(Vault): def __init__(self, config): From 3250ed8f64c232abe075ed8f9740918c4743675e Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 8 Aug 2023 14:32:37 -0400 Subject: [PATCH 16/50] Fix SA2.0 ORM usage in galaxy.queue_worker --- lib/galaxy/queue_worker.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/queue_worker.py b/lib/galaxy/queue_worker.py index af28f0655cbb..7de46f1b9a5c 100644 --- a/lib/galaxy/queue_worker.py +++ b/lib/galaxy/queue_worker.py @@ -23,6 +23,7 @@ import galaxy.queues from galaxy import util from galaxy.config import reload_config_options +from galaxy.model import User from galaxy.tools import ToolBox from galaxy.tools.data_manager.manager import DataManagers from galaxy.tools.special_tools import load_lib_tools @@ -219,7 +220,7 @@ def recalculate_user_disk_usage(app, **kwargs): user_id = kwargs.get("user_id", None) sa_session = app.model.context if user_id: - user = sa_session.query(app.model.User).get(user_id) + user = sa_session.get(User, user_id) if user: user.calculate_and_set_disk_usage(app.object_store) else: From 19a1f90a96cbf245949b1bbd6b9019482ffceade Mon Sep 17 00:00:00 2001 From: John Davis Date: Thu, 10 Aug 2023 15:08:31 -0400 Subject: [PATCH 17/50] Fix SA2.0 ORM usage in galaxy.workflow --- lib/galaxy/workflow/run_request.py | 14 +++++--------- lib/galaxy/workflow/scheduling_manager.py | 4 ++-- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/lib/galaxy/workflow/run_request.py b/lib/galaxy/workflow/run_request.py index 37670ead736c..3e2b57fdbf09 100644 --- a/lib/galaxy/workflow/run_request.py +++ b/lib/galaxy/workflow/run_request.py @@ -370,26 +370,22 @@ def build_workflow_run_configs( input_id = input_dict["id"] try: if input_source == "ldda": - ldda = trans.sa_session.query(LibraryDatasetDatasetAssociation).get( - trans.security.decode_id(input_id) - ) + ldda = trans.sa_session.get(LibraryDatasetDatasetAssociation, trans.security.decode_id(input_id)) assert trans.user_is_admin or trans.app.security_agent.can_access_dataset( trans.get_current_user_roles(), ldda.dataset ) content = ldda.to_history_dataset_association(history, add_to_history=add_to_history) elif input_source == "ld": - ldda = ( - trans.sa_session.query(LibraryDataset) - .get(trans.security.decode_id(input_id)) - .library_dataset_dataset_association - ) + ldda = trans.sa_session.get( + LibraryDataset, trans.security.decode_id(input_id) + ).library_dataset_dataset_association assert trans.user_is_admin or trans.app.security_agent.can_access_dataset( trans.get_current_user_roles(), ldda.dataset ) content = ldda.to_history_dataset_association(history, add_to_history=add_to_history) elif input_source == "hda": # Get dataset handle, add to dict and history if necessary - content = trans.sa_session.query(HistoryDatasetAssociation).get(trans.security.decode_id(input_id)) + content = trans.sa_session.get(HistoryDatasetAssociation, trans.security.decode_id(input_id)) assert trans.user_is_admin or trans.app.security_agent.can_access_dataset( trans.get_current_user_roles(), content.dataset ) diff --git a/lib/galaxy/workflow/scheduling_manager.py b/lib/galaxy/workflow/scheduling_manager.py index a244a54fabd4..fc44e59b699a 100644 --- a/lib/galaxy/workflow/scheduling_manager.py +++ b/lib/galaxy/workflow/scheduling_manager.py @@ -79,12 +79,12 @@ def __startup_recovery(self): log.info( "(%s) Handler unassigned at startup, resubmitting workflow invocation for assignment", invocation_id ) - workflow_invocation = sa_session.query(model.WorkflowInvocation).get(invocation_id) + workflow_invocation = sa_session.get(model.WorkflowInvocation, invocation_id) self._assign_handler(workflow_invocation) def _handle_setup_msg(self, workflow_invocation_id=None): sa_session = self.app.model.context - workflow_invocation = sa_session.query(model.WorkflowInvocation).get(workflow_invocation_id) + workflow_invocation = sa_session.get(model.WorkflowInvocation, workflow_invocation_id) if workflow_invocation.handler is None: workflow_invocation.handler = self.app.config.server_name sa_session.add(workflow_invocation) From 41fa1cbf46423bbc9efbb6879415d13fcea95afd Mon Sep 17 00:00:00 2001 From: John Davis Date: Thu, 10 Aug 2023 15:03:56 -0400 Subject: [PATCH 18/50] Fix SA2.0 ORM usage in galaxy.tools --- lib/galaxy/tools/__init__.py | 23 ++++-- lib/galaxy/tools/actions/__init__.py | 6 +- lib/galaxy/tools/actions/upload_common.py | 17 +++-- lib/galaxy/tools/errors.py | 4 +- lib/galaxy/tools/imp_exp/__init__.py | 5 +- lib/galaxy/tools/parameters/basic.py | 85 ++++++++++++----------- lib/galaxy/tools/parameters/meta.py | 4 +- 7 files changed, 83 insertions(+), 61 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index e2dacab65dcb..fc9b86b07d24 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -29,6 +29,11 @@ from lxml import etree from mako.template import Template from packaging.version import Version +from sqlalchemy import ( + delete, + func, + select, +) from galaxy import ( exceptions, @@ -37,6 +42,10 @@ from galaxy.exceptions import ToolInputsNotReadyException from galaxy.job_execution import output_collect from galaxy.metadata import get_metadata_compute_strategy +from galaxy.model import ( + Job, + StoredWorkflow, +) from galaxy.model.base import transaction from galaxy.tool_shed.util.repository_util import get_installed_repository from galaxy.tool_shed.util.shed_util_common import set_image_paths @@ -351,9 +360,9 @@ def __init__(self, app): def reset_tags(self): log.info( - f"removing all tool tag associations ({str(self.sa_session.query(self.app.model.ToolTagAssociation).count())})" + f"removing all tool tag associations ({str(self.sa_session.scalar(select(func.count(self.app.model.ToolTagAssociation))))})" ) - self.sa_session.query(self.app.model.ToolTagAssociation).delete() + self.sa_session.execute(delete(self.app.model.ToolTagAssociation)) with transaction(self.sa_session): self.sa_session.commit() @@ -364,7 +373,8 @@ def handle_tags(self, tool_id, tool_definition_source): for tag_name in tag_names: if tag_name == "": continue - tag = self.sa_session.query(self.app.model.Tag).filter_by(name=tag_name).first() + stmt = select(self.app.model.Tag).filter_by(name=tag_name).limit(1) + tag = self.sa_session.scalars(stmt).first() if not tag: tag = self.app.model.Tag(name=tag_name) self.sa_session.add(tag) @@ -620,7 +630,8 @@ def _load_workflow(self, workflow_id): which is encoded in the tool panel. """ id = self.app.security.decode_id(workflow_id) - stored = self.app.model.context.query(self.app.model.StoredWorkflow).get(id) + session = self.app.model.context + stored = session.get(StoredWorkflow, id) return stored.latest_workflow def __build_tool_version_select_field(self, tools, tool_id, set_selected): @@ -3017,7 +3028,7 @@ def exec_after_process(self, app, inp_data, out_data, param_dict, job=None, **kw self.sa_session.commit() def job_failed(self, job_wrapper, message, exception=False): - job = job_wrapper.sa_session.query(model.Job).get(job_wrapper.job_id) + job = job_wrapper.sa_session.get(Job, job_wrapper.job_id) if job: inp_data = {} for dataset_assoc in job.input_datasets: @@ -3064,7 +3075,7 @@ def exec_after_process(self, app, inp_data, out_data, param_dict, job=None, **kw def job_failed(self, job_wrapper, message, exception=False): super().job_failed(job_wrapper, message, exception=exception) - job = job_wrapper.sa_session.query(model.Job).get(job_wrapper.job_id) + job = job_wrapper.sa_session.get(Job, job_wrapper.job_id) self.__remove_interactivetool_by_job(job) diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py index 4f9aa4e1b55d..3aa2fa219f4e 100644 --- a/lib/galaxy/tools/actions/__init__.py +++ b/lib/galaxy/tools/actions/__init__.py @@ -18,6 +18,8 @@ from galaxy.exceptions import ItemAccessibilityException from galaxy.job_execution.actions.post import ActionBox from galaxy.model import ( + HistoryDatasetAssociation, + Job, LibraryDatasetDatasetAssociation, WorkflowRequestInputParameter, ) @@ -482,7 +484,7 @@ def handle_output(name, output, hidden=None): if async_tool and name in incoming: # HACK: output data has already been created as a result of the async controller dataid = incoming[name] - data = trans.sa_session.query(app.model.HistoryDatasetAssociation).get(dataid) + data = trans.sa_session.get(HistoryDatasetAssociation, dataid) assert data is not None out_data[name] = data else: @@ -746,7 +748,7 @@ def _remap_job_on_rerun(self, trans, galaxy_session, rerun_remap_job_id, current input datasets to be those of the job that is being rerun. """ try: - old_job = trans.sa_session.query(trans.app.model.Job).get(rerun_remap_job_id) + old_job = trans.sa_session.get(Job, rerun_remap_job_id) assert old_job is not None, f"({rerun_remap_job_id}/{current_job.id}): Old job id is invalid" assert ( old_job.tool_id == current_job.tool_id diff --git a/lib/galaxy/tools/actions/upload_common.py b/lib/galaxy/tools/actions/upload_common.py index 391b9b4a5ee1..507b873172ce 100644 --- a/lib/galaxy/tools/actions/upload_common.py +++ b/lib/galaxy/tools/actions/upload_common.py @@ -13,6 +13,7 @@ Optional, ) +from sqlalchemy import select from sqlalchemy.orm import joinedload from webob.compat import cgi_FieldStorage @@ -87,6 +88,7 @@ class LibraryParams: def handle_library_params( trans, params, folder_id: int, replace_dataset: Optional[LibraryDataset] = None ) -> LibraryParams: + session = trans.sa_session # FIXME: the received params has already been parsed by util.Params() by the time it reaches here, # so no complex objects remain. This is not good because it does not allow for those objects to be # manipulated here. The received params should be the original kwd from the initial request. @@ -94,12 +96,12 @@ def handle_library_params( # See if we have any template field contents template_field_contents = {} template_id = params.get("template_id", None) - folder = trans.sa_session.query(LibraryFolder).get(folder_id) + folder = session.get(LibraryFolder, folder_id) # We are inheriting the folder's info_association, so we may have received inherited contents or we may have redirected # here after the user entered template contents ( due to errors ). template: Optional[FormDefinition] = None if template_id not in [None, "None"]: - template = trans.sa_session.query(FormDefinition).get(template_id) + template = session.get(FormDefinition, template_id) assert template for field in template.fields: field_name = field["name"] @@ -108,7 +110,7 @@ def handle_library_params( template_field_contents[field_name] = field_value roles: List[Role] = [] for role_id in util.listify(params.get("roles", [])): - role = trans.sa_session.query(Role).get(role_id) + role = session.get(Role, role_id) roles.append(role) tags = params.get("tags", None) return LibraryParams( @@ -436,10 +438,11 @@ def active_folders(trans, folder): # Stolen from galaxy.web.controllers.library_common (importing from which causes a circular issues). # Much faster way of retrieving all active sub-folders within a given folder than the # performance of the mapper. This query also eagerloads the permissions on each folder. - return ( - trans.sa_session.query(LibraryFolder) + stmt = ( + select(LibraryFolder) .filter_by(parent=folder, deleted=False) .options(joinedload(LibraryFolder.actions)) - .order_by(LibraryFolder.table.c.name) - .all() + .unique() + .order_by(LibraryFolder.name) ) + return trans.sa_session.scalars(stmt).all() diff --git a/lib/galaxy/tools/errors.py b/lib/galaxy/tools/errors.py index 26aa0e017af9..a2ee14a69ddf 100644 --- a/lib/galaxy/tools/errors.py +++ b/lib/galaxy/tools/errors.py @@ -137,10 +137,10 @@ def __init__(self, hda, app): if not isinstance(hda, model.HistoryDatasetAssociation): hda_id = hda try: - hda = sa_session.query(model.HistoryDatasetAssociation).get(hda_id) + hda = sa_session.get(model.HistoryDatasetAssociation, hda_id) assert hda is not None, ValueError("No HDA yet") except Exception: - hda = sa_session.query(model.HistoryDatasetAssociation).get(app.security.decode_id(hda_id)) + hda = sa_session.get(model.HistoryDatasetAssociation, app.security.decode_id(hda_id)) assert isinstance(hda, model.HistoryDatasetAssociation), ValueError(f"Bad value provided for HDA ({hda}).") self.hda = hda # Get the associated job diff --git a/lib/galaxy/tools/imp_exp/__init__.py b/lib/galaxy/tools/imp_exp/__init__.py index 4c8c53a91141..a1338d8c453b 100644 --- a/lib/galaxy/tools/imp_exp/__init__.py +++ b/lib/galaxy/tools/imp_exp/__init__.py @@ -4,6 +4,8 @@ import shutil from typing import Optional +from sqlalchemy import select + from galaxy import model from galaxy.model import store from galaxy.model.base import transaction @@ -49,7 +51,8 @@ def cleanup_after_job(self): # Import history. # - jiha = self.sa_session.query(model.JobImportHistoryArchive).filter_by(job_id=self.job_id).first() + stmt = select(model.JobImportHistoryArchive).filter_by(job_id=self.job_id).limit(1) + jiha = self.sa_session.scalars(stmt).first() if not jiha: return None user = jiha.job.user diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index a4cc61d5f66c..5025ef1336e8 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1943,13 +1943,15 @@ def single_to_python(value): if isinstance(value, dict) and "src" in value: id = value["id"] if isinstance(value["id"], int) else app.security.decode_id(value["id"]) if value["src"] == "dce": - return app.model.context.query(DatasetCollectionElement).get(id) + return session.get(DatasetCollectionElement, id) elif value["src"] == "hdca": - return app.model.context.query(HistoryDatasetCollectionAssociation).get(id) + return session.get(HistoryDatasetCollectionAssociation, id) elif value["src"] == "ldda": - return app.model.context.query(LibraryDatasetDatasetAssociation).get(id) + return session.get(LibraryDatasetDatasetAssociation, id) else: - return app.model.context.query(HistoryDatasetAssociation).get(id) + return session.get(HistoryDatasetAssociation, id) + + session = app.model.context if isinstance(value, dict) and "values" in value: if hasattr(self, "multiple") and self.multiple is True: @@ -1962,22 +1964,18 @@ def single_to_python(value): if value in none_values: return None if isinstance(value, str) and value.find(",") > -1: - return [ - app.model.context.query(HistoryDatasetAssociation).get(int(v)) - for v in value.split(",") - if v not in none_values - ] + return [session.get(HistoryDatasetAssociation, int(v)) for v in value.split(",") if v not in none_values] elif str(value).startswith("__collection_reduce__|"): decoded_id = str(value)[len("__collection_reduce__|") :] if not decoded_id.isdigit(): decoded_id = app.security.decode_id(decoded_id) - return app.model.context.query(HistoryDatasetCollectionAssociation).get(int(decoded_id)) + return session.get(HistoryDatasetCollectionAssociation, int(decoded_id)) elif str(value).startswith("dce:"): - return app.model.context.query(DatasetCollectionElement).get(int(value[len("dce:") :])) + return session.get(DatasetCollectionElement, int(value[len("dce:") :])) elif str(value).startswith("hdca:"): - return app.model.context.query(HistoryDatasetCollectionAssociation).get(int(value[len("hdca:") :])) + return session.get(HistoryDatasetCollectionAssociation, int(value[len("hdca:") :])) else: - return app.model.context.query(HistoryDatasetAssociation).get(int(value)) + return session.get(HistoryDatasetAssociation, int(value)) def validate(self, value, trans=None): def do_validate(v): @@ -2079,6 +2077,8 @@ def __init__(self, tool, input_source, trans=None): self.conversions.append((name, conv_extension, [conv_type])) def from_json(self, value, trans, other_values=None): + session = trans.sa_session + other_values = other_values or {} if trans.workflow_building_mode is workflow_building_modes.ENABLED or is_runtime_value(value): return None @@ -2090,24 +2090,31 @@ def from_json(self, value, trans, other_values=None): value = self.to_python(value, trans.app) if isinstance(value, str) and value.find(",") > 0: value = [int(value_part) for value_part in value.split(",")] - rval = [] + rval: List[ + Union[ + DatasetCollectionElement, + HistoryDatasetAssociation, + HistoryDatasetCollectionAssociation, + LibraryDatasetDatasetAssociation, + ] + ] = [] if isinstance(value, list): found_hdca = False for single_value in value: if isinstance(single_value, dict) and "src" in single_value and "id" in single_value: if single_value["src"] == "hda": decoded_id = trans.security.decode_id(single_value["id"]) - rval.append(trans.sa_session.query(HistoryDatasetAssociation).get(decoded_id)) + rval.append(session.get(HistoryDatasetAssociation, decoded_id)) elif single_value["src"] == "hdca": found_hdca = True decoded_id = trans.security.decode_id(single_value["id"]) - rval.append(trans.sa_session.query(HistoryDatasetCollectionAssociation).get(decoded_id)) + rval.append(session.get(HistoryDatasetCollectionAssociation, decoded_id)) elif single_value["src"] == "ldda": decoded_id = trans.security.decode_id(single_value["id"]) - rval.append(trans.sa_session.query(LibraryDatasetDatasetAssociation).get(decoded_id)) + rval.append(session.get(LibraryDatasetDatasetAssociation, decoded_id)) elif single_value["src"] == "dce": decoded_id = trans.security.decode_id(single_value["id"]) - rval.append(trans.sa_session.query(DatasetCollectionElement).get(decoded_id)) + rval.append(session.get(DatasetCollectionElement, decoded_id)) else: raise ValueError(f"Unknown input source {single_value['src']} passed to job submission API.") elif isinstance( @@ -2126,7 +2133,7 @@ def from_json(self, value, trans, other_values=None): # support that for integer column types. log.warning("Encoded ID where unencoded ID expected.") single_value = trans.security.decode_id(single_value) - rval.append(trans.sa_session.query(HistoryDatasetAssociation).get(single_value)) + rval.append(session.get(HistoryDatasetAssociation, single_value)) if found_hdca: for val in rval: if not isinstance(val, HistoryDatasetCollectionAssociation): @@ -2142,13 +2149,13 @@ def from_json(self, value, trans, other_values=None): rval.append(trans.sa_session.query(LibraryDatasetDatasetAssociation).get(decoded_id)) if value["src"] == "hda": decoded_id = trans.security.decode_id(value["id"]) - rval.append(trans.sa_session.query(HistoryDatasetAssociation).get(decoded_id)) + rval.append(session.get(HistoryDatasetAssociation, decoded_id)) elif value["src"] == "hdca": decoded_id = trans.security.decode_id(value["id"]) - rval.append(trans.sa_session.query(HistoryDatasetCollectionAssociation).get(decoded_id)) + rval.append(session.get(HistoryDatasetCollectionAssociation, decoded_id)) elif value["src"] == "dce": decoded_id = trans.security.decode_id(value["id"]) - rval.append(trans.sa_session.query(DatasetCollectionElement).get(decoded_id)) + rval.append(session.get(DatasetCollectionElement, decoded_id)) else: raise ValueError(f"Unknown input source {value['src']} passed to job submission API.") elif str(value).startswith("__collection_reduce__|"): @@ -2156,12 +2163,12 @@ def from_json(self, value, trans, other_values=None): decoded_ids = map(trans.security.decode_id, encoded_ids) rval = [] for decoded_id in decoded_ids: - hdca = trans.sa_session.query(HistoryDatasetCollectionAssociation).get(decoded_id) + hdca = session.get(HistoryDatasetCollectionAssociation, decoded_id) rval.append(hdca) elif isinstance(value, HistoryDatasetCollectionAssociation) or isinstance(value, DatasetCollectionElement): rval.append(value) else: - rval.append(trans.sa_session.query(HistoryDatasetAssociation).get(value)) + rval.append(session.get(HistoryDatasetAssociation, int(value))) # type:ignore[arg-type] dataset_matcher_factory = get_dataset_matcher_factory(trans) dataset_matcher = dataset_matcher_factory.dataset_matcher(self, other_values) for v in rval: @@ -2177,12 +2184,12 @@ def from_json(self, value, trans, other_values=None): v = v.hda match = dataset_matcher.hda_match(v) if match and match.implicit_conversion: - v.implicit_conversion = True + v.implicit_conversion = True # type:ignore[union-attr] if not self.multiple: if len(rval) > 1: raise ParameterValueError("more than one dataset supplied to single input dataset parameter", self.name) if len(rval) > 0: - rval = rval[0] + rval = rval[0] # type:ignore[assignment] else: raise ParameterValueError("invalid dataset supplied to single input dataset parameter", self.name) return rval @@ -2428,6 +2435,8 @@ def match_multirun_collections(self, trans, history, dataset_collection_matcher) yield history_dataset_collection, match.implicit_conversion def from_json(self, value, trans, other_values=None): + session = trans.sa_session + other_values = other_values or {} rval: Optional[Union[DatasetCollectionElement, HistoryDatasetCollectionAssociation]] = None if trans.workflow_building_mode is workflow_building_modes.ENABLED: @@ -2449,28 +2458,22 @@ def from_json(self, value, trans, other_values=None): rval = value elif isinstance(value, dict) and "src" in value and "id" in value: if value["src"] == "hdca": - rval = trans.sa_session.query(HistoryDatasetCollectionAssociation).get( - trans.security.decode_id(value["id"]) - ) + rval = session.get(HistoryDatasetCollectionAssociation, trans.security.decode_id(value["id"])) elif isinstance(value, list): if len(value) > 0: value = value[0] if isinstance(value, dict) and "src" in value and "id" in value: if value["src"] == "hdca": - rval = trans.sa_session.query(HistoryDatasetCollectionAssociation).get( - trans.security.decode_id(value["id"]) - ) + rval = session.get(HistoryDatasetCollectionAssociation, trans.security.decode_id(value["id"])) elif value["src"] == "dce": - rval = trans.sa_session.query(DatasetCollectionElement).get( - trans.security.decode_id(value["id"]) - ) + rval = session.get(DatasetCollectionElement, trans.security.decode_id(value["id"])) elif isinstance(value, str): if value.startswith("dce:"): - rval = trans.sa_session.query(DatasetCollectionElement).get(value[len("dce:") :]) + rval = session.get(DatasetCollectionElement, int(value[len("dce:") :])) elif value.startswith("hdca:"): - rval = trans.sa_session.query(HistoryDatasetCollectionAssociation).get(value[len("hdca:") :]) + rval = session.get(HistoryDatasetCollectionAssociation, int(value[len("hdca:") :])) else: - rval = trans.sa_session.query(HistoryDatasetCollectionAssociation).get(value) + rval = session.get(HistoryDatasetCollectionAssociation, int(value)) if rval and isinstance(rval, HistoryDatasetCollectionAssociation): if rval.deleted: raise ParameterValueError("the previously selected dataset collection has been deleted", self.name) @@ -2628,6 +2631,7 @@ def to_python(self, value, app, other_values=None, validate=False): if not isinstance(value, list): value = [value] lst = [] + session = app.model.context for item in value: if isinstance(item, LibraryDatasetDatasetAssociation): lst.append(item) @@ -2640,9 +2644,8 @@ def to_python(self, value, app, other_values=None, validate=False): else: lst = [] break - lda = app.model.context.query(LibraryDatasetDatasetAssociation).get( - lda_id if isinstance(lda_id, int) else app.security.decode_id(lda_id) - ) + id = lda_id if isinstance(lda_id, int) else app.security.decode_id(lda_id) + lda = session.get(LibraryDatasetDatasetAssociation, id) if lda is not None: lst.append(lda) elif validate: diff --git a/lib/galaxy/tools/parameters/meta.py b/lib/galaxy/tools/parameters/meta.py index a9f47ac0eae0..cffb9233b212 100644 --- a/lib/galaxy/tools/parameters/meta.py +++ b/lib/galaxy/tools/parameters/meta.py @@ -12,9 +12,9 @@ from galaxy import ( exceptions, - model, util, ) +from galaxy.model import HistoryDatasetCollectionAssociation from galaxy.model.dataset_collections import ( matching, subcollections, @@ -265,7 +265,7 @@ def __expand_collection_parameter(trans, input_key, incoming_val, collections_to encoded_hdc_id = incoming_val subcollection_type = None hdc_id = trans.app.security.decode_id(encoded_hdc_id) - hdc = trans.sa_session.query(model.HistoryDatasetCollectionAssociation).get(hdc_id) + hdc = trans.sa_session.get(HistoryDatasetCollectionAssociation, hdc_id) collections_to_match.add(input_key, hdc, subcollection_type=subcollection_type, linked=linked) if subcollection_type is not None: subcollection_elements = subcollections.split_dataset_collection_instance(hdc, subcollection_type) From efb097797b0fec58ba119647ed9dec7af5492240 Mon Sep 17 00:00:00 2001 From: John Davis Date: Wed, 20 Sep 2023 15:18:38 -0400 Subject: [PATCH 19/50] Fix SA2.0 ORM usage in managers.cloud --- lib/galaxy/managers/cloud.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/managers/cloud.py b/lib/galaxy/managers/cloud.py index 2a26640c9cee..e9a939b3789f 100644 --- a/lib/galaxy/managers/cloud.py +++ b/lib/galaxy/managers/cloud.py @@ -291,7 +291,7 @@ def get(self, trans, history_id, bucket_name, objects, authz_id, input_args=None params = Params(self._get_inputs(obj, key, input_args), sanitize=False) incoming = params.__dict__ - history = trans.sa_session.query(trans.app.model.History).get(history_id) + history = trans.sa_session.get(model.History, history_id) if not history: raise ObjectNotFound("History with the ID provided was not found.") output = trans.app.toolbox.get_tool("upload1").handle_input(trans, incoming, history=history) @@ -358,7 +358,7 @@ def send(self, trans, history_id, bucket_name, authz_id, dataset_ids=None, overw cloudauthz = trans.app.authnz_manager.try_get_authz_config(trans.sa_session, trans.user.id, authz_id) - history = trans.sa_session.query(trans.app.model.History).get(history_id) + history = trans.sa_session.get(model.History, history_id) if not history: raise ObjectNotFound("History with the provided ID not found.") From efe5224bdfdbb12dd1fa8213ccbfda9af7f15f5a Mon Sep 17 00:00:00 2001 From: John Davis Date: Wed, 20 Sep 2023 15:19:33 -0400 Subject: [PATCH 20/50] Fix SA2.0 ORM usage in managers.dbkeys --- lib/galaxy/managers/dbkeys.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/galaxy/managers/dbkeys.py b/lib/galaxy/managers/dbkeys.py index ead48c9ee756..e005c1499506 100644 --- a/lib/galaxy/managers/dbkeys.py +++ b/lib/galaxy/managers/dbkeys.py @@ -12,6 +12,10 @@ Tuple, ) +from sqlalchemy import select +from sqlalchemy.orm import Session + +from galaxy.model import HistoryDatasetAssociation from galaxy.util import ( galaxy_directory, sanitize_lists_to_string, @@ -96,9 +100,7 @@ def get_genome_build_names(self, trans=None): # It does allow one-off, history specific dbkeys to be created by a user. But we are not filtering, # so a len file will be listed twice (as the build name and again as dataset name), # if custom dbkey creation/conversion occurred within the current history. - datasets = trans.sa_session.query(self._app.model.HistoryDatasetAssociation).filter_by( - deleted=False, history_id=trans.history.id, extension="len" - ) + datasets = get_len_files_by_history(trans.sa_session, trans.history.id) for dataset in datasets: rval.append((dataset.dbkey, f"{dataset.name} ({dataset.dbkey}) [History]")) user = trans.user @@ -140,17 +142,13 @@ def get_chrom_info(self, dbkey, trans=None, custom_build_hack_get_len_from_fasta # fasta-to-len converter. if "fasta" in custom_build_dict and custom_build_hack_get_len_from_fasta_conversion: # Build is defined by fasta; get len file, which is obtained from converting fasta. - build_fasta_dataset = trans.sa_session.query(trans.app.model.HistoryDatasetAssociation).get( - custom_build_dict["fasta"] + build_fasta_dataset = trans.sa_session.get( + HistoryDatasetAssociation, custom_build_dict["fasta"] ) chrom_info = build_fasta_dataset.get_converted_dataset(trans, "len").file_name elif "len" in custom_build_dict: # Build is defined by len file, so use it. - chrom_info = ( - trans.sa_session.query(trans.app.model.HistoryDatasetAssociation) - .get(custom_build_dict["len"]) - .file_name - ) + chrom_info = trans.sa_session.get(HistoryDatasetAssociation, custom_build_dict["len"]).file_name # Check Data table if not chrom_info: dbkey_table = self._app.tool_data_tables.get(self._data_table_name, None) @@ -163,3 +161,8 @@ def get_chrom_info(self, dbkey, trans=None, custom_build_hack_get_len_from_fasta chrom_info = os.path.join(self._static_chrom_info_path, f"{sanitize_lists_to_string(dbkey)}.len") chrom_info = os.path.abspath(chrom_info) return (chrom_info, db_dataset) + + +def get_len_files_by_history(session: Session, history_id: int): + stmt = select(HistoryDatasetAssociation).filter_by(history_id=history_id, extension="len", deleted=False) + return session.scalars(stmt) From 0fd6aec969978a2c15fa62d4d318ace7ff2ede32 Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 8 Aug 2023 18:02:24 -0400 Subject: [PATCH 21/50] Fix SA2.0 ORM usage in galaxy.webapps.base; refactor Drop unused parameters and logic from select methods in legacy controller. --- lib/galaxy/webapps/base/controller.py | 103 ++++++++------------------ lib/galaxy/webapps/base/webapp.py | 22 +++--- 2 files changed, 44 insertions(+), 81 deletions(-) diff --git a/lib/galaxy/webapps/base/controller.py b/lib/galaxy/webapps/base/controller.py index 9335aa84fe2d..dbe30c226fbf 100644 --- a/lib/galaxy/webapps/base/controller.py +++ b/lib/galaxy/webapps/base/controller.py @@ -8,7 +8,10 @@ Optional, ) -from sqlalchemy import true +from sqlalchemy import ( + select, + true, +) from webob.exc import ( HTTPBadRequest, HTTPInternalServerError, @@ -34,6 +37,7 @@ ExtendedMetadata, ExtendedMetadataIndex, HistoryDatasetAssociation, + HistoryDatasetCollectionAssociation, LibraryDatasetDatasetAssociation, ) from galaxy.model.base import transaction @@ -444,7 +448,7 @@ def _copy_hdca_to_library_folder(self, trans, hda_manager, from_hdca_id: int, fo Fetches the collection identified by `from_hcda_id` and dispatches individual collection elements to _copy_hda_to_library_folder """ - hdca = trans.sa_session.query(trans.app.model.HistoryDatasetCollectionAssociation).get(from_hdca_id) + hdca = trans.sa_session.get(HistoryDatasetCollectionAssociation, from_hdca_id) if hdca.collection.collection_type != "list": raise exceptions.NotImplemented( "Cannot add nested collections to library. Please flatten your collection first." @@ -611,7 +615,7 @@ def get_visualization(self, trans, id, check_ownership=True, check_accessible=Fa """ # Load workflow from database try: - visualization = trans.sa_session.query(trans.model.Visualization).get(trans.security.decode_id(id)) + visualization = trans.sa_session.get(model.Visualization, trans.security.decode_id(id)) except TypeError: visualization = None if not visualization: @@ -619,77 +623,32 @@ def get_visualization(self, trans, id, check_ownership=True, check_accessible=Fa else: return self.security_check(trans, visualization, check_ownership, check_accessible) - def get_visualizations_by_user(self, trans, user, order_by=None, query_only=False): - """ - Return query or query results of visualizations filtered by a user. - - Set `order_by` to a column or list of columns to change the order - returned. Defaults to `DEFAULT_ORDER_BY`. - Set `query_only` to return just the query for further filtering or - processing. - """ - # TODO: move into model (as class attr) - DEFAULT_ORDER_BY = [model.Visualization.title] - if not order_by: - order_by = DEFAULT_ORDER_BY - if not isinstance(order_by, list): - order_by = [order_by] - query = trans.sa_session.query(model.Visualization) - query = query.filter(model.Visualization.user == user) - if order_by: - query = query.order_by(*order_by) - if query_only: - return query - return query.all() - - def get_visualizations_shared_with_user(self, trans, user, order_by=None, query_only=False): - """ - Return query or query results for visualizations shared with the given user. + def get_visualizations_by_user(self, trans, user): + """Return query results of visualizations filtered by a user.""" + stmt = select(model.Visualization).filter(model.Visualization.user == user).order_by(model.Visualization.title) + return trans.sa_session.scalars(stmt).all() + + def get_visualizations_shared_with_user(self, trans, user): + """Return query results for visualizations shared with the given user.""" + # The second `where` clause removes duplicates when a user shares with themselves. + stmt = ( + select(model.Visualization) + .join(model.VisualizationUserShareAssociation) + .where(model.VisualizationUserShareAssociation.user_id == user.id) + .where(model.Visualization.user_id != user.id) + .order_by(model.Visualization.title) + ) + return trans.sa_session.scalars(stmt).all() - Set `order_by` to a column or list of columns to change the order - returned. Defaults to `DEFAULT_ORDER_BY`. - Set `query_only` to return just the query for further filtering or - processing. + def get_published_visualizations(self, trans, exclude_user=None): """ - DEFAULT_ORDER_BY = [model.Visualization.title] - if not order_by: - order_by = DEFAULT_ORDER_BY - if not isinstance(order_by, list): - order_by = [order_by] - query = trans.sa_session.query(model.Visualization).join(model.VisualizationUserShareAssociation) - query = query.filter(model.VisualizationUserShareAssociation.user_id == user.id) - # remove duplicates when a user shares with themselves? - query = query.filter(model.Visualization.user_id != user.id) - if order_by: - query = query.order_by(*order_by) - if query_only: - return query - return query.all() - - def get_published_visualizations(self, trans, exclude_user=None, order_by=None, query_only=False): - """ - Return query or query results for published visualizations optionally excluding - the user in `exclude_user`. - - Set `order_by` to a column or list of columns to change the order - returned. Defaults to `DEFAULT_ORDER_BY`. - Set `query_only` to return just the query for further filtering or - processing. + Return query results for published visualizations optionally excluding the user in `exclude_user`. """ - DEFAULT_ORDER_BY = [model.Visualization.title] - if not order_by: - order_by = DEFAULT_ORDER_BY - if not isinstance(order_by, list): - order_by = [order_by] - query = trans.sa_session.query(model.Visualization) - query = query.filter(model.Visualization.published == true()) + stmt = select(model.Visualization).filter(model.Visualization.published == true()) if exclude_user: - query = query.filter(model.Visualization.user != exclude_user) - if order_by: - query = query.order_by(*order_by) - if query_only: - return query - return query.all() + stmt = stmt.filter(model.Visualization.user != exclude_user) + stmt = stmt.order_by(model.Visualization.title) + return trans.sa_session.scalars(stmt).all() # TODO: move into model (to_dict) def get_visualization_summary_dict(self, visualization): @@ -834,7 +793,7 @@ def save_visualization(self, trans, config, type, id=None, title=None, dbkey=Non vis = self._create_visualization(trans, title, type, dbkey, slug, annotation) else: decoded_id = trans.security.decode_id(id) - vis = session.query(trans.model.Visualization).get(decoded_id) + vis = session.get(model.Visualization, decoded_id) # TODO: security check? # Create new VisualizationRevision that will be attached to the viz @@ -1068,7 +1027,7 @@ def get_hda(self, trans, dataset_id, check_ownership=True, check_accessible=Fals raise HTTPBadRequest(f"Invalid dataset id: {str(dataset_id)}.") try: - data = trans.sa_session.query(trans.app.model.HistoryDatasetAssociation).get(int(dataset_id)) + data = trans.sa_session.get(HistoryDatasetAssociation, int(dataset_id)) except Exception: raise HTTPBadRequest(f"Invalid dataset id: {str(dataset_id)}.") diff --git a/lib/galaxy/webapps/base/webapp.py b/lib/galaxy/webapps/base/webapp.py index 8a4e4af3dd73..1c9774efa025 100644 --- a/lib/galaxy/webapps/base/webapp.py +++ b/lib/galaxy/webapps/base/webapp.py @@ -21,6 +21,7 @@ from paste.urlmap import URLMap from sqlalchemy import ( and_, + select, true, ) from sqlalchemy.orm.exc import NoResultFound @@ -865,13 +866,14 @@ def handle_user_logout(self, logout_all=False): self.sa_session.add_all((prev_galaxy_session, self.galaxy_session)) galaxy_user_id = prev_galaxy_session.user_id if logout_all and galaxy_user_id is not None: - for other_galaxy_session in self.sa_session.query(self.app.model.GalaxySession).filter( + stmt = select(self.app.model.GalaxySession).filter( and_( - self.app.model.GalaxySession.table.c.user_id == galaxy_user_id, - self.app.model.GalaxySession.table.c.is_valid == true(), - self.app.model.GalaxySession.table.c.id != prev_galaxy_session.id, + self.app.model.GalaxySession.user_id == galaxy_user_id, + self.app.model.GalaxySession.is_valid == true(), + self.app.model.GalaxySession.id != prev_galaxy_session.id, ) - ): + ) + for other_galaxy_session in self.sa_session.scalars(stmt): other_galaxy_session.is_valid = False self.sa_session.add(other_galaxy_session) with transaction(self.sa_session): @@ -932,9 +934,10 @@ def get_or_create_default_history(self): # Look for default history that (a) has default name + is not deleted and # (b) has no datasets. If suitable history found, use it; otherwise, create # new history. - unnamed_histories = self.sa_session.query(self.app.model.History).filter_by( + stmt = select(self.app.model.History).filter_by( user=self.galaxy_session.user, name=self.app.model.History.default_name, deleted=False ) + unnamed_histories = self.sa_session.scalars(stmt) default_history = None for history in unnamed_histories: if history.empty: @@ -961,12 +964,13 @@ def get_most_recent_history(self): if not user: return None try: - recent_history = ( - self.sa_session.query(self.app.model.History) + stmt = ( + select(self.app.model.History) .filter_by(user=user, deleted=False) .order_by(self.app.model.History.update_time.desc()) - .first() + .limit(1) ) + recent_history = self.sa_session.scalars(stmt).first() except NoResultFound: return None self.set_history(recent_history) From ca63bc89dbff4c14cdba53652de3e69b9e308d43 Mon Sep 17 00:00:00 2001 From: John Davis Date: Wed, 20 Sep 2023 15:22:42 -0400 Subject: [PATCH 22/50] Fix SA2.0 ORM usage in managers.group_roles --- lib/galaxy/managers/group_roles.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/managers/group_roles.py b/lib/galaxy/managers/group_roles.py index 3c54c98b6030..984a5c48f9ab 100644 --- a/lib/galaxy/managers/group_roles.py +++ b/lib/galaxy/managers/group_roles.py @@ -4,9 +4,13 @@ Optional, ) +from sqlalchemy import select +from sqlalchemy.orm import Session + from galaxy import model from galaxy.exceptions import ObjectNotFound from galaxy.managers.context import ProvidesAppContext +from galaxy.model import GroupRoleAssociation from galaxy.model.base import transaction from galaxy.structured_app import MinimalManagerApp @@ -61,13 +65,13 @@ def delete(self, trans: ProvidesAppContext, role_id: int, group_id: int) -> mode return role def _get_group(self, trans: ProvidesAppContext, group_id: int) -> model.Group: - group = trans.sa_session.query(model.Group).get(group_id) + group = trans.sa_session.get(model.Group, group_id) if not group: raise ObjectNotFound("Group with the id provided was not found.") return group def _get_role(self, trans: ProvidesAppContext, role_id: int) -> model.Role: - role = trans.sa_session.query(model.Role).get(role_id) + role = trans.sa_session.get(model.Role, role_id) if not role: raise ObjectNotFound("Role with the id provided was not found.") return role @@ -75,11 +79,7 @@ def _get_role(self, trans: ProvidesAppContext, role_id: int) -> model.Role: def _get_group_role( self, trans: ProvidesAppContext, group: model.Group, role: model.Role ) -> Optional[model.GroupRoleAssociation]: - return ( - trans.sa_session.query(model.GroupRoleAssociation) - .filter(model.GroupRoleAssociation.group == group, model.GroupRoleAssociation.role == role) - .one_or_none() - ) + return get_group_role(trans.sa_session, group, role) def _add_role_to_group(self, trans: ProvidesAppContext, group: model.Group, role: model.Role): gra = model.GroupRoleAssociation(group, role) @@ -91,3 +91,10 @@ def _remove_role_from_group(self, trans: ProvidesAppContext, group_role: model.G trans.sa_session.delete(group_role) with transaction(trans.sa_session): trans.sa_session.commit() + + +def get_group_role(session: Session, group, role) -> Optional[GroupRoleAssociation]: + stmt = ( + select(GroupRoleAssociation).where(GroupRoleAssociation.group == group).where(GroupRoleAssociation.role == role) + ) + return session.execute(stmt).scalar_one_or_none() From 1a665c3b762044734b0577d86f987a09373e13cf Mon Sep 17 00:00:00 2001 From: John Davis Date: Wed, 20 Sep 2023 15:23:34 -0400 Subject: [PATCH 23/50] Fix SA2.0 ORM usage in managers.group_users --- lib/galaxy/managers/group_users.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/managers/group_users.py b/lib/galaxy/managers/group_users.py index 2e382e55da37..e71eb8ecadcf 100644 --- a/lib/galaxy/managers/group_users.py +++ b/lib/galaxy/managers/group_users.py @@ -4,9 +4,16 @@ Optional, ) +from sqlalchemy import select +from sqlalchemy.orm import Session + from galaxy import model from galaxy.exceptions import ObjectNotFound from galaxy.managers.context import ProvidesAppContext +from galaxy.model import ( + User, + UserGroupAssociation, +) from galaxy.model.base import transaction from galaxy.structured_app import MinimalManagerApp @@ -61,13 +68,13 @@ def delete(self, trans: ProvidesAppContext, user_id: int, group_id: int) -> mode return user def _get_group(self, trans: ProvidesAppContext, group_id: int) -> model.Group: - group = trans.sa_session.query(model.Group).get(group_id) + group = trans.sa_session.get(model.Group, group_id) if group is None: raise ObjectNotFound("Group with the id provided was not found.") return group def _get_user(self, trans: ProvidesAppContext, user_id: int) -> model.User: - user = trans.sa_session.query(model.User).get(user_id) + user = trans.sa_session.get(User, user_id) if user is None: raise ObjectNotFound("User with the id provided was not found.") return user @@ -75,11 +82,7 @@ def _get_user(self, trans: ProvidesAppContext, user_id: int) -> model.User: def _get_group_user( self, trans: ProvidesAppContext, group: model.Group, user: model.User ) -> Optional[model.UserGroupAssociation]: - return ( - trans.sa_session.query(model.UserGroupAssociation) - .filter(model.UserGroupAssociation.user == user, model.UserGroupAssociation.group == group) - .one_or_none() - ) + return get_group_user(trans.sa_session, user, group) def _add_user_to_group(self, trans: ProvidesAppContext, group: model.Group, user: model.User): gra = model.UserGroupAssociation(user, group) @@ -91,3 +94,10 @@ def _remove_user_from_group(self, trans: ProvidesAppContext, group_user: model.U trans.sa_session.delete(group_user) with transaction(trans.sa_session): trans.sa_session.commit() + + +def get_group_user(session: Session, user, group) -> Optional[UserGroupAssociation]: + stmt = ( + select(UserGroupAssociation).where(UserGroupAssociation.user == user).where(UserGroupAssociation.group == group) + ) + return session.execute(stmt).scalar_one_or_none() From 0e4df328e21f52e61f7dc698ac408215ba97292d Mon Sep 17 00:00:00 2001 From: John Davis Date: Wed, 20 Sep 2023 15:25:28 -0400 Subject: [PATCH 24/50] Fix SA2.0 ORM usage in managers.groups Move data access method to managers.users --- lib/galaxy/managers/groups.py | 34 ++++++++++++++++++++++++++-------- lib/galaxy/managers/users.py | 11 ++++++++++- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/managers/groups.py b/lib/galaxy/managers/groups.py index 2fc03d42738a..374428c979f1 100644 --- a/lib/galaxy/managers/groups.py +++ b/lib/galaxy/managers/groups.py @@ -4,7 +4,11 @@ List, ) -from sqlalchemy import false +from sqlalchemy import ( + false, + select, +) +from sqlalchemy.orm import Session from galaxy import model from galaxy.exceptions import ( @@ -14,6 +18,11 @@ ) from galaxy.managers.base import decode_id from galaxy.managers.context import ProvidesAppContext +from galaxy.managers.users import get_users_by_ids +from galaxy.model import ( + Group, + Role, +) from galaxy.model.base import transaction from galaxy.model.scoped_session import galaxy_scoped_session from galaxy.schema.fields import ( @@ -35,7 +44,7 @@ def index(self, trans: ProvidesAppContext): Displays a collection (list) of groups. """ rval = [] - for group in trans.sa_session.query(model.Group).filter(model.Group.deleted == false()): + for group in get_not_deleted_groups(trans.sa_session): item = group.to_dict(value_mapper={"id": DecodedDatabaseIdField.encode}) encoded_id = DecodedDatabaseIdField.encode(group.id) item["url"] = url_for("group", id=encoded_id) @@ -101,11 +110,11 @@ def update(self, trans: ProvidesAppContext, group_id: int, payload: Dict[str, An sa_session.commit() def _check_duplicated_group_name(self, sa_session: galaxy_scoped_session, group_name: str) -> None: - if sa_session.query(model.Group).filter(model.Group.name == group_name).first(): + if get_group_by_name(sa_session, group_name): raise Conflict(f"A group with name '{group_name}' already exists") def _get_group(self, sa_session: galaxy_scoped_session, group_id: int) -> model.Group: - group = sa_session.query(model.Group).get(group_id) + group = sa_session.get(model.Group, group_id) if group is None: raise ObjectNotFound("Group with the provided id was not found.") return group @@ -114,18 +123,27 @@ def _get_users_by_encoded_ids( self, sa_session: galaxy_scoped_session, encoded_user_ids: List[EncodedDatabaseIdField] ) -> List[model.User]: user_ids = self._decode_ids(encoded_user_ids) - users = sa_session.query(model.User).filter(model.User.table.c.id.in_(user_ids)).all() - return users + return get_users_by_ids(sa_session, user_ids) def _get_roles_by_encoded_ids( self, sa_session: galaxy_scoped_session, encoded_role_ids: List[EncodedDatabaseIdField] ) -> List[model.Role]: role_ids = self._decode_ids(encoded_role_ids) - roles = sa_session.query(model.Role).filter(model.Role.id.in_(role_ids)).all() - return roles + stmt = select(Role).where(Role.id.in_(role_ids)) + return sa_session.scalars(stmt).all() def _decode_id(self, encoded_id: EncodedDatabaseIdField) -> int: return decode_id(self._app, encoded_id) def _decode_ids(self, encoded_ids: List[EncodedDatabaseIdField]) -> List[int]: return [self._decode_id(encoded_id) for encoded_id in encoded_ids] + + +def get_group_by_name(session: Session, name: str): + stmt = select(Group).filter(Group.name == name).limit(1) + return session.scalars(stmt).first() + + +def get_not_deleted_groups(session: Session): + stmt = select(Group).where(Group.deleted == false()) + return session.scalars(stmt) diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index 4489a00689e6..dedc1d5ef90d 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -23,6 +23,7 @@ select, true, ) +from sqlalchemy.orm import Session from sqlalchemy.orm.exc import NoResultFound from galaxy import ( @@ -36,7 +37,10 @@ base, deletable, ) -from galaxy.model import UserQuotaUsage +from galaxy.model import ( + User, + UserQuotaUsage, +) from galaxy.model.base import transaction from galaxy.security.validate_user_input import ( VALID_EMAIL_RE, @@ -850,3 +854,8 @@ def get_user_by_username(session, user_class, username): return session.execute(stmt).scalar_one() except Exception: return None + + +def get_users_by_ids(session: Session, user_ids): + stmt = select(User).where(User.id.in_(user_ids)) + return session.scalars(stmt).all() From 2084da724335c3f02a627d201082a5c675760d16 Mon Sep 17 00:00:00 2001 From: John Davis Date: Thu, 10 Aug 2023 18:47:43 -0400 Subject: [PATCH 25/50] Fix SA2.0 ORM usage in galaxy.webapps/reports [partially] --- .../webapps/reports/controllers/jobs.py | 12 ++-- .../webapps/reports/controllers/system.py | 69 ++++++++++--------- .../webapps/reports/controllers/users.py | 19 ++--- .../webapps/reports/controllers/workflows.py | 2 +- 4 files changed, 54 insertions(+), 48 deletions(-) diff --git a/lib/galaxy/webapps/reports/controllers/jobs.py b/lib/galaxy/webapps/reports/controllers/jobs.py index ede8c033c843..1c6e9a9c2ec6 100644 --- a/lib/galaxy/webapps/reports/controllers/jobs.py +++ b/lib/galaxy/webapps/reports/controllers/jobs.py @@ -18,12 +18,15 @@ and_, not_, or_, + select, ) from galaxy import ( model, util, ) +from galaxy.model import Job +from galaxy.model.repositories import get_user_by_email from galaxy.web.legacy_framework import grids from galaxy.webapps.base.controller import ( BaseUIController, @@ -1288,7 +1291,7 @@ def tool_per_month(self, trans, **kwd): @web.expose def job_info(self, trans, **kwd): message = "" - job = trans.sa_session.query(model.Job).get(trans.security.decode_id(kwd.get("id", ""))) + job = trans.sa_session.get(Job, trans.security.decode_id(kwd.get("id", ""))) return trans.fill_template("/webapps/reports/job_info.mako", job=job, message=message) @@ -1296,7 +1299,7 @@ def job_info(self, trans, **kwd): def get_job(trans, id): - return trans.sa_session.query(trans.model.Job).get(trans.security.decode_id(id)) + return trans.sa_session.get(Job, trans.security.decode_id(id)) def get_monitor_id(trans, monitor_email): @@ -1304,9 +1307,8 @@ def get_monitor_id(trans, monitor_email): A convenience method to obtain the monitor job id. """ monitor_user_id = None - monitor_row = ( - trans.sa_session.query(trans.model.User.id).filter(trans.model.User.table.c.email == monitor_email).first() - ) + stmt = select(trans.model.User.id).filter(trans.model.User.email == monitor_email).limit(1) + monitor_row = trans.sa_session.scalars(stmt).first() if monitor_row is not None: monitor_user_id = monitor_row[0] return monitor_user_id diff --git a/lib/galaxy/webapps/reports/controllers/system.py b/lib/galaxy/webapps/reports/controllers/system.py index e26446c75d2d..0b18f18f2d5d 100644 --- a/lib/galaxy/webapps/reports/controllers/system.py +++ b/lib/galaxy/webapps/reports/controllers/system.py @@ -11,6 +11,7 @@ desc, false, null, + select, true, ) from sqlalchemy.orm import joinedload @@ -74,13 +75,14 @@ def userless_histories(self, trans, **kwd): cutoff_time = datetime.utcnow() - timedelta(days=userless_histories_days) history_count = 0 dataset_count = 0 - for history in trans.sa_session.query(model.History).filter( + stmt = select(model.History).filter( and_( - model.History.table.c.user_id == null(), - model.History.table.c.deleted == true(), + model.History.user_id == null(), + model.History.deleted == true(), model.History.update_time < cutoff_time, ) - ): + ) + for history in trans.sa_session.scalars(stmt).all(): for dataset in history.datasets: if not dataset.deleted: dataset_count += 1 @@ -106,18 +108,21 @@ def deleted_histories(self, trans, **kwd): history_count = 0 dataset_count = 0 disk_space = 0 - histories = ( - trans.sa_session.query(model.History) + + stmt = ( + select(model.History) .filter( and_( - model.History.table.c.deleted == true(), - model.History.table.c.purged == false(), + model.History.deleted == true(), + model.History.purged == false(), model.History.update_time < cutoff_time, ) ) .options(joinedload(model.History.datasets)) + .unique() ) + histories = trans.sa_session.scalars(stmt).all() for history in histories: for hda in history.datasets: if not hda.dataset.purged: @@ -144,13 +149,14 @@ def deleted_datasets(self, trans, **kwd): cutoff_time = datetime.utcnow() - timedelta(days=deleted_datasets_days) dataset_count = 0 disk_space = 0 - for dataset in trans.sa_session.query(model.Dataset).filter( + stmt = select(model.Dataset).filter( and_( - model.Dataset.table.c.deleted == true(), - model.Dataset.table.c.purged == false(), - model.Dataset.table.c.update_time < cutoff_time, + model.Dataset.deleted == true(), + model.Dataset.purged == false(), + model.Dataset.update_time < cutoff_time, ) - ): + ) + for dataset in trans.sa_session.scalars(stmt).all(): dataset_count += 1 try: disk_space += dataset.file_size @@ -167,28 +173,22 @@ def deleted_datasets(self, trans, **kwd): @web.expose def dataset_info(self, trans, **kwd): message = "" - dataset = trans.sa_session.query(model.Dataset).get(trans.security.decode_id(kwd.get("id", ""))) + dataset = trans.sa_session.get(model.Dataset, trans.security.decode_id(kwd.get("id", ""))) # Get all associated hdas and lddas that use the same disk file. - associated_hdas = ( - trans.sa_session.query(trans.model.HistoryDatasetAssociation) - .filter( - and_( - trans.model.HistoryDatasetAssociation.deleted == false(), - trans.model.HistoryDatasetAssociation.dataset_id == dataset.id, - ) + stmt = select(trans.model.HistoryDatasetAssociation).filter( + and_( + trans.model.HistoryDatasetAssociation.deleted == false(), + trans.model.HistoryDatasetAssociation.dataset_id == dataset.id, ) - .all() ) - associated_lddas = ( - trans.sa_session.query(trans.model.LibraryDatasetDatasetAssociation) - .filter( - and_( - trans.model.LibraryDatasetDatasetAssociation.deleted == false(), - trans.model.LibraryDatasetDatasetAssociation.dataset_id == dataset.id, - ) + associated_hdas = trans.sa_session.scalars(stmt).all() + stmt = select(trans.model.LibraryDatasetDatasetAssociation).filter( + and_( + trans.model.LibraryDatasetDatasetAssociation.deleted == false(), + trans.model.LibraryDatasetDatasetAssociation.dataset_id == dataset.id, ) - .all() ) + associated_lddas = trans.sa_session.scalars(stmt).all() return trans.fill_template( "/webapps/reports/dataset_info.mako", dataset=dataset, @@ -208,11 +208,12 @@ def disk_usage(self, trans, **kwd): disk_usage = self.get_disk_usage(file_path) min_file_size = 2**32 # 4 Gb file_size_str = nice_size(min_file_size) - datasets = ( - trans.sa_session.query(model.Dataset) - .filter(and_(model.Dataset.table.c.purged == false(), model.Dataset.table.c.file_size > min_file_size)) - .order_by(desc(model.Dataset.table.c.file_size)) + stmt = ( + select(model.Dataset) + .filter(and_(model.Dataset.purged == false(), model.Dataset.file_size > min_file_size)) + .order_by(desc(model.Dataset.file_size)) ) + datasets = trans.sa_session.scalars(stmt).all() return file_path, disk_usage, datasets, file_size_str diff --git a/lib/galaxy/webapps/reports/controllers/users.py b/lib/galaxy/webapps/reports/controllers/users.py index bfc9cb233f83..8798ea32c6a0 100644 --- a/lib/galaxy/webapps/reports/controllers/users.py +++ b/lib/galaxy/webapps/reports/controllers/users.py @@ -9,7 +9,11 @@ import sqlalchemy as sa from markupsafe import escape -from sqlalchemy import false +from sqlalchemy import ( + false, + func, + select, +) import galaxy.model from galaxy import util @@ -27,7 +31,8 @@ class Users(BaseUIController, ReportQueryBuilder): @web.expose def registered_users(self, trans, **kwd): message = escape(util.restore_text(kwd.get("message", ""))) - num_users = trans.sa_session.query(galaxy.model.User).count() + stmt = select(func.count(galaxy.model.User.id)) + num_users = trans.sa_session.scalar(stmt) return trans.fill_template("/webapps/reports/registered_users.mako", num_users=num_users, message=message) @web.expose @@ -165,11 +170,9 @@ def name_to_num(name): days_not_logged_in = 0 cutoff_time = datetime.utcnow() - timedelta(days=int(days_not_logged_in)) users = [] - for user in ( - trans.sa_session.query(galaxy.model.User) - .filter(galaxy.model.User.table.c.deleted == false()) - .order_by(galaxy.model.User.table.c.email) - ): + + stmt = select(galaxy.model.User).filter(galaxy.model.User.deleted == false()).order_by(galaxy.model.User.email) + for user in trans.sa_session.scalars(stmt).all(): current_galaxy_session = user.current_galaxy_session if current_galaxy_session: last_galaxy_session = current_galaxy_session @@ -204,7 +207,7 @@ def user_disk_usage(self, trans, **kwd): user_cutoff = int(kwd.get("user_cutoff", 60)) # disk_usage isn't indexed - all_users = trans.sa_session.query(galaxy.model.User).all() + all_users = trans.sa_session.scalars(select(galaxy.model.User)).all() sort_attrgetter = operator.attrgetter(str(sort_id)) users = sorted(all_users, key=lambda x: sort_attrgetter(x) or 0, reverse=_order) if user_cutoff: diff --git a/lib/galaxy/webapps/reports/controllers/workflows.py b/lib/galaxy/webapps/reports/controllers/workflows.py index 999a25fe9897..2291b7a499f2 100644 --- a/lib/galaxy/webapps/reports/controllers/workflows.py +++ b/lib/galaxy/webapps/reports/controllers/workflows.py @@ -491,4 +491,4 @@ def per_workflow(self, trans, **kwd): def get_workflow(trans, id): - return trans.sa_session.query(trans.model.Workflow).get(trans.security.decode_id(id)) + return trans.sa_session.get(model.Workflow, trans.security.decode_id(id)) From 09c2efc8c6f302cb3a38da6a9da09e5ca33779aa Mon Sep 17 00:00:00 2001 From: John Davis Date: Wed, 20 Sep 2023 15:16:47 -0400 Subject: [PATCH 26/50] Fix SA2.0 ORM usage in managers.api_keys; refactor Replace python iteration with SA batch update --- lib/galaxy/managers/api_keys.py | 55 +++++++++++++++++---------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/lib/galaxy/managers/api_keys.py b/lib/galaxy/managers/api_keys.py index 8e9b41c32380..cf36c58fa1c1 100644 --- a/lib/galaxy/managers/api_keys.py +++ b/lib/galaxy/managers/api_keys.py @@ -1,39 +1,36 @@ -from typing import ( - Optional, - TYPE_CHECKING, +from typing import Optional + +from sqlalchemy import ( + false, + select, + update, ) -from galaxy.model import User +from galaxy.model import ( + APIKeys, + User, +) from galaxy.model.base import transaction from galaxy.structured_app import BasicSharedApp -if TYPE_CHECKING: - from galaxy.model import APIKeys - class ApiKeyManager: def __init__(self, app: BasicSharedApp): self.app = app + self.session = self.app.model.context def get_api_key(self, user: User) -> Optional["APIKeys"]: - sa_session = self.app.model.context - api_key = ( - sa_session.query(self.app.model.APIKeys) - .filter_by(user_id=user.id, deleted=False) - .order_by(self.app.model.APIKeys.create_time.desc()) - .first() - ) - return api_key + stmt = select(APIKeys).filter_by(user_id=user.id, deleted=False).order_by(APIKeys.create_time.desc()).limit(1) + return self.session.scalars(stmt).first() def create_api_key(self, user: User) -> "APIKeys": guid = self.app.security.get_new_guid() new_key = self.app.model.APIKeys() new_key.user_id = user.id new_key.key = guid - sa_session = self.app.model.context - sa_session.add(new_key) - with transaction(sa_session): - sa_session.commit() + self.session.add(new_key) + with transaction(self.session): + self.session.commit() return new_key def get_or_create_api_key(self, user: User) -> str: @@ -46,12 +43,18 @@ def get_or_create_api_key(self, user: User) -> str: def delete_api_key(self, user: User) -> None: """Marks the current user API key as deleted.""" - sa_session = self.app.model.context # Before it was possible to create multiple API keys for the same user although they were not considered valid # So all non-deleted keys are marked as deleted for backward compatibility - api_keys = sa_session.query(self.app.model.APIKeys).filter_by(user_id=user.id, deleted=False) - for api_key in api_keys: - api_key.deleted = True - sa_session.add(api_key) - with transaction(sa_session): - sa_session.commit() + self._mark_all_api_keys_as_deleted(user.id) + with transaction(self.session): + self.session.commit() + + def _mark_all_api_keys_as_deleted(self, user_id: int): + stmt = ( + update(APIKeys) + .where(APIKeys.user_id == user_id) + .where(APIKeys.deleted == false()) + .values(deleted=True) + .execution_options(synchronize_session="evaluate") + ) + return self.session.execute(stmt) From f355a199b9fba30630e7809897709e802170c76b Mon Sep 17 00:00:00 2001 From: John Davis Date: Thu, 21 Sep 2023 09:13:19 -0400 Subject: [PATCH 27/50] Fix SA2.0 ORM usage in galaxy/tool_shed/util --- lib/galaxy/tool_shed/util/repository_util.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/lib/galaxy/tool_shed/util/repository_util.py b/lib/galaxy/tool_shed/util/repository_util.py index 75112bb99598..674048937359 100644 --- a/lib/galaxy/tool_shed/util/repository_util.py +++ b/lib/galaxy/tool_shed/util/repository_util.py @@ -69,7 +69,7 @@ def check_for_updates( message += "Unable to retrieve status from the tool shed for the following repositories:\n" message += ", ".join(repository_names_not_updated) else: - repository = get_tool_shed_repository_by_decoded_id(install_model_context, repository_id) + repository = install_model_context.get(ToolShedRepository, repository_id) ok, updated = _check_or_update_tool_shed_status_for_installed_repository( tool_shed_registry, install_model_context, repository ) @@ -632,15 +632,7 @@ def get_tool_shed_from_clone_url(repository_clone_url): def get_tool_shed_repository_by_id(app, repository_id) -> ToolShedRepository: """Return a tool shed repository database record defined by the id.""" # This method is used only in Galaxy, not the tool shed. - return get_tool_shed_repository_by_decoded_id(app.install_model.context, app.security.decode_id(repository_id)) - - -def get_tool_shed_repository_by_decoded_id( - install_model_context: install_model_scoped_session, repository_id: int -) -> ToolShedRepository: - return ( - install_model_context.query(ToolShedRepository).filter(ToolShedRepository.table.c.id == repository_id).first() - ) + return app.install_model.context.get(ToolShedRepository, app.security.decode_id(repository_id)) def get_tool_shed_status_for(tool_shed_registry: Registry, repository: ToolShedRepository): From 9e92c703b3b751553e5c347481e1c148c51d3ee7 Mon Sep 17 00:00:00 2001 From: John Davis Date: Thu, 10 Aug 2023 22:04:33 -0400 Subject: [PATCH 28/50] Fix SA2.0 ORM usage in in galaxy.jobs [partially] Move data access method into manager (get_jobs_to_check_at_startup) --- lib/galaxy/jobs/__init__.py | 26 +++++++++++++++----------- lib/galaxy/jobs/handler.py | 20 ++------------------ lib/galaxy/jobs/runners/__init__.py | 8 +++++--- lib/galaxy/jobs/runners/godocker.py | 3 +-- lib/galaxy/jobs/runners/pulsar.py | 7 +++---- lib/galaxy/managers/jobs.py | 27 ++++++++++++++++++++++++++- 6 files changed, 52 insertions(+), 39 deletions(-) diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index 2c8f615fec11..1e4284b4d1ca 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -25,6 +25,7 @@ import yaml from packaging.version import Version from pulsar.client.staging import COMMAND_VERSION_FILENAME +from sqlalchemy import select from galaxy import ( model, @@ -59,7 +60,11 @@ JobState, ) from galaxy.metadata import get_metadata_compute_strategy -from galaxy.model import store +from galaxy.model import ( + Job, + store, + Task, +) from galaxy.model.base import transaction from galaxy.model.store.discover import MaxDiscoveredFilesExceededError from galaxy.objectstore import ObjectStorePopulator @@ -1044,11 +1049,8 @@ def metadata_strategy(self): def remote_command_line(self): use_remote = self.get_destination_configuration("tool_evaluation_strategy") == "remote" # It wouldn't be hard to support history export, but we want to do this in task queue workers anyway ... - return ( - use_remote - and self.external_output_metadata.extended - and not self.sa_session.query(model.JobExportHistoryArchive).filter_by(job=self.get_job()).first() - ) + stmt = select(model.JobExportHistoryArchive).filter_by(job=self.get_job()).limit(1) + return use_remote and self.external_output_metadata.extended and not self.sa_session.scalars(stmt).first() def tool_directory(self): tool_dir = self.tool and self.tool.tool_dir @@ -1183,7 +1185,7 @@ def galaxy_url(self): return self.get_destination_configuration("galaxy_infrastructure_url") def get_job(self) -> model.Job: - return self.sa_session.query(model.Job).get(self.job_id) + return self.sa_session.get(Job, self.job_id) def get_id_tag(self): # For compatibility with drmaa, which uses job_id right now, and TaskWrapper @@ -1234,10 +1236,12 @@ def prepare(self, compute_environment=None): job = self._load_job() def get_special(): - jeha = self.sa_session.query(model.JobExportHistoryArchive).filter_by(job=job).first() + stmt = select(model.JobExportHistoryArchive).filter_by(job=job).limit(1) + jeha = self.sa_session.scalars(stmt).first() if jeha: return jeha.fda - return self.sa_session.query(model.GenomeIndexToolData).filter_by(job=job).first() + stmt = select(model.GenomeIndexToolData).filter_by(job=job).limit(1) + return self.sa_session.scalars(stmt).first() # TODO: The upload tool actions that create the paramfile can probably be turned in to a configfile to remove this special casing if job.tool_id == "upload1": @@ -2584,12 +2588,12 @@ def can_split(self): def get_job(self): if self.job_id: - return self.sa_session.query(model.Job).get(self.job_id) + return self.sa_session.get(Job, self.job_id) else: return None def get_task(self): - return self.sa_session.query(model.Task).get(self.task_id) + return self.sa_session.get(Task, self.task_id) def get_id_tag(self): # For compatibility with drmaa job runner and TaskWrapper, instead of using job_id directly diff --git a/lib/galaxy/jobs/handler.py b/lib/galaxy/jobs/handler.py index 7148a58884b9..028023784dad 100644 --- a/lib/galaxy/jobs/handler.py +++ b/lib/galaxy/jobs/handler.py @@ -34,6 +34,7 @@ TaskWrapper, ) from galaxy.jobs.mapper import JobNotReadyException +from galaxy.managers.jobs import get_jobs_to_check_at_startup from galaxy.model.base import transaction from galaxy.structured_app import MinimalManagerApp from galaxy.util import unicodify @@ -272,10 +273,9 @@ def __check_jobs_at_startup(self): the database and requeues or cleans up as necessary. Only run as the job handler starts. In case the activation is enforced it will filter out the jobs of inactive users. """ - stmt = self._build_check_jobs_at_startup_statement() with self.sa_session() as session, session.begin(): try: - for job in session.scalars(stmt): + for job in get_jobs_to_check_at_startup(session, self.track_jobs_in_database, self.app.config): with session.begin_nested(): self._check_job_at_startup(job) finally: @@ -319,22 +319,6 @@ def _check_job_at_startup(self, job): self.dispatcher.recover(job, job_wrapper) pass - def _build_check_jobs_at_startup_statement(self): - if self.track_jobs_in_database: - in_list = (model.Job.states.QUEUED, model.Job.states.RUNNING, model.Job.states.STOPPED) - else: - in_list = (model.Job.states.NEW, model.Job.states.QUEUED, model.Job.states.RUNNING) - - stmt = ( - select(model.Job) - .execution_options(yield_per=model.YIELD_PER_ROWS) - .filter(model.Job.state.in_(in_list) & (model.Job.handler == self.app.config.server_name)) - ) - if self.app.config.user_activation_on: - # Filter out the jobs of inactive users. - stmt = stmt.outerjoin(model.User).filter(or_((model.Job.user_id == null()), (model.User.active == true()))) - return stmt - def __recover_job_wrapper(self, job): # Already dispatched and running job_wrapper = self.job_wrapper(job) diff --git a/lib/galaxy/jobs/runners/__init__.py b/lib/galaxy/jobs/runners/__init__.py index 6c95de6312b3..c14aea726f03 100644 --- a/lib/galaxy/jobs/runners/__init__.py +++ b/lib/galaxy/jobs/runners/__init__.py @@ -15,6 +15,7 @@ Queue, ) +from sqlalchemy import select from sqlalchemy.orm import object_session import galaxy.jobs @@ -397,11 +398,12 @@ def _walk_dataset_outputs(self, job: model.Job): dataset_assoc.dataset.dataset.history_associations + dataset_assoc.dataset.dataset.library_associations ): if isinstance(dataset, self.app.model.HistoryDatasetAssociation): - joda = ( - self.sa_session.query(self.app.model.JobToOutputDatasetAssociation) + stmt = ( + select(self.app.model.JobToOutputDatasetAssociation) .filter_by(job=job, dataset=dataset) - .first() + .limit(1) ) + joda = self.sa_session.scalars(stmt).first() yield (joda, dataset) # TODO: why is this not just something easy like: # for dataset_assoc in job.output_datasets + job.output_library_datasets: diff --git a/lib/galaxy/jobs/runners/godocker.py b/lib/galaxy/jobs/runners/godocker.py index 869994a8838b..bcacb84fb700 100644 --- a/lib/galaxy/jobs/runners/godocker.py +++ b/lib/galaxy/jobs/runners/godocker.py @@ -235,8 +235,7 @@ def check_watched_item(self, job_state): def stop_job(self, job_wrapper): """Attempts to delete a dispatched executing Job in GoDocker""" - # This function is called by fail_job() where - # param job = self.sa_session.query(self.app.model.Job).get(job_state.job_wrapper.job_id) + # This function is called by fail_job() # No Return data expected job_id = job_wrapper.job_id log.debug(f"STOP JOB EXECUTION OF JOB ID: {str(job_id)}") diff --git a/lib/galaxy/jobs/runners/pulsar.py b/lib/galaxy/jobs/runners/pulsar.py index aa3ee8cb6f17..2bba3504024a 100644 --- a/lib/galaxy/jobs/runners/pulsar.py +++ b/lib/galaxy/jobs/runners/pulsar.py @@ -36,6 +36,7 @@ # TODO: Perform pulsar release with this included in the client package from pulsar.client.staging import DEFAULT_DYNAMIC_COLLECTION_PATTERN +from sqlalchemy import select from galaxy import model from galaxy.job_execution.compute_environment import ( @@ -974,10 +975,8 @@ def __async_update(self, full_status): remote_job_id = full_status["job_id"] if len(remote_job_id) == 32: # It is a UUID - assign_ids = uuid in destination params... - sa_session = self.app.model.session - galaxy_job_id = ( - sa_session.query(model.Job).filter(model.Job.job_runner_external_id == remote_job_id).one().id - ) + stmt = select(model.Job).filter(model.Job.job_runner_external_id == remote_job_id) + galaxy_job_id = self.app.model.session.execute(stmt).scalar_one().id else: galaxy_job_id = remote_job_id job, job_wrapper = self.app.job_manager.job_handler.job_queue.job_pair_for_id(galaxy_job_id) diff --git a/lib/galaxy/managers/jobs.py b/lib/galaxy/managers/jobs.py index 681dc87a9070..cb792bd27d0a 100644 --- a/lib/galaxy/managers/jobs.py +++ b/lib/galaxy/managers/jobs.py @@ -15,9 +15,14 @@ and_, false, func, + null, or_, + true, +) +from sqlalchemy.orm import ( + aliased, + Session, ) -from sqlalchemy.orm import aliased from sqlalchemy.sql import select from galaxy import model @@ -38,6 +43,8 @@ from galaxy.model import ( Job, JobParameter, + User, + YIELD_PER_ROWS, ) from galaxy.model.base import transaction from galaxy.model.index_filter_util import ( @@ -1040,3 +1047,21 @@ def summarize_job_outputs(job: model.Job, tool, params, security): } ) return outputs + + +def get_jobs_to_check_at_startup(session: Session, track_jobs_in_database: bool, config): + if track_jobs_in_database: + in_list = (Job.states.QUEUED, Job.states.RUNNING, Job.states.STOPPED) + else: + in_list = (Job.states.NEW, Job.states.QUEUED, Job.states.RUNNING) + + stmt = ( + select(Job) + .execution_options(yield_per=YIELD_PER_ROWS) + .filter(Job.state.in_(in_list) & (Job.handler == config.server_name)) + ) + if config.user_activation_on: + # Filter out the jobs of inactive users. + stmt = stmt.outerjoin(User).filter(or_((Job.user_id == null()), (User.active == true()))) + + return session.scalars(stmt) From 8581d05825ace8c32918c0b68aa2585c05e2b4f7 Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 21 Jul 2023 15:08:20 -0400 Subject: [PATCH 29/50] Fix SA2.0 ORM usage in webapps.galaxy.controllers.history --- .../webapps/galaxy/controllers/history.py | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/controllers/history.py b/lib/galaxy/webapps/galaxy/controllers/history.py index 9ab25e6b8fb9..2dd00d2d487d 100644 --- a/lib/galaxy/webapps/galaxy/controllers/history.py +++ b/lib/galaxy/webapps/galaxy/controllers/history.py @@ -4,6 +4,7 @@ from markupsafe import escape from sqlalchemy import ( false, + select, true, ) from sqlalchemy.orm import undefer @@ -15,6 +16,7 @@ ) from galaxy.managers import histories from galaxy.managers.sharable import SlugBuilder +from galaxy.model import Role from galaxy.model.base import transaction from galaxy.model.item_attrs import ( UsesAnnotations, @@ -402,11 +404,12 @@ def _list_switch(self, trans, histories): new_history = histories[0] galaxy_session = trans.get_galaxy_session() try: - association = ( - trans.sa_session.query(trans.app.model.GalaxySessionToHistoryAssociation) + stmt = ( + select(trans.app.model.GalaxySessionToHistoryAssociation) .filter_by(session_id=galaxy_session.id, history_id=new_history.id) - .first() + .limit(1) ) + association = trans.sa_session.scalars(stmt).first() except Exception: association = None new_history.add_galaxy_session(galaxy_session, association=association) @@ -435,11 +438,10 @@ def list_shared(self, trans, **kwargs): # hit if this user isn't having the history shared with her. history = self.history_manager.by_id(self.decode_id(id)) # Current user is the user with which the histories were shared - association = ( - trans.sa_session.query(trans.app.model.HistoryUserShareAssociation) - .filter_by(user=trans.user, history=history) - .one() + stmt = select(trans.app.model.HistoryUserShareAssociation).filter_by( + user=trans.user, history=history ) + association = trans.sa_session.execute(select(stmt)).scalar_one() trans.sa_session.delete(association) with transaction(trans.sa_session): trans.sa_session.commit() @@ -513,8 +515,13 @@ def display_by_username_and_slug(self, trans, username, slug, **kwargs): """ # Get history. session = trans.sa_session - user = session.query(model.User).filter_by(username=username).first() - history = trans.sa_session.query(model.History).filter_by(user=user, slug=slug, deleted=False).first() + + stmt = select(model.User).filter_by(username=username).limit(1) + user = session.scalars(stmt).first() + + stmt = select(model.History).filter_by(user=user, slug=slug, deleted=False).limit(1) + history = session.scalars(stmt).first() + if history is None: raise web.httpexceptions.HTTPNotFound() @@ -571,9 +578,7 @@ def permissions(self, trans, payload=None, **kwd): permissions = {} for action_key, action in trans.app.model.Dataset.permitted_actions.items(): in_roles = payload.get(action_key) or [] - in_roles = [ - trans.sa_session.query(trans.app.model.Role).get(trans.security.decode_id(x)) for x in in_roles - ] + in_roles = [trans.sa_session.get(Role, trans.security.decode_id(x)) for x in in_roles] permissions[trans.app.security_agent.get_action(action.action)] = in_roles trans.app.security_agent.history_set_default_permissions(history, permissions) return {"message": "Default history '%s' dataset permissions have been changed." % history.name} From de1321b2ddc54ff44b11a208aaf8f6fcf271aca0 Mon Sep 17 00:00:00 2001 From: John Davis Date: Wed, 20 Sep 2023 15:06:06 -0400 Subject: [PATCH 30/50] Fix SA2.0 ORM usage in webapps.galaxy.services Move data access method to managers.quotas (get_quotas) Move data access method to managers.histories (get_len_files_by_history) Move data access method to managers.groups (get_group_by_name) --- lib/galaxy/managers/histories.py | 11 +++++ lib/galaxy/managers/quotas.py | 16 +++++++ .../galaxy/services/dataset_collections.py | 2 +- .../webapps/galaxy/services/histories.py | 7 +-- .../webapps/galaxy/services/libraries.py | 3 +- lib/galaxy/webapps/galaxy/services/quotas.py | 40 ++++++++--------- .../galaxy/services/tool_shed_repositories.py | 44 +++++++++---------- lib/galaxy/webapps/galaxy/services/tools.py | 7 ++- lib/galaxy/webapps/galaxy/services/users.py | 21 ++++----- 9 files changed, 88 insertions(+), 63 deletions(-) diff --git a/lib/galaxy/managers/histories.py b/lib/galaxy/managers/histories.py index 2f89fb1bed86..672428ef1edd 100644 --- a/lib/galaxy/managers/histories.py +++ b/lib/galaxy/managers/histories.py @@ -24,6 +24,7 @@ select, true, ) +from sqlalchemy.orm import Session from typing_extensions import Literal from galaxy import ( @@ -43,6 +44,7 @@ StorageCleanerManager, ) from galaxy.managers.export_tracker import StoreExportTracker +from galaxy.model import HistoryDatasetAssociation from galaxy.model.base import transaction from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.schema import ( @@ -900,3 +902,12 @@ def username_eq(self, item, val: str) -> bool: def username_contains(self, item, val: str) -> bool: return val.lower() in str(item.user.username).lower() + + +def get_fasta_hdas_by_history(session: Session, history_id: int): + stmt = ( + select(HistoryDatasetAssociation) + .filter_by(history_id=history_id, extension="fasta", deleted=False) + .order_by(HistoryDatasetAssociation.hid.desc()) + ) + return session.scalars(stmt).all() diff --git a/lib/galaxy/managers/quotas.py b/lib/galaxy/managers/quotas.py index 0340698aa28f..e0601d73081a 100644 --- a/lib/galaxy/managers/quotas.py +++ b/lib/galaxy/managers/quotas.py @@ -11,12 +11,20 @@ Union, ) +from sqlalchemy import ( + false, + select, + true, +) +from sqlalchemy.orm import Session + from galaxy import ( model, util, ) from galaxy.exceptions import ActionInputError from galaxy.managers import base +from galaxy.model import Quota from galaxy.model.base import transaction from galaxy.quota import DatabaseQuotaAgent from galaxy.quota._schema import ( @@ -273,3 +281,11 @@ def purge_quota(self, quota, params=None): def get_quota(self, trans, id: int, deleted: Optional[bool] = None) -> model.Quota: return base.get_object(trans, id, "Quota", check_ownership=False, check_accessible=False, deleted=deleted) + + +def get_quotas(session: Session, deleted: bool = False): + is_deleted = true() + if not deleted: + is_deleted = false() + stmt = select(Quota).where(Quota.deleted == is_deleted) + return session.scalars(stmt) diff --git a/lib/galaxy/webapps/galaxy/services/dataset_collections.py b/lib/galaxy/webapps/galaxy/services/dataset_collections.py index 5c59d260b092..7c40ee1b9db1 100644 --- a/lib/galaxy/webapps/galaxy/services/dataset_collections.py +++ b/lib/galaxy/webapps/galaxy/services/dataset_collections.py @@ -210,7 +210,7 @@ def show( return rval def dce_content(self, trans: ProvidesHistoryContext, dce_id: DecodedDatabaseIdField) -> DCESummary: - dce: Optional[DatasetCollectionElement] = trans.model.session.query(DatasetCollectionElement).get(dce_id) + dce: Optional[DatasetCollectionElement] = trans.model.session.get(DatasetCollectionElement, dce_id) if not dce: raise exceptions.ObjectNotFound("No DatasetCollectionElement found") if not trans.user_is_admin: diff --git a/lib/galaxy/webapps/galaxy/services/histories.py b/lib/galaxy/webapps/galaxy/services/histories.py index 8c7f69d6be0c..b330c77ab68e 100644 --- a/lib/galaxy/webapps/galaxy/services/histories.py +++ b/lib/galaxy/webapps/galaxy/services/histories.py @@ -32,6 +32,7 @@ from galaxy.managers.citations import CitationsManager from galaxy.managers.context import ProvidesHistoryContext from galaxy.managers.histories import ( + get_fasta_hdas_by_history, HistoryDeserializer, HistoryExportManager, HistoryFilters, @@ -640,11 +641,7 @@ def get_custom_builds_metadata( installed_builds = [] for build in glob.glob(os.path.join(trans.app.config.len_file_path, "*.len")): installed_builds.append(os.path.basename(build).split(".len")[0]) - fasta_hdas = ( - trans.sa_session.query(model.HistoryDatasetAssociation) - .filter_by(history=history, extension="fasta", deleted=False) - .order_by(model.HistoryDatasetAssociation.hid.desc()) - ) + fasta_hdas = get_fasta_hdas_by_history(trans.sa_session, history.id) return CustomBuildsMetadataResponse( installed_builds=[LabelValuePair(label=ins, value=ins) for ins in installed_builds], fasta_hdas=[ diff --git a/lib/galaxy/webapps/galaxy/services/libraries.py b/lib/galaxy/webapps/galaxy/services/libraries.py index 938898f85086..6ecfe6252149 100644 --- a/lib/galaxy/webapps/galaxy/services/libraries.py +++ b/lib/galaxy/webapps/galaxy/services/libraries.py @@ -15,6 +15,7 @@ from galaxy.managers.folders import FolderManager from galaxy.managers.libraries import LibraryManager from galaxy.managers.roles import RoleManager +from galaxy.model import Role from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.schema import ( BasicRoleModel, @@ -321,7 +322,7 @@ def set_permissions_old(self, trans, library, payload: Dict[str, Any]) -> Librar permissions = {} for k, v in trans.app.model.Library.permitted_actions.items(): role_params = payload.get(f"{k}_in", []) - in_roles = [trans.sa_session.query(trans.app.model.Role).get(x) for x in util.listify(role_params)] + in_roles = [trans.sa_session.get(Role, x) for x in util.listify(role_params)] permissions[trans.app.security_agent.get_action(v.action)] = in_roles trans.app.security_agent.set_all_library_permissions(trans, library, permissions) trans.sa_session.refresh(library) diff --git a/lib/galaxy/webapps/galaxy/services/quotas.py b/lib/galaxy/webapps/galaxy/services/quotas.py index ca6ef0545924..b2527d130416 100644 --- a/lib/galaxy/webapps/galaxy/services/quotas.py +++ b/lib/galaxy/webapps/galaxy/services/quotas.py @@ -1,17 +1,14 @@ import logging from typing import Optional -from sqlalchemy import ( - false, - true, -) - -from galaxy import ( - model, - util, -) +from galaxy import util from galaxy.managers.context import ProvidesUserContext -from galaxy.managers.quotas import QuotaManager +from galaxy.managers.groups import get_group_by_name +from galaxy.managers.quotas import ( + get_quotas, + QuotaManager, +) +from galaxy.model.repositories import get_user_by_email from galaxy.quota._schema import ( CreateQuotaParams, CreateQuotaResult, @@ -39,14 +36,13 @@ def __init__(self, security: IdEncodingHelper, quota_manager: QuotaManager): def index(self, trans: ProvidesUserContext, deleted: bool = False) -> QuotaSummaryList: """Displays a list of quotas.""" rval = [] - query = trans.sa_session.query(model.Quota) if deleted: route = "deleted_quota" - query = query.filter(model.Quota.deleted == true()) + quotas = get_quotas(trans.sa_session, deleted=True) else: route = "quota" - query = query.filter(model.Quota.deleted == false()) - for quota in query: + quotas = get_quotas(trans.sa_session, deleted=False) + for quota in quotas: item = quota.to_dict(value_mapper={"id": DecodedDatabaseIdField.encode}) encoded_id = DecodedDatabaseIdField.encode(quota.id) item["url"] = url_for(route, id=encoded_id) @@ -119,25 +115,29 @@ def validate_in_users_and_groups(self, trans, payload): For convenience, in_users and in_groups can be encoded IDs or emails/group names in the API. """ - def get_id(item, model_class, column): + def get_user_id(item): + try: + return trans.security.decode_id(item) + except Exception: + return user_repo.get_by_email(item).id + + def get_group_id(item): try: return trans.security.decode_id(item) except Exception: - pass # maybe an email/group name - # this will raise if the item is invalid - return trans.sa_session.query(model_class).filter(column == item).first().id + return get_group_by_name(trans.sa_session, item).id new_in_users = [] new_in_groups = [] invalid = [] for item in util.listify(payload.get("in_users", [])): try: - new_in_users.append(get_id(item, model.User, model.User.email)) + new_in_users.append(get_user_id(item)) except Exception: invalid.append(item) for item in util.listify(payload.get("in_groups", [])): try: - new_in_groups.append(get_id(item, model.Group, model.Group.name)) + new_in_groups.append(get_group_id(item)) except Exception: invalid.append(item) if invalid: diff --git a/lib/galaxy/webapps/galaxy/services/tool_shed_repositories.py b/lib/galaxy/webapps/galaxy/services/tool_shed_repositories.py index 1efab0e1e36f..85de0b380cab 100644 --- a/lib/galaxy/webapps/galaxy/services/tool_shed_repositories.py +++ b/lib/galaxy/webapps/galaxy/services/tool_shed_repositories.py @@ -5,9 +5,9 @@ from pydantic import BaseModel from sqlalchemy import ( - and_, cast, Integer, + select, ) from galaxy.model.scoped_session import install_model_scoped_session @@ -17,10 +17,7 @@ CheckForUpdatesResponse, InstalledToolShedRepository, ) -from galaxy.tool_shed.util.repository_util import ( - check_for_updates, - get_tool_shed_repository_by_decoded_id, -) +from galaxy.tool_shed.util.repository_util import check_for_updates from galaxy.util.tool_shed.tool_shed_registry import Registry from galaxy.web import url_for @@ -43,31 +40,20 @@ def __init__( self._tool_shed_registry = tool_shed_registry def index(self, request: InstalledToolShedRepositoryIndexRequest) -> List[InstalledToolShedRepository]: - clause_list = [] - if request.name is not None: - clause_list.append(ToolShedRepository.table.c.name == request.name) - if request.owner is not None: - clause_list.append(ToolShedRepository.table.c.owner == request.owner) - if request.changeset is not None: - clause_list.append(ToolShedRepository.table.c.changeset_revision == request.changeset) - if request.deleted is not None: - clause_list.append(ToolShedRepository.table.c.deleted == request.deleted) - if request.uninstalled is not None: - clause_list.append(ToolShedRepository.table.c.uninstalled == request.uninstalled) - query = ( - self._install_model_context.query(ToolShedRepository) - .order_by(ToolShedRepository.table.c.name) - .order_by(cast(ToolShedRepository.ctx_rev, Integer).desc()) + repositories = self._get_tool_shed_repositories( + name=request.name, + owner=request.owner, + changeset_revision=request.changeset, + deleted=request.deleted, + uninstalled=request.uninstalled, ) - if len(clause_list) > 0: - query = query.filter(and_(*clause_list)) index = [] - for repository in query.all(): + for repository in repositories: index.append(self._show(repository)) return index def show(self, repository_id: DecodedDatabaseIdField) -> InstalledToolShedRepository: - tool_shed_repository = get_tool_shed_repository_by_decoded_id(self._install_model_context, int(repository_id)) + tool_shed_repository = self._install_model_context.get(ToolShedRepository, repository_id) return self._show(tool_shed_repository) def check_for_updates(self, repository_id: Optional[int]) -> CheckForUpdatesResponse: @@ -81,3 +67,13 @@ def _show(self, tool_shed_repository: ToolShedRepository) -> InstalledToolShedRe tool_shed_repository_dict["error_message"] = tool_shed_repository.error_message or "" tool_shed_repository_dict["url"] = url_for("tool_shed_repositories", id=encoded_id) return InstalledToolShedRepository(**tool_shed_repository_dict) + + def _get_tool_shed_repositories(self, **kwd): + stmt = select(ToolShedRepository) + for key, value in kwd.items(): + if value is not None: + column = ToolShedRepository.table.c[key] + stmt = stmt.filter(column == value) + stmt = stmt.order_by(ToolShedRepository.name).order_by(cast(ToolShedRepository.ctx_rev, Integer).desc()) + session = self._install_model_context + return session.scalars(stmt).all() diff --git a/lib/galaxy/webapps/galaxy/services/tools.py b/lib/galaxy/webapps/galaxy/services/tools.py index cb5b229b4409..9265f51427f5 100644 --- a/lib/galaxy/webapps/galaxy/services/tools.py +++ b/lib/galaxy/webapps/galaxy/services/tools.py @@ -23,7 +23,10 @@ ProvidesUserContext, ) from galaxy.managers.histories import HistoryManager -from galaxy.model import PostJobAction +from galaxy.model import ( + LibraryDatasetDatasetAssociation, + PostJobAction, +) from galaxy.model.base import transaction from galaxy.schema.fetch_data import ( FetchDataFormPayload, @@ -277,7 +280,7 @@ def _patch_library_inputs(self, trans: ProvidesHistoryContext, inputs, target_hi def _patch_library_dataset(self, trans: ProvidesHistoryContext, v, target_history): if isinstance(v, dict) and "id" in v and v.get("src") == "ldda": - ldda = trans.sa_session.query(trans.app.model.LibraryDatasetDatasetAssociation).get(self.decode_id(v["id"])) + ldda = trans.sa_session.get(LibraryDatasetDatasetAssociation, self.decode_id(v["id"])) if trans.user_is_admin or trans.app.security_agent.can_access_dataset( trans.get_current_user_roles(), ldda.dataset ): diff --git a/lib/galaxy/webapps/galaxy/services/users.py b/lib/galaxy/webapps/galaxy/services/users.py index 83d8a47392b7..de291d9c5b19 100644 --- a/lib/galaxy/webapps/galaxy/services/users.py +++ b/lib/galaxy/webapps/galaxy/services/users.py @@ -7,6 +7,7 @@ from sqlalchemy import ( false, or_, + select, true, ) @@ -204,30 +205,30 @@ def get_index( f_any: Optional[str], ) -> List[Union[UserModel, LimitedUserModel]]: rval = [] - query = trans.sa_session.query(User) + stmt = select(User) if f_email and (trans.user_is_admin or trans.app.config.expose_user_email): - query = query.filter(User.email.like(f"%{f_email}%")) + stmt = stmt.filter(User.email.like(f"%{f_email}%")) if f_name and (trans.user_is_admin or trans.app.config.expose_user_name): - query = query.filter(User.username.like(f"%{f_name}%")) + stmt = stmt.filter(User.username.like(f"%{f_name}%")) if f_any: if trans.user_is_admin: - query = query.filter(or_(User.email.like(f"%{f_any}%"), User.username.like(f"%{f_any}%"))) + stmt = stmt.filter(or_(User.email.like(f"%{f_any}%"), User.username.like(f"%{f_any}%"))) else: if trans.app.config.expose_user_email and trans.app.config.expose_user_name: - query = query.filter(or_(User.email.like(f"%{f_any}%"), User.username.like(f"%{f_any}%"))) + stmt = stmt.filter(or_(User.email.like(f"%{f_any}%"), User.username.like(f"%{f_any}%"))) elif trans.app.config.expose_user_email: - query = query.filter(User.email.like(f"%{f_any}%")) + stmt = stmt.filter(User.email.like(f"%{f_any}%")) elif trans.app.config.expose_user_name: - query = query.filter(User.username.like(f"%{f_any}%")) + stmt = stmt.filter(User.username.like(f"%{f_any}%")) if deleted: # only admins can see deleted users if not trans.user_is_admin: return [] - query = query.filter(User.table.c.deleted == true()) + stmt = stmt.filter(User.deleted == true()) else: # special case: user can see only their own user # special case2: if the galaxy admin has specified that other user email/names are @@ -239,8 +240,8 @@ def get_index( ): item = trans.user.to_dict(value_mapper={"id": trans.security.encode_id}) return [item] - query = query.filter(User.table.c.deleted == false()) - for user in query: + stmt = stmt.filter(User.deleted == false()) + for user in trans.sa_session.scalars(stmt).all(): item = user.to_dict(value_mapper={"id": trans.security.encode_id}) # If NOT configured to expose_email, do not expose email UNLESS the user is self, or # the user is an admin From 7a06b006c671c673fe996a9cfcb2d1d64870912a Mon Sep 17 00:00:00 2001 From: John Davis Date: Thu, 21 Sep 2023 09:52:34 -0400 Subject: [PATCH 31/50] Refactor get_user_by_username, get_user_by_email; use across code base --- lib/galaxy/managers/users.py | 43 ++++++++----------- lib/galaxy/visualization/genomes.py | 2 + lib/galaxy/webapps/galaxy/controllers/user.py | 5 +-- lib/galaxy/webapps/galaxy/services/quotas.py | 4 +- .../webapps/reports/controllers/jobs.py | 6 +-- lib/tool_shed/test/base/test_db_util.py | 7 +-- .../webapp/controllers/repository.py | 4 +- test/integration/test_user_preferences.py | 6 +-- test/integration/test_vault_extra_prefs.py | 5 +-- test/integration/test_vault_file_source.py | 17 +++----- 10 files changed, 42 insertions(+), 57 deletions(-) diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index dedc1d5ef90d..69e74c3bae42 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -359,7 +359,7 @@ def get_user_by_identity(self, identity): user = None if VALID_EMAIL_RE.match(identity): # VALID_PUBLICNAME and VALID_EMAIL do not overlap, so 'identity' here is an email address - user = self.session().query(self.model_class).filter(self.model_class.table.c.email == identity).first() + user = get_user_by_email(self.session(), identity, self.model_class) if not user: # Try a case-insensitive match on the email user = ( @@ -369,7 +369,7 @@ def get_user_by_identity(self, identity): .first() ) else: - user = self.session().query(self.model_class).filter(self.model_class.table.c.username == identity).first() + user = get_user_by_username(self.session(), identity, self.model_class) return user # ---- current @@ -532,7 +532,7 @@ def __get_activation_token(self, trans, email): """ Check for the activation token. Create new activation token and store it in the database if no token found. """ - user = trans.sa_session.query(self.app.model.User).filter(self.app.model.User.table.c.email == email).first() + user = get_user_by_email(trans.sa_session, email, self.app.model.User) activation_token = user.activation_token if activation_token is None: activation_token = util.hash_util.new_secure_hash_v2(str(random.getrandbits(256))) @@ -576,9 +576,7 @@ def send_reset_email(self, trans, payload, **kwd): return "Failed to produce password reset token. User not found." def get_reset_token(self, trans, email): - reset_user = ( - trans.sa_session.query(self.app.model.User).filter(self.app.model.User.table.c.email == email).first() - ) + reset_user = get_user_by_email(trans.sa_session, email, self.app.model.User) if not reset_user and email != email.lower(): reset_user = ( trans.sa_session.query(self.app.model.User) @@ -622,12 +620,7 @@ def get_or_create_remote_user(self, remote_user_email): return None if getattr(self.app.config, "normalize_remote_user_email", False): remote_user_email = remote_user_email.lower() - user = ( - self.session() - .query(self.app.model.User) - .filter(self.app.model.User.table.c.email == remote_user_email) - .first() - ) + user = get_user_by_email(self.session(), remote_user_email, self.app.model.User) if user: # GVK: June 29, 2009 - This is to correct the behavior of a previous bug where a private # role and default user / history permissions were not set for remote users. When a @@ -844,18 +837,20 @@ def _add_parsers(self): self.fn_filter_parsers.update({}) -def get_user_by_username(session, user_class, username): - """ - Get a user from the database by username. - (We pass the session and the user_class to accommodate usage from the tool_shed app.) - """ - try: - stmt = select(user_class).filter(user_class.username == username) - return session.execute(stmt).scalar_one() - except Exception: - return None - - def get_users_by_ids(session: Session, user_ids): stmt = select(User).where(User.id.in_(user_ids)) return session.scalars(stmt).all() + + +# The get_user_by_email and get_user_by_username functions may be called from +# the tool_shed app, which has its own User model, which is different from +# galaxy.model.User. In that case, the tool_shed user model should be passed as +# the model_class argument. +def get_user_by_email(session, email: str, model_class=User): + stmt = select(model_class).filter(model_class.email == email).limit(1) + return session.scalars(stmt).first() + + +def get_user_by_username(session, username: str, model_class=User): + stmt = select(model_class).filter(model_class.username == username).limit(1) + return session.scalars(stmt).first() diff --git a/lib/galaxy/visualization/genomes.py b/lib/galaxy/visualization/genomes.py index 9931b889ddf1..2621c94b6e1c 100644 --- a/lib/galaxy/visualization/genomes.py +++ b/lib/galaxy/visualization/genomes.py @@ -11,6 +11,8 @@ ObjectNotFound, ReferenceDataError, ) +from galaxy.managers.users import get_user_by_username +from galaxy.model import HistoryDatasetAssociation from galaxy.structured_app import StructuredApp from galaxy.util.bunch import Bunch diff --git a/lib/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py index 193255a0b59a..7fc9b8c1e017 100644 --- a/lib/galaxy/webapps/galaxy/controllers/user.py +++ b/lib/galaxy/webapps/galaxy/controllers/user.py @@ -18,6 +18,7 @@ ) from galaxy.exceptions import Conflict from galaxy.managers import users +from galaxy.managers.users import get_user_by_email from galaxy.security.validate_user_input import ( validate_email, validate_publicname, @@ -293,9 +294,7 @@ def activate(self, trans, **kwd): ) else: # Find the user - user = ( - trans.sa_session.query(trans.app.model.User).filter(trans.app.model.User.table.c.email == email).first() - ) + user = get_user_by_email(trans.sa_session, email) if not user: # Probably wrong email address return trans.show_error_message( diff --git a/lib/galaxy/webapps/galaxy/services/quotas.py b/lib/galaxy/webapps/galaxy/services/quotas.py index b2527d130416..05e51ea376fe 100644 --- a/lib/galaxy/webapps/galaxy/services/quotas.py +++ b/lib/galaxy/webapps/galaxy/services/quotas.py @@ -8,7 +8,7 @@ get_quotas, QuotaManager, ) -from galaxy.model.repositories import get_user_by_email +from galaxy.managers.users import get_user_by_email from galaxy.quota._schema import ( CreateQuotaParams, CreateQuotaResult, @@ -119,7 +119,7 @@ def get_user_id(item): try: return trans.security.decode_id(item) except Exception: - return user_repo.get_by_email(item).id + return get_user_by_email(trans.sa_session, item).id def get_group_id(item): try: diff --git a/lib/galaxy/webapps/reports/controllers/jobs.py b/lib/galaxy/webapps/reports/controllers/jobs.py index 1c6e9a9c2ec6..e281f538b868 100644 --- a/lib/galaxy/webapps/reports/controllers/jobs.py +++ b/lib/galaxy/webapps/reports/controllers/jobs.py @@ -18,15 +18,14 @@ and_, not_, or_, - select, ) from galaxy import ( model, util, ) +from galaxy.managers.users import get_user_by_email from galaxy.model import Job -from galaxy.model.repositories import get_user_by_email from galaxy.web.legacy_framework import grids from galaxy.webapps.base.controller import ( BaseUIController, @@ -1307,8 +1306,7 @@ def get_monitor_id(trans, monitor_email): A convenience method to obtain the monitor job id. """ monitor_user_id = None - stmt = select(trans.model.User.id).filter(trans.model.User.email == monitor_email).limit(1) - monitor_row = trans.sa_session.scalars(stmt).first() + monitor_row = get_user_by_email(trans.sa_session, monitor_email) if monitor_row is not None: monitor_user_id = monitor_row[0] return monitor_user_id diff --git a/lib/tool_shed/test/base/test_db_util.py b/lib/tool_shed/test/base/test_db_util.py index 3b4cbce31f16..0ae2c8e76828 100644 --- a/lib/tool_shed/test/base/test_db_util.py +++ b/lib/tool_shed/test/base/test_db_util.py @@ -10,6 +10,7 @@ import galaxy.model import galaxy.model.tool_shed_install import tool_shed.webapp.model as model +from galaxy.managers.users import get_user_by_username log = logging.getLogger("test.tool_shed.test_db_util") @@ -171,10 +172,6 @@ def get_user(email): return sa_session().query(model.User).filter(model.User.table.c.email == email).first() -def get_user_by_name(username): - return sa_session().query(model.User).filter(model.User.table.c.username == username).first() - - def mark_obj_deleted(obj): obj.deleted = True sa_session().add(obj) @@ -190,7 +187,7 @@ def ga_refresh(obj): def get_repository_by_name_and_owner(name, owner_username, return_multiple=False): - owner = get_user_by_name(owner_username) + owner = get_user_by_username(sa_session(), owner_username, model.User) repository = ( sa_session() .query(model.Repository) diff --git a/lib/tool_shed/webapp/controllers/repository.py b/lib/tool_shed/webapp/controllers/repository.py index e7627292f426..03ee7e2a6671 100644 --- a/lib/tool_shed/webapp/controllers/repository.py +++ b/lib/tool_shed/webapp/controllers/repository.py @@ -2293,7 +2293,7 @@ def set_malicious(self, trans, id, ctx_str, **kwd): def sharable_owner(self, trans, owner): """Support for sharable URL for each repository owner's tools, e.g. http://example.org/view/owner.""" try: - user = get_user_by_username(trans.model.session, trans.model.User, owner) + user = get_user_by_username(trans.model.session, owner, trans.model.User) except Exception: user = None if user: @@ -2321,7 +2321,7 @@ def sharable_repository(self, trans, owner, name): else: # If the owner is valid, then show all of their repositories. try: - user = get_user_by_username(trans.model.session, trans.model.User, owner) + user = get_user_by_username(trans.model.session, owner, trans.model.User) except Exception: user = None if user: diff --git a/test/integration/test_user_preferences.py b/test/integration/test_user_preferences.py index 84dd304b643c..b01f4850489d 100644 --- a/test/integration/test_user_preferences.py +++ b/test/integration/test_user_preferences.py @@ -7,8 +7,8 @@ get, put, ) -from sqlalchemy import select +from galaxy.managers.users import get_user_by_email from galaxy_test.driver import integration_util TEST_USER_EMAIL = "test_user_preferences@bx.psu.edu" @@ -19,8 +19,8 @@ def test_user_theme(self): user = self._setup_user(TEST_USER_EMAIL) url = self._api_url(f"users/{user['id']}/theme/test_theme", params=dict(key=self.master_api_key)) app = cast(Any, self._test_driver.app if self._test_driver else None) - stmt = select(app.model.User).filter(app.model.User.email == user["email"]).limit(1) - db_user = app.model.session.scalars(stmt).first() + + db_user = get_user_by_email(app.model.session, user["email"]) # create some initial data put(url) diff --git a/test/integration/test_vault_extra_prefs.py b/test/integration/test_vault_extra_prefs.py index 4599d2decbe5..e3ec8fd5c7f1 100644 --- a/test/integration/test_vault_extra_prefs.py +++ b/test/integration/test_vault_extra_prefs.py @@ -9,8 +9,8 @@ get, put, ) -from sqlalchemy import select +from galaxy.managers.users import get_user_by_email from galaxy_test.driver import integration_util TEST_USER_EMAIL = "vault_test_user@bx.psu.edu" @@ -133,5 +133,4 @@ def __url(self, action, user): return self._api_url(f"users/{user['id']}/{action}", params=dict(key=self.master_api_key)) def _get_dbuser(self, app, user): - stmt = select(app.model.User).filter(app.model.User.email == user["email"]).limit(1) - return app.model.session.scalars(stmt).first() + return get_user_by_email(app.model.session, user["email"]) diff --git a/test/integration/test_vault_file_source.py b/test/integration/test_vault_file_source.py index ab6295058ff9..dc39477cd2f6 100644 --- a/test/integration/test_vault_file_source.py +++ b/test/integration/test_vault_file_source.py @@ -1,8 +1,7 @@ import os import tempfile -from sqlalchemy import select - +from galaxy.managers.users import get_user_by_email from galaxy.security.vault import UserVaultWrapper from galaxy_test.base import api_asserts from galaxy_test.base.populators import DatasetPopulator @@ -36,7 +35,7 @@ def test_vault_secret_per_user_in_file_source(self): app = self._app with self._different_user(email=self.USER_1_APP_VAULT_ENTRY): - user = self._get_user_by_email(self.USER_1_APP_VAULT_ENTRY) + user = get_user_by_email(self._app.model.session, self.USER_1_APP_VAULT_ENTRY) user_vault = UserVaultWrapper(app.vault, user) # use a valid symlink path so the posix list succeeds user_vault.write_secret("posix/root_path", app.config.user_library_import_symlink_allowlist[0]) @@ -48,7 +47,7 @@ def test_vault_secret_per_user_in_file_source(self): print(remote_files) with self._different_user(email=self.USER_2_APP_VAULT_ENTRY): - user = self._get_user_by_email(self.USER_2_APP_VAULT_ENTRY) + user = get_user_by_email(self._app.model.session, self.USER_2_APP_VAULT_ENTRY) user_vault = UserVaultWrapper(app.vault, user) # use an invalid symlink path so the posix list fails user_vault.write_secret("posix/root_path", "/invalid/root") @@ -69,7 +68,7 @@ def test_vault_secret_per_app_in_file_source(self): app.vault.write_secret("posix/root_path", app.config.user_library_import_symlink_allowlist[0]) with self._different_user(email=self.USER_1_APP_VAULT_ENTRY): - user = self._get_user_by_email(self.USER_1_APP_VAULT_ENTRY) + user = get_user_by_email(self._app.model.session, self.USER_1_APP_VAULT_ENTRY) user_vault = UserVaultWrapper(app.vault, user) # use a valid symlink path so the posix list succeeds user_vault.write_secret("posix/root_path", app.config.user_library_import_symlink_allowlist[0]) @@ -81,7 +80,7 @@ def test_vault_secret_per_app_in_file_source(self): print(remote_files) with self._different_user(email=self.USER_2_APP_VAULT_ENTRY): - user = self._get_user_by_email(self.USER_2_APP_VAULT_ENTRY) + user = get_user_by_email(self._app.model.session, self.USER_2_APP_VAULT_ENTRY) user_vault = UserVaultWrapper(app.vault, user) # use an invalid symlink path so the posix list would fail if used user_vault.write_secret("posix/root_path", "/invalid/root") @@ -95,7 +94,7 @@ def test_vault_secret_per_app_in_file_source(self): def test_upload_file_from_remote_source(self): with self._different_user(email=self.USER_1_APP_VAULT_ENTRY): app = self._app - user = self._get_user_by_email(self.USER_1_APP_VAULT_ENTRY) + user = get_user_by_email(self._app.model.session, self.USER_1_APP_VAULT_ENTRY) user_vault = UserVaultWrapper(app.vault, user) # use a valid symlink path so the posix list succeeds user_vault.write_secret("posix/root_path", app.config.user_library_import_symlink_allowlist[0]) @@ -120,7 +119,3 @@ def test_upload_file_from_remote_source(self): new_dataset = self.dataset_populator.fetch(payload, assert_ok=True).json()["outputs"][0] content = self.dataset_populator.get_history_dataset_content(history_id, dataset=new_dataset) assert content == "I require access to the vault", content - - def _get_user_by_email(self, email): - stmt = select(self._app.model.User).filter(self._app.model.User.email == email).limit(1) - return self._app.model.session.scalars(stmt).first() From f5d6578d4b4e8b050eef743f893de3483a3bfff5 Mon Sep 17 00:00:00 2001 From: John Davis Date: Mon, 7 Aug 2023 15:34:48 -0400 Subject: [PATCH 32/50] Fix SA2.0 ORM usage in galaxy.visualization --- lib/galaxy/visualization/genomes.py | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/visualization/genomes.py b/lib/galaxy/visualization/genomes.py index 2621c94b6e1c..93f3a2dcbfd2 100644 --- a/lib/galaxy/visualization/genomes.py +++ b/lib/galaxy/visualization/genomes.py @@ -289,11 +289,12 @@ def chroms(self, trans, dbkey=None, num=None, chrom=None, low=None): Returns a naturally sorted list of chroms/contigs for a given dbkey. Use either chrom or low to specify the starting chrom in the return list. """ + session = trans.sa_session self.check_and_reload() # If there is no dbkey owner, default to current user. dbkey_owner, dbkey = decode_dbkey(dbkey) if dbkey_owner: - dbkey_user = trans.sa_session.query(trans.app.model.User).filter_by(username=dbkey_owner).first() + dbkey_user = get_user_by_username(session, dbkey_owner) else: dbkey_user = trans.user @@ -309,12 +310,9 @@ def chroms(self, trans, dbkey=None, num=None, chrom=None, low=None): if dbkey in user_keys: dbkey_attributes = user_keys[dbkey] dbkey_name = dbkey_attributes["name"] - # If there's a fasta for genome, convert to 2bit for later use. if "fasta" in dbkey_attributes: - build_fasta = trans.sa_session.query(trans.app.model.HistoryDatasetAssociation).get( - dbkey_attributes["fasta"] - ) + build_fasta = session.get(HistoryDatasetAssociation, dbkey_attributes["fasta"]) len_file = build_fasta.get_converted_dataset(trans, "len").file_name build_fasta.get_converted_dataset(trans, "twobit") # HACK: set twobit_file to True rather than a file name because @@ -323,11 +321,7 @@ def chroms(self, trans, dbkey=None, num=None, chrom=None, low=None): twobit_file = True # Backwards compatibility: look for len file directly. elif "len" in dbkey_attributes: - len_file = ( - trans.sa_session.query(trans.app.model.HistoryDatasetAssociation) - .get(user_keys[dbkey]["len"]) - .file_name - ) + len_file = session.get(HistoryDatasetAssociation, user_keys[dbkey]["len"]).file_name if len_file: genome = Genome(dbkey, dbkey_name, len_file=len_file, twobit_file=twobit_file) @@ -376,7 +370,7 @@ def reference(self, trans, dbkey, chrom, low, high): # If there is no dbkey owner, default to current user. dbkey_owner, dbkey = decode_dbkey(dbkey) if dbkey_owner: - dbkey_user = trans.sa_session.query(trans.app.model.User).filter_by(username=dbkey_owner).first() + dbkey_user = get_user_by_username(trans.sa_session, dbkey_owner) else: dbkey_user = trans.user @@ -393,9 +387,7 @@ def reference(self, trans, dbkey, chrom, low, high): else: user_keys = loads(dbkey_user.preferences["dbkeys"]) dbkey_attributes = user_keys[dbkey] - fasta_dataset = trans.sa_session.query(trans.app.model.HistoryDatasetAssociation).get( - dbkey_attributes["fasta"] - ) + fasta_dataset = trans.sa_session.get(HistoryDatasetAssociation, dbkey_attributes["fasta"]) msg = fasta_dataset.convert_dataset(trans, "twobit") if msg: return msg From 6da80658d6ee55f6ed72df6ea23ed2574e9b0d5a Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 22 Sep 2023 11:15:54 -0400 Subject: [PATCH 33/50] Update lib/galaxy/tools/parameters/basic.py Co-authored-by: Marius van den Beek --- lib/galaxy/tools/parameters/basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 5025ef1336e8..f921770a1660 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -2189,7 +2189,7 @@ def from_json(self, value, trans, other_values=None): if len(rval) > 1: raise ParameterValueError("more than one dataset supplied to single input dataset parameter", self.name) if len(rval) > 0: - rval = rval[0] # type:ignore[assignment] + return rval[0] else: raise ParameterValueError("invalid dataset supplied to single input dataset parameter", self.name) return rval From 0b1cccd5555d8a195feed936a605556ac6e70e0d Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 22 Sep 2023 11:21:39 -0400 Subject: [PATCH 34/50] Update lib/galaxy/webapps/galaxy/controllers/history.py Co-authored-by: Marius van den Beek --- lib/galaxy/webapps/galaxy/controllers/history.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/controllers/history.py b/lib/galaxy/webapps/galaxy/controllers/history.py index 2dd00d2d487d..8f72a7d17e34 100644 --- a/lib/galaxy/webapps/galaxy/controllers/history.py +++ b/lib/galaxy/webapps/galaxy/controllers/history.py @@ -516,11 +516,8 @@ def display_by_username_and_slug(self, trans, username, slug, **kwargs): # Get history. session = trans.sa_session - stmt = select(model.User).filter_by(username=username).limit(1) - user = session.scalars(stmt).first() - - stmt = select(model.History).filter_by(user=user, slug=slug, deleted=False).limit(1) - history = session.scalars(stmt).first() + user = session.scalars(select(model.User).filter_by(username=username).limit(1)).first() + history = session.scalars(select(model.History).filter_by(user=user, slug=slug, deleted=False).limit(1)).first() if history is None: raise web.httpexceptions.HTTPNotFound() From 1da8b1d954d50c70f2d0596b99c7e18695645308 Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 22 Sep 2023 16:33:43 -0400 Subject: [PATCH 35/50] Move get_quotas into services --- lib/galaxy/managers/quotas.py | 16 --------------- lib/galaxy/webapps/galaxy/services/quotas.py | 21 ++++++++++++++++---- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/lib/galaxy/managers/quotas.py b/lib/galaxy/managers/quotas.py index e0601d73081a..0340698aa28f 100644 --- a/lib/galaxy/managers/quotas.py +++ b/lib/galaxy/managers/quotas.py @@ -11,20 +11,12 @@ Union, ) -from sqlalchemy import ( - false, - select, - true, -) -from sqlalchemy.orm import Session - from galaxy import ( model, util, ) from galaxy.exceptions import ActionInputError from galaxy.managers import base -from galaxy.model import Quota from galaxy.model.base import transaction from galaxy.quota import DatabaseQuotaAgent from galaxy.quota._schema import ( @@ -281,11 +273,3 @@ def purge_quota(self, quota, params=None): def get_quota(self, trans, id: int, deleted: Optional[bool] = None) -> model.Quota: return base.get_object(trans, id, "Quota", check_ownership=False, check_accessible=False, deleted=deleted) - - -def get_quotas(session: Session, deleted: bool = False): - is_deleted = true() - if not deleted: - is_deleted = false() - stmt = select(Quota).where(Quota.deleted == is_deleted) - return session.scalars(stmt) diff --git a/lib/galaxy/webapps/galaxy/services/quotas.py b/lib/galaxy/webapps/galaxy/services/quotas.py index 05e51ea376fe..3c1fdb7ab299 100644 --- a/lib/galaxy/webapps/galaxy/services/quotas.py +++ b/lib/galaxy/webapps/galaxy/services/quotas.py @@ -1,14 +1,19 @@ import logging from typing import Optional +from sqlalchemy import ( + false, + select, + true, +) +from sqlalchemy.orm import Session + from galaxy import util from galaxy.managers.context import ProvidesUserContext from galaxy.managers.groups import get_group_by_name -from galaxy.managers.quotas import ( - get_quotas, - QuotaManager, -) +from galaxy.managers.quotas import QuotaManager from galaxy.managers.users import get_user_by_email +from galaxy.model import Quota from galaxy.quota._schema import ( CreateQuotaParams, CreateQuotaResult, @@ -148,3 +153,11 @@ def get_group_id(item): raise Exception(msg) payload["in_users"] = list(map(str, new_in_users)) payload["in_groups"] = list(map(str, new_in_groups)) + + +def get_quotas(session: Session, deleted: bool = False): + is_deleted = true() + if not deleted: + is_deleted = false() + stmt = select(Quota).where(Quota.deleted == is_deleted) + return session.scalars(stmt) From 23aa5060dd21f6d889faabe1614609a1049e127d Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 22 Sep 2023 16:37:16 -0400 Subject: [PATCH 36/50] Raise exception if no Library found in api method --- lib/galaxy/webapps/galaxy/api/library_contents.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/galaxy/webapps/galaxy/api/library_contents.py b/lib/galaxy/webapps/galaxy/api/library_contents.py index 06b74eda6edb..58899ed32aef 100644 --- a/lib/galaxy/webapps/galaxy/api/library_contents.py +++ b/lib/galaxy/webapps/galaxy/api/library_contents.py @@ -99,6 +99,8 @@ def traverse(folder): return rval library = trans.sa_session.get(Library, self.decode_id(library_id)) + if not library: + raise exceptions.RequestParameterInvalidException("No library found with the id provided.") if not (trans.user_is_admin or trans.app.security_agent.can_access_library(current_user_roles, library)): raise exceptions.RequestParameterInvalidException("No library found with the id provided.") encoded_id = f"F{trans.security.encode_id(library.root_folder.id)}" From 4b6b85e04c04864fe53a9fad631e4f66c05caab4 Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 22 Sep 2023 16:57:30 -0400 Subject: [PATCH 37/50] Move get_fasta_hdas_by_history into services --- lib/galaxy/managers/histories.py | 11 ----------- lib/galaxy/webapps/galaxy/services/histories.py | 13 ++++++++++++- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/galaxy/managers/histories.py b/lib/galaxy/managers/histories.py index 672428ef1edd..2f89fb1bed86 100644 --- a/lib/galaxy/managers/histories.py +++ b/lib/galaxy/managers/histories.py @@ -24,7 +24,6 @@ select, true, ) -from sqlalchemy.orm import Session from typing_extensions import Literal from galaxy import ( @@ -44,7 +43,6 @@ StorageCleanerManager, ) from galaxy.managers.export_tracker import StoreExportTracker -from galaxy.model import HistoryDatasetAssociation from galaxy.model.base import transaction from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.schema import ( @@ -902,12 +900,3 @@ def username_eq(self, item, val: str) -> bool: def username_contains(self, item, val: str) -> bool: return val.lower() in str(item.user.username).lower() - - -def get_fasta_hdas_by_history(session: Session, history_id: int): - stmt = ( - select(HistoryDatasetAssociation) - .filter_by(history_id=history_id, extension="fasta", deleted=False) - .order_by(HistoryDatasetAssociation.hid.desc()) - ) - return session.scalars(stmt).all() diff --git a/lib/galaxy/webapps/galaxy/services/histories.py b/lib/galaxy/webapps/galaxy/services/histories.py index b330c77ab68e..30e10f28f2d7 100644 --- a/lib/galaxy/webapps/galaxy/services/histories.py +++ b/lib/galaxy/webapps/galaxy/services/histories.py @@ -16,8 +16,10 @@ from sqlalchemy import ( false, + select, true, ) +from sqlalchemy.orm import Session from galaxy import ( exceptions as glx_exceptions, @@ -32,7 +34,6 @@ from galaxy.managers.citations import CitationsManager from galaxy.managers.context import ProvidesHistoryContext from galaxy.managers.histories import ( - get_fasta_hdas_by_history, HistoryDeserializer, HistoryExportManager, HistoryFilters, @@ -41,6 +42,7 @@ ) from galaxy.managers.notification import NotificationManager from galaxy.managers.users import UserManager +from galaxy.model import HistoryDatasetAssociation from galaxy.model.base import transaction from galaxy.model.store import payload_to_source_uri from galaxy.schema import ( @@ -788,3 +790,12 @@ def _get_export_record_data(self, history: model.History) -> Optional[WriteStore if export_metadata and isinstance(export_metadata.request_data.payload, WriteStoreToPayload): return export_metadata.request_data.payload return None + + +def get_fasta_hdas_by_history(session: Session, history_id: int): + stmt = ( + select(HistoryDatasetAssociation) + .filter_by(history_id=history_id, extension="fasta", deleted=False) + .order_by(HistoryDatasetAssociation.hid.desc()) + ) + return session.scalars(stmt).all() From b57d6ed05bfb28185108d75cd87598666ec65b32 Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 22 Sep 2023 17:28:17 -0400 Subject: [PATCH 38/50] Move method into User model class --- lib/galaxy/managers/workflows.py | 14 +------------- lib/galaxy/model/__init__.py | 6 ++++++ lib/galaxy/webapps/galaxy/api/workflows.py | 3 +-- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 5a81ba4df14d..4d84e834cd85 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -27,16 +27,13 @@ and_, desc, false, - func, or_, - select, true, ) from sqlalchemy.orm import ( aliased, joinedload, Query, - Session, subqueryload, ) @@ -53,10 +50,7 @@ from galaxy.managers.base import decode_id from galaxy.managers.context import ProvidesUserContext from galaxy.managers.executables import artifact_class -from galaxy.model import ( - StoredWorkflow, - StoredWorkflowUserShareAssociation, -) +from galaxy.model import StoredWorkflow from galaxy.model.base import transaction from galaxy.model.index_filter_util import ( append_user_filter, @@ -1978,9 +1972,3 @@ def import_workflow(self, workflow, **kwds): raise NotImplementedError( "Direct format 2 import of nested workflows is not yet implemented, use bioblend client." ) - - -def count_stored_workflow_user_assocs(session: Session, user, stored_workflow) -> int: - stmt = select(StoredWorkflowUserShareAssociation).filter_by(user=user, stored_workflow=stored_workflow) - stmt = select(func.count()).select_from(stmt) - return session.scalar(stmt) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 2b931668a18d..1b6e41335b36 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -1169,6 +1169,12 @@ def quota_source_usage_for(self, quota_source_label: Optional[str]) -> Optional[ return quota_source_usage return None + def count_stored_workflow_user_assocs(self, stored_workflow) -> int: + stmt = select(StoredWorkflowUserShareAssociation).filter_by(user=self, stored_workflow=stored_workflow) + stmt = select(func.count()).select_from(stmt) + session = object_session(self) + return session.scalar(stmt) + class PasswordResetToken(Base): __tablename__ = "password_reset_token" diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 7971d1115773..1b60d2a516ed 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -40,7 +40,6 @@ invocation_job_source_iter, ) from galaxy.managers.workflows import ( - count_stored_workflow_user_assocs, MissingToolsException, RefactorRequest, WorkflowCreateOptions, @@ -193,7 +192,7 @@ def show(self, trans: GalaxyWebTransaction, id, **kwd): """ stored_workflow = self.__get_stored_workflow(trans, id, **kwd) if stored_workflow.importable is False and stored_workflow.user != trans.user and not trans.user_is_admin: - wf_count = count_stored_workflow_user_assocs(trans.sa_session, trans.user, stored_workflow) + wf_count = trans.user.count_stored_workflow_user_assocs(stored_workflow) if wf_count == 0: message = "Workflow is neither importable, nor owned by or shared with current user" raise exceptions.ItemAccessibilityException(message) From 4256d87b80f29f9529e62f6acf4b6d81ec32cf07 Mon Sep 17 00:00:00 2001 From: John Davis Date: Mon, 25 Sep 2023 12:11:00 -0400 Subject: [PATCH 39/50] Check if user is not null before accessing user attribute --- lib/galaxy/webapps/galaxy/api/workflows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 1b60d2a516ed..2523819056d8 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -192,7 +192,7 @@ def show(self, trans: GalaxyWebTransaction, id, **kwd): """ stored_workflow = self.__get_stored_workflow(trans, id, **kwd) if stored_workflow.importable is False and stored_workflow.user != trans.user and not trans.user_is_admin: - wf_count = trans.user.count_stored_workflow_user_assocs(stored_workflow) + wf_count = 0 if not trans.user else trans.user.count_stored_workflow_user_assocs(stored_workflow) if wf_count == 0: message = "Workflow is neither importable, nor owned by or shared with current user" raise exceptions.ItemAccessibilityException(message) From 3501866c8d71c9e9e8fa06e7a1eab27b0e539151 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 26 Sep 2023 10:21:27 +0200 Subject: [PATCH 40/50] Fix response annotation for `show` in collections It always returns a detailed view of the collection since there are no serialization parameters in this route to change the view. --- client/src/schema/schema.ts | 2 +- lib/galaxy/webapps/galaxy/api/dataset_collections.py | 3 +-- lib/galaxy/webapps/galaxy/services/dataset_collections.py | 3 +-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/client/src/schema/schema.ts b/client/src/schema/schema.ts index 4f89f5262cc4..ac3a1a2ba096 100644 --- a/client/src/schema/schema.ts +++ b/client/src/schema/schema.ts @@ -9963,7 +9963,7 @@ export interface operations { /** @description Successful Response */ 200: { content: { - "application/json": components["schemas"]["HDCADetailed"] | components["schemas"]["HDCASummary"]; + "application/json": components["schemas"]["HDCADetailed"]; }; }; /** @description Validation Error */ diff --git a/lib/galaxy/webapps/galaxy/api/dataset_collections.py b/lib/galaxy/webapps/galaxy/api/dataset_collections.py index 5e9d61d1a314..8f164c4b122d 100644 --- a/lib/galaxy/webapps/galaxy/api/dataset_collections.py +++ b/lib/galaxy/webapps/galaxy/api/dataset_collections.py @@ -10,7 +10,6 @@ from galaxy.managers.context import ProvidesHistoryContext from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.schema import ( - AnyHDCA, CreateNewCollectionPayload, DatasetCollectionInstanceType, DCESummary, @@ -108,7 +107,7 @@ def show( trans: ProvidesHistoryContext = DependsOnTrans, id: DecodedDatabaseIdField = DatasetCollectionIdPathParam, instance_type: DatasetCollectionInstanceType = InstanceTypeQueryParam, - ) -> AnyHDCA: + ) -> HDCADetailed: return self.service.show(trans, id, instance_type) @router.get( diff --git a/lib/galaxy/webapps/galaxy/services/dataset_collections.py b/lib/galaxy/webapps/galaxy/services/dataset_collections.py index 5c59d260b092..dbee0078cddf 100644 --- a/lib/galaxy/webapps/galaxy/services/dataset_collections.py +++ b/lib/galaxy/webapps/galaxy/services/dataset_collections.py @@ -31,7 +31,6 @@ ModelClassField, ) from galaxy.schema.schema import ( - AnyHDCA, CreateNewCollectionPayload, DatasetCollectionInstanceType, DCESummary, @@ -186,7 +185,7 @@ def show( trans: ProvidesHistoryContext, id: DecodedDatabaseIdField, instance_type: DatasetCollectionInstanceType = "history", - ) -> AnyHDCA: + ) -> HDCADetailed: """ Returns information about a particular dataset collection. """ From e6e75b5be4daec8b9b3e28cca53f56ee378eac39 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 26 Sep 2023 10:37:14 +0200 Subject: [PATCH 41/50] Add get and fetch collection to store actions --- client/src/stores/collectionElementsStore.ts | 21 +++++++++++++++++++ .../services/datasetCollection.service.ts | 9 +++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/client/src/stores/collectionElementsStore.ts b/client/src/stores/collectionElementsStore.ts index 426f6a61aea7..b73366ccdabc 100644 --- a/client/src/stores/collectionElementsStore.ts +++ b/client/src/stores/collectionElementsStore.ts @@ -65,11 +65,32 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore", } } + async function getCollection(collectionId: string) { + const collection = storedCollections.value[collectionId]; + if (!collection) { + return await fetchCollection({ id: collectionId }); + } + return collection; + } + + async function fetchCollection(params: { id: string }) { + Vue.set(loadingCollectionElements.value, params.id, true); + try { + const collection = await Service.fetchCollectionDetails({ hdcaId: params.id }); + Vue.set(storedCollections.value, collection.id, collection); + return collection; + } finally { + Vue.delete(loadingCollectionElements.value, params.id); + } + } + return { storedCollections, storedCollectionElements, getCollectionElements, isLoadingCollectionElements, + getCollection, + fetchCollection, loadCollectionElements, saveCollections, }; diff --git a/client/src/stores/services/datasetCollection.service.ts b/client/src/stores/services/datasetCollection.service.ts index 474752989f18..1e0522085ef0 100644 --- a/client/src/stores/services/datasetCollection.service.ts +++ b/client/src/stores/services/datasetCollection.service.ts @@ -1,9 +1,16 @@ import { fetcher } from "@/schema"; -import { DCESummary, HDCASummary } from "."; +import { DCESummary, HDCADetailed, HDCASummary } from "."; const DEFAULT_LIMIT = 50; +const getCollectionDetails = fetcher.path("/api/dataset_collections/{id}").method("get").create(); + +export async function fetchCollectionDetails(params: { hdcaId: string }): Promise { + const { data } = await getCollectionDetails({ id: params.hdcaId }); + return data; +} + const getCollectionContents = fetcher .path("/api/dataset_collections/{hdca_id}/contents/{parent_id}") .method("get") From c029991e52e4fb6d71355722290068ab93804e5f Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 26 Sep 2023 10:41:05 +0200 Subject: [PATCH 42/50] Fetch sub-collection when drilling down if not in store The DCObject information does not contain enough information (like `element_count` or `collection_id`) to handle the sub-collection elements navigation downstream. --- .../CurrentCollection/CollectionPanel.vue | 18 ++++-------------- client/src/stores/services/index.ts | 1 + 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/client/src/components/History/CurrentCollection/CollectionPanel.vue b/client/src/components/History/CurrentCollection/CollectionPanel.vue index 42e159a4039b..9a48db2afdd6 100644 --- a/client/src/components/History/CurrentCollection/CollectionPanel.vue +++ b/client/src/components/History/CurrentCollection/CollectionPanel.vue @@ -7,7 +7,7 @@ import ExpandedItems from "@/components/History/Content/ExpandedItems"; import { updateContentFields } from "@/components/History/model/queries"; import { useCollectionElementsStore } from "@/stores/collectionElementsStore"; import { HistorySummary } from "@/stores/historyStore"; -import { DCESummary, HDCASummary } from "@/stores/services"; +import { DCESummary, DCObject, HDCASummary } from "@/stores/services"; import CollectionDetails from "./CollectionDetails.vue"; import CollectionNavigation from "./CollectionNavigation.vue"; @@ -59,19 +59,9 @@ function onScroll(newOffset: number) { offset.value = newOffset; } -/** - * Passes a sub-collection i.e a collection element object containing another collection, into - * a populated object for drill-down without the need for a separate data request. This object - * is used for breadcrumbs in the navigation component and to render the collection details and - * description at the top of the collection panel. Details include the collection name, the - * collection type, and the element count. - */ -function onViewSubCollection(itemObject: HDCASummary, elementIdentifier: string) { - const collectionObject = { - name: elementIdentifier, - ...itemObject, - }; - emit("view-collection", collectionObject); +async function onViewSubCollection(itemObject: DCObject) { + const collection = await collectionElementsStore.getCollection(itemObject.id); + emit("view-collection", collection); } watch( diff --git a/client/src/stores/services/index.ts b/client/src/stores/services/index.ts index 49d8cc27d665..5c4053cbf0dd 100644 --- a/client/src/stores/services/index.ts +++ b/client/src/stores/services/index.ts @@ -5,6 +5,7 @@ export type DatasetDetails = components["schemas"]["HDADetailed"]; export type DCESummary = components["schemas"]["DCESummary"]; export type HDCASummary = components["schemas"]["HDCASummary"]; export type HDCADetailed = components["schemas"]["HDCADetailed"]; +export type DCObject = components["schemas"]["DCObject"]; export type HistoryContentItemBase = components["schemas"]["EncodedHistoryContentItem"]; From e48a931f12a9ed1825c127e4b96642d240db601d Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 26 Sep 2023 10:17:41 -0400 Subject: [PATCH 43/50] Reset autocommit to False --- lib/galaxy/model/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/model/base.py b/lib/galaxy/model/base.py index d5326c785b20..48c90f5eea90 100644 --- a/lib/galaxy/model/base.py +++ b/lib/galaxy/model/base.py @@ -59,7 +59,7 @@ def transaction(session: Union[scoped_session, Session, "SessionlessContext"]): class ModelMapping(Bunch): def __init__(self, model_modules, engine): self.engine = engine - self._SessionLocal = sessionmaker(autoflush=False, autocommit=True) + self._SessionLocal = sessionmaker(autoflush=False, autocommit=False) versioned_session(self._SessionLocal) context = scoped_session(self._SessionLocal, scopefunc=self.request_scopefunc) # For backward compatibility with "context.current" From 77d10c1bf50171c04585df1e0be7cfd058430f94 Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 26 Sep 2023 13:23:53 -0400 Subject: [PATCH 44/50] Account for shared usage between TS and galaxy apps --- lib/galaxy/managers/api_keys.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/galaxy/managers/api_keys.py b/lib/galaxy/managers/api_keys.py index cf36c58fa1c1..dbc5121670a2 100644 --- a/lib/galaxy/managers/api_keys.py +++ b/lib/galaxy/managers/api_keys.py @@ -1,15 +1,9 @@ -from typing import Optional - from sqlalchemy import ( false, select, update, ) -from galaxy.model import ( - APIKeys, - User, -) from galaxy.model.base import transaction from galaxy.structured_app import BasicSharedApp @@ -19,11 +13,12 @@ def __init__(self, app: BasicSharedApp): self.app = app self.session = self.app.model.context - def get_api_key(self, user: User) -> Optional["APIKeys"]: + def get_api_key(self, user): + APIKeys = self.app.model.APIKeys stmt = select(APIKeys).filter_by(user_id=user.id, deleted=False).order_by(APIKeys.create_time.desc()).limit(1) return self.session.scalars(stmt).first() - def create_api_key(self, user: User) -> "APIKeys": + def create_api_key(self, user): guid = self.app.security.get_new_guid() new_key = self.app.model.APIKeys() new_key.user_id = user.id @@ -33,7 +28,7 @@ def create_api_key(self, user: User) -> "APIKeys": self.session.commit() return new_key - def get_or_create_api_key(self, user: User) -> str: + def get_or_create_api_key(self, user) -> str: # Logic Galaxy has always used - but it would appear to have a race # condition. Worth fixing? Would kind of need a message queue to fix # in multiple process mode. @@ -41,7 +36,7 @@ def get_or_create_api_key(self, user: User) -> str: key = api_key.key if api_key else self.create_api_key(user).key return key - def delete_api_key(self, user: User) -> None: + def delete_api_key(self, user) -> None: """Marks the current user API key as deleted.""" # Before it was possible to create multiple API keys for the same user although they were not considered valid # So all non-deleted keys are marked as deleted for backward compatibility @@ -50,6 +45,7 @@ def delete_api_key(self, user: User) -> None: self.session.commit() def _mark_all_api_keys_as_deleted(self, user_id: int): + APIKeys = self.app.model.APIKeys stmt = ( update(APIKeys) .where(APIKeys.user_id == user_id) From 6114bef9eec5c22f8519a0cd0255e29368c3c09e Mon Sep 17 00:00:00 2001 From: hujambo-dunia Date: Tue, 26 Sep 2023 15:19:34 -0400 Subject: [PATCH 45/50] In table list pages, removed bold-text from non-primary columns where bold-text already used in primary table [high priority] --- client/src/components/Workflow/InvocationsList.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/components/Workflow/InvocationsList.vue b/client/src/components/Workflow/InvocationsList.vue index ac6061a34458..5e0a9d70be83 100644 --- a/client/src/components/Workflow/InvocationsList.vue +++ b/client/src/components/Workflow/InvocationsList.vue @@ -60,7 +60,7 @@ :title="`Switch to
${getHistoryNameById(data.item.history_id)}`" class="truncate"> - {{ getHistoryNameById(data.item.history_id) }} + {{ getHistoryNameById(data.item.history_id) }} From b85970cf491e8048f8a9c10da00c0e20be8cf86c Mon Sep 17 00:00:00 2001 From: hujambo-dunia Date: Tue, 26 Sep 2023 15:32:07 -0400 Subject: [PATCH 46/50] In History panel, removed bold-text from Labels (e.g. enumerated steps) because bold-text already used in Label Details (e.g. filenames) [moderate priority] --- client/src/components/History/Content/ContentItem.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/src/components/History/Content/ContentItem.vue b/client/src/components/History/Content/ContentItem.vue index f92e4a6f4d9b..aaa646b4e054 100644 --- a/client/src/components/History/Content/ContentItem.vue +++ b/client/src/components/History/Content/ContentItem.vue @@ -9,7 +9,7 @@ @keydown="onKeyDown">
- + @@ -46,7 +46,7 @@ {{ id }}: - {{ name }} + {{ name }} From 5482727df6a97e48b1ad7e377c12b0d726c3bf48 Mon Sep 17 00:00:00 2001 From: hujambo-dunia Date: Tue, 26 Sep 2023 15:46:10 -0400 Subject: [PATCH 47/50] In table list pages, removed bold-text from primary columns where cells lacked secondary text requiring de-emphasis (i.e. no change made to Workflows List page) [low priority] --- client/src/components/Dataset/DatasetName.vue | 2 +- client/src/components/Workflow/InvocationsList.vue | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/src/components/Dataset/DatasetName.vue b/client/src/components/Dataset/DatasetName.vue index 29986ea127a3..be8e5382a9c3 100644 --- a/client/src/components/Dataset/DatasetName.vue +++ b/client/src/components/Dataset/DatasetName.vue @@ -2,7 +2,7 @@
From 4b2e2bed0bb655c583c1d34a0e517b5d4797499b Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 27 Sep 2023 10:33:08 +0200 Subject: [PATCH 48/50] Do not add None as extension in elements_datatypes None is not a valid extension and it seems to return this when you have a collection containing other sub-collections --- lib/galaxy/model/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 2b931668a18d..6621cf23115b 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -6689,7 +6689,8 @@ def dataset_dbkeys_and_extensions_summary(self): dbkeys.add(dbkey) else: dbkeys.add(dbkey_field) - extensions.add(row.extension) + if row.extension: + extensions.add(row.extension) self._dataset_dbkeys_and_extensions_summary = (dbkeys, extensions) return self._dataset_dbkeys_and_extensions_summary From 784200bb49025adf48b02bdb8bea4b5ece3b1e20 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Tue, 26 Sep 2023 18:22:57 +0100 Subject: [PATCH 49/50] Don't copy collection elements in ``test_dataset_collection_hide_originals`` Follow-up to https://github.com/galaxyproject/galaxy/pull/16717 . Minimal performance gain but helps document the parameter use after the change in default value to True. --- lib/galaxy_test/api/test_history_contents.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index 74be58604d08..cf5f76c52d5f 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -495,7 +495,7 @@ def test_jobs_summary_implicit_hdca(self, history_id): def test_dataset_collection_hide_originals(self, history_id): payload = self.dataset_collection_populator.create_pair_payload( - history_id, type="dataset_collection", direct_upload=False + history_id, type="dataset_collection", direct_upload=False, copy_elements=False ) payload["hide_source_items"] = True @@ -503,9 +503,7 @@ def test_dataset_collection_hide_originals(self, history_id): self.__check_create_collection_response(dataset_collection_response) contents_response = self._get(f"histories/{history_id}/contents") - datasets = [ - d for d in contents_response.json() if d["history_content_type"] == "dataset" and d["hid"] in [1, 2] - ] + datasets = [d for d in contents_response.json() if d["history_content_type"] == "dataset"] # Assert two datasets in source were hidden. assert len(datasets) == 2 assert not datasets[0]["visible"] From d26186f54860b9eca84cf7fc69aa2c0c25b650df Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 27 Sep 2023 10:34:25 +0100 Subject: [PATCH 50/50] Remove overriding of ``copy_elements`` parameter Prevent unexpected behaviour of the API. Actually raise `RequestParameterInvalidException` exception if this conflicts with `copy_required` (i.e. dbkey not None). --- lib/galaxy/webapps/galaxy/services/history_contents.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/services/history_contents.py b/lib/galaxy/webapps/galaxy/services/history_contents.py index 0e6408227674..f811c7b781dd 100644 --- a/lib/galaxy/webapps/galaxy/services/history_contents.py +++ b/lib/galaxy/webapps/galaxy/services/history_contents.py @@ -1269,9 +1269,9 @@ def __create_dataset_collection( raise exceptions.RequestParameterMissingException("'content' id of target to copy is missing") dbkey = payload.dbkey copy_required = dbkey is not None - copy_elements = payload.copy_elements or copy_required + copy_elements = payload.copy_elements if copy_required and not copy_elements: - raise exceptions.RequestParameterMissingException( + raise exceptions.RequestParameterInvalidException( "copy_elements passed as 'false' but it is required to change specified attributes" ) dataset_instance_attributes = {}