Handle operation for patch separately

Patch remove operation is now able to remove element from list and dict,
Added more test on new changes

committer: Ralf Schlatterbeck <[email protected]>

Author: kinggreedy

roundup/rest.py

@@ -12,6 +12,7 @@
 import sys
 import time
 import traceback
+import xml
 from roundup import hyperdb
 from roundup.exceptions import *
 
@@ -223,6 +224,59 @@ def error_obj(self, status, msg, source=None):
 
         return result
 
+    def patch_data(self, op, old_val, new_val):
+        """Perform patch operation based on old_val and new_val
+
+        Args:
+            op (string): PATCH operation: add, replace, remove
+            old_val: old value of the property
+            new_val: new value of the property
+
+        Returns:
+            result (string): value after performed the operation
+        """
+        # add operation: If neither of the value is None, use the other one
+        #                Otherwise, concat those 2 value
+        if op == 'add':
+            if old_val is None:
+                result = new_val
+            elif new_val is None:
+                result = old_val
+            else:
+                result = old_val + new_val
+        # Replace operation: new value is returned
+        elif op == 'replace':
+            result = new_val
+        # Remove operation:
+        #   if old_val is not a list/dict, change it to None
+        #   if old_val is a list/dict, but the parameter is empty,
+        #       change it to none
+        #   if old_val is a list/dict, and parameter is not empty
+        #       proceed to remove the values from parameter from the list/dict
+        elif op == 'remove':
+            if isinstance(old_val, list):
+                if new_val is None:
+                    result = []
+                elif isinstance(new_val, list):
+                    result = [x for x in old_val if x not in new_val]
+                else:
+                    if new_val in old_val:
+                        old_val.remove(new_val)
+            elif isinstance(old_val, dict):
+                if new_val is None:
+                    result = {}
+                elif isinstance(new_val, dict):
+                    for x in new_val:
+                        old_val.pop(x, None)
+                else:
+                    old_val.pop(new_val, None)
+            else:
+                result = None
+        else:
+            raise UsageError('PATCH Operation %s is not allowed' % op)
+
+        return result
+
     @_data_decorator
     def get_collection(self, class_name, input):
         """GET resource from class URI.
@@ -704,18 +758,9 @@ def patch_element(self, class_name, item_id, input):
                     (prop, class_name, item_id)
                 )
 
-            if op == 'add':
-                props[prop] = class_obj.get(item_id, prop) + props[prop]
-            elif op == 'replace':
-                pass
-            elif op == 'remove':
-                current_prop = class_obj.get(item_id, prop)
-                if isinstance(current_prop, list):
-                    props[prop] = []
-                else:
-                    props[prop] = None
-            else:
-                raise UsageError('PATCH Operation %s is not allowed' % op)
+            props[prop] = self.patch_data(
+                op, class_obj.get(item_id, prop), props[prop]
+            )
 
         try:
             result = class_obj.set(item_id, **props)
@@ -776,18 +821,9 @@ def patch_attribute(self, class_name, item_id, attr_name, input):
             )
         }
 
-        if op == 'add':
-            props[prop] = class_obj.get(item_id, prop) + props[prop]
-        elif op == 'replace':
-            pass
-        elif op == 'remove':
-            current_prop = class_obj.get(item_id, prop)
-            if isinstance(current_prop, list):
-                props[prop] = []
-            else:
-                props[prop] = None
-        else:
-            raise UsageError('PATCH Operation %s is not allowed' % op)
+        props[prop] = self.patch_data(
+            op, class_obj.get(item_id, prop), props[prop]
+        )
 
         try:
             result = class_obj.set(item_id, **props)

test/test_rest.py

@@ -440,7 +440,7 @@ def testPatchRemoveAll(self):
         """
         Test Patch Action 'Remove'
         """
-        # create a new issue with userid 1 in the nosy list
+        # create a new issue with userid 1 and 2 in the nosy list
         issue_id = self.db.issue.create(title='foo', nosy=['1', '2'])
 
         # remove the nosy list and the title
@@ -461,6 +461,29 @@ def testPatchRemoveAll(self):
         self.assertEqual(len(results['attributes']['nosy']), 0)
         self.assertEqual(results['attributes']['nosy'], [])
 
+    def testPatchRemove(self):
+        """
+        Test Patch Action 'Remove' only some element from a list
+        """
+        # create a new issue with userid 1, 2, 3 in the nosy list
+        issue_id = self.db.issue.create(title='foo', nosy=['1', '2', '3'])
+
+        # remove the nosy list and the title
+        form = cgi.FieldStorage()
+        form.list = [
+            cgi.MiniFieldStorage('op', 'remove'),
+            cgi.MiniFieldStorage('nosy', '1, 2'),
+        ]
+        results = self.server.patch_element('issue', issue_id, form)
+        self.assertEqual(self.dummy_client.response_code, 200)
+
+        # verify the result
+        results = self.server.get_element('issue', issue_id, self.empty_form)
+        results = results['data']
+        self.assertEqual(self.dummy_client.response_code, 200)
+        self.assertEqual(len(results['attributes']['nosy']), 1)
+        self.assertEqual(results['attributes']['nosy'], ['3'])
+
 
 def get_obj(path, id):
     return {