Skip to content

Commit

Permalink
[#1200] Do not allow writing if url_type != datastore unless force=tr…
Browse files Browse the repository at this point in the history
…ue is passed
  • Loading branch information
domoritz committed Aug 20, 2013
1 parent 09bde04 commit d8a9f37
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 11 deletions.
35 changes: 30 additions & 5 deletions ckanext/datastore/logic/action.py
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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']
Expand All @@ -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')
Expand All @@ -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
Expand All @@ -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']
Expand All @@ -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')
Expand Down Expand Up @@ -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):
Expand All @@ -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"']
})
3 changes: 3 additions & 0 deletions ckanext/datastore/logic/schema.py
Expand Up @@ -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': {
Expand All @@ -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'])],
Expand All @@ -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')]
Expand Down
11 changes: 11 additions & 0 deletions 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)
Expand Down Expand Up @@ -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)
4 changes: 3 additions & 1 deletion ckanext/datastore/tests/test_create.py
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
4 changes: 3 additions & 1 deletion ckanext/datastore/tests/test_delete.py
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions ckanext/datastore/tests/test_dump.py
Expand Up @@ -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'},
Expand Down
13 changes: 10 additions & 3 deletions ckanext/datastore/tests/test_search.py
Expand Up @@ -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'},
Expand Down Expand Up @@ -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
}]})
Expand All @@ -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,
Expand Down Expand Up @@ -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'},
Expand Down Expand Up @@ -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'},
Expand All @@ -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',
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 7 additions & 1 deletion ckanext/datastore/tests/test_upsert.py
Expand Up @@ -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):
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 = {
Expand Down

0 comments on commit d8a9f37

Please sign in to comment.