From 13c67e3ab89981636e31209c2dad7f0d0d72314f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 23 Sep 2016 17:17:07 +0200 Subject: [PATCH] Allow limited PATCHing of shots from Blender and Web. Soon PATCH should become the only way in which shots are edited, so we should possibly remove PUT permissions. --- attract/shots/__init__.py | 93 +++++++++++++++++++++++- tests/test_shots.py | 146 +++++++++++++++++++++++++++++++++++++- 2 files changed, 235 insertions(+), 4 deletions(-) diff --git a/attract/shots/__init__.py b/attract/shots/__init__.py index 650fd29..47e1d90 100644 --- a/attract/shots/__init__.py +++ b/attract/shots/__init__.py @@ -1,17 +1,38 @@ """Shot management.""" - import collections +import logging import attr +import flask import flask_login +from eve.methods.put import put_internal +from werkzeug import exceptions as wz_exceptions import pillarsdk +import pillar.api.utils from pillar.web.system_util import pillar_api +from pillar.api.nodes.custom import register_patch_handler from attract import attrs_extra from attract.node_types import node_type_shot, node_type_task +# From patch operation name to fields that operation may edit. +VALID_PATCH_OPERATIONS = { + u'from-blender': { + u'name', + u'properties.trim_start_in_frames', + u'properties.duration_in_edit_in_frames', + u'properties.cut_in_timeline_in_frames', + u'properties.status', + }, + u'from-web': { + u'properties.status', + u'properties.notes', + }, +} +log = logging.getLogger(__name__) + @attr.s class ShotManager(object): @@ -23,14 +44,17 @@ class ShotManager(object): :rtype: pillarsdk.Node """ + project_id = project['_id'] + self._log.info('Creating shot for project %s', project_id) + api = pillar_api() node_type = project.get_node_type(node_type_shot['name']) if not node_type: - raise ValueError('Project %s not set up for Attract' % project._id) + raise ValueError('Project %s not set up for Attract' % project_id) node_props = dict( name='New shot', - project=project['_id'], + project=project_id, user=flask_login.current_user.objectid, node_type=node_type['name'], properties={ @@ -106,3 +130,66 @@ class ShotManager(object): shot.update(api=api) return shot + + +def node_setattr(node, key, value): + """Sets a node property by dotted key. + + Modifies the node in-place. + """ + + set_on = node + while key and '.' in key: + head, key = key.split('.', 1) + set_on = node[head] + set_on[key] = value + + +@register_patch_handler(node_type_shot['name']) +def patch_shot(node_id, patch): + assert_is_valid_patch(patch) + + # Find the full node, so we can PUT it through Eve for validation. + nodes_coll = flask.current_app.data.driver.db['nodes'] + node_query = {'_id': node_id, + 'node_type': node_type_shot['name']} + node = nodes_coll.find_one(node_query) + if node is None: + log.warning('How can node %s not be found?', node_id) + raise wz_exceptions.NotFound('Node %s not found' % node_id) + + # Set the fields + log.info('Patching node %s: %s', node_id, patch) + for key, value in patch['$set'].items(): + node_setattr(node, key, value) + + node = pillar.api.utils.remove_private_keys(node) + r, _, _, status = put_internal('nodes', node, _id=node_id) + return pillar.api.utils.jsonify(r, status=status) + + +def assert_is_valid_patch(patch): + """Raises an exception when the patch isn't valid.""" + + try: + op = patch['op'] + except KeyError: + raise wz_exceptions.BadRequest("PATCH should have a key 'op' indicating the operation.") + + try: + allowed_fields = VALID_PATCH_OPERATIONS[op] + except KeyError: + valid_ops = u', '.join(VALID_PATCH_OPERATIONS.keys()) + raise wz_exceptions.BadRequest(u'Operation should be one of %s' % valid_ops) + + try: + fields = set(patch['$set'].keys()) + except KeyError: + raise wz_exceptions.BadRequest("PATCH should have a key '$set' " + "indicating the fields to set.") + + disallowed_fields = fields - allowed_fields + if disallowed_fields: + raise wz_exceptions.BadRequest(u"Operation '%s' does not allow you to set fields %s" % ( + op, disallowed_fields + )) diff --git a/tests/test_shots.py b/tests/test_shots.py index 026f179..7a26343 100644 --- a/tests/test_shots.py +++ b/tests/test_shots.py @@ -1,5 +1,7 @@ # -*- encoding: utf-8 -*- +import unittest + import responses from bson import ObjectId @@ -12,7 +14,7 @@ import pillar.tests.common_test_data as ctd from abstract_attract_test import AbstractAttractTest -class ShotManagerTest(AbstractAttractTest): +class AbstractShotTest(AbstractAttractTest): def setUp(self, **kwargs): AbstractAttractTest.setUp(self, **kwargs) @@ -45,6 +47,7 @@ class ShotManagerTest(AbstractAttractTest): self.assertIsInstance(shot, pillarsdk.Node) return shot +class ShotManagerTest(AbstractShotTest): @responses.activate def test_tasks_for_shot(self): shot1 = self.create_shot() @@ -117,3 +120,144 @@ class ShotManagerTest(AbstractAttractTest): self.assertEqual(u'todo', found['properties']['status']) self.assertEqual(u'Shoot the Pad Thai', found['description']) self.assertNotIn(u'notes', found['properties']) + + +class NodeSetattrTest(unittest.TestCase): + def test_simple(self): + from attract.shots import node_setattr + + node = {} + node_setattr(node, 'a', 5) + self.assertEqual({'a': 5}, node) + + node_setattr(node, 'b', {'complexer': 'value'}) + self.assertEqual({'a': 5, 'b': {'complexer': 'value'}}, node) + + def test_dotted(self): + from attract.shots import node_setattr + + node = {} + self.assertRaises(KeyError, node_setattr, node, 'a.b', 5) + + node = {'b': {}} + node_setattr(node, 'b.simple', 'value') + self.assertEqual({'b': {'simple': 'value'}}, node) + + node_setattr(node, 'b.complex', {'yes': 'value'}) + self.assertEqual({'b': {'simple': 'value', + 'complex': {'yes': 'value'}}}, node) + + node_setattr(node, 'b.complex', {'yes': 5}) + self.assertEqual({'b': {'simple': 'value', + 'complex': {'yes': 5}}}, node) + + def test_none_simple(self): + from attract.shots import node_setattr + + node = {} + node_setattr(node, 'a', None) + node_setattr(node, None, 'b') + self.assertEqual({'a': None, None: 'b'}, node) + + def test_none_dotted(self): + from attract.shots import node_setattr + + node = {} + self.assertRaises(KeyError, node_setattr, node, 'a.b', None) + + node = {'b': {}} + node_setattr(node, 'b.simple', None) + self.assertEqual({'b': {'simple': None}}, node) + + node_setattr(node, 'b.complex', {'yes': None}) + self.assertEqual({'b': {'simple': None, + 'complex': {'yes': None}}}, node) + + node_setattr(node, 'b.complex', {None: 5}) + self.assertEqual({'b': {'simple': None, + 'complex': {None: 5}}}, node) + + +class PatchShotTest(AbstractShotTest): + @responses.activate + def test_patch_from_blender_happy(self): + shot = self.create_shot() + self.create_valid_auth_token(ctd.EXAMPLE_PROJECT_OWNER_ID, 'token') + + url = '/api/nodes/%s' % shot._id + patch = { + 'op': 'from-blender', + '$set': { + 'properties.trim_start_in_frames': 123, + 'properties.duration_in_edit_in_frames': 4215, + 'properties.cut_in_timeline_in_frames': 1245, + 'properties.status': 'todo', + } + } + self.patch(url, json=patch, auth_token='token') + + @responses.activate + def test_patch_bad_op(self): + shot = self.create_shot() + self.create_valid_auth_token(ctd.EXAMPLE_PROJECT_OWNER_ID, 'token') + + url = '/api/nodes/%s' % shot._id + patch = {'properties.status': 'todo'} + self.patch(url, json=patch, auth_token='token', expected_status=400) + + @responses.activate + def test_patch_from_blender_bad_fields(self): + shot = self.create_shot() + self.create_valid_auth_token(ctd.EXAMPLE_PROJECT_OWNER_ID, 'token') + + url = '/api/nodes/%s' % shot._id + patch = { + 'op': 'from-blender', + '$set': { + 'invalid.property': 'JE MOEDER', + } + } + self.patch(url, json=patch, auth_token='token', expected_status=400) + + @responses.activate + def test_patch_from_blender_bad_status(self): + shot = self.create_shot() + self.create_valid_auth_token(ctd.EXAMPLE_PROJECT_OWNER_ID, 'token') + + url = '/api/nodes/%s' % shot._id + patch = { + 'op': 'from-blender', + '$set': { + 'properties.status': 'JE MOEDER', + } + } + self.patch(url, json=patch, auth_token='token', expected_status=422) + + @responses.activate + def test_patch_unauthenticated(self): + shot = self.create_shot() + + url = '/api/nodes/%s' % shot._id + patch = { + 'op': 'from-blender', + '$set': { + 'properties.status': 'in_progress', + } + } + self.patch(url, json=patch, expected_status=403) + + @responses.activate + def test_patch_bad_user(self): + shot = self.create_shot() + + self.create_user(24 * 'a') + self.create_valid_auth_token(24 * 'a', 'other') + + url = '/api/nodes/%s' % shot._id + patch = { + 'op': 'from-blender', + '$set': { + 'properties.status': 'in_progress', + } + } + self.patch(url, json=patch, auth_token='other', expected_status=403)