Merge pull request #862 from ZeitOnline/WCM-3

WCM-3: add include logic to webhooks

core/docs/changelog/WCM-3.change

@@ -0,0 +1 @@
+WCM-3: add e-paper publish webhook

core/src/zeit/cms/checkout/tests/test_webhook.py

@@ -143,6 +143,36 @@ def test_match_path_prefix(self):
         self.assertTrue(hook.should_exclude(self.repository['online']['2007']['01']['Somalia']))
 
 
+class WebhookIncludeTest(zeit.cms.testing.ZeitCmsTestCase):
+    def test_matches_criteria_is_false_when_include_does_not_match_contenttype(self):
+        hook = zeit.cms.checkout.webhook.Hook(None, None)
+        hook.add_include('type', 'testcontenttype')
+        self.assertTrue(hook.should_include(self.repository['testcontent']))
+        self.assertFalse(hook.should_include(self.repository['online']['2007']['01']['Somalia']))
+
+    def test_matches_criteria_is_false_when_include_does_not_match(self):
+        hook = zeit.cms.checkout.webhook.Hook(None, None)
+        hook.add_include('type', 'wrong-content-type')
+        self.assertFalse(hook.matches_criteria(self.repository['testcontent']))
+
+
+class WebhookExcludeStrongerThanIncludeTest(zeit.cms.testing.ZeitCmsTestCase):
+    def test_exclude_weighs_more_than_include_on_same_attribute(self):
+        hook = zeit.cms.checkout.webhook.Hook(None, None)
+        hook.add_include('type', 'testcontenttype')
+        hook.add_exclude('type', 'testcontenttype')
+        self.assertFalse(hook.matches_criteria(self.repository['testcontent']))
+
+    def test_exclude_weighs_more_than_include_on_different_attribute(self):
+        hook = zeit.cms.checkout.webhook.Hook('checkin', None)
+        hook.add_include('type', 'testcontenttype')
+        hook.add_exclude('product_counter', 'online')
+        self.assertTrue(hook.matches_criteria(self.repository['testcontent']))
+        with checked_out(self.repository['testcontent']) as co:
+            co.product = Product('ZEDE')
+        self.assertFalse(hook.matches_criteria(self.repository['testcontent']))
+
+
 class WebhookEventTest(FunctionalTestCase):
     @property
     def config(self):
@@ -159,6 +189,15 @@ def config(self):
               <product_counter>print</product_counter>
             </exclude>
           </webhook>
