From 5cd46c3d5e2d9945dc56372403c298d3dd88ad79 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Sat, 16 Oct 2021 12:50:20 -0400 Subject: [PATCH] Various show page PDF bug fixes. - In FastAPI mode GET /api/pages/{id}.pdf was being masked by /api/pages/{id} and the tested 400 was an invalid ID error. - trans.response was being used in the service layer and was unavailable under FastAPI - moved setting response type back up into the controller. - API tests should skip if the Galaxy configuration is invalid - it wasn't doing this. While I was in there I also tweaked the exception handling a bit to indicate that this is not a USER FORBIDDEN (403) operation so much as an optional SERVER configuration NOT IMPLEMENTED (501). --- lib/galaxy/exceptions/__init__.py | 7 +++++++ lib/galaxy/exceptions/error_codes.json | 5 +++++ lib/galaxy/managers/configuration.py | 3 ++- lib/galaxy/managers/markdown_util.py | 10 ++++++--- lib/galaxy/managers/pages.py | 1 - lib/galaxy/webapps/galaxy/api/pages.py | 29 +++++++++++++------------- lib/galaxy_test/api/test_pages.py | 28 +++++++++++++++++-------- lib/galaxy_test/base/populators.py | 6 ++++++ 8 files changed, 61 insertions(+), 28 deletions(-) diff --git a/lib/galaxy/exceptions/__init__.py b/lib/galaxy/exceptions/__init__.py index 80b4a483774..5252e19b811 100644 --- a/lib/galaxy/exceptions/__init__.py +++ b/lib/galaxy/exceptions/__init__.py @@ -241,6 +241,13 @@ class ReferenceDataError(MessageException): err_code = error_codes_by_name['REFERENCE_DATA_ERROR'] +class ServerNotConfiguredForRequest(MessageException): + # A bit like ConfigDoesNotAllowException but it has nothing to do with the user of the + # request being "forbidden". It just isn't configured. + status_code = 501 + err_code = error_codes_by_name['SERVER_NOT_CONFIGURED_FOR_REQUEST'] + + # non-web exceptions class ContainerCLIError(Exception): diff --git a/lib/galaxy/exceptions/error_codes.json b/lib/galaxy/exceptions/error_codes.json index 981f444b5f8..959145046c9 100644 --- a/lib/galaxy/exceptions/error_codes.json +++ b/lib/galaxy/exceptions/error_codes.json @@ -173,5 +173,10 @@ "name": "NOT_IMPLEMENTED", "code": 501001, "message": "Method is not implemented." + }, + { + "name": "SERVER_NOT_CONFIGURED_FOR_REQUEST", + "code": 501002, + "message": "Server not configured for the request. The Galaxy admin may be able to resolve the problem by installing additional dependencies or setting up new infrastructure." } ] diff --git a/lib/galaxy/managers/configuration.py b/lib/galaxy/managers/configuration.py index 7c893ff20e4..831f87503d5 100644 --- a/lib/galaxy/managers/configuration.py +++ b/lib/galaxy/managers/configuration.py @@ -18,10 +18,10 @@ from typing import ( from galaxy.app import MinimalManagerApp from galaxy.managers import base from galaxy.managers.context import ProvidesUserContext +from galaxy.managers.markdown_util import weasyprint_available from galaxy.schema.fields import EncodedDatabaseIdField from galaxy.schema.types import SerializationParams from galaxy.web.framework.base import server_starttime - log = logging.getLogger(__name__) VERSION_JSON_FILE = 'version.json' @@ -167,6 +167,7 @@ class ConfigSerializer(base.ModelSerializer): 'ga_code': _use_config, 'plausible_server': _use_config, 'plausible_domain': _use_config, + 'markdown_to_pdf_available': lambda config, key, **context: weasyprint_available(), 'matomo_server': _use_config, 'matomo_site_id': _use_config, 'enable_unique_workflow_defaults': _use_config, diff --git a/lib/galaxy/managers/markdown_util.py b/lib/galaxy/managers/markdown_util.py index ae7e6927b80..d037e2c85e0 100644 --- a/lib/galaxy/managers/markdown_util.py +++ b/lib/galaxy/managers/markdown_util.py @@ -28,7 +28,7 @@ except Exception: from galaxy.exceptions import ( MalformedContents, - MessageException, + ServerNotConfiguredForRequest, ) from galaxy.managers.hdcas import HDCASerializer from galaxy.managers.jobs import ( @@ -595,10 +595,14 @@ def to_pdf(trans, basic_markdown, css_paths=None) -> bytes: shutil.rmtree(directory) +def weasyprint_available() -> bool: + return weasyprint is not None + + def _check_can_convert_to_pdf_or_raise(): """Checks if the HTML to PDF converter is available.""" - if not weasyprint: - raise MessageException("PDF conversion service not available.") + if not weasyprint_available(): + raise ServerNotConfiguredForRequest("PDF conversion service not available.") def internal_galaxy_markdown_to_pdf(trans, internal_galaxy_markdown, document_type) -> bytes: diff --git a/lib/galaxy/managers/pages.py b/lib/galaxy/managers/pages.py index e3988aa599b..982329e07a5 100644 --- a/lib/galaxy/managers/pages.py +++ b/lib/galaxy/managers/pages.py @@ -283,7 +283,6 @@ class PagesService: if page.latest_revision.content_format != PageContentFormat.markdown.value: raise exceptions.RequestParameterInvalidException("PDF export only allowed for Markdown based pages") internal_galaxy_markdown = page.latest_revision.content - trans.response.set_content_type("application/pdf") return internal_galaxy_markdown_to_pdf(trans, internal_galaxy_markdown, 'page') diff --git a/lib/galaxy/webapps/galaxy/api/pages.py b/lib/galaxy/webapps/galaxy/api/pages.py index dca536128f2..e04cde3ab52 100644 --- a/lib/galaxy/webapps/galaxy/api/pages.py +++ b/lib/galaxy/webapps/galaxy/api/pages.py @@ -101,19 +101,6 @@ class FastAPIPages: self.service.delete(trans, id) return Response(status_code=status.HTTP_204_NO_CONTENT) - @router.get( - '/api/pages/{id}', - summary="Return a page summary and the content of the last revision.", - response_description="The page summary information.", - ) - async def show( - self, - trans: ProvidesUserContext = DependsOnTrans, - id: EncodedDatabaseIdField = PageIdPathParam, - ) -> PageDetails: - """Return summary information about a specific Page and the content of the last revision.""" - return self.service.show(trans, id) - @router.get( '/api/pages/{id}.pdf', summary="Return a PDF document of the last revision of the Page.", @@ -123,7 +110,7 @@ class FastAPIPages: "description": "PDF document with the last revision of the page.", "content": {"application/pdf": {}}, }, - 400: { + 501: { "description": "PDF conversion service not available." } }, @@ -140,6 +127,19 @@ class FastAPIPages: pdf_bytes = self.service.show_pdf(trans, id) return StreamingResponse(io.BytesIO(pdf_bytes), media_type="application/pdf") + @router.get( + '/api/pages/{id}', + summary="Return a page summary and the content of the last revision.", + response_description="The page summary information.", + ) + async def show( + self, + trans: ProvidesUserContext = DependsOnTrans, + id: EncodedDatabaseIdField = PageIdPathParam, + ) -> PageDetails: + """Return summary information about a specific Page and the content of the last revision.""" + return self.service.show(trans, id) + @router.get( '/api/pages/{id}/sharing', summary="Get the current sharing status of the given Page.", @@ -309,6 +309,7 @@ class PagesController(BaseGalaxyAPIController): :rtype: dict :returns: Dictionary return of the Page.to_dict call with the 'content' field populated by the most recent revision """ + trans.response.set_content_type("application/pdf") return self.service.show_pdf(trans, id) @expose_api diff --git a/lib/galaxy_test/api/test_pages.py b/lib/galaxy_test/api/test_pages.py index 7b04017cf49..9e8f4d916a3 100644 --- a/lib/galaxy_test/api/test_pages.py +++ b/lib/galaxy_test/api/test_pages.py @@ -1,7 +1,10 @@ +from unittest import SkipTest + from requests import delete from galaxy.exceptions import error_codes from galaxy_test.api.sharable import SharingApiTests +from galaxy_test.base import api_asserts from galaxy_test.base.populators import DatasetPopulator, skip_without_tool, WorkflowPopulator from ._framework import ApiTestCase @@ -41,6 +44,11 @@ class PageApiTestCase(BasePageApiTestCase, SharingApiTests): api_name = "pages" + def setUp(self): + super().setUp() + self.dataset_populator = DatasetPopulator(self.galaxy_interactor) + self.workflow_populator = WorkflowPopulator(self.galaxy_interactor) + def create(self, name: str) -> str: response_json = self._create_valid_page_with_slug(name) return response_json["id"] @@ -51,15 +59,13 @@ class PageApiTestCase(BasePageApiTestCase, SharingApiTests): @skip_without_tool("cat") def test_create_from_report(self): - dataset_populator = DatasetPopulator(self.galaxy_interactor) - workflow_populator = WorkflowPopulator(self.galaxy_interactor) test_data = """ input_1: value: 1.bed type: File """ - with dataset_populator.test_history() as history_id: - summary = workflow_populator.run_workflow(""" + with self.dataset_populator.test_history() as history_id: + summary = self.workflow_populator.run_workflow(""" class: GalaxyWorkflow inputs: input_1: data @@ -75,7 +81,7 @@ steps: workflow_id = summary.workflow_id invocation_id = summary.invocation_id - report_json = workflow_populator.workflow_report_json(workflow_id, invocation_id) + report_json = self.workflow_populator.workflow_report_json(workflow_id, invocation_id) assert "markdown" in report_json self._assert_has_keys(report_json, "markdown", "render_format") assert report_json["render_format"] == "markdown" @@ -172,8 +178,7 @@ steps: self._assert_error_code_is(page_response, error_codes.MALFORMED_ID) def test_400_on_invalid_embedded_content(self): - dataset_populator = DatasetPopulator(self.galaxy_interactor) - valid_id = dataset_populator.new_history() + valid_id = self.dataset_populator.new_history() page_request = self._test_page_payload(slug="invalid-embed-content") page_request["content"] = f'''

