Merge pull request #65 in TPSH/ckanext-odsh from dev to master

* commit 'a706fda1':
  adds missing except
  adds correct error handling to logging of API requests
  fixes bug during logging of requests
  allows spatial_url_temp as alias for spatial_uri_temp
  adds posibility to log POST request to API, flag: ckanext.odsh.log_api_requests
  minor changes
  adds tests for validate_extra_groups if groups in extras from API
  further refactoring of validate_extra_groups
  refactors validate_extra_groups
  adds unittests for validate_extra_groups and starts refactoring
  adds unit tests for validate_licenseAttributionByText
  clears proxy after download
  implements upload of linked resources
  makes license attribution optional
  makes license attribution optional
  zwischenstand
  zwischenstand

ckanext/odsh/controller.py

@@ -20,6 +20,7 @@ import ckan.plugins.toolkit as toolkit
 from ckanext.dcat.controllers import DCATController
 import ckan.model as model
 import helpers
+import json
 
 
 abort = base.abort
@@ -143,6 +144,12 @@ class OdshGroupController(OrganizationController):
 
 class OdshApiController(ApiController):
     def action(self, logic_function, ver=None):
+        if toolkit.asbool(config.get('ckanext.odsh.log_api_requests', 'false')):
+            try:
+                request_data = self._get_request_data(try_url_params=False)
+                log.info('POST request body: {}'.format(json.dumps(json.loads(request_data))))
+            except Exception, e:
+                log.error(e)
         if logic_function == 'resource_qv4yAI2rgotamXGk98gJ':
             return helpers.odsh_get_version_id()
         if logic_function == 'resourcelog_qv4yAI2rgotamXGk98gJ':

ckanext/odsh/logic/action.py

-import logging
 import ckan.logic as logic
 from ckan.logic.action.update import user_update
-from ckan.logic.action.create import package_create, user_create, group_member_create
+from ckan.logic.action.create import package_create, resource_create, user_create, group_member_create
 import ckan.model as model
 import ckan.lib.dictization.model_dictize as model_dictize
 from ckan.lib.munge import munge_title_to_name
 import ckan.plugins.toolkit as toolkit
-from ckan.lib.search.common import (
-    make_connection, SearchError, SearchQueryError
-)
+from ckan.lib.search.common import make_connection, SearchError
 import pysolr
 import datetime
+import cgi
+import urllib2
 
+import logging
 log = logging.getLogger(__name__)
 
+from ckanext.odsh.setup_proxy import setup_proxy, clear_proxy
+
 
 def odsh_package_create(context, data_dict):
     pkg_type = data_dict.get('type', None)
@@ -52,8 +54,10 @@ def check_password(password):
             any(c.isupper() for c in password) and
             any((c.isalpha()==False) for c in password)) #Number or Special character
 
+
 PASSWORD_ERROR_MESSAGE =  {'security': ['Passwort muss mindestens acht Zeichen, einen Gross-, einen Kleinbuchstaben und entweder eine Zahl oder ein Sondernzeichen enthalten!']}       
 
+
 def odsh_user_create(context, data_dict):
     model = context['model']
     password = data_dict.get('password')
@@ -69,6 +73,7 @@ def odsh_user_create(context, data_dict):
     else:
         raise logic.ValidationError(PASSWORD_ERROR_MESSAGE)          
   
+
 def tpsh_user_update(context, data_dict):
       password = data_dict.get('password')
       if not password:
@@ -78,8 +83,6 @@ def tpsh_user_update(context, data_dict):
       return user_update(context, data_dict)
 
 
