[#649] Refactor form_to_db_schema()

Replace form_to_db_schema(), form_to_db_schema_options(),
form_to_db_schema_api_create(), form_to_db_schema_api_update(),
form_to_db_package_schema() with just two schemas: create and update.
Then get the tests passing.

schema.py:

- Delete _base_package_schema()
- Delete already deprecated package_form_schema()
- Delete form_to_db_package_schema()
- Tweak default_create_package_schema() and
  default_update_package_schema() to get tests passing

DefaultDatasetForm:

- Delete form_to_db_schema_options()
- Delete form_to_db_schema()
- Delete form_to_db_schema_api_create()
- Delete form_to_db_schema_api_update()
- Add create_package_schema(), returns None
- Add update_package_schema(), returns None

Note that by deleting form_to_db_schema() and
form_to_db_schema_options(), _api_create() and _api_update(), we're
breaking backwards compatibility with any plugins that were using these.

package_create():
- Don't call form_to_db_schema_options() or form_to_db_schema().
- Instead call the package plugin's create_package_schema(), if that
  doesn't exist or if it returns None then call
  default_create_package_schema() instead

package_update():
- Don't call form_to_db_schema_options() or form_to_db_schema().
- Instead call the package plugin's update_package_schema(), if that
  doesn't exist or if it returns None then call
  default_update_package_schema() instead

Note that by no longer calling the form_to_db_schema_options() or
form_to_db_schema() methods of IDatasetForm plugins, we're breaking
backward compatibility with those plugins.

Note this makes IDatasetForm less flexible - plugins can return
different schemas for creating, updating or showing packages, but they
can no longer return different schemas depending on whether we're using
the API or web UI, or on other parameters from the Pylons context.

test_tag_vocab.py:

- Update MockVocabTagsPlugin to work with the new interface

Also update some other tests.

ckan/lib/plugins.py

@@ -218,35 +218,11 @@ def history_template(self):
     def package_form(self):
         return 'package/new_package_form.html'
 
-    def form_to_db_schema_options(self, options):
-        ''' This allows us to select different schemas for different
-        purpose eg via the web interface or via the api or creation vs
-        updating. It is optional and if not available form_to_db_schema
-        should be used.
-        If a context is provided, and it contains a schema, it will be
-        returned.
-        '''
-        schema = options.get('context', {}).get('schema', None)
-        if schema:
-            return schema
-
-        if options.get('api'):
-            if options.get('type') == 'create':
-                return self.form_to_db_schema_api_create()
-            else:
-                assert options.get('type') == 'update'
-                return self.form_to_db_schema_api_update()
-        else:
-            return self.form_to_db_schema()
-
-    def form_to_db_schema(self):
-        return logic.schema.form_to_db_package_schema()
+    def create_package_schema(self):
+        return None
 
-    def form_to_db_schema_api_create(self):
-        return logic.schema.default_create_package_schema()
-
-    def form_to_db_schema_api_update(self):
-        return logic.schema.default_update_package_schema()
+    def update_package_schema(self):
+        return None
 
     def db_to_form_schema(self):
         '''This is an interface to manipulate data from the database

ckan/logic/action/create.py

@@ -108,11 +108,11 @@ def package_create(context, data_dict):
     package_type = data_dict.get('type')
     package_plugin = lib_plugins.lookup_package_plugin(package_type)
     try:
-        schema = package_plugin.form_to_db_schema_options({'type':'create',
-                                               'api':'api_version' in context,
-                                               'context': context})
+        schema = package_plugin.create_package_schema()
     except AttributeError:
-        schema = package_plugin.form_to_db_schema()
+        schema = None
+    if schema is None:
+        schema = ckan.logic.schema.default_create_package_schema()
 
     _check_access('package_create', context, data_dict)
 

ckan/logic/action/update.py

@@ -236,11 +236,11 @@ def package_update(context, data_dict):
     # get the schema
     package_plugin = lib_plugins.lookup_package_plugin(pkg.type)
     try:
-        schema = package_plugin.form_to_db_schema_options({'type':'update',
-                                               'api':'api_version' in context,
-                                               'context': context})
+        schema = package_plugin.update_package_schema()
     except AttributeError:
-        schema = package_plugin.form_to_db_schema()
+        schema = None
+    if schema is None:
+        schema = ckan.logic.schema.default_update_package_schema()
 
     if 'api_version' not in context:
         # old plugins do not support passing the schema so we need

ckan/logic/schema.py

@@ -114,10 +114,9 @@ def default_create_tag_schema():
     return schema
 
 
-def _base_package_schema():
-    '''Return a base schema for other package schemas.'''
+def default_create_package_schema():
     schema = {
-        'id': [ignore_missing, unicode, package_id_exists],
+        'id': [empty],
         'revision_id': [ignore],
         'name': [not_empty, unicode, name_validator, package_name_validator],
         'title': [if_empty_same_as("name"), unicode],
@@ -132,12 +131,17 @@ def _base_package_schema():
         'state': [ignore_not_package_admin, ignore_missing],
         'type': [ignore_missing, unicode],
         'owner_org': [owner_org_validator, unicode],
+        'log_message': [ignore_missing, unicode, no_http],
         'private': [ignore_missing, boolean_validator],
         '__extras': [ignore],
         '__junk': [empty],
         'resources': default_resource_schema(),
         'tags': default_tags_schema(),
+        'tag_string': [ignore_missing, tag_string_convert],
         'extras': default_extras_schema(),
+        'extras_validation': [duplicate_extras_key, ignore],
+        'save': [ignore],
+        'return_to': [ignore],
         'relationships_as_object': default_relationship_schema(),
         'relationships_as_subject': default_relationship_schema(),
         'groups': {
@@ -149,58 +153,31 @@ def _base_package_schema():
     }
     return schema
 
-def default_create_package_schema():
-
-    schema = _base_package_schema()
-    schema["id"] = [empty]
-
-    return schema
-
 def default_update_package_schema():
+    schema = default_create_package_schema()
 
-    schema = _base_package_schema()
-    schema["id"] = [ignore_missing, package_id_not_changed]
-    schema["name"] = [ignore_missing, name_validator, package_name_validator, unicode]
-    schema["title"] = [ignore_missing, unicode]
-
-    schema['private'] = [ignore_missing, boolean_validator]
-    schema['owner_org'] = [ignore_missing, owner_org_validator, unicode]
-    return schema
-
-
-@maintain.deprecated()
-def package_form_schema():
-    '''DEPRECATED. Use form_to_db_package_schema() instead.'''
-    return form_to_db_package_schema()
+    # Users can (optionally) supply the package id when updating a package, but
+    # only to identify the package to be updated, they cannot change the id.
+    schema['id'] = [ignore_missing, package_id_not_changed]
 
+    # Supplying the package name when updating a package is optional (you can
+    # supply the id to identify the package instead).
+    schema['name'] = [ignore_missing, name_validator, package_name_validator,
+            unicode]
 
-def form_to_db_package_schema():
+    # Supplying the package title when updating a package is optional, if it's
+    # not supplied the title will not be changed.
+    schema['title'] = [ignore_missing, unicode]
 
-    schema = _base_package_schema()
-    ##new
-    schema['log_message'] = [ignore_missing, unicode, no_http]
-    schema['groups'] = {
-            'id': [ignore_missing, unicode],
-            '__extras': [ignore],
-    }
-    schema['tag_string'] = [ignore_missing, tag_string_convert]
-    schema['extras_validation'] = [duplicate_extras_key, ignore]
-    schema['save'] = [ignore]
-    schema['return_to'] = [ignore]
-    schema['type'] = [ignore_missing, unicode]
-    schema['private'] = [ignore_missing, boolean_validator]
     schema['owner_org'] = [ignore_missing, owner_org_validator, unicode]
 
-    ##changes
-    schema.pop("id")
-    schema.pop('relationships_as_object')
-    schema.pop('revision_id')
-    schema.pop('relationships_as_subject')
-
     return schema
 
 def db_to_form_package_schema():
-    schema = _base_package_schema()
+    schema = default_create_package_schema()
+
+    # Don't strip ids from package dicts when validating them.
+    schema['id'] = []
 
     schema.update({
         'tags': {'__extras': [ckan.lib.navl.validators.keep_extras]}})
@@ -250,7 +227,7 @@ def db_to_form_package_schema():
     schema['url'] = []
     schema['version'] = []
 
-    # Add several keys that are missing from _base_package_schema(), so
+    # Add several keys that are missing from default_create_package_schema(), so
     # validation doesn't strip the keys from the package dicts.
     schema['metadata_created'] = []
     schema['metadata_modified'] = []

ckan/tests/lib/test_dictization_schema.py

@@ -4,7 +4,8 @@
 from ckan import model
 from ckan.lib.dictization.model_dictize import (package_dictize,
                                                 group_dictize)
-from ckan.logic.schema import (_base_package_schema,
+from ckan.logic.schema import (default_create_package_schema,
+                               default_update_package_schema,
                                default_group_schema,
                                default_tags_schema)
 from ckan.lib.navl.dictization_functions import validate
@@ -60,7 +61,7 @@ def test_1_package_schema(self):
         del result['relationships_as_subject']
 
         converted_data, errors = validate(result,
-                                          _base_package_schema(),
+                                          default_create_package_schema(),
                                           self.context)
 
         expected_data = {
@@ -107,7 +108,7 @@ def test_1_package_schema(self):
         data["resources"][1].pop("url")
 
         converted_data, errors = validate(data,
-                                          _base_package_schema(),
+                                          default_create_package_schema(),
                                           self.context)
 
         assert errors == {
@@ -118,7 +119,7 @@ def test_1_package_schema(self):
         data["id"] = package_id
 
         converted_data, errors = validate(data,
-                                          _base_package_schema(),
+                                          default_update_package_schema(),
                                           self.context)
 
         assert errors == {
@@ -128,7 +129,7 @@ def test_1_package_schema(self):
         data['name'] = '????jfaiofjioafjij'
 
         converted_data, errors = validate(data,
-                                          _base_package_schema(),
+                                          default_update_package_schema(),
                                           self.context)
         assert errors == {
             'name': [u'Url must be purely lowercase alphanumeric (ascii) '

ckan/tests/schema/test_schema.py

@@ -8,7 +8,7 @@ class TestPackage:
     def test_name_validation(self):
         context = {'model': ckan.model,
                    'session': ckan.model.Session}
-        schema = ckan.logic.schema._base_package_schema()
+        schema = ckan.logic.schema.default_create_package_schema()
         def get_package_name_validation_errors(package_name):
             data_dict = {'name': package_name}
             data, errors = validate(data_dict, schema, context)
@@ -37,7 +37,7 @@ def get_package_name_validation_errors(package_name):
     def test_version_validation(self):
         context = {'model': ckan.model,
                    'session': ckan.model.Session}
-        schema = ckan.logic.schema._base_package_schema()
+        schema = ckan.logic.schema.default_create_package_schema()
         def get_package_version_validation_errors(package_version):
             data_dict = {'version': package_version}
             data, errors = validate(data_dict, schema, context)
@@ -94,7 +94,7 @@ def test_tag_string_parsing(self):
         # errors correctly.
         context = {'model': ckan.model,
                    'session': ckan.model.Session}
-        schema = ckan.logic.schema.form_to_db_package_schema()
+        schema = ckan.logic.schema.default_update_package_schema()
 
         # basic parsing of comma separated values
         tests = (('tag', ['tag'], []),

ckanext/test_tag_vocab_plugin.py

@@ -7,7 +7,7 @@
 from genshi.filters import Transformer
 from ckan.logic import get_action
 from ckan.logic.converters import convert_to_tags, convert_from_tags, free_tags_only
-from ckan.logic.schema import _base_package_schema
+from ckan.logic.schema import default_create_package_schema, default_update_package_schema, db_to_form_package_schema
 from ckan.lib.navl.validators import ignore_missing, keep_extras
 from ckan import plugins
 
@@ -49,15 +49,22 @@ def package_form(self):
     def setup_template_variables(self, context, data_dict=None):
         c.vocab_tags = get_action('tag_list')(context, {'vocabulary_id': TEST_VOCAB_NAME})
 
-    def form_to_db_schema(self):
-        schema = _base_package_schema()
+    def create_package_schema(self):
+        schema = default_create_package_schema()
+        schema.update({
+            'vocab_tags': [ignore_missing, convert_to_tags(TEST_VOCAB_NAME)],
+        })
+        return schema
+
+    def update_package_schema(self):
+        schema = default_update_package_schema()
         schema.update({
             'vocab_tags': [ignore_missing, convert_to_tags(TEST_VOCAB_NAME)],
         })
         return schema
 
     def db_to_form_schema(self):
-        schema = _base_package_schema()
+        schema = db_to_form_package_schema()
         schema.update({
             'tags': {
                 '__extras': [keep_extras, free_tags_only]