diff --git a/.github/workflows/conventional-commits.yaml b/.github/workflows/conventional-commits.yaml index f43068f3..53f39847 100644 --- a/.github/workflows/conventional-commits.yaml +++ b/.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) diff --git a/src/posit/connect/client.py b/src/posit/connect/client.py index 6d9a751e..3983c003 100644 --- a/src/posit/connect/client.py +++ b/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: diff --git a/src/posit/connect/content.py b/src/posit/connect/content.py index 56ec07e6..5ce5949c 100644 --- a/src/posit/connect/content.py +++ b/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() diff --git a/src/posit/connect/resources.py b/src/posit/connect/resources.py index b37b4752..5559e31c 100644 --- a/src/posit/connect/resources.py +++ b/src/posit/connect/resources.py @@ -1,13 +1,22 @@ import warnings from abc import ABC, abstractmethod -from typing import Generic, List, Optional, TypeVar +from typing import Any, Generic, List, Optional, TypeVar + +import requests T = TypeVar("T") class Resource(ABC, dict): + def __init__(self, session: requests.Session, url: str, **kwargs): + super().__init__(**kwargs) + self.session: requests.Session + super().__setattr__("session", session) + self.url: str + super().__setattr__("url", url) + def __getitem__(self, key): warnings.warn( f"__getitem__ for '{key}' does not support backwards compatibility. Consider using field based access instead: 'instance.{key}'", @@ -29,6 +38,9 @@ def __delitem__(self, key): ) return super().__delitem__(key) + def __setattr__(self, name: str, value: Any) -> None: + raise AttributeError("cannot set attributes: use update() instead") + class Resources(ABC, Generic[T]): @abstractmethod diff --git a/src/posit/connect/users.py b/src/posit/connect/users.py index bfabf7de..d61e9030 100644 --- a/src/posit/connect/users.py +++ b/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: diff --git a/tests/posit/connect/test_resources.py b/tests/posit/connect/test_resources.py index 5a8a20c4..380ededb 100644 --- a/tests/posit/connect/test_resources.py +++ b/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 @@ -13,12 +19,20 @@ def foo(self) -> Optional[str]: class TestResource: + def test_init(self): + k = "foo" + v = "bar" + d = dict({k: v}) + r = FakeResource(session, url, **d) + assert r.session == session + assert r.url == url + def test__getitem__(self): warnings.filterwarnings("ignore", category=FutureWarning) 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 +41,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 +51,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 +62,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 diff --git a/tests/posit/connect/test_users.py b/tests/posit/connect/test_users.py index 5f77b19a..9ebfcda4 100644 --- a/tests/posit/connect/test_users.py +++ b/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": "test@example.com"}) + user = User(session, url, email="test@example.com") assert user.email == "test@example.com" 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"] @@ -263,6 +265,6 @@ def test_user_cant_setattr(self): with pytest.raises( AttributeError, - match=r"Cannot set attributes: use update\(\) instead", + match=r"cannot set attributes: use update\(\) instead", ): carlos.first_name = "Carlitos" diff --git a/tinkering.py b/tinkering.py index 6054fdff..a1e62f5c 100644 --- a/tinkering.py +++ b/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"))