-
-
 @toolkit.side_effect_free
 def autocomplete(context, data_dict):
     query = {
@@ -102,3 +105,41 @@ def autocomplete(context, data_dict):
         filtered_suggestions.append(suggestion)
     final_suggestions = list(sorted(set(filtered_suggestions), key=filtered_suggestions.index))[:5]
     return final_suggestions
+
+
+def odsh_resource_create(context, data_dict):
+    is_linked_resource = not isinstance(data_dict['upload'], cgi.FieldStorage)
+    if is_linked_resource:
+        _download_linked_resource_to_tmp(data_dict['url'])
+        _emulate_file_upload(data_dict)
+    return resource_create(context, data_dict)
+
+TMP_FILE_PATH = '/tmp/temp_file_upload'
+
+def _download_linked_resource_to_tmp(url):
+    log.debug('Downloading linked resource from {}.'.format(url))
+    setup_proxy()
+    test_file = urllib2.urlopen(url).read()
+    clear_proxy()
+    with open(TMP_FILE_PATH, 'wb') as temporary_file:
+        temporary_file.write(test_file)
+
+def _emulate_file_upload(data_dict):
+    '''
+    This function updates the data_dict in order to emulate
+    the behaviour of resource creation with uploaded file for
+    resource creation with link
+    '''
+    temporary_file = open(TMP_FILE_PATH, 'rb')
+    upload_file = temporary_file
+    filename = data_dict['name']
+    upload = cgi.FieldStorage()
+    upload.disposition = 'form-data'
+    upload.disposition_options = {'filename': filename, 'name': 'upload'}
+    upload.fp = upload_file
+    upload.file = upload_file
+    upload.type = 'application/pdf'
+    upload.headers['content-type'] = upload.type
+    upload.name = 'upload'
+    upload.filename = filename
+    data_dict['upload'] = upload
\ No newline at end of file

ckanext/odsh/plugin.py

@@ -51,7 +51,9 @@ class OdshPlugin(plugins.SingletonPlugin, DefaultTranslation, DefaultDatasetForm
     def get_actions(self):
         return {'package_create': action.odsh_package_create,
                 'user_update':action.tpsh_user_update,
-                'user_create': action.odsh_user_create}
+                'user_create': action.odsh_user_create,
+                'resource_create': action.odsh_resource_create,
+                }
 
     
     # IConfigurer

ckanext/odsh/setup_proxy.py

+import urllib2
+from ckan.common import config
+
+
+def setup_proxy():
+    '''
+    This function declares that a proxy server shall be used to access the web via
+    urllib2. It takes the proxy address from ckan's config file 
+    (
+        field "ckanext.odsh.download_proxy",
+        example: ckanext.odsh.download_proxy = http://1.2.3.4:4123
+    )
+    '''
+
+    proxy_url = config.get('ckanext.odsh.download_proxy', None)
+    if proxy_url:
+        proxy = urllib2.ProxyHandler({'http': proxy_url, 'https': proxy_url})
+        opener = urllib2.build_opener(proxy)
+        urllib2.install_opener(opener)
+
+def clear_proxy():
+    proxy = urllib2.ProxyHandler({})
+    opener = urllib2.build_opener(proxy)
+    urllib2.install_opener(opener)
\ No newline at end of file

ckanext/odsh/tests_tpsh/resources/licenses.json

+[
+  {
+    "id": "http://dcat-ap.de/def/licenses/dl-zero-de/2.0",
+    "is_okd_compliant": true,
+    "is_osi_compliant": true,
+    "status": "active",
+    "title": "Datenlizenz Deutschland – Zero – Version 2.0",
+    "url": "https://www.govdata.de/dl-de/zero-2-0"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/dl-by-de/2.0",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Datenlizenz Deutschland Namensnennung 2.0",
+    "url": "https://www.govdata.de/dl-de/by-2-0"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/officialWork",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Amtliches Werk, lizenzfrei nach §5 Abs. 1 UrhG",
+    "url": "http://www.gesetze-im-internet.de/urhg/__5.html"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/cc-zero",
+    "title": "Creative Commons CC Zero License (cc-zero)",
+    "url": "http://www.opendefinition.org/licenses/cc-zero",
+    "status": "active",
+    "od_conformance": "not reviewed",
+    "osd_conformance": "not reviewed"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/cc-by/4.0",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Creative Commons Namensnennung – 4.0 International (CC BY 4.0)",
+    "url": "http://creativecommons.org/licenses/by/4.0/"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/cc-by-nd/4.0",
+    "is_okd_compliant": false,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Creative Commons Namensnennung - - Keine Bearbeitung 4.0 International (CC BY-ND 4.0)",
+    "url": "https://creativecommons.org/licenses/by-nd/4.0/"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/cc-by-sa/4.0",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Creative Commons Namensnennung - Weitergabe unter gleichen Bedingungen 4.0 International (CC-BY-SA 4.0)",
+    "url": "http://creativecommons.org/licenses/by-sa/4.0/"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/cc-by-nc/4.0",
+    "is_okd_compliant": false,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Creative Commons Namensnennung - Nicht kommerziell 4.0 International (CC BY-NC 4.0)",
+    "url": "http://creativecommons.org/licenses/by-nc/4.0/"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/odbl",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Open Data Commons Open Database License (ODbL)",
+    "url": "http://www.opendefinition.org/licenses/odc-odbl"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/odby",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Open Data Commons Attribution License (ODC-BY 1.0)",
+    "url": "http://www.opendefinition.org/licenses/odc-by"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/odcpddl",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Open Data Commons Public Domain Dedication and Licence (ODC PDDL)",
+    "url": "http://www.opendefinition.org/licenses/odc-pddl"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/apache",
+    "is_okd_compliant": false,
+    "is_osi_compliant": true,
+    "status": "active",
+    "title": "Freie Softwarelizenz der Apache Software Foundation",
+    "url": "http://www.apache.org/licenses"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/bsd",
+    "is_okd_compliant": false,
+    "is_osi_compliant": true,
+    "status": "active",
+    "title": "BSD Lizenz",
+    "url": "http://www.opensource.org/licenses/bsd-license.php"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/gpl/3.0",
+    "is_okd_compliant": false,
+    "is_osi_compliant": true,
+    "status": "active",
+    "title": "GNU General Public License version 3.0 (GPLv3)",
+    "url": "http://www.opensource.org/licenses/gpl-3.0.html"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/mozilla",
+    "is_okd_compliant": false,
+    "is_osi_compliant": true,
+    "status": "active",
+    "tags": [],
+    "title": "Mozilla Public License 2.0 (MPL)",
+    "url": "http://www.mozilla.org/MPL"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/gfdl",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "GNU Free Documentation License (GFDL)",
+    "url": "http://www.opendefinition.org/licenses/gfdl"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/cc-by-de/3.0",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Creative Commons Namensnennung 3.0 Deutschland (CC BY 3.0 DE)",
+    "url": "https://creativecommons.org/licenses/by/3.0/de/"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/cc-by-nd/3.0",
+    "is_okd_compliant": false,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Creative Commons Namensnennung -- Keine Bearbeitung 3.0 Unported (CC BY-ND 3.0)",
+    "url": "http://creativecommons.org/licenses/by-nd/3.0/"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/cc-by-nc-de/3.0",
+    "is_okd_compliant": false,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Creative Commons Namensnennung-Nicht kommerziell 3.0 Deutschland (CC BY-NC 3.0 DE)",
+    "url": "https://creativecommons.org/licenses/by-nc/3.0/de/"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/ccpdm/1.0",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Public Domain Mark 1.0 (PDM)",
+    "url": "http://creativecommons.org/publicdomain/mark/1.0/"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/geonutz/20130319",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Nutzungsbestimmungen für die Bereitstellung von Geodaten des Bundes",
+    "url": "http://www.geodatenzentrum.de/docpdf/geonutzv.pdf"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/dl-by-de/1.0",
+    "is_okd_compliant": true,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Datenlizenz Deutschland Namensnennung 1.0",
+    "url": "https://www.govdata.de/dl-de/by-1-0"
+  },
+  {
+    "id": "http://dcat-ap.de/def/licenses/dl-by-nc-de/1.0",
+    "is_okd_compliant": false,
+    "is_osi_compliant": false,
+    "status": "active",
+    "title": "Datenlizenz Deutschland Namensnennung nicht-kommerziell 1.0",
+    "url": "https://www.govdata.de/dl-de/by-nc-1-0"
+  }
+]

ckanext/odsh/tests_tpsh/test_checksum.py

@@ -54,6 +54,12 @@ class testHashException(object):
             hash = calculate_hash(f)
         nt.assert_equal(hash, expected_hash_pdf)
 
-               
+    def test_pdf_2(self):
+        # expected_hash_pdf produced by following command in bash:
+        # md5sum test.pdf
+        expected_hash_pdf = 'c897e2791f8a396dc73285ca48d824a4'
+        with open(dir_path + '/resources/test_2.pdf') as f:
+            hash = calculate_hash(f)
+        nt.assert_equal(hash, expected_hash_pdf)         
                
                
\ No newline at end of file

ckanext/odsh/tests_tpsh/test_validation.py

+import nose.tools as nt
+from ckan.common import config
+from .test_validation_mocks import WithFrontendValidationMocks, WithAPIValidationMocks
+from ckanext.odsh.validation import validate_licenseAttributionByText, validate_extra_groups
+import ckan.plugins.toolkit as toolkit
+
+
+class Test_validate_licenseAttributionByText(WithFrontendValidationMocks):
+    def test_it_passes(self):
+        validate_licenseAttributionByText(
+            self.key_mock,
+            self.data_mock_without_groups,
+            self.error_mock,
+            self.context_mock
+        )
+
+    def test_fails_if_licenseAttributionByText_missing(self):
+        data_mock = self.data_mock_without_groups.copy()
+        data_mock.update({
+            ('extras', 4, 'key'): u'',
+        })
+        with nt.assert_raises(toolkit.Invalid) as err:
+            validate_licenseAttributionByText(
+                self.key_mock,
+                data_mock,
+                self.error_mock,
+                self.context_mock
+            )
+        nt.assert_equal(err.exception.error,
+                        'licenseAttributionByText: empty not allowed')
+
+    def test_passes_if_licenseAttributionByText_missing_and_flag_set_to_false(self):
+        config.update({
+            'ckanext.odsh.require_license_attribution': 'False',
+        })
+        data_mock = self.data_mock_without_groups.copy()
+        data_mock.update({
+            ('extras', 4, 'key'): u'',
+        })
+        validate_licenseAttributionByText(
+            self.key_mock,
+            data_mock,
+            self.error_mock,
+            self.context_mock
+        )
+
+    def test_fails_if_licenseAttributionByText_given_for_license_wo_name(self):
+        data_mock = self.data_mock_without_groups.copy()
+        data_mock.update({
+            ('license_id',): u'http://dcat-ap.de/def/licenses/dl-zero-de/2.0',
+        })
+        with nt.assert_raises(toolkit.Invalid) as err:
+            validate_licenseAttributionByText(
+                self.key_mock,
+                data_mock,
+                self.error_mock,
+                self.context_mock
+            )
+        nt.assert_equal(
+            err.exception.error, 'licenseAttributionByText: text not allowed for this license')
+
+
+class Test_validate_extra_groupsFrontend(WithFrontendValidationMocks):
+    def test_fails_if_required_and_empty_string_in_extras(self):
+        data = self.data_mock_without_groups.copy()
+        errors = self.error_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=True,
+            errors=errors
+        )
+        nt.assert_equal(errors.get('groups'), 'at least one group needed')
+
+    def test_passes_if_required_and_groups_in_extras(self):
+        data = self.data_mock_with_groups.copy()
+        errors = self.error_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=True,
+            errors=errors
+        )
+        nt.assert_equal(errors.get('groups'), None)
+
+    def test_copies_groups_from_extras_one_level_up(self):
+        data = self.data_mock_with_groups.copy()
+        errors = self.error_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=True,
+            errors=errors
+        )
+        nt.assert_equal(
+            data.get(('groups', 0, 'id')),
+            u'soci'
+        )
+        nt.assert_equal(
+            data.get(('groups', 1, 'id')),
+            u'ener'
+        )
+        nt.assert_equal(
+            data.get(('groups', 2, 'id')),
+            u'intr'
+        )
+
+    def test_adds_empty_group_to_data_if_groups_in_extra_empty(self):
+        data = self.data_mock_without_groups.copy()
+        errors = self.error_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=False,
+            errors=errors
+        )
+        nt.assert_equal(
+            data.get(('groups', 0, 'id')),
+            u''
+        )
+
+
+class Test_validate_extra_groupsAPI(WithAPIValidationMocks):
+    def test_passes_if_required_and_outside_extras(self):
+        data = self.data_mock_with_groups.copy()
+        errors = self.errors_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=True,
+            errors=errors
+        )
+        nt.assert_equal(len(errors), 0)
+
+    def test_passes_if_required_and_in_extras(self):
+        data = self.data_mock_with_groups_in_extras.copy()
+        errors = self.errors_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=True,
+            errors=errors
+        )
+        nt.assert_equal(len(errors), 0)
+
+    def test_fails_if_required_and_not_given(self):
+        data = self.data_mock_without_groups.copy()
+        errors = self.errors_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=True,
+            errors=errors
+        )
+        nt.assert_equal(len(errors), 1)
+
+    def test_fails_if_required_and_empty(self):
+        data = self.data_mock_with_empty_groups.copy()
+        errors = self.errors_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=True,
+            errors=errors
+        )
+        nt.assert_equal(len(errors), 1)
+
+    def test_fails_if_required_and_empty_in_extras(self):
+        data = self.data_mock_with_empty_groups_in_extras.copy()
+        errors = self.errors_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=True,
+            errors=errors
+        )
+        nt.assert_equal(len(errors), 1)
+
+    def test_passes_if_not_required_and_outside_extras(self):
+        data = self.data_mock_with_groups.copy()
+        errors = self.errors_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=False,
+            errors=errors
+        )
+        nt.assert_equal(len(errors), 0)
+
+    def test_passes_if_not_required_and_not_given(self):
+        data = self.data_mock_without_groups.copy()
+        errors = self.errors_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=False,
+            errors=errors
+        )
+        nt.assert_equal(len(errors), 0)
+
+    def test_passes_if_required_and_empty(self):
+        data = self.data_mock_with_empty_groups
+        errors = self.errors_mock.copy()
+        validate_extra_groups(
+            data=data,
+            requireAtLeastOne=False,
+            errors=errors
+        )
+        nt.assert_equal(len(errors), 0)

