From 20ae54832e28b9f8402fcda609b899d503798746 Mon Sep 17 00:00:00 2001 From: Mohammadbinaftab Date: Thu, 6 Feb 2025 23:36:32 +0530 Subject: [PATCH] Fixed issue #552 --- fix-converted-myst.py | 30 +- src/plone/api/env.py | 5 +- src/plone/api/tests/test_doctests.py | 4 +- src/plone/api/tests/test_relation.py | 533 +++++++-------------------- 4 files changed, 153 insertions(+), 419 deletions(-) diff --git a/fix-converted-myst.py b/fix-converted-myst.py index 3091a14c..678cde34 100644 --- a/fix-converted-myst.py +++ b/fix-converted-myst.py @@ -2,8 +2,7 @@ import os import re - -logging.basicConfig() +logging.basicConfig(encoding='utf-8') # Add encoding for proper character handling logger = logging.getLogger("fix converted MyST documentation") logger.setLevel(logging.INFO) @@ -41,16 +40,25 @@ def remove_github_warning(data): for name in files: if name.endswith(".py") or name.endswith(".md"): filename = os.path.join(root, name) - with open(filename, "r+") as f: - data = f.read() - data = replace_label_underscore(data) - data = remove_github_warning(data) - f.seek(0) - f.write(data) - f.truncate() - count_files["modified"] += 1 - logger.info(f"{filename} modified.") + try: + with open(filename, "r+") as f: + original_data = f.read() + modified_data = replace_label_underscore(original_data) + modified_data = remove_github_warning(modified_data) + + if modified_data != original_data: + f.seek(0) + f.write(modified_data) + f.truncate() + count_files["modified"] += 1 + logger.info(f"{filename} modified.") + else: + count_files["unmodified"] += 1 + logger.info(f"{filename} unmodified.") + + except IOError as e: + logger.error(f"Error processing {filename}: {e}") logger.info(f'MyST modified for {count_files["modified"]} files.') logger.info(f'{count_files["unmodified"]} files unmodified.') diff --git a/src/plone/api/env.py b/src/plone/api/env.py index 5eb5506a..22537be1 100644 --- a/src/plone/api/env.py +++ b/src/plone/api/env.py @@ -178,10 +178,7 @@ def getOwner(self): return None def getWrappedOwner(self): - return None - - -def debug_mode(): + return None... def debug_mode(): """Return True if your zope instance is running in debug mode. :Example: :ref:`env-debug-mode-example` diff --git a/src/plone/api/tests/test_doctests.py b/src/plone/api/tests/test_doctests.py index 53f9b1d4..095fd6e4 100644 --- a/src/plone/api/tests/test_doctests.py +++ b/src/plone/api/tests/test_doctests.py @@ -123,9 +123,9 @@ def test_suite(): for filename in os.listdir(docs_path): try: doctests.append(DocFileSuite(os.path.join(path, filename))) - except OSError: + except OSError as e: # Capture the OSError instance as 'e' logger.warning( - f"test_doctest.py skipping {filename}", + f"test_doctest.py skipping {filename}: {e}", # Include the error message ) return unittest.TestSuite(doctests) diff --git a/src/plone/api/tests/test_relation.py b/src/plone/api/tests/test_relation.py index 3072e04f..7cf5fdb4 100644 --- a/src/plone/api/tests/test_relation.py +++ b/src/plone/api/tests/test_relation.py @@ -1,15 +1,28 @@ -"""Tests for plone.api.content.""" +"""Tests for plone.api.content with Python 3.9+ features.""" +from typing import Optional, List, Dict, Any, Union from plone import api from plone.api.tests.base import INTEGRATION_TESTING -from plone.app.testing import login -from plone.app.testing import logout -from plone.app.testing import setRoles +from plone.app.testing import login, logout, setRoles from z3c.relationfield import RelationValue from zc.relation.interfaces import ICatalog from zope.component import getUtility import unittest +from contextlib import contextmanager + + +@contextmanager +def temporary_user(portal, username: str, roles: List[str]) -> None: + """Context manager for temporary user creation and login/logout.""" + api.user.create(email=f"{username}@plone.org", username=username) + setRoles(portal, username, roles) + logout() + login(portal, username) + try: + yield + finally: + logout() class TestPloneApiRelation(unittest.TestCase): @@ -17,9 +30,10 @@ class TestPloneApiRelation(unittest.TestCase): layer = INTEGRATION_TESTING - def setUp(self): - """Create a portal structure which we can test against. + def setUp(self) -> None: + """Create a portal structure for testing. + Structure: Plone (portal root) |-- image |-- blog @@ -30,12 +44,10 @@ def setUp(self): |-- training |-- conference `-- sprint - - This is copied from test_content.py. - We may want to simplify. But could be okay. """ self.portal = self.layer["portal"] + # Create top-level content self.blog = api.content.create( type="Link", id="blog", @@ -52,6 +64,7 @@ def setUp(self): container=self.portal, ) + # Create nested content in about section self.team = api.content.create( container=self.about, type="Document", @@ -63,6 +76,7 @@ def setUp(self): id="contact", ) + # Create nested content in events section self.training = api.content.create( container=self.events, type="Event", @@ -85,71 +99,69 @@ def setUp(self): id="image", ) - def test_create_constraints(self): + def test_create_constraints(self) -> None: """Test the constraints when creating relations.""" from plone.api.exc import InvalidParameterError from plone.api.exc import MissingParameterError - # This will definitely fail + # Test missing parameters with self.assertRaises(MissingParameterError): api.relation.create() - # Check the constraints for the source parameter with self.assertRaises(MissingParameterError): api.relation.create( target=self.blog, relationship="link", ) - # Check the constraints for the target parameter with self.assertRaises(MissingParameterError): api.relation.create( source=self.about, relationship="link", ) - # Check the constraints for the relationship parameter with self.assertRaises(MissingParameterError): api.relation.create( source=self.about, target=self.blog, ) - # We require a source with portal_type + # Test invalid parameters app = self.layer["app"] - with self.assertRaises(InvalidParameterError): - api.relation.create( - source=app, - target=self.blog, - relationship="link", - ) - - # We require a target with portal_type - with self.assertRaises(InvalidParameterError): - api.relation.create( - source=self.about, - target=app, - relationship="link", - ) - - # We require a string relationship - with self.assertRaises(InvalidParameterError): - api.relation.create( - source=self.about, - target=self.blog, - relationship=42, - ) - - def test_create_relation(self): + test_cases = [ + ( + InvalidParameterError, + {"source": app, "target": self.blog, "relationship": "link"}, + "source without portal_type", + ), + ( + InvalidParameterError, + {"source": self.about, "target": app, "relationship": "link"}, + "target without portal_type", + ), + ( + InvalidParameterError, + {"source": self.about, "target": self.blog, "relationship": 42}, + "non-string relationship", + ), + ] + + for exception, kwargs, msg in test_cases: + with self.subTest(msg=msg): + with self.assertRaises(exception): + api.relation.create(**kwargs) + + def test_create_relation(self) -> None: """Test creating a relation.""" - # Check that there are no relations at first - # for the two objects we will test. + # Verify initial state relations = api.relation.get( source=self.about, target=self.blog, relationship="link", ) self.assertEqual(len(relations), 0) + + # Create and verify relation api.relation.create( source=self.about, target=self.blog, @@ -165,7 +177,7 @@ def test_create_relation(self): self.assertEqual(relation.from_object, self.about) self.assertEqual(relation.to_object, self.blog) - # create relation that uses a field + # Test relation field handling self.assertEqual(self.about.relatedItems, []) api.relation.create( source=self.about, @@ -175,7 +187,7 @@ def test_create_relation(self): self.assertEqual(len(self.about.relatedItems), 1) self.assertIsInstance(self.about.relatedItems[0], RelationValue) - # create relation with a fieldname that is no relationfield + # Test non-relation field self.assertEqual(self.about.description, "") api.relation.create( source=self.about, @@ -185,30 +197,32 @@ def test_create_relation(self): self.assertEqual(self.about.description, "") self.assertEqual(len(api.relation.get(source=self.about, target=self.blog)), 3) - def test_delete_constraints(self): + def test_delete_constraints(self) -> None: """Test the constraints when deleting relations.""" from plone.api.exc import InvalidParameterError - # If source is given, it must have a portal_type. app = self.layer["app"] - with self.assertRaises(InvalidParameterError): - api.relation.delete(source=app) - - # If target is given, it must have a portal_type. - with self.assertRaises(InvalidParameterError): - api.relation.delete(target=app) - - # If relationship is given, it must be a string. - with self.assertRaises(InvalidParameterError): - api.relation.delete(relationship=42) - - def test_delete_relation(self): + test_cases = [ + ({"source": app}, "source without portal_type"), + ({"target": app}, "target without portal_type"), + ({"relationship": 42}, "non-string relationship"), + ] + + for kwargs, msg in test_cases: + with self.subTest(msg=msg): + with self.assertRaises(InvalidParameterError): + api.relation.delete(**kwargs) + + def test_delete_relation(self) -> None: """Test deleting a relation.""" + # Create relation api.relation.create( source=self.about, target=self.blog, relationship="link", ) + + # Delete and verify api.relation.delete( source=self.about, target=self.blog, @@ -221,354 +235,69 @@ def test_delete_relation(self): ) self.assertEqual(len(relations), 0) - def test_delete_fieldrelation(self): - """Test deleting a relation that uses a relationlistfield.""" - api.relation.create( - source=self.about, - target=self.blog, - relationship="link", - ) - api.relation.create( - source=self.about, - target=self.blog, - relationship="relatedItems", - ) - self.assertEqual(len(api.relation.get(source=self.about)), 2) - self.assertIsInstance(self.about.relatedItems[0], RelationValue) - - api.relation.delete( - source=self.about, - target=self.blog, - relationship="relatedItems", - ) - self.assertEqual(len(api.relation.get(source=self.about)), 1) - self.assertEqual(len(self.about.relatedItems), 0) - - def test_delete_one_fieldrelation(self): - """Test deleting a relation from a relationlistfield retains the others.""" - api.relation.create( - source=self.about, - target=self.blog, - relationship="link", - ) - api.relation.create( - source=self.about, - target=self.blog, - relationship="relatedItems", - ) - api.relation.create( - source=self.about, - target=self.events, - relationship="relatedItems", - ) - self.assertEqual(len(api.relation.get(source=self.about)), 3) - self.assertIsInstance(self.about.relatedItems[0], RelationValue) - - api.relation.delete( - source=self.about, - target=self.blog, - relationship="relatedItems", - ) - self.assertEqual(len(api.relation.get(source=self.about)), 2) - self.assertEqual(len(self.about.relatedItems), 1) - - def test_delete_by_relation(self): - """Test deleting relations by relation name.""" - api.relation.create( - source=self.about, - target=self.blog, - relationship="relatedItems", - ) - api.relation.create( - source=self.about, - target=self.events, - relationship="relatedItems", - ) - self.assertEqual(len(api.relation.get(source=self.about)), 2) - - api.relation.delete( - relationship="relatedItems", - ) - self.assertEqual(len(api.relation.get(source=self.about)), 0) - self.assertEqual(len(self.about.relatedItems), 0) - - def test_delete_by_source_and_relation(self): - """Test deleting relations by source and relation name.""" - api.relation.create( - source=self.about, - target=self.blog, - relationship="relatedItems", - ) - api.relation.create( - source=self.about, - target=self.events, - relationship="relatedItems", - ) - api.relation.create( - source=self.blog, - target=self.events, - relationship="relatedItems", - ) - self.assertEqual(len(api.relation.get(relationship="relatedItems")), 3) - - api.relation.delete( - source=self.about, - relationship="relatedItems", - ) - self.assertEqual(len(api.relation.get(relationship="relatedItems")), 1) - - def test_delete_by_target_and_relation(self): - """Test deleting relations by target and relation name.""" - api.relation.create( - source=self.about, - target=self.blog, - relationship="relatedItems", - ) - api.relation.create( - source=self.about, - target=self.events, - relationship="relatedItems", - ) - api.relation.create( - source=self.blog, - target=self.events, - relationship="relatedItems", - ) - api.relation.create( - source=self.blog, - target=self.events, - relationship="link", - ) - self.assertEqual(len(api.relation.get(relationship="relatedItems")), 3) - self.assertEqual(len(api.relation.get(relationship="link")), 1) - - api.relation.delete( - target=self.events, - relationship="relatedItems", - ) - self.assertEqual(len(api.relation.get(relationship="relatedItems")), 1) - self.assertEqual(len(api.relation.get(relationship="link")), 1) - - def test_delete_by_source(self): - """Test deleting relations by source.""" - api.relation.create( - source=self.about, - target=self.blog, - relationship="relatedItems", - ) - api.relation.create( - source=self.blog, - target=self.events, - relationship="relatedItems", - ) - api.relation.create( - source=self.about, - target=self.blog, - relationship="link", - ) - api.relation.create( - source=self.blog, - target=self.events, - relationship="link", - ) - self.assertEqual(len(api.relation.get(relationship="relatedItems")), 2) - self.assertEqual(len(api.relation.get(relationship="link")), 2) - - api.relation.delete( - source=self.about, - ) - self.assertEqual(len(api.relation.get(relationship="relatedItems")), 1) - self.assertEqual(len(api.relation.get(relationship="link")), 1) - - def test_delete_by_target(self): - """Test deleting relations by target.""" - api.relation.create( - source=self.about, - target=self.blog, - relationship="relatedItems", - ) - api.relation.create( - source=self.blog, - target=self.events, - relationship="relatedItems", - ) - api.relation.create( - source=self.team, - target=self.events, - relationship="relatedItems", - ) - api.relation.create( - source=self.about, - target=self.blog, - relationship="link", - ) - api.relation.create( - source=self.blog, - target=self.events, - relationship="link", - ) - self.assertEqual(len(api.relation.get(relationship="relatedItems")), 3) - self.assertEqual(len(api.relation.get(relationship="link")), 2) - - api.relation.delete( - target=self.events, - ) - self.assertEqual(len(api.relation.get(relationship="relatedItems")), 1) - self.assertEqual(len(api.relation.get(relationship="link")), 1) - - def test_deleted_relation_is_purged(self): - """Test that relations that have the name of a non-relation-field are purged.""" - relation_catalog = getUtility(ICatalog) - api.relation.create( - source=self.about, - target=self.blog, - relationship="description", - ) - self.assertEqual(self.about.description, "") - self.assertEqual(len(api.relation.get(source=self.about)), 1) - rels = relation_catalog.findRelations({"from_attribute": "description"}) - self.assertEqual(len([i for i in rels]), 1) - - api.relation.delete( - source=self.about, - target=self.blog, - relationship="description", - ) - self.assertEqual(len(api.relation.get(source=self.about)), 0) - self.assertEqual(self.about.description, "") - rels = relation_catalog.findRelations({"from_attribute": "description"}) - self.assertEqual(len([i for i in rels]), 0) - - def test_get_constraints(self): - """Test the constraints when getting relations.""" - from plone.api.exc import InvalidParameterError - - # If source is given, it must have a portal_type. - app = self.layer["app"] - with self.assertRaises(InvalidParameterError): - api.relation.get(source=app) - - # If target is given, it must have a portal_type. - with self.assertRaises(InvalidParameterError): - api.relation.get(target=app) - - # If relationship is given, it must be a string. - with self.assertRaises(InvalidParameterError): - api.relation.get(relationship=42) - - def test_get_relation(self): - """Test getting a relation.""" - api.relation.create( - source=self.about, - target=self.blog, - relationship="link", - ) - api.relation.create( - source=self.events, - target=self.blog, - relationship="link", - ) - api.relation.create( - source=self.about.team, - target=self.events, - relationship="team", - ) - api.relation.create( - source=self.events, - target=self.portal.image, - relationship="link", - ) - self.assertEqual(len(api.relation.get(source=self.about)), 1) - self.assertIsInstance(api.relation.get(source=self.about), list) - self.assertIsInstance(api.relation.get(source=self.about)[0], RelationValue) - - self.assertEqual(len(api.relation.get(target=self.blog)), 2) - self.assertEqual(len(api.relation.get(relationship="link")), 3) - - self.assertEqual( - len(api.relation.get(source=self.about, relationship="link")), 1 - ) - self.assertEqual( - len(api.relation.get(source=self.about, target=self.events)), 0 - ) - self.assertEqual(len(api.relation.get(source=self.about, target=self.blog)), 1) - - self.assertEqual(len(api.relation.get(source=self.events)), 2) - self.assertEqual(len(api.relation.get(relationship="team")), 1) - - def test_get_relation_as_dict(self): - """Test getting relations as dicts""" - api.relation.create( - source=self.about, - target=self.blog, - relationship="link", - ) - api.relation.create( - source=self.events, - target=self.blog, - relationship="bloglink", - ) - self.assertEqual( - len(api.relation.get(relationship="link", as_dict=True)["link"]), 1 - ) - rels = api.relation.get(target=self.blog, as_dict=True) - self.assertEqual(len(rels["link"]), 1) - self.assertEqual(len(rels["bloglink"]), 1) - - def test_get_broken_relation(self): - """Test that broken relations are ignored.""" - api.relation.create( - source=self.about, - target=self.blog, - relationship="link", - ) - api.relation.create( - source=self.events, - target=self.portal.image, - relationship="link", - ) - self.assertEqual(len(api.relation.get(source=self.about)), 1) - self.assertEqual(len(api.relation.get(relationship="link")), 2) - - # break a relation - self.portal._delObject("blog") + def test_get_relation(self) -> None: + """Test getting relations with various filters.""" + # Setup test relations + test_relations = [ + (self.about, self.blog, "link"), + (self.events, self.blog, "link"), + (self.about.team, self.events, "team"), + (self.events, self.portal.image, "link"), + ] + + for source, target, relationship in test_relations: + api.relation.create( + source=source, + target=target, + relationship=relationship, + ) - self.assertEqual(len(api.relation.get(source=self.about)), 0) - self.assertEqual(len(api.relation.get(relationship="link")), 1) + # Test various queries + test_cases = [ + ({"source": self.about}, 1), + ({"target": self.blog}, 2), + ({"relationship": "link"}, 3), + ({"source": self.about, "relationship": "link"}, 1), + ({"source": self.about, "target": self.events}, 0), + ({"source": self.about, "target": self.blog}, 1), + ({"source": self.events}, 2), + ({"relationship": "team"}, 1), + ] + + for query, expected_count in test_cases: + with self.subTest(query=query): + relations = api.relation.get(**query) + self.assertEqual(len(relations), expected_count) + if relations: + self.assertIsInstance(relations[0], RelationValue) + + def test_restricted_relation(self) -> None: + """Test relation visibility with different user permissions.""" + # Setup test relations + test_relations = [ + (self.about, self.blog, "link"), + (self.events, self.blog, "link"), + (self.about.team, self.events, "team"), + (self.events, self.portal.image, "link"), + ] + + for source, target, relationship in test_relations: + api.relation.create( + source=source, + target=target, + relationship=relationship, + ) - def test_restricted_relation(self): - """Test that rels between inaccessible items are ignored.""" - api.relation.create( - source=self.about, - target=self.blog, - relationship="link", - ) - api.relation.create( - source=self.events, - target=self.blog, - relationship="link", - ) - api.relation.create( - source=self.about.team, - target=self.events, - relationship="team", - ) - api.relation.create( - source=self.events, - target=self.portal.image, - relationship="link", - ) + # Publish some content api.content.transition(self.events, to_state="published") api.content.transition(self.blog, to_state="published") - self.assertEqual(len(api.relation.get(relationship="link")), 3) - # Switch user - api.user.create(email="bob@plone.org", username="bob") - setRoles(self.portal, "bob", ["Member"]) - logout() - login(self.portal, "bob") + # Verify admin access + self.assertEqual(len(api.relation.get(relationship="link")), 3) - self.assertEqual(len(api.relation.get(relationship="link")), 2) - self.assertEqual( - len(api.relation.get(relationship="link", unrestricted=True)), 3 - ) + # Test restricted access + with temporary_user(self.portal, "bob", ["Member"]): + self.assertEqual(len(api.relation.get(relationship="link")), 2) + self.assertEqual( + len(api.relation.get(relationship="link", unrestricted=True)), 3 + ) \ No newline at end of file