diff --git a/codecov/settings_test.py b/codecov/settings_test.py index 5636caa723..5d2466ec2f 100644 --- a/codecov/settings_test.py +++ b/codecov/settings_test.py @@ -1,22 +1,12 @@ -from .settings_base import * +import os + +from .settings_dev import * -DEBUG = False ALLOWED_HOSTS = ["localhost"] -WEBHOOK_URL = "" # NGROK TUNNEL HERE -STRIPE_API_KEY = "" CORS_ALLOWED_ORIGINS = ["http://localhost:9000", "http://localhost"] -CORS_ALLOW_CREDENTIALS = True -CODECOV_URL = "localhost" -CODECOV_API_URL = get_config("setup", "codecov_api_url", default=CODECOV_URL) -DATABASE_HOST = "postgres" +SHELTER_PUBSUB_PROJECT_ID = "test-project-id" +SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID = "test-topic-id" -DATABASES = { - "default": { - "ENGINE": "django.db.backends.postgresql", - "NAME": DATABASE_NAME, - "USER": DATABASE_USER, - "PASSWORD": DATABASE_PASSWORD, - "HOST": DATABASE_HOST, - "PORT": "5432", - } -} +# Mock the Pub/Sub host for testing +# this prevents the pubsub SDK from trying to load credentials +os.environ["PUBSUB_EMULATOR_HOST"] = "localhost" diff --git a/codecov_auth/signals.py b/codecov_auth/signals.py index 6a92548a70..8bf81ab68c 100644 --- a/codecov_auth/signals.py +++ b/codecov_auth/signals.py @@ -1,47 +1,53 @@ -import json +from typing import Any, Dict, Optional, Type -from django.conf import settings from django.db.models.signals import post_save from django.dispatch import receiver -from google.cloud import pubsub_v1 from codecov_auth.models import OrganizationLevelToken, Owner, OwnerProfile +from utils.shelter import ShelterPubsub @receiver(post_save, sender=Owner) def create_owner_profile_when_owner_is_created( - sender, instance: Owner, created, **kwargs -): + sender: Type[Owner], instance: Owner, created: bool, **kwargs: Dict[str, Any] +) -> Optional[OwnerProfile]: if created: return OwnerProfile.objects.create(owner_id=instance.ownerid) -_pubsub_publisher = None - - -def _get_pubsub_publisher(): - global _pubsub_publisher - if not _pubsub_publisher: - _pubsub_publisher = pubsub_v1.PublisherClient() - return _pubsub_publisher - - @receiver( post_save, sender=OrganizationLevelToken, dispatch_uid="shelter_sync_org_token" ) -def update_org_token(sender, instance: OrganizationLevelToken, **kwargs): - pubsub_project_id = settings.SHELTER_PUBSUB_PROJECT_ID - topic_id = settings.SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID - if pubsub_project_id and topic_id: - publisher = _get_pubsub_publisher() - topic_path = publisher.topic_path(pubsub_project_id, topic_id) - publisher.publish( - topic_path, - json.dumps( - { - "type": "org_token", - "sync": "one", - "id": instance.id, - } - ).encode("utf-8"), - ) +def update_org_token( + sender: Type[OrganizationLevelToken], + instance: OrganizationLevelToken, + **kwargs: Dict[str, Any], +) -> None: + data = { + "type": "org_token", + "sync": "one", + "id": instance.id, + } + ShelterPubsub.get_instance().publish(data) + + +@receiver(post_save, sender=Owner, dispatch_uid="shelter_sync_owner") +def update_owner( + sender: Type[Owner], instance: Owner, **kwargs: Dict[str, Any] +) -> None: + """ + Shelter tracks a limited set of Owner fields - only update if those fields have changed. + """ + created: bool = kwargs["created"] + tracked_fields = [ + "upload_token_required_for_public_repos", + "username", + "service", + ] + if created or any(instance.tracker.has_changed(field) for field in tracked_fields): + data = { + "type": "owner", + "sync": "one", + "id": instance.ownerid, + } + ShelterPubsub.get_instance().publish(data) diff --git a/codecov_auth/tests/test_signals.py b/codecov_auth/tests/test_signals.py index 60284febd6..639978b453 100644 --- a/codecov_auth/tests/test_signals.py +++ b/codecov_auth/tests/test_signals.py @@ -1,27 +1,121 @@ -import os +from unittest import mock +from unittest.mock import call import pytest -from django.test import override_settings +from django.test import TestCase +from shared.django_apps.codecov_auth.models import Service from shared.django_apps.codecov_auth.tests.factories import ( OrganizationLevelTokenFactory, + OwnerFactory, ) -@override_settings( - SHELTER_PUBSUB_PROJECT_ID="test-project-id", - SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID="test-topic-id", -) @pytest.mark.django_db def test_shelter_org_token_sync(mocker): - # this prevents the pubsub SDK from trying to load credentials - os.environ["PUBSUB_EMULATOR_HOST"] = "localhost" - publish = mocker.patch("google.cloud.pubsub_v1.PublisherClient.publish") # this triggers the publish via Django signals - OrganizationLevelTokenFactory(id=91728376) + OrganizationLevelTokenFactory(id=91728376, owner=OwnerFactory(ownerid=111)) - publish.assert_called_once_with( - "projects/test-project-id/topics/test-topic-id", - b'{"type": "org_token", "sync": "one", "id": 91728376}', + publish.assert_has_calls( + [ + call( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "owner", "sync": "one", "id": 111}', + ), + call( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "org_token", "sync": "one", "id": 91728376}', + ), + ] ) + + +@mock.patch("google.cloud.pubsub_v1.PublisherClient.publish") +class TestCodecovAuthSignals(TestCase): + def test_sync_on_create(self, mock_publish): + OwnerFactory(ownerid=12345) + mock_publish.assert_called_once_with( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "owner", "sync": "one", "id": 12345}', + ) + + def test_sync_on_update_upload_token_required_for_public_repos(self, mock_publish): + owner = OwnerFactory(ownerid=12345, upload_token_required_for_public_repos=True) + owner.upload_token_required_for_public_repos = False + owner.save() + mock_publish.assert_has_calls( + [ + call( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "owner", "sync": "one", "id": 12345}', + ), + call( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "owner", "sync": "one", "id": 12345}', + ), + ] + ) + + def test_sync_on_update_username(self, mock_publish): + owner = OwnerFactory(ownerid=12345, username="hello") + owner.username = "world" + owner.save() + mock_publish.assert_has_calls( + [ + call( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "owner", "sync": "one", "id": 12345}', + ), + call( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "owner", "sync": "one", "id": 12345}', + ), + ] + ) + + def test_sync_on_update_service(self, mock_publish): + owner = OwnerFactory(ownerid=12345, service=Service.GITHUB.value) + owner.service = Service.BITBUCKET.value + owner.save() + mock_publish.assert_has_calls( + [ + call( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "owner", "sync": "one", "id": 12345}', + ), + call( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "owner", "sync": "one", "id": 12345}', + ), + ] + ) + + def test_no_sync_on_update_other_fields(self, mock_publish): + owner = OwnerFactory(ownerid=12345, name="hello") + owner.name = "world" + owner.save() + mock_publish.assert_called_once_with( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "owner", "sync": "one", "id": 12345}', + ) + + @mock.patch("logging.Logger.warning") + def test_sync_error(self, mock_log, mock_publish): + mock_publish.side_effect = Exception("publish error") + + OwnerFactory(ownerid=12345) + + # publish is still called, raises an Exception + mock_publish.assert_called_once_with( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "owner", "sync": "one", "id": 12345}', + ) + + mock_log.assert_called_once_with( + "Failed to publish a message", + extra=dict( + data_to_publish={"type": "owner", "sync": "one", "id": 12345}, + error=mock_publish.side_effect, + ), + ) diff --git a/core/signals.py b/core/signals.py index 55e232739f..c98db4e8e6 100644 --- a/core/signals.py +++ b/core/signals.py @@ -1,68 +1,43 @@ -import json import logging +from typing import Any, Dict, List, Type -from django.conf import settings from django.db.models.signals import post_save from django.dispatch import receiver -from google.cloud import pubsub_v1 from shared.django_apps.core.models import Commit from core.models import Repository +from utils.shelter import ShelterPubsub -_pubsub_publisher = None log = logging.getLogger(__name__) -def _get_pubsub_publisher(): - global _pubsub_publisher - if not _pubsub_publisher: - _pubsub_publisher = pubsub_v1.PublisherClient() - return _pubsub_publisher - - @receiver(post_save, sender=Repository, dispatch_uid="shelter_sync_repo") -def update_repository(sender, instance: Repository, **kwargs): +def update_repository( + sender: Type[Repository], instance: Repository, **kwargs: Dict[str, Any] +) -> None: log.info(f"Signal triggered for repository {instance.repoid}") - created = kwargs["created"] - changes = instance.tracker.changed() - if created or any([field in changes for field in ["name", "upload_token"]]): - try: - pubsub_project_id = settings.SHELTER_PUBSUB_PROJECT_ID - topic_id = settings.SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID - if pubsub_project_id and topic_id: - publisher = _get_pubsub_publisher() - topic_path = publisher.topic_path(pubsub_project_id, topic_id) - publisher.publish( - topic_path, - json.dumps( - { - "type": "repo", - "sync": "one", - "id": instance.repoid, - } - ).encode("utf-8"), - ) - log.info(f"Message published for repository {instance.repoid}") - except Exception as e: - log.warning(f"Failed to publish message for repo {instance.repoid}: {e}") + created: bool = kwargs["created"] + changes: Dict[str, Any] = instance.tracker.changed() + tracked_fields: List[str] = ["name", "upload_token"] + + if created or any([field in changes for field in tracked_fields]): + data = { + "type": "repo", + "sync": "one", + "id": instance.repoid, + } + ShelterPubsub.get_instance().publish(data) @receiver(post_save, sender=Commit, dispatch_uid="shelter_sync_commit") -def update_commit(sender, instance: Commit, **kwargs): - branch = instance.branch +def update_commit( + sender: Type[Commit], instance: Commit, **kwargs: Dict[str, Any] +) -> None: + branch: str = instance.branch if branch and ":" in branch: - pubsub_project_id = settings.SHELTER_PUBSUB_PROJECT_ID - topic_id = settings.SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID - if pubsub_project_id and topic_id: - publisher = _get_pubsub_publisher() - topic_path = publisher.topic_path(pubsub_project_id, topic_id) - publisher.publish( - topic_path, - json.dumps( - { - "type": "commit", - "sync": "one", - "id": instance.id, - } - ).encode("utf-8"), - ) + data = { + "type": "commit", + "sync": "one", + "id": instance.id, + } + ShelterPubsub.get_instance().publish(data) diff --git a/core/tests/test_signals.py b/core/tests/test_signals.py index 9752331ed6..add2422f03 100644 --- a/core/tests/test_signals.py +++ b/core/tests/test_signals.py @@ -1,39 +1,39 @@ -import os from unittest.mock import call import pytest -from django.test import override_settings +from shared.django_apps.codecov_auth.tests.factories import OwnerFactory from shared.django_apps.core.tests.factories import CommitFactory, RepositoryFactory -@override_settings( - SHELTER_PUBSUB_PROJECT_ID="test-project-id", - SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID="test-topic-id", -) @pytest.mark.django_db def test_shelter_repo_sync(mocker): - # this prevents the pubsub SDK from trying to load credentials - os.environ["PUBSUB_EMULATOR_HOST"] = "localhost" - publish = mocker.patch("google.cloud.pubsub_v1.PublisherClient.publish") # this triggers the publish via Django signals - repo = RepositoryFactory(repoid=91728376) + repo = RepositoryFactory(repoid=91728376, author=OwnerFactory(ownerid=555)) # triggers publish on create - publish.assert_called_once_with( - "projects/test-project-id/topics/test-topic-id", - b'{"type": "repo", "sync": "one", "id": 91728376}', + publish.assert_has_calls( + [ + call( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "owner", "sync": "one", "id": 555}', + ), + call( + "projects/test-project-id/topics/test-topic-id", + b'{"type": "repo", "sync": "one", "id": 91728376}', + ), + ] ) repo.upload_token = "b69cf44c-89d8-48c2-80c9-5508610d3ced" repo.save() publish_calls = publish.call_args_list - assert len(publish_calls) == 2 + assert len(publish_calls) == 3 # triggers publish on update - assert publish_calls[1] == call( + assert publish_calls[2] == call( "projects/test-project-id/topics/test-topic-id", b'{"type": "repo", "sync": "one", "id": 91728376}', ) @@ -43,29 +43,29 @@ def test_shelter_repo_sync(mocker): publish_calls = publish.call_args_list # does not trigger another publish - assert len(publish_calls) == 2 + assert len(publish_calls) == 3 -@override_settings( - SHELTER_PUBSUB_PROJECT_ID="test-project-id", - SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID="test-topic-id", -) @pytest.mark.django_db def test_shelter_commit_sync(mocker): - # this prevents the pubsub SDK from trying to load credentials - os.environ["PUBSUB_EMULATOR_HOST"] = "localhost" publish = mocker.patch("google.cloud.pubsub_v1.PublisherClient.publish") # this triggers the publish via Django signals - has to have this format - commit = CommitFactory(id=167829367, branch="random:branch") + owner = OwnerFactory(ownerid=555) + commit = CommitFactory( + id=167829367, + branch="random:branch", + author=owner, + repository=RepositoryFactory(author=owner), + ) publish_calls = publish.call_args_list - # Twice cause there's a signal triggered when the commit factory creates a Repository + # 3x cause there's a signal triggered when the commit factory requires a Repository and Owner # which can't be null - assert len(publish_calls) == 2 + assert len(publish_calls) == 3 # triggers publish on update - assert publish_calls[1] == call( + assert publish_calls[2] == call( "projects/test-project-id/topics/test-topic-id", b'{"type": "commit", "sync": "one", "id": 167829367}', ) @@ -75,4 +75,4 @@ def test_shelter_commit_sync(mocker): publish_calls = publish.call_args_list # does not trigger another publish since unchanged length - assert len(publish_calls) == 2 + assert len(publish_calls) == 3 diff --git a/pytest.ini b/pytest.ini index 1a1cea04cc..ec326ff696 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,3 +1,3 @@ [pytest] -DJANGO_SETTINGS_MODULE = codecov.settings_dev +DJANGO_SETTINGS_MODULE = codecov.settings_test addopts = -p no:warnings --ignore=shared --ignore-glob=**/test_results* diff --git a/requirements.in b/requirements.in index 722646d649..54f90d93a1 100644 --- a/requirements.in +++ b/requirements.in @@ -20,7 +20,7 @@ factory-boy fakeredis freezegun https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem -https://github.com/codecov/shared/archive/067b2e4ec72cdf1b3213306ea8aaaccbb374aecc.tar.gz#egg=shared +https://github.com/codecov/shared/archive/83b3376fef128a8c8f4191212d89b97f3b507d15.tar.gz#egg=shared google-cloud-pubsub gunicorn>=22.0.0 https://github.com/photocrowd/django-cursor-pagination/archive/f560902696b0c8509e4d95c10ba0d62700181d84.tar.gz diff --git a/requirements.txt b/requirements.txt index 0ab24f111d..cd3150ec00 100644 --- a/requirements.txt +++ b/requirements.txt @@ -419,7 +419,7 @@ sentry-sdk[celery]==2.13.0 # shared setproctitle==1.1.10 # via -r requirements.in -shared @ https://github.com/codecov/shared/archive/067b2e4ec72cdf1b3213306ea8aaaccbb374aecc.tar.gz +shared @ https://github.com/codecov/shared/archive/83b3376fef128a8c8f4191212d89b97f3b507d15.tar.gz # via -r requirements.in simplejson==3.17.2 # via -r requirements.in diff --git a/utils/shelter.py b/utils/shelter.py new file mode 100644 index 0000000000..784a2ee6ce --- /dev/null +++ b/utils/shelter.py @@ -0,0 +1,44 @@ +import json +import logging +from typing import Any, Dict + +from django.conf import settings +from google.cloud import pubsub_v1 + +log = logging.getLogger(__name__) + + +class ShelterPubsub: + pubsub_publisher = None + _instance = None + + @classmethod + def get_instance(cls) -> "ShelterPubsub": + """ + This class needs the Django settings to be fully loaded before it can be instantiated, + therefore use this method to get an instance rather than instantiating directly. + """ + if cls._instance is None: + cls._instance = cls() + return cls._instance + + def __init__(self) -> None: + if not self.pubsub_publisher: + self.pubsub_publisher = pubsub_v1.PublisherClient() + pubsub_project_id: str = settings.SHELTER_PUBSUB_PROJECT_ID + + # topic_id has REPO in the name but it is used for all types of objects + topic_id: str = settings.SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID + self.topic_path = self.pubsub_publisher.topic_path(pubsub_project_id, topic_id) + + def publish(self, data: Dict[str, Any]) -> None: + try: + self.pubsub_publisher.publish( + self.topic_path, + json.dumps(data).encode("utf-8"), + ) + except Exception as e: + log.warning( + "Failed to publish a message", + extra=dict(data_to_publish=data, error=e), + )