refactor: moves session and url to resource attributes

.github/workflows/conventional-commits.yaml

@@ -23,6 +23,7 @@ jobs:
             fix
             perf
             style
+            refactor
             test
       - uses: marocchino/sticky-pull-request-comment@v2
         if: always() && (steps.lint.outputs.error_message != null)

src/posit/connect/client.py

@@ -50,7 +50,7 @@ def connect_version(self):
     def me(self) -> User:
         url = urls.append_path(self.config.url, "v1/user")
         response = self.session.get(url)
-        return User(**response.json())
+        return User(self.session, url, **response.json())
 
     @property
     def oauth(self) -> OAuthIntegration:

src/posit/connect/content.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from typing import Any, Callable, List, Optional
+from typing import Callable, List, Optional
 
 from requests import Session
 
@@ -183,9 +183,6 @@ def app_role(self) -> str:
     def id(self) -> str:
         return self.get("id")  # type: ignore
 
-    def __setattr__(self, name: str, value: Any) -> None:
-        raise AttributeError("Cannot set attributes: use update() instead")
-
 
 class Content(Resources[ContentItem]):
     def __init__(self, config: Config, session: Session) -> None:
@@ -197,22 +194,34 @@ def find(
         self, filter: Callable[[ContentItem], bool] = lambda _: True
     ) -> List[ContentItem]:
         results = self.session.get(self.url).json()
-        return [ContentItem(**c) for c in results if filter(ContentItem(**c))]
+        items = (
+            ContentItem(
+                session=self.session,
+                url=urls.append_path(self.url, result["guid"]),
+                **result,
+            )
+            for result in results
+        )
+        return [item for item in items if filter(item)]
 
     def find_one(
         self, filter: Callable[[ContentItem], bool] = lambda _: True
     ) -> ContentItem | None:
         results = self.session.get(self.url).json()
-        for c in results:
-            content_item = ContentItem(**c)
-            if filter(content_item):
-                return content_item
+        for result in results:
+            item = ContentItem(
+                session=self.session,
+                url=urls.append_path(self.url, result["guid"]),
+                **result,
+            )
+            if filter(item):
+                return item
         return None
 
     def get(self, id: str) -> ContentItem:
         url = urls.append_path(self.url, id)
         response = self.session.get(url)
-        return ContentItem(**response.json())
+        return ContentItem(self.session, url, **response.json())
 
     def create(self) -> ContentItem:
         raise NotImplementedError()

src/posit/connect/resources.py

@@ -3,11 +3,18 @@
 from abc import ABC, abstractmethod
 from typing import Generic, List, Optional, TypeVar
 
+import requests
+
 
 T = TypeVar("T")
 
 
 class Resource(ABC, dict):
