From c854ccbb4b24858eecbea1f775ed95a4c64d0b23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 9 May 2017 14:08:35 +0200 Subject: [PATCH] Generic PATCH handler class. A class-based approach is easier to extend than the function-based approach used in the nodes. That one is still there, though -- might look at it at a later time. This handler is primarily for Flamenco. --- pillar/api/patch_handler.py | 90 ++++++++++++++++++++++++++++ tests/test_api/test_patch_handler.py | 89 +++++++++++++++++++++++++++ 2 files changed, 179 insertions(+) create mode 100644 pillar/api/patch_handler.py create mode 100644 tests/test_api/test_patch_handler.py diff --git a/pillar/api/patch_handler.py b/pillar/api/patch_handler.py new file mode 100644 index 00000000..14dc910a --- /dev/null +++ b/pillar/api/patch_handler.py @@ -0,0 +1,90 @@ +"""Handler for PATCH requests. + +This supports PATCH request in the sense described by William Durand: +http://williamdurand.fr/2014/02/14/please-do-not-patch-like-an-idiot/ + +Each PATCH should be a JSON dict with at least a key 'op' with the +name of the operation to perform. +""" + +import logging + +import flask + +from pillar.api.utils import authorization + +log = logging.getLogger(__name__) + + +class AbstractPatchHandler: + """Abstract PATCH handler supporting multiple operations. + + Each operation, i.e. possible value of the 'op' key in the PATCH body, + should be matched to a similarly named "patch_xxx" function in a subclass. + For example, the operation "set-owner" is mapped to "patch_set_owner". + + :cvar route: the Flask/Werkzeug route to attach this handler to. + For most handlers, the default will be fine. + :cvar item_name: the name of the things to patch, like "job", "task" etc. + Only used for logging. + """ + + route: str = '/' + item_name: str = None + + def __init_subclass__(cls, **kwargs): + if not cls.route: + raise ValueError('Subclass must set route') + if not cls.item_name: + raise ValueError('Subclass must set item_name') + + def __init__(self, blueprint: flask.Blueprint): + self.log: logging.Logger = log.getChild(self.__class__.__name__) + self.patch_handlers = { + name[6:].replace('_', '-'): getattr(self, name) + for name in dir(self) + if name.startswith('patch_') and callable(getattr(self, name)) + } + + if self.log.isEnabledFor(logging.INFO): + self.log.info('Creating PATCH handler %s%s for operations: %s', + blueprint.name, self.route, + sorted(self.patch_handlers.keys())) + + blueprint.add_url_rule(self.route, + self.patch.__name__, + self.patch, + methods=['PATCH']) + + @authorization.require_login() + def patch(self, object_id: str): + from flask import request + import werkzeug.exceptions as wz_exceptions + from pillar.api.utils import str2id, authentication + + # Parse the request + real_object_id = str2id(object_id) + patch = request.get_json() + if not patch: + raise wz_exceptions.BadRequest('Patch must contain JSON') + + try: + patch_op = patch['op'] + except KeyError: + raise wz_exceptions.BadRequest("PATCH should contain 'op' key to denote operation.") + + log.debug('User %s wants to PATCH "%s" %s %s', + authentication.current_user_id(), patch_op, self.item_name, real_object_id) + + # Find the PATCH handler for the operation. + try: + handler = self.patch_handlers[patch_op] + except KeyError: + log.warning('No %s PATCH handler for operation %r', self.item_name, patch_op) + raise wz_exceptions.BadRequest('Operation %r not supported' % patch_op) + + # Let the PATCH handler do its thing. + response = handler(real_object_id, patch) + if response is None: + return '', 204 + return response diff --git a/tests/test_api/test_patch_handler.py b/tests/test_api/test_patch_handler.py new file mode 100644 index 00000000..00628439 --- /dev/null +++ b/tests/test_api/test_patch_handler.py @@ -0,0 +1,89 @@ +from pillar.tests import AbstractPillarTest + + +class PatchHandlerTest(AbstractPillarTest): + def setUp(self, **kwargs): + super().setUp(**kwargs) + import flask + from pillar.api import patch_handler + + # Create a patch handler and register it. + class BogusPatchHandler(patch_handler.AbstractPatchHandler): + item_name = 'gremlin' + + def patch_test_echo(self, op: str, patch: dict): + return flask.jsonify({'echo': patch['echo']}) + + def patch_test_empty(self, op: str, patch: dict): + return None + + blueprint = flask.Blueprint('test_patch_handler', __name__) + self.patch_handler = BogusPatchHandler(blueprint) + + self.app.register_api_blueprint(blueprint, url_prefix='/test') + + # Patching always requires a logged-in user. + self.user_id = self.create_user(token='user-token') + + def test_patch_anonymous(self): + import bson + + oid = bson.ObjectId() + self.patch('/api/test/%s' % oid, expected_status=403) + + def test_patch_without_json(self): + import bson + + oid = bson.ObjectId() + self.patch('/api/test/%s' % oid, auth_token='user-token', expected_status=400) + + def test_patch_no_operation(self): + import bson + + oid = bson.ObjectId() + self.patch('/api/test/%s' % oid, auth_token='user-token', + json={'je': 'moeder'}, + expected_status=400) + + def test_patch_invalid_operation(self): + import bson + + oid = bson.ObjectId() + self.patch('/api/test/%s' % oid, auth_token='user-token', + json={'op': 'snowcrash'}, + expected_status=400) + + def test_patch_happy(self): + import bson + + oid = bson.ObjectId() + resp = self.patch('/api/test/%s' % oid, auth_token='user-token', + json={'op': 'test-echo', + 'echo': '¡Thith ith Špahtah!'}) + self.assertEqual({'echo': '¡Thith ith Špahtah!'}, resp.json()) + + def test_patch_empty_response(self): + import bson + + oid = bson.ObjectId() + resp = self.patch('/api/test/%s' % oid, auth_token='user-token', + json={'op': 'test-empty', + 'echo': '¡Thith ith Špahtah!'}, + expected_status=204) + self.assertEqual(b'', resp.data) + + +class PatchHandlerCreationTest(AbstractPillarTest): + def test_without_route(self): + from pillar.api import patch_handler + + with self.assertRaises(ValueError): + class BogusPatchHandler(patch_handler.AbstractPatchHandler): + route = '' + + def test_without_item_name(self): + from pillar.api import patch_handler + + with self.assertRaises(ValueError): + class BogusPatchHandler(patch_handler.AbstractPatchHandler): + pass