refactor: address quality issues

Author: mariajgrimaldi

openedx_authz/engine/enforcer.py

@@ -82,6 +82,17 @@ def configure_enforcer_auto_loading(cls, auto_load_policy_interval: int = None):
         if not cls._enforcer.is_auto_loading_running():
             cls._enforcer.start_auto_load_policy(auto_load_policy_interval)
 
+    @classmethod
+    def is_auto_save_enabled(cls) -> bool:
+        """Check if auto-save is currently enabled on the enforcer.
+
+        Returns:
+            bool: True if auto-save is enabled, False otherwise
+        """
+        if cls._enforcer is None:
+            return False
+        return cls._enforcer._e.auto_save  # pylint: disable=protected-access
+
     @classmethod
     def configure_enforcer_auto_save(cls, auto_save_policy: bool):
         """Configure auto-save on the enforcer.
@@ -95,7 +106,7 @@ def configure_enforcer_auto_save(cls, auto_save_policy: bool):
         Returns:
             None
         """
-        if cls._enforcer._e.auto_save != auto_save_policy:
+        if cls.is_auto_save_enabled() != auto_save_policy:
             cls._enforcer.enable_auto_save(auto_save_policy)
 
     @classmethod

openedx_authz/tests/test_enforcer.py

@@ -6,7 +6,7 @@
 """
 
 import time
-from unittest.mock import Mock, patch
+from unittest.mock import patch
 
 import casbin
 from ddt import data as ddt_data
@@ -594,7 +594,7 @@ def test_enforcer_auto_save_enabled_when_toggle_enabled(self, mock_toggle):
 
         enforcer = AuthzEnforcer.get_enforcer()
 
-        self.assertTrue(enforcer._e.auto_save)
+        self.assertTrue(AuthzEnforcer.is_auto_save_enabled())
 
         test_policy = [
             make_role_key("test_role"),
@@ -623,7 +623,7 @@ def test_enforcer_deactivated_when_toggle_disabled(self, mock_toggle):
 
         enforcer = AuthzEnforcer.get_enforcer()
 
-        self.assertFalse(enforcer._e.auto_save)
+        self.assertFalse(AuthzEnforcer.is_auto_save_enabled())
         self.assertFalse(enforcer.is_auto_loading_running())
 
     @patch("openedx_authz.engine.enforcer.libraries_v2_enabled")
@@ -640,7 +640,7 @@ def test_enforcer_auto_load_starts_when_toggle_enabled(self, mock_toggle):
         enforcer = AuthzEnforcer.get_enforcer()
 
         self.assertTrue(enforcer.is_auto_loading_running())
-        self.assertTrue(enforcer._e.auto_save)
+        self.assertTrue(AuthzEnforcer.is_auto_save_enabled())
 
     @patch("openedx_authz.engine.enforcer.libraries_v2_enabled")
     @override_settings(CASBIN_AUTO_LOAD_POLICY_INTERVAL=0.5)
@@ -660,7 +660,7 @@ def test_enforcer_auto_load_not_restarted_on_subsequent_calls(self, mock_toggle)
         enforcer2 = AuthzEnforcer.get_enforcer()
         self.assertIs(enforcer1, enforcer2)
         self.assertTrue(enforcer2.is_auto_loading_running())
-        self.assertTrue(enforcer2._e.auto_save)
+        self.assertTrue(AuthzEnforcer.is_auto_save_enabled())
 
     @patch("openedx_authz.engine.enforcer.libraries_v2_enabled")
     @override_settings(CASBIN_AUTO_LOAD_POLICY_INTERVAL=0)
@@ -676,12 +676,12 @@ def test_toggle_state_checked_on_every_get_enforcer_call(self, mock_toggle):
         """
         mock_toggle.is_enabled.return_value = False
         enforcer1 = AuthzEnforcer.get_enforcer()
-        self.assertFalse(enforcer1._e.auto_save)
+        self.assertFalse(AuthzEnforcer.is_auto_save_enabled())
 
         mock_toggle.is_enabled.return_value = True
         enforcer2 = AuthzEnforcer.get_enforcer()
         self.assertIs(enforcer1, enforcer2)
-        self.assertTrue(enforcer2._e.auto_save)
+        self.assertTrue(AuthzEnforcer.is_auto_save_enabled())
 
     @patch("openedx_authz.engine.enforcer.libraries_v2_enabled")
     @override_settings(CASBIN_AUTO_LOAD_POLICY_INTERVAL=0)
@@ -701,7 +701,7 @@ def test_dummy_toggle_behavior_in_tests(self, mock_toggle):
         enforcer = AuthzEnforcer.get_enforcer()
 
         self.assertIsNotNone(enforcer)
-        self.assertTrue(enforcer._e.auto_save)
+        self.assertTrue(AuthzEnforcer.is_auto_save_enabled())
 
     @patch("openedx_authz.engine.enforcer.libraries_v2_enabled")
     @override_settings(CASBIN_AUTO_LOAD_POLICY_INTERVAL=0)
@@ -719,12 +719,12 @@ def test_auto_save_preserved_with_interval_zero(self, mock_toggle):
 
         enforcer = AuthzEnforcer.get_enforcer()
         enforcer.enable_auto_save(True)
-        self.assertTrue(enforcer._e.auto_save)
+        self.assertTrue(AuthzEnforcer.is_auto_save_enabled())
 
         # Call get_enforcer() again - should not disable auto-save
         enforcer2 = AuthzEnforcer.get_enforcer()
         self.assertIs(enforcer, enforcer2)
-        self.assertTrue(enforcer2._e.auto_save)
+        self.assertTrue(AuthzEnforcer.is_auto_save_enabled())
 
     @patch("openedx_authz.engine.enforcer.libraries_v2_enabled")
     @override_settings(CASBIN_AUTO_LOAD_POLICY_INTERVAL=0)
@@ -767,7 +767,7 @@ def test_auto_save_disabled_explicitly(self, mock_toggle):
 
         enforcer = AuthzEnforcer.get_enforcer()
 
-        self.assertFalse(enforcer._e.auto_save)
+        self.assertFalse(AuthzEnforcer.is_auto_save_enabled())
         self.assertFalse(enforcer.is_auto_loading_running())
 
     @patch("openedx_authz.engine.enforcer.libraries_v2_enabled")
@@ -817,9 +817,9 @@ def test_multiple_get_enforcer_calls_preserve_auto_save(self, mock_toggle):
         # First call
         enforcer1 = AuthzEnforcer.get_enforcer()
         enforcer1.enable_auto_save(True)
-        self.assertTrue(enforcer1._e.auto_save)
+        self.assertTrue(AuthzEnforcer.is_auto_save_enabled())
 
         # Multiple subsequent calls
         for _ in range(5):
-            enforcer = AuthzEnforcer.get_enforcer()
-            self.assertTrue(enforcer._e.auto_save)
+            AuthzEnforcer.get_enforcer()
+            self.assertTrue(AuthzEnforcer.is_auto_save_enabled())