diff --git a/src/posit/connect/client.py b/src/posit/connect/client.py index a5ecae2c..63b20f9b 100644 --- a/src/posit/connect/client.py +++ b/src/posit/connect/client.py @@ -1,13 +1,50 @@ +import os + from requests import Session from typing import Optional from . import hooks from .auth import Auth -from .config import ConfigBuilder from .users import Users +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 + + class Client: users: Users @@ -16,21 +53,9 @@ def __init__( api_key: Optional[str] = None, endpoint: Optional[str] = None, ) -> None: - builder = ConfigBuilder() - if api_key: - builder.set_api_key(api_key) - if endpoint: - builder.set_endpoint(endpoint) - self._config = builder.build() - - if self._config.api_key is None: - raise ValueError("Invalid value for 'api_key': Must be a non-empty string.") - if self._config.endpoint is None: - raise ValueError( - "Invalid value for 'endpoint': Must be a non-empty string." - ) - + 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._config.api_key) - self.users = Users(self._config.endpoint, self._session) + self._session.auth = Auth(self._api_key) + self.users = Users(self._endpoint, self._session) diff --git a/src/posit/connect/client_test.py b/src/posit/connect/client_test.py index c91f2e0c..ba64e366 100644 --- a/src/posit/connect/client_test.py +++ b/src/posit/connect/client_test.py @@ -1,56 +1,52 @@ import pytest -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import MagicMock, patch -from .client import Client +from .client import Client, _get_api_key, _get_endpoint class TestClient: + @patch("posit.connect.client.Users") @patch("posit.connect.client.Session") - @patch("posit.connect.client.ConfigBuilder") @patch("posit.connect.client.Auth") - def test_init(self, Auth: MagicMock, ConfigBuilder: MagicMock, Session: MagicMock): + def test_init(self, Auth: MagicMock, Session: MagicMock, Users: MagicMock): api_key = "foobar" endpoint = "http://foo.bar" - config = Mock() - config.api_key = api_key - builder = ConfigBuilder.return_value - builder.set_api_key = Mock() - builder.set_endpoint = Mock() - builder.build = Mock(return_value=config) client = Client(api_key=api_key, endpoint=endpoint) - ConfigBuilder.assert_called_once() - builder.set_api_key.assert_called_once_with(api_key) - builder.set_endpoint.assert_called_once_with(endpoint) - builder.build.assert_called_once() + assert client._api_key == api_key + assert client._endpoint == endpoint Session.assert_called_once() Auth.assert_called_once_with(api_key) - assert client._config == config + Users.assert_called_once_with(endpoint, Session.return_value) - @patch("posit.connect.client.ConfigBuilder") - def test_init_without_api_key(self, ConfigBuilder: MagicMock): - api_key = None - endpoint = "http://foo.bar" - config = Mock() - config.api_key = api_key - config.endpoint = endpoint - builder = ConfigBuilder.return_value - builder.set_api_key = Mock() - builder.set_endpoint = Mock() - builder.build = Mock(return_value=config) + +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): - Client(api_key=api_key, endpoint=endpoint) + _get_api_key() - @patch("posit.connect.client.ConfigBuilder") - def test_init_without_endpoint(self, ConfigBuilder: MagicMock): - api_key = "foobar" - endpoint = None - config = Mock() - config.api_key = api_key - config.endpoint = endpoint - builder = ConfigBuilder.return_value - builder.set_api_key = Mock() - builder.set_endpoint = Mock() - builder.build = Mock(return_value=config) + 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): - Client(api_key=api_key, endpoint=endpoint) + _get_endpoint() diff --git a/src/posit/connect/config.py b/src/posit/connect/config.py deleted file mode 100644 index ecbfc131..00000000 --- a/src/posit/connect/config.py +++ /dev/null @@ -1,68 +0,0 @@ -import os -import dataclasses - -from abc import ABC, abstractmethod -from dataclasses import dataclass -from typing import List, Optional - - -@dataclass -class Config: - api_key: Optional[str] = None - endpoint: Optional[str] = None - - -class ConfigProvider(ABC): - @abstractmethod - def get_value(self, key: str) -> Optional[str]: - raise NotImplementedError # pragma: no cover - - -class EnvironmentConfigProvider(ConfigProvider): - def get_value(self, key: str) -> Optional[str]: - if key == "api_key": - value = os.environ.get("CONNECT_API_KEY") - if value: - return value - if value == "": - raise ValueError( - "Invalid value for 'CONNECT_API_KEY': Must be a non-empty string." - ) - - if key == "endpoint": - value = os.environ.get("CONNECT_SERVER") - if value: - return os.path.join(value, "__api__") - if value == "": - raise ValueError( - "Invalid value for 'CONNECT_SERVER': Must be a non-empty string." - ) - - return None - - -class ConfigBuilder: - def __init__( - self, providers: List[ConfigProvider] = [EnvironmentConfigProvider()] - ) -> None: - self._config = Config() - self._providers = providers - - def build(self) -> Config: - for field in dataclasses.fields(Config): - key = field.name - if not getattr(self._config, key): - setattr( - self._config, - key, - next( - (provider.get_value(key) for provider in self._providers), None - ), - ) - return self._config - - def set_api_key(self, api_key: str): - self._config.api_key = api_key - - def set_endpoint(self, endpoint: str): - self._config.endpoint = endpoint diff --git a/src/posit/connect/config_test.py b/src/posit/connect/config_test.py deleted file mode 100644 index 10e431d2..00000000 --- a/src/posit/connect/config_test.py +++ /dev/null @@ -1,70 +0,0 @@ -import pytest - -from unittest.mock import Mock, patch - -from .config import Config, ConfigBuilder, EnvironmentConfigProvider - - -class TestEnvironmentConfigProvider: - @patch.dict("os.environ", {"CONNECT_API_KEY": "foobar"}) - def test_get_api_key(self): - provider = EnvironmentConfigProvider() - api_key = provider.get_value("api_key") - assert api_key == "foobar" - - @patch.dict("os.environ", {"CONNECT_API_KEY": ""}) - def test_get_api_key_empty(self): - provider = EnvironmentConfigProvider() - with pytest.raises(ValueError): - provider.get_value("api_key") - - def test_get_api_key_miss(self): - provider = EnvironmentConfigProvider() - api_key = provider.get_value("api_key") - assert api_key is None - - @patch.dict("os.environ", {"CONNECT_SERVER": "http://foo.bar"}) - def test_get_endpoint(self): - provider = EnvironmentConfigProvider() - endpoint = provider.get_value("endpoint") - assert endpoint == "http://foo.bar/__api__" - - @patch.dict("os.environ", {"CONNECT_SERVER": ""}) - def test_get_endpoint_empty(self): - provider = EnvironmentConfigProvider() - with pytest.raises(ValueError): - provider.get_value("endpoint") - - def test_get_endpoint_miss(self): - provider = EnvironmentConfigProvider() - endpoint = provider.get_value("endpoint") - assert endpoint is None - - def test_get_value_miss(self): - provider = EnvironmentConfigProvider() - value = provider.get_value("foobar") - assert value is None - - -class TestConfigBuilder: - def test_build(self): - builder = ConfigBuilder() - assert builder._config == Config() - - def test_build_with_provider(self): - provider = Mock() - provider.get_value = Mock() - builder = ConfigBuilder([provider]) - builder.build() - for key in Config.__annotations__: - provider.get_value.assert_any_call(key) - - def test_set_api_key(self): - builder = ConfigBuilder() - builder.set_api_key("foobar") - assert builder._config.api_key == "foobar" - - def test_set_endpoint(self): - builder = ConfigBuilder() - builder.set_endpoint("http://foo.bar") - assert builder._config.endpoint == "http://foo.bar" diff --git a/src/posit/connect/users.py b/src/posit/connect/users.py index ade6a8bb..c28fc53f 100644 --- a/src/posit/connect/users.py +++ b/src/posit/connect/users.py @@ -9,9 +9,9 @@ def __init__(self, endpoint: str, session: Session) -> None: self._session = session def get_user(self, user_id: str) -> Response: - endpoint = os.path.join(self._endpoint, "v1/users", user_id) + endpoint = os.path.join(self._endpoint, "__api__/v1/users", user_id) return self._session.get(endpoint) def get_current_user(self) -> Response: - endpoint = os.path.join(self._endpoint, "v1/user") + endpoint = os.path.join(self._endpoint, "__api__/v1/user") return self._session.get(endpoint) diff --git a/src/posit/connect/users_test.py b/src/posit/connect/users_test.py index b79463e8..689abcfd 100644 --- a/src/posit/connect/users_test.py +++ b/src/posit/connect/users_test.py @@ -10,7 +10,7 @@ def test_get_user(self): 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/v1/users/foo") + session.get.assert_called_once_with("http://foo.bar/__api__/v1/users/foo") def test_get_current_user(self): session = Mock() @@ -18,4 +18,4 @@ def test_get_current_user(self): users = Users(endpoint="http://foo.bar/", session=session) response = users.get_current_user() assert response == {} - session.get.assert_called_once_with("http://foo.bar/v1/user") + session.get.assert_called_once_with("http://foo.bar/__api__/v1/user")