From 98bc5ad1c6e012ac3fee3c5d3e4c1b232a6b9fa2 Mon Sep 17 00:00:00 2001 From: Matti Lamppu Date: Sat, 18 Feb 2023 12:36:48 +0200 Subject: [PATCH] Refactoring & add ability to filter hooks to fire based on instance --- .pre-commit-config.yaml | 4 +- pyproject.toml | 4 +- signal_webhooks/handlers.py | 108 +++++++-------------- signal_webhooks/migrations/0001_initial.py | 1 - signal_webhooks/models.py | 18 ++-- signal_webhooks/settings.py | 19 ++-- signal_webhooks/typing.py | 15 ++- signal_webhooks/utils.py | 10 +- tests/my_app/migrations/0001_initial.py | 1 - tests/test_api.py | 2 +- tests/test_hooks.py | 40 ++++---- tests/test_utils.py | 5 - 12 files changed, 103 insertions(+), 124 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 03bef77..fea7efa 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -15,11 +15,11 @@ repos: ] - repo: https://github.com/charliermarsh/ruff-pre-commit - rev: v0.0.192 + rev: v0.0.247 hooks: - id: ruff - repo: https://github.com/ambv/black - rev: 22.12.0 + rev: 23.1.0 hooks: - id: black diff --git a/pyproject.toml b/pyproject.toml index 6f791ac..760067f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "django-signal-webhooks" -version = "0.2.3" +version = "0.2.4" description = "Add webhooks to django using signals." authors = [ "Matti Lamppu ", @@ -99,7 +99,7 @@ select = [ ] ignore = [ "S101", # assert is OK - "S106", # no hardcoded passwords + "S105", # no hardcoded passwords ] [tool.ruff.per-file-ignores] diff --git a/signal_webhooks/handlers.py b/signal_webhooks/handlers.py index 3b74b0b..7c51098 100644 --- a/signal_webhooks/handlers.py +++ b/signal_webhooks/handlers.py @@ -5,7 +5,7 @@ import httpx from asgiref.sync import sync_to_async -from django.db.models import QuerySet, signals +from django.db.models import Model, QuerySet, signals from django.db.models.base import ModelBase from django.dispatch import receiver @@ -15,15 +15,15 @@ TYPE_CHECKING, Any, Callable, - ClientMethodKwargs, + ClientKwargs, Dict, HooksData, JSONData, + Method, Optional, PostDeleteData, PostSaveData, Set, - SignalChoices, ) from .utils import get_webhookhook_model, reference_for_model, tasks_as_completed, truncate @@ -33,8 +33,7 @@ __all__ = [ "default_error_handler", - "default_post_delete_handler", - "default_post_save_handler", + "default_hook_handler", "sync_task_handler", "thread_task_handler", "webhook_delete_handler", @@ -48,61 +47,48 @@ @receiver(signals.post_save, dispatch_uid=webhook_settings.DISPATCH_UID_POST_SAVE) def webhook_update_create_handler(sender: ModelBase, **kwargs) -> None: kwargs: PostSaveData - ref = reference_for_model(type(kwargs["instance"])) + webhook_handler(instance=kwargs["instance"], method="CREATE" if kwargs["created"] else "UPDATE") # type: ignore - hook: Optional[Callable] = ... - hooks: Optional[HooksData] = webhook_settings.HOOKS.get(ref) - if hooks is None: - return - if hooks is not ...: - hook = hooks.get("CREATE") if kwargs["created"] else hooks.get("UPDATE") +@receiver(signals.post_delete, dispatch_uid=webhook_settings.DISPATCH_UID_POST_DELETE) +def webhook_delete_handler(sender: ModelBase, **kwargs) -> None: + kwargs: PostDeleteData + webhook_handler(instance=kwargs["instance"], method="DELETE") + + +def webhook_handler(instance: Model, method: Method) -> None: + ref = reference_for_model(type(instance)) + + hook = find_hook_handler(ref, method) if hook is None: return - if hook is ...: - hook = default_post_save_handler try: - data = webhook_settings.SERIALIZER(kwargs["instance"]) + data = webhook_settings.SERIALIZER(instance) except WebhookCancelled as error: - method = "Create" if kwargs["created"] else "Update" - logger.info(f"{method} webhook for {ref!r} cancelled before it was sent. Reason given: {error}") + logger.info(f"{method.capitalize()} webhook for {ref!r} cancelled before it was sent. Reason given: {error}") return except Exception as error: - method = "Create" if kwargs["created"] else "Update" - logger.exception(f"{method} webhook data for {ref!r} could not be created.", exc_info=error) + logger.exception(f"{method.capitalize()} webhook data for {ref!r} could not be created.", exc_info=error) return - webhook_settings.TASK_HANDLER(hook, ref=ref, data=data, created=kwargs["created"]) + webhook_settings.TASK_HANDLER(hook, instance=instance, data=data, method=method) -@receiver(signals.post_delete, dispatch_uid=webhook_settings.DISPATCH_UID_POST_DELETE) -def webhook_delete_handler(sender: ModelBase, **kwargs) -> None: - kwargs: PostDeleteData - ref = reference_for_model(type(kwargs["instance"])) - +def find_hook_handler(ref: str, method: Method) -> Optional[Callable]: hook: Optional[Callable] = ... hooks: Optional[HooksData] = webhook_settings.HOOKS.get(ref) if hooks is None: - return + return None if hooks is not ...: - hook = hooks.get("DELETE") + hook = hooks.get(method) # type: ignore if hook is None: - return + return None if hook is ...: - hook = default_post_delete_handler + hook = default_hook_handler - try: - data = webhook_settings.SERIALIZER(kwargs["instance"]) - except WebhookCancelled as error: - logger.info(f"Delete webhook for {ref!r} cancelled before it was sent. Reason given: {error}") - return - except Exception as error: - logger.exception(f"Delete webhook data for {ref!r} could not be created.", exc_info=error) - return - - webhook_settings.TASK_HANDLER(hook, ref=ref, data=data) + return hook def default_error_handler(hook: "Webhook", error: Optional[Exception]) -> None: @@ -122,52 +108,32 @@ def sync_task_handler(hook: Callable[..., None], **kwargs: Any) -> None: hook(**kwargs) -def default_post_save_handler(ref: str, data: JSONData, created: bool) -> None: - webhook_model = get_webhookhook_model() - - signal_types = SignalChoices.create_choises() if created else SignalChoices.update_choises() - hooks = webhook_model.objects.get_for_ref(ref, signals=signal_types) - +def default_hook_handler(instance: Model, data: JSONData, method: Method) -> None: + hooks: QuerySet["Webhook"] = get_webhookhook_model().objects.get_for_model(instance, method=method) if not hooks.exists(): return - method_data: Dict[int, ClientMethodKwargs] = {} - for hook in hooks: - method_data[hook.id] = webhook_settings.CLIENT_KWARGS(hook) - method_data[hook.id].setdefault("headers", {}) - method_data[hook.id]["headers"].update(hook.default_headers()) - method_data[hook.id]["json"] = data - - asyncio.run(fire_webhooks(hooks, method_data)) - - -def default_post_delete_handler(ref: str, data: JSONData) -> None: - webhook_model = get_webhookhook_model() + client_kwargs = build_client_kwargs_by_hook_id(hooks) + asyncio.run(fire_webhooks(hooks, data, client_kwargs)) - signal_types = SignalChoices.delete_choises() - hooks = webhook_model.objects.get_for_ref(ref, signals=signal_types) - if not hooks.exists(): - return - - method_data: Dict[int, ClientMethodKwargs] = {} +def build_client_kwargs_by_hook_id(hooks: QuerySet["Webhook"]) -> Dict[int, ClientKwargs]: + client_kwargs_by_hook_id: Dict[int, ClientKwargs] = {} for hook in hooks: - method_data[hook.id] = webhook_settings.CLIENT_KWARGS(hook) - method_data[hook.id].setdefault("headers", {}) - method_data[hook.id]["headers"].update(hook.default_headers()) - method_data[hook.id]["json"] = data - - asyncio.run(fire_webhooks(hooks, method_data)) + client_kwargs_by_hook_id[hook.id] = webhook_settings.CLIENT_KWARGS(hook) + client_kwargs_by_hook_id[hook.id].setdefault("headers", {}) + client_kwargs_by_hook_id[hook.id]["headers"].update(hook.default_headers()) + return client_kwargs_by_hook_id -async def fire_webhooks(hooks: QuerySet["Webhook"], method_data: Dict[int, ClientMethodKwargs]) -> None: +async def fire_webhooks(hooks: QuerySet["Webhook"], data: JSONData, client_kwargs: Dict[int, ClientKwargs]) -> None: futures: Set[asyncio.Task] = set() hooks_by_name: Dict[str, "Webhook"] = {hook.name: hook for hook in hooks} webhook_model = get_webhookhook_model() async with httpx.AsyncClient(timeout=webhook_settings.TIMEOUT, follow_redirects=True) as client: for hook in hooks: - futures.add(asyncio.Task(client.post(hook.endpoint, **method_data[hook.id]), name=hook.name)) + futures.add(asyncio.Task(client.post(hook.endpoint, json=data, **client_kwargs[hook.id]), name=hook.name)) async for task in tasks_as_completed(futures): hook = hooks_by_name[task.get_name()] diff --git a/signal_webhooks/migrations/0001_initial.py b/signal_webhooks/migrations/0001_initial.py index 6a64d2c..94b9bde 100644 --- a/signal_webhooks/migrations/0001_initial.py +++ b/signal_webhooks/migrations/0001_initial.py @@ -7,7 +7,6 @@ class Migration(migrations.Migration): - initial = True dependencies = [] diff --git a/signal_webhooks/models.py b/signal_webhooks/models.py index b030883..1e28c07 100644 --- a/signal_webhooks/models.py +++ b/signal_webhooks/models.py @@ -1,10 +1,11 @@ from datetime import datetime from django.db import models -from django.db.models.base import ModelBase +from django.db.models import Model from .fields import TokenField -from .typing import Any, Dict, Optional, Sequence, SignalChoices +from .settings import webhook_settings +from .typing import METHOD_SIGNALS, Any, Dict, Method, Optional, SignalChoices from .utils import decode_cipher_key, is_dict, model_from_reference, reference_for_model __all__ = [ @@ -16,11 +17,14 @@ class WebhookQuerySet(models.QuerySet["Webhook"]): """Webhook queryset.""" - def get_for_ref(self, ref: str, signals: Sequence[SignalChoices]) -> models.QuerySet["Webhook"]: - return self.filter(ref=ref, signal__in=signals, enabled=True) - - def get_for_model(self, model: ModelBase, signals: Sequence[SignalChoices]) -> models.QuerySet["Webhook"]: - return self.get_for_ref(reference_for_model(model), signals) # pragma: no cover + def get_for_model(self, instance: Model, method: Method) -> models.QuerySet["Webhook"]: + kwargs: Dict[str, Any] = webhook_settings.FILTER_KWARGS(instance, method) + return self.filter( + ref=reference_for_model(type(instance)), + signal__in=METHOD_SIGNALS[method], + enabled=True, + **kwargs, + ) class WebhookBase(models.Model): diff --git a/signal_webhooks/settings.py b/signal_webhooks/settings.py index 5d51e46..030c9bd 100644 --- a/signal_webhooks/settings.py +++ b/signal_webhooks/settings.py @@ -1,7 +1,7 @@ from django.test.signals import setting_changed from settings_holder import SettingsHolder, reload_settings -from .typing import Any, Dict, HooksData, Literal, NamedTuple, Optional, Set, Union +from .typing import Any, Dict, HooksData, Method, NamedTuple, Optional, Set, Union __all__ = [ "webhook_settings", @@ -43,13 +43,19 @@ class DefaultSettings(NamedTuple): # data matching 'signal_webhooks.typing.JSONData'. SERIALIZER: str = "signal_webhooks.utils.default_serializer" # - # Default argument builder function for the http client that sends the webhooks. + # Hook for adding additional arguments for the http client that sends the webhooks. # Takes these arguments (hook: Webhook), and should return data matching - # 'signal_webhooks.typing.ClientMethodKwargs'. Data from 'SERIALIZER' will be - # added to the 'json' argument and headers from the hook will be updated - # to the 'headers' argument. + # 'signal_webhooks.typing.ClientKwargs'. Note that the headers from the hook will be + # updated to the 'headers' argument, and the data sent by the webhook will be in json form. CLIENT_KWARGS: str = "signal_webhooks.utils.default_client_kwargs" # + # Hook for adding additional filtering to the database query when selecting hooks to fire. + # Takes these arguments (instance: Model, method: Literal['CREATE', 'UPDATE', 'DELETE']), + # and should return a dict with the additional arguments passed to 'QuerySet.filter()'. + # See 'signal_webhooks.models.WebhookQuerySet.get_for_model' for the filtering arguments + # that are already added by default. + FILTER_KWARGS: str = "signal_webhooks.utils.default_filter_kwargs" + # # Error handing function that will be called if a webhook fails. Takes these # arguments (hook: Webhook, error: Optional[Exception]) and returns None. # "error" will be given if the webhook timed out, or a response from the @@ -78,6 +84,7 @@ class DefaultSettings(NamedTuple): "HOOKS", "SERIALIZER", "CLIENT_KWARGS", + "FILTER_KWARGS", "ERROR_HANDLER", "TASK_HANDLER", } @@ -91,7 +98,7 @@ def perform_import(self, val: Any, setting: str) -> Any: return super().perform_import(val, setting) # pragma: no cover val: Dict[str, HooksData] - method: Literal["CREATE", "UPDATE", "DELETE"] # noqa: F821 + method: Method for model_path, webhooks in val.items(): if webhooks in (..., None): continue diff --git a/signal_webhooks/typing.py b/signal_webhooks/typing.py index 04b79cc..b2ac204 100644 --- a/signal_webhooks/typing.py +++ b/signal_webhooks/typing.py @@ -40,7 +40,7 @@ __all__ = [ "Any", "Callable", - "ClientMethodKwargs", + "ClientKwargs", "Coroutine", "Dict", "Generator", @@ -50,10 +50,12 @@ "JSONValue", "List", "Literal", + "Method", "NamedTuple", "Optional", "PostDeleteData", "PostSaveData", + "Sequence", "Set", "SignalChoices", "Tuple", @@ -61,7 +63,6 @@ "TYPE_CHECKING", "TypedDict", "Union", - "Sequence", ] @@ -85,9 +86,10 @@ class PostDeleteData(TypedDict): JSONValue = Union[str, int, float, bool, None, Tuple["JSONValue"], List["JSONValue"], Dict[str, "JSONValue"]] JSONData = Union[List[Dict[str, JSONValue]], Dict[str, JSONValue]] +Method = Literal["CREATE", "UPDATE", "DELETE"] # noqa: F821 -class ClientMethodKwargs(TypedDict, total=False): +class ClientKwargs(TypedDict, total=False): content: RequestContent data: RequestData files: RequestFiles @@ -142,3 +144,10 @@ def delete_choises(cls) -> Set["SignalChoices"]: cls.UPDATE_OR_DELETE, cls.ALL, } + + +METHOD_SIGNALS: Dict[Method, Set["SignalChoices"]] = { + "CREATE": SignalChoices.create_choises(), + "UPDATE": SignalChoices.update_choises(), + "DELETE": SignalChoices.delete_choises(), +} diff --git a/signal_webhooks/utils.py b/signal_webhooks/utils.py index c4c2214..a0f66b2 100644 --- a/signal_webhooks/utils.py +++ b/signal_webhooks/utils.py @@ -13,7 +13,7 @@ from .serializers import webhook_serializer from .settings import webhook_settings -from .typing import TYPE_CHECKING, Any, ClientMethodKwargs, Generator, JSONData, Literal, Set, Type +from .typing import TYPE_CHECKING, Any, ClientKwargs, Dict, Generator, JSONData, Literal, Method, Set, Type if TYPE_CHECKING: from .models import Webhook, WebhookBase @@ -68,8 +68,12 @@ def default_serializer(instance: Model) -> JSONData: return webhook_serializer.serialize([instance]) -def default_client_kwargs(hook: "Webhook") -> ClientMethodKwargs: - return ClientMethodKwargs() +def default_client_kwargs(hook: "Webhook") -> ClientKwargs: + return ClientKwargs() + + +def default_filter_kwargs(instance: Model, method: Method) -> Dict[str, Any]: + return {} @lru_cache(maxsize=None) diff --git a/tests/my_app/migrations/0001_initial.py b/tests/my_app/migrations/0001_initial.py index df54968..dd91eff 100644 --- a/tests/my_app/migrations/0001_initial.py +++ b/tests/my_app/migrations/0001_initial.py @@ -7,7 +7,6 @@ class Migration(migrations.Migration): - initial = True dependencies = [] diff --git a/tests/test_api.py b/tests/test_api.py index 5d4f959..081586a 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -50,7 +50,7 @@ def test_webhook_api(settings, api_client: APIClient): is_superuser=True, ) - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_1: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_1: user.save() mock_1.assert_called_once() diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 2850b33..9942ba5 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -31,38 +31,38 @@ def test_webhook__default_setup(settings): is_superuser=True, ) - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_1: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_1: user.save() mock_1.assert_called_once() user.username = "xx" - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_2: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_2: user.save(update_fields=["username"]) mock_2.assert_called_once() - with patch("signal_webhooks.handlers.default_post_delete_handler") as mock_3: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_3: user.delete() mock_3.assert_called_once() group = Group(name="x") - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_4: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_4: group.save() mock_4.assert_not_called() group.name = "xx" - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_5: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_5: group.save(update_fields=["name"]) mock_5.assert_not_called() - with patch("signal_webhooks.handlers.default_post_delete_handler") as mock_6: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_6: group.delete() mock_6.assert_not_called() @@ -84,19 +84,19 @@ def test_webhook__default_setup__expicit_deny(settings): is_superuser=True, ) - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_1: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_1: user.save() mock_1.assert_not_called() user.username = "xx" - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_2: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_2: user.save(update_fields=["username"]) mock_2.assert_not_called() - with patch("signal_webhooks.handlers.default_post_delete_handler") as mock_3: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_3: user.delete() mock_3.assert_not_called() @@ -104,7 +104,6 @@ def test_webhook__default_setup__expicit_deny(settings): @pytest.mark.django_db def test_webhook__default_setup__for_methods(settings): - settings.SIGNAL_WEBHOOKS = { "TASK_HANDLER": "signal_webhooks.handlers.sync_task_handler", "HOOKS": { @@ -123,19 +122,19 @@ def test_webhook__default_setup__for_methods(settings): is_superuser=True, ) - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_1: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_1: user.save() mock_1.assert_called_once() user.username = "xx" - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_2: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_2: user.save(update_fields=["username"]) mock_2.assert_called_once() - with patch("signal_webhooks.handlers.default_post_delete_handler") as mock_3: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_3: user.delete() mock_3.assert_called_once() @@ -143,7 +142,6 @@ def test_webhook__default_setup__for_methods(settings): @pytest.mark.django_db def test_webhook__default_setup__for_methods__not_defined(settings): - settings.SIGNAL_WEBHOOKS = { "TASK_HANDLER": "signal_webhooks.handlers.sync_task_handler", "HOOKS": { @@ -158,19 +156,19 @@ def test_webhook__default_setup__for_methods__not_defined(settings): is_superuser=True, ) - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_1: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_1: user.save() mock_1.assert_not_called() user.username = "xx" - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_2: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_2: user.save(update_fields=["username"]) mock_2.assert_not_called() - with patch("signal_webhooks.handlers.default_post_delete_handler") as mock_3: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_3: user.delete() mock_3.assert_not_called() @@ -178,7 +176,6 @@ def test_webhook__default_setup__for_methods__not_defined(settings): @pytest.mark.django_db def test_webhook__default_setup__for_methods__set_none(settings): - settings.SIGNAL_WEBHOOKS = { "TASK_HANDLER": "signal_webhooks.handlers.sync_task_handler", "HOOKS": { @@ -197,19 +194,19 @@ def test_webhook__default_setup__for_methods__set_none(settings): is_superuser=True, ) - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_1: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_1: user.save() mock_1.assert_not_called() user.username = "xx" - with patch("signal_webhooks.handlers.default_post_save_handler") as mock_2: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_2: user.save(update_fields=["username"]) mock_2.assert_not_called() - with patch("signal_webhooks.handlers.default_post_delete_handler") as mock_3: + with patch("signal_webhooks.handlers.default_hook_handler") as mock_3: user.delete() mock_3.assert_not_called() @@ -217,7 +214,6 @@ def test_webhook__default_setup__for_methods__set_none(settings): @pytest.mark.django_db def test_webhook__custom_setup(settings): - settings.SIGNAL_WEBHOOKS = { "TASK_HANDLER": "signal_webhooks.handlers.sync_task_handler", "HOOKS": { diff --git a/tests/test_utils.py b/tests/test_utils.py index a8332a3..cd9c351 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -18,7 +18,6 @@ def test_model_from_reference(settings): - settings.SIGNAL_WEBHOOKS = { "TASK_HANDLER": "signal_webhooks.handlers.sync_task_handler", "HOOKS": { @@ -32,7 +31,6 @@ def test_model_from_reference(settings): def test_model_from_reference__not_a_dot_import(settings): - settings.SIGNAL_WEBHOOKS = { "TASK_HANDLER": "signal_webhooks.handlers.sync_task_handler", "HOOKS": { @@ -47,7 +45,6 @@ def test_model_from_reference__not_a_dot_import(settings): def test_model_from_reference__does_no_define_given_value(settings): - settings.SIGNAL_WEBHOOKS = { "TASK_HANDLER": "signal_webhooks.handlers.sync_task_handler", "HOOKS": { @@ -65,7 +62,6 @@ def test_model_from_reference__does_no_define_given_value(settings): def test_model_from_reference__not_a_model(settings): - settings.SIGNAL_WEBHOOKS = { "TASK_HANDLER": "signal_webhooks.handlers.sync_task_handler", "HOOKS": { @@ -80,7 +76,6 @@ def test_model_from_reference__not_a_model(settings): def test_model_from_reference__webhooks_not_defined(settings): - settings.SIGNAL_WEBHOOKS = { "TASK_HANDLER": "signal_webhooks.handlers.sync_task_handler", "HOOKS": {