Repository: ithaka/apiron

Issue:
Add endpoints attribute to services
**Is your feature request related to a problem? Please describe.**
It would occasionally be useful to inspect all the endpoints associated with a particular service by accessing an attribute like `SomeService.endpoints`.

**Describe the solution you'd like**
An `endpoints` property is added to the `Service` class to get a list of that service's endpoints.

**Describe alternatives you've considered**
You can inspect the endpoints somewhat by manually exploring `dir(SomeService)`, but this could be less helpful for services you're unfamiliar with (you might not what which things are endpoints vs. other attributes).

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 23d52c1..804d8a9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+### Added
+- An `endpoints` attribute on `Service` classes returns a list of that service's configured endpoints
+- `Endpoint` classes now have a useful `__repr__` implementation, e.g. `"JsonEndpoint(path='/foo')"`.
+
+### Changed
+- `StubEndpoint` now inherits from `Endpoint` and as a result does not accept arbitrary keyword arguments
+
 ## [3.0.0] - 2019-06-20
 ### Added
 - More testing for the bug fixed in v2.6.1
diff --git a/apiron/client.py b/apiron/client.py
index 54f57a9..71b3b82 100644
--- a/apiron/client.py
+++ b/apiron/client.py
@@ -207,32 +207,17 @@ def call(
     """
     logger = logger or LOGGER
 
-    if hasattr(endpoint, "stub_response"):
-        logger.info("Stub call for endpoint defined by {}".format(getattr(endpoint, "endpoint_params", {})))
-        if callable(endpoint.stub_response):
-            return endpoint.stub_response(
-                method=method or getattr(endpoint, "default_method", "GET"),
-                params=params,
-                data=data,
-                json=json,
-                headers=headers,
-                cookies=cookies,
-                auth=auth,
-            )
-        else:
-            return endpoint.stub_response
-
-    managing_session = False
-
-    if not session:
-        session = get_adapted_session(adapters.HTTPAdapter(max_retries=retry_spec))
-        managing_session = True
+    managing_session = not session
+
+    session = get_adapted_session(adapters.HTTPAdapter(max_retries=retry_spec)) if managing_session else session
+
+    method = method or endpoint.default_method
 
     request = build_request_object(
         session,
         service,
         endpoint,
-        method=method or endpoint.default_method,
+        method=method,
         params=params,
         data=data,
         json=json,
@@ -242,7 +227,7 @@ def call(
         **kwargs
     )
 
-    logger.info("{method} {url}".format(method=method or endpoint.default_method, url=request.url))
+    logger.info("{method} {url}".format(method=method, url=request.url))
 
     response = session.send(
         request,
diff --git a/apiron/endpoint/endpoint.py b/apiron/endpoint/endpoint.py
index 653acaa..04421a5 100644
--- a/apiron/endpoint/endpoint.py
+++ b/apiron/endpoint/endpoint.py
@@ -16,11 +16,9 @@ class Endpoint:
     """
 
     def __get__(self, instance, owner):
-        if not instance:
-            caller = partial(client.call, owner, self)
-            update_wrapper(caller, client.call)
-            return caller
-        return self
+        caller = partial(client.call, owner, self)
+        update_wrapper(caller, client.call)
+        return caller
 
     def __call__(self):
         raise TypeError("Endpoints are only callable in conjunction with a Service class.")
@@ -114,6 +112,29 @@ class Endpoint:
                 stacklevel=6,
             )
 
+    def _check_for_empty_params(self, params):
+        empty_params = {param: params[param] for param in params if params[param] in (None, "")}
+
+        if empty_params:
+            warnings.warn(
+                "The {path} endpoint "
+                "was called with empty parameters: {empty_params}".format(path=self.path, empty_params=empty_params),
+                RuntimeWarning,
+                stacklevel=6,
+            )
+
+    def _check_for_unfulfilled_params(self, params):
+        unfulfilled_params = {
+            param for param in self.required_params if param not in params and param not in self.default_params
+        }
+
+        if unfulfilled_params:
+            raise UnfulfilledParameterException(self.path, unfulfilled_params)
+
+    def _validate_params(self, params):
+        self._check_for_empty_params(params)
+        self._check_for_unfulfilled_params(params)
+
     def get_merged_params(self, supplied_params=None):
         """
         Merge this endpoint's default parameters with the supplied parameters
@@ -130,23 +151,7 @@ class Endpoint:
         """
         supplied_params = supplied_params or {}
 