+          <!-- this webhook will be excluded, see exclude type = testconttype -->
+          <webhook id="publish" url="http://localhost/two:{port}">
+            <include>
+              <product_counter>print</product_counter>
+            </include>
+            <exclude>
+              <type>testcontenttype</type>
+            </exclude>
+          </webhook>
         </webhooks>
         """
 
@@ -210,3 +249,52 @@ def test_webhook_is_notified_on_add_and_publish_and_not_on_checkin(self):
         workflow.publish()
         requests = self.layer['request_handler'].requests
         self.assertEqual(2, len(requests))
+
+
+class TestMultipleWebhooksWithSameId(FunctionalTestCase):
+    @property
+    def config(self):
+        port = self.layer['http_port']
+        return f"""<webhooks>
+              <webhook id="publish" url="http://localhost/one:{port}">
+                <include>
+                  <product_counter>print</product_counter>
+                </include>
+              </webhook>
+              <webhook id="publish" url="http://localhost/two:{port}">
+                <include>
+                  <product_counter>print</product_counter>
+                </include>
+              </webhook>
+              <webhook id="publish" url="http://localhost/three:{port}">
+                <include>
+                  <product_counter>print</product_counter>
+                </include>
+              </webhook>
+            </webhooks>
+            """
+
+    def test_multiple_webhooks_with_same_id(self):
+        requests_post_mock = mock.patch('requests.post').start()
+
+        with checked_out(self.repository['testcontent']) as co:
+            co.product = Product('ZEI')
+            info = zeit.cms.workflow.interfaces.IPublishInfo(co)
+            info.urgent = True
+        workflow = zeit.cms.workflow.interfaces.IPublish(self.repository['testcontent'])
+        workflow.publish()
+
+        # Only 3 requests should match, the last one is excluded.
+        self.assertEqual(3, requests_post_mock.call_count)
+
+        # check if all url's match
+        expected_urls = [
+            f'http://localhost/one:{self.layer["http_port"]}',
+            f'http://localhost/two:{self.layer["http_port"]}',
+            f'http://localhost/three:{self.layer["http_port"]}',
+        ]
+        actual_urls = [call.args[0] for call in requests_post_mock.call_args_list]
+        for url in expected_urls:
+            self.assertIn(url, actual_urls)
+
+        mock.patch.stopall()

core/src/zeit/cms/checkout/webhook.py

@@ -1,4 +1,3 @@
-import collections
 import logging
 
 import grokcore.component as grok
@@ -18,31 +17,30 @@
 log = logging.getLogger(__name__)
 
 
-def create_webhook_job(id, context, **kwargs):
-    hook = HOOKS.factory.find(id)
-    if not hook:
-        log.warning('No %s webhook found for %s', id, context.uniqueId)
+def create_webhook_jobs(id, context, **kwargs):
+    hooks = HOOKS.factory.find(id)
+    if not hooks:
+        log.warning('No %s webhooks found for %s', id, context.uniqueId)
         return
-
-    log.info(
-        'After%s: Creating async webhook job for %s',
-        id.capitalize(),
-        context.uniqueId,
-    )
-    notify_webhook.apply_async((context.uniqueId, id), **kwargs)
+    for url in hooks:
+        log.info(
+            f'After{id.capitalize()}: Creating async webhook jobs with'
+            f' url {url} for {context.uniqueId}',
+        )
+        notify_webhook.apply_async((context.uniqueId, id, url), **kwargs)
 
 
 @grok.subscribe(zeit.cms.interfaces.ICMSContent, zeit.cms.checkout.interfaces.IAfterCheckinEvent)
 def notify_after_checkin(context, event):
     # XXX Work around redis/ZODB race condition, see BUG-796.
     if event.publishing:
         return
-    create_webhook_job('checkin', context, countdown=5)
+    create_webhook_jobs('checkin', context, countdown=5)
 
 
 @grok.subscribe(zeit.cms.interfaces.ICMSContent, zeit.cms.workflow.interfaces.IPublishedEvent)
 def notify_after_publish(context, event):
-    create_webhook_job('publish', context, countdown=5)
+    create_webhook_jobs('publish', context, countdown=5)
 
 
 @grok.subscribe(zope.lifecycleevent.IObjectAddedEvent)
@@ -54,21 +52,21 @@ def notify_after_add(event):
         return
     if zeit.cms.workingcopy.interfaces.IWorkingcopy.providedBy(event.newParent):
         return
-    create_webhook_job('add', context)
+    create_webhook_jobs('add', context)
 
 
 @zeit.cms.celery.task(bind=True, queue='webhook')
-def notify_webhook(self, uniqueId, id):
+def notify_webhook(self, uniqueId, id, url):
     content = zeit.cms.interfaces.ICMSContent(uniqueId, None)
     if content is None:
         log.warning('Could not resolve %s, ignoring.', uniqueId)
         return
-    hook = HOOKS.factory.find(id)
-    if hook is None:
+    hooks = HOOKS.factory.find(id)
+    if hooks is None:
         log.warning('Hook configuration for %s has vanished, ignoring.', id)
         return
     try:
-        hook(content)
+        hooks[url](content)
     except TechnicalError as e:
         raise self.retry(countdown=e.countdown)
     # Don't even think about trying to write to DAV cache, to avoid conflicts.
@@ -80,9 +78,10 @@ def __init__(self, id, url):
         self.id = id
         self.url = url
         self.excludes = []
+        self.includes = []
 
     def __call__(self, content):
-        if self.should_exclude(content):
+        if not self.matches_criteria(content):
             return
         log.info('Notifying %s about %s', self.url, content)
         try:
@@ -103,6 +102,20 @@ def deliver(self, content):
     def add_exclude(self, key, value):
         self.excludes.append((key, value))
 
+    def add_include(self, key, value):
+        self.includes.append((key, value))
+
+    def matches_criteria(self, content):
+        return self.should_include(content) and not self.should_exclude(content)
+
+    def should_include(self, content):
+        if not self.includes:
+            return True
+        for include in self.includes:
+            if self._matches(include, content):
+                return True
+        return False
+
     def should_exclude(self, content):
         renameable = getattr(IAutomaticallyRenameable(content, None), 'renameable', False)
         if renameable:
@@ -145,13 +158,17 @@ class HookSource(zeit.cms.content.sources.SimpleXMLSource):
 
     @CONFIG_CACHE.cache_on_arguments()
     def _values(self):
-        result = collections.OrderedDict()
+        result = {}
         tree = self._get_tree()
         for node in tree.iterchildren('webhook'):
             hook = Hook(node.get('id'), node.get('url'))
+            for include in node.xpath('include/*'):
+                hook.add_include(include.tag, include.text)
             for exclude in node.xpath('exclude/*'):
                 hook.add_exclude(exclude.tag, exclude.text)
-            result[hook.id] = hook
+            if not result.get(hook.id):
+                result[hook.id] = {}
+            result[hook.id][hook.url] = hook
         return result
 
     def getValues(self):