5 new commits in galaxy-central: https://bitbucket.org/galaxy/galaxy-central/commits/3cafaba9aaf2/ Changeset: 3cafaba9aaf2 User: jmchilton Date: 2014-01-10 22:10:52 Summary: Pull extract_payload_from_request out of expose_api. Needs to be used in new version of API decorator and resulted in too much nesting in there the way it was. Affected #: 1 file diff -r 4ab2017348e25958bcd8babd8e110f574dff24df -r 3cafaba9aaf288d0abf257032db225fc52ca7287 lib/galaxy/web/framework/__init__.py --- a/lib/galaxy/web/framework/__init__.py +++ b/lib/galaxy/web/framework/__init__.py @@ -103,6 +103,35 @@ return decorator return argcatcher + +def __extract_payload_from_request(trans, func, kwargs): + content_type = trans.request.headers['content-type'] + if content_type.startswith('application/x-www-form-urlencoded') or content_type.startswith('multipart/form-data'): + # If the content type is a standard type such as multipart/form-data, the wsgi framework parses the request body + # and loads all field values into kwargs. However, kwargs also contains formal method parameters etc. which + # are not a part of the request body. This is a problem because it's not possible to differentiate between values + # which are a part of the request body, and therefore should be a part of the payload, and values which should not be + # in the payload. Therefore, the decorated method's formal arguments are discovered through reflection and removed from + # the payload dictionary. This helps to prevent duplicate argument conflicts in downstream methods. + payload = kwargs.copy() + named_args, _, _, _ = inspect.getargspec(func) + for arg in named_args: + payload.pop(arg, None) + for k, v in payload.iteritems(): + if isinstance(v, (str, unicode)): + try: + payload[k] = from_json_string(v) + except: + # may not actually be json, just continue + pass + payload = util.recursively_stringify_dictionary_keys( payload ) + else: + # Assume application/json content type and parse request body manually, since wsgi won't do it. However, the order of this check + # should ideally be in reverse, with the if clause being a check for application/json and the else clause assuming a standard encoding + # such as multipart/form-data. Leaving it as is for backward compatibility, just in case. + payload = util.recursively_stringify_dictionary_keys( from_json_string( trans.request.body ) ) + return payload + def expose_api_raw( func ): """ Expose this function via the API but don't dump the results @@ -140,35 +169,8 @@ error_message = "API Authentication Required for this request" return error if trans.request.body: - def extract_payload_from_request(trans, func, kwargs): - content_type = trans.request.headers['content-type'] - if content_type.startswith('application/x-www-form-urlencoded') or content_type.startswith('multipart/form-data'): - # If the content type is a standard type such as multipart/form-data, the wsgi framework parses the request body - # and loads all field values into kwargs. However, kwargs also contains formal method parameters etc. which - # are not a part of the request body. This is a problem because it's not possible to differentiate between values - # which are a part of the request body, and therefore should be a part of the payload, and values which should not be - # in the payload. Therefore, the decorated method's formal arguments are discovered through reflection and removed from - # the payload dictionary. This helps to prevent duplicate argument conflicts in downstream methods. - payload = kwargs.copy() - named_args, _, _, _ = inspect.getargspec(func) - for arg in named_args: - payload.pop(arg, None) - for k, v in payload.iteritems(): - if isinstance(v, (str, unicode)): - try: - payload[k] = from_json_string(v) - except: - # may not actually be json, just continue - pass - payload = util.recursively_stringify_dictionary_keys( payload ) - else: - # Assume application/json content type and parse request body manually, since wsgi won't do it. However, the order of this check - # should ideally be in reverse, with the if clause being a check for application/json and the else clause assuming a standard encoding - # such as multipart/form-data. Leaving it as is for backward compatibility, just in case. - payload = util.recursively_stringify_dictionary_keys( from_json_string( trans.request.body ) ) - return payload try: - kwargs['payload'] = extract_payload_from_request(trans, func, kwargs) + kwargs['payload'] = __extract_payload_from_request(trans, func, kwargs) except ValueError: error_status = '400 Bad Request' error_message = 'Your request did not appear to be valid JSON, please consult the API documentation' https://bitbucket.org/galaxy/galaxy-central/commits/39b2ae68b153/ Changeset: 39b2ae68b153 User: jmchilton Date: 2014-01-10 22:10:52 Summary: Introduce new generation of API decorator... Add improved error handling. Introduce error code. Use new decorator with tested methods in histories API. Affected #: 5 files diff -r 3cafaba9aaf288d0abf257032db225fc52ca7287 -r 39b2ae68b153ce701e821b0f4609c8ba8f0f5ad5 lib/galaxy/exceptions/__init__.py --- a/lib/galaxy/exceptions/__init__.py +++ b/lib/galaxy/exceptions/__init__.py @@ -6,33 +6,66 @@ eggs.require( "Paste" ) from paste import httpexceptions +from ..exceptions import error_codes + class MessageException( Exception ): """ - Exception to make throwing errors from deep in controllers easier + Exception to make throwing errors from deep in controllers easier. """ - def __init__( self, err_msg, type="info" ): - self.err_msg = err_msg + # status code to be set when used with API. + status_code = 400 + # Error code information embedded into API json responses. + err_code = error_codes.UNKNOWN + + def __init__( self, err_msg=None, type="info", **extra_error_info ): + self.err_msg = err_msg or self.err_code.default_error_message self.type = type + self.extra_error_info = extra_error_info + def __str__( self ): return self.err_msg + class ItemDeletionException( MessageException ): pass + class ItemAccessibilityException( MessageException ): - pass + status_code = 403 + err_code = error_codes.USER_CANNOT_ACCESS_ITEM + class ItemOwnershipException( MessageException ): - pass + status_code = 403 + err_code = error_codes.USER_DOES_NOT_OWN_ITEM + + +class DuplicatedSlugException( MessageException ): + status_code = 400 + err_code = error_codes.USER_SLUG_DUPLICATE + + +class ObjectAttributeInvalidException( MessageException ): + status_code = 400 + err_code = error_codes.USER_OBJECT_ATTRIBUTE_INVALID + + +class ObjectAttributeMissingException( MessageException ): + status_code = 400 + err_code = error_codes.USER_OBJECT_ATTRIBUTE_MISSING + class ActionInputError( MessageException ): def __init__( self, err_msg, type="error" ): super( ActionInputError, self ).__init__( err_msg, type ) -class ObjectNotFound( Exception ): + +class ObjectNotFound( MessageException ): """ Accessed object was not found """ - pass + status_code = 404 + err_code = error_codes.USER_OBJECT_NOT_FOUND + class ObjectInvalid( Exception ): """ Accessed object store ID is invalid """ diff -r 3cafaba9aaf288d0abf257032db225fc52ca7287 -r 39b2ae68b153ce701e821b0f4609c8ba8f0f5ad5 lib/galaxy/exceptions/error_codes.py --- /dev/null +++ b/lib/galaxy/exceptions/error_codes.py @@ -0,0 +1,33 @@ +# Error codes are provided as a convience to Galaxy API clients, but at this +# time they do represent part of the more stable interface. They can change +# without warning between releases. +UNKNOWN_ERROR_MESSAGE = "Unknown error occurred while processing request." + + +class ErrorCode( object ): + + def __init__( self, code, default_error_message ): + self.code = code + self.default_error_message = default_error_message or UNKNOWN_ERROR_MESSAGE + + def __str__( self ): + return str( self.default_error_message ) + + def __int__( self ): + return int( self.code ) + +# TODO: Guidelines for error message langauge? +UNKNOWN = ErrorCode(0, UNKNOWN_ERROR_MESSAGE) + +USER_CANNOT_RUN_AS = ErrorCode(400001, "User does not have permissions to run jobs as another user.") +USER_INVALID_RUN_AS = ErrorCode(400002, "Invalid run_as request - run_as user does not exist.") +USER_INVALID_JSON = ErrorCode(400003, "Your request did not appear to be valid JSON, please consult the API documentation.") +USER_OBJECT_ATTRIBUTE_INVALID = ErrorCode(400004, "Attempted to create or update object with invalid attribute value.") +USER_OBJECT_ATTRIBUTE_MISSING = ErrorCode(400005, "Attempted to create object without required attribute.") +USER_SLUG_DUPLICATE = ErrorCode(400006, "Slug must be unique per user.") + +USER_NO_API_KEY = ErrorCode(403001, "API Authentication Required for this request") +USER_CANNOT_ACCESS_ITEM = ErrorCode(403002, "User cannot access specified item.") +USER_DOES_NOT_OWN_ITEM = ErrorCode(403003, "User does not own specified item.") + +USER_OBJECT_NOT_FOUND = ErrorCode(404001, "No such object not found.") diff -r 3cafaba9aaf288d0abf257032db225fc52ca7287 -r 39b2ae68b153ce701e821b0f4609c8ba8f0f5ad5 lib/galaxy/web/__init__.py --- a/lib/galaxy/web/__init__.py +++ b/lib/galaxy/web/__init__.py @@ -15,3 +15,7 @@ from framework import expose_api_raw from framework import expose_api_raw_anonymous from framework.base import httpexceptions + +# TODO: Drop and make these the default. +from framework import _future_expose_api +from framework import _future_expose_api_anonymous diff -r 3cafaba9aaf288d0abf257032db225fc52ca7287 -r 39b2ae68b153ce701e821b0f4609c8ba8f0f5ad5 lib/galaxy/web/framework/__init__.py --- a/lib/galaxy/web/framework/__init__.py +++ b/lib/galaxy/web/framework/__init__.py @@ -10,9 +10,9 @@ import socket import string import time - +from traceback import format_exc +from Cookie import CookieError from functools import wraps -from Cookie import CookieError pkg_resources.require( "Cheetah" ) from Cheetah.Template import Template @@ -23,6 +23,7 @@ from galaxy import util from galaxy.exceptions import MessageException +from galaxy.exceptions import error_codes from galaxy.util import asbool from galaxy.util import safe_str_cmp from galaxy.util.backports.importlib import import_module @@ -212,6 +213,143 @@ decorator.exposed = True return decorator +API_RESPONSE_CONTENT_TYPE = "application/json" + + +def __api_error_message( trans, **kwds ): + exception = kwds.get( "exception", None ) + if exception: + # If we are passed a MessageException use err_msg. + default_error_code = getattr( exception, "err_code", error_codes.UNKNOWN ) + default_error_message = getattr( exception, "err_msg", default_error_code.default_error_message ) + extra_error_info = getattr( exception, 'extra_error_info', {} ) + if not isinstance( extra_error_info, dict ): + extra_error_info = {} + else: + default_error_message = "Error processing API request." + default_error_code = error_codes.UNKNOWN + extra_error_info = {} + traceback_string = kwds.get( "traceback", "No traceback available." ) + err_msg = kwds.get( "err_msg", default_error_message ) + error_code_object = kwds.get( "err_code", default_error_code ) + try: + error_code = error_code_object.code + except AttributeError: + # Some sort of bad error code sent in, logic failure on part of + # Galaxy developer. + error_code = error_codes.UNKNOWN.code + # Would prefer the terminology of error_code and error_message, but + # err_msg used a good number of places already. Might as well not change + # it? + error_response = dict( err_msg=err_msg, err_code=error_code, **extra_error_info ) + if trans.debug: # TODO: Should admins get to see traceback as well? + error_response[ "traceback" ] = traceback_string + return error_response + + +def __api_error_response( trans, **kwds ): + error_dict = __api_error_message( trans, **kwds ) + exception = kwds.get( "exception", None ) + # If we are given an status code directly - use it - otherwise check + # the exception for a status_code attribute. + if "status_code" in kwds: + status_code = int( kwds.get( "status_code" ) ) + elif hasattr( exception, "status_code" ): + status_code = int( exception.status_code ) + else: + status_code = 500 + response = trans.response + if not response.status or str(response.status).startswith("20"): + # Unset status code appears to be string '200 OK', if anything + # non-success (i.e. not 200 or 201) has been set, do not override + # underlying controller. + response.status = status_code + return to_json_string( error_dict ) + + +# TODO: rename as expose_api and make default. +def _future_expose_api_anonymous( func, to_json=True ): + """ + Expose this function via the API but don't require a set user. + """ + return _future_expose_api( func, to_json=to_json, user_required=False ) + + +# TODO: rename as expose_api and make default. +def _future_expose_api( func, to_json=True, user_required=True ): + """ + Expose this function via the API. + """ + @wraps(func) + def decorator( self, trans, *args, **kwargs ): + if trans.error_message: + # TODO: Document this branch, when can this happen, + # I don't understand it. + return __api_error_response( trans, err_msg=trans.error_message ) + if user_required and trans.anonymous: + error_code = error_codes.USER_NO_API_KEY + # Use error codes default error message. + return __api_error_response( trans, err_code=error_code, status_code=403 ) + if trans.request.body: + try: + kwargs['payload'] = __extract_payload_from_request(trans, func, kwargs) + except ValueError: + error_code = error_codes.USER_INVALID_JSON + return __api_error_response( trans, status_code=400, err_code=error_code ) + + trans.response.set_content_type( API_RESPONSE_CONTENT_TYPE ) + # send 'do not cache' headers to handle IE's caching of ajax get responses + trans.response.headers[ 'Cache-Control' ] = "max-age=0,no-cache,no-store" + # TODO: Refactor next block out into a helper procedure. + # Perform api_run_as processing, possibly changing identity + if 'payload' in kwargs and 'run_as' in kwargs['payload']: + if not trans.user_can_do_run_as(): + error_code = error_codes.USER_CANNOT_RUN_AS + return __api_error_response( trans, err_code=error_code, status_code=403 ) + try: + decoded_user_id = trans.security.decode_id( kwargs['payload']['run_as'] ) + except TypeError: + error_message = "Malformed user id ( %s ) specified, unable to decode." % str( kwargs['payload']['run_as'] ) + error_code = error_codes.USER_INVALID_RUN_AS + return __api_error_response( trans, err_code=error_code, err_msg=error_message, status_code=400) + try: + user = trans.sa_session.query( trans.app.model.User ).get( decoded_user_id ) + trans.api_inherit_admin = trans.user_is_admin() + trans.set_user(user) + except: + error_code = error_codes.USER_INVALID_RUN_AS + return __api_error_response( trans, err_code=error_code, status_code=400 ) + try: + rval = func( self, trans, *args, **kwargs) + if to_json and trans.debug: + rval = to_json_string( rval, indent=4, sort_keys=True ) + elif to_json: + rval = to_json_string( rval ) + return rval + except MessageException as e: + traceback_string = format_exc() + return __api_error_response( trans, exception=e, traceback=traceback_string ) + except paste.httpexceptions.HTTPException: + # TODO: Allow to pass or format for the API??? + raise # handled + except Exception as e: + traceback_string = format_exc() + error_message = 'Uncaught exception in exposed API method:' + log.exception( error_message ) + return __api_error_response( + trans, + status_code=500, + exception=e, + traceback=traceback_string, + err_msg=error_message, + err_code=error_codes.UNKNOWN + ) + if not hasattr(func, '_orig'): + decorator._orig = func + decorator.exposed = True + return decorator + + def require_admin( func ): @wraps(func) def decorator( self, trans, *args, **kwargs ): diff -r 3cafaba9aaf288d0abf257032db225fc52ca7287 -r 39b2ae68b153ce701e821b0f4609c8ba8f0f5ad5 lib/galaxy/webapps/galaxy/api/histories.py --- a/lib/galaxy/webapps/galaxy/api/histories.py +++ b/lib/galaxy/webapps/galaxy/api/histories.py @@ -9,6 +9,8 @@ from paste.httpexceptions import HTTPBadRequest, HTTPForbidden, HTTPInternalServerError, HTTPException from galaxy import web +from galaxy.web import _future_expose_api as expose_api +from galaxy.web import _future_expose_api_anonymous as expose_api_anonymous from galaxy.util import string_as_bool, restore_text from galaxy.util.sanitize_html import sanitize_html from galaxy.web.base.controller import BaseAPIController, UsesHistoryMixin, UsesTagsMixin @@ -20,7 +22,7 @@ class HistoriesController( BaseAPIController, UsesHistoryMixin, UsesTagsMixin ): - @web.expose_api_anonymous + @expose_api_anonymous def index( self, trans, deleted='False', **kwd ): """ index( trans, deleted='False' ) @@ -152,7 +154,7 @@ return history_data - @web.expose_api + @expose_api def create( self, trans, payload, **kwd ): """ create( trans, payload ) https://bitbucket.org/galaxy/galaxy-central/commits/b41d51baaae1/ Changeset: b41d51baaae1 User: jmchilton Date: 2014-01-10 22:10:52 Summary: Add functional tests for pages API. Touch up pages API in response to test cases. Update pages API to use newer API decorator. Affected #: 5 files diff -r 39b2ae68b153ce701e821b0f4609c8ba8f0f5ad5 -r b41d51baaae16c4d6e7c4fb20a28cf9aeb5f1f43 lib/galaxy/webapps/galaxy/api/page_revisions.py --- a/lib/galaxy/webapps/galaxy/api/page_revisions.py +++ b/lib/galaxy/webapps/galaxy/api/page_revisions.py @@ -2,8 +2,9 @@ API for updating Galaxy Pages """ import logging -from galaxy import web +from galaxy.web import _future_expose_api as expose_api from galaxy.web.base.controller import SharableItemSecurityMixin, BaseAPIController, SharableMixin +from galaxy import exceptions from galaxy.model.item_attrs import UsesAnnotations from galaxy.util.sanitize_html import sanitize_html @@ -12,7 +13,7 @@ class PageRevisionsController( BaseAPIController, SharableItemSecurityMixin, UsesAnnotations, SharableMixin ): - @web.expose_api + @expose_api def index( self, trans, page_id, **kwd ): """ index( self, trans, page_id, **kwd ) @@ -24,14 +25,16 @@ :rtype: list :returns: dictionaries containing different revisions of the page """ + page = self._get_page( trans, page_id ) + self._verify_page_ownership( trans, page ) + r = trans.sa_session.query( trans.app.model.PageRevision ).filter_by( page_id=trans.security.decode_id(page_id) ) out = [] for page in r: - if self.security_check( trans, page, True, True ): - out.append( self.encode_all_ids( trans, page.to_dict(), True) ) + out.append( self.encode_all_ids( trans, page.to_dict(), True) ) return out - @web.expose_api + @expose_api def create( self, trans, page_id, payload, **kwd ): """ create( self, trans, page_id, payload **kwd ) @@ -46,39 +49,42 @@ :rtype: dictionary :returns: Dictionary with 'success' or 'error' element to indicate the result of the request """ - error_str = "" + content = payload.get("content", None) + if not content: + raise exceptions.ObjectAttributeMissingException("content undefined or empty") - if not page_id: - error_str = "page_id is required" - elif not payload.get("content", None): - error_str = "content is required" + page = self._get_page( trans, page_id ) + self._verify_page_ownership( trans, page ) + + if 'title' in payload: + title = payload['title'] else: + title = page.title - # Create the new stored page + content = sanitize_html( content, 'utf-8', 'text/html' ) + + page_revision = trans.app.model.PageRevision() + page_revision.title = title + page_revision.page = page + page.latest_revision = page_revision + page_revision.content = content + + # Persist + session = trans.sa_session + session.flush() + + return page_revision.to_dict( view="element" ) + + def _get_page( self, trans, page_id ): + page = None + try: page = trans.sa_session.query( trans.app.model.Page ).get( trans.security.decode_id(page_id) ) - if page is None: - return { "error" : "page not found"} + except Exception: + pass + if not page: + raise exceptions.ObjectNotFound() + return page - if not self.security_check( trans, page, True, True ): - return { "error" : "page not found"} - - if 'title' in payload: - title = payload['title'] - else: - title = page.title - - content = payload.get("content", "") - content = sanitize_html( content, 'utf-8', 'text/html' ) - - page_revision = trans.app.model.PageRevision() - page_revision.title = title - page_revision.page = page - page.latest_revision = page_revision - page_revision.content = content - # Persist - session = trans.sa_session - session.flush() - - return { "success" : "revision posted" } - - return { "error" : error_str } + def _verify_page_ownership( self, trans, page ): + if not self.security_check( trans, page, True, True ): + raise exceptions.ItemOwnershipException() diff -r 39b2ae68b153ce701e821b0f4609c8ba8f0f5ad5 -r b41d51baaae16c4d6e7c4fb20a28cf9aeb5f1f43 lib/galaxy/webapps/galaxy/api/pages.py --- a/lib/galaxy/webapps/galaxy/api/pages.py +++ b/lib/galaxy/webapps/galaxy/api/pages.py @@ -2,8 +2,9 @@ API for updating Galaxy Pages """ import logging -from galaxy import web +from galaxy.web import _future_expose_api as expose_api from galaxy.web.base.controller import SharableItemSecurityMixin, BaseAPIController, SharableMixin +from galaxy import exceptions from galaxy.model.item_attrs import UsesAnnotations from galaxy.util.sanitize_html import sanitize_html @@ -12,7 +13,7 @@ class PagesController( BaseAPIController, SharableItemSecurityMixin, UsesAnnotations, SharableMixin ): - @web.expose_api + @expose_api def index( self, trans, deleted=False, **kwd ): """ index( self, trans, deleted=False, **kwd ) @@ -47,7 +48,7 @@ return out - @web.expose_api + @expose_api def create( self, trans, payload, **kwd ): """ create( self, trans, payload, **kwd ) @@ -64,45 +65,41 @@ :returns: Dictionary return of the Page.to_dict call """ user = trans.get_user() - error_str = "" if not payload.get("title", None): - error_str = "Page name is required" + raise exceptions.ObjectAttributeMissingException( "Page name is required" ) elif not payload.get("slug", None): - error_str = "Page id is required" + raise exceptions.ObjectAttributeMissingException( "Page id is required" ) elif not self._is_valid_slug( payload["slug"] ): - error_str = "Page identifier must consist of only lowercase letters, numbers, and the '-' character" + raise exceptions.ObjectAttributeInvalidException( "Page identifier must consist of only lowercase letters, numbers, and the '-' character" ) elif trans.sa_session.query( trans.app.model.Page ).filter_by( user=user, slug=payload["slug"], deleted=False ).first(): - error_str = "Page id must be unique" - else: + raise exceptions.DuplicatedSlugException( "Page slug must be unique" ) - content = payload.get("content", "") - content = sanitize_html( content, 'utf-8', 'text/html' ) + content = payload.get("content", "") + content = sanitize_html( content, 'utf-8', 'text/html' ) - # Create the new stored page - page = trans.app.model.Page() - page.title = payload['title'] - page.slug = payload['slug'] - page_annotation = sanitize_html( payload.get( "annotation", "" ), 'utf-8', 'text/html' ) - self.add_item_annotation( trans.sa_session, trans.get_user(), page, page_annotation ) - page.user = user - # And the first (empty) page revision - page_revision = trans.app.model.PageRevision() - page_revision.title = payload['title'] - page_revision.page = page - page.latest_revision = page_revision - page_revision.content = content - # Persist - session = trans.sa_session - session.add( page ) - session.flush() + # Create the new stored page + page = trans.app.model.Page() + page.title = payload['title'] + page.slug = payload['slug'] + page_annotation = sanitize_html( payload.get( "annotation", "" ), 'utf-8', 'text/html' ) + self.add_item_annotation( trans.sa_session, trans.get_user(), page, page_annotation ) + page.user = user + # And the first (empty) page revision + page_revision = trans.app.model.PageRevision() + page_revision.title = payload['title'] + page_revision.page = page + page.latest_revision = page_revision + page_revision.content = content + # Persist + session = trans.sa_session + session.add( page ) + session.flush() - rval = self.encode_all_ids( trans, page.to_dict(), True ) - return rval + rval = self.encode_all_ids( trans, page.to_dict(), True ) + return rval - return { "error" : error_str } - - @web.expose_api + @expose_api def delete( self, trans, id, **kwd ): """ delete( self, trans, id, **kwd ) @@ -114,22 +111,14 @@ :rtype: dict :returns: Dictionary with 'success' or 'error' element to indicate the result of the request """ - page_id = id - try: - page = trans.sa_session.query(self.app.model.Page).get(trans.security.decode_id(page_id)) - except Exception, e: - return { "error" : "Page with ID='%s' can not be found\n Exception: %s" % (page_id, str( e )) } + page = self._get_page( trans, id ) - # check to see if user has permissions to selected workflow - if page.user != trans.user and not trans.user_is_admin(): - return { "error" : "Workflow is not owned by or shared with current user" } - - #Mark a workflow as deleted + #Mark a page as deleted page.deleted = True trans.sa_session.flush() - return { "success" : "Deleted", "id" : page_id } + return '' # TODO: Figure out what to return on DELETE, document in guidelines! - @web.expose_api + @expose_api def show( self, trans, id, **kwd ): """ show( self, trans, id, **kwd ) @@ -141,8 +130,22 @@ :rtype: dict :returns: Dictionary return of the Page.to_dict call with the 'content' field populated by the most recent revision """ - page = trans.sa_session.query( trans.app.model.Page ).get( trans.security.decode_id( id ) ) + page = self._get_page( trans, id ) self.security_check( trans, page, check_ownership=False, check_accessible=True) rval = self.encode_all_ids( trans, page.to_dict(), True ) rval['content'] = page.latest_revision.content return rval + + def _get_page( self, trans, id ): # Fetches page object and verifies security. + try: + page = trans.sa_session.query( trans.app.model.Page ).get( trans.security.decode_id( id ) ) + except Exception: + page = None + + if not page: + raise exceptions.ObjectNotFound() + + if page.user != trans.user and not trans.user_is_admin(): + raise exceptions.ItemOwnershipException() + + return page diff -r 39b2ae68b153ce701e821b0f4609c8ba8f0f5ad5 -r b41d51baaae16c4d6e7c4fb20a28cf9aeb5f1f43 test/base/api.py --- a/test/base/api.py +++ b/test/base/api.py @@ -60,6 +60,12 @@ for key in keys: assert key in response, "Response [%s] does not contain key [%s]" % ( response, key ) + def _assert_error_code_is( self, response, error_code ): + if hasattr( response, "json" ): + response = response.json() + self._assert_has_keys( response, "err_code" ) + self.assertEquals( response[ "err_code" ], int( error_code ) ) + def _random_key( self ): # Used for invalid request testing... return "1234567890123456" diff -r 39b2ae68b153ce701e821b0f4609c8ba8f0f5ad5 -r b41d51baaae16c4d6e7c4fb20a28cf9aeb5f1f43 test/functional/api/test_page_revisions.py --- /dev/null +++ b/test/functional/api/test_page_revisions.py @@ -0,0 +1,35 @@ +from galaxy.exceptions import error_codes +from functional.api.pages import BasePageApiTestCase + + +class PageRevisionsApiTestCase( BasePageApiTestCase ): + + def test_create( self ): + page_json = self._create_valid_page_with_slug( "pr1" ) + revision_data = dict( content="<p>NewContent!</p>" ) + page_revision_response = self._post( "pages/%s/revisions" % page_json[ 'id' ], data=revision_data ) + self._assert_status_code_is( page_revision_response, 200 ) + page_revision_json = page_revision_response.json() + self._assert_has_keys( page_revision_json, 'id', 'content' ) + + def test_403_if_create_revision_on_unowned_page( self ): + page_json = self._create_valid_page_as( "pr2@bx.psu.edu", "pr2" ) + revision_data = dict( content="<p>NewContent!</p>" ) + page_revision_response = self._post( "pages/%s/revisions" % page_json[ 'id' ], data=revision_data ) + self._assert_status_code_is( page_revision_response, 403 ) + + def test_revision_index( self ): + page_json = self._create_valid_page_with_slug( "pr3" ) + revision_data = dict( content="<p>NewContent!</p>" ) + revisions_url = "pages/%s/revisions" % page_json[ 'id' ] + self._post( revisions_url, data=revision_data ) + revisions_response = self._get( revisions_url ) + self._assert_status_code_is( revisions_response, 200 ) + revisions_json = revisions_response.json() + assert len( revisions_json ) == 2 # Original revision and new one + + def test_404_if_index_unknown_page( self ): + revisions_url = "pages/%s/revisions" % self._random_key() + revisions_response = self._get( revisions_url ) + self._assert_status_code_is( revisions_response, 404 ) + self._assert_error_code_is( revisions_response, error_codes.USER_OBJECT_NOT_FOUND ) diff -r 39b2ae68b153ce701e821b0f4609c8ba8f0f5ad5 -r b41d51baaae16c4d6e7c4fb20a28cf9aeb5f1f43 test/functional/api/test_pages.py --- /dev/null +++ b/test/functional/api/test_pages.py @@ -0,0 +1,105 @@ +from galaxy.exceptions import error_codes +from base import api +from base.interactor import delete_request + +from operator import itemgetter + + +class BasePageApiTestCase( api.ApiTestCase ): + + def _create_valid_page_with_slug( self, slug ): + page_request = self._test_page_payload( slug=slug ) + page_response = self._post( "pages", page_request ) + self._assert_status_code_is( page_response, 200 ) + return page_response.json() + + def _create_valid_page_as( self, other_email, slug ): + run_as_user = self._setup_user( other_email ) + page_request = self._test_page_payload( slug=slug ) + page_request[ "run_as" ] = run_as_user[ "id" ] + page_response = self._post( "pages", page_request, admin=True ) + self._assert_status_code_is( page_response, 200 ) + return page_response.json() + + def _test_page_payload( self, **kwds ): + request = dict( + slug="mypage", + title="MY PAGE", + content="<p>Page!</p>", + ) + request.update( **kwds ) + return request + + +class PageApiTestCase( BasePageApiTestCase ): + + def test_create( self ): + response_json = self._create_valid_page_with_slug( "mypage" ) + self._assert_has_keys( response_json, "slug", "title", "id" ) + + def test_index( self ): + create_response_json = self._create_valid_page_with_slug( "indexpage" ) + assert self._users_index_has_page_with_id( create_response_json[ "id" ] ) + + def test_index_doesnt_show_unavailable_pages( self ): + create_response_json = self._create_valid_page_as( "others_page_index@bx.psu.edu", "otherspageindex" ) + assert not self._users_index_has_page_with_id( create_response_json[ "id" ] ) + + def test_cannot_create_pages_with_same_slug( self ): + page_request = self._test_page_payload( slug="mypage1" ) + page_response_1 = self._post( "pages", page_request ) + self._assert_status_code_is( page_response_1, 200 ) + page_response_2 = self._post( "pages", page_request ) + self._assert_status_code_is( page_response_2, 400 ) + self._assert_error_code_is( page_response_2, error_codes.USER_SLUG_DUPLICATE ) + + def test_page_requires_name( self ): + page_request = self._test_page_payload() + del page_request[ 'title' ] + page_response = self._post( "pages", page_request ) + self._assert_status_code_is( page_response, 400 ) + self._assert_error_code_is( page_response, error_codes.USER_OBJECT_ATTRIBUTE_MISSING ) + + def test_page_requires_slug( self ): + page_request = self._test_page_payload() + del page_request[ 'slug' ] + page_response = self._post( "pages", page_request ) + self._assert_status_code_is( page_response, 400 ) + + def test_delete( self ): + response_json = self._create_valid_page_with_slug( "testdelete" ) + delete_response = delete_request( self._api_url( "pages/%s" % response_json[ 'id' ], use_key=True ) ) + self._assert_status_code_is( delete_response, 200 ) + + def test_404_on_delete_unknown_page( self ): + delete_response = delete_request( self._api_url( "pages/%s" % self._random_key(), use_key=True ) ) + self._assert_status_code_is( delete_response, 404 ) + self._assert_error_code_is( delete_response, error_codes.USER_OBJECT_NOT_FOUND ) + + def test_403_on_delete_unowned_page( self ): + page_response = self._create_valid_page_as( "others_page@bx.psu.edu", "otherspage" ) + delete_response = delete_request( self._api_url( "pages/%s" % page_response[ "id" ], use_key=True ) ) + self._assert_status_code_is( delete_response, 403 ) + self._assert_error_code_is( delete_response, error_codes.USER_DOES_NOT_OWN_ITEM ) + + def test_show( self ): + response_json = self._create_valid_page_with_slug( "pagetoshow" ) + show_response = self._get( "pages/%s" % response_json['id'] ) + self._assert_status_code_is( show_response, 200 ) + show_json = show_response.json() + self._assert_has_keys( show_json, "slug", "title", "id" ) + self.assertEquals( show_json["slug"], "pagetoshow" ) + self.assertEquals( show_json["title"], "MY PAGE" ) + self.assertEquals( show_json["content"], "<p>Page!</p>" ) + + def test_403_on_unowner_show( self ): + response_json = self._create_valid_page_as( "others_page_show@bx.psu.edu", "otherspageshow" ) + show_response = self._get( "pages/%s" % response_json['id'] ) + self._assert_status_code_is( show_response, 403 ) + self._assert_error_code_is( show_response, error_codes.USER_DOES_NOT_OWN_ITEM ) + + def _users_index_has_page_with_id( self, id ): + index_response = self._get( "pages" ) + self._assert_status_code_is( index_response, 200 ) + pages = index_response.json() + return id in map( itemgetter( "id" ), pages ) https://bitbucket.org/galaxy/galaxy-central/commits/503836d4cea3/ Changeset: 503836d4cea3 User: jmchilton Date: 2014-01-10 22:10:52 Summary: Parse error codes/messages from new JSON file. Same python interface to this data, but the data can now be reused by clients in other languages (e.g. potentially automating the creation of typed Java exceptions). Affected #: 2 files diff -r b41d51baaae16c4d6e7c4fb20a28cf9aeb5f1f43 -r 503836d4cea399f366a02f7e48b75095a8bb876f lib/galaxy/exceptions/error_codes.json --- /dev/null +++ b/lib/galaxy/exceptions/error_codes.json @@ -0,0 +1,57 @@ +[ + { + "name": "UNKNOWN", + "code": 0, + "message": "Unknown error occurred while processing request." + }, + { + "name": "USER_CANNOT_RUN_AS", + "code": 400001, + "message": "User does not have permissions to run jobs as another user." + }, + { + "name": "USER_INVALID_RUN_AS", + "code": 400002, + "message": "Invalid run_as request - run_as user does not exist." + }, + { + "name": "USER_INVALID_JSON", + "code": 400003, + "message": "Your request did not appear to be valid JSON, please consult the API documentation." + }, + { + "name": "USER_OBJECT_ATTRIBUTE_INVALID", + "code": 400004, + "message": "Attempted to create or update object with invalid attribute value." + }, + { + "name": "USER_OBJECT_ATTRIBUTE_MISSING", + "code": 400005, + "message": "Attempted to create object without required attribute." + }, + { + "name": "USER_SLUG_DUPLICATE", + "code": 400006, + "message": "Slug must be unique per user." + }, + { + "name": "USER_NO_API_KEY", + "code": 403001, + "message": "API Authentication Required for this request" + }, + { + "name": "USER_CANNOT_ACCESS_ITEM", + "code": 403002, + "message": "User cannot access specified item." + }, + { + "name": "USER_DOES_NOT_OWN_ITEM", + "code": 403003, + "message": "User does not own specified item." + }, + { + "name": "USER_OBJECT_NOT_FOUND", + "code": 404001, + "message": "No such object not found." + } +] diff -r b41d51baaae16c4d6e7c4fb20a28cf9aeb5f1f43 -r 503836d4cea399f366a02f7e48b75095a8bb876f lib/galaxy/exceptions/error_codes.py --- a/lib/galaxy/exceptions/error_codes.py +++ b/lib/galaxy/exceptions/error_codes.py @@ -1,3 +1,6 @@ +from pkg_resources import resource_string +from json import loads + # Error codes are provided as a convience to Galaxy API clients, but at this # time they do represent part of the more stable interface. They can change # without warning between releases. @@ -16,18 +19,14 @@ def __int__( self ): return int( self.code ) -# TODO: Guidelines for error message langauge? -UNKNOWN = ErrorCode(0, UNKNOWN_ERROR_MESSAGE) + @staticmethod + def from_dict( entry ): + name = entry.get("name") + code = entry.get("code") + message = entry.get("message") + return ( name, ErrorCode( code, message ) ) -USER_CANNOT_RUN_AS = ErrorCode(400001, "User does not have permissions to run jobs as another user.") -USER_INVALID_RUN_AS = ErrorCode(400002, "Invalid run_as request - run_as user does not exist.") -USER_INVALID_JSON = ErrorCode(400003, "Your request did not appear to be valid JSON, please consult the API documentation.") -USER_OBJECT_ATTRIBUTE_INVALID = ErrorCode(400004, "Attempted to create or update object with invalid attribute value.") -USER_OBJECT_ATTRIBUTE_MISSING = ErrorCode(400005, "Attempted to create object without required attribute.") -USER_SLUG_DUPLICATE = ErrorCode(400006, "Slug must be unique per user.") - -USER_NO_API_KEY = ErrorCode(403001, "API Authentication Required for this request") -USER_CANNOT_ACCESS_ITEM = ErrorCode(403002, "User cannot access specified item.") -USER_DOES_NOT_OWN_ITEM = ErrorCode(403003, "User does not own specified item.") - -USER_OBJECT_NOT_FOUND = ErrorCode(404001, "No such object not found.") +error_codes_json = resource_string( __name__, 'error_codes.json' ) +for entry in loads( error_codes_json ): + name, error_code_obj = ErrorCode.from_dict( entry ) + globals()[ name ] = error_code_obj https://bitbucket.org/galaxy/galaxy-central/commits/fa698b544051/ Changeset: fa698b544051 User: dannon Date: 2014-01-15 14:30:38 Summary: Merged in jmchilton/galaxy-central-fork-1 (pull request #294) New API Decorator Affected #: 11 files diff -r 89dc1fd43e7ba1156580e59bb310d09d95ccdb94 -r fa698b544051c61a6b895f04840309a7807ce5a7 lib/galaxy/exceptions/__init__.py --- a/lib/galaxy/exceptions/__init__.py +++ b/lib/galaxy/exceptions/__init__.py @@ -6,33 +6,66 @@ eggs.require( "Paste" ) from paste import httpexceptions +from ..exceptions import error_codes + class MessageException( Exception ): """ - Exception to make throwing errors from deep in controllers easier + Exception to make throwing errors from deep in controllers easier. """ - def __init__( self, err_msg, type="info" ): - self.err_msg = err_msg + # status code to be set when used with API. + status_code = 400 + # Error code information embedded into API json responses. + err_code = error_codes.UNKNOWN + + def __init__( self, err_msg=None, type="info", **extra_error_info ): + self.err_msg = err_msg or self.err_code.default_error_message self.type = type + self.extra_error_info = extra_error_info + def __str__( self ): return self.err_msg + class ItemDeletionException( MessageException ): pass + class ItemAccessibilityException( MessageException ): - pass + status_code = 403 + err_code = error_codes.USER_CANNOT_ACCESS_ITEM + class ItemOwnershipException( MessageException ): - pass + status_code = 403 + err_code = error_codes.USER_DOES_NOT_OWN_ITEM + + +class DuplicatedSlugException( MessageException ): + status_code = 400 + err_code = error_codes.USER_SLUG_DUPLICATE + + +class ObjectAttributeInvalidException( MessageException ): + status_code = 400 + err_code = error_codes.USER_OBJECT_ATTRIBUTE_INVALID + + +class ObjectAttributeMissingException( MessageException ): + status_code = 400 + err_code = error_codes.USER_OBJECT_ATTRIBUTE_MISSING + class ActionInputError( MessageException ): def __init__( self, err_msg, type="error" ): super( ActionInputError, self ).__init__( err_msg, type ) -class ObjectNotFound( Exception ): + +class ObjectNotFound( MessageException ): """ Accessed object was not found """ - pass + status_code = 404 + err_code = error_codes.USER_OBJECT_NOT_FOUND + class ObjectInvalid( Exception ): """ Accessed object store ID is invalid """ diff -r 89dc1fd43e7ba1156580e59bb310d09d95ccdb94 -r fa698b544051c61a6b895f04840309a7807ce5a7 lib/galaxy/exceptions/error_codes.json --- /dev/null +++ b/lib/galaxy/exceptions/error_codes.json @@ -0,0 +1,57 @@ +[ + { + "name": "UNKNOWN", + "code": 0, + "message": "Unknown error occurred while processing request." + }, + { + "name": "USER_CANNOT_RUN_AS", + "code": 400001, + "message": "User does not have permissions to run jobs as another user." + }, + { + "name": "USER_INVALID_RUN_AS", + "code": 400002, + "message": "Invalid run_as request - run_as user does not exist." + }, + { + "name": "USER_INVALID_JSON", + "code": 400003, + "message": "Your request did not appear to be valid JSON, please consult the API documentation." + }, + { + "name": "USER_OBJECT_ATTRIBUTE_INVALID", + "code": 400004, + "message": "Attempted to create or update object with invalid attribute value." + }, + { + "name": "USER_OBJECT_ATTRIBUTE_MISSING", + "code": 400005, + "message": "Attempted to create object without required attribute." + }, + { + "name": "USER_SLUG_DUPLICATE", + "code": 400006, + "message": "Slug must be unique per user." + }, + { + "name": "USER_NO_API_KEY", + "code": 403001, + "message": "API Authentication Required for this request" + }, + { + "name": "USER_CANNOT_ACCESS_ITEM", + "code": 403002, + "message": "User cannot access specified item." + }, + { + "name": "USER_DOES_NOT_OWN_ITEM", + "code": 403003, + "message": "User does not own specified item." + }, + { + "name": "USER_OBJECT_NOT_FOUND", + "code": 404001, + "message": "No such object not found." + } +] diff -r 89dc1fd43e7ba1156580e59bb310d09d95ccdb94 -r fa698b544051c61a6b895f04840309a7807ce5a7 lib/galaxy/exceptions/error_codes.py --- /dev/null +++ b/lib/galaxy/exceptions/error_codes.py @@ -0,0 +1,32 @@ +from pkg_resources import resource_string +from json import loads + +# Error codes are provided as a convience to Galaxy API clients, but at this +# time they do represent part of the more stable interface. They can change +# without warning between releases. +UNKNOWN_ERROR_MESSAGE = "Unknown error occurred while processing request." + + +class ErrorCode( object ): + + def __init__( self, code, default_error_message ): + self.code = code + self.default_error_message = default_error_message or UNKNOWN_ERROR_MESSAGE + + def __str__( self ): + return str( self.default_error_message ) + + def __int__( self ): + return int( self.code ) + + @staticmethod + def from_dict( entry ): + name = entry.get("name") + code = entry.get("code") + message = entry.get("message") + return ( name, ErrorCode( code, message ) ) + +error_codes_json = resource_string( __name__, 'error_codes.json' ) +for entry in loads( error_codes_json ): + name, error_code_obj = ErrorCode.from_dict( entry ) + globals()[ name ] = error_code_obj diff -r 89dc1fd43e7ba1156580e59bb310d09d95ccdb94 -r fa698b544051c61a6b895f04840309a7807ce5a7 lib/galaxy/web/__init__.py --- a/lib/galaxy/web/__init__.py +++ b/lib/galaxy/web/__init__.py @@ -15,3 +15,7 @@ from framework import expose_api_raw from framework import expose_api_raw_anonymous from framework.base import httpexceptions + +# TODO: Drop and make these the default. +from framework import _future_expose_api +from framework import _future_expose_api_anonymous diff -r 89dc1fd43e7ba1156580e59bb310d09d95ccdb94 -r fa698b544051c61a6b895f04840309a7807ce5a7 lib/galaxy/web/framework/__init__.py --- a/lib/galaxy/web/framework/__init__.py +++ b/lib/galaxy/web/framework/__init__.py @@ -10,9 +10,9 @@ import socket import string import time - +from traceback import format_exc +from Cookie import CookieError from functools import wraps -from Cookie import CookieError pkg_resources.require( "Cheetah" ) from Cheetah.Template import Template @@ -23,6 +23,7 @@ from galaxy import util from galaxy.exceptions import MessageException +from galaxy.exceptions import error_codes from galaxy.util import asbool from galaxy.util import safe_str_cmp from galaxy.util.backports.importlib import import_module @@ -103,6 +104,35 @@ return decorator return argcatcher + +def __extract_payload_from_request(trans, func, kwargs): + content_type = trans.request.headers['content-type'] + if content_type.startswith('application/x-www-form-urlencoded') or content_type.startswith('multipart/form-data'): + # If the content type is a standard type such as multipart/form-data, the wsgi framework parses the request body + # and loads all field values into kwargs. However, kwargs also contains formal method parameters etc. which + # are not a part of the request body. This is a problem because it's not possible to differentiate between values + # which are a part of the request body, and therefore should be a part of the payload, and values which should not be + # in the payload. Therefore, the decorated method's formal arguments are discovered through reflection and removed from + # the payload dictionary. This helps to prevent duplicate argument conflicts in downstream methods. + payload = kwargs.copy() + named_args, _, _, _ = inspect.getargspec(func) + for arg in named_args: + payload.pop(arg, None) + for k, v in payload.iteritems(): + if isinstance(v, (str, unicode)): + try: + payload[k] = from_json_string(v) + except: + # may not actually be json, just continue + pass + payload = util.recursively_stringify_dictionary_keys( payload ) + else: + # Assume application/json content type and parse request body manually, since wsgi won't do it. However, the order of this check + # should ideally be in reverse, with the if clause being a check for application/json and the else clause assuming a standard encoding + # such as multipart/form-data. Leaving it as is for backward compatibility, just in case. + payload = util.recursively_stringify_dictionary_keys( from_json_string( trans.request.body ) ) + return payload + def expose_api_raw( func ): """ Expose this function via the API but don't dump the results @@ -140,35 +170,8 @@ error_message = "API Authentication Required for this request" return error if trans.request.body: - def extract_payload_from_request(trans, func, kwargs): - content_type = trans.request.headers['content-type'] - if content_type.startswith('application/x-www-form-urlencoded') or content_type.startswith('multipart/form-data'): - # If the content type is a standard type such as multipart/form-data, the wsgi framework parses the request body - # and loads all field values into kwargs. However, kwargs also contains formal method parameters etc. which - # are not a part of the request body. This is a problem because it's not possible to differentiate between values - # which are a part of the request body, and therefore should be a part of the payload, and values which should not be - # in the payload. Therefore, the decorated method's formal arguments are discovered through reflection and removed from - # the payload dictionary. This helps to prevent duplicate argument conflicts in downstream methods. - payload = kwargs.copy() - named_args, _, _, _ = inspect.getargspec(func) - for arg in named_args: - payload.pop(arg, None) - for k, v in payload.iteritems(): - if isinstance(v, (str, unicode)): - try: - payload[k] = from_json_string(v) - except: - # may not actually be json, just continue - pass - payload = util.recursively_stringify_dictionary_keys( payload ) - else: - # Assume application/json content type and parse request body manually, since wsgi won't do it. However, the order of this check - # should ideally be in reverse, with the if clause being a check for application/json and the else clause assuming a standard encoding - # such as multipart/form-data. Leaving it as is for backward compatibility, just in case. - payload = util.recursively_stringify_dictionary_keys( from_json_string( trans.request.body ) ) - return payload try: - kwargs['payload'] = extract_payload_from_request(trans, func, kwargs) + kwargs['payload'] = __extract_payload_from_request(trans, func, kwargs) except ValueError: error_status = '400 Bad Request' error_message = 'Your request did not appear to be valid JSON, please consult the API documentation' @@ -210,6 +213,143 @@ decorator.exposed = True return decorator +API_RESPONSE_CONTENT_TYPE = "application/json" + + +def __api_error_message( trans, **kwds ): + exception = kwds.get( "exception", None ) + if exception: + # If we are passed a MessageException use err_msg. + default_error_code = getattr( exception, "err_code", error_codes.UNKNOWN ) + default_error_message = getattr( exception, "err_msg", default_error_code.default_error_message ) + extra_error_info = getattr( exception, 'extra_error_info', {} ) + if not isinstance( extra_error_info, dict ): + extra_error_info = {} + else: + default_error_message = "Error processing API request." + default_error_code = error_codes.UNKNOWN + extra_error_info = {} + traceback_string = kwds.get( "traceback", "No traceback available." ) + err_msg = kwds.get( "err_msg", default_error_message ) + error_code_object = kwds.get( "err_code", default_error_code ) + try: + error_code = error_code_object.code + except AttributeError: + # Some sort of bad error code sent in, logic failure on part of + # Galaxy developer. + error_code = error_codes.UNKNOWN.code + # Would prefer the terminology of error_code and error_message, but + # err_msg used a good number of places already. Might as well not change + # it? + error_response = dict( err_msg=err_msg, err_code=error_code, **extra_error_info ) + if trans.debug: # TODO: Should admins get to see traceback as well? + error_response[ "traceback" ] = traceback_string + return error_response + + +def __api_error_response( trans, **kwds ): + error_dict = __api_error_message( trans, **kwds ) + exception = kwds.get( "exception", None ) + # If we are given an status code directly - use it - otherwise check + # the exception for a status_code attribute. + if "status_code" in kwds: + status_code = int( kwds.get( "status_code" ) ) + elif hasattr( exception, "status_code" ): + status_code = int( exception.status_code ) + else: + status_code = 500 + response = trans.response + if not response.status or str(response.status).startswith("20"): + # Unset status code appears to be string '200 OK', if anything + # non-success (i.e. not 200 or 201) has been set, do not override + # underlying controller. + response.status = status_code + return to_json_string( error_dict ) + + +# TODO: rename as expose_api and make default. +def _future_expose_api_anonymous( func, to_json=True ): + """ + Expose this function via the API but don't require a set user. + """ + return _future_expose_api( func, to_json=to_json, user_required=False ) + + +# TODO: rename as expose_api and make default. +def _future_expose_api( func, to_json=True, user_required=True ): + """ + Expose this function via the API. + """ + @wraps(func) + def decorator( self, trans, *args, **kwargs ): + if trans.error_message: + # TODO: Document this branch, when can this happen, + # I don't understand it. + return __api_error_response( trans, err_msg=trans.error_message ) + if user_required and trans.anonymous: + error_code = error_codes.USER_NO_API_KEY + # Use error codes default error message. + return __api_error_response( trans, err_code=error_code, status_code=403 ) + if trans.request.body: + try: + kwargs['payload'] = __extract_payload_from_request(trans, func, kwargs) + except ValueError: + error_code = error_codes.USER_INVALID_JSON + return __api_error_response( trans, status_code=400, err_code=error_code ) + + trans.response.set_content_type( API_RESPONSE_CONTENT_TYPE ) + # send 'do not cache' headers to handle IE's caching of ajax get responses + trans.response.headers[ 'Cache-Control' ] = "max-age=0,no-cache,no-store" + # TODO: Refactor next block out into a helper procedure. + # Perform api_run_as processing, possibly changing identity + if 'payload' in kwargs and 'run_as' in kwargs['payload']: + if not trans.user_can_do_run_as(): + error_code = error_codes.USER_CANNOT_RUN_AS + return __api_error_response( trans, err_code=error_code, status_code=403 ) + try: + decoded_user_id = trans.security.decode_id( kwargs['payload']['run_as'] ) + except TypeError: + error_message = "Malformed user id ( %s ) specified, unable to decode." % str( kwargs['payload']['run_as'] ) + error_code = error_codes.USER_INVALID_RUN_AS + return __api_error_response( trans, err_code=error_code, err_msg=error_message, status_code=400) + try: + user = trans.sa_session.query( trans.app.model.User ).get( decoded_user_id ) + trans.api_inherit_admin = trans.user_is_admin() + trans.set_user(user) + except: + error_code = error_codes.USER_INVALID_RUN_AS + return __api_error_response( trans, err_code=error_code, status_code=400 ) + try: + rval = func( self, trans, *args, **kwargs) + if to_json and trans.debug: + rval = to_json_string( rval, indent=4, sort_keys=True ) + elif to_json: + rval = to_json_string( rval ) + return rval + except MessageException as e: + traceback_string = format_exc() + return __api_error_response( trans, exception=e, traceback=traceback_string ) + except paste.httpexceptions.HTTPException: + # TODO: Allow to pass or format for the API??? + raise # handled + except Exception as e: + traceback_string = format_exc() + error_message = 'Uncaught exception in exposed API method:' + log.exception( error_message ) + return __api_error_response( + trans, + status_code=500, + exception=e, + traceback=traceback_string, + err_msg=error_message, + err_code=error_codes.UNKNOWN + ) + if not hasattr(func, '_orig'): + decorator._orig = func + decorator.exposed = True + return decorator + + def require_admin( func ): @wraps(func) def decorator( self, trans, *args, **kwargs ): diff -r 89dc1fd43e7ba1156580e59bb310d09d95ccdb94 -r fa698b544051c61a6b895f04840309a7807ce5a7 lib/galaxy/webapps/galaxy/api/histories.py --- a/lib/galaxy/webapps/galaxy/api/histories.py +++ b/lib/galaxy/webapps/galaxy/api/histories.py @@ -9,6 +9,8 @@ from paste.httpexceptions import HTTPBadRequest, HTTPForbidden, HTTPInternalServerError, HTTPException from galaxy import web +from galaxy.web import _future_expose_api as expose_api +from galaxy.web import _future_expose_api_anonymous as expose_api_anonymous from galaxy.util import string_as_bool, restore_text from galaxy.util.sanitize_html import sanitize_html from galaxy.web.base.controller import BaseAPIController, UsesHistoryMixin, UsesTagsMixin @@ -20,7 +22,7 @@ class HistoriesController( BaseAPIController, UsesHistoryMixin, UsesTagsMixin ): - @web.expose_api_anonymous + @expose_api_anonymous def index( self, trans, deleted='False', **kwd ): """ index( trans, deleted='False' ) @@ -152,7 +154,7 @@ return history_data - @web.expose_api + @expose_api def create( self, trans, payload, **kwd ): """ create( trans, payload ) diff -r 89dc1fd43e7ba1156580e59bb310d09d95ccdb94 -r fa698b544051c61a6b895f04840309a7807ce5a7 lib/galaxy/webapps/galaxy/api/page_revisions.py --- a/lib/galaxy/webapps/galaxy/api/page_revisions.py +++ b/lib/galaxy/webapps/galaxy/api/page_revisions.py @@ -2,8 +2,9 @@ API for updating Galaxy Pages """ import logging -from galaxy import web +from galaxy.web import _future_expose_api as expose_api from galaxy.web.base.controller import SharableItemSecurityMixin, BaseAPIController, SharableMixin +from galaxy import exceptions from galaxy.model.item_attrs import UsesAnnotations from galaxy.util.sanitize_html import sanitize_html @@ -12,7 +13,7 @@ class PageRevisionsController( BaseAPIController, SharableItemSecurityMixin, UsesAnnotations, SharableMixin ): - @web.expose_api + @expose_api def index( self, trans, page_id, **kwd ): """ index( self, trans, page_id, **kwd ) @@ -24,14 +25,16 @@ :rtype: list :returns: dictionaries containing different revisions of the page """ + page = self._get_page( trans, page_id ) + self._verify_page_ownership( trans, page ) + r = trans.sa_session.query( trans.app.model.PageRevision ).filter_by( page_id=trans.security.decode_id(page_id) ) out = [] for page in r: - if self.security_check( trans, page, True, True ): - out.append( self.encode_all_ids( trans, page.to_dict(), True) ) + out.append( self.encode_all_ids( trans, page.to_dict(), True) ) return out - @web.expose_api + @expose_api def create( self, trans, page_id, payload, **kwd ): """ create( self, trans, page_id, payload **kwd ) @@ -46,39 +49,42 @@ :rtype: dictionary :returns: Dictionary with 'success' or 'error' element to indicate the result of the request """ - error_str = "" + content = payload.get("content", None) + if not content: + raise exceptions.ObjectAttributeMissingException("content undefined or empty") - if not page_id: - error_str = "page_id is required" - elif not payload.get("content", None): - error_str = "content is required" + page = self._get_page( trans, page_id ) + self._verify_page_ownership( trans, page ) + + if 'title' in payload: + title = payload['title'] else: + title = page.title - # Create the new stored page + content = sanitize_html( content, 'utf-8', 'text/html' ) + + page_revision = trans.app.model.PageRevision() + page_revision.title = title + page_revision.page = page + page.latest_revision = page_revision + page_revision.content = content + + # Persist + session = trans.sa_session + session.flush() + + return page_revision.to_dict( view="element" ) + + def _get_page( self, trans, page_id ): + page = None + try: page = trans.sa_session.query( trans.app.model.Page ).get( trans.security.decode_id(page_id) ) - if page is None: - return { "error" : "page not found"} + except Exception: + pass + if not page: + raise exceptions.ObjectNotFound() + return page - if not self.security_check( trans, page, True, True ): - return { "error" : "page not found"} - - if 'title' in payload: - title = payload['title'] - else: - title = page.title - - content = payload.get("content", "") - content = sanitize_html( content, 'utf-8', 'text/html' ) - - page_revision = trans.app.model.PageRevision() - page_revision.title = title - page_revision.page = page - page.latest_revision = page_revision - page_revision.content = content - # Persist - session = trans.sa_session - session.flush() - - return { "success" : "revision posted" } - - return { "error" : error_str } + def _verify_page_ownership( self, trans, page ): + if not self.security_check( trans, page, True, True ): + raise exceptions.ItemOwnershipException() diff -r 89dc1fd43e7ba1156580e59bb310d09d95ccdb94 -r fa698b544051c61a6b895f04840309a7807ce5a7 lib/galaxy/webapps/galaxy/api/pages.py --- a/lib/galaxy/webapps/galaxy/api/pages.py +++ b/lib/galaxy/webapps/galaxy/api/pages.py @@ -2,8 +2,9 @@ API for updating Galaxy Pages """ import logging -from galaxy import web +from galaxy.web import _future_expose_api as expose_api from galaxy.web.base.controller import SharableItemSecurityMixin, BaseAPIController, SharableMixin +from galaxy import exceptions from galaxy.model.item_attrs import UsesAnnotations from galaxy.util.sanitize_html import sanitize_html @@ -12,7 +13,7 @@ class PagesController( BaseAPIController, SharableItemSecurityMixin, UsesAnnotations, SharableMixin ): - @web.expose_api + @expose_api def index( self, trans, deleted=False, **kwd ): """ index( self, trans, deleted=False, **kwd ) @@ -47,7 +48,7 @@ return out - @web.expose_api + @expose_api def create( self, trans, payload, **kwd ): """ create( self, trans, payload, **kwd ) @@ -64,45 +65,41 @@ :returns: Dictionary return of the Page.to_dict call """ user = trans.get_user() - error_str = "" if not payload.get("title", None): - error_str = "Page name is required" + raise exceptions.ObjectAttributeMissingException( "Page name is required" ) elif not payload.get("slug", None): - error_str = "Page id is required" + raise exceptions.ObjectAttributeMissingException( "Page id is required" ) elif not self._is_valid_slug( payload["slug"] ): - error_str = "Page identifier must consist of only lowercase letters, numbers, and the '-' character" + raise exceptions.ObjectAttributeInvalidException( "Page identifier must consist of only lowercase letters, numbers, and the '-' character" ) elif trans.sa_session.query( trans.app.model.Page ).filter_by( user=user, slug=payload["slug"], deleted=False ).first(): - error_str = "Page id must be unique" - else: + raise exceptions.DuplicatedSlugException( "Page slug must be unique" ) - content = payload.get("content", "") - content = sanitize_html( content, 'utf-8', 'text/html' ) + content = payload.get("content", "") + content = sanitize_html( content, 'utf-8', 'text/html' ) - # Create the new stored page - page = trans.app.model.Page() - page.title = payload['title'] - page.slug = payload['slug'] - page_annotation = sanitize_html( payload.get( "annotation", "" ), 'utf-8', 'text/html' ) - self.add_item_annotation( trans.sa_session, trans.get_user(), page, page_annotation ) - page.user = user - # And the first (empty) page revision - page_revision = trans.app.model.PageRevision() - page_revision.title = payload['title'] - page_revision.page = page - page.latest_revision = page_revision - page_revision.content = content - # Persist - session = trans.sa_session - session.add( page ) - session.flush() + # Create the new stored page + page = trans.app.model.Page() + page.title = payload['title'] + page.slug = payload['slug'] + page_annotation = sanitize_html( payload.get( "annotation", "" ), 'utf-8', 'text/html' ) + self.add_item_annotation( trans.sa_session, trans.get_user(), page, page_annotation ) + page.user = user + # And the first (empty) page revision + page_revision = trans.app.model.PageRevision() + page_revision.title = payload['title'] + page_revision.page = page + page.latest_revision = page_revision + page_revision.content = content + # Persist + session = trans.sa_session + session.add( page ) + session.flush() - rval = self.encode_all_ids( trans, page.to_dict(), True ) - return rval + rval = self.encode_all_ids( trans, page.to_dict(), True ) + return rval - return { "error" : error_str } - - @web.expose_api + @expose_api def delete( self, trans, id, **kwd ): """ delete( self, trans, id, **kwd ) @@ -114,22 +111,14 @@ :rtype: dict :returns: Dictionary with 'success' or 'error' element to indicate the result of the request """ - page_id = id - try: - page = trans.sa_session.query(self.app.model.Page).get(trans.security.decode_id(page_id)) - except Exception, e: - return { "error" : "Page with ID='%s' can not be found\n Exception: %s" % (page_id, str( e )) } + page = self._get_page( trans, id ) - # check to see if user has permissions to selected workflow - if page.user != trans.user and not trans.user_is_admin(): - return { "error" : "Workflow is not owned by or shared with current user" } - - #Mark a workflow as deleted + #Mark a page as deleted page.deleted = True trans.sa_session.flush() - return { "success" : "Deleted", "id" : page_id } + return '' # TODO: Figure out what to return on DELETE, document in guidelines! - @web.expose_api + @expose_api def show( self, trans, id, **kwd ): """ show( self, trans, id, **kwd ) @@ -141,8 +130,22 @@ :rtype: dict :returns: Dictionary return of the Page.to_dict call with the 'content' field populated by the most recent revision """ - page = trans.sa_session.query( trans.app.model.Page ).get( trans.security.decode_id( id ) ) + page = self._get_page( trans, id ) self.security_check( trans, page, check_ownership=False, check_accessible=True) rval = self.encode_all_ids( trans, page.to_dict(), True ) rval['content'] = page.latest_revision.content return rval + + def _get_page( self, trans, id ): # Fetches page object and verifies security. + try: + page = trans.sa_session.query( trans.app.model.Page ).get( trans.security.decode_id( id ) ) + except Exception: + page = None + + if not page: + raise exceptions.ObjectNotFound() + + if page.user != trans.user and not trans.user_is_admin(): + raise exceptions.ItemOwnershipException() + + return page diff -r 89dc1fd43e7ba1156580e59bb310d09d95ccdb94 -r fa698b544051c61a6b895f04840309a7807ce5a7 test/base/api.py --- a/test/base/api.py +++ b/test/base/api.py @@ -60,6 +60,12 @@ for key in keys: assert key in response, "Response [%s] does not contain key [%s]" % ( response, key ) + def _assert_error_code_is( self, response, error_code ): + if hasattr( response, "json" ): + response = response.json() + self._assert_has_keys( response, "err_code" ) + self.assertEquals( response[ "err_code" ], int( error_code ) ) + def _random_key( self ): # Used for invalid request testing... return "1234567890123456" diff -r 89dc1fd43e7ba1156580e59bb310d09d95ccdb94 -r fa698b544051c61a6b895f04840309a7807ce5a7 test/functional/api/test_page_revisions.py --- /dev/null +++ b/test/functional/api/test_page_revisions.py @@ -0,0 +1,35 @@ +from galaxy.exceptions import error_codes +from functional.api.pages import BasePageApiTestCase + + +class PageRevisionsApiTestCase( BasePageApiTestCase ): + + def test_create( self ): + page_json = self._create_valid_page_with_slug( "pr1" ) + revision_data = dict( content="<p>NewContent!</p>" ) + page_revision_response = self._post( "pages/%s/revisions" % page_json[ 'id' ], data=revision_data ) + self._assert_status_code_is( page_revision_response, 200 ) + page_revision_json = page_revision_response.json() + self._assert_has_keys( page_revision_json, 'id', 'content' ) + + def test_403_if_create_revision_on_unowned_page( self ): + page_json = self._create_valid_page_as( "pr2@bx.psu.edu", "pr2" ) + revision_data = dict( content="<p>NewContent!</p>" ) + page_revision_response = self._post( "pages/%s/revisions" % page_json[ 'id' ], data=revision_data ) + self._assert_status_code_is( page_revision_response, 403 ) + + def test_revision_index( self ): + page_json = self._create_valid_page_with_slug( "pr3" ) + revision_data = dict( content="<p>NewContent!</p>" ) + revisions_url = "pages/%s/revisions" % page_json[ 'id' ] + self._post( revisions_url, data=revision_data ) + revisions_response = self._get( revisions_url ) + self._assert_status_code_is( revisions_response, 200 ) + revisions_json = revisions_response.json() + assert len( revisions_json ) == 2 # Original revision and new one + + def test_404_if_index_unknown_page( self ): + revisions_url = "pages/%s/revisions" % self._random_key() + revisions_response = self._get( revisions_url ) + self._assert_status_code_is( revisions_response, 404 ) + self._assert_error_code_is( revisions_response, error_codes.USER_OBJECT_NOT_FOUND ) diff -r 89dc1fd43e7ba1156580e59bb310d09d95ccdb94 -r fa698b544051c61a6b895f04840309a7807ce5a7 test/functional/api/test_pages.py --- /dev/null +++ b/test/functional/api/test_pages.py @@ -0,0 +1,105 @@ +from galaxy.exceptions import error_codes +from base import api +from base.interactor import delete_request + +from operator import itemgetter + + +class BasePageApiTestCase( api.ApiTestCase ): + + def _create_valid_page_with_slug( self, slug ): + page_request = self._test_page_payload( slug=slug ) + page_response = self._post( "pages", page_request ) + self._assert_status_code_is( page_response, 200 ) + return page_response.json() + + def _create_valid_page_as( self, other_email, slug ): + run_as_user = self._setup_user( other_email ) + page_request = self._test_page_payload( slug=slug ) + page_request[ "run_as" ] = run_as_user[ "id" ] + page_response = self._post( "pages", page_request, admin=True ) + self._assert_status_code_is( page_response, 200 ) + return page_response.json() + + def _test_page_payload( self, **kwds ): + request = dict( + slug="mypage", + title="MY PAGE", + content="<p>Page!</p>", + ) + request.update( **kwds ) + return request + + +class PageApiTestCase( BasePageApiTestCase ): + + def test_create( self ): + response_json = self._create_valid_page_with_slug( "mypage" ) + self._assert_has_keys( response_json, "slug", "title", "id" ) + + def test_index( self ): + create_response_json = self._create_valid_page_with_slug( "indexpage" ) + assert self._users_index_has_page_with_id( create_response_json[ "id" ] ) + + def test_index_doesnt_show_unavailable_pages( self ): + create_response_json = self._create_valid_page_as( "others_page_index@bx.psu.edu", "otherspageindex" ) + assert not self._users_index_has_page_with_id( create_response_json[ "id" ] ) + + def test_cannot_create_pages_with_same_slug( self ): + page_request = self._test_page_payload( slug="mypage1" ) + page_response_1 = self._post( "pages", page_request ) + self._assert_status_code_is( page_response_1, 200 ) + page_response_2 = self._post( "pages", page_request ) + self._assert_status_code_is( page_response_2, 400 ) + self._assert_error_code_is( page_response_2, error_codes.USER_SLUG_DUPLICATE ) + + def test_page_requires_name( self ): + page_request = self._test_page_payload() + del page_request[ 'title' ] + page_response = self._post( "pages", page_request ) + self._assert_status_code_is( page_response, 400 ) + self._assert_error_code_is( page_response, error_codes.USER_OBJECT_ATTRIBUTE_MISSING ) + + def test_page_requires_slug( self ): + page_request = self._test_page_payload() + del page_request[ 'slug' ] + page_response = self._post( "pages", page_request ) + self._assert_status_code_is( page_response, 400 ) + + def test_delete( self ): + response_json = self._create_valid_page_with_slug( "testdelete" ) + delete_response = delete_request( self._api_url( "pages/%s" % response_json[ 'id' ], use_key=True ) ) + self._assert_status_code_is( delete_response, 200 ) + + def test_404_on_delete_unknown_page( self ): + delete_response = delete_request( self._api_url( "pages/%s" % self._random_key(), use_key=True ) ) + self._assert_status_code_is( delete_response, 404 ) + self._assert_error_code_is( delete_response, error_codes.USER_OBJECT_NOT_FOUND ) + + def test_403_on_delete_unowned_page( self ): + page_response = self._create_valid_page_as( "others_page@bx.psu.edu", "otherspage" ) + delete_response = delete_request( self._api_url( "pages/%s" % page_response[ "id" ], use_key=True ) ) + self._assert_status_code_is( delete_response, 403 ) + self._assert_error_code_is( delete_response, error_codes.USER_DOES_NOT_OWN_ITEM ) + + def test_show( self ): + response_json = self._create_valid_page_with_slug( "pagetoshow" ) + show_response = self._get( "pages/%s" % response_json['id'] ) + self._assert_status_code_is( show_response, 200 ) + show_json = show_response.json() + self._assert_has_keys( show_json, "slug", "title", "id" ) + self.assertEquals( show_json["slug"], "pagetoshow" ) + self.assertEquals( show_json["title"], "MY PAGE" ) + self.assertEquals( show_json["content"], "<p>Page!</p>" ) + + def test_403_on_unowner_show( self ): + response_json = self._create_valid_page_as( "others_page_show@bx.psu.edu", "otherspageshow" ) + show_response = self._get( "pages/%s" % response_json['id'] ) + self._assert_status_code_is( show_response, 403 ) + self._assert_error_code_is( show_response, error_codes.USER_DOES_NOT_OWN_ITEM ) + + def _users_index_has_page_with_id( self, id ): + index_response = self._get( "pages" ) + self._assert_status_code_is( index_response, 200 ) + pages = index_response.json() + return id in map( itemgetter( "id" ), pages ) Repository URL: https://bitbucket.org/galaxy/galaxy-central/ -- This is a commit notification from bitbucket.org. You are receiving this because you have the service enabled, addressing the recipient of this email.