fix: lazy init and infinite recursion fixes#5
fix: lazy init and infinite recursion fixes#5vimzoomer wants to merge 2 commits intoapache:masterfrom
Conversation
- Remove CasbinAdapterConfig initialization in ready - Fix infinite recursion in __getattribute__ - Prevent test runner crash in testing environment
There was a problem hiding this comment.
Pull request overview
This PR refactors the Django Casbin adapter to avoid eager database access during app startup and fixes a recursion issue in the lazy-loading proxy enforcer, improving compatibility with Django’s initialization and test discovery behavior.
Changes:
- Remove
AppConfig.ready()-time enforcer initialization to avoid DB access during Django startup. - Fix infinite recursion in
ProxyEnforcer.__getattribute__by avoidingself.db_aliasattribute lookup. - Update README installation instructions to use
casbin_adapterinINSTALLED_APPS.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
casbin_adapter/enforcer.py |
Adjusts lazy-init mechanics, avoids recursion, and exempts __class__ from triggering initialization. |
casbin_adapter/apps.py |
Removes ready() hook that previously initialized the enforcer (and touched the DB) during app startup. |
README.md |
Updates Django installation instructions consistent with removing the custom AppConfig.ready() behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
casbin_adapter/enforcer.py
Outdated
| class ProxyEnforcer(Enforcer): | ||
| _initialized = False | ||
| db_alias = "default" | ||
| db_alias = getattr(settings, "CASBIN_DB_ALIAS", "default") |
There was a problem hiding this comment.
Using django.conf.settings at import time is generally acceptable, but can be moved to ready for extra pre-cautions
| def __getattribute__(self, name): | ||
| safe_methods = ["__init__", "_load", "_initialized"] | ||
| safe_methods = ["__init__", "_load", "_initialized", "__class__"] | ||
| if not super().__getattribute__("_initialized") and name not in safe_methods: | ||
| initialize_enforcer(self.db_alias) | ||
| initialize_enforcer(super().__getattribute__("db_alias")) | ||
| if not super().__getattribute__("_initialized"): |
There was a problem hiding this comment.
Testing it properly wouldn't be that easy, currently calling any method from safe_methods obviously won't trigger any errors, same with super().__getattribute__("db_alias")
32f9f7a to
70721f8
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Hi, I have two fixes for the adapter:
readyis discouraged by Django, sinceProxyEnforcerlazy-initializes Casbin anyway, initialization should be removed fromCasbinAdapterConfig.__getattribute__(self, name)caused byself.db_alias. Also, Ifenforceris imported in testing environment it breaks Django's test runner which calls__class__on every imported module during test discovery.