add signal for Owner (#940)

Author: nora-codecov

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"

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)

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,
+            ),
+        )

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)