From ef8b75880f99d43bee5f8e88a712b945ec921c0e Mon Sep 17 00:00:00 2001 From: Coki Date: Sun, 24 Nov 2024 17:28:28 +0800 Subject: [PATCH 1/5] feat: optimize filtered file adapter policy loading --- .../persist/adapters/filtered_file_adapter.py | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/casbin/persist/adapters/filtered_file_adapter.py b/casbin/persist/adapters/filtered_file_adapter.py index eeb3a6c..59ad338 100644 --- a/casbin/persist/adapters/filtered_file_adapter.py +++ b/casbin/persist/adapters/filtered_file_adapter.py @@ -58,19 +58,17 @@ def load_filtered_policy(self, model, filter): self.load_filtered_policy_file(model, filter_value, persist.load_policy_line) self.filtered = True - def load_filtered_policy_file(self, model, filter, hanlder): + def load_filtered_policy_file(self, model, filter, handler): with open(self._file_path, "rb") as file: - while True: - line = file.readline() + for line in file: line = line.decode().strip() - if line == "\n": + if not line or line == "\n": continue - if not line: - break + if filter_line(line, filter): continue - hanlder(line, model) + handler(line, model) # is_filtered returns true if the loaded policy has been filtered. def is_filtered(self): @@ -92,20 +90,22 @@ def filter_line(line, filter): return True filter_slice = [] - if p[0].strip() == "p": - filter_slice = filter[0] - elif p[0].strip() == "g": + if p[0].strip() == "g": + if not filter[1] or all(not x.strip() for x in filter[1]): + return False filter_slice = filter[1] + elif p[0].strip() == "p": + filter_slice = filter[0] + return filter_words(p, filter_slice) - def filter_words(line, filter): if len(line) < len(filter) + 1: return True skip_line = False for i, v in enumerate(filter): - if len(v) > 0 and (v.strip() != line[i + 1].strip()): + if v and v.strip() and (v.strip() != line[i + 1].strip()): skip_line = True break - + return skip_line From c1ea804bbf388df46d338cf40716a95c707f6b20 Mon Sep 17 00:00:00 2001 From: Coki Date: Sun, 24 Nov 2024 19:22:08 +0800 Subject: [PATCH 2/5] style: standardize whitespace and formatting in filtered_file_adapter.py --- casbin/persist/adapters/filtered_file_adapter.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/casbin/persist/adapters/filtered_file_adapter.py b/casbin/persist/adapters/filtered_file_adapter.py index 59ad338..e12ba4a 100644 --- a/casbin/persist/adapters/filtered_file_adapter.py +++ b/casbin/persist/adapters/filtered_file_adapter.py @@ -64,7 +64,7 @@ def load_filtered_policy_file(self, model, filter, handler): line = line.decode().strip() if not line or line == "\n": continue - + if filter_line(line, filter): continue @@ -96,9 +96,10 @@ def filter_line(line, filter): filter_slice = filter[1] elif p[0].strip() == "p": filter_slice = filter[0] - + return filter_words(p, filter_slice) + def filter_words(line, filter): if len(line) < len(filter) + 1: return True @@ -107,5 +108,5 @@ def filter_words(line, filter): if v and v.strip() and (v.strip() != line[i + 1].strip()): skip_line = True break - + return skip_line From 2fb4770c31e536b8a9a7eb47e8dd6033da3cdb64 Mon Sep 17 00:00:00 2001 From: Coki Date: Mon, 25 Nov 2024 14:27:57 +0800 Subject: [PATCH 3/5] feat: add test --- .../persist/adapters/filtered_file_adapter.py | 5 + tests/test_filter.py | 167 ++++++++++++++++++ 2 files changed, 172 insertions(+) diff --git a/casbin/persist/adapters/filtered_file_adapter.py b/casbin/persist/adapters/filtered_file_adapter.py index e12ba4a..b0e31cb 100644 --- a/casbin/persist/adapters/filtered_file_adapter.py +++ b/casbin/persist/adapters/filtered_file_adapter.py @@ -52,6 +52,11 @@ def load_filtered_policy(self, model, filter): try: filter_value = [filter.__dict__["P"]] + [filter.__dict__["G"]] + is_empty_filter = all(not f for f in filter_value) or all( + all(not x.strip() for x in f) if f else True for f in filter_value + ) + if is_empty_filter: + return self.load_policy(model) except: raise RuntimeError("invalid filter type") diff --git a/tests/test_filter.py b/tests/test_filter.py index 536505f..1d64e67 100644 --- a/tests/test_filter.py +++ b/tests/test_filter.py @@ -16,6 +16,10 @@ from unittest import TestCase from tests.test_enforcer import get_examples from casbin.persist.adapters import FilteredFileAdapter +from casbin.persist.adapters.filtered_file_adapter import filter_line, filter_words +import tempfile +import shutil +import os class Filter: @@ -141,3 +145,166 @@ def test_filtered_adapter_invalid_filepath(self): with self.assertRaises(RuntimeError): e.load_filtered_policy(None) + + def test_empty_filter_array(self): + """Test filter for empty array.""" + adapter = FilteredFileAdapter(get_examples("rbac_with_domains_policy.csv")) + e = casbin.Enforcer(get_examples("rbac_with_domains_model.conf"), adapter) + filter = Filter() + filter.P = [] + filter.G = [] + + e.load_filtered_policy(filter) + self.assertTrue(e.has_policy(["admin", "domain1", "data1", "read"])) + self.assertTrue(e.has_policy(["admin", "domain2", "data2", "read"])) + + def test_empty_string_filter(self): + """Test the filter for all empty strings.""" + adapter = FilteredFileAdapter(get_examples("rbac_with_domains_policy.csv")) + e = casbin.Enforcer(get_examples("rbac_with_domains_model.conf"), adapter) + filter = Filter() + filter.P = ["", "", ""] + filter.G = ["", "", ""] + + e.load_filtered_policy(filter) + self.assertTrue(e.has_policy(["admin", "domain1", "data1", "read"])) + self.assertTrue(e.has_policy(["admin", "domain2", "data2", "read"])) + + def test_mixed_empty_filter(self): + """测试混合空字符串和非空字符串的过滤器""" + adapter = FilteredFileAdapter(get_examples("rbac_with_domains_policy.csv")) + e = casbin.Enforcer(get_examples("rbac_with_domains_model.conf"), adapter) + filter = Filter() + filter.P = ["", "domain1", ""] + filter.G = ["", "", "domain1"] + + e.load_filtered_policy(filter) + self.assertTrue(e.has_policy(["admin", "domain1", "data1", "read"])) + self.assertFalse(e.has_policy(["admin", "domain2", "data2", "read"])) + + def test_nonexistent_domain_filter(self): + """Testing the filter for a non-existent domain.""" + adapter = FilteredFileAdapter(get_examples("rbac_with_domains_policy.csv")) + e = casbin.Enforcer(get_examples("rbac_with_domains_model.conf"), adapter) + filter = Filter() + filter.P = ["", "domain3"] + filter.G = ["", "", "domain3"] + + e.load_filtered_policy(filter) + self.assertFalse(e.has_policy(["admin", "domain3", "data3", "read"])) + + def test_empty_filter_array(self): + """Test filter for empty array.""" + adapter = FilteredFileAdapter(get_examples("rbac_with_domains_policy.csv")) + e = casbin.Enforcer(get_examples("rbac_with_domains_model.conf"), adapter) + filter = Filter() + filter.P = [] + filter.G = [] + + try: + e.load_filtered_policy(filter) + except: + raise RuntimeError("unexpected error with empty filter arrays") + + self.assertFalse(e.is_filtered(), "Adapter should not be marked as filtered with empty filters") + + self.assertTrue(e.has_policy(["admin", "domain1", "data1", "read"])) + self.assertTrue(e.has_policy(["admin", "domain2", "data2", "read"])) + + def test_empty_string_filter(self): + """Test the filter for all empty strings.""" + adapter = FilteredFileAdapter(get_examples("rbac_with_domains_policy.csv")) + e = casbin.Enforcer(get_examples("rbac_with_domains_model.conf"), adapter) + filter = Filter() + filter.P = ["", "", ""] + filter.G = ["", "", ""] + + try: + e.load_filtered_policy(filter) + except: + raise RuntimeError("unexpected error with empty string filters") + + self.assertFalse(e.is_filtered(), "Adapter should not be marked as filtered with empty string filters") + + try: + e.save_policy() + except: + raise RuntimeError("unexpected error in SavePolicy with empty string filters") + + self.assertTrue(e.has_policy(["admin", "domain1", "data1", "read"])) + self.assertTrue(e.has_policy(["admin", "domain2", "data2", "read"])) + + def test_mixed_empty_filter(self): + """Test the filter for mixed empty and non-empty strings.""" + adapter = FilteredFileAdapter(get_examples("rbac_with_domains_policy.csv")) + e = casbin.Enforcer(get_examples("rbac_with_domains_model.conf"), adapter) + filter = Filter() + filter.P = ["", "domain1", ""] + filter.G = ["", "", "domain1"] + + try: + e.load_filtered_policy(filter) + except: + raise RuntimeError("unexpected error with mixed empty filters") + + self.assertTrue(e.is_filtered(), "Adapter should be marked as filtered") + + with self.assertRaises(RuntimeError): + e.save_policy() + + self.assertTrue(e.has_policy(["admin", "domain1", "data1", "read"])) + self.assertFalse(e.has_policy(["admin", "domain2", "data2", "read"])) + + def test_whitespace_filter(self): + """Test the filter for all blank characters.""" + adapter = FilteredFileAdapter(get_examples("rbac_with_domains_policy.csv")) + e = casbin.Enforcer(get_examples("rbac_with_domains_model.conf"), adapter) + filter = Filter() + filter.P = [" ", " ", "\t"] + filter.G = ["\n", " ", " "] + + e.load_filtered_policy(filter) + + self.assertFalse(e.is_filtered()) + self.assertTrue(e.has_policy(["admin", "domain1", "data1", "read"])) + self.assertTrue(e.has_policy(["admin", "domain2", "data2", "read"])) + + def test_filter_line_edge_cases(self): + """Test the boundary cases of the filter_line function.""" + adapter = FilteredFileAdapter(get_examples("rbac_with_domains_policy.csv")) + + self.assertFalse(filter_line("", [[""], [""]])) + + self.assertFalse(filter_line("invalid_line", [[""], [""]])) + + self.assertFalse(filter_line("p, admin, domain1, data1, read", None)) + + def test_filter_words_edge_cases(self): + """Test the boundary cases of the filter_words function.""" + self.assertTrue(filter_words(["p"], ["filter1", "filter2"])) + + self.assertFalse(filter_words(["p", "admin", "domain1"], [])) + + line = ["admin", "domain1", "data*", "read"] + filter = ["", "", "data1", ""] + self.assertTrue(filter_words(line, filter)) + + def test_load_filtered_policy_with_comments(self): + """Test loading filtering policies with comments.""" + temp_file = tempfile.NamedTemporaryFile(delete=False) + try: + shutil.copyfile(get_examples("rbac_with_domains_policy.csv"), temp_file.name) + + with open(temp_file.name, "a") as f: + f.write("\n# This is a comment\np, admin, domain1, data3, read") + + adapter = FilteredFileAdapter(temp_file.name) + e = casbin.Enforcer(get_examples("rbac_with_domains_model.conf"), adapter) + filter = Filter() + filter.P = ["", "domain1"] + filter.G = ["", "", "domain1"] + + e.load_filtered_policy(filter) + self.assertTrue(e.has_policy(["admin", "domain1", "data3", "read"])) + finally: + os.unlink(temp_file.name) From 4cea4950418fba3d37a98ce7ced683826d87f0c7 Mon Sep 17 00:00:00 2001 From: Coki Date: Mon, 25 Nov 2024 14:32:17 +0800 Subject: [PATCH 4/5] test: improve test_load_filtered_policy_with_comments in test_filter.py --- tests/test_filter.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/tests/test_filter.py b/tests/test_filter.py index 1d64e67..d98878d 100644 --- a/tests/test_filter.py +++ b/tests/test_filter.py @@ -291,14 +291,20 @@ def test_filter_words_edge_cases(self): def test_load_filtered_policy_with_comments(self): """Test loading filtering policies with comments.""" - temp_file = tempfile.NamedTemporaryFile(delete=False) - try: - shutil.copyfile(get_examples("rbac_with_domains_policy.csv"), temp_file.name) + import tempfile + import shutil + + with tempfile.NamedTemporaryFile(mode="w+", delete=False) as temp_file: + with open(get_examples("rbac_with_domains_policy.csv"), "r") as source: + shutil.copyfileobj(source, temp_file) - with open(temp_file.name, "a") as f: - f.write("\n# This is a comment\np, admin, domain1, data3, read") + temp_file.write("\n# This is a comment\np, admin, domain1, data3, read") + temp_file.flush() - adapter = FilteredFileAdapter(temp_file.name) + temp_path = temp_file.name + + try: + adapter = FilteredFileAdapter(temp_path) e = casbin.Enforcer(get_examples("rbac_with_domains_model.conf"), adapter) filter = Filter() filter.P = ["", "domain1"] @@ -307,4 +313,7 @@ def test_load_filtered_policy_with_comments(self): e.load_filtered_policy(filter) self.assertTrue(e.has_policy(["admin", "domain1", "data3", "read"])) finally: - os.unlink(temp_file.name) + try: + os.unlink(temp_path) + except OSError: + pass From fc516d56f94b4912e7b67a07d434a022de4c4a9a Mon Sep 17 00:00:00 2001 From: Coki Date: Mon, 25 Nov 2024 20:29:43 +0800 Subject: [PATCH 5/5] test: update test description for mixed filter --- tests/test_filter.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_filter.py b/tests/test_filter.py index d98878d..0160325 100644 --- a/tests/test_filter.py +++ b/tests/test_filter.py @@ -12,14 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -import casbin +import os from unittest import TestCase +import casbin from tests.test_enforcer import get_examples from casbin.persist.adapters import FilteredFileAdapter from casbin.persist.adapters.filtered_file_adapter import filter_line, filter_words -import tempfile -import shutil -import os class Filter: @@ -171,7 +169,7 @@ def test_empty_string_filter(self): self.assertTrue(e.has_policy(["admin", "domain2", "data2", "read"])) def test_mixed_empty_filter(self): - """测试混合空字符串和非空字符串的过滤器""" + """Test the filter for mixed empty and non-empty strings.""" adapter = FilteredFileAdapter(get_examples("rbac_with_domains_policy.csv")) e = casbin.Enforcer(get_examples("rbac_with_domains_model.conf"), adapter) filter = Filter()