implement SyncWebhookView and refactor GitHubRouter for sync support (#31)

* implement `SyncWebhookView` and refactor `GitHubRouter` for sync support

* add missing space back

* remove spaces

Author: joshuadavidthomas

CHANGELOG.md

@@ -18,6 +18,10 @@ and this project attempts to adhere to [Semantic Versioning](https://semver.org/
 
 ## [Unreleased]
 
+### Added
+
+- Added `SyncWebhookView`, a synchronous counterpart to `AsyncWebhookView` for Django applications running under WSGI. Works with `SyncGitHubAPI` and synchronous event handlers to provide a fully synchronous workflow for processing GitHub webhooks.
+
 ### Changed
 
 - `AsyncGitHubAPI` and `SyncGitHubAPI` clients can now take an instance of `Installation` using the `installation` kwarg, in addition to the previous behavior of providing the `installation_id`. One or the other must be used for authenticated requests, not both.

README.md

@@ -11,7 +11,7 @@ A Django toolkit providing the batteries needed to build GitHub Apps - from webh
 
 Built on [gidgethub](https://github.com/gidgethub/gidgethub) and [httpx](https://github.com/encode/httpx), django-github-app handles the boilerplate of GitHub App development. Features include webhook event routing and storage, API client with automatic authentication, and models for managing GitHub App installations, repositories, and webhook event history.
 
-The library primarily uses async features (following gidgethub), with sync support in active development to better integrate with the majority of Django projects.
+Fully supports both sync (WSGI) and async (ASGI) Django applications.
 
 ## Requirements
 
@@ -51,17 +51,29 @@ The library primarily uses async features (following gidgethub), with sync suppo
 
 4. Add django-github-app's webhook view to your Django project's urls.
 
+   For Django projects running on ASGI, use `django_github_app.views.AsyncWebhookView`:
+
    ```python
    from django.urls import path
-   
+
    from django_github_app.views import AsyncWebhookView
-   
+
    urlpatterns = [
        path("gh/", AsyncWebhookView.as_view()),
    ]
    ```
 
-   For the moment, django-github-app only provides an async webhook view. While sync support is being actively developed, the webhook view remains async-only.
+   For traditional Django projects running on WSGI, use `django_github_app.views.SyncWebhookView`:
+
+   ```python
+   from django.urls import path
+
+   from django_github_app.views import SyncWebhookView
+
+   urlpatterns = [
+       path("gh/", SyncWebhookView.as_view()),
+   ]
+   ```
 
 5. Setup your GitHub App, either by registering a new one or importing an existing one, and configure django-github-app using your GitHub App's information.
 
@@ -169,6 +181,8 @@ django-github-app provides a router-based system for handling GitHub webhook eve
 
 To start handling GitHub webhooks, create your event handlers in a new file (e.g., `events.py`) within your Django app.
 
+For ASGI projects using `django_github_app.views.AsyncWebhookView`:
+
 ```python
 # your_app/events.py
 from django_github_app.routing import GitHubRouter
@@ -204,15 +218,52 @@ async def welcome_new_issue(event, gh, *args, **kwargs):
     })
 ```
 
-In this example, we automatically label issues based on their title and post a welcome comment on newly opened issues. The router ensures each webhook is directed to the appropriate handler based on the event type and action.
+For WSGI projects using `django_github_app.views.SyncWebhookView`:
 
-> [!NOTE]
-> Handlers must be async functions as django-github-app uses gidgethub for webhook event routing which only supports async operations. Sync support is planned to better integrate with Django projects that don't use async.
+```python
+# your_app/events.py
+from django_github_app.routing import GitHubRouter
+
+gh = GitHubRouter()
+
+# Handle any issue event
+@gh.event("issues")
+def handle_issue(event, gh, *args, **kwargs):
+    issue = event.data["issue"]
+    labels = []
+    
+    # Add labels based on issue title
+    title = issue["title"].lower()
+    if "bug" in title:
+        labels.append("bug")
+    if "feature" in title:
+        labels.append("enhancement")
+    
+    if labels:
+        gh.post(
+            issue["labels_url"], 
+            data=labels
+        )
+
+# Handle specific issue actions
+@gh.event("issues", action="opened")
+def welcome_new_issue(event, gh, *args, **kwargs):
+    """Post a comment when a new issue is opened"""
+    url = event.data["issue"]["comments_url"]
+    gh.post(url, data={
+        "body": "Thanks for opening an issue! We'll take a look soon."
+    })
+```
+
+> [!IMPORTANT]
+> Choose either async or sync handlers based on your webhook view - async handlers for `AsyncWebhookView`, sync handlers for `SyncWebhookView`. Mixing async and sync handlers is not supported.
+
+In these examples, we automatically label issues based on their title and post a welcome comment on newly opened issues. The router ensures each webhook is directed to the appropriate handler based on the event type and action.
 
 Each handler receives two arguments:
 
 - `event`: A `gidgethub.sansio.Event` containing the webhook payload
-- `gh`: A GitHub API client for making API calls
+- `gh`: A GitHub API client for making API calls (`AsyncGitHubAPI` for async handlers, `SyncGitHubAPI` for sync handlers)
 
 To activate your webhook handlers, import them in your app's `AppConfig.ready()` method, similar to how Django signals are registered.
 

src/django_github_app/routing.py

@@ -4,16 +4,19 @@
 from typing import Any
 
 from django.utils.functional import classproperty
+from gidgethub import sansio
 from gidgethub.routing import AsyncCallback
 from gidgethub.routing import Router as GidgetHubRouter
 
+from ._typing import override
 
-class GitHubRouter:
+
+class GitHubRouter(GidgetHubRouter):
     _routers: list[GidgetHubRouter] = []
 
-    def __init__(self) -> None:
-        self.router = GidgetHubRouter()
-        GitHubRouter._routers.append(self.router)
+    def __init__(self, *args) -> None:
+        super().__init__(*args)
+        GitHubRouter._routers.append(self)
 
     @classproperty
     def routers(cls):
@@ -23,7 +26,18 @@ def event(
         self, event_type: str, **kwargs: Any
     ) -> Callable[[AsyncCallback], AsyncCallback]:
         def decorator(func: AsyncCallback) -> AsyncCallback:
-            self.router.add(func, event_type, **kwargs)
+            self.add(func, event_type, **kwargs)
             return func
 
         return decorator
+
+    async def adispatch(self, event: sansio.Event, *args: Any, **kwargs: Any) -> None:
+        found_callbacks = self.fetch(event)
+        for callback in found_callbacks:
+            await callback(event, *args, **kwargs)
+
+    @override
+    def dispatch(self, event: sansio.Event, *args: Any, **kwargs: Any) -> None:  # type: ignore[override]
+        found_callbacks = self.fetch(event)
+        for callback in found_callbacks:
+            callback(event, *args, **kwargs)

src/django_github_app/views.py

@@ -15,7 +15,6 @@
 from django.utils.decorators import method_decorator
 from django.views.decorators.csrf import csrf_exempt
 from django.views.generic import View
-from gidgethub.routing import Router as GidgetHubRouter
 from gidgethub.sansio import Event
 
 from ._typing import override
@@ -59,8 +58,8 @@ def get_response(self, event_log: EventLog) -> JsonResponse:
         )
 
     @property
-    def router(self) -> GidgetHubRouter:
-        return GidgetHubRouter(*GitHubRouter.routers)
+    def router(self) -> GitHubRouter:
+        return GitHubRouter(*GitHubRouter.routers)
 
     @abstractmethod
     def post(
@@ -84,7 +83,7 @@ async def post(self, request: HttpRequest) -> JsonResponse:
 
         async with self.get_github_api(installation) as gh:
             await gh.sleep(1)
-            await self.router.dispatch(event, gh)
+            await self.router.adispatch(event, gh)
 
         return self.get_response(event_log)
 
@@ -93,12 +92,6 @@ async def post(self, request: HttpRequest) -> JsonResponse:
 class SyncWebhookView(BaseWebhookView[SyncGitHubAPI]):
     github_api_class = SyncGitHubAPI
 
-    def __init__(self, **kwargs: Any) -> None:
-        super().__init__(**kwargs)
-        raise NotImplementedError(
-            "SyncWebhookView is planned for a future release. For now, please use AsyncWebhookView with async/await."
-        )
-
     def post(self, request: HttpRequest) -> JsonResponse:  # pragma: no cover
         event = self.get_event(request)
 
@@ -110,6 +103,6 @@ def post(self, request: HttpRequest) -> JsonResponse:  # pragma: no cover
 
         with self.get_github_api(installation) as gh:
             time.sleep(1)
-            self.router.dispatch(event, gh)  # type: ignore
+            self.router.dispatch(event, gh)
 
         return self.get_response(event_log)

tests/test_views.py

@@ -71,9 +71,43 @@ def _make_request(
 
 @pytest.fixture
 def test_router():
-    router = GitHubRouter()
-    yield router
-    GitHubRouter._routers.remove(router.router)
+    GitHubRouter._routers = []
+    yield GitHubRouter()
+    GitHubRouter._routers = []
+
+
+@pytest.fixture
+def aregister_webhook_event(test_router):
+    def _make_handler(event_type, should_fail=False):
+        data = {}
+
+        @test_router.event(event_type)
+        async def handle_event(event, gh):
+            if should_fail:
+                pytest.fail("Should not be called")
+            data["event"] = event
+            data["gh"] = gh
+
+        return data
+
+    return _make_handler
+
+
+@pytest.fixture
+def register_webhook_event(test_router):
+    def _make_handler(event_type, should_fail=False):
+        data = {}
+
+        @test_router.event(event_type)
+        def handle_event(event, gh):
+            if should_fail:
+                pytest.fail("Should not be called")
+            data["event"] = event
+            data["gh"] = gh
+
+        return data
+
+    return _make_handler
 
 
 class WebhookView(BaseWebhookView[GitHubAPI]):
@@ -176,14 +210,8 @@ async def test_missing_event(self, webhook_request):
         with pytest.raises(BadRequest):
             await view.post(request)
 
-    async def test_router_dispatch(self, test_router, webhook_request):
-        called_with = {}
-
-        @test_router.event("push")
-        async def handle_push(event, gh):
-            called_with["event"] = event
-            called_with["gh"] = gh
-
+    async def test_router_dispatch(self, aregister_webhook_event, webhook_request):
+        webhook_data = aregister_webhook_event("push")
         request = webhook_request(
             event_type="push",
             body={"action": "created", "repository": {"full_name": "test/repo"}},
@@ -193,15 +221,14 @@ async def handle_push(event, gh):
         response = await view.post(request)
 
         assert response.status_code == HTTPStatus.OK
-        assert called_with["event"].event == "push"
-        assert called_with["event"].data["repository"]["full_name"] == "test/repo"
-        assert isinstance(called_with["gh"], AsyncGitHubAPI)
-
-    async def test_router_dispatch_unhandled_event(self, test_router, webhook_request):
-        @test_router.event("push")
-        async def handle_push(event, gh):
-            pytest.fail("Should not be called")
+        assert webhook_data["event"].event == "push"
+        assert webhook_data["event"].data["repository"]["full_name"] == "test/repo"
+        assert isinstance(webhook_data["gh"], AsyncGitHubAPI)
 
+    async def test_router_dispatch_unhandled_event(
+        self, aregister_webhook_event, webhook_request
+    ):
+        aregister_webhook_event("push", should_fail=True)
         request = webhook_request(event_type="issues", body={"action": "opened"})
         view = AsyncWebhookView()
 
@@ -211,13 +238,68 @@ async def handle_push(event, gh):
 
 
 class TestSyncWebhookView:
-    def test_not_implemented_error(self):
-        with pytest.raises(NotImplementedError):
-            SyncWebhookView()
-
-    def test_post(self, webhook_request): ...
-    def test_csrf_exempt(self, webhook_request): ...
-    def test_event_log_created(self, webhook_request): ...
-    def test_event_log_cleanup(self, webhook_request): ...
-    def test_router_dispatch(self, test_router, webhook_request): ...
-    def test_router_dispatch_unhandled_event(self, test_router, webhook_request): ...
+    def test_post(self, webhook_request):
+        request = webhook_request()
+        view = SyncWebhookView()
+
+        response = view.post(request)
+
+        assert isinstance(response, JsonResponse)
+        assert response.status_code == HTTPStatus.OK
+
+    def test_csrf_exempt(self, webhook_request):
+        request = webhook_request()
+        view = SyncWebhookView()
+
+        response = view.post(request)
+
+        assert response.status_code != HTTPStatus.FORBIDDEN
+
+    def test_event_log_created(self, webhook_request):
+        request = webhook_request()
+        view = SyncWebhookView()
+
+        response = view.post(request)
+
+        event_id = json.loads(response.content)["event_id"]
+        assert EventLog.objects.filter(id=event_id).count() == 1
+
+    def test_event_log_cleanup(self, webhook_request):
+        request = webhook_request()
+        view = SyncWebhookView()
+
+        event = baker.make(
+            "django_github_app.EventLog",
+            received_at=timezone.now() - datetime.timedelta(days=8),
+        )
+        assert EventLog.objects.filter(id=event.id).count() == 1
+
+        view.post(request)
+
+        assert EventLog.objects.filter(id=event.id).count() == 0
+
+    def test_router_dispatch(self, register_webhook_event, webhook_request):
+        webhook_data = register_webhook_event("push")
+        request = webhook_request(
+            event_type="push",
+            body={"action": "created", "repository": {"full_name": "test/repo"}},
+        )
+        view = SyncWebhookView()
+
+        response = view.post(request)
+
+        assert response.status_code == HTTPStatus.OK
+        assert webhook_data["event"].event == "push"
+        assert webhook_data["event"].data["repository"]["full_name"] == "test/repo"
+        assert isinstance(webhook_data["gh"], SyncGitHubAPI)
+
+    def test_router_dispatch_unhandled_event(
+        self, register_webhook_event, webhook_request
+    ):
+        register_webhook_event("push", should_fail=True)
+        request = webhook_request(event_type="issues", body={"action": "opened"})
+        view = SyncWebhookView()
+
+        response = view.post(request)
+
+        assert response.status_code == HTTPStatus.OK