[api]: ckan#662 fix for resubmitting package from api.

Author: David Read

ckan/controllers/apiv1/package.py

@@ -1,3 +1,5 @@
+import copy
+
 from ckan.controllers.rest import RestController
 from ckan.lib.base import _, request, response
 from ckan.lib.cache import ckan_cache
@@ -8,6 +10,11 @@
 
 log = __import__("logging").getLogger(__name__)
 
+readonly_keys = ('id', 'relationships', 'ratings_average',
+                 'ratings_count', 'ckan_url',
+                 'metadata_modified',
+                 'metadata_created')
+
 class PackageController(RestController):
 
     extensions = PluginImplementations(IPackageController)
@@ -59,6 +66,7 @@ def create(self):
         fs = self._get_standard_package_fieldset()
         try:
             request_data = self._get_request_data()
+            request_data = self._strip_readonly_keys(request_data)
             request_fa_dict = ckan.forms.edit_package_dict(ckan.forms.get_package_dict(fs=fs), request_data)
             fs = fs.bind(model.Package, data=request_fa_dict, session=model.Session)
             log.debug('Created object %s' % str(fs.name.value))
@@ -124,6 +132,8 @@ def update(self, id):
             orig_entity_dict = ckan.forms.get_package_dict(pkg=entity, fs=fs)
             try:
                 request_data = self._get_request_data()
+                request_data = self._strip_readonly_keys(request_data,
+                                                         entity.as_dict())
                 request_fa_dict = ckan.forms.edit_package_dict(orig_entity_dict, request_data, id=entity.id)
                 fs = fs.bind(entity, data=request_fa_dict)
                 validation = fs.validate()
@@ -179,3 +189,23 @@ def delete(self, id):
             log.exception(inst)
             raise
         return self._finish_ok()
+
+    def _strip_readonly_keys(self, request_dict, existing_pkg_dict=None):
+        '''Removes keys that are readonly. If there is an existing package,
+        the values of the keys are checked against to see if they have
+        been inadvertantly edited - if so, raise an error.
+        '''
+        stripped_package_dict = copy.deepcopy(request_dict)
+        for key in readonly_keys:
+            if request_dict.has_key(key):
+                if existing_pkg_dict:
+                    if request_dict[key] != existing_pkg_dict.get(key):
+                        raise ckan.forms.PackageDictFormatError(
+                            'Key %r is readonly - do not include in the '
+                            'package or leave it unchanged.')
+                else:
+                    raise ckan.forms.PackageDictFormatError(
+                        'Key %r is readonly - do not include in the '
+                        'package.')                    
+                del stripped_package_dict[key]# = request_dict[key]
+        return stripped_package_dict

ckan/tests/functional/api/model/test_package.py

@@ -84,7 +84,7 @@ def test_register_post_ok(self):
         postparams = '%s=1' % self.dumps(self.package_fixture_data)
         res = self.app.post(offset, params=postparams, status=self.STATUS_409_CONFLICT,
                 extra_environ=self.extra_environ)
-        self.remove()
+        self.remove()        
 
     def test_register_post_bad_request(self):
         test_params = {
@@ -101,6 +101,17 @@ def test_register_post_denied(self):
         postparams = '%s=1' % self.dumps(self.package_fixture_data)
         res = self.app.post(offset, params=postparams, status=self.STATUS_403_ACCESS_DENIED)
 
+    def test_register_post_readonly_fields(self):
+        # (ticket 662) Post a package with readonly field such as 'id'
+        offset = self.offset('/rest/package')
+        data = {'name': u'test_readonly',
+                'id': u'not allowed to be set',
+                }
+        postparams = '%s=1' % self.dumps(data)
+        res = self.app.post(offset, params=postparams,
+                            status=self.STATUS_400_BAD_REQUEST,
+                            extra_environ=self.extra_environ)
+
     def test_entity_get_ok(self):
         package_refs = [self.anna.name, self.anna.id]
         for ref in package_refs:
@@ -122,6 +133,28 @@ def test_entity_get_not_found(self):
         res = self.app.get(offset, status=self.STATUS_404_NOT_FOUND)
         self.remove()
 
+    def test_entity_get_then_post(self):
+        # (ticket 662) Ensure an entity you 'get' from a register can be
+        # returned by posting it back
+        offset = self.package_offset(self.war.name)
+        res = self.app.get(offset, status=self.STATUS_200_OK)
+        data = self.loads(res.body)
+        postparams = '%s=1' % self.dumps(data)
+        res = self.app.post(offset, params=postparams,
+                            status=self.STATUS_200_OK,
+                            extra_environ=self.extra_environ)
+
+    def test_entity_post_changed_readonly(self):
+        # (ticket 662) Edit a readonly field gives error
+        offset = self.package_offset(self.war.name)
+        res = self.app.get(offset, status=self.STATUS_200_OK)
+        data = self.loads(res.body)
+        data['id'] = 'illegally changed value'
+        postparams = '%s=1' % self.dumps(data)
+        res = self.app.post(offset, params=postparams,
+                            status=self.STATUS_400_BAD_REQUEST,
+                            extra_environ=self.extra_environ)
+
     def test_entity_update_denied(self):
         offset = self.anna_offset()
         postparams = '%s=1' % self.dumps(self.package_fixture_data)