#86drpndk1 - Change how to add a method to the permissions #1240
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| elif ((isinstance(methods, tuple) and (any(not isinstance(method, str) for method in methods) or len(methods) == 0)) or | ||
| (isinstance(methods, list) and (any(not isinstance(method, str) for method in methods) or len(methods) == 0))): |
There was a problem hiding this comment.
the validation is the same for both tuple and list right?
you could just wright:
| elif ((isinstance(methods, tuple) and (any(not isinstance(method, str) for method in methods) or len(methods) == 0)) or | |
| (isinstance(methods, list) and (any(not isinstance(method, str) for method in methods) or len(methods) == 0))): | |
| elif ((isinstance(methods, (tuple, list)) and (any(not isinstance(method, str) for method in methods) or len(methods) == 0))): |
| @@ -306,7 +306,8 @@ def add_group(self, pub_key: str, signature: str): | |||
| if new_group not in self._permissions: | |||
| self._groups.append(new_group) | |||
|
|
|||
| def add_permission(self, *, contract: str = IMPORT_WILDCARD, methods: list[str, str] = IMPORT_WILDCARD): | |||
| def add_permission(self, *, contract: str = IMPORT_WILDCARD, | |||
| methods: list[str] | str | tuple = IMPORT_WILDCARD): | |||
There was a problem hiding this comment.
just an observation for tuple type annotation: you can annotate a tuple of any size like this: tuple[str, ...]
jplippi
force-pushed
the
CU-86drpndk1
branch
3 times, most recently
from
18b961e
to
1c4fe29
Compare
boa3_test/test_sc/metadata_test/MetadataInfoPermissionsMismatchedType.py
Show resolved
Hide resolved
boa3_test/test_sc/metadata_test/MetadataInfoPermissionsMismatchedType.py
Show resolved
Hide resolved
boa3_test/test_sc/metadata_test/MetadataInfoPermissionsMismatchedType.py
Show resolved
Hide resolved
| @@ -452,6 +469,33 @@ async def test_metadata_info_permissions_wildcard(self): | |||
| result, _ = await self.call('main', [], return_type=int) | |||
| self.assertEqual(result, 100_000_000) # NEO total supply | |||
|
|
|||
| async def test_metadata_info_permissions_wildcard_list_single_element(self): | |||
| path = self.get_contract_path('MetadataInfoPermissionsWildcardListSingleElement.py') | |||
There was a problem hiding this comment.
Consider adding a test case where the list or tuple contains multiple elements, not just a single element or wildcard. This will ensure that the function can handle multiple methods correctly.
| self.assertIn({"contract": "*", "methods": "*"}, manifest['permissions']) | ||
|
|
||
| async def test_metadata_info_permissions_wildcard_list(self): | ||
| path = self.get_contract_path('MetadataInfoPermissionsWildcardList.py') |
There was a problem hiding this comment.
It would be beneficial to add a negative test case where the input is not a list or tuple to ensure the function behaves as expected in such scenarios.
| self.assertEqual(len(manifest['permissions']), 1) | ||
| self.assertIn({"contract": "*", "methods": "*"}, manifest['permissions']) | ||
|
|
||
| async def test_metadata_info_permissions_wildcard_tuple(self): |
There was a problem hiding this comment.
Please add a test case where the list or tuple contains a mix of specific methods and a wildcard. This will help to verify that the function can correctly handle such cases.
boa3_test/test_sc/metadata_test/MetadataInfoPermissionsMismatchedType.py
Show resolved
Hide resolved
boa3_test/test_sc/metadata_test/MetadataInfoPermissionsMismatchedType.py
Show resolved
Hide resolved
boa3_test/test_sc/metadata_test/MetadataInfoPermissionsMismatchedType.py
Show resolved
Hide resolved
| self.assertIsInstance(manifest['permissions'], list) | ||
| self.assertEqual(len(manifest['permissions']), 1) | ||
| self.assertIn({"contract": "*", "methods": ['total_supply', 'onNEP17Payment']}, manifest['permissions']) | ||
|
|
||
| async def test_metadata_info_permissions_wildcard(self): | ||
| path = self.get_contract_path('MetadataInfoPermissionsWildcard.py') | ||
| output, manifest = self.compile_and_save(path) |
There was a problem hiding this comment.
Consider handling exceptions that may occur during the execution of compile_and_save method.
| @@ -452,6 +469,33 @@ async def test_metadata_info_permissions_wildcard(self): | |||
| result, _ = await self.call('main', [], return_type=int) | |||
| self.assertEqual(result, 100_000_000) # NEO total supply | |||
|
|
|||
| async def test_metadata_info_permissions_wildcard_list_single_element(self): | |||
| path = self.get_contract_path('MetadataInfoPermissionsWildcardListSingleElement.py') | |||
There was a problem hiding this comment.
Consider adding a docstring to the test_metadata_info_permissions_wildcard_list_single_element method to explain what it tests.
| self.assertEqual(len(manifest['permissions']), 1) | ||
| self.assertIn({"contract": "*", "methods": "*"}, manifest['permissions']) | ||
|
|
||
| async def test_metadata_info_permissions_wildcard_list(self): |
There was a problem hiding this comment.
Consider adding a docstring to the test_metadata_info_permissions_wildcard_list method to explain what it tests.
| self.assertEqual(len(manifest['permissions']), 1) | ||
| self.assertIn({"contract": "*", "methods": "*"}, manifest['permissions']) | ||
|
|
||
| async def test_metadata_info_permissions_wildcard_tuple(self): |
There was a problem hiding this comment.
Consider adding a docstring to the test_metadata_info_permissions_wildcard_tuple method to explain what it tests.
Summary or solution description
This issue changes the
add_permissionfunction fromNeoMetadatato accept methods in the form of lists and tuples, including handling of wildcard when present in such iterables.