feat: Add User.groups and Group.members (#341)

Author: schloerke

integration/tests/posit/connect/test_content_item_permissions.py

@@ -1,5 +1,14 @@
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+import pytest
+
 from posit import connect
-from posit.connect.content import ContentItem
+
+if TYPE_CHECKING:
+    from posit.connect.content import ContentItem
+    from posit.connect.permissions import Permission
 
 
 class TestContentPermissions:
@@ -48,7 +57,7 @@ def test_permissions_add_destroy(self):
             role="owner",
         )
 
-        def assert_permissions_match_guids(permissions, objs_with_guid):
+        def assert_permissions_match_guids(permissions: list[Permission], objs_with_guid):
             for permission, obj_with_guid in zip(permissions, objs_with_guid):
                 assert permission["principal_guid"] == obj_with_guid["guid"]
 
@@ -59,11 +68,9 @@ def assert_permissions_match_guids(permissions, objs_with_guid):
         )
 
         # 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],
-        )
+        self.content.permissions.destroy(self.user_aron)
+        with pytest.raises(ValueError):
+            self.content.permissions.destroy(self.user_bill)
 
         # Prove they have been removed
         assert_permissions_match_guids(
@@ -72,11 +79,7 @@ def assert_permissions_match_guids(permissions, objs_with_guid):
         )
 
         # Remove the last permission
-        destroyed_permissions = self.content.permissions.destroy(self.group_friends)
-        assert_permissions_match_guids(
-            destroyed_permissions,
-            [self.group_friends],
-        )
+        self.content.permissions.destroy(self.group_friends)
 
         # Prove they have been removed
         assert self.content.permissions.find() == []

integration/tests/posit/connect/test_users.py

@@ -51,6 +51,48 @@ def test_get(self):
         assert self.client.users.get(self.bill["guid"]) == self.bill
         assert self.client.users.get(self.cole["guid"]) == self.cole
 
+    # Also tests Groups.members
+    def test_user_group_interactions(self):
+        try:
+            test_group = self.client.groups.create(name="UnitFriends")
+
+            # `Group.members.count()`
+            assert test_group.members.count() == 0
+
+            # `Group.members.add()`
+            test_group.members.add(self.bill)
+            # `User.groups.add()`
+            assert test_group.members.count() == 1
+            self.cole.groups.add(test_group)
+            assert test_group.members.count() == 2
+
+            # `Group.members.find()`
+            group_users = test_group.members.find()
+            assert len(group_users) == 2
+            assert group_users[0]["guid"] == self.bill["guid"]
+            assert group_users[1]["guid"] == self.cole["guid"]
+
+            # `User.group.find()`
+            bill_groups = self.bill.groups.find()
+            assert len(bill_groups) == 1
+            assert bill_groups[0]["guid"] == test_group["guid"]
+
+            # `Group.members.delete()`
+            test_group.members.delete(self.bill)
+            assert test_group.members.count() == 1
+
+            # `User.groups.delete()`
+            self.cole.groups.delete(test_group)
+            assert test_group.members.count() == 0
+
+        finally:
+            groups = self.client.groups.find(prefix="UnitFriends")
+            if len(groups) > 0:
+                test_group = groups[0]
+                test_group.delete()
+
+            assert len(self.client.groups.find(prefix="UnitFriends")) == 0
+
 
 class TestUserContent:
     """Checks behavior of the content attribute."""

src/posit/connect/client.py

@@ -156,7 +156,7 @@ def __init__(self, *args, **kwargs) -> None:
         session.hooks["response"].append(hooks.handle_errors)
         self.session = session
         self.resource_params = ResourceParameters(session, self.cfg.url)
-        self._ctx = Context(self.session, self.cfg.url)
+        self._ctx = Context(self)
 
     @property
     def version(self) -> str | None:
@@ -180,7 +180,7 @@ def me(self) -> User:
         User
             The currently authenticated user.
         """
-        return me.get(self.resource_params)
+        return me.get(self._ctx)
 
     @property
     def groups(self) -> Groups:
@@ -191,7 +191,7 @@ def groups(self) -> Groups:
         Groups
             The groups resource interface.
         """
-        return Groups(self.resource_params)
+        return Groups(self._ctx)
 
     @property
     def tasks(self) -> Tasks:
@@ -215,7 +215,7 @@ def users(self) -> Users:
         Users
             The users resource instance.
         """
-        return Users(self.resource_params)
+        return Users(self._ctx)
 
     @property
     def content(self) -> Content:
@@ -227,7 +227,7 @@ def content(self) -> Content:
         Content
             The content resource instance.
         """
-        return Content(self.resource_params)
+        return Content(self._ctx)
 
     @property
     def metrics(self) -> Metrics:

src/posit/connect/content.py

@@ -20,7 +20,6 @@
 from . import tasks
 from ._api import ApiDictEndpoint, JsonifiableDict
 from .bundles import Bundles
-from .context import Context
 from .env import EnvVars
 from .errors import ClientError
 from .jobs import JobsMixin
@@ -32,6 +31,7 @@
 from .variants import Variants
 
 if TYPE_CHECKING:
+    from .context import Context
     from .tasks import Task
 
 