+    def __init__(self, session: requests.Session, url: str, **kwargs):
+        self._session = session
+        self._url = url
+        super().__init__(**kwargs)
+
     def __getitem__(self, key):
         warnings.warn(
             f"__getitem__ for '{key}' does not support backwards compatibility. Consider using field based access instead: 'instance.{key}'",

src/posit/connect/users.py

@@ -1,5 +1,5 @@
 from __future__ import annotations
-from typing import Any, Callable, List, Optional
+from typing import Callable, List, Optional
 
 
 from requests import Session
@@ -57,11 +57,8 @@ def confirmed(self) -> bool:
     def locked(self) -> bool:
         return self.get("locked")  # type: ignore
 
-    def __setattr__(self, name: str, value: Any) -> None:
-        raise AttributeError("Cannot set attributes: use update() instead.")
-
     def _update(self, body):
-        self.get("session").patch(self.get("url"), json=body)
+        self._session.patch(self._url, json=body)
         # If the request is successful, update the local object
         super().update(body)
         # TODO(#99): that patch request returns a payload on success,
@@ -107,15 +104,15 @@ def find(
         self, filter: Callable[[User], bool] = lambda _: True, page_size=_MAX_PAGE_SIZE
     ) -> List[User]:
         results = Paginator(self.session, self.url, page_size=page_size).get_all()
-        return [
+        users = (
             User(
-                **user,
                 session=self.session,
                 url=urls.append_path(self.url, user["guid"]),
+                **user,
             )
             for user in results
-            if filter(User(**user))
-        ]
+        )
+        return [user for user in users if filter(user)]
 
     def find_one(
         self, filter: Callable[[User], bool] = lambda _: True, page_size=_MAX_PAGE_SIZE
@@ -125,9 +122,9 @@ def find_one(
             result = pager.get_next_page()
             for u in result:
                 user = User(
-                    **u,
                     session=self.session,
                     url=urls.append_path(self.url, u["guid"]),
+                    **u,
                 )
                 if filter(user):
                     return user
@@ -138,9 +135,9 @@ def get(self, id: str) -> User:
         response = self.session.get(url)
         raw_user = response.json()
         return User(
-            **raw_user,
             session=self.session,
             url=urls.append_path(self.url, raw_user["guid"]),
+            **raw_user,
         )
 
     def create(self) -> User:

tests/posit/connect/test_resources.py

@@ -1,10 +1,16 @@
+from unittest.mock import Mock
 import pytest
 import warnings
 
 from typing import Any, List, Optional
 
+from requests.sessions import Session as Session
+
 from posit.connect.resources import Resource, Resources
 
+session = Mock()
+url = Mock()
+
 
 class FakeResource(Resource):
     @property
@@ -18,7 +24,7 @@ def test__getitem__(self):
         k = "foo"
         v = "bar"
         d = dict({k: v})
-        r = FakeResource(d)
+        r = FakeResource(session, url, **d)
         assert r.__getitem__(k) == d.__getitem__(k)
         assert r[k] == d[k]
 
@@ -27,7 +33,8 @@ def test__setitem__(self):
         k = "foo"
         v1 = "bar"
         v2 = "baz"
-        r = FakeResource({k: v1})
+        d = dict({k: v1})
+        r = FakeResource(session, url, **d)
         assert r[k] == v1
         r[k] = v2
         assert r[k] == v2
@@ -36,7 +43,8 @@ def test__delitem__(self):
         warnings.filterwarnings("ignore", category=FutureWarning)
         k = "foo"
         v = "bar"
-        r = FakeResource({k: v})
+        d = dict({k: v})
+        r = FakeResource(session, url, **d)
         assert k in r
         assert r[k] == v
         del r[k]
@@ -46,7 +54,7 @@ def test_foo(self):
         k = "foo"
         v = "bar"
         d = dict({k: v})
-        r = FakeResource(d)
+        r = FakeResource(session, url, **d)
         assert r.foo == v
 
 

tests/posit/connect/test_users.py

@@ -1,3 +1,4 @@
+from unittest.mock import Mock
 import pandas as pd
 import pytest
 import responses
@@ -9,83 +10,86 @@
 
 from .api import load_mock  # type: ignore
 
+session = Mock()
+url = Mock()
+
 
 class TestUser:
     def test_guid(self):
-        user = User()
+        user = User(session, url)
         assert hasattr(user, "guid")
         assert user.guid is None
-        user = User({"guid": "test_guid"})
+        user = User(session, url, **{"guid": "test_guid"})
         assert user.guid == "test_guid"
 
     def test_email(self):
-        user = User()
+        user = User(session, url)
         assert hasattr(user, "email")
         assert user.email is None
-        user = User({"email": "[email protected]"})
+        user = User(session, url, **{"email": "[email protected]"})
         assert user.email == "[email protected]"
 
     def test_username(self):
-        user = User()
+        user = User(session, url)
         assert hasattr(user, "username")
         assert user.username is None
-        user = User({"username": "test_user"})
+        user = User(session, url, **{"username": "test_user"})
         assert user.username == "test_user"
 
     def test_first_name(self):
-        user = User()
+        user = User(session, url)
         assert hasattr(user, "first_name")
         assert user.first_name is None
-        user = User({"first_name": "John"})
+        user = User(session, url, **{"first_name": "John"})
         assert user.first_name == "John"
 
     def test_last_name(self):
-        user = User()
+        user = User(session, url)
         assert hasattr(user, "last_name")
         assert user.last_name is None
-        user = User({"last_name": "Doe"})
+        user = User(session, url, **{"last_name": "Doe"})
         assert user.last_name == "Doe"
 
     def test_user_role(self):
-        user = User()
+        user = User(session, url)
         assert hasattr(user, "user_role")
         assert user.user_role is None
-        user = User({"user_role": "admin"})
+        user = User(session, url, **{"user_role": "admin"})
         assert user.user_role == "admin"
 
     def test_created_time(self):
-        user = User()
+        user = User(session, url)
         assert hasattr(user, "created_time")
         assert user.created_time is None
-        user = User({"created_time": "2022-01-01T00:00:00"})
+        user = User(session, url, **{"created_time": "2022-01-01T00:00:00"})
         assert user.created_time == "2022-01-01T00:00:00"
 
     def test_updated_time(self):
-        user = User()
+        user = User(session, url)
         assert hasattr(user, "updated_time")
         assert user.updated_time is None
-        user = User({"updated_time": "2022-01-01T00:00:00"})
+        user = User(session, url, **{"updated_time": "2022-01-01T00:00:00"})
         assert user.updated_time == "2022-01-01T00:00:00"
 
     def test_active_time(self):
-        user = User()
+        user = User(session, url)
         assert hasattr(user, "active_time")
         assert user.active_time is None
-        user = User({"active_time": "2022-01-01T00:00:00"})
+        user = User(session, url, **{"active_time": "2022-01-01T00:00:00"})
         assert user.active_time == "2022-01-01T00:00:00"
 
     def test_confirmed(self):
-        user = User()
+        user = User(session, url)
         assert hasattr(user, "confirmed")
         assert user.confirmed is None
-        user = User({"confirmed": True})
+        user = User(session, url, **{"confirmed": True})
         assert user.confirmed is True
 
     def test_locked(self):
-        user = User()
+        user = User(session, url)
         assert hasattr(user, "locked")
         assert user.locked is None
-        user = User({"locked": False})
+        user = User(session, url, **{"locked": False})
         assert user.locked is False
 
 
@@ -117,7 +121,7 @@ def test_users_find(self):
 
         df = pd.DataFrame(all_users)
         assert isinstance(df, pd.DataFrame)
-        assert df.shape == (3, 13)
+        assert df.shape == (3, 11)
         assert df.columns.to_list() == [
             "email",
             "username",
@@ -130,8 +134,6 @@ def test_users_find(self):
             "confirmed",
             "locked",
             "guid",
-            "session",
-            "url",
         ]
         assert df["username"].to_list() == ["al", "robert", "carlos12"]
 
@@ -250,19 +252,3 @@ def test_user_update(self):
         # * test setting the other fields
         # * test invalid field
         # * error response (e.g. not authorized)
-
-    @responses.activate
-    def test_user_cant_setattr(self):
-        responses.get(
-            "https://connect.example/__api__/v1/users/20a79ce3-6e87-4522-9faf-be24228800a4",
-            json=load_mock("v1/users/20a79ce3-6e87-4522-9faf-be24228800a4.json"),
-        )
-
-        con = Client(api_key="12345", url="https://connect.example/")
-        carlos = con.users.get("20a79ce3-6e87-4522-9faf-be24228800a4")
-
-        with pytest.raises(
-            AttributeError,
-            match=r"Cannot set attributes: use update\(\) instead",
-        ):
-            carlos.first_name = "Carlitos"

tinkering.py

@@ -1,11 +1,7 @@
 from posit.connect import Client
 
 with Client() as client:
-    print(client.get("v1/users"))
-    print(client.users.get("f55ca95d-ce52-43ed-b31b-48dc4a07fe13"))
-
-    users = client.users
-    users = users.find(lambda user: user["first_name"].startswith("T"))
-    users = users.find(lambda user: user["last_name"].startswith("S"))
-    user = users.find_one(lambda user: user["user_role"] == "administrator")
-    print(user)
+    client.get("v1/users")
+    client.users.get("f55ca95d-ce52-43ed-b31b-48dc4a07fe13")
+    client.users.find(lambda user: user.first_name.startswith("T"))
+    client.users.find_one(lambda user: user.first_name.startswith("T"))