[sources] Check source's url attribute value on creation/modification

Similarly as for config. Now, ldap source validation is properly done there
instead of at initialization time.

--HG--
branch : 3.25

cubicweb/hooks/syncsources.py

@@ -51,6 +51,7 @@ class SourceAddedHook(SourceHook):
             raise validation_error(self.entity, {('type', 'subject'): msg})
 
         source = self.get_source(self.entity)
+        source.check_urls(self.entity)
         source.check_config(self.entity)
 
 
@@ -76,5 +77,7 @@ class SourceUpdatedHook(SourceHook):
                 raise validation_error(self.entity, {('name', 'subject'): msg})
 
         source = self.get_source(self.entity)
+        if 'url' in self.entity.cw_edited:
+            source.check_urls(self.entity)
         if 'config' in self.entity.cw_edited:
             source.check_config(self.entity)

cubicweb/hooks/test/unittest_syncsources.py

@@ -26,8 +26,15 @@ class SyncSourcesTC(CubicWebTC):
         with self.admin_access.cnx() as cnx:
             source = cnx.find('CWSource').one()
 
-            with self.assertRaises(ValidationError):
+            with self.assertRaises(ValidationError) as cm:
+                source.cw_set(url=u'whatever')
+            self.assertIn("Configuration of the system source goes to the 'sources' file",
+                          str(cm.exception))
+
+            with self.assertRaises(ValidationError) as cm:
                 source.cw_set(config=u'whatever')
+            self.assertIn("Configuration of the system source goes to the 'sources' file",
+                          str(cm.exception))
 
 
 if __name__ == '__main__':

cubicweb/server/sources/__init__.py

@@ -184,6 +184,17 @@ class AbstractSource(object):
         """
         return cls._check_config_dict(source_entity.eid, source_entity.dictconfig)
 
+    def check_urls(self, source_entity):
+        """Check URL of source entity: `urls` is a string that may contain one
+        URL per line), and return a list of at least one validated URL.
+        """
+        urls = source_entity.url if source_entity.url else ''
+        urls = [url.strip() for url in urls.splitlines() if url.strip()]
+        if not urls:
+            msg = _('specifying an URL is mandatory')
+            raise ValidationError(source_entity.eid, {role_name('url', 'subject'): msg})
+        return urls
+
     # source initialization / finalization #####################################
 
     def set_schema(self, schema):
@@ -200,8 +211,7 @@ class AbstractSource(object):
         """
         source_entity.complete()
         if source_entity.url:
-            self.urls = [url.strip() for url in source_entity.url.splitlines()
-                         if url.strip()]
+            self.urls = self.check_urls(source_entity)
         else:
             self.urls = []
 

cubicweb/server/sources/ldapfeed.py

@@ -176,18 +176,23 @@ You can set multiple groups by separating them by a comma.',
 
     _conn = None
 
+    def check_urls(self, source_entity):
+        urls = super(LDAPFeedSource, self).check_urls(source_entity)
+
+        if len(urls) > 1:
+            raise ValidationError(source_entity.eid, {'url': _('can only have one url')})
+
+        try:
+            protocol, hostport = urls[0].split('://')
+        except ValueError:
+            raise ValidationError(source_entity.eid, {'url': _('badly formatted url')})
+        if protocol not in PROTO_PORT:
+            raise ValidationError(source_entity.eid, {'url': _('unsupported protocol')})
+
+        return urls
+
     def init(self, source_entity):
         super(LDAPFeedSource, self).init(source_entity)
-        if self.urls:
-            if len(self.urls) > 1:
-                raise ValidationError(source_entity.eid, {'url': _('can only have one url')})
-            try:
-                protocol, hostport = self.urls[0].split('://')
-            except ValueError:
-                raise ValidationError(source_entity.eid, {'url': _('badly formatted url')})
-            if protocol not in PROTO_PORT:
-                raise ValidationError(source_entity.eid, {'url': _('unsupported protocol')})
-
         typedconfig = self.config
         self.authmode = typedconfig['auth-mode']
         self._authenticate = getattr(self, '_auth_%s' % self.authmode)