@@ -221,30 +221,32 @@ class _AttrsCreate(_AttrsBase):
     @overload
     def __init__(
         self,
+        ctx: Context,
         /,
-        params: ResourceParameters,
+        *,
         guid: str,
     ) -> None: ...
 
     @overload
     def __init__(
         self,
+        ctx: Context,
         /,
-        params: ResourceParameters,
+        *,
         guid: str,
         **kwargs: Unpack[ContentItem._Attrs],
     ) -> None: ...
 
     def __init__(
         self,
+        ctx: Context,
         /,
-        params: ResourceParameters,
+        *,
         guid: str,
         **kwargs: Unpack[ContentItem._AttrsNotRequired],
     ) -> None:
         _assert_guid(guid)
 
-        ctx = Context(params.session, params.url)
         path = f"v1/content/{guid}"
         super().__init__(ctx, path, guid=guid, **kwargs)
 
@@ -291,8 +293,8 @@ def create_repository(
     def delete(self) -> None:
         """Delete the content item."""
         path = f"v1/content/{self['guid']}"
-        url = self.params.url + path
-        self.params.session.delete(url)
+        url = self._ctx.url + path
+        self._ctx.session.delete(url)
 
     def deploy(self) -> tasks.Task:
         """Deploy the content.
@@ -311,8 +313,8 @@ def deploy(self) -> tasks.Task:
         None
         """
         path = f"v1/content/{self['guid']}/deploy"
-        url = self.params.url + path
-        response = self.params.session.post(url, json={"bundle_id": None})
+        url = self._ctx.url + path
+        response = self._ctx.session.post(url, json={"bundle_id": None})
         result = response.json()
         ts = tasks.Tasks(self.params)
         return ts.get(result["task_id"])
@@ -367,8 +369,8 @@ def restart(self) -> None:
             self.environment_variables.create(key, unix_epoch_in_seconds)
             self.environment_variables.delete(key)
             # GET via the base Connect URL to force create a new worker thread.
-            url = posixpath.join(dirname(self.params.url), f"content/{self['guid']}")
-            self.params.session.get(url)
+            url = posixpath.join(dirname(self._ctx.url), f"content/{self['guid']}")
+            self._ctx.session.get(url)
             return None
         else:
             raise ValueError(
@@ -438,8 +440,8 @@ def update(
         -------
         None
         """
-        url = self.params.url + f"v1/content/{self['guid']}"
-        response = self.params.session.patch(url, json=attrs)
+        url = self._ctx.url + f"v1/content/{self['guid']}"
+        response = self._ctx.session.patch(url, json=attrs)
         super().update(**response.json())
 
     # Relationships
@@ -464,7 +466,9 @@ def owner(self) -> dict:
             # If it's not included, we can retrieve the information by `owner_guid`
             from .users import Users
 
-            self["owner"] = Users(self.params).get(self["owner_guid"])
+            self["owner"] = Users(
+                self._ctx,
+            ).get(self["owner_guid"])
         return self["owner"]
 
     @property
@@ -512,12 +516,13 @@ class Content(Resources):
 
     def __init__(
         self,
-        params: ResourceParameters,
+        ctx: Context,
         *,
         owner_guid: str | None = None,
     ) -> None:
-        super().__init__(params)
+        super().__init__(ctx.client.resource_params)
         self.owner_guid = owner_guid
+        self._ctx = ctx
 
     def count(self) -> int:
         """Count the number of content items.
@@ -592,9 +597,9 @@ def create(
         ContentItem
         """
         path = "v1/content"
-        url = self.params.url + path
-        response = self.params.session.post(url, json=attrs)
-        return ContentItem(self.params, **response.json())
+        url = self._ctx.url + path
+        response = self._ctx.session.post(url, json=attrs)
+        return ContentItem(self._ctx, **response.json())
 
     @overload
     def find(
@@ -680,11 +685,11 @@ def find(self, include: Optional[str | list[Any]] = None, **conditions) -> List[
             conditions["owner_guid"] = self.owner_guid
 
         path = "v1/content"
-        url = self.params.url + path
-        response = self.params.session.get(url, params=conditions)
+        url = self._ctx.url + path
+        response = self._ctx.session.get(url, params=conditions)
         return [
             ContentItem(
-                self.params,
+                self._ctx,
                 **result,
             )
             for result in response.json()
@@ -853,6 +858,6 @@ def get(self, guid: str) -> ContentItem:
         ContentItem
         """
         path = f"v1/content/{guid}"
-        url = self.params.url + path
-        response = self.params.session.get(url)
-        return ContentItem(self.params, **response.json())
+        url = self._ctx.url + path
+        response = self._ctx.session.get(url)
+        return ContentItem(self._ctx, **response.json())

src/posit/connect/context.py

@@ -1,13 +1,15 @@
 from __future__ import annotations
 
 import functools
+import weakref
 from typing import TYPE_CHECKING, Protocol
 
 from packaging.version import Version
 
 if TYPE_CHECKING:
     import requests
 
+    from .client import Client
     from .urls import Url
 
 
@@ -28,9 +30,12 @@ def wrapper(instance: ContextManager, *args, **kwargs):
 
 
 class Context:
-    def __init__(self, session: requests.Session, url: Url):
-        self.session = session
-        self.url = url
+    def __init__(self, client: Client):
+        self.session: requests.Session = client.session
+        self.url: Url = client.cfg.url
+        # Since this is a child object of the client, we use a weak reference to avoid circular
+        # references (which would prevent garbage collection)
+        self.client: Client = weakref.proxy(client)
 
     @property
     def version(self) -> str | None: