From f835bed50f34653c5226ed6da1feddc373cbfb4d Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 24 Jan 2012 18:40:55 +0100 Subject: [PATCH 01/72] [#1705] Begin implementing vocabularies model Add new domain class ckan/model/vocabulary.py:Vocabulary, maps to new vocabulary table in db. Add vocabulary_dictize() and vocabulary_list_dictize() functions to model_dictize.py. Add vocabulary_dict_save() function to model_save.py Add vocabulary_create() logic function to logic/action/create.py. Needs authorization. Add vocabulary_list() and vocabulary_show() logic functions to logic/action/get.py. These need authorization as well. Add 'vocabulary_id' column to tag. Add the beginnings of tests for the new vocabulary model in ckan/tests/functional/api/model/test_vocabulary.py. --- ckan/lib/dictization/model_dictize.py | 9 +++ ckan/lib/dictization/model_save.py | 11 +++ ckan/logic/action/create.py | 19 +++++- ckan/logic/action/get.py | 29 +++++++- ckan/model/__init__.py | 1 + ckan/model/vocabulary.py | 29 ++++++++ .../functional/api/model/test_vocabulary.py | 68 +++++++++++++++++++ 7 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 ckan/model/vocabulary.py create mode 100644 ckan/tests/functional/api/model/test_vocabulary.py diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 7a9a71c79c0..43826b89092 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -441,3 +441,12 @@ def package_to_api2(pkg, context): dictized['relationships'] = relationships return dictized +def vocabulary_dictize(vocabulary, context): + vocabulary_dict = table_dictize(vocabulary, context) + return vocabulary_dict + +def vocabulary_list_dictize(vocabulary_list, context): + list_of_dicts = [] + for vocabulary in vocabulary_list: + list_of_dicts.append(vocabulary_dictize(vocabulary, context)) + return list_of_dicts diff --git a/ckan/lib/dictization/model_save.py b/ckan/lib/dictization/model_save.py index aaa4569fc12..14eb6c5464a 100644 --- a/ckan/lib/dictization/model_save.py +++ b/ckan/lib/dictization/model_save.py @@ -440,3 +440,14 @@ def task_status_dict_save(task_status_dict, context): task_status = table_dict_save(task_status_dict, model.TaskStatus, context) return task_status + +def vocabulary_dict_save(vocabulary_dict, context): + + model = context['model'] + session = context['session'] + + vocabulary_name = vocabulary_dict['name'] + vocabulary_obj = model.Vocabulary(vocabulary_name) + session.add(vocabulary_obj) + + return vocabulary_obj diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 48dc13d4567..e0bee99198c 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -16,12 +16,13 @@ group_dict_save, package_api_to_dict, package_dict_save, - user_dict_save) + user_dict_save, + vocabulary_dict_save) from ckan.lib.dictization.model_dictize import (group_dictize, package_dictize, - user_dictize) - + user_dictize, + vocabulary_dictize) from ckan.logic.schema import default_create_package_schema, default_resource_schema @@ -278,3 +279,15 @@ def group_create_rest(context, data_dict): return group_dict +def vocabulary_create(context, data_dict): + '''Add a new vocabulary to the site.''' + + model = context['model'] + vocabulary_dict = data_dict['vocabulary'] + vocabulary = vocabulary_dict_save(vocabulary_dict, context) + + if not context.get('defer_commit'): + model.repo.commit() + + log.debug("Created vocabulary '%s'" % vocabulary.name) + return vocabulary_dictize(vocabulary, context) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index ab2d4e2a48c..2812cdd76dd 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -20,7 +20,9 @@ group_list_dictize, tag_dictize, task_status_dictize, - user_dictize) + user_dictize, + vocabulary_dictize, + vocabulary_list_dictize) from ckan.lib.dictization.model_dictize import (package_to_api1, package_to_api2, @@ -28,6 +30,7 @@ group_to_api2, tag_to_api1, tag_to_api2) + from ckan.lib.search import query_for, SearchError import logging @@ -872,3 +875,27 @@ def status_show(context, data_dict): 'locale_default': config.get('ckan.locale_default'), 'extensions': config.get('ckan.plugins').split(), } + +def vocabulary_list(context, data_dict): + model = context['model'] + vocabulary_objects = model.Session.query(model.Vocabulary).all() + return vocabulary_list_dictize(vocabulary_objects, context) + +def vocabulary_show(context, data_dict): + model = context['model'] + + if data_dict.has_key('id'): + vocabulary = model.Vocabulary.get(data_dict['id']) + if vocabulary is None: + raise NotFound + elif data_dict.has_key('name'): + vocabulary = model.Vocabulary.get(data_dict['name']) + if vocabulary is None: + raise NotFound + else: + raise NotFound + + # TODO: check_access('vocabulary_show', context, data_dict) + + vocabulary_dict = vocabulary_dictize(vocabulary, context) + return vocabulary_dict diff --git a/ckan/model/__init__.py b/ckan/model/__init__.py index cd0ba3f03c1..36509fe3e20 100644 --- a/ckan/model/__init__.py +++ b/ckan/model/__init__.py @@ -23,6 +23,7 @@ from rating import * from package_relationship import * from task_status import * +from vocabulary import * import ckan.migration from ckan.lib.helpers import OrderedDict, datetime_to_date_str from vdm.sqlalchemy.base import SQLAlchemySession diff --git a/ckan/model/vocabulary.py b/ckan/model/vocabulary.py new file mode 100644 index 00000000000..45ff3603e19 --- /dev/null +++ b/ckan/model/vocabulary.py @@ -0,0 +1,29 @@ +from meta import Table, types, Session +from core import metadata, Column, DomainObject, mapper +from types import make_uuid + +MAX_VOCAB_NAME_LENGTH = 100 + +vocabulary_table = Table( + 'vocabulary', metadata, + Column('id', types.UnicodeText, primary_key=True, default=make_uuid), + Column('name', types.Unicode(MAX_VOCAB_NAME_LENGTH), nullable=False, unique=True), + ) + +class Vocabulary(DomainObject): + + def __init__(self, name): + self.id = make_uuid() + self.name = name + + @classmethod + def get(cls, reference): + """Return a Vocabulary object referenced by its id or name.""" + + query = Session.query(cls).filter(cls.id==reference) + vocab = query.first() + if vocab is None: + vocab = cls.by_name(reference) + return vocab + +mapper(Vocabulary, vocabulary_table) diff --git a/ckan/tests/functional/api/model/test_vocabulary.py b/ckan/tests/functional/api/model/test_vocabulary.py new file mode 100644 index 00000000000..276f583bad8 --- /dev/null +++ b/ckan/tests/functional/api/model/test_vocabulary.py @@ -0,0 +1,68 @@ +import ckan +from pylons.test import pylonsapp +import paste.fixture +from ckan.lib.helpers import json + +class TestVocabulary(object): + + @classmethod + def setup_class(cls): + cls.app = paste.fixture.TestApp(pylonsapp) + ckan.tests.CreateTestData.create() + + @classmethod + def teardown_class(cls): + ckan.model.repo.rebuild_db() + + def post(self, url, param_dict=None): + if param_dict is None: + param_dict = {} + param_string = json.dumps(param_dict) + response = self.app.post(url, params=param_string) + assert not response.errors + return response.json + + def test_vocabulary_create(self): + + # Create a new vocabulary. + vocab_name = "Genre" + params = {'vocabulary': {'name': vocab_name}} + response = self.post('/api/action/vocabulary_create', params) + # Check the values of the response. + assert response['success'] == True + assert response['result'] + created_vocab = response['result'] + assert created_vocab['name'] == vocab_name + assert created_vocab['id'] + + # Get the list of vocabularies. + response = self.post('/api/action/vocabulary_list') + # Check that the vocabulary we created is in the list. + assert response['success'] == True + assert response['result'] + assert response['result'].count(created_vocab) == 1 + + # Get the created vocabulary. + params = {'id': created_vocab['id']} + response = self.post('/api/action/vocabulary_show', params) + # Check that retrieving the vocab by name gives the same result. + by_name_params = {'name': created_vocab['name']} + assert response == self.post('/api/action/vocabulary_show', + by_name_params) + # Check that it matches what we created. + assert response['success'] == True + assert response['result'] == created_vocab + + def test_vocabulary_update(self): + # Create a vocab. + # Update the vocab via the API. + # List vocabs + # Get vocab, assert fields correct. + raise NotImplementedError + + def test_vocabulary_delete(self): + # Create a vocab. + # Delete a vocab via the API + # List vocabs, assert that it's gone. + raise NotImplementedError + From 3a5cc5e356b404fec8e1bf923a766cd61be2b3e7 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Thu, 26 Jan 2012 17:43:29 +0100 Subject: [PATCH 02/72] [#1705] Finish up Vocabulary model, API and tests Add validation and authorization to vocabulary_create() logic action function. Add new vocabulary_update() and vocabulary_delete() logic action functions. Add vocabulary_create(), vocabulary_update() and vocabulary_delete() logic auth functions. These just authorize sysadmins only. Add default_create_vocabulary_schema() and default_update_vocabulary_schema(), and vocabulary_name_validator() and vocabulary_id_not_changed() validators. Add tests for creating, updating, deleting, listing and getting vocabularies via the API. --- ckan/lib/dictization/model_save.py | 10 + ckan/logic/action/create.py | 32 +- ckan/logic/action/delete.py | 18 ++ ckan/logic/action/update.py | 43 ++- ckan/logic/auth/create.py | 4 + ckan/logic/auth/delete.py | 4 + ckan/logic/auth/update.py | 4 + ckan/logic/schema.py | 21 +- ckan/logic/validators.py | 27 +- ckan/model/vocabulary.py | 6 +- .../functional/api/model/test_vocabulary.py | 299 ++++++++++++++++-- 11 files changed, 429 insertions(+), 39 deletions(-) diff --git a/ckan/lib/dictization/model_save.py b/ckan/lib/dictization/model_save.py index 14eb6c5464a..ecdad95287e 100644 --- a/ckan/lib/dictization/model_save.py +++ b/ckan/lib/dictization/model_save.py @@ -451,3 +451,13 @@ def vocabulary_dict_save(vocabulary_dict, context): session.add(vocabulary_obj) return vocabulary_obj + +def vocabulary_dict_update(vocabulary_dict, context): + + model = context['model'] + session = context['session'] + + vocabulary_obj = model.Vocabulary.get(vocabulary_dict['id']) + vocabulary_obj.name = vocabulary_dict['name'] + + return vocabulary_obj diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index e0bee99198c..ce06e8ed419 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -24,7 +24,9 @@ user_dictize, vocabulary_dictize) -from ckan.logic.schema import default_create_package_schema, default_resource_schema +from ckan.logic.schema import (default_create_package_schema, + default_resource_schema, + default_create_vocabulary_schema) from ckan.logic.schema import default_group_schema, default_user_schema from ckan.lib.navl.dictization_functions import validate @@ -280,14 +282,34 @@ def group_create_rest(context, data_dict): return group_dict def vocabulary_create(context, data_dict): - '''Add a new vocabulary to the site.''' model = context['model'] - vocabulary_dict = data_dict['vocabulary'] - vocabulary = vocabulary_dict_save(vocabulary_dict, context) + user = context['user'] + schema = context.get('schema') or default_create_vocabulary_schema() + + model.Session.remove() + model.Session()._context = context + + check_access('vocabulary_create', context, data_dict) + + data, errors = validate(data_dict, schema, context) + + if errors: + model.Session.rollback() + raise ValidationError(errors, package_error_summary(errors)) + + rev = model.repo.new_revision() + rev.author = user + if 'message' in context: + rev.message = context['message'] + else: + rev.message = _(u'API: Create Vocabulary %s') % data.get('name') + + vocabulary = vocabulary_dict_save(data, context) if not context.get('defer_commit'): model.repo.commit() - log.debug("Created vocabulary '%s'" % vocabulary.name) + log.debug('Created Vocabulary %s' % str(vocabulary.name)) + return vocabulary_dictize(vocabulary, context) diff --git a/ckan/logic/action/delete.py b/ckan/logic/action/delete.py index 498e70f7f32..ebb25547d05 100644 --- a/ckan/logic/action/delete.py +++ b/ckan/logic/action/delete.py @@ -103,3 +103,21 @@ def task_status_delete(context, data_dict): entity.delete() model.Session.commit() + +def vocabulary_delete(context, data_dict): + model = context['model'] + user = context['user'] + vocab_id = data_dict['id'] + + vocab_obj = model.Vocabulary.get(vocab_id) + if vocab_obj is None: + raise NotFound + + check_access('vocabulary_delete', context, data_dict) + + rev = model.repo.new_revision() + rev.author = user + rev.message = _(u'REST API: Delete Vocabulary: %s') % vocab_obj.name + + vocab_obj.delete() + model.repo.commit() diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 4d913365384..8e9888cb49d 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -16,19 +16,22 @@ group_dictize, group_to_api1, group_to_api2, - user_dictize) + user_dictize, + vocabulary_dictize) from ckan.lib.dictization.model_save import (group_api_to_dict, package_api_to_dict, group_dict_save, user_dict_save, task_status_dict_save, package_dict_save, - resource_dict_save) + resource_dict_save, + vocabulary_dict_update) from ckan.logic.schema import (default_update_group_schema, default_update_package_schema, default_update_user_schema, default_update_resource_schema, - default_task_status_schema) + default_task_status_schema, + default_update_vocabulary_schema) from ckan.lib.navl.dictization_functions import validate log = logging.getLogger(__name__) @@ -470,3 +473,37 @@ def group_update_rest(context, data_dict): group_dict = group_to_api2(group, context) return group_dict + +def vocabulary_update(context, data_dict): + model = context['model'] + user = context['user'] + vocab_id = data_dict['id'] + + model.Session.remove() + model.Session()._context = context + + vocab = model.Vocabulary.get(vocab_id) + if vocab is None: + raise NotFound(_('Vocabulary was not found.')) + + check_access('vocabulary_update', context, data_dict) + + schema = context.get('schema') or default_update_vocabulary_schema() + data, errors = validate(data_dict, schema, context) + + if errors: + model.Session.rollback() + raise ValidationError(errors) + + rev = model.repo.new_revision() + rev.author = user + if 'message' in context: + rev.message = context['message'] + else: + rev.message = _(u'API: Update Vocabulary %s') % data.get('name') + + updated_vocab = vocabulary_dict_update(data, context) + + if not context.get('defer_commit'): + model.repo.commit() + return vocabulary_dictize(updated_vocab, context) diff --git a/ckan/logic/auth/create.py b/ckan/logic/auth/create.py index 80702bf176f..5840097e3f0 100644 --- a/ckan/logic/auth/create.py +++ b/ckan/logic/auth/create.py @@ -129,3 +129,7 @@ def group_create_rest(context, data_dict): return {'success': False, 'msg': _('Valid API key needed to create a group')} return group_create(context, data_dict) + +def vocabulary_create(context, data_dict): + user = context['user'] + return {'success': Authorizer.is_sysadmin(user)} diff --git a/ckan/logic/auth/delete.py b/ckan/logic/auth/delete.py index 7dd762fc2e8..0e54c4844b0 100644 --- a/ckan/logic/auth/delete.py +++ b/ckan/logic/auth/delete.py @@ -55,3 +55,7 @@ def task_status_delete(context, data_dict): return {'success': False, 'msg': _('User %s not authorized to delete task_status') % str(user)} else: return {'success': True} + +def vocabulary_delete(context, data_dict): + user = context['user'] + return {'success': Authorizer.is_sysadmin(user)} diff --git a/ckan/logic/auth/update.py b/ckan/logic/auth/update.py index b2b15fc6831..54a4230560c 100644 --- a/ckan/logic/auth/update.py +++ b/ckan/logic/auth/update.py @@ -159,6 +159,10 @@ def task_status_update(context, data_dict): else: return {'success': True} +def vocabulary_update(context, data_dict): + user = context['user'] + return {'success': Authorizer.is_sysadmin(user)} + ## Modifications for rest api def package_update_rest(context, data_dict): diff --git a/ckan/logic/schema.py b/ckan/logic/schema.py index 3142df75a95..8cbeef63ad2 100644 --- a/ckan/logic/schema.py +++ b/ckan/logic/schema.py @@ -30,7 +30,9 @@ user_password_not_empty, isodate, int_validator, - user_about_validator) + user_about_validator, + vocabulary_name_validator, + vocabulary_id_not_changed) from formencode.validators import OneOf import ckan.model @@ -265,3 +267,20 @@ def default_task_status_schema(): 'error': [ignore_missing] } return schema + +def default_vocabulary_schema(): + schema = { + 'id': [ignore_empty, ignore_missing, unicode], + 'name': [not_empty, unicode, vocabulary_name_validator], + } + return schema + +def default_create_vocabulary_schema(): + schema = default_vocabulary_schema() + schema['id'] = [empty] + return schema + +def default_update_vocabulary_schema(): + schema = default_vocabulary_schema() + schema['id'] = schema['id'] + [vocabulary_id_not_changed] + return schema diff --git a/ckan/logic/validators.py b/ckan/logic/validators.py index 08aba5fdf8a..f93586f7c42 100644 --- a/ckan/logic/validators.py +++ b/ckan/logic/validators.py @@ -7,7 +7,9 @@ from ckan.lib.helpers import date_str_to_datetime from ckan.model import (MAX_TAG_LENGTH, MIN_TAG_LENGTH, PACKAGE_NAME_MIN_LENGTH, PACKAGE_NAME_MAX_LENGTH, - PACKAGE_VERSION_MAX_LENGTH) + PACKAGE_VERSION_MAX_LENGTH, + VOCABULARY_NAME_MAX_LENGTH, + VOCABULARY_NAME_MIN_LENGTH) def package_id_not_changed(value, context): @@ -318,3 +320,26 @@ def user_about_validator(value,context): raise Invalid(_('Edit not allowed as it looks like spam. Please avoid links in your description.')) return value + +def vocabulary_name_validator(name, context): + model = context['model'] + session = context['session'] + + if len(name) < VOCABULARY_NAME_MIN_LENGTH: + raise Invalid(_('Name must be at least %s characters long') % + VOCABULARY_NAME_MIN_LENGTH) + if len(name) > VOCABULARY_NAME_MAX_LENGTH: + raise Invalid(_('Name must be a maximum of %i characters long') % + VOCABULARY_NAME_MAX_LENGTH) + query = session.query(model.Vocabulary.name).filter_by(name=name) + result = query.first() + if result: + raise Invalid(_('That vocabulary name is already in use.')) + return name + +def vocabulary_id_not_changed(value, context): + vocabulary = context.get('vocabulary') + if vocabulary and value != vocabulary.id: + raise Invalid(_('Cannot change value of key from %s to %s. ' + 'This key is read-only') % (vocabulary.id, value)) + return value diff --git a/ckan/model/vocabulary.py b/ckan/model/vocabulary.py index 45ff3603e19..c1d5b27d750 100644 --- a/ckan/model/vocabulary.py +++ b/ckan/model/vocabulary.py @@ -2,12 +2,14 @@ from core import metadata, Column, DomainObject, mapper from types import make_uuid -MAX_VOCAB_NAME_LENGTH = 100 +VOCABULARY_NAME_MIN_LENGTH = 2 +VOCABULARY_NAME_MAX_LENGTH = 100 vocabulary_table = Table( 'vocabulary', metadata, Column('id', types.UnicodeText, primary_key=True, default=make_uuid), - Column('name', types.Unicode(MAX_VOCAB_NAME_LENGTH), nullable=False, unique=True), + Column('name', types.Unicode(VOCABULARY_NAME_MAX_LENGTH), nullable=False, + unique=True), ) class Vocabulary(DomainObject): diff --git a/ckan/tests/functional/api/model/test_vocabulary.py b/ckan/tests/functional/api/model/test_vocabulary.py index 276f583bad8..cd61e21ae06 100644 --- a/ckan/tests/functional/api/model/test_vocabulary.py +++ b/ckan/tests/functional/api/model/test_vocabulary.py @@ -2,32 +2,46 @@ from pylons.test import pylonsapp import paste.fixture from ckan.lib.helpers import json +import sqlalchemy +from nose.tools import raises, assert_raises class TestVocabulary(object): - @classmethod - def setup_class(cls): - cls.app = paste.fixture.TestApp(pylonsapp) + def setup(self): + self.app = paste.fixture.TestApp(pylonsapp) ckan.tests.CreateTestData.create() + self.sysadmin_user = ckan.model.User.get('testsysadmin') + self.normal_user = ckan.model.User.get('annafan') + # Make a couple of test vocabularies needed later. + self.genre_vocab = self._create_vocabulary(vocab_name="Genre", + user=self.sysadmin_user) + self.timeperiod_vocab = self._create_vocabulary( + vocab_name="Time Period", user=self.sysadmin_user) + self.composers_vocab = self._create_vocabulary(vocab_name="Composers", + user=self.sysadmin_user) - @classmethod - def teardown_class(cls): + def teardown(self): ckan.model.repo.rebuild_db() - def post(self, url, param_dict=None): - if param_dict is None: - param_dict = {} - param_string = json.dumps(param_dict) - response = self.app.post(url, params=param_string) + def _post(self, url, params=None, extra_environ=None): + if params is None: + params = {} + param_string = json.dumps(params) + response = self.app.post(url, params=param_string, + extra_environ=extra_environ) assert not response.errors return response.json - def test_vocabulary_create(self): - + def _create_vocabulary(self, vocab_name=None, user=None): # Create a new vocabulary. - vocab_name = "Genre" - params = {'vocabulary': {'name': vocab_name}} - response = self.post('/api/action/vocabulary_create', params) + params = {'name': vocab_name} + if user: + extra_environ = {'Authorization' : str(user.apikey)} + else: + extra_environ = None + response = self._post('/api/action/vocabulary_create', params=params, + extra_environ=extra_environ) + # Check the values of the response. assert response['success'] == True assert response['result'] @@ -36,7 +50,7 @@ def test_vocabulary_create(self): assert created_vocab['id'] # Get the list of vocabularies. - response = self.post('/api/action/vocabulary_list') + response = self._post('/api/action/vocabulary_list') # Check that the vocabulary we created is in the list. assert response['success'] == True assert response['result'] @@ -44,25 +58,256 @@ def test_vocabulary_create(self): # Get the created vocabulary. params = {'id': created_vocab['id']} - response = self.post('/api/action/vocabulary_show', params) + response = self._post('/api/action/vocabulary_show', params) # Check that retrieving the vocab by name gives the same result. by_name_params = {'name': created_vocab['name']} - assert response == self.post('/api/action/vocabulary_show', + assert response == self._post('/api/action/vocabulary_show', by_name_params) # Check that it matches what we created. assert response['success'] == True assert response['result'] == created_vocab + return created_vocab + + def _update_vocabulary(self, params, user=None): + if user: + extra_environ = {'Authorization' : str(user.apikey)} + else: + extra_environ = None + response = self._post('/api/action/vocabulary_update', params=params, + extra_environ=extra_environ) + + # Check the values of the response. + assert response['success'] == True + assert response['result'] + updated_vocab = response['result'] + if params.has_key('id'): + assert updated_vocab['id'] == params['id'] + else: + assert updated_vocab['id'] + if params.has_key('name'): + assert updated_vocab['name'] == params['name'] + else: + assert updated_vocab['name'] + + # Get the list of vocabularies. + response = self._post('/api/action/vocabulary_list') + # Check that the vocabulary we created is in the list. + assert response['success'] == True + assert response['result'] + assert response['result'].count(updated_vocab) == 1 + + # Get the created vocabulary. + params = {'id': updated_vocab['id']} + response = self._post('/api/action/vocabulary_show', params) + # Check that retrieving the vocab by name gives the same result. + by_name_params = {'name': updated_vocab['name']} + assert response == self._post('/api/action/vocabulary_show', + by_name_params) + # Check that it matches what we created. + assert response['success'] == True + assert response['result'] == updated_vocab + + return updated_vocab + + def _delete_vocabulary(self, vocab_id, user=None): + if user: + extra_environ = {'Authorization' : str(user.apikey)} + else: + extra_environ = None + params = {'id': vocab_id} + response = self._post('/api/action/vocabulary_delete', params=params, + extra_environ=extra_environ) + + # Check the values of the response. + assert response['success'] == True + assert response['result'] is None + response['result'] + + # Get the list of vocabularies. + response = self._post('/api/action/vocabulary_list') + assert response['success'] == True + assert response['result'] + # Check that the vocabulary we deleted is not in the list. + assert vocab_id not in [vocab['id'] for vocab in response['result']] + + # Check that the deleted vocabulary can no longer be retrieved. + response = self.app.post('/api/action/vocabulary_show', + params=json.dumps(params), + extra_environ = {'Authorization': + str(self.sysadmin_user.apikey)}, + status=404) + assert response.json['success'] == False + + def test_vocabulary_create(self): + '''Test adding a new vocabulary to a CKAN instance via the action + API. + + ''' + self._create_vocabulary(vocab_name="My cool vocab", + user=self.sysadmin_user) + + def test_vocabulary_create_id(self): + '''Test error response when user tries to supply their own ID when + creating a vocabulary. + + ''' + params = {'id': 'xxx', 'name': 'foobar'} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_create', + params=param_string, + extra_environ = {'Authorization': + str(self.sysadmin_user.apikey)}, + status=409) + assert response.json['success'] == False + + def test_vocabulary_create_no_name(self): + '''Test error response when user tries to create a vocab without a + name. + + ''' + params = {} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_create', + params=param_string, + extra_environ = {'Authorization': + str(self.sysadmin_user.apikey)}, + status=409) + assert response.json['success'] == False + + def test_vocabulary_create_invalid_name(self): + '''Test error response when user tries to create a vocab with an + invalid name. + + ''' + for name in (None, '', 'a', 'foobar'*100): + params = {'name': name} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_create', + params=param_string, + extra_environ = {'Authorization': + str(self.sysadmin_user.apikey)}, + status=409) + assert response.json['success'] == False + + def test_vocabulary_create_exists(self): + '''Test error response when user tries to create a vocab that already + exists. + + ''' + params = {'name': self.genre_vocab['name']} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_create', + params=param_string, + extra_environ = {'Authorization': + str(self.sysadmin_user.apikey)}, + status=409) + assert response.json['success'] == False + + def test_vocabulary_create_not_logged_in(self): + '''Test that users who are not logged in cannot create vocabularies.''' + + params = {'name': + "Spam Vocabulary: SpamCo Duck Rental: Rent Your Ducks From Us!"} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_create', + params=param_string, + status=403) + assert response.json['success'] == False + + def test_vocabulary_create_not_authorized(self): + '''Test that users who are not authorized cannot create vocabs.''' + + params = {'name': 'My Unauthorised Vocabulary'} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_create', + params=param_string, + extra_environ = {'Authorization': + str(self.normal_user.apikey)}, + status=403) + assert response.json['success'] == False + def test_vocabulary_update(self): - # Create a vocab. - # Update the vocab via the API. - # List vocabs - # Get vocab, assert fields correct. - raise NotImplementedError + self._update_vocabulary({'id': self.genre_vocab['id'], + 'name': 'updated_name'}, self.sysadmin_user) + + def test_vocabulary_update_not_exists(self): + '''Test the error response given when a user tries to update a + vocabulary that doesn't exist. + + ''' + params = {'id': 'xxxxxxx', 'name': 'updated_name'} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_update', + params=param_string, + extra_environ = {'Authorization': + str(self.sysadmin_user.apikey)}, + status=404) + assert response.json['success'] == False + + def test_vocabulary_update_no_name(self): + params = {'id': self.genre_vocab['id']} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_update', + params=param_string, + extra_environ = {'Authorization': + str(self.sysadmin_user.apikey)}, + status=409) + assert response.json['success'] == False + + def test_vocabulary_update_not_logged_in(self): + '''Test that users who are not logged in cannot update vocabularies.''' + params = {'id': self.genre_vocab['id']} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_update', + params=param_string, + status=403) + assert response.json['success'] == False + + def test_vocabulary_update_not_authorized(self): + '''Test that users who are not authorized cannot update vocabs.''' + params = {'id': self.genre_vocab['id']} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_update', + params=param_string, + extra_environ = {'Authorization': + str(self.normal_user.apikey)}, + status=403) + assert response.json['success'] == False def test_vocabulary_delete(self): - # Create a vocab. - # Delete a vocab via the API - # List vocabs, assert that it's gone. - raise NotImplementedError + self._delete_vocabulary(self.genre_vocab['id'], self.sysadmin_user) + + def test_vocabulary_delete_not_exists(self): + '''Test the error response given when a user tries to delete a + vocabulary that doesn't exist. + + ''' + params = {'id': 'xxxxxxx'} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_delete', + params=param_string, + extra_environ = {'Authorization': + str(self.sysadmin_user.apikey)}, + status=404) + assert response.json['success'] == False + + def test_vocabulary_delete_not_logged_in(self): + '''Test that users who are not logged in cannot delete vocabularies.''' + params = {'id': self.genre_vocab['id']} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_delete', + params=param_string, + status=403) + assert response.json['success'] == False + def test_vocabulary_delete_not_authorized(self): + '''Test that users who are not authorized cannot delete vocabs.''' + params = {'id': self.genre_vocab['id']} + param_string = json.dumps(params) + response = self.app.post('/api/action/vocabulary_delete', + params=param_string, + extra_environ = {'Authorization': + str(self.normal_user.apikey)}, + status=403) + assert response.json['success'] == False From ba1f50e6dac78265102c00e1f072603a251b20e1 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Mon, 30 Jan 2012 18:34:11 +0100 Subject: [PATCH 03/72] [#1705] Add vocabulary column to tag table --- ckan/model/tag.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ckan/model/tag.py b/ckan/model/tag.py index b73dcb3d0ef..69f7aac974f 100644 --- a/ckan/model/tag.py +++ b/ckan/model/tag.py @@ -7,6 +7,7 @@ from domain_object import DomainObject from package import Package from core import * +import vocabulary __all__ = ['tag_table', 'package_tag_table', 'Tag', 'PackageTag', 'PackageTagRevision', 'package_tag_revision_table', @@ -17,7 +18,11 @@ tag_table = Table('tag', metadata, Column('id', types.UnicodeText, primary_key=True, default=make_uuid), - Column('name', types.Unicode(MAX_TAG_LENGTH), nullable=False, unique=True), + Column('name', types.Unicode(MAX_TAG_LENGTH), nullable=False, + unique=True), + Column('vocabulary_id', + types.Unicode(vocabulary.VOCABULARY_NAME_MAX_LENGTH), + ForeignKey('vocabulary.id')) ) package_tag_table = Table('package_tag', metadata, From 74ed1a2913b035e549135bce3af506b6dac09e9e Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Mon, 30 Jan 2012 19:03:27 +0100 Subject: [PATCH 04/72] [#1705] Update tag column unique constraint Tag names are no longer unique, instead (name, vocabulary_id) column pairs in the tag table are unique. CKAN can now have two tags with the same name, as long as they belong to different vocabularies. Each tag still has a unique ID as its primary key. --- ckan/model/tag.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/ckan/model/tag.py b/ckan/model/tag.py index 69f7aac974f..691eadb4260 100644 --- a/ckan/model/tag.py +++ b/ckan/model/tag.py @@ -1,5 +1,4 @@ -from sqlalchemy.orm import eagerload_all -from sqlalchemy import and_ +import sqlalchemy import vdm.sqlalchemy from types import make_uuid @@ -18,11 +17,11 @@ tag_table = Table('tag', metadata, Column('id', types.UnicodeText, primary_key=True, default=make_uuid), - Column('name', types.Unicode(MAX_TAG_LENGTH), nullable=False, - unique=True), + Column('name', types.Unicode(MAX_TAG_LENGTH), nullable=False), Column('vocabulary_id', types.Unicode(vocabulary.VOCABULARY_NAME_MAX_LENGTH), - ForeignKey('vocabulary.id')) + ForeignKey('vocabulary.id')), + sqlalchemy.UniqueConstraint('name', 'vocabulary_id') ) package_tag_table = Table('package_tag', metadata, @@ -47,7 +46,7 @@ def delete(self): def get(cls, reference): '''Returns a tag object referenced by its id or name.''' query = Session.query(cls).filter(cls.id==reference) - query = query.options(eagerload_all('package_tags')) + query = query.options(sqlalchemy.orm.eagerload_all('package_tags')) tag = query.first() if tag == None: tag = cls.by_name(reference) @@ -71,7 +70,7 @@ def search_by_name(cls, text_query): def all(cls): q = Session.query(cls) q = q.distinct().join(PackageTagRevision) - q = q.filter(and_( + q = q.filter(sqlalchemy.and_( PackageTagRevision.state == 'active', PackageTagRevision.current == True )) return q @@ -81,7 +80,7 @@ def packages_ordered(self): q = Session.query(Package) q = q.join(PackageTagRevision) q = q.filter(PackageTagRevision.tag_id == self.id) - q = q.filter(and_( + q = q.filter(sqlalchemy.and_( PackageTagRevision.state == 'active', PackageTagRevision.current == True )) packages = [p for p in q] From 2227d512809bcab9221327e99b3f8c2361ec3f35 Mon Sep 17 00:00:00 2001 From: David Read Date: Tue, 31 Jan 2012 11:34:12 +0000 Subject: [PATCH 05/72] [master][#1725][logic/validators]: Ignore trailing commas in tag list (created by tag autocomplete). --- ckan/logic/validators.py | 17 +++++++-------- ckan/tests/logic/test_validators.py | 32 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 ckan/tests/logic/test_validators.py diff --git a/ckan/logic/validators.py b/ckan/logic/validators.py index f93586f7c42..c272fc9502c 100644 --- a/ckan/logic/validators.py +++ b/ckan/logic/validators.py @@ -195,18 +195,15 @@ def tag_not_uppercase(value, context): return value def tag_string_convert(key, data, errors, context): + '''Takes a list of tags that is a comma-separated string (in data[key]) + and parses tag names. These are added to the data dict, enumerated. They + are also validated.''' + tag_string = data[key] - value = data[key] - - # Ensure a tag string with only whitespace - # is converted to the empty list of tags. - # If we were to split(',') on this string, - # we'd get the non-empty list, ['']. - if not value.strip(): - return + tags = [tag.strip() \ + for tag in tag_string.split(',') \ + if tag.strip()] - tags = map(lambda s: s.strip(), - value.split(',')) for num, tag in enumerate(tags): data[('tags', num, 'name')] = tag diff --git a/ckan/tests/logic/test_validators.py b/ckan/tests/logic/test_validators.py new file mode 100644 index 00000000000..aab45038ef7 --- /dev/null +++ b/ckan/tests/logic/test_validators.py @@ -0,0 +1,32 @@ +from nose.tools import assert_equal + +from ckan import model +from ckan.logic.validators import tag_string_convert + +class TestValidators: + def test_01_tag_string_convert(self): + def convert(tag_string): + key = 'tag_string' + data = {key: tag_string} + errors = [] + context = {'model': model, 'session': model.Session} + tag_string_convert(key, data, errors, context) + tags = [] + i = 0 + while True: + tag = data.get(('tags', i, 'name')) + if not tag: + break + tags.append(tag) + i += 1 + return tags + assert_equal(convert('big, good'), ['big', 'good']) + assert_equal(convert('one, several word tag, with-hyphen'), + ['one', 'several word tag', 'with-hyphen']) + assert_equal(convert(''), + []) + assert_equal(convert('trailing comma,'), + ['trailing comma']) + assert_equal(convert('trailing comma space, '), + ['trailing comma space']) + From a411b14b90db68569e177158908f6b6902f6559c Mon Sep 17 00:00:00 2001 From: John Glover Date: Tue, 31 Jan 2012 15:26:44 +0000 Subject: [PATCH 06/72] [logic/converters] Add converter for tag string to tag with vocab --- ckan/logic/converters.py | 37 +++++++++++++++++++++++++---- ckan/tests/logic/test_converters.py | 20 ++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 ckan/tests/logic/test_converters.py diff --git a/ckan/logic/converters.py b/ckan/logic/converters.py index 540ee5a07e6..18a2ad50619 100644 --- a/ckan/logic/converters.py +++ b/ckan/logic/converters.py @@ -1,18 +1,14 @@ from ckan.lib.navl.dictization_functions import Invalid from ckan.lib.field_types import DateType, DateConvertError - - +from ckan.logic.validators import tag_length_validator, tag_name_validator def convert_to_extras(key, data, errors, context): - extras = data.get(('extras',), []) if not extras: data[('extras',)] = extras - extras.append({'key': key[-1], 'value': data[key]}) def convert_from_extras(key, data, errors, context): - for data_key, data_value in data.iteritems(): if (data_key[0] == 'extras' and data_key[-1] == 'key' @@ -33,3 +29,34 @@ def date_to_form(value, context): raise Invalid(str(e)) return value +def convert_to_tags(vocab): + def callable(key, data, errors, context): + tag_string = data.get(key) + new_tags = [tag.strip() for tag in tag_string.split(',') if tag.strip()] + + if not new_tags: + return + + # get current number of tags + n = 0 + for k in data.keys(): + if k[0] == 'tags': + n = max(n, k[1] + 1) + + # validate + for tag in new_tags: + tag_length_validator(tag, context) + tag_name_validator(tag, context) + + # add new tags + for num, tag in enumerate(new_tags): + data[('tags', num+n, 'name')] = tag + data[('tags', num+n, 'vocabulary')] = vocab + + return callable + +def convert_from_tags(vocab): + def callable(key, data, errors, context): + pass + return callable + diff --git a/ckan/tests/logic/test_converters.py b/ckan/tests/logic/test_converters.py new file mode 100644 index 00000000000..9e9e06337bc --- /dev/null +++ b/ckan/tests/logic/test_converters.py @@ -0,0 +1,20 @@ +from ckan import model +from ckan.logic.converters import convert_to_tags +from ckan.lib.navl.dictization_functions import unflatten + +class TestConverters: + def test_convert_to_tags(self): + def convert(tag_string, vocab): + key = 'vocab_tag_string' + data = {key: tag_string} + errors = [] + context = {'model': model, 'session': model.Session} + convert_to_tags('vocab')(key, data, errors, context) + del data[key] + return data + + data = unflatten(convert('tag1, tag2', 'vocab')) + for tag in data['tags']: + assert tag['name'] in ['tag1', 'tag2'] + assert tag['vocabulary'] == 'vocab' + From f7810165b73adb3db3ed60d771bc5b4c37d77751 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 31 Jan 2012 16:33:09 +0100 Subject: [PATCH 07/72] [#1705] Add optional vocab argument to Tag.get() and Tag.by_name() With the default value for the new vocab argument (None) Tag.get() and Tag.by_name() will return only free tags (tags that don't belong to any vocabulary). If a vocab argument is given they'll only return tags from that vocab. All the tests still pass, but new tests will need to be added for this. --- ckan/model/tag.py | 92 ++++++++++++++++++++++++++++++++++------ ckan/model/vocabulary.py | 6 +-- 2 files changed, 81 insertions(+), 17 deletions(-) diff --git a/ckan/model/tag.py b/ckan/model/tag.py index 691eadb4260..e67acbba3e2 100644 --- a/ckan/model/tag.py +++ b/ckan/model/tag.py @@ -7,6 +7,7 @@ from package import Package from core import * import vocabulary +from vocabulary import Vocabulary __all__ = ['tag_table', 'package_tag_table', 'Tag', 'PackageTag', 'PackageTagRevision', 'package_tag_revision_table', @@ -35,23 +36,92 @@ package_tag_revision_table = make_revisioned_table(package_tag_table) class Tag(DomainObject): - def __init__(self, name=''): + def __init__(self, name='', vocabulary_id=None): self.name = name + self.vocabulary_id = vocabulary_id # not stateful so same as purge def delete(self): self.purge() @classmethod - def get(cls, reference): - '''Returns a tag object referenced by its id or name.''' - query = Session.query(cls).filter(cls.id==reference) - query = query.options(sqlalchemy.orm.eagerload_all('package_tags')) + def by_id(cls, tag_id, autoflush=True, vocab=None): + """Return the tag object with the given id, or None if there is no + tag with that id. + + By default only free tags (tags which do not belong to any vocabulary) + are returned. If the optional argument vocab is given then only tags + from that vocabulary are returned, or None if there is no tag with that + id in that vocabulary. + + Arguments: + tag_id -- The id of the tag to return. + vocab -- A Vocabulary object for the vocabulary to look in (optional). + + """ + if vocab: + query = Session.query(Tag).filter(Tag.id==tag_id).filter( + Tag.vocabulary_id==vocab.id) + else: + query = Session.query(Tag).filter(Tag.id==tag_id).filter( + Tag.vocabulary_id==None) + query = query.autoflush(autoflush) tag = query.first() - if tag == None: - tag = cls.by_name(reference) return tag - # Todo: Make sure tag names can't be changed to look like tag IDs? + + @classmethod + def by_name(cls, name, autoflush=True, vocab=None): + """Return the tag object with the given name, or None if there is no + tag with that name. + + By default only free tags (tags which do not belong to any vocabulary) + are returned. If the optional argument vocab is given then only tags + from that vocabulary are returned, or None if there is no tag with that + name in that vocabulary. + + Arguments: + name -- The name of the tag to return. + vocab -- A Vocabulary object for the vocabulary to look in (optional). + + """ + if vocab: + query = Session.query(Tag).filter(Tag.name==name).filter( + Tag.vocabulary_id==vocab.id) + else: + query = Session.query(Tag).filter(Tag.name==name).filter( + Tag.vocabulary_id==None) + query = query.autoflush(autoflush) + tag = query.first() + return tag + + @classmethod + def get(cls, tag_id_or_name, vocab_id_or_name=None): + """Return the tag object with the given id or name, or None if there is + no tag with that id or name. + + By default only free tags (tags which do not belong to any vocabulary) + are returned. If the optional argument vocab_id_or_name is given then + only tags that belong to that vocabulary will be returned, and None + will be returned if there is no vocabulary with that vocabulary id or + name or if there is no tag with that tag id or name in that vocabulary. + + Arguments: + tag_id_or_name -- The id or name of the tag to return. + vocab_id_or_name -- The id or name of the vocabulary to look in. + + """ + if vocab_id_or_name: + vocab = Vocabulary.get(vocab_id_or_name) + if vocab is None: + # The user specified an invalid vocab. + return None + else: + vocab = None + tag = Tag.by_id(tag_id_or_name, vocab=vocab) + if not tag: + tag = Tag.by_name(tag_id_or_name, vocab=vocab) + return tag + # Todo: Make sure tag names can't be changed to look like tag IDs? @classmethod def search_by_name(cls, text_query): @@ -60,12 +130,6 @@ def search_by_name(cls, text_query): q = q.distinct().join(cls.package_tags) return q - #@classmethod - #def by_name(self, name, autoflush=True): - # q = Session.query(self).autoflush(autoflush).filter_by(name=name) - # q = q.distinct().join(self.package_tags) - # return q.first() - @classmethod def all(cls): q = Session.query(cls) diff --git a/ckan/model/vocabulary.py b/ckan/model/vocabulary.py index c1d5b27d750..2505d39a28a 100644 --- a/ckan/model/vocabulary.py +++ b/ckan/model/vocabulary.py @@ -19,13 +19,13 @@ def __init__(self, name): self.name = name @classmethod - def get(cls, reference): + def get(cls, id_or_name): """Return a Vocabulary object referenced by its id or name.""" - query = Session.query(cls).filter(cls.id==reference) + query = Session.query(cls).filter(cls.id==id_or_name) vocab = query.first() if vocab is None: - vocab = cls.by_name(reference) + vocab = cls.by_name(id_or_name) return vocab mapper(Vocabulary, vocabulary_table) From 343c8c5a93fafdd76e94fbc7ee84bdc777131d89 Mon Sep 17 00:00:00 2001 From: John Glover Date: Wed, 1 Feb 2012 10:03:41 +0000 Subject: [PATCH 08/72] [logic/converters] get vocab ID in convert_to_tags --- ckan/logic/converters.py | 13 +++++++------ ckan/tests/logic/test_converters.py | 16 ++++++++++++---- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/ckan/logic/converters.py b/ckan/logic/converters.py index 18a2ad50619..9b8659cb799 100644 --- a/ckan/logic/converters.py +++ b/ckan/logic/converters.py @@ -1,3 +1,4 @@ +from ckan import model from ckan.lib.navl.dictization_functions import Invalid from ckan.lib.field_types import DateType, DateConvertError from ckan.logic.validators import tag_length_validator, tag_name_validator @@ -33,26 +34,26 @@ def convert_to_tags(vocab): def callable(key, data, errors, context): tag_string = data.get(key) new_tags = [tag.strip() for tag in tag_string.split(',') if tag.strip()] - if not new_tags: return - # get current number of tags n = 0 for k in data.keys(): if k[0] == 'tags': n = max(n, k[1] + 1) - # validate for tag in new_tags: tag_length_validator(tag, context) tag_name_validator(tag, context) - + # get the vocab + v = model.Vocabulary.get(vocab) + if not v: + # TODO: raise an exception here + pass # add new tags for num, tag in enumerate(new_tags): data[('tags', num+n, 'name')] = tag - data[('tags', num+n, 'vocabulary')] = vocab - + data[('tags', num+n, 'vocabulary_id')] = v.id return callable def convert_from_tags(vocab): diff --git a/ckan/tests/logic/test_converters.py b/ckan/tests/logic/test_converters.py index 9e9e06337bc..f45bd1e95f2 100644 --- a/ckan/tests/logic/test_converters.py +++ b/ckan/tests/logic/test_converters.py @@ -3,18 +3,26 @@ from ckan.lib.navl.dictization_functions import unflatten class TestConverters: + @classmethod + def setup_class(cls): + # create a new vocabulary + cls.vocab = model.Vocabulary('test-vocab') + model.Session.add(cls.vocab) + model.Session.commit() + model.Session.remove() + def test_convert_to_tags(self): def convert(tag_string, vocab): key = 'vocab_tag_string' data = {key: tag_string} errors = [] context = {'model': model, 'session': model.Session} - convert_to_tags('vocab')(key, data, errors, context) + convert_to_tags(vocab)(key, data, errors, context) del data[key] return data - data = unflatten(convert('tag1, tag2', 'vocab')) + data = unflatten(convert('tag1, tag2', 'test-vocab')) for tag in data['tags']: - assert tag['name'] in ['tag1', 'tag2'] - assert tag['vocabulary'] == 'vocab' + assert tag['name'] in ['tag1', 'tag2'], tag['name'] + assert tag['vocabulary_id'] == self.vocab.id, tag['vocabulary_id'] From 685acdc99d7429556b98ff2534ab54af50a06e98 Mon Sep 17 00:00:00 2001 From: John Glover Date: Wed, 1 Feb 2012 10:06:58 +0000 Subject: [PATCH 09/72] [logic/converters] raise exception for unknown vocab --- ckan/logic/converters.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ckan/logic/converters.py b/ckan/logic/converters.py index 9b8659cb799..ef415cae98c 100644 --- a/ckan/logic/converters.py +++ b/ckan/logic/converters.py @@ -1,3 +1,4 @@ +from pylons.i18n import _ from ckan import model from ckan.lib.navl.dictization_functions import Invalid from ckan.lib.field_types import DateType, DateConvertError @@ -45,11 +46,9 @@ def callable(key, data, errors, context): for tag in new_tags: tag_length_validator(tag, context) tag_name_validator(tag, context) - # get the vocab v = model.Vocabulary.get(vocab) if not v: - # TODO: raise an exception here - pass + raise Invalid(_('Tag vocabulary "%s" does not exist') % vocab) # add new tags for num, tag in enumerate(new_tags): data[('tags', num+n, 'name')] = tag From fa89fe606ad38f995e367d18b43b6e4282c43a08 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 1 Feb 2012 12:31:21 +0100 Subject: [PATCH 10/72] [#1705] Add vocab_id_or_name=None args to some model/tag.py methods. Change the behaviour of the methods so that they should return only free tags (tags without a vocabulary) when called with no vocab_id_or_name argument. With a vocab_id_or_name they return only tags belonging to that vocab. The existing tests still work but new tests for the new argument need to be added. Also added docstrings to these methods because it wasn't too obvious what they did. --- ckan/model/tag.py | 98 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 76 insertions(+), 22 deletions(-) diff --git a/ckan/model/tag.py b/ckan/model/tag.py index e67acbba3e2..d480e028912 100644 --- a/ckan/model/tag.py +++ b/ckan/model/tag.py @@ -124,29 +124,63 @@ def get(cls, tag_id_or_name, vocab_id_or_name=None): # Todo: Make sure tag names can't be changed to look like tag IDs? @classmethod - def search_by_name(cls, text_query): - text_query = text_query.strip().lower() - q = Session.query(cls).filter(cls.name.contains(text_query)) - q = q.distinct().join(cls.package_tags) - return q - + def search_by_name(cls, search_term, vocab_id_or_name=None): + """Return all tags that match the given search term. + + By default only free tags (tags which do not belong to any vocabulary) + are returned. If the optional argument vocab_id_or_name is given then + only tags from that vocabulary are returned. + + """ + if vocab_id_or_name: + vocab = Vocabulary.get(vocab_id_or_name) + if vocab is None: + # The user specified an invalid vocab. + return None + query = Session.query(Tag).filter(Tag.vocabulary_id==vocab.id) + else: + query = Session.query(Tag) + search_term = search_term.strip().lower() + query = query.filter(Tag.name.contains(search_term)) + query = query.distinct().join(Tag.package_tags) + return query + @classmethod - def all(cls): - q = Session.query(cls) - q = q.distinct().join(PackageTagRevision) - q = q.filter(sqlalchemy.and_( - PackageTagRevision.state == 'active', PackageTagRevision.current == True - )) - return q + def all(cls, vocab_id_or_name=None): + """Return all tags that are currently applied to a package. + + By default only free tags (tags which do not belong to any vocabulary) + are returned. If the optional argument vocab_id_or_name is given then + only tags from that vocabulary are returned. + + """ + if vocab_id_or_name: + vocab = Vocabulary.get(vocab_id_or_name) + if vocab is None: + # The user specified an invalid vocab. + return None + query = Session.query(Tag).filter(Tag.vocabulary_id==vocab.id) + else: + query = Session.query(Tag) + query = query.distinct().join(PackageTagRevision) + query = query.filter(sqlalchemy.and_( + PackageTagRevision.state == 'active', + PackageTagRevision.current == True)) + return query @property - def packages_ordered(self): + def packages_ordered(self, vocab_id_or_name=None): + """Return a list of all packages currently tagged with this tag. + + The list is sorted by package name. + + """ q = Session.query(Package) q = q.join(PackageTagRevision) q = q.filter(PackageTagRevision.tag_id == self.id) q = q.filter(sqlalchemy.and_( - PackageTagRevision.state == 'active', PackageTagRevision.current == True - )) + PackageTagRevision.state == 'active', + PackageTagRevision.current == True)) packages = [p for p in q] ourcmp = lambda pkg1, pkg2: cmp(pkg1.name, pkg2.name) return sorted(packages, cmp=ourcmp) @@ -169,12 +203,32 @@ def __repr__(self): return '' % (self.package.name, self.tag.name) @classmethod - def by_name(self, package_name, tag_name, autoflush=True): - q = Session.query(self).autoflush(autoflush).\ - join('package').filter(Package.name==package_name).\ - join('tag').filter(Tag.name==tag_name) - assert q.count() <= 1, q.all() - return q.first() + def by_name(self, package_name, tag_name, vocab_id_or_name=None, + autoflush=True): + """Return the one PackageTag for the given package and tag names, or + None if there is no PackageTag for that package and tag. + + By default only PackageTags for free tags (tags which do not belong to + any vocabulary) are returned. If the optional argument vocab_id_or_name + is given then only PackageTags for tags from that vocabulary are + returned. + + """ + if vocab_id_or_name: + vocab = Vocabulary.get(vocab_id_or_name) + if vocab is None: + # The user specified an invalid vocab. + return None + query = (Session.query(PackageTag, Tag, Package) + .filter(Tag.vocabulary_id == vocab.id) + .filter(Package.name==package_name) + .filter(Tag.name==tag_name)) + else: + query = (Session.query(PackageTag) + .filter(Package.name==package_name) + .filter(Tag.name==tag_name)) + query = query.autoflush(autoflush) + return query.one()[0] def related_packages(self): return [self.package] From b3e72c93ba71290d7dccc8afbc9d3f5f0d7fb3ac Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 1 Feb 2012 12:40:45 +0100 Subject: [PATCH 11/72] [#1705] Remove an arg that I added by accident --- ckan/model/tag.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/model/tag.py b/ckan/model/tag.py index d480e028912..1eab9104dd0 100644 --- a/ckan/model/tag.py +++ b/ckan/model/tag.py @@ -169,7 +169,7 @@ def all(cls, vocab_id_or_name=None): return query @property - def packages_ordered(self, vocab_id_or_name=None): + def packages_ordered(self): """Return a list of all packages currently tagged with this tag. The list is sorted by package name. From d1dbaeb10477216e43b4459ffbcd358bb1368c25 Mon Sep 17 00:00:00 2001 From: John Glover Date: Wed, 1 Feb 2012 12:33:29 +0000 Subject: [PATCH 12/72] [dictization] Deduplicate/sort tags by (name, vocab) before saving --- ckan/lib/dictization/model_save.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/ckan/lib/dictization/model_save.py b/ckan/lib/dictization/model_save.py index ecdad95287e..c1e82bfec88 100644 --- a/ckan/lib/dictization/model_save.py +++ b/ckan/lib/dictization/model_save.py @@ -132,7 +132,6 @@ def group_extras_save(extras_dicts, context): return result_dict def package_tag_list_save(tag_dicts, package, context): - allow_partial_update = context.get("allow_partial_update", False) if not tag_dicts and allow_partial_update: return @@ -150,13 +149,13 @@ def package_tag_list_save(tag_dicts, package, context): pt.state in ['deleted', 'pending-deleted'] ] ) - tag_names = set() + tag_name_vocab = set() tags = set() for tag_dict in tag_dicts: - if tag_dict.get('name') not in tag_names: + if (tag_dict.get('name'), tag_dict.get('vocabulary_id')) not in tag_name_vocab: tag_obj = table_dict_save(tag_dict, model.Tag, context) tags.add(tag_obj) - tag_names.add(tag_obj.name) + tag_name_vocab.add((tag_obj.name, tag_obj.vocabulary_id)) # 3 cases # case 1: currently active but not in new list @@ -167,14 +166,14 @@ def package_tag_list_save(tag_dicts, package, context): else: package_tag.state = 'deleted' - # in new list but never used before + # case 2: in new list but never used before for tag in tags - set(tag_package_tag.keys()): state = 'pending' if pending else 'active' package_tag_obj = model.PackageTag(package, tag, state) session.add(package_tag_obj) tag_package_tag[tag] = package_tag_obj - # in new list and already used but in deleted state + # case 3: in new list and already used but in deleted state for tag in tags.intersection(set(tag_package_tag_inactive.keys())): state = 'pending' if pending else 'active' package_tag = tag_package_tag[tag] From 84386d7dfd9b8751627c9e2ce94b0f538bb0fc11 Mon Sep 17 00:00:00 2001 From: John Glover Date: Wed, 1 Feb 2012 12:34:29 +0000 Subject: [PATCH 13/72] [logic/converters] add converter to remove a vocab tag from the list of all tags --- ckan/logic/converters.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/ckan/logic/converters.py b/ckan/logic/converters.py index ef415cae98c..853778b7699 100644 --- a/ckan/logic/converters.py +++ b/ckan/logic/converters.py @@ -42,6 +42,7 @@ def callable(key, data, errors, context): for k in data.keys(): if k[0] == 'tags': n = max(n, k[1] + 1) + # validate for tag in new_tags: tag_length_validator(tag, context) @@ -49,14 +50,29 @@ def callable(key, data, errors, context): v = model.Vocabulary.get(vocab) if not v: raise Invalid(_('Tag vocabulary "%s" does not exist') % vocab) - # add new tags + for num, tag in enumerate(new_tags): data[('tags', num+n, 'name')] = tag data[('tags', num+n, 'vocabulary_id')] = v.id + return callable def convert_from_tags(vocab): def callable(key, data, errors, context): - pass + v = model.Vocabulary.get(vocab) + if not v: + raise Invalid(_('Tag vocabulary "%s" does not exist') % vocab) + + tags = {} + for k in data.keys(): + if k[0] == 'tags': + if data[k].get('vocabulary_id') == v.id: + tags[k] = data[k] + + # TODO: vocab tags should be removed in a separate converter (and by default) 'tags' + for k in tags.keys(): + del data[k] + data[key] = ', '.join([t['name'] for t in tags.values()]) + return callable From ea4541ac0311bc9f9f1472998479d17d54484c31 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 1 Feb 2012 18:12:35 +0100 Subject: [PATCH 14/72] [#1705] Add vocab=None argument to Package.add_tag_by_name() And change the method to use free tags only if no vocab is given, otherwise use tags from the given vocab only. --- ckan/model/package.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/ckan/model/package.py b/ckan/model/package.py index bdbd8d7d49a..7c787ba98f0 100644 --- a/ckan/model/package.py +++ b/ckan/model/package.py @@ -159,13 +159,25 @@ def add_resource(self, url, format=u'', description=u'', hash=u'', **kw): hash=hash, **kw)) - def add_tag_by_name(self, tagname, autoflush=True): + def add_tag_by_name(self, tag_name, vocab=None, autoflush=True): + """Add a tag with the given name to this package's tags. + + By default only free tags (tags which do not belong to any vocabulary) + are used. If the optional argument vocab is given then only tags + belonging to that vocabulary will be used. + + If no tag with the given name exists, one will be created. If the + optional argument vocab is given and no tag with the given name exists + in the given vocabulary, then a new tag will be created and added to + the vocabulary. + + """ from tag import Tag - if not tagname: + if not tag_name: return - tag = Tag.by_name(tagname, autoflush=autoflush) + tag = Tag.by_name(tag_name, vocab=vocab, autoflush=autoflush) if not tag: - tag = Tag(name=tagname) + tag = Tag(name=tag_name, vocabulary=vocab) if not tag in self.tags: self.tags.append(tag) From c6a14378114e3ae4617eed9619fa69ead1298117 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 1 Feb 2012 18:36:34 +0100 Subject: [PATCH 15/72] Tidy up Package.tags_ordered() Add a docstring, and use key= and attrgetter instead of cmp= and lambda --- ckan/model/package.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ckan/model/package.py b/ckan/model/package.py index 7c787ba98f0..856483165f5 100644 --- a/ckan/model/package.py +++ b/ckan/model/package.py @@ -1,6 +1,7 @@ import datetime from time import gmtime from calendar import timegm +from operator import attrgetter from sqlalchemy.sql import select, and_, union, expression, or_ from sqlalchemy.orm import eagerload_all @@ -84,7 +85,7 @@ def resources(self): assert len(self.resource_groups_all) == 1, "can only use resources on packages if there is only one resource_group" return self.resource_groups_all[0].resources - + def update_resources(self, res_dicts, autoflush=True): '''Change this package\'s resources. @param res_dicts - ordered list of dicts, each detailing a resource @@ -183,8 +184,14 @@ def add_tag_by_name(self, tag_name, vocab=None, autoflush=True): @property def tags_ordered(self): - ourcmp = lambda tag1, tag2: cmp(tag1.name, tag2.name) - return sorted(self.tags, cmp=ourcmp) + """Return a sorted list of this package's tags + + Tags are sorted by the names of the vocabularies that they belong to + and then (within vocabularies) by their tag names + + """ + sorted_tags = sorted(self.tags, key=attrgetter('name')) + return sorted_tags def isopen(self): if self.license and self.license.isopen(): From 9f5d62bdbd2d33a6dea28be3ce3ab9c8fedc29af Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 1 Feb 2012 18:45:19 +0100 Subject: [PATCH 16/72] [#1705] Tidy up order of keyword args in Tag.by_name() --- ckan/model/tag.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/model/tag.py b/ckan/model/tag.py index 1eab9104dd0..b1f5063e4d4 100644 --- a/ckan/model/tag.py +++ b/ckan/model/tag.py @@ -70,7 +70,7 @@ def by_id(cls, tag_id, autoflush=True, vocab=None): return tag @classmethod - def by_name(cls, name, autoflush=True, vocab=None): + def by_name(cls, name, vocab=None, autoflush=True): """Return the tag object with the given name, or None if there is no tag with that name. From 7ea9e639547d37326dfc3c36d4aedd2af9b963e0 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 1 Feb 2012 18:59:58 +0100 Subject: [PATCH 17/72] [#1705] Make vocabulary.get() a module function ...no longer a classmethod. It doesn't seem to need to be a classmethod, and this way you can do: import vocabulary some_vocab = vocabulary.get(some_name) Instead of having to do: `vocabulary.Vocabulary.get(some_name)`, which looks too much like Java. --- ckan/lib/dictization/model_save.py | 2 +- ckan/logic/action/delete.py | 2 +- ckan/logic/action/get.py | 4 ++-- ckan/logic/action/update.py | 2 +- ckan/model/tag.py | 9 ++++----- ckan/model/vocabulary.py | 19 +++++++++---------- 6 files changed, 18 insertions(+), 20 deletions(-) diff --git a/ckan/lib/dictization/model_save.py b/ckan/lib/dictization/model_save.py index ecdad95287e..55d9358d83f 100644 --- a/ckan/lib/dictization/model_save.py +++ b/ckan/lib/dictization/model_save.py @@ -457,7 +457,7 @@ def vocabulary_dict_update(vocabulary_dict, context): model = context['model'] session = context['session'] - vocabulary_obj = model.Vocabulary.get(vocabulary_dict['id']) + vocabulary_obj = model.vocabulary.get(vocabulary_dict['id']) vocabulary_obj.name = vocabulary_dict['name'] return vocabulary_obj diff --git a/ckan/logic/action/delete.py b/ckan/logic/action/delete.py index d3b01c3885c..219e756ec79 100644 --- a/ckan/logic/action/delete.py +++ b/ckan/logic/action/delete.py @@ -108,7 +108,7 @@ def vocabulary_delete(context, data_dict): user = context['user'] vocab_id = data_dict['id'] - vocab_obj = model.Vocabulary.get(vocab_id) + vocab_obj = model.vocabulary.get(vocab_id) if vocab_obj is None: raise NotFound diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 2812cdd76dd..8e1d6203f32 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -885,11 +885,11 @@ def vocabulary_show(context, data_dict): model = context['model'] if data_dict.has_key('id'): - vocabulary = model.Vocabulary.get(data_dict['id']) + vocabulary = model.vocabulary.get(data_dict['id']) if vocabulary is None: raise NotFound elif data_dict.has_key('name'): - vocabulary = model.Vocabulary.get(data_dict['name']) + vocabulary = model.vocabulary.get(data_dict['name']) if vocabulary is None: raise NotFound else: diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 43375e8832a..b274640ef29 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -500,7 +500,7 @@ def vocabulary_update(context, data_dict): model.Session.remove() model.Session()._context = context - vocab = model.Vocabulary.get(vocab_id) + vocab = model.vocabulary.get(vocab_id) if vocab is None: raise NotFound(_('Vocabulary was not found.')) diff --git a/ckan/model/tag.py b/ckan/model/tag.py index b1f5063e4d4..2894015d55d 100644 --- a/ckan/model/tag.py +++ b/ckan/model/tag.py @@ -7,7 +7,6 @@ from package import Package from core import * import vocabulary -from vocabulary import Vocabulary __all__ = ['tag_table', 'package_tag_table', 'Tag', 'PackageTag', 'PackageTagRevision', 'package_tag_revision_table', @@ -111,7 +110,7 @@ def get(cls, tag_id_or_name, vocab_id_or_name=None): """ if vocab_id_or_name: - vocab = Vocabulary.get(vocab_id_or_name) + vocab = vocabulary.get(vocab_id_or_name) if vocab is None: # The user specified an invalid vocab. return None @@ -133,7 +132,7 @@ def search_by_name(cls, search_term, vocab_id_or_name=None): """ if vocab_id_or_name: - vocab = Vocabulary.get(vocab_id_or_name) + vocab = vocabulary.get(vocab_id_or_name) if vocab is None: # The user specified an invalid vocab. return None @@ -155,7 +154,7 @@ def all(cls, vocab_id_or_name=None): """ if vocab_id_or_name: - vocab = Vocabulary.get(vocab_id_or_name) + vocab = vocabulary.get(vocab_id_or_name) if vocab is None: # The user specified an invalid vocab. return None @@ -215,7 +214,7 @@ def by_name(self, package_name, tag_name, vocab_id_or_name=None, """ if vocab_id_or_name: - vocab = Vocabulary.get(vocab_id_or_name) + vocab = vocabulary.get(vocab_id_or_name) if vocab is None: # The user specified an invalid vocab. return None diff --git a/ckan/model/vocabulary.py b/ckan/model/vocabulary.py index 2505d39a28a..af1749930f4 100644 --- a/ckan/model/vocabulary.py +++ b/ckan/model/vocabulary.py @@ -12,20 +12,19 @@ unique=True), ) +def get(id_or_name): + """Return a Vocabulary object referenced by its id or name.""" + + query = Session.query(Vocabulary).filter(Vocabulary.id==id_or_name) + vocab = query.first() + if vocab is None: + vocab = Vocabulary.by_name(id_or_name) + return vocab + class Vocabulary(DomainObject): def __init__(self, name): self.id = make_uuid() self.name = name - @classmethod - def get(cls, id_or_name): - """Return a Vocabulary object referenced by its id or name.""" - - query = Session.query(cls).filter(cls.id==id_or_name) - vocab = query.first() - if vocab is None: - vocab = cls.by_name(id_or_name) - return vocab - mapper(Vocabulary, vocabulary_table) From d5dd1a5fe6abb35e10ec19c5b5e83d77416c82db Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 1 Feb 2012 19:01:58 +0100 Subject: [PATCH 18/72] [#1705] Fix Package.tags_ordered() Broken since commit c6a1437 --- ckan/model/package.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ckan/model/package.py b/ckan/model/package.py index 856483165f5..a2849024a5c 100644 --- a/ckan/model/package.py +++ b/ckan/model/package.py @@ -178,7 +178,10 @@ def add_tag_by_name(self, tag_name, vocab=None, autoflush=True): return tag = Tag.by_name(tag_name, vocab=vocab, autoflush=autoflush) if not tag: - tag = Tag(name=tag_name, vocabulary=vocab) + if vocab: + tag = Tag(name=tag_name, vocabulary_id=vocab.id) + else: + tag = Tag(name=tag_name) if not tag in self.tags: self.tags.append(tag) From 17b3d10a3c4abe5a0ea68ba0f7aff596065a8527 Mon Sep 17 00:00:00 2001 From: kindly Date: Thu, 2 Feb 2012 11:10:06 +0000 Subject: [PATCH 19/72] [#1741] fix license in template and dictization --- ckan/lib/dictization/model_dictize.py | 9 +++++++++ ckan/logic/action/get.py | 22 ---------------------- ckan/templates/package/read.html | 11 +++++------ ckan/tests/lib/test_dictization.py | 4 ++++ 4 files changed, 18 insertions(+), 28 deletions(-) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 7a9a71c79c0..422c2111a86 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -194,6 +194,15 @@ def package_dictize(pkg, context): # type result_dict['type']= pkg.type + # licence + if pkg.license and pkg.license.url: + result_dict['license_url']= pkg.license.url + result_dict['license_title']= pkg.license.title.split('::')[-1] + elif pkg.license: + result_dict['license_title']= pkg.license.title + else: + result_dict['license_title']= pkg.license_id + # creation and modification date result_dict['metadata_modified'] = pkg.metadata_modified.isoformat() \ if pkg.metadata_modified else None diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index ab2d4e2a48c..7036ca0de4f 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -713,28 +713,6 @@ def package_search(context, data_dict): return search_results -def _extend_package_dict(package_dict,context): - model = context['model'] - - resources = model.Session.query(model.Resource)\ - .join(model.ResourceGroup)\ - .filter(model.ResourceGroup.package_id == package_dict['id'])\ - .all() - if resources: - package_dict['resources'] = resource_list_dictize(resources, context) - else: - package_dict['resources'] = [] - license_id = package_dict.get('license_id') - if license_id: - try: - isopen = model.Package.get_license_register()[license_id].isopen() - except KeyError: - isopen = False - package_dict['isopen'] = isopen - else: - package_dict['isopen'] = False - - return package_dict def resource_search(context, data_dict): model = context['model'] diff --git a/ckan/templates/package/read.html b/ckan/templates/package/read.html index c5dad62261e..3403c7cb342 100644 --- a/ckan/templates/package/read.html +++ b/ckan/templates/package/read.html @@ -23,18 +23,17 @@ - From 5903a01eb28374f084854b30af5442e315a65900 Mon Sep 17 00:00:00 2001 From: John Glover Date: Mon, 6 Feb 2012 10:32:27 +0000 Subject: [PATCH 21/72] [1698][logic/converters] Update as location of vocabulary get function has changed --- ckan/logic/converters.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ckan/logic/converters.py b/ckan/logic/converters.py index 853778b7699..80fb6379bbd 100644 --- a/ckan/logic/converters.py +++ b/ckan/logic/converters.py @@ -1,5 +1,6 @@ from pylons.i18n import _ from ckan import model +from ckan.model import vocabulary from ckan.lib.navl.dictization_functions import Invalid from ckan.lib.field_types import DateType, DateConvertError from ckan.logic.validators import tag_length_validator, tag_name_validator @@ -47,7 +48,7 @@ def callable(key, data, errors, context): for tag in new_tags: tag_length_validator(tag, context) tag_name_validator(tag, context) - v = model.Vocabulary.get(vocab) + v = vocabulary.get(vocab) if not v: raise Invalid(_('Tag vocabulary "%s" does not exist') % vocab) @@ -59,7 +60,7 @@ def callable(key, data, errors, context): def convert_from_tags(vocab): def callable(key, data, errors, context): - v = model.Vocabulary.get(vocab) + v = vocabulary.get(vocab) if not v: raise Invalid(_('Tag vocabulary "%s" does not exist') % vocab) From 0ad2fc3515f9afeb92339094cbbd3335b3f351af Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Mon, 6 Feb 2012 16:51:35 +0100 Subject: [PATCH 22/72] Revert 7ea9e639 Make Vocabulary.get() a @classmethod again, not a module function, just to be consistent with the other model classes. --- ckan/lib/dictization/model_save.py | 2 +- ckan/logic/action/delete.py | 2 +- ckan/logic/action/get.py | 4 ++-- ckan/logic/action/update.py | 2 +- ckan/model/tag.py | 8 ++++---- ckan/model/vocabulary.py | 21 ++++++++++++--------- 6 files changed, 21 insertions(+), 18 deletions(-) diff --git a/ckan/lib/dictization/model_save.py b/ckan/lib/dictization/model_save.py index 93a1a5460fa..01d7a688120 100644 --- a/ckan/lib/dictization/model_save.py +++ b/ckan/lib/dictization/model_save.py @@ -456,7 +456,7 @@ def vocabulary_dict_update(vocabulary_dict, context): model = context['model'] session = context['session'] - vocabulary_obj = model.vocabulary.get(vocabulary_dict['id']) + vocabulary_obj = model.vocabulary.Vocabulary.get(vocabulary_dict['id']) vocabulary_obj.name = vocabulary_dict['name'] return vocabulary_obj diff --git a/ckan/logic/action/delete.py b/ckan/logic/action/delete.py index 219e756ec79..2542c44b579 100644 --- a/ckan/logic/action/delete.py +++ b/ckan/logic/action/delete.py @@ -108,7 +108,7 @@ def vocabulary_delete(context, data_dict): user = context['user'] vocab_id = data_dict['id'] - vocab_obj = model.vocabulary.get(vocab_id) + vocab_obj = model.vocabulary.Vocabulary.get(vocab_id) if vocab_obj is None: raise NotFound diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 8e1d6203f32..df34ae6178d 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -885,11 +885,11 @@ def vocabulary_show(context, data_dict): model = context['model'] if data_dict.has_key('id'): - vocabulary = model.vocabulary.get(data_dict['id']) + vocabulary = model.vocabulary.Vocabulary.get(data_dict['id']) if vocabulary is None: raise NotFound elif data_dict.has_key('name'): - vocabulary = model.vocabulary.get(data_dict['name']) + vocabulary = model.vocabulary.Vocabulary.get(data_dict['name']) if vocabulary is None: raise NotFound else: diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index b274640ef29..a13bd8615d8 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -500,7 +500,7 @@ def vocabulary_update(context, data_dict): model.Session.remove() model.Session()._context = context - vocab = model.vocabulary.get(vocab_id) + vocab = model.vocabulary.Vocabulary.get(vocab_id) if vocab is None: raise NotFound(_('Vocabulary was not found.')) diff --git a/ckan/model/tag.py b/ckan/model/tag.py index 2894015d55d..ecd1a2ab9f9 100644 --- a/ckan/model/tag.py +++ b/ckan/model/tag.py @@ -110,7 +110,7 @@ def get(cls, tag_id_or_name, vocab_id_or_name=None): """ if vocab_id_or_name: - vocab = vocabulary.get(vocab_id_or_name) + vocab = vocabulary.Vocabulary.get(vocab_id_or_name) if vocab is None: # The user specified an invalid vocab. return None @@ -132,7 +132,7 @@ def search_by_name(cls, search_term, vocab_id_or_name=None): """ if vocab_id_or_name: - vocab = vocabulary.get(vocab_id_or_name) + vocab = vocabulary.Vocabulary.get(vocab_id_or_name) if vocab is None: # The user specified an invalid vocab. return None @@ -154,7 +154,7 @@ def all(cls, vocab_id_or_name=None): """ if vocab_id_or_name: - vocab = vocabulary.get(vocab_id_or_name) + vocab = vocabulary.Vocabulary.get(vocab_id_or_name) if vocab is None: # The user specified an invalid vocab. return None @@ -214,7 +214,7 @@ def by_name(self, package_name, tag_name, vocab_id_or_name=None, """ if vocab_id_or_name: - vocab = vocabulary.get(vocab_id_or_name) + vocab = vocabulary.Vocabulary.get(vocab_id_or_name) if vocab is None: # The user specified an invalid vocab. return None diff --git a/ckan/model/vocabulary.py b/ckan/model/vocabulary.py index af1749930f4..c4d2dec401a 100644 --- a/ckan/model/vocabulary.py +++ b/ckan/model/vocabulary.py @@ -12,19 +12,22 @@ unique=True), ) -def get(id_or_name): - """Return a Vocabulary object referenced by its id or name.""" - - query = Session.query(Vocabulary).filter(Vocabulary.id==id_or_name) - vocab = query.first() - if vocab is None: - vocab = Vocabulary.by_name(id_or_name) - return vocab - class Vocabulary(DomainObject): def __init__(self, name): self.id = make_uuid() self.name = name + @classmethod + def get(cls, id_or_name): + """Return a Vocabulary object referenced by its id or name, or None if + there is no vocabulary with the given id or name. + + """ + query = Session.query(Vocabulary).filter(Vocabulary.id==id_or_name) + vocab = query.first() + if vocab is None: + vocab = Vocabulary.by_name(id_or_name) + return vocab + mapper(Vocabulary, vocabulary_table) From 431a3b9ab4053791c45929f2a2bcf921aafed6a1 Mon Sep 17 00:00:00 2001 From: John Glover Date: Tue, 7 Feb 2012 13:19:27 +0000 Subject: [PATCH 23/72] [1720][logic/converters] Update for change in location of Vocabulary get function --- ckan/logic/converters.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ckan/logic/converters.py b/ckan/logic/converters.py index 80fb6379bbd..112179b4752 100644 --- a/ckan/logic/converters.py +++ b/ckan/logic/converters.py @@ -44,23 +44,21 @@ def callable(key, data, errors, context): if k[0] == 'tags': n = max(n, k[1] + 1) - # validate for tag in new_tags: tag_length_validator(tag, context) tag_name_validator(tag, context) - v = vocabulary.get(vocab) + v = model.Vocabulary.get(vocab) if not v: raise Invalid(_('Tag vocabulary "%s" does not exist') % vocab) for num, tag in enumerate(new_tags): data[('tags', num+n, 'name')] = tag data[('tags', num+n, 'vocabulary_id')] = v.id - return callable def convert_from_tags(vocab): def callable(key, data, errors, context): - v = vocabulary.get(vocab) + v = model.Vocabulary.get(vocab) if not v: raise Invalid(_('Tag vocabulary "%s" does not exist') % vocab) From 55754884f640a2db921b923b6f694388e8e77943 Mon Sep 17 00:00:00 2001 From: John Glover Date: Tue, 7 Feb 2012 13:20:55 +0000 Subject: [PATCH 24/72] [1720][logic, dictization, templates] Don't show tags with vocabularies in normal 'tags' list in WUI and API --- ckan/lib/dictization/model_dictize.py | 4 ++-- ckan/logic/converters.py | 15 ++++++++++----- ckan/logic/schema.py | 5 +++-- ckan/logic/validators.py | 8 ++++++++ ckan/templates/_util.html | 4 +++- ckan/templates/package/new_package_form.html | 2 +- 6 files changed, 27 insertions(+), 11 deletions(-) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 43826b89092..f044c0187a5 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -340,7 +340,7 @@ def package_to_api1(pkg, context): dictized.pop("revision_timestamp") dictized["groups"] = [group["name"] for group in dictized["groups"]] - dictized["tags"] = [tag["name"] for tag in dictized["tags"]] + dictized["tags"] = [tag["name"] for tag in dictized["tags"] if not tag.get('vocabulary_id')] dictized["extras"] = dict((extra["key"], json.loads(extra["value"])) for extra in dictized["extras"]) dictized['notes_rendered'] = ckan.misc.MarkdownFormat().to_html(pkg.notes) @@ -397,7 +397,7 @@ def package_to_api2(pkg, context): dictized["groups"] = [group["id"] for group in dictized["groups"]] dictized.pop("revision_timestamp") - dictized["tags"] = [tag["name"] for tag in dictized["tags"]] + dictized["tags"] = [tag["name"] for tag in dictized["tags"] if not tag.get('vocabulary_id')] dictized["extras"] = dict((extra["key"], json.loads(extra["value"])) for extra in dictized["extras"]) diff --git a/ckan/logic/converters.py b/ckan/logic/converters.py index 112179b4752..829b78a977c 100644 --- a/ckan/logic/converters.py +++ b/ckan/logic/converters.py @@ -32,6 +32,16 @@ def date_to_form(value, context): raise Invalid(str(e)) return value +def free_tags_only(key, data, errors, context): + tag_number = key[1] + to_delete = [] + if data.get(('tags', tag_number, 'vocabulary_id')): + to_delete.append(tag_number) + for k in data.keys(): + for n in to_delete: + if k[0] == 'tags' and k[1] == n: + del data[k] + def convert_to_tags(vocab): def callable(key, data, errors, context): tag_string = data.get(key) @@ -67,11 +77,6 @@ def callable(key, data, errors, context): if k[0] == 'tags': if data[k].get('vocabulary_id') == v.id: tags[k] = data[k] - - # TODO: vocab tags should be removed in a separate converter (and by default) 'tags' - for k in tags.keys(): - del data[k] data[key] = ', '.join([t['name'] for t in tags.values()]) - return callable diff --git a/ckan/logic/schema.py b/ckan/logic/schema.py index 84ec4d79351..5cfd1021cc8 100644 --- a/ckan/logic/schema.py +++ b/ckan/logic/schema.py @@ -32,7 +32,8 @@ int_validator, user_about_validator, vocabulary_name_validator, - vocabulary_id_not_changed) + vocabulary_id_not_changed, + vocabulary_id_exists) from formencode.validators import OneOf import ckan.model @@ -71,13 +72,13 @@ def default_update_resource_schema(): return schema def default_tags_schema(): - schema = { 'name': [not_empty, unicode, tag_length_validator, tag_name_validator, ], + 'vocabulary_id': [ignore_missing, unicode, vocabulary_id_exists], 'revision_timestamp': [ignore], 'state': [ignore], } diff --git a/ckan/logic/validators.py b/ckan/logic/validators.py index c272fc9502c..ea7702398d9 100644 --- a/ckan/logic/validators.py +++ b/ckan/logic/validators.py @@ -340,3 +340,11 @@ def vocabulary_id_not_changed(value, context): raise Invalid(_('Cannot change value of key from %s to %s. ' 'This key is read-only') % (vocabulary.id, value)) return value + +def vocabulary_id_exists(value, context): + model = context['model'] + session = context['session'] + result = session.query(model.Vocabulary).get(value) + if not result: + raise Invalid(_('Tag vocabulary was not found.')) + return value diff --git a/ckan/templates/_util.html b/ckan/templates/_util.html index a04268eca70..bfdf9b2c503 100644 --- a/ckan/templates/_util.html +++ b/ckan/templates/_util.html @@ -26,9 +26,11 @@
    -
  • + +
  • ${h.link_to(tag['name'], h.url_for(controller='tag', action='read', id=tag['name']))}
  • +