From 5d95633754a53aca7234760f0b06f1bc16dbef25 Mon Sep 17 00:00:00 2001 From: Taylor Steinberg Date: Thu, 15 Feb 2024 12:38:21 -0500 Subject: [PATCH] feat: adds abstraction for resources and subsequent implementation of users. (#20) --- .github/workflows/pull-request.yaml | 48 ++++++++++++++++ .github/workflows/test.yaml | 24 -------- src/posit/connect/__init__.py | 2 +- src/posit/connect/auth.py | 8 ++- src/posit/connect/auth_test.py | 12 ++-- src/posit/connect/client.py | 86 +++++++++++++++-------------- src/posit/connect/client_test.py | 73 +++++++++++------------- src/posit/connect/config.py | 57 +++++++++++++++++++ src/posit/connect/config_test.py | 46 +++++++++++++++ src/posit/connect/endpoints.py | 36 ++++++++++++ src/posit/connect/endpoints_test.py | 16 ++++++ src/posit/connect/resources.py | 81 +++++++++++++++++++++++++++ src/posit/connect/resources_test.py | 40 ++++++++++++++ src/posit/connect/users.py | 86 +++++++++++++++++++++++++---- src/posit/connect/users_test.py | 21 ------- tinkering.py | 13 +++-- 16 files changed, 498 insertions(+), 151 deletions(-) create mode 100644 .github/workflows/pull-request.yaml delete mode 100644 .github/workflows/test.yaml create mode 100644 src/posit/connect/config.py create mode 100644 src/posit/connect/config_test.py create mode 100644 src/posit/connect/endpoints.py create mode 100644 src/posit/connect/endpoints_test.py create mode 100644 src/posit/connect/resources.py create mode 100644 src/posit/connect/resources_test.py diff --git a/.github/workflows/pull-request.yaml b/.github/workflows/pull-request.yaml new file mode 100644 index 00000000..22665bb9 --- /dev/null +++ b/.github/workflows/pull-request.yaml @@ -0,0 +1,48 @@ +name: Pull Request +on: + pull_request: +concurrency: + group: ${{ github.head_ref }} + cancel-in-progress: true +jobs: + test: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: + - '3.8' + - '3.9' + - '3.10' + - '3.11' + - '3.12' + steps: + - uses: actions/checkout@v4 + - uses: extractions/setup-just@v1 + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + - run: just deps + - run: just test + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: extractions/setup-just@v1 + - uses: actions/setup-python@v5 + - run: just deps + - run: just lint + cov: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: extractions/setup-just@v1 + - uses: actions/setup-python@v5 + - run: just deps + - run: just test + - run: just cov xml + - if: always() + uses: orgoro/coverage@v3.1 + with: + coverageFile: coverage.xml + token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml deleted file mode 100644 index d6f44510..00000000 --- a/.github/workflows/test.yaml +++ /dev/null @@ -1,24 +0,0 @@ -name: Test -on: [push] -jobs: - test: - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - python-version: - - '3.8' - - '3.9' - - '3.10' - - '3.11' - - '3.12' - steps: - - uses: actions/checkout@v4 - - uses: extractions/setup-just@v1 - - uses: actions/setup-python@v5 - with: - python-version: ${{ matrix.python-version }} - - run: just deps - - run: just test - - run: just cov - - run: just lint diff --git a/src/posit/connect/__init__.py b/src/posit/connect/__init__.py index 5f22e94e..79add3a0 100644 --- a/src/posit/connect/__init__.py +++ b/src/posit/connect/__init__.py @@ -1 +1 @@ -from .client import Client # noqa +from .client import create_client # noqa diff --git a/src/posit/connect/auth.py b/src/posit/connect/auth.py index 44bdc00e..d4a72d04 100644 --- a/src/posit/connect/auth.py +++ b/src/posit/connect/auth.py @@ -1,11 +1,13 @@ from requests import PreparedRequest from requests.auth import AuthBase +from .config import Config + class Auth(AuthBase): - def __init__(self, key: str) -> None: - self.key = key + def __init__(self, config: Config) -> None: + self._config = config def __call__(self, r: PreparedRequest) -> PreparedRequest: - r.headers["Authorization"] = f"Key {self.key}" + r.headers["Authorization"] = f"Key {self._config.api_key}" return r diff --git a/src/posit/connect/auth_test.py b/src/posit/connect/auth_test.py index 66d4ea8a..59ff231f 100644 --- a/src/posit/connect/auth_test.py +++ b/src/posit/connect/auth_test.py @@ -1,13 +1,15 @@ -from unittest.mock import Mock +from unittest.mock import MagicMock, Mock, patch from .auth import Auth class TestAuth: - def test_auth_headers(self): - key = "foobar" - auth = Auth(key=key) + @patch("posit.connect.auth.Config") + def test_auth_headers(self, Config: MagicMock): + config = Config.return_value + config.api_key = "foobar" + auth = Auth(config=config) r = Mock() r.headers = {} auth(r) - assert r.headers == {"Authorization": f"Key {key}"} + assert r.headers == {"Authorization": f"Key {config.api_key}"} diff --git a/src/posit/connect/client.py b/src/posit/connect/client.py index 63b20f9b..59ac246c 100644 --- a/src/posit/connect/client.py +++ b/src/posit/connect/client.py @@ -1,61 +1,65 @@ -import os +from __future__ import annotations +from contextlib import contextmanager from requests import Session -from typing import Optional +from typing import Generator, Optional from . import hooks from .auth import Auth -from .users import Users +from .config import Config +from .users import LazyUsers, Users -def _get_api_key() -> str: - """Gets the API key from the environment variable 'CONNECT_API_KEY'. +@contextmanager +def create_client( + api_key: Optional[str] = None, endpoint: Optional[str] = None +) -> Generator[Client, None, None]: + """Creates a new :class:`Client` instance - Raises: - ValueError: if CONNECT_API_KEY is not set or invalid + Keyword Arguments: + api_key -- an api_key for authentication (default: {None}) + endpoint -- a base api endpoint (url) (default: {None}) Returns: - The API key + A :class:`Client` instance """ - value = os.environ.get("CONNECT_API_KEY") - if value is None or value == "": - raise ValueError( - "Invalid value for 'CONNECT_API_KEY': Must be a non-empty string." - ) - return value - - -def _get_endpoint() -> str: - """Gets the endpoint from the environment variable 'CONNECT_SERVER'. - - The `requests` library uses 'endpoint' instead of 'server'. We will use 'endpoint' from here forward for consistency. - - Raises: - ValueError: if CONNECT_SERVER is not set or invalid. - - Returns: - The endpoint. - """ - value = os.environ.get("CONNECT_SERVER") - if value is None or value == "": - raise ValueError( - "Invalid value for 'CONNECT_SERVER': Must be a non-empty string." - ) - return value + client = Client(api_key=api_key, endpoint=endpoint) + try: + yield client + finally: + del client class Client: - users: Users - def __init__( self, api_key: Optional[str] = None, endpoint: Optional[str] = None, ) -> None: - self._api_key = api_key or _get_api_key() - self._endpoint = endpoint or _get_endpoint() - self._session = Session() - self._session.hooks["response"].append(hooks.handle_errors) - self._session.auth = Auth(self._api_key) - self.users = Users(self._endpoint, self._session) + """ + Initialize the Client instance. + + Args: + api_key (str, optional): API key for authentication. Defaults to None. + endpoint (str, optional): API endpoint URL. Defaults to None. + """ + # Create a Config object. + config = Config(api_key=api_key, endpoint=endpoint) + # Create a Session object for making HTTP requests. + session = Session() + # Authenticate the session using the provided Config. + session.auth = Auth(config=config) + # Add error handling hooks to the session. + session.hooks["response"].append(hooks.handle_errors) + + # Initialize the Users instance. + self.users: Users = LazyUsers(config=config, session=session) + # Store the Session object. + self._session = session + + def __del__(self): + """ + Close the session when the Client instance is deleted. + """ + self._session.close() diff --git a/src/posit/connect/client_test.py b/src/posit/connect/client_test.py index ba64e366..490ab898 100644 --- a/src/posit/connect/client_test.py +++ b/src/posit/connect/client_test.py @@ -1,52 +1,43 @@ -import pytest - from unittest.mock import MagicMock, patch -from .client import Client, _get_api_key, _get_endpoint +from .client import Client, create_client + + +class TestCreateClient: + @patch("posit.connect.client.Client") + def test(self, Client: MagicMock): + api_key = "foobar" + endpoint = "http://foo.bar" + with create_client(api_key=api_key, endpoint=endpoint) as client: + assert client == Client.return_value class TestClient: - @patch("posit.connect.client.Users") + @patch("posit.connect.client.LazyUsers") @patch("posit.connect.client.Session") + @patch("posit.connect.client.Config") @patch("posit.connect.client.Auth") - def test_init(self, Auth: MagicMock, Session: MagicMock, Users: MagicMock): + def test_init( + self, + Auth: MagicMock, + Config: MagicMock, + Session: MagicMock, + LazyUsers: MagicMock, + ): api_key = "foobar" endpoint = "http://foo.bar" - client = Client(api_key=api_key, endpoint=endpoint) - assert client._api_key == api_key - assert client._endpoint == endpoint + Client(api_key=api_key, endpoint=endpoint) + config = Config.return_value + Auth.assert_called_once_with(config=config) + Config.assert_called_once_with(api_key=api_key, endpoint=endpoint) Session.assert_called_once() - Auth.assert_called_once_with(api_key) - Users.assert_called_once_with(endpoint, Session.return_value) - - -class TestGetApiKey: - @patch.dict("os.environ", {"CONNECT_API_KEY": "foobar"}) - def test_get_api_key(self): - api_key = _get_api_key() - assert api_key == "foobar" - - @patch.dict("os.environ", {"CONNECT_API_KEY": ""}) - def test_get_api_key_empty(self): - with pytest.raises(ValueError): - _get_api_key() + LazyUsers.assert_called_once_with(config=config, session=Session.return_value) - def test_get_api_key_miss(self): - with pytest.raises(ValueError): - _get_api_key() - - -class TestGetEndpoint: - @patch.dict("os.environ", {"CONNECT_SERVER": "http://foo.bar"}) - def test_get_endpoint(self): - endpoint = _get_endpoint() - assert endpoint == "http://foo.bar" - - @patch.dict("os.environ", {"CONNECT_SERVER": ""}) - def test_get_endpoint_empty(self): - with pytest.raises(ValueError): - _get_endpoint() - - def test_get_endpoint_miss(self): - with pytest.raises(ValueError): - _get_endpoint() + @patch("posit.connect.client.Session") + @patch("posit.connect.client.Auth") + def test_del(self, Auth: MagicMock, Session: MagicMock): + api_key = "foobar" + endpoint = "http://foo.bar" + client = Client(api_key=api_key, endpoint=endpoint) + del client + Session.return_value.close.assert_called_once() diff --git a/src/posit/connect/config.py b/src/posit/connect/config.py new file mode 100644 index 00000000..858a80b6 --- /dev/null +++ b/src/posit/connect/config.py @@ -0,0 +1,57 @@ +import os + +from typing import Optional + + +def _get_api_key() -> str: + """Gets the API key from the environment variable 'CONNECT_API_KEY'. + + Raises: + ValueError: if CONNECT_API_KEY is not set or invalid + + Returns: + The API key + """ + value = os.environ.get("CONNECT_API_KEY") + if value is None or value == "": + raise ValueError( + "Invalid value for 'CONNECT_API_KEY': Must be a non-empty string." + ) + return value + + +def _get_endpoint() -> str: + """Gets the endpoint from the environment variable 'CONNECT_SERVER'. + + The `requests` library uses 'endpoint' instead of 'server'. We will use 'endpoint' from here forward for consistency. + + Raises: + ValueError: if CONNECT_SERVER is not set or invalid. + + Returns: + The endpoint. + """ + value = os.environ.get("CONNECT_SERVER") + if value is None or value == "": + raise ValueError( + "Invalid value for 'CONNECT_SERVER': Must be a non-empty string." + ) + return value + + +def _format_endpoint(endpoint: str) -> str: + # todo - format endpoint url and ake sure it ends with __api__ + return endpoint + + +class Config: + """Derived configuration properties""" + + api_key: str + endpoint: str + + def __init__( + self, api_key: Optional[str] = None, endpoint: Optional[str] = None + ) -> None: + self.api_key = api_key or _get_api_key() + self.endpoint = _format_endpoint(endpoint or _get_endpoint()) diff --git a/src/posit/connect/config_test.py b/src/posit/connect/config_test.py new file mode 100644 index 00000000..c95a951a --- /dev/null +++ b/src/posit/connect/config_test.py @@ -0,0 +1,46 @@ +import pytest + +from unittest.mock import patch + +from .config import Config, _get_api_key, _get_endpoint + + +class TestGetApiKey: + @patch.dict("os.environ", {"CONNECT_API_KEY": "foobar"}) + def test_get_api_key(self): + api_key = _get_api_key() + assert api_key == "foobar" + + @patch.dict("os.environ", {"CONNECT_API_KEY": ""}) + def test_get_api_key_empty(self): + with pytest.raises(ValueError): + _get_api_key() + + def test_get_api_key_miss(self): + with pytest.raises(ValueError): + _get_api_key() + + +class TestGetEndpoint: + @patch.dict("os.environ", {"CONNECT_SERVER": "http://foo.bar"}) + def test_get_endpoint(self): + endpoint = _get_endpoint() + assert endpoint == "http://foo.bar" + + @patch.dict("os.environ", {"CONNECT_SERVER": ""}) + def test_get_endpoint_empty(self): + with pytest.raises(ValueError): + _get_endpoint() + + def test_get_endpoint_miss(self): + with pytest.raises(ValueError): + _get_endpoint() + + +class TestConfig: + def test_init(self): + api_key = "foobar" + endpoint = "http://foo.bar" + config = Config(api_key=api_key, endpoint=endpoint) + assert config.api_key == api_key + assert config.endpoint == endpoint diff --git a/src/posit/connect/endpoints.py b/src/posit/connect/endpoints.py new file mode 100644 index 00000000..33dcbc96 --- /dev/null +++ b/src/posit/connect/endpoints.py @@ -0,0 +1,36 @@ +import os +import requests + +_MAX_PAGE_SIZE = 500 + + +def get_users( + endpoint: str, + session: requests.Session, + /, + page_number: int, + *, + page_size: int = 500, +): + """ + Fetches the current page of users. + + Returns: + List[User]: A list of User objects representing the fetched users. + """ + # Construct the endpoint URL. + endpoint = os.path.join(endpoint, "v1/users") + # Redefine the page number using 1-based indexing. + page_number = page_number + 1 + # Define query parameters for pagination. + params = {"page_number": page_number, "page_size": page_size} + # Send a GET request to the endpoint with the specified parameters. + response = session.get(endpoint, params=params) + # Convert response to dict + json = response.json() + # Parse the JSON response and extract the results. + results = json["results"] + # Mark exhausted if the result size is less than the maximum page size. + exhausted = len(results) < page_size + # Create User objects from the results and return them as a list. + return (results, exhausted) diff --git a/src/posit/connect/endpoints_test.py b/src/posit/connect/endpoints_test.py new file mode 100644 index 00000000..bf043e0f --- /dev/null +++ b/src/posit/connect/endpoints_test.py @@ -0,0 +1,16 @@ +from unittest.mock import MagicMock, Mock, patch + +from .endpoints import get_users + + +class TestGetUsers: + @patch("posit.connect.users.Session") + def test(self, Session: MagicMock): + session = Session.return_value + get = session.get = Mock() + response = get.return_value = Mock() + json = response.json = Mock() + json.return_value = {"results": ["foo"]} + users, exhausted = get_users("http://foo.bar", session, page_number=0) + assert users == ["foo"] + assert exhausted diff --git a/src/posit/connect/resources.py b/src/posit/connect/resources.py new file mode 100644 index 00000000..9427b95a --- /dev/null +++ b/src/posit/connect/resources.py @@ -0,0 +1,81 @@ +from __future__ import annotations + + +from abc import ABC, abstractmethod +from typing import Generic, Iterator, Optional, TypeVar, List, TypedDict, Tuple + + +class Resource(TypedDict): + pass + + +T = TypeVar("T", bound=Resource) + + +class Resources(ABC, Generic[T], Iterator[T]): + def __init__(self, data: List[T] = []) -> None: + super().__init__() + self.data = data + + @abstractmethod + def find(self, *args, **kwargs) -> Resources[T]: + raise NotImplementedError() + + @abstractmethod + def find_one(self, *args, **kwargs) -> Optional[T]: + raise NotImplementedError() + + @abstractmethod + def get(self, id: str) -> T: + raise NotImplementedError() + + def __iter__(self) -> Iterator[T]: + self.index = 0 + return self + + def __next__(self) -> T: + if self.index >= len(self.data): + raise StopIteration + + v = self.data[self.index] + self.index += 1 + return v + + def to_pandas(self): + try: + from pandas import DataFrame + + return DataFrame(self) + except ImportError: + return None + + +class LazyResources(Resources[T]): + def __init__(self, data: List[T] = []) -> None: + super().__init__(data) + self.data = data + self.exhausted = False + self.index = 0 + + @abstractmethod + def fetch(self, index) -> Tuple[Optional[Iterator[T]], bool]: + raise NotImplementedError() + + def __iter__(self) -> Iterator[T]: + self.index = 0 + return self + + def __next__(self) -> T: + if self.index >= len(self.data): + if self.exhausted: + raise StopIteration + + results, self.exhausted = self.fetch(self.index) + if not results: + raise StopIteration + + self.data += results + + v = self.data[self.index] + self.index += 1 + return v diff --git a/src/posit/connect/resources_test.py b/src/posit/connect/resources_test.py new file mode 100644 index 00000000..2720690a --- /dev/null +++ b/src/posit/connect/resources_test.py @@ -0,0 +1,40 @@ +from typing import Iterator, Tuple, Optional +from unittest.mock import Mock + +from .resources import Resource, Resources, LazyResources + + +class FakeResource(Resource): + pass + + +class FakeResources(Resources[FakeResource]): + def find(self) -> Resources[FakeResource]: + return self + + def find_one(self) -> Optional[FakeResource]: + return Mock(spec=FakeResource) + + def get(self, _: str) -> FakeResource: + return Mock(spec=FakeResource) + + +class TestResources: + def test(self): + resources = FakeResources() + assert resources == resources.find() + assert resources.find_one() + assert resources.get(None) + + +class FakeLazyResources(FakeResources, LazyResources): + def fetch(self, index) -> Tuple[Optional[Iterator[FakeResource]], bool]: + return iter([FakeResource()]), len(self.data) > 0 + + +class TestFakeLazyResources: + def test(self): + resources = FakeLazyResources() + assert resources == resources.find() + assert resources.find_one() + assert resources.get(None) diff --git a/src/posit/connect/users.py b/src/posit/connect/users.py index c28fc53f..248debeb 100644 --- a/src/posit/connect/users.py +++ b/src/posit/connect/users.py @@ -1,17 +1,81 @@ +from __future__ import annotations + import os -from requests import Session, Response +from datetime import datetime +from typing import Iterator, Callable, List + +from requests import Session + +from .config import Config +from .resources import LazyResources, Resource, Resources + +_MAX_PAGE_SIZE = 500 + + +class User(Resource, total=False): + guid: str + email: str + username: str + first_name: str + last_name: str + user_role: str + created_time: datetime + updated_time: datetime + active_time: datetime + confirmed: bool + locked: bool + + +class Users(Resources[User]): + def find(self, filter: Callable[[User], bool] = lambda _: True) -> Users: + return Users([user for user in self if filter(user)]) + + def find_one(self, filter: Callable[[User], bool] = lambda _: True) -> User | None: + return next((user for user in self if filter(user)), None) + + def get(self, id: str) -> User: + user = next((user for user in self if user["guid"] == id), None) + if user is None: + raise RuntimeError(f"failed to get user with id '{id}'") + return user -class Users: - def __init__(self, endpoint: str, session: Session) -> None: - self._endpoint = endpoint - self._session = session +class LazyUsers(Users, LazyResources[User]): + def __init__( + self, config: Config, session: Session, *, page_size=_MAX_PAGE_SIZE + ) -> None: + super().__init__() + self.config = config + self.session = session + self.page_size = page_size - def get_user(self, user_id: str) -> Response: - endpoint = os.path.join(self._endpoint, "__api__/v1/users", user_id) - return self._session.get(endpoint) + def fetch(self, index) -> tuple[Iterator[User] | None, bool]: + # Check if index is a multiple of page_size. + if (index % self.page_size) != 0: + # + raise ValueError( + f"index ({index}) must be a multiple of page size ({self.page_size})" + ) + # Construct the endpoint URL. + endpoint = os.path.join(self.config.endpoint, "v1/users") + # Define the page number using 1-based indexing. + page_number = int(index / self.page_size) + 1 + # Define query parameters for pagination. + params = {"page_number": page_number, "page_size": self.page_size} + # Send a GET request to the endpoint with the specified parameters. + response = self.session.get(endpoint, params=params) + # Convert response to dict + json: dict = dict(response.json()) + # Parse the JSON response and extract the results. + results: List[dict] = json["results"] + # Mark exhausted if the result size is less than the maximum page size. + exhausted = len(results) < self.page_size + # Create User objects from the results and return them as a list. + users: Iterator[User] = iter(User(**result) for result in results) + return (users, exhausted) - def get_current_user(self) -> Response: - endpoint = os.path.join(self._endpoint, "__api__/v1/user") - return self._session.get(endpoint) + def get(self, id: str) -> User: + endpoint = os.path.join(self.config.endpoint, "v1/users", id) + response = self.session.get(endpoint) + return User(**response.json()) diff --git a/src/posit/connect/users_test.py b/src/posit/connect/users_test.py index 689abcfd..e69de29b 100644 --- a/src/posit/connect/users_test.py +++ b/src/posit/connect/users_test.py @@ -1,21 +0,0 @@ -from unittest.mock import Mock - -from .users import Users - - -class TestUsers: - def test_get_user(self): - session = Mock() - session.get = Mock(return_value={}) - users = Users(endpoint="http://foo.bar/", session=session) - response = users.get_user(user_id="foo") - assert response == {} - session.get.assert_called_once_with("http://foo.bar/__api__/v1/users/foo") - - def test_get_current_user(self): - session = Mock() - session.get = Mock(return_value={}) - users = Users(endpoint="http://foo.bar/", session=session) - response = users.get_current_user() - assert response == {} - session.get.assert_called_once_with("http://foo.bar/__api__/v1/user") diff --git a/tinkering.py b/tinkering.py index 4a025882..a8030972 100644 --- a/tinkering.py +++ b/tinkering.py @@ -1,5 +1,10 @@ -from posit.connect.client import Client +from posit.connect.client import create_client -client = Client() -res = client.users.get_current_user() -print(res.json()) +with create_client() as client: + 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)