cubicweb/server/sources/native.py

@@ -355,6 +355,12 @@ class NativeSQLSource(SQLAdapterMixIn, AbstractSource):
                     "the 'sources' file, not in the database")
             raise ValidationError(source_entity.eid, {role_name('config', 'subject'): msg})
 
+    def check_urls(self, source_entity):
+        if source_entity.url:
+            msg = _("Configuration of the system source goes to "
+                    "the 'sources' file, not in the database")
+            raise ValidationError(source_entity.eid, {role_name('config', 'subject'): msg})
+
     def add_authentifier(self, authentifier):
         self.authentifiers.append(authentifier)
         authentifier.source = self

cubicweb/server/test/unittest_datafeed.py

@@ -146,6 +146,15 @@ class DataFeedTC(CubicWebTC):
                               str(cm.exception))
                 cnx.rollback()
 
+                with self.assertRaises(ValidationError) as cm:
+                    cnx.create_entity(
+                        'CWSource', name=u'error', type=u'datafeed', parser=u'testparser',
+                        url=None,
+                        config=u'synchronization-interval=1min')
+                self.assertIn('specifying an URL is mandatory',
+                              str(cm.exception))
+                cnx.rollback()
+
                 with self.assertRaises(ValidationError) as cm:
                     cnx.create_entity(
                         'CWSource', name=u'error', type=u'datafeed', parser=u'testparser',

cubicweb/server/test/unittest_ldapsource.py

@@ -34,7 +34,7 @@ from os.path import join
 from six import string_types
 from six.moves import range
 
-from cubicweb import AuthenticationError
+from cubicweb import AuthenticationError, ValidationError
 from cubicweb.devtools.testlib import CubicWebTC
 from cubicweb.devtools.httptest import get_available_port
 
@@ -307,6 +307,33 @@ class LDAPFeedUserTC(LDAPFeedTestBase):
             self.assertIsNotNone(pwd)
             self.assertTrue(str(pwd))
 
+    def test_bad_config(self):
+        with self.admin_access.cnx() as cnx:
+
+            with self.assertRaises(ValidationError) as cm:
+                cnx.create_entity(
+                    'CWSource', name=u'erroneous', type=u'ldapfeed', parser=u'ldapfeed',
+                    url=u'ldap.com', config=CONFIG_LDAPFEED)
+            self.assertIn('badly formatted url',
+                          str(cm.exception))
+            cnx.rollback()
+
+            with self.assertRaises(ValidationError) as cm:
+                cnx.create_entity(
+                    'CWSource', name=u'erroneous', type=u'ldapfeed', parser=u'ldapfeed',
+                    url=u'http://ldap.com', config=CONFIG_LDAPFEED)
+            self.assertIn('unsupported protocol',
+                          str(cm.exception))
+            cnx.rollback()
+
+            with self.assertRaises(ValidationError) as cm:
+                cnx.create_entity(
+                    'CWSource', name=u'erroneous', type=u'ldapfeed', parser=u'ldapfeed',
+                    url=u'ldap://host1\nldap://host2', config=CONFIG_LDAPFEED)
+            self.assertIn('can only have one url',
+                          str(cm.exception))
+            cnx.rollback()
+
 
 class LDAPGeneratePwdTC(LDAPFeedTestBase):
     """

cubicweb/web/test/unittest_views_cwsources.py

@@ -35,7 +35,7 @@ class SynchronizeSourceTC(CubicWebTC):
 
             with self.temporary_appobjects(AParser):
                 source = req.create_entity('CWSource', name=u'ext', type=u'datafeed',
-                                           parser=u'cw.entityxml')
+                                           parser=u'cw.entityxml', url=u'whatever')
                 req.cnx.commit()
 
             self.threads = 0