From f737bc658df28255e61af0ea06f10c227711871e Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 10:41:19 -0500 Subject: [PATCH 01/17] add Permissions.delete() method --- src/posit/connect/permissions.py | 80 +++++++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index c5c9a268..9dca01b4 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -2,12 +2,16 @@ from __future__ import annotations -from typing import List, overload +from typing import TYPE_CHECKING, List, overload from requests.sessions import Session as Session from .resources import Resource, ResourceParameters, Resources +if TYPE_CHECKING: + from .groups import Group + from .users import User + class Permission(Resource): def delete(self) -> None: @@ -137,3 +141,77 @@ def get(self, uid: str) -> Permission: url = self.params.url + path response = self.params.session.get(url) return Permission(self.params, **response.json()) + + def delete(self, *permissions: str | Group | User | Permission) -> list[Permission]: + """Delete permissions. + + Removes all provided permissions from the content item's permissions. + + Parameters + ---------- + *permissions : str | Group | User | Permission + The content item permissions to remove. If a `str` is received, it is compared agains + the Permissions' `principal_gruid`. If a `Group`, `User`, or `Permission` is received, + the `guid` is used and compared against the Permissions' `principal_guid`. If a + `Permission` is received, the `principal_guid` is used. + + Returns + ------- + list[Permission] + The deleted permissions. + + Examples + -------- + ```python + from posit import connect + + #### User-defined inputs #### + # 1. specify the guid for the content item + content_guid = "CONTENT_GUID_HERE" + # 2. specify either the principal_guid or group name prefix + principal_guid = "USER_OR_GROUP_GUID_HERE" + group_name_prefix = "GROUP_NAME_PREFIX_HERE" + ############################ + + client = connect.Client() + + # Remove a single permission by principal_guid + client.content.get(content_guid).permissions.delete(principal_guid) + + # Remove by user (if principal_guid is a user) + user = client.users.get(principal_guid) + client.content.get(content_guid).permissions.delete(user) + + # Remove by group (if principal_guid is a group) + group = client.groups.get(principal_guid) + client.content.get(content_guid).permissions.delete(group) + + # Remove all groups with a matching prefix name + groups = client.groups.find(prefix=group_name_prefix) + client.content.get(content_guid).permissions.delete(*groups) + ``` + """ + principal_guids: set[str] = set() + + for arg in permissions: + if isinstance(arg, str): + principal_guid = arg + elif isinstance(arg, Group): + principal_guid: str = arg["guid"] + elif isinstance(arg, User): + principal_guid: str = arg["guid"] + elif isinstance(arg, Permission): + principal_guid: str = arg["principal_guid"] + else: + raise TypeError( + f"delete() expected argument type 'str', 'Group', 'Permission' or 'User', but got '{type(arg).__name__}'", + ) + principal_guids.add(principal_guid) + + deleted_permissions: list[Permission] = [] + for permission in self.find(): + if permission["principal_guid"] in principal_guids: + permission.delete() + deleted_permissions.append(permission) + + return deleted_permissions From e498165df499e9bb8522f144c69a959cb27d08ab Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 11:00:04 -0500 Subject: [PATCH 02/17] add unit tests for permission --- src/posit/connect/permissions.py | 3 ++ tests/posit/connect/api.py | 8 ++-- tests/posit/connect/test_permissions.py | 54 +++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index 9dca01b4..39edf65c 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -191,6 +191,9 @@ def delete(self, *permissions: str | Group | User | Permission) -> list[Permissi client.content.get(content_guid).permissions.delete(*groups) ``` """ + from .groups import Group + from .users import User + principal_guids: set[str] = set() for arg in permissions: diff --git a/tests/posit/connect/api.py b/tests/posit/connect/api.py index de2651b0..06b5f6cc 100644 --- a/tests/posit/connect/api.py +++ b/tests/posit/connect/api.py @@ -2,10 +2,8 @@ import pyjson5 as json -from posit.connect._json import Jsonifiable, JsonifiableDict, JsonifiableList - -def load_mock(path: str) -> Jsonifiable: +def load_mock(path: str): """ Load mock data from a file. @@ -33,13 +31,13 @@ def load_mock(path: str) -> Jsonifiable: return json.loads((Path(__file__).parent / "__api__" / path).read_text()) -def load_mock_dict(path: str) -> JsonifiableDict: +def load_mock_dict(path: str) -> dict: result = load_mock(path) assert isinstance(result, dict) return result -def load_mock_list(path: str) -> JsonifiableList: +def load_mock_list(path: str) -> list: result = load_mock(path) assert isinstance(result, list) return result diff --git a/tests/posit/connect/test_permissions.py b/tests/posit/connect/test_permissions.py index 0f3a390a..ba262611 100644 --- a/tests/posit/connect/test_permissions.py +++ b/tests/posit/connect/test_permissions.py @@ -262,3 +262,57 @@ def test(self): # assert assert permission == fake_permission + + +class TestPermissionsDelete: + @responses.activate + def test_by_guid(self): + # data + uid = "94" + content_guid = "f2f37341-e21d-3d80-c698-a935ad614066" + fake_permissions = load_mock_list(f"v1/content/{content_guid}/permissions.json") + fake_followup_permissions = fake_permissions.copy() + fake_followup_permissions.pop(0) + fake_permission = load_mock_dict(f"v1/content/{content_guid}/permissions/{uid}.json") + + # behavior + + # Used in internal for-loop + mock_permissions_get = [ + responses.get( + f"https://connect.example/__api__/v1/content/{content_guid}/permissions", + json=fake_permissions, + ), + responses.get( + f"https://connect.example/__api__/v1/content/{content_guid}/permissions", + json=fake_followup_permissions, + ), + ] + # + mock_permission_delete = responses.delete( + f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{uid}", + ) + + # setup + params = ResourceParameters(requests.Session(), Url("https://connect.example/__api__")) + permissions = Permissions(params, content_guid=content_guid) + + # invoke + print(fake_permission) + deleted_permissions = permissions.delete(fake_permission["principal_guid"]) + + assert mock_permissions_get[0].call_count == 1 + assert mock_permissions_get[1].call_count == 0 + assert mock_permission_delete.call_count == 1 + assert len(deleted_permissions) == 1 + assert deleted_permissions[0] == fake_permission + + # invoking again is a no-op + deleted_permissions = permissions.delete(fake_permission["principal_guid"]) + + assert mock_permissions_get[0].call_count == 1 + assert mock_permissions_get[1].call_count == 1 + assert mock_permission_delete.call_count == 1 + assert len(deleted_permissions) == 0 + + # TODO-barret- delete by user, group, permission From 32e50240c2b9b71ed6159b7214157521ab28bfc0 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 12:24:20 -0500 Subject: [PATCH 03/17] Rename `Permission.delete` to `Permission.destroy` --- src/posit/connect/permissions.py | 2 +- tests/posit/connect/test_permissions.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index 39edf65c..b5aa33d0 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -14,7 +14,7 @@ class Permission(Resource): - def delete(self) -> None: + def destroy(self) -> None: """Delete the permission.""" path = f"v1/content/{self['content_guid']}/permissions/{self['id']}" url = self.params.url + path diff --git a/tests/posit/connect/test_permissions.py b/tests/posit/connect/test_permissions.py index ba262611..48f8e1e7 100644 --- a/tests/posit/connect/test_permissions.py +++ b/tests/posit/connect/test_permissions.py @@ -30,7 +30,7 @@ def test(self): permission = Permission(params, **fake_permission) # invoke - permission.delete() + permission.destroy() # assert assert mock_delete.call_count == 1 From 85fdc1e7d4206e8b16a4d3f4059e5fe260384ab6 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 12:24:38 -0500 Subject: [PATCH 04/17] Rename `Permissions.delete` to `Permissions.destroy` --- src/posit/connect/permissions.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index b5aa33d0..c013006f 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -142,7 +142,7 @@ def get(self, uid: str) -> Permission: response = self.params.session.get(url) return Permission(self.params, **response.json()) - def delete(self, *permissions: str | Group | User | Permission) -> list[Permission]: + def destroy(self, *permissions: str | Group | User | Permission) -> list[Permission]: """Delete permissions. Removes all provided permissions from the content item's permissions. @@ -184,11 +184,14 @@ def delete(self, *permissions: str | Group | User | Permission) -> list[Permissi # Remove by group (if principal_guid is a group) group = client.groups.get(principal_guid) - client.content.get(content_guid).permissions.delete(group) + client.content.get(content_guid).permissions.destroy(group) # Remove all groups with a matching prefix name groups = client.groups.find(prefix=group_name_prefix) - client.content.get(content_guid).permissions.delete(*groups) + client.content.get(content_guid).permissions.destroy(*groups) + + # Confirm new permissions + client.content.get(content_guid).permissions.find() ``` """ from .groups import Group @@ -207,14 +210,14 @@ def delete(self, *permissions: str | Group | User | Permission) -> list[Permissi principal_guid: str = arg["principal_guid"] else: raise TypeError( - f"delete() expected argument type 'str', 'Group', 'Permission' or 'User', but got '{type(arg).__name__}'", + f"destroy() expected argument type 'str', 'Group', 'Permission' or 'User', but got '{type(arg).__name__}'", ) principal_guids.add(principal_guid) - deleted_permissions: list[Permission] = [] + destroyed_permissions: list[Permission] = [] for permission in self.find(): if permission["principal_guid"] in principal_guids: - permission.delete() - deleted_permissions.append(permission) + permission.destroy() + destroyed_permissions.append(permission) - return deleted_permissions + return destroyed_permissions From 1b503828f47364d2679f06c511e36ebcc35afddb Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 12:25:08 -0500 Subject: [PATCH 05/17] Update test with destroyinng user, group, and permission --- tests/posit/connect/test_permissions.py | 55 ++++++++++++++++++------- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/tests/posit/connect/test_permissions.py b/tests/posit/connect/test_permissions.py index 48f8e1e7..9b72ecfa 100644 --- a/tests/posit/connect/test_permissions.py +++ b/tests/posit/connect/test_permissions.py @@ -1,13 +1,16 @@ import random import uuid +import pytest import requests import responses from responses import matchers +from posit.connect.groups import Group from posit.connect.permissions import Permission, Permissions from posit.connect.resources import ResourceParameters from posit.connect.urls import Url +from posit.connect.users import User from .api import load_mock, load_mock_dict, load_mock_list @@ -264,16 +267,20 @@ def test(self): assert permission == fake_permission -class TestPermissionsDelete: +class TestPermissionsDestroy: @responses.activate - def test_by_guid(self): + def test_destroy(self): # data - uid = "94" + permission_uid = "94" content_guid = "f2f37341-e21d-3d80-c698-a935ad614066" fake_permissions = load_mock_list(f"v1/content/{content_guid}/permissions.json") fake_followup_permissions = fake_permissions.copy() fake_followup_permissions.pop(0) - fake_permission = load_mock_dict(f"v1/content/{content_guid}/permissions/{uid}.json") + fake_permission = load_mock_dict( + f"v1/content/{content_guid}/permissions/{permission_uid}.json" + ) + fake_user = load_mock_dict("v1/user.json") + fake_group = load_mock_dict("v1/groups/6f300623-1e0c-48e6-a473-ddf630c0c0c3.json") # behavior @@ -288,31 +295,49 @@ def test_by_guid(self): json=fake_followup_permissions, ), ] - # + # permission delete mock_permission_delete = responses.delete( - f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{uid}", + f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{permission_uid}", ) # setup params = ResourceParameters(requests.Session(), Url("https://connect.example/__api__")) permissions = Permissions(params, content_guid=content_guid) + # (Doesn't match any permissions, but that's okay) + user_to_remove = User(params, **fake_user) + group_to_remove = Group(params, **fake_group) + permission_to_remove = Permission(params, **fake_permission) + # invoke - print(fake_permission) - deleted_permissions = permissions.delete(fake_permission["principal_guid"]) + destroyed_permission = permissions.destroy( + fake_permission["principal_guid"], + # Make sure duplicates are dropped + fake_permission["principal_guid"], + # Extract info from User, Group, Permission + user_to_remove, + group_to_remove, + permission_to_remove, + ) + + # Assert bad input value + with pytest.raises(TypeError): + permissions.destroy( + 42 # pyright: ignore[reportArgumentType] + ) + + # Assert values assert mock_permissions_get[0].call_count == 1 assert mock_permissions_get[1].call_count == 0 assert mock_permission_delete.call_count == 1 - assert len(deleted_permissions) == 1 - assert deleted_permissions[0] == fake_permission + assert len(destroyed_permission) == 1 + assert destroyed_permission[0] == fake_permission - # invoking again is a no-op - deleted_permissions = permissions.delete(fake_permission["principal_guid"]) + # Invoking again is a no-op + destroyed_permission = permissions.destroy(fake_permission["principal_guid"]) assert mock_permissions_get[0].call_count == 1 assert mock_permissions_get[1].call_count == 1 assert mock_permission_delete.call_count == 1 - assert len(deleted_permissions) == 0 - - # TODO-barret- delete by user, group, permission + assert len(destroyed_permission) == 0 From 1119e316b9687e27ce19dbb356e2500b8129effa Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 15:26:27 -0500 Subject: [PATCH 06/17] Add check for no permissions to be removed --- src/posit/connect/permissions.py | 3 +++ tests/posit/connect/test_permissions.py | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index c013006f..c68d40e5 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -197,6 +197,9 @@ def destroy(self, *permissions: str | Group | User | Permission) -> list[Permiss from .groups import Group from .users import User + if len(permissions) == 0: + raise ValueError("Expected at least one `permission` to remove") + principal_guids: set[str] = set() for arg in permissions: diff --git a/tests/posit/connect/test_permissions.py b/tests/posit/connect/test_permissions.py index 9b72ecfa..a377bede 100644 --- a/tests/posit/connect/test_permissions.py +++ b/tests/posit/connect/test_permissions.py @@ -326,6 +326,8 @@ def test_destroy(self): permissions.destroy( 42 # pyright: ignore[reportArgumentType] ) + with pytest.raises(ValueError): + permissions.destroy() # Assert values assert mock_permissions_get[0].call_count == 1 From a9fbb36d346ee5bc91fcffce7cdf8f409709eeb5 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 15:26:32 -0500 Subject: [PATCH 07/17] Create test_content_item_permissions.py --- .../connect/test_content_item_permissions.py | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 integration/tests/posit/connect/test_content_item_permissions.py diff --git a/integration/tests/posit/connect/test_content_item_permissions.py b/integration/tests/posit/connect/test_content_item_permissions.py new file mode 100644 index 00000000..2cf5badf --- /dev/null +++ b/integration/tests/posit/connect/test_content_item_permissions.py @@ -0,0 +1,86 @@ +from posit import connect +from posit.connect.content import ContentItem + + +class TestContentPermissions: + content: ContentItem + + @classmethod + def setup_class(cls): + cls.client = connect.Client() + cls.content = cls.client.content.create(name="example") + + cls.user_aron = cls.client.users.create( + username="aron", + email="aron@example.com", + password="s3cur3p@ssword", + ) + cls.user_bill = cls.client.users.create( + username="bill", + email="bill@example.com", + password="s3cur3p@ssword", + ) + + cls.group_friends = cls.client.groups.create(name="Friends") + + @classmethod + def teardown_class(cls): + cls.content.delete() + assert cls.client.content.count() == 0 + + cls.group_friends.delete() + assert cls.client.groups.count() == 0 + + # cls.user_aron.delete() + # cls.user_bill.delete() + # assert cls.client.users.count() == 0 + + def test_permissions_add_destroy(self): + assert self.client.groups.count() == 1 + assert self.client.users.count() == 3 + assert self.content.permissions.find() == [] + + # Add permissions + self.content.permissions.create( + principal_guid=self.user_aron["guid"], + principal_type="user", + role="viewer", + ) + self.content.permissions.create( + principal_guid=self.group_friends["guid"], + principal_type="group", + role="owner", + ) + + def assert_permissions_match_guids(permissions, objs_with_guid): + for permission, obj_with_guid in zip(permissions, objs_with_guid): + assert permission["principal_guid"] == obj_with_guid["guid"] + + # Prove they have been added + assert_permissions_match_guids( + self.content.permissions.find(), + [self.user_aron, self.group_friends], + ) + + # Remove permissions (and from some that isn't an owner) + destroyed_permissions = self.content.permissions.destroy(self.user_aron, self.user_bill) + assert_permissions_match_guids( + destroyed_permissions, + [self.user_aron], + ) + + # Prove they have been removed + assert_permissions_match_guids( + self.content.permissions.find(), + [self.group_friends], + ) + + # Remove the last permission + destroyed_permissions = self.content.permissions.destroy(self.group_friends) + assert_permissions_match_guids( + destroyed_permissions, + [self.group_friends], + ) + + # Prove they have been removed + assert self.content.permissions.find() == [] From 9eaf38d4511083272122c4864eca9d481141ab1e Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 15:45:24 -0500 Subject: [PATCH 08/17] Update docs --- src/posit/connect/permissions.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index c68d40e5..e423ad9c 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -143,17 +143,17 @@ def get(self, uid: str) -> Permission: return Permission(self.params, **response.json()) def destroy(self, *permissions: str | Group | User | Permission) -> list[Permission]: - """Delete permissions. + """Destroy supplied content item permissions. Removes all provided permissions from the content item's permissions. Parameters ---------- *permissions : str | Group | User | Permission - The content item permissions to remove. If a `str` is received, it is compared agains + The content item permissions to remove. If a `str` is received, it is compared against the Permissions' `principal_gruid`. If a `Group`, `User`, or `Permission` is received, the `guid` is used and compared against the Permissions' `principal_guid`. If a - `Permission` is received, the `principal_guid` is used. + `Permission` is received, the `principal_guid` is used. Note, only the associated permissions will be destroyed; Any users or groups provided will remain. Returns ------- From 8269629ce2ed62a61a28366a1794a530196d9fd3 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 15:45:46 -0500 Subject: [PATCH 09/17] Have tests play nicely with each other --- .../posit/connect/test_content_item_permissions.py | 12 ++++++------ integration/tests/posit/connect/test_users.py | 8 ++++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/integration/tests/posit/connect/test_content_item_permissions.py b/integration/tests/posit/connect/test_content_item_permissions.py index 2cf5badf..9742eaa4 100644 --- a/integration/tests/posit/connect/test_content_item_permissions.py +++ b/integration/tests/posit/connect/test_content_item_permissions.py @@ -11,14 +11,14 @@ def setup_class(cls): cls.content = cls.client.content.create(name="example") cls.user_aron = cls.client.users.create( - username="aron", - email="aron@example.com", - password="s3cur3p@ssword", + username="permission_aron", + email="permission_aron@example.com", + password="permission_s3cur3p@ssword", ) cls.user_bill = cls.client.users.create( - username="bill", - email="bill@example.com", - password="s3cur3p@ssword", + username="permission_bill", + email="permission_bill@example.com", + password="permission_s3cur3p@ssword", ) cls.group_friends = cls.client.groups.create(name="Friends") diff --git a/integration/tests/posit/connect/test_users.py b/integration/tests/posit/connect/test_users.py index e1f68498..aebf7cf0 100644 --- a/integration/tests/posit/connect/test_users.py +++ b/integration/tests/posit/connect/test_users.py @@ -5,6 +5,10 @@ class TestUser: @classmethod def setup_class(cls): cls.client = client = connect.Client() + + # Play nicely with other tests + cls.existing_users = client.users.count() + cls.aron = client.users.create( username="aron", email="aron@example.com", @@ -29,8 +33,8 @@ def test_lock(self): assert len(self.client.users.find(account_status="locked")) == 0 def test_count(self): - # aron, bill, cole, and me - assert self.client.users.count() == 4 + # aron, bill, cole, and me (and existing user) + assert self.client.users.count() == 3 + self.existing_users def test_find(self): assert self.client.users.find(prefix="aron") == [self.aron] From ecdf800997fb61b7be847b4a3246057724cfd660 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 16:02:15 -0500 Subject: [PATCH 10/17] Do not remove `Permission.destroy()`, instead deprecate it (usage in existing cookbook) --- src/posit/connect/_deprecated.py | 23 ++++++++++++++++++++++ src/posit/connect/permissions.py | 9 ++++++++- tests/posit/connect/test_permissions.py | 26 ++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 src/posit/connect/_deprecated.py diff --git a/src/posit/connect/_deprecated.py b/src/posit/connect/_deprecated.py new file mode 100644 index 00000000..17c2ba35 --- /dev/null +++ b/src/posit/connect/_deprecated.py @@ -0,0 +1,23 @@ +import warnings + + +# From https://github.com/posit-dev/py-shiny/blob/f6b92d8cf49a90f3b3dbb636cd6d7fdeee244cfd/shiny/_deprecated.py#L15C1-L28C1 +# Create our own warning class instead of using built-in DeprecationWarning, because we +# want to be able to control display of these messages without interfering with the +# user's control of DeprecationWarning. +# 2024-11: Change base class to DeprecationWarning +class PositConnectDeprecationWarning(DeprecationWarning): + pass + + +# By default DeprecationWarnings aren't shown; we want to always show them. +warnings.simplefilter("always", PositConnectDeprecationWarning) + + +def warn_deprecated(message: str, stacklevel: int = 3): + warnings.warn(message, PositConnectDeprecationWarning, stacklevel=stacklevel) + + +def warn_deprecated_and_removed_in_future(message: str, stacklevel: int = 3): + message += " This method will be removed in a future release." + warnings.warn(message, PositConnectDeprecationWarning, stacklevel=stacklevel) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index e423ad9c..891b8591 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -6,6 +6,7 @@ from requests.sessions import Session as Session +from ._deprecated import warn_deprecated_and_removed_in_future from .resources import Resource, ResourceParameters, Resources if TYPE_CHECKING: @@ -14,8 +15,14 @@ class Permission(Resource): + def delete(self) -> None: + """[Deprecated] Delete the permission.""" + warn_deprecated_and_removed_in_future("Please use .destroy() instead of .delete().") + + self.destroy() + def destroy(self) -> None: - """Delete the permission.""" + """Destroy the permission.""" path = f"v1/content/{self['content_guid']}/permissions/{self['id']}" url = self.params.url + path self.params.session.delete(url) diff --git a/tests/posit/connect/test_permissions.py b/tests/posit/connect/test_permissions.py index a377bede..1f30fa9b 100644 --- a/tests/posit/connect/test_permissions.py +++ b/tests/posit/connect/test_permissions.py @@ -6,6 +6,7 @@ import responses from responses import matchers +from posit.connect._deprecated import PositConnectDeprecationWarning from posit.connect.groups import Group from posit.connect.permissions import Permission, Permissions from posit.connect.resources import ResourceParameters @@ -15,7 +16,7 @@ from .api import load_mock, load_mock_dict, load_mock_list -class TestPermissionDelete: +class TestPermissionDestroy: @responses.activate def test(self): # data @@ -38,6 +39,29 @@ def test(self): # assert assert mock_delete.call_count == 1 + @responses.activate + def test_destroy_deprecated(self): + # data + uid = "94" + content_guid = "f2f37341-e21d-3d80-c698-a935ad614066" + + # behavior + mock_delete = responses.delete( + f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{uid}", + ) + + # setup + params = ResourceParameters(requests.Session(), Url("https://connect.example/__api__")) + fake_permission = load_mock_dict(f"v1/content/{content_guid}/permissions/{uid}.json") + permission = Permission(params, **fake_permission) + + # invoke + with pytest.warns(PositConnectDeprecationWarning, match="destroy"): + permission.delete() + + # assert + assert mock_delete.call_count == 1 + class TestPermissionUpdate: @responses.activate From b2cd9d24c09b71282b940a7c40ad4628c1bfb910 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 16:06:22 -0500 Subject: [PATCH 11/17] Remove impossible commented code --- .../tests/posit/connect/test_content_item_permissions.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/integration/tests/posit/connect/test_content_item_permissions.py b/integration/tests/posit/connect/test_content_item_permissions.py index 9742eaa4..489bf543 100644 --- a/integration/tests/posit/connect/test_content_item_permissions.py +++ b/integration/tests/posit/connect/test_content_item_permissions.py @@ -31,10 +31,6 @@ def teardown_class(cls): cls.group_friends.delete() assert cls.client.groups.count() == 0 - # cls.user_aron.delete() - # cls.user_bill.delete() - # assert cls.client.users.count() == 0 - def test_permissions_add_destroy(self): assert self.client.groups.count() == 1 assert self.client.users.count() == 3 From ba02668186bc741b11685b0c78cc291ec619425f Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 15 Nov 2024 16:16:18 -0500 Subject: [PATCH 12/17] Correct outdated name --- src/posit/connect/permissions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index 891b8591..41de47bf 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -183,11 +183,11 @@ def destroy(self, *permissions: str | Group | User | Permission) -> list[Permiss client = connect.Client() # Remove a single permission by principal_guid - client.content.get(content_guid).permissions.delete(principal_guid) + client.content.get(content_guid).permissions.destroy(principal_guid) # Remove by user (if principal_guid is a user) user = client.users.get(principal_guid) - client.content.get(content_guid).permissions.delete(user) + client.content.get(content_guid).permissions.destroy(user) # Remove by group (if principal_guid is a group) group = client.groups.get(principal_guid) From c3261edb8953fb45a27b4705d5f610554a6f8c0d Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 22 Nov 2024 13:06:33 -0500 Subject: [PATCH 13/17] Apply suggestions from code review Co-authored-by: Jonathan Keane --- src/posit/connect/permissions.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index 41de47bf..bbd29ad1 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -150,7 +150,7 @@ def get(self, uid: str) -> Permission: return Permission(self.params, **response.json()) def destroy(self, *permissions: str | Group | User | Permission) -> list[Permission]: - """Destroy supplied content item permissions. + """Remove supplied content item permissions. Removes all provided permissions from the content item's permissions. @@ -158,7 +158,7 @@ def destroy(self, *permissions: str | Group | User | Permission) -> list[Permiss ---------- *permissions : str | Group | User | Permission The content item permissions to remove. If a `str` is received, it is compared against - the Permissions' `principal_gruid`. If a `Group`, `User`, or `Permission` is received, + the Permissions' `principal_guid`. If a `Group`, `User`, or `Permission` is received, the `guid` is used and compared against the Permissions' `principal_guid`. If a `Permission` is received, the `principal_guid` is used. Note, only the associated permissions will be destroyed; Any users or groups provided will remain. @@ -220,7 +220,7 @@ def destroy(self, *permissions: str | Group | User | Permission) -> list[Permiss principal_guid: str = arg["principal_guid"] else: raise TypeError( - f"destroy() expected argument type 'str', 'Group', 'Permission' or 'User', but got '{type(arg).__name__}'", + f"destroy() expected argument type 'str', 'User', 'Group', or 'Permission' but got '{type(arg).__name__}'", ) principal_guids.add(principal_guid) From 2ce66b3c884c6e649b2c0d8ec5deb7a2bbeecf06 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 22 Nov 2024 13:25:28 -0500 Subject: [PATCH 14/17] simplify docs. Add note about silently ignoring perms not found --- src/posit/connect/permissions.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index bbd29ad1..70077c8f 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -152,20 +152,21 @@ def get(self, uid: str) -> Permission: def destroy(self, *permissions: str | Group | User | Permission) -> list[Permission]: """Remove supplied content item permissions. - Removes all provided permissions from the content item's permissions. + Removes all provided permissions from the content item's permissions. If a permission isn't + found, it is silently ignored. Parameters ---------- *permissions : str | Group | User | Permission The content item permissions to remove. If a `str` is received, it is compared against - the Permissions' `principal_guid`. If a `Group`, `User`, or `Permission` is received, - the `guid` is used and compared against the Permissions' `principal_guid`. If a - `Permission` is received, the `principal_guid` is used. Note, only the associated permissions will be destroyed; Any users or groups provided will remain. + the `Permissions`' `principal_guid`. If a `Group` or `User` is received, the associated + `Permission` will be removed. Returns ------- list[Permission] - The deleted permissions. + The removed permissions. If a permission is not found, it is not included in the + returned list. Examples -------- From debb45a9d9dab2c7ecdf888ff9e75419f09236c7 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 26 Nov 2024 16:12:20 -0500 Subject: [PATCH 15/17] Remove deprecated code --- src/posit/connect/_deprecated.py | 23 ----------------------- src/posit/connect/permissions.py | 7 ------- tests/posit/connect/test_permissions.py | 24 ------------------------ 3 files changed, 54 deletions(-) delete mode 100644 src/posit/connect/_deprecated.py diff --git a/src/posit/connect/_deprecated.py b/src/posit/connect/_deprecated.py deleted file mode 100644 index 17c2ba35..00000000 --- a/src/posit/connect/_deprecated.py +++ /dev/null @@ -1,23 +0,0 @@ -import warnings - - -# From https://github.com/posit-dev/py-shiny/blob/f6b92d8cf49a90f3b3dbb636cd6d7fdeee244cfd/shiny/_deprecated.py#L15C1-L28C1 -# Create our own warning class instead of using built-in DeprecationWarning, because we -# want to be able to control display of these messages without interfering with the -# user's control of DeprecationWarning. -# 2024-11: Change base class to DeprecationWarning -class PositConnectDeprecationWarning(DeprecationWarning): - pass - - -# By default DeprecationWarnings aren't shown; we want to always show them. -warnings.simplefilter("always", PositConnectDeprecationWarning) - - -def warn_deprecated(message: str, stacklevel: int = 3): - warnings.warn(message, PositConnectDeprecationWarning, stacklevel=stacklevel) - - -def warn_deprecated_and_removed_in_future(message: str, stacklevel: int = 3): - message += " This method will be removed in a future release." - warnings.warn(message, PositConnectDeprecationWarning, stacklevel=stacklevel) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index 70077c8f..b23f53f9 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -6,7 +6,6 @@ from requests.sessions import Session as Session -from ._deprecated import warn_deprecated_and_removed_in_future from .resources import Resource, ResourceParameters, Resources if TYPE_CHECKING: @@ -15,12 +14,6 @@ class Permission(Resource): - def delete(self) -> None: - """[Deprecated] Delete the permission.""" - warn_deprecated_and_removed_in_future("Please use .destroy() instead of .delete().") - - self.destroy() - def destroy(self) -> None: """Destroy the permission.""" path = f"v1/content/{self['content_guid']}/permissions/{self['id']}" diff --git a/tests/posit/connect/test_permissions.py b/tests/posit/connect/test_permissions.py index 1f30fa9b..d47fc74c 100644 --- a/tests/posit/connect/test_permissions.py +++ b/tests/posit/connect/test_permissions.py @@ -6,7 +6,6 @@ import responses from responses import matchers -from posit.connect._deprecated import PositConnectDeprecationWarning from posit.connect.groups import Group from posit.connect.permissions import Permission, Permissions from posit.connect.resources import ResourceParameters @@ -39,29 +38,6 @@ def test(self): # assert assert mock_delete.call_count == 1 - @responses.activate - def test_destroy_deprecated(self): - # data - uid = "94" - content_guid = "f2f37341-e21d-3d80-c698-a935ad614066" - - # behavior - mock_delete = responses.delete( - f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{uid}", - ) - - # setup - params = ResourceParameters(requests.Session(), Url("https://connect.example/__api__")) - fake_permission = load_mock_dict(f"v1/content/{content_guid}/permissions/{uid}.json") - permission = Permission(params, **fake_permission) - - # invoke - with pytest.warns(PositConnectDeprecationWarning, match="destroy"): - permission.delete() - - # assert - assert mock_delete.call_count == 1 - class TestPermissionUpdate: @responses.activate From 19f25dd373f8377bafa3e270c83cbbffe1ea1be8 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 26 Nov 2024 16:14:12 -0500 Subject: [PATCH 16/17] user clearer name --- integration/tests/posit/connect/test_users.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/tests/posit/connect/test_users.py b/integration/tests/posit/connect/test_users.py index aebf7cf0..ed4efbad 100644 --- a/integration/tests/posit/connect/test_users.py +++ b/integration/tests/posit/connect/test_users.py @@ -7,7 +7,7 @@ def setup_class(cls): cls.client = client = connect.Client() # Play nicely with other tests - cls.existing_users = client.users.count() + cls.existing_user_count = client.users.count() cls.aron = client.users.create( username="aron", @@ -34,7 +34,7 @@ def test_lock(self): def test_count(self): # aron, bill, cole, and me (and existing user) - assert self.client.users.count() == 3 + self.existing_users + assert self.client.users.count() == 3 + self.existing_user_count def test_find(self): assert self.client.users.find(prefix="aron") == [self.aron] From 11e473002781923321e94186c47bbf9b244b7685 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 27 Nov 2024 09:35:23 -0500 Subject: [PATCH 17/17] Apply suggestions from code review Co-authored-by: Jonathan Keane --- src/posit/connect/permissions.py | 10 ++++------ tests/posit/connect/test_permissions.py | 1 - 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/posit/connect/permissions.py b/src/posit/connect/permissions.py index b23f53f9..5806e019 100644 --- a/src/posit/connect/permissions.py +++ b/src/posit/connect/permissions.py @@ -152,14 +152,14 @@ def destroy(self, *permissions: str | Group | User | Permission) -> list[Permiss ---------- *permissions : str | Group | User | Permission The content item permissions to remove. If a `str` is received, it is compared against - the `Permissions`' `principal_guid`. If a `Group` or `User` is received, the associated + the `Permissions`'s `principal_guid`. If a `Group` or `User` is received, the associated `Permission` will be removed. Returns ------- list[Permission] - The removed permissions. If a permission is not found, it is not included in the - returned list. + The removed permissions. If a permission is not found, there is nothing to remove and + it is not included in the returned list. Examples -------- @@ -206,9 +206,7 @@ def destroy(self, *permissions: str | Group | User | Permission) -> list[Permiss for arg in permissions: if isinstance(arg, str): principal_guid = arg - elif isinstance(arg, Group): - principal_guid: str = arg["guid"] - elif isinstance(arg, User): + elif isinstance(arg, (Group, User)): principal_guid: str = arg["guid"] elif isinstance(arg, Permission): principal_guid: str = arg["principal_guid"] diff --git a/tests/posit/connect/test_permissions.py b/tests/posit/connect/test_permissions.py index d47fc74c..c8cc4b6a 100644 --- a/tests/posit/connect/test_permissions.py +++ b/tests/posit/connect/test_permissions.py @@ -321,7 +321,6 @@ def test_destroy(self): ) # Assert bad input value - with pytest.raises(TypeError): permissions.destroy( 42 # pyright: ignore[reportArgumentType]