-        empty_params = {
-            param: supplied_params[param] for param in supplied_params if supplied_params[param] in (None, "")
-        }
-        if empty_params:
-            warnings.warn(
-                "The {path} endpoint "
-                "was called with empty parameters: {empty_params}".format(path=self.path, empty_params=empty_params),
-                RuntimeWarning,
-                stacklevel=5,
-            )
-
-        unfulfilled_params = {
-            param for param in self.required_params if param not in supplied_params and param not in self.default_params
-        }
-
-        if unfulfilled_params:
-            raise UnfulfilledParameterException(self.path, unfulfilled_params)
+        self._validate_params(supplied_params)
 
         merged_params = self.default_params.copy()
         merged_params.update(supplied_params)
@@ -154,3 +159,6 @@ class Endpoint:
 
     def __str__(self):
         return self.path
+
+    def __repr__(self):
+        return "{klass}(path='{path}')".format(klass=self.__class__.__name__, path=self.path)
diff --git a/apiron/endpoint/stub.py b/apiron/endpoint/stub.py
index e09e972..57cf562 100644
--- a/apiron/endpoint/stub.py
+++ b/apiron/endpoint/stub.py
@@ -1,9 +1,7 @@
-from functools import partial, update_wrapper
+from apiron.endpoint import Endpoint
 
-from apiron import client
 
-
-class StubEndpoint:
+class StubEndpoint(Endpoint):
     """
     A stub endpoint designed to return a pre-baked response
 
@@ -12,14 +10,7 @@ class StubEndpoint:
     """
 
     def __get__(self, instance, owner):
-        if not instance:
-            caller = partial(client.call, owner, self)
-            update_wrapper(caller, client.call)
-            return caller
-        return self
-
-    def __call__(self):
-        raise TypeError("Endpoints are only callable in conjunction with a Service class.")
+        return self.stub_response
 
     def __init__(self, stub_response=None, **kwargs):
         """
@@ -40,5 +31,10 @@ class StubEndpoint:
             Arbitrary parameters that can match the intended real endpoint.
             These don't do anything for the stub but streamline the interface.
         """
-        self.endpoint_params = kwargs or {}
-        self.stub_response = stub_response or "stub for {}".format(self.endpoint_params)
+        super().__init__(**kwargs)
+        if callable(stub_response):
+            self.stub_response = stub_response
+        elif stub_response:
+            self.stub_response = lambda *args, **kwargs: stub_response
+        else:
+            self.stub_response = lambda *args, **kwargs: {"response": repr(self)}
diff --git a/apiron/service/base.py b/apiron/service/base.py
index 470ca72..0f553f4 100644
--- a/apiron/service/base.py
+++ b/apiron/service/base.py
@@ -1,8 +1,15 @@
+from apiron import Endpoint
+
+
 class ServiceMeta(type):
     @property
     def required_headers(cls):
         return cls().required_headers
 
+    @property
+    def endpoints(cls):
+        return {attr for attr_name, attr in cls.__dict__.items() if isinstance(attr, Endpoint)}
+
     def __str__(cls):
         return str(cls())
 


Test patch:
diff --git a/tests/service/test_base.py b/tests/service/test_base.py
index 405f69e..8ee3a48 100644
--- a/tests/service/test_base.py
+++ b/tests/service/test_base.py
@@ -1,6 +1,6 @@
 import pytest
 
-from apiron import Service, ServiceBase
+from apiron import Endpoint, Service, ServiceBase
 
 
 @pytest.fixture(scope="class")
@@ -26,14 +26,25 @@ class TestService:
     def test_str_method_on_class(self, service):
         assert "http://foo.com" == str(service)
 
-    def test_str_method_on_instance(self, service):
-        assert "http://foo.com" == str(service())
-
     def test_repr_method_on_class(self, service):
         assert "SomeService(domain=http://foo.com)" == repr(service)
 
-    def test_repr_method_on_instance(self, service):
-        assert "SomeService(domain=http://foo.com)" == repr(service())
-
     def test_required_headers_returns_empty_dict_by_default(self, service):
         assert {} == service.required_headers
+
+    def test_endpoints_when_no_endpoints(self, service):
+        assert service.endpoints == set()
+
+    def test_endpoints_when_one_endpoint(self, service):
+        foo = Endpoint(path="/foo")
+        service.foo = foo
+        assert service.endpoints == {foo}
+
+    def test_endpoints_when_multiple_endpoints(self, service):
+        foo = Endpoint(path="/foo")
+        bar = Endpoint(path="/bar")
+
+        service.foo = foo
+        service.bar = bar
+
+        assert service.endpoints == {foo, bar}
diff --git a/tests/service/test_discoverable.py b/tests/service/test_discoverable.py
index ddb4cc3..5cd430b 100644
--- a/tests/service/test_discoverable.py
+++ b/tests/service/test_discoverable.py
@@ -26,11 +26,5 @@ class TestDiscoverableService:
     def test_str_method_on_class(self, service):
         assert "fake-service" == str(service)
 
-    def test_str_method_on_instance(self, service):
-        assert "fake-service" == str(service())
-
     def test_repr_method_on_class(self, service):
         assert "FakeService(service_name=fake-service, host_resolver=FakeResolver)" == repr(service)
-
-    def test_repr_method_on_instance(self, service):
-        assert "FakeService(service_name=fake-service, host_resolver=FakeResolver)" == repr(service())
diff --git a/tests/test_client.py b/tests/test_client.py
index bbc3e79..a11d738 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -216,77 +216,6 @@ class TestClient:
             client.choose_host(service)
 
 
