[sources] Check sources configuration is fine on creation/modification

Reintroduce usage of 'source.check_config' which had almost disappeared, as
well as tests for the currently detected errors.

Part of the system source specific checking done in syncsources reimplemented in
a specific check_config implementation.

Tests are dispatched among ldap / datafeed and syncsources tests but are not
strictly correctly located (notably syncsources tests behaviour of the native
source's check_config).

The system source url checking part which disappears from syncsources will be
reintroduced in a follow-up.

--HG--
branch : 3.25

cubicweb/hooks/syncsources.py

@@ -32,6 +32,12 @@ class SourceHook(hook.Hook):
     __abstract__ = True
     category = 'cw.sources'
 
+    def get_source(self, source_entity):
+        if source_entity.name == 'system':
+            return self._cw.repo.system_source
+        return self._cw.repo.get_source(source_entity.type, source_entity.name,
+                                        {}, source_entity.eid)
+
 
 # repo sources synchronization #################################################
 
@@ -40,17 +46,12 @@ class SourceAddedHook(SourceHook):
     __select__ = SourceHook.__select__ & is_instance('CWSource')
     events = ('after_add_entity',)
     def __call__(self):
-        try:
-            sourcecls = SOURCE_TYPES[self.entity.type]
-        except KeyError:
+        if self.entity.type not in SOURCE_TYPES:
             msg = _('Unknown source type')
             raise validation_error(self.entity, {('type', 'subject'): msg})
-        # ignore creation of the system source done during database
-        # initialisation, as config for this source is in a file and handling
-        # is done separatly (no need for the operation either)
-        if self.entity.name != 'system':
-            sourcecls._check_config_dict(self.entity.eid, self.entity.host_config,
-                                         raise_on_error=not self._cw.vreg.config.repairing)
+
+        source = self.get_source(self.entity)
+        source.check_config(self.entity)
 
 
 class SourceRemovedHook(SourceHook):
@@ -73,8 +74,7 @@ class SourceUpdatedHook(SourceHook):
             if oldname == 'system':
                 msg = _("You cannot rename the system source")
                 raise validation_error(self.entity, {('name', 'subject'): msg})
-        if 'config' in self.entity.cw_edited or 'url' in self.entity.cw_edited:
-            if self.entity.name == 'system' and self.entity.config:
-                msg = _("Configuration of the system source goes to "
-                        "the 'sources' file, not in the database")
-                raise validation_error(self.entity, {('config', 'subject'): msg})
+
+        source = self.get_source(self.entity)
+        if 'config' in self.entity.cw_edited:
+            source.check_config(self.entity)

cubicweb/hooks/test/unittest_syncsources.py

+# copyright 2017 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
+#
+# This file is part of CubicWeb.
+#
+# CubicWeb is free software: you can redistribute it and/or modify it under the
+# terms of the GNU Lesser General Public License as published by the Free
+# Software Foundation, either version 2.1 of the License, or (at your option)
+# any later version.
+#
+# CubicWeb is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Lesser General Public License along
+# with CubicWeb.  If not, see <http://www.gnu.org/licenses/>.
+
+from cubicweb import ValidationError
+from cubicweb.devtools.testlib import CubicWebTC
+
+
+class SyncSourcesTC(CubicWebTC):
+
+    def test_cant_add_config_system_source(self):
+        with self.admin_access.cnx() as cnx:
+            source = cnx.find('CWSource').one()
+
+            with self.assertRaises(ValidationError):
+                source.cw_set(config=u'whatever')
+
+
+if __name__ == '__main__':
+    import unittest
+    unittest.main()

cubicweb/server/sources/native.py

@@ -350,10 +350,9 @@ class NativeSQLSource(SQLAdapterMixIn, AbstractSource):
         self.create_eid = self.eid_generator.create_eid
 
     def check_config(self, source_entity):
-        """check configuration of source entity"""
-        if source_entity.host_config:
-            msg = _('the system source has its configuration '
-                    'stored on the file-system')
+        if source_entity.config:
+            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):

cubicweb/server/test/unittest_datafeed.py

@@ -20,6 +20,7 @@
 from datetime import timedelta
 from contextlib import contextmanager
 
+from cubicweb import ValidationError
 from cubicweb.devtools.testlib import CubicWebTC
 from cubicweb.server.sources import datafeed
 from cubicweb.dataimport.stores import NoHookRQLObjectStore, MetaGenerator
@@ -133,6 +134,27 @@ class DataFeedTC(CubicWebTC):
         self.assertIn(u'failed to load parser for source "ô myfeed"',
                       importlog)
 
+    def test_bad_config(self):
+        with self.admin_access.repo_cnx() as cnx:
+            with self.base_parser(cnx):
+                with self.assertRaises(ValidationError) as cm:
+                    cnx.create_entity(
+                        'CWSource', name=u'error', type=u'datafeed', parser=u'testparser',
+                        url=u'ignored',
+                        config=u'synchronization-interval=1s')
+                self.assertIn('synchronization-interval must be greater than 1 minute',
+                              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=u'ignored',
+                        config=u'synch-interval=1min')
+                self.assertIn('unknown options synch-interval',
+                              str(cm.exception))
+                cnx.rollback()
+
 
 class DataFeedConfigTC(CubicWebTC):
 

flake8-ok-files.txt

@@ -30,6 +30,7 @@ cubicweb/hooks/test/data/hooks.py
 cubicweb/hooks/test/unittest_notificationhooks.py
 cubicweb/hooks/test/unittest_security.py
 cubicweb/hooks/test/unittest_syncsession.py
+cubicweb/hooks/test/unittest_syncsources.py
 cubicweb/pylintext.py
 cubicweb/repoapi.py
 cubicweb/rqlrewrite.py