Merge pull request #184 from ckan/LondonAppDev-master

Allow Harvest Sources with duplicate URLs if they have unique configs

ckanext/harvest/logic/validators.py

@@ -60,16 +60,26 @@ def _normalize_url(url):
 
 
 def harvest_source_url_validator(key, data, errors, context):
-    package = context.get("package")
+    '''Validate the provided harvest source URL
+
+    Checks that the URL & config combination are unique to this HarvestSource.
+    '''
+
+    package = context.get('package')
 
     if package:
         package_id = package.id
     else:
-        package_id = data.get(key[:-1] + ("id",))
+        package_id = data.get(key[:-1] + ('id',))
+
+    try:
+        new_config = data.get(key[:-1] + ('config',))
+    except:
+        new_config = None
 
     new_url = _normalize_url(data[key])
 
-    q = model.Session.query(model.Package.url, model.Package.state) \
+    q = model.Session.query(model.Package.id, model.Package.url) \
              .filter(model.Package.type == DATASET_TYPE_NAME)
 
     if package_id:
@@ -78,11 +88,18 @@ def harvest_source_url_validator(key, data, errors, context):
 
     existing_sources = q.all()
 
-    for url, state in existing_sources:
+    for id_, url in existing_sources:
         url = _normalize_url(url)
-        if url == new_url:
+        conf = model.Session.query(HarvestSource.config).filter(
-            raise Invalid('There already is a Harvest Source for this URL: %s'
+            HarvestSource.id == id_).first()
-                          % data[key])
+        if conf:
+            conf = conf[0]
+        else:
+            conf = None
+
+        if url == new_url and conf == new_config:
+            raise Invalid('There already is a Harvest Source for this URL (& '
+                          'config): url=%s config=%s' % (new_url, new_config))
 
     return data[key]
 

ckanext/harvest/tests/test_action.py

@@ -2,7 +2,7 @@ import json
 import copy
 import factories
 import unittest
-from nose.tools import assert_equal
+from nose.tools import assert_equal, assert_raises
 
 try:
     from ckan.tests import factories as ckan_factories
@@ -13,6 +13,7 @@ except ImportError:
 from ckan import plugins as p
 from ckan.plugins import toolkit
 from ckan import model
+import ckan.lib.search as search
 
 from ckanext.harvest.interfaces import IHarvester
 import ckanext.harvest.model as harvest_model
@@ -142,11 +143,13 @@ SOURCE_DICT = {
 class ActionBase(object):
     @classmethod
     def setup_class(cls):
-        reset_db()
-        harvest_model.setup()
         if not p.plugin_loaded('test_action_harvester'):
             p.load('test_action_harvester')
 
+    def setup(self):
+        reset_db()
+        harvest_model.setup()
+
     @classmethod
     def teardown_class(cls):
         p.unload('test_action_harvester')
@@ -324,6 +327,48 @@ class TestActions(ActionBase):
         assert_equal(harvest_model.HarvestObject.get(object_.id), None)
         assert_equal(model.Package.get(dataset['id']), None)
 
+    def test_harvest_source_create_twice_with_unique_url(self):
+        # don't use factory because it looks for the existing source
+        data_dict = SOURCE_DICT
+        site_user = toolkit.get_action('get_site_user')(
+            {'model': model, 'ignore_auth': True}, {})['name']
+
+        toolkit.get_action('harvest_source_create')(
+            {'user': site_user}, data_dict)
+
+        data_dict['name'] = 'another-source1'
+        data_dict['url'] = 'http://another-url'
+        toolkit.get_action('harvest_source_create')(
+            {'user': site_user}, data_dict)
+
+    def test_harvest_source_create_twice_with_same_url(self):
+        # don't use factory because it looks for the existing source
+        data_dict = SOURCE_DICT
+        site_user = toolkit.get_action('get_site_user')(
+            {'model': model, 'ignore_auth': True}, {})['name']
+
+        toolkit.get_action('harvest_source_create')(
+            {'user': site_user}, data_dict)
+
+        data_dict['name'] = 'another-source2'
+        assert_raises(toolkit.ValidationError,
+                      toolkit.get_action('harvest_source_create'),
+                      {'user': site_user}, data_dict)
+
+    def test_harvest_source_create_twice_with_unique_url_and_config(self):
+        # don't use factory because it looks for the existing source
+        data_dict = SOURCE_DICT
+        site_user = toolkit.get_action('get_site_user')(
+            {'model': model, 'ignore_auth': True}, {})['name']
+
+        toolkit.get_action('harvest_source_create')(
+            {'user': site_user}, data_dict)
+
+        data_dict['name'] = 'another-source3'
+        data_dict['config'] = '{"something": "new"}'
+        toolkit.get_action('harvest_source_create')(
+            {'user': site_user}, data_dict)
+
 
 class TestHarvestObject(unittest.TestCase):
     @classmethod