ckanext/odsh/tests_tpsh/test_validation_mocks.py

+import os
+from ckan.common import config
+from ckan.lib.navl.dictization_functions import Missing
+
+
+def resource_dir():
+    this_module_dir = os.path.dirname(os.path.abspath(__file__))
+    result = os.path.abspath(os.path.join(this_module_dir, 'resources/'))
+    print(result)
+    return result
+
+
+def license_file_url():
+    result = 'file://' + \
+        os.path.abspath(os.path.join(resource_dir(), 'licenses.json'))
+    return result
+
+
+class WithConfig(object):
+    def setUp(self):
+        config.update({
+            'ckanext.odsh.require_license_attribution': 'True',
+            'licenses_group_url': license_file_url(),
+        })
+
+
+class WithFrontendValidationMocks(WithConfig):
+    def setUp(self):
+        super(WithFrontendValidationMocks, self).setUp()
+
+        self.key_mock = ('extras', 0, 'key')
+
+        self.data_mock_without_groups = {
+            ('__extras',): {
+                'pkg_name': u'458ccd2c-5e1b-474f-943f-3bab9439a0e5',
+                'spatial_uri_temp': u'',
+                'tags': []
+            },
+            ('extras', 0, 'key'): u'groups',
+            ('extras', 0, 'revision_timestamp'): Missing(),
+            ('extras', 0, 'state'): Missing(),
+            ('extras', 0, 'value'): u'',
+            ('extras', 1, 'deleted'): Missing(),
+            ('extras', 1, 'id'): Missing(),
+            ('extras', 1, 'key'): u'issued',
+            ('extras', 1, 'revision_timestamp'): Missing(),
+            ('extras', 1, 'state'): Missing(),
+            ('extras', 1, 'value'): u'2019-12-17',
+            ('extras', 2, 'deleted'): Missing(),
+            ('extras', 2, 'id'): Missing(),
+            ('extras', 2, 'key'): u'temporal_end',
+            ('extras', 2, 'revision_timestamp'): Missing(),
+            ('extras', 2, 'state'): Missing(),
+            ('extras', 2, 'value'): u'',
+            ('extras', 3, 'deleted'): Missing(),
+            ('extras', 3, 'id'): Missing(),
+            ('extras', 3, 'key'): u'temporal_start',
+            ('extras', 3, 'revision_timestamp'): Missing(),
+            ('extras', 3, 'state'): Missing(),
+            ('extras', 3, 'value'): u'',
+            ('extras', 4, 'deleted'): Missing(),
+            ('extras', 4, 'id'): Missing(),
+            ('extras', 4, 'key'): u'licenseAttributionByText',
+            ('extras', 4, 'revision_timestamp'): Missing(),
+            ('extras', 4, 'state'): Missing(),
+            ('extras', 4, 'value'): u'Test-Organisation',
+            ('extras', 5, 'deleted'): Missing(),
+            ('extras', 5, 'id'): Missing(),
+            ('extras', 5, 'key'): 'thumbnail',
+            ('extras', 5, 'revision_timestamp'): Missing(),
+            ('extras', 5, 'state'): Missing(),
+            ('extras', 5, 'value'): u'thumbnail_picture_4519ffc6a956a638b7b6f7a6a82418.jpg',
+            ('extras', 6, 'key'): 'language',
+            ('extras', 6, 'value'): u'http://publications.europa.eu/resource/authority/language/DEU',
+            ('extras', 7, 'key'): 'subject_text',
+            ('extras', 7, 'value'): u'Verwaltungsvorschrift',
+            ('extras', 8, 'key'): 'subject',
+            ('extras', 8, 'value'): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Verwaltungsvorschrift',
+            ('extras', 9, 'key'): 'spatial_uri',
+            ('extras', 9, 'value'): u'',
+            ('id',): u'458ccd2c-5e1b-474f-943f-3bab9439a0e5',
+            ('language',): u'http://publications.europa.eu/resource/authority/language/DEU',
+            ('license_id',): u'http://dcat-ap.de/def/licenses/dl-by-de/2.0',
+            ('notes',): u'',
+            ('owner_org',): u'63c87e74-60a9-4a4a-a980-d7983c47f92b',
+            ('private',): False,
+            ('subject',): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Verwaltungsvorschrift',
+            ('tag_string',): u'',
+            ('title',): u'Link again',
+            ('type',): u'dataset'
+        }
+
+        self.data_mock_with_groups = {
+            ('__extras',): {'pkg_name': u'458ccd2c-5e1b-474f-943f-3bab9439a0e5',
+                            'spatial_uri_temp': u'',
+                            'tags': []},
+            ('extras', 0, 'key'): u'groups',
+            ('extras', 0, 'value'): u'soci,ener,intr',
+            ('extras', 1, 'key'): u'issued',
+            ('extras', 1, 'value'): u'2019-12-17',
+            ('extras', 2, 'key'): u'temporal_end',
+            ('extras', 2, 'value'): u'',
+            ('extras', 3, 'key'): u'temporal_start',
+            ('extras', 3, 'value'): u'',
+            ('extras', 5, 'key'): 'thumbnail',
+            ('extras', 5, 'value'): u'thumbnail_picture_4519ffc6a956a638b7b6f7a6a82418.jpg',
+            ('extras', 6, 'key'): 'language',
+            ('extras', 6, 'value'): u'http://publications.europa.eu/resource/authority/language/DEU',
+            ('extras', 7, 'key'): 'subject_text',
+            ('extras', 7, 'value'): u'Verwaltungsvorschrift',
+            ('extras', 8, 'key'): 'subject',
+            ('extras', 8, 'value'): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Verwaltungsvorschrift',
+            ('extras', 9, 'key'): 'spatial_uri',
+            ('extras', 9, 'value'): u'',
+            ('extras', 10, 'key'): 'licenseAttributionByText',
+            ('extras', 10, 'value'): '',
+            ('extras', 11, 'key'): 'licenseAttributionByText',
+            ('extras', 11, 'value'): '',
+            ('extras', 12, 'key'): 'licenseAttributionByText',
+            ('extras', 12, 'value'): '',
+            ('extras', 13, 'key'): 'licenseAttributionByText',
+            ('extras', 13, 'value'): '',
+            ('extras', 14, 'key'): 'licenseAttributionByText',
+            ('extras', 14, 'value'): '',
+            ('extras', 15, 'key'): 'licenseAttributionByText',
+            ('extras', 15, 'value'): '',
+            ('id',): u'458ccd2c-5e1b-474f-943f-3bab9439a0e5',
+            ('language',): u'http://publications.europa.eu/resource/authority/language/DEU',
+            ('license_id',): u'http://dcat-ap.de/def/licenses/dl-zero-de/2.0',
+            ('notes',): u'',
+            ('owner_org',): u'63c87e74-60a9-4a4a-a980-d7983c47f92b',
+            ('private',): False,
+            ('subject',): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Verwaltungsvorschrift',
+            ('tag_string',): u'',
+            ('title',): u'Link again',
+            ('type',): u'dataset'
+        }
+
+        self.error_mock = {
+            ('__before',): [],
+            ('__extras',): [],
+            ('__junk',): [],
+            ('author',): [],
+            ('author_email',): [],
+            ('extras', 0, '__extras'): [],
+            ('extras', 0, 'deleted'): [],
+            ('extras', 0, 'id'): [],
+            ('extras', 0, 'key'): [],
+            ('extras', 0, 'revision_timestamp'): [],
+            ('extras', 0, 'state'): [],
+            ('extras', 0, 'value'): [],
+            ('extras', 1, '__extras'): [],
+            ('extras', 1, 'deleted'): [],
+            ('extras', 1, 'id'): [],
+            ('extras', 1, 'key'): [],
+            ('extras', 1, 'revision_timestamp'): [],
+            ('extras', 1, 'state'): [],
+            ('extras', 1, 'value'): [],
+            ('extras', 2, '__extras'): [],
+            ('extras', 2, 'deleted'): [],
+            ('extras', 2, 'id'): [],
+            ('extras', 2, 'key'): [],
+            ('extras', 2, 'revision_timestamp'): [],
+            ('extras', 2, 'state'): [],
+            ('extras', 2, 'value'): [],
+            ('extras', 3, '__extras'): [],
+            ('extras', 3, 'deleted'): [],
+            ('extras', 3, 'id'): [],
+            ('extras', 3, 'key'): [],
+            ('extras', 3, 'revision_timestamp'): [],
+            ('extras', 3, 'state'): [],
+            ('extras', 3, 'value'): [],
+            ('extras', 4, '__extras'): [],
+            ('extras', 4, 'deleted'): [],
+            ('extras', 4, 'id'): [],
+            ('extras', 4, 'key'): [],
+            ('extras', 4, 'revision_timestamp'): [],
+            ('extras', 4, 'state'): [],
+            ('extras', 4, 'value'): [],
+            ('extras', 5, '__extras'): [],
+            ('extras', 5, 'deleted'): [],
+            ('extras', 5, 'id'): [],
+            ('extras', 5, 'key'): [],
+            ('extras', 5, 'revision_timestamp'): [],
+            ('extras', 5, 'state'): [],
+            ('extras', 5, 'value'): [],
+            ('id',): [],
+            ('language',): [],
+            ('license_id',): [],
+            ('log_message',): [],
+            ('maintainer',): [],
+            ('maintainer_email',): [],
+            ('name',): [],
+            ('notes',): [],
+            ('owner_org',): [],
+            ('private',): [],
+            ('return_to',): [],
+            ('revision_id',): [],
+            ('save',): [],
+            ('state',): [],
+            ('subject',): [],
+            ('tag_string',): [],
+            ('thumbnail',): [],
+            ('title',): [],
+            ('type',): [],
+            ('url',): [],
+            ('version',): []
+        }
+
+        self.context_mock = None
+
+
+class WithAPIValidationMocks(WithConfig):
+    def setUp(self):
+        super(WithAPIValidationMocks, self).setUp()
+
+        self.data_mock_with_groups = {
+            ('__extras',): {'tags': []},
+            ('extras', 0, 'key'): u'issued',
+            ('extras', 0, 'value'): u'2020-02-28T00:00:00.000',
+            ('extras', 1, 'key'): 'subject_text',
+            ('extras', 1, 'value'): u'Gutachten',
+            ('extras', 2, 'key'): 'subject',
+            ('extras', 2, 'value'): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Gutachten',
+            ('extras', 3, 'key'): 'licenseAttributionByText',
+            ('extras', 3, 'value'): '',
+            ('groups', 0, 'name'): u'heal',
+            ('license_id',): u'http://dcat-ap.de/def/licenses/dl-zero-de/2.0',
+            ('name',): u'aaa',
+            ('owner_org',): u'ce91bacd-043d-4546-8239-c891d4a221f8',
+            ('subject',): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Gutachten',
+            ('title',): u'aaa',
+            ('type',): u'dataset'
+        }
+
+        self.data_mock_with_empty_groups = {
+            ('__extras',): {'groups': [], 'tags': []},
+            ('extras', 0, 'key'): u'issued',
+            ('extras', 0, 'value'): u'2020-02-28T00:00:00.000',
+            ('extras', 1, 'key'): 'subject_text',
+            ('extras', 1, 'value'): u'Gutachten',
+            ('extras', 2, 'key'): 'subject',
+            ('extras', 2, 'value'): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Gutachten',
+            ('extras', 3, 'key'): 'licenseAttributionByText',
+            ('extras', 3, 'value'): '',
+            ('license_id',): u'http://dcat-ap.de/def/licenses/dl-zero-de/2.0',
+            ('name',): u'aaa',
+            ('owner_org',): u'ce91bacd-043d-4546-8239-c891d4a221f8',
+            ('subject',): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Gutachten',
+            ('title',): u'aaa',
+            ('type',): u'dataset'
+        }
+
+        self.data_mock_with_groups_in_extras = {
+            ('__extras',): {'tags': []},
+            ('extras', 0, 'key'): u'issued',
+            ('extras', 0, 'value'): u'2020-02-28T00:00:00.000',
+            ('extras', 1, 'key'): u'groups',
+            ('extras', 1, 'value'): u'soci,ener,intr',
+            ('extras', 2, 'key'): 'subject_text',
+            ('extras', 2, 'value'): u'Gutachten',
+            ('extras', 3, 'key'): 'subject',
+            ('extras', 3, 'value'): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Gutachten',
+            ('extras', 4, 'key'): 'licenseAttributionByText',
+            ('extras', 4, 'value'): '',
+            ('extras', 5, 'key'): 'licenseAttributionByText',
+            ('extras', 5, 'value'): '',
+            ('license_id',): u'http://dcat-ap.de/def/licenses/dl-zero-de/2.0',
+            ('name',): u'aaa',
+            ('owner_org',): u'ce91bacd-043d-4546-8239-c891d4a221f8',
+            ('subject',): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Gutachten',
+            ('title',): u'aaa',
+            ('type',): u'dataset'
+        }
+
+        self.data_mock_with_empty_groups_in_extras = {
+            ('__extras',): {'tags': []},
+            ('extras', 0, 'key'): u'issued',
+            ('extras', 0, 'value'): u'2020-02-28T00:00:00.000',
+            ('extras', 1, 'key'): u'groups',
+            ('extras', 1, 'value'): u'',
+            ('extras', 2, 'key'): 'subject_text',
+            ('extras', 2, 'value'): u'Gutachten',
+            ('extras', 3, 'key'): 'subject',
+            ('extras', 3, 'value'): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Gutachten',
+            ('extras', 4, 'key'): 'licenseAttributionByText',
+            ('extras', 4, 'value'): '',
+            ('extras', 5, 'key'): 'licenseAttributionByText',
+            ('extras', 5, 'value'): '',
+            ('license_id',): u'http://dcat-ap.de/def/licenses/dl-zero-de/2.0',
+            ('name',): u'aaa',
+            ('owner_org',): u'ce91bacd-043d-4546-8239-c891d4a221f8',
+            ('subject',): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Gutachten',
+            ('title',): u'aaa',
+            ('type',): u'dataset'
+        }
+
+        self.data_mock_without_groups = {
+            ('__extras',): {'tags': []},
+            ('extras', 0, 'key'): u'issued',
+            ('extras', 0, 'value'): u'2020-02-28T00:00:00.000',
+            ('extras', 1, 'key'): 'subject_text',
+            ('extras', 1, 'value'): u'Gutachten',
+            ('extras', 2, 'key'): 'subject',
+            ('extras', 2, 'value'): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Gutachten',
+            ('extras', 3, 'key'): 'licenseAttributionByText',
+            ('extras', 3, 'value'): '',
+            ('license_id',): u'http://dcat-ap.de/def/licenses/dl-zero-de/2.0',
+            ('name',): u'aaa',
+            ('owner_org',): u'ce91bacd-043d-4546-8239-c891d4a221f8',
+            ('subject',): u'http://transparenz.schleswig-holstein.de/informationsgegenstand#Gutachten',
+            ('title',): u'aaa',
+            ('type',): u'dataset'
+        }
+
+        self.errors_mock = {}

