diff --git a/CLAUDE.md b/CLAUDE.md index 8d06a39..7a54a87 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,9 +24,8 @@ uv build ### Core Components **octohook/__init__.py** - Entry point providing: -- `setup(modules, model_overrides)` - Configures octohook by loading webhook handlers and registering model overrides. Validates that model overrides inherit from base classes. Raises on import errors. Always calls reset() first to clear existing state. -- `reset()` - Clears all registered hooks, imported modules, and model overrides. Returns octohook to unconfigured state. -- `_model_overrides` - Internal dict for extending/replacing model classes (private, set via setup()) +- `setup(modules)` - Configures octohook by loading webhook handlers. Raises on import errors. Always calls reset() first to clear existing state. +- `reset()` - Clears all registered hooks and imported modules. Returns octohook to unconfigured state. - `OctohookConfigError` - Exception raised for configuration errors - Exports: `hook`, `handle_webhook`, `parse`, `setup`, `reset`, `WebhookEvent`, `WebhookEventAction`, `OctohookConfigError` @@ -45,7 +44,7 @@ uv build - `parse(event_name, payload)` - Factory function that returns appropriate event object **octohook/models.py** - Model classes: -- `BaseGithubModel` - Uses `__new__` to check `model_overrides` dict and instantiate override classes +- `BaseGithubModel` - Base class for all GitHub models - Core models: `User`, `Repository`, `PullRequest`, `Issue`, `Comment`, etc. - URL interpolation: Many models have methods like `archive_url(format, ref)` that fill in URL templates - `_transform(url, local_variables)` - Helper that replaces `{param}` and `{/param}` patterns in URLs @@ -63,25 +62,6 @@ Handler resolution order: 1. If any debug hooks exist for the event, ONLY debug hooks run 2. Otherwise: handlers for (event, specific_action, ANY_REPO) + (event, specific_action, specific_repo) + (event, ANY_ACTION, ANY_REPO) -### Model Override System - -Users can extend/replace models via `setup()`: - -```python -from octohook.models import PullRequest - -class MyPullRequest(PullRequest): - def custom_method(self): - pass - -octohook.setup( - modules=["hooks"], - model_overrides={PullRequest: MyPullRequest} -) -``` - -When any model is instantiated, `BaseGithubModel.__new__` checks `_model_overrides` and substitutes the custom class. This allows adding custom methods/properties to GitHub objects without modifying octohook source. - ### Payload Inconsistencies GitHub sends different payload structures for the same model depending on the event type and action. For example: diff --git a/README.md b/README.md index aa66ce7..4d98363 100644 --- a/README.md +++ b/README.md @@ -118,34 +118,3 @@ def webhook(): ``` `handle_webhook` goes through all the handlers sequentially and blocks till everything is done. Any exceptions are logged to `logging.getLogger('octohook')`. You can configure the output stream of this logger to capture the logs. - -### Model Overrides - -`octohook` provides a way to extend/modify the models being provided in the event object. - -```python -import octohook -from octohook.models import PullRequest - -class MyPullRequest(PullRequest): - - def custom_work(self): - pass - -octohook.setup( - modules=["module_a"], - model_overrides={ - PullRequest: MyPullRequest - } -) -``` - -Now, everytime `octohook` attempts to initialize a `PullRequest` object, it will initialize `MyPullRequest` instead. - -Check the [test](tests/test_model_override.py) for example usage. - -**Note** - -- The class is initialized with the relevant `payload: dict` data from the incoming event payload. -- You must subclass the original model class - `setup()` validates this automatically. -- Type hints are no longer reliable for the overridden classes. diff --git a/octohook/__init__.py b/octohook/__init__.py index 4b7c2a4..72cedde 100644 --- a/octohook/__init__.py +++ b/octohook/__init__.py @@ -2,7 +2,7 @@ from importlib import import_module from pathlib import Path from pkgutil import walk_packages -from typing import List, Optional, Dict, Type +from typing import List from .decorators import hook, handle_webhook from .events import parse, WebhookEvent, WebhookEventAction @@ -23,7 +23,6 @@ logger = logging.getLogger("octohook") _imported_modules = [] -_model_overrides = {} class OctohookConfigError(Exception): @@ -49,13 +48,9 @@ def _import_module(module: str) -> List[str]: return imported_modules -def setup( - *, - modules: List[str], - model_overrides: Optional[Dict[Type, Type]] = None, -) -> None: +def setup(*, modules: List[str]) -> None: """ - Configure octohook by loading webhook handlers and registering model overrides. + Configure octohook by loading webhook handlers. This function clears any existing configuration via reset(), then recursively imports the specified modules to discover and register all decorated webhook @@ -64,45 +59,18 @@ def setup( Args: modules: List of fully-qualified module paths containing webhook handlers. Modules are imported recursively. Cannot use relative imports. - model_overrides: Optional mapping of base model classes to custom subclasses. - All custom classes are validated to ensure they inherit from - the base class they override. Raises: ImportError: If any specified module cannot be imported. - TypeError: If a model override is not a class or not a subclass of the base model. Example: >>> import octohook - >>> from octohook.models import PullRequest - >>> - >>> class CustomPullRequest(PullRequest): - ... pass - >>> - >>> octohook.setup( - ... modules=["hooks.github", "hooks.slack"], - ... model_overrides={PullRequest: CustomPullRequest} - ... ) + >>> octohook.setup(modules=["hooks.github", "hooks.slack"]) """ - global _imported_modules, _model_overrides + global _imported_modules reset() - if model_overrides: - for base_class, override_class in model_overrides.items(): - if not isinstance(override_class, type): - raise TypeError( - f"Model override for {base_class.__name__} must be a class, " - f"got {type(override_class).__name__}" - ) - if not issubclass(override_class, base_class): - raise TypeError( - f"Model override {override_class.__name__} must be a subclass of " - f"{base_class.__name__}" - ) - - _model_overrides = model_overrides.copy() - for module in modules: _imported_modules.extend(_import_module(module)) @@ -111,9 +79,8 @@ def reset() -> None: """ Clear all octohook configuration and return to unconfigured state. - Removes all registered webhook handlers, clears the list of imported modules, - and removes all model overrides. After calling reset(), setup() must be called - again before handling webhooks. + Removes all registered webhook handlers and clears the list of imported modules. + After calling reset(), setup() must be called again before handling webhooks. This function is automatically called by setup() to ensure a clean configuration. It can also be called directly to clear octohook state. @@ -123,9 +90,8 @@ def reset() -> None: >>> octohook.reset() >>> octohook.setup(modules=["hooks"]) """ - global _imported_modules, _model_overrides + global _imported_modules from octohook.decorators import _decorator _decorator.handlers.clear() _imported_modules.clear() - _model_overrides.clear() diff --git a/octohook/models.py b/octohook/models.py index b1f206d..04c02de 100644 --- a/octohook/models.py +++ b/octohook/models.py @@ -28,11 +28,7 @@ def _transform(url: str, local_variables: dict) -> str: class BaseGithubModel(ABC): - def __new__(cls, *args, **kwargs): - from octohook import _model_overrides - - cls = _model_overrides.get(cls) or cls - return object.__new__(cls) + pass class Enterprise(BaseGithubModel): diff --git a/tests/test_edge_cases.py b/tests/test_edge_cases.py index 45a7d1b..60f7385 100644 --- a/tests/test_edge_cases.py +++ b/tests/test_edge_cases.py @@ -9,7 +9,6 @@ from octohook.events import WebhookEvent, WebhookEventAction, parse, BaseWebhookEvent -from octohook.models import User import octohook @@ -146,46 +145,6 @@ def test_all_fixture_examples_parse_correctly(fixture_loader): assert hasattr(result, "changes"), f"{event_name} missing 'changes' attribute" -def test_nested_model_overrides_apply_recursively(monkeypatch, fixture_loader): - """ - Verify that model overrides apply to nested objects. - - E.g., if we override User, does PR.user use the override? - Tests that overrides apply to all User instances throughout the object tree. - """ - - class MyUser(User): - @property - def custom_field(self): - return "custom" - - monkeypatch.setattr(octohook, "_model_overrides", {User: MyUser}) - - # Load fixture - payload = fixture_loader.load("pull_request")[0] - - from octohook.events import PullRequestEvent - - event = PullRequestEvent(payload) - - # Test 1: Primary nested User object (pull_request.user) - assert isinstance(event.pull_request.user, MyUser) - assert event.pull_request.user.custom_field == "custom" - - # Test 2: Top-level sender User object - assert isinstance(event.sender, MyUser) - assert event.sender.custom_field == "custom" - - # Test 3: Repository owner User object (if present) - if hasattr(event.repository, "owner") and event.repository.owner: - assert isinstance(event.repository.owner, MyUser) - assert event.repository.owner.custom_field == "custom" - - # Note: The fixture has assignee=None, so we can't test that path - # This is acceptable as we've verified the override applies to multiple - # User instances across different nesting levels - - def test_malformed_payload_missing_required_fields(): """ Verify that parsing handles payloads with missing required fields gracefully. diff --git a/tests/test_model_override.py b/tests/test_model_override.py deleted file mode 100644 index f6d794c..0000000 --- a/tests/test_model_override.py +++ /dev/null @@ -1,44 +0,0 @@ -import pytest - -import octohook -from octohook.events import PullRequestEvent -from octohook.models import PullRequest - - -class MyPullRequest(PullRequest): - @property - def test(self): - return "test" - - -@pytest.fixture -def model_override(monkeypatch): - """ - Set up model override for testing custom model classes. - - This fixture configures octohook to use MyPullRequest instead of - the standard PullRequest class, allowing us to test the model - override system. - """ - monkeypatch.setattr(octohook, "_model_overrides", { - PullRequest: MyPullRequest, - }) - return MyPullRequest - - -def test_model_override_works(model_override, fixture_loader): - """ - Verify that model_overrides dict correctly substitutes custom classes. - - Tests that when a model class is registered in the model_overrides dict, - instances of that model use the custom class instead of the base class, - allowing users to add custom methods and properties to GitHub objects. - """ - payloads = fixture_loader.load("pull_request") - event = PullRequestEvent(payloads[0]) - - pr = event.pull_request - - # Verify override is active - assert isinstance(pr, model_override) - assert pr.test == "test" diff --git a/tests/test_setup.py b/tests/test_setup.py index d30d1e4..1946d36 100644 --- a/tests/test_setup.py +++ b/tests/test_setup.py @@ -1,28 +1,15 @@ """ Tests for octohook.setup() and octohook.reset() functions. -Verifies webhook handler loading, model override validation, and state management. +Verifies webhook handler loading and state management. """ import pytest import octohook from octohook import setup, reset, OctohookConfigError -from octohook.models import PullRequest from octohook.events import WebhookEvent, WebhookEventAction -class CustomPullRequest(PullRequest): - """Test custom model class.""" - @property - def custom_property(self): - return "custom" - - -class InvalidClass: - """Not a subclass of any model.""" - pass - - def test_setup_loads_modules(): """Verify that setup() loads hooks from specified modules.""" setup(modules=["tests.hooks.handle_hooks"]) @@ -31,34 +18,6 @@ def test_setup_loads_modules(): assert len(_decorator.handlers) > 0 -def test_setup_with_model_overrides(): - """Verify that setup() applies model overrides correctly.""" - setup( - modules=["tests.hooks.handle_hooks"], - model_overrides={PullRequest: CustomPullRequest} - ) - - assert octohook._model_overrides[PullRequest] == CustomPullRequest - - -def test_setup_validates_model_override_is_subclass(): - """Verify that setup() validates model overrides are subclasses.""" - with pytest.raises(TypeError, match="must be a subclass"): - setup( - modules=["tests.hooks.handle_hooks"], - model_overrides={PullRequest: InvalidClass} - ) - - -def test_setup_validates_model_override_is_class(): - """Verify that setup() validates model overrides are classes.""" - with pytest.raises(TypeError, match="must be a class"): - setup( - modules=["tests.hooks.handle_hooks"], - model_overrides={PullRequest: "not a class"} - ) - - def test_setup_raises_on_invalid_module(): """Verify that setup() raises ImportError for invalid modules.""" with pytest.raises(ModuleNotFoundError): @@ -93,20 +52,6 @@ def test_reset_clears_hooks(): assert len(_decorator.handlers) == 0 -def test_reset_clears_model_overrides(): - """Verify that reset() clears model overrides.""" - setup( - modules=["tests.hooks.handle_hooks"], - model_overrides={PullRequest: CustomPullRequest} - ) - - assert len(octohook._model_overrides) > 0 - - reset() - - assert len(octohook._model_overrides) == 0 - - def test_reset_clears_imported_modules(): """Verify that reset() clears imported modules tracking.""" setup(modules=["tests.hooks.handle_hooks"])