-@mock.patch("apiron.client.Timeout")
-@mock.patch("apiron.client.get_adapted_session")
-@mock.patch("apiron.client.build_request_object")
-@mock.patch("requests.adapters.HTTPAdapter", autospec=True)
-@mock.patch("requests.Session", autospec=True)
-class TestClientStubCall:
-    def test_call_stub(
-        self, MockSession, MockAdapter, mock_build_request_object, mock_get_adapted_session, mock_timeout
-    ):
-        """
-        Test getting a response for a ``StubEndpoint``
-        """
-        service = mock.Mock()
-        service.get_hosts.return_value = ["http://host1.biz"]
-
-        stub_endpoint = mock.Mock()
-        stub_endpoint.stub_response = {"stub": "response"}
-
-        actual_response = client.call(service, stub_endpoint)
-
-        expected_response = {"stub": "response"}
-
-        assert expected_response == actual_response
-
-    def test_call_stub_dynamic(
-        self, MockSession, MockAdapter, mock_build_request_object, mock_get_adapted_session, mock_timeout
-    ):
-        """
-        Test getting a response for a ``StubEndpoint`` using a dynamic response
-        """
-        service = mock.Mock()
-        service.get_hosts.return_value = ["http://host1.biz"]
-
-        stub_endpoint = mock.Mock()
-
-        def stub_response(**kwargs):
-            return "stub response"
-
-        stub_endpoint.stub_response = stub_response
-
-        actual_response = client.call(service, stub_endpoint)
-
-        expected_response = "stub response"
-
-        assert expected_response == actual_response
-
-    def test_call_stub_dynamic_params(
-        self, MockSession, MockAdapter, mock_build_request_object, mock_get_adapted_session, mock_timeout
-    ):
-        """
-        Test getting a response for a ``StubEndpoint`` using a dynamic response with parameters
-        """
-        service = mock.Mock()
-        service.get_hosts.return_value = ["http://host1.biz"]
-
-        stub_endpoint = mock.Mock()
-
-        def stub_response(**kwargs):
-            response_map = {"param_value": "correct", "default": "incorrect"}
-            data_key = kwargs["params"].setdefault("param_key", "default")
-            return response_map[data_key]
-
-        stub_endpoint.stub_response = stub_response
-
-        actual_response = client.call(service, stub_endpoint, params={"param_key": "param_value"})
-
-        expected_response = "correct"
-
-        assert expected_response == actual_response
-
-
 @pytest.mark.parametrize(
     "host,path,url",
     [
diff --git a/tests/test_endpoint.py b/tests/test_endpoint.py
index ba4b2d1..e832395 100644
--- a/tests/test_endpoint.py
+++ b/tests/test_endpoint.py
@@ -4,7 +4,6 @@ from unittest import mock
 import pytest
 
 import apiron
-from apiron import client
 
 
 @pytest.fixture
@@ -15,6 +14,17 @@ def service():
     return SomeService
 
 
+@pytest.fixture
+def stub_function():
+    def stub_response(**kwargs):
+        if kwargs.get("params") and kwargs["params"].get("param_key") == "param_value":
+            return {"stub response": "stubby!"}
+        else:
+            return {"default": "response"}
+
+    return stub_response
+
+
 class TestEndpoint:
     def test_call(self, service):
         service.foo = apiron.Endpoint()
@@ -45,10 +55,6 @@ class TestEndpoint:
         foo = apiron.Endpoint()
         assert {} == foo.required_headers
 
-    def test_str_method(self):
-        foo = apiron.Endpoint(path="/foo/bar/")
-        assert "/foo/bar/" == str(foo)
-
     def test_path_placeholders_when_none_present(self):
         foo = apiron.Endpoint()
         assert [] == foo.path_placeholders
@@ -80,35 +86,31 @@ class TestEndpoint:
             _ = apiron.Endpoint(path="/?foo=bar")
 
     def test_get_merged_params(self):
-        foo = apiron.JsonEndpoint(default_params={"foo": "bar"}, required_params={"baz"})
+        foo = apiron.Endpoint(default_params={"foo": "bar"}, required_params={"baz"})
         assert {"foo": "bar", "baz": "qux"} == foo.get_merged_params({"baz": "qux"})
 
     def test_get_merged_params_with_unsupplied_param(self):
-        foo = apiron.JsonEndpoint(default_params={"foo": "bar"}, required_params={"baz"})
+        foo = apiron.Endpoint(default_params={"foo": "bar"}, required_params={"baz"})
 
         with pytest.raises(apiron.UnfulfilledParameterException):
             foo.get_merged_params()
 
     def test_get_merged_params_with_empty_param(self):
-        foo = apiron.JsonEndpoint(default_params={"foo": "bar"}, required_params={"baz"})
+        foo = apiron.Endpoint(default_params={"foo": "bar"}, required_params={"baz"})
         with pytest.warns(RuntimeWarning, match="endpoint was called with empty parameters"):
             assert {"foo": "bar", "baz": None} == foo.get_merged_params({"baz": None})
 
     def test_get_merged_params_with_required_and_default_param(self):
-        foo = apiron.JsonEndpoint(default_params={"foo": "bar"}, required_params={"foo"})
+        foo = apiron.Endpoint(default_params={"foo": "bar"}, required_params={"foo"})
         assert {"foo": "bar"} == foo.get_merged_params()
 
-    @mock.patch("apiron.client.Timeout")
-    @mock.patch("requests.Session", autospec=True)
-    def test_legacy_endpoint_usage_with_instantiated_service(self, MockSession, mock_timeout, service):
-        service.foo = apiron.Endpoint(path="/foo/")
-        instantiated_service = service()
-
-        mock_logger = mock.Mock()
-        request = mock.Mock()
-        request.url = "http://host1.biz/foo/"
+    def test_str_method(self):
+        foo = apiron.Endpoint(path="/bar/baz")
+        assert str(foo) == "/bar/baz"
 
-        client.call(instantiated_service, instantiated_service.foo, timeout_spec=mock_timeout, logger=mock_logger)
+    def test_repr_method(self):
+        foo = apiron.Endpoint(path="/bar/baz")
+        assert repr(foo) == "Endpoint(path='/bar/baz')"
 
 
 class TestJsonEndpoint:
@@ -134,6 +136,14 @@ class TestJsonEndpoint:
         foo = apiron.JsonEndpoint()
         assert {"Accept": "application/json"} == foo.required_headers
 
+    def test_str_method(self):
+        foo = apiron.JsonEndpoint(path="/bar/baz")
+        assert str(foo) == "/bar/baz"
+
+    def test_repr_method(self):
+        foo = apiron.JsonEndpoint(path="/bar/baz")
+        assert repr(foo) == "JsonEndpoint(path='/bar/baz')"
+
 
 class TestStreamingEndpoint:
     def test_format_response(self):
@@ -141,70 +151,42 @@ class TestStreamingEndpoint:
         mock_response = mock.Mock()
         assert mock_response.iter_content(chunk_size=None) == foo.format_response(mock_response)
 
+    def test_str_method(self):
+        foo = apiron.StreamingEndpoint(path="/bar/baz")
+        assert str(foo) == "/bar/baz"
+
+    def test_repr_method(self):
+        foo = apiron.StreamingEndpoint(path="/bar/baz")
+        assert repr(foo) == "StreamingEndpoint(path='/bar/baz')"
+
 
 class TestStubEndpoint:
-    def test_stub_response(self):
-        """
-        Test initializing a stub endpoint with a stub response
-        """
-        stub_endpoint = apiron.StubEndpoint(stub_response="stub response")
-        assert "stub response" == stub_endpoint.stub_response
-
-    def test_extra_params(self):
-        """
-        Test initializing a stub endpoint with extra params
-        """
-        stub_endpoint = apiron.StubEndpoint(
-            stub_response="stub response",
-            path="/some/path/",
-            default_params={"param_name": "param_val"},
-            required_params={"param_name"},
-            arbitrary_kwarg="foo",
-        )
-        expected_params = {
-            "path": "/some/path/",
-            "default_params": {"param_name": "param_val"},
-            "required_params": {"param_name"},
-            "arbitrary_kwarg": "foo",
-        }
-        assert expected_params == stub_endpoint.endpoint_params
+    def test_stub_default_response(self, service):
+        service.stub_endpoint = apiron.StubEndpoint()
+        assert service.stub_endpoint() == {"response": "StubEndpoint(path='/')"}
 
     def test_call_static(self, service):
-        """
-        Test calling a ``StubEndpoint`` with a static response
-        """
         service.stub_endpoint = apiron.StubEndpoint(stub_response="stub response")
-        actual_response = service.stub_endpoint()
-        expected_response = "stub response"
+        assert service.stub_endpoint() == "stub response"
+
+    @pytest.mark.parametrize(
+        "test_call_kwargs,expected_response",
+        [({}, {"default": "response"}), ({"params": {"param_key": "param_value"}}, {"stub response": "stubby!"})],
+    )
+    def test_call_dynamic(self, test_call_kwargs, expected_response, service, stub_function):
+        service.stub_endpoint = apiron.StubEndpoint(stub_response=stub_function)
+        actual_response = service.stub_endpoint(**test_call_kwargs)
         assert actual_response == expected_response
 
-    def test_call_dynamic(self, service):
-        """
-        Test calling a StubEndpoint with a dynamic response
-        """
-
-        def _test_case(call_kwargs, expected_response):
-            def stub_response(**kwargs):
-                if kwargs.get("params") and kwargs["params"].get("param_key") == "param_value":
-                    return {"stub response": "for param_key=param_value"}
-                else:
-                    return {"default": "response"}
-
-            service.stub_endpoint = apiron.StubEndpoint(stub_response=stub_response)
-            actual_response = service.stub_endpoint(**call_kwargs)
-            assert actual_response == expected_response
-
-        _test_case(call_kwargs={}, expected_response={"default": "response"})
-        _test_case(
-            call_kwargs={"params": {"param_key": "param_value"}},
-            expected_response={"stub response": "for param_key=param_value"},
-        )
-
     def test_call_without_service_raises_exception(self):
         stub_endpoint = apiron.StubEndpoint(stub_response="foo")
         with pytest.raises(TypeError):
             stub_endpoint()
 
-    def test_call_with_initialized_service_works(self, service):
-        service.stub = apiron.StubEndpoint(stub_response="foo")
-        assert client.call(service(), service().stub) == "foo"
+    def test_str_method(self):
+        foo = apiron.StubEndpoint(path="/bar/baz")
+        assert str(foo) == "/bar/baz"
+
+    def test_repr_method(self):
+        foo = apiron.StubEndpoint(path="/bar/baz")
+        assert repr(foo) == "StubEndpoint(path='/bar/baz')"