Page!

''' page_response = self._post("pages", page_request, json=True) @@ -205,13 +210,18 @@ steps: self._assert_status_code_is(show_response, 403) self._assert_error_code_is(show_response, error_codes.USER_CANNOT_ACCESS_ITEM) - def test_400_on_download_pdf_when_service_unavailable(self): + def test_501_on_download_pdf_when_service_unavailable(self): + configuration = self.dataset_populator.get_configuration() + can_produce_markdown = configuration["markdown_to_pdf_available"] + if can_produce_markdown: + raise SkipTest("Skipping test because server does implement markdown conversion to PDF") page_request = self._test_page_payload(slug="md-page-to-pdf", content_format="markdown") page_response = self._post("pages", page_request, json=True) self._assert_status_code_is(page_response, 200) page_id = page_response.json()['id'] pdf_response = self._get(f"pages/{page_id}.pdf") - self._assert_status_code_is(pdf_response, 400) + api_asserts.assert_status_code_is(pdf_response, 501) + api_asserts.assert_error_code_is(pdf_response, error_codes.error_codes_by_name["SERVER_NOT_CONFIGURED_FOR_REQUEST"]) def test_400_on_download_pdf_when_unsupported_content_format(self): page_request = self._test_page_payload(slug="html-page-to-pdf", content_format="html") diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index cb360e2454d..8c35974af5d 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -586,6 +586,12 @@ class BaseDatasetPopulator(BasePopulator): assert roles_response.status_code == 200 return roles_response.json() + def get_configuration(self, admin=False) -> Dict[str, Any]: + response = self._get("configuration", admin=admin) + api_asserts.assert_status_code_is_ok(response) + configuration = response.json() + return configuration + def user_email(self) -> str: users_response = self._get("users") users = users_response.json()