ckanext/odsh/validation.py

@@ -31,35 +31,46 @@ def _extract_value(data, field):
     return data[(key[0], key[1], 'value')]
 
 
+ERROR_MSG_NO_GROUP = 'at least one group needed'
+
 def validate_extra_groups(data, requireAtLeastOne, errors):
-    value = _extract_value(data, 'groups')
-    error_message_no_group = 'at least one group needed'
-    if value != None:
-        # 'value != None' means the extra key 'groups' was found,
-        # so the dataset came from manual editing via the web-frontend.
-        if not value:
-            if requireAtLeastOne:
-                errors['groups'] = error_message_no_group
-            data[('groups', 0, 'id')] = ''
-            return
+    groups = _groups_from_data(data)
+    if groups is not None:
+        # groups are in extras
+        if len(groups) == 0 and requireAtLeastOne:
+            errors['groups'] = ERROR_MSG_NO_GROUP
+        _clear_groups(data)
+        _copy_groups_one_level_up(data, groups)
+    else:  
+        # no extra-field 'groups'
+        if (not _at_least_one_group_outside_extras(data)) and requireAtLeastOne:
+            errors['groups'] = ERROR_MSG_NO_GROUP
 
-        groups = [g.strip() for g in value.split(',') if value.strip()]
+def _groups_from_data(data):
+    groups_csv = _extract_value(data, 'groups')
+    try:
+        groups = list(csv.reader([groups_csv]))[0]
+    except TypeError:
+        groups = None
+    return groups
+
+def _clear_groups(data):
     for k in data.keys():
         if len(k) == 3 and k[0] == 'groups':
