From d8a9f37bab6d1db3401117690200c9bf7476437a Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Tue, 20 Aug 2013 16:03:13 +0200 Subject: [PATCH] [#1200] Do not allow writing if url_type != datastore unless force=true is passed --- ckanext/datastore/logic/action.py | 35 ++++++++++++++++++++++---- ckanext/datastore/logic/schema.py | 3 +++ ckanext/datastore/tests/helpers.py | 11 ++++++++ ckanext/datastore/tests/test_create.py | 4 ++- ckanext/datastore/tests/test_delete.py | 4 ++- ckanext/datastore/tests/test_dump.py | 1 + ckanext/datastore/tests/test_search.py | 13 +++++++--- ckanext/datastore/tests/test_upsert.py | 8 +++++- 8 files changed, 68 insertions(+), 11 deletions(-) diff --git a/ckanext/datastore/logic/action.py b/ckanext/datastore/logic/action.py index 489616f49dc..ee2a44e9e4a 100644 --- a/ckanext/datastore/logic/action.py +++ b/ckanext/datastore/logic/action.py @@ -35,6 +35,8 @@ def datastore_create(context, data_dict): :param resource_id: resource id that the data is going to be stored against. :type resource_id: string + :param force: set to True to edit a read-only resource + :type force: bool (optional, default: False) :param resource: resource dictionary that is passed to :meth:`~ckan.logic.action.create.resource_create`. Use instead of ``resource_id`` (optional) @@ -154,6 +156,8 @@ def datastore_upsert(context, data_dict): :param resource_id: resource id that the data is going to be stored under. :type resource_id: string + :param force: set to True to edit a read-only resource + :type force: bool (optional, default: False) :param records: the data, eg: [{"dob": "2005", "some_stuff": ["a","b"]}] (optional) :type records: list of dictionaries :param method: the method to use to put the data into the datastore. @@ -174,6 +178,10 @@ def datastore_upsert(context, data_dict): if errors: raise p.toolkit.ValidationError(errors) + p.toolkit.check_access('datastore_upsert', context, data_dict) + + _check_read_only(context, data_dict) + data_dict['connection_url'] = pylons.config['ckan.datastore.write_url'] res_id = data_dict['resource_id'] @@ -187,8 +195,6 @@ def datastore_upsert(context, data_dict): u'Resource "{0}" was not found.'.format(res_id) )) - p.toolkit.check_access('datastore_upsert', context, data_dict) - result = db.upsert(context, data_dict) result.pop('id', None) result.pop('connection_url') @@ -200,6 +206,8 @@ def datastore_delete(context, data_dict): :param resource_id: resource id that the data will be deleted from. (optional) :type resource_id: string + :param force: set to True to edit a read-only resource + :type force: bool (optional, default: False) :param filters: filters to apply before deleting (eg {"name": "fred"}). If missing delete whole table and all dependent views. (optional) :type filters: dictionary @@ -218,6 +226,10 @@ def datastore_delete(context, data_dict): if errors: raise p.toolkit.ValidationError(errors) + p.toolkit.check_access('datastore_delete', context, data_dict) + + _check_read_only(context, data_dict) + data_dict['connection_url'] = pylons.config['ckan.datastore.write_url'] res_id = data_dict['resource_id'] @@ -231,8 +243,6 @@ def datastore_delete(context, data_dict): u'Resource "{0}" was not found.'.format(res_id) )) - p.toolkit.check_access('datastore_delete', context, data_dict) - result = db.delete(context, data_dict) result.pop('id', None) result.pop('connection_url') @@ -431,7 +441,7 @@ def datastore_make_public(context, data_dict): def _resource_exists(context, data_dict): - # Returns true if the resource exists in CKAN and in the datastore + ''' Returns true if the resource exists in CKAN and in the datastore ''' model = _get_or_bust(context, 'model') res_id = _get_or_bust(data_dict, 'resource_id') if not model.Resource.get(res_id): @@ -441,3 +451,18 @@ def _resource_exists(context, data_dict): WHERE name = :id AND alias_of IS NULL''') results = db._get_engine(data_dict).execute(resources_sql, id=res_id) return results.rowcount > 0 + + +def _check_read_only(context, data_dict): + ''' Raises exception if the resource is read-only. + Make sure the resource id is in resource_id + ''' + if data_dict.get('force'): + return + res = p.toolkit.get_action('resource_show')( + context, {'id': data_dict['resource_id']}) + if res.get('url_type') != 'datastore': + raise p.toolkit.ValidationError({ + 'read-only': ['Cannot edit read-only resource. Either pass' + '"force=True" or change url-type to "datastore"'] + }) diff --git a/ckanext/datastore/logic/schema.py b/ckanext/datastore/logic/schema.py index 018eb249d79..6f0a74723ca 100644 --- a/ckanext/datastore/logic/schema.py +++ b/ckanext/datastore/logic/schema.py @@ -68,6 +68,7 @@ def json_validator(value, context): def datastore_create_schema(): schema = { 'resource_id': [ignore_missing, unicode, resource_id_exists], + 'force': [ignore_missing, boolean_validator], 'id': [ignore_missing], 'aliases': [ignore_missing, list_of_strings_or_string], 'fields': { @@ -85,6 +86,7 @@ def datastore_create_schema(): def datastore_upsert_schema(): schema = { 'resource_id': [not_missing, not_empty, unicode], + 'force': [ignore_missing, boolean_validator], 'id': [ignore_missing], 'method': [ignore_missing, unicode, OneOf( ['upsert', 'insert', 'update'])], @@ -97,6 +99,7 @@ def datastore_upsert_schema(): def datastore_delete_schema(): schema = { 'resource_id': [not_missing, not_empty, unicode], + 'force': [ignore_missing, boolean_validator], 'id': [ignore_missing], '__junk': [empty], '__before': [rename('id', 'resource_id')] diff --git a/ckanext/datastore/tests/helpers.py b/ckanext/datastore/tests/helpers.py index cf83f57378a..3ee89cdda20 100644 --- a/ckanext/datastore/tests/helpers.py +++ b/ckanext/datastore/tests/helpers.py @@ -1,6 +1,8 @@ import ckan.model as model import ckan.lib.cli as cli +import ckan.plugins as p + def extract(d, keys): return dict((k, d[k]) for k in keys if k in d) @@ -29,3 +31,12 @@ def rebuild_all_dbs(Session): model.repo.tables_created_and_initialised = False clear_db(Session) model.repo.rebuild_db() + + +def set_url_type(resources, user): + context = {'user': user.name} + for resource in resources: + resource = p.toolkit.get_action('resource_show')( + context, {'id': resource.id}) + resource['url_type'] = 'datastore' + p.toolkit.get_action('resource_update')(context, resource) diff --git a/ckanext/datastore/tests/test_create.py b/ckanext/datastore/tests/test_create.py index 7a6933008e1..f95e8044951 100644 --- a/ckanext/datastore/tests/test_create.py +++ b/ckanext/datastore/tests/test_create.py @@ -17,7 +17,7 @@ import ckan.config.middleware as middleware import ckanext.datastore.db as db -from ckanext.datastore.tests.helpers import rebuild_all_dbs +from ckanext.datastore.tests.helpers import rebuild_all_dbs, set_url_type # avoid hanging tests https://github.com/gabrielfalcao/HTTPretty/issues/34 @@ -45,6 +45,8 @@ def setup_class(cls): engine = db._get_engine( {'connection_url': pylons.config['ckan.datastore.write_url']}) cls.Session = orm.scoped_session(orm.sessionmaker(bind=engine)) + set_url_type( + model.Package.get('annakarenina').resources, cls.sysadmin_user) @classmethod def teardown_class(cls): diff --git a/ckanext/datastore/tests/test_delete.py b/ckanext/datastore/tests/test_delete.py index ce1b02efdb1..bb7db39217a 100644 --- a/ckanext/datastore/tests/test_delete.py +++ b/ckanext/datastore/tests/test_delete.py @@ -11,7 +11,7 @@ import ckan.tests as tests import ckanext.datastore.db as db -from ckanext.datastore.tests.helpers import rebuild_all_dbs +from ckanext.datastore.tests.helpers import rebuild_all_dbs, set_url_type class TestDatastoreDelete(tests.WsgiAppCase): @@ -43,6 +43,8 @@ def setup_class(cls): engine = db._get_engine( {'connection_url': pylons.config['ckan.datastore.write_url']}) cls.Session = orm.scoped_session(orm.sessionmaker(bind=engine)) + set_url_type( + model.Package.get('annakarenina').resources, cls.sysadmin_user) @classmethod def teardown_class(cls): diff --git a/ckanext/datastore/tests/test_dump.py b/ckanext/datastore/tests/test_dump.py index 041105af986..6061535f4ba 100644 --- a/ckanext/datastore/tests/test_dump.py +++ b/ckanext/datastore/tests/test_dump.py @@ -32,6 +32,7 @@ def setup_class(cls): resource = model.Package.get('annakarenina').resources[0] cls.data = { 'resource_id': resource.id, + 'force': True, 'aliases': 'books', 'fields': [{'id': u'b\xfck', 'type': 'text'}, {'id': 'author', 'type': 'text'}, diff --git a/ckanext/datastore/tests/test_search.py b/ckanext/datastore/tests/test_search.py index 21cb19de4e2..1b7c9486da7 100644 --- a/ckanext/datastore/tests/test_search.py +++ b/ckanext/datastore/tests/test_search.py @@ -30,6 +30,7 @@ def setup_class(cls): cls.resource = cls.dataset.resources[0] cls.data = { 'resource_id': cls.resource.id, + 'force': True, 'aliases': 'books3', 'fields': [{'id': u'b\xfck', 'type': 'text'}, {'id': 'author', 'type': 'text'}, @@ -116,7 +117,7 @@ def test_search_private_dataset(self): context, {'name': 'privatedataset', 'private': True, - 'owner_org' : self.organization['id'], + 'owner_org': self.organization['id'], 'groups': [{ 'id': group.id }]}) @@ -128,6 +129,7 @@ def test_search_private_dataset(self): postparams = '%s=1' % json.dumps({ 'resource_id': resource['id'], + 'force': True }) auth = {'Authorization': str(self.sysadmin_user.apikey)} res = self.app.post('/api/action/datastore_create', params=postparams, @@ -425,6 +427,7 @@ def setup_class(cls): resource = model.Package.get('annakarenina').resources[0] cls.data = dict( resource_id=resource.id, + force=True, fields=[ {'id': 'id'}, {'id': 'date', 'type':'date'}, @@ -499,6 +502,7 @@ def setup_class(cls): resource = cls.dataset.resources[0] cls.data = { 'resource_id': resource.id, + 'force': True, 'aliases': 'books4', 'fields': [{'id': u'b\xfck', 'type': 'text'}, {'id': 'author', 'type': 'text'}, @@ -517,7 +521,7 @@ def setup_class(cls): extra_environ=auth) res_dict = json.loads(res.body) assert res_dict['success'] is True - + # Make an organization, because private datasets must belong to one. cls.organization = tests.call_action_api( cls.app, 'organization_create', @@ -669,6 +673,7 @@ def test_new_datastore_table_from_private_resource(self): postparams = '%s=1' % json.dumps({ 'resource_id': resource['id'], + 'force': True }) auth = {'Authorization': str(self.sysadmin_user.apikey)} res = self.app.post('/api/action/datastore_create', params=postparams, @@ -708,7 +713,9 @@ def test_making_resource_private_makes_datastore_private(self): 'package_id': package['id']}) postparams = '%s=1' % json.dumps({ - 'resource_id': resource['id']}) + 'resource_id': resource['id'], + 'force': True + }) auth = {'Authorization': str(self.sysadmin_user.apikey)} res = self.app.post('/api/action/datastore_create', params=postparams, extra_environ=auth) diff --git a/ckanext/datastore/tests/test_upsert.py b/ckanext/datastore/tests/test_upsert.py index e27f8b9c523..9d65700198a 100644 --- a/ckanext/datastore/tests/test_upsert.py +++ b/ckanext/datastore/tests/test_upsert.py @@ -11,7 +11,7 @@ import ckan.tests as tests import ckanext.datastore.db as db -from ckanext.datastore.tests.helpers import rebuild_all_dbs +from ckanext.datastore.tests.helpers import rebuild_all_dbs, set_url_type class TestDatastoreUpsert(tests.WsgiAppCase): @@ -26,6 +26,8 @@ def setup_class(cls): ctd.CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') cls.normal_user = model.User.get('annafan') + set_url_type( + model.Package.get('annakarenina').resources, cls.sysadmin_user) resource = model.Package.get('annakarenina').resources[0] cls.data = { 'resource_id': resource.id, @@ -249,6 +251,8 @@ def setup_class(cls): ctd.CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') cls.normal_user = model.User.get('annafan') + set_url_type( + model.Package.get('annakarenina').resources, cls.sysadmin_user) resource = model.Package.get('annakarenina').resources[0] cls.data = { 'resource_id': resource.id, @@ -349,6 +353,8 @@ def setup_class(cls): ctd.CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') cls.normal_user = model.User.get('annafan') + set_url_type( + model.Package.get('annakarenina').resources, cls.sysadmin_user) resource = model.Package.get('annakarenina').resources[0] hhguide = u"hitchhiker's guide to the galaxy" cls.data = {