From c277d4fa51cd61bf4d780b3db98d8a839f0c2aa8 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 6 Nov 2018 19:50:35 -0500 Subject: [PATCH 01/16] Sync versions when creating/deleting versions on GitHub --- readthedocs/core/views/hooks.py | 22 ++++++++++++ readthedocs/restapi/views/integrations.py | 42 ++++++++++++++++------- 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index 1110853d0a6..fff48819378 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -75,6 +75,28 @@ def build_branches(project, branch_list): return (to_build, not_building) +def sync_versions(project): + """ + Sync the versions of a repo using its latest version. + + This doesn't register a new build, + but clones the repo and syncs the versions. + Due that `sync_repository_task` is bound to a version, + we always pass the latest version. + + :returns: The version that was used to trigger the clone (usually latest). + """ + try: + version = project.versions.get(slug=LATEST) + sync_repository_task.delay(version.pk) + return version.slug + except Project.DoesNotExist: + log.info('Unable to sync from %s version', LATEST) + except Exception as e: + log.exception('Unknown sync versions exception') + return None + + def get_project_from_url(url): if not url: return Project.objects.none() diff --git a/readthedocs/restapi/views/integrations.py b/readthedocs/restapi/views/integrations.py index f2ec92dd030..3967566fc80 100644 --- a/readthedocs/restapi/views/integrations.py +++ b/readthedocs/restapi/views/integrations.py @@ -1,31 +1,39 @@ """Endpoints integrating with Github, Bitbucket, and other webhooks.""" -from __future__ import absolute_import +from __future__ import ( + absolute_import, + division, + print_function, + unicode_literals, +) + import json import logging import re -from builtins import object +import six +from django.shortcuts import get_object_or_404 from rest_framework import permissions -from rest_framework.views import APIView +from rest_framework.exceptions import NotFound, ParseError from rest_framework.renderers import JSONRenderer from rest_framework.response import Response -from rest_framework.exceptions import ParseError, NotFound - -from django.shortcuts import get_object_or_404 +from rest_framework.views import APIView -from readthedocs.core.views.hooks import build_branches -from readthedocs.core.signals import (webhook_github, webhook_bitbucket, - webhook_gitlab) +from readthedocs.core.signals import ( + webhook_bitbucket, + webhook_github, + webhook_gitlab, +) +from readthedocs.core.views.hooks import build_branches, sync_versions from readthedocs.integrations.models import HttpExchange, Integration from readthedocs.integrations.utils import normalize_request_payload from readthedocs.projects.models import Project -import six - log = logging.getLogger(__name__) GITHUB_PUSH = 'push' +GITHUB_CREATE = 'create' +GITHUB_DELETE = 'delete' GITLAB_PUSH = 'push' BITBUCKET_PUSH = 'repo:push' @@ -124,6 +132,14 @@ def get_response_push(self, project, branches): 'project': project.slug, 'versions': list(to_build)} + def sync_versions(self, project): + version = sync_versions(project) + return { + 'build_triggered': False, + 'project': project.slug, + 'versions': [version], + } + class GitHubWebhookView(WebhookMixin, APIView): @@ -154,7 +170,7 @@ def get_data(self): def handle_webhook(self): # Get event and trigger other webhook events - event = self.request.META.get('HTTP_X_GITHUB_EVENT', 'push') + event = self.request.META.get('HTTP_X_GITHUB_EVENT', GITHUB_PUSH) webhook_github.send(Project, project=self.project, data=self.data, event=event) # Handle push events and trigger builds @@ -164,6 +180,8 @@ def handle_webhook(self): return self.get_response_push(self.project, branches) except KeyError: raise ParseError('Parameter "ref" is required') + elif event in (GITHUB_CREATE, GITHUB_DELETE): + return self.sync_versions(self.project) def _normalize_ref(self, ref): pattern = re.compile(r'^refs/(heads|tags)/') From 05baf4fe391d2e0d9a16e6b80d2470efd3cb1d50 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 6 Nov 2018 20:07:33 -0500 Subject: [PATCH 02/16] Listen to create/delete events on GitHub --- readthedocs/oauth/services/github.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index 7f4c9c75786..6106d5cab7b 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -172,7 +172,7 @@ def get_webhook_data(self, project, integration): ), 'content_type': 'json', }, - 'events': ['push', 'pull_request'], + 'events': ['push', 'pull_request', 'create', 'delete'], }) def setup_webhook(self, project): From 50731cbcdeb873ecda11b691936bf426eefa86a6 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 6 Nov 2018 23:46:18 -0500 Subject: [PATCH 03/16] Test --- readthedocs/restapi/views/integrations.py | 3 ++- readthedocs/rtd_tests/tests/test_api.py | 28 ++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/readthedocs/restapi/views/integrations.py b/readthedocs/restapi/views/integrations.py index 3967566fc80..0b61518ed4b 100644 --- a/readthedocs/restapi/views/integrations.py +++ b/readthedocs/restapi/views/integrations.py @@ -31,6 +31,7 @@ log = logging.getLogger(__name__) +GITHUB_EVENT_HEADER = 'HTTP_X_GITHUB_EVENT' GITHUB_PUSH = 'push' GITHUB_CREATE = 'create' GITHUB_DELETE = 'delete' @@ -170,7 +171,7 @@ def get_data(self): def handle_webhook(self): # Get event and trigger other webhook events - event = self.request.META.get('HTTP_X_GITHUB_EVENT', GITHUB_PUSH) + event = self.request.META.get(GITHUB_EVENT_HEADER, GITHUB_PUSH) webhook_github.send(Project, project=self.project, data=self.data, event=event) # Handle push events and trigger builds diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 0f277d841fd..2e0996f9d99 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -27,7 +27,11 @@ from readthedocs.integrations.models import Integration from readthedocs.oauth.models import RemoteOrganization, RemoteRepository from readthedocs.projects.models import APIProject, Feature, Project -from readthedocs.restapi.views.integrations import GitHubWebhookView +from readthedocs.restapi.views.integrations import ( + GITHUB_CREATE, + GITHUB_EVENT_HEADER, + GitHubWebhookView, +) from readthedocs.restapi.views.task_views import get_status_data super_auth = base64.b64encode(b'super:test').decode('utf-8') @@ -674,6 +678,9 @@ def setUp(self): Version, slug='v1.0', verbose_name='v1.0', active=True, project=self.project ) + self.github_payload = { + 'ref': 'master', + } def test_github_webhook_for_branches(self, trigger_build): """GitHub webhook API.""" @@ -731,6 +738,25 @@ def test_github_webhook_for_tags(self, trigger_build): trigger_build.assert_has_calls( [mock.call(force=True, version=self.version_tag, project=self.project)]) + @mock.patch('readthedocs.restapi.views.integrations.sync_versions') + def test_github_create_event(self, sync_versions, trigger_build): + sync_versions.return_value = LATEST + client = APIClient() + + headers = {GITHUB_EVENT_HEADER: GITHUB_CREATE} + resp = client.post( + '/api/v2/webhook/github/{}/'.format(self.project.slug), + self.github_payload, + format='json', + **headers, + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertFalse(resp.data['build_triggered']) + self.assertEqual(resp.data['project'], self.project.slug) + self.assertEqual(resp.data['versions'], [LATEST]) + trigger_build.assert_not_called() + sync_versions.assert_called_with(self.project) + def test_github_parse_ref(self, trigger_build): wh = GitHubWebhookView() From ab879560bd54f6bccce302a048873ee788941568 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 6 Nov 2018 23:52:23 -0500 Subject: [PATCH 04/16] fix python2 syntax --- readthedocs/rtd_tests/tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 2e0996f9d99..278c5b7c6c7 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -748,7 +748,7 @@ def test_github_create_event(self, sync_versions, trigger_build): '/api/v2/webhook/github/{}/'.format(self.project.slug), self.github_payload, format='json', - **headers, + **headers ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertFalse(resp.data['build_triggered']) From 2caab91b0b7f1e334ce82f2c66a20ec61c16d80c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 7 Nov 2018 00:24:40 -0500 Subject: [PATCH 05/16] More coverage --- readthedocs/rtd_tests/tests/test_api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 278c5b7c6c7..281ecd9e5c9 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -738,9 +738,8 @@ def test_github_webhook_for_tags(self, trigger_build): trigger_build.assert_has_calls( [mock.call(force=True, version=self.version_tag, project=self.project)]) - @mock.patch('readthedocs.restapi.views.integrations.sync_versions') - def test_github_create_event(self, sync_versions, trigger_build): - sync_versions.return_value = LATEST + @mock.patch('readthedocs.core.views.hooks.sync_repository_task') + def test_github_create_event(self, sync_repository_task, trigger_build): client = APIClient() headers = {GITHUB_EVENT_HEADER: GITHUB_CREATE} @@ -755,7 +754,8 @@ def test_github_create_event(self, sync_versions, trigger_build): self.assertEqual(resp.data['project'], self.project.slug) self.assertEqual(resp.data['versions'], [LATEST]) trigger_build.assert_not_called() - sync_versions.assert_called_with(self.project) + latest_version = self.project.versions.get(slug=LATEST) + sync_repository_task.delay.assert_called_with(latest_version.pk) def test_github_parse_ref(self, trigger_build): wh = GitHubWebhookView() From d96a412c6230f3c6be1d20f47872a965ace532bf Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 7 Nov 2018 12:02:39 -0500 Subject: [PATCH 06/16] Refactor --- readthedocs/core/views/hooks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index fff48819378..017f1f18368 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -87,11 +87,12 @@ def sync_versions(project): :returns: The version that was used to trigger the clone (usually latest). """ try: - version = project.versions.get(slug=LATEST) + version_slug = LATEST + version = project.versions.get(slug=version_slug) sync_repository_task.delay(version.pk) return version.slug except Project.DoesNotExist: - log.info('Unable to sync from %s version', LATEST) + log.info('Unable to sync from %s version', version_slug) except Exception as e: log.exception('Unknown sync versions exception') return None From e5fe6ea84743531db3c4fbc3ba7ea6fc0368e88a Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 7 Nov 2018 13:32:45 -0500 Subject: [PATCH 07/16] More tests --- readthedocs/restapi/views/integrations.py | 3 +- readthedocs/rtd_tests/tests/test_api.py | 36 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/readthedocs/restapi/views/integrations.py b/readthedocs/restapi/views/integrations.py index 0b61518ed4b..aa1bed8c6b2 100644 --- a/readthedocs/restapi/views/integrations.py +++ b/readthedocs/restapi/views/integrations.py @@ -181,8 +181,9 @@ def handle_webhook(self): return self.get_response_push(self.project, branches) except KeyError: raise ParseError('Parameter "ref" is required') - elif event in (GITHUB_CREATE, GITHUB_DELETE): + if event in (GITHUB_CREATE, GITHUB_DELETE): return self.sync_versions(self.project) + return None def _normalize_ref(self, ref): pattern = re.compile(r'^refs/(heads|tags)/') diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 281ecd9e5c9..0f409543945 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -29,6 +29,7 @@ from readthedocs.projects.models import APIProject, Feature, Project from readthedocs.restapi.views.integrations import ( GITHUB_CREATE, + GITHUB_DELETE, GITHUB_EVENT_HEADER, GitHubWebhookView, ) @@ -757,6 +758,41 @@ def test_github_create_event(self, sync_repository_task, trigger_build): latest_version = self.project.versions.get(slug=LATEST) sync_repository_task.delay.assert_called_with(latest_version.pk) + @mock.patch('readthedocs.core.views.hooks.sync_repository_task') + def test_github_delete_event(self, sync_repository_task, trigger_build): + client = APIClient() + + headers = {GITHUB_EVENT_HEADER: GITHUB_DELETE} + resp = client.post( + '/api/v2/webhook/github/{}/'.format(self.project.slug), + self.github_payload, + format='json', + **headers + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertFalse(resp.data['build_triggered']) + self.assertEqual(resp.data['project'], self.project.slug) + self.assertEqual(resp.data['versions'], [LATEST]) + trigger_build.assert_not_called() + latest_version = self.project.versions.get(slug=LATEST) + sync_repository_task.delay.assert_called_with(latest_version.pk) + + @mock.patch('readthedocs.core.views.hooks.sync_repository_task') + def test_github_unknow_event(self, sync_repository_task, trigger_build): + client = APIClient() + + headers = {GITHUB_EVENT_HEADER: 'foobar'} + resp = client.post( + '/api/v2/webhook/github/{}/'.format(self.project.slug), + self.github_payload, + format='json', + **headers + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual(resp.data, {'detail': 'Unhandled webhook event'}) + trigger_build.assert_not_called() + sync_repository_task.delay.assert_not_called() + def test_github_parse_ref(self, trigger_build): wh = GitHubWebhookView() From 004e9faf13f0f0566d67cf37cfef1ccfccb68cc0 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 7 Nov 2018 16:51:25 -0500 Subject: [PATCH 08/16] Tests for GitLab integration --- readthedocs/restapi/views/integrations.py | 2 + readthedocs/rtd_tests/tests/test_api.py | 152 +++++++++++++++++++--- 2 files changed, 137 insertions(+), 17 deletions(-) diff --git a/readthedocs/restapi/views/integrations.py b/readthedocs/restapi/views/integrations.py index aa1bed8c6b2..a8cf00c17e6 100644 --- a/readthedocs/restapi/views/integrations.py +++ b/readthedocs/restapi/views/integrations.py @@ -36,6 +36,8 @@ GITHUB_CREATE = 'create' GITHUB_DELETE = 'delete' GITLAB_PUSH = 'push' +GITLAB_NULL_HASH = '0' * 40 +GITLAB_TAG_PUSH = 'tag_push' BITBUCKET_PUSH = 'repo:push' diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 0f409543945..7960c8cd877 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -31,6 +31,9 @@ GITHUB_CREATE, GITHUB_DELETE, GITHUB_EVENT_HEADER, + GITLAB_NULL_HASH, + GITLAB_PUSH, + GITLAB_TAG_PUSH, GitHubWebhookView, ) from readthedocs.restapi.views.task_views import get_status_data @@ -682,6 +685,12 @@ def setUp(self): self.github_payload = { 'ref': 'master', } + self.gitlab_payload = { + 'object_kind': GITLAB_PUSH, + 'ref': 'master', + 'before': '95790bf891e76fee5e1747ab589903a6a1f80f22', + 'after': '95790bf891e76fee5e1747ab589903a6a1f80f23', + } def test_github_webhook_for_branches(self, trigger_build): """GitHub webhook API.""" @@ -777,22 +786,6 @@ def test_github_delete_event(self, sync_repository_task, trigger_build): latest_version = self.project.versions.get(slug=LATEST) sync_repository_task.delay.assert_called_with(latest_version.pk) - @mock.patch('readthedocs.core.views.hooks.sync_repository_task') - def test_github_unknow_event(self, sync_repository_task, trigger_build): - client = APIClient() - - headers = {GITHUB_EVENT_HEADER: 'foobar'} - resp = client.post( - '/api/v2/webhook/github/{}/'.format(self.project.slug), - self.github_payload, - format='json', - **headers - ) - self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertEqual(resp.data, {'detail': 'Unhandled webhook event'}) - trigger_build.assert_not_called() - sync_repository_task.delay.assert_not_called() - def test_github_parse_ref(self, trigger_build): wh = GitHubWebhookView() @@ -815,7 +808,7 @@ def test_github_invalid_webhook(self, trigger_build): self.assertEqual(resp.status_code, 200) self.assertEqual(resp.data['detail'], 'Unhandled webhook event') - def test_gitlab_webhook(self, trigger_build): + def test_gitlab_webhook_for_branches(self, trigger_build): """GitLab webhook API.""" client = APIClient() client.post( @@ -833,6 +826,131 @@ def test_gitlab_webhook(self, trigger_build): trigger_build.assert_has_calls( [mock.call(force=True, version=mock.ANY, project=self.project)]) + def test_gitlab_webhook_for_tags(self, trigger_build): + client = APIClient() + self.gitlab_payload.update( + object_kind=GITLAB_TAG_PUSH, + ref='v1.0', + ) + client.post( + '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), + self.gitlab_payload, + format='json', + ) + trigger_build.assert_called_with( + force=True, version=self.version_tag, project=self.project + ) + + trigger_build.reset_mock() + self.gitlab_payload.update( + ref='refs/tags/v1.0', + ) + client.post( + '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), + self.gitlab_payload, + format='json', + ) + trigger_build.assert_called_with( + force=True, version=self.version_tag, project=self.project + ) + + trigger_build.reset_mock() + self.gitlab_payload.update( + ref='refs/heads/non-existent', + ) + client.post( + '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), + self.gitlab_payload, + format='json', + ) + trigger_build.assert_not_called() + + @mock.patch('readthedocs.core.views.hooks.sync_repository_task') + def test_gitlab_push_hook_creation( + self, sync_repository_task, trigger_build): + client = APIClient() + self.gitlab_payload.update( + before=GITLAB_NULL_HASH, + after='95790bf891e76fee5e1747ab589903a6a1f80f22', + ) + resp = client.post( + '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), + self.gitlab_payload, + format='json', + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertFalse(resp.data['build_triggered']) + self.assertEqual(resp.data['project'], self.project.slug) + self.assertEqual(resp.data['versions'], [LATEST]) + trigger_build.assert_not_called() + latest_version = self.project.versions.get(slug=LATEST) + sync_repository_task.delay.assert_called_with(latest_version.pk) + + @mock.patch('readthedocs.core.views.hooks.sync_repository_task') + def test_gitlab_push_hook_deletion( + self, sync_repository_task, trigger_build): + client = APIClient() + self.gitlab_payload.update( + before='95790bf891e76fee5e1747ab589903a6a1f80f22', + after=GITLAB_NULL_HASH, + ) + resp = client.post( + '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), + self.gitlab_payload, + format='json', + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertFalse(resp.data['build_triggered']) + self.assertEqual(resp.data['project'], self.project.slug) + self.assertEqual(resp.data['versions'], [LATEST]) + trigger_build.assert_not_called() + latest_version = self.project.versions.get(slug=LATEST) + sync_repository_task.delay.assert_called_with(latest_version.pk) + + @mock.patch('readthedocs.core.views.hooks.sync_repository_task') + def test_gitlab_tag_push_hook_creation( + self, sync_repository_task, trigger_build): + client = APIClient() + self.gitlab_payload.update( + object_kind=GITLAB_TAG_PUSH, + before=GITLAB_NULL_HASH, + after='95790bf891e76fee5e1747ab589903a6a1f80f22', + ) + resp = client.post( + '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), + self.gitlab_payload, + format='json', + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertFalse(resp.data['build_triggered']) + self.assertEqual(resp.data['project'], self.project.slug) + self.assertEqual(resp.data['versions'], [LATEST]) + trigger_build.assert_not_called() + latest_version = self.project.versions.get(slug=LATEST) + sync_repository_task.delay.assert_called_with(latest_version.pk) + + @mock.patch('readthedocs.core.views.hooks.sync_repository_task') + def test_gitlab_tag_push_hook_deletion( + self, sync_repository_task, trigger_build): + client = APIClient() + self.gitlab_payload.update( + object_kind=GITLAB_TAG_PUSH, + before='95790bf891e76fee5e1747ab589903a6a1f80f22', + after=GITLAB_NULL_HASH, + ) + resp = client.post( + '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), + self.gitlab_payload, + format='json', + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertFalse(resp.data['build_triggered']) + self.assertEqual(resp.data['project'], self.project.slug) + self.assertEqual(resp.data['versions'], [LATEST]) + trigger_build.assert_not_called() + latest_version = self.project.versions.get(slug=LATEST) + sync_repository_task.delay.assert_called_with(latest_version.pk) + def test_gitlab_invalid_webhook(self, trigger_build): """GitLab webhook unhandled event.""" client = APIClient() From 9641ca63ab39aebe3bdc7e640220bed8f74278f3 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 7 Nov 2018 17:09:51 -0500 Subject: [PATCH 09/16] Fix tests --- readthedocs/rtd_tests/tests/test_api.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 7960c8cd877..4c367233e47 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -812,19 +812,24 @@ def test_gitlab_webhook_for_branches(self, trigger_build): """GitLab webhook API.""" client = APIClient() client.post( - '/api/v2/webhook/gitlab/{0}/'.format(self.project.slug), - {'object_kind': 'push', 'ref': 'master'}, + '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), + self.gitlab_payload, format='json', ) - trigger_build.assert_has_calls( - [mock.call(force=True, version=mock.ANY, project=self.project)]) + trigger_build.assert_called_with( + force=True, version=mock.ANY, project=self.project + ) + + trigger_build.reset_mock() + self.gitlab_payload.update( + ref='non-existent', + ) client.post( - '/api/v2/webhook/gitlab/{0}/'.format(self.project.slug), - {'object_kind': 'push', 'ref': 'non-existent'}, + '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), + self.gitlab_payload, format='json', ) - trigger_build.assert_has_calls( - [mock.call(force=True, version=mock.ANY, project=self.project)]) + trigger_build.assert_not_called() def test_gitlab_webhook_for_tags(self, trigger_build): client = APIClient() From e17a70343f9a9d1210eaa28aa11e1e77245ea61c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 7 Nov 2018 17:10:23 -0500 Subject: [PATCH 10/16] Handler GitLab integration --- readthedocs/restapi/views/integrations.py | 24 +++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/readthedocs/restapi/views/integrations.py b/readthedocs/restapi/views/integrations.py index a8cf00c17e6..17cffd0b84c 100644 --- a/readthedocs/restapi/views/integrations.py +++ b/readthedocs/restapi/views/integrations.py @@ -213,15 +213,31 @@ class GitLabWebhookView(WebhookMixin, APIView): def handle_webhook(self): # Get event and trigger other webhook events event = self.request.data.get('object_kind', GITLAB_PUSH) - webhook_gitlab.send(Project, project=self.project, - data=self.request.data, event=event) + webhook_gitlab.send( + Project, + project=self.project, + data=self.request.data, + event=event + ) # Handle push events and trigger builds - if event == GITLAB_PUSH: + if event in (GITLAB_PUSH, GITLAB_TAG_PUSH): + data = self.request.data + before = data['before'] + after = data['after'] + # Tag/branch created/deleted + if before == GITLAB_NULL_HASH or after == GITLAB_NULL_HASH: + return self.sync_versions(self.project) + # Normal push to master try: - branches = [self.request.data['ref'].replace('refs/heads/', '')] + branches = [self._normalize_ref(data['ref'])] return self.get_response_push(self.project, branches) except KeyError: raise ParseError('Parameter "ref" is required') + return None + + def _normalize_ref(self, ref): + pattern = re.compile(r'^refs/(heads|tags)/') + return pattern.sub('', ref) class BitbucketWebhookView(WebhookMixin, APIView): From 97cc096a58252b535c3fe467b940cd13e5889bb7 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 7 Nov 2018 17:20:08 -0500 Subject: [PATCH 11/16] Linter --- readthedocs/restapi/views/integrations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/restapi/views/integrations.py b/readthedocs/restapi/views/integrations.py index 17cffd0b84c..604600299a3 100644 --- a/readthedocs/restapi/views/integrations.py +++ b/readthedocs/restapi/views/integrations.py @@ -225,7 +225,7 @@ def handle_webhook(self): before = data['before'] after = data['after'] # Tag/branch created/deleted - if before == GITLAB_NULL_HASH or after == GITLAB_NULL_HASH: + if GITLAB_NULL_HASH in (before, after): return self.sync_versions(self.project) # Normal push to master try: From 1d3862b4318fb0a0c602a3e547622614a5ee94c9 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 8 Nov 2018 16:17:00 -0500 Subject: [PATCH 12/16] Tests for bitbucket integration --- readthedocs/rtd_tests/tests/test_api.py | 67 ++++++++++++++++++++----- 1 file changed, 55 insertions(+), 12 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 512113a1e74..20e590dc89d 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -818,6 +818,20 @@ def setUp(self): 'before': '95790bf891e76fee5e1747ab589903a6a1f80f22', 'after': '95790bf891e76fee5e1747ab589903a6a1f80f23', } + self.bitbucket_payload = { + 'push': { + 'changes': [{ + 'new': { + 'type': 'branch', + 'name': 'master', + }, + 'old': { + 'type': 'branch', + 'name': 'master', + }, + }], + }, + } def test_github_webhook_for_branches(self, trigger_build): """GitHub webhook API.""" @@ -1098,27 +1112,20 @@ def test_bitbucket_webhook(self, trigger_build): """Bitbucket webhook API.""" client = APIClient() client.post( - '/api/v2/webhook/bitbucket/{0}/'.format(self.project.slug), - { - 'push': { - 'changes': [{ - 'new': { - 'name': 'master', - }, - }], - }, - }, + '/api/v2/webhook/bitbucket/{}/'.format(self.project.slug), + self.bitbucket_payload, format='json', ) trigger_build.assert_has_calls( [mock.call(force=True, version=mock.ANY, project=self.project)]) client.post( - '/api/v2/webhook/bitbucket/{0}/'.format(self.project.slug), + '/api/v2/webhook/bitbucket/{}/'.format(self.project.slug), { 'push': { 'changes': [ { 'new': {'name': 'non-existent'}, + 'old': {'name': 'master'}, }, ], }, @@ -1130,7 +1137,7 @@ def test_bitbucket_webhook(self, trigger_build): trigger_build_call_count = trigger_build.call_count client.post( - '/api/v2/webhook/bitbucket/{0}/'.format(self.project.slug), + '/api/v2/webhook/bitbucket/{}/'.format(self.project.slug), { 'push': { 'changes': [ @@ -1144,6 +1151,42 @@ def test_bitbucket_webhook(self, trigger_build): ) self.assertEqual(trigger_build_call_count, trigger_build.call_count) + @mock.patch('readthedocs.core.views.hooks.sync_repository_task') + def test_bitbucket_push_hook_creation( + self, sync_repository_task, trigger_build): + client = APIClient() + self.bitbucket_payload['push']['changes'][0]['old'] = None + resp = client.post( + '/api/v2/webhook/bitbucket/{}/'.format(self.project.slug), + self.bitbucket_payload, + format='json', + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertFalse(resp.data['build_triggered']) + self.assertEqual(resp.data['project'], self.project.slug) + self.assertEqual(resp.data['versions'], [LATEST]) + trigger_build.assert_not_called() + latest_version = self.project.versions.get(slug=LATEST) + sync_repository_task.delay.assert_called_with(latest_version.pk) + + @mock.patch('readthedocs.core.views.hooks.sync_repository_task') + def test_bitbucket_push_hook_deletion( + self, sync_repository_task, trigger_build): + client = APIClient() + self.bitbucket_payload['push']['changes'][0]['new'] = None + resp = client.post( + '/api/v2/webhook/bitbucket/{}/'.format(self.project.slug), + self.bitbucket_payload, + format='json', + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertFalse(resp.data['build_triggered']) + self.assertEqual(resp.data['project'], self.project.slug) + self.assertEqual(resp.data['versions'], [LATEST]) + trigger_build.assert_not_called() + latest_version = self.project.versions.get(slug=LATEST) + sync_repository_task.delay.assert_called_with(latest_version.pk) + def test_bitbucket_invalid_webhook(self, trigger_build): """Bitbucket webhook unhandled event.""" client = APIClient() From 107235ec17ca23c559f5daac48923c52e5f11e54 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 8 Nov 2018 16:36:54 -0500 Subject: [PATCH 13/16] Listen to creation/deletion events on bitbucket --- readthedocs/restapi/views/integrations.py | 31 ++++++++++++++++------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/readthedocs/restapi/views/integrations.py b/readthedocs/restapi/views/integrations.py index 604600299a3..bc889decad1 100644 --- a/readthedocs/restapi/views/integrations.py +++ b/readthedocs/restapi/views/integrations.py @@ -38,6 +38,7 @@ GITLAB_PUSH = 'push' GITLAB_NULL_HASH = '0' * 40 GITLAB_TAG_PUSH = 'tag_push' +BITBUCKET_EVENT_HEADER = 'HTTP_X_EVENT_KEY' BITBUCKET_PUSH = 'repo:push' @@ -268,19 +269,31 @@ class BitbucketWebhookView(WebhookMixin, APIView): def handle_webhook(self): # Get event and trigger other webhook events - event = self.request.META.get('HTTP_X_EVENT_KEY', BITBUCKET_PUSH) - webhook_bitbucket.send(Project, project=self.project, - data=self.request.data, event=event) - # Handle push events and trigger builds + event = self.request.META.get(BITBUCKET_EVENT_HEADER, BITBUCKET_PUSH) + webhook_bitbucket.send( + Project, + project=self.project, + data=self.request.data, + event=event + ) if event == BITBUCKET_PUSH: try: - changes = self.request.data['push']['changes'] - branches = [change['new']['name'] - for change in changes - if change.get('new')] - return self.get_response_push(self.project, branches) + data = self.request.data + changes = data['push']['changes'] + branches = [] + for change in changes: + old = change['old'] + new = change['new'] + # Normal push to master + if old is not None and new is not None: + branches.append(new['name']) + if branches: + return self.get_response_push(self.project, branches) + else: + return self.sync_versions(self.project) except KeyError: raise ParseError('Invalid request') + return None class IsAuthenticatedOrHasToken(permissions.IsAuthenticated): From 4ee70e8f33d509753161d3df4779f6c954c5cddf Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 8 Nov 2018 17:21:47 -0500 Subject: [PATCH 14/16] Linter --- readthedocs/restapi/views/integrations.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/readthedocs/restapi/views/integrations.py b/readthedocs/restapi/views/integrations.py index bc889decad1..4c64fd8e372 100644 --- a/readthedocs/restapi/views/integrations.py +++ b/readthedocs/restapi/views/integrations.py @@ -289,8 +289,7 @@ def handle_webhook(self): branches.append(new['name']) if branches: return self.get_response_push(self.project, branches) - else: - return self.sync_versions(self.project) + return self.sync_versions(self.project) except KeyError: raise ParseError('Invalid request') return None From dd235f32e437eef8f881d45069dc49464b3c9e0f Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 15 Nov 2018 12:21:31 -0500 Subject: [PATCH 15/16] Use default branch instead of LATEST --- readthedocs/core/views/hooks.py | 35 ++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index 017f1f18368..6b00b2d77ef 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -1,21 +1,26 @@ """Views pertaining to builds.""" -from __future__ import absolute_import +from __future__ import ( + absolute_import, + division, + print_function, + unicode_literals, +) + import json +import logging import re from django.http import HttpResponse, HttpResponseNotFound from django.shortcuts import redirect from django.views.decorators.csrf import csrf_exempt -from readthedocs.core.utils import trigger_build from readthedocs.builds.constants import LATEST +from readthedocs.core.utils import trigger_build from readthedocs.projects import constants -from readthedocs.projects.models import Project, Feature +from readthedocs.projects.models import Feature, Project from readthedocs.projects.tasks import sync_repository_task -import logging - log = logging.getLogger(__name__) @@ -82,18 +87,24 @@ def sync_versions(project): This doesn't register a new build, but clones the repo and syncs the versions. Due that `sync_repository_task` is bound to a version, - we always pass the latest version. + we always pass the default version. - :returns: The version that was used to trigger the clone (usually latest). + :returns: The version slug that was used to trigger the clone. + :rtype: str """ try: - version_slug = LATEST - version = project.versions.get(slug=version_slug) + version_identifier = project.get_default_branch() + version = ( + project.versions + .filter(identifier=version_identifier) + .first() + ) + if not version: + log.info('Unable to sync from %s version', version_identifier) + return None sync_repository_task.delay(version.pk) return version.slug - except Project.DoesNotExist: - log.info('Unable to sync from %s version', version_slug) - except Exception as e: + except Exception: log.exception('Unknown sync versions exception') return None From 804d7175946317a4eead19df3e09ceec83e6321c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 15 Nov 2018 12:50:10 -0500 Subject: [PATCH 16/16] Add docs and comments --- readthedocs/restapi/views/integrations.py | 49 +++++++++++++++++++++-- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/readthedocs/restapi/views/integrations.py b/readthedocs/restapi/views/integrations.py index 4c64fd8e372..008ae1dd826 100644 --- a/readthedocs/restapi/views/integrations.py +++ b/readthedocs/restapi/views/integrations.py @@ -160,6 +160,12 @@ class GitHubWebhookView(WebhookMixin, APIView): "ref": "branch-name", ... } + + See full payload here: + + - https://developer.github.com/v3/activity/events/types/#pushevent + - https://developer.github.com/v3/activity/events/types/#createevent + - https://developer.github.com/v3/activity/events/types/#deleteevent """ integration_type = Integration.GITHUB_WEBHOOK @@ -175,8 +181,12 @@ def get_data(self): def handle_webhook(self): # Get event and trigger other webhook events event = self.request.META.get(GITHUB_EVENT_HEADER, GITHUB_PUSH) - webhook_github.send(Project, project=self.project, - data=self.data, event=event) + webhook_github.send( + Project, + project=self.project, + data=self.data, + event=event + ) # Handle push events and trigger builds if event == GITHUB_PUSH: try: @@ -203,16 +213,29 @@ class GitLabWebhookView(WebhookMixin, APIView): Expects the following JSON:: { + "before": "95790bf891e76fee5e1747ab589903a6a1f80f22", + "after": "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", "object_kind": "push", "ref": "branch-name", ... } + + See full payload here: + + - https://docs.gitlab.com/ce/user/project/integrations/webhooks.html#push-events + - https://docs.gitlab.com/ce/user/project/integrations/webhooks.html#tag-events """ integration_type = Integration.GITLAB_WEBHOOK def handle_webhook(self): - # Get event and trigger other webhook events + """ + Handle GitLab events for push and tag_push. + + GitLab doesn't have a separate event for creation/deletion, + instead, it sets the before/after field to + 0000000000000000000000000000000000000000 ('0' * 40) + """ event = self.request.data.get('object_kind', GITLAB_PUSH) webhook_gitlab.send( Project, @@ -257,18 +280,32 @@ class BitbucketWebhookView(WebhookMixin, APIView): "name": "branch-name", ... }, + "old" { + "name": "branch-name", + ... + }, ... }], ... }, ... } + + See full payload here: + + - https://confluence.atlassian.com/bitbucket/event-payloads-740262817.html#EventPayloads-Push """ integration_type = Integration.BITBUCKET_WEBHOOK def handle_webhook(self): - # Get event and trigger other webhook events + """ + Handle BitBucket events for push. + + BitBucket doesn't have a separate event for creation/deletion, + instead it sets the new attribute (null if it is a deletion) + and the old attribute (null if it is a creation). + """ event = self.request.META.get(BITBUCKET_EVENT_HEADER, BITBUCKET_PUSH) webhook_bitbucket.send( Project, @@ -287,6 +324,10 @@ def handle_webhook(self): # Normal push to master if old is not None and new is not None: branches.append(new['name']) + # BitBuck returns an array of changes rather than + # one webhook per change. If we have at least one normal push + # we don't trigger the sync versions, because that + # will be triggered with the normal push. if branches: return self.get_response_push(self.project, branches) return self.sync_versions(self.project)