-                data[k] = ''
-                # del data[k]
-        if len(groups) == 0:
-            if requireAtLeastOne:
-                errors['groups'] = error_message_no_group
-            return
+            data[k] = '' # dead code [?]
 
+def _copy_groups_one_level_up(data, groups):
+    if len(groups) == 0:
+        data[('groups', 0, 'id')] = ''
+    else:
         for num, group in zip(range(len(groups)), groups):
             data[('groups', num, 'id')] = group
-    else:  # no extra-field 'groups'
-        # dataset might come from a harvest process
-        if not data.get(('groups', 0, 'id'), False) and \
-           not data.get(('groups', 0, 'name'), False):
-            errors['groups'] = error_message_no_group
+
+def _at_least_one_group_outside_extras(data):
+    return (
+        ('groups', 0, 'id') in data or
+        ('groups', 0, 'name') in data
+    )
 
 
 def validate_extras(key, data, errors, context):
@@ -143,6 +154,24 @@ def validate_extra_date_new(key, field, data, optional, errors):
 
 
 def validate_licenseAttributionByText(key, data, errors, context):
+    require_license_attribution = toolkit.asbool(
+        config.get('ckanext.odsh.require_license_attribution', True)
+    )
+    isByLicense = _isByLicense(data)
+    hasAttribution = _hasAttribution(data)
+    if not hasAttribution:
+        _add_empty_attribution(data)
+
+    if isByLicense and (not hasAttribution) and require_license_attribution:
+        raise toolkit.Invalid(
+            'licenseAttributionByText: empty not allowed')
+
+    if (not isByLicense) and hasAttribution:
+        raise toolkit.Invalid(
+            'licenseAttributionByText: text not allowed for this license')
+
+
+def _isByLicense(data):
     register = model.Package.get_license_register()
     isByLicense = False
     for k in data:
@@ -150,6 +179,10 @@ def validate_licenseAttributionByText(key, data, errors, context):
                 'Namensnennung' in register[data[k]].title:
             isByLicense = True
             break
+    return isByLicense
+
+
+def _hasAttribution(data):
     hasAttribution = False
     for k in data:
         if data[k] == 'licenseAttributionByText':
@@ -161,26 +194,24 @@ def validate_licenseAttributionByText(key, data, errors, context):
                 value = data[(k[0], k[1], 'value')]
                 hasAttribution = value != ''
                 break
-    if not hasAttribution:
-        current_indexes = [k[1] for k in data.keys()
-                           if len(k) > 1 and k[0] == 'extras']
+    return hasAttribution
+
 
+def _add_empty_attribution(data):
+    current_indexes = [
+        k[1] for k in data.keys()
+        if len(k) > 1 and k[0] == 'extras'
+    ]
     new_index = max(current_indexes) + 1 if current_indexes else 0
     data[('extras', new_index, 'key')] = 'licenseAttributionByText'
     data[('extras', new_index, 'value')] = ''
 
-    if isByLicense and not hasAttribution:
-        raise toolkit.Invalid(
-            'licenseAttributionByText: empty not allowed')
-
-    if not isByLicense and hasAttribution:
-        raise toolkit.Invalid(
-            'licenseAttributionByText: text not allowed for this license')
-
 
 def known_spatial_uri(key, data, errors, context):
     if data.get(('__extras',)) and 'spatial_uri_temp' in data.get(('__extras',)):
         _copy_spatial_uri_temp_to_extras(data)
+    if data.get(('__extras',)) and 'spatial_url_temp' in data.get(('__extras',)):
+        _copy_spatial_uri_temp_to_extras(data)
     value = _extract_value(data, 'spatial_uri')
     require_spatial_uri = toolkit.asbool(
         config.get('ckanext.odsh.require_spatial_uri', False)
@@ -243,10 +274,13 @@ def known_spatial_uri(key, data, errors, context):
 
 def _copy_spatial_uri_temp_to_extras(data):
     '''
-    copy the field spatial_uri_temp originating 
+    copy the fields spatial_uri_temp or 
+    spatial_url_temp originating 
     from the user interface to extras
     '''
     spatial_uri = data.get(('__extras',)).get('spatial_uri_temp')
+    if spatial_uri is None:
+        spatial_uri = data.get(('__extras',)).get('spatial_url_temp')
     is_spatial_uri_in_extras = _extract_value(data, 'spatial_uri') is not None
     if not is_spatial_uri_in_extras:
         next_index = next_extra_index(data)